From 372651bcb1ffa86aa447ec25fd32bf6d67f35dc4 Mon Sep 17 00:00:00 2001 From: indifferentketchup Date: Sun, 31 May 2026 21:31:35 +0000 Subject: [PATCH] fix(coder): F.1 post-interrupt stale-terminal guard (opencode warm server) opencode emits one trailing session.idle/error for a turn cancelled via client.session.abort(), carrying only a sessionID (no turn id). The warm-server backend settled activeTurn on that event, so after Stop + an immediate new message the orphan idle settled the NEXT turn early as success (one-click reachable since v2.6.5's Send->Stop composer). Adds a pure per-session guard (backends/turn-guard.ts: armAbortGuard / noteTurnActivity / consumeTerminal over swallowNextTerminal) wired into opencode-server.ts: abort arms it, the next terminal is swallowed once, and a new turn's first delta self-heals so a never-arriving orphan can't strand a real turn. Test-first; 3 regression tests in turn-guard.test.ts. Paseo parallel: 1d38aac. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../backends/__tests__/turn-guard.test.ts | 34 +++++++++++++++++ .../src/services/backends/opencode-server.ts | 21 +++++++++- .../coder/src/services/backends/turn-guard.ts | 38 +++++++++++++++++++ 3 files changed, 91 insertions(+), 2 deletions(-) create mode 100644 apps/coder/src/services/backends/__tests__/turn-guard.test.ts create mode 100644 apps/coder/src/services/backends/turn-guard.ts diff --git a/apps/coder/src/services/backends/__tests__/turn-guard.test.ts b/apps/coder/src/services/backends/__tests__/turn-guard.test.ts new file mode 100644 index 0000000..87f73ce --- /dev/null +++ b/apps/coder/src/services/backends/__tests__/turn-guard.test.ts @@ -0,0 +1,34 @@ +import { describe, it, expect } from 'vitest'; +import { + armAbortGuard, + noteTurnActivity, + consumeTerminal, + type AbortTerminalGuard, +} from '../turn-guard.js'; + +describe('post-abort terminal guard (F.1)', () => { + it('swallows the orphan terminal that follows an abort, then settles the next real one', () => { + // Reproduces the v2.6.5 Stop-button bug: abort turn A, then opencode emits a + // trailing session.idle for A. That orphan must NOT settle the next turn. + const g: AbortTerminalGuard = { swallowNextTerminal: false }; + + armAbortGuard(g); // user aborts turn A + expect(consumeTerminal(g)).toBe('swallow'); // opencode's orphan idle for A → dropped + expect(consumeTerminal(g)).toBe('settle'); // turn B's real idle → settles B + }); + + it('settles a terminal when no abort happened', () => { + const g: AbortTerminalGuard = { swallowNextTerminal: false }; + expect(consumeTerminal(g)).toBe('settle'); + }); + + it('self-heals if the orphan never arrives: new-turn activity clears the guard', () => { + // If opencode emits no orphan idle (e.g. abort-before-prompt), the next turn's + // real terminal must still settle rather than being swallowed forever. + const g: AbortTerminalGuard = { swallowNextTerminal: false }; + + armAbortGuard(g); // abort A, but no orphan idle arrives + noteTurnActivity(g); // turn B produces its first delta + expect(consumeTerminal(g)).toBe('settle'); // turn B's idle settles, not swallowed + }); +}); diff --git a/apps/coder/src/services/backends/opencode-server.ts b/apps/coder/src/services/backends/opencode-server.ts index a3d7d8c..4b62456 100644 --- a/apps/coder/src/services/backends/opencode-server.ts +++ b/apps/coder/src/services/backends/opencode-server.ts @@ -37,6 +37,7 @@ import { import type { ToolCallStatus } from '@agentclientprotocol/sdk'; import type { Sql } from '../../db.js'; import type { AcpToolSnapshot } from '../acp-tool-snapshot.js'; +import { armAbortGuard, noteTurnActivity, consumeTerminal } from './turn-guard.js'; import type { AgentBackend, AgentEvent, @@ -78,6 +79,9 @@ interface SessionState { /** Per-session SSE subscription handle. Non-null while the loop is running; * aborting it tears down the underlying fetch and exits the loop. */ sseAbort: AbortController | null; + /** F.1 post-abort orphan-terminal guard: swallow the one session.idle/error + * opencode emits for an aborted turn so it can't settle the next turn. */ + swallowNextTerminal: boolean; } export interface OpenCodeServerBackendDeps { @@ -305,13 +309,19 @@ export class OpenCodeServerBackend implements AgentBackend { } // ─── lifecycle ───────────────────────────────────────────────────────── case 'session.idle': { - this.byOpencodeId.get(ev.properties.sessionID)?.activeTurn?.settle({ ok: true }); + const st = this.byOpencodeId.get(ev.properties.sessionID); + if (!st) return; + if (consumeTerminal(st) === 'swallow') return; // F.1: drop the post-abort orphan + st.activeTurn?.settle({ ok: true }); return; } case 'session.error': { const sid = ev.properties.sessionID; if (!sid) return; - this.byOpencodeId.get(sid)?.activeTurn?.settle({ ok: false, error: errToString(ev.properties.error) }); + const st = this.byOpencodeId.get(sid); + if (!st) return; + if (consumeTerminal(st) === 'swallow') return; // F.1: drop the post-abort orphan + st.activeTurn?.settle({ ok: false, error: errToString(ev.properties.error) }); return; } default: @@ -358,6 +368,8 @@ export class OpenCodeServerBackend implements AgentBackend { /** Reset the inactivity backstop on any event routed to a session's active turn. */ private bumpActivity(st: SessionState): void { if (!st.activeTurn) return; + // A live turn is producing → the post-abort orphan-terminal window is over. + noteTurnActivity(st); if (st.watchdog) clearTimeout(st.watchdog); st.watchdog = setTimeout(() => { void this.onTurnStall(st); @@ -490,6 +502,7 @@ export class OpenCodeServerBackend implements AgentBackend { activeTurn: null, watchdog: null, sseAbort: null, + swallowNextTerminal: false, }; this.byOpencodeId.set(ocSessionId, state); } @@ -528,6 +541,7 @@ export class OpenCodeServerBackend implements AgentBackend { activeTurn: null, watchdog: null, sseAbort: null, + swallowNextTerminal: false, }; this.byOpencodeId.set(oc, state); } @@ -561,6 +575,9 @@ export class OpenCodeServerBackend implements AgentBackend { const onAbort = () => { // Abort the turn only — never the server. client.session.abort({ sessionID: oc, directory: ctx.worktreePath }).catch(() => {}); + // F.1: opencode emits one trailing session.idle/error for the cancelled + // turn — arm the guard so it's swallowed, not used to settle the next turn. + armAbortGuard(session); settle({ ok: false, error: 'aborted' }); }; diff --git a/apps/coder/src/services/backends/turn-guard.ts b/apps/coder/src/services/backends/turn-guard.ts new file mode 100644 index 0000000..3bf469e --- /dev/null +++ b/apps/coder/src/services/backends/turn-guard.ts @@ -0,0 +1,38 @@ +/** + * Guard against opencode's post-abort "orphan" terminal event (F.1). + * + * When a turn is aborted (`client.session.abort`), opencode emits one trailing + * `session.idle` / `session.error` for the cancelled turn. Without a guard that + * orphan settles whatever turn currently holds the session slot — which, after + * the user immediately sends another message, is the NEXT turn, settling it early + * as success (the v2.6.5 Stop-button bug). opencode terminal events carry only a + * `sessionID` (no turn id), so we can't match by id; instead we swallow exactly + * one terminal per abort, and self-heal if that orphan never arrives. + */ +export interface AbortTerminalGuard { + /** True between an abort and the orphan terminal event that follows it. */ + swallowNextTerminal: boolean; +} + +/** Arm on abort: the next terminal event for this session is the orphan. */ +export function armAbortGuard(g: AbortTerminalGuard): void { + g.swallowNextTerminal = true; +} + +/** + * A new turn produced activity (delta) → the orphan window is over. Self-heals + * the case where opencode emits no orphan idle (e.g. abort-before-prompt), so a + * real terminal still settles instead of being swallowed forever. + */ +export function noteTurnActivity(g: AbortTerminalGuard): void { + g.swallowNextTerminal = false; +} + +/** Decide a terminal (idle/error): swallow the post-abort orphan once, else settle. */ +export function consumeTerminal(g: AbortTerminalGuard): 'swallow' | 'settle' { + if (g.swallowNextTerminal) { + g.swallowNextTerminal = false; + return 'swallow'; + } + return 'settle'; +}