fix review findings: keep ask-user opt-in for acp app

This commit is contained in:
Yichen Jiang
2026-07-09 00:40:13 +08:00
parent 7a1ef4b231
commit 56091d5b5d
9 changed files with 9 additions and 17 deletions

View File

@@ -56,7 +56,7 @@ export interface Config {
}
```
Source: [`packages/ui/acp-agent/src/index.ts:51`](../packages/ui/acp-agent/src/index.ts)
Source: [`packages/ui/acp-agent/src/index.ts:50`](../packages/ui/acp-agent/src/index.ts)
## `@deepseek-ai/dsh-agent-core`

View File

@@ -213,7 +213,6 @@ flowchart TD
pkg_acp_agent --> pkg_agent_core
pkg_acp_agent --> pkg_app_boot
pkg_acp_agent --> pkg_session_persistence_jsonl
pkg_acp_agent --> pkg_tool_ask_user
pkg_acp_agent --> pkg_user_interaction
pkg_stdio_agent --> pkg_agent
pkg_stdio_agent --> pkg_agent_core
@@ -275,5 +274,5 @@ flowchart TD
| [`subagent-mock`](../packages/support/subagent-mock) | `support` | [`agent`](../packages/core/agent), [`llm`](../packages/llm/llm), [`subagent`](../packages/subagent/subagent) |
| [`subagent-fork`](../packages/subagent/subagent-fork) | `subagent` | [`agent`](../packages/core/agent), [`session`](../packages/core/session), [`subagent`](../packages/subagent/subagent), [`subagent-inprocess`](../packages/subagent/subagent-inprocess) |
| [`subagent-spawn`](../packages/subagent/subagent-spawn) | `subagent` | [`subagent`](../packages/subagent/subagent), [`subagent-inprocess`](../packages/subagent/subagent-inprocess) |
| [`acp-agent`](../packages/ui/acp-agent) | `ui` | [`acp`](../packages/ui/acp), [`agent-core`](../packages/core/agent-core), [`app-boot`](../packages/ui/app-boot), [`session-persistence-jsonl`](../packages/session-persistence/session-persistence-jsonl), [`tool-ask-user`](../packages/ui/tool-ask-user), [`user-interaction`](../packages/core/user-interaction) |
| [`acp-agent`](../packages/ui/acp-agent) | `ui` | [`acp`](../packages/ui/acp), [`agent-core`](../packages/core/agent-core), [`app-boot`](../packages/ui/app-boot), [`session-persistence-jsonl`](../packages/session-persistence/session-persistence-jsonl), [`user-interaction`](../packages/core/user-interaction) |
| [`stdio-agent`](../packages/ui/stdio-agent) | `ui` | [`agent`](../packages/core/agent), [`agent-core`](../packages/core/agent-core), [`app-boot`](../packages/ui/app-boot), [`llm`](../packages/llm/llm), [`session`](../packages/core/session), [`session-persistence-jsonl`](../packages/session-persistence/session-persistence-jsonl), [`tool-ask-user`](../packages/ui/tool-ask-user), [`user-interaction`](../packages/core/user-interaction) |

View File

@@ -42,7 +42,7 @@ ACP elicitation is currently marked unstable in the SDK. The fallback is still s
The feature gives the model a powerful pause primitive, so prompt guidance matters. The tool description tells the model to ask concise questions and use options when possible. Product policy can later wrap `tools/execute` to restrict when the tool is allowed, but the loop should not special-case it.
`dsh-tool-ask-user` lives in `packages/ui` even though it is a tool, because it is a product-facing human-interaction affordance rather than providerless loop infrastructure. The core package remains only the abstract seam; `agent-core` does not load the tool. Front-door app packages such as `stdio-agent` and `acp-agent` opt into it alongside their UI provider.
`dsh-tool-ask-user` lives in `packages/ui` even though it is a tool, because it is a product-facing human-interaction affordance rather than providerless loop infrastructure. The core package remains only the abstract seam; `agent-core` does not load the tool. `stdio-agent` opts into it alongside its readline provider. `acp-agent` keeps only the `userInteraction` seam/provider by default: ACP elicitation support is still client-dependent, so an ACP leaf must opt into the model-facing tool deliberately once its client can complete elicitation requests.
## Testing

File diff suppressed because one or more lines are too long

View File

@@ -11,10 +11,10 @@ stdout is the ACP JSON-RPC channel, so the cluster is defined as much by what it
| Plugin | Why |
|---|---|
| `@deepseek-ai/dsh-agent-core` | the spine, pre-creating **no** agents (ACP `session/new` creates them on demand) |
| `@deepseek-ai/dsh-user-interaction` | the human question/answer seam used by confirmation tools |
| `@deepseek-ai/dsh-tool-ask-user` | the model-facing `ask_user_question` tool |
| `@deepseek-ai/dsh-user-interaction` | the human question/answer seam used by clients that can complete ACP elicitation requests |
| `@deepseek-ai/dsh-session-persistence-jsonl` | durable JSONL session log (the bridge advertises `loadSession`) |
| `@deepseek-ai/dsh-acp` | the bridge that owns stdout for JSON-RPC and provides ACP-backed user answers |
| `@deepseek-ai/dsh-acp` | the bridge that owns stdout for JSON-RPC and provides ACP-backed user answers when a leaf explicitly exposes a user-question tool |
| ~~`@deepseek-ai/dsh-tool-ask-user`~~ | **omitted by default** — ACP elicitation support is still client-dependent, so leaves must opt in deliberately |
| ~~console logger~~ | **omitted** — it writes to stdout and would corrupt the protocol frames ([the stdout-purity footgun](../acp/README.md)) |
| ~~`hmr`~~ | **omitted** — the editor owns the subprocess |

View File

@@ -36,7 +36,6 @@
"@deepseek-ai/dsh-acp": "^0.0.1",
"@deepseek-ai/dsh-agent-core": "^0.0.1",
"@deepseek-ai/dsh-session-persistence-jsonl": "^0.0.1",
"@deepseek-ai/dsh-tool-ask-user": "^0.0.1",
"@deepseek-ai/dsh-user-interaction": "^0.0.1",
"cordis": "^4.0.0-rc.6",
"schemastery": "^3.17.0"
@@ -49,7 +48,6 @@
"@deepseek-ai/dsh-agent-core": "workspace:^",
"@deepseek-ai/dsh-system-prompt": "workspace:^",
"@deepseek-ai/dsh-session-persistence-jsonl": "workspace:^",
"@deepseek-ai/dsh-tool-ask-user": "workspace:^",
"@deepseek-ai/dsh-user-interaction": "workspace:^",
"cordis": "^4.0.0-rc.6",
"schemastery": "^3.17.0"

View File

@@ -36,7 +36,6 @@ import * as acp from '@deepseek-ai/dsh-acp'
import * as agentCore from '@deepseek-ai/dsh-agent-core'
import SessionPersistenceJsonl from '@deepseek-ai/dsh-session-persistence-jsonl'
import UserInteractionService from '@deepseek-ai/dsh-user-interaction'
import * as toolAskUser from '@deepseek-ai/dsh-tool-ask-user'
export const name = 'acp-agent'
@@ -82,7 +81,6 @@ export function apply(ctx: Context, config: Config): void {
...config.toolOrder !== undefined ? { toolOrder: config.toolOrder } : {},
})
ctx.plugin(UserInteractionService)
ctx.plugin(toolAskUser)
ctx.plugin(SessionPersistenceJsonl, { root: config.persistenceRoot ?? './.sessions' })
ctx.plugin(acp, { model: config.model })
}

View File

@@ -31,7 +31,7 @@ describe('dsh-acp-agent composition', () => {
expect(ctx.get('sessionPersistence')).toBeDefined()
expect(ctx.get('agentLoop')).toBeDefined()
expect(ctx.get('userInteraction')).toBeDefined()
expect(ctx.get('tools')?.get('ask_user_question')).toBeDefined()
expect(ctx.get('tools')?.get('ask_user_question')).toBeUndefined()
// No pre-created agents — ACP session/new creates them on demand.
expect(ctx.get('agents')!.list()).toHaveLength(0)
await ctx.fiber.dispose()
@@ -72,7 +72,7 @@ describe('dsh-acp-agent composition', () => {
})
}
const assembly = await ctx.get('systemPrompt')!.assemble()
expect(assembly.tools.map(tool => tool.name)).toEqual(['zulu', 'alpha', 'ask_user_question'])
expect(assembly.tools.map(tool => tool.name)).toEqual(['zulu', 'alpha'])
await ctx.fiber.dispose()
})

3
pnpm-lock.yaml generated
View File

@@ -970,9 +970,6 @@ importers:
'@deepseek-ai/dsh-system-prompt':
specifier: workspace:^
version: link:../../core/system-prompt
'@deepseek-ai/dsh-tool-ask-user':
specifier: workspace:^
version: link:../tool-ask-user
'@deepseek-ai/dsh-user-interaction':
specifier: workspace:^
version: link:../../core/user-interaction