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 aae4564513..854807c109 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,21 +24,21 @@ 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; 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. +Plugins register `{{name}}` values through `ctx.systemPrompt.variable(name, provider)`. Assembly resolves them into the waterfall-visible variable map. Rendering rejects unknown own-property references, registered providers that return `undefined`, malformed complete references, and unbalanced references that still contain a closing `}}`; a lone unmatched `{{` remains prose, and substituted values are not rescanned. Registration rejects invalid or duplicate variable names, and section names are unique. `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 -`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 the global default `deployment:persona` at order 0, whose text is the plugin's own `persona` config. `AgentOptions.systemPrompt` and the loop's special-case join are gone: `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. An agent-scoped section with the same `deployment:persona` name shadows the default for that agent; programmatic setup may register one directly, and the subagent persona feature installs one before publishing an in-process child when the selected provider supports it. Order bands are convention: harness identity `-100`, persona `0`, tool guidance `100–199`; other negative orders also render before the persona. +`dsh-system-prompt` owns `harness:identity` at order `-100` and the configured `deployment:persona` at order 0, so both survive a replacement loop. Prompt rendering has one path, `renderPrompt(assembly)`, and `agent/pre-step` therefore measures the exact prompt used for compaction. An agent-scoped `deployment:persona` shadows the global default and lets subagent providers install a persona before publication. The conventional order bands are identity `-100`, persona `0`, and tool guidance `100–199`. ### 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. +Per-tool semantics and selection guidance live in tool descriptions. Prompt sections carry only cross-call habits, such as checking bash exit markers or preferring filesystem tools over shell commands. `todo_write` and subagent tools need no section because their descriptions contain the full contract. Deployment personas contain only role and behavior. ### The subagent conversation-history descriptor -`SubagentProvider` gains `readonly inheritsParentContext: boolean` — a DESCRIPTIVE conversation-history fact beside `capabilities`, not in it (capabilities are start-time validation; nothing validates against this flag). Spawn and ACP declare `false`, fork declares `true`. The name refers only to conversation seeding, not Cordis scope, services, tools, or authority. `dsh-tool-subagent` derives both the tool description and the `prompt` parameter description from the flag: the fork instance now tells the model the child is seeded with 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). +`SubagentProvider.inheritsParentContext` describes conversation seeding, not scope, services, tools, or authority. Spawn and ACP set it to `false`; fork sets it to `true`. `dsh-tool-subagent` derives its tool and prompt-parameter descriptions from the flag, including that fork inherits completed turns but not the in-flight turn. Provider lifecycle events keep that wording synchronized with reactive provider registration; their rationale lives in the [provider-lifecycle-events RFC](2026-07-05-subagent-provider-lifecycle-events.md). ## Alternatives considered @@ -56,10 +56,10 @@ Per-tool semantics and when-to-use live in tool DESCRIPTIONS, which already ship ## Shipped invariants -- `renderPrompt(await assemble(assembleContextFor(agent)))` for the coding-agent example renders the harness identity, then the persona (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. -- Snapshot goldens are prompt-independent by construction: llm-replay keys replay on (turn, step) chunk streams and never re-verifies the outgoing request. +- The coding-agent prompt renders identity, persona with the interpolated model, then fs/bash/web guidance through one assembly path. +- Fork and fresh subagent descriptions reflect whether the provider inherits completed conversation turns; the tool appears, disappears, and is reworded with provider lifecycle changes. +- Unknown, valueless, malformed, or unbalanced variable references name the section and throw; duplicate section, variable, and tool registrations also throw. +- Snapshot replay is prompt-independent: it keys recorded chunk streams by turn and step without comparing the outgoing request. ## Consequences diff --git a/packages/subagent/subagent-subprocess/src/index.ts b/packages/subagent/subagent-subprocess/src/index.ts index bd792e7f23..3831d2bb6a 100644 --- a/packages/subagent/subagent-subprocess/src/index.ts +++ b/packages/subagent/subagent-subprocess/src/index.ts @@ -1,19 +1,8 @@ /** - * Shared machinery for OUT-OF-PROCESS subagent backends — providers that spawn - * an external agent as a child process and must keep the parent deployment's - * credentials out of it, tear it down to quiescence, and isolate it from the - * host user's on-disk CLI state. The pieces: credential-shaped env scrubbing - * ({@link buildChildEnv}), spawn-failure capture ({@link spawnFailure}), - * bounded child-exit waits inside the stdin-EOF → SIGTERM → SIGKILL dispose - * ladder ({@link disposeChildProcess}), and the per-run isolated config dir - * ({@link createIsolatedConfigDir}). - * - * This package owns no provider and registers nothing; it is a pure library - * the out-of-process backend packages depend on (the `subagent-inprocess` - * shape, for the process boundary). Every tunable — the ladder's grace - * periods, a pinned config dir — is a PARAMETER here: defaults belong in each - * consuming plugin's Config, per the no-hardcoded-tunables rule. - * + * Shared machinery for OUT-OF-PROCESS subagent backends — providers that spawn an external + * agent as a child process and must keep the parent deployment's credentials out of it, tear + * it down to quiescence, and isolate it from the host user's on-disk CLI state. This package + * registers no provider; consuming plugins own and validate every timing or path default. * @module @deepseek-ai/dsh-subagent-subprocess */ @@ -50,11 +39,8 @@ export function buildChildEnv(extra: Record): NodeJS.ProcessEnv } /** - * Capture the child's spawn-level failure as a promise the run's result path - * can race. A spawn failure (e.g. `ENOENT` for a bad command) is emitted as an - * `error` EVENT, not a thrown exception — and without a listener Node treats - * it as an unhandled error and crashes the parent process. Call this in the - * SAME TICK as `spawn()`, so no window exists for the event to fire unheard. + * Capture the child's spawn-level `error` event as a promise. Call in the same tick as + * `spawn()`; otherwise an early event can be unhandled and crash the parent. * @param child - the just-spawned child process. * @returns a promise that RESOLVES (never rejects) with the child's first * `error` event; for a child that spawns cleanly it never settles. @@ -123,15 +109,8 @@ export interface DisposeLadderGraces { } /** - * Tear a child process down to QUIESCENCE: resolves only once the child has - * actually exited (or was already gone), never merely after requesting it. - * Three-tier escalation — - * - * 1. stdin EOF (when stdin is piped), then wait `disposeEofGraceMs`: a - * cooperative child quiesces on its own, its teardown and flushes intact; - * 2. `SIGTERM`, then wait `disposeGraceMs`; - * 3. `SIGKILL`, then await the (now-certain) exit — a child that ignores EOF - * and traps `SIGTERM` must not wedge dispose forever. + * Tear a child process down to quiescence, resolving only after exit: close stdin and allow + * cooperative flush, then send `SIGTERM`, then `SIGKILL` and await the forced exit. * * @param child - the child process to tear down. * @param graces - the two grace periods, from the consuming plugin's Config. @@ -139,10 +118,7 @@ export interface DisposeLadderGraces { export async function disposeChildProcess(child: ChildProcess, graces: DisposeLadderGraces): Promise { // Already gone: nothing to reap. if (child.exitCode !== null || child.signalCode !== null) return - // 1. Graceful: end the request stream (stdin EOF) and let the child quiesce - // on its own. Sending SIGTERM in the same tick (or too soon) would - // default-terminate a cooperative child mid-flush, orphaning its nested - // work. A child spawned without a stdin pipe skips straight to the wait. + // 1. Close stdin and allow cooperative teardown and durable-state flush. child.stdin?.end() if (await exitsWithin(child, graces.disposeEofGraceMs)) return // 2. SIGTERM, escalating if the child still does not exit within the grace. @@ -171,16 +147,9 @@ export interface IsolatedConfigDir { } /** - * An isolated config dir for one child run, so the child's behavior is a - * function of deployment config alone — never of whatever `~/.claude` / - * `~/.codex`-style state happens to exist on the host machine. Two modes: - * - * - no `pinnedPath` (the default): creates a FRESH private (0700) `mkdtemp` - * dir under the OS temp root; {@link IsolatedConfigDir.remove} deletes it - * best-effort; - * - `pinnedPath` set (a deployment deliberately sharing state across runs): - * the pinned path is returned as-is — never created, never removed — the - * deployment owns that directory's lifecycle. + * An isolated config dir for one child run, independent of host CLI state. Without + * `pinnedPath`, creates a private temp directory and removes it best-effort; a pinned directory + * is returned unchanged and remains deployment-owned. * * @param prefix - the `mkdtemp` name prefix for a fresh dir (e.g. * `dsh-subagent-codex-`); ignored when `pinnedPath` is set. @@ -207,10 +176,8 @@ export async function createIsolatedConfigDir(prefix: string, pinnedPath?: strin try { await rm(path, { recursive: true, force: true }) } catch { - // Best-effort by contract: swallows rm failures (EACCES/EBUSY-style — - // e.g. the dead child left an unreadable entry behind). The dir lives - // under the OS temp root, which reclaims it; failing dispose over - // cleanup would be worse than a leftover temp dir. + // Best-effort by contract: swallows rm failures (EACCES/EBUSY-style — e.g. the dead + // child left an unreadable entry behind). } }, } diff --git a/packages/subagent/tool-subagent/src/index.ts b/packages/subagent/tool-subagent/src/index.ts index ff54bc109a..59821147af 100644 --- a/packages/subagent/tool-subagent/src/index.ts +++ b/packages/subagent/tool-subagent/src/index.ts @@ -1,34 +1,11 @@ /** - * The model-facing `subagent` tool: delegate a task to a child agent and return - * its final output. Pure schema + lifecycle shaping — every transport concern - * lives behind the `ctx.subagents` provider registry - * (`@deepseek-ai/dsh-subagent`), so an in-process, ACP, or future A2A backend - * swaps in without touching what the model sees. - * - * Provider selection is config, not model-facing: this plugin is bound to - * EXACTLY ONE provider name (`Config.provider`). To expose more than one - * transport, load the plugin more than once, each bound to a different provider - * — 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 conversation-history - * descriptor ({@link SubagentProvider.inheritsParentContext}): a - * fresh-conversation provider (spawn, ACP) gets the standalone-prompt wording, - * while a seeded-conversation provider (fork) tells the model the child already - * sees the conversation's completed turns. This descriptor says nothing about - * Cordis scope, services, tools, or authority. 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 - * owned child agent/session is torn down on every path (success, error, abort) - * and never leaks as a live idle child. A non-`completed` stop reason maps to an - * `isError` tool result (by throwing) rather than returning partial output as - * success. + * Model-facing delegation tool bound by configuration to one provider; transport selection is not + * exposed in its `{ description, prompt }` schema. Provider lifecycle controls registration and + * re-derives conversation-history wording after reload, so load order is irrelevant. * + * Execution synchronously awaits the child result and always disposes the run. Non-completed stop + * reasons become error results, while transport details remain behind `ctx.subagents`. Load this + * plugin more than once to expose multiple configured providers. * @module @deepseek-ai/dsh-tool-subagent */ @@ -105,16 +82,8 @@ export const Config: z = z.object({ model: z.string(), }).default(undefined as unknown as { model: string }), persona: z.string(), - // A schemastery object materializes {} (with [] for nested arrays) when the - // key is omitted — for toolFilter that would mean an EMPTY ALLOW-LIST, i.e. - // deny-everything, silently. Force the omitted key to stay absent (the same - // shape discipline as SystemPrompt's toolOrder); the cast is needed because - // .default() expects the object type. - // The NESTED arrays get the same treatment as the object itself: a partial - // filter ({deny: […]}) must not materialize allow: [] beside it — an empty - // allow-list means deny-EVERYTHING, so the materialized default would turn - // a deny-one config into deny-all. An EXPLICIT allow: [] (grant-only - // children) survives, since only the omitted key defaults to undefined. + // Schemastery otherwise materializes omitted objects and nested arrays as `{ allow: [] }`, which + // silently means deny all. Preserve omission while retaining an explicit empty allow-list. toolFilter: z.object({ allow: z.array(z.string()).default(undefined as unknown as string[]), deny: z.array(z.string()).default(undefined as unknown as string[]), @@ -290,7 +259,7 @@ export function apply(ctx: Context, config: Config): void { if (present !== undefined) { mount(present) } else { - // Not an error: the backend's fiber may simply activate after this one. + // Not an error: the backend's fiber may 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`)