From 65165b5d542e6ff987e4d919d7f5d2f1fcaeaa2b Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Wed, 1 Jul 2026 00:38:06 +0800 Subject: [PATCH 1/6] =?UTF-8?q?feat(hooks):=20dsh-hook-protocol=20?= =?UTF-8?q?=E2=80=94=20shared=20Claude=20Code=20/=20Codex=20hook=20wire-pr?= =?UTF-8?q?otocol=20core?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two hook bridges (dsh-hooks-claude, dsh-hooks-codex) would otherwise duplicate the bulk of the protocol — Codex deliberately reimplements a SUBSET of the Claude Code protocol (same hooks.json shape, exit-code/stdout contract, command-hook model). This library holds the genuinely-identical primitives; each bridge owns only what differs (per-event stdin payload, env/substitution, decision mapping). New packages/hooks/ group; hook-protocol is a LIBRARY (no plugin, registers/injects nothing): - matcher: matchesMatcher(pattern, query, mode) — the one dialect axis collapsed to a mode param (claude = literal-or-regex with pipe alternation; codex = always unanchored regex). Match-all on absent/''/'*'; invalid regex matches nothing. - codec: parseHookOutput(exit, stdout, stderr) → dialect-neutral HookOutput. Exit 0 → lenient JSON; exit 2 → blocking error (stderr = reason, surfaced as decision:'block'); other → non-blocking. Parses the CC superset (continue/stopReason/decision/hookSpecificOutput.{permissionDecision, additionalContext,updatedInput}/systemMessage); permissionDecision overrides the legacy top-level decision. - runner: runHook(bash, hook, opts, now) — runs a command hook via ctx.bash (stdin payload + trusted-plugin env), honors timeoutSec, never throws (executor reject → non-blocking-error HookOutput). Injected clock for testable durations. - merge: mergeHookOutputs — most-restrictive fold (deny>ask>allow, sticky stop, block reasons joined, context/system-messages accumulated). - hook/* session events (declaration-merged into SessionEventMap, log-only like compact/*) + appendHookInvoked/appendHookResult helpers. updatedInput is parsed but NOT honored (deferred pre-tool-input-rewrite RFC); a bridge logs+warns. 47 unit tests at per-file 100% (matcher per-mode, codec per exit-code/field, runner plumbing w/ stub executor, merge precedence, hook/* helpers). RFC: implemented/feature/2026-06-30-hook-protocol-lib.md. --- AGENTS.md | 4 + docs/core-data-structures/session.md | 2 +- docs/module-graph.md | 3 + docs/rfc/README.md | 1 + .../feature/2026-06-30-hook-protocol-lib.md | 32 +++++ packages/README.md | 2 + packages/hooks/README.md | 11 ++ packages/hooks/hook-protocol/README.md | 35 +++++ packages/hooks/hook-protocol/package.json | 34 +++++ packages/hooks/hook-protocol/src/codec.ts | 129 +++++++++++++++++ packages/hooks/hook-protocol/src/events.ts | 72 ++++++++++ packages/hooks/hook-protocol/src/index.ts | 38 +++++ packages/hooks/hook-protocol/src/matcher.ts | 49 +++++++ packages/hooks/hook-protocol/src/merge.ts | 109 ++++++++++++++ packages/hooks/hook-protocol/src/runner.ts | 91 ++++++++++++ packages/hooks/hook-protocol/src/types.ts | 131 +++++++++++++++++ .../hooks/hook-protocol/tests/codec.spec.ts | 106 ++++++++++++++ .../hooks/hook-protocol/tests/events.spec.ts | 61 ++++++++ .../hooks/hook-protocol/tests/matcher.spec.ts | 58 ++++++++ .../hooks/hook-protocol/tests/merge.spec.ts | 82 +++++++++++ .../hooks/hook-protocol/tests/runner.spec.ts | 134 ++++++++++++++++++ packages/hooks/hook-protocol/tsconfig.json | 24 ++++ pnpm-lock.yaml | 12 ++ tsconfig.base.json | 1 + tsconfig.build.json | 3 +- tsconfig.json | 3 +- 26 files changed, 1224 insertions(+), 3 deletions(-) create mode 100644 docs/rfc/implemented/feature/2026-06-30-hook-protocol-lib.md create mode 100644 packages/hooks/README.md create mode 100644 packages/hooks/hook-protocol/README.md create mode 100644 packages/hooks/hook-protocol/package.json create mode 100644 packages/hooks/hook-protocol/src/codec.ts create mode 100644 packages/hooks/hook-protocol/src/events.ts create mode 100644 packages/hooks/hook-protocol/src/index.ts create mode 100644 packages/hooks/hook-protocol/src/matcher.ts create mode 100644 packages/hooks/hook-protocol/src/merge.ts create mode 100644 packages/hooks/hook-protocol/src/runner.ts create mode 100644 packages/hooks/hook-protocol/src/types.ts create mode 100644 packages/hooks/hook-protocol/tests/codec.spec.ts create mode 100644 packages/hooks/hook-protocol/tests/events.spec.ts create mode 100644 packages/hooks/hook-protocol/tests/matcher.spec.ts create mode 100644 packages/hooks/hook-protocol/tests/merge.spec.ts create mode 100644 packages/hooks/hook-protocol/tests/runner.spec.ts create mode 100644 packages/hooks/hook-protocol/tsconfig.json diff --git a/AGENTS.md b/AGENTS.md index 833aeeea45..020447fe7a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -77,6 +77,10 @@ packages/ Harness packages, grouped by role at packages///. tool-todo/ model-facing todo_write tool: writes the whole task list to the session log (todo/write), rendered as a stdio checklist / ACP plan + hooks/ hook bridges + shared wire protocol + 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 session-persistence/ persistence capability family session-persistence/ durable persistence seam + write coordinator session-persistence-jsonl/ JSONL-sidecar backend diff --git a/docs/core-data-structures/session.md b/docs/core-data-structures/session.md index 21c92612f2..eb323dcd3e 100644 --- a/docs/core-data-structures/session.md +++ b/docs/core-data-structures/session.md @@ -6,7 +6,7 @@ Source: [`packages/core/session/src/types.ts`](../../packages/core/session/src/t ## `SessionEventMap` — the event vocabulary -The append-only event types. Merge-extensible: a plugin declares extra event types via declaration merging — e.g. the [compaction seam](compaction.md) adds `compact/start` / `compact/summary` / `compact/end`. +The append-only event types. Merge-extensible: a plugin declares extra event types via declaration merging — e.g. the [compaction seam](compaction.md) adds `compact/start` / `compact/summary` / `compact/end`, and `@deepseek-ai/dsh-hook-protocol` adds log-only `hook/invoked` / `hook/result` provenance for a hook bridge. Like `compact/*`, these are NOT `SurfaceEventType`s (no `surfaceOp`). ```ts type-equiv interface SessionEventMap { diff --git a/docs/module-graph.md b/docs/module-graph.md index c2736abca1..3f64c7731e 100644 --- a/docs/module-graph.md +++ b/docs/module-graph.md @@ -20,6 +20,8 @@ graph TD agent --> session compact --> llm compact --> session + hook-protocol --> bash + hook-protocol --> session llm-replay --> llm llm-replay --> session session-persistence --> session @@ -107,6 +109,7 @@ graph TD | `system-prompt` | `llm` | | `agent` | `brand`, `llm`, `session` | | `compact` | `llm`, `session` | +| `hook-protocol` | `bash`, `session` | | `llm-replay` | `llm`, `session` | | `session-persistence` | `session` | | `invariants` | `agent`, `llm`, `session` | diff --git a/docs/rfc/README.md b/docs/rfc/README.md index 632b63ff56..fbeb6966e7 100644 --- a/docs/rfc/README.md +++ b/docs/rfc/README.md @@ -89,6 +89,7 @@ Do NOT write one for a mechanical or local choice (a variable name, a one-file r | [The `todo_write` tool — model task list as event-sourced session state](implemented/feature/2026-06-29-todo-write-tool.md) | 2026-06-29 | | [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 | ### Simplification diff --git a/docs/rfc/implemented/feature/2026-06-30-hook-protocol-lib.md b/docs/rfc/implemented/feature/2026-06-30-hook-protocol-lib.md new file mode 100644 index 0000000000..ecb45ea2c3 --- /dev/null +++ b/docs/rfc/implemented/feature/2026-06-30-hook-protocol-lib.md @@ -0,0 +1,32 @@ +# RFC: dsh-hook-protocol — the shared Claude Code / Codex hook wire-protocol core + +Status: implemented (accepted 2026-06-30) + + + +## Context + +The hooks subsystem ships two bridge plugins: one that runs a user's existing Claude Code (CC) hooks, one for Codex hooks. Studying the reference implementations (`~/repos/refs/claude-code`, `~/repos/refs/codex`) surfaced a decisive fact: **Codex deliberately reimplements a SUBSET of the CC hook protocol.** Its engine reads the same `hooks.json`, uses the same matcher-group shape, the same exit-code/structured-stdout output contract, and the same command-hook execution model — Codex's source even names the engine after Claude's and comments where it "intentionally diverges." So the two bridges would otherwise duplicate the bulk of the protocol. + +This RFC introduces `@deepseek-ai/dsh-hook-protocol`, a **library** (not a plugin — it registers and injects nothing) holding the genuinely-identical primitives both bridges build on. The split between shared and per-dialect is the design's center of gravity. + +## Decision + +A new `packages/hooks/` group with `hook-protocol` as a pure library. It owns four primitive families and the `hook/*` session events; each bridge (PR-F) owns what genuinely differs. + +**Shared (here):** +- **Matcher** — `matchesMatcher(pattern, query, mode)`. The ONE axis the dialects differ on is collapsed to the `mode` parameter: `claude` treats a pure `[A-Za-z0-9_|]+` pattern as a literal (pipe = exact-match alternation) and anything else as a regex; `codex` is always an unanchored regex. Match-all on absent/`''`/`'*'`; an invalid regex matches nothing (never throws into the loop). +- **Execution** — `runHook(bash, hook, options, now)`. Runs a command hook through the `ctx.bash` seam rather than a bespoke `spawn`: the executor already provides the scrubbed-but-overridable env, process-group kills, and timeout the protocol needs, and `dsh-bash`'s `stdin`/`env` fields (added in the bash-seam PR for exactly this) are the trusted-plugin surface an in-process bridge is allowed to use. It serializes the bridge-built payload to stdin (trailing newline iff CC), honors the hook's `timeoutSec`, and never throws (an executor rejection becomes a non-blocking-error `HookOutput`). +- **Decode** — `parseHookOutput(exit, stdout, stderr)`, the exit-code + structured-stdout codec, producing a dialect-neutral `HookOutput`. Exit `0` → lenient JSON parse of stdout; exit `2` → blocking error with `stderr` as the reason (surfaced as `decision: 'block'` so no caller needs a separate exit-code branch); other → non-blocking error. Parses the full CC superset (`continue`/`stopReason`/`suppressOutput`/`decision`/`hookSpecificOutput.{permissionDecision,additionalContext,updatedInput}`/`systemMessage`); the bridge honors only the subset meaningful for its dialect. +- **Merge** — `mergeHookOutputs(outputs)`, folding multiple matched hooks into one most-restrictive `MergedHookOutcome`: permission precedence **deny > ask > allow**, halt sticky on the first `continue:false`, block reasons joined `\n\n`, context/system-messages accumulated in order. +- **`hook/*` session events** — `hook/invoked` / `hook/result`, declaration-merged into `SessionEventMap` (log-only, like `compact/*` — NOT `SurfaceEventType`s), with `appendHookInvoked`/`appendHookResult` helpers so the invoked/result pairing and turn-enclosure stay consistent across bridges. + +**Per-dialect (the bridges, PR-F):** building each event's stdin payload (CC's base+per-event field sets vs Codex's snake_case with `turn_id`/`model` extras), the dialect's env + `${CLAUDE_PLUGIN_ROOT}` substitution (CC) vs none (Codex), and mapping the neutral `HookOutput`/`MergedHookOutcome` onto the harness's seam-specific typed Decisions (`PreToolDecision`, `PromptDecision`, `ContinuationDecision`, `PostToolDecision`). + +### Why "shared core + per-dialect adapters", not "one parameterized engine" + +A single engine parameterized by a full `dialect` descriptor was considered and rejected. The payload construction and decision mapping are where the dialects genuinely diverge (different field names, different supported outputs, CC's env/substitution); folding those into a data-driven descriptor would make the *bridge* logic indirect — a reader of `dsh-hooks-claude` would have to chase a descriptor to see what payload it sends. Keeping the truly-identical primitives shared (matcher, codec, runner, merge, events) and letting each bridge write its own straightforward payload+mapping keeps each bridge readable standalone, at the cost of a little duplication in the payload shape. The primitives are the part where duplication would actually be dangerous (a divergent matcher or exit-code rule is a correctness bug); the payload is the part where explicitness beats sharing. + +## Consequences + +The two bridges (PR-F) become thin: parse the config file, pick a matcher mode, build the per-event payload+env, call `runHook` + `mergeHookOutputs`, map the outcome to a Decision, and append `hook/*`. The protocol's correctness-critical halves (matcher semantics, exit-code contract, merge precedence) live in one tested place — `hook-protocol` ships with heavy unit tests (matcher per-mode, codec per exit-code/field, runner plumbing with a stub executor, merge precedence, the `hook/*` helpers) at per-file 100%. Input rewrite (`updatedInput`) is parsed but not honored (the deferred [pre-tool-input-rewrite RFC](../../proposed/feature/2026-06-30-pre-tool-input-rewrite.md)); a bridge logs+warns on it. The package is a library, so it has no `cordis.yml` load path of its own — its real-load-path coverage comes through the bridge plugins that consume it (PR-F). diff --git a/packages/README.md b/packages/README.md index 7f63e305dc..e14a1bff0e 100644 --- a/packages/README.md +++ b/packages/README.md @@ -14,6 +14,7 @@ Packages are grouped by modular role at `packages///`. The group dir | [`compact/`](compact/README.md) | Compaction capability family: the abstract seam (backend + tool deferred) | Product — stable surface | | [`subagent/`](subagent/README.md) | Subagent capability family: the provider-registry seam and the model-facing delegation tool | Product — stable surface | | [`todo/`](todo/README.md) | Todo/planning family: the model-facing `todo_write` tool (whole-list task tracking on the session log) | Product — stable surface | +| [`hooks/`](hooks/README.md) | Hook bridges + the shared Claude Code / Codex wire-protocol library | Product — stable surface | | [`session-persistence/`](session-persistence/README.md) | Persistence capability family: the seam + JSONL/SQLite backends | Product — stable surface | | [`ui/`](ui/README.md) | Editor/client integration surfaces (the ACP bridge) | Product — stable surface | | [`support/`](support/README.md) | Dev/test/example infrastructure (invariants, stdio UI, replay adapter) | Support — lower compatibility expectations | @@ -88,6 +89,7 @@ The rule: **extension** plugins depend on interfaces, never on the concrete loop | `subagent-mock/` | `support` | Scripted `SubagentProvider` for testing the seam through the real load path | (registers on `ctx.subagents`) | | `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) | | `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/README.md b/packages/hooks/README.md new file mode 100644 index 0000000000..2bdb65d5bf --- /dev/null +++ b/packages/hooks/README.md @@ -0,0 +1,11 @@ +# hooks/ — hook bridges + shared protocol + +The hooks subsystem lets users extend the agent at lifecycle points the way Claude Code and Codex do — by pointing a bridge plugin at an existing `hooks.json` (or settings) so those external shell hooks run faithfully. The canonical extension surface itself is the harness's typed interception seams ([the interception-seams RFC](../../docs/rfc/implemented/feature/2026-06-30-interception-seams.md)); a "native hook" is just an ordinary cordis plugin on those seams. These packages are the **bridges** that translate the external shell-hook protocol onto that same surface, plus the shared wire-protocol library they build on. + +| Package | Role | Shape | +|---|---|---| +| `hook-protocol/` | Shared wire-protocol core: matcher primitive, exit-code/stdout codec, `runHook` (via `ctx.bash`), most-restrictive merge, `hook/*` session events | library (no plugin) | +| `hooks-claude/` | Bridge for a Claude Code `hooks.json` / settings | plugin | +| `hooks-codex/` | Bridge for a Codex `hooks.json` | plugin | + +Codex deliberately reimplements a *subset* of the Claude Code protocol (same `hooks.json` shape, 5 events vs CC's many, command-only, regex-only matcher, no env/substitution), so `hook-protocol` owns the genuinely-identical primitives and each bridge owns only what differs (its per-event stdin payload, env, and the mapping of a hook's neutral outcome onto the harness's typed Decisions). See [hook-protocol/README.md](hook-protocol/README.md). diff --git a/packages/hooks/hook-protocol/README.md b/packages/hooks/hook-protocol/README.md new file mode 100644 index 0000000000..1983cd0411 --- /dev/null +++ b/packages/hooks/hook-protocol/README.md @@ -0,0 +1,35 @@ +# @deepseek-ai/dsh-hook-protocol + +The **shared core** of the Claude Code / Codex hook wire protocol. NOT a cordis plugin — it registers nothing and injects nothing. It is a **library** of dialect-neutral primitives the two bridge plugins (`@deepseek-ai/dsh-hooks-claude`, `@deepseek-ai/dsh-hooks-codex`) import so neither re-implements the identical halves of the protocol. + +Why a shared lib at all: Codex deliberately reimplements a *subset* of the Claude Code hook protocol — the same `hooks.json` matcher-group shape, the same exit-code/stdout output contract, the same command-hook execution model. The genuinely-shared parts live here; each bridge owns only what differs. + +## What's shared (here) vs. per-dialect (the bridges) + +| Concern | Here (`dsh-hook-protocol`) | The bridge (`dsh-hooks-claude` / `-codex`) | +|---|---|---| +| Matcher test | `matchesMatcher(pattern, query, mode)` — literal-or-regex by `mode` | picks its `mode` (`claude` = literal-or-regex, `codex` = always regex) | +| Run a hook | `runHook(bash, hook, opts, now)` — stdin payload + env via `ctx.bash`, decode | builds the per-event stdin **payload** + the dialect's **env** | +| Decode output | `parseHookOutput(exit, stdout, stderr)` → neutral `HookOutput` | maps the neutral `HookOutput` onto a seam-specific typed Decision | +| Merge N hooks | `mergeHookOutputs(outputs)` → most-restrictive `MergedHookOutcome` | — | +| Durable record | `appendHookInvoked` / `appendHookResult` (`hook/*` session events) | calls them around each invocation | + +## Primitives + +- **`matchesMatcher(matcher, query, mode)`** — match-all on absent/`''`/`'*'`; `claude` mode treats a pure `[A-Za-z0-9_|]+` pattern as a literal (pipe = exact-match alternation) and anything else as a regex; `codex` mode is always an unanchored regex. An invalid regex matches nothing (never throws). +- **`runHook(bash, hook, options, now)`** — serialize `options.payload` to the hook's stdin (with a trailing newline iff `options.trailingNewline`), merge `options.env` after the executor's credential scrub (the `dsh-bash` trusted-plugin surface), honor the hook's `timeoutSec` (else `defaultTimeoutMs`), and decode the result. Never throws: an executor rejection (infra fault) becomes a `HookOutput` with `exitCode: undefined` (a non-blocking error). `now` is injected for testable durations. +- **`parseHookOutput(exitCode, stdout, stderr)`** — the exit-code + structured-stdout codec. Exit `0` → parse JSON stdout (lenient: non-JSON is left for the bridge); exit `2` → blocking error, `stderr` is the block reason (surfaced as `decision: 'block'`); other → non-blocking error. `hookSpecificOutput.permissionDecision` (allow/deny/ask) overrides a legacy top-level `decision`; `additionalContext`/`updatedInput`/`systemMessage`/`continue`/`stopReason`/`suppressOutput` are parsed too. Pure and total. +- **`mergeHookOutputs(outputs)`** — fold the results of every hook that matched one point: permission precedence **deny > ask > allow**, halt sticky on the first `continue:false`, block reasons joined with `\n\n`, `additionalContext`/`systemMessages` accumulated in order. + +## `hook/*` session events + +Declaration-merged into `SessionEventMap` (log-only, like `compact/*` — NOT a `SurfaceEventType`, no `surfaceOp`): + +- `hook/invoked` — `{ turn, point, dialect, matcher?, handlerId }`: a hook command ran. +- `hook/result` — `{ turn, point, handlerId, decision, exitCode?, stderrSummary?, durationMs }`: its outcome, paired by `handlerId`. + +Like every event they must sit inside an open turn. The mid-turn points (`PreToolUse`/`PostToolUse`/`UserPromptSubmit`/`Stop`) fire inside the loop's open turn by construction; `SessionStart` gets no `hook/*` record (its injected `context/message` is the durable evidence) — see the hooks RFC. + +## Input rewrite is parsed but not honored + +`HookOutput.updatedInput` carries a hook's requested tool-input rewrite (CC `updatedInput`), but the harness does not honor it yet — 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)). A bridge logs + warns when a hook sets it. See `src/types.ts` for the full contracts. diff --git a/packages/hooks/hook-protocol/package.json b/packages/hooks/hook-protocol/package.json new file mode 100644 index 0000000000..2220220769 --- /dev/null +++ b/packages/hooks/hook-protocol/package.json @@ -0,0 +1,34 @@ +{ + "name": "@deepseek-ai/dsh-hook-protocol", + "description": "Shared Claude Code / Codex hook wire protocol: matcher engine, stdin/exit-code/stdout codec, multi-hook merge, and hook/* session events", + "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", + "peerDependencies": { + "@deepseek-ai/dsh-bash": "^0.0.1", + "@deepseek-ai/dsh-session": "^0.0.1", + "cordis": "^4.0.0-rc.6" + }, + "devDependencies": { + "@deepseek-ai/dsh-bash": "workspace:^", + "@deepseek-ai/dsh-session": "workspace:^", + "cordis": "^4.0.0-rc.6" + } +} diff --git a/packages/hooks/hook-protocol/src/codec.ts b/packages/hooks/hook-protocol/src/codec.ts new file mode 100644 index 0000000000..f6065af90e --- /dev/null +++ b/packages/hooks/hook-protocol/src/codec.ts @@ -0,0 +1,129 @@ +/** + * Parse a finished hook command's process outcome (exit code + stdout + stderr) + * into the dialect-neutral {@link HookOutput} both bridges map from. + * + * The exit-code contract is shared by Claude Code and Codex: + * - exit 0 → success; if stdout is structured JSON, parse it; else the plain + * stdout is available to the bridge (some events treat it as `additionalContext`). + * - exit 2 → BLOCKING error; stderr is the block reason fed back to the model. + * We surface this as `decision: 'block'` with `reason = stderr` so a bridge + * needs no separate exit-code branch — the neutral output already says "block". + * - other → non-blocking error; recorded (exitCode + stderr) but no decision. + * + * Structured-stdout fields are a SUPERSET across dialects (CC is richest); we + * parse every field we recognize and leave it to the bridge to honor only the + * subset meaningful for its dialect/hook point (Codex, e.g., ignores + * `allow`/`ask`/`updatedInput`). + * + * @module @deepseek-ai/dsh-hook-protocol/codec + */ + +import type { HookOutput } from './types.ts' + +/** The exit code a hook uses to signal a blocking error (stderr → model). */ +export const BLOCKING_EXIT_CODE = 2 + +/** Read a string field from a parsed object, or `undefined` if absent/wrong type. */ +function str(obj: Record, key: string): string | undefined { + const v = obj[key] + return typeof v === 'string' ? v : undefined +} + +/** Read a boolean field, or `undefined` if absent/wrong type. */ +function bool(obj: Record, key: string): boolean | undefined { + const v = obj[key] + return typeof v === 'boolean' ? v : undefined +} + +/** A plain (non-null, non-array) object, or `undefined`. */ +function obj(value: unknown): Record | undefined { + return typeof value === 'object' && value !== null && !Array.isArray(value) + ? value as Record + : undefined +} + +/** Normalize a raw `decision`/`permissionDecision` string to the neutral enum. */ +function decisionOf(value: string | undefined): HookOutput['decision'] { + switch (value) { + case 'approve': case 'allow': case 'block': case 'deny': case 'ask': + return value + default: + return undefined + } +} + +/** + * Parse one finished hook command into a {@link HookOutput}. `stdout`/`stderr` + * are the captured streams; `exitCode` is the process exit (`undefined` when the + * hook could not be spawned at all). Pure and total — never throws; malformed + * JSON on a 0 exit is treated as "no structured output" (the plain stdout is + * still on the bridge to use), matching both reference engines' lenient parse of + * non-JSON stdout. + */ +export function parseHookOutput(exitCode: number | undefined, stdout: string, stderr: string): HookOutput { + const trimmedErr = stderr.trim() + const output: HookOutput = { exitCode, stderr: trimmedErr } + + // Exit 2 is a blocking error in both dialects: stderr is the reason. Surface + // it as a `block` decision so the bridge maps it uniformly with a structured + // `decision:'block'` — the exit code and the JSON channel converge here. + if (exitCode === BLOCKING_EXIT_CODE) { + output.decision = 'block' + if (trimmedErr.length > 0) output.reason = trimmedErr + } + + // Structured stdout is only consulted on a clean (0) exit; on a blocking exit + // the stderr channel is authoritative. A non-zero/undefined exit other than 2 + // carries no decision (the bridge records it as a non-blocking error). + if (exitCode === 0) { + const trimmedOut = stdout.trim() + // Only attempt JSON when stdout looks like a JSON object — matches the + // reference engines, which treat other stdout as plain text, not an error. + if (trimmedOut.startsWith('{')) { + let parsed: Record | undefined + try { + parsed = obj(JSON.parse(trimmedOut)) + } catch { + // Malformed JSON on a clean exit = no structured output (lenient, as the + // reference engines are). The plain stdout remains the bridge's to use. + parsed = undefined + } + if (parsed) applyStructured(output, parsed) + } + } + + return output +} + +/** Fold a parsed structured-stdout object into `output` (mutates in place). */ +function applyStructured(output: HookOutput, parsed: Record): void { + const cont = bool(parsed, 'continue') + if (cont !== undefined) output.continue = cont + const stopReason = str(parsed, 'stopReason') + if (stopReason !== undefined) output.stopReason = stopReason + const suppress = bool(parsed, 'suppressOutput') + if (suppress !== undefined) output.suppressOutput = suppress + const sysMsg = str(parsed, 'systemMessage') + if (sysMsg !== undefined) output.systemMessage = sysMsg + + // Top-level legacy `decision` + `reason` (CC approve/block; Codex block). + const topDecision = decisionOf(str(parsed, 'decision')) + if (topDecision !== undefined) output.decision = topDecision + const topReason = str(parsed, 'reason') + if (topReason !== undefined) output.reason = topReason + + // hookSpecificOutput: the per-event channel. permissionDecision (allow/deny/ + // ask) OVERRIDES the legacy top-level decision when present; additionalContext + // and updatedInput live here too. + const hso = obj(parsed.hookSpecificOutput) + if (hso) { + const permission = decisionOf(str(hso, 'permissionDecision')) + if (permission !== undefined) output.decision = permission + const permissionReason = str(hso, 'permissionDecisionReason') + if (permissionReason !== undefined) output.reason = permissionReason + const addCtx = str(hso, 'additionalContext') + if (addCtx !== undefined) output.additionalContext = addCtx + const updated = obj(hso.updatedInput) + if (updated !== undefined) output.updatedInput = updated + } +} diff --git a/packages/hooks/hook-protocol/src/events.ts b/packages/hooks/hook-protocol/src/events.ts new file mode 100644 index 0000000000..0db75995c9 --- /dev/null +++ b/packages/hooks/hook-protocol/src/events.ts @@ -0,0 +1,72 @@ +/** + * Append helpers for the log-only `hook/*` session events — the durable record + * that a hook ran and what it decided. Thin wrappers over `session.append` so a + * bridge does not hand-build the payloads (and so the `turn`-enclosure + + * invoked/result pairing stay consistent across both bridges). + * + * `hook/*` events are log-only (not {@link SurfaceEventType}), so they carry no + * `surfaceOp` and append with no surface intent — but, like every event, they + * must sit inside an OPEN turn (the invariants oracle rejects an un-enclosed + * event). The mid-turn hook points (`PreToolUse`/`PostToolUse`/`UserPromptSubmit`/ + * `Stop`) fire inside the loop's open turn by construction; `SessionStart` is the + * exception (its injected `context/message` is the durable evidence instead), so + * a bridge does NOT write `hook/*` for session-start — see the hooks RFC. + * + * @module @deepseek-ai/dsh-hook-protocol/events + */ + +import type { Session } from '@deepseek-ai/dsh-session' +import type { HookDialect } from './types.ts' + +/** What identifies a hook invocation across its invoked/result pair. */ +export interface HookInvocation { + /** The open turn the invocation lives inside. */ + turn: number + /** The hook point (`PreToolUse`, `Stop`, …). */ + point: string + /** The bridge dialect that ran it. */ + dialect: HookDialect + /** A stable id correlating the invoked event with its result. */ + handlerId: string + /** The matcher-group pattern that selected it (absent for match-all). */ + matcher?: string +} + +/** The decided outcome half of the pair. */ +export interface HookResultRecord { + turn: number + point: string + handlerId: string + /** The dialect-neutral decision the bridge resolved (`deny`/`allow`/`block`/…). */ + decision: string + /** The process exit code (absent when the hook could not run). */ + exitCode?: number + /** A truncated stderr summary (the block-reason source on exit 2). */ + stderrSummary?: string + /** Wall-clock duration of the run. */ + durationMs: number +} + +/** Append a `hook/invoked` provenance event to `session`. */ +export function appendHookInvoked(session: Session, invocation: HookInvocation): void { + session.append('hook/invoked', { + turn: invocation.turn, + point: invocation.point, + dialect: invocation.dialect, + handlerId: invocation.handlerId, + ...invocation.matcher !== undefined ? { matcher: invocation.matcher } : {}, + }) +} + +/** Append a `hook/result` outcome event to `session` (pairs with a prior `hook/invoked`). */ +export function appendHookResult(session: Session, record: HookResultRecord): void { + session.append('hook/result', { + turn: record.turn, + point: record.point, + handlerId: record.handlerId, + decision: record.decision, + ...record.exitCode !== undefined ? { exitCode: record.exitCode } : {}, + ...record.stderrSummary !== undefined ? { stderrSummary: record.stderrSummary } : {}, + durationMs: record.durationMs, + }) +} diff --git a/packages/hooks/hook-protocol/src/index.ts b/packages/hooks/hook-protocol/src/index.ts new file mode 100644 index 0000000000..686a1480ac --- /dev/null +++ b/packages/hooks/hook-protocol/src/index.ts @@ -0,0 +1,38 @@ +/** + * `@deepseek-ai/dsh-hook-protocol` — the shared core of the Claude Code / Codex + * hook wire protocol. NOT a cordis plugin: it registers nothing and injects + * nothing. It is a LIBRARY of dialect-neutral primitives the two bridge plugins + * (`dsh-hooks-claude`, `dsh-hooks-codex`) import to avoid re-implementing the + * identical halves of the protocol: + * + * - {@link matchesMatcher} — the matcher primitive (literal-or-regex by dialect). + * - {@link runHook} + {@link parseHookOutput} — run a command hook via `ctx.bash` + * (stdin payload + env) and decode its exit-code/stdout/stderr into a neutral + * {@link HookOutput}. + * - {@link mergeHookOutputs} — fold multiple matched hooks into one + * most-restrictive {@link MergedHookOutcome} (deny > ask > allow). + * - {@link appendHookInvoked} / {@link appendHookResult} — the log-only `hook/*` + * session-event helpers (declaration-merged into `SessionEventMap`). + * + * Each bridge owns what genuinely DIFFERS: building the per-event stdin payload + * (CC vs Codex field sets), the dialect's env/substitution, and mapping the + * neutral outcome onto the harness's seam-specific typed Decisions. + * + * @module @deepseek-ai/dsh-hook-protocol + */ + +export type { + CommandHook, + HookDialect, + HookOutput, + MatcherGroup, + MatcherMode, +} from './types.ts' +export { matchesMatcher } from './matcher.ts' +export { BLOCKING_EXIT_CODE, parseHookOutput } from './codec.ts' +export { runHook } from './runner.ts' +export type { RunHookOptions, RunHookResult } from './runner.ts' +export { mergeHookOutputs } from './merge.ts' +export type { MergedDecision, MergedHookOutcome } from './merge.ts' +export { appendHookInvoked, appendHookResult } from './events.ts' +export type { HookInvocation, HookResultRecord } from './events.ts' diff --git a/packages/hooks/hook-protocol/src/matcher.ts b/packages/hooks/hook-protocol/src/matcher.ts new file mode 100644 index 0000000000..4ce3e4b62d --- /dev/null +++ b/packages/hooks/hook-protocol/src/matcher.ts @@ -0,0 +1,49 @@ +/** + * The matcher primitive shared by both hook dialects: decide whether a matcher + * pattern selects a given query (a tool name, a session source, …). + * + * The two dialects differ ONLY in how a non-empty pattern is interpreted, so + * that single axis is the {@link MatcherMode} parameter: + * - `claude`: a pattern of purely `[A-Za-z0-9_|]+` is a LITERAL (pipe = + * exact-match alternation, e.g. `Edit|Write`); anything else is a regex. + * - `codex`: every pattern is an unanchored regex (no literal fast path). + * + * Both treat an absent / empty / `'*'` pattern as match-all, and both treat an + * invalid regex as a non-match (the bridge logs it; a broken matcher must not + * throw into the loop). + * + * @module @deepseek-ai/dsh-hook-protocol/matcher + */ + +import type { MatcherMode } from './types.ts' + +/** True for an absent / empty / `'*'` pattern — the match-all sentinels. */ +function isMatchAll(matcher: string | undefined): boolean { + return matcher === undefined || matcher === '' || matcher === '*' +} + +/** A Claude-literal pattern is purely word chars + `|` (the regex-vs-literal discriminator). */ +const CLAUDE_LITERAL = /^[A-Za-z0-9_|]+$/ + +/** + * Whether `matcher` selects `query` under the given dialect {@link MatcherMode}. + * Match-all sentinels (absent/`''`/`'*'`) always match. A `claude` literal + * pattern exact-matches the query (splitting `|` into alternatives); every other + * `claude` pattern and ALL `codex` patterns are tested as an unanchored regex. + * An invalid regex matches nothing (never throws). + */ +export function matchesMatcher(matcher: string | undefined, query: string, mode: MatcherMode): boolean { + if (isMatchAll(matcher)) return true + // matcher is a non-empty string past the match-all guard. + const pattern = matcher as string + if (mode === 'claude' && CLAUDE_LITERAL.test(pattern)) { + return pattern.split('|').includes(query) + } + try { + return new RegExp(pattern).test(query) + } catch { + // Invalid regex: a broken matcher selects nothing rather than throwing into + // the agent loop. The bridge is responsible for surfacing the bad config. + return false + } +} diff --git a/packages/hooks/hook-protocol/src/merge.ts b/packages/hooks/hook-protocol/src/merge.ts new file mode 100644 index 0000000000..d8fb158eb1 --- /dev/null +++ b/packages/hooks/hook-protocol/src/merge.ts @@ -0,0 +1,109 @@ +/** + * Merge the outcomes of MULTIPLE hooks that matched one hook point into a single + * most-restrictive {@link MergedHookOutcome}. Both reference engines run matched + * hooks concurrently and fold their results; the precedence rules here are the + * intersection both dialects agree on (and the strictest interpretation where + * they differ), so a bridge gets one decision to map onto its seam: + * + * - **permission precedence `deny > ask > allow`**: any `deny`/`block` wins; an + * `ask` overrides `allow`; `allow`/`approve` only stands if nothing stricter + * appeared. (Claude Code's explicit precedence; Codex only ever blocks, so the + * rule degenerates correctly for it.) + * - **halt is sticky**: the first hook with `continue:false` sets `stop` and its + * `stopReason`. + * - **reasons accumulate**: block/deny reasons are joined with `\n\n` (Codex's + * `join_text_chunks`), so the model sees every objection, not just the first. + * - **context accumulates**: `additionalContext` from every hook is collected in + * order (CC concatenates; Codex keeps them as separate developer messages — + * either way the bridge gets the ordered list). + * - **systemMessages accumulate** likewise. + * + * @module @deepseek-ai/dsh-hook-protocol/merge + */ + +import type { HookOutput } from './types.ts' + +/** The single decision a hook point resolves to after merging all matched hooks. */ +export type MergedDecision = 'allow' | 'ask' | 'deny' | 'none' + +/** The folded outcome of every hook that matched one point. */ +export interface MergedHookOutcome { + /** + * The most-restrictive permission decision across all hooks (`deny` > `ask` > + * `allow`), or `none` when no hook expressed one. `block`/`deny` both fold to + * `deny`; `approve`/`allow` both fold to `allow`. + */ + decision: MergedDecision + /** Joined (`\n\n`) reasons from every blocking/denying hook, or `undefined`. */ + reason?: string + /** `true` when any hook asked to halt (`continue:false`). */ + stop: boolean + /** The first halting hook's `stopReason`, when one halted. */ + stopReason?: string + /** Every hook's `additionalContext`, in hook order (no joining — the bridge decides). */ + additionalContext: string[] + /** Every hook's `systemMessage`, in hook order. */ + systemMessages: string[] +} + +/** Rank a single hook's decision for the deny>ask>allow precedence (higher = stricter). */ +function rank(decision: HookOutput['decision']): number { + switch (decision) { + case 'deny': case 'block': return 3 + case 'ask': return 2 + case 'approve': case 'allow': return 1 + default: return 0 // no decision + } +} + +/** Collapse a ranked decision back to the merged enum. */ +function decisionForRank(maxRank: number): MergedDecision { + switch (maxRank) { + case 3: return 'deny' + case 2: return 'ask' + case 1: return 'allow' + default: return 'none' + } +} + +/** + * Fold `outputs` (the results of every hook that matched a point, in hook order) + * into one {@link MergedHookOutcome} by the precedence rules above. An empty list + * yields a neutral outcome (`decision: 'none'`, no stop, empty context) — the + * caller treats that as "no hook had anything to say". + */ +export function mergeHookOutputs(outputs: HookOutput[]): MergedHookOutcome { + let maxRank = 0 + const reasons: string[] = [] + let stop = false + let stopReason: string | undefined + const additionalContext: string[] = [] + const systemMessages: string[] = [] + + for (const out of outputs) { + const r = rank(out.decision) + if (r > maxRank) maxRank = r + // Collect a reason only from a blocking/denying hook (rank 3) — an allow's + // "reason" is not an objection the model needs to see. + if (r === 3 && out.reason !== undefined && out.reason.length > 0) reasons.push(out.reason) + if (out.continue === false && !stop) { + stop = true + if (out.stopReason !== undefined) stopReason = out.stopReason + } + if (out.additionalContext !== undefined && out.additionalContext.length > 0) { + additionalContext.push(out.additionalContext) + } + if (out.systemMessage !== undefined && out.systemMessage.length > 0) { + systemMessages.push(out.systemMessage) + } + } + + return { + decision: decisionForRank(maxRank), + ...reasons.length > 0 ? { reason: reasons.join('\n\n') } : {}, + stop, + ...stopReason !== undefined ? { stopReason } : {}, + additionalContext, + systemMessages, + } +} diff --git a/packages/hooks/hook-protocol/src/runner.ts b/packages/hooks/hook-protocol/src/runner.ts new file mode 100644 index 0000000000..9e26607e3d --- /dev/null +++ b/packages/hooks/hook-protocol/src/runner.ts @@ -0,0 +1,91 @@ +/** + * Run one configured command hook through the `ctx.bash` executor seam and parse + * its outcome into a {@link HookOutput}. This is where the wire protocol's + * EXECUTION half lives: feed the hook its JSON payload on stdin, hand it the + * dialect's env vars, honor its timeout, capture stdout/stderr/exit, and decode. + * + * It runs hooks through `ctx.bash` (not a bespoke `spawn`) deliberately — the + * bash seam already provides the scrubbed-but-overridable env, process-group + * kills, and timeout the protocol needs, and `dsh-bash`'s `stdin`/`env` fields + * are the trusted-plugin surface (added for exactly this) that a hook bridge — + * an in-process plugin, not model output — is allowed to use. + * + * @module @deepseek-ai/dsh-hook-protocol/runner + */ + +import type { BashExecutor } from '@deepseek-ai/dsh-bash' +import { parseHookOutput } from './codec.ts' +import type { CommandHook, HookOutput } from './types.ts' + +/** Everything a single hook invocation needs beyond its command line. */ +export interface RunHookOptions { + /** The JSON payload object written to the hook's stdin (the bridge builds it). */ + payload: unknown + /** Extra env vars for the hook process (`CLAUDE_PROJECT_DIR`, …); the bridge builds these. */ + env?: Record + /** Working directory for the hook (defaults to the executor's own default when omitted). */ + cwd?: string + /** Abort signal — cancels the hook run when fired (the parent step aborts). */ + signal?: AbortSignal + /** Default timeout (ms) when the hook config sets none. */ + defaultTimeoutMs: number + /** Whether to append a trailing newline to the stdin payload (CC yes, Codex no). */ + trailingNewline: boolean +} + +/** The {@link HookOutput} plus the wall-clock duration of the run (for `hook/result`). */ +export interface RunHookResult { + output: HookOutput + durationMs: number +} + +/** + * Run `hook` via `bash` with `options.payload` serialized to its stdin, then + * decode the result. `now` is injected (a monotonic-ms source) so the duration + * is testable without a real clock. The hook's configured `timeoutSec` (wire + * unit: seconds) overrides `defaultTimeoutMs`. The command runs with the + * dialect's `env` merged after the executor's credential scrub (the trusted- + * plugin path). NEVER throws: an infrastructure failure (the executor rejecting) + * is surfaced as a {@link HookOutput} with `exitCode: undefined`, so the caller's + * merge logic treats it as a non-blocking error rather than crashing the turn. + */ +export async function runHook( + bash: BashExecutor, + hook: CommandHook, + options: RunHookOptions, + now: () => number, +): Promise { + const started = now() + const timeoutMs = hook.timeoutSec !== undefined ? hook.timeoutSec * 1000 : options.defaultTimeoutMs + const stdin = JSON.stringify(options.payload) + (options.trailingNewline ? '\n' : '') + + const request = { + command: hook.command, + timeoutMs, + stdin, + ...options.cwd !== undefined ? { workdir: options.cwd } : {}, + ...options.env !== undefined ? { env: options.env } : {}, + ...options.signal ? { signal: options.signal } : {}, + } + + try { + const result = await bash.run(bash.resolve(request)) + // BashRunResult.exitCode is `number | null` (null = died by signal); the + // protocol's exit-code contract is numeric, so a signal death maps to + // `undefined` (a non-blocking error — no clean exit code to act on). + const exitCode = result.exitCode ?? undefined + return { + output: parseHookOutput(exitCode, result.stdout.text, result.stderr.text), + durationMs: now() - started, + } + } catch (error: unknown) { + // The executor rejects only on infrastructure faults (unusable workdir, + // missing shell). A hook that cannot run is a non-blocking error: no exit + // code, the failure on stderr for the record. The turn proceeds. + const message = error instanceof Error ? error.message : String(error) + return { + output: parseHookOutput(undefined, '', message), + durationMs: now() - started, + } + } +} diff --git a/packages/hooks/hook-protocol/src/types.ts b/packages/hooks/hook-protocol/src/types.ts new file mode 100644 index 0000000000..a06ae7f837 --- /dev/null +++ b/packages/hooks/hook-protocol/src/types.ts @@ -0,0 +1,131 @@ +/** + * Dialect-neutral vocabulary for the Claude Code / Codex hook wire protocol, + * plus the log-only `hook/*` session events. Types only — runtime helpers live + * in the sibling modules (`matcher`, `codec`, `runner`, `merge`, `events`). + * + * This package is the SHARED CORE: the truly-identical primitives both the + * `dsh-hooks-claude` and `dsh-hooks-codex` bridges build on. Each bridge owns + * its own per-dialect stdin-payload construction and decision mapping on top of + * these primitives — the divergences (which events exist, literal-vs-regex + * matching, env/substitution, snake_case extras, allow/ask support) are the + * BRIDGE's concern, not this lib's. + * + * @module @deepseek-ai/dsh-hook-protocol/types + */ + +declare module '@deepseek-ai/dsh-session' { + interface SessionEventMap { + /** + * A hook command was invoked at a hook point — log-only provenance (like + * `compact/*`; NOT a {@link SurfaceEventType}, carries no `surfaceOp`). + * `dialect` is the bridge that ran it (`claude`/`codex`/`native`), `point` + * the hook point (`PreToolUse`, `Stop`, …), `matcher` the matcher-group + * pattern that selected it (absent for match-all), `handlerId` a stable id + * for the command (so an invoked/result pair correlates). `turn` is the open + * turn the invocation lives inside. + * @mode emit + */ + 'hook/invoked': { + turn: number + point: string + dialect: HookDialect + matcher?: string + handlerId: string + } + /** + * A hook command's outcome — log-only, paired with a prior `hook/invoked` + * (same `handlerId`). `decision` is the resolved dialect-neutral outcome the + * bridge mapped it to (`allow`/`deny`/`ask`/`block`/`continue`/`stop`/`pass`), + * `exitCode` the process exit (absent if it never ran), `stderrSummary` a + * truncated stderr (the block reason source on exit 2), `durationMs` the wall + * time. `turn` matches the `hook/invoked`. + * @mode emit + */ + 'hook/result': { + turn: number + point: string + handlerId: string + decision: string + exitCode?: number + stderrSummary?: string + durationMs: number + } + } +} + +/** Which protocol dialect a hook config / invocation belongs to. */ +export type HookDialect = 'claude' | 'codex' | 'native' + +/** + * One configured command hook (the `{ type: 'command', command, timeout? }` + * shape shared by both dialects). Non-command hook types (CC's `prompt`/`agent`/ + * `http`) are parsed-and-skipped by a bridge, so only this shape reaches the + * runner. + */ +export interface CommandHook { + /** The shell command line to run. */ + command: string + /** Per-hook timeout in SECONDS (the wire unit); the runner converts to ms. */ + timeoutSec?: number +} + +/** + * One matcher group: a `matcher` pattern (absent / `''` / `'*'` = match-all) + * plus the command hooks that run when it matches. Both dialects share this + * shape (CC's `hooks.json` and Codex's `hooks.json`). + */ +export interface MatcherGroup { + matcher?: string + hooks: CommandHook[] +} + +/** + * How a matcher pattern is interpreted. Claude Code uses {@link literal} when the + * pattern is purely `[A-Za-z0-9_|]+` (pipe = exact-match alternation) and + * {@link regex} otherwise; Codex is always {@link regex}. The bridge picks the + * mode for its dialect. + */ +export type MatcherMode = 'claude' | 'codex' + +/** + * The dialect-neutral OUTCOME a hook produced, parsed from its exit code + + * stdout JSON + stderr by {@link parseHookOutput}. A bridge maps this onto a + * seam-specific typed Decision (PreToolDecision, PromptDecision, …). Every field + * is OPTIONAL because a hook may exercise any subset; the bridge decides which + * fields are meaningful for its hook point and which it ignores (faithful-but- + * degraded — e.g. Codex ignores `allow`/`ask`). + */ +export interface HookOutput { + /** The raw process exit code (`undefined` if the hook could not be run). */ + exitCode: number | undefined + /** Trimmed stderr — the block-reason source on a blocking (exit 2) hook. */ + stderr: string + /** + * `false` ⇒ the hook asked to halt (CC/Codex `continue:false`); pairs with + * {@link stopReason}. `true`/absent ⇒ proceed. + */ + continue?: boolean + /** Human-readable reason shown when {@link continue} is `false`. */ + stopReason?: string + /** Hide the hook's stdout from the transcript (CC `suppressOutput`). */ + suppressOutput?: boolean + /** + * The blocking decision a hook expressed via structured stdout (CC's + * `decision` / `hookSpecificOutput.permissionDecision`): `'block'`/`'deny'` + * forbid the action, `'approve'`/`'allow'` permit it, `'ask'` requests + * confirmation. Absent ⇒ no explicit decision (exit code governs). + */ + decision?: 'approve' | 'allow' | 'block' | 'deny' | 'ask' + /** The reason/explanation accompanying {@link decision}. */ + reason?: string + /** Extra context to inject for the next model request (CC `additionalContext`). */ + additionalContext?: string + /** A warning surfaced to the user (CC `systemMessage`). */ + systemMessage?: string + /** + * A tool-input rewrite a hook requested (CC `updatedInput`). PARSED but NOT + * honored — input rewrite is deferred (see the interception-seams RFC); a + * bridge logs + warns when this is present. + */ + updatedInput?: Record +} diff --git a/packages/hooks/hook-protocol/tests/codec.spec.ts b/packages/hooks/hook-protocol/tests/codec.spec.ts new file mode 100644 index 0000000000..41976aeab9 --- /dev/null +++ b/packages/hooks/hook-protocol/tests/codec.spec.ts @@ -0,0 +1,106 @@ +import { describe, expect, it } from 'vitest' +import { parseHookOutput } from '@deepseek-ai/dsh-hook-protocol' + +describe('parseHookOutput — exit code semantics', () => { + it('exit 0 with no stdout is a neutral success', () => { + const out = parseHookOutput(0, '', '') + expect(out.exitCode).toBe(0) + expect(out.decision).toBeUndefined() + expect(out.continue).toBeUndefined() + }) + + it('exit 2 is a blocking error: stderr becomes the block decision + reason', () => { + const out = parseHookOutput(2, '', 'this command is not allowed') + expect(out.decision).toBe('block') + expect(out.reason).toBe('this command is not allowed') + expect(out.stderr).toBe('this command is not allowed') + }) + + it('exit 2 with empty stderr still blocks, with no reason', () => { + const out = parseHookOutput(2, '', ' ') + expect(out.decision).toBe('block') + expect(out.reason).toBeUndefined() + }) + + it('other non-zero exit is a non-blocking error (no decision, stderr recorded)', () => { + const out = parseHookOutput(1, '', 'some warning') + expect(out.decision).toBeUndefined() + expect(out.exitCode).toBe(1) + expect(out.stderr).toBe('some warning') + }) + + it('undefined exit (could not run) carries no decision', () => { + const out = parseHookOutput(undefined, '', 'spawn failed: ENOENT') + expect(out.exitCode).toBeUndefined() + expect(out.decision).toBeUndefined() + expect(out.stderr).toBe('spawn failed: ENOENT') + }) +}) + +describe('parseHookOutput — structured stdout (exit 0 only)', () => { + it('parses top-level continue/stopReason/suppressOutput/systemMessage', () => { + const out = parseHookOutput(0, JSON.stringify({ + continue: false, stopReason: 'budget exceeded', suppressOutput: true, systemMessage: 'heads up', + }), '') + expect(out.continue).toBe(false) + expect(out.stopReason).toBe('budget exceeded') + expect(out.suppressOutput).toBe(true) + expect(out.systemMessage).toBe('heads up') + }) + + it('parses legacy top-level decision + reason (approve/block)', () => { + expect(parseHookOutput(0, JSON.stringify({ decision: 'block', reason: 'nope' }), '').decision).toBe('block') + expect(parseHookOutput(0, JSON.stringify({ decision: 'approve' }), '').decision).toBe('approve') + }) + + it('hookSpecificOutput.permissionDecision OVERRIDES the legacy top-level decision', () => { + const out = parseHookOutput(0, JSON.stringify({ + decision: 'approve', + hookSpecificOutput: { permissionDecision: 'deny', permissionDecisionReason: 'denied by policy' }, + }), '') + expect(out.decision).toBe('deny') + expect(out.reason).toBe('denied by policy') + }) + + it('parses allow/ask permissionDecision (the bridge decides whether to honor)', () => { + expect(parseHookOutput(0, JSON.stringify({ hookSpecificOutput: { permissionDecision: 'allow' } }), '').decision).toBe('allow') + expect(parseHookOutput(0, JSON.stringify({ hookSpecificOutput: { permissionDecision: 'ask' } }), '').decision).toBe('ask') + }) + + it('parses additionalContext and updatedInput from hookSpecificOutput', () => { + const out = parseHookOutput(0, JSON.stringify({ + hookSpecificOutput: { additionalContext: 'remember X', updatedInput: { command: 'safe' } }, + }), '') + expect(out.additionalContext).toBe('remember X') + expect(out.updatedInput).toEqual({ command: 'safe' }) + }) + + it('an unknown decision string is ignored (not coerced)', () => { + expect(parseHookOutput(0, JSON.stringify({ decision: 'maybe' }), '').decision).toBeUndefined() + }) + + it('malformed JSON on a clean exit is lenient (no structured output, no throw)', () => { + const out = parseHookOutput(0, '{ not valid json', '') + expect(out.decision).toBeUndefined() + expect(out.continue).toBeUndefined() + }) + + it('non-object stdout (plain text) on exit 0 is left for the bridge (no JSON attempt)', () => { + const out = parseHookOutput(0, 'just some text output', '') + expect(out.decision).toBeUndefined() + expect(out.continue).toBeUndefined() + }) + + it('a JSON array stdout parses but yields no fields (not an object)', () => { + // Starts with '{'? No — '[' — so it is not even attempted. Neutral. + const out = parseHookOutput(0, '[1,2,3]', '') + expect(out.decision).toBeUndefined() + }) + + it('structured stdout is IGNORED on a blocking (exit 2) run — stderr is authoritative', () => { + const out = parseHookOutput(2, JSON.stringify({ decision: 'approve' }), 'blocked') + // exit 2 forces block regardless of what stdout claims + expect(out.decision).toBe('block') + expect(out.reason).toBe('blocked') + }) +}) diff --git a/packages/hooks/hook-protocol/tests/events.spec.ts b/packages/hooks/hook-protocol/tests/events.spec.ts new file mode 100644 index 0000000000..f63ae2a9cb --- /dev/null +++ b/packages/hooks/hook-protocol/tests/events.spec.ts @@ -0,0 +1,61 @@ +import { describe, expect, it } from 'vitest' +import { Session, SessionId } from '@deepseek-ai/dsh-session' +import { appendHookInvoked, appendHookResult } from '@deepseek-ai/dsh-hook-protocol' + +describe('hook/* session events', () => { + it('appendHookInvoked records a log-only hook/invoked (with matcher when present)', () => { + const session = new Session(SessionId('s')) + appendHookInvoked(session, { turn: 1, point: 'PreToolUse', dialect: 'claude', handlerId: 'h1', matcher: 'Bash' }) + + const ev = [...session.events].find(e => e.type === 'hook/invoked') + expect(ev?.type).toBe('hook/invoked') + if (ev?.type === 'hook/invoked') { + expect(ev.data).toMatchObject({ turn: 1, point: 'PreToolUse', dialect: 'claude', handlerId: 'h1', matcher: 'Bash' }) + } + // Log-only: no surfaceOp on the event. + expect((ev as unknown as { surfaceOp?: unknown }).surfaceOp).toBeUndefined() + }) + + it('omits matcher when absent (match-all hook)', () => { + const session = new Session(SessionId('s')) + appendHookInvoked(session, { turn: 2, point: 'Stop', dialect: 'native', handlerId: 'h2' }) + + const ev = [...session.events].find(e => e.type === 'hook/invoked') + if (ev?.type === 'hook/invoked') { + expect('matcher' in ev.data).toBe(false) + } + }) + + it('appendHookResult records the decided outcome, omitting absent optionals', () => { + const session = new Session(SessionId('s')) + appendHookResult(session, { + turn: 1, point: 'PreToolUse', handlerId: 'h1', decision: 'deny', + exitCode: 2, stderrSummary: 'blocked', durationMs: 12, + }) + const full = [...session.events].find(e => e.type === 'hook/result') + if (full?.type === 'hook/result') { + expect(full.data).toMatchObject({ turn: 1, point: 'PreToolUse', handlerId: 'h1', decision: 'deny', exitCode: 2, stderrSummary: 'blocked', durationMs: 12 }) + } + + // A result with no exit code / no stderr (e.g. a hook that could not run) omits both keys. + const session2 = new Session(SessionId('s2')) + appendHookResult(session2, { turn: 1, point: 'Stop', handlerId: 'h3', decision: 'allow', durationMs: 3 }) + const sparse = [...session2.events].find(e => e.type === 'hook/result') + if (sparse?.type === 'hook/result') { + expect('exitCode' in sparse.data).toBe(false) + expect('stderrSummary' in sparse.data).toBe(false) + expect(sparse.data.durationMs).toBe(3) + } + }) + + it('an invoked/result pair correlates by handlerId', () => { + const session = new Session(SessionId('s')) + appendHookInvoked(session, { turn: 1, point: 'PreToolUse', dialect: 'claude', handlerId: 'pair-1' }) + appendHookResult(session, { turn: 1, point: 'PreToolUse', handlerId: 'pair-1', decision: 'allow', exitCode: 0, durationMs: 7 }) + + const invoked = [...session.events].find(e => e.type === 'hook/invoked') + const result = [...session.events].find(e => e.type === 'hook/result') + expect(invoked?.type === 'hook/invoked' && invoked.data.handlerId).toBe('pair-1') + expect(result?.type === 'hook/result' && result.data.handlerId).toBe('pair-1') + }) +}) diff --git a/packages/hooks/hook-protocol/tests/matcher.spec.ts b/packages/hooks/hook-protocol/tests/matcher.spec.ts new file mode 100644 index 0000000000..37e2acb137 --- /dev/null +++ b/packages/hooks/hook-protocol/tests/matcher.spec.ts @@ -0,0 +1,58 @@ +import { describe, expect, it } from 'vitest' +import { matchesMatcher } from '@deepseek-ai/dsh-hook-protocol' + +describe('matchesMatcher — match-all sentinels (both dialects)', () => { + for (const mode of ['claude', 'codex'] as const) { + it(`${mode}: absent / empty / '*' match everything`, () => { + expect(matchesMatcher(undefined, 'Bash', mode)).toBe(true) + expect(matchesMatcher('', 'anything', mode)).toBe(true) + expect(matchesMatcher('*', 'whatever', mode)).toBe(true) + }) + } +}) + +describe('matchesMatcher — claude dialect (literal-or-regex)', () => { + it('a pure word-char pattern is a LITERAL exact match (not substring)', () => { + expect(matchesMatcher('Bash', 'Bash', 'claude')).toBe(true) + // literal exact: "Bash" must NOT match "BashOutput" (a regex would, substring) + expect(matchesMatcher('Bash', 'BashOutput', 'claude')).toBe(false) + }) + + it('a pipe pattern is literal ALTERNATION (exact match any alternative)', () => { + expect(matchesMatcher('Edit|Write', 'Edit', 'claude')).toBe(true) + expect(matchesMatcher('Edit|Write', 'Write', 'claude')).toBe(true) + expect(matchesMatcher('Edit|Write', 'Read', 'claude')).toBe(false) + // still exact per-alternative, not substring + expect(matchesMatcher('Edit|Write', 'EditFile', 'claude')).toBe(false) + }) + + it('a non-word pattern falls through to regex (unanchored)', () => { + expect(matchesMatcher('^Bash$', 'Bash', 'claude')).toBe(true) + expect(matchesMatcher('Bash.*', 'BashOutput', 'claude')).toBe(true) + expect(matchesMatcher('.*\\.ts$', 'foo.ts', 'claude')).toBe(true) + expect(matchesMatcher('.*\\.ts$', 'foo.js', 'claude')).toBe(false) + }) +}) + +describe('matchesMatcher — codex dialect (always regex)', () => { + it('a word pattern is an unanchored regex (substring matches, unlike claude literal)', () => { + expect(matchesMatcher('Bash', 'Bash', 'codex')).toBe(true) + // codex has NO literal fast path: "Bash" is /Bash/, so it DOES match a substring + expect(matchesMatcher('Bash', 'BashOutput', 'codex')).toBe(true) + }) + + it('regex alternation and anchors work', () => { + expect(matchesMatcher('Edit|Write', 'Edit', 'codex')).toBe(true) + expect(matchesMatcher('^Bash$', 'Bash', 'codex')).toBe(true) + expect(matchesMatcher('^Bash$', 'BashOutput', 'codex')).toBe(false) + }) +}) + +describe('matchesMatcher — invalid regex is a non-match (never throws)', () => { + it('an unbalanced pattern matches nothing rather than throwing', () => { + // '(' is not the claude-literal charset, so it goes to the regex path and is invalid. + expect(() => matchesMatcher('(', 'x', 'claude')).not.toThrow() + expect(matchesMatcher('(', 'x', 'claude')).toBe(false) + expect(matchesMatcher('[', 'x', 'codex')).toBe(false) + }) +}) diff --git a/packages/hooks/hook-protocol/tests/merge.spec.ts b/packages/hooks/hook-protocol/tests/merge.spec.ts new file mode 100644 index 0000000000..6e52d15020 --- /dev/null +++ b/packages/hooks/hook-protocol/tests/merge.spec.ts @@ -0,0 +1,82 @@ +import { describe, expect, it } from 'vitest' +import { mergeHookOutputs } from '@deepseek-ai/dsh-hook-protocol' +import type { HookOutput } from '@deepseek-ai/dsh-hook-protocol' + +function out(over: Partial = {}): HookOutput { + return { exitCode: 0, stderr: '', ...over } +} + +describe('mergeHookOutputs — permission precedence deny > ask > allow', () => { + it('empty list yields a neutral outcome', () => { + const m = mergeHookOutputs([]) + expect(m.decision).toBe('none') + expect(m.stop).toBe(false) + expect(m.additionalContext).toEqual([]) + expect(m.systemMessages).toEqual([]) + }) + + it('a single allow yields allow', () => { + expect(mergeHookOutputs([out({ decision: 'allow' })]).decision).toBe('allow') + expect(mergeHookOutputs([out({ decision: 'approve' })]).decision).toBe('allow') + }) + + it('deny beats ask beats allow regardless of order', () => { + expect(mergeHookOutputs([out({ decision: 'allow' }), out({ decision: 'ask' })]).decision).toBe('ask') + expect(mergeHookOutputs([out({ decision: 'ask' }), out({ decision: 'deny' })]).decision).toBe('deny') + expect(mergeHookOutputs([out({ decision: 'deny' }), out({ decision: 'allow' })]).decision).toBe('deny') + // block folds to deny + expect(mergeHookOutputs([out({ decision: 'allow' }), out({ decision: 'block' })]).decision).toBe('deny') + }) + + it('no decision anywhere yields none', () => { + expect(mergeHookOutputs([out(), out()]).decision).toBe('none') + }) +}) + +describe('mergeHookOutputs — reasons, stop, context, systemMessages accumulate', () => { + it('joins block/deny reasons with a blank line (only from blocking hooks)', () => { + const m = mergeHookOutputs([ + out({ decision: 'deny', reason: 'first objection' }), + out({ decision: 'allow', reason: 'this allow reason is NOT collected' }), + out({ decision: 'block', reason: 'second objection' }), + ]) + expect(m.reason).toBe('first objection\n\nsecond objection') + }) + + it('no reason when nothing blocked', () => { + expect(mergeHookOutputs([out({ decision: 'allow' })]).reason).toBeUndefined() + }) + + it('stop is sticky on the first continue:false, capturing its stopReason', () => { + const m = mergeHookOutputs([ + out({ continue: true }), + out({ continue: false, stopReason: 'halt now' }), + out({ continue: false, stopReason: 'second halt — ignored' }), + ]) + expect(m.stop).toBe(true) + expect(m.stopReason).toBe('halt now') + }) + + it('no stop when every hook continues', () => { + const m = mergeHookOutputs([out({ continue: true }), out()]) + expect(m.stop).toBe(false) + expect(m.stopReason).toBeUndefined() + }) + + it('a continue:false with no stopReason stops with an undefined reason', () => { + const m = mergeHookOutputs([out({ continue: false })]) + expect(m.stop).toBe(true) + expect(m.stopReason).toBeUndefined() + }) + + it('collects additionalContext and systemMessages in hook order, skipping empties', () => { + const m = mergeHookOutputs([ + out({ additionalContext: 'ctx-A', systemMessage: 'warn-A' }), + out({ additionalContext: '', systemMessage: '' }), // empties skipped + out({ additionalContext: 'ctx-B' }), + out({ systemMessage: 'warn-B' }), + ]) + expect(m.additionalContext).toEqual(['ctx-A', 'ctx-B']) + expect(m.systemMessages).toEqual(['warn-A', 'warn-B']) + }) +}) diff --git a/packages/hooks/hook-protocol/tests/runner.spec.ts b/packages/hooks/hook-protocol/tests/runner.spec.ts new file mode 100644 index 0000000000..cfa7a76ab0 --- /dev/null +++ b/packages/hooks/hook-protocol/tests/runner.spec.ts @@ -0,0 +1,134 @@ +import { describe, expect, it } from 'vitest' +import type { BashExecRequest, BashExecSpec, BashExecutor, BashRunResult } from '@deepseek-ai/dsh-bash' +import { runHook } from '@deepseek-ai/dsh-hook-protocol' + +/** + * A minimal stand-in for the bits of {@link BashExecutor} that {@link runHook} + * actually calls (`resolve` then `run`). `runHook` is pure plumbing over those + * two methods, so a duck-typed recorder is the right test seam — the REAL + * executor (dsh-bash-local) is exercised end-to-end by the bridge e2e tests in + * PR-F, not here. + */ +function recordingBash(run: (spec: BashExecSpec) => Promise): { + bash: BashExecutor + specs: BashExecSpec[] +} { + const specs: BashExecSpec[] = [] + const bash = { + resolve(request: BashExecRequest): BashExecSpec { + // Carry the request through verbatim, defaulting the required spec fields — + // exactly what dsh-bash-local's resolve does for the fields runHook sets. + return { + command: request.command, + workdir: request.workdir ?? '/stub', + timeoutMs: request.timeoutMs ?? 0, + ...request.signal ? { signal: request.signal } : {}, + ...request.stdin !== undefined ? { stdin: request.stdin } : {}, + ...request.env !== undefined ? { env: request.env } : {}, + owner: request.owner, + } + }, + async run(spec: BashExecSpec): Promise { + specs.push(spec) + return run(spec) + }, + } as unknown as BashExecutor + return { bash, specs } +} + +function result(over: Partial = {}): BashRunResult { + return { + exitCode: 0, + signal: null, + timedOut: false, + aborted: false, + timeoutMs: 1000, + stdout: { text: '', truncated: false }, + stderr: { text: '', truncated: false }, + ...over, + } +} + +const clock = () => { let t = 0; return () => (t += 5) } // +5ms per call → duration 5 + +describe('runHook — payload + env + stdin plumbing', () => { + it('serializes the payload to stdin (with trailing newline when requested)', async () => { + const { bash, specs } = recordingBash(async () => result({ stdout: { text: '', truncated: false } })) + await runHook(bash, { command: 'my-hook.sh' }, { + payload: { hook_event_name: 'PreToolUse', tool_name: 'Bash' }, + defaultTimeoutMs: 60000, + trailingNewline: true, + }, clock()) + expect(specs[0]!.stdin).toBe(JSON.stringify({ hook_event_name: 'PreToolUse', tool_name: 'Bash' }) + '\n') + expect(specs[0]!.command).toBe('my-hook.sh') + }) + + it('omits the trailing newline when trailingNewline is false (Codex)', async () => { + const { bash, specs } = recordingBash(async () => result()) + await runHook(bash, { command: 'h' }, { payload: { a: 1 }, defaultTimeoutMs: 1000, trailingNewline: false }, clock()) + expect(specs[0]!.stdin).toBe('{"a":1}') + }) + + it('threads env and cwd into the request', async () => { + const { bash, specs } = recordingBash(async () => result()) + await runHook(bash, { command: 'h' }, { + payload: {}, env: { CLAUDE_PROJECT_DIR: '/proj' }, cwd: '/work', + defaultTimeoutMs: 1000, trailingNewline: true, + }, clock()) + expect(specs[0]!.env).toEqual({ CLAUDE_PROJECT_DIR: '/proj' }) + expect(specs[0]!.workdir).toBe('/work') + }) + + it('a per-hook timeoutSec (seconds) overrides the default (ms)', async () => { + const { bash, specs } = recordingBash(async () => result()) + await runHook(bash, { command: 'h', timeoutSec: 3 }, { payload: {}, defaultTimeoutMs: 60000, trailingNewline: true }, clock()) + expect(specs[0]!.timeoutMs).toBe(3000) + }) + + it('falls back to the default timeout when the hook sets none', async () => { + const { bash, specs } = recordingBash(async () => result()) + await runHook(bash, { command: 'h' }, { payload: {}, defaultTimeoutMs: 60000, trailingNewline: true }, clock()) + expect(specs[0]!.timeoutMs).toBe(60000) + }) + + it('passes the abort signal through', async () => { + const controller = new AbortController() + const { bash, specs } = recordingBash(async () => result()) + await runHook(bash, { command: 'h' }, { payload: {}, signal: controller.signal, defaultTimeoutMs: 1000, trailingNewline: true }, clock()) + expect(specs[0]!.signal).toBe(controller.signal) + }) +}) + +describe('runHook — outcome decoding + duration', () => { + it('decodes a clean exit with structured stdout and reports a duration', async () => { + const { bash } = recordingBash(async () => result({ + exitCode: 0, stdout: { text: JSON.stringify({ decision: 'block', reason: 'no' }), truncated: false }, + })) + const { output, durationMs } = await runHook(bash, { command: 'h' }, { payload: {}, defaultTimeoutMs: 1000, trailingNewline: true }, clock()) + expect(output.decision).toBe('block') + expect(output.reason).toBe('no') + expect(durationMs).toBe(5) + }) + + it('a signal death (exitCode null) decodes as undefined exit (non-blocking error)', async () => { + const { bash } = recordingBash(async () => result({ exitCode: null, signal: 'SIGKILL', stderr: { text: 'killed', truncated: false } })) + const { output } = await runHook(bash, { command: 'h' }, { payload: {}, defaultTimeoutMs: 1000, trailingNewline: true }, clock()) + expect(output.exitCode).toBeUndefined() + expect(output.decision).toBeUndefined() + expect(output.stderr).toBe('killed') + }) + + it('an executor rejection (infra fault) becomes a non-blocking error, never throws', async () => { + const { bash } = recordingBash(async () => { throw new Error('bad workdir: ENOENT') }) + const { output } = await runHook(bash, { command: 'h' }, { payload: {}, defaultTimeoutMs: 1000, trailingNewline: true }, clock()) + expect(output.exitCode).toBeUndefined() + expect(output.stderr).toBe('bad workdir: ENOENT') + expect(output.decision).toBeUndefined() + }) + + it('a non-Error rejection is stringified onto stderr', async () => { + const { bash } = recordingBash(async () => { throw 'plain string fault' }) + const { output } = await runHook(bash, { command: 'h' }, { payload: {}, defaultTimeoutMs: 1000, trailingNewline: true }, clock()) + expect(output.stderr).toBe('plain string fault') + }) +}) diff --git a/packages/hooks/hook-protocol/tsconfig.json b/packages/hooks/hook-protocol/tsconfig.json new file mode 100644 index 0000000000..dc4f8d9e16 --- /dev/null +++ b/packages/hooks/hook-protocol/tsconfig.json @@ -0,0 +1,24 @@ +{ + "extends": "../../../tsconfig.base.json", + "compilerOptions": { + "rootDir": "src", + "outDir": "lib/types" + }, + "include": [ + "src" + ], + "references": [ + { + "path": "../../../vendor/cosmokit" + }, + { + "path": "../../../vendor/cordis" + }, + { + "path": "../../bash/bash" + }, + { + "path": "../../core/session" + } + ] +} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index fc57472bef..b5ca00356a 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -248,6 +248,18 @@ 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/hook-protocol: + devDependencies: + '@deepseek-ai/dsh-bash': + specifier: workspace:^ + version: link:../../bash/bash + '@deepseek-ai/dsh-session': + specifier: workspace:^ + version: link:../../core/session + 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.base.json b/tsconfig.base.json index 3f2d828cb4..98661684fb 100644 --- a/tsconfig.base.json +++ b/tsconfig.base.json @@ -46,6 +46,7 @@ "./packages/compact/*/src", "./packages/subagent/*/src", "./packages/todo/*/src", + "./packages/hooks/*/src", "./packages/session-persistence/*/src", "./packages/ui/*/src", "./packages/util/*/src", diff --git a/tsconfig.build.json b/tsconfig.build.json index 22ba7a0687..cf468b4842 100644 --- a/tsconfig.build.json +++ b/tsconfig.build.json @@ -40,6 +40,7 @@ { "path": "./packages/subagent/subagent-spawn" }, { "path": "./packages/subagent/subagent-fork" }, { "path": "./packages/subagent/subagent-acp" }, - { "path": "./packages/todo/tool-todo" } + { "path": "./packages/todo/tool-todo" }, + { "path": "./packages/hooks/hook-protocol" } ] } diff --git a/tsconfig.json b/tsconfig.json index f0a4389df5..8f2b2c8fb1 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -51,6 +51,7 @@ { "path": "./packages/subagent/subagent-spawn" }, { "path": "./packages/subagent/subagent-fork" }, { "path": "./packages/subagent/subagent-acp" }, - { "path": "./packages/todo/tool-todo" } + { "path": "./packages/todo/tool-todo" }, + { "path": "./packages/hooks/hook-protocol" } ] } From c658f4d1559f869b64c5f69b4a6f049d262240bc Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Wed, 1 Jul 2026 01:12:04 +0800 Subject: [PATCH 2/6] =?UTF-8?q?fix(hooks):=20address=20Codex=20review=20?= =?UTF-8?q?=E2=80=94=20tighten=20codec=20to=20the=20reference=20schemas,?= =?UTF-8?q?=20preserve=20stdout?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex's PR-E review found three protocol-fidelity blockers + two doc gaps, all verified against ~/repos/refs: - (A) Top-level `decision` accepted allow/deny/ask, but both reference schemas reserve those for hookSpecificOutput.permissionDecision — the legacy top-level decision is approve/block ONLY. Split topLevelDecisionOf (approve/block) from permissionDecisionOf (allow/deny/ask), so an out-of-band {"decision":"deny"} is now invalid and ignored instead of becoming a real blocking decision. - (A) hookSpecificOutput was parsed without its hookEventName discriminator. HookOutput now surfaces hookEventName so a bridge can discard a block whose claimed event doesn't match the firing one (the schemas key the block by event). - (A) runHook discarded raw stdout. HookOutput now carries `stdout` (trimmed, verbatim) so a bridge can reproduce CC's plain-stdout rendering / Codex's plain-stdout-as-additionalContext behavior. - (B) hook/* SessionEventMap variants were only named in prose; added a payload/role table to core-data-structures/session.md (a maintained catalog surface). - (B) Removed PR-stack-position references (PR-F / "future bridge packages") from a test comment and the RFC, per the current-state-wording rule. New codec tests: top-level allow/deny/ask invalid+ignored, hookEventName capture, raw stdout preserved on plain + JSON + empty stdout. 51 tests, per-file 100%. --- docs/core-data-structures/session.md | 11 +++++ .../feature/2026-06-30-hook-protocol-lib.md | 6 +-- packages/hooks/hook-protocol/src/codec.ts | 45 ++++++++++++------- packages/hooks/hook-protocol/src/types.ts | 28 ++++++++++-- .../hooks/hook-protocol/tests/codec.spec.ts | 30 ++++++++++++- .../hooks/hook-protocol/tests/merge.spec.ts | 2 +- .../hooks/hook-protocol/tests/runner.spec.ts | 4 +- 7 files changed, 99 insertions(+), 27 deletions(-) diff --git a/docs/core-data-structures/session.md b/docs/core-data-structures/session.md index eb323dcd3e..2d462f96d9 100644 --- a/docs/core-data-structures/session.md +++ b/docs/core-data-structures/session.md @@ -211,6 +211,17 @@ interface TurnEndReasonMap { Every session event lives **inside** a turn (between a `turn/start` and its `turn/end`). The loop appends queued `user/message` events *after* `turn/start`, and an idle `agent.inject()` wraps its `context/message` in a one-shot `injection` turn. This makes the turn the single durability/replay boundary: a backend can treat anything after the last `turn/end` as an interrupted-crash tail without risking the loss of legitimately-recorded between-turn context. The `dsh-invariants` plugin enforces it in dev (a message event outside an open turn throws). See [the turn-enclosure invariant RFC](../rfc/implemented/architecture/2026-06-15-turn-enclosure-invariant.md). +## Plugin-contributed log-only events + +A plugin may declaration-merge extra `SessionEventMap` types. These are **log-only**: NOT `SurfaceEventType`s (they carry no `surfaceOp` and contribute nothing to derived history), but, like every event, they must sit inside an open turn. The compaction seam's `compact/*` are documented on [compaction.md](compaction.md); the hook bridges' `hook/*` provenance (from `@deepseek-ai/dsh-hook-protocol`) are: + +| Event | Payload | Role | +|---|---|---| +| `hook/invoked` | `{ turn, point, dialect, matcher?, handlerId }` | A hook command was invoked at a hook `point` (`PreToolUse`, `Stop`, …). `dialect` is the bridge (`claude`/`codex`/`native`); `matcher` the matcher-group pattern that selected it (absent for match-all); `handlerId` correlates with the result. | +| `hook/result` | `{ turn, point, handlerId, decision, exitCode?, stderrSummary?, durationMs }` | The decided outcome, paired by `handlerId`. `decision` is the resolved neutral outcome (`deny`/`allow`/`block`/`stop`/`pass`/…); `exitCode` absent when the hook could not run; `stderrSummary` the truncated block-reason source. | + +The mid-turn hook points (`PreToolUse`/`PostToolUse`/`UserPromptSubmit`/`Stop`) fire inside the loop's open turn, so their `hook/*` records are turn-enclosed by construction. `SessionStart` gets no `hook/*` record — its injected `context/message` is the durable evidence — because it has no open turn to enclose one (see the hooks RFC). + ## Durability contract What a persistence backend relies on: the durable log persists every event verbatim, **including** `assistant/chunk` — `seq` must stay contiguous, so chunks cannot be filtered out of the canonical log. All `event.data` must be JSON-serializable; `Session.append` enforces this at the source (throwing on non-serializable data), so a bad event never enters the log and `session.events` always equals what a backend can persist. Adding an event type that carries non-serializable data, or that breaks the turn/step nesting the invariants plugin checks, is a breaking change to the on-disk format. diff --git a/docs/rfc/implemented/feature/2026-06-30-hook-protocol-lib.md b/docs/rfc/implemented/feature/2026-06-30-hook-protocol-lib.md index ecb45ea2c3..848eeec219 100644 --- a/docs/rfc/implemented/feature/2026-06-30-hook-protocol-lib.md +++ b/docs/rfc/implemented/feature/2026-06-30-hook-protocol-lib.md @@ -12,7 +12,7 @@ This RFC introduces `@deepseek-ai/dsh-hook-protocol`, a **library** (not a plugi ## Decision -A new `packages/hooks/` group with `hook-protocol` as a pure library. It owns four primitive families and the `hook/*` session events; each bridge (PR-F) owns what genuinely differs. +A new `packages/hooks/` group with `hook-protocol` as a pure library. It owns four primitive families and the `hook/*` session events; each bridge plugin (`dsh-hooks-claude`, `dsh-hooks-codex`) owns what genuinely differs. **Shared (here):** - **Matcher** — `matchesMatcher(pattern, query, mode)`. The ONE axis the dialects differ on is collapsed to the `mode` parameter: `claude` treats a pure `[A-Za-z0-9_|]+` pattern as a literal (pipe = exact-match alternation) and anything else as a regex; `codex` is always an unanchored regex. Match-all on absent/`''`/`'*'`; an invalid regex matches nothing (never throws into the loop). @@ -21,7 +21,7 @@ A new `packages/hooks/` group with `hook-protocol` as a pure library. It owns fo - **Merge** — `mergeHookOutputs(outputs)`, folding multiple matched hooks into one most-restrictive `MergedHookOutcome`: permission precedence **deny > ask > allow**, halt sticky on the first `continue:false`, block reasons joined `\n\n`, context/system-messages accumulated in order. - **`hook/*` session events** — `hook/invoked` / `hook/result`, declaration-merged into `SessionEventMap` (log-only, like `compact/*` — NOT `SurfaceEventType`s), with `appendHookInvoked`/`appendHookResult` helpers so the invoked/result pairing and turn-enclosure stay consistent across bridges. -**Per-dialect (the bridges, PR-F):** building each event's stdin payload (CC's base+per-event field sets vs Codex's snake_case with `turn_id`/`model` extras), the dialect's env + `${CLAUDE_PLUGIN_ROOT}` substitution (CC) vs none (Codex), and mapping the neutral `HookOutput`/`MergedHookOutcome` onto the harness's seam-specific typed Decisions (`PreToolDecision`, `PromptDecision`, `ContinuationDecision`, `PostToolDecision`). +**Per-dialect (the bridge plugins):** building each event's stdin payload (CC's base+per-event field sets vs Codex's snake_case with `turn_id`/`model` extras), the dialect's env + `${CLAUDE_PLUGIN_ROOT}` substitution (CC) vs none (Codex), and mapping the neutral `HookOutput`/`MergedHookOutcome` onto the harness's seam-specific typed Decisions (`PreToolDecision`, `PromptDecision`, `ContinuationDecision`, `PostToolDecision`). ### Why "shared core + per-dialect adapters", not "one parameterized engine" @@ -29,4 +29,4 @@ A single engine parameterized by a full `dialect` descriptor was considered and ## Consequences -The two bridges (PR-F) become thin: parse the config file, pick a matcher mode, build the per-event payload+env, call `runHook` + `mergeHookOutputs`, map the outcome to a Decision, and append `hook/*`. The protocol's correctness-critical halves (matcher semantics, exit-code contract, merge precedence) live in one tested place — `hook-protocol` ships with heavy unit tests (matcher per-mode, codec per exit-code/field, runner plumbing with a stub executor, merge precedence, the `hook/*` helpers) at per-file 100%. Input rewrite (`updatedInput`) is parsed but not honored (the deferred [pre-tool-input-rewrite RFC](../../proposed/feature/2026-06-30-pre-tool-input-rewrite.md)); a bridge logs+warns on it. The package is a library, so it has no `cordis.yml` load path of its own — its real-load-path coverage comes through the bridge plugins that consume it (PR-F). +The two bridge plugins become thin: parse the config file, pick a matcher mode, build the per-event payload+env, call `runHook` + `mergeHookOutputs`, map the outcome to a Decision, and append `hook/*`. The protocol's correctness-critical halves (matcher semantics, exit-code contract, merge precedence) live in one tested place — `hook-protocol` ships with heavy unit tests (matcher per-mode, codec per exit-code/field, runner plumbing with a stub executor, merge precedence, the `hook/*` helpers) at per-file 100%. Input rewrite (`updatedInput`) is parsed but not honored (the deferred [pre-tool-input-rewrite RFC](../../proposed/feature/2026-06-30-pre-tool-input-rewrite.md)); a bridge logs+warns on it. The package is a library, so it has no `cordis.yml` load path of its own — its real-load-path coverage comes through the bridge plugins that consume it. diff --git a/packages/hooks/hook-protocol/src/codec.ts b/packages/hooks/hook-protocol/src/codec.ts index f6065af90e..ac49526600 100644 --- a/packages/hooks/hook-protocol/src/codec.ts +++ b/packages/hooks/hook-protocol/src/codec.ts @@ -42,14 +42,19 @@ function obj(value: unknown): Record | undefined { : undefined } -/** Normalize a raw `decision`/`permissionDecision` string to the neutral enum. */ -function decisionOf(value: string | undefined): HookOutput['decision'] { - switch (value) { - case 'approve': case 'allow': case 'block': case 'deny': case 'ask': - return value - default: - return undefined - } +/** + * The legacy TOP-LEVEL `decision` is only `approve`/`block` in both reference + * schemas — `allow`/`deny`/`ask` are reserved for `hookSpecificOutput. + * permissionDecision`. So an out-of-band `{"decision":"deny"}` is invalid and + * ignored here (it must not become a real blocking decision). + */ +function topLevelDecisionOf(value: string | undefined): HookOutput['decision'] { + return value === 'approve' || value === 'block' ? value : undefined +} + +/** A `hookSpecificOutput.permissionDecision` is `allow`/`deny`/`ask` only. */ +function permissionDecisionOf(value: string | undefined): HookOutput['decision'] { + return value === 'allow' || value === 'deny' || value === 'ask' ? value : undefined } /** @@ -62,7 +67,11 @@ function decisionOf(value: string | undefined): HookOutput['decision'] { */ export function parseHookOutput(exitCode: number | undefined, stdout: string, stderr: string): HookOutput { const trimmedErr = stderr.trim() - const output: HookOutput = { exitCode, stderr: trimmedErr } + const trimmedOut = stdout.trim() + // Keep the raw stdout verbatim: a clean-exit hook may emit PLAIN text the + // protocol renders/uses (CC output; Codex SessionStart/UserPromptSubmit + // additionalContext), so the bridge needs it even when there's no JSON. + const output: HookOutput = { exitCode, stderr: trimmedErr, stdout: trimmedOut } // Exit 2 is a blocking error in both dialects: stderr is the reason. Surface // it as a `block` decision so the bridge maps it uniformly with a structured @@ -76,7 +85,6 @@ export function parseHookOutput(exitCode: number | undefined, stdout: string, st // the stderr channel is authoritative. A non-zero/undefined exit other than 2 // carries no decision (the bridge records it as a non-blocking error). if (exitCode === 0) { - const trimmedOut = stdout.trim() // Only attempt JSON when stdout looks like a JSON object — matches the // reference engines, which treat other stdout as plain text, not an error. if (trimmedOut.startsWith('{')) { @@ -106,18 +114,23 @@ function applyStructured(output: HookOutput, parsed: Record): v const sysMsg = str(parsed, 'systemMessage') if (sysMsg !== undefined) output.systemMessage = sysMsg - // Top-level legacy `decision` + `reason` (CC approve/block; Codex block). - const topDecision = decisionOf(str(parsed, 'decision')) + // Top-level legacy `decision` (approve/block ONLY — allow/deny/ask there are + // invalid per both schemas) + its `reason`. + const topDecision = topLevelDecisionOf(str(parsed, 'decision')) if (topDecision !== undefined) output.decision = topDecision const topReason = str(parsed, 'reason') if (topReason !== undefined) output.reason = topReason - // hookSpecificOutput: the per-event channel. permissionDecision (allow/deny/ - // ask) OVERRIDES the legacy top-level decision when present; additionalContext - // and updatedInput live here too. + // hookSpecificOutput: the per-event channel, keyed by `hookEventName`. We + // surface that discriminator so the bridge can DISCARD a block whose event + // doesn't match the firing one (the schemas make it the discriminator). The + // permissionDecision (allow/deny/ask) OVERRIDES the legacy top-level decision; + // additionalContext and updatedInput live here too. const hso = obj(parsed.hookSpecificOutput) if (hso) { - const permission = decisionOf(str(hso, 'permissionDecision')) + const eventName = str(hso, 'hookEventName') + if (eventName !== undefined) output.hookEventName = eventName + const permission = permissionDecisionOf(str(hso, 'permissionDecision')) if (permission !== undefined) output.decision = permission const permissionReason = str(hso, 'permissionDecisionReason') if (permissionReason !== undefined) output.reason = permissionReason diff --git a/packages/hooks/hook-protocol/src/types.ts b/packages/hooks/hook-protocol/src/types.ts index a06ae7f837..dbc4b57aab 100644 --- a/packages/hooks/hook-protocol/src/types.ts +++ b/packages/hooks/hook-protocol/src/types.ts @@ -100,6 +100,14 @@ export interface HookOutput { exitCode: number | undefined /** Trimmed stderr — the block-reason source on a blocking (exit 2) hook. */ stderr: string + /** + * Trimmed stdout, verbatim. On a clean exit a hook may emit PLAIN (non-JSON) + * stdout that the protocol renders as output (CC) or treats as + * `additionalContext` (Codex SessionStart/UserPromptSubmit) — so the bridge + * needs the raw text, not just the parsed structured fields. Empty string when + * the hook produced no stdout. + */ + stdout: string /** * `false` ⇒ the hook asked to halt (CC/Codex `continue:false`); pairs with * {@link stopReason}. `true`/absent ⇒ proceed. @@ -110,14 +118,26 @@ export interface HookOutput { /** Hide the hook's stdout from the transcript (CC `suppressOutput`). */ suppressOutput?: boolean /** - * The blocking decision a hook expressed via structured stdout (CC's - * `decision` / `hookSpecificOutput.permissionDecision`): `'block'`/`'deny'` - * forbid the action, `'approve'`/`'allow'` permit it, `'ask'` requests - * confirmation. Absent ⇒ no explicit decision (exit code governs). + * The neutral blocking decision a hook expressed, folded from the two channels + * the reference protocols keep DISTINCT: the legacy top-level `decision` + * (`approve`/`block` only) and `hookSpecificOutput.permissionDecision` + * (`allow`/`deny`/`ask`). We normalize them to one enum — `'block'`/`'deny'` + * forbid, `'approve'`/`'allow'` permit, `'ask'` requests confirmation — but + * `'allow'`/`'deny'`/`'ask'` arise ONLY from a `permissionDecision`, never from + * a top-level `decision` (an out-of-band `{"decision":"deny"}` is invalid and + * ignored, matching the schemas). Absent ⇒ no explicit decision (exit code governs). */ decision?: 'approve' | 'allow' | 'block' | 'deny' | 'ask' /** The reason/explanation accompanying {@link decision}. */ reason?: string + /** + * The `hookSpecificOutput.hookEventName` discriminator, when the hook emitted + * a `hookSpecificOutput` block. The reference schemas key that block by event; + * a bridge compares this to the firing event and DISCARDS a mismatched block + * (a hook claiming `PreToolUse` output on a `Stop` event is malformed). Absent + * when the hook emitted no `hookSpecificOutput`. + */ + hookEventName?: string /** Extra context to inject for the next model request (CC `additionalContext`). */ additionalContext?: string /** A warning surfaced to the user (CC `systemMessage`). */ diff --git a/packages/hooks/hook-protocol/tests/codec.spec.ts b/packages/hooks/hook-protocol/tests/codec.spec.ts index 41976aeab9..7964745056 100644 --- a/packages/hooks/hook-protocol/tests/codec.spec.ts +++ b/packages/hooks/hook-protocol/tests/codec.spec.ts @@ -48,11 +48,25 @@ describe('parseHookOutput — structured stdout (exit 0 only)', () => { expect(out.systemMessage).toBe('heads up') }) - it('parses legacy top-level decision + reason (approve/block)', () => { + it('parses legacy top-level decision + reason (approve/block ONLY)', () => { expect(parseHookOutput(0, JSON.stringify({ decision: 'block', reason: 'nope' }), '').decision).toBe('block') expect(parseHookOutput(0, JSON.stringify({ decision: 'approve' }), '').decision).toBe('approve') }) + it('a top-level decision of allow/deny/ask is INVALID and ignored (reserved for permissionDecision)', () => { + // Both reference schemas restrict the legacy top-level `decision` to + // approve/block; allow/deny/ask must come from hookSpecificOutput.permissionDecision. + expect(parseHookOutput(0, JSON.stringify({ decision: 'deny' }), '').decision).toBeUndefined() + expect(parseHookOutput(0, JSON.stringify({ decision: 'allow' }), '').decision).toBeUndefined() + expect(parseHookOutput(0, JSON.stringify({ decision: 'ask' }), '').decision).toBeUndefined() + }) + + it('captures hookEventName from hookSpecificOutput (the discriminator a bridge validates)', () => { + const out = parseHookOutput(0, JSON.stringify({ hookSpecificOutput: { hookEventName: 'PreToolUse', permissionDecision: 'deny' } }), '') + expect(out.hookEventName).toBe('PreToolUse') + expect(out.decision).toBe('deny') + }) + it('hookSpecificOutput.permissionDecision OVERRIDES the legacy top-level decision', () => { const out = parseHookOutput(0, JSON.stringify({ decision: 'approve', @@ -89,6 +103,20 @@ describe('parseHookOutput — structured stdout (exit 0 only)', () => { const out = parseHookOutput(0, 'just some text output', '') expect(out.decision).toBeUndefined() expect(out.continue).toBeUndefined() + // The raw stdout is preserved verbatim so the bridge can render/use it + // (CC output; Codex additionalContext) — trimmed. + expect(out.stdout).toBe('just some text output') + }) + + it('preserves raw stdout (trimmed) alongside parsed structured fields', () => { + const json = JSON.stringify({ decision: 'block' }) + const out = parseHookOutput(0, ` ${json} \n`, '') + expect(out.stdout).toBe(json) + expect(out.decision).toBe('block') + }) + + it('stdout is empty string when the hook emits none', () => { + expect(parseHookOutput(0, '', '').stdout).toBe('') }) it('a JSON array stdout parses but yields no fields (not an object)', () => { diff --git a/packages/hooks/hook-protocol/tests/merge.spec.ts b/packages/hooks/hook-protocol/tests/merge.spec.ts index 6e52d15020..9709060d79 100644 --- a/packages/hooks/hook-protocol/tests/merge.spec.ts +++ b/packages/hooks/hook-protocol/tests/merge.spec.ts @@ -3,7 +3,7 @@ import { mergeHookOutputs } from '@deepseek-ai/dsh-hook-protocol' import type { HookOutput } from '@deepseek-ai/dsh-hook-protocol' function out(over: Partial = {}): HookOutput { - return { exitCode: 0, stderr: '', ...over } + return { exitCode: 0, stderr: '', stdout: '', ...over } } describe('mergeHookOutputs — permission precedence deny > ask > allow', () => { diff --git a/packages/hooks/hook-protocol/tests/runner.spec.ts b/packages/hooks/hook-protocol/tests/runner.spec.ts index cfa7a76ab0..698d6e0fa3 100644 --- a/packages/hooks/hook-protocol/tests/runner.spec.ts +++ b/packages/hooks/hook-protocol/tests/runner.spec.ts @@ -6,8 +6,8 @@ import { runHook } from '@deepseek-ai/dsh-hook-protocol' * A minimal stand-in for the bits of {@link BashExecutor} that {@link runHook} * actually calls (`resolve` then `run`). `runHook` is pure plumbing over those * two methods, so a duck-typed recorder is the right test seam — the REAL - * executor (dsh-bash-local) is exercised end-to-end by the bridge e2e tests in - * PR-F, not here. + * executor (dsh-bash-local) is exercised end-to-end by the hook-bridge plugins + * that consume this library, not here. */ function recordingBash(run: (spec: BashExecSpec) => Promise): { bash: BashExecutor From c28d6b837b53f77f09426c80688c511bd94cd478 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Wed, 1 Jul 2026 02:20:12 +0800 Subject: [PATCH 3/6] fix(hooks): merge surfaces the WINNING decision's reason, not only deny's MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mergeHookOutputs collected reasons only from rank-3 (deny/block) hooks, so an ask-winning outcome lost its reason — a bridge mapping an `ask` decision to a PreToolDecision had no reason to attach. Collect reasons per rank and emit the ones explaining the winning decision: a deny-winning fold shows deny reasons, an ask-winning fold shows ask reasons, allow contributes none. Found while building the hooks-claude bridge's PreToolUse `ask` path. --- packages/hooks/hook-protocol/src/merge.ts | 15 +++++++++++---- .../hooks/hook-protocol/tests/merge.spec.ts | 18 ++++++++++++++++++ 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/packages/hooks/hook-protocol/src/merge.ts b/packages/hooks/hook-protocol/src/merge.ts index d8fb158eb1..1e53dbaaea 100644 --- a/packages/hooks/hook-protocol/src/merge.ts +++ b/packages/hooks/hook-protocol/src/merge.ts @@ -74,7 +74,11 @@ function decisionForRank(maxRank: number): MergedDecision { */ export function mergeHookOutputs(outputs: HookOutput[]): MergedHookOutcome { let maxRank = 0 - const reasons: string[] = [] + // Reasons collected PER RANK, so the merged reason can be the one explaining + // the WINNING decision (a deny-winning outcome surfaces deny reasons; an + // ask-winning outcome surfaces ask reasons). An `allow`'s reason is never an + // objection the model needs, so rank 1 collects none. + const reasonsByRank = new Map() let stop = false let stopReason: string | undefined const additionalContext: string[] = [] @@ -83,9 +87,11 @@ export function mergeHookOutputs(outputs: HookOutput[]): MergedHookOutcome { for (const out of outputs) { const r = rank(out.decision) if (r > maxRank) maxRank = r - // Collect a reason only from a blocking/denying hook (rank 3) — an allow's - // "reason" is not an objection the model needs to see. - if (r === 3 && out.reason !== undefined && out.reason.length > 0) reasons.push(out.reason) + if ((r === 3 || r === 2) && out.reason !== undefined && out.reason.length > 0) { + const list = reasonsByRank.get(r) ?? [] + list.push(out.reason) + reasonsByRank.set(r, list) + } if (out.continue === false && !stop) { stop = true if (out.stopReason !== undefined) stopReason = out.stopReason @@ -98,6 +104,7 @@ export function mergeHookOutputs(outputs: HookOutput[]): MergedHookOutcome { } } + const reasons = reasonsByRank.get(maxRank) ?? [] return { decision: decisionForRank(maxRank), ...reasons.length > 0 ? { reason: reasons.join('\n\n') } : {}, diff --git a/packages/hooks/hook-protocol/tests/merge.spec.ts b/packages/hooks/hook-protocol/tests/merge.spec.ts index 9709060d79..def2474927 100644 --- a/packages/hooks/hook-protocol/tests/merge.spec.ts +++ b/packages/hooks/hook-protocol/tests/merge.spec.ts @@ -47,6 +47,24 @@ describe('mergeHookOutputs — reasons, stop, context, systemMessages accumulate expect(mergeHookOutputs([out({ decision: 'allow' })]).reason).toBeUndefined() }) + it('surfaces the reason of the WINNING decision: an ask-winning outcome shows the ask reason', () => { + const m = mergeHookOutputs([ + out({ decision: 'allow', reason: 'allow reason — not surfaced' }), + out({ decision: 'ask', reason: 'needs approval' }), + ]) + expect(m.decision).toBe('ask') + expect(m.reason).toBe('needs approval') + }) + + it('when deny wins over ask, the ask reasons are dropped (only the winning rank\'s reasons)', () => { + const m = mergeHookOutputs([ + out({ decision: 'ask', reason: 'ask reason — not surfaced once deny wins' }), + out({ decision: 'deny', reason: 'the real objection' }), + ]) + expect(m.decision).toBe('deny') + expect(m.reason).toBe('the real objection') + }) + it('stop is sticky on the first continue:false, capturing its stopReason', () => { const m = mergeHookOutputs([ out({ continue: true }), From 24e9c0fa70692b45a4ff27bb045ea4a3dbf9310e Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Wed, 1 Jul 2026 10:45:58 +0800 Subject: [PATCH 4/6] fix(hook-protocol): discard a hookSpecificOutput block whose hookEventName mismatches the firing event MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reference schemas key the `hookSpecificOutput` block by `hookEventName`, so a block naming a DIFFERENT event than the one firing is malformed — a hook emitting `hookSpecificOutput.hookEventName: "PreToolUse"` on a `Stop` event must not deny the Stop. The codec surfaced `hookEventName` for a bridge to compare but never enforced the discard, so both bridges pushed every parsed output into the merge unconditionally. parseHookOutput now takes an optional `expectedEventName`; when the block's `hookEventName` names a different event, its event-scoped fields (permissionDecision/permissionDecisionReason/additionalContext/updatedInput) are discarded (the discriminator is still surfaced for the log, and the event-agnostic top-level decision/continue/etc. are unaffected). runHook threads it via RunHookOptions.expectedEventName; a caller that omits it opts out. Codex review finding on the bridges PR (PR-F); fixed here on the codec that owns the fold and knows field provenance, then flows down to both bridges. --- packages/hooks/hook-protocol/README.md | 4 +- packages/hooks/hook-protocol/src/codec.ts | 37 +++++++++++--- packages/hooks/hook-protocol/src/runner.ts | 9 +++- packages/hooks/hook-protocol/src/types.ts | 9 ++-- .../hooks/hook-protocol/tests/codec.spec.ts | 48 +++++++++++++++++++ .../hooks/hook-protocol/tests/runner.spec.ts | 13 +++++ 6 files changed, 107 insertions(+), 13 deletions(-) diff --git a/packages/hooks/hook-protocol/README.md b/packages/hooks/hook-protocol/README.md index 1983cd0411..abf822ea29 100644 --- a/packages/hooks/hook-protocol/README.md +++ b/packages/hooks/hook-protocol/README.md @@ -17,8 +17,8 @@ Why a shared lib at all: Codex deliberately reimplements a *subset* of the Claud ## Primitives - **`matchesMatcher(matcher, query, mode)`** — match-all on absent/`''`/`'*'`; `claude` mode treats a pure `[A-Za-z0-9_|]+` pattern as a literal (pipe = exact-match alternation) and anything else as a regex; `codex` mode is always an unanchored regex. An invalid regex matches nothing (never throws). -- **`runHook(bash, hook, options, now)`** — serialize `options.payload` to the hook's stdin (with a trailing newline iff `options.trailingNewline`), merge `options.env` after the executor's credential scrub (the `dsh-bash` trusted-plugin surface), honor the hook's `timeoutSec` (else `defaultTimeoutMs`), and decode the result. Never throws: an executor rejection (infra fault) becomes a `HookOutput` with `exitCode: undefined` (a non-blocking error). `now` is injected for testable durations. -- **`parseHookOutput(exitCode, stdout, stderr)`** — the exit-code + structured-stdout codec. Exit `0` → parse JSON stdout (lenient: non-JSON is left for the bridge); exit `2` → blocking error, `stderr` is the block reason (surfaced as `decision: 'block'`); other → non-blocking error. `hookSpecificOutput.permissionDecision` (allow/deny/ask) overrides a legacy top-level `decision`; `additionalContext`/`updatedInput`/`systemMessage`/`continue`/`stopReason`/`suppressOutput` are parsed too. Pure and total. +- **`runHook(bash, hook, options, now)`** — serialize `options.payload` to the hook's stdin (with a trailing newline iff `options.trailingNewline`), merge `options.env` after the executor's credential scrub (the `dsh-bash` trusted-plugin surface), honor the hook's `timeoutSec` (else `defaultTimeoutMs`), and decode the result (threading `options.expectedEventName` to the codec). Never throws: an executor rejection (infra fault) becomes a `HookOutput` with `exitCode: undefined` (a non-blocking error). `now` is injected for testable durations. +- **`parseHookOutput(exitCode, stdout, stderr, expectedEventName?)`** — the exit-code + structured-stdout codec. Exit `0` → parse JSON stdout (lenient: non-JSON is left for the bridge); exit `2` → blocking error, `stderr` is the block reason (surfaced as `decision: 'block'`); other → non-blocking error. `hookSpecificOutput.permissionDecision` (allow/deny/ask) overrides a legacy top-level `decision`; `additionalContext`/`updatedInput`/`systemMessage`/`continue`/`stopReason`/`suppressOutput` are parsed too. The schemas key the `hookSpecificOutput` block by `hookEventName`, so passing `expectedEventName` (the firing event) DISCARDS a block whose `hookEventName` names a different event — its event-scoped fields don't take effect (a `PreToolUse` block on a `Stop` hook is malformed), while the event-agnostic top-level fields still apply. Pure and total. - **`mergeHookOutputs(outputs)`** — fold the results of every hook that matched one point: permission precedence **deny > ask > allow**, halt sticky on the first `continue:false`, block reasons joined with `\n\n`, `additionalContext`/`systemMessages` accumulated in order. ## `hook/*` session events diff --git a/packages/hooks/hook-protocol/src/codec.ts b/packages/hooks/hook-protocol/src/codec.ts index ac49526600..1b246a3a14 100644 --- a/packages/hooks/hook-protocol/src/codec.ts +++ b/packages/hooks/hook-protocol/src/codec.ts @@ -64,8 +64,19 @@ function permissionDecisionOf(value: string | undefined): HookOutput['decision'] * JSON on a 0 exit is treated as "no structured output" (the plain stdout is * still on the bridge to use), matching both reference engines' lenient parse of * non-JSON stdout. + * + * `expectedEventName` is the event the hook is FIRING for (e.g. `'PreToolUse'`). + * The reference schemas key the `hookSpecificOutput` block by `hookEventName`, + * so a block whose `hookEventName` names a DIFFERENT event is malformed and its + * event-scoped fields (`permissionDecision`/`permissionDecisionReason`/ + * `additionalContext`/`updatedInput`) are DISCARDED — a `PreToolUse` block on a + * `Stop` hook must not deny the `Stop`. The block's `hookEventName` is still + * surfaced (for the log/diagnostics), and the event-agnostic top-level fields + * (`decision`/`reason`/`continue`/`stopReason`/`suppressOutput`/`systemMessage`) + * are unaffected. Omit `expectedEventName` (or pass a matching one) to apply the + * block as-is — a caller that doesn't key by event opts out of the check. */ -export function parseHookOutput(exitCode: number | undefined, stdout: string, stderr: string): HookOutput { +export function parseHookOutput(exitCode: number | undefined, stdout: string, stderr: string, expectedEventName?: string): HookOutput { const trimmedErr = stderr.trim() const trimmedOut = stdout.trim() // Keep the raw stdout verbatim: a clean-exit hook may emit PLAIN text the @@ -96,15 +107,20 @@ export function parseHookOutput(exitCode: number | undefined, stdout: string, st // reference engines are). The plain stdout remains the bridge's to use. parsed = undefined } - if (parsed) applyStructured(output, parsed) + if (parsed) applyStructured(output, parsed, expectedEventName) } } return output } -/** Fold a parsed structured-stdout object into `output` (mutates in place). */ -function applyStructured(output: HookOutput, parsed: Record): void { +/** + * Fold a parsed structured-stdout object into `output` (mutates in place). + * `expectedEventName` (the firing event) gates the per-event `hookSpecificOutput` + * block: a block whose `hookEventName` names a different event has its + * event-scoped fields discarded (only its `hookEventName` is recorded). + */ +function applyStructured(output: HookOutput, parsed: Record, expectedEventName?: string): void { const cont = bool(parsed, 'continue') if (cont !== undefined) output.continue = cont const stopReason = str(parsed, 'stopReason') @@ -121,15 +137,22 @@ function applyStructured(output: HookOutput, parsed: Record): v const topReason = str(parsed, 'reason') if (topReason !== undefined) output.reason = topReason - // hookSpecificOutput: the per-event channel, keyed by `hookEventName`. We - // surface that discriminator so the bridge can DISCARD a block whose event - // doesn't match the firing one (the schemas make it the discriminator). The + // hookSpecificOutput: the per-event channel, keyed by `hookEventName`. The // permissionDecision (allow/deny/ask) OVERRIDES the legacy top-level decision; // additionalContext and updatedInput live here too. const hso = obj(parsed.hookSpecificOutput) if (hso) { const eventName = str(hso, 'hookEventName') + // Always surface the discriminator (for the log/diagnostics), even on a + // mismatch — the record should show what the malformed block claimed. if (eventName !== undefined) output.hookEventName = eventName + // The schemas key this block by event: if it names a DIFFERENT event than the + // one firing, it is malformed — discard its event-scoped fields (a PreToolUse + // block must not deny a Stop hook). A caller that passes no expectedEventName + // opts out of the check (applies the block as-is). + if (expectedEventName !== undefined && eventName !== undefined && eventName !== expectedEventName) { + return + } const permission = permissionDecisionOf(str(hso, 'permissionDecision')) if (permission !== undefined) output.decision = permission const permissionReason = str(hso, 'permissionDecisionReason') diff --git a/packages/hooks/hook-protocol/src/runner.ts b/packages/hooks/hook-protocol/src/runner.ts index 9e26607e3d..cea09c1fe7 100644 --- a/packages/hooks/hook-protocol/src/runner.ts +++ b/packages/hooks/hook-protocol/src/runner.ts @@ -31,6 +31,13 @@ export interface RunHookOptions { defaultTimeoutMs: number /** Whether to append a trailing newline to the stdin payload (CC yes, Codex no). */ trailingNewline: boolean + /** + * The event this hook is firing for (e.g. `'PreToolUse'`). When set, a + * structured `hookSpecificOutput` block whose `hookEventName` names a DIFFERENT + * event is treated as malformed and its event-scoped fields are discarded (see + * {@link parseHookOutput}). Omit it to apply any block as-is. + */ + expectedEventName?: string } /** The {@link HookOutput} plus the wall-clock duration of the run (for `hook/result`). */ @@ -75,7 +82,7 @@ export async function runHook( // `undefined` (a non-blocking error — no clean exit code to act on). const exitCode = result.exitCode ?? undefined return { - output: parseHookOutput(exitCode, result.stdout.text, result.stderr.text), + output: parseHookOutput(exitCode, result.stdout.text, result.stderr.text, options.expectedEventName), durationMs: now() - started, } } catch (error: unknown) { diff --git a/packages/hooks/hook-protocol/src/types.ts b/packages/hooks/hook-protocol/src/types.ts index dbc4b57aab..c3b75e7c08 100644 --- a/packages/hooks/hook-protocol/src/types.ts +++ b/packages/hooks/hook-protocol/src/types.ts @@ -132,9 +132,12 @@ export interface HookOutput { reason?: string /** * The `hookSpecificOutput.hookEventName` discriminator, when the hook emitted - * a `hookSpecificOutput` block. The reference schemas key that block by event; - * a bridge compares this to the firing event and DISCARDS a mismatched block - * (a hook claiming `PreToolUse` output on a `Stop` event is malformed). Absent + * a `hookSpecificOutput` block. The reference schemas key that block by event, + * so a block whose `hookEventName` names a DIFFERENT event than the one firing + * is malformed: {@link parseHookOutput} DISCARDS its event-scoped fields when + * given the firing event's `expectedEventName` (a hook claiming `PreToolUse` + * output on a `Stop` event does not affect the `Stop`). This field is still + * surfaced even on a mismatch — the record shows what the block claimed. Absent * when the hook emitted no `hookSpecificOutput`. */ hookEventName?: string diff --git a/packages/hooks/hook-protocol/tests/codec.spec.ts b/packages/hooks/hook-protocol/tests/codec.spec.ts index 7964745056..4f37f804bb 100644 --- a/packages/hooks/hook-protocol/tests/codec.spec.ts +++ b/packages/hooks/hook-protocol/tests/codec.spec.ts @@ -93,6 +93,54 @@ describe('parseHookOutput — structured stdout (exit 0 only)', () => { expect(parseHookOutput(0, JSON.stringify({ decision: 'maybe' }), '').decision).toBeUndefined() }) + it('DISCARDS a hookSpecificOutput block whose hookEventName mismatches the firing event', () => { + // A PreToolUse block emitted on a Stop hook is malformed — its event-scoped + // fields must not take effect (a stray PreToolUse deny must not deny the Stop). + const out = parseHookOutput(0, JSON.stringify({ + hookSpecificOutput: { hookEventName: 'PreToolUse', permissionDecision: 'deny', permissionDecisionReason: 'no', additionalContext: 'x', updatedInput: { command: 'y' } }, + }), '', 'Stop') + expect(out.hookEventName).toBe('PreToolUse') // still recorded for the log + expect(out.decision).toBeUndefined() // event-scoped fields discarded + expect(out.reason).toBeUndefined() + expect(out.additionalContext).toBeUndefined() + expect(out.updatedInput).toBeUndefined() + }) + + it('APPLIES a hookSpecificOutput block whose hookEventName matches the firing event', () => { + const out = parseHookOutput(0, JSON.stringify({ + hookSpecificOutput: { hookEventName: 'PreToolUse', permissionDecision: 'deny', additionalContext: 'x' }, + }), '', 'PreToolUse') + expect(out.decision).toBe('deny') + expect(out.additionalContext).toBe('x') + }) + + it('applies the block when expectedEventName is omitted (opt-out) even if it names an event', () => { + const out = parseHookOutput(0, JSON.stringify({ + hookSpecificOutput: { hookEventName: 'PreToolUse', permissionDecision: 'deny' }, + }), '') + expect(out.decision).toBe('deny') + }) + + it('applies a block that has NO hookEventName regardless of expectedEventName', () => { + // No discriminator to mismatch — the block applies (a hook that omits the key). + const out = parseHookOutput(0, JSON.stringify({ + hookSpecificOutput: { permissionDecision: 'deny' }, + }), '', 'Stop') + expect(out.decision).toBe('deny') + }) + + it('a mismatched block does NOT discard the event-agnostic top-level decision/continue', () => { + // Only the per-event block is scoped; top-level fields are event-agnostic. + const out = parseHookOutput(0, JSON.stringify({ + decision: 'block', reason: 'top', continue: false, stopReason: 'halt', + hookSpecificOutput: { hookEventName: 'PreToolUse', permissionDecision: 'allow' }, + }), '', 'Stop') + expect(out.decision).toBe('block') // top-level survives; the allow block was discarded + expect(out.reason).toBe('top') + expect(out.continue).toBe(false) + expect(out.stopReason).toBe('halt') + }) + it('malformed JSON on a clean exit is lenient (no structured output, no throw)', () => { const out = parseHookOutput(0, '{ not valid json', '') expect(out.decision).toBeUndefined() diff --git a/packages/hooks/hook-protocol/tests/runner.spec.ts b/packages/hooks/hook-protocol/tests/runner.spec.ts index 698d6e0fa3..1cbe1b46de 100644 --- a/packages/hooks/hook-protocol/tests/runner.spec.ts +++ b/packages/hooks/hook-protocol/tests/runner.spec.ts @@ -131,4 +131,17 @@ describe('runHook — outcome decoding + duration', () => { const { output } = await runHook(bash, { command: 'h' }, { payload: {}, defaultTimeoutMs: 1000, trailingNewline: true }, clock()) expect(output.stderr).toBe('plain string fault') }) + + it('threads expectedEventName so a mismatched hookSpecificOutput block is discarded', async () => { + const { bash } = recordingBash(async () => result({ + exitCode: 0, + stdout: { text: JSON.stringify({ hookSpecificOutput: { hookEventName: 'PreToolUse', permissionDecision: 'deny' } }), truncated: false }, + })) + const { output } = await runHook(bash, { command: 'h' }, { + payload: {}, defaultTimeoutMs: 1000, trailingNewline: true, expectedEventName: 'Stop', + }, clock()) + // A PreToolUse block on a Stop hook is malformed → its decision is discarded. + expect(output.hookEventName).toBe('PreToolUse') + expect(output.decision).toBeUndefined() + }) }) From 8f2ef9dc9bb493bdd22fffeaaa39ea092b49f611 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Wed, 1 Jul 2026 15:58:56 +0800 Subject: [PATCH 5/6] docs(hook-protocol): matcher's invalid-regex handling is SILENT, not bridge-logged MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review noted the module docs promised an invalid regex is "logged by the bridge", but matchesMatcher only returns `false` — callers cannot distinguish a genuine non-match from a compile failure, so a typo'd pattern silently disables that matcher with no warning. Both bridges call matchesMatcher directly, so no log happens anywhere. Correct the docs to state the silence explicitly; surfacing bad config would need a diagnostic-returning variant or parse-time validation, marked TODO(matcher-diagnostics). No behavior change. --- packages/hooks/hook-protocol/src/matcher.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/hooks/hook-protocol/src/matcher.ts b/packages/hooks/hook-protocol/src/matcher.ts index 4ce3e4b62d..ee1dd324b3 100644 --- a/packages/hooks/hook-protocol/src/matcher.ts +++ b/packages/hooks/hook-protocol/src/matcher.ts @@ -9,8 +9,11 @@ * - `codex`: every pattern is an unanchored regex (no literal fast path). * * Both treat an absent / empty / `'*'` pattern as match-all, and both treat an - * invalid regex as a non-match (the bridge logs it; a broken matcher must not - * throw into the loop). + * invalid regex as a non-match: a broken matcher selects nothing rather than + * throwing into the loop. This is SILENT — the boolean return cannot distinguish + * "did not match" from "failed to compile", so a typo'd pattern (e.g. `[`) + * quietly disables that matcher with no warning. Surfacing bad config would need + * a diagnostic-returning variant or parse-time validation (`TODO(matcher-diagnostics)`). * * @module @deepseek-ai/dsh-hook-protocol/matcher */ @@ -43,7 +46,9 @@ export function matchesMatcher(matcher: string | undefined, query: string, mode: return new RegExp(pattern).test(query) } catch { // Invalid regex: a broken matcher selects nothing rather than throwing into - // the agent loop. The bridge is responsible for surfacing the bad config. + // the agent loop. This is silent — callers get `false`, indistinguishable + // from a genuine non-match, so a typo'd pattern quietly disables the matcher. + // Surfacing it needs a diagnostic-returning variant (TODO(matcher-diagnostics)). return false } } From 9428acdc9609ffca5a2f56760974cc0313cc22be Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Thu, 2 Jul 2026 17:13:42 +0800 Subject: [PATCH 6/6] fix(hook-protocol): discard a discriminator-less hookSpecificOutput block MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review on the hook-protocol PR: the event-scope guard only rejected a `hookSpecificOutput` block whose `hookEventName` NAMED a different event than the firing one. A block with NO `hookEventName` slipped through and applied its event-scoped permission fields to whatever event was firing. Under the keyed Claude Code schema (where `hookEventName` is part of the block) a missing discriminator is as malformed as a mismatched one — a Stop/UserPromptSubmit hook emitting a bare `{ permissionDecision: 'deny' }` could deny the current point. Drop the `eventName !== undefined` clause so the guard fires on both a mismatch and an omission when the caller passes `expectedEventName`; the opt-out (no expectedEventName) still applies a discriminator-less block as-is. Flipped the test that pinned the old behavior (it documented an artifact, not a contract) and proved the corrected one red on the old guard. --- packages/hooks/hook-protocol/README.md | 2 +- packages/hooks/hook-protocol/src/codec.ts | 17 ++++++++++------- .../hooks/hook-protocol/tests/codec.spec.ts | 18 +++++++++++++++--- 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/packages/hooks/hook-protocol/README.md b/packages/hooks/hook-protocol/README.md index abf822ea29..8478f8aa74 100644 --- a/packages/hooks/hook-protocol/README.md +++ b/packages/hooks/hook-protocol/README.md @@ -18,7 +18,7 @@ Why a shared lib at all: Codex deliberately reimplements a *subset* of the Claud - **`matchesMatcher(matcher, query, mode)`** — match-all on absent/`''`/`'*'`; `claude` mode treats a pure `[A-Za-z0-9_|]+` pattern as a literal (pipe = exact-match alternation) and anything else as a regex; `codex` mode is always an unanchored regex. An invalid regex matches nothing (never throws). - **`runHook(bash, hook, options, now)`** — serialize `options.payload` to the hook's stdin (with a trailing newline iff `options.trailingNewline`), merge `options.env` after the executor's credential scrub (the `dsh-bash` trusted-plugin surface), honor the hook's `timeoutSec` (else `defaultTimeoutMs`), and decode the result (threading `options.expectedEventName` to the codec). Never throws: an executor rejection (infra fault) becomes a `HookOutput` with `exitCode: undefined` (a non-blocking error). `now` is injected for testable durations. -- **`parseHookOutput(exitCode, stdout, stderr, expectedEventName?)`** — the exit-code + structured-stdout codec. Exit `0` → parse JSON stdout (lenient: non-JSON is left for the bridge); exit `2` → blocking error, `stderr` is the block reason (surfaced as `decision: 'block'`); other → non-blocking error. `hookSpecificOutput.permissionDecision` (allow/deny/ask) overrides a legacy top-level `decision`; `additionalContext`/`updatedInput`/`systemMessage`/`continue`/`stopReason`/`suppressOutput` are parsed too. The schemas key the `hookSpecificOutput` block by `hookEventName`, so passing `expectedEventName` (the firing event) DISCARDS a block whose `hookEventName` names a different event — its event-scoped fields don't take effect (a `PreToolUse` block on a `Stop` hook is malformed), while the event-agnostic top-level fields still apply. Pure and total. +- **`parseHookOutput(exitCode, stdout, stderr, expectedEventName?)`** — the exit-code + structured-stdout codec. Exit `0` → parse JSON stdout (lenient: non-JSON is left for the bridge); exit `2` → blocking error, `stderr` is the block reason (surfaced as `decision: 'block'`); other → non-blocking error. `hookSpecificOutput.permissionDecision` (allow/deny/ask) overrides a legacy top-level `decision`; `additionalContext`/`updatedInput`/`systemMessage`/`continue`/`stopReason`/`suppressOutput` are parsed too. The schemas key the `hookSpecificOutput` block by `hookEventName`, so passing `expectedEventName` (the firing event) DISCARDS a block whose `hookEventName` names a different event — or omits it entirely — its event-scoped fields don't take effect (a `PreToolUse` block on a `Stop` hook is malformed, and so is a discriminator-less block that would otherwise apply to any event), while the event-agnostic top-level fields still apply. Pure and total. - **`mergeHookOutputs(outputs)`** — fold the results of every hook that matched one point: permission precedence **deny > ask > allow**, halt sticky on the first `continue:false`, block reasons joined with `\n\n`, `additionalContext`/`systemMessages` accumulated in order. ## `hook/*` session events diff --git a/packages/hooks/hook-protocol/src/codec.ts b/packages/hooks/hook-protocol/src/codec.ts index 1b246a3a14..b5170028c2 100644 --- a/packages/hooks/hook-protocol/src/codec.ts +++ b/packages/hooks/hook-protocol/src/codec.ts @@ -117,8 +117,8 @@ export function parseHookOutput(exitCode: number | undefined, stdout: string, st /** * Fold a parsed structured-stdout object into `output` (mutates in place). * `expectedEventName` (the firing event) gates the per-event `hookSpecificOutput` - * block: a block whose `hookEventName` names a different event has its - * event-scoped fields discarded (only its `hookEventName` is recorded). + * block: a block whose `hookEventName` names a different event — OR omits it — has + * its event-scoped fields discarded (any present `hookEventName` is still recorded). */ function applyStructured(output: HookOutput, parsed: Record, expectedEventName?: string): void { const cont = bool(parsed, 'continue') @@ -146,11 +146,14 @@ function applyStructured(output: HookOutput, parsed: Record, ex // Always surface the discriminator (for the log/diagnostics), even on a // mismatch — the record should show what the malformed block claimed. if (eventName !== undefined) output.hookEventName = eventName - // The schemas key this block by event: if it names a DIFFERENT event than the - // one firing, it is malformed — discard its event-scoped fields (a PreToolUse - // block must not deny a Stop hook). A caller that passes no expectedEventName - // opts out of the check (applies the block as-is). - if (expectedEventName !== undefined && eventName !== undefined && eventName !== expectedEventName) { + // The schemas key this block by event: when a caller passes the firing event + // (`expectedEventName`), the block's `hookEventName` MUST name it. A different + // name — or a MISSING one — is malformed under the keyed schema, so discard the + // event-scoped fields (a PreToolUse block must not deny a Stop hook; nor may a + // discriminator-less block silently apply PreToolUse-scoped permission fields to + // whatever event is firing). A caller that passes no expectedEventName opts out + // of the check (applies the block as-is). + if (expectedEventName !== undefined && eventName !== expectedEventName) { return } const permission = permissionDecisionOf(str(hso, 'permissionDecision')) diff --git a/packages/hooks/hook-protocol/tests/codec.spec.ts b/packages/hooks/hook-protocol/tests/codec.spec.ts index 4f37f804bb..5f72753c57 100644 --- a/packages/hooks/hook-protocol/tests/codec.spec.ts +++ b/packages/hooks/hook-protocol/tests/codec.spec.ts @@ -121,11 +121,23 @@ describe('parseHookOutput — structured stdout (exit 0 only)', () => { expect(out.decision).toBe('deny') }) - it('applies a block that has NO hookEventName regardless of expectedEventName', () => { - // No discriminator to mismatch — the block applies (a hook that omits the key). + it('DISCARDS a block with NO hookEventName when a firing event is expected', () => { + // Under the keyed schema a missing discriminator is as malformed as a + // mismatched one: a discriminator-less block must not apply its event-scoped + // permission fields to whatever event happens to be firing. + const out = parseHookOutput(0, JSON.stringify({ + hookSpecificOutput: { permissionDecision: 'deny', additionalContext: 'x' }, + }), '', 'Stop') + expect(out.hookEventName).toBeUndefined() // none to record + expect(out.decision).toBeUndefined() // event-scoped fields discarded + expect(out.additionalContext).toBeUndefined() + }) + + it('applies a discriminator-less block when expectedEventName is omitted (opt-out)', () => { + // With no firing event to validate against, the block applies as-is. const out = parseHookOutput(0, JSON.stringify({ hookSpecificOutput: { permissionDecision: 'deny' }, - }), '', 'Stop') + }), '') expect(out.decision).toBe('deny') })