Description
Summary
Second PR of the agent hooks Phase 1 stack. It proves the PR 1 scaffold end to end with real parts instead of test doubles: a minimal Cursor hooks.json parser that understands only beforeShellExecution, a minimal runner that spawns a real process with the JSON payload on stdin, and one explicit way to point the registry at a hooks file. A real script denies a real shell command in a test, through try_to_execute_action, with no test doubles anywhere in the path. Whatever the slice taught about PR 1's shared types was applied back to PR 1 (two small amendments, listed below).
crates/ai/src/hooks/adapters/cursor:CursorAdapter.config.rsreadsversionand thehooksobject, keeps thebeforeShellExecutiondefinitions (command,timeout,matcher,failClosed), reports other event names as warnings, and validates matchers at parse time.decode.rsturns a process outcome into effects by Cursor's rules: exit 2 denies, exit 0 means "read the JSON", everything else follows the definition's failure policy, and output that is not a valid decision denies.matchestests the shell command text with an unanchored, case-sensitive regex (cached per pattern);encode_inputproduces Cursor's documented stdin shape. The adapter is registered in the format table.crates/ai/src/hooks/runner/command.rs:CommandHookRunner. Runs the configured command string through/bin/sh -c(Unix, in its own process group) orcmd.exe /C(Windows), with the invocation's working directory, the inherited process environment plus the invocation's variables, the captured login shellPATHwhen one is known, andWARP_HOOK_PROTOCOL_VERSION. Stdin, stdout, and stderr are piped and driven together with the exit wait so a hook that fills one pipe before touching another cannot deadlock. The hook's exit decides the outcome as soon as it arrives; its pipes get one more second to drain in case a background process it started still holds them. The invocation timeout, and the cancel flag (polled every 50 ms), kill the whole process group and returnTimedOutorCancelled.kill_on_drop(true)so a dropped future does not leave the direct child running.crates/ai/src/hooks/registry.rs:HookRegistry::install_explicit_source(path, format, kind), in its ownimplblock. Reads the file, parses it with the adapter for the format, turns capability warnings into diagnostics, and publishes a new snapshot atrevision + 1in which this file's bindings replace any loaded from the same file before. Relative commands resolve against the file's directory.app/src/ai/blocklist/hooks/mod.rs:HookRuntime::install_command_runner, called frominitialize_appright afterLocalShellStateis registered. It installsCommandHookRunnerand, when the hooks flag is on, starts the interactive login shellPATHcapture; the result lands in a slot the runner reads on every invocation.- Tests: config, decode, and adapter unit tests; runner tests with real
/bin/shscripts;e2e_slice_tests.rsinai(ahooks.jsonon disk, a script, the real loader, dispatcher, and runner); and oneapptest that drives a real deny throughtry_to_execute_actionand ends inRequestCommandOutputResult::HookDeniedwith the script'sagent_message.
Decisions
Cursor adapter
permissionis required in abeforeShellExecutionresponse (§6.4,research/01§2.2). Cursor says a response that does not match the event's schema blocks a permission hook, and does not say whetherpermissionmay be omitted. A JSON object with nopermission, or with a value other thanallow,deny, orask, is treated as malformed and denies even when the definition fails open. Empty stdout is the separate "no output" case and followsfailClosed. Also recorded inquestions.md.- Denial reason (§9.3): on a deny,
agent_messageis the reason the model reads; when there is noagent_message,user_messageis; when there is neither, the reason isDenied by hook <command> (exit 2).for exit 2,Denied by hook <command>.for a JSON deny with no message, and a Warp-written sentence for the other deny paths. Every reason carries the binding it came from. Auser_messagethat is not already the reason is delivered as a user notice. Onallowandask,user_messagebecomes a user notice andagent_messagebecomes model context, whichvalidate_capabilitiesreports as unsupported in Phase 1 rather than dropping silently. - Exit 2 with JSON on stdout: the messages are still read. Exit 2 with anything else on stdout: the default reason. Other non-zero exits ignore stdout.
ProcessExit::Cancelledalways decodes as a deny, regardless offailClosed(§6.4 says cancellation never becomes fail-open execution). The dispatcher's caller drops a cancelled verdict anyway; this only matters if one is ever applied. Also recorded inquestions.md.- Timeouts, crashes, and non-zero exits other than 2 fail open unless
failClosed: true, as Cursor documents. A slowbeforeShellExecutionhook therefore allows the command by default until strict hook policy (PR 6). Also recorded inquestions.md. - Prompt hooks (
type: "prompt") are skipped one definition at a time, each with a warning diagnostic naming it (beforeShellExecution[0]: prompt hooks are not supported yet; it was skipped); the command hooks beside them load and keep their positions in the array, so merge order is unchanged. This follows Cursor's own headless surfaces, which run command hooks only, and §4.4's view of the handler kind as something to report rather than a reason to refuse the file. The skip happens in the adapter's parser rather than as a registry capability check; PR 3 may move it when it parses the other events. Unknowntypevalues are errors. Also recorded inquestions.md. timeoutis read as a positive number of seconds (fractions allowed); zero, negative, non-finite, or non-numeric values are errors. The default isHookBoundary::ToolPre.default_timeout()(30 s). The ceiling rule (§6.4) is PR 5's.versionmust be a positive integer;0or a missingversionis an error. A missinghooksobject is an empty file, not an error, since Cursor does not say it is required.- A matcher that does not compile makes the file invalid at parse time (§6.4), so
matchesnever sees one.""and"*"match everything and are not compiled. Compiled matchers are cached per pattern on the adapter so a regex is not built per envelope. matchesreturns false for anytool.preenvelope that is not a shell command, and for every other boundary. Unknown events in the file are skipped with a warning diagnostic rather than an error (PR 3 adds them).- Stdin shape (§4.2):
conversation_id,generation_id(the turn id, or the conversation id whileturn_idisNone),hook_event_name,cursor_version(Warp's version),workspace_roots(the detected roots, or the working directory when none was detected),user_email(nullwhen unknown),transcript_path(null),command, andcwd.tool_use_id(the action id) is added because Cursor's release notes document it.modelis present only when the envelope knows the model;sandboxandmodel_paramsare left out rather than invented. - Reason and notice text is trimmed and has control characters other than newline and tab removed, so a script cannot put terminal escape sequences into what the model or the user sees. Stdout that is not valid UTF-8 cannot parse as JSON and is malformed.
Runner
- Shell (§9.1):
/bin/sh -c <command>on Unix andcmd.exe /C <command>on Windows. The absolute/bin/shpath keeps the shell lookup independent of thePATHthe hook itself receives. The command string is the configuration's literal text; payload bytes only ever travel over stdin. - Environment: the runner's own process environment, then the invocation's variables, then
PATHfrom the capture when it returned one, thenWARP_HOOK_PROTOCOL_VERSIONlast. The full environment policy (scrub list,WARP_*identity,CURSOR_*aliases) is PR 4. - The exit decides (§9.1 "parse after exit"): as soon as the hook's own exit status arrives, that is the outcome. Its pipes are read for up to one more second (
PIPE_GRACE) in case a background process the hook started still holds them; after that, reading stops and whatever arrived so far is the output. Without this,some-logger & exit 2was reported as a timeout once the deadline passed, and the deny fell through to the fail-open default. - A background process that outlives the hook is left alone. Once the hook has been collected its pid, and so its process group id, may belong to someone else, so the group is signalled only while
try_statussays the hook is still running. That also closes the window on the timeout path where a child that exited a moment before the deadline could have had its recycled pid signalled; a hook found exited at that point is reported by its exit status, not as a timeout. - Timeout and cancellation: one
SIGKILLto the process group (Unix) plusChild::kill, then up to 2 s waiting for the child to be collected so a pid check afterwards sees it gone. The SIGTERM, wait, then SIGKILL sequence, output limits, and bounded concurrency are PR 4. On Windows only the directcmd.exechild is killed; descendants can survive a timeout until PR 4 adds Job Objects. - A dropped dispatch future kills the direct child through
kill_on_drop(true); it does not kill the process group, because async-process only signals the pid. PR 4's SIGTERM, wait, then SIGKILL sequence covers that case. - A finished process wins over a deadline that expired in the same poll (
select_biased!), so a completed hook is never reported as timed out. The cancel flag is also checked before spawning; a flag that is already set spawns nothing. - Output kept on a timeout or cancellation: whatever was read before the kill, so the stderr tail still reaches the log line.
- Stderr tail: the last 4 KiB as lossy UTF-8, matching PR 4's planned
HOOK_STDERR_TAIL. - No wasm stand-in: the
commandmodule is compiled only withlocal_fs. PR 1'sUnavailableRunneris the runner for builds that cannot spawn processes, and everyappuse ofCommandHookRunneris behind the same feature, so a separate stub type was not needed. - The runner's shell program can be replaced in tests (
with_shell_program, behindtest-util), so a missing shell is testable as a spawn failure.
Registry loader
install_explicit_sourceidentifies the source by its canonical path andkind, scopes its bindingsEverywhereon the local host, uses the file's directory as the working directory for every binding, and consults no trust policy. Reloading the same file replaces that file's bindings and diagnostics and leaves other sources alone. It does not emitHookRegistryEvent::SnapshotChanged(nothing subscribes yet). Nothing in production calls it; PR 5 replaces or keeps it. Also recorded inquestions.md.- Capability warnings from
validate_capabilitiesbecome per-binding warning diagnostics in the snapshot. Every Cursor shell binding gets one today because the contract putsModelContextin its capabilities and Phase 1 cannot deliver model context.
App wiring
PathCapture(amendment 1):LocalShellState::get_interactive_path_env_varneeds&mut selfand aModelContext, and returns a future, so a plainfn() -> Option<OsString>cannot call it.PathCaptureis nowArc<dyn Fn() -> Option<OsString> + Send + Sync>.HookRuntime::install_command_runnerbuilds the runner with a closure over a shared slot and fills the slot when the capture resolves. Until then, and on surfaces whereLocalShellStatedoes not exist, hooks inherit the processPATH.- The interactive capture starts at startup, but only when
FeatureFlag::AgentHooksis on andLocalShellStateis registered, because it spawns an interactive login shell. With the flag off (every channel today) startup does no extra work. - The runner is installed from
initialize_appright afterLocalShellState, rather than where PR 1 registeredHookRuntime, becauseLocalShellStatedoes not exist yet at that point. - Dependencies:
crates/ai/Cargo.tomlgainscommandunder the non-wasm dependencies (it depends only onwarp_errors,log, andasync-process), andasync-processitself so the runner can name theChildtype, aslspalready does.Cargo.lockrecords the new edges.
Departures from architecture.md
- Amendment to PR 1 (
dc9114902):PathCaptureincrates/ai/src/hooks/runner/mod.rsbecame a shared closure instead of a function pointer, for the reason above. - Amendment to PR 1 (
dcd155bf8):HookConfigErrorincrates/ai/src/hooks/adapters/mod.rsgained anUnreadablevariant for a source file that cannot be read before parsing starts. - Follow-through for PR 3's amendment to PR 1 (
aa6c0d0a4, formatting8640b2e15): the minimal Cursor adapter'sdecode_outputtakes the newDecodePolicyparameter (unused here) and its bindings setloop_limit: None, so this branch builds on the amended trait. PR 3 replaces this adapter. - Follow-through for PR 4's amendment to PR 1 (
1eb3e0126):HookProcessOutcomegainedstdout_truncated(PR 1 commit17c9e32b1), so this branch's minimal runner and its decode tests set the field tofalsewhere they build outcomes; the minimal runner never truncates. - §9.1 describes the runner with a termination grace period, output limits, bounded concurrency, a scrubbed environment, and Windows Job Objects. This PR has a single kill, no output limits, and no concurrency bound. The hook inherits Warp's full process environment with no scrub policy, plus
PATH, the invocation's variables, andWARP_HOOK_PROTOCOL_VERSION; every variable in an Oz runner's environment therefore reaches hook scripts until PR 4 lands the scrub list. PR 4 owns all of these, and should also decide what to do about a background process a hook leaves holding its pipes (this PR stops reading after one second and leaves it running). - §7.3 and §9.1 say a project file's relative commands resolve against the project root and its bindings apply within that root. The stopgap loader resolves against the file's own directory and applies everywhere, for every source kind. PR 5 owns discovery and scoping.
- §8 says activation goes through
TrustPolicy. The stopgap loader does not consult it. Explicit sources are approved by construction and no production code calls the loader yet. - Files outside this PR's ownership that changed:
app/src/lib.rs(one guarded call toHookRuntime::install_command_runnerafterLocalShellStateis registered),app/src/ai/blocklist/hooks/mod.rs(the runner wiring the contract asked for;set_runnerlost its unused-code allowance because it now has a production caller),app/src/ai/blocklist/action_model/execute_tests.rs(the one test the contract asked for),crates/ai/src/hooks/runner/mod.rs(onepub mod command;line),crates/ai/src/hooks/mod.rs(one test module line), andCargo.lock(the new dependency edge).
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)'— passed (95 tests)cargo check -p warp— passedcargo nextest run -p warp -E 'test(hooks)' --no-fail-fast— passed (51 tests)cargo nextest run -p warp --no-fail-fast -E 'test(ai::blocklist::action_model) | test(ai::blocklist::hooks)'— passed (266 tests)cargo clippy -p ai --features local_fs,test-util --all-targets --tests -- -D warnings— passedcargo clippy -p warp --all-targets --tests -- -D warnings— passedcargo check -p ai --target wasm32-unknown-unknown— not usable here: checkingaialone for wasm fails insidewarpui_core(feature unification, unrelated to this PR) before reachingai. CI's whole-workspace wasm clippy is the check for thelocal_fsgating../script/format— ran last
Notes on the tests:
- The process-backed tests (
runner/command_tests.rs,e2e_slice_tests.rs, and theapptest) are#[cfg(unix)]: they write#!/bin/shscripts. Windows gets its.cmdtests with the full runner in PR 4. - The
apptest runs without an environment variable guard; the executor test harness spawns processes fine underApp::test. If CI cannot, the fallback the contract allows is aWARP_HOOKS_E2E=1guard. - Two pure parsing tests were marked
LEAKby nextest in single runs under heavy parallel load and passed cleanly on every rerun; the same mark appeared on an unrelated pure test in PR 1's runs.
New tests, by module:
-
adapters/cursor/config: a minimal file; timeout, matcher, andfailClosed; unknown events skipped and reported; nohooksobject; invalid JSON; missing or zeroversion; missing or blankcommand; a prompt hook skipped while the command hook after it keeps its position; an unknown hook type; bad timeouts including values that overflow aDuration; a matcher that does not compile;""and"*"accepted; the event value must be an array. -
adapters/cursor/decode: exit 2 with no output, with messages, and with unreadable output; exit 0 withallow,deny(with and without a message), andask; a deny with only auser_message; messages on an allow; empty output under both failure policies; six malformed shapes including a missingpermission; stdout that is not UTF-8; five failure kinds under both policies; cancellation; control characters removed. -
adapters/cursor(adapter): the table entry; bindings declared in file order with the right ids, capabilities, and timeout; a skipped prompt hook becomes a warning and the next binding keeps its index; matcher semantics (unanchored, case-sensitive, match-all forms); non-shell envelopes never match; the full stdin shape; absent-or-null fields when the envelope lacks them; a bad matcher rejects the file. -
runner/command: stdin delivery and stdout capture; exit code and stderr; stderr tail; a signalled process; working directory, injectedPATH, protocol version, and invocation variables; inheritedPATHwhen nothing was captured; the command string runs as written; a timeout kills the script and its background child; the cancel flag stops a running hook; a pre-set flag spawns nothing; a missing shell and a missing working directory are spawn failures; a hook that floods stderr before reading a 300 KB stdin does not deadlock; a hook that exits 2 while a background child holds its pipes is reported as exit 2 with its output, within the pipe grace, and the child is left alone; a hook that never reads stdin still finishes. -
e2e_slice_tests(criteria 1 to 4): deny withagent_messageand a routed user notice; silent exit 2; allow; empty output fails open; non-JSON denies;failClosedwith exit 1 denies while fail-open does not; the Cursor stdin shape as the script saw it; the hook runs in the file's directory; a timed-out script is killed and its pid is gone; a fail-closed timeout denies; a non-matching matcher leaves the command alone; reloading a file replaces its bindings; a missing or invalid file is an error that changes nothing. -
execute_tests(criterion 5): a real script loaded throughinstall_explicit_sourceand run throughCommandHookRunnerdenies a shell action withHookDenied { command, reason }, and the recording adapter saw nothing. -
I have manually tested my changes locally with
./script/run
Manual testing is deferred to the PR 9 instructions; the flag is off and no source activates in production yet.
Agent Mode
- Warp Agent Mode - This PR was created via Warp's AI Agent Mode
CHANGELOG-NONE
Activity
23This 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.