diff --git a/docs/core-data-structures/bash.md b/docs/core-data-structures/bash.md index df6c5ab16b..0cc82e5165 100644 --- a/docs/core-data-structures/bash.md +++ b/docs/core-data-structures/bash.md @@ -37,7 +37,7 @@ interface BashExecRequest { env?: Record | undefined /** * Opaque OWNER token for a background task — the consumer's isolation key - * (the tool layer passes the owning agent's `session.header.id`). The + * (the tool layer passes the owning agent's shared `id`). The * executor stores it on the task and exposes it via {@link BashExecutor.ownerOf}; * the executor itself NEVER interprets it (no access policy lives in the * seam — that is the consumer's job). Absent for foreground runs and for an 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 d64329262e..d75ec379fc 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 @@ -22,7 +22,7 @@ A new `cancel()` verb on the `Agent` interface — the single public stop primit ### 3. Bash owner token in the seam -Background-task ownership moved from a `tool-bash` plugin-local `Map` into the executor. `BashExecRequest` gains an optional `owner?: string`; the resolved `BashExecSpec` carries it as required-but-nullable `owner: string | undefined` (a forgotten owner is a visible `undefined`, never a silently-absent property). The executor stores the token on its task and exposes it via a new `BashExecutor.ownerOf(id): string | undefined` seam (NOT on the public `BashTask` — one read path, no redundant API). `tool-bash` deletes its `Map` entirely: it stamps `exec.agent?.session.header.id` as the owner at `start`, and `bash_output`/`bash_kill` compare `ctx.bash.ownerOf(id)` to the caller's token with `!== undefined` semantics (an empty-string token is still a real owner). The completion notice finds the live agent by scanning `ctx.get('agents')?.list()` for `agent.session.header.id === ownerToken` (read via `ctx.get` — `onTaskDone` runs on the bash fiber, a foreign fiber, where the `ctx.agents` proxy would throw). Because ownership now lives on the task in the executor (disposed with the `dsh-bash` fiber), it SURVIVES a `tool-bash` HMR reload — closing the old `XXX(tool-bash-owner-hmr)` gap. (The `onTaskDone` listener is still effect-scoped to `tool-bash`'s `apply`, so a completion landing during the reload gap still drops its one notice — the pre-existing reload-gap drop — but the ownership fence itself is HMR-proof.) +Background-task ownership moved from a `tool-bash` plugin-local `Map` into the executor. `BashExecRequest` gains an optional `owner?: string`; the resolved `BashExecSpec` carries it as required-but-nullable `owner: string | undefined` (a forgotten owner is a visible `undefined`, never a silently-absent property). The executor stores the token on its task and exposes it via a new `BashExecutor.ownerOf(id): string | undefined` seam (NOT on the public `BashTask` — one read path, no redundant API). `tool-bash` deletes its `Map` entirely: it stamps `exec.agent?.id` (the shared registry/session id) as the owner at `start`, and `bash_output`/`bash_kill` compare `ctx.bash.ownerOf(id)` to the caller's token with `!== undefined` semantics (an empty-string token is still a real owner). The completion notice finds the live agent by scanning `ctx.get('agents')?.list()` for `agent.id === ownerToken` (read via `ctx.get` — `onTaskDone` runs on the bash fiber, a foreign fiber, where the `ctx.agents` proxy would throw). Because ownership now lives on the task in the executor (disposed with the `dsh-bash` fiber), it SURVIVES a `tool-bash` HMR reload — closing the old `XXX(tool-bash-owner-hmr)` gap. (The `onTaskDone` listener is still effect-scoped to `tool-bash`'s `apply`, so a completion landing during the reload gap still drops its one notice — the pre-existing reload-gap drop — but the ownership fence itself is HMR-proof.) ## Verification @@ -35,7 +35,7 @@ These invariants hold and are pinned by tests: ## Session owner tokens are unique among live agents -The bash owner-token comparison relies on `session.header.id` being unique among live agents. Concurrent same-ID operations may both prepare privately, but publication enters the session and agent in order; `SessionStore.enter()` rejects a duplicate live session id, and every losing transaction rolls its private state back. A programmatic caller therefore cannot publish two live agents with one session token. The access *policy* (token comparison) stays in `tool-bash` (the consumer); the bash seam stores only an opaque `owner` string and never interprets it — the correct interface/implementation/consumer split. +The bash owner-token comparison relies on the shared `Agent.id`/`SessionId` being unique among live agents. Concurrent same-ID operations may both prepare privately, but publication enters the session and agent in order; `SessionStore.enter()` rejects a duplicate live session id, and every losing transaction rolls its private state back. A programmatic caller therefore cannot publish two live agents with one session token. The access *policy* (token comparison) stays in `tool-bash` (the consumer); the bash seam stores only an opaque `owner` string and never interprets it — the correct interface/implementation/consumer split. ## Alternatives considered diff --git a/docs/rfc/implemented/architecture/2026-06-20-branded-ids.md b/docs/rfc/implemented/architecture/2026-06-20-branded-ids.md index 4fe7f58168..fd5c5bf953 100644 --- a/docs/rfc/implemented/architecture/2026-06-20-branded-ids.md +++ b/docs/rfc/implemented/architecture/2026-06-20-branded-ids.md @@ -8,7 +8,7 @@ The harness brands `CallId` (`packages/llm/llm/src/brand.ts`) and the shared age **Gap 1 — unbranded cross-boundary IDs in the bash seam.** The background-task id is a plain `string`: `BashTask.id: string` (`packages/bash/bash/src/types.ts`), carried as `string` through the whole executor seam (`BashExecutor.get`/`ownerOf`/`readOutput`/`kill(id: string)` in `packages/bash/bash/src/index.ts`) and validated/passed as `string` by the model-facing tools (`validateTaskId`, `assertTaskAccess`, the `task_id` schema arg in `packages/bash/tool-bash/src/index.ts`). It is generated by a per-executor counter — `` `bash-${this.nextTaskId++}` `` in `packages/bash/bash-local/src/index.ts` — which gives it **exactly the same `name-N` shape as `SessionId`'s default** (`` `session-${++counter}` `` in `packages/core/session/src/index.ts`). A bash task id and a session id are trivially swappable at a call site and the compiler says nothing. This is the headline case the user asked about, and it is a model-facing id (the model passes `task_id` back to `bash_output`/`bash_kill`), so a confusion here is reachable from untrusted input. -The bash **owner token** is the related sub-case: `BashExecRequest.owner?: string` and `BashExecSpec.owner: string | undefined` (`packages/bash/bash/src/types.ts`) are documented as a deliberately *opaque* isolation key, but in every live caller the value IS the owning agent's `session.header.id` (`callerToken = (exec) => exec.agent?.session.header.id` in `packages/bash/tool-bash/src/index.ts`) — i.e. a `SessionId` wearing a `string` disguise. It is compared for access control (`owner !== callerToken(exec)`), so a mismatched-but-well-typed string here is a cross-session isolation bug the type system currently cannot catch. This is the same `session.header.id`-as-owner alias covered by the [unified agent/session identity decision](../simplification/2026-06-20-unify-agent-and-session-id.md). +The bash **owner token** is the related sub-case: `BashExecRequest.owner?: string` and `BashExecSpec.owner: string | undefined` (`packages/bash/bash/src/types.ts`) are documented as a deliberately *opaque* isolation key, but in every live caller the value IS the owning agent's shared `Agent.id`/`SessionId` (`callerToken = (exec) => exec.agent?.id` in `packages/bash/tool-bash/src/index.ts`) wearing a different seam-local name. It is compared for access control (`owner !== callerToken(exec)`), so a mismatched-but-well-typed string here is a cross-session isolation bug the type system currently cannot catch. This is the shared id alias covered by the [unified agent/session identity decision](../simplification/2026-06-20-unify-agent-and-session-id.md). **Gap 2 — brand erosion at the seams of the *already-branded* IDs.** Even `CallId` and `SessionId` decay back to bare `string` at exactly the places confusion is most likely: registry/store key types and public method params. Representative sites include the session store, the agent registry (both keyed by the shared `SessionId`), `ToolPresenter`'s call-id map, ACP's session-id records and loading set, and the persistence coordinator. A brand that is dropped at a collection key buys nothing on lookups — the value of the existing brands is partly unrealized. @@ -18,7 +18,7 @@ A type-only change. Brands are zero-cost casts; nothing about runtime behavior, - **Brand the bash task id.** Add `BashTaskId = Branded<'BashTaskId'>` plus its same-named factory in `packages/bash/bash/src/types.ts` (the package that *owns* the id), importing `Branded` from `@deepseek-ai/dsh-brand` exactly as `SessionId` does. The brand primitive lives in the dependency-free `dsh-brand` utility package precisely so `dsh-bash` can brand its ids by depending on it alone — it never pulls in `dsh-llm` (or `dsh-session`) just to reach `Branded`. Thread it through `BashTask.id`, the `BashExecutor` seam methods (`get`/`ownerOf`/`readOutput`/`kill`), the generation site in `dsh-bash-local` (brand the counter output once, at creation), and the `dsh-tool-bash` validate/access surface (`validateTaskId` returns a `BashTaskId`; `task_id` is branded at the tool boundary where the model's string arrives). -- **Mint a distinct `OwnerToken` brand.** Add `OwnerToken = Branded<'OwnerToken'>` in `packages/bash/bash/src/types.ts`; type `BashExecRequest.owner` / `BashExecSpec.owner` / `BashExecutor.ownerOf` as `OwnerToken | undefined`. The `dsh-tool-bash` consumer casts the agent's `session.header.id` (a `SessionId`) into an `OwnerToken` at the boundary — the one place the two vocabularies meet. The bash seam never imports `dsh-session`. (Rationale in the next section.) +- **Mint a distinct `OwnerToken` brand.** Add `OwnerToken = Branded<'OwnerToken'>` in `packages/bash/bash/src/types.ts`; type `BashExecRequest.owner` / `BashExecSpec.owner` / `BashExecutor.ownerOf` as `OwnerToken | undefined`. The `dsh-tool-bash` consumer casts the agent's shared `id` (`SessionId`) into an `OwnerToken` at the boundary — the one place the two vocabularies meet. The bash seam never imports `dsh-session`. (Rationale in the next section.) - **Stop the brand erosion.** Propagate the existing brands to the `Map` key types and public method params listed under Gap 2 — `Map`, `Map`, `get(id: SessionId)`, `Map`, ACP's `SessionId` surface, and the coordinator's `Map`. This is the larger mechanical share of the diff and the part that makes the *existing* brands actually load-bearing on lookups, not just on struct fields. diff --git a/packages/bash/bash/src/types.ts b/packages/bash/bash/src/types.ts index 39dbc162c6..c4c48645e1 100644 --- a/packages/bash/bash/src/types.ts +++ b/packages/bash/bash/src/types.ts @@ -117,7 +117,7 @@ export interface BashExecRequest { env?: Record | undefined /** * Opaque OWNER token for a background task — the consumer's isolation key - * (the tool layer passes the owning agent's `session.header.id`). The + * (the tool layer passes the owning agent's shared `id`). The * executor stores it on the task and exposes it via {@link BashExecutor.ownerOf}; * the executor itself NEVER interprets it (no access policy lives in the * seam — that is the consumer's job). Absent for foreground runs and for an diff --git a/packages/bash/tool-bash/README.md b/packages/bash/tool-bash/README.md index 800de0132c..c4ea498e07 100644 --- a/packages/bash/tool-bash/README.md +++ b/packages/bash/tool-bash/README.md @@ -34,7 +34,7 @@ Result text: stdout, then a `[stderr]` section, then status markers — `[sandbo ### Task ownership (cross-session isolation) -The owning agent's session token (`session.header.id`) is stamped onto the task at spawn — passed to the executor via `resolve({ …, owner })` and stored ON THE TASK inside the executor (the `dsh-bash` `ownerOf(id)` seam), **not** in a plugin-local map. `bash_output`/`bash_kill` compare `ctx.bash.ownerOf(id)` to the caller's token (`session.header.id`) with `!== undefined` semantics and reject a task owned by a *different* session with `task belongs to another session` (a task started with no agent — a non-loop caller — has no owner token and is open to anyone; a call with no `exec.agent` cannot access an owned task). Task ids are global and predictable, so under multi-session ACP this token check is the fence that stops one session's agent from reading or killing another session's background task. Because ownership lives on the task in the executor (disposed with the `dsh-bash` fiber), it **survives an independent `tool-bash` HMR reload** — closing the old plugin-local-map gap where a reload orphaned pre-reload tasks. (The `onTaskDone` listener is still effect-scoped to this plugin's `apply`, so a completion landing during the reload gap still drops its one notice — the pre-existing reload-gap drop — but the ownership fence itself is HMR-proof.) +The owning agent's shared registry/session id (`agent.id`) is stamped onto the task at spawn — passed to the executor via `resolve({ …, owner })` and stored ON THE TASK inside the executor (the `dsh-bash` `ownerOf(id)` seam), **not** in a plugin-local map. `bash_output`/`bash_kill` compare `ctx.bash.ownerOf(id)` to the caller's shared id with `!== undefined` semantics and reject a task owned by a *different* session with `task belongs to another session` (a task started with no agent — a non-loop caller — has no owner token and is open to anyone; a call with no `exec.agent` cannot access an owned task). Task ids are global and predictable, so under multi-session ACP this token check is the fence that stops one session's agent from reading or killing another session's background task. Because ownership lives on the task in the executor (disposed with the `dsh-bash` fiber), it **survives an independent `tool-bash` HMR reload** — closing the old plugin-local-map gap where a reload orphaned pre-reload tasks. (The `onTaskDone` listener is still effect-scoped to this plugin's `apply`, so a completion landing during the reload gap still drops its one notice — the pre-existing reload-gap drop — but the ownership fence itself is HMR-proof.) ## UI presentation @@ -42,7 +42,7 @@ These tools own how their calls render in a UI (an editor's tool-call card) via ## Background completion notices -When a background task finishes, a short notice is injected into the owning agent's session (`agent.inject()`, source `{kind: 'plugin', plugin: 'tool-bash'}`). The owning agent is found by its session token: the listener reads `ctx.bash.ownerOf(task.id)` and scans `ctx.get('agents')?.list()` for an agent whose `session.header.id` matches (read via `ctx.get` — `onTaskDone` runs on the bash fiber, a foreign fiber, so the `ctx.agents` proxy would throw). If no live agent carries that token — e.g. the owning session disconnected and its agent was disposed while the task ran on — the notice is dropped cleanly. Injection is **durable context for the next model request, not a wake-up** — an idle agent stays idle until something sends a message. That's why the tool descriptions tell the model to poll with `bash_output`. +When a background task finishes, a short notice is injected into the owning agent's session (`agent.inject()`, source `{kind: 'plugin', plugin: 'tool-bash'}`). The listener reads `ctx.bash.ownerOf(task.id)` and scans `ctx.get('agents')?.list()` for that shared agent/session id (read via `ctx.get` — `onTaskDone` runs on the bash fiber, a foreign fiber, so the `ctx.agents` proxy would throw). If no live agent carries that token — e.g. the owning session disconnected and its agent was disposed while the task ran on — the notice is dropped cleanly. Injection is **durable context for the next model request, not a wake-up** — an idle agent stays idle until something sends a message. That's why the tool descriptions tell the model to poll with `bash_output`. ## The tool builds its request from named args only diff --git a/packages/bash/tool-bash/src/index.ts b/packages/bash/tool-bash/src/index.ts index 31e6512ae0..fe07b80989 100644 --- a/packages/bash/tool-bash/src/index.ts +++ b/packages/bash/tool-bash/src/index.ts @@ -12,7 +12,7 @@ * `bash_output`. * * Task ownership: a background task's OWNER is an opaque token — the owning - * agent's `session.header.id` — passed to the executor at spawn + * agent's shared `id` — passed to the executor at spawn * (`resolve({ …, owner })`) and stored ON THE TASK inside the executor * (`@deepseek-ai/dsh-bash`'s `ownerOf(id)` seam), NOT in a plugin-local map. * `bash_output`/`bash_kill` compare `ctx.bash.ownerOf(id)` to the caller's token @@ -414,16 +414,12 @@ export function apply(ctx: Context): void { }) /** - * The caller's owner TOKEN — the owning agent's `session.header.id`, or - * `undefined` for a non-agent caller. Read `session.header.id` (NOT - * `session.id`): every other subsystem keys off the header id (the ACP bridge, - * both persistence backends), and the sibling `resolveWorkdir` already reads - * `session.header.cwd`, so using `session.id` here would be the asymmetry smell - * the conventions flag. The two are equal in production, but the header is the - * canonical identity. + * The caller's owner TOKEN — the owning agent's shared registry/session id, + * or `undefined` for a non-agent caller. Agent and Session deliberately have + * one live identity; workdir remains separate session metadata. */ const callerToken = (exec: { agent?: Agent }): OwnerToken | undefined => - exec.agent ? OwnerToken(exec.agent.session.header.id) : undefined + exec.agent ? OwnerToken(exec.agent.id) : undefined /** * Authorize a `bash_output`/`bash_kill` call against the task's stored owner @@ -443,18 +439,16 @@ export function apply(ctx: Context): void { } // Background completion → inject a notice into the owning agent's session. - // Find the live agent by its session id token via the agent registry, read + // Find the live agent by its shared registry/session token, read // opportunistically with `ctx.get('agents')` (NOT `ctx.agents`/static inject): // this listener runs from `task.done.then` on the bash fiber — a foreign // fiber — where the `ctx.agents` property proxy would throw through the // traceable shadow; `ctx.get(name)` is the topology-independent lookup. No - // registry mounted (`undefined`) → drop the notice. Match on - // `agent.session.header.id`, NOT the registry key: a config agent's id differs - // from its session id, and the owner token IS the session id. + // registry mounted (`undefined`) → drop the notice. ctx.bash.onTaskDone((task) => { const ownerToken = ctx.bash.ownerOf(task.id) if (ownerToken === undefined) return - const agent = ctx.get('agents')?.list().find(a => OwnerToken(a.session.header.id) === ownerToken) + const agent = ctx.get('agents')?.list().find(a => OwnerToken(a.id) === ownerToken) if (!agent) return try { agent.inject( diff --git a/packages/bash/tool-bash/tests/tools.spec.ts b/packages/bash/tool-bash/tests/tools.spec.ts index c68c445f0a..639dd8631a 100644 --- a/packages/bash/tool-bash/tests/tools.spec.ts +++ b/packages/bash/tool-bash/tests/tools.spec.ts @@ -47,22 +47,16 @@ async function setup() { } /** - * Build a fake {@link Agent} whose session token is `sessionId`, REGISTER it in - * `ctx.agents` (the completion-notice path finds the owning agent by scanning - * the registry for a matching `session.header.id`), and return it. The returned + * Build a fake {@link Agent} with the shared registry/session `sessionId`, + * REGISTER it in `ctx.agents`, and return it. The returned * agent is also passed to `execute` as `exec.agent` so it owns the spawned task. * The registration disposer is tracked so {@link unregisterFakeAgents} can drop * it (simulating the owning session disconnecting before a task completes). */ const fakeAgentDisposers = new Map Promise | void)[]>() function registerFakeAgent(ctx: Context, sessionId: string, inject: (...args: unknown[]) => void): Agent { - // The registry KEY (agent.id) is deliberately DIFFERENT from the session - // token (session.header.id), which is also the agent's durable id. The - // owner token IS the session id, so the notice path must find the agent by - // `session.header.id`, NOT the registry key. Using distinct values here makes - // the test fail if a regression matched on the wrong field (a same-value fake - // would pass either way — the "hits the line but not the scenario" trap). - const agent = { id: `agent-${sessionId}`, inject, session: { header: { version: 0, id: sessionId, createdAt: 0 } } } as unknown as Agent + const id = SessionId(sessionId) + const agent = { id, inject, session: new Session(id) } as unknown as Agent const dispose = ctx.agents.register(agent) const list = fakeAgentDisposers.get(ctx) ?? [] list.push(dispose) @@ -418,9 +412,9 @@ describe('background tools', () => { it('injects a completion notice into the owning agent (found via the registry by session token)', async () => { const ctx = await setup() const inject = vi.fn() - // The notice path looks the agent up in ctx.agents by its session token, so + // The notice path looks the agent up in ctx.agents by its shared id, so // the agent must be REGISTERED (not merely passed to execute). Mount a - // registry and register a fake whose session.header.id IS the owner token. + // registry and register a fake whose agent/session id IS the owner token. const agent = registerFakeAgent(ctx, 'bg', inject) const started = await ctx.tools.execute({ @@ -517,13 +511,14 @@ describe('background task ownership (cross-session isolation)', () => { function callAs(ctx: Context, agent: import('@deepseek-ai/dsh-agent').Agent | undefined, name: string, args: unknown) { return ctx.tools.execute({ callId: CallId(`own-${++callCounter}`), name, arguments: args, ...agent ? { agent } : {} }) } - // Ownership is by TOKEN (session.header.id), NOT agent object identity — so - // each agent needs a DISTINCT session id, else every fake yields the same + // Ownership is by the shared agent/session TOKEN, NOT agent object identity — + // so each agent needs a DISTINCT id, else every fake yields the same // token and the isolation tests pass for the wrong reason (all tasks owned by - // the same token). The impl reads `session.header.id`, so the fakes MUST carry - // it. - const fakeAgent = (sessionId: string) => - ({ inject: () => undefined, session: { header: { version: 0, id: sessionId, createdAt: 0 } } }) as unknown as import('@deepseek-ai/dsh-agent').Agent + // the same token). + const fakeAgent = (sessionId: string) => { + const id = SessionId(sessionId) + return { id, inject: () => undefined, session: new Session(id) } as unknown as import('@deepseek-ai/dsh-agent').Agent + } it('rejects bash_output/bash_kill for a task owned by a DIFFERENT session token', async () => { const ctx = await setup() @@ -548,7 +543,7 @@ describe('background task ownership (cross-session isolation)', () => { }) it('a DIFFERENT Agent object with the SAME session token may access the task (ownership is by token, not object identity)', async () => { - // Ownership fences by session.header.id, NOT Agent object identity. Two + // Ownership fences by the shared id, NOT Agent object identity. Two // distinct Agent objects sharing one session token (e.g. an agent re-created // on the same session) are the SAME owner. const ctx = await setup() @@ -638,8 +633,10 @@ describe('session-cwd routing (per-session workdir)', () => { return ctx.tools.execute({ callId: CallId(`cwd-${++callCounter}`), name: 'bash', arguments: args, ...agent ? { agent } : {} }) } // An agent whose session header carries a cwd (what session/new records). - const agentInCwd = (cwd: string) => - ({ inject: () => undefined, session: { header: { version: 0, id: 'c', createdAt: 0, cwd } } }) as unknown as import('@deepseek-ai/dsh-agent').Agent + const agentInCwd = (cwd: string) => { + const id = SessionId('c') + return { id, inject: () => undefined, session: { header: { version: 0, id, createdAt: 0, cwd } } } as unknown as import('@deepseek-ai/dsh-agent').Agent + } it('defaults bash to the agent\'s session cwd (not the server launch dir)', async () => { const ctx = await setup() @@ -1188,10 +1185,11 @@ describe('sandbox escalation (sandbox_permissions / justification)', () => { * enforces the enclosure. */ function escalationAgent(events: Array<{ type: string; data: Record }>): Agent { + const id = SessionId('sess-esc') return { - id: 'agent-esc', + id, session: { - header: { version: 0, id: 'sess-esc', createdAt: 0 }, + header: { version: 0, id, createdAt: 0 }, events: [{ type: 'turn/start' }], append: (type: string, data: Record) => { events.push({ type, data }) }, }, @@ -1406,7 +1404,7 @@ describe('per-session sandbox mode (the bash/sandbox-mode fold)', () => { session.append('turn/start', { turn: 1, trigger: { kind: 'message', source: { kind: 'user' } } }) const injected: string[] = [] const agent = { - id, + id: SessionId(id), session, inject: (content: { type: string; text: string }[]) => { injected.push(content[0]?.text ?? '') }, } as unknown as Agent diff --git a/packages/cordis/tool-cordis/src/api-catalog.ts b/packages/cordis/tool-cordis/src/api-catalog.ts index 23a8c8e1d0..36ec3d0e54 100644 --- a/packages/cordis/tool-cordis/src/api-catalog.ts +++ b/packages/cordis/tool-cordis/src/api-catalog.ts @@ -69,10 +69,11 @@ export const SERVICE_API: readonly ServiceApiEntry[] = [ 'async create(options: CreateAgentOptions): Promise', 'async resume(options: ResumeAgentOptions): Promise', 'register(agent: Agent): () => void', - 'enter(agent: Agent): () => void', + 'enter(agent: Agent, owner: Agent | undefined): () => void', 'announce(agent: Agent): void', 'get(id: SessionId): Agent | undefined', 'list(): Agent[]', + 'roots(): Agent[]', ], }, { diff --git a/packages/support/acp-snapshot/README.md b/packages/support/acp-snapshot/README.md index 26192f1cac..b95615abea 100644 --- a/packages/support/acp-snapshot/README.md +++ b/packages/support/acp-snapshot/README.md @@ -4,7 +4,7 @@ The ACP snapshot suite kit: the shared machinery behind the keyless snapshot tie Four layers, importable separately: -- **`launchAcpTestAgent` (launcher)** — boots an unbuilt ACP agent from a temp cwd, pins tsx to the repo tsconfig, connects the SDK client over a raw-byte stdout tee, collects session updates and stderr, surfaces asynchronous spawn failures through its startup lifecycle, fails closed on unhandled permission requests, and owns graceful or signalled shutdown. Shutdown waits for process exit before resolving or propagating a child error, so callers can remove owned paths after either outcome. Snapshot and ordinary e2e suites share this process boundary; a test supplies only agent paths, cwd, environment overrides, and any permission policy. +- **`launchAcpTestAgent` (launcher)** — boots an unbuilt ACP agent from a temp cwd, pins tsx to the repo tsconfig, connects the SDK client over a raw-byte stdout tee, collects session updates and stderr, surfaces asynchronous spawn failures through its startup lifecycle, fails closed on unhandled permission requests, and owns graceful or signalled shutdown. Shutdown waits for process exit, inherited stdio closure, and ACP parser exhaustion before resolving or propagating a child error, so captures are complete and callers can remove owned paths after either outcome. Snapshot and ordinary e2e suites share this process boundary; a test supplies only agent paths, cwd, environment overrides, and any permission policy. - **`runScenario` (harness)** — boots the real agent bin as a subprocess via tsx (unbuilt, Loader path), drives it over ACP JSON-RPC stdio from a deterministic `input.json` script, tees raw stdout for the golden + purity check, and harvests every persisted session JSONL (parent + subagent children, primary-first) after a graceful stdin-EOF shutdown. Parameterized by `AgentUnderTest` (`binScript`, `configPath`, `tsconfigPath` — absolute paths; the subprocess cwd is a temp dir outside the repo). - **Normalizers** — pure functions turning the two captured surfaces into stable text: `normalizeStdout` (JSON-RPC ids → first-seen sequence; UUIDs/cwd → tokens; doubles as the stdout-purity check), `normalizeSessionLog` (times zeroed, `seq` kept), `scrubSystemPrompts` (prompt text → `{{system}}` in every JSONL), and `scrubRequestHeaders` (the remaining header bulk → `{{tools}}`/`{{messagePrefix}}` outside each pin, structure kept — [pinned-header RFC](../../../docs/rfc/implemented/testing/2026-07-06-pin-request-header-content-in-one-scenario.md)). - **`defineAcpSnapshotSuite` (factory)** — registers the whole describe/it tree for a scenario table: per-scenario golden + re-persisted-log compares, record/refresh fixture write-back, the per-header-class pin (`system-prompt.golden.md` plus the JSONL's full tool schemas) with its live uniformity guard, and the fixture guard block (no orphan scenario dirs, required files present, exactly one pin per class, every JSONL prompt-scrubbed, non-pinning fixtures fully header-scrubbed). Each scenario directory's `session.jsonl` plus contiguous `session..jsonl` siblings are the ordered primary/child inventory; the scenario table does not duplicate their count. Must be called at vitest collection time. diff --git a/packages/support/acp-snapshot/src/launcher.ts b/packages/support/acp-snapshot/src/launcher.ts index 253e67fd5a..815753ae5f 100644 --- a/packages/support/acp-snapshot/src/launcher.ts +++ b/packages/support/acp-snapshot/src/launcher.ts @@ -65,7 +65,7 @@ export interface LaunchedAcpTestAgent { stderr(): string /** Resolve when a future session update matches the predicate. */ waitForUpdate(match: (update: SessionNotification['update']) => boolean): Promise - /** Gracefully close stdin, or send a signal, and wait for process exit. */ + /** Gracefully close stdin, or send a signal, then wait for process exit, inherited stdio closure, and ACP parser drain. */ close(signal?: NodeJS.Signals): Promise } @@ -132,7 +132,6 @@ export function launchAcpTestAgent(options: AcpTestLaunchOptions): LaunchedAcpTe }) child.stdout.on('end', () => { passthrough.push(null) - closeUpdateStream() }) const stream = ndJsonStream( Writable.toWeb(child.stdin) as WritableStream, @@ -163,6 +162,17 @@ export function launchAcpTestAgent(options: AcpTestLaunchOptions): LaunchedAcpTe ?? (() => Promise.resolve({ outcome: { outcome: 'cancelled' } })), }) const client = new ClientSideConnection(makeClient, stream) + // `exit` only reports the parent process's status. Descendants may retain + // inherited stdout/stderr handles and buffered ACP frames may still be + // crossing the SDK parser. Node's `close` follows stdio closure; the SDK's + // `closed` follows parser exhaustion. Capture both eagerly so a caller that + // invokes close after process exit still joins the complete drain boundary. + const stdioClosed = new Promise(resolve => child.once('close', () => { resolve() })) + const drained = Promise.all([stdioClosed, client.closed]).then(() => undefined) + // A caller may await a pending update without calling close(). Make natural + // stream exhaustion terminal for those waiters too, but only after the + // parser has dispatched every buffered frame. + void client.closed.then(closeUpdateStream) return { child, @@ -183,6 +193,7 @@ export function launchAcpTestAgent(options: AcpTestLaunchOptions): LaunchedAcpTe throw error } if (!isRunning(child)) { + await drained closeUpdateStream() return } @@ -194,6 +205,7 @@ export function launchAcpTestAgent(options: AcpTestLaunchOptions): LaunchedAcpTe childFailure, ]) if (failure === undefined) { + await drained closeUpdateStream() return } @@ -204,6 +216,7 @@ export function launchAcpTestAgent(options: AcpTestLaunchOptions): LaunchedAcpTe // callers may safely remove cwd/session resources after close rejects. child.kill('SIGKILL') await exited + await drained closeUpdateStream() throw failure }, diff --git a/packages/support/acp-snapshot/tests/fixtures/fake-acp-agent.ts b/packages/support/acp-snapshot/tests/fixtures/fake-acp-agent.ts index 861c9d2b04..9b0760213c 100644 --- a/packages/support/acp-snapshot/tests/fixtures/fake-acp-agent.ts +++ b/packages/support/acp-snapshot/tests/fixtures/fake-acp-agent.ts @@ -18,6 +18,7 @@ import { mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs' import { readdirSync } from 'node:fs' +import { spawn } from 'node:child_process' import { dirname, join } from 'node:path' import { randomUUID } from 'node:crypto' import { createInterface } from 'node:readline' @@ -50,6 +51,8 @@ interface Behavior { echoWorkspace?: boolean /** Write a line to stderr on boot (spec-side stderr-capture assertions). */ stderrNote?: string + /** Let a short-lived descendant retain stdio and emit one final ACP update plus stderr line after this parent exits. */ + lateInheritedOutput?: boolean /** Session logs to persist on stdin EOF. */ logs?: ScriptedLog[] /** Leave a stray FILE directly under the sessions root (harvest must skip it). */ @@ -256,6 +259,24 @@ function flushLogsAndExit(): void { writeFileSync(join(sessionsRoot, 'bucket-noise', 'notes.txt'), 'not a session log\n') } if (behavior.deleteSessionsRoot === true) rmSync(sessionsRoot, { recursive: true, force: true }) + if (behavior.lateInheritedOutput === true) { + const frame = JSON.stringify({ + jsonrpc: '2.0', + method: 'session/update', + params: { + sessionId, + update: { + sessionUpdate: 'agent_message_chunk', + content: { type: 'text', text: 'late inherited stdout' }, + }, + }, + }) + const code = [ + `setTimeout(() => process.stdout.write(${JSON.stringify(`${frame}\n`)}), 50)`, + `setTimeout(() => process.stderr.write(${JSON.stringify('late inherited stderr\n')}), 75)`, + ].join(';') + spawn(process.execPath, ['-e', code], { stdio: ['ignore', 1, 2] }).unref() + } process.exit(0) } diff --git a/packages/support/acp-snapshot/tests/harness.spec.ts b/packages/support/acp-snapshot/tests/harness.spec.ts index 192d36dc6c..051db6a0ba 100644 --- a/packages/support/acp-snapshot/tests/harness.spec.ts +++ b/packages/support/acp-snapshot/tests/harness.spec.ts @@ -91,6 +91,27 @@ describe('runScenario', () => { expect(exited).toBe(true) }) + it('waits for inherited stdio and buffered ACP parsing after the parent exits', { timeout: 20_000 }, async () => { + const { dir, fixtureFile } = await scenario({ lateInheritedOutput: true }) + const launched = launchAcpTestAgent({ + agent: AGENT, + cwd: dir, + env: { DSH_SNAPSHOT_FILE: fixtureFile }, + }) + await launched.client.initialize({ protocolVersion: PROTOCOL_VERSION, clientCapabilities: {} }) + await launched.client.newSession({ cwd: dir, mcpServers: [] }) + const lateUpdate = launched.waitForUpdate(update => + update.sessionUpdate === 'agent_message_chunk' + && update.content.type === 'text' + && update.content.text === 'late inherited stdout') + + await launched.close() + + await expect(lateUpdate).resolves.toMatchObject({ sessionUpdate: 'agent_message_chunk' }) + expect(launched.rawStdout()).toContain('late inherited stdout') + expect(launched.stderr()).toContain('late inherited stderr') + }) + it('drives a full turn: initialize (terminal caps), session, prompt, permission stub, harvest', { timeout: 20_000 }, async () => { const { fixtureFile } = await scenario({ permissionProbe: true, diff --git a/packages/ui/jsonrpc/README.md b/packages/ui/jsonrpc/README.md index 2324227ff7..735cfead0a 100644 --- a/packages/ui/jsonrpc/README.md +++ b/packages/ui/jsonrpc/README.md @@ -4,7 +4,7 @@ The **SDK server plugin** (`jsonrpc`): mounting it serves a stdio JSON-RPC serve ## Wiring -`inject: ['agents']` — the server creates one agent per SDK `sessionId` (get-or-create on `session/prompt`). A local subagent's shared agent/session id supplies `subagent.finished.childSessionId` directly; the server caches only parent lineage because the child may be disposed before `subagent/end`. Runs from remote providers are not reported through this local-session notification pair because they create no local `session/created`/`subagent.started` edge. The LLM seam is read opportunistically via `ctx.get('llm')` (not injected): when `initialize.model` has no registered adapter, the plugin mounts `dsh-llm-deepseek` for it (credentials from `$DEEPSEEK_API_KEY` / `$DEEPSEEK_BASE_URL`); a config-registered adapter for the model wins. Everything else — persistence, the tool stacks, the adapter set — comes from the surrounding `cordis.yml`. +`inject: ['agents']` — the server creates one agent per SDK `sessionId` (get-or-create on `session/prompt`). A local subagent's shared agent/session id supplies `subagent.finished.childSessionId` directly; the server caches runtime-local identity plus optional parent lineage because the child may be disposed before `subagent/end`, and the provider contract does not require lineage. Runs from remote providers are not reported because they create no local agent. The LLM seam is read opportunistically via `ctx.get('llm')` (not injected): when `initialize.model` has no registered adapter, the plugin mounts `dsh-llm-deepseek` for it (credentials from `$DEEPSEEK_API_KEY` / `$DEEPSEEK_BASE_URL`) — a config-registered adapter for the model wins. Everything else — persistence, the tool stacks, the adapter set — comes from the surrounding `cordis.yml`. ## Config diff --git a/packages/ui/jsonrpc/src/server.ts b/packages/ui/jsonrpc/src/server.ts index 3c803a6c8c..bf21f7fc2d 100644 --- a/packages/ui/jsonrpc/src/server.ts +++ b/packages/ui/jsonrpc/src/server.ts @@ -58,6 +58,11 @@ interface SessionRecord { activePrompt: boolean } +/** Runtime-local agent identity plus optional durable fork lineage. */ +interface LocalAgentRecord { + parentSessionId?: SessionId +} + /** * The SDK server over a booted harness context. Constructing it subscribes to * the context's `session/event`, `session/created`, `agent/created`, and @@ -71,7 +76,7 @@ export class HarnessSdkServer { private llmFiber: { dispose(): Promise } | undefined private readonly sessions = new Map() private readonly sessionCreations = new Map>() - private readonly subagentParents = new Map() + private readonly localAgents = new Map() private readonly disposers: (() => void)[] = [] private shutdownTask: Promise> | undefined private shuttingDown = false @@ -95,23 +100,24 @@ export class HarnessSdkServer { childSessionId: String(session.id), }) })) - // Cache parent lineage on creation: by the time `subagent/end` fires the - // child agent may already be disposed and gone from the registry. The child - // session id needs no cache because it is the shared agent/session id. + // Cache runtime-local identity and optional lineage on creation: by the + // time `subagent/end` fires the child agent may already be disposed and + // gone from the registry. Parent lineage is not required by the provider + // contract, so an empty record remains a load-bearing locality marker. this.disposers.push(ctx.on('agent/created', (agent) => { const parentSessionId = agent.session.header.parentSession - if (parentSessionId !== undefined) this.subagentParents.set(agent.id, parentSessionId) + this.localAgents.set(agent.id, parentSessionId === undefined ? {} : { parentSessionId }) })) this.disposers.push(ctx.on('subagent/end', (info: SubagentRunEndInfo) => { const agent = this.ctx.agents.get(info.id) - const cachedParentSessionId = this.subagentParents.get(info.id) - this.subagentParents.delete(info.id) - // This protocol reports LOCAL child sessions, paired with the - // session/created-driven subagent.started notification above. A remote - // provider may use a real remote SessionId for its run, but that session - // does not exist in this harness and therefore has no paired start event. - if (cachedParentSessionId === undefined && agent === undefined) return - const parentSessionId = cachedParentSessionId ?? agent?.session.header.parentSession + const cachedLocalAgent = this.localAgents.get(info.id) + this.localAgents.delete(info.id) + // This protocol reports LOCAL child sessions. A lineage-bearing child + // has the session/created-driven start notification above; a parentless + // local provider still gets its terminal notification. A remote provider + // has neither a cached creation nor a live local agent and is ignored. + if (cachedLocalAgent === undefined && agent === undefined) return + const parentSessionId = cachedLocalAgent?.parentSessionId ?? agent?.session.header.parentSession this.transport.notify('subagent.finished', { provider: info.provider, agentId: String(info.id), @@ -189,7 +195,7 @@ export class HarnessSdkServer { this.sessionCreations.clear() const records = [...this.sessions.values()] this.sessions.clear() - this.subagentParents.clear() + this.localAgents.clear() const failures: unknown[] = [] while (this.disposers.length > 0) { try { diff --git a/packages/ui/jsonrpc/tests/server.spec.ts b/packages/ui/jsonrpc/tests/server.spec.ts index e7f591b88f..163aee4c0c 100644 --- a/packages/ui/jsonrpc/tests/server.spec.ts +++ b/packages/ui/jsonrpc/tests/server.spec.ts @@ -272,15 +272,26 @@ describe('HarnessSdkServer', () => { meta: { cwd: storageDir, parentSession: SessionId('main') }, agentOptions: { model: 'deepseek' }, }) + const parentlessHandle = await ctx.agents.create({ + sessionId: SessionId('parentless-child-session'), + meta: { cwd: storageDir }, + agentOptions: { model: 'deepseek' }, + }) // The backend may dispose the child before publishing its run outcome; - // only the cached parent lineage should be needed at this point. + // cached locality must survive with or without optional parent lineage. await handle.dispose() + await parentlessHandle.dispose() await settleSubagent(ctx, parentHandle.agent, { provider: 'spawn', id: SessionId('child-session'), stopReason: 'completed', lastAssistantMessage: [{ type: 'text', text: 'child done' }], }) + await settleSubagent(ctx, parentHandle.agent, { + provider: 'spawn', + id: SessionId('parentless-child-session'), + stopReason: 'error', + }) expect(transport.notifications).toContainEqual({ method: 'subagent.finished', @@ -294,6 +305,16 @@ describe('HarnessSdkServer', () => { lastAssistantMessage: [{ type: 'text', text: 'child done' }], }, }) + expect(transport.notifications).toContainEqual({ + method: 'subagent.finished', + params: { + provider: 'spawn', + agentId: 'parentless-child-session', + childSessionId: 'parentless-child-session', + status: 'error', + stopReason: 'error', + }, + }) await parentHandle.dispose() await server.shutdown()