diff --git a/docs/rfc/010-acp-agent-client-protocol.md b/docs/rfc/010-acp-agent-client-protocol.md index 47f59792de..cf945bc377 100644 --- a/docs/rfc/010-acp-agent-client-protocol.md +++ b/docs/rfc/010-acp-agent-client-protocol.md @@ -2,7 +2,7 @@ Status: proposed -> **Implementation status (MVP landed):** steps 1, 2, 3, 4, 6, 7, 8 are implemented in `packages/acp` + `examples/acp-agent`. **Step 5 (the `session/request_permission` permission gate) is deferred** — the bridge ships a pass-through (tools run with the executor's full authority) marked `TODO(rfc010-permission-gate)`, and lays down only the `WeakMap` ownership seam the gate will build on. Status stays `proposed` until the gate lands. One further best-effort limitation is tracked as `TODO(rfc010-cancel-prestep)`: `session/cancel` aborts a running step and settles the RPC as `cancelled`, but a turn still queued (not yet started) when the cancel arrives may execute before the abort takes effect, pending a loop-level pre-step cancel. **Per-session `cwd` is now honored** (lifting the original "launch the server in the workspace root" restriction — see § Deferred): any absolute `cwd` is accepted and routed to the bash workdir via `session.header.cwd`, so an editor can open any project folder and N sessions can each target a different directory. +> **Implementation status (MVP landed):** steps 1, 2, 3, 4, 6, 7, 8 are implemented in `packages/acp` + `examples/acp-agent`. **Step 5 (the `session/request_permission` permission gate) is deferred** — the bridge ships a pass-through (tools run with the executor's full authority) marked `TODO(rfc010-permission-gate)`, and lays down only the `WeakMap` ownership seam the gate will build on. Status stays `proposed` until the gate lands. Queue-aware pre-step cancellation and per-agent disposal are follow-up seam work in [RFC 014](014-agent-lifecycle-and-ownership-seams.md). **Per-session `cwd` is honored**: `session/new` accepts any absolute cwd; `session/load` requires the request cwd to match the persisted session cwd so the editor and bash executor agree on the workspace. ## Problem @@ -10,7 +10,7 @@ The coding agent is reachable only through the readline `stdio-chat` plugin: it Editors are converging on the Agent Client Protocol (ACP), which Zed and others speak: JSON-RPC 2.0 over newline-delimited stdio, modeled on the Language Server Protocol. An editor boots the agent as a subprocess and exchanges `initialize` / `session/new` / `session/prompt`, rendering streamed `session/update` notifications and `session/request_permission` prompts. The goal is for the agent to be a drop-in ACP server — implement the protocol once and run in any ACP client, with no per-editor glue. -This RFC has a hard prerequisite on RFC 009: it assumes durable session persistence (the `SessionPersistence` service and the async `AgentLoop.resume` seam) is implemented, so resuming a session via `session/load` is in scope. None of those APIs exist yet — `AgentLoop` currently exposes only the synchronous `create` — so 010 must land after, or in the same change as, 009, and pins to 009's `resume(agentId, resumeSessionId)` contract. RFC 009 persists every `SessionEvent` verbatim (including `assistant/chunk`), so a loaded session has the stream chunks needed to replay turns to the client. +This RFC has a hard prerequisite on RFC 009: durable session persistence (the `SessionPersistence` service and the async `ctx.agents.resume` factory seam) is implemented, so resuming a session via `session/load` is in scope. RFC 009 persists every `SessionEvent` verbatim (including `assistant/chunk`), so a loaded session has the stream chunks needed to replay turns to the client. ## Proposal @@ -22,10 +22,10 @@ The mapping between ACP and existing harness seams — each row names the seam a | ACP (client ⇄ agent) | Harness seam | Notes | |---|---|---| -| `initialize` | static handler | negotiate `protocolVersion` (echo the supported version, else error); advertise text-only `promptCapabilities` and `loadSession: true`; report agent name/version | -| `session/new {cwd, mcpServers, additionalDirectories}` → `{sessionId}` | the `dsh-agent` create factory (see Dependency note + Plan) | the seam must accept `{ sessionId, meta }` so the ACP-generated `sessionId` becomes the live/persisted session id and the validated `cwd` is attached as the `SessionHeader` (today `AgentLoop.create(id)` hardcodes `${id}-session` and takes no metadata); reject a 2nd session (single-session MVP, see RFC 011); `cwd` validated (require absolute) — any absolute cwd is honored: it becomes the session's `SessionHeader.cwd` and the default bash workdir (per-session cwd, see § Deferred → RESOLVED), so the server need not launch in the workspace; `mcpServers` ignored (no `mcpCapabilities` advertised); non-empty `additionalDirectories` rejected for the MVP (the bridge cannot yet widen bash/tool filesystem scope, so silently ignoring them would desync the client's filesystem-scope UI) | -| `session/load {sessionId, cwd, mcpServers, additionalDirectories}` | the `dsh-agent` resume factory (RFC 009 + Dependency note) | load `{ meta, events }`, seed the session, re-derive history via `deriveMessages()`, replay prior turns to the client as `session/update` per the ACP load contract; `additionalDirectories` rejected as in `session/new` | -| `session/prompt {prompt}` | `agent.send()` (idle) | text blocks → `TextBlock`; reject image/audio per advertised capabilities; one in-flight prompt per session | +| `initialize` | static handler | negotiate `protocolVersion` (echo the supported version, else error); advertise baseline text/resource-link prompt support, no image/audio/embedded resources, and `loadSession: true`; report agent name/version | +| `session/new {cwd, mcpServers, additionalDirectories}` → `{sessionId}` | the `dsh-agent` create factory (see Dependency note + Plan) | the seam accepts `{ sessionId, meta }` so the ACP-generated `sessionId` becomes the live/persisted session id and the validated `cwd` is attached as the `SessionHeader`; N concurrent sessions are allowed (RFC 011); `cwd` validated (require absolute); non-empty `mcpServers` and `additionalDirectories` rejected for the MVP rather than silently ignored | +| `session/load {sessionId, cwd, mcpServers, additionalDirectories}` | the `dsh-agent` resume factory (RFC 009 + Dependency note) | load `{ meta, events }`, require the request `cwd` to match the persisted session cwd, seed the session, re-derive history via `deriveMessages()`, replay prior turns to the client as `session/update` per the ACP load contract; `mcpServers` and `additionalDirectories` rejected as in `session/new` | +| `session/prompt {prompt}` | `agent.send()` (idle) | baseline `text` and `resource_link` blocks are supported (`resource_link` renders as explicit text); reject image/audio/embedded resource per advertised capabilities; one in-flight prompt per session | | resolve `session/prompt` → `{stopReason}` | `agent/turn-end` (extended, see Plan) | map the harness kebab `TurnEndReason` to the ACP snake_case `StopReason` wire enum: `completed`→`end_turn`, `max-tokens`→`max_tokens`, `aborted`(cancel)→`cancelled`, plus `refusal`/`max_turn_requests` when applicable; honor the batch-into-one-turn and send-not-synchronously-running settle semantics | | `session/update: agent_message_chunk` | `agent/stream-chunk` `text-delta` only | do NOT also emit on `block-end(TextBlock)` — it carries the fully-assembled block and would duplicate the streamed text | | `session/update: agent_thought_chunk` | `agent/stream-chunk` `reasoning-delta` | | diff --git a/examples/acp-agent/README.md b/examples/acp-agent/README.md index 846e49fe78..a75279e3ed 100644 --- a/examples/acp-agent/README.md +++ b/examples/acp-agent/README.md @@ -21,15 +21,15 @@ Add to your Zed `settings.json` under `agent_servers`: "agent_servers": { "DeepSeek Harness": { "command": "pnpm", - "args": ["run", "demo:acp"], + "args": ["--dir", "/path/to/deepseek-harness", "run", "demo:acp"], "env": { "DEEPSEEK_API_KEY": "sk-…" } } } } ``` -The editor sets each session's `cwd` to the project it opens; the agent's bash tools run there (see the per-session `cwd` note in `packages/acp`), so the server does not need to be launched in the workspace. +The editor sets each session's `cwd` to the project it opens; the agent's bash tools run there (see the per-session `cwd` note in `packages/acp`), so launch the server from the harness repo with `pnpm --dir …` and let ACP carry the workspace path per session. ## MVP limitations -The bridge supports N concurrent sessions per connection, each in its own workspace `cwd` (RFC 011). Remaining limits: text-only prompts, `additionalDirectories` rejected (a session operates in its single `cwd`), and the tool-permission gate is deferred (`TODO(rfc010-permission-gate)` — tools run with the executor's full authority). See `packages/acp/README.md` for the full contract. +The bridge supports N concurrent sessions per connection, each in its own workspace `cwd` (RFC 011). Remaining limits: prompts support ACP's baseline `text` and `resource_link` blocks only, `additionalDirectories` and `mcpServers` are rejected, and the tool-permission gate is deferred (`TODO(rfc010-permission-gate)` — tools run with the executor's full authority). See `packages/acp/README.md` for the full contract. diff --git a/examples/acp-agent/start.ts b/examples/acp-agent/start.ts index 1c97c0c8c0..7c21ed9262 100644 --- a/examples/acp-agent/start.ts +++ b/examples/acp-agent/start.ts @@ -1,4 +1,4 @@ -import { pathToFileURL } from 'node:url' +import { fileURLToPath, pathToFileURL } from 'node:url' import { Context } from 'cordis' import Loader from '@cordisjs/plugin-loader' @@ -18,6 +18,8 @@ try { // ENOENT (no .env) is fine — rely on the ambient environment. } +process.chdir(fileURLToPath(new URL('../..', import.meta.url))) + const ctx = new Context() ctx.baseUrl = pathToFileURL(import.meta.dirname).href + '/' diff --git a/examples/acp-agent/tests/acp.e2e.ts b/examples/acp-agent/tests/acp.e2e.ts index e69a4bfc0f..08d3180840 100644 --- a/examples/acp-agent/tests/acp.e2e.ts +++ b/examples/acp-agent/tests/acp.e2e.ts @@ -27,11 +27,11 @@ import { */ const startScript = fileURLToPath(new URL('../start.ts', import.meta.url)) -// Resolve tsx's loader to an ABSOLUTE path: the subprocess runs with cwd set to -// a temp workdir (this test launches there and uses it as the session cwd; the -// bridge no longer requires cwd === the launch dir, but a temp dir keeps the -// test hermetic), where a bare `--import tsx` would not resolve from -// node_modules. import.meta.resolve gives the worktree's tsx regardless of cwd. +const repoRoot = fileURLToPath(new URL('../../../', import.meta.url)) +// Resolve tsx's loader to an ABSOLUTE path. The subprocess launches from the +// harness repo (so pnpm/package resolution is stable) while each ACP session's +// request cwd points at the temp workspace; import.meta.resolve gives the +// worktree's tsx regardless of launch cwd. const tsxLoader = fileURLToPath(import.meta.resolve('tsx')) interface Spawned { @@ -41,11 +41,11 @@ interface Spawned { stderr: string[] } -function spawnAcpAgent(cwd: string): Spawned { +function spawnAcpAgent(): Spawned { const child = spawn( process.execPath, ['--import', tsxLoader, startScript], - { cwd, env: { ...process.env }, stdio: ['pipe', 'pipe', 'pipe'] }, + { cwd: repoRoot, env: { ...process.env }, stdio: ['pipe', 'pipe', 'pipe'] }, ) const stderr: string[] = [] child.stderr.setEncoding('utf8') @@ -91,13 +91,16 @@ describe('acp-agent stdout purity (no key required)', () => { // present at boot, not valid — the key is used only on a real model call, // which this purity test never triggers). So this runs WITHOUT real creds. const child = spawn(process.execPath, ['--import', tsxLoader, startScript], { - cwd: workdir, + cwd: repoRoot, env: { ...process.env, DEEPSEEK_API_KEY: process.env.DEEPSEEK_API_KEY ?? 'sk-dummy-for-boot' }, stdio: ['pipe', 'pipe', 'pipe'], }) const out: string[] = [] + const stderr: string[] = [] child.stdout.setEncoding('utf8') + child.stderr.setEncoding('utf8') child.stdout.on('data', (c: string) => out.push(c)) + child.stderr.on('data', (c: string) => stderr.push(c)) // Send a single initialize request as a newline-delimited JSON-RPC frame. const req = JSON.stringify({ jsonrpc: '2.0', id: 1, method: 'initialize', params: { protocolVersion: PROTOCOL_VERSION, clientCapabilities: {} } }) @@ -108,7 +111,7 @@ describe('acp-agent stdout purity (no key required)', () => { child.kill('SIGKILL') const lines = out.join('').split('\n').filter(l => l.trim().length > 0) - expect(lines.length).toBeGreaterThan(0) + expect(lines.length, stderr.join('')).toBeGreaterThan(0) for (const line of lines) { // Every stdout line MUST parse as JSON (a JSON-RPC frame). A non-JSON // line means a logger/print leaked onto the protocol channel. @@ -120,7 +123,7 @@ describe('acp-agent stdout purity (no key required)', () => { describe.skipIf(!process.env.DEEPSEEK_API_KEY)('acp-agent e2e: real prompt over ACP', () => { it('runs a real turn and the agent writes the requested file (verified on disk)', async () => { workdir = await mkdtemp(join(tmpdir(), 'acp-e2e-')) - spawned = spawnAcpAgent(workdir) + spawned = spawnAcpAgent() const { client, updates } = spawned await client.initialize({ protocolVersion: PROTOCOL_VERSION, clientCapabilities: {} }) diff --git a/packages/acp/README.md b/packages/acp/README.md index 3d2fd15feb..a8b441bb82 100644 --- a/packages/acp/README.md +++ b/packages/acp/README.md @@ -23,12 +23,12 @@ It is a **client-driver / UI plugin**, the structured analogue of the readline ` | ACP method | Harness seam | Notes | |---|---|---| -| `initialize` | static | negotiate `protocolVersion`; advertise text-only `promptCapabilities` and `loadSession: true` | -| `session/new` | `ctx.agents.create({ sessionId, meta:{cwd} })` | creates a new session/agent; N concurrent sessions are allowed (RFC 011), keyed by id; `cwd` must be absolute (it becomes the session's workspace — see Per-session cwd); `additionalDirectories` rejected; `mcpServers` ignored | -| `session/load` | `ctx.agents.resume(...)` | replays the persisted event log to the client as `session/update` — the USER side (`user/message` → `user_message_chunk`), assistant text/reasoning (`assistant/chunk`), and tool calls/results (`tool/call` + `tool/result`). Re-loading an already-live id is rejected; the id's load slot is reserved (`loadingIds`) BEFORE the async resume so a pipelined load of the SAME id can't leak a second agent (distinct ids load concurrently). The resumed session keeps its PERSISTED header `cwd`, so its bash tools run in the original workspace; the requested `cwd` only needs to be absolute. After the async resume a `closed` re-check refuses to install a record if the bridge tore down mid-load | -| `session/prompt` | `agent.send()` | text-only; rejects image/audio and empty prompts; one in-flight prompt PER session (independent); settles on the OWNING turn's end (a turn that ends in `error` rejects the RPC) | +| `initialize` | static | negotiate `protocolVersion`; advertise baseline text/resource-link prompt support, no image/audio/embedded resources, and `loadSession: true` | +| `session/new` | `ctx.agents.create({ sessionId, meta:{cwd} })` | creates a new session/agent; N concurrent sessions are allowed (RFC 011), keyed by id; `cwd` must be absolute (it becomes the session's workspace — see Per-session cwd); non-empty `additionalDirectories` and `mcpServers` are rejected until those scopes are implemented | +| `session/load` | `ctx.agents.resume(...)` | replays the persisted event log to the client as `session/update` — the USER side (`user/message` → `user_message_chunk`), assistant text/reasoning (`assistant/chunk`), and tool calls/results (`tool/call` + `tool/result`). Re-loading an already-live id is rejected; the id's load slot is reserved (`loadingIds`) BEFORE the async resume so a pipelined load of the SAME id can't leak a second agent (distinct ids load concurrently). The resumed session keeps its PERSISTED header `cwd`, and the requested `cwd` must match it so editor UI and bash execution agree on the workspace. After the async resume a `closed` re-check refuses to install a record if the bridge tore down mid-load | +| `session/prompt` | `agent.send()` | accepts ACP baseline `text` and `resource_link` blocks; rejects image/audio/embedded resources and empty prompts; one in-flight prompt PER session (independent); settles on the OWNING turn's end (a turn that ends in `error` rejects the RPC) | | `session/cancel` | `agent.abort()` | aborts a running step + settles the prompt `cancelled` for ONLY that session — a cancel never touches another session's stream or prompt (see limitation below) | -| `session/update` | `session/event` | `agent_message_chunk` (text-delta), `agent_thought_chunk` (reasoning-delta), `user_message_chunk` (load replay), `tool_call`/`tool_call_update` | +| `session/update` | `session/event` | `agent_message_chunk` (text-delta), `agent_thought_chunk` (reasoning-delta), `user_message_chunk` (load replay only), `tool_call`/`tool_call_update` | ## Multi-session (RFC 011) @@ -38,7 +38,7 @@ Background-task isolation rides on `dsh-tool-bash`: bash task ids are global and ## Per-session cwd -Each session runs in its own workspace, recorded as the session's `SessionHeader.cwd`. On `session/new` the (absolute) request `cwd` becomes that header cwd; on `session/load` the resumed session keeps its PERSISTED header cwd (the request `cwd` is only shape-checked — it does not override the stored one), and a load whose persisted session has no absolute cwd is REJECTED up front via a metadata-only `list()` check, BEFORE resume constructs an agent (else bash would silently fall back to the server's launch dir, and a post-resume reject would leak the registered agent). `dsh-tool-bash` then defaults the bash workdir to the calling agent's `session.header.cwd` (an explicit model `workdir` still wins; a relative one resolves against the session cwd; with no session cwd the executor falls back to its own config / `process.cwd()`). So the server no longer has to be launched in the workspace — an editor can open any project folder, and N sessions over one connection can each target a different directory. (`additionalDirectories` is still rejected: widening the tool/filesystem scope beyond the single cwd is a separate sandbox concern.) +Each session runs in its own workspace, recorded as the session's `SessionHeader.cwd`. On `session/new` the (absolute) request `cwd` becomes that header cwd; on `session/load` the resumed session keeps its PERSISTED header cwd and the request `cwd` must match it (a mismatch is rejected up front) so the editor never believes tools run in one workspace while bash runs in another. A load whose persisted session has no absolute cwd is also rejected via a metadata-only `list()` check, BEFORE resume constructs an agent. `dsh-tool-bash` then defaults the bash workdir to the calling agent's `session.header.cwd` (an explicit model `workdir` still wins; a relative one resolves against the session cwd; with no session cwd the executor falls back to its own config / `process.cwd()`). So the server no longer has to be launched in the workspace — an editor can open any project folder, and N sessions over one connection can each target a different directory. (`additionalDirectories` and `mcpServers` are still rejected: widening tool/filesystem/protocol scope is separate work.) ## Settle-exactly-once @@ -53,7 +53,7 @@ Teardown reaches quiescence: for EVERY live session settle any pending prompt as - **`TODO(rfc010-permission-gate)`** — the `tools/execute` permission gate (`session/request_permission`) is NOT implemented; tools run with the executor's full authority. The `agent→sessionId` reverse map is in place so the gate can route a permission request (which receives only `exec.agent`) back to its originating session. RFC 010/011 stay `proposed` until the gate (and per-session permission ownership) land. - **`TODO(rfc010-cancel-prestep)`** — `session/cancel` (and teardown/disconnect) is honest RPC/UI cancellation plus best-effort abort: a *running* step is aborted, but a turn that is queued-but-not-yet-started (the gap before `agent.abort()` has an `AbortController` to signal) may still run to completion. This same window means disposal/disconnect can return while one short queued turn per session still runs, and a prompt accepted right after a pre-step cancel can be batched into the cancelled turn (the loop merges queued messages into one turn). A loop-level queue-aware cancel will close this; the single-in-flight-per-session rule bounds the worst case to one extra prompt per session. - **`TODO(rfc010-agent-disposal)`** — the factory (`ctx.agents.create`/`resume`) returns no per-agent disposer, so teardown aborts+drains each agent but cannot individually unregister it; on a bare client disconnect (no host dispose) the idled agents linger in `ctx.agents` until the host context disposes. A reconnect spins up a fresh context, so this strands no work; a per-agent disposal seam is the follow-up. -- **`additionalDirectories`** — rejected. A session operates in its single `cwd` (see Per-session cwd); widening the tool/filesystem scope to extra roots is a separate sandbox concern, not yet implemented. +- **`additionalDirectories` / `mcpServers`** — rejected. A session operates in its single `cwd` and no MCP bridge is wired yet; silently ignoring requested roots or servers would desync client expectations. ## stdout is the protocol @@ -61,14 +61,15 @@ The JSON-RPC frames go on stdout, so this plugin MUST run in an example that loa ## Running -`pnpm run demo:acp` boots `examples/acp-agent` (needs `DEEPSEEK_API_KEY`). Point an ACP client at it; for Zed, add to `agent_servers`: +`pnpm --dir /path/to/deepseek-harness run demo:acp` boots `examples/acp-agent` (needs `DEEPSEEK_API_KEY`). Point an ACP client at it; for Zed, add to `agent_servers`: ```json { "agent_servers": { "DeepSeek Harness": { "command": "pnpm", - "args": ["run", "demo:acp"] + "args": ["--dir", "/path/to/deepseek-harness", "run", "demo:acp"], + "env": { "DEEPSEEK_API_KEY": "sk-…" } } } } diff --git a/packages/acp/src/codec.ts b/packages/acp/src/codec.ts index 5ef31d00dd..dd28bc5ae6 100644 --- a/packages/acp/src/codec.ts +++ b/packages/acp/src/codec.ts @@ -59,8 +59,9 @@ export function turnEndToStopReason(reason: TurnEndReason): StopReason { /** * Translate a harness {@link ContentBlock} from a prompt into ACP content for * replay, or `undefined` for block kinds the bridge does not surface to the - * client as message content. Today only `text` maps (text-only - * `promptCapabilities`); `reasoning` is surfaced via `agent_thought_chunk` + * client as message content. Today only `text` maps; `resource_link` is an + * ACP prompt-only input rendered into text by {@link acpPromptToText}; + * `reasoning` is surfaced via `agent_thought_chunk` * streaming rather than as a message block, and `tool-call`/`tool-result`/ * `image` are handled by the tool-call update path or not advertised. */ @@ -70,34 +71,38 @@ export function harnessBlockToAcpContent(block: ContentBlock): AcpContentBlock | return { type: 'text', text: block.text } // reasoning → streamed as agent_thought_chunk, not a message block // tool-call / tool-result → the tool_call / tool_call_update path - // image → not advertised (text-only promptCapabilities) + // image → not advertised default: return undefined } } /** - * Extract plain text from an ACP prompt's content blocks, concatenating every - * `text` block. Non-text blocks are ignored here; the caller rejects a prompt - * carrying image/audio per the advertised text-only capabilities BEFORE - * calling this, so dropping them here only affects `resource`/`resource_link` - * (which carry no inline text to forward in the MVP). + * Extract plain text from an ACP prompt's content blocks. Text blocks are + * concatenated verbatim; resource links become explicit textual references so + * baseline ACP clients can point at files without the bridge silently dropping + * that context. */ export function acpPromptToText(prompt: readonly AcpContentBlock[]): string { return prompt - .filter((block): block is AcpContentBlock & { type: 'text'; text: string } => block.type === 'text') - .map(block => block.text) + .flatMap((block): string[] => { + switch (block.type) { + case 'text': + return [block.text] + case 'resource_link': + return [`\n[resource_link name=${JSON.stringify(block.name)} uri=${JSON.stringify(block.uri)}]\n`] + default: + return [] + } + }) .join('') } /** - * Whether an ACP prompt contains any content the text-only bridge cannot - * accept — i.e. ANY non-`text` block (image, audio, `resource`, `resource_link`, - * …). The caller rejects such a prompt up front rather than silently dropping - * the unsupported parts: a prompt like `[text, resource_link]` carries context - * the model would otherwise never see, so running it text-only would be silent - * data loss. When richer block kinds are supported, narrow this. + * Whether an ACP prompt contains content the bridge cannot accept. Baseline ACP + * requires `text` and `resource_link`; richer inline payloads (`resource`, + * image, audio, …) are rejected rather than silently dropped. */ export function promptHasUnsupportedContent(prompt: readonly AcpContentBlock[]): boolean { - return prompt.some(block => block.type !== 'text') + return prompt.some(block => block.type !== 'text' && block.type !== 'resource_link') } diff --git a/packages/acp/src/index.ts b/packages/acp/src/index.ts index 03e0d17a5e..710db476d7 100644 --- a/packages/acp/src/index.ts +++ b/packages/acp/src/index.ts @@ -8,7 +8,7 @@ * the existing `agent/*` event taxonomy, the `dsh-agent` create/resume factory, * and `dsh-session-persistence` (for `session/load`). It maps: * - * - `initialize` → protocol-version negotiation, text-only capabilities + * - `initialize` → protocol-version negotiation, baseline prompt capabilities * - `session/new` → `ctx.agents.create({ sessionId, meta:{cwd} })` * - `session/load` → `ctx.agents.resume(...)` then replay the event log * - `session/prompt` → `agent.send()`, settle on the owning turn's end (a turn @@ -259,7 +259,7 @@ export function apply(ctx: Context, config: AcpConfig): void { ctx.on('session/event', (session, event: SessionEvent) => { const rec = sessions.get(session.header.id) if (rec === undefined) return - streamSessionEventUpdate(rec.sessionId, event, notify) + streamSessionEventUpdate(rec.sessionId, event, notify, { includeUserMessages: false }) const inflight = rec.inflight if (inflight === undefined) return if (event.type === 'turn/start') { @@ -360,7 +360,7 @@ export function apply(ctx: Context, config: AcpConfig): void { agentInfo: { name: agentName, version: agentVersion }, agentCapabilities: { loadSession: true, - // text-only: no image/audio/embeddedContext, no mcpCapabilities + // Baseline text/resource_link only: no image/audio/embedded resource, no mcpCapabilities. promptCapabilities: { image: false, audio: false, embeddedContext: false }, }, authMethods: [], @@ -419,6 +419,9 @@ export function apply(ctx: Context, config: AcpConfig): void { `session ${params.sessionId} has no absolute persisted cwd; cannot determine its workspace (it predates per-session cwd, or was created without one)`, ) } + if (meta !== undefined && meta.cwd !== params.cwd) { + throw invalidParams(`session ${params.sessionId} cwd mismatch: persisted ${meta.cwd}, requested ${params.cwd}`) + } const agent = await ctx.agents.resume({ agentId: params.sessionId, resumeSessionId: params.sessionId, @@ -459,7 +462,7 @@ export function apply(ctx: Context, config: AcpConfig): void { throw invalidParams('a prompt is already in flight for this session') } if (promptHasUnsupportedContent(params.prompt)) { - throw invalidParams('only text prompt content is supported (text-only promptCapabilities); image/audio/resource blocks are rejected rather than silently dropped') + throw invalidParams('only text and resource_link prompt content is supported; image/audio/resource blocks are rejected rather than silently dropped') } const text = acpPromptToText(params.prompt) if (text.trim().length === 0) { @@ -615,19 +618,22 @@ export function agentOptions(config: AcpConfig): { model?: string; systemPrompt? * bash workdir — the request cwd does not override it. * Any absolute path is accepted (the per-session cwd flows to the bash executor * — see `dsh-tool-bash`), so the server no longer has to launch in the - * workspace. `additionalDirectories` must still be empty: widening the - * tool/filesystem scope beyond the single cwd is a separate, unimplemented - * concern (a sandbox seam), and silently ignoring extra roots would desync the - * client's filesystem-scope UI. Both request shapes carry `cwd: string` and - * `additionalDirectories?: string[]`, so one validator covers both. + * workspace. `additionalDirectories` and `mcpServers` must still be empty: + * widening tool/filesystem/protocol scope is separate, unimplemented work, and + * silently ignoring requested roots/servers would desync the client's UI. Both + * request shapes carry the same workspace/scope fields, so one validator covers + * both. */ -function validateWorkspaceParams(params: { cwd: string; additionalDirectories?: string[] }): void { +function validateWorkspaceParams(params: { cwd: string; additionalDirectories?: string[]; mcpServers?: unknown[] }): void { if (!isAbsolute(params.cwd)) { throw invalidParams(`cwd must be an absolute path: ${params.cwd}`) } if (params.additionalDirectories !== undefined && params.additionalDirectories.length > 0) { throw invalidParams('additionalDirectories is not supported in this MVP') } + if (params.mcpServers !== undefined && params.mcpServers.length > 0) { + throw invalidParams('mcpServers is not supported in this MVP') + } } /** @@ -637,8 +643,9 @@ function validateWorkspaceParams(params: { cwd: string; additionalDirectories?: * identical update stream from the same event log. * * - `assistant/chunk` text-delta/reasoning-delta → message/thought chunks - * - `user/message` → `user_message_chunk` (text blocks) — so a `session/load` - * replay reconstructs the USER side of each turn, not just the agent's + * - `user/message` → `user_message_chunk` during load replay only — so a + * loaded transcript reconstructs the USER side of each turn without echoing + * a live `session/prompt` back to the client * - `tool/call` → `tool_call` (pending) * - `tool/result` → `tool_call_update` (completed/failed) * @@ -649,7 +656,9 @@ export function streamSessionEventUpdate( sessionId: string, event: SessionEvent, notify: (notification: SessionNotification) => void, + options: { includeUserMessages?: boolean } = {}, ): void { + const includeUserMessages = options.includeUserMessages ?? true switch (event.type) { case 'assistant/chunk': { const chunk = event.data.chunk @@ -661,9 +670,10 @@ export function streamSessionEventUpdate( return } case 'user/message': { + if (!includeUserMessages) return // Replay the user's prompt so a loaded session shows both sides of each - // turn. Only text blocks carry inline content the bridge surfaces (the - // prompt path is text-only); other block kinds produce no chunk. + // turn. Live prompt turns suppress this path to avoid duplicating what + // the client just sent. for (const block of event.data.content) { const content = harnessBlockToAcpContent(block) if (content !== undefined) { diff --git a/packages/acp/tests/bridge.spec.ts b/packages/acp/tests/bridge.spec.ts index d537f4f684..dd10a88bcb 100644 --- a/packages/acp/tests/bridge.spec.ts +++ b/packages/acp/tests/bridge.spec.ts @@ -105,19 +105,20 @@ describe('acp bridge', () => { })).rejects.toThrow(/text/) }) - it('rejects a prompt carrying a non-text block alongside text (no silent context loss)', async () => { - // A text + resource_link prompt must be rejected, not run text-only with the - // resource silently dropped — that would feed the model an incomplete prompt. - harness = await makeBridgeHarness({ storageDir, script: [] }) + it('accepts a resource_link prompt by rendering the link into the text sent to the agent', async () => { + harness = await makeBridgeHarness({ storageDir, script: [textResponse('ok')] }) await harness.client.initialize({ protocolVersion: PROTOCOL_VERSION, clientCapabilities: {} }) const { sessionId } = await harness.client.newSession({ cwd: process.cwd(), mcpServers: [] }) - await expect(harness.client.prompt({ + const result = await harness.client.prompt({ sessionId, prompt: [ { type: 'text', text: 'fix the bug in' }, { type: 'resource_link', uri: 'file:///x.ts', name: 'x.ts' }, ], - })).rejects.toThrow(/text/) + }) + expect(result.stopReason).toBe('end_turn') + const user = harness.ctx.agents.get(sessionId)!.session.events.find(event => event.type === 'user/message') + expect(JSON.stringify(user)).toContain('resource_link') }) it('rejects a prompt for an unknown session', async () => { diff --git a/packages/acp/tests/codec.spec.ts b/packages/acp/tests/codec.spec.ts index 58c5a4bcdf..38a7a6cb41 100644 --- a/packages/acp/tests/codec.spec.ts +++ b/packages/acp/tests/codec.spec.ts @@ -39,27 +39,29 @@ describe('harnessBlockToAcpContent', () => { }) describe('acpPromptToText', () => { - it('concatenates text blocks and ignores non-text', () => { + it('concatenates text blocks and renders resource links explicitly', () => { const prompt: AcpContentBlock[] = [ { type: 'text', text: 'hello ' }, { type: 'resource_link', uri: 'file:///x', name: 'x' }, { type: 'text', text: 'world' }, ] - expect(acpPromptToText(prompt)).toBe('hello world') + expect(acpPromptToText(prompt)).toBe('hello \n[resource_link name="x" uri="file:///x"]\nworld') }) it('returns empty string for a prompt with no text blocks', () => { - expect(acpPromptToText([{ type: 'resource_link', uri: 'file:///x', name: 'x' }])).toBe('') + expect(acpPromptToText([{ type: 'image', mimeType: 'image/png', data: 'AA==' }])).toBe('') }) }) describe('promptHasUnsupportedContent', () => { - it('detects image and audio blocks', () => { + it('detects image, audio, and embedded resource blocks', () => { expect(promptHasUnsupportedContent([{ type: 'image', mimeType: 'image/png', data: 'AA==' }])).toBe(true) expect(promptHasUnsupportedContent([{ type: 'audio', mimeType: 'audio/wav', data: 'AA==' }])).toBe(true) + expect(promptHasUnsupportedContent([{ type: 'resource', resource: { uri: 'file:///x', text: 'x' } }])).toBe(true) }) - it('passes a text-only prompt', () => { + it('passes baseline text and resource_link prompt blocks', () => { expect(promptHasUnsupportedContent([{ type: 'text', text: 'hi' }])).toBe(false) + expect(promptHasUnsupportedContent([{ type: 'resource_link', uri: 'file:///x', name: 'x' }])).toBe(false) }) }) diff --git a/packages/acp/tests/edges.spec.ts b/packages/acp/tests/edges.spec.ts index 2a48ae6d8d..9484368322 100644 --- a/packages/acp/tests/edges.spec.ts +++ b/packages/acp/tests/edges.spec.ts @@ -52,4 +52,13 @@ describe('acp bridge — demux & config edges', () => { const a = await harness.client.newSession({ cwd: process.cwd(), mcpServers: [], additionalDirectories: [] }) expect(a.sessionId).toBeTruthy() }) + + it('rejects non-empty mcpServers until MCP wiring is implemented', async () => { + harness = await makeBridgeHarness({ storageDir }) + await harness.client.initialize({ protocolVersion: PROTOCOL_VERSION, clientCapabilities: {} }) + await expect(harness.client.newSession({ + cwd: process.cwd(), + mcpServers: [{ name: 'fs', command: 'npx', args: ['server'], env: [] }], + })).rejects.toThrow(/mcpServers/) + }) }) diff --git a/packages/acp/tests/load.spec.ts b/packages/acp/tests/load.spec.ts index 3bed1f836d..8e3748241a 100644 --- a/packages/acp/tests/load.spec.ts +++ b/packages/acp/tests/load.spec.ts @@ -85,7 +85,7 @@ describe('acp bridge — session/load replay', () => { expect(loader.ctx.agents.get(sessionId)).toBeUndefined() }) - it('loads a session whose persisted cwd differs from the launch dir (honors per-session cwd)', async () => { + it('rejects load when the requested cwd does not match the persisted session cwd', async () => { // Seed a session on disk whose header.cwd is a DIFFERENT absolute path than // the server's launch dir. The bridge must LOAD it (per-session cwd is // honored — the resumed session keeps header.cwd, and bash routes there), no @@ -101,10 +101,12 @@ describe('acp bridge — session/load replay', () => { ]) await loader.client.initialize({ protocolVersion: PROTOCOL_VERSION, clientCapabilities: {} }) - // Load succeeds even though the requested cwd is the launch dir, not otherCwd. - const res = await loader.client.loadSession({ sessionId: 'elsewhere', cwd: process.cwd(), mcpServers: [] }) + await expect(loader.client.loadSession({ sessionId: 'elsewhere', cwd: process.cwd(), mcpServers: [] })) + .rejects.toThrow(/cwd mismatch/) + expect(loader.ctx.agents.get('elsewhere')).toBeUndefined() + + const res = await loader.client.loadSession({ sessionId: 'elsewhere', cwd: otherCwd, mcpServers: [] }) expect(res).toBeDefined() - // The resumed session retains its ORIGINAL workspace cwd (so bash runs there). expect(loader.ctx.agents.get('elsewhere')!.session.header.cwd).toBe(otherCwd) }) diff --git a/packages/acp/tests/stream-update.spec.ts b/packages/acp/tests/stream-update.spec.ts index c37de8b344..cf877ea286 100644 --- a/packages/acp/tests/stream-update.spec.ts +++ b/packages/acp/tests/stream-update.spec.ts @@ -11,6 +11,12 @@ function updatesFor(event: SessionEvent): SessionNotification['update'][] { return out } +function liveUpdatesFor(event: SessionEvent): SessionNotification['update'][] { + const out: SessionNotification['update'][] = [] + streamSessionEventUpdate('s1', event, n => out.push(n.update), { includeUserMessages: false }) + return out +} + function evt(type: T, data: Extract['data']): SessionEvent { return { type, seq: 0, time: 0, data } as SessionEvent } @@ -92,6 +98,13 @@ describe('streamSessionEventUpdate', () => { expect(updatesFor(evt('user/message', { content: [], source: { kind: 'user' } }))).toEqual([]) }) + it('can suppress user/message chunks for live prompt turns', () => { + expect(liveUpdatesFor(evt('user/message', { + content: [{ type: 'text', text: 'hi' }], + source: { kind: 'user' }, + }))).toEqual([]) + }) + it('produces no update for boundary/other event types', () => { expect(updatesFor(evt('turn/start', { turn: 1, trigger: { kind: 'message', source: { kind: 'user' } } }))).toEqual([]) expect(updatesFor(evt('turn/end', { turn: 1, reason: { kind: 'completed' } }))).toEqual([])