mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-08 09:48:20 +01:00
fix: recognize native plan dialogs and reviewer controls
This commit is contained in:
Garry Tan committed
1 parent
27d83b8c96
commit
c850ebbff0
17 files changed
+1342
-108
No files matched your search
@@ -92,7 +92,7 @@ For this phase (eng), retain the historical review-log skill identifier. Add `"h
|
||||
|
||||
Error handling: same as Phase 1 (both foreground/blocking, degradation matrix applies).
|
||||
|
||||
- Architecture choices: explicit over clever (P5). If codex disagrees with valid reason → TASTE DECISION. Scope changes both models agree on → USER CHALLENGE.
|
||||
- Architecture choices: explicit over clever (P5). If Codex disagrees with valid reason → TASTE DECISION. Scope changes both models agree on → USER CHALLENGE.
|
||||
- Evals: always include all relevant suites (P1)
|
||||
- Test plan: generate artifact at `~/.gstack/projects/$SLUG/{user}-{branch}-test-plan-{datetime}.md`
|
||||
- TODOS.md: collect all deferred scope expansions from every prior phase (Eng runs last), auto-write
|
||||
|
||||
@@ -51,7 +51,7 @@ Outer tool timeout: 720000ms. On any failed invocation or incomplete review, mar
|
||||
|
||||
Error handling: same as Phase 1 (both foreground/blocking, degradation matrix applies).
|
||||
|
||||
- Architecture choices: explicit over clever (P5). If codex disagrees with valid reason → TASTE DECISION. Scope changes both models agree on → USER CHALLENGE.
|
||||
- Architecture choices: explicit over clever (P5). If {{OUTSIDE_LABEL}} disagrees with valid reason → TASTE DECISION. Scope changes both models agree on → USER CHALLENGE.
|
||||
- Evals: always include all relevant suites (P1)
|
||||
- Test plan: generate artifact at `~/.gstack/projects/$SLUG/{user}-{branch}-test-plan-{datetime}.md`
|
||||
- TODOS.md: collect all deferred scope expansions from every prior phase (Eng runs last), auto-write
|
||||
|
||||
@@ -1,5 +1,8 @@
|
||||
import { expect, test } from 'bun:test';
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { readFileSync, existsSync, readdirSync } from 'node:fs';
|
||||
import { spawnSync } from 'node:child_process';
|
||||
import { createNativeReviewState } from './helpers/plan-count-fixture';
|
||||
import { getHermeticDirs } from './helpers/hermetic-env';
|
||||
import { resolve } from 'node:path';
|
||||
import { E2E_TOUCHFILES, selectTests } from './helpers/touchfiles';
|
||||
|
||||
@@ -27,3 +30,52 @@ test('the new fixture is isolated to the chain and its selection dependencies',
|
||||
expect(selectTests(['test/fixtures/plans/ui-heavy-feature.md'], E2E_TOUCHFILES).selected)
|
||||
.toEqual(['plan-design-with-ui-scope']);
|
||||
});
|
||||
|
||||
|
||||
test('native sequencing config reaches the real CLI reader without changing shared state', () => {
|
||||
const shared = getHermeticDirs().gstackHome;
|
||||
const before = readFileSync(resolve(shared, 'config.yaml'), 'utf8');
|
||||
const first = createNativeReviewState();
|
||||
const second = createNativeReviewState();
|
||||
try {
|
||||
expect(first.env.GSTACK_HOME).not.toBe(shared);
|
||||
expect(first.env.GSTACK_HOME).not.toBe(second.env.GSTACK_HOME);
|
||||
expect(first.env.GSTACK_STATE_ROOT).toBe(first.env.GSTACK_HOME);
|
||||
const result = spawnSync('bash', [resolve(root, 'bin/gstack-config'), 'get', 'codex_reviews'], {
|
||||
cwd: root, env: { ...process.env, ...first.env }, encoding: 'utf8', timeout: 5000,
|
||||
});
|
||||
expect(result.status, result.stderr).toBe(0);
|
||||
expect(result.stdout.trim()).toBe('disabled');
|
||||
for (const marker of readdirSync(shared).filter(name => name === '.activated' ||
|
||||
/^\..*(?:-seen|-prompted|-shown)$/.test(name) || name.startsWith('.feature-prompted-'))) {
|
||||
expect(readFileSync(resolve(first.env.GSTACK_HOME!, marker), 'utf8'))
|
||||
.toBe(readFileSync(resolve(shared, marker), 'utf8'));
|
||||
}
|
||||
first.cleanup();
|
||||
first.cleanup();
|
||||
expect(existsSync(first.env.GSTACK_HOME!)).toBe(false);
|
||||
expect(existsSync(second.env.GSTACK_HOME!)).toBe(true);
|
||||
expect(readFileSync(resolve(shared, 'config.yaml'), 'utf8')).toBe(before);
|
||||
} finally {
|
||||
first.cleanup();
|
||||
second.cleanup();
|
||||
}
|
||||
expect(existsSync(second.env.GSTACK_HOME!)).toBe(false);
|
||||
});
|
||||
|
||||
test('the UI/API chain requires all four native phases and registers its config dependency', () => {
|
||||
const source = read('test/skill-e2e-autoplan-chain.test.ts');
|
||||
const plan = read(fixture);
|
||||
expect(plan).toContain('## UI Scope');
|
||||
expect(plan).toContain('New REST endpoint `GET /api/dashboard`');
|
||||
expect(source).toContain('env: nativeState.env');
|
||||
expect(source).toContain('if (!ceo || !design || !dx || !eng)');
|
||||
expect(source).toContain('expect(ceo.ts).toBeLessThan(design.ts)');
|
||||
expect(source).toContain('expect(design.ts).toBeLessThan(dx.ts)');
|
||||
expect(source).toContain('expect(dx.ts).toBeLessThan(eng.ts)');
|
||||
expect(source).toContain('nativeState?.cleanup()');
|
||||
for (const file of ['test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'bin/gstack-config']) {
|
||||
expect(E2E_TOUCHFILES['autoplan-chain-pty']).toContain(file);
|
||||
expect(selectTests([file], E2E_TOUCHFILES).selected).toContain('autoplan-chain-pty');
|
||||
}
|
||||
});
|
||||
@@ -34,15 +34,61 @@ const PATH_ADJACENT = /\/\$\{?_BRANCH|\$\{_BRANCH\}\/|\$_BRANCH\//;
|
||||
// Raw $_BRANCH as a filename prefix (…-reviews.jsonl and friends).
|
||||
const FILENAME_PREFIX = /\$\{?_BRANCH\}?[A-Za-z0-9._-]*\.(?:jsonl|json|md|txt|log)/;
|
||||
|
||||
function renderedSkillFiles(): string[] {
|
||||
const out = execSync(
|
||||
`find "${ROOT}" -name 'SKILL.md' -not -path '*/node_modules/*' -not -path '*/.claude/*' ; find "${ROOT}" -path '*/sections/*.md' -not -path '*/node_modules/*' -not -path '*/.claude/*'`,
|
||||
{ encoding: 'utf-8', timeout: 30_000 },
|
||||
);
|
||||
return out.split('\n').filter(Boolean);
|
||||
function renderedSkillFiles(root = ROOT): string[] {
|
||||
// Enumerate managed render trees without buffering a shell's file census.
|
||||
// .context holds archived/experimental copies, not shipped skill output.
|
||||
const excluded = new Set(['node_modules', '.claude', '.context', '.git']);
|
||||
const files: string[] = [];
|
||||
function visit(dir: string, inSections = false) {
|
||||
for (const entry of fs.readdirSync(dir, { withFileTypes: true })) {
|
||||
const file = path.join(dir, entry.name);
|
||||
if (entry.isDirectory()) {
|
||||
if (!excluded.has(entry.name)) visit(file, inSections || entry.name === 'sections');
|
||||
} else if (entry.name === 'SKILL.md' || (inSections && entry.name.endsWith('.md'))) {
|
||||
files.push(file);
|
||||
}
|
||||
}
|
||||
}
|
||||
visit(root);
|
||||
return files;
|
||||
}
|
||||
|
||||
describe('branch slug hygiene (#2550, #1851)', () => {
|
||||
test('render discovery excludes scratch copies and retains every managed host without a pipe-size limit', () => {
|
||||
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-render-census-'));
|
||||
const add = (relative: string) => {
|
||||
const file = path.join(root, relative);
|
||||
fs.mkdirSync(path.dirname(file), { recursive: true });
|
||||
fs.writeFileSync(file, '# Render fixture\n');
|
||||
return file;
|
||||
};
|
||||
try {
|
||||
const expected = [
|
||||
add('review/SKILL.md'), add('review/sections/analysis.md'),
|
||||
add('.agents/skills/gstack-review/SKILL.md'),
|
||||
add('.kiro/skills/gstack-review/sections/nested/analysis.md'),
|
||||
add('skill with $quotes/sections/line\nbreak.md'),
|
||||
];
|
||||
for (const excluded of ['.context', '.claude', '.git', 'node_modules']) {
|
||||
add(`${excluded}/old-render/review/SKILL.md`);
|
||||
add(`${excluded}/old-render/review/sections/analysis.md`);
|
||||
}
|
||||
// The old execSync census failed at its 1 MiB stdout default once
|
||||
// enough isolated host renders existed in a workspace.
|
||||
for (let i = 0; i < 4500; i++) {
|
||||
expected.push(add(`host-output/skill-${i}-${'x'.repeat(210)}/SKILL.md`));
|
||||
}
|
||||
expect(Buffer.byteLength(expected.join('\n'))).toBeGreaterThan(1024 * 1024);
|
||||
const actual = renderedSkillFiles(root);
|
||||
const expectedSet = new Set(expected);
|
||||
expect(actual).toHaveLength(expected.length);
|
||||
expect(new Set(actual).size).toBe(expected.length);
|
||||
expect(actual.every(file => expectedSet.has(file))).toBe(true);
|
||||
} finally {
|
||||
fs.rmSync(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test('no generated SKILL.md or section interpolates raw $_BRANCH in a path position', () => {
|
||||
const offenders: string[] = [];
|
||||
for (const file of renderedSkillFiles()) {
|
||||
|
||||
@@ -1,5 +1,8 @@
|
||||
import { describe, expect, test } from 'bun:test';
|
||||
import { capturePlanCountQuestion, ceoStep0Boundary, nativePlanCallFingerprint, planCountQuestionPhase } from './helpers/claude-pty-runner';
|
||||
import * as fs from 'node:fs';
|
||||
import * as os from 'node:os';
|
||||
import * as path from 'node:path';
|
||||
import { capturePlanCountQuestion, ceoStep0Boundary, hasNativePlanTerminal, nativePlanCallFingerprint, planCountQuestionPhase } from './helpers/claude-pty-runner';
|
||||
import { isCeoCompletionHandoff, pickCeoCompletionHandoff } from './helpers/ceo-completion-handoff';
|
||||
import type { NativePlanQuestionCall } from './helpers/plan-count-transcript';
|
||||
import captures from './fixtures/ceo-completion-handoff-calls.json';
|
||||
@@ -127,3 +130,257 @@ describe('CEO completion handoff classification and selection', () => {
|
||||
expect(pickCeoCompletionHandoff(fingerprint(pending), unbound)).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
describe('completed CEO handoff with native next-step identity', () => {
|
||||
function capturedHandoff(): NativePlanQuestionCall {
|
||||
const question = 'D7 — CEO review is complete. Run /plan-eng-review next (the required shipping gate)? <gstack-qid:plan-ceo-review-next-step>';
|
||||
return {
|
||||
sessionId: 'e10cf0b4-525b-442d-9c2a-7a48d6b39f50',
|
||||
toolUseId: 'toolu_01FmkkRpoE3s6Y93KX6zLN1q',
|
||||
answered: true,
|
||||
failed: false,
|
||||
questions: [{
|
||||
question,
|
||||
header: 'Next review',
|
||||
multiSelect: false,
|
||||
options: [
|
||||
{ label: 'Run /plan-eng-review next (recommended)' },
|
||||
{ label: "Skip — I'll handle reviews manually" },
|
||||
],
|
||||
}],
|
||||
answers: { [question]: 'Run /plan-eng-review next (recommended)' },
|
||||
unansweredQuestionIndices: [],
|
||||
};
|
||||
}
|
||||
|
||||
test('captured completed-review menu is administrative and retains every independent finding and TODO', () => {
|
||||
const calls = captures.cases[1]!.calls.slice(0, -1).map(c => nativeCall(c));
|
||||
const result = replay([...calls, capturedHandoff()]);
|
||||
expect(result).toMatchObject({ reviewCount: 4, administrativeCount: 1, step0Count: 0 });
|
||||
expect(result.classifications.slice(0, -1).every(p => !p.administrative)).toBe(true);
|
||||
});
|
||||
|
||||
test('only the positively bound pending handoff selects manual, in either option order', () => {
|
||||
for (const reverse of [false, true]) {
|
||||
const call = capturedHandoff();
|
||||
call.answered = false;
|
||||
delete call.answers;
|
||||
if (reverse) call.questions[0]!.options.reverse();
|
||||
expect(pickCeoCompletionHandoff(fingerprint(call))).toBe(reverse ? 1 : 2);
|
||||
expect(isCeoCompletionHandoff(fingerprint(call))).toBe(false);
|
||||
}
|
||||
});
|
||||
|
||||
test('incomplete review, missing gate, findings, mixed choices, and unoffered answers stay substantive', () => {
|
||||
for (const mutate of [
|
||||
(c: NativePlanQuestionCall) => { c.questions[0]!.question = c.questions[0]!.question.replace('is complete', 'has an unresolved test gap'); },
|
||||
(c: NativePlanQuestionCall) => { c.questions[0]!.question = c.questions[0]!.question.replace('required shipping gate', 'optional follow-up'); },
|
||||
(c: NativePlanQuestionCall) => { c.questions[0]!.question = c.questions[0]!.question.replace('plan-ceo-review-next-step', 'plan-ceo-security-finding'); },
|
||||
(c: NativePlanQuestionCall) => { c.questions[0]!.header = 'TODO: email queue'; },
|
||||
(c: NativePlanQuestionCall) => { c.questions[0]!.options.push({ label: 'Add missing staging validation to this plan' }); },
|
||||
]) {
|
||||
const call = capturedHandoff();
|
||||
mutate(call);
|
||||
call.answers = { [call.questions[0]!.question]: call.questions[0]!.options[0]!.label };
|
||||
expect(isCeoCompletionHandoff(fingerprint(call))).toBe(false);
|
||||
expect(replay([call]).reviewCount).toBe(1);
|
||||
}
|
||||
const call = capturedHandoff();
|
||||
call.answers = { [call.questions[0]!.question]: 'First add the missing retry test' };
|
||||
expect(isCeoCompletionHandoff(fingerprint(call))).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
const CAPTURED_PAIRED_RETRY_CALLS: NativePlanQuestionCall[] = [
|
||||
{
|
||||
"sessionId": "eaedca8a-f52b-4739-a559-3f330e10b3c6",
|
||||
"toolUseId": "toolu_01CG8hh817d7CvFk9kH5ZFW4",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D6 — Section 2 finding: the 502 failure path test's assertion is under-specified. What does 'fails clean' mean as an observable outcome? <gstack-qid:plan-ceo-fails-clean>",
|
||||
"header": "502 failure mode",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "Specify the exception type in the plan (Recommended)",
|
||||
"description": "Update the plan to name the exception class processPayment() raises after 502 exhaustion (e.g. 'assert raises Stripe::APIConnectionError' or 'assert raises PaymentFailedError'). The test must assert a concrete observable: the exception class, not just 'something goes wrong.' Effort: add 1 line to the plan. Verify: test fails with wrong exception type.",
|
||||
"preview": "REMEDY:\n Plan change: add to item 2 under ## Tests:\n 'The 502 test must assert the specific exception class\n (or nil return, or error struct) processPayment() raises\n after retry exhaustion. The test factory already exposes\n mock call history; the test should also assert exactly 2\n charge attempts and 1 backoff sleep call.'\n\nWhy: without this, the implementer will write\n expect { processPayment() }.not_to raise_error\nwhich passes on the wrong behavior (swallowed exception)."
|
||||
},
|
||||
{
|
||||
"label": "Accept 'fails clean' as implementation-determined",
|
||||
"description": "Trust the implementer to look at processPayment() and assert whatever behavior they find. The test is still useful. Risk: if processPayment() silently swallows the error (no raise, no return value), the test will pass even when payment silently fails."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D6 — Section 2 finding: the 502 failure path test's assertion is under-specified. What does 'fails clean' mean as an observable outcome? <gstack-qid:plan-ceo-fails-clean>": "Specify the exception type in the plan (Recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-08T20:57:40.308Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "eaedca8a-f52b-4739-a559-3f330e10b3c6",
|
||||
"toolUseId": "toolu_01Bc1mwoqXgNQK7NVx8MA21L",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D7 — Section 4 finding: the happy path assertion 'correct receipt is generated' needs to be field-specific to be a correctness test. <gstack-qid:plan-ceo-receipt-fields>",
|
||||
"header": "Receipt assertion",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "Add field-level assertion requirement to the plan (Recommended)",
|
||||
"description": "Update the plan: the happy path test must assert specific receipt fields (at minimum: amount matches charged amount, stripe_charge_id matches the mock's returned charge ID). Prevents the test from being just a nil-check smoke test. Effort: add 1 line to the plan. Verify: test fails if receipt has wrong charge ID.",
|
||||
"preview": "REMEDY:\n Plan change: add to item 1 under ## Tests:\n 'The happy path test must assert field-level receipt\n correctness: at minimum, the receipt amount equals the\n charged amount and the receipt stripe_charge_id matches\n the charge ID returned by the Stripe mock.\n assert receipt.amount == expected_amount\n assert receipt.stripe_charge_id == mock_charge.id'\n\nWhy: 'assert receipt is generated' is a smoke test.\n It passes even if receipt contains wrong amount or\n no charge ID, which is the correctness we care about."
|
||||
},
|
||||
{
|
||||
"label": "Accept 'correct receipt generated' as-is",
|
||||
"description": "Leave it to the implementer. Risk: a future refactor that breaks receipt field accuracy (e.g., amount in wrong currency unit) could pass the test silently."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D7 — Section 4 finding: the happy path assertion 'correct receipt is generated' needs to be field-specific to be a correctness test. <gstack-qid:plan-ceo-receipt-fields>": "Add field-level assertion requirement to the plan (Recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-08T20:58:04.465Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "eaedca8a-f52b-4739-a559-3f330e10b3c6",
|
||||
"toolUseId": "toolu_014MYMCNbEGQfNYwqFkQQmKm",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D8 — TODO candidate: test the orphaned-payment scenario (receipt-builder fails after Stripe charge succeeds). Add to TODOS.md? <gstack-qid:plan-ceo-todo-orphaned-payment>",
|
||||
"header": "TODO: orphaned payment",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "Add to TODOS.md (Recommended)",
|
||||
"description": "Stripe charge succeeds, then receipt-builder throws — processPayment() returns an error to the caller. If the caller retries, the card gets charged twice. The existing receipt-builder regression tests cover receipt-builder itself, but nothing tests processPayment()'s behavior in this specific sequence. P2, M effort (human: ~1h / CC: ~8min).",
|
||||
"preview": "TODO entry:\n What: Test orphaned-payment scenario in processPayment()\n Why: Stripe charge succeeds, receipt-builder throws,\n caller retries → double charge. No test covers this.\n Where: payment test factory already supports this setup.\n Effort: M (human ~1h / CC ~8min)\n Priority: P2\n Depends on: this PR (test infra in place)"
|
||||
},
|
||||
{
|
||||
"label": "Skip — not valuable enough",
|
||||
"description": "The receipt-builder regression tests provide sufficient coverage. Double-charge scenario is handled by idempotency keys at the Stripe level."
|
||||
},
|
||||
{
|
||||
"label": "Build it now in this PR",
|
||||
"description": "Add a third test case to this PR: receipt-builder throws after successful charge — assert processPayment() returns the expected error and Stripe mock shows only 1 charge attempt (no retry on receipt failure). Expands scope from HOLD SCOPE decision."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D8 — TODO candidate: test the orphaned-payment scenario (receipt-builder fails after Stripe charge succeeds). Add to TODOS.md? <gstack-qid:plan-ceo-todo-orphaned-payment>": "Add to TODOS.md (Recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-08T20:59:02.919Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "eaedca8a-f52b-4739-a559-3f330e10b3c6",
|
||||
"toolUseId": "toolu_019ppgizjxzRiJd2QXPV7rYQ",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D9 — CEO review complete. Run /plan-eng-review next? <gstack-qid:plan-ceo-next-review>",
|
||||
"header": "Next review",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "Run /plan-eng-review next (Recommended)",
|
||||
"description": "Eng review is the required shipping gate. It covers architecture, code quality, and test correctness at the code level — what the CEO review doesn't dig into. The 2 spec gaps found here (exception type, receipt fields) should be verified at the code level too."
|
||||
},
|
||||
{
|
||||
"label": "Skip — handle reviews manually",
|
||||
"description": "Proceed without running eng review now. You can run it later with /plan-eng-review. Note: eng review is the only gate that blocks shipping by default."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D9 — CEO review complete. Run /plan-eng-review next? <gstack-qid:plan-ceo-next-review>": "Run /plan-eng-review next (Recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-08T21:03:34.802Z"
|
||||
}
|
||||
];
|
||||
|
||||
|
||||
describe('completed CEO next-review declaration and final report order', () => {
|
||||
test('canonical identity alone never replaces actual completion and the next-review header', () => {
|
||||
for (const mutate of [
|
||||
(call: NativePlanQuestionCall) => { call.questions[0]!.question = 'Should we finish reviewing? <gstack-qid:plan-ceo-next-steps>'; },
|
||||
(call: NativePlanQuestionCall) => { call.questions[0]!.header = 'New security issue'; },
|
||||
]) {
|
||||
const call = structuredClone(CAPTURED_PAIRED_RETRY_CALLS.at(-1)!);
|
||||
call.questions[0]!.question = call.questions[0]!.question.replace('plan-ceo-next-review', 'plan-ceo-next-steps');
|
||||
call.questions[0]!.options[1]!.label = "Skip — I'll handle reviews manually";
|
||||
mutate(call);
|
||||
call.answers = { [call.questions[0]!.question]: call.questions[0]!.options[0]!.label };
|
||||
expect(isCeoCompletionHandoff(fingerprint(call))).toBe(false);
|
||||
}
|
||||
});
|
||||
|
||||
test('the captured retry keeps its three findings/TODOs and recognizes only the completed handoff', () => {
|
||||
expect(replay(structuredClone(CAPTURED_PAIRED_RETRY_CALLS))).toMatchObject({
|
||||
reviewCount: 3, administrativeCount: 1, step0Count: 0,
|
||||
});
|
||||
const call = structuredClone(CAPTURED_PAIRED_RETRY_CALLS.at(-1)!);
|
||||
call.answered = false;
|
||||
delete call.answers;
|
||||
expect(pickCeoCompletionHandoff(fingerprint(call))).toBe(2);
|
||||
call.questions[0]!.options.reverse();
|
||||
expect(pickCeoCompletionHandoff(fingerprint(call))).toBe(1);
|
||||
});
|
||||
|
||||
test('a report written before the administrative handoff can reach the real plan-approval gate', () => {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'ceo-handoff-order-'));
|
||||
const file = path.join(dir, 'plan.md');
|
||||
try {
|
||||
fs.writeFileSync(file, '# Plan\n\n## GSTACK REVIEW REPORT\n\n' +
|
||||
'| Review | Runs | Status | Findings |\n|---|---|---|---|\n| CEO | 1 | COMPLETE | 3 |\n\n' +
|
||||
'VERDICT: CEO CLEARED\n\nNO UNRESOLVED DECISIONS\n');
|
||||
// Native Write succeeded at this time, before the final handoff. The
|
||||
// live inode was cleaned up; this fixture replays that observed order.
|
||||
const reportAt = Date.parse('2026-09-08T21:01:38.295Z') / 1000;
|
||||
fs.utimesSync(file, reportAt, reportAt);
|
||||
const calls = structuredClone(CAPTURED_PAIRED_RETRY_CALLS);
|
||||
const transcript = {
|
||||
status: 'ready' as const,
|
||||
calls,
|
||||
assistantMessages: [],
|
||||
planReadyRequests: [{
|
||||
sessionId: calls[0]!.sessionId,
|
||||
toolUseId: 'toolu_01XK7amzoCx4VTm1r2bHdtsH',
|
||||
timestamp: '2026-09-08T21:03:46.725Z',
|
||||
failed: false,
|
||||
}],
|
||||
};
|
||||
const admin = new Set(calls.filter(call => isCeoCompletionHandoff(fingerprint(call)))
|
||||
.map(call => `${call.sessionId}:${call.toolUseId}`));
|
||||
const startedAt = Date.parse('2026-09-08T20:51:50Z');
|
||||
expect(admin.size).toBe(1);
|
||||
expect(hasNativePlanTerminal(transcript, file, startedAt, 'plan_ready')).toBe(false);
|
||||
expect(hasNativePlanTerminal(transcript, file, startedAt, 'plan_ready', admin)).toBe(true);
|
||||
transcript.planReadyRequests[0]!.failed = true;
|
||||
expect(hasNativePlanTerminal(transcript, file, startedAt, 'plan_ready', admin)).toBe(false);
|
||||
transcript.planReadyRequests[0]!.failed = false;
|
||||
// A new substantive answer after the Write remains a freshness boundary.
|
||||
calls.splice(-1, 0, { ...structuredClone(calls[0]!), toolUseId: 'later-substantive-fix',
|
||||
answeredAt: '2026-09-08T21:03:00.000Z' });
|
||||
expect(hasNativePlanTerminal(transcript, file, startedAt, 'plan_ready', admin)).toBe(false);
|
||||
} finally {
|
||||
fs.rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,104 @@
|
||||
import { describe, expect, test } from 'bun:test';
|
||||
import { capturePlanCountQuestion, nativePlanCallFingerprint } from './helpers/claude-pty-runner';
|
||||
import { pickDesignCountOutsideVoices } from './helpers/design-count-outside';
|
||||
import type { NativePlanQuestionCall } from './helpers/plan-count-transcript';
|
||||
|
||||
const packet: NativePlanQuestionCall = {
|
||||
"sessionId": "52868766-97d8-4406-8230-4f263be36546",
|
||||
"toolUseId": "toolu_013ghrvY7M8hMrMHox3tnJLn",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D3 (Step 0D) — I've rated this plan 5/10 on design completeness. The three biggest gaps are: (1) the 5 identified implementation gaps describe the problem but not the solution, (2) no explicit state coverage table, (3) no user journey emotional arc. I'll skip mockups and review all 7 dimensions as you requested. Any specific areas to prioritize, or cover all 7 equally? <gstack-qid:plan-design-focus>",
|
||||
"header": "Focus areas",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "Cover all 7 equally (Recommended)",
|
||||
"description": "Standard review: all 7 design dimensions get full treatment. Takes longer but produces a complete plan."
|
||||
},
|
||||
{
|
||||
"label": "Focus on the 5 identified gaps first",
|
||||
"description": "Prioritize Pass 5 (Design System Alignment) to close the gap descriptions into actionable specs, then cover remaining passes more quickly."
|
||||
},
|
||||
{
|
||||
"label": "Prioritize accessibility and states",
|
||||
"description": "Focus on Pass 2 (Interaction States) and Pass 6 (Responsive/A11y), since the form has sensitive UX requirements (ARIA, contrast, keyboard)."
|
||||
}
|
||||
]
|
||||
},
|
||||
{
|
||||
"question": "D4 — Want outside design voices before the detailed review? Codex evaluates against OpenAI's design hard rules + litmus checks; a Claude subagent does an independent completeness review. (Requires Codex CLI to be installed.) <gstack-qid:outside-voices-design>",
|
||||
"header": "Outside voices",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "Yes, run outside design voices",
|
||||
"description": "Launches Codex design critique + Claude subagent completeness review in parallel before the 7 passes. Adds 1–2 minutes."
|
||||
},
|
||||
{
|
||||
"label": "No, proceed without (Recommended)",
|
||||
"description": "Skip outside voices and go straight to the 7 review passes. Faster; sufficient for most plans."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": false,
|
||||
"failed": false
|
||||
};
|
||||
|
||||
function screen(index: number, call = packet) {
|
||||
const q = call.questions[index]!;
|
||||
return '← ☐ Focus areas ☐ Outside voices ✔ Submit →\n│ ' + q.question + '\n' +
|
||||
q.options.map((option, i) => (i === 0 ? '❯' : '') + `${i + 1}. ${option.label}`).join('\n') +
|
||||
'\nEnter to select · Tab/Arrow keys to navigate · Esc to cancel\n';
|
||||
}
|
||||
|
||||
describe('Design count fixture outside-review choice', () => {
|
||||
test('the captured focus tab stays unchanged and only its outside-review tab declines', () => {
|
||||
const seen = new Set<string>();
|
||||
const focus = capturePlanCountQuestion(screen(0), seen, 0, true, packet)!;
|
||||
expect(pickDesignCountOutsideVoices(focus, focus)).toBeNull();
|
||||
const outside = capturePlanCountQuestion(screen(1), seen, 1, true, packet)!;
|
||||
expect(pickDesignCountOutsideVoices(outside, outside)).toBe(2);
|
||||
expect(capturePlanCountQuestion(screen(1), seen, 2, true, packet)).toBeNull();
|
||||
expect(pickDesignCountOutsideVoices(nativePlanCallFingerprint(packet, 0, true), focus)).toBeNull();
|
||||
});
|
||||
|
||||
test('the current opt-in question remains recognizable when native metadata arrives after the answer', () => {
|
||||
const fp = capturePlanCountQuestion(screen(1), new Set(), 0, true)!;
|
||||
expect(fp.nativeCall).toBeUndefined();
|
||||
expect(pickDesignCountOutsideVoices(fp, fp), fp.promptSnippet).toBe(2);
|
||||
const focus = capturePlanCountQuestion(screen(0), new Set(), 0, true)!;
|
||||
expect(pickDesignCountOutsideVoices(focus, focus)).toBeNull();
|
||||
});
|
||||
|
||||
test('single questions and reversed choices still select only the explicit No action', () => {
|
||||
for (const reverse of [false, true]) {
|
||||
const call = structuredClone(packet);
|
||||
call.questions = [call.questions[1]!];
|
||||
if (reverse) call.questions[0]!.options.reverse();
|
||||
const fp = nativePlanCallFingerprint(call, 0, true);
|
||||
expect(pickDesignCountOutsideVoices(fp, fp)).toBe(reverse ? 1 : 2);
|
||||
}
|
||||
});
|
||||
|
||||
test('pending packet metadata without the matching active question cannot steer a choice', () => {
|
||||
const fp = nativePlanCallFingerprint(packet, 0, true);
|
||||
expect(pickDesignCountOutsideVoices(fp, fp)).toBeNull();
|
||||
const outside = capturePlanCountQuestion(screen(1), new Set(), 0, true, packet)!;
|
||||
expect(pickDesignCountOutsideVoices(outside, { ...outside, signature: 'unrelated' })).toBeNull();
|
||||
for (const mutate of [
|
||||
(call: NativePlanQuestionCall) => { call.answered = true; },
|
||||
(call: NativePlanQuestionCall) => { call.failed = true; },
|
||||
(call: NativePlanQuestionCall) => { call.questions[1]!.multiSelect = true; },
|
||||
(call: NativePlanQuestionCall) => { call.questions[1]!.question = 'Should the product ask customers to use outside design voices?'; },
|
||||
(call: NativePlanQuestionCall) => { call.questions[1]!.question = call.questions[1]!.question.replace('outside-voices-design', 'design-review-finding'); },
|
||||
(call: NativePlanQuestionCall) => { call.questions[1]!.options[1]!.label = 'No, leave the design defect unfixed'; },
|
||||
(call: NativePlanQuestionCall) => { call.questions[1]!.options.push({ label: 'Change the design now' }); },
|
||||
]) {
|
||||
const call = structuredClone(packet);
|
||||
mutate(call);
|
||||
expect(pickDesignCountOutsideVoices(outside, { ...outside, nativeCall: call })).toBeNull();
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -9,13 +9,18 @@ function manualHandoffIndex(fp: AskUserQuestionFingerprint): number | null {
|
||||
const q = call.questions[0]!;
|
||||
if (q.multiSelect || q.options.length < 2) return null;
|
||||
const id = /<gstack-qid:\s*([a-z0-9-]+)\s*>/i.exec(q.question)?.[1]?.toLowerCase();
|
||||
if (id && id !== 'plan-ceo-next-steps') return null;
|
||||
const completion = /\bCEO\s+review\s+(?:is\s+)?(?:complete\b|cleared\b)/i.test(q.question);
|
||||
if (id !== 'plan-ceo-next-steps' && (!/^next\s+(?:review|steps?)$/i.test(q.header.trim()) || !completion)) return null;
|
||||
if (id && !/^plan-ceo-(?:review-)?next-(?:steps?|review)$/.test(id)) return null;
|
||||
const declaration = q.question.replace(/^D\s*\d+\s*[—–:-]\s*/i, '');
|
||||
const completion = /(?:^|[.!?]\s+)(?:The\s+)?CEO\s+review\s+(?:is\s+)?(?:complete\b|cleared\b)/i.test(declaration);
|
||||
const gateContext = [q.question, ...q.options.map(option => option.description ?? '')].join('\n');
|
||||
const requiredEng = /(?:\bEng(?:ineering)?\s+review|\/plan-eng-review)\b[^.!?]{0,180}\brequired(?:\s+shipping)?\s+gate\b/i.test(gateContext);
|
||||
// A qid names the menu; it cannot replace its completed-review declaration
|
||||
// or authorize another fix. The named gate can be explained in a choice.
|
||||
if (!/^next\s+(?:review|steps?)$/i.test(q.header.trim()) || !completion || !requiredEng) return null;
|
||||
|
||||
const labels = q.options.map(o => o.label.trim().replace(/^[A-Z][).]\s*/i, '').replace(/\s*\(recommended\)\s*$/i, '').trim());
|
||||
const runs = labels.map(label => /^Run\s+\/plan-(?:eng|design)-review\s+next(?:\s*\(required gate\))?$/i.test(label));
|
||||
const manual = labels.map(label => /^(?:Skip|Done)\s*[—–-]\s*I['’]ll\s+handle\s+reviews\s+manually$/i.test(label));
|
||||
const manual = labels.map(label => /^(?:Skip|Done)\s*[—–-]\s*(?:I['’]ll\s+)?handle\s+reviews\s+manually$/i.test(label));
|
||||
// Every choice must be administrative. Adding a fix/TODO/build option
|
||||
// makes the whole call substantive, even if it also offers a next review.
|
||||
if (!runs.some(Boolean) || manual.filter(Boolean).length !== 1 || !labels.every((_, i) => runs[i] || manual[i])) return null;
|
||||
|
||||
@@ -189,7 +189,11 @@ export function isTrustDialogVisible(visible: string): boolean {
|
||||
export function isPlanReadyVisible(visible: string): boolean {
|
||||
if (/ready to execute|Would you like to proceed/i.test(visible)) return true;
|
||||
const collapsed = visible.replace(/\s+/g, '');
|
||||
return /readytoexecute|Wouldyouliketoproceed/i.test(collapsed);
|
||||
if (/readytoexecute|Wouldyouliketoproceed/i.test(collapsed)) return true;
|
||||
// Claude also renders a compact ExitPlanMode approval, without a plan
|
||||
// preview. Recognize its complete active menu, not prose mentioning exit.
|
||||
// This identifies an input gate only; native/report evidence is separate.
|
||||
return /(?:^|\n)\s*Exit plan mode\?\s*\n\s*Claude wants to exit plan mode\s*\n\s*❯\s*1\.\s*Yes, and switch to default \(ask each time\) for this session\s*\n\s*2\.\s*No\s*$/i.test(visible);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -1712,11 +1716,14 @@ function isCompletedDxHandoff(call: NativePlanQuestionCall): boolean {
|
||||
!Array.isArray(call.unansweredQuestionIndices) || call.unansweredQuestionIndices.length) return false;
|
||||
const q = call.questions[0]!;
|
||||
if (q.multiSelect || !/^next\s+steps?$/i.test(q.header.trim()) ||
|
||||
!/^DX review (?:done|complete)[.!]/i.test(q.question) ||
|
||||
!/^DX review (?:done|complete)[.!]/i.test(q.question.replace(/^D\s*\d+\s*[—–:-]\s*/i, '')) ||
|
||||
!/<gstack-qid:devex-next-steps>/.test(q.question)) return false;
|
||||
const labels = q.options.map(o => o.label.trim().replace(/\s*\(recommended\)\s*$/i, ''));
|
||||
const allowed = (label: string) => /^(?:Run \/plan-eng-review|Ready to implement|Skip, handle manually)$/.test(label);
|
||||
return labels.every(allowed) && labels.includes('Run /plan-eng-review') && labels.includes('Skip, handle manually') &&
|
||||
const runEng = (label: string) => /^Run \/plan-eng-review(?: next)?$/i.test(label);
|
||||
const ready = (label: string) => /^Ready to implement(?:\s*[—–-]\s*run \/devex-review after shipping)?$/i.test(label);
|
||||
const manual = (label: string) => /^Skip(?:, handle manually|\s*[—–-]\s*I['’]ll handle next steps manually)$/i.test(label);
|
||||
return labels.every(label => runEng(label) || ready(label) || manual(label)) &&
|
||||
labels.filter(runEng).length === 1 && labels.filter(manual).length === 1 &&
|
||||
q.options.some(o => o.label === call.answers?.[q.question]);
|
||||
}
|
||||
|
||||
@@ -1830,7 +1837,16 @@ export const engSetupAUQ: Step0BoundaryPredicate = (fp) => {
|
||||
const wholePlanStatement = /(?:^|\n)(?:ELI10:\s*)?(?:this|the|whole|entire)\s+plan\s+(?:touches|spans|covers|changes|introduces|involves)\b/i.test(body);
|
||||
const wholePlanScope = (/^(?:(?:scope(?:\s+(?:challenge|reduction|complexity))?|complexity\s+check)\s*:\s*)?(?:this|the|whole|entire)\s+plan\s+(?:touches|spans|covers|changes|introduces|involves)\b/i.test(body) ||
|
||||
(scopeHeading && wholePlanStatement)) && /\b\d+\s+(?:new\s+)?(?:files|classes|services)\b/i.test(body);
|
||||
const scopeComplexity = id === 'plan-eng-scope-complexity' || id === 'plan-eng-review-scope-reduce' || wholePlanScope;
|
||||
// The native complexity gate may give the plan's file/class counts
|
||||
// directly, without spelling out "this plan touches". Keep that shape
|
||||
// tied to a Scope header and the explicit whole-scope opposed action.
|
||||
const countedComplexityGate = /^scope$/i.test(q.header.trim()) &&
|
||||
(/^complexity\s+check(?:\s+triggered)?\s*:\s*\d+\s+files\b/i.test(body) ||
|
||||
/^(?:the\s+)?plan['’]s\s+scope\b[^.!?]{0,180}\bcomplexity(?:\s+smell)?\s+check\b/i.test(body)) &&
|
||||
/\b\d+\s+files\b/i.test(body) &&
|
||||
/\b\d+\+?\s+(?:new\s+)?(?:classes|services)\b/i.test(body) &&
|
||||
actions.some(action => /^reduce\s+scope\b/i.test(action));
|
||||
const scopeComplexity = id === 'plan-eng-scope-complexity' || id === 'plan-eng-review-scope-reduce' || wholePlanScope || countedComplexityGate;
|
||||
return scopeComplexity &&
|
||||
actions.some((proceed, i) =>
|
||||
(/^proceed\s+as[- ]is\b/i.test(proceed) || /^accept\b.*\bdesign\b.*\bfocus\b.*\bquality\b/i.test(proceed)) &&
|
||||
@@ -2839,7 +2855,13 @@ export async function runPlanSkillCounting(opts: {
|
||||
let frame = terminalHint;
|
||||
// Clear unverified hints before routing active permissions and Submit.
|
||||
if (opts.expectedPlanPath && isTerminalHint && !verifiedTerminal) frame = null;
|
||||
const permission = nativeQuestionVisible ? null : filePermission(visible, session.visibleText());
|
||||
// A real approval gate is never an AUQ or a file permission. Wait for
|
||||
// its report/native evidence before input routing; verified gates still
|
||||
// pass the existing silent-write check below. A positively matched
|
||||
// native question above takes precedence over gate text.
|
||||
if (opts.expectedPlanPath && terminalHint === 'plan_ready' && !verifiedTerminal) continue;
|
||||
const permission = nativeQuestionVisible || terminalHint === 'plan_ready'
|
||||
? null : filePermission(visible, session.visibleText());
|
||||
if (frame === 'permission' || (permission === 'grant' && frame === null)) {
|
||||
if (permission !== 'handled') session.send(`${defaultPick}\r`);
|
||||
await Bun.sleep(1500);
|
||||
@@ -2905,10 +2927,6 @@ export async function runPlanSkillCounting(opts: {
|
||||
`native review completion and final report verified (step0=${step0Count}, review=${reviewCount})`, visible);
|
||||
}
|
||||
|
||||
// Never submit a plan-approval menu while its final report/native gate
|
||||
// is unverified: choosing option 1 would enter implementation.
|
||||
if (opts.expectedPlanPath && terminalHint === 'plan_ready') continue;
|
||||
|
||||
// A dismissed or repainted permission is never a native question.
|
||||
if (permission === 'handled') continue;
|
||||
|
||||
@@ -2921,10 +2939,13 @@ export async function runPlanSkillCounting(opts: {
|
||||
? nativePlanCallFingerprint(pending, fp.observedAtMs, fp.preReview) : fp;
|
||||
const prerequisitePick = planCountPrerequisitePick(routing);
|
||||
// Native tool records may flush only after the answer. Let a guarded
|
||||
// caller recognize that visible menu, but never override a known packet.
|
||||
// caller recognize that visible menu. A known packet needs a positively
|
||||
// matched active tab before a caller can change that tab's choice.
|
||||
// The captured fingerprint alone proves whether native metadata matched
|
||||
// this active UI; an unrelated pending record is not a routing identity.
|
||||
const callerPick = !pending || pending.questions.length === 1 ? opts.pickAUQ?.(routing, fp) ?? null : null;
|
||||
const boundNativeTab = pending && fp.nativeCall === pending && fp.nativeQuestionIndex !== undefined;
|
||||
const callerPick = !pending || pending.questions.length === 1 || boundNativeTab
|
||||
? opts.pickAUQ?.(routing, fp) ?? null : null;
|
||||
const pickIdx = prerequisitePick ?? callerPick ??
|
||||
(isFirstAUQ && opts.firstAUQPick ? opts.firstAUQPick(routing) : defaultPick);
|
||||
isFirstAUQ = false;
|
||||
|
||||
@@ -2683,3 +2683,508 @@ describe('native question identity outranks permission wording', () => {
|
||||
expect(capturePlanCountQuestion(frame, new Set(), 0, false)).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
describe('native file/class complexity gate classification', () => {
|
||||
const rows = [
|
||||
{
|
||||
"id": "toolu_01HJHA5nCKyCAEPRVnm84udk",
|
||||
"header": "Prerequisite",
|
||||
"question": "D1 — No design doc found for this branch. Run /office-hours first? <gstack-qid:plan-eng-review-office-hours-prereq>",
|
||||
"labels": [
|
||||
"Skip — proceed with review (Recommended)",
|
||||
"Run /office-hours first"
|
||||
],
|
||||
"answer": "Skip — proceed with review (Recommended)"
|
||||
},
|
||||
{
|
||||
"id": "toolu_01BF2LE9pqkt3Cs8PxC1uPaA",
|
||||
"header": "Scope",
|
||||
"question": "D2 — Complexity check triggered: 12 files, 4+ new classes. Reduce scope or proceed as-is? <gstack-qid:plan-eng-review-complexity-check>",
|
||||
"labels": [
|
||||
"Proceed as-is",
|
||||
"Reduce scope (Recommended)"
|
||||
],
|
||||
"answer": "Proceed as-is"
|
||||
},
|
||||
{
|
||||
"id": "toolu_01J9SZHLFAuHjn19o9XFJvHo",
|
||||
"header": "Learnings",
|
||||
"question": "D3 — Cross-project learnings: search past sessions from other projects on this machine? <gstack-qid:plan-eng-review-cross-project-learnings>",
|
||||
"labels": [
|
||||
"Enable cross-project (Recommended)",
|
||||
"Project-scoped only"
|
||||
],
|
||||
"answer": "Enable cross-project (Recommended)"
|
||||
},
|
||||
{
|
||||
"id": "toolu_01CRbNCM44doMT1ubMAtcuBp",
|
||||
"header": "Architecture",
|
||||
"question": "D4 — Architecture A1: Two services mutate a shared AuthCache with no mutation serialization. How should this be resolved? <gstack-qid:plan-eng-review-arch-shared-mutable-cache>",
|
||||
"labels": [
|
||||
"Single write-coordinator (Recommended)",
|
||||
"Serialize via mutex/queue",
|
||||
"Accept the risk, add monitoring"
|
||||
],
|
||||
"answer": "Single write-coordinator (Recommended)"
|
||||
},
|
||||
{
|
||||
"id": "toolu_01UwpB3rC52W99ZB6g6FMV6K",
|
||||
"header": "Architecture",
|
||||
"question": "D5 — Architecture A2: legacyAuthFlow() gets rewritten with no regression tests capturing prior behavior. Approach? <gstack-qid:plan-eng-review-arch-legacy-rewrite>",
|
||||
"labels": [
|
||||
"Regression tests first (Recommended)",
|
||||
"Strangler fig",
|
||||
"Big-bang rewrite as planned"
|
||||
],
|
||||
"answer": "Regression tests first (Recommended)"
|
||||
},
|
||||
{
|
||||
"id": "toolu_011biwzYyT1qCzVL8rn8gmTe",
|
||||
"header": "Code Quality",
|
||||
"question": "D6 — Code Quality CQ1: validateAndDispatch() has 3 nested try/catch blocks that each swallow a different error class. Fix? <gstack-qid:plan-eng-review-cq-validate-dispatch>",
|
||||
"labels": [
|
||||
"Decompose + surface errors (Recommended)",
|
||||
"Flatten catch hierarchy only",
|
||||
"Leave as-is"
|
||||
],
|
||||
"answer": "Decompose + surface errors (Recommended)"
|
||||
},
|
||||
{
|
||||
"id": "toolu_01HbUJX6fD6CN3obeoyh8S5Z",
|
||||
"header": "Code Quality",
|
||||
"question": "D7 — Code Quality CQ2: AuthCache shared via module-level export (implicit global). Switch to dependency injection? <gstack-qid:plan-eng-review-cq-module-export>",
|
||||
"labels": [
|
||||
"Dependency injection (Recommended)",
|
||||
"Keep module export"
|
||||
],
|
||||
"answer": "Dependency injection (Recommended)"
|
||||
},
|
||||
{
|
||||
"id": "toolu_019jNc2fhvrJhHkNZKTdztUm",
|
||||
"header": "Tests",
|
||||
"question": "D8 — Tests T1: No described test for coordinator ordering under concurrent mutations. Add concurrency tests to the plan? <gstack-qid:plan-eng-review-test-concurrency>",
|
||||
"labels": [
|
||||
"Add concurrency tests (Recommended)",
|
||||
"Defer to code review"
|
||||
],
|
||||
"answer": "Add concurrency tests (Recommended)"
|
||||
},
|
||||
{
|
||||
"id": "toolu_0132EDXRcCffruHaV1E4kKRH",
|
||||
"header": "Tests",
|
||||
"question": "D9 — Tests T2: No E2E tests planned for core auth flows (cross-tenant validation, suspension, logout, revocation). Add them? <gstack-qid:plan-eng-review-test-e2e-auth>",
|
||||
"labels": [
|
||||
"Add E2E tests for auth flows (Recommended)",
|
||||
"Unit/integration only as planned"
|
||||
],
|
||||
"answer": "Add E2E tests for auth flows (Recommended)"
|
||||
},
|
||||
{
|
||||
"id": "toolu_01C2sC9k9YbvAvgyWHGZUTyb",
|
||||
"header": "Performance",
|
||||
"question": "D10 — Performance P1: 5 sequential IDP calls per token validation. Parallelize or eliminate? <gstack-qid:plan-eng-review-perf-idp-calls>",
|
||||
"labels": [
|
||||
"Local JWT validation (Recommended)",
|
||||
"Promise.all parallelization",
|
||||
"Leave sequential as-is"
|
||||
],
|
||||
"answer": "Local JWT validation (Recommended)"
|
||||
}
|
||||
];
|
||||
|
||||
function callAt(index: number) {
|
||||
const row = rows[index]!;
|
||||
return {
|
||||
sessionId: '122ddb02-2346-4a38-9824-f04f9d5d8cae',
|
||||
toolUseId: row.id,
|
||||
answered: true,
|
||||
failed: false,
|
||||
questions: [{
|
||||
header: row.header,
|
||||
question: row.question,
|
||||
options: row.labels.map(label => ({ label })),
|
||||
multiSelect: false,
|
||||
}],
|
||||
answers: { [row.question]: row.answer },
|
||||
unansweredQuestionIndices: [],
|
||||
};
|
||||
}
|
||||
|
||||
test('actual ten calls remain three setup and seven independent review decisions in either setup order', () => {
|
||||
for (const order of [[0, 1, 2], [0, 2, 1]]) {
|
||||
let started = false;
|
||||
const phases = [...order, 3, 4, 5, 6, 7, 8, 9].map(index => {
|
||||
const phase = planCountQuestionPhase(nativePlanCallFingerprint(callAt(index), 0, !started),
|
||||
started, engStep0Boundary, engFirstReviewAUQ, engSetupAUQ);
|
||||
started = phase.reviewStarted;
|
||||
return phase;
|
||||
});
|
||||
expect(phases.map(p => p.preReview)).toEqual([true, true, true, false, false, false, false, false, false, false]);
|
||||
expect(phases.filter(p => !p.preReview)).toHaveLength(7);
|
||||
}
|
||||
});
|
||||
|
||||
test('the answered numeric complexity gate does not depend on qid or action order', () => {
|
||||
for (const qid of ['plan-eng-review-complexity-check', 'model-chosen-size-gate']) {
|
||||
const call = callAt(1);
|
||||
const q = call.questions[0]!;
|
||||
q.question = q.question.replace('plan-eng-review-complexity-check', qid);
|
||||
for (const reverse of [false, true]) {
|
||||
if (reverse) q.options.reverse();
|
||||
call.answers = { [q.question]: q.options[0]!.label };
|
||||
expect(engSetupAUQ(nativePlanCallFingerprint(call, 0, false))).toBe(true);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
test('a component finding, TODO, missing count, or missing whole-scope opposition stays substantive', () => {
|
||||
for (const mutate of [
|
||||
(call: ReturnType<typeof callAt>) => { call.questions[0]!.header = 'Architecture issue'; },
|
||||
(call: ReturnType<typeof callAt>) => { call.questions[0]!.header = 'TODO: scope'; },
|
||||
(call: ReturnType<typeof callAt>) => { call.questions[0]!.question = call.questions[0]!.question.replace('Complexity check triggered:', 'AuthCache complexity issue:'); },
|
||||
(call: ReturnType<typeof callAt>) => { call.questions[0]!.question = call.questions[0]!.question.replace('12 files, 4+ new classes', '12 cache entries'); },
|
||||
(call: ReturnType<typeof callAt>) => { call.questions[0]!.options[1]!.label = 'Reduce cache lock scope'; },
|
||||
(call: ReturnType<typeof callAt>) => { call.questions[0]!.options[0]!.label = 'Investigate the cache'; },
|
||||
]) {
|
||||
const call = callAt(1);
|
||||
mutate(call);
|
||||
call.answers = { [call.questions[0]!.question]: call.questions[0]!.options[0]!.label };
|
||||
expect(engSetupAUQ(nativePlanCallFingerprint(call, 0, false))).toBe(false);
|
||||
}
|
||||
});
|
||||
|
||||
test('failed, pending, unoffered and mixed answered calls cannot be discarded as setup', () => {
|
||||
for (const mutate of [
|
||||
(call: ReturnType<typeof callAt>) => { call.answered = false; },
|
||||
(call: ReturnType<typeof callAt>) => { call.failed = true; },
|
||||
(call: ReturnType<typeof callAt>) => { call.answers = {}; },
|
||||
(call: ReturnType<typeof callAt>) => { call.answers = { [call.questions[0]!.question]: 'Add a new cache repair instead' }; },
|
||||
(call: ReturnType<typeof callAt>) => {
|
||||
const issue = callAt(3);
|
||||
call.questions.push(issue.questions[0]!);
|
||||
Object.assign(call.answers, issue.answers);
|
||||
},
|
||||
]) {
|
||||
const call = callAt(1);
|
||||
mutate(call);
|
||||
expect(engSetupAUQ(nativePlanCallFingerprint(call, 0, false))).toBe(false);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
describe('explicit whole-plan scope complexity premise', () => {
|
||||
const calls = [
|
||||
{
|
||||
"sessionId": "3326e31d-a9e2-41ea-9489-f99013a81de8",
|
||||
"toolUseId": "toolu_01RY4Ec4BVKuX7JRycy5Y7zB",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D1 — The plan's scope (12 files, 4 new classes) triggers the complexity smell check. Proceed as-is or reduce scope first? <gstack-qid:plan-eng-complexity-check>",
|
||||
"header": "Scope",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "Proceed as-is (recommended)",
|
||||
"description": "Review all 4 planned classes (TokenStore, SessionMint, AuthCache, RequestPolicy) across 12 files. The plan already flags this as a smell worth investigating — we'll scrutinize every class boundary during the Architecture section and call out anything that should be merged or deferred. Completeness: 10/10"
|
||||
},
|
||||
{
|
||||
"label": "Reduce scope first",
|
||||
"description": "Propose a minimal-surface version before reviewing: collapse TokenStore into the existing cache adapter extension and defer RequestPolicy extraction (it's just a slice of validateAndDispatch()). This yields ~2 new classes and ~6-8 files. The Architecture section would then review the leaner design. Completeness: 7/10 — defers policy extraction"
|
||||
},
|
||||
{
|
||||
"label": "Skip complexity and go straight to issues",
|
||||
"description": "Treat scope as fixed, skip the scope challenge, and dive directly into the 4 review sections (Architecture, Code Quality, Tests, Performance). Fastest path but misses the structural smell the plan itself flagged. Completeness: 7/10"
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D1 — The plan's scope (12 files, 4 new classes) triggers the complexity smell check. Proceed as-is or reduce scope first? <gstack-qid:plan-eng-complexity-check>": "Proceed as-is (recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-08T20:58:36.032Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "3326e31d-a9e2-41ea-9489-f99013a81de8",
|
||||
"toolUseId": "toolu_01Vz57KiddxkZg3v4NiWKzsC",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D2 — Architecture: shared global mutable AuthCache between AuthBroker and SessionMint, with no serialization, in a multi-tenant system. <gstack-qid:plan-eng-arch-shared-cache>",
|
||||
"header": "Architecture",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "Inject via constructor (recommended)",
|
||||
"description": "[P1] (confidence: 9/10) Plan says: 'Two new services share a global mutable AuthCache instance via module-level export. Both services mutate it' and 'they do not serialize mutations.' In a multi-tenant system, concurrent mutations from AuthBroker (e.g., evict-on-logout) and SessionMint (e.g., write-on-mint) to the same cache can corrupt tenant isolation boundaries without any test catching it. Fix: pass AuthCache as a constructor argument to both services. Each test provides a fresh instance; production wires it once at startup. Zero mutations to the existing adapter. Effort: human ~1h / CC ~5min. Completeness: 10/10"
|
||||
},
|
||||
{
|
||||
"label": "Add a mutation coordinator",
|
||||
"description": "Keep the module-level export but wrap all mutating calls in a queue or lock. Protects against races but does not fix the coupling (tests still share state, import side-effects are still global). Higher complexity than DI. Effort: human ~2h / CC ~10min. Completeness: 8/10 — misses the testability problem"
|
||||
},
|
||||
{
|
||||
"label": "Accept as-is",
|
||||
"description": "Trust the underlying adapter's existing invalidation hooks to prevent cross-tenant reads. This works only if the adapter already serializes all writes — the plan does not state this, and 'they do not serialize mutations' explicitly says it does not. Accepted risk of cross-tenant cache corruption on concurrent requests. Completeness: 5/10"
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D2 — Architecture: shared global mutable AuthCache between AuthBroker and SessionMint, with no serialization, in a multi-tenant system. <gstack-qid:plan-eng-arch-shared-cache>": "Inject via constructor (recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-08T20:59:24.191Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "3326e31d-a9e2-41ea-9489-f99013a81de8",
|
||||
"toolUseId": "toolu_01MpiyqoPfnhoFmN1hBtNHdu",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D3 — Architecture: the plan describes 5 IDP API calls for token validation but says nothing about partial failure semantics. <gstack-qid:plan-eng-arch-idp-partial-failure>",
|
||||
"header": "Architecture",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "Add explicit failure semantics to the plan (recommended)",
|
||||
"description": "[P1] (confidence: 9/10) Plan: 'Token validation issues 5 sequential API calls to the IDP.' No text covers what happens when call 3 of 5 returns a 503. In an auth system, the fail-open vs fail-closed decision is load-bearing for security. Recommendation: add a plan section stating the rule explicitly — 'on any IDP call failure, token validation fails closed (reject the token, return 401, do not cache a partial result).' Effort: human ~30min / CC ~3min. Completeness: 10/10"
|
||||
},
|
||||
{
|
||||
"label": "Defer to implementation",
|
||||
"description": "Leave the failure semantics undecided in the plan; let implementers decide per-call. Risk: two developers make different assumptions and one path fails open (accepts a partially-validated token). In auth systems, a silent fail-open is a security hole, not just a bug. Completeness: 5/10"
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D3 — Architecture: the plan describes 5 IDP API calls for token validation but says nothing about partial failure semantics. <gstack-qid:plan-eng-arch-idp-partial-failure>": "Add explicit failure semantics to the plan (recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-08T20:59:52.277Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "3326e31d-a9e2-41ea-9489-f99013a81de8",
|
||||
"toolUseId": "toolu_019eGcLXXteu52dC67WNtJyL",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D4 — Architecture: TokenStore is listed as one of 4 new classes but the plan never describes its responsibility. The existing cache adapter already stores tokens keyed by tenant/issuer/audience/policy. <gstack-qid:plan-eng-arch-tokenstore-purpose>",
|
||||
"header": "Architecture",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "Define TokenStore's boundary in the plan (recommended)",
|
||||
"description": "[P2] (confidence: 8/10) Without a description, implementers may duplicate the existing adapter's logic inside TokenStore, producing two storage layers for the same data. The plan should state: what TokenStore does that the existing adapter does not, whether it holds in-memory tokens separately from the cache adapter, and how AuthCache (the facade) relates to TokenStore vs the adapter. Effort: human ~20min / CC ~2min. Completeness: 10/10"
|
||||
},
|
||||
{
|
||||
"label": "Keep it implicit",
|
||||
"description": "Accept that TokenStore's boundary will be defined during implementation. Risk: two interpretations surface mid-sprint — one where TokenStore wraps the adapter (double indirection) and one where it holds state independently (split storage, double-write bugs). Completeness: 5/10"
|
||||
},
|
||||
{
|
||||
"label": "Merge TokenStore into AuthCache",
|
||||
"description": "If TokenStore is just a typed wrapper around what AuthCache already exposes, drop it and let AuthCache handle all token storage concerns. Reduces new-class count from 4 to 3 without losing capability. Completeness: 9/10 — assumes they genuinely overlap"
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D4 — Architecture: TokenStore is listed as one of 4 new classes but the plan never describes its responsibility. The existing cache adapter already stores tokens keyed by tenant/issuer/audience/policy. <gstack-qid:plan-eng-arch-tokenstore-purpose>": "Define TokenStore's boundary in the plan (recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-08T21:00:16.358Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "3326e31d-a9e2-41ea-9489-f99013a81de8",
|
||||
"toolUseId": "toolu_01WXRQF8z4JKkKvnwH7h6U6Z",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D5 — Code Quality: validateAndDispatch() is 60 lines with 3 nested try/catch blocks, each swallowing a different error class. <gstack-qid:plan-eng-cq-validate-dispatch>",
|
||||
"header": "Code Quality",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "Refactor as part of this PR (recommended)",
|
||||
"description": "[P1] (confidence: 9/10) Plan: 'each catch swallows a different error class.' In auth code, a swallowed error is a silent failure — a rejection that should surface as a 401 becomes invisible, or a partial validation looks like success. Since this PR already touches the function (RequestPolicy extraction), refactoring it is a same-diff change. Fix: extract each catch branch into a named handler, propagate errors explicitly via throw or typed Result<T,E>; add one log line per catch so failures appear in traces. Effort: human ~2h / CC ~10min. Completeness: 10/10"
|
||||
},
|
||||
{
|
||||
"label": "Add logging, keep structure",
|
||||
"description": "Add a structured log at each catch site so failures are at least visible, but leave the nesting and swallowing in place. Reduces debuggability debt without restructuring. Still leaves the logical errors silently absorbed — a 401 that should have been thrown may still become a phantom pass. Effort: human ~30min / CC ~5min. Completeness: 7/10 — errors are visible but not propagated"
|
||||
},
|
||||
{
|
||||
"label": "Defer to follow-up",
|
||||
"description": "Capture as a TODO and address in a later PR. Risk: the refactor grows harder once 4 new classes depend on the current swallowing behavior. The 'right behavior' becomes ambiguous when callers have already been written against the current (broken) semantics. Completeness: 3/10"
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D5 — Code Quality: validateAndDispatch() is 60 lines with 3 nested try/catch blocks, each swallowing a different error class. <gstack-qid:plan-eng-cq-validate-dispatch>": "Refactor as part of this PR (recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-08T21:00:40.446Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "3326e31d-a9e2-41ea-9489-f99013a81de8",
|
||||
"toolUseId": "toolu_01UwMPiH1hZutbzPCGGQ3sUZ",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D6 — Tests: IDP partial failure (calls 1-4 of 5 succeed, call 5 fails) has no test coverage in the plan. We just added the fail-closed semantic in D3. <gstack-qid:plan-eng-test-idp-partial>",
|
||||
"header": "Tests",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "Add to plan as required test (recommended)",
|
||||
"description": "[P1] (confidence: 9/10) We added 'fail closed on any IDP error' as explicit plan language in D3. That semantic needs a test that stubs each IDP call position as the one that fails (5 separate test cases, or one parametrized one) and asserts the validator returns 401 and writes nothing to cache. Without this, the fail-closed rule is a comment in a doc, not a contract in code. Effort: human ~1h / CC ~8min. Completeness: 10/10"
|
||||
},
|
||||
{
|
||||
"label": "Cover only all-pass and all-fail",
|
||||
"description": "Test the two extreme cases (all 5 calls succeed, all fail) and skip partial-failure positions. Simpler, but misses the case where calls 1-4 succeeded and call 5 fails — the most likely real-world scenario when an IDP endpoint degrades under load. Completeness: 6/10"
|
||||
},
|
||||
{
|
||||
"label": "Defer",
|
||||
"description": "Capture as TODO and address post-merge. Risk: the fail-closed rule we just added has no verification path; a future refactor could silently revert it and no test would catch it. Completeness: 3/10"
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D6 — Tests: IDP partial failure (calls 1-4 of 5 succeed, call 5 fails) has no test coverage in the plan. We just added the fail-closed semantic in D3. <gstack-qid:plan-eng-test-idp-partial>": "Add to plan as required test (recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-08T21:02:00.740Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "3326e31d-a9e2-41ea-9489-f99013a81de8",
|
||||
"toolUseId": "toolu_0152qGvSSuBnRkqTe8ix2WTj",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D7 — Tests: the plan says coverage for 'new components and their success/error paths' is planned, but validateAndDispatch() is an existing function being refactored. Its 3 catch blocks that currently swallow errors need explicit tests for each error branch. <gstack-qid:plan-eng-test-validate-dispatch-errors>",
|
||||
"header": "Tests",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "Add 3 explicit error-path tests to the plan (recommended)",
|
||||
"description": "[P2] (confidence: 8/10) After the refactor from D5, each of the 3 catch blocks becomes a named handler. Each named handler needs a test: trigger error class A/B/C, assert the function returns the expected error response (not silently succeeds). Without these tests, the refactor from D5 has no verification that the new explicit error handling is correct. Effort: human ~1h / CC ~8min. Completeness: 10/10"
|
||||
},
|
||||
{
|
||||
"label": "Cover in integration tests only",
|
||||
"description": "Rely on integration tests that exercise validateAndDispatch() indirectly through AuthBroker. Lower signal — integration tests that cover error paths often don't isolate which branch triggered, making regressions hard to pinpoint. Completeness: 6/10"
|
||||
},
|
||||
{
|
||||
"label": "Defer",
|
||||
"description": "Leave error-path test coverage for a follow-up. Risk: the refactored function's error behavior is unverified until the next sprint. Completeness: 3/10"
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D7 — Tests: the plan says coverage for 'new components and their success/error paths' is planned, but validateAndDispatch() is an existing function being refactored. Its 3 catch blocks that currently swallow errors need explicit tests for each error branch. <gstack-qid:plan-eng-test-validate-dispatch-errors>": "Add 3 explicit error-path tests to the plan (recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-08T21:02:10.780Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "3326e31d-a9e2-41ea-9489-f99013a81de8",
|
||||
"toolUseId": "toolu_0194VcsnAMMDRFEafkx4dEsQ",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D8 — Performance: token validation makes 5 sequential IDP API calls that the plan itself identifies as trivially parallelizable. <gstack-qid:plan-eng-perf-idp-parallel>",
|
||||
"header": "Performance",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "Parallelize as part of this PR (recommended)",
|
||||
"description": "[P2] (confidence: 9/10) Plan: 'Token validation issues 5 sequential API calls to the IDP; they could be parallelized via Promise.all trivially (calls are independent).' The plan identifies the fix itself. In a multi-tenant system, token validation is on the hot path for every authenticated request. Sequential IDP calls multiply latency by 5x unnecessarily. Fix: wrap the 5 calls in Promise.all(); the refactor from D5 (validateAndDispatch extraction) is the natural point to do this. Effort: human ~30min / CC ~5min. Completeness: 10/10"
|
||||
},
|
||||
{
|
||||
"label": "Defer to performance pass",
|
||||
"description": "Merge first, profile in production, then optimize. Risk: the plan already identifies this as trivial to fix. Deferring a known, safe, low-effort improvement costs real latency on every auth request from day 1 of deployment. Completeness: 6/10"
|
||||
},
|
||||
{
|
||||
"label": "Add to TODOS.md only",
|
||||
"description": "Capture in TODOS.md and address when performance metrics confirm it matters. Lowest risk of introducing a bug, but leaves 5x unnecessary serial latency in prod indefinitely. Completeness: 4/10"
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D8 — Performance: token validation makes 5 sequential IDP API calls that the plan itself identifies as trivially parallelizable. <gstack-qid:plan-eng-perf-idp-parallel>": "Parallelize as part of this PR (recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-08T21:02:26.856Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "3326e31d-a9e2-41ea-9489-f99013a81de8",
|
||||
"toolUseId": "toolu_01AHbQGxj8dc6F4MPYhsPo2q",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D9 — TODOS: the plan has no mention of IDP circuit breaker or timeout per call. With 5 calls now running in parallel (D8 decision), an IDP outage generates 5 concurrent timeouts per request. <gstack-qid:plan-eng-todo-idp-circuit-breaker>",
|
||||
"header": "TODO",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "Add to TODOS.md (recommended)",
|
||||
"description": "Capture as a deferred item: add per-call timeout + a circuit breaker so that IDP degradation fails fast instead of hanging. Not blocking this PR, but the parallelization we added in D8 increases the concurrent timeout surface. Concrete TODO: implement exponential backoff + circuit breaker with configurable open/half-open thresholds."
|
||||
},
|
||||
{
|
||||
"label": "Skip — not valuable enough",
|
||||
"description": "Accept that IDP timeout handling is the IDP library's responsibility, or that network timeouts are set at the HTTP client level. No additional app-level circuit breaker needed."
|
||||
},
|
||||
{
|
||||
"label": "Build it now in this PR",
|
||||
"description": "Add circuit breaker logic as part of the validateAndDispatch refactor (D5). The refactor is already touching that function; adding a circuit breaker is incremental. Effort: human ~4h / CC ~20min. Higher complexity, but IDP outages in auth systems have severe user impact."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D9 — TODOS: the plan has no mention of IDP circuit breaker or timeout per call. With 5 calls now running in parallel (D8 decision), an IDP outage generates 5 concurrent timeouts per request. <gstack-qid:plan-eng-todo-idp-circuit-breaker>": "Add to TODOS.md (recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-08T21:03:23.081Z"
|
||||
}
|
||||
];
|
||||
|
||||
test('captured retry counts its scope as setup but keeps all eight substantive decisions above the original ceiling', () => {
|
||||
let started = false;
|
||||
const phases = calls.map(call => {
|
||||
const phase = planCountQuestionPhase(nativePlanCallFingerprint(call, 0, !started),
|
||||
started, engStep0Boundary, engFirstReviewAUQ, engSetupAUQ);
|
||||
started = phase.reviewStarted;
|
||||
return phase;
|
||||
});
|
||||
expect(phases.map(p => p.preReview)).toEqual([true, false, false, false, false, false, false, false, false]);
|
||||
expect(phases.filter(p => !p.preReview)).toHaveLength(8);
|
||||
});
|
||||
|
||||
test('an individual component, missing numeric premise, or a finding cannot borrow a scope label', () => {
|
||||
for (const mutate of [
|
||||
(call: typeof calls[number]) => { call.questions[0]!.question = call.questions[0]!.question.replace("The plan's scope", "The cache's scope"); },
|
||||
(call: typeof calls[number]) => { call.questions[0]!.question = call.questions[0]!.question.replace('12 files, 4 new classes', '12 cached tokens'); },
|
||||
(call: typeof calls[number]) => { call.questions[0]!.question = call.questions[0]!.question.replace('triggers the complexity smell check', 'has a race in AuthCache'); },
|
||||
(call: typeof calls[number]) => { call.questions[0]!.header = 'Architecture finding'; },
|
||||
]) {
|
||||
const call = structuredClone(calls[0]!);
|
||||
mutate(call);
|
||||
call.answers = { [call.questions[0]!.question]: call.questions[0]!.options[0]!.label };
|
||||
expect(engSetupAUQ(nativePlanCallFingerprint(call, 0, false))).toBe(false);
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,42 @@
|
||||
import type { AskUserQuestionFingerprint } from './claude-pty-runner';
|
||||
|
||||
/** Seeded-count fixtures cover native review cadence; outside voices have separate evals. */
|
||||
export function pickDesignCountOutsideVoices(
|
||||
_routing: AskUserQuestionFingerprint,
|
||||
active: AskUserQuestionFingerprint,
|
||||
): number | null {
|
||||
const call = active.nativeCall;
|
||||
let question: string;
|
||||
let labels: string[];
|
||||
if (call) {
|
||||
if (call.answered || call.failed) return null;
|
||||
const index = active.nativeQuestionIndex ?? (call.questions.length === 1 ? 0 : undefined);
|
||||
if (index === undefined || !Number.isInteger(index) || index < 0 || index >= call.questions.length) return null;
|
||||
const identity = `${call.sessionId}:${call.toolUseId}` +
|
||||
(call.questions.length > 1 ? `:question:${index}` : '');
|
||||
if (active.signature !== identity) return null;
|
||||
const q = call.questions[index]!;
|
||||
if (q.multiSelect || !/^outside(?: design)? voices$/i.test(q.header.trim()) ||
|
||||
!/<gstack-qid:outside-voices-design>/.test(q.question)) return null;
|
||||
question = q.question;
|
||||
labels = q.options.map(option => option.label);
|
||||
} else {
|
||||
// Native JSONL can arrive after the answer. The caller supplies the
|
||||
// active viewport fingerprint; a known but unmatched packet is blocked
|
||||
// before this hook. Require the specific opt-in premise and both actions.
|
||||
question = active.promptSnippet;
|
||||
const packetBar = /^←[^→]*[☐☒]\s+Outside voices\b[^→]*✔\s*Submit\s*→\s*[│┃]?\s*/i.exec(question);
|
||||
if (packetBar) question = question.slice(packetBar[0].length);
|
||||
else if (!/^(?:[☐□]\s*)?outside(?: design)? voices\b/i.test(question)) return null;
|
||||
labels = active.options.map(option => option.label);
|
||||
while (labels.length > 2 && /^(?:Type something\.?|Chat about this)$/i.test(labels.at(-1)!.trim())) labels.pop();
|
||||
}
|
||||
if (!/\b(?:want|run|include|enable)\b[^?]{0,90}\boutside design voices\b/i.test(question) ||
|
||||
!/\b(?:before|for)\s+(?:the\s+)?(?:detailed\s+)?(?:design\s+)?review\b/i.test(question)) return null;
|
||||
labels = labels.map(label => label.trim().replace(/\s*\(recommended\)\s*$/i, ''));
|
||||
if (labels.length !== 2) return null;
|
||||
const yes = labels.map(label => /^Yes,?\s+run outside design voices$/i.test(label));
|
||||
const no = labels.map(label => /^No,?\s+proceed without$/i.test(label));
|
||||
if (yes.filter(Boolean).length !== 1 || no.filter(Boolean).length !== 1) return null;
|
||||
return no.findIndex(Boolean) + 1;
|
||||
}
|
||||
@@ -4,6 +4,34 @@ import * as path from 'node:path';
|
||||
import { spawnSync } from 'node:child_process';
|
||||
import { getHermeticDirs } from './hermetic-env';
|
||||
|
||||
/** Disposable config for evals that explicitly cover native review only. */
|
||||
export function createNativeReviewState(): {
|
||||
env: Record<string, string>;
|
||||
cleanup(): void;
|
||||
} {
|
||||
const sharedState = getHermeticDirs().gstackHome;
|
||||
const stateRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-native-review-state-'));
|
||||
const cleanup = () => fs.rmSync(stateRoot, { recursive: true, force: true });
|
||||
try {
|
||||
for (const entry of fs.readdirSync(sharedState, { withFileTypes: true })) {
|
||||
// Keep onboarding seeds; never copy sibling review logs/artifacts.
|
||||
if (entry.isFile() && (entry.name === '.activated' ||
|
||||
/^\..*(?:-seen|-prompted|-shown)$/.test(entry.name) ||
|
||||
entry.name.startsWith('.feature-prompted-'))) {
|
||||
fs.copyFileSync(path.join(sharedState, entry.name), path.join(stateRoot, entry.name));
|
||||
}
|
||||
}
|
||||
const config = fs.readFileSync(path.join(sharedState, 'config.yaml'), 'utf8')
|
||||
.replace(/^codex_reviews:.*(?:\r?\n|$)/gm, '');
|
||||
fs.writeFileSync(path.join(stateRoot, 'config.yaml'), config + '\ncodex_reviews: disabled\n');
|
||||
// Readers and onboarding writers must agree on the owned state.
|
||||
return { env: { GSTACK_HOME: stateRoot, GSTACK_STATE_ROOT: stateRoot }, cleanup };
|
||||
} catch (error) {
|
||||
cleanup();
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Count evals review a seeded plan, never the checkout that supplies skills.
|
||||
* Put the complete request in Claude's initial project context before the
|
||||
@@ -16,13 +44,13 @@ export function createPlanCountFixture(prompt: string, opts: { nativeReviewOnly?
|
||||
cleanup(): void;
|
||||
} {
|
||||
const cwd = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-plan-count-'));
|
||||
let stateRoot: string | undefined;
|
||||
let nativeState: ReturnType<typeof createNativeReviewState> | undefined;
|
||||
const env: Record<string, string> = {};
|
||||
const cleanup = () => {
|
||||
try {
|
||||
fs.rmSync(cwd, { recursive: true, force: true });
|
||||
} finally {
|
||||
if (stateRoot) fs.rmSync(stateRoot, { recursive: true, force: true });
|
||||
nativeState?.cleanup();
|
||||
}
|
||||
};
|
||||
try {
|
||||
@@ -38,28 +66,9 @@ export function createPlanCountFixture(prompt: string, opts: { nativeReviewOnly?
|
||||
}
|
||||
}
|
||||
if (opts.nativeReviewOnly) {
|
||||
// Seeded-N count bands measure the main review's finding cadence.
|
||||
// An outside review can legitimately add findings beyond that band;
|
||||
// these fixtures do not cover its separate approval-question cadence.
|
||||
// Opt in only from runPlanSkillCounting: mode fixtures keep defaults.
|
||||
const sharedState = getHermeticDirs().gstackHome;
|
||||
stateRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-plan-count-state-'));
|
||||
for (const entry of fs.readdirSync(sharedState, { withFileTypes: true })) {
|
||||
// Preserve onboarding seeds, without copying sibling review logs or
|
||||
// artifacts from the per-process shared hermetic state directory.
|
||||
if (entry.isFile() && (entry.name === '.activated' ||
|
||||
/^\..*(?:-seen|-prompted|-shown)$/.test(entry.name) ||
|
||||
entry.name.startsWith('.feature-prompted-'))) {
|
||||
fs.copyFileSync(path.join(sharedState, entry.name), path.join(stateRoot, entry.name));
|
||||
}
|
||||
}
|
||||
const config = fs.readFileSync(path.join(sharedState, 'config.yaml'), 'utf8')
|
||||
.replace(/^codex_reviews:.*(?:\r?\n|$)/gm, '');
|
||||
fs.writeFileSync(path.join(stateRoot, 'config.yaml'), config + '\ncodex_reviews: disabled\n');
|
||||
// Config readers prefer STATE_ROOT, while onboarding writers use HOME.
|
||||
// Both must resolve to owned state, even with explicit caller overrides.
|
||||
env.GSTACK_HOME = stateRoot;
|
||||
env.GSTACK_STATE_ROOT = stateRoot;
|
||||
// Seeded-N bands cover native finding cadence; mode fixtures keep defaults.
|
||||
nativeState = createNativeReviewState();
|
||||
Object.assign(env, nativeState.env);
|
||||
}
|
||||
fs.writeFileSync(path.join(cwd, 'PLAN.md'), prompt);
|
||||
fs.writeFileSync(path.join(cwd, 'CLAUDE.md'), [
|
||||
|
||||
@@ -154,7 +154,7 @@ export const E2E_TOUCHFILES: Record<string, string[]> = {
|
||||
// the selector sets GSTACK_CARVE_SKILL=<name> to scope cost to the changed
|
||||
// skill (D-CODEX A). Touching the registry/helper or sections.ts runs all.
|
||||
'carve-section-loading': ['scripts/resolvers/composition.ts', 'test/autoplan-review-discovery.test.ts', 'test/autoplan-phase-order.test.ts', 'design-html/**', 'design-shotgun/**', 'qa/**', 'browse/**', 'retro/**', 'autoplan/**', 'spec/**', 'setup-gbrain/**', 'review/**', 'codex/**', 'land-and-deploy/**', 'plan-eng-review/**', 'plan-design-review/**', 'plan-devex-review/**', 'office-hours/**', 'document-release/**', 'design-consultation/**', 'cso/**', 'test/helpers/carve-guards.ts', 'scripts/resolvers/sections.ts', 'scripts/resolvers/redact-doc.ts', 'scripts/gen-skill-docs.ts', 'test/helpers/auq-sdk-capture.ts', 'test/helpers/session-runner.ts', 'test/session-runner-tools.test.ts', 'test/carve-section-loading.test.ts'],
|
||||
'autoplan-chain-pty': ['scripts/resolvers/composition.ts', 'test/autoplan-review-discovery.test.ts', 'test/autoplan-phase-order.test.ts', 'autoplan/**', 'plan-ceo-review/**', 'plan-design-review/**', 'plan-eng-review/**', 'plan-devex-review/**', 'test/fixtures/plans/autoplan-dashboard.md', 'test/autoplan-chain-fixture.test.ts', 'test/helpers/claude-pty-runner.ts', 'test/plan-count-native-input.test.ts', 'test/helpers/pty-screen.ts', 'test/pty-screen.test.ts', 'test/pty-screen-session.test.ts', 'test/fixtures/pty-screen/**', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/autoplan-setup-question.ts', 'test/autoplan-setup-question.test.ts', 'test/helpers/autoplan-phase-observer.ts', 'test/autoplan-phase-observer.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/skill-e2e-autoplan-chain.test.ts'],
|
||||
'autoplan-chain-pty': ['scripts/resolvers/composition.ts', 'test/autoplan-review-discovery.test.ts', 'test/autoplan-phase-order.test.ts', 'autoplan/**', 'plan-ceo-review/**', 'plan-design-review/**', 'plan-eng-review/**', 'plan-devex-review/**', 'test/fixtures/plans/autoplan-dashboard.md', 'test/autoplan-chain-fixture.test.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'bin/gstack-config', 'test/helpers/claude-pty-runner.ts', 'test/plan-count-native-input.test.ts', 'test/helpers/pty-screen.ts', 'test/pty-screen.test.ts', 'test/pty-screen-session.test.ts', 'test/fixtures/pty-screen/**', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/autoplan-setup-question.ts', 'test/autoplan-setup-question.test.ts', 'test/helpers/autoplan-phase-observer.ts', 'test/autoplan-phase-observer.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/skill-e2e-autoplan-chain.test.ts'],
|
||||
|
||||
// Per-finding AskUserQuestion count + review-report-at-bottom assertion.
|
||||
// Each test drives its skill end-to-end; touchfiles include preamble +
|
||||
@@ -162,7 +162,7 @@ export const E2E_TOUCHFILES: Record<string, string[]> = {
|
||||
// terminal output (the regression surface this test catches).
|
||||
'plan-ceo-finding-count': ['bin/gstack-config', 'bin/gstack-skill-start', 'bin/gstack-skill-end', 'plan-ceo-review/**', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'scripts/resolvers/preamble/generate-completion-status.ts', 'test/helpers/claude-pty-runner.ts', 'test/plan-count-native-input.test.ts', 'test/helpers/pty-screen.ts', 'test/pty-screen.test.ts', 'test/pty-screen-session.test.ts', 'test/fixtures/pty-screen/**', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/plan-count-completion.test.ts', 'test/helpers/ceo-completion-handoff.ts', 'test/ceo-completion-handoff.test.ts', 'test/fixtures/ceo-completion-handoff-calls.json', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/helpers/eval-store.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/skill-e2e-plan-ceo-finding-count.test.ts'],
|
||||
'plan-eng-finding-count': ['bin/gstack-config', 'bin/gstack-skill-start', 'bin/gstack-skill-end', 'plan-eng-review/**', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'scripts/resolvers/preamble/generate-completion-status.ts', 'test/helpers/claude-pty-runner.ts', 'test/plan-count-native-input.test.ts', 'test/helpers/pty-screen.ts', 'test/pty-screen.test.ts', 'test/pty-screen-session.test.ts', 'test/fixtures/pty-screen/**', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/plan-count-completion.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/helpers/eval-store.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/skill-e2e-plan-eng-finding-count.test.ts'],
|
||||
'plan-design-finding-count': ['bin/gstack-config', 'bin/gstack-skill-start', 'bin/gstack-skill-end', 'plan-design-review/**', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'scripts/resolvers/preamble/generate-completion-status.ts', 'test/helpers/claude-pty-runner.ts', 'test/plan-count-native-input.test.ts', 'test/helpers/pty-screen.ts', 'test/pty-screen.test.ts', 'test/pty-screen-session.test.ts', 'test/fixtures/pty-screen/**', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/plan-count-completion.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/helpers/eval-store.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/skill-e2e-plan-design-finding-count.test.ts'],
|
||||
'plan-design-finding-count': ['bin/gstack-config', 'bin/gstack-skill-start', 'bin/gstack-skill-end', 'plan-design-review/**', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'scripts/resolvers/preamble/generate-completion-status.ts', 'test/helpers/claude-pty-runner.ts', 'test/plan-count-native-input.test.ts', 'test/helpers/pty-screen.ts', 'test/pty-screen.test.ts', 'test/pty-screen-session.test.ts', 'test/fixtures/pty-screen/**', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/plan-count-completion.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/helpers/eval-store.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/helpers/design-count-outside.ts', 'test/design-count-outside.test.ts', 'test/skill-e2e-plan-design-finding-count.test.ts'],
|
||||
'plan-devex-finding-count': ['bin/gstack-config', 'bin/gstack-skill-start', 'bin/gstack-skill-end', 'plan-devex-review/**', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'scripts/resolvers/preamble/generate-completion-status.ts', 'test/helpers/claude-pty-runner.ts', 'test/plan-count-native-input.test.ts', 'test/helpers/pty-screen.ts', 'test/pty-screen.test.ts', 'test/pty-screen-session.test.ts', 'test/fixtures/pty-screen/**', 'test/helpers/hermetic-skill-runtime.ts', 'test/hermetic-skill-runtime.test.ts', 'test/helpers/claude-pty-runner.unit.test.ts', 'test/plan-count-completion.test.ts', 'test/helpers/pty-trust-dialog.ts', 'test/pty-trust-dialog.test.ts', 'test/helpers/plan-count-transcript.ts', 'test/plan-count-transcript.test.ts', 'test/helpers/plan-count-artifacts.ts', 'test/plan-count-artifacts.test.ts', 'test/helpers/eval-store.ts', 'test/helpers/plan-count-fixture.ts', 'test/plan-count-fixture.test.ts', 'test/helpers/devex-count-fixture.ts', 'test/devex-count-fixture.test.ts', 'test/skill-e2e-plan-devex-finding-count.test.ts'],
|
||||
|
||||
// Gate-tier reviewCount-floor counterparts. Catch the May 2026 transcript
|
||||
@@ -637,7 +637,7 @@ export const E2E_TIERS: Record<string, 'gate' | 'periodic'> = {
|
||||
'ship-section-loading': 'periodic', // ~$3/run, real /ship; asserts section reads
|
||||
'plan-ceo-section-loading': 'periodic', // ~$3-5/run, real /plan-ceo-review; asserts section read
|
||||
'carve-section-loading': 'periodic', // ~$1-2/skill, data-driven; GSTACK_CARVE_SKILL scopes to one
|
||||
'autoplan-chain-pty': 'periodic', // ~$8/run, all 3 phases sequential
|
||||
'autoplan-chain-pty': 'periodic', // ~$8/run, full native CEO → Design → DX → Eng sequence; outside disabled
|
||||
|
||||
// Per-finding count + review-report-at-bottom — periodic because each
|
||||
// run drives a full skill end-to-end (~25 min, ~$5/run). Sequential
|
||||
|
||||
@@ -4,7 +4,7 @@ import * as fs from 'node:fs';
|
||||
import * as os from 'node:os';
|
||||
import * as path from 'node:path';
|
||||
import { pathToFileURL } from 'node:url';
|
||||
import { hasNativePlanCompletion, hasNativePlanTerminal, classifyPlanCountFrame } from './helpers/claude-pty-runner';
|
||||
import { hasNativePlanCompletion, hasNativePlanTerminal, isPlanReadyVisible, classifyPlanCountFrame } from './helpers/claude-pty-runner';
|
||||
import type { PlanCountTranscript } from './helpers/plan-count-transcript';
|
||||
|
||||
const CAPTURED_CALL = {
|
||||
@@ -391,7 +391,7 @@ describe('guarded native terminal and report lifecycle', () => {
|
||||
});
|
||||
|
||||
test.skipIf(process.platform === 'win32')('real PTY waits through streamed summary, another question and partial report writes', async () => {
|
||||
await Promise.all(['completion_summary', 'plan_ready'].map(async (terminal) => {
|
||||
await Promise.all(['completion_summary', 'plan_ready', 'compact_plan_ready', 'ready_with_unsanctioned_write'].map(async (terminal) => {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'guarded-terminal-pty-'));
|
||||
const fake = path.join(dir, 'fake-claude');
|
||||
const worker = path.join(dir, 'worker.ts');
|
||||
@@ -411,11 +411,16 @@ const native = (role, content, extra = {}) => fs.appendFileSync(file, JSON.strin
|
||||
cwd: process.cwd(), sessionId, isSidechain: false, timestamp: new Date().toISOString(), message: { role, content }, ...extra,
|
||||
}) + '\n');
|
||||
const event = name => fs.appendFileSync(process.env.PROBE_EVENTS, JSON.stringify({ name, at: Date.now() }) + '\n');
|
||||
const q = { header: 'Tests', question: 'D2 — Test finding: add the missing retry assertion? <gstack-qid:plan-eng-test-missing-retry>', options: [{ label: 'Add retry assertion' }, { label: 'Leave untested' }] };
|
||||
const q = { header: 'Tests', question: 'D2 — Test “Would you like to proceed?” <gstack-qid:plan-eng-test-retry>', options: [{ label: 'Add retry assertion' }, { label: 'Leave untested' }] };
|
||||
const ask = id => native('assistant', [{ type: 'tool_use', id, name: 'AskUserQuestion', input: { questions: [q] } }]);
|
||||
const answer = id => native('user', [{ type: 'tool_result', tool_use_id: id, content: 'Answered.' }], { toolUseResult: { answers: { [q.question]: q.options[0].label } } });
|
||||
const summary = 'Completion Summary:\n- Architecture Review: 1 issue resolved\n- Test Review: 1 missing test resolved\n';
|
||||
const ready = () => process.stdout.write('\nClaude has written up a plan and is ready to execute. Would you like to proceed?\n❯ 1. Yes, and use auto mode\n 2. Yes, manually approve edits\n 3. Tell Claude what to change\n');
|
||||
const compact = process.env.PROBE_TERMINAL === 'compact_plan_ready';
|
||||
const ready = () => process.stdout.write(process.env.PROBE_TERMINAL === 'ready_with_unsanctioned_write'
|
||||
? '\x1b[2J\x1b[H⏺ Write(src/unreviewed.ts)\nClaude has written up a plan and is ready to execute.\n'
|
||||
: compact
|
||||
? '\x1b[2J\x1b[HExit plan mode?\n\nClaude wants to exit plan mode\n\n❯ 1. Yes, and switch to default (ask each time) for this session\n 2. No\n'
|
||||
: '\nClaude has written up a plan and is ready to execute. Would you like to proceed?\n❯ 1. Yes, and use auto mode\n 2. Yes, manually approve edits\n 3. Tell Claude what to change\n');
|
||||
let stage = 0;
|
||||
process.stdin.setRawMode?.(true);
|
||||
process.stdin.on('data', data => {
|
||||
@@ -423,9 +428,16 @@ process.stdin.on('data', data => {
|
||||
if (stage === 0) {
|
||||
stage = 1; ask('first'); answer('first');
|
||||
event('streamed-heading'); process.stdout.write('● Completion Summary:\n- Architecture Review: 1 issue resolved\n');
|
||||
if (compact) {
|
||||
// Native metadata for an unrelated question and a prior permission
|
||||
// cannot authorize this ExitPlanMode menu. No report exists yet.
|
||||
ask('second');
|
||||
process.stdout.write('Do you want to make this edit to plan.md?\n❯ 1. Yes\n 2. Yes, and allow all edits\n 3. No\nEsc to cancel · Tab to amend\n⎿ Added 1 line\n');
|
||||
event('unverified-compact-gate'); ready();
|
||||
}
|
||||
setTimeout(() => {
|
||||
ask('second'); event('pending-question');
|
||||
process.stdout.write('\n← ☐ Tests ✔ Submit →\n' + q.question + '\n❯ 1. Add retry assertion\n 2. Leave untested\nEnter to select · Tab/Arrow keys to navigate · Esc to cancel\n');
|
||||
if (!compact) ask('second'); event('pending-question');
|
||||
process.stdout.write('\n☐ Tests\n' + q.question + '\n❯ 1. Add retry assertion\n 2. Leave untested\nEnter to select · ↑/↓ to navigate · Esc to cancel\n');
|
||||
}, 3500);
|
||||
return;
|
||||
}
|
||||
@@ -434,12 +446,13 @@ process.stdin.on('data', data => {
|
||||
fs.writeFileSync(process.env.PROBE_PLAN, '# Draft\n');
|
||||
native('assistant', [{ type: 'text', text: summary }], { timestamp: new Date(Date.now() + 1).toISOString() });
|
||||
process.stdout.write('\n● ' + summary);
|
||||
if (compact) ready();
|
||||
setTimeout(() => {
|
||||
event('partial-report'); fs.writeFileSync(process.env.PROBE_PLAN, process.env.PROBE_REPORT.split('VERDICT:')[0]);
|
||||
}, 3000);
|
||||
setTimeout(() => {
|
||||
event('complete-report'); fs.writeFileSync(process.env.PROBE_PLAN, process.env.PROBE_REPORT);
|
||||
if (process.env.PROBE_TERMINAL === 'plan_ready') {
|
||||
if (process.env.PROBE_TERMINAL !== 'completion_summary') {
|
||||
native('assistant', [{ type: 'tool_use', id: 'rejected-ready', name: 'ExitPlanMode', input: {} }]);
|
||||
native('user', [{ type: 'tool_result', tool_use_id: 'rejected-ready', is_error: true, content: 'Gate not ready' }]);
|
||||
ready();
|
||||
@@ -464,13 +477,16 @@ process.stdin.resume();
|
||||
expect(code, stdout + stderr).toBe(0);
|
||||
const observation = JSON.parse(fs.readFileSync(result, 'utf8'));
|
||||
expect(fs.existsSync(output), `D19: report must exist before stopping; ${observation.summary}`).toBe(true);
|
||||
expect(observation.outcome, JSON.stringify(observation)).toBe(terminal);
|
||||
expect(observation.outcome, JSON.stringify(observation)).toBe(
|
||||
terminal === 'ready_with_unsanctioned_write' ? 'silent_write' :
|
||||
terminal === 'compact_plan_ready' ? 'plan_ready' : terminal,
|
||||
);
|
||||
expect(observation.reviewCount).toBe(2);
|
||||
expect(fs.readFileSync(output, 'utf8')).toBe(REPORT);
|
||||
const recorded = fs.readFileSync(events, 'utf8').trim().split('\n').map(line => JSON.parse(line));
|
||||
expect(recorded.map(e => e.name)).toEqual(['streamed-heading', 'pending-question', 'answered-second', 'partial-report', 'complete-report', ...(terminal === 'plan_ready' ? ['valid-native-ready'] : [])]);
|
||||
expect(recorded.map(e => e.name)).toEqual(['streamed-heading', ...(terminal === 'compact_plan_ready' ? ['unverified-compact-gate'] : []), 'pending-question', 'answered-second', 'partial-report', 'complete-report', ...(terminal !== 'completion_summary' ? ['valid-native-ready'] : [])]);
|
||||
expect(fs.readFileSync(record, 'utf8').trim().split('\n').map(line => JSON.parse(line))).toEqual(['/plan-eng-review\r', '1']);
|
||||
if (terminal === 'plan_ready') expect(observation.transcript.planReadyRequests.map(r => r.failed)).toEqual([true, false]);
|
||||
if (terminal !== 'completion_summary') expect(observation.transcript.planReadyRequests.map(r => r.failed)).toEqual([true, false]);
|
||||
} finally { clearTimeout(timer); child.kill('SIGKILL'); await child.exited; fs.rmSync(dir, { recursive: true, force: true }); }
|
||||
}));
|
||||
}, 50000);
|
||||
@@ -491,3 +507,142 @@ test('captured completed CEO prose supports the ordinary summary path', () => {
|
||||
expect(hasNativePlanTerminal(f.transcript, f.file, f.startedAt, 'completion_summary')).toBe(false);
|
||||
} finally { f.cleanup(); }
|
||||
});
|
||||
|
||||
|
||||
const CAPTURED_NUMBERED_DX_HANDOFF = {
|
||||
"sessionId": "0b7d664e-8d15-4c0e-8be2-d03a833d5d33",
|
||||
"toolUseId": "toolu_01RgoUjV9vFRaSFoySjm5QqY",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D8 — DX review complete. 5 friction points resolved, DX score 4/10 → 8/10, TTHW target achievable. What’s next? <gstack-qid:devex-next-steps>",
|
||||
"header": "Next steps",
|
||||
"options": [
|
||||
{
|
||||
"label": "Run /plan-eng-review next (recommended)",
|
||||
"description": "Three of the five fixes (CI skip flag, auth error upgrade, deprecated alias) have architectural implications. Eng review validates the contracts before implementation."
|
||||
},
|
||||
{
|
||||
"label": "Ready to implement — run /devex-review after shipping",
|
||||
"description": "Skip eng review for now. Implement the five recommendations, then run /devex-review on the live beta to verify TTHW <2 min is achieved."
|
||||
},
|
||||
{
|
||||
"label": "Skip — I'll handle next steps manually",
|
||||
"description": "No further review automation. You have the five recommendations; implementation is up to you."
|
||||
}
|
||||
],
|
||||
"multiSelect": false
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D8 — DX review complete. 5 friction points resolved, DX score 4/10 → 8/10, TTHW target achievable. What’s next? <gstack-qid:devex-next-steps>": "Run /plan-eng-review next (recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-08T20:49:56.950Z"
|
||||
};
|
||||
|
||||
|
||||
describe('numbered completed DX handoff preserves native report freshness', () => {
|
||||
function completedHandoffTranscript(f: ReturnType<typeof fixture>): PlanCountTranscript {
|
||||
const transcript = structuredClone(f.transcript);
|
||||
const sessionId = transcript.calls[0]!.sessionId;
|
||||
transcript.calls.push({ ...structuredClone(CAPTURED_NUMBERED_DX_HANDOFF), sessionId });
|
||||
transcript.planReadyRequests = [{
|
||||
sessionId,
|
||||
toolUseId: 'toolu_01QwXDuSELZNrgexVf88bQxG',
|
||||
timestamp: '2026-09-08T20:50:30.273Z',
|
||||
failed: false,
|
||||
}];
|
||||
return transcript;
|
||||
}
|
||||
|
||||
test('the captured three-choice handoff changes orchestration without requiring another report write', () => {
|
||||
const f = fixture();
|
||||
try {
|
||||
const transcript = completedHandoffTranscript(f);
|
||||
expect(hasNativePlanTerminal(transcript, f.file, f.startedAt, 'plan_ready')).toBe(true);
|
||||
transcript.planReadyRequests![0]!.failed = true;
|
||||
expect(hasNativePlanTerminal(transcript, f.file, f.startedAt, 'plan_ready')).toBe(false);
|
||||
transcript.planReadyRequests = [];
|
||||
expect(hasNativePlanTerminal(transcript, f.file, f.startedAt, 'plan_ready')).toBe(false);
|
||||
} finally {
|
||||
f.cleanup();
|
||||
}
|
||||
});
|
||||
|
||||
test('substantive, incomplete, unanswered and mixed menus remain report freshness boundaries', () => {
|
||||
const f = fixture();
|
||||
try {
|
||||
for (const mutate of [
|
||||
(call: any) => { call.questions[0].question = call.questions[0].question.replace('review complete.', 'review still has unresolved auth behavior.'); },
|
||||
(call: any) => { call.questions[0].question = call.questions[0].question.replace('devex-next-steps', 'devex-auth-finding'); },
|
||||
(call: any) => { call.questions[0].header = 'Auth policy'; },
|
||||
(call: any) => { call.questions[0].options.push({ label: 'Add retry support to the SDK now' }); },
|
||||
(call: any) => { call.questions[0].options[1].label = 'Ready to implement — change authentication first'; },
|
||||
(call: any) => { call.questions[0].options[2].label = 'Skip — keep the broken authentication'; },
|
||||
(call: any) => { call.answers[call.questions[0].question] = 'First implement the missing retry fix'; },
|
||||
(call: any) => { call.unansweredQuestionIndices = [0]; },
|
||||
(call: any) => { call.questions.push(structuredClone(CAPTURED_CALL.questions[0])); },
|
||||
]) {
|
||||
const transcript = completedHandoffTranscript(f);
|
||||
mutate(transcript.calls[1]);
|
||||
expect(hasNativePlanTerminal(transcript, f.file, f.startedAt, 'plan_ready')).toBe(false);
|
||||
}
|
||||
} finally {
|
||||
f.cleanup();
|
||||
}
|
||||
});
|
||||
|
||||
test('a later actual review answer still requires a fresh complete report', () => {
|
||||
const f = fixture();
|
||||
try {
|
||||
const transcript = completedHandoffTranscript(f);
|
||||
transcript.calls.splice(1, 0, {
|
||||
...structuredClone(CAPTURED_CALL),
|
||||
toolUseId: 'later-review-decision',
|
||||
answeredAt: '2026-09-08T20:49:00.000Z',
|
||||
});
|
||||
expect(hasNativePlanTerminal(transcript, f.file, f.startedAt, 'plan_ready')).toBe(false);
|
||||
fs.utimesSync(f.file, Date.parse('2026-09-08T20:49:20Z') / 1000, Date.parse('2026-09-08T20:49:20Z') / 1000);
|
||||
expect(hasNativePlanTerminal(transcript, f.file, f.startedAt, 'plan_ready')).toBe(true);
|
||||
fs.writeFileSync(f.file, '## GSTACK REVIEW REPORT\n');
|
||||
expect(hasNativePlanTerminal(transcript, f.file, f.startedAt, 'plan_ready')).toBe(false);
|
||||
} finally {
|
||||
f.cleanup();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
// Source-I Eng displayed this compact native approval after ExitPlanMode.
|
||||
// Its classification must not itself claim a completed review/report.
|
||||
const CAPTURED_COMPACT_PLAN_GATE = `Exit plan mode?
|
||||
|
||||
Claude wants to exit plan mode
|
||||
|
||||
❯ 1. Yes, and switch to default (ask each time) for this session
|
||||
2. No`;
|
||||
|
||||
test('the captured compact ExitPlanMode menu is an approval gate, not an ordinary answer', () => {
|
||||
expect(isPlanReadyVisible(CAPTURED_COMPACT_PLAN_GATE)).toBe(true);
|
||||
expect(classifyPlanCountFrame(CAPTURED_COMPACT_PLAN_GATE)).toBe('plan_ready');
|
||||
const f = fixture();
|
||||
try {
|
||||
expect(hasNativePlanTerminal(f.transcript, f.file, f.startedAt, 'plan_ready')).toBe(false);
|
||||
f.transcript.planReadyRequests = [{ sessionId: f.transcript.calls[0]!.sessionId,
|
||||
toolUseId: 'exit', timestamp: CAPTURED_FINAL.timestamp, failed: false }];
|
||||
fs.writeFileSync(f.file, '# Draft\n');
|
||||
expect(hasNativePlanTerminal(f.transcript, f.file, f.startedAt, 'plan_ready')).toBe(false);
|
||||
} finally { f.cleanup(); }
|
||||
});
|
||||
|
||||
test('compact approval recognition rejects quoted, incomplete and superseded menus', () => {
|
||||
for (const visible of [
|
||||
'Should the application show an Exit plan mode? confirmation?',
|
||||
'> ' + CAPTURED_COMPACT_PLAN_GATE.replaceAll('\n', '\n> '),
|
||||
CAPTURED_COMPACT_PLAN_GATE.replace('Claude wants to exit plan mode', 'The documentation mentions exiting plan mode'),
|
||||
CAPTURED_COMPACT_PLAN_GATE.replace(' 2. No', ''),
|
||||
CAPTURED_COMPACT_PLAN_GATE + '\nTests\nAdd retry coverage?\n❯ 1. Add test\n 2. Skip\nEnter to select · Esc to cancel',
|
||||
]) expect(isPlanReadyVisible(visible), visible).toBe(false);
|
||||
});
|
||||
@@ -15,6 +15,43 @@ import {
|
||||
} from './helpers/ceo-mode-option';
|
||||
import { autoplanRoutingSetupInput } from './helpers/autoplan-setup-question';
|
||||
|
||||
const designOutsideQuestions = [
|
||||
{
|
||||
"question": "D3 (Step 0D) — I've rated this plan 5/10 on design completeness. The three biggest gaps are: (1) the 5 identified implementation gaps describe the problem but not the solution, (2) no explicit state coverage table, (3) no user journey emotional arc. I'll skip mockups and review all 7 dimensions as you requested. Any specific areas to prioritize, or cover all 7 equally? <gstack-qid:plan-design-focus>",
|
||||
"header": "Focus areas",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "Cover all 7 equally (Recommended)",
|
||||
"description": "Standard review: all 7 design dimensions get full treatment. Takes longer but produces a complete plan."
|
||||
},
|
||||
{
|
||||
"label": "Focus on the 5 identified gaps first",
|
||||
"description": "Prioritize Pass 5 (Design System Alignment) to close the gap descriptions into actionable specs, then cover remaining passes more quickly."
|
||||
},
|
||||
{
|
||||
"label": "Prioritize accessibility and states",
|
||||
"description": "Focus on Pass 2 (Interaction States) and Pass 6 (Responsive/A11y), since the form has sensitive UX requirements (ARIA, contrast, keyboard)."
|
||||
}
|
||||
]
|
||||
},
|
||||
{
|
||||
"question": "D4 — Want outside design voices before the detailed review? Codex evaluates against OpenAI's design hard rules + litmus checks; a Claude subagent does an independent completeness review. (Requires Codex CLI to be installed.) <gstack-qid:outside-voices-design>",
|
||||
"header": "Outside voices",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "Yes, run outside design voices",
|
||||
"description": "Launches Codex design critique + Claude subagent completeness review in parallel before the 7 passes. Adds 1–2 minutes."
|
||||
},
|
||||
{
|
||||
"label": "No, proceed without (Recommended)",
|
||||
"description": "Skip outside voices and go straight to the 7 review passes. Faster; sufficient for most plans."
|
||||
}
|
||||
]
|
||||
}
|
||||
];
|
||||
|
||||
const questions = [
|
||||
{
|
||||
header: 'Routing rules',
|
||||
@@ -261,6 +298,8 @@ describe('native AUQ accepts one action per displayed question', () => {
|
||||
long: true,
|
||||
},
|
||||
{ name: 'late-packet', count: 2, late: true, permission: false },
|
||||
{ name: 'design-outside-tab', count: 2, late: false, permission: false, designQuestions: designOutsideQuestions },
|
||||
{ name: 'design-outside-late', count: 2, late: true, permission: false, designQuestions: designOutsideQuestions },
|
||||
].map((item) => ({
|
||||
...item,
|
||||
record: path.join(dir, item.name + '.jsonl'),
|
||||
@@ -290,7 +329,7 @@ const native = (role, content, extra = {}) =>
|
||||
);
|
||||
native('assistant', [{ type: 'text', text: 'Fixture started.' }]);
|
||||
record({ type: 'startup', pid: process.pid, cwd: process.cwd() });
|
||||
const questions = Array.from({ length: item.count }, (_, i) => ({
|
||||
const questions = item.designQuestions ?? Array.from({ length: item.count }, (_, i) => ({
|
||||
header: 'Decision ' + i,
|
||||
question:
|
||||
(item.long && i === 0
|
||||
@@ -330,7 +369,8 @@ const render = () => {
|
||||
.join(' ') +
|
||||
' ✔ Submit →\n│ ' +
|
||||
questions[index].question +
|
||||
'\n❯1.First remedy\n2.Second remedy\n3.Type something.\n4.Chat about this\nEnter to select · Tab/Arrow keys to navigate · Esc to cancel\n';
|
||||
'\n' + questions[index].options.map((option, i) => (i === 0 ? '❯' : '') + (i + 1) + '.' + option.label).join('\n') +
|
||||
'\nEnter to select · Tab/Arrow keys to navigate · Esc to cancel\n';
|
||||
else if (index === questions.length)
|
||||
screen =
|
||||
'← ' +
|
||||
@@ -385,6 +425,7 @@ process.stdin.on('data', (data) => {
|
||||
{ toolUseResult: { answers } },
|
||||
);
|
||||
record({ type: 'submitted', answers });
|
||||
if (item.designQuestions && Object.values(answers)[1] === item.designQuestions[1].options[0].label) record({ type: 'outside-dispatched' });
|
||||
done = true;
|
||||
process.stdout.write('\x1b[2J\x1b[HGSTACK REVIEW REPORT\r\n');
|
||||
return;
|
||||
@@ -410,6 +451,7 @@ process.stdin.resume();
|
||||
fs.writeFileSync(
|
||||
worker,
|
||||
`import {runPlanSkillCounting} from ${JSON.stringify(pathToFileURL(path.resolve(import.meta.dir, 'helpers/claude-pty-runner.ts')).href)};
|
||||
import {pickDesignCountOutsideVoices} from ${JSON.stringify(pathToFileURL(path.resolve(import.meta.dir, 'helpers/design-count-outside.ts')).href)};
|
||||
const cases=${JSON.stringify(cases)};
|
||||
const results = await Promise.all(cases.map(async item => ({
|
||||
name: item.name,
|
||||
@@ -419,7 +461,8 @@ const results = await Promise.all(cases.map(async item => ({
|
||||
followUpPrompt: 'Review only this fixture.',
|
||||
isLastStep0AUQ: () => false,
|
||||
isReviewAUQ: () => true,
|
||||
firstAUQPick: () => 2,
|
||||
firstAUQPick: item.designQuestions ? undefined : () => 2,
|
||||
pickAUQ: item.designQuestions ? pickDesignCountOutsideVoices : undefined,
|
||||
reviewCountCeiling: 8,
|
||||
timeoutMs: 22000,
|
||||
env: { NATIVE_INPUT_CASE: JSON.stringify(item) },
|
||||
@@ -465,8 +508,7 @@ await Bun.write(${JSON.stringify(results)},JSON.stringify(results));`,
|
||||
).toEqual([
|
||||
'/plan-eng-review\r',
|
||||
...(item.permission ? ['1\r'] : []),
|
||||
'2',
|
||||
...Array(item.count - 1).fill('1'),
|
||||
...(item.designQuestions ? ['1', '2'] : ['2', ...Array(item.count - 1).fill('1')]),
|
||||
'\r',
|
||||
]);
|
||||
expect(events.filter((e) => e.type === 'screen' && e.blank)).toEqual(
|
||||
@@ -477,10 +519,10 @@ await Bun.write(${JSON.stringify(results)},JSON.stringify(results));`,
|
||||
).toEqual([]);
|
||||
const submitted = events.filter((e) => e.type === 'submitted');
|
||||
expect(submitted).toHaveLength(1);
|
||||
expect(Object.values(submitted[0].answers)).toEqual([
|
||||
'Second remedy',
|
||||
...Array(item.count - 1).fill('First remedy'),
|
||||
]);
|
||||
expect(Object.values(submitted[0].answers)).toEqual(item.designQuestions
|
||||
? [item.designQuestions[0].options[0].label, item.designQuestions[1].options[1].label]
|
||||
: ['Second remedy', ...Array(item.count - 1).fill('First remedy')]);
|
||||
expect(events.filter(e => e.type === 'outside-dispatched')).toEqual([]);
|
||||
expect(() => process.kill(events[0].pid, 0)).toThrow();
|
||||
expect(fs.existsSync(events[0].cwd)).toBe(false);
|
||||
}
|
||||
|
||||
@@ -72,7 +72,11 @@ describe('owned PTY viewport', () => {
|
||||
|
||||
test('clipped native handoff preserves its bound manual choice in either option order', async () => {
|
||||
for (const contextLines of [36, 41]) for (const reverse of [false, true]) {
|
||||
const options = [{ label: 'Run /plan-eng-review next' }, { label: "Done — I'll handle reviews manually" }];
|
||||
// The captured CEO handoff explains the required gate in this choice.
|
||||
// Keep that native evidence even when the viewport clips its heading.
|
||||
const options = [{ label: 'Run /plan-eng-review next',
|
||||
description: 'Eng Review is the only required gate before shipping. Covers architecture, code quality, tests, and performance at the diff level. Run it now to clear the shipping gate.',
|
||||
}, { label: "Done — I'll handle reviews manually" }];
|
||||
if (reverse) options.reverse();
|
||||
const call: NativePlanQuestionCall = { sessionId: 'long-handoff', toolUseId: 'pending', answered: false,
|
||||
questions: [{ header: 'Next review',
|
||||
|
||||
@@ -1,26 +1,17 @@
|
||||
/**
|
||||
* /autoplan cross-skill chain (periodic, paid, real-PTY).
|
||||
* /autoplan native chain sequencing (periodic, paid, real PTY).
|
||||
*
|
||||
* Asserts: when /autoplan runs against a plan fixture, the phase markers
|
||||
* the autoplan template emits appear in the correct order:
|
||||
* The calibrated UI/API fixture requires full CEO → Design → DX → Eng review.
|
||||
* This test disables only the outside CLI through codex_reviews; the native
|
||||
* subagents and every applicable phase still run. Require real native phase
|
||||
* completion announcements in order, with Eng last. Completion order does not
|
||||
* establish phase start times or prove non-overlap.
|
||||
*
|
||||
* "**Phase 1 complete." (CEO) →
|
||||
* "**Phase 2 complete." (Design — only if UI scope detected) →
|
||||
* "**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)
|
||||
* Outside coverage here is disabled, never completed. The separate dual-voice
|
||||
* and cross-harness evals exercise provider dispatch; this test does not replace
|
||||
* those or establish per-phase Autoplan outside completion coverage.
|
||||
*
|
||||
* Why this exists: each individual phase has its own plan-mode smoke
|
||||
* test. This checks cross-phase completion order, including the conditional
|
||||
* Design/DX phases when they run. Completion markers do not establish phase
|
||||
* start times or prove that no work overlapped between phases.
|
||||
*
|
||||
* Approach: read standalone phase-completion announcements and their native
|
||||
* assistant timestamps from the isolated transcript. Assert observed ordering. Phase 2 is
|
||||
* optional — UI-heavy fixture should make it run; backend-only fixtures
|
||||
* should make it skip.
|
||||
*
|
||||
* Cost: ~$5-8/run, 10-15 min wall clock. Periodic — runs weekly.
|
||||
* Existing budget: 15 min work, 20 min absolute test ceiling.
|
||||
*/
|
||||
|
||||
import { test, expect } from 'bun:test';
|
||||
@@ -42,6 +33,7 @@ import { autoplanRoutingSetupInput } from './helpers/autoplan-setup-question';
|
||||
import { autoplanPhaseCompletions, type AutoplanPhaseHit } from './helpers/autoplan-phase-observer';
|
||||
import { readPlanCountTranscript, type PlanCountTranscript } from './helpers/plan-count-transcript';
|
||||
import { createPlanCountSnapshotWriter } from './helpers/plan-count-artifacts';
|
||||
import { createNativeReviewState } from './helpers/plan-count-fixture';
|
||||
|
||||
const describeE2E = describeE2ETier('periodic');
|
||||
|
||||
@@ -54,13 +46,14 @@ function diagnosticTail(text: string): string {
|
||||
.slice(-3000);
|
||||
}
|
||||
|
||||
describeE2E('/autoplan chain ordering (periodic)', () => {
|
||||
describeE2E('/autoplan native chain ordering (periodic)', () => {
|
||||
test(
|
||||
'phase completions are ordered: Phase 1 (CEO) before Phase 3 (Eng), Phase 2 (Design) between when present',
|
||||
'full native phase completions are ordered: CEO before Design before DX before Eng',
|
||||
async () => {
|
||||
// Chain-only fixture retains all new UI/API work and supplies existing
|
||||
// application contracts; the shared design-scope fixture stays unchanged.
|
||||
const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-autoplan-chain-'));
|
||||
let nativeState: ReturnType<typeof createNativeReviewState> | undefined;
|
||||
try {
|
||||
const gitRun = (args: string[]) =>
|
||||
spawnSync('git', args, { cwd: tempDir, stdio: 'pipe', timeout: 5000 });
|
||||
@@ -75,7 +68,9 @@ describeE2E('/autoplan chain ordering (periodic)', () => {
|
||||
gitRun(['add', '.']);
|
||||
gitRun(['commit', '-m', 'init UI-heavy fixture']);
|
||||
|
||||
nativeState = createNativeReviewState();
|
||||
const session = await launchClaudePty({
|
||||
env: nativeState.env,
|
||||
permissionMode: 'plan',
|
||||
cwd: tempDir,
|
||||
timeoutMs: 1_080_000, // 18 min, slightly above test budget
|
||||
@@ -198,8 +193,9 @@ describeE2E('/autoplan chain ordering (periodic)', () => {
|
||||
// Phase 3 (Eng) MUST have been seen.
|
||||
const ceo = hits.find(h => h.phase === 1);
|
||||
const design = hits.find(h => h.phase === 2);
|
||||
const dx = hits.find(h => h.phase === 2.5);
|
||||
const eng = hits.find(h => h.phase === 3);
|
||||
if (!ceo || !eng) {
|
||||
if (!ceo || !design || !dx || !eng) {
|
||||
throw new Error(
|
||||
`Required phase markers missing. Saw: ${JSON.stringify(hits)}\n` +
|
||||
`Native transcript: ${transcript.status}; artifacts=${JSON.stringify(artifacts)}\n` +
|
||||
@@ -207,24 +203,16 @@ describeE2E('/autoplan chain ordering (periodic)', () => {
|
||||
);
|
||||
}
|
||||
|
||||
// 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);
|
||||
}
|
||||
// This fixture has UI and API scope: all four phases are required.
|
||||
expect(ceo.ts).toBeLessThan(design.ts);
|
||||
expect(design.ts).toBeLessThan(dx.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 */ }
|
||||
finally { nativeState?.cleanup(); }
|
||||
}
|
||||
},
|
||||
PTY_LONG_MS, // 20 min absolute test ceiling
|
||||
|
||||
@@ -10,6 +10,7 @@
|
||||
|
||||
import { test } from 'bun:test';
|
||||
import { describeE2ETier } from './helpers/e2e-gate';
|
||||
import { pickDesignCountOutsideVoices } from './helpers/design-count-outside';
|
||||
import * as fs from 'node:fs';
|
||||
import * as os from 'node:os';
|
||||
import * as path from 'node:path';
|
||||
@@ -175,6 +176,9 @@ describeE2E('/plan-design-review per-finding AskUserQuestion count (periodic)',
|
||||
isLastStep0AUQ: designStep0Boundary,
|
||||
isFirstReviewAUQ: designFirstReviewAUQ,
|
||||
fixtureFiles: { 'DESIGN.md': designSystem },
|
||||
// Design's explicit opt-in is separate from codex_reviews. Keep
|
||||
// this native-cadence fixture within its declared review scope.
|
||||
pickAUQ: pickDesignCountOutsideVoices,
|
||||
reviewCountCeiling: CEILING + 1,
|
||||
timeoutMs: 1_500_000,
|
||||
env: { QUESTION_TUNING: 'false', EXPLAIN_LEVEL: 'default' },
|
||||
|
||||
Reference in new issue
Block a user