Description
Summary
Ninth PR of the agent hooks Phase 1 stack. It adds no product code. It proves the Phase 1 test contracts in architecture.md §11 end to end, with real sh scripts run by the real CommandHookRunner, through the same seams the agent uses: the registry and its loaders, the dispatcher, the action model, the controller, and the agent driver. It also checks in a sample hooks project a person can point warp agent run at. The manual test procedure for that sample is in the run's progress notes, not in the repository.
crates/ai/src/hooks/fixtures/:sample_project/.cursor/hooks.jsonwith five events (sessionStartsets one variable,beforeSubmitPromptlogs the prompt length,beforeShellExecutiondeniesrm -rfandgit push --forceand allows the rest,afterShellExecutionappends to.hook-audit.log,stopasks for nothing), four POSIXshscripts (nojq; checked underdashtoo; committed as100755),sample_project/run_hooks/hooks.jsonbinding the same scripts for use with--hook-dir,malformed/.cursor/hooks.json(a bad matcher and an unknown event), and a README.crates/ai/src/hooks/e2e_tests.rs: tests that need onlyai. PR 2'se2e_slice_tests.rsis folded in (asshell_slice) and deleted.app/src/ai/blocklist/hooks/e2e_tests.rs: tests that need the loop, with the realHookRuntimeand runner.
Every test copies its fixture into a temporary directory and sets the executable bit after copying. Every runner gets an explicit HOME and PATH (/usr/bin:/bin) through CommandHookRunner::with_home and the path capture, every file a hook writes lands in a temporary directory, and nothing calls std::env::set_var.
Coverage of the §11 Phase 1 test contracts
| §11 clause | Test | File | What it proves |
|---|---|---|---|
| Adapter and reducer fixtures | adapter_and_reducer_fixtures | crates/ai/src/hooks/e2e_tests.rs | The sample parses into its five bindings; a run-delivered deny and the project's deny reduce in precedence order with both reasons. |
| Real-subprocess stdin | real_subprocess_stdin | crates/ai/src/hooks/e2e_tests.rs | deny_dangerous.sh receives the documented beforeShellExecution fields on stdin. |
| Malformed and empty output | malformed_and_empty_output | crates/ai/src/hooks/e2e_tests.rs | Garbage and empty stdout follow each failure policy on beforeShellExecution and beforeSubmitPrompt. |
| Timeout, cancellation, output limit | timeout_cancellation_output_limit | crates/ai/src/hooks/e2e_tests.rs | A timeout fails open and the process is gone; a cancel mid-run stops it and marks the outcome cancelled; valid JSON padded past 1 MiB is cut off and denied. |
| Permission matrix (run-to-completion) | permission_matrix_run_to_completion | app/src/ai/blocklist/hooks/e2e_tests.rs | On a Bash session Warp can classify, Warp first runs each command on its own; then the denylist still refuses a command both hooks allow, a headless ask denies with its reason, a hook deny holds when the user approves mid-decision, and under conversation auto-approve the project's deny of git push --force holds. |
| Batch denial with cwd changes | batch_denial_with_cwd_changes | app/src/ai/blocklist/hooks/e2e_tests.rs | After cd nested runs in a three-command batch, the third command is judged only by the nested repository's .cursor/hooks.json. |
| Stop versus Oz idle exit | stop_versus_idle_exit | app/src/ai/blocklist/hooks/e2e_tests.rs | Through the driver's own completion logic, the run's exit receiver stays empty and no Success status appears while a stop hook queues a follow-up. |
| Stale registry revision | stale_registry_revision, a_pinned_hook_file_touch_keeps_the_verdict_in_flight | app/src/ai/blocklist/hooks/e2e_tests.rs | An unpinned run-delivered reload moves the revision and the in-flight hook runs once more; a pinned project file touch moves only the generation, records a diagnostic, and the verdict in flight stands. |
Duplicate session.start | duplicate_session_start | app/src/ai/blocklist/hooks/e2e_tests.rs | session.start fires once on the first request, not on creation, the driver run, later prompts, or a conversation that already has exchanges. |
| Reload valid, malformed, valid, with pinning | reload_valid_malformed_valid_with_pinning, the_malformed_fixture_loads_what_it_can_and_reports_the_rest | crates/ai/src/hooks/e2e_tests.rs | The real watcher keeps the prior bindings on a malformed write, applies the fixed write, and treats post-pin writes as diagnostic-only. |
| No orphans after timeout | no_orphans_after_timeout | crates/ai/src/hooks/e2e_tests.rs | The timed-out hook and its background child that ignores TERM are gone within the kill grace (Windows version written and ignored). |
| Once-per-message response observations | once_per_message_response_observations | app/src/ai/blocklist/hooks/e2e_tests.rs | Text, reasoning, tool call, text runs afterAgentResponse twice and afterAgentThought once. |
| Posture rule activation | posture_rule_activation, an_inert_project_file_says_it_waits_for_a_profile_that_runs_commands | app/src/ai/blocklist/hooks/e2e_tests.rs | The project file activates on the command line under always-allow and stays inert with a diagnostic under always-ask; a run-delivered source activates in both. |
| Protected hook paths | protected_hook_paths | app/src/ai/blocklist/hooks/e2e_tests.rs | Through the action model, in and out of a sandbox, a RequestFileEdits for .cursor/hooks.json (relative and absolute), a hook script, or run_hooks/hooks.json never starts and the conversation is left Blocked; a file beside them is not held. |
| Deny reason reaches the wire | deny_reason_reaches_the_wire | app/src/ai/blocklist/hooks/e2e_tests.rs | A shell denial fills permission_denied.hook_denied.reason with the script's text (the empty-result half is under "Not covered"). |
| Oz messenger output | oz_messenger_output | app/src/ai/blocklist/hooks/e2e_tests.rs | In Ndjson, a denial is one hook_notice line and a hook directory that does not parse is one hook_diagnostic line. |
| Latency | a_trivial_hook_adds_under_two_seconds | crates/ai/src/hooks/e2e_tests.rs | A warm invocation of a trivial shell hook finishes in under 2 seconds. |
Not covered, and why:
-
The empty-result half of the deny-reason clause. A
preToolUsedeny ofInitProjectfinishes with its cancelled result and the reason reaches only aDeniednotice, as PR 6 decided (questions.md, "PR 6: a denied action with no text slot ends the turn as cancelled, or reports completion; should the proto slot move into Phase 1?");deny_reason_reaches_the_wirepins that. -
The byte-for-byte write to the process's stdout.
oz_messenger_outputrecords through the Oz messenger's ownwrite_noticeandwrite_diagnosticinto a buffer;with_stdout_bufferedcannot be captured in a test. -
Operating-system file events reaching
HookRuntime's own watcher. The app test feeds that watcher a file event directly; real event delivery is covered by theaireload test, which uses the real watcher. -
The driver's own first-prompt path. The driver tests start the run with no initial turn and send the first prompt through the controller, which reaches the same
send_queryandprepare_turn. -
Windows: written, not run (no Windows machine or target here).
Decisions
- Fixture commands are written relative to the project root (
.cursor/scripts/deny_dangerous.sh), as Cursor's project hooks are, so the protected-path rule can resolve them (§7.3, §8). Because a run-delivered file runs from its own directory (§9.1), the fixture also shipsrun_hooks/hooks.jsonwith../.cursor/scripts/...commands for use with--hook-dir. - The malformed fixture follows PR 3: a bad matcher and an unknown event are warnings, and the file still loads. The "malformed" step of the reload test therefore writes a truncated copy of the sample, which does not parse (§6.4).
- Hermetic hooks: the runner's
HOMEandPATHare injected per test instead of changing the process environment (contract design constraints). - The deny script writes its latest input to
$WARP_PROJECT_DIR/.hook-last-shell-input.json, so tests and people can see exactly what it received without a separate debug switch. - A project's own hook file only activates on a headless surface, and the surface follows the execution mode, not the recorded run alone (§8). The loop tests that need a command-line run therefore record the run and switch the test app's
AppExecutionModetoSdkthrough its model handle; no product code changes. Those tests also register a Bash session as the harness's active session, so Warp can classify commands and give its own permission answer; without one it never auto-executes anything and a headless run refuses every command asDenylisted, which hid the denylist and auto-approve cases (review round 1). once_per_message_response_observationsuses a small audit script that writes to its own directory rather than the sample'saudit.sh, because the controller harness's session has no temporary working directory for$WARP_PROJECT_DIRto point at, and the test must not write outside a temporary directory.protected_hook_pathssends each edit through the action model and tells "held for confirmation" from "not ready" by the conversation's status, since the action model reports both as a blocked action. The harness prepares no diffs, so the unprotected edit goes on to execution and waits there.- The driver tests (
stop_versus_idle_exit,duplicate_session_start,oz_messenger_output) use the realAgentDriverthrough a test-only entry point (see Departures) so the run's completion and hook loading are the driver's own code. - The latency test runs the trivial hook once untimed and times the second run against the contract's 2 second bound: the first run of a freshly written script also pays for loading it (and, on machines with endpoint scanning, for the scan), which is not what a hook adds to each operation (review round 1).
- The sample's scripts keep commands relative to the project root rather than
"$WARP_PROJECT_DIR"/.cursor/scripts/.... A command that starts with a variable cannot be resolved by the protected-path rule, so the four scripts would stop being protected; keeping them protected is the cautious choice.run_hooks/hooks.jsoncovers--hook-dir. - Tests that use only a run-delivered hook in its own directory start from
Fixture::empty()(a temporaryworkroot andhome) rather than a copy of the sample project, so each test sets up only what it reads (style round). - No test needs the
WARP_HOOKS_E2E=1guard: real processes spawn fine underApp::test, as PR 2 found. - The Windows no-orphans test is behind
#[cfg(windows)]and#[ignore]with its reason, as the contract allows for tests that cannot be verified here.
Departures from architecture.md
None. The test-only support edits outside this PR's files:
app/src/ai/agent_sdk/driver/hooks.rs(PR 8's file):#[cfg(test)] AgentDriver::start_run_with_hooks_for_test, which sets a run'sHookRunOptions, loads its hook sources, and startsexecute_runwith no initial turn, so tests outside the driver module can drive the driver's own completion logic.app/src/ai/blocklist/hooks/mod.rs(PR 1's file):#[cfg(all(test, feature = "local_fs"))] HookRuntime::config_watcher_for_test, so a test can hand the runtime's project file watcher a file event; thee2e_testsmodule line; and, in thecfg(test)testingmodule, asessionsfield onHookTestApp(the mock session'sSessionshandle) so a test can register a shell session.crates/ai/src/hooks/mod.rs: thee2e_slice_testsmodule line now namese2e_tests;e2e_slice_tests.rsis deleted (its tests moved).
Findings
a_project_file_also_named_as_a_hook_dir_runs_from_its_cursor_directory(crates/ai/src/hooks/e2e_tests.rs). Contract: PR 5's decision "One file, two kinds" (§6.3, §7.4), which namesWARP_HOOK_DIRS=<root>/.cursoras "the natural way to force a repository's hooks on", read together with §9.1 (a run-delivered source runs in its declaring directory). Observed: when the sample's.cursordirectory is loaded both as the project file and as a hook directory, the registry runs it once, as the run-delivered source, so its commands run from.cursorinstead of the project root. A Cursor-style command (.cursor/scripts/deny_dangerous.sh) is then not found (exit 127),beforeShellExecutionfails open, and the project'srm -rfdenial is lost; the project-file copy, which would have run correctly, is suppressed. The registry follows §6.3, §7.4, and §9.1 literally, so this is a design question rather than a PR 5 defect: §8 promises "an explicit flag or configuration entry" to turn a repository's hooks on under a profile that asks, no such flag exists, and PR 5's text filled the gap with this spelling. Options, now inquestions.md("PR 9: how should a run turn on a repository's own hooks under a profile that asks?") for David: (1) keep project semantics (working directory = the root) for an explicit source that is a detected root's.cursor/hooks.json, and say so in §7.3; or (2) add the §8 flag and stop recommending theWARP_HOOK_DIRS=<root>/.cursorspelling in §7.3, PR 5's description, and the fixture README. The test pins today's behavior; the fixture README and the manual test instructions say not to use that spelling.
Resolved since the first push (no longer findings): the relative-path hole in protected hook paths (a RequestFileEdits for .cursor/hooks.json written relative to the session directory was not recognized) is fixed in PR 5 (RequestFileEditsExecutor::should_autoexecute resolves against the session's working directory), and protected_hook_paths fails with that fix undone. The post-pin revision churn (touching a pinned hook file re-ran every in-flight pre-tool hook) is fixed in PR 5 by the two counters, and stale_registry_revision asserts the fixed behavior.
Routed from review round 2 (not PR 9's code): pre_tool::file_write_floors passes the model's paths to can_write_files unresolved, so the protected_write_path floor is false for a relative hook-file path (latent in Phase 1, where the executor's own answer holds the edit); this goes to PR 6 as an amendment. Two more paths reach the protected-path check unresolved outside the stack, the ACP harness policy and a relative path before the session reports a working directory; both are in questions.md ("PR 9: paths that reach can_write_files unresolved outside the stack") with the conservative option (require confirmation when a relative path cannot be resolved).
Environment note, not a finding: on this machine, under the full parallel suite, process-backed tests can stall for several seconds at spawn (the endpoint security agent). In the round 1 full run, a_trivial_hook_adds_under_two_seconds and no_orphans_after_timeout failed that way; both pass alone, three runs out of three.
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.
cargo check -p ai— passedcargo nextest run -p ai --features local_fs,test-util -E 'test(e2e)' --no-fail-fast— 21 of 22 passed in the first full run; the latency test failed under load and passed alone (environment note above)cargo nextest run -p ai --features local_fs,test-util -E 'test(hooks)' --no-fail-fast— 279 of 280 passed; the one failure was the latency test, under loadcargo nextest run -p ai --features local_fs,test-util -E 'test(a_trivial_hook_adds_under_two_seconds) | test(a_project_file_also_named)'— passed (the latency test also passed alone three times in a row)cargo nextest run -p warp -E 'test(hooks::e2e_tests)' --no-fail-fast— passed (11 tests, after the last test change)cargo nextest run -p warp -E 'test(hooks)' --no-fail-fast— passed (142 tests; nextest marked one test leaky, no failure)cargo clippy -p ai --features local_fs,test-util --all-targets --tests -- -D warnings— passedcargo clippy -p ai --all-targets --tests -- -D warnings— passedcargo clippy -p warp --all-targets --tests -- -D warnings— passed (after switching one import toinstant::Instant, which the lint requires)./script/format— ran last- Fixture scripts run by hand under
/bin/shand/bin/dash.
Review round 1 (after the restack onto PR 5's fixes):
cargo nextest run -p ai --features local_fs,test-util -E 'test(hooks)' --no-fail-fast— 279 of 281;a_trivial_hook_adds_under_two_secondsandno_orphans_after_timeoutstalled at spawn under load and passed alone three times eachcargo nextest run -p warp -E 'test(hooks)' --no-fail-fast— passed (144 tests)cargo check -p ai,cargo clippy -p ai --features local_fs,test-util --all-targets --tests -- -D warnings,cargo clippy -p ai --all-targets --tests -- -D warnings,cargo clippy -p warp --all-targets --tests -- -D warnings— passed./script/format— ran last (oneformattingcommit)- Guards broken on purpose and reverted, each run against the e2e tests: (1) a hook deny loses to Warp's own decision when Warp would run the command —
permission_matrix_run_to_completionfailed at the user-approval case andbatch_denial_with_cwd_changesfailed at the nestedls; (1b) the same but only for actions the user did not approve —permission_matrix_run_to_completionfailed at the run-to-completiongit push --forcecase; (2) the executor treats a hook'sallowas permission to skip Warp's own checks —permission_matrix_run_to_completionfailed at the denylist case; (3) PR 5's relative-path resolution inRequestFileEditsExecutor::should_autoexecuteundone —protected_hook_pathsfailed at.cursor/hooks.json. All pass with the code as committed.
Review round 2 and style round (test helpers, the split stale-revision test, fixture comments and README; no behavior change):
-
cargo nextest run -p ai --features local_fs,test-util -E 'test(hooks)' --no-fail-fast— 280 of 281;a_trivial_hook_adds_under_two_secondsstalled at spawn under load and passed alone -
cargo nextest run -p warp -E 'test(hooks)' --no-fail-fast— passed (145 tests) -
cargo check -p ai,cargo clippy -p ai --features local_fs,test-util --all-targets --tests -- -D warnings,cargo clippy -p ai --all-targets --tests -- -D warnings,cargo clippy -p warp --all-targets --tests -- -D warnings— passed -
Fixture scripts parsed with
sh -nanddash -n; the deny and prompt scripts run by hand under both -
./script/format— ran last (oneformattingcommit) -
I have manually tested my changes locally with
./script/run
Manual testing: the procedure for David (building a warp binary with the flag on, the fixture repository, three warp agent run invocations, and what each should show) is in the run's progress notes. The flag is still off on every channel.
Agent Mode
- Warp Agent Mode - This PR was created via Warp's AI Agent Mode
CHANGELOG-NONE
Activity
12b989e72This 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.