feat(serve): confirm idle before closing, and grade the health signal

Completes the active-work rework with the two facts a restart controller was still missing and the one guarantee automatic cleanup was missing. Automatic cleanup no longer destroys a Session on the strength of a cached snapshot. It asks the child to close only if unheld, and the child answers under its own close gate — with the gate held no prompt is admitted and no automatic turn starts, so a hold cannot appear between the check and the teardown. A refusal hands back the current holds and the daemon adopts them. An unanswered request is neither retried nor assumed: the Session stays, and the next snapshot settles it, because a Session absent from one has provably been released. Every automatic path — detach, attach rollback, prompt settle, notification settle, a child reporting itself idle — now funnels through one decision point instead of four near-copies. Health gains activeWorkReporting and activeWorkStaleMs. Without them activeWork:false cannot be told apart from "no child told me anything", which is the one case where acting on it is unsafe. Freshness is graded by the daemon rather than the controller, since the cadence is negotiated per channel; a stale snapshot or a child omitting a category degrades the grade instead of silently narrowing what the boolean covers. Tests: acp-bridge 489/489, acpAgent 383/383, Session 534/534, serve suites 1188 with one pre-existing cross-file flake in the Live Appshot integration tests (reproduces on the unmodified tree, failing a different test each run). Co-Authored-By: Claude Opus 5 <[email protected]>
doudouOUC · Aug 8, 2026 · Aug 5, 2026 · Aug 6, 2026 · Aug 6, 2026 · Aug 6, 2026
commit 612bcb7330818df35dcd85f88c81182b01b63d9f
diff --git a/docs/design/2026-08-06-active-work-health.md b/docs/design/2026-08-06-active-work-health.md
@@ -0,0 +1,99 @@
+# Active-work health signal
+
+## Problem
+
+`activePrompts` counts prompts currently dispatched to an ACP child. A prompt can finish after starting background Agents, leaving `activePrompts` at zero while session-owned work is still running. A restart controller that reads zero active prompts as idle can restart the daemon before those Agents finish and before their terminal notifications reach the parent session.
+
+## Scope
+
+`GET /health?deep=1` gains three fields: `activeWork`, `activeWorkReporting`, and `activeWorkStaleMs`.
+
+`activeWork` is true while any managed workspace has an accepted-but-unsettled prompt, a running background Agent, or an Agent terminal notification that is queued, awaiting acceptance, or being processed by its parent continuation. It deliberately does **not** cover background shells, Monitors, workflows, or cron. That exclusion is a scope decision, not an oversight: those categories have no equivalent signal today, and a controller that treats `activeWork: false` as "nothing at all is running" will be wrong about them.
+
+Restart policy stays with the external controller. The daemon publishes facts; it does not publish `restartSafe`.
+
+## Why holds, and why full snapshots
+
+Each Session reports a set of named **holds**, each carrying a category (`agent`, `notification`). Two properties follow, and both are the point:
+
+**Holds are derived, never maintained.** `Session.collectActiveWorkHolds()` reads the owners of the work — the background-task registry's unfinalized set, the notification queue, the in-flight acceptance and continuation state — on every call. There is no acquire/release ledger kept alongside the work, because a ledger can miss a release, and a leaked hold would pin its Session forever while every snapshot faithfully republished the leak.
+
+The agent category uses `BackgroundTaskRegistry.hasUnfinalizedTasks()`'s predicate rather than `hasRunningTasks()`'. A cancelled agent still owes its terminal task-notification: `cancel()` flips status and emits a status change, but the notification arrives later from `finalizeCancelled()` or the 5s grace timer. Keying on "running" would make the Session look idle inside that window, and a detached Session would be closed with the notification still owed.
+
+**Reports are complete snapshots at channel scope, not per-Session transitions.** One message per ACP channel carries every Session the child owns and every hold it holds:
+
+```json
+{
+  "v": 1,
+  "seq": 12,
+  "sessions": [
+    { "sessionId": "…", "holds": [{ "category": "agent", "id": "a1b2" }] }
+  ]
+}
+```
+
+A dropped report therefore costs one interval of staleness and needs no retransmit, ack, or "last reported" state to diff against — the next snapshot is the whole truth again. `seq` guards against reordering only; a gap is not an error. Channel scope is what keeps an always-on cadence affordable (one small message per interval regardless of Session count) and it gives the daemon a second fact for free: a Session **absent** from a fresh snapshot is positive evidence the child released it.
+
+Prompts are absent from the child's report on purpose. The daemon accepts, queues, dispatches, and settles them, so its own `pendingPromptCount` is authoritative and strictly wider — it covers prompts still waiting in the FIFO, which the child cannot see. Reporting them from both sides would create two sources of truth for one fact with nothing to reconcile them.
+
+## Ordering
+
+A snapshot is flushed ahead of the prompt response on the same stream. The daemon drops its pending-prompt count the instant that response lands, so a hold the prompt left behind — a background Agent it started — must already be on the wire, or the daemon briefly sees neither fact.
+
+## Three states, and closing atomically
+
+Per Session the daemon holds one of:
+
+- **unsupported** — the channel never negotiated. Contributes nothing; pre-existing cleanup behavior applies unchanged. Treating this as "unknown" would make every legacy Session permanently unreapable.
+- **unknown** — negotiated, not yet heard from. Reads as retained, but is not a state the daemon sits in: it asks.
+- **known** — a snapshot has been applied.
+
+The cache decides _when_ it is worth asking. It never authorizes destruction, because a fresh empty snapshot only describes the moment it was built and work can start in the gap. So automatic cleanup closes through a conditional RPC:
+
+```
+qwen/control/session/close { sessionId, onlyIfUnheld: true }
+  → { closed: true, holds: [] } | { closed: false, holds: [...] }
+```
+
+The child evaluates it under its own close gate, before anything destructive runs. With the gate held the Session admits no new prompt and starts no new automatic turn, so a hold cannot appear between the check and the teardown. If holds exist, the gate is released and they are handed back; the daemon adopts them and backs off.
+
+On timeout the daemon cannot tell whether the child closed. It does not retry and does not assume: it leaves the Session in place and lets the next snapshot settle it — present means the close never happened, absent means it did. A genuinely wedged channel is not this mechanism's problem; see below.
+
+Explicit close, kill, shutdown, and channel exit keep their force semantics and do not go through this path.
+
+## What this deliberately does not do
+
+There is no heartbeat watchdog and no channel kill driven by work state. Inferring "this channel is dead" from "one Session stopped reporting" kills every Session on that process, and a suspend, a long event-loop stall, or a single dropped notification all look identical to a stalled child. Three separate concerns, three separate mechanisms:
+
+| Concern                                                 | Mechanism                                 |
+| ------------------------------------------------------- | ----------------------------------------- |
+| Transport / process liveness                            | channel ping-pong (separate change)       |
+| Agent logic stalling while the process stays responsive | progress-based watchdog (separate change) |
+| Session work retention                                  | this document                             |
+
+Killing a whole multiplexed channel is reasonable when the channel is _actually_ dead — every Session on it is unreachable anyway. It is not reasonable as an inference from one Session's reporting.
+
+## Health surface
+
+| Field                 | Meaning                                                               |
+| --------------------- | --------------------------------------------------------------------- |
+| `activeWork`          | OR across runtimes of daemon-owned work and reported holds            |
+| `activeWorkReporting` | `full` / `partial` / `none` — how much of that boolean is vouched for |
+| `activeWorkStaleMs`   | Age of the oldest snapshot it rests on; `0` when nothing is covered   |
+
+Freshness is graded by the daemon, not the controller: the reporting cadence is negotiated per channel (the child proposes, the daemon clamps into an agreed range), so only the daemon can judge it. A stale snapshot or a child that omits a category degrades the grade to `partial` rather than silently narrowing what the boolean covers. `activeWorkStaleMs` is diagnostic.
+
+Controllers should treat the daemon as busy when:
+
+```ts
+const busy =
+  health.activePrompts > 0 ||
+  health.activeWork ||
+  health.activeWorkReporting !== 'full';
+```
+
+`activePrompts` keeps its exact previous meaning as an independent compatibility signal.
+
+## Limits
+
+This is an observation cache, not a restart lease. Even a fresh, empty, fully-graded snapshot describes the moment it was taken; new work can begin immediately afterwards. The rule above substantially lowers the risk of a wrong restart — it does not eliminate it. Strict safety needs a prepare-restart fence that stops new work admission, confirms the drain, and only then shuts down. That is graceful shutdown, and it is out of scope here.
diff --git a/docs/design/active-work-health.md b/docs/design/active-work-health.md
diff --git a/docs/design/daemon-global-deep-health.md b/docs/design/daemon-global-deep-health.md
@@ -24,6 +24,8 @@ that are draining but have not completed bridge cleanup.
 | `pendingPermissions` | Sum                                                      |
 | `activePrompts`      | Sum                                                      |
 | `activeWork`         | True when any managed runtime reports active work        |
+| `activeWorkReporting`| Worst grade across runtimes (`full`/`partial`/`none`)    |
+| `activeWorkStaleMs`  | Age of the oldest snapshot behind it; `0` when uncovered |
 | `connectedClients`   | Existing daemon-wide REST SSE count                      |
 | `channelAlive`       | True when any managed runtime channel is live            |
 | `lastActivityAt`     | Latest non-null bridge activity time                     |

diff --git a/docs/developers/qwen-serve-protocol.md b/docs/developers/qwen-serve-protocol.md
@@ -487,18 +487,31 @@ Pass `?deep=1` (also accepts `?deep=true` or bare `?deep`) for a daemon-wide pro
   "pendingPermissions": 1,
   "activePrompts": 1,
   "activeWork": true,
+  "activeWorkReporting": "full",
+  "activeWorkStaleMs": 4200,
   "connectedClients": 2,
   "channelAlive": true,
   "lastActivityAt": "2026-07-15T08:30:00.000Z",
   "idleSinceMs": 120000
 }
 ```
 
-`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, or a queued/in-progress Agent terminal notification. It intentionally does not count background shells, Monitors, workflows, cron jobs, or follow-up suggestions. `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` **does not count background shells, Monitors, workflows, cron jobs, or follow-up suggestions** — it is true when any runtime has an accepted but unsettled prompt (including a FIFO-waiting prompt), a running background Agent, or a queued/in-progress Agent terminal notification, and nothing else. `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 categories, `none` when no session is, `partial` for anything between — including a stale snapshot or an older child that never acknowledged the capability. `activeWorkStaleMs` is the age of the oldest snapshot the boolean rests on 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). `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.
 
-Restart controllers that understand `activeWork` should treat the daemon as busy when `health.activeWork === true || health.activePrompts > 0`. Unknown responses, failed probes, and insufficient idle grace should prevent restart. `activePrompts` remains an independent compatibility signal; `activeWork` is a fact about the scoped work above, not a complete `restartSafe` policy.
+Restart controllers should treat the daemon as busy when:
 
-> ⚠️ The deep probe is **informational**, not a real liveness verification or an atomic reclaim lease. Negotiated ACP children send per-Session active-work heartbeats, allowing the daemon to recycle an owning channel after 45 seconds without a report while active; this detects a wedged child process, event loop, or transport, but not background Agent logic that stalls while the child can still send heartbeats. `connectedClients` counts REST SSE connections, not every ACP transport. Use repeated samples and graceful shutdown for idle reclamation; use authenticated `/daemon/status` for transport and per-workspace diagnostics. If any managed runtime getter throws, deep health fails closed with `503 {"status":"degraded","reason":"aggregation_failed"}` rather than returning partial totals, and the daemon log identifies the failing workspace runtime. During bootstrap, before the runtime registry is ready, it returns `503 {"status":"degraded","reason":"bootstrap"}` with `Retry-After: 1`. For listener liveness, use the default `/health` without `?deep`.
+```ts
+const busy =
+  health.activePrompts > 0 ||
+  health.activeWork ||
+  health.activeWorkReporting !== 'full';
+```
+
+Dropping the third term makes `activeWork === false` indistinguishable from "no child told me anything", which is the one case where acting on it is unsafe. Unknown responses and failed probes must also prevent restart. `activePrompts` remains an independent compatibility signal.
+
+These fields are an observation cache, not a restart lease: even a fresh, fully-graded, empty answer describes the moment it was sampled, and work can start immediately afterwards. The rule above lowers the risk of a wrong restart substantially but does not eliminate it — strict safety needs a prepare-restart fence that stops new work admission, confirms the drain, and only then shuts down.
+
+> ⚠️ The deep probe is **informational**, not a real liveness verification or an atomic reclaim lease. Negotiated ACP children publish channel-wide active-work snapshots on a negotiated cadence, and the daemon grades their freshness into `activeWorkReporting` — but it never kills a channel over a missing report, because one session's silence is not evidence the process died. Transport liveness and stalled-Agent detection are separate mechanisms. `connectedClients` counts REST SSE connections, not every ACP transport. Use repeated samples and graceful shutdown for idle reclamation; use authenticated `/daemon/status` for transport and per-workspace diagnostics. If any managed runtime getter throws, deep health fails closed with `503 {"status":"degraded","reason":"aggregation_failed"}` rather than returning partial totals, and the daemon log identifies the failing workspace runtime. During bootstrap, before the runtime registry is ready, it returns `503 {"status":"degraded","reason":"bootstrap"}` with `Retry-After: 1`. For listener liveness, use the default `/health` without `?deep`.
 
 **Auth:** required **only on non-loopback binds**. On loopback (`127.0.0.1`, `::1`, `[::1]`) `/health` is registered before the bearer middleware so k8s/Compose probes inside the pod don't need to carry the token. On non-loopback (`--hostname 0.0.0.0` etc.) the route is registered after the bearer middleware and returns 401 without a valid token — otherwise an unauthenticated caller could probe arbitrary addresses to confirm a `qwen serve` exists, a low-severity info leak that combines poorly with port scanning. CORS deny + Host allowlist still apply on the loopback exemption.