#16590

hooks: dispatch bindings at once and apply the permission policy

Draftvorporealwants to merge intofrom

Description

Summary

Sixth PR of the agent hooks Phase 1 stack. It finishes the two parts that decide what happens: the dispatcher runs every matching binding at the same time and merges the results in a fixed order, and the permission policy composes a merged verdict with Warp's own result so that a hook can only ever tighten what Warp decided. It also widens the pre-tool verdict key so a changed command, working directory, or hook configuration runs the hooks again, and gives every tool family a result to finish with when a hook denies it.

  • crates/ai/src/hooks/dispatch.rs: dispatch(envelope, pinned, policy, cancel) takes a DispatchPolicy { strict }. Matching bindings run under join_all and are reduced in the pinned snapshot's order whatever order their processes finished in; a cancel observed after the processes return discards the partial result instead of merging it, and the DispatchOutcome says cancelled so no caller reads it as no opinion. refuse_unhonorable_effects answers a control effect a boundary cannot honor in this phase: a rewrite, a result replacement, a turn stop or continuation request, a deferral on a headless surface or under the strict policy, and any control field the adapter already refused because the event does not define it. Before an operation (tool.pre, tool.permission_request, file.exposure, subagent.start) that denies the action with a reason naming the binding and either what it asked for (a channel it filled) or what it returned (a field the event does not accept, worded by the field's purpose because UnsupportedEffect records only the effect kind, not the field's value); on prompt.submit it holds the prompt; after the fact it is only reported. Model context, which this phase cannot deliver, is logged. The invocation environment comes from invocation_environment with a HookIdentity built from the binding's format, the project directory (a project file's own root; for an explicit source the operation's project, else the working directory, else the directory the hook runs in), the run id, the user, and whether the surface is a remote run. One summary log line per dispatch (event, action, bindings matched, verdict, halt, duration); the per-invocation line stays in the runner. A debug assertion checks the registry's promise that no source reaches the dispatcher twice.
  • crates/ai/src/hooks/test_util/latency_runner.rs: LatencyRunner, a runner double that waits a scripted time per binding and polls the cancel flag, so concurrency and cancellation can be tested without processes.
  • crates/ai/src/hooks/permission.rs: compose(merged, warp, floors, run) matches on the verdict with Warp's own decision as the starting point. Deny denies with the winning binding's reason (or a default) whatever is_user_initiated says. Ask adds a confirmation step where someone can answer, is satisfied by the user's approval of this revision on an interactive surface, and on a headless surface denies with Approval unavailable: an agent hook asked for confirmation (<hook reason>) and this run has no one to ask. Defer is ignored on an interactive surface and denied on a headless one or under the strict policy. Allow and NoOpinion leave Warp's decision alone. A debug assertion checks that the result is never less strict than Warp's own decision.
  • app/src/ai/blocklist/hooks/pre_tool.rs: VerdictKey { conversation_id, action_id, argument_revision, cwd, registry_revision }, one entry per action in PreToolVerdictCache, whose consult holds the lookup rule. A lookup whose key differs from the cached one, or whose dispatch died before answering, drops the entry, tells its dispatch to stop, and starts over; user approval carries over only when the key is unchanged. An action that starts over more than three times is denied with a Warp-written reason. on_resolved drops a verdict judged under a registry revision that has since moved but still resumes the action, without the approval it carried, so its next lookup runs the hooks against the new configuration. decide composes the policy with RunFacts { is_headless, is_strict }, sends a Denied notice for every denial, a Warning notice with the hook's reason once per verdict when a hook asks for confirmation, and under HeadlessAskBehavior::FailRun also emits HookRuntimeEvent::ApprovalUnavailable.
  • app/src/ai/blocklist/hooks/denied_result.rs: denied_result_for(action, reason), exhaustive over AIAgentActionType. A shell command finishes with HookDenied; every family with a text slot finishes with its Error or Denied variant carrying Denied by agent hook: <reason>; the families without one finish with their cancelled or rejected result and the reason reaches only the user notice.
  • app/src/ai/blocklist/hooks/strict_policy.rs: StrictHookPolicy { enabled } and its DispatchPolicy. No setting exists for it; it is a field on HookRuntime with a setter for the run's launcher.
  • app/src/ai/blocklist/hooks/mod.rs: HookRuntimeEvent as the runtime's entity event, HeadlessAskBehavior { DenyWithReason, FailRun } beside it, strict_policy and headless_ask_behavior fields with getters and setters, is_headless_for(conversation_id), and dispatch_policy(); dispatch_detached passes the policy.
  • app/src/ai/blocklist/hooks/pre_tool.rs also holds the floors derivation: permission_floors(action, conversation_id, can_auto_execute, active_session, terminal_view_id, scope, ctx) returns none when Warp auto-executed and otherwise reads the same permission functions the per-family checks call, one helper per family (shell_floors from can_autoexecute_command's reason, or the execute-commands setting when the shell type is unknown; file_write_floors from can_write_files's reason, after resolving the model's paths against the session's working directory with host_native_absolute_path the way the file-edit executor does, since the protected paths are absolute; mcp_floors from the profile's MCP denylist and setting).
  • app/src/ai/blocklist/action_model/execute.rs (frozen; the allowed edits): a one-call permission_floors wrapper hands the executor's session, terminal view, and team scope to pre_tool::permission_floors when a verdict is present. The decision match handles Deny with denied_result_for (and the run-agents completion telemetry), holds a RequireConfirmation as NeedsConfirmation, and lets Execute fall through to the checks the policy started from.

Decisions

Dispatcher (§4.3, §4.4, §6.2, §6.3, §6.4)

  • Concurrency is join_all over one future per binding (§6.4). There is no dispatch-wide timer: each invocation is held to its own timeout by the runner, so the dispatch ends when the slowest invocation does, and the runner's process bound is the only limit on how many run at once. A queued hook that times out because of queueing denies under strict mode exactly like any other timeout; the runner's log line records the wait.
  • Snapshot order (§6.3): join_all returns results in the order the futures were given, and the reduce input is built by zipping the matched bindings with those results, so nothing is read from completion order. Tested with latencies 300, 100, 200 ms: the dispatch takes about 300 ms and the first binding's verdict wins.
  • Cancellation (§6.1, §6.4): the cancel flag is checked before each binding starts and again after join_all returns; a dispatch that was cancelled while running returns no opinion with the invocation records it has, nothing is merged, and DispatchOutcome::cancelled is set so a caller cannot mistake it for permission to proceed; on_resolved checks it next to its own ticket. The runner and the LatencyRunner poll the flag, so a cancel reaches a process that is still running.
  • Unsupported control effects before an operation deny; on prompt.submit they halt the prompt (§4.3, §4.4). Every control channel is covered, including fields the adapter refused because the event does not define them, so nothing a hook returned is dropped silently before an operation runs. The adapter reads such a field's value first (amendment to PR 3, below): continue: true, permission: "allow", a decision other than "block", and an empty followup_message ask for nothing and are only logged, so Claude-shaped output copied onto a beforeShellExecution hook does not deny the command. A value that asks for something is refused with a reason that names the field's purpose ("returned a field that controls whether the turn continues, which this event does not accept") rather than claiming what the hook asked for. Also recorded in questions.md.
  • A rewrite request has one owner (§4.3, §6.2): the dispatcher denies it before an operation with a reason that names the binding, so HookPermissionPolicy::compose's replacement_input branch (DenyCause::RewriteUnsupported, REWRITE_UNSUPPORTED_DENY_REASON) is unreachable through the dispatcher on the boundaries that call the policy. The branch stays as a backstop for a caller that hands the policy effects without the dispatcher's reduction, because a rewrite must never be run as if it had been honored; its text is Warp's generic one because it has no binding to name. The dispatcher's reason is composed per binding (it may name several refused channels at once), so the two are not one constant; the comment on the branch says which one the model sees.
  • Defer by surface and policy (§4.3, §6.4): the dispatcher refuses a deferral before an operation on a headless surface, where nobody could resume it, and under the strict policy on any surface; otherwise an interactive surface keeps the vendor's "warn and ignore" behavior, which reduce_effects reports as a diagnostic. reduce_effects never produces a merged Defer, so the Defer arm of HookPermissionPolicy::compose is a defense in depth for a caller that hands the policy effects without going through the dispatcher's reduction; it applies the same rule.
  • Hook identity (§7.2): remote is true only for OzRun; a command-line run and a hidden local child are local processes. The project directory for an explicit source is the operation's first project root, so a hook delivered with the run is told the repository it is checking rather than its own directory.
  • Logging (§10 item 3): one summary line per dispatch. The per-invocation line PR 1 wrote in the dispatcher is gone; the runner writes it.
  • DispatchPolicy is passed per call rather than stored on the dispatcher, so a policy change takes effect at the next dispatch without rebuilding the dispatcher.

Permission policy (§6.1, §6.5)

  • Allow is structurally inert: the policy computes Warp's decision first and only the Deny, Ask, and Defer arms produce anything else, all of them stricter. The generated matrix test asserts strictness(decision) >= strictness(warp_alone) for every cell, and a debug assertion repeats it at run time.
  • Floors are inputs the policy does not read yet (§6.5): nothing in Phase 1 loosens, so no path consults them. They exist so bounded loosening has named facts to check, and the matrix test runs over every combination to show they are already never crossed.
  • A headless ask denies even when the revision was marked as user-approved (§6.5): no person can have approved anything on an Oz run, a command-line run, or a hidden child. Also recorded in questions.md.
  • Defer under the strict policy denies on an interactive surface too (§6.4): a deferral is unsupported control output, and strict mode turns every such output into a denial. No Phase 1 adapter produces Defer. Also recorded in questions.md.
  • HeadlessAskBehavior lives in hooks/mod.rs beside StrictHookPolicy's storage on the runtime and the event it causes, with DenyWithReason as the default. Under FailRun the action is still denied; the runtime additionally emits HookRuntimeEvent::ApprovalUnavailable for the run's owner. Nothing ends the run yet (PR 8). Also recorded in questions.md.
  • The strict policy has no setting (§3 "Activation controls"): StrictHookPolicy is a HookRuntime field, off by default, that PR 8 sets from the run configuration.

Verdict cache (§6.1, §11)

  • argument_revision is a hash of the action's derived Debug form, which covers every field. A field that prints differently for the same value can only cause an extra hook run, never a stale verdict. The explicit invalidate calls PR 1 added to action_model.rs for an edited command are now redundant but harmless and were left in place (action_model.rs is frozen).
  • A changed key cancels the earlier dispatch and starts over, and user approval does not carry over: approval covers the operation the user saw under the hook configuration of that moment, and a changed configuration may ask for a confirmation the user never saw. Approval survives only when the key is unchanged, which happens when a dispatch died (below). Also recorded in questions.md.
  • A verdict judged under an older registry revision is dropped in on_resolved and the action is resumed anyway, without approval, so the executor's next lookup sees a key mismatch and re-dispatches exactly once against the new snapshot; if the user had approved, the card is shown again, with the hook's reason as a notice. Tested.
  • A pending entry whose dispatch future was dropped before it finished (the cancel flag set with nothing resolved) is treated as outdated by the next lookup, so a fresh dispatch starts instead of the action waiting forever.
  • The cheap "any pre-tool binding at all" check runs before the action is hashed for the key, so a configuration with only post-tool hooks does not pay for the hash on every try.
  • A hook's ask reason reaches the user as a Warning notice when the policy asks for confirmation, once per verdict (VerdictEntry::confirmation_noticed), since a waiting action is tried more than once. Phase 1.5's card can show it.
  • Restart bound: an action whose check starts over more than three times is denied with CONFIGURATION_UNSTABLE_DENY_REASON, because the registry bumps its revision on every reload attempt (even after pinning) and a hook file written in a loop would otherwise re-run the hooks forever. Also recorded in questions.md.
  • HookRuntime::is_headless_for(conversation_id) decides headlessness per conversation rather than per process, so a hidden local child in a desktop process is headless while its parent is not.

Floors derivation (§6.5)

  • One command_denylist_hit rather than separate org and user flags: can_autoexecute_command already folds in auto_approve_bypasses_command_denylist and reports only that a denylist that applies matched; telling the lists apart would mean copying its command decomposition. The flag is read from that function's own answer, so the derivation is shared code, not a copy. Also recorded in questions.md.
  • Floors exist only when Warp did not auto-execute: an action Warp runs has nothing to loosen, so permission_floors returns none, which keeps a file edit that auto-executes to report a failed diff from carrying a floor. They are computed only when a verdict is present, so the no-hooks path pays nothing beyond PR 1's check.
  • mcp_server_denylisted is read from the profile's denylist after resolving the server the way can_call_mcp_tool and can_read_mcp_resource find it (installation id, else the tool or resource name). Because floors exist only when Warp did not auto-execute, an auto-approved conversation, which bypasses that list in Warp's own check, never carries it. Also recorded in questions.md.
  • The floors are read again from the permission functions rather than from the executor that ran them, so where that executor answered before reaching them they still describe the settings and lists as they stand: a shell command with no known shell type reports always_ask_setting from the execute-commands setting, and a passive diff reports can_write_files's answer. Sub-executor getters would be the tighter shape; they live in files nobody owns.
  • protected_write_path is read against resolved paths (PR 9's review round 2): the model writes .cursor/hooks.json relative to the session's working directory, and can_write_files matches protected paths by absolute path, so the floor resolves the paths the way RequestFileEditsExecutor::should_autoexecute does before its own check. Harmless in Phase 1, where the policy only tightens and the executor's own answer holds the action; it is the floor bounded loosening will consult, so it must agree with the executor. Tested with a relative and an absolute spelling of a protected hook file and a file beside it.

Denied results (§9.3)

  • SuggestNewConversation finishes Rejected rather than cancelled: a declined suggestion is the answer the model expects for "no", and its cancelled result is dropped from the wire.
  • SearchCodebase has no plain Error; it finishes Failed { reason: ClientError, message } with the prefixed reason in the message.
  • InsertCodeReviewComments has a text slot after all (InsertReviewCommentsResult::Error { repo_path, message }) and carries the reason; the contract listed it among the empty-result tools.
  • RunAgents finishes RunAgentsResult::Denied { reason } and sends the same completion telemetry the cancellation and orchestration-denial paths send.
  • ReadSkillResult::Error now counts as failed in AIAgentActionResultType::is_failed; it was the one error variant left out, which would have made a denied skill read look successful to the loop.

Departures from architecture.md

  • Amendment to PR 1 (9db79f047, field renamed in 30a77c620): the permission policy's input and output types. permission.rs gains RunFacts { is_headless, is_strict }, PermissionFloors with named fields (command_denylist_hit, protected_write_path, mcp_server_denylisted, always_ask_setting) and any(), DenyCause { Hook, RewriteUnsupported, ApprovalUnavailable, DeferralUnsupported }, PermissionDecision::Deny { reason, cause }, and PermissionDecision::strictness(). model.rs gains HookSurface::is_headless() (OzRun, CliRun, LocalChild). envelope.rs gains surface_for_conversation, which reports LocalChild for a conversation with a parent in a desktop or TUI process, and build() uses it. PR 1's execute.rs pattern became Deny { reason, .. }; hooks/mod.rs re-exports the new types; a testing::add_child_conversation helper was added.
  • Amendment to PR 3 (65a41d610): report_undocumented_fields in crates/ai/src/hooks/adapters/cursor/decode.rs reads a control field's value before reporting it on an event that does not define it, so a value that asks for nothing (continue: true, permission: "allow", a decision other than "block", an empty followup_message) is logged and not reported. Without it the dispatcher denied a shell command for a benign continue: true.
  • §6.5 names org_denylist_hit and user_denylist_hit. The floors carry one command_denylist_hit (reason above).
  • §9.3: the families with no text slot get no proto slot for the reason, and what the model sees differs by family. WriteToLongRunningShellCommand, ReadShellCommandOutput, TransferShellCommandControlToUser, SuggestPrompt, and WaitForEvents finish with their cancelled result; a cancelled result sends no follow-up request when it is the batch's last action, so the conversation is marked cancelled, the turn ends, and the model never learns the action was denied. InitProject and OpenCodeReview have a single result, which reaches the wire as the completed marker, so the model believes the action happened. The reason reaches only the user through the Denied notice, which is log-only in a run until PR 8. The non-cancelled results that do exist (Error(BlockNotFound) for the shell families, Completed for WaitForEvents) were not used because each would tell the model something false it would act on. The first two shell families are real Phase 1 actions in Oz runs, so questions.md asks whether the proto slot should move into Phase 1; denied_result_tests.rs pins the current behavior.
  • execute.rs is frozen. Edits: the floors derivation the contract allows as a permission_facts method is a one-call permission_floors wrapper over pre_tool::permission_floors (the helpers live in pre_tool.rs, which this PR owns); a terminal_view_id field on the executor so the floors read the same profile Warp's check reads; the decision match handles RequireConfirmation and sends the run-agents completion telemetry on a denial; denied_result_for replaces PR 1's cancelled-result fallback for non-shell families.
  • action_model_tests.rs is frozen. One assertion changed: a hook-denied run_agents now finishes RunAgentsResult::Denied { reason: "Denied by agent hook: ..." } instead of its cancelled result.
  • crates/ai/src/agent/action_result/mod.rs: is_failed includes ReadSkill(ReadSkillResult::Error(_)) (reason above). The contract names this file for PR 6 when other families need it.
  • Files outside this PR's ownership that changed: crates/ai/src/hooks/e2e_slice_tests.rs and crates/ai/src/hooks/adapters/cursor/mod_test.rs (the new DispatchPolicy argument to dispatch), crates/ai/src/hooks/test_util/mod.rs (registers latency_runner.rs), crates/ai/src/hooks/mod.rs (re-exports DispatchPolicy, APPROVAL_UNAVAILABLE_PREFIX, DEFAULT_ASK_REASON, DEFERRAL_UNSUPPORTED_DENY_REASON), app/src/ai/blocklist/hooks/mod.rs (the additions above; HookRuntime's entity event type is now HookRuntimeEvent).

Linked Issue

No linked issue. Part of the agent hooks Phase 1 stack; design is in specs/agent-hooks/architecture.md.

  • The linked issue is labeled ready-to-spec or ready-to-implement.
  • Where appropriate, screenshots or a short video of the implementation are included below (especially for user-visible or UI changes).

Testing

All commands run with CARGO_TARGET_DIR=/Users/david/src/warp/target. The warp commands also set WARP_LOCAL_SKIP_METAL=1, which this machine needs because it has no Metal toolchain.

  • cargo check -p ai — passed
  • cargo nextest run -p ai --features local_fs,test-util -E 'test(hooks) | test(action_result)' — passed (281 tests after the style round)
  • cargo nextest run -p ai --features local_fs,test-util -E 'test(hooks)' — passed (271 tests, on PR 7's branch after the second style round; 281 tests with --no-fail-fast on PR 9's branch after the protected-path floor fix, where the first run saw three of PR 9's real-process tests stall on spawn and they passed alone and in the second full run)
  • cargo nextest run -p ai --features local_fs,test-util -E 'test(cursor)' — passed (91 tests, on PR 3's branch for the amendment)
  • cargo clippy -p ai --features local_fs,test-util --all-targets --tests -- -D warnings — passed
  • cargo clippy -p ai --all-targets --tests -- -D warnings — passed
  • cargo nextest run -p warp --no-fail-fast -E 'test(hooks)' — passed (80 tests after review round 1; nextest reports one pre-existing test as leaky, with no failure)
  • cargo nextest run -p warp --no-fail-fast -E 'test(ai::blocklist::action_model) | test(ai::blocklist::hooks) | test(ai::blocklist::permissions) | test(ai::blocklist::controller) | test(ai::agent::conversation)' — passed (461 tests, before review round 1)
  • cargo nextest run -p warp --no-fail-fast -E 'test(hooks) | test(ai::blocklist::action_model)' — passed (298 tests, after review round 1 and again after the style round; 334 tests on PR 7's branch after the second style round, where one run saw PR 7's load-sensitive a_long_running_commands_real_exit_is_reported_again_on_its_own_action fail and it passed alone and in a second full run)
  • cargo nextest run -p warp --no-fail-fast -E 'test(hooks) | test(protected) | test(ai::blocklist::action_model)' — passed (370 tests, on PR 9's branch after the protected-path floor fix)
  • cargo clippy -p warp --all-targets --tests -- -D warnings — passed
  • ./script/format — ran last

New tests, by module:

  • dispatch: three bindings with latencies 300, 100, 200 ms finish in about 300 ms and merge in snapshot order; a cancel during the run discards the partial result and marks the outcome cancelled; a cancel set before dispatch runs nothing and marks the outcome cancelled; no bindings is a real no opinion; the strict policy reaches the adapter; a rewrite request before an operation denies with the binding named and after the fact is only reported; a continuation request before a tool runs denies; Defer denies headless before an operation, denies under the strict policy on an interactive surface, is ignored with a diagnostic interactively otherwise, and is only reported after the fact; through the real Cursor adapter, a fail-open timeout has no opinion, a fail-closed timeout denies, malformed output on a permission binding denies even when fail-open, strict mode turns the fail-open timeout into a denial and honors preToolUse ask, strict mode never weakens a denial, {"permission":"allow","continue":true} on beforeShellExecution passes through, and continue: false is refused in the field's own words without claiming the hook asked to stop the turn; the invocation carries the identity variables for its format; a project binding is told its own root and a local run is not remote; a binding whose input cannot be encoded follows its failure policy; a file delivered with the run and found as a project file runs once; distinct sources are told apart by file and position.

  • permission: a generated matrix over verdict, Warp's three facts, headless, strict, and all sixteen floor combinations asserting the decision is never less strict than Warp's own; Allow cannot make Warp execute what it would ask about or deny; Deny denies a user-initiated action with the denying binding's reason; Ask adds a confirmation step with the hook's reason, is satisfied by the user's approval of this revision, denies where nobody can answer with the hook's reason inside the approval-unavailable text, and denies headless even for a user-initiated revision; Defer is ignored interactively and denied headless or under strict; a rewrite request denies.

  • denied_result: a shell command finishes HookDenied with the reason; every family with a text slot carries the prefixed reason on the wire (checked through the proto conversion); every family without one finishes cancelled or rejected; the five cancelled families send no follow-up request and the two single-result families reach the wire as the completed marker (pinned).

  • pre_tool: a relative path to a protected hook file carries the protected_write_path floor, the absolute spelling does too, and a file beside it does not; a changed command gets a new dispatch without an explicit invalidate; a changed working directory and a registry revision bump each get a new dispatch; a verdict judged under an older revision is dropped and the hooks run once more; an approval does not survive a hook configuration change, on the resumption or on the restarted entry; a lookup after a dropped dispatch starts over and keeps the approval; a configuration that keeps changing ends in a denial; an ask on an interactive surface requires confirmation and tells the user why, once per verdict; an ask for a hidden child denies with the approval-unavailable reason; FailRun announces the unanswerable ask and still denies.

  • execute: a hook ask adds a confirmation step, sends the reason as a warning notice once, and is satisfied by the user's approval; a hook ask for a hidden child ends in HookDenied with "Approval unavailable"; each command in a batch is matched against the working directory it runs in when the first command changes it.

  • I have manually tested my changes locally with ./script/run

Manual testing is deferred to the PR 9 instructions; the flag is off.

Agent Mode

  • Warp Agent Mode - This PR was created via Warp's AI Agent Mode

CHANGELOG-NONE

Activity

10

This pull request is a draft

Merging is unavailable until it is marked ready for review.