fix(agent-session): never open a sibling terminal on an unproven create (#18735)

An `agentSession.create` the host could not confirm — it committed the session but
could not publish its tab, and answered `agent_session_operation_unknown` — was
rejected with a bare `Error` carrying a `code`. Nothing in the type said "unknown",
so the verdict lived only in the code string, and the shared transport matcher was
still free to re-read that error's *message*: an unknown refusal whose text ends in
a definitive token (`Owner check failed: method_not_found`) classified as definitive,
which is exactly the answer that permits a legacy sibling terminal.

Make the class the verdict. `StructuredAgentSessionCreateUnknownOutcomeError` is a
sibling of `StructuredAgentSessionCreateRefusalError`, not a subclass, so the nine
existing `instanceof` consumers keep reading "refusal" as "you may fall back" with
zero edits, and an unknown outcome flows down the lost-reply path instead —
replaying the same envelope, re-publishing the tab the host failed to publish, and
parking as visibility-unknown rather than creating anything. Classification now
short-circuits on our own classes, so a message we wrote can never invert the
verdict we already reached.

Adds an end-to-end guard that drives the real classifier through
`startStructuredAgentLaunch`: an unknown outcome opens zero legacy terminals, a
definitive refusal opens exactly one. Ablating the branch turns that green suite red
with `['legacy-terminal']` — the duplicate session the guard exists to prevent.

Co-authored-by: Merge Sim <sim@local>

src/renderer/src/lib/launch-structured-agent-session.test.ts

@@ -5,7 +5,8 @@ import {
   createStructuredAgentSessionLaunchIntent,
   isDefinitiveStructuredAgentSessionCreateError,
   launchStructuredAgentSession,
-  StructuredAgentSessionCreateRefusalError
+  StructuredAgentSessionCreateRefusalError,
+  StructuredAgentSessionCreateUnknownOutcomeError
 } from './launch-structured-agent-session'
 
 vi.mock('@/runtime/structured-agent-session-client', () => ({
@@ -256,11 +257,31 @@ describe('structured agent session launch', () => {
       createStructuredAgentSessionLaunchIntent('workspace-unknown', 'codex')
     ).catch((caught: unknown) => caught)
 
+    expect(error).toBeInstanceOf(StructuredAgentSessionCreateUnknownOutcomeError)
     expect(error).not.toBeInstanceOf(StructuredAgentSessionCreateRefusalError)
     expect(error).toMatchObject({ code: 'agent_session_operation_unknown' })
     expect(isDefinitiveStructuredAgentSessionCreateError(error)).toBe(false)
   })
 
+  /** The class is the verdict, so a refusal message that happens to end in a definitive token
+   *  must not be re-read into one by the transport-error matcher. */
+  it('keeps an unknown outcome unknown even when its message ends in a definitive token', async () => {
+    vi.mocked(callStructuredAgentSession).mockResolvedValue({
+      ok: false,
+      refusal: {
+        code: 'agent_session_ownership_unknown',
+        message: 'Owner check failed: method_not_found'
+      }
+    })
+
+    const error = await launchStructuredAgentSession(
+      createStructuredAgentSessionLaunchIntent('workspace-unknown-token', 'codex')
+    ).catch((caught: unknown) => caught)
+
+    expect(error).toBeInstanceOf(StructuredAgentSessionCreateUnknownOutcomeError)
+    expect(isDefinitiveStructuredAgentSessionCreateError(error)).toBe(false)
+  })
+
   it('preserves a definitive refusal code for the fallback path', async () => {
     vi.mocked(callStructuredAgentSession).mockResolvedValue({
       ok: false,

src/renderer/src/lib/launch-structured-agent-session.ts

@@ -33,24 +33,52 @@ export type StructuredAgentSessionLaunchIntent = {
   params: StructuredAgentSessionCreateParams
 }
 
-export class StructuredAgentSessionCreateRefusalError extends Error {
+class StructuredAgentSessionCreateError extends Error {
   constructor(
     message: string,
-    readonly code: string = 'structured_agent_session_unsupported'
+    /** The wire refusal code, or the RPC error code when the create never reached a handler. */
+    readonly code: string
   ) {
     super(message)
+  }
+}
+
+/**
+ * The host proved it created nothing, so a caller may open a legacy terminal instead. The class
+ * itself is the verdict: `launchStructuredAgentSession` is the only place that decides it, against
+ * the shared allowlist, so no consumer has to remember to re-check a code.
+ */
+export class StructuredAgentSessionCreateRefusalError extends StructuredAgentSessionCreateError {
+  constructor(message: string, code: string = 'structured_agent_session_unsupported') {
+    super(message, code)
     this.name = 'StructuredAgentSessionCreateRefusalError'
   }
 }
 
+/**
+ * Refused with a code that does not prove the session is absent. A sibling opened here would sit
+ * beside a session the host may already hold, so this deliberately is NOT a refusal error: it flows
+ * down the same path as a lost reply, which replays the intent and reconciles.
+ */
+export class StructuredAgentSessionCreateUnknownOutcomeError extends StructuredAgentSessionCreateError {
+  constructor(message: string, code: string) {
+    super(message, code)
+    this.name = 'StructuredAgentSessionCreateUnknownOutcomeError'
+  }
+}
+
 const DEFINITIVE_CREATE_FAILURE_CODES = [
   'structured_agent_session_unsupported',
   'method_not_found'
 ] as const
 
 function definitiveStructuredAgentSessionCreateErrorCode(error: unknown): string | null {
-  if (error instanceof StructuredAgentSessionCreateRefusalError) {
-    return isDefinitiveAgentSessionCreateRefusal(error.code) ? error.code : null
+  if (error instanceof StructuredAgentSessionCreateError) {
+    // Our own classes already carry the verdict; message sniffing below could only invert it.
+    return error instanceof StructuredAgentSessionCreateRefusalError &&
+      isDefinitiveAgentSessionCreateRefusal(error.code)
+      ? error.code
+      : null
   }
   for (const code of DEFINITIVE_CREATE_FAILURE_CODES) {
     if (hasRuntimeRpcErrorCode(error, code)) {
@@ -200,15 +228,13 @@ export async function launchStructuredAgentSession(
     throw error
   }
   if (!result.ok) {
-    const error = new StructuredAgentSessionCreateRefusalError(
-      result.refusal.message,
-      result.refusal.code
-    )
-    if (isDefinitiveStructuredAgentSessionCreateError(error)) {
-      abandonStructuredAgentSessionLaunchIntent(intent)
-      throw error
+    const { code, message } = result.refusal
+    if (!isDefinitiveAgentSessionCreateRefusal(code)) {
+      // Keep the focus intent: the session may exist, and recovery still has to adopt it.
+      throw new StructuredAgentSessionCreateUnknownOutcomeError(message, code)
     }
-    throw Object.assign(new Error(error.message), { code: error.code })
+    abandonStructuredAgentSessionLaunchIntent(intent)
+    throw new StructuredAgentSessionCreateRefusalError(message, code)
   }
   return { sessionId: result.value.sessionId, fence: result.value.fence }
 }

src/renderer/src/lib/structured-agent-session-launch-refusal-fallback.test.ts

@@ -0,0 +1,208 @@
+// @vitest-environment happy-dom
+
+// The duplicate-session guard: which create refusals may open a legacy terminal beside the chat.
+// Deliberately exercises the real `launch-structured-agent-session`, because the classification
+// under test lives there — mocking it out would assert nothing.
+
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+import { toast } from 'sonner'
+import type { RuntimeMobileSessionTabsResult } from '../../../shared/runtime-session-contracts'
+import { RuntimeRpcCallError } from '@/runtime/runtime-rpc-client'
+
+const mocks = vi.hoisted(() => ({
+  call: vi.fn(),
+  refresh: vi.fn()
+}))
+
+vi.mock('sonner', () => ({
+  toast: { error: vi.fn(), message: vi.fn() }
+}))
+
+vi.mock('@/i18n/i18n', () => ({
+  translate: (_key: string, fallback: string, options?: { value0?: string }) =>
+    fallback.replace('{{value0}}', options?.value0 ?? '')
+}))
+
+vi.mock('@/lib/agent-catalog', () => ({
+  getAgentCatalog: () => [{ id: 'codex', label: 'Codex' }]
+}))
+
+vi.mock('@/runtime/structured-agent-session-client', () => ({
+  callStructuredAgentSession: mocks.call
+}))
+
+vi.mock('@/runtime/local-structured-session-tabs-sync', () => ({
+  LOCAL_STRUCTURED_SESSION_OWNER: 'local',
+  refreshLocalStructuredSessionTabs: mocks.refresh
+}))
+
+vi.mock('@/store', () => ({
+  useAppStore: {
+    getState: () => ({ unifiedTabsByWorktree: {} }),
+    subscribe: () => () => {}
+  }
+}))
+
+import {
+  StructuredAgentSessionCreateRefusalError,
+  StructuredAgentSessionCreateUnknownOutcomeError
+} from '@/lib/launch-structured-agent-session'
+import {
+  getStructuredAgentLaunchStatus,
+  startStructuredAgentLaunch
+} from './structured-agent-session-launch'
+
+type CreateReply = { ok: boolean; refusal?: { code: string; message: string } }
+
+/** Replies to every `agentSession.create` in turn, repeating the last reply thereafter. */
+function replyToCreates(...replies: CreateReply[]): void {
+  let index = 0
+  mocks.call.mockImplementation(async (_target: unknown, method: string, params: unknown) => {
+    if (method !== 'agentSession.create') {
+      return { ok: true, page: { fence: 1 } }
+    }
+    const reply = replies[Math.min(index, replies.length - 1)]
+    index += 1
+    if (!reply.ok) {
+      return reply
+    }
+    const sessionId = (params as { envelope: { sessionId: string } }).envelope.sessionId
+    return { ok: true, replayed: index > 1, fence: 1, value: { sessionId, fence: 1 } }
+  })
+}
+
+function refused(code: string): CreateReply {
+  return { ok: false, refusal: { code, message: `create refused: ${code}` } }
+}
+
+function publishedSnapshot(worktreeId: string, sessionId: string): RuntimeMobileSessionTabsResult {
+  return {
+    worktree: worktreeId,
+    publicationEpoch: 'epoch-1',
+    snapshotVersion: 1,
+    activeGroupId: null,
+    activeTabId: null,
+    activeTabType: null,
+    tabs: [
+      {
+        type: 'agent-session',
+        id: 'tab-1',
+        title: 'Codex',
+        sessionId,
+        agent: 'codex',
+        isActive: true
+      }
+    ]
+  }
+}
+
+async function flushLaunchSettlement(): Promise<void> {
+  for (let i = 0; i < 20; i += 1) {
+    await Promise.resolve()
+  }
+}
+
+describe('legacy terminal fallback after a refused structured create', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+    localStorage.clear()
+    mocks.refresh.mockResolvedValue([])
+  })
+
+  it.each(['agent_session_operation_unknown', 'agent_session_ownership_unknown'])(
+    'opens no sibling terminal when the host answers %s',
+    async (code) => {
+      const worktreeId = `wt-${code}`
+      const legacyTerminals: string[] = []
+      replyToCreates(refused(code))
+
+      const launch = startStructuredAgentLaunch(worktreeId, 'codex')
+      void launch.claimDefinitiveRefusalFallback(() => {
+        legacyTerminals.push('legacy-terminal')
+      })
+
+      await expect(launch.launchResult).rejects.toBeInstanceOf(
+        StructuredAgentSessionCreateUnknownOutcomeError
+      )
+      await flushLaunchSettlement()
+
+      // The host may already hold the session, so the user keeps exactly one thing: no chat it
+      // could confirm, and no terminal beside a session it could not rule out.
+      expect(legacyTerminals).toEqual([])
+      expect(launch.isVisibilityUnknown()).toBe(true)
+      expect(toast.error).toHaveBeenCalledOnce()
+    }
+  )
+
+  it('adopts the session an unknown outcome had already created, without a sibling', async () => {
+    const worktreeId = 'wt-unknown-then-published'
+    const legacyTerminals: string[] = []
+    replyToCreates(refused('agent_session_operation_unknown'), { ok: true })
+
+    const launch = startStructuredAgentLaunch(worktreeId, 'codex')
+    const fallbackRan = launch.claimDefinitiveRefusalFallback(() => {
+      legacyTerminals.push('legacy-terminal')
+    })
+    mocks.refresh
+      .mockResolvedValueOnce([])
+      .mockResolvedValue([publishedSnapshot(worktreeId, launch.sessionId)])
+
+    await expect(launch.launchResult).resolves.toEqual({
+      sessionId: launch.sessionId,
+      fence: 1
+    })
+    await expect(fallbackRan).resolves.toBe(false)
+    await flushLaunchSettlement()
+
+    expect(legacyTerminals).toEqual([])
+    expect(toast.error).not.toHaveBeenCalled()
+  })
+
+  it('opens exactly one legacy terminal when the refusal is on the definitive allowlist', async () => {
+    const worktreeId = 'wt-unsupported'
+    const legacyTerminals: string[] = []
+    replyToCreates(refused('structured_agent_session_unsupported'))
+
+    const launch = startStructuredAgentLaunch(worktreeId, 'codex')
+    const fallbackRan = launch.claimDefinitiveRefusalFallback(() => {
+      legacyTerminals.push('legacy-terminal')
+    })
+
+    await expect(launch.launchResult).rejects.toBeInstanceOf(
+      StructuredAgentSessionCreateRefusalError
+    )
+    await expect(fallbackRan).resolves.toBe(true)
+    await flushLaunchSettlement()
+
+    expect(legacyTerminals).toEqual(['legacy-terminal'])
+    // A proven "nothing was created" needs no replay, so the terminal is the only surface open.
+    expect(
+      mocks.call.mock.calls.filter(([, method]) => method === 'agentSession.create')
+    ).toHaveLength(1)
+    expect(launch.isVisibilityUnknown()).toBe(false)
+  })
+
+  it('opens exactly one legacy terminal when an older runtime has no create method', async () => {
+    const legacyTerminals: string[] = []
+    mocks.call.mockRejectedValue(
+      new RuntimeRpcCallError({
+        id: 'rpc-old-runtime',
+        ok: false,
+        error: { code: 'method_not_found', message: 'Unknown method: agentSession.create' }
+      })
+    )
+
+    const launch = startStructuredAgentLaunch('wt-old-runtime', 'codex')
+    const fallbackRan = launch.claimDefinitiveRefusalFallback(() => {
+      legacyTerminals.push('legacy-terminal')
+    })
+
+    await expect(launch.launchResult).rejects.toBeInstanceOf(
+      StructuredAgentSessionCreateRefusalError
+    )
+    await expect(fallbackRan).resolves.toBe(true)
+    expect(legacyTerminals).toEqual(['legacy-terminal'])
+    expect(mocks.call).toHaveBeenCalledOnce()
+    expect(getStructuredAgentLaunchStatus('wt-old-runtime', 'codex')).toBe('idle')
+  })
+})