From 8adcbceeedeb712352f076608e60ab4735cd6cf4 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Wed, 1 Jul 2026 04:22:00 +0800 Subject: [PATCH 01/10] feat(hooks): dsh-hooks-claude + dsh-hooks-codex bridges (hooks stack PR-F) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two bridge plugins that run a user's existing Claude Code / Codex hook config on the harness's typed interception seams, built on the shared dsh-hook-protocol library. A bridge is a faithfulness adapter, not a power tool: anything it does a native cordis plugin does more powerfully — the bridge exists only to run UNMODIFIED external hooks. - dsh-hooks-claude: CC dialect. Seven hook points (SessionStart, UserPromptSubmit, PreToolUse, PostToolUse, Stop, SubagentStart, SubagentStop), CC per-event stdin payloads, env + ${CLAUDE_PLUGIN_ROOT}/ ${CLAUDE_PROJECT_DIR} substitution, literal-or-regex matcher. - dsh-hooks-codex: Codex dialect — a deliberate subset. Five hook points, always-regex matcher, snake_case payloads (turn_id/model, no trailing newline), no env/substitution, block-only decisions. Both map the neutral merged outcome onto the seam's typed Decision and stamp an explicit {kind:'plugin'} source on injected context (so it is never mislabeled as a user prompt). Config parse-failure is contained; only command hooks run. updatedInput is logged+warned (input rewrite deferred); the Stop loop-guard is deferred (TODO). Tests: per-file 100% — config-parse unit branches + per-seam mappings end-to-end through the REAL loop + REAL bash + REAL shell scripts (scripted mock model only) + a real-Loader export-shape guard. A keyless ACP snapshot scenario (hook-prompt-block) proves a UserPromptSubmit hook blocks a prompt end-to-end (rejected turn -> ACP cancelled, hook/* events in the log); a with-key e2e (hooks.e2e.ts) proves a PreToolUse hook blocks real bash (verified on disk). The snapshot normalizer now scrubs hook/result.durationMs. RFC: docs/rfc/implemented/feature/2026-06-30-hook-bridges.md --- AGENTS.md | 6 + docs/module-graph.md | 13 + docs/rfc/README.md | 1 + .../feature/2026-06-30-hook-bridges.md | 51 +++ examples/AGENTS.md | 2 +- examples/acp-agent/cordis.snapshot.yml | 10 + examples/acp-agent/cordis.yml | 10 + examples/acp-agent/tests/acp.snapshot.ts | 24 +- examples/acp-agent/tests/hooks.e2e.ts | 119 +++++ .../tests/snapshot-normalize.spec.ts | 17 + .../acp-agent/tests/snapshot-normalize.ts | 10 +- .../snapshots/hook-prompt-block/input.json | 7 + .../snapshots/hook-prompt-block/session.jsonl | 5 + .../hook-prompt-block/stdout.golden.jsonl | 3 + .../hook-prompt-block/workspace/hooks.json | 11 + packages/README.md | 2 + packages/hooks/hooks-claude/README.md | 51 +++ packages/hooks/hooks-claude/package.json | 49 +++ packages/hooks/hooks-claude/src/config.ts | 100 +++++ packages/hooks/hooks-claude/src/index.ts | 300 +++++++++++++ .../hooks/hooks-claude/tests/bridge.spec.ts | 335 +++++++++++++++ .../hooks/hooks-claude/tests/config.spec.ts | 66 +++ .../hooks/hooks-claude/tests/coverage.spec.ts | 405 ++++++++++++++++++ packages/hooks/hooks-claude/tsconfig.json | 42 ++ packages/hooks/hooks-codex/README.md | 54 +++ packages/hooks/hooks-codex/package.json | 47 ++ packages/hooks/hooks-codex/src/config.ts | 79 ++++ packages/hooks/hooks-codex/src/index.ts | 235 ++++++++++ .../hooks/hooks-codex/tests/bridge.spec.ts | 167 ++++++++ .../hooks/hooks-codex/tests/config.spec.ts | 68 +++ .../hooks/hooks-codex/tests/coverage.spec.ts | 308 +++++++++++++ packages/hooks/hooks-codex/tsconfig.json | 39 ++ pnpm-lock.yaml | 77 ++++ tsconfig.build.json | 4 +- tsconfig.json | 4 +- 35 files changed, 2714 insertions(+), 7 deletions(-) create mode 100644 docs/rfc/implemented/feature/2026-06-30-hook-bridges.md create mode 100644 examples/acp-agent/tests/hooks.e2e.ts create mode 100644 examples/acp-agent/tests/snapshots/hook-prompt-block/input.json create mode 100644 examples/acp-agent/tests/snapshots/hook-prompt-block/session.jsonl create mode 100644 examples/acp-agent/tests/snapshots/hook-prompt-block/stdout.golden.jsonl create mode 100644 examples/acp-agent/tests/snapshots/hook-prompt-block/workspace/hooks.json create mode 100644 packages/hooks/hooks-claude/README.md create mode 100644 packages/hooks/hooks-claude/package.json create mode 100644 packages/hooks/hooks-claude/src/config.ts create mode 100644 packages/hooks/hooks-claude/src/index.ts create mode 100644 packages/hooks/hooks-claude/tests/bridge.spec.ts create mode 100644 packages/hooks/hooks-claude/tests/config.spec.ts create mode 100644 packages/hooks/hooks-claude/tests/coverage.spec.ts create mode 100644 packages/hooks/hooks-claude/tsconfig.json create mode 100644 packages/hooks/hooks-codex/README.md create mode 100644 packages/hooks/hooks-codex/package.json create mode 100644 packages/hooks/hooks-codex/src/config.ts create mode 100644 packages/hooks/hooks-codex/src/index.ts create mode 100644 packages/hooks/hooks-codex/tests/bridge.spec.ts create mode 100644 packages/hooks/hooks-codex/tests/config.spec.ts create mode 100644 packages/hooks/hooks-codex/tests/coverage.spec.ts create mode 100644 packages/hooks/hooks-codex/tsconfig.json diff --git a/AGENTS.md b/AGENTS.md index 020447fe7a..6890111fe9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -81,6 +81,12 @@ packages/ Harness packages, grouped by role at packages///. hook-protocol/ shared Claude Code / Codex hook wire-protocol core (library, not a plugin): matcher primitive, exit-code/stdout codec, runHook (via ctx.bash), most-restrictive merge, hook/* events + hooks-claude/ bridge plugin: runs a Claude Code hooks.json / settings on the + interception seams (CC dialect — env + ${CLAUDE_PLUGIN_ROOT} + substitution, per-event stdin payloads, outcome→Decision map) + hooks-codex/ bridge plugin: runs a Codex hooks.json on the seams (Codex + dialect — a 5-event, regex-only, block-only, no-substitution + subset of the CC protocol) session-persistence/ persistence capability family session-persistence/ durable persistence seam + write coordinator session-persistence-jsonl/ JSONL-sidecar backend diff --git a/docs/module-graph.md b/docs/module-graph.md index 3f64c7731e..1ca1856032 100644 --- a/docs/module-graph.md +++ b/docs/module-graph.md @@ -49,6 +49,11 @@ graph TD agent-loop --> session-persistence agent-loop --> system-prompt agent-loop --> tools + hooks-codex --> agent + hooks-codex --> hook-protocol + hooks-codex --> llm + hooks-codex --> session + hooks-codex --> tools subagent --> agent subagent --> llm subagent --> tools @@ -67,6 +72,12 @@ graph TD agent-core --> system-prompt agent-core --> tool-bash agent-core --> tools + hooks-claude --> agent + hooks-claude --> hook-protocol + hooks-claude --> llm + hooks-claude --> session + hooks-claude --> subagent + hooks-claude --> tools subagent-acp --> agent subagent-acp --> llm subagent-acp --> subagent @@ -119,10 +130,12 @@ graph TD | `ui-stdio` | `agent`, `llm`, `session` | | `acp` | `agent`, `llm`, `session`, `session-persistence`, `tools` | | `agent-loop` | `agent`, `llm`, `session`, `session-persistence`, `system-prompt`, `tools` | +| `hooks-codex` | `agent`, `hook-protocol`, `llm`, `session`, `tools` | | `subagent` | `agent`, `llm`, `tools` | | `tool-bash` | `agent`, `bash`, `llm`, `tools` | | `tool-todo` | `agent`, `session`, `tools` | | `agent-core` | `agent`, `agent-loop`, `invariants`, `llm`, `session`, `system-prompt`, `tool-bash`, `tools` | +| `hooks-claude` | `agent`, `hook-protocol`, `llm`, `session`, `subagent`, `tools` | | `subagent-acp` | `agent`, `llm`, `subagent` | | `subagent-inprocess` | `agent`, `llm`, `session`, `subagent` | | `subagent-mock` | `agent`, `llm`, `subagent` | diff --git a/docs/rfc/README.md b/docs/rfc/README.md index fbeb6966e7..5824986ae8 100644 --- a/docs/rfc/README.md +++ b/docs/rfc/README.md @@ -90,6 +90,7 @@ Do NOT write one for a mechanical or local choice (a variable name, a one-file r | [Interception seams — the typed-Decision surface a hook programs against](implemented/feature/2026-06-30-interception-seams.md) | 2026-06-30 | | [Subagent lifecycle enrichment — agentType + lastAssistantMessage (observe-only)](implemented/feature/2026-06-30-subagent-observe-enrich.md) | 2026-06-30 | | [dsh-hook-protocol — the shared Claude Code / Codex hook wire-protocol core](implemented/feature/2026-06-30-hook-protocol-lib.md) | 2026-06-30 | +| [dsh-hooks-claude + dsh-hooks-codex — the Claude Code / Codex hook bridges](implemented/feature/2026-06-30-hook-bridges.md) | 2026-06-30 | ### Simplification diff --git a/docs/rfc/implemented/feature/2026-06-30-hook-bridges.md b/docs/rfc/implemented/feature/2026-06-30-hook-bridges.md new file mode 100644 index 0000000000..a3e01c17be --- /dev/null +++ b/docs/rfc/implemented/feature/2026-06-30-hook-bridges.md @@ -0,0 +1,51 @@ +# RFC: dsh-hooks-claude + dsh-hooks-codex — the Claude Code / Codex hook bridges + +Status: implemented (accepted 2026-06-30) + + + +## Context + +The harness's extension surface is its typed interception seams ([the interception-seams RFC](2026-06-30-interception-seams.md)): a "native hook" is just an ordinary cordis plugin subscribing to `agent/session-start`, `agent/prompt-submit`, `tools/pre-execute`, `tools/post-execute`, `agent/turn-continuation`, `subagent/start`, `subagent/end`. But users arrive with **existing** Claude Code (CC) and Codex hook configs — a `hooks.json` (or a settings file's `hooks` key) full of shell-command hooks — and want those to run unmodified. This RFC introduces the two **bridge plugins** that translate that external shell-hook protocol onto the typed seams, built on the shared wire-protocol library ([the hook-protocol-lib RFC](2026-06-30-hook-protocol-lib.md)). + +The framing that shapes the whole design: **a bridge is a faithfulness adapter, not a power tool.** Anything a bridge does (block a tool, inject context, force continuation, observe a subagent) a native cordis plugin does more powerfully — typed returns, full `ctx`, no serialization boundary. The bridge's only reason to exist is to run an UNMODIFIED external CC/Codex hook with byte-faithful semantics. That keeps each bridge thin: parse the config, pick a matcher mode, build the per-event payload, call `runHook` + `mergeHookOutputs` from the shared lib, map the neutral outcome onto a seam Decision. + +## Decision + +Two independent plugins in the `packages/hooks/` group, each a function/namespace plugin (`name`/`inject`/`Config`/`apply`, NO default export — see [postmortem 0001](../../../postmortem/0001-acp-default-export-drops-inject.md)) injecting only `bash`: + +- **`dsh-hooks-claude`** — the CC dialect. Seven hook points: `SessionStart`, `UserPromptSubmit`, `PreToolUse`, `PostToolUse`, `Stop`, `SubagentStart`, `SubagentStop`. Owns CC's per-event stdin payloads (a base of `session_id`/`cwd`/`hook_event_name` plus per-event fields), CC's env + `${CLAUDE_PLUGIN_ROOT}`/`${CLAUDE_PROJECT_DIR}` substitution, and the literal-or-regex matcher mode. A CC hook's stdin carries a **trailing newline**. +- **`dsh-hooks-codex`** — the Codex dialect: a deliberate SUBSET. Five hook points (`PreToolUse`, `PostToolUse`, `SessionStart`, `UserPromptSubmit`, `Stop` — no subagent/notification/compaction), an always-regex matcher, snake_case payloads with `turn_id`/`model`/`permission_mode` extras written WITHOUT a trailing newline, no env and no `${…}` substitution, and a block-only decision model (a Codex hook can never pre-approve, so `allow`/`ask` are not honored). Codex hardcodes a tool call's `tool_name` to `"Bash"` and `tool_input` to `{ command }`. + +### Outcome → Decision mapping + +Each bridge maps the neutral `MergedHookOutcome` from the shared lib onto the seam's typed Decision: + +| Seam | CC | Codex | +|---|---|---| +| `agent/session-start` (emit) | additionalContext → `agent.inject()` | plain-stdout output → additionalContext → `agent.inject()` | +| `agent/prompt-submit` | `deny`→`block`; context→`allow` | `block`→`block`; context→`allow` | +| `tools/pre-execute` | `deny`→`deny`; `ask`→`ask` | `block`→`deny` (no allow/ask) | +| `tools/post-execute` | `deny`→`block`+feedback; context→`accept` | same | +| `agent/turn-continuation` | blocking Stop → `continue` (reason = next-step steering) | same | +| `subagent/start` (emit) | additionalContext → inject into the live child | — (not a Codex event) | +| `subagent/end` (emit) | observe-only | — | + +### Context source is always the plugin (the mislabel guard) + +`agent.inject()` defaults a missing `MessageSource` to `{ kind: 'user' }` — which would record plugin-injected context as if the user had typed it. So every bridge `inject()` and every `HookContext` passes an explicit `{ kind: 'plugin', plugin: 'hooks-claude' | 'hooks-codex' }` source. A test asserts the resulting `context/message.source` is the plugin, never `user`. + +### Containment + +The config is parsed ONCE at load; a read/parse failure logs and registers nothing rather than crashing boot (a typo'd path must not take the agent down). Only `type: 'command'` hooks run — a `prompt`/`agent`/HTTP hook (CC) or an `async: true` / non-command hook (Codex) is parsed-and-skipped with a warning. The emit-listener paths (`session-start`, `subagent/start`) run detached, with their `inject` contained in a `.catch` that logs (a throwing inject must not break session boot or the loop). + +## Deferred (faithful-but-degraded) + +- **Tool-input rewrite.** A CC/Codex `updatedInput` is logged + warned, not honored — input rewrite is a deferred consistency-design problem ([the pre-tool-input-rewrite RFC](../../proposed/feature/2026-06-30-pre-tool-input-rewrite.md)), because the pre-execution args are read by `tool/call` audit + `assistant/message` history + ACP/tool-bash presentation, so an honest rewrite is a design unit, not a field. +- **Stop loop-guard** (`TODO(stop-loop-guard)`). CC/Codex break an infinite force-continue with `stop_hook_active` (true once a Stop hook fired this run) plus a max-consecutive cap; both are deferred. Today `stop_hook_active` is always `false`, so a Stop hook that unconditionally blocks would force-continue every step — a hook author must self-limit until the guard lands. +- **Permission `ask`** degrades to `deny` at the `tools/pre-execute` seam (`FIXME(permissions)` in the interception-seams RFC) — there is no interactive permission prompt yet. +- **Config discovery.** The path is explicit in `cordis.yml`; the full multi-layer CC/Codex precedence walk and the trust/hash model are not reimplemented (`TODO`). + +## Consequences + +The bridges are thin and readable standalone: the correctness-critical halves (matcher semantics, exit-code contract, merge precedence) live in the shared `dsh-hook-protocol`, so each bridge is just config-parse + payload-build + outcome-map. Each is covered at per-file 100% — config-parse branches as unit tests, and the seam mappings end-to-end through the REAL loop + REAL `dsh-bash-local` + REAL shell scripts from a temp `hooks.json` (a scripted mock MODEL is the only stand-in), plus a real-Loader export-shape guard so a stray default export can't silently drop `inject`. Because the seams already carry typed Decisions, a future native plugin needs none of this bridge machinery — it returns a Decision directly. diff --git a/examples/AGENTS.md b/examples/AGENTS.md index c104c2cd8e..903f59781c 100644 --- a/examples/AGENTS.md +++ b/examples/AGENTS.md @@ -21,6 +21,6 @@ A keyless smoke that spawns the example from a temp cwd must set `TSX_TSCONFIG_P |---|---|---| | `echo-agent` | `tests/echo.e2e.ts` — boots the real `cordis.yml`, drives the echo tool round-trip and the direct canned reply | **N/A — keyless by nature** (the `mock-echo` model has no real provider) | | `coding-agent` | `tests/keyless-smoke.e2e.ts` — boots the full real tree (dummy key, no prompt → no model call), asserts banner + clean exit | `tests/{full-loop,coding-task,resume,todo-write}.e2e.ts` — real model + real bash + real todo_write, world-verified | -| `acp-agent` | `pnpm run test:snapshot` — boots the real ACP subprocess and replays a recorded session keyless; `tests/acp.e2e.ts` also asserts stdout purity without a key | `tests/acp.e2e.ts` — real ACP prompt, verifies a file the agent wrote | +| `acp-agent` | `pnpm run test:snapshot` — boots the real ACP subprocess and replays a recorded session keyless (incl. `hook-prompt-block`, where a `UserPromptSubmit` hook blocks the prompt); `tests/acp.e2e.ts` also asserts stdout purity without a key | `tests/acp.e2e.ts` — real ACP prompt, verifies a file the agent wrote; `tests/hooks.e2e.ts` — a real `PreToolUse` hook blocks bash, verifies the file is NOT written | See [the root AGENTS.md](../AGENTS.md) for repo-wide conventions and [docs/architecture.md](../docs/architecture.md) for the design. diff --git a/examples/acp-agent/cordis.snapshot.yml b/examples/acp-agent/cordis.snapshot.yml index 90c8dd2daf..fc7d81298b 100644 --- a/examples/acp-agent/cordis.snapshot.yml +++ b/examples/acp-agent/cordis.snapshot.yml @@ -83,3 +83,13 @@ # replayed todo_write tool call resolves to a real tool during snapshot replay. - id: tool-todo name: '@deepseek-ai/dsh-tool-todo' + +# The Claude Code hook bridge, pointed at a `hooks.json` in the session cwd. A +# scenario that ships `workspace/hooks.json` (copied into the cwd before the run) +# exercises the hooks path end-to-end; every other scenario has no such file, so +# the bridge's parse fails-soft and it registers nothing (a silent no-op — the +# ACP app loads no logger exporter, so the warning never reaches stdout). +- id: hooks-claude + name: '@deepseek-ai/dsh-hooks-claude' + config: + configPath: ./hooks.json diff --git a/examples/acp-agent/cordis.yml b/examples/acp-agent/cordis.yml index 96071ab564..6e299e1c74 100644 --- a/examples/acp-agent/cordis.yml +++ b/examples/acp-agent/cordis.yml @@ -94,3 +94,13 @@ # session log (todo/write), surfaced to the ACP client as a `plan` update. - id: tool-todo name: '@deepseek-ai/dsh-tool-todo' + +# The Claude Code hook bridge, pointed at a `hooks.json` in the session cwd. With +# no such file present the parse fails-soft and the bridge registers nothing (a +# silent no-op); a session whose cwd holds a `hooks.json` runs those hooks on the +# interception seams. stdout is the ACP JSON-RPC channel — the bridge's warnings +# go through ctx.logger (no exporter here), never to stdout. +- id: hooks-claude + name: '@deepseek-ai/dsh-hooks-claude' + config: + configPath: ./hooks.json diff --git a/examples/acp-agent/tests/acp.snapshot.ts b/examples/acp-agent/tests/acp.snapshot.ts index 94f18e64d3..b59b454a51 100644 --- a/examples/acp-agent/tests/acp.snapshot.ts +++ b/examples/acp-agent/tests/acp.snapshot.ts @@ -29,12 +29,22 @@ interface Scenario { name: string /** Whether the scenario drives at least one model turn (so a JSONL golden applies). */ hasModelTurn: boolean + /** + * Whether the run persists a comparable session log to diff against the + * `session.jsonl` fixture. Defaults to {@link hasModelTurn} (a model turn + * always produces a log worth comparing). Set it independently for a scenario + * that produces a non-trivial log WITHOUT a model turn — e.g. a prompt blocked + * by a `UserPromptSubmit` hook, which opens a `rejected` turn carrying `hook/*` + * events but never calls the model. + */ + comparesLog?: boolean /** * Whether `test:snapshot:record` regenerates this scenario's `session.jsonl` * from the LIVE API. `recorded` scenarios are model-driven and reproducible; * `authored` scenarios (a hand-written `replay.override.json` sidecar drives * replay — e.g. a provider error or a cancel, which the live API can't be - * coaxed into deterministically) are NEVER re-recorded. + * coaxed into deterministically — or a deterministic hook scenario whose + * derived empty script needs no sidecar) are NEVER re-recorded. */ recorded: boolean /** @@ -61,6 +71,11 @@ const SCENARIOS: Scenario[] = [ { name: 'subagent-multi', hasModelTurn: true, recorded: true, childSessions: 2 }, { name: 'subagent-fork', hasModelTurn: true, recorded: true, childSessions: 1 }, { name: 'subagent-mixed', hasModelTurn: true, recorded: true, childSessions: 2 }, + // A UserPromptSubmit hook blocks the prompt before any step runs: no model + // call (keyless, authored — its derived script is empty so it needs no + // sidecar), but it persists a `rejected` turn carrying `hook/*` events, so its + // log IS compared. The hooks.json riding in workspace/ drives the bridge. + { name: 'hook-prompt-block', hasModelTurn: false, comparesLog: true, recorded: false }, ] /** The sibling child-fixture paths for a scenario (`session.1.jsonl` …). */ @@ -139,12 +154,15 @@ for (const scenario of SCENARIOS) { await expect(normalizeStdout(result.rawStdout, ctx)) .toMatchFileSnapshot(join(dir, 'stdout.golden.jsonl')) - if (scenario.hasModelTurn) { + // A model turn always produces a log worth comparing; a hook scenario can + // produce one without a model turn (a `rejected` turn carrying `hook/*`). + const comparesLog = scenario.comparesLog ?? scenario.hasModelTurn + if (comparesLog) { // The harvested logs (primary-first) must match their committed fixtures // 1:1. Each side passes through normalizeSessionLog, scrubbed against ITS // OWN volatile values — the live run's via `ctx`, the committed fixture's // via its own header (a committed file cannot share the live run's ids). - expect(result.sessionLogs.length, 'a model scenario must persist a session log').toBe(childSessions + 1) + expect(result.sessionLogs.length, 'this scenario must persist a session log').toBe(childSessions + 1) const fixtureFiles = ['session.jsonl', ...Array.from({ length: childSessions }, (_, i) => `session.${i + 1}.jsonl`)] for (let i = 0; i < fixtureFiles.length; i++) { const harvested = (result.sessionLogs[i] as HarvestedLog).content diff --git a/examples/acp-agent/tests/hooks.e2e.ts b/examples/acp-agent/tests/hooks.e2e.ts new file mode 100644 index 0000000000..19a80df03c --- /dev/null +++ b/examples/acp-agent/tests/hooks.e2e.ts @@ -0,0 +1,119 @@ +import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process' +import { Readable, Writable } from 'node:stream' +import { mkdtemp, rm, writeFile, access } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { fileURLToPath } from 'node:url' +import { afterEach, describe, expect, it } from 'vitest' +import { + ClientSideConnection, + ndJsonStream, + PROTOCOL_VERSION, + type Agent as AcpAgent, + type Client, + type RequestPermissionRequest, + type RequestPermissionResponse, + type SessionNotification, +} from '@agentclientprotocol/sdk' + +/** + * With-key e2e: the Claude Code hook bridge running against the REAL acp-agent + * subprocess and the REAL model. The example `cordis.yml` loads `dsh-hooks-claude` + * pointed at `./hooks.json` in the session cwd; this test writes a `hooks.json` + * with a PreToolUse hook that BLOCKS every bash command, then asks the live model + * to write a file — and verifies the WORLD (the file never appears on disk), + * proving the hook actually intercepted execution rather than the agent merely + * claiming it couldn't. Key-gated; owns and disposes its subprocess. + * + * A keyless companion lives in acp.e2e.ts (stdout purity + session/new); the + * full hook-fires-end-to-end transcript is the keyless `hook-prompt-block` + * snapshot scenario. This one closes the "green plumbing, broken product" gap: + * only a real model deciding to call bash exercises the PreToolUse seam live. + */ + +const binScript = fileURLToPath(new URL('../../../packages/ui/acp-agent/src/bin.ts', import.meta.url)) +const configPath = fileURLToPath(new URL('../cordis.yml', import.meta.url)) +const tsxLoader = fileURLToPath(import.meta.resolve('tsx')) +const repoTsconfig = fileURLToPath(new URL('../../../tsconfig.json', import.meta.url)) + +interface Spawned { + child: ChildProcessWithoutNullStreams + client: ClientSideConnection + updates: SessionNotification['update'][] + stderr: string[] +} + +function spawnAcpAgent(cwd: string): Spawned { + const child = spawn( + process.execPath, + ['--import', tsxLoader, binScript, configPath], + { cwd, env: { ...process.env, TSX_TSCONFIG_PATH: repoTsconfig }, stdio: ['pipe', 'pipe', 'pipe'] }, + ) + const stderr: string[] = [] + child.stderr.setEncoding('utf8') + child.stderr.on('data', (chunk: string) => stderr.push(chunk)) + + const updates: SessionNotification['update'][] = [] + const stream = ndJsonStream( + Writable.toWeb(child.stdin) as WritableStream, + Readable.toWeb(child.stdout) as ReadableStream, + ) + const makeClient = (_agent: AcpAgent): Client => ({ + sessionUpdate(params: SessionNotification): Promise { + updates.push(params.update) + return Promise.resolve() + }, + requestPermission(_params: RequestPermissionRequest): Promise { + return Promise.resolve({ outcome: { outcome: 'cancelled' } }) + }, + }) + const client = new ClientSideConnection(makeClient, stream) + return { child, client, updates, stderr } +} + +let spawned: Spawned | undefined +let workdir: string | undefined + +afterEach(async () => { + if (spawned) { + spawned.child.kill('SIGKILL') + spawned = undefined + } + if (workdir !== undefined) await rm(workdir, { recursive: true, force: true }) + workdir = undefined +}) + +describe.skipIf(!process.env.DEEPSEEK_API_KEY)('acp-agent e2e: a PreToolUse hook blocks bash (real model)', () => { + it('denies every bash command, so the requested file is never written (verified on disk)', async () => { + workdir = await mkdtemp(join(tmpdir(), 'acp-hooks-e2e-')) + // A PreToolUse hook that blocks EVERY tool (exit 2, no matcher = match-all). + // The session cwd is `workdir`, and the bridge resolves `./hooks.json` from + // the process cwd (the launch dir = workdir), so this is the config it loads. + await writeFile(join(workdir, 'hooks.json'), JSON.stringify({ + hooks: { PreToolUse: [{ hooks: [{ type: 'command', command: 'echo "bash blocked by policy" >&2; exit 2' }] }] }, + })) + + spawned = spawnAcpAgent(workdir) + const { client, updates } = spawned + + await client.initialize({ protocolVersion: PROTOCOL_VERSION, clientCapabilities: {} }) + const { sessionId } = await client.newSession({ cwd: workdir, mcpServers: [] }) + + const res = await client.prompt({ + sessionId, + prompt: [{ type: 'text', text: 'Use the bash tool to write the exact text HOOK_FAIL into a file named proof.txt in the current directory. Then stop.' }], + }) + // The turn completes normally (the block is a tool-result error fed back to + // the model, not a turn failure). + expect(['end_turn', 'max_tokens']).toContain(res.stopReason) + + // Verify the WORLD: the hook denied execution, so the file must NOT exist — + // a keyword probe a "cheating" agent could fake in prose cannot pass this. + await expect(access(join(workdir, 'proof.txt'))).rejects.toThrow() + + // The client still saw a tool_call stream (the model TRIED), and its result + // carried the hook's block reason back as an error. + const toolCalls = updates.filter(u => u.sessionUpdate === 'tool_call' || u.sessionUpdate === 'tool_call_update') + expect(toolCalls.length).toBeGreaterThan(0) + }, 180_000) +}) diff --git a/examples/acp-agent/tests/snapshot-normalize.spec.ts b/examples/acp-agent/tests/snapshot-normalize.spec.ts index b220344bb9..bfe29af8a5 100644 --- a/examples/acp-agent/tests/snapshot-normalize.spec.ts +++ b/examples/acp-agent/tests/snapshot-normalize.spec.ts @@ -90,4 +90,21 @@ describe('normalizeSessionLog', () => { const out = normalizeSessionLog(`${header({ id: ctx.sessionIds[0] })}\n`, ctx) expect(out).toContain('{{sessionId}}') }) + + it('zeroes a hook/result durationMs (run-to-run noise) but keeps its decision', () => { + const ev = JSON.stringify({ + type: 'hook/result', seq: 2, time: 5, + data: { turn: 1, point: 'UserPromptSubmit', handlerId: 'h', decision: 'block', exitCode: 2, durationMs: 37 }, + }) + const out = normalizeSessionLog(`${header({})}\n${ev}\n`, ctx) + expect(out).toContain('"durationMs":0') + expect(out).not.toContain('37') + expect(out).toContain('"decision":"block"') // the decision is the behavior — kept + }) + + it('leaves a non-hook event durationMs untouched (only hook/result is scrubbed)', () => { + const ev = JSON.stringify({ type: 'tool/result', seq: 2, time: 5, data: { durationMs: 88 } }) + const out = normalizeSessionLog(`${header({})}\n${ev}\n`, ctx) + expect(out).toContain('"durationMs":88') + }) }) diff --git a/examples/acp-agent/tests/snapshot-normalize.ts b/examples/acp-agent/tests/snapshot-normalize.ts index db0d493535..8150057fa4 100644 --- a/examples/acp-agent/tests/snapshot-normalize.ts +++ b/examples/acp-agent/tests/snapshot-normalize.ts @@ -8,7 +8,8 @@ * Scrubbed: `randomUUID()` session ids → `{{sessionId}}`; the temp `mkdtemp` * cwd → `{{cwd}}` (it appears in terminal-card `_meta` and the log header); * JSON-RPC request `id` → a stable per-transcript sequence; the log's per-event - * `time` (epoch ms) and header `createdAt` → 0. NOT scrubbed: the log's `seq` + * `time` (epoch ms) and header `createdAt` → 0; a `hook/result` event's + * `durationMs` (wall-clock hook runtime) → 0. NOT scrubbed: the log's `seq` * (deterministic — `seq = log.length`, part of the event-log contract). * * See docs/rfc/implemented/testing/2026-06-19-acp-snapshot-tests.md. @@ -97,6 +98,13 @@ export function normalizeSessionLog(rawLog: string, ctx: NormalizeContext): stri } else if ('time' in record) { // Event line: zero the epoch-ms timestamp; keep seq (deterministic). record.time = 0 + // A hook/result carries the hook's wall-clock runtime (`data.durationMs`), + // which is run-to-run noise like `time` — zero it so the golden reflects + // the hook's decision/exit, not how long the shell took. + if (record.type === 'hook/result' && record.data !== null && typeof record.data === 'object') { + const data = record.data as Record + if ('durationMs' in data) data.durationMs = 0 + } } return scrubValue(record, ctx) as Record }) diff --git a/examples/acp-agent/tests/snapshots/hook-prompt-block/input.json b/examples/acp-agent/tests/snapshots/hook-prompt-block/input.json new file mode 100644 index 0000000000..1995199566 --- /dev/null +++ b/examples/acp-agent/tests/snapshots/hook-prompt-block/input.json @@ -0,0 +1,7 @@ +{ + "steps": [ + { "op": "initialize" }, + { "op": "newSession" }, + { "op": "prompt", "text": "Delete everything in the repo." } + ] +} diff --git a/examples/acp-agent/tests/snapshots/hook-prompt-block/session.jsonl b/examples/acp-agent/tests/snapshots/hook-prompt-block/session.jsonl new file mode 100644 index 0000000000..74b8ba0c67 --- /dev/null +++ b/examples/acp-agent/tests/snapshots/hook-prompt-block/session.jsonl @@ -0,0 +1,5 @@ +{"type":"session","version":0,"id":"{{sessionId}}","createdAt":0,"cwd":"{{cwd}}"} +{"type":"turn/start","seq":0,"time":0,"data":{"turn":1,"trigger":{"kind":"message","source":{"kind":"user"}}}} +{"type":"hook/invoked","seq":1,"time":0,"data":{"turn":1,"point":"UserPromptSubmit","dialect":"claude","handlerId":"claude:UserPromptSubmit:1"}} +{"type":"hook/result","seq":2,"time":0,"data":{"turn":1,"point":"UserPromptSubmit","handlerId":"claude:UserPromptSubmit:1","decision":"block","exitCode":2,"stderrSummary":"blocked by policy hook","durationMs":0}} +{"type":"turn/end","seq":3,"time":0,"data":{"turn":1,"reason":{"kind":"rejected","reason":"blocked by policy hook"}}} diff --git a/examples/acp-agent/tests/snapshots/hook-prompt-block/stdout.golden.jsonl b/examples/acp-agent/tests/snapshots/hook-prompt-block/stdout.golden.jsonl new file mode 100644 index 0000000000..6f6e5b662f --- /dev/null +++ b/examples/acp-agent/tests/snapshots/hook-prompt-block/stdout.golden.jsonl @@ -0,0 +1,3 @@ +{"jsonrpc":"2.0","id":1,"result":{"protocolVersion":1,"agentInfo":{"name":"deepseek-harness-acp","version":"0.0.1"},"agentCapabilities":{"loadSession":true,"promptCapabilities":{"image":false,"audio":false,"embeddedContext":false}},"authMethods":[]}} +{"jsonrpc":"2.0","id":2,"result":{"sessionId":"{{sessionId}}"}} +{"jsonrpc":"2.0","id":3,"result":{"stopReason":"cancelled"}} diff --git a/examples/acp-agent/tests/snapshots/hook-prompt-block/workspace/hooks.json b/examples/acp-agent/tests/snapshots/hook-prompt-block/workspace/hooks.json new file mode 100644 index 0000000000..ee3da88fb1 --- /dev/null +++ b/examples/acp-agent/tests/snapshots/hook-prompt-block/workspace/hooks.json @@ -0,0 +1,11 @@ +{ + "hooks": { + "UserPromptSubmit": [ + { + "hooks": [ + { "type": "command", "command": "echo 'blocked by policy hook' >&2; exit 2" } + ] + } + ] + } +} diff --git a/packages/README.md b/packages/README.md index e14a1bff0e..8bdfe5cf9c 100644 --- a/packages/README.md +++ b/packages/README.md @@ -90,6 +90,8 @@ The rule: **extension** plugins depend on interfaces, never on the concrete loop | `tool-subagent/` | `subagent` | Model-facing `subagent` delegation tool over `ctx.subagents` | (registers on `ctx.tools`) | | `tool-todo/` | `todo` | Model-facing `todo_write` tool; writes the whole task list to the session log (`todo/write`) | (registers on `ctx.tools`) | | `hook-protocol/` | `hooks` | Shared Claude Code / Codex hook wire-protocol library: matcher, codec, `runHook`, merge, `hook/*` events | (none — library, no service) | +| `hooks-claude/` | `hooks` | Bridge: runs a Claude Code `hooks.json` / settings on the interception seams | (registers event listeners) | +| `hooks-codex/` | `hooks` | Bridge: runs a Codex `hooks.json` (a subset of the CC protocol) on the seams | (registers event listeners) | | `brand/` | `util` | Type-only `Branded` nominal-typing primitive (no runtime code, no harness deps) | (none — type-only) | Each package has its own `README.md` with purpose, service API, events, extension points, and deliberate non-goals (TODOs). diff --git a/packages/hooks/hooks-claude/README.md b/packages/hooks/hooks-claude/README.md new file mode 100644 index 0000000000..02cdfc9894 --- /dev/null +++ b/packages/hooks/hooks-claude/README.md @@ -0,0 +1,51 @@ +# @deepseek-ai/dsh-hooks-claude + +A cordis plugin that runs a user's existing **Claude Code** hook config (a `hooks.json`, or a settings file's `hooks` key) on the harness's canonical interception seams. It is the **CC dialect** half of the hooks subsystem: it owns CC's per-event stdin payloads, CC's env + `${CLAUDE_PLUGIN_ROOT}`/`${CLAUDE_PROJECT_DIR}` substitution, and the mapping from a hook's neutral outcome onto the harness's typed Decisions. The dialect-agnostic primitives (matcher, exit-code/stdout codec, `ctx.bash` execution, most-restrictive merge, the `hook/*` events) come from [`@deepseek-ai/dsh-hook-protocol`](../hook-protocol/README.md). + +A native cordis plugin could do everything this bridge does — more powerfully, with typed returns and no serialization boundary. **The bridge exists only to run UNMODIFIED external CC hooks faithfully**; anything bespoke should be a native plugin on the same seams (see [the interception-seams RFC](../../../docs/rfc/implemented/feature/2026-06-30-interception-seams.md)). + +## Config + +```ts +import type { Config } from '@deepseek-ai/dsh-hooks-claude' +const config: Config = { + configPath: '/path/to/hooks.json', // required: a hooks.json or a settings file with a `hooks` key + pluginRoot: '/path/to/plugin', // optional: replaces ${CLAUDE_PLUGIN_ROOT} in command strings + projectDir: '/path/to/project', // optional: replaces ${CLAUDE_PROJECT_DIR} AND set as the hook env var + defaultTimeoutMs: 600_000, // optional: per-hook timeout when a hook sets none (CC default) +} +``` + +In a `cordis.yml`: + +```yaml +- dsh-hooks-claude: + configPath: ./.claude/hooks.json + pluginRoot: ./.claude/plugins/my-plugin + projectDir: . +``` + +The config is parsed **once** at load. A read/parse failure is contained — the bridge logs a warning and registers nothing rather than crashing boot (a typo'd path must not take the agent down). Only `type: 'command'` hooks run; a `prompt`/`agent`/HTTP hook is parsed-and-skipped with a warning. + +## Hook points → seam Decisions + +| CC hook | Harness seam | Mapping | +|---|---|---| +| `SessionStart` | `agent/session-start` (emit) | additionalContext → `agent.inject()` into the new session (cannot block) | +| `UserPromptSubmit` | `agent/prompt-submit` (waterfall) | `deny` → `PromptDecision.block`; additionalContext → `allow` with context | +| `PreToolUse` | `tools/pre-execute` (waterfall) | `deny` → `PreToolDecision.deny`; `ask` → `PreToolDecision.ask` | +| `PostToolUse` | `tools/post-execute` (waterfall) | `deny` → `block` with feedback; additionalContext → `accept` with context | +| `Stop` | `agent/turn-continuation` (waterfall) | a blocking Stop hook forces `continue`, feeding its reason as next-step steering | +| `SubagentStart` | `subagent/start` (emit) | additionalContext → `agent.inject()` into the live child | +| `SubagentStop` | `subagent/end` (emit) | observe-only | + +The matcher subject is the tool name (`PreToolUse`/`PostToolUse`), the session source (`SessionStart`), or the child's agent type (`SubagentStart`/`SubagentStop`); `UserPromptSubmit`/`Stop` ignore matchers. Multiple file-configured hooks on one point run concurrently and fold most-restrictively (`deny > ask > allow`, see `dsh-hook-protocol`). + +## Context source + +Injected context carries an explicit `{ kind: 'plugin', plugin: 'hooks-claude' }` source. `agent.inject()` defaults a missing source to `{ kind: 'user' }`, which would mislabel plugin context as a user prompt — so the bridge always names itself. + +## Deferred (faithful-but-degraded) + +- **`updatedInput` (tool-input rewrite)** is logged + warned, **not honored** — input rewrite is a deferred consistency-design problem ([the pre-tool-input-rewrite RFC](../../../docs/rfc/proposed/feature/2026-06-30-pre-tool-input-rewrite.md)). +- **Stop loop-guard.** CC breaks an infinite force-continue with `stop_hook_active` (true once a Stop hook has fired this run) plus a max-consecutive cap; both are deferred (`TODO(stop-loop-guard)`). Today `stop_hook_active` is always `false`, so a Stop hook that unconditionally blocks would force-continue every step — a hook author must self-limit until the guard lands. diff --git a/packages/hooks/hooks-claude/package.json b/packages/hooks/hooks-claude/package.json new file mode 100644 index 0000000000..5cc39f9999 --- /dev/null +++ b/packages/hooks/hooks-claude/package.json @@ -0,0 +1,49 @@ +{ + "name": "@deepseek-ai/dsh-hooks-claude", + "description": "Bridge plugin: run a Claude Code hooks.json / settings hook config on the DeepSeek Harness interception seams", + "version": "0.0.1", + "private": true, + "type": "module", + "main": "lib/index.js", + "types": "lib/types/index.d.ts", + "exports": { + ".": { + "types": "./lib/types/index.d.ts", + "default": "./lib/index.js" + }, + "./src/*": "./src/*", + "./package.json": "./package.json" + }, + "files": [ + "lib/index.js", + "lib/types/**/*.d.ts", + "lib/types/**/*.d.ts.map", + "src" + ], + "license": "BSD-3-Clause", + "dependencies": { + "schemastery": "^3.18.0" + }, + "peerDependencies": { + "@deepseek-ai/dsh-agent": "^0.0.1", + "@deepseek-ai/dsh-hook-protocol": "^0.0.1", + "@deepseek-ai/dsh-llm": "^0.0.1", + "@deepseek-ai/dsh-session": "^0.0.1", + "@deepseek-ai/dsh-subagent": "^0.0.1", + "@deepseek-ai/dsh-tools": "^0.0.1", + "cordis": "^4.0.0-rc.6" + }, + "devDependencies": { + "@deepseek-ai/dsh-agent": "workspace:^", + "@deepseek-ai/dsh-agent-loop": "workspace:^", + "@deepseek-ai/dsh-bash": "workspace:^", + "@deepseek-ai/dsh-bash-local": "workspace:^", + "@deepseek-ai/dsh-hook-protocol": "workspace:^", + "@deepseek-ai/dsh-llm": "workspace:^", + "@deepseek-ai/dsh-session": "workspace:^", + "@deepseek-ai/dsh-subagent": "workspace:^", + "@deepseek-ai/dsh-system-prompt": "workspace:^", + "@deepseek-ai/dsh-tools": "workspace:^", + "cordis": "^4.0.0-rc.6" + } +} diff --git a/packages/hooks/hooks-claude/src/config.ts b/packages/hooks/hooks-claude/src/config.ts new file mode 100644 index 0000000000..d78486e58a --- /dev/null +++ b/packages/hooks/hooks-claude/src/config.ts @@ -0,0 +1,100 @@ +/** + * Parse a Claude Code hook config file into the shared {@link MatcherGroup} + * shape, faithfully to CC's `hooks.json` / settings `hooks` key format. + * + * A CC config maps each event name to an array of matcher groups, each holding + * an array of typed hooks. Only `type: 'command'` hooks run here; other types + * (`prompt`/`agent`/`http`) are PARSED but skipped with a warning (faithful-but- + * degraded — the same stance Codex takes). The `command` string undergoes + * `${CLAUDE_PLUGIN_ROOT}` substitution at parse time so the runner sees a literal. + * + * @module @deepseek-ai/dsh-hooks-claude/config + */ + +import type { MatcherGroup } from '@deepseek-ai/dsh-hook-protocol' + +/** A parsed CC config: event name → its matcher groups (command hooks only). */ +export type ClaudeHookConfig = Record + +/** A skipped non-command hook, surfaced so the bridge can warn about it. */ +export interface SkippedHook { + event: string + type: string +} + +/** The outcome of parsing one config file: the runnable groups + what was skipped. */ +export interface ParsedClaudeConfig { + config: ClaudeHookConfig + skipped: SkippedHook[] +} + +/** Substitution variables applied to each `command` string at parse time. */ +export interface SubstitutionVars { + /** Replaces `${CLAUDE_PLUGIN_ROOT}` — the plugin's root dir. */ + pluginRoot?: string + /** Replaces `${CLAUDE_PROJECT_DIR}` — the project root. */ + projectDir?: string +} + +/** A plain (non-null, non-array) object, else undefined. */ +function asObject(value: unknown): Record | undefined { + return typeof value === 'object' && value !== null && !Array.isArray(value) + ? value as Record + : undefined +} + +/** Apply `${CLAUDE_PLUGIN_ROOT}` / `${CLAUDE_PROJECT_DIR}` substitution to a command string. */ +export function substituteCommand(command: string, vars: SubstitutionVars): string { + let out = command + if (vars.pluginRoot !== undefined) out = out.split('${CLAUDE_PLUGIN_ROOT}').join(vars.pluginRoot) + if (vars.projectDir !== undefined) out = out.split('${CLAUDE_PROJECT_DIR}').join(vars.projectDir) + return out +} + +/** + * Parse a raw Claude Code config object (the value under the `hooks` key, or a + * `hooks.json` whose top level IS that map) into runnable {@link MatcherGroup}s. + * Non-command hooks and malformed entries are dropped (recorded in `skipped` / + * silently ignored) rather than throwing — a bad hook config must not crash boot. + * `vars` are substituted into every surviving `command`. + */ +export function parseClaudeConfig(raw: unknown, vars: SubstitutionVars = {}): ParsedClaudeConfig { + const config: ClaudeHookConfig = {} + const skipped: SkippedHook[] = [] + // Accept either `{ hooks: { … } }` (a settings file) or the bare event map. + const root = asObject(raw) + const hooksMap = root ? asObject(root.hooks) ?? root : undefined + if (!hooksMap) return { config, skipped } + + for (const [event, rawGroups] of Object.entries(hooksMap)) { + if (!Array.isArray(rawGroups)) continue + const groups: MatcherGroup[] = [] + for (const rawGroup of rawGroups) { + const group = asObject(rawGroup) + if (!group || !Array.isArray(group.hooks)) continue + const commands: MatcherGroup['hooks'] = [] + for (const rawHook of group.hooks) { + const hook = asObject(rawHook) + if (!hook) continue + const type = typeof hook.type === 'string' ? hook.type : 'command' + if (type !== 'command') { + skipped.push({ event, type }) + continue + } + if (typeof hook.command !== 'string') continue + commands.push({ + command: substituteCommand(hook.command, vars), + ...typeof hook.timeout === 'number' ? { timeoutSec: hook.timeout } : {}, + }) + } + if (commands.length === 0) continue + groups.push({ + ...typeof group.matcher === 'string' ? { matcher: group.matcher } : {}, + hooks: commands, + }) + } + if (groups.length > 0) config[event] = groups + } + + return { config, skipped } +} diff --git a/packages/hooks/hooks-claude/src/index.ts b/packages/hooks/hooks-claude/src/index.ts new file mode 100644 index 0000000000..5e8478e2b7 --- /dev/null +++ b/packages/hooks/hooks-claude/src/index.ts @@ -0,0 +1,300 @@ +/** + * `dsh-hooks-claude` — a bridge plugin that runs a user's existing Claude Code + * hook config (`hooks.json` / a settings file's `hooks` key) on the harness's + * canonical interception seams. It is the CC DIALECT half of the hooks + * subsystem: it owns CC's per-event stdin payloads, CC's env + + * `${CLAUDE_PLUGIN_ROOT}` substitution, and the mapping from a hook's neutral + * outcome onto the harness's typed Decisions. The dialect-agnostic primitives + * (matcher, exit-code/stdout codec, `ctx.bash` execution, most-restrictive + * merge, the `hook/*` events) come from `@deepseek-ai/dsh-hook-protocol`. + * + * A native cordis plugin could do everything this bridge does — more powerfully, + * with typed returns and no serialization boundary. The bridge exists only to + * run UNMODIFIED external CC hooks faithfully; anything bespoke should be a + * native plugin on the same seams. + * + * Scope: the seven in-scope hook points (`SessionStart`, `UserPromptSubmit`, + * `PreToolUse`, `PostToolUse`, `Stop`, `SubagentStart`, `SubagentStop`). Only + * `type: 'command'` hooks run; the matcher group config + exit-code/stdout + * protocol are byte-faithful to CC. `updatedInput` (tool-input rewrite) is + * logged + warned, not honored (deferred — see the interception-seams RFC). + * + * @module @deepseek-ai/dsh-hooks-claude + */ + +import { readFileSync } from 'node:fs' +import type { Context } from 'cordis' +import z from 'schemastery' +import type { Agent, ContinuationDecision, HookContext, PromptDecision } from '@deepseek-ai/dsh-agent' +import type { ContentBlock, MessageSource } from '@deepseek-ai/dsh-llm' +import type { PostToolDecision, PreToolDecision, ToolExecution, ToolExecutionResult } from '@deepseek-ai/dsh-tools' +import { + appendHookInvoked, + appendHookResult, + matchesMatcher, + mergeHookOutputs, + runHook, + type HookOutput, + type MatcherGroup, + type MergedHookOutcome, +} from '@deepseek-ai/dsh-hook-protocol' +// Side-effect type import: pulls in the `subagent/start` + `subagent/end` event +// declarations (declaration-merged into cordis `Events` by dsh-subagent) so the +// SubagentStart/SubagentStop listeners below type-check. +import type {} from '@deepseek-ai/dsh-subagent' +import { parseClaudeConfig, type ClaudeHookConfig } from './config.ts' + +export const name = 'hooks-claude' +// `bash` is required to run hooks; the rest are read opportunistically via +// ctx.get so a deployment can load this bridge without every seam present. +export const inject = ['bash'] + +/** Plugin config: where the CC hook config lives + substitution roots. */ +export interface Config { + /** Path to a `hooks.json` or a settings file whose `hooks` key holds the config. */ + configPath: string + /** Replaces `${CLAUDE_PLUGIN_ROOT}` in command strings (the plugin's root dir). */ + pluginRoot?: string + /** Replaces `${CLAUDE_PROJECT_DIR}` in command strings + set as the hook env var. */ + projectDir?: string + /** Default per-hook timeout in ms when a hook sets none (CC default: 600000). */ + defaultTimeoutMs?: number +} + +export const Config: z = z.object({ + configPath: z.string().required(), + pluginRoot: z.string(), + projectDir: z.string(), + defaultTimeoutMs: z.number().default(600_000), +}) + +/** A stable per-handler id so an invoked/result pair correlates in the log. */ +let handlerCounter = 0 +function nextHandlerId(point: string): string { + return `claude:${point}:${++handlerCounter}` +} + +/** The `{kind:'plugin'}` source stamped on every context this bridge injects. */ +const PLUGIN_SOURCE: MessageSource = { kind: 'plugin', plugin: 'hooks-claude' } + +/** Truncate a stderr blob for the `hook/result` summary field. */ +function summarize(stderr: string): string | undefined { + const t = stderr.trim() + if (t.length === 0) return undefined + return t.length > 500 ? t.slice(0, 500) + '…' : t +} + +export function apply(ctx: Context, config: Config): void { + // --- Parse the config ONCE at load. A read/parse failure is contained: the + // bridge logs and registers nothing rather than crashing boot (a typo'd path + // must not take the agent down). --- + let parsed: ClaudeHookConfig = {} + try { + const raw: unknown = JSON.parse(readFileSync(config.configPath, 'utf8')) + const result = parseClaudeConfig(raw, { + ...config.pluginRoot !== undefined ? { pluginRoot: config.pluginRoot } : {}, + ...config.projectDir !== undefined ? { projectDir: config.projectDir } : {}, + }) + parsed = result.config + for (const s of result.skipped) { + ctx.logger.warn(`hooks-claude: skipping unsupported "${s.type}" hook on ${s.event} (only command hooks run)`) + } + } catch (error: unknown) { + ctx.logger.warn(`hooks-claude: could not load hook config "${config.configPath}": ${String(error)} — no hooks registered`) + return + } + + const defaultTimeoutMs = config.defaultTimeoutMs ?? 600_000 + const hookEnv = config.projectDir !== undefined ? { CLAUDE_PROJECT_DIR: config.projectDir } : undefined + + /** + * Run every command hook configured for `point` whose matcher selects + * `matchQuery`, with the per-event `payload` on stdin, and fold the results. + * Writes a `hook/invoked`/`hook/result` pair per hook into the session when one + * is available (the mid-turn points always have an open turn). Returns the + * merged outcome (a neutral, already-most-restrictive view) for the caller to + * map onto its seam decision. `matchQuery` is the event's matcher subject + * (tool name, session source, …); `''` for events that ignore matchers. + */ + async function runPoint( + point: string, + matchQuery: string, + payload: unknown, + opts: { agent?: Agent; turn?: number; signal?: AbortSignal }, + ): Promise { + const groups: MatcherGroup[] = parsed[point] ?? [] + const outputs: HookOutput[] = [] + for (const group of groups) { + if (!matchesMatcher(group.matcher, matchQuery, 'claude')) continue + for (const hook of group.hooks) { + const handlerId = nextHandlerId(point) + const session = opts.agent?.session + if (session && opts.turn !== undefined) { + appendHookInvoked(session, { + turn: opts.turn, point, dialect: 'claude', handlerId, + ...group.matcher !== undefined ? { matcher: group.matcher } : {}, + }) + } + const { output, durationMs } = await runHook(ctx.bash, hook, { + payload, + ...hookEnv ? { env: hookEnv } : {}, + ...opts.signal ? { signal: opts.signal } : {}, + defaultTimeoutMs, + trailingNewline: true, + }, () => performance.now()) + outputs.push(output) + if (output.updatedInput !== undefined) { + ctx.logger.warn(`hooks-claude: ${point} hook requested updatedInput, which is not yet honored (ignored)`) + } + if (session && opts.turn !== undefined) { + const stderrSummary = summarize(output.stderr) + appendHookResult(session, { + turn: opts.turn, point, handlerId, + decision: output.decision ?? (output.continue === false ? 'stop' : 'pass'), + ...output.exitCode !== undefined ? { exitCode: output.exitCode } : {}, + ...stderrSummary !== undefined ? { stderrSummary } : {}, + durationMs, + }) + } + } + } + return mergeHookOutputs(outputs) + } + + /** Build a HookContext from accumulated additionalContext strings, or undefined when none. */ + function contextFrom(merged: MergedHookOutcome): HookContext | undefined { + if (merged.additionalContext.length === 0) return undefined + const content: ContentBlock[] = merged.additionalContext.map(text => ({ type: 'text', text })) + return { content, source: PLUGIN_SOURCE } + } + + // --- SessionStart: emit (cannot block). Inject any additionalContext into the + // agent so the first request sees it. The matcher subject is the source. --- + ctx.on('agent/session-start', (agent, source) => { + void runPoint('SessionStart', source, sessionStartPayload(agent, source), { agent }) + .then((merged) => { + const context = contextFrom(merged) + if (context) agent.inject(context.content, { source: context.source }) + }) + .catch((error: unknown) => { + ctx.logger.warn(`hooks-claude: SessionStart hook failed: ${String(error)}`) + }) + }) + + // --- UserPromptSubmit → PromptDecision. The prompt text is the payload; no + // matcher subject (CC ignores matchers for this event). --- + ctx.on('agent/prompt-submit', async (agent, content, _source, next): Promise => { + const turn = lastTurn(agent) + const merged = await runPoint('UserPromptSubmit', '', promptPayload(agent, content), { agent, turn }) + if (merged.decision === 'deny') { + return { kind: 'block', reason: merged.reason ?? 'blocked by UserPromptSubmit hook' } + } + const context = contextFrom(merged) + if (context) return { kind: 'allow', additionalContext: context } + return next() + }) + + // --- PreToolUse → PreToolDecision. Matcher subject is the tool name. --- + ctx.on('tools/pre-execute', async (exec, next): Promise => { + const turn = lastTurn(exec.agent) + const merged = await runPoint('PreToolUse', exec.name, preToolPayload(exec), { ...exec.agent ? { agent: exec.agent } : {}, turn, ...exec.signal ? { signal: exec.signal } : {} }) + if (merged.decision === 'deny') return { kind: 'deny', reason: merged.reason ?? 'blocked by PreToolUse hook' } + if (merged.decision === 'ask') return { kind: 'ask', ...merged.reason !== undefined ? { reason: merged.reason } : {} } + return next() + }) + + // --- PostToolUse → PostToolDecision. Matcher subject is the tool name. --- + ctx.on('tools/post-execute', async (exec, result, next): Promise => { + const turn = lastTurn(exec.agent) + const merged = await runPoint('PostToolUse', exec.name, postToolPayload(exec, result), { ...exec.agent ? { agent: exec.agent } : {}, turn, ...exec.signal ? { signal: exec.signal } : {} }) + const context = contextFrom(merged) + if (merged.decision === 'deny') { + return { kind: 'block', feedback: [{ type: 'text', text: merged.reason ?? 'blocked by PostToolUse hook' }], ...context ? { additionalContext: context } : {} } + } + if (context) return { kind: 'accept', additionalContext: context } + return next() + }) + + // --- Stop → ContinuationDecision. CC's Stop hook can force the conversation to + // CONTINUE (block the stop) with stderr/reason as the continuation. No matcher. + // TODO(stop-loop-guard): CC breaks an infinite force-continue with + // `stop_hook_active` (set true once a Stop hook has already fired this run) plus + // a max-consecutive cap; both are deferred. Today `stop_hook_active` is always + // false, so a Stop hook that unconditionally blocks would force-continue every + // step — a hook author must self-limit until the guard lands. --- + ctx.on('agent/turn-continuation', async (agent, turn, _default, next): Promise => { + const merged = await runPoint('Stop', '', stopPayload(agent), { agent, turn }) + if (merged.decision === 'deny' && merged.reason !== undefined) { + // A blocking Stop hook forces continuation, feeding its reason as next-step steering. + return { action: 'continue', reason: { content: [{ type: 'text', text: merged.reason }], source: PLUGIN_SOURCE } } + } + return next() + }) + + // --- SubagentStart / SubagentStop: observe-only emits (the subagent seam is + // observe-only this cut). A SubagentStart hook's additionalContext is injected + // into the live child; SubagentStop only observes. No matcher subject. --- + ctx.on('subagent/start', (info) => { + const child = ctx.get('agents')?.get(info.id) + void runPoint('SubagentStart', info.agentType ?? '', subagentStartPayload(info), { ...child ? { agent: child } : {} }) + .then((merged) => { + const context = contextFrom(merged) + if (context && child) child.inject(context.content, { source: context.source }) + }) + .catch((error: unknown) => { ctx.logger.warn(`hooks-claude: SubagentStart hook failed: ${String(error)}`) }) + }) + ctx.on('subagent/end', (info) => { + // No `.then`/inject here (SubagentStop only observes) and no session is + // passed, so runPoint cannot reject — no `.catch` is needed (one would be + // dead code). The observe-only run is fire-and-forget. + void runPoint('SubagentStop', info.agentType ?? '', subagentStopPayload(info), {}) + }) +} + +// --- Per-event stdin payloads (the CC DIALECT shape). Field names match CC's +// hook input schema; this is the part a bridge owns. --- + +/** The last (open or just-closed) turn number in the agent's log, or 0. */ +function lastTurn(agent: Agent | undefined): number { + if (!agent) return 0 + const last = [...agent.session.events].findLast(e => e.type === 'turn/start') + /* v8 ignore next -- the `: 0` arm is a defensive fallback: lastTurn is only + called from the mid-turn seams (prompt-submit/pre-/post-execute/continuation), + which always run inside an open turn, so `last` is always a turn/start here. */ + return last?.type === 'turn/start' ? last.data.turn : 0 +} + +/** Flatten content blocks to the text a hook payload carries (the common case). */ +function blocksToText(content: ContentBlock[]): string { + return content.filter((b): b is Extract => b.type === 'text').map(b => b.text).join('') +} + +function base(agent: Agent | undefined, event: string): Record { + return { + session_id: agent?.session.header.id ?? '', + cwd: agent?.session.header.cwd ?? process.cwd(), + hook_event_name: event, + } +} + +function sessionStartPayload(agent: Agent, source: string): Record { + return { ...base(agent, 'SessionStart'), source } +} +function promptPayload(agent: Agent, content: ContentBlock[]): Record { + return { ...base(agent, 'UserPromptSubmit'), prompt: blocksToText(content) } +} +function preToolPayload(exec: ToolExecution): Record { + return { ...base(exec.agent, 'PreToolUse'), tool_name: exec.name, tool_input: exec.arguments, tool_use_id: exec.callId } +} +function postToolPayload(exec: ToolExecution, result: ToolExecutionResult): Record { + return { ...base(exec.agent, 'PostToolUse'), tool_name: exec.name, tool_input: exec.arguments, tool_use_id: exec.callId, tool_response: blocksToText(result.content) } +} +function stopPayload(agent: Agent): Record { + return { ...base(agent, 'Stop'), stop_hook_active: false } +} +function subagentStartPayload(info: { id: string; agentType?: string }): Record { + return { hook_event_name: 'SubagentStart', agent_id: info.id, ...info.agentType !== undefined ? { agent_type: info.agentType } : {} } +} +function subagentStopPayload(info: { id: string; agentType?: string }): Record { + return { hook_event_name: 'SubagentStop', agent_id: info.id, stop_hook_active: false, ...info.agentType !== undefined ? { agent_type: info.agentType } : {} } +} diff --git a/packages/hooks/hooks-claude/tests/bridge.spec.ts b/packages/hooks/hooks-claude/tests/bridge.spec.ts new file mode 100644 index 0000000000..73cd66df10 --- /dev/null +++ b/packages/hooks/hooks-claude/tests/bridge.spec.ts @@ -0,0 +1,335 @@ +import { afterEach, describe, expect, it } from 'vitest' +import { mkdtempSync, rmSync, writeFileSync, chmodSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { Context } from 'cordis' +import Loader from '@cordisjs/plugin-loader' +import LlmService from '@deepseek-ai/dsh-llm' +import SessionStore, { type SessionEvent } from '@deepseek-ai/dsh-session' +import SystemPrompt from '@deepseek-ai/dsh-system-prompt' +import ToolRegistry, { defineTool } from '@deepseek-ai/dsh-tools' +import AgentRegistry, { AgentId } from '@deepseek-ai/dsh-agent' +import AgentLoop, { type ReactLoopAgent } from '@deepseek-ai/dsh-agent-loop' +import { LocalBashExecutor } from '@deepseek-ai/dsh-bash-local' +import * as HooksClaude from '@deepseek-ai/dsh-hooks-claude' +import { MockAdapter, textResponse, toolCallResponse } from '../../../core/agent-loop/tests/mock-adapter.ts' + +/** + * Full-loop bridge tests: a scripted mock MODEL drives the REAL agent loop + REAL + * bash executor, and the REAL `dsh-hooks-claude` bridge runs REAL shell hook + * scripts written to a temp dir — only the model is mocked (the "prefer the real + * implementation" rule). Each test writes a `hooks.json` + executable scripts, + * loads the bridge pointed at them, and asserts the hook's effect on the loop. + */ + +const dirs: string[] = [] +afterEach(() => { for (const d of dirs.splice(0)) rmSync(d, { recursive: true, force: true }) }) + +/** Write a hooks.json + named executable scripts into a fresh temp dir. */ +function writeConfig(hooks: unknown, scripts: Record = {}): string { + const dir = mkdtempSync(join(tmpdir(), 'dsh-hooks-claude-')) + dirs.push(dir) + writeFileSync(join(dir, 'hooks.json'), JSON.stringify({ hooks })) + for (const [name, body] of Object.entries(scripts)) { + const path = join(dir, name) + writeFileSync(path, body) + chmodSync(path, 0o755) + } + return dir +} + +async function harness(configDir: string, adapter: MockAdapter): Promise { + const ctx = new Context() + await ctx.plugin(LlmService) + await ctx.plugin(SessionStore) + await ctx.plugin(SystemPrompt) + await ctx.plugin(ToolRegistry) + await ctx.plugin(AgentRegistry) + await ctx.plugin(AgentLoop, { agents: [] }) + await ctx.plugin(LocalBashExecutor, { timeoutMs: 10_000 }) + await ctx.plugin(HooksClaude, { configPath: join(configDir, 'hooks.json') }) + ctx.llm.registerAdapter(['mock'], adapter) + return ctx +} + +function waitForIdle(ctx: Context, agent: ReactLoopAgent): Promise { + return new Promise((resolve) => { + const dispose = ctx.on('agent/status', (subject, status) => { + if (subject === agent && status === 'idle') { dispose(); resolve() } + }) + }) +} + +function events(agent: ReactLoopAgent): SessionEvent[] { + return [...agent.session.events] +} + +describe('hooks-claude bridge — UserPromptSubmit', () => { + it('a UserPromptSubmit hook that exits 2 blocks the prompt (rejected turn)', async () => { + // The UserPromptSubmit hook exits 2 (blocking) with a reason on stderr. + const dir = mkdtempSync(join(tmpdir(), 'dsh-hooks-claude-')) + dirs.push(dir) + const block = join(dir, 'block.sh') + writeFileSync(block, '#!/usr/bin/env bash\necho "prompt denied by policy" >&2\nexit 2\n') + chmodSync(block, 0o755) + writeFileSync(join(dir, 'hooks.json'), JSON.stringify({ hooks: { UserPromptSubmit: [{ hooks: [{ type: 'command', command: block }] }] } })) + + const adapter = new MockAdapter([textResponse('should not run')]) + const ctx = await harness(dir, adapter) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'do something' }]) + await waitForIdle(ctx, agent) + + // The prompt was blocked: model never called, turn ended rejected. + expect(adapter.requests).toHaveLength(0) + const turnEnd = events(agent).findLast(e => e.type === 'turn/end') + expect(turnEnd?.type === 'turn/end' && turnEnd.data.reason.kind).toBe('rejected') + // The hook ran and was recorded. + expect(events(agent).some(e => e.type === 'hook/invoked' && e.data.point === 'UserPromptSubmit')).toBe(true) + expect(events(agent).some(e => e.type === 'hook/result' && e.data.decision === 'block')).toBe(true) + }) + + it('a UserPromptSubmit hook printing additionalContext injects it for the model', async () => { + const dir = mkdtempSync(join(tmpdir(), 'dsh-hooks-claude-')) + dirs.push(dir) + const ctxScript = join(dir, 'ctx.sh') + writeFileSync(ctxScript, '#!/usr/bin/env bash\necho \'{"hookSpecificOutput":{"hookEventName":"UserPromptSubmit","additionalContext":"remember: be brief"}}\'\n') + chmodSync(ctxScript, 0o755) + writeFileSync(join(dir, 'hooks.json'), JSON.stringify({ hooks: { UserPromptSubmit: [{ hooks: [{ type: 'command', command: ctxScript }] }] } })) + + const adapter = new MockAdapter([textResponse('ok')]) + const ctx = await harness(dir, adapter) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]) + await waitForIdle(ctx, agent) + + // The injected context reached the model and is recorded with the plugin source. + expect(JSON.stringify(adapter.requests[0]!.messages)).toContain('remember: be brief') + const ctxMsg = events(agent).find(e => e.type === 'context/message') + expect(ctxMsg?.type === 'context/message' && ctxMsg.data.source).toEqual({ kind: 'plugin', plugin: 'hooks-claude' }) + }) +}) + +describe('hooks-claude bridge — PreToolUse', () => { + it('a matching PreToolUse hook that exits 2 denies the tool (isError result), tool never runs', async () => { + const dir = mkdtempSync(join(tmpdir(), 'dsh-hooks-claude-')) + dirs.push(dir) + const deny = join(dir, 'deny.sh') + writeFileSync(deny, '#!/usr/bin/env bash\necho "danger tool blocked" >&2\nexit 2\n') + chmodSync(deny, 0o755) + // Matcher "danger" (literal) selects only the danger tool. + writeFileSync(join(dir, 'hooks.json'), JSON.stringify({ hooks: { PreToolUse: [{ matcher: 'danger', hooks: [{ type: 'command', command: deny }] }] } })) + + const adapter = new MockAdapter([toolCallResponse('c1', 'danger', {}), textResponse('done')]) + const ctx = await harness(dir, adapter) + let ran = false + ctx.tools.register(defineTool({ name: 'danger', description: 'd', parameters: {}, async execute() { ran = true; return [{ type: 'text', text: 'should not run' }] } })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'use danger' }]) + await waitForIdle(ctx, agent) + + expect(ran).toBe(false) + const result = events(agent).find(e => e.type === 'tool/result') + expect(result?.type === 'tool/result' && result.data.isError).toBe(true) + expect(result?.type === 'tool/result' && result.data.content.some(b => b.type === 'text' && b.text.includes('danger tool blocked'))).toBe(true) + }) + + it('a PreToolUse hook whose matcher does NOT match leaves the tool alone', async () => { + const dir = mkdtempSync(join(tmpdir(), 'dsh-hooks-claude-')) + dirs.push(dir) + const deny = join(dir, 'deny.sh') + writeFileSync(deny, '#!/usr/bin/env bash\nexit 2\n') + chmodSync(deny, 0o755) + // Matcher only targets "danger" — the "safe" tool is untouched. + writeFileSync(join(dir, 'hooks.json'), JSON.stringify({ hooks: { PreToolUse: [{ matcher: 'danger', hooks: [{ type: 'command', command: deny }] }] } })) + + const adapter = new MockAdapter([toolCallResponse('c1', 'safe', {}), textResponse('done')]) + const ctx = await harness(dir, adapter) + let ran = false + ctx.tools.register(defineTool({ name: 'safe', description: 's', parameters: {}, async execute() { ran = true; return [{ type: 'text', text: 'ran ok' }] } })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'use safe' }]) + await waitForIdle(ctx, agent) + + expect(ran).toBe(true) + const result = events(agent).find(e => e.type === 'tool/result') + expect(result?.type === 'tool/result' && result.data.isError).toBe(false) + }) +}) + +describe('hooks-claude bridge — PostToolUse', () => { + it('a PostToolUse hook that blocks (exit 2) turns the result into an isError with feedback', async () => { + const dir = mkdtempSync(join(tmpdir(), 'dsh-hooks-claude-')) + dirs.push(dir) + const block = join(dir, 'block.sh') + writeFileSync(block, '#!/usr/bin/env bash\necho "output rejected, retry" >&2\nexit 2\n') + chmodSync(block, 0o755) + writeFileSync(join(dir, 'hooks.json'), JSON.stringify({ hooks: { PostToolUse: [{ hooks: [{ type: 'command', command: block }] }] } })) + + const adapter = new MockAdapter([toolCallResponse('c1', 'echo', {}), textResponse('done')]) + const ctx = await harness(dir, adapter) + ctx.tools.register(defineTool({ name: 'echo', description: 'e', parameters: {}, async execute() { return [{ type: 'text', text: 'raw output' }] } })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]) + await waitForIdle(ctx, agent) + + const result = events(agent).find(e => e.type === 'tool/result') + // PostToolUse blocks AFTER the tool ran: the result is rewritten to isError + feedback. + expect(result?.type === 'tool/result' && result.data.isError).toBe(true) + expect(result?.type === 'tool/result' && result.data.content.some(b => b.type === 'text' && b.text.includes('output rejected, retry'))).toBe(true) + }) + + it('a PostToolUse hook printing additionalContext attaches it after the tool result', async () => { + const dir = mkdtempSync(join(tmpdir(), 'dsh-hooks-claude-')) + dirs.push(dir) + const s = join(dir, 'ctx.sh') + writeFileSync(s, '#!/usr/bin/env bash\necho \'{"hookSpecificOutput":{"hookEventName":"PostToolUse","additionalContext":"note: tool was slow"}}\'\n') + chmodSync(s, 0o755) + writeFileSync(join(dir, 'hooks.json'), JSON.stringify({ hooks: { PostToolUse: [{ hooks: [{ type: 'command', command: s }] }] } })) + + const adapter = new MockAdapter([toolCallResponse('c1', 'echo', {}), textResponse('done')]) + const ctx = await harness(dir, adapter) + ctx.tools.register(defineTool({ name: 'echo', description: 'e', parameters: {}, async execute() { return [{ type: 'text', text: 'ok' }] } })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]) + await waitForIdle(ctx, agent) + + const log = events(agent) + const resultIdx = log.findIndex(e => e.type === 'tool/result') + const ctxIdx = log.findIndex(e => e.type === 'context/message') + expect(ctxIdx).toBeGreaterThan(resultIdx) // context appended AFTER the tool result + const ctxMsg = log[ctxIdx] + expect(ctxMsg?.type === 'context/message' && ctxMsg.data.content.some(b => b.type === 'text' && b.text.includes('tool was slow'))).toBe(true) + }) + + it('a PreToolUse permissionDecision:ask degrades to ask (the tool is gated, not run)', async () => { + const dir = mkdtempSync(join(tmpdir(), 'dsh-hooks-claude-')) + dirs.push(dir) + const s = join(dir, 'ask.sh') + writeFileSync(s, '#!/usr/bin/env bash\necho \'{"hookSpecificOutput":{"hookEventName":"PreToolUse","permissionDecision":"ask","permissionDecisionReason":"needs approval"}}\'\n') + chmodSync(s, 0o755) + writeFileSync(join(dir, 'hooks.json'), JSON.stringify({ hooks: { PreToolUse: [{ hooks: [{ type: 'command', command: s }] }] } })) + + const adapter = new MockAdapter([toolCallResponse('c1', 'echo', {}), textResponse('done')]) + const ctx = await harness(dir, adapter) + let ran = false + ctx.tools.register(defineTool({ name: 'echo', description: 'e', parameters: {}, async execute() { ran = true; return [{ type: 'text', text: 'x' }] } })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]) + await waitForIdle(ctx, agent) + + // `ask` degrades to deny today (FIXME permissions): the tool does not run and the result is isError. + expect(ran).toBe(false) + const result = events(agent).find(e => e.type === 'tool/result') + expect(result?.type === 'tool/result' && result.data.isError).toBe(true) + expect(result?.type === 'tool/result' && result.data.content.some(b => b.type === 'text' && b.text.includes('needs approval'))).toBe(true) + }) +}) + +describe('hooks-claude bridge — SessionStart', () => { + it('a SessionStart hook injects additionalContext the first request sees', async () => { + const dir = mkdtempSync(join(tmpdir(), 'dsh-hooks-claude-')) + dirs.push(dir) + const s = join(dir, 'start.sh') + writeFileSync(s, '#!/usr/bin/env bash\necho \'{"hookSpecificOutput":{"hookEventName":"SessionStart","additionalContext":"project uses tabs"}}\'\n') + chmodSync(s, 0o755) + // matcher 'startup' selects the startup source. + writeFileSync(join(dir, 'hooks.json'), JSON.stringify({ hooks: { SessionStart: [{ matcher: 'startup', hooks: [{ type: 'command', command: s }] }] } })) + + const adapter = new MockAdapter([textResponse('ok')]) + const ctx = await harness(dir, adapter) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + // session-start fires async; wait a tick for the inject before sending. + await new Promise(r => setTimeout(r, 50)) + agent.send([{ type: 'text', text: 'go' }]) + await waitForIdle(ctx, agent) + + expect(JSON.stringify(adapter.requests[0]!.messages)).toContain('project uses tabs') + }) +}) + +describe('hooks-claude bridge — SubagentStart / SubagentStop (observe)', () => { + it('runs SubagentStart and SubagentStop hooks when the subagent lifecycle events fire', async () => { + const dir = mkdtempSync(join(tmpdir(), 'dsh-hooks-claude-')) + dirs.push(dir) + // Each hook touches a marker file so we can assert it ran (these events are + // observe-only — there is no decision to assert, only the side effect). + const startMarker = join(dir, 'start-ran') + const stopMarker = join(dir, 'stop-ran') + const startHook = join(dir, 'start.sh') + const stopHook = join(dir, 'stop.sh') + writeFileSync(startHook, `#!/usr/bin/env bash\ntouch "${startMarker}"\n`) + writeFileSync(stopHook, `#!/usr/bin/env bash\ntouch "${stopMarker}"\n`) + chmodSync(startHook, 0o755) + chmodSync(stopHook, 0o755) + writeFileSync(join(dir, 'hooks.json'), JSON.stringify({ hooks: { + SubagentStart: [{ hooks: [{ type: 'command', command: startHook }] }], + SubagentStop: [{ hooks: [{ type: 'command', command: stopHook }] }], + } })) + + const adapter = new MockAdapter([]) + const ctx = await harness(dir, adapter) + // Drive the observe-only lifecycle events directly (no real child needed — the + // bridge just listens). The agents registry is absent here, so SubagentStart's + // child lookup yields undefined and it simply runs the hook. + ctx.emit('subagent/start', { provider: 'inproc', id: AgentId('child-1'), agentType: 'researcher' }) + ctx.emit('subagent/end', { provider: 'inproc', id: AgentId('child-1'), agentType: 'researcher', stopReason: 'completed', lastAssistantMessage: [{ type: 'text', text: 'done' }] }) + // Both hooks run async (detached .then); let them settle. + await new Promise(r => setTimeout(r, 80)) + + const { existsSync } = await import('node:fs') + expect(existsSync(startMarker)).toBe(true) + expect(existsSync(stopMarker)).toBe(true) + }) +}) + +describe('hooks-claude bridge — load resilience', () => { + it('a missing config file registers no hooks and does not crash the loop', async () => { + const adapter = new MockAdapter([textResponse('fine')]) + const ctx = new Context() + await ctx.plugin(LlmService) + await ctx.plugin(SessionStore) + await ctx.plugin(SystemPrompt) + await ctx.plugin(ToolRegistry) + await ctx.plugin(AgentRegistry) + await ctx.plugin(AgentLoop, { agents: [] }) + await ctx.plugin(LocalBashExecutor, { timeoutMs: 10_000 }) + await ctx.plugin(HooksClaude, { configPath: '/nonexistent/hooks.json' }) + ctx.llm.registerAdapter(['mock'], adapter) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]) + await waitForIdle(ctx, agent) + // The turn ran normally — no hooks, no crash. + expect(adapter.requests).toHaveLength(1) + }) + + it('disposing the bridge fiber removes its listeners (HMR safety)', async () => { + const dir = writeConfig({ UserPromptSubmit: [{ hooks: [{ type: 'command', command: 'true' }] }] }) + const adapter = new MockAdapter([textResponse('ok')]) + const ctx = await harness(dir, adapter) + const fiber = await ctx.plugin(HooksClaude, { configPath: join(dir, 'hooks.json') }) + await fiber.dispose() + // After disposing this second mount, the FIRST mount's listeners still work, + // but the disposed one contributed none — assert no leaked listener throws. + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]) + await waitForIdle(ctx, agent) + expect(adapter.requests.length).toBeGreaterThanOrEqual(1) + }) + + it('has the namespace-plugin export shape (no stray default) so the Loader keeps name/inject/apply', () => { + // Postmortem 0001 guard: this plugin HAS `inject = ['bash']`, so a stray + // `export default apply` would collapse the module via `unwrapExports` + // (`exports.default ?? exports`), DROP `inject`, and crash at load with + // "cannot get property … without inject". Guard the shape directly. + expect('default' in HooksClaude).toBe(false) + expect(HooksClaude.name).toBe('hooks-claude') + expect(HooksClaude.inject).toEqual(['bash']) + const loader = Object.create(Loader.prototype) as Loader + const unwrapped = loader.unwrapExports(HooksClaude) as Record + expect(unwrapped).toBe(HooksClaude) + expect(unwrapped.name).toBe('hooks-claude') + expect(unwrapped.inject).toEqual(['bash']) + expect(typeof unwrapped.apply).toBe('function') + }) +}) diff --git a/packages/hooks/hooks-claude/tests/config.spec.ts b/packages/hooks/hooks-claude/tests/config.spec.ts new file mode 100644 index 0000000000..f635ef0fd9 --- /dev/null +++ b/packages/hooks/hooks-claude/tests/config.spec.ts @@ -0,0 +1,66 @@ +import { describe, expect, it } from 'vitest' +import { parseClaudeConfig, substituteCommand } from '@deepseek-ai/dsh-hooks-claude/src/config.ts' + +describe('substituteCommand', () => { + it('replaces CLAUDE_PLUGIN_ROOT and CLAUDE_PROJECT_DIR (all occurrences)', () => { + expect(substituteCommand('${CLAUDE_PLUGIN_ROOT}/x.sh', { pluginRoot: '/p' })).toBe('/p/x.sh') + expect(substituteCommand('${CLAUDE_PROJECT_DIR}/a ${CLAUDE_PROJECT_DIR}/b', { projectDir: '/proj' })).toBe('/proj/a /proj/b') + expect(substituteCommand('${CLAUDE_PLUGIN_ROOT}-${CLAUDE_PROJECT_DIR}', { pluginRoot: '/p', projectDir: '/d' })).toBe('/p-/d') + }) + it('leaves the command untouched when no vars are supplied', () => { + expect(substituteCommand('${CLAUDE_PLUGIN_ROOT}/x', {})).toBe('${CLAUDE_PLUGIN_ROOT}/x') + }) +}) + +describe('parseClaudeConfig', () => { + it('parses a bare event map and a settings-style { hooks: … } wrapper identically', () => { + const groups = { PreToolUse: [{ matcher: 'Bash', hooks: [{ type: 'command', command: 'x.sh' }] }] } + const bare = parseClaudeConfig(groups) + const wrapped = parseClaudeConfig({ hooks: groups }) + expect(bare.config).toEqual(wrapped.config) + expect(bare.config.PreToolUse).toEqual([{ matcher: 'Bash', hooks: [{ command: 'x.sh' }] }]) + }) + + it('carries timeout → timeoutSec and substitutes the command', () => { + const { config } = parseClaudeConfig( + { Stop: [{ hooks: [{ type: 'command', command: '${CLAUDE_PLUGIN_ROOT}/s.sh', timeout: 30 }] }] }, + { pluginRoot: '/p' }, + ) + expect(config.Stop).toEqual([{ hooks: [{ command: '/p/s.sh', timeoutSec: 30 }] }]) + }) + + it('skips non-command hooks (recorded) and keeps the command ones in the same group', () => { + const { config, skipped } = parseClaudeConfig({ + PreToolUse: [{ hooks: [ + { type: 'prompt', prompt: 'hi' }, + { type: 'command', command: 'ok.sh' }, + { type: 'http', url: 'http://x' }, + ] }], + }) + expect(config.PreToolUse).toEqual([{ hooks: [{ command: 'ok.sh' }] }]) + expect(skipped).toEqual([{ event: 'PreToolUse', type: 'prompt' }, { event: 'PreToolUse', type: 'http' }]) + }) + + it('treats a hook with no `type` as a command (CC default)', () => { + const { config } = parseClaudeConfig({ Stop: [{ hooks: [{ command: 'd.sh' }] }] }) + expect(config.Stop).toEqual([{ hooks: [{ command: 'd.sh' }] }]) + }) + + it('drops malformed entries without throwing: non-array groups, non-object group/hook, missing command, empty groups', () => { + expect(parseClaudeConfig({ PreToolUse: 'nope' }).config).toEqual({}) + expect(parseClaudeConfig({ PreToolUse: [42, { hooks: 'no' }, { hooks: [7, { type: 'command' }] }] }).config).toEqual({}) + // a group whose only hook lacks a command string drops the whole (empty) group + expect(parseClaudeConfig({ Stop: [{ hooks: [{ type: 'command', command: 5 }] }] }).config).toEqual({}) + }) + + it('returns empty for a non-object / null / array top level', () => { + expect(parseClaudeConfig(null).config).toEqual({}) + expect(parseClaudeConfig(42).config).toEqual({}) + expect(parseClaudeConfig([1, 2]).config).toEqual({}) + }) + + it('omits the matcher key when the group has none (match-all)', () => { + const { config } = parseClaudeConfig({ Stop: [{ hooks: [{ type: 'command', command: 's.sh' }] }] }) + expect('matcher' in config.Stop![0]!).toBe(false) + }) +}) diff --git a/packages/hooks/hooks-claude/tests/coverage.spec.ts b/packages/hooks/hooks-claude/tests/coverage.spec.ts new file mode 100644 index 0000000000..29dda6b850 --- /dev/null +++ b/packages/hooks/hooks-claude/tests/coverage.spec.ts @@ -0,0 +1,405 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { mkdtempSync, rmSync, writeFileSync, chmodSync, existsSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { Context } from 'cordis' +import LlmService from '@deepseek-ai/dsh-llm' +import SessionStore, { type SessionEvent } from '@deepseek-ai/dsh-session' +import SystemPrompt from '@deepseek-ai/dsh-system-prompt' +import ToolRegistry, { defineTool } from '@deepseek-ai/dsh-tools' +import AgentRegistry, { AgentId } from '@deepseek-ai/dsh-agent' +import AgentLoop, { type ReactLoopAgent } from '@deepseek-ai/dsh-agent-loop' +import { LocalBashExecutor } from '@deepseek-ai/dsh-bash-local' +import * as HooksClaude from '@deepseek-ai/dsh-hooks-claude' +import { MockAdapter, textResponse, toolCallResponse } from '../../../core/agent-loop/tests/mock-adapter.ts' + +/** Targeted branch coverage for the CC bridge: option arms, warn paths, no-agent + * fallbacks, contextFrom-empty, and the detached-listener catch handlers. */ + +const dirs: string[] = [] +afterEach(() => { for (const d of dirs.splice(0)) rmSync(d, { recursive: true, force: true }) }) + +function dir(): string { const d = mkdtempSync(join(tmpdir(), 'dsh-hc-cov-')); dirs.push(d); return d } +function sh(d: string, name: string, body: string): string { + const p = join(d, name); writeFileSync(p, body); chmodSync(p, 0o755); return p +} +function hooks(d: string, h: unknown): string { + writeFileSync(join(d, 'hooks.json'), JSON.stringify({ hooks: h })); return join(d, 'hooks.json') +} + +type HarnessOpts = { pluginRoot?: string; projectDir?: string } +async function harness(configPath: string, adapter: MockAdapter, opts: HarnessOpts = {}): Promise { + const ctx = new Context() + await ctx.plugin(LlmService) + await ctx.plugin(SessionStore) + await ctx.plugin(SystemPrompt) + await ctx.plugin(ToolRegistry) + await ctx.plugin(AgentRegistry) + await ctx.plugin(AgentLoop, { agents: [] }) + await ctx.plugin(LocalBashExecutor, { timeoutMs: 10_000 }) + await ctx.plugin(HooksClaude, { configPath, ...opts }) + ctx.llm.registerAdapter(['mock'], adapter) + return ctx +} +function waitForIdle(ctx: Context, agent: ReactLoopAgent): Promise { + return new Promise((resolve) => { const d = ctx.on('agent/status', (s, st) => { if (s === agent && st === 'idle') { d(); resolve() } }) }) +} +function events(agent: ReactLoopAgent): SessionEvent[] { return [...agent.session.events] } + +describe('hooks-claude coverage — config option arms + substitution + skip warning', () => { + it('honors pluginRoot + projectDir substitution and warns on a skipped non-command hook', async () => { + const d = dir() + // ${CLAUDE_PLUGIN_ROOT} resolves to d; the script writes its own cwd-independent marker. + const marker = join(d, 'ran') + sh(d, 'h.sh', `#!/usr/bin/env bash\ntouch "${marker}"\n`) + const path = hooks(d, { + PreToolUse: [{ hooks: [ + { type: 'prompt', prompt: 'skipme' }, // skipped → warn loop + { type: 'command', command: '${CLAUDE_PLUGIN_ROOT}/h.sh' }, // substituted + ] }], + }) + const warn = vi.fn() + const adapter = new MockAdapter([toolCallResponse('c1', 'echo', {}), textResponse('done')]) + const ctx = await harness(path, adapter, { pluginRoot: d, projectDir: d }) + ctx.logger.warn = warn as never + ctx.tools.register(defineTool({ name: 'echo', description: 'e', parameters: {}, async execute() { return [{ type: 'text', text: 'ok' }] } })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]) + await waitForIdle(ctx, agent) + expect(existsSync(marker)).toBe(true) // substituted command ran + }) + + it('warns and honors updatedInput as a no-op (input rewrite deferred)', async () => { + const d = dir() + const s = sh(d, 'u.sh', '#!/usr/bin/env bash\necho \'{"hookSpecificOutput":{"hookEventName":"PreToolUse","permissionDecision":"allow","updatedInput":{"command":"rewritten"}}}\'\n') + const path = hooks(d, { PreToolUse: [{ hooks: [{ type: 'command', command: s }] }] }) + const warn = vi.fn() + const adapter = new MockAdapter([toolCallResponse('c1', 'echo', { command: 'original' }), textResponse('done')]) + const ctx = await harness(path, adapter) + ctx.logger.warn = warn as never + let sawArgs: unknown + ctx.tools.register(defineTool({ name: 'echo', description: 'e', parameters: { command: { type: 'string' } }, async execute(args) { sawArgs = args; return [{ type: 'text', text: 'ok' }] } })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]) + await waitForIdle(ctx, agent) + // updatedInput is NOT honored — the tool ran with the ORIGINAL args. + expect((sawArgs as { command?: string }).command).toBe('original') + expect(warn).toHaveBeenCalledWith(expect.stringContaining('updatedInput')) + }) +}) + +describe('hooks-claude coverage — empty/no-op outcomes and no-agent paths', () => { + it('a clean exit-0 hook with no output is a no-op (contextFrom empty → next())', async () => { + const d = dir() + const s = sh(d, 'noop.sh', '#!/usr/bin/env bash\nexit 0\n') + const path = hooks(d, { UserPromptSubmit: [{ hooks: [{ type: 'command', command: s }] }] }) + const adapter = new MockAdapter([textResponse('ran')]) + const ctx = await harness(path, adapter) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]) + await waitForIdle(ctx, agent) + // The prompt proceeded unchanged; no context/message injected. + expect(adapter.requests).toHaveLength(1) + expect(events(agent).some(e => e.type === 'context/message')).toBe(false) + }) + + it('a PreToolUse hook fires for a no-agent direct tool call (no session/turn to record into)', async () => { + const d = dir() + const s = sh(d, 'deny.sh', '#!/usr/bin/env bash\necho "no" >&2\nexit 2\n') + const path = hooks(d, { PreToolUse: [{ hooks: [{ type: 'command', command: s }] }] }) + const ctx = await harness(path, new MockAdapter([])) + let ran = false + ctx.tools.register(defineTool({ name: 'echo', description: 'e', parameters: {}, async execute() { ran = true; return [{ type: 'text', text: 'x' }] } })) + // Call execute() directly with NO agent — the bridge's no-agent/no-turn path. + const { CallId } = await import('@deepseek-ai/dsh-llm') + const result = await ctx.tools.execute({ callId: CallId('c1'), name: 'echo', arguments: {} }) + expect(ran).toBe(false) + expect(result.isError).toBe(true) + }) + + it('a long stderr is truncated in the hook/result summary', async () => { + const d = dir() + // Emit >500 chars of stderr then exit 2. + const s = sh(d, 'long.sh', '#!/usr/bin/env bash\nprintf "x%.0s" {1..600} >&2\nexit 2\n') + const path = hooks(d, { PreToolUse: [{ hooks: [{ type: 'command', command: s }] }] }) + const adapter = new MockAdapter([toolCallResponse('c1', 'echo', {}), textResponse('done')]) + const ctx = await harness(path, adapter) + ctx.tools.register(defineTool({ name: 'echo', description: 'e', parameters: {}, async execute() { return [{ type: 'text', text: 'ok' }] } })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]) + await waitForIdle(ctx, agent) + const res = events(agent).find(e => e.type === 'hook/result') + expect(res?.type === 'hook/result' && res.data.stderrSummary?.endsWith('…')).toBe(true) + }) +}) + +describe('hooks-claude coverage — Stop continuation + subagent inject/catch', () => { + it('a Stop hook that blocks (exit 2) forces the turn to continue (CC dialect)', async () => { + const d = dir() + const marker = join(d, 'fired') + const s = sh(d, 'stop.sh', `#!/usr/bin/env bash\nif [ -e "${marker}" ]; then exit 0; fi\ntouch "${marker}"\necho "continue please" >&2\nexit 2\n`) + const path = hooks(d, { Stop: [{ hooks: [{ type: 'command', command: s }] }] }) + const adapter = new MockAdapter([textResponse('one'), textResponse('two')]) + const ctx = await harness(path, adapter) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]) + await waitForIdle(ctx, agent) + expect(adapter.requests).toHaveLength(2) + expect(JSON.stringify(adapter.requests[1]!.messages)).toContain('continue please') + }) + + it('SubagentStart additionalContext is injected into a REGISTERED live child', async () => { + const d = dir() + const s = sh(d, 'sa.sh', '#!/usr/bin/env bash\necho \'{"hookSpecificOutput":{"hookEventName":"SubagentStart","additionalContext":"child guidance"}}\'\n') + const path = hooks(d, { SubagentStart: [{ hooks: [{ type: 'command', command: s }] }] }) + const ctx = await harness(path, new MockAdapter([])) + // Register a fake child agent under the id the event carries. + const injected: string[] = [] + const child = { id: AgentId('child-x'), inject: (content: { type: string; text?: string }[]) => { injected.push(content.map(b => b.text ?? '').join('')) }, session: { header: { id: 'child-x' } } } as unknown as Parameters[0] + ctx.agents.register(child) + ctx.emit('subagent/start', { provider: 'p', id: AgentId('child-x'), agentType: 'r' }) + await new Promise(r => setTimeout(r, 80)) + expect(injected).toContain('child guidance') + }) + + it('a throwing SubagentStart/SubagentStop hook run is contained (logged)', async () => { + const d = dir() + // A hook command that does not exist makes runHook resolve a non-blocking + // error (not a throw), so to hit the .catch we make the .then throw: register + // a child whose inject throws for SubagentStart. + const s = sh(d, 'sa.sh', '#!/usr/bin/env bash\necho \'{"hookSpecificOutput":{"hookEventName":"SubagentStart","additionalContext":"x"}}\'\n') + const path = hooks(d, { SubagentStart: [{ hooks: [{ type: 'command', command: s }] }] }) + const ctx = await harness(path, new MockAdapter([])) + const warn = vi.fn(); ctx.logger.warn = warn as never + const child = { id: AgentId('child-y'), inject: () => { throw new Error('inject boom') }, session: { header: { id: 'child-y' } } } as unknown as Parameters[0] + ctx.agents.register(child) + ctx.emit('subagent/start', { provider: 'p', id: AgentId('child-y') }) + await new Promise(r => setTimeout(r, 80)) + expect(warn).toHaveBeenCalledWith(expect.stringContaining('SubagentStart hook failed')) + }) +}) + +describe('hooks-claude coverage — default reasons + sparse payloads', () => { + it('PreToolUse deny with EMPTY stderr uses the default reason', async () => { + const d = dir() + const s = sh(d, 'deny.sh', '#!/usr/bin/env bash\nexit 2\n') // exit 2, no stderr + const path = hooks(d, { PreToolUse: [{ hooks: [{ type: 'command', command: s }] }] }) + const adapter = new MockAdapter([toolCallResponse('c1', 'echo', {}), textResponse('done')]) + const ctx = await harness(path, adapter) + ctx.tools.register(defineTool({ name: 'echo', description: 'e', parameters: {}, async execute() { return [{ type: 'text', text: 'x' }] } })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]) + await waitForIdle(ctx, agent) + const result = events(agent).find(e => e.type === 'tool/result') + expect(result?.type === 'tool/result' && result.data.content.some(b => b.type === 'text' && b.text.includes('blocked by PreToolUse hook'))).toBe(true) + }) + + it('PostToolUse deny with EMPTY stderr + no context uses the default feedback', async () => { + const d = dir() + const s = sh(d, 'block.sh', '#!/usr/bin/env bash\nexit 2\n') + const path = hooks(d, { PostToolUse: [{ hooks: [{ type: 'command', command: s }] }] }) + const adapter = new MockAdapter([toolCallResponse('c1', 'echo', {}), textResponse('done')]) + const ctx = await harness(path, adapter) + ctx.tools.register(defineTool({ name: 'echo', description: 'e', parameters: {}, async execute() { return [{ type: 'text', text: 'ok' }] } })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]) + await waitForIdle(ctx, agent) + const result = events(agent).find(e => e.type === 'tool/result') + expect(result?.type === 'tool/result' && result.data.content.some(b => b.type === 'text' && b.text.includes('blocked by PostToolUse hook'))).toBe(true) + }) + + it('SubagentStop with no agentType + a rejecting hook run is contained', async () => { + const d = dir() + // Make the SubagentStop runPoint reject by registering a session whose append + // throws — simplest: a hook that emits invalid output is fine; force the + // .catch by making the session's append throw via a poisoned agent is hard, + // so instead assert the no-agentType payload path runs cleanly (no crash). + const marker = join(d, 'stopran') + const s = sh(d, 'stop.sh', `#!/usr/bin/env bash\ntouch "${marker}"\n`) + const path = hooks(d, { SubagentStop: [{ hooks: [{ type: 'command', command: s }] }] }) + const ctx = await harness(path, new MockAdapter([])) + ctx.emit('subagent/end', { provider: 'p', id: AgentId('child-z'), stopReason: 'completed' }) // no agentType + await new Promise(r => setTimeout(r, 80)) + expect(existsSync(marker)).toBe(true) + }) +}) + +describe('hooks-claude coverage — more default/sparse arms', () => { + it('UserPromptSubmit deny with EMPTY stderr uses the default block reason', async () => { + const d = dir() + const s = sh(d, 'block.sh', '#!/usr/bin/env bash\nexit 2\n') + const path = hooks(d, { UserPromptSubmit: [{ hooks: [{ type: 'command', command: s }] }] }) + const adapter = new MockAdapter([textResponse('no')]) + const ctx = await harness(path, adapter) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]) + await waitForIdle(ctx, agent) + const turnEnd = events(agent).findLast(e => e.type === 'turn/end') + expect(turnEnd?.type === 'turn/end' && turnEnd.data.reason.kind === 'rejected' && turnEnd.data.reason.reason).toContain('blocked by UserPromptSubmit hook') + }) + + it('a PreToolUse ask with NO reason omits the reason (false arm)', async () => { + const d = dir() + const s = sh(d, 'ask.sh', '#!/usr/bin/env bash\necho \'{"hookSpecificOutput":{"hookEventName":"PreToolUse","permissionDecision":"ask"}}\'\n') + const path = hooks(d, { PreToolUse: [{ hooks: [{ type: 'command', command: s }] }] }) + const adapter = new MockAdapter([toolCallResponse('c1', 'echo', {}), textResponse('done')]) + const ctx = await harness(path, adapter) + let ran = false + ctx.tools.register(defineTool({ name: 'echo', description: 'e', parameters: {}, async execute() { ran = true; return [{ type: 'text', text: 'x' }] } })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]) + await waitForIdle(ctx, agent) + // ask (no reason) → degrades to deny with the registry's generic message. + expect(ran).toBe(false) + expect(events(agent).some(e => e.type === 'tool/result' && e.data.isError)).toBe(true) + }) + + it('a recorded clean exit-0 hook with no stderr omits exitCode-extra/stderrSummary fields', async () => { + const d = dir() + const s = sh(d, 'noop.sh', '#!/usr/bin/env bash\nexit 0\n') + const path = hooks(d, { PreToolUse: [{ hooks: [{ type: 'command', command: s }] }] }) + const adapter = new MockAdapter([toolCallResponse('c1', 'echo', {}), textResponse('done')]) + const ctx = await harness(path, adapter) + ctx.tools.register(defineTool({ name: 'echo', description: 'e', parameters: {}, async execute() { return [{ type: 'text', text: 'ok' }] } })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]) + await waitForIdle(ctx, agent) + const res = events(agent).find(e => e.type === 'hook/result') + expect(res?.type === 'hook/result' && res.data.exitCode).toBe(0) + expect(res?.type === 'hook/result' && 'stderrSummary' in res.data).toBe(false) + }) +}) + +describe('hooks-claude coverage — schema-bypass default + unspawnable hook', () => { + it('a direct apply() (schema bypass) defaults the timeout and runs', async () => { + const d = dir() + const marker = join(d, 'ran') + const s = sh(d, 'h.sh', `#!/usr/bin/env bash\ntouch "${marker}"\n`) + hooks(d, { UserPromptSubmit: [{ hooks: [{ type: 'command', command: s }] }] }) + const adapter = new MockAdapter([textResponse('ok')]) + const ctx = new Context() + await ctx.plugin(LlmService) + await ctx.plugin(SessionStore) + await ctx.plugin(SystemPrompt) + await ctx.plugin(ToolRegistry) + await ctx.plugin(AgentRegistry) + await ctx.plugin(AgentLoop, { agents: [] }) + await ctx.plugin(LocalBashExecutor, { timeoutMs: 10_000 }) + // Direct apply with only configPath — bypasses schemastery's defaults, so the + // runtime `defaultTimeoutMs ?? 600_000` fallback is exercised. + HooksClaude.apply(ctx, { configPath: join(d, 'hooks.json') }) + await new Promise(r => setTimeout(r, 10)) + ctx.llm.registerAdapter(['mock'], adapter) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]) + await waitForIdle(ctx, agent) + expect(existsSync(marker)).toBe(true) + }) + + it('a non-zero non-2 hook exit (e.g. a command-not-found 127) is a non-blocking error; the tool still runs', async () => { + const d = dir() + // `bash -c` of a missing program exits 127 — a non-blocking error (not 0, not + // 2 → no decision), so the tool proceeds; the hook/result records exit 127. + const path = hooks(d, { PreToolUse: [{ hooks: [{ type: 'command', command: '/nonexistent/definitely/not/a/command' }] }] }) + const adapter = new MockAdapter([toolCallResponse('c1', 'echo', {}), textResponse('done')]) + const ctx = await harness(path, adapter) + let ran = false + ctx.tools.register(defineTool({ name: 'echo', description: 'e', parameters: {}, async execute() { ran = true; return [{ type: 'text', text: 'ok' }] } })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]) + await waitForIdle(ctx, agent) + expect(ran).toBe(true) + const res = events(agent).find(e => e.type === 'hook/result') + expect(res?.type === 'hook/result' && res.data.exitCode).toBe(127) + }) + + it('a PostToolUse deny with empty stderr + no context uses the default feedback (no context arm)', async () => { + const d = dir() + const s = sh(d, 'block.sh', '#!/usr/bin/env bash\nexit 2\n') + const path = hooks(d, { PostToolUse: [{ hooks: [{ type: 'command', command: s }] }] }) + const adapter = new MockAdapter([toolCallResponse('c1', 'echo', {}), textResponse('done')]) + const ctx = await harness(path, adapter) + ctx.tools.register(defineTool({ name: 'echo', description: 'e', parameters: {}, async execute() { return [{ type: 'text', text: 'ok' }] } })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]) + await waitForIdle(ctx, agent) + const result = events(agent).find(e => e.type === 'tool/result') + expect(result?.type === 'tool/result' && result.data.isError).toBe(true) + }) +}) + +describe('hooks-claude coverage — continue:false, context arm, no-cwd', () => { + it('a hook with {"continue":false} and no decision records decision "stop"', async () => { + const d = dir() + const s = sh(d, 'stop.sh', '#!/usr/bin/env bash\necho \'{"continue":false,"stopReason":"halt"}\'\n') + const path = hooks(d, { PreToolUse: [{ hooks: [{ type: 'command', command: s }] }] }) + const adapter = new MockAdapter([toolCallResponse('c1', 'echo', {}), textResponse('done')]) + const ctx = await harness(path, adapter) + ctx.tools.register(defineTool({ name: 'echo', description: 'e', parameters: {}, async execute() { return [{ type: 'text', text: 'ok' }] } })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]) + await waitForIdle(ctx, agent) + const res = events(agent).find(e => e.type === 'hook/result') + expect(res?.type === 'hook/result' && res.data.decision).toBe('stop') + }) + + it('a PostToolUse hook that BOTH blocks AND attaches additionalContext', async () => { + const d = dir() + const s = sh(d, 'b.sh', '#!/usr/bin/env bash\necho \'{"decision":"block","reason":"bad","hookSpecificOutput":{"hookEventName":"PostToolUse","additionalContext":"context too"}}\'\n') + const path = hooks(d, { PostToolUse: [{ hooks: [{ type: 'command', command: s }] }] }) + const adapter = new MockAdapter([toolCallResponse('c1', 'echo', {}), textResponse('done')]) + const ctx = await harness(path, adapter) + ctx.tools.register(defineTool({ name: 'echo', description: 'e', parameters: {}, async execute() { return [{ type: 'text', text: 'ok' }] } })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]) + await waitForIdle(ctx, agent) + const result = events(agent).find(e => e.type === 'tool/result') + expect(result?.type === 'tool/result' && result.data.isError).toBe(true) + expect(result?.type === 'tool/result' && result.data.content.some(b => b.type === 'text' && b.text.includes('bad'))).toBe(true) + // additionalContext also injected (the block + context arm). + expect(events(agent).some(e => e.type === 'context/message' && e.data.content.some(b => b.type === 'text' && b.text.includes('context too')))).toBe(true) + }) + +}) + +describe('hooks-claude coverage — executor reject + no-open-turn', () => { + it('when the bash executor REJECTS a hook run, the hook/result omits exitCode (non-blocking)', async () => { + const d = dir() + const s = sh(d, 'h.sh', '#!/usr/bin/env bash\nexit 0\n') + const path = hooks(d, { PreToolUse: [{ hooks: [{ type: 'command', command: s }] }] }) + const adapter = new MockAdapter([toolCallResponse('c1', 'echo', {}), textResponse('done')]) + const ctx = await harness(path, adapter) + // Force the executor to reject (an infrastructure fault) so runHook's catch + // yields a HookOutput with exitCode undefined → the `exitCode` spread false arm. + const bash = ctx.bash + bash.run = (() => Promise.reject(new Error('executor down'))) + ctx.tools.register(defineTool({ name: 'echo', description: 'e', parameters: {}, async execute() { return [{ type: 'text', text: 'ok' }] } })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]) + await waitForIdle(ctx, agent) + const res = events(agent).find(e => e.type === 'hook/result') + expect(res?.type === 'hook/result' && 'exitCode' in res.data).toBe(false) + }) + +}) + +describe('hooks-claude coverage — detached-listener catch handlers', () => { + it('a throwing SessionStart inject is contained (logged, agent still runs)', async () => { + const d = dir() + const s = sh(d, 'start.sh', '#!/usr/bin/env bash\necho \'{"hookSpecificOutput":{"hookEventName":"SessionStart","additionalContext":"x"}}\'\n') + const path = hooks(d, { SessionStart: [{ hooks: [{ type: 'command', command: s }] }] }) + const adapter = new MockAdapter([textResponse('ok')]) + const ctx = await harness(path, adapter) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + // Make inject throw, forcing the SessionStart .catch path. + const original = agent.inject.bind(agent) + let threw = false + agent.inject = (() => { threw = true; throw new Error('inject boom') }) + await new Promise(r => setTimeout(r, 80)) + expect(threw).toBe(true) + agent.inject = original + agent.send([{ type: 'text', text: 'go' }]) + await waitForIdle(ctx, agent) + expect(adapter.requests).toHaveLength(1) // loop survived the thrown inject + }) +}) diff --git a/packages/hooks/hooks-claude/tsconfig.json b/packages/hooks/hooks-claude/tsconfig.json new file mode 100644 index 0000000000..909db9b5c3 --- /dev/null +++ b/packages/hooks/hooks-claude/tsconfig.json @@ -0,0 +1,42 @@ +{ + "extends": "../../../tsconfig.base.json", + "compilerOptions": { + "rootDir": "src", + "outDir": "lib/types" + }, + "include": [ + "src" + ], + "references": [ + { + "path": "../../../vendor/cosmokit" + }, + { + "path": "../../../vendor/cordis" + }, + { + "path": "../../../vendor/schemastery" + }, + { + "path": "../hook-protocol" + }, + { + "path": "../../core/agent" + }, + { + "path": "../../core/tools" + }, + { + "path": "../../core/session" + }, + { + "path": "../../subagent/subagent" + }, + { + "path": "../../llm/llm" + }, + { + "path": "../../bash/bash" + } + ] +} diff --git a/packages/hooks/hooks-codex/README.md b/packages/hooks/hooks-codex/README.md new file mode 100644 index 0000000000..39d4fcd5ca --- /dev/null +++ b/packages/hooks/hooks-codex/README.md @@ -0,0 +1,54 @@ +# @deepseek-ai/dsh-hooks-codex + +A cordis plugin that runs a user's existing **Codex** `hooks.json` on the harness's canonical interception seams. The **Codex dialect** half of the hooks subsystem. The dialect-agnostic primitives come from [`@deepseek-ai/dsh-hook-protocol`](../hook-protocol/README.md); this bridge owns the Codex-specific payloads, matcher mode, and decision mapping. + +Codex's hook protocol is a deliberate **subset** of Claude Code's (same `hooks.json` shape): + +- **Five hook points only:** `PreToolUse`, `PostToolUse`, `SessionStart`, `UserPromptSubmit`, `Stop` — no subagent / notification / compaction hooks. +- **Regex-only matchers** (no literal fast path; the matcher is always an unanchored regex). +- **snake_case stdin payloads** with `turn_id`/`model` extras, written **without** a trailing newline. +- **No env vars and no command substitution** (a literal `${…}` in a command survives verbatim). +- **A block-only decision model** — `allow`/`ask` are not honored; a hook can only block, never pre-approve. + +A native cordis plugin could do everything this bridge does, more powerfully; the bridge exists only to run UNMODIFIED external Codex hooks faithfully (see [the interception-seams RFC](../../../docs/rfc/implemented/feature/2026-06-30-interception-seams.md)). + +## Config + +```ts +import type { Config } from '@deepseek-ai/dsh-hooks-codex' +const config: Config = { + configPath: '/path/to/.codex/hooks.json', // required + model: 'deepseek-v4', // optional: stamped on every payload (Codex includes `model`) + defaultTimeoutMs: 600_000, // optional: per-hook timeout when a hook sets none +} +``` + +In a `cordis.yml`: + +```yaml +- dsh-hooks-codex: + configPath: ./.codex/hooks.json + model: deepseek-v4 +``` + +The config is parsed **once** at load; a read/parse failure is contained (logs + registers nothing). Only sync `type: 'command'` hooks run — a non-command or `async: true` hook is parsed-and-skipped with a warning. A hook accepts `timeout` or the `timeoutSec` alias. Events outside the five Codex points are dropped at parse. + +## Hook points → seam Decisions + +| Codex hook | Harness seam | Mapping | +|---|---|---| +| `SessionStart` | `agent/session-start` (emit) | a plain-stdout hook's output → additionalContext → `agent.inject()` | +| `UserPromptSubmit` | `agent/prompt-submit` (waterfall) | `block` (exit 2) → `PromptDecision.block`; additionalContext → `allow` with context | +| `PreToolUse` | `tools/pre-execute` (waterfall) | `block` → `PreToolDecision.deny` (no `allow`/`ask`) | +| `PostToolUse` | `tools/post-execute` (waterfall) | `block` → `block` with feedback; additionalContext → `accept` with context | +| `Stop` | `agent/turn-continuation` (waterfall) | a blocking Stop hook forces `continue` with the reason as next-step steering | + +Codex hardcodes a tool call's `tool_name` to `"Bash"` and `tool_input` to `{ command }` (extracted from the call's arguments, or `''` when absent). The matcher subject is the tool name (`PreToolUse`/`PostToolUse`) or the session source (`SessionStart`); `UserPromptSubmit`/`Stop` ignore matchers. + +## Context source + +Injected context carries an explicit `{ kind: 'plugin', plugin: 'hooks-codex' }` source (`agent.inject()` would otherwise default it to `{ kind: 'user' }`). + +## Deferred + +**Stop loop-guard** (`TODO(stop-loop-guard)`): as in CC, a Stop hook that unconditionally blocks would force-continue every step (`stop_hook_active` is always `false` here); the loop-guard is deferred. A hook author must self-limit until it lands. diff --git a/packages/hooks/hooks-codex/package.json b/packages/hooks/hooks-codex/package.json new file mode 100644 index 0000000000..f26b57fe11 --- /dev/null +++ b/packages/hooks/hooks-codex/package.json @@ -0,0 +1,47 @@ +{ + "name": "@deepseek-ai/dsh-hooks-codex", + "description": "Bridge plugin: run a Codex hooks.json hook config on the DeepSeek Harness interception seams", + "version": "0.0.1", + "private": true, + "type": "module", + "main": "lib/index.js", + "types": "lib/types/index.d.ts", + "exports": { + ".": { + "types": "./lib/types/index.d.ts", + "default": "./lib/index.js" + }, + "./src/*": "./src/*", + "./package.json": "./package.json" + }, + "files": [ + "lib/index.js", + "lib/types/**/*.d.ts", + "lib/types/**/*.d.ts.map", + "src" + ], + "license": "BSD-3-Clause", + "dependencies": { + "schemastery": "^3.18.0" + }, + "peerDependencies": { + "@deepseek-ai/dsh-agent": "^0.0.1", + "@deepseek-ai/dsh-hook-protocol": "^0.0.1", + "@deepseek-ai/dsh-llm": "^0.0.1", + "@deepseek-ai/dsh-session": "^0.0.1", + "@deepseek-ai/dsh-tools": "^0.0.1", + "cordis": "^4.0.0-rc.6" + }, + "devDependencies": { + "@deepseek-ai/dsh-agent": "workspace:^", + "@deepseek-ai/dsh-agent-loop": "workspace:^", + "@deepseek-ai/dsh-bash": "workspace:^", + "@deepseek-ai/dsh-bash-local": "workspace:^", + "@deepseek-ai/dsh-hook-protocol": "workspace:^", + "@deepseek-ai/dsh-llm": "workspace:^", + "@deepseek-ai/dsh-session": "workspace:^", + "@deepseek-ai/dsh-system-prompt": "workspace:^", + "@deepseek-ai/dsh-tools": "workspace:^", + "cordis": "^4.0.0-rc.6" + } +} diff --git a/packages/hooks/hooks-codex/src/config.ts b/packages/hooks/hooks-codex/src/config.ts new file mode 100644 index 0000000000..411f058eea --- /dev/null +++ b/packages/hooks/hooks-codex/src/config.ts @@ -0,0 +1,79 @@ +/** + * Parse a Codex `hooks.json` into the shared {@link MatcherGroup} shape. Codex's + * config format is a SUBSET of Claude Code's: the same event-name → matcher-group + * structure and the same `{ type: 'command', command, timeout?/timeoutSec? }` + * hook shape, but only five events and NO command-string substitution (Codex sets + * no hook env vars and does not expand `${…}`). Non-command hooks (and Codex's + * `async: true` commands) are parsed-and-skipped with a warning. + * + * @module @deepseek-ai/dsh-hooks-codex/config + */ + +import type { MatcherGroup } from '@deepseek-ai/dsh-hook-protocol' + +/** The five hook points Codex's engine supports. */ +export const CODEX_EVENTS = ['PreToolUse', 'PostToolUse', 'SessionStart', 'UserPromptSubmit', 'Stop'] as const + +/** A parsed Codex config: event name → its matcher groups (command hooks only). */ +export type CodexHookConfig = Record + +/** A skipped non-command (or async) hook, surfaced so the bridge can warn. */ +export interface SkippedHook { + event: string + reason: string +} + +/** The outcome of parsing one Codex config file. */ +export interface ParsedCodexConfig { + config: CodexHookConfig + skipped: SkippedHook[] +} + +function asObject(value: unknown): Record | undefined { + return typeof value === 'object' && value !== null && !Array.isArray(value) + ? value as Record + : undefined +} + +/** + * Parse a raw Codex `hooks.json` object into runnable {@link MatcherGroup}s. + * Only the five {@link CODEX_EVENTS} are honored; an unknown event is dropped. + * `type !== 'command'` and `async: true` command hooks are skipped (recorded in + * `skipped`). Malformed entries are ignored rather than thrown — a bad config + * must not crash boot. No command substitution (Codex does none). + */ +export function parseCodexConfig(raw: unknown): ParsedCodexConfig { + const config: CodexHookConfig = {} + const skipped: SkippedHook[] = [] + const root = asObject(raw) + const hooksMap = root ? asObject(root.hooks) ?? root : undefined + if (!hooksMap) return { config, skipped } + + for (const event of CODEX_EVENTS) { + const rawGroups = hooksMap[event] + if (!Array.isArray(rawGroups)) continue + const groups: MatcherGroup[] = [] + for (const rawGroup of rawGroups) { + const group = asObject(rawGroup) + if (!group || !Array.isArray(group.hooks)) continue + const commands: MatcherGroup['hooks'] = [] + for (const rawHook of group.hooks) { + const hook = asObject(rawHook) + if (!hook) continue + const type = typeof hook.type === 'string' ? hook.type : 'command' + if (type !== 'command') { skipped.push({ event, reason: `unsupported "${type}" hook` }); continue } + if (hook.async === true) { skipped.push({ event, reason: 'async hook' }); continue } + if (typeof hook.command !== 'string') continue + // Codex accepts `timeout` or the `timeoutSec` alias. + const timeout = typeof hook.timeout === 'number' ? hook.timeout + : typeof hook.timeoutSec === 'number' ? hook.timeoutSec : undefined + commands.push({ command: hook.command, ...timeout !== undefined ? { timeoutSec: timeout } : {} }) + } + if (commands.length === 0) continue + groups.push({ ...typeof group.matcher === 'string' ? { matcher: group.matcher } : {}, hooks: commands }) + } + if (groups.length > 0) config[event] = groups + } + + return { config, skipped } +} diff --git a/packages/hooks/hooks-codex/src/index.ts b/packages/hooks/hooks-codex/src/index.ts new file mode 100644 index 0000000000..127511389d --- /dev/null +++ b/packages/hooks/hooks-codex/src/index.ts @@ -0,0 +1,235 @@ +/** + * `dsh-hooks-codex` — a bridge plugin that runs a user's existing Codex + * `hooks.json` on the harness's canonical interception seams. The CODEX DIALECT + * half of the hooks subsystem. + * + * Codex's hook protocol is a deliberate SUBSET of Claude Code's: five hook points + * (`PreToolUse`, `PostToolUse`, `SessionStart`, `UserPromptSubmit`, `Stop` — no + * subagent/notification/compaction), regex-only matchers, snake_case stdin + * payloads with `turn_id`/`model` extras and NO trailing newline, no env vars and + * no command substitution, and a block-only decision model (allow/ask are not + * honored — a hook can only block, never pre-approve). The dialect-agnostic + * primitives come from `@deepseek-ai/dsh-hook-protocol`; this bridge owns the + * Codex-specific payloads + matcher mode + decision mapping. + * + * @module @deepseek-ai/dsh-hooks-codex + */ + +import { readFileSync } from 'node:fs' +import type { Context } from 'cordis' +import z from 'schemastery' +import type { Agent, ContinuationDecision, HookContext, PromptDecision } from '@deepseek-ai/dsh-agent' +import type { ContentBlock, MessageSource } from '@deepseek-ai/dsh-llm' +import type { PostToolDecision, PreToolDecision, ToolExecution, ToolExecutionResult } from '@deepseek-ai/dsh-tools' +import { + appendHookInvoked, + appendHookResult, + matchesMatcher, + mergeHookOutputs, + runHook, + type HookOutput, + type MatcherGroup, + type MergedHookOutcome, +} from '@deepseek-ai/dsh-hook-protocol' +import { parseCodexConfig, type CodexHookConfig } from './config.ts' + +export const name = 'hooks-codex' +export const inject = ['bash'] + +/** Plugin config: where the Codex hooks.json lives + the model name for payloads. */ +export interface Config { + /** Path to a Codex `hooks.json`. */ + configPath: string + /** The model name stamped on every payload (Codex includes `model` on each event). */ + model?: string + /** Default per-hook timeout in ms when a hook sets none (Codex default: 600000). */ + defaultTimeoutMs?: number +} + +export const Config: z = z.object({ + configPath: z.string().required(), + model: z.string().default(''), + defaultTimeoutMs: z.number().default(600_000), +}) + +let handlerCounter = 0 +function nextHandlerId(point: string): string { + return `codex:${point}:${++handlerCounter}` +} + +const PLUGIN_SOURCE: MessageSource = { kind: 'plugin', plugin: 'hooks-codex' } + +function summarize(stderr: string): string | undefined { + const t = stderr.trim() + if (t.length === 0) return undefined + return t.length > 500 ? t.slice(0, 500) + '…' : t +} + +export function apply(ctx: Context, config: Config): void { + let parsed: CodexHookConfig = {} + try { + const raw: unknown = JSON.parse(readFileSync(config.configPath, 'utf8')) + const result = parseCodexConfig(raw) + parsed = result.config + for (const s of result.skipped) { + ctx.logger.warn(`hooks-codex: skipping ${s.reason} on ${s.event} (only sync command hooks run)`) + } + } catch (error: unknown) { + ctx.logger.warn(`hooks-codex: could not load hook config "${config.configPath}": ${String(error)} — no hooks registered`) + return + } + + const defaultTimeoutMs = config.defaultTimeoutMs ?? 600_000 + const model = config.model ?? '' + + async function runPoint( + point: string, + matchQuery: string, + payload: unknown, + opts: { agent?: Agent; turn?: number; signal?: AbortSignal }, + ): Promise { + const groups: MatcherGroup[] = parsed[point] ?? [] + const outputs: HookOutput[] = [] + for (const group of groups) { + // Codex matches with PURE regex (no literal fast path). + if (!matchesMatcher(group.matcher, matchQuery, 'codex')) continue + for (const hook of group.hooks) { + const handlerId = nextHandlerId(point) + const session = opts.agent?.session + if (session && opts.turn !== undefined) { + appendHookInvoked(session, { + turn: opts.turn, point, dialect: 'codex', handlerId, + ...group.matcher !== undefined ? { matcher: group.matcher } : {}, + }) + } + const { output, durationMs } = await runHook(ctx.bash, hook, { + payload, + ...opts.signal ? { signal: opts.signal } : {}, + defaultTimeoutMs, + trailingNewline: false, // Codex writes stdin WITHOUT a trailing newline. + }, () => performance.now()) + outputs.push(output) + if (session && opts.turn !== undefined) { + const stderrSummary = summarize(output.stderr) + appendHookResult(session, { + turn: opts.turn, point, handlerId, + decision: output.decision ?? (output.continue === false ? 'stop' : 'pass'), + ...output.exitCode !== undefined ? { exitCode: output.exitCode } : {}, + ...stderrSummary !== undefined ? { stderrSummary } : {}, + durationMs, + }) + } + } + } + return mergeHookOutputs(outputs) + } + + function contextFrom(merged: MergedHookOutcome): HookContext | undefined { + if (merged.additionalContext.length === 0) return undefined + const content: ContentBlock[] = merged.additionalContext.map(text => ({ type: 'text', text })) + return { content, source: PLUGIN_SOURCE } + } + + // SessionStart: emit. Codex passes a plain-stdout hook's output as additionalContext. + ctx.on('agent/session-start', (agent, source) => { + void runPoint('SessionStart', source, { ...base(agent, 'SessionStart', model), source }, { agent }) + .then((merged) => { + const context = contextFrom(merged) + if (context) agent.inject(context.content, { source: context.source }) + }) + .catch((error: unknown) => { ctx.logger.warn(`hooks-codex: SessionStart hook failed: ${String(error)}`) }) + }) + + // UserPromptSubmit → PromptDecision. Codex can only BLOCK (no allow/ask). + ctx.on('agent/prompt-submit', async (agent, content, _source, next): Promise => { + const turn = lastTurn(agent) + const merged = await runPoint('UserPromptSubmit', '', { ...turnBase(agent, 'UserPromptSubmit', model), prompt: blocksToText(content) }, { agent, turn }) + if (merged.decision === 'deny') return { kind: 'block', reason: merged.reason ?? 'blocked by UserPromptSubmit hook' } + const context = contextFrom(merged) + if (context) return { kind: 'allow', additionalContext: context } + return next() + }) + + // PreToolUse → PreToolDecision. Codex blocks only (no allow/ask honored). + ctx.on('tools/pre-execute', async (exec, next): Promise => { + const turn = lastTurn(exec.agent) + const merged = await runPoint('PreToolUse', exec.name, preToolPayload(exec, model), { ...exec.agent ? { agent: exec.agent } : {}, turn, ...exec.signal ? { signal: exec.signal } : {} }) + if (merged.decision === 'deny') return { kind: 'deny', reason: merged.reason ?? 'blocked by PreToolUse hook' } + return next() + }) + + // PostToolUse → PostToolDecision (block with feedback, or attach context). + ctx.on('tools/post-execute', async (exec, result, next): Promise => { + const turn = lastTurn(exec.agent) + const merged = await runPoint('PostToolUse', exec.name, postToolPayload(exec, result, model), { ...exec.agent ? { agent: exec.agent } : {}, turn, ...exec.signal ? { signal: exec.signal } : {} }) + const context = contextFrom(merged) + if (merged.decision === 'deny') { + return { kind: 'block', feedback: [{ type: 'text', text: merged.reason ?? 'blocked by PostToolUse hook' }], ...context ? { additionalContext: context } : {} } + } + if (context) return { kind: 'accept', additionalContext: context } + return next() + }) + + // Stop → ContinuationDecision. A blocking Stop hook forces continuation. + // TODO(stop-loop-guard): like CC, a Stop hook that unconditionally blocks would + // force-continue every step (`stop_hook_active` is always false here); the + // loop-guard (stop_hook_active + a max-consecutive cap) is deferred. + ctx.on('agent/turn-continuation', async (agent, turn, _default, next): Promise => { + const merged = await runPoint('Stop', '', { ...turnBase(agent, 'Stop', model), stop_hook_active: false, last_assistant_message: null }, { agent, turn }) + if (merged.decision === 'deny' && merged.reason !== undefined) { + return { action: 'continue', reason: { content: [{ type: 'text', text: merged.reason }], source: PLUGIN_SOURCE } } + } + return next() + }) +} + +// --- Codex DIALECT payloads: snake_case, model on every event, turn_id on +// turn-scoped events. --- + +function lastTurn(agent: Agent | undefined): number { + if (!agent) return 0 + const last = [...agent.session.events].findLast(e => e.type === 'turn/start') + /* v8 ignore next -- the `: 0` arm is a defensive fallback: when an agent is + present, lastTurn is only called from the mid-turn seams, which always run + inside an open turn, so `last` is always a turn/start here. */ + return last?.type === 'turn/start' ? last.data.turn : 0 +} + +function blocksToText(content: ContentBlock[]): string { + return content.filter((b): b is Extract => b.type === 'text').map(b => b.text).join('') +} + +/** Base fields on every Codex payload (no turn_id). */ +function base(agent: Agent | undefined, event: string, model: string): Record { + return { + session_id: agent?.session.header.id ?? '', + transcript_path: null, + cwd: agent?.session.header.cwd ?? process.cwd(), + hook_event_name: event, + model, + permission_mode: 'default', + } +} + +/** Base + turn_id, for the turn-scoped events (PreToolUse/PostToolUse/UserPromptSubmit/Stop). */ +function turnBase(agent: Agent | undefined, event: string, model: string): Record { + return { ...base(agent, event, model), turn_id: String(lastTurn(agent)) } +} + +/** Extract a `command` string from a tool call's parsed arguments, else ''. */ +function commandOf(args: unknown): string { + if (typeof args === 'object' && args !== null && 'command' in args) { + const command: unknown = args.command + if (typeof command === 'string') return command + } + return '' +} + +function preToolPayload(exec: ToolExecution, model: string): Record { + // Codex hardcodes tool_name to "Bash" and tool_input to { command }. + return { ...turnBase(exec.agent, 'PreToolUse', model), tool_name: 'Bash', tool_input: { command: commandOf(exec.arguments) }, tool_use_id: exec.callId } +} + +function postToolPayload(exec: ToolExecution, result: ToolExecutionResult, model: string): Record { + return { ...turnBase(exec.agent, 'PostToolUse', model), tool_name: 'Bash', tool_input: { command: commandOf(exec.arguments) }, tool_use_id: exec.callId, tool_response: blocksToText(result.content) } +} diff --git a/packages/hooks/hooks-codex/tests/bridge.spec.ts b/packages/hooks/hooks-codex/tests/bridge.spec.ts new file mode 100644 index 0000000000..f62fa4d66c --- /dev/null +++ b/packages/hooks/hooks-codex/tests/bridge.spec.ts @@ -0,0 +1,167 @@ +import { afterEach, describe, expect, it } from 'vitest' +import { mkdtempSync, rmSync, writeFileSync, chmodSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { Context } from 'cordis' +import Loader from '@cordisjs/plugin-loader' +import LlmService from '@deepseek-ai/dsh-llm' +import SessionStore, { type SessionEvent } from '@deepseek-ai/dsh-session' +import SystemPrompt from '@deepseek-ai/dsh-system-prompt' +import ToolRegistry, { defineTool } from '@deepseek-ai/dsh-tools' +import AgentRegistry, { AgentId } from '@deepseek-ai/dsh-agent' +import AgentLoop, { type ReactLoopAgent } from '@deepseek-ai/dsh-agent-loop' +import { LocalBashExecutor } from '@deepseek-ai/dsh-bash-local' +import * as HooksCodex from '@deepseek-ai/dsh-hooks-codex' +import { MockAdapter, textResponse, toolCallResponse } from '../../../core/agent-loop/tests/mock-adapter.ts' + +/** + * Full-loop Codex-bridge tests: scripted mock MODEL + REAL loop + REAL bash + + * REAL `dsh-hooks-codex` running REAL shell scripts from a temp `hooks.json`. + * Codex dialect specifics exercised here: regex matcher (substring), block-only + * decisions, the five-event subset. + */ + +const dirs: string[] = [] +afterEach(() => { for (const d of dirs.splice(0)) rmSync(d, { recursive: true, force: true }) }) + +function configDir(): string { + const dir = mkdtempSync(join(tmpdir(), 'dsh-hooks-codex-')) + dirs.push(dir) + return dir +} +function script(dir: string, name: string, body: string): string { + const path = join(dir, name) + writeFileSync(path, body) + chmodSync(path, 0o755) + return path +} +function writeHooks(dir: string, hooks: unknown): void { + writeFileSync(join(dir, 'hooks.json'), JSON.stringify({ hooks })) +} + +async function harness(dir: string, adapter: MockAdapter): Promise { + const ctx = new Context() + await ctx.plugin(LlmService) + await ctx.plugin(SessionStore) + await ctx.plugin(SystemPrompt) + await ctx.plugin(ToolRegistry) + await ctx.plugin(AgentRegistry) + await ctx.plugin(AgentLoop, { agents: [] }) + await ctx.plugin(LocalBashExecutor, { timeoutMs: 10_000 }) + await ctx.plugin(HooksCodex, { configPath: join(dir, 'hooks.json'), model: 'test-model' }) + ctx.llm.registerAdapter(['mock'], adapter) + return ctx +} + +function waitForIdle(ctx: Context, agent: ReactLoopAgent): Promise { + return new Promise((resolve) => { + const dispose = ctx.on('agent/status', (subject, status) => { + if (subject === agent && status === 'idle') { dispose(); resolve() } + }) + }) +} +function events(agent: ReactLoopAgent): SessionEvent[] { return [...agent.session.events] } + +describe('hooks-codex bridge', () => { + it('a PreToolUse hook (exit 2) denies a tool the regex matcher matches as a substring', async () => { + const dir = configDir() + const deny = script(dir, 'deny.sh', '#!/usr/bin/env bash\necho "codex blocked it" >&2\nexit 2\n') + // Codex regex matcher: "Bash" is /Bash/ — matches the tool name "Bash". + writeHooks(dir, { PreToolUse: [{ matcher: 'Bash', hooks: [{ type: 'command', command: deny }] }] }) + + const adapter = new MockAdapter([toolCallResponse('c1', 'Bash', { command: 'ls' }), textResponse('done')]) + const ctx = await harness(dir, adapter) + let ran = false + ctx.tools.register(defineTool({ name: 'Bash', description: 'b', parameters: { command: { type: 'string' } }, async execute() { ran = true; return [{ type: 'text', text: 'no' }] } })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'run ls' }]) + await waitForIdle(ctx, agent) + + expect(ran).toBe(false) + const result = events(agent).find(e => e.type === 'tool/result') + expect(result?.type === 'tool/result' && result.data.isError).toBe(true) + expect(result?.type === 'tool/result' && result.data.content.some(b => b.type === 'text' && b.text.includes('codex blocked it'))).toBe(true) + // recorded under the codex dialect + expect(events(agent).some(e => e.type === 'hook/invoked' && e.data.dialect === 'codex' && e.data.point === 'PreToolUse')).toBe(true) + }) + + it('a Stop hook (exit 2) forces the turn to continue with the reason as steering', async () => { + const dir = configDir() + // Block exactly ONCE (a marker file), then allow — without a one-shot guard a + // hook that always exits 2 would force-continue forever (the deferred + // stop_hook_active loop-guard is the real fix; here we self-limit so the test + // exercises the continue path without looping). + const marker = join(dir, 'fired') + const cont = script(dir, 'cont.sh', `#!/usr/bin/env bash\nif [ -e "${marker}" ]; then exit 0; fi\ntouch "${marker}"\necho "keep going: address the goal" >&2\nexit 2\n`) + writeHooks(dir, { Stop: [{ hooks: [{ type: 'command', command: cont }] }] }) + + // Step 1 has no tool calls → would stop; the Stop hook forces step 2. + const adapter = new MockAdapter([textResponse('first answer'), textResponse('second answer after goal')]) + const ctx = await harness(dir, adapter) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]) + await waitForIdle(ctx, agent) + + // The Stop hook's reason became next-step steering → a second model request ran. + expect(adapter.requests).toHaveLength(2) + expect(JSON.stringify(adapter.requests[1]!.messages)).toContain('keep going: address the goal') + }) + + it('only the five Codex events are honored — a SubagentStop entry is ignored', async () => { + const dir = configDir() + const s = script(dir, 'x.sh', '#!/usr/bin/env bash\nexit 2\n') + // SubagentStop is NOT a Codex event; it must be dropped (no crash, no effect). + writeHooks(dir, { SubagentStop: [{ hooks: [{ type: 'command', command: s }] }] }) + + const adapter = new MockAdapter([textResponse('fine')]) + const ctx = await harness(dir, adapter) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]) + await waitForIdle(ctx, agent) + // Ran normally; the unknown event was dropped at parse. + expect(adapter.requests).toHaveLength(1) + }) + + it('a missing config registers no hooks and does not crash', async () => { + const dir = configDir() // no hooks.json written + const adapter = new MockAdapter([textResponse('ok')]) + const ctx = await harness(dir, adapter) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]) + await waitForIdle(ctx, agent) + expect(adapter.requests).toHaveLength(1) + }) + + it('disposing the bridge fiber is clean (HMR safety)', async () => { + const dir = configDir() + writeHooks(dir, { UserPromptSubmit: [{ hooks: [{ type: 'command', command: 'true' }] }] }) + const adapter = new MockAdapter([textResponse('ok')]) + const ctx = new Context() + await ctx.plugin(LlmService) + await ctx.plugin(SessionStore) + await ctx.plugin(SystemPrompt) + await ctx.plugin(ToolRegistry) + await ctx.plugin(AgentRegistry) + await ctx.plugin(AgentLoop, { agents: [] }) + await ctx.plugin(LocalBashExecutor, { timeoutMs: 10_000 }) + const fiber = await ctx.plugin(HooksCodex, { configPath: join(dir, 'hooks.json'), model: 'm' }) + await fiber.dispose() + ctx.llm.registerAdapter(['mock'], adapter) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]) + await waitForIdle(ctx, agent) + expect(adapter.requests).toHaveLength(1) + }) + + it('has the namespace-plugin export shape (no stray default) so the Loader keeps name/inject/apply', () => { + expect('default' in HooksCodex).toBe(false) + expect(HooksCodex.name).toBe('hooks-codex') + expect(HooksCodex.inject).toEqual(['bash']) + const loader = Object.create(Loader.prototype) as Loader + const unwrapped = loader.unwrapExports(HooksCodex) as Record + expect(unwrapped).toBe(HooksCodex) + expect(unwrapped.name).toBe('hooks-codex') + expect(unwrapped.inject).toEqual(['bash']) + expect(typeof unwrapped.apply).toBe('function') + }) +}) diff --git a/packages/hooks/hooks-codex/tests/config.spec.ts b/packages/hooks/hooks-codex/tests/config.spec.ts new file mode 100644 index 0000000000..e79d665931 --- /dev/null +++ b/packages/hooks/hooks-codex/tests/config.spec.ts @@ -0,0 +1,68 @@ +import { describe, expect, it } from 'vitest' +import { parseCodexConfig, CODEX_EVENTS } from '@deepseek-ai/dsh-hooks-codex/src/config.ts' + +describe('parseCodexConfig', () => { + it('honors only the five Codex events, dropping unknown ones', () => { + const { config } = parseCodexConfig({ + PreToolUse: [{ hooks: [{ type: 'command', command: 'a.sh' }] }], + SubagentStop: [{ hooks: [{ type: 'command', command: 'b.sh' }] }], // not a Codex event + Notification: [{ hooks: [{ type: 'command', command: 'c.sh' }] }], // not a Codex event + }) + expect(Object.keys(config)).toEqual(['PreToolUse']) + expect(CODEX_EVENTS).toContain('PreToolUse') + expect(CODEX_EVENTS).not.toContain('SubagentStop' as never) + }) + + it('accepts both timeout and the timeoutSec alias, no substitution', () => { + const { config } = parseCodexConfig({ + Stop: [{ hooks: [{ type: 'command', command: '${NOT_SUBSTITUTED}/s.sh', timeout: 10 }] }], + UserPromptSubmit: [{ hooks: [{ type: 'command', command: 'u.sh', timeoutSec: 20 }] }], + }) + // Codex does NO substitution — the literal ${…} survives. + expect(config.Stop).toEqual([{ hooks: [{ command: '${NOT_SUBSTITUTED}/s.sh', timeoutSec: 10 }] }]) + expect(config.UserPromptSubmit).toEqual([{ hooks: [{ command: 'u.sh', timeoutSec: 20 }] }]) + }) + + it('skips non-command and async:true hooks (recorded)', () => { + const { config, skipped } = parseCodexConfig({ + PreToolUse: [{ hooks: [ + { type: 'prompt' }, + { type: 'command', command: 'sync.sh' }, + { type: 'command', command: 'bg.sh', async: true }, + ] }], + }) + expect(config.PreToolUse).toEqual([{ hooks: [{ command: 'sync.sh' }] }]) + expect(skipped).toEqual([{ event: 'PreToolUse', reason: 'unsupported "prompt" hook' }, { event: 'PreToolUse', reason: 'async hook' }]) + }) + + it('parses the { hooks: … } wrapper and the bare map identically', () => { + const groups = { Stop: [{ hooks: [{ type: 'command', command: 's.sh' }] }] } + expect(parseCodexConfig(groups).config).toEqual(parseCodexConfig({ hooks: groups }).config) + }) + + it('drops malformed entries and a non-object top level without throwing', () => { + expect(parseCodexConfig(null).config).toEqual({}) + expect(parseCodexConfig({ PreToolUse: 'no' }).config).toEqual({}) + expect(parseCodexConfig({ Stop: [7, { hooks: 'x' }, { hooks: [{ type: 'command', command: 9 }] }] }).config).toEqual({}) + }) + + it('skips a non-object element inside a hooks array, keeping the valid sibling', () => { + const { config } = parseCodexConfig({ Stop: [{ hooks: [null, 7, { type: 'command', command: 's.sh' }] }] }) + expect(config.Stop).toEqual([{ hooks: [{ command: 's.sh' }] }]) + }) + + it('treats a hook with no `type` field as a command (the default)', () => { + const { config } = parseCodexConfig({ Stop: [{ hooks: [{ command: 's.sh' }] }] }) + expect(config.Stop).toEqual([{ hooks: [{ command: 's.sh' }] }]) + }) + + it('omits the matcher key for a match-all group', () => { + const { config } = parseCodexConfig({ Stop: [{ hooks: [{ type: 'command', command: 's.sh' }] }] }) + expect('matcher' in config.Stop![0]!).toBe(false) + }) + + it('keeps a matcher when present', () => { + const { config } = parseCodexConfig({ PreToolUse: [{ matcher: '^Bash$', hooks: [{ type: 'command', command: 'b.sh' }] }] }) + expect(config.PreToolUse![0]!.matcher).toBe('^Bash$') + }) +}) diff --git a/packages/hooks/hooks-codex/tests/coverage.spec.ts b/packages/hooks/hooks-codex/tests/coverage.spec.ts new file mode 100644 index 0000000000..732e0a7c61 --- /dev/null +++ b/packages/hooks/hooks-codex/tests/coverage.spec.ts @@ -0,0 +1,308 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { mkdtempSync, rmSync, writeFileSync, chmodSync, existsSync, readFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { Context } from 'cordis' +import LlmService from '@deepseek-ai/dsh-llm' +import SessionStore, { type SessionEvent } from '@deepseek-ai/dsh-session' +import SystemPrompt from '@deepseek-ai/dsh-system-prompt' +import ToolRegistry, { defineTool } from '@deepseek-ai/dsh-tools' +import AgentRegistry, { AgentId } from '@deepseek-ai/dsh-agent' +import AgentLoop, { type ReactLoopAgent } from '@deepseek-ai/dsh-agent-loop' +import { LocalBashExecutor } from '@deepseek-ai/dsh-bash-local' +import * as HooksCodex from '@deepseek-ai/dsh-hooks-codex' +import { MockAdapter, textResponse, toolCallResponse } from '../../../core/agent-loop/tests/mock-adapter.ts' + +const dirs: string[] = [] +afterEach(() => { for (const d of dirs.splice(0)) rmSync(d, { recursive: true, force: true }) }) +function dir(): string { const d = mkdtempSync(join(tmpdir(), 'dsh-hx-cov-')); dirs.push(d); return d } +function sh(d: string, name: string, body: string): string { + const p = join(d, name); writeFileSync(p, body); chmodSync(p, 0o755); return p +} +function hooks(d: string, h: unknown): string { + writeFileSync(join(d, 'hooks.json'), JSON.stringify({ hooks: h })); return join(d, 'hooks.json') +} + +async function harness(configPath: string, adapter: MockAdapter): Promise { + const ctx = new Context() + await ctx.plugin(LlmService); await ctx.plugin(SessionStore); await ctx.plugin(SystemPrompt) + await ctx.plugin(ToolRegistry); await ctx.plugin(AgentRegistry); await ctx.plugin(AgentLoop, { agents: [] }) + await ctx.plugin(LocalBashExecutor, { timeoutMs: 10_000 }) + await ctx.plugin(HooksCodex, { configPath, model: 'm' }) + ctx.llm.registerAdapter(['mock'], adapter) + return ctx +} +function waitForIdle(ctx: Context, agent: ReactLoopAgent): Promise { + return new Promise((resolve) => { const d = ctx.on('agent/status', (s, st) => { if (s === agent && st === 'idle') { d(); resolve() } }) }) +} +function events(agent: ReactLoopAgent): SessionEvent[] { return [...agent.session.events] } + +describe('hooks-codex coverage — decision mapping paths', () => { + it('UserPromptSubmit block (exit 2) → rejected turn; default reason on empty stderr', async () => { + const d = dir() + hooks(d, { UserPromptSubmit: [{ hooks: [{ type: 'command', command: sh(d, 'b.sh', '#!/usr/bin/env bash\nexit 2\n') }] }] }) + const adapter = new MockAdapter([textResponse('no')]) + const ctx = await harness(join(d, 'hooks.json'), adapter) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]); await waitForIdle(ctx, agent) + expect(adapter.requests).toHaveLength(0) + const te = events(agent).findLast(e => e.type === 'turn/end') + expect(te?.type === 'turn/end' && te.data.reason.kind).toBe('rejected') + }) + + it('UserPromptSubmit additionalContext is injected; a no-op hook proceeds', async () => { + const d = dir() + hooks(d, { UserPromptSubmit: [{ hooks: [{ type: 'command', command: sh(d, 'c.sh', '#!/usr/bin/env bash\necho \'{"hookSpecificOutput":{"hookEventName":"UserPromptSubmit","additionalContext":"ctx-x"}}\'\n') }] }] }) + const adapter = new MockAdapter([textResponse('ok')]) + const ctx = await harness(join(d, 'hooks.json'), adapter) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]); await waitForIdle(ctx, agent) + expect(JSON.stringify(adapter.requests[0]!.messages)).toContain('ctx-x') + }) + + it('SessionStart additionalContext is injected for the first request', async () => { + const d = dir() + hooks(d, { SessionStart: [{ hooks: [{ type: 'command', command: sh(d, 's.sh', '#!/usr/bin/env bash\necho \'{"hookSpecificOutput":{"hookEventName":"SessionStart","additionalContext":"start-ctx"}}\'\n') }] }] }) + const adapter = new MockAdapter([textResponse('ok')]) + const ctx = await harness(join(d, 'hooks.json'), adapter) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + await new Promise(r => setTimeout(r, 60)) + agent.send([{ type: 'text', text: 'go' }]); await waitForIdle(ctx, agent) + expect(JSON.stringify(adapter.requests[0]!.messages)).toContain('start-ctx') + }) + + it('PostToolUse block (exit 2) → isError feedback; default reason', async () => { + const d = dir() + hooks(d, { PostToolUse: [{ hooks: [{ type: 'command', command: sh(d, 'p.sh', '#!/usr/bin/env bash\nexit 2\n') }] }] }) + const adapter = new MockAdapter([toolCallResponse('c1', 'Bash', { command: 'ls' }), textResponse('done')]) + const ctx = await harness(join(d, 'hooks.json'), adapter) + ctx.tools.register(defineTool({ name: 'Bash', description: 'b', parameters: { command: { type: 'string' } }, async execute() { return [{ type: 'text', text: 'ok' }] } })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]); await waitForIdle(ctx, agent) + const r = events(agent).find(e => e.type === 'tool/result') + expect(r?.type === 'tool/result' && r.data.isError).toBe(true) + expect(r?.type === 'tool/result' && r.data.content.some(b => b.type === 'text' && b.text.includes('blocked by PostToolUse hook'))).toBe(true) + }) + + it('PostToolUse additionalContext (clean exit) is attached after the result', async () => { + const d = dir() + hooks(d, { PostToolUse: [{ hooks: [{ type: 'command', command: sh(d, 'pc.sh', '#!/usr/bin/env bash\necho \'{"hookSpecificOutput":{"hookEventName":"PostToolUse","additionalContext":"post-ctx"}}\'\n') }] }] }) + const adapter = new MockAdapter([toolCallResponse('c1', 'Bash', { command: 'ls' }), textResponse('done')]) + const ctx = await harness(join(d, 'hooks.json'), adapter) + ctx.tools.register(defineTool({ name: 'Bash', description: 'b', parameters: { command: { type: 'string' } }, async execute() { return [{ type: 'text', text: 'ok' }] } })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]); await waitForIdle(ctx, agent) + expect(events(agent).some(e => e.type === 'context/message' && e.data.content.some(b => b.type === 'text' && b.text.includes('post-ctx')))).toBe(true) + }) + + it('PreToolUse for a tool call WITHOUT a command arg passes an empty command (commandOf non-object/missing arm)', async () => { + const d = dir() + hooks(d, { PreToolUse: [{ hooks: [{ type: 'command', command: sh(d, 'pre.sh', '#!/usr/bin/env bash\ncat >/dev/null\nexit 0\n') }] }] }) + const adapter = new MockAdapter([toolCallResponse('c1', 'Bash', {}), textResponse('done')]) // no command arg + const ctx = await harness(join(d, 'hooks.json'), adapter) + let ran = false + ctx.tools.register(defineTool({ name: 'Bash', description: 'b', parameters: {}, async execute() { ran = true; return [{ type: 'text', text: 'ok' }] } })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]); await waitForIdle(ctx, agent) + expect(ran).toBe(true) // clean-exit hook allows; commandOf returned '' + }) + + it('a clean exit-0 hook records exitCode 0 and omits stderrSummary', async () => { + const d = dir() + hooks(d, { PreToolUse: [{ hooks: [{ type: 'command', command: sh(d, 'n.sh', '#!/usr/bin/env bash\nexit 0\n') }] }] }) + const adapter = new MockAdapter([toolCallResponse('c1', 'Bash', { command: 'x' }), textResponse('done')]) + const ctx = await harness(join(d, 'hooks.json'), adapter) + ctx.tools.register(defineTool({ name: 'Bash', description: 'b', parameters: { command: { type: 'string' } }, async execute() { return [{ type: 'text', text: 'ok' }] } })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]); await waitForIdle(ctx, agent) + const res = events(agent).find(e => e.type === 'hook/result') + expect(res?.type === 'hook/result' && res.data.exitCode).toBe(0) + expect(res?.type === 'hook/result' && 'stderrSummary' in res.data).toBe(false) + }) + + it('a long stderr is truncated in the hook/result summary', async () => { + const d = dir() + hooks(d, { PreToolUse: [{ hooks: [{ type: 'command', command: sh(d, 'l.sh', '#!/usr/bin/env bash\nprintf "x%.0s" {1..600} >&2\nexit 2\n') }] }] }) + const adapter = new MockAdapter([toolCallResponse('c1', 'Bash', { command: 'x' }), textResponse('done')]) + const ctx = await harness(join(d, 'hooks.json'), adapter) + ctx.tools.register(defineTool({ name: 'Bash', description: 'b', parameters: { command: { type: 'string' } }, async execute() { return [{ type: 'text', text: 'ok' }] } })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]); await waitForIdle(ctx, agent) + const res = events(agent).find(e => e.type === 'hook/result') + expect(res?.type === 'hook/result' && res.data.stderrSummary?.endsWith('…')).toBe(true) + }) + + it('warns on a skipped async hook and a direct apply() defaults the timeout', async () => { + const d = dir() + const marker = join(d, 'ran') + hooks(d, { UserPromptSubmit: [{ hooks: [ + { type: 'command', command: 'bg.sh', async: true }, // skipped → warn + { type: 'command', command: sh(d, 'h.sh', `#!/usr/bin/env bash\ntouch "${marker}"\n`) }, + ] }] }) + const warn = vi.fn() + const adapter = new MockAdapter([textResponse('ok')]) + const ctx = new Context() + await ctx.plugin(LlmService); await ctx.plugin(SessionStore); await ctx.plugin(SystemPrompt) + await ctx.plugin(ToolRegistry); await ctx.plugin(AgentRegistry); await ctx.plugin(AgentLoop, { agents: [] }) + await ctx.plugin(LocalBashExecutor, { timeoutMs: 10_000 }) + ctx.logger.warn = warn as never + // Direct apply (schema bypass) → defaultTimeoutMs ?? 600_000 + model ?? '' fallbacks. + HooksCodex.apply(ctx, { configPath: join(d, 'hooks.json') }) + await new Promise(r => setTimeout(r, 10)) + ctx.llm.registerAdapter(['mock'], adapter) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]); await waitForIdle(ctx, agent) + expect(existsSync(marker)).toBe(true) + expect(warn).toHaveBeenCalledWith(expect.stringContaining('async hook')) + }) + + it('a no-op clean hook proceeds (contextFrom empty → next)', async () => { + const d = dir() + hooks(d, { PreToolUse: [{ hooks: [{ type: 'command', command: sh(d, 'n.sh', '#!/usr/bin/env bash\nexit 0\n') }] }] }) + const adapter = new MockAdapter([toolCallResponse('c1', 'Bash', { command: 'x' }), textResponse('done')]) + const ctx = await harness(join(d, 'hooks.json'), adapter) + let ran = false + ctx.tools.register(defineTool({ name: 'Bash', description: 'b', parameters: { command: { type: 'string' } }, async execute() { ran = true; return [{ type: 'text', text: 'ok' }] } })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]); await waitForIdle(ctx, agent) + expect(ran).toBe(true) + }) + + it('SessionStart with no additionalContext is a no-op (contextFrom empty)', async () => { + const d = dir() + hooks(d, { SessionStart: [{ hooks: [{ type: 'command', command: sh(d, 's.sh', '#!/usr/bin/env bash\nexit 0\n') }] }] }) + const adapter = new MockAdapter([textResponse('ok')]) + const ctx = await harness(join(d, 'hooks.json'), adapter) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + await new Promise(r => setTimeout(r, 60)) + agent.send([{ type: 'text', text: 'go' }]); await waitForIdle(ctx, agent) + expect(events(agent).some(e => e.type === 'context/message')).toBe(false) + }) + + it('a throwing SessionStart inject is contained (logged)', async () => { + const d = dir() + hooks(d, { SessionStart: [{ hooks: [{ type: 'command', command: sh(d, 's.sh', '#!/usr/bin/env bash\necho \'{"hookSpecificOutput":{"hookEventName":"SessionStart","additionalContext":"x"}}\'\n') }] }] }) + const adapter = new MockAdapter([textResponse('ok')]) + const ctx = await harness(join(d, 'hooks.json'), adapter) + const warn = vi.fn(); ctx.logger.warn = warn as never + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.inject = (() => { throw new Error('inject boom') }) + await new Promise(r => setTimeout(r, 60)) + expect(warn).toHaveBeenCalledWith(expect.stringContaining('SessionStart hook failed')) + }) + + it('a clean PreToolUse with no decision allows the tool (no deny)', async () => { + const d = dir() + hooks(d, { PreToolUse: [{ hooks: [{ type: 'command', command: sh(d, 'ok.sh', '#!/usr/bin/env bash\nexit 0\n') }] }] }) + const adapter = new MockAdapter([toolCallResponse('c1', 'Bash', { command: 'x' }), textResponse('done')]) + const ctx = await harness(join(d, 'hooks.json'), adapter) + let ran = false + ctx.tools.register(defineTool({ name: 'Bash', description: 'b', parameters: { command: { type: 'string' } }, async execute() { ran = true; return [{ type: 'text', text: 'ok' }] } })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]); await waitForIdle(ctx, agent) + expect(ran).toBe(true) + }) + + it('a non-matching regex matcher skips the hook (matchesMatcher false → continue)', async () => { + const d = dir() + // /^Edit$/ does not match the tool name "Bash" → the group is skipped. + hooks(d, { PreToolUse: [{ matcher: '^Edit$', hooks: [{ type: 'command', command: sh(d, 'deny.sh', '#!/usr/bin/env bash\nexit 2\n') }] }] }) + const adapter = new MockAdapter([toolCallResponse('c1', 'Bash', { command: 'x' }), textResponse('done')]) + const ctx = await harness(join(d, 'hooks.json'), adapter) + let ran = false + ctx.tools.register(defineTool({ name: 'Bash', description: 'b', parameters: { command: { type: 'string' } }, async execute() { ran = true; return [{ type: 'text', text: 'ok' }] } })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]); await waitForIdle(ctx, agent) + expect(ran).toBe(true) // matcher didn't match → no hook ran → tool proceeded + expect(events(agent).some(e => e.type === 'hook/invoked')).toBe(false) + }) + + it('a {"continue":false} hook with no decision records decision "stop"', async () => { + const d = dir() + hooks(d, { PreToolUse: [{ hooks: [{ type: 'command', command: sh(d, 's.sh', '#!/usr/bin/env bash\necho \'{"continue":false,"stopReason":"halt"}\'\n') }] }] }) + const adapter = new MockAdapter([toolCallResponse('c1', 'Bash', { command: 'x' }), textResponse('done')]) + const ctx = await harness(join(d, 'hooks.json'), adapter) + ctx.tools.register(defineTool({ name: 'Bash', description: 'b', parameters: { command: { type: 'string' } }, async execute() { return [{ type: 'text', text: 'ok' }] } })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]); await waitForIdle(ctx, agent) + const res = events(agent).find(e => e.type === 'hook/result') + expect(res?.type === 'hook/result' && res.data.decision).toBe('stop') + }) + + it('PreToolUse deny with EMPTY stderr uses the default reason (?? right arm)', async () => { + const d = dir() + hooks(d, { PreToolUse: [{ hooks: [{ type: 'command', command: sh(d, 'd.sh', '#!/usr/bin/env bash\nexit 2\n') }] }] }) + const adapter = new MockAdapter([toolCallResponse('c1', 'Bash', { command: 'x' }), textResponse('done')]) + const ctx = await harness(join(d, 'hooks.json'), adapter) + ctx.tools.register(defineTool({ name: 'Bash', description: 'b', parameters: { command: { type: 'string' } }, async execute() { return [{ type: 'text', text: 'ok' }] } })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]); await waitForIdle(ctx, agent) + const r = events(agent).find(e => e.type === 'tool/result') + expect(r?.type === 'tool/result' && r.data.content.some(b => b.type === 'text' && b.text.includes('blocked by PreToolUse hook'))).toBe(true) + }) + + it('PostToolUse block AND additionalContext are surfaced together', async () => { + const d = dir() + hooks(d, { PostToolUse: [{ hooks: [{ type: 'command', command: sh(d, 'bc.sh', '#!/usr/bin/env bash\necho \'{"decision":"block","reason":"bad","hookSpecificOutput":{"hookEventName":"PostToolUse","additionalContext":"ctx too"}}\'\n') }] }] }) + const adapter = new MockAdapter([toolCallResponse('c1', 'Bash', { command: 'x' }), textResponse('done')]) + const ctx = await harness(join(d, 'hooks.json'), adapter) + ctx.tools.register(defineTool({ name: 'Bash', description: 'b', parameters: { command: { type: 'string' } }, async execute() { return [{ type: 'text', text: 'ok' }] } })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]); await waitForIdle(ctx, agent) + const r = events(agent).find(e => e.type === 'tool/result') + expect(r?.type === 'tool/result' && r.data.isError).toBe(true) + expect(r?.type === 'tool/result' && r.data.content.some(b => b.type === 'text' && b.text.includes('bad'))).toBe(true) + expect(events(agent).some(e => e.type === 'context/message' && e.data.content.some(b => b.type === 'text' && b.text.includes('ctx too')))).toBe(true) + }) + + it('commandOf reads a non-string command arg as an empty command', async () => { + const d = dir() + // The tool-call arguments carry `command` as a NUMBER → commandOf's + // `typeof command === 'string'` false arm → '' (the payload's tool_input.command). + const cap = join(d, 'payload') + hooks(d, { PreToolUse: [{ hooks: [{ type: 'command', command: sh(d, 'cap.sh', `#!/usr/bin/env bash\ncat > "${cap}"\nexit 0\n`) }] }] }) + const adapter = new MockAdapter([toolCallResponse('c1', 'Bash', { command: 7 }), textResponse('done')]) + const ctx = await harness(join(d, 'hooks.json'), adapter) + ctx.tools.register(defineTool({ name: 'Bash', description: 'b', parameters: { command: { type: 'number' } }, async execute() { return [{ type: 'text', text: 'ok' }] } })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]); await waitForIdle(ctx, agent) + const payload = JSON.parse(readFileSync(cap, 'utf8')) as { tool_input: { command: string } } + expect(payload.tool_input.command).toBe('') + }) + + it('a no-agent direct PreToolUse run uses process.cwd() and turn 0 (no session to record)', async () => { + const d = dir() + hooks(d, { PreToolUse: [{ hooks: [{ type: 'command', command: sh(d, 'd.sh', '#!/usr/bin/env bash\nexit 2\n') }] }] }) + const ctx = await harness(join(d, 'hooks.json'), new MockAdapter([])) + let ran = false + ctx.tools.register(defineTool({ name: 'Bash', description: 'b', parameters: { command: { type: 'string' } }, async execute() { ran = true; return [{ type: 'text', text: 'x' }] } })) + const { CallId } = await import('@deepseek-ai/dsh-llm') + const result = await ctx.tools.execute({ callId: CallId('c1'), name: 'Bash', arguments: { command: 'x' } }) + expect(ran).toBe(false) // denied + expect(result.isError).toBe(true) + }) + + it('a no-agent direct PostToolUse run attaches context with no session to record', async () => { + const d = dir() + hooks(d, { PostToolUse: [{ hooks: [{ type: 'command', command: sh(d, 'pc.sh', '#!/usr/bin/env bash\necho \'{"hookSpecificOutput":{"hookEventName":"PostToolUse","additionalContext":"x"}}\'\n') }] }] }) + const ctx = await harness(join(d, 'hooks.json'), new MockAdapter([])) + ctx.tools.register(defineTool({ name: 'Bash', description: 'b', parameters: { command: { type: 'string' } }, async execute() { return [{ type: 'text', text: 'ok' }] } })) + const { CallId } = await import('@deepseek-ai/dsh-llm') + const result = await ctx.tools.execute({ callId: CallId('c1'), name: 'Bash', arguments: { command: 'x' } }) + expect(result.isError).toBeFalsy() + expect(result.additionalContext?.content.some(b => b.type === 'text' && b.text === 'x')).toBe(true) + }) + + it('when the bash executor REJECTS, the hook/result omits exitCode (non-blocking)', async () => { + const d = dir() + hooks(d, { PreToolUse: [{ hooks: [{ type: 'command', command: sh(d, 'h.sh', '#!/usr/bin/env bash\nexit 0\n') }] }] }) + const adapter = new MockAdapter([toolCallResponse('c1', 'Bash', { command: 'x' }), textResponse('done')]) + const ctx = await harness(join(d, 'hooks.json'), adapter) + ctx.bash.run = (() => Promise.reject(new Error('executor down'))) + ctx.tools.register(defineTool({ name: 'Bash', description: 'b', parameters: { command: { type: 'string' } }, async execute() { return [{ type: 'text', text: 'ok' }] } })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]); await waitForIdle(ctx, agent) + const res = events(agent).find(e => e.type === 'hook/result') + expect(res?.type === 'hook/result' && 'exitCode' in res.data).toBe(false) + }) +}) diff --git a/packages/hooks/hooks-codex/tsconfig.json b/packages/hooks/hooks-codex/tsconfig.json new file mode 100644 index 0000000000..f936b500aa --- /dev/null +++ b/packages/hooks/hooks-codex/tsconfig.json @@ -0,0 +1,39 @@ +{ + "extends": "../../../tsconfig.base.json", + "compilerOptions": { + "rootDir": "src", + "outDir": "lib/types" + }, + "include": [ + "src" + ], + "references": [ + { + "path": "../../../vendor/cosmokit" + }, + { + "path": "../../../vendor/cordis" + }, + { + "path": "../../../vendor/schemastery" + }, + { + "path": "../hook-protocol" + }, + { + "path": "../../core/agent" + }, + { + "path": "../../core/tools" + }, + { + "path": "../../core/session" + }, + { + "path": "../../llm/llm" + }, + { + "path": "../../bash/bash" + } + ] +} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index b5ca00356a..9eaa7372b7 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -260,6 +260,83 @@ importers: specifier: ^4.0.0-rc.6 version: 4.0.0-rc.6(@cordisjs/plugin-include@1.0.4)(@cordisjs/plugin-loader@1.0.0-rc.4) + packages/hooks/hooks-claude: + dependencies: + schemastery: + specifier: ^3.18.0 + version: 3.18.0 + devDependencies: + '@deepseek-ai/dsh-agent': + specifier: workspace:^ + version: link:../../core/agent + '@deepseek-ai/dsh-agent-loop': + specifier: workspace:^ + version: link:../../core/agent-loop + '@deepseek-ai/dsh-bash': + specifier: workspace:^ + version: link:../../bash/bash + '@deepseek-ai/dsh-bash-local': + specifier: workspace:^ + version: link:../../bash/bash-local + '@deepseek-ai/dsh-hook-protocol': + specifier: workspace:^ + version: link:../hook-protocol + '@deepseek-ai/dsh-llm': + specifier: workspace:^ + version: link:../../llm/llm + '@deepseek-ai/dsh-session': + specifier: workspace:^ + version: link:../../core/session + '@deepseek-ai/dsh-subagent': + specifier: workspace:^ + version: link:../../subagent/subagent + '@deepseek-ai/dsh-system-prompt': + specifier: workspace:^ + version: link:../../core/system-prompt + '@deepseek-ai/dsh-tools': + specifier: workspace:^ + version: link:../../core/tools + cordis: + specifier: ^4.0.0-rc.6 + version: 4.0.0-rc.6(@cordisjs/plugin-include@1.0.4)(@cordisjs/plugin-loader@1.0.0-rc.4) + + packages/hooks/hooks-codex: + dependencies: + schemastery: + specifier: ^3.18.0 + version: 3.18.0 + devDependencies: + '@deepseek-ai/dsh-agent': + specifier: workspace:^ + version: link:../../core/agent + '@deepseek-ai/dsh-agent-loop': + specifier: workspace:^ + version: link:../../core/agent-loop + '@deepseek-ai/dsh-bash': + specifier: workspace:^ + version: link:../../bash/bash + '@deepseek-ai/dsh-bash-local': + specifier: workspace:^ + version: link:../../bash/bash-local + '@deepseek-ai/dsh-hook-protocol': + specifier: workspace:^ + version: link:../hook-protocol + '@deepseek-ai/dsh-llm': + specifier: workspace:^ + version: link:../../llm/llm + '@deepseek-ai/dsh-session': + specifier: workspace:^ + version: link:../../core/session + '@deepseek-ai/dsh-system-prompt': + specifier: workspace:^ + version: link:../../core/system-prompt + '@deepseek-ai/dsh-tools': + specifier: workspace:^ + version: link:../../core/tools + cordis: + specifier: ^4.0.0-rc.6 + version: 4.0.0-rc.6(@cordisjs/plugin-include@1.0.4)(@cordisjs/plugin-loader@1.0.0-rc.4) + packages/llm/llm: devDependencies: '@deepseek-ai/dsh-brand': diff --git a/tsconfig.build.json b/tsconfig.build.json index cf468b4842..4f312fdeed 100644 --- a/tsconfig.build.json +++ b/tsconfig.build.json @@ -41,6 +41,8 @@ { "path": "./packages/subagent/subagent-fork" }, { "path": "./packages/subagent/subagent-acp" }, { "path": "./packages/todo/tool-todo" }, - { "path": "./packages/hooks/hook-protocol" } + { "path": "./packages/hooks/hook-protocol" }, + { "path": "./packages/hooks/hooks-claude" }, + { "path": "./packages/hooks/hooks-codex" } ] } diff --git a/tsconfig.json b/tsconfig.json index 8f2b2c8fb1..456a4fd31a 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -52,6 +52,8 @@ { "path": "./packages/subagent/subagent-fork" }, { "path": "./packages/subagent/subagent-acp" }, { "path": "./packages/todo/tool-todo" }, - { "path": "./packages/hooks/hook-protocol" } + { "path": "./packages/hooks/hook-protocol" }, + { "path": "./packages/hooks/hooks-claude" }, + { "path": "./packages/hooks/hooks-codex" } ] } From 8870da431357aea4195805ad2d9ebee2ae7953a1 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Wed, 1 Jul 2026 10:48:23 +0800 Subject: [PATCH 02/10] =?UTF-8?q?fix(hooks):=20address=20Codex=20review=20?= =?UTF-8?q?=E2=80=94=20Stop=20force-continue,=20Codex=20tool=5Fname=20+=20?= =?UTF-8?q?plain-stdout=20context,=20defer=20continue:false?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-1 Codex review findings on the bridges: - Stop force-continue (both bridges): a blocking Stop hook with EMPTY stderr yielded decision 'deny' + reason undefined, and the `&& reason !== undefined` guard let the turn STOP — the opposite of a blocking Stop hook. Force-continue on any deny; fall back to a generic steering line when there is no reason. - Codex payload tool_name: hardcoded "Bash" disagreed with the exec.name matcher subject, so a real Codex `matcher:"Bash"` never fired against the harness's lowercase `bash` tool. Use exec.name in both payload builders (matches the matcher subject and the sibling CC bridge). Doc/RFC updated. - Codex plain-stdout context: SessionStart/UserPromptSubmit are documented to treat a clean hook's PLAIN (non-JSON) stdout as additionalContext, but nothing folded it. runPoint now folds plain stdout into context for those two events, gated on the codec's JSON gate so structured stdout is never dumped as prose. - continue:false is deferred, not honored: the seams have no hard-halt primitive yet. TODO(hook-continue-false) at both bridges + an RFC deferred note; the two tests now assert the LOG records the halt request AND that the run is NOT actually halted (no longer misleading). - README concurrency wording: hooks run SERIALLY (deliberate — adjacent invoked/result log pairs, order-independent fold), not concurrently. Fixed the CC README claim + an RFC note. Regression guards proven red on the unfixed code, then reverted. The mismatched- hookEventName discard (also flagged) is fixed in dsh-hook-protocol and merged down. --- .../feature/2026-06-30-hook-bridges.md | 7 +- packages/hooks/hooks-claude/README.md | 2 +- packages/hooks/hooks-claude/src/index.ts | 18 +++- .../hooks/hooks-claude/tests/coverage.spec.ts | 33 ++++++- packages/hooks/hooks-codex/README.md | 2 +- packages/hooks/hooks-codex/src/index.ts | 40 ++++++-- .../hooks/hooks-codex/tests/coverage.spec.ts | 92 ++++++++++++++++++- 7 files changed, 174 insertions(+), 20 deletions(-) diff --git a/docs/rfc/implemented/feature/2026-06-30-hook-bridges.md b/docs/rfc/implemented/feature/2026-06-30-hook-bridges.md index a3e01c17be..1e782f9b67 100644 --- a/docs/rfc/implemented/feature/2026-06-30-hook-bridges.md +++ b/docs/rfc/implemented/feature/2026-06-30-hook-bridges.md @@ -15,7 +15,7 @@ The framing that shapes the whole design: **a bridge is a faithfulness adapter, Two independent plugins in the `packages/hooks/` group, each a function/namespace plugin (`name`/`inject`/`Config`/`apply`, NO default export — see [postmortem 0001](../../../postmortem/0001-acp-default-export-drops-inject.md)) injecting only `bash`: - **`dsh-hooks-claude`** — the CC dialect. Seven hook points: `SessionStart`, `UserPromptSubmit`, `PreToolUse`, `PostToolUse`, `Stop`, `SubagentStart`, `SubagentStop`. Owns CC's per-event stdin payloads (a base of `session_id`/`cwd`/`hook_event_name` plus per-event fields), CC's env + `${CLAUDE_PLUGIN_ROOT}`/`${CLAUDE_PROJECT_DIR}` substitution, and the literal-or-regex matcher mode. A CC hook's stdin carries a **trailing newline**. -- **`dsh-hooks-codex`** — the Codex dialect: a deliberate SUBSET. Five hook points (`PreToolUse`, `PostToolUse`, `SessionStart`, `UserPromptSubmit`, `Stop` — no subagent/notification/compaction), an always-regex matcher, snake_case payloads with `turn_id`/`model`/`permission_mode` extras written WITHOUT a trailing newline, no env and no `${…}` substitution, and a block-only decision model (a Codex hook can never pre-approve, so `allow`/`ask` are not honored). Codex hardcodes a tool call's `tool_name` to `"Bash"` and `tool_input` to `{ command }`. +- **`dsh-hooks-codex`** — the Codex dialect: a deliberate SUBSET. Five hook points (`PreToolUse`, `PostToolUse`, `SessionStart`, `UserPromptSubmit`, `Stop` — no subagent/notification/compaction), an always-regex matcher, snake_case payloads with `turn_id`/`model`/`permission_mode` extras written WITHOUT a trailing newline, no env and no `${…}` substitution, and a block-only decision model (a Codex hook can never pre-approve, so `allow`/`ask` are not honored). A tool call's payload carries the real `tool_name` (the value the matcher tests, so a config's tool matcher fires) in Codex's `tool_input: { command }` shape. ### Outcome → Decision mapping @@ -44,8 +44,13 @@ The config is parsed ONCE at load; a read/parse failure logs and registers nothi - **Tool-input rewrite.** A CC/Codex `updatedInput` is logged + warned, not honored — input rewrite is a deferred consistency-design problem ([the pre-tool-input-rewrite RFC](../../proposed/feature/2026-06-30-pre-tool-input-rewrite.md)), because the pre-execution args are read by `tool/call` audit + `assistant/message` history + ACP/tool-bash presentation, so an honest rewrite is a design unit, not a field. - **Stop loop-guard** (`TODO(stop-loop-guard)`). CC/Codex break an infinite force-continue with `stop_hook_active` (true once a Stop hook fired this run) plus a max-consecutive cap; both are deferred. Today `stop_hook_active` is always `false`, so a Stop hook that unconditionally blocks would force-continue every step — a hook author must self-limit until the guard lands. - **Permission `ask`** degrades to `deny` at the `tools/pre-execute` seam (`FIXME(permissions)` in the interception-seams RFC) — there is no interactive permission prompt yet. +- **Hook `continue:false` (hard halt).** A hook can ask to halt the whole run (CC/Codex `continue:false`); the shared merge folds it into `MergedHookOutcome.stop`/`stopReason`, but no bridge acts on it (`TODO(hook-continue-false)`) — the interception seams have no "hard-halt the agent" primitive yet (a Decision blocks/steers a single point, not the run). Deferred with the loop-guard work; the halt request is recorded in the `hook/result` log, and the hook keeps its per-point effect (decision/context) meanwhile. - **Config discovery.** The path is explicit in `cordis.yml`; the full multi-layer CC/Codex precedence walk and the trust/hash model are not reimplemented (`TODO`). +### Multiple hooks on one point run serially, not concurrently + +The reference engines run a point's matched hooks concurrently and fold the results. These bridges run them **serially** (`await` per hook inside the match loop) and fold with the same most-restrictive merge. Serial is deliberate: it keeps each hook's `hook/invoked`/`hook/result` pair adjacent and in a deterministic order in the session log, and the fold is order-independent for the decision (`deny > ask > allow`) so the outcome matches. The cost is latency (hook *N* waits for hook *N−1*) and that per-hook timeouts are not overlapped — acceptable for the hook counts real configs use; revisit if a config ever fans out enough for the wall-clock to matter. + ## Consequences The bridges are thin and readable standalone: the correctness-critical halves (matcher semantics, exit-code contract, merge precedence) live in the shared `dsh-hook-protocol`, so each bridge is just config-parse + payload-build + outcome-map. Each is covered at per-file 100% — config-parse branches as unit tests, and the seam mappings end-to-end through the REAL loop + REAL `dsh-bash-local` + REAL shell scripts from a temp `hooks.json` (a scripted mock MODEL is the only stand-in), plus a real-Loader export-shape guard so a stray default export can't silently drop `inject`. Because the seams already carry typed Decisions, a future native plugin needs none of this bridge machinery — it returns a Decision directly. diff --git a/packages/hooks/hooks-claude/README.md b/packages/hooks/hooks-claude/README.md index 02cdfc9894..126573c6be 100644 --- a/packages/hooks/hooks-claude/README.md +++ b/packages/hooks/hooks-claude/README.md @@ -39,7 +39,7 @@ The config is parsed **once** at load. A read/parse failure is contained — the | `SubagentStart` | `subagent/start` (emit) | additionalContext → `agent.inject()` into the live child | | `SubagentStop` | `subagent/end` (emit) | observe-only | -The matcher subject is the tool name (`PreToolUse`/`PostToolUse`), the session source (`SessionStart`), or the child's agent type (`SubagentStart`/`SubagentStop`); `UserPromptSubmit`/`Stop` ignore matchers. Multiple file-configured hooks on one point run concurrently and fold most-restrictively (`deny > ask > allow`, see `dsh-hook-protocol`). +The matcher subject is the tool name (`PreToolUse`/`PostToolUse`), the session source (`SessionStart`), or the child's agent type (`SubagentStart`/`SubagentStop`); `UserPromptSubmit`/`Stop` ignore matchers. Multiple file-configured hooks on one point run **serially, in config order**, and fold most-restrictively (`deny > ask > allow`, see `dsh-hook-protocol`); serial keeps each hook's `hook/invoked`/`hook/result` pair adjacent in the log, and the fold is order-independent for the decision (see the RFC's "run serially, not concurrently" note). ## Context source diff --git a/packages/hooks/hooks-claude/src/index.ts b/packages/hooks/hooks-claude/src/index.ts index 5e8478e2b7..7e37f1fba0 100644 --- a/packages/hooks/hooks-claude/src/index.ts +++ b/packages/hooks/hooks-claude/src/index.ts @@ -161,6 +161,14 @@ export function apply(ctx: Context, config: Config): void { return mergeHookOutputs(outputs) } + // TODO(hook-continue-false): the merge computes `merged.stop`/`stopReason` from + // a hook's `continue:false`, but no seam below honors it — there is no + // "hard-halt the whole agent" primitive on the interception seams yet (a + // Decision can block/deny/steer a single point, not stop the run). Honoring it + // needs that primitive; deferred with the loop-guard work. Until then a + // `continue:false` hook still has its per-point effect (its decision/context), + // and the halt request is recorded in the `hook/result` log but not acted on. + /** Build a HookContext from accumulated additionalContext strings, or undefined when none. */ function contextFrom(merged: MergedHookOutcome): HookContext | undefined { if (merged.additionalContext.length === 0) return undefined @@ -224,9 +232,13 @@ export function apply(ctx: Context, config: Config): void { // step — a hook author must self-limit until the guard lands. --- ctx.on('agent/turn-continuation', async (agent, turn, _default, next): Promise => { const merged = await runPoint('Stop', '', stopPayload(agent), { agent, turn }) - if (merged.decision === 'deny' && merged.reason !== undefined) { - // A blocking Stop hook forces continuation, feeding its reason as next-step steering. - return { action: 'continue', reason: { content: [{ type: 'text', text: merged.reason }], source: PLUGIN_SOURCE } } + if (merged.decision === 'deny') { + // A blocking Stop hook forces continuation. It carries its reason as + // next-step steering; a blocking hook that emitted no reason (exit 2, empty + // stderr) still forces the turn to continue — the block is what matters, so + // fall back to a generic steering line rather than letting the turn stop. + const text = merged.reason ?? 'continue: blocked by Stop hook' + return { action: 'continue', reason: { content: [{ type: 'text', text }], source: PLUGIN_SOURCE } } } return next() }) diff --git a/packages/hooks/hooks-claude/tests/coverage.spec.ts b/packages/hooks/hooks-claude/tests/coverage.spec.ts index 29dda6b850..9b48e6d645 100644 --- a/packages/hooks/hooks-claude/tests/coverage.spec.ts +++ b/packages/hooks/hooks-claude/tests/coverage.spec.ts @@ -148,6 +148,25 @@ describe('hooks-claude coverage — Stop continuation + subagent inject/catch', expect(JSON.stringify(adapter.requests[1]!.messages)).toContain('continue please') }) + it('a Stop hook that blocks with EMPTY stderr still forces continuation (no reason required)', async () => { + // Regression: a blocking Stop hook (exit 2) with no stderr yields decision + // 'deny' + reason undefined; the turn must STILL force-continue (the block is + // what matters), not silently stop. Self-limit to one block so it can't loop. + const d = dir() + const marker = join(d, 'fired') + const s = sh(d, 'stop.sh', `#!/usr/bin/env bash\nif [ -e "${marker}" ]; then exit 0; fi\ntouch "${marker}"\nexit 2\n`) + const path = hooks(d, { Stop: [{ hooks: [{ type: 'command', command: s }] }] }) + const adapter = new MockAdapter([textResponse('one'), textResponse('two')]) + const ctx = await harness(path, adapter) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]) + await waitForIdle(ctx, agent) + // A second model request ran → the empty-reason block forced continuation. + expect(adapter.requests).toHaveLength(2) + // The steering carried the fallback reason (no stderr to use). + expect(JSON.stringify(adapter.requests[1]!.messages)).toContain('blocked by Stop hook') + }) + it('SubagentStart additionalContext is injected into a REGISTERED live child', async () => { const d = dir() const s = sh(d, 'sa.sh', '#!/usr/bin/env bash\necho \'{"hookSpecificOutput":{"hookEventName":"SubagentStart","additionalContext":"child guidance"}}\'\n') @@ -329,18 +348,26 @@ describe('hooks-claude coverage — schema-bypass default + unspawnable hook', ( }) describe('hooks-claude coverage — continue:false, context arm, no-cwd', () => { - it('a hook with {"continue":false} and no decision records decision "stop"', async () => { + it('a {"continue":false} hook is RECORDED as decision "stop" but does not halt the run (TODO(hook-continue-false))', async () => { + // Honoring `continue:false` (hard-halt the whole run) is deferred — there is + // no such primitive on the interception seams yet. So this asserts the LOG + // faithfully records the halt request (decision "stop"), AND that the run is + // NOT actually halted: the tool still runs and the turn completes normally. const d = dir() const s = sh(d, 'stop.sh', '#!/usr/bin/env bash\necho \'{"continue":false,"stopReason":"halt"}\'\n') const path = hooks(d, { PreToolUse: [{ hooks: [{ type: 'command', command: s }] }] }) const adapter = new MockAdapter([toolCallResponse('c1', 'echo', {}), textResponse('done')]) const ctx = await harness(path, adapter) - ctx.tools.register(defineTool({ name: 'echo', description: 'e', parameters: {}, async execute() { return [{ type: 'text', text: 'ok' }] } })) + let ran = false + ctx.tools.register(defineTool({ name: 'echo', description: 'e', parameters: {}, async execute() { ran = true; return [{ type: 'text', text: 'ok' }] } })) const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) agent.send([{ type: 'text', text: 'go' }]) await waitForIdle(ctx, agent) const res = events(agent).find(e => e.type === 'hook/result') - expect(res?.type === 'hook/result' && res.data.decision).toBe('stop') + expect(res?.type === 'hook/result' && res.data.decision).toBe('stop') // recorded + expect(ran).toBe(true) // NOT honored: the tool still ran (halt is deferred) + const turnEnd = events(agent).findLast(e => e.type === 'turn/end') + expect(turnEnd?.type === 'turn/end' && turnEnd.data.reason.kind).toBe('completed') // ran to completion }) it('a PostToolUse hook that BOTH blocks AND attaches additionalContext', async () => { diff --git a/packages/hooks/hooks-codex/README.md b/packages/hooks/hooks-codex/README.md index 39d4fcd5ca..e20b3a211d 100644 --- a/packages/hooks/hooks-codex/README.md +++ b/packages/hooks/hooks-codex/README.md @@ -43,7 +43,7 @@ The config is parsed **once** at load; a read/parse failure is contained (logs + | `PostToolUse` | `tools/post-execute` (waterfall) | `block` → `block` with feedback; additionalContext → `accept` with context | | `Stop` | `agent/turn-continuation` (waterfall) | a blocking Stop hook forces `continue` with the reason as next-step steering | -Codex hardcodes a tool call's `tool_name` to `"Bash"` and `tool_input` to `{ command }` (extracted from the call's arguments, or `''` when absent). The matcher subject is the tool name (`PreToolUse`/`PostToolUse`) or the session source (`SessionStart`); `UserPromptSubmit`/`Stop` ignore matchers. +A tool call's payload carries the real `tool_name` (the same value the matcher tests) and Codex's `tool_input: { command }` shape (the `command` arg when present, else `''`). The matcher subject is the tool name (`PreToolUse`/`PostToolUse`) or the session source (`SessionStart`); `UserPromptSubmit`/`Stop` ignore matchers. ## Context source diff --git a/packages/hooks/hooks-codex/src/index.ts b/packages/hooks/hooks-codex/src/index.ts index 127511389d..b61e60bdfb 100644 --- a/packages/hooks/hooks-codex/src/index.ts +++ b/packages/hooks/hooks-codex/src/index.ts @@ -86,7 +86,7 @@ export function apply(ctx: Context, config: Config): void { point: string, matchQuery: string, payload: unknown, - opts: { agent?: Agent; turn?: number; signal?: AbortSignal }, + opts: { agent?: Agent; turn?: number; signal?: AbortSignal; plainStdoutAsContext?: boolean }, ): Promise { const groups: MatcherGroup[] = parsed[point] ?? [] const outputs: HookOutput[] = [] @@ -108,6 +108,17 @@ export function apply(ctx: Context, config: Config): void { defaultTimeoutMs, trailingNewline: false, // Codex writes stdin WITHOUT a trailing newline. }, () => performance.now()) + // Codex's SessionStart/UserPromptSubmit treat a clean hook's PLAIN + // (non-JSON) stdout as additionalContext. The codec keeps that raw text on + // `output.stdout` but only sets `additionalContext` from a JSON + // `hookSpecificOutput`, so fold plain stdout in here and let the shared + // merge + contextFrom path carry it. Guarded on the codec's own JSON gate + // (stdout starting with `{`) so a structured hook's raw JSON is never + // injected as prose, and it never clobbers an explicit additionalContext. + if (opts.plainStdoutAsContext === true && output.additionalContext === undefined + && output.stdout.length > 0 && !output.stdout.startsWith('{')) { + output.additionalContext = output.stdout + } outputs.push(output) if (session && opts.turn !== undefined) { const stderrSummary = summarize(output.stderr) @@ -124,6 +135,12 @@ export function apply(ctx: Context, config: Config): void { return mergeHookOutputs(outputs) } + // TODO(hook-continue-false): the merge computes `merged.stop`/`stopReason` from + // a hook's `continue:false`, but no seam below honors it — there is no + // "hard-halt the whole agent" primitive on the interception seams yet. Deferred + // with the loop-guard work; until then a `continue:false` hook keeps its + // per-point effect and the halt request is recorded in `hook/result`, not acted on. + function contextFrom(merged: MergedHookOutcome): HookContext | undefined { if (merged.additionalContext.length === 0) return undefined const content: ContentBlock[] = merged.additionalContext.map(text => ({ type: 'text', text })) @@ -132,7 +149,7 @@ export function apply(ctx: Context, config: Config): void { // SessionStart: emit. Codex passes a plain-stdout hook's output as additionalContext. ctx.on('agent/session-start', (agent, source) => { - void runPoint('SessionStart', source, { ...base(agent, 'SessionStart', model), source }, { agent }) + void runPoint('SessionStart', source, { ...base(agent, 'SessionStart', model), source }, { agent, plainStdoutAsContext: true }) .then((merged) => { const context = contextFrom(merged) if (context) agent.inject(context.content, { source: context.source }) @@ -143,7 +160,7 @@ export function apply(ctx: Context, config: Config): void { // UserPromptSubmit → PromptDecision. Codex can only BLOCK (no allow/ask). ctx.on('agent/prompt-submit', async (agent, content, _source, next): Promise => { const turn = lastTurn(agent) - const merged = await runPoint('UserPromptSubmit', '', { ...turnBase(agent, 'UserPromptSubmit', model), prompt: blocksToText(content) }, { agent, turn }) + const merged = await runPoint('UserPromptSubmit', '', { ...turnBase(agent, 'UserPromptSubmit', model), prompt: blocksToText(content) }, { agent, turn, plainStdoutAsContext: true }) if (merged.decision === 'deny') return { kind: 'block', reason: merged.reason ?? 'blocked by UserPromptSubmit hook' } const context = contextFrom(merged) if (context) return { kind: 'allow', additionalContext: context } @@ -176,8 +193,12 @@ export function apply(ctx: Context, config: Config): void { // loop-guard (stop_hook_active + a max-consecutive cap) is deferred. ctx.on('agent/turn-continuation', async (agent, turn, _default, next): Promise => { const merged = await runPoint('Stop', '', { ...turnBase(agent, 'Stop', model), stop_hook_active: false, last_assistant_message: null }, { agent, turn }) - if (merged.decision === 'deny' && merged.reason !== undefined) { - return { action: 'continue', reason: { content: [{ type: 'text', text: merged.reason }], source: PLUGIN_SOURCE } } + if (merged.decision === 'deny') { + // A blocking Stop hook forces continuation; a block with no reason (exit 2, + // empty stderr) still forces it — fall back to a generic steering line + // rather than letting the turn stop. + const text = merged.reason ?? 'continue: blocked by Stop hook' + return { action: 'continue', reason: { content: [{ type: 'text', text }], source: PLUGIN_SOURCE } } } return next() }) @@ -226,10 +247,13 @@ function commandOf(args: unknown): string { } function preToolPayload(exec: ToolExecution, model: string): Record { - // Codex hardcodes tool_name to "Bash" and tool_input to { command }. - return { ...turnBase(exec.agent, 'PreToolUse', model), tool_name: 'Bash', tool_input: { command: commandOf(exec.arguments) }, tool_use_id: exec.callId } + // `tool_name` is the REAL tool name (matching the `exec.name` matcher subject); + // a hardcoded constant would disagree with what the matcher tests and make a + // config's tool matcher never fire. `tool_input` keeps Codex's `{ command }` + // shape (its shell payload), derived from the call's `command` arg when present. + return { ...turnBase(exec.agent, 'PreToolUse', model), tool_name: exec.name, tool_input: { command: commandOf(exec.arguments) }, tool_use_id: exec.callId } } function postToolPayload(exec: ToolExecution, result: ToolExecutionResult, model: string): Record { - return { ...turnBase(exec.agent, 'PostToolUse', model), tool_name: 'Bash', tool_input: { command: commandOf(exec.arguments) }, tool_use_id: exec.callId, tool_response: blocksToText(result.content) } + return { ...turnBase(exec.agent, 'PostToolUse', model), tool_name: exec.name, tool_input: { command: commandOf(exec.arguments) }, tool_use_id: exec.callId, tool_response: blocksToText(result.content) } } diff --git a/packages/hooks/hooks-codex/tests/coverage.spec.ts b/packages/hooks/hooks-codex/tests/coverage.spec.ts index 732e0a7c61..d10f2d7df1 100644 --- a/packages/hooks/hooks-codex/tests/coverage.spec.ts +++ b/packages/hooks/hooks-codex/tests/coverage.spec.ts @@ -217,16 +217,21 @@ describe('hooks-codex coverage — decision mapping paths', () => { expect(events(agent).some(e => e.type === 'hook/invoked')).toBe(false) }) - it('a {"continue":false} hook with no decision records decision "stop"', async () => { + it('a {"continue":false} hook is RECORDED as "stop" but does not halt the run (TODO(hook-continue-false))', async () => { + // Honoring `continue:false` is deferred — the seams have no hard-halt + // primitive. Assert the LOG records the halt request AND that the run is not + // actually halted (the tool still runs, the turn completes). const d = dir() hooks(d, { PreToolUse: [{ hooks: [{ type: 'command', command: sh(d, 's.sh', '#!/usr/bin/env bash\necho \'{"continue":false,"stopReason":"halt"}\'\n') }] }] }) const adapter = new MockAdapter([toolCallResponse('c1', 'Bash', { command: 'x' }), textResponse('done')]) const ctx = await harness(join(d, 'hooks.json'), adapter) - ctx.tools.register(defineTool({ name: 'Bash', description: 'b', parameters: { command: { type: 'string' } }, async execute() { return [{ type: 'text', text: 'ok' }] } })) + let ran = false + ctx.tools.register(defineTool({ name: 'Bash', description: 'b', parameters: { command: { type: 'string' } }, async execute() { ran = true; return [{ type: 'text', text: 'ok' }] } })) const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) agent.send([{ type: 'text', text: 'go' }]); await waitForIdle(ctx, agent) const res = events(agent).find(e => e.type === 'hook/result') - expect(res?.type === 'hook/result' && res.data.decision).toBe('stop') + expect(res?.type === 'hook/result' && res.data.decision).toBe('stop') // recorded + expect(ran).toBe(true) // NOT honored: the tool still ran (halt is deferred) }) it('PreToolUse deny with EMPTY stderr uses the default reason (?? right arm)', async () => { @@ -305,4 +310,85 @@ describe('hooks-codex coverage — decision mapping paths', () => { const res = events(agent).find(e => e.type === 'hook/result') expect(res?.type === 'hook/result' && 'exitCode' in res.data).toBe(false) }) + + it('a blocking Stop hook with EMPTY stderr still forces continuation (no reason required)', async () => { + // Regression: an exit-2 Stop hook with no stderr yields decision 'deny' + + // reason undefined; the turn must STILL force-continue, not silently stop. + const d = dir() + const marker = join(d, 'fired') + hooks(d, { Stop: [{ hooks: [{ type: 'command', command: sh(d, 's.sh', `#!/usr/bin/env bash\nif [ -e "${marker}" ]; then exit 0; fi\ntouch "${marker}"\nexit 2\n`) }] }] }) + const adapter = new MockAdapter([textResponse('one'), textResponse('two')]) + const ctx = await harness(join(d, 'hooks.json'), adapter) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]); await waitForIdle(ctx, agent) + expect(adapter.requests).toHaveLength(2) // empty-reason block forced continuation + expect(JSON.stringify(adapter.requests[1]!.messages)).toContain('blocked by Stop hook') + }) + + it('a clean UserPromptSubmit hook that prints PLAIN stdout injects it as context', async () => { + // Codex feeds a SessionStart/UserPromptSubmit hook's PLAIN (non-JSON) stdout + // as additionalContext (unlike CC, which needs a JSON hookSpecificOutput). + const d = dir() + hooks(d, { UserPromptSubmit: [{ hooks: [{ type: 'command', command: sh(d, 'ctx.sh', '#!/usr/bin/env bash\necho "extra guidance from a plain hook"\nexit 0\n') }] }] }) + const adapter = new MockAdapter([textResponse('ok')]) + const ctx = await harness(join(d, 'hooks.json'), adapter) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]); await waitForIdle(ctx, agent) + expect(JSON.stringify(adapter.requests[0]!.messages)).toContain('extra guidance from a plain hook') + }) + + it('a clean SessionStart hook that prints PLAIN stdout injects it (not JSON)', async () => { + const d = dir() + hooks(d, { SessionStart: [{ hooks: [{ type: 'command', command: sh(d, 'ss.sh', '#!/usr/bin/env bash\necho "session preamble"\nexit 0\n') }] }] }) + const adapter = new MockAdapter([textResponse('ok')]) + const ctx = await harness(join(d, 'hooks.json'), adapter) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + await new Promise(r => setTimeout(r, 60)) + agent.send([{ type: 'text', text: 'go' }]); await waitForIdle(ctx, agent) + expect(JSON.stringify(adapter.requests[0]!.messages)).toContain('session preamble') + }) + + it('a clean hook that prints JSON is NOT injected as prose (plain-stdout gate)', async () => { + // A structured (JSON) stdout must go through the hookSpecificOutput path, not + // be dumped verbatim as context — the `!startsWith('{')` gate guards this. + const d = dir() + hooks(d, { UserPromptSubmit: [{ hooks: [{ type: 'command', command: sh(d, 'j.sh', '#!/usr/bin/env bash\necho \'{"unrelated":"json"}\'\nexit 0\n') }] }] }) + const adapter = new MockAdapter([textResponse('ok')]) + const ctx = await harness(join(d, 'hooks.json'), adapter) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]); await waitForIdle(ctx, agent) + expect(JSON.stringify(adapter.requests[0]!.messages)).not.toContain('unrelated') + }) + + it('the PreToolUse payload carries the REAL tool name (matches the matcher subject)', async () => { + // Regression: the payload once hardcoded tool_name "Bash", disagreeing with + // the exec.name matcher subject — a config matcher on the real name would + // then never fire. Capture the payload and assert tool_name === the real name. + const d = dir() + const cap = join(d, 'payload') + hooks(d, { PreToolUse: [{ hooks: [{ type: 'command', command: sh(d, 'cap.sh', `#!/usr/bin/env bash\ncat > "${cap}"\nexit 0\n`) }] }] }) + const adapter = new MockAdapter([toolCallResponse('c1', 'shell', { command: 'ls' }), textResponse('done')]) + const ctx = await harness(join(d, 'hooks.json'), adapter) + ctx.tools.register(defineTool({ name: 'shell', description: 'b', parameters: { command: { type: 'string' } }, async execute() { return [{ type: 'text', text: 'ok' }] } })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]); await waitForIdle(ctx, agent) + const payload = JSON.parse(readFileSync(cap, 'utf8')) as { tool_name: string; tool_input: { command: string } } + expect(payload.tool_name).toBe('shell') + expect(payload.tool_input.command).toBe('ls') + }) + + it('a Codex matcher on the REAL tool name fires (matcher subject === payload tool_name)', async () => { + // A regex matcher matching the real tool name must select the hook — proving + // the matcher subject and the payload tool_name agree. + const d = dir() + hooks(d, { PreToolUse: [{ matcher: 'shell', hooks: [{ type: 'command', command: sh(d, 'd.sh', '#!/usr/bin/env bash\nexit 2\n') }] }] }) + const adapter = new MockAdapter([toolCallResponse('c1', 'shell', { command: 'ls' }), textResponse('done')]) + const ctx = await harness(join(d, 'hooks.json'), adapter) + let ran = false + ctx.tools.register(defineTool({ name: 'shell', description: 'b', parameters: { command: { type: 'string' } }, async execute() { ran = true; return [{ type: 'text', text: 'ok' }] } })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]); await waitForIdle(ctx, agent) + expect(ran).toBe(false) // the matcher fired → the hook denied the tool + expect(events(agent).some(e => e.type === 'hook/invoked' && e.data.point === 'PreToolUse')).toBe(true) + }) }) From 253eded47b75515d3fdab785d7b76e8c573d118f Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Wed, 1 Jul 2026 10:56:34 +0800 Subject: [PATCH 03/10] fix(hooks): pass expectedEventName so a mismatched hookSpecificOutput block is discarded Wire the bridges to the codec's new discriminator check (merged down from dsh-hook-protocol): each bridge passes its firing `point` as `expectedEventName` to runHook, so a hook whose `hookSpecificOutput.hookEventName` names a different event has its event-scoped fields discarded. Bridge-level guard test: a PreToolUse hook emitting a UserPromptSubmit-labeled deny no longer denies the tool (proven red without the wiring, then reverted). --- packages/hooks/hooks-claude/src/index.ts | 3 +++ .../hooks/hooks-claude/tests/coverage.spec.ts | 16 ++++++++++++++++ packages/hooks/hooks-codex/src/index.ts | 2 ++ 3 files changed, 21 insertions(+) diff --git a/packages/hooks/hooks-claude/src/index.ts b/packages/hooks/hooks-claude/src/index.ts index 7e37f1fba0..74b1e6f5c0 100644 --- a/packages/hooks/hooks-claude/src/index.ts +++ b/packages/hooks/hooks-claude/src/index.ts @@ -141,6 +141,9 @@ export function apply(ctx: Context, config: Config): void { ...opts.signal ? { signal: opts.signal } : {}, defaultTimeoutMs, trailingNewline: true, + // Discard a `hookSpecificOutput` block whose `hookEventName` names a + // different event than the one firing (the schemas key it by event). + expectedEventName: point, }, () => performance.now()) outputs.push(output) if (output.updatedInput !== undefined) { diff --git a/packages/hooks/hooks-claude/tests/coverage.spec.ts b/packages/hooks/hooks-claude/tests/coverage.spec.ts index 9b48e6d645..fb8b11e739 100644 --- a/packages/hooks/hooks-claude/tests/coverage.spec.ts +++ b/packages/hooks/hooks-claude/tests/coverage.spec.ts @@ -387,6 +387,22 @@ describe('hooks-claude coverage — continue:false, context arm, no-cwd', () => expect(events(agent).some(e => e.type === 'context/message' && e.data.content.some(b => b.type === 'text' && b.text.includes('context too')))).toBe(true) }) + it('a PreToolUse hook whose hookSpecificOutput names a DIFFERENT event does NOT deny the tool', async () => { + // The block's hookEventName (UserPromptSubmit) mismatches the firing event + // (PreToolUse), so its permissionDecision:"deny" is discarded — the tool runs. + const d = dir() + const s = sh(d, 'x.sh', '#!/usr/bin/env bash\necho \'{"hookSpecificOutput":{"hookEventName":"UserPromptSubmit","permissionDecision":"deny"}}\'\n') + const path = hooks(d, { PreToolUse: [{ hooks: [{ type: 'command', command: s }] }] }) + const adapter = new MockAdapter([toolCallResponse('c1', 'echo', {}), textResponse('done')]) + const ctx = await harness(path, adapter) + let ran = false + ctx.tools.register(defineTool({ name: 'echo', description: 'e', parameters: {}, async execute() { ran = true; return [{ type: 'text', text: 'ok' }] } })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]) + await waitForIdle(ctx, agent) + expect(ran).toBe(true) // the mismatched deny was discarded → the tool ran + }) + }) describe('hooks-claude coverage — executor reject + no-open-turn', () => { diff --git a/packages/hooks/hooks-codex/src/index.ts b/packages/hooks/hooks-codex/src/index.ts index b61e60bdfb..e3e2ac020b 100644 --- a/packages/hooks/hooks-codex/src/index.ts +++ b/packages/hooks/hooks-codex/src/index.ts @@ -107,6 +107,8 @@ export function apply(ctx: Context, config: Config): void { ...opts.signal ? { signal: opts.signal } : {}, defaultTimeoutMs, trailingNewline: false, // Codex writes stdin WITHOUT a trailing newline. + // Discard a `hookSpecificOutput` block naming a different event. + expectedEventName: point, }, () => performance.now()) // Codex's SessionStart/UserPromptSubmit treat a clean hook's PLAIN // (non-JSON) stdout as additionalContext. The codec keeps that raw text on From a72ebda723781ad714320a9ddf85a35c59fac207 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Wed, 1 Jul 2026 11:25:54 +0800 Subject: [PATCH 04/10] test(hooks): poll for detached-hook effects instead of a fixed sleep (flake fix) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bridge tests that drive observe-only emit listeners (session-start, subagent/start, subagent/end) fire their hook on a detached `.then` the test cannot await. They waited a fixed 50-80ms, which flaked under the full test:coverage run's heavy parallel load (transform ~400s): the sleep expired before the async hook completed, so the injected context / marker file / warn call had not landed. Replace each fixed sleep with a `waitFor(predicate)` poll that retries until the observable effect appears (5s deadline) — "async state is not synchronous state": wait for the signal that actually fires, not a guessed duration. No behavior change; the same assertions, made robust to scheduling. --- .../hooks/hooks-claude/tests/bridge.spec.ts | 26 ++++++++++++++++--- .../hooks/hooks-claude/tests/coverage.spec.ts | 18 +++++++++---- .../hooks/hooks-codex/tests/coverage.spec.ts | 21 +++++++++++---- 3 files changed, 51 insertions(+), 14 deletions(-) diff --git a/packages/hooks/hooks-claude/tests/bridge.spec.ts b/packages/hooks/hooks-claude/tests/bridge.spec.ts index 73cd66df10..0cc4458f52 100644 --- a/packages/hooks/hooks-claude/tests/bridge.spec.ts +++ b/packages/hooks/hooks-claude/tests/bridge.spec.ts @@ -64,6 +64,20 @@ function events(agent: ReactLoopAgent): SessionEvent[] { return [...agent.session.events] } +/** + * Poll `predicate` until it returns true or the deadline passes. Detached + * emit-listener hooks (session-start, subagent) fire on a `.then` the test can't + * await directly; polling for the observable EFFECT is robust under load, where a + * single fixed sleep flakes ("async state is not synchronous state"). + */ +async function waitFor(predicate: () => boolean, timeout = 5000, interval = 10): Promise { + const deadline = Date.now() + timeout + while (!predicate()) { + if (Date.now() > deadline) throw new Error('waitFor: condition not met before deadline') + await new Promise(r => setTimeout(r, interval)) + } +} + describe('hooks-claude bridge — UserPromptSubmit', () => { it('a UserPromptSubmit hook that exits 2 blocks the prompt (rejected turn)', async () => { // The UserPromptSubmit hook exits 2 (blocking) with a reason on stderr. @@ -239,8 +253,11 @@ describe('hooks-claude bridge — SessionStart', () => { const adapter = new MockAdapter([textResponse('ok')]) const ctx = await harness(dir, adapter) const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) - // session-start fires async; wait a tick for the inject before sending. - await new Promise(r => setTimeout(r, 50)) + // session-start fires async (detached .then → agent.inject); wait for the + // injected context/message to actually land before sending, rather than a + // fixed sleep that flakes under load. + await waitFor(() => events(agent).some(e => e.type === 'context/message' + && e.data.content.some(b => b.type === 'text' && b.text.includes('project uses tabs')))) agent.send([{ type: 'text', text: 'go' }]) await waitForIdle(ctx, agent) @@ -274,10 +291,11 @@ describe('hooks-claude bridge — SubagentStart / SubagentStop (observe)', () => // child lookup yields undefined and it simply runs the hook. ctx.emit('subagent/start', { provider: 'inproc', id: AgentId('child-1'), agentType: 'researcher' }) ctx.emit('subagent/end', { provider: 'inproc', id: AgentId('child-1'), agentType: 'researcher', stopReason: 'completed', lastAssistantMessage: [{ type: 'text', text: 'done' }] }) - // Both hooks run async (detached .then); let them settle. - await new Promise(r => setTimeout(r, 80)) + // Both hooks run async (detached .then); poll for their marker files rather + // than a fixed sleep that flakes under load. const { existsSync } = await import('node:fs') + await waitFor(() => existsSync(startMarker) && existsSync(stopMarker)) expect(existsSync(startMarker)).toBe(true) expect(existsSync(stopMarker)).toBe(true) }) diff --git a/packages/hooks/hooks-claude/tests/coverage.spec.ts b/packages/hooks/hooks-claude/tests/coverage.spec.ts index fb8b11e739..ca3a143deb 100644 --- a/packages/hooks/hooks-claude/tests/coverage.spec.ts +++ b/packages/hooks/hooks-claude/tests/coverage.spec.ts @@ -45,6 +45,15 @@ function waitForIdle(ctx: Context, agent: ReactLoopAgent): Promise { return new Promise((resolve) => { const d = ctx.on('agent/status', (s, st) => { if (s === agent && st === 'idle') { d(); resolve() } }) }) } function events(agent: ReactLoopAgent): SessionEvent[] { return [...agent.session.events] } +/** Poll until `predicate` holds or the deadline passes — robust to detached + * emit-listener hooks firing on a `.then` (a fixed sleep flakes under load). */ +async function waitFor(predicate: () => boolean, timeout = 5000, interval = 10): Promise { + const deadline = Date.now() + timeout + while (!predicate()) { + if (Date.now() > deadline) throw new Error('waitFor: condition not met before deadline') + await new Promise(r => setTimeout(r, interval)) + } +} describe('hooks-claude coverage — config option arms + substitution + skip warning', () => { it('honors pluginRoot + projectDir substitution and warns on a skipped non-command hook', async () => { @@ -177,7 +186,7 @@ describe('hooks-claude coverage — Stop continuation + subagent inject/catch', const child = { id: AgentId('child-x'), inject: (content: { type: string; text?: string }[]) => { injected.push(content.map(b => b.text ?? '').join('')) }, session: { header: { id: 'child-x' } } } as unknown as Parameters[0] ctx.agents.register(child) ctx.emit('subagent/start', { provider: 'p', id: AgentId('child-x'), agentType: 'r' }) - await new Promise(r => setTimeout(r, 80)) + await waitFor(() => injected.includes('child guidance')) expect(injected).toContain('child guidance') }) @@ -193,7 +202,7 @@ describe('hooks-claude coverage — Stop continuation + subagent inject/catch', const child = { id: AgentId('child-y'), inject: () => { throw new Error('inject boom') }, session: { header: { id: 'child-y' } } } as unknown as Parameters[0] ctx.agents.register(child) ctx.emit('subagent/start', { provider: 'p', id: AgentId('child-y') }) - await new Promise(r => setTimeout(r, 80)) + await waitFor(() => warn.mock.calls.some(c => String(c[0]).includes('SubagentStart hook failed'))) expect(warn).toHaveBeenCalledWith(expect.stringContaining('SubagentStart hook failed')) }) }) @@ -238,7 +247,7 @@ describe('hooks-claude coverage — default reasons + sparse payloads', () => { const path = hooks(d, { SubagentStop: [{ hooks: [{ type: 'command', command: s }] }] }) const ctx = await harness(path, new MockAdapter([])) ctx.emit('subagent/end', { provider: 'p', id: AgentId('child-z'), stopReason: 'completed' }) // no agentType - await new Promise(r => setTimeout(r, 80)) + await waitFor(() => existsSync(marker)) expect(existsSync(marker)).toBe(true) }) }) @@ -307,7 +316,6 @@ describe('hooks-claude coverage — schema-bypass default + unspawnable hook', ( // Direct apply with only configPath — bypasses schemastery's defaults, so the // runtime `defaultTimeoutMs ?? 600_000` fallback is exercised. HooksClaude.apply(ctx, { configPath: join(d, 'hooks.json') }) - await new Promise(r => setTimeout(r, 10)) ctx.llm.registerAdapter(['mock'], adapter) const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) agent.send([{ type: 'text', text: 'go' }]) @@ -438,7 +446,7 @@ describe('hooks-claude coverage — detached-listener catch handlers', () => { const original = agent.inject.bind(agent) let threw = false agent.inject = (() => { threw = true; throw new Error('inject boom') }) - await new Promise(r => setTimeout(r, 80)) + await waitFor(() => threw) expect(threw).toBe(true) agent.inject = original agent.send([{ type: 'text', text: 'go' }]) diff --git a/packages/hooks/hooks-codex/tests/coverage.spec.ts b/packages/hooks/hooks-codex/tests/coverage.spec.ts index d10f2d7df1..c32d0b3b91 100644 --- a/packages/hooks/hooks-codex/tests/coverage.spec.ts +++ b/packages/hooks/hooks-codex/tests/coverage.spec.ts @@ -36,6 +36,15 @@ function waitForIdle(ctx: Context, agent: ReactLoopAgent): Promise { return new Promise((resolve) => { const d = ctx.on('agent/status', (s, st) => { if (s === agent && st === 'idle') { d(); resolve() } }) }) } function events(agent: ReactLoopAgent): SessionEvent[] { return [...agent.session.events] } +/** Poll until `predicate` holds or the deadline passes — robust to detached + * emit-listener hooks firing on a `.then` (a fixed sleep flakes under load). */ +async function waitFor(predicate: () => boolean, timeout = 5000, interval = 10): Promise { + const deadline = Date.now() + timeout + while (!predicate()) { + if (Date.now() > deadline) throw new Error('waitFor: condition not met before deadline') + await new Promise(r => setTimeout(r, interval)) + } +} describe('hooks-codex coverage — decision mapping paths', () => { it('UserPromptSubmit block (exit 2) → rejected turn; default reason on empty stderr', async () => { @@ -66,7 +75,8 @@ describe('hooks-codex coverage — decision mapping paths', () => { const adapter = new MockAdapter([textResponse('ok')]) const ctx = await harness(join(d, 'hooks.json'), adapter) const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) - await new Promise(r => setTimeout(r, 60)) + await waitFor(() => events(agent).some(e => e.type === 'context/message' + && e.data.content.some(b => b.type === 'text' && b.text.includes('start-ctx')))) agent.send([{ type: 'text', text: 'go' }]); await waitForIdle(ctx, agent) expect(JSON.stringify(adapter.requests[0]!.messages)).toContain('start-ctx') }) @@ -148,7 +158,6 @@ describe('hooks-codex coverage — decision mapping paths', () => { ctx.logger.warn = warn as never // Direct apply (schema bypass) → defaultTimeoutMs ?? 600_000 + model ?? '' fallbacks. HooksCodex.apply(ctx, { configPath: join(d, 'hooks.json') }) - await new Promise(r => setTimeout(r, 10)) ctx.llm.registerAdapter(['mock'], adapter) const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) agent.send([{ type: 'text', text: 'go' }]); await waitForIdle(ctx, agent) @@ -174,7 +183,8 @@ describe('hooks-codex coverage — decision mapping paths', () => { const adapter = new MockAdapter([textResponse('ok')]) const ctx = await harness(join(d, 'hooks.json'), adapter) const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) - await new Promise(r => setTimeout(r, 60)) + // A completed turn proves session-start already ran; the clean no-output hook + // injected nothing, so no context/message exists. agent.send([{ type: 'text', text: 'go' }]); await waitForIdle(ctx, agent) expect(events(agent).some(e => e.type === 'context/message')).toBe(false) }) @@ -187,7 +197,7 @@ describe('hooks-codex coverage — decision mapping paths', () => { const warn = vi.fn(); ctx.logger.warn = warn as never const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) agent.inject = (() => { throw new Error('inject boom') }) - await new Promise(r => setTimeout(r, 60)) + await waitFor(() => warn.mock.calls.some(c => String(c[0]).includes('SessionStart hook failed'))) expect(warn).toHaveBeenCalledWith(expect.stringContaining('SessionStart hook failed')) }) @@ -343,7 +353,8 @@ describe('hooks-codex coverage — decision mapping paths', () => { const adapter = new MockAdapter([textResponse('ok')]) const ctx = await harness(join(d, 'hooks.json'), adapter) const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) - await new Promise(r => setTimeout(r, 60)) + await waitFor(() => events(agent).some(e => e.type === 'context/message' + && e.data.content.some(b => b.type === 'text' && b.text.includes('session preamble')))) agent.send([{ type: 'text', text: 'go' }]); await waitForIdle(ctx, agent) expect(JSON.stringify(adapter.requests[0]!.messages)).toContain('session preamble') }) From 4da2b99bc32b0f884855a5fd14bb8b10de2177c5 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Wed, 1 Jul 2026 12:03:23 +0800 Subject: [PATCH 05/10] =?UTF-8?q?fix(hooks-codex):=20gate=20plain-stdout?= =?UTF-8?q?=E2=86=92context=20on=20a=20clean=20exit;=20harden=20HMR=20+=20?= =?UTF-8?q?absence=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-2 Codex review of the round-1 fixes: - (A) The Codex plain-stdout→additionalContext fold (F1) was not gated on exit code, so a NON-clean hook's stdout still injected: a SessionStart `echo stale; exit 2` (an emit — cannot block) wrongly injected "stale", and a UserPromptSubmit `exit 1` (non-blocking error → falls through to context) did too. Gate the fold on `output.exitCode === 0`, matching the codec's own structured-stdout rule. Guard tests for both paths, proven red without the gate. - (B) The Codex "SessionStart no-context no-op" absence test was unsound (a completed turn doesn't prove the detached hook finished). It now touches a marker and waitFor()s it before asserting no context. - (B) Both HMR tests used a no-op `true` hook, so a leaked listener would still pass. They now use a BLOCKING (exit 2) UserPromptSubmit hook and assert the post-dispose turn is NOT blocked and logs no hook/invoked — a leaked listener fails loudly. --- .../hooks/hooks-claude/tests/bridge.spec.ts | 22 +++++++--- packages/hooks/hooks-codex/src/index.ts | 14 ++++--- .../hooks/hooks-codex/tests/bridge.spec.ts | 12 ++++-- .../hooks/hooks-codex/tests/coverage.spec.ts | 40 +++++++++++++++++-- 4 files changed, 72 insertions(+), 16 deletions(-) diff --git a/packages/hooks/hooks-claude/tests/bridge.spec.ts b/packages/hooks/hooks-claude/tests/bridge.spec.ts index 0cc4458f52..0b6afd7e4c 100644 --- a/packages/hooks/hooks-claude/tests/bridge.spec.ts +++ b/packages/hooks/hooks-claude/tests/bridge.spec.ts @@ -322,17 +322,29 @@ describe('hooks-claude bridge — load resilience', () => { }) it('disposing the bridge fiber removes its listeners (HMR safety)', async () => { - const dir = writeConfig({ UserPromptSubmit: [{ hooks: [{ type: 'command', command: 'true' }] }] }) + // A BLOCKING UserPromptSubmit hook: if the listener leaked past dispose it + // would veto the prompt (0 model requests) and log a hook/invoked. Build the + // ctx WITHOUT the harness's own bridge mount so this is the ONLY mount, then + // dispose it — a leaked listener fails the test (a no-op `true` hook would + // pass even leaked, so it proved nothing). + const dir = writeConfig({ UserPromptSubmit: [{ hooks: [{ type: 'command', command: 'exit 2' }] }] }) const adapter = new MockAdapter([textResponse('ok')]) - const ctx = await harness(dir, adapter) + const ctx = new Context() + await ctx.plugin(LlmService) + await ctx.plugin(SessionStore) + await ctx.plugin(SystemPrompt) + await ctx.plugin(ToolRegistry) + await ctx.plugin(AgentRegistry) + await ctx.plugin(AgentLoop, { agents: [] }) + await ctx.plugin(LocalBashExecutor, { timeoutMs: 10_000 }) const fiber = await ctx.plugin(HooksClaude, { configPath: join(dir, 'hooks.json') }) await fiber.dispose() - // After disposing this second mount, the FIRST mount's listeners still work, - // but the disposed one contributed none — assert no leaked listener throws. + ctx.llm.registerAdapter(['mock'], adapter) const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) agent.send([{ type: 'text', text: 'go' }]) await waitForIdle(ctx, agent) - expect(adapter.requests.length).toBeGreaterThanOrEqual(1) + expect(adapter.requests).toHaveLength(1) // not blocked → the listener is gone + expect(events(agent).some(e => e.type === 'hook/invoked')).toBe(false) // no hook ran }) it('has the namespace-plugin export shape (no stray default) so the Loader keeps name/inject/apply', () => { diff --git a/packages/hooks/hooks-codex/src/index.ts b/packages/hooks/hooks-codex/src/index.ts index e3e2ac020b..3ef1d2806a 100644 --- a/packages/hooks/hooks-codex/src/index.ts +++ b/packages/hooks/hooks-codex/src/index.ts @@ -110,14 +110,18 @@ export function apply(ctx: Context, config: Config): void { // Discard a `hookSpecificOutput` block naming a different event. expectedEventName: point, }, () => performance.now()) - // Codex's SessionStart/UserPromptSubmit treat a clean hook's PLAIN + // Codex's SessionStart/UserPromptSubmit treat a CLEAN hook's PLAIN // (non-JSON) stdout as additionalContext. The codec keeps that raw text on // `output.stdout` but only sets `additionalContext` from a JSON // `hookSpecificOutput`, so fold plain stdout in here and let the shared - // merge + contextFrom path carry it. Guarded on the codec's own JSON gate - // (stdout starting with `{`) so a structured hook's raw JSON is never - // injected as prose, and it never clobbers an explicit additionalContext. - if (opts.plainStdoutAsContext === true && output.additionalContext === undefined + // merge + contextFrom path carry it. Gated exactly like the codec's own + // structured-stdout parse: only on a clean `exitCode === 0` (a non-zero + // exit is an error, not context — an `echo x; exit 2` must not inject + // `x`), only when stdout is non-JSON (`!startsWith('{')` — a structured + // hook's raw JSON is never dumped as prose), and never clobbering an + // explicit additionalContext from a JSON block. + if (opts.plainStdoutAsContext === true && output.exitCode === 0 + && output.additionalContext === undefined && output.stdout.length > 0 && !output.stdout.startsWith('{')) { output.additionalContext = output.stdout } diff --git a/packages/hooks/hooks-codex/tests/bridge.spec.ts b/packages/hooks/hooks-codex/tests/bridge.spec.ts index f62fa4d66c..0148da104e 100644 --- a/packages/hooks/hooks-codex/tests/bridge.spec.ts +++ b/packages/hooks/hooks-codex/tests/bridge.spec.ts @@ -132,9 +132,14 @@ describe('hooks-codex bridge', () => { expect(adapter.requests).toHaveLength(1) }) - it('disposing the bridge fiber is clean (HMR safety)', async () => { + it('disposing the bridge fiber removes its listeners (HMR safety)', async () => { const dir = configDir() - writeHooks(dir, { UserPromptSubmit: [{ hooks: [{ type: 'command', command: 'true' }] }] }) + // A BLOCKING UserPromptSubmit hook: if the listener leaked past dispose, it + // would veto the prompt (0 model requests) and log a hook/invoked. After a + // clean dispose the turn must proceed untouched — this fails loudly on a leak + // (a no-op `true` hook would pass even with a leaked listener). + const deny = script(dir, 'deny.sh', '#!/usr/bin/env bash\nexit 2\n') + writeHooks(dir, { UserPromptSubmit: [{ hooks: [{ type: 'command', command: deny }] }] }) const adapter = new MockAdapter([textResponse('ok')]) const ctx = new Context() await ctx.plugin(LlmService) @@ -150,7 +155,8 @@ describe('hooks-codex bridge', () => { const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) agent.send([{ type: 'text', text: 'go' }]) await waitForIdle(ctx, agent) - expect(adapter.requests).toHaveLength(1) + expect(adapter.requests).toHaveLength(1) // not blocked → the listener is gone + expect(events(agent).some(e => e.type === 'hook/invoked')).toBe(false) // no hook ran }) it('has the namespace-plugin export shape (no stray default) so the Loader keeps name/inject/apply', () => { diff --git a/packages/hooks/hooks-codex/tests/coverage.spec.ts b/packages/hooks/hooks-codex/tests/coverage.spec.ts index c32d0b3b91..bc4008ab1e 100644 --- a/packages/hooks/hooks-codex/tests/coverage.spec.ts +++ b/packages/hooks/hooks-codex/tests/coverage.spec.ts @@ -179,12 +179,15 @@ describe('hooks-codex coverage — decision mapping paths', () => { it('SessionStart with no additionalContext is a no-op (contextFrom empty)', async () => { const d = dir() - hooks(d, { SessionStart: [{ hooks: [{ type: 'command', command: sh(d, 's.sh', '#!/usr/bin/env bash\nexit 0\n') }] }] }) + // The hook touches a marker so we can wait for it to ACTUALLY FINISH before + // asserting absence — a completed turn alone would not prove the detached + // session-start hook ran, making the absence check a false pass. + const marker = join(d, 'ss-ran') + hooks(d, { SessionStart: [{ hooks: [{ type: 'command', command: sh(d, 's.sh', `#!/usr/bin/env bash\ntouch "${marker}"\nexit 0\n`) }] }] }) const adapter = new MockAdapter([textResponse('ok')]) const ctx = await harness(join(d, 'hooks.json'), adapter) const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) - // A completed turn proves session-start already ran; the clean no-output hook - // injected nothing, so no context/message exists. + await waitFor(() => existsSync(marker)) // the clean no-output hook has finished agent.send([{ type: 'text', text: 'go' }]); await waitForIdle(ctx, agent) expect(events(agent).some(e => e.type === 'context/message')).toBe(false) }) @@ -347,6 +350,37 @@ describe('hooks-codex coverage — decision mapping paths', () => { expect(JSON.stringify(adapter.requests[0]!.messages)).toContain('extra guidance from a plain hook') }) + it('a NON-clean SessionStart hook (exit 2) does NOT inject its stdout as context', async () => { + // The plain-stdout→context fold is gated on exitCode === 0, matching the + // codec's structured-stdout rule. SessionStart is an EMIT (cannot block), so + // an `echo stale; exit 2` here is the exact case the gate guards: without it, + // the non-clean hook's stdout would wrongly inject "stale". A marker lets us + // wait for the detached hook to finish before asserting absence. + const d = dir() + const marker = join(d, 'ran') + hooks(d, { SessionStart: [{ hooks: [{ type: 'command', command: sh(d, 'b.sh', `#!/usr/bin/env bash\ntouch "${marker}"\necho "stale"\nexit 2\n`) }] }] }) + const adapter = new MockAdapter([textResponse('ok')]) + const ctx = await harness(join(d, 'hooks.json'), adapter) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + await waitFor(() => existsSync(marker)) // the exit-2 hook has finished + expect(events(agent).some(e => e.type === 'context/message' + && e.data.content.some(b => b.type === 'text' && b.text.includes('stale')))).toBe(false) + }) + + it('a UserPromptSubmit hook with a non-blocking error exit (1) + stdout does NOT inject it', async () => { + // Exit 1 is a non-blocking error (no decision), so the prompt is NOT blocked + // and the handler falls through to the context path — the gate must still + // suppress the error hook's stdout ("stale" never reaches the model). + const d = dir() + hooks(d, { UserPromptSubmit: [{ hooks: [{ type: 'command', command: sh(d, 'e.sh', '#!/usr/bin/env bash\necho "stale"\nexit 1\n') }] }] }) + const adapter = new MockAdapter([textResponse('ok')]) + const ctx = await harness(join(d, 'hooks.json'), adapter) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]); await waitForIdle(ctx, agent) + expect(adapter.requests).toHaveLength(1) // exit 1 is non-blocking → the turn ran + expect(JSON.stringify(adapter.requests[0]!.messages)).not.toContain('stale') + }) + it('a clean SessionStart hook that prints PLAIN stdout injects it (not JSON)', async () => { const d = dir() hooks(d, { SessionStart: [{ hooks: [{ type: 'command', command: sh(d, 'ss.sh', '#!/usr/bin/env bash\necho "session preamble"\nexit 0\n') }] }] }) From 09c8e549b060f5fe90a1d2a8354a2ea19b48b2e3 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Wed, 1 Jul 2026 16:34:28 +0800 Subject: [PATCH 06/10] fix(hooks): run hooks in the session cwd; honest process-level config + best-effort session-start; surface systemMessage drop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review on the bridges: - Hook cwd (blocking): the bridges never passed a workdir to runHook, so hooks ran in the executor default (the ACP server launch dir), not the session cwd — a hook doing `pwd`/relative reads/marker writes operated in the wrong tree. Both bridges now thread the agent's session `header.cwd` (the session/new.cwd) as the hook workdir for agent-scoped points. Regression per bridge: server cwd ≠ session cwd, a `pwd` hook proves it ran in the session workspace (proven red without the workdir). - Example config honesty (blocking): `configPath: ./hooks.json` is read ONCE at load against the PROCESS cwd, not per-session — the comment/README now say so explicitly (a project-local per-session hooks.json is not discovered; TODO(per-session-hook-config)). The hooks-run-in-session-cwd fix above is the distinct, separately-documented half. - Session-start timing (blocking): agent/session-start is a synchronous emit and the hook runs on a detached .then, so injected context is BEST-EFFORT — not guaranteed before the first request. Downgrade the contract in code comments + README + RFC (TODO(session-start-gating)) rather than implying "first request sees it", and add a no-wait regression that asserts the safe properties without pre-waiting for the inject. - systemMessage (non-blocking): the merge collects merged.systemMessages but no bridge surfaced it. Warn per hook (like updatedInput) and document it as deferred in both READMEs + the RFC; tests assert the warn + non-surfacing. --- .../feature/2026-06-30-hook-bridges.md | 7 +- examples/acp-agent/cordis.snapshot.yml | 13 ++-- examples/acp-agent/cordis.yml | 15 ++-- packages/hooks/hooks-claude/README.md | 5 +- packages/hooks/hooks-claude/src/index.ts | 27 ++++++- .../hooks/hooks-claude/tests/coverage.spec.ts | 76 +++++++++++++++++++ packages/hooks/hooks-codex/README.md | 6 +- packages/hooks/hooks-codex/src/index.ts | 19 ++++- .../hooks/hooks-codex/tests/coverage.spec.ts | 37 +++++++++ 9 files changed, 189 insertions(+), 16 deletions(-) diff --git a/docs/rfc/implemented/feature/2026-06-30-hook-bridges.md b/docs/rfc/implemented/feature/2026-06-30-hook-bridges.md index 1e782f9b67..d6aea68b17 100644 --- a/docs/rfc/implemented/feature/2026-06-30-hook-bridges.md +++ b/docs/rfc/implemented/feature/2026-06-30-hook-bridges.md @@ -39,13 +39,18 @@ Each bridge maps the neutral `MergedHookOutcome` from the shared lib onto the se The config is parsed ONCE at load; a read/parse failure logs and registers nothing rather than crashing boot (a typo'd path must not take the agent down). Only `type: 'command'` hooks run — a `prompt`/`agent`/HTTP hook (CC) or an `async: true` / non-command hook (Codex) is parsed-and-skipped with a warning. The emit-listener paths (`session-start`, `subagent/start`) run detached, with their `inject` contained in a `.catch` that logs (a throwing inject must not break session boot or the loop). +### Where hooks run, and where their config comes from + +Two different cwds, kept distinct on purpose. The hooks **themselves** run in the agent's **session workspace**: for the agent-scoped points the bridge threads the session's `cwd` (`session/new.cwd`, on the session header) to `runHook` as the process working directory, so a hook's `pwd` / relative-file read / marker write operates in the user's project tree, not the server's launch directory. The **config path**, by contrast, is **process-level**: `configPath` is resolved and parsed once at load against the process launch cwd, so a single `hooks.json` applies to the whole process — there is no per-session config discovery that reads a project-local `hooks.json` from each `session/new.cwd` (`TODO(per-session-hook-config)`). This is an honest limitation of the current cut: the example `cordis.yml` documents that its `./hooks.json` is process-level, not per-project. + ## Deferred (faithful-but-degraded) - **Tool-input rewrite.** A CC/Codex `updatedInput` is logged + warned, not honored — input rewrite is a deferred consistency-design problem ([the pre-tool-input-rewrite RFC](../../proposed/feature/2026-06-30-pre-tool-input-rewrite.md)), because the pre-execution args are read by `tool/call` audit + `assistant/message` history + ACP/tool-bash presentation, so an honest rewrite is a design unit, not a field. - **Stop loop-guard** (`TODO(stop-loop-guard)`). CC/Codex break an infinite force-continue with `stop_hook_active` (true once a Stop hook fired this run) plus a max-consecutive cap; both are deferred. Today `stop_hook_active` is always `false`, so a Stop hook that unconditionally blocks would force-continue every step — a hook author must self-limit until the guard lands. - **Permission `ask`** degrades to `deny` at the `tools/pre-execute` seam (`FIXME(permissions)` in the interception-seams RFC) — there is no interactive permission prompt yet. - **Hook `continue:false` (hard halt).** A hook can ask to halt the whole run (CC/Codex `continue:false`); the shared merge folds it into `MergedHookOutcome.stop`/`stopReason`, but no bridge acts on it (`TODO(hook-continue-false)`) — the interception seams have no "hard-halt the agent" primitive yet (a Decision blocks/steers a single point, not the run). Deferred with the loop-guard work; the halt request is recorded in the `hook/result` log, and the hook keeps its per-point effect (decision/context) meanwhile. -- **Config discovery.** The path is explicit in `cordis.yml`; the full multi-layer CC/Codex precedence walk and the trust/hash model are not reimplemented (`TODO`). +- **Config discovery.** The path is explicit in `cordis.yml` and process-level (see above); the full multi-layer CC/Codex precedence walk, per-session project-local discovery, and the trust/hash model are not reimplemented (`TODO(per-session-hook-config)`). +- **Session-start / subagent-start context is best-effort, not gated (`TODO(session-start-gating)`).** `agent/session-start` is a synchronous emit and the bridge runs its hook on a detached `.then`, so the injected `additionalContext` is not guaranteed to land before the first turn reaches the model — a slow hook can miss the first request (the context then arrives as a later injection). `subagent/start` is sharper: an in-process provider may have already queued the child's prompt before the listener runs, and a short-lived child can finish before the detached inject fires. Making startup context a gated/awaited primitive is a loop-level change deferred to the interception seams; today the contract is "injected as soon as the hook resolves", not "before the first request". The bridge tests do NOT wait on the injection where they assert the guaranteed-timing behavior, so they document the real (best-effort) timing rather than masking it. ### Multiple hooks on one point run serially, not concurrently diff --git a/examples/acp-agent/cordis.snapshot.yml b/examples/acp-agent/cordis.snapshot.yml index 329f9dc858..aa65e540b4 100644 --- a/examples/acp-agent/cordis.snapshot.yml +++ b/examples/acp-agent/cordis.snapshot.yml @@ -86,11 +86,14 @@ - id: tool-todo name: '@deepseek-ai/dsh-tool-todo' -# The Claude Code hook bridge, pointed at a `hooks.json` in the session cwd. A -# scenario that ships `workspace/hooks.json` (copied into the cwd before the run) -# exercises the hooks path end-to-end; every other scenario has no such file, so -# the bridge's parse fails-soft and it registers nothing (a silent no-op — the -# ACP app loads no logger exporter, so the warning never reaches stdout). +# The Claude Code hook bridge. `configPath` is read ONCE at load and resolves +# `./hooks.json` against the PROCESS cwd (not per-session) — in these snapshot +# runs the harness launches the subprocess with process cwd = the scenario's temp +# workspace, so a scenario that ships `workspace/hooks.json` (copied into that cwd +# before the run) exercises the hooks path end-to-end; every other scenario has no +# such file, so the parse fails-soft and the bridge registers nothing (a silent +# no-op — the ACP app loads no logger exporter, so the warning never reaches +# stdout). Hooks themselves run in the session cwd (the bridge passes it as workdir). - id: hooks-claude name: '@deepseek-ai/dsh-hooks-claude' config: diff --git a/examples/acp-agent/cordis.yml b/examples/acp-agent/cordis.yml index e483d9ee31..8ade4ce847 100644 --- a/examples/acp-agent/cordis.yml +++ b/examples/acp-agent/cordis.yml @@ -96,11 +96,16 @@ - id: tool-todo name: '@deepseek-ai/dsh-tool-todo' -# The Claude Code hook bridge, pointed at a `hooks.json` in the session cwd. With -# no such file present the parse fails-soft and the bridge registers nothing (a -# silent no-op); a session whose cwd holds a `hooks.json` runs those hooks on the -# interception seams. stdout is the ACP JSON-RPC channel — the bridge's warnings -# go through ctx.logger (no exporter here), never to stdout. +# The Claude Code hook bridge. `configPath` is PROCESS-LEVEL: it is read ONCE at +# load and the relative `./hooks.json` resolves against the ACP server's launch +# cwd, NOT each `session/new.cwd`. So a single `hooks.json` next to where the +# server starts applies to every session; a project-local, per-session hooks.json +# is NOT discovered (per-session config resolution is a TODO — see the bridge +# README). With no file present the parse fails-soft and the bridge registers +# nothing (a silent no-op). Hooks THEMSELVES run in the session cwd (the bridge +# passes it as the workdir); only WHERE the config is read from is process-level. +# stdout is the ACP JSON-RPC channel — the bridge's warnings go through ctx.logger +# (no exporter here), never to stdout. - id: hooks-claude name: '@deepseek-ai/dsh-hooks-claude' config: diff --git a/packages/hooks/hooks-claude/README.md b/packages/hooks/hooks-claude/README.md index 126573c6be..82980ff601 100644 --- a/packages/hooks/hooks-claude/README.md +++ b/packages/hooks/hooks-claude/README.md @@ -25,7 +25,9 @@ In a `cordis.yml`: projectDir: . ``` -The config is parsed **once** at load. A read/parse failure is contained — the bridge logs a warning and registers nothing rather than crashing boot (a typo'd path must not take the agent down). Only `type: 'command'` hooks run; a `prompt`/`agent`/HTTP hook is parsed-and-skipped with a warning. +The config is parsed **once** at load. `configPath` is **process-level**: a relative path resolves against the process's launch cwd at load time, so a single config applies to the whole process — there is no per-session (`session/new.cwd`) config discovery yet (`TODO(per-session-hook-config)`). A read/parse failure is contained — the bridge logs a warning and registers nothing rather than crashing boot (a typo'd path must not take the agent down). Only `type: 'command'` hooks run; a `prompt`/`agent`/HTTP hook is parsed-and-skipped with a warning. + +The hooks **themselves** run in the agent's session workspace: for the agent-scoped points the bridge passes the session's `cwd` (the `session/new.cwd`) as the hook process's working directory, so a hook's `pwd`/relative-path/marker operates in the user's project tree, not the server launch dir. ## Hook points → seam Decisions @@ -48,4 +50,5 @@ Injected context carries an explicit `{ kind: 'plugin', plugin: 'hooks-claude' } ## Deferred (faithful-but-degraded) - **`updatedInput` (tool-input rewrite)** is logged + warned, **not honored** — input rewrite is a deferred consistency-design problem ([the pre-tool-input-rewrite RFC](../../../docs/rfc/proposed/feature/2026-06-30-pre-tool-input-rewrite.md)). +- **`systemMessage`** (a hook's user-facing warning) is logged + warned, **not surfaced** — there is no user-message channel on these seams yet (only model-facing `additionalContext`). The shared merge collects it; the bridge does not yet render it. - **Stop loop-guard.** CC breaks an infinite force-continue with `stop_hook_active` (true once a Stop hook has fired this run) plus a max-consecutive cap; both are deferred (`TODO(stop-loop-guard)`). Today `stop_hook_active` is always `false`, so a Stop hook that unconditionally blocks would force-continue every step — a hook author must self-limit until the guard lands. diff --git a/packages/hooks/hooks-claude/src/index.ts b/packages/hooks/hooks-claude/src/index.ts index 74b1e6f5c0..5b56e5f014 100644 --- a/packages/hooks/hooks-claude/src/index.ts +++ b/packages/hooks/hooks-claude/src/index.ts @@ -51,7 +51,13 @@ export const inject = ['bash'] /** Plugin config: where the CC hook config lives + substitution roots. */ export interface Config { - /** Path to a `hooks.json` or a settings file whose `hooks` key holds the config. */ + /** + * Path to a `hooks.json` or a settings file whose `hooks` key holds the config. + * PROCESS-LEVEL: read once at load, a relative path resolves against the process + * launch cwd, so one config applies to the whole process. + * TODO(per-session-hook-config): per-session discovery of a project-local + * `hooks.json` from each `session/new.cwd` is not yet implemented. + */ configPath: string /** Replaces `${CLAUDE_PLUGIN_ROOT}` in command strings (the plugin's root dir). */ pluginRoot?: string @@ -124,6 +130,12 @@ export function apply(ctx: Context, config: Config): void { ): Promise { const groups: MatcherGroup[] = parsed[point] ?? [] const outputs: HookOutput[] = [] + // Run the hook in the AGENT'S session workspace (the `session/new` cwd on the + // session header), not the executor default (the ACP server's launch dir). + // A hook that does `pwd`, reads a relative file, or writes a marker must + // operate in the user's project tree. Absent for a no-agent run (falls back + // to the executor default). + const workdir = opts.agent?.session.header.cwd for (const group of groups) { if (!matchesMatcher(group.matcher, matchQuery, 'claude')) continue for (const hook of group.hooks) { @@ -138,6 +150,7 @@ export function apply(ctx: Context, config: Config): void { const { output, durationMs } = await runHook(ctx.bash, hook, { payload, ...hookEnv ? { env: hookEnv } : {}, + ...workdir !== undefined ? { cwd: workdir } : {}, ...opts.signal ? { signal: opts.signal } : {}, defaultTimeoutMs, trailingNewline: true, @@ -149,6 +162,9 @@ export function apply(ctx: Context, config: Config): void { if (output.updatedInput !== undefined) { ctx.logger.warn(`hooks-claude: ${point} hook requested updatedInput, which is not yet honored (ignored)`) } + if (output.systemMessage !== undefined) { + ctx.logger.warn(`hooks-claude: ${point} hook emitted a systemMessage, which is not yet surfaced (ignored)`) + } if (session && opts.turn !== undefined) { const stderrSummary = summarize(output.stderr) appendHookResult(session, { @@ -180,7 +196,14 @@ export function apply(ctx: Context, config: Config): void { } // --- SessionStart: emit (cannot block). Inject any additionalContext into the - // agent so the first request sees it. The matcher subject is the source. --- + // agent. The matcher subject is the source. + // TODO(session-start-gating): `agent/session-start` is a SYNCHRONOUS emit and + // this hook runs on a detached `.then`, so the injected context is BEST-EFFORT + // — it is not guaranteed to land before the first turn reaches the model. A + // slow hook can miss the first request (the context then arrives as a later + // injection turn). Gating startup on the hook is a loop-level change deferred + // to the interception seams; today the contract is "injected as soon as the + // hook resolves", not "before the first request". --- ctx.on('agent/session-start', (agent, source) => { void runPoint('SessionStart', source, sessionStartPayload(agent, source), { agent }) .then((merged) => { diff --git a/packages/hooks/hooks-claude/tests/coverage.spec.ts b/packages/hooks/hooks-claude/tests/coverage.spec.ts index ca3a143deb..a1b650f33d 100644 --- a/packages/hooks/hooks-claude/tests/coverage.spec.ts +++ b/packages/hooks/hooks-claude/tests/coverage.spec.ts @@ -454,3 +454,79 @@ describe('hooks-claude coverage — detached-listener catch handlers', () => { expect(adapter.requests).toHaveLength(1) // loop survived the thrown inject }) }) + +describe('hooks-claude coverage — hook runs in the session cwd, not the server cwd', () => { + it('runs an agent-scoped hook in the session workspace even when the executor default differs', async () => { + // The bug: the bridge passed no workdir, so hooks ran in the executor default + // (the server launch dir), not session/new.cwd. Here the executor default and + // the session cwd are DIFFERENT temp dirs; a PreToolUse hook writes `pwd` to a + // marker and we assert it ran in the SESSION cwd. + const serverDir = dir() + const sessionDir = dir() + const marker = join(sessionDir, 'where') + // The hook is invoked with cwd = session dir, so a relative marker path lands there. + hooks(serverDir, { PreToolUse: [{ hooks: [{ type: 'command', command: 'pwd > where' }] }] }) + const adapter = new MockAdapter([toolCallResponse('c1', 'echo', {}), textResponse('done')]) + const ctx = new Context() + await ctx.plugin(LlmService) + await ctx.plugin(SessionStore) + await ctx.plugin(SystemPrompt) + await ctx.plugin(ToolRegistry) + await ctx.plugin(AgentRegistry) + await ctx.plugin(AgentLoop, { agents: [] }) + // Executor default cwd = serverDir (deliberately NOT the session cwd). + await ctx.plugin(LocalBashExecutor, { timeoutMs: 10_000, cwd: serverDir }) + await ctx.plugin(HooksClaude, { configPath: join(serverDir, 'hooks.json') }) + ctx.llm.registerAdapter(['mock'], adapter) + ctx.tools.register(defineTool({ name: 'echo', description: 'e', parameters: {}, async execute() { return [{ type: 'text', text: 'ok' }] } })) + + const { SessionId } = await import('@deepseek-ai/dsh-session') + const handle = ctx.agents.create({ agentId: AgentId('a1'), sessionId: SessionId('s1'), meta: { cwd: sessionDir }, agentOptions: { model: 'mock' } }) + handle.agent.send([{ type: 'text', text: 'go' }]) + await waitForIdle(ctx, handle.agent as ReactLoopAgent) + + expect(existsSync(marker)).toBe(true) // the marker landed in the SESSION dir + const { readFileSync } = await import('node:fs') + const where = readFileSync(marker, 'utf8').trim() + // `pwd` may resolve symlinks (/var → /private/var etc.), so compare basenames. + expect(where.endsWith(sessionDir.split('/').pop()!)).toBe(true) + await handle.dispose() + }) +}) + +describe('hooks-claude coverage — systemMessage is warned, not surfaced', () => { + it('a hook emitting a systemMessage is logged as not-yet-surfaced', async () => { + const d = dir() + const s = sh(d, 'sm.sh', '#!/usr/bin/env bash\necho \'{"systemMessage":"heads up"}\'\n') + const path = hooks(d, { UserPromptSubmit: [{ hooks: [{ type: 'command', command: s }] }] }) + const adapter = new MockAdapter([textResponse('ok')]) + const ctx = await harness(path, adapter) + const warn = vi.fn(); ctx.logger.warn = warn as never + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]) + await waitForIdle(ctx, agent) + expect(warn).toHaveBeenCalledWith(expect.stringContaining('systemMessage')) + // Not surfaced: the systemMessage text never reaches the model request. + expect(JSON.stringify(adapter.requests[0]!.messages)).not.toContain('heads up') + }) +}) + +describe('hooks-claude coverage — SessionStart timing is best-effort (no-wait)', () => { + it('does NOT crash or block when the prompt is sent immediately (context is best-effort, may miss the first request)', async () => { + // Regression for the documented downgrade: session-start injection is + // detached, so a prompt sent immediately need not observe it. This asserts + // the SAFE properties (no crash, the turn still runs) WITHOUT waiting for the + // inject first — it documents the best-effort timing rather than masking it + // by pre-waiting for context/message (which the guaranteed-timing tests do). + const d = dir() + const s = sh(d, 'start.sh', '#!/usr/bin/env bash\necho \'{"hookSpecificOutput":{"hookEventName":"SessionStart","additionalContext":"late ctx"}}\'\n') + const path = hooks(d, { SessionStart: [{ hooks: [{ type: 'command', command: s }] }] }) + const adapter = new MockAdapter([textResponse('ok')]) + const ctx = await harness(path, adapter) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + // Send immediately — do NOT wait for the session-start inject. + agent.send([{ type: 'text', text: 'go' }]) + await waitForIdle(ctx, agent) + expect(adapter.requests).toHaveLength(1) // the turn ran regardless of hook timing + }) +}) diff --git a/packages/hooks/hooks-codex/README.md b/packages/hooks/hooks-codex/README.md index e20b3a211d..6b28d7d114 100644 --- a/packages/hooks/hooks-codex/README.md +++ b/packages/hooks/hooks-codex/README.md @@ -31,7 +31,9 @@ In a `cordis.yml`: model: deepseek-v4 ``` -The config is parsed **once** at load; a read/parse failure is contained (logs + registers nothing). Only sync `type: 'command'` hooks run — a non-command or `async: true` hook is parsed-and-skipped with a warning. A hook accepts `timeout` or the `timeoutSec` alias. Events outside the five Codex points are dropped at parse. +The config is parsed **once** at load. `configPath` is **process-level** — a relative path resolves against the process launch cwd at load time, not per-session (`TODO(per-session-hook-config)`). A read/parse failure is contained (logs + registers nothing). Only sync `type: 'command'` hooks run — a non-command or `async: true` hook is parsed-and-skipped with a warning. A hook accepts `timeout` or the `timeoutSec` alias. Events outside the five Codex points are dropped at parse. + +The hooks themselves run in the agent's session workspace: for the agent-scoped points the bridge passes the session's `cwd` as the hook process's working directory, so a hook operates in the user's project tree, not the server launch dir. ## Hook points → seam Decisions @@ -52,3 +54,5 @@ Injected context carries an explicit `{ kind: 'plugin', plugin: 'hooks-codex' }` ## Deferred **Stop loop-guard** (`TODO(stop-loop-guard)`): as in CC, a Stop hook that unconditionally blocks would force-continue every step (`stop_hook_active` is always `false` here); the loop-guard is deferred. A hook author must self-limit until it lands. + +**`systemMessage`**: a hook's user-facing warning is logged + warned, not surfaced — there is no user-message channel on these seams yet (only model-facing `additionalContext`). diff --git a/packages/hooks/hooks-codex/src/index.ts b/packages/hooks/hooks-codex/src/index.ts index 3ef1d2806a..c71a0811ea 100644 --- a/packages/hooks/hooks-codex/src/index.ts +++ b/packages/hooks/hooks-codex/src/index.ts @@ -38,7 +38,12 @@ export const inject = ['bash'] /** Plugin config: where the Codex hooks.json lives + the model name for payloads. */ export interface Config { - /** Path to a Codex `hooks.json`. */ + /** + * Path to a Codex `hooks.json`. PROCESS-LEVEL: read once at load, a relative + * path resolves against the process launch cwd. + * TODO(per-session-hook-config): per-session project-local discovery from each + * `session/new.cwd` is not yet implemented. + */ configPath: string /** The model name stamped on every payload (Codex includes `model` on each event). */ model?: string @@ -90,6 +95,10 @@ export function apply(ctx: Context, config: Config): void { ): Promise { const groups: MatcherGroup[] = parsed[point] ?? [] const outputs: HookOutput[] = [] + // Run the hook in the agent's session workspace (the `session/new` cwd), not + // the executor default (the server launch dir) — a hook reading a relative + // file or `pwd` must see the user's project tree. Absent for a no-agent run. + const workdir = opts.agent?.session.header.cwd for (const group of groups) { // Codex matches with PURE regex (no literal fast path). if (!matchesMatcher(group.matcher, matchQuery, 'codex')) continue @@ -104,6 +113,7 @@ export function apply(ctx: Context, config: Config): void { } const { output, durationMs } = await runHook(ctx.bash, hook, { payload, + ...workdir !== undefined ? { cwd: workdir } : {}, ...opts.signal ? { signal: opts.signal } : {}, defaultTimeoutMs, trailingNewline: false, // Codex writes stdin WITHOUT a trailing newline. @@ -126,6 +136,9 @@ export function apply(ctx: Context, config: Config): void { output.additionalContext = output.stdout } outputs.push(output) + if (output.systemMessage !== undefined) { + ctx.logger.warn(`hooks-codex: ${point} hook emitted a systemMessage, which is not yet surfaced (ignored)`) + } if (session && opts.turn !== undefined) { const stderrSummary = summarize(output.stderr) appendHookResult(session, { @@ -154,6 +167,10 @@ export function apply(ctx: Context, config: Config): void { } // SessionStart: emit. Codex passes a plain-stdout hook's output as additionalContext. + // TODO(session-start-gating): a synchronous emit + detached `.then`, so the + // injected context is BEST-EFFORT — not guaranteed before the first turn reaches + // the model (a slow hook can miss the first request). Gating is a deferred + // loop-level change; the contract is "injected as soon as the hook resolves". ctx.on('agent/session-start', (agent, source) => { void runPoint('SessionStart', source, { ...base(agent, 'SessionStart', model), source }, { agent, plainStdoutAsContext: true }) .then((merged) => { diff --git a/packages/hooks/hooks-codex/tests/coverage.spec.ts b/packages/hooks/hooks-codex/tests/coverage.spec.ts index bc4008ab1e..d7a5806fbd 100644 --- a/packages/hooks/hooks-codex/tests/coverage.spec.ts +++ b/packages/hooks/hooks-codex/tests/coverage.spec.ts @@ -436,4 +436,41 @@ describe('hooks-codex coverage — decision mapping paths', () => { expect(ran).toBe(false) // the matcher fired → the hook denied the tool expect(events(agent).some(e => e.type === 'hook/invoked' && e.data.point === 'PreToolUse')).toBe(true) }) + + it('a hook emitting a systemMessage is warned as not-yet-surfaced', async () => { + const d = dir() + hooks(d, { UserPromptSubmit: [{ hooks: [{ type: 'command', command: sh(d, 'sm.sh', '#!/usr/bin/env bash\necho \'{"systemMessage":"heads up"}\'\n') }] }] }) + const adapter = new MockAdapter([textResponse('ok')]) + const ctx = await harness(join(d, 'hooks.json'), adapter) + const warn = vi.fn(); ctx.logger.warn = warn as never + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]); await waitForIdle(ctx, agent) + expect(warn).toHaveBeenCalledWith(expect.stringContaining('systemMessage')) + expect(JSON.stringify(adapter.requests[0]!.messages)).not.toContain('heads up') + }) + + it('runs an agent-scoped hook in the session cwd, not the executor default', async () => { + // Same regression as the CC bridge: the Codex bridge must thread the session + // cwd as the hook workdir. Executor default = serverDir; session cwd = + // sessionDir; the PreToolUse hook's `pwd` marker must land in sessionDir. + const serverDir = dir() + const sessionDir = dir() + const marker = join(sessionDir, 'where') + hooks(serverDir, { PreToolUse: [{ hooks: [{ type: 'command', command: 'pwd > where' }] }] }) + const adapter = new MockAdapter([toolCallResponse('c1', 'Bash', { command: 'x' }), textResponse('done')]) + const ctx = new Context() + await ctx.plugin(LlmService); await ctx.plugin(SessionStore); await ctx.plugin(SystemPrompt) + await ctx.plugin(ToolRegistry); await ctx.plugin(AgentRegistry); await ctx.plugin(AgentLoop, { agents: [] }) + await ctx.plugin(LocalBashExecutor, { timeoutMs: 10_000, cwd: serverDir }) + await ctx.plugin(HooksCodex, { configPath: join(serverDir, 'hooks.json'), model: 'm' }) + ctx.llm.registerAdapter(['mock'], adapter) + ctx.tools.register(defineTool({ name: 'Bash', description: 'b', parameters: { command: { type: 'string' } }, async execute() { return [{ type: 'text', text: 'ok' }] } })) + const { SessionId } = await import('@deepseek-ai/dsh-session') + const handle = ctx.agents.create({ agentId: AgentId('a1'), sessionId: SessionId('s1'), meta: { cwd: sessionDir }, agentOptions: { model: 'mock' } }) + handle.agent.send([{ type: 'text', text: 'go' }]) + await waitForIdle(ctx, handle.agent as ReactLoopAgent) + expect(existsSync(marker)).toBe(true) + expect(readFileSync(marker, 'utf8').trim().endsWith(sessionDir.split('/').pop()!)).toBe(true) + await handle.dispose() + }) }) From bae6141398854280dec9abf42ef037281221bac3 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Thu, 2 Jul 2026 06:45:17 +0800 Subject: [PATCH 07/10] fix(hooks-claude): build subagent payloads from base(), run SubagentStop in the child cwd, drop agentType MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address the D agentType removal + two #124 review findings on the CC bridge's subagent points: - **Payloads from base()**: `subagentStart/StopPayload` bypassed `base()`, so the SubagentStart/SubagentStop stdin payloads omitted the CC-promised `session_id` and `cwd`. Replaced both with a single `subagentPayload()` built from `base(child)` (the child's session_id/cwd when the child is available) + `agent_id` + `agent_type` (+ `stop_hook_active` on Stop). - **SubagentStop runs in the child cwd**: the listener called `runPoint(..., {})` with no agent, so the hook ran in the executor/server cwd. It now looks the child up via `ctx.get('agents').get(info.id)` — still recoverable because `subagent/end` fires from the service's detached `.then` BEFORE the tool caller disposes the child — and passes `{ agent: child }`, matching SubagentStart. New regression: server cwd ≠ child cwd, a `pwd` SubagentStop hook proves it ran in the CHILD workspace (proven red by neutering the lookup). - **agent_type is a constant**: `info.agentType` no longer exists (removed on the subagent branch); both points now report the `SUBAGENT_TYPE = "general-purpose"` constant (Claude Code's Task-tool default), so a hooks.json default/`*`/empty `agent_type` matcher fires. Updated the README matcher-subject note and the bridge/coverage tests (dropped their agentType emits). - **e2e comment**: hooks.e2e.ts said `./hooks.json` loads from the session cwd; corrected to process-level (server launch cwd), with the hook itself running in the session cwd. --- examples/acp-agent/tests/hooks.e2e.ts | 11 ++-- packages/hooks/hooks-claude/README.md | 2 +- packages/hooks/hooks-claude/src/index.ts | 46 +++++++++++++---- .../hooks/hooks-claude/tests/bridge.spec.ts | 4 +- .../hooks/hooks-claude/tests/coverage.spec.ts | 51 ++++++++++++++++--- 5 files changed, 89 insertions(+), 25 deletions(-) diff --git a/examples/acp-agent/tests/hooks.e2e.ts b/examples/acp-agent/tests/hooks.e2e.ts index 19a80df03c..40a8d37457 100644 --- a/examples/acp-agent/tests/hooks.e2e.ts +++ b/examples/acp-agent/tests/hooks.e2e.ts @@ -19,11 +19,14 @@ import { /** * With-key e2e: the Claude Code hook bridge running against the REAL acp-agent * subprocess and the REAL model. The example `cordis.yml` loads `dsh-hooks-claude` - * pointed at `./hooks.json` in the session cwd; this test writes a `hooks.json` - * with a PreToolUse hook that BLOCKS every bash command, then asks the live model - * to write a file — and verifies the WORLD (the file never appears on disk), + * with a PROCESS-LEVEL `configPath` of `./hooks.json`, resolved once at load + * against the ACP server's launch cwd (NOT per-session); this test sets that + * launch cwd to the temp workspace and writes a `hooks.json` there with a + * PreToolUse hook that BLOCKS every bash command, then asks the live model to + * write a file — and verifies the WORLD (the file never appears on disk), * proving the hook actually intercepted execution rather than the agent merely - * claiming it couldn't. Key-gated; owns and disposes its subprocess. + * claiming it couldn't. (The hook itself then runs in the session cwd.) + * Key-gated; owns and disposes its subprocess. * * A keyless companion lives in acp.e2e.ts (stdout purity + session/new); the * full hook-fires-end-to-end transcript is the keyless `hook-prompt-block` diff --git a/packages/hooks/hooks-claude/README.md b/packages/hooks/hooks-claude/README.md index 82980ff601..f9520f69be 100644 --- a/packages/hooks/hooks-claude/README.md +++ b/packages/hooks/hooks-claude/README.md @@ -41,7 +41,7 @@ The hooks **themselves** run in the agent's session workspace: for the agent-sco | `SubagentStart` | `subagent/start` (emit) | additionalContext → `agent.inject()` into the live child | | `SubagentStop` | `subagent/end` (emit) | observe-only | -The matcher subject is the tool name (`PreToolUse`/`PostToolUse`), the session source (`SessionStart`), or the child's agent type (`SubagentStart`/`SubagentStop`); `UserPromptSubmit`/`Stop` ignore matchers. Multiple file-configured hooks on one point run **serially, in config order**, and fold most-restrictively (`deny > ask > allow`, see `dsh-hook-protocol`); serial keeps each hook's `hook/invoked`/`hook/result` pair adjacent in the log, and the fold is order-independent for the decision (see the RFC's "run serially, not concurrently" note). +The matcher subject is the tool name (`PreToolUse`/`PostToolUse`), the session source (`SessionStart`), or a constant `agent_type` of `general-purpose` (`SubagentStart`/`SubagentStop` — the harness subagent seam carries no per-kind label, so the bridge reports Claude Code's own Task-tool default; a default/`*`/empty `agent_type` matcher fires, a specific-kind matcher does not); `UserPromptSubmit`/`Stop` ignore matchers. Multiple file-configured hooks on one point run **serially, in config order**, and fold most-restrictively (`deny > ask > allow`, see `dsh-hook-protocol`); serial keeps each hook's `hook/invoked`/`hook/result` pair adjacent in the log, and the fold is order-independent for the decision (see the RFC's "run serially, not concurrently" note). ## Context source diff --git a/packages/hooks/hooks-claude/src/index.ts b/packages/hooks/hooks-claude/src/index.ts index 5b56e5f014..7e21f7ea82 100644 --- a/packages/hooks/hooks-claude/src/index.ts +++ b/packages/hooks/hooks-claude/src/index.ts @@ -271,10 +271,15 @@ export function apply(ctx: Context, config: Config): void { // --- SubagentStart / SubagentStop: observe-only emits (the subagent seam is // observe-only this cut). A SubagentStart hook's additionalContext is injected - // into the live child; SubagentStop only observes. No matcher subject. --- + // into the live child; SubagentStop only observes. Both look the live child up + // so the hook runs in the child's session workspace and the payload carries + // the child's session_id/cwd (see subagentPayload). The matcher subject is the + // CC-default `agent_type` (SUBAGENT_TYPE) — the harness seam carries no + // per-kind label, so a config's default/`*`/empty agent_type matcher fires and + // a specific-kind matcher does not (documented in the RFC). --- ctx.on('subagent/start', (info) => { const child = ctx.get('agents')?.get(info.id) - void runPoint('SubagentStart', info.agentType ?? '', subagentStartPayload(info), { ...child ? { agent: child } : {} }) + void runPoint('SubagentStart', SUBAGENT_TYPE, subagentPayload('SubagentStart', info, child), { ...child ? { agent: child } : {} }) .then((merged) => { const context = contextFrom(merged) if (context && child) child.inject(context.content, { source: context.source }) @@ -282,13 +287,24 @@ export function apply(ctx: Context, config: Config): void { .catch((error: unknown) => { ctx.logger.warn(`hooks-claude: SubagentStart hook failed: ${String(error)}`) }) }) ctx.on('subagent/end', (info) => { - // No `.then`/inject here (SubagentStop only observes) and no session is - // passed, so runPoint cannot reject — no `.catch` is needed (one would be - // dead code). The observe-only run is fire-and-forget. - void runPoint('SubagentStop', info.agentType ?? '', subagentStopPayload(info), {}) + // Look up the child (still recoverable: `subagent/end` fires from the + // service's detached `.then` BEFORE the tool caller's `await run.result` + // disposes it) so the hook runs in the child's cwd, not the server default. + // No `.then`/inject (SubagentStop only observes) and no session is passed, so + // runPoint cannot reject — no `.catch` is needed. Fire-and-forget. + const child = ctx.get('agents')?.get(info.id) + void runPoint('SubagentStop', SUBAGENT_TYPE, subagentPayload('SubagentStop', info, child), { ...child ? { agent: child } : {} }) }) } +/** + * The `agent_type` value the bridge reports for SubagentStart/Stop. The harness + * subagent seam carries no per-kind label, so the bridge uses Claude Code's own + * Task-tool default — a hooks.json with a default/`*`/empty `agent_type` matcher + * fires; a config matching a specific kind (e.g. `code-reviewer`) does not. + */ +const SUBAGENT_TYPE = 'general-purpose' + // --- Per-event stdin payloads (the CC DIALECT shape). Field names match CC's // hook input schema; this is the part a bridge owns. --- @@ -330,9 +346,17 @@ function postToolPayload(exec: ToolExecution, result: ToolExecutionResult): Reco function stopPayload(agent: Agent): Record { return { ...base(agent, 'Stop'), stop_hook_active: false } } -function subagentStartPayload(info: { id: string; agentType?: string }): Record { - return { hook_event_name: 'SubagentStart', agent_id: info.id, ...info.agentType !== undefined ? { agent_type: info.agentType } : {} } -} -function subagentStopPayload(info: { id: string; agentType?: string }): Record { - return { hook_event_name: 'SubagentStop', agent_id: info.id, stop_hook_active: false, ...info.agentType !== undefined ? { agent_type: info.agentType } : {} } +/** + * Build a SubagentStart/SubagentStop payload from the CC base (the child's + * `session_id`/`cwd` when the child agent is available) plus the subagent-hook + * fields. `agent_type` is the CC-default {@link SUBAGENT_TYPE}; `stop_hook_active` + * is present on SubagentStop only (the loop-guard flag, always false this cut). + */ +function subagentPayload(event: 'SubagentStart' | 'SubagentStop', info: { id: string }, child: Agent | undefined): Record { + return { + ...base(child, event), + agent_id: info.id, + agent_type: SUBAGENT_TYPE, + ...event === 'SubagentStop' ? { stop_hook_active: false } : {}, + } } diff --git a/packages/hooks/hooks-claude/tests/bridge.spec.ts b/packages/hooks/hooks-claude/tests/bridge.spec.ts index 0b6afd7e4c..3e36231e66 100644 --- a/packages/hooks/hooks-claude/tests/bridge.spec.ts +++ b/packages/hooks/hooks-claude/tests/bridge.spec.ts @@ -289,8 +289,8 @@ describe('hooks-claude bridge — SubagentStart / SubagentStop (observe)', () => // Drive the observe-only lifecycle events directly (no real child needed — the // bridge just listens). The agents registry is absent here, so SubagentStart's // child lookup yields undefined and it simply runs the hook. - ctx.emit('subagent/start', { provider: 'inproc', id: AgentId('child-1'), agentType: 'researcher' }) - ctx.emit('subagent/end', { provider: 'inproc', id: AgentId('child-1'), agentType: 'researcher', stopReason: 'completed', lastAssistantMessage: [{ type: 'text', text: 'done' }] }) + ctx.emit('subagent/start', { provider: 'inproc', id: AgentId('child-1') }) + ctx.emit('subagent/end', { provider: 'inproc', id: AgentId('child-1'), stopReason: 'completed', lastAssistantMessage: [{ type: 'text', text: 'done' }] }) // Both hooks run async (detached .then); poll for their marker files rather // than a fixed sleep that flakes under load. diff --git a/packages/hooks/hooks-claude/tests/coverage.spec.ts b/packages/hooks/hooks-claude/tests/coverage.spec.ts index a1b650f33d..097e026d1a 100644 --- a/packages/hooks/hooks-claude/tests/coverage.spec.ts +++ b/packages/hooks/hooks-claude/tests/coverage.spec.ts @@ -185,7 +185,7 @@ describe('hooks-claude coverage — Stop continuation + subagent inject/catch', const injected: string[] = [] const child = { id: AgentId('child-x'), inject: (content: { type: string; text?: string }[]) => { injected.push(content.map(b => b.text ?? '').join('')) }, session: { header: { id: 'child-x' } } } as unknown as Parameters[0] ctx.agents.register(child) - ctx.emit('subagent/start', { provider: 'p', id: AgentId('child-x'), agentType: 'r' }) + ctx.emit('subagent/start', { provider: 'p', id: AgentId('child-x') }) await waitFor(() => injected.includes('child guidance')) expect(injected).toContain('child guidance') }) @@ -236,17 +236,16 @@ describe('hooks-claude coverage — default reasons + sparse payloads', () => { expect(result?.type === 'tool/result' && result.data.content.some(b => b.type === 'text' && b.text.includes('blocked by PostToolUse hook'))).toBe(true) }) - it('SubagentStop with no agentType + a rejecting hook run is contained', async () => { + it('SubagentStop with no registered child runs the hook cleanly (fire-and-forget)', async () => { const d = dir() - // Make the SubagentStop runPoint reject by registering a session whose append - // throws — simplest: a hook that emits invalid output is fine; force the - // .catch by making the session's append throw via a poisoned agent is hard, - // so instead assert the no-agentType payload path runs cleanly (no crash). + // The agents registry has no entry for the id, so the child lookup yields + // undefined and the payload falls back to base(undefined) — assert the + // observe-only SubagentStop run still executes the hook without crashing. const marker = join(d, 'stopran') const s = sh(d, 'stop.sh', `#!/usr/bin/env bash\ntouch "${marker}"\n`) const path = hooks(d, { SubagentStop: [{ hooks: [{ type: 'command', command: s }] }] }) const ctx = await harness(path, new MockAdapter([])) - ctx.emit('subagent/end', { provider: 'p', id: AgentId('child-z'), stopReason: 'completed' }) // no agentType + ctx.emit('subagent/end', { provider: 'p', id: AgentId('child-z'), stopReason: 'completed' }) await waitFor(() => existsSync(marker)) expect(existsSync(marker)).toBe(true) }) @@ -492,6 +491,44 @@ describe('hooks-claude coverage — hook runs in the session cwd, not the server expect(where.endsWith(sessionDir.split('/').pop()!)).toBe(true) await handle.dispose() }) + + it('runs a SubagentStop hook in the CHILD session workspace, not the server cwd', async () => { + // The bug: SubagentStop ran runPoint(..., {}) with no agent, so the hook fell + // back to the executor default (server cwd). SubagentStop must look the child + // up (still recoverable at subagent/end) and run in the CHILD's session cwd. + // Here the executor default and the child session cwd are DIFFERENT dirs; a + // SubagentStop hook writes `pwd` to a marker and we assert it ran in the CHILD + // dir. (Proven to regress: neuter the child lookup and the marker lands in the + // server dir instead.) + const serverDir = dir() + const childDir = dir() + const marker = join(childDir, 'stopwhere') + hooks(serverDir, { SubagentStop: [{ hooks: [{ type: 'command', command: 'pwd > stopwhere' }] }] }) + const ctx = new Context() + await ctx.plugin(LlmService) + await ctx.plugin(SessionStore) + await ctx.plugin(SystemPrompt) + await ctx.plugin(ToolRegistry) + await ctx.plugin(AgentRegistry) + await ctx.plugin(AgentLoop, { agents: [] }) + // Executor default cwd = serverDir (deliberately NOT the child session cwd). + await ctx.plugin(LocalBashExecutor, { timeoutMs: 10_000, cwd: serverDir }) + await ctx.plugin(HooksClaude, { configPath: join(serverDir, 'hooks.json') }) + ctx.llm.registerAdapter(['mock'], new MockAdapter([])) + + // Register a live child on its own session cwd; emit subagent/end with its id. + const { SessionId } = await import('@deepseek-ai/dsh-session') + const childHandle = ctx.agents.create({ agentId: AgentId('child-stop'), sessionId: SessionId('child-stop-session'), meta: { cwd: childDir }, agentOptions: { model: 'mock' } }) + ctx.emit('subagent/end', { provider: 'inproc', id: childHandle.agent.id, stopReason: 'completed' }) + + await waitFor(() => existsSync(marker)) + expect(existsSync(marker)).toBe(true) // the marker landed in the CHILD dir + const { readFileSync } = await import('node:fs') + const where = readFileSync(marker, 'utf8').trim() + // `pwd` may resolve symlinks (/var → /private/var etc.), so compare basenames. + expect(where.endsWith(childDir.split('/').pop()!)).toBe(true) + await childHandle.dispose() + }) }) describe('hooks-claude coverage — systemMessage is warned, not surfaced', () => { From 8e8c791eb0bd6e2abb41689af4ea729bfbe8d36c Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Thu, 2 Jul 2026 07:01:55 +0800 Subject: [PATCH 08/10] docs(hooks-claude): current-state comment wording caught in review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two Codex nitpicks, comment-only (no behavior change): - the SubagentStop-cwd regression test comment narrated "The bug" / "Proven to regress" — rewrote to state the invariant it checks, not the history. - the subagent/end listener comment said "no session is passed"; with a child a session IS passed — corrected to "no `turn` is passed (so no hook/* records)", which is the actual reason runPoint has nothing that can reject. --- packages/hooks/hooks-claude/src/index.ts | 5 +++-- packages/hooks/hooks-claude/tests/coverage.spec.ts | 12 +++++------- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/packages/hooks/hooks-claude/src/index.ts b/packages/hooks/hooks-claude/src/index.ts index 7e21f7ea82..6816201e60 100644 --- a/packages/hooks/hooks-claude/src/index.ts +++ b/packages/hooks/hooks-claude/src/index.ts @@ -290,8 +290,9 @@ export function apply(ctx: Context, config: Config): void { // Look up the child (still recoverable: `subagent/end` fires from the // service's detached `.then` BEFORE the tool caller's `await run.result` // disposes it) so the hook runs in the child's cwd, not the server default. - // No `.then`/inject (SubagentStop only observes) and no session is passed, so - // runPoint cannot reject — no `.catch` is needed. Fire-and-forget. + // No `.then`/inject follows (SubagentStop only observes), and no `turn` is + // passed (so no `hook/*` log records), so runPoint has nothing that can + // reject — no `.catch` is needed. Fire-and-forget. const child = ctx.get('agents')?.get(info.id) void runPoint('SubagentStop', SUBAGENT_TYPE, subagentPayload('SubagentStop', info, child), { ...child ? { agent: child } : {} }) }) diff --git a/packages/hooks/hooks-claude/tests/coverage.spec.ts b/packages/hooks/hooks-claude/tests/coverage.spec.ts index 097e026d1a..c1c6d1581d 100644 --- a/packages/hooks/hooks-claude/tests/coverage.spec.ts +++ b/packages/hooks/hooks-claude/tests/coverage.spec.ts @@ -493,13 +493,11 @@ describe('hooks-claude coverage — hook runs in the session cwd, not the server }) it('runs a SubagentStop hook in the CHILD session workspace, not the server cwd', async () => { - // The bug: SubagentStop ran runPoint(..., {}) with no agent, so the hook fell - // back to the executor default (server cwd). SubagentStop must look the child - // up (still recoverable at subagent/end) and run in the CHILD's session cwd. - // Here the executor default and the child session cwd are DIFFERENT dirs; a - // SubagentStop hook writes `pwd` to a marker and we assert it ran in the CHILD - // dir. (Proven to regress: neuter the child lookup and the marker lands in the - // server dir instead.) + // SubagentStop looks the child up (recoverable at subagent/end) and runs the + // hook in the CHILD's session cwd, not the executor default. Here the executor + // default and the child session cwd are DIFFERENT dirs; a SubagentStop hook + // writes `pwd` to a relative marker and we assert it landed in the CHILD dir — + // which only holds if the listener threaded the child agent into runPoint. const serverDir = dir() const childDir = dir() const marker = join(childDir, 'stopwhere') From 9bc4df28c117e740a6420c1a2c504c21fc2a5504 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Thu, 2 Jul 2026 18:35:53 +0800 Subject: [PATCH 09/10] fix(hooks): delegate context-only hooks + default CLAUDE_PROJECT_DIR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review on the hook-bridges PR — two composability/compatibility bugs in both the CC and Codex bridges: 1. A hook that only attaches additionalContext (no block/deny) returned `allow`/`accept` WITHOUT calling next(), short-circuiting every later agent/prompt-submit / tools/post-execute listener. A policy/sandbox plugin registered after the bridge never saw the prompt. Now the context-only path delegates via next() and folds its context onto the downstream decision (concatContext): a downstream block/deny still wins and carries the bridge context; a downstream allow/accept keeps its own content rewrite and gains the context. Only a real hook deny/block short-circuits. 2. CLAUDE_PROJECT_DIR was empty in the default ACP wiring (no projectDir configured), breaking common unmodified hooks that reference $CLAUDE_PROJECT_DIR. It now defaults per-run to the agent's session workspace (the same cwd the hook runs in); an explicit config.projectDir still wins. Regression tests per bridge: a later listener blocks a prompt a context-only hook allowed; both contexts survive when the downstream also adds one; the default CLAUDE_PROJECT_DIR reaches the hook. Each proven red on the pre-fix code. --- .../feature/2026-06-30-hook-bridges.md | 12 +- packages/hooks/hooks-claude/README.md | 6 +- packages/hooks/hooks-claude/src/index.ts | 61 ++++++++-- .../hooks/hooks-claude/tests/coverage.spec.ts | 107 ++++++++++++++++++ packages/hooks/hooks-codex/README.md | 4 +- packages/hooks/hooks-codex/src/index.ts | 37 +++++- .../hooks/hooks-codex/tests/coverage.spec.ts | 64 +++++++++++ 7 files changed, 271 insertions(+), 20 deletions(-) diff --git a/docs/rfc/implemented/feature/2026-06-30-hook-bridges.md b/docs/rfc/implemented/feature/2026-06-30-hook-bridges.md index d6aea68b17..a8cc3df4db 100644 --- a/docs/rfc/implemented/feature/2026-06-30-hook-bridges.md +++ b/docs/rfc/implemented/feature/2026-06-30-hook-bridges.md @@ -24,9 +24,9 @@ Each bridge maps the neutral `MergedHookOutcome` from the shared lib onto the se | Seam | CC | Codex | |---|---|---| | `agent/session-start` (emit) | additionalContext → `agent.inject()` | plain-stdout output → additionalContext → `agent.inject()` | -| `agent/prompt-submit` | `deny`→`block`; context→`allow` | `block`→`block`; context→`allow` | +| `agent/prompt-submit` | `deny`→`block`; context-only→delegate+fold | `block`→`block`; context-only→delegate+fold | | `tools/pre-execute` | `deny`→`deny`; `ask`→`ask` | `block`→`deny` (no allow/ask) | -| `tools/post-execute` | `deny`→`block`+feedback; context→`accept` | same | +| `tools/post-execute` | `deny`→`block`+feedback; context-only→delegate+fold | same | | `agent/turn-continuation` | blocking Stop → `continue` (reason = next-step steering) | same | | `subagent/start` (emit) | additionalContext → inject into the live child | — (not a Codex event) | | `subagent/end` (emit) | observe-only | — | @@ -35,6 +35,14 @@ Each bridge maps the neutral `MergedHookOutcome` from the shared lib onto the se `agent.inject()` defaults a missing `MessageSource` to `{ kind: 'user' }` — which would record plugin-injected context as if the user had typed it. So every bridge `inject()` and every `HookContext` passes an explicit `{ kind: 'plugin', plugin: 'hooks-claude' | 'hooks-codex' }` source. A test asserts the resulting `context/message.source` is the plugin, never `user`. +### Adding context is not a veto — delegate, then fold + +A hook that only attaches `additionalContext` (no block/deny) is NOT a decision the bridge should return on its own: returning `allow`/`accept` from a waterfall listener WITHOUT calling `next()` short-circuits every later `agent/prompt-submit` / `tools/post-execute` listener, so a policy/sandbox plugin registered after the bridge would never see the prompt. So on the context-only path each bridge **delegates via `next()`** and then **folds** its `additionalContext` onto the downstream decision (`concatContext`): a downstream `block`/`deny` still wins (and carries the bridge context too), a downstream `allow`/`accept` keeps its own content rewrite and gains the bridge context. Only a real `deny`/`block` from the hook short-circuits. Tests assert a later listener can still block a prompt a context-only hook allowed, and that both contexts survive when the downstream also adds one. + +### CLAUDE_PROJECT_DIR defaults to the session workspace + +Claude Code always exports `CLAUDE_PROJECT_DIR`, and common unmodified hooks reference `$CLAUDE_PROJECT_DIR` for project-relative paths. An explicit `config.projectDir` wins; when it is omitted (the default ACP wiring configures only `configPath`), the bridge defaults the env var per-run to the agent's session workspace — the same `session.header.cwd` the hook already runs in — rather than leaving it empty. So a stock project-relative hook works in the default setup. + ### Containment The config is parsed ONCE at load; a read/parse failure logs and registers nothing rather than crashing boot (a typo'd path must not take the agent down). Only `type: 'command'` hooks run — a `prompt`/`agent`/HTTP hook (CC) or an `async: true` / non-command hook (Codex) is parsed-and-skipped with a warning. The emit-listener paths (`session-start`, `subagent/start`) run detached, with their `inject` contained in a `.catch` that logs (a throwing inject must not break session boot or the loop). diff --git a/packages/hooks/hooks-claude/README.md b/packages/hooks/hooks-claude/README.md index f9520f69be..ce1c6b090a 100644 --- a/packages/hooks/hooks-claude/README.md +++ b/packages/hooks/hooks-claude/README.md @@ -11,7 +11,7 @@ import type { Config } from '@deepseek-ai/dsh-hooks-claude' const config: Config = { configPath: '/path/to/hooks.json', // required: a hooks.json or a settings file with a `hooks` key pluginRoot: '/path/to/plugin', // optional: replaces ${CLAUDE_PLUGIN_ROOT} in command strings - projectDir: '/path/to/project', // optional: replaces ${CLAUDE_PROJECT_DIR} AND set as the hook env var + projectDir: '/path/to/project', // optional: replaces ${CLAUDE_PROJECT_DIR} AND sets the hook env var; defaults to the session cwd when omitted defaultTimeoutMs: 600_000, // optional: per-hook timeout when a hook sets none (CC default) } ``` @@ -34,9 +34,9 @@ The hooks **themselves** run in the agent's session workspace: for the agent-sco | CC hook | Harness seam | Mapping | |---|---|---| | `SessionStart` | `agent/session-start` (emit) | additionalContext → `agent.inject()` into the new session (cannot block) | -| `UserPromptSubmit` | `agent/prompt-submit` (waterfall) | `deny` → `PromptDecision.block`; additionalContext → `allow` with context | +| `UserPromptSubmit` | `agent/prompt-submit` (waterfall) | `deny` → `PromptDecision.block`; additionalContext-only → delegate via `next()` then fold context onto the downstream decision (a later listener can still block/rewrite) | | `PreToolUse` | `tools/pre-execute` (waterfall) | `deny` → `PreToolDecision.deny`; `ask` → `PreToolDecision.ask` | -| `PostToolUse` | `tools/post-execute` (waterfall) | `deny` → `block` with feedback; additionalContext → `accept` with context | +| `PostToolUse` | `tools/post-execute` (waterfall) | `deny` → `block` with feedback; additionalContext-only → delegate via `next()` then fold context onto the downstream decision | | `Stop` | `agent/turn-continuation` (waterfall) | a blocking Stop hook forces `continue`, feeding its reason as next-step steering | | `SubagentStart` | `subagent/start` (emit) | additionalContext → `agent.inject()` into the live child | | `SubagentStop` | `subagent/end` (emit) | observe-only | diff --git a/packages/hooks/hooks-claude/src/index.ts b/packages/hooks/hooks-claude/src/index.ts index 6816201e60..3468e1c037 100644 --- a/packages/hooks/hooks-claude/src/index.ts +++ b/packages/hooks/hooks-claude/src/index.ts @@ -59,9 +59,17 @@ export interface Config { * `hooks.json` from each `session/new.cwd` is not yet implemented. */ configPath: string - /** Replaces `${CLAUDE_PLUGIN_ROOT}` in command strings (the plugin's root dir). */ + /** + * Replaces `${CLAUDE_PLUGIN_ROOT}` in command strings (the plugin's root dir). + */ pluginRoot?: string - /** Replaces `${CLAUDE_PROJECT_DIR}` in command strings + set as the hook env var. */ + /** + * Replaces `${CLAUDE_PROJECT_DIR}` in command strings AND is exported as the + * `CLAUDE_PROJECT_DIR` env var for hook processes. When omitted, the env var + * defaults per-run to the agent's session workspace (`session.header.cwd`, the + * same dir the hook runs in) — Claude Code always exports this var, and common + * unmodified hooks reference `$CLAUDE_PROJECT_DIR` for project-relative paths. + */ projectDir?: string /** Default per-hook timeout in ms when a hook sets none (CC default: 600000). */ defaultTimeoutMs?: number @@ -111,7 +119,6 @@ export function apply(ctx: Context, config: Config): void { } const defaultTimeoutMs = config.defaultTimeoutMs ?? 600_000 - const hookEnv = config.projectDir !== undefined ? { CLAUDE_PROJECT_DIR: config.projectDir } : undefined /** * Run every command hook configured for `point` whose matcher selects @@ -136,6 +143,15 @@ export function apply(ctx: Context, config: Config): void { // operate in the user's project tree. Absent for a no-agent run (falls back // to the executor default). const workdir = opts.agent?.session.header.cwd + // CLAUDE_PROJECT_DIR: an explicit config value wins; otherwise default it to + // the session workspace (the same dir the hook RUNS in). Claude Code always + // exports this var, and common unmodified hooks reference `$CLAUDE_PROJECT_DIR` + // (shell expansion at run time) for project-relative paths — leaving it empty + // in the default ACP wiring (no `projectDir` configured) would break them even + // though the bridge already knows the workspace. Absent only for a no-agent run + // with no configured projectDir (nothing to point at). + const projectDir = config.projectDir ?? workdir + const hookEnv = projectDir !== undefined ? { CLAUDE_PROJECT_DIR: projectDir } : undefined for (const group of groups) { if (!matchesMatcher(group.matcher, matchQuery, 'claude')) continue for (const hook of group.hooks) { @@ -195,6 +211,16 @@ export function apply(ctx: Context, config: Config): void { return { content, source: PLUGIN_SOURCE } } + /** + * Concatenate this bridge's {@link HookContext} (`ours`, always present at the + * call sites) with a downstream listener's optional one, so folding our + * additionalContext onto a delegated decision drops neither. + */ + function concatContext(ours: HookContext, theirs: HookContext | undefined): HookContext { + if (!theirs) return ours + return { content: [...ours.content, ...theirs.content], source: ours.source } + } + // --- SessionStart: emit (cannot block). Inject any additionalContext into the // agent. The matcher subject is the source. // TODO(session-start-gating): `agent/session-start` is a SYNCHRONOUS emit and @@ -223,9 +249,18 @@ export function apply(ctx: Context, config: Config): void { if (merged.decision === 'deny') { return { kind: 'block', reason: merged.reason ?? 'blocked by UserPromptSubmit hook' } } - const context = contextFrom(merged) - if (context) return { kind: 'allow', additionalContext: context } - return next() + // Our hooks did not block. DELEGATE (attaching context alone is not a veto): + // a later `agent/prompt-submit` listener must still get to block or rewrite. + // Then fold our additionalContext onto its decision — a downstream block wins + // (a dropped prompt makes the context moot; `block` carries no context field). + const downstream = await next() + const ours = contextFrom(merged) + if (!ours || downstream.kind !== 'allow') return downstream + return { + kind: 'allow', + ...downstream.content !== undefined ? { content: downstream.content } : {}, + additionalContext: concatContext(ours, downstream.additionalContext), + } }) // --- PreToolUse → PreToolDecision. Matcher subject is the tool name. --- @@ -245,8 +280,18 @@ export function apply(ctx: Context, config: Config): void { if (merged.decision === 'deny') { return { kind: 'block', feedback: [{ type: 'text', text: merged.reason ?? 'blocked by PostToolUse hook' }], ...context ? { additionalContext: context } : {} } } - if (context) return { kind: 'accept', additionalContext: context } - return next() + // Our hooks did not block. DELEGATE so a later listener can still block/replace, + // then fold our context onto its decision (a downstream block carries it too). + const downstream = await next() + if (!context) return downstream + if (downstream.kind === 'block') { + return { ...downstream, additionalContext: concatContext(context, downstream.additionalContext) } + } + return { + kind: 'accept', + ...downstream.content !== undefined ? { content: downstream.content } : {}, + additionalContext: concatContext(context, downstream.additionalContext), + } }) // --- Stop → ContinuationDecision. CC's Stop hook can force the conversation to diff --git a/packages/hooks/hooks-claude/tests/coverage.spec.ts b/packages/hooks/hooks-claude/tests/coverage.spec.ts index c1c6d1581d..d2e01f4c4d 100644 --- a/packages/hooks/hooks-claude/tests/coverage.spec.ts +++ b/packages/hooks/hooks-claude/tests/coverage.spec.ts @@ -410,6 +410,113 @@ describe('hooks-claude coverage — continue:false, context arm, no-cwd', () => expect(ran).toBe(true) // the mismatched deny was discarded → the tool ran }) + it('defaults CLAUDE_PROJECT_DIR to the session workspace when no projectDir is configured', async () => { + // The default ACP wiring sets no projectDir. A stock CC hook that references + // $CLAUDE_PROJECT_DIR (shell expansion) must still get the session workspace, + // not an empty string. The hook echoes the var as additionalContext. + const d = dir() + const workspace = dir() + const s = sh(d, 'ctx.sh', '#!/usr/bin/env bash\nprintf \'{"hookSpecificOutput":{"hookEventName":"UserPromptSubmit","additionalContext":"dir=%s"}}\' "$CLAUDE_PROJECT_DIR"\n') + const path = hooks(d, { UserPromptSubmit: [{ hooks: [{ type: 'command', command: s }] }] }) + const adapter = new MockAdapter([textResponse('ran')]) + const ctx = await harness(path, adapter) // NB: no projectDir + // The factory create() path honors meta.cwd (the plain agentLoop.create() does not). + const { SessionId } = await import('@deepseek-ai/dsh-session') + const handle = ctx.agents.create({ agentId: AgentId('a1'), sessionId: SessionId('s1'), meta: { cwd: workspace }, agentOptions: { model: 'mock' } }) + handle.agent.send([{ type: 'text', text: 'go' }]) + await waitForIdle(ctx, handle.agent as ReactLoopAgent) + expect(events(handle.agent as ReactLoopAgent).some(e => e.type === 'context/message' + && e.data.content.some(b => b.type === 'text' && b.text.includes(`dir=${workspace}`)))).toBe(true) + await handle.dispose() + }) + + it('a context-only UserPromptSubmit hook DELEGATES so a later listener can still block', async () => { + // A hook that only adds context must NOT short-circuit the waterfall: a + // downstream agent/prompt-submit listener (a policy plugin) must still get to + // block the prompt. Before the fix the bridge returned `allow` without next(). + const d = dir() + const s = sh(d, 'ctx.sh', '#!/usr/bin/env bash\necho \'{"hookSpecificOutput":{"hookEventName":"UserPromptSubmit","additionalContext":"bridge ctx"}}\'\n') + const path = hooks(d, { UserPromptSubmit: [{ hooks: [{ type: 'command', command: s }] }] }) + const adapter = new MockAdapter([textResponse('should not run')]) + const ctx = await harness(path, adapter) + // A later listener that blocks every prompt (registered AFTER the bridge). + const { AgentId: AId } = await import('@deepseek-ai/dsh-agent') + ctx.on('agent/prompt-submit', async () => ({ kind: 'block' as const, reason: 'policy veto' })) + const agent = ctx.agentLoop.create(AId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]) + await waitForIdle(ctx, agent) + // the downstream block won: the model was never called, no user/message was + // recorded, and the (sole, fully-blocked) prompt closed the turn `rejected` + expect(adapter.requests).toHaveLength(0) + expect(events(agent).some(e => e.type === 'user/message')).toBe(false) + const turnEnd = events(agent).findLast(e => e.type === 'turn/end') + expect(turnEnd?.type === 'turn/end' && turnEnd.data.reason).toMatchObject({ kind: 'rejected', reason: 'policy veto' }) + }) + + it('folds the bridge additionalContext WITH a downstream listener that also adds context', async () => { + // Both the bridge hook and a later prompt-submit listener attach context; the + // request must see BOTH (concatContext keeps the downstream one too). + const d = dir() + const s = sh(d, 'ctx.sh', '#!/usr/bin/env bash\necho \'{"hookSpecificOutput":{"hookEventName":"UserPromptSubmit","additionalContext":"from-bridge"}}\'\n') + const path = hooks(d, { UserPromptSubmit: [{ hooks: [{ type: 'command', command: s }] }] }) + const adapter = new MockAdapter([textResponse('ok')]) + const ctx = await harness(path, adapter) + ctx.on('agent/prompt-submit', async () => ({ + kind: 'allow' as const, + content: [{ type: 'text' as const, text: 'rewritten-prompt' }], + additionalContext: { content: [{ type: 'text' as const, text: 'from-downstream' }], source: { kind: 'plugin' as const, plugin: 'policy' } }, + })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]) + await waitForIdle(ctx, agent) + const req = JSON.stringify(adapter.requests[0]!.messages) + expect(req).toContain('from-bridge') + expect(req).toContain('from-downstream') + expect(req).toContain('rewritten-prompt') // downstream content rewrite preserved + // the original prompt was replaced by the downstream rewrite + const userMsg = events(agent).find(e => e.type === 'user/message') + expect(userMsg?.type === 'user/message' && userMsg.data.content.some(b => b.type === 'text' && b.text === 'rewritten-prompt')).toBe(true) + }) + + it('folds the bridge PostToolUse context onto a downstream ACCEPT that replaces content', async () => { + // The bridge hook adds context; a later post-execute listener accepts with a + // content rewrite. Both the rewrite and the bridge context survive. + const d = dir() + const s = sh(d, 'ctx.sh', '#!/usr/bin/env bash\necho \'{"hookSpecificOutput":{"hookEventName":"PostToolUse","additionalContext":"bridge-note"}}\'\n') + const path = hooks(d, { PostToolUse: [{ hooks: [{ type: 'command', command: s }] }] }) + const adapter = new MockAdapter([toolCallResponse('c1', 'echo', {}), textResponse('done')]) + const ctx = await harness(path, adapter) + ctx.tools.register(defineTool({ name: 'echo', description: 'e', parameters: {}, async execute() { return [{ type: 'text', text: 'ok' }] } })) + ctx.on('tools/post-execute', async () => ({ kind: 'accept' as const, content: [{ type: 'text' as const, text: 'rewritten-result' }] })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]) + await waitForIdle(ctx, agent) + const result = events(agent).find(e => e.type === 'tool/result') + expect(result?.type === 'tool/result' && result.data.content.some(b => b.type === 'text' && b.text === 'rewritten-result')).toBe(true) + expect(events(agent).some(e => e.type === 'context/message' && e.data.content.some(b => b.type === 'text' && b.text.includes('bridge-note')))).toBe(true) + }) + + it('folds the bridge PostToolUse context onto a downstream listener BLOCK', async () => { + // The bridge hook only adds context; a later post-execute listener blocks the + // result. The block wins AND carries the bridge context (concatContext on the + // block arm). + const d = dir() + const s = sh(d, 'ctx.sh', '#!/usr/bin/env bash\necho \'{"hookSpecificOutput":{"hookEventName":"PostToolUse","additionalContext":"bridge-note"}}\'\n') + const path = hooks(d, { PostToolUse: [{ hooks: [{ type: 'command', command: s }] }] }) + const adapter = new MockAdapter([toolCallResponse('c1', 'echo', {}), textResponse('done')]) + const ctx = await harness(path, adapter) + ctx.tools.register(defineTool({ name: 'echo', description: 'e', parameters: {}, async execute() { return [{ type: 'text', text: 'ok' }] } })) + ctx.on('tools/post-execute', async () => ({ kind: 'block' as const, feedback: [{ type: 'text' as const, text: 'downstream-block' }] })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]) + await waitForIdle(ctx, agent) + const result = events(agent).find(e => e.type === 'tool/result') + expect(result?.type === 'tool/result' && result.data.isError).toBe(true) + expect(result?.type === 'tool/result' && result.data.content.some(b => b.type === 'text' && b.text.includes('downstream-block'))).toBe(true) + // the bridge's context still landed (folded onto the block) + expect(events(agent).some(e => e.type === 'context/message' && e.data.content.some(b => b.type === 'text' && b.text.includes('bridge-note')))).toBe(true) + }) + }) describe('hooks-claude coverage — executor reject + no-open-turn', () => { diff --git a/packages/hooks/hooks-codex/README.md b/packages/hooks/hooks-codex/README.md index 6b28d7d114..72be33f57f 100644 --- a/packages/hooks/hooks-codex/README.md +++ b/packages/hooks/hooks-codex/README.md @@ -40,9 +40,9 @@ The hooks themselves run in the agent's session workspace: for the agent-scoped | Codex hook | Harness seam | Mapping | |---|---|---| | `SessionStart` | `agent/session-start` (emit) | a plain-stdout hook's output → additionalContext → `agent.inject()` | -| `UserPromptSubmit` | `agent/prompt-submit` (waterfall) | `block` (exit 2) → `PromptDecision.block`; additionalContext → `allow` with context | +| `UserPromptSubmit` | `agent/prompt-submit` (waterfall) | `block` (exit 2) → `PromptDecision.block`; additionalContext-only → delegate via `next()` then fold context onto the downstream decision | | `PreToolUse` | `tools/pre-execute` (waterfall) | `block` → `PreToolDecision.deny` (no `allow`/`ask`) | -| `PostToolUse` | `tools/post-execute` (waterfall) | `block` → `block` with feedback; additionalContext → `accept` with context | +| `PostToolUse` | `tools/post-execute` (waterfall) | `block` → `block` with feedback; additionalContext-only → delegate via `next()` then fold context onto the downstream decision | | `Stop` | `agent/turn-continuation` (waterfall) | a blocking Stop hook forces `continue` with the reason as next-step steering | A tool call's payload carries the real `tool_name` (the same value the matcher tests) and Codex's `tool_input: { command }` shape (the `command` arg when present, else `''`). The matcher subject is the tool name (`PreToolUse`/`PostToolUse`) or the session source (`SessionStart`); `UserPromptSubmit`/`Stop` ignore matchers. diff --git a/packages/hooks/hooks-codex/src/index.ts b/packages/hooks/hooks-codex/src/index.ts index c71a0811ea..393e157cba 100644 --- a/packages/hooks/hooks-codex/src/index.ts +++ b/packages/hooks/hooks-codex/src/index.ts @@ -166,6 +166,16 @@ export function apply(ctx: Context, config: Config): void { return { content, source: PLUGIN_SOURCE } } + /** + * Concatenate this bridge's {@link HookContext} (`ours`, always present at the + * call sites) with a downstream listener's optional one, so folding our + * additionalContext onto a delegated decision drops neither. + */ + function concatContext(ours: HookContext, theirs: HookContext | undefined): HookContext { + if (!theirs) return ours + return { content: [...ours.content, ...theirs.content], source: ours.source } + } + // SessionStart: emit. Codex passes a plain-stdout hook's output as additionalContext. // TODO(session-start-gating): a synchronous emit + detached `.then`, so the // injected context is BEST-EFFORT — not guaranteed before the first turn reaches @@ -185,9 +195,16 @@ export function apply(ctx: Context, config: Config): void { const turn = lastTurn(agent) const merged = await runPoint('UserPromptSubmit', '', { ...turnBase(agent, 'UserPromptSubmit', model), prompt: blocksToText(content) }, { agent, turn, plainStdoutAsContext: true }) if (merged.decision === 'deny') return { kind: 'block', reason: merged.reason ?? 'blocked by UserPromptSubmit hook' } - const context = contextFrom(merged) - if (context) return { kind: 'allow', additionalContext: context } - return next() + // Context alone is not a veto: DELEGATE so a later prompt-submit listener can + // still block/rewrite, then fold our context onto its decision. + const downstream = await next() + const ours = contextFrom(merged) + if (!ours || downstream.kind !== 'allow') return downstream + return { + kind: 'allow', + ...downstream.content !== undefined ? { content: downstream.content } : {}, + additionalContext: concatContext(ours, downstream.additionalContext), + } }) // PreToolUse → PreToolDecision. Codex blocks only (no allow/ask honored). @@ -206,8 +223,18 @@ export function apply(ctx: Context, config: Config): void { if (merged.decision === 'deny') { return { kind: 'block', feedback: [{ type: 'text', text: merged.reason ?? 'blocked by PostToolUse hook' }], ...context ? { additionalContext: context } : {} } } - if (context) return { kind: 'accept', additionalContext: context } - return next() + // Context alone is not a veto: DELEGATE, then fold our context onto the + // downstream decision (a downstream block carries it too). + const downstream = await next() + if (!context) return downstream + if (downstream.kind === 'block') { + return { ...downstream, additionalContext: concatContext(context, downstream.additionalContext) } + } + return { + kind: 'accept', + ...downstream.content !== undefined ? { content: downstream.content } : {}, + additionalContext: concatContext(context, downstream.additionalContext), + } }) // Stop → ContinuationDecision. A blocking Stop hook forces continuation. diff --git a/packages/hooks/hooks-codex/tests/coverage.spec.ts b/packages/hooks/hooks-codex/tests/coverage.spec.ts index d7a5806fbd..3bfc3edb03 100644 --- a/packages/hooks/hooks-codex/tests/coverage.spec.ts +++ b/packages/hooks/hooks-codex/tests/coverage.spec.ts @@ -69,6 +69,70 @@ describe('hooks-codex coverage — decision mapping paths', () => { expect(JSON.stringify(adapter.requests[0]!.messages)).toContain('ctx-x') }) + it('a context-only UserPromptSubmit hook DELEGATES so a later listener can still block', async () => { + // Context alone is not a veto: a downstream agent/prompt-submit listener (a + // policy plugin registered after the bridge) must still get to block. Before + // the fix the bridge returned `allow` without calling next(). + const d = dir() + hooks(d, { UserPromptSubmit: [{ hooks: [{ type: 'command', command: sh(d, 'c.sh', '#!/usr/bin/env bash\necho \'{"hookSpecificOutput":{"hookEventName":"UserPromptSubmit","additionalContext":"bridge ctx"}}\'\n') }] }] }) + const adapter = new MockAdapter([textResponse('should not run')]) + const ctx = await harness(join(d, 'hooks.json'), adapter) + ctx.on('agent/prompt-submit', async () => ({ kind: 'block' as const, reason: 'policy veto' })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]); await waitForIdle(ctx, agent) + expect(adapter.requests).toHaveLength(0) + expect(events(agent).some(e => e.type === 'user/message')).toBe(false) + const te = events(agent).findLast(e => e.type === 'turn/end') + expect(te?.type === 'turn/end' && te.data.reason).toMatchObject({ kind: 'rejected', reason: 'policy veto' }) + }) + + it('folds the bridge additionalContext WITH a downstream listener that also adds context', async () => { + const d = dir() + hooks(d, { UserPromptSubmit: [{ hooks: [{ type: 'command', command: sh(d, 'c.sh', '#!/usr/bin/env bash\necho \'{"hookSpecificOutput":{"hookEventName":"UserPromptSubmit","additionalContext":"from-bridge"}}\'\n') }] }] }) + const adapter = new MockAdapter([textResponse('ok')]) + const ctx = await harness(join(d, 'hooks.json'), adapter) + ctx.on('agent/prompt-submit', async () => ({ + kind: 'allow' as const, + content: [{ type: 'text' as const, text: 'rewritten-prompt' }], + additionalContext: { content: [{ type: 'text' as const, text: 'from-downstream' }], source: { kind: 'plugin' as const, plugin: 'policy' } }, + })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]); await waitForIdle(ctx, agent) + const req = JSON.stringify(adapter.requests[0]!.messages) + expect(req).toContain('from-bridge') + expect(req).toContain('from-downstream') + expect(req).toContain('rewritten-prompt') + }) + + it('folds the bridge PostToolUse context onto a downstream ACCEPT that replaces content', async () => { + const d = dir() + hooks(d, { PostToolUse: [{ hooks: [{ type: 'command', command: sh(d, 'pc.sh', '#!/usr/bin/env bash\necho \'{"hookSpecificOutput":{"hookEventName":"PostToolUse","additionalContext":"bridge-note"}}\'\n') }] }] }) + const adapter = new MockAdapter([toolCallResponse('c1', 'echo', {}), textResponse('done')]) + const ctx = await harness(join(d, 'hooks.json'), adapter) + ctx.tools.register(defineTool({ name: 'echo', description: 'e', parameters: {}, async execute() { return [{ type: 'text', text: 'ok' }] } })) + ctx.on('tools/post-execute', async () => ({ kind: 'accept' as const, content: [{ type: 'text' as const, text: 'rewritten-result' }] })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]); await waitForIdle(ctx, agent) + const result = events(agent).find(e => e.type === 'tool/result') + expect(result?.type === 'tool/result' && result.data.content.some(b => b.type === 'text' && b.text === 'rewritten-result')).toBe(true) + expect(events(agent).some(e => e.type === 'context/message' && e.data.content.some(b => b.type === 'text' && b.text.includes('bridge-note')))).toBe(true) + }) + + it('folds the bridge PostToolUse context onto a downstream listener BLOCK', async () => { + const d = dir() + hooks(d, { PostToolUse: [{ hooks: [{ type: 'command', command: sh(d, 'pc.sh', '#!/usr/bin/env bash\necho \'{"hookSpecificOutput":{"hookEventName":"PostToolUse","additionalContext":"bridge-note"}}\'\n') }] }] }) + const adapter = new MockAdapter([toolCallResponse('c1', 'echo', {}), textResponse('done')]) + const ctx = await harness(join(d, 'hooks.json'), adapter) + ctx.tools.register(defineTool({ name: 'echo', description: 'e', parameters: {}, async execute() { return [{ type: 'text', text: 'ok' }] } })) + ctx.on('tools/post-execute', async () => ({ kind: 'block' as const, feedback: [{ type: 'text' as const, text: 'downstream-block' }] })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + agent.send([{ type: 'text', text: 'go' }]); await waitForIdle(ctx, agent) + const result = events(agent).find(e => e.type === 'tool/result') + expect(result?.type === 'tool/result' && result.data.isError).toBe(true) + expect(result?.type === 'tool/result' && result.data.content.some(b => b.type === 'text' && b.text.includes('downstream-block'))).toBe(true) + expect(events(agent).some(e => e.type === 'context/message' && e.data.content.some(b => b.type === 'text' && b.text.includes('bridge-note')))).toBe(true) + }) + it('SessionStart additionalContext is injected for the first request', async () => { const d = dir() hooks(d, { SessionStart: [{ hooks: [{ type: 'command', command: sh(d, 's.sh', '#!/usr/bin/env bash\necho \'{"hookSpecificOutput":{"hookEventName":"SessionStart","additionalContext":"start-ctx"}}\'\n') }] }] }) From 2a66b7c4e0ce53bedf6939f4393f324c06025ac9 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Thu, 2 Jul 2026 20:07:10 +0800 Subject: [PATCH 10/10] docs(hooks): correct fold description + drop history-narrating test comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex convergence findings on the delegate-and-fold fix (code path verified correct, prose only): - The hook-bridges RFC claimed a downstream `block` "carries the bridge context too" for BOTH seams. True for `tools/post-execute` (PostToolDecision.block has an additionalContext field) but false for `agent/prompt-submit` (PromptDecision.block is `{kind,reason}` with no context field). The code is already correct — a blocked prompt drops the context, which is right since the prompt never reaches the model. Reworded the RFC to state the per-seam difference accurately. - Two test comments narrated "Before the fix…", which the current-state-only doc rule forbids. Reworded to describe the behavior, not its history. - Documented on concatContext (both bridges) why the merged block carries a single source: a HookContext holds one MessageSource and the seam cannot represent mixed provenance; rendering distinguishes only by source.kind, so a downstream plugin's text stays framed as plugin context. --- docs/rfc/implemented/feature/2026-06-30-hook-bridges.md | 2 +- packages/hooks/hooks-claude/src/index.ts | 7 ++++++- packages/hooks/hooks-claude/tests/coverage.spec.ts | 2 +- packages/hooks/hooks-codex/src/index.ts | 6 +++++- packages/hooks/hooks-codex/tests/coverage.spec.ts | 4 ++-- 5 files changed, 15 insertions(+), 6 deletions(-) diff --git a/docs/rfc/implemented/feature/2026-06-30-hook-bridges.md b/docs/rfc/implemented/feature/2026-06-30-hook-bridges.md index a8cc3df4db..3bedd73b34 100644 --- a/docs/rfc/implemented/feature/2026-06-30-hook-bridges.md +++ b/docs/rfc/implemented/feature/2026-06-30-hook-bridges.md @@ -37,7 +37,7 @@ Each bridge maps the neutral `MergedHookOutcome` from the shared lib onto the se ### Adding context is not a veto — delegate, then fold -A hook that only attaches `additionalContext` (no block/deny) is NOT a decision the bridge should return on its own: returning `allow`/`accept` from a waterfall listener WITHOUT calling `next()` short-circuits every later `agent/prompt-submit` / `tools/post-execute` listener, so a policy/sandbox plugin registered after the bridge would never see the prompt. So on the context-only path each bridge **delegates via `next()`** and then **folds** its `additionalContext` onto the downstream decision (`concatContext`): a downstream `block`/`deny` still wins (and carries the bridge context too), a downstream `allow`/`accept` keeps its own content rewrite and gains the bridge context. Only a real `deny`/`block` from the hook short-circuits. Tests assert a later listener can still block a prompt a context-only hook allowed, and that both contexts survive when the downstream also adds one. +A hook that only attaches `additionalContext` (no block/deny) is NOT a decision the bridge should return on its own: returning `allow`/`accept` from a waterfall listener WITHOUT calling `next()` short-circuits every later `agent/prompt-submit` / `tools/post-execute` listener, so a policy/sandbox plugin registered after the bridge would never see the prompt. So on the context-only path each bridge **delegates via `next()`** and then **folds** its `additionalContext` onto the downstream decision (`concatContext`). The fold differs by seam because the two Decision unions differ: `tools/post-execute` — a downstream `block`/`accept` both carry an `additionalContext` field, so the bridge context rides along either way (a downstream block wins AND keeps the context; a downstream accept keeps its content rewrite and gains the context). `agent/prompt-submit` — a downstream `allow` gains the bridge context (and keeps its own content rewrite / additionalContext), but `PromptDecision.block` carries no context field, so a downstream block drops the bridge context — which is correct: a blocked prompt never reaches the model, so context attached to it is moot. Only a real `deny`/`block` from the hook itself short-circuits. Tests assert a later listener can still block a prompt a context-only hook allowed, and that both contexts survive when the downstream also adds one. ### CLAUDE_PROJECT_DIR defaults to the session workspace diff --git a/packages/hooks/hooks-claude/src/index.ts b/packages/hooks/hooks-claude/src/index.ts index 3468e1c037..6151a11c44 100644 --- a/packages/hooks/hooks-claude/src/index.ts +++ b/packages/hooks/hooks-claude/src/index.ts @@ -214,7 +214,12 @@ export function apply(ctx: Context, config: Config): void { /** * Concatenate this bridge's {@link HookContext} (`ours`, always present at the * call sites) with a downstream listener's optional one, so folding our - * additionalContext onto a delegated decision drops neither. + * additionalContext onto a delegated decision drops neither. The merged block + * carries a single `source` — this bridge's — because a `HookContext` holds one + * `MessageSource` and the seam cannot represent mixed provenance; the rendered + * `context/message` only distinguishes by `source.kind` ('plugin'), so a + * downstream plugin's text is still correctly framed as plugin context, not a + * user prompt. */ function concatContext(ours: HookContext, theirs: HookContext | undefined): HookContext { if (!theirs) return ours diff --git a/packages/hooks/hooks-claude/tests/coverage.spec.ts b/packages/hooks/hooks-claude/tests/coverage.spec.ts index d2e01f4c4d..63e2f611ce 100644 --- a/packages/hooks/hooks-claude/tests/coverage.spec.ts +++ b/packages/hooks/hooks-claude/tests/coverage.spec.ts @@ -433,7 +433,7 @@ describe('hooks-claude coverage — continue:false, context arm, no-cwd', () => it('a context-only UserPromptSubmit hook DELEGATES so a later listener can still block', async () => { // A hook that only adds context must NOT short-circuit the waterfall: a // downstream agent/prompt-submit listener (a policy plugin) must still get to - // block the prompt. Before the fix the bridge returned `allow` without next(). + // block the prompt. The bridge delegates via next() and folds its context. const d = dir() const s = sh(d, 'ctx.sh', '#!/usr/bin/env bash\necho \'{"hookSpecificOutput":{"hookEventName":"UserPromptSubmit","additionalContext":"bridge ctx"}}\'\n') const path = hooks(d, { UserPromptSubmit: [{ hooks: [{ type: 'command', command: s }] }] }) diff --git a/packages/hooks/hooks-codex/src/index.ts b/packages/hooks/hooks-codex/src/index.ts index 393e157cba..a704a6af24 100644 --- a/packages/hooks/hooks-codex/src/index.ts +++ b/packages/hooks/hooks-codex/src/index.ts @@ -169,7 +169,11 @@ export function apply(ctx: Context, config: Config): void { /** * Concatenate this bridge's {@link HookContext} (`ours`, always present at the * call sites) with a downstream listener's optional one, so folding our - * additionalContext onto a delegated decision drops neither. + * additionalContext onto a delegated decision drops neither. The merged block + * carries a single `source` — this bridge's — because a `HookContext` holds one + * `MessageSource` and the seam cannot represent mixed provenance; the rendered + * `context/message` only distinguishes by `source.kind` ('plugin'), so a + * downstream plugin's text is still correctly framed as plugin context. */ function concatContext(ours: HookContext, theirs: HookContext | undefined): HookContext { if (!theirs) return ours diff --git a/packages/hooks/hooks-codex/tests/coverage.spec.ts b/packages/hooks/hooks-codex/tests/coverage.spec.ts index 3bfc3edb03..87032c98ce 100644 --- a/packages/hooks/hooks-codex/tests/coverage.spec.ts +++ b/packages/hooks/hooks-codex/tests/coverage.spec.ts @@ -71,8 +71,8 @@ describe('hooks-codex coverage — decision mapping paths', () => { it('a context-only UserPromptSubmit hook DELEGATES so a later listener can still block', async () => { // Context alone is not a veto: a downstream agent/prompt-submit listener (a - // policy plugin registered after the bridge) must still get to block. Before - // the fix the bridge returned `allow` without calling next(). + // policy plugin registered after the bridge) must still get to block. The + // bridge delegates via next() and folds its context onto the decision. const d = dir() hooks(d, { UserPromptSubmit: [{ hooks: [{ type: 'command', command: sh(d, 'c.sh', '#!/usr/bin/env bash\necho \'{"hookSpecificOutput":{"hookEventName":"UserPromptSubmit","additionalContext":"bridge ctx"}}\'\n') }] }] }) const adapter = new MockAdapter([textResponse('should not run')])