fix: address feedback menu review and Linux process assertion

2026-09-18 17:30:19 +08:00
1 parent fa009c79e4
commit de3f4c24ea
@@ -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 .agents/notes/implemented/feature/2026-09-08-feedback-dialog-and-categories.md
-2026-09-08-feedback-dialog-and-categories.md: 271eb080dcca866ddbb5fee9df2d72ef9a1300ee
-2026-09-08-feedback-dialog-and-categories.zh.md: 10f662e7d020c283bc250ffc75d71f757ddd6516
+2026-09-08-feedback-dialog-and-categories.md: 1d544e6ef978c10008ac62768b6faaf9eaaf8b73
+2026-09-08-feedback-dialog-and-categories.zh.md: 6e58820bf308a3a9f7d15b64027f90e7e6771bd4
@@ -14,12 +14,14 @@ The Web client had two disconnected feedback paths with no visible outcome. `/fe

 `ui-message-feedback` becomes the Web feedback surface. A per-session `FeedbackSurface` owns the message-feedback controller, a `FeedbackDialogController` for the draft, the submission, and the toast sequence, and the routing between them: a message target puts its selected judgment with the dialog's category and note through the message controller, while the Session target records through `ctx.remote.sessionFeedback`. A `FeedbackDialog` entry of `conversation.input.overlay` renders the Modal and Toast primitives from the dialog store. A decoration on the Host's `feedback` command opens the dialog for the Session from a menu pick or a bare Enter while `/feedback <text>` still reaches the Host; it uses the `action` kind in `CommandUiSpec`, which consumes the trigger token and runs a client callback without submitting anything. The later [symmetric message feedback submission](2026-09-10-symmetric-message-feedback-submission.md) decision owns the rating entry rule: either unrecorded rating opens the dialog, while clicking the recorded rating retracts it. The note popover, `clearNote`, and `clear` remain absent because the dialog is the only note editor.

-The Session Header’s more-actions menu also opens the Session dialog through the `feedbackUi.openSession` Client service. The menu remains owned by `session-log-export`, while the existing per-session feedback controller owns the draft and submission. An observable tracks the optional feedback plugin’s availability, so unloading feedback removes only its menu row and leaves export usable. Opening or dismissing the form writes no feedback event; Submit records one Session remark.
+The Session Header's more-actions menu also opens the Session dialog through the `feedbackUi.openSession` Client service. The menu remains owned by `session-log-export`, while the existing per-session feedback controller owns the draft and submission. An observable tracks the optional feedback plugin's availability, so unloading feedback removes only its menu row and leaves export usable. Opening or dismissing the form writes no feedback event; Submit records one Session remark.

 The dialog is the shared Modal card at the design's width; the design's checkbox for including the conversation log is not built, because the log travels with every feedback event and is not optional. An oversized description still fails on submit with `note-too-large`; the dialog stays open with its draft and a warning toast presents the localized failure.

 ## Alternatives considered

+**A child slot for Header menu actions.** A slot would let each plugin own its menu row, but requires a shared menu-item API beyond the feedback entry requested in #4445. The optional `feedbackUi` service keeps this change local while leaving dialog state and submission in the feedback plugin.
+
 **Encode the category into the note text.** A prefix in free text is not filterable without parsing and would leak into the verbatim note that telemetry uploads; a durable id in the payload is what a consumer can group by.

 **Submit the dialog through the command plane as `/feedback <text>`.** The command rejects empty text, cannot carry a category, and writes an acknowledgement row the design replaces with a toast; the Remote records the same event with neither constraint.
@@ -20,6 +20,8 @@ Web 客户端有两条互不相连的反馈路径，且都没有可见结果。`

 ## 考虑过的替代方案

+**为标题栏菜单动作声明子 slot。** slot 可以让每个插件持有自己的菜单行，但需要共享菜单项 API，超出 #4445 所需反馈入口的范围。可选的 `feedbackUi` 服务将改动限制在局部，同时让弹窗状态与提交继续由反馈插件持有。
+
 **把分类编进备注文本。** 自由文本里的前缀不解析就无法过滤，还会混进遥测上传的原样备注；载荷里的持久 id 才是消费方能分组的东西。

 **让弹窗经命令平面以 `/feedback <text>` 提交。** 命令拒绝空文本、带不了分类，还会写一行设计稿已用 toast 取代的确认；Remote 记录同一个事件且没有这两个约束。
@@ -47,7 +47,20 @@ describe('packaging run records', () => {
        cwd: root, env: environment,
      })).rejects.toThrow('fatal failed')
      const descendant = JSON.parse(await readFile(join(run.directory, 'descendant.json'), 'utf8')) as { pid: number }
-      expect(() => process.kill(descendant.pid, 0)).toThrow()
+      // Linux can retain a dead descendant as a zombie until its new parent reaps it.
+      await expect.poll(async () => {
+        try {
+          if (process.platform === 'linux') {
+            const status = await readFile(`/proc/${descendant.pid}/status`, 'utf8')
+            return /^State:\s+[ZXx]\b/m.test(status)
+          }
+          process.kill(descendant.pid, 0)
+          return false
+        } catch (error) {
+          if (['ENOENT', 'ESRCH'].includes((error as NodeJS.ErrnoException).code ?? '')) return true
+          throw error
+        }
+      }, { timeout: 5_000 }).toBe(true)
      const events = (await readFile(join(run.directory, 'events.jsonl'), 'utf8')).trim().split('\n').map(line => JSON.parse(line) as { type: string; fatalObserved?: boolean; terminationError?: boolean })
      expect(events.at(-1)).toMatchObject({ type: 'stage-end', fatalObserved: true, terminationError: false })
      run.finish(false)
@@ -1,8 +1,6 @@
-// Keyless browser regression for durable per-message feedback. Cold-seeds a
-// settled two-turn transcript (zero model calls), records a Like through the
-// feedback dialog, replaces it through the same dialog with a Dislike, proves
-// the judgment survives a full page reload from the Host's canonical log, then
-// retracts it.
+// Keyless browser regression for durable message and Session feedback. A
+// cold-seeded, settled two-turn transcript avoids model calls while exercising
+// message ratings and the Session Header dialog against the Host's canonical log.
 import { readFile } from 'node:fs/promises'
 import { fileURLToPath } from 'node:url'
 import type { Browser, Page } from 'playwright'
@@ -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/README.md
-README.md: 3a1a54f801090456a1038d0e3d59fe5223374f3c
-README.zh.md: 231449ab574224a3e4fc5436c57e9ece5b0183b5
+README.md: 9966f2f84b1fe13f524c0cd717308586209c81fe
+README.zh.md: 2fa58031b2d5c3c2a0e995adc153ff3360e90bc8
@@ -76,7 +76,7 @@ The kernel packages boot and serve the page; the UI feature packages present it.
 | [`ui-plugin-manager/`](ui-plugin-manager/README.md) | Contributes the sidebar Plugins panel: install, enable, disable, retry, and compose installed packages | — |
 | [`ui-settings-plugin-inventory/`](ui-settings-plugin-inventory/README.md) | Contributes the read-only Host Loader inventory tab to Plugins settings | — |
 | [`ui-deliverables/`](ui-deliverables/README.md) | Produces the changed-files card with its comparison tabs, delivery cards, and clickable final-response file references | — |
-| [`ui-message-feedback/`](ui-message-feedback/README.md) | The feedback surface: per-message Like/Dislike in the assistant-message action strip, and the feedback dialog behind both ratings and `/feedback` | — |
+| [`ui-message-feedback/`](ui-message-feedback/README.md) | Provides message ratings and feedback dialogs opened from ratings, `/feedback`, or the Session Header menu | `ctx.feedbackUi` |
 | [`ui-directory-picker-browse/`](ui-directory-picker-browse/README.md) | In-app directory browsing surface for the workspace directory flow | — |
 | [`ui-directory-picker-native/`](ui-directory-picker-native/README.md) | Native directory-picker surface driving the local Desktop or Host OS chooser | — |
 | [`ui-open-in-app/`](ui-open-in-app/README.md) | Session-header split button opening the workspace directory in an installed application | — |
@@ -76,7 +76,7 @@ kind: "package-group"
 | [`ui-plugin-manager/`](ui-plugin-manager/README.zh.md) | 贡献侧栏的“插件”面板：安装、启用、停用、重试与组合已安装的包 | — |
 | [`ui-settings-plugin-inventory/`](ui-settings-plugin-inventory/README.zh.md) | 向「插件」设置贡献只读的 Host Loader 清单标签页 | — |
 | [`ui-deliverables/`](ui-deliverables/README.zh.md) | 生成改动文件卡片及其对比 tab、交付文件卡片与可点击的最终响应文件引用 | — |
-| [`ui-message-feedback/`](ui-message-feedback/README.zh.md) | 反馈界面：助手消息操作条中的逐消息赞踩，以及点赞、点踩与 `/feedback` 背后的反馈弹窗 | — |
+| [`ui-message-feedback/`](ui-message-feedback/README.zh.md) | 提供消息赞踩，以及从赞踩、`/feedback` 或会话标题栏菜单打开的反馈弹窗 | `ctx.feedbackUi` |
 | [`ui-directory-picker-browse/`](ui-directory-picker-browse/README.zh.md) | 面向工作区目录流程的应用内目录浏览界面 | — |
 | [`ui-directory-picker-native/`](ui-directory-picker-native/README.zh.md) | 驱动本地 Desktop 或 Host OS 选择器的原生目录选择界面 | — |
 | [`ui-open-in-app/`](ui-open-in-app/README.zh.md) | 在已安装应用中打开工作区目录的会话标题栏拆分按钮 | — |
@@ -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-message-feedback/README.md
-README.md: bc33b02c6966bfa851614ef616c1ff9c583ae67d
-README.zh.md: 88c1ba476c1a9c2a2df4b6cad471d78af65b83c9
+README.md: b6c6f227ebbeb3d6862d10658b1b125517b25581
+README.zh.md: a294c73c87e8e2da8139fe965e70d9eaf34096a6
@@ -25,7 +25,7 @@ This package is the Web GUI's feedback surface: the Like/Dislike pair in the fin
 <a id="use-this-package"></a>
 ## Use this package

-Mount this plugin alongside `ui-conversation` and `ui-commands`; the Like/Dislike pair then appears in the action row of each turn's closing assistant message, between copy and branch, and Feedback in either the composer menu or the Session Header’s more-actions menu opens the same dialog. A recorded rating shows the filled glyph and stays visible without hover. Like and Dislike both open the dialog: seven category chips and a detail box are optional, and Submit records the selected judgment with whatever was filled in before the toast thanks the user; the conversation log travels with every feedback event. Clicking the recorded rating retracts it without opening the dialog. A bare `/feedback`, picked from the menu or typed and sent without text, opens the same dialog for the Session; `/feedback <text>` keeps the Host command path and its acknowledgement row.
+Mount this plugin alongside `ui-conversation` and `ui-commands`; the Like/Dislike pair then appears in the action row of each turn's closing assistant message, between copy and branch, and Feedback in the composer menu opens the same dialog. Mounting `session-log-export` also adds Feedback to the Session Header's more-actions menu. A recorded rating shows the filled glyph and stays visible without hover. Like and Dislike both open the dialog: seven category chips and a detail box are optional, and Submit records the selected judgment with whatever was filled in before the toast thanks the user; the conversation log travels with every feedback event. Clicking the recorded rating retracts it without opening the dialog. A bare `/feedback`, picked from the menu or typed and sent without text, opens the same dialog for the Session; `/feedback <text>` keeps the Host command path and its acknowledgement row.

 ### Failures

@@ -90,4 +90,4 @@ None.

 </details>

-**Runtime invariant:** No companion is published. The plugin owns two slot registrations, one command decoration, and one per-session controller-pair map, all released by the plugin fiber's effect disposers. The lifecycle spec proves the registrations are withdrawn and every controller pair is dropped when the owning fiber is disposed, so no second authority exists to check at runtime.
+**Runtime invariant:** No companion is published. The plugin owns two slot registrations, one command decoration, the `feedbackUi` service, and one per-session controller-pair map, all released with the owning plugin fiber. The lifecycle spec verifies that the registrations and service are withdrawn and every controller pair is dropped when the fiber is disposed, so no second authority exists to check at runtime.
@@ -25,7 +25,7 @@ kind: "package-reference"
 <a id="use-this-package"></a>
 ## 使用本包

-与 `ui-conversation`、`ui-commands` 一起挂载本插件；Like/Dislike 对随即出现在每个轮次收尾助手消息的动作行中，位于复制与分支之间，输入框菜单或会话标题栏更多操作菜单里的「反馈」会打开同一个弹窗。已记录的评分显示实心图标，不需要悬停也一直可见。点赞和点踩都会打开弹窗：七个分类标签和一个详情框都可不填；提交后才会记录带所填内容的对应评分并弹出感谢 toast，对话日志随每个反馈事件一起投递。再次点击已记录的评分会直接撤回，不打开弹窗。不带文本的 `/feedback`，无论是从菜单选中还是直接输入后发送，都会为 Session 打开同一个弹窗；`/feedback <text>` 仍走宿主命令路径并显示确认行。
+与 `ui-conversation`、`ui-commands` 一起挂载本插件；Like/Dislike 对随即出现在每个轮次收尾助手消息的动作行中，位于复制与分支之间，输入框菜单里的「反馈」会打开同一个弹窗。挂载 `session-log-export` 后，会话标题栏的更多操作菜单也会增加「反馈」。已记录的评分显示实心图标，不需要悬停也一直可见。点赞和点踩都会打开弹窗：七个分类标签和一个详情框都可不填；提交后才会记录带所填内容的对应评分并弹出感谢 toast，对话日志随每个反馈事件一起投递。再次点击已记录的评分会直接撤回，不打开弹窗。不带文本的 `/feedback`，无论是从菜单选中还是直接输入后发送，都会为 Session 打开同一个弹窗；`/feedback <text>` 仍走宿主命令路径并显示确认行。

 ### 失败

@@ -90,4 +90,4 @@ kind: "package-reference"

 </details>

-**运行时不变式：** 不发布伴生入口。插件持有两个 slot 注册、一个命令装饰，以及一个按 Session 划分的控制器对 map；它们都由插件 fiber 的同一个 effect disposer 释放。生命周期规格测试证明，所属 fiber 释放时会撤销所有注册并丢弃所有控制器，因此不存在需要在运行时检查的第二权威来源。
+**运行时不变式：** 不发布伴生入口。插件持有两个 slot 注册、一个命令装饰、`feedbackUi` 服务，以及一个按 Session 划分的控制器对 map；它们都随所属插件 fiber 释放。生命周期规格测试验证，fiber 释放时会撤销所有注册和服务并丢弃所有控制器，因此不存在需要在运行时检查的第二权威来源。
@@ -3,7 +3,8 @@
 * conversation.chat.assistant-actions strip, the feedback dialog and its
 * acknowledgement and failure toasts in conversation.input.overlay, and the `/feedback`
 * decoration that opens the dialog from the composer menu or a bare typed
- * command. One FeedbackSurface per Session backs every entry in that Session.
+ * command. The feedbackUi service opens the same dialog for other plugins.
+ * One FeedbackSurface per Session backs every entry in that Session.
 * @module @deepseek-ai/dsh-client-ui-message-feedback/client
 */

@@ -60,7 +61,8 @@ export const inject = ['slots', 'remote', 'remote.messageFeedback', 'remote.sess

 /**
 * Client plugin body: the per-message feedback entry, the Session's dialog
- * entry, the `/feedback` decoration, and their per-session surfaces.
+ * entry, the `feedbackUi` service, the `/feedback` decoration, and their
+ * per-session surfaces.
 * @param ctx - client root context.
 */
 export function apply(ctx: ClientContext): void {
@@ -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/session-query/session-log-export/README.md
-README.md: 23388e10b9c64071a666e9842a669e37d6b9545d
+README.md: fb07e9bf00127ebea991a7bf10a8de8e35ad1210
 README.zh.md: 2af96f8621745fd6393f03905e4c6a978dbaf400
@@ -27,7 +27,7 @@ English | [中文](README.zh.md)

 Use this package when the Web bundle should let users export a session log. It requires Connection, the command registry, Session query and persistence, and attachments. Mount the plugin, then choose `Download session log` from the Session Header's more-actions menu or type `/export`; the browser downloads `dsh-session-<id>.zip`.

-When `ui-message-feedback` is mounted, the same menu also offers `Feedback`, which opens its existing Session feedback dialog. Opening or dismissing that form does not export the Session or submit feedback. The feedback row follows the feedback plugin’s availability; export remains available independently.
+When `ui-message-feedback` is mounted, the same menu also offers `Feedback`, which opens its existing Session feedback dialog. Opening or dismissing that form does not export the Session or submit feedback. The feedback row follows the feedback plugin's availability; export remains available independently.

 ### When to choose it

@@ -8,10 +8,13 @@ import { SessionLogDownloadDialog, type SessionLogDownloadDialogProps } from './
 import type { SessionLogDownloadDialogInjected } from './Dialog.tsx'
 import css from './HeaderAction.module.css'

-/** The optional feedback plugin controls whether the Header offers its form. */
+/** Session download controls with observable feedback availability and a Session feedback action. */
 export interface SessionLogDownloadHeaderInjected extends SessionLogDownloadDialogInjected {
  hooks: SessionLogDownloadDialogInjected['hooks'] & { feedbackAvailable: ObservableSnapshot<boolean> }
-  /** @param sessionId - Session whose existing feedback form to open. */
+  /**
+   * Open the existing Session feedback draft without recording feedback; no-op after the feedback plugin unloads.
+   * @param sessionId - Session whose feedback form to open.
+   */
  openFeedback: (sessionId: SessionId) => void
 }

@@ -56,6 +56,7 @@ export function apply(ctx: ClientContext): void {
      hooks: { sessionLogDownload: controller.store, feedbackAvailable },
      request: (sessionId: SessionId) => controller.download(sessionId),
      dismiss: (sessionId: SessionId) => { controller.dismiss(sessionId) },
+      // The feedback plugin can unload between the menu render and this click.
      openFeedback: (sessionId: SessionId) => { ctx.get('feedbackUi')?.openSession(sessionId) },
    }),
  }, SessionLogDownloadHeaderAction))
@@ -1,8 +1,9 @@
 // @vitest-environment jsdom
-import { cleanup, fireEvent, render, waitFor } from '@testing-library/react'
+import { act, cleanup, fireEvent, render, waitFor } from '@testing-library/react'
 import { afterEach, describe, expect, it, vi } from 'vitest'
 import { useSyncExternalStore } from 'react'
 import type { SessionId } from '@deepseek-ai/dsh-session/types'
+import { createSnapshotStore, type ObservableSnapshot } from '@deepseek-ai/dsh-client-store'
 import { SessionLogDownloadController } from '../src/client/controller.ts'
 import { SessionLogDownloadHeaderAction } from '../src/client/HeaderAction.tsx'
 import type { SessionLogDownloadHeaderProps } from '../src/client/HeaderAction.tsx'
@@ -10,11 +11,11 @@ import { en } from '../src/client/locales.ts'

 const SID = 'session-export-header' as SessionId

-function bindSessionExport(controller: SessionLogDownloadController) {
-  return function useSessionLogDownload<T>(selector: (state: ReturnType<typeof controller.store.getSnapshot>) => T): T {
+function bindSnapshot<State>(store: ObservableSnapshot<State>) {
+  return function useSnapshot<T>(selector: (state: State) => T): T {
    return useSyncExternalStore(
-      listener => controller.store.subscribe(listener),
-      () => selector(controller.store.getSnapshot()),
+      listener => store.subscribe(listener),
+      () => selector(store.getSnapshot()),
    )
  }
 }
@@ -24,18 +25,19 @@ function bench(feedbackAvailable = false) {
  const request = vi.fn((sessionId: SessionId) => controller.download(sessionId))
  const dismiss = vi.fn((sessionId: SessionId) => { controller.dismiss(sessionId) })
  const openFeedback = vi.fn()
-  const useSessionLogDownload = bindSessionExport(controller)
+  const feedback = createSnapshotStore(feedbackAvailable)
+  const useSessionLogDownload = bindSnapshot(controller.store)
  const props = {
    sessionId: SID,
    useSessionLogDownload,
-    useFeedbackAvailable: (select: (available: boolean) => unknown) => select(feedbackAvailable),
+    useFeedbackAvailable: bindSnapshot(feedback),
    openFeedback,
    request,
    dismiss,
    t: (key: keyof typeof en): string => en[key],
  } as unknown as SessionLogDownloadHeaderProps
  const view = render(<SessionLogDownloadHeaderAction {...props} />)
-  return { controller, request, openFeedback, view }
+  return { controller, request, openFeedback, feedback, view }
 }

 afterEach(cleanup)
@@ -57,6 +59,24 @@ describe('Session export Header action', () => {
    expect(b.view.getByRole('menuitem', { name: 'Download session log' })).toBeTruthy()
  })

+  it('updates the open menu when feedback becomes available, unloads, and reloads', () => {
+    const b = bench()
+    fireEvent.click(b.view.getByRole('button', { name: 'More actions' }))
+    expect(b.view.queryByRole('menuitem', { name: 'Feedback' })).toBeNull()
+
+    act(() => { b.feedback.set(true) })
+    expect(b.view.getByRole('menuitem', { name: 'Feedback' })).toBeTruthy()
+    act(() => { b.feedback.set(false) })
+    expect(b.view.queryByRole('menuitem', { name: 'Feedback' })).toBeNull()
+    expect(b.view.getByRole('menuitem', { name: 'Download session log' })).toBeTruthy()
+
+    act(() => { b.feedback.set(true) })
+    fireEvent.click(b.view.getByRole('menuitem', { name: 'Feedback' }))
+    expect(b.openFeedback).toHaveBeenCalledWith(SID)
+    expect(b.request).not.toHaveBeenCalled()
+    expect(b.view.queryByRole('menu')).toBeNull()
+  })
+
  it('opens the more-actions menu and downloads through the shared controller', async () => {
    const b = bench()
    const button = b.view.getByRole('button', { name: 'More actions' })
@@ -80,15 +100,15 @@ describe('Session export Header action', () => {
  })

  it('disables the download row while either entry path downloads this Session', async () => {
-    const b = bench()
+    const b = bench(true)
    let release!: (response: Response) => void
    const pending = new Promise<Response>((resolve) => { release = resolve })
    const controller = new SessionLogDownloadController(() => pending, vi.fn())
-    const useSessionLogDownload = bindSessionExport(controller)
+    const useSessionLogDownload = bindSnapshot(controller.store)
    b.view.rerender(<SessionLogDownloadHeaderAction {...({
      sessionId: SID,
      useSessionLogDownload,
-      useFeedbackAvailable: (select: (available: boolean) => unknown) => select(true),
+      useFeedbackAvailable: bindSnapshot(b.feedback),
      openFeedback: b.openFeedback,
      request: (sessionId: SessionId) => controller.download(sessionId),
      dismiss: (sessionId: SessionId) => { controller.dismiss(sessionId) },