#16588

hooks: discover project files, trust by posture, pin the run

Draftvorporealwants to merge intofrom

Description

Summary

Fifth PR of the agent hooks Phase 1 stack. It gives the registry real sources and a real trust decision. A HookConfigWatcher finds .cursor/hooks.json at each project root the app hands it and reloads it on change; explicit sources (WARP_HOOK_DIRS directories, run configuration entries, factory definitions) are loaded once through load_explicit_sources; the registry publishes an immutable snapshot with a higher revision on every change and keeps the last valid configuration when a reload fails; a static TrustPolicy encodes the posture rule; the active set is pinned when the run's first request goes out; hook files and the scripts they name become protected write paths, in sandboxed runs too; and bindings are ordered by precedence and deduplicated by the resolved file. It replaces PR 2's stopgap install_explicit_source.

  • crates/ai/src/hooks/registry.rs: HookRegistry keeps one SourceState per HookSourceId (its LoadState: never loaded, loaded, stale with the last valid parse and the failure, or failed; the trust decision; the roots a project file was found under; whether it was present at pin time; and changes seen after pinning). placements says where one source's bindings apply and the directory its hooks run in. upsert_source, record_read_failure, remove_source, remove_source_root, set_trust, and pin each rebuild the snapshot and return the new revision. The snapshot carries project_roots and protected_paths next to the bindings and diagnostics. bindings_for applies a project binding only when its root is the nearest project root containing the working directory, applies explicit bindings everywhere, and sorts by HookSourceKind::priority() with each source's declared order kept. resolve_timeout rejects a definition above HOOK_TIMEOUT_CEILING. HookRegistry::change applies a change through a model handle and emits HookRegistryEvent::SnapshotChanged when the revision moved.
  • crates/ai/src/hooks/trust.rs: StaticTrustPolicy, an exhaustive table over surface, source kind, and posture. Explicit sources activate everywhere (their loader activates them without asking the table; the rows are there so the table is complete); a project file activates only on OzRun and CliRun with AutoApproves; RequiresApproval, Unknown, every interactive surface, and a local child are inert, each with its own reason. CommandExecutionPosture::strictest and strictest_of fold several postures into the most cautious one; with none to fold the posture is unknown. DenyAllTrustPolicy stays for tests.
  • crates/ai/src/hooks/sources.rs: WARP_HOOK_DIRS_ENV, parse_hook_dirs_value (the WARP_SKILL_DIRS grammar, with a test that compares the two parsers), resolve_hook_dirs, sources_for_hook_dirs, ExplicitHookSource, ExplicitSourceScope, and load_explicit_sources (behind local_fs), which reads each file, identifies it by its canonical path, and activates it without consulting the trust policy. It returns every error the registry recorded for the files it loaded (see "Contract for PR 8" below).
  • crates/ai/src/hooks/watcher.rs (behind local_fs): HookConfigWatcher, a model that owns one BulkFilesystemWatcher and, for each root it is given, watches only the root directory and its .cursor directory for the hook file. It reads the file on the spot when a root is added and when a debounced change touches the file or the .cursor directory, and upserts it with the trust decision the policy gives for the current surface and posture. add_root returns a future that resolves when the platform watch is attached; set_trust_inputs decides trust again for every loaded root. watch_matrix(surface, is_cloud_prepared_root) is the explicit form of what FileMCPWatcher::should_watch_repository decides.
  • app/src/ai/blocklist/hooks/mod.rs: HookRuntime subscribes to DetectedRepositories and adds roots the surface watches to the watcher (created on the first root, so a process that never visits a repository starts no file watcher); subscribes to the history model and calls pin_for_run on the first AppendedExchange; pin_for_run reads the posture, re-trusts the watcher's roots, and pins the registry. command_execution_posture reads the active profile's execute-commands setting through each open window's team policy and takes the strictest. The remote server daemon does neither (process_runs_agents).
  • crates/warp_core/src/execution_mode.rs: AppExecutionMode::is_remote_server_daemon, next to is_tui.
  • app/src/ai/blocklist/permissions.rs: check_protected_write_paths now takes the context. The MCP config check keeps its sandbox exemption; the hook source check applies everywhere and compares the edit's path after resolving symlinks. Protected files are matched by absolute path, so can_write_files says its paths must already be resolved against the session's working directory.
  • app/src/ai/blocklist/action_model/execute/request_file_edits.rs: should_autoexecute resolves the model's paths against the session's working directory before the permission check, the way the read-files executor and the diff application already do. Before this, an edit of .cursor/hooks.json written relative to the session directory never matched a protected path and was applied under an auto-approving profile.

Contract for PR 8

load_explicit_sources(&[ExplicitHookSource], &mut HookRegistry) -> Vec<HookDiagnostic> returns every DiagnosticSeverity::Error the registry recorded for the files it was given: a file that is missing or unreadable, a file that does not parse, a definition the registry rejected (for example a timeout above the ceiling), and a load that came after the run was pinned ("explicit sources must be loaded before the run starts"). An empty result means every named file loaded and its definitions may run. Warnings (unknown events, capability gaps) are not returned; they are in the snapshot's diagnostics. Call it before HookRuntime::pin_for_run, or before the first request goes out; a run that wants fail-closed behavior treats a non-empty result as a reason not to start.

Decisions

Trust and pinning (§8)

  • Posture mapping (§8): the active execution profile's execute-commands setting, read through BlocklistAIPermissions::get_execute_commands_setting so a workspace rule that replaces it is seen. AlwaysAllow is AutoApproves; AlwaysAsk and AgentDecides are RequiresApproval; Unknown is Unknown. The setting is read through every open window's team context and the strictest answer wins (RequiresApproval over Unknown over AutoApproves). A process with no window gets Unknown, which keeps every project file inert, so the rule rests on headless runs having a window. They do: warp agent run, an Oz run inside its sandbox (the same command, started from the execution bootstrap), and a pre-warmed sandbox (standby::prepare_sandbox) all go through AgentDriver::create (app/src/ai/agent_sdk/driver.rs), which calls TerminalDriver::create → create_terminal_view (app/src/ai/agent_sdk/driver/terminal.rs) → open_new_with_workspace_source (app/src/root_view.rs), and that is ctx.add_window. The terminal view the run's conversation lives in is in that window. Tests: the_posture_is_read_through_an_open_window_and_is_unknown_without_one (a headless App::test with an AlwaysAllow profile is Unknown with no window and AutoApproves once one is open) and the_pin_records_the_posture_read_through_the_window. The per-conversation auto-approve toggle (autoexecute_any_action) does not count.
  • Pin moment (§8): the first BlocklistAIHistoryEvent::AppendedExchange in the process, which is when the first request of a run is prepared. It was chosen over prompt.submit for two reasons: notify_prompt_submit only runs when some binding is already active, and project files have no active binding before they are trusted, so pinning there could never happen for a run with only project hooks; and an Oz run's first prompt is resolved server-side (StartFromAmbientRunPrompt), so no prompt.submit fires on the client while an exchange is still appended. pin_for_run is public and a second call does nothing, so PR 8 can call it at execute_run as well. Pinning is once per process; a long-lived GUI session would keep the set from its first prompt, which Phase 1.5 revisits with per-root approval.
  • Project files are inert until the run starts (§8): the watcher trusts every project file with CommandExecutionPosture::Unknown until pin_for_run, which the table maps to inert, then decides again with the posture read at that moment and pins. A command-line run applies --profile (AgentDriver::configure_terminal) after the terminal has navigated into its repository, so a decision made on discovery could use the default CLI profile for a run whose real profile asks for approval.
  • Posture is read once (§8): PinnedRun { posture, revision } is stored on the registry; a profile change after pinning does not re-trust anything.
  • After pinning, nothing changes (§8): an upsert, a read failure, a removal, or a trust change for a pinned source bumps the revision and records one warning per distinct message, and leaves the bindings as they were. A source first seen after pinning is parsed, so its file and scripts are protected, but it is inert: a project file waits for the next run start (warning), and an explicit source is an error the loader returns. A removed project file keeps its hooks until the run ends, because dropping them would let a shell command delete the file to escape a policy hook. The set of projects is pinned too (below).
  • The daemon is not a surface (§9.1, §10 item 7): the remote server daemon goes through initialize_app and is_autonomous() is true for it, so envelope::surface would call it CliRun. HookRuntime neither watches detected roots nor pins there, so no file is read on a remote host and nothing can activate if a conversation ever lands in the daemon.
  • Two counters on the snapshot (§3 Registry versioning, §6.1): every change, including a failed reload and a change after pinning, publishes a new snapshot and moves generation, which is what SnapshotChanged and the diagnostics reporting follow. revision, which pre-tool verdicts and envelopes are keyed on, moves only when the bindings that can run or the project roots that scope them differ from the previous snapshot. So a touch of a pinned hook file, a reload that fails and keeps the last valid configuration, or a reload with the same definitions does not drop an in-flight verdict or run a hook twice, while a change that could alter bindings_for still does. Changes to unknown sources move neither.
  • Which detected roots are watched (§7.3, §10 item 5): roots found by terminal navigation on every surface; cloud-prepared roots only on OzRun and CliRun; roots found by project rules indexing or code review initialization never, since they are not where the agent's session is.

Registry shape (§3, §6.4, §7.3, §7.4)

  • Source identity: a file's canonical path (dunce::canonicalize), so a symlink and its target are one source and two files with the same text are two. A .cursor/hooks.json that is a symlink to a file outside the repository is still that repository's project source, scoped to the root that holds the link, with the target as its protected path; the repository chose to ship the link, and the posture rule is what admits it. A project file reached from two roots is one source that applies under each root; remove_source_root withdraws one root and the source is forgotten when no root is left.
  • One file, two kinds (§6.3): a file delivered with the run (WARP_HOOK_DIRS=<root>/.cursor is the natural way to force a repository's hooks on under a profile that asks) and also found as that repository's project file is loaded under both kinds but runs once, as the explicit source; the project load gets an info diagnostic and contributes no bindings while the explicit one exists.
  • Nearest root (§3, §7.3): the project for a working directory is the deepest known root that contains it, where the known roots are every root the watcher was given plus the root of every project file that loaded, with or without a hook file. A project with no hook file, or with a broken one, has no project hooks, and an enclosing project's hooks do not reach into it, which is what FileBasedMCPManager::get_servers_for_working_directory does for MCP configs. The working directory is canonicalized when it exists, because roots are canonical.
  • The project set is pinned with the run (§8): a root detected after the run started (a directory the agent ran git init in, or cloned) would otherwise take its working directory away from the enclosing project's hooks, so add_project_root after pinning only records a warning, and remove_project_root does nothing.
  • Working directory (§7.3, §9.1): working_directory_for from PR 4. A named explicit source with no file has no directory, so its definitions are not activated and the source gets an error diagnostic; nothing falls back to Warp's own working directory.
  • Protected-path resolution rule (§8): every source file the registry knows, whether it loaded, is inert, is stale, or never parsed, plus each definition's script when it resolves (dunce::canonicalize) to a file relative to the source's working directory: the first word of the command (./check.sh --strict), or the second word when the first names a program rather than a path (bash scripts/check.sh, node ./hooks/format.js, python3 hooks/audit.py). npx foo resolves to nothing and is not protected; a script named with a quoted path containing spaces is not resolved either. Scripts are resolved when the snapshot is built, so a script created after the file loaded is protected at the next reload. Inert files are protected because they can activate at the next run start; a file that never parsed is still a hook file the agent should not repair unasked.
  • Protected-path comparison (can_write_files): the edit's path resolved against the session's working directory by the caller, then after resolving symlinks when the file exists, otherwise as given. A path left relative cannot match a protected file, which is why the file-edit executor resolves before it asks. This is an agent file-edit guard, as §8 says; shell commands can still change these files.
  • Timeout ceiling (§6.4): adapters already fill in the boundary default, so resolve_timeout only applies the ceiling; a definition above 600 s is skipped with an error diagnostic and the rest of the file loads.
  • Diagnostics: a source's snapshot diagnostics are its parse warnings, its activation warnings (capability gaps, rejected definitions), an info line when it is inert (with the policy's reason), its latest load failure (which says whether a previous configuration stays in effect), and its changes after pinning.
  • install_snapshot_for_test now installs a set that rides along with every snapshot the sources produce, so pinning or reloading a source in a test does not drop the test doubles.
  • is_active is not cached: it reads snapshot().is_empty() through an Arc clone, which is already one lookup and one pointer copy, and a cached flag would be one more thing that can go stale on a missed notification. SnapshotChanged is emitted for anyone who needs it.

Watcher (§3, §10 item 5)

  • Loads are synchronous: the file is small and changes rarely, and reading it on the spot means a root added before the run starts is in the registry before the pin happens, with no startup race between an async read and the first request.
  • The platform watch attaches on a background thread, which can take over a second (0.5 to 1.4 s was measured on one machine). add_root loads the file first and returns the future that resolves when the watch is attached; the app does not wait on it, so a change made in that window is not seen until the next one. The real-watcher test waits on it before writing, which is what makes it deterministic.
  • One watch per root, filtered to the root directory and its .cursor directory, so a large repository costs no more than a small one. The watcher reloads a root whenever the hook file or the .cursor directory appears in an event, and the state on disk decides what happens, so an atomic replacement reported as a delete plus an add loads once.
  • Debounce: BulkFilesystemWatcher with a 500 ms window; a burst of writes arrives as one event and loads once.
  • No wasm stand-in for HookConfigWatcher, as PR 2 decided for the runner: the app's uses are behind local_fs. watch_matrix and the sources parsing compile everywhere.

Departures from architecture.md

  • Amendment to PR 1 (80cdb1627): surface in app/src/ai/blocklist/hooks/envelope.rs is pub(super) so HookRuntime can tell the trust policy which surface the process fires boundaries from.
  • §8 says the active hook set is pinned at run start. This PR pins on the first appended exchange, which is the first request of the run; PR 8 may also call pin_for_run at execute_run. Between process start and that moment, project files are discovered but inert, so no project hook fires before the posture is known; a project sessionStart hook therefore does not fire for the first conversation until PR 7 moves session.start to the barrier before the first request.
  • §3 says HookConfigWatcher publishes revisions and diagnostics with generation tags. Loads are synchronous here, so there is no in-flight parse to tag.
  • The contract's upsert_source(source, bytes, format, kind, trust) is upsert_source(SourceUpdate { source, format, bytes, project_root, trust }); the kind travels in HookSourceId and the project root is needed for scoping and the working directory.
  • Files outside this PR's ownership that changed: crates/warp_core/src/execution_mode.rs (AppExecutionMode::is_remote_server_daemon, so the hook runtime can keep the daemon out, and a test-only set_sandboxed_for_test), app/src/ai/blocklist/action_model/execute/request_file_edits.rs (should_autoexecute resolves paths against the session's working directory; the smallest change that closes the relative-path hole, since only the executor knows the session), crates/ai/src/hooks/dispatch_tests.rs and crates/ai/src/hooks/adapters/cursor/mod_test.rs (one ..Default::default() each, because RegistrySnapshot gained two fields), app/src/ai/blocklist/action_model/execute_tests.rs (the real-process test now loads its file through load_explicit_sources), and app/src/ai/blocklist/hooks/mod_test.rs (new; tests for the additions to hooks/mod.rs). crates/ai/src/hooks/e2e_slice_tests.rs moved from the stopgap loader to load_explicit_sources, and its missing-file test now expects returned failures rather than an error.

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 check -p ai --features local_fs,test-util — passed
  • cargo nextest run -p ai --features local_fs,test-util -E 'test(hooks)' --no-fail-fast — passed (246 tests on this branch, 271 at the top of the stack; the real-watcher test also passed five times in a row on its own)
  • cargo check -p warp — passed
  • cargo nextest run -p warp -E 'test(hooks) | test(protected)' --no-fail-fast — passed (68 tests on this branch, 122 at the top of the stack)
  • cargo nextest run -p warp --no-fail-fast -E 'test(ai::blocklist::action_model) | test(ai::blocklist::controller) | test(ai::blocklist::hooks) | test(ai::blocklist::history_model) | test(ai::blocklist::permissions) | test(ai::agent::conversation)' — passed (520 tests, before the later rounds of changes)
  • 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_core --all-targets --tests -- -D warnings — passed
  • cargo clippy -p warp --all-targets --tests -- -D warnings — passed
  • ./script/format — ran last

New tests, by module:

  • registry: valid, malformed, valid publishes three snapshots with the malformed step keeping the bindings, the revision, and marking the source stale; a reload with the same definitions keeps the revision; the generation moves on every change and the revision only with the active set; a first load that fails is inactive with an error, still counts as a project, and its file is protected; a read failure behaves like a failed parse; a known root without a hook file is its own project, a broken inner file does not hand the directory back to the outer project, and removing the root does; a root found after pinning is reported and the project set does not change; after pinning, a changed file, a removal, and a trust change each publish a snapshot, keep the revision, and are reported but not applied, a new project file is parsed and protected but inert, and a new explicit source is an error; a second pin changes nothing; the revision only moves forward; unknown sources do not move it; precedence (FactoryAgent, FactoryWide, RunDelivered, ProjectCursorFile), nearest root, and a cwd outside every root; array order within a source; a project file runs in its root; an inert source is reported and still counts as a project root; a symlink and its target are one source and two files are two; a file delivered with the run and found as a project file runs once, as the explicit source; a file reached from two roots applies under each; protected paths cover the file and every script that resolves (first-word and interpreter forms), resolve explicit scripts against the declaring directory, include inert and stale sources, and are recomputed on removal; a named explicit source with no directory does not activate; the timeout ceiling, both as a function and as a skipped definition.

  • trust: every (surface, kind, posture) cell; project files inert on interactive surfaces even with AutoApproves; Unknown and RequiresApproval inert on headless surfaces; strictest prefers needing approval, then not knowing, then auto-approving.

  • sources: comma splitting, trimming, blanks, ~, relative resolution; the hook and skill parsers agree on seven values; directory to source mapping; scope to kind; explicit sources activate without the trust table and in priority order; a missing file, a file that does not parse, a rejected definition, and a load after the run was pinned are each a returned failure; a valid file with warnings returns none; a directory named twice through a symlink loads once.

  • watcher: the watch matrix; adding a root announces the project and loads its file scoped to that root; adding it twice loads once; create, modify, and delete each update the registry once, and the directory stays a project without its file; an event that touches the file several ways loads once; unrelated paths do not reload; removing .cursor withdraws the source; a rewrite that does not parse keeps the previous bindings; two roots are independent; a nested root without a hook file is its own project until it is removed; removing a root withdraws its source and the project; trust is decided again from the posture when the run starts and never activates on an interactive surface; a root added after the posture is known uses it; a change after pinning is reported and not applied, and a file found late is protected; through the real file system watcher, a burst of writes loads the file once and files the filter does not report change nothing.

  • hooks (app): the posture fold; the posture follows the active profile and ignores the conversation toggle; the real posture read is Unknown without a window and AutoApproves or RequiresApproval through an open window; the pin records the posture read through the window; the first appended exchange pins the registry once; nothing pins with the flag off; a detected root feeds its file to the registry, inert on the desktop surface, with the file protected; roots the surface does not watch and roots seen with the flag off load nothing; the remote server daemon neither watches a detected root nor pins.

  • permissions: hook files and their scripts are protected when writes auto-approve, in sandboxed runs (where the MCP config exemption is unchanged), and when the conversation auto-approves; a symlink to a hook file is protected; nothing is protected with the flag off.

  • action_model (executor): a RequestFileEdits naming the hook file or its script relative to the session's working directory waits for confirmation even though diffs auto-approve, in and out of a sandbox, while a file beside them goes through.

  • 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 project file can activate on an interactive surface yet.

Agent Mode

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

CHANGELOG-NONE

Activity

8
vorporeal9d 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
  • #16602
  • #16593
  • #16590
  • #16588 👈 (View in Graphite)
  • #16583
  • #16582
  • #16579
  • #16577
  • #16558
  • master

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

This pull request is a draft

Merging is unavailable until it is marked ready for review.