feat: daemon-native hook bus — uniform tool/turn/lifecycle hooks for every backend - #137
Conversation
…every backend
pi's extension hooks only help pi sessions because they run inside the pi
process. This adds codeoid's OWN hook layer at the daemon, keyed by the
provider-neutral events Session already sees, so one config-declared rule
set (block/mutate tool calls, patch recorded output, inject prompt
context, audit lifecycle) applies uniformly across claude, pi, gemini,
and openai sessions.
- src/daemon/hooks/{types,bus}.ts: HookBus built once at startup (like
ProviderRegistry), shared by every session. v1 hook kinds: `command`
(shell, event JSON on stdin, Claude Code-style contract: exit 2 or
{"decision":"block"} blocks, stdout JSON can mutate) and `webhook`
(POST, same outcome shape). No in-process JS plugin kind by design.
- Events: tool_call (block/mutate, runs BEFORE the approval gate — a
policy deny never prompts and never burns the autonomous budget),
tool_result (patch the RECORDED output — canonical history, scrollback,
transcript), before_turn (systemPromptAppend), after_turn +
session_start/session_end/provider_switched/rotated (observe-only,
fire-and-forget).
- Security: hook commands get the hardened subprocess env
(buildSubprocessEnv shared basics — CODEOID_/ZEROID_/TELEGRAM_ never
leak), payload on stdin, output capped at 1 MiB. Infra failures fail
open; blocking is always an explicit hook decision.
- Config: `hooks.entries[]` (event, tool-name matcher regex, type,
command/url, timeoutMs) validated at load; CODEOID_HOOKS_ENABLED kill
switch.
- Session: shared #resolveToolCallMessage finalizer so the hook-block
path and the manual deny path can't drift; hook input mutations update
the displayed tool message so the user approves what will actually run.
- Wire-neutral: no protocol changes; hook blocks/mutations surface as
standard info messages.
Tests: bus unit (block/mutate/fail-open/timeout/matcher/env-hardening/
webhook), Session integration over MockSessionProvider (block before
approval + budget untouched, safe-tool block, mutation reaches provider,
before_turn append, tool_result redaction, lifecycle emits), config
schema fidelity + env kill switch.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #137 +/- ##
==========================================
+ Coverage 81.57% 82.03% +0.45%
==========================================
Files 95 97 +2
Lines 16406 16834 +428
==========================================
+ Hits 13384 13810 +426
- Misses 3022 3024 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
/gemini-review |
🤖 Gemini code reviewThis PR introduces a uniform, daemon-native hook bus that enables config-declared hooks (shell commands and webhooks) to run across all backend providers. It features a hardened subprocess environment to prevent credential leakage and supports blocking and input mutation prior to the user approval gate. The implementation is robust, but there are potential improvements regarding cross-platform support and memory safety under massive webhook payloads. Findings: 🔴 0 · 🟠 0 · 🟡 1 · 🟢 1 Tokens spent · ⬆️ Input: 25,197 · ⬇️ Output: 577 · Σ Total: 32,450 |
Gemini review: res.text() buffered an arbitrarily large webhook body in full before the slice, so MAX_OUTPUT_BYTES bounded parsing but not memory. Read the body via the stream reader, stop at the cap, and cancel the rest. Test: a 4 MiB body fails open without blocking.
…view (#139) Every issue_comment on a PR enters the per-PR concurrency group before the job-level if filters it out, so unconditional cancel-in-progress let ANY comment (observed: codecov's coverage comment on #137) kill a running review, with the superseding run then skipping itself. Make cancellation conditional: only a pull_request trigger or an explicit /gemini-review command supersedes; other comments queue and skip.
* fix: close post-merge test gaps in hooks + Phase-2 history Post-merge audit of #137/#138 found four untested behaviors; all now covered, no product code changes: - canonical-sdk-compat.test.ts: toGeminiContent output validated by the REAL @google/generative-ai validateChatHistory (startChat constructor, offline) with a negative control proving the validator fires — pins the role:"function" decision to the SDK, not to a code comment. toOpenAIMessages output assigned to ChatCompletionMessageParam[] WITHOUT a cast so typecheck enforces shape compatibility (the provider call site casts, which would hide drift). - session-hooks H8: hook input mutation composed with MANUAL approval — the approval UI delta shows the hook-rewritten input and the approved merge base is the mutated input, not the model's original. - session-hooks H9/H10/H11: provider_switched, rotated, and session_end observe emits (previously only session_start/after_turn were covered). Also hardens the observe-hook file waits: wait for parseable JSON, not file existence — 'cat > file' creates the file before the payload lands. * fix: run the real-SDK Gemini validation in a subprocess fixture provider-gemini.test.ts installs a process-global mock.module("@google/generative-ai"), so whether an in-process import sees the real SDK depends on test-file execution order — passed locally, failed in CI (startChat returned the mock's {sendMessageStream} stub and the negative control stopped throwing). Move the validation into a spawned fixture (fake-pi pattern): a fresh process is mock-proof and order-independent.
What
pi's extension hooks (tool_call block/mutate, lifecycle) are the best idea in the pi harness — and they only work for pi sessions, because they run inside the pi process. This PR adds codeoid's own hook layer at the daemon, dispatched on the provider-neutral events Session already sees, so one config-declared rule set applies uniformly whether a session runs on claude, pi, gemini, or openai.
Rules like "block writes to
.env", "rewrite dangerous Bash before the approval prompt", "redact secrets from recorded output", "git-checkpoint per turn", "audit to a webhook" — written once, applied to every backend.Full docs:
docs/hooks.md.Design
HookBus(src/daemon/hooks/bus.ts) — built once at daemon startup (likeProviderRegistry/CompressionRegistry), threaded to every session viaSessionCreateOptions. Sessions without matching hooks pay zero latency (hasHooksgate on every hot path).command(shell via/bin/sh -c, event JSON on stdin, mirrors Claude Code's hook contract: exit 2 or{"decision":"block"}blocks; stdout JSON can mutate) andwebhook(POST, same outcome shape). Deliberately no in-process JS plugin loader — much bigger security surface, left to a future kind.tool_call— inside#makeCanUseToolFn, before the approval gate. A hook block is a policy deny: never prompts, never burns the autonomous budget, wins even for auto-approved safe tools. Mutations feed the sameupdatedInputpath the approval sanitizer uses, and the displayed tool message is updated so the user approves what will actually run.tool_result— patches the recorded output (canonical history → what a switched-to backend sees, scrollback, transcript). The native backend already consumed the original inside its own loop; docs state this honestly (redaction use case).before_turn— contributes asystemPromptAppend, composed after the stable base so the cached prompt prefix is untouched when no hook fires.after_turn,session_start/session_end/provider_switched/rotated— observe-only, fire-and-forget.buildSubprocessEnv, shared basics only;CODEOID_*/ZEROID_*/TELEGRAM_*never leak;CODEOID_AGENT_ENV_ALLOWescape hatch applies). Payload on stdin, never env. Output capped at 1 MiB. Infra failures fail open (a crashed hook script can't brick every session); blocking is always an explicit hook decision.hooks.entries[]with event, tool-namematcherregex (validated at load — a typo fails loudly), type, command/url, per-hook timeout.CODEOID_HOOKS_ENABLED=falsekill switch.hook.blocked/hook.updated_inputmetadata), so no client work is required. A read-onlyhooks.configsnapshot verb (parallel toclaude.config) is noted as a possible follow-up.#resolveToolCallMessageso the hook-block path and the manual deny path share one finalizer and can't drift.Tests
hook-bus.test.ts— command kind (block via JSON + via exit 2, mutation chaining, short-circuit, fail-open on crash/garbled JSON/timeout, matcher gating, invalid-matcher skip, env hardening with a root-key-shaped secret, stdin payload), webhook kind (block/mutate/non-2xx/unreachable via localBun.serve),createHookBusgating.session-hooks.test.ts— end-to-end overMockSessionProvider: block before the approval gate with budget untouched, block wins on safe tools, mutation reaches the provider,before_turnappend inTurnOpts,tool_resultredaction in the completed message, lifecycle observe hooks, and a no-bus control asserting unchanged behavior.config.test.ts— schema defaults, entry parsing, kill switch, rejection of malformed entries (missing command/url, bad regex, unknown event, out-of-range timeout).bun run test(1219 pass),typecheck,lintall green.🤖 Generated with Claude Code