Skip to content

Commit 22a5b64

Browse files
committed
Fix native Claude plan approval replay
1 parent ef77c50 commit 22a5b64

7 files changed

Lines changed: 254 additions & 23 deletions

File tree

AGENTS.md

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ For environments that don't run `npm install` after fetching the plugin (Claude
5353
- `opencode/bridge.js` — Spawns the binary with a fake HookEvent, parses the HookOutput response.
5454
- `opencode/config.js` — Reads `open-plan-annotator.json` config for implementation handoff settings.
5555
- `ui/` — React + Vite frontend, built to a single `build/index.html` embedded at compile time.
56-
- `hooks/hooks.json` — Claude Code hook registration. `SessionStart` runs `scripts/install-runtime.mjs` (runtime fetch) and `scripts/session-context.mjs` (injects plan-routing instructions into Claude's session context). `PreToolUse:ExitPlanMode` launches the annotator binary.
56+
- `hooks/hooks.json` — Claude Code hook registration. `SessionStart` runs `scripts/install-runtime.mjs` (runtime fetch) and `scripts/session-context.mjs` (injects plan-routing instructions into Claude's session context). `PreToolUse:ExitPlanMode` launches the annotator binary for Conductor-compatible denies; `PermissionRequest:ExitPlanMode` preserves native Claude Code approval behavior.
5757
- `skills/plan-review-triggers/SKILL.md` — Auto-loaded Claude Code skill with the full trigger heuristics. This is the long-form reference; the SessionStart context injection is the always-on nudge that keeps Claude from rationalizing past it.
5858

5959
## Critical Rules
@@ -99,14 +99,21 @@ bun run format # Format
9999

100100
## Hook Protocol
101101

102-
Claude Code sends a `HookEvent` JSON on stdin with `tool_input.plan` containing the plan markdown. The binary responds on stdout with a `HookOutput` JSON:
102+
Claude Code sends a `HookEvent` JSON on stdin with `tool_input.plan` containing the plan markdown. The binary responds on stdout with a `HookOutput` JSON matching the incoming hook event.
103+
104+
For `PreToolUse`:
103105

104106
- Approve: `{ hookSpecificOutput: { hookEventName: "PreToolUse", permissionDecision: "allow" } }`
105107
- Deny: `{ hookSpecificOutput: { hookEventName: "PreToolUse", permissionDecision: "deny", permissionDecisionReason: "..." } }`
106108

107-
The deny `permissionDecisionReason` contains serialized annotations (deletions, replacements, insertions, comments) as markdown so Claude can revise the plan.
109+
For `PermissionRequest`:
110+
111+
- Approve: `{ hookSpecificOutput: { hookEventName: "PermissionRequest", decision: { behavior: "allow" } } }`
112+
- Deny: `{ hookSpecificOutput: { hookEventName: "PermissionRequest", decision: { behavior: "deny", message: "..." } } }`
113+
114+
The deny feedback field (`permissionDecisionReason` or `decision.message`) contains serialized annotations (deletions, replacements, insertions, comments) as markdown so Claude can revise the plan.
108115

109-
The hook is registered on `PreToolUse` (not `PermissionRequest`) so it fires before the permission flow and regardless of `--permission-mode`. This makes Request Changes (`deny`) work across all hosts, including Conductor — which runs Claude Code with `--permission-prompt-tool stdio --permission-mode bypassPermissions` and never surfaces a `PermissionRequest` hook decision. (Note: in Conductor, exiting plan mode on approve is still owned by Conductor's own plan-approval UI, so an `allow` decision does not by itself leave plan mode there.)
116+
The hook is registered on both `PreToolUse` and `PermissionRequest`. `PreToolUse` fires before the permission flow and regardless of `--permission-mode`, which makes Request Changes (`deny`) work in Conductor. Native Claude Code still needs `PermissionRequest` to apply final plan approval. When both fire for the same tool call, the `PreToolUse` decision is cached briefly and replayed to `PermissionRequest` so the browser UI opens only once.
110117

111118
The OpenCode bridge (`opencode/bridge.js`) constructs the same `HookEvent` format and parses the same `HookOutput` response, so the binary always goes through the same code path.
112119

hooks/hooks.json

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,18 @@
2828
}
2929
]
3030
}
31+
],
32+
"PermissionRequest": [
33+
{
34+
"matcher": "ExitPlanMode",
35+
"hooks": [
36+
{
37+
"type": "command",
38+
"command": "node \"${CLAUDE_PLUGIN_ROOT}/bin/open-plan-annotator.mjs\"",
39+
"timeout": 345600
40+
}
41+
]
42+
}
3143
]
3244
}
3345
}

server/historyLifecycle.test.ts

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,82 @@ describe("stdout immediacy", () => {
244244
rmSync(tempRoot, { recursive: true, force: true });
245245
}
246246
}, 15000);
247+
248+
test("PermissionRequest replays a cached PreToolUse decision without reopening the UI", async () => {
249+
const tempRoot = mkdtempSync(join(tmpdir(), "open-plan-annotator-replay-"));
250+
const fakeBin = join(tempRoot, "bin");
251+
const configHome = join(tempRoot, "config");
252+
253+
try {
254+
mkdirSync(fakeBin, { recursive: true });
255+
mkdirSync(configHome, { recursive: true });
256+
seedUpdateCheckCache(configHome);
257+
258+
for (const name of ["open", "xdg-open", "cmd"]) {
259+
const shimPath = join(fakeBin, name);
260+
writeFileSync(shimPath, "#!/bin/sh\nexit 0\n", "utf8");
261+
chmodSync(shimPath, 0o755);
262+
}
263+
264+
const hookEvent = {
265+
transcript_path: join(tempRoot, "transcript.jsonl"),
266+
session_id: `session-${tempRoot}`,
267+
cwd: "/repo",
268+
permission_mode: "acceptEdits",
269+
hook_event_name: "PreToolUse",
270+
tool_name: "ExitPlanMode",
271+
tool_use_id: "tool-replay",
272+
};
273+
const env = {
274+
NODE_ENV: "test",
275+
XDG_CONFIG_HOME: configHome,
276+
SHUTDOWN_DELAY_MS: "100",
277+
PATH: `${fakeBin}${delimiter}${process.env.PATH ?? ""}`,
278+
};
279+
280+
await runSession({ decision: "approve", plan: "Replay plan", env, hookEvent });
281+
282+
const child = spawn(process.execPath, ["run", "server/index.ts"], {
283+
cwd: join(import.meta.dir, ".."),
284+
env: { ...process.env, ...env },
285+
stdio: ["pipe", "pipe", "pipe"],
286+
});
287+
288+
let stdout = "";
289+
let stderr = "";
290+
child.stdout.on("data", (chunk) => {
291+
stdout += String(chunk);
292+
});
293+
child.stderr.on("data", (chunk) => {
294+
stderr += String(chunk);
295+
});
296+
297+
const childExit = new Promise<number>((resolve, reject) => {
298+
child.once("error", reject);
299+
child.once("close", (code) => resolve(code ?? -1));
300+
});
301+
302+
const payload = {
303+
...hookEvent,
304+
hook_event_name: "PermissionRequest",
305+
tool_input: { plan: "Replay plan" },
306+
};
307+
child.stdin.write(`${JSON.stringify(payload)}\n`);
308+
child.stdin.end();
309+
310+
const exitCode = await Promise.race([childExit, Bun.sleep(3000).then(() => -2)]);
311+
expect(exitCode).toBe(0);
312+
expect(stderr).not.toContain("UI available at");
313+
314+
const output = JSON.parse(stdout.trim()) as {
315+
hookSpecificOutput: { hookEventName: "PermissionRequest"; decision: { behavior: "allow" | "deny" } };
316+
};
317+
expect(output.hookSpecificOutput.hookEventName).toBe("PermissionRequest");
318+
expect(output.hookSpecificOutput.decision.behavior).toBe("allow");
319+
} finally {
320+
rmSync(tempRoot, { recursive: true, force: true });
321+
}
322+
}, 15000);
247323
});
248324

249325
describe("generic review CLI mode", () => {

server/index.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
writeHookDecisionToStdout,
1313
writeReviewDecisionToStdout,
1414
} from "./runtime/decision.ts";
15+
import { consumeDecisionForPermissionRequest, storeDecisionForPermissionRequest } from "./runtime/decisionCache.ts";
1516
import { cleanupHistory, loadPlanHistory } from "./runtime/history.ts";
1617
import { loadHtmlContent } from "./runtime/html.ts";
1718
import { parseReviewRuntimeInput, parseRuntimeInput } from "./runtime/input.ts";
@@ -79,6 +80,15 @@ const runtimeInput = await (cliMode === "review"
7980
});
8081

8182
const { hookEvent, planContent } = runtimeInput;
83+
84+
if (runtimeInput.mode === "hook" && hookEvent.hook_event_name === "PermissionRequest") {
85+
const cachedDecision = await consumeDecisionForPermissionRequest(hookEvent);
86+
if (cachedDecision) {
87+
await writeHookDecisionToStdout(cachedDecision, hookEvent);
88+
process.exit(0);
89+
}
90+
}
91+
8292
const { configDir, historyRootDir, preferencesPath } = resolveRuntimePaths();
8393
const preferences = await loadPreferences(preferencesPath);
8494
const persistPreferences = createPreferencesPersister(preferencesPath, configDir);
@@ -135,7 +145,13 @@ await cleanupHistory(isDev, decision.approved, historyDir);
135145
if (runtimeInput.mode === "review") {
136146
await writeReviewDecisionToStdout(decision, runtimeInput.planPath ?? "");
137147
} else {
138-
await writeHookDecisionToStdout(decision);
148+
if (hookEvent.hook_event_name === "PreToolUse") {
149+
await storeDecisionForPermissionRequest(hookEvent, decision).catch((err: unknown) => {
150+
const message = err instanceof Error ? err.message : String(err);
151+
process.stderr.write(`open-plan-annotator: failed to cache hook decision: ${message}\n`);
152+
});
153+
}
154+
await writeHookDecisionToStdout(decision, hookEvent);
139155
}
140156

141157
const keepAliveMs = Number(process.env.SHUTDOWN_DELAY_MS) || 5000;

server/runtime/decision.ts

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type { HookOutput, ReviewOutput, ServerDecision } from "../types.ts";
1+
import type { HookEvent, HookOutput, ReviewOutput, ServerDecision } from "../types.ts";
22

33
export interface DecisionController {
44
decisionPromise: Promise<ServerDecision>;
@@ -14,19 +14,29 @@ export function createDecisionController(): DecisionController {
1414
return { decisionPromise, resolveDecision };
1515
}
1616

17-
export async function writeHookDecisionToStdout(decision: ServerDecision): Promise<void> {
18-
const output: HookOutput = {
19-
hookSpecificOutput: decision.approved
17+
export async function writeHookDecisionToStdout(decision: ServerDecision, hookEvent: HookEvent): Promise<void> {
18+
const output: HookOutput =
19+
hookEvent.hook_event_name === "PermissionRequest"
2020
? {
21-
hookEventName: "PreToolUse",
22-
permissionDecision: "allow",
21+
hookSpecificOutput: {
22+
hookEventName: "PermissionRequest",
23+
decision: decision.approved
24+
? { behavior: "allow" }
25+
: { behavior: "deny", message: decision.feedback ?? "Plan changes requested." },
26+
},
2327
}
2428
: {
25-
hookEventName: "PreToolUse",
26-
permissionDecision: "deny",
27-
permissionDecisionReason: decision.feedback ?? "Plan changes requested.",
28-
},
29-
};
29+
hookSpecificOutput: decision.approved
30+
? {
31+
hookEventName: "PreToolUse",
32+
permissionDecision: "allow",
33+
}
34+
: {
35+
hookEventName: "PreToolUse",
36+
permissionDecision: "deny",
37+
permissionDecisionReason: decision.feedback ?? "Plan changes requested.",
38+
},
39+
};
3040

3141
const jsonLine = `${JSON.stringify(output)}\n`;
3242
const { closeSync, writeSync } = await import("node:fs");

server/runtime/decisionCache.ts

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
import { createHash } from "node:crypto";
2+
import { mkdir, readdir, readFile, rm, writeFile } from "node:fs/promises";
3+
import { tmpdir } from "node:os";
4+
import { join } from "node:path";
5+
import type { HookEvent, ServerDecision } from "../types.ts";
6+
7+
const CACHE_TTL_MS = 10 * 60 * 1000;
8+
const CACHE_DIR = join(tmpdir(), "open-plan-annotator-decisions");
9+
10+
interface CachedDecision {
11+
approved: boolean;
12+
feedback?: string;
13+
createdAt: number;
14+
}
15+
16+
function cachePathForHookEvent(hookEvent: HookEvent): string {
17+
const key = JSON.stringify({
18+
sessionId: hookEvent.session_id,
19+
transcriptPath: hookEvent.transcript_path,
20+
cwd: hookEvent.cwd,
21+
toolName: hookEvent.tool_name,
22+
toolUseId: hookEvent.tool_use_id,
23+
});
24+
const digest = createHash("sha256").update(key).digest("hex");
25+
return join(CACHE_DIR, `${digest}.json`);
26+
}
27+
28+
function parseCachedDecision(value: unknown): CachedDecision | null {
29+
if (!value || typeof value !== "object") {
30+
return null;
31+
}
32+
33+
const cached = value as { approved?: unknown; feedback?: unknown; createdAt?: unknown };
34+
if (typeof cached.approved !== "boolean" || typeof cached.createdAt !== "number") {
35+
return null;
36+
}
37+
38+
if (cached.feedback !== undefined && typeof cached.feedback !== "string") {
39+
return null;
40+
}
41+
42+
return {
43+
approved: cached.approved,
44+
feedback: cached.feedback,
45+
createdAt: cached.createdAt,
46+
};
47+
}
48+
49+
async function cleanupExpiredCacheEntries(now = Date.now()): Promise<void> {
50+
let entries: string[];
51+
try {
52+
entries = await readdir(CACHE_DIR);
53+
} catch {
54+
return;
55+
}
56+
57+
await Promise.all(
58+
entries.map(async (entry) => {
59+
if (!entry.endsWith(".json")) {
60+
return;
61+
}
62+
63+
const path = join(CACHE_DIR, entry);
64+
try {
65+
const cached = parseCachedDecision(JSON.parse(await readFile(path, "utf8")));
66+
if (!cached || now - cached.createdAt > CACHE_TTL_MS) {
67+
await rm(path, { force: true });
68+
}
69+
} catch {
70+
await rm(path, { force: true });
71+
}
72+
}),
73+
);
74+
}
75+
76+
export async function storeDecisionForPermissionRequest(hookEvent: HookEvent, decision: ServerDecision): Promise<void> {
77+
await cleanupExpiredCacheEntries();
78+
await mkdir(CACHE_DIR, { recursive: true });
79+
const cached: CachedDecision = {
80+
approved: decision.approved,
81+
feedback: decision.feedback,
82+
createdAt: Date.now(),
83+
};
84+
await writeFile(cachePathForHookEvent(hookEvent), `${JSON.stringify(cached)}\n`, "utf8");
85+
}
86+
87+
export async function consumeDecisionForPermissionRequest(hookEvent: HookEvent): Promise<ServerDecision | null> {
88+
await cleanupExpiredCacheEntries();
89+
const path = cachePathForHookEvent(hookEvent);
90+
91+
try {
92+
const cached = parseCachedDecision(JSON.parse(await readFile(path, "utf8")));
93+
await rm(path, { force: true });
94+
95+
if (!cached || Date.now() - cached.createdAt > CACHE_TTL_MS) {
96+
return null;
97+
}
98+
99+
return { approved: cached.approved, feedback: cached.feedback };
100+
} catch {
101+
return null;
102+
}
103+
}

server/types.ts

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,20 @@ export interface UserPreferences {
2525
autoCloseOnSubmit: boolean;
2626
}
2727

28-
export interface HookOutput {
29-
hookSpecificOutput: {
30-
hookEventName: "PreToolUse";
31-
permissionDecision: "allow" | "deny";
32-
permissionDecisionReason?: string;
33-
};
34-
}
28+
export type HookOutput =
29+
| {
30+
hookSpecificOutput: {
31+
hookEventName: "PreToolUse";
32+
permissionDecision: "allow" | "deny";
33+
permissionDecisionReason?: string;
34+
};
35+
}
36+
| {
37+
hookSpecificOutput: {
38+
hookEventName: "PermissionRequest";
39+
decision: { behavior: "allow" } | { behavior: "deny"; message: string };
40+
};
41+
};
3542

3643
export interface HistoryKeySource {
3744
transcript_path?: unknown;

0 commit comments

Comments
 (0)