From 7c5234428420350796e5356dccf628013e5ca367 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Wed, 2 Sep 2026 13:45:14 -0400 Subject: [PATCH] fix(#4003): anchor the safe-resume gate's plan-scope greps to the milestone (#4194) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(#4003): safe_resume_gate must grep an anchored padding-tolerant scope * fix(#4003): anchor the resume-gate scope greps and bound them to the milestone tag Emitted-Drift-Ack-Growth: execute-phase.md — #4003 rewrites three commit-scope greps (safe_resume_gate, TDD RED, completion spot-check) to anchored zero-pad-tolerant regexes with a milestone tag bound; growth is the fix itself * test(#4003): align shape assertions with the implemented gate text * fix: bump fast-uri past GHSA-jqff-g426-hqxp (transitive, advisory reddened next) * fix(#4003): bound the TDD RED grep to the milestone and fix tdd.md's example greps * test(#4003): the gate pin tracks the anchored scope grep * fix(#4003): trim the gate rationale to hold the 93400 margin ceiling * test(#4003): the RED-grep pin tracks the milestone-bounded invocation * chore(#4003): changeset for the anchored resume-gate scope * chore(#4003): backfill changeset pr number --------- Co-authored-by: sim --- .changeset/lucky-jays-travel.md | 5 + gsd-core/references/tdd.md | 9 +- gsd-core/workflows/execute-phase.md | 22 +++- package-lock.json | 6 +- tests/config.test.cjs | 5 +- tests/safe-resume-gate-anchoring.test.cjs | 122 ++++++++++++++++++++++ 6 files changed, 158 insertions(+), 11 deletions(-) create mode 100644 .changeset/lucky-jays-travel.md create mode 100644 tests/safe-resume-gate-anchoring.test.cjs diff --git a/.changeset/lucky-jays-travel.md b/.changeset/lucky-jays-travel.md new file mode 100644 index 000000000..7ff19fdd4 --- /dev/null +++ b/.changeset/lucky-jays-travel.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4194 +--- +**`/gsd-execute-phase` crash-recovery gate now lists the crashed plan's own commits** — the safe-resume gate grepped a padded, unanchored plan scope, citing other milestones' commits and never the plan's own; all three commit-scope greps are now anchored, zero-pad-tolerant, and bounded to the current milestone tag. (#4003) diff --git a/gsd-core/references/tdd.md b/gsd-core/references/tdd.md index 2e2de42f6..8e41e23df 100644 --- a/gsd-core/references/tdd.md +++ b/gsd-core/references/tdd.md @@ -270,12 +270,15 @@ When `workflow.tdd_mode` is enabled in config, the RED/GREEN/REFACTOR gate seque After completing a `type: tdd` plan, the executor validates the git log: ```bash +# The commit protocol promises no zero-padding for ${PHASE}/${PLAN} — strip both and +# match the commit-scope position anchored (#4003). +PHASE_N=$((10#${PHASE})); PLAN_N=$((10#${PLAN})) # Check for RED gate commit -git log --oneline --grep="^test(${PHASE}-${PLAN})" | head -1 +git log --oneline -E --grep="^test\((0*${PHASE_N})-(0*${PLAN_N})\):" | head -1 # Check for GREEN gate commit -git log --oneline --grep="^feat(${PHASE}-${PLAN})" | head -1 +git log --oneline -E --grep="^feat\((0*${PHASE_N})-(0*${PLAN_N})\):" | head -1 # Check for optional REFACTOR gate commit -git log --oneline --grep="^refactor(${PHASE}-${PLAN})" | head -1 +git log --oneline -E --grep="^refactor\((0*${PHASE_N})-(0*${PLAN_N})\):" | head -1 ``` If RED or GREEN gate commits are missing, add a `## TDD Gate Compliance` section to SUMMARY.md with the violation details. diff --git a/gsd-core/workflows/execute-phase.md b/gsd-core/workflows/execute-phase.md index e7e6def40..c57512b92 100644 --- a/gsd-core/workflows/execute-phase.md +++ b/gsd-core/workflows/execute-phase.md @@ -182,9 +182,14 @@ TDD_MODE=$(gsd_run loop render-hooks execute:post --active-cap tdd) Before trusting `STATE.md` or dispatching any executor, derive `CURRENT_PLAN_ID` from the active incomplete plan in `INIT`, then search recent history: ```bash -CURRENT_PLAN_ID="{phase_number}-{plan_padded}" SUMMARY_PATH="{phase_dir}/{plan_padded}-SUMMARY.md" -PLAN_COMMITS=$(git log --oneline --grep="${CURRENT_PLAN_ID}" -30) +# #4003: no padding rule in the commit protocol, so zero-strip both components and +# match ANCHORED at the commit scope; bound to the latest reachable tag (milestone marker). +PHASE_N=$((10#{phase_number})) +PLAN_N=$((10#{plan_padded})) +PLAN_SCOPE_RE="^[a-z]+\((0*${PHASE_N})-(0*${PLAN_N})\):" +MILESTONE_BASE=$(git describe --tags --abbrev=0 2>/dev/null || echo "") +PLAN_COMMITS=$(git log --oneline -E ${MILESTONE_BASE:+"$MILESTONE_BASE..HEAD"} --grep="${PLAN_SCOPE_RE}" -30) ``` If production commits exist and `SUMMARY.md is missing` (no `.planning/async-jobs/*.json` manifest matches it: a match is a legal `external_job_waiting` deferral - reconcile per `docs/reference/planning-artifacts.md`, never re-dispatch), stop before spawning a new executor; continuing risks duplicate work and stale `STATE.md`/ROADMAP progress. @@ -199,7 +204,13 @@ Offer these recovery options: if [ "$TDD_MODE" = "true" ]; then IS_BEHAVIOR_ADDING=$(gsd_run query task.is-behavior-adding "$TASK_FILE" --pick is_behavior_adding) if [ "$IS_BEHAVIOR_ADDING" = "true" ]; then - RED_COMMIT=$(git log --oneline --grep="^test(${PHASE_NUMBER}-${PLAN_ID}):" -- "**/*.test.*" "**/*.spec.*" "tests/" | head -1) + # #4003: same anchored scope and milestone bound as safe_resume_gate — a padded + # literal grep hard-halts on a correct unpadded RED commit. + PHASE_N=$((10#${PHASE_NUMBER})) + PLAN_N=$((10#${PLAN_ID})) + PLAN_SCOPE_RE="^[a-z]+\((0*${PHASE_N})-(0*${PLAN_N})\):" + TDD_MILESTONE_BASE=$(git describe --tags --abbrev=0 2>/dev/null || echo "") + RED_COMMIT=$(git log --oneline -E ${TDD_MILESTONE_BASE:+"$TDD_MILESTONE_BASE..HEAD"} --grep="${PLAN_SCOPE_RE}" -- "**/*.test.*" "**/*.spec.*" "tests/" | head -1) if [ -z "$RED_COMMIT" ]; then gsd_run query state.update last_gate_trip "${PLAN_ID}/${TASK_ID}" || true echo "TDD GATE TRIPPED: missing RED commit for ${PLAN_ID}/${TASK_ID}" @@ -841,7 +852,10 @@ increases monotonically across waves. `{status}` is `complete` (success), ```bash # For each plan in this wave, check if the executor finished: SUMMARY_EXISTS=$(test -f "{phase_dir}/{plan_number}-{plan_padded}-SUMMARY.md" && echo "true" || echo "false") - COMMITS_FOUND=$(git log --oneline --all --grep="{phase_number}-{plan_padded}" --since="1 hour ago" | head -1) + # #4003: anchored, zero-pad-tolerant scope (see safe_resume_gate); --since stays. + SPOT_PHASE_N=$((10#{phase_number})) + SPOT_PLAN_N=$((10#{plan_padded})) + COMMITS_FOUND=$(git log --oneline --all -E --grep="^[a-z]+\((0*${SPOT_PHASE_N})-(0*${SPOT_PLAN_N})\):" --since="1 hour ago" | head -1) COMMITS_SINCE_DISPATCH=$(git log "${EXPECTED_BRANCH}" --since="${DISPATCH_TS}" --oneline | head -1) ``` diff --git a/package-lock.json b/package-lock.json index 8d34afbcb..8799b029b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3240,9 +3240,9 @@ } }, "node_modules/fast-uri": { - "version": "3.1.5", - "resolved": "https://registry.npmjs.org/fast-uri/-/fast-uri-3.1.5.tgz", - "integrity": "sha512-gHwA1O9LDIcKunMKhObS/HimwtehO1nPUECKAu5TpKgaO19fcWEl4bliWe1jWxVFvIXztJjjQ4L8XQ1EU9f7Jw==", + "version": "3.1.7", + "resolved": "https://registry.npmjs.org/fast-uri/-/fast-uri-3.1.7.tgz", + "integrity": "sha512-dOvZVzjdZdz7phd9v6jCbwxrBW3fK6n8Rc0CtdmM4bumzMnxywBYhuph6J819RRw/ku+rLbelwfMunktuzVVHg==", "funding": [ { "type": "github", diff --git a/tests/config.test.cjs b/tests/config.test.cjs index b61febc24..2f7318695 100644 --- a/tests/config.test.cjs +++ b/tests/config.test.cjs @@ -2467,7 +2467,10 @@ describe('bug #3212 execute-phase stall detection and safe resume', () => { const workflow = read('gsd-core/workflows/execute-phase.md'); assert.match(workflow, /, + * gsd-core/references/tdd.md:99 — specifies no padding rule, and both spellings are + * live in this repository's history). Workflow text IS the deployed product here, so + * the shape assertions are the faithful check; the behavioral fixture row runs the + * actual pipeline against a crafted history. + */ + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const { createTempGitProject, cleanup } = require('./helpers.cjs'); +const { gitOrThrow } = require('./helpers/git-fixture.cjs'); + +const WORKFLOW = path.join(__dirname, '..', 'gsd-core', 'workflows', 'execute-phase.md'); + +describe('#4003 — safe_resume_gate commit-scope greps', () => { + test('safe_resume_gate greps an anchored, padding-tolerant plan scope', () => { + const w = fs.readFileSync(WORKFLOW, 'utf8'); + // Anchored, ERE, zero-pad-tolerant on BOTH components — matches feat(2-02): and + // feat(02-02): alike, never a substring elsewhere in the message. + assert.ok(w.includes('PHASE_N=$((10#{phase_number}))'), + 'phase component must be zero-stripped via arithmetic base-10'); + assert.ok(w.includes('PLAN_N=$((10#{plan_padded}))'), + 'plan component must be zero-stripped via arithmetic base-10'); + assert.ok(w.includes('PLAN_SCOPE_RE="^[a-z]+\\((0*${PHASE_N})-(0*${PLAN_N})\\):"'), + 'the scope regex must be anchored to the commit-scope position and zero-pad-tolerant'); + assert.ok(w.includes('--grep="${PLAN_SCOPE_RE}"'), + 'the gate must grep the derived scope regex, not a padded literal'); + // The old unanchored padded-literal grep must be gone. + assert.ok(!w.includes('--grep="${CURRENT_PLAN_ID}"'), + 'the bare substring grep over the padded id must not remain'); + assert.ok(!w.includes('CURRENT_PLAN_ID="{phase_number}-{plan_padded}"'), + 'the padded id derivation must not remain'); + }); + + test('the gate bounds history to the current milestone with a no-tag fallback', () => { + const w = fs.readFileSync(WORKFLOW, 'utf8'); + assert.ok(w.includes('git describe --tags --abbrev=0'), + 'the milestone bound derives from the most recent reachable tag (complete-milestone git_tag)'); + assert.ok(w.includes('${MILESTONE_BASE:+"$MILESTONE_BASE..HEAD"}'), + 'the bounded invocation must range BASE..HEAD only when a base resolved'); + // Degrade must keep the anchor: a repo with no tags still gets the positional grep. + assert.ok(w.includes('MILESTONE_BASE=$(git describe --tags --abbrev=0 2>/dev/null || echo "")'), + 'a missing tag base must degrade to empty, not fail the gate'); + }); + + test('tdd red gate tolerates both commit-scope spellings (#4011 keying untouched)', () => { + const w = fs.readFileSync(WORKFLOW, 'utf8'); + assert.ok(w.includes('PHASE_N=$((10#${PHASE_NUMBER}))') && w.includes('PLAN_N=$((10#${PLAN_ID}))'), + 'the TDD block derives zero-stripped components'); + assert.ok(w.includes('RED_COMMIT=$(git log --oneline -E ${TDD_MILESTONE_BASE:+"$TDD_MILESTONE_BASE..HEAD"} --grep="${PLAN_SCOPE_RE}" -- "**/*.test.*"'), + 'the RED grep must use the same anchored padding-tolerant scope, milestone-bounded'); + assert.ok(!w.includes('--grep="^test(${PHASE_NUMBER}-${PLAN_ID})"'), + 'the padded-literal RED grep must not remain'); + assert.ok(w.includes('TDD_MILESTONE_BASE=$(git describe --tags --abbrev=0 2>/dev/null || echo "")'), + 'the RED grep carries the same milestone bound as the resume gate (#4003 review)'); + assert.ok(w.includes('${TDD_MILESTONE_BASE:+"$TDD_MILESTONE_BASE..HEAD"}'), + 'the RED grep range bound is applied'); + assert.ok(w.includes('if [ "$TDD_MODE" = "true" ]'), '#4011 TDD_MODE keying preserved'); + }); + + test('tdd.md gate-enforcement examples use the anchored padding-tolerant scope', () => { + const ref = fs.readFileSync(path.join(__dirname, '..', 'gsd-core', 'references', 'tdd.md'), 'utf8'); + assert.ok(!ref.includes('--grep="^test(${PHASE}-${PLAN})"'), + 'the padded-literal example grep must not remain'); + assert.ok(ref.includes('--grep="^test\\((0*${PHASE_N})-(0*${PLAN_N})\\):"'), + 'the RED example is anchored and zero-pad-tolerant'); + assert.ok(ref.includes('PHASE_N=$((10#${PHASE})); PLAN_N=$((10#${PLAN}))'), + 'the examples derive zero-stripped components'); + }); + + test('completion spot-check uses the anchored scope and keeps its time bound', () => { + const w = fs.readFileSync(WORKFLOW, 'utf8'); + assert.ok(!w.includes('--grep="{phase_number}-{plan_padded}"'), + 'the raw padded placeholder substring grep must not remain'); + assert.ok(w.includes('SPOT_PHASE_N=$((10#{phase_number}))') && w.includes('SPOT_PLAN_N=$((10#{plan_padded}))'), + 'the spot-check derives zero-stripped components'); + assert.ok(w.includes('--since="1 hour ago"'), 'the spot-check keeps its temporal bound'); + }); + + test('the gate pipeline separates same-scope commits across a milestone tag (behavioral)', (t) => { + // Reproduces the report on a crafted history: an OLD milestone commit with the + // same scope, a tag, then THIS plan's unpadded commits. The pipeline shape is + // the workflow's: tag base (when present) + anchored, padding-tolerant ERE. + const repo = createTempGitProject('gsd-4003-gate-'); + t.after(() => cleanup(repo)); + const g = (args) => gitOrThrow(args, { cwd: repo }); + + g(['commit', '--allow-empty', '-m', 'feat(02-02): old milestone same-scope commit']); + // Annotated: a plain `git tag` can demand a message under some git configs. + g(['tag', '-a', 'v9.0.0', '-m', 'milestone close']); + g(['commit', '--allow-empty', '-m', 'test(2-02): RED for this plan']); + g(['commit', '--allow-empty', '-m', 'feat(2-02): GREEN for this plan']); + g(['commit', '--allow-empty', '-m', 'feat(2-20): adjacent plan must not match']); + g(['commit', '--allow-empty', '-m', 'feat: mentions 02-02 in prose but not in scope']); + + const scope = '^[a-z]+\\((0*2)-(0*2)\\):'; + const base = g(['describe', '--tags', '--abbrev=0']).trim(); + const bounded = g(['log', '--oneline', '-E', `${base}..HEAD`, `--grep=${scope}`]).trim().split('\n'); + assert.ok(bounded.some((l) => /test\(2-02\): RED for this plan/.test(l)), 'this plan RED commit is found'); + assert.ok(bounded.some((l) => /feat\(2-02\): GREEN for this plan/.test(l)), 'this plan GREEN commit is found'); + assert.ok(!bounded.some((l) => /old milestone same-scope/.test(l)), 'the pre-tag same-scope commit is excluded'); + assert.ok(!bounded.some((l) => /adjacent plan/.test(l)), 'an adjacent plan scope does not match'); + assert.ok(!bounded.some((l) => /in prose/.test(l)), 'a prose mention outside the scope position does not match'); + + // No-tag fallback: strip the tag, keep the anchor — the old milestone commit + // becomes reachable again, but prose/adjacent scopes still never match. + g(['tag', '-d', 'v9.0.0']); + const unbounded = g(['log', '--oneline', '-E', `--grep=${scope}`]).trim().split('\n'); + assert.ok(unbounded.some((l) => /old milestone same-scope/.test(l)), + 'without a tag base the anchor alone cannot exclude prior milestones (degrade is honest)'); + assert.ok(!unbounded.some((l) => /in prose/.test(l)), 'the anchor still holds without a tag'); + }); +});