diff --git a/apps/web/tests/plan-review.e2e.ts b/apps/web/tests/plan-review.e2e.ts index ea2a30d938..fd8217a2d8 100644 --- a/apps/web/tests/plan-review.e2e.ts +++ b/apps/web/tests/plan-review.e2e.ts @@ -7,12 +7,14 @@ // Replay is deterministic: the plan content arrives from replayed chunks, the // review wait is real, and the approve click is the test's own gesture (the // turn cannot complete without it, in record and replay alike). -import { readFile } from 'node:fs/promises' +import { mkdtemp, readFile, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' import { fileURLToPath } from 'node:url' import { join } from 'node:path' import type { Browser, ConsoleMessage, Page } from 'playwright' import { chromium } from 'playwright' import { afterAll, beforeAll, describe, expect, it, onTestFailed } from 'vitest' +import { deriveReplayScript, parseSessionLog } from '@deepseek-ai/dsh-llm-replay' import type { SessionEvent } from '@deepseek-ai/dsh-session' import type { SessionId } from '@deepseek-ai/dsh-session/types' import { @@ -390,8 +392,12 @@ describe.skipIf(MODE === 'record')('web e2e: pending plan review across Sidebar describe('web e2e: dismissed plan history', () => { it.skipIf(MODE === 'record')('reopens the permanent plan card after dismissing its review', async () => { - // Replay supplies a finite continuation; the real question rejection and retained document are the assertions. - const scaffold = await launchWebScaffold({ replayFixture: FIXTURE, compareReplaySession: false, paceMs: 15 }) + // A dismissed review concludes the turn, so only the call presenting the plan replays. + const replayRoot = await mkdtemp(join(tmpdir(), 'dsh-plan-review-dismiss-replay-')) + const replayOverride = join(replayRoot, 'replay.override.json') + const script = deriveReplayScript(parseSessionLog(await readFile(FIXTURE, 'utf8'))) + await writeFile(replayOverride, JSON.stringify(script.slice(0, 1))) + const scaffold = await launchWebScaffold({ replayFixture: FIXTURE, replayOverride, compareReplaySession: false, paceMs: 15 }) let browser: Browser | undefined const events: SessionEvent[] = [] scaffold.ctx.on('session/event', (_session, event: SessionEvent) => { events.push(event) }) @@ -417,9 +423,10 @@ describe('web e2e: dismissed plan history', () => { expect(call).toBeDefined() const results = events.filter(event => event.type === 'tool/result') const result = results.find(event => event.data.message.source.callId === call?.data.callId) - expect(result?.data.message).toMatchObject({ role: 'tool', toolCallId: call?.data.callId, isError: true }) + expect(result?.data.message).toMatchObject({ role: 'tool', toolCallId: call?.data.callId, isError: false }) expect(JSON.stringify(result)).toContain('dismissed the plan review') expect(results.some(event => JSON.stringify(event).includes('Plan approved'))).toBe(false) + expect(events.filter(event => event.type === 'assistant/message')).toHaveLength(1) const modes = events.filter(event => event.type === 'plan/mode') expect(modes).toHaveLength(1) expect(modes[0]).toMatchObject({ data: { active: true } }) @@ -443,6 +450,7 @@ describe('web e2e: dismissed plan history', () => { } finally { await browser?.close() await scaffold.close() + await rm(replayRoot, { recursive: true, force: true }) } }) }) diff --git a/packages/plan/plan-mode/README.i18n.yaml b/packages/plan/plan-mode/README.i18n.yaml index 534d4abb49..0551ab68f2 100644 --- a/packages/plan/plan-mode/README.i18n.yaml +++ b/packages/plan/plan-mode/README.i18n.yaml @@ -90,8 +90,8 @@ en: 9622e96bdc01276d zh: 53801e68a49f3d1d /deepseek-ai-dsh-plan-mode/model-experience/exit-tool-schema-and-review-exchange/what-the-model-sees: - en: 3768d0189f9f13af - zh: 2168a9b24551b7c6 + en: 16c57e8e64ad5a85 + zh: d81932553025c875 /deepseek-ai-dsh-plan-mode/model-experience/exit-tool-schema-and-review-exchange/token-effect: en: d1b1418f20ee119d zh: c2b89ea3cae57175 diff --git a/packages/plan/plan-mode/README.md b/packages/plan/plan-mode/README.md index 6cbcbf2632..466a5d2126 100644 --- a/packages/plan/plan-mode/README.md +++ b/packages/plan/plan-mode/README.md @@ -164,7 +164,7 @@ The user block is append-only conversation growth. Entering or leaving plan mode #### What the model sees -The [`exit_plan_mode` schema](../../../docs/tool-catalog.md#deepseek-aidsh-plan-mode) remains available in both states; execution outside plan mode fails, while an approved in-mode review returns the canonical `{ approved: true }` value and renders the existing confirmation text. Rejection remains a failed call carrying review feedback, and a dismissed review a failed call naming the user's takeover. +The [`exit_plan_mode` schema](../../../docs/tool-catalog.md#deepseek-aidsh-plan-mode) remains available in both states; execution outside plan mode fails, while an approved in-mode review returns the canonical `{ approved: true }` value and renders the existing confirmation text. Rejection remains a failed call carrying review feedback. A dismissed review returns `{ approved: false }`, renders a note that the user will reply, and concludes the turn without another model request. #### Token effect diff --git a/packages/plan/plan-mode/README.zh.md b/packages/plan/plan-mode/README.zh.md index 8ecb60b1db..0053aa53ee 100644 --- a/packages/plan/plan-mode/README.zh.md +++ b/packages/plan/plan-mode/README.zh.md @@ -164,7 +164,7 @@ You are in plan mode. Explore and design before presenting the complete plan thr #### 模型看到什么 -[`exit_plan_mode` schema](../../../docs/tool-catalog.zh.md#deepseek-aidsh-plan-mode) 在两种状态下均可用;在计划模式之外执行会失败,而计划模式内经批准的评审返回规范的 `{ approved: true }` 值,并渲染既有的确认文本。拒绝仍是携带评审反馈的失败调用,放弃评审则是一次指明用户接手的失败调用。 +[`exit_plan_mode` schema](../../../docs/tool-catalog.zh.md#deepseek-aidsh-plan-mode) 在两种状态下均可用;在计划模式之外执行会失败,而计划模式内经批准的评审返回规范的 `{ approved: true }` 值,并渲染既有的确认文本。拒绝仍是携带评审反馈的失败调用。放弃评审返回 `{ approved: false }`,渲染一条说明用户将自行回复的文本,并结束当前轮次,不再发起模型请求。 #### Token 影响 diff --git a/packages/plan/plan-mode/src/index.ts b/packages/plan/plan-mode/src/index.ts index e711607da8..a2385491ef 100644 --- a/packages/plan/plan-mode/src/index.ts +++ b/packages/plan/plan-mode/src/index.ts @@ -30,7 +30,7 @@ import { createUserMessage } from '@deepseek-ai/dsh-llm' import type { ContextFormed } from '@deepseek-ai/dsh-llm' import type { Session, UserMessage } from '@deepseek-ai/dsh-session' import { defineTool } from '@deepseek-ai/dsh-tools' -import { UserQuestionError } from '@deepseek-ai/dsh-user-questions' +import { UserQuestionError, type AskUserQuestionAnswer } from '@deepseek-ai/dsh-user-questions' import type { CommandDefinitionId, CommandId } from '@deepseek-ai/dsh-commands' import type {} from '@deepseek-ai/dsh-session-projection' import type { ProjectionDefinition } from '@deepseek-ai/dsh-session-projection' @@ -285,10 +285,15 @@ export class PlanModeController extends Service { type: 'object', additionalProperties: false, properties: { - approved: { type: 'boolean', const: true, required: true }, + approved: { type: 'boolean', required: true }, }, }, - render: () => [{ type: 'text', text: 'Plan approved — plan mode exited; carry out the plan starting with your next step.' }], + render: (_args, { approved }) => [{ + type: 'text', + text: approved + ? 'Plan approved — plan mode exited; carry out the plan starting with your next step.' + : 'The user dismissed the plan review to reply in their own words; plan mode remains active.', + }], }, execute: async (args, exec) => { const agent = exec.agent @@ -303,35 +308,37 @@ export class PlanModeController extends Service { if (interaction === undefined) { throw new Error('no user-questions channel is available to review the plan; ask the user to switch the session mode instead') } - const answer = await interaction.ask({ - questions: [{ - id: REVIEW_ID, - header: 'Plan review', - question: 'Approve this plan and leave plan mode?', - detail: args.plan, - options: [ - { label: APPROVE_LABEL, description: 'Leave plan mode; the plan is carried out from the next step.' }, - { label: KEEP_PLANNING_LABEL, description: 'Stay in plan mode; feedback goes back to the model.' }, - ], - // Presentation only: a capable UI renders the plan as a review - // decision instead of a generic question, and answers with one of - // the labels above either way. - intent: { kind: 'plan-review', approve: APPROVE_LABEL, callId: exec.callId }, - }], - agent, - signal: exec.signal, - }).catch((cause: unknown) => { + let answer: AskUserQuestionAnswer + try { + answer = await interaction.ask({ + questions: [{ + id: REVIEW_ID, + header: 'Plan review', + question: 'Approve this plan and leave plan mode?', + detail: args.plan, + options: [ + { label: APPROVE_LABEL, description: 'Leave plan mode; the plan is carried out from the next step.' }, + { label: KEEP_PLANNING_LABEL, description: 'Stay in plan mode; feedback goes back to the model.' }, + ], + // Presentation only: a capable UI renders the plan as a review + // decision instead of a generic question, and answers with one of + // the labels above either way. + intent: { kind: 'plan-review', approve: APPROVE_LABEL, callId: exec.callId }, + }], + agent, + signal: exec.signal, + }) + } catch (cause: unknown) { // A dismissed review is not a failed one: the user took the turn back - // to say something the two options do not cover. Say so, because the - // generic channel message names ask_user_question, which the model - // never called. An abort (turn cancel, provider teardown) keeps its - // own message — there is no user to wait for. + // to say something the two options do not cover, so the turn ends + // here instead of spending a model request on waiting. An abort (turn + // cancel, provider teardown) keeps its own failure. if (cause instanceof UserQuestionError && cause.code === 'ASK_CANCELLED') { - throw new Error('The user dismissed the plan review to speak instead; ' - + 'stay in plan mode, stop here, and wait for their message.') + exec.concludeTurn() + return { approved: false } } throw cause - }) + } // A review may outlive this plugin fiber. Without its pre-step listener, // an approved selection could never be appended, so fail and keep planning. if (disposed) { diff --git a/packages/plan/plan-mode/tests/integration.spec.ts b/packages/plan/plan-mode/tests/integration.spec.ts index b1742a1ba2..bdb02dfca5 100644 --- a/packages/plan/plan-mode/tests/integration.spec.ts +++ b/packages/plan/plan-mode/tests/integration.spec.ts @@ -8,6 +8,7 @@ import ToolRuntime, { defineContentToolFixture } from '@deepseek-ai/dsh-tools' import AgentRegistry, { type Agent } from '@deepseek-ai/dsh-agent' import AgentLoop from '@deepseek-ai/dsh-agent-loop' import PlanModeController from '@deepseek-ai/dsh-plan-mode' +import UserQuestionService, { UserQuestionError } from '@deepseek-ai/dsh-user-questions' import { MockAdapter, textResponse, toolCallResponse } from '../../../core/agent-loop/tests/mock-adapter.ts' const PLAN_CONFIG = { section: 'Test plan mode instructions.' } @@ -223,4 +224,28 @@ describe('plan mode through the agent loop', () => { { type: 'text', text: 'The user switched this session to plan mode.' }, ]) }) + + it('a dismissed plan review ends the turn without another model request', async () => { + const adapter = new MockAdapter([ + toolCallResponse('call-exit', 'exit_plan_mode', { plan: '# Plan\n\ndo things' }), + textResponse('Waiting for your message.'), + ]) + const ctx = await harness(adapter) + await ctx.plugin(UserQuestionService) + ctx.on('user-questions/request', () => Promise.reject(new UserQuestionError('the user cancelled ask_user_question', 'ASK_CANCELLED'))) + const agent = await ctx.agentLoop.create(SessionId('it-plan-dismiss'), { provider: 'mock', model: 'mock' }) + ctx.planMode.set(agent, true) + + const idle = waitForIdle(ctx, agent) + agent.followup(createUserMessage({ content: [{ type: 'text', text: 'plan it' }], source: { kind: 'user' } })) + await idle + + expect(adapter.requests).toHaveLength(1) + const log = agent.session.snapshotEvents() + const result = findEvent(log, 'tool/result') + expect(result.data.message.isError).toBe(false) + expect(result.data.message.content).toEqual([{ type: 'text', text: 'The user dismissed the plan review to reply in their own words; plan mode remains active.' }]) + expect(findEvent(log, 'turn/end').data.reason).toEqual({ kind: 'completed' }) + expect(planActive(ctx, agent)).toBe(true) + }) }) diff --git a/packages/plan/plan-mode/tests/plan-mode.spec.ts b/packages/plan/plan-mode/tests/plan-mode.spec.ts index bf97cc789c..6024269b6d 100644 --- a/packages/plan/plan-mode/tests/plan-mode.spec.ts +++ b/packages/plan/plan-mode/tests/plan-mode.spec.ts @@ -165,7 +165,7 @@ function expectPlanPtcSdkBindings(sdk: string): void { expect(sdk).toContain('read: Record;') expect(sdk).toContain('write: Record;') expect(sdk).toContain('interface ToolOutputMap {') - expect(sdk).toContain('exit_plan_mode: {\n approved: true;\n };') + expect(sdk).toContain('exit_plan_mode: {\n approved: boolean;\n };') expect(sdk).toContain('[K in ToolName]: (args: ToolArgsMap[K]) => Promise;') } @@ -1058,7 +1058,7 @@ describe('exit_plan_mode', () => { expect(question?.options?.map(option => option.label)).toContain(question?.intent?.approve) }) - it('reads a dismissed review as the user taking the turn back, not as a failure', async () => { + it('reads a dismissed review as the user taking the turn back: a turn-concluding success, not a failure', async () => { const { ctx, agent } = await setupWithReview() registerQuestionAnswerer(ctx, { ask: () => Promise.reject(Object.assign( @@ -1067,8 +1067,9 @@ describe('exit_plan_mode', () => { )), }) const result = await callExit(ctx, agent) - expect(result.isError).toBe(true) - expect(result.content).toEqual([{ type: 'text', text: 'Error: The user dismissed the plan review to speak instead; stay in plan mode, stop here, and wait for their message.' }]) + expect(result.isError).toBe(false) + expect(result.concludesTurn).toBe(true) + expect(result.content).toEqual([{ type: 'text', text: 'The user dismissed the plan review to reply in their own words; plan mode remains active.' }]) expect(foldPlanMode(agent.session.snapshotEvents())).toBe(true) }) diff --git a/snapshots/sdk/tool-error-details/system-prompt.expected.md b/snapshots/sdk/tool-error-details/system-prompt.expected.md index 9070405e5b..6c90496ba2 100644 --- a/snapshots/sdk/tool-error-details/system-prompt.expected.md +++ b/snapshots/sdk/tool-error-details/system-prompt.expected.md @@ -360,7 +360,7 @@ interface ToolOutputMap { after: string; }; exit_plan_mode: { - approved: true; + approved: boolean; }; get_goal: { goal: null; diff --git a/snapshots/session/advanced-toolchain/system-prompt.expected.md b/snapshots/session/advanced-toolchain/system-prompt.expected.md index cf4dcd9c3f..3684573a5f 100644 --- a/snapshots/session/advanced-toolchain/system-prompt.expected.md +++ b/snapshots/session/advanced-toolchain/system-prompt.expected.md @@ -375,7 +375,7 @@ interface ToolOutputMap { after: string; }; exit_plan_mode: { - approved: true; + approved: boolean; }; get_goal: { goal: null; diff --git a/snapshots/session/both-mode-turn/system-prompt.expected.md b/snapshots/session/both-mode-turn/system-prompt.expected.md index 329e451ce8..ac312ca2a6 100644 --- a/snapshots/session/both-mode-turn/system-prompt.expected.md +++ b/snapshots/session/both-mode-turn/system-prompt.expected.md @@ -327,7 +327,7 @@ interface ToolOutputMap { after: string; }; exit_plan_mode: { - approved: true; + approved: boolean; }; get_goal: { goal: null; diff --git a/snapshots/session/mcp-resources-ptc/system-prompt.expected.md b/snapshots/session/mcp-resources-ptc/system-prompt.expected.md index a6b0b07911..c5f57abc8c 100644 --- a/snapshots/session/mcp-resources-ptc/system-prompt.expected.md +++ b/snapshots/session/mcp-resources-ptc/system-prompt.expected.md @@ -356,7 +356,7 @@ interface ToolOutputMap { after: string; }; exit_plan_mode: { - approved: true; + approved: boolean; }; get_goal: { goal: null; diff --git a/snapshots/session/ptc-node-read-only/system-prompt.expected.md b/snapshots/session/ptc-node-read-only/system-prompt.expected.md index 39529ce115..23698d455d 100644 --- a/snapshots/session/ptc-node-read-only/system-prompt.expected.md +++ b/snapshots/session/ptc-node-read-only/system-prompt.expected.md @@ -331,7 +331,7 @@ interface ToolOutputMap { after: string; }; exit_plan_mode: { - approved: true; + approved: boolean; }; get_goal: { goal: null; diff --git a/snapshots/session/ptc-node-workspace/system-prompt.expected.md b/snapshots/session/ptc-node-workspace/system-prompt.expected.md index 39529ce115..23698d455d 100644 --- a/snapshots/session/ptc-node-workspace/system-prompt.expected.md +++ b/snapshots/session/ptc-node-workspace/system-prompt.expected.md @@ -331,7 +331,7 @@ interface ToolOutputMap { after: string; }; exit_plan_mode: { - approved: true; + approved: boolean; }; get_goal: { goal: null; diff --git a/snapshots/session/ptc-python-turn/system-prompt.expected.md b/snapshots/session/ptc-python-turn/system-prompt.expected.md index 23dc2e1494..e0444f7b83 100644 --- a/snapshots/session/ptc-python-turn/system-prompt.expected.md +++ b/snapshots/session/ptc-python-turn/system-prompt.expected.md @@ -158,7 +158,7 @@ class ExitPlanModeArgs(TypedDict): # Additional keys beyond those declared are allowed. class ExitPlanModeOutput(TypedDict): - approved: Literal[True] + approved: bool class GetGoalOutput1(TypedDict): goal: None diff --git a/snapshots/session/ptc-read-image/system-prompt.expected.md b/snapshots/session/ptc-read-image/system-prompt.expected.md index f0a3c2426f..809cc3304e 100644 --- a/snapshots/session/ptc-read-image/system-prompt.expected.md +++ b/snapshots/session/ptc-read-image/system-prompt.expected.md @@ -327,7 +327,7 @@ interface ToolOutputMap { after: string; }; exit_plan_mode: { - approved: true; + approved: boolean; }; get_goal: { goal: null; diff --git a/snapshots/session/ptc-turn/system-prompt.expected.md b/snapshots/session/ptc-turn/system-prompt.expected.md index 39529ce115..23698d455d 100644 --- a/snapshots/session/ptc-turn/system-prompt.expected.md +++ b/snapshots/session/ptc-turn/system-prompt.expected.md @@ -331,7 +331,7 @@ interface ToolOutputMap { after: string; }; exit_plan_mode: { - approved: true; + approved: boolean; }; get_goal: { goal: null; diff --git a/snapshots/web/official-subagent-bundle/system-prompt.expected.md b/snapshots/web/official-subagent-bundle/system-prompt.expected.md index f55fe02e4b..4a49609765 100644 --- a/snapshots/web/official-subagent-bundle/system-prompt.expected.md +++ b/snapshots/web/official-subagent-bundle/system-prompt.expected.md @@ -429,7 +429,7 @@ interface ToolOutputMap { after: string; }; exit_plan_mode: { - approved: true; + approved: boolean; }; get_goal: { goal: null; diff --git a/snapshots/web/optional-string-editor/system-prompt.expected.md b/snapshots/web/optional-string-editor/system-prompt.expected.md index a903888013..d5b908f02b 100644 --- a/snapshots/web/optional-string-editor/system-prompt.expected.md +++ b/snapshots/web/optional-string-editor/system-prompt.expected.md @@ -437,7 +437,7 @@ interface ToolOutputMap { after: string; }; exit_plan_mode: { - approved: true; + approved: boolean; }; get_goal: { goal: null; diff --git a/snapshots/web/ptc-round/system-prompt.expected.md b/snapshots/web/ptc-round/system-prompt.expected.md index 47cc71cb02..a3250ff7eb 100644 --- a/snapshots/web/ptc-round/system-prompt.expected.md +++ b/snapshots/web/ptc-round/system-prompt.expected.md @@ -420,7 +420,7 @@ interface ToolOutputMap { after: string; }; exit_plan_mode: { - approved: true; + approved: boolean; }; get_goal: { goal: null;