fix(runtime-host): detach sandbox graph wake reconciliation

[testikun]

Summary

Refs #3328

A long Agent Graph run could freeze a Desktop Session after the user answered a request_sandbox_boundary prompt. The durable answer path held Session admission while awaiting graph-wake reconciliation. A wake-started turn can still be parked on that prompt while holding the graph activity lease; reconciliation then waits for the same lease, so the answer and every later Session operation queue forever.

This is the first durable fault behind the long-task freeze described in #3328 and the related reproduction in #3866. The reported React error #185 is a separate historical 0.1.11 inline-completion crash: that seam was already removed and fenced by #3292/#3755/#4208, and current main does not reintroduce it.

The fix detaches reconciliation after the durable answer is applied and routes detached failures through the existing fail-stop path. It does not change provider retry policy, sandbox authority rules, or local-model support.

Verification

• Direct Node 24 TypeScript compilation of @maka/runtime-host succeeded.
• Biome check and git diff --check passed.
• Targeted Runtime/Runtime Host tests passed: 62 tests covering Agent Graph wake delivery, ToolRuntime sandbox-boundary lifecycle, RuntimeKernel interaction cleanup, Host interaction coordination, and graph-wake lineage.
• Focused Host/graph run passed: 17 tests.
• The regression fails on the old awaited callback and passes with the detached callback.

The repository npm scripts could not be used with the system Node 20 in this environment because the current tree requires stripTypeScriptTypes and node:sqlite; the same checks were run with the bundled Node 24 runtime.

AI use

• No generative tool made a substantive contribution
• Generative tooling made a substantive contribution

Tool(s) and scope: OpenAI Codex analyzed the issue and related reproductions, edited the Runtime Host fix and regression test, and ran verification. A human contributor remains responsible for review, accuracy, and submission.

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

• Yes — the Session answer no longer waits on graph-wake reconciliation
• No

[jackwener]

Approved at exact head b659ff4e0ae2d21397c17d630db6950154b62c2e. I found no blocking or non-blocking issues.

The previous path held Session admission while notifyPermissionResponse() waited for the activity lease owned by the wake-started turn that was itself parked on the answered sandbox boundary. The new detached callback lets the durable answer and captured continuation complete, releases Session admission, and still routes callback failures into the Host fail-stop path.

I rebuilt the affected Runtime and Runtime Host packages and passed 64 interaction, sandbox-boundary, graph-wake, and restart-recovery tests. Restoring the previous awaited callback made the new regression fail deterministically because the interaction answer remained unsettled. A synthetic merge with current main was clean; that merge result also built and passed the same 64 tests. The exact-head hosted test check is complete and successful.

---

Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.

[Astro-Han]

The deadlock is real and I traced the full cycle rather than taking it from the description: #answer holds the session admission lease for the whole of #answerSandboxBoundary (interaction-coordinator.ts:735), SessionAdmissionGate serializes per session, and the awaited reconciliation reaches #settlePermissionResponse, whose first act is activityRegistry.acquire(rootSessionId) (agent-graph-supervisor-wake.ts:721), which waits for the session to go idle and is non-reentrant. The parked turn cannot reach a terminal state without re-entering admission, and admission is held by the answer. Closed loop.

Detaching is the right shape, and worth saying why: notifyPermissionResponse is already a fire-and-forget contract, it returns Promise<void> | undefined, tracks its own work in #runTracked and converts its own errors through onError. Awaiting it on the answer path was the anomaly. So this restores the intended shape rather than routing around something deeper, and it adds no new state or authority.

On whether this PR is needed: the fault is provable in code, but #3328 does not establish that users hit it. That report is a vague "long tasks keep erroring" on 0.1.11 with one request_sandbox_boundary sitting at 4m54s, and no lease-contention evidence. The PR body is honest about calling it the first durable fault behind that issue rather than the confirmed cause. I would still fix it, a provable deadlock on a normal path does not need a witness. But Fixes #3328 would be overclaiming.

One thing to fix before merge.

[P2] The detach also moved two throwing reads out of the lease, and they land on #poison

The new call is

void Promise.resolve()
  .then(() => this.#onSandboxBoundarySettled(request.sessionId))
  .catch((error: unknown) => { this.#poison(error); });

but #onSandboxBoundarySettled is notifySandboxBoundaryGraphWake (execution-composition.ts:673), which does two reads before it notifies, and both can throw:

sandbox-boundary-graph-wake.ts:49 calls readHeaderSnapshot, which throws SessionNotFoundError once the session is gone, with no isSessionNotFoundError guard here (compare execution-composition.ts:1284, which has one). And :33-38 throws Graph operator Session ... does not match root Session ... when listGraphIds falls back to agentGraphIdForRootSession after the root's epoch rows are cleared.

Before this change both reads sat inside the answer's lease, and retirement runs through this.#admission.runMany(...) (session-retirement-coordinator.ts:441), so they could not interleave. After it, they can.

Where it lands matters: .catch goes to #poison, and composition's onPoison is retainUntilProcessExit(); beginDrain(); requestDrain();. So a graph subagent answering a boundary and then having its root retired shortly after takes the whole Host's interaction authority into fail-stop. The window is narrow and wakes are durable so recover() converges on restart, which is why this is P2 and not P1, but the failure mode is Host-wide.

The smaller fix is also the smaller diff: resolve the lineage inside the lease and detach only notifyPermissionResponse(rootSessionId), by splitting onSandboxBoundarySettled into resolve and notify. That keeps the old serialization and the new detach at once.

Two smaller notes. A client receiving answered no longer implies the reconciliation ran in this process: notifyPermissionResponse returns early when #closed, and close() does not track this detached task, so a drain between the two leaves the wake parked until the next recover(). Worth a line in the behavior-change section, which currently says only that it no longer waits. And the new .catch → #poison branch has no test; the fake onSandboxBoundarySettled never rejects, so the escalation path above has no regression cover.

Next step

Fix the lineage-read scope, then this is good to go. The 88-line test does go through the real interaction.answer handler and fails deterministically without the change, which is the right shape.

Worth flagging on process: the only approval here is from an automated review agent whose own comment says it is not an independent human review. So this has not had a human look at it yet, and if it merges as-is someone should make an explicit call on the P2 path.

Manual acceptance: a real long Agent Graph run in Desktop, park on a request_sandbox_boundary prompt, answer it, confirm the Session is not frozen and subsequent Session operations respond immediately. Then answer a boundary on a graph operator and delete the root session right after, confirming the Host does not enter fail-stop.

Evidence boundary: static read at b659ff4e0, no build, no tests, and I did not reproduce either the deadlock or the P2 race. How wide the retirement window actually is comes from reading admission usage in session-retirement-coordinator.ts, not from walking every precondition of a removal plan, so it may be narrower than I describe, or wider if there is an automatic removal trigger I did not find.

AI-assisted review: drafted with Maka.

[Astro-Han]

Re-reviewed at 7c858bf. The P2 is fixed the way I asked: lineage resolves inside the admission lease (interaction-coordinator.ts:900-902), only the root notification is detached (:903-910), SessionNotFoundError is treated as already retired (execution-composition.ts:682), and other resolver failures land where they did before the PR. git merge-tree against current main is clean; the 39 intervening commits touch execution-composition.ts only below this hunk. Refs #3328 is right.

Three things before I would call it clean, all small:

• P2: the fix has no regression line. The new test at interaction-coordinator.test.ts:391 asserts the resolver received the right session id, and the fixture ignores the admission argument, so moving the resolver call back into the detached .then (the shape of b659ff4e) keeps both new tests green. One concurrent interaction.query on the same session inside the fake resolver, asserting it cannot proceed until the resolver returns, pins it.
• P3: notifySandboxBoundaryGraphWake is now dead in production. execution-composition.ts:160 imports only resolveSandboxBoundaryGraphWake; the only caller left is its own test. Delete sandbox-boundary-graph-wake.ts:53-61 and point that test at the resolver.
• P3: resolveSandboxBoundaryGraphWake? is optional for a single constructor that always passes it, which leaves :905's guard and :906's ?? request.sessionId as production-dead branches and gives onSandboxBoundarySettled two meanings (settled id vs root id) under one name. Make it required and rename the parameter.

Also worth one sentence in the body: answered no longer implies reconciliation ran in this process; close() does not wait for the detached hop, so a drain in that microtask leaves the wake parked until recover(). Narrow and self-healing, I would document rather than add tracking.

Evidence boundary: static read, no build, no tests. The approval on file is from an automated agent at the previous head.

AI-assisted review: drafted with Maka; I verified the lease scope, the dead function and the merge-tree myself.

[testikun]

Review follow-up: the detached graph-wake close race is addressed in the current head and covered by the regression test. The remaining observations concern API semantics/retirement policy rather than a correctness defect; they are documented in the reviewer reply for maintainer direction.

[Astro-Han]

Reviewed current head 3a939fb3f0c362c241b4dda0cb570ed7222122c3 (OPEN). Technical GO — no P0–P2, three P3s below. The deadlock being fixed is real and the cut is correct.

P3 — close does not wait for detached notifications; a wake can be lost

close() (interaction-coordinator.ts:437-450) checks #runs, #live, pending interactions and pending sandbox boundaries — but nothing tracks notifications already detached and still in flight. Worse, the boundary was already deleted by #applySandboxBoundaryDecisionAndDelete before detach, so #readAllPendingSandboxBoundaries() necessarily misses it. Net: user answers, Host closes immediately after, the notification may never arrive — the blocked graph turn stalls, the very symptom this PR fixes in a narrower window. Graded P3 not P2 on the premise that shutdown is stopping anyway and a post-restart recovery flow likely picks it up — but that recovery flow was not independently verified; if it does not exist, this should be upgraded. Suggest tracking in-flight notifications in a set and settling them in close().

P3 — the answer returns success while the failure surfaces later, unattributed

Notification failures previously propagated back to the answering caller via await. Now the answer returns answered normally while the failure takes effect later through #poison(error) (:907), making subsequent unrelated operations start throwing. Decoupling success from its failure complicates attribution; worth documenting the new contract.

P3 — missing session silently swallowed

execution-composition.ts: isSessionNotFoundError → return undefined, treated as "no root session", no notice, no log. Reasonable in the close race, but it also covers genuinely inconsistent lineage data — indistinguishable here. One debug log would separate the two.

What I could not judge

No tests/build/Host run — conclusions from reading this head's source. The claimed regression behavior (fails on old await, passes on detach) was read, not reproduced.

---

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

简体中文

评审结论来自自动化审查流程；发布者没有读这份 diff，核的是当前 head 有没有漂移。当前 head 是 3a939fb，未关闭。修的死锁是真的，三条 P3：关闭不等在途通知、成功与故障对不上号、缺 session 静默吞。等人类拍板。

[testikun]

@Astro-Han Follow-up to your current P3 review.

The close race is fixed in commit 3151c716c: detached graph-wake notifications are tracked and close() awaits them before final pending-state validation. The interaction-coordinator regression test holds the detached wake open and proves close does not complete early. The branch was rebased onto current apache/main and pushed to codex/issue-3328-delegated.

I am leaving the other two P3s unresolved without code changes. The detached notification already reports failures through the existing coordinator fail-stop path (onPoison), while awaiting it in the answer path recreates the Session-admission deadlock this PR fixes; changing the answer result contract is a product/API decision rather than a correctness fix. Missing-session handling is intentionally idempotent for the close/retirement race: the resolver returns no wake when the session has already been retired, while non-session lineage errors still propagate to fail-stop. Adding a routine debug log for the expected race would change operational noise without distinguishing a user-actionable failure.