From 7f2674ab702155e5b7a36931b415a727dd32d123 Mon Sep 17 00:00:00 2001 From: Menghuan1918 Date: Thu, 10 Sep 2026 22:26:02 +0800 Subject: [PATCH] =?UTF-8?q?docs+test:=20=E5=AF=B9=E9=BD=90=E7=AD=89?= =?UTF-8?q?=E5=BE=85=20banner=20=E7=9A=84=E6=B3=A8=E9=87=8A/=E8=AE=BE?= =?UTF-8?q?=E8=AE=A1=E5=8F=A3=E5=BE=84=E5=B9=B6=E9=92=89=E4=BD=8F=20skip-w?= =?UTF-8?q?ait=20=E7=9A=84=20404=20=E5=A5=91=E7=BA=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit v0.19.0 发版前 review 发现两处「文档与实现互相矛盾」（均不改运行时行为）： - use-host-feeds.ts 的 effect 注释仍写「terminal tab 类型禁用时 pushes are ignored」，而 mirrorAgentWaits 恰恰改成了不忽略（只跳过 tab 增删 reconcile）。 注释改为如实描述； - 设计文档 §1 写「轮询每圈优先检查 skipped」，实现是 aborted → exited → skipped。维持实现现状（pty 已死是客观事实，两种顺序下用户可见结果——banner 消失 / skipWait 归零 / 等待结束——完全相同，只有 kind 文案不同），设计原文 同步为「退出优先」，并在实施偏差记录写清未采纳「用户意图优先」的理由与日后 改动点； - src/index.ts 的 agent-pty.skip-wait 注释把「无活动等待 → 0」泛化成了 「幂等」，收窄为「无活动等待返回 {skipped:0}；未知 uuid 走 expect → 404， 客户端 .catch 容忍并等下一次推送收敛」。 另把 skipWait 未知 uuid 的既有断言从「只校验 /not found/ 文案」升级为 「SidebarError + status 404 + code not-found」——HTTP 层 writeError 直接把 status 映射成响应码，此前这条契约没有测试守护（把 404 改成 410 时新断言即 失败，已验证）。 验证：pnpm test 126 files / 1312 passed / 0 failed，tsc --noEmit 与 eslint 均 exit 0；改动面仅 5 个文件，src/client/service.ts、builtins/ 与 external-plugin-guide.md 继续零改动。 --- .../2026-09-09-terminal-wait-banner-design.md | 29 +++++++++++++++++-- src/agent-pty.ts | 7 +++++ src/client/sidebar/use-host-feeds.ts | 5 ++-- src/index.ts | 6 ++-- tests/agent-pty.spec.ts | 12 ++++++++ 5 files changed, 53 insertions(+), 6 deletions(-) diff --git a/docs/plans/2026-09-09-terminal-wait-banner-design.md b/docs/plans/2026-09-09-terminal-wait-banner-design.md index b7fd64d..78a689f 100644 --- a/docs/plans/2026-09-09-terminal-wait-banner-design.md +++ b/docs/plans/2026-09-09-terminal-wait-banner-design.md @@ -39,8 +39,9 @@ needle 出现 / 超时 / 终端退出。等待期间用户在侧边栏看不到 - `waitFor()`：快速路径（已 exited / needle 已在 transcript）不登记、不推 banner； 进入轮询前登记记录并 `notify()`（banner 出现），`finally` 移除记录并 `notify()`（banner 消失——found/timeout/exited/skipped/工具 abort 五条出路统一走 finally）。 -- 轮询每圈优先检查本记录的 `skipped` → 返回 `{ kind:'skipped', needle }`；50ms 轮询 - 天然承载跳过延迟，无需事件唤醒。 +- 轮询每圈检查本记录的 `skipped` → 返回 `{ kind:'skipped', needle }`；50ms 轮询 + 天然承载跳过延迟，无需事件唤醒。与终端退出在同一 tick 竞争时**退出优先**（见实施 + 偏差记录）。 - 新方法 `skipWait(uuid): number`：标记该 uuid 全部活动记录并返回条数；uuid 未知 → `not-found` 404（与 `expect` 一致）。 - `AgentTerminalSnapshot` 增 `waiting?: { needle: string; since: number }`（取最新登记 @@ -165,3 +166,27 @@ needle 出现 / 超时 / 终端退出。等待期间用户在侧边栏看不到 5. 补并发双 `waitFor` 同 uuid 测试：快照显示最新 needle（`waits.at(-1)`）、 `skipWait` 返回 2、两个 promise 都以各自 needle `skipped` 收敛、快照 清空且 skipWait 幂等归零。 +- **轮询优先级：终端退出优先于用户跳过**（v0.19.0 发版前 review 发现）：设计原文 + 写「每圈优先检查 `skipped`」，实现是 `aborted → handle.exited → record.skipped`， + 即 pty 退出与用户点跳过落进同一个 50ms tick 时返回 `exited`。**维持实现现状**， + 设计原文已同步为「退出优先」：两种顺序下用户可见结果完全相同（banner 消失、 + `skipWait` 返回 0、等待结束），只有工具结果的 `kind` 文案不同；而进程已死是 + 客观事实，报 `exited` 不撒谎，工具结果本身也引导 agent 用 `terminal_read` 复核。 + 若日后要让「用户意图优先」，改动点是循环内两行先后 + 一条同 tick 竞态测试。 +- **两个已知限制的补充记录**（同一次 review）： + 1. **pinned tab 的 banner 与 ⏳ 不对称**：pinned 虚拟 id 是 + `pinned::agent:`，`isAgentTabId` 只认 `agent:` 前缀，因此 + 钉住的 agent tab **不会误挂 ⏳**（比"收不到"更强的保证）；但 Workbench 渲染 + pinned tab 时给 TerminalView 传的是**原 tab id + home 作用域**，所以 home 会话 + 正在等待时该视图**会**显示 banner，而它的 strip 徽章仍不出现。两个信号数据源 + 不同（banner 读 home store，徽章读 viewer 的 `agentWaits`）。 + 2. **⏳ 到不了 DSH 原生右侧栏**：徽章经 `getTabBadge` 渲染，唯一消费点是插件 + 自绘的 `TabBar`；原生栏的 `NativeTabTitle` 契约只带 title，不带 badge。agent + 终端标签页走 `openTabInBottomPane`（底部工作台），故 agent 流程可见；这是原生 + 承载面契约决定的上限，非本特性缺陷。 +- **注释随实现同步**（v0.19.0 发版前 review）：`use-host-feeds.ts` effect 头部原写 + 「terminal tab 类型禁用时 pushes are ignored」，与上面第 3 项的 `mirrorAgentWaits` + 改法矛盾，已改为「不增删 tab，但仍镜像权威 wait map」；`src/index.ts` 的 + `agent-pty.skip-wait` 注释原文把「无活动等待 → 0」泛化成了「幂等」，已收窄为 + 「无活动等待返回 `{skipped:0}`；未知 uuid 走 `expect` → 404（已退出的终端即属此 + 列），客户端 `.catch` 容忍并等下一次推送收敛」。 diff --git a/src/agent-pty.ts b/src/agent-pty.ts index 8a1472c..cd67df2 100644 --- a/src/agent-pty.ts +++ b/src/agent-pty.ts @@ -569,6 +569,13 @@ export class AgentPtyRegistry { handle.waits.push(record) this.notify() try { + // Exit outranks a skip in the same tick: `skipWait` flips `record.skipped` + // and the next poll observes it, but a pty that died first is the + // objective fact (a skip is moot once the process is gone). The user + // sees the same three things either way — the banner clears, `skipWait` + // reports 0 for the resolved wait, and the tool result says the wait + // ended — only the reported `kind` differs (see the design doc's + // implementation-deviation record). while (true) { if (signal?.aborted) signal.throwIfAborted() if (handle.exited) { diff --git a/src/client/sidebar/use-host-feeds.ts b/src/client/sidebar/use-host-feeds.ts index 59fb8a0..d80e1c1 100644 --- a/src/client/sidebar/use-host-feeds.ts +++ b/src/client/sidebar/use-host-feeds.ts @@ -94,8 +94,9 @@ export function useHostFeeds(feeds: { * the same shell without losing the agent's work — capped like the * terminal view's own reconnect loop, so a refused endpoint never spins * forever (the next session switch restarts the loop). - * While the terminal tab type is disabled in settings, pushes are - * ignored (no auto-added tabs); re-enabling makes the next push converge. + * While the terminal tab type is disabled in settings, pushes add / remove + * no tabs — but the authoritative wait map is STILL mirrored (see the + * branch below); re-enabling makes the next push converge on both. */ useEffect(() => { if (sessionId === undefined) return diff --git a/src/index.ts b/src/index.ts index 7271ac1..9598be8 100644 --- a/src/index.ts +++ b/src/index.ts @@ -547,8 +547,10 @@ function buildApi( return { ok: true } }, // The sidebar wait banner's skip button: abort every active - // terminal_wait_for on one agent terminal. Idempotent — 0 when nothing - // is waiting (a stale banner racing a wait that already resolved). + // terminal_wait_for on one agent terminal. An unknown uuid (a terminal + // already closed / reaped) goes through `expect` and surfaces as 404 + // not-found; the client tolerates that and lets the next push converge. + // Nothing waiting on a live terminal is not an error: 0 skipped. // Degraded mode (node-pty unavailable) has no registry and no waits: an // honest ok. 'agent-pty.skip-wait': (payload) => { diff --git a/tests/agent-pty.spec.ts b/tests/agent-pty.spec.ts index 663f4b6..06b842c 100644 --- a/tests/agent-pty.spec.ts +++ b/tests/agent-pty.spec.ts @@ -12,6 +12,7 @@ import { snapshotOf, tryResizePty, } from '../src/agent-pty.ts' +import { SidebarError } from '../src/wire.ts' /** * Resolve a shell binary for tests: on Windows use PowerShell (available on @@ -478,6 +479,17 @@ describe('AgentPtyRegistry', () => { const registry = new AgentPtyRegistry(testShell()) try { expect(() => registry.skipWait('missing-uuid')).toThrow(/not found/) + // The HTTP layer maps `status` straight onto the response, so the 404 + // contract rides this field — assert it, not just the message. + let thrown: unknown + try { + registry.skipWait('missing-uuid') + } catch (error) { + thrown = error + } + expect(thrown).toBeInstanceOf(SidebarError) + expect((thrown as SidebarError).status).toBe(404) + expect((thrown as SidebarError).code).toBe('not-found') } finally { registry.disposeAll() }