Skip to content

Commit 0176ee4

Browse files
euxaristiaclaude
andauthored
feat(agent): add PermissionModePlan for interactive read-only planning (#853)
* feat(agent): add PermissionModePlan for interactive read-only planning * fix(agent): drop redundant modeName branch, add plan mode regression tests string(permissionMode) already yields "plan" / "spec-draft", so the if/else recomputing modeName in the denial message was dead branching on the same values. Also add plan-mode coverage mirroring three of the four existing spec-draft regression tests: advertised tool set, and denied write_file/bash calls. The fourth (submit-and-stop review control) has no plan-mode analog, since plan mode has no submit tool. * fix(agent): deny request_permissions in plan/spec-draft even when the registry omits it request_permissions is dispatched by name in executeToolCall before the registry-based ToolAdvertised gate runs, so that gate only helps when the tool happens to be present in the caller's registry. A plan- or spec-draft-mode registry that simply omits the tool (rather than registering it as denied) let the call fall through to a real turn/session-scoped permission grant, defeating the read-only boundary. Deny it unconditionally at the top of executeRequestPermissions for both read-only modes instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(agent): suppress executable hooks while plan/spec-draft mode is active Plan mode promises a read-only turn, but sessionStart/sessionEnd fire on every run and beforeTool/afterTool fire around allowed read calls, and all four execute configured host commands outside the advertised-tool and sandbox gates — so a project hook could mutate the workspace or spawn a process from a session that advertises it cannot. Gate all four dispatch points on the run's permission mode, with a regression test asserting no hook command launches during a plan-mode run. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(agent): close plan-mode tool advertisement bypass toolAdvertisedInPlan whitelisted ask_user and update_plan by name alone, so a caller could register a mutating tool under either name and have it advertised and executed in plan mode. Validate every tool against its Safety() instead. Also exclude lsp_navigate, which is marked SideEffectRead but lazily spawns a real language-server process, contradicting plan mode's read-only guarantee. Add regression tests for both. * fix(agent): keep trust-gated hooks in spec-draft mode Plan mode still suppresses executable hooks so a read-only planning turn cannot spawn host processes via session or tool hooks. Spec-draft keeps the existing trust model: project hooks fire when the workspace (or its worktree trust root) is trusted. Unconditionally suppressing hooks in spec-draft broke TestExecSpecWorktreeInheritsTrustEndToEnd for trusted worktrees under --use-spec --worktree. * fix(agent): cover plan-mode spoof and lsp_navigate denials Expand the name-only spoof regression to both update_plan and ask_user, and add an execution-path denial for lsp_navigate so plan mode cannot spawn language servers even when a call still arrives. * fix(agent): filter tool_search deferred candidates by plan/spec-draft visibility tool_search resolved and ranked deferred tools by EnabledTools/DisabledTools only, never by the run's permission-mode visibility. tool_search itself is already denied at dispatch in plan/spec-draft (its Safety carries no side effect, so it fails the same SideEffect==Read advertisement gate direct calls use), so this was not reachable through the normal Run() path today. But it is a real landmine: if that outer gate is ever loosened independently (e.g. tool_search's no-side-effect Safety is judged advertisable), the loader had no gate of its own and would hand a deferred write/mutator tool's name, description, and full schema straight to a plan/spec-draft model. Mirror agent.ToolAdvertised's plan/spec-draft branches inside the tools package (toolAdvertisedForPermissionMode, next to the existing toolAllowedByFilters mirror that avoids the same import cycle) and apply it alongside the operator filters in visibleDeferredTools and visibleEagerToolNames. Added unit tests in tool_search_test.go for both modes, and an end-to-end agent test that force-calls tool_search in plan mode and asserts no schema leaks. Verified by reverting tool_search.go and confirming the new tests fail (one shows load_tools resolving to the mutator's name); restored and confirmed they pass. Also confirmed via a temporary probe that if plan mode's outer advertisement gate is loosened, this filter is what actually stops the leak. * fix(agent): require Safety for spec-draft ask_user/submit_spec Do not advertise or load re-registered control tools by name alone in spec-draft mode. ask_user must be SideEffectRead+Allow and submit_spec must be SideEffectWrite+Allow, matching the real tools. Apply the same filter in tool_search and add spoof regression tests. Refs #642 * fix(agent): wire plan mode into TUI, CLI, and ACP entry points Address the P1 finding that PermissionModePlan was documented but never selected by /plan, zero exec, or ACP mode selectors. /plan on|off now toggles the session permission mode (restoring the prior mode on off), zero exec --plan selects plan for a run, and ACP advertises plan as a client-selectable mode. Integration coverage for each entry path. * fix(cli): reject --plan combined with --worktree Worktree preparation runs in runExec before the plan permission mode is assigned, so `zero exec --plan --worktree` could still trigger workspace mutation ahead of the read-only gate. Reject the combination during option validation, alongside the existing --use-spec/--skip-permissions- unsafe conflict checks, so no worktree prep can occur. Addresses a coderabbitai finding on PR #642. * test(tools): assert spoofed tool schema doesn't leak via tool_search The spoofed-control-tool regression only asserted on the description string; Parameters() exposed no distinctive schema marker, so a regression that leaked the schema without the description would still have passed. Add a spoofed_secret property to the test tool's schema and assert it's absent from result.Output alongside the description. Addresses a coderabbitai finding on PR #642. * fix(tui): gate local mutating commands behind plan mode /plan on only flips the agent permission mode, which gates agent tool calls. Local TUI commands that run entirely inside the TUI process bypass that gate: /rewind restores workspace files from a checkpoint, /export writes a transcript to disk, and /sandbox-setup spawns a native host process. Add a shared plan-mode guard at the start of dispatchCommand (mirroring the existing BTW-unavailable guard) that rejects these three commands while permissionMode is agent.PermissionModePlan, with regression coverage proving each is blocked with no mutation/process spawn in plan mode and unaffected outside it. Addresses a coderabbitai finding on PR #642. * fix(agent,specialist): layer plan mode system prompt and enforce read-only subagent mode * Fix jatmn review findings for PR 642 * fix(agent,cli,tui): resolve CodeRabbit review comments on active turn model field and --plan permission mode conflict * fix(agent): propagate permission mode to exec options and serialize ACP mode changes Parse permissionMode in resolveExecPermissionMode, acquire turnMu.Lock in ACP handleSetMode to serialize mode changes with active turns, and block MCP subcommands in TUI plan mode. Refs #642 * fix(agent,tui): update tests off removed tools.CoreTools/NewWriteFileTool/NewLSPNavigateTool wrappers Those were thin unscoped wrappers around the Scoped variants, deleted upstream in #706 since nothing else called them directly. Only these tests still did; switch to the Scoped calls main's own tests already use. * fix(agent): fail-closed beforeTool vetoes and permission-mode plan guards Keep beforeTool deny gates active under plan mode so hooksSuppressed no longer fails open, and stop propagating --permission-mode for auto/ask/member children so swarm members keep write tools. Apply --plan combination rejects to --permission-mode plan as well. Refs #853 * fix(agent): address CodeRabbit findings for plan-mode advertisement and entry paths Unify plan/spec-draft tool advertisement in tools.ToolAdvertisedForPermissionMode (with PermissionDeny short-circuit), cover ACP config and --permission-mode plan list-tools paths, and allow bare /mcp while blocking mutating MCP subcommands. Refs #853 --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: euxaristia <euxaristia@users.noreply.github.com>
1 parent 160e3be commit 0176ee4

25 files changed

Lines changed: 1926 additions & 55 deletions

‎internal/acp/agent.go‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -349,9 +349,11 @@ func (a *Agent) handleSetMode(_ context.Context, params json.RawMessage) (any, e
349349
if sess == nil {
350350
return nil, RPCError(codeInvalidParams, "unknown session: "+p.SessionID)
351351
}
352+
sess.turnMu.Lock()
353+
defer sess.turnMu.Unlock()
352354
mode := agent.PermissionMode(p.ModeID)
353355
switch mode {
354-
case agent.PermissionModeAuto, agent.PermissionModeAsk:
356+
case agent.PermissionModeAuto, agent.PermissionModeAsk, agent.PermissionModePlan:
355357
sess.setMode(mode)
356358
(&notifier{conn: a.conn, sessionID: sess.id}).currentMode(string(mode))
357359
return SetSessionModeResult{}, nil
@@ -381,9 +383,13 @@ func (a *Agent) handleSetConfigOption(_ context.Context, params json.RawMessage)
381383
return nil, err
382384
}
383385
case configIDMode:
386+
// Same turnMu as handleSetMode so the two advertised mode doors (session
387+
// set_mode and set_config_option) serialize mode flips consistently.
388+
sess.turnMu.Lock()
389+
defer sess.turnMu.Unlock()
384390
mode := agent.PermissionMode(p.Value)
385391
switch mode {
386-
case agent.PermissionModeAuto, agent.PermissionModeAsk:
392+
case agent.PermissionModeAuto, agent.PermissionModeAsk, agent.PermissionModePlan:
387393
sess.setMode(mode)
388394
(&notifier{conn: a.conn, sessionID: sess.id}).currentMode(string(mode))
389395
case agent.PermissionModeUnsafe:
@@ -439,13 +445,16 @@ func (a *Agent) handleCancel(_ context.Context, params json.RawMessage) {
439445
// ---- advertising helpers ----
440446

441447
func (a *Agent) modeState(s *acpSession) *SessionModeState {
442-
// Only auto/ask are offered over ACP; Unsafe is gated to the operator (see
443-
// handleSetMode) so a client can't grant itself no-prompt host access.
448+
// auto/ask/plan are offered over ACP; Unsafe is gated to the operator (see
449+
// handleSetMode) so a client can't grant itself no-prompt host access. Plan
450+
// only narrows what a client can do (read-only, no write/shell tools), so
451+
// unlike Unsafe there is no elevation risk in letting a client select it.
444452
return &SessionModeState{
445453
CurrentModeID: string(s.currentMode()),
446454
AvailableModes: []SessionMode{
447455
{ID: string(agent.PermissionModeAuto), Name: "Auto", Description: "Run safe tools automatically; ask before risky ones."},
448456
{ID: string(agent.PermissionModeAsk), Name: "Ask", Description: "Ask before every tool that changes state."},
457+
{ID: string(agent.PermissionModePlan), Name: "Plan", Description: "Read-only planning; write and shell tools are hidden."},
449458
},
450459
}
451460
}
@@ -516,6 +525,7 @@ func (a *Agent) configOptions(s *acpSession) []SessionConfigOption {
516525
Options: []SessionConfigOptionValue{
517526
{Value: string(agent.PermissionModeAuto), Name: "Auto", Description: "Run safe tools automatically; ask before risky ones."},
518527
{Value: string(agent.PermissionModeAsk), Name: "Ask", Description: "Ask before every tool that changes state."},
528+
{Value: string(agent.PermissionModePlan), Name: "Plan", Description: "Read-only planning; write and shell tools are hidden."},
519529
},
520530
}}
521531
}

‎internal/acp/agent_test.go‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -393,6 +393,32 @@ func TestACPSetModeUpdatesSession(t *testing.T) {
393393
if got := configured.ConfigOptions[1].CurrentValue; got != string(agent.PermissionModeAuto) {
394394
t.Fatalf("configured mode = %q", got)
395395
}
396+
// Plan is accepted: it only narrows capability (read-only), so unlike Unsafe
397+
// there is no elevation risk in letting a client select it.
398+
if err := h.client.Call(ctx, MethodSessionSetMode, SetSessionModeParams{SessionID: newRes.SessionID, ModeID: string(agent.PermissionModePlan)}, &SetSessionModeResult{}); err != nil {
399+
t.Fatalf("set_mode plan: %v", err)
400+
}
401+
// Configuration path is a separate contract from MethodSessionSetMode: the
402+
// mode option is advertised via configOptions and applied by handleSetConfigOption.
403+
var planConfigured SetSessionConfigOptionResult
404+
if err := h.client.Call(ctx, MethodSessionSetConfigOption, SetSessionConfigOptionParams{
405+
SessionID: newRes.SessionID, ConfigID: configIDMode, Value: string(agent.PermissionModePlan),
406+
}, &planConfigured); err != nil {
407+
t.Fatalf("set_config_option plan: %v", err)
408+
}
409+
if got := planConfigured.ConfigOptions[1].CurrentValue; got != string(agent.PermissionModePlan) {
410+
t.Fatalf("configured mode after plan = %q, want plan", got)
411+
}
412+
hasPlanOption := false
413+
for _, opt := range planConfigured.ConfigOptions[1].Options {
414+
if opt.Value == string(agent.PermissionModePlan) {
415+
hasPlanOption = true
416+
break
417+
}
418+
}
419+
if !hasPlanOption {
420+
t.Fatalf("config mode options missing plan: %#v", planConfigured.ConfigOptions[1].Options)
421+
}
396422
// Unsafe must be rejected over ACP — a client can't self-grant no-prompt host access.
397423
if err := h.client.Call(ctx, MethodSessionSetMode, SetSessionModeParams{SessionID: newRes.SessionID, ModeID: string(agent.PermissionModeUnsafe)}, &SetSessionModeResult{}); err == nil {
398424
t.Fatal("expected Unsafe mode to be rejected over ACP")
@@ -403,6 +429,37 @@ func TestACPSetModeUpdatesSession(t *testing.T) {
403429
}
404430
}
405431

432+
// TestACPPlanModeWiresPermissionModeIntoAgentOptions confirms selecting "plan"
433+
// over ACP actually reaches agent.Options.PermissionMode for the next turn —
434+
// the same gap this test's TUI counterpart covers for /plan on.
435+
func TestACPPlanModeWiresPermissionModeIntoAgentOptions(t *testing.T) {
436+
deps := testDeps(t)
437+
var captured agent.Options
438+
deps.RunAgent = func(_ context.Context, _ string, _ zeroruntime.Provider, opts agent.Options) (agent.Result, error) {
439+
captured = opts
440+
return agent.Result{FinalAnswer: "ok"}, nil
441+
}
442+
443+
h := newHarness(t, deps)
444+
defer h.stop()
445+
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
446+
defer cancel()
447+
448+
var newRes NewSessionResult
449+
if err := h.client.Call(ctx, MethodSessionNew, NewSessionParams{Cwd: t.TempDir(), McpServers: []McpServer{}}, &newRes); err != nil {
450+
t.Fatalf("session/new: %v", err)
451+
}
452+
if err := h.client.Call(ctx, MethodSessionSetMode, SetSessionModeParams{SessionID: newRes.SessionID, ModeID: string(agent.PermissionModePlan)}, &SetSessionModeResult{}); err != nil {
453+
t.Fatalf("set_mode plan: %v", err)
454+
}
455+
if err := h.client.Call(ctx, MethodSessionPrompt, PromptParams{SessionID: newRes.SessionID, Prompt: []ContentBlock{TextBlock("plan it out")}}, &PromptResult{}); err != nil {
456+
t.Fatalf("session/prompt: %v", err)
457+
}
458+
if captured.PermissionMode != agent.PermissionModePlan {
459+
t.Fatalf("agent.Options.PermissionMode = %q, want plan", captured.PermissionMode)
460+
}
461+
}
462+
406463
// TestACPRunTurnWiresSandboxAndScopedRegistry proves the sandbox engine and the
407464
// scoped registry from BuildWorkspace actually reach agent.Options — i.e. ACP
408465
// shell tools run confined, not unconfined on the host.

‎internal/agent/deferred_loop_test.go‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -797,3 +797,98 @@ func TestDisabledToolSearchFallsBackToEager(t *testing.T) {
797797
t.Fatalf("deferred tool must be callable under eager fallback, got status=%s output=%q", result.Status, result.Output)
798798
}
799799
}
800+
801+
// fakeDeferredMutatorTool is a deferred-eligible tool with mutating Safety
802+
// (SideEffectWrite), standing in for a real write/mutator MCP tool that would
803+
// be hidden behind tool_search once deferral activates.
804+
type fakeDeferredMutatorTool struct{ name string }
805+
806+
func (t fakeDeferredMutatorTool) Name() string { return t.name }
807+
func (t fakeDeferredMutatorTool) Description() string { return "mutates the workspace, deferred" }
808+
func (t fakeDeferredMutatorTool) Parameters() tools.Schema {
809+
return tools.Schema{Type: "object", AdditionalProperties: false}
810+
}
811+
func (t fakeDeferredMutatorTool) Safety() tools.Safety {
812+
return tools.Safety{SideEffect: tools.SideEffectWrite, Permission: tools.PermissionPrompt, Reason: "mutates"}
813+
}
814+
func (t fakeDeferredMutatorTool) Run(_ context.Context, _ map[string]any) tools.Result {
815+
return tools.Result{Status: tools.StatusOK, Output: "mutated"}
816+
}
817+
func (t fakeDeferredMutatorTool) Deferred() bool { return true }
818+
819+
// TestPlanModeToolSearchNeverLeaksDeferredMutatorSchema guards the concern
820+
// raised in PR #642 review (jatmn, P2): that tool_search filters deferred
821+
// candidates only by EnabledTools/DisabledTools, not by plan-mode visibility,
822+
// so a plan-mode model could call `tool_search select:<deferred write tool>`
823+
// and receive that tool's full schema even though a direct call to it is
824+
// correctly denied the following turn.
825+
//
826+
// tool_search's own Safety is SideEffectNone, and ToolAdvertisedForPermissionMode (the
827+
// same gate executeToolCall uses to deny a direct call) requires
828+
// SideEffect==Read to advertise a tool in plan mode. That means tool_search
829+
// itself is never advertised, never activates deferral (loaderUsable in
830+
// partitionToolsCached requires ToolAdvertised(loader, permissionMode)), and
831+
// is denied at dispatch like any other hidden tool if a stale/adversarial
832+
// call reaches it anyway — so a deferred mutator's schema can never reach the
833+
// model through tool_search while in plan mode. This test pins that
834+
// end-to-end: tool_search is absent from the advertised tool list, and a
835+
// forced call to it is denied before rendering any tool schema.
836+
func TestPlanModeToolSearchNeverLeaksDeferredMutatorSchema(t *testing.T) {
837+
root := t.TempDir()
838+
registry := tools.NewRegistry()
839+
registry.Register(tools.NewReadFileTool(root))
840+
registry.Register(fakeDeferredMutatorTool{name: "mcp__srv__mutate"})
841+
registry.Register(fakeDeferredMutatorTool{name: "mcp__srv__mutate2"})
842+
registry.Register(tools.NewToolSearchTool(registry))
843+
844+
provider := &mockProvider{turns: [][]zeroruntime.StreamEvent{
845+
{ // turn 1: force a call to tool_search even though it should not be advertised.
846+
{Type: zeroruntime.StreamEventToolCallStart, ToolCallID: "c1", ToolName: tools.ToolSearchToolName},
847+
{Type: zeroruntime.StreamEventToolCallDelta, ToolCallID: "c1", ArgumentsFragment: `{"query":"select:mcp__srv__mutate"}`},
848+
{Type: zeroruntime.StreamEventToolCallEnd, ToolCallID: "c1"},
849+
{Type: zeroruntime.StreamEventDone},
850+
},
851+
{ // turn 2: final answer.
852+
{Type: zeroruntime.StreamEventText, Content: "done"},
853+
{Type: zeroruntime.StreamEventDone},
854+
},
855+
}}
856+
857+
result, err := Run(context.Background(), "plan", provider, Options{
858+
Registry: registry,
859+
PermissionMode: PermissionModePlan,
860+
DeferThreshold: 2, // 2 deferred mutators registered => eligible for deferral.
861+
MaxTurns: 2,
862+
})
863+
if err != nil {
864+
t.Fatal(err)
865+
}
866+
867+
// tool_search (and the deferred mutators) must not be advertised in plan
868+
// mode's turn 1 tool list at all.
869+
for _, def := range provider.requests[0].Tools {
870+
if def.Name == tools.ToolSearchToolName {
871+
t.Fatalf("plan mode must not advertise tool_search, got %#v", provider.requests[0].Tools)
872+
}
873+
if def.Name == "mcp__srv__mutate" || def.Name == "mcp__srv__mutate2" {
874+
t.Fatalf("plan mode must not advertise a deferred mutator, got %#v", provider.requests[0].Tools)
875+
}
876+
}
877+
878+
// The forced call must be denied outright, never a loaded-schema result:
879+
// the tool result must not mention the mutator's name or carry a
880+
// load_tools signal.
881+
var toolMessage string
882+
for _, message := range result.Messages {
883+
if message.Role == zeroruntime.MessageRoleTool {
884+
toolMessage = message.Content
885+
break
886+
}
887+
}
888+
if !strings.Contains(toolMessage, "not available in plan mode") {
889+
t.Fatalf("expected tool_search call denied in plan mode, got %q", toolMessage)
890+
}
891+
if strings.Contains(toolMessage, "mcp__srv__mutate") {
892+
t.Fatalf("denial must not leak the deferred mutator's name/schema, got %q", toolMessage)
893+
}
894+
}

‎internal/agent/loop.go‎

Lines changed: 50 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1134,12 +1134,13 @@ func executeToolCall(ctx context.Context, registry *tools.Registry, call ToolCal
11341134
}, nil
11351135
}
11361136
tool, toolFound := registry.Get(call.Name)
1137-
if permissionMode == PermissionModeSpecDraft && toolFound && !ToolAdvertised(tool, permissionMode) {
1137+
if (permissionMode == PermissionModeSpecDraft || permissionMode == PermissionModePlan) && toolFound && !ToolAdvertised(tool, permissionMode) {
1138+
modeName := string(permissionMode)
11381139
return ToolResult{
11391140
ToolCallID: call.ID,
11401141
Name: call.Name,
11411142
Status: tools.StatusError,
1142-
Output: `Error: Tool "` + call.Name + `" is not available in spec-draft mode.`,
1143+
Output: `Error: Tool "` + call.Name + `" is not available in ` + modeName + ` mode.`,
11431144
DenialReason: DenialFiltered,
11441145
}, nil
11451146
}
@@ -1818,9 +1819,31 @@ func toolResultFromPrePermissionReject(call ToolCall, result tools.Result) ToolR
18181819
}
18191820
}
18201821

1822+
// hooksSuppressed reports whether advisory (non-veto) hooks must not run for
1823+
// this run's permission mode. Plan mode promises a read-only turn, but
1824+
// sessionStart/sessionEnd/afterTool hooks execute configured host commands
1825+
// outside the advertised-tool and sandbox gates, so dispatching them would let
1826+
// merely starting a plan session or finishing a read mutate the workspace.
1827+
//
1828+
// beforeTool is intentionally NOT suppressed: a non-zero exit is a deny gate,
1829+
// and skipping it fails open (operators who block secret-file reads via
1830+
// beforeTool would lose that protection under /plan on). See dispatchBeforeTool.
1831+
//
1832+
// Spec-draft keeps the existing trust-gated hook model: project hooks still
1833+
// fire when the workspace (or its worktree trust root) is trusted. That is
1834+
// intentional; trust inheritance for --use-spec --worktree is covered by
1835+
// TestExecSpecWorktreeInheritsTrustEndToEnd.
1836+
func hooksSuppressed(options Options) bool {
1837+
return options.PermissionMode == PermissionModePlan
1838+
}
1839+
18211840
// dispatchBeforeTool runs configured beforeTool hooks for a tool call. A hook
18221841
// that exits non-zero vetoes the call: the returned bool is true and the tool
18231842
// must not run. A nil dispatcher (no hooks wired) is a no-op.
1843+
//
1844+
// Unlike advisory hooks, beforeTool still runs under plan mode. Suppressing it
1845+
// would fail open: a project policy that blocks reads of secrets via
1846+
// beforeTool would silently stop applying the moment permission mode is plan.
18241847
func dispatchBeforeTool(ctx context.Context, options Options, call ToolCall, args map[string]any) (hooks.DispatchOutcome, bool) {
18251848
if options.Hooks == nil {
18261849
return hooks.DispatchOutcome{}, false
@@ -1845,7 +1868,7 @@ func dispatchBeforeTool(ctx context.Context, options Options, call ToolCall, arg
18451868
// returns any advisory output (e.g. a formatter or vet result) to surface back
18461869
// to the model. afterTool hooks never block. A nil dispatcher is a no-op.
18471870
func dispatchAfterTool(ctx context.Context, options Options, call ToolCall, args map[string]any, result tools.Result) string {
1848-
if options.Hooks == nil {
1871+
if options.Hooks == nil || hooksSuppressed(options) {
18491872
return ""
18501873
}
18511874
outcome := options.Hooks.Dispatch(ctx, hooks.DispatchInput{
@@ -1869,7 +1892,7 @@ func dispatchAfterTool(ctx context.Context, options Options, call ToolCall, args
18691892
// model turn. Lifecycle hooks are advisory: dispatcher failures are audited but
18701893
// never block the run.
18711894
func dispatchSessionStart(ctx context.Context, options Options) {
1872-
if options.Hooks == nil {
1895+
if options.Hooks == nil || hooksSuppressed(options) {
18731896
return
18741897
}
18751898
options.Hooks.Dispatch(ctx, hooks.DispatchInput{
@@ -1890,7 +1913,7 @@ func dispatchSessionStart(ctx context.Context, options Options) {
18901913
// dispatchSessionEnd runs configured sessionEnd hooks once when the agent run
18911914
// exits, including early error returns. Lifecycle hooks are advisory.
18921915
func dispatchSessionEnd(ctx context.Context, options Options, result Result, runErr error) {
1893-
if options.Hooks == nil {
1916+
if options.Hooks == nil || hooksSuppressed(options) {
18941917
return
18951918
}
18961919
payload := map[string]any{
@@ -2190,6 +2213,20 @@ type requestPermissionsArgs struct {
21902213
}
21912214

21922215
func executeRequestPermissions(ctx context.Context, call ToolCall, args map[string]any, permissionMode PermissionMode, options Options) (ToolResult, error) {
2216+
// request_permissions is dispatched by name above, before the registry-based
2217+
// ToolAdvertised gate runs, so a read-only mode's registry omitting this tool
2218+
// (rather than registering it as denied) must not fall through to a real
2219+
// grant. Deny it here unconditionally for spec-draft/plan, independent of
2220+
// whether the caller's registry happens to contain the tool.
2221+
if permissionMode == PermissionModeSpecDraft || permissionMode == PermissionModePlan {
2222+
return ToolResult{
2223+
ToolCallID: call.ID,
2224+
Name: call.Name,
2225+
Status: tools.StatusError,
2226+
Output: `Error: Tool "` + call.Name + `" is not available in ` + string(permissionMode) + ` mode.`,
2227+
DenialReason: DenialFiltered,
2228+
}, nil
2229+
}
21932230
parsed, err := parseRequestPermissionsArgs(args)
21942231
if err != nil {
21952232
return ToolResult{
@@ -3219,11 +3256,17 @@ func propertyToRuntimeMap(property tools.PropertySchema) map[string]any {
32193256
}
32203257

32213258
func ToolAdvertised(tool tools.Tool, permissionMode PermissionMode) bool {
3259+
// Denied tools are never advertised in any mode. Keep this short-circuit
3260+
// here so auto/member-auto/ask/unsafe all honor it before mode branches;
3261+
// tools.ToolAdvertisedForPermissionMode repeats it for the tool_search path
3262+
// which never enters this function.
32223263
if tool.Safety().Permission == tools.PermissionDeny {
32233264
return false
32243265
}
3225-
if permissionMode == PermissionModeSpecDraft {
3226-
return toolAdvertisedInSpecDraft(tool)
3266+
// plan/spec-draft policy lives in tools so tool_search and the dispatch
3267+
// gate cannot drift. PermissionDeny was already checked above.
3268+
if permissionMode == PermissionModeSpecDraft || permissionMode == PermissionModePlan {
3269+
return tools.ToolAdvertisedForPermissionMode(tool, string(permissionMode))
32273270
}
32283271
if permissionMode == PermissionModeAuto {
32293272
return tool.Safety().Permission == tools.PermissionAllow || tool.Safety().AdvertiseInAuto
@@ -3246,17 +3289,6 @@ func ToolAdvertised(tool tools.Tool, permissionMode PermissionMode) bool {
32463289
return true
32473290
}
32483291

3249-
func toolAdvertisedInSpecDraft(tool tools.Tool) bool {
3250-
switch tool.Name() {
3251-
case "ask_user", "submit_spec":
3252-
return true
3253-
case "update_plan":
3254-
return false
3255-
}
3256-
safety := tool.Safety()
3257-
return safety.SideEffect == tools.SideEffectRead && safety.Permission == tools.PermissionAllow
3258-
}
3259-
32603292
func stopReasonFromToolResult(result ToolResult) StopReason {
32613293
if result.Meta == nil {
32623294
return ""

0 commit comments

Comments
 (0)