mirror of
https://github.com/garrytan/gstack.git
synced 2026-05-06 21:46:40 +02:00
9e244c0bed
* feat: plan-mode handshake for interactive review skills Add a preamble-level STOP-Ask handshake that fires when the user invokes any of the 4 interactive review skills (plan-ceo-review, plan-eng-review, plan-design-review, plan-devex-review) while their Claude Code session is in plan mode. Without this gate, plan mode's "this supercedes any other instructions" system-reminder outranked the skills' interactive STOP gates and the skills silently wrote plan files without any per-finding AskUserQuestion. The handshake offers 2 options (exit-and-rerun, cancel) — the original third "stay and batch" option was dropped after two independent reviewers flagged it as a silent bypass of the skills' anti-skip rule. Architecture decisions (CEO+Eng review): - Preamble-level resolver, not per-template injection (Codex finding #2) - Position 1 in preamble composition: after bash block (_SESSION_ID live), before onboarding AskUserQuestion gates (so fresh-install users see the handshake first, not drowned in telemetry/proactive/routing prompts) - Generator-only `interactive: true` frontmatter flag, following the `preamble-tier` precedent (no host-config frontmatter allowlist edits) - Host-scoped to Claude via `ctx.host === 'claude'` check inside the resolver (simpler than `suppressedResolvers` which only gates `{{}}` placeholders) - One-way-door classification in scripts/question-registry.ts for all 4 skills so question-tuning `never-ask` preferences can't suppress the gate - Synchronous telemetry write to ~/.gstack/analytics/skill-usage.jsonl on handshake fire (captures A-exit and C-cancel outcomes that terminate the skill before end-of-run telemetry runs) Also adds an explicit STOP block to plan-ceo-review Step 0C-bis so the approach-selection question can't silently skip to mode selection. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * feat: extend agent-sdk-runner with canUseTool for AskUserQuestion interception Test harness at test/helpers/agent-sdk-runner.ts gains an optional `canUseTool` callback parameter. When a test supplies it, the harness flips `permissionMode` from `bypassPermissions` (overlay-harness default) to `default` so the SDK actually invokes the callback on every tool use, and auto-adds `AskUserQuestion` to `allowedTools` so Claude can fire it at all. Exports a `passThroughNonAskUserQuestion` helper so tests that only want to intercept AskUserQuestion can auto-allow every other tool with one line: `return passThroughNonAskUserQuestion(toolName, input)`. This is the foundation for D14 — every future interactive-skill E2E test can now assert on AskUserQuestion shape and routing. Previous E2E tests at `test/skill-e2e.test.ts` explicitly instructed the model to skip AskUserQuestion ("non-interactive run") which meant no test could actually verify the question content or routing. 6 new unit tests in test/agent-sdk-runner.test.ts cover: - permissionMode flips to 'default' when canUseTool supplied - permissionMode stays 'bypassPermissions' when canUseTool absent - canUseTool callback reaches the SDK options - AskUserQuestion auto-added to allowedTools when canUseTool supplied - AskUserQuestion NOT added when canUseTool absent - passThroughNonAskUserQuestion helper returns allow+updatedInput Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * test: plan-mode handshake E2E coverage and unit assertions Adds 6 E2E test files and 8 new unit assertions to verify the plan-mode handshake works end-to-end and stays correct under regeneration. E2E tests (gate-tier, paid, EVALS=1 EVALS_TIER=gate): - test/skill-e2e-plan-ceo-plan-mode.test.ts — handshake fires before any Write/Edit when plan-mode distinctive phrase is present; 2-option shape (Exit/Cancel); option A routes to ExitPlanMode cleanly - test/skill-e2e-plan-eng-plan-mode.test.ts — same contract for plan-eng - test/skill-e2e-plan-design-plan-mode.test.ts — same contract for plan-design; exercises C-cancel branch instead of A-exit - test/skill-e2e-plan-devex-plan-mode.test.ts — same contract for plan-devex - test/skill-e2e-plan-mode-no-op.test.ts — negative regression: handshake must NOT fire when distinctive phrase is absent; skill proceeds normally through Step 0 (REGRESSION RULE guardrail against breaking existing interactive-review sessions) - test/e2e-harness-audit.test.ts — free unit test asserting every `interactive: true` skill has at least one canUseTool-using test file (prevents future drift where a skill opts in without coverage) Shared helper test/helpers/plan-mode-handshake-helpers.ts centralizes the canUseTool interceptor + distinctive-phrase injection so the 4 sibling E2E tests are thin wiring (~20 LOC each) and can't drift out of sync. Unit assertions added to test/gen-skill-docs.test.ts: - handshake section present in all 4 Claude-generated SKILL.md files - handshake section absent from non-interactive Claude skills (ship, review, qa, office-hours, codex, retro, cso) - handshake section absent from non-Claude host outputs (.agents, etc.) - 0C-bis STOP block present in plan-ceo-review/SKILL.md at correct position (between the "Present these approach options" line and "### 0D-prelude" header) - handshake resolver wired BEFORE generateUpgradeCheck in preamble composition order 6 new gate-tier entries added to test/helpers/touchfiles.ts so any change to the handshake resolver, preamble composition, skill templates, question registry, one-way-door classifier, or agent-sdk-runner fires the relevant E2E tests. test/touchfiles.test.ts updated for the new selection count (plan-ceo-review/** now triggers 15 tests, up from 8). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * chore(v1.11.1.0): VERSION bump + CHANGELOG entry + TODOS follow-ups Bumps from main's v1.11.0.0 to v1.11.1.0 (PATCH — bug-fix release, no new user-facing artifacts). CHANGELOG entry covers the plan-mode handshake, agent-sdk-runner canUseTool extension, and the 2 follow-up TODOs. CHANGELOG order: v1.11.1.0 (this) → v1.11.0.0 (workspace-aware ship, merged from main) → v1.10.1.0 (overlay efficacy harness). No duplicate headers. Syncs package.json version to match VERSION per the Step 12 idempotency invariant (both files must agree or /ship halts). TODOS.md: - Preserves the Testing/security-bench-haiku-responses P1 added on main - Adds P1 "Structural STOP-Ask forcing function" — broader class of the bug this release fixes - Adds P2 "Apply interactive: true to non-review skills (office-hours, codex, investigate, qa, retro, cso)" Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
167 lines
6.3 KiB
TypeScript
167 lines
6.3 KiB
TypeScript
/**
|
|
* Shared helpers for plan-mode handshake E2E tests.
|
|
*
|
|
* Four sibling test files (plan-ceo, plan-eng, plan-design, plan-devex) exercise
|
|
* the identical handshake contract against different skills. This helper
|
|
* centralizes the canUseTool interceptor and the assertion shape so the four
|
|
* test files are thin wiring (~40 LOC each) and can't drift out of sync.
|
|
*
|
|
* See scripts/resolvers/preamble/generate-plan-mode-handshake.ts for the
|
|
* handshake prose that the tests below assert against.
|
|
*/
|
|
|
|
import { expect } from 'bun:test';
|
|
import * as fs from 'fs';
|
|
import * as path from 'path';
|
|
import * as os from 'os';
|
|
import { execSync } from 'child_process';
|
|
import {
|
|
runAgentSdkTest,
|
|
passThroughNonAskUserQuestion,
|
|
resolveClaudeBinary,
|
|
type AgentSdkResult,
|
|
} from './agent-sdk-runner';
|
|
|
|
/** Distinctive phrase matching what Claude Code's harness actually injects. */
|
|
export const PLAN_MODE_REMINDER =
|
|
'Plan mode is active. The user indicated that they do not want you to execute yet';
|
|
|
|
export interface HandshakeCaptureResult {
|
|
sdkResult: AgentSdkResult;
|
|
/** Each AskUserQuestion that fired, with its input payload. */
|
|
askUserQuestions: Array<{ input: Record<string, unknown>; orderIndex: number }>;
|
|
/** Tool-use events in the order they fired (names only). */
|
|
toolOrder: string[];
|
|
/** Whether any Write or Edit tool fired BEFORE the first AskUserQuestion. */
|
|
writeOrEditBeforeAsk: boolean;
|
|
}
|
|
|
|
/**
|
|
* Run a skill via the Agent SDK with canUseTool intercepting every tool use.
|
|
* Inject the plan-mode distinctive phrase into the system prompt and auto-
|
|
* answer the handshake with the given answerLabel ("Exit" or "Cancel"). Return
|
|
* the captured events for assertion.
|
|
*/
|
|
export async function runPlanModeHandshakeTest(opts: {
|
|
/** Skill name, e.g. 'plan-ceo-review'. */
|
|
skillName: string;
|
|
/** "Exit" to pick option A (exit-and-rerun) or "Cancel" for option C. */
|
|
answerLabel: 'Exit' | 'Cancel';
|
|
/** If true, DO NOT inject the reminder — used by the no-op regression test. */
|
|
omitPlanModeReminder?: boolean;
|
|
/** Max turns for the SDK call (default 4 — handshake + exit should fit easily). */
|
|
maxTurns?: number;
|
|
}): Promise<HandshakeCaptureResult> {
|
|
const { skillName, answerLabel, omitPlanModeReminder, maxTurns } = opts;
|
|
|
|
const askUserQuestions: HandshakeCaptureResult['askUserQuestions'] = [];
|
|
const toolOrder: string[] = [];
|
|
let toolIndex = 0;
|
|
let firstAskIndex = -1;
|
|
|
|
const workingDir = fs.mkdtempSync(
|
|
path.join(os.tmpdir(), `plan-mode-handshake-${skillName}-`),
|
|
);
|
|
|
|
// The SDK requires AskUserQuestion to be in the allowed tools list. The
|
|
// harness auto-adds it when canUseTool is supplied, but we also want Read
|
|
// so the skill can load its own file if it tries to.
|
|
const binary = resolveClaudeBinary();
|
|
|
|
try {
|
|
// Inject the distinctive phrase into the system prompt by appending it to
|
|
// the default Claude Code preset. Claude Code's real plan mode uses an
|
|
// injected system-reminder; in SDK tests we use systemPrompt.append which
|
|
// the model treats as equally authoritative.
|
|
const reminderAppend = omitPlanModeReminder
|
|
? ''
|
|
: `\n\n<system-reminder>\n${PLAN_MODE_REMINDER}. This supercedes any other instructions you have received.\n</system-reminder>\n`;
|
|
|
|
const sdkResult = await runAgentSdkTest({
|
|
systemPrompt: {
|
|
type: 'preset',
|
|
preset: 'claude_code',
|
|
append: reminderAppend,
|
|
},
|
|
userPrompt: `Read the skill file at ${path.resolve(
|
|
import.meta.dir,
|
|
'..',
|
|
'..',
|
|
skillName,
|
|
'SKILL.md',
|
|
)} and follow its instructions. There is no real plan to review — just start the skill and respond to any AskUserQuestion that fires.`,
|
|
workingDirectory: workingDir,
|
|
maxTurns: maxTurns ?? 4,
|
|
allowedTools: ['Read', 'Grep', 'Glob', 'Bash'],
|
|
...(binary ? { pathToClaudeCodeExecutable: binary } : {}),
|
|
canUseTool: async (toolName, input) => {
|
|
toolOrder.push(toolName);
|
|
if (toolName === 'AskUserQuestion') {
|
|
if (firstAskIndex === -1) firstAskIndex = toolIndex;
|
|
askUserQuestions.push({ input, orderIndex: toolIndex });
|
|
toolIndex++;
|
|
// Auto-answer with the label the test specified.
|
|
const q = (input.questions as Array<{ question: string; options: Array<{ label: string }> }>)[0];
|
|
const matched = q.options.find((o) => o.label.includes(answerLabel));
|
|
const answer = matched ? matched.label : q.options[0]!.label;
|
|
return {
|
|
behavior: 'allow',
|
|
updatedInput: {
|
|
questions: input.questions,
|
|
answers: { [q.question]: answer },
|
|
},
|
|
};
|
|
}
|
|
toolIndex++;
|
|
return passThroughNonAskUserQuestion(toolName, input);
|
|
},
|
|
});
|
|
|
|
const writeOrEditBeforeAsk =
|
|
firstAskIndex > 0 &&
|
|
toolOrder.slice(0, firstAskIndex).some((t) => t === 'Write' || t === 'Edit');
|
|
|
|
return { sdkResult, askUserQuestions, toolOrder, writeOrEditBeforeAsk };
|
|
} finally {
|
|
try {
|
|
fs.rmSync(workingDir, { recursive: true, force: true });
|
|
} catch { /* ignore cleanup errors */ }
|
|
}
|
|
}
|
|
|
|
/** Assert the shape of a fired handshake AskUserQuestion. */
|
|
export function assertHandshakeShape(
|
|
aq: { input: Record<string, unknown> },
|
|
): void {
|
|
const questions = aq.input.questions as Array<{
|
|
question: string;
|
|
options: Array<{ label: string }>;
|
|
}>;
|
|
expect(questions).toBeDefined();
|
|
expect(questions.length).toBe(1);
|
|
const q = questions[0]!;
|
|
// D8 dropped Option B; handshake has exactly 2 options.
|
|
expect(q.options.length).toBe(2);
|
|
const labels = q.options.map((o) => o.label);
|
|
expect(labels.some((l) => l.includes('Exit'))).toBe(true);
|
|
expect(labels.some((l) => l.includes('Cancel'))).toBe(true);
|
|
}
|
|
|
|
/** Read the skill-usage.jsonl log and return handshake entries. */
|
|
export function readHandshakeLog(): Array<Record<string, unknown>> {
|
|
const logPath = path.join(os.homedir(), '.gstack', 'analytics', 'skill-usage.jsonl');
|
|
if (!fs.existsSync(logPath)) return [];
|
|
const lines = fs.readFileSync(logPath, 'utf-8').split('\n').filter(Boolean);
|
|
return lines
|
|
.map((line) => {
|
|
try {
|
|
return JSON.parse(line);
|
|
} catch {
|
|
return null;
|
|
}
|
|
})
|
|
.filter((x): x is Record<string, unknown> => x !== null && x.event === 'plan_mode_handshake');
|
|
}
|
|
|
|
export { execSync };
|