From f256f3961d834391ac1210d8d84d7211221339a0 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sun, 5 Jul 2026 01:54:46 +0800 Subject: [PATCH 1/8] feat(system-prompt): prompt variables, persona-as-section, tool-guidance ownership One principle: every fact in the assembled prompt has exactly one owner. - dsh-system-prompt: merge-extensible AssembleContext on assemble(); a variable(name, provider) registry; {{name}} interpolation in renderPrompt, strict (unknown/valueless/malformed references throw); duplicate section and variable names rejected; assembly carries resolved section text + variables through the assemble waterfall. - dsh-agent declares AssembleContext.agent; dsh-agent-loop registers the agent:persona section (order 0 - identity renders before tool guidance) and the model/cwd variables, and drops its string join: renderPrompt(assembly) IS the full prompt. - Tool guidance moves to its owners: descriptions carry per-tool semantics; sections only cross-call habits (tool:bash exit-code habit at order 105; read's not-shell nudge). todo/subagent need no section - their descriptions already carry the contract. - SubagentProvider.inheritsParentContext (spawn/acp false, fork true); dsh-tool-subagent derives truthful per-provider wording and resolves the provider at load (backend must be listed first). - Example personas shrink to identity + behavior with {{model}} (and {{cwd}} in the ACP tree); the welcome banner stops enumerating tools. RFC: docs/rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md --- CONTEXT.md | 37 ++++ docs/architecture.md | 5 +- docs/cordis-catalog/events-and-services.md | 39 ++-- docs/core-data-structures/subagent.md | 3 +- docs/module-graph.md | 6 +- docs/rfc/README.md | 1 + ...t-variables-and-tool-guidance-ownership.md | 69 +++++++ examples/acp-agent/cordis.yml | 33 +-- examples/coding-agent/cordis.yml | 37 +--- packages/bash/tool-bash/README.md | 4 +- packages/bash/tool-bash/package.json | 1 + packages/bash/tool-bash/src/index.ts | 12 +- packages/bash/tool-bash/tests/tools.spec.ts | 10 + packages/core/agent-loop/README.md | 6 +- packages/core/agent-loop/src/index.ts | 14 ++ packages/core/agent-loop/src/loop.ts | 13 +- packages/core/agent-loop/tests/loop.spec.ts | 47 ++++- .../agent-loop/tests/review-fixes.spec.ts | 6 +- packages/core/agent/package.json | 2 + packages/core/agent/src/types.ts | 23 ++- packages/core/agent/tsconfig.json | 3 + packages/core/system-prompt/README.md | 29 +-- packages/core/system-prompt/src/index.ts | 192 +++++++++++++++--- .../system-prompt/tests/system-prompt.spec.ts | 178 ++++++++++++++-- packages/fs/tool-fs/src/read.ts | 2 +- packages/subagent/subagent-acp/src/index.ts | 2 + packages/subagent/subagent-fork/src/index.ts | 2 + packages/subagent/subagent-spawn/src/index.ts | 2 + packages/subagent/subagent/README.md | 2 + packages/subagent/subagent/src/types.ts | 10 + .../subagent/subagent/tests/service.spec.ts | 4 + packages/subagent/tool-subagent/README.md | 4 + packages/subagent/tool-subagent/src/index.ts | 57 +++++- .../tool-subagent/tests/tool-subagent.spec.ts | 45 +++- packages/support/subagent-mock/README.md | 1 + packages/support/subagent-mock/src/index.ts | 9 + packages/ui/acp-agent/README.md | 2 +- packages/ui/acp/README.md | 2 +- packages/ui/stdio-agent/README.md | 4 +- packages/web/tool-web/tests/tool-web.spec.ts | 2 +- pnpm-lock.yaml | 3 + 41 files changed, 746 insertions(+), 177 deletions(-) create mode 100644 CONTEXT.md create mode 100644 docs/rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md diff --git a/CONTEXT.md b/CONTEXT.md new file mode 100644 index 0000000000..017fada6b8 --- /dev/null +++ b/CONTEXT.md @@ -0,0 +1,37 @@ +# DeepSeek Harness + +Ubiquitous language for the harness. Started during the 2026-07-05 system-prompt redesign session; grows as terms crystallize. Decisions live in `docs/rfc/` (this repo's ADR equivalent), not here. + +## Language — prompt assembly + +**Section**: +One named, ordered fragment of the system prompt, contributed by a plugin through `ctx.systemPrompt.section()`. +_Avoid_: block, snippet + +**Assembly**: +The collated output of `assemble()` — sections, tool schemas, and resolved prompt variables — before rendering. + +**Full system prompt**: +The rendered text the model actually receives: all sections interpolated and joined. There is no other composition path. +_Avoid_: using "system prompt" for any single fragment + +**Persona**: +The per-agent, deployment-authored prompt fragment (config key `systemPrompt` on an agent). A template, not final text; rendered as the order-0 section. It is one section of the full system prompt, never the whole. +_Avoid_: calling it "the system prompt" + +**Prompt variable**: +A named per-assembly value contributed by a plugin (e.g. `model`) and referenced from section or persona text as `{{name}}`. +_Avoid_: placeholder, macro + +**Assemble context**: +The per-agent input to one `assemble()` call, carrying which agent the prompt is for. Merge-extensible; variable providers and section text providers are functions of it. + +**Tool guidance**: +The model-facing usage prose for one tool, owned by the tool's package as a section (order band 100–199) — never hand-written in leaf config. +_Avoid_: tool prompt, tool docs + +## Language — subagents + +**Context contract**: +Whether a subagent provider's child sees the parent conversation (`inheritsParentContext`): fork inherits the log, spawn and ACP start fresh. Declared by the provider, consumed by tool wording. + diff --git a/docs/architecture.md b/docs/architecture.md index 44302fb726..c9090c185a 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -69,7 +69,7 @@ A `Session` is an append-only log of typed `SessionEvent`s — the single source ## Prompt assembly (dsh-system-prompt) -Plugins contribute `PromptSection`s (named, ordered, static or computed) and tool-schema providers; `assemble()` returns `PromptAssembly { sections, tools }` through the `system-prompt/assemble` waterfall. Tool schemas are deliberately part of the assembly — "what the model is told it can do" is one coherent thing — though adapters transmit them as the wire-level `tools` field ([RFC](rfc/implemented/architecture/2026-06-11-tool-schemas-in-prompt-assembly.md)). +Plugins contribute `PromptSection`s (named, ordered, static or computed from the per-call `AssembleContext`), tool-schema providers, and named **prompt variables** interpolated as `{{name}}` at render (strict: an unknown or valueless reference throws). `renderPrompt(assemble({ agent }))` IS the full prompt: the loop's `agent:persona` section (order 0) and its `model`/`cwd` variables carry the per-agent facts — no second composition path. Tool schemas are deliberately part of the assembly ([RFC](rfc/implemented/architecture/2026-06-11-tool-schemas-in-prompt-assembly.md)); prompt-fact ownership (persona vs description vs section vs variable) is pinned by [the prompt-variables RFC](rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md). ## Tool pipeline (dsh-tools) @@ -99,7 +99,8 @@ forever: every prompt blocked → 'turn/end'(rejected), 0 steps ⟵ zero-step turn, model never called STEP loop: drain steering (late steering from previous step's listeners) - assembly = ctx.systemPrompt.assemble() ⟵ waterfall system-prompt/assemble + assembly = ctx.systemPrompt.assemble({agent}) ⟵ waterfall system-prompt/assemble; renderPrompt + (persona section + {{variables}}) IS the prompt await ctx.serial('agent/pre-step') ⟵ surface mutation (compaction) OUTSIDE the step session('step/start') ⟵ durable step boundary (no agent/* mirror) req = {model, system, tools, messages: session.deriveMessages(), signal} diff --git a/docs/cordis-catalog/events-and-services.md b/docs/cordis-catalog/events-and-services.md index 922a0ea018..8433f5c9ab 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:234`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:255`](../../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:241`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:262`](../../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:380`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:401`](../../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:319`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:340`](../../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:332`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:353`](../../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:259`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:280`](../../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:345`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:366`](../../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:274`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:295`](../../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:250`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:271`](../../packages/core/agent/src/types.ts) #### `agent/step-result` — waterfall @@ -135,7 +135,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:355`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:376`](../../packages/core/agent/src/types.ts) #### `agent/turn-continuation` — waterfall @@ -147,7 +147,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:368`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:389`](../../packages/core/agent/src/types.ts) ### `fs/*` @@ -261,23 +261,23 @@ Source: [`packages/subagent/subagent/src/index.ts:70`](../../packages/subagent/s #### `system-prompt/assemble` — waterfall -Waterfall around prompt assembly — mutate or extend the PromptAssembly (sections + tool schemas) before it is rendered. Bound to the SystemPrompt service; call `next()` to delegate. +Waterfall around prompt assembly — mutate or extend the PromptAssembly (sections + tools + variables) before it is rendered. Bound to the SystemPrompt service; call `next()` to delegate. ```ts cordis-catalog -'system-prompt/assemble'(this: SystemPrompt, assembly: PromptAssembly, next: () => Promise): Promise +'system-prompt/assemble'(this: SystemPrompt, assembly: PromptAssembly, context: AssembleContext, next: () => Promise): Promise ``` -Source: [`packages/core/system-prompt/src/index.ts:26`](../../packages/core/system-prompt/src/index.ts) +Source: [`packages/core/system-prompt/src/index.ts:32`](../../packages/core/system-prompt/src/index.ts) #### `system-prompt/change` — emit -A section or tool provider was registered or unregistered (the assembly inputs changed). +A section, tool provider, or variable provider was registered or unregistered (the assembly inputs changed). ```ts cordis-catalog 'system-prompt/change'(): void ``` -Source: [`packages/core/system-prompt/src/index.ts:32`](../../packages/core/system-prompt/src/index.ts) +Source: [`packages/core/system-prompt/src/index.ts:38`](../../packages/core/system-prompt/src/index.ts) ### `tools/*` @@ -490,15 +490,16 @@ Source: [`packages/subagent/subagent/src/index.ts:123`](../../packages/subagent/ ### `ctx.systemPrompt` — `SystemPrompt` -Registry service (`ctx.systemPrompt`): plugins contribute ordered text sections and tool-schema providers; the agent loop calls `assemble()` once per step. +Registry service (`ctx.systemPrompt`): plugins contribute ordered text sections, tool-schema providers, and named prompt variables; the agent loop calls `assemble(context)` once per step. ```ts cordis-catalog section(section: PromptSection): () => void tools(provider: () => ToolSchema[]): () => void -assemble(): Promise +variable(name: string, provider: (context: AssembleContext) => string | undefined): () => void +assemble(context: AssembleContext = {}): Promise ``` -Source: [`packages/core/system-prompt/src/index.ts:73`](../../packages/core/system-prompt/src/index.ts) +Source: [`packages/core/system-prompt/src/index.ts:149`](../../packages/core/system-prompt/src/index.ts) ### `ctx.tools` — `ToolRegistry` diff --git a/docs/core-data-structures/subagent.md b/docs/core-data-structures/subagent.md index 1d998b60b7..9ad875075c 100644 --- a/docs/core-data-structures/subagent.md +++ b/docs/core-data-structures/subagent.md @@ -75,12 +75,13 @@ interface SubagentRun { ## The provider seam: `SubagentProvider` -One transport for running a child agent. Implementations register under a unique name via `SubagentService.registerProvider`; multiple coexist in one context. The service validates every requested start-time capability before calling `start`, so an implementation may assume e.g. `request.maxDepth` is honorable when present. +One transport for running a child agent. Implementations register under a unique name via `SubagentService.registerProvider`; multiple coexist in one context. The service validates every requested start-time capability before calling `start`, so an implementation may assume e.g. `request.maxDepth` is honorable when present. `inheritsParentContext` is a DESCRIPTIVE fact beside the capabilities (nothing validates against it): whether a child sees the parent conversation (`fork`: true, `spawn`/`acp`: false) — the model-facing consumer derives truthful tool wording from it. ```ts type-equiv interface SubagentProvider { readonly name: string readonly capabilities: SubagentCapabilities + readonly inheritsParentContext: boolean start(request: SubagentStartRequest): SubagentRun } ``` diff --git a/docs/module-graph.md b/docs/module-graph.md index 50ef532fa4..c4e6fbdccf 100644 --- a/docs/module-graph.md +++ b/docs/module-graph.md @@ -21,6 +21,7 @@ graph TD agent --> brand agent --> llm agent --> session + agent --> system-prompt compact --> llm compact --> session fs-local --> fs @@ -70,6 +71,7 @@ graph TD tool-bash --> agent tool-bash --> bash tool-bash --> llm + tool-bash --> system-prompt tool-bash --> tools tool-fs --> fs tool-fs --> llm @@ -142,7 +144,7 @@ graph TD | `session` | `brand`, `llm` | | `system-prompt` | `llm` | | `web` | `llm` | -| `agent` | `brand`, `llm`, `session` | +| `agent` | `brand`, `llm`, `session`, `system-prompt` | | `compact` | `llm`, `session` | | `fs-local` | `fs` | | `fs-policy` | `fs` | @@ -162,7 +164,7 @@ graph TD | `agent-loop` | `agent`, `llm`, `session`, `session-persistence`, `system-prompt`, `tools` | | `hooks-codex` | `agent`, `hook-protocol`, `llm`, `session`, `tools` | | `subagent` | `agent`, `llm`, `tools` | -| `tool-bash` | `agent`, `bash`, `llm`, `tools` | +| `tool-bash` | `agent`, `bash`, `llm`, `system-prompt`, `tools` | | `tool-fs` | `fs`, `llm`, `session`, `system-prompt`, `tools` | | `tool-todo` | `agent`, `session`, `tools` | | `tool-web` | `llm`, `system-prompt`, `tools`, `web` | diff --git a/docs/rfc/README.md b/docs/rfc/README.md index 54fdd5de70..9752c22c08 100644 --- a/docs/rfc/README.md +++ b/docs/rfc/README.md @@ -152,6 +152,7 @@ Do NOT write one for a mechanical or local choice (a variable name, a one-file r | [Result-time applied-hunk diffs for file mutations](implemented/architecture/2026-07-02-result-time-applied-hunk-diffs.md) | 2026-07-02 | | [Tagged render-intent union for tool-call presentation](implemented/architecture/2026-07-02-tool-render-intent-union.md) | 2026-07-02 | | [Add direct directory listing to the filesystem seam](implemented/architecture/2026-07-03-filesystem-directory-listing-seam.md) | 2026-07-03 | +| [Prompt variables and tool-guidance ownership](implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md) | 2026-07-05 | ### Process diff --git a/docs/rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md b/docs/rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md new file mode 100644 index 0000000000..14b9990881 --- /dev/null +++ b/docs/rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md @@ -0,0 +1,69 @@ +# RFC: Prompt variables and tool-guidance ownership + +Status: implemented (proposed and accepted 2026-07-05) + +## Problem + +The assembled system prompt had four defects, all of one family: facts the harness already knows were restated by hand somewhere else, and drifted. + +**The model could not know its own name.** `AgentOptions.model` drives every request, but no prompt text carried it — and nothing COULD carry it: sections in `dsh-system-prompt` were context-global while the model name is per-agent, and `assemble()` took no per-agent input at all. + +**Tool guidance was hand-written prose in leaf YAML.** The bash/subagent/todo_write usage guidance lived in the `systemPrompt` strings of `examples/coding-agent/cordis.yml` and `examples/acp-agent/cordis.yml` — two drifting copies (the ACP one was already abridged) — while `dsh-tool-fs` and `dsh-tool-web` owned their guidance as `ctx.systemPrompt.section()` contributions. Loading or dropping a tool plugin meant editing every deployment's persona by hand; both YAMLs carried a `FIXME(config-comments)` apologizing for a symptom of the split, and the stdio welcome banner hand-enumerated the tool set too. + +**The persona rendered after tool guidance.** The loop string-joined `agent.options.systemPrompt` AFTER the assembled sections, so the model read "Use the read tool…" before "You are coding-agent" — backwards relative to the identity-first convention (Claude Code, Codex) and a second composition path besides the section pipeline. + +**The fork tool's description was false.** `dsh-tool-subagent` hardcoded one description written for spawn semantics — "a separate agent that works in its own context … it does not see this conversation" — and the `subagent_fork` instance (whose child inherits the parent's completed turns) got the same words; the YAML prose corrected the lie out-of-band. Minor kin: `PromptSection.name` was documented "(diagnostics / dedup)" but duplicates were silently accepted. + +## Decision + +**One principle: every fact in the prompt has exactly one owner.** The model name and workspace are config/session facts → the harness exposes them as variables and the persona references them. Per-tool semantics and when-to-use → the tool's `description`. Cross-call habits a description cannot carry → the tool package's prompt section. Identity and behavior → the deployment's persona, and nothing else. + +### Assemble context + +`SystemPrompt.assemble(context)` takes an `AssembleContext` — declared EMPTY and merge-extensible in `dsh-system-prompt` (the package stays agnostic of who assembles); `dsh-agent` declaration-merges `agent?: Agent` onto it (a new type-level edge `agent → system-prompt`, no cycle — `tools` already depends on both). The loop passes `{ agent }` each step; section text providers become `string | ((context) => string)` (zero-arg providers stay valid), and the `system-prompt/assemble` waterfall gains the context parameter so a listener can filter or extend per agent. + +### Prompt variables + +Plugins contribute named values via `ctx.systemPrompt.variable(name, provider)`; prompt text references them as `{{name}}`. Providers are functions of the `AssembleContext` and may return `undefined` — "no value for THIS assembly". `assemble()` resolves every registered variable into `PromptAssembly.variables` (waterfall listeners can see, add, or override); `renderPrompt` interpolates. Rendering is STRICT — fail loud beats shipping a malformed prompt: a reference to an unregistered name throws (listing what exists), a registered-but-valueless reference throws, and a complete `{{…}}` group that is not a well-formed name (`[a-z][a-z0-9_]*`, e.g. `{{ model }}`) throws. Only complete double-brace groups are interpreted; a lone `{{` passes through verbatim. Registration rejects duplicate and unreferenceable names, mirroring the tool registry — and `section()` now rejects duplicate section names, making the documented dedup real. + +`dsh-agent-loop` registers the two built-ins, both pure projections of the context agent: `model` (= `options.model`) and `cwd` (= `session.header.cwd`). The example personas write `powered by the {{model}} model` — the model name is stated once, in the `model:` config key. `{{cwd}}` is demonstrated in the ACP example only: every ACP session carries the client's cwd, while config-pre-created stdio agents have none (a persona claiming `{{cwd}}` there fails the turn — by design). + +### Persona as the order-0 section + +The loop plugin registers ONE section, `agent:persona` at order 0, whose text is `context.agent?.options.systemPrompt ?? ''`. The loop's special-case join is deleted: `fullSystemPrompt ≡ renderPrompt(assembly)`, one ordered pipeline for everything the model sees, and `agent/pre-step` (compaction's token-pressure input) measures exactly the real prompt. Order bands are now convention: persona `0`, tool guidance `100–199`, negative orders render before the persona. `AgentOptions.systemPrompt` keeps its familiar key but is documented as what it is — the persona template fragment, one section of the full prompt, never the whole (see `CONTEXT.md`). + +### Tool guidance ownership + +Per-tool semantics and when-to-use live in tool DESCRIPTIONS, which already ship in every request — the YAML prose was ~fully redundant with them. Sections carry only the cross-call habits a single call's description cannot: `dsh-tool-bash` contributes `tool:bash` (order 105) — check the `[exit code: N]` marker on every result; `dsh-tool-fs`'s read section gains the "not shell commands like cat" contrast. `todo_write` and the subagent tools need NO section — their descriptions already carry the whole contract. The leaf personas shrink to identity + behavior (verify your work; keep answers brief), and the welcome banner stops enumerating tools. + +### The subagent context contract + +`SubagentProvider` gains `readonly inheritsParentContext: boolean` — a DESCRIPTIVE fact beside `capabilities`, not in it (capabilities are start-time validation; nothing validates against this flag). Spawn and ACP declare `false`, fork declares `true`. `dsh-tool-subagent` derives both the tool description and the `prompt` parameter description from the flag (`providerWording`): the fork instance now tells the model the child inherits the conversation's completed turns (not the in-flight turn) and that its prompt should state only what is new. `apply` resolves the provider at LOAD time and throws if it is not registered — the backend plugin must be listed before the tool plugin in `cordis.yml`; a wiring mistake fails loudly at boot instead of shipping a lying description. + +## Rejected alternatives + +- **The loop composes an identity line itself** — hardcodes model-facing prose in the one package that must stay thin ("plugins, not loop changes"), and contradicts `dsh-system-prompt`'s "no hardcoded prompt text" stance. +- **Inject the model name via the `agent/request` waterfall** — prompt text composed in two places, and `agent/pre-step`'s `fullSystemPrompt` would omit it, so compaction would measure a prompt that is not what the model sees. +- **Hand-write the model name in each persona** — duplicates the `model:` key one line above and silently lies after a config edit; the exact disease this RFC cures. +- **Lenient interpolation (leave unknown refs verbatim, or substitute empty)** — a typo ships `{{modle}}` (or a hole) to the model and nobody notices until transcript review. +- **Per-instance subagent wording in config** — returns model-facing prose to every deployment × instance, the P2 disease again. **Keying wording off the provider NAME** — `providerName` is itself config, so a renamed provider silently gets the wrong words. +- **Resolving the subagent flag lazily (section-only wording)** — would tolerate any load order, but the DESCRIPTION is fixed at tool registration and is where tool-choice guidance belongs; a deterministic load-order requirement with a loud, actionable failure is the smaller cost. + +## What we give up + +- `{{model}}` reflects `AgentOptions.model` at assembly time. A plugin that switches models in the `agent/request` waterfall makes the prompt's claim stale for that step; such a plugin can rewrite `options.system` in the same waterfall if it cares. Accepted. +- `dsh-tool-subagent` now has a hard load-order requirement on its backend. The examples already ordered backends first; the failure mode is an immediate boot error naming the fix. +- Strictness means a persona can fail a turn at render (e.g. `{{cwd}}` on a cwd-less session). The failure is contained — the turn ends `error`, the loop survives — and it is an authoring error we WANT loud. +- No escape syntax for a literal `{{name}}` in prompt prose yet; add one if a real prompt ever needs it. + +## Out of scope + +- Further variables (`date`, platform, git state) — the registry makes each a one-line contribution by whichever plugin owns the fact; none is claimed here. +- A config `cwd` for pre-created stdio agents (would let the stdio persona use `{{cwd}}` and partition persistence by real path) — deferred until the session-cwd story is revisited. + +## Acceptance criteria + +- `renderPrompt(assemble({agent}))` for the coding-agent example contains the persona FIRST (with the agent's model name interpolated), then fs/bash/web guidance sections; the loop contains no other prompt-composition path. +- The `subagent_fork` schema description says the child inherits the conversation; the `subagent` one says it does not. Loading `dsh-tool-subagent` before its backend fails at load with a message naming the ordering fix. +- Unknown/valueless/malformed `{{…}}` references throw with the section name in the message; duplicate section, variable, and tool-name registrations all throw. +- The gating runs (`test:coverage`, `test:snapshot`, `doc-sync`, `build`, `hygiene`) are green; no golden re-record is needed (replay never re-verifies the outgoing request). diff --git a/examples/acp-agent/cordis.yml b/examples/acp-agent/cordis.yml index 01849bb66e..ca85698c36 100644 --- a/examples/acp-agent/cordis.yml +++ b/examples/acp-agent/cordis.yml @@ -23,9 +23,8 @@ - deepseek-v4-flash - deepseek-v4-pro -# Local bash executor for agent-core's tool-bash schema. -# FIXME(config-comments): keep this executor note from implying bash is the -# whole tool set; filesystem, subagent, and todo_write are loaded below. +# Local bash executor for agent-core's tool-bash schema (one of several tool +# stacks in this tree: filesystem, subagent, and todo_write load below). - id: bash name: '@deepseek-ai/dsh-bash-local' config: @@ -39,28 +38,16 @@ config: model: deepseek-v4-flash persistenceRoot: !!js process.env.DSH_SNAPSHOT_SESSIONS_ROOT ?? './.sessions' + # The persona: identity + behavior only. Tool guidance lives with each tool + # plugin (descriptions + prompt sections); {{model}} and {{cwd}} are prompt + # variables the agent loop resolves per session (every ACP session carries + # the client's cwd, so the persona can state the workspace). systemPrompt: | - You are a coding assistant driven over the Agent Client Protocol. + You are a coding assistant powered by the {{model}} model, driven over + the Agent Client Protocol. Your working directory is {{cwd}}. - Your tools are read/write/edit for file operations, bash (plus - bash_output/bash_kill for background tasks), and subagent. Use read to - inspect UTF-8 text files, write to create or replace files, and edit for - targeted literal replacements. Use bash for shell commands, tests, - searches, and operations that are not ordinary file reads or edits. Each - bash call runs in a fresh shell — pass workdir instead of cd. Check the - [exit code: N] marker; verify your work. Keep answers brief and factual. - - Use the subagent tool to delegate a focused, self-contained subtask to - a fresh child agent (it works in its own context and returns only its - final result) — give it a complete, standalone instruction. Use - subagent_fork instead when the subtask needs THIS conversation's - context: the child inherits the log so far. - - For multi-step work, use the todo_write tool to track a task list: - send the WHOLE list each call (it replaces the previous one), keep at - most one task in_progress (exactly one while work remains), and mark a - task completed as soon as it is done. Skip it for trivial single-step - tasks. + Verify your work by running the code or tests. Keep answers brief and + factual. # The subagent seam + both in-process backends + two model-facing tools, as leaf # entries after the app (which provides ctx.agents/ctx.tools). spawn (a fresh diff --git a/examples/coding-agent/cordis.yml b/examples/coding-agent/cordis.yml index 0c95299fca..1980c10aa6 100644 --- a/examples/coding-agent/cordis.yml +++ b/examples/coding-agent/cordis.yml @@ -28,9 +28,8 @@ - deepseek-v4-pro - deepseek-v4-flash -# Local bash executor for agent-core's tool-bash schema. -# FIXME(config-comments): keep this executor note from implying bash is the -# whole tool set; filesystem, subagent, and todo_write are loaded below. +# Local bash executor for agent-core's tool-bash schema (one of several tool +# stacks in this tree: filesystem, subagent, and todo_write load below). - id: bash name: '@deepseek-ai/dsh-bash-local' config: @@ -46,33 +45,15 @@ # under ./.sessions); unset starts a fresh session each run. resumeSessionId: !!js process.env.RESUME_SESSION_ID persistenceRoot: './.sessions' - welcome: 'agent REPL ready. Give it a coding task (its tools are read, write, edit, bash, subagent, and todo_write).' + welcome: 'agent REPL ready. Give it a coding task.' + # The persona: identity + behavior only. Tool guidance lives with each tool + # plugin (descriptions + prompt sections); {{model}} is the prompt variable + # the agent loop resolves from this agent's configured model. systemPrompt: | - You are coding-agent, a CLI coding assistant. + You are coding-agent, a CLI coding assistant powered by the {{model}} model. - Your tools are read/write/edit for file operations, bash (plus - bash_output/bash_kill for background tasks), and subagent. Use read to - inspect UTF-8 text files, write to create or replace files, and edit for - targeted literal replacements. Use bash for shell commands, tests, - searches, and operations that are not ordinary file reads or edits. Each - bash call runs in a fresh shell — pass workdir instead of cd, and never - rely on shell state between calls. - - Use the subagent tool to delegate a focused, self-contained subtask - to a fresh child agent (it works in its own context and returns only - its final result) — give it a complete, standalone instruction. Use - subagent_fork instead when the subtask needs THIS conversation's - context: the child inherits the log so far. - - Check the [exit code: N] marker on every command; investigate - failures before moving on. Verify your work by running the code or - tests. Keep answers brief and factual. - - For multi-step work, use the todo_write tool to track a task list: - send the WHOLE list each call (it replaces the previous one), keep at - most one task in_progress (exactly one while work remains), and mark a - task completed as soon as it is done. Skip it for trivial single-step - tasks. + Verify your work by running the code or tests. Keep answers brief and + factual. # Automatic context compaction: when the derived history approaches the model's # context window, summarize an older range into a checkpoint so a long-running diff --git a/packages/bash/tool-bash/README.md b/packages/bash/tool-bash/README.md index 436e3f034a..eabb9298aa 100644 --- a/packages/bash/tool-bash/README.md +++ b/packages/bash/tool-bash/README.md @@ -2,7 +2,9 @@ The model-facing bash tools — `bash`, `bash_output`, `bash_kill` — registered over the `ctx.bash` executor seam (`@deepseek-ai/dsh-bash`). Pure schema + text shaping; every process concern lives behind the seam, so sandboxed or remote executor implementations swap in without changing what the model sees. -Requires a loaded executor implementation (e.g. `@deepseek-ai/dsh-bash-local`); the plugin stays pending until `ctx.bash` exists (`inject: ['tools', 'bash']`). +Requires a loaded executor implementation (e.g. `@deepseek-ai/dsh-bash-local`); the plugin stays pending until `ctx.bash` exists (`inject: ['tools', 'bash', 'systemPrompt']`). + +The plugin also contributes the `tool:bash` prompt section (order 105) — the cross-call habit the per-tool descriptions cannot carry: check the `[exit code: N]` marker on every result and investigate failures before moving on. ## Tools diff --git a/packages/bash/tool-bash/package.json b/packages/bash/tool-bash/package.json index f9092fabb6..d8836d6a21 100644 --- a/packages/bash/tool-bash/package.json +++ b/packages/bash/tool-bash/package.json @@ -25,6 +25,7 @@ "@deepseek-ai/dsh-agent": "^0.0.1", "@deepseek-ai/dsh-bash": "^0.0.1", "@deepseek-ai/dsh-llm": "^0.0.1", + "@deepseek-ai/dsh-system-prompt": "^0.0.1", "@deepseek-ai/dsh-tools": "^0.0.1", "cordis": "^4.0.0-rc.6" }, diff --git a/packages/bash/tool-bash/src/index.ts b/packages/bash/tool-bash/src/index.ts index 88d14cc0f0..e208fbeb06 100644 --- a/packages/bash/tool-bash/src/index.ts +++ b/packages/bash/tool-bash/src/index.ts @@ -43,11 +43,12 @@ import { isAbsolute, resolve as resolvePath } from 'node:path' import { defineTool } from '@deepseek-ai/dsh-tools' import type { GenericCallView, TerminalCallView, ToolResult, ToolResultView } from '@deepseek-ai/dsh-tools' import type { Agent } from '@deepseek-ai/dsh-agent' +import type {} from '@deepseek-ai/dsh-system-prompt' import { BashTaskId, OwnerToken } from '@deepseek-ai/dsh-bash' import type { BashRunResult, BashTask, CollectedOutput } from '@deepseek-ai/dsh-bash' export const name = 'tool-bash' -export const inject = ['tools', 'bash'] +export const inject = ['tools', 'bash', 'systemPrompt'] /** * Validate the constraints the SchemaSpec can't express. `defineTool` now @@ -285,6 +286,15 @@ function statusLine(task: BashTask): string { } export function apply(ctx: Context): void { + // The bash tools' cross-call HABIT, which the per-tool descriptions cannot + // carry (they describe one call each): the exit-code marker is only useful + // if the model actually checks it every time. + ctx.systemPrompt.section({ + name: 'tool:bash', + order: 105, + text: 'Check the [exit code: N] marker on every bash result; investigate failures before moving on.', + }) + /** * The caller's owner TOKEN — the owning agent's `session.header.id`, or * `undefined` for a non-agent caller. Read `session.header.id` (NOT diff --git a/packages/bash/tool-bash/tests/tools.spec.ts b/packages/bash/tool-bash/tests/tools.spec.ts index 0187302ef1..f6a985f594 100644 --- a/packages/bash/tool-bash/tests/tools.spec.ts +++ b/packages/bash/tool-bash/tests/tools.spec.ts @@ -261,6 +261,14 @@ describe('bash tool', () => { }) }) + it('contributes the exit-code habit as its prompt section (guidance the descriptions cannot carry)', async () => { + const ctx = await setup() + const assembly = await ctx.systemPrompt.assemble() + const section = assembly.sections.find(s => s.name === 'tool:bash') + expect(section?.order).toBe(105) + expect(section?.text).toContain('[exit code: N]') + }) + it('unregisters everything when the plugin fiber is disposed (HMR safety)', async () => { const ctx = new Context() await ctx.plugin(SystemPrompt) @@ -268,8 +276,10 @@ describe('bash tool', () => { await ctx.plugin(LocalBashExecutor, {}) const fiber = await ctx.plugin(ToolBash) expect(ctx.tools.schemas()).toHaveLength(3) + expect((await ctx.systemPrompt.assemble()).sections.map(s => s.name)).toEqual(['tool:bash']) await fiber.dispose() expect(ctx.tools.schemas()).toHaveLength(0) + expect((await ctx.systemPrompt.assemble()).sections).toHaveLength(0) }) it('tools depend on the executor: no registration without ctx.bash', async () => { diff --git a/packages/core/agent-loop/README.md b/packages/core/agent-loop/README.md index 60b8a5d779..fec733c34e 100644 --- a/packages/core/agent-loop/README.md +++ b/packages/core/agent-loop/README.md @@ -28,12 +28,12 @@ interface Config { agents: Array<{ id: string // required model?: string - systemPrompt?: string + systemPrompt?: string // the agent's persona TEMPLATE (may reference {{model}}/{{cwd}}) }> } ``` -Agents listed in config are auto-created at startup. +Agents listed in config are auto-created at startup. The plugin also registers the per-agent prompt pieces on `ctx.systemPrompt`: the `agent:persona` section (order 0 — `AgentOptions.systemPrompt` renders before all tool guidance) and the built-in `model`/`cwd` prompt variables, each resolved per step from the `assemble({ agent })` context. ### Classes @@ -55,7 +55,7 @@ forever: if every prompt blocked: 'turn/end'(rejected), no step ⟵ zero-step turn STEP loop: drain steering - assembly = systemPrompt.assemble() + assembly = systemPrompt.assemble({agent}) ⟵ renderPrompt(assembly) IS the full prompt await serial agent/pre-step ⟵ surface mutation (compaction) outside the step session('step/start') request = waterfall agent/request diff --git a/packages/core/agent-loop/src/index.ts b/packages/core/agent-loop/src/index.ts index 33672dc9b4..8bc35a8f93 100644 --- a/packages/core/agent-loop/src/index.ts +++ b/packages/core/agent-loop/src/index.ts @@ -82,6 +82,20 @@ export class AgentLoop extends Service implements AgentFactory { // Provide the agent-creation factory to the registry (effect-scoped: the // slot is cleared on dispose). ctx.effect(() => this.ctx.agents.setFactory(this), 'agentLoop.setFactory()') + // The per-agent prompt pieces, registered once and resolved per assembly + // from the AssembleContext the loop passes (loop.ts assembles with + // `{ agent }` each step). The persona is the order-0 section — identity + // renders before all tool guidance; `{{model}}`/`{{cwd}}` are the built-in + // prompt variables projecting the agent's configured model and its + // session workspace. A provider returns undefined when the fact is absent + // (renderPrompt then rejects a persona that claims it — fail loud). + ctx.systemPrompt.section({ + name: 'agent:persona', + order: 0, + text: context => context.agent?.options.systemPrompt ?? '', + }) + ctx.systemPrompt.variable('model', context => context.agent?.options.model) + ctx.systemPrompt.variable('cwd', context => context.agent?.session.header.cwd) for (const { id, resumeSessionId, ...options } of config.agents) { if (resumeSessionId !== undefined && resumeSessionId !== '') { // Resume a prior session instead of starting fresh. resume() needs diff --git a/packages/core/agent-loop/src/loop.ts b/packages/core/agent-loop/src/loop.ts index ef5c50b7ce..0b84bcb83f 100644 --- a/packages/core/agent-loop/src/loop.ts +++ b/packages/core/agent-loop/src/loop.ts @@ -152,7 +152,8 @@ export interface LoopHandle { * every prompt blocked → 'turn/end'(rejected), 0 steps * STEP loop: * drain steering → session('steering/message') ⟵ catches late steering - * assembly = ctx.systemPrompt.assemble() ⟵ waterfall system-prompt/assemble + * assembly = ctx.systemPrompt.assemble({agent}) ⟵ waterfall system-prompt/assemble; renderPrompt + * (persona section + {{variables}}) IS the full prompt * await ctx.serial('agent/pre-step') ⟵ surface mutation (compaction) OUTSIDE the step * session('step/start') ⟵ durable step boundary (no agent/* mirror) * req = {model, system, tools, messages: session.deriveMessages(), signal} @@ -434,11 +435,11 @@ async function runTurn(ctx: Context, agent: ReactLoopAgent, handle: LoopHandle, // because the pre-step seam needs it: compaction measures token pressure // against the system prompt (it counts toward the budget). runStep reuses // this same assembly for the request, so the prompt is assembled once per - // step. - const assembly = await ctx.systemPrompt.assemble() - const fullSystemPrompt = [renderPrompt(assembly), agent.options.systemPrompt ?? ''] - .filter(text => text.length > 0) - .join('\n\n') + // step. renderPrompt IS the full prompt — the persona is the order-0 + // section (registered by the AgentLoop plugin) and `{{variable}}` + // interpolation happens in the render, so there is no separate join. + const assembly = await ctx.systemPrompt.assemble({ agent }) + const fullSystemPrompt = renderPrompt(assembly) // Interruption landing after assembly: dispose() or cancel() in a // turn-start listener (or a listener whose promise resolved before the diff --git a/packages/core/agent-loop/tests/loop.spec.ts b/packages/core/agent-loop/tests/loop.spec.ts index 934c19953a..014ddb548c 100644 --- a/packages/core/agent-loop/tests/loop.spec.ts +++ b/packages/core/agent-loop/tests/loop.spec.ts @@ -141,10 +141,10 @@ describe('agent loop', () => { .toEqual({ diffs: [{ path: 'a.txt', oldText: null, newText: 'x' }] }) }) - it('passes assembled system prompt and tool schemas into the request', async () => { + it('renders the persona as the order-0 section — before tool guidance — with {{variables}} resolved', async () => { const adapter = new MockAdapter([textResponse('ok')]) const ctx = await harness(adapter) - ctx.systemPrompt.section({ name: 'persona', order: 0, text: 'You are a test agent.' }) + ctx.systemPrompt.section({ name: 'tool:noop', order: 100, text: 'Use the noop tool wisely.' }) ctx.tools.register(defineTool({ name: 'noop', description: 'does nothing', @@ -153,16 +153,55 @@ describe('agent loop', () => { return [] }, })) - const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock', systemPrompt: 'Agent-specific suffix.' }) + // The persona is a TEMPLATE: {{model}} is the loop-registered variable + // projecting this agent's configured model, so the model knows its own name. + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock', systemPrompt: 'You are a test agent on {{model}}.' }) send(agent, 'hi') await waitForIdle(ctx, agent) const request = adapter.requests[0] - expect(request!.system).toBe('You are a test agent.\n\nAgent-specific suffix.') + expect(request!.system).toBe('You are a test agent on mock.\n\nUse the noop tool wisely.') expect(request!.tools?.map(t => t.name)).toEqual(['noop']) }) + it('resolves {{cwd}} from the agent session workspace (factory create with meta.cwd)', async () => { + const adapter = new MockAdapter([textResponse('ok')]) + const ctx = await harness(adapter) + const handle = ctx.agents.create({ + agentId: AgentId('a-cwd'), + sessionId: SessionId('s-cwd'), + meta: { cwd: '/work/space' }, + agentOptions: { model: 'mock', systemPrompt: 'Working in {{cwd}}.' }, + }) + + const agent = handle.agent as ReactLoopAgent + send(agent, 'hi') + await waitForIdle(ctx, agent) + + expect(adapter.requests[0]!.system).toBe('Working in /work/space.') + }) + + it('contains a strict-variable render failure: the turn errors, the loop survives', async () => { + // A persona claiming {{cwd}} on a session with NO cwd is a deployment + // authoring error — renderPrompt throws, the turn ends with an error, and + // the agent (and loop) stay alive for the next prompt. + const adapter = new MockAdapter([textResponse('never reached'), textResponse('ok')]) + const ctx = await harness(adapter) + const errors: Error[] = [] + ctx.on('agent/error', (_agent, _turn, _step, error) => void errors.push(error)) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock', systemPrompt: 'In {{cwd}}.' }) + + send(agent, 'hi') + await waitForIdle(ctx, agent) + + expect(adapter.requests).toHaveLength(0) // the request was never sent + expect(errors.some(e => e.message.includes('no value for this assembly'))).toBe(true) + const turnEnd = agent.session.events.find(e => e.type === 'turn/end') + expect(turnEnd?.type === 'turn/end' && turnEnd.data.reason.kind).toBe('error') + expect(agent.status).toBe('idle') // contained: the loop is still serving + }) + it('records raw chunks for replay as assistant/chunk session events', async () => { const adapter = new MockAdapter([textResponse('abc')]) const ctx = await harness(adapter) diff --git a/packages/core/agent-loop/tests/review-fixes.spec.ts b/packages/core/agent-loop/tests/review-fixes.spec.ts index 76c377fd61..21be9e8267 100644 --- a/packages/core/agent-loop/tests/review-fixes.spec.ts +++ b/packages/core/agent-loop/tests/review-fixes.spec.ts @@ -1016,7 +1016,7 @@ describe('disposal/cancel honored during pre-step assembly (P1-1)', () => { ctx.llm.registerAdapter(['mock'], adapter) // Blocking listener on the parent context (survives fiber disposal). - const unlisten = ctx.on('system-prompt/assemble', async function (_assembly, next) { + const unlisten = ctx.on('system-prompt/assemble', async function (_assembly, _context, next) { await blocked return next() }) @@ -1072,7 +1072,7 @@ describe('disposal/cancel honored during pre-step assembly (P1-1)', () => { await ctx.plugin(Invariants, { freeze: false }) ctx.llm.registerAdapter(['mock'], adapter) - const unlisten = ctx.on('system-prompt/assemble', async function (_assembly, next) { + const unlisten = ctx.on('system-prompt/assemble', async function (_assembly, _context, next) { await blocker return next() }) @@ -1229,7 +1229,7 @@ describe('disposal/cancel honored during pre-step assembly (P1-1)', () => { await ctx.plugin(Invariants, { freeze: false }) ctx.llm.registerAdapter(['mock'], adapter) - ctx.on('system-prompt/assemble', async function (_assembly, next) { + ctx.on('system-prompt/assemble', async function (_assembly, _context, next) { await blocker return next() }) diff --git a/packages/core/agent/package.json b/packages/core/agent/package.json index fa6946b8cf..e38a6c8d61 100644 --- a/packages/core/agent/package.json +++ b/packages/core/agent/package.json @@ -25,12 +25,14 @@ "@deepseek-ai/dsh-brand": "^0.0.1", "@deepseek-ai/dsh-llm": "^0.0.1", "@deepseek-ai/dsh-session": "^0.0.1", + "@deepseek-ai/dsh-system-prompt": "^0.0.1", "cordis": "^4.0.0-rc.6" }, "devDependencies": { "@deepseek-ai/dsh-brand": "workspace:^", "@deepseek-ai/dsh-llm": "workspace:^", "@deepseek-ai/dsh-session": "workspace:^", + "@deepseek-ai/dsh-system-prompt": "workspace:^", "cordis": "^4.0.0-rc.6" } } diff --git a/packages/core/agent/src/types.ts b/packages/core/agent/src/types.ts index 2a5d713b85..26dde9c764 100644 --- a/packages/core/agent/src/types.ts +++ b/packages/core/agent/src/types.ts @@ -45,6 +45,7 @@ import type { Branded } from '@deepseek-ai/dsh-brand' import type { ContentBlock, GenerateOptions, Message, MessageSource } from '@deepseek-ai/dsh-llm' +import type {} from '@deepseek-ai/dsh-system-prompt' /** Identifies one live agent in the registry. */ export type AgentId = Branded<'AgentId'> @@ -55,6 +56,20 @@ export function AgentId(id: string): AgentId { } import type { Session } from '@deepseek-ai/dsh-session' +declare module '@deepseek-ai/dsh-system-prompt' { + interface AssembleContext { + /** + * The agent this assembly is for. The agent loop passes it on every + * per-step `assemble({ agent })`; section text and variable providers + * project per-agent facts from it (`options.systemPrompt` → the persona + * section, `options.model` → `{{model}}`, `session.header.cwd` → + * `{{cwd}}`). Optional because a bare `assemble()` (tests, diagnostics) + * has no agent — providers must tolerate its absence. + */ + agent?: Agent + } +} + /** * Options an agent is created with. * Merge-extensible: plugins declare extra fields via declaration merging. @@ -62,7 +77,13 @@ import type { Session } from '@deepseek-ai/dsh-session' export interface AgentOptions { /** Model name (must have a registered adapter at call time). */ model?: string - /** Per-agent system prompt appended after the assembled sections. */ + /** + * The agent's persona: a deployment-authored prompt-template fragment, + * rendered as the order-0 section of the assembled system prompt (before + * all tool guidance). It may reference registered `{{variables}}` (e.g. + * `{{model}}`, `{{cwd}}`); it is one section of the full prompt, never the + * whole. + */ systemPrompt?: string } diff --git a/packages/core/agent/tsconfig.json b/packages/core/agent/tsconfig.json index 4d23ac46d3..7f4f457598 100644 --- a/packages/core/agent/tsconfig.json +++ b/packages/core/agent/tsconfig.json @@ -22,6 +22,9 @@ }, { "path": "../../core/session" + }, + { + "path": "../../core/system-prompt" } ] } diff --git a/packages/core/system-prompt/README.md b/packages/core/system-prompt/README.md index 1c18bdf1d6..28d382f54a 100644 --- a/packages/core/system-prompt/README.md +++ b/packages/core/system-prompt/README.md @@ -1,37 +1,42 @@ # dsh-system-prompt -System prompt assembly registry. Plugins contribute ordered text sections and tool-schema providers; the agent loop calls `assemble()` once per step. +System prompt assembly registry. Plugins contribute ordered text sections, tool-schema providers, and named prompt variables; the agent loop calls `assemble(context)` once per step, and `renderPrompt(assembly)` is the full system prompt the model sees. ## Service: `SystemPrompt` (ctx key: `systemPrompt`) ### Public API -- `ctx.systemPrompt.section(section: PromptSection): () => void` Contribute a section. Disposed with the calling fiber. +- `ctx.systemPrompt.section(section: PromptSection): () => void` Contribute a section. Duplicate names throw. Disposed with the calling fiber. - `ctx.systemPrompt.tools(provider: () => ToolSchema[]): () => void` Contribute tool schemas (evaluated at each assembly). Disposed with the calling fiber. -- `ctx.systemPrompt.assemble(): Promise` Assemble the current prompt. Runs through the `system-prompt/assemble` waterfall. +- `ctx.systemPrompt.variable(name: string, provider: (context) => string | undefined): () => void` Contribute a prompt variable, referenced from section text as `{{name}}`. Duplicate or unreferenceable names throw; `undefined` means "no value for this assembly". Disposed with the calling fiber. +- `ctx.systemPrompt.assemble(context?: AssembleContext): Promise` Assemble the prompt for one caller. Runs through the `system-prompt/assemble` waterfall. ### Events | Event | Mode | Purpose | |---|---|---| -| `system-prompt/assemble` | waterfall | Mutate/extend the assembly before it reaches the model | -| `system-prompt/change` | emit | A section or tool provider was registered or unregistered | +| `system-prompt/assemble` | waterfall | Mutate/extend the assembly (with the caller's context) before it reaches the model | +| `system-prompt/change` | emit | A section, tool provider, or variable was registered or unregistered | ### Key types -- `PromptSection` — `{ name, order, text: string | (() => string) }`. Sections are concatenated in ascending `order`. -- `PromptAssembly` — `{ sections: PromptSection[], tools: ToolSchema[] }`. Tool schemas are part of the assembly by design: "what the model is told it can do" is one coherent thing, even though adapters transmit schemas as a separate wire field. -- `renderPrompt(assembly)` — joins section texts with blank lines. +- `AssembleContext` — what one `assemble()` call is FOR. Declared empty here and merge-extensible; `dsh-agent` declares `agent?: Agent`, so providers project per-agent facts. Providers must tolerate absent fields (a bare `assemble()` carries an empty context). +- `PromptSection` — `{ name, order, text: string | ((context) => string) }`. Sections are concatenated in ascending `order`. Order bands: `0` is the per-agent persona (registered by the agent loop), tool guidance uses `100–199`; negative orders render before the persona. +- `PromptAssembly` — `{ sections: AssembledSection[], tools: ToolSchema[], variables: Record }`. Section texts arrive resolved but not yet interpolated; `variables` holds every registered variable resolved against the context. Tool schemas are part of the assembly by design: "what the model is told it can do" is one coherent thing, even though adapters transmit schemas as a separate wire field. +- `renderPrompt(assembly)` — interpolates `{{variable}}` references in each section, drops empty sections, joins with blank lines. STRICT: an unknown reference, a registered-but-valueless reference, or a malformed complete `{{…}}` group throws (fail loud beats shipping a malformed prompt). Only complete double-brace groups are interpreted; a lone `{{` passes through verbatim. -Merge-extensible: plugins can declare extra fields on `PromptAssembly` via declaration merging. +Merge-extensible: plugins can declare extra fields on `PromptAssembly` and `AssembleContext` via declaration merging. ### Extension points -- Section providers: AGENTS.md reader, cwd notifier, persona config, etc. +- Section providers: tool packages own their cross-call guidance (`tool:bash`, `tool:read`, …); the agent loop owns `agent:persona`. +- Variable providers: the agent loop registers `model` and `cwd`; any plugin can register the facts it owns (a future `date`, git state, …). - Tool schema providers: `ToolRegistry` registers itself as a tool provider automatically. -- The `system-prompt/assemble` waterfall: mutate or replace the assembly (system-prompt configurability, dynamic tool filtering). +- The `system-prompt/assemble` waterfall: mutate or replace the assembly per caller (dynamic tool filtering, extra variables). ### What is NOT here -- Any hardcoded prompt text — every section comes from plugins. +- Any hardcoded prompt text — every section comes from plugins, every deployment-authored word from config. - Prompt compaction (belongs on the `agent/pre-step` seam in `dsh-agent`). + +Design rationale: [the prompt-variables RFC](../../../docs/rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md). diff --git a/packages/core/system-prompt/src/index.ts b/packages/core/system-prompt/src/index.ts index 37dc10b9ee..3faf2373fa 100644 --- a/packages/core/system-prompt/src/index.ts +++ b/packages/core/system-prompt/src/index.ts @@ -1,7 +1,8 @@ /** - * System prompt assembly registry. Plugins contribute ordered text sections and - * tool schema providers; `assemble()` collates them through a waterfall that - * runs once per step. + * System prompt assembly registry. Plugins contribute ordered text sections, + * tool schema providers, and named prompt variables; `assemble(context)` + * collates them through a waterfall that runs once per step, and + * `renderPrompt` interpolates `{{variable}}` references into the final text. * * @module @deepseek-ai/dsh-system-prompt */ @@ -17,30 +18,63 @@ declare module 'cordis' { interface Events { /** * Waterfall around prompt assembly — mutate or extend the - * {@link PromptAssembly} (sections + tool schemas) before it is rendered. - * Bound to the {@link SystemPrompt} service; call `next()` to delegate. - * @param assembly - the assembly built from the registered sections and - * tool providers; listeners may mutate it or return a replacement. + * {@link PromptAssembly} (sections + tools + variables) before it is + * rendered. Bound to the {@link SystemPrompt} service; call `next()` to + * delegate. + * @param assembly - the assembly built from the registered sections, tool + * providers, and variable providers; listeners may mutate it or return a + * replacement. + * @param context - the per-assembly {@link AssembleContext} the caller + * passed to {@link SystemPrompt.assemble} (e.g. which agent the prompt + * is for), so a listener can filter or extend per agent. * @mode waterfall */ - 'system-prompt/assemble'(this: SystemPrompt, assembly: PromptAssembly, next: () => Promise): Promise + 'system-prompt/assemble'(this: SystemPrompt, assembly: PromptAssembly, context: AssembleContext, next: () => Promise): Promise /** - * A section or tool provider was registered or unregistered (the assembly - * inputs changed). + * A section, tool provider, or variable provider was registered or + * unregistered (the assembly inputs changed). * @mode emit */ 'system-prompt/change'(): void } } -/** One contributed section of the system prompt. */ +/** + * Per-assembly input: what one {@link SystemPrompt.assemble} call is FOR. + * Declared empty here so this package stays agnostic of who assembles; + * merge-extensible — `@deepseek-ai/dsh-agent` declares the `agent` field, so + * section text and variable providers can be functions of the calling agent. + * Every field is optional by nature: a bare `assemble()` (tests, diagnostics) + * carries an empty context, and providers must tolerate absent fields. + */ +export interface AssembleContext {} + +/** One contributed section of the system prompt (registry input). */ export interface PromptSection { - /** Unique name (diagnostics / dedup). */ + /** Unique name — a duplicate registration throws (see {@link SystemPrompt.section}). */ name: string - /** Sections are concatenated in ascending order. */ + /** + * Sections are concatenated in ascending order. Convention: `0` is the + * per-agent persona, tool guidance uses 100–199; negative orders render + * before the persona. + */ order: number - /** Static text or a provider evaluated at each assembly. */ - text: string | (() => string) + /** + * Static text or a provider evaluated at each assembly with that assembly's + * {@link AssembleContext}. The text may reference `{{variable}}`s — they are + * interpolated later, by {@link renderPrompt}. + */ + text: string | ((context: AssembleContext) => string) +} + +/** One section of an assembly: {@link PromptSection} with its text resolved. */ +export interface AssembledSection { + /** The contributing section's unique name. */ + name: string + /** The contributing section's order (sections arrive sorted ascending). */ + order: number + /** The resolved (but not yet interpolated) section text. */ + text: string } /** @@ -50,29 +84,72 @@ export interface PromptSection { * can do" is one coherent thing managed here, even though adapters transmit * `tools` as a separate wire field rather than prompt text. * + * `variables` carries every registered prompt variable resolved against this + * assembly's context — key present means registered, `undefined` value means + * "no value for this assembly" (referencing it renders an error). Section + * texts are resolved but NOT yet interpolated; {@link renderPrompt} applies + * the variables, so waterfall listeners can still add sections or variables. + * * Merge-extensible: plugins can declare extra fields on this interface. */ export interface PromptAssembly { - sections: PromptSection[] + sections: AssembledSection[] tools: ToolSchema[] + variables: Record } -/** Renders the text part of an assembly (sections joined by blank lines). */ +/** Valid variable names: how they are written between the braces. */ +const VARIABLE_NAME = /^[a-z][a-z0-9_]*$/ + +/** A complete `{{...}}` reference group (any inner content, validated after). */ +const REFERENCE = /\{\{([^{}]*)\}\}/g + +/** + * Renders the text part of an assembly: interpolates `{{variable}}` + * references in each section from `assembly.variables`, drops empty sections, + * and joins the rest with blank lines. + * + * Strict by design (fail loud beats shipping a malformed prompt): a reference + * to an unregistered variable, to a registered variable with no value for + * this assembly, or a complete `{{...}}` group that is not a well-formed + * variable name (e.g. `{{ model }}`) throws. Only complete double-brace + * groups are interpreted; a lone `{{` without a closing `}}` passes through + * verbatim. + */ export function renderPrompt(assembly: PromptAssembly): string { return assembly.sections - .map(section => typeof section.text === 'function' ? section.text() : section.text) + .map(section => interpolate(section, assembly.variables)) .filter(text => text.length > 0) .join('\n\n') } +/** Interpolate one section's `{{variable}}` references (see {@link renderPrompt}). */ +function interpolate(section: AssembledSection, variables: Record): string { + return section.text.replace(REFERENCE, (_match, name: string) => { + if (!VARIABLE_NAME.test(name)) { + throw new Error(`malformed prompt variable reference "{{${name}}}" in section "${section.name}" (variable names match ${String(VARIABLE_NAME)})`) + } + if (!(name in variables)) { + const known = Object.keys(variables) + throw new Error(`unknown prompt variable "{{${name}}}" in section "${section.name}"; registered variables: ${known.length > 0 ? known.join(', ') : '(none)'}`) + } + const value = variables[name] + if (value === undefined) { + throw new Error(`prompt variable "{{${name}}}" has no value for this assembly (section "${section.name}")`) + } + return value + }) +} + /** * Registry service (`ctx.systemPrompt`): plugins contribute ordered text - * sections and tool-schema providers; the agent loop calls `assemble()` once - * per step. + * sections, tool-schema providers, and named prompt variables; the agent loop + * calls `assemble(context)` once per step. */ export class SystemPrompt extends Service { private sections: PromptSection[] = [] private toolProviders: (() => ToolSchema[])[] = [] + private variableProviders = new Map string | undefined>() constructor(ctx: Context) { super(ctx, 'systemPrompt') @@ -80,13 +157,18 @@ export class SystemPrompt extends Service { /** * Contribute a text section to the system prompt. Order is determined by - * `section.order` (ascending). The section is removed when the calling + * `section.order` (ascending). Throws if a section with the same name is + * already registered (a duplicate would silently double prompt text — e.g. + * a double-loaded tool plugin). The section is removed when the calling * fiber is disposed. Emits `system-prompt/change` on register/unregister. * @param section - the section to contribute (name, order, text or provider). * @returns the disposer that removes the section. */ section(section: PromptSection): () => void { const dispose = this.ctx.effect(function* (this: SystemPrompt) { + if (this.sections.some(existing => existing.name === section.name)) { + throw new Error(`prompt section "${section.name}" is already registered`) + } this.sections.push(section) // Yield the rollback BEFORE emitting `system-prompt/change`: a generator // effect collects each yielded disposer before the next step runs, so a @@ -130,25 +212,71 @@ export class SystemPrompt extends Service { } /** - * Assemble the current prompt (sections sorted by order, tools collected - * from all providers). Section records are top-level clones (the `text` - * provider may be a function and is intentionally shared); tool schemas are - * deep-cloned because adapters and request waterfalls may mutate schema - * objects. Runs through the `system-prompt/assemble` waterfall, giving - * listeners the opportunity to mutate or replace the assembly before it - * reaches the model. Await the result before reading the assembly values — - * waterfall listeners may be async. + * Contribute a named prompt variable, referenced from section text as + * `{{name}}`. The provider is evaluated at each assembly with that + * assembly's {@link AssembleContext}; returning `undefined` means "no value + * for this assembly" (a section referencing it then fails to render — a + * deployment must not claim facts it does not have). Throws on a name that + * does not match `[a-z][a-z0-9_]*` (it could never be referenced) or is + * already registered. Removed when the calling fiber is disposed; emits + * `system-prompt/change` on register/unregister. + * @param name - the reference name (matches `[a-z][a-z0-9_]*`). + * @param provider - evaluated at every {@link assemble} for the value. + * @returns the disposer that removes the variable. + */ + variable(name: string, provider: (context: AssembleContext) => string | undefined): () => void { + const dispose = this.ctx.effect(function* (this: SystemPrompt) { + if (!VARIABLE_NAME.test(name)) { + throw new Error(`invalid prompt variable name "${name}" (must match ${String(VARIABLE_NAME)})`) + } + if (this.variableProviders.has(name)) { + throw new Error(`prompt variable "${name}" is already registered`) + } + this.variableProviders.set(name, provider) + // Yield the rollback BEFORE emitting `system-prompt/change` (see section()). + yield () => { + this.variableProviders.delete(name) + this.ctx.emit('system-prompt/change') + } + this.ctx.emit('system-prompt/change') + }.bind(this), 'systemPrompt.variable()') + // ctx.effect's disposer returns Promise; our disposer API is + // synchronous fire-and-forget — discard the (always-resolved) promise. + return () => void dispose() + } + + /** + * Assemble the current prompt for one caller: section texts are resolved + * against `context` and sorted by order, tools collected from all + * providers, and every registered variable resolved against `context` into + * `assembly.variables`. Tool schemas are deep-cloned because adapters and + * request waterfalls may mutate schema objects. Runs through the + * `system-prompt/assemble` waterfall, giving listeners the opportunity to + * mutate or replace the assembly before it reaches the model. Await the + * result before reading the assembly values — waterfall listeners may be + * async. Interpolation happens later, in {@link renderPrompt}. + * @param context - what this assembly is for (defaults to an empty context; + * see {@link AssembleContext}). * @returns the assembly after the waterfall has run. */ - assemble(): Promise { + assemble(context: AssembleContext = {}): Promise { + const variables: Record = {} + for (const [name, provider] of this.variableProviders) { + variables[name] = provider(context) + } const assembly: PromptAssembly = { sections: this.sections - .map(section => ({ ...section })) + .map(section => ({ + name: section.name, + order: section.order, + text: typeof section.text === 'function' ? section.text(context) : section.text, + })) .sort((a, b) => a.order - b.order), tools: this.toolProviders.flatMap(provider => provider().map(tool => ({ ...tool, parameters: structuredClone(tool.parameters) }))), + variables, } - return this.ctx.waterfall(this, 'system-prompt/assemble', assembly, () => Promise.resolve(assembly)) + return this.ctx.waterfall(this, 'system-prompt/assemble', assembly, context, () => Promise.resolve(assembly)) } } diff --git a/packages/core/system-prompt/tests/system-prompt.spec.ts b/packages/core/system-prompt/tests/system-prompt.spec.ts index cfbf36cbec..ce6760cd20 100644 --- a/packages/core/system-prompt/tests/system-prompt.spec.ts +++ b/packages/core/system-prompt/tests/system-prompt.spec.ts @@ -1,9 +1,9 @@ import { describe, expect, it } from 'vitest' import { Context } from 'cordis' -import SystemPrompt, { PromptAssembly, PromptSection, renderPrompt } from '@deepseek-ai/dsh-system-prompt' +import SystemPrompt, { AssembleContext, PromptAssembly, renderPrompt } from '@deepseek-ai/dsh-system-prompt' describe('SystemPrompt', () => { - it('assembles sections in order with dynamic text and collected tools', async () => { + it('assembles sections in order with context-resolved text and collected tools', async () => { const ctx = new Context() await ctx.plugin(SystemPrompt) @@ -14,10 +14,28 @@ describe('SystemPrompt', () => { const assembly = await ctx.systemPrompt.assemble() expect(assembly.sections.map(s => s.name)).toEqual(['persona', 'rules', 'cwd']) + expect(assembly.sections.map(s => s.text)).toEqual(['You are DeepSeek Code.', 'Be precise.', 'cwd: /tmp']) expect(assembly.tools).toEqual([{ name: 'echo', description: 'echo back', parameters: {} }]) + expect(assembly.variables).toEqual({}) expect(renderPrompt(assembly)).toBe('You are DeepSeek Code.\n\nBe precise.\n\ncwd: /tmp') }) + it('resolves section text providers against the assemble context, at each assemble call', async () => { + // The context is HOW per-agent sections work (the loop passes { agent }); + // this spec stays agent-agnostic and smuggles a marker through a plain field. + const ctx = new Context() + await ctx.plugin(SystemPrompt) + let calls = 0 + ctx.systemPrompt.section({ + name: 'dynamic', + order: 0, + text: (context: AssembleContext) => `call ${++calls} for ${(context as { who?: string }).who ?? 'nobody'}`, + }) + + expect((await ctx.systemPrompt.assemble({ who: 'alice' } as AssembleContext)).sections[0]!.text).toBe('call 1 for alice') + expect((await ctx.systemPrompt.assemble()).sections[0]!.text).toBe('call 2 for nobody') + }) + it('removes contributions when the contributing fiber is disposed (HMR safety)', async () => { const ctx = new Context() await ctx.plugin(SystemPrompt) @@ -25,13 +43,28 @@ describe('SystemPrompt', () => { const fiber = await ctx.plugin(Object.assign((inner: Context) => { inner.systemPrompt.section({ name: 'scoped', order: 0, text: 'scoped section' }) inner.systemPrompt.tools(() => [{ name: 'scoped-tool', description: '', parameters: {} }]) + inner.systemPrompt.variable('scoped_var', () => 'v') }, { inject: ['systemPrompt'] })) - expect((await ctx.systemPrompt.assemble()).sections).toHaveLength(1) + const before = await ctx.systemPrompt.assemble() + expect(before.sections).toHaveLength(1) + expect(before.variables).toEqual({ scoped_var: 'v' }) await fiber.dispose() const assembly = await ctx.systemPrompt.assemble() expect(assembly.sections).toHaveLength(0) expect(assembly.tools).toHaveLength(0) + expect(assembly.variables).toEqual({}) + }) + + it('rejects a duplicate section name (a double-loaded plugin must fail, not double its text)', async () => { + const ctx = new Context() + await ctx.plugin(SystemPrompt) + ctx.systemPrompt.section({ name: 'dup', order: 0, text: 'first' }) + expect(() => ctx.systemPrompt.section({ name: 'dup', order: 1, text: 'second' })) + .toThrow('prompt section "dup" is already registered') + // The failed registration leaked nothing; the original stays intact. + const assembly = await ctx.systemPrompt.assemble() + expect(assembly.sections.map(s => s.text)).toEqual(['first']) }) it('rolls back a section when a system-prompt/change listener throws (P1-1)', async () => { @@ -72,26 +105,47 @@ describe('SystemPrompt', () => { expect((await ctx.systemPrompt.assemble()).tools.map(t => t.name)).toEqual(['t']) }) - it('composes multiple system-prompt/assemble waterfall listeners in order', async () => { + it('rolls back a variable when a system-prompt/change listener throws (P1-1)', async () => { + const ctx = new Context() + await ctx.plugin(SystemPrompt) + + let threw = false + const off = ctx.on('system-prompt/change', () => { + if (!threw) { threw = true; throw new Error('boom change listener') } + }) + + expect(() => ctx.systemPrompt.variable('v', () => 'x')).toThrow('boom change listener') + expect((await ctx.systemPrompt.assemble()).variables).toEqual({}) // nothing leaked + + off() + ctx.systemPrompt.variable('v', () => 'x') + expect((await ctx.systemPrompt.assemble()).variables).toEqual({ v: 'x' }) + }) + + it('composes multiple system-prompt/assemble waterfall listeners in order, with the context', async () => { const ctx = new Context() await ctx.plugin(SystemPrompt) ctx.systemPrompt.section({ name: 'base', order: 0, text: 'base' }) // Listener A appends a section, then delegates. - ctx.on('system-prompt/assemble', async (assembly: PromptAssembly, next) => { + const contexts: AssembleContext[] = [] + ctx.on('system-prompt/assemble', async (assembly: PromptAssembly, context, next) => { + contexts.push(context) assembly.sections.push({ name: 'from-a', order: 100, text: 'a' }) return next() }) // Listener B (registered later, runs after A) sees A's contribution. const seen: string[][] = [] - ctx.on('system-prompt/assemble', async (assembly: PromptAssembly, next) => { + ctx.on('system-prompt/assemble', async (assembly: PromptAssembly, _context, next) => { seen.push(assembly.sections.map(s => s.name)) return next() }) - const assembly = await ctx.systemPrompt.assemble() + const passed: AssembleContext = {} + const assembly = await ctx.systemPrompt.assemble(passed) expect(seen).toEqual([['base', 'from-a']]) expect(assembly.sections.map(s => s.name)).toEqual(['base', 'from-a']) + expect(contexts[0]).toBe(passed) // the caller's context reaches listeners }) it('lets a waterfall listener short-circuit by not calling next()', async () => { @@ -100,7 +154,7 @@ describe('SystemPrompt', () => { ctx.systemPrompt.section({ name: 'real', order: 0, text: 'real' }) ctx.on('system-prompt/assemble', async () => { - return { sections: [], tools: [] } satisfies PromptAssembly + return { sections: [], tools: [], variables: {} } satisfies PromptAssembly }) const assembly = await ctx.systemPrompt.assemble() @@ -115,40 +169,33 @@ describe('SystemPrompt', () => { const first = await ctx.systemPrompt.assemble() first.sections[0]!.name = 'mutated' + first.sections[0]!.text = 'mutated' first.tools[0]!.description = 'mutated' const firstParameters = first.tools[0]!.parameters as { properties: Record } firstParameters.properties['leak'] = { type: 'string' } const second = await ctx.systemPrompt.assemble() expect(second.sections.map(section => section.name)).toEqual(['base']) + expect(second.sections.map(section => section.text)).toEqual(['base']) expect(second.tools).toEqual([{ name: 't', description: 'tool', parameters: { type: 'object', properties: {} } }]) }) it('filters out empty section text from renderPrompt', () => { - // Direct test of renderPrompt: function returning empty string, and empty static text const result = renderPrompt({ sections: [ - { name: 'empty-fn', order: 0, text: () => '' }, + { name: 'empty', order: 0, text: '' }, { name: 'real', order: 1, text: 'content' }, - { name: 'empty-static', order: 2, text: '' }, ], tools: [], + variables: {}, }) expect(result).toBe('content') }) - it('evaluates dynamic function-text sections at each renderPrompt call', () => { - let counter = 0 - const section: PromptSection = { name: 'dynamic', order: 0, text: () => `call ${++counter}` } - expect(renderPrompt({ sections: [section], tools: [] })).toBe('call 1') - expect(renderPrompt({ sections: [section], tools: [] })).toBe('call 2') - }) - it('emits system-prompt/change when a tool provider is registered and disposed', async () => { const ctx = new Context() await ctx.plugin(SystemPrompt) - const changes: number = 0 let changeCount = 0 ctx.on('system-prompt/change', () => void changeCount++) @@ -159,7 +206,6 @@ describe('SystemPrompt', () => { dispose() // disposal emits change again expect(changeCount).toBe(2) - void changes // silence unused }) it('cleans up tool providers on fiber dispose', async () => { @@ -196,4 +242,96 @@ describe('SystemPrompt', () => { dispose() expect((await ctx.systemPrompt.assemble()).tools).toHaveLength(0) }) + + describe('prompt variables', () => { + it('resolves each variable against the assemble context and emits change on register/unregister', async () => { + const ctx = new Context() + await ctx.plugin(SystemPrompt) + let changeCount = 0 + ctx.on('system-prompt/change', () => void changeCount++) + + const dispose = ctx.systemPrompt.variable('who', context => (context as { who?: string }).who) + expect(changeCount).toBe(1) + + expect((await ctx.systemPrompt.assemble({ who: 'alice' } as AssembleContext)).variables).toEqual({ who: 'alice' }) + // A provider returning undefined records "registered but no value here". + expect((await ctx.systemPrompt.assemble()).variables).toEqual({ who: undefined }) + + dispose() + expect(changeCount).toBe(2) + expect((await ctx.systemPrompt.assemble()).variables).toEqual({}) + }) + + it('rejects a duplicate variable name and an unreferenceable name', async () => { + const ctx = new Context() + await ctx.plugin(SystemPrompt) + ctx.systemPrompt.variable('model', () => 'm1') + expect(() => ctx.systemPrompt.variable('model', () => 'm2')) + .toThrow('prompt variable "model" is already registered') + expect(() => ctx.systemPrompt.variable('Not Valid', () => 'x')) + .toThrow('invalid prompt variable name "Not Valid"') + // Neither failed registration leaked. + expect((await ctx.systemPrompt.assemble()).variables).toEqual({ model: 'm1' }) + }) + + it('interpolates {{name}} references in section text at render', async () => { + const ctx = new Context() + await ctx.plugin(SystemPrompt) + ctx.systemPrompt.section({ name: 'persona', order: 0, text: 'You run on {{model}} in {{cwd}}.' }) + ctx.systemPrompt.variable('model', () => 'deepseek-v4') + ctx.systemPrompt.variable('cwd', () => '/work') + + expect(renderPrompt(await ctx.systemPrompt.assemble())).toBe('You run on deepseek-v4 in /work.') + }) + + it('lets a waterfall listener add or override variables before render', async () => { + const ctx = new Context() + await ctx.plugin(SystemPrompt) + ctx.systemPrompt.section({ name: 's', order: 0, text: '{{extra}}' }) + ctx.on('system-prompt/assemble', async (assembly: PromptAssembly, _context, next) => { + assembly.variables['extra'] = 'from-waterfall' + return next() + }) + expect(renderPrompt(await ctx.systemPrompt.assemble())).toBe('from-waterfall') + }) + + it('throws on a reference to an unregistered variable, listing what exists', async () => { + const ctx = new Context() + await ctx.plugin(SystemPrompt) + ctx.systemPrompt.section({ name: 'persona', order: 0, text: 'on {{modle}}' }) + ctx.systemPrompt.variable('model', () => 'm') + await expect(async () => renderPrompt(await ctx.systemPrompt.assemble())) + .rejects.toThrow('unknown prompt variable "{{modle}}" in section "persona"; registered variables: model') + }) + + it('names "(none)" when no variables are registered at all', () => { + expect(() => renderPrompt({ sections: [{ name: 's', order: 0, text: '{{x}}' }], tools: [], variables: {} })) + .toThrow('unknown prompt variable "{{x}}" in section "s"; registered variables: (none)') + }) + + it('throws when a referenced variable has no value for this assembly', () => { + expect(() => renderPrompt({ + sections: [{ name: 'persona', order: 0, text: 'in {{cwd}}' }], + tools: [], + variables: { cwd: undefined }, + })).toThrow('prompt variable "{{cwd}}" has no value for this assembly (section "persona")') + }) + + it('throws on a malformed complete reference, e.g. inner spaces', () => { + expect(() => renderPrompt({ + sections: [{ name: 's', order: 0, text: 'on {{ model }}' }], + tools: [], + variables: { model: 'm' }, + })).toThrow('malformed prompt variable reference "{{ model }}" in section "s"') + }) + + it('leaves a lone {{ without a closing }} verbatim (only complete groups are interpreted)', () => { + const text = renderPrompt({ + sections: [{ name: 's', order: 0, text: 'shell ${X:-{{fallback} stays' }], + tools: [], + variables: {}, + }) + expect(text).toBe('shell ${X:-{{fallback} stays') + }) + }) }) diff --git a/packages/fs/tool-fs/src/read.ts b/packages/fs/tool-fs/src/read.ts index b211d6d80e..a9cc4a9806 100644 --- a/packages/fs/tool-fs/src/read.ts +++ b/packages/fs/tool-fs/src/read.ts @@ -72,7 +72,7 @@ export function applyReadTool(ctx: Context, caps: ReadToolCaps): void { ctx.systemPrompt.section({ name: 'tool:read', order: 100, - text: 'Use the read tool to inspect text files. Results include line numbers. Use offset and limit to continue reading large files.', + text: 'Use the read tool — not shell commands like cat — to inspect text files. Results include line numbers. Use offset and limit to continue reading large files.', }) ctx.tools.register(defineTool({ diff --git a/packages/subagent/subagent-acp/src/index.ts b/packages/subagent/subagent-acp/src/index.ts index cd425e763c..b2ddd9ab2d 100644 --- a/packages/subagent/subagent-acp/src/index.ts +++ b/packages/subagent/subagent-acp/src/index.ts @@ -90,6 +90,8 @@ type ResolvedConfig = Required> & Pick */ class AcpProvider implements SubagentProvider { readonly capabilities: SubagentCapabilities = { outputSchema: false, depthLimit: false, toolFilter: false } + // Context contract: an out-of-process ACP child starts fresh — no parent conversation crosses the process boundary. + readonly inheritsParentContext = false constructor(readonly name: string, private readonly ctx: Context, private readonly config: ResolvedConfig) {} diff --git a/packages/subagent/subagent-fork/src/index.ts b/packages/subagent/subagent-fork/src/index.ts index 02c1811d82..b6d0c10e44 100644 --- a/packages/subagent/subagent-fork/src/index.ts +++ b/packages/subagent/subagent-fork/src/index.ts @@ -62,6 +62,8 @@ export function completedTurnPrefix(parent: Agent): SessionEvent[] { */ class ForkProvider implements SubagentProvider { readonly capabilities: SubagentCapabilities = { outputSchema: false, depthLimit: true, toolFilter: false } + // Context contract: a forked child IS seeded with the parent's completed-turn prefix. + readonly inheritsParentContext = true constructor(readonly name: string, private readonly ctx: Context) {} diff --git a/packages/subagent/subagent-spawn/src/index.ts b/packages/subagent/subagent-spawn/src/index.ts index 2ea082e20a..e6cf5039a7 100644 --- a/packages/subagent/subagent-spawn/src/index.ts +++ b/packages/subagent/subagent-spawn/src/index.ts @@ -39,6 +39,8 @@ export const Config: z = z.object({ */ class SpawnProvider implements SubagentProvider { readonly capabilities: SubagentCapabilities = { outputSchema: false, depthLimit: true, toolFilter: false } + // Context contract: a spawned child starts fresh — it never sees the parent conversation. + readonly inheritsParentContext = false constructor(readonly name: string, private readonly ctx: Context) {} diff --git a/packages/subagent/subagent/README.md b/packages/subagent/subagent/README.md index 3b72971dc1..9ccfb249d8 100644 --- a/packages/subagent/subagent/README.md +++ b/packages/subagent/subagent/README.md @@ -28,6 +28,8 @@ Unlike the bash seam (one executor per context, second load throws), **multiple - **Start-time features** (`outputSchema`, `depthLimit`, `toolFilter`) are a static `provider.capabilities` descriptor, checked by the service BEFORE a run exists. A request that needs one the provider lacks is **rejected loud** (`UNSUPPORTED_CAPABILITY`), never accepted-then-ignored. - **Runtime features** (steering, resume) are **optional methods** on `SubagentRun` (`sendMessage?`, `resume?`). The method's presence IS the capability; TS narrowing is the discovery mechanism — a consumer cannot call an absent method without narrowing first, so there is no silent degradation path. +Beside `capabilities` sits one DESCRIPTIVE fact, not validated by the service: `provider.inheritsParentContext` — whether a child sees the parent conversation (`fork`: true — seeded with the completed-turn prefix; `spawn`/`acp`: false). The model-facing consumer (`dsh-tool-subagent`) derives truthful tool wording from it. + ## Run lifecycle `provider.start(request)` returns a `SubagentRun`: a handle with a `result` promise, `cancel()`, `dispose()`, and the optional runtime methods. `result` resolves with a `SubagentResult` (`output`, optional `structured`, `stopReason`) — it does **not** reject on a child-level failure (a model/transport failure resolves with `stopReason: 'error'`), so the consumer maps a non-`completed` reason to an `isError` tool result. The consumer MUST `dispose()` on every path (success, error, abort) to reach child quiescence and avoid leaking an idle child / session. diff --git a/packages/subagent/subagent/src/types.ts b/packages/subagent/subagent/src/types.ts index fb60d5667c..ef76a96e5d 100644 --- a/packages/subagent/subagent/src/types.ts +++ b/packages/subagent/subagent/src/types.ts @@ -163,6 +163,16 @@ export interface SubagentProvider { readonly name: string /** The start-time features this provider supports (see {@link SubagentCapabilities}). */ readonly capabilities: SubagentCapabilities + /** + * The provider's context contract: `true` when a child SEES the parent + * conversation (fork — the child is seeded with the parent's completed-turn + * prefix), `false` when it starts fresh (spawn, ACP). A DESCRIPTIVE fact, + * not a start-time capability: the service validates nothing against it — + * the model-facing consumer (`dsh-tool-subagent`) derives truthful tool + * wording from it, so a tool bound to a fork provider stops telling the + * model the child "does not see this conversation". + */ + readonly inheritsParentContext: boolean /** * Start a child run. The service has already validated that every requested * start-time capability is supported, so an implementation may assume e.g. diff --git a/packages/subagent/subagent/tests/service.spec.ts b/packages/subagent/subagent/tests/service.spec.ts index 3a8807ad0d..6aa5dfe021 100644 --- a/packages/subagent/subagent/tests/service.spec.ts +++ b/packages/subagent/subagent/tests/service.spec.ts @@ -22,6 +22,7 @@ const NO_CAPS: SubagentCapabilities = { outputSchema: false, depthLimit: false, /** A scripted provider whose run settles immediately with a fixed result. */ class StubProvider implements SubagentProvider { startCount = 0 + readonly inheritsParentContext = false constructor( readonly name: string, readonly capabilities: SubagentCapabilities = ALL_CAPS, @@ -234,6 +235,7 @@ describe('SubagentService', () => { ctx.subagents.registerProvider({ name: 'rej', capabilities: NO_CAPS, + inheritsParentContext: false, start: () => ({ id: AgentId('rej-child'), result: Promise.reject(new Error('infra fault')), @@ -267,6 +269,7 @@ describe('SubagentService', () => { ctx.subagents.registerProvider({ name: 'unclone', capabilities: NO_CAPS, + inheritsParentContext: false, start: () => ({ id: AgentId('unclone-child'), result: Promise.resolve({ output: uncloneable, stopReason: 'completed' } as SubagentResult), @@ -296,6 +299,7 @@ describe('SubagentService', () => { ctx.subagents.registerProvider({ name: 'rejecter', capabilities: NO_CAPS, + inheritsParentContext: false, start: () => ({ id: AgentId('rej-child'), result: Promise.reject(new Error('infra fault')), diff --git a/packages/subagent/tool-subagent/README.md b/packages/subagent/tool-subagent/README.md index 1bb48f29ff..df00f6f169 100644 --- a/packages/subagent/tool-subagent/README.md +++ b/packages/subagent/tool-subagent/README.md @@ -6,6 +6,10 @@ The model-facing `subagent` tool: delegate a self-contained task to a child agen This plugin binds to **exactly one** provider (`Config.provider`). The model sees only `{ description, prompt }` — there is no provider/type parameter in the schema. To expose more than one transport, load the plugin more than once, each bound to a different provider **and a distinct `toolName`** (the tool registry rejects a duplicate name, so a second load that kept the default `subagent` name would throw). Keeping selection in config (not the schema) is the deliberate split: the *service* holds a multi-provider registry; the *tool* picks one. +## The description states the provider's context contract + +The tool description and the `prompt` parameter description are DERIVED from the bound provider's `inheritsParentContext` (`providerWording`): a fresh-context provider (spawn, ACP) gets the standalone-prompt wording ("it does not see this conversation"), an inheriting provider (fork) tells the model the child already sees the conversation's completed turns and its prompt should state only what is new. Because the description is fixed at tool registration, `apply` resolves the provider at LOAD time and **throws if it is not registered yet — list the backend plugin before this one in `cordis.yml`**; a wiring mistake fails loudly at boot instead of shipping a lying description. + | Config key | Meaning | |---|---| | `provider` (required) | The `ctx.subagents` provider name to start runs on (`spawn`, `fork`, `acp`, …). | diff --git a/packages/subagent/tool-subagent/src/index.ts b/packages/subagent/tool-subagent/src/index.ts index 05490127ea..766e3711f3 100644 --- a/packages/subagent/tool-subagent/src/index.ts +++ b/packages/subagent/tool-subagent/src/index.ts @@ -11,6 +11,13 @@ * — there is no provider/type parameter in the model-facing schema. The model * sees only `{ description, prompt }`. * + * The tool DESCRIPTION is derived from the bound provider's context contract + * ({@link providerWording}): a fresh-context provider (spawn, ACP) gets the + * standalone-prompt wording, an inheriting provider (fork) tells the model the + * child already sees the conversation's completed turns. `apply` therefore + * resolves the provider at load time and throws if it is not registered yet — + * list the backend plugin before this one in `cordis.yml`. + * * Collection is SYNCHRONOUS this cut: `execute` starts a run and awaits * `run.result` inside a `try/finally` that always disposes the run, so the * owned child agent/session is torn down on every path (success, error, abort) @@ -92,15 +99,56 @@ function stopReasonError(result: SubagentResult): string | undefined { } } -export function apply(ctx: Context, config: Config): void { - ctx.tools.register(defineTool({ - name: config.toolName ?? 'subagent', +/** + * Model-facing wording per context contract ({@link SubagentProvider.inheritsParentContext}). + * A fresh child needs a standalone prompt; a forked child already sees the + * conversation's completed turns — telling the model to restate everything + * (or, worse, that the child "does not see this conversation") would be false + * for a fork. Exported for tests. + * @param inherits - the bound provider's context contract. + * @returns the tool `description` and the `prompt` parameter description. + */ +export function providerWording(inherits: boolean): { description: string; promptDescription: string } { + if (inherits) { + return { + description: + 'Delegate a task to a subagent that INHERITS this conversation: a child agent seeded with all ' + + 'completed turns so far (it does not see the current in-flight turn), returning only its final ' + + 'result. Use this when the subtask builds on this conversation\'s context — a follow-up analysis, ' + + 'a review, a continuation — without consuming this conversation\'s context for the work itself. ' + + 'You receive only its final answer, not its intermediate steps.', + promptDescription: + 'The task for the subagent. It already sees this conversation\'s completed turns, so build on them ' + + 'freely and state only what is new.', + } + } + return { description: 'Delegate a self-contained task to a subagent (a separate agent that works in its own context) ' + 'and return its final result. Use this to offload focused, independent work — research, a scoped ' + 'implementation, an analysis — so it does not consume this conversation\'s context. The subagent ' + 'runs to completion and you receive only its final answer, not its intermediate steps. Give it a ' + 'complete, standalone prompt: it does not see this conversation.', + promptDescription: + 'The complete, self-contained task for the subagent. It does not share this ' + + 'conversation\'s context, so include everything it needs.', + } +} + +export function apply(ctx: Context, config: Config): void { + // Resolve the bound provider NOW: the tool description must state the + // provider's context contract, so the backend plugin must be loaded before + // this one (list it earlier in cordis.yml). Fail loud at load, not with a + // lying description at model time. + const provider = ctx.subagents.getProvider(config.provider) + if (provider === undefined) { + throw new Error( + `subagent provider "${config.provider}" is not registered; load its backend plugin before tool-subagent`) + } + const wording = providerWording(provider.inheritsParentContext) + ctx.tools.register(defineTool({ + name: config.toolName ?? 'subagent', + description: wording.description, parameters: { description: { type: 'string', @@ -110,8 +158,7 @@ export function apply(ctx: Context, config: Config): void { prompt: { type: 'string', required: true, - description: 'The complete, self-contained task for the subagent. It does not share this ' - + 'conversation\'s context, so include everything it needs.', + description: wording.promptDescription, }, }, async execute(args, exec): Promise { diff --git a/packages/subagent/tool-subagent/tests/tool-subagent.spec.ts b/packages/subagent/tool-subagent/tests/tool-subagent.spec.ts index 2f40cd6f8c..8f00c01366 100644 --- a/packages/subagent/tool-subagent/tests/tool-subagent.spec.ts +++ b/packages/subagent/tool-subagent/tests/tool-subagent.spec.ts @@ -112,6 +112,7 @@ describe('dsh-tool-subagent', () => { ctx.subagents.registerProvider({ name: 'weird', capabilities: { outputSchema: false, depthLimit: false, toolFilter: false }, + inheritsParentContext: false, start: () => ({ id: AgentId('weird-child'), result: Promise.resolve({ output: [{ type: 'text', text: 'partial' }], stopReason: 'frobnicated' as never }), @@ -137,6 +138,7 @@ describe('dsh-tool-subagent', () => { ctx.subagents.registerProvider({ name: 'capture', capabilities: { outputSchema: false, depthLimit: false, toolFilter: false }, + inheritsParentContext: false, start: (request) => { seen = request return { @@ -166,6 +168,7 @@ describe('dsh-tool-subagent', () => { ctx.subagents.registerProvider({ name: 'bare', capabilities: { outputSchema: false, depthLimit: false, toolFilter: false }, + inheritsParentContext: false, start: (request) => { seen = request return { @@ -192,14 +195,36 @@ describe('dsh-tool-subagent', () => { expect(text(result)).toContain('requires a calling agent') }) - it('surfaces an UNSUPPORTED_CAPABILITY rejection as an isError result is NOT applicable here ' - + '(the tool requests no capabilities) — a missing provider IS surfaced', async () => { - // Bind the tool to a provider name that is not registered: the service throws - // NO_PROVIDER, the registry turns it into an isError result. - const ctx = await setup({ provider: 'does-not-exist' }) - const result = await callSubagent(ctx, { description: 'd', prompt: 'p' }) - expect(result.isError).toBe(true) - expect(text(result)).toContain('no subagent provider') + it('fails loud AT LOAD when the bound provider is not registered (backend must load first)', async () => { + // The tool description states the provider's context contract, so apply() + // resolves the provider at load time — a missing backend is a wiring error + // surfaced immediately, not a lying description discovered at model time. + const ctx = new Context() + await ctx.plugin(SystemPrompt) + await ctx.plugin(ToolRegistry) + await ctx.plugin(SubagentService) + await expect(async () => { + await ctx.plugin(tool, { provider: 'does-not-exist' }) + await new Promise(r => setTimeout(r, 20)) + }).rejects.toThrow('is not registered; load its backend plugin before tool-subagent') + expect(ctx.tools.schemas().some(s => s.name === 'subagent')).toBe(false) + }) + + it('derives spawn-shaped wording from a fresh-context provider (default mock)', async () => { + const ctx = await setup({ provider: 'mock' }) + const schema = ctx.tools.schemas().find(s => s.name === 'subagent')! + expect(schema.description).toContain('does not see this conversation') + const props = (schema.parameters as { properties: Record }).properties + expect(props['prompt']!.description).toContain('include everything it needs') + }) + + it('derives fork-shaped wording from an inheriting provider (the description stops lying)', async () => { + const ctx = await setup({ provider: 'mock', toolName: 'subagent' }, { inheritsParentContext: true }) + const schema = ctx.tools.schemas().find(s => s.name === 'subagent')! + expect(schema.description).toContain('INHERITS this conversation') + expect(schema.description).not.toContain('does not see this conversation') + const props = (schema.parameters as { properties: Record }).properties + expect(props['prompt']!.description).toContain('completed turns') }) it('disposes the run on the success path (no leaked child)', async () => { @@ -213,6 +238,7 @@ describe('dsh-tool-subagent', () => { ctx.subagents.registerProvider({ name: 'spy', capabilities: { outputSchema: false, depthLimit: false, toolFilter: false }, + inheritsParentContext: false, start: () => ({ id: AgentId('spy-child'), result: Promise.resolve({ output: [{ type: 'text', text: 'ok' }], stopReason: 'completed' as const }), @@ -235,6 +261,7 @@ describe('dsh-tool-subagent', () => { ctx.subagents.registerProvider({ name: 'spy', capabilities: { outputSchema: false, depthLimit: false, toolFilter: false }, + inheritsParentContext: false, start: () => ({ id: AgentId('spy-child'), result: Promise.resolve({ output: [], stopReason: 'error' as const }), @@ -258,6 +285,7 @@ describe('dsh-tool-subagent', () => { ctx.subagents.registerProvider({ name: 'spy', capabilities: { outputSchema: false, depthLimit: false, toolFilter: false }, + inheritsParentContext: false, start: () => { let resolveResult: (r: { output: never[]; stopReason: 'aborted' }) => void const result = new Promise<{ output: never[]; stopReason: 'aborted' }>((res) => { resolveResult = res }) @@ -304,6 +332,7 @@ describe('dsh-tool-subagent', () => { ctx.subagents.registerProvider({ name: 'spy', capabilities: { outputSchema: false, depthLimit: false, toolFilter: false }, + inheritsParentContext: false, start: () => { let resolveResult: (r: { output: never[]; stopReason: 'aborted' }) => void const result = new Promise<{ output: never[]; stopReason: 'aborted' }>((res) => { resolveResult = res }) diff --git a/packages/support/subagent-mock/README.md b/packages/support/subagent-mock/README.md index 305aea93c4..af169ed4c2 100644 --- a/packages/support/subagent-mock/README.md +++ b/packages/support/subagent-mock/README.md @@ -14,6 +14,7 @@ Load it as a plugin (functional shape: `name`/`inject`/`Config`/`apply`, no defa | `reply` | `mock subagent reply` | The scripted child's final answer text. | | `stopReason` | `completed` | The stop reason `result` settles with. | | `capabilities` | all `true` | Which start-time capabilities (`outputSchema`/`depthLimit`/`toolFilter`) the provider advertises. | +| `inheritsParentContext` | `false` | The context contract to declare; `true` exercises the fork-shaped tool wording in consumer tests. | | `structured` | `{ reply }` | Structured value surfaced when a request carries an `outputSchema` and the capability is on. | A `cancel()` issued before `result` settles flips the stop reason to `aborted`, so the cancellation path is observable. diff --git a/packages/support/subagent-mock/src/index.ts b/packages/support/subagent-mock/src/index.ts index c1a988fbfe..e2effe5b4b 100644 --- a/packages/support/subagent-mock/src/index.ts +++ b/packages/support/subagent-mock/src/index.ts @@ -37,12 +37,14 @@ const DEFAULT_CAPS: SubagentCapabilities = { outputSchema: true, depthLimit: tru */ class MockSubagentProvider implements SubagentProvider { readonly capabilities: SubagentCapabilities + readonly inheritsParentContext: boolean constructor( readonly name: string, private readonly config: Config, ) { this.capabilities = { ...DEFAULT_CAPS, ...config.capabilities } + this.inheritsParentContext = config.inheritsParentContext ?? false } start(request: SubagentStartRequest): SubagentRun { @@ -88,6 +90,12 @@ export interface Config { stopReason?: SubagentStopReason /** Which start-time capabilities to advertise (default: all `true`). */ capabilities?: Partial + /** + * The context contract to declare ({@link SubagentProvider.inheritsParentContext}); + * default `false` (spawn-like). Set `true` to exercise the fork-shaped tool + * wording in consumer tests. + */ + inheritsParentContext?: boolean /** * Structured value surfaced when a request carries an `outputSchema` and the * `outputSchema` capability is on (default: `{ reply }`). @@ -104,6 +112,7 @@ export const Config: z = z.object({ depthLimit: z.boolean(), toolFilter: z.boolean(), }), + inheritsParentContext: z.boolean(), structured: z.any(), }) diff --git a/packages/ui/acp-agent/README.md b/packages/ui/acp-agent/README.md index 3403ef2126..a1f35172c5 100644 --- a/packages/ui/acp-agent/README.md +++ b/packages/ui/acp-agent/README.md @@ -23,7 +23,7 @@ Because the package wires no logger entry, an ACP leaf has **nothing to get wron | Key | Default | Routed to | |---|---|---| | `model` | (required) | the per-session agent template the bridge creates agents from | -| `systemPrompt` | (required) | the per-session agent's system prompt | +| `systemPrompt` | (required) | the per-session agent's persona template (may reference `{{model}}`/`{{cwd}}`) | | `persistenceRoot` | `./.sessions` | the JSONL backend's root directory | The leaf supplies the swappable backends: an LLM adapter (`llm-deepseek` for the real model, `llm-replay` for keyless snapshot replay) and a bash executor (`bash-local`). diff --git a/packages/ui/acp/README.md b/packages/ui/acp/README.md index 38596b7fd0..42a856c5ea 100644 --- a/packages/ui/acp/README.md +++ b/packages/ui/acp/README.md @@ -15,7 +15,7 @@ It is a **client-driver / UI plugin**, the structured analogue of the readline ` | Key | Default | Meaning | |---|---|---| | `model` | — | Model name for created agents (must have a registered adapter). | -| `systemPrompt` | — | Per-agent system prompt. | +| `systemPrompt` | — | Per-agent persona template (may reference `{{model}}`/`{{cwd}}`). | The `initialize` handshake reports a fixed server identity (`agentInfo: { name: 'deepseek-harness-acp', version: '0.0.1' }`) — branding is a literal at the `initialize` site, not config. diff --git a/packages/ui/stdio-agent/README.md b/packages/ui/stdio-agent/README.md index c75fee966a..9e48ba7f75 100644 --- a/packages/ui/stdio-agent/README.md +++ b/packages/ui/stdio-agent/README.md @@ -24,7 +24,7 @@ The leaf `cordis.yml` supplies only the **swappable backends** — an LLM adapte | Key | Default | Routed to | |---|---|---| | `model` | (required) | the pre-created `main` agent's model | -| `systemPrompt` | (required) | the `main` agent's system prompt | +| `systemPrompt` | (required) | the `main` agent's persona template (may reference `{{model}}`) | | `persistenceRoot` | `./.sessions` | the JSONL backend's root directory | | `welcome` | `ready.` | the stdin-chat banner | | `resumeSessionId` | — | resume a persisted session id instead of starting fresh (sourced from an env var in the leaf) | @@ -54,7 +54,7 @@ The leaf `cordis.yml` supplies only the **swappable backends** — an LLM adapte name: '@deepseek-ai/dsh-stdio-agent' config: model: deepseek-v4-flash - systemPrompt: 'You are a CLI coding assistant. Your only tools are bash…' + systemPrompt: 'You are a CLI coding assistant powered by the {{model}} model.' ``` Swap `llm-deepseek` for a `mock-llm` leaf plugin and you have the echo demo — "swap the backend, keep the app". diff --git a/packages/web/tool-web/tests/tool-web.spec.ts b/packages/web/tool-web/tests/tool-web.spec.ts index 2347f6a8a8..924e0aaeb2 100644 --- a/packages/web/tool-web/tests/tool-web.spec.ts +++ b/packages/web/tool-web/tests/tool-web.spec.ts @@ -200,7 +200,7 @@ describe('tool-web registration', () => { it('contributes prompt sections for the enabled tools', async () => { const { fiber, ctx } = await mountTools() const prompt = await ctx.systemPrompt.assemble() - const text = prompt.sections.map(s => (typeof s.text === 'function' ? s.text() : s.text)).join('\n') + const text = prompt.sections.map(s => s.text).join('\n') expect(text).toContain('web_search') expect(text).toContain('web_fetch') await fiber.dispose() diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 06189862f2..149a786f32 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -171,6 +171,9 @@ importers: '@deepseek-ai/dsh-session': specifier: workspace:^ version: link:../session + '@deepseek-ai/dsh-system-prompt': + specifier: workspace:^ + version: link:../system-prompt cordis: specifier: ^4.0.0-rc.6 version: 4.0.0-rc.6(@cordisjs/plugin-include@1.0.4)(@cordisjs/plugin-loader@1.0.0-rc.4) From e85e21c8b05fb74e3c95723c50a644da4b4d7ed0 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sun, 5 Jul 2026 02:42:48 +0800 Subject: [PATCH 2/8] fix(review): close interpolation strictness holes; make tool-subagent mirror provider lifecycle Codex round-1 findings, both confirmed: - renderPrompt: variable lookup now uses Object.hasOwn (an unregistered {{constructor}} previously resolved through Object.prototype and spliced function source into the prompt), and a {{ that opens no complete group while a }} still follows ({{{model}}}, {{a{b}}) now throws instead of passing or partially interpolating. A lone {{ with no }} after it stays verbatim; substituted values are never re-scanned. - tool-subagent: the apply-time provider lookup assumed a load order the cordis Loader does not guarantee (siblings start concurrently). The seam now announces subagent/provider-added/-removed and the tool mirrors the provider's lifecycle: registers when the provider is (or becomes) available, unregisters when it goes away, re-derives wording on reload. No load-order requirement remains. - loop.spec containment test now proves live continuation: after the contained render failure, a waterfall listener rescues {{cwd}} and the same agent completes a real model turn. RFC/READMEs updated to the shipped contract; cordis catalog regenerated. --- docs/cordis-catalog/events-and-services.md | 28 ++- ...t-variables-and-tool-guidance-ownership.md | 13 +- packages/core/agent-loop/tests/loop.spec.ts | 24 ++- packages/core/system-prompt/README.md | 2 +- packages/core/system-prompt/src/index.ts | 45 +++-- .../system-prompt/tests/system-prompt.spec.ts | 40 ++++- packages/subagent/subagent/README.md | 2 +- packages/subagent/subagent/src/index.ts | 28 ++- .../subagent/subagent/tests/service.spec.ts | 32 ++++ packages/subagent/tool-subagent/README.md | 2 +- packages/subagent/tool-subagent/src/index.ts | 167 ++++++++++-------- .../tool-subagent/tests/tool-subagent.spec.ts | 53 +++++- 12 files changed, 328 insertions(+), 108 deletions(-) diff --git a/docs/cordis-catalog/events-and-services.md b/docs/cordis-catalog/events-and-services.md index 8433f5c9ab..6eede54b97 100644 --- a/docs/cordis-catalog/events-and-services.md +++ b/docs/cordis-catalog/events-and-services.md @@ -245,7 +245,27 @@ A subagent run settled — emitted when SubagentRun.result resolves (any stop re 'subagent/end'(info: SubagentRunEndInfo): void ``` -Source: [`packages/subagent/subagent/src/index.ts:77`](../../packages/subagent/subagent/src/index.ts) +Source: [`packages/subagent/subagent/src/index.ts:96`](../../packages/subagent/subagent/src/index.ts) + +#### `subagent/provider-added` — emit + +A provider became resolvable in the SubagentService registry. Consumers that derive state from a named provider (e.g. the model-facing tool wording in `dsh-tool-subagent`) react HERE instead of assuming load order — the cordis Loader starts sibling plugins concurrently, so "listed earlier in cordis.yml" does not mean "registered earlier". + +```ts cordis-catalog +'subagent/provider-added'(provider: SubagentProvider): void +``` + +Source: [`packages/subagent/subagent/src/index.ts:72`](../../packages/subagent/subagent/src/index.ts) + +#### `subagent/provider-removed` — emit + +A provider left the registry (its plugin's fiber was disposed — an unload or an HMR reload). Consumers holding provider-derived state drop it here; a reload re-fires `subagent/provider-added` with the fresh provider. + +```ts cordis-catalog +'subagent/provider-removed'(name: string): void +``` + +Source: [`packages/subagent/subagent/src/index.ts:81`](../../packages/subagent/subagent/src/index.ts) #### `subagent/start` — emit @@ -255,7 +275,7 @@ A subagent run started — emitted after the provider is resolved and its capabi 'subagent/start'(info: SubagentRunInfo): void ``` -Source: [`packages/subagent/subagent/src/index.ts:70`](../../packages/subagent/subagent/src/index.ts) +Source: [`packages/subagent/subagent/src/index.ts:89`](../../packages/subagent/subagent/src/index.ts) ### `system-prompt/*` @@ -486,7 +506,7 @@ list(): string[] start(name: string, request: SubagentStartRequest): SubagentRun ``` -Source: [`packages/subagent/subagent/src/index.ts:123`](../../packages/subagent/subagent/src/index.ts) +Source: [`packages/subagent/subagent/src/index.ts:142`](../../packages/subagent/subagent/src/index.ts) ### `ctx.systemPrompt` — `SystemPrompt` @@ -499,7 +519,7 @@ variable(name: string, provider: (context: AssembleContext) => string | undefine assemble(context: AssembleContext = {}): Promise ``` -Source: [`packages/core/system-prompt/src/index.ts:149`](../../packages/core/system-prompt/src/index.ts) +Source: [`packages/core/system-prompt/src/index.ts:174`](../../packages/core/system-prompt/src/index.ts) ### `ctx.tools` — `ToolRegistry` diff --git a/docs/rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md b/docs/rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md index 14b9990881..0802e56aac 100644 --- a/docs/rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md +++ b/docs/rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md @@ -24,7 +24,7 @@ The assembled system prompt had four defects, all of one family: facts the harne ### Prompt variables -Plugins contribute named values via `ctx.systemPrompt.variable(name, provider)`; prompt text references them as `{{name}}`. Providers are functions of the `AssembleContext` and may return `undefined` — "no value for THIS assembly". `assemble()` resolves every registered variable into `PromptAssembly.variables` (waterfall listeners can see, add, or override); `renderPrompt` interpolates. Rendering is STRICT — fail loud beats shipping a malformed prompt: a reference to an unregistered name throws (listing what exists), a registered-but-valueless reference throws, and a complete `{{…}}` group that is not a well-formed name (`[a-z][a-z0-9_]*`, e.g. `{{ model }}`) throws. Only complete double-brace groups are interpreted; a lone `{{` passes through verbatim. Registration rejects duplicate and unreferenceable names, mirroring the tool registry — and `section()` now rejects duplicate section names, making the documented dedup real. +Plugins contribute named values via `ctx.systemPrompt.variable(name, provider)`; prompt text references them as `{{name}}`. Providers are functions of the `AssembleContext` and may return `undefined` — "no value for THIS assembly". `assemble()` resolves every registered variable into `PromptAssembly.variables` (waterfall listeners can see, add, or override); `renderPrompt` interpolates. Rendering is STRICT — fail loud beats shipping a malformed prompt: a reference to an unregistered name throws (listing what exists; lookup is `Object.hasOwn`, so a prototype property like `{{constructor}}` is unknown, not a function spliced into the prompt), a registered-but-valueless reference throws, a complete `{{…}}` group that is not a well-formed name (`[a-z][a-z0-9_]*`, e.g. `{{ model }}`) throws, and a `{{` that opens no complete group while a `}}` still follows (`{{{model}}}`, `{{a{b}}`) throws. A lone `{{` with no `}}` anywhere after it is ordinary prose and passes through verbatim; substituted values are never re-scanned. Registration rejects duplicate and unreferenceable names, mirroring the tool registry — and `section()` now rejects duplicate section names, making the documented dedup real. `dsh-agent-loop` registers the two built-ins, both pure projections of the context agent: `model` (= `options.model`) and `cwd` (= `session.header.cwd`). The example personas write `powered by the {{model}} model` — the model name is stated once, in the `model:` config key. `{{cwd}}` is demonstrated in the ACP example only: every ACP session carries the client's cwd, while config-pre-created stdio agents have none (a persona claiming `{{cwd}}` there fails the turn — by design). @@ -38,7 +38,7 @@ Per-tool semantics and when-to-use live in tool DESCRIPTIONS, which already ship ### The subagent context contract -`SubagentProvider` gains `readonly inheritsParentContext: boolean` — a DESCRIPTIVE fact beside `capabilities`, not in it (capabilities are start-time validation; nothing validates against this flag). Spawn and ACP declare `false`, fork declares `true`. `dsh-tool-subagent` derives both the tool description and the `prompt` parameter description from the flag (`providerWording`): the fork instance now tells the model the child inherits the conversation's completed turns (not the in-flight turn) and that its prompt should state only what is new. `apply` resolves the provider at LOAD time and throws if it is not registered — the backend plugin must be listed before the tool plugin in `cordis.yml`; a wiring mistake fails loudly at boot instead of shipping a lying description. +`SubagentProvider` gains `readonly inheritsParentContext: boolean` — a DESCRIPTIVE fact beside `capabilities`, not in it (capabilities are start-time validation; nothing validates against this flag). Spawn and ACP declare `false`, fork declares `true`. `dsh-tool-subagent` derives both the tool description and the `prompt` parameter description from the flag (`providerWording`): the fork instance now tells the model the child inherits the conversation's completed turns (not the in-flight turn) and that its prompt should state only what is new. Because the description is fixed at tool registration while providers arrive on their own fibers, the registry announces provider lifecycle (`subagent/provider-added`/`subagent/provider-removed`) and the tool MIRRORS it: it registers when its provider is (or becomes) available, unregisters when the provider goes away, and re-derives the wording on re-registration (HMR). There is deliberately NO load-order requirement — the cordis Loader starts sibling entries concurrently (`Promise.all` over the group), so "listed first" never guaranteed "registered first"; while the provider is absent the tool does not exist, which cannot lie. ## Rejected alternatives @@ -47,12 +47,13 @@ Per-tool semantics and when-to-use live in tool DESCRIPTIONS, which already ship - **Hand-write the model name in each persona** — duplicates the `model:` key one line above and silently lies after a config edit; the exact disease this RFC cures. - **Lenient interpolation (leave unknown refs verbatim, or substitute empty)** — a typo ships `{{modle}}` (or a hole) to the model and nobody notices until transcript review. - **Per-instance subagent wording in config** — returns model-facing prose to every deployment × instance, the P2 disease again. **Keying wording off the provider NAME** — `providerName` is itself config, so a renamed provider silently gets the wrong words. -- **Resolving the subagent flag lazily (section-only wording)** — would tolerate any load order, but the DESCRIPTION is fixed at tool registration and is where tool-choice guidance belongs; a deterministic load-order requirement with a loud, actionable failure is the smaller cost. +- **Resolving the provider at `apply` time and throwing when absent (a load-order requirement)** — the first implementation. Rejected after review reproduced the failure: the Loader starts sibling entries concurrently and `Entry.init()` does not await activation, so a backend whose activation is delayed leaves the tool's fiber permanently failed even when "listed first" — the ordering the requirement leaned on is not a contract the Loader offers ("async state is not synchronous state"). Provider-lifecycle events make the ordering question disappear instead of documenting it. +- **Section-only subagent wording (lazily resolved at assemble)** — would also tolerate any load order, but the DESCRIPTION is fixed at tool registration and is where tool-choice guidance belongs; reactive registration keeps the description authoritative AND order-free. ## What we give up - `{{model}}` reflects `AgentOptions.model` at assembly time. A plugin that switches models in the `agent/request` waterfall makes the prompt's claim stale for that step; such a plugin can rewrite `options.system` in the same waterfall if it cares. Accepted. -- `dsh-tool-subagent` now has a hard load-order requirement on its backend. The examples already ordered backends first; the failure mode is an immediate boot error naming the fix. +- While a bound provider is absent (not yet activated, unloaded, mid-HMR-reload), the subagent tool does not exist and a model request in that window simply lacks it. That is the honest state — the alternative was a registered tool whose description or execution could not be trusted. - Strictness means a persona can fail a turn at render (e.g. `{{cwd}}` on a cwd-less session). The failure is contained — the turn ends `error`, the loop survives — and it is an authoring error we WANT loud. - No escape syntax for a literal `{{name}}` in prompt prose yet; add one if a real prompt ever needs it. @@ -64,6 +65,6 @@ Per-tool semantics and when-to-use live in tool DESCRIPTIONS, which already ship ## Acceptance criteria - `renderPrompt(assemble({agent}))` for the coding-agent example contains the persona FIRST (with the agent's model name interpolated), then fs/bash/web guidance sections; the loop contains no other prompt-composition path. -- The `subagent_fork` schema description says the child inherits the conversation; the `subagent` one says it does not. Loading `dsh-tool-subagent` before its backend fails at load with a message naming the ordering fix. -- Unknown/valueless/malformed `{{…}}` references throw with the section name in the message; duplicate section, variable, and tool-name registrations all throw. +- The `subagent_fork` schema description says the child inherits the conversation; the `subagent` one says it does not. The tool follows its provider: absent before the backend activates, present after, gone when the backend unloads, re-worded from the fresh provider on reload. +- Unknown/valueless/malformed/unbalanced `{{…}}` references throw with the section name in the message; duplicate section, variable, and tool-name registrations all throw. - The gating runs (`test:coverage`, `test:snapshot`, `doc-sync`, `build`, `hygiene`) are green; no golden re-record is needed (replay never re-verifies the outgoing request). diff --git a/packages/core/agent-loop/tests/loop.spec.ts b/packages/core/agent-loop/tests/loop.spec.ts index 014ddb548c..6a0763814b 100644 --- a/packages/core/agent-loop/tests/loop.spec.ts +++ b/packages/core/agent-loop/tests/loop.spec.ts @@ -182,11 +182,13 @@ describe('agent loop', () => { expect(adapter.requests[0]!.system).toBe('Working in /work/space.') }) - it('contains a strict-variable render failure: the turn errors, the loop survives', async () => { + it('contains a strict-variable render failure: the turn errors, the loop keeps serving turns', async () => { // A persona claiming {{cwd}} on a session with NO cwd is a deployment // authoring error — renderPrompt throws, the turn ends with an error, and - // the agent (and loop) stay alive for the next prompt. - const adapter = new MockAdapter([textResponse('never reached'), textResponse('ok')]) + // the same agent must then RUN a later turn to completion (not merely + // report idle status): a rescue listener supplies the variable and the + // follow-up prompt reaches the model. + const adapter = new MockAdapter([textResponse('ok after rescue')]) const ctx = await harness(adapter) const errors: Error[] = [] ctx.on('agent/error', (_agent, _turn, _step, error) => void errors.push(error)) @@ -199,7 +201,21 @@ describe('agent loop', () => { expect(errors.some(e => e.message.includes('no value for this assembly'))).toBe(true) const turnEnd = agent.session.events.find(e => e.type === 'turn/end') expect(turnEnd?.type === 'turn/end' && turnEnd.data.reason.kind).toBe('error') - expect(agent.status).toBe('idle') // contained: the loop is still serving + + // The loop survived: a waterfall listener rescues {{cwd}} and the SAME + // agent completes a real model turn. + ctx.on('system-prompt/assemble', async (assembly, _context, next) => { + assembly.variables['cwd'] = '/rescued' + return next() + }) + send(agent, 'again') + await waitForIdle(ctx, agent) + + expect(adapter.requests).toHaveLength(1) + expect(adapter.requests[0]!.system).toBe('In /rescued.') + const turnEnds = agent.session.events.filter(e => e.type === 'turn/end') + expect(turnEnds).toHaveLength(2) + expect(turnEnds[1]?.type === 'turn/end' && turnEnds[1].data.reason.kind).toBe('completed') }) it('records raw chunks for replay as assistant/chunk session events', async () => { diff --git a/packages/core/system-prompt/README.md b/packages/core/system-prompt/README.md index 28d382f54a..6207571618 100644 --- a/packages/core/system-prompt/README.md +++ b/packages/core/system-prompt/README.md @@ -23,7 +23,7 @@ System prompt assembly registry. Plugins contribute ordered text sections, tool- - `AssembleContext` — what one `assemble()` call is FOR. Declared empty here and merge-extensible; `dsh-agent` declares `agent?: Agent`, so providers project per-agent facts. Providers must tolerate absent fields (a bare `assemble()` carries an empty context). - `PromptSection` — `{ name, order, text: string | ((context) => string) }`. Sections are concatenated in ascending `order`. Order bands: `0` is the per-agent persona (registered by the agent loop), tool guidance uses `100–199`; negative orders render before the persona. - `PromptAssembly` — `{ sections: AssembledSection[], tools: ToolSchema[], variables: Record }`. Section texts arrive resolved but not yet interpolated; `variables` holds every registered variable resolved against the context. Tool schemas are part of the assembly by design: "what the model is told it can do" is one coherent thing, even though adapters transmit schemas as a separate wire field. -- `renderPrompt(assembly)` — interpolates `{{variable}}` references in each section, drops empty sections, joins with blank lines. STRICT: an unknown reference, a registered-but-valueless reference, or a malformed complete `{{…}}` group throws (fail loud beats shipping a malformed prompt). Only complete double-brace groups are interpreted; a lone `{{` passes through verbatim. +- `renderPrompt(assembly)` — interpolates `{{variable}}` references in each section, drops empty sections, joins with blank lines. STRICT: an unknown reference (`Object.hasOwn` lookup — prototype names like `{{constructor}}` are unknown), a registered-but-valueless reference, a malformed complete `{{…}}` group, or a `{{` that opens no complete group while a `}}` still follows (`{{{model}}}`) throws — fail loud beats shipping a malformed prompt. A lone `{{` with no `}}` anywhere after it passes through verbatim; substituted values are never re-scanned. Merge-extensible: plugins can declare extra fields on `PromptAssembly` and `AssembleContext` via declaration merging. diff --git a/packages/core/system-prompt/src/index.ts b/packages/core/system-prompt/src/index.ts index 3faf2373fa..62889a2793 100644 --- a/packages/core/system-prompt/src/index.ts +++ b/packages/core/system-prompt/src/index.ts @@ -101,8 +101,8 @@ export interface PromptAssembly { /** Valid variable names: how they are written between the braces. */ const VARIABLE_NAME = /^[a-z][a-z0-9_]*$/ -/** A complete `{{...}}` reference group (any inner content, validated after). */ -const REFERENCE = /\{\{([^{}]*)\}\}/g +/** A complete `{{...}}` reference group at the scan position (validated after). */ +const GROUP_AT = /^\{\{([^{}]*)\}\}/ /** * Renders the text part of an assembly: interpolates `{{variable}}` @@ -111,10 +111,11 @@ const REFERENCE = /\{\{([^{}]*)\}\}/g * * Strict by design (fail loud beats shipping a malformed prompt): a reference * to an unregistered variable, to a registered variable with no value for - * this assembly, or a complete `{{...}}` group that is not a well-formed - * variable name (e.g. `{{ model }}`) throws. Only complete double-brace - * groups are interpreted; a lone `{{` without a closing `}}` passes through - * verbatim. + * this assembly, a complete `{{…}}` group that is not a well-formed variable + * name (e.g. `{{ model }}`), or a `{{` that does not open a complete group + * while a `}}` still follows (e.g. `{{{model}}}`, `{{a{b}}`) all throw. A + * lone `{{` with no `}}` anywhere after it is ordinary prose and passes + * through verbatim. Substituted values are never re-scanned. */ export function renderPrompt(assembly: PromptAssembly): string { return assembly.sections @@ -125,11 +126,33 @@ export function renderPrompt(assembly: PromptAssembly): string { /** Interpolate one section's `{{variable}}` references (see {@link renderPrompt}). */ function interpolate(section: AssembledSection, variables: Record): string { - return section.text.replace(REFERENCE, (_match, name: string) => { + const text = section.text + let result = '' + let last = 0 + for (let open = text.indexOf('{{'); open >= 0; open = text.indexOf('{{', last)) { + const group = GROUP_AT.exec(text.slice(open)) + if (group === null) { + // No complete simple group starts at this `{{`. A `}}` further on means + // a mangled reference (extra or nested braces) — fail loud. With no + // closing `}}` anywhere after, it is ordinary prose (shell, JSON) and + // passes through verbatim. + if (text.indexOf('}}', open + 2) >= 0) { + throw new Error(`malformed prompt variable reference at "${text.slice(open, open + 16)}…" in section "${section.name}" (references are complete simple {{name}} groups)`) + } + result += text.slice(last, open + 2) + last = open + 2 + continue + } + // group[0] is the whole `{{...}}` match (a plain string, no optional + // index): the name is its interior. `{{}}` yields '' → the malformed path. + const name = group[0].slice(2, -2) if (!VARIABLE_NAME.test(name)) { throw new Error(`malformed prompt variable reference "{{${name}}}" in section "${section.name}" (variable names match ${String(VARIABLE_NAME)})`) } - if (!(name in variables)) { + // Object.hasOwn, NOT `in`: `in` walks the prototype chain, so an + // unregistered `{{constructor}}` would resolve to Object.prototype's and + // splice a function's source text into the prompt instead of throwing. + if (!Object.hasOwn(variables, name)) { const known = Object.keys(variables) throw new Error(`unknown prompt variable "{{${name}}}" in section "${section.name}"; registered variables: ${known.length > 0 ? known.join(', ') : '(none)'}`) } @@ -137,8 +160,10 @@ function interpolate(section: AssembledSection, variables: Record { })).toThrow('malformed prompt variable reference "{{ model }}" in section "s"') }) - it('leaves a lone {{ without a closing }} verbatim (only complete groups are interpreted)', () => { + it('leaves a lone {{ verbatim only when NO }} follows anywhere after it', () => { const text = renderPrompt({ sections: [{ name: 's', order: 0, text: 'shell ${X:-{{fallback} stays' }], tools: [], @@ -333,5 +333,43 @@ describe('SystemPrompt', () => { }) expect(text).toBe('shell ${X:-{{fallback} stays') }) + + it.each([ + { text: '{{{model}}}', label: 'extra outer braces' }, + { text: 'x {{a{b}} y {{model}}', label: 'nested brace inside a would-be group' }, + ])('throws on a mangled reference with a }} still following ($label)', ({ text }) => { + expect(() => renderPrompt({ + sections: [{ name: 's', order: 0, text }], + tools: [], + variables: { model: 'm' }, + })).toThrow('malformed prompt variable reference at') + }) + + it('rejects {{constructor}} as UNKNOWN — prototype properties are not variables', () => { + // `in` would find Object.prototype.constructor and splice function + // source into the prompt; Object.hasOwn must reject it instead. + expect(() => renderPrompt({ + sections: [{ name: 's', order: 0, text: 'on {{constructor}}' }], + tools: [], + variables: { model: 'm' }, + })).toThrow('unknown prompt variable "{{constructor}}"') + }) + + it('a variable NAMED like a prototype property works once actually registered', async () => { + const ctx = new Context() + await ctx.plugin(SystemPrompt) + ctx.systemPrompt.section({ name: 's', order: 0, text: '{{constructor}}' }) + ctx.systemPrompt.variable('constructor', () => 'own-value') + expect(renderPrompt(await ctx.systemPrompt.assemble())).toBe('own-value') + }) + + it('never re-scans substituted values (a value containing {{sneaky}} stays literal)', () => { + const text = renderPrompt({ + sections: [{ name: 's', order: 0, text: 'v = {{model}}!' }], + tools: [], + variables: { model: 'literal {{sneaky}} inside' }, + }) + expect(text).toBe('v = literal {{sneaky}} inside!') + }) }) }) diff --git a/packages/subagent/subagent/README.md b/packages/subagent/subagent/README.md index 9ccfb249d8..8bab4c61c9 100644 --- a/packages/subagent/subagent/README.md +++ b/packages/subagent/subagent/README.md @@ -34,7 +34,7 @@ Beside `capabilities` sits one DESCRIPTIVE fact, not validated by the service: ` `provider.start(request)` returns a `SubagentRun`: a handle with a `result` promise, `cancel()`, `dispose()`, and the optional runtime methods. `result` resolves with a `SubagentResult` (`output`, optional `structured`, `stopReason`) — it does **not** reject on a child-level failure (a model/transport failure resolves with `stopReason: 'error'`), so the consumer maps a non-`completed` reason to an `isError` tool result. The consumer MUST `dispose()` on every path (success, error, abort) to reach child quiescence and avoid leaking an idle child / session. -The service emits `subagent/start` (payload `SubagentRunInfo`) and `subagent/end` (payload `SubagentRunEndInfo`) around the run — both **observe-only** (plain `emit`s; `subagent/end` fires from a detached `.then` and awaits no listener). `subagent/end` carries `lastAssistantMessage` (a deep clone of the child's final `output`) on the settle path, absent when the run rejected at the infrastructure level. The clone keeps the surface observe-only: the end emit fires from a detached `.then` before the caller's `await run.result` resumes, so a shared reference would let a mutating listener corrupt the caller's result. A `subagent/start` listener can still reach the live child via `ctx.agents.get(info.id)`; a `subagent/end` listener can only observe (the run has settled). Any run-affecting decision (continuation, injection that changes the run) is out of scope for this observe-only surface. +The service also announces provider lifecycle: `subagent/provider-added` (the live provider) fires after a registration and `subagent/provider-removed` (the name) after an unregistration, so a consumer deriving state from a named provider (the model-facing tool wording) mirrors registry membership instead of assuming load order — the cordis Loader starts sibling plugins concurrently, so "listed earlier" does not mean "registered earlier". The service emits `subagent/start` (payload `SubagentRunInfo`) and `subagent/end` (payload `SubagentRunEndInfo`) around the run — both **observe-only** (plain `emit`s; `subagent/end` fires from a detached `.then` and awaits no listener). `subagent/end` carries `lastAssistantMessage` (a deep clone of the child's final `output`) on the settle path, absent when the run rejected at the infrastructure level. The clone keeps the surface observe-only: the end emit fires from a detached `.then` before the caller's `await run.result` resumes, so a shared reference would let a mutating listener corrupt the caller's result. A `subagent/start` listener can still reach the live child via `ctx.agents.get(info.id)`; a `subagent/end` listener can only observe (the run has settled). Any run-affecting decision (continuation, injection that changes the run) is out of scope for this observe-only surface. ## Scope (first cut) diff --git a/packages/subagent/subagent/src/index.ts b/packages/subagent/subagent/src/index.ts index b0514edcac..a9541eabce 100644 --- a/packages/subagent/subagent/src/index.ts +++ b/packages/subagent/subagent/src/index.ts @@ -60,6 +60,25 @@ declare module 'cordis' { } interface Events { + /** + * A provider became resolvable in the {@link SubagentService} registry. + * Consumers that derive state from a named provider (e.g. the model-facing + * tool wording in `dsh-tool-subagent`) react HERE instead of assuming load + * order — the cordis Loader starts sibling plugins concurrently, so + * "listed earlier in cordis.yml" does not mean "registered earlier". + * @param provider - the provider that just registered, live in the registry. + * @mode emit + */ + 'subagent/provider-added'(provider: SubagentProvider): void + /** + * A provider left the registry (its plugin's fiber was disposed — an + * unload or an HMR reload). Consumers holding provider-derived state drop + * it here; a reload re-fires `subagent/provider-added` with the fresh + * provider. + * @param name - the registry name that no longer resolves. + * @mode emit + */ + 'subagent/provider-removed'(name: string): void /** * A subagent run started — emitted after the provider is resolved and its * capabilities validated, as the child run begins. Paired with @@ -130,7 +149,9 @@ export class SubagentService extends Service { /** * Register a provider under its `provider.name`. Throws {@link SubagentError} * (`DUPLICATE_PROVIDER`) if the name is already taken. Effect-scoped: disposed - * with the calling fiber (HMR-safe). + * with the calling fiber (HMR-safe). Emits `subagent/provider-added` after + * the registration and `subagent/provider-removed` on unregistration, so + * consumers can mirror provider lifecycle instead of assuming load order. * @param provider - the provider; its `name` is the registry key. * @returns the disposer that unregisters the provider. */ @@ -140,9 +161,14 @@ export class SubagentService extends Service { throw new SubagentError(`a subagent provider named "${provider.name}" is already registered`, 'DUPLICATE_PROVIDER') } this.providers.set(provider.name, provider) + // Yield the rollback BEFORE emitting `subagent/provider-added`: a + // throwing added-listener then unregisters the provider (and announces + // the removal) instead of leaking it into the registry. yield () => { this.providers.delete(provider.name) + this.ctx.emit('subagent/provider-removed', provider.name) } + this.ctx.emit('subagent/provider-added', provider) }.bind(this), 'subagents.registerProvider()') // ctx.effect's disposer returns Promise; our disposer API is // synchronous fire-and-forget — discard the (always-resolved) promise. diff --git a/packages/subagent/subagent/tests/service.spec.ts b/packages/subagent/subagent/tests/service.spec.ts index 6aa5dfe021..6b5e737a3d 100644 --- a/packages/subagent/subagent/tests/service.spec.ts +++ b/packages/subagent/subagent/tests/service.spec.ts @@ -45,6 +45,38 @@ function baseRequest(overrides: Partial = {}): SubagentSta } describe('SubagentService', () => { + it('announces provider lifecycle: added on register, removed on dispose', async () => { + const ctx = new Context() + await ctx.plugin(SubagentService) + const added: string[] = [] + const removed: string[] = [] + ctx.on('subagent/provider-added', provider => void added.push(provider.name)) + ctx.on('subagent/provider-removed', name => void removed.push(name)) + + const dispose = ctx.subagents.registerProvider(new StubProvider('alpha')) + expect(added).toEqual(['alpha']) + expect(removed).toEqual([]) + + dispose() + expect(removed).toEqual(['alpha']) + }) + + it('rolls back the registration when a provider-added listener throws', async () => { + const ctx = new Context() + await ctx.plugin(SubagentService) + let threw = false + const off = ctx.on('subagent/provider-added', () => { + if (!threw) { threw = true; throw new Error('boom added listener') } + }) + + expect(() => ctx.subagents.registerProvider(new StubProvider('alpha'))).toThrow('boom added listener') + expect(ctx.subagents.getProvider('alpha')).toBeUndefined() // nothing leaked + + off() + ctx.subagents.registerProvider(new StubProvider('alpha')) + expect(ctx.subagents.getProvider('alpha')).toBeDefined() + }) + it('registers a provider and starts a run on it by name', async () => { const ctx = new Context() await ctx.plugin(SubagentService) diff --git a/packages/subagent/tool-subagent/README.md b/packages/subagent/tool-subagent/README.md index df00f6f169..fd04fba3b2 100644 --- a/packages/subagent/tool-subagent/README.md +++ b/packages/subagent/tool-subagent/README.md @@ -8,7 +8,7 @@ This plugin binds to **exactly one** provider (`Config.provider`). The model see ## The description states the provider's context contract -The tool description and the `prompt` parameter description are DERIVED from the bound provider's `inheritsParentContext` (`providerWording`): a fresh-context provider (spawn, ACP) gets the standalone-prompt wording ("it does not see this conversation"), an inheriting provider (fork) tells the model the child already sees the conversation's completed turns and its prompt should state only what is new. Because the description is fixed at tool registration, `apply` resolves the provider at LOAD time and **throws if it is not registered yet — list the backend plugin before this one in `cordis.yml`**; a wiring mistake fails loudly at boot instead of shipping a lying description. +The tool description and the `prompt` parameter description are DERIVED from the bound provider's `inheritsParentContext` (`providerWording`): a fresh-context provider (spawn, ACP) gets the standalone-prompt wording ("it does not see this conversation"), an inheriting provider (fork) tells the model the child already sees the conversation's completed turns and its prompt should state only what is new. Because the description is fixed at tool registration, the tool **mirrors the provider's lifecycle** (`subagent/provider-added`/`-removed`): it registers when the bound provider is (or becomes) available and unregisters when the provider goes away — no load-order requirement (the cordis Loader starts sibling entries concurrently, so "listed first" never guaranteed "registered first"), and an HMR reload of the backend re-derives the wording from the fresh provider. While the provider is absent the tool simply does not exist (a `ctx.logger` note records the wait; a typo'd provider name shows up as a tool that never materializes). | Config key | Meaning | |---|---| diff --git a/packages/subagent/tool-subagent/src/index.ts b/packages/subagent/tool-subagent/src/index.ts index 766e3711f3..357172db9a 100644 --- a/packages/subagent/tool-subagent/src/index.ts +++ b/packages/subagent/tool-subagent/src/index.ts @@ -14,9 +14,11 @@ * The tool DESCRIPTION is derived from the bound provider's context contract * ({@link providerWording}): a fresh-context provider (spawn, ACP) gets the * standalone-prompt wording, an inheriting provider (fork) tells the model the - * child already sees the conversation's completed turns. `apply` therefore - * resolves the provider at load time and throws if it is not registered yet — - * list the backend plugin before this one in `cordis.yml`. + * child already sees the conversation's completed turns. The tool MIRRORS the + * provider's lifecycle via `subagent/provider-added`/`-removed` — it registers + * when the provider is (or becomes) available and unregisters when the + * provider goes away — so no load-order requirement exists and an HMR reload + * of the backend re-derives the wording from the fresh provider. * * Collection is SYNCHRONOUS this cut: `execute` starts a run and awaits * `run.result` inside a `try/finally` that always disposes the run, so the @@ -33,7 +35,7 @@ import z from 'schemastery' import { defineTool } from '@deepseek-ai/dsh-tools' import type { AgentOptions } from '@deepseek-ai/dsh-agent' import type { ContentBlock } from '@deepseek-ai/dsh-llm' -import type { SubagentResult, SubagentRun, SubagentStartRequest } from '@deepseek-ai/dsh-subagent' +import type { SubagentProvider, SubagentResult, SubagentRun, SubagentStartRequest } from '@deepseek-ai/dsh-subagent' export const name = 'tool-subagent' export const inject = ['tools', 'subagents'] @@ -136,73 +138,96 @@ export function providerWording(inherits: boolean): { description: string; promp } export function apply(ctx: Context, config: Config): void { - // Resolve the bound provider NOW: the tool description must state the - // provider's context contract, so the backend plugin must be loaded before - // this one (list it earlier in cordis.yml). Fail loud at load, not with a - // lying description at model time. - const provider = ctx.subagents.getProvider(config.provider) - if (provider === undefined) { - throw new Error( - `subagent provider "${config.provider}" is not registered; load its backend plugin before tool-subagent`) - } - const wording = providerWording(provider.inheritsParentContext) - ctx.tools.register(defineTool({ - name: config.toolName ?? 'subagent', - description: wording.description, - parameters: { - description: { - type: 'string', - required: true, - description: 'A short (3-5 word) description of the delegated task, for display.', + // The tool MIRRORS its provider's lifecycle instead of assuming load order: + // the cordis Loader starts sibling entries concurrently, so "backend listed + // first in cordis.yml" does not guarantee "provider registered first", and + // an HMR reload of the backend replaces the provider while this fiber stays + // loaded. Register the tool when the bound provider is (or becomes) + // available — deriving the wording from THAT provider — and unregister it + // when the provider goes away, so the description can never outlive or + // predate the provider it describes. + let disposeTool: (() => void) | undefined + const mount = (provider: SubagentProvider): void => { + const wording = providerWording(provider.inheritsParentContext) + disposeTool = ctx.tools.register(defineTool({ + name: config.toolName ?? 'subagent', + description: wording.description, + parameters: { + description: { + type: 'string', + required: true, + description: 'A short (3-5 word) description of the delegated task, for display.', + }, + prompt: { + type: 'string', + required: true, + description: wording.promptDescription, + }, }, - prompt: { - type: 'string', - required: true, - description: wording.promptDescription, - }, - }, - async execute(args, exec): Promise { - const parent = exec.agent - if (!parent) { - // The loop sets `exec.agent` for every model-driven call; its absence - // means a non-agent caller invoked the tool directly, which has no - // parent to attribute the child to. Fail loud rather than guess. - throw new Error('subagent tool requires a calling agent (exec.agent was undefined)') - } - - const request: SubagentStartRequest = { - prompt: [{ type: 'text', text: args.prompt }], - parent, - ...exec.signal ? { signal: exec.signal } : {}, - ...config.agentOptions ? { agentOptions: config.agentOptions } : {}, - } - - const run: SubagentRun = ctx.subagents.start(config.provider, request) - - // Bridge the tool's abort signal to the run: if the parent step is - // aborted while the child is in flight, cancel the child too. - const onAbort = (): void => { run.cancel('parent step aborted') } - exec.signal?.addEventListener('abort', onAbort, { once: true }) - // `addEventListener` does NOT fire for a signal already aborted before this - // line, so a step cancelled before the tool ran would never reach the - // child. Cancel explicitly in that case — the bridge must honor an - // already-aborted signal, not lean on each provider re-checking it. - if (exec.signal?.aborted) run.cancel('parent step aborted') - - try { - const result = await run.result - const error = stopReasonError(result) - if (error !== undefined) { - // Map a non-clean finish to an isError result (the registry turns a - // throw into an isError). Report the reason, not partial output. - throw new Error(error) + async execute(args, exec): Promise { + const parent = exec.agent + if (!parent) { + // The loop sets `exec.agent` for every model-driven call; its absence + // means a non-agent caller invoked the tool directly, which has no + // parent to attribute the child to. Fail loud rather than guess. + throw new Error('subagent tool requires a calling agent (exec.agent was undefined)') } - return [{ type: 'text', text: outputText(result.output) }] - } finally { - exec.signal?.removeEventListener('abort', onAbort) - // Always reach child quiescence — never leak a live idle child/session. - await run.dispose() - } - }, - })) + + const request: SubagentStartRequest = { + prompt: [{ type: 'text', text: args.prompt }], + parent, + ...exec.signal ? { signal: exec.signal } : {}, + ...config.agentOptions ? { agentOptions: config.agentOptions } : {}, + } + + const run: SubagentRun = ctx.subagents.start(config.provider, request) + + // Bridge the tool's abort signal to the run: if the parent step is + // aborted while the child is in flight, cancel the child too. + const onAbort = (): void => { run.cancel('parent step aborted') } + exec.signal?.addEventListener('abort', onAbort, { once: true }) + // `addEventListener` does NOT fire for a signal already aborted before this + // line, so a step cancelled before the tool ran would never reach the + // child. Cancel explicitly in that case — the bridge must honor an + // already-aborted signal, not lean on each provider re-checking it. + if (exec.signal?.aborted) run.cancel('parent step aborted') + + try { + const result = await run.result + const error = stopReasonError(result) + if (error !== undefined) { + // Map a non-clean finish to an isError result (the registry turns a + // throw into an isError). Report the reason, not partial output. + throw new Error(error) + } + return [{ type: 'text', text: outputText(result.output) }] + } finally { + exec.signal?.removeEventListener('abort', onAbort) + // Always reach child quiescence — never leak a live idle child/session. + await run.dispose() + } + }, + })) + } + + // Listeners first, then the presence check: both run synchronously, so no + // registration can slip between them; the `disposeTool === undefined` guard + // makes a same-tick added-event after a successful mount a no-op. + ctx.on('subagent/provider-added', (provider) => { + if (provider.name === config.provider && disposeTool === undefined) mount(provider) + }) + ctx.on('subagent/provider-removed', (name) => { + if (name !== config.provider || disposeTool === undefined) return + disposeTool() + disposeTool = undefined + }) + const present = ctx.subagents.getProvider(config.provider) + if (present !== undefined) { + mount(present) + } else { + // Not an error: the backend's fiber may simply activate after this one. + // The tool appears the moment the provider registers; a typo'd provider + // name shows up as this note plus a tool that never materializes. + ctx.logger.info(`subagent provider "${config.provider}" not registered yet; the "${config.toolName ?? 'subagent'}" tool will register when it appears`) + } } diff --git a/packages/subagent/tool-subagent/tests/tool-subagent.spec.ts b/packages/subagent/tool-subagent/tests/tool-subagent.spec.ts index 8f00c01366..3756806de3 100644 --- a/packages/subagent/tool-subagent/tests/tool-subagent.spec.ts +++ b/packages/subagent/tool-subagent/tests/tool-subagent.spec.ts @@ -195,19 +195,56 @@ describe('dsh-tool-subagent', () => { expect(text(result)).toContain('requires a calling agent') }) - it('fails loud AT LOAD when the bound provider is not registered (backend must load first)', async () => { - // The tool description states the provider's context contract, so apply() - // resolves the provider at load time — a missing backend is a wiring error - // surfaced immediately, not a lying description discovered at model time. + it('registers when the provider appears LATER — no load-order requirement (Loader starts siblings concurrently)', async () => { const ctx = new Context() await ctx.plugin(SystemPrompt) await ctx.plugin(ToolRegistry) await ctx.plugin(SubagentService) - await expect(async () => { - await ctx.plugin(tool, { provider: 'does-not-exist' }) - await new Promise(r => setTimeout(r, 20)) - }).rejects.toThrow('is not registered; load its backend plugin before tool-subagent') + // Tool first: no provider yet — the tool must be absent, not broken. + // Direct apply (schema bypass): also covers the waiting-note's default + // toolName fallback, which validated config pre-fills. + tool.apply(ctx, { provider: 'mock' }) expect(ctx.tools.schemas().some(s => s.name === 'subagent')).toBe(false) + // Backend arrives (as a delayed sibling fiber would): the tool appears. + await ctx.plugin(mock, { name: 'mock', reply: 'late but fine' }) + expect(ctx.tools.schemas().some(s => s.name === 'subagent')).toBe(true) + const result = await callSubagent(ctx, { description: 'd', prompt: 'p' }) + expect(text(result)).toBe('late but fine') + }) + + it('mirrors the provider lifecycle: gone on backend dispose, re-derived wording on re-registration', async () => { + const ctx = new Context() + await ctx.plugin(SystemPrompt) + await ctx.plugin(ToolRegistry) + await ctx.plugin(SubagentService) + const backend = await ctx.plugin(mock, { name: 'mock' }) // spawn-shaped (inherits: false) + await ctx.plugin(tool, { provider: 'mock' }) + expect(ctx.tools.schemas().find(s => s.name === 'subagent')!.description).toContain('does not see this conversation') + + // Backend unloads (HMR shape): the tool must not outlive its provider. + await backend.dispose() + expect(ctx.tools.schemas().some(s => s.name === 'subagent')).toBe(false) + + // Backend reloads with a DIFFERENT contract: the wording is re-derived + // from the fresh provider, not served stale from the first mount. + await ctx.plugin(mock, { name: 'mock', inheritsParentContext: true }) + expect(ctx.tools.schemas().find(s => s.name === 'subagent')!.description).toContain('INHERITS this conversation') + }) + + it('ignores lifecycle events for OTHER providers', async () => { + const ctx = new Context() + await ctx.plugin(SystemPrompt) + await ctx.plugin(ToolRegistry) + await ctx.plugin(SubagentService) + await ctx.plugin(mock, { name: 'mock' }) + await ctx.plugin(tool, { provider: 'mock' }) + // An unrelated provider registering (added-event with another name) and + // unregistering (removed-event with another name) must not touch the tool. + const other = await ctx.plugin(mock, { name: 'other', inheritsParentContext: true }) + expect(ctx.tools.schemas().filter(s => s.name === 'subagent')).toHaveLength(1) + expect(ctx.tools.schemas().find(s => s.name === 'subagent')!.description).toContain('does not see this conversation') + await other.dispose() + expect(ctx.tools.schemas().some(s => s.name === 'subagent')).toBe(true) }) it('derives spawn-shaped wording from a fresh-context provider (default mock)', async () => { From e890a3373e237a34861bfea2fd1570dfed0d9235 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sun, 5 Jul 2026 03:40:22 +0800 Subject: [PATCH 3/8] test(review): pin the tool-subagent plugin fiber's lifecycle ownership; doc nits Codex round-2 findings: - A committed test now proves the REAL plugin fiber (not a direct apply) owns the provider-lifecycle listeners: disposing a mounted tool's fiber unmounts the tool and leaves the provider intact, and a fiber disposed while WAITING never zombie-mounts when its provider arrives later. - TODO(subagent-dup-toolname) records the invalid-config blast radius of two waiting fibers sharing a toolName (the duplicate throw propagates through subagent/provider-added and rolls back the provider). - CONTEXT.md drops its creation-history sentence; the RFC's acceptance checklist becomes present-tense shipped invariants (docs/AGENTS.md writing rules). --- CONTEXT.md | 2 +- ...t-variables-and-tool-guidance-ownership.md | 6 ++--- packages/subagent/tool-subagent/src/index.ts | 7 ++++++ .../tool-subagent/tests/tool-subagent.spec.ts | 23 +++++++++++++++++++ 4 files changed, 34 insertions(+), 4 deletions(-) diff --git a/CONTEXT.md b/CONTEXT.md index 017fada6b8..4264c40107 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -1,6 +1,6 @@ # DeepSeek Harness -Ubiquitous language for the harness. Started during the 2026-07-05 system-prompt redesign session; grows as terms crystallize. Decisions live in `docs/rfc/` (this repo's ADR equivalent), not here. +Ubiquitous language for the harness; grows as terms crystallize. Decisions live in `docs/rfc/` (this repo's decision log), not here. ## Language — prompt assembly diff --git a/docs/rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md b/docs/rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md index 0802e56aac..7f5f4e7afe 100644 --- a/docs/rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md +++ b/docs/rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md @@ -62,9 +62,9 @@ Per-tool semantics and when-to-use live in tool DESCRIPTIONS, which already ship - Further variables (`date`, platform, git state) — the registry makes each a one-line contribution by whichever plugin owns the fact; none is claimed here. - A config `cwd` for pre-created stdio agents (would let the stdio persona use `{{cwd}}` and partition persistence by real path) — deferred until the session-cwd story is revisited. -## Acceptance criteria +## Shipped invariants -- `renderPrompt(assemble({agent}))` for the coding-agent example contains the persona FIRST (with the agent's model name interpolated), then fs/bash/web guidance sections; the loop contains no other prompt-composition path. +- `renderPrompt(assemble({ agent }))` for the coding-agent example renders the persona FIRST (with the agent's model name interpolated), then the fs/bash/web guidance sections; the loop has no other prompt-composition path. - The `subagent_fork` schema description says the child inherits the conversation; the `subagent` one says it does not. The tool follows its provider: absent before the backend activates, present after, gone when the backend unloads, re-worded from the fresh provider on reload. - Unknown/valueless/malformed/unbalanced `{{…}}` references throw with the section name in the message; duplicate section, variable, and tool-name registrations all throw. -- The gating runs (`test:coverage`, `test:snapshot`, `doc-sync`, `build`, `hygiene`) are green; no golden re-record is needed (replay never re-verifies the outgoing request). +- Snapshot goldens are prompt-independent by construction: llm-replay keys replay on (turn, step) chunk streams and never re-verifies the outgoing request. diff --git a/packages/subagent/tool-subagent/src/index.ts b/packages/subagent/tool-subagent/src/index.ts index 357172db9a..383bef0f0d 100644 --- a/packages/subagent/tool-subagent/src/index.ts +++ b/packages/subagent/tool-subagent/src/index.ts @@ -213,6 +213,13 @@ export function apply(ctx: Context, config: Config): void { // Listeners first, then the presence check: both run synchronously, so no // registration can slip between them; the `disposeTool === undefined` guard // makes a same-tick added-event after a successful mount a no-op. + // TODO(subagent-dup-toolname): two WAITING fibers configured with the same + // toolName collide only when their provider finally arrives — the duplicate + // tool-name throw then propagates through `subagent/provider-added` and + // rolls back the PROVIDER registration, so an invalid config blasts the + // backend's fiber instead of the misconfigured tool's. Config-time detection + // would need a cross-fiber registry of intended tool names; revisit if a + // real deployment ever hits it. ctx.on('subagent/provider-added', (provider) => { if (provider.name === config.provider && disposeTool === undefined) mount(provider) }) diff --git a/packages/subagent/tool-subagent/tests/tool-subagent.spec.ts b/packages/subagent/tool-subagent/tests/tool-subagent.spec.ts index 3756806de3..8799eba06b 100644 --- a/packages/subagent/tool-subagent/tests/tool-subagent.spec.ts +++ b/packages/subagent/tool-subagent/tests/tool-subagent.spec.ts @@ -231,6 +231,29 @@ describe('dsh-tool-subagent', () => { expect(ctx.tools.schemas().find(s => s.name === 'subagent')!.description).toContain('INHERITS this conversation') }) + it('the tool PLUGIN fiber owns its lifecycle listeners: disposal unmounts, and a disposed fiber never zombie-mounts', async () => { + const ctx = new Context() + await ctx.plugin(SystemPrompt) + await ctx.plugin(ToolRegistry) + await ctx.plugin(SubagentService) + + // Arm 1: a mounted tool dies with its plugin fiber; the provider survives. + await ctx.plugin(mock, { name: 'mock' }) + const mounted = await ctx.plugin(tool, { provider: 'mock' }) + expect(ctx.tools.schemas().some(s => s.name === 'subagent')).toBe(true) + await mounted.dispose() + expect(ctx.tools.schemas().some(s => s.name === 'subagent')).toBe(false) + expect(ctx.subagents.getProvider('mock')).toBeDefined() + + // Arm 2: a fiber disposed while WAITING must not react to the provider + // arriving later — a surviving listener would re-register a tool that no + // live plugin owns (the zombie mount). + const waiting = await ctx.plugin(tool, { provider: 'later', toolName: 'subagent_later' }) + await waiting.dispose() + await ctx.plugin(mock, { name: 'later' }) + expect(ctx.tools.schemas().some(s => s.name === 'subagent_later')).toBe(false) + }) + it('ignores lifecycle events for OTHER providers', async () => { const ctx = new Context() await ctx.plugin(SystemPrompt) From 2f191cc72b5ee294d48e9fcfce181f09685e3323 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sun, 5 Jul 2026 10:48:40 +0800 Subject: [PATCH 4/8] docs(agent): state the persona's template contract in its JSDoc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The AgentOptions.systemPrompt doc said the persona 'may reference' variables without saying that every complete {{...}} group IS interpreted, strictly, and that literal {{...}} prose has no escape syntax yet (the RFC's recorded deferral). A persona author reads this line first; it now carries the contract. (ds-review-bot inline finding: the escape mechanism itself stays deferred per the RFC — pre-release, no external consumers, and the failure is loud with the fix in the message.) --- docs/cordis-catalog/events-and-services.md | 22 +++++++++++----------- packages/core/agent/src/types.ts | 7 +++++-- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/docs/cordis-catalog/events-and-services.md b/docs/cordis-catalog/events-and-services.md index 6eede54b97..ef3671644c 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:255`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:258`](../../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:262`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:265`](../../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:401`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:404`](../../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:340`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:343`](../../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:353`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:356`](../../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:280`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:283`](../../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:366`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:369`](../../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:295`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:298`](../../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:271`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:274`](../../packages/core/agent/src/types.ts) #### `agent/step-result` — waterfall @@ -135,7 +135,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:376`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:379`](../../packages/core/agent/src/types.ts) #### `agent/turn-continuation` — waterfall @@ -147,7 +147,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:389`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:392`](../../packages/core/agent/src/types.ts) ### `fs/*` diff --git a/packages/core/agent/src/types.ts b/packages/core/agent/src/types.ts index 26dde9c764..3823b0db61 100644 --- a/packages/core/agent/src/types.ts +++ b/packages/core/agent/src/types.ts @@ -78,11 +78,14 @@ export interface AgentOptions { /** Model name (must have a registered adapter at call time). */ model?: string /** - * The agent's persona: a deployment-authored prompt-template fragment, + * The agent's persona: a deployment-authored prompt-TEMPLATE fragment, * rendered as the order-0 section of the assembled system prompt (before * all tool guidance). It may reference registered `{{variables}}` (e.g. * `{{model}}`, `{{cwd}}`); it is one section of the full prompt, never the - * whole. + * whole. Template, not free-form text: every complete `{{…}}` group IS + * interpreted, strictly — an unknown or malformed reference fails the turn + * loudly — and there is no escape syntax for literal `{{…}}` prose yet (a + * deliberate deferral; see the prompt-variables RFC). */ systemPrompt?: string } From 00cf8b693a1f877d76089c9de6843e08a137ab7b Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sun, 5 Jul 2026 11:37:32 +0800 Subject: [PATCH 5/8] feat(agent-loop): open every prompt with the harness identity section MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A static harness:identity section at order -100 — the first occupant of the documented negative band — states that the agent is powered by the DeepSeek Harness SDK before the deployment's persona renders. Harness attribution is a harness fact: it lives on the loop plugin, not in each deployment's persona, so every agent (subagents included) carries it and no YAML can forget it. A deployment that must drop it can remove the section in the system-prompt/assemble waterfall. Order-band docs updated in all five homes (PromptSection JSDoc, the system-prompt and agent-loop READMEs, architecture.md, the RFC). --- docs/architecture.md | 2 +- ...t-variables-and-tool-guidance-ownership.md | 2 +- packages/core/agent-loop/README.md | 2 +- packages/core/agent-loop/src/index.ts | 21 ++++++++++++------- packages/core/agent-loop/tests/loop.spec.ts | 16 +++++++------- packages/core/system-prompt/README.md | 4 ++-- packages/core/system-prompt/src/index.ts | 6 +++--- 7 files changed, 31 insertions(+), 22 deletions(-) diff --git a/docs/architecture.md b/docs/architecture.md index c9090c185a..6ad71af75b 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -69,7 +69,7 @@ A `Session` is an append-only log of typed `SessionEvent`s — the single source ## Prompt assembly (dsh-system-prompt) -Plugins contribute `PromptSection`s (named, ordered, static or computed from the per-call `AssembleContext`), tool-schema providers, and named **prompt variables** interpolated as `{{name}}` at render (strict: an unknown or valueless reference throws). `renderPrompt(assemble({ agent }))` IS the full prompt: the loop's `agent:persona` section (order 0) and its `model`/`cwd` variables carry the per-agent facts — no second composition path. Tool schemas are deliberately part of the assembly ([RFC](rfc/implemented/architecture/2026-06-11-tool-schemas-in-prompt-assembly.md)); prompt-fact ownership (persona vs description vs section vs variable) is pinned by [the prompt-variables RFC](rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md). +Plugins contribute `PromptSection`s (named, ordered, static or computed from the per-call `AssembleContext`), tool-schema providers, and named **prompt variables** interpolated as `{{name}}` at render (strict: an unknown or valueless reference throws). `renderPrompt(assemble({ agent }))` IS the full prompt: the loop's `harness:identity` (−100) and `agent:persona` (0) sections plus its `model`/`cwd` variables carry the harness and per-agent facts — no second composition path. Tool schemas are deliberately part of the assembly ([RFC](rfc/implemented/architecture/2026-06-11-tool-schemas-in-prompt-assembly.md)); prompt-fact ownership (persona vs description vs section vs variable) is pinned by [the prompt-variables RFC](rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md). ## Tool pipeline (dsh-tools) diff --git a/docs/rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md b/docs/rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md index 7f5f4e7afe..a47e1bc69f 100644 --- a/docs/rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md +++ b/docs/rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md @@ -30,7 +30,7 @@ Plugins contribute named values via `ctx.systemPrompt.variable(name, provider)`; ### Persona as the order-0 section -The loop plugin registers ONE section, `agent:persona` at order 0, whose text is `context.agent?.options.systemPrompt ?? ''`. The loop's special-case join is deleted: `fullSystemPrompt ≡ renderPrompt(assembly)`, one ordered pipeline for everything the model sees, and `agent/pre-step` (compaction's token-pressure input) measures exactly the real prompt. Order bands are now convention: persona `0`, tool guidance `100–199`, negative orders render before the persona. `AgentOptions.systemPrompt` keeps its familiar key but is documented as what it is — the persona template fragment, one section of the full prompt, never the whole (see `CONTEXT.md`). +The loop plugin registers ONE section, `agent:persona` at order 0, whose text is `context.agent?.options.systemPrompt ?? ''`. The loop's special-case join is deleted: `fullSystemPrompt ≡ renderPrompt(assembly)`, one ordered pipeline for everything the model sees, and `agent/pre-step` (compaction's token-pressure input) measures exactly the real prompt. Order bands are now convention: harness identity `-100` (the loop's static `harness:identity` section — every agent's prompt opens by stating it is powered by the DeepSeek Harness SDK), persona `0`, tool guidance `100–199`; other negative orders also render before the persona. `AgentOptions.systemPrompt` keeps its familiar key but is documented as what it is — the persona template fragment, one section of the full prompt, never the whole (see `CONTEXT.md`). ### Tool guidance ownership diff --git a/packages/core/agent-loop/README.md b/packages/core/agent-loop/README.md index fec733c34e..25480403bd 100644 --- a/packages/core/agent-loop/README.md +++ b/packages/core/agent-loop/README.md @@ -33,7 +33,7 @@ interface Config { } ``` -Agents listed in config are auto-created at startup. The plugin also registers the per-agent prompt pieces on `ctx.systemPrompt`: the `agent:persona` section (order 0 — `AgentOptions.systemPrompt` renders before all tool guidance) and the built-in `model`/`cwd` prompt variables, each resolved per step from the `assemble({ agent })` context. +Agents listed in config are auto-created at startup. The plugin also registers the harness-owned prompt pieces on `ctx.systemPrompt`: the `harness:identity` section (order −100 — every agent's prompt opens by stating it is powered by the DeepSeek Harness SDK), the `agent:persona` section (order 0 — `AgentOptions.systemPrompt` renders after it, before all tool guidance), and the built-in `model`/`cwd` prompt variables, resolved per step from the `assemble({ agent })` context. ### Classes diff --git a/packages/core/agent-loop/src/index.ts b/packages/core/agent-loop/src/index.ts index 8bc35a8f93..ed7243caf8 100644 --- a/packages/core/agent-loop/src/index.ts +++ b/packages/core/agent-loop/src/index.ts @@ -82,13 +82,20 @@ export class AgentLoop extends Service implements AgentFactory { // Provide the agent-creation factory to the registry (effect-scoped: the // slot is cleared on dispose). ctx.effect(() => this.ctx.agents.setFactory(this), 'agentLoop.setFactory()') - // The per-agent prompt pieces, registered once and resolved per assembly - // from the AssembleContext the loop passes (loop.ts assembles with - // `{ agent }` each step). The persona is the order-0 section — identity - // renders before all tool guidance; `{{model}}`/`{{cwd}}` are the built-in - // prompt variables projecting the agent's configured model and its - // session workspace. A provider returns undefined when the fact is absent - // (renderPrompt then rejects a persona that claims it — fail loud). + // The prompt pieces the harness itself owns, registered once. The + // harness-identity section states what every agent on this loop IS, + // ahead of everything (order −100 — before the deployment's persona); + // the persona is the order-0 section resolved per assembly from the + // AssembleContext the loop passes (loop.ts assembles with `{ agent }` + // each step); `{{model}}`/`{{cwd}}` are the built-in prompt variables + // projecting the agent's configured model and its session workspace. A + // provider returns undefined when the fact is absent (renderPrompt then + // rejects a persona that claims it — fail loud). + ctx.systemPrompt.section({ + name: 'harness:identity', + order: -100, + text: 'You are an AI agent powered by the DeepSeek Harness SDK.', + }) ctx.systemPrompt.section({ name: 'agent:persona', order: 0, diff --git a/packages/core/agent-loop/tests/loop.spec.ts b/packages/core/agent-loop/tests/loop.spec.ts index 6a0763814b..1a0525c0b2 100644 --- a/packages/core/agent-loop/tests/loop.spec.ts +++ b/packages/core/agent-loop/tests/loop.spec.ts @@ -141,7 +141,7 @@ describe('agent loop', () => { .toEqual({ diffs: [{ path: 'a.txt', oldText: null, newText: 'x' }] }) }) - it('renders the persona as the order-0 section — before tool guidance — with {{variables}} resolved', async () => { + it('renders harness identity, then the persona, then tool guidance — with {{variables}} resolved', async () => { const adapter = new MockAdapter([textResponse('ok')]) const ctx = await harness(adapter) ctx.systemPrompt.section({ name: 'tool:noop', order: 100, text: 'Use the noop tool wisely.' }) @@ -161,7 +161,7 @@ describe('agent loop', () => { await waitForIdle(ctx, agent) const request = adapter.requests[0] - expect(request!.system).toBe('You are a test agent on mock.\n\nUse the noop tool wisely.') + expect(request!.system).toBe('You are an AI agent powered by the DeepSeek Harness SDK.\n\nYou are a test agent on mock.\n\nUse the noop tool wisely.') expect(request!.tools?.map(t => t.name)).toEqual(['noop']) }) @@ -179,7 +179,7 @@ describe('agent loop', () => { send(agent, 'hi') await waitForIdle(ctx, agent) - expect(adapter.requests[0]!.system).toBe('Working in /work/space.') + expect(adapter.requests[0]!.system).toBe('You are an AI agent powered by the DeepSeek Harness SDK.\n\nWorking in /work/space.') }) it('contains a strict-variable render failure: the turn errors, the loop keeps serving turns', async () => { @@ -212,7 +212,7 @@ describe('agent loop', () => { await waitForIdle(ctx, agent) expect(adapter.requests).toHaveLength(1) - expect(adapter.requests[0]!.system).toBe('In /rescued.') + expect(adapter.requests[0]!.system).toBe('You are an AI agent powered by the DeepSeek Harness SDK.\n\nIn /rescued.') const turnEnds = agent.session.events.filter(e => e.type === 'turn/end') expect(turnEnds).toHaveLength(2) expect(turnEnds[1]?.type === 'turn/end' && turnEnds[1].data.reason.kind).toBe('completed') @@ -426,10 +426,12 @@ describe('agent loop', () => { send(agent, 'go') await waitForIdle(ctx, agent) - // One fire per step, in order, each with the assembled system prompt. + // One fire per step, in order, each with the assembled system prompt + // (here just the loop's own harness-identity section — no persona set). + const HARNESS = 'You are an AI agent powered by the DeepSeek Harness SDK.' expect(fires).toEqual([ - { turn: 1, step: 1, fullSystemPrompt: '' }, - { turn: 1, step: 2, fullSystemPrompt: '' }, + { turn: 1, step: 1, fullSystemPrompt: HARNESS }, + { turn: 1, step: 2, fullSystemPrompt: HARNESS }, ]) }) diff --git a/packages/core/system-prompt/README.md b/packages/core/system-prompt/README.md index 6207571618..5626d06a1c 100644 --- a/packages/core/system-prompt/README.md +++ b/packages/core/system-prompt/README.md @@ -21,7 +21,7 @@ System prompt assembly registry. Plugins contribute ordered text sections, tool- ### Key types - `AssembleContext` — what one `assemble()` call is FOR. Declared empty here and merge-extensible; `dsh-agent` declares `agent?: Agent`, so providers project per-agent facts. Providers must tolerate absent fields (a bare `assemble()` carries an empty context). -- `PromptSection` — `{ name, order, text: string | ((context) => string) }`. Sections are concatenated in ascending `order`. Order bands: `0` is the per-agent persona (registered by the agent loop), tool guidance uses `100–199`; negative orders render before the persona. +- `PromptSection` — `{ name, order, text: string | ((context) => string) }`. Sections are concatenated in ascending `order`. Order bands: `-100` is the harness identity, `0` the per-agent persona (both registered by the agent loop), tool guidance uses `100–199`; other negative orders also render before the persona. - `PromptAssembly` — `{ sections: AssembledSection[], tools: ToolSchema[], variables: Record }`. Section texts arrive resolved but not yet interpolated; `variables` holds every registered variable resolved against the context. Tool schemas are part of the assembly by design: "what the model is told it can do" is one coherent thing, even though adapters transmit schemas as a separate wire field. - `renderPrompt(assembly)` — interpolates `{{variable}}` references in each section, drops empty sections, joins with blank lines. STRICT: an unknown reference (`Object.hasOwn` lookup — prototype names like `{{constructor}}` are unknown), a registered-but-valueless reference, a malformed complete `{{…}}` group, or a `{{` that opens no complete group while a `}}` still follows (`{{{model}}}`) throws — fail loud beats shipping a malformed prompt. A lone `{{` with no `}}` anywhere after it passes through verbatim; substituted values are never re-scanned. @@ -29,7 +29,7 @@ Merge-extensible: plugins can declare extra fields on `PromptAssembly` and `Asse ### Extension points -- Section providers: tool packages own their cross-call guidance (`tool:bash`, `tool:read`, …); the agent loop owns `agent:persona`. +- Section providers: tool packages own their cross-call guidance (`tool:bash`, `tool:read`, …); the agent loop owns `harness:identity` and `agent:persona`. - Variable providers: the agent loop registers `model` and `cwd`; any plugin can register the facts it owns (a future `date`, git state, …). - Tool schema providers: `ToolRegistry` registers itself as a tool provider automatically. - The `system-prompt/assemble` waterfall: mutate or replace the assembly per caller (dynamic tool filtering, extra variables). diff --git a/packages/core/system-prompt/src/index.ts b/packages/core/system-prompt/src/index.ts index 62889a2793..4da727f4e5 100644 --- a/packages/core/system-prompt/src/index.ts +++ b/packages/core/system-prompt/src/index.ts @@ -54,9 +54,9 @@ export interface PromptSection { /** Unique name — a duplicate registration throws (see {@link SystemPrompt.section}). */ name: string /** - * Sections are concatenated in ascending order. Convention: `0` is the - * per-agent persona, tool guidance uses 100–199; negative orders render - * before the persona. + * Sections are concatenated in ascending order. Convention: `-100` is the + * harness identity, `0` the per-agent persona, tool guidance uses 100–199; + * other negative orders also render before the persona. */ order: number /** From 3f83a4ee96a428e9022eebe9b18037e0062e1c50 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sun, 5 Jul 2026 23:23:46 +0800 Subject: [PATCH 6/8] review: the persona becomes the system-prompt plugin's deployment config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review round 2 (tianyicui inline comments): - dsh-system-prompt itself registers the harness:identity (-100) and deployment:persona (0) sections — they must survive a swapped loop plugin, so they leave dsh-agent-loop; the persona text is the plugin's own validated 'persona' config. The model/cwd variables STAY on the loop: runtime facts of the agents it drives. - AgentOptions.systemPrompt is deleted with all its forwarding plumbing: the app configs' systemPrompt keys become 'persona' routed through dsh-agent-core (schema = z.intersect of the owners'), the ACP bridge and tool-subagent stop carrying persona configuration, and subagent children now render the deployment persona like every other agent. - Example personas drop transport/interface trivia (ACP, CLI) — facts irrelevant to the model. - Root CONTEXT.md removed (not idiomatic); its persona definition was wrong under the new ownership anyway. - Docs, READMEs, the prompt-variables RFC, and generated catalogs updated; new loop test pins the assemble-waterfall escape valve (an emptied assembly sends NO system field). --- CONTEXT.md | 37 -------- docs/architecture.md | 2 +- docs/cordis-catalog/events.md | 26 +++--- docs/cordis-catalog/services.md | 4 +- docs/core-data-structures/core.md | 2 +- docs/event-producer-consumer.md | 26 +++--- ...t-variables-and-tool-guidance-ownership.md | 6 +- ...-04-trim-acp-bridge-unreachable-surface.md | 2 +- examples/acp-agent/cordis.yml | 15 +-- examples/coding-agent/cordis.yml | 11 ++- .../coding-agent/tests/coding-task.e2e.ts | 7 +- examples/coding-agent/tests/compaction.e2e.ts | 6 +- examples/coding-agent/tests/full-loop.e2e.ts | 7 +- examples/coding-agent/tests/harness.ts | 7 +- examples/coding-agent/tests/resume.e2e.ts | 8 +- examples/coding-agent/tests/todo-write.e2e.ts | 7 +- examples/echo-agent/cordis.yml | 2 +- packages/bash/tool-bash/tests/tools.spec.ts | 5 +- packages/core/agent-core/README.md | 6 +- packages/core/agent-core/package.json | 3 + packages/core/agent-core/src/index.ts | 45 +++++---- .../core/agent-core/tests/agent-core.spec.ts | 20 +++- packages/core/agent-core/tsconfig.json | 3 + packages/core/agent-loop/README.md | 3 +- packages/core/agent-loop/src/index.ts | 28 ++---- .../tests/config-session-id.spec.ts | 8 +- packages/core/agent-loop/tests/loop.spec.ts | 38 +++++--- packages/core/agent/src/index.ts | 4 +- packages/core/agent/src/types.ts | 20 +--- packages/core/system-prompt/README.md | 14 ++- packages/core/system-prompt/package.json | 3 + packages/core/system-prompt/src/index.ts | 52 ++++++++++- .../system-prompt/tests/system-prompt.spec.ts | 91 ++++++++++++++----- packages/core/system-prompt/tsconfig.json | 3 + packages/fs/tool-fs/tests/fs-tools.e2e.ts | 8 +- packages/fs/tool-fs/tests/harness.ts | 7 +- packages/fs/tool-fs/tests/tools.spec.ts | 5 +- .../subagent/subagent-inprocess/src/index.ts | 6 +- .../subagent/subagent-spawn/tests/harness.ts | 5 +- .../subagent-spawn/tests/spawn.e2e.ts | 6 +- packages/subagent/tool-subagent/README.md | 2 +- packages/subagent/tool-subagent/src/index.ts | 7 +- .../tool-subagent/tests/tool-subagent.spec.ts | 4 +- packages/ui/acp-agent/README.md | 2 +- packages/ui/acp-agent/src/index.ts | 27 +++--- packages/ui/acp-agent/tests/acp-agent.spec.ts | 5 +- packages/ui/acp/README.md | 3 +- packages/ui/acp/src/index.ts | 6 +- packages/ui/acp/tests/bridge.spec.ts | 8 +- packages/ui/acp/tests/harness.ts | 4 +- packages/ui/acp/tests/stream-update.spec.ts | 2 - packages/ui/stdio-agent/README.md | 6 +- packages/ui/stdio-agent/src/index.ts | 19 ++-- .../ui/stdio-agent/tests/stdio-agent.spec.ts | 12 ++- pnpm-lock.yaml | 8 ++ 55 files changed, 389 insertions(+), 284 deletions(-) delete mode 100644 CONTEXT.md diff --git a/CONTEXT.md b/CONTEXT.md deleted file mode 100644 index 4264c40107..0000000000 --- a/CONTEXT.md +++ /dev/null @@ -1,37 +0,0 @@ -# DeepSeek Harness - -Ubiquitous language for the harness; grows as terms crystallize. Decisions live in `docs/rfc/` (this repo's decision log), not here. - -## Language — prompt assembly - -**Section**: -One named, ordered fragment of the system prompt, contributed by a plugin through `ctx.systemPrompt.section()`. -_Avoid_: block, snippet - -**Assembly**: -The collated output of `assemble()` — sections, tool schemas, and resolved prompt variables — before rendering. - -**Full system prompt**: -The rendered text the model actually receives: all sections interpolated and joined. There is no other composition path. -_Avoid_: using "system prompt" for any single fragment - -**Persona**: -The per-agent, deployment-authored prompt fragment (config key `systemPrompt` on an agent). A template, not final text; rendered as the order-0 section. It is one section of the full system prompt, never the whole. -_Avoid_: calling it "the system prompt" - -**Prompt variable**: -A named per-assembly value contributed by a plugin (e.g. `model`) and referenced from section or persona text as `{{name}}`. -_Avoid_: placeholder, macro - -**Assemble context**: -The per-agent input to one `assemble()` call, carrying which agent the prompt is for. Merge-extensible; variable providers and section text providers are functions of it. - -**Tool guidance**: -The model-facing usage prose for one tool, owned by the tool's package as a section (order band 100–199) — never hand-written in leaf config. -_Avoid_: tool prompt, tool docs - -## Language — subagents - -**Context contract**: -Whether a subagent provider's child sees the parent conversation (`inheritsParentContext`): fork inherits the log, spawn and ACP start fresh. Declared by the provider, consumed by tool wording. - diff --git a/docs/architecture.md b/docs/architecture.md index 48ca1a8a22..d24b142869 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -88,7 +88,7 @@ forever: checkpoint persistence and notify idle/running status ``` -Prompt assembly is single-path: `renderPrompt(assemble({ agent }))` IS the system prompt sent to the model. Plugins contribute ordered sections (static or computed from the per-call `AssembleContext`), tool schemas, and named variables interpolated as `{{name}}` at render — strictly, so an unknown or valueless reference fails the turn instead of shipping a hole. The loop itself registers the `harness:identity` (order −100) and `agent:persona` (order 0) sections and the `model`/`cwd` variables; prompt-fact ownership is pinned by the [prompt-variables RFC](rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md). +Prompt assembly is single-path: `renderPrompt(assemble({ agent }))` IS the system prompt sent to the model. Plugins contribute ordered sections (static or computed from the per-call `AssembleContext`), tool schemas, and named variables interpolated as `{{name}}` at render — strictly, so an unknown or valueless reference fails the turn instead of shipping a hole. `dsh-system-prompt` itself owns the openers — the static `harness:identity` section (order −100) and the deployment's persona (order 0, from its `persona` config, shared by every agent in the context) — while the shipped loop registers the `model`/`cwd` variables; prompt-fact ownership is pinned by the [prompt-variables RFC](rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md). Post-tool context lands after all tool results so tool-call/result adjacency stays stable. Steering drains between steps; leftover steering after a turn is re-queued as ordinary input. diff --git a/docs/cordis-catalog/events.md b/docs/cordis-catalog/events.md index 549d400d0e..8db44c5f1b 100644 --- a/docs/cordis-catalog/events.md +++ b/docs/cordis-catalog/events.md @@ -23,7 +23,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:258`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:248`](../../packages/core/agent/src/types.ts) ### `agent/disposed` — emit @@ -35,7 +35,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:265`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:255`](../../packages/core/agent/src/types.ts) ### `agent/error` — emit @@ -47,7 +47,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:404`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:394`](../../packages/core/agent/src/types.ts) ### `agent/pre-step` — serial @@ -61,7 +61,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:343`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:333`](../../packages/core/agent/src/types.ts) ### `agent/prompt-submit` — waterfall @@ -73,7 +73,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:356`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:346`](../../packages/core/agent/src/types.ts) ### `agent/queued` — emit @@ -85,7 +85,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:283`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:273`](../../packages/core/agent/src/types.ts) ### `agent/request` — waterfall @@ -97,7 +97,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:369`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:359`](../../packages/core/agent/src/types.ts) ### `agent/session-start` — emit @@ -109,7 +109,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:298`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:288`](../../packages/core/agent/src/types.ts) ### `agent/status` — emit @@ -121,7 +121,7 @@ Agent status changed (`idle` ⇄ `running`, or → `disposed`). Drive lifecycle Types: [Agent](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:274`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:264`](../../packages/core/agent/src/types.ts) ### `agent/step-result` — waterfall @@ -133,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:379`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:369`](../../packages/core/agent/src/types.ts) ### `agent/turn-continuation` — waterfall @@ -145,7 +145,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:392`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:382`](../../packages/core/agent/src/types.ts) ## `fs/*` @@ -285,7 +285,7 @@ Waterfall around prompt assembly — mutate or extend the PromptAssembly (sectio 'system-prompt/assemble'(this: SystemPrompt, assembly: PromptAssembly, context: AssembleContext, next: () => Promise): Promise ``` -Source: [`packages/core/system-prompt/src/index.ts:32`](../../packages/core/system-prompt/src/index.ts) +Source: [`packages/core/system-prompt/src/index.ts:38`](../../packages/core/system-prompt/src/index.ts) ### `system-prompt/change` — emit @@ -295,7 +295,7 @@ A section, tool provider, or variable provider was registered or unregistered (t 'system-prompt/change'(): void ``` -Source: [`packages/core/system-prompt/src/index.ts:38`](../../packages/core/system-prompt/src/index.ts) +Source: [`packages/core/system-prompt/src/index.ts:44`](../../packages/core/system-prompt/src/index.ts) ## `tools/*` diff --git a/docs/cordis-catalog/services.md b/docs/cordis-catalog/services.md index 1601d78920..ca2ee64334 100644 --- a/docs/cordis-catalog/services.md +++ b/docs/cordis-catalog/services.md @@ -180,7 +180,7 @@ Source: [`packages/subagent/subagent/src/index.ts:142`](../../packages/subagent/ ## `ctx.systemPrompt` — `SystemPrompt` -Registry service (`ctx.systemPrompt`): plugins contribute ordered text sections, tool-schema providers, and named prompt variables; the agent loop calls `assemble(context)` once per step. +Registry service (`ctx.systemPrompt`): plugins contribute ordered text sections, tool-schema providers, and named prompt variables; the agent loop calls `assemble(context)` once per step. Registers the harness-owned `harness:identity` and `deployment:persona` sections itself (see Config.persona). ```ts cordis-catalog section(section: PromptSection): () => void @@ -189,7 +189,7 @@ variable(name: string, provider: (context: AssembleContext) => string | undefine assemble(context: AssembleContext = {}): Promise ``` -Source: [`packages/core/system-prompt/src/index.ts:174`](../../packages/core/system-prompt/src/index.ts) +Source: [`packages/core/system-prompt/src/index.ts:198`](../../packages/core/system-prompt/src/index.ts) ## `ctx.tools` — `ToolRegistry` diff --git a/docs/core-data-structures/core.md b/docs/core-data-structures/core.md index 7f38abe985..61f980ff73 100644 --- a/docs/core-data-structures/core.md +++ b/docs/core-data-structures/core.md @@ -303,7 +303,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 emits incl. `agent/session-start`, the serial `agent/pre-step` surface-mutation seam, and the `agent/prompt-submit`/`agent/request`/`agent/step-result`/`agent/turn-continuation` waterfalls) is in [architecture.md § Event taxonomy](../architecture.md#event-taxonomy); turn/step boundaries are durable `session/event` records, not `agent/*` emits. +`AgentStatus` is `'idle' | 'running' | 'disposed'`. `AgentId` is a branded string. `AgentOptions` (`model?`) is merge-extensible — plugins add creation options by declaration merging; the persona is NOT an agent option but the `dsh-system-prompt` plugin's `persona` config, shared context-wide. The `agent/*` event taxonomy (lifecycle emits incl. `agent/session-start`, the serial `agent/pre-step` surface-mutation seam, and the `agent/prompt-submit`/`agent/request`/`agent/step-result`/`agent/turn-continuation` waterfalls) is in [architecture.md § Event taxonomy](../architecture.md#event-taxonomy); turn/step boundaries are durable `session/event` records, not `agent/*` emits. ## Interception decisions diff --git a/docs/event-producer-consumer.md b/docs/event-producer-consumer.md index e27e6fcf28..f6cd631296 100644 --- a/docs/event-producer-consumer.md +++ b/docs/event-producer-consumer.md @@ -7,17 +7,17 @@ This matrix shows which packages dispatch each harness-owned event and which pac | Event | Mode | Declared in | Dispatchers | Listeners | | --- | --- | --- | --- | --- | -| `agent/created` | `emit` | [`packages/core/agent/src/types.ts:258`](../packages/core/agent/src/types.ts) | [`agent`](../packages/core/agent) (`emit`) | [`stdio-agent`](../packages/ui/stdio-agent) | -| `agent/disposed` | `emit` | [`packages/core/agent/src/types.ts:265`](../packages/core/agent/src/types.ts) | [`agent`](../packages/core/agent) (`emit`) | [`stdio-agent`](../packages/ui/stdio-agent) | -| `agent/error` | `emit` | [`packages/core/agent/src/types.ts:404`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`emit`) | - | -| `agent/pre-step` | `serial` | [`packages/core/agent/src/types.ts:343`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`serial`) | [`compact-basic`](../packages/compact/compact-basic) | -| `agent/prompt-submit` | `waterfall` | [`packages/core/agent/src/types.ts:356`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`waterfall`) | [`hooks-claude`](../packages/hooks/hooks-claude), [`hooks-codex`](../packages/hooks/hooks-codex) | -| `agent/queued` | `emit` | [`packages/core/agent/src/types.ts:283`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`emit`) | - | -| `agent/request` | `waterfall` | [`packages/core/agent/src/types.ts:369`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`waterfall`), [`compact-basic`](../packages/compact/compact-basic) (`waterfall`) | - | -| `agent/session-start` | `emit` | [`packages/core/agent/src/types.ts:298`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`emit`) | [`hooks-claude`](../packages/hooks/hooks-claude), [`hooks-codex`](../packages/hooks/hooks-codex) | -| `agent/status` | `emit` | [`packages/core/agent/src/types.ts:274`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`emit`) | [`acp`](../packages/ui/acp), [`invariants`](../packages/support/invariants), [`stdio-agent`](../packages/ui/stdio-agent) | -| `agent/step-result` | `waterfall` | [`packages/core/agent/src/types.ts:379`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`waterfall`) | - | -| `agent/turn-continuation` | `waterfall` | [`packages/core/agent/src/types.ts:392`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`waterfall`) | [`hooks-claude`](../packages/hooks/hooks-claude), [`hooks-codex`](../packages/hooks/hooks-codex) | +| `agent/created` | `emit` | [`packages/core/agent/src/types.ts:248`](../packages/core/agent/src/types.ts) | [`agent`](../packages/core/agent) (`emit`) | [`stdio-agent`](../packages/ui/stdio-agent) | +| `agent/disposed` | `emit` | [`packages/core/agent/src/types.ts:255`](../packages/core/agent/src/types.ts) | [`agent`](../packages/core/agent) (`emit`) | [`stdio-agent`](../packages/ui/stdio-agent) | +| `agent/error` | `emit` | [`packages/core/agent/src/types.ts:394`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`emit`) | - | +| `agent/pre-step` | `serial` | [`packages/core/agent/src/types.ts:333`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`serial`) | [`compact-basic`](../packages/compact/compact-basic) | +| `agent/prompt-submit` | `waterfall` | [`packages/core/agent/src/types.ts:346`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`waterfall`) | [`hooks-claude`](../packages/hooks/hooks-claude), [`hooks-codex`](../packages/hooks/hooks-codex) | +| `agent/queued` | `emit` | [`packages/core/agent/src/types.ts:273`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`emit`) | - | +| `agent/request` | `waterfall` | [`packages/core/agent/src/types.ts:359`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`waterfall`), [`compact-basic`](../packages/compact/compact-basic) (`waterfall`) | - | +| `agent/session-start` | `emit` | [`packages/core/agent/src/types.ts:288`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`emit`) | [`hooks-claude`](../packages/hooks/hooks-claude), [`hooks-codex`](../packages/hooks/hooks-codex) | +| `agent/status` | `emit` | [`packages/core/agent/src/types.ts:264`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`emit`) | [`acp`](../packages/ui/acp), [`invariants`](../packages/support/invariants), [`stdio-agent`](../packages/ui/stdio-agent) | +| `agent/step-result` | `waterfall` | [`packages/core/agent/src/types.ts:369`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`waterfall`) | - | +| `agent/turn-continuation` | `waterfall` | [`packages/core/agent/src/types.ts:382`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`waterfall`) | [`hooks-claude`](../packages/hooks/hooks-claude), [`hooks-codex`](../packages/hooks/hooks-codex) | | `fs/edit-intent` | `waterfall` | [`packages/fs/fs/src/index.ts:123`](../packages/fs/fs/src/index.ts) | [`tool-fs`](../packages/fs/tool-fs) (`waterfall`) | [`fs-policy`](../packages/fs/fs-policy) | | `fs/observed` | `emit` | [`packages/fs/fs/src/index.ts:138`](../packages/fs/fs/src/index.ts) | [`tool-fs`](../packages/fs/tool-fs) (`emit`) | [`fs-policy`](../packages/fs/fs-policy) | | `fs/write-intent` | `waterfall` | [`packages/fs/fs/src/index.ts:109`](../packages/fs/fs/src/index.ts) | [`tool-fs`](../packages/fs/tool-fs) (`waterfall`) | [`fs-policy`](../packages/fs/fs-policy) | @@ -29,8 +29,8 @@ This matrix shows which packages dispatch each harness-owned event and which pac | `subagent/provider-added` | `emit` | [`packages/subagent/subagent/src/index.ts:72`](../packages/subagent/subagent/src/index.ts) | [`subagent`](../packages/subagent/subagent) (`emit`) | [`tool-subagent`](../packages/subagent/tool-subagent) | | `subagent/provider-removed` | `emit` | [`packages/subagent/subagent/src/index.ts:81`](../packages/subagent/subagent/src/index.ts) | [`subagent`](../packages/subagent/subagent) (`emit`) | [`tool-subagent`](../packages/subagent/tool-subagent) | | `subagent/start` | `emit` | [`packages/subagent/subagent/src/index.ts:89`](../packages/subagent/subagent/src/index.ts) | [`subagent`](../packages/subagent/subagent) (`events.dispatch`) | [`hooks-claude`](../packages/hooks/hooks-claude) | -| `system-prompt/assemble` | `waterfall` | [`packages/core/system-prompt/src/index.ts:32`](../packages/core/system-prompt/src/index.ts) | [`system-prompt`](../packages/core/system-prompt) (`waterfall`) | - | -| `system-prompt/change` | `emit` | [`packages/core/system-prompt/src/index.ts:38`](../packages/core/system-prompt/src/index.ts) | [`system-prompt`](../packages/core/system-prompt) (`emit`) | - | +| `system-prompt/assemble` | `waterfall` | [`packages/core/system-prompt/src/index.ts:38`](../packages/core/system-prompt/src/index.ts) | [`system-prompt`](../packages/core/system-prompt) (`waterfall`) | - | +| `system-prompt/change` | `emit` | [`packages/core/system-prompt/src/index.ts:44`](../packages/core/system-prompt/src/index.ts) | [`system-prompt`](../packages/core/system-prompt) (`emit`) | - | | `tools/change` | `emit` | [`packages/core/tools/src/index.ts:87`](../packages/core/tools/src/index.ts) | [`tools`](../packages/core/tools) (`emit`) | - | | `tools/post-execute` | `waterfall` | [`packages/core/tools/src/index.ts:82`](../packages/core/tools/src/index.ts) | [`tools`](../packages/core/tools) (`waterfall`) | [`hooks-claude`](../packages/hooks/hooks-claude), [`hooks-codex`](../packages/hooks/hooks-codex) | | `tools/pre-execute` | `waterfall` | [`packages/core/tools/src/index.ts:66`](../packages/core/tools/src/index.ts) | [`tools`](../packages/core/tools) (`waterfall`) | [`hooks-claude`](../packages/hooks/hooks-claude), [`hooks-codex`](../packages/hooks/hooks-codex) | diff --git a/docs/rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md b/docs/rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md index a47e1bc69f..eafd42cf9e 100644 --- a/docs/rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md +++ b/docs/rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md @@ -26,11 +26,11 @@ The assembled system prompt had four defects, all of one family: facts the harne Plugins contribute named values via `ctx.systemPrompt.variable(name, provider)`; prompt text references them as `{{name}}`. Providers are functions of the `AssembleContext` and may return `undefined` — "no value for THIS assembly". `assemble()` resolves every registered variable into `PromptAssembly.variables` (waterfall listeners can see, add, or override); `renderPrompt` interpolates. Rendering is STRICT — fail loud beats shipping a malformed prompt: a reference to an unregistered name throws (listing what exists; lookup is `Object.hasOwn`, so a prototype property like `{{constructor}}` is unknown, not a function spliced into the prompt), a registered-but-valueless reference throws, a complete `{{…}}` group that is not a well-formed name (`[a-z][a-z0-9_]*`, e.g. `{{ model }}`) throws, and a `{{` that opens no complete group while a `}}` still follows (`{{{model}}}`, `{{a{b}}`) throws. A lone `{{` with no `}}` anywhere after it is ordinary prose and passes through verbatim; substituted values are never re-scanned. Registration rejects duplicate and unreferenceable names, mirroring the tool registry — and `section()` now rejects duplicate section names, making the documented dedup real. -`dsh-agent-loop` registers the two built-ins, both pure projections of the context agent: `model` (= `options.model`) and `cwd` (= `session.header.cwd`). The example personas write `powered by the {{model}} model` — the model name is stated once, in the `model:` config key. `{{cwd}}` is demonstrated in the ACP example only: every ACP session carries the client's cwd, while config-pre-created stdio agents have none (a persona claiming `{{cwd}}` there fails the turn — by design). +`dsh-agent-loop` registers the two built-ins, both pure projections of the context agent: `model` (= `options.model`) and `cwd` (= `session.header.cwd`). The example personas write `powered by the {{model}} model` — the model name is stated once, in the `model:` config key. `{{cwd}}` is demonstrated in the ACP example only: every ACP session carries the client's cwd, while config-pre-created stdio agents have none (a persona claiming `{{cwd}}` there fails the turn — by design). The variables stay on the loop plugin (unlike the sections below): they are runtime facts of the agents THIS loop drives, and a replacement loop supplies its own. ### Persona as the order-0 section -The loop plugin registers ONE section, `agent:persona` at order 0, whose text is `context.agent?.options.systemPrompt ?? ''`. The loop's special-case join is deleted: `fullSystemPrompt ≡ renderPrompt(assembly)`, one ordered pipeline for everything the model sees, and `agent/pre-step` (compaction's token-pressure input) measures exactly the real prompt. Order bands are now convention: harness identity `-100` (the loop's static `harness:identity` section — every agent's prompt opens by stating it is powered by the DeepSeek Harness SDK), persona `0`, tool guidance `100–199`; other negative orders also render before the persona. `AgentOptions.systemPrompt` keeps its familiar key but is documented as what it is — the persona template fragment, one section of the full prompt, never the whole (see `CONTEXT.md`). +`dsh-system-prompt` itself registers the two harness-owned sections (they must survive a swapped loop plugin, so they do NOT live on `dsh-agent-loop`): the static `harness:identity` at order `-100` — every prompt opens by stating the agent is powered by the DeepSeek Harness SDK — and `deployment:persona` at order 0, whose text is the plugin's own `persona` config. The persona is per-DEPLOYMENT, not per-agent: every agent in the context (subagents included) renders the same one, `AgentOptions.systemPrompt` is deleted along with the per-agent forwarding plumbing (the app configs' `systemPrompt` keys become a `persona` key routed to this plugin through `dsh-agent-core`), and the ACP bridge and `dsh-tool-subagent` stop carrying persona configuration entirely. The loop's special-case join is deleted: `fullSystemPrompt ≡ renderPrompt(assembly)`, one ordered pipeline for everything the model sees, and `agent/pre-step` (compaction's token-pressure input) measures exactly the real prompt. Order bands are now convention: harness identity `-100`, persona `0`, tool guidance `100–199`; other negative orders also render before the persona. ### Tool guidance ownership @@ -42,7 +42,7 @@ Per-tool semantics and when-to-use live in tool DESCRIPTIONS, which already ship ## Rejected alternatives -- **The loop composes an identity line itself** — hardcodes model-facing prose in the one package that must stay thin ("plugins, not loop changes"), and contradicts `dsh-system-prompt`'s "no hardcoded prompt text" stance. +- **The loop composes an identity line itself** — hardcodes model-facing prose in the one package that must stay thin ("plugins, not loop changes"), and outside the section pipeline it would be a second composition path. (The identity DOES ship as a code literal — but as an ordinary section registered by `dsh-system-prompt`, whose `system-prompt/assemble` waterfall remains the escape valve for a deployment that must drop it.) - **Inject the model name via the `agent/request` waterfall** — prompt text composed in two places, and `agent/pre-step`'s `fullSystemPrompt` would omit it, so compaction would measure a prompt that is not what the model sees. - **Hand-write the model name in each persona** — duplicates the `model:` key one line above and silently lies after a config edit; the exact disease this RFC cures. - **Lenient interpolation (leave unknown refs verbatim, or substitute empty)** — a typo ships `{{modle}}` (or a hole) to the model and nobody notices until transcript review. diff --git a/docs/rfc/implemented/simplification/2026-07-04-trim-acp-bridge-unreachable-surface.md b/docs/rfc/implemented/simplification/2026-07-04-trim-acp-bridge-unreachable-surface.md index 3df0e0322a..727b59f4d4 100644 --- a/docs/rfc/implemented/simplification/2026-07-04-trim-acp-bridge-unreachable-surface.md +++ b/docs/rfc/implemented/simplification/2026-07-04-trim-acp-bridge-unreachable-surface.md @@ -6,7 +6,7 @@ Status: implemented (accepted 2026-07-04) Two pieces of `dsh-acp` surface were unreachable from any shipped configuration: -1. **`AcpConfig.agentName` / `agentVersion`** (`packages/ui/acp/src/index.ts`). The shipped app package hands the bridge only `{ model, systemPrompt }` (`packages/ui/acp-agent/src/index.ts`), so no leaf `cordis.yml` — the only production config surface — could set the knobs at all; they were settable solely by direct-mounting the bridge, which only a unit test did. Every snapshot golden — the hook-matrix scenarios included — pins the schema defaults (`deepseek-harness-acp` / `0.0.1`). The pair also carried a live `TODO(double-default)`: the literals existed twice (schema `.default(...)` plus `??` fallbacks), with the TODO asking to pick one home. +1. **`AcpConfig.agentName` / `agentVersion`** (`packages/ui/acp/src/index.ts`). The shipped app package hands the bridge only `{ model }` (`packages/ui/acp-agent/src/index.ts`), so no leaf `cordis.yml` — the only production config surface — could set the knobs at all; they were settable solely by direct-mounting the bridge, which only a unit test did. Every snapshot golden — the hook-matrix scenarios included — pins the schema defaults (`deepseek-harness-acp` / `0.0.1`). The pair also carried a live `TODO(double-default)`: the literals existed twice (schema `.default(...)` plus `??` fallbacks), with the TODO asking to pick one home. 2. **The `toolKindFor` name heuristic** (same file) special-cased `bash*`/`read*`/`write`/`edit*` tool names in the generic-fallback path. Since the [render-intent union](../architecture/2026-07-02-tool-render-intent-union.md), every first-party tool those arms matched ships its own `presentCall` carrying its kind, and the presenter-less production tools (`subagent`, `subagent_fork`) fell through to `other` anyway. The arms were production-reachable only when a tool declined to present its own call — a `presentCall` that THROWS (the containment fallback), or model arguments that fail the tool's schema so `defineTool`'s `presentCall` wrapper returns `undefined` (e.g. a `bash` call missing the required `description`) — and the bridge's own module doc states the design rule the heuristic violated: "the bridge never special-cases tool names". ## Decision diff --git a/examples/acp-agent/cordis.yml b/examples/acp-agent/cordis.yml index ca85698c36..83ac07ce55 100644 --- a/examples/acp-agent/cordis.yml +++ b/examples/acp-agent/cordis.yml @@ -38,13 +38,14 @@ config: model: deepseek-v4-flash persistenceRoot: !!js process.env.DSH_SNAPSHOT_SESSIONS_ROOT ?? './.sessions' - # The persona: identity + behavior only. Tool guidance lives with each tool - # plugin (descriptions + prompt sections); {{model}} and {{cwd}} are prompt - # variables the agent loop resolves per session (every ACP session carries - # the client's cwd, so the persona can state the workspace). - systemPrompt: | - You are a coding assistant powered by the {{model}} model, driven over - the Agent Client Protocol. Your working directory is {{cwd}}. + # The persona: identity + behavior only, nothing about transports or + # tooling — tool guidance lives with each tool plugin (descriptions + + # prompt sections). {{model}} and {{cwd}} are prompt variables the agent + # loop resolves per session (every ACP session carries the client's cwd, + # so the persona can state the workspace). + persona: | + You are a coding assistant powered by the {{model}} model. Your working + directory is {{cwd}}. Verify your work by running the code or tests. Keep answers brief and factual. diff --git a/examples/coding-agent/cordis.yml b/examples/coding-agent/cordis.yml index 1980c10aa6..0439262e33 100644 --- a/examples/coding-agent/cordis.yml +++ b/examples/coding-agent/cordis.yml @@ -46,11 +46,12 @@ resumeSessionId: !!js process.env.RESUME_SESSION_ID persistenceRoot: './.sessions' welcome: 'agent REPL ready. Give it a coding task.' - # The persona: identity + behavior only. Tool guidance lives with each tool - # plugin (descriptions + prompt sections); {{model}} is the prompt variable - # the agent loop resolves from this agent's configured model. - systemPrompt: | - You are coding-agent, a CLI coding assistant powered by the {{model}} model. + # The persona: identity + behavior only, nothing about transports or + # tooling — tool guidance lives with each tool plugin (descriptions + + # prompt sections). {{model}} is the prompt variable the agent loop + # resolves from this agent's configured model. + persona: | + You are coding-agent, a coding assistant powered by the {{model}} model. Verify your work by running the code or tests. Keep answers brief and factual. diff --git a/examples/coding-agent/tests/coding-task.e2e.ts b/examples/coding-agent/tests/coding-task.e2e.ts index 68bca5cdfa..ce716bdb2c 100644 --- a/examples/coding-agent/tests/coding-task.e2e.ts +++ b/examples/coding-agent/tests/coding-task.e2e.ts @@ -53,11 +53,8 @@ describe.skipIf(!process.env.DEEPSEEK_API_KEY)('coding task: fix a failing test const before = spawnSync('node', ['add.test.js'], { cwd: workdir }) expect(before.status).not.toBe(0) - ctx = await codingHarness(workdir) - const agent = ctx.agentLoop.create(AgentId('e2e-task'), { - model: 'deepseek-v4-flash', - systemPrompt: SYSTEM_PROMPT, - }) + ctx = await codingHarness(workdir, { persona: SYSTEM_PROMPT }) + const agent = ctx.agentLoop.create(AgentId('e2e-task'), { model: 'deepseek-v4-flash' }) agent.send([{ type: 'text', diff --git a/examples/coding-agent/tests/compaction.e2e.ts b/examples/coding-agent/tests/compaction.e2e.ts index 2b8f278be3..cb7ce43811 100644 --- a/examples/coding-agent/tests/compaction.e2e.ts +++ b/examples/coding-agent/tests/compaction.e2e.ts @@ -53,6 +53,7 @@ describe.skipIf(!process.env.DEEPSEEK_API_KEY)('compaction: a long session compa // budget even though those blocks are stripped before the checkpoint is // stored. ctx = await codingHarness(workdir, { + persona: SYSTEM_PROMPT, compact: { contextWindow: 2400, thresholdRatio: 0.5, @@ -63,10 +64,7 @@ describe.skipIf(!process.env.DEEPSEEK_API_KEY)('compaction: a long session compa }, persistenceRoot: './.sessions', }) - const agent = ctx.agentLoop.create(AgentId('e2e-compaction'), { - model: 'deepseek-v4-flash', - systemPrompt: SYSTEM_PROMPT, - }) + const agent = ctx.agentLoop.create(AgentId('e2e-compaction'), { model: 'deepseek-v4-flash' }) agent.send([{ type: 'text', diff --git a/examples/coding-agent/tests/full-loop.e2e.ts b/examples/coding-agent/tests/full-loop.e2e.ts index 2b70d6f339..095d2a42a1 100644 --- a/examples/coding-agent/tests/full-loop.e2e.ts +++ b/examples/coding-agent/tests/full-loop.e2e.ts @@ -20,11 +20,8 @@ afterEach(async () => { describe.skipIf(!process.env.DEEPSEEK_API_KEY)('full loop: real model + real bash tool', () => { it('runs a bash command on request and reports its output', async () => { - ctx = await codingHarness(process.cwd()) - const agent = ctx.agentLoop.create(AgentId('e2e-loop'), { - model: 'deepseek-v4-flash', - systemPrompt: SYSTEM_PROMPT, - }) + ctx = await codingHarness(process.cwd(), { persona: SYSTEM_PROMPT }) + const agent = ctx.agentLoop.create(AgentId('e2e-loop'), { model: 'deepseek-v4-flash' }) agent.send([{ type: 'text', text: 'Run `echo e2e-ok` with the bash tool and tell me its exact output.' }]) await waitForIdle(ctx, agent) diff --git a/examples/coding-agent/tests/harness.ts b/examples/coding-agent/tests/harness.ts index 7ce24913cf..dd0bc42a1b 100644 --- a/examples/coding-agent/tests/harness.ts +++ b/examples/coding-agent/tests/harness.ts @@ -33,6 +33,11 @@ export const TODO_SYSTEM_PROMPT = 'You are a coding agent. For multi-step work, /** Options for {@link codingHarness}. */ export interface CodingHarnessOptions { + /** + * Deployment persona for the tree (the system-prompt plugin's `persona` + * config — per-context, not per-agent). Omitted ⇒ no persona section. + */ + persona?: string /** Durable JSONL persistence root (the resume suite needs it; others stay file-free). */ persistenceRoot?: string /** @@ -47,7 +52,7 @@ export async function codingHarness(workdir: string, options: CodingHarnessOptio const ctx = new Context() await ctx.plugin(LlmService) await ctx.plugin(SessionStore) - await ctx.plugin(SystemPrompt) + await ctx.plugin(SystemPrompt, { persona: options.persona ?? '' }) await ctx.plugin(ToolRegistry) await ctx.plugin(AgentRegistry) await ctx.plugin(AgentLoop, { agents: [] }) diff --git a/examples/coding-agent/tests/resume.e2e.ts b/examples/coding-agent/tests/resume.e2e.ts index 4be11ed3ea..fc216a9848 100644 --- a/examples/coding-agent/tests/resume.e2e.ts +++ b/examples/coding-agent/tests/resume.e2e.ts @@ -38,11 +38,11 @@ describe.skipIf(!process.env.DEEPSEEK_API_KEY)('resume: continue a persisted ses // Run 1: a fresh agent on a KNOWN session id learns a secret, then we // dispose the whole context (simulating process exit) so only the JSONL // log on disk survives. - ctx = await codingHarness(process.cwd(), { persistenceRoot: root }) + ctx = await codingHarness(process.cwd(), { persona: SYSTEM_PROMPT, persistenceRoot: root }) const first = ctx.agents.create({ agentId: AgentId('resume-1'), sessionId: SESSION_ID, - agentOptions: { model: 'deepseek-v4-flash', systemPrompt: SYSTEM_PROMPT }, + agentOptions: { model: 'deepseek-v4-flash' }, }).agent as ReactLoopAgent first.send([{ type: 'text', text: `Remember this code for later: ${SECRET}. Just acknowledge it.` }]) await waitForIdle(ctx, first) @@ -52,11 +52,11 @@ describe.skipIf(!process.env.DEEPSEEK_API_KEY)('resume: continue a persisted ses // Run 2: a brand-new context over the SAME root resumes the persisted // session. The loaded event log seeds the live session, so the model sees // run 1's exchange as conversation history. - ctx = await codingHarness(process.cwd(), { persistenceRoot: root }) + ctx = await codingHarness(process.cwd(), { persona: SYSTEM_PROMPT, persistenceRoot: root }) const resumed = (await ctx.agents.resume({ agentId: AgentId('resume-2'), resumeSessionId: SESSION_ID, - agentOptions: { model: 'deepseek-v4-flash', systemPrompt: SYSTEM_PROMPT }, + agentOptions: { model: 'deepseek-v4-flash' }, })).agent as ReactLoopAgent expect(resumed.session.id).toBe(SESSION_ID) // The prior user turn is in the rehydrated log before the model is asked. diff --git a/examples/coding-agent/tests/todo-write.e2e.ts b/examples/coding-agent/tests/todo-write.e2e.ts index 33cac531cf..b100091a0f 100644 --- a/examples/coding-agent/tests/todo-write.e2e.ts +++ b/examples/coding-agent/tests/todo-write.e2e.ts @@ -18,11 +18,8 @@ afterEach(async () => { describe.skipIf(!process.env.DEEPSEEK_API_KEY)('todo_write: real model records a plan', () => { it('appends a todo/write event with the model-produced task list', async () => { - ctx = await codingHarness(process.cwd()) - const agent = ctx.agentLoop.create(AgentId('e2e-todo'), { - model: 'deepseek-v4-flash', - systemPrompt: TODO_SYSTEM_PROMPT, - }) + ctx = await codingHarness(process.cwd(), { persona: TODO_SYSTEM_PROMPT }) + const agent = ctx.agentLoop.create(AgentId('e2e-todo'), { model: 'deepseek-v4-flash' }) agent.send([{ type: 'text', text: 'Use the todo_write tool to record a plan of exactly two steps: first ' diff --git a/examples/echo-agent/cordis.yml b/examples/echo-agent/cordis.yml index 9eef3d1a1b..b66c5e8163 100644 --- a/examples/echo-agent/cordis.yml +++ b/examples/echo-agent/cordis.yml @@ -33,6 +33,6 @@ name: '@deepseek-ai/dsh-stdio-agent' config: model: mock-echo - systemPrompt: 'You are echo-agent, a demo agent.' + persona: 'You are echo-agent, a demo agent.' welcome: 'echo-agent ready. Type a message ("echo " triggers the tool).' persistenceRoot: './.sessions' diff --git a/packages/bash/tool-bash/tests/tools.spec.ts b/packages/bash/tool-bash/tests/tools.spec.ts index f6a985f594..7d4b34f74f 100644 --- a/packages/bash/tool-bash/tests/tools.spec.ts +++ b/packages/bash/tool-bash/tests/tools.spec.ts @@ -276,10 +276,11 @@ describe('bash tool', () => { await ctx.plugin(LocalBashExecutor, {}) const fiber = await ctx.plugin(ToolBash) expect(ctx.tools.schemas()).toHaveLength(3) - expect((await ctx.systemPrompt.assemble()).sections.map(s => s.name)).toEqual(['tool:bash']) + expect((await ctx.systemPrompt.assemble()).sections.map(s => s.name)).toEqual(['harness:identity', 'deployment:persona', 'tool:bash']) await fiber.dispose() expect(ctx.tools.schemas()).toHaveLength(0) - expect((await ctx.systemPrompt.assemble()).sections).toHaveLength(0) + // Only the system-prompt plugin's own built-in sections remain. + expect((await ctx.systemPrompt.assemble()).sections.map(s => s.name)).toEqual(['harness:identity', 'deployment:persona']) }) it('tools depend on the executor: no registration without ctx.bash', async () => { diff --git a/packages/core/agent-core/README.md b/packages/core/agent-core/README.md index 022ccba4f4..353e68332d 100644 --- a/packages/core/agent-core/README.md +++ b/packages/core/agent-core/README.md @@ -18,6 +18,7 @@ This is the package to read to see **the whole plugin tree at once** — the tea @deepseek-ai/dsh-invariants dev-mode event-contract assertions @deepseek-ai/dsh-tool-bash the model-facing bash/bash_output/bash_kill schemas @deepseek-ai/dsh-agent-loop THE concrete loop (gets the forwarded `agents`) + (dsh-system-prompt gets the forwarded `persona`) ``` ## What it deliberately leaves OUTSIDE the bundle @@ -34,10 +35,11 @@ This is the [interface/implementation/consumer seam](../../../docs/rfc/implement ```ts import type { Config } from '@deepseek-ai/dsh-agent-core' -// Config === AgentLoop.Config — the `agents` list, default []. +// { agents?, persona? } — the schema is z.intersect([AgentLoop.Config, SystemPrompt.Config]), +// so validation and defaulting can never drift from the owners'. ``` -The bundle FORWARDS `agent-loop`'s `agents` list as its own (default `[]`), so each app supplies its own pre-created agents — a stdio app pre-creates a `main`; the ACP app pre-creates none (it creates agents on demand at `session/new`). Forwarding the list is exactly why the loop can live in the shared spine even though the apps disagree on which agents to pre-create. +The bundle FORWARDS each field to the child that owns it: `agents` to `agent-loop` (default `[]`), so each app supplies its own pre-created agents — a stdio app pre-creates a `main`; the ACP app pre-creates none (it creates agents on demand at `session/new`) — and `persona` to `dsh-system-prompt` (default `''`), the deployment's persona section. Forwarding is exactly why the owners can live in the shared spine even though the apps disagree on what to configure. ## Why a code bundle, not a shared YAML include diff --git a/packages/core/agent-core/package.json b/packages/core/agent-core/package.json index a70ee30e71..5b1eed413a 100644 --- a/packages/core/agent-core/package.json +++ b/packages/core/agent-core/package.json @@ -44,5 +44,8 @@ "@deepseek-ai/dsh-tool-bash": "workspace:^", "@deepseek-ai/dsh-tools": "workspace:^", "cordis": "^4.0.0-rc.6" + }, + "dependencies": { + "schemastery": "^3.18.0" } } diff --git a/packages/core/agent-core/src/index.ts b/packages/core/agent-core/src/index.ts index ad3f5d8c46..0831ae929e 100644 --- a/packages/core/agent-core/src/index.ts +++ b/packages/core/agent-core/src/index.ts @@ -44,9 +44,10 @@ import type { Context } from 'cordis' import Timer from '@cordisjs/plugin-timer' +import z from 'schemastery' import LlmService from '@deepseek-ai/dsh-llm' import SessionStore from '@deepseek-ai/dsh-session' -import SystemPrompt from '@deepseek-ai/dsh-system-prompt' +import SystemPrompt, { type Config as SystemPromptConfig } from '@deepseek-ai/dsh-system-prompt' import ToolRegistry from '@deepseek-ai/dsh-tools' import AgentRegistry from '@deepseek-ai/dsh-agent' import * as invariants from '@deepseek-ai/dsh-invariants' @@ -56,33 +57,45 @@ import AgentLoop, { type Config as AgentLoopConfig } from '@deepseek-ai/dsh-agen export const name = 'agent-core' /** - * Bundle config: the agent-loop `agents` list, forwarded verbatim. Default `[]` - * — an app that pre-creates no agents (the ACP bridge creates them on demand at - * `session/new`) simply omits it; an app that needs a pre-created `main` (the - * stdio chat) supplies one. This IS {@link AgentLoopConfig}, so the schema and - * the forwarded shape can never drift. + * Bundle config: each field forwarded verbatim to the child that owns it — + * `agents` to the agent loop (an app that pre-creates no agents, like the ACP + * bridge, simply omits it), `persona` to the system-prompt plugin (the + * deployment's persona section). Both are optional INPUT here because each + * owner's schema supplies the default (`[]` / `''`); the schema is the + * INTERSECTION of the owners' own schemas, so validation and defaulting can + * never drift from them. */ -export type Config = AgentLoopConfig +export interface Config { + /** The agent-loop `agents` list (see dsh-agent-loop's `Config`). */ + agents?: AgentLoopConfig['agents'] + /** The deployment persona (see dsh-system-prompt's `Config`). */ + persona?: SystemPromptConfig['persona'] +} -/** Forward the loop's own schema so validation + defaulting stay identical. */ -export const Config = AgentLoop.Config +/** Intersect the owners' schemas so validation + defaulting stay identical. */ +export const Config = z.intersect([AgentLoop.Config, SystemPrompt.Config]) as unknown as z /** * Load the spine. Each `ctx.plugin(...)` mounts one child of the bundle fiber; - * `agent-loop` receives the forwarded `agents` list. Load order is irrelevant - * (cordis pends each fiber on its `inject` until the services it needs exist), - * but the listing mirrors the dependency layering for readability: the LLM - * vocabulary and core registries first, then the dev tripwire and the bash tool - * consumer, then the loop that drives them. + * `agent-loop` receives the forwarded `agents` list and `system-prompt` the + * forwarded `persona`. Load order is irrelevant (cordis pends each fiber on + * its `inject` until the services it needs exist), but the listing mirrors the + * dependency layering for readability: the LLM vocabulary and core registries + * first, then the dev tripwire and the bash tool consumer, then the loop that + * drives them. */ export function apply(ctx: Context, config: Config): void { ctx.plugin(Timer) ctx.plugin(LlmService) ctx.plugin(SessionStore) - ctx.plugin(SystemPrompt) + // The forwarded fields are validated + defaulted by this bundle's intersected + // schema before apply runs, so the ?? fallbacks only narrow the + // optional-input TYPES — they mirror the owners' schema defaults, never + // introduce different ones. + ctx.plugin(SystemPrompt, { persona: config.persona ?? '' }) ctx.plugin(ToolRegistry) ctx.plugin(AgentRegistry) ctx.plugin(invariants) ctx.plugin(toolBash) - ctx.plugin(AgentLoop, { agents: config.agents }) + ctx.plugin(AgentLoop, { agents: config.agents ?? [] }) } diff --git a/packages/core/agent-core/tests/agent-core.spec.ts b/packages/core/agent-core/tests/agent-core.spec.ts index 67f5d88532..4a4c5587ed 100644 --- a/packages/core/agent-core/tests/agent-core.spec.ts +++ b/packages/core/agent-core/tests/agent-core.spec.ts @@ -43,11 +43,27 @@ describe('dsh-agent-core bundle', () => { await ctx.fiber.dispose() }) - it('forwards a pre-created agent to the loop', async () => { + it('forwards a pre-created agent to the loop and the persona to system-prompt', async () => { const ctx = await mount({ - agents: [{ id: AgentId('main'), model: 'mock', systemPrompt: 'hi' }], + agents: [{ id: AgentId('main'), model: 'mock' }], + persona: 'You are main.', }) expect(ctx.get('agents')?.get(AgentId('main'))).toBeDefined() + const assembly = await ctx.get('systemPrompt')!.assemble() + expect(assembly.sections.find(s => s.name === 'deployment:persona')?.text).toBe('You are main.') + await ctx.fiber.dispose() + }) + + it('tolerates a schema-bypassing direct apply (the ?? fallbacks fire)', async () => { + // ctx.plugin validates + defaults the bundle config first; a direct apply + // skips the schema, so the forwarding `?? []` / `?? ''` are what fire. + const ctx = new Context() + agentCore.apply(ctx, {}) + await new Promise(resolve => setTimeout(resolve, 50)) + expect(ctx.get('agentLoop')).toBeDefined() + expect(ctx.get('agents')?.list()).toHaveLength(0) + const assembly = await ctx.get('systemPrompt')!.assemble() + expect(assembly.sections.find(s => s.name === 'deployment:persona')?.text).toBe('') await ctx.fiber.dispose() }) diff --git a/packages/core/agent-core/tsconfig.json b/packages/core/agent-core/tsconfig.json index 83bf06c586..91e5ec894e 100644 --- a/packages/core/agent-core/tsconfig.json +++ b/packages/core/agent-core/tsconfig.json @@ -11,6 +11,9 @@ { "path": "../../../vendor/cordis" }, + { + "path": "../../../vendor/schemastery" + }, { "path": "../../../vendor/timer" }, diff --git a/packages/core/agent-loop/README.md b/packages/core/agent-loop/README.md index 25480403bd..e169fce45a 100644 --- a/packages/core/agent-loop/README.md +++ b/packages/core/agent-loop/README.md @@ -28,12 +28,11 @@ interface Config { agents: Array<{ id: string // required model?: string - systemPrompt?: string // the agent's persona TEMPLATE (may reference {{model}}/{{cwd}}) }> } ``` -Agents listed in config are auto-created at startup. The plugin also registers the harness-owned prompt pieces on `ctx.systemPrompt`: the `harness:identity` section (order −100 — every agent's prompt opens by stating it is powered by the DeepSeek Harness SDK), the `agent:persona` section (order 0 — `AgentOptions.systemPrompt` renders after it, before all tool guidance), and the built-in `model`/`cwd` prompt variables, resolved per step from the `assemble({ agent })` context. +Agents listed in config are auto-created at startup. (There is no per-agent persona: the deployment persona is `dsh-system-prompt`'s own `persona` config, shared by every agent in the context.) The plugin registers the built-in `model`/`cwd` prompt variables on `ctx.systemPrompt`, resolved per step from the `assemble({ agent })` context — runtime facts of the agents THIS loop drives, unlike the `harness:identity`/`deployment:persona` sections, which live on `dsh-system-prompt` so they survive a swapped loop plugin. ### Classes diff --git a/packages/core/agent-loop/src/index.ts b/packages/core/agent-loop/src/index.ts index ed7243caf8..9eaa61624a 100644 --- a/packages/core/agent-loop/src/index.ts +++ b/packages/core/agent-loop/src/index.ts @@ -72,7 +72,6 @@ export class AgentLoop extends Service implements AgentFactory { agents: z.array(z.object({ id: z.string().required(), model: z.string(), - systemPrompt: z.string(), resumeSessionId: z.string(), })).default([]), }) as unknown as z @@ -82,25 +81,14 @@ export class AgentLoop extends Service implements AgentFactory { // Provide the agent-creation factory to the registry (effect-scoped: the // slot is cleared on dispose). ctx.effect(() => this.ctx.agents.setFactory(this), 'agentLoop.setFactory()') - // The prompt pieces the harness itself owns, registered once. The - // harness-identity section states what every agent on this loop IS, - // ahead of everything (order −100 — before the deployment's persona); - // the persona is the order-0 section resolved per assembly from the - // AssembleContext the loop passes (loop.ts assembles with `{ agent }` - // each step); `{{model}}`/`{{cwd}}` are the built-in prompt variables - // projecting the agent's configured model and its session workspace. A - // provider returns undefined when the fact is absent (renderPrompt then - // rejects a persona that claims it — fail loud). - ctx.systemPrompt.section({ - name: 'harness:identity', - order: -100, - text: 'You are an AI agent powered by the DeepSeek Harness SDK.', - }) - ctx.systemPrompt.section({ - name: 'agent:persona', - order: 0, - text: context => context.agent?.options.systemPrompt ?? '', - }) + // The prompt variables the shipped loop provides, registered once. The + // sections themselves (`harness:identity`, `deployment:persona`) belong to + // dsh-system-prompt — they must survive a swapped loop plugin — but + // `{{model}}`/`{{cwd}}` are runtime facts of the agents THIS loop drives: + // it assembles with `{ agent }` each step (loop.ts), and the variables + // project the agent's configured model and its session workspace from that + // context. A provider returns undefined when the fact is absent + // (renderPrompt then rejects a persona that claims it — fail loud). ctx.systemPrompt.variable('model', context => context.agent?.options.model) ctx.systemPrompt.variable('cwd', context => context.agent?.session.header.cwd) for (const { id, resumeSessionId, ...options } of config.agents) { diff --git a/packages/core/agent-loop/tests/config-session-id.spec.ts b/packages/core/agent-loop/tests/config-session-id.spec.ts index 8cf5bd81f8..a645fb3553 100644 --- a/packages/core/agent-loop/tests/config-session-id.spec.ts +++ b/packages/core/agent-loop/tests/config-session-id.spec.ts @@ -35,7 +35,7 @@ describe('config-driven session id', () => { await ctx1.plugin(SystemPrompt) await ctx1.plugin(ToolRegistry) await ctx1.plugin(AgentRegistry) - await ctx1.plugin(AgentLoop, { agents: [{ id: AgentId('cfg'), model: 'mock', systemPrompt: '' }] }) + await ctx1.plugin(AgentLoop, { agents: [{ id: AgentId('cfg'), model: 'mock' }] }) await ctx1.plugin(SessionPersistenceJsonl, { root }) ctx1.llm.registerAdapter(['mock'], new MockAdapter([textResponse('cfg')])) const a1 = ctx1.agents.get(AgentId('cfg')) as ReactLoopAgent @@ -52,7 +52,7 @@ describe('config-driven session id', () => { await ctx2.plugin(SystemPrompt) await ctx2.plugin(ToolRegistry) await ctx2.plugin(AgentRegistry) - await ctx2.plugin(AgentLoop, { agents: [{ id: AgentId('cfg'), model: 'mock', systemPrompt: '' }] }) + await ctx2.plugin(AgentLoop, { agents: [{ id: AgentId('cfg'), model: 'mock' }] }) await ctx2.plugin(SessionPersistenceJsonl, { root }) ctx2.llm.registerAdapter(['mock'], new MockAdapter([textResponse('cfg2')])) const a2 = ctx2.agents.get(AgentId('cfg')) as ReactLoopAgent @@ -92,7 +92,7 @@ describe('config-driven session id', () => { await ctx2.plugin(SystemPrompt) await ctx2.plugin(ToolRegistry) await ctx2.plugin(AgentRegistry) - await ctx2.plugin(AgentLoop, { agents: [{ id: AgentId('main'), model: 'mock', systemPrompt: '', resumeSessionId: SessionId('sticky-1') }] }) + await ctx2.plugin(AgentLoop, { agents: [{ id: AgentId('main'), model: 'mock', resumeSessionId: SessionId('sticky-1') }] }) await ctx2.plugin(SessionPersistenceJsonl, { root }) ctx2.llm.registerAdapter(['mock'], new MockAdapter([textResponse('second')])) @@ -120,7 +120,7 @@ describe('config-driven session id', () => { await ctx.plugin(SystemPrompt) await ctx.plugin(ToolRegistry) await ctx.plugin(AgentRegistry) - await ctx.plugin(AgentLoop, { agents: [{ id: AgentId('main'), model: 'mock', systemPrompt: '', resumeSessionId: SessionId('does-not-exist') }] }) + await ctx.plugin(AgentLoop, { agents: [{ id: AgentId('main'), model: 'mock', resumeSessionId: SessionId('does-not-exist') }] }) const warn = vi.spyOn((ctx.agentLoop as unknown as { ctx: { logger: { warn: (...a: unknown[]) => void } } }).ctx.logger, 'warn') .mockImplementation(() => undefined) await ctx.plugin(SessionPersistenceJsonl, { root }) diff --git a/packages/core/agent-loop/tests/loop.spec.ts b/packages/core/agent-loop/tests/loop.spec.ts index 1a0525c0b2..7de441a928 100644 --- a/packages/core/agent-loop/tests/loop.spec.ts +++ b/packages/core/agent-loop/tests/loop.spec.ts @@ -8,11 +8,11 @@ import AgentRegistry, { AgentId } from '@deepseek-ai/dsh-agent' import AgentLoop, { ReactLoopAgent } from '@deepseek-ai/dsh-agent-loop' import { MockAdapter, maxTokensResponse, textResponse, toolCallResponse } from './mock-adapter.ts' -async function harness(adapter: MockAdapter) { +async function harness(adapter: MockAdapter, persona = '') { const ctx = new Context() await ctx.plugin(LlmService) await ctx.plugin(SessionStore) - await ctx.plugin(SystemPrompt) + await ctx.plugin(SystemPrompt, { persona }) await ctx.plugin(ToolRegistry) await ctx.plugin(AgentRegistry) await ctx.plugin(AgentLoop, { agents: [] }) @@ -143,7 +143,9 @@ describe('agent loop', () => { it('renders harness identity, then the persona, then tool guidance — with {{variables}} resolved', async () => { const adapter = new MockAdapter([textResponse('ok')]) - const ctx = await harness(adapter) + // The persona is a TEMPLATE: {{model}} is the loop-registered variable + // projecting this agent's configured model, so the model knows its own name. + const ctx = await harness(adapter, 'You are a test agent on {{model}}.') ctx.systemPrompt.section({ name: 'tool:noop', order: 100, text: 'Use the noop tool wisely.' }) ctx.tools.register(defineTool({ name: 'noop', @@ -153,9 +155,7 @@ describe('agent loop', () => { return [] }, })) - // The persona is a TEMPLATE: {{model}} is the loop-registered variable - // projecting this agent's configured model, so the model knows its own name. - const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock', systemPrompt: 'You are a test agent on {{model}}.' }) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) send(agent, 'hi') await waitForIdle(ctx, agent) @@ -167,12 +167,12 @@ describe('agent loop', () => { it('resolves {{cwd}} from the agent session workspace (factory create with meta.cwd)', async () => { const adapter = new MockAdapter([textResponse('ok')]) - const ctx = await harness(adapter) + const ctx = await harness(adapter, 'Working in {{cwd}}.') const handle = ctx.agents.create({ agentId: AgentId('a-cwd'), sessionId: SessionId('s-cwd'), meta: { cwd: '/work/space' }, - agentOptions: { model: 'mock', systemPrompt: 'Working in {{cwd}}.' }, + agentOptions: { model: 'mock' }, }) const agent = handle.agent as ReactLoopAgent @@ -189,10 +189,10 @@ describe('agent loop', () => { // report idle status): a rescue listener supplies the variable and the // follow-up prompt reaches the model. const adapter = new MockAdapter([textResponse('ok after rescue')]) - const ctx = await harness(adapter) + const ctx = await harness(adapter, 'In {{cwd}}.') const errors: Error[] = [] ctx.on('agent/error', (_agent, _turn, _step, error) => void errors.push(error)) - const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock', systemPrompt: 'In {{cwd}}.' }) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) send(agent, 'hi') await waitForIdle(ctx, agent) @@ -218,6 +218,22 @@ describe('agent loop', () => { expect(turnEnds[1]?.type === 'turn/end' && turnEnds[1].data.reason.kind).toBe('completed') }) + it('omits the system field when a system-prompt/assemble veto empties the assembly', async () => { + // The documented escape valve: a deployment that must drop the harness + // openers short-circuits the assemble waterfall; the request then carries + // NO system field at all (not an empty string). + const adapter = new MockAdapter([textResponse('ok')]) + const ctx = await harness(adapter) + ctx.on('system-prompt/assemble', async () => ({ sections: [], tools: [], variables: {} })) + const agent = ctx.agentLoop.create(AgentId('a-no-system'), { model: 'mock' }) + + send(agent, 'hi') + await waitForIdle(ctx, agent) + + expect(adapter.requests).toHaveLength(1) + expect('system' in adapter.requests[0]!).toBe(false) + }) + it('records raw chunks for replay as assistant/chunk session events', async () => { const adapter = new MockAdapter([textResponse('abc')]) const ctx = await harness(adapter) @@ -853,7 +869,7 @@ describe('agent loop', () => { await ctx.plugin(ToolRegistry) await ctx.plugin(AgentRegistry) await ctx.plugin(AgentLoop, { - agents: [{ id: AgentId('config-agent'), model: 'mock', systemPrompt: 'Config prompt' }], + agents: [{ id: AgentId('config-agent'), model: 'mock' }], }) ctx.llm.registerAdapter(['mock'], adapter) diff --git a/packages/core/agent/src/index.ts b/packages/core/agent/src/index.ts index ec9ba796b9..096555f925 100644 --- a/packages/core/agent/src/index.ts +++ b/packages/core/agent/src/index.ts @@ -49,7 +49,7 @@ export interface CreateAgentOptions { * for a fresh (spawn) child. */ seed?: SessionEvent[] - /** Per-agent options (model, system prompt). */ + /** Per-agent options (model, …). */ agentOptions?: AgentOptions } @@ -62,7 +62,7 @@ export interface ResumeAgentOptions { agentId: AgentId /** The persisted session id to load and resume on. */ resumeSessionId: SessionId - /** Per-agent options (model, system prompt). */ + /** Per-agent options (model, …). */ agentOptions?: AgentOptions } diff --git a/packages/core/agent/src/types.ts b/packages/core/agent/src/types.ts index 3823b0db61..1b4af61d9a 100644 --- a/packages/core/agent/src/types.ts +++ b/packages/core/agent/src/types.ts @@ -60,9 +60,8 @@ declare module '@deepseek-ai/dsh-system-prompt' { interface AssembleContext { /** * The agent this assembly is for. The agent loop passes it on every - * per-step `assemble({ agent })`; section text and variable providers - * project per-agent facts from it (`options.systemPrompt` → the persona - * section, `options.model` → `{{model}}`, `session.header.cwd` → + * per-step `assemble({ agent })`; variable providers project per-agent + * facts from it (`options.model` → `{{model}}`, `session.header.cwd` → * `{{cwd}}`). Optional because a bare `assemble()` (tests, diagnostics) * has no agent — providers must tolerate its absence. */ @@ -71,23 +70,14 @@ declare module '@deepseek-ai/dsh-system-prompt' { } /** - * Options an agent is created with. + * Options an agent is created with. The persona is NOT here — it is the + * deployment's `persona` config on the dsh-system-prompt plugin, shared by + * every agent in the context. * Merge-extensible: plugins declare extra fields via declaration merging. */ export interface AgentOptions { /** Model name (must have a registered adapter at call time). */ model?: string - /** - * The agent's persona: a deployment-authored prompt-TEMPLATE fragment, - * rendered as the order-0 section of the assembled system prompt (before - * all tool guidance). It may reference registered `{{variables}}` (e.g. - * `{{model}}`, `{{cwd}}`); it is one section of the full prompt, never the - * whole. Template, not free-form text: every complete `{{…}}` group IS - * interpreted, strictly — an unknown or malformed reference fails the turn - * loudly — and there is no escape syntax for literal `{{…}}` prose yet (a - * deliberate deferral; see the prompt-variables RFC). - */ - systemPrompt?: string } export interface SendOptions { diff --git a/packages/core/system-prompt/README.md b/packages/core/system-prompt/README.md index 5626d06a1c..be705de03e 100644 --- a/packages/core/system-prompt/README.md +++ b/packages/core/system-prompt/README.md @@ -1,6 +1,12 @@ # dsh-system-prompt -System prompt assembly registry. Plugins contribute ordered text sections, tool-schema providers, and named prompt variables; the agent loop calls `assemble(context)` once per step, and `renderPrompt(assembly)` is the full system prompt the model sees. +System prompt assembly registry. Plugins contribute ordered text sections, tool-schema providers, and named prompt variables; the agent loop calls `assemble(context)` once per step, and `renderPrompt(assembly)` is the full system prompt the model sees. The plugin registers the harness-owned openers itself — the static `harness:identity` section and the deployment's `deployment:persona` section — so they exist for every agent regardless of which loop plugin drives it. + +## Config + +| Key | Default | Meaning | +|---|---|---| +| `persona` | `''` | The deployment persona: the ONE deployment-authored prompt fragment, rendered as the order-0 `deployment:persona` section and shared by every agent in the context (subagents included). A template — complete `{{…}}` groups are interpreted strictly against the registered variables (the shipped loop registers `{{model}}`/`{{cwd}}`), with no escape syntax for literal braces yet. Empty ⇒ the section is dropped at render. | ## Service: `SystemPrompt` (ctx key: `systemPrompt`) @@ -21,7 +27,7 @@ System prompt assembly registry. Plugins contribute ordered text sections, tool- ### Key types - `AssembleContext` — what one `assemble()` call is FOR. Declared empty here and merge-extensible; `dsh-agent` declares `agent?: Agent`, so providers project per-agent facts. Providers must tolerate absent fields (a bare `assemble()` carries an empty context). -- `PromptSection` — `{ name, order, text: string | ((context) => string) }`. Sections are concatenated in ascending `order`. Order bands: `-100` is the harness identity, `0` the per-agent persona (both registered by the agent loop), tool guidance uses `100–199`; other negative orders also render before the persona. +- `PromptSection` — `{ name, order, text: string | ((context) => string) }`. Sections are concatenated in ascending `order`. Order bands: `-100` is the harness identity, `0` the deployment persona (both registered by this plugin), tool guidance uses `100–199`; other negative orders also render before the persona. - `PromptAssembly` — `{ sections: AssembledSection[], tools: ToolSchema[], variables: Record }`. Section texts arrive resolved but not yet interpolated; `variables` holds every registered variable resolved against the context. Tool schemas are part of the assembly by design: "what the model is told it can do" is one coherent thing, even though adapters transmit schemas as a separate wire field. - `renderPrompt(assembly)` — interpolates `{{variable}}` references in each section, drops empty sections, joins with blank lines. STRICT: an unknown reference (`Object.hasOwn` lookup — prototype names like `{{constructor}}` are unknown), a registered-but-valueless reference, a malformed complete `{{…}}` group, or a `{{` that opens no complete group while a `}}` still follows (`{{{model}}}`) throws — fail loud beats shipping a malformed prompt. A lone `{{` with no `}}` anywhere after it passes through verbatim; substituted values are never re-scanned. @@ -29,14 +35,14 @@ Merge-extensible: plugins can declare extra fields on `PromptAssembly` and `Asse ### Extension points -- Section providers: tool packages own their cross-call guidance (`tool:bash`, `tool:read`, …); the agent loop owns `harness:identity` and `agent:persona`. +- Section providers: tool packages own their cross-call guidance (`tool:bash`, `tool:read`, …); this plugin owns `harness:identity` and `deployment:persona`. - Variable providers: the agent loop registers `model` and `cwd`; any plugin can register the facts it owns (a future `date`, git state, …). - Tool schema providers: `ToolRegistry` registers itself as a tool provider automatically. - The `system-prompt/assemble` waterfall: mutate or replace the assembly per caller (dynamic tool filtering, extra variables). ### What is NOT here -- Any hardcoded prompt text — every section comes from plugins, every deployment-authored word from config. +- Any deployment-authored prompt text outside config — the persona is this plugin's `persona` config, and every other section comes from the plugin that owns the fact. (The `harness:identity` line is deliberately a code literal: a harness fact, not a deployment choice; the `system-prompt/assemble` waterfall is the escape valve for a deployment that must drop it.) - Prompt compaction (belongs on the `agent/pre-step` seam in `dsh-agent`). Design rationale: [the prompt-variables RFC](../../../docs/rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md). diff --git a/packages/core/system-prompt/package.json b/packages/core/system-prompt/package.json index 672f7a03ef..d97a7b8538 100644 --- a/packages/core/system-prompt/package.json +++ b/packages/core/system-prompt/package.json @@ -25,6 +25,9 @@ "@deepseek-ai/dsh-llm": "^0.0.1", "cordis": "^4.0.0-rc.6" }, + "dependencies": { + "schemastery": "^3.18.0" + }, "devDependencies": { "@deepseek-ai/dsh-llm": "workspace:^", "cordis": "^4.0.0-rc.6" diff --git a/packages/core/system-prompt/src/index.ts b/packages/core/system-prompt/src/index.ts index 4da727f4e5..c970c66b30 100644 --- a/packages/core/system-prompt/src/index.ts +++ b/packages/core/system-prompt/src/index.ts @@ -4,10 +4,16 @@ * collates them through a waterfall that runs once per step, and * `renderPrompt` interpolates `{{variable}}` references into the final text. * + * The harness-owned prompt openers live here too: this plugin registers the + * static `harness:identity` section (order −100) and the deployment's + * `deployment:persona` section (order 0, from its `persona` config), so they + * exist for every agent regardless of which loop plugin drives it. + * * @module @deepseek-ai/dsh-system-prompt */ import { Context, Service } from 'cordis' +import z from 'schemastery' import type { ToolSchema } from '@deepseek-ai/dsh-llm' declare module 'cordis' { @@ -55,7 +61,7 @@ export interface PromptSection { name: string /** * Sections are concatenated in ascending order. Convention: `-100` is the - * harness identity, `0` the per-agent persona, tool guidance uses 100–199; + * harness identity, `0` the deployment persona, tool guidance uses 100–199; * other negative orders also render before the persona. */ order: number @@ -104,6 +110,22 @@ const VARIABLE_NAME = /^[a-z][a-z0-9_]*$/ /** A complete `{{...}}` reference group at the scan position (validated after). */ const GROUP_AT = /^\{\{([^{}]*)\}\}/ +export interface Config { + /** + * The deployment's persona — the ONE deployment-authored fragment of the + * system prompt, rendered as the order-0 `deployment:persona` section + * (after the harness identity, before all tool guidance). Every agent in + * the context shares it, subagents included. Template, not free-form text: + * every complete `{{…}}` group is interpreted strictly against the + * registered prompt variables (the shipped agent loop registers `{{model}}` + * and `{{cwd}}`), and there is no escape syntax for literal `{{…}}` prose + * yet (a deliberate deferral; see the prompt-variables RFC). Defaults to + * `''` — the empty section is dropped at render, so a persona-less + * deployment opens with the harness identity alone. + */ + persona?: string +} + /** * Renders the text part of an assembly: interpolates `{{variable}}` * references in each section from `assembly.variables`, drops empty sections, @@ -169,15 +191,39 @@ function interpolate(section: AssembledSection, variables: Record = z.object({ + persona: z.string().default(''), + }) + private sections: PromptSection[] = [] private toolProviders: (() => ToolSchema[])[] = [] private variableProviders = new Map string | undefined>() - constructor(ctx: Context) { + constructor(ctx: Context, public config: Config) { super(ctx, 'systemPrompt') + // The harness-owned openers. They live HERE (not on the loop plugin) so a + // deployment that swaps in a different loop keeps them: the identity is a + // harness fact stated ahead of everything, and the persona is the + // deployment's config, one section of the full prompt, never the whole. + // An empty persona still RESERVES the section name (one owner — a plugin + // re-registering it throws); renderPrompt drops the empty text. + this.section({ + name: 'harness:identity', + order: -100, + text: 'You are an AI agent powered by the DeepSeek Harness SDK.', + }) + this.section({ + name: 'deployment:persona', + order: 0, + // The schema already defaulted an omitted persona to ''; the ?? only + // narrows the optional-input TYPE, it never supplies a different value. + text: config.persona ?? '', + }) } /** diff --git a/packages/core/system-prompt/tests/system-prompt.spec.ts b/packages/core/system-prompt/tests/system-prompt.spec.ts index 4aef36e9d0..c0bd8be6d2 100644 --- a/packages/core/system-prompt/tests/system-prompt.spec.ts +++ b/packages/core/system-prompt/tests/system-prompt.spec.ts @@ -2,22 +2,64 @@ import { describe, expect, it } from 'vitest' import { Context } from 'cordis' import SystemPrompt, { AssembleContext, PromptAssembly, renderPrompt } from '@deepseek-ai/dsh-system-prompt' +/** + * Every assembly carries the plugin's own built-ins — `harness:identity` + * (order −100) and `deployment:persona` (order 0, from config). Tests about + * registry MECHANICS strip them with {@link contributed} to stay focused on + * their own sections; the built-ins' behavior is pinned by its own describe. + */ +const BUILT_IN = ['harness:identity', 'deployment:persona'] +const IDENTITY = 'You are an AI agent powered by the DeepSeek Harness SDK.' +function contributed(assembly: PromptAssembly): PromptAssembly['sections'] { + return assembly.sections.filter(section => !BUILT_IN.includes(section.name)) +} + describe('SystemPrompt', () => { + describe('built-in sections', () => { + it('registers the harness identity and the configured deployment persona', async () => { + const ctx = new Context() + await ctx.plugin(SystemPrompt, { persona: 'You are DeepSeek Harness SDK.' }) + + const assembly = await ctx.systemPrompt.assemble() + expect(assembly.sections.map(s => [s.name, s.order])).toEqual([ + ['harness:identity', -100], + ['deployment:persona', 0], + ]) + expect(renderPrompt(assembly)).toBe(`${IDENTITY}\n\nYou are DeepSeek Harness SDK.`) + // The names are reserved by the plugin — one owner per section. + expect(() => ctx.systemPrompt.section({ name: 'deployment:persona', order: 0, text: 'imposter' })) + .toThrow('prompt section "deployment:persona" is already registered') + }) + + it('renders no persona section for a persona-less deployment (empty default)', async () => { + const ctx = new Context() + await ctx.plugin(SystemPrompt) + expect(renderPrompt(await ctx.systemPrompt.assemble())).toBe(IDENTITY) + }) + + it('tolerates a schema-bypassing direct construction (persona omitted)', async () => { + // ctx.plugin validates + defaults the config first; a direct construction + // skips the schema, so the ctor's `?? ''` narrowing is what fires. + const ctx = new Context() + const service = new SystemPrompt(ctx, {}) + expect(renderPrompt(await service.assemble())).toBe(IDENTITY) + }) + }) + it('assembles sections in order with context-resolved text and collected tools', async () => { const ctx = new Context() - await ctx.plugin(SystemPrompt) + await ctx.plugin(SystemPrompt, { persona: 'You are DeepSeek Harness SDK.' }) - ctx.systemPrompt.section({ name: 'persona', order: 0, text: 'You are DeepSeek Harness SDK.' }) ctx.systemPrompt.section({ name: 'cwd', order: 20, text: () => 'cwd: /tmp' }) ctx.systemPrompt.section({ name: 'rules', order: 10, text: 'Be precise.' }) ctx.systemPrompt.tools(() => [{ name: 'echo', description: 'echo back', parameters: {} }]) const assembly = await ctx.systemPrompt.assemble() - expect(assembly.sections.map(s => s.name)).toEqual(['persona', 'rules', 'cwd']) - expect(assembly.sections.map(s => s.text)).toEqual(['You are DeepSeek Harness SDK.', 'Be precise.', 'cwd: /tmp']) + expect(assembly.sections.map(s => s.name)).toEqual(['harness:identity', 'deployment:persona', 'rules', 'cwd']) + expect(assembly.sections.map(s => s.text)).toEqual([IDENTITY, 'You are DeepSeek Harness SDK.', 'Be precise.', 'cwd: /tmp']) expect(assembly.tools).toEqual([{ name: 'echo', description: 'echo back', parameters: {} }]) expect(assembly.variables).toEqual({}) - expect(renderPrompt(assembly)).toBe('You are DeepSeek Harness SDK.\n\nBe precise.\n\ncwd: /tmp') + expect(renderPrompt(assembly)).toBe(`${IDENTITY}\n\nYou are DeepSeek Harness SDK.\n\nBe precise.\n\ncwd: /tmp`) }) it('resolves section text providers against the assemble context, at each assemble call', async () => { @@ -32,8 +74,8 @@ describe('SystemPrompt', () => { text: (context: AssembleContext) => `call ${++calls} for ${(context as { who?: string }).who ?? 'nobody'}`, }) - expect((await ctx.systemPrompt.assemble({ who: 'alice' } as AssembleContext)).sections[0]!.text).toBe('call 1 for alice') - expect((await ctx.systemPrompt.assemble()).sections[0]!.text).toBe('call 2 for nobody') + expect(contributed(await ctx.systemPrompt.assemble({ who: 'alice' } as AssembleContext))[0]!.text).toBe('call 1 for alice') + expect(contributed(await ctx.systemPrompt.assemble())[0]!.text).toBe('call 2 for nobody') }) it('removes contributions when the contributing fiber is disposed (HMR safety)', async () => { @@ -47,11 +89,13 @@ describe('SystemPrompt', () => { }, { inject: ['systemPrompt'] })) const before = await ctx.systemPrompt.assemble() - expect(before.sections).toHaveLength(1) + expect(contributed(before)).toHaveLength(1) expect(before.variables).toEqual({ scoped_var: 'v' }) await fiber.dispose() const assembly = await ctx.systemPrompt.assemble() - expect(assembly.sections).toHaveLength(0) + expect(contributed(assembly)).toHaveLength(0) + // The built-ins belong to the service fiber, so they survive the plugin's disposal. + expect(assembly.sections.map(s => s.name)).toEqual(BUILT_IN) expect(assembly.tools).toHaveLength(0) expect(assembly.variables).toEqual({}) }) @@ -64,7 +108,7 @@ describe('SystemPrompt', () => { .toThrow('prompt section "dup" is already registered') // The failed registration leaked nothing; the original stays intact. const assembly = await ctx.systemPrompt.assemble() - expect(assembly.sections.map(s => s.text)).toEqual(['first']) + expect(contributed(assembly).map(s => s.text)).toEqual(['first']) }) it('rolls back a section when a system-prompt/change listener throws (P1-1)', async () => { @@ -80,12 +124,12 @@ describe('SystemPrompt', () => { }) expect(() => ctx.systemPrompt.section({ name: 'p', order: 0, text: 'persona' })).toThrow('boom change listener') - expect((await ctx.systemPrompt.assemble()).sections).toHaveLength(0) // nothing leaked + expect(contributed(await ctx.systemPrompt.assemble())).toHaveLength(0) // nothing leaked // Subsequent listener-free register contributes exactly once. off() ctx.systemPrompt.section({ name: 'p', order: 0, text: 'persona' }) - expect((await ctx.systemPrompt.assemble()).sections.map(s => s.name)).toEqual(['p']) + expect(contributed(await ctx.systemPrompt.assemble()).map(s => s.name)).toEqual(['p']) }) it('rolls back a tool provider when a system-prompt/change listener throws (P1-1)', async () => { @@ -143,8 +187,8 @@ describe('SystemPrompt', () => { const passed: AssembleContext = {} const assembly = await ctx.systemPrompt.assemble(passed) - expect(seen).toEqual([['base', 'from-a']]) - expect(assembly.sections.map(s => s.name)).toEqual(['base', 'from-a']) + expect(seen).toEqual([['harness:identity', 'deployment:persona', 'base', 'from-a']]) + expect(assembly.sections.map(s => s.name)).toEqual(['harness:identity', 'deployment:persona', 'base', 'from-a']) expect(contexts[0]).toBe(passed) // the caller's context reaches listeners }) @@ -175,8 +219,8 @@ describe('SystemPrompt', () => { firstParameters.properties['leak'] = { type: 'string' } const second = await ctx.systemPrompt.assemble() - expect(second.sections.map(section => section.name)).toEqual(['base']) - expect(second.sections.map(section => section.text)).toEqual(['base']) + expect(second.sections.map(section => section.name)).toEqual(['harness:identity', 'deployment:persona', 'base']) + expect(second.sections[0]!.text).toBe(IDENTITY) expect(second.tools).toEqual([{ name: 't', description: 'tool', parameters: { type: 'object', properties: {} } }]) }) @@ -226,10 +270,10 @@ describe('SystemPrompt', () => { await ctx.plugin(SystemPrompt) const dispose = ctx.systemPrompt.section({ name: 'direct', order: 0, text: 'direct section' }) - expect((await ctx.systemPrompt.assemble()).sections).toHaveLength(1) + expect(contributed(await ctx.systemPrompt.assemble())).toHaveLength(1) dispose() - expect((await ctx.systemPrompt.assemble()).sections).toHaveLength(0) + expect(contributed(await ctx.systemPrompt.assemble())).toHaveLength(0) }) it('removes tool provider when returned disposer is called directly', async () => { @@ -274,14 +318,13 @@ describe('SystemPrompt', () => { expect((await ctx.systemPrompt.assemble()).variables).toEqual({ model: 'm1' }) }) - it('interpolates {{name}} references in section text at render', async () => { + it('interpolates {{name}} references in section text at render — the persona included', async () => { const ctx = new Context() - await ctx.plugin(SystemPrompt) - ctx.systemPrompt.section({ name: 'persona', order: 0, text: 'You run on {{model}} in {{cwd}}.' }) + await ctx.plugin(SystemPrompt, { persona: 'You run on {{model}} in {{cwd}}.' }) ctx.systemPrompt.variable('model', () => 'deepseek-v4') ctx.systemPrompt.variable('cwd', () => '/work') - expect(renderPrompt(await ctx.systemPrompt.assemble())).toBe('You run on deepseek-v4 in /work.') + expect(renderPrompt(await ctx.systemPrompt.assemble())).toBe(`${IDENTITY}\n\nYou run on deepseek-v4 in /work.`) }) it('lets a waterfall listener add or override variables before render', async () => { @@ -292,7 +335,7 @@ describe('SystemPrompt', () => { assembly.variables['extra'] = 'from-waterfall' return next() }) - expect(renderPrompt(await ctx.systemPrompt.assemble())).toBe('from-waterfall') + expect(renderPrompt(await ctx.systemPrompt.assemble())).toBe(`${IDENTITY}\n\nfrom-waterfall`) }) it('throws on a reference to an unregistered variable, listing what exists', async () => { @@ -360,7 +403,7 @@ describe('SystemPrompt', () => { await ctx.plugin(SystemPrompt) ctx.systemPrompt.section({ name: 's', order: 0, text: '{{constructor}}' }) ctx.systemPrompt.variable('constructor', () => 'own-value') - expect(renderPrompt(await ctx.systemPrompt.assemble())).toBe('own-value') + expect(renderPrompt(await ctx.systemPrompt.assemble())).toBe(`${IDENTITY}\n\nown-value`) }) it('never re-scans substituted values (a value containing {{sneaky}} stays literal)', () => { diff --git a/packages/core/system-prompt/tsconfig.json b/packages/core/system-prompt/tsconfig.json index 9f687793d7..e9de391ba1 100644 --- a/packages/core/system-prompt/tsconfig.json +++ b/packages/core/system-prompt/tsconfig.json @@ -14,6 +14,9 @@ { "path": "../../../vendor/cordis" }, + { + "path": "../../../vendor/schemastery" + }, { "path": "../../llm/llm" } diff --git a/packages/fs/tool-fs/tests/fs-tools.e2e.ts b/packages/fs/tool-fs/tests/fs-tools.e2e.ts index 5e13e229fb..da5412530d 100644 --- a/packages/fs/tool-fs/tests/fs-tools.e2e.ts +++ b/packages/fs/tool-fs/tests/fs-tools.e2e.ts @@ -32,10 +32,10 @@ const SYSTEM = 'You are a coding assistant. Use the write tool to create files, describe.skipIf(!process.env.DEEPSEEK_API_KEY)('fs tools with-key smoke', () => { it('creates, reads, then edits a file — verified on disk', async () => { workdir = await mkdtemp(join(tmpdir(), 'dsh-fs-e2e-')) - ctx = await fsHarness(workdir) + ctx = await fsHarness(workdir, SYSTEM) // agentLoop.create prepares a session with no cwd, so the provider default // (config.cwd = workdir) is the workspace. - const agent = ctx.agentLoop.create(AgentId('fs-e2e'), { model: 'deepseek-v4-flash', systemPrompt: SYSTEM }) + const agent = ctx.agentLoop.create(AgentId('fs-e2e'), { model: 'deepseek-v4-flash' }) agent.send([{ type: 'text', text: 'Create a file named note.txt containing exactly the line: status: draft. ' @@ -63,12 +63,12 @@ describe.skipIf(!process.env.DEEPSEEK_API_KEY)('fs tools with-key smoke', () => workdir = configDir const sessionDir = await mkdtemp(join(tmpdir(), 'dsh-fs-e2e-session-')) try { - ctx = await fsHarness(configDir) + ctx = await fsHarness(configDir, SYSTEM) const handle = ctx.agents.create({ agentId: AgentId('fs-e2e-cwd'), sessionId: SessionId(`fs-e2e-cwd-${Date.now()}`), meta: { cwd: sessionDir }, - agentOptions: { model: 'deepseek-v4-flash', systemPrompt: SYSTEM }, + agentOptions: { model: 'deepseek-v4-flash' }, }) handle.agent.send([{ type: 'text', text: 'Use the write tool to create a file named where.txt containing exactly the line: here. Tell me when done.' }]) diff --git a/packages/fs/tool-fs/tests/harness.ts b/packages/fs/tool-fs/tests/harness.ts index 0a492c509e..61c163c28b 100644 --- a/packages/fs/tool-fs/tests/harness.ts +++ b/packages/fs/tool-fs/tests/harness.ts @@ -18,13 +18,14 @@ import * as LlmDeepSeek from '@deepseek-ai/dsh-llm-deepseek' * * `fsCwd` is the local backend's default base; a per-session cwd (set via a * session header) overrides it, but this harness creates agents without a - * session cwd, so the provider default IS the workspace. + * session cwd, so the provider default IS the workspace. `persona` is the + * deployment persona (the system-prompt plugin's per-context config). */ -export async function fsHarness(fsCwd: string): Promise { +export async function fsHarness(fsCwd: string, persona = ''): Promise { const ctx = new Context() await ctx.plugin(LlmService) await ctx.plugin(SessionStore) - await ctx.plugin(SystemPrompt) + await ctx.plugin(SystemPrompt, { persona }) await ctx.plugin(ToolRegistry) await ctx.plugin(AgentRegistry) await ctx.plugin(AgentLoop, { agents: [] }) diff --git a/packages/fs/tool-fs/tests/tools.spec.ts b/packages/fs/tool-fs/tests/tools.spec.ts index ac99d176d4..53f912cce2 100644 --- a/packages/fs/tool-fs/tests/tools.spec.ts +++ b/packages/fs/tool-fs/tests/tools.spec.ts @@ -133,10 +133,11 @@ describe('registration', () => { // withdraw both, not just the schemas. expect(ctx.tools.schemas()).toHaveLength(3) const sectionNames = (a: { sections: { name: string }[] }) => a.sections.map(s => s.name).sort() - expect(sectionNames(await ctx.systemPrompt.assemble())).toEqual(['tool:edit', 'tool:read', 'tool:write']) + expect(sectionNames(await ctx.systemPrompt.assemble())).toEqual(['deployment:persona', 'harness:identity', 'tool:edit', 'tool:read', 'tool:write']) await fiber.dispose() expect(ctx.tools.schemas()).toHaveLength(0) - expect((await ctx.systemPrompt.assemble()).sections).toHaveLength(0) + // Only the system-prompt plugin's own built-in sections remain. + expect(sectionNames(await ctx.systemPrompt.assemble())).toEqual(['deployment:persona', 'harness:identity']) }) }) diff --git a/packages/subagent/subagent-inprocess/src/index.ts b/packages/subagent/subagent-inprocess/src/index.ts index 4b8d2d4c99..1107926aa2 100644 --- a/packages/subagent/subagent-inprocess/src/index.ts +++ b/packages/subagent/subagent-inprocess/src/index.ts @@ -107,9 +107,9 @@ export function startInProcessRun( const seedLength = options.seed?.length ?? 0 const parentHeader = request.parent.session.header // Inherit the parent's model by default (a child with no model cannot run); - // an explicit `request.agentOptions.model` overrides it. The parent's - // systemPrompt is NOT inherited — a fresh child is a clean specialist unless - // the caller supplies one. + // an explicit `request.agentOptions.model` overrides it. The persona needs + // no inheritance: the deployment persona is a context-wide prompt section, + // so parent and child render the same one. const agentOptions: AgentOptions = { ...request.parent.options.model !== undefined ? { model: request.parent.options.model } : {}, ...request.agentOptions, diff --git a/packages/subagent/subagent-spawn/tests/harness.ts b/packages/subagent/subagent-spawn/tests/harness.ts index ff551cfc3f..b3e9d4ec24 100644 --- a/packages/subagent/subagent-spawn/tests/harness.ts +++ b/packages/subagent/subagent-spawn/tests/harness.ts @@ -23,7 +23,10 @@ export async function spawnHarness(workdir: string): Promise { const ctx = new Context() await ctx.plugin(LlmService) await ctx.plugin(SessionStore) - await ctx.plugin(SystemPrompt) + // The deployment persona is context-wide (parent AND spawned children + // render it), so it stays neutral for both roles; the delegation nudge + // lives in the e2e's user prompt and the subagent tool's own description. + await ctx.plugin(SystemPrompt, { persona: 'You are a coding agent. Report only when the requested work is done.' }) await ctx.plugin(ToolRegistry) await ctx.plugin(AgentRegistry) await ctx.plugin(AgentLoop, { agents: [] }) diff --git a/packages/subagent/subagent-spawn/tests/spawn.e2e.ts b/packages/subagent/subagent-spawn/tests/spawn.e2e.ts index 8179027976..daa032199e 100644 --- a/packages/subagent/subagent-spawn/tests/spawn.e2e.ts +++ b/packages/subagent/subagent-spawn/tests/spawn.e2e.ts @@ -29,11 +29,7 @@ describe.skipIf(!process.env.DEEPSEEK_API_KEY)('spawn backend with-key smoke', ( it('a parent delegates to a child that writes a file on disk', async () => { workdir = await mkdtemp(join(tmpdir(), 'dsh-subagent-spawn-e2e-')) ctx = await spawnHarness(workdir) - const parent = ctx.agentLoop.create(AgentId('e2e-parent'), { - model: 'deepseek-v4-flash', - systemPrompt: 'You are an orchestrator. To do file work, delegate to a subagent with the `subagent` tool — ' - + 'give it a complete, standalone instruction. Report only when done.', - }) + const parent = ctx.agentLoop.create(AgentId('e2e-parent'), { model: 'deepseek-v4-flash' }) parent.send([{ type: 'text', text: 'Use the subagent tool to delegate this exact task: "Use the bash tool to write the text ' diff --git a/packages/subagent/tool-subagent/README.md b/packages/subagent/tool-subagent/README.md index fd04fba3b2..6fe26d3083 100644 --- a/packages/subagent/tool-subagent/README.md +++ b/packages/subagent/tool-subagent/README.md @@ -14,7 +14,7 @@ The tool description and the `prompt` parameter description are DERIVED from the |---|---| | `provider` (required) | The `ctx.subagents` provider name to start runs on (`spawn`, `fork`, `acp`, …). | | `toolName` | The model-facing tool name to register (default `subagent`). Set a distinct value per load when exposing multiple providers, e.g. `subagent` + `subagent_acp`. | -| `agentOptions` | Default per-child `{ model?, systemPrompt? }` applied to every spawned child. | +| `agentOptions` | Default per-child `{ model? }` applied to every spawned child. (No per-child persona: the deployment persona is a context-wide section every agent shares.) | ## Lifecycle (synchronous collect) diff --git a/packages/subagent/tool-subagent/src/index.ts b/packages/subagent/tool-subagent/src/index.ts index 383bef0f0d..f48ef4345e 100644 --- a/packages/subagent/tool-subagent/src/index.ts +++ b/packages/subagent/tool-subagent/src/index.ts @@ -53,8 +53,10 @@ export interface Config { */ toolName?: string /** - * Default per-child agent options (model, system prompt) applied to every - * spawned child. Omitted fields fall back to the child loop's own defaults. + * Default per-child agent options (model) applied to every spawned child. + * Omitted fields fall back to the child loop's own defaults. There is no + * per-child persona: the deployment persona (the system-prompt plugin's + * `persona` config) is a context-wide section every agent shares. */ agentOptions?: AgentOptions } @@ -64,7 +66,6 @@ export const Config: z = z.object({ toolName: z.string().default('subagent'), agentOptions: z.object({ model: z.string(), - systemPrompt: z.string(), }), }) diff --git a/packages/subagent/tool-subagent/tests/tool-subagent.spec.ts b/packages/subagent/tool-subagent/tests/tool-subagent.spec.ts index 8799eba06b..611069ef76 100644 --- a/packages/subagent/tool-subagent/tests/tool-subagent.spec.ts +++ b/packages/subagent/tool-subagent/tests/tool-subagent.spec.ts @@ -149,10 +149,10 @@ describe('dsh-tool-subagent', () => { } }, }) - await ctx.plugin(tool, { provider: 'capture', agentOptions: { model: 'child-model', systemPrompt: 'be terse' } }) + await ctx.plugin(tool, { provider: 'capture', agentOptions: { model: 'child-model' } }) await callSubagent(ctx, { description: 'd', prompt: 'p' }) - expect(seen?.agentOptions).toEqual({ model: 'child-model', systemPrompt: 'be terse' }) + expect(seen?.agentOptions).toEqual({ model: 'child-model' }) }) it('defaults toolName and omits agentOptions when apply() is called directly (schema bypass)', async () => { diff --git a/packages/ui/acp-agent/README.md b/packages/ui/acp-agent/README.md index a1f35172c5..4a114619a6 100644 --- a/packages/ui/acp-agent/README.md +++ b/packages/ui/acp-agent/README.md @@ -23,7 +23,7 @@ Because the package wires no logger entry, an ACP leaf has **nothing to get wron | Key | Default | Routed to | |---|---|---| | `model` | (required) | the per-session agent template the bridge creates agents from | -| `systemPrompt` | (required) | the per-session agent's persona template (may reference `{{model}}`/`{{cwd}}`) | +| `persona` | — | the deployment persona template (may reference `{{model}}`/`{{cwd}}`), routed to `dsh-system-prompt` | | `persistenceRoot` | `./.sessions` | the JSONL backend's root directory | The leaf supplies the swappable backends: an LLM adapter (`llm-deepseek` for the real model, `llm-replay` for keyless snapshot replay) and a bash executor (`bash-local`). diff --git a/packages/ui/acp-agent/src/index.ts b/packages/ui/acp-agent/src/index.ts index c505e9bf41..3bd498dc63 100644 --- a/packages/ui/acp-agent/src/index.ts +++ b/packages/ui/acp-agent/src/index.ts @@ -39,35 +39,38 @@ import SessionPersistenceJsonl from '@deepseek-ai/dsh-session-persistence-jsonl' export const name = 'acp-agent' /** - * App config: the swappable per-deployment values. `model`/`systemPrompt` - * configure the agent template the ACP bridge creates each session's agent from - * (NOT a pre-created agent — ACP creates agents at `session/new`); + * App config: the swappable per-deployment values. `model` configures the + * agent template the ACP bridge creates each session's agent from (NOT a + * pre-created agent — ACP creates agents at `session/new`); `persona` is the + * deployment persona (forwarded to the system-prompt plugin); * `persistenceRoot` is the JSONL backend's directory. */ export interface Config { /** Model name for ACP-created agents (must have a registered adapter). */ model: string - /** Per-agent system prompt for ACP-created agents. */ - systemPrompt: string + /** Deployment persona (the system-prompt plugin's `persona` config). */ + persona?: string /** Directory the JSONL session backend writes under. Defaults to `./.sessions`. */ persistenceRoot?: string } export const Config: z = z.object({ model: z.string().required(), - systemPrompt: z.string().required(), + persona: z.string(), persistenceRoot: z.string().default('./.sessions'), }) /** * Compose the spine with the ACP front door. The agent-core bundle pre-creates - * NO agents (its `agents` list defaults to `[]`); the JSONL backend persists - * under `persistenceRoot`; the ACP bridge owns stdout for JSON-RPC and creates - * one agent per `session/new` from `model`/`systemPrompt`. No logger, no `hmr` — - * stdout stays pure. + * NO agents (its `agents` list defaults to `[]`) and carries the deployment + * `persona`; the JSONL backend persists under `persistenceRoot`; the ACP + * bridge owns stdout for JSON-RPC and creates one agent per `session/new` + * from `model`. No logger, no `hmr` — stdout stays pure. */ export function apply(ctx: Context, config: Config): void { - ctx.plugin(agentCore) + ctx.plugin(agentCore, { + ...config.persona !== undefined ? { persona: config.persona } : {}, + }) ctx.plugin(SessionPersistenceJsonl, { root: config.persistenceRoot ?? './.sessions' }) - ctx.plugin(acp, { model: config.model, systemPrompt: config.systemPrompt }) + ctx.plugin(acp, { model: config.model }) } diff --git a/packages/ui/acp-agent/tests/acp-agent.spec.ts b/packages/ui/acp-agent/tests/acp-agent.spec.ts index 7a02837fca..87cf670107 100644 --- a/packages/ui/acp-agent/tests/acp-agent.spec.ts +++ b/packages/ui/acp-agent/tests/acp-agent.spec.ts @@ -24,7 +24,7 @@ async function mount(config: acpAgent.Config): Promise { describe('dsh-acp-agent composition', () => { it('brings up the spine + persistence + the ACP bridge', async () => { - const ctx = await mount({ model: 'mock', systemPrompt: 'hi', persistenceRoot: '/tmp/dsh-acp-agent-test' }) + const ctx = await mount({ model: 'mock', persona: 'hi', persistenceRoot: '/tmp/dsh-acp-agent-test' }) expect(ctx.get('agents')).toBeDefined() expect(ctx.get('sessions')).toBeDefined() expect(ctx.get('sessionPersistence')).toBeDefined() @@ -40,7 +40,8 @@ describe('dsh-acp-agent composition', () => { // `ctx.plugin`, which validates+defaults the config first) with no // persistenceRoot, so the runtime fallback is the one that fires. const ctx = new Context() - acpAgent.apply(ctx, { model: 'mock', systemPrompt: 'hi' }) + // No persona: covers the omitted-persona forwarding branch too. + acpAgent.apply(ctx, { model: 'mock' }) await new Promise(resolve => setTimeout(resolve, 50)) expect(ctx.get('sessionPersistence')).toBeDefined() await ctx.fiber.dispose() diff --git a/packages/ui/acp/README.md b/packages/ui/acp/README.md index b9f75d6a89..60363566b7 100644 --- a/packages/ui/acp/README.md +++ b/packages/ui/acp/README.md @@ -15,7 +15,8 @@ It is a **client-driver / UI plugin**, the structured analogue of the readline ` | Key | Default | Meaning | |---|---|---| | `model` | — | Model name for created agents (must have a registered adapter). | -| `systemPrompt` | — | Per-agent persona template (may reference `{{model}}`/`{{cwd}}`). | + +(No persona key: the deployment persona is `dsh-system-prompt`'s own `persona` config — a context-wide section, so ACP-created agents render it without the bridge carrying prompt text.) The `initialize` handshake reports a fixed server identity (`agentInfo: { name: 'deepseek-harness-acp', version: '0.0.1' }`) — branding is a literal at the `initialize` site, not config. diff --git a/packages/ui/acp/src/index.ts b/packages/ui/acp/src/index.ts index 717e57a1f5..a819e607d0 100644 --- a/packages/ui/acp/src/index.ts +++ b/packages/ui/acp/src/index.ts @@ -115,8 +115,6 @@ function sameWorkspaceCwd(left: string, right: string): boolean { export interface AcpConfig { /** Model name for created agents (must have a registered adapter). */ model?: string - /** Per-agent system prompt. */ - systemPrompt?: string /** * Transport stream override. Production omits this (the plugin wires * `process.stdin`/`process.stdout` via `ndJsonStream`). Tests inject an @@ -129,7 +127,6 @@ export interface AcpConfig { export const Config: Schema = Schema.object({ model: Schema.string(), - systemPrompt: Schema.string(), }) /** @@ -705,10 +702,9 @@ export function apply(ctx: Context, config: AcpConfig): void { * (exactOptionalPropertyTypes: never assign `undefined` to an optional key). * Exported for unit coverage of both the present and absent branches. */ -export function agentOptions(config: AcpConfig): { model?: string; systemPrompt?: string } { +export function agentOptions(config: AcpConfig): { model?: string } { return { ...config.model !== undefined ? { model: config.model } : {}, - ...config.systemPrompt !== undefined ? { systemPrompt: config.systemPrompt } : {}, } } diff --git a/packages/ui/acp/tests/bridge.spec.ts b/packages/ui/acp/tests/bridge.spec.ts index 6f8edd341f..1e7e9ae511 100644 --- a/packages/ui/acp/tests/bridge.spec.ts +++ b/packages/ui/acp/tests/bridge.spec.ts @@ -148,15 +148,15 @@ describe('acp bridge', () => { await expect(harness.client.authenticate({ methodId: 'whatever' })).resolves.toBeDefined() }) - it('honors systemPrompt config', async () => { + it('renders the deployment persona into ACP-created agents\' requests', async () => { harness = await makeBridgeHarness({ storageDir, script: [textResponse('ok')], - config: { systemPrompt: 'be terse' }, + persona: 'be terse', }) await harness.client.initialize({ protocolVersion: PROTOCOL_VERSION, clientCapabilities: {} }) - // Create + prompt so the systemPrompt config flows through agentOptions and - // reaches the model request. + // Create + prompt so the system-prompt plugin's persona section reaches + // the model request of an agent the BRIDGE created (session/new). const { sessionId } = await harness.client.newSession({ cwd: process.cwd(), mcpServers: [] }) await harness.client.prompt({ sessionId, prompt: [{ type: 'text', text: 'hi' }] }) expect(harness.adapter.requests[0]?.system).toContain('be terse') diff --git a/packages/ui/acp/tests/harness.ts b/packages/ui/acp/tests/harness.ts index 01a5d30abc..39d77fe624 100644 --- a/packages/ui/acp/tests/harness.ts +++ b/packages/ui/acp/tests/harness.ts @@ -153,6 +153,8 @@ export interface BridgeHarness { export async function makeBridgeHarness(options: { script?: (StreamChunk[] | 'hang')[] config?: Partial + /** Deployment persona for the tree (the system-prompt plugin's config). */ + persona?: string storageDir: string /** * Plug the REAL `dsh-bash-local` executor + `dsh-tool-bash` tools (instead of @@ -183,7 +185,7 @@ export async function makeBridgeHarness(options: { const ctx = new Context() await ctx.plugin(LlmService) await ctx.plugin(SessionStore) - await ctx.plugin(SystemPrompt) + await ctx.plugin(SystemPrompt, { persona: options.persona ?? '' }) await ctx.plugin(ToolRegistry) await ctx.plugin(AgentRegistry) await ctx.plugin(AgentLoop, { agents: [] }) diff --git a/packages/ui/acp/tests/stream-update.spec.ts b/packages/ui/acp/tests/stream-update.spec.ts index 311a853acd..cb3eab3545 100644 --- a/packages/ui/acp/tests/stream-update.spec.ts +++ b/packages/ui/acp/tests/stream-update.spec.ts @@ -818,7 +818,5 @@ describe('agentOptions', () => { it('includes only the fields present in config', () => { expect(agentOptions({})).toEqual({}) expect(agentOptions({ model: 'm' })).toEqual({ model: 'm' }) - expect(agentOptions({ systemPrompt: 'sp' })).toEqual({ systemPrompt: 'sp' }) - expect(agentOptions({ model: 'm', systemPrompt: 'sp' })).toEqual({ model: 'm', systemPrompt: 'sp' }) }) }) diff --git a/packages/ui/stdio-agent/README.md b/packages/ui/stdio-agent/README.md index 9e48ba7f75..fd608b7888 100644 --- a/packages/ui/stdio-agent/README.md +++ b/packages/ui/stdio-agent/README.md @@ -11,7 +11,7 @@ A terminal chat always wants the same cluster, so the package owns it rather tha | Plugin | Why it is here | |---|---| | `@cordisjs/plugin-logger-console` | the console logger — stdout is just the terminal here, so logging to it is correct (the ACP app must NOT have this) | -| `@deepseek-ai/dsh-agent-core` | the spine, pre-creating a `main` agent from this app's `model`/`systemPrompt` | +| `@deepseek-ai/dsh-agent-core` | the spine, pre-creating a `main` agent from this app's `model` and carrying its `persona` | | `@deepseek-ai/dsh-session-persistence-jsonl` | durable JSONL session log under `persistenceRoot` | | `stdio-chat` (in-package module) | the readline UI, bound to the `main` agent | @@ -24,7 +24,7 @@ The leaf `cordis.yml` supplies only the **swappable backends** — an LLM adapte | Key | Default | Routed to | |---|---|---| | `model` | (required) | the pre-created `main` agent's model | -| `systemPrompt` | (required) | the `main` agent's persona template (may reference `{{model}}`) | +| `persona` | — | the deployment persona template (may reference `{{model}}`), routed to `dsh-system-prompt` | | `persistenceRoot` | `./.sessions` | the JSONL backend's root directory | | `welcome` | `ready.` | the stdin-chat banner | | `resumeSessionId` | — | resume a persisted session id instead of starting fresh (sourced from an env var in the leaf) | @@ -54,7 +54,7 @@ The leaf `cordis.yml` supplies only the **swappable backends** — an LLM adapte name: '@deepseek-ai/dsh-stdio-agent' config: model: deepseek-v4-flash - systemPrompt: 'You are a CLI coding assistant powered by the {{model}} model.' + persona: 'You are a coding assistant powered by the {{model}} model.' ``` Swap `llm-deepseek` for a `mock-llm` leaf plugin and you have the echo demo — "swap the backend, keep the app". diff --git a/packages/ui/stdio-agent/src/index.ts b/packages/ui/stdio-agent/src/index.ts index c576d6aaba..fe63e02643 100644 --- a/packages/ui/stdio-agent/src/index.ts +++ b/packages/ui/stdio-agent/src/index.ts @@ -51,15 +51,16 @@ export const name = 'stdio-agent' /** * App config: the swappable per-demo values, each routed to where the app wires - * it. `model`/`systemPrompt`/`resumeSessionId` configure the pre-created `main` - * agent (through {@link @deepseek-ai/dsh-agent-core}'s forwarded `agents` list); + * it. `model`/`resumeSessionId` configure the pre-created `main` agent (through + * {@link @deepseek-ai/dsh-agent-core}'s forwarded `agents` list); `persona` is + * the deployment persona (forwarded to the system-prompt plugin); * `persistenceRoot` is the JSONL backend's directory; `welcome` is the UI banner. */ export interface Config { /** Model name for the `main` agent (must have a registered adapter). */ model: string - /** System prompt for the `main` agent. */ - systemPrompt: string + /** Deployment persona (the system-prompt plugin's `persona` config). */ + persona?: string /** Directory the JSONL session backend writes under. Defaults to `./.sessions`. */ persistenceRoot?: string /** stdin-chat banner printed once on start. Defaults to `'ready.'`. */ @@ -74,7 +75,7 @@ export interface Config { export const Config: z = z.object({ model: z.string().required(), - systemPrompt: z.string().required(), + persona: z.string(), persistenceRoot: z.string().default('./.sessions'), welcome: z.string().default('ready.'), resumeSessionId: z.string(), @@ -83,17 +84,17 @@ export const Config: z = z.object({ /** * Compose the spine with the stdio front door. The console logger comes first * (infra), then the agent-core bundle pre-creating the `main` agent from this - * app's `model`/`systemPrompt`/`resumeSessionId`, then the JSONL backend, then - * the readline UI bound to `main`. The `hmr` dev-reload plugin is a leaf - * concern (see the module doc), so it is not mounted here. + * app's `model`/`resumeSessionId` with the deployment `persona`, then the JSONL + * backend, then the readline UI bound to `main`. The `hmr` dev-reload plugin is + * a leaf concern (see the module doc), so it is not mounted here. */ export function apply(ctx: Context, config: Config): void { ctx.plugin(ConsoleExporter) ctx.plugin(agentCore, { + ...config.persona !== undefined ? { persona: config.persona } : {}, agents: [{ id: AgentId('main'), model: config.model, - systemPrompt: config.systemPrompt, ...config.resumeSessionId !== undefined ? { resumeSessionId: SessionId(config.resumeSessionId) } : {}, }], }) diff --git a/packages/ui/stdio-agent/tests/stdio-agent.spec.ts b/packages/ui/stdio-agent/tests/stdio-agent.spec.ts index 5c22fdb984..09fbf8987d 100644 --- a/packages/ui/stdio-agent/tests/stdio-agent.spec.ts +++ b/packages/ui/stdio-agent/tests/stdio-agent.spec.ts @@ -8,8 +8,9 @@ import * as stdioAgent from '../src/index.ts' * Unit coverage for the @deepseek-ai/dsh-stdio-agent app plugin: mounting it * composes the console logger, the agent-core spine (pre-creating the `main` * agent from the app config), the JSONL backend, and the readline UI in one - * `ctx.plugin`. The forwarded `model`/`systemPrompt` reach the pre-created - * agent; `persistenceRoot`/`welcome`/`resumeSessionId` route to their backends. + * `ctx.plugin`. The forwarded `model` reaches the pre-created agent and + * `persona` the system-prompt plugin; `persistenceRoot`/`welcome`/ + * `resumeSessionId` route to their backends. * * `hmr` is NOT part of this plugin (it is a leaf entry — a Loader-only dev * plugin the in-process tier cannot import); the keyless echo smoke in @@ -30,7 +31,7 @@ async function mount(config: stdioAgent.Config): Promise { describe('dsh-stdio-agent app', () => { it('composes the spine + front-door cluster and pre-creates the main agent', async () => { - const ctx = await mount({ model: 'mock', systemPrompt: 'hi', persistenceRoot: '/tmp/dsh-stdio-agent-spec' }) + const ctx = await mount({ model: 'mock', persona: 'hi', persistenceRoot: '/tmp/dsh-stdio-agent-spec' }) // The spine services (brought up by the agent-core bundle) are all present. expect(ctx.get('agents')).toBeDefined() expect(ctx.get('agentLoop')).toBeDefined() @@ -46,7 +47,8 @@ describe('dsh-stdio-agent app', () => { // apply()'s last two lines are the ones that fire — covering a // schema-bypassing direct-mount caller. const ctx = new Context() - stdioAgent.apply(ctx, { model: 'mock', systemPrompt: 'hi' }) + // No persona: covers the omitted-persona forwarding branch too. + stdioAgent.apply(ctx, { model: 'mock' }) await new Promise(resolve => setTimeout(resolve, 80)) expect(ctx.get('sessionPersistence')).toBeDefined() expect(ctx.get('agents')?.get(AgentId('main'))).toBeDefined() @@ -59,7 +61,7 @@ describe('dsh-stdio-agent app', () => { // the branch that maps resumeSessionId through is what this covers. const ctx = await mount({ model: 'mock', - systemPrompt: 'hi', + persona: 'hi', persistenceRoot: '/tmp/dsh-stdio-agent-spec-resume', resumeSessionId: 'no-such-session', }) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 6ec5a17017..d5fb68c746 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -188,6 +188,10 @@ importers: version: 4.0.0-rc.6(@cordisjs/plugin-include@1.0.4)(@cordisjs/plugin-loader@1.0.0-rc.4) packages/core/agent-core: + dependencies: + schemastery: + specifier: ^3.18.0 + version: 3.18.0 devDependencies: '@cordisjs/plugin-timer': specifier: workspace:^ @@ -267,6 +271,10 @@ importers: version: 4.0.0-rc.6(@cordisjs/plugin-include@1.0.4)(@cordisjs/plugin-loader@1.0.0-rc.4) packages/core/system-prompt: + dependencies: + schemastery: + specifier: ^3.18.0 + version: 3.18.0 devDependencies: '@deepseek-ai/dsh-llm': specifier: workspace:^ From 6bca9cbeb3b84462b4663753e0fb6dc40769c645 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sun, 5 Jul 2026 23:28:13 +0800 Subject: [PATCH 7/8] docs(rfc): record the subagent provider-lifecycle events as their own RFC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The subagent/provider-added and subagent/provider-removed events carry a decision of their own — reactive consumer registration instead of a load-order requirement the cordis Loader never guaranteed — buried in a section of the prompt-variables RFC. Extract it: the mechanism, the Loader-concurrency evidence, the rejected alternatives (apply-time lookup, polling, assemble-time wording, name-keyed wording), and what we give up (the absent-tool window, the dup-toolName blast radius) move to 2026-07-05-subagent-provider-lifecycle-events.md; the prompt-variables RFC keeps the context contract and points there; index regenerated. --- docs/rfc/README.md | 1 + ...t-variables-and-tool-guidance-ownership.md | 5 ++- ...7-05-subagent-provider-lifecycle-events.md | 36 +++++++++++++++++++ 3 files changed, 39 insertions(+), 3 deletions(-) create mode 100644 docs/rfc/implemented/architecture/2026-07-05-subagent-provider-lifecycle-events.md diff --git a/docs/rfc/README.md b/docs/rfc/README.md index 3d41de0811..17e59c514d 100644 --- a/docs/rfc/README.md +++ b/docs/rfc/README.md @@ -153,6 +153,7 @@ Do NOT write one for a mechanical or local choice (a variable name, a one-file r | [Tagged render-intent union for tool-call presentation](implemented/architecture/2026-07-02-tool-render-intent-union.md) | 2026-07-02 | | [Add direct directory listing to the filesystem seam](implemented/architecture/2026-07-03-filesystem-directory-listing-seam.md) | 2026-07-03 | | [Prompt variables and tool-guidance ownership](implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md) | 2026-07-05 | +| [Subagent provider-lifecycle events: `subagent/provider-added` / `subagent/provider-removed`](implemented/architecture/2026-07-05-subagent-provider-lifecycle-events.md) | 2026-07-05 | ### Process diff --git a/docs/rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md b/docs/rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md index eafd42cf9e..2442212f11 100644 --- a/docs/rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md +++ b/docs/rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md @@ -38,7 +38,7 @@ Per-tool semantics and when-to-use live in tool DESCRIPTIONS, which already ship ### The subagent context contract -`SubagentProvider` gains `readonly inheritsParentContext: boolean` — a DESCRIPTIVE fact beside `capabilities`, not in it (capabilities are start-time validation; nothing validates against this flag). Spawn and ACP declare `false`, fork declares `true`. `dsh-tool-subagent` derives both the tool description and the `prompt` parameter description from the flag (`providerWording`): the fork instance now tells the model the child inherits the conversation's completed turns (not the in-flight turn) and that its prompt should state only what is new. Because the description is fixed at tool registration while providers arrive on their own fibers, the registry announces provider lifecycle (`subagent/provider-added`/`subagent/provider-removed`) and the tool MIRRORS it: it registers when its provider is (or becomes) available, unregisters when the provider goes away, and re-derives the wording on re-registration (HMR). There is deliberately NO load-order requirement — the cordis Loader starts sibling entries concurrently (`Promise.all` over the group), so "listed first" never guaranteed "registered first"; while the provider is absent the tool does not exist, which cannot lie. +`SubagentProvider` gains `readonly inheritsParentContext: boolean` — a DESCRIPTIVE fact beside `capabilities`, not in it (capabilities are start-time validation; nothing validates against this flag). Spawn and ACP declare `false`, fork declares `true`. `dsh-tool-subagent` derives both the tool description and the `prompt` parameter description from the flag (`providerWording`): the fork instance now tells the model the child inherits the conversation's completed turns (not the in-flight turn) and that its prompt should state only what is new. Deriving the description from a provider that arrives on its own fiber is what forced the provider-lifecycle events and the tool's reactive registration — that mechanism, its Loader-concurrency rationale, and its rejected alternatives are recorded in [the provider-lifecycle-events RFC](2026-07-05-subagent-provider-lifecycle-events.md). ## Rejected alternatives @@ -47,8 +47,7 @@ Per-tool semantics and when-to-use live in tool DESCRIPTIONS, which already ship - **Hand-write the model name in each persona** — duplicates the `model:` key one line above and silently lies after a config edit; the exact disease this RFC cures. - **Lenient interpolation (leave unknown refs verbatim, or substitute empty)** — a typo ships `{{modle}}` (or a hole) to the model and nobody notices until transcript review. - **Per-instance subagent wording in config** — returns model-facing prose to every deployment × instance, the P2 disease again. **Keying wording off the provider NAME** — `providerName` is itself config, so a renamed provider silently gets the wrong words. -- **Resolving the provider at `apply` time and throwing when absent (a load-order requirement)** — the first implementation. Rejected after review reproduced the failure: the Loader starts sibling entries concurrently and `Entry.init()` does not await activation, so a backend whose activation is delayed leaves the tool's fiber permanently failed even when "listed first" — the ordering the requirement leaned on is not a contract the Loader offers ("async state is not synchronous state"). Provider-lifecycle events make the ordering question disappear instead of documenting it. -- **Section-only subagent wording (lazily resolved at assemble)** — would also tolerate any load order, but the DESCRIPTION is fixed at tool registration and is where tool-choice guidance belongs; reactive registration keeps the description authoritative AND order-free. +- **Resolving the provider at `apply` time (a load-order requirement)** and **section-only subagent wording (lazily resolved at assemble)** — the alternatives to the provider-lifecycle events; both rejected in [the provider-lifecycle-events RFC](2026-07-05-subagent-provider-lifecycle-events.md). ## What we give up diff --git a/docs/rfc/implemented/architecture/2026-07-05-subagent-provider-lifecycle-events.md b/docs/rfc/implemented/architecture/2026-07-05-subagent-provider-lifecycle-events.md new file mode 100644 index 0000000000..026894c6ba --- /dev/null +++ b/docs/rfc/implemented/architecture/2026-07-05-subagent-provider-lifecycle-events.md @@ -0,0 +1,36 @@ +# Subagent provider-lifecycle events: `subagent/provider-added` / `subagent/provider-removed` + +Status: implemented (accepted 2026-07-05) + +## Context + +[The prompt-variables RFC](2026-07-05-prompt-variables-and-tool-guidance-ownership.md) makes `dsh-tool-subagent` DERIVE its model-facing wording from its provider: `SubagentProvider.inheritsParentContext` (spawn/ACP `false`, fork `true`) drives both the tool description and the `prompt` parameter description (`providerWording`), so the fork tool stops lying about context inheritance. That fix created a cross-fiber data dependency: a tool's description is fixed at TOOL REGISTRATION (deliberately — the description is where tool-choice guidance lives), but the provider arrives on its own plugin fiber, on no particular schedule. + +The first implementation resolved the provider at the tool plugin's `apply` time and threw when it was absent — an implicit load-order requirement ("list the backend before the tool in cordis.yml"). Review reproduced the failure that requirement hides: the cordis Loader starts sibling entries CONCURRENTLY (`Promise.all` over the group) and `Entry.init()` does not await activation, so a backend whose activation is delayed leaves the tool's fiber permanently failed even when "listed first". The ordering the requirement leaned on is not a contract the Loader offers — "async state is not synchronous state" ([defensive patterns](../../../defensive-patterns.md)). + +## Decision + +The registry announces provider membership as typed events, and the consumer mirrors them instead of assuming order: + +- **`subagent/provider-added(provider)`** — a provider became resolvable in the `ctx.subagents` registry. Emitted on registration. +- **`subagent/provider-removed(name)`** — a provider left the registry (its plugin's fiber was disposed — an unload or an HMR reload). Emitted from the registration's disposer. + +`dsh-tool-subagent` mirrors its named provider's lifecycle: it registers the tool when the provider is (or becomes) available — deriving the wording from that provider at that moment — unregisters the tool when the provider goes away, and re-derives on re-registration (HMR reload). While the provider is absent the tool does not exist, which cannot lie to the model. There is deliberately NO load-order requirement left to document: the events make the ordering question disappear instead of pinning it. + +The events also complete the seam's vocabulary: `ctx.subagents` is a named registry on which multiple delegation backends coexist (`spawn`, `fork`, `acp`), and a registry whose contents other plugins derive state from should announce membership changes as typed events rather than requiring polling or load-order faith. + +## Rejected alternatives + +- **Resolving the provider at `apply` time and throwing when absent (a load-order requirement)** — the first implementation, rejected after review reproduced the failure above. Documenting the requirement ("list backends first") would pin a guarantee the Loader does not make. +- **Retrying the lookup (poll until the provider appears)** — converges eventually but invents a private readiness protocol beside the one the framework already has (effect registration + disposal); it also cannot notice a provider LEAVING, so HMR would strand a tool whose wording describes a disposed backend. +- **Section-only subagent wording, lazily resolved at assemble time** — tolerates any load order too, but moves tool-choice guidance out of the DESCRIPTION, contradicting the ownership rule the prompt-variables RFC establishes (per-tool semantics and when-to-use belong in the description). Reactive registration keeps the description authoritative AND order-free. +- **Keying wording off the provider NAME instead of the provider object** — `providerName` is itself config, so a renamed provider silently gets the wrong words; deriving from the resolved provider's own `inheritsParentContext` cannot drift. + +## What we give up + +- **A window where the tool is absent.** Between backend disposal and re-registration (an HMR reload), the model sees no subagent tool. This is the honest state — the alternative is a tool that dispatches into nothing — and the tool registry's `tools/change` emit keeps prompt assembly current. +- **Two waiting fibers sharing a `toolName` is an invalid config caught late.** If two loads of `dsh-tool-subagent` name different providers but the same `toolName`, both wait, and whichever provider arrives first registers; the second registration throws only when ITS provider arrives. `TODO(subagent-dup-toolname)` in the plugin records this blast radius; the tool registry's duplicate-name rejection remains the backstop. + +## Consequences + +Consumers deriving state from a named provider react to `subagent/provider-added`/`-removed` instead of reading the registry at `apply` time; `dsh-tool-subagent` is the reference implementation. The [events catalog](../../../cordis-catalog/events.md) carries the exact signatures, and the [producer/consumer map](../../../event-producer-consumer.md) shows `dsh-subagent` emitting and `dsh-tool-subagent` consuming both events. From 3633cf90c80ab44e984b76580aa4e889dd93a868 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Mon, 6 Jul 2026 00:34:47 +0800 Subject: [PATCH 8/8] fix(review): contain provider-removed listener failures; pin the model-via-request path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ds-review-bot round 2, both warnings: - subagent/provider-removed now routes through emitLifecycle (per-listener containment, the subagent/start|end precedent) instead of raw ctx.emit, whose dispatch halts on the first throw: a throwing subscriber can no longer starve a later mirror into keeping a stale tool, nor disrupt the backend fiber's teardown mid-disposer. provider-added deliberately keeps propagation (register-time rollback semantics, like the system-prompt registries); the asymmetry is documented on emitLifecycle, the event JSDoc, and the provider-lifecycle RFC. - The documented model-via-agent/request fallback composes with a {{model}} persona via the ownership rule itself: the plugin supplying the model late states it early on the system-prompt/assemble waterfall. Declined re-ordering render after agent/request — it would break the agent/pre-step contract (compaction must measure the prompt the model sees). New loop test pins the supply path end-to-end; the RFC's {{model}} consequence bullet now covers supply as well as switch. --- docs/cordis-catalog/events.md | 8 +++--- docs/cordis-catalog/services.md | 2 +- docs/event-producer-consumer.md | 6 ++--- ...t-variables-and-tool-guidance-ownership.md | 2 +- ...7-05-subagent-provider-lifecycle-events.md | 3 ++- packages/core/agent-loop/tests/loop.spec.ts | 26 ++++++++++++++++++ packages/subagent/subagent/src/index.ts | 27 +++++++++++++++---- .../subagent/subagent/tests/service.spec.ts | 21 +++++++++++++++ 8 files changed, 80 insertions(+), 15 deletions(-) diff --git a/docs/cordis-catalog/events.md b/docs/cordis-catalog/events.md index 8db44c5f1b..d10a7eeabd 100644 --- a/docs/cordis-catalog/events.md +++ b/docs/cordis-catalog/events.md @@ -243,7 +243,7 @@ A subagent run settled — emitted when SubagentRun.result resolves (any stop re 'subagent/end'(info: SubagentRunEndInfo): void ``` -Source: [`packages/subagent/subagent/src/index.ts:96`](../../packages/subagent/subagent/src/index.ts) +Source: [`packages/subagent/subagent/src/index.ts:98`](../../packages/subagent/subagent/src/index.ts) ### `subagent/provider-added` — emit @@ -257,13 +257,13 @@ Source: [`packages/subagent/subagent/src/index.ts:72`](../../packages/subagent/s ### `subagent/provider-removed` — emit -A provider left the registry (its plugin's fiber was disposed — an unload or an HMR reload). Consumers holding provider-derived state drop it here; a reload re-fires `subagent/provider-added` with the fresh provider. +A provider left the registry (its plugin's fiber was disposed — an unload or an HMR reload). Consumers holding provider-derived state drop it here; a reload re-fires `subagent/provider-added` with the fresh provider. Delivered with per-listener containment: a throwing subscriber is logged, never starves later subscribers, and never disrupts the provider's teardown. ```ts cordis-catalog 'subagent/provider-removed'(name: string): void ``` -Source: [`packages/subagent/subagent/src/index.ts:81`](../../packages/subagent/subagent/src/index.ts) +Source: [`packages/subagent/subagent/src/index.ts:83`](../../packages/subagent/subagent/src/index.ts) ### `subagent/start` — emit @@ -273,7 +273,7 @@ A subagent run started — emitted after the provider is resolved and its capabi 'subagent/start'(info: SubagentRunInfo): void ``` -Source: [`packages/subagent/subagent/src/index.ts:89`](../../packages/subagent/subagent/src/index.ts) +Source: [`packages/subagent/subagent/src/index.ts:91`](../../packages/subagent/subagent/src/index.ts) ## `system-prompt/*` diff --git a/docs/cordis-catalog/services.md b/docs/cordis-catalog/services.md index ca2ee64334..c91f1578b2 100644 --- a/docs/cordis-catalog/services.md +++ b/docs/cordis-catalog/services.md @@ -176,7 +176,7 @@ list(): string[] start(name: string, request: SubagentStartRequest): SubagentRun ``` -Source: [`packages/subagent/subagent/src/index.ts:142`](../../packages/subagent/subagent/src/index.ts) +Source: [`packages/subagent/subagent/src/index.ts:144`](../../packages/subagent/subagent/src/index.ts) ## `ctx.systemPrompt` — `SystemPrompt` diff --git a/docs/event-producer-consumer.md b/docs/event-producer-consumer.md index f6cd631296..18c2b9faf0 100644 --- a/docs/event-producer-consumer.md +++ b/docs/event-producer-consumer.md @@ -25,10 +25,10 @@ This matrix shows which packages dispatch each harness-owned event and which pac | `session/created` | `emit` | [`packages/core/session/src/index.ts:36`](../packages/core/session/src/index.ts) | [`session`](../packages/core/session) (`emit`) | [`invariants`](../packages/support/invariants), [`session-persistence`](../packages/session-persistence/session-persistence) | | `session/event` | `emit` | [`packages/core/session/src/index.ts:44`](../packages/core/session/src/index.ts) | [`session`](../packages/core/session) (`emit`) | [`acp`](../packages/ui/acp), [`invariants`](../packages/support/invariants), [`session-persistence`](../packages/session-persistence/session-persistence), [`stdio-agent`](../packages/ui/stdio-agent) | | `session/flush` | `parallel` | [`packages/core/session/src/index.ts:54`](../packages/core/session/src/index.ts) | [`agent-loop`](../packages/core/agent-loop) (`parallel`) | [`session-persistence`](../packages/session-persistence/session-persistence) | -| `subagent/end` | `emit` | [`packages/subagent/subagent/src/index.ts:96`](../packages/subagent/subagent/src/index.ts) | [`subagent`](../packages/subagent/subagent) (`events.dispatch`) | [`hooks-claude`](../packages/hooks/hooks-claude) | +| `subagent/end` | `emit` | [`packages/subagent/subagent/src/index.ts:98`](../packages/subagent/subagent/src/index.ts) | [`subagent`](../packages/subagent/subagent) (`events.dispatch`) | [`hooks-claude`](../packages/hooks/hooks-claude) | | `subagent/provider-added` | `emit` | [`packages/subagent/subagent/src/index.ts:72`](../packages/subagent/subagent/src/index.ts) | [`subagent`](../packages/subagent/subagent) (`emit`) | [`tool-subagent`](../packages/subagent/tool-subagent) | -| `subagent/provider-removed` | `emit` | [`packages/subagent/subagent/src/index.ts:81`](../packages/subagent/subagent/src/index.ts) | [`subagent`](../packages/subagent/subagent) (`emit`) | [`tool-subagent`](../packages/subagent/tool-subagent) | -| `subagent/start` | `emit` | [`packages/subagent/subagent/src/index.ts:89`](../packages/subagent/subagent/src/index.ts) | [`subagent`](../packages/subagent/subagent) (`events.dispatch`) | [`hooks-claude`](../packages/hooks/hooks-claude) | +| `subagent/provider-removed` | `emit` | [`packages/subagent/subagent/src/index.ts:83`](../packages/subagent/subagent/src/index.ts) | - | [`tool-subagent`](../packages/subagent/tool-subagent) | +| `subagent/start` | `emit` | [`packages/subagent/subagent/src/index.ts:91`](../packages/subagent/subagent/src/index.ts) | [`subagent`](../packages/subagent/subagent) (`events.dispatch`) | [`hooks-claude`](../packages/hooks/hooks-claude) | | `system-prompt/assemble` | `waterfall` | [`packages/core/system-prompt/src/index.ts:38`](../packages/core/system-prompt/src/index.ts) | [`system-prompt`](../packages/core/system-prompt) (`waterfall`) | - | | `system-prompt/change` | `emit` | [`packages/core/system-prompt/src/index.ts:44`](../packages/core/system-prompt/src/index.ts) | [`system-prompt`](../packages/core/system-prompt) (`emit`) | - | | `tools/change` | `emit` | [`packages/core/tools/src/index.ts:87`](../packages/core/tools/src/index.ts) | [`tools`](../packages/core/tools) (`emit`) | - | diff --git a/docs/rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md b/docs/rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md index f7ba9b4480..24a525307e 100644 --- a/docs/rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md +++ b/docs/rfc/implemented/architecture/2026-07-05-prompt-variables-and-tool-guidance-ownership.md @@ -64,7 +64,7 @@ Per-tool semantics and when-to-use live in tool DESCRIPTIONS, which already ship ## Consequences - Every fact in the assembled prompt now has exactly one owner, and the hand-maintained tool prose in leaf YAML is gone: loading or dropping a tool plugin no longer means editing any deployment's persona. -- `{{model}}` reflects `AgentOptions.model` at assembly time. A plugin that switches models in the `agent/request` waterfall makes the prompt's claim stale for that step; such a plugin can rewrite `options.system` in the same waterfall if it cares. Accepted. +- `{{model}}` reflects `AgentOptions.model` at assembly time. A plugin that switches models in the `agent/request` waterfall makes the prompt's claim stale for that step, and one that SUPPLIES the model there (options.model unset — the loop's documented fallback) leaves the variable valueless at render, failing a `{{model}}` persona before the waterfall runs. Both have the same remedy, and it is the ownership rule itself: the plugin that owns the late-bound model fact states it early on the `system-prompt/assemble` waterfall (`assembly.variables['model'] = …`) — one owner, both statements; a loop test pins the supply path end-to-end. Accepted. - While a bound provider is absent (not yet activated, unloaded, mid-HMR-reload), the subagent tool does not exist and a model request in that window simply lacks it. That is the honest state — the alternative was a registered tool whose description or execution could not be trusted. - Strictness means a persona can fail a turn at render (e.g. `{{cwd}}` on a cwd-less session). The failure is contained — the turn ends `error`, the loop survives — and it is an authoring error we WANT loud. - No escape syntax for a literal `{{name}}` in prompt prose yet; add one if a real prompt ever needs it. diff --git a/docs/rfc/implemented/architecture/2026-07-05-subagent-provider-lifecycle-events.md b/docs/rfc/implemented/architecture/2026-07-05-subagent-provider-lifecycle-events.md index 2be670ddcd..46e7ea4e71 100644 --- a/docs/rfc/implemented/architecture/2026-07-05-subagent-provider-lifecycle-events.md +++ b/docs/rfc/implemented/architecture/2026-07-05-subagent-provider-lifecycle-events.md @@ -28,6 +28,7 @@ The events also complete the seam's vocabulary: `ctx.subagents` is a named regis ## Consequences -- Consumers deriving state from a named provider react to `subagent/provider-added`/`-removed` instead of reading the registry at `apply` time; `dsh-tool-subagent` is the reference implementation. The [events catalog](../../../cordis-catalog/events.md) carries the exact signatures, and the [producer/consumer map](../../../event-producer-consumer.md) shows `dsh-subagent` emitting and `dsh-tool-subagent` consuming both events. +- Consumers deriving state from a named provider react to `subagent/provider-added`/`-removed` instead of reading the registry at `apply` time; `dsh-tool-subagent` is the reference implementation. +- **The two emits carry asymmetric failure semantics, deliberately.** `provider-removed` fires inside the registration's disposer and is delivered with PER-LISTENER containment (the service's `emitLifecycle`, not raw `ctx.emit`, which halts dispatch on the first throw): a throwing subscriber is logged, never starves a later mirror into holding a stale tool, and never disrupts the backend fiber's teardown — dispose reaches quiescence. `provider-added` propagates: it fires at registration time, where a throwing listener unwinds the yielded rollback — the same fail-loud register-time semantics as the system-prompt registries. The run-time backstop bounds what a stale mirror could cost anyway: `start()` re-resolves the provider by name per run, so a tool that outlived its provider fails that call cleanly instead of dispatching into a dead backend. The [events catalog](../../../cordis-catalog/events.md) carries the exact signatures, and the [producer/consumer map](../../../event-producer-consumer.md) shows `dsh-subagent` emitting and `dsh-tool-subagent` consuming both events. - **A window where the tool is absent.** Between backend disposal and re-registration (an HMR reload), the model sees no subagent tool. This is the honest state — the alternative is a tool that dispatches into nothing — and the tool registry's `tools/change` emit keeps prompt assembly current. - **Two waiting fibers sharing a `toolName` is an invalid config caught late.** If two loads of `dsh-tool-subagent` name different providers but the same `toolName`, both wait, and whichever provider arrives first registers; the second registration throws only when ITS provider arrives. `TODO(subagent-dup-toolname)` in the plugin records this blast radius; the tool registry's duplicate-name rejection remains the backstop. diff --git a/packages/core/agent-loop/tests/loop.spec.ts b/packages/core/agent-loop/tests/loop.spec.ts index 7de441a928..16a9b5e394 100644 --- a/packages/core/agent-loop/tests/loop.spec.ts +++ b/packages/core/agent-loop/tests/loop.spec.ts @@ -218,6 +218,32 @@ describe('agent loop', () => { expect(turnEnds[1]?.type === 'turn/end' && turnEnds[1].data.reason.kind).toBe('completed') }) + it('supports the model-via-agent/request path with a {{model}} persona: the supplier states it via the assemble waterfall', async () => { + // AgentOptions.model unset: the model arrives in the agent/request + // waterfall (the loop's documented fallback — see runStep's no-model + // error). {{model}} renders BEFORE that waterfall, so the SAME plugin + // states the fact early on system-prompt/assemble — the owner of a + // late-bound fact owns stating it wherever it is claimed. + const adapter = new MockAdapter([textResponse('ok')]) + const ctx = await harness(adapter, 'You run on {{model}}.') + ctx.on('system-prompt/assemble', async (assembly, _context, next) => { + assembly.variables['model'] = 'mock' + return next() + }) + ctx.on('agent/request', async (_agent, _turn, _step, options, next) => { + options.model = 'mock' + return next() + }) + const agent = ctx.agentLoop.create(AgentId('a-late-model'), {}) + + send(agent, 'hi') + await waitForIdle(ctx, agent) + + expect(adapter.requests).toHaveLength(1) + expect(adapter.requests[0]!.model).toBe('mock') + expect(adapter.requests[0]!.system).toBe('You are an AI agent powered by the DeepSeek Harness SDK.\n\nYou run on mock.') + }) + it('omits the system field when a system-prompt/assemble veto empties the assembly', async () => { // The documented escape valve: a deployment that must drop the harness // openers short-circuits the assemble waterfall; the request then carries diff --git a/packages/subagent/subagent/src/index.ts b/packages/subagent/subagent/src/index.ts index a9541eabce..91230b7ff4 100644 --- a/packages/subagent/subagent/src/index.ts +++ b/packages/subagent/subagent/src/index.ts @@ -74,7 +74,9 @@ declare module 'cordis' { * A provider left the registry (its plugin's fiber was disposed — an * unload or an HMR reload). Consumers holding provider-derived state drop * it here; a reload re-fires `subagent/provider-added` with the fresh - * provider. + * provider. Delivered with per-listener containment: a throwing + * subscriber is logged, never starves later subscribers, and never + * disrupts the provider's teardown. * @param name - the registry name that no longer resolves. * @mode emit */ @@ -163,10 +165,13 @@ export class SubagentService extends Service { this.providers.set(provider.name, provider) // Yield the rollback BEFORE emitting `subagent/provider-added`: a // throwing added-listener then unregisters the provider (and announces - // the removal) instead of leaking it into the registry. + // the removal) instead of leaking it into the registry. The removal + // announcement itself is contained PER LISTENER ({@link emitLifecycle}): + // it runs inside this disposer, where a propagating subscriber would + // disrupt the backend fiber's teardown and starve later mirrors. yield () => { this.providers.delete(provider.name) - this.ctx.emit('subagent/provider-removed', provider.name) + this.emitLifecycle('subagent/provider-removed', provider.name) } this.ctx.emit('subagent/provider-added', provider) }.bind(this), 'subagents.registerProvider()') @@ -265,10 +270,22 @@ export class SubagentService extends Service { * on the first throw — so this resolves the listener callbacks via * `ctx.events.dispatch` and contains each call, the same guarantee * `BashExecutor.notifyTaskDone` gives its own listener set. + * + * `subagent/provider-removed` routes through here too: it fires inside the + * provider registration's DISPOSER, where a propagating listener would + * disrupt the backend fiber's teardown (dispose must reach quiescence) and a + * starved later listener would leave a mirror consumer (`dsh-tool-subagent`) + * holding a tool for a provider that no longer exists. `subagent/provider-added` + * deliberately does NOT: it fires at registration time, where a throwing + * listener unwinds the yielded rollback — the same fail-loud register-time + * semantics as the system-prompt registries. */ + private emitLifecycle(name: 'subagent/start', info: SubagentRunInfo): void + private emitLifecycle(name: 'subagent/end', info: SubagentRunEndInfo): void + private emitLifecycle(name: 'subagent/provider-removed', info: string): void private emitLifecycle( - name: 'subagent/start' | 'subagent/end', - info: SubagentRunInfo | SubagentRunEndInfo, + name: 'subagent/start' | 'subagent/end' | 'subagent/provider-removed', + info: SubagentRunInfo | SubagentRunEndInfo | string, ): void { for (const callback of this.ctx.events.dispatch('emit', [name, info])) { try { diff --git a/packages/subagent/subagent/tests/service.spec.ts b/packages/subagent/subagent/tests/service.spec.ts index 6b5e737a3d..e49b4c12e0 100644 --- a/packages/subagent/subagent/tests/service.spec.ts +++ b/packages/subagent/subagent/tests/service.spec.ts @@ -77,6 +77,27 @@ describe('SubagentService', () => { expect(ctx.subagents.getProvider('alpha')).toBeDefined() }) + it('contains a throwing provider-removed listener: later mirrors still hear it, teardown completes', async () => { + // provider-removed fires inside the registration's DISPOSER, so a + // propagating listener would disrupt the backend's teardown; and cordis + // emit halts on the first throw, so an uncontained one would starve every + // mirror registered after it (a stale model-facing tool). Both are + // prevented by per-listener containment. + const ctx = new Context() + await ctx.plugin(SubagentService) + const warnings: string[] = [] + ctx.logger.warn = ((message: unknown) => void warnings.push(String(message))) as typeof ctx.logger.warn + ctx.on('subagent/provider-removed', () => { throw new Error('boom removed listener') }) + const heard: string[] = [] + ctx.on('subagent/provider-removed', name => void heard.push(name)) + + const dispose = ctx.subagents.registerProvider(new StubProvider('alpha')) + expect(() => { dispose() }).not.toThrow() + expect(heard).toEqual(['alpha']) // the listener AFTER the thrower still ran + expect(ctx.subagents.getProvider('alpha')).toBeUndefined() // teardown reached quiescence + expect(warnings.some(w => w.includes('boom removed listener'))).toBe(true) + }) + it('registers a provider and starts a run on it by name', async () => { const ctx = new Context() await ctx.plugin(SubagentService)