diff --git a/server/index.js b/server/index.js index 0766706..a00a8bf 100755 --- a/server/index.js +++ b/server/index.js @@ -63,6 +63,11 @@ import { spawnOmp, abortOmpSession, } from './omp-cli.js'; +import { + spawnOmo, + abortOmoSession, +} from './omo-cli.js'; +import { findLiveTmuxSpawnBlock } from './modules/providers/services/live-spawn-guard.service.js'; import { stripAnsiSequences, normalizeDetectedUrl, @@ -170,14 +175,11 @@ const wss = createWebSocketServer(server, { opencode: spawnOpenCode, gjc: spawnGjc, omp: spawnOmp, - // omo is intentionally NOT registered. Its transcripts are indexed - // like any other session, so a session that is currently live in a - // tmux pane can also be opened here; spawning would start a second - // headless omo on the same --session-id and both processes would - // append to one transcript. Observed: a full turn landed in a live - // session that the running agent never saw. Live sessions must go - // through the tmux relay. Re-enable only behind a guard that - // refuses to spawn when the session is live. + // Safe to register because findLiveTmuxSpawnBlock below refuses to + // spawn on a session that is currently live in a tmux pane (#44): + // a second headless omo on the same --session-id would otherwise + // append to one transcript the live agent never sees. + omo: spawnOmo, }, abortFns: { claude: abortClaudeSDKSession, @@ -186,9 +188,13 @@ const wss = createWebSocketServer(server, { opencode: abortOpenCodeSession, gjc: abortGjcSession, omp: abortOmpSession, + omo: abortOmoSession, }, resolveToolApproval: resolveProviderToolApproval, getPendingApprovalsForSession: getPendingProviderApprovalsForSession, + // #44 guard: chat.send refuses to fork a session that a live tmux pane + // owns, for every provider that resumes by provider-native session id. + findLiveTmuxSpawnBlock, }, shell: { resolveProviderSessionId: (sessionId, provider) => { diff --git a/server/modules/providers/services/live-spawn-guard.service.ts b/server/modules/providers/services/live-spawn-guard.service.ts new file mode 100644 index 0000000..1ae0f6f --- /dev/null +++ b/server/modules/providers/services/live-spawn-guard.service.ts @@ -0,0 +1,57 @@ +import { + getExternalCliSessionsDetailedFresh, + type ExternalCliSession, + type ExternalLocalCliKind, +} from '@/modules/providers/services/external-cli-sessions.service.js'; +import type { LLMProvider } from '@/shared/types.js'; + +/** + * Providers whose transcripts are indexed as ordinary sessions even while a + * CLI is attached to them in a tmux pane. For these, `chat.send` spawning a + * headless resume would put a SECOND writer on the same transcript: the user + * sees a reply, the live agent never sees the message, and the JSONL + * interleaves two processes (#44). + * + * gjc is deliberately absent: its live sessions are reached through the SDK + * connect lane (`connectGjcSdkSession`), which attaches to the running agent + * instead of forking a new one, so the duplicate-writer failure cannot occur. + */ +const GUARDED_PROVIDERS: ReadonlySet = new Set([ + 'claude', 'codex', 'cursor', 'opencode', 'omp', 'omo', +]); + +export type LiveTmuxSpawnBlock = { tmuxName: string }; + +/** Pure matcher, exported for tests. */ +export function findLiveTmuxPaneForSession( + provider: LLMProvider | string, + providerSessionId: string, + sessions: readonly ExternalCliSession[], +): LiveTmuxSpawnBlock | null { + if (!GUARDED_PROVIDERS.has(provider)) return null; + const owner = sessions.find((session) => ( + session.kind === provider && session.providerSessionId === providerSessionId + )); + return owner ? { tmuxName: owner.tmuxName } : null; +} + +/** + * Returns the live tmux owner of a provider-native session id, or null when + * spawning is safe. Fail-open on unavailable or failed discovery evidence: + * a false block would break the core chat path outright, while a false allow + * merely restores the pre-guard behavior — and when tmux is not running at + * all, no pane can own the transcript anyway. + */ +export async function findLiveTmuxSpawnBlock( + provider: LLMProvider | string, + providerSessionId: string | null | undefined, +): Promise { + if (!providerSessionId || !GUARDED_PROVIDERS.has(provider)) return null; + try { + const detailed = await getExternalCliSessionsDetailedFresh(); + if (!detailed.ok) return null; + return findLiveTmuxPaneForSession(provider, providerSessionId, detailed.sessions); + } catch { + return null; + } +} diff --git a/server/modules/providers/services/provider-capabilities.service.ts b/server/modules/providers/services/provider-capabilities.service.ts index 5992bcc..17a6d3e 100644 --- a/server/modules/providers/services/provider-capabilities.service.ts +++ b/server/modules/providers/services/provider-capabilities.service.ts @@ -113,10 +113,10 @@ const PROVIDER_CAPABILITIES: Record = { provider: 'omo', permissionModes: ['default'], defaultPermissionMode: 'default', - // omo is discovery, transcript, and tmux relay only. `server/omo-cli.ts` - // implements the send runtime, but it is deliberately not registered in - // `spawnFns` (see the comment there): spawning would put a second headless - // omo on a session that may already be live in a tmux pane. + // The omo send runtime is registered behind the live-pane spawn guard + // (#44, `live-spawn-guard.service.ts`); like Oh My Pi, the runtime also + // forwards @ images and SIGTERM aborts, but this matrix mirrors the + // omp posture and keeps both UI controls hidden until verified end to end. supportsImages: false, supportsAbort: false, // omo's TUI renders "↑↓ navigate • enter select • esc close" and has no diff --git a/server/modules/providers/tests/live-spawn-guard.test.ts b/server/modules/providers/tests/live-spawn-guard.test.ts new file mode 100644 index 0000000..d656f3e --- /dev/null +++ b/server/modules/providers/tests/live-spawn-guard.test.ts @@ -0,0 +1,60 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; + +import type { ExternalCliSession } from '@/modules/providers/services/external-cli-sessions.service.js'; +import { findLiveTmuxPaneForSession } from '@/modules/providers/services/live-spawn-guard.service.js'; + +const tmux = { + socketPath: '/tmp/tmux-1000/default', + sessionId: '$1', + windowId: '@1', + paneId: '%1', +}; + +function session(overrides: Partial): ExternalCliSession { + return { tmuxName: 'work', tmux, kind: 'omo', ...overrides }; +} + +// #44 regression lock: a chat.send to a session whose transcript is owned by a +// live tmux pane must be refused — a second headless resume would interleave +// two writers in one JSONL and the live agent would never see the message. +test('a live pane owning the exact provider session blocks the spawn', () => { + const sessions = [ + session({ kind: 'omo', providerSessionId: 'S-1', tmuxName: 'omo-pane' }), + session({ kind: 'omp', providerSessionId: 'S-2', tmuxName: 'omp-pane' }), + ]; + + assert.deepEqual(findLiveTmuxPaneForSession('omo', 'S-1', sessions), { tmuxName: 'omo-pane' }); + assert.deepEqual(findLiveTmuxPaneForSession('omp', 'S-2', sessions), { tmuxName: 'omp-pane' }); +}); + +test('a different provider, different id, or unresolved pane never blocks', () => { + const sessions = [ + session({ kind: 'omo', providerSessionId: 'S-1' }), + // A pane whose native id inference has not resolved yet cannot claim ownership. + session({ kind: 'omo', providerSessionId: undefined }), + ]; + + // Same id under a different provider is a different session space. + assert.equal(findLiveTmuxPaneForSession('omp', 'S-1', sessions), null); + assert.equal(findLiveTmuxPaneForSession('omo', 'S-other', sessions), null); + assert.equal(findLiveTmuxPaneForSession('omo', 'S-1', []), null); +}); + +test('every headless-resume provider is guarded; gjc and non-CLI lanes are not', () => { + for (const kind of ['claude', 'codex', 'cursor', 'opencode', 'omp', 'omo'] as const) { + const sessions = [session({ kind, providerSessionId: 'S-1', tmuxName: `${kind}-pane` })]; + assert.deepEqual( + findLiveTmuxPaneForSession(kind, 'S-1', sessions), + { tmuxName: `${kind}-pane` }, + kind, + ); + } + + // gjc reaches live sessions through the SDK connect lane, not a fork — + // its rows must never be blocked by this guard. + assert.equal( + findLiveTmuxPaneForSession('gjc', 'S-1', [session({ kind: 'omo', providerSessionId: 'S-1' })]), + null, + ); +}); diff --git a/server/modules/websocket/services/chat-websocket.service.ts b/server/modules/websocket/services/chat-websocket.service.ts index f164f5a..955bf17 100644 --- a/server/modules/websocket/services/chat-websocket.service.ts +++ b/server/modules/websocket/services/chat-websocket.service.ts @@ -78,6 +78,17 @@ type ChatWebSocketDependencies = { ) => void; /** Provider-runtime approvals included in `chat_subscribed` after reconnect. */ getPendingApprovalsForSession: (providerSessionId: string) => unknown[]; + /** + * Live-pane spawn guard (#44): resolves the tmux session that currently + * owns a provider-native session id, or null when spawning is safe. When a + * transcript is live in a tmux pane, spawning a headless resume would put a + * second writer on the same JSONL and the live agent would never see the + * message, so `chat.send` must refuse instead. + */ + findLiveTmuxSpawnBlock?: ( + provider: LLMProvider, + providerSessionId: string | null | undefined, + ) => Promise<{ tmuxName: string } | null>; /** Optional non-chat protocol mounted on the authenticated /ws gateway. */ handleDiscovery?: (ws: WebSocket, data: AnyRecord) => boolean; }; @@ -173,6 +184,23 @@ async function handleChatSend( return; } + if (session.provider_session_id && dependencies.findLiveTmuxSpawnBlock) { + const liveOwner = await dependencies.findLiveTmuxSpawnBlock( + provider, + session.provider_session_id, + ); + if (liveOwner) { + sendProtocolError( + ws, + 'SESSION_LIVE_IN_TMUX', + `This session is currently live in tmux session "${liveOwner.tmuxName}". ` + + 'Send input through the live session view instead of starting a second process.', + sessionId, + ); + return; + } + } + const run = chatRunRegistry.startRun({ appSessionId: sessionId, provider, @@ -215,7 +243,7 @@ async function handleChatSend( try { const providerRun = spawnFn(command, runtimeOptions, run.writer); - if (provider === 'gjc' || provider === 'omp') { + if (provider === 'gjc' || provider === 'omp' || provider === 'omo') { const abortHandle = (providerRun as ProviderSpawnResult).abortHandle; if (abortHandle) { run.writer.setAbortHandle(abortHandle); @@ -263,7 +291,7 @@ async function handleChatAbort( if (abortFn && abortSessionId) { success = Boolean(await abortFn(abortSessionId)); } - if (!success && (run.provider === 'gjc' || run.provider === 'omp')) { + if (!success && (run.provider === 'gjc' || run.provider === 'omp' || run.provider === 'omo')) { sendProtocolError(ws, 'ABORT_FAILED', `Session "${sessionId}" could not be aborted.`, sessionId); return; }