Skip to content

feat(hooks): external subprocess hook protocol with prompt mutation#102

Merged
Fullstop000 merged 11 commits into
masterfrom
worktree-feat-prompt-translate-hook
Jun 2, 2026
Merged

feat(hooks): external subprocess hook protocol with prompt mutation#102
Fullstop000 merged 11 commits into
masterfrom
worktree-feat-prompt-translate-hook

Conversation

@Fullstop000
Copy link
Copy Markdown
Owner

Summary

  • New external-subprocess hook protocol (CC-shaped JSON envelope) closing the gap where CC's UserPromptSubmit can only inject context but not rewrite — ignis hooks return updatedInput and the prompt is rewritten before history.push.
  • Two events ship in v1: UserPromptSubmit and AssistantMessageRender, both mutate via hookSpecificOutput.
  • /hooks reload re-reads ~/.ignis/hooks.json without restart.
  • Reference translator at examples/hooks/translate-en/ — stdlib Python, ~130 LOC, drop-in.

Architecture (one paragraph)

Each event spawns the configured command with a JSON envelope on stdin (hook_event_name, session_id, cwd, plus event-specific fields) and reads a JSON response on stdout. Hooks never kill a turn: timeout / non-zero exit / malformed JSON / missing binary all degrade to "use the original value + emit [warn] {event}: {reason}". Session::prompt runs the user-prompt chain before history.push and emits a new UserPromptCommitted event so the console renders the post-hook text (without this, scrollback would diverge from history). The assistant-side rewrite renders as a follow-up [hook rewrite] block below the original, documented inline.

Security model — v1 ships UNSANDBOXED (deliberate)

Hooks run with ignis's full privileges. Installing a hook is equivalent to running an arbitrary script with your user account — see docs/usage/hooks.md for the threat model. v2 (immediate follow-up PR) adds:

  • Env-var allowlist (env: [...] per hook + universal allowlist) — stops credential exfil on all platforms.
  • Linux Landlock filesystem sandbox via Fullstop000/mini-sandbox.

Why v1 ships without: the user explicitly chose protocol-only for the first PR to keep it shippable, with the security PR tracked as an immediate follow-up.

Spec

Approved design lives at docs/superpowers/specs/2026-06-02-hook-protocol-design.md (local-only per .git/info/exclude).

Test plan

  • cargo fmt --all -- --check
  • cargo clippy --workspace --all-targets -- -D warnings — 0 warnings
  • cargo test --workspace — 515 passed / 0 failed / 1 ignored (network test). 22 new unit tests in hooks/, 3 integration roundtrips in tests/hook_roundtrip.rs.
  • cargo build --release
  • Dogfooded end-to-end on all four user paths: input-rewrite, display-rewrite ([hook rewrite] follow-up block), /hooks reload, failing hook ([warn] line). Screenshots in a follow-up comment.

What's deferred

  • v2 PR: env-allowlist + Linux Landlock sandbox.
  • Out of v1+v2: marketplace install CLI, additional event types (SessionStart, PreCompact, …), persistent stdio per hook, network restrictions, Windows AppContainer.

Fullstop000 added a commit that referenced this pull request Jun 2, 2026
Adds a CC-shaped subprocess hook protocol that closes the gap CC stopped
short of: `UserPromptSubmit` and `AssistantMessageRender` events both
mutate via `hookSpecificOutput.updatedInput`/`updatedOutput`. Each event
spawns the configured command with a JSON envelope on stdin and reads a
JSON response on stdout; hooks never kill a turn (timeout/crash/malformed
JSON degrade to the original value plus a `[warn]` line).

- New `ignis/src/hooks/` module: protocol, dispatch, config, registry.
- `Session::prompt` runs the user-prompt chain before history.push and
  emits the new `UserPromptCommitted` event so the console renders the
  post-hook text (fixes a divergence between scrollback and history).
- Console renders assistant-side rewrites as a follow-up `[hook rewrite]`
  block; documented in `runner.rs::maybe_spawn_render_hook`.
- `/hooks reload` re-reads `~/.ignis/hooks.json` without a restart.
- Reference translator at `examples/hooks/translate-en/` (stdlib Python,
  no third-party deps; routes input vs display from `hook_event_name`).
- `docs/usage/hooks.md` opens with the v1 unsandboxed warning.

Security: hooks run with ignis's full privileges in v1. Env-allowlist
plus a Linux Landlock sandbox land in an immediate v2 PR.

Test coverage: 22 unit tests in `hooks/`, 3 integration roundtrips in
`tests/hook_roundtrip.rs`. Dogfooded end-to-end on all four user paths.
@Fullstop000 Fullstop000 force-pushed the worktree-feat-prompt-translate-hook branch from 6c06960 to 4138537 Compare June 2, 2026 12:24
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jun 2, 2026

Codecov Report

❌ Patch coverage is 90.57117% with 104 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.36%. Comparing base (809949d) to head (82b3e4a).

Files with missing lines Patch % Lines
ignis/src/hooks/dispatch.rs 88.61% 28 Missing ⚠️
ignis/src/console/keys.rs 0.00% 22 Missing ⚠️
ignis/src/console/runner.rs 90.81% 18 Missing ⚠️
ignis/src/console/app.rs 11.76% 15 Missing ⚠️
ignis/src/hooks/mod.rs 97.14% 9 Missing ⚠️
ignis/src/session/mod.rs 33.33% 6 Missing ⚠️
ignis/src/agent/mod.rs 92.30% 3 Missing ⚠️
ignis/src/hooks/config.rs 99.01% 2 Missing ⚠️
ignis/src/main.rs 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #102      +/-   ##
==========================================
+ Coverage   72.14%   73.36%   +1.21%     
==========================================
  Files          61       65       +4     
  Lines       15236    16322    +1086     
==========================================
+ Hits        10992    11974     +982     
- Misses       4244     4348     +104     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

trim_one_line / trim_stderr sliced &s[..200] without checking UTF-8
boundaries — any CJK / emoji stderr longer than 200 bytes would panic
the truncation, which is exactly the soft-failure path that must never
panic. Add a char-counting helper and route both call sites through it.
Test with a 250-char CJK string.
…lock

Previously, dispatch.rs wrote the JSON envelope to the hook's stdin and
only then started tokio::time::timeout around wait_with_output(). A hook
that never reads stdin would block the write forever once the ~64 KiB
pipe buffer filled — and the timeout would never arm. The agent loop
hung indefinitely on a misbehaving hook.

Wrap the entire interaction (stdin write + drop + wait_with_output) in
ONE timeout. On expiry, dropping the interaction future drops the child,
which fires kill_on_drop's SIGKILL. Add a regression test using a hook
that sleeps 30 s without reading stdin + a 256 KiB payload to defeat any
pipe-buffer slack — asserts the timeout fires promptly.
…ace)

docs/usage/hooks.md and dispatch.rs's module docstring previously
claimed SIGTERM-then-SIGKILL-after-1s. The code uses kill_on_drop which
sends SIGKILL immediately when the future drops. Documenting reality.
The single-string command form whitespace-splits, so a hook command like
"/Users/foo bar/run.py" silently becomes program "/Users/foo" with
"bar/run.py" as arg 0. There is no in-band quoting in JSON to escape
that ambiguity.

Add an alternate argv form: `{ "argv": ["prog", "arg1"], ... }`.
argv[0] is the program (with ~/ expansion); the rest are arguments
verbatim. command and argv are mutually exclusive; missing both or both
present is a startup error.

Document both forms in docs/usage/hooks.md.
Previously, run_user_prompt_submit returned the last-good prompt on
exit 2 / continue:false and Session::prompt blindly pushed it into
history and called the model. A DLP-style hook that returned exit 2
would leak the original prompt anyway — defeating the only event where
blocking is meaningful per the spec.

Change run_user_prompt_submit to return PromptHookResult::{Continue,
Blocked}. Session::prompt on Blocked: warning is already emitted on the
event channel; do not push history; do not call agent.run; return Ok(()).
The render chain (run_assistant_message_render) keeps the original
String return — for it, blocking is degraded to a Warning because the
assistant message already exists.

Add a session-level integration test that asserts on block: the
provider is never called, history stays empty, the Warning event
carries the hook stderr, and no UserPromptCommitted is emitted.
Session::prompt ran the initial prompt through the UserPromptSubmit
chain, but drain_injected pushed steer messages straight to history.
For the bilingual-translation use case, every steer message reached
the model untranslated — the hook saw the first prompt only.

Plumb HookRegistry onto Agent via set_prompt_hooks(registry, session_id,
cwd), called from Session::open and Session::set_hook_registry so the
console runner's /hooks reload swap propagates. drain_injected now runs
each inject's text through run_user_prompt_submit before pushing; a
Blocked outcome drops the inject entirely (warning already emitted),
matching Session::prompt's short-circuit posture.

Add a hook_roundtrip integration test: preload a steer message on the
inject channel + install an uppercasing hook, then assert both the
initial 'hi there' and the inject 'steer me' land in history as
'HI THERE' and 'STEER ME'.
Two issues with the previous `maybe_spawn_render_hook`:

1. Fired on EVERY MessageEnd, including reasoning-only (✻ Thinking)
   blocks. The translator hook ran over the model's internal monologue
   — wrong and wasteful, since the user never sees those as the
   assistant's reply.
2. Each MessageEnd spawned an independent tokio::spawn, so two
   back-to-back messages with different hook latencies could commit
   their rewrites out-of-order — and the spawned task could outlive
   the AgentEnd that closed the conversation.

Replace with a per-session worker task owning a single
mpsc::Receiver<RenderJob>. The runner enqueues jobs as they arrive;
the worker drains serially so submission order is preserved. The
`MessageEnd` filter now also rejects reasoning-only messages
(reasoning_content set + content empty).

Add two tests: reasoning-only MessageEnd does not enqueue a job; two
jobs where the first hook is slower than the second still commit in
submission order.

Also hardens util::unique_temp_dir with a process-wide atomic counter
so the new tests don't collide with existing ones on same-nanosecond
buckets under cargo's thread pool.
- Format the new code paths (mod.rs, config.rs, runner.rs, tests).
- Replace manual char-counter loop in truncate_chars with
  `iter.by_ref().take(n).collect()` (clippy::explicit_counter_loop).
- Replace `std::iter::repeat(c).take(n).collect()` with `c.repeat(n)`
  in the CJK truncation test (clippy::manual_str_repeat).

All gates pass:
  cargo fmt --all --check        PASS
  cargo clippy -D warnings       PASS  (0 warnings)
  cargo test --workspace         PASS  (560+ tests)
  cargo build --release          PASS
…okContext

Agent stored prompt-hook context as Option<(String, String)>, requiring
readers to remember "index 0 is session_id, index 1 is cwd". drain_injected
had the matching (&str, &str) param shape. Introduce OwnedHookContext as
the storage shape (alongside the existing borrowed HookContext used at
dispatch) with as_ref() to borrow back. Pure rename; no semantic change.
Agent stored its own Option<HookRegistry> + Option<OwnedHookContext> so
drain_injected could reach them inside the run loop. But Session already
owns the canonical registry, so each Session::open cloned the Arc into
two places and /hooks reload had to propagate to both.

Push the registry + context through `Agent::run` parameters instead:
single source of truth (Session::hooks), set_hook_registry becomes a
single-field write, and Agent's surface area shrinks by one field +
one setter. The subagent tool keeps its zero-hooks behavior by passing
None, None. Drops OwnedHookContext since nothing stores hook context
across calls any longer.
@Fullstop000 Fullstop000 merged commit d587339 into master Jun 2, 2026
4 checks passed
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.

2 participants