Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
c7f52e7
feat(serve): establish workspace runtime ownership
Jul 30, 2026
d3ac4c3
Merge branch 'main' into codex/workspace-runtime-foundation-v2
qwen-code-dev-bot Jul 31, 2026
59ec610
Merge branch 'main' into codex/workspace-runtime-foundation-v2
wenshao Jul 31, 2026
d9d491c
Merge branch 'main' into codex/workspace-runtime-foundation-v2
wenshao Jul 31, 2026
95216ea
Merge remote-tracking branch 'origin/main' into codex/workspace-runti…
qwen-code-ci-bot Jul 31, 2026
c4a9ccf
fix(acp-bridge): remove unreachable idle-timer branches (#8213)
qwen-code-ci-bot Jul 31, 2026
230ea2e
Merge branch 'main' into codex/workspace-runtime-foundation-v2
qwen-code-dev-bot Jul 31, 2026
d2641c4
test(cli): cover WorkspaceRuntimeStillStartingError 503 mapping (#8213)
Jul 31, 2026
57268b4
Merge branch 'main' into codex/workspace-runtime-foundation-v2
wenshao Aug 1, 2026
40771a3
Merge branch 'main' into codex/workspace-runtime-foundation-v2
wenshao Aug 1, 2026
b0d9f3d
Merge branch 'main' into codex/workspace-runtime-foundation-v2
wenshao Aug 2, 2026
7755079
chore: merge main into codex/workspace-runtime-foundation-v2
qwen-code-dev-bot Aug 2, 2026
90bbec5
Merge branch 'main' into codex/workspace-runtime-foundation-v2
wenshao Aug 2, 2026
82fee67
Merge branch 'main' into codex/workspace-runtime-foundation-v2
wenshao Aug 2, 2026
350741a
Merge branch 'main' into codex/workspace-runtime-foundation-v2
wenshao Aug 3, 2026
67d66c9
Merge branch 'main' into codex/workspace-runtime-foundation-v2
qwen-code-dev-bot Aug 3, 2026
c8c7782
fix(serve): address workspace-runtime review findings (#8213)
Aug 3, 2026
15c30e3
Merge branch 'main' into codex/workspace-runtime-foundation-v2
qwen-code-dev-bot Aug 3, 2026
c02bd06
Merge branch 'main' into codex/workspace-runtime-foundation-v2
wenshao Aug 3, 2026
8d07696
Merge branch 'main' into codex/workspace-runtime-foundation-v2
wenshao Aug 4, 2026
bc32ca1
Merge branch 'main' into codex/workspace-runtime-foundation-v2
Aug 4, 2026
d634ae6
Merge branch 'main' into codex/workspace-runtime-foundation-v2
qwen-code-dev-bot Aug 4, 2026
e34cce8
chore: merge main into workspace-runtime-foundation-v2
qwen-code-dev-bot Aug 5, 2026
1c3d08a
chore: merge main into workspace-runtime-foundation-v2
qwen-code-dev-bot Aug 12, 2026
b39737e
chore: merge main into workspace-runtime-foundation-v2
qwen-code-dev-bot Aug 12, 2026
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
26 changes: 13 additions & 13 deletions docs/design/daemon-multi-workspace-hardening.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,14 @@ fallback when resolution fails.

## Failure semantics

| State | Required behavior |
| ---------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| Unknown workspace or session | Fail closed with the route's stable mismatch/not-found response. Do not probe or execute against primary. |
| Untrusted workspace | Reject runtime-backed execution and mutation. An untrusted secondary may use only explicitly documented read-only surfaces, including bounded filesystem and persisted catalog/transcript reads, without starting ACP or writing repair state. Legacy primary preheat does not authorize requests. |
| Ambiguous live-session owner | Return a server error because dispatch cannot be made safely. Execute on no bridge. |
| Bootstrapping runtime | Keep process-global liveness responsive; runtime-backed work waits for or reports the declared startup failure. Deep health returns `503` with a reason while aggregation is unavailable. |
| Draining runtime | Refuse new work with the stable draining response. A non-forced removal rolls back with `workspace_busy` if activity exists; a forced removal requests termination and bounded cleanup of active resources. The runtime remains in daemon-global accounting until removal completes. |
| Removed runtime | Treat it as unknown. It must disappear from capabilities, routing, and health aggregation before the same workspace can be re-added. Cleanup after the persistence commit point is best-effort; failures are logged and do not restore routing. |
| State | Required behavior |
| ---------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| Unknown workspace or session | Fail closed with the route's stable mismatch/not-found response. Do not probe or execute against primary. |
| Untrusted workspace | Reject runtime-backed execution and mutation. An untrusted secondary may use only explicitly documented read-only surfaces, including bounded filesystem and persisted catalog/transcript reads, without starting ACP or writing repair state. Primary compatibility routing does not authorize requests. |
| Ambiguous live-session owner | Return a server error because dispatch cannot be made safely. Execute on no bridge. |
| Bootstrapping runtime | Keep process-global liveness responsive; runtime-backed work waits for or reports the declared startup failure. Deep health returns `503` with a reason while aggregation is unavailable. |
| Draining runtime | Refuse new work with the stable draining response. A non-forced removal rolls back with `workspace_busy` if activity exists; a forced removal requests termination and bounded cleanup of active resources. The runtime remains in daemon-global accounting until removal completes. |
| Removed runtime | Treat it as unknown. It must disappear from capabilities, routing, and health aggregation before the same workspace can be re-added. Cleanup after the persistence commit point is best-effort; failures are logged and do not restore routing. |

## Invariants

Expand All @@ -40,11 +40,11 @@ fallback when resolution fails.
be absolute and canonicalize to a registered runtime.
- Each active workspace runtime owns its environment snapshot, bridge, workspace
services, filesystem/trust boundary, Voice state, and ACP/MCP resource
boundary. Production attempts to preheat the primary bridge for compatibility
and retries on first use after a preheat failure. A trusted secondary starts
its ACP child on demand and, when `mcp_workspace_pool` is enabled, owns the
pool inside that child; an untrusted secondary must not start either. Primary
preheat does not bypass route trust gates. A
boundary. Production may preheat the trusted primary child for compatibility;
trusted secondaries start on first runtime-backed use. When
`mcp_workspace_pool` is enabled, each started child owns its pool; an untrusted
workspace must not start either. Primary compatibility routing does not bypass

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] This diff rewrote the correctly-scoped invariant "an untrusted secondary must not start either" into the over-general "an untrusted workspace must not start either", but an untrusted primary still starts its ACP child via the legacy singular session routes. This document is the self-declared review-contract baseline for the hardening work. — Failure scenario: a future reviewer wielding this invariant will flag the legitimate untrusted-primary session path as a hardening violation. Suggested fix: restore the secondary scoping, e.g. "…each started child owns its pool; an untrusted secondary must not start either (an untrusted primary starts only through the legacy singular Session compatibility surface). Primary compatibility routing does not bypass route trust gates."

中文说明

本 diff 把范围正确的不变量 "an untrusted secondary must not start either" 改写成了过度泛化的 "an untrusted workspace must not start either",但不受信任的 primary 仍会通过 legacy singular session 路由启动其 ACP 子进程。该文档是硬化工作自述的评审契约基线。— 失败场景:未来拿着这条不变量评审的人会把合法的"不受信任 primary session 路径"标记为硬化违规。建议恢复 secondary 范围限定,例如:"…each started child owns its pool; an untrusted secondary must not start either (an untrusted primary starts only through the legacy singular Session compatibility surface). Primary compatibility routing does not bypass route trust gates."

— qwen3.8-max-preview via Qwen Code /review (v0.21.6)

route trust gates. A
process-global Voice coordinator enforces the shared admission cap while
tracking leases by owning runtime. Same-named environment keys must not cross
runtimes, and a workspace overlay must not mutate the parent process
Expand Down
34 changes: 18 additions & 16 deletions docs/design/session-idle-reaper/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ Bridge closure (createHttpAcpBridge)

| Mechanism | Scope | What it manages |
| ----------------------------------------- | ------------------------- | -------------------------------------------------------------------------------- |
| `channelIdleTimeoutMs` + `startIdleTimer` | Channel (child process) | Kills the `qwen --acp` child when ALL sessions are gone |
| `channelIdleTimeoutMs` + `startIdleTimer` | Channel (child process) | Unset or `0` reaps immediately; a positive value delays reap |
| **Session reaper** (this design) | Session (in-memory entry) | Closes individual sessions when idle |
| `ConnectionRegistry` sweep | ACP-over-HTTP connection | Reaps `/acp` transport-layer connections (different layer) |
| `writerIdleTimeoutMs` | SSE subscriber | Evicts a single stuck SSE subscriber |
Expand Down Expand Up @@ -309,6 +309,8 @@ function.

- `startSessionReaper()` is called at bridge construction time (after
option validation, alongside the existing `channelIdleTimeoutMs` setup).
Omitting that channel option or setting it to `0` reaps an idle Workspace
Runtime immediately; configured values must be non-negative.
Comment on lines +312 to +313

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The sentence added by this PR — "Omitting that channel option or setting it to 0 reaps an idle Workspace Runtime immediately" — has a live counterexample: a preheated channel that never serves a session is never reaped. startBridgePreheat calls bridge.preheat() with no keepAliveMs; at resolved timeout 0 the preheat finally settles with armIdleTimer = false (otherwise preheat would kill its own freshly spawned child), and nothing else reaps a session-less channel — so the daemon sits with a zero-session runtime the snapshot reports as idle, indefinitely. The behavior is deliberate and test-pinned ('preserves the preheated channel with an omitted/explicit zero timeout'), so this is a doc-overgeneralization. The same claim recurs at docs/developers/daemon/03-acp-bridge.md:180 (defaultEntry bullet), docs/developers/daemon/20-quickstart-operations.md (--channel-idle-timeout-ms row), packages/cli/src/commands/serve.ts:388 (flag help text), and the bridgeOptions.ts docstring. — Failure scenario: an operator trusting the sentence expects bounded idle resource use under the default config and files the retained warm child as a leak; inversion — setting the flag to 30000 reaps that same session-less child after 30 s, but the default 0 never does.

Suggested change
Omitting that channel option or setting it to `0` reaps an idle Workspace
Runtime immediately; configured values must be non-negative.
Omitting that channel option or setting it to `0` reaps an idle Workspace
Runtime immediately once its last session closes; a preheated channel that has
not served any session is held warm for first use. Configured values must be
non-negative.
中文说明

本 PR 新增的这句话——"Omitting that channel option or setting it to 0 reaps an idle Workspace Runtime immediately"——存在一个现实反例:从未服务过任何 session 的 preheat channel 永远不会被回收。startBridgePreheat 调用 bridge.preheat() 时不带 keepAliveMs;在解析超时为 0 时,preheat 的 finally 以 armIdleTimer = false 结算(否则 preheat 会杀掉自己刚拉起的子进程),而没有其他路径会回收无 session 的 channel——于是 daemon 会一直挂着一个零 session、快照报告为 idle 的 runtime。该行为是有意为之且被测试钉住('preserves the preheated channel with an omitted/explicit zero timeout'),因此这是文档的过度泛化。同样的表述还出现在 docs/developers/daemon/03-acp-bridge.md:180(defaultEntry 条目)、docs/developers/daemon/20-quickstart-operations.md--channel-idle-timeout-ms 行)、packages/cli/src/commands/serve.ts:388(flag 帮助文本)以及 bridgeOptions.ts 的 docstring。— 失败场景:信任这句话的操作员在默认配置下期望空闲资源有界,会把被保留的温热子进程当作泄漏上报;且存在反转——把 flag 设为 30000 会在 30 秒后回收同一个无 session 子进程,而默认值 0 反而永远不会。

— qwen3.8-max-preview via Qwen Code /review (v0.21.6)

- `stopSessionReaper()` is called in both `shutdown()` and `killAllSync()`.

### 4.6 Interaction with existing `closeSession` callers
Expand Down Expand Up @@ -367,20 +369,20 @@ generic terminal-frame handler (`isTerminalLifecycleEvent`) already handles

### 5.1 Unit tests (`bridge.test.ts`)

| # | Test | Description |
| --- | ------------------------------------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| 1 | Idle session is reaped after timeout | Create a session, advance time past `sessionIdleTimeoutMs`, trigger reaper tick, verify session removed from `byId` and `session_closed` event published with `reason: 'idle_timeout'` |
| 2 | Session with active prompt is NOT reaped | Create a session, start a prompt, advance time, verify session survives reaper tick |
| 3 | Session with live SSE subscriber is NOT reaped | Create a session, subscribe to its EventBus, advance time, verify session survives |
| 4 | Session with registered client is NOT reaped | Create a session, register a clientId, advance time, verify session survives |
| 5 | Reaper disabled when interval = 0 | Pass `sessionReapIntervalMs: 0`, verify no `setInterval` is armed |
| 6 | Reaper disabled when timeout = 0 | Pass `sessionIdleTimeoutMs: 0`, verify no `setInterval` is armed |
| 7 | Reaper stopped on shutdown | Call `shutdown()`, verify `clearInterval` was called |
| 8 | closeSession reason defaults to 'client_close' | Call `closeSession` without explicit reason, verify published event has `reason: 'client_close'` |
| 9 | closeSession with explicit reason | Call `closeSession` with `reason: 'idle_timeout'`, verify published event |
| 10 | Multiple idle sessions reaped in one tick | Create 3 idle sessions, advance time, trigger tick, verify all 3 reaped |
| 11 | Session with heartbeat within TTL survives | Create a session, record heartbeat, advance time to just under TTL, verify session survives |
| 12 | Channel idle timer triggered after last session reaped | Create 1 session (last on channel), reap it, verify `startIdleTimer` is called on the channel |
| # | Test | Description |
| --- | ------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| 1 | Idle session is reaped after timeout | Create a session, advance time past `sessionIdleTimeoutMs`, trigger reaper tick, verify session removed from `byId` and `session_closed` event published with `reason: 'idle_timeout'` |
| 2 | Session with active prompt is NOT reaped | Create a session, start a prompt, advance time, verify session survives reaper tick |
| 3 | Session with live SSE subscriber is NOT reaped | Create a session, subscribe to its EventBus, advance time, verify session survives |
| 4 | Session with registered client is NOT reaped | Create a session, register a clientId, advance time, verify session survives |
| 5 | Reaper disabled when interval = 0 | Pass `sessionReapIntervalMs: 0`, verify no `setInterval` is armed |
| 6 | Reaper disabled when timeout = 0 | Pass `sessionIdleTimeoutMs: 0`, verify no `setInterval` is armed |
| 7 | Reaper stopped on shutdown | Call `shutdown()`, verify `clearInterval` was called |
| 8 | closeSession reason defaults to 'client_close' | Call `closeSession` without explicit reason, verify published event has `reason: 'client_close'` |
| 9 | closeSession with explicit reason | Call `closeSession` with `reason: 'idle_timeout'`, verify published event |
| 10 | Multiple idle sessions reaped in one tick | Create 3 idle sessions, advance time, trigger tick, verify all 3 reaped |
| 11 | Session with heartbeat within TTL survives | Create a session, record heartbeat, advance time to just under TTL, verify session survives |
| 12 | Channel idle policy evaluated after last session reaped | Create 1 session (last on channel), reap it, verify an unset timeout reaps the channel and an explicit positive timeout arms the compatibility timer |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The rewritten test-plan row 12 claims test 12 verifies both idle-policy branches, but the actual test (triggers channel idle timer after reaping the last session, bridge.test.ts:23523, untouched by this PR) configures only channelIdleTimeoutMs: 2_000 — the reaper-triggered unset-timeout combination the row names is untested. (Correction vs the filed finding: the immediate-kill branch itself IS asserted elsewhere by kills the channel immediately when timeout is 0 (default); what no test exercises is the reaper-triggered last-session scenario at unset timeout that this row describes.) — Failure scenario: a regression of the reaper → closeSessionstartIdleTimer path at unset timeout keeps every test green while an idle workspace runtime's qwen --acp child is never reaped (process/FD leak until daemon restart) — and this row discourages adding the missing test because the design doc claims coverage exists. — Concrete cost: extend test 12 (or add a sibling) that builds the bridge without channelIdleTimeoutMs, reaps the last session via the reaper, and asserts the channel is killed immediately; or weaken the row to describe only the positive-timeout assertion the test actually makes.

中文说明

重写后的测试计划第 12 行声称测试 12 同时验证了空闲策略的两个分支,但实际测试(triggers channel idle timer after reaping the last session,bridge.test.ts:23523,本 PR 未改动)只配置了 channelIdleTimeoutMs: 2_000——该行所描述的「由 reaper 触发、未设置超时」的组合并没有测试。(相对原始发现的更正:立即 kill 分支本身确实由 kills the channel immediately when timeout is 0 (default) 在别处断言;没有任何测试覆盖的是该行描述的、未设置超时下由 reaper 触发最后一个 session 回收的场景。)— 失败场景:reaper → closeSessionstartIdleTimer 路径在未设置超时时的回归不会让任何测试变红,而空闲 workspace runtime 的 qwen --acp 子进程将永不被回收(进程/FD 泄漏直至 daemon 重启)——并且由于设计文档声称覆盖已存在,这一行还会阻碍补上缺失的测试。— 具体代价:扩展测试 12(或新增一个兄弟测试),在不设置 channelIdleTimeoutMs 的情况下构造 bridge、经 reaper 回收最后一个 session,并断言 channel 被立即杀掉;或者把该行弱化为只描述测试实际断言的正超时行为。

— qwen3.8-max via Qwen Code /review (v0.21.10)


### 5.2 Integration tests (`server.test.ts`)

Expand Down Expand Up @@ -431,4 +433,4 @@ generic terminal-frame handler (`isTerminalLifecycleEvent`) already handles
| `closeSession` inside reaper throws, poisoning the scan loop | Each close is in its own `.catch()` — one failure doesn't block others |
| Reaper iteration over `byId` during concurrent `closeSession` from another path | ES2015 Map iteration tolerates deletion of current/previous keys. Double-close is idempotent (`byId.get` returns undefined → `SessionNotFoundError` caught by reaper's `.catch`). |
| Performance of scanning 20 sessions every 60s | Trivial — 20 Map reads + 4 field checks each. No I/O. |
| Channel idle timer interaction | When the last session is reaped, `closeSession` already calls `startIdleTimer` on the channel. No additional logic needed. |
| Channel idle timer interaction | When the last session is reaped, `closeSession` calls `startIdleTimer`; an unset or zero timeout reaps immediately, while an explicit positive timeout arms the timer. |
Loading
Loading