Skip to content

feat(agent): Claude Code-compatible project hooks system (#114)#117

Merged
pablof7z merged 1 commit into
masterfrom
feat/issue-114-hooks-system
May 30, 2026
Merged

feat(agent): Claude Code-compatible project hooks system (#114)#117
pablof7z merged 1 commit into
masterfrom
feat/issue-114-hooks-system

Conversation

@pablof7z

Copy link
Copy Markdown
Collaborator

Summary

Implements issue #114: a Claude Code-compatible hooks system. Project-local shell commands fire at pre-tool and post-tool boundaries, configured via .tenex-hooks.json in the agent's workspace directory. Hooks receive JSON context on stdin and can block a tool call (non-zero exit on pre-tool) or inject context into the next LLM call (stdout on exit 0). This enables the proactive-context pattern and any generic Claude Code-compatible hook command.

What's included

New project_hooks module (crates/tenex-agent/src/project_hooks.rs)

  • ProjectHooksConfig::loadNotFound → empty config, malformed JSON → hard error (mirrors the tenex-mcp project-config pattern). A broken hook file must not silently disable the gating an operator configured.
  • ProjectHooksRunner — spawns hooks with the workspace as cwd, writes the JSON stdin payload, enforces a 30s per-hook timeout (pre-tool timeout → continue; post-tool timeout → ignore), and forwards hook stderr to the agent's stderr.
  • Stdin protocol: {"event":"pre-tool","tool":"…","args":{…}} / {"event":"post-tool",…,"result":"…"}. args is embedded as parsed JSON.
  • Pre-tool semantics: exit 0 + empty stdout → continue; exit 0 + stdout → continue with injection; non-zero → block with the hook's stderr (or a generic fallback).

EmitHook integration (hook.rs)

  • Optional ProjectHooksRunner plus an injection buffer. on_tool_call fires pre-tool hooks before the supervisor check (blocking via ToolCallHookAction::skip); on_tool_result fires post-tool hooks; drain_hook_injections hands collected stdout to the turn loop.

Turn-loop wiring (turn_loop/step/tools.rs)

  • Drains injections after on_tool_result and folds them into the persisted ToolCallRecord for that call, so the model reads the injected context on its next turn. (Draining in recording.rs would have mis-ordered post-tool injections, since the record is taken before on_tool_result fires.)

Bootstrap wiringstages::load_project_hooks loads the workspace config; assembly::init_supervisor_and_hook constructs the runner and threads it into EmitHook::new.

E2E probe scenario hooks-pre-tool — seeds a workspace .tenex-hooks.json whose hook blocks the shell tool with hook-blocked on stderr. The verdict confirms the block reason reaches the model as the tool result and the command never executed. (A blocked tool is skipped before any tool-use event is published, so the verdict keys on the request record, not a nostr tool event.)

Tests

  • 12 unit tests in project_hooks (config loading, block/inject semantics, stdin protocol, stderr block reason, fallback).
  • 4 unit tests for the turn-loop injection folding (append_to_record_result).
  • cargo test -p tenex-agent — all pass.

The probe harness cannot run in this environment (missing nostr-tools + local relay/runtime), so the scenario is validated by bun build of the full probe graph and pattern-matched against existing scenarios (file-modification-tracking, todo-stop).

Closes #114

🤖 Generated with Claude Code

Adds project-local shell hooks that fire at pre-tool and post-tool
boundaries, configured via .tenex-hooks.json in the agent workspace.
Hooks receive JSON context on stdin and can block a tool call
(non-zero exit on pre-tool) or inject context into the next LLM call
(stdout on exit 0). This enables the proactive-context pattern and any
generic Claude Code-compatible hook command.

- New `project_hooks` module owns config loading (NotFound -> default,
  malformed -> hard error, matching the tenex-mcp pattern), hook
  spawning, the stdin protocol, and a 30s per-hook timeout.
- `EmitHook` gains an optional `ProjectHooksRunner` and an injection
  buffer: `on_tool_call` fires pre-tool hooks (blocking before the
  supervisor check), `on_tool_result` fires post-tool hooks, and
  `drain_hook_injections` hands the collected stdout to the turn loop.
- The turn loop drains injections after `on_tool_result` and folds them
  into the persisted tool record so the model reads them on its next
  turn. Pre-tool block reasons surface from the hook's stderr.
- E2E probe scenario `hooks-pre-tool` exercises a workspace hook that
  blocks the shell tool; the verdict confirms the block reason reaches
  the model and the command never executed.

Closes #114

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@claude

claude Bot commented May 30, 2026

Copy link
Copy Markdown

Code Review

Overview

This PR implements a Claude Code-compatible project hooks system: shell commands fire at pre-tool and post-tool boundaries, configured via .tenex-hooks.json. The design is clean — strict config parsing, a dedicated ProjectHooksRunner, and clear separation between the runner and the EmitHook wiring layer.


Bug: Orphaned child processes on timeout

run_hook has no process cleanup on timeout. When tokio::time::timeout fires, the Child value is dropped but the OS process keeps running:

Err(_) => HookRun::TimedOut,  // child is leaked

Over a long session with slow hooks this accumulates orphans. Fix with tokio::select! so the child can be killed:

tokio::select! {
    result = child.wait_with_output() => {
        match result {
            Ok(output) => { /* existing handling */ }
            Err(e) => HookRun::SpawnFailed(format!("awaiting hook: {e}")),
        }
    }
    _ = tokio::time::sleep(HOOK_TIMEOUT) => {
        let _ = child.kill().await;
        HookRun::TimedOut
    }
}

Security concern: spawn failures silently bypass pre-tool gates

The config parser is deliberately strict ("malformed config is fatal"), but a spawn failure at runtime is treated as non-blocking:

HookRun::SpawnFailed(e) => {
    eprintln!("[tenex-agent] warn: pre-tool hook '{}' failed to run: {e}; continuing", ...);
    // falls through to Continue
}

If an operator configures a security-blocking hook (e.g. ["./guard.sh"]) and that binary is absent or not executable, the gate silently disappears. The timeout-as-continue behaviour is defensible (a flaky hook shouldn't stall the agent), but a spawn failure is a misconfiguration, not flakiness. Pre-tool spawn failures should probably block or at least be configurable.


Code style: records_before index vs last_mut()

In tools.rs:

let records_before = records.len();
// ... one record is pushed somewhere below ...
if let Some(record) = records.get_mut(records_before) {

This assumes exactly one record is pushed per loop iteration. records.last_mut() makes the intent explicit and is less fragile to restructuring.


CLAUDE.md: multi-line comment blocks

The project standard says "Never write multi-paragraph docstrings or multi-line comment blocks — one short line max." Several additions violate this:

  • The module-level doc comment in project_hooks.rs (8 lines)
  • drain_hook_injections doc comment (5 lines)
  • Block comments in hook.rs lines 303–306 and tools.rs lines 738–741

The ordering rationale in tools.rs is the most defensible ("non-obvious invariant"), but the others explain what the code does, which well-named types already convey.


Minor: post-tool stderr silently discarded in the non-zero branch

In fire_post_tool:

HookRun::Completed { exit_ok, stdout, stderr: _ } => {
    if exit_ok { ... } else {
        eprintln!("... post-tool hook '{}' exited non-zero ... failure is ignored", hook.name);
    }
}

run_hook already forwards stderr to the agent's stderr before returning, so stderr is visible — but the warning message doesn't mention it. The log line would be more actionable if it said "see above stderr output for details" or similar. Low priority but ops-facing.


What's done well

  • The two-tier severity model (pre-tool blocks; post-tool non-zero is ignored) is exactly right and clearly documented.
  • ProjectHooksConfig::load has the correct NotFound → empty config, malformed → hard error semantics.
  • pre_tool_stdin embedding args as parsed JSON (not a re-serialized string) is a nice touch for hook author ergonomics.
  • Unit tests cover all the key paths: block, inject, fallback, silent-block, stdin protocol.
  • The records_before / drain-after-on_tool_result ordering is correct and the PR description explains why clearly.

Summary

Must fix Orphaned child on timeout
Should fix Spawn failure silently bypasses pre-tool gate
Style last_mut() over records_before; multi-line comments
Low priority Post-tool stderr mention in warning message

@pablof7z pablof7z merged commit 89cee26 into master May 30, 2026
1 check passed
pablof7z added a commit that referenced this pull request May 30, 2026
Addresses review feedback on #117 (project hooks system).

- Kill the timed-out hook child instead of detaching it: set
  kill_on_drop(true) so dropping the wait future on timeout sends
  SIGKILL, removing the orphaned-process bug in run_hook.
- Pre-tool spawn failures now block the tool call (a missing or
  non-executable hook binary is a misconfiguration, not flakiness).
  Post-tool spawn failures stay warnings since the tool already ran.
- Use records.last_mut() instead of a records_before index when
  folding hook injections into the tool record.
- Collapse multi-line doc/comment blocks to single WHY lines per
  the project comment policy.

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
pablof7z added a commit that referenced this pull request May 30, 2026
…ios pass (#120)

Four fixes to the e2e probe scenarios added in #116 and #117:

- file-modification-tracking: pm agent must be 'generalist' (not orchestrator)
  so workspace fs tools are available; orchestrators are workspace-restricted.
- file-modification-tracking: cassette reordered so the more-specific second-run
  entry (with fileModificationSecondRequest in history) is checked before the
  first-run entry, preventing false matches on the shared conversation history.
- file-modification-tracking: scenario driver switches from waitForObservedEvent
  (relay push, blocked by relay ACL deferral) to waitForStoredMessage (DB poll).
- hooks-pre-tool: same relay ACL fix — final completion now detected via DB.
- verdicts: requestDebug uses Rust {:?} escaping; quote checks updated to match
  the escaped form (type=\"file-modifications\").

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: Add Claude Code-compatible hooks system (support proactive-context and generic hook commands)

1 participant