feat(serve): Expose active work state - #8588

E2E test report

Automated verification completed on macOS:

• npm run build && npm run typecheck — passed.
• ACP bridge unit suite — 485/485 passed.
• Session unit suite — 533/533 passed.
• ACP agent unit suite — 383/383 passed.
• Focused deep-health aggregation tests — 13/13 passed.
• Focused daemon startup deep-health test — 1/1 passed.
• Pre-commit Prettier and ESLint checks — passed.

The full serve server test file completed with 865/867 passing. The two failures are pre-existing workspace tool-auth baseline failures unrelated to this diff: passes client identity into the bridge expected 200 and received 401, and 400 invalid_client_id... expected 400 and received 404.

The manual daemon restart scenario has not been executed in this local environment. The prepared E2E plan covers the released-build baseline, activePrompts: 0 with activeWork: true while a background Agent remains active, terminal-notification continuation, FIFO hand-off, managed/draining workspace aggregation, per-Session heartbeat loss, legacy-child compatibility, and the unchanged shallow health response.

Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration.

Force-pushed: the design changed, not just the code

Rebased onto current main (this PR was conflicting) and reworked in response to review. If you looked at the previous revision, the mechanism is different now — please re-read rather than diffing against memory. The PR body has been rewritten to match.

Removed: the per-Session boolean heartbeat and the 45s channel recycle. Inferring channel death from one Session's silence kills every Session on that process, and a host suspend, a long event-loop stall, or a single dropped notification are all indistinguishable from a wedged child. Transport liveness is now its own layer (new PR 2 in #8586).

Replaced with: channel-wide full snapshots of named holds, derived on every report from the owners of the work rather than kept in a parallel ledger; a three-state daemon cache that distinguishes "never negotiated" from "not yet heard from"; and a conditional close where the child confirms under its own close gate before anything is destroyed.

Added: activeWorkReporting and activeWorkStaleMs, so activeWork: false can be told apart from "no child reported". The busy rule for controllers is three terms now, not two.

Concrete bug fixed along the way: agent holds key on hasUnfinalizedTasks(), not hasRunningTasks(). A cancelled Agent still owes its terminal notification, and the previous predicate made a detached Session look idle for that whole window — reaping it and stranding the notification.

Full reasoning: #8586 (comment)

Please note the testing status section in the body: unit suites are green, but end-to-end has not been run and none of the seven reviewer test items were executed manually.

Self-audit: three things to fix before this leaves draft

I re-audited this adversarially. The read side — the part the title is about — holds up. The write side does not, and the three problems below share one root cause, so they should be fixed as one change rather than three patches.

Since this is a draft I am not calling these merge blockers, but they are design defects rather than unfinished work: the functions involved are all written, and it is the guard model itself that does not hold.

What is actually fine

The exposed surface is a whitelist projection of three scalars on GET /health?deep=1 — no object serialization, no absolute paths, no $HOME, no prompt text, no tool arguments, no tokens. Hold payloads ({category, id}) stay in the daemon's in-memory SessionEntry.childHolds and never reach an HTTP response. Auth reuses the existing /health tiering, and the CORS deny-wall plus host allowlist are registered ahead of the pre-auth /health route, so page JS cannot read it. Scope matches the existing sessions / activePrompts fields — process-global aggregation, not a new scope. collectActiveWorkHolds() is derived per call rather than a ledger, so it does not grow with session lifetime. Cleanup is sound: closeSessionImpl deletes the entry and all three getters fall to zero.

The root cause

This PR promotes a cached child-reported snapshot from a hint into the authority that permits destroying a session. Three destruction paths now consult it, their guards are inconsistent with each other, and every one of them is weaker than what main had.

1. Snapshot absence tears down sessions a user is actively watching. The absence loop checks only activeWorkCloseInFlight and entryHasLocalWork. It does not check entry.events.subscriberCount or entry.clientIds.size — and maybeCloseIdleSession treats both as hard guards. So a session with a live SSE subscriber and a registered client is destroyed because one snapshot omitted it.

This also contradicts the PR description directly. I wrote that an unreported or unconfirmed Session is retained, never destroyed. bridge.test.ts:406-428 encodes the opposite as expected behaviour: the session is never detached, its spawn-owner clientId is still registered, and the test asserts sessionCount goes to 0 after an empty snapshot. The description claims a guarantee the test proves absent. That wording has to change or the behaviour does — currently they disagree, and the test would keep the wrong one honest.

2. A ten-second TOCTOU window that main does not have. confirmChildUnheld is a withTimeout(..., ACTIVE_WORK_CLOSE_TIMEOUT_MS) of 10 seconds, and it never sets entry.closing. So between the last guard and closeSessionImpl setting closing = true there is a window up to ten seconds wide, and attach, sendPrompt and rewindSession all gate on closing alone. On main the detach path was a synchronous guard sequence followed directly by closeSessionImpl, whose first await comes after closing is set — the transition was atomic. I split it. A client that attaches inside the window gets its session destroyed out from under it, with the prompt lost.

3. Arbitrarily stale cache authorizes reaping. entryHasActiveWork never looks at childHoldsAt. Staleness is implemented — activeWorkReporting compares against intervalMs * ACTIVE_WORK_STALE_INTERVALS — but only to report health, never to gate destruction. So a child that goes silent after emitting one empty snapshot leaves a cache that permits reaping indefinitely, even if it has since restarted a background agent.

Fix direction

One shared helper for "may this session be destroyed", with a single in-flight flag and a single freshness gate, used by all three paths. Concretely: add the in-flight gate before confirmChildUnheld and have attach/sendPrompt/rewindSession check it; give the absence loop the same subscriber/client guards maybeCloseIdleSession uses, and recover a lost close response by re-sending sessionClose{onlyIfUnheld} for that one session instead of treating absence as consent; make entryHasActiveWork fall back to the conservative childHolds === null semantics once the snapshot is stale.

Also worth noting: confirmChildUnheld's comment argues that a hold cannot appear between check and teardown while the gate is held. That is true on the child side and I verified it, but it says nothing about the daemon-side window in item 2, and a reader will take it as covering the whole path. It should say what it does not cover.

Smaller items, not part of the above

• A runtime with zero sessions reports 'full', which sets reportingAny and makes a mixed deployment aggregate to 'partial' when real coverage is zero. No test can currently produce an aggregate 'none'.
• activeWorkOldestReportAt does not use the covered predicate that activeWorkReporting uses, so with sessions present but all stale it returns a positive activeWorkStaleMs while activeWorkReporting says 'none'. Three places document the opposite ("0 when nothing is covered").
• setStatusChangeCallback is a single slot, and Session now takes it unconditionally and clears it to undefined on dispose even if someone else owned it. Harmless under ACP today because the TUI is absent, but it should be an add/remove listener pair.
• parseActiveWorkSnapshot validates every field type and discards the whole packet on malformation (correct), but caps neither sessions[] nor holds[] length. Zero-cost to add.
• activeWork only walks byId, so it reports false while hasNoChannelWork is simultaneously refusing to kill the channel for in-flight spawns, pending restores, or MCP discovery. A field whose stated purpose is "can this daemon be reclaimed" should not disagree with the daemon's own answer.

Thanks — both findings taken, and one correction to the record: this review is against fb68eeb53, but that commit is not the current state of the work. An adversarial self-audit after it turned up three defects in the write side that the review's "safety posture is consistent throughout" reading does not cover, so the fixes below are on top of a guard rework, not a patch to the tree you read.

Finding 1 — the "unknown" state promises an ask the code never made. Correct, and it survived the rework unchanged. Fixed by splitting the predicate on what it actually knows rather than by softening the wording: childReportsHeldWork is positive knowledge only, childWorkIsUnknown is the absence of a gradeable report. The health surface ORs both (a controller must never read "nobody told me" as "nothing is running"); automatic cleanup blocks only on known work and lets unknown through to confirmChildUnheld.

Worth stating why the wording was not the thing to change. Skipping on unknown looks like the safe direction and is in fact the worse failure — nothing resolves it, the reaper skips it too, and a Session on a channel that went quiet is retained forever with no path out. Asking resolves it definitively: the child answers under its own close gate whether or not its snapshots are arriving, the round trip is bounded, and every non-answer still retains. Two regression tests cover it: the ask happens for a never-reported Session, and a refusal resolves the unknown toward retention with the reason attached.

Finding 2 — parseActiveWorkSnapshot between two import blocks. Fixed, pure relocation.

What the review could not have seen. The self-audit found that this PR had promoted a cached child report from a hint into the authority that permits destroying a Session, across four teardown paths whose guards disagreed with each other:

1. Absence from a snapshot tore down a Session with a live SSE subscriber and a registered client — guards maybeCloseIdleSession treats as hard. That contradicted this PR's own "unreported or unconfirmed is retained, never destroyed" claim, and bridge.test.ts asserted the destruction as expected behaviour.
2. confirmChildUnheld is a 10s round trip that never set closing, so attach/prompt/rewind could be admitted into a Session already authorized for teardown. On main that sequence was synchronous; splitting it opened the window.
3. Staleness was computed but only ever used to grade health, never to gate destruction — so a child that went quiet after one empty report left a cache that permitted reaping indefinitely.
4. The idle reaper did not call confirmChildUnheld at all.

All four are the same defect with four exits, so they are fixed as one change: one shared candidacy predicate, one in-flight flag held across confirm-then-teardown, one freshness gate, and the reaper routed through the same ask. The absence loop is gone — because reports are complete, absence and reported-with-no-holds are the same fact and now take the same path.

On the two deferred items: agreed that /verify is the right instrument, since the central claims are live-lifecycle behaviours that the unit suite pins only through mocked channels — the earlier "E2E test report" comment on this PR was automated verification, not a live lifecycle exercise, and I have not run one. macOS/Windows remain untested. Neither the docs/comment mismatch nor the four items above would have been caught by CI, which is the honest argument for the sandboxed lane before merge.

Re-review will need to happen at the new head rather than fb68eeb53.

Self-audit + triage findings addressed — head is now 41853db8

Two commits on top of fb68eeb53 (appended, not force-pushed, so existing review comments stay anchored). Everything reviewed at fb68eeb53 predates them.

The root cause worth stating plainly

This PR had promoted a cached child report from a hint into the authority that permits destroying a Session. Four teardown paths consulted it, their guards disagreed with each other, and every one was weaker than what main had. They are one defect with four exits, so they are fixed as one change rather than four patches.

Defect 1 also contradicted this PR's own description — it claimed an unreported or unconfirmed Session is retained, never destroyed, while bridge.test.ts asserted the destruction as expected behaviour. Both are corrected; the description is updated, and the test now asserts retention under a registered client and under a live subscriber.

Triage findings

• "unknown promises an ask the code never made" — correct, and it survived the rework. Fixed by splitting the predicate on what it actually knows rather than by softening the wording: childReportsHeldWork (positive knowledge) vs childWorkIsUnknown (absence of a gradeable report). Health ORs both; cleanup blocks only on known work and lets unknown through to the ask. Skipping on unknown looks safe and is the worse failure — nothing resolves it, so the Session is retained forever with no path out.
• parseActiveWorkSnapshot between two import blocks — fixed, pure relocation.

Also in scope

Snapshot sessions[] / holds[] are now bounded; health coverage is exposed as counts and graded once daemon-wide (an empty runtime is vacuously full and could vouch for another workspace's unreported Sessions); activeWorkStaleMs counts only covered Sessions; the background-task status callback is retracted by identity instead of blanking a single-slot setter the TUI shares; and activeWork's Session-scoped boundary vs channel-level work is documented rather than widened.

Verification

New regressions: retention under a registered client and under a live subscriber; recovery of a lost close response; the stale-snapshot gate; admission refusal during a conditional close; the reaper's confirmation; the oversized-snapshot discard; the mixed empty/uncovered health aggregate; and the ask-on-unknown path in both directions.

Still not done

No live end-to-end run. Agreed that @qwen-code /verify is the right instrument — the central claims are lifecycle behaviours the unit suite pins only through mocked channels, and note that none of the four defects above would have been caught by CI, which is the honest argument for the sandboxed lane before merge. macOS and Windows remain untested.

CI: Check voice guard mirror sync was base drift, not this diff — synced in 80019aca

Test (ubuntu-latest, Node 22.x) failed on 41853db8 at step 19:

npm error Missing script: "check:voice-guard-sync"

Not a flake and not in this diff. The step and the script it calls were added together in 732f4d8 (#8350); the workflow runs from main, this branch was 46 commits behind, so the check invoked a script absent from the checkout. A rerun would have failed identically.

Merged main in rather than rebasing, so the existing review comments stay anchored — the repo's own force-push reminder is the reason. Merge was conflict-free. npm run check:voice-guard-sync now passes locally.

Re-verified on the merged tree, because main had touched many of the same files (acpAgent.ts, bridgeTypes.ts, Session.ts, health-demo.ts, background-tasks.ts, plus run-qwen-serve.ts and server.ts):

The two serve failures are Live conversation runtime lifecycle — a pre-existing cross-file flake in this file, unrelated to active work. All 6 tests in that describe block pass in isolation, and the same file fails a different test on different runs.

Two interactions worth naming explicitly, since both are the class of thing a clean textual merge hides:

• main changed AcpSessionBridge in bridgeTypes.ts too — getChildResourceSnapshot gained an optional ageMs. Independent of activeWorkCoverage; no interaction.
• main added 184 lines to acpAgent.test.ts, the same file whose Session mock previously caused an uncaught exception in the reporter. Checked directly: none of the new tests construct a Session, and the SessionMock.prototype.collectActiveWorkHolds patch survived the merge.

Also picked up cb3dc107f (#8604), which deflakes the GlobTool external-path test that had to be reran on an earlier revision of this PR.

The three route checks reported cancelled on separate workflow runs — CI runner routing pre-empted by the newer push, no action needed; the merge push re-triggers them.

@qwen-code /triage

Runtime verification (maintainer review)

I built a real end-to-end environment for this PR and executed all seven reviewer test-plan items, plus a genuine mixed-version run. All seven pass. Two non-blocking notes below.

Setup. Both arms compiled from source into runnable dist artifacts and driven as real processes — no mocks of the bridge, the child, or the health route:

Reviewer test plan

The decisive A/B (item 3)

Same script, same mock, two builds:

Extra checks beyond the plan

• Capability negotiation captured verbatim: daemon sends {"v":1,"intervalMs":15000}, child answers {"v":1,"intervalMs":15000,"categories":["agent","notification"]}, then publishes channel-wide full snapshots on qwen/notify/channel/active-work.
• unknown grading is honest. With every snapshot dropped by the tee, an adopted hold set ages past 3 × 15 s and the daemon degrades to activeWorkReporting: "partial", activeWorkStaleMs: 0, activeWork: true — retained, and correctly flagged as not-vouched-for.
• Mutation teeth on the hasUnfinalizedTasks() predicate. Driving the real compiled BackgroundTaskRegistry: inside the cancel window hasRunningTasks() is false, hasUnfinalizedTasks() is true, and listUnfinalizedBackgroundAgentIds() still returns the id. Patching the compiled dist to the running-only predicate makes it return [] — the choice is load-bearing, exactly as documented.
• Unit suites at PR head, all green: acp-bridge 498, cli acp-integration 929, cli serve + Session 1230, core background-tasks 123 — 2780 tests, no failures (I did not hit the Live Appshot flake).

Note A — a prompt inside the confirm window is accepted (202) and lost

rewindSession and spawnOrAttach throw synchronously, so REST answers 404 "The session is closing" — verified. sendPrompt instead returns a rejected promise, and the route's try/catch around it only catches synchronous throws, so it has already replied 202 {promptId}. Measured inside a stalled 10 s confirm window:

POST /session/:id/rewind -> 404  "The session is closing"
POST /session/:id/prompt -> 202  {promptId}
    daemon log: [SessionNotFoundError] ... The session is closing; retry after close completes
    events published for that promptId: 0

The caller gets no error and nothing on the event stream. This shape predates the PR (if (entry.closing) return Promise.reject(...)), so it is not a regression — but the PR widens the window it applies to from a fast synchronous teardown to a bounded 10 s round trip, and test-plan item 5 states the prompt is "refused rather than accepted and lost", which is not what a REST caller observes. Worth either softening that sentence or making sendPrompt's guard throw synchronously like its two siblings.

Note B — the conditional close does not carry the daemon's drain budget

confirmChildUnheld sends only {sessionId, onlyIfUnheld} (confirmed on the wire), so the child falls back to SESSION_DRAIN_TIMEOUT_MS = 30_000, while the real close sends drainTimeoutMs = initTimeoutMs * 0.8 = 8000 and the daemon itself only waits ACTIVE_WORK_CLOSE_TIMEOUT_MS = 10_000. A Session whose drain legitimately takes 10–30 s therefore times out the confirm and is retained for a later round. Self-correcting and in the safe direction, but it is an extra cycle the 8 s budget exists to avoid — consider forwarding drainTimeoutMs on the conditional call. (The missing parameter is runtime-confirmed; the consequence is a code read.)

Verdict

The mechanism does what the description says, the failure direction is genuinely the safe one (unreported or unconfirmed ⇒ retained, never destroyed), and the mixed-version path leaves old children behaving exactly as before. The two notes are documentation/polish, not correctness blockers.

LGTM — recommend merge. Note A's wording is worth a one-line fix before or after merge.

POST /session/:id/rewind -> 404  "The session is closing"
POST /session/:id/prompt -> 202  {promptId}
    daemon 日志: [SessionNotFoundError] ... The session is closing; retry after close completes
    该 promptId 产生的事件数: 0

Review round addressed — 3 Critical fixed in 12adf869, 9 Suggestions deferred

All three [Critical] findings were real. I verified each against the code rather than taking the tag, and one had an inaccurate provenance claim that I've corrected in-thread while accepting the underlying bug.

Fixed

All three share one cause: turning a synchronous guard-then-teardown into an awaited span made three previously unobservable mid-teardown states reachable. That is the same root cause as the guard rework earlier in this PR, which is the honest reading — the confirm window is genuinely the risky part of this design, and this is the third round of finding things inside it.

Verification: acp-bridge 1111/1111, active-work subset 18/18 including a new regression for the restore-path admission refusal. The other two fixes are pinned only by reading the code paths — a mid-restore snapshot and a kill-then-reload interleave are not something the mocked-channel harness can stage honestly, and I'd rather say so than imply test coverage I don't have.

Deferred — 9 Suggestions, replied and resolved individually

Not disputed, and none silently dropped. This is round 6+ of automated review on a PR that already carries a maintainer approval, so autonomous changes are held to critical-level findings rather than accumulating commits on an approved diff.

Five are flagged to the author as worth promoting, because they are the same class of problem this PR has already had to correct twice — a claim broader than the implementation:

• The refusal-response hold set is adopted unbounded, while parseActiveWorkSnapshot caps the identical payload. Capping only one of two ingestion paths is an inconsistency introduced here.
• A single huge-but-valid seq permanently latches the channel high-water mark, costing the self-healing property the design doc advertises (fails closed, so no destruction).
• The reporter's eager constructor publish appears to be always discarded, which would make the comment justifying it false.
• Design doc drift: the close-response success shape, and the child proposes, the daemon clamps is inverted.
• Protocol doc drift: the activeWork enumeration omits the fail-closed unknown ⇒ active behavior a restart controller depends on.

The remaining four are bounded churn or scope (enqueueBackgroundNotification, setSessionApprovalMode / setSessionModel, a dead readonly, and the test-coverage gap — partly exercised by the maintainer's live run, though not by a committed test, so the mutation described would still pass CI).

Threads: 12/12 replied and resolved. 0 unresolved.

Runtime re-verification at 12adf869 — the three critical fixes

My earlier report tested 80019acab7. Three [Critical] fixes landed since, and the author states two of them are "pinned only by reading the code paths" — a mid-restore snapshot and a kill-then-reload interleave. I rebuilt the environment at the new head and went after exactly those.

Setup. Three arms, all compiled from source into runnable dist and driven as real processes — no mocked bridge, no mocked child, no mocked health route:

The three fixes

Fix 1 — restore admission (confirmed)

Same driver, same stalled child (conditional-close response held 7 s), two daemons. POST /session/:id/load issued 1.2 s into the confirm window:

On 80019ac the caller is told 200 {"attached":true, clientId:…} and the teardown it raced then destroys the Session under it — no error ever reaches the client. On 12adf869 it is refused up front with 404 "The session is closing; retry after close completes". This is a real defect, really fixed.

The full admission matrix inside an 8 s window at the new head is in the screenshot: load 404, rewind 404, attach-by-id 409, prompt 202 then lost, DELETE 409.

Fix 2 — identity re-check (could not be staged, and that is informative)

To reach it I need the id removed and re-registered during the confirm round trip. Over REST that is not reachable at this head:

• there is no kill route (killSession is only called from sub-session/archive/rollback paths), and
• DELETE /session/:id inside the window returns 409 session_archiving, Retry-After: 5 — because POST /session/:id/detach runs under withOwnerMutableSession, which holds a shared archive lock for the whole handler, and the handler now awaits the conditional close.

So the lock that blocks my repro is also what narrows this race's real-world exposure — the re-registration has to come from a non-REST path. The guard costs one map lookup and fails safe; I'd keep it, but it stays code-read-only.

Fix 3 — mid-restore exclusion (inconclusive, and I tried hard)

I could not reach the state it prevents:

• The window is ~1 ms. Polling /health?deep=1 at ~5 samples/ms across a 501 ms session/load of a 1200-record transcript, exactly 1 of 2453 samples saw the entry registered while the load was still in flight.
• Snapshot path: replaying the child's own qwen/notify/channel/active-work snapshot 700 times at 1 ms resolution across the whole restore did not tear the Session down on 80019ac.
• Reaper path: with --session-reap-interval-ms 1 --session-idle-timeout-ms 1, both builds behave identically — the conditional close for the restoring id goes out 5–6 ms after the child's load response on 80019ac and on 12adf869. I cannot separate "decided inside the guarded window" from "decided in the unguarded tail between the restore promise settling and the HTTP response being written", so the test does not discriminate.

Not evidence the fix is wrong — evidence that the live path is too narrow to pin. If you want it pinned, a unit test with a controllable await inside the restore is the only honest way.

What still holds at the new head

• Headline claim, re-proven against the merge base. Main prompt settled with a background Agent still running: activePrompts: 0, activeWork: true, reporting: "full". Detach the last client → merge-base destroys the Session (404, channel dead, Agent killed with it); PR head retains it (200). Agent + terminal notification settle → clean close, sessions: 0, activeWork: false.
• Negotiation and holds captured verbatim. Daemon sends initialize._meta {"v":1,"intervalMs":15000}; child answers {"v":1,"intervalMs":15000,"categories":["agent","notification"]}; holds appear as {"category":"agent","id":"general-purpose-call_3"} while activePrompts is already 0.
• Fail-closed is honest. A Session on a negotiated channel that has not been named by a snapshot yet reads activeWork: true, reporting: "partial" until its first report — the documented "unknown ⇒ retained" state, observed live.
• Unit suites at 12adf869: acp-bridge 499/499, cli acp-integration 929/929, cli serve 1218/1218, core background-tasks 123/123 — 2769 tests, no failures. CI on this head is green on every job.

Notes (non-blocking)

A. A prompt inside the confirm window is still accepted (202) and lost. Unchanged from my last report and re-measured here: POST /session/:id/prompt → 202 {promptId}, and after the window GET /session/:id/pending-prompts → 404. load and rewind both throw synchronously and answer 404; sendPrompt returns a rejected promise the route's try/catch cannot see. Pre-existing shape, but the PR widens the window it applies to, and test-plan item 5 claims the prompt is "refused rather than accepted and lost", which is not what a REST caller observes.

B. The conditional close still omits the drain budget. On the wire: {"sessionId":…,"onlyIfUnheld":true} versus the real close's {"sessionId":…,"drainTimeoutMs":8000}. The child falls back to SESSION_DRAIN_TIMEOUT_MS = 30_000 while the daemon waits only 10 s. Self-correcting, safe direction, one wasted cycle.

C. Detach costs one extra bounded round trip. I checked whether this is new before reporting it: with every session/close response stalled 8 s, POST /session/:id/detach takes 8019 ms on the merge base, 16017 ms on 80019ac, 16016 ms on 12adf869. So blocking detach is pre-existing; the PR adds one bounded confirm in front of it (worst case 10 s confirm + 8 s drain). With a healthy child it is 15 ms. Worth knowing because the route holds its shared archive lock for that whole span, so DELETE/archive on that Session are refused with 409 meanwhile — "explicit close keeps force semantics" is true of the bridge, but not of what a REST caller sees.

Verdict

The one fix that is observable from outside the daemon is genuinely fixed, and I reproduced the bug it fixes on the previous head. The other two are in the safe direction and cheap; I could not stage either, which matches the author's own statement rather than contradicting it. Everything I verified last round still holds at 12adf869, and the failure direction remains "unreported or unconfirmed ⇒ retained, never destroyed".

LGTM — recommend merge, my previous approval stands at this head. Note A is worth one line of code or one line of prose before or after merge.

@qwen-code /triage

@qwen-code /review