fix(workflows): report overlay operation keys in declaration order - #4146
Open
jawwad-ali wants to merge 1 commit into
Open
fix(workflows): report overlay operation keys in declaration order#4146jawwad-ali wants to merge 1 commit into
jawwad-ali wants to merge 1 commit into
Conversation
`_parse_edit` collected the shorthand operation keys by iterating the
frozenset:
shorthand_keys = [key for key in _SHORTHAND_OPERATION_KEYS if key in edit_raw]
`_SHORTHAND_OPERATION_KEYS` is `VALID_OPERATIONS`, a frozenset, so its
iteration order depends on per-process string-hash randomization. The two
error messages built from that list named the offending keys in a different
order on every run for the exact same overlay file:
["Edit at index 0 has multiple operation keys: 'insert_after', 'remove'."]
["Edit at index 0 has multiple operation keys: 'remove', 'insert_after'."]
["Edit at index 0 has multiple operation keys: 'remove', 'insert_after'."]
Iterating `edit_raw` instead yields the user's declared order and is
deterministic. Dict keys are always hashable, so the membership test is
safe in this direction too.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
_parse_editcollects the shorthand operation keys by iterating the frozenset:_SHORTHAND_OPERATION_KEYSisVALID_OPERATIONS, afrozenset. Its iteration order for these strings depends on per-process string-hash randomization, so the two error messages built from that list name the offending keys in a different order on every run for the exact same overlay file.Reproduction on current
mainSix consecutive fresh processes, same input:
Underlying cause, same six processes:
Both messages are affected — the multiple-keys one above, and
"...mixes shorthand operation key ({shorthand_keys[0]!r})...", which picks an arbitrary one of the user's keys to name.This is reached by a very ordinary YAML slip: forgetting the
-on a second edit merges both operation keys into one mapping.Fix
Iterate
edit_raw, which yields the user's declared order and is deterministic. Dict keys are always hashable, so the membership test is safe in this direction too.Verification
The test is designed to fail deterministically in every process, not ~50% of the time: it parametrizes both declaration orders. Whatever fixed order a frozenset happens to have in a given process, one of the two parametrizations must contradict it.
insert_afterfirst, so the tworemove-firstcases were the ones that failed — in a process with the opposite order, theinsert_after-firstpair fails instead.)tests/workflows: no new failures vs a clean-mainbaseline (10 pre-existing, Windows symlink-privilege).uvx ruff@0.15.0 check src tests→ cleanNo breaking change — message content is unchanged, only its ordering becomes stable. The existing
test_multiple_operation_fields_rejected(which assertsany("multiple" in e.lower() ...)) still passes untouched.Written with assistance from Claude Code. Bug found, reproduced, and verified by me on current
main.