mirror of
https://github.com/garrytan/gstack.git
synced 2026-05-16 17:22:12 +08:00
v1.39.1.0 feat: EXIT PLAN MODE GATE for plan-mode review skills (#1512)
* feat: EXIT PLAN MODE GATE for plan-mode review skills Add a terminal BLOCKING checklist that verifies the plan file ends with `## GSTACK REVIEW REPORT` before ExitPlanMode is called. Lives at EOF of all four plan-* review skills (eng/ceo/design/devex) and inside codex Step 2A. Tones down the preamble's "Plan Status Footer" to a neutral forward reference so review-report rules don't bleed into operational skills (/ship /qa /review). Single source of truth: `generateExitPlanModeGate` in scripts/resolvers/review.ts, registered as EXIT_PLAN_MODE_GATE in scripts/resolvers/index.ts. New test in test/gen-skill-docs.test.ts strips fenced code blocks before matching `## ` headings and asserts the gate is the terminal heading in all four plan-* review SKILL.md files. Codex's SKILL.md uses toContain (mid-file by design — Step 2B/2C are not plan-touching modes). Decisions locked via /plan-eng-review + /codex outside-voice: - D1=A: 4 plan-* reviews + codex (autoplan, office-hours deferred) - D2=B → D4=A: tone preamble down to neutral forward reference - D3=A: add automated test in test/gen-skill-docs.test.ts - D5=B: keep codex gate inside Step 2A (mid-file acceptable per gate self-gating) Codex pre-merge findings folded in: line numbers obsolete (use EOF), test regex must strip fences, fresh skill list (not stale REVIEW_SKILLS constant), gate check 4 short-circuits when no plan file in context. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * chore: bump version and changelog (v1.39.1.0) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix: package.json build script uses subshells, not brace groups The three `{ git rev-parse HEAD 2>/dev/null || true; } > path/.version` brace groups in the build script regressed when v1.38.0.0 merged into this branch (resolved with --ours during conflict). Bun on Windows can't parse brace groups in this position; the v1.38.0.0 invariant requires `(...)` subshells. Windows CI test `package.json build scripts — POSIX shell compat` caught it. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
4
test/fixtures/golden/claude-ship-SKILL.md
vendored
4
test/fixtures/golden/claude-ship-SKILL.md
vendored
@@ -763,9 +763,7 @@ Replace `SKILL_NAME`, `OUTCOME`, and `USED_BROWSE` before running.
|
||||
|
||||
## Plan Status Footer
|
||||
|
||||
In plan mode before ExitPlanMode: if the plan file lacks `## GSTACK REVIEW REPORT`, run `~/.claude/skills/gstack/bin/gstack-review-read` and append the standard runs/status/findings table. With `NO_REVIEWS` or empty, append a 5-row placeholder with verdict "NO REVIEWS YET — run `/autoplan`". If a richer report exists, skip.
|
||||
|
||||
PLAN MODE EXCEPTION — always allowed (it's the plan file).
|
||||
Skills that run plan reviews (`/plan-*-review`, `/codex review`) include the EXIT PLAN MODE GATE blocking checklist at the end of the skill, which verifies the plan file ends with `## GSTACK REVIEW REPORT` before ExitPlanMode is called. Skills that don't run plan reviews (operational skills like `/ship`, `/qa`, `/review`) typically don't operate in plan mode and have no review report to verify; this footer is a no-op for them. Writing the plan file is the one edit allowed in plan mode.
|
||||
|
||||
## Step 0: Detect platform and base branch
|
||||
|
||||
|
||||
4
test/fixtures/golden/codex-ship-SKILL.md
vendored
4
test/fixtures/golden/codex-ship-SKILL.md
vendored
@@ -752,9 +752,7 @@ Replace `SKILL_NAME`, `OUTCOME`, and `USED_BROWSE` before running.
|
||||
|
||||
## Plan Status Footer
|
||||
|
||||
In plan mode before ExitPlanMode: if the plan file lacks `## GSTACK REVIEW REPORT`, run `$GSTACK_ROOT/bin/gstack-review-read` and append the standard runs/status/findings table. With `NO_REVIEWS` or empty, append a 5-row placeholder with verdict "NO REVIEWS YET — run `/autoplan`". If a richer report exists, skip.
|
||||
|
||||
PLAN MODE EXCEPTION — always allowed (it's the plan file).
|
||||
Skills that run plan reviews (`/plan-*-review`, `/codex review`) include the EXIT PLAN MODE GATE blocking checklist at the end of the skill, which verifies the plan file ends with `## GSTACK REVIEW REPORT` before ExitPlanMode is called. Skills that don't run plan reviews (operational skills like `/ship`, `/qa`, `/review`) typically don't operate in plan mode and have no review report to verify; this footer is a no-op for them. Writing the plan file is the one edit allowed in plan mode.
|
||||
|
||||
## Step 0: Detect platform and base branch
|
||||
|
||||
|
||||
4
test/fixtures/golden/factory-ship-SKILL.md
vendored
4
test/fixtures/golden/factory-ship-SKILL.md
vendored
@@ -754,9 +754,7 @@ Replace `SKILL_NAME`, `OUTCOME`, and `USED_BROWSE` before running.
|
||||
|
||||
## Plan Status Footer
|
||||
|
||||
In plan mode before ExitPlanMode: if the plan file lacks `## GSTACK REVIEW REPORT`, run `$GSTACK_ROOT/bin/gstack-review-read` and append the standard runs/status/findings table. With `NO_REVIEWS` or empty, append a 5-row placeholder with verdict "NO REVIEWS YET — run `/autoplan`". If a richer report exists, skip.
|
||||
|
||||
PLAN MODE EXCEPTION — always allowed (it's the plan file).
|
||||
Skills that run plan reviews (`/plan-*-review`, `/codex review`) include the EXIT PLAN MODE GATE blocking checklist at the end of the skill, which verifies the plan file ends with `## GSTACK REVIEW REPORT` before ExitPlanMode is called. Skills that don't run plan reviews (operational skills like `/ship`, `/qa`, `/review`) typically don't operate in plan mode and have no review report to verify; this footer is a no-op for them. Writing the plan file is the one edit allowed in plan mode.
|
||||
|
||||
## Step 0: Detect platform and base branch
|
||||
|
||||
|
||||
@@ -1090,14 +1090,16 @@ describe('Retro plan completion section', () => {
|
||||
// --- Plan status footer in preamble ---
|
||||
|
||||
describe('Plan status footer in preamble', () => {
|
||||
test('preamble contains plan status footer', () => {
|
||||
test('preamble contains plan status footer as neutral forward reference to EXIT PLAN MODE GATE', () => {
|
||||
// Read any skill that uses PREAMBLE
|
||||
const content = fs.readFileSync(path.join(ROOT, 'office-hours', 'SKILL.md'), 'utf-8');
|
||||
expect(content).toContain('Plan Status Footer');
|
||||
expect(content).toContain('GSTACK REVIEW REPORT');
|
||||
expect(content).toContain('gstack-review-read');
|
||||
expect(content).toContain('ExitPlanMode');
|
||||
expect(content).toContain('NO REVIEWS YET');
|
||||
expect(content).toContain('EXIT PLAN MODE GATE');
|
||||
// The preamble must NOT impose review-report rules on operational skills
|
||||
// that have no review report. It's a forward reference, not enforcement.
|
||||
expect(content).not.toContain('NO REVIEWS YET');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -3096,3 +3098,30 @@ describe('LEARNINGS_SEARCH resolver: query parameter', () => {
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('EXIT PLAN MODE GATE placement', () => {
|
||||
// Fresh skill list — do NOT reuse REVIEW_SKILLS upstream (3 entries, missing plan-devex).
|
||||
const planSkills = ['plan-eng-review', 'plan-ceo-review', 'plan-design-review', 'plan-devex-review'];
|
||||
|
||||
// Strip fenced code blocks before matching headings — PLAN_FILE_REVIEW_REPORT
|
||||
// already contains `## GSTACK REVIEW REPORT` inside a markdown example fence,
|
||||
// and the gate text itself shows `## GSTACK REVIEW REPORT` inside a fence too.
|
||||
const stripFences = (md: string) => md.replace(/```[\s\S]*?```/g, '');
|
||||
|
||||
test('gate is the terminal ## heading in every plan-* review SKILL.md', () => {
|
||||
for (const skill of planSkills) {
|
||||
const md = fs.readFileSync(path.join(ROOT, skill, 'SKILL.md'), 'utf-8');
|
||||
const stripped = stripFences(md);
|
||||
const headings = [...stripped.matchAll(/^## .+$/gm)].map(m => m[0]);
|
||||
const lastH2 = headings.at(-1);
|
||||
expect(lastH2, `${skill}/SKILL.md last ## heading (fences stripped)`).toBe('## EXIT PLAN MODE GATE (BLOCKING)');
|
||||
expect(md, `${skill}/SKILL.md gate body`).toContain('Failing this gate and calling ExitPlanMode anyway is a contract violation');
|
||||
}
|
||||
});
|
||||
|
||||
test('codex/SKILL.md contains gate (mid-file per D5; Step 2B/2C follow)', () => {
|
||||
const codex = fs.readFileSync(path.join(ROOT, 'codex', 'SKILL.md'), 'utf-8');
|
||||
expect(codex).toContain('## EXIT PLAN MODE GATE (BLOCKING)');
|
||||
expect(codex).toContain('Failing this gate and calling ExitPlanMode anyway is a contract violation');
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user