From dc95a7881d8c45084069d5c856c15aefe1a6d197 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Tue, 30 Jun 2026 17:11:18 +0800 Subject: [PATCH 1/6] =?UTF-8?q?feat(events):=20interception=20seams=20?= =?UTF-8?q?=E2=80=94=20the=20typed-Decision=20surface=20for=20hooks?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reshape the agent's interception surface so every seam returns a small, typed Decision union, and the set covers the hook points a CC/Codex bridge (and a native plugin) needs. "Native hooks" are not a package — a native hook is just a cordis plugin on these canonical events; the bridges (a later PR) only translate an external protocol onto the same surface. dsh-agent: - NEW agent/session-start(agent, source) emit (once before turn 1; SessionStartSource startup|resume|clear|compact) — a pure notification, seeds context via inject(). - NEW agent/prompt-submit waterfall → PromptDecision (allow, optionally rewriting the prompt or attaching additionalContext, or block). - RESHAPE agent/turn-continuation boolean → ContinuationDecision ({action:'stop'} | {action:'continue', reason?}; a continue reason is recorded as next-step steering). - New HookContext envelope (required source — inject() would mislabel a missing one). dsh-tools: split the single tools/execute waterfall into tools/pre-execute (PreToolDecision allow/deny/ask gate) and tools/post-execute (PostToolDecision accept/block, optionally replacing content or attaching additionalContext). Core dispatch sits between as plain code; the tool body keeps its inner try/catch so a thrown tool still reaches post-execute as an isError. ToolExecutionResult gains additionalContext (ferried to the loop's per-step buffer). Input rewrite is deliberately NOT offered (a proposed RFC designs it consistently). dsh-session: new `rejected` TurnEndReason — a turn whose whole prompt batch was blocked by prompt-submit. agent-loop firing points: session-start emitted at create (source threaded — startup for create/fork, resume for resume()); prompt-submit per drained message with the always-open-turn rule (a fully-blocked batch is a zero-step rejected turn); the continuation reshape; post-tool additionalContext buffered and appended after all tool/results (adjacency). ACP codec maps rejected→cancelled. A worked native-plugin example (interception.spec.ts) proves all four seams compose end-to-end through the real loop with NO hook/* events (those belong to the bridge lib). All existing tools/execute + turn-continuation tests migrated. The tool-subagent abort test now aborts after a microtask so it still exercises the live onAbort bridge (execute() awaits pre-execute before the body runs). RFCs: implemented/feature/2026-06-30-interception-seams.md (the reshape) + proposed/feature/2026-06-30-pre-tool-input-rewrite.md (the deferred rewrite design). --- docs/architecture.md | 35 +- docs/cookbook/extension-cookbook.md | 12 +- docs/cordis-catalog/events-and-services.md | 76 ++- docs/core-data-structures/core.md | 37 +- docs/core-data-structures/session.md | 12 +- docs/core-data-structures/tools.md | 30 +- docs/rfc/README.md | 2 + .../feature/2026-06-30-interception-seams.md | 45 ++ .../2026-06-30-pre-tool-input-rewrite.md | 39 ++ .../bash/tool-bash/tests/integration.spec.ts | 7 +- packages/core/agent-loop/README.md | 20 +- packages/core/agent-loop/src/index.ts | 35 +- packages/core/agent-loop/src/loop.ts | 84 +++- packages/core/agent-loop/tests/cancel.spec.ts | 2 +- .../agent-loop/tests/interception.spec.ts | 446 ++++++++++++++++++ packages/core/agent-loop/tests/loop.spec.ts | 6 +- packages/core/agent-loop/tests/resume.spec.ts | 30 ++ .../agent-loop/tests/review-fixes.spec.ts | 19 +- packages/core/agent/README.md | 8 +- packages/core/agent/src/types.ts | 97 +++- packages/core/session/src/types.ts | 10 + packages/core/tools/README.md | 13 +- packages/core/tools/src/index.ts | 175 ++++++- packages/core/tools/tests/tools.spec.ts | 140 ++++-- .../tool-subagent/tests/tool-subagent.spec.ts | 8 + packages/ui/acp/src/codec.ts | 6 + packages/ui/acp/tests/codec.spec.ts | 1 + scripts/type-equiv.manifest.json | 6 + 28 files changed, 1255 insertions(+), 146 deletions(-) create mode 100644 docs/rfc/implemented/feature/2026-06-30-interception-seams.md create mode 100644 docs/rfc/proposed/feature/2026-06-30-pre-tool-input-rewrite.md create mode 100644 packages/core/agent-loop/tests/interception.spec.ts diff --git a/docs/architecture.md b/docs/architecture.md index 67d6a06a20..c5d3a9123a 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -69,7 +69,7 @@ Swappable capabilities are split into **three packages** so each part evolves in The LLM seam has the same topology folded differently: `dsh-llm` carries the interface (`LlmAdapter`) AND the consumer surface (`ctx.llm.stream()`), with adapters as implementation packages — there the consumer is the loop itself, not a swappable schema surface. Use the full three-package split when the consumer is independently replaceable; keep interface + consumer together when they are one concern. Don't split preemptively: a capability with one conceivable implementation and one consumer stays one package until proven otherwise. -> **"Capability" — two unrelated meanings.** (1) The *seam pattern* above ("one plugin provides a capability, another needs it") is realized by plain Cordis **services + `inject`**: a provider registers a service (`ctx.bash`, declared in `interface Context`); a consumer declares `inject: ['bash']` and its fiber stays pending until the service exists, tearing down via HMR if it later vanishes. No extra library is needed. (2) `@cordisjs/plugin-capability` is a different axis entirely — a **permission/capability-security** service (named permissions with inheritance/dependency, tested against a session via `ctx.capability.test`). It is a candidate for the deferred permissions/sandbox work (the `tools/execute` veto seam), NOT a mechanism for swapping implementations. +> **"Capability" — two unrelated meanings.** (1) The *seam pattern* above ("one plugin provides a capability, another needs it") is realized by plain Cordis **services + `inject`**: a provider registers a service (`ctx.bash`, declared in `interface Context`); a consumer declares `inject: ['bash']` and its fiber stays pending until the service exists, tearing down via HMR if it later vanishes. No extra library is needed. (2) `@cordisjs/plugin-capability` is a different axis entirely — a **permission/capability-security** service (named permissions with inheritance/dependency, tested against a session via `ctx.capability.test`). It is a candidate for the deferred permissions/sandbox work (the `tools/pre-execute` deny/ask gate), NOT a mechanism for swapping implementations. ## The vocabulary (dsh-llm) @@ -102,7 +102,7 @@ Tool schemas are deliberately **part of the assembly**: "what the model is told `ToolRegistry.register()` takes schema + `execute()`. The registry feeds its schemas into the system-prompt assembly automatically. -`execute()` runs through the **`tools/execute` waterfall** — the single seam where sandbox, permission, hooks, and plan-mode plugins wrap or veto a call. This collapses Claude Code's validate → PreToolUse → permission → execute → PostToolUse pipeline into ordered waterfall listeners. +`execute()` runs through a **two-waterfall pipeline** — `tools/pre-execute` (the allow/deny/ask gate) → core dispatch → `tools/post-execute` (inspect/replace the result, attach context) — the seams where sandbox, permission, hooks, and plan-mode plugins gate or transform a call. This maps Claude Code's validate → PreToolUse → permission → execute → PostToolUse pipeline onto two ordered waterfalls: `pre-execute` returns a `PreToolDecision` (allow/deny/ask), `post-execute` a `PostToolDecision` (accept/block, optionally replacing content or attaching `additionalContext`). Core dispatch sits between them as plain code, inside `execute`'s outer try/catch, with the tool body's own try/catch preserved so a thrown tool still reaches `post-execute` as an `isError`. **TODO**: tool shapes get revisited now that real tools exist (the bash suite landed; the `TODO(review)` in dsh-tools is still open) — e.g. a concurrency-safety hint for parallel execution; phase 1 executes tool calls sequentially. @@ -126,11 +126,16 @@ Tool schemas are deliberately **part of the assembly**: "what the model is told - **Step**: one model request + its tool executions. ``` +create agent → emit agent/session-start(source) ⟵ once, before turn 1 (startup|resume) forever: wait for queued messages (idle) emit agent/status(running) TURN (error-contained — a throwing plugin ends the turn, never the loop): - drain queued → 'turn/start' → session('user/message'…) → emit agent/turn-start + 'turn/start' + each queued msg: waterfall agent/prompt-submit ⟵ allow (rewrite/+context) | block + allow → session('user/message'…); inject additionalContext + every prompt blocked → 'turn/end'(rejected), 0 steps ⟵ zero-step turn, model never called + emit agent/turn-start STEP loop: drain steering (late steering from previous step's listeners) session('step/start') ⟵ durable step boundary (no agent/* mirror) @@ -145,15 +150,19 @@ forever: msg = waterfall agent/step-result ⟵ runs BEFORE the log append, so the session('assistant/message' {content, usage?}) log records what tool dispatch uses each tool-call (sequential, abort-checked between calls): - session('tool/call'); ctx.tools.execute() ⟵ waterfall tools/execute + session('tool/call'); ctx.tools.execute() ⟵ waterfall tools/pre-execute (allow/ + deny/ask gate) → dispatch → tools/post-execute (accept/block, replace, +context) tool execution may append tool-owned session events, e.g. `todo/write` session('tool/result') + append buffered post-execute additionalContext → session('context/message')(s) + ⟵ after ALL tool/results (adjacency) drain steering → session('steering/message'); emit agent/steering session('step/end') ⟵ durable step boundary (no agent/* mirror) - cont = waterfall agent/turn-continuation(default = hadToolCalls || steered) - steering pending forces cont = true (from continuation listeners OR from - step/end session-event listeners — the /goal pattern; hasSteering override) - if !cont: break + cont = waterfall agent/turn-continuation(default = {action: hadToolCalls||steered + ? 'continue' : 'stop'}) → ContinuationDecision + a continue's reason is recorded as next-step steering (same turn); steering pending + also forces continue (continuation OR step/end listeners — the /goal pattern) + if action==stop: break session('turn/end'); emit agent/turn-end await ctx.parallel('session/flush', session) ⟵ durability checkpoint (failure reported via agent/error, not fatal) @@ -163,7 +172,7 @@ forever: Error containment: a throwing `agent/turn-continuation` listener or a broken step ends the **turn** with `turn/end { reason: { kind: 'error', step, message, code? } }` — the failure's step number rides on the durable turn reason (there is no separate session `error` event); live diagnostics fire via `agent/error`. Never the driver loop. An adapter that ends its stream with a `finish {kind:'error'}` or `{kind:'aborted'}` chunk (the in-band error path, for adapters that can't throw mid-stream) is likewise translated into a step error, so the turn ends `error`/`aborted` instead of logging a normal `completed` assistant message. A `cancel()` is honored mid-stream **and** between tool calls; disposal mid-turn ends the turn with reason `disposed` and emits `agent/status('disposed')`. -Turn-end reasons: a turn ends with one `TurnEndReason` — `completed`, `aborted`, `error`, `disposed`, or `max-tokens`. `max-tokens` mirrors the model-call `FinishReason` of the same name (DeepSeek's `length`): a step that hit the output-token ceiling makes the turn end `max-tokens` rather than `completed`, by the rule *any `max-tokens` step in the turn surfaces as `max-tokens`* (a continuation plugin may run further steps after one, but the cut-short fact wins; the `disposed`/`aborted`/`error` outcomes still take precedence). This lets a consumer distinguish a clean stop from a truncated one (the ACP bridge maps it to the `max_tokens` stop reason). `TurnEndReason` is merge-extensible; `refusal` and `max_turn_requests` are the next variants to add when an adapter/loop first emits them. +Turn-end reasons: a turn ends with one `TurnEndReason` — `completed`, `aborted`, `error`, `disposed`, `max-tokens`, `rejected`, or `interrupted`. `max-tokens` mirrors the model-call `FinishReason` of the same name (DeepSeek's `length`): a step that hit the output-token ceiling makes the turn end `max-tokens` rather than `completed`, by the rule *any `max-tokens` step in the turn surfaces as `max-tokens`* (a continuation plugin may run further steps after one, but the cut-short fact wins; the `disposed`/`aborted`/`error` outcomes still take precedence). `rejected` is a zero-step turn whose entire prompt batch was blocked by an `agent/prompt-submit` hook (the turn still opens and closes balanced; the ACP bridge maps it to `cancelled`). `interrupted` is synthesized by a persistence backend closing a crash-orphaned turn on reload. This lets a consumer distinguish a clean stop from a truncated/blocked one (the ACP bridge maps `max-tokens` to the `max_tokens` stop reason). `TurnEndReason` is merge-extensible; `refusal` and `max_turn_requests` are the next variants to add when an adapter/loop first emits them. A failure that happens once the turn is already closed has no in-turn position for a turn-end error reason (the turn already ended). So a rejecting `session/flush` (the post-`turn/end` durability checkpoint) and a throwing `agent/turn-end` listener are reported via `agent/error` + the logger only, NOT as a session event; the turn stays balanced and the persistence backend keeps its buffered events for the next flush. @@ -189,7 +198,7 @@ Every MVP feature (including the TODO-marked ones), with the mechanism that impl | MVP feature | Plugin mechanism | |---|---| -| Hook system (user + project level) | listeners on `agent/request`, `agent/step-result`, `tools/execute`, `agent/turn-continuation`; a hooks plugin bridges config files to shell commands | +| Hook system (user + project level) | listeners on `agent/session-start`, `agent/prompt-submit`, `agent/request`, `agent/step-result`, `tools/pre-execute`, `tools/post-execute`, `agent/turn-continuation` (each interception waterfall returns a typed Decision); a hooks bridge plugin maps config files / shell commands onto those seams, a native hook plugin uses them directly | | `/goal` | force-continue via `agent/turn-continuation` + `steer()` reminders | | `/loop` | on `agent/turn-end`, `send()` the next iteration; or force-continue | | Dynamic workflow | orchestrator plugin on `agent/turn-end` (or the `step/end` session event) driving `send`/`steer` (+ sub-agents later) | @@ -200,9 +209,9 @@ Every MVP feature (including the TODO-marked ones), with the mechanism that impl | AGENTS.md (subdir, on-touch) + file-change notices | `agent.inject()` from a watcher / tool-result listener | | Built-in tools (Read/Write/Edit/Bash/…) | `ctx.tools.register()`; schemas flow into the assembly automatically. **Bash: implemented** — `dsh-bash` (seam) + `dsh-bash-local` (subprocesses) + `dsh-tool-bash` (`bash`/`bash_output`/`bash_kill`, incl. background tasks). **`todo_write`: implemented** — `dsh-tool-todo` writes the whole task list to the session log (`todo/write`), rendered as a stdio checklist / ACP `plan` | | ToolSearch / progressive disclosure | wrap `agent/request`, filter `req.tools` | -| Tool sandbox (landlock / sandbox-exec) | wrap `tools/execute`, or implement a sandboxing `BashExecutor` (the dsh-bash seam) | -| Permission system / AskUserQuestion | wrap `tools/execute` (veto or ask); register an ask tool | -| Plan mode | wrap `tools/execute` (deny writes) + `agent/request` (inject mode prompt) | +| Tool sandbox (landlock / sandbox-exec) | `tools/pre-execute` (deny), or implement a sandboxing `BashExecutor` (the dsh-bash seam) | +| Permission system / AskUserQuestion | `tools/pre-execute` (deny/ask); register an ask tool | +| Plan mode | `tools/pre-execute` (deny writes) + `agent/request` (inject mode prompt) | | Sub-agents (spawn / fork / steer) | TODO seam on `AgentLoop.create()`; fork = seed Session with parent events; `steer()` on the child handle | | MCP | one plugin per server: discover tools → `ctx.tools.register()` | | Skills | section + tool registration; `inject()` skill content on invocation | diff --git a/docs/cookbook/extension-cookbook.md b/docs/cookbook/extension-cookbook.md index e6c0378361..98565a5c9e 100644 --- a/docs/cookbook/extension-cookbook.md +++ b/docs/cookbook/extension-cookbook.md @@ -8,24 +8,20 @@ A tool registers on `ctx.tools`. The annotated `defineTool` example (typed `exec ## A hook plugin (permission gate) -A hook wraps the `tools/execute` waterfall to veto or rewrite a call — the seam where sandbox, permission, and plan-mode plugins live. +A hook returns a typed decision from the `tools/pre-execute` gate to allow or deny a call — the seam where sandbox, permission, and plan-mode plugins live. (A "native hook" is just this: an ordinary cordis plugin on the interception seams, returning typed decisions — no external protocol needed.) ```ts import type { Context } from 'cordis' -import type { ToolExecution } from '@deepseek-ai/dsh-tools' +import type { PreToolDecision, ToolExecution } from '@deepseek-ai/dsh-tools' declare function isAllowed(exec: ToolExecution): Promise export const name = 'permission-gate' export function apply(ctx: Context) { - ctx.on('tools/execute', async (exec, next) => { + ctx.on('tools/pre-execute', async (exec, next): Promise => { if (!(await isAllowed(exec))) { - return { - callId: exec.callId, - content: [{ type: 'text', text: 'Denied by policy.' }], - isError: true, - } + return { kind: 'deny', reason: 'Denied by policy.' } } return next() }) diff --git a/docs/cordis-catalog/events-and-services.md b/docs/cordis-catalog/events-and-services.md index 3fac9263db..d37d2d4070 100644 --- a/docs/cordis-catalog/events-and-services.md +++ b/docs/cordis-catalog/events-and-services.md @@ -25,7 +25,7 @@ An agent was registered in the AgentRegistry and is ready to receive messages. Types: [Agent](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:165`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:228`](../../packages/core/agent/src/types.ts) #### `agent/disposed` — emit @@ -37,7 +37,7 @@ An agent was disposed and removed from the registry; its fiber and any in-flight Types: [Agent](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:171`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:234`](../../packages/core/agent/src/types.ts) #### `agent/error` — emit @@ -49,7 +49,19 @@ A step or turn errored. The loop reports a failure here (plus the logger) even w Types: [Agent](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:245`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:334`](../../packages/core/agent/src/types.ts) + +#### `agent/prompt-submit` — waterfall + +Waterfall: decide what happens to ONE drained queued message before it becomes a `user/message` — allow (optionally rewriting the prompt bytes or attaching `additionalContext`) or block it. Fires inside the already-open turn, per drained message. Maps onto Claude Code's `UserPromptSubmit` hook. Call `next()` to delegate to the default (allow unchanged), or return a PromptDecision without calling `next()` to short-circuit. + +```ts cordis-catalog +'agent/prompt-submit'(agent: Agent, content: ContentBlock[], source: MessageSource, next: () => Promise): Promise +``` + +Types: [Agent](../core-data-structures/core.md) · [ContentBlock](../core-data-structures/core.md) · [MessageSource](../core-data-structures/core.md) + +Source: [`packages/core/agent/src/types.ts:293`](../../packages/core/agent/src/types.ts) #### `agent/queued` — emit @@ -61,7 +73,7 @@ A message entered the agent's inbox (queued or steering). `source` is the resolv Types: [Agent](../core-data-structures/core.md) · [ContentBlock](../core-data-structures/core.md) · [MessageSource](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:184`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:247`](../../packages/core/agent/src/types.ts) #### `agent/request` — waterfall @@ -73,7 +85,19 @@ Waterfall: mutate the fully-assembled GenerateOptions before the model call (hoo Types: [Agent](../core-data-structures/core.md) · [GenerateOptions](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:214`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:300`](../../packages/core/agent/src/types.ts) + +#### `agent/session-start` — emit + +The agent's session lifecycle began, fired once before its first turn. `source` says why (SessionStartSource: fresh startup, a resumed persisted session, …). A pure NOTIFICATION (emit, not waterfall): it carries no veto — a session-start listener that wants to seed context does so via `agent.inject()` (a `context/message` the first request sees), not by returning a decision. Cannot block the session from starting; that gap is deliberate (a bridge logs/injects, it does not gate startup). + +```ts cordis-catalog +'agent/session-start'(agent: Agent, source: SessionStartSource): void +``` + +Types: [Agent](../core-data-structures/core.md) + +Source: [`packages/core/agent/src/types.ts:260`](../../packages/core/agent/src/types.ts) #### `agent/status` — emit @@ -85,7 +109,7 @@ Agent status changed (`idle` ⇄ `running`, or → `disposed`). Drive lifecycle Types: [Agent](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:178`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:241`](../../packages/core/agent/src/types.ts) #### `agent/steering` — emit @@ -97,7 +121,7 @@ Steering content was injected into a running turn. Types: [Agent](../core-data-structures/core.md) · [ContentBlock](../core-data-structures/core.md) · [MessageSource](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:239`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:328`](../../packages/core/agent/src/types.ts) #### `agent/step-result` — waterfall @@ -109,7 +133,7 @@ Waterfall: post-process the assembled assistant Message before tool dispatch (va Types: [Agent](../core-data-structures/core.md) · [Message](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:220`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:306`](../../packages/core/agent/src/types.ts) #### `agent/stream-chunk` — emit @@ -121,19 +145,19 @@ A raw StreamChunk arrived from the model (token-level UI/log feed). Types: [Agent](../core-data-structures/core.md) · [StreamChunk](../core-data-structures/llm-streaming.md) -Source: [`packages/core/agent/src/types.ts:234`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:323`](../../packages/core/agent/src/types.ts) #### `agent/turn-continuation` — waterfall -Waterfall: override the turn-continuation decision. The default (computed by the loop) is `hadToolCalls || steeringInjected`. Listeners can force-continue (/goal, /loop) or force-stop (budget guards). +Waterfall: override the turn-continuation decision via a typed ContinuationDecision. The loop's `defaultDecision` is `continue` when the step had tool calls or steering was injected, else `stop`. Listeners force-continue (`/goal`, `/loop` — optionally attaching a `reason` recorded as next-step steering) or force-stop (budget guards). Call `next()` to delegate to the default, or return a decision to override. ```ts cordis-catalog -'agent/turn-continuation'(agent: Agent, turn: number, defaultDecision: boolean, next: () => Promise): Promise +'agent/turn-continuation'(agent: Agent, turn: number, defaultDecision: ContinuationDecision, next: () => Promise): Promise ``` Types: [Agent](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:227`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:316`](../../packages/core/agent/src/types.ts) #### `agent/turn-end` — emit @@ -145,7 +169,7 @@ A turn ended. `reason` distinguishes a clean stop from a truncated, aborted, fai Types: [Agent](../core-data-structures/core.md) · [TurnEndReason](../core-data-structures/session.md) -Source: [`packages/core/agent/src/types.ts:205`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:281`](../../packages/core/agent/src/types.ts) #### `agent/turn-start` — emit @@ -157,7 +181,7 @@ A turn began. `turn` is the 1-based turn number within the session. Types: [Agent](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:197`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:273`](../../packages/core/agent/src/types.ts) ### `llm/*` @@ -261,19 +285,31 @@ A tool was registered or unregistered (the available tool set changed). 'tools/change'(): void ``` -Source: [`packages/core/tools/src/index.ts:48`](../../packages/core/tools/src/index.ts) +Source: [`packages/core/tools/src/index.ts:66`](../../packages/core/tools/src/index.ts) -#### `tools/execute` — waterfall +#### `tools/post-execute` — waterfall -Waterfall around every tool execution — the single seam where sandbox, permission, hook, and plan-mode plugins wrap or veto a call. Listeners receive `(exec, next)`: call `next()` to proceed (possibly around your own logic), or return a ToolExecutionResult without calling `next()` to short-circuit (veto). +Waterfall AFTER a tool runs — where hook plugins inspect the result and accept it (optionally REPLACING the model-facing content, and/or attaching `additionalContext` for the next request) or block it with corrective `feedback` (Claude Code's `PostToolUse`). Listeners receive `(exec, result, next)`: call `next()` to delegate to the default (accept unchanged), or return a PostToolDecision to override. The core tool dispatch sits between the two waterfalls as plain code, all inside `execute`'s outer try/catch (and the tool body keeps its own inner try/catch, so a thrown tool still reaches `post-execute` as an `isError` result). ```ts cordis-catalog -'tools/execute'(this: ToolRegistry, exec: ToolExecution, next: () => Promise): Promise +'tools/post-execute'(this: ToolRegistry, exec: ToolExecution, result: ToolExecutionResult, next: () => Promise): Promise ``` Types: [ToolExecution](../core-data-structures/tools.md) · [ToolExecutionResult](../core-data-structures/tools.md) -Source: [`packages/core/tools/src/index.ts:43`](../../packages/core/tools/src/index.ts) +Source: [`packages/core/tools/src/index.ts:61`](../../packages/core/tools/src/index.ts) + +#### `tools/pre-execute` — waterfall + +Waterfall BEFORE a tool runs — the gate where sandbox, permission, and hook plugins allow or deny a call (Claude Code's `PreToolUse`). Listeners receive `(exec, next)`: call `next()` to delegate to the default (allow), or return a PreToolDecision without calling `next()` to short-circuit. A `deny` skips dispatch and yields an `isError` result; the tool body never runs. Input rewrite is deliberately NOT offered here (see PreToolDecision); `ask` degrades to deny until the permission system lands (`FIXME(permissions)`). + +```ts cordis-catalog +'tools/pre-execute'(this: ToolRegistry, exec: ToolExecution, next: () => Promise): Promise +``` + +Types: [ToolExecution](../core-data-structures/tools.md) + +Source: [`packages/core/tools/src/index.ts:47`](../../packages/core/tools/src/index.ts) ## Services @@ -446,7 +482,7 @@ async execute(exec: ToolExecution): Promise Types: [ToolDefinition](../core-data-structures/tools.md) · [ToolExecution](../core-data-structures/tools.md) · [ToolExecutionResult](../core-data-structures/tools.md) -Source: [`packages/core/tools/src/index.ts:277`](../../packages/core/tools/src/index.ts) +Source: [`packages/core/tools/src/index.ts:341`](../../packages/core/tools/src/index.ts) ## Inherited tier (cordis core + loader/hmr/timer) diff --git a/docs/core-data-structures/core.md b/docs/core-data-structures/core.md index 757e6fb400..78fc36edd9 100644 --- a/docs/core-data-structures/core.md +++ b/docs/core-data-structures/core.md @@ -306,7 +306,42 @@ interface Agent { } ``` -`AgentStatus` is `'idle' | 'running' | 'disposed'`. `AgentId` is a branded string. `AgentOptions` (`model?`, `systemPrompt?`) is merge-extensible — plugins add creation options by declaration merging. The `agent/*` event taxonomy (lifecycle, turn/step boundaries, the `agent/request`/`agent/step-result`/`agent/turn-continuation` waterfalls) is in [architecture.md § Event taxonomy](../architecture.md#event-taxonomy). +`AgentStatus` is `'idle' | 'running' | 'disposed'`. `AgentId` is a branded string. `AgentOptions` (`model?`, `systemPrompt?`) is merge-extensible — plugins add creation options by declaration merging. The `agent/*` event taxonomy (lifecycle, turn/step boundaries, the `agent/prompt-submit`/`agent/request`/`agent/step-result`/`agent/turn-continuation` waterfalls) is in [architecture.md § Event taxonomy](../architecture.md#event-taxonomy). + +## Interception decisions + +Each `agent/*` interception waterfall returns a small, seam-specific typed union — the unified Decision idiom (the tool seams' `PreToolDecision`/`PostToolDecision` in [tools.md](tools.md) follow the same shape). A CC/Codex hook bridge maps its `permissionDecision`/`decision`/`continue`/`additionalContext` fields onto these; a native plugin returns them directly. They share one envelope for model-facing context, `HookContext`, which is `inject()`ed as a `context/message` and so carries a REQUIRED `source` (a missing source would default to `{kind:'user'}` and mislabel plugin context as a user prompt). + +Source: [`packages/core/agent/src/types.ts`](../../packages/core/agent/src/types.ts) + +```ts type-equiv +interface HookContext { + content: ContentBlock[] + source: MessageSource +} +``` + +`agent/prompt-submit` returns a `PromptDecision` (allow a drained queued message — optionally rewriting its `content` or attaching `additionalContext` — or block it; a batch whose every prompt is blocked opens a zero-step turn that ends `rejected`): + +```ts type-equiv +type PromptDecision = + | { kind: 'allow'; content?: ContentBlock[]; additionalContext?: HookContext } + | { kind: 'block'; reason: string } +``` + +`agent/turn-continuation` returns a `ContinuationDecision` (the loop's default is `continue` when the step had tool calls or steering was injected, else `stop`; a `continue` `reason` is recorded as next-step steering in the same turn — the typed `/goal` pattern): + +```ts type-equiv +type ContinuationDecision = + | { action: 'stop' } + | { action: 'continue'; reason?: HookContext } +``` + +`agent/session-start` carries a `SessionStartSource` (why the session lifecycle began; a bridge keys its SessionStart matcher on it): + +```ts type-equiv +type SessionStartSource = 'startup' | 'resume' | 'clear' | 'compact' +``` ## `ToolDefinition` diff --git a/docs/core-data-structures/session.md b/docs/core-data-structures/session.md index dc1c8e228a..21c92612f2 100644 --- a/docs/core-data-structures/session.md +++ b/docs/core-data-structures/session.md @@ -181,6 +181,16 @@ interface TurnEndReasonMap { error: { kind: 'error'; step: number; message: string; code?: string } disposed: { kind: 'disposed' } 'max-tokens': { kind: 'max-tokens' } + /** + * The turn's entire prompt batch was BLOCKED before any step ran — every + * drained queued message was vetoed by an `agent/prompt-submit` listener (a + * hook). The turn still opened (so the boundary stays balanced and the block + * is a durable in-turn fact), but ran zero steps. `reason` carries the block + * message from the vetoing decision. Distinct from `aborted` (a user-driven + * cancel) and `error` (a failure): the prompt was rejected by policy, not + * interrupted or broken. A UI renders it as "prompt blocked by hook". + */ + rejected: { kind: 'rejected'; reason: string } /** * The turn never ended on its own: the process crashed mid-turn and a * persistence backend later closed the orphaned (open) turn on reload so the @@ -195,7 +205,7 @@ interface TurnEndReasonMap { } ``` -`max-tokens` mirrors the model-call `FinishReason` of the same name: any `max-tokens` step in a turn makes the whole turn end `max-tokens` (the cut-short fact wins over a later continuation), so a consumer can tell a clean stop from a truncated one. `interrupted` is the one reason no loop emits — it is synthesized by crash recovery (see [persistence.md](persistence.md)). Both maps are merge-extensible. +`max-tokens` mirrors the model-call `FinishReason` of the same name: any `max-tokens` step in a turn makes the whole turn end `max-tokens` (the cut-short fact wins over a later continuation), so a consumer can tell a clean stop from a truncated one. `rejected` is a zero-step turn whose whole prompt batch an `agent/prompt-submit` hook blocked (the ACP bridge maps it to `cancelled`). `interrupted` is the one reason no loop emits — it is synthesized by crash recovery (see [persistence.md](persistence.md)). Both maps are merge-extensible. ## The turn-enclosure invariant diff --git a/docs/core-data-structures/tools.md b/docs/core-data-structures/tools.md index f255fdbc32..4d6306de14 100644 --- a/docs/core-data-structures/tools.md +++ b/docs/core-data-structures/tools.md @@ -73,7 +73,7 @@ type InferArgs = Simplify< ## Execution: the `tools/execute` waterfall shapes -`ctx.tools.execute()` runs each call through the `tools/execute` waterfall — the single seam where sandbox, permission, hook, and plan-mode plugins wrap or veto. The pending call is a `ToolExecution`; the outcome is a `ToolExecutionResult`. +`ctx.tools.execute()` runs each call through a two-waterfall pipeline — `tools/pre-execute` (the allow/deny/ask gate) → core dispatch → `tools/post-execute` (inspect/replace the result, attach context) — the seams where sandbox, permission, hook, and plan-mode plugins gate or transform a call. The pending call is a `ToolExecution`; the outcome is a `ToolExecutionResult`. ```ts type-equiv interface ToolExecution { @@ -98,10 +98,36 @@ interface ToolExecutionResult { * text in `content` is always present; this is extra structure for code. */ error?: ToolErrorInfo + /** + * Extra model-facing context a `tools/post-execute` listener attached for the + * NEXT request (Claude Code's PostToolUse `additionalContext`). It is NOT part + * of this call's `content` — `content`/`feedback` shape the tool RESULT, but + * `additionalContext` is a SEPARATE `context/message`. A step can carry + * multiple tool calls, so the loop BUFFERS every call's `additionalContext` + * and appends them only AFTER all `tool/result`s for the step, keeping + * tool-call/result adjacency intact. Carried on the result purely to ferry it + * from `execute()` up to the loop's per-step buffer. + */ + additionalContext?: HookContext } ``` -A waterfall listener receives `(exec, next)`: call `next()` to proceed (possibly around your own logic), or return a `ToolExecutionResult` without calling `next()` to veto. An unregistered tool routes through the same catch as a tool-thrown error, so both failure classes get a structured `{ name, code }` (`ToolNotFoundError` → `UNKNOWN_TOOL`) — the loop records a failed tool call instead of failing the whole turn. +Each interception waterfall returns a typed **Decision** (the idiom shared with the `agent/*` seams). `tools/pre-execute` listeners receive `(exec, next)` and return a `PreToolDecision`; `tools/post-execute` listeners receive `(exec, result, next)` and return a `PostToolDecision`: + +```ts type-equiv +type PreToolDecision = + | { kind: 'allow' } + | { kind: 'deny'; reason: string } + | { kind: 'ask'; reason?: string } +``` + +```ts type-equiv +type PostToolDecision = + | { kind: 'accept'; content?: ContentBlock[]; additionalContext?: HookContext } + | { kind: 'block'; feedback: ContentBlock[]; additionalContext?: HookContext } +``` + +Call `next()` to delegate to the default (allow / accept-unchanged), or return a decision to short-circuit. A `pre-execute` `deny` (or `ask`, which degrades to deny until the permission system lands) skips dispatch and yields an `isError` result; input rewrite is deliberately NOT offered on `PreToolDecision` (it would desync the pre-execution audit/history/UI from what ran — its own proposed RFC). A `post-execute` `accept` may replace the model-facing `content` (clean, because `tool/result` is logged after `execute()` returns); a `block` turns the call into an `isError` whose content is the corrective `feedback`. Core dispatch sits between the waterfalls as plain code; the tool body keeps its own try/catch so a thrown tool still reaches `post-execute` as an `isError`. An unregistered tool routes through the same catch as a tool-thrown error, so both failure classes get a structured `{ name, code }` (`ToolNotFoundError` → `UNKNOWN_TOOL`) — the loop records a failed tool call instead of failing the whole turn. ## Tool-presentation UI vocabulary diff --git a/docs/rfc/README.md b/docs/rfc/README.md index 9b1996e89b..a05a7d5c5c 100644 --- a/docs/rfc/README.md +++ b/docs/rfc/README.md @@ -45,6 +45,7 @@ Do NOT write one for a mechanical or local choice (a variable name, a one-file r | [Multiplex concurrent ACP sessions over one connection](proposed/feature/2026-06-14-acp-multi-session.md) | 2026-06-14 | | [Optional Code Mode — model writes TypeScript against an SDK of all tools](proposed/feature/2026-06-15-optional-code-mode.md) | 2026-06-15 | | [Compaction as a capability seam (abstract contract + basic backend)](proposed/feature/2026-06-18-compaction-capability-seam.md) | 2026-06-18 | +| [Pre-tool input rewrite — a consistent design](proposed/feature/2026-06-30-pre-tool-input-rewrite.md) | 2026-06-30 | ### Simplification @@ -86,6 +87,7 @@ Do NOT write one for a mechanical or local choice (a variable name, a one-file r | [Subagent capability seam](implemented/feature/2026-06-21-subagent-capability-seam.md) | 2026-06-21 | | [ACP subagent backend (out-of-process delegation)](implemented/feature/2026-06-22-acp-subagent-backend.md) | 2026-06-22 | | [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 | ### Simplification diff --git a/docs/rfc/implemented/feature/2026-06-30-interception-seams.md b/docs/rfc/implemented/feature/2026-06-30-interception-seams.md new file mode 100644 index 0000000000..5c00760c46 --- /dev/null +++ b/docs/rfc/implemented/feature/2026-06-30-interception-seams.md @@ -0,0 +1,45 @@ +# RFC: Interception seams — the typed-Decision surface a hook programs against + +Status: implemented (accepted 2026-06-30) + + + +## Context + +The harness needs a hooks subsystem: users extend or gate the agent at lifecycle points the way Claude Code (CC) and Codex do. The key reframe driving this design is that **"native hooks" are not a package** — a native hook is just an ordinary Cordis plugin subscribing to the canonical lifecycle events. So the real product is a *powerful, well-typed canonical event surface*; the CC/Codex bridges (a later stack PR) are merely translators that map an external shell-hook protocol onto that same surface. Anything a bridge can do, a plain plugin can do directly — more powerfully (no serialization boundary, full `ctx`, typed returns). + +Before this change the interception surface was incomplete and inconsistent for that goal: there was no per-prompt seam (CC's `UserPromptSubmit`), no session-start signal (CC's `SessionStart`), the single `tools/execute` waterfall conflated the pre-gate and post-inspect phases (CC splits `PreToolUse`/`PostToolUse`), and `agent/turn-continuation` returned a bare `boolean` with no room for a force-continue *reason*. The [event-domain-semantics RFC](../architecture/2026-06-30-event-domain-semantics.md) (the stack's first change) pinned down the three-domain rule and the typed-Decision idiom as the interception convention; this RFC builds the actual seams on top of it. + +## Decision + +Add/​reshape the interception seams so every one returns a small, seam-specific **typed Decision union**, and the set covers the hook points in scope (`session-start`, `prompt-submit`, `pre-tool`, `post-tool`, `stop`-via-continuation). + +**New `agent/*` events** (`dsh-agent`): +- `agent/session-start(agent, source)` — emit, once before turn 1, carrying a `SessionStartSource` (`startup` for a fresh/forked create, `resume` for a reloaded persisted session; `clear`/`compact` reserved). A pure notification — it CANNOT block startup (a deliberate gap: a bridge logs/injects, it does not gate startup). A listener seeds context via `agent.inject()`. +- `agent/prompt-submit(agent, content, source, next) → PromptDecision` — waterfall, fired per drained queued message inside the open turn, before the `user/message` append. `allow` (optionally rewriting the prompt `content` or attaching `additionalContext`) or `block`. + +**Reshaped** `agent/turn-continuation` from `(…, defaultDecision: boolean) → boolean` to `(…, defaultDecision: ContinuationDecision) → ContinuationDecision`. A `{action:'continue', reason?}` may carry model-facing context recorded as next-step steering in the same turn — the typed twin of the existing `/goal` step-end-steer pattern. + +**Split** the single `tools/execute` waterfall into `tools/pre-execute` (→ `PreToolDecision` allow/deny/ask gate) and `tools/post-execute` (→ `PostToolDecision` accept/block, optionally replacing content or attaching `additionalContext`). Core dispatch sits between them as plain code inside `ToolRegistry.execute`'s outer try/catch, and the tool body keeps its own inner try/catch so a thrown tool still becomes an `isError` result that `post-execute` listeners can inspect. + +**New `TurnEndReason` variant** `rejected` (`dsh-session`): a turn whose entire prompt batch was blocked by `prompt-submit`. + +### Three load-bearing loop decisions + +1. **Always open the turn first; a fully-blocked batch is a zero-step `rejected` turn.** `prompt-submit` fires AFTER `turn/start`, per message. A batch whose every prompt is blocked does NOT skip the turn — it opens a zero-step turn that closes with `rejected`. This one move resolves three problems at once: (1) turn-enclosure holds (every event has an open turn to live in); (2) `agent/turn-end` fires and the ACP bridge settles normally (mapping `rejected`→`cancelled`) instead of hanging; (3) the block reason is a durable in-turn fact. An `allow`'s `additionalContext` is `inject()`ed into this now-open turn. + +2. **Post-tool `additionalContext` is buffered and appended AFTER all `tool/result`s.** `content`/`feedback` shape the result `execute()` returns, but `additionalContext` is a SEPARATE `context/message`, and a single step can carry multiple tool calls. Appending context right after each result would interleave `result(c1) → context → result(c2)` and break tool-call/result adjacency. So `execute()` surfaces `additionalContext` on its `ToolExecutionResult`, and the loop buffers every per-call context for the step and appends them as `context/message`(s) only after every `tool/result` is appended. + +3. **A forced `continue` `reason` is enqueued through the steering channel**, so the next step's top-of-loop drain records it as steering for the continued turn — next-*step* steering within the SAME turn, not a next-*turn* prompt (matching the existing `hasSteering` force-continue override). + +### Pre-tool INPUT rewrite is DEFERRED (the over-reach signal) + +`PreToolDecision` is allow/deny/ask only — **no `arguments` rewrite**. Output replacement (`PostToolDecision.accept.content`) is safe because `tool/result` is logged AFTER execution (one source of truth). Input rewrite is NOT safe today: `assistant/message` (the model-history source) and `tool/call` (the audit record) are both logged BEFORE execution, and live consumers READ `tool/call.arguments` for presentation (the ACP bridge remembers them for `presentResult`; `dsh-tool-bash` derives the title/cwd/terminal-vs-background from them). A rewrite that changed only execution would make the UI show one command while another RAN. Designing that consistently (rewriting the audit + history + presentation as one unit) is a real consistency-design problem CC itself warns is racy — so it gets its own [proposed RFC](../../proposed/feature/2026-06-30-pre-tool-input-rewrite.md), and `TODO(pre-tool-input-rewrite)` anchors it at the loop's pre-execute call site. This does not regress any production consumer (no production `tools/execute` listener mutated `exec.arguments`). The low-level capability to mutate `exec` in a `pre-execute` listener still exists (unadvertised — a test shim uses it to thread a generated id), but it is not a first-class advertised contract. + +### What this PR does NOT do + +It does **not** declare `hook/*` SessionEvents (the durable hook-invocation log) — those belong to the `dsh-hook-protocol` library (a later stack PR), because a native plugin can already use the typed Decisions without a durable hook log. A worked native-plugin example/test in this PR (`packages/core/agent-loop/tests/interception.spec.ts`) proves all the seams compose end-to-end through the REAL loop with NO `hook/*` involved — the concrete proof that "native hooks are just a plugin". Compaction (`PreCompact`/`PostCompact`), the Notification hook, Codex `PermissionRequest`, the permission/`ask` system, and the Stop loop-guard remain deferred (`FIXME(permissions)` marks the `ask`→deny degrade). + +## Consequences + +The canonical interception surface is now complete and uniformly typed: a native plugin returns typed decisions directly, and a CC/Codex bridge maps its protocol fields onto the same unions. The loop gained four firing points (session-start emit, prompt-submit waterfall, the post-tool context buffer, the continuation reshape) and the `dsh-tools` registry runs a two-waterfall pipeline; both are documented in [architecture.md](../../../architecture.md) and the package READMEs, and the decision types in [core-data-structures](../../../core-data-structures/core.md#interception-decisions) + [tools.md](../../../core-data-structures/tools.md). All existing `tools/execute` and `turn-continuation` listeners (tests, docs) migrated to the new seams. The ACP bridge maps the new `rejected` reason to `cancelled` (its codec). A pure internal change with no editor-visible transcript shift for the existing scenarios — the new behavior only fires when a hook is registered — so the snapshot goldens are unchanged; a hook-driven snapshot scenario lands with the bridges (the PR that makes a hook observable end-to-end through ACP). diff --git a/docs/rfc/proposed/feature/2026-06-30-pre-tool-input-rewrite.md b/docs/rfc/proposed/feature/2026-06-30-pre-tool-input-rewrite.md new file mode 100644 index 0000000000..4c91b0c4a4 --- /dev/null +++ b/docs/rfc/proposed/feature/2026-06-30-pre-tool-input-rewrite.md @@ -0,0 +1,39 @@ +# RFC: Pre-tool input rewrite — a consistent design (proposed) + +Status: proposed (2026-06-30) + + + +## Context + +The [interception-seams RFC](../../implemented/feature/2026-06-30-interception-seams.md) added `tools/pre-execute` returning a `PreToolDecision` (allow/deny/ask) — but deliberately NOT input rewrite (a hook changing a tool call's `arguments` before it runs). Claude Code's `PreToolUse` hook offers an `updatedInput`, so a faithful CC bridge wants the same. This RFC designs that, separately, because doing it consistently is a real problem — not a field to bolt onto the allow decision. + +## The problem: three readers of pre-execution arguments + +In the loop, a tool call's arguments are committed to the log and read by live consumers BEFORE the tool executes: + +1. **`assistant/message`** is appended before tool dispatch — it is the model-history source `deriveMessages()` replays, so it carries the tool-call arguments the model itself emitted. +2. **`tool/call`** is the durable AUDIT record, appended before `ctx.tools.execute()`. +3. **Live presentation reads `tool/call.arguments`**: the ACP bridge remembers them and passes them to `presentResult`; `dsh-tool-bash` derives the card title, the rawInput, the cwd, and the terminal-vs-background treatment from them. + +So an "input rewrite" that changes ONLY what executes would make the UI show one command while another RAN, and render result state against the wrong arguments — a real inconsistency, not a documentable gap. (The existing low-level capability to mutate `exec.arguments` in a listener has exactly this latent inconsistency; it is unadvertised precisely because of this.) + +## Proposed design (sketch — to validate against the code when built) + +Treat input rewrite as a consistency unit: when a `pre-execute` hook supplies `updatedInput`, the rewrite must be reflected in ALL three readers, atomically, before execution: + +- The `tool/call` audit event records the REWRITTEN arguments (with the original retained in a sidecar field for the audit trail — a hook changed the call, and both the original and the effective arguments are facts worth keeping). +- The `assistant/message` in derived history must agree with what executed — options to evaluate: rewrite the assistant message's tool-call block in place (changes what the model "sees it said"), or record a separate correction the next request carries. The CC model is that the model sees the rewrite took effect. +- Presentation (`presentCall`/`presentResult`) reads the rewritten arguments, so the UI shows what actually ran. + +The shape would extend `PreToolDecision` with an allow-variant `arguments` (or a dedicated `{kind:'rewrite', arguments}`), and the loop would thread the rewrite through the three readers above rather than only into `ctx.tools.execute()`. + +## Why not now + +The interception-seams RFC notes input rewrite "fought the code across two review rounds" — the signal AGENTS.md names for an over-reaching change. Shipping allow/deny/ask first keeps the seam honest (no advertised contract that silently desyncs the UI), and a CC/Codex bridge that receives an `updatedInput` logs it and surfaces a faithful-but-degraded warning (like `ask`→deny) until this lands. This RFC is the home for the consistent design; `TODO(pre-tool-input-rewrite)` in the loop's pre-execute call site anchors it. + +## Open questions + +- Does rewriting the `assistant/message` tool-call block corrupt any provider's expectation on replay, or is a separate correction safer? +- Should the original arguments be preserved on the `tool/call` event (audit) and, if so, under what field? +- How does this interact with a future permission `ask` flow (a user approving a rewritten call)? diff --git a/packages/bash/tool-bash/tests/integration.spec.ts b/packages/bash/tool-bash/tests/integration.spec.ts index a67809ffee..b3d6bb3f77 100644 --- a/packages/bash/tool-bash/tests/integration.spec.ts +++ b/packages/bash/tool-bash/tests/integration.spec.ts @@ -123,7 +123,10 @@ describe('bash tool through the agent loop', () => { textResponse('Background task finished.'), ]) // The second tool call needs the REAL task id from the first result; - // a tools/execute waterfall listener rewrites the scripted arguments. + // a tools/pre-execute listener rewrites the scripted arguments. (This uses + // the low-level capability to mutate `exec` before dispatch — the + // unadvertised mechanism behind a future first-class input-rewrite decision; + // here it is a test shim to thread the generated id, not a product feature.) let taskId = '' const ctx = await harness(adapter) @@ -137,7 +140,7 @@ describe('bash tool through the agent loop', () => { if (match) taskId = match[1]! } }) - ctx.on('tools/execute', async (exec, next) => { + ctx.on('tools/pre-execute', async (exec, next) => { if (exec.name === 'bash_output') { exec.arguments = { task_id: taskId } } diff --git a/packages/core/agent-loop/README.md b/packages/core/agent-loop/README.md index 4892b357bf..ed2e288304 100644 --- a/packages/core/agent-loop/README.md +++ b/packages/core/agent-loop/README.md @@ -45,10 +45,14 @@ Agents listed in config are auto-created at startup. One invocation of `runLoop()` drives one agent for its whole lifetime: ``` +create agent → emit agent/session-start(source) ⟵ once, before turn 1 forever: wait for queued messages (idle) TURN (error-contained): - drain queued → 'turn/start' → session('user/message') + 'turn/start' + each queued: waterfall agent/prompt-submit → allow (→ session('user/message'), + inject additionalContext) | block (drop) + if every prompt blocked: 'turn/end'(rejected), no step ⟵ zero-step turn STEP loop: drain steering assembly = systemPrompt.assemble() @@ -56,10 +60,14 @@ forever: stream llm.stream(request) → session('assistant/chunk') message = waterfall agent/step-result session('assistant/message') - each tool-call: session('tool/call') → tools.execute() → session('tool/result') + each tool-call: session('tool/call') + → tools.execute() [waterfall tools/pre-execute → dispatch → tools/post-execute] + → session('tool/result') + append buffered post-execute additionalContext as session('context/message')(s) drain steering → session('steering/message') - cont = waterfall agent/turn-continuation - if !cont: break + cont = waterfall agent/turn-continuation → ContinuationDecision + ({action:'continue', reason?} records reason as next-step steering) + if action==stop (and no pending steering): break session('turn/end') await session/flush re-enqueue leftover steering as queued @@ -73,9 +81,9 @@ Cancellation: `agent.cancel()` is the single public stop primitive — it clears ### What is NOT here Everything that goes beyond "call the model, run the tools, repeat" belongs to plugins listening on the event taxonomy: -- Hooks: `agent/request`, `agent/step-result`, `tools/execute`, `agent/turn-continuation` +- Hooks: `agent/session-start`, `agent/prompt-submit`, `agent/request`, `agent/step-result`, `tools/pre-execute`, `tools/post-execute`, `agent/turn-continuation` - Compaction: `agent/request` -- Sandbox, permission, plan mode: `tools/execute` +- Sandbox, permission, plan mode: `tools/pre-execute` (deny/ask gate), `tools/post-execute` - Sub-agents: TODO seam on `AgentLoop.create()` - Persistence: `session/event` + `session/flush` - UI: `agent/stream-chunk` + `agent/*` events diff --git a/packages/core/agent-loop/src/index.ts b/packages/core/agent-loop/src/index.ts index ab95ea5aac..9813dadda4 100644 --- a/packages/core/agent-loop/src/index.ts +++ b/packages/core/agent-loop/src/index.ts @@ -10,7 +10,7 @@ import { Context, Service } from 'cordis' import { randomUUID } from 'node:crypto' import z from 'schemastery' -import type { AgentFactory, AgentHandle, AgentId, AgentOptions, CreateAgentOptions, ResumeAgentOptions } from '@deepseek-ai/dsh-agent' +import type { AgentFactory, AgentHandle, AgentId, AgentOptions, CreateAgentOptions, ResumeAgentOptions, SessionStartSource } from '@deepseek-ai/dsh-agent' import type {} from '@deepseek-ai/dsh-llm' import { SessionId } from '@deepseek-ai/dsh-session' import type { Session } from '@deepseek-ai/dsh-session' @@ -129,7 +129,7 @@ export class AgentLoop extends Service implements AgentFactory { // session + agent down as one ordered chain, capturing the loop's closing // flush). The whole effect is owned by THIS fiber; no AgentHandle is needed. const session = this.ctx.sessions.prepare(SessionId(`${id}-session-${randomUUID()}`), { meta: {} }) - const { agent } = this.start(id, options, session) + const { agent } = this.start(id, options, session, 'startup') return agent } @@ -152,7 +152,9 @@ export class AgentLoop extends Service implements AgentFactory { ...options.seed !== undefined ? { seed: options.seed } : {}, meta: options.meta ?? {}, }) - return this.startOwned(options.agentId, options.agentOptions ?? {}, session) + // A seeded (forked) create is still a fresh start, NOT a resume — `resume` + // is reserved for reloading a PERSISTED session via resume()/resumeWith(). + return this.startOwned(options.agentId, options.agentOptions ?? {}, session, 'startup') } /** @@ -224,7 +226,7 @@ export class AgentLoop extends Service implements AgentFactory { ...meta.seedLength !== undefined ? { seedLength: meta.seedLength } : {}, }, }) - return this.startOwned(options.agentId, options.agentOptions ?? {}, session) + return this.startOwned(options.agentId, options.agentOptions ?? {}, session, 'resume') } /** @@ -261,14 +263,33 @@ export class AgentLoop extends Service implements AgentFactory { * so a throwing `session/created`/`agent/created` listener unwinds the * already-yielded disposers instead of leaking. * + * `source` says why the session began ({@link SessionStartSource}); it is + * emitted as `agent/session-start` once, AFTER the agent is registered (so a + * listener can resolve the agent via `ctx.agents.get(id)` and `inject()` into + * it) and BEFORE the loop starts its first turn. The emit is contained: a + * throwing session-start listener must not abort agent construction — it is + * logged, and the agent still starts. (Unlike a turn-boundary throw, there is + * no open turn here to balance; the durable evidence of a session-start hook + * is whatever it `inject()`ed.) + * * Returns the agent plus the composite effect's disposer (`disposeAgent`). */ - private start(id: AgentId, options: AgentOptions, session: Session): { agent: ReactLoopAgent; disposeAgent: () => Promise } { + private start( + id: AgentId, options: AgentOptions, session: Session, source: SessionStartSource, + ): { agent: ReactLoopAgent; disposeAgent: () => Promise } { const agent = new ReactLoopAgent(this.ctx, id, options, session) const dispose = this.ctx.effect(function* (this: AgentLoop) { yield this.ctx.sessions.enter(session) this.ctx.sessions.announce(session) yield this.ctx.agents.register(agent) + // Fire AFTER register (a listener can ctx.agents.get(id) + inject()) and + // BEFORE the loop's first turn. Contained: a throwing listener is logged, + // never aborts construction (no open turn to balance here). + try { + this.ctx.emit('agent/session-start', agent, source) + } catch (error: unknown) { + this.ctx.logger.warn(`agent "${id}": agent/session-start listener threw: ${String(error)}`) + } const stop = agent.start() // Disposed FIRST (LIFO): request loop stop (sync), then AWAIT the loop's // actual exit so its closing flush lands while onAppend (yielded above, @@ -295,8 +316,8 @@ export class AgentLoop extends Service implements AgentFactory { * `AgentHandle.dispose(): Promise` contract (mirrors the ACP `quiesce()` * helper). */ - private startOwned(id: AgentId, options: AgentOptions, session: Session): AgentHandle { - const { agent, disposeAgent } = this.start(id, options, session) + private startOwned(id: AgentId, options: AgentOptions, session: Session, source: SessionStartSource): AgentHandle { + const { agent, disposeAgent } = this.start(id, options, session, source) let disposing: Promise | undefined return { agent, dispose: () => (disposing ??= disposeAgent()) } } diff --git a/packages/core/agent-loop/src/loop.ts b/packages/core/agent-loop/src/loop.ts index 7ee480d570..6ac98743a2 100644 --- a/packages/core/agent-loop/src/loop.ts +++ b/packages/core/agent-loop/src/loop.ts @@ -10,6 +10,7 @@ import type { Context } from 'cordis' import type { FinishReason, GenerateOptions, Message } from '@deepseek-ai/dsh-llm' import { BlockAssembler, HarnessError } from '@deepseek-ai/dsh-llm' +import type { ContinuationDecision, HookContext, PromptDecision } from '@deepseek-ai/dsh-agent' import type { Session, TurnEndReason, TurnTrigger } from '@deepseek-ai/dsh-session' import { renderPrompt } from '@deepseek-ai/dsh-system-prompt' import type {} from '@deepseek-ai/dsh-tools' @@ -367,15 +368,48 @@ async function runTurn(ctx: Context, agent: ReactLoopAgent, handle: LoopHandle, // decides "owed" from the log via isTurnOpen, so even a throwing turn/start // listener — append pushes before notifying — still gets its turn/end). session.append('turn/start', { turn, trigger }) - // Record the queued user messages INSIDE the turn (after turn/start), so - // every event in the log is turn-enclosed. turn/end is now owed, so a throw - // while appending these is caught below and the turn is still closed. + // Each drained queued message runs the `agent/prompt-submit` waterfall before + // it becomes a `user/message` — a hook can rewrite the prompt or block it. + // Recorded INSIDE the turn (after turn/start) so every event is turn-enclosed; + // turn/end is now owed, so a throwing prompt-submit listener (the waterfall + // throws) is caught below and the turn still closes. + let anyAllowed = false + // Seeded with a floor (only observable if the batch were empty, which + // runTurn never allows — it is called with ≥1 queued message); each `block` + // decision carries a required `reason` and overwrites it, so a fully-blocked + // batch always reports the last vetoing reason. + let lastBlockReason = 'prompt blocked by hook' for (const message of queued) { - session.append('user/message', { content: message.content, source: message.source }, { surfaceOp: 'append' }) + const decision = await ctx.waterfall( + 'agent/prompt-submit', agent, message.content, message.source, + () => Promise.resolve({ kind: 'allow' }), + ) + if (decision.kind === 'block') { + lastBlockReason = decision.reason + continue + } + anyAllowed = true + // `allow.content` REPLACES the prompt bytes (a rewrite); absent keeps them. + const content = decision.content ?? message.content + session.append('user/message', { content, source: message.source }, { surfaceOp: 'append' }) + // `allow.additionalContext` is a SEPARATE context/message the next request + // also sees. The turn is open, so inject() appends it into THIS turn. + if (decision.additionalContext) { + agent.inject(decision.additionalContext.content, { source: decision.additionalContext.source }) + } } ctx.emit('agent/turn-start', agent, turn) while (true) { + // A fully-blocked batch (every prompt vetoed by prompt-submit) opens a + // zero-step turn that ends `rejected`: break BEFORE the first step so the + // boundary stays balanced (turn/start → turn/end) and the block is a + // durable in-turn fact. `anyAllowed` never changes inside the loop, so this + // only ever fires on the first iteration. + if (!anyAllowed) { + reason = { kind: 'rejected', reason: lastBlockReason } + break + } step += 1 // Steering from the previous round's continuation listeners (or @@ -448,10 +482,10 @@ async function runTurn(ctx: Context, agent: ReactLoopAgent, handle: LoopHandle, if (closeStep()) break - const defaultDecision = stepOutcome.hadToolCalls || steered - let shouldContinue: boolean + const defaultDecision: ContinuationDecision = { action: stepOutcome.hadToolCalls || steered ? 'continue' : 'stop' } + let decision: ContinuationDecision try { - shouldContinue = await ctx.waterfall( + decision = await ctx.waterfall( 'agent/turn-continuation', agent, turn, defaultDecision, () => Promise.resolve(defaultDecision), ) @@ -461,9 +495,18 @@ async function runTurn(ctx: Context, agent: ReactLoopAgent, handle: LoopHandle, break } + // A forced `continue` may carry model-facing context: record it as + // next-STEP steering (the steering channel), so the continued turn's next + // iteration drains it before its request — the typed twin of the /goal + // step/end-steer pattern. + if (decision.action === 'continue' && decision.reason) { + agent.inbox.steer({ content: decision.reason.content, source: decision.reason.source }) + } + let shouldContinue = decision.action === 'continue' + // Steering from step/end session-event or continuation listeners (the - // /goal pattern) demands the model see it — it overrides a negative - // decision; the next iteration's drain records it. + // /goal pattern) demands the model see it — it overrides a stop decision; + // the next iteration's drain records it. if (!shouldContinue && agent.inbox.hasSteering) shouldContinue = true // A cancel that landed during the continuation window — after the step's @@ -645,6 +688,12 @@ async function runStep( // ToolRegistry.execute converts tool failures (including aborts) into // isError results, so abort is re-checked around every call here. const toolCalls = message.content.filter(block => block.type === 'tool-call') + // Per-step buffer of `additionalContext` attached by tools/post-execute + // listeners. Appended as context/message(s) only AFTER every tool/result for + // the step, so a multi-call step keeps tool-call/result adjacency + // (interleaving context between a call's result and the next call's would + // break the pairing the next model request relies on). + const pendingContext: HookContext[] = [] for (const call of toolCalls) { /* v8 ignore next -- signal.reason always set: cancel()/disposal provide a default */ if (signal.aborted) throw new Error(String(signal.reason ?? 'aborted')) @@ -655,6 +704,12 @@ async function runStep( } catch { parsedArguments = call.arguments } + // TODO(pre-tool-input-rewrite): tools/pre-execute deliberately cannot rewrite + // `arguments` — tool/call (the audit record) and assistant/message (the + // model-history source) are logged BEFORE execute, and live consumers (ACP, + // tool-bash presentation) read the pre-execution args, so an execution-only + // rewrite would desync the UI from what ran. Designing that consistently is + // its own proposed RFC (docs/rfc/proposed/feature/…-pre-tool-input-rewrite.md). const result = await ctx.tools.execute({ callId: call.id, name: call.name, @@ -666,7 +721,7 @@ async function runStep( turn, step, // The correlation id MUST be the loop's authoritative call.id (the // model-transcript id that deriveMessages turns into toolCallId), NOT - // result.callId — a tools/execute waterfall listener returning a + // result.callId — a post-execute waterfall listener returning a // mismatched id would otherwise orphan the call↔result pairing in the // next model request. A listener-internal id, if ever needed, belongs in // a separate diagnostic field, never overloaded onto callId. @@ -675,6 +730,8 @@ async function runStep( isError: result.isError, ...result.error ? { error: result.error } : {}, }, { surfaceOp: 'append', sourceEventSeqs: [callEvent.seq] }) + // Buffer (don't append yet) any post-execute additionalContext for this call. + if (result.additionalContext) pendingContext.push(result.additionalContext) // signal CAN flip during the await above (abort() inside a tool); // the analyzer can't see through the await boundary. /* v8 ignore start -- signal.reason default unreachable: cancel()/disposal always set it */ @@ -683,6 +740,13 @@ async function runStep( /* v8 ignore stop */ } + // Append buffered post-execute context AFTER every tool/result, preserving + // tool-call/result adjacency across the whole batch. inject() appends into the + // open turn (a context/message at its chronological position). + for (const context of pendingContext) { + agent.inject(context.content, { source: context.source }) + } + return { hadToolCalls: toolCalls.length > 0, finish: assembler.finish } } diff --git a/packages/core/agent-loop/tests/cancel.spec.ts b/packages/core/agent-loop/tests/cancel.spec.ts index 775e08e6de..6c5b92d7d2 100644 --- a/packages/core/agent-loop/tests/cancel.spec.ts +++ b/packages/core/agent-loop/tests/cancel.spec.ts @@ -213,7 +213,7 @@ describe('Agent.cancel()', () => { if (subject === agent && !continued) { continued = true agent.cancel('from continuation') - return true // vote to continue — the post-waterfall marker check must override + return { action: 'continue' as const } // vote to continue — the post-waterfall marker check must override } return next() }) diff --git a/packages/core/agent-loop/tests/interception.spec.ts b/packages/core/agent-loop/tests/interception.spec.ts new file mode 100644 index 0000000000..33410d9c91 --- /dev/null +++ b/packages/core/agent-loop/tests/interception.spec.ts @@ -0,0 +1,446 @@ +import { describe, expect, it } from 'vitest' +import { Context } from 'cordis' +import LlmService, { CallId } from '@deepseek-ai/dsh-llm' +import SessionStore, { type SessionEvent, type TurnEndReason } from '@deepseek-ai/dsh-session' +import SystemPrompt from '@deepseek-ai/dsh-system-prompt' +import ToolRegistry, { defineTool, type PostToolDecision, type PreToolDecision } from '@deepseek-ai/dsh-tools' +import AgentRegistry, { + AgentId, + type ContinuationDecision, + type PromptDecision, + type SessionStartSource, +} from '@deepseek-ai/dsh-agent' +import AgentLoop, { type ReactLoopAgent } from '@deepseek-ai/dsh-agent-loop' +import { MockAdapter, textResponse, toolCallResponse } from './mock-adapter.ts' + +/** + * The interception seams introduced by the hooks taxonomy: `agent/prompt-submit`, + * `agent/session-start`, the reshaped `agent/turn-continuation` + * ({@link ContinuationDecision}), and the `tools/pre-execute` / `tools/post-execute` + * split with `additionalContext` buffering. These verify the canonical event + * surface a hook bridge (or a native plugin) programs against, WITHOUT any + * external protocol — a native plugin uses the typed decisions directly. + */ + +async function harness(adapter: MockAdapter) { + const ctx = new Context() + await ctx.plugin(LlmService) + await ctx.plugin(SessionStore) + await ctx.plugin(SystemPrompt) + await ctx.plugin(ToolRegistry) + await ctx.plugin(AgentRegistry) + await ctx.plugin(AgentLoop, { agents: [] }) + ctx.llm.registerAdapter(['mock'], adapter) + return ctx +} + +function waitForIdle(ctx: Context, agent: ReactLoopAgent): Promise { + return new Promise((resolve) => { + const dispose = ctx.on('agent/status', (subject, status) => { + if (subject === agent && status === 'idle') { + dispose() + resolve() + } + }) + }) +} + +function send(agent: ReactLoopAgent, text: string) { + agent.send([{ type: 'text', text }]) +} + +function events(agent: ReactLoopAgent): SessionEvent[] { + return [...agent.session.events] +} + +describe('agent/prompt-submit', () => { + it('allow (default via next) records the user/message unchanged', async () => { + const adapter = new MockAdapter([textResponse('ok')]) + const ctx = await harness(adapter) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + + const seen: string[] = [] + ctx.on('agent/prompt-submit', async (_agent, content, _source, next) => { + seen.push(content.map(b => (b.type === 'text' ? b.text : '')).join('')) + return next() + }) + + send(agent, 'hello') + await waitForIdle(ctx, agent) + + expect(seen).toEqual(['hello']) + const userMsg = events(agent).find(e => e.type === 'user/message') + expect(userMsg?.type === 'user/message' && userMsg.data.content).toEqual([{ type: 'text', text: 'hello' }]) + }) + + it('allow with content REWRITES the prompt before it is recorded', async () => { + const adapter = new MockAdapter([textResponse('ok')]) + const ctx = await harness(adapter) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + + ctx.on('agent/prompt-submit', async (): Promise => + ({ kind: 'allow', content: [{ type: 'text', text: 'REWRITTEN' }] })) + + send(agent, 'original') + await waitForIdle(ctx, agent) + + const userMsg = events(agent).find(e => e.type === 'user/message') + expect(userMsg?.type === 'user/message' && userMsg.data.content).toEqual([{ type: 'text', text: 'REWRITTEN' }]) + // the rewritten prompt is what reached the model + expect(JSON.stringify(adapter.requests[0]!.messages)).toContain('REWRITTEN') + expect(JSON.stringify(adapter.requests[0]!.messages)).not.toContain('original') + }) + + it('allow with additionalContext injects a separate context/message into the turn', async () => { + const adapter = new MockAdapter([textResponse('ok')]) + const ctx = await harness(adapter) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + + ctx.on('agent/prompt-submit', async (): Promise => + ({ + kind: 'allow', + additionalContext: { content: [{ type: 'text', text: 'extra ctx' }], source: { kind: 'plugin', plugin: 'test' } }, + })) + + send(agent, 'go') + await waitForIdle(ctx, agent) + + const log = events(agent) + const userMsg = log.find(e => e.type === 'user/message') + const ctxMsg = log.find(e => e.type === 'context/message') + expect(userMsg).toBeDefined() + expect(ctxMsg?.type === 'context/message' && ctxMsg.data.content).toEqual([{ type: 'text', text: 'extra ctx' }]) + expect(ctxMsg?.type === 'context/message' && ctxMsg.data.source).toEqual({ kind: 'plugin', plugin: 'test' }) + // both the prompt and the injected context reach the model + const sent = JSON.stringify(adapter.requests[0]!.messages) + expect(sent).toContain('extra ctx') + }) + + it('block drops the (only) prompt → zero-step turn ends rejected, model never called', async () => { + const adapter = new MockAdapter([textResponse('should not run')]) + const ctx = await harness(adapter) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + + ctx.on('agent/prompt-submit', async (): Promise => + ({ kind: 'block', reason: 'blocked by policy' })) + + const reasons: TurnEndReason[] = [] + ctx.on('agent/turn-end', (_a, _t, reason) => void reasons.push(reason)) + + send(agent, 'do something') + await waitForIdle(ctx, agent) + + // the model was never called + expect(adapter.requests).toHaveLength(0) + // the turn opened and closed balanced, with no user/message and no step + const log = events(agent) + expect(log.some(e => e.type === 'turn/start')).toBe(true) + expect(log.some(e => e.type === 'turn/end')).toBe(true) + expect(log.some(e => e.type === 'user/message')).toBe(false) + expect(log.some(e => e.type === 'step/start')).toBe(false) + // ended rejected with the block reason + expect(reasons).toEqual([{ kind: 'rejected', reason: 'blocked by policy' }]) + const turnEnd = log.findLast(e => e.type === 'turn/end') + expect(turnEnd?.type === 'turn/end' && turnEnd.data.reason).toEqual({ kind: 'rejected', reason: 'blocked by policy' }) + }) + + it('a throwing prompt-submit listener ends the turn balanced (error), loop survives', async () => { + const adapter = new MockAdapter([textResponse('after')]) + const ctx = await harness(adapter) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + + let threw = false + ctx.on('agent/prompt-submit', async () => { + if (!threw) { threw = true; throw new Error('prompt hook broke') } + return { kind: 'allow' as const } + }) + const errors: Error[] = [] + ctx.on('agent/error', (_a, _t, _s, error) => void errors.push(error)) + + send(agent, 'first') + await waitForIdle(ctx, agent) + expect(errors.map(e => e.message)).toEqual(['prompt hook broke']) + // turn balanced + const log = events(agent) + expect(log.filter(e => e.type === 'turn/start')).toHaveLength(1) + expect(log.filter(e => e.type === 'turn/end')).toHaveLength(1) + + // loop survives: a second prompt runs normally + send(agent, 'second') + await waitForIdle(ctx, agent) + expect(adapter.requests.length).toBeGreaterThanOrEqual(1) + }) +}) + +describe('agent/session-start', () => { + it('fires once with source "startup" for a fresh create, before the first turn', async () => { + const adapter = new MockAdapter([textResponse('ok')]) + const ctx = await harness(adapter) + + const sources: SessionStartSource[] = [] + ctx.on('agent/session-start', (_agent, source) => void sources.push(source)) + + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + // fires synchronously at create, before any turn + expect(sources).toEqual(['startup']) + expect(events(agent).some(e => e.type === 'turn/start')).toBe(false) + + send(agent, 'go') + await waitForIdle(ctx, agent) + // still only one session-start + expect(sources).toEqual(['startup']) + }) + + it('a session-start listener can inject context the first request sees', async () => { + const adapter = new MockAdapter([textResponse('ok')]) + const ctx = await harness(adapter) + + ctx.on('agent/session-start', (agent) => { + agent.inject([{ type: 'text', text: 'session preamble' }], { source: { kind: 'plugin', plugin: 'test' } }) + }) + + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + send(agent, 'go') + await waitForIdle(ctx, agent) + + // the injected context reached the model on the first (only) request + expect(JSON.stringify(adapter.requests[0]!.messages)).toContain('session preamble') + // and is recorded with the plugin source, never mislabeled as a user prompt + const ctxMsg = events(agent).find(e => e.type === 'context/message') + expect(ctxMsg?.type === 'context/message' && ctxMsg.data.source).toEqual({ kind: 'plugin', plugin: 'test' }) + }) + + it('a throwing session-start listener does not abort agent construction', async () => { + const adapter = new MockAdapter([textResponse('ok')]) + const ctx = await harness(adapter) + + ctx.on('agent/session-start', () => { throw new Error('session-start hook broke') }) + + // create must not throw — the listener error is contained/logged + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + expect(agent.id).toBe(AgentId('a1')) + + // and the agent still runs + send(agent, 'go') + await waitForIdle(ctx, agent) + expect(adapter.requests).toHaveLength(1) + }) +}) + +describe('agent/turn-continuation (ContinuationDecision)', () => { + it('a continue decision with a reason records next-step steering in the same turn', async () => { + const adapter = new MockAdapter([textResponse('step 1 no tools'), textResponse('step 2')]) + const ctx = await harness(adapter) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + + let forced = false + ctx.on('agent/turn-continuation', async (_agent, _turn, _default, next): Promise => { + if (!forced) { + forced = true + return { action: 'continue', reason: { content: [{ type: 'text', text: 'keep going on the goal' }], source: { kind: 'plugin', plugin: 'goal' } } } + } + return next() + }) + + send(agent, 'go') + await waitForIdle(ctx, agent) + + const log = events(agent) + // same turn, two steps + expect(log.filter(e => e.type === 'turn/start')).toHaveLength(1) + expect(log.filter(e => e.type === 'step/start')).toHaveLength(2) + // the reason was recorded as steering BEFORE step 2, with its plugin source + const steering = log.find(e => e.type === 'steering/message') + expect(steering?.type === 'steering/message' && steering.data.content).toEqual([{ type: 'text', text: 'keep going on the goal' }]) + expect(steering?.type === 'steering/message' && steering.data.source).toEqual({ kind: 'plugin', plugin: 'goal' }) + // and reached the next request + expect(JSON.stringify(adapter.requests[1]!.messages)).toContain('keep going on the goal') + }) + + it('a stop decision ends the turn even when the step had tool calls', async () => { + const adapter = new MockAdapter([toolCallResponse('c1', 'echo', { text: 'hi' })]) + const ctx = await harness(adapter) + ctx.tools.register(defineTool({ + name: 'echo', description: 'echo', parameters: { text: { type: 'string' } }, + async execute(args) { return [{ type: 'text', text: String(args.text) }] }, + })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + + ctx.on('agent/turn-continuation', async (): Promise => ({ action: 'stop' })) + + send(agent, 'go') + await waitForIdle(ctx, agent) + + // default would have continued (had tool calls), but the stop decision wins + expect(adapter.requests).toHaveLength(1) + expect(events(agent).some(e => e.type === 'tool/result')).toBe(true) + }) +}) + +describe('tools/post-execute additionalContext buffering across a multi-call step', () => { + it('appends each call\'s additionalContext only AFTER all tool/results, preserving adjacency', async () => { + // One assistant step with TWO tool calls; the second model response stops. + const twoCalls = [ + { type: 'block-start' as const, index: 0, blockType: 'tool-call' as const }, + { type: 'block-end' as const, index: 0, block: { type: 'tool-call' as const, id: CallId('c1'), name: 'echo', arguments: '{"text":"a"}' } }, + { type: 'block-start' as const, index: 1, blockType: 'tool-call' as const }, + { type: 'block-end' as const, index: 1, block: { type: 'tool-call' as const, id: CallId('c2'), name: 'echo', arguments: '{"text":"b"}' } }, + { type: 'usage' as const, usage: { inputTokens: 5, outputTokens: 5 } }, + { type: 'finish' as const, reason: { kind: 'tool-calls' as const } }, + ] + const adapter = new MockAdapter([twoCalls, textResponse('done')]) + const ctx = await harness(adapter) + ctx.tools.register(defineTool({ + name: 'echo', description: 'echo', parameters: { text: { type: 'string' } }, + async execute(args) { return [{ type: 'text', text: String(args.text) }] }, + })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + + // Each call attaches additionalContext naming itself. + ctx.on('tools/post-execute', async (exec, _result): Promise => + ({ kind: 'accept', additionalContext: { content: [{ type: 'text', text: `ctx-${exec.callId}` }], source: { kind: 'plugin', plugin: 'p' } } })) + + send(agent, 'go') + await waitForIdle(ctx, agent) + + // Event order in the log: both tool/results, THEN both context/messages — + // never interleaved (which would break tool-call/result adjacency). + const types = events(agent).map(e => e.type) + const firstResult = types.indexOf('tool/result') + const lastResult = types.lastIndexOf('tool/result') + const firstCtx = types.indexOf('context/message') + expect(firstResult).toBeGreaterThanOrEqual(0) + expect(lastResult).toBeGreaterThan(firstResult) // two results + expect(firstCtx).toBeGreaterThan(lastResult) // context only after ALL results + // both contexts present + const ctxTexts = events(agent) + .filter(e => e.type === 'context/message') + .flatMap(e => (e.type === 'context/message' ? e.data.content : [])) + .map(b => (b.type === 'text' ? b.text : '')) + expect(ctxTexts).toEqual(['ctx-c1', 'ctx-c2']) + }) +}) + +describe('tools/pre-execute gate (native-plugin permission pattern, end-to-end through the loop)', () => { + it('deny short-circuits dispatch into an isError result the model sees', async () => { + const adapter = new MockAdapter([toolCallResponse('c1', 'danger', {}), textResponse('ok')]) + const ctx = await harness(adapter) + let ran = false + ctx.tools.register(defineTool({ + name: 'danger', description: 'danger', parameters: {}, + async execute() { ran = true; return [{ type: 'text', text: 'should not run' }] }, + })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + + ctx.on('tools/pre-execute', async (exec, next): Promise => { + if (exec.name === 'danger') return { kind: 'deny', reason: 'blocked dangerous tool' } + return next() + }) + + send(agent, 'go') + await waitForIdle(ctx, agent) + + expect(ran).toBe(false) + const result = events(agent).find(e => e.type === 'tool/result') + expect(result?.type === 'tool/result' && result.data.isError).toBe(true) + expect(result?.type === 'tool/result' + && result.data.content.some(b => b.type === 'text' && b.text.includes('blocked dangerous tool'))).toBe(true) + }) +}) + +describe('worked example: a native hook plugin is just a cordis plugin on the seams', () => { + // The whole point of the interception taxonomy: a "native hook" needs no + // dsh-hook-protocol, no external command, no hook/* log — it is an ordinary + // cordis plugin subscribing to the canonical events and returning typed + // decisions. This proves all four seams compose end-to-end through the REAL + // loop, with NO hook/* SessionEvents involved (those belong to the bridge lib). + const NativeGuard = { + name: 'native-guard', + apply(ctx: Context) { + // 1. SessionStart: seed a standing instruction. + ctx.on('agent/session-start', (agent, source) => { + agent.inject( + [{ type: 'text', text: `policy active (started: ${source})` }], + { source: { kind: 'plugin', plugin: 'native-guard' } }, + ) + }) + // 2. PromptSubmit: block a forbidden prompt, annotate the rest. + ctx.on('agent/prompt-submit', async (_agent, content, _source, next): Promise => { + const text = content.map(b => (b.type === 'text' ? b.text : '')).join('') + if (text.includes('rm -rf')) return { kind: 'block', reason: 'destructive prompt blocked' } + return next() + }) + // 3. PreToolUse: deny a dangerous tool by name. + ctx.on('tools/pre-execute', async (exec, next): Promise => { + if (exec.name === 'danger') return { kind: 'deny', reason: 'danger tool denied' } + return next() + }) + // 4. PostToolUse: attach context after a tool runs. + ctx.on('tools/post-execute', async (_exec, _result, next): Promise => { + const decision = await next() + if (decision.kind === 'accept') { + return { kind: 'accept', additionalContext: { content: [{ type: 'text', text: 'audited' }], source: { kind: 'plugin', plugin: 'native-guard' } } } + } + return decision + }) + }, + } + + it('all four seams fire for a real allowed turn with a tool call', async () => { + const adapter = new MockAdapter([toolCallResponse('c1', 'echo', { text: 'hi' }), textResponse('done')]) + const ctx = await harness(adapter) + await ctx.plugin(NativeGuard) + ctx.tools.register(defineTool({ + name: 'echo', description: 'echo', parameters: { text: { type: 'string' } }, + async execute(args) { return [{ type: 'text', text: String(args.text) }] }, + })) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + + send(agent, 'please echo hi') + await waitForIdle(ctx, agent) + + const log = events(agent) + // session-start preamble injected + expect(log.some(e => e.type === 'context/message' + && e.data.content.some(b => b.type === 'text' && b.text.includes('policy active (started: startup)')))).toBe(true) + // prompt allowed → user/message recorded + expect(log.some(e => e.type === 'user/message')).toBe(true) + // tool ran (echo allowed) and post-execute attached "audited" context + expect(log.some(e => e.type === 'tool/result' && !e.data.isError)).toBe(true) + expect(log.some(e => e.type === 'context/message' + && e.data.content.some(b => b.type === 'text' && b.text === 'audited'))).toBe(true) + // NO hook/* events — a native plugin needs none + expect(log.some(e => e.type.startsWith('hook/'))).toBe(false) + }) + + it('the same plugin blocks a destructive prompt → rejected turn, model never called', async () => { + const adapter = new MockAdapter([textResponse('should not run')]) + const ctx = await harness(adapter) + await ctx.plugin(NativeGuard) + const agent = ctx.agentLoop.create(AgentId('a2'), { model: 'mock' }) + + const reasons: TurnEndReason[] = [] + ctx.on('agent/turn-end', (_a, _t, reason) => void reasons.push(reason)) + + send(agent, 'run rm -rf /') + await waitForIdle(ctx, agent) + + expect(adapter.requests).toHaveLength(0) + expect(reasons).toEqual([{ kind: 'rejected', reason: 'destructive prompt blocked' }]) + }) + + it('HMR-safety: disposing the plugin fiber removes all four listeners', async () => { + const adapter = new MockAdapter([textResponse('ok')]) + const ctx = await harness(adapter) + const fiber = await ctx.plugin(NativeGuard) + await fiber.dispose() + + // After disposal, a destructive prompt is NOT blocked (the listener is gone). + const agent = ctx.agentLoop.create(AgentId('a3'), { model: 'mock' }) + send(agent, 'run rm -rf /') + await waitForIdle(ctx, agent) + // the prompt ran (not rejected) — proving the prompt-submit listener was disposed + expect(adapter.requests).toHaveLength(1) + expect(events(agent).some(e => e.type === 'user/message')).toBe(true) + }) +}) diff --git a/packages/core/agent-loop/tests/loop.spec.ts b/packages/core/agent-loop/tests/loop.spec.ts index 8d8224ad5f..9377b161ca 100644 --- a/packages/core/agent-loop/tests/loop.spec.ts +++ b/packages/core/agent-loop/tests/loop.spec.ts @@ -277,7 +277,7 @@ describe('agent loop', () => { let steps = 0 ctx.on('session/event', (_session, event) => { if (event.type === 'step/end') steps++ }) ctx.on('agent/turn-continuation', async (_agent, _turn, _defaultDecision, next) => { - if (steps < 3) return true + if (steps < 3) return { action: 'continue' as const } return next() }) @@ -300,7 +300,7 @@ describe('agent loop', () => { })) const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) - ctx.on('agent/turn-continuation', async () => false as const) + ctx.on('agent/turn-continuation', async () => ({ action: 'stop' }) as const) send(agent, 'go') await waitForIdle(ctx, agent) @@ -381,7 +381,7 @@ describe('agent loop', () => { // Force exactly one continuation (step 1 → step 2), then defer to default // (step 2 is a plain stop with no tool calls → stops). ctx.on('agent/turn-continuation', async (_agent, _turn, _defaultDecision, next) => { - if (steps < 2) return true + if (steps < 2) return { action: 'continue' as const } return next() }) diff --git a/packages/core/agent-loop/tests/resume.spec.ts b/packages/core/agent-loop/tests/resume.spec.ts index 074e84d78a..805f61d392 100644 --- a/packages/core/agent-loop/tests/resume.spec.ts +++ b/packages/core/agent-loop/tests/resume.spec.ts @@ -94,6 +94,36 @@ describe('the session-persistence RFC: AgentLoop factory create/resume', () => { await ctx2.fiber.dispose() }) + it('agent/session-start fires "startup" for createAgent and "resume" for resume()', async () => { + // Lifecycle 1: a fresh createAgent emits session-start with source 'startup'. + const adapter1 = new MockAdapter([textResponse('a')]) + const { ctx: ctx1, root } = await persistentHarness(adapter1) + const sources1: string[] = [] + ctx1.on('agent/session-start', (_agent, source) => void sources1.push(source)) + const a1 = ctx1.agents.create({ agentId: AgentId('s'), sessionId: SessionId('start-sess') }).agent as ReactLoopAgent + expect(sources1).toEqual(['startup']) + a1.send([{ type: 'text', text: 'q' }], { source: { kind: 'user' } }) + await waitForIdle(ctx1, a1) + await ctx1.fiber.dispose() + + // Lifecycle 2: resuming the persisted session emits session-start 'resume'. + const adapter2 = new MockAdapter([textResponse('b')]) + const ctx2 = new Context() + await ctx2.plugin(LlmService) + await ctx2.plugin(SessionStore) + await ctx2.plugin(SystemPrompt) + await ctx2.plugin(ToolRegistry) + await ctx2.plugin(AgentRegistry) + await ctx2.plugin(AgentLoop, { agents: [] }) + await ctx2.plugin(SessionPersistenceJsonl, { root }) + ctx2.llm.registerAdapter(['mock'], adapter2) + const sources2: string[] = [] + ctx2.on('agent/session-start', (_agent, source) => void sources2.push(source)) + await ctx2.agents.resume({ agentId: AgentId('s'), resumeSessionId: SessionId('start-sess') }) + expect(sources2).toEqual(['resume']) + await ctx2.fiber.dispose() + }) + it('resume of a forked session preserves the parentSession lineage and seed boundary in the header', async () => { // Lifecycle 1: persist a FORKED session (carries parentSession + seedLength // in its header) by creating it with a complete-turn seed — the write path diff --git a/packages/core/agent-loop/tests/review-fixes.spec.ts b/packages/core/agent-loop/tests/review-fixes.spec.ts index ab421aaa2e..434ad31fcb 100644 --- a/packages/core/agent-loop/tests/review-fixes.spec.ts +++ b/packages/core/agent-loop/tests/review-fixes.spec.ts @@ -4,7 +4,7 @@ import LlmService, { CallId, MessageSource, StreamChunk } from '@deepseek-ai/dsh import SessionStore, { Session, SessionEvent, SessionId, TurnEndReason } from '@deepseek-ai/dsh-session' import SystemPrompt from '@deepseek-ai/dsh-system-prompt' import ToolRegistry, { defineTool } from '@deepseek-ai/dsh-tools' -import AgentRegistry, { AgentId } from '@deepseek-ai/dsh-agent' +import AgentRegistry, { AgentId, type ContinuationDecision } from '@deepseek-ai/dsh-agent' import AgentLoop, { ReactLoopAgent } from '@deepseek-ai/dsh-agent-loop' import * as Invariants from '@deepseek-ai/dsh-invariants' import { MockAdapter, textResponse, toolCallResponse } from './mock-adapter.ts' @@ -271,12 +271,12 @@ describe('HIGH: plugin exceptions are contained', () => { const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) let threwOnce = false - ctx.on('agent/turn-continuation', async (): Promise => { + ctx.on('agent/turn-continuation', async (): Promise => { if (!threwOnce) { threwOnce = true throw new Error('broken continuation plugin') } - return false + return { action: 'stop' } }) const errors: Error[] = [] @@ -1005,7 +1005,7 @@ describe('P1-5: a started turn (and any open step) is always closed on a boundar }) describe('P1-7: tool/result is logged under the originating call.id, not result.callId', () => { - it('a tools/execute listener returning a mismatched callId cannot orphan the call↔result pairing', async () => { + it('the loop records tool/result under the model call.id even when a post-execute listener replaces content', async () => { // Model emits a tool-call with id "c1", then a final text turn. const adapter = new MockAdapter([ toolCallResponse('c1', 'echo', { x: 1 }), @@ -1019,12 +1019,13 @@ describe('P1-7: tool/result is logged under the originating call.id, not result. async execute() { return [{ type: 'text', text: 'ok' }] }, })) - // A waterfall listener short-circuits with a result carrying the WRONG - // callId (a listener-internal/proxy id). The loop must still record the - // tool/result under the model's authoritative call.id. - ctx.on('tools/execute', (exec) => { + // A post-execute listener transforms the result (accept-with-replacement). + // The loop must still record the tool/result under the model's authoritative + // call.id (the loop ignores result.callId — which the registry always sets to + // exec.callId anyway — and uses call.id, the model-transcript id). + ctx.on('tools/post-execute', (exec, _result) => { expect(exec.callId).toBe(CallId('c1')) // the loop passed the real id in - return Promise.resolve({ callId: CallId('wrong-proxy-id'), content: [{ type: 'text', text: 'ok' }], isError: false }) + return Promise.resolve({ kind: 'accept', content: [{ type: 'text', text: 'ok' }] }) }, { prepend: true }) const agent = ctx.agentLoop.create(AgentId('a-callid'), { model: 'mock' }) diff --git a/packages/core/agent/README.md b/packages/core/agent/README.md index 1d1b839da3..c66038ff43 100644 --- a/packages/core/agent/README.md +++ b/packages/core/agent/README.md @@ -31,6 +31,7 @@ The full `agent/*` event taxonomy is declared via declaration merging in `dsh-ag - `agent/created`, `agent/disposed` — registration/deregistration - `agent/status` — idle / running / disposed transition - `agent/queued` — message entered inbox (source-resolved, steering flag) +- `agent/session-start` — the session lifecycle began (once, before turn 1), carrying a `SessionStartSource` (`startup` for a fresh or forked create, `resume` for a reloaded persisted session; `clear`/`compact` reserved). A pure notification — it cannot block startup; a listener seeds context via `agent.inject()` (a `context/message` the first request sees). #### Turn boundaries (emit) @@ -40,9 +41,14 @@ Step boundaries are NOT mirrored as `agent/*` emits: a consumer that needs per-s #### Interception seams (waterfall) +Each interception waterfall returns a small, seam-specific typed **Decision** union (the unified idiom across the taxonomy — a CC/Codex bridge maps its `permissionDecision`/`decision`/`continue` fields onto these, a native plugin returns them directly): + +- `agent/prompt-submit` — decide what happens to one drained queued message before it becomes a `user/message`: `PromptDecision` = `allow` (optionally rewriting the prompt `content` or attaching `additionalContext`) or `block` (drop it; a batch whose every prompt is blocked opens a zero-step turn that ends `rejected`). Maps onto Claude Code's `UserPromptSubmit`. - `agent/request` — mutate `GenerateOptions` before the model call (hooks, compaction, model switching, tool filtering) - `agent/step-result` — post-process the assembled assistant message before tool dispatch (validates what the log records) -- `agent/turn-continuation` — override the continue/stop decision (force-continue /loop, force-stop budget guard) +- `agent/turn-continuation` — override the continue/stop decision via `ContinuationDecision` = `{action:'stop'}` or `{action:'continue', reason?}` (a `continue` `reason` is recorded as next-step steering in the same turn — the typed `/goal` pattern). Force-continue `/loop`, force-stop budget guard. + +Tool interception is the `tools/pre-execute` / `tools/post-execute` pair in [`dsh-tools`](../tools/README.md) (`PreToolDecision` allow/deny/ask, `PostToolDecision` accept/block) — same typed-Decision idiom, owned there because it is the tool registry's seam. #### Streaming + tool (emit) diff --git a/packages/core/agent/src/types.ts b/packages/core/agent/src/types.ts index 6c3f2b8226..cdc0fce538 100644 --- a/packages/core/agent/src/types.ts +++ b/packages/core/agent/src/types.ts @@ -34,6 +34,11 @@ * See `docs/rfc/implemented/architecture/2026-06-11-microkernel-event-taxonomy.md` * and the related `docs/rfc/proposed/simplification/2026-06-20-remove-agent-boundary-mirror-events.md`. * + * The interception waterfalls here (`agent/prompt-submit`, `agent/request`, + * `agent/step-result`, `agent/turn-continuation`) each return a typed Decision — + * the convention pinned by + * `docs/rfc/implemented/feature/2026-06-30-interception-seams.md`. + * * @module @deepseek-ai/dsh-agent/types */ @@ -66,6 +71,64 @@ export interface SendOptions { export type AgentStatus = 'idle' | 'running' | 'disposed' +/** + * Model-facing context an interception listener wants the agent to SEE on the + * next request — the canonical shape behind every "inject extra context" + * decision ({@link PromptDecision}, {@link PostToolDecision}, + * {@link ContinuationDecision}). It is `agent.inject()`ed as a + * `context/message`, so it carries a REQUIRED {@link MessageSource}: `inject()` + * defaults a missing source to `{kind:'user'}`, which would MISLABEL plugin + * context as a user prompt and corrupt derived history. A bridge sets + * `{kind:'plugin', plugin:'…'}`; a native plugin names itself. Required, not + * optional — the label is load-bearing, never defaulted here. + */ +export interface HookContext { + content: ContentBlock[] + source: MessageSource +} + +/** + * The decision an {@link Agent} `agent/prompt-submit` waterfall listener returns + * for ONE drained queued message, before it becomes a `user/message`. Maps onto + * the Claude Code `UserPromptSubmit` hook's allow/block + `additionalContext`. + * + * - `allow` proceeds with the prompt; optional `content` REPLACES the prompt + * bytes (a rewrite), and optional `additionalContext` is `inject()`ed as a + * separate `context/message` the next request also sees. + * - `block` drops the prompt entirely; `reason` is the durable record of why. + * A batch whose every prompt is blocked still opens a zero-step turn that ends + * with {@link TurnEndReason} `rejected` (so the boundary stays balanced and a + * UI can render "blocked by hook"). + */ +export type PromptDecision = + | { kind: 'allow'; content?: ContentBlock[]; additionalContext?: HookContext } + | { kind: 'block'; reason: string } + +/** + * The decision an {@link Agent} `agent/turn-continuation` waterfall listener + * returns. The loop computes the default (`continue` when the step had tool + * calls or steering was injected, else `stop`); listeners override it to + * force-continue (`/goal`, `/loop`) or force-stop (budget guards). + * + * A `continue` may carry a `reason`: model-facing context recorded as next-STEP + * steering within the SAME turn (the loop enqueues it through the steering + * channel, so the continued turn's next step sees it). This is the typed twin of + * the existing "steer from a step/end listener" `/goal` pattern. + */ +export type ContinuationDecision = + | { action: 'stop' } + | { action: 'continue'; reason?: HookContext } + +/** + * Why an agent's session lifecycle began, carried by `agent/session-start`. A + * bridge keys its SessionStart hook's matcher on this (Claude Code's + * `startup`/`resume`/`clear`/`compact` source set). `startup` = a fresh create + * (including a seeded/forked create — a seed is NOT a resume); `resume` = a + * persisted session reloaded via `ctx.agents.resume()`. `clear`/`compact` are + * driven by those subsystems (compact = `TODO(compaction)`). + */ +export type SessionStartSource = 'startup' | 'resume' | 'clear' | 'compact' + /** * The agent handle — the surface every plugin (UI, hooks, orchestrators) * programs against. The concrete implementation lives in @@ -183,6 +246,19 @@ declare module 'cordis' { */ 'agent/queued'(agent: Agent, content: ContentBlock[], info: { source: MessageSource; steering: boolean }): void + // ---- session lifecycle (emit) ---- + /** + * The agent's session lifecycle began, fired once before its first turn. + * `source` says why ({@link SessionStartSource}: fresh startup, a resumed + * persisted session, …). A pure NOTIFICATION (emit, not waterfall): it + * carries no veto — a session-start listener that wants to seed context does + * so via `agent.inject()` (a `context/message` the first request sees), not + * by returning a decision. Cannot block the session from starting; that gap + * is deliberate (a bridge logs/injects, it does not gate startup). + * @mode emit + */ + 'agent/session-start'(agent: Agent, source: SessionStartSource): void + // ---- turn boundaries (emit) — the live boundary surface ---- // Step boundaries are NOT mirrored here: a consumer that needs per-step // boundaries reads the durable `step/start`/`step/end` session events (the @@ -205,6 +281,16 @@ declare module 'cordis' { 'agent/turn-end'(agent: Agent, turn: number, reason: TurnEndReason): void // ---- interception seams (waterfall) ---- + /** + * Waterfall: decide what happens to ONE drained queued message before it + * becomes a `user/message` — allow (optionally rewriting the prompt bytes or + * attaching `additionalContext`) or block it. Fires inside the already-open + * turn, per drained message. Maps onto Claude Code's `UserPromptSubmit` hook. + * Call `next()` to delegate to the default (allow unchanged), or return a + * {@link PromptDecision} without calling `next()` to short-circuit. + * @mode waterfall + */ + 'agent/prompt-submit'(agent: Agent, content: ContentBlock[], source: MessageSource, next: () => Promise): Promise /** * Waterfall: mutate the fully-assembled {@link GenerateOptions} before the * model call (hooks, compaction, model switching, tool filtering, …). Call @@ -219,12 +305,15 @@ declare module 'cordis' { */ 'agent/step-result'(agent: Agent, turn: number, step: number, message: Message, next: () => Promise): Promise /** - * Waterfall: override the turn-continuation decision. The default - * (computed by the loop) is `hadToolCalls || steeringInjected`. Listeners - * can force-continue (/goal, /loop) or force-stop (budget guards). + * Waterfall: override the turn-continuation decision via a typed + * {@link ContinuationDecision}. The loop's `defaultDecision` is `continue` + * when the step had tool calls or steering was injected, else `stop`. + * Listeners force-continue (`/goal`, `/loop` — optionally attaching a + * `reason` recorded as next-step steering) or force-stop (budget guards). + * Call `next()` to delegate to the default, or return a decision to override. * @mode waterfall */ - 'agent/turn-continuation'(agent: Agent, turn: number, defaultDecision: boolean, next: () => Promise): Promise + 'agent/turn-continuation'(agent: Agent, turn: number, defaultDecision: ContinuationDecision, next: () => Promise): Promise // ---- streaming + tool notifications (emit) ---- /** diff --git a/packages/core/session/src/types.ts b/packages/core/session/src/types.ts index 581fc29df1..a4ba452193 100644 --- a/packages/core/session/src/types.ts +++ b/packages/core/session/src/types.ts @@ -134,6 +134,16 @@ export interface TurnEndReasonMap { error: { kind: 'error'; step: number; message: string; code?: string } disposed: { kind: 'disposed' } 'max-tokens': { kind: 'max-tokens' } + /** + * The turn's entire prompt batch was BLOCKED before any step ran — every + * drained queued message was vetoed by an `agent/prompt-submit` listener (a + * hook). The turn still opened (so the boundary stays balanced and the block + * is a durable in-turn fact), but ran zero steps. `reason` carries the block + * message from the vetoing decision. Distinct from `aborted` (a user-driven + * cancel) and `error` (a failure): the prompt was rejected by policy, not + * interrupted or broken. A UI renders it as "prompt blocked by hook". + */ + rejected: { kind: 'rejected'; reason: string } /** * The turn never ended on its own: the process crashed mid-turn and a * persistence backend later closed the orphaned (open) turn on reload so the diff --git a/packages/core/tools/README.md b/packages/core/tools/README.md index 6b1634cd70..e81a0fb007 100644 --- a/packages/core/tools/README.md +++ b/packages/core/tools/README.md @@ -1,6 +1,6 @@ # dsh-tools -Tool registry and execution waterfall. Tool plugins register their schemas and executors; the agent loop executes calls through the `tools/execute` waterfall. +Tool registry and execution pipeline. Tool plugins register their schemas and executors; the agent loop executes each call through `tools/pre-execute` (the allow/deny gate) → core dispatch → `tools/post-execute` (inspect/replace the result, attach context). ## Service: `ToolRegistry` (ctx key: `tools`) @@ -9,7 +9,7 @@ Tool registry and execution waterfall. Tool plugins register their schemas and e - `ctx.tools.register(definition: ToolDefinition): () => void` Register a tool. Disposed with the calling fiber. - `ctx.tools.get(name: string): ToolDefinition | undefined` - `ctx.tools.schemas(): ToolSchema[]` Schemas of all registered tools (without the `execute` functions). -- `ctx.tools.execute(exec: ToolExecution): Promise` Execute one tool call through the `tools/execute` waterfall. +- `ctx.tools.execute(exec: ToolExecution): Promise` Execute one tool call through the `tools/pre-execute` → dispatch → `tools/post-execute` pipeline. ### Injected services @@ -19,20 +19,23 @@ Tool registry and execution waterfall. Tool plugins register their schemas and e | Event | Mode | Purpose | |---|---|---| -| `tools/execute` | waterfall | Wrap/veto tool execution (sandbox, permission, hooks, plan mode) | +| `tools/pre-execute` | waterfall | Allow/deny gate BEFORE a tool runs (sandbox, permission, hooks); returns `PreToolDecision` | +| `tools/post-execute` | waterfall | Inspect/replace the result AFTER a tool runs, attach context; returns `PostToolDecision` | | `tools/change` | emit | A tool was registered or unregistered | ### Key types - `ToolDefinition` — `ToolSchema` + `execute(args, exec): Promise`, plus optional `presentCall(args)` / `presentResult(args, result)` for tool-owned UI presentation (see below). - `ToolExecution` — one pending tool call: `{ callId, name, arguments, agent?, signal? }`. -- `ToolExecutionResult` — outcome: `{ callId, content, isError, error? }`. On failure with a `HarnessError`, `error: { name, code }` carries the structured failure class alongside the model-facing text (the loop forwards it onto the `tool/result` session event for retry/sandbox plugins and replay). +- `ToolExecutionResult` — outcome: `{ callId, content, isError, error?, additionalContext? }`. On failure with a `HarnessError`, `error: { name, code }` carries the structured failure class alongside the model-facing text (the loop forwards it onto the `tool/result` session event for retry/sandbox plugins and replay). `additionalContext` (a `HookContext`) ferries any `tools/post-execute` context up to the loop, which buffers it and appends it as a `context/message` after all `tool/result`s in the step. +- `PreToolDecision` — `{kind:'allow'}` | `{kind:'deny', reason}` | `{kind:'ask', reason?}`. Input rewrite (changing `arguments`) is deliberately NOT offered (it would desync the pre-execution audit/history/UI from what ran — its own proposed RFC); `ask` degrades to `deny` until the permission system lands. +- `PostToolDecision` — `{kind:'accept', content?, additionalContext?}` (keep the call successful, optionally replacing the model-facing content) | `{kind:'block', feedback, additionalContext?}` (turn it into an `isError` whose content is the corrective feedback). Output replacement is clean because `tool/result` is logged AFTER `execute()` returns. - `ToolCallPresentation` / `ToolResultPresentation` — provider-neutral shapes a tool returns from `presentCall` / `presentResult` to own how a UI renders ITS calls (see "Tool-owned UI presentation"). ### Extension points - Tool plugins call `ctx.tools.register()` — schemas flow into the assembly automatically. -- The `tools/execute` waterfall is the single seam for sandbox, permission, hooks, and plan-mode plugins to wrap or veto a call. Listeners receive `(exec, next)`: call `next()` to proceed, or return a result without calling `next()` to short-circuit (veto). +- `tools/pre-execute` is the allow/deny gate (sandbox, permission, hooks): listeners receive `(exec, next)` and call `next()` to delegate to the default (allow) or return a `PreToolDecision` to short-circuit; a `deny`/`ask` skips dispatch and yields an `isError` result. `tools/post-execute` is the inspect/transform seam: `(exec, result, next)` → a `PostToolDecision` that can replace content, block with feedback, or attach `additionalContext`. Core dispatch sits between them as plain code; the tool body keeps its own try/catch so a thrown tool still reaches `post-execute` as an `isError`. Both follow the typed-Decision idiom shared with the `agent/*` interception seams (see [`dsh-agent`](../agent/README.md)). - MCP servers: one plugin per server, discover tools, call `ctx.tools.register()` with the server's schemas. ### Typed tool parameter schemas diff --git a/packages/core/tools/src/index.ts b/packages/core/tools/src/index.ts index 5a17aa2b0c..25f57ed400 100644 --- a/packages/core/tools/src/index.ts +++ b/packages/core/tools/src/index.ts @@ -1,8 +1,9 @@ /** - * Tool registry and execution waterfall. Plugins register tools; the registry + * Tool registry and execution pipeline. Plugins register tools; the registry * feeds schemas into the system prompt, and `execute()` dispatches each call - * through the `tools/execute` waterfall for sandbox, permission, and hook - * plugins to wrap or veto. + * through `tools/pre-execute` (the allow/deny gate) → core dispatch → + * `tools/post-execute` (inspect/replace the result, attach context) for + * sandbox, permission, and hook plugins to gate or transform a call. * * @module @deepseek-ai/dsh-tools */ @@ -10,7 +11,7 @@ import { Context, Service } from 'cordis' import type { CallId, ContentBlock, ToolSchema } from '@deepseek-ai/dsh-llm' import { HarnessError } from '@deepseek-ai/dsh-llm' -import type { Agent } from '@deepseek-ai/dsh-agent' +import type { Agent, HookContext } from '@deepseek-ai/dsh-agent' import type {} from '@deepseek-ai/dsh-system-prompt' export { @@ -33,14 +34,31 @@ declare module 'cordis' { interface Events { /** - * Waterfall around every tool execution — the single seam where sandbox, - * permission, hook, and plan-mode plugins wrap or veto a call. Listeners - * receive `(exec, next)`: call `next()` to proceed (possibly around your - * own logic), or return a {@link ToolExecutionResult} without calling - * `next()` to short-circuit (veto). + * Waterfall BEFORE a tool runs — the gate where sandbox, permission, and + * hook plugins allow or deny a call (Claude Code's `PreToolUse`). Listeners + * receive `(exec, next)`: call `next()` to delegate to the default (allow), + * or return a {@link PreToolDecision} without calling `next()` to + * short-circuit. A `deny` skips dispatch and yields an `isError` result; the + * tool body never runs. Input rewrite is deliberately NOT offered here (see + * {@link PreToolDecision}); `ask` degrades to deny until the permission + * system lands (`FIXME(permissions)`). * @mode waterfall */ - 'tools/execute'(this: ToolRegistry, exec: ToolExecution, next: () => Promise): Promise + 'tools/pre-execute'(this: ToolRegistry, exec: ToolExecution, next: () => Promise): Promise + /** + * Waterfall AFTER a tool runs — where hook plugins inspect the result and + * accept it (optionally REPLACING the model-facing content, and/or attaching + * `additionalContext` for the next request) or block it with corrective + * `feedback` (Claude Code's `PostToolUse`). Listeners receive + * `(exec, result, next)`: call `next()` to delegate to the default (accept + * unchanged), or return a {@link PostToolDecision} to override. The core tool + * dispatch sits between the two waterfalls as plain code, all inside + * `execute`'s outer try/catch (and the tool body keeps its own inner + * try/catch, so a thrown tool still reaches `post-execute` as an `isError` + * result). + * @mode waterfall + */ + 'tools/post-execute'(this: ToolRegistry, exec: ToolExecution, result: ToolExecutionResult, next: () => Promise): Promise /** * A tool was registered or unregistered (the available tool set changed). * @mode emit @@ -247,8 +265,54 @@ export interface ToolExecutionResult { * text in `content` is always present; this is extra structure for code. */ error?: ToolErrorInfo + /** + * Extra model-facing context a `tools/post-execute` listener attached for the + * NEXT request (Claude Code's PostToolUse `additionalContext`). It is NOT part + * of this call's `content` — `content`/`feedback` shape the tool RESULT, but + * `additionalContext` is a SEPARATE `context/message`. A step can carry + * multiple tool calls, so the loop BUFFERS every call's `additionalContext` + * and appends them only AFTER all `tool/result`s for the step, keeping + * tool-call/result adjacency intact. Carried on the result purely to ferry it + * from `execute()` up to the loop's per-step buffer. + */ + additionalContext?: HookContext } +/** + * The decision a `tools/pre-execute` listener returns for one pending call. + * Maps onto Claude Code's `PreToolUse` `permissionDecision`. + * + * - `allow` proceeds to dispatch. (Input rewrite — changing `exec.arguments` — + * is deliberately NOT offered: `tool/call` and `assistant/message` are logged + * BEFORE execution and live consumers, e.g. the ACP bridge and `dsh-tool-bash` + * presentation, read the pre-execution arguments, so an execution-only rewrite + * would desync the UI from what RAN. That consistency redesign is its own + * `proposed` RFC; `TODO(pre-tool-input-rewrite)` anchors it at the call site.) + * - `deny` skips dispatch; the loop records an `isError` result carrying `reason`. + * - `ask` is the permission-prompt intent; until the permission system exists it + * degrades to `deny` (`FIXME(permissions)`). + */ +export type PreToolDecision = + | { kind: 'allow' } + | { kind: 'deny'; reason: string } + | { kind: 'ask'; reason?: string } + +/** + * The decision a `tools/post-execute` listener returns for one finished call. + * Maps onto Claude Code's `PostToolUse` decision. + * + * - `accept` keeps the call successful; optional `content` REPLACES the + * model-facing result (clean: `tool/result` is logged AFTER `execute()` + * returns, so a replaced result is the single source of truth for both derived + * history and UI). Optional `additionalContext` rides to the next request. + * - `block` turns the call into an `isError` result whose content is the + * corrective `feedback` (the model is told the call was rejected and why), + * optionally also attaching `additionalContext`. + */ +export type PostToolDecision = + | { kind: 'accept'; content?: ContentBlock[]; additionalContext?: HookContext } + | { kind: 'block'; feedback: ContentBlock[]; additionalContext?: HookContext } + /** * Best-effort human-readable message from an arbitrary thrown value: Error * instances use `.message`; non-Error objects with a string `message` @@ -336,31 +400,90 @@ export class ToolRegistry extends Service { } /** - * Execute one tool call through the `tools/execute` waterfall. If the tool is - * not registered, the result is an `isError` carrying a `UNKNOWN_TOOL` - * structured error. If the tool or a waterfall listener throws, the error is - * caught and returned as an `isError` result so the loop records a failed tool - * call instead of failing the whole turn; a thrown {@link HarnessError} + * Execute one tool call through the `tools/pre-execute` → dispatch → + * `tools/post-execute` pipeline. The two waterfalls are the gate (allow/deny) + * and the inspect/transform seam; core dispatch sits between them as plain + * code. The whole thing is wrapped in one outer try/catch so a throwing + * listener (in either waterfall) becomes an `isError` result instead of + * failing the turn; the tool body ALSO keeps its own inner try/catch, so a + * thrown tool becomes an `isError` result that `post-execute` listeners can + * still inspect. If the tool is not registered, the result is an `isError` + * carrying a `UNKNOWN_TOOL` structured error. A thrown {@link HarnessError} * surfaces its `{ name, code }` on the result. */ async execute(exec: ToolExecution): Promise { try { - return await this.ctx.waterfall(this, 'tools/execute', exec, async (): Promise => { - try { - const tool = this.store.get(exec.name) - // Unknown tool routes through the same catch as a tool-thrown error, so - // both failure classes get structured `{ name, code }` from one path. - if (!tool) throw new ToolNotFoundError(exec.name) - const content = await tool.execute(exec.arguments, exec) - return { callId: exec.callId, content, isError: false } - } catch (error: unknown) { - return toolErrorResult(exec.callId, error) + // --- Gate: tools/pre-execute. A deny (or an ask, which degrades to deny + // until the permission system lands) skips dispatch entirely. --- + const decision = await this.ctx.waterfall( + this, 'tools/pre-execute', exec, + () => Promise.resolve({ kind: 'allow' }), + ) + if (decision.kind !== 'allow') { + // deny → isError. ask has no permission UI yet, so degrade to deny + // (FIXME(permissions)): a forthcoming permission system turns `ask` into + // a real prompt; today it is the conservative "not allowed". + const reason = decision.kind === 'deny' + ? decision.reason + : decision.reason ?? `tool "${exec.name}" requires approval (not yet supported)` + const denied: ToolExecutionResult = { + callId: exec.callId, + content: [{ type: 'text', text: `Error: ${reason}` }], + isError: true, } - }) + return await this.postExecute(exec, denied) + } + + // --- Core dispatch (plain code between the waterfalls). The tool body's + // own try/catch turns a throw into an isError result so post-execute can + // inspect it; an unknown tool routes through the same catch. --- + let result: ToolExecutionResult + try { + const tool = this.store.get(exec.name) + if (!tool) throw new ToolNotFoundError(exec.name) + const content = await tool.execute(exec.arguments, exec) + result = { callId: exec.callId, content, isError: false } + } catch (error: unknown) { + result = toolErrorResult(exec.callId, error) + } + + return await this.postExecute(exec, result) } catch (error: unknown) { + // Outer backstop: a throwing pre/post-execute listener (or the waterfall + // machinery) becomes an isError result, never a turn failure. return toolErrorResult(exec.callId, error) } } + + /** + * Run the `tools/post-execute` waterfall over a dispatched `result` and apply + * its {@link PostToolDecision}: `accept` keeps the call successful (replacing + * `content` when given), `block` turns it into an `isError` whose content is + * the corrective `feedback`. Either decision may attach `additionalContext`, + * which is ferried on the returned result for the loop's per-step buffer. + * Runs inside `execute`'s outer try/catch (a throwing listener → isError). + */ + private async postExecute(exec: ToolExecution, result: ToolExecutionResult): Promise { + const decision = await this.ctx.waterfall( + this, 'tools/post-execute', exec, result, + () => Promise.resolve({ kind: 'accept' }), + ) + const additionalContext = decision.additionalContext + if (decision.kind === 'block') { + return { + callId: result.callId, + content: decision.feedback, + isError: true, + ...additionalContext ? { additionalContext } : {}, + } + } + // accept: replace content if supplied, preserve the dispatched isError/error. + return { + ...result, + ...decision.content ? { content: decision.content } : {}, + ...additionalContext ? { additionalContext } : {}, + } + } } function toolErrorResult(callId: ToolExecution['callId'], error: unknown): ToolExecutionResult { diff --git a/packages/core/tools/tests/tools.spec.ts b/packages/core/tools/tests/tools.spec.ts index c88963ecc1..e67650ec95 100644 --- a/packages/core/tools/tests/tools.spec.ts +++ b/packages/core/tools/tests/tools.spec.ts @@ -4,7 +4,7 @@ import { CallId, HarnessError } from '@deepseek-ai/dsh-llm' import SystemPrompt from '@deepseek-ai/dsh-system-prompt' import ToolRegistry, { defineTool, schemaSpecToJsonSchema, validateArgs, ToolArgsError, ToolNotFoundError, - type InferArgs, type SchemaSpec, type ToolExecutionResult, + type InferArgs, type SchemaSpec, type PreToolDecision, type PostToolDecision, } from '@deepseek-ai/dsh-tools' async function setup() { @@ -112,53 +112,123 @@ describe('ToolRegistry', () => { expect(err.message).toBe('unknown tool "ghost"') }) - it('lets tools/execute waterfall listeners veto a call (permission pattern)', async () => { + it('lets a tools/pre-execute listener deny a call (permission pattern)', async () => { const ctx = await setup() ctx.tools.register(echoTool) - ctx.on('tools/execute', async (exec, next): Promise => { - if (exec.name === 'echo') { - return { - callId: exec.callId, - content: [{ type: 'text', text: 'denied by policy' }], - isError: true, - } - } + ctx.on('tools/pre-execute', async (exec, next): Promise => { + if (exec.name === 'echo') return { kind: 'deny', reason: 'denied by policy' } return next() }) const result = await ctx.tools.execute({ callId: CallId('c1'), name: 'echo', arguments: { text: 'hi' } }) expect(result.isError).toBe(true) - expect(result.content[0]).toMatchObject({ text: 'denied by policy' }) + expect(result.content[0]).toMatchObject({ text: 'Error: denied by policy' }) }) - it('composes multiple tools/execute listeners (sandbox-wrap pattern)', async () => { + it('an ask decision degrades to deny until the permission system lands', async () => { + const ctx = await setup() + ctx.tools.register(echoTool) + + ctx.on('tools/pre-execute', async (_exec, _next): Promise => + ({ kind: 'ask', reason: 'needs approval' })) + + const result = await ctx.tools.execute({ callId: CallId('c1'), name: 'echo', arguments: { text: 'hi' } }) + expect(result.isError).toBe(true) + expect(result.content[0]).toMatchObject({ text: 'Error: needs approval' }) + }) + + it('an ask decision with no reason degrades to deny with a default message', async () => { + const ctx = await setup() + ctx.tools.register(echoTool) + + ctx.on('tools/pre-execute', async (_exec, _next): Promise => ({ kind: 'ask' })) + + const result = await ctx.tools.execute({ callId: CallId('c1'), name: 'echo', arguments: { text: 'hi' } }) + expect(result.isError).toBe(true) + expect(result.content[0]).toMatchObject({ text: 'Error: tool "echo" requires approval (not yet supported)' }) + }) + + it('a tools/post-execute listener can replace the result content (accept) ', async () => { + const ctx = await setup() + ctx.tools.register(echoTool) + + ctx.on('tools/post-execute', async (_exec, _result, _next): Promise => + ({ kind: 'accept', content: [{ type: 'text', text: 'rewritten' }] })) + + const result = await ctx.tools.execute({ callId: CallId('c1'), name: 'echo', arguments: { text: 'hi' } }) + expect(result.isError).toBe(false) + expect(result.content[0]).toMatchObject({ text: 'rewritten' }) + }) + + it('a tools/post-execute block turns the call into an isError with corrective feedback', async () => { + const ctx = await setup() + ctx.tools.register(echoTool) + + ctx.on('tools/post-execute', async (_exec, _result, _next): Promise => + ({ kind: 'block', feedback: [{ type: 'text', text: 'output rejected: try again' }] })) + + const result = await ctx.tools.execute({ callId: CallId('c1'), name: 'echo', arguments: { text: 'hi' } }) + expect(result.isError).toBe(true) + expect(result.content[0]).toMatchObject({ text: 'output rejected: try again' }) + }) + + it('a block decision can ALSO attach additionalContext', async () => { + const ctx = await setup() + ctx.tools.register(echoTool) + + ctx.on('tools/post-execute', async (_exec, _result, _next): Promise => + ({ + kind: 'block', + feedback: [{ type: 'text', text: 'rejected' }], + additionalContext: { content: [{ type: 'text', text: 'why it was rejected' }], source: { kind: 'plugin', plugin: 'test' } }, + })) + + const result = await ctx.tools.execute({ callId: CallId('c1'), name: 'echo', arguments: { text: 'hi' } }) + expect(result.isError).toBe(true) + expect(result.content[0]).toMatchObject({ text: 'rejected' }) + expect(result.additionalContext).toMatchObject({ content: [{ text: 'why it was rejected' }], source: { kind: 'plugin', plugin: 'test' } }) + }) + + it('a post-execute additionalContext rides on the result for the loop to buffer', async () => { + const ctx = await setup() + ctx.tools.register(echoTool) + + ctx.on('tools/post-execute', async (_exec, _result, _next): Promise => + ({ kind: 'accept', additionalContext: { content: [{ type: 'text', text: 'fyi' }], source: { kind: 'plugin', plugin: 'test' } } })) + + const result = await ctx.tools.execute({ callId: CallId('c1'), name: 'echo', arguments: { text: 'hi' } }) + expect(result.additionalContext).toMatchObject({ content: [{ text: 'fyi' }], source: { kind: 'plugin', plugin: 'test' } }) + }) + + it('composes pre + post waterfalls around dispatch (sandbox-wrap pattern)', async () => { const ctx = await setup() ctx.tools.register(echoTool) const order: string[] = [] - ctx.on('tools/execute', async (_exec, next) => { - order.push('first:before') - const result = await next() - order.push('first:after') - return result + ctx.on('tools/pre-execute', async (_exec, next) => { + order.push('pre:before') + const decision = await next() + order.push('pre:after') + return decision }) - ctx.on('tools/execute', async (_exec, next) => { - order.push('second:before') - const result = await next() - order.push('second:after') - return result + ctx.on('tools/post-execute', async (_exec, _result, next) => { + order.push('post:before') + const decision = await next() + order.push('post:after') + return decision }) const result = await ctx.tools.execute({ callId: CallId('c1'), name: 'echo', arguments: { text: 'x' } }) expect(result.isError).toBe(false) - expect(order).toEqual(['first:before', 'second:before', 'second:after', 'first:after']) + // pre runs fully (gate) before dispatch, then post runs over the result. + expect(order).toEqual(['pre:before', 'pre:after', 'post:before', 'post:after']) }) - it('returns an isError result when a tools/execute listener throws', async () => { + it('returns an isError result when a tools/pre-execute listener throws', async () => { const ctx = await setup() ctx.tools.register(echoTool) - ctx.on('tools/execute', async () => { + ctx.on('tools/pre-execute', async () => { throw new Error('permission hook broke') }) @@ -171,10 +241,26 @@ describe('ToolRegistry', () => { }) }) - it('preserves structured error info when a tools/execute listener throws HarnessError', async () => { + it('returns an isError result when a tools/post-execute listener throws', async () => { const ctx = await setup() ctx.tools.register(echoTool) - ctx.on('tools/execute', async () => { + ctx.on('tools/post-execute', async () => { + throw new Error('post hook broke') + }) + + const result = await ctx.tools.execute({ callId: CallId('c1'), name: 'echo', arguments: { text: 'hi' } }) + + expect(result).toEqual({ + callId: CallId('c1'), + content: [{ type: 'text', text: 'Error: post hook broke' }], + isError: true, + }) + }) + + it('preserves structured error info when a tools/pre-execute listener throws HarnessError', async () => { + const ctx = await setup() + ctx.tools.register(echoTool) + ctx.on('tools/pre-execute', async () => { throw new HarnessError('denied', 'DENIED') }) diff --git a/packages/subagent/tool-subagent/tests/tool-subagent.spec.ts b/packages/subagent/tool-subagent/tests/tool-subagent.spec.ts index dda4e7c3d0..2f40cd6f8c 100644 --- a/packages/subagent/tool-subagent/tests/tool-subagent.spec.ts +++ b/packages/subagent/tool-subagent/tests/tool-subagent.spec.ts @@ -276,6 +276,14 @@ describe('dsh-tool-subagent', () => { const controller = new AbortController() const pending = callSubagent(ctx, { description: 'd', prompt: 'p' }, { signal: controller.signal }) + // Abort AFTER the tool body has had a chance to register its abort listener + // (ctx.tools.execute now awaits the tools/pre-execute waterfall before the + // body runs, so the listener is not registered synchronously). A few + // microtask turns let execute() reach `addEventListener('abort')`, so this + // exercises the LIVE onAbort bridge — distinct from the already-aborted + // sync path the next test covers. + await Promise.resolve() + await Promise.resolve() controller.abort() const result = await pending expect(cancelled).toHaveBeenCalledTimes(1) diff --git a/packages/ui/acp/src/codec.ts b/packages/ui/acp/src/codec.ts index 5f5a53f529..57622fbbfe 100644 --- a/packages/ui/acp/src/codec.ts +++ b/packages/ui/acp/src/codec.ts @@ -34,6 +34,10 @@ import type { ContentBlock as AcpContentBlock, StopReason } from '@agentclientpr * for any non-bridge caller / property test.) * - `disposed` → `cancelled` (the agent was torn down mid-turn — closest to a * cancellation from the client's perspective) + * - `rejected` → `cancelled` (the prompt was blocked by an `agent/prompt-submit` + * hook before any step ran — ACP has no "rejected" reason, and a + * blocked prompt is, from the client's view, the prompt not being + * carried out; `cancelled` is the closest legal wire reason) */ export function turnEndToStopReason(reason: TurnEndReason): StopReason { switch (reason.kind) { @@ -45,6 +49,8 @@ export function turnEndToStopReason(reason: TurnEndReason): StopReason { return 'cancelled' case 'disposed': return 'cancelled' + case 'rejected': + return 'cancelled' case 'error': return 'end_turn' // Merge-extensible: an unknown future TurnEndReason kind still has to diff --git a/packages/ui/acp/tests/codec.spec.ts b/packages/ui/acp/tests/codec.spec.ts index 9d82fe7533..859e8d40cd 100644 --- a/packages/ui/acp/tests/codec.spec.ts +++ b/packages/ui/acp/tests/codec.spec.ts @@ -16,6 +16,7 @@ describe('turnEndToStopReason', () => { expect(turnEndToStopReason({ kind: 'max-tokens' })).toBe('max_tokens') expect(turnEndToStopReason({ kind: 'aborted', reason: 'x' })).toBe('cancelled') expect(turnEndToStopReason({ kind: 'disposed' })).toBe('cancelled') + expect(turnEndToStopReason({ kind: 'rejected', reason: 'blocked by hook' })).toBe('cancelled') expect(turnEndToStopReason({ kind: 'error', step: 1, message: 'boom' })).toBe('end_turn') }) diff --git a/scripts/type-equiv.manifest.json b/scripts/type-equiv.manifest.json index c5e4c10b71..a1d31973ac 100644 --- a/scripts/type-equiv.manifest.json +++ b/scripts/type-equiv.manifest.json @@ -10,6 +10,10 @@ { "doc": "docs/core-data-structures/core.md", "symbol": "ToolSchema", "source": "packages/llm/llm/src/types.ts" }, { "doc": "docs/core-data-structures/core.md", "symbol": "SessionEvent", "source": "packages/core/session/src/types.ts" }, { "doc": "docs/core-data-structures/core.md", "symbol": "Agent", "source": "packages/core/agent/src/types.ts" }, + { "doc": "docs/core-data-structures/core.md", "symbol": "HookContext", "source": "packages/core/agent/src/types.ts" }, + { "doc": "docs/core-data-structures/core.md", "symbol": "PromptDecision", "source": "packages/core/agent/src/types.ts" }, + { "doc": "docs/core-data-structures/core.md", "symbol": "ContinuationDecision", "source": "packages/core/agent/src/types.ts" }, + { "doc": "docs/core-data-structures/core.md", "symbol": "SessionStartSource", "source": "packages/core/agent/src/types.ts" }, { "doc": "docs/core-data-structures/llm-streaming.md", "symbol": "StreamChunk", "source": "packages/llm/llm/src/types.ts" }, { "doc": "docs/core-data-structures/llm-streaming.md", "symbol": "TokenUsage", "source": "packages/llm/llm/src/types.ts" }, @@ -34,6 +38,8 @@ { "doc": "docs/core-data-structures/tools.md", "symbol": "InferArgs", "source": "packages/core/tools/src/schema.ts" }, { "doc": "docs/core-data-structures/tools.md", "symbol": "ToolExecution", "source": "packages/core/tools/src/index.ts" }, { "doc": "docs/core-data-structures/tools.md", "symbol": "ToolExecutionResult", "source": "packages/core/tools/src/index.ts" }, + { "doc": "docs/core-data-structures/tools.md", "symbol": "PreToolDecision", "source": "packages/core/tools/src/index.ts" }, + { "doc": "docs/core-data-structures/tools.md", "symbol": "PostToolDecision", "source": "packages/core/tools/src/index.ts" }, { "doc": "docs/core-data-structures/bash.md", "symbol": "BashExecRequest", "source": "packages/bash/bash/src/types.ts" }, { "doc": "docs/core-data-structures/bash.md", "symbol": "BashExecSpec", "source": "packages/bash/bash/src/types.ts" }, From 483e0e5edf5fcd965afeba0cb61d2819fb6dcad2 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Tue, 30 Jun 2026 20:25:30 +0800 Subject: [PATCH 2/6] =?UTF-8?q?fix(events):=20address=20Codex=20review=20?= =?UTF-8?q?=E2=80=94=20protect=20post-execute=20result,=20purge=20stale=20?= =?UTF-8?q?tools/execute=20refs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex's PR-C review found two (A) blockers: - tools/post-execute could corrupt the protected outcome. postExecute passed the mutable `result` to listeners and then read result.callId / spread result on the return paths, so a listener mutating the reference (flipping isError, rewriting callId, injecting an error) escaped the decision channel. Now the authoritative callId/isError/error are SNAPSHOT before the waterfall and the return value is rebuilt from the snapshot + the typed PostToolDecision — the decision is the only sanctioned way to change the outcome, and callId is always exec.callId. Added a regression test that mutates the result reference and asserts it has no effect; proven to fail red on the unfixed code. - Public docs/JSDoc still advertised the removed `tools/execute` waterfall after the split. Swept every current-state reference to tools/pre-execute + tools/post-execute: the ToolRegistry class JSDoc (and the regenerated catalog), loop.ts's ASCII flow (also added the prompt-submit/session-start steps it was missing), the package-map READMEs (packages, core, agent-core), core-data-structures core.md/tools.md, the bash + acp + invariants src/READMEs (the deferred permission gate is the tools/pre-execute deny/ask seam now), the cookbook, and the implemented RFCs whose factual seam catalog drifted. codec.ts's totality prose now lists `rejected`. Proposed-RFC references are left as-is (frozen proposals, validated when built). --- docs/cookbook/adding-a-tool.md | 2 +- docs/cordis-catalog/events-and-services.md | 4 ++-- docs/core-data-structures/core.md | 2 +- docs/core-data-structures/tools.md | 2 +- ...06-11-dev-invariants-over-deep-readonly.md | 2 +- .../2026-06-11-microkernel-event-taxonomy.md | 2 +- .../2026-06-13-capability-seams.md | 2 +- .../2026-06-21-subagent-capability-seam.md | 2 +- packages/README.md | 2 +- packages/bash/bash-local/README.md | 2 +- packages/bash/bash-local/src/index.ts | 4 ++-- packages/bash/tool-bash/README.md | 2 +- packages/bash/tool-bash/src/index.ts | 2 +- packages/core/README.md | 2 +- packages/core/agent-core/README.md | 2 +- packages/core/agent-loop/src/loop.ts | 17 +++++++++---- packages/core/tools/src/index.ts | 22 +++++++++++++---- packages/core/tools/tests/tools.spec.ts | 24 +++++++++++++++++++ packages/support/invariants/README.md | 2 +- packages/support/invariants/src/index.ts | 4 ++-- packages/ui/acp/README.md | 2 +- packages/ui/acp/src/codec.ts | 3 ++- packages/ui/acp/src/index.ts | 2 +- 23 files changed, 78 insertions(+), 32 deletions(-) diff --git a/docs/cookbook/adding-a-tool.md b/docs/cookbook/adding-a-tool.md index 73706c84f1..53906e3a49 100644 --- a/docs/cookbook/adding-a-tool.md +++ b/docs/cookbook/adding-a-tool.md @@ -46,7 +46,7 @@ Follow tool-bash's background pattern: a `run_in_background` flag returns a task ## Permissions / sandboxing -Prefer not to build policy into the tool. The seam is the `tools/execute` waterfall (veto or wrap — see the permission-gate example in [extension-cookbook.md](./extension-cookbook.md)), or a sandboxing implementation behind the tool's executor seam. +Prefer not to build policy into the tool. The seam is the `tools/pre-execute` gate (deny/ask — see the permission-gate example in [extension-cookbook.md](./extension-cookbook.md)) and the `tools/post-execute` inspect/transform seam, or a sandboxing implementation behind the tool's executor seam. ## Tests every tool needs diff --git a/docs/cordis-catalog/events-and-services.md b/docs/cordis-catalog/events-and-services.md index d37d2d4070..a8afdbd5f1 100644 --- a/docs/cordis-catalog/events-and-services.md +++ b/docs/cordis-catalog/events-and-services.md @@ -471,7 +471,7 @@ Source: [`packages/core/system-prompt/src/index.ts:71`](../../packages/core/syst ### `ctx.tools` — `ToolRegistry` -Tool registry (`ctx.tools`): tool plugins register definitions; the agent loop executes calls through the `tools/execute` waterfall. The registry contributes its schemas into the system-prompt assembly. +Tool registry (`ctx.tools`): tool plugins register definitions; the agent loop executes calls through the `tools/pre-execute` → dispatch → `tools/post-execute` pipeline. The registry contributes its schemas into the system-prompt assembly. ```ts cordis-catalog register(definition: ToolDefinition): () => void @@ -482,7 +482,7 @@ async execute(exec: ToolExecution): Promise Types: [ToolDefinition](../core-data-structures/tools.md) · [ToolExecution](../core-data-structures/tools.md) · [ToolExecutionResult](../core-data-structures/tools.md) -Source: [`packages/core/tools/src/index.ts:341`](../../packages/core/tools/src/index.ts) +Source: [`packages/core/tools/src/index.ts:342`](../../packages/core/tools/src/index.ts) ## Inherited tier (cordis core + loader/hmr/timer) diff --git a/docs/core-data-structures/core.md b/docs/core-data-structures/core.md index 78fc36edd9..2a430cebab 100644 --- a/docs/core-data-structures/core.md +++ b/docs/core-data-structures/core.md @@ -18,7 +18,7 @@ Everything else is documented on a **sub-page**, not here. The rule that draws t | [llm-streaming.md](llm-streaming.md) | the `StreamChunk` wire protocol + adapter contract, `BlockAssembler`, the `LlmAdapter` seam | | [session.md](session.md) | the full `SessionEventMap` variant catalog, `TurnTrigger`/`TurnEndReason`, `deriveMessages()`, the turn-enclosure invariant | | [persistence.md](persistence.md) | the durability seam: `SessionPersistence`, JSONL + SQLite backends, `session/flush`, crash recovery, `SessionHeader` | -| [tools.md](tools.md) | `ToolDefinition` full fields, the schema DSL, `ToolExecution`/`ToolResult`, tool-presentation UI types, the `tools/execute` waterfall | +| [tools.md](tools.md) | `ToolDefinition` full fields, the schema DSL, `ToolExecution`/`ToolResult`, tool-presentation UI types, the `tools/pre-execute`/`tools/post-execute` pipeline | | [bash.md](bash.md) | the bash executor seam: `BashExecRequest`/`Spec`, `BashRunResult`, background `BashTask`s | | [compaction.md](compaction.md) | the compaction seam: the `compact/*` session events, `CompactionResult`, the `CompactService` interface | | [subagent.md](subagent.md) | the subagent seam: the named-provider registry, `SubagentStartRequest`/`Result`/`Run`, the start-time-vs-runtime capability split | diff --git a/docs/core-data-structures/tools.md b/docs/core-data-structures/tools.md index 4d6306de14..e5a404803b 100644 --- a/docs/core-data-structures/tools.md +++ b/docs/core-data-structures/tools.md @@ -71,7 +71,7 @@ type InferArgs = Simplify< `defineTool({ name, description, parameters, execute, … })` ties it together: `parameters` is a `SchemaSpec`, `execute(args, exec)` gets `args: InferArgs`, and the helper converts the spec to JSON Schema (`schemaSpecToJsonSchema`) for the wire and validates model-generated args (`validateArgs`) before the typed body runs. A mismatch throws `ToolArgsError` (`code: 'INVALID_ARGS'`), which the registry turns into an `isError` result so the model can self-correct. Why a custom DSL and not schemastery: tool parameters need JSON Schema (the LLM wire format), not validation/transformation — the lightweight DSL gives the best authoring DX with the smallest surface. -## Execution: the `tools/execute` waterfall shapes +## Execution: the `tools/pre-execute` / `tools/post-execute` pipeline shapes `ctx.tools.execute()` runs each call through a two-waterfall pipeline — `tools/pre-execute` (the allow/deny/ask gate) → core dispatch → `tools/post-execute` (inspect/replace the result, attach context) — the seams where sandbox, permission, hook, and plan-mode plugins gate or transform a call. The pending call is a `ToolExecution`; the outcome is a `ToolExecutionResult`. diff --git a/docs/rfc/implemented/architecture/2026-06-11-dev-invariants-over-deep-readonly.md b/docs/rfc/implemented/architecture/2026-06-11-dev-invariants-over-deep-readonly.md index b9a182a4e9..212d2e82b7 100644 --- a/docs/rfc/implemented/architecture/2026-06-11-dev-invariants-over-deep-readonly.md +++ b/docs/rfc/implemented/architecture/2026-06-11-dev-invariants-over-deep-readonly.md @@ -17,7 +17,7 @@ Reject the pervasive `DeepReadonly` type flip. Instead: 1. **Always-on:** `deriveMessages()` deep-clones the content it emits (one `structuredClone` per derived message). In-flight mutation of a request can no longer reach the log — this is the real fix, and it costs nothing meaningful next to a model call. 2. **Dev-mode:** a new `dsh-invariants` plugin (pure listeners, off in production, on in tests and demos) asserts the event contract and `Object.freeze`s logged event data so any *other* code that mutates a logged event throws instead of corrupting silently. Seeded sessions are frozen and checked on `session/created` (the constructor copies the seed without emitting `session/event`). -The invariants encode the *real* contract, not an idealized one: a `tool/call` may have no `tool/result` (a thrown `tools/execute` waterfall ends the step), and both `idle→disposed` and `running→disposed` are legal. +The invariants encode the *real* contract, not an idealized one: a `tool/call` may have no `tool/result` (a thrown tool-execution pipeline step ends the turn), and both `idle→disposed` and `running→disposed` are legal. `DeepReadonly` was rejected because it is compile-time only (a plugin casts straight through it), high type-noise across every log/message consumer and adapter, and would force readonly types through code where mutation is the sanctioned API. The clone draws the mutable/immutable boundary exactly at "logged vs in-flight" without any of that noise. diff --git a/docs/rfc/implemented/architecture/2026-06-11-microkernel-event-taxonomy.md b/docs/rfc/implemented/architecture/2026-06-11-microkernel-event-taxonomy.md index c8869c0616..c1a7aba68f 100644 --- a/docs/rfc/implemented/architecture/2026-06-11-microkernel-event-taxonomy.md +++ b/docs/rfc/implemented/architecture/2026-06-11-microkernel-event-taxonomy.md @@ -12,7 +12,7 @@ The product principle (see the 微内核Harness实现思路 design doc) is "ever Pure Cordis event taxonomy. The loop's extension seams are typed events with deliberate dispatch modes: -- **waterfall** (around-middleware) where plugins mutate or veto: `agent/request`, `agent/step-result`, `agent/turn-continuation`, `tools/execute`, `llm/stream`, `system-prompt/assemble`. +- **waterfall** (around-middleware) where plugins mutate or veto: `agent/prompt-submit`, `agent/request`, `agent/step-result`, `agent/turn-continuation`, `tools/pre-execute`, `tools/post-execute`, `llm/stream`, `system-prompt/assemble`. - **emit** (sync fire-and-forget) for notifications: turn/step boundaries, stream chunks, lifecycle, errors. - **parallel** (awaited) for the one durability checkpoint: `session/flush`. diff --git a/docs/rfc/implemented/architecture/2026-06-13-capability-seams.md b/docs/rfc/implemented/architecture/2026-06-13-capability-seams.md index e66440c3ee..46bfe88d50 100644 --- a/docs/rfc/implemented/architecture/2026-06-13-capability-seams.md +++ b/docs/rfc/implemented/architecture/2026-06-13-capability-seams.md @@ -20,7 +20,7 @@ A swappable capability is **three packages**: Implementation and consumer then evolve independently: a sandboxed executor replaces `dsh-bash-local` without touching a tool schema. -Alternatives considered: **one combined package** — rejected because it recouples the three rates of change the split exists to separate (the whole point). **`@cordisjs/plugin-capability`** — a different axis entirely: it is a permission/capability-*security* service (named permissions with inheritance, tested against a session via `ctx.capability.test`), a candidate for the deferred permissions/sandbox work on the `tools/execute` veto seam, NOT a mechanism for swapping implementations. Confusing the two ("capability") is the trap this RFC names. +Alternatives considered: **one combined package** — rejected because it recouples the three rates of change the split exists to separate (the whole point). **`@cordisjs/plugin-capability`** — a different axis entirely: it is a permission/capability-*security* service (named permissions with inheritance, tested against a session via `ctx.capability.test`), a candidate for the deferred permissions/sandbox work on the `tools/pre-execute` deny/ask seam, NOT a mechanism for swapping implementations. Confusing the two ("capability") is the trap this RFC names. The split is not mandatory when the parts are genuinely one concern: the LLM seam folds interface + consumer into `dsh-llm` (the consumer is the loop itself, not a swappable schema surface) with adapters as the implementation packages. Don't split preemptively — a capability with one conceivable implementation and one consumer stays one package until a second appears. diff --git a/docs/rfc/implemented/feature/2026-06-21-subagent-capability-seam.md b/docs/rfc/implemented/feature/2026-06-21-subagent-capability-seam.md index f733daca7b..77a838b299 100644 --- a/docs/rfc/implemented/feature/2026-06-21-subagent-capability-seam.md +++ b/docs/rfc/implemented/feature/2026-06-21-subagent-capability-seam.md @@ -66,7 +66,7 @@ The `dsh-tool-subagent` consumer awaits `run.result` and returns the child's fin ## Risks and deferrals -- **Recursion.** Without a guard, an in-process child inherits the spawn tool and can spawn unboundedly. Depth-limit is an optional capability (the in-process backends enforce it; ACP advertises it off and rejects a `maxDepth` request); tool-filtering is likewise optional. Tool-filtering, when implemented, needs a `tools/execute` veto in the child context — schema filtering alone is insufficient because a model can hallucinate a denied tool name. +- **Recursion.** Without a guard, an in-process child inherits the spawn tool and can spawn unboundedly. Depth-limit is an optional capability (the in-process backends enforce it; ACP advertises it off and rejects a `maxDepth` request); tool-filtering is likewise optional. Tool-filtering, when implemented, needs a `tools/pre-execute` deny in the child context — schema filtering alone is insufficient because a model can hallucinate a denied tool name. - **Blocking the parent turn.** Synchronous collect holds the parent's `runStep` open for the child's full duration. This is acceptable for the first cut; **background / poll / spill semantics are deferred to a future redesign that unifies long-running-tool handling across subagents AND bash** (a sub-agent and a long `bash` background task pose the same "the model started something slow, how does it collect later" problem, and should share one mechanism rather than each inventing its own). - **Live progress.** This cut surfaces only lifecycle + final result; a per-chunk child→parent update stream is deferred with the background redesign. - **ACP client surface.** Proxying `fs`/`terminal` from the ACP child back to the parent (a shared-workspace mode) is future work; the first cut advertises neither, so the child self-serves in its own process. diff --git a/packages/README.md b/packages/README.md index 3997fd5190..7f63e305dc 100644 --- a/packages/README.md +++ b/packages/README.md @@ -62,7 +62,7 @@ The rule: **extension** plugins depend on interfaces, never on the concrete loop | `llm/` | `llm` | Abstract LLM service + content-block vocabulary + chunk assembler | `ctx.llm` | | `session/` | `core` | Event-sourced session log + in-memory store | `ctx.sessions` | | `system-prompt/` | `core` | Prompt-section + tool-schema assembly registry | `ctx.systemPrompt` | -| `tools/` | `core` | Tool registry + `tools/execute` waterfall | `ctx.tools` | +| `tools/` | `core` | Tool registry + `tools/pre-execute`/`tools/post-execute` pipeline | `ctx.tools` | | `agent/` | `core` | Agent interface, registry, `agent/*` event vocabulary | `ctx.agents` | | `agent-loop/` | `core` | THE concrete loop plugin: `ReactLoopAgent` + the loop driver | `ctx.agentLoop` | | `agent-core/` | `core` | Bundle plugin: the providerless/executor-less/UI-less spine as code (forwards `agent-loop`'s `agents`) | (loads the spine) | diff --git a/packages/bash/bash-local/README.md b/packages/bash/bash-local/README.md index 2ae905b628..9625442f68 100644 --- a/packages/bash/bash-local/README.md +++ b/packages/bash/bash-local/README.md @@ -26,4 +26,4 @@ Design surveyed against the bash tools of Claude Code, OpenCode, Codex, and pi; ## Sandboxing -`TODO(permissions/sandbox)`: execution policy does NOT belong in this package. Wrap the `tools/execute` waterfall (veto/ask) or implement a sandboxing `BashExecutor` — see docs/architecture.md § plugin checklist. Reference points: Claude Code wraps commands in sandbox-exec/bubblewrap; Codex applies seatbelt/landlock plus an execpolicy prefix-rule engine. +`TODO(permissions/sandbox)`: execution policy does NOT belong in this package. Use the `tools/pre-execute` deny/ask gate or implement a sandboxing `BashExecutor` — see docs/architecture.md § plugin checklist. Reference points: Claude Code wraps commands in sandbox-exec/bubblewrap; Codex applies seatbelt/landlock plus an execpolicy prefix-rule engine. diff --git a/packages/bash/bash-local/src/index.ts b/packages/bash/bash-local/src/index.ts index 53c369a24c..8b9cf04e7d 100644 --- a/packages/bash/bash-local/src/index.ts +++ b/packages/bash/bash-local/src/index.ts @@ -4,8 +4,8 @@ * own process group (see `./run.ts` for the plumbing and the agent-tool * survey notes), tracks background tasks, and kills everything on dispose. * - * TODO(permissions/sandbox): execution policy does NOT belong here — wrap - * the `tools/execute` waterfall (see docs/architecture.md § plugin + * TODO(permissions/sandbox): execution policy does NOT belong here — use + * the `tools/pre-execute` deny/ask gate (see docs/architecture.md § plugin * checklist) or implement a sandboxing `BashExecutor`. Reference points: * Claude Code wraps commands in sandbox-exec/bubblewrap; Codex applies * seatbelt/landlock plus an execpolicy prefix-rule engine. diff --git a/packages/bash/tool-bash/README.md b/packages/bash/tool-bash/README.md index f7a15894f9..893a989e55 100644 --- a/packages/bash/tool-bash/README.md +++ b/packages/bash/tool-bash/README.md @@ -46,4 +46,4 @@ The `BashExecRequest` seam carries optional `stdin` and `env` (a **trusted-plugi ## Permissions -`TODO(permissions)`: commands run with the executor's full authority. The permission/sandbox seam is the `tools/execute` waterfall (veto or ask) plus sandboxing `BashExecutor` implementations — see docs/architecture.md. `@cordisjs/plugin-capability` (a named-permission service with a session `test()`) is a candidate building block for that work. +`TODO(permissions)`: commands run with the executor's full authority. The permission/sandbox seam is the `tools/pre-execute` waterfall (deny or ask) plus sandboxing `BashExecutor` implementations — see docs/architecture.md. `@cordisjs/plugin-capability` (a named-permission service with a session `test()`) is a candidate building block for that work. diff --git a/packages/bash/tool-bash/src/index.ts b/packages/bash/tool-bash/src/index.ts index 9ad1f9a17c..492bd28d3c 100644 --- a/packages/bash/tool-bash/src/index.ts +++ b/packages/bash/tool-bash/src/index.ts @@ -31,7 +31,7 @@ * pre-existing reload-gap drop — but the ownership fence itself is HMR-proof.) * * TODO(permissions): commands run with the executor's full authority. The - * permission/sandbox seam is the `tools/execute` waterfall (veto/ask) plus + * permission/sandbox seam is the `tools/pre-execute` waterfall (deny/ask) plus * sandboxing `BashExecutor` implementations — see docs/architecture.md * § plugin checklist. * diff --git a/packages/core/README.md b/packages/core/README.md index 8d8805471a..7030dcc0eb 100644 --- a/packages/core/README.md +++ b/packages/core/README.md @@ -6,7 +6,7 @@ The packages every harness build is assembled from: the session log, the system- |---|---|---| | `session/` | Event-sourced session log + in-memory store | `ctx.sessions` | | `system-prompt/` | Prompt-section + tool-schema assembly registry | `ctx.systemPrompt` | -| `tools/` | Tool registry + `tools/execute` waterfall | `ctx.tools` | +| `tools/` | Tool registry + `tools/pre-execute`/`tools/post-execute` pipeline | `ctx.tools` | | `agent/` | Agent interface, registry, `agent/*` event vocabulary | `ctx.agents` | | `agent-loop/` | The concrete loop plugin: `ReactLoopAgent` + the loop driver | `ctx.agentLoop` | | `agent-core/` | Bundle plugin: the providerless/executor-less/UI-less spine as code | (loads the spine) | diff --git a/packages/core/agent-core/README.md b/packages/core/agent-core/README.md index 28a3592ac6..022ccba4f4 100644 --- a/packages/core/agent-core/README.md +++ b/packages/core/agent-core/README.md @@ -13,7 +13,7 @@ This is the package to read to see **the whole plugin tree at once** — the tea @deepseek-ai/dsh-llm abstract LLM service + content-block vocabulary @deepseek-ai/dsh-session event-sourced session log + store @deepseek-ai/dsh-system-prompt prompt-section + tool-schema assembly -@deepseek-ai/dsh-tools tool registry + tools/execute waterfall +@deepseek-ai/dsh-tools tool registry + tools/pre-execute/post-execute @deepseek-ai/dsh-agent agent registry + agent/* event vocabulary @deepseek-ai/dsh-invariants dev-mode event-contract assertions @deepseek-ai/dsh-tool-bash the model-facing bash/bash_output/bash_kill schemas diff --git a/packages/core/agent-loop/src/loop.ts b/packages/core/agent-loop/src/loop.ts index 6ac98743a2..735db7c20a 100644 --- a/packages/core/agent-loop/src/loop.ts +++ b/packages/core/agent-loop/src/loop.ts @@ -142,10 +142,13 @@ export interface LoopHandle { * The agent loop. One invocation drives one agent for its whole lifetime: * * ``` + * create agent → emit agent/session-start(source) ⟵ once, before turn 1 * forever: * wait for queued messages (idle) * TURN (error-contained — a throwing plugin ends the turn, never the loop): - * drain queued → 'turn/start' → session('user/message'…) → emit agent/turn-start + * 'turn/start'; each queued msg: waterfall agent/prompt-submit + * allow → session('user/message'…) (+ inject additionalContext) | block → drop + * every prompt blocked → 'turn/end'(rejected), 0 steps; emit agent/turn-start * STEP loop: * drain steering → session('steering/message') ⟵ catches late steering * session('step/start') ⟵ durable step boundary (no agent/* mirror) @@ -157,13 +160,17 @@ export interface LoopHandle { * msg = waterfall agent/step-result ⟵ BEFORE the log append, so the * session('assistant/message' {content, usage?}) session records what actually ran * each tool-call in msg (sequential, abort-checked): - * session('tool/call'); ctx.tools.execute() ⟵ waterfall tools/execute + * session('tool/call'); ctx.tools.execute() ⟵ tools/pre-execute (allow/deny/ask) + * → dispatch → tools/post-execute * session('tool/result') + * append buffered post-execute additionalContext → session('context/message')(s) * drain steering → session('steering/message'); emit agent/steering * session('step/end') ⟵ durable step boundary (no agent/* mirror) - * cont = waterfall agent/turn-continuation(default = hadToolCalls || steered) - * if !cont && steering arrived from step/end session-event/continuation listeners: cont = true - * if !cont: break + * cont = waterfall agent/turn-continuation ⟵ ContinuationDecision; default + * {action: hadToolCalls||steered ? 'continue':'stop'}; a continue.reason is + * recorded as next-step steering + * if action==stop && steering arrived (step/end/continuation listeners): continue anyway + * if action==stop: break * session('turn/end'); emit agent/turn-end * await ctx.parallel('session/flush', session) ⟵ durability checkpoint * re-enqueue leftover steering as queued ⟵ steering is never stranded diff --git a/packages/core/tools/src/index.ts b/packages/core/tools/src/index.ts index 25f57ed400..a8f881c214 100644 --- a/packages/core/tools/src/index.ts +++ b/packages/core/tools/src/index.ts @@ -335,8 +335,9 @@ function errorInfo(error: unknown): ToolErrorInfo | undefined { /** * Tool registry (`ctx.tools`): tool plugins register definitions; the agent - * loop executes calls through the `tools/execute` waterfall. The registry - * contributes its schemas into the system-prompt assembly. + * loop executes calls through the `tools/pre-execute` → dispatch → + * `tools/post-execute` pipeline. The registry contributes its schemas into the + * system-prompt assembly. */ export class ToolRegistry extends Service { static inject = ['systemPrompt'] @@ -464,6 +465,19 @@ export class ToolRegistry extends Service { * Runs inside `execute`'s outer try/catch (a throwing listener → isError). */ private async postExecute(exec: ToolExecution, result: ToolExecutionResult): Promise { + // Snapshot the protected outcome BEFORE the waterfall. A listener receives + // the same `result` reference, so a post-waterfall read of `result.callId`/ + // `.isError`/`.error` could carry a listener's mutation — violating the + // authoritative-call-id requirement and the "preserve the dispatched + // isError/error" contract. The decision is the ONLY sanctioned channel for a + // listener to change the outcome (block, or accept-with-replacement); the + // call id is always the authoritative `exec.callId`. + const dispatched = { + callId: exec.callId, + content: result.content, + isError: result.isError, + ...result.error ? { error: result.error } : {}, + } const decision = await this.ctx.waterfall( this, 'tools/post-execute', exec, result, () => Promise.resolve({ kind: 'accept' }), @@ -471,7 +485,7 @@ export class ToolRegistry extends Service { const additionalContext = decision.additionalContext if (decision.kind === 'block') { return { - callId: result.callId, + callId: dispatched.callId, content: decision.feedback, isError: true, ...additionalContext ? { additionalContext } : {}, @@ -479,7 +493,7 @@ export class ToolRegistry extends Service { } // accept: replace content if supplied, preserve the dispatched isError/error. return { - ...result, + ...dispatched, ...decision.content ? { content: decision.content } : {}, ...additionalContext ? { additionalContext } : {}, } diff --git a/packages/core/tools/tests/tools.spec.ts b/packages/core/tools/tests/tools.spec.ts index e67650ec95..ef67eddb95 100644 --- a/packages/core/tools/tests/tools.spec.ts +++ b/packages/core/tools/tests/tools.spec.ts @@ -201,6 +201,30 @@ describe('ToolRegistry', () => { expect(result.additionalContext).toMatchObject({ content: [{ text: 'fyi' }], source: { kind: 'plugin', plugin: 'test' } }) }) + it('a post-execute listener mutating the result object cannot corrupt callId/isError/error', async () => { + // The decision is the ONLY sanctioned channel to change the outcome. A + // listener that reaches in and mutates the passed result reference (flipping + // isError, rewriting callId, attaching a bogus error) must NOT affect what + // execute() returns — the registry snapshots the authoritative fields before + // the waterfall and rebuilds from the snapshot + decision. + const ctx = await setup() + ctx.tools.register(echoTool) + + ctx.on('tools/post-execute', async (_exec, result, next) => { + const mutable = result as { callId: string; isError: boolean; error?: unknown } + mutable.callId = 'hijacked' + mutable.isError = true + mutable.error = { name: 'Evil', code: 'EVIL' } + return next() // delegate to the default accept — no decision-level override + }) + + const result = await ctx.tools.execute({ callId: CallId('c1'), name: 'echo', arguments: { text: 'hi' } }) + expect(result.callId).toBe(CallId('c1')) // authoritative exec.callId, not 'hijacked' + expect(result.isError).toBe(false) // the real (successful) dispatch outcome + expect(result.error).toBeUndefined() // no listener-injected error + expect(result.content[0]).toMatchObject({ text: 'hi' }) + }) + it('composes pre + post waterfalls around dispatch (sandbox-wrap pattern)', async () => { const ctx = await setup() ctx.tools.register(echoTool) diff --git a/packages/support/invariants/README.md b/packages/support/invariants/README.md index a08ccf01a7..b26170d48e 100644 --- a/packages/support/invariants/README.md +++ b/packages/support/invariants/README.md @@ -34,7 +34,7 @@ Session log (per session): - **turns pair and nest** — `turn/start` opens a turn, `turn/end` closes the matching one; no overlapping turns. - **steps nest in turns** — `step/start` opens a step in the open turn; `step/end` closes the matching step. - **chunks belong to an open step** — `step/start` precedes its `assistant/chunk`s. -- **a `tool/result` needs a prior `tool/call`** — but NOT the converse: a `tool/call` may have no result (a thrown `tools/execute` waterfall ends the step with no `tool/result`, which is legal). +- **a `tool/result` needs a prior `tool/call`** — but NOT the converse: a `tool/call` may have no result (a thrown tool-execution pipeline step ends the turn with no `tool/result`, which is legal). Agent status (per agent): diff --git a/packages/support/invariants/src/index.ts b/packages/support/invariants/src/index.ts index b2372e28db..5e3fd26b82 100644 --- a/packages/support/invariants/src/index.ts +++ b/packages/support/invariants/src/index.ts @@ -259,8 +259,8 @@ function checkEvent(trace: SessionTrace, event: SessionEvent): void { case 'tool/result': { requireOpenStep(trace, 'tool/result', event.data.turn, event.data.step) // A result needs a prior matching call in the same step. (The converse - // does NOT hold: a call may have no result — a throwing tools/execute - // waterfall ends the step with no tool/result, which is legal.) + // does NOT hold: a call may have no result — a throwing tool-execution + // pipeline step ends the turn with no tool/result, which is legal.) const syntheticInterrupted = event.data.isError && event.data.error?.code === 'interrupted' if (!trace.pendingCalls.delete(event.data.callId) && !syntheticInterrupted) { throw new InvariantError(`tool/result for ${event.data.callId} with no prior tool/call in this step`) diff --git a/packages/ui/acp/README.md b/packages/ui/acp/README.md index ad50383542..524a873467 100644 --- a/packages/ui/acp/README.md +++ b/packages/ui/acp/README.md @@ -65,7 +65,7 @@ Teardown reaches quiescence: for EVERY live session settle any pending prompt as ## Known limitations (tracked TODOs) -- **`TODO(rfc010-permission-gate)`** — the `tools/execute` permission gate (`session/request_permission`) is NOT implemented; tools run with the executor's full authority. The `agent→sessionId` reverse map is in place so the gate can route a permission request (which receives only `exec.agent`) back to its originating session. [ACP support](../../../docs/rfc/proposed/feature/2026-06-14-acp-agent-client-protocol.md) and [ACP multi-session](../../../docs/rfc/proposed/feature/2026-06-14-acp-multi-session.md) stay `proposed` until the gate (and per-session permission ownership) land. +- **`TODO(rfc010-permission-gate)`** — the `tools/pre-execute` permission gate (`session/request_permission`) is NOT implemented; tools run with the executor's full authority. The `agent→sessionId` reverse map is in place so the gate can route a permission request (which receives only `exec.agent`) back to its originating session. [ACP support](../../../docs/rfc/proposed/feature/2026-06-14-acp-agent-client-protocol.md) and [ACP multi-session](../../../docs/rfc/proposed/feature/2026-06-14-acp-multi-session.md) stay `proposed` until the gate (and per-session permission ownership) land. - **`additionalDirectories`** — rejected. A session operates in its single `cwd` (see Per-session cwd); widening the tool/filesystem scope to extra roots is a separate sandbox concern, not yet implemented. ## stdout is the protocol diff --git a/packages/ui/acp/src/codec.ts b/packages/ui/acp/src/codec.ts index 57622fbbfe..3b03a81c83 100644 --- a/packages/ui/acp/src/codec.ts +++ b/packages/ui/acp/src/codec.ts @@ -17,7 +17,8 @@ import type { ContentBlock as AcpContentBlock, StopReason } from '@agentclientpr * Map a harness {@link TurnEndReason} to the ACP `StopReason` wire enum. * * The mapping is total over the kinds the loop actually produces today - * (`completed`/`aborted`/`error`/`disposed`/`max-tokens`). `TurnEndReason` is + * (`completed`/`aborted`/`error`/`disposed`/`max-tokens`/`rejected`). + * `TurnEndReason` is * merge-extensible, so an unknown future kind falls through to `end_turn` — * the safest default (the turn DID end; we just lack a more specific wire * reason) — rather than throwing into the SDK, which would reject an unknown diff --git a/packages/ui/acp/src/index.ts b/packages/ui/acp/src/index.ts index 754b1750be..b3ca2d3bf1 100644 --- a/packages/ui/acp/src/index.ts +++ b/packages/ui/acp/src/index.ts @@ -22,7 +22,7 @@ * `agent→sessionId` reverse map for O(1) demux of `agent/*` events; every * `session/event` and `agent/*` event is routed strictly to its owning session * record, so two sessions streaming at once never interleave their - * `session/update` notifications. The `tools/execute` permission gate is + * `session/update` notifications. The `tools/pre-execute` permission gate is * deferred — see the TODO(rfc010-permission-gate) note below. * * stdout is the protocol: this plugin must run in an example that loads NO From 60418a5779f073bfbfa19191e8ac095afe24edf9 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Tue, 30 Jun 2026 20:48:07 +0800 Subject: [PATCH 3/6] =?UTF-8?q?docs(events):=20address=20Codex=20round-2?= =?UTF-8?q?=20=E2=80=94=20purge=20remaining=20single-waterfall=20reference?= =?UTF-8?q?s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex's confirmation review confirmed blocker #1 (post-execute mutation) fixed and the guard real, but found three current-state references the first sweep missed (they phrase the seam without the literal "tools/execute" string): - packages/core/tools/src/index.ts: the ToolExecution JSDoc said "flows through the execution waterfall" → now names the pre-execute → dispatch → post-execute pipeline. - packages/core/tools/src/schema.ts: ToolArgsError's JSDoc said "the registry's execute waterfall catches it" → "the registry's execution pipeline". - AGENTS.md repo layout still described tools/ as "tool registry + tools/execute waterfall" → "tools/pre-execute/post-execute pipeline". Doc-comment-only; no behavior change. --- AGENTS.md | 2 +- packages/core/tools/src/index.ts | 2 +- packages/core/tools/src/schema.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index db20d3f7e4..833aeeea45 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -59,7 +59,7 @@ packages/ Harness packages, grouped by role at packages///. core/ product API spine session/ event-sourced session log + in-memory store system-prompt/ prompt-section + tool-schema assembly registry - tools/ tool registry + tools/execute waterfall + tools/ tool registry + tools/pre-execute/post-execute pipeline agent/ Agent interface, registry, agent/* event vocabulary agent-loop/ THE concrete plugin: ReactLoopAgent + the loop driver agent-core/ bundle plugin: the providerless/executor-less/UI-less spine diff --git a/packages/core/tools/src/index.ts b/packages/core/tools/src/index.ts index a8f881c214..b08403503e 100644 --- a/packages/core/tools/src/index.ts +++ b/packages/core/tools/src/index.ts @@ -224,7 +224,7 @@ export interface ToolResult { isError: boolean } -/** One pending tool call, as it flows through the execution waterfall. */ +/** One pending tool call, as it flows through the execution pipeline (`tools/pre-execute` → dispatch → `tools/post-execute`). */ export interface ToolExecution { callId: CallId name: string diff --git a/packages/core/tools/src/schema.ts b/packages/core/tools/src/schema.ts index b717eabf9a..05197986bd 100644 --- a/packages/core/tools/src/schema.ts +++ b/packages/core/tools/src/schema.ts @@ -182,7 +182,7 @@ export function schemaSpecToJsonSchema(spec: SchemaSpec): JsonSchemaObject { /** * Thrown by a {@link defineTool} tool when the model-generated arguments don't * match the declared {@link SchemaSpec}. Extends {@link HarnessError} - * (`code: 'INVALID_ARGS'`); the registry's execute waterfall catches it and + * (`code: 'INVALID_ARGS'`); the registry's execution pipeline catches it and * returns an `isError` ToolExecutionResult carrying the structured error, so * the model can self-correct and downstream plugins can route on the code. */ From 3712f67bc647083fe45c66230a05f0fc86b00d90 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Wed, 1 Jul 2026 15:39:08 +0800 Subject: [PATCH 4/6] =?UTF-8?q?fix(events):=20address=20review=20=E2=80=94?= =?UTF-8?q?=20core.md=20turn-only=20taxonomy,=20RFC=20mechanism=20names,?= =?UTF-8?q?=20post-execute=20content=20snapshot?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - core-data-structures/core.md: the `agent/*` taxonomy said "turn/step boundaries", but the step-boundary mirror emits were dropped — `agent/*` mirrors only turn boundaries; step boundaries are durable `step/start`/ `step/end` session events. Narrow the catalog so plugin authors aren't pointed at nonexistent `agent/*` step events. - interception-seams RFC: replace stack-position phrasing ("a later stack PR", "the stack's first change", "the PR that makes...") with durable mechanism/RFC names (the hook bridge packages, the event-domain-semantics RFC). - tools/post-execute snapshot: `dispatched.content` was the same array reference as `result.content`, so a listener's in-place `push`/`splice` leaked into the returned content while a reassignment was masked — the "protect from tampering" comment over-claimed. Copy content into a fresh array so the snapshot guards the array structure; comment now states it is not deep immutability. Regression extended to push a block in-place and assert it does not leak (proven red without the copy). --- docs/core-data-structures/core.md | 2 +- .../implemented/feature/2026-06-30-interception-seams.md | 8 ++++---- packages/core/tools/src/index.ts | 7 +++++-- packages/core/tools/tests/tools.spec.ts | 5 ++++- 4 files changed, 14 insertions(+), 8 deletions(-) diff --git a/docs/core-data-structures/core.md b/docs/core-data-structures/core.md index 2a430cebab..8141df44da 100644 --- a/docs/core-data-structures/core.md +++ b/docs/core-data-structures/core.md @@ -306,7 +306,7 @@ interface Agent { } ``` -`AgentStatus` is `'idle' | 'running' | 'disposed'`. `AgentId` is a branded string. `AgentOptions` (`model?`, `systemPrompt?`) is merge-extensible — plugins add creation options by declaration merging. The `agent/*` event taxonomy (lifecycle, turn/step boundaries, the `agent/prompt-submit`/`agent/request`/`agent/step-result`/`agent/turn-continuation` waterfalls) is in [architecture.md § Event taxonomy](../architecture.md#event-taxonomy). +`AgentStatus` is `'idle' | 'running' | 'disposed'`. `AgentId` is a branded string. `AgentOptions` (`model?`, `systemPrompt?`) is merge-extensible — plugins add creation options by declaration merging. The `agent/*` event taxonomy (lifecycle, live turn boundaries, the `agent/prompt-submit`/`agent/request`/`agent/step-result`/`agent/turn-continuation` waterfalls) is in [architecture.md § Event taxonomy](../architecture.md#event-taxonomy); step boundaries are durable `step/start`/`step/end` session events only — `agent/*` mirrors turn boundaries, not steps. ## Interception decisions diff --git a/docs/rfc/implemented/feature/2026-06-30-interception-seams.md b/docs/rfc/implemented/feature/2026-06-30-interception-seams.md index 5c00760c46..c79f237343 100644 --- a/docs/rfc/implemented/feature/2026-06-30-interception-seams.md +++ b/docs/rfc/implemented/feature/2026-06-30-interception-seams.md @@ -6,9 +6,9 @@ Status: implemented (accepted 2026-06-30) ## Context -The harness needs a hooks subsystem: users extend or gate the agent at lifecycle points the way Claude Code (CC) and Codex do. The key reframe driving this design is that **"native hooks" are not a package** — a native hook is just an ordinary Cordis plugin subscribing to the canonical lifecycle events. So the real product is a *powerful, well-typed canonical event surface*; the CC/Codex bridges (a later stack PR) are merely translators that map an external shell-hook protocol onto that same surface. Anything a bridge can do, a plain plugin can do directly — more powerfully (no serialization boundary, full `ctx`, typed returns). +The harness needs a hooks subsystem: users extend or gate the agent at lifecycle points the way Claude Code (CC) and Codex do. The key reframe driving this design is that **"native hooks" are not a package** — a native hook is just an ordinary Cordis plugin subscribing to the canonical lifecycle events. So the real product is a *powerful, well-typed canonical event surface*; the CC/Codex bridges (the `dsh-hooks-claude` / `dsh-hooks-codex` packages) are merely translators that map an external shell-hook protocol onto that same surface. Anything a bridge can do, a plain plugin can do directly — more powerfully (no serialization boundary, full `ctx`, typed returns). -Before this change the interception surface was incomplete and inconsistent for that goal: there was no per-prompt seam (CC's `UserPromptSubmit`), no session-start signal (CC's `SessionStart`), the single `tools/execute` waterfall conflated the pre-gate and post-inspect phases (CC splits `PreToolUse`/`PostToolUse`), and `agent/turn-continuation` returned a bare `boolean` with no room for a force-continue *reason*. The [event-domain-semantics RFC](../architecture/2026-06-30-event-domain-semantics.md) (the stack's first change) pinned down the three-domain rule and the typed-Decision idiom as the interception convention; this RFC builds the actual seams on top of it. +Before this change the interception surface was incomplete and inconsistent for that goal: there was no per-prompt seam (CC's `UserPromptSubmit`), no session-start signal (CC's `SessionStart`), the single `tools/execute` waterfall conflated the pre-gate and post-inspect phases (CC splits `PreToolUse`/`PostToolUse`), and `agent/turn-continuation` returned a bare `boolean` with no room for a force-continue *reason*. The [event-domain-semantics RFC](../architecture/2026-06-30-event-domain-semantics.md) pinned down the three-domain rule and the typed-Decision idiom as the interception convention; this RFC builds the actual seams on top of it. ## Decision @@ -38,8 +38,8 @@ Add/​reshape the interception seams so every one returns a small, seam-specifi ### What this PR does NOT do -It does **not** declare `hook/*` SessionEvents (the durable hook-invocation log) — those belong to the `dsh-hook-protocol` library (a later stack PR), because a native plugin can already use the typed Decisions without a durable hook log. A worked native-plugin example/test in this PR (`packages/core/agent-loop/tests/interception.spec.ts`) proves all the seams compose end-to-end through the REAL loop with NO `hook/*` involved — the concrete proof that "native hooks are just a plugin". Compaction (`PreCompact`/`PostCompact`), the Notification hook, Codex `PermissionRequest`, the permission/`ask` system, and the Stop loop-guard remain deferred (`FIXME(permissions)` marks the `ask`→deny degrade). +It does **not** declare `hook/*` SessionEvents (the durable hook-invocation log) — those belong to the `dsh-hook-protocol` library, because a native plugin can already use the typed Decisions without a durable hook log. A worked native-plugin example/test in this PR (`packages/core/agent-loop/tests/interception.spec.ts`) proves all the seams compose end-to-end through the REAL loop with NO `hook/*` involved — the concrete proof that "native hooks are just a plugin". Compaction (`PreCompact`/`PostCompact`), the Notification hook, Codex `PermissionRequest`, the permission/`ask` system, and the Stop loop-guard remain deferred (`FIXME(permissions)` marks the `ask`→deny degrade). ## Consequences -The canonical interception surface is now complete and uniformly typed: a native plugin returns typed decisions directly, and a CC/Codex bridge maps its protocol fields onto the same unions. The loop gained four firing points (session-start emit, prompt-submit waterfall, the post-tool context buffer, the continuation reshape) and the `dsh-tools` registry runs a two-waterfall pipeline; both are documented in [architecture.md](../../../architecture.md) and the package READMEs, and the decision types in [core-data-structures](../../../core-data-structures/core.md#interception-decisions) + [tools.md](../../../core-data-structures/tools.md). All existing `tools/execute` and `turn-continuation` listeners (tests, docs) migrated to the new seams. The ACP bridge maps the new `rejected` reason to `cancelled` (its codec). A pure internal change with no editor-visible transcript shift for the existing scenarios — the new behavior only fires when a hook is registered — so the snapshot goldens are unchanged; a hook-driven snapshot scenario lands with the bridges (the PR that makes a hook observable end-to-end through ACP). +The canonical interception surface is now complete and uniformly typed: a native plugin returns typed decisions directly, and a CC/Codex bridge maps its protocol fields onto the same unions. The loop gained four firing points (session-start emit, prompt-submit waterfall, the post-tool context buffer, the continuation reshape) and the `dsh-tools` registry runs a two-waterfall pipeline; both are documented in [architecture.md](../../../architecture.md) and the package READMEs, and the decision types in [core-data-structures](../../../core-data-structures/core.md#interception-decisions) + [tools.md](../../../core-data-structures/tools.md). All existing `tools/execute` and `turn-continuation` listeners (tests, docs) migrated to the new seams. The ACP bridge maps the new `rejected` reason to `cancelled` (its codec). A pure internal change with no editor-visible transcript shift for the existing scenarios — the new behavior only fires when a hook is registered — so the snapshot goldens are unchanged; a hook-driven snapshot scenario lands with the `dsh-hooks-claude` bridge, which is what makes a hook observable end-to-end through ACP. diff --git a/packages/core/tools/src/index.ts b/packages/core/tools/src/index.ts index b08403503e..73500d9d13 100644 --- a/packages/core/tools/src/index.ts +++ b/packages/core/tools/src/index.ts @@ -471,10 +471,13 @@ export class ToolRegistry extends Service { // authoritative-call-id requirement and the "preserve the dispatched // isError/error" contract. The decision is the ONLY sanctioned channel for a // listener to change the outcome (block, or accept-with-replacement); the - // call id is always the authoritative `exec.callId`. + // call id is always the authoritative `exec.callId`. `content` is copied into + // a fresh array so a listener's in-place `push`/`splice` on `result.content` + // cannot leak into the returned content either (the elements are the same + // references — the snapshot guards the array structure, not deep immutability). const dispatched = { callId: exec.callId, - content: result.content, + content: [...result.content], isError: result.isError, ...result.error ? { error: result.error } : {}, } diff --git a/packages/core/tools/tests/tools.spec.ts b/packages/core/tools/tests/tools.spec.ts index ef67eddb95..a924ab234b 100644 --- a/packages/core/tools/tests/tools.spec.ts +++ b/packages/core/tools/tests/tools.spec.ts @@ -211,10 +211,11 @@ describe('ToolRegistry', () => { ctx.tools.register(echoTool) ctx.on('tools/post-execute', async (_exec, result, next) => { - const mutable = result as { callId: string; isError: boolean; error?: unknown } + const mutable = result as { callId: string; isError: boolean; error?: unknown; content: unknown[] } mutable.callId = 'hijacked' mutable.isError = true mutable.error = { name: 'Evil', code: 'EVIL' } + mutable.content.push({ type: 'text', text: 'INJECTED' }) // in-place array mutation return next() // delegate to the default accept — no decision-level override }) @@ -222,7 +223,9 @@ describe('ToolRegistry', () => { expect(result.callId).toBe(CallId('c1')) // authoritative exec.callId, not 'hijacked' expect(result.isError).toBe(false) // the real (successful) dispatch outcome expect(result.error).toBeUndefined() // no listener-injected error + expect(result.content).toHaveLength(1) // the in-place push did not leak in expect(result.content[0]).toMatchObject({ text: 'hi' }) + expect(result.content.some(b => (b as { text?: string }).text === 'INJECTED')).toBe(false) }) it('composes pre + post waterfalls around dispatch (sandbox-wrap pattern)', async () => { From d3be934a173a0d58295c8be48904dafba85added Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Thu, 2 Jul 2026 05:13:16 +0800 Subject: [PATCH 5/6] docs(events): fix stale turn-mirror / tools-execute prose caught in review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex review of the interception-seams merge flagged current-state prose still describing removed/renamed surfaces: - packages/core/agent/src/types.ts module doc: the agent/* "transient emits" list still ended with "the turn boundaries" — corrected to state turn/step boundaries are durable session/event records, not agent/* emits, and to list the actual interception seams (prompt-submit/pre-step/request/step-result/ turn-continuation) + agent/session-start. - interception-seams RFC: "agent/turn-end fires and the ACP bridge settles" → the durable turn/end is appended and ACP settles off it (no turn mirror). - two proposed RFCs (acp-agent-client-protocol, optional-code-mode) named the pre-split `tools/execute` waterfall → the `tools/pre-execute`/`tools/post-execute` pair. Regenerated the cordis catalog (module-doc change). No code/behavior change. --- docs/cordis-catalog/events-and-services.md | 26 +++++++++---------- .../feature/2026-06-30-interception-seams.md | 2 +- .../2026-06-14-acp-agent-client-protocol.md | 2 +- .../feature/2026-06-15-optional-code-mode.md | 2 +- packages/core/agent/src/types.ts | 14 +++++----- 5 files changed, 24 insertions(+), 22 deletions(-) diff --git a/docs/cordis-catalog/events-and-services.md b/docs/cordis-catalog/events-and-services.md index 62174667b1..dc4b91f6b3 100644 --- a/docs/cordis-catalog/events-and-services.md +++ b/docs/cordis-catalog/events-and-services.md @@ -25,7 +25,7 @@ An agent was registered in the AgentRegistry and is ready to receive messages. Types: [Agent](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:227`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:229`](../../packages/core/agent/src/types.ts) #### `agent/disposed` — emit @@ -37,7 +37,7 @@ An agent was disposed and removed from the registry; its fiber and any in-flight Types: [Agent](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:233`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:235`](../../packages/core/agent/src/types.ts) #### `agent/error` — emit @@ -49,7 +49,7 @@ A step or turn errored. The loop reports a failure here (plus the logger) even w Types: [Agent](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:352`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:354`](../../packages/core/agent/src/types.ts) #### `agent/pre-step` — serial @@ -63,7 +63,7 @@ Serial (awaited in registration order), not a waterfall: a listener mutates the Types: [Agent](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:299`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:301`](../../packages/core/agent/src/types.ts) #### `agent/prompt-submit` — waterfall @@ -75,7 +75,7 @@ Waterfall: decide what happens to ONE drained queued message before it becomes a Types: [Agent](../core-data-structures/core.md) · [ContentBlock](../core-data-structures/core.md) · [MessageSource](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:309`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:311`](../../packages/core/agent/src/types.ts) #### `agent/queued` — emit @@ -87,7 +87,7 @@ A message entered the agent's inbox (queued or steering). `source` is the resolv Types: [Agent](../core-data-structures/core.md) · [ContentBlock](../core-data-structures/core.md) · [MessageSource](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:246`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:248`](../../packages/core/agent/src/types.ts) #### `agent/request` — waterfall @@ -99,7 +99,7 @@ Waterfall: mutate the fully-assembled GenerateOptions before the model call (hoo Types: [Agent](../core-data-structures/core.md) · [GenerateOptions](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:318`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:320`](../../packages/core/agent/src/types.ts) #### `agent/session-start` — emit @@ -111,7 +111,7 @@ The agent's session lifecycle began, fired once before its first turn. `source` Types: [Agent](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:259`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:261`](../../packages/core/agent/src/types.ts) #### `agent/status` — emit @@ -123,7 +123,7 @@ Agent status changed (`idle` ⇄ `running`, or → `disposed`). Drive lifecycle Types: [Agent](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:240`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:242`](../../packages/core/agent/src/types.ts) #### `agent/steering` — emit @@ -135,7 +135,7 @@ Steering content was injected into a running turn. Types: [Agent](../core-data-structures/core.md) · [ContentBlock](../core-data-structures/core.md) · [MessageSource](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:346`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:348`](../../packages/core/agent/src/types.ts) #### `agent/step-result` — waterfall @@ -147,7 +147,7 @@ Waterfall: post-process the assembled assistant Message before tool dispatch (va Types: [Agent](../core-data-structures/core.md) · [Message](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:324`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:326`](../../packages/core/agent/src/types.ts) #### `agent/stream-chunk` — emit @@ -159,7 +159,7 @@ A raw StreamChunk arrived from the model (token-level UI/log feed). Types: [Agent](../core-data-structures/core.md) · [StreamChunk](../core-data-structures/llm-streaming.md) -Source: [`packages/core/agent/src/types.ts:341`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:343`](../../packages/core/agent/src/types.ts) #### `agent/turn-continuation` — waterfall @@ -171,7 +171,7 @@ Waterfall: override the turn-continuation decision via a typed ContinuationDecis Types: [Agent](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:334`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:336`](../../packages/core/agent/src/types.ts) ### `llm/*` diff --git a/docs/rfc/implemented/feature/2026-06-30-interception-seams.md b/docs/rfc/implemented/feature/2026-06-30-interception-seams.md index c79f237343..7f2d23ab3e 100644 --- a/docs/rfc/implemented/feature/2026-06-30-interception-seams.md +++ b/docs/rfc/implemented/feature/2026-06-30-interception-seams.md @@ -26,7 +26,7 @@ Add/​reshape the interception seams so every one returns a small, seam-specifi ### Three load-bearing loop decisions -1. **Always open the turn first; a fully-blocked batch is a zero-step `rejected` turn.** `prompt-submit` fires AFTER `turn/start`, per message. A batch whose every prompt is blocked does NOT skip the turn — it opens a zero-step turn that closes with `rejected`. This one move resolves three problems at once: (1) turn-enclosure holds (every event has an open turn to live in); (2) `agent/turn-end` fires and the ACP bridge settles normally (mapping `rejected`→`cancelled`) instead of hanging; (3) the block reason is a durable in-turn fact. An `allow`'s `additionalContext` is `inject()`ed into this now-open turn. +1. **Always open the turn first; a fully-blocked batch is a zero-step `rejected` turn.** `prompt-submit` fires AFTER `turn/start`, per message. A batch whose every prompt is blocked does NOT skip the turn — it opens a zero-step turn that closes with `rejected`. This one move resolves three problems at once: (1) turn-enclosure holds (every event has an open turn to live in); (2) the durable `turn/end` is appended and the ACP bridge settles normally off it (mapping `rejected`→`cancelled`) instead of hanging; (3) the block reason is a durable in-turn fact. An `allow`'s `additionalContext` is `inject()`ed into this now-open turn. 2. **Post-tool `additionalContext` is buffered and appended AFTER all `tool/result`s.** `content`/`feedback` shape the result `execute()` returns, but `additionalContext` is a SEPARATE `context/message`, and a single step can carry multiple tool calls. Appending context right after each result would interleave `result(c1) → context → result(c2)` and break tool-call/result adjacency. So `execute()` surfaces `additionalContext` on its `ToolExecutionResult`, and the loop buffers every per-call context for the step and appends them as `context/message`(s) only after every `tool/result` is appended. diff --git a/docs/rfc/proposed/feature/2026-06-14-acp-agent-client-protocol.md b/docs/rfc/proposed/feature/2026-06-14-acp-agent-client-protocol.md index 0aa5af4dc4..1e31a3fdb8 100644 --- a/docs/rfc/proposed/feature/2026-06-14-acp-agent-client-protocol.md +++ b/docs/rfc/proposed/feature/2026-06-14-acp-agent-client-protocol.md @@ -15,7 +15,7 @@ This RFC has a hard prerequisite on [session persistence](../../implemented/arch ## Proposal -A new plugin package `@deepseek-ai/dsh-acp` — a client-driver / UI plugin, the structured analogue of `stdio-chat`. It is NOT a change to the loop and NOT an [capability seams](../../implemented/architecture/2026-06-13-capability-seams.md) interface/implementation/consumer capability split; it consumes the existing `agent/*` event taxonomy and the `tools/execute` waterfall. +A new plugin package `@deepseek-ai/dsh-acp` — a client-driver / UI plugin, the structured analogue of `stdio-chat`. It is NOT a change to the loop and NOT an [capability seams](../../implemented/architecture/2026-06-13-capability-seams.md) interface/implementation/consumer capability split; it consumes the existing `agent/*` event taxonomy and the `tools/pre-execute`/`tools/post-execute` waterfalls. It depends on the official `@agentclientprotocol/sdk` (the `AgentSideConnection` class) — Apache-2.0, actively versioned. The SDK declares a `zod` peer dependency and imports `zod/v4` at runtime, so `packages/ui/acp` must declare `zod` itself (per the workspace dependency constraints). This is the renamed successor to `@zed-industries/agent-client-protocol`, which is now deprecated on npm. diff --git a/docs/rfc/proposed/feature/2026-06-15-optional-code-mode.md b/docs/rfc/proposed/feature/2026-06-15-optional-code-mode.md index eb41750d1d..f5da38c14c 100644 --- a/docs/rfc/proposed/feature/2026-06-15-optional-code-mode.md +++ b/docs/rfc/proposed/feature/2026-06-15-optional-code-mode.md @@ -55,7 +55,7 @@ These are illustrations of the seam's reach, **not commitments** — the MVP shi **3c. The single tool — `run_code`.** Registered normally in `ctx.tools` with one parameter `{ code: string (required) }`. Because it is an ordinary tool, the unchanged loop dispatches it through the normal path — this is the crux of "zero loop changes." Its `execute(args, exec)`: -1. Builds the SDK bindings. For each real tool, an async `invoke(callArgs)` that **checks `exec.signal?.aborted` (throwing if set) before and after** calling `ctx.tools.execute({ callId: , name, arguments: callArgs, agent: exec.agent, signal: exec.signal })`, then maps the resulting `ContentBlock[]` to a simplified `{ output, isError }` (text blocks for the MVP), and emits an observability event. The explicit abort check matters because `ctx.tools.execute()` *catches* thrown tool errors and converts them to `isError` results — without the check, an aborted sub-call would look like ordinary error data and the program would keep running instead of stopping. Sub-dispatch still flows through the `tools/execute` waterfall, so permission/sandbox/hook plugins apply to code-mode calls exactly as to native ones. +1. Builds the SDK bindings. For each real tool, an async `invoke(callArgs)` that **checks `exec.signal?.aborted` (throwing if set) before and after** calling `ctx.tools.execute({ callId: , name, arguments: callArgs, agent: exec.agent, signal: exec.signal })`, then maps the resulting `ContentBlock[]` to a simplified `{ output, isError }` (text blocks for the MVP), and emits an observability event. The explicit abort check matters because `ctx.tools.execute()` *catches* thrown tool errors and converts them to `isError` results — without the check, an aborted sub-call would look like ordinary error data and the program would keep running instead of stopping. Sub-dispatch still flows through the `tools/pre-execute`/`tools/post-execute` waterfalls, so permission/sandbox/hook plugins apply to code-mode calls exactly as to native ones. 2. Calls `ctx.codeRuntime.run({ code: args.code, sdk: bindings, signal: exec.signal })`. 3. Surfaces the outcome. A *successful* run returns `[{ type: 'text', text: }]`. A *runtime-error* result cannot be reported by returning content, because a normal `ToolDefinition.execute()` returns only `Promise` and `ToolRegistry.execute()` hardcodes `isError: false` on any successful return — `isError: true` arises only from the registry's catch path. So on an error result the tool **throws a `CodeRunError extends HarnessError`** (`HarnessError` is exported from `dsh-llm`; the registry catch turns any throw into `isError: true` with the message as text, and a `HarnessError` additionally carries structured `{ name, code }`). An alternative — registering `run_code` handling as a `tools/execute` listener that returns a full `ToolExecutionResult` and can set `isError` directly — is noted; the throw is simpler and preferred. diff --git a/packages/core/agent/src/types.ts b/packages/core/agent/src/types.ts index e7511935a9..b92bc08d39 100644 --- a/packages/core/agent/src/types.ts +++ b/packages/core/agent/src/types.ts @@ -16,12 +16,14 @@ * durability checkpoint. Answers "what happened, durably/replayably." A * consumer that wants the live transcript subscribes here. * - **`agent/*`** (this module) — the LIVE runtime surface. Always carries the - * live `Agent`. Two shapes: INTERCEPTION waterfalls (`agent/request`, - * `agent/step-result`, `agent/turn-continuation`) that mutate/veto, and - * TRANSIENT emits (`agent/status`, `agent/stream-chunk`, `agent/error`, - * `agent/created`/`agent/disposed`, `agent/queued`, `agent/steering`, and the - * turn boundaries) that notify with the `Agent` in hand. Answers "right now, - * with the agent object — intercept or observe." + * live `Agent`. Two shapes: INTERCEPTION seams (the `agent/prompt-submit`/ + * `agent/request`/`agent/step-result`/`agent/turn-continuation` waterfalls and + * the serial `agent/pre-step`) that mutate/veto, and TRANSIENT emits + * (`agent/status`, `agent/stream-chunk`, `agent/error`, `agent/created`/ + * `agent/disposed`, `agent/queued`, `agent/steering`, `agent/session-start`) + * that notify with the `Agent` in hand. Turn/step boundaries are NOT here — + * they are durable `session/event` records. Answers "right now, with the agent + * object — intercept or observe." * - **`tools/*`** (`@deepseek-ai/dsh-tools`) — the tool registry + execution. * * **The rule:** a durable, replayable fact is a SessionEvent; a live From abe80cec68cf2ca989f710dec4ecc5c691471602 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Thu, 2 Jul 2026 17:03:53 +0800 Subject: [PATCH 6/6] fix(loop): record a durable prompt/blocked for every vetoed prompt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review on the interception-seams PR: PromptDecision.reason is documented as the durable record of why a prompt was blocked, but the loop only surfaced it via the fully-blocked batch's `rejected` turn/end. In a MIXED batch — one queued prompt blocked, another allowed — the turn does not end `rejected`, so the blocked prompt and its reason vanished from the session log entirely. Add a `prompt/blocked` SessionEventMap variant (content + source + reason), appended in the open turn at the veto point in place of the user/message the prompt would have become. It is a non-surface, turn-enclosed event (like todo/write): it never reaches deriveMessages(). The fully-blocked batch still also ends `rejected` for boundary balance + ACP settlement. Regression test drives a mixed batch and asserts the blocked prompt is recorded while the allowed one runs — proven red without the append. --- docs/cordis-catalog/events-and-services.md | 26 +++++------ docs/core-data-structures/core.md | 2 +- docs/core-data-structures/session.md | 11 +++++ .../feature/2026-06-30-interception-seams.md | 4 +- packages/core/agent-loop/README.md | 2 +- packages/core/agent-loop/src/loop.ts | 8 ++++ .../agent-loop/tests/interception.spec.ts | 45 +++++++++++++++++++ packages/core/agent/src/types.ts | 12 +++-- packages/core/session/README.md | 2 +- packages/core/session/src/types.ts | 11 +++++ 10 files changed, 101 insertions(+), 22 deletions(-) diff --git a/docs/cordis-catalog/events-and-services.md b/docs/cordis-catalog/events-and-services.md index dc4b91f6b3..282e161235 100644 --- a/docs/cordis-catalog/events-and-services.md +++ b/docs/cordis-catalog/events-and-services.md @@ -25,7 +25,7 @@ An agent was registered in the AgentRegistry and is ready to receive messages. Types: [Agent](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:229`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:233`](../../packages/core/agent/src/types.ts) #### `agent/disposed` — emit @@ -37,7 +37,7 @@ An agent was disposed and removed from the registry; its fiber and any in-flight Types: [Agent](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:235`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:239`](../../packages/core/agent/src/types.ts) #### `agent/error` — emit @@ -49,7 +49,7 @@ A step or turn errored. The loop reports a failure here (plus the logger) even w Types: [Agent](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:354`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:358`](../../packages/core/agent/src/types.ts) #### `agent/pre-step` — serial @@ -63,7 +63,7 @@ Serial (awaited in registration order), not a waterfall: a listener mutates the Types: [Agent](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:301`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:305`](../../packages/core/agent/src/types.ts) #### `agent/prompt-submit` — waterfall @@ -75,7 +75,7 @@ Waterfall: decide what happens to ONE drained queued message before it becomes a Types: [Agent](../core-data-structures/core.md) · [ContentBlock](../core-data-structures/core.md) · [MessageSource](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:311`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:315`](../../packages/core/agent/src/types.ts) #### `agent/queued` — emit @@ -87,7 +87,7 @@ A message entered the agent's inbox (queued or steering). `source` is the resolv Types: [Agent](../core-data-structures/core.md) · [ContentBlock](../core-data-structures/core.md) · [MessageSource](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:248`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:252`](../../packages/core/agent/src/types.ts) #### `agent/request` — waterfall @@ -99,7 +99,7 @@ Waterfall: mutate the fully-assembled GenerateOptions before the model call (hoo Types: [Agent](../core-data-structures/core.md) · [GenerateOptions](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:320`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:324`](../../packages/core/agent/src/types.ts) #### `agent/session-start` — emit @@ -111,7 +111,7 @@ The agent's session lifecycle began, fired once before its first turn. `source` Types: [Agent](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:261`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:265`](../../packages/core/agent/src/types.ts) #### `agent/status` — emit @@ -123,7 +123,7 @@ Agent status changed (`idle` ⇄ `running`, or → `disposed`). Drive lifecycle Types: [Agent](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:242`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:246`](../../packages/core/agent/src/types.ts) #### `agent/steering` — emit @@ -135,7 +135,7 @@ Steering content was injected into a running turn. Types: [Agent](../core-data-structures/core.md) · [ContentBlock](../core-data-structures/core.md) · [MessageSource](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:348`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:352`](../../packages/core/agent/src/types.ts) #### `agent/step-result` — waterfall @@ -147,7 +147,7 @@ Waterfall: post-process the assembled assistant Message before tool dispatch (va Types: [Agent](../core-data-structures/core.md) · [Message](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:326`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:330`](../../packages/core/agent/src/types.ts) #### `agent/stream-chunk` — emit @@ -159,7 +159,7 @@ A raw StreamChunk arrived from the model (token-level UI/log feed). Types: [Agent](../core-data-structures/core.md) · [StreamChunk](../core-data-structures/llm-streaming.md) -Source: [`packages/core/agent/src/types.ts:343`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:347`](../../packages/core/agent/src/types.ts) #### `agent/turn-continuation` — waterfall @@ -171,7 +171,7 @@ Waterfall: override the turn-continuation decision via a typed ContinuationDecis Types: [Agent](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:336`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:340`](../../packages/core/agent/src/types.ts) ### `llm/*` diff --git a/docs/core-data-structures/core.md b/docs/core-data-structures/core.md index 68f5357e3a..1846bf0bdc 100644 --- a/docs/core-data-structures/core.md +++ b/docs/core-data-structures/core.md @@ -214,7 +214,7 @@ type SessionEvent = { }[T] ``` -The twelve event variants (`turn/start`, `turn/end`, `step/start`, `step/end`, `user/message`, `context/message`, `assistant/chunk`, `assistant/message`, `tool/call`, `tool/result`, `steering/message`, `todo/write`), the `deriveMessages()` projection rules, the `TurnTrigger`/`TurnEndReason` reasons, and the turn-enclosure invariant are on **[session.md](session.md)**. How the log is made durable — the `SessionPersistence` seam, JSONL/SQLite backends, the `session/flush` checkpoint, crash recovery, and `SessionHeader` — is on **[persistence.md](persistence.md)**. +The thirteen event variants (`turn/start`, `turn/end`, `step/start`, `step/end`, `user/message`, `prompt/blocked`, `context/message`, `assistant/chunk`, `assistant/message`, `tool/call`, `tool/result`, `steering/message`, `todo/write`), the `deriveMessages()` projection rules, the `TurnTrigger`/`TurnEndReason` reasons, and the turn-enclosure invariant are on **[session.md](session.md)**. How the log is made durable — the `SessionPersistence` seam, JSONL/SQLite backends, the `session/flush` checkpoint, crash recovery, and `SessionHeader` — is on **[persistence.md](persistence.md)**. ## The agent handle diff --git a/docs/core-data-structures/session.md b/docs/core-data-structures/session.md index c5188da309..59555b4111 100644 --- a/docs/core-data-structures/session.md +++ b/docs/core-data-structures/session.md @@ -16,6 +16,17 @@ interface SessionEventMap { 'step/end': { turn: number; step: number } /** A user-visible prompt (queued message drained at turn start). */ 'user/message': { content: ContentBlock[]; source: MessageSource } + /** + * A queued prompt an `agent/prompt-submit` listener VETOED — the durable + * record of a blocked prompt and why. Appended in place of the `user/message` + * the prompt would have become, so the block survives replay even in a MIXED + * batch where another queued prompt is allowed (there the turn does not end + * `rejected`, so the boundary reason alone would not preserve it). `content` + * is the original prompt the listener rejected; `reason` is the veto text + * ({@link PromptDecision} `block.reason`). NOT a {@link SurfaceEventType}: a + * blocked prompt produces no LLM message and never reaches `deriveMessages()`. + */ + 'prompt/blocked': { content: ContentBlock[]; source: MessageSource; reason: string } /** * In-session context injection (file-change notices, subdir AGENTS.md, * skill content, cron notifications, …). Rendered into the derived history diff --git a/docs/rfc/implemented/feature/2026-06-30-interception-seams.md b/docs/rfc/implemented/feature/2026-06-30-interception-seams.md index 7f2d23ab3e..138fe975a1 100644 --- a/docs/rfc/implemented/feature/2026-06-30-interception-seams.md +++ b/docs/rfc/implemented/feature/2026-06-30-interception-seams.md @@ -16,7 +16,7 @@ Add/​reshape the interception seams so every one returns a small, seam-specifi **New `agent/*` events** (`dsh-agent`): - `agent/session-start(agent, source)` — emit, once before turn 1, carrying a `SessionStartSource` (`startup` for a fresh/forked create, `resume` for a reloaded persisted session; `clear`/`compact` reserved). A pure notification — it CANNOT block startup (a deliberate gap: a bridge logs/injects, it does not gate startup). A listener seeds context via `agent.inject()`. -- `agent/prompt-submit(agent, content, source, next) → PromptDecision` — waterfall, fired per drained queued message inside the open turn, before the `user/message` append. `allow` (optionally rewriting the prompt `content` or attaching `additionalContext`) or `block`. +- `agent/prompt-submit(agent, content, source, next) → PromptDecision` — waterfall, fired per drained queued message inside the open turn, before the `user/message` append. `allow` (optionally rewriting the prompt `content` or attaching `additionalContext`) or `block` (dropping the prompt; the loop appends a durable `prompt/blocked` in its place — see the dispatch note below). **Reshaped** `agent/turn-continuation` from `(…, defaultDecision: boolean) → boolean` to `(…, defaultDecision: ContinuationDecision) → ContinuationDecision`. A `{action:'continue', reason?}` may carry model-facing context recorded as next-step steering in the same turn — the typed twin of the existing `/goal` step-end-steer pattern. @@ -26,7 +26,7 @@ Add/​reshape the interception seams so every one returns a small, seam-specifi ### Three load-bearing loop decisions -1. **Always open the turn first; a fully-blocked batch is a zero-step `rejected` turn.** `prompt-submit` fires AFTER `turn/start`, per message. A batch whose every prompt is blocked does NOT skip the turn — it opens a zero-step turn that closes with `rejected`. This one move resolves three problems at once: (1) turn-enclosure holds (every event has an open turn to live in); (2) the durable `turn/end` is appended and the ACP bridge settles normally off it (mapping `rejected`→`cancelled`) instead of hanging; (3) the block reason is a durable in-turn fact. An `allow`'s `additionalContext` is `inject()`ed into this now-open turn. +1. **Always open the turn first; a fully-blocked batch is a zero-step `rejected` turn; every veto is recorded as `prompt/blocked`.** `prompt-submit` fires AFTER `turn/start`, per message. A batch whose every prompt is blocked does NOT skip the turn — it opens a zero-step turn that closes with `rejected`. This one move resolves three problems at once: (1) turn-enclosure holds (every event has an open turn to live in); (2) the durable `turn/end` is appended and the ACP bridge settles normally off it (mapping `rejected`→`cancelled`) instead of hanging; (3) the block reason is a durable in-turn fact. Independently, each individual veto appends a `prompt/blocked` session event (the original `content`, `source`, and `reason`) in place of the `user/message` the prompt would have become — necessary because a MIXED batch (one prompt blocked, another allowed) does NOT end `rejected`, so the boundary reason alone would silently lose the blocked prompt on replay. An `allow`'s `additionalContext` is `inject()`ed into this now-open turn. 2. **Post-tool `additionalContext` is buffered and appended AFTER all `tool/result`s.** `content`/`feedback` shape the result `execute()` returns, but `additionalContext` is a SEPARATE `context/message`, and a single step can carry multiple tool calls. Appending context right after each result would interleave `result(c1) → context → result(c2)` and break tool-call/result adjacency. So `execute()` surfaces `additionalContext` on its `ToolExecutionResult`, and the loop buffers every per-call context for the step and appends them as `context/message`(s) only after every `tool/result` is appended. diff --git a/packages/core/agent-loop/README.md b/packages/core/agent-loop/README.md index 7bb41be102..1cf6028f98 100644 --- a/packages/core/agent-loop/README.md +++ b/packages/core/agent-loop/README.md @@ -51,7 +51,7 @@ forever: TURN (error-contained): 'turn/start' each queued: waterfall agent/prompt-submit → allow (→ session('user/message'), - inject additionalContext) | block (drop) + inject additionalContext) | block (→ session('prompt/blocked'), drop) if every prompt blocked: 'turn/end'(rejected), no step ⟵ zero-step turn STEP loop: drain steering diff --git a/packages/core/agent-loop/src/loop.ts b/packages/core/agent-loop/src/loop.ts index 2923a3dd22..d08b5ccdba 100644 --- a/packages/core/agent-loop/src/loop.ts +++ b/packages/core/agent-loop/src/loop.ts @@ -385,6 +385,14 @@ async function runTurn(ctx: Context, agent: ReactLoopAgent, handle: LoopHandle, ) if (decision.kind === 'block') { lastBlockReason = decision.reason + // Record the veto durably: `PromptDecision.reason` is the durable record + // of why a prompt was blocked, but a fully-blocked batch's `rejected` + // turn/end only preserves the LAST reason, and a MIXED batch (this prompt + // blocked, another allowed) does not end `rejected` at all — so without + // this append a blocked prompt would vanish from the log whenever any + // sibling prompt is allowed. `prompt/blocked` sits in the open turn in + // place of the `user/message` this prompt would have become. + session.append('prompt/blocked', { content: message.content, source: message.source, reason: decision.reason }) continue } anyAllowed = true diff --git a/packages/core/agent-loop/tests/interception.spec.ts b/packages/core/agent-loop/tests/interception.spec.ts index b2a5c932d6..e76ac30fa9 100644 --- a/packages/core/agent-loop/tests/interception.spec.ts +++ b/packages/core/agent-loop/tests/interception.spec.ts @@ -176,12 +176,57 @@ describe('agent/prompt-submit', () => { expect(log.some(e => e.type === 'turn/end')).toBe(true) expect(log.some(e => e.type === 'user/message')).toBe(false) expect(log.some(e => e.type === 'step/start')).toBe(false) + // the veto is recorded durably as a prompt/blocked in the open turn + const blocked = log.find(e => e.type === 'prompt/blocked') + expect(blocked?.type === 'prompt/blocked' && blocked.data).toMatchObject({ + content: [{ type: 'text', text: 'do something' }], + reason: 'blocked by policy', + }) // ended rejected with the block reason expect(reasons).toEqual([{ kind: 'rejected', reason: 'blocked by policy' }]) const turnEnd = log.findLast(e => e.type === 'turn/end') expect(turnEnd?.type === 'turn/end' && turnEnd.data.reason).toEqual({ kind: 'rejected', reason: 'blocked by policy' }) }) + it('a mixed batch records a prompt/blocked for the vetoed prompt while the allowed one runs', async () => { + // Two prompts queued into ONE turn: block "secret", allow "safe". The turn is + // NOT rejected (a prompt was allowed), so without a durable prompt/blocked the + // vetoed prompt and its reason would vanish from the log entirely. + const adapter = new MockAdapter([textResponse('ran once')]) + const ctx = await harness(adapter) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + + ctx.on('agent/prompt-submit', async (_agent, content, _source, next): Promise => { + const text = content.map(b => (b.type === 'text' ? b.text : '')).join('') + return text === 'secret' ? { kind: 'block', reason: 'policy: no secrets' } : next() + }) + + const reasons: TurnEndReason[] = [] + ctx.on('session/event', (_s, event: SessionEvent) => { if (event.type === 'turn/end') reasons.push(event.data.reason) }) + + // both sends land before the loop drains → one batched turn + send(agent, 'secret') + send(agent, 'safe') + await waitForIdle(ctx, agent) + + const log = events(agent) + // the allowed prompt became a user/message and drove exactly one model call + const userMsgs = log.filter(e => e.type === 'user/message') + expect(userMsgs).toHaveLength(1) + expect(userMsgs[0]?.type === 'user/message' && userMsgs[0].data.content).toEqual([{ type: 'text', text: 'safe' }]) + expect(adapter.requests.length).toBeGreaterThanOrEqual(1) + // the blocked prompt is durably recorded, with its content + reason + const blocked = log.filter(e => e.type === 'prompt/blocked') + expect(blocked).toHaveLength(1) + expect(blocked[0]?.type === 'prompt/blocked' && blocked[0].data).toMatchObject({ + content: [{ type: 'text', text: 'secret' }], + reason: 'policy: no secrets', + }) + // the turn did NOT reject — a sibling was allowed — so the boundary reason + // alone would not have preserved the block + expect(reasons.some(r => r.kind === 'rejected')).toBe(false) + }) + it('a throwing prompt-submit listener ends the turn balanced (error), loop survives', async () => { const adapter = new MockAdapter([textResponse('after')]) const ctx = await harness(adapter) diff --git a/packages/core/agent/src/types.ts b/packages/core/agent/src/types.ts index b92bc08d39..a8e2113b25 100644 --- a/packages/core/agent/src/types.ts +++ b/packages/core/agent/src/types.ts @@ -96,10 +96,14 @@ export interface HookContext { * - `allow` proceeds with the prompt; optional `content` REPLACES the prompt * bytes (a rewrite), and optional `additionalContext` is `inject()`ed as a * separate `context/message` the next request also sees. - * - `block` drops the prompt entirely; `reason` is the durable record of why. - * A batch whose every prompt is blocked still opens a zero-step turn that ends - * with {@link TurnEndReason} `rejected` (so the boundary stays balanced and a - * UI can render "blocked by hook"). + * - `block` drops the prompt (it never becomes a `user/message`); `reason` is + * the durable record of why. The loop appends a `prompt/blocked` session event + * (carrying the original content, source, and `reason`) in place of the + * dropped `user/message`, so the veto survives replay even in a MIXED batch + * where a sibling prompt is allowed. A batch whose EVERY prompt is blocked + * additionally opens a zero-step turn that ends with {@link TurnEndReason} + * `rejected` (so the boundary stays balanced and a UI can render "blocked by + * hook"). */ export type PromptDecision = | { kind: 'allow'; content?: ContentBlock[]; additionalContext?: HookContext } diff --git a/packages/core/session/README.md b/packages/core/session/README.md index 45512c9d9e..96d267c79b 100644 --- a/packages/core/session/README.md +++ b/packages/core/session/README.md @@ -49,7 +49,7 @@ Plain class (not a Cordis Service). Create via `ctx.sessions.create()`. ### Session event vocabulary (`types.ts`) -The append-only log: `turn/start`, `turn/end`, `step/start`, `step/end`, `user/message`, `assistant/message`, `assistant/chunk`, `tool/call`, `tool/result`, `steering/message`, `context/message`, `todo/write`. Token usage rides on `assistant/message.usage`; an operational error's step is on `turn/end.reason` for `kind: 'error'`. +The append-only log: `turn/start`, `turn/end`, `step/start`, `step/end`, `user/message`, `prompt/blocked`, `assistant/message`, `assistant/chunk`, `tool/call`, `tool/result`, `steering/message`, `context/message`, `todo/write`. Token usage rides on `assistant/message.usage`; an operational error's step is on `turn/end.reason` for `kind: 'error'`. Merge-extensible via `SessionEventMap` — the compaction seam adds `compact/start`, `compact/summary`, and `compact/end`. diff --git a/packages/core/session/src/types.ts b/packages/core/session/src/types.ts index c26750c606..13965d823d 100644 --- a/packages/core/session/src/types.ts +++ b/packages/core/session/src/types.ts @@ -204,6 +204,17 @@ export interface SessionEventMap { 'step/end': { turn: number; step: number } /** A user-visible prompt (queued message drained at turn start). */ 'user/message': { content: ContentBlock[]; source: MessageSource } + /** + * A queued prompt an `agent/prompt-submit` listener VETOED — the durable + * record of a blocked prompt and why. Appended in place of the `user/message` + * the prompt would have become, so the block survives replay even in a MIXED + * batch where another queued prompt is allowed (there the turn does not end + * `rejected`, so the boundary reason alone would not preserve it). `content` + * is the original prompt the listener rejected; `reason` is the veto text + * ({@link PromptDecision} `block.reason`). NOT a {@link SurfaceEventType}: a + * blocked prompt produces no LLM message and never reaches `deriveMessages()`. + */ + 'prompt/blocked': { content: ContentBlock[]; source: MessageSource; reason: string } /** * In-session context injection (file-change notices, subdir AGENTS.md, * skill content, cron notifications, …). Rendered into the derived history