diff --git a/scripts/resolvers/plan-gates.ts b/scripts/resolvers/plan-gates.ts index 930329a64..ccb215995 100644 --- a/scripts/resolvers/plan-gates.ts +++ b/scripts/resolvers/plan-gates.ts @@ -186,36 +186,52 @@ table-bearing section that must be the file's terminal heading.`; // ─── Plan File Discovery (shared helper) ────────────────────────────── +export const PLAN_AUDIT_NOT_RUN = + 'Plan completion audit: not run (no plan is bound to this branch and no docs/designs/ file matches). Fix: add "Plan: " to the PR body, or run /autoplan.'; + function generatePlanFileDiscovery(ship = false): string { return `### Plan File Discovery -1. **Conversation context (primary):** Use the active plan file from this conversation or its plan-mode system context. +Audit the plan this branch was built from, never a plan that is merely the newest file. -2. **Content-based search (fallback):** Without a conversation-supplied path, search by content: +1. **Conversation context (primary):** the plan-mode file in this conversation's system context, or the \`ACTIVE_PLAN\` of a \`/autoplan\` run in this conversation. Either is a binding. +2. **PR body binding:** a \`Plan: \` line in this branch's open PR body, printed below as \`PLAN_BINDING:\`. A relative path resolves against the repository root. +3. **Content-based search (fallback):** without a binding, list candidates; never pick one silently. \`\`\`bash setopt +o nomatch 2>/dev/null || true # zsh compat BRANCH=$(git branch --show-current 2>/dev/null | tr '/' '-' | tr -cd 'a-zA-Z0-9._-') -REPO=$(basename "$(git rev-parse --show-toplevel 2>/dev/null)") +_REPOTOP=$(git rev-parse --show-toplevel 2>/dev/null) +_BOUND=$(gh pr view --json body -q .body 2>/dev/null | tr -d '\\r\`' | sed -n 's/^[[:space:]]*Plan:[[:space:]]*\\([^[:space:]]*\\).*/\\1/p' | head -1) +[ -n "$_BOUND" ] && echo "PLAN_BINDING: $_BOUND" +if [ -n "$_REPOTOP" ]; then + _BASE=$(git merge-base "origin/" HEAD 2>/dev/null) + { [ -n "$_BASE" ] && git -C "$_REPOTOP" diff --name-only --diff-filter=AM "$_BASE" -- 'docs/designs/*.md' + [ -n "$BRANCH" ] && git -C "$_REPOTOP" grep -l -F -e "$BRANCH" -- 'docs/designs/*.md' + } 2>/dev/null | sort -u | sed "s|^|PLAN_CANDIDATE: $_REPOTOP/|" +fi GSTACK_STATE_ROOT=$(~/.claude/skills/gstack/bin/gstack-paths --get GSTACK_STATE_ROOT); : "\${GSTACK_STATE_ROOT:?gstack-paths failed; reinstall with ./setup or /gstack-upgrade}" _PLAN_SLUG=$(~/.claude/skills/gstack/bin/gstack-slug 2>/dev/null | sed -n 's/^SLUG=//p') || true _PLAN_SLUG="\${_PLAN_SLUG:-$(basename "$PWD" | tr -cd 'a-zA-Z0-9._-')}" for PLAN_DIR in "$GSTACK_STATE_ROOT/projects/$_PLAN_SLUG" "$HOME/.claude/plans" "$HOME/.codex/plans" ".gstack/plans"; do - [ -d "$PLAN_DIR" ] || continue - PLAN=$(ls -t "$PLAN_DIR"/*.md 2>/dev/null | xargs grep -l "$BRANCH" 2>/dev/null | head -1) - [ -z "$PLAN" ] && PLAN=$(ls -t "$PLAN_DIR"/*.md 2>/dev/null | xargs grep -l "$REPO" 2>/dev/null | head -1) - [ -z "$PLAN" ] && PLAN=$(find "$PLAN_DIR" -name '*.md' -mmin -1440 -maxdepth 1 2>/dev/null | xargs -r ls -t 2>/dev/null | head -1) - [ -n "$PLAN" ] && break + [ -d "$PLAN_DIR" ] && [ -n "$BRANCH" ] || continue + grep -l -F -e "$BRANCH" "$PLAN_DIR"/*.md 2>/dev/null | sed 's|^|PLAN_CANDIDATE: |' done -[ -n "$PLAN" ] && echo "PLAN_FILE: $PLAN" || echo "NO_PLAN_FILE" \`\`\` -3. **Validation:** For search results, read the first 20 lines and verify the project, feature and current branch. A mismatch means "no plan file found." Conversation-supplied paths bypass this search-result check. +\`PLAN_CANDIDATE:\` lines are repo-committed \`docs/designs/\` files this branch changed or that name the branch, then personal plan files that name it. Offer them with AskUserQuestion: one option per candidate (at most four), plus "No plan: skip the audit". Recommend the candidate only when exactly one \`docs/designs/\` file changed on this branch; otherwise recommend skipping. Spawned or non-interactive runs take the recommendation. Read the chosen file's first 20 lines to confirm the project and feature. -**Error handling:** -${ship ? `- No plan file found → skip with "No plan file detected — skipping." -- Plan file found but unreadable (permissions, encoding) → return an audit error to the parent. Do not report no plan or successful zero counts; the parent applies its audit-failure recovery and skip/stop decision.` : `- No plan file found → say "No plan file detected." and use the Fallback Intent Sources below. -- Plan file found but unreadable (permissions, encoding) → say "Plan file found but unreadable." and use the Fallback Intent Sources below; never report plan items as verified.`}`; +4. **No binding and no chosen candidate:** print exactly this line, then ${ship ? 'skip dispatch and record zero counts with this line as the summary' : 'use the Fallback Intent Sources below'}: + \`${PLAN_AUDIT_NOT_RUN}\` + +**Error handling:** a bound or chosen plan file that is unreadable (permissions, encoding) ${ship ? 'is an audit error, not "no plan": the parent applies its audit-failure recovery and skip/stop decision.' : '→ say "Plan file found but unreadable." and use the Fallback Intent Sources below; never report plan items as verified.'}`; +} + +/** Ship's child audit: the parent already bound the plan, so the child never searches. */ +function planInputShip(): string { + return `### Plan input + +Audit only the plan the parent supplied (path or full text). Do not search for another plan. If the supplied file is unreadable (permissions, encoding), return an audit error: do not report no plan or successful zero counts.`; } // ─── Plan Completion Audit ──────────────────────────────────────────── @@ -483,8 +499,8 @@ function generatePlanCompletionAuditInner(mode: PlanCompletionMode, part: 'audit const sections: string[] = []; let gate = ''; - // ── Plan file discovery (shared) ── - sections.push(generatePlanFileDiscovery(mode === 'ship')); + // ── Plan input: ship's parent binds the plan before dispatch (F1) ── + sections.push(mode === 'ship' ? planInputShip() : generatePlanFileDiscovery(false)); // ── Item extraction ── sections.push(planItemExtraction(mode)); @@ -513,7 +529,9 @@ export function generatePlanCompletionAuditShip(_ctx: TemplateContext): string { return generatePlanCompletionAuditInner('ship'); } -export function generatePlanCompletionGateShip(_ctx: TemplateContext): string { +/** `{{PLAN_COMPLETION_GATE_SHIP:discovery}}` renders the parent-side plan binding that runs before dispatch. */ +export function generatePlanCompletionGateShip(_ctx: TemplateContext, args?: string[]): string { + if (args?.includes('discovery')) return generatePlanFileDiscovery(true); return generatePlanCompletionAuditInner('ship', 'gate'); } diff --git a/ship/sections/plan-completion.md.tmpl b/ship/sections/plan-completion.md.tmpl index cada4329e..ce3640b1b 100644 --- a/ship/sections/plan-completion.md.tmpl +++ b/ship/sections/plan-completion.md.tmpl @@ -11,10 +11,13 @@ and `run_in_background: false`. Use Step 7's shared foreground-dispatch rule. The child reads the plan and every referenced code file; the parent validates its report and applies the gates below. -**Subagent prompt:** Substitute `` and supply the active plan's absolute path -or complete text, including user-approved scope changes. If none is known, say -so; the child runs the fallback search below. If discovery found no plan, skip -dispatch. The child does not inherit the parent's conversation. +**Before dispatch, the parent binds the plan:** + +{{PLAN_COMPLETION_GATE_SHIP:discovery}} + +**Subagent prompt:** Substitute `` and supply the bound plan's absolute path +or complete text, including user-approved scope changes. When discovery printed the +not-run line, skip dispatch. The child does not inherit the parent's conversation. ````text You are running a ship-workflow plan completion audit. The base branch is ``. Use `git diff origin/` and inspect untracked files from `git status` to see the full proposed change. Do not commit or push. Report only: classify every item, but do not execute Gate Logic, ask the user, or advance the workflow. The parent applies those gates to your report. diff --git a/test/remote-identity-vectors.test.ts b/test/remote-identity-vectors.test.ts index 2932dd821..04ffb71c8 100644 --- a/test/remote-identity-vectors.test.ts +++ b/test/remote-identity-vectors.test.ts @@ -138,7 +138,8 @@ describe('every resolver files a nested-group repo under the same slug', () => { encoding: 'utf-8', timeout: 30_000, }); - expect(r.stdout).toContain(`PLAN_FILE: ${path.join(planDir, 'plan.md')}`); + // F1 (#2768): search results are offered as candidates, never auto-bound. + expect(r.stdout).toContain(`PLAN_CANDIDATE: ${path.join(planDir, 'plan.md')}`); } finally { fs.rmSync(tmp, { recursive: true, force: true }); } diff --git a/test/review-workflow-clarity.test.ts b/test/review-workflow-clarity.test.ts index e3a63df17..e42948bb4 100644 --- a/test/review-workflow-clarity.test.ts +++ b/test/review-workflow-clarity.test.ts @@ -3,7 +3,7 @@ import { readFileSync } from 'node:fs'; import { join } from 'node:path'; import { generateReviewArmy } from '../scripts/resolvers/review-army'; import { generateCrossReviewDedup, generateSharedCodeReuse, generateScopeDrift } from '../scripts/resolvers/review-scope'; -import { generatePlanCompletionAuditReview, generatePlanCompletionAuditShip } from '../scripts/resolvers/plan-gates'; +import { generatePlanCompletionAuditReview, generatePlanCompletionAuditShip, generatePlanCompletionGateShip, PLAN_AUDIT_NOT_RUN } from '../scripts/resolvers/plan-gates'; import { generateQAReview } from '../scripts/resolvers/qa'; import { generateConfidenceCalibration } from '../scripts/resolvers/confidence'; import { HOST_PATHS, type TemplateContext } from '../scripts/resolvers/types'; @@ -468,12 +468,15 @@ test('review keeps its smoke-clock, setup-authority, plan-gate and findings-sour expect(qa).not.toContain('Ask for setup/permission'); expect(generateQAReview({ ...ctx, skillName: 'ship', tmplPath: 'ship/SKILL.md.tmpl' })).not.toContain('/review sets none'); const audit = generatePlanCompletionAuditReview(ctx).replace(/\s+/g, ' '); - expect(audit).toContain('"No plan file detected."'); - expect(audit).toContain('Fallback Intent Sources'); - expect(audit).not.toContain('skip with "No plan file detected — skipping."'); + // F1 (#2768): no bound plan prints the exact not-run line instead of the old notice. + expect(audit).toContain(PLAN_AUDIT_NOT_RUN); + expect(audit).toMatch(/use the Fallback Intent Sources below/); + expect(audit).not.toMatch(/skip dispatch/); expect(audit).toContain('**HIGH-impact plan-file discrepancies** trigger AskUserQuestion'); expect(audit).toMatch(/derived only from fallback sources[\s\S]{0,80}never trigger this question/i); - expect(generatePlanCompletionAuditShip({ ...ctx, skillName: 'ship', tmplPath: 'ship/SKILL.md.tmpl' })).toContain('skip with "No plan file detected — skipping."'); + const shipCtx = { ...ctx, skillName: 'ship', tmplPath: 'ship/SKILL.md.tmpl' }; + expect(generatePlanCompletionGateShip(shipCtx, ['discovery'])).toMatch(/skip dispatch and record zero counts/); + expect(generatePlanCompletionAuditShip(shipCtx)).toContain('Do not search for another plan'); const persist = skill.slice(skill.indexOf('### 2. Fill the record')).replace(/\s+/g, ' '); expect(persist).toMatch(/combined final-pass findings \(core, specialist, adversarial, actionable Greptile, verified exploratory QA findings\)/i); }); diff --git a/test/ship-plan-completion-invariants.test.ts b/test/ship-plan-completion-invariants.test.ts index e7a30d546..1aa7e3e24 100644 --- a/test/ship-plan-completion-invariants.test.ts +++ b/test/ship-plan-completion-invariants.test.ts @@ -238,3 +238,105 @@ test('push idempotency requires the live remote SHA and fails closed on transpor expect(unavailable.stdout).toContain('BLOCKED'); } finally { fs.rmSync(cwd, { recursive: true, force: true }); } }, 120_000); + +// F1 (#2768, #2800): the Plan Completion Audit used to fall back to "the newest +// plan file from the last day" and to any plan naming the repo, so a branch +// with no plan was audited against an unrelated one, and repo-committed +// docs/designs/ was never searched. The rendered discovery block now binds +// explicitly, lists docs/designs/ candidates, and never auto-picks. +describe('F1: plan binding, docs/designs candidates, exact not-run line', () => { + const ROOT = path.join(__dirname, '..'); + const NOT_RUN = 'Plan completion audit: not run (no plan is bound to this branch and no docs/designs/ file matches). Fix: add "Plan: " to the PR body, or run /autoplan.'; + const sections = { + ship: path.join(ROOT, 'ship', 'sections', 'plan-completion.md'), + review: path.join(ROOT, 'review', 'sections', 'plan-completion.md'), + }; + + function discovery(file: string): string { + const md = fs.readFileSync(file, 'utf8'); + return md.slice(md.indexOf('### Plan File Discovery')).match(/```bash\n([\s\S]*?)```/)![1] + .replaceAll('~/.claude/skills/gstack', ROOT).replaceAll('', 'main'); + } + + function fixture(opts: { design?: boolean; prBody?: string }) { + const tmp = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'plan-bind-'))); + const repo = path.join(tmp, 'repo'); + fs.mkdirSync(repo); + const env = { ...process.env, HOME: tmp, GSTACK_HOME: path.join(tmp, '.gstack'), GIT_CONFIG_GLOBAL: '/dev/null', GIT_AUTHOR_NAME: 't', GIT_AUTHOR_EMAIL: 't@t', GIT_COMMITTER_NAME: 't', GIT_COMMITTER_EMAIL: 't@t' } as Record; + delete env.GSTACK_STATE_ROOT; delete env.GSTACK_PROJECT_SLUG; + const git = (...a: string[]) => spawnSync('git', a, { cwd: repo, env, encoding: 'utf8', timeout: 30_000 }); + git('init', '-q', '-b', 'main'); + fs.mkdirSync(path.join(repo, 'docs', 'designs'), { recursive: true }); + fs.writeFileSync(path.join(repo, 'docs', 'designs', 'OLD_FEATURE.md'), '# an older, unrelated design\n'); + git('add', '-A'); git('commit', '-qm', 'base'); + git('update-ref', 'refs/remotes/origin/main', 'HEAD'); + git('checkout', '-qb', 'feat-pricing'); + fs.writeFileSync(path.join(repo, 'src.ts'), 'x\n'); + if (opts.design) fs.writeFileSync(path.join(repo, 'docs', 'designs', 'PRICING.md'), '# pricing plan\n'); + git('add', '-A'); git('commit', '-qm', 'work'); + // An unrelated plan-mode file from today, naming neither branch nor feature. + fs.mkdirSync(path.join(tmp, '.claude', 'plans'), { recursive: true }); + fs.writeFileSync(path.join(tmp, '.claude', 'plans', 'recent-unrelated.md'), '# refactor the billing page for repo\n'); + const bin = path.join(tmp, 'bin'); + fs.mkdirSync(bin); + if (opts.prBody !== undefined) fs.writeFileSync(path.join(tmp, 'pr-body.txt'), opts.prBody); + fs.writeFileSync(path.join(bin, 'gh'), opts.prBody === undefined ? '#!/bin/sh\nexit 1\n' : `#!/bin/sh\ncat '${path.join(tmp, 'pr-body.txt')}'\n`, { mode: 0o755 }); + env.PATH = `${bin}:${process.env.PATH}`; + return { tmp, repo, env }; + } + + function run(which: 'ship' | 'review', f: ReturnType) { + return spawnSync('bash', ['-c', discovery(sections[which])], { cwd: f.repo, env: f.env, encoding: 'utf8', timeout: 30_000 }); + } + + for (const which of ['ship', 'review'] as const) { + test(`${which}: a docs/designs file changed on this branch is a candidate; an unrelated recent plan is not`, () => { + const f = fixture({ design: true }); + try { + const out = run(which, f).stdout; + expect(out).toContain(`PLAN_CANDIDATE: ${path.join(f.repo, 'docs', 'designs', 'PRICING.md')}`); + expect(out).not.toContain('OLD_FEATURE.md'); + expect(out).not.toContain('recent-unrelated.md'); + expect(out).not.toContain('PLAN_FILE:'); + } finally { fs.rmSync(f.tmp, { recursive: true, force: true }); } + }); + + test(`${which}: nothing bound and nothing matching yields no plan at all`, () => { + const f = fixture({}); + try { + const out = run(which, f).stdout; + expect(out).not.toContain('PLAN_CANDIDATE:'); + expect(out).not.toContain('PLAN_FILE:'); + expect(out).not.toContain('recent-unrelated.md'); + } finally { fs.rmSync(f.tmp, { recursive: true, force: true }); } + }); + + test(`${which}: a "Plan:" line in the PR body is printed as the binding`, () => { + const f = fixture({ prBody: 'Summary\n\nPlan: `docs/designs/PRICING.md`\n' }); + try { + expect(run(which, f).stdout).toContain('PLAN_BINDING: docs/designs/PRICING.md'); + } finally { fs.rmSync(f.tmp, { recursive: true, force: true }); } + }); + + test(`${which}: binding precedence and the exact not-run line`, () => { + const md = fs.readFileSync(sections[which], 'utf8'); + const text = md.slice(md.indexOf('### Plan File Discovery')).replace(/\s+/g, ' '); + expect(text).toContain(NOT_RUN); + const order = ['Conversation context (primary)', 'PR body binding', 'Content-based search (fallback)', 'No binding and no chosen candidate']; + const at = order.map(o => text.indexOf(o)); + expect(at.every(i => i >= 0)).toBe(true); + expect([...at].sort((a, b) => a - b)).toEqual(at); + expect(text).toMatch(/never pick one silently/i); + expect(text).toContain('No plan: skip the audit'); + }); + } + + test('ship binds the plan in the parent before dispatch; the child never searches', () => { + const md = fs.readFileSync(sections.ship, 'utf8'); + expect(md.indexOf('### Plan File Discovery')).toBeLessThan(md.indexOf('````text')); + const child = md.slice(md.indexOf('````text'), md.lastIndexOf('````')); + expect(child).not.toContain('### Plan File Discovery'); + expect(child).toMatch(/Do not search for another plan/); + expect(md).toMatch(/not-run line, skip dispatch/); + }); +});