#16605

hooks: end-to-end tests and a sample hooks fixture

Draftvorporealwants to merge intofrom

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.json with five events (sessionStart sets one variable, beforeSubmitPrompt logs the prompt length, beforeShellExecution denies rm -rf and git push --force and allows the rest, afterShellExecution appends to .hook-audit.log, stop asks for nothing), four POSIX sh scripts (no jq; checked under dash too; committed as 100755), sample_project/run_hooks/hooks.json binding 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 only ai. PR 2's e2e_slice_tests.rs is folded in (as shell_slice) and deleted.
  • app/src/ai/blocklist/hooks/e2e_tests.rs: tests that need the loop, with the real HookRuntime and 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 clauseTestFileWhat it proves
Adapter and reducer fixturesadapter_and_reducer_fixturescrates/ai/src/hooks/e2e_tests.rsThe sample parses into its five bindings; a run-delivered deny and the project's deny reduce in precedence order with both reasons.
Real-subprocess stdinreal_subprocess_stdincrates/ai/src/hooks/e2e_tests.rsdeny_dangerous.sh receives the documented beforeShellExecution fields on stdin.
Malformed and empty outputmalformed_and_empty_outputcrates/ai/src/hooks/e2e_tests.rsGarbage and empty stdout follow each failure policy on beforeShellExecution and beforeSubmitPrompt.
Timeout, cancellation, output limittimeout_cancellation_output_limitcrates/ai/src/hooks/e2e_tests.rsA 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_completionapp/src/ai/blocklist/hooks/e2e_tests.rsOn 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 changesbatch_denial_with_cwd_changesapp/src/ai/blocklist/hooks/e2e_tests.rsAfter 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 exitstop_versus_idle_exitapp/src/ai/blocklist/hooks/e2e_tests.rsThrough 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 revisionstale_registry_revision, a_pinned_hook_file_touch_keeps_the_verdict_in_flightapp/src/ai/blocklist/hooks/e2e_tests.rsAn 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.startduplicate_session_startapp/src/ai/blocklist/hooks/e2e_tests.rssession.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 pinningreload_valid_malformed_valid_with_pinning, the_malformed_fixture_loads_what_it_can_and_reports_the_restcrates/ai/src/hooks/e2e_tests.rsThe 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 timeoutno_orphans_after_timeoutcrates/ai/src/hooks/e2e_tests.rsThe 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 observationsonce_per_message_response_observationsapp/src/ai/blocklist/hooks/e2e_tests.rsText, reasoning, tool call, text runs afterAgentResponse twice and afterAgentThought once.
Posture rule activationposture_rule_activation, an_inert_project_file_says_it_waits_for_a_profile_that_runs_commandsapp/src/ai/blocklist/hooks/e2e_tests.rsThe 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 pathsprotected_hook_pathsapp/src/ai/blocklist/hooks/e2e_tests.rsThrough 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 wiredeny_reason_reaches_the_wireapp/src/ai/blocklist/hooks/e2e_tests.rsA shell denial fills permission_denied.hook_denied.reason with the script's text (the empty-result half is under "Not covered").
Oz messenger outputoz_messenger_outputapp/src/ai/blocklist/hooks/e2e_tests.rsIn Ndjson, a denial is one hook_notice line and a hook directory that does not parse is one hook_diagnostic line.
Latencya_trivial_hook_adds_under_two_secondscrates/ai/src/hooks/e2e_tests.rsA 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 preToolUse deny of InitProject finishes with its cancelled result and the reason reaches only a Denied notice, 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_wire pins that.

  • The byte-for-byte write to the process's stdout. oz_messenger_output records through the Oz messenger's own write_notice and write_diagnostic into a buffer; with_stdout_buffered cannot 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 the ai reload 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_query and prepare_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 ships run_hooks/hooks.json with ../.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 HOME and PATH are 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 AppExecutionMode to Sdk through 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 as Denylisted, which hid the denylist and auto-approve cases (review round 1).
  • once_per_message_response_observations uses a small audit script that writes to its own directory rather than the sample's audit.sh, because the controller harness's session has no temporary working directory for $WARP_PROJECT_DIR to point at, and the test must not write outside a temporary directory.
  • protected_hook_paths sends 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 real AgentDriver through 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.json covers --hook-dir.
  • Tests that use only a run-delivered hook in its own directory start from Fixture::empty() (a temporary work root and home) 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=1 guard: real processes spawn fine under App::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's HookRunOptions, loads its hook sources, and starts execute_run with 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; the e2e_tests module line; and, in the cfg(test) testing module, a sessions field on HookTestApp (the mock session's Sessions handle) so a test can register a shell session.
  • crates/ai/src/hooks/mod.rs: the e2e_slice_tests module line now names e2e_tests; e2e_slice_tests.rs is deleted (its tests moved).

Findings

  1. 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 names WARP_HOOK_DIRS=<root>/.cursor as "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 .cursor directory 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 .cursor instead of the project root. A Cursor-style command (.cursor/scripts/deny_dangerous.sh) is then not found (exit 127), beforeShellExecution fails open, and the project's rm -rf denial 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 in questions.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 the WARP_HOOK_DIRS=<root>/.cursor spelling 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-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.

  • cargo check -p ai — passed
  • cargo 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 load
  • cargo 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 — passed
  • cargo clippy -p ai --all-targets --tests -- -D warnings — passed
  • cargo clippy -p warp --all-targets --tests -- -D warnings — passed (after switching one import to instant::Instant, which the lint requires)
  • ./script/format — ran last
  • Fixture scripts run by hand under /bin/sh and /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_seconds and no_orphans_after_timeout stalled at spawn under load and passed alone three times each
  • cargo 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 (one formatting commit)
  • 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_completion failed at the user-approval case and batch_denial_with_cwd_changes failed at the nested ls; (1b) the same but only for actions the user did not approve — permission_matrix_run_to_completion failed at the run-to-completion git push --force case; (2) the executor treats a hook's allow as permission to skip Warp's own checks — permission_matrix_run_to_completion failed at the denylist case; (3) PR 5's relative-path resolution in RequestFileEditsExecutor::should_autoexecute undone — protected_hook_paths failed 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_seconds stalled 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 -n and dash -n; the deny and prompt scripts run by hand under both

  • ./script/format — ran last (one formatting commit)

  • 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

12
vorporeal8d ago

WARNING

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite. Learn more

  • #16605 👈 (View in Graphite)
  • #16602
  • #16593
  • #16590
  • #16588
  • #16583
  • #16582
  • #16579
  • #16577
  • #16558
  • master

This stack of pull requests is managed by Graphite. Learn more about stacking.

D
f1a8157
D
1fa2c54

This pull request is a draft

Merging is unavailable until it is marked ready for review.