From a39d3091875727c83cc50db656ddc775b259c552 Mon Sep 17 00:00:00 2001 From: Huanqi Cao Date: Mon, 7 Sep 2026 21:02:29 +0800 Subject: [PATCH 001/164] fix(directory-picker-native): foreground-activate the Win32 folder dialog via a synthesized Alt press --- ...-win32-picker-foreground-alt-key.i18n.yaml | 6 +++++ ...6-09-07-win32-picker-foreground-alt-key.md | 25 +++++++++++++++++++ ...9-07-win32-picker-foreground-alt-key.zh.md | 25 +++++++++++++++++++ .../directory-picker-native/README.i18n.yaml | 4 +-- .../host/directory-picker-native/README.md | 3 ++- .../host/directory-picker-native/README.zh.md | 3 ++- .../host/directory-picker-native/src/index.ts | 4 ++- .../src/win32-dialog-bindings.ts | 9 +++++++ .../src/win32-dialog-host.ts | 7 +++--- .../src/win32-dialog-logic.ts | 15 +++++++++++ .../src/win32-dialog-worker.ts | 9 ++++--- .../tests/win32-dialog-bindings.spec.ts | 21 +++++++++++++++- .../tests/win32-dialog-logic.spec.ts | 14 ++++++++--- 13 files changed, 130 insertions(+), 15 deletions(-) create mode 100644 .agents/notes/implemented/bug-fix/2026-09-07-win32-picker-foreground-alt-key.i18n.yaml create mode 100644 .agents/notes/implemented/bug-fix/2026-09-07-win32-picker-foreground-alt-key.md create mode 100644 .agents/notes/implemented/bug-fix/2026-09-07-win32-picker-foreground-alt-key.zh.md diff --git a/.agents/notes/implemented/bug-fix/2026-09-07-win32-picker-foreground-alt-key.i18n.yaml b/.agents/notes/implemented/bug-fix/2026-09-07-win32-picker-foreground-alt-key.i18n.yaml new file mode 100644 index 0000000000..0a54d83ab9 --- /dev/null +++ b/.agents/notes/implemented/bug-fix/2026-09-07-win32-picker-foreground-alt-key.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/bug-fix/2026-09-07-win32-picker-foreground-alt-key.md +2026-09-07-win32-picker-foreground-alt-key.md: cd1bae56845c74bf8fb0b3c0e6b8e6bab8032daf +2026-09-07-win32-picker-foreground-alt-key.zh.md: 8cbacaee97892b2ba6feb3d0250212fa9d393376 diff --git a/.agents/notes/implemented/bug-fix/2026-09-07-win32-picker-foreground-alt-key.md b/.agents/notes/implemented/bug-fix/2026-09-07-win32-picker-foreground-alt-key.md new file mode 100644 index 0000000000..cd1bae5684 --- /dev/null +++ b/.agents/notes/implemented/bug-fix/2026-09-07-win32-picker-foreground-alt-key.md @@ -0,0 +1,25 @@ +# Agent Note: Foreground activation for the Win32 picker via a synthesized Alt press + +Status: implemented + +English | [中文](2026-09-07-win32-picker-foreground-alt-key.zh.md) + +## Problem + +The web GUI host picks a workspace directory through the native Win32 folder dialog, which runs in a child process the host spawns (issue #3543). Windows grants the foreground only to the foreground process, to a process it started, or to a process that received recent input; a child of a background server process qualifies for none of these, so the dialog that `Show` opens sits behind every visible window even though it is the child's first window. The first-window activation assumption behind the spawn design ([archived feature note](../../archived/feature/2026-08-02-win32-in-process-folder-dialog.md)) holds only when the spawner chain owns the console foreground, as in a console-launched CLI. + +## Decision + +`runFolderDialog` calls a new `pressAltForForeground` binding between the `showing` notice and the blocking `Show`. The binding synthesizes one Alt press (`keybd_event` with `VK_MENU`, down then up) on the dialog thread, which makes Windows count this process as the most recent input owner — one of the documented grounds for foreground activation — so the dialog window `Show` creates activates as foreground. The bindings module already loads koffi's `user32`, so the change adds one function fetch and two invocations. The press is unconditional on Windows. When the process already holds foreground rights (a console-launched CLI), the dialog activates anyway and the press is inert; the window focused at that moment still receives the lone Alt and may briefly highlight its menu bar. Environments that suppress injected input (secure desktops, restricted remote sessions, an elevated foreground window) leave the dialog behind other windows, and the package README records that limit. + +## Alternatives considered + +**Custom URL protocol with a browser click gesture.** Draft PR #3544 granted the foreground by navigating the foreground browser to a registered `dsh-picker://` URL, which makes the shell launch the dialog process as a foreground descendant. The grant is deterministic by design, but the mechanism spans registry and VBS launcher files, a protocol entry point, a picker-result HTTP route with per-boot tokens, and a first-use browser confirmation, and it adds a server route the browser can reach. The synthesized press removes that entire surface. + +**AllowSetForegroundWindow from the clicker.** The API must be called by the current foreground process — the browser — and may name only one permitted process; the spawner cannot invoke it on the browser's behalf. + +**AttachThreadInput to the focused thread.** Attaching the dialog thread to the focused window's thread also bypasses the foreground restriction and avoids the keystroke side effect, but it is equally undocumented, needs the focused window's thread id at show time, and fails when the focused window belongs to a higher-integrity process; it was not prototyped. + +## Consequences + +The picker keeps its single spawned-child design and gains foreground behavior in the background-host case at the cost of one koffi call pair. The bindings spec pins the Alt down/up sequence and its position immediately before `Show` over the fake COM world; the logic spec pins the full showing → press → `Show` order. The Windows CI lane still opens and abort-closes a real dialog with the press present but asserts no activation. Validation on a Windows 11 machine with the foreground lock forced to its maximum reproduced the failure without the press (dialog behind other windows) and the foreground dialog with it in five of five repeat runs; Windows 10 is unverified. Synthesized input is consumed asynchronously by the raw input thread, so the activation grant is in principle race-prone; no miss appeared across the repeat runs, and fragile environments stay a documented package limitation rather than a second mechanism, because the browse backend remains the composition-level answer where native picking cannot be trusted. diff --git a/.agents/notes/implemented/bug-fix/2026-09-07-win32-picker-foreground-alt-key.zh.md b/.agents/notes/implemented/bug-fix/2026-09-07-win32-picker-foreground-alt-key.zh.md new file mode 100644 index 0000000000..8cbacaee97 --- /dev/null +++ b/.agents/notes/implemented/bug-fix/2026-09-07-win32-picker-foreground-alt-key.zh.md @@ -0,0 +1,25 @@ +# Agent Note: 通过合成的 Alt 按键让 Win32 选择器获得前台激活 + +Status: implemented + +[English](2026-09-07-win32-picker-foreground-alt-key.md) | 中文 + +## Problem + +web GUI 宿主通过原生 Win32 文件夹对话框选择工作区目录,对话框运行在宿主 spawn 的子进程中(issue #3543)。Windows 只把前台授予前台进程、由前台进程启动的进程或最近收到输入的进程;后台服务器进程的子进程三者都不满足,因此 `Show` 打开的对话框即使属于子进程的首个窗口,也会落在所有可见窗口之后。spawn 设计背后的"首窗口即激活"假设([归档功能记录](../../archived/feature/2026-08-02-win32-in-process-folder-dialog.md))只在 spawn 链持有控制台前台时成立,例如控制台启动的 CLI。 + +## Decision + +`runFolderDialog` 在 `showing` 通知与阻塞式 `Show` 之间调用新增的 `pressAltForForeground` 绑定。该绑定在对话框线程上合成一次 Alt 按键(`keybd_event` 携带 `VK_MENU`,先按下后抬起),使 Windows 把该进程计为最近的输入所有者——文档记载的允许前台激活的理由之一——于是 `Show` 创建的对话框窗口以前台方式激活。bindings 模块已经加载 koffi 的 `user32`,因此改动只增加一次函数获取与两次调用。该按键在 Windows 上无条件执行。当进程已经持有前台权利(控制台启动的 CLI)时,对话框本来就会激活,按键不起作用;此刻获得焦点的窗口仍会收到这一次单独的 Alt,可能短暂高亮其菜单栏。在合成输入被抑制的环境(安全桌面、受限远程会话、提权前台窗口)中,对话框仍会落在其他窗口后面,包 README 记录了该限制。 + +## Alternatives considered + +**自定义 URL 协议加浏览器点击手势。** 草稿 PR #3544 通过让前台浏览器导航到已注册的 `dsh-picker://` URL 来授予前台,使 shell 把对话框进程作为前台进程的后代启动。该授权按设计具有确定性,但机制横跨注册表与 VBS 启动器文件、协议入口点、携带每次启动令牌的 picker-result HTTP 路由,以及首次使用时的浏览器确认,还增加了一条浏览器可达的服务端路由。合成按键删除了整个这一面。 + +**由点击方调用 AllowSetForegroundWindow。** 该 API 必须由当前前台进程——浏览器——调用,并且只能点名一个被允许的进程;spawner 无法代替浏览器调用它。 + +**对聚焦线程 AttachThreadInput。** 把对话框线程附着到焦点窗口所属线程同样能绕过前台限制且没有按键副作用,但同样没有文档契约,需要在显示时取得焦点窗口的线程 id,并在焦点窗口属于更高完整性进程时失败;未做原型验证。 + +## Consequences + +选择器保持单一 spawn 子进程设计,并在后台宿主场景获得前台行为,代价是一次 koffi 调用对。bindings spec 在假 COM 世界上固定了 Alt 按下/抬起序列及其紧邻 `Show` 之前的位置;logic spec 固定完整的 showing → press → `Show` 顺序。Windows CI lane 仍会真实打开并中止关闭一个对话框(按键存在),但不断言激活。在一台把前台锁强制到最大值的 Windows 11 机器上做了验证:不加按键复现失败(对话框在其他窗口后面),加上按键后对话框获得前台,五轮重复全部成功;Windows 10 尚未验证。合成输入由 raw input thread 异步消费,因此激活授权原则上存在竞争窗口;重复运行中未出现错过,脆弱环境仍是记录的包限制而非第二套机制,因为浏览后端仍是原生选择不可信场景在组合层面的答案。 diff --git a/packages/host/directory-picker-native/README.i18n.yaml b/packages/host/directory-picker-native/README.i18n.yaml index 1a337e6e05..cb2629b029 100644 --- a/packages/host/directory-picker-native/README.i18n.yaml +++ b/packages/host/directory-picker-native/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/host/directory-picker-native/README.md -README.md: a432bdc19175c690287745cc9c2df004d03b7478 -README.zh.md: 98ea9a033dbf6720fa08fbb068e243371998dab6 +README.md: 3f980831848965b976e32b0e5ef25abb77fe4e6e +README.zh.md: 66f77a0b715aee8b4087dd03d4d4eea69d39bb09 diff --git a/packages/host/directory-picker-native/README.md b/packages/host/directory-picker-native/README.md index a432bdc191..3f98083184 100644 --- a/packages/host/directory-picker-native/README.md +++ b/packages/host/directory-picker-native/README.md @@ -53,7 +53,7 @@ The backend is a thin service over a platform chooser: `NativeDirectoryPicker` r ### Platform mechanics -Platform tools run without a shell: `osascript` on macOS, and Zenity with a KDialog fallback on Linux; the caller's abort terminates the native process. Windows opens the modern `IFileOpenDialog` in a spawned child process — a koffi-driven COM conversation on the child's main thread with the best thread DPI awareness the host accepts (per-monitor-v2 first), aborted by posting `WM_CLOSE` to the dialog thread. +Platform tools run without a shell: `osascript` on macOS, and Zenity with a KDialog fallback on Linux; the caller's abort terminates the native process. Windows opens the modern `IFileOpenDialog` in a spawned child process — a koffi-driven COM conversation on the child's main thread with the best thread DPI awareness the host accepts (per-monitor-v2 first), aborted by posting `WM_CLOSE` to the dialog thread. Immediately before `Show`, the child synthesizes one Alt press through `keybd_event`, which lets the dialog activate as the foreground window even when a background host process spawned the child. ### Source map @@ -98,6 +98,7 @@ These limits define when the native interaction is unavailable or fragile. They - **Linux requires desktop tooling** — with neither Zenity nor KDialog installed, `pick` rejects with an actionable error; it does not fall back to a typed-path prompt (the browse backend is that fallback at the composition level). - **Windows has no mechanism fallback** — the child-process picker through packaged koffi is the only native tier, so a COM refusal or dialog crash surfaces the failure; the browse backend remains the fallback at the composition level. +- **Windows foreground grant relies on injected input** — the child synthesizes an Alt press before `Show` so the dialog can take the foreground from a background host; where synthesized input is suppressed (secure desktops, restricted remote sessions, an elevated foreground window), the dialog may still open behind other windows. The technique is validated on Windows 11 only. ### Dev Note diff --git a/packages/host/directory-picker-native/README.zh.md b/packages/host/directory-picker-native/README.zh.md index 98ea9a033d..66f77a0b71 100644 --- a/packages/host/directory-picker-native/README.zh.md +++ b/packages/host/directory-picker-native/README.zh.md @@ -53,7 +53,7 @@ kind: "package-reference" ### 平台机制 -平台工具不经 shell 调用:macOS 使用 `osascript`,Linux 使用 Zenity 并以 KDialog 回退;调用方的中止信号会终止原生进程。Windows 在 spawn 的子进程中打开现代 `IFileOpenDialog`——由 koffi 在子进程主线程上驱动的 COM 会话,采用宿主接受的最佳线程 DPI 感知(优先 per-monitor-v2),中止时向对话框线程投递 `WM_CLOSE`。 +平台工具不经 shell 调用:macOS 使用 `osascript`,Linux 使用 Zenity 并以 KDialog 回退;调用方的中止信号会终止原生进程。Windows 在 spawn 的子进程中打开现代 `IFileOpenDialog`——由 koffi 在子进程主线程上驱动的 COM 会话,采用宿主接受的最佳线程 DPI 感知(优先 per-monitor-v2),中止时向对话框线程投递 `WM_CLOSE`。在 `Show` 之前,子进程立即通过 `keybd_event` 合成一次 Alt 按键,让对话框即使由后台宿主进程 spawn 也能激活为前台窗口。 ### 源码地图 @@ -98,6 +98,7 @@ kind: "package-reference" - **Linux 依赖桌面工具**——Zenity 与 KDialog 均未安装时,`pick` 以包含解决建议的错误拒绝;它不会回退为手输路径提示(组合层面的回退是浏览后端)。 - **Windows 没有机制级回退**——通过打包依赖 koffi 运行的子进程选择器是唯一原生层级,因此 COM 拒绝或对话框崩溃会直接上报失败;组合层面的回退仍是浏览后端。 +- **Windows 前台授权依赖注入的输入**——子进程在 `Show` 之前合成一次 Alt 按键,对话框才能从后台宿主取得前台;在合成输入被抑制的环境(安全桌面、受限远程会话、提权前台窗口)中,对话框仍可能在其他窗口后面打开。该技术仅在 Windows 11 上验证过。 ### 开发备注 diff --git a/packages/host/directory-picker-native/src/index.ts b/packages/host/directory-picker-native/src/index.ts index 3a7e3bd05f..2fa0489ca3 100644 --- a/packages/host/directory-picker-native/src/index.ts +++ b/packages/host/directory-picker-native/src/index.ts @@ -3,7 +3,9 @@ * with the `native` capability, opening one native OS chooser on the host * display per pick (macOS `osascript`, Linux Zenity with a KDialog fallback; * Windows opens the modern `IFileOpenDialog` in a spawned child process — a - * koffi-driven COM conversation on the child's main thread). Only viable when + * koffi-driven COM conversation on the child's main thread, preceded by a + * synthesized Alt press so the dialog activates as foreground even when a + * background host spawned the child). Only viable when * the operator sits at the host's screen; remote deployments compose the * browse backend instead. * @module @deepseek-ai/dsh-host-directory-picker-native diff --git a/packages/host/directory-picker-native/src/win32-dialog-bindings.ts b/packages/host/directory-picker-native/src/win32-dialog-bindings.ts index 53ce525280..67abdf5859 100644 --- a/packages/host/directory-picker-native/src/win32-dialog-bindings.ts +++ b/packages/host/directory-picker-native/src/win32-dialog-bindings.ts @@ -55,6 +55,10 @@ const SIGDN_FILESYSPATH = 0x80058000 | 0 */ const DPI_AWARENESS_CONTEXTS = [-4, -3, -2] const WM_CLOSE = 0x10 +/** `VK_MENU`: the synthesized Alt press's virtual key. */ +const VK_MENU = 0x12 +/** `KEYEVENTF_KEYUP`: the synthesized Alt press's release flag. */ +const KEYEVENTF_KEYUP = 0x2 /** IFileOpenDialog vtable slots (IUnknown 0-2, IModalWindow 3, IFileDialog 4+). */ const SLOT_RELEASE = 2 @@ -101,6 +105,7 @@ export async function loadWin32DialogBindings(): Promise { const coCreateInstance = ole32.func('__stdcall', 'CoCreateInstance', 'int32', ['void *', 'void *', 'uint32', 'void *', 'void *']) const coTaskMemFree = ole32.func('__stdcall', 'CoTaskMemFree', 'void', ['void *']) const getCurrentThreadId = kernel32.func('__stdcall', 'GetCurrentThreadId', 'uint32', []) + const keybdEvent = user32.func('__stdcall', 'keybd_event', 'void', ['uint8', 'uint8', 'uint32', 'uintptr']) const protoShow = koffi.proto('int32 __stdcall DshDialogShow(void *self, void *owner)') const protoSetOptions = koffi.proto('int32 __stdcall DshDialogSetOptions(void *self, uint32 options)') @@ -140,6 +145,10 @@ export async function loadWin32DialogBindings(): Promise { coUninitialize() }, currentThreadId: () => getCurrentThreadId() as number, + pressAltForForeground: () => { + keybdEvent(VK_MENU, 0, 0, 0) + keybdEvent(VK_MENU, 0, KEYEVENTF_KEYUP, 0) + }, createFolderDialog: (): Win32FolderDialog => { const out = Buffer.alloc(pointerSize) const created = coCreateInstance(CLSID_FILE_OPEN_DIALOG, null, CLSCTX_INPROC_SERVER, IID_IFILE_OPEN_DIALOG, out) as number diff --git a/packages/host/directory-picker-native/src/win32-dialog-host.ts b/packages/host/directory-picker-native/src/win32-dialog-host.ts index 7a60ab05ed..658ccd8d56 100644 --- a/packages/host/directory-picker-native/src/win32-dialog-host.ts +++ b/packages/host/directory-picker-native/src/win32-dialog-host.ts @@ -14,9 +14,10 @@ import type { Win32DialogWorkerData } from './win32-dialog-worker.ts' /** * Spawn the dialog child process. Built consumers launch the bundled CJS * entry next to this module under plain node; unbuilt (source) consumers - * bootstrap tsx first, mirroring the dsh CLI's source launch. The dialog is - * the child's first window, so Windows activates it without a foreground - * call. + * bootstrap tsx first, mirroring the dsh CLI's source launch. The child + * opens its dialog as foreground on its own: `runFolderDialog` synthesizes + * an Alt press before `Show`, which matters when a background host spawned + * the child. * @param data - the child payload (dialog title). * @returns the spawned child process. */ diff --git a/packages/host/directory-picker-native/src/win32-dialog-logic.ts b/packages/host/directory-picker-native/src/win32-dialog-logic.ts index 9ce2b42464..e6752b5e08 100644 --- a/packages/host/directory-picker-native/src/win32-dialog-logic.ts +++ b/packages/host/directory-picker-native/src/win32-dialog-logic.ts @@ -79,6 +79,20 @@ export interface Win32DialogBindings { * @returns the calling thread's native id. */ currentThreadId(): number + /** + * Make the dialog that `Show` is about to create able to take the + * foreground. Windows grants activation only to the foreground process, + * to a process it started, or to a process that received recent input; a + * worker spawned by a background host (the web GUI server) qualifies for + * none, so the dialog would open behind every other window. Synthesizing + * one Alt press (down, then up) through `keybd_event` counts this process + * as the most recent input owner — a community-documented technique with + * no documented contract. Call immediately before `Show`. When the + * process already holds foreground rights (a console-launched CLI), the + * press is inert, but the focused window still receives the lone Alt and + * may briefly highlight its menu bar before the dialog activates. + */ + pressAltForForeground(): void } /** @@ -117,6 +131,7 @@ export function runFolderDialog( check(dialog.setOptions(FOS_PICKFOLDERS | FOS_FORCEFILESYSTEM | FOS_NOCHANGEDIR), 'SetOptions') check(dialog.setTitle(title), 'SetTitle') onShowing(bindings.currentThreadId()) + bindings.pressAltForForeground() const shown = dialog.show() if (shown === HRESULT_CANCELLED) return null check(shown, 'Show') diff --git a/packages/host/directory-picker-native/src/win32-dialog-worker.ts b/packages/host/directory-picker-native/src/win32-dialog-worker.ts index 0b422b3ca7..89589a8197 100644 --- a/packages/host/directory-picker-native/src/win32-dialog-worker.ts +++ b/packages/host/directory-picker-native/src/win32-dialog-worker.ts @@ -1,9 +1,12 @@ /** * Child-process entry for the Win32 folder dialog: blocks THIS process * inside the modal `Show` so the host event loop stays live, reporting over - * the IPC channel. Spawned as a child process (not a worker thread) so the - * dialog is the process's first window and Windows activates it without a - * manual foreground call. Protocol: `{kind:'showing',threadId}` right + * the IPC channel. Spawned as a child process (not a worker thread) so a + * native fault stays contained and the modal call never wedges the host. + * A background host (the web GUI server) leaves this process without + * foreground rights, so `runFolderDialog` synthesizes an Alt press + * immediately before `Show` and the dialog then activates as foreground. + * Protocol: `{kind:'showing',threadId}` right * before the blocking call (the driver's abort lever needs the native * thread id), then exactly one of `{kind:'done',path}` or * `{kind:'error',message}`. diff --git a/packages/host/directory-picker-native/tests/win32-dialog-bindings.spec.ts b/packages/host/directory-picker-native/tests/win32-dialog-bindings.spec.ts index ad7ba3d9ea..360c7d3d8b 100644 --- a/packages/host/directory-picker-native/tests/win32-dialog-bindings.spec.ts +++ b/packages/host/directory-picker-native/tests/win32-dialog-bindings.spec.ts @@ -38,6 +38,10 @@ interface ComWorld { registered: number unregistered: number uninitialized: number + /** The synthesized keybd_event calls, in order. */ + keyEvents: { vk: number; flags: number }[] + /** Cross-cutting call trace shared by keybd_event and the dialog Show slot. */ + nativeOrder: string[] } function comWorld(overrides: Partial = {}): ComWorld { @@ -48,6 +52,7 @@ function comWorld(overrides: Partial = {}): ComWorld { titles: [], options: [], dpiContexts: [], freed: [], released: [], posted: [], str16PointerSizes: [], registered: 0, unregistered: 0, uninitialized: 0, + keyEvents: [], nativeOrder: [], ...overrides, } } @@ -77,7 +82,7 @@ function installFakeKoffi(world: ComWorld, options: { switch (slot) { case 9: world.options.push(args[0] as number); return 0 case 17: world.titles.push(args[0] as string); return 0 - case 3: return world.showHr + case 3: world.nativeOrder.push('show'); return world.showHr case 20: { if (world.getResultHr < 0) return world.getResultHr ;(args[0] as unknown[])[0] = itemPtr @@ -116,6 +121,10 @@ function installFakeKoffi(world: ComWorld, options: { } case 'CoTaskMemFree': return (ptr: unknown) => { world.freed.push(ptr) } case 'GetCurrentThreadId': return () => 31337 + case 'keybd_event': return (vk: number, _scan: number, flags: number, _extra: unknown) => { + world.keyEvents.push({ vk, flags }) + world.nativeOrder.push(flags === 0 ? 'alt-down' : 'alt-up') + } case 'SetThreadDpiAwarenessContext': { if (!world.hasThreadDpi) throw new Error(`${dll}: SetThreadDpiAwarenessContext not found`) return (context: unknown) => { @@ -188,6 +197,14 @@ describe('loadWin32DialogBindings over the fake COM world', () => { expect(world.titles).toEqual(['选择工作区目录']) expect(world.options).toHaveLength(1) expect(showing).toHaveBeenCalledWith(31337) + // One synthesized Alt press (down, then up) immediately precedes Show, so + // the dialog's activation attempt finds this process as the recent-input + // owner. + expect(world.keyEvents).toEqual([ + { vk: 0x12, flags: 0 }, + { vk: 0x12, flags: 2 }, + ]) + expect(world.nativeOrder.slice(-3)).toEqual(['alt-down', 'alt-up', 'show']) expect(world.freed).toHaveLength(1) expect(world.str16PointerSizes).toEqual([FAKE_POINTER_SIZE]) expect(world.released).toEqual(['item', 'dialog']) @@ -244,6 +261,7 @@ describe('loadWin32DialogBindings over the fake COM world', () => { const { loadWin32DialogBindings } = await loadBindingsModule() const bindings = await loadWin32DialogBindings() expect(runFolderDialog(bindings, 'Pick', vi.fn())).toBeNull() + expect(world.keyEvents).toHaveLength(2) expect(world.released).toEqual(['dialog']) expect(world.uninitialized).toBe(1) }) @@ -357,6 +375,7 @@ describe('the worker entry over a mocked process boundary', () => { coInitializeSta: () => 0, coUninitialize: () => undefined, currentThreadId: () => 11, + pressAltForForeground: () => undefined, createFolderDialog: () => ({ setOptions: () => 0, setTitle: () => 0, diff --git a/packages/host/directory-picker-native/tests/win32-dialog-logic.spec.ts b/packages/host/directory-picker-native/tests/win32-dialog-logic.spec.ts index 718c93c2c0..085d8a0b38 100644 --- a/packages/host/directory-picker-native/tests/win32-dialog-logic.spec.ts +++ b/packages/host/directory-picker-native/tests/win32-dialog-logic.spec.ts @@ -15,6 +15,7 @@ const E_FAIL = 0x80004005 | 0 interface FakeWorld { bindings: Win32DialogBindings dpi: ReturnType + pressAlt: ReturnType createDialog: ReturnType uninitialize: ReturnType dialog: { @@ -36,6 +37,7 @@ function world(overrides: Partial = {}, coInit = 0): FakeWorl ...overrides, } const dpi = vi.fn() + const pressAlt = vi.fn() const createDialog = vi.fn(() => dialog) const uninitialize = vi.fn() const bindings: Win32DialogBindings = { @@ -44,13 +46,14 @@ function world(overrides: Partial = {}, coInit = 0): FakeWorl coUninitialize: uninitialize, createFolderDialog: createDialog, currentThreadId: vi.fn(() => 4242), + pressAltForForeground: pressAlt, } - return { bindings, dpi, createDialog, uninitialize, dialog: dialog as FakeWorld['dialog'] } + return { bindings, dpi, pressAlt, createDialog, uninitialize, dialog: dialog as FakeWorld['dialog'] } } describe('runFolderDialog', () => { it('sequences DPI, STA, options, title, show, result extraction, and apartment teardown', () => { - const { bindings, dpi, dialog, uninitialize } = world() + const { bindings, dpi, pressAlt, dialog, uninitialize } = world() const showing = vi.fn() expect(runFolderDialog(bindings, 'Pick', showing)).toBe('C:\\picked\\目录') expect(dpi).toHaveBeenCalledOnce() @@ -60,12 +63,17 @@ describe('runFolderDialog', () => { expect(dialog.setTitle).toHaveBeenCalledWith('Pick') expect(showing).toHaveBeenCalledWith(4242) expect(showing.mock.invocationCallOrder[0]).toBeLessThan(dialog.show.mock.invocationCallOrder[0] as number) + // The foreground press sits between the showing notice and the blocking Show. + expect(pressAlt).toHaveBeenCalledOnce() + expect(showing.mock.invocationCallOrder[0]).toBeLessThan(pressAlt.mock.invocationCallOrder[0] as number) + expect(pressAlt.mock.invocationCallOrder[0]).toBeLessThan(dialog.show.mock.invocationCallOrder[0] as number) expect(dialog.release).toHaveBeenCalledOnce() }) it('maps the cancelled HRESULT to null and still releases the dialog and apartment', () => { - const { bindings, dialog, uninitialize } = world({ show: vi.fn(() => HRESULT_CANCELLED) }) + const { bindings, pressAlt, dialog, uninitialize } = world({ show: vi.fn(() => HRESULT_CANCELLED) }) expect(runFolderDialog(bindings, 'Pick', vi.fn())).toBeNull() + expect(pressAlt).toHaveBeenCalledOnce() expect(dialog.resultPath).not.toHaveBeenCalled() expect(dialog.release).toHaveBeenCalledOnce() expect(uninitialize).toHaveBeenCalledOnce() From 4550bb6e69e018770061b5ae45026f0bac5feea5 Mon Sep 17 00:00:00 2001 From: Turtle Date: Tue, 8 Sep 2026 19:08:00 +0800 Subject: [PATCH 002/164] feat: enforce weighted pull request approvals --- .github/review-ownership/README.md | 23 +- .github/review-ownership/approval-policy.json | 12 + .github/review-ownership/check-approval.mjs | 298 ++++++++++++++++++ .../review-ownership/check-approval.test.mjs | 244 ++++++++++++++ .github/workflows/weighted-approval.yml | 35 ++ package.json | 1 + scripts/ci-workflow.spec.ts | 45 +++ scripts/run-gates.ts | 2 + 8 files changed, 655 insertions(+), 5 deletions(-) create mode 100644 .github/review-ownership/approval-policy.json create mode 100644 .github/review-ownership/check-approval.mjs create mode 100644 .github/review-ownership/check-approval.test.mjs create mode 100644 .github/workflows/weighted-approval.yml diff --git a/.github/review-ownership/README.md b/.github/review-ownership/README.md index 59eed9dca1..ab5ba83903 100644 --- a/.github/review-ownership/README.md +++ b/.github/review-ownership/README.md @@ -1,12 +1,13 @@ -# Automated review requests +# Automated pull-request reviews ## Summary -The [`request-review` workflow](../workflows/request-review.yml) reads the CODEOWNERS-compatible [ownership map](CODEOWNERS) from the trusted default branch. It classifies changed files, requests missing owners for reviewable code, and cancels its outstanding requests when a pull request becomes a draft. The ownership map is outside GitHub's native CODEOWNERS locations, so GitHub does not apply it directly. +The [`request-review` workflow](../workflows/request-review.yml) requests owners for reviewable code. The [`weighted-approval` workflow](../workflows/weighted-approval.yml) publishes an approval score for branch rules. Both workflows execute policy from the trusted default branch. ## Table of Contents - [Routing](#routing) +- [Approval scoring](#approval-scoring) - [Review exclusions](#review-exclusions) - [Security](#security) - [Verification](#verification) @@ -26,6 +27,18 @@ The ownership map accepts explicit absolute directory patterns and one or two in The policy test measures non-test tracked lines under matched directories and requires `@turtle1999` to own no more than one third of that eligible owned codebase. + + +## Approval scoring + +The weighted approval workflow publishes the `weighted approval` commit status on the pull request head. Branch rules must require this status context. The status succeeds at two approval points, remains pending below two points or while the pull request is a draft, fails while a write-capable reviewer has an effective `CHANGES_REQUESTED` review, and reports an error when policy evaluation fails. + +Reviewers whose calculated base repository permission is `write` or `admin` count. The [approval policy](approval-policy.json) gives `@07akioni`, `@imccyu`, `@tianyicui`, `@tianyicui-bot`, `@turtle1999`, and `@turtle2099` two points each; every other write-capable reviewer gets one point. The pull-request author and reviewers without write permission do not count. + +Each reviewer contributes only the latest non-comment `APPROVED` or `CHANGES_REQUESTED` decision that GitHub returns. Comment-only and dismissed review records do not replace an effective decision. The workflow does not invalidate an approval by its review commit; the repository's native pull-request rules own stale-review and latest-push requirements. + +The workflow runs when a pull request opens, synchronizes, reopens, becomes ready, or becomes a draft, and when a review is submitted, edited, or dismissed. It fetches every review and resolves current repository permission before publishing the status. Permission changes take effect on the next subscribed pull-request or review event. + ## Review exclusions @@ -42,15 +55,15 @@ For a modified file with a supported source extension, the scanner compares the ## Security -The write-capable `pull_request_target` job checks out only the repository default branch. It does not check out or execute pull-request code and does not use repository secrets. Pull-request filenames are treated as API data and escaped in logs. +The write-capable jobs check out only the repository default branch. They do not check out or execute pull-request code and do not use repository secrets. Pull-request filenames and reviews are treated as API data and escaped in logs. -Ownership changes take effect only after they merge into the default branch. This prevents an untrusted pull request from changing the routing program or its owner assignments for its own run. +Ownership and approval policy changes take effect only after they merge into the default branch. This prevents an untrusted pull request from changing either program or policy for its own run. ## Verification -Run `pnpm run test:request-review` for ownership parsing, file classification, complete-patch checks, comment parsing, changed-LOC ranking, pagination, logging order, non-draft reconciliation, draft cancellation, reviewer provenance, reviewer filtering, and API behavior. [Workflow tests](../../scripts/ci-workflow.spec.ts) pin the trusted checkout, permissions, events, and command. The repository gate graph runs both checks in CI. +Run `pnpm run test:request-review` for routing behavior and `pnpm run test:approval-policy` for policy parsing, effective review decisions, pagination, permission filtering, weighted scoring, blockers, drafts, status publication, and API failures. [Workflow tests](../../scripts/ci-workflow.spec.ts) pin both workflows' trusted checkout, permissions, events, and commands. The repository gate graph runs all three checks in CI. diff --git a/.github/review-ownership/approval-policy.json b/.github/review-ownership/approval-policy.json new file mode 100644 index 0000000000..9ecc46ff5b --- /dev/null +++ b/.github/review-ownership/approval-policy.json @@ -0,0 +1,12 @@ +{ + "requiredPoints": 2, + "defaultPoints": 1, + "reviewerPoints": { + "07akioni": 2, + "imccyu": 2, + "tianyicui": 2, + "tianyicui-bot": 2, + "turtle1999": 2, + "turtle2099": 2 + } +} diff --git a/.github/review-ownership/check-approval.mjs b/.github/review-ownership/check-approval.mjs new file mode 100644 index 0000000000..3f3ef4a633 --- /dev/null +++ b/.github/review-ownership/check-approval.mjs @@ -0,0 +1,298 @@ +#!/usr/bin/env node + +import { readFileSync } from 'node:fs' +import process from 'node:process' +import { pathToFileURL } from 'node:url' + +const API_VERSION = '2026-03-10' +const MAX_PULL_REQUEST_REVIEWS = 3_000 +const PAGE_SIZE = 100 +const STATUS_CONTEXT = 'weighted approval' +const STATUS_PREFIX = 'This is by automated Angry Turtle Cyborg, not a human' +const WRITABLE_PERMISSIONS = new Set(['admin', 'write']) +const REVIEW_STATES = new Set(['APPROVED', 'CHANGES_REQUESTED', 'COMMENTED', 'DISMISSED', 'PENDING']) +const LOGIN = /^[A-Za-z0-9-]+(?:\[bot\])?$/u + +/** + * Parse the approval score policy. + * @param {string} source Approval policy JSON. + * @returns {{requiredPoints: number, defaultPoints: number, reviewerPoints: Map}} Validated policy. + */ +export function parseApprovalPolicy(source) { + const value = JSON.parse(source) + if (!isRecord(value)) throw new Error('approval policy must be an object') + const fields = Object.keys(value).sort() + if (fields.join(',') !== 'defaultPoints,requiredPoints,reviewerPoints') { + throw new Error('approval policy must contain only defaultPoints, requiredPoints, and reviewerPoints') + } + const requiredPoints = positiveInteger(value.requiredPoints, 'requiredPoints') + const defaultPoints = positiveInteger(value.defaultPoints, 'defaultPoints') + if (!isRecord(value.reviewerPoints)) throw new Error('reviewerPoints must be an object') + const reviewerPoints = new Map() + for (const [login, pointsValue] of Object.entries(value.reviewerPoints)) { + validateLogin(login, 'approval policy reviewer') + const key = login.toLowerCase() + if (reviewerPoints.has(key)) throw new Error(`duplicate approval policy reviewer @${login}`) + reviewerPoints.set(key, positiveInteger(pointsValue, `reviewerPoints.${login}`)) + } + return { requiredPoints, defaultPoints, reviewerPoints } +} + +/** + * Select each reviewer's latest non-comment review decision. + * @param {unknown[]} reviews Pull-request review records in GitHub's chronological order. + * @returns {Array<{login: string, state: 'APPROVED' | 'CHANGES_REQUESTED'}>} Effective review decisions. + */ +export function effectiveReviewDecisions(reviews) { + const decisions = new Map() + for (const review of reviews) { + if (!isRecord(review) || !isRecord(review.user) || typeof review.user.login !== 'string') { + throw new Error('pull-request review has no reviewer login') + } + const login = validateLogin(review.user.login, 'pull-request reviewer') + if (typeof review.state !== 'string' || !REVIEW_STATES.has(review.state.toUpperCase())) { + throw new Error(`pull-request review by @${login} has an invalid state`) + } + const state = review.state.toUpperCase() + if (state === 'APPROVED' || state === 'CHANGES_REQUESTED') { + decisions.set(login.toLowerCase(), { login, state }) + } + } + return [...decisions.values()] +} + +/** + * Create a repository-scoped GitHub JSON API caller. + * @param {{token: string, apiUrl?: string, fetchImpl?: typeof fetch}} options API dependencies. + * @returns {(path: string, options?: {method?: string, body?: unknown}) => Promise} API caller. + */ +export function createGitHubApi({ token, apiUrl = 'https://api.github.com', fetchImpl = globalThis.fetch }) { + if (!token) throw new Error('GITHUB_TOKEN is not set') + if (typeof fetchImpl !== 'function') throw new Error('fetch is unavailable') + const root = apiUrl.replace(/\/+$/u, '') + return async (path, { method = 'GET', body } = {}) => { + const response = await fetchImpl(`${root}${path}`, { + method, + headers: { + Accept: 'application/vnd.github+json', + Authorization: `Bearer ${token}`, + 'Content-Type': 'application/json', + 'User-Agent': 'deepseek-harness-weighted-approval', + 'X-GitHub-Api-Version': API_VERSION, + }, + ...(body === undefined ? {} : { body: JSON.stringify(body) }), + }) + if (!response.ok) { + const responseBody = await response.text() + throw new Error(`GitHub API ${method} ${path} returned ${response.status}: ${JSON.stringify(responseBody)}`) + } + if (response.status === 204) return undefined + return response.json() + } +} + +/** + * Fetch every pull-request review or fail before scoring a partial list. + * @param {(path: string, options?: {method?: string, body?: unknown}) => Promise} api GitHub API caller. + * @param {string} repository Owner/name repository identifier. + * @param {number} pullNumber Pull-request number. + * @returns {Promise} Complete review list within the supported limit. + */ +export async function listPullRequestReviews(api, repository, pullNumber) { + const reviews = [] + for (let page = 1; ; page++) { + const response = await api(`/repos/${repository}/pulls/${pullNumber}/reviews?per_page=${PAGE_SIZE}&page=${page}`) + if (!Array.isArray(response)) throw new Error('pull-request reviews response is not an array') + reviews.push(...response) + if (response.length < PAGE_SIZE) return reviews + if (reviews.length >= MAX_PULL_REQUEST_REVIEWS) { + throw new Error(`pull-request reviews exceed ${MAX_PULL_REQUEST_REVIEWS} records`) + } + } +} + +/** + * Evaluate approval points from current reviews and repository permissions. + * @param {{event: unknown, policySource: string, api: (path: string, options?: {method?: string, body?: unknown}) => Promise}} options Runtime inputs. + * @returns {Promise<{pull: {repository: string, number: number, headSha: string}, state: 'failure' | 'pending' | 'success', description: string, points: number, requiredPoints: number, approvals: Array<{login: string, points: number}>, blockers: string[], ignoredReviewers: string[]}>} Approval decision and status payload fields. + */ +export async function evaluateApproval({ event, policySource, api }) { + const pull = pullRequestFromEvent(event) + const policy = parseApprovalPolicy(policySource) + if (pull.draft) { + return approvalResult(pull, policy.requiredPoints, [], [], [], 'pending', 'draft pull request') + } + + const reviews = await listPullRequestReviews(api, pull.repository, pull.number) + const decisions = effectiveReviewDecisions(reviews) + .filter(({ login }) => login.toLowerCase() !== pull.author.toLowerCase()) + const permissions = [] + for (const { login, state } of decisions) { + permissions.push({ login, state, permission: await reviewerPermission(api, pull.repository, login) }) + } + const approvals = [] + const blockers = [] + const ignoredReviewers = [] + for (const { login, state, permission } of permissions) { + if (!WRITABLE_PERMISSIONS.has(permission)) { + ignoredReviewers.push(login) + } else if (state === 'CHANGES_REQUESTED') { + blockers.push(login) + } else { + approvals.push({ + login, + points: policy.reviewerPoints.get(login.toLowerCase()) ?? policy.defaultPoints, + }) + } + } + approvals.sort((left, right) => left.login.localeCompare(right.login, 'en')) + blockers.sort((left, right) => left.localeCompare(right, 'en')) + ignoredReviewers.sort((left, right) => left.localeCompare(right, 'en')) + const points = approvals.reduce((total, approval) => { + const next = total + approval.points + if (!Number.isSafeInteger(next)) throw new Error('approval points exceed the safe integer range') + return next + }, 0) + if (blockers.length > 0) { + return approvalResult(pull, policy.requiredPoints, approvals, blockers, ignoredReviewers, 'failure', + `${blockers.length} blocking change request${blockers.length === 1 ? '' : 's'}`) + } + const state = points >= policy.requiredPoints ? 'success' : 'pending' + return approvalResult( + pull, + policy.requiredPoints, + approvals, + blockers, + ignoredReviewers, + state, + `${points}/${policy.requiredPoints} approval points`, + ) +} + +/** + * Evaluate and publish the required commit status, publishing an error status when evaluation fails. + * @param {{event: unknown, policySource: string, api: (path: string, options?: {method?: string, body?: unknown}) => Promise, runUrl: string, write?: (line: string) => void}} options Runtime inputs. + * @returns {Promise>>} Published approval decision. + */ +export async function runApprovalCheck({ event, policySource, api, runUrl, write = line => process.stdout.write(`${line}\n`) }) { + const pull = pullRequestFromEvent(event) + write(STATUS_PREFIX) + let result + try { + result = await evaluateApproval({ event, policySource, api }) + } catch (error) { + await publishStatus(api, pull, 'error', `${STATUS_PREFIX}: approval evaluation failed.`, runUrl) + throw error + } + write(`Approval score: ${result.points}/${result.requiredPoints}.`) + writeList(write, 'Counted approvals', result.approvals.map(({ login, points }) => `@${login}: ${points}`)) + writeList(write, 'Blocking change requests', result.blockers.map(login => `@${login}`)) + writeList(write, 'Ignored reviewers without write access', result.ignoredReviewers.map(login => `@${login}`)) + await publishStatus(api, pull, result.state, result.description, runUrl) + write(`Published ${JSON.stringify(STATUS_CONTEXT)} status ${JSON.stringify(result.state)}.`) + return result +} + +function approvalResult(pull, requiredPoints, approvals, blockers, ignoredReviewers, state, detail) { + return { + pull: { repository: pull.repository, number: pull.number, headSha: pull.headSha }, + state, + description: `${STATUS_PREFIX}: ${detail}.`, + points: approvals.reduce((total, approval) => total + approval.points, 0), + requiredPoints, + approvals, + blockers, + ignoredReviewers, + } +} + +async function reviewerPermission(api, repository, login) { + const response = await api(`/repos/${repository}/collaborators/${encodeURIComponent(login)}/permission`) + if (!isRecord(response) || typeof response.permission !== 'string') { + throw new Error(`collaborator permission response for @${login} has no permission`) + } + return response.permission.toLowerCase() +} + +async function publishStatus(api, pull, state, description, runUrl) { + if (!/^https:\/\/[^\s]+$/u.test(runUrl)) throw new Error('workflow run URL must use HTTPS') + if (description.length > 140) throw new Error('commit status description exceeds 140 characters') + await api(`/repos/${pull.repository}/statuses/${pull.headSha}`, { + method: 'POST', + body: { + state, + context: STATUS_CONTEXT, + description, + target_url: runUrl, + }, + }) +} + +function pullRequestFromEvent(event) { + if (!isRecord(event) || !isRecord(event.repository) || typeof event.repository.full_name !== 'string' + || !/^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+$/u.test(event.repository.full_name)) { + throw new Error('event has no valid repository.full_name') + } + if (!isRecord(event.pull_request) || !isRecord(event.pull_request.user) + || !isRecord(event.pull_request.head)) { + throw new Error('event has no pull_request') + } + const pull = event.pull_request + if (!Number.isSafeInteger(pull.number) || pull.number <= 0) throw new Error('pull request has no valid number') + if (typeof pull.draft !== 'boolean') throw new Error('pull request has no draft flag') + const author = validateLogin(pull.user.login, 'pull-request author') + if (typeof pull.head.sha !== 'string' || !/^[0-9a-f]{40}$/u.test(pull.head.sha)) { + throw new Error('pull request has no valid head SHA') + } + return { + repository: event.repository.full_name, + number: pull.number, + draft: pull.draft, + author, + headSha: pull.head.sha, + } +} + +function positiveInteger(value, field) { + if (!Number.isSafeInteger(value) || value <= 0) throw new Error(`${field} must be a positive integer`) + return value +} + +function validateLogin(value, subject) { + if (typeof value !== 'string' || !LOGIN.test(value)) throw new Error(`${subject} has an invalid login`) + return value +} + +function writeList(write, title, entries) { + write(`${title}:`) + if (entries.length === 0) write('- (none)') + else for (const entry of entries) write(`- ${entry}`) +} + +function isRecord(value) { + return typeof value === 'object' && value !== null && !Array.isArray(value) +} + +async function main() { + const eventPath = process.env.GITHUB_EVENT_PATH + if (!eventPath) throw new Error('GITHUB_EVENT_PATH is not set') + const event = JSON.parse(readFileSync(eventPath, 'utf8')) + const policySource = readFileSync(new URL('approval-policy.json', import.meta.url), 'utf8') + const api = createGitHubApi({ + token: process.env.GITHUB_TOKEN ?? '', + apiUrl: process.env.GITHUB_API_URL, + }) + await runApprovalCheck({ + event, + policySource, + api, + runUrl: process.env.GITHUB_RUN_URL ?? '', + }) +} + +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { + main().catch((error) => { + process.stderr.write(`weighted approval failed: ${error instanceof Error ? error.message : String(error)}\n`) + process.exitCode = 1 + }) +} diff --git a/.github/review-ownership/check-approval.test.mjs b/.github/review-ownership/check-approval.test.mjs new file mode 100644 index 0000000000..fc033d5450 --- /dev/null +++ b/.github/review-ownership/check-approval.test.mjs @@ -0,0 +1,244 @@ +import assert from 'node:assert/strict' +import { readFileSync } from 'node:fs' +import test from 'node:test' + +import { + createGitHubApi, + effectiveReviewDecisions, + evaluateApproval, + listPullRequestReviews, + parseApprovalPolicy, + runApprovalCheck, +} from './check-approval.mjs' + +const policySource = readFileSync(new URL('approval-policy.json', import.meta.url), 'utf8') +const HEAD_SHA = '1234567890abcdef1234567890abcdef12345678' + +const pullRequestEvent = ({ author = 'author', draft = false } = {}) => ({ + repository: { full_name: 'deepseek-harness/deepseek-harness' }, + pull_request: { + number: 42, + draft, + user: { login: author }, + head: { sha: HEAD_SHA }, + }, +}) + +const review = (login, state) => ({ user: { login }, state }) + +test('loads the repository approval score policy', () => { + const policy = parseApprovalPolicy(policySource) + assert.equal(policy.requiredPoints, 2) + assert.equal(policy.defaultPoints, 1) + assert.deepEqual([...policy.reviewerPoints], [ + ['07akioni', 2], + ['imccyu', 2], + ['tianyicui', 2], + ['tianyicui-bot', 2], + ['turtle1999', 2], + ['turtle2099', 2], + ]) +}) + +test('rejects invalid approval score policies', () => { + for (const [source, message] of [ + ['[]', /must be an object/u], + ['{"requiredPoints":0,"defaultPoints":1,"reviewerPoints":{}}', /requiredPoints/u], + ['{"requiredPoints":2,"defaultPoints":0,"reviewerPoints":{}}', /defaultPoints/u], + ['{"requiredPoints":2,"defaultPoints":1,"reviewerPoints":[]}', /reviewerPoints must be an object/u], + ['{"requiredPoints":2,"defaultPoints":1,"reviewerPoints":{},"typo":2}', /contain only/u], + ['{"requiredPoints":2,"defaultPoints":1,"reviewerPoints":{"bad login":2}}', /invalid login/u], + ['{"requiredPoints":2,"defaultPoints":1,"reviewerPoints":{"User":2,"user":2}}', /duplicate/u], + ['{"requiredPoints":2,"defaultPoints":1,"reviewerPoints":{"user":-1}}', /positive integer/u], + ]) { + assert.throws(() => parseApprovalPolicy(source), message) + } +}) + +test('uses each reviewer latest approval or change request while ignoring comments and dismissed reviews', () => { + assert.deepEqual(effectiveReviewDecisions([ + review('first', 'APPROVED'), + review('first', 'COMMENTED'), + review('second', 'CHANGES_REQUESTED'), + review('second', 'APPROVED'), + review('third', 'APPROVED'), + review('third', 'CHANGES_REQUESTED'), + review('dismissed', 'DISMISSED'), + ]), [ + { login: 'first', state: 'APPROVED' }, + { login: 'second', state: 'APPROVED' }, + { login: 'third', state: 'CHANGES_REQUESTED' }, + ]) +}) + +test('fetches every pull-request review and rejects an unbounded history', async () => { + let calls = 0 + const reviews = await listPullRequestReviews(async () => { + calls++ + return calls === 1 ? Array.from({ length: 100 }, () => review('user', 'COMMENTED')) : [] + }, 'owner/repo', 42) + assert.equal(reviews.length, 100) + assert.equal(calls, 2) + + calls = 0 + await assert.rejects(listPullRequestReviews(async () => { + calls++ + return Array.from({ length: 100 }, () => review('user', 'COMMENTED')) + }, 'owner/repo', 42), /exceed 3000/u) + assert.equal(calls, 30) +}) + +test('accepts one two-point approval from a write-capable reviewer', async () => { + const calls = [] + const result = await evaluateApproval({ + event: pullRequestEvent(), + policySource, + api: async (path) => { + calls.push(path) + if (path.includes('/reviews?')) return [review('07akioni', 'APPROVED')] + if (path.includes('/collaborators/07akioni/permission')) return { permission: 'write' } + throw new Error(`unexpected API path ${path}`) + }, + }) + assert.equal(result.state, 'success') + assert.equal(result.points, 2) + assert.deepEqual(result.approvals, [{ login: '07akioni', points: 2 }]) + assert.equal(calls.length, 2) +}) + +test('accepts two one-point approvals and ignores reviews without write access', async () => { + const result = await evaluateApproval({ + event: pullRequestEvent(), + policySource, + api: async (path) => { + if (path.includes('/reviews?')) { + return [ + review('reader', 'APPROVED'), + review('writer-b', 'APPROVED'), + review('writer-a', 'APPROVED'), + ] + } + if (path.includes('/collaborators/reader/permission')) return { permission: 'read' } + if (path.includes('/collaborators/writer-a/permission')) return { permission: 'admin' } + if (path.includes('/collaborators/writer-b/permission')) return { permission: 'write' } + throw new Error(`unexpected API path ${path}`) + }, + }) + assert.equal(result.state, 'success') + assert.equal(result.points, 2) + assert.deepEqual(result.approvals, [ + { login: 'writer-a', points: 1 }, + { login: 'writer-b', points: 1 }, + ]) + assert.deepEqual(result.ignoredReviewers, ['reader']) +}) + +test('blocks on a write-capable change request but ignores the author and read-only blockers', async () => { + const result = await evaluateApproval({ + event: pullRequestEvent({ author: 'author' }), + policySource, + api: async (path) => { + if (path.includes('/reviews?')) { + return [ + review('turtle1999', 'APPROVED'), + review('blocker', 'CHANGES_REQUESTED'), + review('reader', 'CHANGES_REQUESTED'), + review('author', 'CHANGES_REQUESTED'), + ] + } + if (path.includes('/collaborators/turtle1999/permission')) return { permission: 'admin' } + if (path.includes('/collaborators/blocker/permission')) return { permission: 'write' } + if (path.includes('/collaborators/reader/permission')) return { permission: 'read' } + throw new Error(`unexpected API path ${path}`) + }, + }) + assert.equal(result.state, 'failure') + assert.equal(result.points, 2) + assert.deepEqual(result.blockers, ['blocker']) + assert.deepEqual(result.ignoredReviewers, ['reader']) +}) + +test('keeps drafts pending without reading reviews', async () => { + const result = await evaluateApproval({ + event: pullRequestEvent({ draft: true }), + policySource, + api: async () => { throw new Error('draft evaluation must not call GitHub') }, + }) + assert.equal(result.state, 'pending') + assert.equal(result.points, 0) + assert.match(result.description, /draft pull request/u) +}) + +test('publishes the required status and replaces stale success with error on evaluation failure', async () => { + const calls = [] + const output = [] + const result = await runApprovalCheck({ + event: pullRequestEvent(), + policySource, + runUrl: 'https://github.example/actions/runs/1', + api: async (path, options = {}) => { + calls.push({ path, options }) + if (path.includes('/reviews?')) return [review('turtle2099', 'APPROVED')] + if (path.includes('/collaborators/turtle2099/permission')) return { permission: 'write' } + if (path.includes('/statuses/')) return {} + throw new Error(`unexpected API path ${path}`) + }, + write: line => output.push(line), + }) + assert.equal(result.state, 'success') + assert.deepEqual(calls.at(-1), { + path: `/repos/deepseek-harness/deepseek-harness/statuses/${HEAD_SHA}`, + options: { + method: 'POST', + body: { + state: 'success', + context: 'weighted approval', + description: 'This is by automated Angry Turtle Cyborg, not a human: 2/2 approval points.', + target_url: 'https://github.example/actions/runs/1', + }, + }, + }) + assert.equal(output[0], 'This is by automated Angry Turtle Cyborg, not a human') + + const failures = [] + await assert.rejects(runApprovalCheck({ + event: pullRequestEvent(), + policySource, + runUrl: 'https://github.example/actions/runs/2', + api: async (path, options = {}) => { + if (path.includes('/reviews?')) throw new Error('reviews unavailable') + if (path.includes('/statuses/')) { + failures.push({ path, options }) + return {} + } + throw new Error(`unexpected API path ${path}`) + }, + write: () => {}, + }), /reviews unavailable/u) + assert.equal(failures[0].options.body.state, 'error') +}) + +test('sends authenticated JSON and escapes an API error body', async () => { + const requests = [] + const api = createGitHubApi({ + token: 'secret', + apiUrl: 'https://github.example/api/v3/', + fetchImpl: async (url, options) => { + requests.push({ url, options }) + return new Response(JSON.stringify({ ok: true }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }) + }, + }) + assert.deepEqual(await api('/repos/owner/repo', { method: 'POST', body: { value: 1 } }), { ok: true }) + assert.equal(requests[0].url, 'https://github.example/api/v3/repos/owner/repo') + assert.equal(requests[0].options.headers.Authorization, 'Bearer secret') + assert.equal(requests[0].options.body, '{"value":1}') + + const failing = createGitHubApi({ + token: 'secret', + fetchImpl: async () => new Response('::error::untrusted\nbody', { status: 422 }), + }) + await assert.rejects(failing('/failure'), /"::error::untrusted\\nbody"/u) +}) diff --git a/.github/workflows/weighted-approval.yml b/.github/workflows/weighted-approval.yml new file mode 100644 index 0000000000..fe0d34deb5 --- /dev/null +++ b/.github/workflows/weighted-approval.yml @@ -0,0 +1,35 @@ +name: weighted-approval + +on: + pull_request_target: + types: [opened, synchronize, reopened, ready_for_review, converted_to_draft] + pull_request_review: + types: [submitted, edited, dismissed] + +permissions: + contents: read + pull-requests: read + statuses: write + +concurrency: + group: weighted-approval-${{ github.event.pull_request.number }} + cancel-in-progress: true + +jobs: + publish-status: + name: publish weighted approval status + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + # SECURITY: the status-writing job executes policy from the trusted default + # branch and reads pull-request reviews only as API data. + - name: Check out trusted approval policy + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 + with: + ref: ${{ github.event.repository.default_branch }} + persist-credentials: false + - name: Publish weighted approval status + env: + GITHUB_TOKEN: ${{ github.token }} + GITHUB_RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + run: node .github/review-ownership/check-approval.mjs diff --git a/package.json b/package.json index 1c4c8f6f86..929c3788a4 100644 --- a/package.json +++ b/package.json @@ -57,6 +57,7 @@ "test:expected": "vitest run --config vitest.expected.config.ts", "test:expected:refresh": "DSH_SNAPSHOT=refresh vitest run --config vitest.expected.config.ts", "test:issue-management": "node .github/issue-management/policy.test.mjs", + "test:approval-policy": "node --test .github/review-ownership/check-approval.test.mjs", "test:request-review": "node --test .github/review-ownership/request-review.test.mjs", "test:snapshot": "vitest run --config vitest.snapshot.config.ts", "test:snapshot:record": "DSH_SNAPSHOT=record vitest run --config vitest.snapshot.config.ts --update", diff --git a/scripts/ci-workflow.spec.ts b/scripts/ci-workflow.spec.ts index 8c458d0b0c..70097a9cf9 100644 --- a/scripts/ci-workflow.spec.ts +++ b/scripts/ci-workflow.spec.ts @@ -815,6 +815,51 @@ describe('Request review workflow', () => { }) }) +describe('Weighted approval workflow', () => { + it('publishes a trusted status on pull request and review updates', () => { + const workflow = loadWorkflow('.github/workflows/weighted-approval.yml') + const pullRequest = workflowEvent(workflow, 'pull_request_target') + const review = workflowEvent(workflow, 'pull_request_review') + const job = workflowJob(workflow, 'publish-status') + if (!isRecord(workflow.on)) throw new TypeError('weighted-approval workflow must define events') + if (!Array.isArray(job.steps)) throw new TypeError('weighted-approval job must define steps') + const steps = job.steps.filter(isRecord) + const checkout = steps.find(step => step.name === 'Check out trusted approval policy') + const publish = steps.find(step => step.name === 'Publish weighted approval status') + + expect(workflow.name).toBe('weighted-approval') + expect(Object.keys(workflow.on)).toEqual(['pull_request_target', 'pull_request_review']) + expect(pullRequest.types).toEqual(['opened', 'synchronize', 'reopened', 'ready_for_review', 'converted_to_draft']) + expect(review.types).toEqual(['submitted', 'edited', 'dismissed']) + expect(workflow.permissions).toEqual({ contents: 'read', 'pull-requests': 'read', statuses: 'write' }) + expect(workflow.concurrency).toEqual({ + group: 'weighted-approval-${{ github.event.pull_request.number }}', + 'cancel-in-progress': true, + }) + expect(job).toMatchObject({ + name: 'publish weighted approval status', + 'runs-on': 'ubuntu-latest', + 'timeout-minutes': 5, + }) + expect(checkout).toMatchObject({ + uses: 'actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1', + with: { + ref: '${{ github.event.repository.default_branch }}', + 'persist-credentials': false, + }, + }) + expect(publish).toMatchObject({ + env: { + GITHUB_TOKEN: '${{ github.token }}', + GITHUB_RUN_URL: '${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}', + }, + run: 'node .github/review-ownership/check-approval.mjs', + }) + expect(JSON.stringify(workflow)).not.toContain('github.event.pull_request.head') + expect(JSON.stringify(workflow)).not.toContain('secrets.') + }) +}) + describe('Issue lifecycle workflow', () => { it('runs the lifecycle job on every PR/review event but gates token and board steps', () => { const lifecycle = loadWorkflow('.github/workflows/issue-lifecycle.yml') diff --git a/scripts/run-gates.ts b/scripts/run-gates.ts index d8f82f33d5..a1c022efd0 100644 --- a/scripts/run-gates.ts +++ b/scripts/run-gates.ts @@ -267,6 +267,7 @@ export function gatesForMode(selected: Mode): Gate[] { pnpmScript('client-domain-graph', 'verify-client-domain-graph', { label: 'client domain graph' }), pnpmScript('test', 'test'), pnpmScript('issue-management', 'test:issue-management', { label: 'Issue management policy' }), + pnpmScript('approval-policy', 'test:approval-policy', { label: 'Weighted approval policy' }), pnpmScript('request-review', 'test:request-review', { label: 'Review request policy' }), pnpmScript('duplication', 'duplication'), snapshotGate(), @@ -311,6 +312,7 @@ function ciSharedStaticGates(): Gate[] { pnpmScript('client-ui-i18n', 'verify-client-ui-i18n', { label: 'client UI i18n' }), pnpmScript('no-bare-dispatcher', 'verify-no-bare-dispatcher', { label: 'proxy-aware dispatchers' }), pnpmScript('issue-management', 'test:issue-management', { label: 'Issue management policy' }), + pnpmScript('approval-policy', 'test:approval-policy', { label: 'Weighted approval policy' }), pnpmScript('request-review', 'test:request-review', { label: 'Review request policy' }), ] } From c4855581c3e01b8d0e735741f117483d0df06d65 Mon Sep 17 00:00:00 2001 From: Turtle Date: Tue, 8 Sep 2026 20:19:29 +0800 Subject: [PATCH 003/164] fix: preserve weighted approval review semantics --- .github/review-ownership/README.md | 12 +- .github/review-ownership/check-approval.mjs | 111 +++++++++++++++--- .../review-ownership/check-approval.test.mjs | 77 +++++++++++- .../weighted-approval-review-event.yml | 19 +++ .github/workflows/weighted-approval.yml | 10 +- package.json | 2 +- scripts/ci-workflow.spec.ts | 55 ++++++--- scripts/run-gates.spec.ts | 9 ++ scripts/run-gates.ts | 4 +- 9 files changed, 254 insertions(+), 45 deletions(-) create mode 100644 .github/workflows/weighted-approval-review-event.yml diff --git a/.github/review-ownership/README.md b/.github/review-ownership/README.md index ab5ba83903..abdeae89d2 100644 --- a/.github/review-ownership/README.md +++ b/.github/review-ownership/README.md @@ -2,7 +2,7 @@ ## Summary -The [`request-review` workflow](../workflows/request-review.yml) requests owners for reviewable code. The [`weighted-approval` workflow](../workflows/weighted-approval.yml) publishes an approval score for branch rules. Both workflows execute policy from the trusted default branch. +The [`request-review` workflow](../workflows/request-review.yml) requests owners for reviewable code. The [`weighted-approval` workflow](../workflows/weighted-approval.yml) publishes an approval score for branch rules. Both write-capable workflows execute policy from the trusted default branch. ## Table of Contents @@ -31,13 +31,13 @@ The policy test measures non-test tracked lines under matched directories and re ## Approval scoring -The weighted approval workflow publishes the `weighted approval` commit status on the pull request head. Branch rules must require this status context. The status succeeds at two approval points, remains pending below two points or while the pull request is a draft, fails while a write-capable reviewer has an effective `CHANGES_REQUESTED` review, and reports an error when policy evaluation fails. +The weighted approval workflow publishes the `weighted approval` commit status on the pull request head. Branch rules must require this status with GitHub Actions as its expected source; a context-only requirement can accept a same-named status from another integration. The status succeeds at two approval points, remains pending below two points or while the pull request is a draft, fails while a write-capable reviewer has an effective `CHANGES_REQUESTED` review, and reports an error when policy evaluation fails. Reviewers whose calculated base repository permission is `write` or `admin` count. The [approval policy](approval-policy.json) gives `@07akioni`, `@imccyu`, `@tianyicui`, `@tianyicui-bot`, `@turtle1999`, and `@turtle2099` two points each; every other write-capable reviewer gets one point. The pull-request author and reviewers without write permission do not count. -Each reviewer contributes only the latest non-comment `APPROVED` or `CHANGES_REQUESTED` decision that GitHub returns. Comment-only and dismissed review records do not replace an effective decision. The workflow does not invalidate an approval by its review commit; the repository's native pull-request rules own stale-review and latest-push requirements. +Each reviewer contributes only the current `APPROVED` or `CHANGES_REQUESTED` decision that GitHub returns. A `DISMISSED` record clears that reviewer's standing decision, including earlier approvals. Comment-only and pending records do not replace a decision. Reviews from deleted accounts and reviewers without current repository access do not count. The workflow does not invalidate an approval by its review commit; the repository's native pull-request rules own stale-review and latest-push requirements. -The workflow runs when a pull request opens, synchronizes, reopens, becomes ready, or becomes a draft, and when a review is submitted, edited, or dismissed. It fetches every review and resolves current repository permission before publishing the status. Permission changes take effect on the next subscribed pull-request or review event. +The publisher runs when a pull request opens, synchronizes, reopens, becomes ready, or becomes a draft. Review submissions, edits, and dismissals run the no-permission [`weighted-approval-review-event` workflow](../workflows/weighted-approval-review-event.yml); its validated run title supplies the pull-request number to the default-branch publisher. The publisher validates the current head, fetches every review, and resolves current repository permission before publishing the status. Permission changes take effect on the next subscribed pull-request or review event. @@ -55,7 +55,7 @@ For a modified file with a supported source extension, the scanner compares the ## Security -The write-capable jobs check out only the repository default branch. They do not check out or execute pull-request code and do not use repository secrets. Pull-request filenames and reviews are treated as API data and escaped in logs. +The write-capable jobs check out only the repository default branch. They do not check out or execute pull-request code and do not use repository secrets. The review-event workflow has no `GITHUB_TOKEN` permissions and passes only a decimal pull-request number in its run title. The publisher rejects an invalid run title and a number that does not resolve to the workflow run's current pull-request head. Pull-request filenames and reviews are treated as API data and escaped in logs. Ownership and approval policy changes take effect only after they merge into the default branch. This prevents an untrusted pull request from changing either program or policy for its own run. @@ -63,7 +63,7 @@ Ownership and approval policy changes take effect only after they merge into the ## Verification -Run `pnpm run test:request-review` for routing behavior and `pnpm run test:approval-policy` for policy parsing, effective review decisions, pagination, permission filtering, weighted scoring, blockers, drafts, status publication, and API failures. [Workflow tests](../../scripts/ci-workflow.spec.ts) pin both workflows' trusted checkout, permissions, events, and commands. The repository gate graph runs all three checks in CI. +Run `pnpm run test:request-review` for routing behavior and `pnpm run test:approval-policy` for policy parsing, effective review decisions, review-event validation, pagination, permission filtering, weighted scoring, blockers, drafts, status publication, and API failures. [Workflow tests](../../scripts/ci-workflow.spec.ts) pin the trusted checkout, no-permission review handoff, permissions, events, and commands. The repository gate graph runs both policy checks and the workflow tests in CI. diff --git a/.github/review-ownership/check-approval.mjs b/.github/review-ownership/check-approval.mjs index 3f3ef4a633..ac354e601f 100644 --- a/.github/review-ownership/check-approval.mjs +++ b/.github/review-ownership/check-approval.mjs @@ -13,6 +13,14 @@ const WRITABLE_PERMISSIONS = new Set(['admin', 'write']) const REVIEW_STATES = new Set(['APPROVED', 'CHANGES_REQUESTED', 'COMMENTED', 'DISMISSED', 'PENDING']) const LOGIN = /^[A-Za-z0-9-]+(?:\[bot\])?$/u +class GitHubApiError extends Error { + constructor(message, status) { + super(message) + this.name = 'GitHubApiError' + this.status = status + } +} + /** * Parse the approval score policy. * @param {string} source Approval policy JSON. @@ -39,14 +47,16 @@ export function parseApprovalPolicy(source) { } /** - * Select each reviewer's latest non-comment review decision. + * Select each reviewer's current approval or change-request decision. * @param {unknown[]} reviews Pull-request review records in GitHub's chronological order. * @returns {Array<{login: string, state: 'APPROVED' | 'CHANGES_REQUESTED'}>} Effective review decisions. */ export function effectiveReviewDecisions(reviews) { const decisions = new Map() for (const review of reviews) { - if (!isRecord(review) || !isRecord(review.user) || typeof review.user.login !== 'string') { + if (!isRecord(review)) throw new Error('pull-request review is not an object') + if (review.user === null) continue + if (!isRecord(review.user) || typeof review.user.login !== 'string') { throw new Error('pull-request review has no reviewer login') } const login = validateLogin(review.user.login, 'pull-request reviewer') @@ -54,8 +64,11 @@ export function effectiveReviewDecisions(reviews) { throw new Error(`pull-request review by @${login} has an invalid state`) } const state = review.state.toUpperCase() - if (state === 'APPROVED' || state === 'CHANGES_REQUESTED') { - decisions.set(login.toLowerCase(), { login, state }) + const key = login.toLowerCase() + if (state === 'DISMISSED') { + decisions.delete(key) + } else if (state === 'APPROVED' || state === 'CHANGES_REQUESTED') { + decisions.set(key, { login, state }) } } return [...decisions.values()] @@ -84,7 +97,10 @@ export function createGitHubApi({ token, apiUrl = 'https://api.github.com', fetc }) if (!response.ok) { const responseBody = await response.text() - throw new Error(`GitHub API ${method} ${path} returned ${response.status}: ${JSON.stringify(responseBody)}`) + throw new GitHubApiError( + `GitHub API ${method} ${path} returned ${response.status}: ${JSON.stringify(responseBody)}`, + response.status, + ) } if (response.status === 204) return undefined return response.json() @@ -193,6 +209,32 @@ export async function runApprovalCheck({ event, policySource, api, runUrl, write return result } +/** + * Resolve the reviewed pull request from a completed review-event workflow run. + * @param {{event: unknown, api: (path: string, options?: {method?: string, body?: unknown}) => Promise}} options Trusted workflow inputs. + * @returns {Promise | null>} Event with a current pull request, or null after the pull-request head changes. + */ +export async function approvalEventFromWorkflowRun({ event, api }) { + const repository = repositoryFromEvent(event) + if (!isRecord(event.workflow_run) || event.workflow_run.name !== 'weighted-approval-review-event' + || event.workflow_run.event !== 'pull_request_review' || event.workflow_run.conclusion !== 'success') { + throw new Error('event has no successful weighted approval review workflow run') + } + const expectedHeadSha = validateHeadSha(event.workflow_run.head_sha, 'workflow run') + const pullNumber = parsePullNumber(event.workflow_run.display_title) + if (!Array.isArray(event.workflow_run.pull_requests)) { + throw new Error('workflow run has no pull_requests array') + } + if (event.workflow_run.pull_requests.length > 0 + && !event.workflow_run.pull_requests.some(pull => isRecord(pull) && pull.number === pullNumber)) { + throw new Error(`workflow run is not associated with pull request #${pullNumber}`) + } + const pull = await api(`/repos/${repository}/pulls/${pullNumber}`) + if (!isRecord(pull) || !isRecord(pull.head)) throw new Error(`pull request #${pullNumber} response is invalid`) + if (pull.head.sha !== expectedHeadSha) return null + return { ...event, pull_request: pull } +} + function approvalResult(pull, requiredPoints, approvals, blockers, ignoredReviewers, state, detail) { return { pull: { repository: pull.repository, number: pull.number, headSha: pull.headSha }, @@ -207,7 +249,13 @@ function approvalResult(pull, requiredPoints, approvals, blockers, ignoredReview } async function reviewerPermission(api, repository, login) { - const response = await api(`/repos/${repository}/collaborators/${encodeURIComponent(login)}/permission`) + let response + try { + response = await api(`/repos/${repository}/collaborators/${encodeURIComponent(login)}/permission`) + } catch (error) { + if (error instanceof GitHubApiError && error.status === 404) return 'none' + throw error + } if (!isRecord(response) || typeof response.permission !== 'string') { throw new Error(`collaborator permission response for @${login} has no permission`) } @@ -229,10 +277,7 @@ async function publishStatus(api, pull, state, description, runUrl) { } function pullRequestFromEvent(event) { - if (!isRecord(event) || !isRecord(event.repository) || typeof event.repository.full_name !== 'string' - || !/^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+$/u.test(event.repository.full_name)) { - throw new Error('event has no valid repository.full_name') - } + const repository = repositoryFromEvent(event) if (!isRecord(event.pull_request) || !isRecord(event.pull_request.user) || !isRecord(event.pull_request.head)) { throw new Error('event has no pull_request') @@ -241,18 +286,40 @@ function pullRequestFromEvent(event) { if (!Number.isSafeInteger(pull.number) || pull.number <= 0) throw new Error('pull request has no valid number') if (typeof pull.draft !== 'boolean') throw new Error('pull request has no draft flag') const author = validateLogin(pull.user.login, 'pull-request author') - if (typeof pull.head.sha !== 'string' || !/^[0-9a-f]{40}$/u.test(pull.head.sha)) { - throw new Error('pull request has no valid head SHA') - } + const headSha = validateHeadSha(pull.head.sha, 'pull request') return { - repository: event.repository.full_name, + repository, number: pull.number, draft: pull.draft, author, - headSha: pull.head.sha, + headSha, } } +function repositoryFromEvent(event) { + if (!isRecord(event) || !isRecord(event.repository) || typeof event.repository.full_name !== 'string' + || !/^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+$/u.test(event.repository.full_name)) { + throw new Error('event has no valid repository.full_name') + } + return event.repository.full_name +} + +function validateHeadSha(value, subject) { + if (typeof value !== 'string' || !/^[0-9a-f]{40}$/u.test(value)) { + throw new Error(`${subject} has no valid head SHA`) + } + return value +} + +function parsePullNumber(source) { + if (typeof source !== 'string') throw new Error('review event has no valid run title') + const match = /^weighted-approval-review-event:([1-9][0-9]*)$/u.exec(source) + if (!match) throw new Error('review event has no valid run title') + const pullNumber = Number(match[1]) + if (!Number.isSafeInteger(pullNumber)) throw new Error('review event pull request number is not a safe integer') + return pullNumber +} + function positiveInteger(value, field) { if (!Number.isSafeInteger(value) || value <= 0) throw new Error(`${field} must be a positive integer`) return value @@ -276,12 +343,24 @@ function isRecord(value) { async function main() { const eventPath = process.env.GITHUB_EVENT_PATH if (!eventPath) throw new Error('GITHUB_EVENT_PATH is not set') - const event = JSON.parse(readFileSync(eventPath, 'utf8')) + let event = JSON.parse(readFileSync(eventPath, 'utf8')) const policySource = readFileSync(new URL('approval-policy.json', import.meta.url), 'utf8') const api = createGitHubApi({ token: process.env.GITHUB_TOKEN ?? '', apiUrl: process.env.GITHUB_API_URL, }) + if (isRecord(event) && isRecord(event.workflow_run)) { + const resolved = await approvalEventFromWorkflowRun({ + event, + api, + }) + if (resolved === null) { + process.stdout.write(`${STATUS_PREFIX}\n`) + process.stdout.write('Skipped a review event for a superseded pull-request head.\n') + return + } + event = resolved + } await runApprovalCheck({ event, policySource, diff --git a/.github/review-ownership/check-approval.test.mjs b/.github/review-ownership/check-approval.test.mjs index fc033d5450..9a1412b25a 100644 --- a/.github/review-ownership/check-approval.test.mjs +++ b/.github/review-ownership/check-approval.test.mjs @@ -3,6 +3,7 @@ import { readFileSync } from 'node:fs' import test from 'node:test' import { + approvalEventFromWorkflowRun, createGitHubApi, effectiveReviewDecisions, evaluateApproval, @@ -55,22 +56,61 @@ test('rejects invalid approval score policies', () => { } }) -test('uses each reviewer latest approval or change request while ignoring comments and dismissed reviews', () => { +test('uses each reviewer current decision and clears it on dismissal', () => { assert.deepEqual(effectiveReviewDecisions([ + review('first', 'APPROVED'), review('first', 'APPROVED'), review('first', 'COMMENTED'), + review('first', 'DISMISSED'), review('second', 'CHANGES_REQUESTED'), review('second', 'APPROVED'), review('third', 'APPROVED'), review('third', 'CHANGES_REQUESTED'), review('dismissed', 'DISMISSED'), + { user: null, state: 'APPROVED' }, ]), [ - { login: 'first', state: 'APPROVED' }, { login: 'second', state: 'APPROVED' }, { login: 'third', state: 'CHANGES_REQUESTED' }, ]) }) +test('resolves a review workflow run to the current pull request and rejects stale heads', async () => { + const workflowRunEvent = { + repository: { full_name: 'deepseek-harness/deepseek-harness' }, + workflow_run: { + name: 'weighted-approval-review-event', + event: 'pull_request_review', + conclusion: 'success', + head_sha: HEAD_SHA, + display_title: 'weighted-approval-review-event:42', + pull_requests: [], + }, + } + const current = await approvalEventFromWorkflowRun({ + event: workflowRunEvent, + api: async path => { + assert.equal(path, '/repos/deepseek-harness/deepseek-harness/pulls/42') + return pullRequestEvent().pull_request + }, + }) + assert.equal(current.pull_request.number, 42) + + assert.equal(await approvalEventFromWorkflowRun({ + event: workflowRunEvent, + api: async () => ({ + ...pullRequestEvent().pull_request, + head: { sha: 'abcdef1234567890abcdef1234567890abcdef12' }, + }), + }), null) + await assert.rejects(approvalEventFromWorkflowRun({ + event: { + ...workflowRunEvent, + workflow_run: { ...workflowRunEvent.workflow_run, display_title: '../42' }, + }, + api: async () => { throw new Error('invalid number must not call GitHub') }, + }), /valid run title/u) +}) + test('fetches every pull-request review and rejects an unbounded history', async () => { let calls = 0 const reviews = await listPullRequestReviews(async () => { @@ -133,6 +173,39 @@ test('accepts two one-point approvals and ignores reviews without write access', assert.deepEqual(result.ignoredReviewers, ['reader']) }) +test('keeps one one-point approval pending without failing the status', async () => { + const result = await evaluateApproval({ + event: pullRequestEvent(), + policySource, + api: async (path) => { + if (path.includes('/reviews?')) return [review('writer', 'APPROVED')] + if (path.includes('/collaborators/writer/permission')) return { permission: 'write' } + throw new Error(`unexpected API path ${path}`) + }, + }) + assert.equal(result.state, 'pending') + assert.equal(result.points, 1) +}) + +test('ignores a reviewer whose collaborator permission lookup returns 404', async () => { + const api = createGitHubApi({ + token: 'secret', + fetchImpl: async (url) => { + if (url.includes('/reviews?')) { + return new Response(JSON.stringify([review('former-writer', 'APPROVED')]), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }) + } + if (url.includes('/collaborators/former-writer/permission')) return new Response('Not Found', { status: 404 }) + throw new Error(`unexpected API URL ${url}`) + }, + }) + const result = await evaluateApproval({ event: pullRequestEvent(), policySource, api }) + assert.equal(result.state, 'pending') + assert.deepEqual(result.ignoredReviewers, ['former-writer']) +}) + test('blocks on a write-capable change request but ignores the author and read-only blockers', async () => { const result = await evaluateApproval({ event: pullRequestEvent({ author: 'author' }), diff --git a/.github/workflows/weighted-approval-review-event.yml b/.github/workflows/weighted-approval-review-event.yml new file mode 100644 index 0000000000..403b8c14dd --- /dev/null +++ b/.github/workflows/weighted-approval-review-event.yml @@ -0,0 +1,19 @@ +name: weighted-approval-review-event +run-name: weighted-approval-review-event:${{ github.event.pull_request.number }} + +on: + pull_request_review: + types: [submitted, edited, dismissed] + +permissions: {} + +jobs: + record-review-event: + name: record weighted approval review event + runs-on: ubuntu-latest + timeout-minutes: 2 + steps: + - name: Record review event + run: | + echo 'This is by automated Angry Turtle Cyborg, not a human' + echo 'Recorded a weighted approval review event.' diff --git a/.github/workflows/weighted-approval.yml b/.github/workflows/weighted-approval.yml index fe0d34deb5..ed57b60d8d 100644 --- a/.github/workflows/weighted-approval.yml +++ b/.github/workflows/weighted-approval.yml @@ -3,8 +3,9 @@ name: weighted-approval on: pull_request_target: types: [opened, synchronize, reopened, ready_for_review, converted_to_draft] - pull_request_review: - types: [submitted, edited, dismissed] + workflow_run: + workflows: [weighted-approval-review-event] + types: [completed] permissions: contents: read @@ -12,11 +13,12 @@ permissions: statuses: write concurrency: - group: weighted-approval-${{ github.event.pull_request.number }} - cancel-in-progress: true + group: weighted-approval-${{ github.event.pull_request.number || github.event.workflow_run.head_sha }} + cancel-in-progress: false jobs: publish-status: + if: github.event_name != 'workflow_run' || github.event.workflow_run.conclusion == 'success' name: publish weighted approval status runs-on: ubuntu-latest timeout-minutes: 5 diff --git a/package.json b/package.json index 929c3788a4..6cdfea5ad8 100644 --- a/package.json +++ b/package.json @@ -56,8 +56,8 @@ "test:bench:built": "vitest run --config vitest.bench.config.ts", "test:expected": "vitest run --config vitest.expected.config.ts", "test:expected:refresh": "DSH_SNAPSHOT=refresh vitest run --config vitest.expected.config.ts", - "test:issue-management": "node .github/issue-management/policy.test.mjs", "test:approval-policy": "node --test .github/review-ownership/check-approval.test.mjs", + "test:issue-management": "node .github/issue-management/policy.test.mjs", "test:request-review": "node --test .github/review-ownership/request-review.test.mjs", "test:snapshot": "vitest run --config vitest.snapshot.config.ts", "test:snapshot:record": "DSH_SNAPSHOT=record vitest run --config vitest.snapshot.config.ts --update", diff --git a/scripts/ci-workflow.spec.ts b/scripts/ci-workflow.spec.ts index 70097a9cf9..6f0cd051da 100644 --- a/scripts/ci-workflow.spec.ts +++ b/scripts/ci-workflow.spec.ts @@ -816,27 +816,44 @@ describe('Request review workflow', () => { }) describe('Weighted approval workflow', () => { - it('publishes a trusted status on pull request and review updates', () => { - const workflow = loadWorkflow('.github/workflows/weighted-approval.yml') - const pullRequest = workflowEvent(workflow, 'pull_request_target') - const review = workflowEvent(workflow, 'pull_request_review') - const job = workflowJob(workflow, 'publish-status') - if (!isRecord(workflow.on)) throw new TypeError('weighted-approval workflow must define events') + it('publishes from the trusted default branch after pull request and review updates', () => { + const publisher = loadWorkflow('.github/workflows/weighted-approval.yml') + const reviewEvent = loadWorkflow('.github/workflows/weighted-approval-review-event.yml') + const pullRequest = workflowEvent(publisher, 'pull_request_target') + const workflowRun = workflowEvent(publisher, 'workflow_run') + const review = workflowEvent(reviewEvent, 'pull_request_review') + const job = workflowJob(publisher, 'publish-status') + const recordJob = workflowJob(reviewEvent, 'record-review-event') + if (!isRecord(publisher.on)) throw new TypeError('weighted-approval workflow must define events') + if (!isRecord(reviewEvent.on)) throw new TypeError('weighted-approval review event workflow must define events') if (!Array.isArray(job.steps)) throw new TypeError('weighted-approval job must define steps') + if (!Array.isArray(recordJob.steps)) throw new TypeError('weighted-approval review event job must define steps') const steps = job.steps.filter(isRecord) const checkout = steps.find(step => step.name === 'Check out trusted approval policy') const publish = steps.find(step => step.name === 'Publish weighted approval status') + const recordSteps = recordJob.steps.filter(isRecord) + const record = recordSteps.find(step => step.name === 'Record review event') - expect(workflow.name).toBe('weighted-approval') - expect(Object.keys(workflow.on)).toEqual(['pull_request_target', 'pull_request_review']) + expect(publisher.name).toBe('weighted-approval') + expect(Object.keys(publisher.on)).toEqual(['pull_request_target', 'workflow_run']) expect(pullRequest.types).toEqual(['opened', 'synchronize', 'reopened', 'ready_for_review', 'converted_to_draft']) + expect(workflowRun).toEqual({ workflows: ['weighted-approval-review-event'], types: ['completed'] }) + expect(reviewEvent.name).toBe('weighted-approval-review-event') + expect(reviewEvent['run-name']).toBe('weighted-approval-review-event:${{ github.event.pull_request.number }}') + expect(Object.keys(reviewEvent.on)).toEqual(['pull_request_review']) expect(review.types).toEqual(['submitted', 'edited', 'dismissed']) - expect(workflow.permissions).toEqual({ contents: 'read', 'pull-requests': 'read', statuses: 'write' }) - expect(workflow.concurrency).toEqual({ - group: 'weighted-approval-${{ github.event.pull_request.number }}', - 'cancel-in-progress': true, + expect(reviewEvent.permissions).toEqual({}) + expect(publisher.permissions).toEqual({ + contents: 'read', + 'pull-requests': 'read', + statuses: 'write', + }) + expect(publisher.concurrency).toEqual({ + group: 'weighted-approval-${{ github.event.pull_request.number || github.event.workflow_run.head_sha }}', + 'cancel-in-progress': false, }) expect(job).toMatchObject({ + if: "github.event_name != 'workflow_run' || github.event.workflow_run.conclusion == 'success'", name: 'publish weighted approval status', 'runs-on': 'ubuntu-latest', 'timeout-minutes': 5, @@ -855,8 +872,18 @@ describe('Weighted approval workflow', () => { }, run: 'node .github/review-ownership/check-approval.mjs', }) - expect(JSON.stringify(workflow)).not.toContain('github.event.pull_request.head') - expect(JSON.stringify(workflow)).not.toContain('secrets.') + expect(recordJob).toMatchObject({ + name: 'record weighted approval review event', + 'runs-on': 'ubuntu-latest', + 'timeout-minutes': 2, + }) + expect(record).toBeDefined() + expect(record?.run).toContain('This is by automated Angry Turtle Cyborg, not a human') + expect(recordSteps).toHaveLength(1) + expect(JSON.stringify(publisher)).not.toContain('github.event.pull_request.head') + expect(JSON.stringify(publisher)).not.toContain('secrets.') + expect(JSON.stringify(reviewEvent)).not.toContain('github.token') + expect(JSON.stringify(reviewEvent)).not.toContain('secrets.') }) }) diff --git a/scripts/run-gates.spec.ts b/scripts/run-gates.spec.ts index eec211e7da..2c5d0afb82 100644 --- a/scripts/run-gates.spec.ts +++ b/scripts/run-gates.spec.ts @@ -246,6 +246,15 @@ describe('gate graph validation', () => { }, ) + it.each(['ci-primary', 'ci-static', 'check-all'] as const)( + 'keeps weighted approval policy tests in %s', + (mode) => { + const ids = withPnpmEntrypoint(() => gatesForMode(mode).map(subject => subject.id)) + + expect(ids).toContain('approval-policy') + }, + ) + it.each(['ci-primary', 'ci-static', 'check-all'] as const)( 'keeps review request policy tests in %s', (mode) => { diff --git a/scripts/run-gates.ts b/scripts/run-gates.ts index a1c022efd0..78425fe990 100644 --- a/scripts/run-gates.ts +++ b/scripts/run-gates.ts @@ -266,8 +266,8 @@ export function gatesForMode(selected: Mode): Gate[] { pnpmScript('cordis-config', 'verify-cordis-config', { label: 'Cordis config' }), pnpmScript('client-domain-graph', 'verify-client-domain-graph', { label: 'client domain graph' }), pnpmScript('test', 'test'), - pnpmScript('issue-management', 'test:issue-management', { label: 'Issue management policy' }), pnpmScript('approval-policy', 'test:approval-policy', { label: 'Weighted approval policy' }), + pnpmScript('issue-management', 'test:issue-management', { label: 'Issue management policy' }), pnpmScript('request-review', 'test:request-review', { label: 'Review request policy' }), pnpmScript('duplication', 'duplication'), snapshotGate(), @@ -311,8 +311,8 @@ function ciSharedStaticGates(): Gate[] { pnpmScript('client-packages', 'verify-client-packages', { label: 'client packages' }), pnpmScript('client-ui-i18n', 'verify-client-ui-i18n', { label: 'client UI i18n' }), pnpmScript('no-bare-dispatcher', 'verify-no-bare-dispatcher', { label: 'proxy-aware dispatchers' }), - pnpmScript('issue-management', 'test:issue-management', { label: 'Issue management policy' }), pnpmScript('approval-policy', 'test:approval-policy', { label: 'Weighted approval policy' }), + pnpmScript('issue-management', 'test:issue-management', { label: 'Issue management policy' }), pnpmScript('request-review', 'test:request-review', { label: 'Review request policy' }), ] } From 3c21d4c28ccfa66c4cf72e982f042ded69f2aad2 Mon Sep 17 00:00:00 2001 From: Turtle Date: Tue, 8 Sep 2026 20:38:10 +0800 Subject: [PATCH 004/164] fix(client): name collapsed Settings trigger --- .../ui-settings-general/README.i18n.yaml | 4 +-- packages/client/ui-settings-general/README.md | 2 +- .../client/ui-settings-general/README.zh.md | 2 +- .../src/client/SettingsRoot.tsx | 5 +-- .../tests/settings-root.client.spec.tsx | 33 +++++++++++-------- 5 files changed, 26 insertions(+), 20 deletions(-) diff --git a/packages/client/ui-settings-general/README.i18n.yaml b/packages/client/ui-settings-general/README.i18n.yaml index e86cb10e45..f5d7229b25 100644 --- a/packages/client/ui-settings-general/README.i18n.yaml +++ b/packages/client/ui-settings-general/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-settings-general/README.md -README.md: c5bf1c1ccf0215e5902e65e66aea465a94350b9c -README.zh.md: 8fa4b48640374f414162a4f129d2ef177efb6287 +README.md: 845d9c48dce2264d478f0ac854ef85a480828a14 +README.zh.md: 4a96e78a13dd8e6aefd759926f6e2cb153f0b259 diff --git a/packages/client/ui-settings-general/README.md b/packages/client/ui-settings-general/README.md index c5bf1c1ccf..845d9c48dc 100644 --- a/packages/client/ui-settings-general/README.md +++ b/packages/client/ui-settings-general/README.md @@ -25,7 +25,7 @@ Use this package to give the dsh web client a Settings panel, connection-recover ## Use this package -Users reach the shell through the sidebar's bottom Settings control; feature plugins contribute their pages and onboarding steps through the slot ledgers this shell projects. A pale-yellow **Disconnected** action beside Settings indicates browser offline suspension. Automatic recovery shows **Reconnecting** with one to three dots advancing every 500ms. Hover or keyboard focus changes either yellow label to **Reconnect now** without changing its background; press feedback stays within the warning palette, and selecting it starts retry 1 immediately. Recovery changes the region to pale-green **Connected** for two seconds before it disappears. The icon, left-aligned text origin, height, and width remain fixed across every visible state. Initial startup and uninterrupted healthy operation remain silent. The shell renders the modal panel, the navigation built from `settings.section` entries, and exactly one mounted onboarding step at a time. +Users reach the shell through the sidebar's bottom Settings control; feature plugins contribute their pages and onboarding steps through the slot ledgers this shell projects. In both the expanded sidebar and collapsed rail, the control exposes the localized Settings label as its accessible name. A pale-yellow **Disconnected** action beside Settings indicates browser offline suspension. Automatic recovery shows **Reconnecting** with one to three dots advancing every 500ms. Hover or keyboard focus changes either yellow label to **Reconnect now** without changing its background; press feedback stays within the warning palette, and selecting it starts retry 1 immediately. Recovery changes the region to pale-green **Connected** for two seconds before it disappears. The icon, left-aligned text origin, height, and width remain fixed across every visible state. Initial startup and uninterrupted healthy operation remain silent. The shell renders the modal panel, the navigation built from `settings.section` entries, and exactly one mounted onboarding step at a time. ### The General section diff --git a/packages/client/ui-settings-general/README.zh.md b/packages/client/ui-settings-general/README.zh.md index 8fa4b48640..4a96e78a13 100644 --- a/packages/client/ui-settings-general/README.zh.md +++ b/packages/client/ui-settings-general/README.zh.md @@ -25,7 +25,7 @@ kind: "package-reference" ## 使用本包 -用户通过侧边栏底部的 Settings 控件进入外壳;功能插件通过本外壳所投影的 slot 账本贡献自己的页面与引导步骤。Settings 右侧浅黄色的**连接异常**操作表示浏览器离线暂停;自动恢复期间显示**自动重连中**,其后一至三个点每 500ms 前进一次。鼠标悬浮或键盘聚焦任一黄色状态时,只有文案变为**立即重连**,背景保持不变;按压反馈留在黄色色阶内,选中后立即从 retry 1 开始。恢复后该区域变为浅绿色的**连接成功**,驻留 2 秒再消失。所有可见状态的文字都左对齐,且图标、文字起点、高度和宽度保持固定。首次启动与未曾中断的健康连接保持静默。外壳渲染模态面板、由 `settings.section` 条目构建的导航,以及每次只挂载一个的引导步骤。 +用户通过侧边栏底部的 Settings 控件进入外壳;功能插件通过本外壳所投影的 slot 账本贡献自己的页面与引导步骤。在展开侧边栏和收起轨道中,该控件都会把本地化的 Settings 文案作为其可访问名称。Settings 右侧浅黄色的**连接异常**操作表示浏览器离线暂停;自动恢复期间显示**自动重连中**,其后一至三个点每 500ms 前进一次。鼠标悬浮或键盘聚焦任一黄色状态时,只有文案变为**立即重连**,背景保持不变;按压反馈留在黄色色阶内,选中后立即从 retry 1 开始。恢复后该区域变为浅绿色的**连接成功**,驻留 2 秒再消失。所有可见状态的文字都左对齐,且图标、文字起点、高度和宽度保持固定。首次启动与未曾中断的健康连接保持静默。外壳渲染模态面板、由 `settings.section` 条目构建的导航,以及每次只挂载一个的引导步骤。 ### 「通用」分区 diff --git a/packages/client/ui-settings-general/src/client/SettingsRoot.tsx b/packages/client/ui-settings-general/src/client/SettingsRoot.tsx index d5b1df190a..e0b20dceed 100644 --- a/packages/client/ui-settings-general/src/client/SettingsRoot.tsx +++ b/packages/client/ui-settings-general/src/client/SettingsRoot.tsx @@ -1,9 +1,9 @@ /** * Settings shell root: the sidebar-foot trigger row plus the centered modal * panel (figma 501:29947, 1080x700) with the section nav rail. The shell is - * a pure composition face — every piece of text (trigger label, panel title, + * a pure composition face — slot-owned text (trigger label, panel title, * close label, sections) arrives from registrants through slots; accessible - * names resolve to that content (trigger: its own text; dialog: + * names resolve from localized content (trigger: shell locale; dialog: * aria-labelledby the title node; close: visually-hidden slot text). Modal * open state and the active section id are component-local viewing state; * the onboarding coordinator mounts exactly one ordered registrant while the @@ -184,6 +184,7 @@ export function SettingsRoot(props: SettingsRootComponentProps) { ref={triggerButton} type="button" className={clsx(css.trigger, !wide && css.rail)} + aria-label={t('trigger')} aria-haspopup="dialog" aria-expanded={open} onClick={() => { setOpen(true) }} diff --git a/packages/client/ui-settings-general/tests/settings-root.client.spec.tsx b/packages/client/ui-settings-general/tests/settings-root.client.spec.tsx index 3ebce030b4..18d96544a0 100644 --- a/packages/client/ui-settings-general/tests/settings-root.client.spec.tsx +++ b/packages/client/ui-settings-general/tests/settings-root.client.spec.tsx @@ -6,7 +6,7 @@ import { act, cleanup, fireEvent, render, screen } from '@testing-library/react' import { makeTranslate } from '@deepseek-ai/dsh-client-test-runtime' import type { SettingsRootComponentProps } from '../src/client/shell-contract.ts' import { SettingsRoot } from '../src/client/SettingsRoot.tsx' -import { en } from '../src/client/locales.ts' +import { en, zh } from '../src/client/locales.ts' // Every fixture carries the resource hook the resources plugin merges into GlobalStandardProps. const useResource = (() => ({ status: 'none' as const, value: undefined, failure: undefined, reload: () => {} })) as GlobalStandardProps['useResource'] @@ -34,6 +34,7 @@ const useSessionPendingInteraction: SettingsRootComponentProps['useSessionPendin function mount({ wide = true, + dictionary = en, connectionState = 'connected', onboardingActive = true, rows = [ @@ -47,6 +48,7 @@ function mount({ ], }: { wide?: boolean + dictionary?: typeof en | typeof zh connectionState?: ConnectionSnapshot onboardingActive?: boolean rows?: Row[] @@ -80,7 +82,7 @@ function mount({ useWorkspaces: unusedHook, wide, reconnect, - t: makeTranslate(en), + t: makeTranslate(dictionary), useConnectionState: (select) => { const [, force] = useState(0) useEffect(() => { @@ -126,20 +128,23 @@ function openPanel() { } describe('SettingsRoot trigger', () => { - it('renders the trigger seat content as the accessible name (no aria-label of its own)', () => { - const { renderSlot } = mount() - const trigger = screen.getByRole('button', { name: 'Settings' }) - expect(trigger.hasAttribute('aria-label')).toBe(false) - expect(renderSlot).toHaveBeenCalledWith('settings.trigger', { wide: true }) + it.each([ + { column: 'expanded English', wide: true, dictionary: en, name: 'Settings' }, + { column: 'collapsed English', wide: false, dictionary: en, name: 'Settings' }, + { column: 'expanded Chinese', wide: true, dictionary: zh, name: '设置' }, + { column: 'collapsed Chinese', wide: false, dictionary: zh, name: '设置' }, + ])('uses the locale name and accepts keyboard-style activation for the $column trigger', ({ + wide, dictionary, name, + }) => { + const { renderSlot } = mount({ wide, dictionary }) + const trigger = screen.getByRole('button', { name }) + expect(trigger.getAttribute('aria-label')).toBe(name) + expect(renderSlot).toHaveBeenCalledWith('settings.trigger', { wide }) expect(trigger.getAttribute('aria-expanded')).toBe('false') - fireEvent.click(trigger) + trigger.focus() + fireEvent.click(trigger, { detail: 0 }) expect(screen.getByRole('dialog')).toBeTruthy() - expect(screen.getByRole('button', { name: 'Settings', expanded: true })).toBeTruthy() - }) - - it('hands the rail state to the trigger seat', () => { - const { renderSlot } = mount({ wide: false }) - expect(renderSlot).toHaveBeenCalledWith('settings.trigger', { wide: false }) + expect(screen.getByRole('button', { name, expanded: true })).toBeTruthy() }) it('shows outage, retry progress, and a two-second recovery confirmation', () => { From 555c04bdfcda76fd023c71668bb00cfd70ea8c25 Mon Sep 17 00:00:00 2001 From: Turtle Date: Tue, 8 Sep 2026 21:05:31 +0800 Subject: [PATCH 005/164] fix(web): localize Chinese model copy --- ...8-23-locale-owned-client-ui-copy.i18n.yaml | 4 +- .../2026-08-23-locale-owned-client-ui-copy.md | 4 +- ...26-08-23-locale-owned-client-ui-copy.zh.md | 4 +- .../settings-chrome/dialog.expected.md | 4 +- apps/web/tests/settings-chrome.e2e.ts | 14 +++--- packages/client/ui-chat/README.i18n.yaml | 4 +- packages/client/ui-chat/README.md | 2 +- packages/client/ui-chat/README.zh.md | 2 +- packages/client/ui-chat/src/client/locale.ts | 4 +- .../tests/transcript-view-row.client.spec.tsx | 13 ++++-- .../ui-model-selection/README.i18n.yaml | 4 +- packages/client/ui-model-selection/README.md | 2 +- .../client/ui-model-selection/README.zh.md | 2 +- .../ui-model-selection/src/client/index.ts | 17 +++++++- .../ui-model-selection/src/client/locales.ts | 4 ++ .../tests/browser-plugin.client.spec.ts | 43 +++++++++++++++---- 16 files changed, 92 insertions(+), 35 deletions(-) diff --git a/.agents/notes/implemented/architecture/2026-08-23-locale-owned-client-ui-copy.i18n.yaml b/.agents/notes/implemented/architecture/2026-08-23-locale-owned-client-ui-copy.i18n.yaml index 49ee6b91b8..1aa080d1fe 100644 --- a/.agents/notes/implemented/architecture/2026-08-23-locale-owned-client-ui-copy.i18n.yaml +++ b/.agents/notes/implemented/architecture/2026-08-23-locale-owned-client-ui-copy.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 .agents/notes/implemented/architecture/2026-08-23-locale-owned-client-ui-copy.md -2026-08-23-locale-owned-client-ui-copy.md: 623189b8faa45f0ef2a1b724d3e050d33f7d59bd -2026-08-23-locale-owned-client-ui-copy.zh.md: e2a6f49de89e6e6ba2734dd69cf536a3e13d2aea +2026-08-23-locale-owned-client-ui-copy.md: bc4d8efbce1f246f712e3afe6af40d1518e8cb17 +2026-08-23-locale-owned-client-ui-copy.zh.md: ce996a4aafc392e739bec2eccd8714cfeff7d8e9 diff --git a/.agents/notes/implemented/architecture/2026-08-23-locale-owned-client-ui-copy.md b/.agents/notes/implemented/architecture/2026-08-23-locale-owned-client-ui-copy.md index 623189b8fa..bc4d8efbce 100644 --- a/.agents/notes/implemented/architecture/2026-08-23-locale-owned-client-ui-copy.md +++ b/.agents/notes/implemented/architecture/2026-08-23-locale-owned-client-ui-copy.md @@ -12,6 +12,8 @@ Typed locale namespaces and bilingual dictionary parity proved that registered d **Locale dictionaries own all product-authored client UI wording.** Visible text, accessibility names, tooltips, placeholders, empty states, status labels, units, and formatting templates reach presentation through a typed `t` seat or an already-localized prop. A value authored by a user, model, provider, plugin, wire peer, or operating system remains data and renders verbatim; protocol tags, tool names, paths, URLs, JSON/JavaScript literals, and stable internal ids are not translated. +Product-owned catalog descriptions follow the same rule. The client maps an exact built-in provider, model, and description to a locale key; a changed description or an external provider description remains provider data and renders verbatim. + **Cordis-free primitives require complete localized copy props and own no language fallback.** `MarkdownText`, `JsonTree`, `TerminalBlock`, `DiffBlock`, `ReadBlock`, `SearchBlock`, `WebBlock`, `CodeBlock`, `JsonBlock`, `HoverCard`, and `ConnectionIndicator` receive their chrome from the feature render site. This preserves the primitive package's runtime independence while making omission a type error instead of silently selecting Chinese or English. Shared words live in the `common` namespace; feature-specific phrases stay with the feature that decides their meaning. **Localized display text is never an identity.** Models and stores retain discriminants, stable ids, and non-display markers. Renderers translate after matching, and request maps carry stable group membership into the trajectory ledger. A client-synthesized error that must survive in a view model uses a stable marker and is translated only when displayed. Language switching therefore changes wording without changing selection, grouping, search identity, or lifecycle state. @@ -22,7 +24,7 @@ The product-authored error and design-literal exclusions, primitive defaults, an ## Verification -The AST check's own Vitest spec pins direct JSX, template branches, semantic copy props, label data, formatter returns, locale-key calls, structural attributes, and dictionary owners. Locale dictionary parity pins identical `zh`/`en` keys. Client component suites exercise both direct translated seats and locale-prop adapters, and the assembled web replay plus the required real-server GIF demonstrate the shipped locale switch on the actual trajectory surface. +The AST check's own Vitest spec pins direct JSX, template branches, semantic copy props, label data, formatter returns, locale-key calls, structural attributes, and dictionary owners. Locale dictionary parity pins identical `zh`/`en` keys. Client suites exercise direct translated seats, locale-prop adapters, and built-in catalog-description localization without altering external descriptions. The assembled web replay plus the required real-server GIF demonstrate the shipped locale switch on the actual trajectory surface. ## Alternatives considered diff --git a/.agents/notes/implemented/architecture/2026-08-23-locale-owned-client-ui-copy.zh.md b/.agents/notes/implemented/architecture/2026-08-23-locale-owned-client-ui-copy.zh.md index e2a6f49de8..ce996a4aaf 100644 --- a/.agents/notes/implemented/architecture/2026-08-23-locale-owned-client-ui-copy.zh.md +++ b/.agents/notes/implemented/architecture/2026-08-23-locale-owned-client-ui-copy.zh.md @@ -12,6 +12,8 @@ typed locale namespace 与双语字典对等性可以证明已注册字典完整 **所有产品编写的 client UI 措辞都由 locale 字典持有。** 可见文本、无障碍名称、tooltip、placeholder、空状态、状态标签、单位和格式模板必须经 typed `t` 席位或已本地化 prop 到达展示层。由用户、模型、提供方、插件、wire 对端或操作系统编写的值仍是数据并原样渲染;协议 tag、工具名称、路径、URL、JSON/JavaScript 字面量和稳定内部 id 不翻译。 +产品持有的目录说明遵循同一规则。client 将完全匹配的内置提供方、模型与说明映射到 locale key;说明发生变化或来自外部提供方时,它仍是提供方数据并原样渲染。 + **Cordis-free 原子组件要求完整的本地化文案 prop,且自身不持有语言回落值。** `MarkdownText`、`JsonTree`、`TerminalBlock`、`DiffBlock`、`ReadBlock`、`SearchBlock`、`WebBlock`、`CodeBlock`、`JsonBlock`、`HoverCard` 与 `ConnectionIndicator` 的 chrome 均由功能渲染点传入。这样既保留原子组件包的运行时独立性,也让遗漏成为类型错误,而不是静默选择中文或英文。共享用词进入 `common` namespace;功能专属短语留在决定其语义的功能侧。 **本地化展示文本绝不承担身份。** 模型与存储保留判别字段、稳定 id 和非展示 marker。渲染器先匹配再翻译,请求映射通过稳定的组成员关系进入 trajectory ledger。必须保存在视图模型中的 client 合成错误使用稳定 marker,只在展示时翻译。因此语言切换只改变措辞,不改变选择、分组、搜索身份或生命周期状态。 @@ -22,7 +24,7 @@ typed locale namespace 与双语字典对等性可以证明已注册字典完整 ## Verification -AST 检查自身的 Vitest spec 固定直接 JSX、模板分支、语义文案 prop、label 数据、格式化函数返回值、locale key 调用、结构属性和字典 owner。locale 字典对等性固定 `zh`/`en` key 一致。client 组件测试同时覆盖直接翻译席位与 locale prop 适配器;组装 web 回放和规定的真实服务器 GIF 在实际 trajectory 界面上展示发布的语言切换。 +AST 检查自身的 Vitest spec 固定直接 JSX、模板分支、语义文案 prop、label 数据、格式化函数返回值、locale key 调用、结构属性和字典 owner。locale 字典对等性固定 `zh`/`en` key 一致。client 测试覆盖直接翻译席位、locale prop 适配器,以及不改变外部说明的内置目录说明本地化。组装 web 回放和规定的真实服务器 GIF 在实际 trajectory 界面上展示发布的语言切换。 ## Alternatives considered diff --git a/apps/web/tests/expected/settings-chrome/dialog.expected.md b/apps/web/tests/expected/settings-chrome/dialog.expected.md index 2f620a88d4..f80893bfb2 100644 --- a/apps/web/tests/expected/settings-chrome/dialog.expected.md +++ b/apps/web/tests/expected/settings-chrome/dialog.expected.md @@ -41,8 +41,8 @@ - button "减小字号": - img - text: px 对话显示 控制已完成轮次的过程内容 - - button "Compact": - - text: Compact + - button "紧凑": + - text: 紧凑 - img - text: 繁忙时的发送行为 智能体运行时 Enter 键和发送按钮的行为;Cmd/Ctrl+Enter 使用另一行为 - button "排队发送": diff --git a/apps/web/tests/settings-chrome.e2e.ts b/apps/web/tests/settings-chrome.e2e.ts index f3be2fcd39..f1db2bb51f 100644 --- a/apps/web/tests/settings-chrome.e2e.ts +++ b/apps/web/tests/settings-chrome.e2e.ts @@ -424,9 +424,9 @@ describe('web e2e: settings modal and General preferences', () => { const dialog = page.getByRole('dialog', { name: '设置' }) await dialog.waitFor({ timeout: 10_000 }) await dialog.getByText('对话显示', { exact: true }).waitFor({ timeout: 10_000 }) - await dialog.getByRole('button', { name: 'Compact', exact: true }).click() - await page.getByRole('menuitem', { name: 'Normal', exact: true }).click() - await dialog.getByRole('button', { name: 'Normal', exact: true }).waitFor({ timeout: 10_000 }) + await dialog.getByRole('button', { name: '紧凑', exact: true }).click() + await page.getByRole('menuitem', { name: '标准', exact: true }).click() + await dialog.getByRole('button', { name: '标准', exact: true }).waitFor({ timeout: 10_000 }) await expect.poll(async () => readFile(join(scaffold.harnessHome, 'settings.yaml'), 'utf8'), { timeout: 5_000 }) .toMatch(/ui-chat:\n\s+transcriptView: normal/) await page.keyboard.press('Escape') @@ -437,11 +437,11 @@ describe('web e2e: settings modal and General preferences', () => { acknowledgeReloadConnectionLoss(tripwire, warningStart) await page.getByRole('button', { name: '设置', exact: true }).click() const reloaded = page.getByRole('dialog', { name: '设置' }) - await reloaded.getByRole('button', { name: 'Normal', exact: true }).waitFor({ timeout: 10_000 }) + await reloaded.getByRole('button', { name: '标准', exact: true }).waitFor({ timeout: 10_000 }) - await reloaded.getByRole('button', { name: 'Normal', exact: true }).click() - await page.getByRole('menuitem', { name: 'Compact', exact: true }).click() - await reloaded.getByRole('button', { name: 'Compact', exact: true }).waitFor({ timeout: 10_000 }) + await reloaded.getByRole('button', { name: '标准', exact: true }).click() + await page.getByRole('menuitem', { name: '紧凑', exact: true }).click() + await reloaded.getByRole('button', { name: '紧凑', exact: true }).waitFor({ timeout: 10_000 }) await expect.poll(async () => readFile(join(scaffold.harnessHome, 'settings.yaml'), 'utf8'), { timeout: 5_000 }) .toMatch(/ui-chat:\n\s+transcriptView: compact/) await page.keyboard.press('Escape') diff --git a/packages/client/ui-chat/README.i18n.yaml b/packages/client/ui-chat/README.i18n.yaml index eed23f8972..40a2dc51ea 100644 --- a/packages/client/ui-chat/README.i18n.yaml +++ b/packages/client/ui-chat/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-chat/README.md -README.md: 91618e226dd25c8629ff103ac5a958e688aeed26 -README.zh.md: 4f9629c8974fbee9e56ed003ebd57bbc3b029577 +README.md: 62b6a8c8644953010ba61dc19780c5e8a5a01991 +README.zh.md: 9aa8cec85bde83130ecf06806f9831d491f09ea5 diff --git a/packages/client/ui-chat/README.md b/packages/client/ui-chat/README.md index 91618e226d..62b6a8c864 100644 --- a/packages/client/ui-chat/README.md +++ b/packages/client/ui-chat/README.md @@ -39,7 +39,7 @@ A completed Turn shows an expandable usage row only when the loaded window inclu ## Turn Process Folding -Settings → General exposes a persisted `Normal` / `Compact` conversation-display preference in the `ui-chat` namespace; `Compact` is the default. Normal leaves process rows visible and renders no Turn-process control. In Compact mode, the System prompt remains independently visible before the opening User throughout the Turn. Context injection, reasoning, Assistant material, Tool rows, and Retry rows remain expanded while a Turn is open. At `turn/end`, its latest Step becomes the final-answer boundary only when it contains non-blank text, an image, or an unknown visible block—and no Tool-call block; preceding Context injection, reasoning, earlier Assistant material, Tool rows, and Retry rows then collapse by default. The control reports Turn-wide durable counts for non-subagent Tool calls, reply-bearing Assistant messages before the final answer, and subagent delegation calls; zero-valued segments are omitted, the Tool and subagent figures are mutually exclusive, and neither System prompt nor Context injection contributes a count. When all three counts are zero, the process still folds and the control reads `Thought for a while`. A full-width divider below the summary separates it from the answer or expanded process rows. User and steering messages, System prompt, error, max-token, and turn-tail rows stay outside, and a closed Turn with no final answer keeps all process evidence visible. A newly available process control is inserted without changing the relative order of existing rows: opening human input precedes the control and process rows from their first projection, while System prompt remains above that input. While older history remains available through Load earlier, process controls stay absent and no members are hidden; once history is complete, every eligible closed Turn uses the collapsed default immediately. Stable Chat Node Seats keep every renderer mounted, hidden members add no flow spacing, and a closed control sits 8px above its answer only when no independent input intervenes. Completion collapse does not depend on tail-follow position, so a reader above the tail may see the transcript reflow. An automatic collapse that would hide keyboard focus keeps the group open and leaves focus in place; a manual close focuses the process control before hiding its members. The session-scoped store records only manually expanded Turn-and-answer-Step generations; a different answer generation starts collapsed. +Settings → General exposes a persisted, localized `Normal` / `Compact` conversation-display preference in the `ui-chat` namespace; `Compact` is the default. Normal leaves process rows visible and renders no Turn-process control. In Compact mode, the System prompt remains independently visible before the opening User throughout the Turn. Context injection, reasoning, Assistant material, Tool rows, and Retry rows remain expanded while a Turn is open. At `turn/end`, its latest Step becomes the final-answer boundary only when it contains non-blank text, an image, or an unknown visible block—and no Tool-call block; preceding Context injection, reasoning, earlier Assistant material, Tool rows, and Retry rows then collapse by default. The control reports Turn-wide durable counts for non-subagent Tool calls, reply-bearing Assistant messages before the final answer, and subagent delegation calls; zero-valued segments are omitted, the Tool and subagent figures are mutually exclusive, and neither System prompt nor Context injection contributes a count. When all three counts are zero, the process still folds and the control reads `Thought for a while`. A full-width divider below the summary separates it from the answer or expanded process rows. User and steering messages, System prompt, error, max-token, and turn-tail rows stay outside, and a closed Turn with no final answer keeps all process evidence visible. A newly available process control is inserted without changing the relative order of existing rows: opening human input precedes the control and process rows from their first projection, while System prompt remains above that input. While older history remains available through Load earlier, process controls stay absent and no members are hidden; once history is complete, every eligible closed Turn uses the collapsed default immediately. Stable Chat Node Seats keep every renderer mounted, hidden members add no flow spacing, and a closed control sits 8px above its answer only when no independent input intervenes. Completion collapse does not depend on tail-follow position, so a reader above the tail may see the transcript reflow. An automatic collapse that would hide keyboard focus keeps the group open and leaves focus in place; a manual close focuses the process control before hiding its members. The session-scoped store records only manually expanded Turn-and-answer-Step generations; a different answer generation starts collapsed. ----- diff --git a/packages/client/ui-chat/README.zh.md b/packages/client/ui-chat/README.zh.md index 4f9629c897..9aa8cec85b 100644 --- a/packages/client/ui-chat/README.zh.md +++ b/packages/client/ui-chat/README.zh.md @@ -39,7 +39,7 @@ Chat 会为非空的初始请求、显式消息序列起点、真实 system 字 ## 轮次过程折叠 -「设置 → 通用设置」提供持久化到 `ui-chat` 命名空间的 `Normal` / `Compact` 对话显示偏好,默认使用 `Compact`。Normal 保持所有过程行可见且不渲染轮次过程控件。Compact 模式下,系统提示词在整个轮次中始终独立显示于开场 User 上方。轮次打开期间,上下文注入、推理、Assistant 内容、工具行与重试行始终展开。到 `turn/end` 时,最后一个步骤只有在包含非空文本、图片或未知可见块且不含工具调用块时才成为最终正文边界;边界之前的上下文注入、推理、较早 Assistant 内容、工具行与重试行随后默认收起。控件展示覆盖整个轮次的非 subagent 工具调用数、最终正文之前带回复内容的 Assistant 消息数和 subagent 委派数;值为 0 的分段省略,工具调用与 subagent 两项互斥,系统提示词与上下文注入都不增加计数。三项全为 0 时过程仍会收起,控件标题显示「已思考」(英文为 `Thought for a while`)。摘要下方的通栏分隔线将其与正文或展开后的过程行隔开。用户与 steering 消息、系统提示词、错误、最大 token 与 turn-tail 行留在过程组外;关闭时没有最终正文的轮次保留全部过程证据。新的过程控件插入时不会改变既有行的相对顺序:开场人工输入从首次投影起便位于控件和过程行之前,系统提示词则始终位于该输入上方。只要仍可通过「加载更早」获取历史,过程控件就不出现,也不会隐藏任何成员;历史加载完整后,每个合格的已关闭轮次立即使用默认收起状态。稳定 Chat Node Seat 会让每个 renderer 保持挂载,隐藏成员不产生消息流间距;只有中间没有独立输入时,收起控件才与正文相隔 8px。完成后的收起不依赖是否跟随尾部,因此正在上方阅读的用户可能看到 transcript 高度变化。若自动收起会隐藏当前键盘焦点,则过程组保持展开且焦点留在原处;手动收起会先把焦点移到过程控件,再隐藏成员。会话作用域 store 只记录用户手动展开的「轮次 + 正文步骤」generation;不同正文 generation 默认收起。 +「设置 → 通用设置」提供持久化到 `ui-chat` 命名空间的「标准」/「紧凑」对话显示偏好,默认使用「紧凑」。标准模式保持所有过程行可见且不渲染轮次过程控件。紧凑模式下,系统提示词在整个轮次中始终独立显示于开场 User 上方。轮次打开期间,上下文注入、推理、Assistant 内容、工具行与重试行始终展开。到 `turn/end` 时,最后一个步骤只有在包含非空文本、图片或未知可见块且不含工具调用块时才成为最终正文边界;边界之前的上下文注入、推理、较早 Assistant 内容、工具行与重试行随后默认收起。控件展示覆盖整个轮次的非 subagent 工具调用数、最终正文之前带回复内容的 Assistant 消息数和 subagent 委派数;值为 0 的分段省略,工具调用与 subagent 两项互斥,系统提示词与上下文注入都不增加计数。三项全为 0 时过程仍会收起,控件标题显示「已思考」(英文为 `Thought for a while`)。摘要下方的通栏分隔线将其与正文或展开后的过程行隔开。用户与 steering 消息、系统提示词、错误、最大 token 与 turn-tail 行留在过程组外;关闭时没有最终正文的轮次保留全部过程证据。新的过程控件插入时不会改变既有行的相对顺序:开场人工输入从首次投影起便位于控件和过程行之前,系统提示词则始终位于该输入上方。只要仍可通过「加载更早」获取历史,过程控件就不出现,也不会隐藏任何成员;历史加载完整后,每个合格的已关闭轮次立即使用默认收起状态。稳定 Chat Node Seat 会让每个 renderer 保持挂载,隐藏成员不产生消息流间距;只有中间没有独立输入时,收起控件才与正文相隔 8px。完成后的收起不依赖是否跟随尾部,因此正在上方阅读的用户可能看到 transcript 高度变化。若自动收起会隐藏当前键盘焦点,则过程组保持展开且焦点留在原处;手动收起会先把焦点移到过程控件,再隐藏成员。会话作用域 store 只记录用户手动展开的「轮次 + 正文步骤」generation;不同正文 generation 默认收起。 ----- diff --git a/packages/client/ui-chat/src/client/locale.ts b/packages/client/ui-chat/src/client/locale.ts index e426a56a14..4da8226a3c 100644 --- a/packages/client/ui-chat/src/client/locale.ts +++ b/packages/client/ui-chat/src/client/locale.ts @@ -29,8 +29,8 @@ export const zh = { 'chat.turnNavigation.turn': '第 {turn} 轮', 'settings.transcript.title': '对话显示', 'settings.transcript.description': '控制已完成轮次的过程内容', - 'settings.transcript.normal': 'Normal', - 'settings.transcript.compact': 'Compact', + 'settings.transcript.normal': '标准', + 'settings.transcript.compact': '紧凑', 'fileOpen.title': '无法打开文件', 'fileOpen.unknown': '无法打开此文件', 'message.extraBlock': '附加内容块', diff --git a/packages/client/ui-chat/tests/transcript-view-row.client.spec.tsx b/packages/client/ui-chat/tests/transcript-view-row.client.spec.tsx index 5aa8e31047..20a18b0889 100644 --- a/packages/client/ui-chat/tests/transcript-view-row.client.spec.tsx +++ b/packages/client/ui-chat/tests/transcript-view-row.client.spec.tsx @@ -8,7 +8,7 @@ import type { GlobalStandardProps } from '@deepseek-ai/dsh-client-ui-slots' import { createSnapshotStore } from '@deepseek-ai/dsh-client-store' import { bindSnapshotSelector, makeTranslate } from '@deepseek-ai/dsh-client-test-runtime' import { TranscriptViewRow, type TranscriptViewRowProps } from '../src/client/settings/TranscriptViewRow.tsx' -import { en } from '../src/client/locale.ts' +import { en, zh } from '../src/client/locale.ts' afterEach(cleanup) @@ -31,7 +31,7 @@ function noPendingInteraction() { // The resource hook the resources plugin merges into GlobalStandardProps; this row reads no address. const useResource = (() => ({ status: 'none' as const, value: undefined, failure: undefined, reload: () => {} })) as GlobalStandardProps['useResource'] -function mount(mode: 'normal' | 'compact' = 'compact') { +function mount(mode: 'normal' | 'compact' = 'compact', dictionary: typeof en | typeof zh = en) { const source = createSnapshotStore(mode) const setTranscriptView = vi.fn((next: 'normal' | 'compact') => { source.set(next) }) const props: TranscriptViewRowProps = { @@ -41,7 +41,7 @@ function mount(mode: 'normal' | 'compact' = 'compact') { useResource, useTranscriptView: bindSnapshotSelector(source), setTranscriptView, - t: makeTranslate(en), + t: makeTranslate(dictionary), } render() return { setTranscriptView } @@ -66,4 +66,11 @@ describe('TranscriptViewRow', () => { fireEvent.pointerDown(document.body) expect(screen.queryByRole('menuitem', { name: 'Compact' })).toBeNull() }) + + it('shows the conversation-display values in Chinese', () => { + mount('compact', zh) + fireEvent.click(screen.getByRole('button', { name: '紧凑' })) + fireEvent.click(screen.getByRole('menuitem', { name: '标准' })) + expect(screen.getByRole('button', { name: '标准' })).toBeDefined() + }) }) diff --git a/packages/client/ui-model-selection/README.i18n.yaml b/packages/client/ui-model-selection/README.i18n.yaml index 24e6e2c6b7..4673103622 100644 --- a/packages/client/ui-model-selection/README.i18n.yaml +++ b/packages/client/ui-model-selection/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-model-selection/README.md -README.md: b050f1fa7e680b7669e208d88851754aa72f09e2 -README.zh.md: 85b4d44a1beb45a5a11fa946d2f36d83f9201e29 +README.md: ae939ce678bcc50aa69c6c260f1782830eea103f +README.zh.md: d9826f55a48f141f8baa65f230e24ea8d0254e30 diff --git a/packages/client/ui-model-selection/README.md b/packages/client/ui-model-selection/README.md index b050f1fa7e..ae939ce678 100644 --- a/packages/client/ui-model-selection/README.md +++ b/packages/client/ui-model-selection/README.md @@ -29,7 +29,7 @@ Mount this plugin alongside `ui-conversation` and the commands package; the comp ### Model and effort -Models stay grouped by provider. The menu shows model and effort names only; catalog descriptions remain available to other consumers. The `/model` popup applies the selected model's default effort; the composer can then choose any advertised effort. An adapter without reasoning metadata leaves the Effort row absent; there is no arbitrary effort input. +Models stay grouped by provider. The composer menu shows model and effort names only. The `/model` popup shows provider names and catalog descriptions; it localizes the two built-in DeepSeek descriptions and leaves external provider descriptions verbatim. The popup applies the selected model's default effort; the composer can then choose any advertised effort. An adapter without reasoning metadata leaves the Effort row absent; there is no arbitrary effort input. ### Unroutable sessions diff --git a/packages/client/ui-model-selection/README.zh.md b/packages/client/ui-model-selection/README.zh.md index 85b4d44a1b..d9826f55a4 100644 --- a/packages/client/ui-model-selection/README.zh.md +++ b/packages/client/ui-model-selection/README.zh.md @@ -29,7 +29,7 @@ Web GUI 允许用户通过 `/model` 弹窗或 composer 模型控件切换既有 ### 模型与推理强度 -模型按提供方分组。菜单只显示模型与推理强度名称;目录中的说明仍可供其他消费方使用。`/model` 弹窗应用所选模型的默认推理强度;composer 随后可以选择任一已公布的推理强度。适配器没有推理元数据时不显示 Effort 行;不存在任意推理强度输入。 +模型按提供方分组。composer 菜单只显示模型与推理强度名称。`/model` 弹窗显示提供方名称与目录说明;其中两个内置 DeepSeek 模型的说明使用当前语言,外部提供方说明保持原文。弹窗应用所选模型的默认推理强度;composer 随后可以选择任一已公布的推理强度。适配器没有推理元数据时不显示 Effort 行;不存在任意推理强度输入。 ### 不可路由的会话 diff --git a/packages/client/ui-model-selection/src/client/index.ts b/packages/client/ui-model-selection/src/client/index.ts index b6c9e0562b..5d545451a6 100644 --- a/packages/client/ui-model-selection/src/client/index.ts +++ b/packages/client/ui-model-selection/src/client/index.ts @@ -46,15 +46,30 @@ function rowId(providerId: string, modelId: string): string { return `${providerId}/${modelId}` } +const BUILTIN_DESCRIPTION_KEYS: Readonly> = { + 'deepseek-official/deepseek-v4-flash': 'option.deepseekV4Flash.description', + 'deepseek-official/deepseek-v4-pro': 'option.deepseekV4Pro.description', +} + +function descriptionOf( + providerId: string, + model: ModelDirectoryState['groups'][number]['models'][number], + t: TranslateNS<'model'>, +): string | undefined { + const key = BUILTIN_DESCRIPTION_KEYS[rowId(providerId, model.id)] + return key !== undefined && model.description === en[key] ? t(key) : model.description +} + /** Flatten the directory into popup rows; failure rows are listed for visibility but never selectable. */ function optionsOf(directory: ModelDirectoryState, t: TranslateNS<'model'>): SelectOption[] { const rows: SelectOption[] = [] for (const group of directory.groups) { for (const model of group.models) { + const description = descriptionOf(group.id, model, t) rows.push({ id: rowId(group.id, model.id), label: model.name, - detail: model.description !== undefined ? `${group.name} · ${model.description}` : group.name, + detail: description !== undefined ? `${group.name} · ${description}` : group.name, ...(directory.current !== null && directory.current.provider === group.id && directory.current.model === model.id diff --git a/packages/client/ui-model-selection/src/client/locales.ts b/packages/client/ui-model-selection/src/client/locales.ts index 3bf0774679..bf4b1ff922 100644 --- a/packages/client/ui-model-selection/src/client/locales.ts +++ b/packages/client/ui-model-selection/src/client/locales.ts @@ -12,6 +12,8 @@ export const zh = { 'command.description': '选择本会话使用的模型', 'option.loadError': '目录加载失败:{message}', + 'option.deepseekV4Flash.description': '快速、高效且经济;适合目标明确、常规或并行任务。', + 'option.deepseekV4Pro.description': '更强的自主编码、知识与复杂推理能力;适合复杂或质量优先的任务,但成本更高。', 'trigger.fallback': '选择模型', 'trigger.loading': '正在加载模型…', 'trigger.selectAria': '选择模型', @@ -37,6 +39,8 @@ export type ModelKey = keyof typeof zh export const en = { 'command.description': 'Select the model for this conversation', 'option.loadError': 'Catalog failed to load: {message}', + 'option.deepseekV4Flash.description': 'Fast, efficient, and economical; suited to focused, routine, or parallel tasks.', + 'option.deepseekV4Pro.description': 'Stronger agentic coding, knowledge, and difficult reasoning; suited to complex or quality-critical tasks at higher cost.', 'trigger.fallback': 'Select model', 'trigger.loading': 'Loading models…', 'trigger.selectAria': 'Select model', diff --git a/packages/client/ui-model-selection/tests/browser-plugin.client.spec.ts b/packages/client/ui-model-selection/tests/browser-plugin.client.spec.ts index 1bf11733b5..c3523b8f6d 100644 --- a/packages/client/ui-model-selection/tests/browser-plugin.client.spec.ts +++ b/packages/client/ui-model-selection/tests/browser-plugin.client.spec.ts @@ -30,6 +30,7 @@ const GROUPS = [{ { id: 'deepseek-v4-flash', name: 'DeepSeek-V4-Flash', + description: 'Fast, efficient, and economical; suited to focused, routine, or parallel tasks.', reasoning: { efforts: [ { id: 'off', name: 'Off' }, @@ -42,6 +43,7 @@ const GROUPS = [{ { id: 'deepseek-v4-pro', name: 'DeepSeek-V4-Pro', + description: 'Stronger agentic coding, knowledge, and difficult reasoning; suited to complex or quality-critical tasks at higher cost.', reasoning: { efforts: [ { id: 'off', name: 'Off' }, @@ -52,10 +54,18 @@ const GROUPS = [{ }, }, ], +}, { + id: 'external', + name: 'External Provider', + models: [{ + id: 'deepseek-v4-flash', + name: 'External Flash', + description: 'Provider-authored description.', + }], }] /** Boot the plugin over fake faces + a stateful fake host (current moves on selectModel). */ -async function bench() { +async function bench(locale: 'zh' | 'en' = 'zh') { const ctx = new Context() let defaultSelection: ModelSelection = { provider: 'deepseek-official', model: 'deepseek-v4-flash' } let selected = defaultSelection @@ -117,10 +127,9 @@ async function bench() { }, }) const localeRuntime = new LocaleRuntime(ctx) - // This spec asserts the shipped Chinese copy. There is no jsdom `window` in - // this lane, so browser-language detection never runs and the locale comes - // from FALLBACK_LOCALE (en): state the asserted locale explicitly. - localeRuntime.setLocale('zh') + // There is no jsdom `window` in this lane, so browser-language detection + // never runs. Each bench states the locale its assertions require. + localeRuntime.setLocale(locale) ctx.provide('locale', localeRuntime) const scopes = new Map() const addressed = new Set() @@ -179,15 +188,33 @@ describe('ui-model-selection dual entry', () => { expect(b.seat().locale).toBe('model') }) - it('popup options mark the host current active with the provider group in the detail', async () => { + it('localizes built-in descriptions and preserves external provider descriptions', async () => { const b = await bench() b.mint('s1') const options = await b.contribution().ui.options(projection('s1'), new AbortController().signal) - expect(options.map((o: SelectOption) => o.label)).toEqual(['DeepSeek-V4-Flash', 'DeepSeek-V4-Pro']) - expect(options[0]).toMatchObject({ active: true, detail: 'DeepSeek' }) + expect(options.map((o: SelectOption) => o.label)).toEqual([ + 'DeepSeek-V4-Flash', 'DeepSeek-V4-Pro', 'External Flash', + ]) + expect(options[0]).toMatchObject({ + active: true, + detail: 'DeepSeek · 快速、高效且经济;适合目标明确、常规或并行任务。', + }) + expect(options[1]?.detail) + .toBe('DeepSeek · 更强的自主编码、知识与复杂推理能力;适合复杂或质量优先的任务,但成本更高。') + expect(options[2]?.detail).toBe('External Provider · Provider-authored description.') expect(options[1]?.active).toBeUndefined() }) + it('keeps built-in descriptions unchanged in English', async () => { + const b = await bench('en') + b.mint('s1') + const options = await b.contribution().ui.options(projection('s1'), new AbortController().signal) + expect(options[0]?.detail) + .toBe('DeepSeek · Fast, efficient, and economical; suited to focused, routine, or parallel tasks.') + expect(options[1]?.detail) + .toBe('DeepSeek · Stronger agentic coding, knowledge, and difficult reasoning; suited to complex or quality-critical tasks at higher cost.') + }) + it('a seat selection is the current the popup marks active next — one shared state', async () => { const b = await bench() b.mint('s1') From c2a5540ddb5d67cb05b326ecb54ada5edf1310ab Mon Sep 17 00:00:00 2001 From: Turtle Date: Tue, 8 Sep 2026 21:05:37 +0800 Subject: [PATCH 006/164] fix(web): distinguish plugin configuration instances --- .../plugin-instances.expected.md | 37 +++++++++++++ .../settings-chrome/plugins.expected.md | 6 +-- apps/web/tests/settings-chrome.e2e.ts | 42 +++++++++++++-- .../README.i18n.yaml | 4 +- .../ui-settings-plugin-inventory/README.md | 6 +-- .../ui-settings-plugin-inventory/README.zh.md | 6 +-- .../PluginInventorySettingsTab.module.css | 24 +++++++-- .../src/client/PluginInventorySettingsTab.tsx | 9 ++-- .../tests/components.client.spec.tsx | 54 ++++++++++++++----- 9 files changed, 155 insertions(+), 33 deletions(-) create mode 100644 apps/web/tests/expected/settings-chrome/plugin-instances.expected.md diff --git a/apps/web/tests/expected/settings-chrome/plugin-instances.expected.md b/apps/web/tests/expected/settings-chrome/plugin-instances.expected.md new file mode 100644 index 0000000000..5ec98e1f86 --- /dev/null +++ b/apps/web/tests/expected/settings-chrome/plugin-instances.expected.md @@ -0,0 +1,37 @@ +- list: + - listitem: + - button "tool-subagent-control, tool-subagent-control, 已启用": + - strong: tool-subagent-control + - code: tool-subagent-control + - text: 已启用 + - img + - listitem: + - button "tool-subagent-control/list-agents, tool-subagent-list-agents, 已启用": + - strong: tool-subagent-control/list-agents + - code: tool-subagent-list-agents + - text: 已启用 + - img + - listitem: + - button "tool-subagent, tool-subagent, 已启用": + - strong: tool-subagent + - code: tool-subagent + - text: 已启用 + - img + - listitem: + - button "tool-subagent, tool-subagent-fork, 已启用": + - strong: tool-subagent + - code: tool-subagent-fork + - text: 已启用 + - img + - listitem: + - button "tool-subagent, tool-subagent-codex, 已停用": + - strong: tool-subagent + - code: tool-subagent-codex + - text: 已停用 + - img + - listitem: + - button "tool-subagent, tool-subagent-claude-code, 已停用": + - strong: tool-subagent + - code: tool-subagent-claude-code + - text: 已停用 + - img diff --git a/apps/web/tests/expected/settings-chrome/plugins.expected.md b/apps/web/tests/expected/settings-chrome/plugins.expected.md index c4e0cbf9f0..71e84da312 100644 --- a/apps/web/tests/expected/settings-chrome/plugins.expected.md +++ b/apps/web/tests/expected/settings-chrome/plugins.expected.md @@ -1,6 +1,6 @@ - listitem: - - button "ui-settings, 已启用": - - strong: ui-settings - - img "运行中" + - button "tool-subagent, tool-subagent, 已启用": + - strong: tool-subagent + - code: tool-subagent - text: 已启用 - img diff --git a/apps/web/tests/settings-chrome.e2e.ts b/apps/web/tests/settings-chrome.e2e.ts index f3be2fcd39..ea2d4d9276 100644 --- a/apps/web/tests/settings-chrome.e2e.ts +++ b/apps/web/tests/settings-chrome.e2e.ts @@ -24,9 +24,10 @@ import { ZH_BROWSER_LOCALE, saveFailureShot } from './support.ts' const SNAPSHOT_DIR = fileURLToPath(new URL('./expected/settings-chrome', import.meta.url)) const DIALOG_EXPECTED = join(SNAPSHOT_DIR, 'dialog.expected.md') const PLUGINS_EXPECTED = join(SNAPSHOT_DIR, 'plugins.expected.md') +const PLUGIN_INSTANCES_EXPECTED = join(SNAPSHOT_DIR, 'plugin-instances.expected.md') // The English fallback surface: a browser naming no shipped language. const DIALOG_EN_EXPECTED = join(SNAPSHOT_DIR, 'dialog-en.expected.md') -const PLUGIN_ROW_SELECTOR = '[data-plugin-entry$="ui-settings"]' +const PLUGIN_ROW_SELECTOR = '[data-plugin-scope="preset"] [data-plugin-entry="tool-subagent"]' const MODE = webSnapshotMode() describe('web e2e: settings modal and General preferences', () => { @@ -114,7 +115,8 @@ describe('web e2e: settings modal and General preferences', () => { const expectedPluginCount = [...scaffold.ctx.loader.entries()] .filter(entry => !entry.options.group) .length - expect(await dialog.getByRole('searchbox', { name: '搜索插件' }).count()).toBe(1) + const pluginSearch = dialog.getByRole('searchbox', { name: '搜索插件' }) + expect(await pluginSearch.count()).toBe(1) // Every Loader entry appears exactly once in the global group — rows the // presets took over included, preset compositions excluded. expect(await dialog.locator('[data-plugin-scope="global"] [data-plugin-entry]').count()) @@ -130,6 +132,35 @@ describe('web e2e: settings modal and General preferences', () => { scaffold.workspaceCwd, ) await compareOrRefreshGolden(PLUGINS_EXPECTED, pluginsSnapshot, MODE) + await pluginSearch.fill('tool-subagent') + const instanceRows = [ + ['tool-subagent', '已启用'], + ['tool-subagent-fork', '已启用'], + ['tool-subagent-codex', '已停用'], + ['tool-subagent-claude-code', '已停用'], + ] as const + for (const [entryId, status] of instanceRows) { + const row = dialog.locator(`[data-plugin-scope="preset"] [data-plugin-entry="${entryId}"]`) + const trigger = row.getByRole('button', { name: `tool-subagent, ${entryId}, ${status}`, exact: true }) + await trigger.waitFor({ timeout: 10_000 }) + expect(await trigger.getAttribute('aria-expanded')).toBe('false') + const identity = row.locator('code') + expect(await identity.textContent()).toBe(entryId) + expect(await identity.getAttribute('title')).toBe(entryId) + } + const instancesSnapshot = await captureStableAria( + page, + '[data-plugin-scope="preset"] ul', + scaffold.workspaceCwd, + ) + await compareOrRefreshGolden(PLUGIN_INSTANCES_EXPECTED, instancesSnapshot, MODE) + await dialog.getByRole('button', { + name: 'tool-subagent, tool-subagent-claude-code, 已停用', + exact: true, + }).click() + expect(await dialog.locator('[data-plugin-entry="tool-subagent-claude-code"] button') + .getAttribute('aria-expanded')).toBe('true') + await pluginSearch.fill('') // Close path 1: Escape. await page.keyboard.press('Escape') await expect.poll(() => page.getByRole('dialog', { name: '设置' }).count(), { timeout: 5_000 }).toBe(0) @@ -635,6 +666,11 @@ describe('web e2e: settings modal and General preferences', () => { it.skipIf(MODE === 'record')('keeps the fixture inventory closed', async () => { expect(tripwire.warnings).toEqual([]) - await assertFixtureInventory(SNAPSHOT_DIR, ['dialog-en.expected.md', 'dialog.expected.md', 'plugins.expected.md']) + await assertFixtureInventory(SNAPSHOT_DIR, [ + 'dialog-en.expected.md', + 'dialog.expected.md', + 'plugin-instances.expected.md', + 'plugins.expected.md', + ]) }) }) diff --git a/packages/client/ui-settings-plugin-inventory/README.i18n.yaml b/packages/client/ui-settings-plugin-inventory/README.i18n.yaml index 3be2ee8298..5d5a900ece 100644 --- a/packages/client/ui-settings-plugin-inventory/README.i18n.yaml +++ b/packages/client/ui-settings-plugin-inventory/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-settings-plugin-inventory/README.md -README.md: 9a7ea91521c08dd1855925af59521d3af6e47b25 -README.zh.md: 2427d667d92b7a4c620fc6222507ecd8c2fabcef +README.md: 2a7a00f037905c9e70edb283c22946a2bc6c0247 +README.zh.md: 7be131a8a1a3f1e1fbbf2fbff1ac65c1829f4c10 diff --git a/packages/client/ui-settings-plugin-inventory/README.md b/packages/client/ui-settings-plugin-inventory/README.md index 9a7ea91521..2a7a00f037 100644 --- a/packages/client/ui-settings-plugin-inventory/README.md +++ b/packages/client/ui-settings-plugin-inventory/README.md @@ -9,7 +9,7 @@ English | [中文](README.zh.md) ## Summary -The **Plugin list** tab lets Web users inspect plugins without changing their configuration. It presents agent-preset compositions first and keeps the global inventory collapsed until needed. Cards expose enablement, provenance, runtime status, disabled conditions, and discovery failures; global entries supplied by presets are identified with the enabling presets. Search covers both groups and points to matches in other presets. The tab supports loading, empty, no-match, failure, and retry states without exposing transport details, and it still shows the global inventory when no preset roster is available. +The **Plugin list** tab lets Web users inspect plugins without changing their configuration. It presents agent presets first and collapses the global inventory until needed. Cards retain the package name as the primary title, identify instances by stable entry id, and expose enablement, provenance, runtime status, disabled conditions, and discovery failures; preset-provided global entries name their presets. Search covers both groups and points to matches in other presets. The tab handles loading, empty, no-match, failure, and retry states without exposing transport details, and still shows the global inventory without a preset roster. ## Table of Contents @@ -29,7 +29,7 @@ Open the Plugins section in Settings and select the **Plugin list** tab to inspe ### Reading a card -Each collapsed card uses the short module name as its title and a small enablement tag; enabled entries also show a colored root-fiber status dot. Expanding one card reveals the declared entry id, the full module specifier, and the state facts: a preset row names the preset it comes from, its runtime status when the composition is live, and its disable condition when it carries one; a preset-provided global row explains that agent presets provide it per session, names the presets that enable it, and offers a jump into the preset group. Preset names resolve through the shared `presetDisplayText` fold (`dsh-agent-presets/display`) over [`ui-agent-preset`](../ui-agent-preset/README.md)'s dictionaries: shipped presets follow the active locale while user-authored ones keep their own metadata, so an English surface never echoes the preset files' Chinese names. Search filters both groups by module name and entry id. +Each collapsed card uses the short module name as its primary title, shows the stable entry id underneath, and carries a small enablement tag; enabled entries also show a colored root-fiber status dot. Long entry ids truncate in the row and remain available on hover. Expanding one card reveals the declared entry id, the full module specifier, and the state facts: a preset row names the preset it comes from, its runtime status when the composition is live, and its disable condition when it carries one; a preset-provided global row explains that agent presets provide it per session, names the presets that enable it, and offers a jump into the preset group. Preset names resolve through the shared `presetDisplayText` fold (`dsh-agent-presets/display`) over [`ui-agent-preset`](../ui-agent-preset/README.md)'s dictionaries: shipped presets follow the active locale while user-authored ones keep their own metadata, so an English surface never echoes the preset files' Chinese names. Search filters both groups by module name and entry id. ### The preset switcher @@ -55,7 +55,7 @@ The browser plugin registers one localized `settings.plugins.tab` contribution w ### Rendering -Row keys are scope-qualified (`global:`, `preset::`), so one module appearing in both scopes keeps distinct disclosure state; an entry id is shown as detail only when the row declares one and is never classified by string shape. The preset-provided marking is derived client-side: a global entry carries it when it is disabled there while at least one preset row for the same module specifier is actually enabled, so a module every preset gates off (or declares only conditionally) stays plainly disabled rather than over-claiming provision. +Row keys are scope-qualified (`global:`, `preset::`), so one module appearing in both scopes keeps distinct disclosure state; a declared entry id appears in the collapsed row and expanded detail, while a row without one stays unlabeled, and the renderer never classifies ids by string shape. The preset-provided marking is derived client-side: a global entry carries it when it is disabled there while at least one preset row for the same module specifier is actually enabled, so a module every preset gates off (or declares only conditionally) stays plainly disabled rather than over-claiming provision. diff --git a/packages/client/ui-settings-plugin-inventory/README.zh.md b/packages/client/ui-settings-plugin-inventory/README.zh.md index 2427d667d9..7be131a8a1 100644 --- a/packages/client/ui-settings-plugin-inventory/README.zh.md +++ b/packages/client/ui-settings-plugin-inventory/README.zh.md @@ -9,7 +9,7 @@ kind: "package-reference" ## 概述 -**插件列表**标签页让 Web 用户查看插件,而不改变其配置。它优先展示 Agent 预设组合,并在需要前收起全局清单。卡片展示启停状态、出处、运行状态、禁用条件与发现失败;由预设提供的全局条目会标明启用它的预设。搜索覆盖两个分组,并指出其他预设中的匹配。标签页支持加载、空结果、无匹配、失败与重试状态,且不暴露传输细节;没有预设 roster 时仍会展示全局清单。 +**插件列表**标签页让 Web 用户查看插件,而不改变其配置。它优先展示 Agent 预设,并在需要前收起全局清单。卡片保留包名作为主标题,以稳定的条目 id 标识实例,并展示启停状态、出处、运行状态、禁用条件与发现失败;由预设提供的全局条目会列出对应预设。搜索覆盖两个分组,并指出其他预设中的匹配。标签页处理加载、空结果、无匹配、失败与重试状态,且不暴露传输细节;没有预设 roster 时仍会展示全局清单。 ## 目录 @@ -29,7 +29,7 @@ kind: "package-reference" ### 阅读卡片 -每张收起的卡片使用模块短名称作为标题,并以小标签表示启停状态;已启用的条目还会显示彩色根 fiber 状态圆点。展开卡片后会显示声明的条目 id、完整模块标识与状态事实:预设行说明它来自哪个预设、组合存活时的运行状态,以及它携带的禁用条件;被预设提供的全局行说明它由 Agent 预设按会话提供、列出启用它的预设,并提供跳转到预设组的入口。预设名经共享的 `presetDisplayText` 纯函数(`dsh-agent-presets/display`)叠在 [`ui-agent-preset`](../ui-agent-preset/README.zh.md) 的字典上解析:内置预设走当前语言,用户自建预设保留自己的元数据,因此英文界面不会回显预设文件里的中文名。搜索按模块名称与条目 id 过滤两组。 +每张收起的卡片使用模块短名称作为主标题,在下方显示稳定的条目 id,并以小标签表示启停状态;已启用的条目还会显示彩色根 fiber 状态圆点。长条目 id 会在行内截断,悬停时仍可查看完整值。展开卡片后会显示声明的条目 id、完整模块标识与状态事实:预设行说明它来自哪个预设、组合存活时的运行状态,以及它携带的禁用条件;被预设提供的全局行说明它由 Agent 预设按会话提供、列出启用它的预设,并提供跳转到预设组的入口。预设名经共享的 `presetDisplayText` 纯函数(`dsh-agent-presets/display`)叠在 [`ui-agent-preset`](../ui-agent-preset/README.zh.md) 的字典上解析:内置预设走当前语言,用户自建预设保留自己的元数据,因此英文界面不会回显预设文件里的中文名。搜索按模块名称与条目 id 过滤两组。 ### 预设切换器 @@ -55,7 +55,7 @@ kind: "package-reference" ### 渲染 -行 key 按作用域限定(`global:`、`preset::`),因此同一模块出现在两个作用域时保持各自的展开状态;条目 id 只在行声明了它时作为详情展示,代码不按字符串形状对它分类。预设提供标记在客户端推导:一个全局条目在全局被停用、且至少一个预设行对同一模块标识实际启用时才携带它,因此被所有预设关掉(或仅条件声明)的模块保持单纯的已停用,而不是夸大提供关系。 +行 key 按作用域限定(`global:`、`preset::`),因此同一模块出现在两个作用域时保持各自的展开状态;声明的条目 id 同时出现在收起行与展开详情中,没有 id 的行不显示次级标签,渲染器也不按字符串形状对 id 分类。预设提供标记在客户端推导:一个全局条目在全局被停用、且至少一个预设行对同一模块标识实际启用时才携带它,因此被所有预设关掉(或仅条件声明)的模块保持单纯的已停用,而不是夸大提供关系。 diff --git a/packages/client/ui-settings-plugin-inventory/src/client/PluginInventorySettingsTab.module.css b/packages/client/ui-settings-plugin-inventory/src/client/PluginInventorySettingsTab.module.css index d914492230..35ea6e7fbc 100644 --- a/packages/client/ui-settings-plugin-inventory/src/client/PluginInventorySettingsTab.module.css +++ b/packages/client/ui-settings-plugin-inventory/src/client/PluginInventorySettingsTab.module.css @@ -150,14 +150,32 @@ outline-offset: -2px; } -.cardTitle { +.cardHeading { + display: flex; + flex: 1; + flex-direction: column; min-width: 0; overflow: hidden; +} + +.cardTitle, +.cardIdentity { + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} + +.cardTitle { font-size: 14px; line-height: 20px; font-weight: 600; - text-overflow: ellipsis; - white-space: nowrap; +} + +.cardIdentity { + color: var(--dsw-alias-label-tertiary); + font-family: var(--ds-font-family-code); + font-size: 11px; + line-height: 16px; } .cardTrailing { diff --git a/packages/client/ui-settings-plugin-inventory/src/client/PluginInventorySettingsTab.tsx b/packages/client/ui-settings-plugin-inventory/src/client/PluginInventorySettingsTab.tsx index 31e37a6ca0..b062f1b157 100644 --- a/packages/client/ui-settings-plugin-inventory/src/client/PluginInventorySettingsTab.tsx +++ b/packages/client/ui-settings-plugin-inventory/src/client/PluginInventorySettingsTab.tsx @@ -113,7 +113,10 @@ function PluginCard({ rowKey, moduleName, entryId, trailing, ariaLabel, failed, aria-label={ariaLabel} onClick={() => { onToggle(rowKey) }} > - {moduleShortName(moduleName)} + + {moduleShortName(moduleName)} + {entryId === null ? null : {entryId}} + {trailing}