From a9cb70d89685a6e5fa6dc0fb093ec2ae7987747b Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sun, 12 Jul 2026 05:13:17 +0800 Subject: [PATCH] fix(scope): harden final ownership boundaries --- docs/config-catalog.md | 6 +- docs/cordis-catalog/events.md | 46 ++- docs/cordis-catalog/services.md | 16 +- docs/core-data-structures/core.md | 2 + docs/core-data-structures/scope.md | 31 ++ docs/core-data-structures/system-prompt.md | 37 +++ docs/event-producer-consumer.md | 33 +- ...-18-agent-lifecycle-and-ownership-seams.md | 4 +- .../2026-07-08-agent-scope-contexts.md | 123 ++++--- .../cordis/tool-cordis/src/api-catalog.ts | 20 +- packages/core/agent-loop/README.md | 4 +- packages/core/agent-loop/src/agent.ts | 84 +++-- packages/core/agent-loop/src/index.ts | 117 ++++--- packages/core/agent-loop/tests/agent.spec.ts | 51 ++- .../agent-loop/tests/scope-lifecycle.spec.ts | 56 ++++ packages/core/agent/README.md | 6 +- packages/core/agent/src/dispatch.ts | 33 +- packages/core/agent/src/index.ts | 153 ++++++++- packages/core/agent/src/types.ts | 5 +- packages/core/agent/tests/agent.spec.ts | 171 +++++++++- packages/core/scope/README.md | 2 +- packages/core/scope/src/index.ts | 156 +++++++-- packages/core/scope/tests/scope.spec.ts | 161 ++++++++- packages/core/session/README.md | 15 +- packages/core/session/src/index.ts | 246 ++++++++++++-- packages/core/session/tests/scoped.spec.ts | 17 + packages/core/session/tests/session.spec.ts | 183 +++++++++- packages/core/system-prompt/README.md | 8 +- packages/core/system-prompt/src/index.ts | 143 ++++++-- .../system-prompt/tests/system-prompt.spec.ts | 130 ++++++++ packages/core/tools/README.md | 4 +- packages/core/tools/src/index.ts | 33 +- packages/core/tools/tests/tools.spec.ts | 91 ++++- .../subagent/subagent-inprocess/README.md | 4 +- .../subagent/subagent-inprocess/src/index.ts | 57 ++-- .../tests/subagent-inprocess.spec.ts | 32 ++ packages/subagent/subagent-spawn/README.md | 2 +- .../tests/subagent-spawn.spec.ts | 31 +- packages/subagent/subagent/README.md | 8 +- packages/subagent/subagent/src/index.ts | 313 +++++++++++++----- .../subagent/subagent/tests/service.spec.ts | 306 ++++++++++++++++- packages/support/invariants/src/index.ts | 1 + packages/ui/acp/src/index.ts | 2 +- packages/ui/acp/tests/dispose.spec.ts | 8 +- packages/ui/user-approval/README.md | 4 +- packages/ui/user-approval/src/index.ts | 158 ++++++--- .../ui/user-approval/tests/approval.spec.ts | 161 +++++++++ .../workflow/workflow-workerthread/README.md | 2 +- .../workflow-workerthread/src/host.ts | 27 +- .../tests/workflow-workerthread.spec.ts | 35 ++ scripts/gen-doc-graphs.ts | 7 + scripts/type-equiv.manifest.json | 8 + 52 files changed, 2839 insertions(+), 514 deletions(-) create mode 100644 docs/core-data-structures/scope.md create mode 100644 docs/core-data-structures/system-prompt.md diff --git a/docs/config-catalog.md b/docs/config-catalog.md index d7b7be2d88..0a1c0289ae 100644 --- a/docs/config-catalog.md +++ b/docs/config-catalog.md @@ -144,7 +144,7 @@ export interface Config { Depends on: [`AgentId`](../packages/core/agent/src/index.ts) · [`AgentOptions`](../packages/core/agent/src/index.ts) · [`SessionId`](../packages/core/session/src/index.ts) -Source: [`packages/core/agent-loop/src/index.ts:37`](../packages/core/agent-loop/src/index.ts) +Source: [`packages/core/agent-loop/src/index.ts:44`](../packages/core/agent-loop/src/index.ts) ## `@deepseek-ai/dsh-bash-local` @@ -790,7 +790,7 @@ export interface Config { } ``` -Source: [`packages/core/system-prompt/src/index.ts:265`](../packages/core/system-prompt/src/index.ts) +Source: [`packages/core/system-prompt/src/index.ts:315`](../packages/core/system-prompt/src/index.ts) ## `@deepseek-ai/dsh-tool-cordis` @@ -1001,7 +1001,7 @@ export interface Config { export type ApprovalPolicy = 'ask' | 'never' ``` -Source: [`packages/ui/user-approval/src/index.ts:268`](../packages/ui/user-approval/src/index.ts) +Source: [`packages/ui/user-approval/src/index.ts:281`](../packages/ui/user-approval/src/index.ts) ## `@deepseek-ai/dsh-web` diff --git a/docs/cordis-catalog/events.md b/docs/cordis-catalog/events.md index f5a0533842..cc450a782e 100644 --- a/docs/cordis-catalog/events.md +++ b/docs/cordis-catalog/events.md @@ -15,7 +15,7 @@ Dispatch modes: **emit** (fire-and-forget), **waterfall** (each listener gets `n ### `agent/created` — emit -An agent's fully composed scoped world was published in the AgentRegistry. Its session is already live in the session store, but concrete factories may keep driving verbs locked until the subsequent `agent/session-start` boundary; that event is the first supported place to inject or queue work during startup. +An agent's fully composed scoped world was published in the AgentRegistry. Its session is already live in the session store, but concrete factories may keep driving verbs locked until the subsequent `agent/session-start` boundary; that event is the first supported place to inject or queue work during startup. A synchronous listener throw vetoes publication and rollback emits the matching disposal edges; returned-promise rejection is observed and logged but cannot retroactively veto this synchronous boundary. ```ts cordis-catalog 'agent/created'(this: Scoped, agent: Agent): void @@ -23,7 +23,7 @@ An agent's fully composed scoped world was published in the AgentRegistry. Its s Types: [Agent](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:300`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:303`](../../packages/core/agent/src/types.ts) ### `agent/disposed` — emit @@ -35,7 +35,7 @@ An agent was removed from the registry after its driver and any in-flight turn r Types: [Agent](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:314`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:317`](../../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:587`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:590`](../../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) · [Message](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:419`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:422`](../../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:437`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:440`](../../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:342`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:345`](../../packages/core/agent/src/types.ts) ### `agent/request` — waterfall @@ -97,7 +97,7 @@ Waterfall: shape the step's call configuration — model switching, sampling ove Types: [Agent](../core-data-structures/core.md) · [LlmCallConfig](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:466`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:469`](../../packages/core/agent/src/types.ts) ### `agent/session-prefix` — waterfall @@ -113,7 +113,7 @@ The seed is a frozen empty list; a contributing listener returns a NEW array — Types: [Agent](../core-data-structures/core.md) · [Message](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:518`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:521`](../../packages/core/agent/src/types.ts) ### `agent/session-start` — emit @@ -125,7 +125,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:362`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:365`](../../packages/core/agent/src/types.ts) ### `agent/status` — emit @@ -137,7 +137,7 @@ Agent status changed (`idle` ⇄ `running`, or → `disposed`). Drive lifecycle Types: [Agent](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:328`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:331`](../../packages/core/agent/src/types.ts) ### `agent/step-result` — waterfall @@ -149,7 +149,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:533`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:536`](../../packages/core/agent/src/types.ts) ### `agent/turn-continuation` — waterfall @@ -161,7 +161,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:551`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:554`](../../packages/core/agent/src/types.ts) ### `agent/turn-stop` — serial @@ -173,7 +173,7 @@ Serial terminal-stop checkpoint after the ordinary `agent/turn-continuation` wat Types: [Agent](../core-data-structures/core.md) -Source: [`packages/core/agent/src/types.ts:570`](../../packages/core/agent/src/types.ts) +Source: [`packages/core/agent/src/types.ts:573`](../../packages/core/agent/src/types.ts) ## `approval/*` @@ -245,13 +245,23 @@ Source: [`packages/llm/llm/src/index.ts:39`](../../packages/llm/llm/src/index.ts ### `session/created` — emit -A session was created in the store. Scope-filtered dispatch (`@deepseek-ai/dsh-scope`): the carrier is the session's owner scope, captured when the session was ENTERED (an agent's session is entered through `agent.ctx`, so its events dispatch in that agent's scope; a bare `sessions.create()` from a plain plugin dispatches subject-less). A listener registered through `agent.ctx` hears only that agent's sessions; a plain plugin listener hears every session. +A session was created in the store. A synchronous listener throw vetoes publication and rollback emits the matching `session/disposed` edge; returned-promise rejection is observed and logged but cannot retroactively veto this synchronous boundary. Scope-filtered dispatch (`@deepseek-ai/dsh-scope`): the carrier is the session's owner scope, captured when the session was ENTERED (an agent's session is entered through `agent.ctx`, so its events dispatch in that agent's scope; a bare `sessions.create()` from a plain plugin dispatches subject-less). A listener registered through `agent.ctx` hears only that agent's sessions; a plain plugin listener hears every session. ```ts cordis-catalog 'session/created'(this: Scoped, session: Session): void ``` -Source: [`packages/core/session/src/index.ts:47`](../../packages/core/session/src/index.ts) +Source: [`packages/core/session/src/index.ts:50`](../../packages/core/session/src/index.ts) + +### `session/disposed` — emit + +A previously announced session left the store. Emitted exactly once on normal detach or publication rollback, and never for a prepared/entered session whose `session/created` announcement did not begin. Listener failures (including returned-promise rejections) are logged and contained per listener so teardown always reaches quiescence. Scope-filtered dispatch uses the same owner carrier captured at entry; agent-scoped listeners hear only their own session's teardown. + +```ts cordis-catalog +'session/disposed'(this: Scoped, session: Session): void +``` + +Source: [`packages/core/session/src/index.ts:62`](../../packages/core/session/src/index.ts) ### `session/event` — emit @@ -263,7 +273,7 @@ An event was appended to a session log (sync, fire-and-forget). This is the per- Types: [SessionEvent](../core-data-structures/core.md) -Source: [`packages/core/session/src/index.ts:61`](../../packages/core/session/src/index.ts) +Source: [`packages/core/session/src/index.ts:76`](../../packages/core/session/src/index.ts) ### `session/flush` — parallel @@ -273,7 +283,7 @@ Awaited durability checkpoint. The agent loop awaits `ctx.sessions.flush(session 'session/flush'(this: Scoped, session: Session): Promise | void ``` -Source: [`packages/core/session/src/index.ts:79`](../../packages/core/session/src/index.ts) +Source: [`packages/core/session/src/index.ts:94`](../../packages/core/session/src/index.ts) ## `skill/*` diff --git a/docs/cordis-catalog/services.md b/docs/cordis-catalog/services.md index 903c368b7c..b7caa6f842 100644 --- a/docs/cordis-catalog/services.md +++ b/docs/cordis-catalog/services.md @@ -21,18 +21,19 @@ async createAgent(options: CreateAgentOptions): Promise async resume(options: ResumeAgentOptions): Promise ``` -Source: [`packages/core/agent-loop/src/index.ts:71`](../../packages/core/agent-loop/src/index.ts) +Source: [`packages/core/agent-loop/src/index.ts:78`](../../packages/core/agent-loop/src/index.ts) ## `ctx.agents` — `AgentRegistry` Agent registry (`ctx.agents`): tracks live agents so UI, hook, and orchestrator plugins can find them without depending on the concrete loop package. Agent *creation* is provided by whichever plugin implements the AgentFactory (`@deepseek-ai/dsh-agent-loop`), registered via setFactory. ```ts cordis-catalog +reserve(id: AgentId): AgentRegistrationReservation setFactory(factory: AgentFactory): () => Promise | void async create(options: CreateAgentOptions): Promise async resume(options: ResumeAgentOptions): Promise register(agent: Agent): () => Promise | void -enter(agent: Agent): () => void +enter(agent: Agent, reservation?: AgentRegistrationReservation): () => void announce(agent: Agent): void get(id: AgentId): Agent | undefined list(): Agent[] @@ -40,7 +41,7 @@ list(): Agent[] Types: [Agent](../core-data-structures/core.md) -Source: [`packages/core/agent/src/index.ts:174`](../../packages/core/agent/src/index.ts) +Source: [`packages/core/agent/src/index.ts:202`](../../packages/core/agent/src/index.ts) ## `ctx.approval` — `ApprovalService` @@ -54,7 +55,7 @@ async request(req: ApprovalRequest): Promise Types: [ApprovalOutcome](../core-data-structures/approval.md) · [ApprovalRequest](../core-data-structures/approval.md) -Source: [`packages/ui/user-approval/src/index.ts:292`](../../packages/ui/user-approval/src/index.ts) +Source: [`packages/ui/user-approval/src/index.ts:305`](../../packages/ui/user-approval/src/index.ts) ## `ctx.bash` — `BashExecutor` (abstract seam) @@ -210,9 +211,10 @@ In-memory session store (`ctx.sessions`). Persistence is intentionally not implemented here — persistence plugins subscribe to `session/event` and flush on `session/flush` / dispose. ```ts cordis-catalog +reserve(id: SessionId): SessionRegistrationReservation create(id?: SessionId, options?: CreateSessionOptions): Session prepare(id?: SessionId, options?: CreateSessionOptions): Session -enter(session: Session): () => void +enter(session: Session, reservation?: SessionRegistrationReservation): () => void announce(session: Session): void async flush(session: Session): Promise get(id: SessionId): Session | undefined @@ -220,7 +222,7 @@ list(): Session[] fork(source: SessionForkSource, boundary?: number, childSessionId?: SessionId): Session ``` -Source: [`packages/core/session/src/index.ts:608`](../../packages/core/session/src/index.ts) +Source: [`packages/core/session/src/index.ts:663`](../../packages/core/session/src/index.ts) ## `ctx.skills` — `SkillService` @@ -260,7 +262,7 @@ protect(protection: PromptProtection): () => Promise | void async assemble(context: AssembleContext = {}): Promise ``` -Source: [`packages/core/system-prompt/src/index.ts:380`](../../packages/core/system-prompt/src/index.ts) +Source: [`packages/core/system-prompt/src/index.ts:430`](../../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 b601c2ae29..76ec27e744 100644 --- a/docs/core-data-structures/core.md +++ b/docs/core-data-structures/core.md @@ -16,8 +16,10 @@ Everything else is documented on a **sub-page**, not here. The rule that draws t | Sub-page | Owns | |---|---| | [llm-streaming.md](llm-streaming.md) | the `StreamChunk` wire protocol + adapter contract, `BlockAssembler`, the `LlmAdapter` seam | +| [scope.md](scope.md) | scoped registration identity, dispatch carriers, and the owned `Scope` context | | [session.md](session.md) | the full `SessionEventMap` variant catalog, `TurnTrigger`/`TurnEndReason`, `deriveMessages()`, the turn-enclosure invariant | | [persistence.md](persistence.md) | the durability seam: `SessionPersistence`, JSONL + SQLite backends, `session/flush`, crash recovery, `SessionHeader` | +| [system-prompt.md](system-prompt.md) | per-assembly context, tool-provider results, and canonical contribution protection | | [tools.md](tools.md) | `ToolDefinition` full fields, the schema DSL, `ToolExecution`/`ToolResult`, tool-presentation UI types, and the guarded execution pipeline | | [user-interaction.md](user-interaction.md) | the UI-backed human question/answer seam: `AskUserQuestionRequest`, answer/options vocabulary, provider API, error taxonomy | | [approval.md](approval.md) | the one-shot user-approval seam: `ApprovalRequest`, `ApprovalOutcome`, per-session policy, audit and answerer contracts | diff --git a/docs/core-data-structures/scope.md b/docs/core-data-structures/scope.md new file mode 100644 index 0000000000..d2a2fc47d4 --- /dev/null +++ b/docs/core-data-structures/scope.md @@ -0,0 +1,31 @@ +# Scoped Registration + +The [scope package](../../packages/core/scope) supplies the identity and carrier vocabulary that makes one registration context mean both per-agent visibility and shared lifetime ownership. It is a library primitive rather than a Cordis service; the [agent-scope RFC](../rfc/implemented/architecture/2026-07-08-agent-scope-contexts.md) owns the design rationale, while the package [README](../../packages/core/scope/README.md) owns the callable API and filtering semantics. + +Source: [`packages/core/scope/src/index.ts`](../../packages/core/scope/src/index.ts). + +## Identity and dispatch carrier + +`ScopeKey` is an opaque object identity. The shipped loop uses the live `Agent` object as its own key, but the primitive never inspects the object. + +```ts type-equiv +type ScopeKey = object +``` + +`Scoped` is the compile-time brand on the proxy returned by `scopeTarget(base, key)`. Scope-filtered event declarations require this carrier as their `this` type, preventing an ordinary subject object from type-checking as the dispatch carrier. + +```ts type-equiv +type Scoped = T & { readonly [ScopedBrand]: 'dsh.scope.carrier' } +``` + +## Owned registration context + +`Scope` pairs the tagged registration context with two teardown surfaces. `rawDispose` preserves the exact Cordis disposer identity needed by an ordered composite effect; `dispose()` is the public shared quiescence boundary for direct and racing callers. + +```ts type-equiv +interface Scope { + ctx: Context + rawDispose: () => Promise | void + dispose(): Promise +} +``` diff --git a/docs/core-data-structures/system-prompt.md b/docs/core-data-structures/system-prompt.md new file mode 100644 index 0000000000..0406ddb1a3 --- /dev/null +++ b/docs/core-data-structures/system-prompt.md @@ -0,0 +1,37 @@ +# System Prompt Assembly + +The [system-prompt package](../../packages/core/system-prompt) owns the data exchanged between prompt contributors and one assembly call. The package [README](../../packages/core/system-prompt/README.md) documents registration, ordering, scoping, and rendering behavior; this page pins the literal cross-package shapes that plugins implement or pass. + +Source: [`packages/core/system-prompt/src/index.ts`](../../packages/core/system-prompt/src/index.ts). + +## Assembly context + +`AssembleContext` identifies the scope layer one assembly resolves. It is merge-extensible: `dsh-agent` adds the optional live `agent` field, and `assembleContextFor(agent)` sets that field and `scope` together. + +```ts type-equiv +interface AssembleContext { + scope?: ScopeKey +} +``` + +## Tool-provider result + +`ToolProviderResult.schemas` is the model-visible set for the current assembly. `knownNames` is the provider's pre-restriction name universe used to distinguish a configured-name typo from a known tool that is deliberately hidden in this scope. + +```ts type-equiv +interface ToolProviderResult { + schemas: ToolSchema[] + knownNames?: readonly string[] +} +``` + +## Canonical contribution protection + +`PromptProtection` names section and tool contributions whose canonical registry output remains authoritative after the assembly waterfall. Either field may be omitted, but a registration with no names is rejected. + +```ts type-equiv +interface PromptProtection { + sections?: readonly string[] + tools?: readonly string[] +} +``` diff --git a/docs/event-producer-consumer.md b/docs/event-producer-consumer.md index 5a0c547e70..7b26f43b12 100644 --- a/docs/event-producer-consumer.md +++ b/docs/event-producer-consumer.md @@ -7,27 +7,28 @@ 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:300`](../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:314`](../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:587`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`emit`) | - | -| `agent/pre-step` | `serial` | [`packages/core/agent/src/types.ts:419`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`serial`) | [`compact-basic`](../packages/compact/compact-basic), [`user-approval`](../packages/ui/user-approval) | -| `agent/prompt-submit` | `waterfall` | [`packages/core/agent/src/types.ts:437`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`waterfall`) | [`acp`](../packages/ui/acp), [`hooks-claude`](../packages/hooks/hooks-claude), [`hooks-codex`](../packages/hooks/hooks-codex), [`repeat-tool-guard`](../packages/guard/repeat-tool-guard) | -| `agent/queued` | `emit` | [`packages/core/agent/src/types.ts:342`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`emit`) | - | -| `agent/request` | `waterfall` | [`packages/core/agent/src/types.ts:466`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`waterfall`) | - | -| `agent/session-prefix` | `waterfall` | [`packages/core/agent/src/types.ts:518`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`waterfall`) | [`tool-skill`](../packages/skill/tool-skill) | -| `agent/session-start` | `emit` | [`packages/core/agent/src/types.ts:362`](../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), [`invariants`](../packages/support/invariants) | -| `agent/status` | `emit` | [`packages/core/agent/src/types.ts:328`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`emit`) | [`acp`](../packages/ui/acp), [`invariants`](../packages/support/invariants), [`repeat-tool-guard`](../packages/guard/repeat-tool-guard), [`stdio-agent`](../packages/ui/stdio-agent) | -| `agent/step-result` | `waterfall` | [`packages/core/agent/src/types.ts:533`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`waterfall`) | - | -| `agent/turn-continuation` | `waterfall` | [`packages/core/agent/src/types.ts:551`](../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/turn-stop` | `serial` | [`packages/core/agent/src/types.ts:570`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`strictSerial (serial)`) | [`subagent-inprocess`](../packages/subagent/subagent-inprocess) | +| `agent/created` | `emit` | [`packages/core/agent/src/types.ts:303`](../packages/core/agent/src/types.ts) | [`agent`](../packages/core/agent) (`events.dispatch`) | [`stdio-agent`](../packages/ui/stdio-agent) | +| `agent/disposed` | `emit` | [`packages/core/agent/src/types.ts:317`](../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:590`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`emit`) | - | +| `agent/pre-step` | `serial` | [`packages/core/agent/src/types.ts:422`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`serial`) | [`compact-basic`](../packages/compact/compact-basic), [`user-approval`](../packages/ui/user-approval) | +| `agent/prompt-submit` | `waterfall` | [`packages/core/agent/src/types.ts:440`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`waterfall`) | [`acp`](../packages/ui/acp), [`hooks-claude`](../packages/hooks/hooks-claude), [`hooks-codex`](../packages/hooks/hooks-codex), [`repeat-tool-guard`](../packages/guard/repeat-tool-guard) | +| `agent/queued` | `emit` | [`packages/core/agent/src/types.ts:345`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`emit`) | - | +| `agent/request` | `waterfall` | [`packages/core/agent/src/types.ts:469`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`waterfall`) | - | +| `agent/session-prefix` | `waterfall` | [`packages/core/agent/src/types.ts:521`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`waterfall`) | [`tool-skill`](../packages/skill/tool-skill) | +| `agent/session-start` | `emit` | [`packages/core/agent/src/types.ts:365`](../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), [`invariants`](../packages/support/invariants) | +| `agent/status` | `emit` | [`packages/core/agent/src/types.ts:331`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`emit`) | [`acp`](../packages/ui/acp), [`invariants`](../packages/support/invariants), [`repeat-tool-guard`](../packages/guard/repeat-tool-guard), [`stdio-agent`](../packages/ui/stdio-agent) | +| `agent/step-result` | `waterfall` | [`packages/core/agent/src/types.ts:536`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`waterfall`) | - | +| `agent/turn-continuation` | `waterfall` | [`packages/core/agent/src/types.ts:554`](../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/turn-stop` | `serial` | [`packages/core/agent/src/types.ts:573`](../packages/core/agent/src/types.ts) | [`agent-loop`](../packages/core/agent-loop) (`strictSerial (serial)`) | [`subagent-inprocess`](../packages/subagent/subagent-inprocess) | | `approval/request` | `waterfall` | [`packages/ui/user-approval/src/index.ts:72`](../packages/ui/user-approval/src/index.ts) | [`user-approval`](../packages/ui/user-approval) (`waterfall`) | [`acp`](../packages/ui/acp) | | `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) | | `llm/stream` | `waterfall` | [`packages/llm/llm/src/index.ts:39`](../packages/llm/llm/src/index.ts) | [`llm`](../packages/llm/llm) (`waterfall`) | [`invariants`](../packages/support/invariants), [`llm-replay`](../packages/support/llm-replay) | -| `session/created` | `emit` | [`packages/core/session/src/index.ts:47`](../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:61`](../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:79`](../packages/core/session/src/index.ts) | [`session`](../packages/core/session) (`parallel`) | [`session-persistence`](../packages/session-persistence/session-persistence) | +| `session/created` | `emit` | [`packages/core/session/src/index.ts:50`](../packages/core/session/src/index.ts) | [`session`](../packages/core/session) (`events.dispatch`) | [`invariants`](../packages/support/invariants), [`session-persistence`](../packages/session-persistence/session-persistence) | +| `session/disposed` | `emit` | [`packages/core/session/src/index.ts:62`](../packages/core/session/src/index.ts) | [`session`](../packages/core/session) (`events.dispatch`) | - | +| `session/event` | `emit` | [`packages/core/session/src/index.ts:76`](../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:94`](../packages/core/session/src/index.ts) | [`session`](../packages/core/session) (`parallel`) | [`session-persistence`](../packages/session-persistence/session-persistence) | | `skill/provider-added` | `emit` | [`packages/skill/skill/src/index.ts:132`](../packages/skill/skill/src/index.ts) | [`skill`](../packages/skill/skill) (`emit`) | - | | `skill/provider-removed` | `emit` | [`packages/skill/skill/src/index.ts:138`](../packages/skill/skill/src/index.ts) | [`skill`](../packages/skill/skill) (`emit`) | - | | `subagent/end` | `emit` | [`packages/subagent/subagent/src/index.ts:115`](../packages/subagent/subagent/src/index.ts) | [`subagent`](../packages/subagent/subagent) (`events.dispatch`) | [`hooks-claude`](../packages/hooks/hooks-claude) | diff --git a/docs/rfc/implemented/architecture/2026-06-18-agent-lifecycle-and-ownership-seams.md b/docs/rfc/implemented/architecture/2026-06-18-agent-lifecycle-and-ownership-seams.md index 2b1c198812..8e1acd1638 100644 --- a/docs/rfc/implemented/architecture/2026-06-18-agent-lifecycle-and-ownership-seams.md +++ b/docs/rfc/implemented/architecture/2026-06-18-agent-lifecycle-and-ownership-seams.md @@ -18,7 +18,7 @@ A new `cancel()` verb on the `Agent` interface — the single public stop primit `ctx.agents.create`/`resume` (and the `AgentFactory` interface) return `AgentHandle = { agent: Agent; dispose(): Promise }`. The disposer is a **capability** — only the holder can tear down exactly this agent: stop its loop, `await` the loop's exit (true quiescence, not just the `disposed` status flip), unregister it, and remove its session from the store. `ctx.agents.get(id)` still returns a bare `Agent`. Config-created agents stay owned by the `AgentLoop` fiber (the handle is discarded). ACP holds each session's disposer in its `SessionRecord` and runs it on disconnect/teardown, so a bare client disconnect leaves no registered agent and no session-store entry — even when `session/load` races teardown (the just-resumed handle is disposed before the closed-guard throw). -**Teardown ORDER is load-bearing for durability**, and the implementation folds the session lifecycle into the agent's SINGLE composite cordis effect (`SessionStore.prepare`/`enter`/`announce`, replacing a sibling-effect split). A fiber unload disposes sibling effects concurrently (`Promise.all`), which would race the session's `onAppend` detach against the loop's closing `session/flush` and drop the closing `turn/end`; inside one effect the disposers run as an ordered LIFO chain (loop stopped + `await agent.done` BEFORE the session detaches), so the loop's final flush is captured on BOTH the handle's `dispose()` and a fiber unload. The register disposer's `agent/disposed` emit is contained (a throwing listener must not reject the chain and skip the later session detach). +**Teardown ORDER is load-bearing for durability**, and the implementation folds the session lifecycle into the agent's SINGLE composite cordis effect (`SessionStore.prepare`/`enter`/`announce`, replacing a sibling-effect split). A fiber unload disposes sibling effects concurrently (`Promise.all`), which would race detaching the session store's private append observer against the loop's closing `session/flush` and drop the closing `turn/end`; inside one effect the disposers run as an ordered LIFO chain (loop stopped + `await agent.done` BEFORE the session detaches), so the loop's final flush is captured on BOTH the handle's `dispose()` and a fiber unload. The contained `agent/disposed` and `session/disposed` notifications cannot reject the chain or skip later teardown. ### 3. Bash owner token in the seam @@ -40,7 +40,7 @@ The bash owner-token comparison relies on `session.header.id` being unique among ## Alternatives considered - **A public `BashTask.owner` field** instead of the `BashExecutor.ownerOf(id)` seam — rejected: one read path, no redundant API. -- **Sibling cordis effects for the agent's session lifecycle** — rejected: a fiber unload disposes sibling effects concurrently (`Promise.all`), racing the session's `onAppend` detach against the loop's closing `session/flush`; the single composite effect's ordered LIFO chain is what captures the closing `turn/end` on both disposal paths. +- **Sibling cordis effects for the agent's session lifecycle** — rejected: a fiber unload disposes sibling effects concurrently (`Promise.all`), racing the store-owned append observer's detach against the loop's closing `session/flush`; the single composite effect's ordered LIFO chain is what captures the closing `turn/end` on both disposal paths. - **A separate step-only `abort()` beside `cancel()`** — shipped originally, then removed as unused; `cancel()` is the single public stop primitive ([the public-stop-surface RFC](../simplification/2026-06-20-public-agent-stop-surface.md)). ## Consequences diff --git a/docs/rfc/implemented/architecture/2026-07-08-agent-scope-contexts.md b/docs/rfc/implemented/architecture/2026-07-08-agent-scope-contexts.md index 981603cb19..436a208be8 100644 --- a/docs/rfc/implemented/architecture/2026-07-08-agent-scope-contexts.md +++ b/docs/rfc/implemented/architecture/2026-07-08-agent-scope-contexts.md @@ -11,7 +11,7 @@ This is a composition problem, not an application-isolation problem. Starting a | Surface | What varies by agent | Failure when it is only global | |---|---|---| | Tools | Available capabilities, a child-only tool, or a scoped replacement for one implementation | The model receives excess authority, or a child-specific tool leaks into every prompt | -| Prompt state | Persona, instructions, variables, and Code Mode SDK declarations | Every agent receives the same instructions or runtime facts | +| Prompt state | Persona, instructions, variables, and [Code Mode](../feature/2026-06-15-code-mode.md) SDK declarations | Every agent receives the same instructions or runtime facts | | Live policy | Hooks, execution guards, result observers, and continuation rules | A listener intended for one agent can alter another agent's work | | Lifetime | Cleanup when the agent fails, is cancelled, is disposed, or loses its owner | Registrations outlive the agent or disappear before its final work settles | @@ -19,23 +19,30 @@ Two consistency requirements make the problem deeper than filtering a list. Firs Second, some rules are invariants rather than cooperative extensions. An ordinary middleware listener may replace a prompt assembly, turn an allow into a deny, rewrite a result, force another model step, or short-circuit listeners registered after it. Structured output therefore cannot rely on being “first” or “last” in an extensible listener chain; the owning service needs a final boundary for rules that later listeners must not undo. -The subagent API makes both needs concrete. Two concurrent children can request different personas, tool filters, and output schemas. Those requests are honest only when each child receives an independently owned view and when its terminal-output protocol survives unrelated plugins. +Third, accepting a value must transfer ownership of the exact value that was checked. TypeScript `readonly` annotations disappear at runtime, callers and providers may expose stateful accessors, and a validation pass followed by a clone reads mutable input twice. Identity fields, schemas, session data, requests, and results therefore need runtime boundaries that capture each caller-owned field once, materialize data once, and expose only owner-controlled snapshots. Otherwise the checked, executed, logged, and observed views can diverge even when scope resolution itself is correct. + +The subagent API makes these requirements concrete. Two concurrent children can request different personas, tool filters, and output schemas. Those requests are honest only when each child receives an independently owned view and when its terminal-output protocol survives unrelated plugins. ## Decision Each live agent owns a registration context named `agent.ctx`, and services expose narrow owner-final policy boundaries where ordinary middleware ordering is not strong enough. Together these choices make one agent's world composable with normal plugin APIs while keeping authority, observation, and cleanup aligned. -The design has three parts: +The design has four parts: | Part | Rule | Purpose | |---|---|---| | Registration scope | A registration through a plain plugin context is global; the same registration through `agent.ctx` belongs to that agent | Reuse existing APIs for per-agent tools, prompt state, and listeners | | Lifecycle transaction | Create and resume await scoped setup while the agent and session are unpublished, then publish them in an ordered rollback-covered sequence | No observer sees a partially composed agent, and every failure path owns cleanup | | Owner-final policy | Prompt protection, tool guards, final tool-result observation, and terminal turn stopping run at service-owned boundaries | Invariants do not depend on listener registration order | +| Boundary ownership | Services capture fixed fields once, materialize lossless-JSON data once, and publish owner-controlled views | Validation, execution, persistence, and telemetry cannot observe different values from one call | + +Three domain terms recur below. A **Session** is one agent run's append-only event log, from which model history and durable replay are derived. **Lossless JSON** means JSON primitives plus dense arrays and plain objects that can be copied without changing meaning; the boundary rejects sparse arrays, cycles, exotic prototypes, non-finite numbers, negative zero, `undefined`, `bigint`, functions, and symbols instead of coercing or erasing them. **Code Mode** presents the model with a generated software-development-kit interface and a reserved `run_code` transport, rather than advertising every end-capability as a native tool. + +Ownership stays with the component that can enforce each fact. The scope package owns scope tags and carrier construction; each registry owns acceptance snapshots and resolution; the agent factory owns identity reservation, setup, and publication; the session owns accepted history; the tool and subagent services own their pipeline records; and the workflow host owns cancellation of the runs it started. A caller never validates a value that another component later rereads from the caller's mutable object. The scope is flat. An agent resolves the deployment-global layer plus its own layer; a child does not inherit registrations from its parent's scope. Parent/child lineage remains explicit session data, and parent-owned disposal links lifetimes without silently inheriting authority. -The implementation lives primarily in [`dsh-scope`](../../../../packages/core/scope/README.md), [`dsh-agent`](../../../../packages/core/agent/README.md), [`dsh-system-prompt`](../../../../packages/core/system-prompt/README.md), and [`dsh-tools`](../../../../packages/core/tools/README.md). The [generated Cordis event catalog](../../../cordis-catalog/events.md) is the exhaustive event-signature reference; this RFC explains why the contracts have their current shape. +The core implementation lives in [`dsh-scope`](../../../../packages/core/scope/README.md), [`dsh-agent`](../../../../packages/core/agent/README.md), [`dsh-agent-loop`](../../../../packages/core/agent-loop/README.md), [`dsh-session`](../../../../packages/core/session/README.md), [`dsh-system-prompt`](../../../../packages/core/system-prompt/README.md), and [`dsh-tools`](../../../../packages/core/tools/README.md). The composition example spans [`dsh-subagent`](../../../../packages/subagent/subagent/README.md), [`dsh-subagent-inprocess`](../../../../packages/subagent/subagent-inprocess/README.md), and [`dsh-workflow-workerthread`](../../../../packages/workflow/workflow-workerthread/README.md). The [generated Cordis event catalog](../../../cordis-catalog/events.md) is the exhaustive event-signature reference; this RFC explains why the contracts have their current shape. ## Background: the small Cordis vocabulary used here @@ -149,7 +156,11 @@ Calling a service through `agent.ctx` does not implicitly make every later read The tool view must not change because a caller kept the object it passed to `register()` or received a definition from `get()` or `visible()`. Registration therefore creates the stored identity once; future changes happen through explicit unregister/register effects. -Tool parameters cross the model and log boundary, so the registry materializes them with `snapshotJsonValue`: one recursive traversal reads each property once, rejects anything outside lossless JSON, and constructs the detached value that is actually stored. A check followed by `structuredClone` is not equivalent—a getter could return plain JSON to the check and a class instance to the clone, which would erase its prototype and silently accept different data. The first-party `defineTool()` helper closes the earlier authoring boundary with the same primitive: it reads every top-level option once, materializes the `SchemaSpec`, and derives an independent wire schema plus all later execute/presentation validation from that accepted snapshot. Without that split, mutating an author-owned spec after definition could make the model call a schema that the tool no longer accepts. Registration then reads every top-level definition field exactly once, validates, binds, and stores only those captured values; a stateful `parameters` or callback accessor therefore cannot make the checked definition differ from the executable one. It snapshots the scalar fields, binds each callback once to the original definition as its method receiver, and deep-freezes the stored record. Replacing `definition.execute` after registration therefore has no effect, while a callback can still deliberately read mutable state from its closure or original receiver. `get()` and `visible()` return the frozen stored definitions; `schemas()` returns detached schema projections. +Tool parameters cross the model and log boundary, so the registry materializes them with `snapshotJsonValue`: one recursive traversal reads each property once, rejects anything outside lossless JSON, and constructs the detached value that is actually stored. A check followed by `structuredClone` is not equivalent—a getter could return plain JSON to the check and a class instance to the clone, which would erase its prototype and silently accept different data. + +The first-party `defineTool()` helper closes the authoring boundary with the same primitive. It reads every top-level option once, materializes the `SchemaSpec`, and derives an independent wire schema plus all later execute and presentation validation from that accepted snapshot. Without that split, mutating an author-owned spec after definition could make the model call a schema that the tool no longer accepts. + +Registration then reads every top-level definition field exactly once, validates, binds, and stores only those captured values; a stateful `parameters` or callback accessor therefore cannot make the checked definition differ from the executable one. It snapshots the scalar fields, binds each callback once to the original definition as its method receiver, and deep-freezes the stored record. Replacing `definition.execute` after registration therefore has no effect, while a callback can still deliberately read mutable state from its closure or original receiver. `get()` and `visible()` return the frozen stored definitions; `schemas()` returns detached schema projections. ```text defineTool(options): @@ -208,7 +219,7 @@ The operation being described determines the key; callers cannot attach an unrel | `approval/request` | `ApprovalRequest.agent` | | `tools/pre-execute`, `tools/execute`, `tools/post-execute`, `tools/result` | `ToolExecution.agent`, or no key for an agent-less call | | `system-prompt/assemble` | `AssembleContext.scope` | -| `session/created`, `session/event`, `session/flush` | The owner scope captured when the session enters the store | +| `session/created`, `session/disposed`, `session/event`, `session/flush` | The owner scope captured when the session enters the store | | `subagent/start`, `subagent/end` | The delegating parent agent | Approval requests cross an asynchronous answer boundary, so the service snapshots the accepted record synchronously. It preserves the exact agent and abort-signal identities but copies the scalar fields, captures the agent's session once, and uses that one snapshot for `approval/asked`, scoped dispatch, cancellation, policy, and `approval/decided`. Mutating the caller-owned record after `request()` returns therefore cannot split the audit pair or redirect the question to another agent's listeners. @@ -234,7 +245,7 @@ The real helpers fuse values that must agree. `agentEvents(context, agent)` uses Function-style listeners receive the carrier as `this`, and agent event APIs allow them to call subject methods. The carrier is therefore a JavaScript proxy that reads and writes through to the real subject and binds methods to it. -Binding matters for classes with JavaScript private fields: a method called with the proxy itself as receiver would fail the runtime private-field identity check. The proxy preserves the subject's existing event filter and JavaScript object invariants, but it is intentionally not identity-equal to the subject; event arguments carry the real object whenever identity matters. +Binding matters for classes with JavaScript private fields: a method called with the proxy itself as receiver would fail the runtime private-field identity check. The carrier therefore uses a dedicated surrogate proxy target with its own immutable composed-filter slot, while ordinary property access, writes, own-key visibility, methods, invocation, and construction delegate to the real subject; callable carriers also preserve whether the subject is constructable. For non-overlay properties owned by the subject, descriptor queries preserve values and flags except that `configurable` is reported as `true`, which is the Proxy-safe way for an extensible surrogate to expose a property it does not itself own. A filter property pinned on the subject before, during, or after construction cannot trigger the proxy invariant that would otherwise force delivery to use the subject's raw filter and silently drop scope isolation. The carrier is intentionally not identity-equal to the subject; event arguments carry the real object whenever identity matters. `Scoped` is a TypeScript-only marker that requires this carrier at declared scoped dispatch sites. It improves authoring but adds no runtime security, so runtime marks and development invariants check the same contract for JavaScript, casts, and hand-written dispatches. @@ -246,11 +257,13 @@ An agent's scope, session, registry entry, and driver form one owned transaction Programmatic create and resume reserve both the agent ID and session ID before work that can await. Create prepares a fresh or seeded session; resume first loads and reconstructs the persisted session. Both paths then construct the agent, mint `agent.ctx`, and install the complete teardown skeleton before awaiting setup. -The factory captures IDs and the setup callback and clones caller-owned agent options before the first asynchronous boundary. Seed events and session metadata take a stricter route: pre-cloning either could erase a class or exotic prototype before the session validator saw it, so the factory reads each reference once and hands it synchronously to `SessionStore.prepare`. That boundary rejects exotic shells, reads each accepted metadata field once, and recursively validates and copies every seed value in one pass. One-pass materialization matters because `validate(value); structuredClone(value); validate(clone)` still reads a getter twice, and the clone can erase the prototype of a class instance returned only on the second read. The accepted metadata becomes a detached, deep-frozen `SessionHeader` whose id must equal the session id. Resume applies the same rule after persistence loading by capturing `createdAt`, `cwd`, `parentSession`, and `seedLength` once before reconstruction. A caller or stateful backend therefore cannot move the transaction away from the identities it reserved, change persistence routing or lineage after publication, or sanitize invalid data into acceptance. +The factory first captures the requested IDs, setup callback, and caller-owned agent options. Seed events and session metadata take a stricter route than a preliminary clone: cloning can erase an exotic prototype before validation sees it, so the factory reads each reference once and hands it synchronously to the session store's reservation-bound prepare operation. That boundary rejects exotic shells, reads accepted metadata fields once, and recursively materializes each seed record in one pass. Resume applies the same rule to persistence output by capturing the loaded header fields once before reconstruction. The transaction therefore cannot move to different identities, storage routing, or lineage after an asynchronous boundary. -The session log also closes the ownership boundary after acceptance. Seed and append paths share exact runtime surface-metadata checks: surface events require either `'append'` or an exact replace record with non-negative safe-integer bounds, provenance is an array of non-negative safe integers, and non-surface events reject both fields. Accepted events are deep-frozen, and `session.events` returns a cached frozen array snapshot rather than the mutable internal array. A later append invalidates the cache and publishes a new snapshot; any earlier snapshot remains unchanged. This preserves append-only behavior even for JavaScript callers that cast away TypeScript's readonly view or retain an event reference received from `append` or `session/event`. +Before setup can observe the new objects, their ownership-bearing public properties become stable runtime data slots rather than TypeScript-only `readonly` promises. The concrete agent pins its ID, accepted options, and session; the factory binds its scope context exactly once. The session pins its ID and detached, deep-frozen header. Registry detach closures likewise close over their accepted map keys instead of rereading public properties during teardown. A JavaScript assignment or stateful accessor therefore cannot split registry lookup, dispatch, persistence, and the driver into different identities. -Reservations prevent two concurrent transactions from composing different unpublished agents under the same public identity. They remain held across persistence loading and setup and are released on every success or failure path. +The session owns the accepted log as described in [the session-immutability RFC](2026-06-11-dev-invariants-over-deep-readonly.md). Seed and append paths materialize lossless JSON once, validate both the event envelope and the metadata that places message-producing events into derived model history, and deep-freeze the exact accepted event. `session.events` returns a frozen snapshot that never grows later. The store keeps append notification and scope-carrier state in store-owned private tables instead of caller-writable `Session` fields, so outside JavaScript cannot suppress or redirect `session/event` dispatch. + +Reservations prevent two concurrent factory transactions from composing different unpublished objects under the same public identities. Each reservation belongs both to the factory transaction and to the Cordis fiber that requested it: explicit release covers every success or failure path, while owner-fiber disposal is the backstop for an abandoned handle during plugin unload or HMR. The agent registry and session store recognize their own reserved keys: setup code that calls public reserve, prepare, create, register, or bare enter APIs with the same IDs fails. The session capability can prepare exactly one object, and publication succeeds only when both stores receive the factory-held exact capabilities; the session store additionally checks that the capability owns that exact prepared session. This closes the otherwise possible path in which setup publishes a substitute object under an ID that the factory merely tracked in a separate pending set, without letting a vanished owner wedge the ID forever. Resume installs an owner-liveness sentinel before reserving IDs or starting persistence I/O, then races loading against owner disposal. If disposal wins, resume rejects and releases both reservations immediately; a backend promise that settles later cannot publish. After a successful load, the factory synchronously installs the full agent lifecycle before removing the sentinel, so ownership passes from load to setup without an unobserved disposal gap. @@ -260,18 +273,18 @@ The sentinel exists only for the interval in which no agent lifecycle can exist resume(request): snapshot request ids, options, and setup callback sentinel = owner.effect(onDispose => signal ownerDisposed) - reserve(agentId, sessionId) + reservations = reserve agentId in AgentRegistry and sessionId in SessionStore try: persisted = await firstOf(persistence.load(sessionId), ownerDisposed) - session = reconstruct(persisted) + session = reservations.session.prepare(reconstruct persisted data) # This call installs the full lifecycle before its first await. - starting = startOwned(agentId, session, options, setup) + starting = startOwned(agentId, session, options, reservations, setup) disarm and dispose sentinel return await starting finally: - release both ids + release both reservation capabilities settle the sentinel transaction ``` @@ -326,8 +339,8 @@ The implementation keeps publication synchronous and leaves rollback to the surr ```text publish(world): - world.detachSession = world.agent.ctx.sessions.enter(world.session) - world.detachAgent = app.agents.enter(world.agent) + world.detachSession = world.agent.ctx.sessions.enter(world.session, world.sessionReservation) + world.detachAgent = app.agents.enter(world.agent, world.agentReservation) app.sessions.announce(world.session) app.agents.announce(world.agent) world.driver.enableDrivingVerbs() @@ -337,7 +350,11 @@ publish(world): Both registry entries exist before the first creation listener runs, and setup-installed listeners receive both announcements. Driving opens immediately before `agent/session-start`, so that event remains the first supported place for a listener to inject or queue startup work. -The sequence is not described as atomic because observers run between its steps. If a `session/created` or `agent/created` listener throws, the transaction rolls the registry entries and scope back, but effects already performed by an earlier listener cannot be retracted. An announced agent is paired with its disposal notification during rollback. `agent/session-start` is a non-vetoing notification: listener failures are logged and contained so the loop still starts. +The sequence is not described as atomic because observers run between its steps. If a `session/created` or `agent/created` listener throws synchronously, the transaction rolls the registry entries and scope back, but effects already performed by an earlier listener cannot be retracted. Each store therefore marks its announcement as begun before invoking creation listeners and rejects a repeat or reentrant announcement before dispatch. Rollback emits `session/disposed` or `agent/disposed` exactly once for every corresponding creation announcement that began, including a partial emit in which an early listener observed creation before a later listener threw. An object entered but never announced has no disposal notification because no observer was told it existed. + +Creation notification preserves that synchronous veto while also defending against JavaScript's asynchronous callback shape. A listener may return a promise even though the event type returns `void`; the dispatcher does not await it because publication has no asynchronous gap, but it observes and logs a later rejection. Such a rejection is too late to roll back, does not become unhandled, and does not starve the listeners invoked after that callback. + +The disposal notifications and `agent/session-start` are deliberately non-vetoing. Their dispatchers invoke every listener synchronously and independently; they log and contain both a synchronous throw and a rejection from a returned promise. Returned promises are observed for failure but not awaited, so an asynchronous notification listener cannot delay rollback or teardown, veto driver startup, or starve a later listener. ### Teardown stops work before revoking its world @@ -346,14 +363,16 @@ Every owner path uses the same reverse order: stop the loop and await its actual ```text disposeOwnedAgent(world): await world.stopDriver() # waits for loop exit and all agent-started flushes - world.detachAgent() # emits agent/disposed when announced - world.detachSession() + world.detachAgent() # leaves registry; emits agent/disposed if announced + world.detachSession() # stops event feed, leaves store; emits session/disposed if announced await world.scope.dispose() ``` The actual Cordis generator yields these disposers in reverse so its last-in-first-out teardown executes in the order shown. -`agent/disposed` means the driver is quiescent and the agent has left the registry; session detachment and scope unwind may still be completing after that notification. `AgentHandle.dispose()` is memoized so concurrent owners await the same full transaction, and `Scope.dispose()` provides the corresponding shared boundary for direct scope disposal and raw-disposer races. +`agent/disposed` means the driver is quiescent and the agent has left the registry; the session is still live during that notification. `session/disposed` follows after append notification has been detached and the session has left its store. The scope is still live when each disposal listener is selected and invoked, although returned asynchronous work is observed rather than awaited. Both notifications use the same scope key and delivery rule as their creation partners and occur exactly once only when those creation announcements began. + +`AgentHandle.dispose()` is memoized so concurrent owners await the same full transaction, and `Scope.dispose()` provides the corresponding shared boundary for direct scope disposal and raw-disposer races. Parent-owned subagents use explicit ownership rather than capability inheritance. The driver creates one run-owner fiber under `parent.ctx` and invokes the child factory through that fiber, so lifecycle ownership exists before setup or publication begins; disposing a parent reaches its descendants even if a delegating tool never reaches its own `finally`. The child still receives a newly minted scope and resolves only global plus child-scoped capabilities. @@ -371,6 +390,8 @@ A global section protection also reserves the registry name against scoped shado Restoration is intentionally not a whole-assembly reset. The service first removes protected names from the waterfall result, then reinserts protected canonical entries in their canonical order immediately before the first surviving later unprotected canonical neighbor, or at the end when no such neighbor survives. Unprotected entries keep the ordering and definitions chosen by the waterfall. This anchor rule preserves the protected contribution's meaningful local placement without claiming that protection restores every global relative position after arbitrary listener reordering. +Only the restoration inputs are detached before dispatch: the canonical section array when section protection is active and the canonical tool array when tool protection is active. The waterfall receives the original mutable assembly, not a clone, and variables and other merge-extensible fields remain entirely under ordinary waterfall semantics. + ```text registerSection(input, scope): stored = copy(input.name, input.order, input.text) @@ -379,12 +400,14 @@ registerSection(input, scope): sectionLayer(scope).add(stored) assemble(context): - canonical = assemble registries for context.scope - transformed = await systemPromptAssembleWaterfall(clone(canonical)) + assembly = assemble registries for context.scope + canonicalSections = active section protection ? clone(assembly.sections) : absent + canonicalTools = active tool protection ? clone(assembly.tools) : absent + transformed = await systemPromptAssembleWaterfall(assembly) - for each protected name: + for each protected name in the corresponding canonical array: remove every transformed entry with that name - if canonical contains the name: + if the canonical array contains the name: if a later unprotected canonical neighbor survived: insert the canonical entry before that neighbor else: @@ -399,9 +422,11 @@ Code Mode uses global protection for the `tools:sdk` section and reserved `run_c ### Tool executions have stable identity -`ctx.tools.execute(input)` accepts a caller-owned `ToolExecutionInput` and snapshots it into a distinct pipeline-owned `ToolExecution`. It captures the required `callId`/`name` correlation identity, then reads every other top-level caller field once before using it, so parent-token validation, scope routing, policy, dispatch, and final observation all see one coherent identity; those captured optional fields construct the normalized error shell if a later accessor or argument validation fails. The registry materializes `arguments` in one lossless-JSON traversal and deep-freezes the result, so policy and dispatch receive exactly the value that passed validation. A cloneable but mutable exotic such as `Map` or a class instance is rejected before policy rather than smuggled through an apparently frozen wrapper. Invalid input still produces one normalized final error notification; a throwing `callId` or `name` accessor is outside that guarantee because no trustworthy result correlation exists. +`ctx.tools.execute(input)` accepts a caller-owned `ToolExecutionInput` and snapshots it into a distinct pipeline-owned `ToolExecution`. It reads `callId` and `name` once and requires each value to be a string before treating the pair as trustworthy correlation identity. A throwing accessor or non-string value rejects before `tools/result`, because even an error result could not carry a valid identity. After that boundary, the registry reads every other top-level caller field once, and any later accessor or validation failure becomes one normalized final error notification built from the already accepted strings and captured optional fields. -The registry assigns each pipeline trip a frozen, property-free `ToolExecutionToken`; callers cannot choose that token. The execution's `token`, `callId`, `name`, `agent`, optional opaque `parent` token, and detached `arguments` are non-writable and non-configurable from the first policy listener onward. `signal` is the only operational field: an around-dispatch wrapper may add, replace, or remove it, and the registry freezes the complete execution before outcome observation. +The registry materializes `arguments` in one lossless-JSON traversal and deep-freezes the result, so parent-token validation, scope routing, policy, dispatch, and final observation receive exactly the value that passed validation. A cloneable but mutable exotic such as `Map` or a class instance is rejected before policy rather than smuggled through an apparently frozen wrapper. + +The registry assigns each pipeline trip a frozen, property-free `ToolExecutionToken`; callers cannot choose that token. The execution is identity-stable, not fully immutable, while the pipeline runs: its `token`, `callId`, `name`, `agent`, optional opaque `parent` token, and detached `arguments` are non-writable and non-configurable from the first policy listener onward. `signal` is the only operational field; an around-dispatch wrapper may add, replace, or remove it. The registry freezes the complete execution before outcome observation. Stable identity prevents a listener from changing which capability or scope was authorized after policy ran. It also gives commit-style observers a safe `WeakMap` key even when an adapter reuses a model call ID. @@ -411,14 +436,19 @@ The input-to-execution conversion is intentionally one-way: ```text prepareExecution(input): - accepted = read callId, name, arguments, agent, parent, signal exactly once + callId = read input.callId exactly once + name = read input.name exactly once + require callId and name are strings + # A failure above rejects: no trustworthy correlation identity exists. + + accepted = read arguments, agent, parent, and signal exactly once require accepted.parent is absent or a registry-minted token detachedArguments = snapshotLosslessJson(accepted.arguments) execution = { token: new frozen property-free object, - callId: accepted.callId, - name: accepted.name, + callId, + name, arguments: deepFreeze(detachedArguments), agent: accepted.agent, parent: accepted.parent, @@ -447,8 +477,12 @@ The entire registry method reads like one authority ladder: ```text execute(input): + callId = read input.callId exactly once + name = read input.name exactly once + require callId and name are strings + try: - execution = prepareExecution(input) + execution = prepareExecutionFromTrustedIdentity(input, callId, name) catch invalidInput: execution = frozen identity shell with arguments = undefined result = errorResult(invalidInput) @@ -488,6 +522,8 @@ Waterfalls can transform only at their named stages. Guards can only deny, and t ### `agent/turn-stop` makes a composed continuation terminal +Steering is input injected into an already running turn for the next model step; ordinary queued prompts wait for a future turn. The loop normally preserves that distinction by moving leftover steering into another step while leaving the queued-prompt FIFO alone. + Ordinary continuation remains extensible. The loop computes a default, runs the `agent/turn-continuation` waterfall, records any force-continue reason as steering, and folds pending steering into the decision because steering normally demands another model step. The scoped serial `agent/turn-stop` checkpoint runs after that folding. Its strict serial helper consults listeners in order until one returns a non-`undefined` value; a listener returns `{ action: 'stop' }` or abstains with `undefined`. The dedicated helper exists because ordinary Cordis serial dispatch treats `null` and `false` as framework abstentions, while this public contract has exactly one abstention value. A stop is terminal, so later listeners and pending steering cannot restore continuation. A malformed result, including `null` or `false`, or a throwing policy closes the current turn with an error while leaving the driver available for later work. @@ -524,13 +560,17 @@ In-process subagents demonstrate how the scope, lifecycle, and final-policy piec ### Inputs and ownership are fixed before asynchronous creation -Provider registration first freezes an acceptance snapshot of the provider name, capability flags, parent-context descriptor, and `start` callback; the callback is bound to the original provider receiver so its intentional internal state stays live. Lookup, validation, model-facing wording, dispatch, lifecycle notifications, and HMR cleanup all use that snapshot. Mutating or reusing the caller's provider object later therefore cannot rename a live entry, change its advertised powers, replace its callback, or make its disposer delete the wrong key. +Provider registration first freezes an acceptance snapshot of the provider name, capability flags, parent-context descriptor, and `start` callback; the callback is bound to the original provider object so its intentional internal state stays live. Lookup, validation, model-facing wording, dispatch, lifecycle notifications, and hot-reload cleanup all use that snapshot. Mutating or reusing the caller's provider object later therefore cannot rename a live entry, change its advertised powers, replace its callback, or make its disposer delete the wrong key. Starting a run reads every top-level request field once before capability validation, then snapshots every accepted field before asynchronous owner setup. This order makes checked and delegated capabilities identical even for a JavaScript caller with stateful accessors. Fixed scalars are checked at the same boundary: `maxDepth` must be a non-negative safe integer and `persona` must be a string. The parent and abort signal are retained as identity capabilities but never reread from the mutable request record; tool filters, seed events, agent options, output schema, and prompt are detached through the one-pass lossless-JSON materializer. The exported in-process driver repeats this boundary for direct callers before it awaits run-owner activation, including taking one seed snapshot from which it derives both the child prefix and `seedLength`. Later caller mutation therefore cannot change lifecycle scope, configuration, the schema enforced by the capture tool, or the prompt eventually logged and sent. The driver first installs provider ownership. Only after that succeeds does it attach the request's abort listener and create one run-owner Cordis fiber under `parent.ctx`; an already-unloading provider therefore leaves neither a child nor an orphaned listener. The child factory runs through the owner fiber. Parent teardown, provider teardown, and manual run disposal all dispose this same node; moving it out of the active state synchronously prevents an unpublished setup from publishing afterward, while all three paths follow one quiescence promise. This structured ownership does not change the child's flat capability view. -The provider's run separates acceptance from publication with `started: Promise`, but the service does not return that caller-owned handle directly. It reads `id`, `started`, `result`, and every method once, binds methods to the original provider receiver, and returns a frozen service-owned wrapper. Its `result` promise captures `output`, optional `structured`, and `stopReason` once and resolves to one detached, deeply frozen lossless-JSON value shared by the caller and lifecycle telemetry; malformed provider data rejects as an infrastructure fault and produces contained `error` telemetry. For spawn and fork, the accepted `started` promise fulfills only after the child factory returns a published handle, so the service can emit `subagent/start` with `ctx.agents.get(id)` already live; it rejects when rollback prevents publication. The service observes the normalized result immediately but buffers its end payload until readiness, preserving start-before-end order without leaving an early rejection unhandled. Both lifecycle payloads are deeply frozen before contained per-listener dispatch, so one observer cannot corrupt the caller or a peer. A readiness rejection emits neither lifecycle event. The result driver awaits the same boundary before sending the child prompt. +The provider's run separates acceptance from publication with `started: Promise`, but the service does not expose that caller-owned handle directly. It captures `id`, `started`, `result`, and each method once, binds methods to the provider-owned run handle, and returns a frozen service-owned wrapper. Capturing `dispose` first also preserves a rollback capability if a later accessor or method check reveals a malformed handle. + +The wrapper's `result` promise captures `output`, optional `structured`, and `stopReason` once and resolves to one detached, deeply frozen lossless-JSON value shared by the caller and lifecycle telemetry. Malformed terminal data is an infrastructure fault; it rejects only after the service has started rollback of the provider attempt. The service observes the normalized result immediately, before waiting for readiness, so an early rejection is never temporarily unhandled. + +For spawn and fork, the accepted `started` promise fulfills only after the child factory returns a published handle. The service can then emit `subagent/start` with `ctx.agents.get(id)` already live and release any buffered terminal event; if readiness rejects, it emits neither start nor end. Lifecycle notification is fire-and-forget and non-vetoing: each listener receives the same deeply frozen payload, and synchronous throws or returned-promise rejections are logged and contained per listener without awaiting them. The child result driver awaits the same readiness boundary before sending the prompt. ```text startInProcessRun(providerContext, acceptedRequest): @@ -563,9 +603,10 @@ SubagentService.start(...): result: normalize once into detached, deeply frozen lossless JSON }) attach settlement handlers to serviceRun.result immediately - await serviceRun.started - emit subagent/start; later emit the buffered or eventual subagent/end - return serviceRun + attach handlers to serviceRun.started: + on fulfillment, emit subagent/start and then buffered or eventual subagent/end + on rejection, discard buffered lifecycle telemetry + return serviceRun immediately Workflow worker bridge after receiving returnedRun: register the run so cancellation can reach pre-publication work @@ -581,7 +622,11 @@ Before publishing the workflow's own result: only then settle the workflow result ``` -Every downstream protocol that announces a subagent must honor the same boundary. The workflow worker bridge therefore registers the returned run before waiting, observes and snapshots `result` immediately, sends `ChildStarted` only after `started` fulfills, and sends `ChildStartError` plus host-driven disposal when readiness rejects. Before the workflow result becomes observable, the host also drives both permitted cancellation channels—the shared abort signal and each registered run's explicit `cancel()`—because a fire-and-forget child still waiting on readiness has no worker-side handle that could relay cancellation. Provider cancel callbacks are contained independently so one broken implementation cannot prevent peers from receiving cancellation or wedge the workflow result. This keeps cancellation able to reach pending creation, prevents an early result rejection from going unhandled, ensures `workflow/agent-start` never names an unpublished child, and prevents a child from publishing after its workflow has ended. +Every downstream protocol that announces a subagent must honor the same boundary. The workflow worker bridge therefore registers the returned run before waiting, observes and snapshots `result` immediately, sends `ChildStarted` only after `started` fulfills, and sends `ChildStartError` plus host-driven disposal when readiness rejects. + +Cancellation before readiness is a publication decision, not merely a flag for later result mapping. The in-process run synchronously deactivates its owner fiber, so the agent factory's liveness check fails, `started` rejects, and neither the child session nor agent can publish. The run's result still settles as `aborted`. Before the workflow's own result becomes observable, its host likewise drives both permitted cancellation channels: it aborts the shared request signal and calls each registered run's `cancel()`, including runs still waiting on readiness. Provider cancel callbacks are contained independently so one broken implementation cannot prevent peers from receiving cancellation or wedge the workflow result. + +Together these rules prevent an early result rejection from going unhandled, ensure `workflow/agent-start` never names an unpublished child, and prevent a child from publishing after its workflow has ended. Parent teardown reaches `runOwner` by nesting; the provider and returned run handle reach the same node through their explicit disposers. @@ -607,7 +652,7 @@ The table describes the registry's named canonical contribution. An unrelated as ### Capture uses stage, final commit, monotonic denial, and terminal stop -The capture tool validates its arguments and stages the cloned value in a JavaScript `WeakMap` keyed by the immutable `ToolExecution`. This is an object-identity table whose key does not keep an abandoned execution alive. Validation failure becomes the ordinary `INVALID_ARGS` error that the model can correct within the turn. +The capture tool validates its arguments and stages the cloned value in a JavaScript `WeakMap` keyed by the identity-stable `ToolExecution`. This is an object-identity table whose key does not keep an abandoned execution alive. Validation failure becomes the ordinary `INVALID_ARGS` error that the model can correct within the turn. The scoped `tools/result` observer commits a direct native capture only when that exact execution's authoritative final result succeeds. A later call with a reused string call ID cannot reach the weak-keyed stage, and a post-execution block cannot promote it. @@ -744,9 +789,9 @@ The costs are concentrated in dispatch discipline, per-scope registry state, and - `run_code` is protected transport infrastructure rather than a filterable end capability, so a policy that must forbid programs denies execution at the tool-policy layer instead of removing the transport from a Code Mode prompt. - Prompt protection restores named canonical contributions and their anchor placement, not the entire assembly; unprotected output remains extensible, while a globally protected section name is deliberately unavailable for scoped shadowing. - Terminal turn stopping has authority to discard pending steering. That power is appropriate for owner-enforced terminal protocols and too strong for ordinary cooperative continuation policy. -- Programmatic `ctx.agents.create()` and `ctx.agents.resume()` are asynchronous because they await setup. The config-only `ctx.agentLoop.create()` path has no setup callback and remains synchronous. +- Programmatic `ctx.agents.create()` and `ctx.agents.resume()` are asynchronous because they await setup. The direct no-setup `ctx.agentLoop.create()` path, used by configuration and programmatic callers that already have complete options, remains synchronous. - Ordered composition requires both an exact raw scope disposer and a shared public quiescence promise; the dual surface reflects two distinct Cordis lifecycle requirements. ### Deliberate boundaries -The scope primitive is generic, but this decision applies it only where one agent needs a coherent registration view: tools, prompt state, scoped events, sessions, and in-process subagent composition. `agent.ctx` does not automatically scope every service call; filesystem policy, LLM interception, background subagent state, and future registries retain their existing seams until their own designs explicitly adopt the context rule. +The scope primitive is generic, but this decision applies it only where one agent needs a coherent registration view: tools, prompt state, scoped events, sessions, and in-process subagent composition. `agent.ctx` does not automatically scope every service call; filesystem policy, LLM interception, background subagent state, and other registries retain their existing seams until their own designs explicitly adopt the context rule. diff --git a/packages/cordis/tool-cordis/src/api-catalog.ts b/packages/cordis/tool-cordis/src/api-catalog.ts index aea7927571..af8e9905e4 100644 --- a/packages/cordis/tool-cordis/src/api-catalog.ts +++ b/packages/cordis/tool-cordis/src/api-catalog.ts @@ -65,11 +65,12 @@ export const SERVICE_API: readonly ServiceApiEntry[] = [ key: 'agents', summary: 'Agent registry (`ctx.agents`): tracks live agents so UI, hook, and orchestrator plugins can find them without depending on the concrete loop package.', methods: [ + 'reserve(id: AgentId): AgentRegistrationReservation', 'setFactory(factory: AgentFactory): () => Promise | void', 'async create(options: CreateAgentOptions): Promise', 'async resume(options: ResumeAgentOptions): Promise', 'register(agent: Agent): () => Promise | void', - 'enter(agent: Agent): () => void', + 'enter(agent: Agent, reservation?: AgentRegistrationReservation): () => void', 'announce(agent: Agent): void', 'get(id: AgentId): Agent | undefined', 'list(): Agent[]', @@ -155,9 +156,10 @@ export const SERVICE_API: readonly ServiceApiEntry[] = [ key: 'sessions', summary: 'In-memory session store (`ctx.sessions`).', methods: [ + 'reserve(id: SessionId): SessionRegistrationReservation', 'create(id?: SessionId, options?: CreateSessionOptions): Session', 'prepare(id?: SessionId, options?: CreateSessionOptions): Session', - 'enter(session: Session): () => void', + 'enter(session: Session, reservation?: SessionRegistrationReservation): () => void', 'announce(session: Session): void', 'async flush(session: Session): Promise', 'get(id: SessionId): Session | undefined', @@ -353,6 +355,12 @@ export const EVENT_API: readonly EventApiEntry[] = [ signature: '\'session/created\'(this: Scoped, session: Session): void', summary: 'A session was created in the store.', }, + { + name: 'session/disposed', + mode: 'emit', + signature: '\'session/disposed\'(this: Scoped, session: Session): void', + summary: 'A previously announced session left the store.', + }, { name: 'session/event', mode: 'emit', @@ -503,6 +511,10 @@ export const TYPE_API: readonly TypeApiEntry[] = [ name: 'AgentOptions', declaration: 'export interface AgentOptions {\n model?: string;\n}', }, + { + name: 'AgentRegistrationReservation', + declaration: 'export interface AgentRegistrationReservation {\n readonly id: AgentId;\n release(): void;\n}', + }, { name: 'AgentStatus', declaration: 'export type AgentStatus = \'idle\' | \'running\' | \'disposed\';', @@ -803,6 +815,10 @@ export const TYPE_API: readonly TypeApiEntry[] = [ name: 'SessionId', declaration: 'export type SessionId = Branded<\'SessionId\'>;', }, + { + name: 'SessionRegistrationReservation', + declaration: 'export interface SessionRegistrationReservation {\n readonly id: SessionId;\n prepare(options?: CreateSessionOptions): Session;\n release(): void;\n}', + }, { name: 'SkillCandidate', declaration: 'export interface SkillCandidate extends SkillSummary {\n rank: number;\n locator: unknown;\n path?: string;\n metadata?: Record;\n}', diff --git a/packages/core/agent-loop/README.md b/packages/core/agent-loop/README.md index ac1a0e079f..a8da612305 100644 --- a/packages/core/agent-loop/README.md +++ b/packages/core/agent-loop/README.md @@ -8,9 +8,9 @@ This is the only package in the harness that contains concrete loop logic. Every ### Public API -Lifecycle (scoped): programmatic creation and resume snapshot caller-owned identity/configuration data, reserve both IDs, mint `agent.ctx`, and install the ordered teardown skeleton before awaiting optional `setup`. A create hands one-read raw seed and metadata references synchronously to the session boundary, which rejects exotic shells and materializes accepted values in a single recursive pass; pre-cloning either value could incorrectly sanitize prototypes. Resume installs an owner-liveness sentinel before persistence load, captures each loaded metadata field once, then hands ownership directly to the full lifecycle. After setup resolves, the factory checks its lifecycle flag, owner-fiber state, and owning agent status around one microtask checkpoint so a same-turn Cordis unload wins before publication. Successful setup inserts both session and agent before announcing either, enables driving immediately before `agent/session-start`, then starts the loop. Setup calls to `send`/`steer`/`inject`/`cancel` reject structurally; load/setup rejection or owner unload publishes nothing. Teardown runs stop/drain (including outstanding idle-injection flushes) → unregister → detach session → unwind scope. All `agent/*` dispatches go through `agentEvents(ctx, agent)`; per-step assembly through `assembleContextFor(agent)`; the turn-end durability checkpoint through `ctx.sessions.flush(session)`. +Lifecycle (scoped): programmatic creation and resume snapshot caller-owned identity/configuration data, obtain registry/store-owned capabilities for both unpublished IDs, mint `agent.ctx`, and install the ordered teardown skeleton before awaiting optional `setup`. The capabilities reject competing `register`/`enter`/`prepare`/`create` calls, so setup cannot publish the factory objects or same-id replacements. A create hands one-read raw seed and metadata references synchronously to the session boundary, which rejects exotic shells and materializes accepted values in a single recursive pass; pre-cloning either value could incorrectly sanitize prototypes. Resume installs an owner-liveness sentinel before persistence load, captures each loaded metadata field once, then hands ownership directly to the full lifecycle. After setup resolves, the factory checks its lifecycle flag, owner-fiber state, and owning agent status around one microtask checkpoint so a same-turn Cordis unload wins before publication. Successful setup inserts both session and agent before announcing either, enables driving immediately before `agent/session-start`, then starts the loop. The concrete agent owns runtime-pinned `id`, frozen detached `options`, `session`, and `ctx` bindings. Load/setup rejection or owner unload publishes nothing; partial creation announcements are paired during rollback. Teardown runs stop/drain (including outstanding idle-injection flushes) → unregister → detach session → unwind scope. All non-vetoing `agent/*` notifications go through `agentEvents(ctx, agent)`, which contains sync/async listener failures per observer; per-step assembly goes through `assembleContextFor(agent)`; the turn-end durability checkpoint goes through `ctx.sessions.flush(session)`. -- `ctx.agentLoop.create(id: string, options?: AgentOptions, meta?: { cwd?: string }): ReactLoopAgent` — config-driven create: an agent on a fresh per-run session id `${id}-session-` with optional session metadata. Used for `cordis.yml`-configured agents. The per-run uuid avoids colliding with the on-disk log a prior run materialized once a durable persistence backend is loaded; each run is a new session (a deliberate demo simplification — a real resume-or-create policy is a TODO). Disposed with the calling fiber. +- `ctx.agentLoop.create(id: string, options?: AgentOptions, meta?: { cwd?: string }): ReactLoopAgent` — synchronous no-setup create, used directly by programs and by `cordis.yml`-configured agents. It creates a fresh per-run session id `${id}-session-` with optional metadata; the uuid avoids colliding with a prior durable log. Each call is a new session (a deliberate demo simplification — a real resume-or-create policy is a TODO). Disposed with the calling fiber. `AgentLoop` also implements the `AgentFactory` seam and registers itself via `ctx.agents.setFactory(this)`, so plugins create/resume agents through `ctx.agents` (the interface): diff --git a/packages/core/agent-loop/src/agent.ts b/packages/core/agent-loop/src/agent.ts index c4c35f34db..3234a47939 100644 --- a/packages/core/agent-loop/src/agent.ts +++ b/packages/core/agent-loop/src/agent.ts @@ -7,10 +7,10 @@ */ import type { Context } from 'cordis' -import { scopeTarget } from '@deepseek-ai/dsh-scope' -import type { Scoped } from '@deepseek-ai/dsh-scope' +import { agentEvents } from '@deepseek-ai/dsh-agent' import type { AgentId, AgentOptions, AgentStatus, SendOptions } from '@deepseek-ai/dsh-agent' import type { Agent } from '@deepseek-ai/dsh-agent' +import { deepFreeze } from '@deepseek-ai/dsh-llm' import type { ContentBlock, MessageSource } from '@deepseek-ai/dsh-llm' import type { Session } from '@deepseek-ai/dsh-session' import { Inbox } from './inbox.ts' @@ -65,6 +65,26 @@ export function prepareReactLoopAgent( } } +/** + * Install the concrete agent's scope context exactly once. Construction and + * scope minting are mutually referential (the scope key is the agent), so the + * factory performs this one post-construction binding before setup receives + * the unpublished agent. The runtime slot is non-writable/non-configurable; + * TypeScript `readonly` alone would still let JavaScript redirect later + * registrations to another context. + * @param agent - the unpublished concrete agent to bind. + * @param ctx - its fully extended agent scope context. + */ +export function bindReactLoopAgentContext(agent: ReactLoopAgent, ctx: Context): void { + if (Object.hasOwn(agent, 'ctx')) throw new Error(`agent "${agent.id}" context is already bound`) + Object.defineProperty(agent, 'ctx', { + value: ctx, + enumerable: true, + writable: false, + configurable: false, + }) +} + /** * The concrete {@link Agent} implementation owned by the agent-loop plugin. * @@ -84,18 +104,7 @@ export class ReactLoopAgent implements Agent { * context are mutually referential (the scope is keyed BY this agent), so * neither can exist strictly before the other. */ - ctx!: Context - - /** - * The dispatch carrier for this agent's own emits (`agent/status`, - * `agent/queued`, `agent/error`): keyed by the agent, base = the agent - * (listener `this` is the agent). Built lazily because it is self-referential. - */ - private get carrier(): Scoped { - return (this.#carrier ??= scopeTarget(this, this)) - } - - #carrier: Scoped | undefined + declare readonly ctx: Context private _status: AgentStatus = 'idle' private currentAbort: AbortController | undefined @@ -143,6 +152,16 @@ export class ReactLoopAgent implements Agent { public readonly options: AgentOptions, public readonly session: Session, ) { + const acceptedOptions = deepFreeze(structuredClone(options)) + // Pin the public ownership/identity bindings in the runtime object. A + // JavaScript caller can otherwise replace TS-readonly parameter properties + // after publication and split the registry, driver, session, and model + // configuration into different worlds. + Object.defineProperties(this, { + id: { value: id, enumerable: true, writable: false, configurable: false }, + options: { value: acceptedOptions, enumerable: true, writable: false, configurable: false }, + session: { value: session, enumerable: true, writable: false, configurable: false }, + }) const { promise, resolve } = Promise.withResolvers() this.disposed = promise this.resolveDisposed = resolve @@ -161,11 +180,7 @@ export class ReactLoopAgent implements Agent { // waiter (docs/defensive-patterns.md "contain callback exceptions" — a lifecycle await must // not hang on one bad listener). if (status !== 'running') this.settleIdleWaiters() - try { - this.loopCtx.emit(this.carrier, 'agent/status', this, status) - } catch (error: unknown) { - this.loopCtx.logger.warn(`agent "${this.id}": agent/status listener threw on ${status}: ${String(error)}`) - } + agentEvents(this.loopCtx, this).emit('agent/status', status) } /** @@ -194,7 +209,7 @@ export class ReactLoopAgent implements Agent { if (this._status === 'disposed') throw new Error(`agent "${this.id}" is disposed`) const source = this.resolveSource(options) this.#inbox.enqueue({ content, source }) - this.loopCtx.emit(this.carrier, 'agent/queued', this, content, { source, steering: false }) + agentEvents(this.loopCtx, this).emit('agent/queued', content, { source, steering: false }) } steer(content: ContentBlock[], options?: SendOptions): void { @@ -203,7 +218,7 @@ export class ReactLoopAgent implements Agent { if (this._status !== 'running') { this.send(content, options); return } const source = this.resolveSource(options) this.#inbox.steer({ content, source }) - this.loopCtx.emit(this.carrier, 'agent/queued', this, content, { source, steering: true }) + agentEvents(this.loopCtx, this).emit('agent/queued', content, { source, steering: true }) } inject(content: ContentBlock[], options?: SendOptions): void { @@ -269,14 +284,10 @@ export class ReactLoopAgent implements Agent { if (turnRecorded) { // Through the store's flush (the carrier owner), never a raw parallel. const flush = this.loopCtx.sessions.flush(this.session).catch((error: unknown) => { - const err = error instanceof Error ? error : new Error(String(error)) - this.loopCtx.logger.warn(`agent "${this.id}": flush after idle injection failed: ${err.message}`) - try { - this.loopCtx.emit(this.carrier, 'agent/error', this, turn, 0, err) - } catch { - // contained: the failure is already logged; a throwing agent/error - // listener must not escape this fire-and-forget catch. - } + const rendered = renderThrown(error) + const err = error instanceof Error ? error : new Error(rendered) + this.loopCtx.logger.warn(`agent "${this.id}": flush after idle injection failed: ${rendered}`) + agentEvents(this.loopCtx, this).emit('agent/error', turn, 0, err) }) this.pendingIdleFlushes.add(flush) // Attach the same retirement callback to both settlement arms so even a @@ -393,11 +404,7 @@ export class ReactLoopAgent implements Agent { // setStatus refuses transitions out of 'disposed', so emit directly — // 'disposed' is part of the agent/status contract. Guarded: a throwing // listener must not break the disposal chain. - try { - this.loopCtx.emit(this.carrier, 'agent/status', this, 'disposed') - } catch { - // listener error during disposal — nothing safe left to do with it - } + agentEvents(this.loopCtx, this).emit('agent/status', 'disposed') } // An unexpected driver rejection must not skip registry/session/scope // cleanup. The normal loop contains turn failures itself; allSettled is the @@ -414,3 +421,12 @@ export class ReactLoopAgent implements Agent { } } } + +/** Render an arbitrary thrown value without allowing coercion to throw again. */ +function renderThrown(value: unknown): string { + try { + return value instanceof Error ? value.message : String(value) + } catch { + return '' + } +} diff --git a/packages/core/agent-loop/src/index.ts b/packages/core/agent-loop/src/index.ts index 1e5c122fc5..902e1e2185 100644 --- a/packages/core/agent-loop/src/index.ts +++ b/packages/core/agent-loop/src/index.ts @@ -13,17 +13,24 @@ import z from 'schemastery' import { createScope } from '@deepseek-ai/dsh-scope' import type { Scope } from '@deepseek-ai/dsh-scope' import { agentEvents } from '@deepseek-ai/dsh-agent' -import type { AgentFactory, AgentHandle, AgentId, AgentOptions, CreateAgentOptions, ResumeAgentOptions, SessionStartSource } from '@deepseek-ai/dsh-agent' +import type { AgentFactory, AgentHandle, AgentId, AgentOptions, AgentRegistrationReservation, CreateAgentOptions, ResumeAgentOptions, SessionStartSource } from '@deepseek-ai/dsh-agent' import type {} from '@deepseek-ai/dsh-llm' import { SessionId, type SessionHeader } from '@deepseek-ai/dsh-session' -import type { Session } from '@deepseek-ai/dsh-session' +import type { Session, SessionRegistrationReservation } from '@deepseek-ai/dsh-session' import type {} from '@deepseek-ai/dsh-system-prompt' import type {} from '@deepseek-ai/dsh-tools' import type { SessionPersistence } from '@deepseek-ai/dsh-session-persistence' -import { prepareReactLoopAgent, ReactLoopAgent } from './agent.ts' +import { bindReactLoopAgentContext, prepareReactLoopAgent, ReactLoopAgent } from './agent.ts' export { ReactLoopAgent } from './agent.ts' +/** Both unpublished identity capabilities held by one factory transaction. */ +interface RegistrationReservations { + agent: AgentRegistrationReservation + session: SessionRegistrationReservation + release(): void +} + declare module 'cordis' { interface Context { agentLoop: AgentLoop @@ -71,10 +78,6 @@ export interface Config { export class AgentLoop extends Service implements AgentFactory { static inject = ['agents', 'sessions', 'llm', 'tools', 'systemPrompt'] - /** IDs held by unpublished async creation transactions. */ - private pendingAgentIds = new Set() - private pendingSessionIds = new Set() - // The schema validates plain strings (cordis.yml config values are untyped at // runtime); the {@link Config} TYPE declares the branded `id`/`resumeSessionId` // because the config format is the boundary where an id enters. The brand is a @@ -153,14 +156,19 @@ export class AgentLoop extends Service implements AgentFactory { * @returns the running agent, owned by the calling fiber (no handle). */ create(id: AgentId, options: AgentOptions = {}, meta: Pick = {}): ReactLoopAgent { - this.assertAgentIdFree(id) + const sessionId = SessionId(`${id}-session-${randomUUID()}`) + const reservations = this.reserve(id, sessionId) // Config/programmatic path: prepare the session and let start() fold its // lifecycle into the agent's composite effect (so a fiber unload tears the // session + agent down as one ordered chain, capturing the loop's closing // flush). The whole effect is owned by THIS fiber; no AgentHandle is needed. - const session = this.ctx.sessions.prepare(SessionId(`${id}-session-${randomUUID()}`), { meta }) - const { agent } = this.start(id, options, session, 'startup') - return agent + try { + const session = reservations.session.prepare({ meta }) + const { agent } = this.start(id, options, session, 'startup', reservations) + return agent + } finally { + reservations.release() + } } /** @@ -188,16 +196,16 @@ export class AgentLoop extends Service implements AgentFactory { const agentOptions = structuredClone(options.agentOptions ?? {}) const seed = options.seed const meta = options.meta - const release = this.reserve(agentId, sessionId) + const reservations = this.reserve(agentId, sessionId) try { - const session = this.ctx.sessions.prepare(sessionId, { + const session = reservations.session.prepare({ ...seed !== undefined ? { seed } : {}, ...meta !== undefined ? { meta } : {}, }) // A seeded (forked) create is still a fresh start, NOT a resume. - return await this.startOwned(agentId, agentOptions, session, 'startup', setup) + return await this.startOwned(agentId, agentOptions, session, 'startup', reservations, setup) } finally { - release() + reservations.release() } } @@ -273,7 +281,7 @@ export class AgentLoop extends Service implements AgentFactory { return transactionSettled }, `agentLoop.resumeLoad(${agentId})`) try { - const release = this.reserve(agentId, sessionId) + const reservations = this.reserve(agentId, sessionId) try { const loadTask = persistence.load(sessionId) const { meta, events } = await Promise.race([ @@ -292,7 +300,7 @@ export class AgentLoop extends Service implements AgentFactory { // An out-of-band direct registry/session insertion can still race this // service's reservation, so the public enter primitives re-check exact // liveness at publication. - const session = this.ctx.sessions.prepare(sessionId, { + const session = reservations.session.prepare({ seed: events, meta: { createdAt, @@ -305,12 +313,12 @@ export class AgentLoop extends Service implements AgentFactory { // effect before it reaches its first setup await. Only then disarm the // load sentinel: ownership passes directly from one effect to the other // with no disposal gap. - const starting = this.startOwned(agentId, agentOptions, session, 'resume', setup) + const starting = this.startOwned(agentId, agentOptions, session, 'resume', reservations, setup) observingOwner = false await disposeLoadSentinel() return await starting } finally { - release() + reservations.release() } } finally { try { @@ -327,29 +335,24 @@ export class AgentLoop extends Service implements AgentFactory { } } - /** - * Reject a duplicate agent id BEFORE the session is entered into the store, so - * a failed factory call never leaves an orphaned live session (and lazy - * persistence state) behind. `register()` enforces the same uniqueness, but - * only after the session has already entered the store. - */ - private assertAgentIdFree(id: AgentId): void { - if (this.ctx.agents.get(id) !== undefined || this.pendingAgentIds.has(id)) { - throw new Error(`agent "${id}" is already registered`) - } - } - - /** Reserve both public identities for one unpublished async transaction. */ - private reserve(agentId: AgentId, sessionId: SessionId): () => void { - this.assertAgentIdFree(agentId) - if (this.ctx.sessions.get(sessionId) !== undefined || this.pendingSessionIds.has(sessionId)) { - throw new Error(`session "${sessionId}" already exists`) - } - this.pendingAgentIds.add(agentId) - this.pendingSessionIds.add(sessionId) - return () => { - this.pendingAgentIds.delete(agentId) - this.pendingSessionIds.delete(sessionId) + /** Reserve both public identities in their owning registries. */ + private reserve(agentId: AgentId, sessionId: SessionId): RegistrationReservations { + const agent = this.ctx.agents.reserve(agentId) + try { + const session = this.ctx.sessions.reserve(sessionId) + return { + agent, + session, + release() { + // Both owner capabilities are independently idempotent, so the + // composite needs no second state machine of its own. + session.release() + agent.release() + }, + } + } catch (error: unknown) { + agent.release() + throw error } } @@ -361,7 +364,12 @@ export class AgentLoop extends Service implements AgentFactory { * `active`, unwinds the scope, and wins the race without any late Cordis * effect collection. */ - private prepareLifecycle(id: AgentId, options: AgentOptions, session: Session): { + private prepareLifecycle( + id: AgentId, + options: AgentOptions, + session: Session, + reservations: RegistrationReservations, + ): { agent: ReactLoopAgent active: () => boolean deactivated: Promise @@ -378,7 +386,7 @@ export class AgentLoop extends Service implements AgentFactory { const driver = prepareReactLoopAgent(this.ctx, id, options, session) const { agent } = driver const scope: Scope = createScope(this.ctx, agent) - agent.ctx = scope.ctx.extend({ agent }) + bindReactLoopAgentContext(agent, scope.ctx.extend({ agent })) let active = true let detachSession: (() => void) | undefined @@ -422,19 +430,15 @@ export class AgentLoop extends Service implements AgentFactory { const publish = (source: SessionStartSource): void => { // Publication is one synchronous, rollback-covered sequence. Setup has // already completed, so its scoped listeners observe both announcements. - detachSession = agent.ctx.sessions.enter(session) - detachAgent = this.ctx.agents.enter(agent) + detachSession = agent.ctx.sessions.enter(session, reservations.session) + detachAgent = this.ctx.agents.enter(agent, reservations.agent) this.ctx.sessions.announce(session) this.ctx.agents.announce(agent) // Setup is over and both entries are live. Open the driving surface just // before session-start so its listeners retain their supported ability to // inject/queue, while setup itself can never drive an unpublished agent. driver.enableDrive() - try { - agentEvents(this.ctx, agent).emit('agent/session-start', source) - } catch (error: unknown) { - this.ctx.logger.warn(`agent "${id}": agent/session-start listener threw: ${String(error)}`) - } + agentEvents(this.ctx, agent).emit('agent/session-start', source) stop = driver.startDriver() } @@ -453,9 +457,13 @@ export class AgentLoop extends Service implements AgentFactory { /** Publish a no-setup config agent synchronously. */ private start( - id: AgentId, options: AgentOptions, session: Session, source: SessionStartSource, + id: AgentId, + options: AgentOptions, + session: Session, + source: SessionStartSource, + reservations: RegistrationReservations, ): { agent: ReactLoopAgent; disposeAgent: () => Promise } { - const lifecycle = this.prepareLifecycle(id, options, session) + const lifecycle = this.prepareLifecycle(id, options, session, reservations) try { lifecycle.publish(source) return { agent: lifecycle.agent, disposeAgent: lifecycle.disposeAgent } @@ -485,9 +493,10 @@ export class AgentLoop extends Service implements AgentFactory { */ private async startOwned( id: AgentId, options: AgentOptions, session: Session, source: SessionStartSource, + reservations: RegistrationReservations, setup?: (agentCtx: Context) => Promise | void, ): Promise { - const lifecycle = this.prepareLifecycle(id, options, session) + const lifecycle = this.prepareLifecycle(id, options, session, reservations) try { // The owner-disposal branch makes a never-settling setup unable to hold // the transaction or its ID reservations forever. Promise.race installs diff --git a/packages/core/agent-loop/tests/agent.spec.ts b/packages/core/agent-loop/tests/agent.spec.ts index ab3494d991..b2a4fe30d5 100644 --- a/packages/core/agent-loop/tests/agent.spec.ts +++ b/packages/core/agent-loop/tests/agent.spec.ts @@ -7,7 +7,7 @@ import SystemPrompt from '@deepseek-ai/dsh-system-prompt' import ToolRegistry from '@deepseek-ai/dsh-tools' import AgentRegistry from '@deepseek-ai/dsh-agent' import AgentLoop, { ReactLoopAgent } from '@deepseek-ai/dsh-agent-loop' -import { prepareReactLoopAgent } from '../src/agent.ts' +import { bindReactLoopAgentContext, prepareReactLoopAgent } from '../src/agent.ts' import { MockAdapter, textResponse } from './mock-adapter.ts' async function harness(adapter: MockAdapter) { @@ -49,6 +49,34 @@ function send(agent: ReactLoopAgent, text: string) { } describe('ReactLoopAgent', () => { + it('owns immutable runtime bindings for id, options, session, and scoped context', async () => { + const ctx = await harness(new MockAdapter([textResponse('unused')])) + const options = { model: 'mock' } + const agent = ctx.agentLoop.create(AgentId('owned-bindings'), options) + const acceptedSession = agent.session + const acceptedContext = agent.ctx + + options.model = 'caller-mutated' + expect(agent.options).toEqual({ model: 'mock' }) + expect(Object.isFrozen(agent.options)).toBe(true) + expect(Reflect.set(agent, 'id', AgentId('redirected'))).toBe(false) + expect(Reflect.set(agent, 'options', { model: 'other' })).toBe(false) + expect(Reflect.set(agent, 'session', ctx.sessions.create(SessionId('other')))).toBe(false) + expect(Reflect.set(agent, 'ctx', new Context())).toBe(false) + expect(agent.id).toBe('owned-bindings') + expect(agent.session).toBe(acceptedSession) + expect(agent.ctx).toBe(acceptedContext) + expect(() => { bindReactLoopAgentContext(agent, new Context()) }).toThrow(/context is already bound/) + for (const name of ['id', 'options', 'session', 'ctx']) { + expect(Object.getOwnPropertyDescriptor(agent, name)).toMatchObject({ + configurable: false, + writable: false, + }) + } + + await ctx.fiber.dispose() + }) + it('send() throws after disposal', async () => { const adapter = new MockAdapter(['hang']) const ctx = await harness(adapter) @@ -197,6 +225,23 @@ describe('ReactLoopAgent', () => { warn.mockRestore() }) + it('idle inject() safely renders a hostile non-Error flush failure', async () => { + const ctx = await harness(new MockAdapter([textResponse('ok')])) + const hostile = { [Symbol.toPrimitive]() { throw new Error('no coercion') } } + ctx.on('session/flush', () => { throw hostile }) + const warn = vi.spyOn(ctx.logger, 'warn').mockImplementation(() => undefined) + const agent = ctx.agentLoop.create(AgentId('hostile-flush'), { model: 'mock' }) + const errors: string[] = [] + ctx.on('agent/error', (_a, _turn, _step, error) => void errors.push(error.message)) + + agent.inject([{ type: 'text', text: 'notice' }]) + await new Promise(r => setTimeout(r, 20)) + + expect(errors).toEqual(['']) + expect(warn).toHaveBeenCalledWith(expect.stringContaining('')) + warn.mockRestore() + }) + it('idle inject() with a non-serializable source opens no turn (nothing to close)', async () => { const adapter = new MockAdapter([textResponse('ok')]) const ctx = await harness(adapter) @@ -416,7 +461,7 @@ describe('ReactLoopAgent', () => { expect(adapter.requests).toHaveLength(1) expect(agent.status).toBe('idle') - expect(warn).toHaveBeenCalledWith(expect.stringContaining('agent/status listener threw on running')) + expect(warn).toHaveBeenCalledWith(expect.stringContaining('agent event "agent/status" listener threw')) warn.mockRestore() }) @@ -434,7 +479,7 @@ describe('ReactLoopAgent', () => { expect(adapter.requests).toHaveLength(1) expect(agent.status).toBe('idle') - expect(warn).toHaveBeenCalledWith(expect.stringContaining('agent/status listener threw on idle')) + expect(warn).toHaveBeenCalledWith(expect.stringContaining('agent event "agent/status" listener threw')) warn.mockRestore() }) }) diff --git a/packages/core/agent-loop/tests/scope-lifecycle.spec.ts b/packages/core/agent-loop/tests/scope-lifecycle.spec.ts index 6d0bb3107b..5ed11d396a 100644 --- a/packages/core/agent-loop/tests/scope-lifecycle.spec.ts +++ b/packages/core/agent-loop/tests/scope-lifecycle.spec.ts @@ -197,6 +197,35 @@ describe('agent scope lifecycle', () => { await handle.dispose() }) + it('makes setup-time publication structurally impossible through public stores', async () => { + const ctx = await harness() + const lifecycle: string[] = [] + ctx.on('session/created', () => void lifecycle.push('session')) + ctx.on('agent/created', () => void lifecycle.push('agent')) + + const handle = await ctx.agents.create({ + agentId: AgentId('guarded-publication'), + sessionId: SessionId('guarded-publication-s'), + agentOptions: { model: 'mock' }, + setup: (agentCtx) => { + const agent = agentCtx.agent! + expect(() => agentCtx.agents.enter(agent)).toThrow(/reserved for unpublished creation/) + expect(() => agentCtx.agents.register(agent)).toThrow(/reserved for unpublished creation/) + expect(() => agentCtx.sessions.enter(agent.session)).toThrow(/reserved for unpublished creation/) + expect(() => agentCtx.sessions.prepare(agent.session.id)).toThrow(/reserved for unpublished creation/) + expect(() => agentCtx.sessions.create(agent.session.id)).toThrow(/reserved for unpublished creation/) + expect(lifecycle).toEqual([]) + expect(ctx.agents.get(agent.id)).toBeUndefined() + expect(ctx.sessions.get(agent.session.id)).toBeUndefined() + }, + }) + + expect(lifecycle).toEqual(['session', 'agent']) + expect(ctx.agents.get(handle.agent.id)).toBe(handle.agent) + expect(ctx.sessions.get(handle.agent.session.id)).toBe(handle.agent.session) + await handle.dispose() + }) + it('structurally rejects every driving verb during setup', async () => { const ctx = await harness() const handle = await ctx.agents.create({ @@ -357,6 +386,33 @@ describe('agent scope lifecycle', () => { await retry.dispose() }) + it('pairs session and agent announcements when agent creation aborts publication', async () => { + const ctx = await harness() + const lifecycle: string[] = [] + ctx.on('session/created', (session) => { lifecycle.push(`session-created:${session.id}`) }) + ctx.on('session/disposed', (session) => { lifecycle.push(`session-disposed:${session.id}`) }) + ctx.on('agent/created', (agent) => { + lifecycle.push(`agent-created:${agent.id}`) + throw new Error('agent observer failed') + }) + ctx.on('agent/disposed', (agent) => { lifecycle.push(`agent-disposed:${agent.id}`) }) + + await expect(ctx.agents.create({ + agentId: AgentId('partial-agent'), + sessionId: SessionId('partial-session'), + agentOptions: { model: 'mock' }, + })).rejects.toThrow('agent observer failed') + + expect(lifecycle).toEqual([ + 'session-created:partial-session', + 'agent-created:partial-agent', + 'agent-disposed:partial-agent', + 'session-disposed:partial-session', + ]) + expect(ctx.agents.get(AgentId('partial-agent'))).toBeUndefined() + expect(ctx.sessions.get(SessionId('partial-session'))).toBeUndefined() + }) + it('the synchronous config helper rolls back when publication throws', async () => { const ctx = await harness() const sessionsBefore = ctx.sessions.list().length diff --git a/packages/core/agent/README.md b/packages/core/agent/README.md index 23fa073ea1..1b1e65a289 100644 --- a/packages/core/agent/README.md +++ b/packages/core/agent/README.md @@ -8,10 +8,10 @@ Tracks live agents so UI, hook, and orchestrator plugins can find them without i ### Public API -The scoped-registration surface: `Agent.ctx` is the agent's scope context (`dsh-scope`, key = the agent) — register tools/sections/variables/listeners through it for that agent alone, all unwound on disposal. `agentEvents(ctx, agent)` is the fused dispatcher every agent-subject event goes through (carrier + injected subject in one move); `assembleContextFor(agent)` builds the per-agent assembly context (`agent` + `scope` together). `CreateAgentOptions.setup(agentCtx)` and `ResumeAgentOptions.setup(agentCtx)` compose a fresh or resumed agent's scoped world while the factory keeps the agent and session unpublished; creation awaits setup and a same-turn owner-unload checkpoint before either creation notification or the first assembly. Setup composes, it never drives: the concrete loop rejects driving verbs until the `agent/session-start` boundary. +The scoped-registration surface: `Agent.ctx` is the agent's scope context (`dsh-scope`, key = the agent) — register tools/sections/variables/listeners through it for that agent alone, all unwound on disposal. `agentEvents(ctx, agent)` is the fused dispatcher every agent-subject event goes through (carrier + injected subject in one move); its notification mode invokes every listener and contains both synchronous throws and returned-promise rejections. `assembleContextFor(agent)` builds the per-agent assembly context (`agent` + `scope` together). `CreateAgentOptions.setup(agentCtx)` and `ResumeAgentOptions.setup(agentCtx)` compose a fresh or resumed agent's scoped world while registry/store-owned reservation capabilities keep both identities unpublished; creation awaits setup and a same-turn owner-unload checkpoint before either creation notification or the first assembly. Setup composes, it never drives or publishes: driving verbs and ordinary agent/session insertion both reject until the owning publication boundary. - `ctx.agents.register(agent: Agent): () => Promise | void` — record an **already-constructed** agent. Disposed with the calling fiber. -- Advanced ordered lifecycle: `enter(agent): () => void` inserts without announcing, and `announce(agent)` emits `agent/created` only for that exact live entry. The async factory uses this split after setup; ordinary plugins use `register()`. +- Advanced ordered lifecycle: `reserve(id)` returns an opaque unpublished-identity capability owned by the calling fiber (owner unload releases an abandoned reservation); `enter(agent, reservation?): () => void` inserts under one captured, runtime-pinned id without announcing; and `announce(agent)` emits `agent/created` exactly once for that exact live entry, rejecting repeat or reentrant announcement. While reserved, bare `register`/`enter` calls for the id reject, including from setup. The factory uses this split; ordinary plugins use `register()`. - `ctx.agents.get(id: AgentId): Agent | undefined` - `ctx.agents.list(): Agent[]` @@ -20,7 +20,7 @@ The scoped-registration surface: `Agent.ctx` is the agent's scope context (`dsh- Agent *creation* is provided by the plugin implementing `AgentFactory` (`dsh-agent-loop`), registered via `setFactory`. This keeps creation on the `dsh-agent` interface so consumers (UI, the ACP bridge) program against `ctx.agents` without depending on the concrete loop package. - `ctx.agents.setFactory(factory: AgentFactory): () => Promise | void` — register the creation factory (the loop calls this on construction). Throws on a second factory; the slot clears on dispose. -- `ctx.agents.create(options: CreateAgentOptions): Promise` — snapshot caller-owned IDs/options/metadata and hand the one-read raw seed synchronously to the session boundary for one-pass lossless-JSON materialization, construct and await optional setup while unpublished, insert and announce both session and agent, open the `agent/session-start` driving boundary, then start a new loop on the caller-supplied `sessionId`. Agent/session IDs are reserved across setup; seed rejection, setup rejection, or owner unload publishes nothing. Publication is rollback-covered: if a creation listener throws, entries and scope unwind but effects of already-delivered notifications remain observable; an agent whose announcement began emits `agent/disposed` during that rollback. Rejects if no factory is registered. +- `ctx.agents.create(options: CreateAgentOptions): Promise` — snapshot caller-owned IDs/options/metadata and hand the one-read raw seed synchronously to the session boundary for one-pass lossless-JSON materialization, construct and await optional setup while unpublished, insert and announce both session and agent, open the `agent/session-start` driving boundary, then start a new loop on the caller-supplied `sessionId`. Registry/store reservation capabilities block every competing public insertion across setup; seed rejection, setup rejection, or owner unload publishes nothing. Publication is rollback-covered: if a creation listener throws, entries and scope unwind but effects of already-delivered notifications remain observable; any creation announcement that began is paired by `agent/disposed` or `session/disposed`. Rejects if no factory is registered. - `ctx.agents.resume(options: ResumeAgentOptions): Promise` — snapshot caller-owned IDs/options, load a persisted session ([session persistence](../../../docs/rfc/implemented/architecture/2026-06-14-session-persistence.md)), mint a fresh agent scope, await optional setup while unpublished, then follow the same insert → announce → session-start → loop-start boundary. The IDs are reserved across persistence load and setup; load/setup rejection or owner unload publishes nothing. Rejects if no factory is registered or session persistence is unconfigured. `AgentHandle = { agent: Agent; dispose(): Promise }`. The disposer is a **capability** — only the holder can tear this agent down. `dispose()` stops the loop, `await`s its exit plus every outstanding idle-injection flush (quiescence — NOT just the `disposed` status flip), unregisters the agent, removes its session from the store, and finally unwinds its scoped world. This order captures every agent-started `session/flush` before the session is detached and keeps scoped listeners alive through those checkpoints. `ctx.agents.get(id)` still returns a bare `Agent` — the handle is only for the OWNER that created it. The ACP bridge and in-process subagent backends are production consumers; config-created agents are owned by the loop fiber and never need a handle. diff --git a/packages/core/agent/src/dispatch.ts b/packages/core/agent/src/dispatch.ts index b02dfeab57..881faa675f 100644 --- a/packages/core/agent/src/dispatch.ts +++ b/packages/core/agent/src/dispatch.ts @@ -45,7 +45,10 @@ type Tail = Params extends [Agent, ...in */ export interface AgentEventDispatch { /** - * Fire-and-forget notification (Cordis `emit`) in the agent's scope. + * Fire-and-forget notification in the agent's scope. Every listener is + * invoked; synchronous throws and returned-promise rejections are logged and + * contained per listener, so a notification cannot veto lifecycle progress + * or starve a later observer. * @param name - the agent-subject event to emit. * @param rest - the event's arguments after the injected agent. */ @@ -96,9 +99,22 @@ export function agentEvents(ctx: Context, agent: Agent): AgentEventDispatch { // tuple — hence one contained, shape-preserving cast per method. return { emit(name, ...rest) { - // eslint-disable-next-line @typescript-eslint/unbound-method -- the events mixin accessor returns a pre-bound function - const emit = ctx.emit as (thisArg: Scoped, name: string, ...args: unknown[]) => void - emit(carrier, name, agent, ...rest) + // Cordis emit invokes callbacks through Array.map: one synchronous throw + // starves later listeners, and returned promises are discarded. Agent + // notifications are non-vetoing, so resolve the same filtered callback + // set ourselves and contain both failure modes independently. + const args: unknown[] = [carrier, name, agent, ...rest] + const callbacks = ctx.events.dispatch('emit', args) + for (const callback of callbacks) { + try { + const returned: unknown = callback(...args) + void Promise.resolve(returned).catch((error: unknown) => { + ctx.logger.warn(`agent event "${name}" listener rejected: ${renderThrown(error)}`) + }) + } catch (error: unknown) { + ctx.logger.warn(`agent event "${name}" listener threw: ${renderThrown(error)}`) + } + } }, async serial(name, ...rest) { // eslint-disable-next-line @typescript-eslint/unbound-method -- the events mixin accessor returns a pre-bound function @@ -129,6 +145,15 @@ export function agentEvents(ctx: Context, agent: Agent): AgentEventDispatch { } } +/** Render an arbitrary thrown value without allowing coercion to throw again. */ +function renderThrown(value: unknown): string { + try { + return value instanceof Error ? `${value.name}: ${value.message}` : String(value) + } catch { + return '' + } +} + /** * The assembly context for one agent's prompt: the typed `agent` DX field and * the `scope` layer selector, set together (setting `agent` without `scope` diff --git a/packages/core/agent/src/index.ts b/packages/core/agent/src/index.ts index 959ca4784c..be7bb02c67 100644 --- a/packages/core/agent/src/index.ts +++ b/packages/core/agent/src/index.ts @@ -9,6 +9,7 @@ import { Context, Service } from 'cordis' import { scopeTarget } from '@deepseek-ai/dsh-scope' import type { SessionEvent, SessionId } from '@deepseek-ai/dsh-session' import type { Agent, AgentId, AgentOptions } from './types.ts' +import { agentEvents } from './dispatch.ts' export * from './types.ts' export { agentEvents, assembleContextFor } from './dispatch.ts' @@ -141,9 +142,9 @@ export interface AgentFactory { * creation notifications in order, unlocks driving at * `agent/session-start`, and only then starts the loop. The sequence is * rollback-covered, but notifications delivered before a later listener - * failure remain observable; if agent announcement began, rollback emits - * `agent/disposed`, while the session entry is removed without a separate - * disposal event. The owner disposes the resolved handle to stop/drain, + * failure remain observable; every agent or session creation announcement + * that began is paired by `agent/disposed` or `session/disposed` during + * rollback. The owner disposes the resolved handle to stop/drain, * unregister, remove the session, and unwind the scope. * @param options - agent/session identity, configuration, and optional setup. * @returns the owned handle after setup, both announcements, and loop start complete. @@ -164,6 +165,33 @@ export interface AgentFactory { /** Thrown when create/resume is called before an agent factory is registered. */ const NO_FACTORY_MESSAGE = 'no agent factory registered (load an agent-loop plugin)' +/** Render an arbitrary thrown value without allowing coercion to throw again. */ +function renderThrown(value: unknown): string { + try { + return value instanceof Error ? `${value.name}: ${value.message}` : String(value) + } catch { + return '' + } +} + +/** + * Unforgeable ownership handle for one unpublished agent id. The factory holds + * this object across asynchronous setup; while it is live, ordinary public + * registration of that id fails, so setup cannot publish the factory's agent + * (or a replacement with the same id) ahead of the transaction. Callers obtain + * handles only from {@link AgentRegistry.reserve}. + */ +export interface AgentRegistrationReservation { + /** The reserved registry id. */ + readonly id: AgentId + /** + * Release the unpublished reservation; idempotent. The registry also + * releases it automatically when the fiber that called `reserve` disposes. + * @returns nothing. + */ + release(): void +} + /** * Agent registry (`ctx.agents`): tracks live agents so UI, hook, and * orchestrator plugins can find them without depending on the concrete loop @@ -173,6 +201,10 @@ const NO_FACTORY_MESSAGE = 'no agent factory registered (load an agent-loop plug */ export class AgentRegistry extends Service { private store = new Map() + /** The one accepted registry key for each live agent; never reread caller state. */ + private acceptedIds = new WeakMap() + /** Unpublished identities held across factory setup/load transactions. */ + private reservations = new Map() /** Entries whose `agent/created` announcement phase began. */ private announced = new WeakSet() private factory: AgentFactory | undefined @@ -188,6 +220,49 @@ export class AgentRegistry extends Service { ctx.accessor('agent', { get: () => undefined }) } + /** + * Reserve an unpublished agent id. Registration through {@link register} or + * bare {@link enter} fails until the returned capability is released; the + * owning factory passes the exact capability back to `enter` at publication. + * This makes “setup cannot publish” structural rather than a cooperative + * convention, including attempts to register a different object under the + * reserved id. The reservation belongs to the calling fiber and is released + * automatically if that owner unloads before the transaction settles. + * @param id - the id the factory transaction will publish. + * @returns the opaque reservation capability. + * @throws if the id is malformed, live, or already reserved. + */ + reserve(id: AgentId): AgentRegistrationReservation { + if (typeof id !== 'string') throw new TypeError('agent id must be a string') + if (this.store.has(id) || this.reservations.has(id)) { + throw new Error(`agent "${id}" is already registered or reserved`) + } + let active = true + const rawRelease = (): void => { + if (!active) return + active = false + this.reservations.delete(id) + } + let disposeEffect!: () => Promise | void + const reservation: AgentRegistrationReservation = Object.freeze({ + id, + release: () => { + rawRelease() + // Remove the now-inert ownership effect on manual transaction settle; + // its cleanup is the exact idempotent raw release above. + void disposeEffect() + }, + }) + this.reservations.set(id, reservation) + try { + disposeEffect = this.ctx.effect(() => rawRelease, `agents.reserve(${id})`) + } catch (error: unknown) { + rawRelease() + throw error + } + return reservation + } + /** * Register the agent-creation factory (the loop calls this on construction, * effect-scoped). Throws if a factory is already registered. Returns the @@ -269,43 +344,87 @@ export class AgentRegistry extends Service { * returned detach closure into its pre-installed composite teardown before * calling {@link announce}. Ordinary callers use {@link register}. * @param agent - the prepared, unpublished agent. + * @param reservation - the exact unpublished-id capability, when a factory + * reserved this id across setup. * @returns an idempotent closure that removes this exact entry and emits * `agent/disposed` with listener failures contained. */ - enter(agent: Agent): () => void { - if (this.store.has(agent.id)) { - throw new Error(`agent "${agent.id}" is already registered`) + enter(agent: Agent, reservation?: AgentRegistrationReservation): () => void { + const id = agent.id + if (typeof id !== 'string') throw new TypeError('agent id must be a string') + const held = this.reservations.get(id) + if (reservation === undefined) { + if (held !== undefined) throw new Error(`agent "${id}" is reserved for unpublished creation`) + } else if (reservation.id !== id || held !== reservation) { + throw new Error(`agent "${id}" registration reservation is not active for this id`) } - this.store.set(agent.id, agent) + if (this.acceptedIds.has(agent)) { + throw new Error(`agent "${id}" is already registered`) + } + if (this.store.has(id)) { + throw new Error(`agent "${id}" is already registered`) + } + try { + // Registration accepts ownership of the public identity contract. Pin an + // own data slot from the one captured value so a custom JavaScript Agent + // with a getter or writable field cannot later present a different id to + // event listeners while the registry still owns the accepted key. + Object.defineProperty(agent, 'id', { + value: id, + enumerable: true, + writable: false, + configurable: false, + }) + } catch { + // Only the engine's property-definition failure is swallowed; the stable + // public error below is the registration contract exposed to callers. + throw new TypeError('agent id must be installable as a stable own property') + } + this.store.set(id, agent) + this.acceptedIds.set(agent, id) let entered = true return () => { if (!entered) return entered = false - this.store.delete(agent.id) + this.store.delete(id) + this.acceptedIds.delete(agent) // An insertion rolled back before announce was never externally created, // so emitting disposed would invent an impossible lifecycle edge. Marking // happens before the created emit: if a later created listener throws, // earlier listeners may already have observed it and must see disposal. if (!this.announced.delete(agent)) return - try { - this.ctx.emit(scopeTarget(agent, agent), 'agent/disposed', agent) - } catch (error: unknown) { - this.ctx.logger.warn(`agent "${agent.id}": agent/disposed listener threw: ${String(error)}`) - } + agentEvents(this.ctx, agent).emit('agent/disposed') } } /** * Announce an agent previously inserted with {@link enter}. * @param agent - the live inserted agent to announce. - * @throws if `agent` is not the exact live registry entry for its id. + * @throws if `agent` is not the exact live registry entry for its id, or its + * creation announcement already began (including a reentrant call from a + * creation listener). */ announce(agent: Agent): void { - if (this.store.get(agent.id) !== agent) { - throw new Error(`agent "${agent.id}" is not live in this registry`) + const id = this.acceptedIds.get(agent) + if (id === undefined || this.store.get(id) !== agent) { + throw new Error(`agent "${id ?? ''}" is not live in this registry`) } + if (this.announced.has(agent)) { + throw new Error(`agent "${id}" was already announced`) + } + // Mark before dispatch so a listener cannot recursively create a second + // lifecycle edge; detach still pairs a partially delivered first edge. this.announced.add(agent) - this.ctx.emit(scopeTarget(agent, agent), 'agent/created', agent) + const args: unknown[] = [scopeTarget(agent, agent), 'agent/created', agent] + for (const callback of this.ctx.events.dispatch('emit', args)) { + // A synchronous creation failure vetoes publication and rolls back. + // Returned-promise rejection happens after this synchronous boundary, so + // observe and report it instead of leaking an unhandled rejection. + const returned: unknown = callback(...args) + void Promise.resolve(returned).catch((error: unknown) => { + this.ctx.logger.warn(`agent "${id}": agent/created listener rejected: ${renderThrown(error)}`) + }) + } } /** diff --git a/packages/core/agent/src/types.ts b/packages/core/agent/src/types.ts index 4196eb43d6..b6a32fa050 100644 --- a/packages/core/agent/src/types.ts +++ b/packages/core/agent/src/types.ts @@ -288,7 +288,10 @@ declare module 'cordis' { * {@link AgentRegistry}. Its session is already live in the session store, * but concrete factories may keep driving verbs locked until the subsequent * `agent/session-start` boundary; that event is the first supported place - * to inject or queue work during startup. + * to inject or queue work during startup. A synchronous listener throw + * vetoes publication and rollback emits the matching disposal edges; + * returned-promise rejection is observed and logged but cannot + * retroactively veto this synchronous boundary. * @param agent - the newly registered agent with its live session and completed setup. * Scope-filtered dispatch (`@deepseek-ai/dsh-scope`): a listener registered * through `agent.ctx` fires only for that agent's dispatches; a listener on a diff --git a/packages/core/agent/tests/agent.spec.ts b/packages/core/agent/tests/agent.spec.ts index 4e89dc0e84..87a0e51e9d 100644 --- a/packages/core/agent/tests/agent.spec.ts +++ b/packages/core/agent/tests/agent.spec.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from 'vitest' import { Context } from 'cordis' import { Session, SessionId } from '@deepseek-ai/dsh-session' -import AgentRegistry, { Agent, AgentId } from '@deepseek-ai/dsh-agent' +import AgentRegistry, { Agent, AgentId, agentEvents } from '@deepseek-ai/dsh-agent' function stubAgent(rawId: string): Agent { const id = AgentId(rawId) @@ -78,6 +78,32 @@ describe('AgentRegistry', () => { expect(ctx.agents.get(AgentId('main'))).toBeUndefined() }) + it('observes async agent/created rejection without rolling back or starving peers', async () => { + const ctx = new Context() + await ctx.plugin(AgentRegistry) + const warnings: string[] = [] + ctx.logger.warn = ((message: unknown) => { warnings.push(String(message)) }) as typeof ctx.logger.warn + const hostile = { [Symbol.toPrimitive]() { throw new Error('cannot stringify') } } + const heard: string[] = [] + ctx.on('agent/created', () => Promise.reject(new Error('ordinary async failure')) as never) + // eslint-disable-next-line @typescript-eslint/prefer-promise-reject-errors -- hostile thrown values are the boundary under test + ctx.on('agent/created', () => Promise.reject(hostile) as never) + ctx.on('agent/created', (agent) => { heard.push(agent.id) }) + + const agent = stubAgent('async-created') + const dispose = ctx.agents.register(agent) + await Promise.resolve() + await Promise.resolve() + + expect(ctx.agents.get(agent.id)).toBe(agent) + expect(heard).toEqual(['async-created']) + expect(warnings).toEqual([ + 'agent "async-created": agent/created listener rejected: Error: ordinary async failure', + 'agent "async-created": agent/created listener rejected: ', + ]) + await dispose() + }) + it('splits insertion from announcement and makes the detach exact/idempotent', async () => { const ctx = new Context() await ctx.plugin(AgentRegistry) @@ -107,6 +133,149 @@ describe('AgentRegistry', () => { // no disposed-without-created notification. expect(disposed).toEqual([first]) }) + + it('captures and pins one runtime id before insertion, announcement, and detach', async () => { + const ctx = new Context() + await ctx.plugin(AgentRegistry) + const existing = stubAgent('occupied') + const disposeExisting = ctx.agents.register(existing) + const candidate = stubAgent('placeholder') + let reads = 0 + Object.defineProperty(candidate, 'id', { + configurable: true, + get() { + reads += 1 + return reads === 1 ? AgentId('accepted') : AgentId('occupied') + }, + }) + + const detach = ctx.agents.enter(candidate) + expect(reads).toBe(1) + expect(candidate.id).toBe('accepted') + expect(reads).toBe(1) + expect(Object.getOwnPropertyDescriptor(candidate, 'id')).toMatchObject({ + configurable: false, + writable: false, + value: 'accepted', + }) + expect(ctx.agents.get(AgentId('accepted'))).toBe(candidate) + expect(ctx.agents.get(AgentId('occupied'))).toBe(existing) + expect(() => ctx.agents.enter(candidate)).toThrow(/already registered/) + + ctx.agents.announce(candidate) + detach() + expect(ctx.agents.get(AgentId('accepted'))).toBeUndefined() + expect(ctx.agents.get(AgentId('occupied'))).toBe(existing) + await disposeExisting() + + expect(() => ctx.agents.enter({ ...stubAgent('bad'), id: 42 } as unknown as Agent)) + .toThrow(/id must be a string/) + const pinnedAccessor = stubAgent('pinned') + Object.defineProperty(pinnedAccessor, 'id', { + configurable: false, + get: () => AgentId('pinned'), + }) + expect(() => ctx.agents.enter(pinnedAccessor)).toThrow(/installable as a stable own property/) + }) + + it('uses an opaque one-id reservation to gate unpublished factory insertion', async () => { + const ctx = new Context() + await ctx.plugin(AgentRegistry) + const held = ctx.agents.reserve(AgentId('held')) + + expect(() => ctx.agents.reserve(AgentId('held'))).toThrow(/already registered or reserved/) + expect(() => ctx.agents.enter(stubAgent('held'))).toThrow(/reserved for unpublished creation/) + const other = ctx.agents.reserve(AgentId('other')) + expect(() => ctx.agents.enter(stubAgent('held'), other)).toThrow(/not active for this id/) + + const agent = stubAgent('held') + const detach = ctx.agents.enter(agent, held) + ctx.agents.announce(agent) + held.release() + held.release() + expect(ctx.agents.get(AgentId('held'))).toBe(agent) + expect(() => ctx.agents.reserve(AgentId('held'))).toThrow(/already registered or reserved/) + detach() + other.release() + + const expired = ctx.agents.reserve(AgentId('expired')) + expired.release() + expect(() => ctx.agents.enter(stubAgent('expired'), expired)).toThrow(/not active for this id/) + expect(() => ctx.agents.reserve(42 as unknown as AgentId)).toThrow(/id must be a string/) + }) + + it('owns reservations by the calling fiber and rolls back failed ownership registration', async () => { + const ctx = new Context() + await ctx.plugin(AgentRegistry) + let held!: import('@deepseek-ai/dsh-agent').AgentRegistrationReservation + let scopedAgents!: AgentRegistry + const owner = await ctx.plugin(Object.assign((inner: Context) => { + scopedAgents = inner.agents + held = inner.agents.reserve(AgentId('fiber-held')) + }, { inject: ['agents'] })) + + expect(() => ctx.agents.reserve(AgentId('fiber-held'))).toThrow(/already registered or reserved/) + await owner.dispose() + const reused = ctx.agents.reserve(AgentId('fiber-held')) + reused.release() + held.release() // idempotent after the automatic owner-disposal release + + // A disposed tracker cannot own a new effect. The failed effect install + // must remove the map entry it tentatively reserved before propagating. + expect(() => scopedAgents.reserve(AgentId('inactive-owner'))).toThrow(/inactive context/) + const recovered = ctx.agents.reserve(AgentId('inactive-owner')) + recovered.release() + }) + + it('rejects direct and reentrant repeat announcements to preserve one lifecycle pair', async () => { + const ctx = new Context() + await ctx.plugin(AgentRegistry) + let created = 0 + let disposed = 0 + let reentrantError = '' + ctx.on('agent/created', (agent) => { + created += 1 + try { + ctx.agents.announce(agent) + } catch (error: unknown) { + reentrantError = String(error) + } + }) + ctx.on('agent/disposed', () => { disposed += 1 }) + + const agent = stubAgent('once') + const detach = ctx.agents.enter(agent) + ctx.agents.announce(agent) + expect(reentrantError).toMatch(/already announced/) + expect(() => { ctx.agents.announce(agent) }).toThrow(/already announced/) + detach() + expect({ created, disposed }).toEqual({ created: 1, disposed: 1 }) + }) +}) + +describe('agentEvents()', () => { + it('contains synchronous throws and returned-promise rejections per listener', async () => { + const ctx = new Context() + const warnings: string[] = [] + ctx.logger.warn = ((message: unknown) => { warnings.push(String(message)) }) as typeof ctx.logger.warn + const agent = stubAgent('contained') + const heard: string[] = [] + const hostile = { [Symbol.toPrimitive]() { throw new Error('cannot stringify') } } + + ctx.on('agent/status', () => { throw hostile }) + ctx.on('agent/status', () => Promise.reject(new Error('async listener')) as never) + ctx.on('agent/status', (_subject, status) => { heard.push(status) }) + + expect(() => { agentEvents(ctx, agent).emit('agent/status', 'running') }).not.toThrow() + await Promise.resolve() + await Promise.resolve() + + expect(heard).toEqual(['running']) + expect(warnings).toEqual([ + 'agent event "agent/status" listener threw: ', + 'agent event "agent/status" listener rejected: Error: async listener', + ]) + }) }) describe('AgentRegistry factory seam', () => { diff --git a/packages/core/scope/README.md b/packages/core/scope/README.md index f779dab57e..727acea4fb 100644 --- a/packages/core/scope/README.md +++ b/packages/core/scope/README.md @@ -9,7 +9,7 @@ Scoped-context registration primitive. `createScope(ctx, key)` mints a Cordis co - `Scope.rawDispose` The EXACT Cordis disposer for the backing fiber — a composite (generator) effect yields THIS function to nest the scope's teardown at that yield position (Cordis dedupes nested effects by function identity; yielding a wrapper leaves the scope disposing as a concurrent sibling). - `Scope.dispose(): Promise` Idempotent, shared quiescence boundary for every registration made through the scope. Racing/repeat calls await the same teardown, including when `rawDispose` invoked the underlying single-shot Cordis disposer first. - `scopeOf(ctx: Context): ScopeKey | undefined` The tag a context (or any context derived from it) carries; `undefined` = context-global. -- `scopeTarget(base: T, key?: ScopeKey): Scoped` Build the dispatch `thisArg` for a scope-filtered event: composes `base`'s own `Context.filter` with the scope predicate (untagged listener ⇒ admitted; tagged ⇒ admitted iff tag === key; `key === undefined` ⇒ untagged only). Listener `this` stays `base`-shaped. `{ global: true }` listeners bypass filtering (Cordis semantics). +- `scopeTarget(base: T, key: ScopeKey | undefined): Scoped` Build the dispatch `thisArg` for a scope-filtered event: capture and compose `base`'s own `Context.filter` with the scope predicate (untagged listener ⇒ admitted; tagged ⇒ admitted iff tag === key; `key === undefined` ⇒ untagged only). The carrier uses a dedicated surrogate proxy target whose immutable filter slot cannot be replaced by a base property pinned before, during, or after construction; ordinary property access, writes, own-key visibility, methods, invocation, and construction delegate to `base`, and callable carriers match the base's constructable/non-constructable shape. For non-overlay base-owned properties, descriptor queries preserve values and flags except that configurable is normalized to `true`, as required to report those properties through an extensible surrogate. Listener `this` stays `base`-shaped. `{ global: true }` listeners bypass filtering (Cordis semantics). - `Scoped` The compile-time carrier brand: scope-filtered events demand it as their `this` type, so dispatching with a bare subject is a compile error. - `isScopeCarrier(value)` / `carrierKeyOf(value)` Runtime carrier marks, used by the dev invariants to assert every scope-filtered dispatch carries a carrier keyed to the subject its arguments name. - `scopeHost(ctx, services)` Test/tooling host that snapshots the requested service list before activation, fails loud with stable missing-service diagnostics, and whose shared `dispose()` waits for both the host fiber and every minted scope, including a child already tearing down through `rawDispose`. diff --git a/packages/core/scope/src/index.ts b/packages/core/scope/src/index.ts index 276b406dcd..9c917fc89d 100644 --- a/packages/core/scope/src/index.ts +++ b/packages/core/scope/src/index.ts @@ -171,6 +171,20 @@ export function scopeOf(ctx: Context): ScopeKey | undefined { return (ctx as Context & { [kScope]?: ScopeKey })[kScope] } +/** Whether a callable has JavaScript's internal construction capability. */ +function isConstructable(value: (...args: unknown[]) => unknown): boolean { + try { + // A Proxy has [[Construct]] iff its target does. Its trap returns before + // the engine invokes `value` or reads `value.prototype`, so a hostile but + // constructable callable cannot be mistaken for a non-constructor. + Reflect.construct(new Proxy(value, { construct: () => ({}) }), []) + return true + } catch { + // The harmless outer trap leaves lack of [[Construct]] as the only failure. + return false + } +} + /** * Build the dispatch carrier for a scope-filtered event: `base` overlaid with * a `Context.filter` that admits a listener iff @@ -206,7 +220,10 @@ export function scopeOf(ctx: Context): ScopeKey | undefined { * @returns the carrier to pass as the dispatch `thisArg`. */ export function scopeTarget(base: T, key: ScopeKey | undefined): Scoped { - const baseFilter = (base as { [CordisContext.filter]?: (ctx: Context) => boolean })[CordisContext.filter] + const baseFilter: unknown = (base as { [CordisContext.filter]?: unknown })[CordisContext.filter] + if (baseFilter !== undefined && typeof baseFilter !== 'function') { + throw new TypeError('scope target Context.filter must be a function when present') + } const filter = (ctx: Context): boolean => { if (baseFilter && !baseFilter.call(base, ctx)) return false const tag = scopeOf(ctx) @@ -214,34 +231,57 @@ export function scopeTarget(base: T, key: ScopeKey | undefined } const overlay: Record = { [CordisContext.filter]: filter, - [kCarrier]: { key }, + [kCarrier]: Object.freeze({ key }), } - // A hand-rolled proxy, NOT cordis withProps: withProps delegates gets with - // the PROXY as receiver, so a getter on `base` runs with proxy `this` and a - // method call through the carrier gets a proxy receiver — either one throws - // on a native `#private` field of the subject (TypeError: private member - // not declared). Cordis hands the carrier to listeners as `this`, and the - // event declarations type it `Scoped` — so subject method calls - // through it are a SUPPORTED shape and must reach the real object: gets - // delegate with `base` as receiver, functions come back bound to `base`, - // and sets land on `base` directly. - return new Proxy(base, { + // Use a dedicated extensible proxy TARGET, never `base` itself. Proxy get + // invariants force a trap to return a base's non-configurable/non-writable + // own value verbatim; if a caller pinned Context.filter during or after + // construction, a base-target proxy would therefore silently replace the + // composed scope predicate with the caller's filter. The surrogate owns the + // two immutable overlay slots, so later descriptor changes on `base` cannot + // affect isolation. It shares the base prototype and delegates ordinary + // reads/writes/keys to preserve the supported transparent shape. Callable + // targets use native bound built-ins so V8 contributes no user-code surface; + // the chosen built-in matches whether `base` has [[Construct]], and the traps + // below delegate the actual call/construction to `base`. + const callableBase = typeof base === 'function' + ? base as unknown as (...args: unknown[]) => unknown + : undefined + const constructable = callableBase !== undefined && isConstructable(callableBase) + const target: object = callableBase === undefined + ? {} + : constructable + ? Object.bind(undefined) + : Math.max.bind(undefined) + Reflect.setPrototypeOf(target, Reflect.getPrototypeOf(base)) + Object.defineProperties(target, { + [CordisContext.filter]: { + value: filter, + enumerable: false, + writable: false, + configurable: false, + }, + [kCarrier]: { + value: overlay[kCarrier], + enumerable: false, + writable: false, + configurable: false, + }, + }) + const carrier = new Proxy(target, { get(target, prop) { - // Proxy get invariants pin what this trap may report for a - // non-configurable OWN property of the base: a non-writable data prop - // must be reported AS-IS (neither overlaid nor bound), a getterless - // accessor as undefined — checked FIRST so even an overlay key - // colliding with a frozen own prop of a (pathological) base yields the - // base's value instead of an engine TypeError. Such a base forgoes - // scope filtering; no production base freezes these keys. + // The callable surrogate has engine-owned pinned properties (`prototype`, + // `caller`, …); honor those target invariants. For object carriers the + // only pinned target properties are the exact overlay values above. const own = Reflect.getOwnPropertyDescriptor(target, prop) const pinned = own !== undefined && own.configurable === false && own.get === undefined && own.writable !== true - // hasOwn, not `in`: the overlay literal inherits Object.prototype, so - // `in` would claim `toString`/`constructor` and shadow the subject's. - if (!pinned && Object.hasOwn(overlay, prop)) return overlay[prop] - const value: unknown = Reflect.get(target, prop, target) - if (typeof value !== 'function' || pinned) return value + if (pinned) { + const value: unknown = Reflect.get(target, prop, target) + return value + } + const value: unknown = Reflect.get(base, prop, base) + if (typeof value !== 'function') return value // `constructor` is looked up, never invoked as a subject method — keep // the real one (withProps special-cases it the same way), so // `carrier.constructor` still identifies the subject's class. @@ -249,12 +289,66 @@ export function scopeTarget(base: T, key: ScopeKey | undefined // `Function.prototype.bind` types as `any`; the value is structurally // T[prop] and the trap's contract is untyped (`any`), so unknown is the // honest safe return. - return value.bind(target) as unknown + return value.bind(base) as unknown }, - set(target, prop, value) { - return Reflect.set(target, prop, value, target) + set(_target, prop, value) { + if (Object.hasOwn(overlay, prop)) return false + return Reflect.set(base, prop, value, base) }, - }) as Scoped + has(_target, prop) { + // A Proxy may not hide a non-configurable target key. Configurable + // surrogate-only keys (bound-function name/length) are omitted; the + // base's own/inherited surface remains authoritative. + const own = Reflect.getOwnPropertyDescriptor(target, prop) + return own?.configurable === false || Reflect.has(base, prop) + }, + ownKeys(target) { + const requiredTargetKeys = Reflect.ownKeys(target).filter((prop) => { + return Reflect.getOwnPropertyDescriptor(target, prop)?.configurable === false + }) + return [...new Set([...requiredTargetKeys, ...Reflect.ownKeys(base)])] + }, + getOwnPropertyDescriptor(target, prop) { + const targetDescriptor = Reflect.getOwnPropertyDescriptor(target, prop) + if (targetDescriptor?.configurable === false) return targetDescriptor + const baseDescriptor = Reflect.getOwnPropertyDescriptor(base, prop) + if (baseDescriptor !== undefined) return { ...baseDescriptor, configurable: true } + // Configurable surrogate-only function metadata is intentionally hidden. + return undefined + }, + defineProperty(_target, prop, attributes) { + if (Object.hasOwn(overlay, prop)) return false + return Reflect.defineProperty(base, prop, attributes) + }, + deleteProperty(_target, prop) { + if (Object.hasOwn(overlay, prop)) return false + return Reflect.deleteProperty(base, prop) + }, + preventExtensions() { + // Keeping the surrogate extensible is required for ownKeys to report + // caller-owned base fields that may change over the carrier's lifetime. + return false + }, + setPrototypeOf() { + // The carrier prototype and base delegation must not be split. + return false + }, + apply(_target, thisArg, args) { + const callable = callableBase as (...values: unknown[]) => unknown + const result: unknown = Reflect.apply(callable, thisArg, args) + return result + }, + construct(_target, args, newTarget) { + const constructor = callableBase as unknown as new (...values: unknown[]) => object + const result: unknown = Reflect.construct( + constructor, + args, + newTarget === carrier ? constructor : newTarget, + ) + return result as object + }, + }) + return carrier as Scoped } /** @@ -266,10 +360,8 @@ export function scopeTarget(base: T, key: ScopeKey | undefined * @returns true iff `value` came from {@link scopeTarget}. */ export function isScopeCarrier(value: unknown): value is Scoped { - if (typeof value !== 'object' || value === null) return false - // A property READ, not an `in` check: the carrier overlays its marks in the - // get trap only (no `has` trap), so `kCarrier in carrier` would fall - // through to the wrapped base and always answer false. + if ((typeof value !== 'object' && typeof value !== 'function') || value === null) return false + // A property read checks the immutable marker owned by the surrogate target. return (value as { [kCarrier]?: { key: ScopeKey | undefined } })[kCarrier] !== undefined } diff --git a/packages/core/scope/tests/scope.spec.ts b/packages/core/scope/tests/scope.spec.ts index e572584fb1..5b379d15e5 100644 --- a/packages/core/scope/tests/scope.spec.ts +++ b/packages/core/scope/tests/scope.spec.ts @@ -232,7 +232,7 @@ describe('scopeTarget dispatch filtering', () => { expect(detached()).toBe(2) }) - it('delegates sets to the base and leaves frozen own function props unbound (proxy invariant)', () => { + it('delegates the ordinary reflective surface while keeping overlays immutable', () => { const frozenFn = (): string => 'frozen' const base: { mutable: number; pinned: () => string; toString: () => string } = { mutable: 0, @@ -243,25 +243,158 @@ describe('scopeTarget dispatch filtering', () => { const carrier = scopeTarget(base, undefined) carrier.mutable = 7 expect(base.mutable).toBe(7) // sets land on the base, not a detached overlay - // A non-configurable, non-writable own data prop must be reported - // unchanged (binding it would violate the proxy get invariant). - expect(carrier.pinned).toBe(frozenFn) + // The surrogate target frees reads from the base property's proxy + // invariant, so even a frozen own method can be safely bound to the base. + expect(carrier.pinned).not.toBe(frozenFn) + expect(carrier.pinned()).toBe('frozen') // The overlay literal inherits Object.prototype; hasOwn (not `in`) keeps // it from shadowing the subject's own prototype-surface members. expect(String(carrier)).toBe('base-str') + expect('mutable' in carrier).toBe(true) + expect(Object.hasOwn(carrier, 'mutable')).toBe(true) + expect(Object.keys(carrier)).toEqual(['mutable', 'pinned', 'toString']) + Object.defineProperty(carrier, 'extra', { value: 1, configurable: true }) + expect((base as typeof base & { extra?: number }).extra).toBe(1) + expect(delete (carrier as typeof carrier & { extra?: number }).extra).toBe(true) + expect(Reflect.preventExtensions(carrier)).toBe(false) + expect(Reflect.setPrototypeOf(carrier, null)).toBe(false) }) - it('honors the get invariant even when an overlay key collides with a frozen own prop of the base', () => { - // Pathological but engine-enforced: a base whose own [Context.filter] is - // a non-configurable, non-writable data prop pins what any proxy over it - // may report for that key. The carrier must yield the base's value (an - // overlay there would be a runtime TypeError from the engine, not a - // filtering choice). Such a base forgoes scope filtering by construction. + it('keeps isolation when the base filter is pinned before, during, or after construction', async () => { + const ctx = new Context() + const keyA = { name: 'A' } + const keyB = { name: 'B' } + const scopeA = await mintScope(ctx, keyA) + const scopeB = await mintScope(ctx, keyB) + const heard: string[] = [] + ctx.on('scope-test/ping', value => void heard.push(`global:${value}`)) + scopeA.ctx.on('scope-test/ping', value => void heard.push(`A:${value}`)) + scopeB.ctx.on('scope-test/ping', value => void heard.push(`B:${value}`)) const pinnedFilter = (): boolean => true - const base = {} - Object.defineProperty(base, Context.filter, { value: pinnedFilter, writable: false, configurable: false }) - const carrier = scopeTarget(base, { name: 'key' }) - expect((carrier as Record)[Context.filter]).toBe(pinnedFilter) + + const pinnedData = {} + Object.defineProperty(pinnedData, Context.filter, { + value: pinnedFilter, + writable: false, + configurable: false, + }) + const pinnedCarrier = scopeTarget(pinnedData, keyA) + ctx.emit(pinnedCarrier, 'scope-test/ping', 'before') + + const duringRead = {} + Object.defineProperty(duringRead, Context.filter, { + configurable: true, + get() { + Object.defineProperty(duringRead, Context.filter, { + value: pinnedFilter, + writable: false, + configurable: false, + }) + return pinnedFilter + }, + }) + ctx.emit(scopeTarget(duringRead, keyA), 'scope-test/ping', 'during') + + const pinnedAfter = { [Context.filter]: pinnedFilter } + const afterCarrier = scopeTarget(pinnedAfter, keyA) + Object.defineProperty(pinnedAfter, Context.filter, { + value: pinnedFilter, + writable: false, + configurable: false, + }) + ctx.emit(afterCarrier, 'scope-test/ping', 'after') + + const pinnedGetterless = {} + Object.defineProperty(pinnedGetterless, Context.filter, { set(_value: unknown) {}, configurable: false }) + ctx.emit(scopeTarget(pinnedGetterless, keyA), 'scope-test/ping', 'getterless') + + expect(heard).toEqual([ + 'global:before', 'A:before', + 'global:during', 'A:during', + 'global:after', 'A:after', + 'global:getterless', 'A:getterless', + ]) + expect((pinnedCarrier as Record)[Context.filter]).not.toBe(pinnedFilter) + expect(Reflect.set(pinnedCarrier, Context.filter, pinnedFilter)).toBe(false) + expect(Reflect.defineProperty(pinnedCarrier, Context.filter, { value: pinnedFilter })).toBe(false) + expect(Reflect.deleteProperty(pinnedCarrier, Context.filter)).toBe(false) + + expect(() => scopeTarget({ [Context.filter]: 1 }, { name: 'A' })).toThrow( + /Context\.filter must be a function/, + ) + }) + + it('preserves callable and constructable bases', () => { + function Subject(this: { value?: number }, value: number): number { + if (new.target) { + this.value = value + return value + } + return value * 2 + } + const carrier = scopeTarget(Subject as typeof Subject & (new (value: number) => { value: number }), { + name: 'callable', + }) + + const called: unknown = Reflect.apply(carrier, { value: 0 }, [3]) + expect(called).toBe(6) + const instance = new carrier(4) + expect(instance).toBeInstanceOf(Subject) + expect(instance.value).toBe(4) + const prototypeDescriptor = Object.getOwnPropertyDescriptor(carrier, 'prototype') + const subjectPrototype: unknown = Reflect.get(Subject, 'prototype') + expect(prototypeDescriptor?.configurable).toBe(true) + expect(prototypeDescriptor?.value).toBe(subjectPrototype) + class Derived extends carrier {} + const derived = new Derived(5) + expect(derived).toBeInstanceOf(Derived) + expect(derived).toBeInstanceOf(Subject) + expect(derived.value).toBe(5) + expect(isScopeCarrier(carrier)).toBe(true) + }) + + it('matches non-constructable and bound-constructor function shapes', () => { + const arrow = (value: number): number => value + 1 + const arrowCarrier = scopeTarget(arrow, { name: 'arrow' }) + const arrowResult: unknown = Reflect.apply(arrowCarrier, undefined, [2]) + expect(arrowResult).toBe(3) + expect('prototype' in arrowCarrier).toBe(false) + expect(Object.getOwnPropertyDescriptor(arrowCarrier, 'prototype')).toBeUndefined() + expect(() => { Reflect.construct(arrowCarrier, []) }).toThrow(TypeError) + + class Subject { + constructor(readonly value: number) {} + } + const bound = Subject.bind(undefined, 7) + const boundCarrier = scopeTarget(bound, { name: 'bound-constructor' }) + expect('prototype' in boundCarrier).toBe(false) + expect(Object.getOwnPropertyDescriptor(boundCarrier, 'prototype')).toBeUndefined() + const instance = new boundCarrier() + expect(instance).toBeInstanceOf(Subject) + expect(instance.value).toBe(7) + }) + + it('detects construction without reading a hostile base prototype', () => { + class Subject { + constructor(readonly value: number) {} + } + let prototypeReads = 0 + const hostile = new Proxy(Subject, { + get(target, prop, receiver) { + if (prop === 'prototype') { + prototypeReads += 1 + throw new Error('hostile prototype getter') + } + return Reflect.get(target, prop, receiver) as unknown + }, + }) + + const carrier = scopeTarget(hostile, { name: 'hostile-constructor' }) + expect(prototypeReads).toBe(0) + const instance: unknown = Reflect.construct(carrier, [9], Subject) + expect(instance).toBeInstanceOf(Subject) + expect(instance).toMatchObject({ value: 9 }) + expect(prototypeReads).toBe(0) }) it('keeps the real constructor: class identity survives the carrier', () => { diff --git a/packages/core/session/README.md b/packages/core/session/README.md index 36052d9e7a..29f89e35c8 100644 --- a/packages/core/session/README.md +++ b/packages/core/session/README.md @@ -4,7 +4,7 @@ Event-sourced session log and in-memory store. A `Session` is the append-only so ## Service: `SessionStore` (ctx key: `sessions`) -Creates and holds event-sourced `Session` instances. Persistence is intentionally not implemented here — plugins subscribe to `session/event` and flush on `session/flush`. +Creates and holds event-sourced `Session` instances. Persistence is intentionally not implemented here — plugins subscribe to `session/event`, flush on `session/flush`, and may mirror the paired `session/created`/`session/disposed` lifecycle. ### Public API @@ -16,17 +16,18 @@ Creates and holds event-sourced `Session` instances. Persistence is intentionall #### Advanced: ordered-teardown lifecycle primitives -`create()` covers the common case (the session is owned by the calling fiber). When a session must be torn down **in order with another resource** — so a final flush is captured before `onAppend` detaches — `create()`'s self-contained effect is wrong, because a fiber unload disposes sibling effects *concurrently*. For that, split the lifecycle and fold it into the owner's single effect: +`create()` covers the common case (the session is owned by the calling fiber). When a session must be torn down **in order with another resource** — so a final flush is captured before the store-owned append observer detaches — `create()`'s self-contained effect is wrong, because a fiber unload disposes sibling effects *concurrently*. For that, split the lifecycle and fold it into the owner's single effect: - `ctx.sessions.prepare(id?, options?): Session` — read `options.seed`/`options.meta` once, validate and detach the metadata/header, and construct the `Session` WITHOUT entering it into the store. Same options as `create`. -- `ctx.sessions.enter(session): () => void` — wire `onAppend` → `session/event`, capture its scope carrier, and add the session to the store; returns the idempotent DETACH disposer, which clears both notification and carrier state. Does NOT emit `session/created` (the caller installs the disposer first, then calls `announce`, so a throwing listener rolls the attach back). It re-checks the id because public `prepare`/`enter` calls may be interleaved; a stale prepared object must not overwrite a live same-id session. -- `ctx.sessions.announce(session): void` — emit `session/created` for an entered session. +- `ctx.sessions.reserve(id): SessionRegistrationReservation` — hold an unpublished id under the calling fiber and construct its one owned Session through `reservation.prepare(options?)`. Until `release()` or owner unload, bare `prepare`/`create`/`enter` calls for that id reject; the factory later presents the exact capability to `enter`, making setup-time publication structurally impossible without leaking an abandoned reservation across HMR disposal. +- `ctx.sessions.enter(session, reservation?): () => void` — install the module-private `session/event` observer, capture its scope carrier, and add the session under one accepted id; returns the idempotent DETACH disposer, which clears notification, carrier, and accepted-key state. Does NOT emit `session/created` (the caller installs the disposer first, then calls `announce`, so a throwing listener rolls the attach back). It re-checks the id because public `prepare`/`enter` calls may be interleaved; a stale prepared object must not overwrite a live same-id session. A factory passes the opaque capability from `reserve(id)` so setup cannot enter the reserved session or publish a same-id replacement before the owning transaction. +- `ctx.sessions.announce(session): void` — begin the one allowed `session/created` announcement for an entered session; repeat and reentrant calls reject before dispatch. Its detach emits `session/disposed` exactly once, including rollback after a partially delivered creation notification; a never-announced entry emits neither edge. `dsh-agent-loop` is the canonical consumer: after unpublished agent setup it enters both session and agent before announcing either, then nests loop stop, agent removal, session detach, and scope unwind in one ordered lifecycle. The final flush therefore settles before this package detaches the session, whether teardown starts from an `AgentHandle` or owner-fiber unload. ### Live service events -The store announces creation, publishes each append, and provides an awaited durability checkpoint. Exact `session/*` signatures, modes, and scope-carrier behavior live in the generated [Cordis event catalog](../../../docs/cordis-catalog/events.md); the append-only payload vocabulary is separately generated into the [persistence catalog](../../../docs/persistence-catalog.md). Persistence consumers write behind from the append notification and drain on the store-owned flush entry point rather than dispatching the event directly. +The store pairs announced creation with disposal, publishes each append, and provides an awaited durability checkpoint. Disposal listener failures, including returned-promise rejections, are contained per observer so teardown cannot be interrupted. Exact `session/*` signatures, modes, and scope-carrier behavior live in the generated [Cordis event catalog](../../../docs/cordis-catalog/events.md); the append-only payload vocabulary is separately generated into the [persistence catalog](../../../docs/persistence-catalog.md). Persistence consumers write behind from the append notification and drain on the store-owned flush entry point rather than dispatching the event directly. ### Class: `Session` @@ -37,8 +38,8 @@ Plain class (not a Cordis Service). Create via `ctx.sessions.create()`. - `session.deriveEventMessage(event): Message | null` — the per-event projection `deriveMessages()` folds: one event's derived message (an unfrozen clone), or `null` when it produces none (a non-surface event, or an empty-content `assistant/message` hosting only usage). External reconstructors and the dev invariant fold the same function over a log prefix's surface, so no two paths can disagree about what a request's messages were (the reconstructability RFC). - `session.surface: SurfaceManager` — the derived surface, lazily rebuilt from `surfaceOp` markers in the log. Processes only new events (delta) on each access — the log is append-only, so prior events never change. `surface.replaceGeneration` is the rewrite signal: bumped by every folded `replace` and by `invalidate()`, never reset, so an incremental consumer comparing generations cannot be fooled. - `session.events` — a cached, frozen array snapshot over deep-frozen events. Repeated reads without an append return the same array; an append invalidates the cache and the next read returns a new snapshot, while earlier snapshots stay unchanged. Neither a cast nor a retained reference can push into the live log or rewrite an accepted event. -- `session.seq`, `session.id` -- `session.header: SessionHeader` — detached, deep-frozen creation metadata (`version`, `id`, `createdAt`, optional `cwd`/`parentSession`/`seedLength`). Construction validates its lossless-JSON shape and requires the header id to match `session.id`, so a caller cannot later mutate persistence routing or lineage through an aliased header. Kept out of the event log (a storage concern, not replayable state); a minimal header (stamped with the current `SESSION_FORMAT_VERSION`) is synthesized for bare `Session` construction. +- `session.seq`, `session.id` — `id` is a non-writable, non-configurable runtime identity slot, not merely TypeScript-readonly. +- `session.header: SessionHeader` — detached, deep-frozen creation metadata (`version`, `id`, `createdAt`, optional `cwd`/`parentSession`/`seedLength`) published through a non-writable, non-configurable slot. Construction validates its lossless-JSON shape and requires the header id to match `session.id`, so a caller cannot later replace or mutate persistence routing or lineage. Kept out of the event log (a storage concern, not replayable state); a minimal header (stamped with the current `SESSION_FORMAT_VERSION`) is synthesized for bare `Session` construction. ### Lossless JSON utilities diff --git a/packages/core/session/src/index.ts b/packages/core/session/src/index.ts index 51500e9e1b..b000e7083b 100644 --- a/packages/core/session/src/index.ts +++ b/packages/core/session/src/index.ts @@ -34,7 +34,10 @@ declare module 'cordis' { interface Events { /** - * A session was created in the store. + * A session was created in the store. A synchronous listener throw vetoes + * publication and rollback emits the matching `session/disposed` edge; + * returned-promise rejection is observed and logged but cannot retroactively + * veto this synchronous boundary. * Scope-filtered dispatch (`@deepseek-ai/dsh-scope`): the carrier is the * session's owner scope, captured when the session was ENTERED (an agent's * session is entered through `agent.ctx`, so its events dispatch in that @@ -45,6 +48,18 @@ declare module 'cordis' { * @mode emit */ 'session/created'(this: Scoped, session: Session): void + /** + * A previously announced session left the store. Emitted exactly once on + * normal detach or publication rollback, and never for a prepared/entered + * session whose `session/created` announcement did not begin. Listener + * failures (including returned-promise rejections) are logged and contained + * per listener so teardown always reaches quiescence. + * Scope-filtered dispatch uses the same owner carrier captured at entry; + * agent-scoped listeners hear only their own session's teardown. + * @param session - the session that is no longer live in the store. + * @mode emit + */ + 'session/disposed'(this: Scoped, session: Session): void /** * An event was appended to a session log (sync, fire-and-forget). This is * the per-append feed a UI or invariant plugin tails. @@ -253,6 +268,17 @@ function assertSessionEventEnvelope(value: Record, index: numbe } } +/** Render an arbitrary thrown value without allowing coercion to throw again. */ +function renderThrown(value: unknown): string { + try { + return value instanceof Error ? `${value.name}: ${value.message}` : String(value) + } catch { + return '' + } +} + +const appendObservers = new WeakMap void>() + /** * An event-sourced session: an append-only log of {@link SessionEvent}s. * @@ -261,8 +287,6 @@ function assertSessionEventEnvelope(value: Record, index: numbe */ export class Session { private log: SessionEvent[] = [] - /** Set by the store so appends are observable; undefined when detached. */ - onAppend: ((event: SessionEvent) => void) | undefined /** * Derived surface — a cached linked list of message-producing events. @@ -336,6 +360,14 @@ export class Session { }) } this.header = snapshotSessionHeader(id, header) + // TypeScript readonly prevents ordinary typed assignment only. Pin both + // public identity bindings at runtime too: setup/plugins receive the live + // Session object, and replacing either slot would split registry keys, + // persistence routing, and the already-validated header. + Object.defineProperties(this, { + id: { value: id, enumerable: true, writable: false, configurable: false }, + header: { value: this.header, enumerable: true, writable: false, configurable: false }, + }) } /** Cached immutable public snapshot of the private append-only log. */ @@ -359,8 +391,8 @@ export class Session { /** * Append one typed event to the log and synchronously notify observers via - * `onAppend`. The hot path never blocks on I/O — persistence plugins buffer - * asynchronously. + * the store-owned, module-private append observer. The hot path never blocks + * on I/O — persistence plugins buffer asynchronously. * * @param type - The event type (key of {@link SessionEventMap}). * @param data - The event payload; must be JSON-serializable. @@ -444,7 +476,7 @@ export class Session { const acceptedEvent = deepFreeze(event) this.log.push(acceptedEvent as unknown as SessionEvent) this.eventsSnapshot = undefined - this.onAppend?.(acceptedEvent as unknown as SessionEvent) + appendObservers.get(this)?.(acceptedEvent as unknown as SessionEvent) return acceptedEvent } @@ -599,6 +631,29 @@ export class SessionForkError extends Error { } } +/** + * Unforgeable ownership handle for one unpublished session id. A factory keeps + * this capability across load/setup, preventing setup code from entering the + * prepared Session or publishing a replacement under the same id. Obtain it + * only from {@link SessionStore.reserve}. + */ +export interface SessionRegistrationReservation { + /** The reserved store id. */ + readonly id: SessionId + /** + * Construct the one Session owned by this reservation. + * @param options - seed events and creation metadata. + * @returns the still-unpublished Session. + */ + prepare(options?: CreateSessionOptions): Session + /** + * Release the unpublished reservation; idempotent. The store also releases + * it automatically when the fiber that called `reserve` disposes. + * @returns nothing. + */ + release(): void +} + /** * In-memory session store (`ctx.sessions`). * @@ -607,6 +662,14 @@ export class SessionForkError extends Error { */ export class SessionStore extends Service { private store = new Map() + /** The one accepted map key for each live session; never reread caller state. */ + private acceptedIds = new WeakMap() + /** Sessions whose creation announcement began and therefore require a pair. */ + private announced = new WeakSet() + /** Unpublished identities held across factory load/setup transactions. */ + private reservations = new Map() + /** The exact prepared object owned by each reservation capability. */ + private reservedSessions = new WeakMap() /** * Each live session's dispatch carrier, captured at {@link enter} from the * ENTERING context's scope tag (an agent session is entered through @@ -621,6 +684,59 @@ export class SessionStore extends Service { super(ctx, 'sessions') } + /** + * Reserve one unpublished session id across an asynchronous factory + * transaction. Bare `prepare`/`create`/`enter` calls for the id reject until + * release; the capability constructs exactly one Session and is passed back + * to {@link enter} at publication. The reservation belongs to the calling + * fiber, so owner unload releases an abandoned id automatically. + * @param id - the session id the transaction will publish. + * @returns the opaque reservation capability. + * @throws if the id is malformed, live, or already reserved. + */ + reserve(id: SessionId): SessionRegistrationReservation { + if (typeof id !== 'string') throw new TypeError('session id must be a string') + if (this.store.has(id) || this.reservations.has(id)) { + throw new Error(`session "${id}" already exists or is reserved`) + } + let active = true + let prepared = false + const rawRelease = (): void => { + if (!active) return + active = false + this.reservedSessions.delete(reservation) + this.reservations.delete(id) + } + let disposeEffect!: () => Promise | void + const reservation: SessionRegistrationReservation = Object.freeze({ + id, + prepare: (options?: CreateSessionOptions) => { + if (!active) { + throw new Error(`session "${id}" reservation is no longer active`) + } + if (prepared) throw new Error(`session "${id}" reservation already prepared a session`) + prepared = true + const session = this.prepareReserved(id, options, reservation) + this.reservedSessions.set(reservation, session) + return session + }, + release: () => { + rawRelease() + // Remove the now-inert ownership effect on manual transaction settle; + // its cleanup is the exact idempotent raw release above. + void disposeEffect() + }, + }) + this.reservations.set(id, reservation) + try { + disposeEffect = this.ctx.effect(() => rawRelease, `sessions.reserve(${id})`) + } catch (error: unknown) { + rawRelease() + throw error + } + return reservation + } + /** * Create a session owned by the calling fiber: disposing that fiber stops * event notification and removes the session from the store. `options.seed` @@ -630,7 +746,7 @@ export class SessionStore extends Service { * fills `version`/`id`/`createdAt`). * * For an agent whose session must be torn down IN ORDER with its loop (so the - * loop's final flush is captured before `onAppend` detaches), do NOT use this + * loop's final flush is captured before the store-owned observer detaches), do NOT use this * — fold the session lifecycle into the agent's own effect via * {@link prepare} + {@link enter} + {@link announce} (see `dsh-agent-loop`'s * `startOwned`). @@ -647,7 +763,7 @@ export class SessionStore extends Service { // Single effect owned by the calling fiber. Yield the detach BEFORE // announcing so a throwing `session/created` listener rolls the attach back // (the generator effect disposes already-yielded disposers on a throw) - // instead of leaking the store entry + onAppend. + // instead of leaking the store entry + append observer. this.ctx.effect(function* (this: SessionStore) { yield this.enter(session) this.announce(session) @@ -661,7 +777,7 @@ export class SessionStore extends Service { * Pairs with {@link enter} + {@link announce}: a caller that owns a composite * `ctx.effect` (the agent factory) folds the session lifecycle into that ONE * effect so a fiber unload tears the session + agent down as a single ORDERED - * chain rather than as racing sibling effects — which would detach `onAppend` + * chain rather than as racing sibling effects — which would detach the append observer * before the loop's closing `session/flush`, dropping the closing events. * * @param id - the session id; omitted, the store mints `session-`. @@ -672,7 +788,27 @@ export class SessionStore extends Service { * non-absolute path. */ prepare(id?: SessionId, options?: CreateSessionOptions): Session { - const sessionId = SessionId(id ?? `session-${++this.counter}`) + return this.prepareReserved(id, options) + } + + /** Shared prepare implementation, optionally authorized by a reservation. */ + private prepareReserved( + id?: SessionId, + options?: CreateSessionOptions, + reservation?: SessionRegistrationReservation, + ): Session { + let sessionId: SessionId + if (id === undefined) { + do sessionId = SessionId(`session-${++this.counter}`) + while (this.store.has(sessionId) || this.reservations.has(sessionId)) + } else { + sessionId = SessionId(id) + } + if (typeof sessionId !== 'string') throw new TypeError('session id must be a string') + const held = this.reservations.get(sessionId) + if (reservation === undefined && held !== undefined) { + throw new Error(`session "${sessionId}" is reserved for unpublished creation`) + } if (this.store.has(sessionId)) throw new Error(`session "${sessionId}" already exists`) const seed = options?.seed const meta = snapshotSessionMeta(options?.meta) @@ -691,9 +827,9 @@ export class SessionStore extends Service { } /** - * Enter a {@link prepare}d session into the store: wire `onAppend` → - * `session/event` and add it to the store. Returns the DETACH disposer - * (`onAppend = undefined` + store removal). Does NOT emit `session/created` — + * Enter a {@link prepare}d session into the store: wire the module-private + * append observer to `session/event` and add it to the store. Returns the + * DETACH disposer (observer + store removal). Does NOT emit `session/created` — * the caller yields this disposer inside its effect and THEN calls * {@link announce}, so a throwing `session/created` listener rolls the attach * back instead of leaking it. @@ -707,11 +843,23 @@ export class SessionStore extends Service { * assume that. * * @param session - a {@link prepare}d session not yet in the store. - * @returns the detach disposer (`onAppend = undefined` + store removal). + * @param reservation - the exact unpublished-id capability when a factory + * reserved this session across setup. + * @returns the detach disposer (observer + store removal). * @throws if a session with this id is already in the store. */ - enter(session: Session): () => void { - if (this.store.has(session.id)) throw new Error(`session "${session.id}" already exists`) + enter(session: Session, reservation?: SessionRegistrationReservation): () => void { + const id = session.id + if (typeof id !== 'string') throw new TypeError('session id must be a string') + const held = this.reservations.get(id) + if (reservation === undefined) { + if (held !== undefined) throw new Error(`session "${id}" is reserved for unpublished creation`) + } else if (reservation.id !== id || held !== reservation + || this.reservedSessions.get(reservation) !== session) { + throw new Error(`session "${id}" registration reservation does not own this prepared session`) + } + if (this.store.has(id)) throw new Error(`session "${id}" already exists`) + if (appendObservers.has(session)) throw new Error(`session "${id}" is already attached to a store`) // The carrier is decided HERE, once, from the ENTERING context's scope tag // (`this.ctx` is the caller's context — the tracker mechanism): every // session/created|event|flush dispatch for this session uses it, so the @@ -720,24 +868,65 @@ export class SessionStore extends Service { const carrier = scopeTarget(session, scopeOf(this.ctx)) this.carriers.set(session, carrier) const emitCtx = this.ctx - session.onAppend = (event) => { emitCtx.emit(carrier, 'session/event', session, event) } - this.store.set(session.id, session) + appendObservers.set(session, (event) => { emitCtx.emit(carrier, 'session/event', session, event) }) + this.acceptedIds.set(session, id) + this.store.set(id, session) let entered = true return () => { if (!entered) return entered = false - session.onAppend = undefined + const wasAnnounced = this.announced.delete(session) + appendObservers.delete(session) + this.acceptedIds.delete(session) this.carriers.delete(session) - this.store.delete(session.id) + this.store.delete(id) + if (wasAnnounced) this.emitDisposed(session, carrier, id) } } - /** Emit `session/created` for an {@link enter}ed session (with the carrier - * {@link enter} captured). Separate from {@link enter} so the caller can - * yield the detach disposer first (rollback safety — see {@link enter}). - * @param session - the entered session to announce to listeners. */ + /** Emit `session/created` exactly once for an {@link enter}ed session (with + * the carrier {@link enter} captured). Separate from {@link enter} so the + * caller can yield the detach disposer first (rollback safety — see + * {@link enter}). + * @param session - the entered session to announce to listeners. + * @throws if the session is not live or its announcement already began, + * including a reentrant call from a creation listener. */ announce(session: Session): void { - this.ctx.emit(this.liveCarrierFor(session), 'session/created', session) + const carrier = this.liveCarrierFor(session) + if (this.announced.has(session)) { + throw new Error(`session "${session.id}" was already announced`) + } + // Mark before emit: Cordis emit may deliver to earlier listeners and then + // throw. Rollback must still pair that partial creation with disposal, and + // a listener cannot recursively create a second lifecycle edge. + this.announced.add(session) + const args: unknown[] = [carrier, 'session/created', session] + for (const callback of this.ctx.events.dispatch('emit', args)) { + // Synchronous throws intentionally propagate and veto publication; the + // yielded detach then emits the paired disposal edge. An async function + // is nevertheless assignable to a void listener, so observe its returned + // promise: rejection is too late to roll back and must be logged instead + // of becoming unhandled. + const returned: unknown = callback(...args) + void Promise.resolve(returned).catch((error: unknown) => { + this.ctx.logger.warn(`session "${session.id}": session/created listener rejected: ${renderThrown(error)}`) + }) + } + } + + /** Emit the paired teardown notification with per-listener containment. */ + private emitDisposed(session: Session, carrier: Scoped, id: SessionId): void { + const args: unknown[] = [carrier, 'session/disposed', session] + for (const callback of this.ctx.events.dispatch('emit', args)) { + try { + const returned: unknown = callback(...args) + void Promise.resolve(returned).catch((error: unknown) => { + this.ctx.logger.warn(`session "${id}": session/disposed listener rejected: ${renderThrown(error)}`) + }) + } catch (error: unknown) { + this.ctx.logger.warn(`session "${id}": session/disposed listener threw: ${renderThrown(error)}`) + } + } } /** @@ -756,8 +945,9 @@ export class SessionStore extends Service { /** Return the exact live session's carrier; detached/prepared objects reject. */ private liveCarrierFor(session: Session): Scoped { - if (this.store.get(session.id) !== session) { - throw new Error(`session "${session.id}" is not live in this store`) + const id = this.acceptedIds.get(session) + if (id === undefined || this.store.get(id) !== session) { + throw new Error(`session "${id ?? session.id}" is not live in this store`) } const carrier = this.carriers.get(session) // enter() installs store + carrier in one synchronous sequence; a live @@ -765,7 +955,7 @@ export class SessionStore extends Service { // to subject-less dispatch (that would silently cross scope boundaries). /* v8 ignore next -- enter installs store and carrier in one synchronous sequence */ if (carrier === undefined) { - throw new Error(`session "${session.id}" has no dispatch carrier`) + throw new Error(`session "${id}" has no dispatch carrier`) } return carrier } diff --git a/packages/core/session/tests/scoped.spec.ts b/packages/core/session/tests/scoped.spec.ts index ee1e710397..80dd8510d2 100644 --- a/packages/core/session/tests/scoped.spec.ts +++ b/packages/core/session/tests/scoped.spec.ts @@ -60,6 +60,23 @@ describe('session dispatch carriers', () => { bare.append('turn/start', { turn: 1, trigger: { kind: 'message', source: { kind: 'user' } } }) expect(heard).toEqual(['global:turn/start']) }) + + it('reuses the captured owner carrier for the paired disposal notification', async () => { + const ctx = await mount() + const owner = await mintScope(ctx, 'owner') + const other = await mintScope(ctx, 'other') + const heard: string[] = [] + ctx.on('session/disposed', (session) => { heard.push(`global:${session.id}`) }) + owner.ctx.on('session/disposed', (session) => { heard.push(`owner:${session.id}`) }) + other.ctx.on('session/disposed', (session) => { heard.push(`other:${session.id}`) }) + + const session = owner.ctx.sessions.prepare() + const detach = owner.ctx.sessions.enter(session) + owner.ctx.sessions.announce(session) + detach() + + expect(heard).toEqual([`global:${session.id}`, `owner:${session.id}`]) + }) }) describe('sessions.flush()', () => { diff --git a/packages/core/session/tests/session.spec.ts b/packages/core/session/tests/session.spec.ts index f7037609e6..ac0f06123c 100644 --- a/packages/core/session/tests/session.spec.ts +++ b/packages/core/session/tests/session.spec.ts @@ -578,6 +578,17 @@ describe('Session', () => { expect(session.header).not.toBe(input) expect(Object.isFrozen(session.header)).toBe(true) expect(Reflect.set(session.header, 'cwd', '/published-mutated')).toBe(false) + expect(Reflect.set(session, 'id', SessionId('redirected'))).toBe(false) + expect(Reflect.set(session, 'header', input)).toBe(false) + expect(Object.getOwnPropertyDescriptor(session, 'id')).toMatchObject({ + configurable: false, + writable: false, + }) + expect(Object.getOwnPropertyDescriptor(session, 'header')).toMatchObject({ + configurable: false, + writable: false, + }) + expect(session.id).toBe('header-owned') expect(session.header.cwd).toBe('/accepted') }) @@ -691,6 +702,10 @@ describe('SessionStore', () => { const session = ctx.sessions.create() expect(created).toEqual([session]) + // The store-owned append observer is module-private. A JavaScript caller + // may create an unrelated property with the old implementation's name, + // but cannot suppress the durable event feed. + expect(Reflect.set(session, 'onAppend', undefined)).toBe(true) session.append('user/message', { content: [{ type: 'text', text: 'x' }], source: { kind: 'user' } }, { surfaceOp: 'append' }) expect(events).toHaveLength(1) expect(events[0]![0]).toBe(session) @@ -746,6 +761,114 @@ describe('SessionStore', () => { expect(ctx.sessions.get(SessionId('lifecycle'))).toBeUndefined() }) + it('captures the accepted id once and prevents simultaneous attachment to two stores', async () => { + const firstCtx = new Context() + const secondCtx = new Context() + await firstCtx.plugin(SessionStore) + await secondCtx.plugin(SessionStore) + const session = new Session(SessionId('owned-key')) + const detachFirst = firstCtx.sessions.enter(session) + + expect(Reflect.set(session, 'id', SessionId('redirected'))).toBe(false) + expect(() => secondCtx.sessions.enter(session)).toThrow(/already attached to a store/) + expect(firstCtx.sessions.get(SessionId('owned-key'))).toBe(session) + + detachFirst() + expect(firstCtx.sessions.get(SessionId('owned-key'))).toBeUndefined() + const detachSecond = secondCtx.sessions.enter(session) + expect(secondCtx.sessions.get(SessionId('owned-key'))).toBe(session) + detachSecond() + + expect(() => firstCtx.sessions.enter({ id: 42 } as unknown as Session)).toThrow(/id must be a string/) + }) + + it('uses an opaque one-session reservation to gate unpublished factory insertion', async () => { + const ctx = new Context() + await ctx.plugin(SessionStore) + const held = ctx.sessions.reserve(SessionId('held-session')) + + expect(() => ctx.sessions.reserve(SessionId('held-session'))).toThrow(/already exists or is reserved/) + expect(() => ctx.sessions.prepare(SessionId('held-session'))).toThrow(/reserved for unpublished creation/) + expect(() => ctx.sessions.create(SessionId('held-session'))).toThrow(/reserved for unpublished creation/) + const session = held.prepare({ meta: { cwd: '/held' } }) + expect(() => held.prepare()).toThrow(/already prepared/) + expect(() => ctx.sessions.enter(session)).toThrow(/reserved for unpublished creation/) + + const other = ctx.sessions.reserve(SessionId('other-session')) + expect(() => ctx.sessions.enter(session, other)).toThrow(/does not own this prepared session/) + expect(() => ctx.sessions.enter(new Session(SessionId('held-session')), held)) + .toThrow(/does not own this prepared session/) + + const detach = ctx.sessions.enter(session, held) + ctx.sessions.announce(session) + held.release() + held.release() + expect(ctx.sessions.get(SessionId('held-session'))).toBe(session) + expect(() => ctx.sessions.reserve(SessionId('held-session'))).toThrow(/already exists or is reserved/) + detach() + other.release() + + const expired = ctx.sessions.reserve(SessionId('expired-session')) + expired.release() + expect(() => expired.prepare()).toThrow(/no longer active/) + expect(() => ctx.sessions.enter(new Session(SessionId('expired-session')), expired)) + .toThrow(/does not own this prepared session/) + expect(() => ctx.sessions.reserve(42 as unknown as SessionId)).toThrow(/id must be a string/) + expect(() => ctx.sessions.prepare(42 as unknown as SessionId)).toThrow(/id must be a string/) + + // Auto-generated ids skip unpublished reservations just as they skip live + // store entries; no hidden collision can be published later. + const firstAuto = ctx.sessions.reserve(SessionId('session-1')) + expect(ctx.sessions.prepare().id).toBe('session-2') + firstAuto.release() + }) + + it('owns reservations by the calling fiber and rolls back failed ownership registration', async () => { + const ctx = new Context() + await ctx.plugin(SessionStore) + let held!: import('@deepseek-ai/dsh-session').SessionRegistrationReservation + let scopedSessions!: SessionStore + const owner = await ctx.plugin(Object.assign((inner: Context) => { + scopedSessions = inner.sessions + held = inner.sessions.reserve(SessionId('fiber-held')) + }, { inject: ['sessions'] })) + + expect(() => ctx.sessions.reserve(SessionId('fiber-held'))).toThrow(/already exists or is reserved/) + await owner.dispose() + const reused = ctx.sessions.reserve(SessionId('fiber-held')) + reused.release() + held.release() // idempotent after the automatic owner-disposal release + + expect(() => scopedSessions.reserve(SessionId('inactive-owner'))).toThrow(/inactive context/) + const recovered = ctx.sessions.reserve(SessionId('inactive-owner')) + recovered.release() + }) + + it('rejects direct and reentrant repeat announcements to preserve one lifecycle pair', async () => { + const ctx = new Context() + await ctx.plugin(SessionStore) + let created = 0 + let disposed = 0 + let reentrantError = '' + ctx.on('session/created', (session) => { + created += 1 + try { + ctx.sessions.announce(session) + } catch (error: unknown) { + reentrantError = String(error) + } + }) + ctx.on('session/disposed', () => { disposed += 1 }) + + const session = ctx.sessions.prepare(SessionId('once')) + const detach = ctx.sessions.enter(session) + ctx.sessions.announce(session) + expect(reentrantError).toMatch(/already announced/) + expect(() => { ctx.sessions.announce(session) }).toThrow(/already announced/) + detach() + expect({ created, disposed }).toEqual({ created: 1, disposed: 1 }) + }) + it('synthesizes a minimal current-version header for a bare-created session', async () => { const ctx = new Context() await ctx.plugin(SessionStore) @@ -864,11 +987,13 @@ describe('SessionStore', () => { expect(observed).toBe(0) }) - it('rolls back the session (and onAppend) when a session/created listener throws (P1-1)', async () => { + it('pairs a partial session/created announcement with disposal during rollback', async () => { const ctx = new Context() await ctx.plugin(SessionStore) let threw = false + const disposed: Session[] = [] + ctx.on('session/disposed', (session) => { disposed.push(session) }) ctx.on('session/created', () => { if (!threw) { threw = true; throw new Error('boom created listener') } }) @@ -876,9 +1001,10 @@ describe('SessionStore', () => { // The throwing emit must roll the store entry back, not leak it. expect(() => ctx.sessions.create(SessionId('fixed'))).toThrow('boom created listener') expect(ctx.sessions.get(SessionId('fixed'))).toBeUndefined() // rolled back, not leaked + expect(disposed.map(session => session.id)).toEqual(['fixed']) // A subsequent create of the SAME id succeeds (the already-exists check is - // not wedged) and its onAppend is correctly wired (events observable). + // not wedged) and its store-owned observer is correctly wired (events observable). const events: SessionEvent[] = [] ctx.on('session/event', (_session, event) => void events.push(event)) const session = ctx.sessions.create(SessionId('fixed')) @@ -886,6 +1012,59 @@ describe('SessionStore', () => { session.append('user/message', { content: [{ type: 'text', text: 'hi' }], source: { kind: 'user' } }, { surfaceOp: 'append' }) expect(events).toHaveLength(1) }) + + it('observes async session/created rejection without rolling back or starving peers', async () => { + const ctx = new Context() + await ctx.plugin(SessionStore) + const warnings: string[] = [] + ctx.logger.warn = ((message: unknown) => { warnings.push(String(message)) }) as typeof ctx.logger.warn + const heard: string[] = [] + ctx.on('session/created', () => Promise.reject(new Error('late creation failure')) as never) + ctx.on('session/created', (session) => { heard.push(session.id) }) + + const session = ctx.sessions.create(SessionId('async-created')) + await Promise.resolve() + await Promise.resolve() + + expect(ctx.sessions.get(session.id)).toBe(session) + expect(heard).toEqual(['async-created']) + expect(warnings).toEqual([ + 'session "async-created": session/created listener rejected: Error: late creation failure', + ]) + }) + + it('contains synchronous and async session/disposed listener failures per observer', async () => { + const ctx = new Context() + await ctx.plugin(SessionStore) + const warnings: string[] = [] + ctx.logger.warn = ((message: unknown) => { warnings.push(String(message)) }) as typeof ctx.logger.warn + const hostile = { [Symbol.toPrimitive]() { throw new Error('cannot stringify') } } + const printable = { toString: () => 'printable failure' } + const heard: string[] = [] + ctx.on('session/disposed', () => { throw hostile }) + ctx.on('session/disposed', () => Promise.reject(new Error('async disposed')) as never) + ctx.on('session/disposed', () => { throw printable }) + ctx.on('session/disposed', (session) => { heard.push(session.id) }) + + const unannounced = ctx.sessions.prepare(SessionId('never-announced')) + const detachUnannounced = ctx.sessions.enter(unannounced) + detachUnannounced() + expect(heard).toEqual([]) + + const announced = ctx.sessions.prepare(SessionId('contained-disposal')) + const detach = ctx.sessions.enter(announced) + ctx.sessions.announce(announced) + expect(() => { detach() }).not.toThrow() + await Promise.resolve() + await Promise.resolve() + + expect(heard).toEqual(['contained-disposal']) + expect(warnings).toEqual([ + 'session "contained-disposal": session/disposed listener threw: ', + 'session "contained-disposal": session/disposed listener threw: printable failure', + 'session "contained-disposal": session/disposed listener rejected: Error: async disposed', + ]) + }) }) describe('todo/write event', () => { diff --git a/packages/core/system-prompt/README.md b/packages/core/system-prompt/README.md index 439ebebfc7..dd395b3f88 100644 --- a/packages/core/system-prompt/README.md +++ b/packages/core/system-prompt/README.md @@ -13,10 +13,10 @@ System prompt assembly registry. Plugins contribute ordered text sections, tool- ### Public API -- `ctx.systemPrompt.section(section: PromptSection): () => Promise | void` Contribute a section. The registry snapshots `name`, `order`, and the text value/callback, so later caller-object mutation cannot rename a stored section. The layer is the CALLING context's scope: `agent.ctx` contributes to that agent alone, SHADOWING a same-named global section there (the per-agent persona mechanism — a scoped `deployment:persona`). Duplicate names within one layer throw, and a globally protected section name cannot be shadowed. Disposed with the calling fiber. -- `ctx.systemPrompt.tools(provider: (context: AssembleContext) => ToolProviderResult): () => Promise | void` Contribute tool schemas, evaluated at each assembly with that assembly's context. `ToolProviderResult` = `{ schemas, knownNames? }`: `schemas` is the post-restriction visible set for `context.scope`; `knownNames` (defaulting to the same captured schemas' names) is the pre-restriction universe `toolOrder` validates against. Assembly reads the result, each schema field, and the optional known-name list once before detaching them, rejects non-string schema names/descriptions or known names, and uses those same accepted strings for validation and the model-visible collection. A provider must not return a schema named `TOOL_ORDER_REST`. Scoped providers are consulted only for their scope's assemblies. Disposed with the calling fiber. -- `ctx.systemPrompt.variable(name: string, provider: (context) => string | undefined): () => Promise | void` Contribute a prompt variable, referenced from section text as `{{name}}`. Scoped variables (via `agent.ctx`) shadow a same-named global for that agent. Duplicate-in-layer or unreferenceable names throw; `undefined` means "no value for this assembly". Disposed with the calling fiber. -- `ctx.systemPrompt.protect(protection: PromptProtection): () => Promise | void` Make named section/tool contributions authoritative after the assembly waterfall. Protection restores canonical registry/provider presence and definition; restored entries keep canonical order with one another and anchor before their first surviving later unprotected canonical neighbor (or at the end), without undoing listener reordering of unprotected entries. Canonical absence is authoritative too, so a mode-hidden tool cannot be fabricated by a listener. Calling through `agent.ctx` protects only that agent's assemblies. A global section protection additionally reserves its name against scoped shadows; registering either side of that conflict fails loudly instead of treating the shadow as canonical. Each input array is read once and snapshotted, empty protections throw, and disposal removes the protection. +- `ctx.systemPrompt.section(section: PromptSection): () => Promise | void` Contribute a section. The registry reads `name`, `order`, and the text value/callback once, validates their fixed string/finite-number/string-or-function types, and stores only that accepted record; later caller-object mutation cannot rename or reshape it. The layer is the CALLING context's scope: `agent.ctx` contributes to that agent alone, SHADOWING a same-named global section there (the per-agent persona mechanism — a scoped `deployment:persona`). Duplicate names within one layer throw, and a globally protected section name cannot be shadowed. Disposed with the calling fiber. +- `ctx.systemPrompt.tools(provider: (context: AssembleContext) => ToolProviderResult): () => Promise | void` Contribute tool schemas, evaluated at each assembly with that assembly's context; a non-function provider rejects before effect storage. `ToolProviderResult` = `{ schemas, knownNames? }`: `schemas` is the post-restriction visible set for `context.scope`; `knownNames` (defaulting to the same captured schemas' names) is the pre-restriction universe `toolOrder` validates against. Assembly reads the result, each schema field, and the optional known-name list once before detaching them, rejects non-string schema names/descriptions or known names, and uses those same accepted strings for validation and the model-visible collection. A provider must not return a schema named `TOOL_ORDER_REST`. Scoped providers are consulted only for their scope's assemblies. Disposed with the calling fiber. +- `ctx.systemPrompt.variable(name: string, provider: (context) => string | undefined): () => Promise | void` Contribute a prompt variable, referenced from section text as `{{name}}`. The fixed string name and function provider types reject before effect storage. Scoped variables (via `agent.ctx`) shadow a same-named global for that agent. Duplicate-in-layer or unreferenceable names throw; `undefined` means "no value for this assembly". Disposed with the calling fiber. +- `ctx.systemPrompt.protect(protection: PromptProtection): () => Promise | void` Make named section/tool contributions authoritative after the assembly waterfall. Protection restores canonical registry/provider presence and definition; restored entries keep canonical order with one another and anchor before their first surviving later unprotected canonical neighbor (or at the end), without undoing listener reordering of unprotected entries. Canonical absence is authoritative too, so a mode-hidden tool cannot be fabricated by a listener. Calling through `agent.ctx` protects only that agent's assemblies. A global section protection additionally reserves its name against scoped shadows; registering either side of that conflict fails loudly instead of treating the shadow as canonical. Each optional field and array slot is read once, non-array fields or non-string names reject before effect storage, and the accepted arrays are deduplicated and frozen. Finalization materializes each waterfall-produced entry name once, so a stateful getter cannot evade canonical replacement. Empty protections throw, and disposal removes the protection. - `ctx.systemPrompt.assemble(context?: AssembleContext): Promise` Assemble the prompt for one caller: the global layer merged with `context.scope`'s layer (scoped shadows global). Provider output becomes one coherent detached snapshot before `toolOrder` validation. Runs through the scope-filtered `system-prompt/assemble` waterfall, then restores protected contributions from the pre-waterfall canonical assembly. Rejects when a configured `toolOrder` names a tool outside the providers' `knownNames` universe (a restricted-away KNOWN tool is a normal absence), or when a provider returns the reserved rest-entry name. ### Live events diff --git a/packages/core/system-prompt/src/index.ts b/packages/core/system-prompt/src/index.ts index 8d3cc501a8..b810d07559 100644 --- a/packages/core/system-prompt/src/index.ts +++ b/packages/core/system-prompt/src/index.ts @@ -234,11 +234,41 @@ function orderTools(tools: ToolSchema[], toolOrder: string[] | undefined, knownN name === TOOL_ORDER_REST ? rest : tools.filter(tool => tool.name === name)) } +/** Snapshot one waterfall-produced named entry with a stable, own data `name`. */ +function snapshotNamedEntry(entry: T): { entry: T; name: string } { + // Read the name exactly once before protection matching. The waterfall owns + // its output and may return accessor-backed records; retaining such an entry + // would let a getter answer "unprotected" during filtering and the protected + // name later when a consumer reads the final assembly. + const name = entry.name + const snapshot: Record = {} + Object.defineProperty(snapshot, 'name', { + value: name, + enumerable: true, + configurable: true, + writable: true, + }) + // Copy every other enumerable field once while deliberately skipping name. + // defineProperty keeps a literal "__proto__" extension field ordinary data. + for (const key of Object.keys(entry)) { + if (key === 'name') continue + Object.defineProperty(snapshot, key, { + value: (entry as unknown as Record)[key], + enumerable: true, + configurable: true, + writable: true, + }) + } + return { entry: snapshot as T, name } +} + /** Restore protected named entries from `canonical`, anchored before their next unprotected canonical neighbor. */ function restoreProtected( canonical: readonly T[], result: readonly T[], protectedNames: ReadonlySet, ): T[] { - const restored = result.filter(entry => !protectedNames.has(entry.name)) + const restored = result + .map(snapshotNamedEntry) + .filter(record => !protectedNames.has(record.name)) for (const [index, entry] of canonical.entries()) { if (!protectedNames.has(entry.name)) continue // Protected entries are inserted in canonical order. Anchor each one @@ -251,9 +281,29 @@ function restoreProtected( .map(candidate => candidate.name), ) const next = restored.findIndex(candidate => following.has(candidate.name)) - restored.splice(next < 0 ? restored.length : next, 0, structuredClone(entry)) + restored.splice(next < 0 ? restored.length : next, 0, { + entry: structuredClone(entry), + name: entry.name, + }) } - return restored + return restored.map(record => record.entry) +} + +/** Validate and detach one protection-name array without rereading an element. */ +function snapshotProtectionNames(value: unknown, field: 'sections' | 'tools'): readonly string[] { + if (!Array.isArray(value)) { + throw new TypeError(`systemPrompt.protect() ${field} must be an array of strings`) + } + const names: string[] = [] + const length = value.length + for (let index = 0; index < length; index += 1) { + const name: unknown = value[index] + if (typeof name !== 'string') { + throw new TypeError(`systemPrompt.protect() ${field} must be an array of strings`) + } + names.push(name) + } + return Object.freeze([...new Set(names)]) } /** Lexicographic (code-unit) name comparison — locale-independent, so the order is identical on every machine. */ @@ -433,9 +483,10 @@ export class SystemPrompt extends Service { * `deployment:persona`) unless that global name is protected: global * protection reserves its section name against scoped shadows so the * registration owner—not a later scope—defines the canonical value. The - * registry snapshots `name`, `order`, and `text` before checking/storing, so - * later caller-object mutation cannot rename a contribution. Throws - * if the SAME layer already has the name (a + * registry reads `name`, `order`, and `text` once, validates their fixed + * string/finite-number/string-or-function types, and stores only that + * accepted record, so later caller-object mutation cannot rename or reshape + * a contribution. Throws if the SAME layer already has the name (a * duplicate would silently double prompt text — e.g. a double-loaded tool * plugin; the global-duplicate message names `agent.ctx` as the per-agent * alternative). Removed when the calling fiber is disposed. Emits @@ -446,12 +497,23 @@ export class SystemPrompt extends Service { * yield it directly — exact identity nests the teardown in order. */ section(section: PromptSection): () => Promise | void { - const scope = scopeOf(this.ctx) - const snapshot: PromptSection = { - name: section.name, - order: section.order, - text: section.text, + const input: unknown = section + if (typeof input !== 'object' || input === null) { + throw new TypeError('systemPrompt.section() requires a section object') } + const accepted = input as PromptSection + const name = accepted.name + const order = accepted.order + const text = accepted.text + if (typeof name !== 'string') throw new TypeError('prompt section name must be a string') + if (typeof order !== 'number' || !Number.isFinite(order)) { + throw new TypeError(`prompt section "${name}" order must be a finite number`) + } + if (typeof text !== 'string' && typeof text !== 'function') { + throw new TypeError(`prompt section "${name}" text must be a string or function`) + } + const scope = scopeOf(this.ctx) + const snapshot: PromptSection = { name, order, text } if (scope !== undefined && this.protections.some(record => record.sections?.includes(snapshot.name))) { throw new Error(`prompt section "${snapshot.name}" is globally protected and cannot be shadowed in an agent scope`) } @@ -498,7 +560,8 @@ export class SystemPrompt extends Service { * `schemas`/`knownNames` split). The layer is decided by the calling * context: a scoped provider (registered through `agent.ctx`) is consulted * only for that scope's assemblies. Removed when the calling fiber is - * disposed. A provider must not return a schema named + * disposed. A non-function provider is rejected before any effect is stored. + * A provider must not return a schema named * {@link TOOL_ORDER_REST}; that name is reserved for * {@link Config.toolOrder}'s rest entry and rejects the assembly. Emits * `system-prompt/change`. @@ -508,6 +571,9 @@ export class SystemPrompt extends Service { * yield it directly — exact identity nests the teardown in order. */ tools(provider: (context: AssembleContext) => ToolProviderResult): () => Promise | void { + if (typeof provider !== 'function') { + throw new TypeError('system prompt tool provider must be a function') + } const scope = scopeOf(this.ctx) const dispose = this.ctx.effect(function* (this: SystemPrompt) { const layer = scope === undefined @@ -545,10 +611,11 @@ export class SystemPrompt extends Service { * deployment must not claim facts it does not have). The layer is decided * by the calling context: a scoped variable (registered through * `agent.ctx`) resolves only for that scope's assemblies and SHADOWS a - * same-named global variable there. Throws on a name that does not match - * `[a-z][a-z0-9_]*` (it could never be referenced) or one already - * registered in the SAME layer. Removed when the calling fiber is disposed; - * emits `system-prompt/change` on register/unregister. + * same-named global variable there. The fixed name and callback types are + * validated before effect storage. Throws on a name that does not match + * `[a-z][a-z0-9_]*` (it could never be referenced) or one already registered + * in the SAME layer. 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. The exact @@ -556,11 +623,16 @@ export class SystemPrompt extends Service { * yield it directly — exact identity nests the teardown in order. */ variable(name: string, provider: (context: AssembleContext) => string | undefined): () => Promise | void { + const inputName: unknown = name + if (typeof inputName !== 'string') throw new TypeError('prompt variable name must be a string') + if (!VARIABLE_NAME.test(inputName)) { + throw new Error(`invalid prompt variable name "${inputName}" (must match ${String(VARIABLE_NAME)})`) + } + if (typeof provider !== 'function') { + throw new TypeError(`prompt variable "${inputName}" provider must be a function`) + } const scope = scopeOf(this.ctx) 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)})`) - } const layer = scope === undefined ? this.variableProviders : this.scopedVariableProviders.get(scope) ?? (() => { @@ -599,9 +671,13 @@ export class SystemPrompt extends Service { * restored AFTER the whole waterfall, so listener registration order cannot * strip, replace, duplicate, or fabricate it. Canonical absence is restored * too: if the protected name is intentionally absent for an assembly, a - * listener-injected entry with that name is removed. Each input array is - * read once and snapshotted; an empty protection throws because it cannot - * affect output. + * listener-injected entry with that name is removed. Each optional field and + * array slot is read once; non-array fields or non-string names reject before + * effect storage, and the accepted deduplicated arrays are frozen. During + * finalization each waterfall-produced entry name is likewise read once into + * an owned data record, so a stateful getter cannot look unprotected during + * filtering and later impersonate a protected name. An empty protection + * throws because it cannot affect output. * Removed with the calling fiber and emits `system-prompt/change` on * registration/unregistration. A global section protection also reserves the * name against scoped section shadows; registering protection when such a @@ -610,13 +686,24 @@ export class SystemPrompt extends Service { * @returns the exact Cordis effect disposer that removes the protection. */ protect(protection: PromptProtection): () => Promise | void { - const scope = scopeOf(this.ctx) - const sections = protection.sections - const tools = protection.tools - const snapshot: PromptProtection = { - ...sections !== undefined ? { sections: [...new Set(sections)] } : {}, - ...tools !== undefined ? { tools: [...new Set(tools)] } : {}, + const input: unknown = protection + if (typeof input !== 'object' || input === null) { + throw new TypeError('systemPrompt.protect() requires a protection object') } + const accepted = input as PromptProtection + const inputSections = accepted.sections + const inputTools = accepted.tools + const sections = inputSections === undefined + ? undefined + : snapshotProtectionNames(inputSections, 'sections') + const tools = inputTools === undefined + ? undefined + : snapshotProtectionNames(inputTools, 'tools') + const scope = scopeOf(this.ctx) + const snapshot: PromptProtection = Object.freeze({ + ...sections !== undefined ? { sections } : {}, + ...tools !== undefined ? { tools } : {}, + }) if ((snapshot.sections?.length ?? 0) === 0 && (snapshot.tools?.length ?? 0) === 0) { throw new Error('systemPrompt.protect() requires at least one section or tool name') } diff --git a/packages/core/system-prompt/tests/system-prompt.spec.ts b/packages/core/system-prompt/tests/system-prompt.spec.ts index 9eef467a8e..c37ddd9c6c 100644 --- a/packages/core/system-prompt/tests/system-prompt.spec.ts +++ b/packages/core/system-prompt/tests/system-prompt.spec.ts @@ -111,6 +111,86 @@ describe('SystemPrompt', () => { expect(contributed(assembly).map(s => s.text)).toEqual(['first']) }) + it('rejects malformed fixed registration fields before storing an effect', async () => { + const ctx = new Context() + await ctx.plugin(SystemPrompt) + const badName = { value: 'name' } + const badText = { value: 'text' } + + expect(() => ctx.systemPrompt.section(null as unknown as Parameters[0])) + .toThrow('requires a section object') + expect(() => ctx.systemPrompt.section(1 as unknown as Parameters[0])) + .toThrow('requires a section object') + expect(() => ctx.systemPrompt.section({ name: badName as unknown as string, order: 1, text: 'x' })) + .toThrow('prompt section name must be a string') + expect(() => ctx.systemPrompt.section({ name: 'bad-order', order: '1' as unknown as number, text: 'x' })) + .toThrow('order must be a finite number') + expect(() => ctx.systemPrompt.section({ name: 'bad-order', order: Number.NaN, text: 'x' })) + .toThrow('order must be a finite number') + expect(() => ctx.systemPrompt.section({ name: 'bad-text', order: 1, text: badText as unknown as string })) + .toThrow('text must be a string or function') + expect(() => ctx.systemPrompt.tools(1 as unknown as Parameters[0])) + .toThrow('tool provider must be a function') + expect(() => ctx.systemPrompt.variable({} as unknown as string, () => 'x')) + .toThrow('prompt variable name must be a string') + expect(() => ctx.systemPrompt.variable('valid', 1 as unknown as Parameters[1])) + .toThrow('provider must be a function') + expect(() => ctx.systemPrompt.protect(null as unknown as Parameters[0])) + .toThrow('requires a protection object') + expect(() => ctx.systemPrompt.protect(1 as unknown as Parameters[0])) + .toThrow('requires a protection object') + expect(() => ctx.systemPrompt.protect({ sections: 'x' as unknown as string[] })) + .toThrow('sections must be an array of strings') + expect(() => ctx.systemPrompt.protect({ tools: 'x' as unknown as string[] })) + .toThrow('tools must be an array of strings') + expect(() => ctx.systemPrompt.protect({ sections: ['ok', {} as unknown as string] })) + .toThrow('sections must be an array of strings') + expect(() => ctx.systemPrompt.protect({ tools: [{} as unknown as string] })) + .toThrow('tools must be an array of strings') + + expect(Object.isFrozen(badName)).toBe(false) + expect(Object.isFrozen(badText)).toBe(false) + expect(contributed(await ctx.systemPrompt.assemble())).toEqual([]) + }) + + it('reads each section field and protection-name slot once at registration', async () => { + const ctx = new Context() + await ctx.plugin(SystemPrompt) + const reads = { name: 0, order: 0, text: 0, sections: 0, item: 0 } + const section = Object.defineProperties({}, { + name: { + enumerable: true, + get: () => (++reads.name === 1 ? 'stable' : 42), + }, + order: { + enumerable: true, + get: () => (++reads.order === 1 ? 10 : Number.NaN), + }, + text: { + enumerable: true, + get: () => (++reads.text === 1 ? 'stable text' : null), + }, + }) as unknown as Parameters[0] + const names = new Array(1) + Object.defineProperty(names, 0, { + enumerable: true, + get: () => (++reads.item === 1 ? 'stable' : 'drifted'), + }) + const protection = { + get sections(): string[] { + reads.sections += 1 + return reads.sections === 1 ? names : ['drifted'] + }, + } + + ctx.systemPrompt.section(section) + ctx.systemPrompt.protect(protection) + const assembly = await ctx.systemPrompt.assemble() + + expect(reads).toEqual({ name: 1, order: 1, text: 1, sections: 1, item: 1 }) + expect(assembly.sections).toContainEqual({ name: 'stable', order: 10, text: 'stable text' }) + }) + it('rolls back a section when a system-prompt/change listener throws (P1-1)', async () => { const ctx = new Context() await ctx.plugin(SystemPrompt) @@ -282,6 +362,56 @@ describe('SystemPrompt', () => { expect(assembly.sections).toContainEqual({ name: 'protected', order: 10, text: 'canonical' }) }) + it('materializes waterfall entry names once before restoring protected definitions', async () => { + const ctx = new Context() + await ctx.plugin(SystemPrompt) + ctx.systemPrompt.section({ name: 'protected', order: 10, text: 'canonical section' }) + ctx.systemPrompt.tools(() => ({ schemas: [{ name: 'protected', description: 'canonical tool', parameters: {} }] })) + ctx.systemPrompt.protect({ sections: ['protected'], tools: ['protected'] }) + let sectionNameReads = 0 + let toolNameReads = 0 + const hostileSection = { + get name(): string { + sectionNameReads += 1 + return sectionNameReads === 1 ? 'impostor-section' : 'protected' + }, + order: 999, + text: 'listener section', + } + const hostileTool = { + get name(): string { + toolNameReads += 1 + return toolNameReads === 1 ? 'impostor-tool' : 'protected' + }, + description: 'listener tool', + parameters: {}, + } + ctx.on('system-prompt/assemble', async (_assembly, _context, next) => { + const result = await next() + result.sections = [ + ...result.sections.filter(section => section.name !== 'protected'), + hostileSection, + ] + result.tools = [ + ...result.tools.filter(tool => tool.name !== 'protected'), + hostileTool, + ] + return result + }) + + const assembly = await ctx.systemPrompt.assemble() + + expect(sectionNameReads).toBe(1) + expect(toolNameReads).toBe(1) + expect(assembly.sections.map(section => section.name)).toEqual([ + 'harness:identity', + 'deployment:persona', + 'impostor-section', + 'protected', + ]) + expect(assembly.tools.map(tool => tool.name)).toEqual(['impostor-tool', 'protected']) + }) + it('protects canonical absence and rejects an empty protection', async () => { const ctx = new Context() await ctx.plugin(SystemPrompt) diff --git a/packages/core/tools/README.md b/packages/core/tools/README.md index 0776ff39f6..74f1ff14c4 100644 --- a/packages/core/tools/README.md +++ b/packages/core/tools/README.md @@ -22,7 +22,7 @@ tools: - `ctx.tools.knownNames(scope?: ScopeKey): string[]` The PRE-restriction end-capability name universe `restrict` validates against: a typo fails loud while a restricted-away tool stays a normal absence. Presentation providers add reserved transport names separately when validating `toolOrder`. - `ctx.tools.schemas(scope?: ScopeKey): ToolSchema[]` Schemas of everything the scope can see (without the `execute` functions). The shipped tools' schemas are catalogued in [docs/tool-catalog.md](../../../docs/tool-catalog.md), generated by booting each tool plugin and harvesting this method (see [the tool-schema-catalog RFC](../../../docs/rfc/implemented/process/2026-07-02-tool-schema-catalog.md)). - `ctx.tools.guard(guard: ToolGuard): () => Promise | void` Register a monotonic synchronous execution guard after `tools/pre-execute`: returning a reason denies the call, while `undefined` leaves it unchanged. A plain-context guard applies globally; an `agent.ctx` guard applies only to that agent. Later waterfall listeners cannot turn a guard denial back into permission. Disposed with the calling fiber. -- `ctx.tools.execute(exec: ToolExecutionInput): Promise` Read each caller-owned top-level field once, snapshot the single-use call into a pipeline-owned execution, assign its opaque correlation token, materialize `arguments` through one lossless-JSON traversal, deep-freeze them, and protect identity before running `tools/pre-execute` → guards → `tools/execute` → `tools/post-execute`; optional `signal` is the only operational field an around-dispatch wrapper may add, replace, or remove. After the required `callId`/`name` correlation identity is captured, the same captured optional fields build the normalized error shell if a later accessor or validation fails, so policy, dispatch, routing, and `tools/result` cannot observe different caller values. Every top-level result field is likewise captured once and the complete result or post-decision is losslessly materialized before final observation. Invalid input—including cloneable mutable exotics—and malformed or non-JSON listener/tool results normalize to `isError` outcomes rather than bypassing policy or failing later at the session log. A throwing `callId` or `name` accessor rejects because no trustworthy result identity exists yet. +- `ctx.tools.execute(exec: ToolExecutionInput): Promise` Read each caller-owned top-level field once, require `callId` and `name` to yield strings, snapshot the single-use call into a pipeline-owned execution, assign its opaque correlation token, materialize `arguments` through one lossless-JSON traversal, deep-freeze them, and protect identity before running `tools/pre-execute` → guards → `tools/execute` → `tools/post-execute`; optional `signal` is the only operational field an around-dispatch wrapper may add, replace, or remove. After the required string correlation identity is captured, the same captured optional fields build the normalized error shell if a later accessor or validation fails, so policy, dispatch, routing, and `tools/result` cannot observe different caller values. Every top-level result field is likewise captured once and the complete result or post-decision is losslessly materialized before final observation. Invalid later input—including cloneable mutable exotics—and malformed or non-JSON listener/tool results normalize to `isError` outcomes rather than bypassing policy or failing later at the session log. A throwing accessor or non-string value in `callId` or `name` rejects before `tools/result` because no trustworthy result identity exists yet. ### Injected services @@ -35,7 +35,7 @@ The live registry pipeline has three transformable waterfalls followed by the ow ### Key types - `ToolDefinition` — `ToolSchema` + `execute(args, exec): Promise` (the bare array is the model-facing content; the object form additionally attaches an opaque, JSON-serializable `meta` presentation payload persisted on the `tool/result` event and handed back to `presentResult`), plus optional `presentCall(args)` / `presentResult(args, result)` for tool-owned UI presentation (see below). It also carries an optional cooperative timeout budget `timeoutMs?: number` (ms) enforced by `@deepseek-ai/dsh-timeout-policy`, never sent to the model. Registration stores a frozen snapshot with detached JSON parameters and once-bound callback identities. -- `ToolExecutionInput` — the caller-supplied call description: `{ callId, name, arguments, agent?, parent?, signal? }`; `arguments` must be losslessly JSON-serializable, and callers may pass an enclosing execution's opaque token as `parent` but never choose the new execution's own token. +- `ToolExecutionInput` — the caller-supplied call description: `{ callId, name, arguments, agent?, parent?, signal? }`; `callId` and `name` must be strings, `arguments` must be losslessly JSON-serializable, and callers may pass an enclosing execution's opaque token as `parent` but never choose the new execution's own token. - `ToolExecutionToken` — a frozen, property-free identity value assigned by the registry. It supports equality correlation only and exposes no live outer execution state. - `ToolExecution` — the pipeline-owned call: immutable `{ token, callId, name, arguments, agent?, parent? }` identity plus optional operational `signal`, which an around wrapper may add, replace, remove, and restore. A nested call's `parent` is a `ToolExecutionToken`, not an execution object. - `ToolExecutionResult` — losslessly JSON-serializable outcome: `{ callId, content, isError, error?, additionalContext?, meta? }`. The registry validates the complete post-policy value before final observation. On failure with a `HarnessError`, `error: { name, code }` carries the structured failure class alongside the model-facing text (the loop forwards it onto the `tool/result` session event for retry/sandbox plugins and replay). `additionalContext` (a `HookContext`) ferries any `tools/post-execute` context up to the loop, which buffers it and appends it as a `context/message` after all `tool/result`s in the step. `meta` is the tool's opaque presentation payload from a successful `execute` (the object return form); the loop forwards it onto the `tool/result` session event for result-card rendering. diff --git a/packages/core/tools/src/index.ts b/packages/core/tools/src/index.ts index b466875c75..d757f6cffa 100644 --- a/packages/core/tools/src/index.ts +++ b/packages/core/tools/src/index.ts @@ -951,22 +951,33 @@ export class ToolRegistry extends Service { * Caller-owned arguments are validated and detached in one recursive * lossless-JSON traversal; a violation normalizes to an error before policy * or dispatch. - * @param exec - the single-use call input; every top-level field is read once - * and that identity snapshot is protected before policy runs (and reused by - * the normalized error shell if validation fails). - * @returns the final result after every waterfall. Once the required - * `callId` and `name` correlation identity has been captured, later - * accessor, validation, listener, and tool failures resolve as `isError` - * results rather than rejections. A throwing `callId` or `name` accessor - * rejects because no trustworthy result identity exists yet. + * @param exec - the single-use call input; every top-level field is read once. + * `callId` and `name` must each yield a string before that identity snapshot + * is protected and policy begins. + * @returns the final result after every waterfall. Once the required string + * `callId` and `name` correlation identity has been captured, later accessor, + * validation, listener, and tool failures resolve as `isError` results rather + * than rejections. A throwing accessor or non-string value in either identity + * field rejects because no trustworthy result correlation exists yet. */ async execute(exec: ToolExecutionInput): Promise { // callId/name are the minimum correlation identity needed to construct a - // result at all. Every other caller-controlled accessor is read once - // INSIDE the normalization boundary; if one throws, the error shell uses - // the fields captured before it and never rereads the hostile record. + // result at all. Capture each once, then validate the captured scalar before + // anything can treat it as a trustworthy identity. A JavaScript/casted + // caller that supplies another type rejects at this outer boundary: an error + // result carrying the same malformed value would not satisfy the correlation + // contract and might itself fail lossless-JSON materialization. Every other + // caller-controlled accessor is read once INSIDE the normalization boundary; + // if one throws, the error shell uses the fields captured before it and never + // rereads the hostile record. const callId = exec.callId const name = exec.name + if (typeof callId !== 'string') { + throw new TypeError('tool execution callId must be a string') + } + if (typeof name !== 'string') { + throw new TypeError('tool execution name must be a string') + } let agent: Agent | undefined let parent: ToolExecutionToken | undefined let signal: AbortSignal | undefined diff --git a/packages/core/tools/tests/tools.spec.ts b/packages/core/tools/tests/tools.spec.ts index 5bff94c960..1b83a8f633 100644 --- a/packages/core/tools/tests/tools.spec.ts +++ b/packages/core/tools/tests/tools.spec.ts @@ -7,7 +7,7 @@ import ApprovalService, { type ApprovalOutcome, type ApprovalRequest } from '@de import ToolRegistry, { defineTool, schemaSpecToJsonSchema, validateArgs, ToolArgsError, ToolNotFoundError, type DefineToolOptions, type InferArgs, type SchemaSpec, type PreToolDecision, type PostToolDecision, - type ToolDefinition, type ToolExecution, type ToolExecutionResult, type ToolGuard, + type ToolDefinition, type ToolExecution, type ToolExecutionInput, type ToolExecutionResult, type ToolGuard, } from '@deepseek-ai/dsh-tools' async function setup() { @@ -83,6 +83,95 @@ describe('ToolRegistry', () => { expect(result).toEqual({ callId: CallId('c1'), content: [{ type: 'text', text: 'hi' }], isError: false }) }) + it.each([ + { field: 'callId', value: 1n }, + { field: 'callId', value: 123 }, + { field: 'name', value: 1n }, + { field: 'name', value: 123 }, + ] as const)('rejects a non-string $field before final observation', async ({ field, value }) => { + const ctx = await setup() + let observed = 0 + ctx.on('tools/result', () => { observed += 1 }) + const input: Record = { + callId: CallId('valid-call'), + name: 'missing', + arguments: {}, + } + input[field] = value + + await expect(ctx.tools.execute(input as unknown as ToolExecutionInput)) + .rejects.toThrow(`tool execution ${field} must be a string`) + expect(observed).toBe(0) + }) + + it('reads correlation accessors once and normalizes a later hostile accessor', async () => { + const ctx = await setup() + const reads = { callId: 0, name: 0, arguments: 0 } + let observed: { callId: unknown; name: unknown; isError: boolean } | undefined + ctx.on('tools/result', (exec, result) => { + observed = { callId: exec.callId, name: exec.name, isError: result.isError } + }) + const input = Object.defineProperties({}, { + callId: { + enumerable: true, + get: () => { + reads.callId += 1 + if (reads.callId > 1) throw new Error('callId reread') + return CallId('one-read-call') + }, + }, + name: { + enumerable: true, + get: () => { + reads.name += 1 + if (reads.name > 1) throw new Error('name reread') + return 'missing' + }, + }, + arguments: { + enumerable: true, + get: () => { + reads.arguments += 1 + throw new Error('arguments accessor broke') + }, + }, + }) as unknown as ToolExecutionInput + + const result = await ctx.tools.execute(input) + + expect(reads).toEqual({ callId: 1, name: 1, arguments: 1 }) + expect(result).toMatchObject({ callId: CallId('one-read-call'), isError: true }) + expect(result.content[0]).toMatchObject({ text: 'Error: arguments accessor broke' }) + expect(observed).toEqual({ callId: CallId('one-read-call'), name: 'missing', isError: true }) + }) + + it('reads callId once before a hostile name accessor rejects correlation', async () => { + const ctx = await setup() + const reads = { callId: 0, name: 0 } + let observed = 0 + ctx.on('tools/result', () => { observed += 1 }) + const input = Object.defineProperties({ arguments: {} }, { + callId: { + enumerable: true, + get: () => { + reads.callId += 1 + return CallId('hostile-name') + }, + }, + name: { + enumerable: true, + get: () => { + reads.name += 1 + throw new Error('name accessor broke') + }, + }, + }) as unknown as ToolExecutionInput + + await expect(ctx.tools.execute(input)).rejects.toThrow('name accessor broke') + expect(reads).toEqual({ callId: 1, name: 1 }) + expect(observed).toBe(0) + }) + it('threads a tool-attached meta (object return form) onto the result', async () => { const ctx = await setup() ctx.tools.register({ diff --git a/packages/subagent/subagent-inprocess/README.md b/packages/subagent/subagent-inprocess/README.md index e9f7beb540..e5b40ccfdc 100644 --- a/packages/subagent/subagent-inprocess/README.md +++ b/packages/subagent/subagent-inprocess/README.md @@ -9,11 +9,11 @@ The shared **in-process subagent run driver**. A library with no provider or imp Runs a child as a child [`Agent`](../../core/agent) on the same cordis context (`ctx.agents`): 1. reads every public request and seed field once before asynchronous owner setup: the parent and signal remain identity capabilities, while tool filter, seed, agent options, output schema, and prompt are each materialized by the shared one-pass lossless-JSON snapshot. It computes child depth = `depthOf(parent) + 1`, rejects `request.maxDepth` overflow with `SubagentDepthError`, reports an invalid schema as `OutputSchemaError`, and derives both the child prefix and `seedLength` from the same detached seed; -2. first installs provider ownership, then attaches the request abort listener and creates one run-owner Cordis fiber under `parent.ctx`; an already-unloading provider therefore leaves no child or orphaned listener. Async child creation goes through that fiber's `ctx.agents` service with fresh IDs, lineage/seed, inherited model, and an unpublished setup transaction for persona, tool restriction, and structured output. Parent teardown, provider teardown, and manual `run.dispose()` all dispose this exact node, preventing publication after it becomes inactive and awaiting the same quiescence boundary. `startInProcessRun` still returns its `SubagentRun` immediately: `run.started` resolves only after `ctx.agents.create()` has published the child (and rejects if publication never happens), while cancellation during creation is recorded and applied when a child exists; +2. first installs provider ownership, then attaches the request abort listener and creates one run-owner Cordis fiber under `parent.ctx`; an already-unloading provider therefore leaves no child or orphaned listener. Async child creation goes through that fiber's `ctx.agents` service with fresh IDs, lineage/seed, inherited model, and an unpublished setup transaction for persona, tool restriction, and structured output. Parent teardown, provider teardown, manual `run.dispose()`, and cancellation before readiness all dispose this exact node, preventing publication after it becomes inactive and sharing the same quiescence boundary. `startInProcessRun` still returns its `SubagentRun` immediately: `run.started` resolves only after `ctx.agents.create()` has published the child and rejects when pre-readiness cancellation rolls the transaction back; 3. drives the one-shot: `child.send(prompt)` then `await child.whenIdle()` (ordering matters — `send` enqueues synchronously, so `whenIdle` observes the queued work and resolves on the child's `running → idle` transition, never before the turn starts); there is deliberately NO re-prompt for a structured child that finished cleanly without calling `structured_output` — the shortfall maps to an `error` result for the parent; 4. reads the result, scoped to the child's OWN events (everything at or after `seedLength`, so a seeded child that produced no message of its own never returns the seeded parent's last message): the last `assistant/message` content (deep-cloned — the log is frozen) and the last `turn/end.reason` mapped to a `SubagentStopReason`. A structured run surfaces the captured value as `result.structured`; a structured child that finished cleanly WITHOUT ever capturing settles `error` (a clean finish without the demanded result is a failure, not a success with a missing field). -`SubagentService` waits for `run.started` before emitting `subagent/start`, so a synchronous start observer can resolve the published child with `ctx.agents.get(run.id)`; the result driver awaits the same boundary before sending the prompt. An attempt that never publishes rejects readiness and emits no false start/end pair; its result reports a deliberate cancel/dispose as `aborted` and propagates an infrastructure fault. `dispose()` awaits creation or rollback and then delegates to `AgentHandle.dispose()` (stop and drain → remove agent → detach session → unwind scope); `cancel()` records its request even before publication and cancels the child immediately once available. A cancel landing before any `turn/end` still settles `aborted`, honoring the cancel contract rather than the generic no-turn `error`. +`SubagentService` waits for `run.started` before emitting `subagent/start`, so a synchronous start observer can resolve the published child with `ctx.agents.get(run.id)`; the result driver awaits the same boundary before sending the prompt. An attempt that never publishes rejects readiness and emits no false start/end pair; its result reports a deliberate cancel/dispose as `aborted` and propagates an infrastructure fault. `dispose()` awaits creation or rollback and then delegates to `AgentHandle.dispose()` (stop and drain → remove agent → detach session → unwind scope). Before readiness, `cancel()` deactivates the unpublished owner so no agent, session, or lifecycle event can escape; after readiness it cancels the live child immediately. Either path records the cancellation, so a cancel landing before any `turn/end` settles `aborted`, honoring the cancel contract rather than the generic no-turn `error`. ### `InProcessRunOptions` diff --git a/packages/subagent/subagent-inprocess/src/index.ts b/packages/subagent/subagent-inprocess/src/index.ts index 518ea6a8b4..511b686c13 100644 --- a/packages/subagent/subagent-inprocess/src/index.ts +++ b/packages/subagent/subagent-inprocess/src/index.ts @@ -110,7 +110,10 @@ async function quiesceFiber(fiber: Fiber): Promise { * before the turn starts). The final `assistant/message` is the result output, * the matching `turn/end.reason` the stop reason. `dispose()` delegates to the * factory's {@link AgentHandle.dispose} (stop loop → await quiescence → remove - * session); `cancel()` cancels the child's in-flight turn. + * session). `cancel()` cancels a published child's in-flight turn; before + * readiness it instead deactivates the unpublished run-owner transaction, so + * `started` rejects, no agent/session lifecycle is published, and `result` + * resolves `aborted`. * * Throws {@link SubagentDepthError} before creating anything when the child's * depth (parent depth + 1) would exceed `request.maxDepth`. @@ -219,9 +222,10 @@ export function startInProcessRun( // Install it after provider ownership succeeds but BEFORE awaiting creation, // so an inactive provider cannot leave an orphaned listener and abort/dispose // during async setup is still recorded and applied the moment a child exists. - // `cancelled` records that a cancel was requested at all, so the pre-turn - // cancel window — where the child clears the queued prompt before any - // `turn/end` is logged — settles as `aborted` (honoring the cancel contract) + // `cancelled` records that a cancel was requested at all. Before readiness, + // cancellation deactivates the unpublished run-owner transaction so the + // factory cannot publish an agent or session. After readiness, it cancels the + // live child. Either path settles as `aborted` (honoring the cancel contract) // rather than falling through to the no-turn `error` mapping. let cancelled = false // An accessor, not an inline read: `cancelled` mutates from closures (the @@ -230,13 +234,6 @@ export function startInProcessRun( const isCancelled = (): boolean => cancelled let child: Agent | undefined let handle: AgentHandle | undefined - let disposeRequested = false - const isDisposeRequested = (): boolean => disposeRequested - const requestCancel = (reason: string): void => { - cancelled = true - child?.cancel(reason) - } - const onAbort = (): void => { requestCancel('subagent cancelled') } // One run-owned Cordis fiber is the common ownership node. Install the // provider effect FIRST: a start racing an already-unloading provider fails @@ -250,11 +247,28 @@ export function startInProcessRun( let ownerFiber: (Fiber & PromiseLike) | undefined let ownerSetupError: unknown let ownerDisposing: Promise | undefined - const disposeOwner = (): Promise => (ownerDisposing ??= ownerFiber === undefined - ? Promise.resolve() - : quiesceFiber(ownerFiber)) - let manualDisposeRequested = false - const isManualDisposeRequested = (): boolean => manualDisposeRequested + const disposeOwner = (): Promise => { + if (ownerDisposing !== undefined) return ownerDisposing + // An already-aborted request is observed before the owner fiber is minted. + // Do not memoize that no-op: the post-plugin cancellation check below must + // still be able to claim and deactivate the real fiber. + if (ownerFiber === undefined) return Promise.resolve() + ownerDisposing = quiesceFiber(ownerFiber) + // Pre-readiness cancellation is synchronous fire-and-forget at the public + // `cancel()` boundary. Observe a teardown rejection here; dispose() still + // awaits the same memoized promise and reports it to an explicit caller. + void ownerDisposing.catch(() => undefined) + return ownerDisposing + } + const requestCancel = (reason: string): void => { + cancelled = true + if (child === undefined) { + if (ownerFiber !== undefined) void disposeOwner() + return + } + child.cancel(reason) + } + const onAbort = (): void => { requestCancel('subagent cancelled') } const unlinkProvider = ctx.effect(() => () => { requestCancel('subagent provider disposed') return disposeOwner() @@ -265,6 +279,10 @@ export function startInProcessRun( ownerFiber = parent.ctx.plugin(Object.assign(subagentRunOwner, { inject: ['agents', 'sessions', 'llm', 'tools', 'systemPrompt'], })) + // `signal.aborted` is checked before this fiber exists. Once it does, make + // that recorded cancellation effective immediately; awaiting creation must + // observe an inactive owner instead of reaching the publication boundary. + if (isCancelled()) void disposeOwner() } catch (error: unknown) { ownerSetupError = error } @@ -299,8 +317,6 @@ export function startInProcessRun( }) handle = created child = created.agent - - if (isCancelled()) created.agent.cancel('subagent cancelled') return created.agent })() @@ -322,10 +338,9 @@ export function startInProcessRun( // without manufacturing an unreachable runtime branch. liveChild = child as Agent } catch (error: unknown) { - if (isManualDisposeRequested()) return { output: [], stopReason: 'aborted' } + if (isCancelled()) return { output: [], stopReason: 'aborted' } throw error instanceof Error ? error : new Error('subagent child creation failed with a non-Error value', { cause: error }) } - if (isCancelled() || isDisposeRequested()) return { output: [], stopReason: 'aborted' } liveChild.send(prompt) await liveChild.whenIdle() // Deliberately NO re-prompt when a structured child finishes cleanly @@ -348,8 +363,6 @@ export function startInProcessRun( async dispose(): Promise { return (disposing ??= (async () => { signal?.removeEventListener('abort', onAbort) - disposeRequested = true - manualDisposeRequested = true requestCancel('subagent disposed during creation') // Removing provider ownership and disposing the common run-owner fiber // are the same quiescence transaction; parent disposal may already have diff --git a/packages/subagent/subagent-inprocess/tests/subagent-inprocess.spec.ts b/packages/subagent/subagent-inprocess/tests/subagent-inprocess.spec.ts index f8e18ed572..cb394d8d95 100644 --- a/packages/subagent/subagent-inprocess/tests/subagent-inprocess.spec.ts +++ b/packages/subagent/subagent-inprocess/tests/subagent-inprocess.spec.ts @@ -269,6 +269,38 @@ describe('startInProcessRun', () => { await expect(run.result).resolves.toEqual({ output: [], stopReason: 'aborted' }) }) + it('observes detached pre-readiness teardown failure and reports it to explicit dispose', async () => { + const { ctx, parent } = await setup([]) + function inertOwner(): void {} + const ownerFiber = ctx.plugin(inertOwner) + await ownerFiber + const disposeFailure = new Error('owner dispose exploded') + const disposeSpy = vi.spyOn(ownerFiber, 'dispose').mockImplementation(() => { throw disposeFailure }) + const rejectingOwnerCtx = { + agents: { create: () => Promise.reject(new Error('creation stopped by cancellation')) }, + } as unknown as Context + const parentWithFailingTeardown = { + options: parent.options, + session: parent.session, + ctx: { + plugin(plugin: (inner: Context) => void) { + plugin(rejectingOwnerCtx) + return ownerFiber + }, + }, + } as unknown as Agent + const run = startInProcessRun(ctx, { + prompt: [{ type: 'text', text: 'must never start' }], + parent: parentWithFailingTeardown, + }, {}) + + run.cancel('cancel before readiness') + await expect(run.result).resolves.toEqual({ output: [], stopReason: 'aborted' }) + await expect(run.dispose()).rejects.toBe(disposeFailure) + disposeSpy.mockRestore() + await ownerFiber.dispose() + }) + it('does not attach an abort listener when provider ownership is already inactive', async () => { const { ctx, parent } = await setup([]) let providerCtx: Context | undefined diff --git a/packages/subagent/subagent-spawn/README.md b/packages/subagent/subagent-spawn/README.md index e411d44ab0..07c237b303 100644 --- a/packages/subagent/subagent-spawn/README.md +++ b/packages/subagent/subagent-spawn/README.md @@ -6,7 +6,7 @@ The run mechanics live in the shared [`@deepseek-ai/dsh-subagent-inprocess`](../ ## What it does -`start(request)` delegates to `startInProcessRun(ctx, request, {})` with no seed: a fresh child agent with the parent's `cwd`/`parentSession` lineage and (by default) the parent's model. The driver creates one run-owner fiber under `parent.ctx`; parent teardown, this provider's teardown, and manual disposal all converge there before child publication. Its `run.started` boundary resolves only after the fresh child is published, so `subagent/start` observers see a live registry entry. See the [driver README](../subagent-inprocess/README.md) for the full lifecycle (depth check, one-shot drive, result read, dispose). +`start(request)` delegates to `startInProcessRun(ctx, request, {})` with no seed: a fresh child agent with the parent's `cwd`/`parentSession` lineage and (by default) the parent's model. The driver creates one run-owner fiber under `parent.ctx`; parent teardown, this provider's teardown, manual disposal, and cancellation before readiness all converge there before child publication. Its `run.started` boundary resolves only after the fresh child is published, so `subagent/start` observers see a live registry entry; a same-tick cancel deactivates the unpublished transaction instead, rejects readiness, resolves the result as `aborted`, and emits no agent/session or subagent lifecycle. See the [driver README](../subagent-inprocess/README.md) for the full lifecycle (depth check, one-shot drive, result read, dispose). ## Capabilities diff --git a/packages/subagent/subagent-spawn/tests/subagent-spawn.spec.ts b/packages/subagent/subagent-spawn/tests/subagent-spawn.spec.ts index 3d23fca285..f00eddeb25 100644 --- a/packages/subagent/subagent-spawn/tests/subagent-spawn.spec.ts +++ b/packages/subagent/subagent-spawn/tests/subagent-spawn.spec.ts @@ -163,20 +163,31 @@ describe('dsh-subagent-spawn', () => { await run.dispose() }) - it('cancelling BEFORE the child turn starts settles aborted, not error', async () => { - // Regression: a cancel landing in the pre-turn window clears the queued - // prompt before any `turn/end` is logged. Deriving the stop reason from - // `turn/end` alone then mis-maps the no-turn case to `error`; the run must - // honor the cancel contract and settle `aborted`. The cancel is synchronous - // (same tick as start, before the loop's queued-wait continuation runs), so - // the turn is dropped and the empty script is never consumed. + it('same-tick cancellation rejects readiness and prevents child publication', async () => { + // Regression: cancellation before readiness used to set a flag but let the + // async factory publish a child anyway, so `started` fulfilled and lifecycle + // observers saw an agent for an attempt the caller had already cancelled. + // The empty script also proves no model turn can run. const { ctx, parent } = await setup([]) + const beforeAgents = ctx.agents.list().length + const beforeSessions = ctx.sessions.list().length + const published: string[] = [] + ctx.on('session/created', () => void published.push('session/created')) + ctx.on('agent/created', () => void published.push('agent/created')) + ctx.on('agent/session-start', () => void published.push('agent/session-start')) + ctx.on('subagent/start', () => void published.push('subagent/start')) + ctx.on('subagent/end', () => void published.push('subagent/end')) const run = ctx.subagents.start('spawn', { prompt: [{ type: 'text', text: 'p' }], parent }) run.cancel('early') - const result = await run.result - expect(result.stopReason).toBe('aborted') - expect(result.output).toEqual([]) + + await expect(run.started).rejects.toThrow() + await expect(run.result).resolves.toEqual({ output: [], stopReason: 'aborted' }) await run.dispose() + await Promise.resolve() + expect(ctx.agents.get(run.id)).toBeUndefined() + expect(ctx.agents.list()).toHaveLength(beforeAgents) + expect(ctx.sessions.list()).toHaveLength(beforeSessions) + expect(published).toEqual([]) }) it('a cancel from agent/queued maps a no-turn child log to aborted', async () => { diff --git a/packages/subagent/subagent/README.md b/packages/subagent/subagent/README.md index b068cfe79d..7e63f07376 100644 --- a/packages/subagent/subagent/README.md +++ b/packages/subagent/subagent/README.md @@ -18,23 +18,23 @@ Unlike the bash seam (one executor per context, second load throws), **multiple | Member | Semantics | |---|---| -| `registerProvider(provider)` | Register a frozen acceptance snapshot under `provider.name`; later caller mutation cannot change registry behavior or HMR cleanup, while `start` stays bound to the original provider receiver. Throws `SubagentError('DUPLICATE_PROVIDER')` on a name clash. Effect-scoped (HMR-safe); returns the disposer. | +| `registerProvider(provider)` | Read and validate the name, capability object and four boolean flags, `inheritsParentContext`, and `start` callback exactly once, then register a frozen acceptance snapshot under the accepted name. Malformed fixed fields fail loud before registration; later caller mutation cannot change registry behavior or HMR cleanup, while `start` stays bound to the original provider receiver. Throws `SubagentError('DUPLICATE_PROVIDER')` on a name clash. Effect-scoped (HMR-safe); returns the disposer. | | `getProvider(name)` | Look up the frozen registry snapshot (`undefined` if absent). | | `list()` | Registered provider names (insertion order). | -| `start(name, request)` | Resolve the provider (`NO_PROVIDER` if absent), read every caller field once into one acceptance snapshot, validate every requested START-TIME capability and scalar value before any child is created, and materialize prompt/schema/options/filter data through a single-pass lossless-JSON snapshot before delegating to `provider.start`. Return a frozen service-owned run wrapper whose provider fields are captured once, whose methods remain bound to the provider handle, and whose `result` is one detached, deeply frozen normalization shared by the caller and telemetry. Emit `subagent/start` only after `run.started` fulfills and the paired `subagent/end` after that started run settles; a pre-publication readiness rejection emits neither. | +| `start(name, request)` | Resolve the provider (`NO_PROVIDER` if absent), read every caller field once into one acceptance snapshot, validate every requested START-TIME capability and scalar value before any child is created, and materialize prompt/schema/options/filter data through a single-pass lossless-JSON snapshot before delegating to `provider.start`. Acquire and memoize the provider run's disposer before reading the rest of its handle, then return a frozen service-owned wrapper whose fields are captured once, whose methods remain bound to the provider handle, and whose `result` is one detached, deeply frozen normalization shared by the caller and telemetry. Malformed handle access/binding starts rollback before the synchronous fault escapes; malformed terminal data rejects only after rollback reaches quiescence. Emit `subagent/start` only after `run.started` fulfills and the paired `subagent/end` after that started run settles; a pre-publication readiness rejection emits neither. | ## Capabilities: two kinds, discovered two ways - **Start-time features** (`outputSchema`, `depthLimit`, `toolFilter/persona`) 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. +Beside `capabilities` sits one DESCRIPTIVE fact: `provider.inheritsParentContext` — whether a child sees the parent conversation (`fork`: true — seeded with the completed-turn prefix; `spawn`/`acp`: false). The service validates that the descriptor is a boolean but does not interpret or enforce its meaning; the model-facing consumer (`dsh-tool-subagent`) derives truthful tool wording from it. ## Run lifecycle `provider.start(request)` returns a provider-owned `SubagentRun`; `SubagentService.start` captures that handle once and returns a frozen service-owned wrapper with `started` (the publication/readiness promise), a normalized `result` (the terminal outcome), bound `cancel()` and `dispose()`, and bound optional runtime methods. `started` resolves only after the provider has established a real child and rejects if the attempt fails or is cancelled first. `result` resolves with one detached, deeply frozen `SubagentResult` (`output`, optional `structured`, `stopReason`) that the service and caller share — it does **not** reject on a child-level failure (a model/transport failure resolves with `stopReason: 'error'`), but malformed provider data rejects as an infrastructure contract fault. The consumer maps a non-`completed` reason to an `isError` tool result and MUST `dispose()` on every path (success, error, abort) to reach child quiescence and avoid leaking an idle child / session. -The service also announces provider lifecycle: `subagent/provider-added` (the frozen registry snapshot) fires after a registration and `subagent/provider-removed` (the accepted 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". Run lifecycle is gated by provider readiness: the service captures the provider handle's public fields once, `subagent/start` (payload `SubagentRunInfo`) fires only after the accepted `started` promise fulfills, and `subagent/end` (payload `SubagentRunEndInfo`) uses the same accepted id and normalized result; readiness rejection emits neither. For spawn/fork, the start listener can resolve the published child via `ctx.agents.get(info.id)`; a remote provider need not have a local registry entry. Both events are **observe-only** plain emits whose service-owned payloads are deeply frozen before per-listener dispatch. The service observes the normalized `result` immediately even while readiness is pending and buffers that end payload until start has fired; a malformed provider result rejects the returned result promise and becomes contained `error` telemetry, a rejection cannot become an unhandled detached promise, start always precedes end, and one listener cannot corrupt either the caller or later listeners. `subagent/end` carries the same frozen output as `lastAssistantMessage` on a valid settle path and omits it on infrastructure or result-contract failure. Any run-affecting decision is out of scope for this observe-only surface. +The service also announces provider lifecycle: `subagent/provider-added` (the frozen registry snapshot) fires after a registration and `subagent/provider-removed` (the accepted 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". Run lifecycle is gated by provider readiness: the service captures the provider handle's public fields once, `subagent/start` (payload `SubagentRunInfo`) fires only after the accepted `started` promise fulfills, and `subagent/end` (payload `SubagentRunEndInfo`) uses the same accepted id and normalized result; readiness rejection emits neither. For spawn/fork, the start listener can resolve the published child via `ctx.agents.get(info.id)`; a remote provider need not have a local registry entry. Both events are **observe-only** plain emits whose service-owned payloads are deeply frozen before per-listener dispatch. A synchronous listener throw or returned-promise rejection is logged per listener; later listeners still run synchronously, and async listeners remain concurrent fire-and-forget. The service observes the normalized `result` immediately even while readiness is pending and buffers that end payload until start has fired; a malformed provider result rejects the returned result promise and becomes contained `error` telemetry, a rejection cannot become an unhandled detached promise, start always precedes end, and one listener cannot corrupt either the caller or later listeners. `subagent/end` carries the same frozen output as `lastAssistantMessage` on a valid settle path and omits it on infrastructure or result-contract failure. Any run-affecting decision 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 de376686fd..b319a7f31d 100644 --- a/packages/subagent/subagent/src/index.ts +++ b/packages/subagent/subagent/src/index.ts @@ -169,9 +169,11 @@ export class SubagentService extends Service { * Register a provider under its `provider.name`. Throws {@link SubagentError} * (`DUPLICATE_PROVIDER`) if the name is already taken. The registry snapshots * the name, static descriptors, and `start` callback identity at acceptance; - * later caller mutation cannot change lookup, capability validation, consumer - * wording, dispatch, or HMR cleanup. The callback remains bound to the - * original provider object, so provider-owned mutable state stays live. + * every fixed field and capability flag is read once and validated before + * registration, so malformed provider objects fail loud without entering the + * registry. Later caller mutation cannot change lookup, capability validation, + * consumer wording, dispatch, or HMR cleanup. The callback remains bound to + * the original provider object, so provider-owned mutable state stays live. * Effect-scoped: disposed with the calling fiber (HMR-safe). Emits * `subagent/provider-added` after the registration and * `subagent/provider-removed` on unregistration, so consumers can mirror @@ -187,18 +189,49 @@ export class SubagentService extends Service { // mutate or reuse the provider object before its old fiber unloads. Binding // preserves the provider method's receiver while making replacement of the // public callback field after registration inert. - const inputCapabilities = provider.capabilities + const name: unknown = provider.name + const inputCapabilities: unknown = provider.capabilities + const inheritsParentContext: unknown = provider.inheritsParentContext + // eslint-disable-next-line @typescript-eslint/unbound-method + const inputStart: unknown = provider.start + if (typeof name !== 'string') { + throw new TypeError('subagent provider name must be a string') + } + if (inputCapabilities === null || typeof inputCapabilities !== 'object' || Array.isArray(inputCapabilities)) { + throw new TypeError(`subagent provider "${name}" capabilities must be an object`) + } + const inputCapabilityFields = inputCapabilities as Record + const outputSchema = inputCapabilityFields.outputSchema + const depthLimit = inputCapabilityFields.depthLimit + const toolFilter = inputCapabilityFields.toolFilter + const persona = inputCapabilityFields.persona + for (const [capability, value] of [ + ['outputSchema', outputSchema], + ['depthLimit', depthLimit], + ['toolFilter', toolFilter], + ['persona', persona], + ] as const) { + if (typeof value !== 'boolean') { + throw new TypeError(`subagent provider "${name}" capability "${capability}" must be a boolean`) + } + } + if (typeof inheritsParentContext !== 'boolean') { + throw new TypeError(`subagent provider "${name}" inheritsParentContext must be a boolean`) + } + if (typeof inputStart !== 'function') { + throw new TypeError(`subagent provider "${name}" start must be a function`) + } const capabilities: SubagentCapabilities = Object.freeze({ - outputSchema: inputCapabilities.outputSchema, - depthLimit: inputCapabilities.depthLimit, - toolFilter: inputCapabilities.toolFilter, - persona: inputCapabilities.persona, + outputSchema: outputSchema as boolean, + depthLimit: depthLimit as boolean, + toolFilter: toolFilter as boolean, + persona: persona as boolean, }) const snapshot: SubagentProvider = Object.freeze({ - name: provider.name, + name, capabilities, - inheritsParentContext: provider.inheritsParentContext, - start: provider.start.bind(provider), + inheritsParentContext, + start: Function.prototype.bind.call(inputStart, provider) as SubagentProvider['start'], }) const dispose = this.ctx.effect(function* (this: SubagentService) { if (this.providers.has(snapshot.name)) { @@ -255,7 +288,10 @@ export class SubagentService extends Service { * {@link SubagentProvider.start}. The returned handle is a service-owned, * frozen wrapper: provider fields are captured once, methods stay bound to the * provider handle, and `result` resolves to one detached, deeply frozen value - * shared by the caller and lifecycle telemetry. Emits `subagent/start` / + * shared by the caller and lifecycle telemetry. Once a provider returns a + * callable disposer, malformed handle access/binding starts rollback before + * the synchronous fault escapes; malformed terminal data rejects only after + * that same memoized disposal reaches quiescence. Emits `subagent/start` / * `subagent/end` only after the run's readiness boundary fulfills. A provider * that fails before establishing a child emits neither event. * @param name - the provider to run on. @@ -324,82 +360,176 @@ export class SubagentService extends Service { ...toolFilter !== undefined ? { toolFilter } : {}, ...input.persona !== undefined ? { persona: input.persona } : {}, } - const providerRun = provider.start(accepted) + const providerRun: unknown = provider.start(accepted) + if (providerRun === null || (typeof providerRun !== 'object' && typeof providerRun !== 'function')) { + throw new TypeError(`subagent provider "${name}" start must return a SubagentRun object`) + } + const acceptedRun = providerRun as SubagentRun + // Acquire the one rollback capability BEFORE touching any other provider-run + // field. Once start() returned a handle, the service owns an accepted live + // attempt; a hostile later accessor or bind must not make that attempt + // unreachable. The wrapper also memoizes provider disposal, so automatic + // rollback and a racing caller join one quiescence transaction even if a + // contract-violating provider forgot to make its own method idempotent. + // eslint-disable-next-line @typescript-eslint/unbound-method + const inputDispose = acceptedRun.dispose + if (typeof inputDispose !== 'function') { + throw new TypeError(`subagent provider "${name}" run dispose must be a function`) + } + let disposal: Promise | undefined + const dispose = (): Promise => { + if (disposal === undefined) { + try { + // Invoke through the captured callable without reading its public + // `bind`/`length`/`name` properties. Disposal is the recovery + // capability itself; hostile function metadata must not prevent the + // seam from exercising it when a later handle field is malformed. + disposal = Promise.resolve(Reflect.apply(inputDispose, acceptedRun, [])) + } catch (error: unknown) { + disposal = Promise.reject(error instanceof Error + ? error + : new Error('subagent provider run dispose threw a non-Error value', { cause: error })) + } + } + return disposal + } // Provider-owned run objects can be accessor-backed too. Capture every // public field exactly once, bind methods to the provider's original handle, // and expose only this service-owned wrapper. The normalized result promise // is also the one lifecycle telemetry observes, so the caller and observers // cannot receive different values from stateful accessors. - const id = providerRun.id - const started = providerRun.started - const providerResult = providerRun.result - const cancel = providerRun.cancel.bind(providerRun) - const sendMessage = providerRun.sendMessage?.bind(providerRun) - const dispose = providerRun.dispose.bind(providerRun) - const resume = providerRun.resume?.bind(providerRun) - const result = providerResult.then(value => this.snapshotRunResult(value)) - const run: SubagentRun = Object.freeze({ - id, - started, - result, - cancel, - dispose, - ...sendMessage === undefined - ? {} - : { sendMessage }, - ...resume === undefined - ? {} - : { resume }, - }) - - // Observe result settlement IMMEDIATELY, before waiting on readiness. A - // provider may fail both promises in the same turn; deferring the rejection - // handler until `started` fulfilled would leave `result` transiently - // unhandled. The settled event is buffered until start has been announced, - // preserving start → end order even for an already-settled scripted run. - let readiness: 'pending' | 'started' | 'failed' = 'pending' - let pendingEnd: SubagentRunEndInfo | undefined - const deliverEnd = (info: SubagentRunEndInfo): void => { - if (readiness === 'started') this.emitLifecycle('subagent/end', info, parent) - else if (readiness === 'pending') pendingEnd = info - // A pre-publication readiness failure has no lifecycle pair; result - // remains observable by the run's consumer, but telemetry must not claim - // that a child started. - } - void result.then( - (value) => { - deliverEnd({ - provider: name, - id, - stopReason: value.stopReason, - lastAssistantMessage: value.output, - }) - }, - () => { deliverEnd({ provider: name, id, stopReason: 'error' }) }, - ) - - // Readiness is the publication boundary owned by the provider. For - // in-process runs, fulfillment means the agent registry already contains - // `run.id`; for ACP it means the remote session exists. Emit start with - // per-listener containment, then flush an outcome that settled unusually - // early. A readiness rejection is handled here and deliberately emits no - // false start/end pair; the result path above remains independently handled. - void started.then( - () => { - readiness = 'started' - this.emitLifecycle('subagent/start', { provider: name, id }, parent) - if (pendingEnd !== undefined) { - const info = pendingEnd - pendingEnd = undefined - this.emitLifecycle('subagent/end', info, parent) + try { + const id = acceptedRun.id + if (typeof id !== 'string') { + throw new TypeError(`subagent provider "${name}" run id must be a string`) + } + const started = acceptedRun.started + if (!(started instanceof Promise)) { + throw new TypeError(`subagent provider "${name}" run started must be a Promise`) + } + // Observe each accepted provider promise before reading the next hostile + // field. A later accessor/validation failure prevents a wrapper from being + // returned, but must not leave an already-rejected provider promise + // unhandled while rollback proceeds. + void started.catch(() => undefined) + const providerResult = acceptedRun.result + if (!(providerResult instanceof Promise)) { + throw new TypeError(`subagent provider "${name}" run result must be a Promise`) + } + void providerResult.catch(() => undefined) + // eslint-disable-next-line @typescript-eslint/unbound-method + const inputCancel = acceptedRun.cancel + if (typeof inputCancel !== 'function') { + throw new TypeError(`subagent provider "${name}" run cancel must be a function`) + } + // eslint-disable-next-line @typescript-eslint/unbound-method + const inputSendMessage = acceptedRun.sendMessage + if (inputSendMessage !== undefined && typeof inputSendMessage !== 'function') { + throw new TypeError(`subagent provider "${name}" run sendMessage must be a function when provided`) + } + // eslint-disable-next-line @typescript-eslint/unbound-method + const inputResume = acceptedRun.resume + if (inputResume !== undefined && typeof inputResume !== 'function') { + throw new TypeError(`subagent provider "${name}" run resume must be a function when provided`) + } + const cancel = Function.prototype.bind.call(inputCancel, acceptedRun) as SubagentRun['cancel'] + const sendMessage = inputSendMessage === undefined + ? undefined + : Function.prototype.bind.call(inputSendMessage, acceptedRun) as NonNullable + const resume = inputResume === undefined + ? undefined + : Function.prototype.bind.call(inputResume, acceptedRun) as NonNullable + const result = providerResult.then(async (value) => { + try { + return this.snapshotRunResult(value) + } catch (error: unknown) { + // A malformed terminal value is an infrastructure contract fault. The + // result rejects only after the accepted provider attempt has reached + // quiescence, so a caller cannot lose the only cleanup handle by merely + // observing the normalization failure. + await this.rollbackProviderRun(name, dispose) + throw error } - }, - () => { - readiness = 'failed' - pendingEnd = undefined - }, - ) - return run + }) + const run: SubagentRun = Object.freeze({ + id, + started, + result, + cancel, + dispose, + ...sendMessage === undefined + ? {} + : { sendMessage }, + ...resume === undefined + ? {} + : { resume }, + }) + + // Observe result settlement IMMEDIATELY, before waiting on readiness. A + // provider may fail both promises in the same turn; deferring the rejection + // handler until `started` fulfilled would leave `result` transiently + // unhandled. The settled event is buffered until start has been announced, + // preserving start → end order even for an already-settled scripted run. + let readiness: 'pending' | 'started' | 'failed' = 'pending' + let pendingEnd: SubagentRunEndInfo | undefined + const deliverEnd = (info: SubagentRunEndInfo): void => { + if (readiness === 'started') this.emitLifecycle('subagent/end', info, parent) + else if (readiness === 'pending') pendingEnd = info + // A pre-publication readiness failure has no lifecycle pair; result + // remains observable by the run's consumer, but telemetry must not claim + // that a child started. + } + void result.then( + (value) => { + deliverEnd({ + provider: name, + id, + stopReason: value.stopReason, + lastAssistantMessage: value.output, + }) + }, + () => { deliverEnd({ provider: name, id, stopReason: 'error' }) }, + ) + + // Readiness is the publication boundary owned by the provider. For + // in-process runs, fulfillment means the agent registry already contains + // `run.id`; for ACP it means the remote session exists. Emit start with + // per-listener containment, then flush an outcome that settled unusually + // early. A readiness rejection is handled here and deliberately emits no + // false start/end pair; the result path above remains independently handled. + void started.then( + () => { + readiness = 'started' + this.emitLifecycle('subagent/start', { provider: name, id }, parent) + if (pendingEnd !== undefined) { + const info = pendingEnd + pendingEnd = undefined + this.emitLifecycle('subagent/end', info, parent) + } + }, + () => { + readiness = 'failed' + pendingEnd = undefined + }, + ) + return run + } catch (error: unknown) { + // start() has already transferred a live attempt to the seam. Begin + // rollback synchronously before surfacing the malformed-handle failure; + // the contained cleanup promise prevents either a resource leak or an + // unhandled rejection even though this API cannot synchronously await it. + void this.rollbackProviderRun(name, dispose) + throw error + } + } + + /** Dispose one malformed provider attempt without letting cleanup mask the contract fault. */ + private async rollbackProviderRun(providerName: string, dispose: () => Promise): Promise { + try { + await dispose() + } catch (error: unknown) { + this.ctx.logger.warn(`subagent provider "${providerName}" malformed run rollback failed: ${renderThrown(error)}`) + } } /** Normalize one provider result into the immutable seam value. */ @@ -452,10 +582,12 @@ export class SubagentService extends Service { /** * Emit a `subagent/*` lifecycle event with PER-LISTENER containment: dispatch - * each subscriber individually and log (never propagate) a thrown one, so one - * bad subscriber can neither strand the already-live run, surface as an - * unhandled rejection on the detached settle hook, NOR starve the listeners - * registered after it. A single try/catch around `ctx.emit` would not do the + * each subscriber individually and log (never propagate) either a synchronous + * throw or a returned-promise rejection, so one bad subscriber can neither + * strand the already-live run, surface as an unhandled rejection on the + * detached settle hook, NOR starve the listeners registered after it. Async + * listeners remain concurrent fire-and-forget; dispatch does not await or + * serialize them. A single try/catch around `ctx.emit` would not do the * last part — cordis `emit` runs listeners in a `.map(cb => cb())` that halts * on the first throw — so this resolves the listener callbacks via * `ctx.events.dispatch` and contains each call, the same guarantee @@ -488,7 +620,14 @@ export class SubagentService extends Service { : [scopeTarget(this, parent), name, acceptedInfo] for (const callback of this.ctx.events.dispatch('emit', dispatchArgs)) { try { - callback(acceptedInfo) + const returned: unknown = callback(acceptedInfo) + // Plain emits remain fire-and-forget and every callback is still invoked + // synchronously in this loop. Observe a returned promise independently so + // an async listener rejection is contained without serializing listeners + // or delaying provider/run lifecycle. + void Promise.resolve(returned).catch((error: unknown) => { + this.ctx.logger.warn(`subagent: ${name} listener rejected: ${renderThrown(error)}`) + }) } catch (error: unknown) { this.ctx.logger.warn(`subagent: ${name} listener threw: ${renderThrown(error)}`) } diff --git a/packages/subagent/subagent/tests/service.spec.ts b/packages/subagent/subagent/tests/service.spec.ts index ba4b58c2fc..d1a531ca09 100644 --- a/packages/subagent/subagent/tests/service.spec.ts +++ b/packages/subagent/subagent/tests/service.spec.ts @@ -150,6 +150,93 @@ describe('SubagentService', () => { } }) + it.each([ + { label: 'a non-string name', patch: { name: 42 }, message: 'name must be a string' }, + { label: 'null capabilities', patch: { capabilities: null }, message: 'capabilities must be an object' }, + { label: 'primitive capabilities', patch: { capabilities: 42 }, message: 'capabilities must be an object' }, + { label: 'array capabilities', patch: { capabilities: [] }, message: 'capabilities must be an object' }, + { + label: 'a non-boolean outputSchema capability', + patch: { capabilities: { ...NO_CAPS, outputSchema: 'yes' } }, + message: 'capability "outputSchema" must be a boolean', + }, + { + label: 'a non-boolean depthLimit capability', + patch: { capabilities: { ...NO_CAPS, depthLimit: 'yes' } }, + message: 'capability "depthLimit" must be a boolean', + }, + { + label: 'a non-boolean toolFilter capability', + patch: { capabilities: { ...NO_CAPS, toolFilter: 'yes' } }, + message: 'capability "toolFilter" must be a boolean', + }, + { + label: 'a non-boolean persona capability', + patch: { capabilities: { ...NO_CAPS, persona: 'yes' } }, + message: 'capability "persona" must be a boolean', + }, + { label: 'a non-boolean context descriptor', patch: { inheritsParentContext: 'yes' }, message: 'inheritsParentContext must be a boolean' }, + { label: 'a non-callable start field', patch: { start: 42 }, message: 'start must be a function' }, + ])('rejects a provider registration with $label before entering the registry', async ({ patch, message }) => { + const ctx = new Context() + await ctx.plugin(SubagentService) + const provider = Object.assign(new StubProvider('invalid'), patch) + + expect(() => ctx.subagents.registerProvider(provider as unknown as SubagentProvider)).toThrow(message) + expect(ctx.subagents.list()).toEqual([]) + expect(Object.isFrozen(provider)).toBe(false) + }) + + it('reads every registration field once and binds the accepted start callback to the provider', async () => { + const ctx = new Context() + await ctx.plugin(SubagentService) + const reads = { + name: 0, + capabilities: 0, + outputSchema: 0, + depthLimit: 0, + toolFilter: 0, + persona: 0, + inheritsParentContext: 0, + start: 0, + } + const capabilities = Object.defineProperties({}, { + outputSchema: { enumerable: true, get: () => { reads.outputSchema += 1; return false } }, + depthLimit: { enumerable: true, get: () => { reads.depthLimit += 1; return false } }, + toolFilter: { enumerable: true, get: () => { reads.toolFilter += 1; return false } }, + persona: { enumerable: true, get: () => { reads.persona += 1; return false } }, + }) as SubagentCapabilities + const acceptedStart = function (this: SubagentProvider, request: SubagentStartRequest): SubagentRun { + expect(this).toBe(provider) + return { + id: AgentId(`one-read:${request.parent.id}`), + started: Promise.resolve(), + result: Promise.resolve({ output: [], stopReason: 'completed' }), + cancel() {}, + async dispose() {}, + } + } + const provider = Object.defineProperties({}, { + name: { enumerable: true, get: () => { reads.name += 1; return 'one-read' } }, + capabilities: { enumerable: true, get: () => { reads.capabilities += 1; return capabilities } }, + inheritsParentContext: { enumerable: true, get: () => { reads.inheritsParentContext += 1; return false } }, + start: { enumerable: true, get: () => { reads.start += 1; return acceptedStart } }, + }) as SubagentProvider + + ctx.subagents.registerProvider(provider) + await expect(ctx.subagents.start('one-read', baseRequest()).result).resolves.toMatchObject({ stopReason: 'completed' }) + expect(reads).toEqual({ + name: 1, + capabilities: 1, + outputSchema: 1, + depthLimit: 1, + toolFilter: 1, + persona: 1, + inheritsParentContext: 1, + start: 1, + }) + }) + it('unregisters a provider when its owning fiber is disposed (HMR safety)', async () => { const ctx = new Context() await ctx.plugin(SubagentService) @@ -608,6 +695,178 @@ describe('SubagentService', () => { }) }) + it.each([ + { label: 'a non-string id', field: 'id', value: 42, message: 'run id must be a string' }, + { label: 'a non-Promise started field', field: 'started', value: undefined, message: 'run started must be a Promise' }, + { label: 'a non-Promise result field', field: 'result', value: undefined, message: 'run result must be a Promise' }, + { label: 'a non-callable cancel field', field: 'cancel', value: undefined, message: 'run cancel must be a function' }, + { label: 'a non-callable sendMessage field', field: 'sendMessage', value: 42, message: 'run sendMessage must be a function' }, + { label: 'a non-callable resume field', field: 'resume', value: 42, message: 'run resume must be a function' }, + ])('rolls back a provider run with $label', async ({ field, value, message }) => { + const ctx = new Context() + await ctx.plugin(SubagentService) + const providerDispose = vi.fn(async () => {}) + const providerRun = { + id: AgentId('invalid-handle-child'), + started: Promise.resolve(), + result: Promise.resolve({ output: [], stopReason: 'completed' } satisfies SubagentResult), + cancel() {}, + dispose: providerDispose, + [field]: value, + } as unknown as SubagentRun + ctx.subagents.registerProvider({ + name: 'invalid-handle', + capabilities: NO_CAPS, + inheritsParentContext: false, + start: () => providerRun, + }) + + expect(() => ctx.subagents.start('invalid-handle', baseRequest())).toThrow(message) + expect(providerDispose).toHaveBeenCalledOnce() + }) + + it.each([ + { label: 'null', value: null }, + { label: 'a primitive', value: 42 }, + ])('rejects $label returned by provider.start before reading a disposer', async ({ value }) => { + const ctx = new Context() + await ctx.plugin(SubagentService) + ctx.subagents.registerProvider({ + name: 'invalid-run-shell', + capabilities: NO_CAPS, + inheritsParentContext: false, + start: () => value as unknown as SubagentRun, + }) + + expect(() => ctx.subagents.start('invalid-run-shell', baseRequest())).toThrow('must return a SubagentRun object') + }) + + it('rejects a run without a callable disposer before accepting ownership', async () => { + const ctx = new Context() + await ctx.plugin(SubagentService) + ctx.subagents.registerProvider({ + name: 'invalid-dispose', + capabilities: NO_CAPS, + inheritsParentContext: false, + start: () => ({ dispose: 42 }) as unknown as SubagentRun, + }) + + expect(() => ctx.subagents.start('invalid-dispose', baseRequest())).toThrow('run dispose must be a function') + }) + + it('observes accepted provider promises when a later handle field is malformed', async () => { + const ctx = new Context() + await ctx.plugin(SubagentService) + const providerDispose = vi.fn(async () => {}) + ctx.subagents.registerProvider({ + name: 'rejected-malformed-handle', + capabilities: NO_CAPS, + inheritsParentContext: false, + start: () => ({ + id: AgentId('rejected-malformed-child'), + started: Promise.reject(new Error('readiness already rejected')), + result: Promise.reject(new Error('result already rejected')), + cancel: 42, + dispose: providerDispose, + }) as unknown as SubagentRun, + }) + + expect(() => ctx.subagents.start('rejected-malformed-handle', baseRequest())).toThrow('run cancel must be a function') + expect(providerDispose).toHaveBeenCalledOnce() + // Let both provider rejections run: the seam's immediate observers keep + // them from surfacing as unhandled after no wrapper was returned. + await Promise.resolve() + }) + + it('starts rollback before surfacing a hostile run accessor failure', async () => { + const ctx = new Context() + await ctx.plugin(SubagentService) + const disposalGate = Promise.withResolvers() + const order: string[] = [] + const providerRun = Object.defineProperties({}, { + dispose: { + get: () => { + order.push('dispose:get') + return async function (this: SubagentRun): Promise { + expect(this).toBe(providerRun) + order.push('dispose:call') + await disposalGate.promise + order.push('dispose:quiescent') + } + }, + }, + id: { get: () => { order.push('id:get'); return AgentId('hostile-handle-child') } }, + started: { get: () => { order.push('started:get'); return Promise.resolve() } }, + result: { get: () => { order.push('result:get'); throw new Error('result accessor exploded') } }, + }) as SubagentRun + ctx.subagents.registerProvider({ + name: 'hostile-handle', + capabilities: NO_CAPS, + inheritsParentContext: false, + start: () => providerRun, + }) + + expect(() => ctx.subagents.start('hostile-handle', baseRequest())).toThrow('result accessor exploded') + expect(order).toEqual(['dispose:get', 'id:get', 'started:get', 'result:get', 'dispose:call']) + disposalGate.resolve(undefined) + await vi.waitFor(() => { expect(order).toContain('dispose:quiescent') }) + }) + + it('rolls back when binding a hostile optional run method fails', async () => { + const ctx = new Context() + await ctx.plugin(SubagentService) + const providerDispose = vi.fn(async () => {}) + const hostileCancel = new Proxy(() => {}, { + get(_target, property) { + if (property === 'length') throw new Error('cancel bind exploded') + return undefined + }, + }) + ctx.subagents.registerProvider({ + name: 'hostile-bind', + capabilities: NO_CAPS, + inheritsParentContext: false, + start: () => ({ + id: AgentId('hostile-bind-child'), + started: Promise.resolve(), + result: Promise.resolve({ output: [], stopReason: 'completed' }), + cancel: hostileCancel, + dispose: providerDispose, + }), + }) + + expect(() => ctx.subagents.start('hostile-bind', baseRequest())).toThrow('cancel bind exploded') + expect(providerDispose).toHaveBeenCalledOnce() + }) + + it.each([ + { label: 'an Error', thrown: new Error('cleanup exploded'), warning: 'cleanup exploded' }, + { label: 'a non-Error value', thrown: 'naked cleanup fault', warning: 'dispose threw a non-Error value' }, + ])('contains rollback failure from $label while preserving the malformed-handle fault', async ({ thrown, warning }) => { + const ctx = new Context() + await ctx.plugin(SubagentService) + const warnings: string[] = [] + ctx.logger.warn = ((message: unknown) => { warnings.push(String(message)) }) as typeof ctx.logger.warn + ctx.subagents.registerProvider({ + name: 'rollback-failure', + capabilities: NO_CAPS, + inheritsParentContext: false, + start: () => ({ + id: 42, + started: Promise.resolve(), + result: Promise.resolve({ output: [], stopReason: 'completed' }), + cancel() {}, + dispose: () => { + // Deliberately violate the seam contract to exercise normalization. + throw thrown + }, + }) as unknown as SubagentRun, + }) + + expect(() => ctx.subagents.start('rollback-failure', baseRequest())).toThrow('run id must be a string') + await vi.waitFor(() => { expect(warnings.some(message => message.includes(warning))).toBe(true) }) + }) + it('waits for provider readiness and observes an early result rejection without reordering lifecycle', async () => { const ctx = new Context() await ctx.plugin(SubagentService) @@ -811,6 +1070,8 @@ describe('SubagentService', () => { const ctx = new Context() await ctx.plugin(SubagentService) const nonJsonOutput = [{ type: 'text', text: 'x', evil: () => 0 }] as unknown as SubagentResult['output'] + const disposalGate = Promise.withResolvers() + const providerDispose = vi.fn(async () => { await disposalGate.promise }) ctx.subagents.registerProvider({ name: 'unclone', capabilities: NO_CAPS, @@ -820,16 +1081,23 @@ describe('SubagentService', () => { started: Promise.resolve(), result: Promise.resolve({ output: nonJsonOutput, stopReason: 'completed' } as SubagentResult), cancel() {}, - dispose: async () => {}, + dispose: providerDispose, }), }) const ended = vi.fn() ctx.on('subagent/end', ended) const run = ctx.subagents.start('unclone', baseRequest()) + let resultSettled = false + void run.result.catch(() => { resultSettled = true }) + await vi.waitFor(() => { expect(providerDispose).toHaveBeenCalledOnce() }) + expect(resultSettled).toBe(false) + disposalGate.resolve(undefined) await expect(run.result).rejects.toThrow('subagent result must be losslessly JSON-serializable') + await run.dispose() await Promise.resolve() + expect(providerDispose).toHaveBeenCalledOnce() const endInfo = ended.mock.calls[0]![0] as Record expect(endInfo.stopReason).toBe('error') expect('lastAssistantMessage' in endInfo).toBe(false) @@ -922,6 +1190,42 @@ describe('SubagentService', () => { await expect(run.result).resolves.toMatchObject({ stopReason: 'completed' }) }) + it('contains asynchronous lifecycle-listener rejections without serializing later listeners', async () => { + const ctx = new Context() + await ctx.plugin(SubagentService) + const warnings: string[] = [] + ctx.logger.warn = ((message: unknown) => { warnings.push(String(message)) }) as typeof ctx.logger.warn + const laterStart = vi.fn() + const laterEnd = vi.fn() + const laterRemoved = vi.fn() + const asyncStart = (async () => { await Promise.resolve(); throw new Error('async start listener') }) as unknown as () => void + const asyncEnd = (async () => { await Promise.resolve(); throw new Error('async end listener') }) as unknown as () => void + const asyncRemoved = (async () => { await Promise.resolve(); throw new Error('async removed listener') }) as unknown as () => void + ctx.on('subagent/start', asyncStart) + ctx.on('subagent/start', laterStart) + ctx.on('subagent/end', asyncEnd) + ctx.on('subagent/end', laterEnd) + ctx.on('subagent/provider-removed', asyncRemoved) + ctx.on('subagent/provider-removed', laterRemoved) + const unregister = ctx.subagents.registerProvider(new StubProvider('async-listeners')) + + const run = ctx.subagents.start('async-listeners', baseRequest()) + await run.started + expect(laterStart).toHaveBeenCalledOnce() + await run.result + await vi.waitFor(() => { + expect(laterEnd).toHaveBeenCalledOnce() + expect(warnings.some(message => message.includes('async start listener'))).toBe(true) + expect(warnings.some(message => message.includes('async end listener'))).toBe(true) + }) + + await unregister() + expect(laterRemoved).toHaveBeenCalledWith('async-listeners') + await vi.waitFor(() => { + expect(warnings.some(message => message.includes('async removed listener'))).toBe(true) + }) + }) + it('contains a listener whose thrown value cannot be stringified', async () => { const ctx = new Context() await ctx.plugin(SubagentService) diff --git a/packages/support/invariants/src/index.ts b/packages/support/invariants/src/index.ts index 153d97a45c..8d8f52f3d7 100644 --- a/packages/support/invariants/src/index.ts +++ b/packages/support/invariants/src/index.ts @@ -367,6 +367,7 @@ export function apply(ctx: Context): void { 'tools/result': args => (args[0] as ToolExecution).agent, 'system-prompt/assemble': args => (args[1] as AssembleContext).scope, 'session/created': null, + 'session/disposed': null, 'session/event': null, 'session/flush': null, 'subagent/start': null, diff --git a/packages/ui/acp/src/index.ts b/packages/ui/acp/src/index.ts index a3ce9b5117..35ff870080 100644 --- a/packages/ui/acp/src/index.ts +++ b/packages/ui/acp/src/index.ts @@ -1010,7 +1010,7 @@ export function apply(ctx: Context, config: AcpConfig): void { * quiescence"): for each session settle any pending prompt `cancelled`, then * run that session's {@link AgentHandle} `dispose()` — which stops the loop * (sets `disposed`, aborts the in-flight step), AWAITS the loop's exit (the - * final `turn/end` + `session/flush` are captured while `onAppend` is still + * final `turn/end` + `session/flush` are captured while the store-owned append observer is still * attached), unregisters the agent, and removes its session from the store. * The per-session disposes run in parallel. Idempotent — clears the `sessions` * map first and memoizes, so a second call (close racing dispose) is a no-op. diff --git a/packages/ui/acp/tests/dispose.spec.ts b/packages/ui/acp/tests/dispose.spec.ts index 037d00f17e..9dbd72aa35 100644 --- a/packages/ui/acp/tests/dispose.spec.ts +++ b/packages/ui/acp/tests/dispose.spec.ts @@ -159,8 +159,8 @@ describe('acp bridge — disposal & HMR safety', () => { it('the final turn closing events are persisted across an AgentHandle dispose (durability)', async () => { // The teardown-ORDER guarantee: a per-agent dispose must stop the loop, // AWAIT its exit (so the loop's final `turn/end` + `session/flush` fire - // through the still-attached `session.onAppend` → `session/event`), and only - // THEN detach onAppend + remove the session. If the order were inverted + // through the still-attached store observer → `session/event`), and only + // THEN detach that observer + remove the session. If the order were inverted // (detach first), the closing events would never reach persistence. Drive a // CLEAN turn to completion, dispose JUST the bridge, then re-load the // persisted log from disk and assert the closing turn/end is on disk — the @@ -190,7 +190,7 @@ describe('acp bridge — disposal & HMR safety', () => { // produced BY the dispose itself. Here the model stream HANGS, so the turn is // still open when teardown runs: the composite agent effect stops the loop, // the loop unwinds and appends `turn/end {disposed}` + runs its final - // `session/flush` — all while `onAppend` is still attached (the session + // `session/flush` — all while the store-owned append observer is still attached (the session // detach is the LAST disposer in the same effect's LIFO chain) — and only // THEN is the session detached. If the order were inverted (or the session // were a racing SIBLING effect), the abort-produced `turn/end` would never @@ -255,7 +255,7 @@ describe('acp bridge — disposal & HMR safety', () => { // into ONE composite effect whose disposers run as a `.then()` chain. The // register disposer emits `agent/disposed`; if a listener throws and the // emit is UNCONTAINED, the rejected chain skips the LATER session-detach - // disposer — stranding the session in the store with `onAppend` attached (a + // disposer — stranding the session in the store with its append observer attached (a // leak AND a durability hole, since the new design relies on detach // running). The emit must be contained. Register a throwing listener, drive // a clean turn, dispose, and assert the session was STILL removed. diff --git a/packages/ui/user-approval/README.md b/packages/ui/user-approval/README.md index 821e98638f..1c573cdf3f 100644 --- a/packages/ui/user-approval/README.md +++ b/packages/ui/user-approval/README.md @@ -2,11 +2,11 @@ User-approval seam. Owns the `ctx.approval` service ([`ApprovalService`](src/index.ts)) and the one-shot permission vocabulary the harness shares: `ApprovalRequest` (agent + tool identity + reason + abort signal), the closed `ApprovalOutcome` union (`allowed-once` / `rejected` / `cancelled` / `unavailable`), the `ApprovalRequestId` brand pairing the two log-only audit events (`approval/asked` / `approval/decided`), and the `approval/request` waterfall the answerers listen on. It lives in the UI group because its purpose is human permission, while remaining channel-neutral: it depends only on Cordis and core vocabulary packages, never on a concrete UI. -The contract in one line: `ctx.approval.request(req)` puts exactly one question — "may this specific action proceed?" — to whatever answerers the deployment composed, and always resolves to an outcome, never rejects: an aborted signal yields `cancelled`, a throwing or missing answerer yields `unavailable`, and `allowed-once` is a grant for the single asked-about action, never a class of future ones. Acceptance is synchronous: the service shallow-freezes a detached request record before dispatch, preserving the exact `agent` and `AbortSignal` identities while making later caller mutation unable to redirect scope, payload, cancellation, or either audit event. Session observers run after an event enters the append-only log; if one throws, the service recognizes that the audit is already authoritative, contains the observer failure, and completes the pair. The one precondition: ask from inside an open turn — the audit pair is turn-enclosed by contract (the turn is the durable log's commit/replay boundary; a bare event between turns is crash-tail garbage on reload), so an idle ask throws before appending anything. +The contract in one line: `ctx.approval.request(req)` puts exactly one question — "may this specific action proceed?" — to whatever answerers the deployment composed, and its decision phase always resolves to an outcome: an aborted signal yields `cancelled`, a throwing or missing answerer yields `unavailable`, and `allowed-once` is a grant for the single asked-about action, never a class of future ones. Acceptance is synchronous: the service reads the request fields and `agent.session` binding once, requires object agent/session identities, a string `toolName`, optional string `callId`/`reason`, and an AbortSignal-shaped live capability, then shallow-freezes a detached request record while preserving the exact `agent` and signal identities. A malformed request rejects before any audit append; later caller mutation cannot redirect scope, payload, cancellation, policy lookup, or either audit event. The other precondition is an open turn on the captured session — the audit pair is turn-enclosed by contract (the turn is the durable log's commit/replay boundary; a bare event between turns is crash-tail garbage on reload), so an idle ask also rejects before appending. Session observers run after an event enters the append-only log; if one throws, the service recognizes that the audit is already authoritative, contains the observer failure, and completes the pair. The service is the mechanism, answerers are the policy. Answerers are `approval/request` waterfall listeners occupying a single decision slot: answer for an agent you own by returning an outcome without calling `next()`, or delegate an agent you don't recognize by calling `next()` — the chain's built-in default is `unavailable`, so a deployment with no answerer (headless, CI) fails closed with zero configuration. Dispatch is keyed by `req.agent`: a listener registered through `agent.ctx` receives only that agent's questions, while a plain-context listener receives every agent's. Registration order across sibling plugins is not load-order deterministic; compose one terminal answerer per deployment and use `prepend` listeners only for decide-or-delegate gates. -The seam also owns the per-session POLICY tier ([the sandbox RFC § Per-session mode switching](../../../docs/rfc/implemented/feature/2026-07-06-sandbox.md)): `ApprovalPolicy` is `'ask'` (delegate to the answerers) or `'never'` (deterministically reject without prompting anyone; the strict CI/unattended stance), with `effective = fold(the session's 'approval/policy' events, last one wins) ?? Config.policy` — the session log is the store, written only through `setApprovalPolicy(session, policy)`. The service decides `'never'` inside `request()` itself, before dispatching the waterfall (`'never'` → `'rejected'` with the audit pair still landing; no listener registration, including a later `prepend`, can precede it), states `'never'` — and only `'never'` in prose — in a per-agent prompt section, records either value with a source-owned header marker, and narrates a policy switch to the model in at most one coalesced `agent/pre-step` notice. The restart fallback reads the marker rather than deployment-controlled persona prose; attribution is positional (an override event after the last `request/header*` reads `changed by the user`, otherwise `changed by the operator/config`). +The seam also owns the per-session POLICY tier ([the sandbox RFC § Per-session mode switching](../../../docs/rfc/implemented/feature/2026-07-06-sandbox.md)): `ApprovalPolicy` is `'ask'` (delegate to the answerers) or `'never'` (deterministically reject without prompting anyone; the strict CI/unattended stance), with `effective = fold(the session's 'approval/policy' events, last one wins) ?? Config.policy` — the session log is the store, written only through `setApprovalPolicy(session, policy)`, which rejects any value outside that closed vocabulary before appending. The service decides `'never'` inside `request()` itself, before dispatching the waterfall (`'never'` → `'rejected'` with the audit pair still landing; no listener registration, including a later `prepend`, can precede it), states `'never'` — and only `'never'` in prose — in a per-agent prompt section, records either value with a source-owned header marker, and narrates a policy switch to the model in at most one coalesced `agent/pre-step` notice. The restart fallback reads the marker rather than deployment-controlled persona prose; attribution is positional (an override event after the last `request/header*` reads `changed by the user`, otherwise `changed by the operator/config`). One seam serves both ask paths of [the sandbox RFC](../../../docs/rfc/implemented/feature/2026-07-06-sandbox.md): the `tools/pre-execute` `ask` decision (routed by [`@deepseek-ai/dsh-tools`](../../core/tools/) when this service is mounted; degrading to deny when it is not), and the sandbox post-denial escalated retry (the bash tool's `sandbox_permissions` gate in [`@deepseek-ai/dsh-tool-bash`](../../bash/tool-bash/) — [the sandbox RFC § Escalation](../../../docs/rfc/implemented/feature/2026-07-06-sandbox.md)). The full design: [the approval-seam RFC](../../../docs/rfc/implemented/feature/2026-07-06-approval-seam.md). diff --git a/packages/ui/user-approval/src/index.ts b/packages/ui/user-approval/src/index.ts index 4fb50ba23b..93ccd973c7 100644 --- a/packages/ui/user-approval/src/index.ts +++ b/packages/ui/user-approval/src/index.ts @@ -223,12 +223,16 @@ function hasOpenTurn(events: readonly SessionEvent[]): boolean { * THE write path for a session's approval-policy override: appends exactly * one `approval/policy` event — the switch IS its event; nothing mutates * policy state out of band. Takes effect on the session's next ask and next - * prompt assembly (the consumers fold on every read). + * prompt assembly (the consumers fold on every read). Rejects a value outside + * {@link APPROVAL_POLICIES} before appending anything. * @param session - the session the override belongs to. * @param policy - the policy every subsequent ask for this session resolves * under (until the next switch). */ export function setApprovalPolicy(session: Session, policy: ApprovalPolicy): void { + if (!APPROVAL_POLICIES.includes(policy)) { + throw new TypeError('approval policy must be one of "ask" or "never"') + } session.append('approval/policy', { policy }) } @@ -238,8 +242,10 @@ export function setApprovalPolicy(session: Session, policy: ApprovalPolicy): voi * was asked — it deliberately does NOT carry tool arguments: a UI answerer * attaches the prompt to the already-streamed tool call via `callId` instead * of re-rendering the call. `request()` synchronously copies and shallow-freezes - * this record before crossing an asynchronous boundary. Scalar fields are - * detached; the `agent` and `signal` identity capabilities are preserved. + * this record before crossing an asynchronous boundary. It reads each field + * and the agent's session binding once, validates the public fixed-field + * contract before audit, and detaches the scalar values; the `agent` and live + * `signal` identity capabilities are preserved rather than cloned or frozen. */ export interface ApprovalRequest { /** @@ -264,6 +270,13 @@ export interface ApprovalRequest { signal?: AbortSignal } +/** Live signal capability accepted at the synchronous request boundary. */ +interface AcceptedSignal { + signal: AbortSignal + addEventListener: AbortSignal['addEventListener'] + removeEventListener: AbortSignal['removeEventListener'] +} + /** Plugin config. All optional — `static Config` supplies the defaults. */ export interface Config { /** @@ -297,7 +310,7 @@ export class ApprovalService extends Service { constructor(ctx: Context, public config: Config) { super(ctx, 'approval') - const effective = (agent: Agent): ApprovalPolicy => this.effectivePolicy(agent) + const effective = (agent: Agent): ApprovalPolicy => this.effectivePolicy(agent.session) // Visibility layer 1, scoped on the prompt registry so headless // compositions mount the seam without it: state the one deterministic @@ -345,7 +358,7 @@ export class ApprovalService extends Service { } // Same fold effectivePolicy performs — override is scanned here anyway // for POSITIONAL attribution; the default lives once, in the method. - const current = this.effectivePolicy(agent) + const current = this.effectivePolicy(session) const header = session.requestHeader() const told = narrated.get(session) ?? toldApprovalPolicy(header?.system) narrated.set(session, current) @@ -361,17 +374,21 @@ export class ApprovalService extends Service { } /** - * Ask the composed answerers to decide one request. Requires an open turn - * on the requesting agent's session — the audit pair below is turn-enclosed - * by contract (the turn is the log's commit/replay boundary; an idle append - * would be dropped as crash tail on reload) — and throws before appending - * anything when called idle; asking outside a turn is a deferred design. - * Within that precondition it always resolves to an outcome, never rejects: - * an aborted signal yields `'cancelled'`, a missing or throwing answerer - * yields `'unavailable'` (fail closed), and a rogue non-vocabulary return - * value is normalized to `'unavailable'`. The caller-owned request is - * synchronously snapshotted, so later mutation cannot split routing, - * dispatch payload, cancellation, or the audit pair across agents/sessions. + * Ask the composed answerers to decide one request. Synchronously reads each + * request field and the agent's session binding once, validates the fixed + * agent/session, string, and live-signal contracts, and rejects before any + * audit append when malformed. The signal remains the caller's exact live + * identity capability; it is neither cloned nor frozen. Requires an open + * turn on the accepted session — the audit pair below is turn-enclosed by + * contract (the turn is the log's commit/replay boundary; an idle append + * would be dropped as crash tail on reload) — and likewise throws before + * appending anything when called idle; asking outside a turn is a deferred + * design. Once accepted it always resolves to an outcome, never rejects: an + * aborted signal yields `'cancelled'`, a missing or throwing answerer yields + * `'unavailable'` (fail closed), and a rogue non-vocabulary return value is + * normalized to `'unavailable'`. The caller-owned request is synchronously + * snapshotted, so later mutation cannot split routing, dispatch payload, + * cancellation, policy lookup, or the audit pair across agents/sessions. * Appends the * `approval/asked`/`approval/decided` audit pair (log-only) around the * decision regardless of outcome. A synchronous session observer failure @@ -385,20 +402,73 @@ export class ApprovalService extends Service { // Accept one immutable request shape before the first async boundary. The // caller retains its record and may mutate it as soon as this async method // returns; identity capabilities stay live, but the record is never reread. - const agent = req.agent - const toolName = req.toolName - const callId = req.callId - const reason = req.reason - const signal = req.signal + const input: unknown = req + if (typeof input !== 'object' || input === null) { + throw new TypeError('approval.request() requires a request object') + } + const source = input as Record + const agentInput = source['agent'] + const toolName = source['toolName'] + const callId = source['callId'] + const reason = source['reason'] + const signalInput = source['signal'] + if (typeof agentInput !== 'object' || agentInput === null) { + throw new TypeError('approval request agent must be an object') + } + if (typeof toolName !== 'string') { + throw new TypeError('approval request toolName must be a string') + } + if (callId !== undefined && typeof callId !== 'string') { + throw new TypeError('approval request callId must be a string when provided') + } + if (reason !== undefined && typeof reason !== 'string') { + throw new TypeError('approval request reason must be a string when provided') + } + let acceptedSignal: AcceptedSignal | undefined + if (signalInput !== undefined) { + if (typeof signalInput !== 'object' || signalInput === null) { + throw new TypeError('approval request signal must be an AbortSignal when provided') + } + const signalRecord = signalInput as unknown as Record + const aborted = signalRecord['aborted'] + const addEventListener = signalRecord['addEventListener'] + const removeEventListener = signalRecord['removeEventListener'] + if (typeof aborted !== 'boolean' + || typeof addEventListener !== 'function' + || typeof removeEventListener !== 'function') { + throw new TypeError('approval request signal must be an AbortSignal when provided') + } + acceptedSignal = { + signal: signalInput as AbortSignal, + addEventListener: addEventListener as AbortSignal['addEventListener'], + removeEventListener: removeEventListener as AbortSignal['removeEventListener'], + } + } + const sessionInput = (agentInput as unknown as Record)['session'] + if (typeof sessionInput !== 'object' || sessionInput === null) { + throw new TypeError('approval request agent session must be an object') + } + const sessionRecord = sessionInput as unknown as Record + const events = sessionRecord['events'] + const append = sessionRecord['append'] + if (!Array.isArray(events)) { + throw new TypeError('approval request session events must be an array') + } + if (typeof append !== 'function') { + throw new TypeError('approval request session append must be a function') + } + const agent = agentInput as Agent + const session = sessionInput as Session + const acceptedCallId = callId as CallId | undefined + const signal = signalInput as AbortSignal | undefined const accepted: Readonly = Object.freeze({ agent, toolName, - ...callId !== undefined ? { callId } : {}, + ...acceptedCallId !== undefined ? { callId: acceptedCallId } : {}, ...reason !== undefined ? { reason } : {}, ...signal !== undefined ? { signal } : {}, }) - const session = accepted.agent.session - if (!hasOpenTurn(session.events)) { + if (!hasOpenTurn(events)) { throw new Error( 'approval.request() outside an open turn: the approval/asked + approval/decided audit pair ' + 'must be turn-enclosed (a bare event between turns is crash-tail garbage on reload). ' @@ -407,16 +477,16 @@ export class ApprovalService extends Service { } const id = ApprovalRequestId(randomUUID()) this.appendAudit(session, 'approval/asked', id, () => { - session.append('approval/asked', { + Reflect.apply(append, session, ['approval/asked', { id, toolName: accepted.toolName, ...accepted.callId !== undefined ? { callId: accepted.callId } : {}, ...accepted.reason !== undefined ? { reason: accepted.reason } : {}, - }) + }]) }) - const outcome = await this.decide(accepted) + const outcome = await this.decide(accepted, session, acceptedSignal) this.appendAudit(session, 'approval/decided', id, () => { - session.append('approval/decided', { id, outcome }) + Reflect.apply(append, session, ['approval/decided', { id, outcome }]) }) return outcome } @@ -451,22 +521,30 @@ export class ApprovalService extends Service { * The session's effective policy: its own `approval/policy` fold, else the * configured default (the schema already defaulted an omitted policy to * `'ask'`; the `??` only narrows the optional-input TYPE). - * @param agent - the agent whose session's policy applies. - * @returns the policy every ask for this agent resolves under right now. + * @param session - the exact accepted session whose policy applies. + * @returns the policy every ask for this session resolves under right now. */ - private effectivePolicy(agent: Agent): ApprovalPolicy { - return effectiveApprovalPolicy(agent.session.events) ?? this.config.policy ?? 'ask' + private effectivePolicy(session: Session): ApprovalPolicy { + return effectiveApprovalPolicy(session.events) ?? this.config.policy ?? 'ask' } - /** Dispatch the waterfall, contained and raced against the accepted signal. */ - private async decide(req: Readonly): Promise { - if (req.signal?.aborted) return 'cancelled' + /** + * Dispatch the waterfall, contained and raced against the accepted signal. + * @param req - the detached public request snapshot. + * @param session - the captured session used for policy lookup. + * @param acceptedSignal - the validated live signal capability, if supplied. + * @returns the normalized closed outcome. + */ + private async decide( + req: Readonly, session: Session, acceptedSignal: AcceptedSignal | undefined, + ): Promise { + if (acceptedSignal?.signal.aborted) return 'cancelled' // The 'never' policy is decided HERE, before any dispatch: a listener // registered with `prepend: true` after this service mounts would sit // ahead of any gate LISTENER, so a listener-shaped gate cannot keep the // documented promise that 'never' rejects deterministically regardless // of registration order — only the service's own request path can. - if (this.effectivePolicy(req.agent) === 'never') return 'rejected' + if (this.effectivePolicy(session) === 'never') return 'rejected' // Enter the promise chain BEFORE dispatching: a listener that throws // SYNCHRONOUSLY (before its first await) must land in the same rejection // path as an async one — `Promise.resolve(call())` would let it escape @@ -484,13 +562,13 @@ export class ApprovalService extends Service { // tool call open — the seam contains its callbacks. () => 'unavailable', ) - const signal = req.signal - if (signal === undefined) return answer + if (acceptedSignal === undefined) return answer + const { signal, addEventListener, removeEventListener } = acceptedSignal return await new Promise((resolve) => { const onAbort = () => { resolve('cancelled') } - signal.addEventListener('abort', onAbort, { once: true }) + addEventListener.call(signal, 'abort', onAbort, { once: true }) void answer.then((outcome) => { - signal.removeEventListener('abort', onAbort) + removeEventListener.call(signal, 'abort', onAbort) // After an abort won the race this resolve is a settled-promise no-op: // the late answer is discarded by construction. resolve(outcome) diff --git a/packages/ui/user-approval/tests/approval.spec.ts b/packages/ui/user-approval/tests/approval.spec.ts index 0e69b8dfd7..831b50111b 100644 --- a/packages/ui/user-approval/tests/approval.spec.ts +++ b/packages/ui/user-approval/tests/approval.spec.ts @@ -39,6 +39,158 @@ function requestOf(agent: Agent, overrides: Partial = {}): Appr } describe('ApprovalService.request', () => { + it('rejects malformed fixed fields and identities before appending or dispatching', async () => { + const ctx = await mounted() + const consulted = vi.fn() + ctx.on('approval/request', () => { + consulted() + return Promise.resolve('allowed-once') + }) + const { agent, appended } = fakeAgent() + const badSessionAppends: Array> = [] + const badSession = (events: unknown, append: unknown): Agent => ({ + session: { events, append }, + }) as unknown as Agent + const appendSpy = (): ReturnType => { + const append = vi.fn() + badSessionAppends.push(append) + return append + } + const validSignalShape = { + aborted: false, + addEventListener: () => {}, + removeEventListener: () => {}, + } + const cases: Array<{ request: unknown; message: string }> = [ + { request: null, message: 'requires a request object' }, + { request: 1, message: 'requires a request object' }, + { request: { agent: null, toolName: 'echo' }, message: 'agent must be an object' }, + { request: { agent: 1, toolName: 'echo' }, message: 'agent must be an object' }, + { request: { agent, toolName: 1 }, message: 'toolName must be a string' }, + { request: { agent, toolName: 'echo', callId: 1 }, message: 'callId must be a string' }, + { request: { agent, toolName: 'echo', reason: 1 }, message: 'reason must be a string' }, + { request: { agent, toolName: 'echo', signal: null }, message: 'signal must be an AbortSignal' }, + { request: { agent, toolName: 'echo', signal: 1 }, message: 'signal must be an AbortSignal' }, + { + request: { agent, toolName: 'echo', signal: { ...validSignalShape, aborted: 'no' } }, + message: 'signal must be an AbortSignal', + }, + { + request: { agent, toolName: 'echo', signal: { ...validSignalShape, addEventListener: 1 } }, + message: 'signal must be an AbortSignal', + }, + { + request: { agent, toolName: 'echo', signal: { ...validSignalShape, removeEventListener: 1 } }, + message: 'signal must be an AbortSignal', + }, + { + request: { agent: { session: null }, toolName: 'echo' }, + message: 'agent session must be an object', + }, + { + request: { agent: { session: 1 }, toolName: 'echo' }, + message: 'agent session must be an object', + }, + { + request: { agent: badSession(null, appendSpy()), toolName: 'echo' }, + message: 'session events must be an array', + }, + { + request: { agent: badSession([{ type: 'turn/start' }], 1), toolName: 'echo' }, + message: 'session append must be a function', + }, + ] + + for (const { request, message } of cases) { + await expect(ctx.approval.request(request as ApprovalRequest)).rejects.toThrow(message) + } + + expect(appended).toEqual([]) + for (const append of badSessionAppends) expect(append).not.toHaveBeenCalled() + expect(consulted).not.toHaveBeenCalled() + }) + + it('reads request fields, the agent session, and the session append method once', async () => { + const ctx = await mounted() + const { agent: acceptedSessionOwner, appended: acceptedAudit } = fakeAgent() + const { agent: replacementAgent, appended: replacementAudit } = fakeAgent() + const acceptedSession = acceptedSessionOwner.session + const acceptedAppend = acceptedSession.append.bind(acceptedSession) + const signal = new AbortController().signal + const reads = { + agent: 0, + toolName: 0, + callId: 0, + reason: 0, + signal: 0, + session: 0, + append: 0, + } + const session = { + events: acceptedSession.events, + get append(): Session['append'] { + reads.append += 1 + return reads.append === 1 ? acceptedAppend : undefined as unknown as Session['append'] + }, + } as Session + const agent = Object.defineProperty({}, 'session', { + enumerable: true, + get: () => { + reads.session += 1 + return reads.session === 1 ? session : replacementAgent.session + }, + }) as Agent + const request = Object.defineProperties({}, { + agent: { + enumerable: true, + get: () => (++reads.agent === 1 ? agent : null), + }, + toolName: { + enumerable: true, + get: () => (++reads.toolName === 1 ? 'stable-tool' : 1), + }, + callId: { + enumerable: true, + get: () => (++reads.callId === 1 ? CallId('stable-call') : {}), + }, + reason: { + enumerable: true, + get: () => (++reads.reason === 1 ? 'stable reason' : {}), + }, + signal: { + enumerable: true, + get: () => (++reads.signal === 1 ? signal : {}), + }, + }) as ApprovalRequest + let received: ApprovalRequest | undefined + ctx.on('approval/request', (accepted) => { + received = accepted + return Promise.resolve('allowed-once') + }) + + await expect(ctx.approval.request(request)).resolves.toBe('allowed-once') + + expect(reads).toEqual({ + agent: 1, + toolName: 1, + callId: 1, + reason: 1, + signal: 1, + session: 1, + append: 1, + }) + expect(received).toMatchObject({ + agent, + toolName: 'stable-tool', + callId: 'stable-call', + reason: 'stable reason', + signal, + }) + expect(Object.isFrozen(received)).toBe(true) + expect(acceptedAudit.map(event => event.type)).toEqual(['approval/asked', 'approval/decided']) + expect(replacementAudit).toEqual([]) + }) + it('throws before appending anything when no turn has ever opened (idle ask)', async () => { const ctx = await mounted() const { agent, appended } = fakeAgent([]) @@ -420,6 +572,15 @@ describe('approval policy (the approval/policy fold)', () => { expect(session.events.at(-1)).toMatchObject({ type: 'approval/policy', data: { policy: 'ask' } }) }) + it('rejects a policy outside the closed vocabulary before appending', () => { + const append = vi.fn() + const session = { append } as unknown as Session + + expect(() => { setApprovalPolicy(session, 'sometimes' as Parameters[1]) }) + .toThrow('approval policy must be one of "ask" or "never"') + expect(append).not.toHaveBeenCalled() + }) + it('defaults a schema-less construction to ask (the ?? narrows the optional TYPE)', async () => { // Direct construction bypasses the plugin schema (the SystemPrompt-test // precedent for covering a defaulted Config field's type-narrowing ??). diff --git a/packages/workflow/workflow-workerthread/README.md b/packages/workflow/workflow-workerthread/README.md index d7cd9ed64c..2ea1a78c39 100644 --- a/packages/workflow/workflow-workerthread/README.md +++ b/packages/workflow/workflow-workerthread/README.md @@ -35,7 +35,7 @@ Values LEAVING the script (hook options/schemas, the script's return) are materi ## Cancellation, death, disposal -Per-run limits: a concurrency semaphore (`maxConcurrentAgents`), a total-`agent()` cap (`maxTotalAgents`), and a per-call item cap (`maxItemsPerCall`), all config. `cancel()` posts the cancel to the worker (its hooks start throwing `CANCELLED`; the script dies at its next await) and cancels every host-side child NOW on **both seam channels** — the shared request signal aborts AND each registered child's explicit `cancel()` is called host-side, because the seam leaves a provider free to honor either channel and a worker wedged in a synchronous spin could not relay its own per-child cancel RPCs (those later land as idempotent no-ops). Each provider-owned cancel callback is exception-contained independently, so a broken child cannot prevent peer cancellation or workflow settlement. The grace then arms: a run still unsettled `disposeGraceMs` later force-settles `cancelled` and the worker is **terminated**. A cancellation that lands before the body runs (the ready→go handshake) reports `cancelled` without executing anything; a worker `result` racing an in-flight host cancellation reports `cancelled` too (first-wins settlement — the seam-visible result had not settled when cancellation was requested); post-cancel `phase`/`log` narration is suppressed host-side, while cancelled children still deliver their paired `agent-end`. +Per-run limits: a concurrency semaphore (`maxConcurrentAgents`), a total-`agent()` cap (`maxTotalAgents`), and a per-call item cap (`maxItemsPerCall`), all config. `cancel()` posts the cancel to the worker (its hooks start throwing `CANCELLED`; the script dies at its next await) and cancels every host-side child NOW on **both seam channels** — the shared request signal aborts AND each registered child's explicit `cancel()` is called host-side, because the seam leaves a provider free to honor either channel and a worker wedged in a synchronous spin could not relay its own per-child cancel RPCs (those later land as idempotent no-ops). Each provider-owned cancel callback is exception-contained independently, so a broken child cannot prevent peer cancellation or workflow settlement. The caller's optional start-signal callback is retained by exact identity only while the run is live and removed at the first settlement or teardown, so a long-lived signal cannot retain completed `WorkerRun` instances. The grace then arms: a run still unsettled `disposeGraceMs` later force-settles `cancelled` and the worker is **terminated**. A cancellation that lands before the body runs (the ready→go handshake) reports `cancelled` without executing anything; a worker `result` racing an in-flight host cancellation reports `cancelled` too (first-wins settlement — the seam-visible result had not settled when cancellation was requested); post-cancel `phase`/`log` narration is suppressed host-side, while cancelled children still deliver their paired `agent-end`. A worker that dies unexpectedly (an OOM, a script reaching `process.exit` through the documented vm escape) settles the run `stopReason: 'error'` with the exit diagnostics — or `'cancelled'` when a cancel was in flight — and the host-side child registry is what winds every surviving child down. `dispose()` = cancel + immediate host-driven disposal of every registered child (a wedged worker can relay no dispose RPC, so child teardown overlaps the grace instead of starting after it; the worker's own dispose RPCs join the same per-child disposal) + bounded wait (result, then child-registry quiescence, capped by the grace) + unconditional `worker.terminate()`: the thread never outlives its run. Before an ordinary run settlement becomes observable, the host cancels every stray child on both channels too—even a fire-and-forget run still waiting on `started`, for which the worker has no handle yet—and `dispose()` then waits for their disposal (bounded by the grace) before returning. `agent-start`/`agent-end` pairing is host-guaranteed the same way: forwarded starts live in a ledger, worker-reported ends pair them on the graceful paths, and the termination paths (grace force-settle, worker death) synthesize the missing ends (outcome `cancelled`) before the run settles — a start still in flight across the force-settle can surface after `workflow/end`, immediately paired the same way. diff --git a/packages/workflow/workflow-workerthread/src/host.ts b/packages/workflow/workflow-workerthread/src/host.ts index b1d92ac1d0..e1c1a6542f 100644 --- a/packages/workflow/workflow-workerthread/src/host.ts +++ b/packages/workflow/workflow-workerthread/src/host.ts @@ -130,6 +130,9 @@ export class WorkerRun implements WorkflowRun { private readonly quiescenceWaiters: (() => void)[] = [] /** The per-run abort fanout every child start request carries. */ private readonly controller = new AbortController() + /** External start signal and the exact callback installed on it, retained only until first settle/teardown. */ + private inputSignal: AbortSignal | undefined + private inputSignalAbort: (() => void) | undefined private disposed: Promise | undefined constructor( @@ -159,8 +162,14 @@ export class WorkerRun implements WorkflowRun { }) if (signal?.aborted) { this.cancel('workflow start signal already aborted') - } else { - signal?.addEventListener('abort', () => { this.cancel('workflow signal aborted') }, { once: true }) + } else if (signal !== undefined) { + const onAbort = (): void => { + this.detachInputSignal() + this.cancel('workflow signal aborted') + } + this.inputSignal = signal + this.inputSignalAbort = onAbort + signal.addEventListener('abort', onAbort, { once: true }) } } @@ -217,6 +226,7 @@ export class WorkerRun implements WorkflowRun { */ dispose(): Promise { this.disposed ??= (async () => { + this.detachInputSignal() this.cancel('workflow disposed') for (const [callId, run] of [...this.children]) void this.disposeChild(callId, run) await Promise.race([ @@ -522,10 +532,21 @@ export class WorkerRun implements WorkflowRun { return { value: null, stopReason: 'cancelled', error: `workflow run cancelled: ${reason}`, agentsStarted } } - /** First settle wins; disarms the grace timer. */ + /** Remove the exact abort callback installed on the caller's start signal. */ + private detachInputSignal(): void { + const signal = this.inputSignal + const onAbort = this.inputSignalAbort + if (signal === undefined || onAbort === undefined) return + this.inputSignal = undefined + this.inputSignalAbort = undefined + signal.removeEventListener('abort', onAbort) + } + + /** First settle wins; disarms the grace timer and releases the caller signal. */ private settleResult(result: WorkflowResult): void { if (this.settled) return this.settled = true + this.detachInputSignal() clearTimeout(this.graceTimer) this.settleResolve(result) } diff --git a/packages/workflow/workflow-workerthread/tests/workflow-workerthread.spec.ts b/packages/workflow/workflow-workerthread/tests/workflow-workerthread.spec.ts index 46a42e4036..d5b16a8e2f 100644 --- a/packages/workflow/workflow-workerthread/tests/workflow-workerthread.spec.ts +++ b/packages/workflow/workflow-workerthread/tests/workflow-workerthread.spec.ts @@ -585,6 +585,41 @@ describe('dsh-workflow-workerthread', () => { await second.dispose() }) + it('removes the exact external abort callback on first settlement or teardown', async () => { + const { ctx, parent } = await setup() + const settledController = new AbortController() + const settledAdd = vi.spyOn(settledController.signal, 'addEventListener') + const settledRemove = vi.spyOn(settledController.signal, 'removeEventListener') + const completed = ctx.workflows.start({ ...scripted('return 123'), parent, signal: settledController.signal }) + const settledAbort = settledAdd.mock.calls.find(([type]) => type === 'abort')?.[1] + expect(typeof settledAbort).toBe('function') + + await expect(completed.result).resolves.toMatchObject({ value: 123, stopReason: 'completed' }) + expect(settledRemove).toHaveBeenCalledWith('abort', settledAbort) + const cancelAfterSettle = vi.spyOn(completed, 'cancel') + settledController.abort() + expect(cancelAfterSettle).not.toHaveBeenCalled() + cancelAfterSettle.mockRestore() + await completed.dispose() + + const manual = await setup({ manual: true }) + const teardownController = new AbortController() + const teardownAdd = vi.spyOn(teardownController.signal, 'addEventListener') + const teardownRemove = vi.spyOn(teardownController.signal, 'removeEventListener') + const tornDown = manual.ctx.workflows.start({ + ...scripted("return await agent('job')"), + parent: manual.parent, + signal: teardownController.signal, + }) + await waitFor(() => { expect(manual.provider.runs).toHaveLength(1) }) + const teardownAbort = teardownAdd.mock.calls.find(([type]) => type === 'abort')?.[1] + expect(typeof teardownAbort).toBe('function') + + const disposing = tornDown.dispose() + expect(teardownRemove).toHaveBeenCalledWith('abort', teardownAbort) + await disposing + }) + it('a child-start racing the host cancel is refused: no child starts after cancellation', async () => { const { ctx, parent, provider } = await setup({ manual: true }) // Cancel from INSIDE the log listener: the worker has already posted diff --git a/scripts/gen-doc-graphs.ts b/scripts/gen-doc-graphs.ts index 074cfd83d2..5459755be5 100644 --- a/scripts/gen-doc-graphs.ts +++ b/scripts/gen-doc-graphs.ts @@ -246,6 +246,13 @@ const SERVICE_ROLES: ServiceRole[] = [ ] const DYNAMIC_EVENT_DISPATCHERS: Array<{ event: string; pkg: string; method: string }> = [ + // Creation notifications preserve synchronous veto/rollback but observe + // returned promises explicitly so async listener rejection is not unhandled. + { event: 'agent/created', pkg: 'agent', method: 'events.dispatch' }, + { event: 'session/created', pkg: 'session', method: 'events.dispatch' }, + // Session disposal uses direct callback resolution so teardown contains each + // synchronous throw and returned-promise rejection independently. + { event: 'session/disposed', pkg: 'session', method: 'events.dispatch' }, // tools/result uses ctx.events.dispatch directly so the registry can await // every observer while containing each callback independently. { event: 'tools/result', pkg: 'tools', method: 'events.dispatch' }, diff --git a/scripts/type-equiv.manifest.json b/scripts/type-equiv.manifest.json index 4be54d89f4..e238583e69 100644 --- a/scripts/type-equiv.manifest.json +++ b/scripts/type-equiv.manifest.json @@ -17,6 +17,14 @@ { "doc": "docs/core-data-structures/core.md", "symbol": "ContinuationStop", "source": "packages/core/agent/src/types.ts" }, { "doc": "docs/core-data-structures/core.md", "symbol": "SessionStartSource", "source": "packages/core/agent/src/types.ts" }, + { "doc": "docs/core-data-structures/scope.md", "symbol": "ScopeKey", "source": "packages/core/scope/src/index.ts" }, + { "doc": "docs/core-data-structures/scope.md", "symbol": "Scoped", "source": "packages/core/scope/src/index.ts" }, + { "doc": "docs/core-data-structures/scope.md", "symbol": "Scope", "source": "packages/core/scope/src/index.ts" }, + + { "doc": "docs/core-data-structures/system-prompt.md", "symbol": "AssembleContext", "source": "packages/core/system-prompt/src/index.ts" }, + { "doc": "docs/core-data-structures/system-prompt.md", "symbol": "ToolProviderResult", "source": "packages/core/system-prompt/src/index.ts" }, + { "doc": "docs/core-data-structures/system-prompt.md", "symbol": "PromptProtection", "source": "packages/core/system-prompt/src/index.ts" }, + { "doc": "docs/core-data-structures/llm-streaming.md", "symbol": "StreamChunk", "source": "packages/llm/llm/src/types.ts" }, { "doc": "docs/core-data-structures/llm-streaming.md", "symbol": "TokenUsage", "source": "packages/llm/llm/src/types.ts" }, { "doc": "docs/core-data-structures/llm-streaming.md", "symbol": "ContentBlockMap", "source": "packages/llm/llm/src/types.ts" },