Parcourir la source

Merge pull request #646 from deepseek-harness/fix/intent-draft-sync-echo

fix(web): IME composition corrupts the "Let's start building" composer
Chinesezjc il y a 2 mois
Parent
commit
3586422b69

+ 6 - 0
.agents/notes/implemented/bug-fix/2026-07-26-intent-draft-same-tick-echo.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
+2026-07-26-intent-draft-same-tick-echo.md: 1a4fdb48c0434bd37d7771dddb640720e1b610e6
+2026-07-26-intent-draft-same-tick-echo.zh.md: 9ecdf7154f5014de242021f99d2e51959c2a3169

+ 27 - 0
.agents/notes/implemented/bug-fix/2026-07-26-intent-draft-same-tick-echo.md

@@ -0,0 +1,27 @@
+# Agent Note: Intent draft echoes in the same tick
+
+Status: implemented
+
+English | [中文](2026-07-26-intent-draft-same-tick-echo.zh.md)
+
+## Problem
+
+The hero composer ("Let's start building") is a controlled textarea whose value is the frontend Session Intent's retained prompt, read from the sessions **list** snapshot (`EmptyState` binds `intent.prompt` via `useSessions`). Typing routed through `SessionManager.updateIntent → Session.updatePendingPrompt`, which flushes the **Session's own** notifier synchronously — but the list snapshot the composer actually renders from only heard about the change through the intent watch subscription in `startIntent`, which calls `markDirty()`, a microtask-deferred flush.
+
+A deferred echo violates the controlled-input contract documented on the Notifier (see the [web client architecture note](../architecture/2026-07-19-gui-web-client-architecture.md)): React compares the DOM value against the still-stale snapshot during the same tick as `onChange` and rolls the textarea back. With plain typing this shows as caret jumps; with an IME it corrupts input — every composition update gets rolled back and re-applied against a stale value, so typing Pinyin "nihao" commits fragments like "nnini hni hani hao你好". The resident composer (`ConversationRoot`) was not affected: its draft lives in the chat store (sync flush) or comes from `updateSessionPrompt`, which reads the Session snapshot directly rather than the list projection.
+
+## Decision
+
+`SessionManager.updateIntent` calls `this.notifier.notifyNow()` after `updatePendingPrompt`, flushing the list snapshot in the same tick as the change event. This matches the Notifier's channel rule: a direct echo of a user gesture whose controlled input renders from this snapshot uses `notifyNow`; the intent watch keeps `markDirty` for every other (async) intent transition.
+
+## Alternatives considered
+
+**Change the intent watch callback in `startIntent` to `notifyNow`.** Wrong channel for that seam: the watch also fires on frame-driven Session changes (publication, send phases), and the architecture note bans `notifyNow` for frame-driven sources because it collapses batching.
+
+**Have `EmptyState` read the prompt from the Session snapshot instead of the list.** Restructures the slot contract (EmptyState is deliberately bound to the standard `useSessions` feed and has no session scope yet — the frontend Session is page-local) for no gain over flushing the projection it already reads.
+
+**Suppress the rollback in `InputBar` with local uncontrolled state.** Hides the symptom, forfeits the single-source-of-truth draft (the retained prompt must survive workspace retargeting and send/retry), and leaves every other list-snapshot-controlled input exposed.
+
+## Consequences
+
+Typing in the hero composer, IME composition included, echoes synchronously. `updateIntent` on a no-intent state stays a no-op with no notification. The web workspace-flow snapshot's composer helper now asserts the same-tick echo instead of waiting for it, so a regression to a deferred echo fails the keyless snapshot gate; a runtime unit test pins the same contract at the manager seam.

+ 27 - 0
.agents/notes/implemented/bug-fix/2026-07-26-intent-draft-same-tick-echo.zh.md

@@ -0,0 +1,27 @@
+# Agent Note: Intent draft echoes in the same tick
+
+Status: implemented
+
+[English](2026-07-26-intent-draft-same-tick-echo.md) | 中文
+
+## Problem
+
+hero composer(「Let's start building」)是一个受控(controlled)的 textarea,它的值取自前端 Session Intent 保留下来的提示词,读自会话**列表**快照(`EmptyState` 通过 `useSessions` 绑定 `intent.prompt`)。输入经由 `SessionManager.updateIntent → Session.updatePendingPrompt`,后者会同步刷新 **Session 自身的** notifier——但 composer 实际渲染所依据的那份列表快照,只能通过 `startIntent` 中的 intent watch 订阅得知这次变更,而该订阅调用的是 `markDirty()`,即一次延迟到微任务的刷新。
+
+延迟的回显违反了 Notifier 上所记录的受控输入契约(见 [web 客户端架构笔记](../architecture/2026-07-19-gui-web-client-architecture.md)):React 在与 `onChange` 相同的 tick 内,把 DOM 值与仍然陈旧的快照相比对,随后把 textarea 回滚。普通输入时,这表现为光标跳动;使用输入法(IME)时,它会损坏输入——每一次 composition 更新都会被回滚,并针对陈旧的值重新应用,因此输入拼音「nihao」会提交出类似「nnini hni hani hao你好」这样的片段。resident composer(`ConversationRoot`)不受影响:它的草稿存放在 chat store 中(同步刷新),或来自 `updateSessionPrompt`,后者直接读取 Session 快照,而不是列表投影。
+
+## Decision
+
+`SessionManager.updateIntent` 在 `updatePendingPrompt` 之后调用 `this.notifier.notifyNow()`,从而在与变更事件相同的 tick 内刷新列表快照。这符合 Notifier 的通道规则:当某个用户手势的受控输入正是从该快照渲染时,对它的直接回显使用 `notifyNow`;而 intent watch 对其余所有(异步的)intent 状态转换仍保留 `markDirty`。
+
+## Alternatives considered
+
+**把 `startIntent` 中的 intent watch 回调改为 `notifyNow`。** 对那个 seam 而言是错误的通道:该 watch 也会在帧驱动的 Session 变更(发布、发送阶段)时触发,而架构笔记禁止对帧驱动的来源使用 `notifyNow`,因为那会瓦解批处理。
+
+**让 `EmptyState` 从 Session 快照而非列表读取提示词。** 这会重构槽位契约(EmptyState 有意绑定到标准的 `useSessions` 数据源,且尚无 session 作用域——前端 Session 是页面本地的),相比刷新它本就读取的那份投影并无收益。
+
+**在 `InputBar` 中用本地的非受控状态抑制回滚。** 这只是掩盖症状,放弃了单一真源的草稿(保留下来的提示词必须在工作区重定向以及发送/重试后依然存在),并让其余每一个由列表快照控制的输入都暴露在同一问题之下。
+
+## Consequences
+
+在 hero composer 中输入(包括输入法 composition 在内)会同步回显。在无 intent 的状态上调用 `updateIntent` 仍是一次空操作,不发出任何通知。web workspace-flow 快照的 composer 辅助函数现在断言的是同一 tick 内的回显,而不是等待它,因此一旦回退成延迟回显,就会让无密钥快照门禁失败;一个运行时单元测试在 manager 这一 seam 处钉住了同一份契约。

+ 10 - 8
apps/web/tests/workspace-flow.snapshot.ts

@@ -119,10 +119,12 @@ function workspaceChip(): HTMLElement {
   return chip
 }
 
-/** Wait for the runtime-owned controlled input to echo a browser edit. */
-async function setComposerText(composer: HTMLElement, value: string): Promise<void> {
+/** Edit the runtime-owned controlled input and assert the same-tick echo:
+ *  a deferred echo makes React roll the textarea back mid-IME-composition,
+ *  committing partial keystrokes (e.g. Pinyin "nihao" leaking as "nnini h…"). */
+function setComposerText(composer: HTMLElement, value: string): void {
   fireEvent.change(composer, { target: { value } })
-  await waitFor(() => { expect((composer as HTMLTextAreaElement).value).toBe(value) })
+  expect((composer as HTMLTextAreaElement).value).toBe(value)
 }
 
 it('starts a writable page-local draft without inventing a sidebar Workspace', async () => {
@@ -130,7 +132,7 @@ it('starts a writable page-local draft without inventing a sidebar Workspace', a
 
   const composer = await screen.findByPlaceholderText('Describe what you want to build', {}, { timeout: 10_000 })
   const tree = screen.getByRole('tree', { name: 'Sessions' })
-  await setComposerText(composer, 'keep this local')
+  setComposerText(composer, 'keep this local')
 
   expect({
     headline: visibleText(screen.getByText("Let's start building")),
@@ -191,7 +193,7 @@ it('drops the page-local draft on refresh while retaining real Workspaces and Se
 
   const composer = await screen.findByPlaceholderText('Describe what you want to build', {}, { timeout: 10_000 })
   const tree = screen.getByRole('tree', { name: 'Sessions' })
-  await setComposerText(composer, 'discard this page-local draft')
+  setComposerText(composer, 'discard this page-local draft')
   const beforeGroup = within(tree).getByText('4 sessions').closest('[role="treeitem"]')
   if (beforeGroup === null) throw new Error('fixture Workspace projection missing before refresh')
 
@@ -235,7 +237,7 @@ it('keeps a published Session with only cwd membership evidence in Ungrouped', a
   boot('?fixture&fixtureAttach=fail')
 
   const composer = await screen.findByPlaceholderText('Describe what you want to build', {}, { timeout: 10_000 })
-  await setComposerText(composer, 'keep this cwd-only session')
+  setComposerText(composer, 'keep this cwd-only session')
   fireEvent.click(screen.getByRole('button', { name: 'Send message' }))
 
   const tree = screen.getByRole('tree', { name: 'Sessions' })
@@ -270,7 +272,7 @@ it('materializes the automatic Workspace and Session on the first successful sen
   boot('?fixture=empty')
 
   const composer = await screen.findByPlaceholderText('Describe what you want to build', {}, { timeout: 10_000 })
-  await setComposerText(composer, 'build a lighthouse')
+  setComposerText(composer, 'build a lighthouse')
   fireEvent.click(screen.getByRole('button', { name: 'Send message' }))
 
   const tree = screen.getByRole('tree', { name: 'Sessions' })
@@ -299,7 +301,7 @@ it('keeps the published Workspace, Session, and unsent prompt after rejection',
   boot('?fixture=empty&fixturePrompt=reject')
 
   const composer = await screen.findByPlaceholderText('Describe what you want to build', {}, { timeout: 10_000 })
-  await setComposerText(composer, 'do not lose this')
+  setComposerText(composer, 'do not lose this')
   fireEvent.click(screen.getByRole('button', { name: 'Send message' }))
 
   const alert = await screen.findByRole('alert', {}, { timeout: 10_000 })

+ 8 - 1
packages/client/runtime/src/client/sessions/manager.ts

@@ -162,7 +162,14 @@ export class SessionManager {
    * @param text - exact controlled-input value for the active frontend Session.
    */
   updateIntent(text: string): void {
-    this.getIntent()?.updatePendingPrompt(text)
+    const session = this.getIntent()
+    if (session === undefined) return
+    session.updatePendingPrompt(text)
+    // The intent watch defers via markDirty, but the hero composer reads this
+    // prompt from the LIST snapshot as a controlled value: it must flush in
+    // the same tick as onChange (see Notifier.notifyNow) or React rolls the
+    // textarea back and IME composition breaks.
+    this.notifier.notifyNow()
   }
 
   private discardIntent(): void {

+ 28 - 0
packages/client/runtime/tests/session-intents.spec.ts

@@ -60,6 +60,34 @@ describe('frontend Session and Workspace intents', () => {
     expect(workspaces.list.getSnapshot().intent).toBeUndefined()
   })
 
+  it('echoes updateIntent into the list snapshot in the same tick (controlled-input contract)', async () => {
+    const api = new FakeApiClient()
+    const { sessions, workspaces } = services(api)
+    await ready(api, workspaces, sessions, [workspace('target')])
+    let notified = 0
+    sessions.list.subscribe(() => { notified += 1 })
+    // IME composition drives change events that a controlled textarea must see
+    // reflected before the handler returns; a microtask-deferred echo makes
+    // React roll the DOM back and the composition commits partial keystrokes.
+    sessions.updateIntent('你')
+    expect(sessions.list.getSnapshot().intent?.prompt).toBe('你')
+    expect(notified).toBeGreaterThan(0)
+  })
+
+  it('ignores updateIntent with no active Intent', async () => {
+    const api = new FakeApiClient()
+    const { sessions, workspaces } = services(api)
+    await ready(api, workspaces, sessions, [workspace('only', [sid('s-real')])], [
+      { sessionId: sid('s-real'), updatedAt: 1, running: false },
+    ])
+    sessions.open(sid('s-real'))
+    expect(sessions.list.getSnapshot().intent).toBeUndefined()
+    let notified = 0
+    sessions.list.subscribe(() => { notified += 1 })
+    sessions.updateIntent('dropped')
+    expect(notified).toBe(0)
+  })
+
   it('materializes zero-state Workspace and Session intents and retains a rejected first prompt', async () => {
     const api = new FakeApiClient()
     const { sessions, workspaces } = services(api)