diff --git a/AGENTS.md b/AGENTS.md index 8b6cdd7..c74c0b0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -53,7 +53,7 @@ For environments that don't run `npm install` after fetching the plugin (Claude - `opencode/bridge.js` — Spawns the binary with a fake HookEvent, parses the HookOutput response. - `opencode/config.js` — Reads `open-plan-annotator.json` config for implementation handoff settings. - `ui/` — React + Vite frontend, built to a single `build/index.html` embedded at compile time. -- `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). `PermissionRequest:ExitPlanMode` launches the annotator binary. +- `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. - `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. ## Critical Rules @@ -101,10 +101,12 @@ bun run format # Format 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: -- Approve: `{ hookSpecificOutput: { hookEventName: "PermissionRequest", decision: { behavior: "allow" } } }` -- Deny: `{ hookSpecificOutput: { hookEventName: "PermissionRequest", decision: { behavior: "deny", message: "..." } } }` +- Approve: `{ hookSpecificOutput: { hookEventName: "PreToolUse", permissionDecision: "allow" } }` +- Deny: `{ hookSpecificOutput: { hookEventName: "PreToolUse", permissionDecision: "deny", permissionDecisionReason: "..." } }` -The deny message contains serialized annotations (deletions, replacements, insertions, comments) as markdown so Claude can revise the plan. +The deny `permissionDecisionReason` contains serialized annotations (deletions, replacements, insertions, comments) as markdown so Claude can revise the plan. + +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.) 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. diff --git a/hooks/hooks.json b/hooks/hooks.json index 773d7ae..33db315 100644 --- a/hooks/hooks.json +++ b/hooks/hooks.json @@ -17,7 +17,7 @@ ] } ], - "PermissionRequest": [ + "PreToolUse": [ { "matcher": "ExitPlanMode", "hooks": [ diff --git a/opencode/bridge.js b/opencode/bridge.js index d3f37b8..879f746 100644 --- a/opencode/bridge.js +++ b/opencode/bridge.js @@ -21,14 +21,14 @@ export async function runPlanReview(options) { detached: true, }); - const decision = output.hookSpecificOutput.decision; + const { permissionDecision, permissionDecisionReason } = output.hookSpecificOutput; - if (decision.behavior === "allow") { + if (permissionDecision === "allow") { return { approved: true }; } return { approved: false, - feedback: decision.message, + feedback: permissionDecisionReason, }; } diff --git a/server/historyLifecycle.test.ts b/server/historyLifecycle.test.ts index 9b1f8e2..7b207c5 100644 --- a/server/historyLifecycle.test.ts +++ b/server/historyLifecycle.test.ts @@ -124,13 +124,13 @@ async function runSession(args: { } const output = JSON.parse(stdout.trim()) as { - hookSpecificOutput: { decision: { behavior: "allow" | "deny" } }; + hookSpecificOutput: { permissionDecision: "allow" | "deny" }; }; return { version: planJson.version, history: planJson.history, - outputBehavior: output.hookSpecificOutput.decision.behavior, + outputBehavior: output.hookSpecificOutput.permissionDecision, }; } finally { if (child.exitCode === null) { @@ -166,7 +166,7 @@ describe("stdout immediacy", () => { session_id: "session-stdout", cwd: "/repo", permission_mode: "acceptEdits", - hook_event_name: "PermissionRequest", + hook_event_name: "PreToolUse", tool_name: "Write", tool_use_id: "tool-stdout", }; @@ -231,7 +231,7 @@ describe("stdout immediacy", () => { // Verify it's valid hook output const output = JSON.parse(stdout.trim()); - expect(output.hookSpecificOutput.decision.behavior).toBe("allow"); + expect(output.hookSpecificOutput.permissionDecision).toBe("allow"); // Clean up: kill the process (it would otherwise wait for the shutdown delay) child.kill("SIGTERM"); @@ -361,7 +361,7 @@ describe("history lifecycle", () => { session_id: "session-abc", cwd: "/repo", permission_mode: "acceptEdits", - hook_event_name: "PermissionRequest", + hook_event_name: "PreToolUse", tool_name: "Write", tool_use_id: "tool-1", }; diff --git a/server/runtime/decision.ts b/server/runtime/decision.ts index 2da0574..eb1c9ab 100644 --- a/server/runtime/decision.ts +++ b/server/runtime/decision.ts @@ -16,12 +16,16 @@ export function createDecisionController(): DecisionController { export async function writeHookDecisionToStdout(decision: ServerDecision): Promise { const output: HookOutput = { - hookSpecificOutput: { - hookEventName: "PermissionRequest", - decision: decision.approved - ? { behavior: "allow" } - : { behavior: "deny", message: decision.feedback ?? "Plan changes requested." }, - }, + hookSpecificOutput: decision.approved + ? { + hookEventName: "PreToolUse", + permissionDecision: "allow", + } + : { + hookEventName: "PreToolUse", + permissionDecision: "deny", + permissionDecisionReason: decision.feedback ?? "Plan changes requested.", + }, }; const jsonLine = `${JSON.stringify(output)}\n`; diff --git a/server/runtime/input.ts b/server/runtime/input.ts index d9a5c05..53bd501 100644 --- a/server/runtime/input.ts +++ b/server/runtime/input.ts @@ -30,7 +30,7 @@ function buildDevHookEvent(): HookEvent { transcript_path: "", cwd: process.cwd(), permission_mode: "default", - hook_event_name: "PermissionRequest", + hook_event_name: "PreToolUse", tool_name: "ExitPlanMode", tool_use_id: "dev-tool-use", tool_input: { plan: DEV_PLAN }, diff --git a/server/types.ts b/server/types.ts index 28b8370..5809b7e 100644 --- a/server/types.ts +++ b/server/types.ts @@ -27,8 +27,9 @@ export interface UserPreferences { export interface HookOutput { hookSpecificOutput: { - hookEventName: "PermissionRequest"; - decision: { behavior: "allow" } | { behavior: "deny"; message: string }; + hookEventName: "PreToolUse"; + permissionDecision: "allow" | "deny"; + permissionDecisionReason?: string; }; } diff --git a/shared/piExtension.mjs b/shared/piExtension.mjs index 0ebca03..50679bd 100644 --- a/shared/piExtension.mjs +++ b/shared/piExtension.mjs @@ -51,10 +51,10 @@ async function reviewPlan(plan, ctx) { detached: true, }); - const decision = result.hookSpecificOutput.decision; + const { permissionDecision, permissionDecisionReason } = result.hookSpecificOutput; return { - approved: decision.behavior === "allow", - feedback: decision.behavior === "deny" ? decision.message : undefined, + approved: permissionDecision === "allow", + feedback: permissionDecision === "deny" ? permissionDecisionReason : undefined, }; } diff --git a/shared/planReview.mjs b/shared/planReview.mjs index 20fae12..2249692 100644 --- a/shared/planReview.mjs +++ b/shared/planReview.mjs @@ -9,8 +9,9 @@ const PKG_ROOT = fileURLToPath(new URL("..", import.meta.url)); /** * @typedef {{ * hookSpecificOutput: { - * hookEventName: "PermissionRequest", - * decision: { behavior: "allow" } | { behavior: "deny", message: string } + * hookEventName: "PreToolUse", + * permissionDecision: "allow" | "deny", + * permissionDecisionReason?: string * } * }} HookOutput */ @@ -24,7 +25,7 @@ export function buildHookPayload(options) { transcript_path: "", cwd: options.cwd ?? process.cwd(), permission_mode: "default", - hook_event_name: "PermissionRequest", + hook_event_name: "PreToolUse", tool_name: "ExitPlanMode", tool_use_id: randomUUID(), tool_input: { @@ -43,17 +44,18 @@ export function validateHookOutput(value) { } const output = /** @type {HookOutput} */ (value); - const decision = output?.hookSpecificOutput?.decision; + const hookSpecificOutput = output?.hookSpecificOutput; + const permissionDecision = hookSpecificOutput?.permissionDecision; - if (!decision || typeof decision !== "object" || typeof decision.behavior !== "string") { + if (!hookSpecificOutput || typeof permissionDecision !== "string") { throw new Error("missing decision in hook output"); } - if (decision.behavior === "allow") { + if (permissionDecision === "allow") { return output; } - if (decision.behavior === "deny" && typeof decision.message === "string") { + if (permissionDecision === "deny" && typeof hookSpecificOutput.permissionDecisionReason === "string") { return output; } diff --git a/shared/planReview.test.ts b/shared/planReview.test.ts index 74c99ef..9888aef 100644 --- a/shared/planReview.test.ts +++ b/shared/planReview.test.ts @@ -13,23 +13,23 @@ describe("planReview", () => { test("validateHookOutput accepts allow and deny decisions", () => { expect( validateHookOutput({ - hookSpecificOutput: { hookEventName: "PermissionRequest", decision: { behavior: "allow" } }, - }).hookSpecificOutput.decision.behavior, + hookSpecificOutput: { hookEventName: "PreToolUse", permissionDecision: "allow" }, + }).hookSpecificOutput.permissionDecision, ).toBe("allow"); expect( validateHookOutput({ - hookSpecificOutput: { hookEventName: "PermissionRequest", decision: { behavior: "deny", message: "no" } }, - }).hookSpecificOutput.decision.behavior, + hookSpecificOutput: { hookEventName: "PreToolUse", permissionDecision: "deny", permissionDecisionReason: "no" }, + }).hookSpecificOutput.permissionDecision, ).toBe("deny"); }); test("parseHookOutput finds hook JSON in noisy stdout", () => { const output = parseHookOutput( - 'open-plan-annotator: UI available at http://localhost:1234\n{"hookSpecificOutput":{"hookEventName":"PermissionRequest","decision":{"behavior":"allow"}}}', + 'open-plan-annotator: UI available at http://localhost:1234\n{"hookSpecificOutput":{"hookEventName":"PreToolUse","permissionDecision":"allow"}}', "", ); - expect(output.hookSpecificOutput.decision.behavior).toBe("allow"); + expect(output.hookSpecificOutput.permissionDecision).toBe("allow"); }); });