* chore(#4424): assert launcher snippet env surface is covered by scrub lists SNIPPET_SCRUB is hand-maintained for vars TEST_ENV_BASE's registry-derived list can't carry. Nothing asserted the union actually covers every ${VAR:-default} arm in _runtime-launcher.snippet.sh, so a new runtime-home arm with no scrub entry could drift silently — the #4205 shape, one door over. Adds (A2): extracts every ${[A-Z_]+:-} capture from the snippet and checks membership in TEST_ENV_BASE, SNIPPET_SCRUB, or the two vars the snippet/fixtures set themselves (RUNTIME_DIR, GSD_TOOLS). * fix(#4424): allow digits in the (A2) fallback-var regex CodeRabbit review on fork PR #37: [A-Z_]+ silently drops any \${VAR:-...} capture whose name contains a digit (e.g. CLAUDE2_CONFIG_DIR) instead of flagging it uncovered, defeating the guard's own purpose. Matches bash identifier syntax instead: leading letter/underscore, then alnum/underscore. * fix(#4424): rename SELF_ASSIGNED to reflect RUNTIME_DIR's real provenance Gemini adversarial review (agy) on fork PR #37: RUNTIME_DIR is an external input the snippet reads via \${RUNTIME_DIR:-...}, never assigns — every fixture sets it in-script before sourcing the snippet. Only GSD_TOOLS is truly snippet-self-assigned. SELF_ASSIGNED conflated the two; renamed to CALLER_OR_SELF_ASSIGNED. No behavior change. Reviewed and rejected: moving RUNTIME_DIR into SNIPPET_SCRUB (blanking it is indistinguishable from unset to the resolver's own \${RUNTIME_DIR:-...} fallback, re-opening the #4205 ambient-leak this suite guards against — see the existing comment at line ~1801); widening the regex to mixed-case, colon-less \${VAR-default}, or \${VAR:=default} forms (none exist in the snippet, and the issue's own spec scopes this to \${[A-Z_]+:- captures); stripping bash comments before matching (the snippet is one physical line with zero '#' characters, so no comment can exist in it). * fix(#4424): guard CALLER_OR_SELF_ASSIGNED against silent future additions trek-e review on PR #4503: a future ${VAR:-default} arm could be dropped into this set without confirming it is genuinely caller-supplied/ self-assigned rather than a real coverage gap. Adds a comment requiring justification for any addition, pointing to SNIPPET_SCRUB as the default when in doubt. No behavior change. * fix(#4424): guard (A2) against a vacuous pass on empty extraction agy adversarial review (gemini-3.8-flash-high, /gsd-review lane) on PR #4503: if the snippet becomes unreadable/truncated/renamed, matchAll yields zero matches, uncovered stays [], and assert.deepStrictEqual passes vacuously — same "guards the guard" gap the sibling (E)-adjacent tests already close with assert.ok(files.length > 0, ...). Asserts extracted.length >= 15 before filtering. Reviewer's second finding (regex misses colon-less ${VAR-default}) is not applied: no such form exists in the snippet today, and 688cc1c already recorded this exact widening as scope creep the issue's own spec (${[A-Z_]+:-) does not ask for. --------- Co-authored-by: Test <test@test.com> Co-authored-by: Tom Boucher <trekkie@nomorestars.com>
This commit is contained in:
committed by
GitHub
parent
eadcba5f53
commit
1316e03b84
@@ -380,6 +380,42 @@ describe('runtime-launcher-parity (#373)', () => {
|
||||
);
|
||||
});
|
||||
|
||||
// ─── (A2) Snippet's ${VAR:-default} surface is fully covered by the scrub lists (#4424) ──
|
||||
// SNIPPET_SCRUB above is hand-maintained for vars TEST_ENV_BASE cannot derive.
|
||||
// Nothing previously asserted the union actually covers every fallback arm in
|
||||
// the snippet, so a new runtime-home arm with no scrub entry could drift
|
||||
// silently — the same shape as #4205, arriving through the hand-listed half.
|
||||
test('(A2) every ${VAR:-default} arm in the snippet is covered by TEST_ENV_BASE, SNIPPET_SCRUB, or a caller-supplied var', () => {
|
||||
// RUNTIME_DIR: an external input the snippet reads, never assigns — every
|
||||
// fixture that sources the snippet sets it in-script before doing so.
|
||||
// GSD_TOOLS: the snippet assigns this one itself, before this arm's
|
||||
// ${GSD_TOOLS:-} check runs.
|
||||
// Any addition here must justify, in a comment like the two above, why the
|
||||
// var is genuinely caller-supplied/self-assigned — not a real coverage gap
|
||||
// silenced by exemption. When in doubt, add a SNIPPET_SCRUB entry instead.
|
||||
const CALLER_OR_SELF_ASSIGNED = new Set(['RUNTIME_DIR', 'GSD_TOOLS']);
|
||||
const snippetContent = fs.readFileSync(SNIPPET_FILE, 'utf8');
|
||||
const covered = new Set([...Object.keys(TEST_ENV_BASE), ...Object.keys(SNIPPET_SCRUB), ...CALLER_OR_SELF_ASSIGNED]);
|
||||
const extracted = [...new Set(
|
||||
[...snippetContent.matchAll(/\$\{([A-Z_][A-Z0-9_]*):-/g)].map((m) => m[1]),
|
||||
)];
|
||||
// Guards the guard: a truncated/renamed/unreadable snippet would make
|
||||
// `extracted` empty, and an empty `uncovered` below would pass vacuously.
|
||||
assert.ok(
|
||||
extracted.length >= 15,
|
||||
`expected the snippet to yield many distinct \${VAR:-default} arms, got ${extracted.length}`,
|
||||
);
|
||||
const uncovered = extracted.filter((name) => !covered.has(name));
|
||||
|
||||
assert.deepStrictEqual(
|
||||
uncovered,
|
||||
[],
|
||||
'Snippet fallback arm(s) not covered by TEST_ENV_BASE, SNIPPET_SCRUB, or a caller-supplied var — ' +
|
||||
'add a SNIPPET_SCRUB entry (or confirm the capability registry should carry it):\n' +
|
||||
uncovered.join('\n'),
|
||||
);
|
||||
});
|
||||
|
||||
// ─── (B) Exactly ONE canonical preamble per using file ───────────────────
|
||||
test('(B) each workflow .md using gsd_run contains exactly ONE canonical preamble, before the first gsd_run call', () => {
|
||||
const preamble = expectedPreamble();
|
||||
|
||||
Reference in New Issue
Block a user