fix(cli): let /session detach from a running Turn instead of trapping the client

[me2seeks]

Closes #3380

Problem

A second TUI that switched onto a Session with an in-flight Turn (via /session or the picker) was trapped: /session was intercepted with "Cannot run /session while a turn is running", and every apparent way out was destructive — following the hint (Esc/Ctrl+C) or quitting called driver.stop(), aborting the shared Turn another client was watching. The only clean escape was kill -9.

Multi-client attach is a designed Runtime Host capability: the Turn is Host-owned and each TUI is only its viewport. Desktop already treats session switching as pure view navigation; this fix brings the CLI to the same model.

Fix: let /session detach from a running Turn instead of touching it

• New 'switch' mid-turn slash disposition (alongside 'local' from refactor(cli): move the mid-turn slash disposition onto the command spec #3379). Routed through like 'local', but its handler must use busy-aware wrappers: idle it runs under runControl's serial lock as before; mid-turn the switch goes through the new detach path instead of silently no-oping on runControl's busy gate.
• switchAwayMidTurn never calls driver.stop(). After driver.switchSession confirms, a monotonic turnEpoch fence orphans the in-flight drain:
  • late events and synthesized stream failures ("ended without a completion event") from the abandoned Session can no longer reach the adopted transcript;
  • the orphaned runAgentTurn tail skips all old-session continuations (queue flushes would steer the NEW Session), releases busy/activity, and starts the freshly attached Turn exactly once — whichever of the detach path or the orphan tail observes an idle runner first.
• Interrupt guard during handoff: requestTurnInterrupt is swallowed while a detach is in flight — the driver already points at the next Session, so a stop there would abort whatever that Session has attached.
• Escape owns the picker: while the mid-turn session picker is open, Escape closes it instead of arming the double-Escape interrupt for the Turn being left running.
• Foreign import rows hidden mid-turn: the Claude Code/Codex import flow starts a new Session and cannot detach.

Scope notes

• Interrupt semantics are unchanged by design (discussed in fix(cli): a second TUI attached to a Session with a running Turn is trapped — every exit path aborts the Turn #3380): any attached client may stop a Turn.
• Switching away from a self-started turn keeps working through the same detach path; host-driven Goal continuation on the abandoned Session is unaffected (the Host keeps running it).
• A failed switch leaves everything fully live — the epoch fence is only armed after the driver confirms the switch (covered by test).

Testing

• 3 new tests in pi-tui-runner.test.ts with a parking-turn driver:
  • mid-turn /session <id>: switches with zero stop() calls, replaces the transcript, fences leaked events, starts the attached Turn after the orphan unwinds, lands follow-ups on the adopted Session;
  • /session opens the picker mid-turn and Escape closes it without arming an interrupt;
  • a failed switch reports the error and leaves the running Turn streaming normally.
• Full CLI suite: 360/360 pass; biome + typecheck clean.

AI use

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

Tool(s) and scope: Maka (AI coding agent) authored the implementation and tests; the diff was human-reviewed before push. Generated-by: Maka trailers are present on the branch commits.

[me2seeks]

Working on this PR — happy to iterate on review feedback. Local verification: CLI suite 360/360, biome + typecheck clean.

[Astro-Han]

Thanks — this is a well-argued fix and the framing is right: in Runtime Host mode the Turn belongs to the Host and this TUI is only a viewport, so refusing /session mid-turn was trapping the user in a view rather than protecting anything. The monotonic turnEpoch fence is the correct shape for it — one counter makes every callback of the abandoned drain a no-op, instead of trying to unsubscribe them individually. Fencing only after switchSession resolves, so a failed switch leaves the drain fully live, is a detail that is easy to get backwards and you got it right.

Reviewed at exact head 24a3a6711870a427bd82f2b8f03b86b4f52b37c4 against base 8b60ddffa89682c03238ddce6595f531eb2b6f29. One P2 inline, plus two P3 verification gaps below. No checks have run on this head yet.

P3 — switching to the session you are already on. goToSession has no same-session short-circuit, so /session <current-id> mid-turn bumps the epoch, orphans the live drain, and prints Detached from the running Turn while the user is still on the same session. Whether this is harmless depends on what driver.switchSession does with its own session id, which we could not settle from the client side. A test pinning the intended behaviour would close it either way.

P3 — re-attaching to the same still-running Turn. The new tests cover attaching to a different session's live turn. Detaching and immediately re-attaching to the same one is the case where a double consumer would show up if the orphan tail and the new attach ever overlapped; the startPendingAttachedTurn no-op guards look like they prevent it, but nothing pins it.

Neither P3 blocks. The P2 does, and it is small to fix.

---

This review was AI-assisted. Findings were verified against the exact head listed above; any mistakes are ours to correct — please push back where we got it wrong.

[Astro-Han]

[P2] switchAwayMidTurn is re-entrant, and a second detach clears detaching while the first is still in flight.

Mid-turn, both /session entry points deliberately skip runControl's serial lock — that is the point of the change, since the lock is held by the running Turn. Both are also invoked fire-and-forget: void goToSession(sessionId) and void openSessionPicker(). So nothing serializes switchAwayMidTurn against itself, and it has an await boundary in the middle of it (await input.driver.switchSession(sessionId)).

Sequence: the user types /session s2. While that request is in flight they type /session s3 — normal impatience, and the window is as long as the round trip. Now two switchAwayMidTurn calls are live.

The consequence we care about is the finally block. detaching is a boolean, so when the first call settles it runs detaching = false while the second is still awaiting its switch. That reopens requestTurnInterrupt, whose new guard exists precisely for this window — your own comment there says "a stop here would abort whatever that Session has attached." A Ctrl+C landing in that reopened window calls driver.stop() against the freshly adopted session, which is the exact outcome this PR promises cannot happen. The invariant is stated correctly in the code and then broken by re-entry.

Two smaller effects ride along: applySwitchResult runs twice in whatever order the responses return, so the user can end up on s2 after asking for s3; and two Detached from the running Turn notices land in the adopted transcript.

The mechanism to fix it already exists — detaching is shaped like a re-entrancy flag, it just is not read at the entry points. Rejecting the second detach is enough:

const goToSession = async (sessionId: string): Promise<void> => {
  if (!turnRunning) { await runControl(() => switchSession(sessionId)); return; }
  if (detaching) return;               // a detach is already handing this view over
  await switchAwayMidTurn(sessionId).catch(reportError);
};

with the same guard on openSessionPicker. Turning detaching into a counter would also work but is more machinery than the situation needs — a second detach during a handoff has no meaningful semantics, so dropping it is the honest behaviour.

A regression test would issue a second /session before the first switchSession promise resolves and assert that driver.stop() is still never called and that exactly one Detached notice appears. The existing failNextSwitch test already shows the harness can control that promise's timing.

[me2seeks]

Confirmed both mechanisms locally before fixing (reproduced the double switchSession and the early detaching clear with a parked first switch), fixed at 332404759:

• goToSession now rejects while detaching is held. The picker path needed no separate guard — its selection routes through goToSession (:2179), so one guard covers both entry points.
• Pinned by a test: second mid-turn /session during a parked switch yields exactly one detach notice, no extra stop(), single adoption.

One nuance worth flagging: after the blocked re-entry, the queued command text can replay through the idle path once the adopted turn settles, producing a benign second switchSession to the same id — that is pre-existing queue behavior outside this window, not a reopen of it.

[Astro-Han]

[P2] This comment says every callback below stops touching shared runner state once the epoch is bumped, but two of them do not check.

superseded() is consulted at :1226 (onEvent), :1263 and :1278. It is not consulted in:

• onPrepared (:1197-1210) — the non-attached branch ends in if (turn.summary) adoptSessionMetadata(turn.summary), which writes title/cwd/model on the runner
• onSkillInvocation (:1211-1222) — splices state.entries and calls showSkillInvocation

Both are reachable after a switch-away, because turnRunning is set at the top of runAgentTurn while pi-tui-turn.ts:87-88 only invokes these two after await preparePrompt resolves. So the whole preparePrompt window is a period where /session can already enter switchAwayMidTurn, and neither callback has returned yet.

Concrete sequence:

1. a turn stalls in preparePrompt (slow or hung)
2. /session <other> — switchSession succeeds, turnEpoch bumps, applySwitchResult replaces the transcript with the adopted session
3. preparePrompt finally resolves
4. onPrepared runs with the abandoned turn's summary and calls adoptSessionMetadata on it

The screen now describes the old session while the driver is on the new one, and the next prompt goes to the new one. The skill callback has the same shape: an overlay belonging to the abandoned session appearing over the adopted viewport.

shouldAbort doesn't help — it only reads closed, not the epoch.

Fix looks like the same one-liner already used in the three fenced callbacks: if (superseded()) return; at the top of both, including the onSkillInvocation on the SkillInvocationBlockedError path. A test that switches while preparePrompt is still unresolved, then asserts the metadata still belongs to the adopted session, would pin it.

This is separate from the re-entrant detaching P2 already reported inline on this head — that one is about entering the path twice, this one is about callbacks that were already in flight when it was entered once.

[me2seeks]

Confirmed before fixing: reproduced an abandoned Turn adopting its summary over the adopted session when preparePrompt resolved after a completed switch-away. Fixed at 332404759 — onPrepared and onSkillInvocation now open with the same superseded() fence as the other callbacks, which also covers the SkillInvocationBlockedError invocation since both call sites share the callback. New test parks preparePrompt, detaches mid-park, then resolves: the abandoned title/cwd never reach the runner metadata and the skill card cannot land on the adopted viewport. CLI suite 362/362, biome clean.

[Astro-Han]

Second review pass on 24a3a6711870a427bd82f2b8f03b86b4f52b37c4, from a different model and a different route than the earlier one on this head. One new [P2], left inline at the epoch fence: #discussion_r3837660462.

The mechanism itself holds up, and we checked the thing that usually goes wrong with an escape path — what happens to the thing you escaped from:

• The Host Turn is not stopped. switchAwayMidTurn never calls driver.stop() (verified: zero stop calls in the covering tests), which is the correct asymmetry against the interrupt path. In Runtime Host mode the Turn is Host-owned and this TUI was only its viewport, so detaching the view should not end the work.
• A failed switch does not orphan the live drain. turnEpoch += 1 happens only after await switchSession resolves, so a throw leaves the in-flight drain fully live — pinned by a test asserting late deltas still land in the same transcript.
• The adopted Turn cannot start twice. startPendingAttachedTurn no-ops while busy || turnRunning, and the old drain's finishTurnUi clears turnRunning before releasing; whichever of the two observes an idle runner first starts it.
• Escape on the picker doesn't interrupt. Early return while sessionPickerOverlayOpen, with a test asserting no stop call.

On structure: one epoch counter, not a second authority over Turn lifetime. That's the right shape for this.

Two things noted without grading, since neither is this PR's job to fix:

• Removing /transcript and the TranscriptViewerOverlay reference is a behaviour deletion unrelated to detach. The viewer module file itself isn't in this diff, so it will be left behind — worth a follow-up so it doesn't linger as dead code.
• /session <current id> still detaches from itself; goToSession has no short-circuit. Already reported previously, not re-graded here.

The re-entrant detaching [P2] already on this head stands on its own; we looked at it independently and agree, and are deliberately not re-grading it. The finding above is a different failure: not entering the path twice, but callbacks that were already in flight when it was entered once.

On CI: the run on this head was sitting at action_required and had never executed — I've approved it, so there should be a real signal shortly. Not approving while the P2 above and the existing one are open.

[Astro-Han]

Approving 3324047598651d6b725bdb579cb66c3b0eeb99cc. Required test is completed / success bound to that exact SHA. No P0–P2.

Re-review at the current head. Both earlier findings were re-derived from the code rather than taken as fixed.

Both [P2]s are closed.

The re-entrant detach is guarded at the entry point, and — importantly — the guard is not cleared early: if (detaching) return; at pi-tui-runner.ts:1625 sits behind a comment explaining that a second detach arriving while the first is still handing the view over would otherwise reset the flag and reopen the path. That is the actual failure mode, closed at the right place.

The epoch fence is now complete. superseded() is consulted at five call sites (:1201, :1219, :1234, :1271, :1286) rather than three, so the two callbacks that previously kept touching shared runner state after the epoch bump no longer do. The comment claiming every callback below the fence checks it is now true — before, the comment described an intent the code did not implement, which is the more dangerous of the two states.

The production increment is +12 lines. It fixes exactly these two holes and introduces nothing else.

Residual, non-blocking: the same-session detach [P3] stands; detaching and immediately reattaching to the same running Turn produces a two-consumer situation that no test currently pins; and the /transcript dead-module follow-up is unchanged.

Stated rather than implied: the local CLI suite and a real multi-client Host were not exercised here — the evidence is the hosted test on this exact head plus code-level verification of both fixes.

Disclosure, because it changes what this approval is worth: this is an AI review. Under CONTRIBUTING.md §Review it does not count as the required independent human review — merge still needs a committer other than the author to give LGTM and to decide.