From 470389f3a2134f5bf838f5f300c80226a5779957 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Thu, 13 Aug 2026 23:08:34 -0400 Subject: [PATCH] =?UTF-8?q?chore(#3212):=20tokenizer-first=20for=20statefu?= =?UTF-8?q?l=20grammars=20=E2=80=94=20a=20shared=20scanner=20=E2=80=94=20P?= =?UTF-8?q?hase=203=20(#3424)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 ) — 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 * 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 * docs(#3414): add changeset for #3169 fix Co-Authored-By: Claude Sonnet 5 * docs(#3414): backfill changeset pr number to 3424 Co-Authored-By: Claude Sonnet 5 --------- Co-authored-by: sim Co-authored-by: Claude Sonnet 5 --- .changeset/sharp-birds-wave.md | 5 ++ .gitignore | 1 + CONTEXT.md | 3 + docs/INVENTORY-MANIFEST.json | 1 + docs/INVENTORY.md | 4 +- eslint.config.mjs | 1 + hooks/lib/git-cmd.js | 156 +++++++++++++++++++-------------- src/decisions.cts | 26 ++++++ src/token-scanner.cts | 68 ++++++++++++++ tests/decisions.test.cjs | 120 +++++++++++++++++++++++++ tests/token-scanner.test.cjs | 154 ++++++++++++++++++++++++++++++++ tests/worktree-safety.test.cjs | 32 ++++++- 12 files changed, 502 insertions(+), 69 deletions(-) create mode 100644 .changeset/sharp-birds-wave.md create mode 100644 src/token-scanner.cts create mode 100644 tests/token-scanner.test.cjs diff --git a/.changeset/sharp-birds-wave.md b/.changeset/sharp-birds-wave.md new file mode 100644 index 000000000..33fb991c8 --- /dev/null +++ b/.changeset/sharp-birds-wave.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3424 +--- +**`/gsd-plan-phase`'s §13a Decision Coverage Gate no longer reports false total-coverage failures when a decision's own body contains a bulleted cross-reference to a sibling decision** — a bullet nested (indented) under an already-open decision, elaborating on how it relates to another decision, was previously indistinguishable from a malformed top-level declaration attempt. A single such bullet forced the whole coverage analysis to `could-not-parse`, discarding every decision that DID parse correctly and reporting `covered: 0` even when every decision was, in fact, fully covered by the phase's plans. (#3169) diff --git a/.gitignore b/.gitignore index 59f94d3ca..ec1ee11a0 100644 --- a/.gitignore +++ b/.gitignore @@ -198,6 +198,7 @@ build/ /gsd-core/bin/lib/planning-snapshot.cjs /gsd-core/bin/lib/pattern.cjs /gsd-core/bin/lib/text-lines.cjs +/gsd-core/bin/lib/token-scanner.cjs /gsd-core/bin/lib/health-diagnostic-types.cjs /gsd-core/bin/lib/health-diagnostic.cjs /gsd-core/bin/lib/health-diagnostic-rules/root-existence.cjs diff --git a/CONTEXT.md b/CONTEXT.md index bd89a58b2..eebc5ea6c 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -109,6 +109,9 @@ Leaf module owning the construction of a regex from a **runtime value**, per ADR ### Text Lines Module Leaf module owning `\r?\n` line-terminator splitting and CRLF normalization, per ADR-3212 §3 (epic #3212 Phase 2, #3413). Exposes `splitLines(content) → string[]` (splits on `\r\n` or `\n`; a lone `\r` is not a delimiter), `normalizeEol(content) → string` (strips every `\r`, matching the four `scripts/gen-*.cjs` `--check` copies it replaces), `detectEol(content) → '\n' | '\r\n'` (dominant terminator, `'\r\n'`-default on a tie or no terminator), and `joinLines(lines, eol?) → string` (inverse of `splitLines`; round-trips byte-for-byte with `detectEol`). Closes #3360: `parseMustHavesBlock` (`src/frontmatter.cts`) matched `^(\s*)must_haves:\s*$` and a sibling block-header regex against the WHOLE multi-line YAML string under `/m`, and `\r` is its own ECMA-262 LineTerminator — a greedy `\s*` anchored on `^` could cross a CRLF boundary and inflate the captured indent by one character, tripping the nesting guard and silently returning `[]` for every `must_haves` block on a CRLF plan file. The fix reroutes both lookups through split-then-match (`splitLines` first, then match per already-split line), which cannot straddle the delimiter that produced it. Pure and import-free — a leaf, so any consumer can depend on it without a cycle (mirrors the Pattern Module's position). Enforced going forward by the widened `eslint-rules/no-crlf-fragile-split.cjs` (now also scanned against `src/**/*.cts`, not only `tests/`). Source of truth: `gsd-core/bin/lib/text-lines.cjs` (generated from `src/text-lines.cts`). Design: `.gsd/phase/chore-3413-text-lines-seam/40-design.md`. +### Token Scanner Module +Leaf module generalizing the proven `hooks/lib/git-cmd.js` token-walk (#3129) into a shared primitive for stateful grammars, per ADR-3212 §4 (epic #3212 Phase 3, #3414). Exposes `tokenizeShellLike(cmd) → string[]` (quote-aware shell tokenizer — single/double-quoted spans as one token, no escape or brace/variable expansion, byte-identical port of `git-cmd.js`'s original `tokenize()`) and `indentWidth(line) → number` (leading-whitespace column count, tabs counted as one column each, no tab-width policy introduced). `hooks/lib/git-cmd.js` migrates its `tokenize()`/`isGitSubcommand()` onto `tokenizeShellLike` with zero behavior change (ADR §6 extend-never-mutate; parity-asserted against every existing fixture) and gains `extractBranchArgument(cmd) → string | null` (`git checkout -b ` / `git branch `) — a new capability exercising the seam on the domain the ADR names, not a migration of existing duplicated logic (none existed). Closes #3169: `src/decisions.cts`'s decision-bullet parser could not distinguish a cross-reference bullet NESTED under an already-open decision from a fresh, malformed top-level declaration attempt — both have identical shape under any bullet-*content* classifier (an earlier design using bold-run-content classification was disproven against the repo's own existing FIX-B fixtures before being adopted, per the phase's design doc). `indentWidth` supplies the structural signal a per-line regex cannot see: a bulleted line indented deeper than the currently-open decision's own bullet is that decision's elaboration, folded into its text like a continuation line, never tested against the declaration/parse-miss regexes. A bullet at the same-or-shallower indent is unaffected. Pure and import-free — a leaf, so any consumer can depend on it without a cycle (mirrors the Pattern and Text Lines modules' position). Hook-staging note: `hooks/` scripts are staged as standalone files at install time, so `git-cmd.js` requires the BUILT `gsd-core/bin/lib/token-scanner.cjs` artifact (matching Phase 2's `scripts/gen-*.cjs` consolidation), not a sibling `hooks/lib/` file. Source of truth: `gsd-core/bin/lib/token-scanner.cjs` (generated from `src/token-scanner.cts`). Design: `.gsd/phase/chore-3414-tokenizer-first-seam/40-design.md`. + ### Planning Snapshot Module Module owning the parsed projection of `.planning/` that a diagnostic rule may read, per ADR-3180 §8.1 (Decision 8, Phase 10, #3308). `buildPlanningSnapshot(cwd) → PlanningSnapshot` is composed EXCLUSIVELY from the already-consolidated §7 owners — `getMilestoneInfo` (Roadmap Parser Module), `listMilestonePhaseDirs` (Phase Locator Module), `isPhaseComplete` (Verification Module), `scanPhasePlans` (Plan Scan Module), `stateFieldValue`/`stateCurrentPositionSlice` (STATE.md Document Module), `planningPaths` (Planning Workspace Module) — and introduces no new semantic derivation of its own. `PlanningSnapshot` exposes `milestone`/`phaseDirs`/`phases`/`currentPhaseLabel`, each a `{value, scope}` pair per the Planning Scope Module's frozen `SCOPE` enum; `phases` additionally carries a `PhaseSnapshot[]` (`dir`, `complete`, `verificationStatus`, `planCount`, `summaryCount`, `scope`). The one new piece of logic this module adds is `worstScope(...scopes) → Scope`, a pure severity-ordered combinator (`UNREADABLE` > `UNSCOPED` > `TRUNCATED` > `COMPLETE`) that folds several independently-scoped owner answers about the same phase directory into one composite signal — NOT a re-derivation of any owner (each owner's own algorithm is untouched; only their already-computed `scope` verdicts are combined), but new coordination logic no single owner has the visibility to express. Every exposed field carries PARSED values only, never raw document text — this is structural, not advisory: a diagnostic rule given only the parsed value cannot re-derive a field's location the way `#3162`'s three inert `Current Phase` literal-search predicates did. Read failures on STATE.md (exists-but-unreadable, distinct from absent) are reported via the Unusable Input Diagnostic Module's `warnUnusableInput(UNUSABLE_REASON.STATE_UNREADABLE)`. Guarded by `scripts/lint-planning-snapshot-bypass-drift.cjs` (ratcheted per Decision 4(e), scoped to `DIAGNOSTIC_RULE_FUNCTIONS` — currently `cmdValidateHealth` in `src/verify.cts` only, acknowledging its existing raw `.planning/` reads as debt owned by Phase 11, #3309, which migrates it onto this snapshot). Source of truth: `gsd-core/bin/lib/planning-snapshot.cjs` (generated from `src/planning-snapshot.cts`). Design: `.gsd/phase/refactor-3308-planning-snapshot-parsed-projection/40-design.md`. diff --git a/docs/INVENTORY-MANIFEST.json b/docs/INVENTORY-MANIFEST.json index 2e9697977..f225a2f96 100644 --- a/docs/INVENTORY-MANIFEST.json +++ b/docs/INVENTORY-MANIFEST.json @@ -499,6 +499,7 @@ "teams-status.cjs", "template.cjs", "text-lines.cjs", + "token-scanner.cjs", "uat-predicate.cjs", "uat.cjs", "ui-consideration-probe.cjs", diff --git a/docs/INVENTORY.md b/docs/INVENTORY.md index ff6b96318..a2a8bf0e2 100644 --- a/docs/INVENTORY.md +++ b/docs/INVENTORY.md @@ -598,8 +598,8 @@ Full listing: `gsd-core/bin/lib/*.cjs`. | `surface.cjs` | Runtime surface module — manages the runtime enable/disable surface state independently of the install-time profile marker (ADR-0011 Phase 2) | | `task-command-router.cjs` | Thin CJS subcommand router adapter for `gsd-tools task` | | `template.cjs` | Template selection and filling with variable substitution | -| `text-lines.cjs` | Line-terminator handling seam — `splitLines`/`normalizeEol`/`detectEol`/`joinLines`, the sole owner of ` ? -` splitting and CRLF normalization; closes #3360's split-then-match fix in `frontmatter.cjs` (ADR-3212 §3, epic #3212 Phase 2, #3413) | +| `text-lines.cjs` | Line-terminator handling seam — `splitLines`/`normalizeEol`/`detectEol`/`joinLines`, the sole owner of `\r?\n` splitting and CRLF normalization; closes #3360's split-then-match fix in `frontmatter.cjs` (ADR-3212 §3, epic #3212 Phase 2, #3413) | +| `token-scanner.cjs` | Tokenizer-first seam for stateful grammars — `tokenizeShellLike` (quote-aware shell tokenizer, the primitive `hooks/lib/git-cmd.js` migrated onto) and `indentWidth` (bullet-nesting depth, closes #3169's cross-reference-vs-declaration false positive in `decisions.cts`) (ADR-3212 §4, epic #3212 Phase 3, #3414) | | `normalize-test-command.cjs` | Normalizes a resolved test command to a one-shot form so a watch-mode runner (vitest/jest) cannot hang a verification gate (#1857); shared by all three live test-command gates (regression, post-merge, audit-fix) | | `uat.cjs` | UAT file parsing, verification debt tracking, audit-uat support | | `uat-predicate.cjs` | UAT-passed predicate — markdown-aware evaluation of HUMAN-UAT results; returns pass only when all required checks pass; ignores false-positive contexts (frontmatter, fenced code, blockquotes, HTML comments) | diff --git a/eslint.config.mjs b/eslint.config.mjs index 850c377e9..a6deeb823 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -139,6 +139,7 @@ export default tseslint.config( 'gsd-core/bin/lib/planning-snapshot.cjs', 'gsd-core/bin/lib/pattern.cjs', 'gsd-core/bin/lib/text-lines.cjs', + 'gsd-core/bin/lib/token-scanner.cjs', 'gsd-core/bin/lib/health-diagnostic-types.cjs', 'gsd-core/bin/lib/health-diagnostic.cjs', 'gsd-core/bin/lib/health-diagnostic-rules/root-existence.cjs', diff --git a/hooks/lib/git-cmd.js b/hooks/lib/git-cmd.js index b578ca559..41fb8365f 100644 --- a/hooks/lib/git-cmd.js +++ b/hooks/lib/git-cmd.js @@ -19,9 +19,19 @@ * hook's own __dirname: * * const { isGitSubcommand } = require(path.join(__dirname, 'lib', 'git-cmd.js')); + * + * `tokenize()` delegates to the shared `src/token-scanner.cts` seam (ADR-3212 + * §4, epic #3212 Phase 3, #3414) — the built `gsd-core/bin/lib/token-scanner.cjs` + * artifact, not a sibling hooks/-tree file, because hook scripts are staged as + * standalone files at install time and a sibling require is a staging + * dependency that can fail silently (see gsd-workflow-guard.js's own + * KIMI_TOOL_NAMES comment for the precedent this follows). Re-exported here + * unchanged — every existing caller's behavior is identical (parity-asserted + * in tests/token-scanner.test.cjs row 5). */ const path = require('path'); +const { tokenizeShellLike } = require(path.join(__dirname, '..', '..', 'gsd-core', 'bin', 'lib', 'token-scanner.cjs')); /** * Git global options that take a following argument. @@ -57,39 +67,87 @@ const BOOLEAN_FLAGS = new Set([ * Handles single-quoted strings, double-quoted strings, and unquoted tokens. * Does NOT perform variable expansion or brace expansion. * + * Delegates to the shared `src/token-scanner.cts` seam — see the module + * header comment for why the built artifact, not a sibling require, is used. + * * @param {string} cmd * @returns {string[]} */ function tokenize(cmd) { - const tokens = []; + return tokenizeShellLike(cmd); +} + +/** + * Walk past leading env-prefix assignments and global git options, same as + * `isGitSubcommand`'s phases 1-3. Returns the index of the subcommand token, + * or -1 if the command does not resolve to a git invocation at all. + * + * @param {string[]} tokens + * @returns {number} + */ +function skipToSubcommand(tokens) { let i = 0; - const len = cmd.length; + while (i < tokens.length && /^[A-Za-z_][A-Za-z0-9_]*=/.test(tokens[i])) { + i++; + } + if (i >= tokens.length) return -1; + const gitToken = tokens[i++]; + if (path.basename(gitToken) !== 'git') return -1; - while (i < len) { - // Skip whitespace - while (i < len && /\s/.test(cmd[i])) i++; - if (i >= len) break; - - let token = ''; - while (i < len && !/\s/.test(cmd[i])) { - if (cmd[i] === "'") { - // Single-quoted string: take everything until closing ' - i++; - while (i < len && cmd[i] !== "'") token += cmd[i++]; - if (i < len) i++; // consume closing ' - } else if (cmd[i] === '"') { - // Double-quoted string: take everything until closing " (no escape handling) - i++; - while (i < len && cmd[i] !== '"') token += cmd[i++]; - if (i < len) i++; // consume closing " - } else { - token += cmd[i++]; - } + while (i < tokens.length) { + const t = tokens[i]; + const eqIdx = t.indexOf('='); + const flagName = eqIdx !== -1 ? t.slice(0, eqIdx) : t; + if (ARGUMENT_TAKING_FLAGS.has(flagName)) { + i += eqIdx !== -1 ? 1 : 2; + continue; } - if (token) tokens.push(token); + if (BOOLEAN_FLAGS.has(t)) { + i++; + continue; + } + break; + } + return i; +} + +/** + * Extract the branch-name argument from a git command line that creates or + * references one — `git checkout -b ` or `git branch `. Returns + * null for any other command, including plain `git checkout ` (switches + * branches, does not create one) and commands where a checkout/branch-shaped + * substring appears only inside a quoted argument (e.g. a commit message). + * + * New capability (ADR-3212 §4, epic #3212 Phase 3, #3414) exercising the + * shared scanner on the domain the ADR names ("a branch name... [is] not + * regular") — not a migration of existing duplicated logic; no prior + * implementation of this existed in the repo (design doc §1.2). + * + * @param {string} cmd + * @returns {string | null} + */ +function extractBranchArgument(cmd) { + if (!cmd) return null; + const tokens = tokenizeShellLike(cmd); + const subIdx = skipToSubcommand(tokens); + if (subIdx === -1 || subIdx >= tokens.length) return null; + const sub = tokens[subIdx]; + + if (sub === 'checkout') { + for (let j = subIdx + 1; j < tokens.length; j++) { + if (tokens[j] === '-b' && j + 1 < tokens.length) return tokens[j + 1]; + } + return null; } - return tokens; + if (sub === 'branch') { + for (let j = subIdx + 1; j < tokens.length; j++) { + if (!tokens[j].startsWith('-')) return tokens[j]; + } + return null; + } + + return null; } /** @@ -102,49 +160,15 @@ function tokenize(cmd) { function isGitSubcommand(cmd, sub) { if (!cmd || !sub) return false; - const tokens = tokenize(cmd); - let i = 0; - - // Phase 1: skip leading VAR=VALUE environment assignments - while (i < tokens.length && /^[A-Za-z_][A-Za-z0-9_]*=/.test(tokens[i])) { - i++; - } - - // Phase 2: the next token must be the git executable - if (i >= tokens.length) return false; - const gitToken = tokens[i++]; - if (path.basename(gitToken) !== 'git') return false; - - // Phase 3: consume git global options - while (i < tokens.length) { - const t = tokens[i]; - - // --flag=value form for argument-taking flags - const eqIdx = t.indexOf('='); - const flagName = eqIdx !== -1 ? t.slice(0, eqIdx) : t; - if (ARGUMENT_TAKING_FLAGS.has(flagName)) { - if (eqIdx !== -1) { - // consumed as one token: --git-dir=.git - i++; - } else { - // consumed as two tokens: -C /path - i += 2; - } - continue; - } - - if (BOOLEAN_FLAGS.has(t)) { - i++; - continue; - } - - // Not a global option — this is the subcommand - break; - } + // Phases 1-3 (env-prefix skip, git-executable check, global-option consume) + // extracted verbatim into skipToSubcommand — byte-identical logic, shared + // with extractBranchArgument rather than a second copy (#3212 Phase 3). + const tokens = tokenizeShellLike(cmd); + const subIdx = skipToSubcommand(tokens); // Phase 4: check the subcommand - if (i >= tokens.length) return false; - return tokens[i] === sub; + if (subIdx === -1 || subIdx >= tokens.length) return false; + return tokens[subIdx] === sub; } -module.exports = { isGitSubcommand, tokenize }; +module.exports = { isGitSubcommand, tokenize, extractBranchArgument }; diff --git a/src/decisions.cts b/src/decisions.cts index 1a88ae013..c7846b417 100644 --- a/src/decisions.cts +++ b/src/decisions.cts @@ -22,6 +22,7 @@ import { extractTaggedBlocks, collectSection, } from './markdown-sectionizer.cjs'; +import { indentWidth } from './token-scanner.cjs'; export interface Decision { id: string; @@ -125,6 +126,7 @@ function parseDecisionLines(block: string): ParseDecisionLinesResult { let category = ''; let inDiscretion = false; let current: Decision | null = null; + let openIndent: number | null = null; let parseMisses = 0; const flush = (): void => { @@ -132,6 +134,7 @@ function parseDecisionLines(block: string): ParseDecisionLinesResult { current.text = current.text.trim(); out.push(current); current = null; + openIndent = null; } }; @@ -155,6 +158,26 @@ function parseDecisionLines(block: string): ParseDecisionLinesResult { continue; } + // Nested bullet under an open decision (#3212 Phase 3, #3169): a bullet + // indented deeper than the currently-open decision's own bullet is that + // decision's elaboration (e.g. a cross-reference to a sibling decision), + // not a fresh declaration attempt — fold it into current.text exactly + // like a continuation line, before it ever reaches the declaration/ + // parse-miss regexes below. A bullet at the SAME or a SHALLOWER indent + // is unaffected — tested exactly as before this fix. See design doc + // .gsd/phase/chore-3414-tokenizer-first-seam/40-design.md §1.3 for why + // nesting depth, not bullet content, is the signal that distinguishes + // this from a genuinely malformed top-level declaration. + if ( + current && + openIndent !== null && + trimmed.startsWith('-') && + indentWidth(line) > openIndent + ) { + current.text += ' ' + trimmed; + continue; + } + // Colon form: `- **D-NN[ [tags]]:** text` const colonMatch = line.match(bulletColonRe); if (colonMatch) { @@ -165,6 +188,7 @@ function parseDecisionLines(block: string): ParseDecisionLinesResult { : []; const trackable = !inDiscretion && !tags.some((t) => NON_TRACKABLE_TAGS.has(t)); current = { id, text: colonMatch[3], category, tags, trackable }; + openIndent = indentWidth(line); continue; } @@ -181,6 +205,7 @@ function parseDecisionLines(block: string): ParseDecisionLinesResult { // title itself is embedded in the bold run but we report the body as text // (consistent with how the gate cares only about coverage, not title/body split). current = { id, text: emDashMatch[3] || '', category, tags, trackable }; + openIndent = indentWidth(line); continue; } @@ -197,6 +222,7 @@ function parseDecisionLines(block: string): ParseDecisionLinesResult { : []; const trackable = !inDiscretion && !tags.some((t) => NON_TRACKABLE_TAGS.has(t)); current = { id, text: titledColonMatch[3] || '', category, tags, trackable }; + openIndent = indentWidth(line); continue; } diff --git a/src/token-scanner.cts b/src/token-scanner.cts new file mode 100644 index 000000000..e7999d426 --- /dev/null +++ b/src/token-scanner.cts @@ -0,0 +1,68 @@ +/** + * token-scanner.cts — the tokenizer-first seam for stateful grammars (ADR-3212 + * §4, epic #3212 Phase 3, #3414). + * + * Source in src/token-scanner.cts, compiled to gsd-core/bin/lib/token-scanner.cjs + * (gitignored), per the repo's ADR-457 build-at-publish convention. + * + * Generalizes the proven `hooks/lib/git-cmd.js` token-walk (#3129 — "has not + * re-opened") into a primitive other stateful-grammar consumers can share. + * `git-cmd.js` migrates onto `tokenizeShellLike` with zero behavior change + * (ADR §6); its own env-prefix/flag/subcommand walk logic stays put — only + * the character-level tokenizer moves here. + * + * `indentWidth` is the primitive `src/decisions.cts`'s #3169 fix needs: a + * decision-bullet list can NEST (a bullet elaborating on an already-open + * decision, indented deeper than it), and a per-line regex cannot see that + * nesting — only tracking indentation relative to the currently-open + * decision can (ADR §4 criterion 1). See + * .gsd/phase/chore-3414-tokenizer-first-seam/40-design.md §1.3 for the + * disproven bold-run-content-classification alternative and why nesting + * depth, not bullet content, is the actual distinguishing signal. + */ + +/** + * Tokenize a shell-like command string: whitespace-split, with single- and + * double-quoted spans taken as one token each. No escape handling, no + * brace/variable expansion — matches `hooks/lib/git-cmd.js`'s original + * `tokenize()` exactly (parity-asserted in tests/token-scanner.test.cjs). + */ +export function tokenizeShellLike(cmd: string): string[] { + const tokens: string[] = []; + let i = 0; + const len = cmd.length; + + while (i < len) { + while (i < len && /\s/.test(cmd[i])) i++; + if (i >= len) break; + + let token = ''; + while (i < len && !/\s/.test(cmd[i])) { + if (cmd[i] === "'") { + i++; + while (i < len && cmd[i] !== "'") token += cmd[i++]; + if (i < len) i++; + } else if (cmd[i] === '"') { + i++; + while (i < len && cmd[i] !== '"') token += cmd[i++]; + if (i < len) i++; + } else { + token += cmd[i++]; + } + } + if (token) tokens.push(token); + } + + return tokens; +} + +/** + * Count a line's leading whitespace width. Tabs count as one column each + * (not expanded) — matches `src/decisions.cts`'s existing continuation-line + * check (`/^[ \t]/`), which never expanded tabs either; this function does + * not introduce a new tab-width policy. + */ +export function indentWidth(line: string): number { + const match = line.match(/^[ \t]*/); + return match ? match[0].length : 0; +} diff --git a/tests/decisions.test.cjs b/tests/decisions.test.cjs index e3a4383b5..84fd81fad 100644 --- a/tests/decisions.test.cjs +++ b/tests/decisions.test.cjs @@ -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 = [ + '', + "- **D-15:** some decision", + " - **D-06's fix does not close this.** Gating the Passed arm on the derived status passes cleanly here.", + '', + ].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 = [ + '', + "- **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.', + '', + ].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, [ + '', + '- **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.', + '', + ].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 = '\n- **D-01:** Use OAuth 2.0\n- **D-02** ratio 3:1\n'; + 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 = '\n- **D-01 no colon no dash here** just text\n'; + 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 = '\n - **D-01** malformed, nothing open above it\n'; + 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)', () => { diff --git a/tests/token-scanner.test.cjs b/tests/token-scanner.test.cjs new file mode 100644 index 000000000..5b2f84984 --- /dev/null +++ b/tests/token-scanner.test.cjs @@ -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); + }), + ); + }); +}); diff --git a/tests/worktree-safety.test.cjs b/tests/worktree-safety.test.cjs index e6ba62e0d..f31842410 100644 --- a/tests/worktree-safety.test.cjs +++ b/tests/worktree-safety.test.cjs @@ -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 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); + }); +}); }); }