From 80910e42bca3f19f781ae46773ded1d7e9313f61 Mon Sep 17 00:00:00 2001 From: me2seeks Date: Sat, 22 Aug 2026 21:10:38 +0800 Subject: [PATCH 1/3] fix(cli): let /session detach from a running Turn instead of trapping the client MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A second TUI that switched onto a Session with an in-flight Turn was trapped: every exit path was destructive. /session was intercepted with 'Cannot run /session while a turn is running', and following the hint (Esc/Ctrl+C) or quitting aborted the shared Turn via driver.stop() — killing work another client was watching (#3380). Multi-client attach is a designed Runtime Host capability: the Turn is Host-owned and the TUI is only its viewport, so switching Sessions mid-turn is view navigation, not a session mutation. - new 'switch' mid-turn slash disposition alongside 'local': routed through like 'local', but its handler must use the busy-aware goToSession/openSessionPicker wrappers (runControl's serial lock is held by the running Turn mid-turn) - switchAwayMidTurn adopts the next Session without ever calling driver.stop(); a turnEpoch fence orphans the in-flight drain after the switch is confirmed so late events, synthesized stream failures, and old-session queue flushes can never reach the adopted transcript; the orphaned runAgentTurn tail releases busy/activity and hands the freshly attached Turn its start exactly once - requestTurnInterrupt is swallowed while a detach handoff is in flight: the driver already points at the next Session, so a stop there would abort whatever that Session has attached - Escape closes the mid-turn session picker instead of arming the double-Escape interrupt for the Turn being left running - foreign-session import rows are hidden from the picker mid-turn (the import flow starts a new Session; it cannot detach) Generated-by: Maka --- packages/cli/src/pi-tui-pickers.ts | 13 ++- packages/cli/src/pi-tui-runner.ts | 171 ++++++++++++++++++++++++++--- 2 files changed, 161 insertions(+), 23 deletions(-) diff --git a/packages/cli/src/pi-tui-pickers.ts b/packages/cli/src/pi-tui-pickers.ts index 139d3e8c93..7e68f66b6e 100644 --- a/packages/cli/src/pi-tui-pickers.ts +++ b/packages/cli/src/pi-tui-pickers.ts @@ -258,16 +258,19 @@ export interface MakaSlashCommandMetadata { * /exit with arguments, which isExitPrompt does not match — can still reach * the disposition, where it falls through to 'refuse'. */ -export type SlashCommandMidTurnDisposition = 'local' | 'refuse' | 'intercepted'; +export type SlashCommandMidTurnDisposition = 'local' | 'switch' | 'refuse' | 'intercepted'; export interface MakaSlashCommand extends MakaSlashCommandMetadata { /** * Mid-turn disposition. 'local' requires the handler to be independent of * the running turn — it must not enter runControl, whose busy gate would - * silently no-op. 'refuse' is the safe default for anything that mutates - * session state or opens a picker the turn would race. Declared on every - * handler so a newly added command must state its answer instead of - * inheriting one from the routing call site. + * silently no-op. 'switch' is allowed mid-turn because it detaches this + * client's VIEW from the running Turn without touching it (Runtime Host + * mode keeps the Turn alive; #3380) — its handler must route through the + * detach path, not runControl, while a turn runs. 'refuse' is the safe + * default for anything that mutates session state or opens a picker the + * turn would race. Declared on every handler so a newly added command must + * state its answer instead of inheriting one from the routing call site. */ midTurn: SlashCommandMidTurnDisposition; run(parts: string[], rawTail: string | undefined, context: { idleMs: number }): void; diff --git a/packages/cli/src/pi-tui-runner.ts b/packages/cli/src/pi-tui-runner.ts index 74da0e8664..bde4e26c3c 100644 --- a/packages/cli/src/pi-tui-runner.ts +++ b/packages/cli/src/pi-tui-runner.ts @@ -326,8 +326,21 @@ export async function runMakaPiTui(input: MakaPiTuiInput): Promise { } | undefined; let turnRunning = false; + // Monotonic generation for visible agent turns. A mid-turn `/session` + // switch-away (#3380) bumps it to orphan the in-flight drain: every callback + // of that runAgentTurn (events, failures, queue flushes) captured the epoch + // at start and becomes a no-op once superseded, so nothing from the + // abandoned Session reaches the adopted one's transcript. + let turnEpoch = 0; let turnStartedAt: number | undefined; let interruptRequested = false; + // True while a mid-turn detach-switch is in flight: an interrupt issued in + // that window would target the freshly attached Session instead of the Turn + // being left behind. + let detaching = false; + // True while the /session picker is open mid-turn: Escape must close the + // overlay, not arm the double-Escape interrupt for the running Turn (#3380). + let sessionPickerOverlayOpen = false; let lastTurnEscapeAt = 0; let lastIdleEscapeAt = 0; let lastIdleCtrlCAt = 0; @@ -777,7 +790,10 @@ export async function runMakaPiTui(input: MakaPiTuiInput): Promise { }; const requestTurnInterrupt = () => { - if (interruptRequested) return; + // A detach in flight is not the running Turn's owner acting on it — the + // driver already points at the next Session, so a stop here would abort + // whatever that Session has attached. Swallow until the handoff settles. + if (interruptRequested || detaching) return; interruptRequested = true; // The convergence window (stop issued, turn not yet terminal) accepts no // new input: submits would race the abort and could open work the user @@ -1087,14 +1103,16 @@ export async function runMakaPiTui(input: MakaPiTuiInput): Promise { // Known slash commands typed mid-turn follow the disposition declared on // the command itself (`midTurn`, review finding on turnRunning routing): // 'local' commands answer immediately because their handler is - // independent of the running turn; every other known command is refused - // with a clear message, since it would either mutate session state - // behind the turn's back, open a picker the turn would race, or silently - // no-op on the runControl busy gate. ('intercepted' commands — /exit, - // /swarm, /graph — were claimed by their dedicated checks above and - // reaching the refusal here only means an unrecognized form.) Unknown - // slash-prefixed text still steers: it may be intended prompt text (a - // skill invocation such as `/skill:`, or a path). + // independent of the running turn; 'switch' commands detach this + // client's view from the running Turn and adopt another Session (#3380); + // every other known command is refused with a clear message, since it + // would either mutate session state behind the turn's back, open a + // picker the turn would race, or silently no-op on the runControl busy + // gate. ('intercepted' commands — /exit, /swarm, /graph — were claimed + // by their dedicated checks above and reaching the refusal here only + // means an unrecognized form.) Unknown slash-prefixed text still + // steers: it may be intended prompt text (a skill invocation such as + // `/skill:`, or a path). const commandToken = prompt.trim().split(/\s+/, 1)[0] ?? ''; const knownCommand = slashCommands.find( (candidate) => @@ -1103,7 +1121,10 @@ export async function runMakaPiTui(input: MakaPiTuiInput): Promise { ); if (knownCommand) { editor.addToHistory(prompt); - if (knownCommand.midTurn === 'local') { + // 'switch' dispositions route through like 'local': their handlers are + // busy-aware and detach from the running Turn instead of touching it + // (#3380). + if (knownCommand.midTurn === 'local' || knownCommand.midTurn === 'switch') { handleSlashCommand(prompt, 0); } else { state.entries.push({ @@ -1127,6 +1148,11 @@ export async function runMakaPiTui(input: MakaPiTuiInput): Promise { authoritativeAttachedTurn?: MakaAttachedSessionTurn, ): Promise { busy = true; + const epoch = ++turnEpoch; + // A mid-turn /session switch-away (#3380) bumps turnEpoch and orphans this + // drain: from that point every callback below must stop touching shared + // runner state — the adopted Session owns it now. + const superseded = () => epoch !== turnEpoch; const activity = beginActivity(); turnRunning = true; turnStartedAt = Date.now(); @@ -1195,6 +1221,9 @@ export async function runMakaPiTui(input: MakaPiTuiInput): Promise { showSkillInvocation(skillInvocation); }, onEvent: (event) => { + // Orphaned by a mid-turn detach: the abandoned Session's stream must + // not reach the adopted Session's transcript or overlays. + if (superseded()) return; if ( (event.type === 'sandbox_boundary_request' || event.type === 'user_question_request') && resolvedInteractionIds.delete(event.requestId) @@ -1227,6 +1256,11 @@ export async function runMakaPiTui(input: MakaPiTuiInput): Promise { // A turn failing is worth pulling the user back, regardless of how long it // ran — a quick failure in a background tab would otherwise stay silent. onFailure: (error) => { + // Orphaned by a mid-turn detach: the abandoned drain ends without a + // terminal event (channel close finishes its queue), which surfaces + // here as "ended without completion" — never report that against the + // adopted Session. + if (superseded()) return; appendTurnFailureToTranscript(state, error); attention.attentionNeeded(); shellRunElapsedTicker.sync(); @@ -1241,6 +1275,22 @@ export async function runMakaPiTui(input: MakaPiTuiInput): Promise { activity.finish(); return outcome; } + if (superseded()) { + // Orphaned by a mid-turn detach (#3380): the Session this turn ran + // on is no longer adopted. Skip every continuation that belongs to + // it — queue flushes would steer the NEW Session, fallback texts + // would refill the editor with abandoned-session context, and a + // failure notice would misreport the still-running Host Turn. Only + // release the slot and hand the freshly attached Turn its start; + // startPendingAttachedTurn no-ops until applySwitchResult has + // installed it and we are idle, and the detach path re-arms it, so + // exactly one side starts it whichever unwinds first. + busy = false; + activity.finish(); + requestRender(); + startPendingAttachedTurn(); + return outcome; + } // Turn boundary flush: CLI-held fallback texts that never reached the // runtime (the enqueue retry never found a live owner) are delivered @@ -1499,6 +1549,76 @@ export async function runMakaPiTui(input: MakaPiTuiInput): Promise { requestRender(); }; + // Mid-turn `/session` switch-away (#3380): adopt another Session while a + // Turn is still running on the current one. In Runtime Host mode the Turn is + // Host-owned — this TUI was only its viewport — so detaching the view must + // not stop it (unlike the interrupt path, driver.stop() is never called + // here). Bumping turnEpoch orphans the in-flight drain; its runAgentTurn + // tail unwinds through the superseded branch and releases busy/activity, + // then either that tail or the startPendingAttachedTurn below starts the + // freshly attached Turn, whichever observes an idle runner first. + const switchAwayMidTurn = async (sessionId: string) => { + resolvedInteractionIds.clear(); + detaching = true; + try { + // Fence only after the driver confirms the switch: a failed switch must + // leave the in-flight drain fully live. Events the abandoned queue + // yields between the channel closing inside switchSession and + // replaceTranscript below are wiped by that replacement; everything + // after it hits the superseded fence. + const result = await input.driver.switchSession(sessionId); + turnEpoch += 1; + await applySwitchResult(result); + // Same adoption-time announcement as the idle path: applySwitchResult + // replaced the transcript, so a live durable Goal on the adopted Session + // must be re-announced here rather than silently auto-continuing. + currentGoal = input.driver.getGoal?.() ?? null; + if ( + currentGoal !== null && + (currentGoal.status === 'active' || currentGoal.status === 'waiting') + ) { + state.entries.push({ + kind: 'notice', + level: 'info', + text: goalAttachedNoticeText(currentGoal), + }); + } + if (result.messages.length === 0) { + state.entries.push({ + kind: 'notice', + level: 'info', + text: `Resumed session "${result.summary.name}"`, + }); + } + state.entries.push({ + kind: 'notice', + level: 'info', + text: 'Detached from the running Turn — it keeps running. /session back to reattach.', + }); + requestRender(); + } finally { + detaching = false; + startPendingAttachedTurn(); + } + }; + + // `/session` is view navigation (#3380). Idle, it runs under runControl's + // serial lock like any control action; mid-turn that lock is held by the + // running Turn, so the switch goes through the detach path instead of + // silently no-oping on the busy gate. + const goToSession = async (sessionId: string): Promise => { + if (!turnRunning) { + await runControl(() => switchSession(sessionId)); + return; + } + await switchAwayMidTurn(sessionId).catch(reportError); + }; + const openSessionPicker = (): Promise => { + if (!turnRunning) return runControl(showSessionList); + // The picker itself is a passive overlay; only its selection detaches. + return showSessionList().catch(reportError); + }; + // Rewind branches the active session to just before the chosen turn and // switches onto the branch (driver.rewindToTurn), then refills the editor with // that turn's prompt. The original session is left intact, so this is @@ -1978,7 +2098,11 @@ export async function runMakaPiTui(input: MakaPiTuiInput): Promise { ] as const; }), ), - input.foreignSessions + // Foreign (Claude Code / Codex) rows are an import flow: it starts a NEW + // Session and hands off a turn, which cannot detach from the running + // one (#3380). Skip the scan mid-turn instead of offering rows whose + // selection would silently no-op on importForeignSession's busy guard. + input.foreignSessions && !turnRunning ? input.foreignSessions.listSessions({ cwd }).then( (summaries) => ({ summaries }), (error: unknown) => ({ error }), @@ -2039,18 +2163,23 @@ export async function runMakaPiTui(input: MakaPiTuiInput): Promise { maxPrimaryColumnWidth: Math.max(20, terminal.columns - 30), }); let overlay: OverlayHandle | undefined; + const closeOverlay = () => { + sessionPickerOverlayOpen = false; + overlay?.hide(); + }; list.onSelect = (item) => { const foreign = foreignByValue.get(item.value); if (foreign) { - overlay?.hide(); + closeOverlay(); void importForeignSession(foreign); return; } if (availability.get(item.value)?.available === false) return; - overlay?.hide(); - void runControl(() => switchSession(item.value)); + closeOverlay(); + void goToSession(item.value); }; - list.onCancel = () => overlay?.hide(); + list.onCancel = () => closeOverlay(); + sessionPickerOverlayOpen = true; overlay = showBottomPicker( new PickerOverlay(list, { title: 'Resume Session', @@ -2934,10 +3063,13 @@ export async function runMakaPiTui(input: MakaPiTuiInput): Promise { }, session: { description: primaryGuidance.commands.session, - midTurn: 'refuse', + // View navigation, not a session mutation: mid-turn it detaches from the + // running Turn instead of touching it (#3380), so it is allowed through + // where mutating commands are refused. + midTurn: 'switch', run: (parts: string[]) => { if (parts.length === 1) { - void runControl(showSessionList); + void openSessionPicker(); return; } const sessionId = parts.length === 2 ? parts[1] : undefined; @@ -2950,7 +3082,7 @@ export async function runMakaPiTui(input: MakaPiTuiInput): Promise { requestRender(); return; } - void runControl(() => switchSession(sessionId)); + void goToSession(sessionId); }, }, graph: { @@ -3094,6 +3226,9 @@ export async function runMakaPiTui(input: MakaPiTuiInput): Promise { // boundary branch so Escape keeps meaning "deny" while a prompt is // pending, and it only arms while a prompt turn is actually running. if (turnRunning && matchesKey(data, Key.escape)) { + // The mid-turn /session picker owns Escape while it is open — closing + // it must never arm an interrupt for the Turn being left running. + if (sessionPickerOverlayOpen) return undefined; // Once an interrupt is issued, swallow further Escapes until the turn // ends so a still-settling stop is not requested twice. A rejected stop // re-arms interruption so the user can retry within the same turn. From 24a3a6711870a427bd82f2b8f03b86b4f52b37c4 Mon Sep 17 00:00:00 2001 From: me2seeks Date: Sat, 22 Aug 2026 21:10:53 +0800 Subject: [PATCH 2/3] test(cli): cover mid-turn /session detach, picker Escape, and failed-switch recovery - '/session ' mid-turn: switches without driver.stop(), replaces the transcript with the adopted Session's history, fences late events from the abandoned drain (no content leak, no synthesized 'ended without a completion event' failure), starts the freshly attached Turn only after the orphaned drain unwinds, and lands follow-up prompts on the adopted Session - '/session' mid-turn opens the picker; Escape closes it and must not arm the double-Escape interrupt (stopCalls stays 0) - a rejected switch leaves the running Turn fully live: error notice, no stop, subsequent events still render into the same transcript Generated-by: Maka --- .../cli/src/__tests__/pi-tui-runner.test.ts | 247 ++++++++++++++++++ 1 file changed, 247 insertions(+) diff --git a/packages/cli/src/__tests__/pi-tui-runner.test.ts b/packages/cli/src/__tests__/pi-tui-runner.test.ts index 0414e7c6e9..c9e04f37c2 100644 --- a/packages/cli/src/__tests__/pi-tui-runner.test.ts +++ b/packages/cli/src/__tests__/pi-tui-runner.test.ts @@ -5047,6 +5047,159 @@ describe('Maka Pi TUI runner', () => { await run; }); + test('/session mid-turn detaches from the running Turn instead of refusing', async () => { + const terminal = new FakeTerminal(); + const driver = new DetachingSwitchDriver([ + storedUserMessage('user-s2', 'turn-old-2', 'history from session two'), + storedAssistantMessage('assistant-s2', 'turn-old-2', 'prior answer'), + ]); + const run = runMakaPiTui({ + title: 'Maka', + driver, + cwd: '/repo', + model: 'm', + connectionSlug: 'c', + permissionMode: 'bypass', + terminal, + }); + + terminal.input('start the long task'); + terminal.input('\r'); + await waitFor(() => terminal.progressStates.at(-1) === true); + + // The escape hatch a second TUI needs (#3380): switching Sessions + // mid-turn detaches the view and leaves the Host-owned Turn running, + // instead of refusing (trapping the client) or stopping the Turn. + terminal.input('/session session-2'); + terminal.input('\r'); + await waitFor(() => + plainTerminalOutput(terminal.output()).includes('Detached from the running Turn'), + ); + assert.equal(driver.stopCalls, 0); + assert.deepEqual(driver.sessionIds, ['session-2']); + // The adopted Session's history replaced the old transcript. + assert.match(plainTerminalOutput(terminal.screenOutput()), /history from session two/); + + // Late events from the abandoned Turn never reach the adopted + // transcript — neither as content nor as a synthesized failure about + // the stream ending without a completion event. + driver.emit({ + type: 'text_delta', + id: 'delta-leak', + turnId: 'turn-1', + messageId: 'assistant-old', + ts: 2, + text: 'LEAK-OLD-DELTA', + }); + driver.releaseOldTurn(); + await waitFor(() => plainTerminalOutput(terminal.output()).includes('attached replay done')); + const after = plainTerminalOutput(terminal.output()); + assert.doesNotMatch(after, /LEAK-OLD-DELTA/); + assert.doesNotMatch(after, /without a completion event/); + assert.equal(driver.stopCalls, 0); + // The orphaned drain released the runner, and only then did the freshly + // attached Turn of session-2 start and complete. + await waitFor(() => terminal.progressStates.at(-1) === false); + + // A follow-up prompt lands on the adopted Session. + terminal.input('next step'); + terminal.input('\r'); + await waitFor(() => driver.displayPrompts.includes('next step')); + assert.deepEqual(driver.sessionIds, ['session-2']); + + terminal.input('/exit'); + terminal.input('\r'); + await run; + }); + + test('/session mid-turn opens the picker and Escape closes it without arming an interrupt', async () => { + const terminal = new FakeTerminal(); + const driver = new DetachingSwitchDriver([]); + const run = runMakaPiTui({ + title: 'Maka', + driver, + cwd: '/repo', + model: 'm', + connectionSlug: 'c', + permissionMode: 'bypass', + terminal, + }); + + terminal.input('start the long task'); + terminal.input('\r'); + await waitFor(() => terminal.progressStates.at(-1) === true); + + terminal.input('/session'); + terminal.input('\r'); + await waitFor(() => plainTerminalOutput(terminal.screenOutput()).includes('Resume Session')); + assert.doesNotMatch( + plainTerminalOutput(terminal.output()), + /Cannot run \/session while a turn is running/, + ); + + // Escape belongs to the overlay while it is open: closing it must not + // arm the double-Escape interrupt — a second Escape would otherwise + // abort the very Turn the user is navigating away from (#3380). + terminal.input('\x1b'); + await waitFor(() => !plainTerminalOutput(terminal.screenOutput()).includes('Resume Session')); + await delay(50); + assert.equal(driver.stopCalls, 0); + assert.equal(terminal.progressStates.at(-1), true); + + // Settle the parked Turn normally, then leave. + driver.releaseOldTurn(); + await waitFor(() => terminal.progressStates.at(-1) === false); + terminal.input('/exit'); + terminal.input('\r'); + await run; + }); + + test('a failed mid-turn /session leaves the running Turn fully live', async () => { + const terminal = new FakeTerminal(); + const driver = new DetachingSwitchDriver([]); + const run = runMakaPiTui({ + title: 'Maka', + driver, + cwd: '/repo', + model: 'm', + connectionSlug: 'c', + permissionMode: 'bypass', + terminal, + }); + + terminal.input('start the long task'); + terminal.input('\r'); + await waitFor(() => terminal.progressStates.at(-1) === true); + + // A rejected switch must not orphan the in-flight drain: the error is + // reported, nothing was switched, and the Turn keeps streaming into the + // same transcript. + driver.failNextSwitch = true; + terminal.input('/session does-not-exist'); + terminal.input('\r'); + await waitFor(() => plainTerminalOutput(terminal.output()).includes('session not found')); + assert.equal(driver.stopCalls, 0); + assert.equal(terminal.progressStates.at(-1), true); + + driver.emit({ + type: 'text_delta', + id: 'delta-after-failure', + turnId: 'turn-1', + messageId: 'assistant-old', + ts: 3, + text: 'still streaming after failure', + }); + await waitFor(() => + plainTerminalOutput(terminal.output()).includes('still streaming after failure'), + ); + + driver.releaseOldTurn(); + await waitFor(() => terminal.progressStates.at(-1) === false); + terminal.input('/exit'); + terminal.input('\r'); + await run; + }); + test('unknown slash-prefixed text still steers into the running turn', async () => { const terminal = new FakeTerminal(); const driver = new SteeringTurnDriver(); @@ -6888,6 +7041,100 @@ class ActiveResumeDriver extends SlashCommandDriver { } } +// A parking first Turn on session-1 plus a switchable session-2, for the +// mid-turn /session detach tests (#3380). The parked stream ends only when +// the test releases it or stop() lands — a detach leaves it running, exactly +// like a Host-owned Turn surviving a client that switches away. Later prompts +// (submitted after switching) complete immediately. +class DetachingSwitchDriver extends SlashCommandDriver { + stopCalls = 0; + /** When set, the next switchSession rejects — a failed detach must leave + * the running drain fully live. */ + failNextSwitch = false; + private pendingEvents: SessionEvent[] = []; + private wakeTurn: (() => void) | null = null; + private turnEnded = false; + private promptCount = 0; + + constructor(sessionTwoMessages: StoredMessage[]) { + super([fakeSessionSummary('session-2', '/repo')], new Map([['session-2', sessionTwoMessages]])); + } + + /** Queues an event onto the parked first-session Turn. */ + emit(event: SessionEvent): void { + this.pendingEvents.push(event); + this.wakeTurn?.(); + this.wakeTurn = null; + } + + /** Ends the parked stream the way a Host does when its Turn settles. */ + releaseOldTurn(): void { + this.turnEnded = true; + this.wakeTurn?.(); + this.wakeTurn = null; + } + + override async *promptEvents(_prompt: string, turnId = 'turn-1'): AsyncIterable { + this.promptCount += 1; + if (this.promptCount > 1) { + yield { type: 'complete', id: `complete-${turnId}`, turnId, ts: 9, stopReason: 'end_turn' }; + return; + } + for (;;) { + while (this.pendingEvents.length > 0) yield this.pendingEvents.shift()!; + if (this.turnEnded) break; + await new Promise((resolve) => { + this.wakeTurn = resolve; + }); + } + yield { type: 'abort', id: 'abort-old', turnId, ts: 8, reason: 'user_stop' }; + yield { type: 'complete', id: 'complete-old', turnId, ts: 9, stopReason: 'user_stop' }; + } + + override async stop(): Promise { + this.stopCalls += 1; + this.turnEnded = true; + this.wakeTurn?.(); + this.wakeTurn = null; + } + + // session-2 carries a live Turn, so adopting it hands back an activeTurn — + // the reattach path the runner must start once the orphaned drain unwinds. + override async switchSession(sessionId: string): Promise { + if (this.failNextSwitch) { + this.failNextSwitch = false; + throw new Error('session not found'); + } + const switched = await super.switchSession(sessionId); + if (sessionId !== 'session-2') return switched; + const attachedTurnId = 'turn-attached-2'; + return { + ...switched, + activeTurn: { + sessionId, + turnId: attachedTurnId, + events: (async function* () { + yield { + type: 'text_complete', + id: 'text-attached', + turnId: attachedTurnId, + messageId: 'assistant-attached', + ts: 3, + text: 'attached replay done', + } satisfies SessionEvent; + yield { + type: 'complete', + id: 'complete-attached', + turnId: attachedTurnId, + ts: 4, + stopReason: 'end_turn', + } satisfies SessionEvent; + })(), + }, + }; + } +} + class HostSuccessorDriver extends SlashCommandDriver { #startedTurnListener: ((turn: MakaAttachedSessionTurn) => void) | undefined; readonly #probeFirst = deferred(); From 3324047598651d6b725bdb579cb66c3b0eeb99cc Mon Sep 17 00:00:00 2001 From: me2seeks Date: Sun, 23 Aug 2026 13:33:07 +0800 Subject: [PATCH 3/3] fix(cli): fence mid-turn detach against re-entry and in-flight callbacks Two review findings on the #3380 detach path: - switchAwayMidTurn was re-entrant: a second mid-turn /session while the first was still handing the view over cleared the detaching flag early, reopening the interrupt window and double-applying adoption. The entry now rejects while a detach is in flight (the picker selection routes through the same guard). - onPrepared and onSkillInvocation ran without the superseded() fence the other callbacks use. Both are reachable after a switch-away because they fire only after preparePrompt resolves, so an abandoned Turn could still adopt its metadata onto the adopted Session's view and surface its skill card over the adopted viewport. Both fixes are pinned by tests: a parked second /session yields exactly one detach notice, and a Turn prepared across a detach can no longer overwrite the adopted session's title/cwd. Generated-by: maka --- .../cli/src/__tests__/pi-tui-runner.test.ts | 124 ++++++++++++++++++ packages/cli/src/pi-tui-runner.ts | 12 ++ 2 files changed, 136 insertions(+) diff --git a/packages/cli/src/__tests__/pi-tui-runner.test.ts b/packages/cli/src/__tests__/pi-tui-runner.test.ts index c9e04f37c2..36536a3875 100644 --- a/packages/cli/src/__tests__/pi-tui-runner.test.ts +++ b/packages/cli/src/__tests__/pi-tui-runner.test.ts @@ -5200,6 +5200,120 @@ describe('Maka Pi TUI runner', () => { await run; }); + test('a second mid-turn /session while a detach is in flight is ignored', async () => { + const terminal = new FakeTerminal(); + const driver = new DetachingSwitchDriver([ + storedUserMessage('user-s2', 'turn-old-2', 'history from session two'), + storedAssistantMessage('assistant-s2', 'turn-old-2', 'prior answer'), + ]); + const run = runMakaPiTui({ + title: 'Maka', + driver, + cwd: '/repo', + model: 'm', + connectionSlug: 'c', + permissionMode: 'bypass', + terminal, + }); + + terminal.input('start the long task'); + terminal.input('\r'); + await waitFor(() => terminal.progressStates.at(-1) === true); + + // Park the first switch inside driver.switchSession, then fire a second + // /session while `detaching` is still held: re-entry would clear the + // flag early, reopen the interrupt window, and double-apply adoption. + let releaseSwitch!: () => void; + driver.holdSwitch = new Promise((resolve) => { + releaseSwitch = resolve; + }); + terminal.input('/session session-2'); + terminal.input('\r'); + await waitFor(() => driver.switchEntries >= 1); + terminal.input('/session session-2'); + terminal.input('\r'); + releaseSwitch(); + + await waitFor(() => + plainTerminalOutput(terminal.output()).includes('Detached from the running Turn'), + ); + assert.equal(driver.stopCalls, 0); + assert.deepEqual(driver.sessionIds, ['session-2']); + // Exactly one detach notice: the second switch never ran. + const notices = plainTerminalOutput(terminal.output()).match( + /Detached from the running Turn/g, + ); + assert.equal(notices?.length, 1); + + driver.releaseOldTurn(); + await waitFor(() => terminal.progressStates.at(-1) === false); + terminal.input('/exit'); + terminal.input('\r'); + await run; + }); + + test('a turn prepared after a mid-turn detach does not adopt abandoned metadata', async () => { + const terminal = new FakeTerminal(); + const driver = new DetachingSwitchDriver([ + storedUserMessage('user-s2', 'turn-old-2', 'history from session two'), + storedAssistantMessage('assistant-s2', 'turn-old-2', 'prior answer'), + ]); + const run = runMakaPiTui({ + title: 'Maka', + driver, + cwd: '/repo', + model: 'm', + connectionSlug: 'c', + permissionMode: 'bypass', + terminal, + }); + + // Park preparePrompt itself: while it is unresolved, /session can + // already detach — onPrepared/onSkillInvocation then fire for the + // abandoned Turn after the epoch fence moved. + let releasePrepare!: () => void; + const parkedPrepare = new Promise((resolve) => { + releasePrepare = resolve; + }); + const basePrepare = driver.preparePrompt.bind(driver); + driver.preparePrompt = async (prompt, options) => { + const turn = await basePrepare(prompt, options); + await parkedPrepare; + return { + ...turn, + summary: fakeSessionSummary('abandoned-session', '/abandoned-cwd', 'ABANDONED TITLE'), + }; + }; + + terminal.input('start the long task'); + terminal.input('\r'); + await waitFor(() => terminal.progressStates.at(-1) === true); + + terminal.input('/session session-2'); + terminal.input('\r'); + // Nothing else drives the frame loop while preparePrompt stays parked, + // so force a repaint for the detach notices. + terminal.resize(80, 24); + await waitFor(() => + plainTerminalOutput(terminal.output()).includes('Detached from the running Turn'), + ); + assert.match(plainTerminalOutput(terminal.screenOutput()), /history from session two/); + + // The abandoned Turn's prepare resolves only now — its summary must + // not steal the adopted Session's metadata. + releasePrepare(); + driver.releaseOldTurn(); + await waitFor(() => plainTerminalOutput(terminal.output()).includes('attached replay done')); + assert.equal(terminal.titles.includes('ABANDONED TITLE (Maka)'), false); + assert.equal(terminal.titles.at(-1), 'Existing chat (Maka)'); + assert.doesNotMatch(plainTerminalOutput(terminal.screenOutput()), /\/abandoned-cwd/); + + await waitFor(() => terminal.progressStates.at(-1) === false); + terminal.input('/exit'); + terminal.input('\r'); + await run; + }); + test('unknown slash-prefixed text still steers into the running turn', async () => { const terminal = new FakeTerminal(); const driver = new SteeringTurnDriver(); @@ -7051,6 +7165,10 @@ class DetachingSwitchDriver extends SlashCommandDriver { /** When set, the next switchSession rejects — a failed detach must leave * the running drain fully live. */ failNextSwitch = false; + /** When set, the next switchSession parks until released — a second + * mid-turn /session arriving while the first is still in flight. */ + holdSwitch: Promise | undefined; + switchEntries = 0; private pendingEvents: SessionEvent[] = []; private wakeTurn: (() => void) | null = null; private turnEnded = false; @@ -7101,10 +7219,16 @@ class DetachingSwitchDriver extends SlashCommandDriver { // session-2 carries a live Turn, so adopting it hands back an activeTurn — // the reattach path the runner must start once the orphaned drain unwinds. override async switchSession(sessionId: string): Promise { + this.switchEntries += 1; if (this.failNextSwitch) { this.failNextSwitch = false; throw new Error('session not found'); } + if (this.holdSwitch) { + const gate = this.holdSwitch; + this.holdSwitch = undefined; + await gate; + } const switched = await super.switchSession(sessionId); if (sessionId !== 'session-2') return switched; const attachedTurnId = 'turn-attached-2'; diff --git a/packages/cli/src/pi-tui-runner.ts b/packages/cli/src/pi-tui-runner.ts index bde4e26c3c..1a47fc299a 100644 --- a/packages/cli/src/pi-tui-runner.ts +++ b/packages/cli/src/pi-tui-runner.ts @@ -1195,6 +1195,10 @@ export async function runMakaPiTui(input: MakaPiTuiInput): Promise { requestRender(); }, onPrepared: async (turn) => { + // Orphaned by a mid-turn detach: this can still fire after the + // switch resolved (preparePrompt was in flight), and the abandoned + // Turn's metadata must not overwrite the adopted Session's view. + if (superseded()) return; if (authoritativeAttachedTurn) { adoptSessionMetadata(authoritativeAttachedTurn.summary); replaceTranscript(authoritativeAttachedTurn.messages); @@ -1209,6 +1213,10 @@ export async function runMakaPiTui(input: MakaPiTuiInput): Promise { if (turn.summary) adoptSessionMetadata(turn.summary); }, onSkillInvocation: (skillInvocation) => { + // Same mid-turn detach fence as onPrepared/onEvent: a skill card + // belonging to the abandoned Session must not land on the adopted + // viewport (covers the blocked-invocation path too). + if (superseded()) return; if ( skillInvocation.loaded.length === 0 && skillInvocation.failed.length > 0 && @@ -1611,6 +1619,10 @@ export async function runMakaPiTui(input: MakaPiTuiInput): Promise { await runControl(() => switchSession(sessionId)); return; } + // One detach at a time (#3380): a second mid-turn switch while the first + // is still handing the view over would clear `detaching` early, reopen + // the interrupt window, and double-apply the adoption. + if (detaching) return; await switchAwayMidTurn(sessionId).catch(reportError); }; const openSessionPicker = (): Promise => {