docs+test: 对齐等待 banner 的注释/设计口径并钉住 skip-wait 的 404 契约
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 继续零改动。
This commit is contained in:
@@ -39,8 +39,9 @@ needle 出现 / 超时 / 终端退出。等待期间用户在侧边栏看不到
|
|||||||
- `waitFor()`:快速路径(已 exited / needle 已在 transcript)不登记、不推 banner;
|
- `waitFor()`:快速路径(已 exited / needle 已在 transcript)不登记、不推 banner;
|
||||||
进入轮询前登记记录并 `notify()`(banner 出现),`finally` 移除记录并 `notify()`(banner
|
进入轮询前登记记录并 `notify()`(banner 出现),`finally` 移除记录并 `notify()`(banner
|
||||||
消失——found/timeout/exited/skipped/工具 abort 五条出路统一走 finally)。
|
消失——found/timeout/exited/skipped/工具 abort 五条出路统一走 finally)。
|
||||||
- 轮询每圈优先检查本记录的 `skipped` → 返回 `{ kind:'skipped', needle }`;50ms 轮询
|
- 轮询每圈检查本记录的 `skipped` → 返回 `{ kind:'skipped', needle }`;50ms 轮询
|
||||||
天然承载跳过延迟,无需事件唤醒。
|
天然承载跳过延迟,无需事件唤醒。与终端退出在同一 tick 竞争时**退出优先**(见实施
|
||||||
|
偏差记录)。
|
||||||
- 新方法 `skipWait(uuid): number`:标记该 uuid 全部活动记录并返回条数;uuid 未知 →
|
- 新方法 `skipWait(uuid): number`:标记该 uuid 全部活动记录并返回条数;uuid 未知 →
|
||||||
`not-found` 404(与 `expect` 一致)。
|
`not-found` 404(与 `expect` 一致)。
|
||||||
- `AgentTerminalSnapshot` 增 `waiting?: { needle: string; since: number }`(取最新登记
|
- `AgentTerminalSnapshot` 增 `waiting?: { needle: string; since: number }`(取最新登记
|
||||||
@@ -165,3 +166,27 @@ needle 出现 / 超时 / 终端退出。等待期间用户在侧边栏看不到
|
|||||||
5. 补并发双 `waitFor` 同 uuid 测试:快照显示最新 needle(`waits.at(-1)`)、
|
5. 补并发双 `waitFor` 同 uuid 测试:快照显示最新 needle(`waits.at(-1)`)、
|
||||||
`skipWait` 返回 2、两个 promise 都以各自 needle `skipped` 收敛、快照
|
`skipWait` 返回 2、两个 promise 都以各自 needle `skipped` 收敛、快照
|
||||||
清空且 skipWait 幂等归零。
|
清空且 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:<homeSessionId>:agent:<uuid>`,`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` 容忍并等下一次推送收敛」。
|
||||||
|
|||||||
@@ -569,6 +569,13 @@ export class AgentPtyRegistry {
|
|||||||
handle.waits.push(record)
|
handle.waits.push(record)
|
||||||
this.notify()
|
this.notify()
|
||||||
try {
|
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) {
|
while (true) {
|
||||||
if (signal?.aborted) signal.throwIfAborted()
|
if (signal?.aborted) signal.throwIfAborted()
|
||||||
if (handle.exited) {
|
if (handle.exited) {
|
||||||
|
|||||||
@@ -94,8 +94,9 @@ export function useHostFeeds(feeds: {
|
|||||||
* the same shell without losing the agent's work — capped like the
|
* the same shell without losing the agent's work — capped like the
|
||||||
* terminal view's own reconnect loop, so a refused endpoint never spins
|
* terminal view's own reconnect loop, so a refused endpoint never spins
|
||||||
* forever (the next session switch restarts the loop).
|
* forever (the next session switch restarts the loop).
|
||||||
* While the terminal tab type is disabled in settings, pushes are
|
* While the terminal tab type is disabled in settings, pushes add / remove
|
||||||
* ignored (no auto-added tabs); re-enabling makes the next push converge.
|
* no tabs — but the authoritative wait map is STILL mirrored (see the
|
||||||
|
* branch below); re-enabling makes the next push converge on both.
|
||||||
*/
|
*/
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (sessionId === undefined) return
|
if (sessionId === undefined) return
|
||||||
|
|||||||
+4
-2
@@ -547,8 +547,10 @@ function buildApi(
|
|||||||
return { ok: true }
|
return { ok: true }
|
||||||
},
|
},
|
||||||
// The sidebar wait banner's skip button: abort every active
|
// The sidebar wait banner's skip button: abort every active
|
||||||
// terminal_wait_for on one agent terminal. Idempotent — 0 when nothing
|
// terminal_wait_for on one agent terminal. An unknown uuid (a terminal
|
||||||
// is waiting (a stale banner racing a wait that already resolved).
|
// 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
|
// Degraded mode (node-pty unavailable) has no registry and no waits: an
|
||||||
// honest ok.
|
// honest ok.
|
||||||
'agent-pty.skip-wait': (payload) => {
|
'agent-pty.skip-wait': (payload) => {
|
||||||
|
|||||||
@@ -12,6 +12,7 @@ import {
|
|||||||
snapshotOf,
|
snapshotOf,
|
||||||
tryResizePty,
|
tryResizePty,
|
||||||
} from '../src/agent-pty.ts'
|
} from '../src/agent-pty.ts'
|
||||||
|
import { SidebarError } from '../src/wire.ts'
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Resolve a shell binary for tests: on Windows use PowerShell (available on
|
* Resolve a shell binary for tests: on Windows use PowerShell (available on
|
||||||
@@ -478,6 +479,17 @@ describe('AgentPtyRegistry', () => {
|
|||||||
const registry = new AgentPtyRegistry(testShell())
|
const registry = new AgentPtyRegistry(testShell())
|
||||||
try {
|
try {
|
||||||
expect(() => registry.skipWait('missing-uuid')).toThrow(/not found/)
|
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 {
|
} finally {
|
||||||
registry.disposeAll()
|
registry.disposeAll()
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user