From 331e959c9782a25c16a5e13b250ba7783585cad8 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Fri, 24 Jul 2026 01:51:20 +0800 Subject: [PATCH] fix(acp): tighten automation-only contracts --- .../2026-07-23-acp-automation-only-protocol.i18n.yaml | 4 ++-- .../2026-07-23-acp-automation-only-protocol.md | 8 ++------ .../2026-07-23-acp-automation-only-protocol.zh.md | 8 ++------ examples/acp-agent/README.md | 2 +- packages/acp/acp/src/index.ts | 2 +- packages/acp/acp/tests/bridge.spec.ts | 8 ++++++++ packages/acp/acp/tests/codec.spec.ts | 8 -------- packages/support/acp-snapshot/README.md | 2 +- 8 files changed, 17 insertions(+), 25 deletions(-) diff --git a/.agents/notes/implemented/simplification/2026-07-23-acp-automation-only-protocol.i18n.yaml b/.agents/notes/implemented/simplification/2026-07-23-acp-automation-only-protocol.i18n.yaml index f1e78b0d82..6d477ec53a 100644 --- a/.agents/notes/implemented/simplification/2026-07-23-acp-automation-only-protocol.i18n.yaml +++ b/.agents/notes/implemented/simplification/2026-07-23-acp-automation-only-protocol.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write -2026-07-23-acp-automation-only-protocol.md: e9c98e8c5eb75396c895eb61a33a66d48f848436 -2026-07-23-acp-automation-only-protocol.zh.md: 30922fd60fa56e74c1d5c57c9b6e02c189d24bef +2026-07-23-acp-automation-only-protocol.md: 5faeae49d8730c12995c80057437350af68cf962 +2026-07-23-acp-automation-only-protocol.zh.md: e464ca023a56eb5cb5b997c12a14cafb1f93ac23 diff --git a/.agents/notes/implemented/simplification/2026-07-23-acp-automation-only-protocol.md b/.agents/notes/implemented/simplification/2026-07-23-acp-automation-only-protocol.md index e9c98e8c5e..5faeae49d8 100644 --- a/.agents/notes/implemented/simplification/2026-07-23-acp-automation-only-protocol.md +++ b/.agents/notes/implemented/simplification/2026-07-23-acp-automation-only-protocol.md @@ -26,11 +26,7 @@ Disconnect and plugin disposal share one memoized quiescence boundary. Both succ ## Snapshot boundary -The ACP snapshot suite retains the backend-oriented scenarios and still boots the assembled ACP example. The refactor keeps 53 scenarios covering loop, tool, hook, compaction, subagent, filesystem, PTY, Code Mode, permission escalation, and persistence behavior. Names that described deleted presentation are backend-oriented (`bash-tool-turn` and `todo-write`). - -Seven scenarios are removed because their scripts exercised deleted ACP UI controls: configuration advertisement, mode advertisement, model selection, permission-preset selection, command status, and plan-mode review through the picker and elicitation flow. Their owning packages retain focused keyless coverage. The semantic-checkpoint scenario uses the headless `stream-json` example instead of ACP. - -A FIXME in [`examples/acp-agent/tests/acp.snapshot.ts`](../../../../examples/acp-agent/tests/acp.snapshot.ts) records the deliberate follow-up: move the remaining backend corpus to the headless `stream-json` suite, leaving ACP snapshots responsible only for the automation protocol. That migration is separate because rewriting the shared snapshot harness and every fixture would obscure this protocol simplification. +The ACP snapshot suite still boots the assembled ACP example and retains scenarios that pin backend behavior. Only scenarios driven through deleted UI methods leave the suite; semantic-checkpoint recovery runs through the headless `stream-json` example because ACP no longer loads sessions. ## Alternatives considered @@ -48,4 +44,4 @@ ACP has a narrow contract suitable for agents and automation, while TUI and Web Automation clients receive complete committed text rather than token deltas or structured tool UI. They inspect durable logs or another API when they need reasoning, tool traces, titles, or richer state. Fresh-session-only operation also means callers that need durable browsing or resume use a host API rather than ACP. -The backend snapshot coverage remains available during the transition, but its transport is temporarily incidental. The FIXME makes that debt explicit without expanding this PR into a repository-wide snapshot migration. +Backend snapshot coverage therefore remains transport-coupled to ACP even though that transport is incidental to the behavior under test. diff --git a/.agents/notes/implemented/simplification/2026-07-23-acp-automation-only-protocol.zh.md b/.agents/notes/implemented/simplification/2026-07-23-acp-automation-only-protocol.zh.md index 30922fd60f..e464ca023a 100644 --- a/.agents/notes/implemented/simplification/2026-07-23-acp-automation-only-protocol.zh.md +++ b/.agents/notes/implemented/simplification/2026-07-23-acp-automation-only-protocol.zh.md @@ -26,11 +26,7 @@ ACP 仍有一个有用的职责：另一个 agent（智能体）或自动化控 ## 快照边界 -ACP 快照套件保留面向后端的场景，并继续启动组装后的 ACP 示例。该重构保留 53 个场景，覆盖循环、工具、钩子、压缩（compaction）、subagent、文件系统、PTY、Code Mode、权限提升与持久化行为。原本描述已删除展示层的名称改为面向后端的名称（`bash-tool-turn` 和 `todo-write`）。 - -删除七个场景，因为其脚本覆盖的是已删除的 ACP UI 控件：配置通告、模式通告、模型选择、权限预设选择、命令状态，以及通过选择器与询问流程实现的 plan mode 评审。它们所属的包仍保留专门的无密钥覆盖。语义检查点场景改用 headless `stream-json` 示例，不再使用 ACP。 - -[`examples/acp-agent/tests/acp.snapshot.ts`](../../../../examples/acp-agent/tests/acp.snapshot.ts) 中的 FIXME 记录了明确的后续工作：将余下的后端测试集转移到 headless `stream-json` 套件，使 ACP 快照只负责自动化协议。该迁移独立实施，因为在本次变更中重写共享快照 harness 与每个 fixture（测试前置数据），会模糊本次协议精简的主线。 +ACP 快照套件仍会启动组装后的 ACP 示例，并保留用于锁定后端行为的场景。从该套件移出的只有通过已删除的 UI 方法驱动的场景；由于 ACP 不再加载会话，语义检查点恢复通过 headless `stream-json` 示例执行。 ## 考虑过的替代方案 @@ -48,4 +44,4 @@ ACP 具有适合 agent 与自动化的精简契约，而 TUI 和 Web 拥有面 自动化客户端收到完整的已提交文本，而不是 token 增量或结构化工具 UI。当它们需要推理、工具跟踪信息、标题或更丰富的状态时，需要查看持久日志或其他 API。只支持全新会话也意味着，需要浏览持久会话或恢复会话的调用方必须使用 host API，而不是 ACP。 -过渡期间仍可使用后端快照覆盖，但其传输方式暂时只是附带选择。FIXME 明确记录了这项技术债，又不会将本 PR（Pull Request）扩展为全仓库快照迁移。 +因此，后端快照覆盖仍与 ACP 传输层耦合，尽管对于受测行为而言，该传输层只是附带因素。 diff --git a/examples/acp-agent/README.md b/examples/acp-agent/README.md index 0e4b694d14..bd4ca4d6ff 100644 --- a/examples/acp-agent/README.md +++ b/examples/acp-agent/README.md @@ -25,7 +25,7 @@ Under `workspace-write`, a model retry requesting wider sandbox access triggers This example owns the ACP snapshot suite. It boots the real automation server, replays committed model streams through `dsh-llm-replay`, and compares both normalized protocol output and re-persisted session logs. Recording uses the real model; refresh reuses committed replay input. Overrides cover throw/hang behavior, and optional `workspace/` fixtures seed world-state checks. -Most scenarios pin backend behavior rather than ACP-specific behavior and remain in this suite until they can move without losing coverage. The FIXME in [`tests/acp.snapshot.ts`](tests/acp.snapshot.ts) names their migration to the headless `stream-json` suite. +Most scenarios pin backend behavior rather than ACP-specific behavior; the [automation-only ACP decision](../../.agents/notes/implemented/simplification/2026-07-23-acp-automation-only-protocol.md#snapshot-boundary) owns why that coverage remains transport-coupled. ## Protocol limitations diff --git a/packages/acp/acp/src/index.ts b/packages/acp/acp/src/index.ts index 51531cbdd4..59ed499b00 100644 --- a/packages/acp/acp/src/index.ts +++ b/packages/acp/acp/src/index.ts @@ -300,7 +300,7 @@ export function apply(ctx: Context, config: AcpConfig): void { * @param config - ACP provider/model configuration. * @returns the configured fields only. */ -export function agentOptions(config: AcpConfig): { provider?: string; model?: string } { +function agentOptions(config: AcpConfig): { provider?: string; model?: string } { return { ...config.provider !== undefined ? { provider: config.provider } : {}, ...config.model !== undefined ? { model: config.model } : {}, diff --git a/packages/acp/acp/tests/bridge.spec.ts b/packages/acp/acp/tests/bridge.spec.ts index c7d5a176d6..a462506038 100644 --- a/packages/acp/acp/tests/bridge.spec.ts +++ b/packages/acp/acp/tests/bridge.spec.ts @@ -54,6 +54,14 @@ describe('automation-only ACP bridge', () => { expect(harness.adapter.requests[0]?.messages.at(-1)?.content).toEqual([{ type: 'text', text: 'say hello' }]) }) + it('leaves absent agent targets for request listeners to supply', async () => { + harness = await makeBridgeHarness({ config: { provider: undefined, model: undefined } }) + await harness.client.initialize({ protocolVersion: PROTOCOL_VERSION, clientCapabilities: {} }) + const { sessionId } = await harness.client.newSession({ cwd: process.cwd(), mcpServers: [] }) + + expect(harness.ctx.agents.get(SessionId(sessionId))?.options).toEqual({}) + }) + it('concatenates text blocks without exposing protocol framing to the model', async () => { harness = await makeBridgeHarness({ script: [textResponse('done')] }) await harness.client.initialize({ protocolVersion: PROTOCOL_VERSION, clientCapabilities: {} }) diff --git a/packages/acp/acp/tests/codec.spec.ts b/packages/acp/acp/tests/codec.spec.ts index 2aee641c06..4f8eaecd56 100644 --- a/packages/acp/acp/tests/codec.spec.ts +++ b/packages/acp/acp/tests/codec.spec.ts @@ -1,7 +1,6 @@ import { describe, expect, it } from 'vitest' import type { TurnEndReason } from '@deepseek-ai/dsh-session' import { acpPromptToText, promptHasUnsupportedContent, turnEndToStopReason } from '../src/codec.ts' -import { agentOptions } from '../src/index.ts' describe('ACP automation codec', () => { it('maps every known turn outcome to a legal stop reason', () => { @@ -27,11 +26,4 @@ describe('ACP automation codec', () => { expect(promptHasUnsupportedContent([{ type: 'text', text: 'ok' }])).toBe(false) expect(promptHasUnsupportedContent([{ type: 'resource_link', name: 'x', uri: 'file:///x' }])).toBe(true) }) - - it('copies only configured agent target fields', () => { - expect(agentOptions({})).toEqual({}) - expect(agentOptions({ provider: 'deepseek' })).toEqual({ provider: 'deepseek' }) - expect(agentOptions({ model: 'model' })).toEqual({ model: 'model' }) - expect(agentOptions({ provider: 'deepseek', model: 'model' })).toEqual({ provider: 'deepseek', model: 'model' }) - }) }) diff --git a/packages/support/acp-snapshot/README.md b/packages/support/acp-snapshot/README.md index 1a19d1715e..080a2fdb02 100644 --- a/packages/support/acp-snapshot/README.md +++ b/packages/support/acp-snapshot/README.md @@ -67,4 +67,4 @@ None; this package neither assembles nor sends a provider request. - **Session harvest requires raw JSONL mode** — `runScenario` collects persisted `.jsonl` logs, so snapshot configs set `persistenceCompression: 'none'`; compressed JSONL and SQLite compositions have no snapshot-harvest path. - **Built mode requires current artifacts** — run `pnpm run build` before selecting `DSH_EXAMPLE_MODE=lib`; source mode remains the zero-build path. -- **Backend coverage still rides an ACP driver** — most `examples/acp-agent` scenarios assert assembled agent behavior through the persisted log rather than ACP-specific behavior. The suite carries an explicit FIXME to migrate those scenarios to headless `stream-json`; this package remains until that coverage moves without loss. +- **Backend coverage still rides an ACP driver** — see the [automation-only ACP decision](../../../.agents/notes/implemented/simplification/2026-07-23-acp-automation-only-protocol.md#snapshot-boundary) for why retained scenarios use this transport.