fix(serve): report child-owned session turns as active work - #11265
Conversation
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
Thanks for the PR! Template looks good ✓ — every required heading is present, the Risk & Scope bullets are filled in properly, and the Chinese translation is complete rather than abbreviated. Problem: observed, not theoretical, and about as well evidenced as an issue gets. #11118 carries a production incident (one session accumulating 973 consecutive failed reclaim rounds at a constant 60s interval over ~16 hours, each round burning a full 8s drain budget, while Direction: aligned. The issue itself worked through three candidate fixes and ruled two out on evidence (cancelling inside the conditional close is a teardown that would permanently kill cron on a session the daemon decided to keep; refusing instead of accepting doesn't terminate the loop, because an empty hold set over an already-empty cache is indistinguishable from the snapshot that triggered the probe, and it would remove the only diagnostic signal). Widening the reported categories is the one that actually terminates it, and there is direct precedent — Size: core paths touched — this is a cross-package change spanning Approach: the scope feels right and genuinely minimal — one new category, one derived predicate, six change-notification calls at the transitions that were previously silent. No drive-by refactors or formatting churn. Updating the three docs in the same PR is necessary rather than scope creep, since all three explicitly asserted the old exclusion ("deliberately does not cover Monitors, workflows, cron"), and two of them were already stale on One question worth thinking about, not a blocker: the fix leaves two hand-maintained predicates that have to agree. Risk: Stage 1e matched — Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ —— 所有必需标题齐全，Risk & Scope 三项都认真填写，中文翻译也是完整的而非缩略。 问题： 是已观测到的问题，不是理论性加固，而且证据非常充分。#11118 里有真实生产事故（某个 session 在约 16 小时内以恒定 60s 间隔累计 973 轮连续失败的回收尝试，每轮烧满 8s drain 预算，而 方向： 对齐。issue 自己已经把三个候选方案推演过一遍，并用证据排除了两个（在条件式 close 内部取消其实是一次拆除，会让 daemon 决定保留的 session 永久失去 cron；改为拒绝并不能终止循环，因为空 hold 集合写进本就为空的缓存，与触发这次 probe 的快照无法区分，而且会移除唯一的诊断信号）。扩大上报类别是唯一真正能终止循环的方案，并且有直接先例 —— 规模： 触及核心路径 —— 这是横跨 方案： 范围合理，而且确实做到了最小化 —— 一个新类别、一个派生判据、六处此前静默的状态变化通知调用。没有顺手重构，也没有格式抖动。在同一个 PR 里更新三份文档是必要的而非范围蔓延，因为这三份都明确写着旧的排除规则（"deliberately does not cover Monitors, workflows, cron"），其中两份关于 有一个值得思考的问题，不是阻塞项：这个修复留下了两个必须保持一致、但各自手工维护的判据。 风险： Stage 1e 命中 —— 进入代码审查 🔍 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
Code reviewNo correctness blockers. This is a small, surgical change and it holds up. Because the fix is a predicate, the useful signal is which ways it could have been wrong, so here is what I actually checked rather than a restatement of the diff:
Suggestions (none blocking)Nothing pins the new behaviour. Zero test lines across the five files, and the gap is of the specific kind that matters: delete the entire One existing test now contradicts the docs this PR writes. Two predicates, still hand-synced — same point as in the gate comment, restating it here because it is a code-level concern: A deferred monitor notification now pins the session, by design but worth a second look. An item sitting in Also noting for completeness, not as a defect: widening the collector widens sequenceDiagram
participant P1 as Daemon reaper
participant P2 as Bridge coverage grading
participant P3 as AcpAgent conditional close
participant P4 as Session derived holds
P1->>P2: idle past threshold, snapshot reports no holds
P2->>P3: session close with onlyIfUnheld
P3->>P4: collectActiveWorkHolds (early read, gate held)
P4-->>P3: empty before this PR, session active-turn after
P3-->>P1: refuse with holds, no drain budget spent
P4->>P2: heartbeat snapshot on every transition
P2->>P1: hold visible, session is not a reclaim candidate
Files changed (5 of 5)
Test evidenceWhat this section carries: the PR's own CI results, read from the checks API for the reviewed commit. No PR code was built, executed or checked out here — the review is static, per the gate's rules. Nothing below is the author's self-report; for the record the PR body states no local test, build or typecheck was run, and I am not presenting that as evidence of anything. 34 checks on Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Sandboxed verification would settle this: 中文说明代码审查没有阻塞性正确性问题。 改动小而精准，站得住。因为修复本身就是一个判据，有价值的信号是它「可能错在哪里」，所以以下是我实际核对过的内容，而不是把 diff 重述一遍：
建议（均不阻塞）没有任何测试钉住新行为。 五个文件里测试行数为零，而这个缺口正属于最要紧的那一类：把 有一条已有测试现在与本 PR 写下的文档相矛盾。 两个判据，仍然靠手工同步 —— 与门禁评论里同一点，在此重述是因为它属于代码层面的问题： 一个被延迟的 monitor 通知现在会钉住 session —— 这是设计使然，但值得再看一眼。 因为 另外为完整性说明，不作为缺陷：扩大 collector 也就扩大了 （时序图与变更文件表见上，中文不重复。） 测试证据本节承载的内容： 该 PR 自身的 CI 结果，针对被审查的 commit 通过 checks API 读取。这里没有构建、执行或检出任何 PR 代码 —— 按门禁规则，审查是静态的。以下没有一项是作者自述；作为记录，PR 正文写明未运行本地测试、构建或 typecheck，我也不把这当作任何事情的证据。
沙箱验证可以把这件事定下来： — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
|
Confidence: 4/5 — the predicate is right and I could not break it after checking five separate ways it could have been; the one real gap is that nothing in the suite pins it, so CI green will not actually prove the fix works. Stepping back. Before reading the diff I wrote down what I would do: one derived hold-set function feeding both the heartbeat snapshot and the conditional-close path, one aggregate category for child-owned turn work, foreground prompts left to the daemon, and the existing category negotiation left to fail closed for mixed-version peers. That is what this PR does. It is better than my version in one respect I did not think of — it reuses the existing collector instead of adding a parallel derivation point, and it adds the six It is less ambitious than my version in one respect, which is the shared-predicate suggestion in the previous comment. I would not hold the PR for it, but I do think it is the difference between a fix and an invariant. The part I want to be straight about: this reads as agent-generated work — the branch name, and a PR body whose verification section says two static diff reviews and On the mixed-version tradeoff: widening the required category set means a new daemon beside an old child grades Am I approving because it is good or because I ran out of reasons to say no? The five things I expected to find wrong — a cron-registration-scoped abort controller pinning every cron session forever, a monitor-continuation gap between the two predicates, a missing negotiation change on one side, a transient mis-categorization reaching the wire, and a broken existing monitor test — each came back clean on inspection, and each was a plausible way for this to be broken. That is what a 4 looks like. Verdict: approve, deferred until CI lands. 中文说明置信度：4/5 —— 判据是正确的，我按五种它可能出错的方式逐一核对后都没能把它打破；唯一真实的缺口是测试套件里没有任何东西钉住它，所以 CI 变绿实际上并不能证明这个修复有效。 退一步看。在读 diff 之前我写下了自己会怎么做：一个派生的 hold 集合同时喂给 heartbeat 快照与条件式 close 路径，为子进程自有的 turn 工作设一个聚合类别，前台 prompt 留给 daemon，混合版本的 peer 交给已有的类别协商去 fail closed。这正是本 PR 所做的。有一点它比我的版本更好，而我没想到 —— 它复用了已有的 collector，而不是新增一个并行的派生点；并且它在 goal、history-mutation 与 cron 的状态变化处补上了六次 有一点它不如我的版本激进，就是上一条评论里共享判据的建议。我不会因此卡住这个 PR，但我确实认为那是「一个修复」与「一条不变量」之间的差别。 我想坦诚说明的部分：这看起来是 agent 生成的工作 —— 分支名，以及一份验证章节写着两轮静态 diff 审查加 关于混合版本的取舍：扩大必需类别集合意味着新 daemon 搭配旧 child 时会评级为 我是因为它好才批准，还是因为说不出反对理由才批准？我预期会出问题的五件事 —— 一个以 cron 注册为作用域的 abort controller 把每个 cron session 永久钉死、两个判据之间存在 monitor-continuation 缺口、某一侧缺少协商配套改动、一次瞬时错误分类到达线上、以及一条已有 monitor 测试被打破 —— 每一件在核查后都是干净的，而每一件都是这个改动可能出错的合理方式。这就是 4 分的样子。 结论：批准，但推迟到 CI 落定。 撰写本文时， — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
Maintainer verification — built and run end to end against a real
|
| PR arm | 3de833d784 (this PR head), full npm ci build |
| Base arm | 92a8a8d17957b800548d6aaca7feb2916fbe6593 (merge base), full npm ci build |
| Runtime | Node v22.22.2, Debian 13, Linux 6.12.63 x86_64, qwen-code 0.23.0 |
| Daemon | real qwen serve --port … --workspace … --no-web, isolated HOME/QWEN_HOME/workspace/port per arm |
| Reaper | --session-reap-interval-ms 3000 --session-idle-timeout-ms 25000 (production defaults 60 000 / 1 800 000, compressed so a 150 s window covers several rounds) |
| Model | a throwaway OpenAI-compatible mock; no credentials, no network |
Reproduction, following the recipe in #11118 with no source modification:
POST /session(sessionScope: thread),approval-mode: yolo.- One foreground prompt; the mock answers with a
cron_createtool call (durable: false, soQWEN_CODE_TEST_CRON_FAST=1fires it 5 s later). - The foreground turn settles normally. The SSE subscriber is dropped, the client id stays registered — so only the reaper can trigger a close.
- The cron turn's model request is accepted and never answered (the mock holds the socket open).
Instrumentation: one process.stderr.write added to the daemon's applyActiveWorkSnapshot in the built bundle so the channel snapshots are observable on the wire. Nothing else was patched; both bundles were byte-restored afterwards and both worktrees verified clean.
1. The loop reproduces on the merge base
Every snapshot reports holds: [] while a cron turn is wedged, GET /health?deep=1 answers activeWork: false with activeWorkReporting: full, and the reaper burns the full 8 000 ms drain budget three times in 150 s before the #11120 ladder pushes the next probe out to 120 s. Exactly the shape described in the issue. A genuinely idle control Session in the same run was reclaimed cleanly, so the reaper itself is healthy.
2. The PR closes it, without over-holding
Same flags, same mock, same scenario:
base 92a8a8d17 |
PR 3de833d784 |
|
|---|---|---|
| snapshot holds during the cron turn | [] |
[{"category":"session","id":"session:active-turn"}] |
GET /health?deep=1 → activeWork |
false |
true |
reaping idle session rounds / 150 s |
3 | 0 |
close-if-unheld … did not resolve |
3 | 0 |
| drain budget burned | 3 × 8 000 ms | 0 ms |
| Session closed to admission | 3 × ~8 s | never |
| genuinely idle control Session | reclaimed at t+25 s | reclaimed at t+25 s |
The wedged Session never becomes an auto-close candidate at all — entryIsAutoCloseCandidate returns on !childReportsHeldWork(entry) before the reaper's own log line, so the round trip is not merely fast, it is not made.
Reviewer Test Plan item 4 also confirmed in the same run: during a 30 s foreground prompt the snapshots stay holds: [] while /health?deep=1 already reports activeWork: true, activePrompts: 1. Prompts remain represented only by the daemon's own count.
3. The hold releases itself, and ordinary cleanup then proceeds
With the mock answering the held request after 60 s: seq 4–12 carry the hold, seq 13 is [], and the very next reaper pass closes the Session in one round — no timeout, no refusal, no ladder. No leak; item 3 of the test plan holds.
4. The transition is published ahead of the work, not on the next tick
The snapshot carrying the hold reaches the daemon at 06:33:25.812; the cron turn's own model request leaves at 06:33:25.817 — 5 ms earlier, i.e. on the microtask after cronProcessing = true, not 15 s later on the heartbeat. So there is no window in which the daemon can see the turn's effects but not its hold.
5. Version skew, measured
Both directions driven by patching one side's negotiated category set in the built bundle and leaving the other on this PR's code.
- New daemon + child that cannot report
session— deep health pins atactiveWorkReporting: partialand a completely idle Session is never reclaimed (0 probes in 85 s, vs reclaimed at t+30 s on a matched pair). Fails closed, as documented. Worth noting in the PR body: with the default spawn factory the child isprocess.execPath <process.argv[1]> --acp, so a running daemon always spawns the newer on-disk entry — this direction is not reachable from a single in-place upgrade, only from a heterogeneous install. - Old daemon + new child — the direction an in-place upgrade does reach. See finding B below.
6. The change is entirely untested
Nine one-at-a-time mutants of the new predicate, each run against Session.test.ts + active-work-reporter.test.ts (861 tests). All nine survive, including M0, which deletes the whole block. Nothing in the suite distinguishes this PR from main.
Findings
A — Important: no test, and one existing test now asserts the opposite of the new behaviour
Beyond the mutation table: packages/cli/src/acp-integration/session/Session.test.ts:1516 (does not hold for a Monitor notification) still ends with
// Monitors are outside activeWork's declared scope.
expect(session.collectActiveWorkHolds()).toEqual([]);That comment is now false — the PR adds notificationQueue.some((item) => item.kind === 'monitor') to the predicate. The test still passes only because it samples while recordNotificationStrict is still pending, i.e. before the item reaches notificationQueue. So the monitor arm the PR advertises is not just untested, the one test that names monitors documents the pre-PR contract as intended.
Suggested minimum: one assertion per arm (cron / goal / history-mutation / monitor-queue / notification-continuation), plus fixing that comment. Session.test.ts already has the createReportingSession() + holdIds(category) scaffolding a few tests above, so these are cheap.
(I drove the cron arm end to end. goalProcessing, historyMutationActive and the Monitor/notification arms I verified by reading only — and the mutation run says the suite does not cover them either.)
B — Important: old daemon + new child re-probes on every reaper tick, with #11120's backoff permanently disarmed
Measured on the wedged cron Session, with only the daemon side's category set patched to drop session:
| main (old daemon + old child) | old daemon + this PR's child | |
|---|---|---|
| reap rounds / 150 s | 3 | 46 |
did not resolve timeouts |
3 | 0 |
| spacing between rounds | ladder: 0 → 60 s → 120 s | flat 3 s = the reaper tick |
Mechanism: the new child now refuses immediately (holds are non-empty at the first collectActiveWorkHolds() read in closeSession), so confirmChildUnheld takes the answered path — which hoists entry.activeWorkCloseFailures = 0; entry.activeWorkCloseRetryAt = null; above the closed branch. The old daemon then drops the unknown session category while adopting the refusal, so entry.childHolds lands empty and childReportsHeldWork is false again on the next tick. The counter never reaches the ladder.
Net effect is genuinely mixed and I don't think it blocks the merge: each round costs ~1 ms instead of 8 000 ms, and the ~8 s admission freeze per round is gone — the user-visible half of #11118 improves even under skew. What regresses is log volume, back to roughly the pre-#11120 rate (one reaping idle session line per reaper interval, i.e. ~1 400/day at the 60 s default vs ~24/day with the ladder armed), with the refusal path still logging nothing to explain it. The bound in bridgeTypes.ts even anticipates this case in prose ("the child answering a probe either way … resets") but the comment assumes an answer means progress.
Options, in the order I'd pick them: (1) say so explicitly in Risk & Scope — the PR currently says only that "an old daemon cannot consume the new hold until it is upgraded or restarted", which is true but understates it; (2) don't reset the failure counter when a refusal carried at least one hold whose category the daemon does not understand — that is precisely "the child says it is busy in a language I don't speak", which is not evidence the world moved on.
C — Nit: hasStandaloneRelocationBlockers() and isIdle() widen too
Both consume the same collector, so both now refuse during cron/goal/history-mutation work. acpAgent.ts:10996 already awaits waitForActiveTurnsToSettle() before the relocation check, so in practice only a hold created during the drain can trip it; and isIdle() already gated on #hasActiveTurn(), which covered all of these except a queued monitor notification — which now behaves like a queued agent notification. Verified benign, mentioning it only because neither is called out in Risk & Scope.
Harness, scripts, and the raw logs behind every number above are reproducible from the description in the collapsed section; happy to hand them over if useful.
中文版
维护者验证 —— 在真实 qwen serve daemon 上端到端构建并运行
结论：建议合入。 缺陷在 merge base 上可复现，本 PR 消除了它；工作结束后 hold 会自行释放，真正空闲的 Session 仍然会被正常回收。有两点希望在合入前后处理，都不是正确性阻塞项：改动完全没有测试（新判据的每一个变异体都能存活），以及 旧 daemon + 新 child 这条真实原地升级会走到的版本错配路径，会静默地让 #11120 的退避阶梯失效。
环境与方法
| PR 侧 | 3de833d784（本 PR head），完整 npm ci 构建 |
| Base 侧 | 92a8a8d17957b800548d6aaca7feb2916fbe6593（merge base），完整 npm ci 构建 |
| 运行环境 | Node v22.22.2、Debian 13、Linux 6.12.63 x86_64、qwen-code 0.23.0 |
| Daemon | 真实 qwen serve，每侧独立的 HOME/QWEN_HOME/workspace/端口 |
| Reaper | --session-reap-interval-ms 3000 --session-idle-timeout-ms 25000（生产默认 60 000 / 1 800 000，压缩后 150 s 窗口能覆盖多轮） |
| 模型 | 一次性 OpenAI 兼容 mock，无凭据、无网络 |
复现步骤完全按 #11118 的方案，未改动任何源码：创建 thread session、设为 yolo、发一个前台 prompt，mock 回一个 cron_create（durable: false，配合 QWEN_CODE_TEST_CRON_FAST=1 在 5 s 后触发）；前台 turn 正常结束后断开 SSE 订阅、保留 client id，因此只有 reaper 能触发 close；cron turn 的模型请求被接受后永不响应。
仅有的插桩：在构建产物的 applyActiveWorkSnapshot 里加一行 stderr 输出，使 channel snapshot 的线上内容可观测。其余未改，两份 bundle 事后按字节还原，两个 worktree 均验证干净。
1. 循环在 merge base 上复现
cron turn 卡住期间，每份 snapshot 都是 holds: []，GET /health?deep=1 返回 activeWork: false 且 activeWorkReporting: full，reaper 在 150 s 内 三次烧满 8 000 ms 的 drain 预算，随后被 #11120 的阶梯推到 120 s。与 issue 描述完全一致。同一次运行里，一个真正空闲的对照 Session 被干净回收，说明 reaper 本身正常。
2. 本 PR 消除了它，且没有过度持有
base 92a8a8d17 |
PR 3de833d784 |
|
|---|---|---|
| cron turn 期间的 snapshot holds | [] |
[{"category":"session","id":"session:active-turn"}] |
GET /health?deep=1 → activeWork |
false |
true |
150 s 内 reaping idle session 轮数 |
3 | 0 |
close-if-unheld … did not resolve |
3 | 0 |
| 烧掉的 drain 预算 | 3 × 8 000 ms | 0 ms |
| Session 对 admission 关闭 | 3 × 约 8 s | 从未 |
| 真正空闲的对照 Session | t+25 s 被回收 | t+25 s 被回收 |
被卡住的 Session 根本没有成为自动关闭候选：entryIsAutoCloseCandidate 在 reaper 自己的日志行之前就因 !childReportsHeldWork(entry) 返回，所以往返不是变快了，而是压根没有发生。
Reviewer Test Plan 第 4 条同批验证： 一个 30 s 的前台 prompt 期间，snapshot 始终是 holds: []，而 /health?deep=1 已经是 activeWork: true, activePrompts: 1。前台 prompt 仍只由 daemon 自己的计数表示。
3. hold 会自行释放，随后普通清理继续
mock 在 60 s 后回答被挂起的请求：seq 4–12 带 hold，seq 13 变回 []，紧接着的一次 reaper 扫描一轮就关掉了 Session —— 无超时、无拒绝、无重试阶梯。没有泄漏，测试计划第 3 条成立。
4. 状态变化先于工作本身上报，而不是等下一个心跳
带 hold 的 snapshot 于 06:33:25.812 抵达 daemon，而 cron turn 自己的模型请求 06:33:25.817 才发出 —— 早 5 ms，即 cronProcessing = true 之后的那个 microtask，而不是 15 s 后的心跳。因此不存在「daemon 看得见 turn 的副作用却看不见它的 hold」的窗口。
5. 版本错配的实测结果
两个方向都通过在构建产物里只改一侧的协商类别集合、另一侧保持本 PR 代码来驱动。
- 新 daemon + 无法上报
session的 child —— deep health 永久钉在activeWorkReporting: partial，一个完全空闲的 Session 永不被回收（85 s 内 0 次探测；匹配版本下 t+30 s 就回收了）。按文档 fail closed。建议在 PR 描述里补一句：默认 spawn factory 用的是process.execPath <process.argv[1]> --acp，运行中的 daemon 总是拉起磁盘上更新的入口，所以这个方向不是单次原地升级能到达的，只有异构安装才会出现。 - 旧 daemon + 新 child —— 这才是原地升级真正会走到的方向，见下面的发现 B。
6. 改动完全没有测试
九个一次一个的变异体，逐个跑 Session.test.ts + active-work-reporter.test.ts（861 个用例）。九个全部存活，包括直接删掉整段判据的 M0。 测试套件无法把本 PR 与 main 区分开。
发现 A —— 重要：没有测试，且一条既有测试现在断言的是相反行为
除了变异表之外：packages/cli/src/acp-integration/session/Session.test.ts:1516（does not hold for a Monitor notification）结尾仍然是
// Monitors are outside activeWork's declared scope.
expect(session.collectActiveWorkHolds()).toEqual([]);这条注释现在是错的 —— 本 PR 已把 notificationQueue.some((item) => item.kind === 'monitor') 加进判据。它还能通过，只是因为断言发生在 recordNotificationStrict 仍 pending 时，即元素还没进入 notificationQueue。所以 PR 宣称的 monitor 覆盖不仅没有测试，唯一提到 monitor 的那条测试还把 PR 之前的契约记录为「预期行为」。
建议的最小补充：每个分支各一条断言（cron / goal / history-mutation / monitor 队列 / notification 续跑），并修掉那条注释。Session.test.ts 上方已有 createReportingSession() + holdIds(category) 脚手架，成本很低。
（我端到端驱动的是 cron 分支；goalProcessing、historyMutationActive 与 Monitor/notification 分支仅做了代码审读 —— 而变异结果表明测试套件同样没有覆盖它们。）
发现 B —— 重要：旧 daemon + 新 child 会每个 reaper tick 都重新探测，#11120 的退避被永久解除
在同一个卡住的 cron Session 上，仅把 daemon 侧的类别集合改成不含 session：
| main（旧 daemon + 旧 child） | 旧 daemon + 本 PR 的 child | |
|---|---|---|
| 150 s 内 reap 轮数 | 3 | 46 |
did not resolve 超时 |
3 | 0 |
| 轮次间隔 | 阶梯 0 → 60 s → 120 s | 恒定 3 s，即 reaper tick |
机制：新 child 现在会立即拒绝（closeSession 里第一次 collectActiveWorkHolds() 就非空），于是 confirmChildUnheld 走的是已应答分支 —— 而 entry.activeWorkCloseFailures = 0; entry.activeWorkCloseRetryAt = null; 被提升到了 closed 分支之上。旧 daemon 在采纳这次拒绝时又会丢掉它不认识的 session 类别，于是 entry.childHolds 落成空集，下一个 tick 里 childReportsHeldWork 又是 false。计数器永远到不了阶梯。
净效果其实是喜忧参半，我认为不构成合入阻塞：每轮成本从 8 000 ms 降到约 1 ms，每轮约 8 s 的 admission 冻结也没有了 —— #11118 中用户可见的那一半即使在版本错配下也变好了。退化的是日志量，回到大致 #11120 之前的水平（每个 reaper 间隔一行 reaping idle session，按 60 s 默认约 1 400 行/天，而阶梯生效时约 24 行/天），且拒绝路径依然什么都不打，没有任何解释。bridgeTypes.ts 里的注释其实预见到了这个情形（"the child answering a probe either way … resets"），但它默认「有应答」就等于「有进展」。
按我的偏好排序的处理方式：(1) 在 Risk & Scope 里明确写出来 —— 目前只写了「旧 daemon 在升级或重启前无法消费新 hold」，属实但低估了影响；(2) 当一次拒绝里至少带有一个 daemon 不认识的类别时，不要重置失败计数 —— 那正是「child 说它在忙，只是用了我听不懂的语言」，并不构成「世界已经往前走了」的证据。
发现 C —— 细节：hasStandaloneRelocationBlockers() 与 isIdle() 也随之放宽
两者共用同一个收集器，因此在 cron/goal/history-mutation 期间都会拒绝。acpAgent.ts:10996 在 relocation 检查前已经 await waitForSessionDrain(waitForActiveTurnsToSettle())，所以实际上只有 drain 期间新产生的 hold 才会触发；isIdle() 本就先过 #hasActiveTurn()，除了排队中的 monitor 通知之外都已覆盖 —— 而它现在的行为与排队中的 agent 通知一致。已验证无害，提出来只是因为 Risk & Scope 里没有提到这两处。
qqqys
left a comment
qqqys
left a comment
There was a problem hiding this comment.
APPROVE (verified at head 3de833d)
Historical items
No CHANGES_REQUESTED and zero inline findings have ever existed on this PR; the two review stages and the human approval on this head are the maintainer's, with my independent read below.
My Critical-only scan
- The new
sessionaggregate (session:active-turn) is derived from the same six already-tracked Session fields —historyMutationActive,goalProcessing,cronProcessing/cronAbortController/cronCompletion, monitor-kind queue entries, and the notification-completion term with agent/workflow/shell continuations excluded by their owncurrent*flags — and I confirmed each field's write-sites: every one is set/cleared around a turn with an identity check in thefinally, so no merely-registered feature (a cron job ID, a stored goal) can pin a session permanently; that was the inverse-bug risk and it closes. - Coverage flows through the single
ACTIVE_WORK_HOLD_CATEGORIESconstant on both negotiation sides (daemon advertise/intersect, child intersect) with no per-category switches anywhere — the new category grades intofull/partial, refusal adoption, and the reporter filter without companion edits;ACTIVE_WORK_LEGACY_HOLD_CATEGORIEScorrectly untouched; one aggregate hold cannot approach the 1024 bound. - The six added
#activeWorkChanged()calls are load-bearing, not decoration: reclaim and idle accounting both key off report transitions, and without them the predicate would widen while the event stream stayed silent (the very shape of the reported 2.4-day stale symptom). - The three doc files are updated in-lockstep with the code (activeWork scope, negotiation tables, the conditional-close drain-window sentence whose risk the new category now subsumes).
- Mixed-version cost is fail-closed and stated: an older child that never advertises
sessiongradespartialand loses ordinary automatic cleanup (explicit close/kill unchanged) — retention, not destruction, is the default for the unknown.
Disclosed gaps, accepted not ignored
The new predicate line has no test pin at this head (session:active-turn appears only in Session.ts), which both review stages say plainly — a CI-green merge would not execute the branch. Per this channel's rule a missing test gates nothing unless the uncovered path is demonstrably wrong, and my field-lifetime walk found no wrongness; the maintainer's own ask for a /verify run before trusting it on a long-lived daemon is the right next artifact, cheap to produce, and worth doing either as a pre-merge comment or immediately after. CI itself at this head: 21 green, zero failures, two lanes still running (non-gating).
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): "agent 1a": could not execute any test suite to confirm green — npx vitest run src/acp-integration/active-work-reporter.test.ts was stopped by the worktree's globalSetup…; "agent 1a": could not read the previous release's packages/acp-bridge/src/bridge.ts to confirm its refusal-adoption and failure-backoff code matched the HEAD shape I reas…; "agent 6b": did not execute packages/cli Session.test.ts or packages/acp-bridge bridge.test.ts to confirm empirically that the new hold and the widened category con…; "agent reverse-audit (round 2)": whether buildGoalContinuationParts(turn) (Session.ts:2538) can throw — it executes after goalProcessing = true and before the try at 2541 ( #emitGoalStart…; "agent reverse-audit (round 2)": the body of Session.test.ts:1516 it('does not hold for a Monitor notification') — I verified the test exists at that line but did not read which window it sam…, and 7 more.
中文说明
已审查。 建议见行内评论。
未探索到全部深度（达到工具调用预算）："agent 1a"：could not execute any test suite to confirm green — npx vitest run src/acp-integration/active-work-reporter.test.ts was stopped by the worktree's globalSetup…；"agent 1a"：could not read the previous release's packages/acp-bridge/src/bridge.ts to confirm its refusal-adoption and failure-backoff code matched the HEAD shape I reas…；"agent 6b"：did not execute packages/cli Session.test.ts or packages/acp-bridge bridge.test.ts to confirm empirically that the new hold and the widened category con…；"agent reverse-audit (round 2)"：whether buildGoalContinuationParts(turn) (Session.ts:2538) can throw — it executes after goalProcessing = true and before the try at 2541 ( #emitGoalStart…；"agent reverse-audit (round 2)"：the body of Session.test.ts:1516 it('does not hold for a Monitor notification') — I verified the test exists at that line but did not read which window it sam…，另有 7 条。
— qwen3.8-max-2026-09-02 via Qwen Code /review (v0.23.0)
| | new daemon + new child | `full`; shell hold crosses the wire | existing conditional-close flow | | ||
| | new daemon + old v1 child | `partial`; `shell` is missing | disabled for that Session | | ||
| | new daemon + new child | `full`; current holds cross the wire | existing conditional-close flow | | ||
| | new daemon + old v1 child | `partial`; newer categories are missing | disabled for that Session | |
There was a problem hiding this comment.
[Suggestion] R1-2: This compatibility row scopes the fail-closed consequence to a single Session, but the code reads the capability per channel. entryIsAutoCloseCandidate resolves owner = channelInfoForEntry(entry) and then capability = owner?.activeWork, returning false for the whole channel when any required category is missing (packages/acp-bridge/src/bridge.ts:3172-3186), and activeWorkCoverage grades the same way (bridge.ts:9380-9404), so deep health pins at partial daemon-wide. One incomplete child therefore disables ordinary automatic cleanup for every Session sharing its channel, not for one Session.
This commit is what makes the distinction start to matter. Adding session to the required set means "old v1 child" no longer describes a child missing shell — at the merge base that was the only way to be incomplete — it now describes every child built before this commit, which is the ordinary in-place-upgrade window. A reader of this table concludes one Session loses cleanup during that window, when in fact every Session on the channel does, for as long as the daemon runs against a pre-session child. That scope is the fact the reserved maintainer decision in issue #11118 turns on, and the issue's own cost analysis states it correctly: "ordinary automatic cleanup is disabled for every session on that channel, and deep health pins at partial".
Witness — one channel, two Sessions (a restored one with no client, a spawned one whose client detaches), child answering the literal ['agent','notification','shell','workflow']:
INTACT reportingGrade=partial conditionalCloseCalls=0 sessionCount=2 <- both retained
MUTANT AssertionError: expected 'full' to be 'partial'
AssertionError: expected 2 to be +0 <- both offered for teardown
('session' dropped from the required set)
Change the cell to name the real scope, e.g. disabled for every Session on that channel — the wording the issue's cost analysis uses — and leave the rest of the row as rewritten. Prettier will re-align the table's other rows once this column widens, so it is worth running npm run format on the file rather than hand-aligning.
The cell must keep matching bridge.ts:3172-3186, which grades per channel through channelInfoForEntry(entry) rather than per Session.
中文说明
[Suggestion] R1-2：这张兼容性表格把 fail-closed 的后果限定在单个 Session，但代码读取的是 channel 级别的能力。entryIsAutoCloseCandidate 先解析 owner = channelInfoForEntry(entry)，再取 capability = owner?.activeWork，只要缺少任何一个必需 category 就对整个 channel 返回 false（packages/acp-bridge/src/bridge.ts:3172-3186）；activeWorkCoverage 的分级逻辑相同（bridge.ts:9380-9404），因此 deep health 会在整个 daemon 范围钉在 partial。也就是说，一个不完整的 child 会让该 channel 上的每一个 Session 都失去普通自动清理，而不只是一个。
这个 commit 正是让该区别开始变得重要的原因。把 session 加入必需集合后，"old v1 child" 不再指缺少 shell 的 child（在 merge base 上那是唯一的不完整形态），而是指这个 commit 之前构建的所有 child，也就是普通的就地升级窗口。读这张表的人会以为该窗口内只有一个 Session 失去清理，实际上是 channel 上的每个 Session 都会失去，只要 daemon 还在对着一个不认识 session 的 child 运行。而这个范围正是 issue #11118 中保留给 maintainer 决策所依据的事实，issue 自己的成本分析也写对了："ordinary automatic cleanup is disabled for every session on that channel, and deep health pins at partial"。
证据 —— 一个 channel、两个 Session（一个无 client 的 restored session，一个 client 已 detach 的 spawned session），child 回答字面量 ['agent','notification','shell','workflow']：
INTACT reportingGrade=partial conditionalCloseCalls=0 sessionCount=2 <- 两个都被保留
MUTANT AssertionError: expected 'full' to be 'partial'
AssertionError: expected 2 to be +0 <- 两个都被送去 teardown
（从必需集合中移除 'session'）
建议把该单元格改成描述真实范围，例如 disabled for every Session on that channel（即 issue 成本分析用的措辞），该行其余内容保持本次改写后的样子。这一列变宽后 Prettier 会重新对齐表格其他行，因此建议对该文件跑一次 npm run format，而不是手工对齐。
该单元格必须与 bridge.ts:3172-3186 保持一致 —— 那里是通过 channelInfoForEntry(entry) 按 channel 分级，而不是按 Session。
— qwen3.8-max-2026-09-02 via Qwen Code /review (v0.23.0)
| `Session.isIdle()` consumes the same unfiltered collector. Workspace reload therefore skips a Session while a background shell or its terminal continuation is active. | ||
|
|
||
| Conditional close reads the unfiltered collector once before disturbing active turns and again after those turns drain, while the Session close gate remains held. The final read closes the window where an already-running, otherwise out-of-scope cron or automatic turn registers a shell during drain; the new shell refuses ordinary teardown without adding cron itself to `activeWork`. | ||
| Conditional close reads the unfiltered collector once before disturbing active turns and again after those turns drain, while the Session close gate remains held. Child-owned cron, goal, history-mutation, and Monitor-continuation work is now represented by the aggregate `session` category, so it refuses ordinary teardown before the drain begins. |
There was a problem hiding this comment.
[Suggestion] R1-3: This commit swept both design-doc sentences that justified the conditional close's post-drain re-read — the one at docs/design/2026-08-06-active-work-health.md:69 ("The second read matters because an already-running out-of-scope turn such as cron can register a background shell while it drains.") and the one replaced here ("The final read closes the window where an already-running, otherwise out-of-scope cron or automatic turn registers a shell during drain…") — but left the identical claim standing as the only in-code rationale for that re-read, in a file this diff does not touch:
// packages/cli/src/acp-integration/acpAgent.ts:4430-4433
// Existing out-of-scope work such as a cron turn may have
// registered a background shell while it drained. Re-check after
// every active turn has settled, while both the close gate and the
// history-mutation gate still block destructive races.
The cron example is genuinely dead now. A running cron turn sets cronProcessing, which the first read (acpAgent.ts:4370-4376) reports as a session hold, so the close is refused before any drain begins; and no new cron turn can start after the gate either, since #drainCronQueue and #drainCronQueueExclusive both return on this.closing (Session.ts:8888, :8904, :8918-8927). So a maintainer who reads that comment as the reason the third collector read exists, confirms cron is covered by this PR, and removes the re-read as redundant, drops a guard that is still load-bearing — for the windows that open after read #2: the generation-controller abort and cancelPendingPrompt() (a cancelled Agent stays in listUnfinalizedBackgroundAgentIds() because it still owes its terminal task-notification), the await previous on the history-mutation tail, and, new with this PR, a concurrent Session-level historyMutationActive that reads #1 and #2 could not see. The Session is then destroyed while that work is starting.
One correction to the obvious framing: the re-check is not the only guard for the foreground-prompt case. Read #2 (acpAgent.ts:4388) follows waitForActiveTurnsToSettle(), which rejects on timeout, so a shell registered by a prompt that settles is caught there. Rewriting the comment to claim the foreground-prompt case as read #3's sole justification would trade one wrong rationale for another.
Witness:
git show 21e11335:docs/design/2026-08-06-active-work-health.md -> line 69 carries the cron rationale
git show 21e11335:docs/design/2026-08-13-active-work-background-shell.md -> line 40 carries it too
HEAD -> both gone
git show HEAD:packages/cli/src/acp-integration/acpAgent.ts -> lines 4430-4433 still carry it
probe (scratch tree, packages/cli, intact PR):
cronProcessing = true -> collectActiveWorkHolds() = [{ category: 'session', id: 'session:active-turn' }]
with diff hunk Session.ts:3 reverted (revert-hunk -> applied: true) -> []
Rewrite the example and keep the re-check, e.g. "A foreground turn that was already running when the close gate shut can register a background shell or Agent while it drains, and work can still appear after the post-settle read — the generation abort and cancelPendingPrompt(), the history-mutation tail, a concurrent historyMutationActive."
The re-check itself must survive: docs/design/2026-08-06-active-work-health.md:71 still specifies "It rejects known holds immediately, drains any turn that was already active when the gate closed, then evaluates the unfiltered collector again", and acpAgent.ts:4367-4369 still states that the early read "is an optimization rather than the final authorization".
中文说明
[Suggestion] R1-3：这个 commit 删掉了两处为 conditional close「drain 之后再读一次」提供理由的设计文档句子 —— docs/design/2026-08-06-active-work-health.md:69（"The second read matters because an already-running out-of-scope turn such as cron can register a background shell while it drains."）以及此处被替换掉的那句（"The final read closes the window where an already-running, otherwise out-of-scope cron or automatic turn registers a shell during drain…"）—— 但同样的说法仍然作为该重读唯一的代码内理由留在一个本 diff 没有触碰的文件里：
// packages/cli/src/acp-integration/acpAgent.ts:4430-4433
// Existing out-of-scope work such as a cron turn may have
// registered a background shell while it drained. Re-check after
// every active turn has settled, while both the close gate and the
// history-mutation gate still block destructive races.
cron 这个例子现在确实已经不成立了。运行中的 cron turn 会置位 cronProcessing，而第一次读取（acpAgent.ts:4370-4376）就会把它上报为 session hold，于是在 drain 开始之前 close 就被拒绝了；gate 之后也不可能有新的 cron turn 启动，因为 #drainCronQueue 和 #drainCronQueueExclusive 都在 this.closing 时直接返回（Session.ts:8888、:8904、:8918-8927）。所以，如果一位维护者把这条注释当作第三次读取 collector 的存在理由，确认 cron 已被本 PR 覆盖，进而把这次重读当作冗余删掉，就会移除一个仍然承重的保护 —— 它覆盖的是在 read #2 之后才出现的窗口：generation controller 的 abort 与 cancelPendingPrompt()（被取消的 Agent 仍留在 listUnfinalizedBackgroundAgentIds() 中，因为它还欠一个 terminal task-notification）、history-mutation tail 上的 await previous，以及本 PR 新增的、read #1 与 read #2 都看不到的并发 Session 级 historyMutationActive。此时 Session 会在这些工作正要开始时被销毁。
对一个容易写错的说法做个修正：这次重读并不是前台 prompt 场景的唯一保护。read #2（acpAgent.ts:4388）跟在 waitForActiveTurnsToSettle() 之后，而后者超时会 reject，因此由「正常 settle 的 prompt」注册的 shell 会在 read #2 被捕获。如果把注释改写成「前台 prompt 场景是 read #3 的唯一理由」，那只是把一个错误理由换成另一个。
证据：
git show 21e11335:docs/design/2026-08-06-active-work-health.md -> 第 69 行含 cron 理由
git show 21e11335:docs/design/2026-08-13-active-work-background-shell.md -> 第 40 行也含
HEAD -> 两处都已删除
git show HEAD:packages/cli/src/acp-integration/acpAgent.ts -> 4430-4433 行仍然保留
probe（scratch tree，packages/cli，未修改的 PR）：
cronProcessing = true -> collectActiveWorkHolds() = [{ category: 'session', id: 'session:active-turn' }]
回退 diff hunk Session.ts:3（revert-hunk -> applied: true）后 -> []
建议改写这个例子但保留重读逻辑，例如："A foreground turn that was already running when the close gate shut can register a background shell or Agent while it drains, and work can still appear after the post-settle read — the generation abort and cancelPendingPrompt(), the history-mutation tail, a concurrent historyMutationActive."
重读本身必须保留：docs/design/2026-08-06-active-work-health.md:71 仍规定 "It rejects known holds immediately, drains any turn that was already active when the gate closed, then evaluates the unfiltered collector again"，acpAgent.ts:4367-4369 也仍说明前面那次读取 "is an optimization rather than the final authorization"。
— qwen3.8-max-2026-09-02 via Qwen Code /review (v0.23.0)
| ``` | ||
|
|
||
| `sessions`, `pendingPermissions`, and `activePrompts` are sums. `activeWork` is true when any runtime has an accepted but unsettled prompt (including a FIFO-waiting prompt), a running background Agent, a queued/in-progress Agent terminal notification, or Session-managed background shell work. Shell work remains active while the shell registry reports a running entry and while its terminal notification is queued or driving the parent continuation; any number of shells contributes one bounded aggregate hold. Monitors, workflows, cron jobs, follow-up suggestions, and external processes the shell registry can no longer track remain outside the field. It is session-scoped: channel-level work with no session attached yet — a spawn in flight, a pending restore, MCP discovery or authentication — is not counted, so `activeWork` may read false while the daemon still declines to reclaim that channel. Do not read this field as "the daemon is reclaimable"; it describes session-owned work only. `activeWorkReporting` says how much of that boolean is actually vouched for: `full` when every live session is covered by a fresh report from a child that reports all required categories, `none` when no session negotiated reporting, and `partial` for anything between — including a stale snapshot or a negotiated child that omits a required category. A snapshot older than three report intervals stops counting as coverage: it is not a report that the session is idle, so the session goes back to reading as retained, exactly as if the child had never reported. Ordinary automatic cleanup is also disabled for a negotiated-but-incomplete child; a child that does not understand `shell` cannot safely authorize conditional close according to the complete current predicate. Completely unsupported historical children retain legacy cleanup behavior, and explicit close, kill, shutdown, and channel exit remain force operations. `activeWorkStaleMs` is the age of the oldest snapshot the boolean rests on **among the covered sessions**, and is `0` when no session is covered; it is diagnostic, because freshness is already graded into `activeWorkReporting` by the daemon (only the daemon knows each channel's negotiated cadence). The grade is computed once over every managed runtime rather than per runtime and then combined — a runtime with no sessions is vacuously complete, and treating that as evidence would let an empty workspace vouch for another workspace's unreported sessions. `lastActivityAt` is the latest non-null workspace activity time and `idleSinceMs` is derived from that same snapshot. `channelAlive` means at least one managed workspace channel is live; it does not mean every workspace is healthy. `connectedClients` and the optional `rateLimitHits` remain daemon-wide counters rather than per-workspace sums. | ||
| `sessions`, `pendingPermissions`, and `activePrompts` are sums. `activeWork` is true when any runtime has an accepted but unsettled prompt (including a FIFO-waiting prompt), a running background Agent, a queued/in-progress Agent terminal notification, Session-managed background shell or workflow work, or a child-owned Session turn. The aggregate `session` hold covers goal and cron processing, history mutation, and queued or running Monitor continuations; foreground prompts remain daemon-owned. Running Monitors, follow-up suggestions, and external processes the shell registry can no longer track remain outside the field. It is session-scoped: channel-level work with no session attached yet — a spawn in flight, a pending restore, MCP discovery or authentication — is not counted, so `activeWork` may read false while the daemon still declines to reclaim that channel. Do not read this field as "the daemon is reclaimable"; it describes session-owned work only. `activeWorkReporting` says how much of that boolean is actually vouched for: `full` when every live session is covered by a fresh report from a child that reports all required categories, `none` when no session negotiated reporting, and `partial` for anything between — including a stale snapshot or a negotiated child that omits a required category. A snapshot older than three report intervals stops counting as coverage: it is not a report that the session is idle, so the session goes back to reading as retained, exactly as if the child had never reported. Ordinary automatic cleanup is also disabled for a negotiated-but-incomplete child; a child that does not understand `shell` or `session` cannot safely authorize conditional close according to the complete current predicate. Completely unsupported historical children retain legacy cleanup behavior, and explicit close, kill, shutdown, and channel exit remain force operations. `activeWorkStaleMs` is the age of the oldest snapshot the boolean rests on **among the covered sessions**, and is `0` when no session is covered; it is diagnostic, because freshness is already graded into `activeWorkReporting` by the daemon (only the daemon knows each channel's negotiated cadence). The grade is computed once over every managed runtime rather than per runtime and then combined — a runtime with no sessions is vacuously complete, and treating that as evidence would let an empty workspace vouch for another workspace's unreported sessions. `lastActivityAt` is the latest non-null workspace activity time and `idleSinceMs` is derived from that same snapshot. `channelAlive` means at least one managed workspace channel is live; it does not mean every workspace is healthy. `connectedClients` and the optional `rateLimitHits` remain daemon-wide counters rather than per-workspace sums. |
There was a problem hiding this comment.
[Suggestion] R1-4: This rewrite of the integrator-facing activeWork paragraph deletes the whole shell-coverage sentence and nothing replaces it:
Shell work remains active while the shell registry reports a running entry and while its terminal notification is queued or driving the parent continuation; any number of shells contributes one bounded aggregate hold.
That sentence was the only statement in this reference of when shell coverage ends, and of the one-aggregate-hold bound — the rest of the paragraph only says what makes activeWork true. A restart-controller author reading the new text sees "Session-managed background shell or workflow work" with no duration, and naturally reads activeWork as tracking a running shell entry, so it goes false when the shell process exits. That is exactly the window docs/design/2026-08-13-active-work-background-shell.md:5 was written to close: "A restart controller could therefore treat the daemon as idle and terminate the Session before the shell's terminal notification reached the parent continuation." The controller then terminates a Session whose terminal continuation has not settled, and the shell's notification is lost. The same deletion also drops the only documented bound on shell holds, which is what keeps a large shell roster inside ACTIVE_WORK_MAX_SESSION_HOLDS.
The behaviour is unaffected — Session.ts:4033-4039 still holds while hasRunningEntries() || notificationQueue.some(kind === 'shell') || currentShellNotificationActive, and still pushes one aggregate background-shells hold regardless of shell count — so this is a loss of documentation for a guarantee the code still provides. The window remains described in the internal design doc, but a third-party integrator reads this page, not docs/design/.
Witness:
git show 21e11335:docs/developers/qwen-serve-protocol.md | sed -n '639p'
-> contains "Shell work remains active while the shell registry reports a running entry..."
HEAD line 639 -> does not
whole-file sweep for hold|terminal notification|background shell|shell registry at HEAD
-> exactly one active-work hit (line 639); every other "hold" match is unrelated prose
(:309 a client holding the daemon token, :779/:783 an object holding,
:2649/:2677/:2679 the writer lease)
Restore the sentence after "…or a child-owned Session turn." If the new session aggregate deserves the same treatment, its duration is already stated one sentence later, so only the shell half is missing.
The restored sentence must keep matching Session.ts:4038 — holds.push({ category: 'shell', id: 'background-shells' }) — i.e. one aggregate shell hold, not one per shell.
中文说明
[Suggestion] R1-4：本次对面向集成者的 activeWork 段落的改写，删掉了整句 shell 覆盖范围的说明，而且没有任何内容替代它：
Shell work remains active while the shell registry reports a running entry and while its terminal notification is queued or driving the parent continuation; any number of shells contributes one bounded aggregate hold.
这句话是本参考文档中唯一说明 shell 覆盖何时结束、以及「一个聚合 hold」上限的地方 —— 段落其余部分只说明什么会让 activeWork 变为 true。一位 restart controller 的作者读到新文本时，只看到 "Session-managed background shell or workflow work"，没有任何时间范围，自然会理解为 activeWork 跟踪的是「正在运行的 shell 条目」，于是认为 shell 进程退出后它就变为 false。而这正是 docs/design/2026-08-13-active-work-background-shell.md:5 当初要关闭的窗口："A restart controller could therefore treat the daemon as idle and terminate the Session before the shell's terminal notification reached the parent continuation."。这样一来 controller 会在 terminal continuation 尚未 settle 时终止 Session，shell 的通知就此丢失。同一次删除还去掉了 shell hold 数量上限的唯一文档说明，而正是这个上限让规模较大的 shell 集合不会超出 ACTIVE_WORK_MAX_SESSION_HOLDS。
行为本身没有受影响 —— Session.ts:4033-4039 仍然在 hasRunningEntries() || notificationQueue.some(kind === 'shell') || currentShellNotificationActive 期间保持 hold，并且不论 shell 数量多少都只推一个聚合的 background-shells hold —— 所以这是「代码仍然提供的保证失去了文档」。该窗口在内部设计文档中仍有描述，但第三方集成者读的是本页面，而不是 docs/design/。
证据：
git show 21e11335:docs/developers/qwen-serve-protocol.md | sed -n '639p'
-> 含 "Shell work remains active while the shell registry reports a running entry..."
HEAD 第 639 行 -> 不含
在 HEAD 全文件检索 hold|terminal notification|background shell|shell registry
-> 只有一处 active-work 命中（第 639 行）；其余 "hold" 命中都是无关文本
（:309 持有 daemon token 的 client，:779/:783 对象持有，
:2649/:2677/:2679 writer lease）
建议把这句话恢复到 "…or a child-owned Session turn." 之后。如果新的 session 聚合同样需要这类说明，它的持续范围在后一句已经写明，因此只缺 shell 这半边。
恢复的句子必须与 Session.ts:4038 保持一致 —— holds.push({ category: 'shell', id: 'background-shells' }) —— 即一个聚合的 shell hold，而不是每个 shell 一个。
— qwen3.8-max-2026-09-02 via Qwen Code /review (v0.23.0)
| 'shell', | ||
| 'session', | ||
| 'workflow', | ||
| ]; |
There was a problem hiding this comment.
[Suggestion] R1-5: Adding 'session' here changes two daemon predicates — the health grade (bridge.ts:9397-9402) and the automatic-cleanup gate (bridge.ts:3178-3185) — but no test pins either change, because every negotiation test computes both sides of its assertion from this same constant. acpAgent.test.ts:3026 (the request the test sends) and :3038 (the expected answer) both spread ACTIVE_WORK_HOLD_CATEGORIES, as do bridge.test.ts:354 and :428, so the assertion moves with the code and can never fail on it. Every incomplete-child case uses ACTIVE_WORK_LEGACY_HOLD_CATEGORIES instead (bridge.test.ts:483, 528, 11761, 12735, 14667, 14719; acpAgent.test.ts:3185), and a grep for categories in bridge.test.ts excluding those two constants returns zero lines — no test in the repo spells a literal category list.
So the population this PR actually downgrades — a child from the immediately previous release answering agent|notification|shell|workflow — is untested at both consumers. Moving 'session' into ACTIVE_WORK_LEGACY_HOLD_CATEGORIES, or replacing the completeness check with a hard-coded three-category list, would let a child that cannot see cron/goal/history-mutation work grade full and authorize destructive teardown of a Session with a cron turn running, with the whole suite green.
Witness — executed mutant, 'session' removed from ACTIVE_WORK_HOLD_CATEGORIES in a scratch tree:
packages/acp-bridge (whole suite) 1 failed | 1959 passed (1960) <- the one failure is the probe itself
packages/cli acpAgent.test.ts MUTANT 2 failed | 620 passed (622)
INTACT 2 failed | 620 passed (622) <- identical failure names
active-work-reporter.test.ts 10 passed (10) <- under the mutant
positive control (probe) INTACT partial / 0 conditional closes
MUTANT expected 'full' to be 'partial' / expected 2 to be +0
The mutant is invisible to every pre-existing test in both packages; only the probe sees it. Both production and test import the constant via '@qwen-code/acp-bridge/bridgeTypes', which packages/cli/vitest.config.ts:153-156 aliases to source, so the mutation was genuinely in scope for that run rather than masked by a stale dist.
Add a bridge.test.ts case whose child answers the literal ['agent','notification','shell','workflow'], asserting reportingGrade(bridge) is 'partial' and that the Session is retained (no sessionClose probe issued) after detachClient; and pin the literal expected category list in the initialize negotiation assertion in acpAgent.test.ts instead of spreading the constant on both sides.
That new case must go red if 'session' is dropped from ACTIVE_WORK_HOLD_CATEGORIES or moved into the legacy constant — the mutant above is exactly that red, and today nothing produces it. The child answers ACTIVE_WORK_LEGACY_HOLD_CATEGORIES when the daemon's initialize carries no categories (acpAgent.ts:4816-4821), so that baseline must stay ['agent','notification'] and must not be derived from the widened union.
中文说明
[Suggestion] R1-5：在此处加入 'session' 改变了 daemon 的两个判定 —— health 分级（bridge.ts:9397-9402）与自动清理准入（bridge.ts:3178-3185）—— 但没有任何测试钉住这两个变化，因为所有协商测试的断言两端都取自同一个常量。acpAgent.test.ts:3026（测试发出的请求）与 :3038（期望的回答）都展开 ACTIVE_WORK_HOLD_CATEGORIES，bridge.test.ts:354 与 :428 也是如此，所以断言会随代码一起移动，永远不会因它而失败。而所有「不完整 child」的用例改用 ACTIVE_WORK_LEGACY_HOLD_CATEGORIES（bridge.test.ts:483, 528, 11761, 12735, 14667, 14719；acpAgent.test.ts:3185），在 bridge.test.ts 中排除这两个常量后检索 categories 返回零行 —— 仓库里没有任何测试写出字面量 category 列表。
因此本 PR 真正降级的那一类对象 —— 上一个版本、回答 agent|notification|shell|workflow 的 child —— 在两个消费点都没有测试覆盖。把 'session' 移进 ACTIVE_WORK_LEGACY_HOLD_CATEGORIES，或把完整性检查换成硬编码的三 category 列表，都会让一个看不见 cron/goal/history-mutation 工作的 child 被评为 full，从而授权销毁一个正在跑 cron turn 的 Session，而整个测试套件依然是绿的。
证据 —— 已执行的 mutant，在 scratch tree 中从 ACTIVE_WORK_HOLD_CATEGORIES 移除 'session'：
packages/acp-bridge（全套） 1 failed | 1959 passed (1960) <- 唯一失败就是 probe 本身
packages/cli acpAgent.test.ts MUTANT 2 failed | 620 passed (622)
INTACT 2 failed | 620 passed (622) <- 失败用例名完全相同
active-work-reporter.test.ts 10 passed (10) <- 在 mutant 下
positive control（probe） INTACT partial / 0 次 conditional close
MUTANT expected 'full' to be 'partial' / expected 2 to be +0
这个 mutant 对两个包中所有既有测试都不可见，只有 probe 能观察到。生产代码与测试都通过 '@qwen-code/acp-bridge/bridgeTypes' 引入该常量，而 packages/cli/vitest.config.ts:153-156 将其 alias 到源码，所以该变异确实在那次运行的作用域内，并没有被过期的 dist 掩盖。
建议新增一个 bridge.test.ts 用例，让 child 回答字面量 ['agent','notification','shell','workflow']，断言 reportingGrade(bridge) 为 'partial'，并且在 detachClient 之后 Session 被保留（不发出 sessionClose 探测）；同时在 acpAgent.test.ts 的 initialize 协商断言中钉住字面量 category 列表，而不是两端都展开常量。
该新用例必须在 'session' 从 ACTIVE_WORK_HOLD_CATEGORIES 中被删除或被移入 legacy 常量时变红 —— 上面的 mutant 正是这种红，而今天没有任何测试会产生它。当 daemon 的 initialize 不携带 categories 时，child 会回答 ACTIVE_WORK_LEGACY_HOLD_CATEGORIES（acpAgent.ts:4816-4821），因此该基线必须保持 ['agent','notification']，也不能由变宽后的 union 推导得出。
— qwen3.8-max-2026-09-02 via Qwen Code /review (v0.23.0)
| this.historyMutationActive || | ||
| this.goalProcessing || | ||
| this.cronProcessing || |
There was a problem hiding this comment.
[Suggestion] R1-6: This enumerates today's unreported busy states rather than deriving the hold from the predicate that actually decides whether the Session can settle, so the class now holds two hand-synced copies of "which flags mean a child-owned turn is in flight" — this block and #hasActiveTurn() 84 lines below it.
Six disjuncts are shared verbatim between the two, and the four that differ are deliberate and correct (prompts are daemon-owned; notificationProcessing/notificationAbortController are coextensive with notificationCompletion; the queued-monitor term has no counterpart in the settlement predicate) — but nothing records that mapping. Issue #11118's root cause is these two notions of busy disagreeing, and this change makes them agree by enumeration, so they can diverge again the same silent way: the next child-owned automatic-work flag gets added to #hasActiveTurn() only — exactly how historyMutationActive came to be listed there — and the collector stays silent. The daemon's early read then passes, waitForActiveTurnsToSettle() falls into its poll branch on a flag that publishes no completion promise (Session.ts:4186-4191), the drain budget expires, the refusal comes back with holds: [], and activeWorkCloseFailures re-probes forever — #11118 verbatim (973 rounds over 16 h in the field), with the fix sitting one method away and nothing local showing it, since isIdle() still reads busy and /health still reads ok.
Witness — both method bodies parsed with the repository's own TypeScript compiler as the oracle, collecting every this.<prop> reference:
SHARED (6): cronAbortController, cronCompletion, cronProcessing,
goalProcessing, historyMutationActive, notificationCompletion
COLLECTOR-ONLY (4): currentAgentNotificationTaskId, currentShellNotificationActive,
currentWorkflowNotificationTaskId, notificationQueue
HASACTIVETURN-ONLY (4): notificationAbortController, notificationProcessing,
pendingPrompt, pendingPromptCompletion
LINES: collectActiveWorkHolds@4007 #hasActiveTurn@4091 apart=84
To be clear about severity: the present state is correct. Walking all ten #hasActiveTurn() disjuncts against this collector, the only two unrepresented are pendingPrompt and pendingPromptCompletion, both documented as deliberate (Session.ts:4003-4006) and covered daemon-side by entryHasLocalWork (bridge.ts:3069-3072). This is about the next flag, not this one.
Extract the child-owned half of the settlement predicate — a #hasChildOwnedActiveTurn() holding the five shared flags — declare it immediately above #hasActiveTurn(), and compose: #hasActiveTurn() becomes pendingPrompt || pendingPromptCompletion || notificationProcessing || notificationAbortController || #hasChildOwnedActiveTurn(), while this collector calls the same method and keeps its two monitor terms local. A new child-owned turn state then cannot enter the settlement predicate without entering the hold set. If the extraction is not wanted, a comment here stating why the two sets differ is the minimum — the why is genuinely non-obvious, which is the bar for adding one.
Do not unify the two into a single predicate in the other direction: the queued-monitor term has no completion promise, so folding it into #hasActiveTurn() would make beginHistoryMutation() (Session.ts:4110, which rejects on if (this.#hasActiveTurn())) refuse while a monitor notification merely sits queued. The unification must also not make an agent/workflow/shell continuation emit a second, redundant session hold — expect(session.collectActiveWorkHolds()).toHaveLength(1) (Session.test.ts:1344) and the exact-array assertions at :1446, 1479, 1511, 1595, 1639, 1672, 1719 all require one hold per fact. And cronCompletion is not redundant with cronProcessing, so it must survive the extraction: Session.ts:4741-4753 aborts and nulls the controller and sets cronProcessing = false, then awaits cronCompletion before nulling it, so that disjunct alone covers the aborted-cron settle window.
中文说明
[Suggestion] R1-6：这里是把当前尚未上报的繁忙状态逐条枚举出来，而不是从「真正决定 Session 能否 settle」的那个判定推导出 hold，于是这个类里现在存在两份需要手工同步的「哪些 flag 表示 child 自有 turn 正在进行」—— 本代码块，以及下方 84 行的 #hasActiveTurn()。
两者之间有六个析取项完全相同，而不同的四个是刻意且正确的（prompt 归 daemon 所有；notificationProcessing/notificationAbortController 与 notificationCompletion 同生同灭；queued-monitor 这一项在 settle 判定中没有对应项）—— 但没有任何地方记录这个对应关系。issue #11118 的根因正是这两种「繁忙」定义不一致，而本次改动用枚举的方式让它们一致，因此它们仍会以同样静默的方式再次分叉：下一个 child 自有的自动工作 flag 只被加进 #hasActiveTurn() —— historyMutationActive 当初就是这样被列进去的 —— 而 collector 保持沉默。于是 daemon 的提前读取通过，waitForActiveTurnsToSettle() 落入其轮询分支去等一个不会发布 completion promise 的 flag（Session.ts:4186-4191），drain 预算耗尽，拒绝返回 holds: []，activeWorkCloseFailures 无限重试 —— 完全是 #11118 的重演（线上 16 小时 973 轮），而修复就在隔壁一个方法里，本地却看不出任何异常，因为 isIdle() 仍显示繁忙、/health 仍显示 ok。
证据 —— 以仓库自带的 TypeScript 编译器为 oracle 解析两个方法体，收集所有 this.<prop> 引用：
SHARED (6): cronAbortController, cronCompletion, cronProcessing,
goalProcessing, historyMutationActive, notificationCompletion
COLLECTOR-ONLY (4): currentAgentNotificationTaskId, currentShellNotificationActive,
currentWorkflowNotificationTaskId, notificationQueue
HASACTIVETURN-ONLY (4): notificationAbortController, notificationProcessing,
pendingPrompt, pendingPromptCompletion
LINES: collectActiveWorkHolds@4007 #hasActiveTurn@4091 apart=84
关于严重级别需要说明清楚：当前状态是正确的。把 #hasActiveTurn() 的十个析取项逐一对照本 collector，只有 pendingPrompt 与 pendingPromptCompletion 未被表示，二者都被明确记录为刻意排除（Session.ts:4003-4006），并且在 daemon 侧由 entryHasLocalWork 覆盖（bridge.ts:3069-3072）。本条针对的是「下一个 flag」，不是当前这些。
建议把 settle 判定中属于 child 的那一半抽出来 —— 一个包含五个共享 flag 的 #hasChildOwnedActiveTurn() —— 声明在 #hasActiveTurn() 正上方，并做组合：#hasActiveTurn() 变为 pendingPrompt || pendingPromptCompletion || notificationProcessing || notificationAbortController || #hasChildOwnedActiveTurn()，而本 collector 调用同一个方法，并把两个 monitor 相关项保留在本地。这样，新的 child 自有 turn 状态就不可能进入 settle 判定而不进入 hold 集合。如果不希望抽取，那至少在此处加一条注释说明两个集合为何不同 —— 这个「为什么」确实不明显，正好达到添加注释的门槛。
不要反方向把两者合并成单一判定：queued-monitor 这一项没有 completion promise，因此把它并入 #hasActiveTurn() 会让 beginHistoryMutation()（Session.ts:4110，在 if (this.#hasActiveTurn()) 时拒绝）在一个 monitor 通知仅仅排队时就拒绝。合并也不能让 agent/workflow/shell continuation 多发出一个冗余的 session hold —— expect(session.collectActiveWorkHolds()).toHaveLength(1)（Session.test.ts:1344）以及 :1446, 1479, 1511, 1595, 1639, 1672, 1719 处的精确数组断言都要求「一个事实一个 hold」。另外 cronCompletion 与 cronProcessing 并不冗余，抽取时必须保留：Session.ts:4741-4753 会 abort 并置空 controller、把 cronProcessing 设为 false，然后才 await cronCompletion 并将其置空，因此只有这个析取项覆盖 cron 被中止后的 settle 窗口。
— qwen3.8-max-2026-09-02 via Qwen Code /review (v0.23.0)
| this.currentWorkflowNotificationTaskId === null && | ||
| !this.currentShellNotificationActive) | ||
| ) { | ||
| holds.push({ category: 'session', id: 'session:active-turn' }); |
There was a problem hiding this comment.
[Suggestion] R1-7: The entire behavioural change this PR makes — the seven-term predicate above and the six new #activeWorkChanged() publishes — ships with no test. session:active-turn occurs exactly twice in the repo, this line and the new design-doc line at docs/design/2026-08-06-active-work-health.md:27, and in no test file; the diff touches no test at all. So deleting this.cronProcessing ||, or any one of the seven terms, or the whole block, leaves every in-scope suite green, and the regression ships silently: the collector returns [] during a cron/goal/history-mutation/Monitor-continuation turn, the daemon's candidate gate admits the Session, conditional close enters the drain, times out, and #11118's indefinite probe loop returns.
The one existing Monitor-hold test makes that worse rather than better. it('does not hold for a Monitor notification') (Session.test.ts:1513-1546) carries the comment // Monitors are outside activeWork's declared scope. — the exact contract this PR reverses in three documents — and injects kind: 'monitor' at line 1533 through the public daemon-push method, a route production rejects outright: acpAgent.ts:10571-10576 throws Invalid background notification kind for any non-agent kind, and the daemon-side type is kind: 'agent' (bridgeTypes.ts:1405-1413). A real monitor terminal notification arrives child-side through monitorRegistry.setNotificationCallback into the private #enqueueBackgroundNotification, which pushes synchronously (Session.ts:9709-9710) and holds on the same tick. So the repo's only "monitor + activeWork" coverage pins a state production cannot reach and asserts the opposite of shipped behaviour — the next maintainer reads it as an enforced contract and drops the queued-monitor term at Session.ts:4066, with the suite blessing the removal.
Witness:
test-efficacy probe over the real harness (no diff-owned tests exist to collect):
probed: [] mutants: 0 hunks: 0 harnessValidated: null
-> reverting the hunk leaves every in-scope suite exactly as green as it is now
env -u QWEN_RUNTIME_DIR npx vitest run src/acp-integration/session/Session.test.ts \
-t 'does not hold for a Monitor notification'
Tests 1 passed | 850 skipped (851)
PROBE-A outcome for kind=monitor: REJECTED Invalid background notification kind
session.enqueueBackgroundNotification calls: []
PROBE-B2 holds immediately after a real monitor dispatch (no await):
[{"category":"session","id":"session:active-turn"}]
recordNotificationStrict calls on this path: 0
PROBE-B1 (positive control) holds during the agent acceptance window:
[{"category":"notification","id":"probe-b1-agent"}]
flip: reverting diff hunk Session.ts:3 turns B2 into [] with B1 unchanged
Widen holdIds() at Session.test.ts:1252 to the exported ActiveWorkHoldCategory — import it, as active-work-reporter.ts:11 already does — rather than adding a third copy of the enumeration. Then add cases to the existing active work holds describe asserting exactly one { category: 'session', id: 'session:active-turn' } hold for each trigger independently (beginHistoryMutation() unreleased, an in-flight goal turn, an in-flight cron turn, a queued monitor item, a running monitor continuation with notificationCompletion set and all three current* markers clear) and [] after each settles; plus the negatives the protocol doc promises — a running Monitor registry entry, an in-flight follow-up suggestion, and a foreground prompt alone each yield no session hold — and an agent/shell/workflow continuation yielding its own category and no session hold. Retarget the Monitor test at a state production can reach (capture mockMonitorRegistry.setNotificationCallback's callback and fire it under a held close gate, as holds a queued shell notification before its continuation can start at Session.test.ts:1420 does) and drop the scope comment.
Each new case must go red when its own term is deleted from the block above; the negative control must go red if the three marker guards are removed or the kind-field assignment in #drainNotificationQueueExclusive moves after an await; and a changes-counter case must go red if any of the six added #activeWorkChanged() calls is deleted. Today the measured mutation — reverting this hunk — turns nothing red. Four constraints on the new tests: holdIds's parameter union at Session.test.ts:1252 must widen first (strict TS, no any); the hold must stay one aggregate at any queue depth, since ACTIVE_WORK_MAX_SESSION_HOLDS = 1024 (bridgeTypes.ts:376) is the per-Session bound the daemon enforces on snapshots and refusals alike (bridge.ts:3487); a monitor item can only reach notificationQueue via the child-side registry callback, so a test driven through the public enqueueBackgroundNotification() pins the persistence window instead of the queued hold; and createReportingSession() (Session.test.ts:1237-1250) disposes and rebuilds the Session, so registry mocks must be installed before it and the change counter is the 6th constructor argument.
中文说明
[Suggestion] R1-7：本 PR 的全部行为改动 —— 上方那个七项判定，以及六处新增的 #activeWorkChanged() 发布 —— 都没有测试。session:active-turn 在仓库中只出现两次：本行，以及新增的设计文档行 docs/design/2026-08-06-active-work-health.md:27，在任何测试文件中都没有出现；本 diff 完全没有触碰测试。因此删掉 this.cronProcessing ||、或七个项中的任意一个、或整个代码块，所有相关测试套件依然全绿，回归会静默上线：collector 在 cron/goal/history-mutation/Monitor-continuation turn 期间返回 []，daemon 的候选准入放行该 Session，conditional close 进入 drain、超时，#11118 的无限探测循环重现。
而现有的唯一一个 Monitor-hold 测试让情况更糟而不是更好。it('does not hold for a Monitor notification')（Session.test.ts:1513-1546）带着注释 // Monitors are outside activeWork's declared scope. —— 正是本 PR 在三份文档中反转掉的约定 —— 并且在第 1533 行通过公开的 daemon 推送方法注入 kind: 'monitor'，而这条路径在生产中会被直接拒绝：acpAgent.ts:10571-10576 对任何非 agent 的 kind 抛出 Invalid background notification kind，daemon 侧的类型也是 kind: 'agent'（bridgeTypes.ts:1405-1413）。真实的 monitor terminal notification 是在 child 侧经 monitorRegistry.setNotificationCallback 进入私有的 #enqueueBackgroundNotification，同步 push（Session.ts:9709-9710）并在同一个 tick 上产生 hold。因此仓库中唯一的「monitor + activeWork」覆盖钉住的是生产不可达的状态，并断言了与已发布行为相反的结论 —— 下一位维护者会把它当作已强制执行的约定，从而删掉 Session.ts:4066 的 queued-monitor 项，而测试套件还会为这次删除背书。
证据：
基于真实 harness 的 test-efficacy probe（没有 diff 自带的测试可收集）：
probed: [] mutants: 0 hunks: 0 harnessValidated: null
-> 回退该 hunk 后，所有相关套件与现在一样全绿
env -u QWEN_RUNTIME_DIR npx vitest run src/acp-integration/session/Session.test.ts \
-t 'does not hold for a Monitor notification'
Tests 1 passed | 850 skipped (851)
PROBE-A kind=monitor 的结果：REJECTED Invalid background notification kind
session.enqueueBackgroundNotification calls: []
PROBE-B2 真实 monitor 派发后立即（无 await）的 holds：
[{"category":"session","id":"session:active-turn"}]
recordNotificationStrict calls on this path: 0
PROBE-B1（positive control）agent acceptance 窗口期间的 holds：
[{"category":"notification","id":"probe-b1-agent"}]
flip： 回退 diff hunk Session.ts:3 后 B2 变为 []，B1 不变
建议把 Session.test.ts:1252 的 holdIds() 改为使用导出的 ActiveWorkHoldCategory —— 像 active-work-reporter.ts:11 那样 import —— 而不是再加第三份枚举副本。然后在既有的 active work holds describe 中补用例，针对每个触发条件分别断言恰好一个 { category: 'session', id: 'session:active-turn' } hold（未释放的 beginHistoryMutation()、进行中的 goal turn、进行中的 cron turn、已排队的 monitor item、notificationCompletion 已置位且三个 current* 标记都为空的运行中 monitor continuation），并在各自 settle 后断言为 []；再补上协议文档承诺的否定用例 —— 运行中的 Monitor registry 条目、进行中的 follow-up suggestion、以及仅有前台 prompt 时都不产生 session hold —— 以及 agent/shell/workflow continuation 只产生自身 category、不产生 session hold。把 Monitor 测试改成钉住生产可达的状态（捕获 mockMonitorRegistry.setNotificationCallback 的回调，并在持有 close gate 的情况下触发它，做法参照 Session.test.ts:1420 的 holds a queued shell notification before its continuation can start），并删掉那句 scope 注释。
每个新用例都必须在上方代码块中对应项被删除时变红；否定控制用例必须在三个标记 guard 被移除、或 #drainNotificationQueueExclusive 中 kind 字段赋值被移到某个 await 之后时变红；changes 计数用例必须在六处新增 #activeWorkChanged() 调用中任意一处被删除时变红。而今天实测的变异 —— 回退这个 hunk —— 不会让任何测试变红。新测试有四点约束：Session.test.ts:1252 的 holdIds 参数 union 必须先放宽（strict TS，不允许 any）；无论队列多深都必须只有一个聚合 hold，因为 ACTIVE_WORK_MAX_SESSION_HOLDS = 1024（bridgeTypes.ts:376）是 daemon 对 snapshot 与拒绝同样执行的每 Session 上限（bridge.ts:3487）；monitor item 只能经 child 侧 registry 回调进入 notificationQueue，所以通过公开 enqueueBackgroundNotification() 驱动的测试钉住的是 persistence 窗口而不是排队后的 hold；createReportingSession()（Session.test.ts:1237-1250）会 dispose 并重建 Session，因此 registry mock 必须在它之前安装，且 change counter 是第 6 个构造函数参数。
— qwen3.8-max-2026-09-02 via Qwen Code /review (v0.23.0)
|
Released in v0.23.1. |
4 participants
What this PR does
Extends the negotiated active-work hold set with one aggregate
sessioncategory for child-owned work that blocks Session settlement but was not previously reported: goal and cron processing, history mutation, and queued or running Monitor continuations. The same derived hold is used by heartbeat snapshots and conditional close, and existing change notifications now publish its transitions promptly.Why it's needed
Automatic cleanup currently authorizes conditional close from a narrower predicate than the drain waits on. A Session doing one of these unreported jobs advertises no holds, enters close, then waits on that job until the drain deadline and repeats the same failed probe indefinitely. Reporting one aggregate hold makes automatic reclamation lose to work the Session already owns without cancelling or otherwise disturbing that work.
Reviewer Test Plan
How to verify
session:active-turnhold while each operation is outstanding.onlyIfUnheldrefuses immediately with that hold instead of entering the drain timeout loop.sessionas partial and disables ordinary automatic cleanup for that Session.Evidence (Before & After)
N/A — protocol and lifecycle logic only; no UI change.
Tested on
Environment (optional)
No local test, build, typecheck, or CI command was run. Verification was limited to two clean static diff reviews and
git diff --check.Risk & Scope
activeWorkmeaning. During a mixed-version window, a new daemon with an older child reports partial coverage and fails closed; an old daemon cannot consume the new hold until it is upgraded or restarted.Linked Issues
Fixes #11118
中文说明
本 PR 做了什么
在协商式 active-work hold 集合中新增一个聚合的
session类别，用来覆盖会阻止 Session settle、但此前没有上报的子进程自有工作：goal 与 cron 处理、history mutation，以及排队中或执行中的 Monitor continuation。heartbeat snapshot 与 conditional close 共用同一个派生 hold，既有 change notification 也会及时发布这些状态变化。为什么需要
自动清理当前用来授权 conditional close 的判据比 drain 实际等待的判据更窄。Session 执行这些未上报工作时会宣称没有 hold，进入 close 后又等待该工作直到 drain 超时，然后无限重复同一轮失败探测。新增一个聚合 hold 后，自动回收会让位于 Session 已经拥有的工作，而且不会取消或扰动这些工作。
Reviewer Test Plan
如何验证
session:active-turnhold。onlyIfUnheld会立即携带该 hold 拒绝，而不是进入 drain 超时循环。session的旧 child 评为 partial，并为该 Session 禁用普通自动清理。证据（Before & After）
N/A —— 仅协议与生命周期逻辑变更，无 UI 改动。
已测试平台
环境（可选）
未运行本地测试、构建、typecheck 或 CI 命令。验证仅包含两轮干净的静态 diff 审核与
git diff --check。风险与范围
activeWork的语义。混合版本期间，新 daemon 与旧 child 组合会报告 partial coverage 并 fail closed；旧 daemon 在升级或重启前无法消费新 hold。关联 Issue
Fixes #11118