Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions docs/design/2026-08-06-active-work-health.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,20 +63,20 @@ The cache decides _when_ it is worth asking. It never authorizes destruction, be

```
qwen/control/session/close { sessionId, onlyIfUnheld: true }
→ { closed: true, holds: [] } | { closed: false, holds: [...] }
→ { sessionId, closed: true } | { sessionId, closed: false, holds: [...] }
```

The child evaluates it under its own close gate, before anything destructive runs. It rejects known holds immediately, drains any turn that was already active when the gate closed, then evaluates the unfiltered collector again. The second read matters because an already-running out-of-scope turn such as cron can register a background shell while it drains. With the gate still held no new turn can start after that final read, so a hold cannot appear between final authorization and teardown **on the child side**. If either read finds holds, the gate is released and they are handed back; the daemon adopts them and backs off.
The child evaluates it under its own close gate, before anything destructive runs. It rejects known holds immediately, drains any turn that was already active when the gate closed, then evaluates the unfiltered collector again. The second read matters because an already-running out-of-scope turn such as cron can register a background shell while it drains. With the gate still held no new turn can start after that final read, so a hold cannot appear between final authorization and teardown **on the child side**. If either read finds holds, the gate is released and they are handed back. The daemon adopts the returned hold set only when it stays within the same 1,024-hold per-Session bound as snapshots; an oversized refusal still retains the Session but does not replace the last valid cache.

The daemon side needs its own cover, because the round trip is an await of up to ten seconds. A Session with a conditional close outstanding is marked in-flight, and every admission path — attach, prompt, rewind — refuses it exactly as it refuses one that is already closing. Without that, a prompt accepted during the round trip is lost when the teardown it raced completes; the previous synchronous guard-then-teardown sequence got this for free, and splitting it is what created the need to say so explicitly.

On timeout the daemon cannot tell whether the child closed. It does not retry in place and does not assume: it leaves the Session alone and lets the next snapshot settle it. Absence from that snapshot is not consent to destroy — it makes the Session a candidate, and the candidate still has to clear every ordinary guard (no SSE subscriber, no registered client, nothing daemon-owned in flight) before the daemon asks the child once more. A child that already closed the Session answers `closed` for a Session it no longer has, which is how a lost close response is recovered without ever guessing.

Explicit close, kill, shutdown, and channel exit keep their force semantics and do not go through this path.

## One guard model, four triggers
## One guard model, every trigger

Four things can decide it is time to look at a Session: the last client detaching, a prompt settling, a terminal notification settling, and the idle reaper's TTL. Each brings its own policy, and none of them may weaken the shared part:
Six event families can decide it is time to look at a Session: the last client detaching, a prompt settling, a terminal notification settling, an attach registration rolling back, a full child snapshot reporting the Session idle or omitting it, and the idle reaper's TTL. Each brings its own policy, and none of them may weaken the shared part:

| Guard | Why it is shared |
| ------------------------------------------- | -------------------------------------------------------------------------------------------------------------------- |
Expand Down
2 changes: 1 addition & 1 deletion docs/design/daemon-global-deep-health.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ 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`) |
| `activeWorkReporting` | Grade once after summing coverage counts across runtimes |
| `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 |
Expand Down
44 changes: 44 additions & 0 deletions packages/acp-bridge/src/bridge.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -769,6 +769,50 @@ describe('createAcpSessionBridge', () => {
await bridge.shutdown();
});

it.each([
{
count: ACTIVE_WORK_MAX_SESSION_HOLDS,
expectedActiveWork: true,
},
{
count: ACTIVE_WORK_MAX_SESSION_HOLDS + 1,
expectedActiveWork: false,
},
])(
'bounds close-refusal adoption at $count holds',
async ({ count, expectedActiveWork }) => {
const refusalHolds = Array.from({ length: count }, (_unused, index) =>
agentHold(`h${index}`),
);
const handle = makeChannel({
initializeImpl: () => activeWorkInitializeResponse(),
extMethodImpl: async (method, params) => {
if (method !== SERVE_CONTROL_EXT_METHODS.sessionClose) return {};
if (params?.[ACTIVE_WORK_CLOSE_IF_UNHELD_PARAM] === true) {
return { closed: false, holds: refusalHolds };
}
return { closed: true };
},
});
const bridge = makeBridge({
channelFactory: async () => handle.channel,
sessionReapIntervalMs: 0,
});
const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A });
await sendActiveWorkSnapshot(handle, 1, [
{ sessionId: session.sessionId, holds: [] },
]);

await bridge.detachClient(session.sessionId, session.clientId);

expect(bridge.sessionCount).toBe(1);
expect(bridge.activeWork).toBe(expectedActiveWork);
expect(reportingGrade(bridge)).toBe('full');
Comment thread
doudouOUC marked this conversation as resolved.
Resolved

await bridge.shutdown();
},
);

it('leaves the session in place when the close request never resolves', async () => {
let closeAttempts = 0;
const handle = makeChannel({
Expand Down
10 changes: 7 additions & 3 deletions packages/acp-bridge/src/bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ import {
ACTIVE_WORK_HEARTBEAT_META_KEY,
ACTIVE_WORK_HEARTBEAT_VERSION,
ACTIVE_WORK_HOLD_CATEGORIES,
ACTIVE_WORK_MAX_SESSION_HOLDS,
ACTIVE_WORK_STALE_INTERVALS,
clampActiveWorkIntervalMs,
type ActiveWorkHeartbeatCapabilityV1,
Expand Down Expand Up @@ -3133,10 +3134,13 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge {
// The child is done with it; only local teardown remains.
return true;
}
// Refused: adopt the hold set it handed back so the cache reflects the
// reason we are backing off rather than the stale set that sent us here.
// Refused: adopt a bounded hold set so the cache reflects the reason we
// are backing off rather than the stale set that sent us here.
const holds = response['holds'];
if (Array.isArray(holds)) {
if (
Array.isArray(holds) &&
holds.length <= ACTIVE_WORK_MAX_SESSION_HOLDS
) {
Comment thread
doudouOUC marked this conversation as resolved.
Resolved
const adopted = new Map<string, ActiveWorkHoldCategory>();
for (const hold of holds) {
if (typeof hold !== 'object' || hold === null) continue;
Expand Down
9 changes: 5 additions & 4 deletions packages/acp-bridge/src/bridgeTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -281,11 +281,12 @@ export const ACTIVE_WORK_CLOSE_TIMEOUT_MS = 10_000;
export function sessionCloseDrainBudgetMs(outerWaitMs: number): number {
return Math.max(1, Math.floor(outerWaitMs * 0.8));
}
/** Bounds on a single snapshot. Generous next to any real deployment — they
* exist so a version-skewed or buggy child cannot make the daemon walk an
* unbounded structure per report, not to constrain legitimate use. A packet
* over either bound is discarded whole, like any other malformed one. */
/** Bounds on a single snapshot. Generous next to any real deployment — it
* exists so a version-skewed or buggy child cannot make the daemon walk an
* unbounded Session list per report. An oversized packet is discarded whole. */
export const ACTIVE_WORK_MAX_SNAPSHOT_SESSIONS = 1024;
/** Shared per-Session hold bound. Oversized snapshots are discarded whole;
* oversized close refusals retain the Session without replacing its cache. */
export const ACTIVE_WORK_MAX_SESSION_HOLDS = 1024;
export const WORKTREE_MCP_DEFER_META_KEY = 'qwen.session.deferMcpDiscovery';

Expand Down
Loading