Skip to content

feat(serve): add workspace runtime coordination #7288

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ytahdn wants to merge 2 commits into from
Closed

feat(serve): add workspace runtime coordination #7288

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c8a35f8
feat(serve): add workspace runtime coordination
Jul 18, 2026
baa7315
Merge branch 'main' into codex/workspace-runtime-serve
yiliang114 Jul 20, 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
25 changes: 12 additions & 13 deletions docs/design/daemon-multi-workspace-hardening.md
View file
Open in desktop
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,10 @@ 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. Every trusted workspace starts its ACP child on first runtime-backed
use and, when `mcp_workspace_pool` is enabled, owns the pool inside that child;
an untrusted workspace must not start either. Primary compatibility routing
does not bypass 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
View file
Open in desktop
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 keeps the child live; 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 keeps the Workspace Runtime live; configured
values must be positive.
- `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 keeps the channel and an explicit positive timeout arms the compatibility timer |

### 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 timeout is sticky, while an explicit positive compatibility timeout arms the timer. |
Loading
Loading