* feat(#3414): promote git-cmd.js token-walk into a shared scanner, fix #3169 Phase 3 of epic #3212 (ADR-3212 §4). New src/token-scanner.cts generalizes hooks/lib/git-cmd.js's proven token-walk (#3129 — "has not re-opened"): tokenizeShellLike (quote-aware shell tokenizer, byte-identical port) and indentWidth (bullet-nesting depth). git-cmd.js migrates onto tokenizeShellLike with zero behavior change (parity-asserted against every existing #3129 fixture in tests/worktree-safety.test.cjs's folded block); isGitSubcommand's phases 1-3 (env-prefix skip, executable check, global-option consume) extracted into skipToSubcommand, shared with the new extractBranchArgument (git checkout -b / git branch <name>) — a new capability exercising the seam on the domain the ADR names, not a migration of existing duplicated logic (none existed). Fixes #3169: src/decisions.cts's parseDecisionLines couldn't distinguish a cross-reference bullet nested under an open decision from a fresh malformed declaration attempt. An earlier bold-run-content-classification design was tried and disproven against the repo's own existing FIX-B fixtures (D-02, "no colon no dash") before being adopted — both have identical shape under any content-only rule. Nesting depth (via indentWidth) is the actual distinguishing signal: a bullet indented deeper than the currently-open decision's own bullet is elaboration, folded into its text like a continuation line, never tested against the parse-miss guard. A bullet at the same-or-shallower indent is unchanged. Scope-narrowing disclosed, not silent: of the ADR's four named bugs (#3197, #3169, #2570, #2528), three no longer need this phase's work. were independently fixed and closed since the ADR was authored — #2570's fix is already a correctly-bounded regex per the ADR's own decidability test (no scanner needed); #2528's fix is a deliberate, twice-reviewed non-scanner design (its own code comment records a scanner-based attempt that regressed a symmetric case and was reverted) that this phase does not disturb. Only #3169 required new work. get_impact: isGitSubcommand CRITICAL/196 affected symbols, parseDecisionLines CRITICAL/164 affected symbols (ADR §6 due diligence). Six-gate ripple: .gitignore, eslint.config.mjs, docs/INVENTORY.md, docs/INVENTORY-MANIFEST.json (regenerated), CONTEXT.md glossary. Design: .gsd/phase/chore-3414-tokenizer-first-seam/40-design.md Test matrix: .gsd/phase/chore-3414-tokenizer-first-seam/50-test-matrix.md Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(#3414): add required fast-check property tests per code review TESTING-STANDARDS.md:169 requires at least one fast-check property test for any module that implements parsing — src/token-scanner.cts had none, an orthogonal Standards-axis review finding. Adds two seeded property tests (mirroring Phase 1/2's fast-check-setup.cjs convention): indentWidth counts exactly a generated leading-space run; tokenizeShellLike round-trips a generated array of whitespace/quote-free words joined with single spaces. The design doc's own "no property test needed" rationale was wrong — it argued no algebraic law applied, but the standard is unconditional for parsing modules regardless of whether one "feels" applicable. Corrected in .gsd/phase/chore-3414-tokenizer-first-seam/50-test-matrix.md. Also fixes two Spec-axis wording drifts the same review found between the design doc and the shipped code (doc-only, no behavior change): extractBranchArgument's documented signature dropped an unused subVariants parameter that was never implemented, and the #3169 fail-first fixture description corrected from "15-decision plan via cmdDecisionCoverageVerify" to the actual compact 3-decision analog via the real blocking gate, check.decision-coverage-plan. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(#3414): add changeset for #3169 fix Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(#3414): backfill changeset pr number to 3424 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: sim <sim@local> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
@@ -782,6 +782,126 @@ describe('FIX B gate-level: parse-miss → passed:false regardless of covered de
|
||||
});
|
||||
});
|
||||
|
||||
// ─── #3169: nested cross-reference bullet must not zero out coverage (#3212 Phase 3) ─
|
||||
//
|
||||
// parseDecisionLines' parse-miss guard fires on any line whose bold run starts
|
||||
// with `D-`, including a cross-reference bullet NESTED (deeper-indented) under
|
||||
// an already-open decision — e.g. a decision's own body elaborating on how it
|
||||
// relates to a sibling decision. A single such miss forces the whole extraction
|
||||
// to `could-not-parse`, discarding every decision that DID parse correctly.
|
||||
//
|
||||
// Fix (design doc §1.3): track the indent width of the currently-open decision's
|
||||
// bullet. A subsequent bulleted line indented DEEPER than that is nested content
|
||||
// under the open decision (append to its text, like a continuation line) rather
|
||||
// than a fresh declaration attempt — never tested against the parse-miss guard.
|
||||
// A bullet at the same-or-shallower indent is unchanged (still tested normally),
|
||||
// which is what keeps the existing FIX-B fixtures (`D-02`, "no colon no dash")
|
||||
// still failing as genuine misses — see rows 21-22 below.
|
||||
|
||||
describe('#3169: nested cross-reference bullet does not increment parseMisses', () => {
|
||||
test('FAIL-FIRST row 18: a bullet nested under an open decision is elaboration, not a fresh declaration attempt', () => {
|
||||
const md = [
|
||||
'<decisions>',
|
||||
"- **D-15:** some decision",
|
||||
" - **D-06's fix does not close this.** Gating the Passed arm on the derived status passes cleanly here.",
|
||||
'</decisions>',
|
||||
].join('\n');
|
||||
const result = extractDecisions(md);
|
||||
assert.strictEqual(result.outcome, 'parsed',
|
||||
`Nested cross-reference must not force could-not-parse. Got: ${JSON.stringify(result)}`);
|
||||
assert.deepStrictEqual(result.decisions.map((d) => d.id), ['D-15'],
|
||||
`Only D-15 should be extracted as a decision — the nested bullet is elaboration, not a second entry. Got: ${JSON.stringify(result.decisions.map((d) => d.id))}`);
|
||||
assert.ok(
|
||||
result.decisions[0].text.includes("D-06's fix does not close this"),
|
||||
`The nested bullet's text should be folded into D-15's own text (continuation-style). Got: ${JSON.stringify(result.decisions[0].text)}`,
|
||||
);
|
||||
});
|
||||
|
||||
test('FAIL-FIRST row 19: a second nested bullet under the same open decision is also elaboration', () => {
|
||||
const md = [
|
||||
'<decisions>',
|
||||
"- **D-15:** some decision",
|
||||
" - **D-06's fix does not close this.** first note.",
|
||||
' - **D-13 (999.76) must not land without this fix.** second note.',
|
||||
'</decisions>',
|
||||
].join('\n');
|
||||
const result = extractDecisions(md);
|
||||
assert.strictEqual(result.outcome, 'parsed',
|
||||
`Two nested cross-references must not force could-not-parse. Got: ${JSON.stringify(result)}`);
|
||||
assert.deepStrictEqual(result.decisions.map((d) => d.id), ['D-15']);
|
||||
});
|
||||
|
||||
test('FAIL-FIRST row 20: end-to-end — /gsd-plan-phase §13a gate reports full coverage with nested cross-reference bullets present', () => {
|
||||
const tmpDir = createTempProject('gsd-3169-');
|
||||
const planningDir = path.join(tmpDir, '.planning');
|
||||
const phaseDir = path.join(planningDir, 'phases', '01-init');
|
||||
fs.mkdirSync(phaseDir, { recursive: true });
|
||||
try {
|
||||
// Compact 3-decision analog of the issue's 15-decision repro: D-03 carries
|
||||
// the same nested-cross-reference shape that zeroed coverage in the report.
|
||||
writeContextFile(phaseDir, [
|
||||
'<decisions>',
|
||||
'- **D-01:** use JWT tokens',
|
||||
'- **D-02:** use Redis sessions',
|
||||
'- **D-03:** derive status from the Passed arm',
|
||||
" - **D-01's token choice does not close this.** Criterion 3 and criterion 4 are separate deliverables.",
|
||||
' - **D-02 (session store) must not land without this fix.** Moving discovery to the execution root makes this common.',
|
||||
'</decisions>',
|
||||
].join('\n'));
|
||||
writePlanFile(phaseDir, '01', [
|
||||
'# Plan',
|
||||
'',
|
||||
'## Must Haves',
|
||||
'',
|
||||
'- D-01: implement JWT token issuance and validation',
|
||||
'- D-02: wire Redis session storage',
|
||||
'- D-03: derive status from the Passed arm',
|
||||
].join('\n'));
|
||||
|
||||
const contextPath = path.join(phaseDir, 'CONTEXT.md');
|
||||
const result = runDecisionCoveragePlan(phaseDir, contextPath, tmpDir);
|
||||
const parsed = JSON.parse(result.output || '');
|
||||
assert.strictEqual(parsed.passed, true,
|
||||
`Gate must pass — all 3 decisions are covered and the nested bullets are not parse-misses. Got: ${JSON.stringify(parsed)}`);
|
||||
assert.strictEqual(parsed.total, 3, `Got: ${JSON.stringify(parsed)}`);
|
||||
assert.strictEqual(parsed.covered, 3,
|
||||
`Must report 3/3 covered, not the false 0/3 #3169 reports today. Got: ${JSON.stringify(parsed)}`);
|
||||
assert.deepStrictEqual(parsed.uncovered, [], `Got: ${JSON.stringify(parsed)}`);
|
||||
} finally {
|
||||
cleanup(tmpDir);
|
||||
}
|
||||
});
|
||||
|
||||
test('row 21 (negative control, disproven-design record): a top-level malformed bullet with no open decision above it still forces could-not-parse', () => {
|
||||
// This is tests/decisions.test.cjs's OWN existing FIX-B fixture (line ~651/709),
|
||||
// re-asserted here to record it as the case that disproved an earlier
|
||||
// bold-run-content-classification design for #3169 (design doc §1.3) — a
|
||||
// content-only rule cannot distinguish this from the #3169 cross-reference
|
||||
// shape above; indentation (0, nothing open to nest under) is what does.
|
||||
const md = '<decisions>\n- **D-01:** Use OAuth 2.0\n- **D-02** ratio 3:1\n</decisions>';
|
||||
const result = extractDecisions(md);
|
||||
assert.strictEqual(result.outcome, 'could-not-parse',
|
||||
`A standalone top-level malformed bullet must still be a genuine miss. Got: ${JSON.stringify(result)}`);
|
||||
});
|
||||
|
||||
test('row 22: another standalone top-level malformed bullet still forces could-not-parse', () => {
|
||||
const md = '<decisions>\n- **D-01 no colon no dash here** just text\n</decisions>';
|
||||
const result = extractDecisions(md);
|
||||
assert.strictEqual(result.outcome, 'could-not-parse',
|
||||
`Got: ${JSON.stringify(result)}`);
|
||||
});
|
||||
|
||||
test('row 23: a bullet indented as if nested, but with no decision open above it, falls through to normal handling', () => {
|
||||
// No `current` is open when this line is reached, so the nesting check
|
||||
// cannot apply (there is nothing to nest under) — it must be tested as an
|
||||
// ordinary top-level bullet, exactly as before this fix.
|
||||
const md = '<decisions>\n - **D-01** malformed, nothing open above it\n</decisions>';
|
||||
const result = extractDecisions(md);
|
||||
assert.strictEqual(result.outcome, 'could-not-parse',
|
||||
`A leading indented bullet with nothing open above it must still be tested normally. Got: ${JSON.stringify(result)}`);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── FIX C regressions: curly-quote Claude's Discretion ───────────────────────
|
||||
|
||||
describe('FIX C: curly-quote Claude’s Discretion → trackable:false (#1372)', () => {
|
||||
|
||||
154
tests/token-scanner.test.cjs
Normal file
154
tests/token-scanner.test.cjs
Normal file
@@ -0,0 +1,154 @@
|
||||
'use strict';
|
||||
|
||||
/**
|
||||
* Tests for `src/token-scanner.cts` — the tokenizer-first seam (#3212 Phase 3, #3414).
|
||||
*
|
||||
* Design: .gsd/phase/chore-3414-tokenizer-first-seam/40-design.md
|
||||
* Test matrix: .gsd/phase/chore-3414-tokenizer-first-seam/50-test-matrix.md
|
||||
* ADR: docs/adr/3212-lexical-seam-consolidation.md §4, §6, §7
|
||||
*
|
||||
* TDD RED: `src/token-scanner.cts` does not exist yet — this file's
|
||||
* `require('../gsd-core/bin/lib/token-scanner.cjs')` throws MODULE_NOT_FOUND until
|
||||
* the implementing phase adds it (mirrors tests/pattern.test.cjs / tests/text-lines.test.cjs's
|
||||
* RED convention from Phases 1-2).
|
||||
*
|
||||
* Covers test-matrix rows 1-10: tokenizeShellLike (1-5), indentWidth (6-10).
|
||||
* extractBranchArgument (rows 11-17) lives in git-cmd.js, tested in
|
||||
* tests/worktree-safety.test.cjs's folded bug-3129 block, not here.
|
||||
*/
|
||||
|
||||
const { test, describe } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const path = require('node:path');
|
||||
// Seeded fast-check convention: require the shared setup helper (NOT
|
||||
// 'fast-check' directly) so numRuns/seed are configured globally before any
|
||||
// fc.assert() call — mirrors tests/pattern.test.cjs / tests/text-lines.test.cjs.
|
||||
// seed: 42, overridable via GSD_FC_SEED. Required by TESTING-STANDARDS.md:169
|
||||
// ("modules that implement parsing... must include at least one fast-check
|
||||
// property test asserting a domain invariant") — a review finding this phase
|
||||
// missed on first pass; both invariants below are added in response to it.
|
||||
const fc = require('./helpers/fast-check-setup.cjs');
|
||||
|
||||
const { tokenizeShellLike, indentWidth } = require('../gsd-core/bin/lib/token-scanner.cjs');
|
||||
const { tokenize: gitCmdTokenize } = require(path.join(__dirname, '..', 'hooks', 'lib', 'git-cmd.js'));
|
||||
|
||||
// ─── tokenizeShellLike — rows 1-5 ──────────────────────────────────────────
|
||||
|
||||
describe('tokenizeShellLike', () => {
|
||||
test('row 1: bare git commit, double-quoted message', () => {
|
||||
assert.deepStrictEqual(
|
||||
tokenizeShellLike('git commit -m "msg"'),
|
||||
['git', 'commit', '-m', 'msg'],
|
||||
);
|
||||
});
|
||||
|
||||
test('row 2: single-quoted message', () => {
|
||||
assert.deepStrictEqual(
|
||||
tokenizeShellLike("git commit -m 'my message'"),
|
||||
['git', 'commit', '-m', 'my message'],
|
||||
);
|
||||
});
|
||||
|
||||
test('row 3: env-prefix assignment token preserved', () => {
|
||||
assert.deepStrictEqual(
|
||||
tokenizeShellLike('GIT_AUTHOR_NAME=x git commit'),
|
||||
['GIT_AUTHOR_NAME=x', 'git', 'commit'],
|
||||
);
|
||||
});
|
||||
|
||||
test('row 4: empty string yields no tokens', () => {
|
||||
assert.deepStrictEqual(tokenizeShellLike(''), []);
|
||||
});
|
||||
|
||||
test('row 5: parity with git-cmd.js\'s current tokenize() on every existing #3129 fixture', () => {
|
||||
const fixtures = [
|
||||
'git commit -m "feat: add thing"',
|
||||
"git commit -m 'fix: typo'",
|
||||
'git commit --no-verify -m "wip"',
|
||||
'git -C /some/path commit -m "fix: x"',
|
||||
'GIT_AUTHOR_NAME=Alice git commit -m "fix"',
|
||||
'/usr/bin/git commit -m "feat: y"',
|
||||
'GIT_AUTHOR_NAME=A GIT_AUTHOR_EMAIL=b@c git commit -m "x"',
|
||||
'git --git-dir=.git commit -m "x"',
|
||||
'git --git-dir .git commit -m "x"',
|
||||
'git --no-pager commit -m "x"',
|
||||
'/usr/bin/git -C /proj commit -m "x"',
|
||||
'git -p commit -m "x"',
|
||||
'git push origin main',
|
||||
'git status',
|
||||
'git add .',
|
||||
'git log --oneline',
|
||||
'npm install',
|
||||
'',
|
||||
'git checkout main',
|
||||
'git -C /path push',
|
||||
];
|
||||
for (const cmd of fixtures) {
|
||||
assert.deepStrictEqual(
|
||||
tokenizeShellLike(cmd),
|
||||
gitCmdTokenize(cmd),
|
||||
`tokenizeShellLike must match git-cmd.js's tokenize() for: ${JSON.stringify(cmd)}`,
|
||||
);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// ─── indentWidth — rows 6-10 ────────────────────────────────────────────────
|
||||
|
||||
describe('indentWidth', () => {
|
||||
test('row 6: no leading whitespace', () => {
|
||||
assert.strictEqual(indentWidth('- **D-01:** text'), 0);
|
||||
});
|
||||
|
||||
test('row 7: counts leading spaces', () => {
|
||||
assert.strictEqual(indentWidth(" - **D-06's fix does not close this.** text"), 2);
|
||||
});
|
||||
|
||||
test('row 8: a tab counts as one column, not expanded', () => {
|
||||
assert.strictEqual(indentWidth('\t- text'), 1);
|
||||
});
|
||||
|
||||
test('row 9: empty string and no-leading-whitespace both return 0', () => {
|
||||
assert.strictEqual(indentWidth(''), 0);
|
||||
assert.strictEqual(indentWidth('no leading space'), 0);
|
||||
});
|
||||
|
||||
test('row 10: all-whitespace line returns its full length', () => {
|
||||
assert.strictEqual(indentWidth(' '), 3);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── Property tests (TESTING-STANDARDS.md:169) ─────────────────────────────
|
||||
|
||||
describe('property: indentWidth counts exactly the generated leading-space run', () => {
|
||||
test('indentWidth(spaces + non-space content) === spaces.length', () => {
|
||||
fc.assert(
|
||||
fc.property(
|
||||
fc.nat({ max: 30 }),
|
||||
fc.string({ minLength: 1, maxLength: 20 }).filter((s) => !/^[ \t]/.test(s)),
|
||||
(n, content) => {
|
||||
const line = ' '.repeat(n) + content;
|
||||
assert.strictEqual(indentWidth(line), n);
|
||||
},
|
||||
),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('property: tokenizeShellLike is the inverse of joining whitespace-free words with single spaces', () => {
|
||||
test('tokenizeShellLike(words.join(" ")) deepStrictEqual words', () => {
|
||||
// A "word" here is any non-empty run with no whitespace and no quote
|
||||
// characters — the shape that round-trips through the tokenizer without
|
||||
// needing quoting to survive the join (quoting is exercised separately
|
||||
// by rows 1-2/5; this property covers the bijective unquoted case
|
||||
// TESTING-STANDARDS.md:169 asks for).
|
||||
const wordArb = fc
|
||||
.string({ minLength: 1, maxLength: 12 })
|
||||
.filter((s) => s.length > 0 && !/[\s'"]/.test(s));
|
||||
fc.assert(
|
||||
fc.property(fc.array(wordArb, { minLength: 0, maxLength: 10 }), (words) => {
|
||||
assert.deepStrictEqual(tokenizeShellLike(words.join(' ')), words);
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -4357,7 +4357,7 @@ const path = require('node:path');
|
||||
const fs = require('node:fs');
|
||||
|
||||
const ROOT = path.join(__dirname, '..');
|
||||
const { isGitSubcommand, tokenize } = require(path.join(ROOT, 'hooks', 'lib', 'git-cmd.js'));
|
||||
const { isGitSubcommand, tokenize, extractBranchArgument } = require(path.join(ROOT, 'hooks', 'lib', 'git-cmd.js'));
|
||||
|
||||
// ── tokenize ─────────────────────────────────────────────────────────────────
|
||||
|
||||
@@ -4454,6 +4454,36 @@ describe('gsd-validate-commit.sh delegates to git-cmd.js', () => {
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
// ── extractBranchArgument (#3212 Phase 3, #3414) ─────────────────────────────
|
||||
// New capability on the shared scanner (design doc §1.2) — not a migration of
|
||||
// existing duplicated logic; no existing consumer wired to it this phase.
|
||||
|
||||
describe('git-cmd.js extractBranchArgument', () => {
|
||||
test('row 11: git checkout -b', () => {
|
||||
assert.strictEqual(extractBranchArgument('git checkout -b feat/123-slug'), 'feat/123-slug');
|
||||
});
|
||||
|
||||
test('row 12: quoted branch name', () => {
|
||||
assert.strictEqual(extractBranchArgument('git checkout -b "feat/with spaces"'), 'feat/with spaces');
|
||||
});
|
||||
|
||||
test('row 13: git branch <name> form', () => {
|
||||
assert.strictEqual(extractBranchArgument('git branch feat/123'), 'feat/123');
|
||||
});
|
||||
|
||||
test('row 14: -C path prefix does not confuse the branch-name extraction', () => {
|
||||
assert.strictEqual(extractBranchArgument('git -C /repo checkout -b feat/123'), 'feat/123');
|
||||
});
|
||||
|
||||
test('row 15: unrelated command with checkout-shaped text inside a quoted message returns null', () => {
|
||||
assert.strictEqual(extractBranchArgument('git commit -m "checkout -b fake"'), null);
|
||||
});
|
||||
|
||||
test('row 16: plain checkout (no -b) is not a branch-creation command', () => {
|
||||
assert.strictEqual(extractBranchArgument('git checkout main'), null);
|
||||
});
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user