fix(acp): align prompt and workspace contracts

This commit is contained in:
Tianyi Cui
2026-06-17 21:26:31 +08:00
parent 2b36620e55
commit b920239389
12 changed files with 123 additions and 75 deletions

View File

@@ -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-…" }
}
}
}

View File

@@ -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')
}

View File

@@ -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) {

View File

@@ -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 () => {

View File

@@ -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)
})
})

View File

@@ -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/)
})
})

View File

@@ -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)
})

View File

@@ -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<T extends SessionEvent['type']>(type: T, data: Extract<SessionEvent, { type: T }>['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([])