diff --git a/.agents/notes/implemented/feature/2026-07-30-plan-review-presentation-intent.i18n.yaml b/.agents/notes/implemented/feature/2026-07-30-plan-review-presentation-intent.i18n.yaml new file mode 100644 index 0000000000..3de0b88f5c --- /dev/null +++ b/.agents/notes/implemented/feature/2026-07-30-plan-review-presentation-intent.i18n.yaml @@ -0,0 +1,6 @@ +# Bilingual-pair consistency record (docs/i18n/README.md): the git blob hash of each +# 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 .agents/notes/implemented/feature/2026-07-30-plan-review-presentation-intent.md +2026-07-30-plan-review-presentation-intent.md: aa85156d87a35d049f3cfab465fc668a63781a73 +2026-07-30-plan-review-presentation-intent.zh.md: fffee3de6341d45b26b88e1cc67a3895e747afac diff --git a/.agents/notes/implemented/feature/2026-07-30-plan-review-presentation-intent.md b/.agents/notes/implemented/feature/2026-07-30-plan-review-presentation-intent.md new file mode 100644 index 0000000000..aa85156d87 --- /dev/null +++ b/.agents/notes/implemented/feature/2026-07-30-plan-review-presentation-intent.md @@ -0,0 +1,55 @@ +# Agent Note: Plan review as a decision, not a question + +Status: implemented + +English | [中文](2026-07-30-plan-review-presentation-intent.zh.md) + +## Problem + +`exit_plan_mode` presents a finished plan for review through `ctx.userInteraction.ask()`, the same seam `ask_user_question` uses. On the Web GUI that made a plan review render as the generic question flow of [the ask-question Web presentation](2026-07-29-ask-question-web-presentation.md): a `1 / 1` pager, the plan as a question's supporting detail, the two verdicts as numbered radio rows with descriptions, an "Other — enter a custom answer" row, and `Skip this question` / `Submit` in the footer. + +Every one of those affordances is wrong for the surface. Reviewing a plan is one decision over one document, and the quiz chrome told the user they were being examined rather than asked to approve work — reported as "让人很困惑以为在做题". The paging controls page a set of one. Skipping is not an outcome the tool accepts (it folds into keep-planning). Worst, the surface gave no hint that this was the plan gate at all, while the adjacent waiting-approval takeover already had exactly the right shape for a decision: a tinted strip naming what is being decided, the subject as the body, and a right-aligned action row. + +## Decision + +A question may declare a **presentation intent**, and the Web composer renders a declared intent as its own surface. `AskUserQuestionItem` gains `intent?: AskUserQuestionIntent`, a tagged shape whose one member is `{ kind: 'plan-review', approve: string }`; `plan-mode` sets it on the review question, naming `Approve` as the label that approves. + +An intent shapes presentation only. The answer protocol is untouched: a UI honouring the intent answers with the same option labels a generic UI would send, so `exit_plan_mode` reads one answer shape regardless of which surface collected it, and a UI that does not know a tag renders the generic flow with nothing lost but the layout. + +`approve` names the affirmative option instead of relying on option order, so no UI infers a verdict from a position. Because the types cannot tie that label to the question's own option list, `UserInteractionService.ask()` rejects an intent whose `approve` names none of its options (`BAD_INTENT`) — at the asker, before any UI can answer a choice that was never offered. On the wire the intent is a discriminated union, so an unrecognised tag is a rejected frame rather than a silently generic render. + +`ui-question` renders the intent as `PlanReviewPanel`, in the waiting-approval card language: the amber strip carries `Plan review`, the plan is the scrolling markdown body, and the decision row holds three actions — `Chat about it`, `Refuse`, `Approve`. The question text becomes the card's accessible name rather than a headline, because the buttons already say what the decision is. Approve and Refuse answer with the asker's own option labels and keep the asker's descriptions as tooltips; `Chat about it` cancels the request, which returns the composer so the user can simply say what they want. All copy is bilingual under the existing `question` namespace. + +Routing lives inside the single composer entry (`QuestionComposer` chooses the shape) rather than in a second chain registration, and `planReviewOf` claims a request only when the batch is one question that declares the intent, carries the plan as its `detail`, and offers the named approve label. Anything else stays a generic question — the client is downstream of a wire boundary, so a request it cannot render as a card must still be answerable. + +Dismissal became its own model-facing outcome. `ASK_CANCELLED` previously reached the model as "the user cancelled ask_user_question", naming a tool it never called; `exit_plan_mode` now reports that the user dismissed the review to speak instead and to stay in plan mode and wait. Every other ask failure — an abort from turn cancel or provider teardown, where no user is coming — keeps its own message. + +## Alternatives considered + +**Make plan review its own pending kind (`plan-review/requested`).** Rejected as the wrong size for a presentation problem. It buys an honest response shape (approve / decline / discuss instead of an answer batch) at the cost of a third `PendingKind`, new requested/resolved frames and schemas, an api-proxy registry and respond branch, client session and baseline-replay handling, and a new three-package capability seam for a decision the question protocol already expresses. Worth revisiting only if plan review grows outcomes the answer shape cannot carry. + +**Route the card on the question's `id` or `header` (`plan-review` / `Plan review`).** Rejected: string-sniffing a foreign package's copy across a wire boundary, which any wording change silently breaks. The intent is the declaration that makes the routing legible. + +**Order the options and let the card read position 0 as approve.** Rejected: a positional contract at a package seam, invisible in both the type and the wire frame, and unenforceable — a producer that reorders its options would invert a user's verdict. Naming the label costs one string. + +**Register a second composer-chain entry for the plan card.** Rejected: two entries would select over the same pending question carrier, making the surface depend on chain priority and on whether the plan package's client half is composed at all. One entry that picks its own shape cannot race itself, and the generic flow is the built-in fallback. + +**Put the panel in `ui-plan` beside the plan chip.** Rejected: the panel's whole behavior is the question carrier's answer encoding (`PendingQuestion`), which `ui-question` owns; the intent is a question-protocol field, not plan-mode's private channel. Rendering declared intents belongs to the package that owns question rendering, as tool render intents belong to the tool renderer. + +**Extract a shared takeover card with `ui-conversation`'s `ApprovalPanel`.** Not done: the two takeovers agree on tokens and geometry but not on content — this body is scrolling markdown, that one a headline plus a command line — and the shared shell would be two elements wide. They are kept in step by token, not by component. + +**Give `Chat about it` its own protocol outcome.** Rejected: dismissing a request is a verb the generic flow already has (the `×` that cancels the batch). Promoting it to a labelled button is presentation; inventing a fourth wire outcome for it is not. + +## Consequences + +The question protocol now carries a presentation axis. Adding a second intent is a tag on the union, a producer that sets it, a schema member, and a panel — no new frame, service, or answer shape. The cost is that the question seam knows presentation exists at all, and that `ui-question` knows the word "plan"; both are the price of one entry owning every question surface. + +The plan gate reads as a plan gate: the plan is the card's content, the verdict is two labelled buttons, and taking the turn back is a third. The generic flow is untouched for every other question, and its committed goldens did not move. + +A deployment whose client half predates this change still shows the quiz layout — correct, answerable, and merely unstyled — because the intent is additive and the fallback is the generic flow. + +## Testing + +`ui-question` tests pin the narrowing (single-question batch, intent present, plan as detail, named approve label offered, decline absent when only approve is offered) and the panel (strip, markdown plan, accessible name, absence of pager/radio/skip/custom, approve and decline answering with the asker's labels, dismissal cancelling, one-shot latch with re-arm and message on a rejected receipt, tooltips present and absent, both locales). `user-interaction` tests pin `BAD_INTENT` and intent pass-through; `plan-mode` tests pin the declared intent against its own option list and both failure messages; the apiproxy schema test pins wire acceptance and an unknown tag's rejection. + +The `plan-review` Web e2e lane records `/plan` entering plan mode for real, the model calling `exit_plan_mode`, the decision card taking the composer (asserting the generic flow did **not** claim the request), and the card's own Approve completing the turn — two keyless goldens, the waiting card and the approved transcript. diff --git a/.agents/notes/implemented/feature/2026-07-30-plan-review-presentation-intent.zh.md b/.agents/notes/implemented/feature/2026-07-30-plan-review-presentation-intent.zh.md new file mode 100644 index 0000000000..fffee3de63 --- /dev/null +++ b/.agents/notes/implemented/feature/2026-07-30-plan-review-presentation-intent.zh.md @@ -0,0 +1,55 @@ +# Agent Note：计划审阅是一次决定，不是一道题 + +Status: implemented + +[English](2026-07-30-plan-review-presentation-intent.md) | 中文 + +## 问题 + +`exit_plan_mode` 通过 `ctx.userInteraction.ask()` 把写好的计划交给用户审阅，而这正是 `ask_user_question` 使用的同一个 seam。在 Web GUI 上，这导致计划审阅渲染为[ask-question Web 呈现](2026-07-29-ask-question-web-presentation.md)里的通用问题流程：一个 `1 / 1` 分页器、计划作为问题的补充说明、两个裁决作为带描述的编号单选行、一行"其他，请填写自定义答案"，以及底部的 `跳过本题` / `提交`。 + +这些可交互元素对这个界面而言无一正确。审阅一份计划是对一份文档做一次决定，而做题式的界面告诉用户他正在被考试，而不是被请求批准一份工作 —— 实际反馈是"让人很困惑以为在做题"。分页控件在给只有一项的集合分页。跳过并不是该工具接受的结果（它会折叠成继续规划）。最糟的是，这个界面完全没有暗示这就是计划关口，而旁边的等待审批接管早就具备了一次决定该有的形状：一条带色条带说明正在决定什么、主体是决定的对象、右对齐的操作行。 + +## 决定 + +一个问题可以声明**呈现意图（presentation intent）**，Web 输入区把已声明的意图渲染为它自己的界面。`AskUserQuestionItem` 新增 `intent?: AskUserQuestionIntent`，一个带标签的形状，目前唯一成员是 `{ kind: 'plan-review', approve: string }`；`plan-mode` 在审阅问题上设置它，并指明 `Approve` 是表示批准的标签。 + +意图只塑造呈现。回答协议不变：遵循意图的 UI 回答的仍是通用 UI 会发送的那些选项标签，因此无论由哪个界面收集，`exit_plan_mode` 读到的都是同一种回答形状；而不认识某个标签的 UI 渲染通用流程，除布局之外一无所失。 + +`approve` 指名肯定选项，而不依赖选项顺序，因此没有任何 UI 会从位置推断裁决。由于类型无法把该标签绑定到问题自身的选项列表，`UserInteractionService.ask()` 会拒绝 `approve` 未命中任一选项的意图（`BAD_INTENT`）—— 拦在提问方一侧，早于任何 UI 回答一个从未被提供过的选择。在协议格式（wire format）上意图是可辨识联合，因此无法识别的标签是被拒绝的帧，而不是静默退回通用渲染。 + +`ui-question` 把该意图渲染为 `PlanReviewPanel`，沿用等待审批卡片的语言：琥珀色条带写着 `Plan review`，计划是可滚动的 markdown 主体，决定行放三个操作 —— `Chat about it`、`Refuse`、`Approve`。问题文本成为卡片的无障碍名称而非标题，因为按钮已经说明了这次决定是什么。Approve 与 Refuse 用提问方自己的选项标签回答，并把提问方的描述保留为 tooltip；`Chat about it` 取消该请求，从而让输入区归位，用户直接说他想说的话即可。所有文案在既有 `question` 命名空间下双语。 + +路由住在单一输入区条目内部（由 `QuestionComposer` 选择形状），而不是第二个链式注册；`planReviewOf` 仅在这一批只有一个问题、该问题声明了意图、以 `detail` 承载计划、并提供了被指名的批准标签时才接管请求。其他情形一律保持通用问题 —— 客户端位于协议边界的下游，它无法渲染成卡片的请求仍必须可回答。 + +放弃审阅成为面向模型的独立结果。`ASK_CANCELLED` 以前传到模型的是"the user cancelled ask_user_question"，指名了一个它从未调用的工具；现在 `exit_plan_mode` 报告用户放弃审阅是为了改用说话，并要求留在 plan mode 中等待。其余每一种 ask 失败 —— 轮次取消或提供方拆卸导致的中止，那里并没有用户会来 —— 保留它们自己的消息。 + +## 备选方案 + +**让计划审阅成为自己的待处理种类（`plan-review/requested`）。** 否决：对一个呈现问题来说尺寸不对。它换来的是诚实的响应形状（approve / decline / discuss 而非一批回答），代价是第三个 `PendingKind`、新的 requested/resolved 帧与 schema、一个 api-proxy 注册表与响应分支、客户端会话与基线重放处理，以及为一个问题协议已能表达的决定新增一个三包能力 seam。只有当计划审阅长出回答形状承载不了的结果时才值得重新考虑。 + +**按问题的 `id` 或 `header`（`plan-review` / `Plan review`）路由卡片。** 否决：这是跨协议边界嗅探另一个包的文案字符串，任何措辞改动都会静默破坏它。意图才是让路由可读的那个声明。 + +**约定选项顺序，让卡片把第 0 个位置读作批准。** 否决：这是包边界上的位置约定，在类型和协议帧里都看不见，也无法强制 —— 生产方一旦重排选项，就会颠倒用户的裁决。指名标签只花一个字符串。 + +**为计划卡片注册第二个输入区链条目。** 否决：两个条目会对同一个待回答问题载体做选择，使界面取决于链优先级、以及计划包的客户端半边是否被组合。一个自己挑形状的条目不会和自己抢，而通用流程正是内建的回退。 + +**把面板放在 `ui-plan` 里、紧挨计划状态标签。** 否决：面板的全部行为就是问题载体的回答编码（`PendingQuestion`），那是 `ui-question` 拥有的；意图是问题协议的字段，不是 plan-mode 的私有通道。渲染已声明的意图属于拥有问题渲染的那个包，正如工具渲染意图属于工具渲染方。 + +**与 `ui-conversation` 的 `ApprovalPanel` 抽出共享的接管卡片。** 未做：两个接管在 token 和几何上一致，但内容不一致 —— 这边的主体是可滚动 markdown，那边是一行标题加一行命令 —— 共享外壳只会剩两个元素宽。它们靠 token 保持一致，而不是靠组件。 + +**给 `Chat about it` 自己的协议结果。** 否决：放弃一个请求是通用流程已有的动词（取消整批的 `×`）。把它提升为带标签的按钮属于呈现；为它发明第四种协议结果不属于。 + +## 结果 + +问题协议从此带有一个呈现轴。新增第二个意图 = 联合上的一个标签、一个设置它的生产方、一个 schema 成员、一个面板 —— 不需要新的帧、服务或回答形状。代价是问题 seam 从此知道"呈现"这件事存在，且 `ui-question` 知道"plan"这个词；两者都是由单一条目拥有全部问题界面所要付的价钱。 + +计划关口读起来就像计划关口：计划是卡片的内容，裁决是两个带标签的按钮，把轮次拿回来是第三个。通用流程对其他每个问题都未受影响，其已提交的 golden 也没有变动。 + +客户端半边早于本次改动的部署仍然显示做题式布局 —— 正确、可回答、只是没有专门样式 —— 因为意图是增量的，而回退就是通用流程。 + +## 测试 + +`ui-question` 测试钉住收窄（单问题批、意图存在、计划作为 detail、被指名的批准标签确实被提供、只提供批准时 decline 缺席）与面板（条带、markdown 计划、无障碍名称、无分页/单选/跳过/自定义、批准与拒绝用提问方的标签回答、放弃触发取消、一次性闭锁在回执被拒时重新武装并给出消息、tooltip 有与无、两种语言）。`user-interaction` 测试钉住 `BAD_INTENT` 与意图透传；`plan-mode` 测试钉住已声明的意图与其自身选项列表的一致、以及两条失败消息；apiproxy schema 测试钉住协议接受与未知标签的拒绝。 + +`plan-review` Web e2e 通道录制了 `/plan` 真实进入 plan mode、模型调用 `exit_plan_mode`、决定卡片接管输入区（并断言通用流程**没有**接管该请求）、以及卡片自身的 Approve 完成该轮 —— 两份无密钥 golden：等待中的卡片与批准后的会话记录。 diff --git a/apps/web/tests/plan-review.e2e.ts b/apps/web/tests/plan-review.e2e.ts new file mode 100644 index 0000000000..28272777fb --- /dev/null +++ b/apps/web/tests/plan-review.e2e.ts @@ -0,0 +1,113 @@ +// Web e2e scenario: the plan-review takeover. The shipped composition mounts +// plan mode and its client seat, so `/plan ` enters plan mode for real +// and the recorded turn ends on exit_plan_mode blocking against the live +// userInteraction seam. The composer is then occupied by the plan decision +// card — not the generic question flow — and approving it through the card +// completes the turn with the approval in the log. +// 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 { fileURLToPath } from 'node:url' +import { join } from 'node:path' +import type { Browser, Page } from 'playwright' +import { chromium } from 'playwright' +import { afterAll, beforeAll, describe, expect, it, onTestFailed } from 'vitest' +import type { SessionEvent } from '@deepseek-ai/dsh-session' +import { + assertFixtureInventory, captureStableAria, compareOrRefreshGolden, fixtureUserPrompts, + launchWebScaffold, recordFixture, watchConsole, webSnapshotMode, type WebScaffold, +} from './scaffold.ts' +import { connectFreshWorkspace, newEnglishPage, saveFailureShot } from './support.ts' + +const SNAPSHOT_DIR = fileURLToPath(new URL('./snapshots/plan-review', import.meta.url)) +const FIXTURE = join(SNAPSHOT_DIR, 'session.jsonl') +// The waiting golden owns the decision card; the approved golden owns the +// transcript the approval leaves behind — the state the card cannot see. +const REVIEW_EXPECTED = join(SNAPSHOT_DIR, 'review.expected.md') +const APPROVED_EXPECTED = join(SNAPSHOT_DIR, 'approved.expected.md') +const MODE = webSnapshotMode() + +// One command line: /plan enters plan mode and submits the rest as the turn's +// message. The task is deliberately self-contained (nothing to explore in a +// fresh workspace) so the recorded turn is a plan and its review, and the +// approved continuation is one word. +const TASK = 'Plan a small change: add a --greeting flag to a CLI. Do not read or write any files. ' + + 'Call exit_plan_mode with a short plan of at most five bullet points. ' + + 'Once the plan is approved, reply with the single word DONE and stop.' +const LINE = `/plan ${TASK}` + +describe('web e2e: plan review takeover round trip', () => { + let scaffold: WebScaffold + let browser: Browser + let page: Page + let tripwire: ReturnType + const sessionEvents: SessionEvent[] = [] + + beforeAll(async () => { + scaffold = await launchWebScaffold(MODE === 'record' ? {} : { replayFixture: FIXTURE, paceMs: 15 }) + scaffold.ctx.on('session/event', (_session, event: SessionEvent) => { sessionEvents.push(event) }) + browser = await chromium.launch() + // English page: the decision copy is the surface under test, and the + // golden pins one language. + page = await newEnglishPage(browser) + tripwire = watchConsole(page) + await page.goto(scaffold.baseUrl, { waitUntil: 'load' }) + await page.waitForSelector('[class*="frame"]', { timeout: 30_000 }) + await connectFreshWorkspace(page) + }, 120_000) + + afterAll(async () => { + await browser?.close() + await scaffold?.close() + }) + + it('reviews the plan on a decision card and approves through it', async () => { + onTestFailed(() => saveFailureShot(page, 'web-e2e-plan-review')) + if (MODE !== 'record') { + expect(fixtureUserPrompts(await readFile(FIXTURE, 'utf8'))).toEqual([TASK]) + } + const input = page.locator('textarea').first() + await input.waitFor({ timeout: 10_000 }) + const settled = scaffold.whenTurnSettled(MODE === 'record' ? 180_000 : 30_000) + await input.fill(LINE) + await input.press('Enter') + + // The card takes over the input area while exit_plan_mode blocks. Its + // presence is a STABLE waiting state (it stays until answered), so a plain + // waitFor is race-free. + const card = page.locator('[data-plan-review-key]') + await card.waitFor({ timeout: MODE === 'record' ? 120_000 : 30_000 }) + // The plan-review request must NOT land on the generic question flow. + expect(await page.locator('[data-question-key]').count()).toBe(0) + await expect.poll(() => card.getByText('Plan review').count(), { timeout: 10_000 }).toBeGreaterThan(0) + + if (MODE !== 'record') { + const snapshot = await captureStableAria(page, '[data-plan-review-key]', scaffold.workspaceCwd) + await compareOrRefreshGolden(REVIEW_EXPECTED, snapshot, MODE) + } + + await card.getByRole('button', { name: 'Approve' }).click() + + const sessionId = await settled + if (MODE === 'record') { + await recordFixture(scaffold, sessionId, FIXTURE) + return + } + // World state: the approval reached the tool, and plan mode is left behind. + const results = sessionEvents.filter(e => e.type === 'tool/result') + expect(JSON.stringify(results.at(-1))).toContain('Plan approved') + await expect.poll(() => page.getByText('DONE', { exact: true }).count(), { timeout: 15_000 }).toBeGreaterThanOrEqual(1) + // Card gone; regular input restored. + expect(await page.locator('[data-plan-review-key]').count()).toBe(0) + await expect.poll(() => page.locator('textarea').first().isEnabled(), { timeout: 10_000 }).toBe(true) + const snapshot = await captureStableAria(page, '[class*="centerCol"]', scaffold.workspaceCwd) + await compareOrRefreshGolden(APPROVED_EXPECTED, snapshot, MODE) + expect(tripwire.pageErrors).toEqual([]) + expect(tripwire.warnings).toEqual([]) + }, 200_000) + + it.skipIf(MODE === 'record')('keeps the fixture inventory closed', async () => { + await assertFixtureInventory(SNAPSHOT_DIR, ['session.jsonl', 'review.expected.md', 'approved.expected.md']) + }) +}) diff --git a/apps/web/tests/snapshots/plan-review/approved.expected.md b/apps/web/tests/snapshots/plan-review/approved.expected.md new file mode 100644 index 0000000000..aca0bc31bb --- /dev/null +++ b/apps/web/tests/snapshots/plan-review/approved.expected.md @@ -0,0 +1,51 @@ +- banner: + - navigation "Session hierarchy": + - 'button "Plan a small change: add" [disabled]' + - tablist: + - tab "Chat" [selected] + - tab "Trajectory" +- img +- text: "/plan Plan a small change: add a --greeting flag to a CLI. Do not read or write any files. Call exit_plan_mode with a short plan of at most five bullet points. Once the plan is approved, reply with the single word DONE and stop. Plan mode on. Use /plan off to leave. Plan a small change: add a --greeting flag to a CLI. Do not read or write any files. Call exit_plan_mode with a short plan of at most five bullet points. Once the plan is approved, reply with the single word DONE and stop. {{clock}}" +- button "复制": + - img +- button "在新对话中分支": + - img +- button "编辑": + - img +- 'button "Think The user wants me to plan a small change to add a `--greeting` flag to a CLI. They explicitly told me not to read or write any files, and to call exit_plan_mode with a short plan. Let me do that directly."': + - img + - img + - text: "Think The user wants me to plan a small change to add a `--greeting` flag to a CLI. They explicitly told me not to read or write any files, and to call exit_plan_mode with a short plan. Let me do that directly." +- paragraph: + - text: Since the user has explicitly asked me not to read or write any files and to go straight to planning, I'll proceed with + - code: exit_plan_mode + - text: . +- button "复制": + - img +- button "在新对话中分支": + - img +- text: {{clock}} +- button: + - img + - img +- text: "Tool call exit_plan_mode · # Add `--greeting` flag to CLI" +- 'button "Think The plan was approved. The user''s last instruction says: \"Once the plan is approved, reply with the single word DONE and stop.\" So I should just reply with DONE and stop."': + - img + - img + - text: "Think The plan was approved. The user's last instruction says: \"Once the plan is approved, reply with the single word DONE and stop.\" So I should just reply with DONE and stop." +- paragraph: DONE +- button "复制": + - img +- button "在新对话中分支": + - img +- text: {{clock}} +- textbox "Message the agent" +- button "Add attachment": + - img +- 'button "Access mode, current: Danger Full Access"': Danger Full Access +- button "Plan mode off, press to turn on": Plan off +- button "Select model, current DeepSeek-V4-Flash": + - text: DeepSeek-V4-Flash + - img +- button "Send message" [disabled] +- text: 1 turns · 2 steps Tool call {{duration}} Cache hit 51% Input 10.2K tok · Output 346 tok diff --git a/apps/web/tests/snapshots/plan-review/review.expected.md b/apps/web/tests/snapshots/plan-review/review.expected.md new file mode 100644 index 0000000000..81d3af911c --- /dev/null +++ b/apps/web/tests/snapshots/plan-review/review.expected.md @@ -0,0 +1,44 @@ +- region "Approve this plan and leave plan mode?": + - text: Plan review + - heading "Add --greeting flag to CLI" [level=1]: + - text: Add + - code: "--greeting" + - text: flag to CLI + - list: + - listitem: + - strong: Locate the CLI entry point + - text: (e.g., + - code: cli.py + - text: "," + - code: main.go + - text: "," + - code: index.js + - text: etc.) and find the argument parser definition (argparse, click, cobra, yargs, or similar). + - listitem: + - strong: Register a new optional string argument + - text: named + - code: "--greeting" + - text: with a short alias ( + - code: "-g" + - text: if available) and a sensible default value (e.g., + - code: "\"Hello\"" + - text: ). + - listitem: + - strong: Thread the parsed value + - text: through the main handler function so it is passed where the greeting string is used (e.g., the welcome/response message). + - listitem: + - strong: Update the help text + - text: so + - code: "--help" + - text: or + - code: "-h" + - text: shows the new flag with its description. + - listitem: + - strong: No tests or config changes + - text: unless they already exist and directly validate the flag's presence. + - status + - button "Chat about it": + - img + - text: Chat about it + - button "Refuse" + - button "Approve" diff --git a/apps/web/tests/snapshots/plan-review/session.jsonl b/apps/web/tests/snapshots/plan-review/session.jsonl new file mode 100644 index 0000000000..960c05393f --- /dev/null +++ b/apps/web/tests/snapshots/plan-review/session.jsonl @@ -0,0 +1,39 @@ +{"type":"session","version":0,"id":"{{sessionId}}","createdAt":1785406804293,"cwd":"{{cwd}}/workspace"} +{"type":"command/run","seq":0,"time":1785406804350,"data":{"commandId":"cmd-228a60ef-1","name":"plan","args":" Plan a small change: add a --greeting flag to a CLI. Do not read or write any files. Call exit_plan_mode with a short plan of at most five bullet points. Once the plan is approved, reply with the single word DONE and stop.","source":{"kind":"user"}}} +{"type":"plan/mode","seq":1,"time":1785406804350,"data":{"active":true}} +{"type":"command/done","seq":2,"time":1785406804352,"data":{"commandId":"cmd-228a60ef-1","kind":"success","text":"Plan mode on. Use /plan off to leave."}} +{"type":"turn/start","seq":3,"time":1785406804353,"data":{"turn":1,"trigger":{"kind":"message","source":{"kind":"user"}}}} +{"type":"user/message","seq":4,"time":1785406804353,"data":{"content":[{"type":"text","text":"Plan a small change: add a --greeting flag to a CLI. Do not read or write any files. Call exit_plan_mode with a short plan of at most five bullet points. Once the plan is approved, reply with the single word DONE and stop."}],"source":{"kind":"user"},"role":"user","id":"a22bd0d2-15dc-4bfb-b978-0d5961459e57"},"surfaceOp":"append"} +{"type":"session/title","seq":5,"time":1785406804354,"data":{"title":"Plan a small change: add","messageSeqs":[4],"source":{"kind":"fallback"}}} +{"type":"step/start","seq":6,"time":1785406804355,"data":{"turn":1,"step":1}} +{"type":"request/header","seq":7,"time":1785406804356,"data":{"header":{"config":{"provider":"deepseek","model":"deepseek-v4-flash","reasoningEffort":"high"},"system":"{{system}}","tools":"{{tools}}"},"reason":"initial"}} +{"type":"assistant/chunk","seq":8,"time":1785406805697,"data":{"turn":1,"step":1,"chunk":{"type":"block-start","index":0,"blockType":"reasoning"}}} +{"type":"reasoning-chunks","seq0":9,"time0":1785406805698,"data":{"turn":1,"step":1,"index":0,"dt":[151,31,0,0,0,0,18,1,0,0,0,31,1,0,0,0,1,23,0,0,0,19,0,0,37,1,0,0,0,0,12,1,0,0,28,0,1,0,24,1,0,0,0,0,26,1,0,0,26,1,22],"texts":["The"," user"," wants"," me"," to"," plan"," a"," small"," change"," to"," add"," a"," `","--","gre","eting","`"," flag"," to"," a"," CLI","."," They"," explicitly"," told"," me"," not"," to"," read"," or"," write"," any"," files",","," and"," to"," call"," exit","_","plan","_mode"," with"," a"," short"," plan","."," Let"," me"," do"," that"," directly","."]}} +{"type":"assistant/chunk","seq":61,"time":1785406806155,"data":{"turn":1,"step":1,"chunk":{"type":"block-start","index":1,"blockType":"text"}}} +{"type":"text-chunks","seq0":62,"time0":1785406806156,"data":{"turn":1,"step":1,"index":1,"dt":[0,20,0,34,23,26,0,1,0,0,28,0,0,0,32,0,15,1,0,30,1,0,0,23,1,0,24,0,0,0],"texts":["Since"," the"," user"," has"," explicitly"," asked"," me"," not"," to"," read"," or"," write"," any"," files"," and"," to"," go"," straight"," to"," planning",","," I","'ll"," proceed"," with"," `","exit","_","plan","_mode","`."]}} +{"type":"assistant/chunk","seq":93,"time":1785406806493,"data":{"turn":1,"step":1,"chunk":{"type":"block-start","index":2,"blockType":"tool-call"}}} +{"type":"tool-call-chunks","seq0":94,"time0":1785406806493,"data":{"turn":1,"step":1,"index":2,"dt":[27,1,0,0,0,24,0,0,29,1,0,0,0,1,20,0,27,1,0,21,28,0,0,31,1,23,0,0,1,23,0,0,0,23,1,0,28,20,26,28,1,0,29,1,0,0,21,28,0,0,25,1,26,1,0,0,24,1,0,26,0,0,0,1,23,1,0,0,22,29,0,31,1,1,0,21,1,0,0,0,0,20,0,32,25,0,0,22,0,0,27,0,1,0,31,0,1,0,0,20,1,0,0,0,0,24,0,1,0,26,1,29,1,26,1,0,25,30,1,18,0,30,0,0,30,21,1,28,0,21,1,0,0,23,1,23,36,0,0,1,0,13,1,28,27,0,0,25,1,45,1,11,0,21,0,0,0,0,23,0,0,32,1,0,0,0,16,28,0,27,1,0,22,31,1,24,1,21,32,1,0,24,1,0,24],"id":"call_00_TPh5fH8x5kRezK115kgx8155","name":"exit_plan_mode","args":["","{","\"","plan","\"",": ","\"","#"," Add"," `","--","gre","eting","`"," flag"," to"," CLI","\\n\\n","-"," **","Loc","ate"," the"," CLI"," entry"," point","**"," (","e",".g",".,"," `","cli",".py","`,"," `","main",".go","`,"," `","index",".js","`"," etc",".)"," and"," find"," the"," argument"," parser"," definition"," (","arg","parse",","," click",","," cob","ra",","," y","args",","," or"," similar",").\\n","-"," **","Register"," a"," new"," optional"," string"," argument","**"," named"," `","--","gre","eting","`"," with"," a"," short"," alias"," (`","-","g","`"," if"," available",")"," and"," a"," sensible"," default"," value"," (","e",".g",".,"," `","\\\"","Hello","\\\"","`",").\\n","-"," **","Thread"," the"," parsed"," value","**"," through"," the"," main"," handler"," function"," so"," it"," is"," passed"," where"," the"," greeting"," string"," is"," used"," (","e",".g",".,"," the"," welcome","/","response"," message",").\\n","-"," **","Update"," the"," help"," text","**"," so"," `","--","help","`"," or"," `","-","h","`"," shows"," the"," new"," flag"," with"," its"," description",".\\n","-"," **","No"," tests"," or"," config"," changes","**"," unless"," they"," already"," exist"," and"," directly"," validate"," the"," flag","'s"," presence",".","\"","}"]}} +{"type":"assistant/chunk","seq":280,"time":1785406808318,"data":{"turn":1,"step":1,"chunk":{"type":"block-end","index":0,"block":{"type":"reasoning","text":"The user wants me to plan a small change to add a `--greeting` flag to a CLI. They explicitly told me not to read or write any files, and to call exit_plan_mode with a short plan. Let me do that directly."}}}} +{"type":"assistant/chunk","seq":281,"time":1785406808318,"data":{"turn":1,"step":1,"chunk":{"type":"block-end","index":1,"block":{"type":"text","text":"Since the user has explicitly asked me not to read or write any files and to go straight to planning, I'll proceed with `exit_plan_mode`."}}}} +{"type":"assistant/chunk","seq":282,"time":1785406808318,"data":{"turn":1,"step":1,"chunk":{"type":"block-end","index":2,"block":{"type":"tool-call","id":"call_00_TPh5fH8x5kRezK115kgx8155","name":"exit_plan_mode","arguments":"{\"plan\": \"# Add `--greeting` flag to CLI\\n\\n- **Locate the CLI entry point** (e.g., `cli.py`, `main.go`, `index.js` etc.) and find the argument parser definition (argparse, click, cobra, yargs, or similar).\\n- **Register a new optional string argument** named `--greeting` with a short alias (`-g` if available) and a sensible default value (e.g., `\\\"Hello\\\"`).\\n- **Thread the parsed value** through the main handler function so it is passed where the greeting string is used (e.g., the welcome/response message).\\n- **Update the help text** so `--help` or `-h` shows the new flag with its description.\\n- **No tests or config changes** unless they already exist and directly validate the flag's presence.\"}"}}}} +{"type":"assistant/chunk","seq":283,"time":1785406808319,"data":{"turn":1,"step":1,"chunk":{"type":"usage","usage":{"inputTokens":4529,"outputTokens":303,"cacheReadTokens":640,"reasoningTokens":52}}}} +{"type":"assistant/chunk","seq":284,"time":1785406808319,"data":{"turn":1,"step":1,"chunk":{"type":"finish","reason":{"kind":"tool-calls"}}}} +{"type":"assistant/message","seq":285,"time":1785406808324,"data":{"turn":1,"step":1,"message":{"role":"assistant","content":[{"type":"reasoning","text":"The user wants me to plan a small change to add a `--greeting` flag to a CLI. They explicitly told me not to read or write any files, and to call exit_plan_mode with a short plan. Let me do that directly."},{"type":"text","text":"Since the user has explicitly asked me not to read or write any files and to go straight to planning, I'll proceed with `exit_plan_mode`."},{"type":"tool-call","id":"call_00_TPh5fH8x5kRezK115kgx8155","name":"exit_plan_mode","arguments":"{\"plan\": \"# Add `--greeting` flag to CLI\\n\\n- **Locate the CLI entry point** (e.g., `cli.py`, `main.go`, `index.js` etc.) and find the argument parser definition (argparse, click, cobra, yargs, or similar).\\n- **Register a new optional string argument** named `--greeting` with a short alias (`-g` if available) and a sensible default value (e.g., `\\\"Hello\\\"`).\\n- **Thread the parsed value** through the main handler function so it is passed where the greeting string is used (e.g., the welcome/response message).\\n- **Update the help text** so `--help` or `-h` shows the new flag with its description.\\n- **No tests or config changes** unless they already exist and directly validate the flag's presence.\"}"}],"source":{"kind":"model","provider":"deepseek","model":"deepseek-v4-flash"},"id":"b9e18116-68f6-4442-b5a5-a33277ef526c"},"usage":{"inputTokens":4529,"outputTokens":303,"cacheReadTokens":640,"reasoningTokens":52}},"sourceEventSeqs":[8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51,52,53,54,55,56,57,58,59,60,61,62,63,64,65,66,67,68,69,70,71,72,73,74,75,76,77,78,79,80,81,82,83,84,85,86,87,88,89,90,91,92,93,94,95,96,97,98,99,100,101,102,103,104,105,106,107,108,109,110,111,112,113,114,115,116,117,118,119,120,121,122,123,124,125,126,127,128,129,130,131,132,133,134,135,136,137,138,139,140,141,142,143,144,145,146,147,148,149,150,151,152,153,154,155,156,157,158,159,160,161,162,163,164,165,166,167,168,169,170,171,172,173,174,175,176,177,178,179,180,181,182,183,184,185,186,187,188,189,190,191,192,193,194,195,196,197,198,199,200,201,202,203,204,205,206,207,208,209,210,211,212,213,214,215,216,217,218,219,220,221,222,223,224,225,226,227,228,229,230,231,232,233,234,235,236,237,238,239,240,241,242,243,244,245,246,247,248,249,250,251,252,253,254,255,256,257,258,259,260,261,262,263,264,265,266,267,268,269,270,271,272,273,274,275,276,277,278,279,280,281,282,283,284],"surfaceOp":"append"} +{"type":"tool/call","seq":286,"time":1785406808325,"data":{"turn":1,"step":1,"callId":"call_00_TPh5fH8x5kRezK115kgx8155","name":"exit_plan_mode","arguments":"{\"plan\": \"# Add `--greeting` flag to CLI\\n\\n- **Locate the CLI entry point** (e.g., `cli.py`, `main.go`, `index.js` etc.) and find the argument parser definition (argparse, click, cobra, yargs, or similar).\\n- **Register a new optional string argument** named `--greeting` with a short alias (`-g` if available) and a sensible default value (e.g., `\\\"Hello\\\"`).\\n- **Thread the parsed value** through the main handler function so it is passed where the greeting string is used (e.g., the welcome/response message).\\n- **Update the help text** so `--help` or `-h` shows the new flag with its description.\\n- **No tests or config changes** unless they already exist and directly validate the flag's presence.\"}"}} +{"type":"tool/result","seq":287,"time":1785406808720,"data":{"turn":1,"step":1,"message":{"source":{"kind":"tool","callId":"call_00_TPh5fH8x5kRezK115kgx8155"},"content":[{"type":"tool-result","toolCallId":"call_00_TPh5fH8x5kRezK115kgx8155","content":[{"type":"text","text":"Plan approved — plan mode exited; carry out the plan starting with your next step."}],"isError":false}],"role":"user","id":"e7cf1f9f-e143-48df-b975-440a9d61a538"}},"sourceEventSeqs":[286],"surfaceOp":"append"} +{"type":"step/end","seq":288,"time":1785406808721,"data":{"turn":1,"step":1}} +{"type":"plan/mode","seq":289,"time":1785406808721,"data":{"active":false}} +{"type":"step/start","seq":290,"time":1785406808723,"data":{"turn":1,"step":2}} +{"type":"request/header","seq":291,"time":1785406808723,"data":{"header":{"config":{"provider":"deepseek","model":"deepseek-v4-flash","reasoningEffort":"high"},"system":"{{system}}","tools":"{{tools}}"},"reason":"change"}} +{"type":"assistant/chunk","seq":292,"time":1785406809855,"data":{"turn":1,"step":2,"chunk":{"type":"block-start","index":0,"blockType":"reasoning"}}} +{"type":"reasoning-chunks","seq0":293,"time0":1785406809855,"data":{"turn":1,"step":2,"index":0,"dt":[133,28,1,0,0,17,0,28,1,0,0,29,0,0,0,0,0,21,0,0,0,35,0,0,0,0,0,14,1,37,1,0,1,17,0,0,0,0,1],"texts":["The"," plan"," was"," approved","."," The"," user","'s"," last"," instruction"," says",":"," \"","Once"," the"," plan"," is"," approved",","," reply"," with"," the"," single"," word"," D","ONE"," and"," stop",".\""," So"," I"," should"," just"," reply"," with"," D","ONE"," and"," stop","."]}} +{"type":"assistant/chunk","seq":333,"time":1785406810252,"data":{"turn":1,"step":2,"chunk":{"type":"block-start","index":1,"blockType":"text"}}} +{"type":"assistant/chunk","seq":334,"time":1785406810253,"data":{"turn":1,"step":2,"chunk":{"type":"text-delta","index":1,"text":"D"}}} +{"type":"assistant/chunk","seq":335,"time":1785406810253,"data":{"turn":1,"step":2,"chunk":{"type":"text-delta","index":1,"text":"ONE"}}} +{"type":"assistant/chunk","seq":336,"time":1785406810256,"data":{"turn":1,"step":2,"chunk":{"type":"block-end","index":0,"block":{"type":"reasoning","text":"The plan was approved. The user's last instruction says: \"Once the plan is approved, reply with the single word DONE and stop.\" So I should just reply with DONE and stop."}}}} +{"type":"assistant/chunk","seq":337,"time":1785406810256,"data":{"turn":1,"step":2,"chunk":{"type":"block-end","index":1,"block":{"type":"text","text":"DONE"}}}} +{"type":"assistant/chunk","seq":338,"time":1785406810256,"data":{"turn":1,"step":2,"chunk":{"type":"usage","usage":{"inputTokens":441,"outputTokens":43,"cacheReadTokens":4608,"reasoningTokens":40}}}} +{"type":"assistant/chunk","seq":339,"time":1785406810256,"data":{"turn":1,"step":2,"chunk":{"type":"finish","reason":{"kind":"stop"}}}} +{"type":"assistant/message","seq":340,"time":1785406810257,"data":{"turn":1,"step":2,"message":{"role":"assistant","content":[{"type":"reasoning","text":"The plan was approved. The user's last instruction says: \"Once the plan is approved, reply with the single word DONE and stop.\" So I should just reply with DONE and stop."},{"type":"text","text":"DONE"}],"source":{"kind":"model","provider":"deepseek","model":"deepseek-v4-flash"},"id":"2d9da552-d236-4166-9d06-dffa9bace4cd"},"usage":{"inputTokens":441,"outputTokens":43,"cacheReadTokens":4608,"reasoningTokens":40}},"sourceEventSeqs":[292,293,294,295,296,297,298,299,300,301,302,303,304,305,306,307,308,309,310,311,312,313,314,315,316,317,318,319,320,321,322,323,324,325,326,327,328,329,330,331,332,333,334,335,336,337,338,339],"surfaceOp":"append"} +{"type":"step/end","seq":341,"time":1785406810258,"data":{"turn":1,"step":2}} +{"type":"turn/end","seq":342,"time":1785406810258,"data":{"turn":1,"reason":{"kind":"completed"}}} diff --git a/packages/client/ui-question/README.i18n.yaml b/packages/client/ui-question/README.i18n.yaml index 26f790b296..cd2f0556a4 100644 --- a/packages/client/ui-question/README.i18n.yaml +++ b/packages/client/ui-question/README.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 packages/client/ui-question/README.md -README.md: 0700375758774610fcd897b9a3e16484206a871d -README.zh.md: d9e5eb22cef13e16ab1ce2cebba9e563bd9d08d9 +README.md: 6b40c503be61c2458cefceabbb35e16a3befbe66 +README.zh.md: 7c0c4ee67d1ef8b4507f3899bdb1ae2f76408032 diff --git a/packages/client/ui-question/README.md b/packages/client/ui-question/README.md index 0700375758..6b40c503be 100644 --- a/packages/client/ui-question/README.md +++ b/packages/client/ui-question/README.md @@ -6,6 +6,8 @@ Web `ask_user_question` feature plugin. Its host half mounts `dsh-tool-ask-user` The component renders one question at a time with progress navigation, single- and multi-select choices, recommendation badges derived from label suffixes, and custom answers. Question detail reuses the assistant-output `MarkdownText` primitive, including its GFM rendering and untrusted-content policy. The capped card keeps its title, navigation, and submission actions fixed while long detail and choices share an internal scroll region. Single-select choices advance immediately, and Enter submits once every question is answered or skipped; Enter during IME composition confirms the input candidate without advancing. It submits one structured answer batch for the whole request: “Skip this question” retains other drafts and emits the existing blank `{ selected: [] }` shape for that item, while close rejects the whole wait as `ASK_CANCELLED`. +A request whose single question declares a presentation intent renders as that intent's own surface instead. `plan-review` — set by `dsh-plan-mode` on the `exit_plan_mode` review — takes the waiting-approval card shape: a `Plan review` strip, the plan as the scrolling markdown body, the question text as the card's accessible name, and one decision row of `Chat about it` / `Refuse` / `Approve`. Approve and Refuse answer with the asker's own option labels (the intent names which label approves, so the verdict never rides option order) and keep the asker's descriptions as tooltips; `Chat about it` rejects the wait as `ASK_CANCELLED`, returning the composer so the user can say what they want instead. A request that declares no intent, batches more than one question, or fails to offer the named approve label stays on the generic flow — the layout is all an intent changes, never the answer. + Selection state is local to a component keyed by the request rpcId. A replay with the same id preserves a still-mounted draft, while `question/resolved` from the host removes the composer. The host remains authoritative: successful HTTP delivery does not remove pending state locally. Composer chrome copy (pager, buttons, placeholders, validation feedback) is bilingual: the plugin registers zh/en dictionaries under the `question` namespace of `dsh-client-locale` and hands the entry its bound translator plus the locale snapshot source through the inject face, so a locale switch re-renders a mounted composer. Question and option text arrives from the model and renders verbatim; carrier failure messages also display untranslated. diff --git a/packages/client/ui-question/README.zh.md b/packages/client/ui-question/README.zh.md index d9e5eb22ce..7c0c4ee67d 100644 --- a/packages/client/ui-question/README.zh.md +++ b/packages/client/ui-question/README.zh.md @@ -6,6 +6,8 @@ Web `ask_user_question` 功能插件。只有选择 Web 功能时，其主机侧 组件每次渲染一个问题，提供进度导航、单选和多选选项、由标签后缀派生的推荐徽标，以及自定义答案。问题详情复用助手输出的 `MarkdownText` 原语，包括其 GFM 渲染与不受信内容策略。封顶卡片保持标题、导航与提交动作固定，超长的详情与选项共享内部滚动区。单选选项会立即前进；所有问题均已回答或跳过后，Enter 会提交；IME 输入法组合期间按 Enter 只会确认输入候选，不会前进。组件为整个请求提交一批结构化答案：「跳过此问题」会保留其他草稿，并为该项发出既有的空 `{ selected: [] }` 形状；关闭则以 `ASK_CANCELLED` 拒绝整个等待。 +若某个请求的唯一问题声明了呈现意图，则改为渲染该意图自己的界面。`plan-review` —— 由 `dsh-plan-mode` 在 `exit_plan_mode` 审阅上设置 —— 采用等待审批卡片的形状：一条 `Plan review` 条带、计划作为可滚动的 markdown 主体、问题文本作为卡片的无障碍名称，以及一行 `Chat about it` / `Refuse` / `Approve` 的决定操作。Approve 与 Refuse 用提问方自己的选项标签回答（意图指名哪个标签表示批准，因此裁决绝不依赖选项顺序），并把提问方的描述保留为 tooltip；`Chat about it` 以 `ASK_CANCELLED` 拒绝该等待，让编辑器归位，用户可以直接说出他想说的话。未声明意图、一批含多个问题、或未提供被指名的批准标签的请求，一律留在通用流程上 —— 意图改变的只是布局，从不改变答案。 + 选择状态只存在于以请求 rpcId 为 key 的组件本地。使用相同 id 回放时，只要组件仍挂载，就会保留草稿；主机发出的 `question/resolved` 则会移除编辑器。主机仍具有最终决定权：HTTP 交付成功不会在本地移除待处理状态。 编辑器外框文案（翻页器、按钮、占位符、校验提示）是双语的：插件在 `dsh-client-locale` 的 `question` 命名空间下注册 zh/en 词典，并通过 inject face 把绑定的翻译函数和 locale 快照源交给该配置项，因此切换语言会重新渲染已挂载的编辑器。问题与选项文本来自模型并原样渲染；载体失败消息也不经翻译直接显示。 diff --git a/packages/client/ui-question/src/client/PlanReviewPanel.module.css b/packages/client/ui-question/src/client/PlanReviewPanel.module.css new file mode 100644 index 0000000000..428effab2b --- /dev/null +++ b/packages/client/ui-question/src/client/PlanReviewPanel.module.css @@ -0,0 +1,113 @@ +/* Plan-review takeover: the waiting-approval card language (amber strip on a + floating capsule, right-aligned actions) applied to a reviewed plan. Kept as + its own module rather than shared with ui-conversation's ApprovalPanel: the + two takeovers agree on tokens and geometry, not on content — this one's body + is scrollable markdown, that one's is a headline plus a command line. Warn + semantics ride the alias state tokens; no hardcoded colors. */ + +/* Mirrors the question card's frame so the takeover is a content swap. */ +.frame { + display: flex; + justify-content: center; + padding: 6px 24px 10px; +} + +.card { + display: flex; + overflow: hidden; + flex-direction: column; + width: 100%; + max-width: 776px; + /* Composer seat sits in a fixed-height conversation column (overflow + hidden): cap the card against the viewport and scroll the plan, so the + strip and the decision row stay reachable on a long plan. */ + max-height: min(60vh, 520px); + border: 1px solid var(--dsw-alias-state-warn-secondary); + border-radius: 20px; + background: var(--dsw-specific-input-major); + box-shadow: var(--dsw-shadow-lv2); + color: var(--dsw-alias-label-primary); + /* Elevated surface in dark: the plan body inside scrolls once the card hits + the cap above, so the thumb takes the l2 pair (see ui-theme + styles/scrollbar.css for the rebinding contract). */ + --dsh-scrollbar-thumb: var(--dsw-alias-scrollbar-bg-l2); + --dsh-scrollbar-thumb-hover: var(--dsw-alias-scrollbar-hover-l2); +} + +.card, +.card * { + box-sizing: border-box; +} + +/* Tinted full-width header band, as on the approval takeover. */ +.strip { + display: flex; + align-items: center; + flex-shrink: 0; + gap: 8px; + padding: 10px 16px; + background: var(--dsw-alias-state-warn-tertiary); + color: var(--dsw-alias-state-warn-primary); + font-size: 13px; + line-height: 18px; +} + +.dot { + width: 8px; + height: 8px; + border-radius: 50%; + background: var(--dsw-alias-state-warn-primary); +} + +/* The plan is the panel's message: it takes the whole body and the scroll. */ +.body { + flex: 1 1 auto; + min-height: 0; + overflow-y: auto; + overscroll-behavior: contain; + padding: 12px 16px 4px; + font-size: 14px; + line-height: 22px; +} + +.footer { + display: flex; + align-items: center; + justify-content: space-between; + flex-shrink: 0; + gap: 12px; + padding: 8px 16px 12px; +} + +.feedback { + min-height: 16px; + color: var(--dsw-alias-state-error-primary); + font-size: 11px; + line-height: 16px; +} + +.actions { + display: flex; + align-items: center; + flex-shrink: 0; + gap: 8px; +} + +@media (max-width: 720px) { + .frame { + padding: 6px 10px 10px; + } + + .card { + border-radius: 16px; + } + + .body { + padding: 10px 12px 4px; + } + + .footer { + align-items: flex-end; + padding: 8px 12px 10px; + } +} diff --git a/packages/client/ui-question/src/client/PlanReviewPanel.tsx b/packages/client/ui-question/src/client/PlanReviewPanel.tsx new file mode 100644 index 0000000000..020df9c82c --- /dev/null +++ b/packages/client/ui-question/src/client/PlanReviewPanel.tsx @@ -0,0 +1,100 @@ +// PlanReviewPanel: the composer takeover for a question carrying the +// `plan-review` presentation intent. A plan under review is one decision over +// one body of markdown, so it takes the waiting-approval card shape — tinted +// strip, content, right-aligned action row — instead of the generic question +// flow's pager, numbered options, skip and custom-answer affordances, which +// read as a quiz the user is being graded on. +// +// The three actions are the whole decision surface: approve and decline answer +// the question with the option labels the asker offered (localised copy on the +// buttons, the asker's descriptions as their tooltips), while "discuss" +// dismisses the request so the composer returns and the user can simply say +// what they want. Dismissal is the generic flow's own cancel verb, promoted to +// a labelled button because in a two-outcome decision it is the third real +// answer, not an escape hatch. + +import { useState } from 'react' +import { Button, IconEditOutline16, MarkdownText } from '@deepseek-ai/dsh-client-ui-primitives' +import type { PendingQuestion, PlanReview, QuestionComposerProps } from './contract/slots.ts' +import css from './PlanReviewPanel.module.css' + +/** The panel's own props: the question domain face, the narrowed review, and the locale seat. */ +export type PlanReviewPanelProps = + { pending: PendingQuestion; review: PlanReview } & Pick + +/** + * Optional-prop spread for a decision button's tooltip: `title` is optional on + * the DOM props, and exactOptionalPropertyTypes rejects an explicit undefined. + * + * @param description - the asker's option description, when it carries one. + * @returns The `title` prop to spread, or nothing. + */ +function tooltip(description: string | undefined): { title?: string } { + return description === undefined ? {} : { title: description } +} + +/** + * Render a plan review as a decision card. + * + * @param props - the question domain face, the narrowed plan review, and `t`. + * @returns The plan-review takeover for this request. + */ +export function PlanReviewPanel({ pending, review, t }: PlanReviewPanelProps) { + // One-shot latch shaped like the approval takeover's: the panel leaves only + // when the host's resolved frame lands, so until then a second click must + // not re-fire. A failed send (rejected receipt / transport) re-arms it and + // shows why, since nothing else would tell the user the click was lost. + const [busy, setBusy] = useState(false) + const [error, setError] = useState(null) + const settle = (send: () => Promise): void => { + setBusy(true) + setError(null) + void send().catch((cause: unknown) => { + setBusy(false) + setError(cause instanceof Error ? cause.message : String(cause)) + }) + } + const decide = (label: string): void => { + settle(() => pending.answer({ answers: [{ id: review.id, selected: [label] }] })) + } + const decline = review.decline + + return ( +
+
+
+ + {t('plan.header')} +
+
+ +
+
+
{error}
+
+ + {decline !== undefined && ( + + )} + +
+
+
+
+ ) +} diff --git a/packages/client/ui-question/src/client/QuestionComposer.tsx b/packages/client/ui-question/src/client/QuestionComposer.tsx index 30caf7f98e..abe87f567e 100644 --- a/packages/client/ui-question/src/client/QuestionComposer.tsx +++ b/packages/client/ui-question/src/client/QuestionComposer.tsx @@ -5,9 +5,10 @@ import { IconCloseOutline16, IconEditOutline16, MarkdownText, } from '@deepseek-ai/dsh-client-ui-primitives' import { - PendingQuestion, + PendingQuestion, planReviewOf, type QuestionAnswer, type QuestionComposerProps, } from './contract/slots.ts' +import { PlanReviewPanel } from './PlanReviewPanel.tsx' import css from './QuestionComposer.module.css' interface DraftAnswer { @@ -46,14 +47,24 @@ function isComposing(event: KeyboardEvent new PendingQuestion(props.matched), [props.matched]) - return + const review = useMemo(() => planReviewOf(question.questions), [question]) + return review === undefined + ? + : } function QuestionFlow({ pending, t }: { pending: PendingQuestion } & Pick) { diff --git a/packages/client/ui-question/src/client/contract/slots.ts b/packages/client/ui-question/src/client/contract/slots.ts index 54e87c016d..f92eb33f52 100644 --- a/packages/client/ui-question/src/client/contract/slots.ts +++ b/packages/client/ui-question/src/client/contract/slots.ts @@ -19,6 +19,65 @@ export type QuestionWait = PendingWait<'question'> /** One structured answer batch covering every question of the request. */ export type QuestionAnswer = QuestionResponsePayload['answer'] +/** One question of the request, as the carrier payload carries it. */ +type QuestionItem = QuestionWait['payload']['questions'][number] + +/** One option the asker offered on a question. */ +type QuestionOption = NonNullable[number] + +/** + * A request narrowed to the `plan-review` presentation intent: everything the + * decision card renders and answers with, so the panel never re-reads the + * request shape. `approve` and `decline` are the asker's own options — an + * answer must carry one of those labels verbatim — and `plan` is the markdown + * body under review. + */ +export interface PlanReview { + /** The reviewed question's id, echoed in the answer. */ + id: string + /** The question text, kept as the card's accessible name. */ + question: string + /** The plan markdown under review. */ + plan: string + /** The option that approves the plan. */ + approve: QuestionOption + /** The option that declines it; absent when the asker offered no other option. */ + decline?: QuestionOption +} + +/** + * Narrow a request to a renderable plan review, or return undefined to leave it + * to the generic question flow. + * + * The card is one decision over one plan, so it claims a request only when the + * batch is a single question that declares the intent, carries the plan as its + * detail, and offers the approve label the intent names. The asker's own + * service validates that label, but this is a wire boundary: a request failing + * any part of it still renders and stays answerable as a generic question + * rather than reaching a card that cannot express it. + * + * @param questions - the request's whole question batch. + * @returns The narrowed review, or undefined when the generic flow owns it. + */ +export function planReviewOf(questions: readonly QuestionItem[]): PlanReview | undefined { + if (questions.length !== 1) return undefined + // Length-checked above; the index read is the narrowing tax, not a guess. + const question = questions[0] as QuestionItem + const intent = question.intent + if (intent?.kind !== 'plan-review' || question.detail === undefined) return undefined + const options = question.options ?? [] + const approve = options.find(option => option.label === intent.approve) + if (approve === undefined) return undefined + const decline = options.find(option => option.label !== intent.approve) + return { + id: question.id, + question: question.question, + plan: question.detail, + approve, + ...(decline === undefined ? {} : { decline }), + } +} + /** * Question domain face over the carrier: render identity and questions * transparently forwarded; answer/cancel own the wire encoding (the ok value diff --git a/packages/client/ui-question/src/client/index.ts b/packages/client/ui-question/src/client/index.ts index 63f7517c3a..9c6893fd18 100644 --- a/packages/client/ui-question/src/client/index.ts +++ b/packages/client/ui-question/src/client/index.ts @@ -5,6 +5,12 @@ * question carrier (matched prop), and the whole behavior surface rides the * carrier (domain encoding in contract/slots.ts PendingQuestion); copy rides * the standard locale seat. Export discipline: packages/client/AGENTS.md. + * + * One entry, two shapes: the composer renders a request that declares a + * presentation intent as that intent's own surface (`plan-review` → the plan + * decision card) and every other request as the generic question flow. A + * separate chain entry per shape would race the same carrier, so the shape + * choice lives inside this entry — see QuestionComposer. */ import type { ClientContext } from '@deepseek-ai/dsh-client-runtime/client' import type { ComposerChainProps } from '@deepseek-ai/dsh-client-ui-conversation/client' @@ -14,8 +20,10 @@ import type { QuestionWait } from './contract/slots.ts' import { QuestionComposer } from './QuestionComposer.tsx' import { en, zh, type QuestionKey } from './locales.ts' -export { PendingQuestion } from './contract/slots.ts' -export type { QuestionAnswer, QuestionComposerProps, QuestionWait } from './contract/slots.ts' +export { PendingQuestion, planReviewOf } from './contract/slots.ts' +export type { + PlanReview, QuestionAnswer, QuestionComposerProps, QuestionWait, +} from './contract/slots.ts' export type { QuestionKey } from './locales.ts' declare module '@deepseek-ai/dsh-client-ui-slots' { diff --git a/packages/client/ui-question/src/client/locales.ts b/packages/client/ui-question/src/client/locales.ts index f124b34da2..bd5ee515ae 100644 --- a/packages/client/ui-question/src/client/locales.ts +++ b/packages/client/ui-question/src/client/locales.ts @@ -11,6 +11,10 @@ export const zh = { 'custom.placeholder': '输入你的答案', 'action.skip': '跳过本题', 'action.next': '下一题', + 'plan.header': '计划待审', + 'plan.approve': '确认执行', + 'plan.decline': '拒绝', + 'plan.discuss': '去聊天里说', } satisfies Record /** The question namespace key union. */ @@ -27,4 +31,8 @@ export const en = { 'custom.placeholder': 'Type your answer', 'action.skip': 'Skip this question', 'action.next': 'Next', + 'plan.header': 'Plan review', + 'plan.approve': 'Approve', + 'plan.decline': 'Refuse', + 'plan.discuss': 'Chat about it', } satisfies Record diff --git a/packages/client/ui-question/tests/plan-review-panel.spec.tsx b/packages/client/ui-question/tests/plan-review-panel.spec.tsx new file mode 100644 index 0000000000..3c2c456c57 --- /dev/null +++ b/packages/client/ui-question/tests/plan-review-panel.spec.tsx @@ -0,0 +1,221 @@ +// @vitest-environment jsdom +// The plan-review takeover, driven through the composer entry that routes to +// it: a request carrying the intent must reach the decision card and answer +// with the asker's own option labels, and a request that does not (or cannot) +// must keep the generic question flow. +import { afterEach, describe, expect, it, vi } from 'vitest' +import { cleanup, fireEvent, render, screen } from '@testing-library/react' +import type { + ConversationSnapshot, SessionId, SessionListState, WorkspaceListState, +} from '@deepseek-ai/dsh-client-runtime/client' +import { PendingWait } from '@deepseek-ai/dsh-client-runtime/client' +import type { RpcReceipt } from '@deepseek-ai/dsh-client-connection/client' +import { RpcId } from '@deepseek-ai/dsh-client-connection/client' +import type { SnapshotSelectorHook } from '@deepseek-ai/dsh-client-ui-slots' +import { planReviewOf, type QuestionComposerProps, type QuestionWait } from '../src/client/contract/slots.ts' +import { QuestionComposer } from '../src/client/QuestionComposer.tsx' +import { en, zh } from '../src/client/locales.ts' +import { en as commonEn } from '@deepseek-ai/dsh-client-locale/src/locales/en.ts' +import { zh as commonZh } from '@deepseek-ai/dsh-client-locale/src/locales/zh.ts' + +afterEach(cleanup) + +const SID = 's1' as SessionId + +/** Seat stub over a dictionary pair mirroring the real lookup chain: package dictionary, then common vocabulary, then the key. */ +const seatOver = (dict: Record, common: Record): QuestionComposerProps['t'] => + (key => dict[key] ?? common[key] ?? key) + +/** Framework standard-kit stubs: the panel consumes only the locale seat. */ +const kit = { + sessionId: SID, + useSession: (() => { throw new Error('unused') }) as unknown as SnapshotSelectorHook, + useSessions: (() => { throw new Error('unused') }) as unknown as SnapshotSelectorHook, + useWorkspaces: (() => { throw new Error('unused') }) as unknown as SnapshotSelectorHook, + useProjection: (() => undefined) as never, + useInput: (() => { throw new Error('unused') }) as never, + inputActions: { setDraft: () => { throw new Error('unused') }, submit: () => { throw new Error('unused') } } as never, + t: seatOver(zh, commonZh), +} + +const PLAN = '# Ship the picker\n\n- read the store\n- render the rows\n' + +/** The plan-mode request shape: one question, the plan as detail, approve named. */ +const questions = (): QuestionWait['payload']['questions'] => [{ + id: 'plan-review', + header: 'Plan review', + question: 'Approve this plan and leave plan mode?', + detail: PLAN, + options: [ + { label: 'Approve', description: 'Leave plan mode; the plan is carried out from the next step.' }, + { label: 'Keep planning', description: 'Stay in plan mode; feedback goes back to the model.' }, + ], + intent: { kind: 'plan-review', approve: 'Approve' }, +}] + +/** Carrier fixture over a scripted respond carrier. */ +function wait( + payload: QuestionWait['payload'] = { questions: questions() }, + respond = vi.fn(() => Promise.resolve({ accepted: true })), +) { + return { carrier: new PendingWait('question', RpcId('q-1'), SID, payload, respond), respond } +} + +/** The client-response envelope respond must have received for a decision. */ +function decidedEnvelope(label: string) { + return { + type: 'client-response', rpcId: RpcId('q-1'), + result: { ok: true, value: { sessionId: SID, answer: { answers: [{ id: 'plan-review', selected: [label] }] } } }, + } +} + +describe('planReviewOf', () => { + it('narrows a plan-review request to its decision, options included', () => { + expect(planReviewOf(questions())).toEqual({ + id: 'plan-review', + question: 'Approve this plan and leave plan mode?', + plan: PLAN, + approve: { label: 'Approve', description: 'Leave plan mode; the plan is carried out from the next step.' }, + decline: { label: 'Keep planning', description: 'Stay in plan mode; feedback goes back to the model.' }, + }) + }) + + it('leaves the decline absent when the asker offered approve alone', () => { + const [question] = questions() + const review = planReviewOf([{ ...question as object, options: [{ label: 'Approve' }] } as never]) + expect(review?.approve).toEqual({ label: 'Approve' }) + expect(review === undefined ? true : 'decline' in review).toBe(false) + }) + + it.each([ + ['a batch of more than one question', () => [...questions(), ...questions()]], + ['no intent at all', () => [{ ...questions()[0] as object, intent: undefined }]], + ['an intent without the plan as detail', () => [{ ...questions()[0] as object, detail: undefined }]], + ['an intent whose approve names no option', () => [{ + ...questions()[0] as object, intent: { kind: 'plan-review', approve: 'Ship it' }, + }]], + ['an intent with no options at all', () => [{ ...questions()[0] as object, options: undefined }]], + ])('declines %s, leaving the request to the generic flow', (_case, build) => { + expect(planReviewOf(build() as never)).toBeUndefined() + }) + + it('declines an empty batch, which the generic flow reports as such', () => { + expect(planReviewOf([])).toBeUndefined() + }) +}) + +describe('PlanReviewPanel', () => { + it('renders the plan under a review strip, with none of the quiz affordances', () => { + const { carrier } = wait() + render() + + expect(document.querySelector('[data-plan-review-key="q:q-1"]')).toBeTruthy() + expect(screen.getByText(zh['plan.header'])).toBeTruthy() + // The plan renders as markdown, so its heading is a heading. + expect(screen.getByRole('heading', { name: 'Ship the picker' })).toBeTruthy() + expect(screen.getByText('render the rows')).toBeTruthy() + // The question text stays as the card's accessible name rather than a title + // that reads like a test item. + expect(screen.getByLabelText('Approve this plan and leave plan mode?')).toBeTruthy() + // No pager, no numbered options, no skip, no custom answer. + expect(screen.queryByText('1 / 1')).toBeNull() + expect(screen.queryByRole('radio')).toBeNull() + expect(screen.queryByText(zh['action.skip'])).toBeNull() + expect(screen.queryByRole('textbox')).toBeNull() + }) + + it('answers with the asker\'s approve label and keeps its description as the tooltip', () => { + const { carrier, respond } = wait() + render() + + const approve = screen.getByRole('button', { name: zh['plan.approve'] }) + expect(approve.getAttribute('title')).toBe('Leave plan mode; the plan is carried out from the next step.') + fireEvent.click(approve) + expect(respond).toHaveBeenCalledWith(decidedEnvelope('Approve')) + // One-shot: every action locks until the host's resolved frame lands. + expect(approve.hasAttribute('disabled')).toBe(true) + expect(screen.getByRole('button', { name: zh['plan.decline'] }).hasAttribute('disabled')).toBe(true) + fireEvent.click(approve) + expect(respond).toHaveBeenCalledTimes(1) + }) + + it('answers with the asker\'s decline label', () => { + const { carrier, respond } = wait() + render() + + fireEvent.click(screen.getByRole('button', { name: zh['plan.decline'] })) + expect(respond).toHaveBeenCalledWith(decidedEnvelope('Keep planning')) + }) + + it('dismisses the request so the composer returns for a plain message', () => { + const { carrier, respond } = wait() + render() + + fireEvent.click(screen.getByRole('button', { name: zh['plan.discuss'] })) + expect(respond).toHaveBeenCalledWith({ + type: 'client-response', rpcId: RpcId('q-1'), + result: { + ok: false, + error: { code: 'cancelled', message: 'the user closed this question request', details: {} }, + }, + }) + }) + + it('omits the tooltip for an option carrying no description', () => { + const { carrier } = wait({ questions: [{ + ...questions()[0] as object, + options: [{ label: 'Approve' }, { label: 'Keep planning' }], + }] as never }) + render() + + expect(screen.getByRole('button', { name: zh['plan.approve'] }).hasAttribute('title')).toBe(false) + expect(screen.getByRole('button', { name: zh['plan.decline'] }).hasAttribute('title')).toBe(false) + }) + + it('hides the decline action when the asker offered approve alone', () => { + const { carrier } = wait({ questions: [{ + ...questions()[0] as object, options: [{ label: 'Approve' }], + }] as never }) + render() + + expect(screen.queryByRole('button', { name: zh['plan.decline'] })).toBeNull() + expect(screen.getByRole('button', { name: zh['plan.approve'] })).toBeTruthy() + }) + + it('re-arms the actions and says why when the decision does not land', async () => { + const { carrier, respond } = wait( + { questions: questions() }, + vi.fn(() => Promise.resolve({ accepted: false, reason: 'not-pending' })), + ) + render() + + fireEvent.click(screen.getByRole('button', { name: zh['plan.approve'] })) + const failure = await screen.findByText('question response rejected: not-pending') + expect(failure.getAttribute('role')).toBe('status') + // Re-armed for the retry: a lost click must not leave a dead card. + expect(screen.getByRole('button', { name: zh['plan.approve'] }).hasAttribute('disabled')).toBe(false) + fireEvent.click(screen.getByRole('button', { name: zh['plan.approve'] })) + expect(respond).toHaveBeenCalledTimes(2) + }) + + it('reports a non-Error transport failure as its stringified value', async () => { + // A non-Error rejection is the case under test: a carrier can reject with + // anything, and the panel must still show the user something. + // oxlint-disable-next-line typescript/prefer-promise-reject-errors + const { carrier } = wait({ questions: questions() }, vi.fn(() => Promise.reject('socket gone'))) + render() + + fireEvent.click(screen.getByRole('button', { name: zh['plan.discuss'] })) + expect(await screen.findByText('socket gone')).toBeTruthy() + }) + + it('carries the same decision surface in English', () => { + const { carrier } = wait() + render() + + expect(screen.getByText('Plan review')).toBeTruthy() + expect(screen.getByRole('button', { name: 'Approve' })).toBeTruthy() + expect(screen.getByRole('button', { name: 'Refuse' })).toBeTruthy() + expect(screen.getByRole('button', { name: 'Chat about it' })).toBeTruthy() + }) +})