From 48271de43fd60d02d7432e1a940a8eb4b9d7d03f Mon Sep 17 00:00:00 2001 From: 0xdhx Date: Mon, 14 Sep 2026 18:32:12 -0500 Subject: [PATCH] fix(#4660): widen the 6 shell/markdown phase-id mirrors to the canonical grammar's letter axis (#4744) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(#4660): pin the letter-axis parity defect across all 6 shell/markdown phase-id sites Extends tests/nsegment-phase-grammar.test.cjs (#4568) one axis over: for each of the six sites, reads the live regex off disk and asserts it agrees with src/phase-id.cts's PHASE_NUMBER_TOKEN_SOURCE on the letter axis in BOTH directions — accepts `12A` / `3A` / `03A` / `23A.1.2`, still rejects `3a`, `3AB`, `A3` and the other canonical-invalid shapes — and that the two extracting sites return the full letter-suffixed token rather than its digit prefix (or nothing). Negative control against the unfixed tree: 22 failures, exactly the "(fails before the fix)" cases; every reject-parity case already green. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01NLtEbRc1Qfbe95HRMNqwp3 * fix(#4660): widen the 6 shell/markdown phase-id mirrors to the canonical grammar's letter axis Adds `[A-Z]?` after the leading digit run at all six sites #4568 widened — the ERE translation of src/phase-id.cts's `\d+[A-Z]?(?:\.\d+)*` — so a documented, canonical-valid id like `12A` or `23A.1.2` is no longer refused by the four validating sites (code-review.md, code-review-fix.md, gsd-code-fixer.md, gsd-code-fixer.compact.md) or truncated to its digit prefix by the two extracting sites (execute-plan.md's plan-filename grep, plan-phase.md's --research-phase capture). Behaviour is byte-identical for every id that matched before; the adjacent comment and error-message text now names the grammar it mirrors. Driven: `init code-review 3A` on a fixture with a `03A-slug/` directory and a `### Phase 3A:` heading emits `padded_phase: "03A"`, which the old regex rejects and the widened one accepts — nothing upstream of the validator mangles the id. At execute-plan.md the trailing `-[0-9]+` is the PLAN number and stays digit-only; plan and milestone dimensions are out of scope per the brief. `CASE_FLEXIBLE_PHASE_NUMBER_TOKEN_SOURCE` derives from the canonical source by a literal `.replaceAll('A-Z', 'A-Za-z')`, so src/phase-id.cts is deliberately untouched. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01NLtEbRc1Qfbe95HRMNqwp3 * chore(#4634): extend lint-phase-id-drift to ban a letter-less phase-id mirror in workflows/ and agents/ Adds findLetterlessPhaseMirrorDrift — the letter-axis twin of the #4568 single-segment rule — flagging the unbounded-segment shape `[0-9]+(\.[0-9]+)*` (and its \d / doubled-backslash near-variants) whose digit run is NOT followed by the `[A-Z]?` class, on any phase-carrying line across gsd-core/workflows/**/*.md, gsd-core/references/**/*.md and agents/**/*.md. Sanctioned the same way (``), tolerates the case-flexible `[A-Za-z]?` directory-scanning variant so it cannot force that separate axis to narrow, and is wired into scanAll. Confirmed zero violations against the real tree post-#4660 fix, and one violation when a single site is reverted. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01NLtEbRc1Qfbe95HRMNqwp3 * docs(#4660): add Fixed changeset Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01NLtEbRc1Qfbe95HRMNqwp3 * chore: regenerate conformance-tier manifests for the extended grammar test tests/nsegment-phase-grammar.test.cjs now requires the compiled gsd-core/bin/lib/phase-id.cjs (to assert the canonical grammar agrees with each site's live regex), which moves it to a different platform-conformance tier; `gen-platform-conformance-tier.cjs --check` in lint:ci flagged the macOS manifest as stale. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01NLtEbRc1Qfbe95HRMNqwp3 * test(#4660): reword a comment that tripped lint-docs-guard-registration The comment mentioned `docs/CONFIGURATION.md` between two backticked tokens, which the lint's template-literal detector read as a docs/ path expression. The test reads no docs/ file. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01NLtEbRc1Qfbe95HRMNqwp3 * chore(#4660): refresh the compact-content benchmark baseline and acknowledge emitted growth plan-phase.md grew by 4 bytes (`[A-Z]?`), which moves the committed compact-content benchmark; refreshed with `benchmark-compact-content.cjs --write`. The six shipped files below grew by the widened regex literal plus the comment and error-message text that now names the canonical grammar. Emitted-Drift-Ack-Growth: code-review.md — #4660: `[A-Z]?` at the PADDED_PHASE validator plus a comment/error message naming the canonical grammar and the `12A` example Emitted-Drift-Ack-Growth: code-review-fix.md — #4660: `[A-Z]?` at the PADDED_PHASE validator plus a comment/error message naming the canonical grammar and the `12A` example Emitted-Drift-Ack-Growth: gsd-code-fixer.md — #4660: `[A-Z]?` at the padded_phase sink validator plus the defense-in-depth comment and error message updated to the canonical grammar Emitted-Drift-Ack-Growth: gsd-code-fixer.compact.md — #4660: `[A-Z]?` at the padded_phase sink validator plus the comment and error message updated to the canonical grammar Emitted-Drift-Ack-Growth: execute-plan.md — #4660: `[A-Z]?` in the plan-filename phase extraction (6 bytes) Emitted-Drift-Ack-Growth: plan-phase.md — #4660: `[A-Z]?` in the --research-phase capture (6 bytes) Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01NLtEbRc1Qfbe95HRMNqwp3 * chore(#4660): set changeset fragment pr to 4744 * chore: re-trigger Validate Branch Name The required check-branch context was cancelled on this head by the workflow's cancel-in-progress group when the changeset pr-field backfill push landed three seconds after the PR opened; no completed run exists for the current head, and a fork contributor cannot re-run it. Empty commit to re-run it. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01NLtEbRc1Qfbe95HRMNqwp3 --------- Co-authored-by: CI Rebase Check Co-authored-by: Claude Fable 5.1 Co-authored-by: Tom Boucher --- .changeset/daring-badgers-glide.md | 5 + agents/gsd-code-fixer.compact.md | 9 +- agents/gsd-code-fixer.md | 11 +- gsd-core/workflows/code-review-fix.md | 7 +- gsd-core/workflows/code-review.md | 7 +- gsd-core/workflows/execute-plan.md | 2 +- gsd-core/workflows/plan-phase.md | 2 +- .../lib/macos-conformance-tier.generated.cjs | 1 + scripts/lint-phase-id-drift.cjs | 67 +++++++++ .../compact-content-benchmark-baseline.json | 8 +- tests/lint-phase-id-drift.test.cjs | 53 +++++++ tests/nsegment-phase-grammar.test.cjs | 141 ++++++++++++++++++ 12 files changed, 292 insertions(+), 21 deletions(-) create mode 100644 .changeset/daring-badgers-glide.md diff --git a/.changeset/daring-badgers-glide.md b/.changeset/daring-badgers-glide.md new file mode 100644 index 000000000..f15c0b8b2 --- /dev/null +++ b/.changeset/daring-badgers-glide.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4744 +--- +**`/gsd-code-review`, `/gsd-code-review-fix`, `gsd-code-fixer`, `/gsd-execute-plan` and `/gsd-plan-phase` now accept letter-variant phase ids (`12A`, `3A`, `23A.1.2`)** — the six shell/markdown phase-number mirrors #4568 widened on the segment-count axis were still digit-only on the letter axis, so a documented, canonical-valid id like `12A` was refused with "Invalid phase number format" by the four validating sites and silently truncated to its digit prefix by the two extracting ones. All six now match the canonical grammar (`src/phase-id.cts`), a parity test proves both sides agree on the letter axis in both directions, and `lint-phase-id-drift` gains a ratchet so a digit-only mirror cannot re-diverge silently. (#4660) diff --git a/agents/gsd-code-fixer.compact.md b/agents/gsd-code-fixer.compact.md index 2352e63a4..babe3eec6 100644 --- a/agents/gsd-code-fixer.compact.md +++ b/agents/gsd-code-fixer.compact.md @@ -136,10 +136,11 @@ branch=$(git branch --show-current) test -n "$branch" || { echo "Detached HEAD is not supported for review-fix (#2686)"; exit 1; } # padded_phase is interpolated into a worktree PATH and a git BRANCH NAME — -# validate at this sink too (defense in depth): digits + one or more dotted -# numeric segments only (e.g. '02' or '36.14'); reject '../', spaces, shell metachars. -if ! [[ "$padded_phase" =~ ^[0-9]+(\.[0-9]+)*$ ]]; then - echo "Invalid padded_phase for review-fix: '$padded_phase' (expected e.g. '02', '36.14', or '23.1.2')"; exit 1 +# validate at this sink too (defense in depth): the canonical phase-number grammar +# (src/phase-id.cts) — digits, an optional single uppercase letter, then dotted +# numeric segments (e.g. '02', '36.14', '12A'); reject '../', spaces, shell metachars. +if ! [[ "$padded_phase" =~ ^[0-9]+[A-Z]?(\.[0-9]+)*$ ]]; then + echo "Invalid padded_phase for review-fix: '$padded_phase' (expected e.g. '02', '36.14', '23.1.2', or '12A')"; exit 1 fi # Recovery-sentinel: ${phase_dir}/.review-fix-recovery-pending.json existing means diff --git a/agents/gsd-code-fixer.md b/agents/gsd-code-fixer.md index d8377ad93..459dc14a7 100644 --- a/agents/gsd-code-fixer.md +++ b/agents/gsd-code-fixer.md @@ -254,13 +254,14 @@ test -n "$branch" || { echo "Detached HEAD is not supported for review-fix (#268 # #2647 defense-in-depth: padded_phase is interpolated into a worktree PATH # and a git BRANCH NAME below. The orchestrator (code-review-fix.md) already -# validates it as ^[0-9]+(\.[0-9]+)*$, but this agent prompt is a literal bash +# validates it as ^[0-9]+[A-Z]?(\.[0-9]+)*$, but this agent prompt is a literal bash # contract any caller can spawn — validate at the SINK too, so a future caller # that forgets cannot turn ${padded_phase} into a path-traversal or branch-name -# injection. Reject anything that is not digits + one or more dotted numeric -# segments (e.g. '02' or '36.14'); reject '../', spaces, shell metachars. -if ! [[ "$padded_phase" =~ ^[0-9]+(\.[0-9]+)*$ ]]; then - echo "Invalid padded_phase for review-fix: '$padded_phase' (expected e.g. '02', '36.14', or '23.1.2')"; exit 1 +# injection. Reject anything that is not the canonical phase-number grammar +# (src/phase-id.cts): digits, an optional single uppercase letter, then dotted +# numeric segments (e.g. '02', '36.14', '12A'); reject '../', spaces, shell metachars. +if ! [[ "$padded_phase" =~ ^[0-9]+[A-Z]?(\.[0-9]+)*$ ]]; then + echo "Invalid padded_phase for review-fix: '$padded_phase' (expected e.g. '02', '36.14', '23.1.2', or '12A')"; exit 1 fi # Recovery-sentinel handling (#2839): diff --git a/gsd-core/workflows/code-review-fix.md b/gsd-core/workflows/code-review-fix.md index fc39bf998..af82a0973 100644 --- a/gsd-core/workflows/code-review-fix.md +++ b/gsd-core/workflows/code-review-fix.md @@ -35,9 +35,10 @@ Parse from init JSON: `phase_found`, `phase_dir`, `phase_number`, `phase_name`, **Input sanitization (defense-in-depth):** ```bash -# Validate PADDED_PHASE contains only digits and dotted segments (e.g., "02", "03.1", "23.1.2") -if ! [[ "$PADDED_PHASE" =~ ^[0-9]+(\.[0-9]+)*$ ]]; then - echo "Error: Invalid phase number format: '${PADDED_PHASE}'. Expected digits (e.g., 02, 03.1, 23.1.2)." +# Validate PADDED_PHASE matches the canonical phase-number grammar (src/phase-id.cts): digits, +# an optional single uppercase letter, then dotted segments (e.g., "02", "03.1", "23.1.2", "12A") +if ! [[ "$PADDED_PHASE" =~ ^[0-9]+[A-Z]?(\.[0-9]+)*$ ]]; then + echo "Error: Invalid phase number format: '${PADDED_PHASE}'. Expected digits with an optional letter suffix (e.g., 02, 03.1, 23.1.2, 12A)." # Exit workflow fi ``` diff --git a/gsd-core/workflows/code-review.md b/gsd-core/workflows/code-review.md index 08ac00213..bcb863806 100644 --- a/gsd-core/workflows/code-review.md +++ b/gsd-core/workflows/code-review.md @@ -59,9 +59,10 @@ Parse from init JSON: `phase_found`, `phase_dir`, `phase_number`, `phase_name`, **Input sanitization (defense-in-depth):** ```bash -# Validate PADDED_PHASE contains only digits and dotted segments (e.g., "02", "03.1", "23.1.2") -if ! [[ "$PADDED_PHASE" =~ ^[0-9]+(\.[0-9]+)*$ ]]; then - echo "Error: Invalid phase number format: '${PADDED_PHASE}'. Expected digits (e.g., 02, 03.1, 23.1.2)." +# Validate PADDED_PHASE matches the canonical phase-number grammar (src/phase-id.cts): digits, +# an optional single uppercase letter, then dotted segments (e.g., "02", "03.1", "23.1.2", "12A") +if ! [[ "$PADDED_PHASE" =~ ^[0-9]+[A-Z]?(\.[0-9]+)*$ ]]; then + echo "Error: Invalid phase number format: '${PADDED_PHASE}'. Expected digits with an optional letter suffix (e.g., 02, 03.1, 23.1.2, 12A)." # Exit workflow fi ``` diff --git a/gsd-core/workflows/execute-plan.md b/gsd-core/workflows/execute-plan.md index 4922295a5..f0cdc58be 100644 --- a/gsd-core/workflows/execute-plan.md +++ b/gsd-core/workflows/execute-plan.md @@ -67,7 +67,7 @@ Find first PLAN without matching SUMMARY. Decimal phases supported (`01.1-hotfix **Exclude `external_job_waiting` plans from selection.** When choosing the first PLAN that lacks a matching SUMMARY, skip any plan whose `plan_id` matches an async-job manifest in `.planning/async-jobs/` (any status) — that plan is `external_job_waiting` or awaiting reconciliation, never work to (re-)dispatch (re-dispatching would duplicate the external job). Reconcile via the manifest / safe_resume_gate instead. ```bash -PHASE=$(echo "$PLAN_PATH" | grep -oE '[0-9]+(\.[0-9]+)*-[0-9]+') +PHASE=$(echo "$PLAN_PATH" | grep -oE '[0-9]+[A-Z]?(\.[0-9]+)*-[0-9]+') # config settings can be fetched via gsd_run query config-get if needed ``` diff --git a/gsd-core/workflows/plan-phase.md b/gsd-core/workflows/plan-phase.md index e3b722a4b..10182477c 100644 --- a/gsd-core/workflows/plan-phase.md +++ b/gsd-core/workflows/plan-phase.md @@ -128,7 +128,7 @@ In research-only mode, two modifiers control behavior when `RESEARCH.md` already ```bash RESEARCH_ONLY=false VIEW_ONLY=false -if [[ "$ARGUMENTS" =~ --research-phase[[:space:]]+([0-9]+(\.[0-9]+)*) ]]; then +if [[ "$ARGUMENTS" =~ --research-phase[[:space:]]+([0-9]+[A-Z]?(\.[0-9]+)*) ]]; then RESEARCH_ONLY=true PHASE="${BASH_REMATCH[1]}" fi diff --git a/scripts/lib/macos-conformance-tier.generated.cjs b/scripts/lib/macos-conformance-tier.generated.cjs index 0bdbc94c3..7af1d93a0 100644 --- a/scripts/lib/macos-conformance-tier.generated.cjs +++ b/scripts/lib/macos-conformance-tier.generated.cjs @@ -127,6 +127,7 @@ module.exports = { "tests/no-posix-mode-bit-assert.rule.test.cjs", "tests/no-private-binary-resolution.rule.test.cjs", "tests/no-unguarded-nonportable-exec.rule.test.cjs", + "tests/nsegment-phase-grammar.test.cjs", "tests/observability/event.test.cjs", "tests/onboard-command.test.cjs", "tests/opencode-plugin-adapter.test.cjs", diff --git a/scripts/lint-phase-id-drift.cjs b/scripts/lint-phase-id-drift.cjs index 4f48270b5..b811b0215 100644 --- a/scripts/lint-phase-id-drift.cjs +++ b/scripts/lint-phase-id-drift.cjs @@ -389,6 +389,66 @@ function scanMarkdownSingleSegmentPhaseRegex(root) { return violations; } +// #4660 (epic #4634): the six shell/markdown mirrors #4568 widened on the +// segment-count axis stayed digit-only on the LETTER axis — the canonical +// grammar (`src/phase-id.cts`) is `\d+[A-Z]?(?:\.\d+)*`, with an optional +// single uppercase letter after the leading digits (`12A`, `3A`, `23A.1.2`, +// documented in docs/CONFIGURATION.md and relied on by `renameIntegerPhases`). +// A digit-only mirror `[0-9]+(\.[0-9]+)*` hard-rejects (validating sites) or +// silently truncates (extracting sites) a letter-suffixed id. This rule is the +// ratchet for that axis, the twin of the single-segment rule above: it flags +// the unbounded-segment shape whose digit run is NOT followed by the letter +// class. `[A-Z]` is the canonical spelling; the case-flexible `[A-Za-z]` +// directory-scanning variant is a deliberately separate axis and is tolerated +// here so this rule cannot force it to narrow. +const LETTERLESS_PHASE_MIRROR_DRIFT_RE = + /(?:\\{1,2}d|\[0-9\])\+(?!\[A-Z(?:a-z)?\]\?)\(\\{1,2}\.(?:\\{1,2}d|\[0-9\])\+\)\*/; + +/** + * Pure: find every unsanctioned digit-only (letter-less) unbounded-segment + * phase regex in `text`, restricted to lines that plausibly carry a + * phase-number variable — the same `PHASE_CARRYING_LINE_RE` filter and the + * same `` sanction as the single-segment rule. + * Returns [{ line, found }]. + */ +function findLetterlessPhaseMirrorDrift(text) { + const out = []; + const lines = text.split('\n'); + for (let i = 0; i < lines.length; i++) { + const line = lines[i]; + const m = LETTERLESS_PHASE_MIRROR_DRIFT_RE.exec(line); + if (!m) continue; + if (!PHASE_CARRYING_LINE_RE.test(line)) continue; + if (isSanctionedByPrecedingComment(lines, i, MD_OWNER_RE)) continue; + out.push({ line: i + 1, found: m[0] }); + } + return out; +} + +/** + * Scan the same three markdown roots as the single-segment rule for + * unsanctioned letter-less phase-regex mirrors. Returns [{ file, line, found }] + * with repo-relative paths. + */ +function scanMarkdownLetterlessPhaseMirror(root) { + const violations = []; + for (const dir of SINGLE_SEGMENT_SCAN_DIRS) { + for (const file of walkMd(path.join(root, dir), [])) { + const rel = path.relative(root, file); + let text; + try { + text = fs.readFileSync(file, 'utf8'); + } catch { + continue; + } + for (const d of findLetterlessPhaseMirrorDrift(text)) { + violations.push({ file: rel, kind: 'letterless-phase-mirror', ...d }); + } + } + } + return violations; +} + // Authored TypeScript source only (the generated bin/lib/*.cjs mirror it). const SCAN_DIRS = ['src']; const SCAN_EXT = new Set(['.cts', '.ts', '.mts']); @@ -541,6 +601,7 @@ function scanAll(root) { ...scanRepo(root), ...scanMarkdownShellArith(root), ...scanMarkdownSingleSegmentPhaseRegex(root), + ...scanMarkdownLetterlessPhaseMirror(root), ]; } @@ -567,6 +628,9 @@ function main() { process.stderr.write('near-variant) is banned outright in gsd-core/workflows/**/*.md,\n'); process.stderr.write('gsd-core/references/**/*.md, and agents/**/*.md — widen it to `*` (unbounded\n'); process.stderr.write('segments) or sanction with ``.\n'); + process.stderr.write('A digit-only unbounded-segment phase regex `[0-9]+(\\.[0-9]+)*` on the same roots\n'); + process.stderr.write('is missing the canonical letter axis (#4660) — widen to `[0-9]+[A-Z]?(\\.[0-9]+)*`\n'); + process.stderr.write('or sanction with ``.\n'); process.stderr.write('A `.replace(\'{slug}\', ... || \'phase\')` fallback is banned outright (#4126) —\n'); process.stderr.write('use `renderPhaseBranchName(` or sanction with\n'); process.stderr.write('`// phase-id-owner: ` on the line directly above:\n'); @@ -585,8 +649,10 @@ module.exports = { findBranchSlugFallbackDrift, findShellPhaseArithDrift, findSingleSegmentPhaseRegexDrift, + findLetterlessPhaseMirrorDrift, scanMarkdownShellArith, scanMarkdownSingleSegmentPhaseRegex, + scanMarkdownLetterlessPhaseMirror, scanRepo, scanAll, countSelectorBaselines, @@ -597,4 +663,5 @@ module.exports = { BRANCH_SLUG_FALLBACK_DRIFT_RE, SHELL_PHASE_ARITH_DRIFT_RE, SINGLE_SEGMENT_PHASE_DRIFT_RE, + LETTERLESS_PHASE_MIRROR_DRIFT_RE, }; diff --git a/tests/fixtures/compact-content-benchmark-baseline.json b/tests/fixtures/compact-content-benchmark-baseline.json index bd548125c..093ee7676 100644 --- a/tests/fixtures/compact-content-benchmark-baseline.json +++ b/tests/fixtures/compact-content-benchmark-baseline.json @@ -28,8 +28,8 @@ "reductionPct": 13.59 }, "plan-phase": { - "offTokens": 27637, - "onTokens": 24347, + "offTokens": 27641, + "onTokens": 24351, "reductionPct": 11.9 }, "verify-work": { @@ -39,8 +39,8 @@ } }, "aggregate": { - "offTokens": 107592, - "onTokens": 90944, + "offTokens": 107596, + "onTokens": 90948, "reductionPct": 15.47 } } diff --git a/tests/lint-phase-id-drift.test.cjs b/tests/lint-phase-id-drift.test.cjs index 4ecca449d..d59eb179a 100644 --- a/tests/lint-phase-id-drift.test.cjs +++ b/tests/lint-phase-id-drift.test.cjs @@ -10,6 +10,8 @@ const { findShellPhaseArithDrift, findSingleSegmentPhaseRegexDrift, scanMarkdownSingleSegmentPhaseRegex, + findLetterlessPhaseMirrorDrift, + scanMarkdownLetterlessPhaseMirror, } = require('../scripts/lint-phase-id-drift.cjs'); const ROOT = path.join(__dirname, '..'); @@ -159,3 +161,54 @@ test('scanMarkdownSingleSegmentPhaseRegex against the real repo tree reports zer const violations = scanMarkdownSingleSegmentPhaseRegex(ROOT); assert.deepEqual(violations, []); }); + +// #4660 (epic #4634): the letter-less phase-mirror ban — the letter-axis twin +// of the single-segment rule above. +test('findLetterlessPhaseMirrorDrift flags the digit-only [0-9]+(\\.[0-9]+)* shape on a phase-carrying line', () => { + const text = 'if ! [[ "$PADDED_PHASE" =~ ^[0-9]+(\\.[0-9]+)*$ ]]; then'; + const found = findLetterlessPhaseMirrorDrift(text); + assert.equal(found.length, 1); + assert.equal(found[0].line, 1); +}); + +test('findLetterlessPhaseMirrorDrift flags the extracting grep -oE form', () => { + const text = "PHASE=$(echo \"$PLAN_PATH\" | grep -oE '[0-9]+(\\.[0-9]+)*-[0-9]+')"; + assert.equal(findLetterlessPhaseMirrorDrift(text).length, 1); +}); + +test('findLetterlessPhaseMirrorDrift flags the \\d near-variant on a phase-carrying line', () => { + const text = 'if ! [[ "$padded_phase" =~ ^\\d+(\\.\\d+)*$ ]]; then'; + assert.equal(findLetterlessPhaseMirrorDrift(text).length, 1); +}); + +test('findLetterlessPhaseMirrorDrift is SILENT on the fixed [A-Z]? form', () => { + const text = 'if ! [[ "$PADDED_PHASE" =~ ^[0-9]+[A-Z]?(\\.[0-9]+)*$ ]]; then'; + assert.deepEqual(findLetterlessPhaseMirrorDrift(text), []); +}); + +test('findLetterlessPhaseMirrorDrift tolerates the case-flexible [A-Za-z]? directory-scanning variant', () => { + const text = 'if [[ "$phase_dir" =~ ^[0-9]+[A-Za-z]?(\\.[0-9]+)*- ]]; then'; + assert.deepEqual(findLetterlessPhaseMirrorDrift(text), []); +}); + +test('findLetterlessPhaseMirrorDrift does NOT flag the bounded single-segment shape (that is the other rule)', () => { + const text = 'if ! [[ "$PADDED_PHASE" =~ ^[0-9]+(\\.[0-9]+)?$ ]]; then'; + assert.deepEqual(findLetterlessPhaseMirrorDrift(text), []); +}); + +test('findLetterlessPhaseMirrorDrift does NOT flag a non-phase-carrying line (e.g. a version number)', () => { + const text = 'if ! [[ "$VERSION" =~ ^[0-9]+(\\.[0-9]+)*$ ]]; then'; + assert.deepEqual(findLetterlessPhaseMirrorDrift(text), []); +}); + +test('findLetterlessPhaseMirrorDrift does NOT flag a site sanctioned with an HTML comment', () => { + const text = [ + '', + 'if ! [[ "$PADDED_PHASE" =~ ^[0-9]+(\\.[0-9]+)*$ ]]; then', + ].join('\n'); + assert.deepEqual(findLetterlessPhaseMirrorDrift(text), []); +}); + +test('scanMarkdownLetterlessPhaseMirror against the real repo tree reports zero violations (#4660 fixed)', () => { + assert.deepEqual(scanMarkdownLetterlessPhaseMirror(ROOT), []); +}); diff --git a/tests/nsegment-phase-grammar.test.cjs b/tests/nsegment-phase-grammar.test.cjs index 178e9a112..891123b9e 100644 --- a/tests/nsegment-phase-grammar.test.cjs +++ b/tests/nsegment-phase-grammar.test.cjs @@ -177,3 +177,144 @@ describe('#4568 — plan-phase.md captures the full N-segment --research-phase v assert.equal(captureResearchPhase('--research-phase 23.1.2'), '23.1.2'); }); }); + +// --------------------------------------------------------------------------- +// #4660 — the LETTER axis. #4568 widened the six sites on the segment-count +// axis only; the canonical grammar also admits an optional single uppercase +// letter after the leading digits (`12A`, `3A`, `23A.1.2` — a documented +// phase-number shape in CONFIGURATION.md, relied on by renameIntegerPhases in +// src/phase.cts). These tests prove each site's live pattern and the canonical +// source AGREE on that axis, in both directions, rather than each merely +// "looking right" in isolation. +// --------------------------------------------------------------------------- + +// The canonical grammar is read from the committed bin/lib mirror the other +// grammar tests use (shell cannot import it; the test can). +const { PHASE_NUMBER_TOKEN_SOURCE } = require('../gsd-core/bin/lib/phase-id.cjs'); +const CANONICAL_ANCHORED = new RegExp('^(?:' + PHASE_NUMBER_TOKEN_SOURCE + ')$'); + +// Inputs the canonical grammar ACCEPTS. `03A` is what `normalizePhaseName('3A')` +// emits, i.e. the real `padded_phase` the four validating sites receive from +// `init`; the bare forms are what a user types or names a directory with. +const LETTER_ACCEPT = ['12A', '3A', '03A', '23A.1.2']; +// Inputs the canonical grammar REJECTS on the same axis — a parity test that +// only checks accepts would pass against `.*`. Lowercase is refused because +// the canonical source is case-sensitive `[A-Z]` (the case-flexible variant +// is a separate, deliberately distinct axis — see phase-id.cts). +const LETTER_REJECT = ['3a', '3AB', 'A3', '3A.', '3.A', '3A-1']; + +describe('#4660 — canonical grammar fixture agrees with the inputs this file uses', () => { + for (const v of LETTER_ACCEPT) { + test(`canonical accepts ${v}`, () => { + assert.equal(CANONICAL_ANCHORED.test(v), true); + }); + } + for (const v of LETTER_REJECT) { + test(`canonical rejects ${v}`, () => { + assert.equal(CANONICAL_ANCHORED.test(v), false); + }); + } +}); + +describe('#4660 — validating sites agree with the canonical grammar on the letter axis', () => { + const sites = [ + { name: 'code-review.md', file: CODE_REVIEW, anchor: 'if ! [[ "$PADDED_PHASE" =~ ' }, + { name: 'code-review-fix.md', file: CODE_REVIEW_FIX, anchor: 'if ! [[ "$PADDED_PHASE" =~ ' }, + { name: 'gsd-code-fixer.md', file: CODE_FIXER, anchor: 'if ! [[ "$padded_phase" =~ ' }, + { name: 'gsd-code-fixer.compact.md', file: CODE_FIXER_COMPACT, anchor: 'if ! [[ "$padded_phase" =~ ' }, + ]; + + for (const site of sites) { + describe(site.name, () => { + const text = fs.readFileSync(site.file, 'utf8'); + const pattern = extractAnchoredRegex(text, site.anchor); + + for (const v of LETTER_ACCEPT) { + test(`letter-suffixed id ${v} matches (fails before the fix)`, () => { + assert.equal(matchesValidatingRegex(pattern, v), true); + assert.equal(matchesValidatingRegex(pattern, v), CANONICAL_ANCHORED.test(v)); + }); + } + + for (const v of LETTER_REJECT) { + test(`canonical-invalid ${v} is still rejected (parity, not a blanket widening)`, () => { + assert.equal(matchesValidatingRegex(pattern, v), false); + assert.equal(matchesValidatingRegex(pattern, v), CANONICAL_ANCHORED.test(v)); + }); + } + }); + } +}); + +describe('#4660 — execute-plan.md extracts the full letter-suffixed phase from a plan filename', () => { + const text = fs.readFileSync(EXECUTE_PLAN, 'utf8'); + const pattern = extractGrepPattern(text, 'grep -oE'); + + function extractPhase(planPath) { + const script = `echo "$PLAN_PATH" | grep -oE '${pattern}'`; + let out; + try { + out = execFileSync('bash', [], { + input: script, + encoding: 'utf8', + timeout: TIMEOUT, + env: { ...process.env, PLAN_PATH: planPath }, + }).trim(); + } catch { + out = ''; + } + return out; + } + + // Before the fix a letter-suffixed filename either extracts NOTHING (the + // digit run is followed by the letter, so `-[0-9]+` never attaches) or the + // wrong tail (`23A.1.2-01` → `1.2-01`). Both are silent mis-extractions. + for (const [planPath, expected] of [ + ['/x/12A-01-PLAN.md', '12A-01'], + ['/x/03A-02-PLAN.md', '03A-02'], + ['/x/23A.1.2-01-PLAN.md', '23A.1.2-01'], + ]) { + test(`${planPath} extracts ${expected} (fails before the fix)`, () => { + const got = extractPhase(planPath); + assert.equal(got, expected); + // The phase half of the extraction is canonical-valid — parity with src/phase-id.cts. + assert.equal(CANONICAL_ANCHORED.test(got.replace(/-\d+$/, '')), true); + }); + } + + test('regression control: the letter class is admitted at the PHASE position only (plan numbers stay digit-only)', () => { + // `12A-B1`: the plan half must start with a digit, so nothing attaches to + // `12A-` and the digit-only tail `1` has no `-[0-9]+` after it either. + assert.equal(extractPhase('/x/12A-B1-PLAN.md'), ''); + }); +}); + +describe('#4660 — plan-phase.md captures the full letter-suffixed --research-phase value', () => { + const text = fs.readFileSync(PLAN_PHASE, 'utf8'); + const pattern = extractAnchoredRegex(text, '=~ --research-phase[[:space:]]+('); + + function captureResearchPhase(args) { + const script = [ + 'if [[ "$ARGUMENTS" =~ ' + pattern + ' ]]; then', + ' echo "${BASH_REMATCH[1]}"', + 'else', + ' echo NOMATCH', + 'fi', + ].join('\n'); + return execFileSync('bash', [], { + input: script, + encoding: 'utf8', + timeout: TIMEOUT, + env: { ...process.env, ARGUMENTS: args }, + }).trim(); + } + + // Before the fix the capture stops at the digit boundary: `12A` → `12`. + for (const v of ['12A', '3A', '23A.1.2']) { + test(`--research-phase ${v} captures ${v}, not its digit prefix (fails before the fix)`, () => { + const got = captureResearchPhase(`--research-phase ${v}`); + assert.equal(got, v); + assert.equal(CANONICAL_ANCHORED.test(got), true); + }); + } +});