mirror of
https://github.com/garrytan/gstack.git
synced 2026-08-31 10:20:42 +02:00
feat(autoplan): eng review always runs last — the gate reviews the final amended plan
Reorder the pipeline to CEO -> Design (if UI scope) -> DX (if developer-facing scope) -> Eng. The old order (CEO -> Design -> Eng -> DX) let DX findings land AFTER the required gate signed off, so eng validated a stale plan. Accept-all semantics made explicit: every AskUserQuestion resolves to the recommended option; premises no longer pause the pipeline mid-run (clearly-wrong ones queue as User-Challenge items at the single Final Approval Gate). Eng's Codex voice now sees the DX consensus summary. New free static test pins the order; the chain E2E gains DX-between and Eng-terminal assertions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
394db326f2
commit
848973007c
@@ -0,0 +1,70 @@
|
||||
/**
|
||||
* /autoplan phase-order pin (free, static).
|
||||
*
|
||||
* The pipeline order is a deliberate design decision (2026-08-25, user-directed):
|
||||
* CEO → Design (if UI scope) → DX (if developer-facing scope) → Eng, ALWAYS LAST.
|
||||
* Eng is the required shipping gate — it must review the FINAL amended plan, so
|
||||
* every other phase's amendments land before it. The original order buried Eng
|
||||
* mid-pipeline (CEO → Design → Eng → DX), which let DX findings land AFTER the
|
||||
* gate had signed off — the gate validated a stale plan.
|
||||
*
|
||||
* These assertions pin the template so a refactor can't silently restore the
|
||||
* old order. The paid chain E2E (skill-e2e-autoplan-chain.test.ts) verifies the
|
||||
* runtime behavior; this pins the source of truth for free on every PR.
|
||||
*/
|
||||
import { describe, test, expect } from 'bun:test';
|
||||
import * as fs from 'fs';
|
||||
import * as path from 'path';
|
||||
|
||||
const ROOT = path.join(import.meta.dir, '..');
|
||||
const read = (p: string) => fs.readFileSync(path.join(ROOT, p), 'utf-8');
|
||||
|
||||
describe('autoplan phase order (Eng always last)', () => {
|
||||
const tmpl = read('autoplan/SKILL.md.tmpl');
|
||||
|
||||
test('Sequential Execution block names Eng as the terminal phase', () => {
|
||||
const block = tmpl.split('## Sequential Execution')[1]?.split('---')[0] ?? '';
|
||||
expect(block).toContain('Eng runs LAST, always');
|
||||
expect(block).toMatch(/CEO → Design.*→ DX.*→ Eng/s);
|
||||
// The old order must not resurface anywhere in the template.
|
||||
expect(tmpl).not.toContain('CEO → Design → Eng → DX');
|
||||
});
|
||||
|
||||
test('phase headings appear in the new order: 1, 2, 2.5, 3', () => {
|
||||
const idx = (h: string) => {
|
||||
const i = tmpl.indexOf(h);
|
||||
expect(i).toBeGreaterThan(-1);
|
||||
return i;
|
||||
};
|
||||
const p1 = idx('## Phase 1: CEO Review');
|
||||
const p2 = idx('## Phase 2: Design Review');
|
||||
const p25 = idx('## Phase 2.5: DX Review');
|
||||
const p3 = idx('## Phase 3: Eng Review');
|
||||
expect(p1).toBeLessThan(p2);
|
||||
expect(p2).toBeLessThan(p25);
|
||||
expect(p25).toBeLessThan(p3);
|
||||
// No stale Phase 3.5 heading or transition marker survives.
|
||||
expect(tmpl).not.toContain('Phase 3.5');
|
||||
});
|
||||
|
||||
test('phase sections hand off in the new order', () => {
|
||||
expect(read('autoplan/sections/dx-phase.md.tmpl')).toContain(
|
||||
'Passing to Phase 3 (Eng Review',
|
||||
);
|
||||
expect(read('autoplan/sections/eng-phase.md.tmpl')).toContain(
|
||||
'Passing to Phase 4 (Final Gate)',
|
||||
);
|
||||
// Eng's Codex voice sees every prior phase's consensus, DX included.
|
||||
expect(read('autoplan/sections/eng-phase.md.tmpl')).toContain(
|
||||
'DX: <insert DX consensus table summary',
|
||||
);
|
||||
});
|
||||
|
||||
test('single final gate: premises queue for the gate, never a mid-run stop', () => {
|
||||
expect(tmpl).toContain('One exception class — never auto-decided');
|
||||
expect(tmpl).not.toContain('Premise gate passed (user confirmed)');
|
||||
const ceo = read('autoplan/sections/ceo-phase.md.tmpl');
|
||||
expect(ceo).not.toContain('GATE: Present premises to user for confirmation');
|
||||
expect(ceo).toContain('Final');
|
||||
});
|
||||
});
|
||||
@@ -6,8 +6,9 @@
|
||||
*
|
||||
* "**Phase 1 complete." (CEO) →
|
||||
* "**Phase 2 complete." (Design — only if UI scope detected) →
|
||||
* "**Phase 3 complete." (Eng) →
|
||||
* "**Phase 3.5 complete." (DX — optional, skipped if no DX scope)
|
||||
* "**Phase 2.5 complete." (DX — optional, skipped if no DX scope) →
|
||||
* "**Phase 3 complete." (Eng — always runs, always LAST: the required
|
||||
* gate reviews the final amended plan)
|
||||
*
|
||||
* Why this exists: each individual phase has its own plan-mode smoke
|
||||
* test. Nothing verifies the SEQUENCING — that phases don't run in
|
||||
@@ -88,7 +89,7 @@ describeE2E('/autoplan chain ordering (periodic)', () => {
|
||||
// Phase markers live in autoplan's carved phase sections
|
||||
// (autoplan/sections/{ceo,design,eng,dx}-phase.md — the skeleton
|
||||
// STOP-Reads each one at its phase boundary):
|
||||
// "**Phase 1 complete." / "**Phase 2 complete." / "**Phase 3 complete." / "**Phase 3.5 complete."
|
||||
// "**Phase 1 complete." / "**Phase 2 complete." / "**Phase 2.5 complete." / "**Phase 3 complete."
|
||||
const phasePattern = /\*\*Phase\s+(\d+(?:\.\d+)?)\s+complete\.?\*\*/g;
|
||||
|
||||
let lastPermSig = '';
|
||||
@@ -163,13 +164,22 @@ describeE2E('/autoplan chain ordering (periodic)', () => {
|
||||
);
|
||||
}
|
||||
|
||||
// Sequencing: CEO must end before Eng ends. Design (if observed)
|
||||
// must end after CEO and before Eng.
|
||||
// Sequencing: CEO must end before Eng ends — and Eng is the terminal
|
||||
// phase (the required gate reviews the final amended plan). Design and
|
||||
// DX (if observed) must end after CEO and before Eng.
|
||||
expect(ceo.ts).toBeLessThan(eng.ts);
|
||||
if (design) {
|
||||
expect(design.ts).toBeGreaterThan(ceo.ts);
|
||||
expect(design.ts).toBeLessThan(eng.ts);
|
||||
}
|
||||
const dx = hits.find(h => h.phase === 2.5);
|
||||
if (dx) {
|
||||
expect(dx.ts).toBeGreaterThan(ceo.ts);
|
||||
expect(dx.ts).toBeLessThan(eng.ts);
|
||||
}
|
||||
// No phase marker may appear after Eng's (Eng-last invariant).
|
||||
const maxTs = Math.max(...hits.map(h => h.ts));
|
||||
expect(eng.ts).toBe(maxTs);
|
||||
} finally {
|
||||
try { fs.rmSync(tempDir, { recursive: true, force: true }); } catch { /* ignore */ }
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user