stable
1 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Tom Boucher
|
40628d8050 |
fix(#4409): delete the fold's shadow copies of extractShellBlocks and collectWorkflowFiles (#4720)
* test(#4409): prove the fold shadows two module-scope helpers Failing-first regression coverage for #4409. tests/runtime-launcher-parity.test.cjs declares extractShellBlocks twice. The module-scope copy (line 220) splits on /\r?\n/ and pushes {index, lang, lines}; the folded copy (line 1315) splits on '\n' and pushes {lines} only. The fold opens with a bare `{` at line 1226, so the inner declaration shadows the outer one for everything inside it -- and on a CRLF checkout every extracted line keeps a trailing \r. collectWorkflowFiles is duplicated the same way and is currently byte-identical, i.e. latent rather than active. Every row walks an AST via espree, a declared devDependency. None reads a .cjs and calls .includes(): that is local/no-source-grep's exact shape, and it is also the wrong instrument -- "how many declarations exist" is a construct count, and a regex would match the name inside a comment or a string literal too. Row 2 pins the rest of the corpus as an EXACT SORTED LIST of the 26 fold-shadowed helpers that remain in 15 other files, measured with this same walker rather than guessed. A bare count was rejected: `27 !== 26` names no offender and costs a round-trip to diagnose. Row 3 keeps that baseline from rotting into strings that match nothing. Row 4 is the behavioural half -- the defect a Windows user actually hits -- and asserts the surviving extractShellBlocks splits on a regex that tolerates \r, not on the bare '\n' literal. Red round: rows 1, 2 and 4 fail; row 3 passes, since every baseline entry is real today. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(#4409): delete the fold's shadow copies of two helpers Both folded copies are removed so the fold resolves the module-scope pair. Deletion, not repair: patching the folded split to /\r?\n/ would leave two definitions and the next edit could diverge them again, and the issue asks for one definition per helper. Verified safe before deleting, not assumed: - the fold's WORKFLOWS_DIR and SNIPPET_FILE are byte-identical to the module-scope ones, and escapeRegex resolves to the same gsd-core/bin/lib/pattern.cjs, so the surviving functions close over the same values the folded copies did; - the fold has exactly ONE call site (test (E)) and it consumes blocks only as blocks.flatMap(b => b.lines). The module-scope extractor returns {index, lang, lines}, a SUPERSET of the folded {lines}, so no caller is starved. The deepStrictEqual 11 lines below that call compares `missing`, an array of path strings -- not block objects. Also removes the fold's now-dead `escapeRegex` re-require. It was used only by the folded extractShellBlocks; lint:ci caught it as an unused binding. The module-scope require stays -- the surviving extractor needs it. Applied by AST range rather than line numbers so the deletion boundaries are exact. Not fixed here, and not a deferral: the same walker finds 28 fold-shadowed helpers across 16 test files, of which 16 DIVERGE from their module-scope twin. The 15 diverged pairs outside this file are the same defect class, but the tracker already drew this boundary -- #4337 removed three runBashFile shadows "only because that PR had to edit all four identically" and deliberately left these two to #4409. A diverged shadow also cannot be deleted mechanically the way these could: its fold's tests were written against its own copy. The new suite pins all 26 remaining as an exact list so none can be forgotten. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(#4409): close five holes both reviewers found in the new suite Review round. Every change is to the new suite; the fix itself is untouched. 1. The corpus walk was NON-RECURSIVE, so 64 files under tests/{dispatch, qa,observability,health-diagnostic-rules,helpers} were invisible to the baseline guard. Now recursive: 1013 files parsed, up from 949. The baseline is unchanged at 26 -- coverage widened without moving the pin. 2. `catch { continue }` silently exempted any file espree could not parse, so the guard could go quietly blind on exactly the file that broke. Parse failures now propagate, and row 2 asserts the scanned count. 3. Only FunctionDeclaration was detected, so a `const helper = () => {}` shadow was invisible. Arrow and function-expression bindings now count too. Zero such shadows exist today; the hole was future-facing. 4. The old row 3 was VACUOUS -- subsumed by row 2, which already fails on a stale baseline entry because the lists stop matching. Deleted; its message folded into row 2. 5. The CRLF row inspected only declarations[0]. On the pre-fix tree that is the MODULE-SCOPE copy, which was already correct -- so only the length===1 assertion went red and the row never actually saw the bug. It now checks EVERY declaration, and pins the split to the one applied to that declaration's own content parameter rather than to whichever `.split()` appears first in the body. Also removes the `allow-test-rule: source-text-is-the-product` marker. It was inert: the readFileSync result feeds espree.parse, never a text method, so local/no-source-grep has no candidate site to suppress. Proven by running the rule through the Linter API with suppression neutralized -- 0 messages either way. A marker naming an exemption that does not exist is misleading. The divergence figure in the header comment now states its normalization. Both reviewers were right and measured different things: over the 26 remaining shadows, whitespace-only normalization gives 16 diverged / 10 identical; stripping comments as well gives 15 / 11. Exactly one pair differs only in its comments. Control re-run against the pre-fix subject file with the new logic: 3 of 3 rows fail, 3 of 3 pass after. Every row is now load-bearing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: sim <sim@local> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |