test(compat): the fail-on-red gate's node -e invocation must be top-level (#704) - #706
Open
AhmedElBanna80 wants to merge 2 commits into
Open
test(compat): the fail-on-red gate's node -e invocation must be top-level (#704)#706AhmedElBanna80 wants to merge 2 commits into
AhmedElBanna80 wants to merge 2 commits into
Conversation
…evel (#704) The JS half already asserts each tooth's owning `if` is a top-level statement of the embedded program. The shell half had no sibling, and the asymmetry was live. Wrapping ONLY the invocation — leaving the prelude alone — disarms all three teeth while every existing signal stays byte-identical to baseline: if [ "${KNEXT_RUNTIME:-node}" != "bun" ]; then node -e '…' "${SUMMARY}" fi audit: teeth 0/0/0 · branchesFound 3 · exitReachesStep 0 · escapeHatches 0 Measured against the real step (`bash -e`, KNEXT_RUNTIME=bun, a red 8-failure Bun-lane summary): exit 1 → exit 0, zero `::error::red shard` lines. A night with real test failures concludes SUCCESS and counts toward the 14-night window. It slips every existing guard by construction. Wrapping the WHOLE prelude is already caught, because `shellIfBlocks` skips nested blocks (`i = j`) so the missing-summary branch stops being found — it is the narrower edit that survives. And it introduces no `exit`, so the escape scan has nothing to find. `shellNestingDepthAt` nets block openers against closers on each line, so a construct that opens and closes on one line contributes zero — which is what keeps `"${SUMMARY}"` and `[ ! -f … ]` from registering as nesting. The embedded program is blanked by offset first, or its JS braces would count as shell blocks. MEASURED, NOT ANTICIPATED — the first cut caught four of five shapes and missed the `case` arm: `case … in` opened the block and the very next pattern line, `node)`, closed it again, netting zero at the invocation. Conditional, brace group, subshell and function body were all caught. The shape that shipped broken would have been the one nobody probed, which is why `CASE_PATTERN` exists and why all five are exercised rather than the one that motivated the fix. Rides the existing `expect(escapeHatches).toEqual([])` assertion rather than adding a competing one. Mutation-proved: the wrap reds that assertion, restore verified byte-exact, residue scan clean, baseline green. Closes #704 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0198oVxWkEEDJJnVhES6ULnx
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…ction (#704) Self-review found a limit I had not documented: an unbalanced `(` or `{` inside a QUOTED string counts as an opener that never closes. Measured — `echo "a smiley ( in a string"` nets +1 — so two such lines before the invocation would report nesting that is not there. The DIRECTION matters and is stated explicitly, because this file got exactly that backwards once before (#701 round 9, where a fail-open/fail-closed note was inverted and the "never" was false). This one is **fail-CLOSED**: the only reachable error is a spurious finding, never a missed disarm. That is the opposite direction from #702, whose concern was fail-OPEN — a real `|| true` hidden by mispaired quotes. Conflating the two is how a limitation gets waved through as already-known, so the note says which is which. Not live: the gate's prelude carries no unbalanced bracket inside a string, and the baseline measures 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0198oVxWkEEDJJnVhES6ULnx
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.
Closes #704. Also closes #702 — see below; that one needed no code.
The defect
#701 made the gate's three teeth independently provable inside the step's script, and gave the JS half a "must be top-level" assertion. The shell half had no sibling. Wrapping only the
node -einvocation — leaving the prelude untouched — disarms all three teeth:exit=1, error lines presentexit=0, 0::error::red shardlinesMeasured with
bash -e,KNEXT_RUNTIME=bun, a red 8-failure Bun-lane summary. A night with real test failures concludes SUCCESS and counts toward the 14-night window indocs/compat/window-node-lane.md.It slips every existing guard by construction. Wrapping the whole prelude is already caught —
shellIfBlocksskips nested blocks viai = j, so the missing-summary branch stops being found. The narrower edit survives that. And it introduces noexit, so the escape-hatch scan has nothing to find.The fix
shellNestingDepthAtnets block openers against closers per line, so anything opening and closing on one line contributes zero — which keeps"${SUMMARY}"and[ ! -f … ]from registering as nesting. The embedded program is blanked by offset first, or its JS braces would count as shell blocks.Five shapes now red: conditional, brace group, subshell,
casearm, function body. Baseline 0.The near-miss, recorded because it is the point
The first cut caught four of five and missed the
casearm.case … inopened the block, and the very next pattern line —node)— closed it again, netting zero at the invocation. The other four were caught.The shape that would have shipped broken is the one nobody probes. That is the same failure eleven rounds of review on #701 kept surfacing, and it was caught here only by running all five shapes rather than the one that motivated the fix. Hence
CASE_PATTERN, and hence the five-way probe.#702 needs no code — its subject no longer exists
#702 reported that
blankSingleQuoted's even-quote parity check is necessary but not sufficient, so a|| truebracketed by two stray apostrophes is blanked and survives.blankSingleQuotedwas deleted by #701 round 9. No quote-pairing logic remains intests/helpers/fail-on-red-gate.ts— blanking is offset-based (embeddedNodeProgramSpan) andexitis matched only in command position; neither depends on pairing.Verified empirically rather than by reading, using #702's own reproduction:
Evidence
expect(escapeHatches).toEqual([]); restore byte-exact;scan-mutation-residue.mjsclean; baseline green.tests/compat-suite-workflow.test.ts+tests/compat-shard-flake-attribution.test.ts: 153 passed.What I did not verify
scripts/mutation-prove-compat-fail-on-red-teeth.mjswhile test(compat): guard the fail-on-red gate from OUTSIDE the script (#703) #705 was in flight.tests/compat-suite-workflow.test.ts— test(compat): guard the fail-on-red gate from OUTSIDE the script (#703) #705 owns it this round.