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 aDispatchPolicy { strict }. Matching bindings run underjoin_alland 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 theDispatchOutcomesayscancelledso no caller reads it as no opinion.refuse_unhonorable_effectsanswers 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 becauseUnsupportedEffectrecords only the effect kind, not the field's value); onprompt.submitit holds the prompt; after the fact it is only reported. Model context, which this phase cannot deliver, is logged. The invocation environment comes frominvocation_environmentwith aHookIdentitybuilt 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.Denydenies with the winning binding's reason (or a default) whateveris_user_initiatedsays.Askadds 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 withApproval unavailable: an agent hook asked for confirmation (<hook reason>) and this run has no one to ask.Deferis ignored on an interactive surface and denied on a headless one or under the strict policy.AllowandNoOpinionleave 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 inPreToolVerdictCache, whoseconsultholds 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_resolveddrops 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.decidecomposes the policy withRunFacts { is_headless, is_strict }, sends aDeniednotice for every denial, aWarningnotice with the hook's reason once per verdict when a hook asks for confirmation, and underHeadlessAskBehavior::FailRunalso emitsHookRuntimeEvent::ApprovalUnavailable.app/src/ai/blocklist/hooks/denied_result.rs:denied_result_for(action, reason), exhaustive overAIAgentActionType. A shell command finishes withHookDenied; every family with a text slot finishes with itsErrororDeniedvariant carryingDenied 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 itsDispatchPolicy. No setting exists for it; it is a field onHookRuntimewith a setter for the run's launcher.app/src/ai/blocklist/hooks/mod.rs:HookRuntimeEventas the runtime's entity event,HeadlessAskBehavior { DenyWithReason, FailRun }beside it,strict_policyandheadless_ask_behaviorfields with getters and setters,is_headless_for(conversation_id), anddispatch_policy();dispatch_detachedpasses the policy.app/src/ai/blocklist/hooks/pre_tool.rsalso 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_floorsfromcan_autoexecute_command's reason, or the execute-commands setting when the shell type is unknown;file_write_floorsfromcan_write_files's reason, after resolving the model's paths against the session's working directory withhost_native_absolute_paththe way the file-edit executor does, since the protected paths are absolute;mcp_floorsfrom the profile's MCP denylist and setting).app/src/ai/blocklist/action_model/execute.rs(frozen; the allowed edits): a one-callpermission_floorswrapper hands the executor's session, terminal view, and team scope topre_tool::permission_floorswhen a verdict is present. The decision match handlesDenywithdenied_result_for(and the run-agents completion telemetry), holds aRequireConfirmationasNeedsConfirmation, and letsExecutefall through to the checks the policy started from.
Decisions
Dispatcher (§4.3, §4.4, §6.2, §6.3, §6.4)
- Concurrency is
join_allover 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_allreturns 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_allreturns; a dispatch that was cancelled while running returns no opinion with the invocation records it has, nothing is merged, andDispatchOutcome::cancelledis set so a caller cannot mistake it for permission to proceed;on_resolvedchecks it next to its own ticket. The runner and theLatencyRunnerpoll the flag, so a cancel reaches a process that is still running. - Unsupported control effects before an operation deny; on
prompt.submitthey 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", adecisionother than"block", and an emptyfollowup_messageask for nothing and are only logged, so Claude-shaped output copied onto abeforeShellExecutionhook 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 inquestions.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'sreplacement_inputbranch (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. Deferby 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, whichreduce_effectsreports as a diagnostic.reduce_effectsnever produces a mergedDefer, so theDeferarm ofHookPermissionPolicy::composeis 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):
remoteis true only forOzRun; 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.
DispatchPolicyis 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)
Allowis structurally inert: the policy computes Warp's decision first and only theDeny,Ask, andDeferarms produce anything else, all of them stricter. The generated matrix test assertsstrictness(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
askdenies 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 inquestions.md. Deferunder 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 producesDefer. Also recorded inquestions.md.HeadlessAskBehaviorlives inhooks/mod.rsbesideStrictHookPolicy's storage on the runtime and the event it causes, withDenyWithReasonas the default. UnderFailRunthe action is still denied; the runtime additionally emitsHookRuntimeEvent::ApprovalUnavailablefor the run's owner. Nothing ends the run yet (PR 8). Also recorded inquestions.md.- The strict policy has no setting (§3 "Activation controls"):
StrictHookPolicyis aHookRuntimefield, off by default, that PR 8 sets from the run configuration.
Verdict cache (§6.1, §11)
argument_revisionis a hash of the action's derivedDebugform, 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 explicitinvalidatecalls PR 1 added toaction_model.rsfor an edited command are now redundant but harmless and were left in place (action_model.rsis 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_resolvedand 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
askreason reaches the user as aWarningnotice 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 inquestions.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_hitrather than separate org and user flags:can_autoexecute_commandalready folds inauto_approve_bypasses_command_denylistand 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 inquestions.md. - Floors exist only when Warp did not auto-execute: an action Warp runs has nothing to loosen, so
permission_floorsreturns 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_denylistedis read from the profile's denylist after resolving the server the waycan_call_mcp_toolandcan_read_mcp_resourcefind 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 inquestions.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_settingfrom the execute-commands setting, and a passive diff reportscan_write_files's answer. Sub-executor getters would be the tighter shape; they live in files nobody owns. protected_write_pathis read against resolved paths (PR 9's review round 2): the model writes.cursor/hooks.jsonrelative to the session's working directory, andcan_write_filesmatches protected paths by absolute path, so the floor resolves the paths the wayRequestFileEditsExecutor::should_autoexecutedoes 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)
SuggestNewConversationfinishesRejectedrather than cancelled: a declined suggestion is the answer the model expects for "no", and its cancelled result is dropped from the wire.SearchCodebasehas no plainError; it finishesFailed { reason: ClientError, message }with the prefixed reason in the message.InsertCodeReviewCommentshas a text slot after all (InsertReviewCommentsResult::Error { repo_path, message }) and carries the reason; the contract listed it among the empty-result tools.RunAgentsfinishesRunAgentsResult::Denied { reason }and sends the same completion telemetry the cancellation and orchestration-denial paths send.ReadSkillResult::Errornow counts as failed inAIAgentActionResultType::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 in30a77c620): the permission policy's input and output types.permission.rsgainsRunFacts { is_headless, is_strict },PermissionFloorswith named fields (command_denylist_hit,protected_write_path,mcp_server_denylisted,always_ask_setting) andany(),DenyCause { Hook, RewriteUnsupported, ApprovalUnavailable, DeferralUnsupported },PermissionDecision::Deny { reason, cause }, andPermissionDecision::strictness().model.rsgainsHookSurface::is_headless()(OzRun,CliRun,LocalChild).envelope.rsgainssurface_for_conversation, which reportsLocalChildfor a conversation with a parent in a desktop or TUI process, andbuild()uses it. PR 1'sexecute.rspattern becameDeny { reason, .. };hooks/mod.rsre-exports the new types; atesting::add_child_conversationhelper was added. - Amendment to PR 3 (
65a41d610):report_undocumented_fieldsincrates/ai/src/hooks/adapters/cursor/decode.rsreads 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", adecisionother than"block", an emptyfollowup_message) is logged and not reported. Without it the dispatcher denied a shell command for a benigncontinue: true. - §6.5 names
org_denylist_hitanduser_denylist_hit. The floors carry onecommand_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, andWaitForEventsfinish 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.InitProjectandOpenCodeReviewhave 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 theDeniednotice, which is log-only in a run until PR 8. The non-cancelled results that do exist (Error(BlockNotFound)for the shell families,CompletedforWaitForEvents) 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, soquestions.mdasks whether the proto slot should move into Phase 1;denied_result_tests.rspins the current behavior. execute.rsis frozen. Edits: the floors derivation the contract allows as apermission_factsmethod is a one-callpermission_floorswrapper overpre_tool::permission_floors(the helpers live inpre_tool.rs, which this PR owns); aterminal_view_idfield on the executor so the floors read the same profile Warp's check reads; the decision match handlesRequireConfirmationand sends the run-agents completion telemetry on a denial;denied_result_forreplaces PR 1's cancelled-result fallback for non-shell families.action_model_tests.rsis frozen. One assertion changed: a hook-deniedrun_agentsnow finishesRunAgentsResult::Denied { reason: "Denied by agent hook: ..." }instead of its cancelled result.crates/ai/src/agent/action_result/mod.rs:is_failedincludesReadSkill(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.rsandcrates/ai/src/hooks/adapters/cursor/mod_test.rs(the newDispatchPolicyargument todispatch),crates/ai/src/hooks/test_util/mod.rs(registerslatency_runner.rs),crates/ai/src/hooks/mod.rs(re-exportsDispatchPolicy,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 nowHookRuntimeEvent).
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-specorready-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— passedcargo 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-faston 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— passedcargo clippy -p ai --all-targets --tests -- -D warnings— passedcargo 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-sensitivea_long_running_commands_real_exit_is_reported_again_on_its_own_actionfail 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;Deferdenies 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 honorspreToolUseask, strict mode never weakens a denial,{"permission":"allow","continue":true}onbeforeShellExecutionpasses through, andcontinue: falseis 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;Allowcannot make Warp execute what it would ask about or deny;Denydenies a user-initiated action with the denying binding's reason;Askadds 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;Deferis ignored interactively and denied headless or under strict; a rewrite request denies. -
denied_result: a shell command finishesHookDeniedwith 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 theprotected_write_pathfloor, the absolute spelling does too, and a file beside it does not; a changed command gets a new dispatch without an explicitinvalidate; 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; anaskon an interactive surface requires confirmation and tells the user why, once per verdict; anaskfor a hidden child denies with the approval-unavailable reason;FailRunannounces the unanswerableaskand still denies. -
execute: a hookaskadds a confirmation step, sends the reason as a warning notice once, and is satisfied by the user's approval; a hookaskfor a hidden child ends inHookDeniedwith "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
10This pull request is a draft
Merging is unavailable until it is marked ready for review.
masterThis stack of pull requests is managed by Graphite. Learn more about stacking.