fix(#755): collapse duplicate engines keys, teach gate-toolchain to detect them - #822
Merged
Merged
Conversation
…etect them Root cause: the #646/#647 engines sweep appended a second "engines" block to 32 package.json manifests that already had one (likely a re-run of the sweep over an already-touched worklist). JSON.parse silently keeps the LAST occurrence of a duplicate key, so every one of these manifests parsed to a single well-formed engines value and nothing complained -- gate-toolchain's entire job is policing engines, and it was structurally blind to engines being declared twice, because it reads manifests via JSON.parse. PR #814 already fixed 4 of the original 36 (sdk, services/billing-service, services/email-service, services/sms-service) as a side effect of an axios bump. Re-measured from latest origin/master: 32 manifests were still affected. All 32 duplicate pairs were verified byte-identical (node ">=24.0.0", npm ">=10.0.0") before collapsing -- no divergent copies were found, so this is a mechanical de-dup, not a behavior change. Part B: gate-toolchain.mjs now tokenizes each package.json's raw text and walks brace/bracket nesting to find top-level keys declared more than once, BEFORE any JSON.parse-based check runs -- so a duplicate is caught even though JSON.parse can't see it. Generalized beyond "engines" per the issue's suggestion: any duplicated top-level key in a manifest is a latent version of this same bug (JSON.parse takes last, other parsers take first or reject, npm normalises and silently discards one). Verified the new check both detects the regression (re-ran the gate against the pre-fix duplicates via git stash -- 12/12 correctly flagged and failed) and produces zero false positives on legitimately repeated key names nested under different parents. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013tMciHkPE8To7V67CsgKKc
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
1 participant
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.
Root cause: the #646/#647 engines sweep appended a second "engines" block to
32 package.json manifests that already had one (likely a re-run of the sweep
over an already-touched worklist). JSON.parse silently keeps the LAST
occurrence of a duplicate key, so every one of these manifests parsed to a
single well-formed engines value and nothing complained -- gate-toolchain's
entire job is policing engines, and it was structurally blind to engines
being declared twice, because it reads manifests via JSON.parse.
PR #814 already fixed 4 of the original 36 (sdk, services/billing-service,
services/email-service, services/sms-service) as a side effect of an axios
bump. Re-measured from latest origin/master: 32 manifests were still
affected. All 32 duplicate pairs were verified byte-identical
(node ">=24.0.0", npm ">=10.0.0") before collapsing -- no divergent copies
were found, so this is a mechanical de-dup, not a behavior change.
Part B: gate-toolchain.mjs now tokenizes each package.json's raw text and
walks brace/bracket nesting to find top-level keys declared more than once,
BEFORE any JSON.parse-based check runs -- so a duplicate is caught even
though JSON.parse can't see it. Generalized beyond "engines" per the issue's
suggestion: any duplicated top-level key in a manifest is a latent version of
this same bug (JSON.parse takes last, other parsers take first or reject,
npm normalises and silently discards one). Verified the new check both
detects the regression (re-ran the gate against the pre-fix duplicates via
git stash -- 12/12 correctly flagged and failed) and produces zero false
positives on legitimately repeated key names nested under different parents.
Co-Authored-By: Claude Opus 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_013tMciHkPE8To7V67CsgKKc