Skip to content

Commit 739cb5b

Browse files
ndom91claude
andcommitted
Defer native plan-mode approve so PermissionRequest exits plan mode
On approve, PreToolUse:ExitPlanMode returned permissionDecision "allow". In native Claude Code that resolves the tool's permission early and skips the plan-approval dialog, so the PermissionRequest hook — the only documented path that exits plan mode — never fires. Claude Code then falls back to the TUI prompt, forcing the user to approve a second time after clicking Approve in the browser UI. Gate the PreToolUse approve output on permission_mode: in native plan mode ("plan") emit permissionDecision "ask" instead of "allow". The dialog then fires, PermissionRequest replays the cached approval as {decision:{behavior:"allow"}}, and plan mode exits with no terminal prompt. Conductor (bypassPermissions) and OpenCode/pi (default) have no PermissionRequest, so they keep the immediate "allow". - server/types.ts: PreToolUse permissionDecision union += "ask" - server/runtime/decision.ts: defer approve when permission_mode === "plan" - server/historyLifecycle.test.ts: plan-mode defer + PermissionRequest replay test - AGENTS.md: document the deferral (with a don't-revert-to-allow warning) server/index.ts unchanged: the PreToolUse decision cache write is already ungated, so PermissionRequest can replay the "ask"-deferred approval. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent f2cf19c commit 739cb5b

4 files changed

Lines changed: 96 additions & 4 deletions

File tree

AGENTS.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,8 @@ The deny feedback field (`permissionDecisionReason` or `decision.message`) conta
115115

116116
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.
117117

118+
**Native plan-mode approve must be deferred, not allowed.** In native Claude Code the ExitPlanMode `PreToolUse` event arrives with `permission_mode: "plan"`. Returning `permissionDecision: "allow"` there resolves the tool's permission early and skips the plan-approval dialog — which means the `PermissionRequest` hook that actually exits plan mode never runs, and the user is forced to approve a second time in the TUI. So on approve in `permission_mode: "plan"` the binary emits `permissionDecision: "ask"` (defer) instead of `allow`; the dialog then fires, `PermissionRequest` replays the cached approval as `{decision:{behavior:"allow"}}`, and plan mode exits with no terminal prompt. Every other host (Conductor `bypassPermissions`, OpenCode/pi `default`) has no `PermissionRequest`, so those keep the immediate `PreToolUse` `allow`. Do not "simplify" plan-mode approve back to `allow` — that reintroduces the double-approve.
119+
118120
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.
119121

120122
## Plan Review Trigger Rules

server/historyLifecycle.test.ts

Lines changed: 84 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ type SessionDecision = "approve" | "deny";
4545
interface SessionResult {
4646
version: number;
4747
history: string[];
48-
outputBehavior: "allow" | "deny";
48+
outputBehavior: "allow" | "deny" | "ask";
4949
}
5050

5151
function seedUpdateCheckCache(configHome: string) {
@@ -124,7 +124,7 @@ async function runSession(args: {
124124
}
125125

126126
const output = JSON.parse(stdout.trim()) as {
127-
hookSpecificOutput: { permissionDecision: "allow" | "deny" };
127+
hookSpecificOutput: { permissionDecision: "allow" | "deny" | "ask" };
128128
};
129129

130130
return {
@@ -321,6 +321,88 @@ describe("stdout immediacy", () => {
321321
rmSync(tempRoot, { recursive: true, force: true });
322322
}
323323
}, 15000);
324+
325+
test("plan-mode PreToolUse approve defers with ask, then PermissionRequest replays allow", async () => {
326+
const tempRoot = mkdtempSync(join(tmpdir(), "open-plan-annotator-planmode-"));
327+
const fakeBin = join(tempRoot, "bin");
328+
const configHome = join(tempRoot, "config");
329+
330+
try {
331+
mkdirSync(fakeBin, { recursive: true });
332+
mkdirSync(configHome, { recursive: true });
333+
seedUpdateCheckCache(configHome);
334+
335+
for (const name of ["open", "xdg-open", "cmd"]) {
336+
const shimPath = join(fakeBin, name);
337+
writeFileSync(shimPath, "#!/bin/sh\nexit 0\n", "utf8");
338+
chmodSync(shimPath, 0o755);
339+
}
340+
341+
const hookEvent = {
342+
transcript_path: join(tempRoot, "transcript.jsonl"),
343+
session_id: `session-${tempRoot}`,
344+
cwd: "/repo",
345+
permission_mode: "plan",
346+
hook_event_name: "PreToolUse",
347+
tool_name: "ExitPlanMode",
348+
tool_use_id: "tool-planmode",
349+
};
350+
const env = {
351+
NODE_ENV: "test",
352+
XDG_CONFIG_HOME: configHome,
353+
SHUTDOWN_DELAY_MS: "100",
354+
PATH: `${fakeBin}${delimiter}${process.env.PATH ?? ""}`,
355+
};
356+
357+
// Native CC plan mode: PreToolUse approve must defer with "ask" (not "allow"),
358+
// so the plan-approval dialog fires and PermissionRequest gets to exit plan mode.
359+
const preToolUse = await runSession({ decision: "approve", plan: "Plan-mode plan", env, hookEvent });
360+
expect(preToolUse.outputBehavior).toBe("ask");
361+
362+
// PermissionRequest then replays the cached approval as behavior "allow" without
363+
// reopening the browser — this is what actually exits plan mode with no TUI prompt.
364+
const child = spawn(process.execPath, ["run", "server/index.ts"], {
365+
cwd: join(import.meta.dir, ".."),
366+
env: { ...process.env, ...env },
367+
stdio: ["pipe", "pipe", "pipe"],
368+
});
369+
370+
let stdout = "";
371+
let stderr = "";
372+
child.stdout.on("data", (chunk) => {
373+
stdout += String(chunk);
374+
});
375+
child.stderr.on("data", (chunk) => {
376+
stderr += String(chunk);
377+
});
378+
379+
const childExit = new Promise<number>((resolve, reject) => {
380+
child.once("error", reject);
381+
child.once("close", (code) => resolve(code ?? -1));
382+
});
383+
384+
const payload = {
385+
...hookEvent,
386+
hook_event_name: "PermissionRequest",
387+
tool_use_id: "tool-planmode-permission",
388+
tool_input: { plan: "Plan-mode plan" },
389+
};
390+
child.stdin.write(`${JSON.stringify(payload)}\n`);
391+
child.stdin.end();
392+
393+
const exitCode = await Promise.race([childExit, Bun.sleep(3000).then(() => -2)]);
394+
expect(exitCode).toBe(0);
395+
expect(stderr).not.toContain("UI available at");
396+
397+
const output = JSON.parse(stdout.trim()) as {
398+
hookSpecificOutput: { hookEventName: "PermissionRequest"; decision: { behavior: "allow" | "deny" } };
399+
};
400+
expect(output.hookSpecificOutput.hookEventName).toBe("PermissionRequest");
401+
expect(output.hookSpecificOutput.decision.behavior).toBe("allow");
402+
} finally {
403+
rmSync(tempRoot, { recursive: true, force: true });
404+
}
405+
}, 15000);
324406
});
325407

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

server/runtime/decision.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,15 @@ export async function writeHookDecisionToStdout(decision: ServerDecision, hookEv
2929
hookSpecificOutput: decision.approved
3030
? {
3131
hookEventName: "PreToolUse",
32-
permissionDecision: "allow",
32+
// In native Claude Code plan mode, returning "allow" here resolves the
33+
// tool's permission early and skips the plan-approval dialog — which
34+
// means the PermissionRequest hook that actually exits plan mode never
35+
// runs, and the user is left to confirm again in the TUI. Defer with
36+
// "ask" so the dialog fires and PermissionRequest replays the cached
37+
// approval (see server/index.ts + decisionCache.ts). Other hosts
38+
// (Conductor bypassPermissions, OpenCode/pi default) have no
39+
// PermissionRequest, so they keep the immediate "allow".
40+
permissionDecision: hookEvent.permission_mode === "plan" ? "ask" : "allow",
3341
}
3442
: {
3543
hookEventName: "PreToolUse",

server/types.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ export type HookOutput =
2929
| {
3030
hookSpecificOutput: {
3131
hookEventName: "PreToolUse";
32-
permissionDecision: "allow" | "deny";
32+
permissionDecision: "allow" | "deny" | "ask";
3333
permissionDecisionReason?: string;
3434
};
3535
}

0 commit comments

Comments
 (0)