From f4640b4b2ee72ddd956c512afd40dafddf8b5337 Mon Sep 17 00:00:00 2001 From: Merge Sim Date: Tue, 8 Sep 2026 19:09:03 -0700 Subject: [PATCH] refactor(native-chat): route the new-tab launcher through the shared settle loop The new-tab launcher fired its refusal fallback and forgot it: nobody learned whether the terminal fallback ran, and a visibility-unknown outcome was never surfaced. Its structured branch now runs through settleStructuredAgentLaunch with the terminal launch as the legacy fallback. launchAgentInNewTab stays synchronous; the result gains a structuredSettlement promise, and promptDeliveryResult keeps following the terminal fallback's delivery on refusal as it did through the callers bridge before. --- .../folder-workspace-composer-submit.ts | 3 +- .../composer-state/full-creation-execution.ts | 6 +- ...launch-agent-in-new-tab-structured.test.ts | 221 ++++++++++++++++++ .../lib/launch-agent-in-new-tab-structured.ts | 85 +++++++ .../src/lib/launch-agent-in-new-tab.ts | 29 ++- ...launch-agent-structured-chat-guard.test.ts | 66 +++++- ...structured-agent-launch-settlement.test.ts | 18 ++ .../lib/structured-agent-launch-settlement.ts | 3 +- 8 files changed, 409 insertions(+), 22 deletions(-) create mode 100644 src/renderer/src/lib/launch-agent-in-new-tab-structured.test.ts create mode 100644 src/renderer/src/lib/launch-agent-in-new-tab-structured.ts diff --git a/src/renderer/src/components/sidebar/folder-workspace-composer-submit.ts b/src/renderer/src/components/sidebar/folder-workspace-composer-submit.ts index f612f8071b9..c9e0b405dca 100644 --- a/src/renderer/src/components/sidebar/folder-workspace-composer-submit.ts +++ b/src/renderer/src/components/sidebar/folder-workspace-composer-submit.ts @@ -256,7 +256,8 @@ export async function submitFolderWorkspaceCreate({ } if (settlement.kind === 'refused-then-legacy') { structuredLaunchAccepted = false - activation = settlement.activation + // Why: this flow's own fallback always activates; `??` only satisfies the shared type. + activation = settlement.activation ?? false } } if ( diff --git a/src/renderer/src/hooks/composer-state/full-creation-execution.ts b/src/renderer/src/hooks/composer-state/full-creation-execution.ts index 974bc95cc06..b7d3877f339 100644 --- a/src/renderer/src/hooks/composer-state/full-creation-execution.ts +++ b/src/renderer/src/hooks/composer-state/full-creation-execution.ts @@ -246,8 +246,12 @@ export function useFullCreationExecution(input: FullCreationExecutionInput) { return } const structuredLaunchAccepted = settlement?.kind === 'structured' + // Why: the workspace was already activated before launch; the fallback's activation, when + // present, supersedes it. const activation = - settlement?.kind === 'refused-then-legacy' ? settlement.activation : initialActivation + settlement?.kind === 'refused-then-legacy' + ? (settlement.activation ?? initialActivation) + : initialActivation if (!structuredLaunchAccepted && startupPlan) { const optionScopeKey = diff --git a/src/renderer/src/lib/launch-agent-in-new-tab-structured.test.ts b/src/renderer/src/lib/launch-agent-in-new-tab-structured.test.ts new file mode 100644 index 00000000000..fe0c18ce03b --- /dev/null +++ b/src/renderer/src/lib/launch-agent-in-new-tab-structured.test.ts @@ -0,0 +1,221 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { + StructuredAgentLaunchHooks, + StructuredAgentLaunchSettlement +} from './structured-agent-launch-settlement' + +const mocks = vi.hoisted(() => ({ + settleStructuredAgentLaunch: vi.fn() +})) + +vi.mock('@/lib/structured-agent-launch-settlement', () => ({ + settleStructuredAgentLaunch: mocks.settleStructuredAgentLaunch +})) + +import { launchAgentInStructuredNewTab } from './launch-agent-in-new-tab-structured' + +const delivered = { delivered: true, failureNotified: false } +const undelivered = { delivered: false, failureNotified: true } + +/** Mirrors the shared loop: a refusal runs the caller's fallback once and settles with its result. */ +function settleWith(settlement: StructuredAgentLaunchSettlement | 'refusal') { + mocks.settleStructuredAgentLaunch.mockImplementation( + async ( + _worktreeId: string, + _agent: string, + _options: unknown, + hooks: StructuredAgentLaunchHooks + ) => { + if (settlement !== 'refusal') { + return settlement + } + const fallback = await hooks.legacyFallback?.() + return fallback + ? { kind: 'refused-then-legacy', ...fallback } + : { kind: 'failed', error: null } + } + ) +} + +describe('launchAgentInStructuredNewTab', () => { + let consoleError: ReturnType + + beforeEach(() => { + vi.clearAllMocks() + consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}) + }) + + afterEach(() => { + consoleError.mockRestore() + }) + + it('hands the launch to the shared settle loop and follows the structured delivery', async () => { + const promptDeliveryResult = Promise.resolve(delivered) + settleWith({ kind: 'structured', sessionId: 'session-1', promptDeliveryResult }) + const legacyLaunch = vi.fn() + const onPromptDelivered = vi.fn() + + const result = launchAgentInStructuredNewTab({ + worktreeId: 'wt-1', + agent: 'codex', + prompt: 'Fix it', + promptDelivery: 'submit-after-ready', + onPromptDelivered, + legacyLaunch + }) + + expect(mocks.settleStructuredAgentLaunch).toHaveBeenCalledWith( + 'wt-1', + 'codex', + { prompt: 'Fix it', promptDelivery: 'submit-after-ready', onPromptDelivered }, + expect.objectContaining({ legacyFallback: expect.any(Function) }) + ) + await expect(result.structuredSettlement).resolves.toEqual({ + kind: 'structured', + sessionId: 'session-1', + promptDeliveryResult + }) + await expect(result.promptDeliveryResult).resolves.toEqual(delivered) + expect(legacyLaunch).not.toHaveBeenCalled() + expect(consoleError).not.toHaveBeenCalled() + }) + + it('runs the terminal launch exactly once on refusal and reports its delivery', async () => { + settleWith('refusal') + const legacyDelivery = Promise.resolve(delivered) + const legacyLaunch = vi.fn(() => ({ + tabId: 'tab-1', + startupPlan: {} as never, + pasteDraftAfterLaunch: true, + promptDeliveryResult: legacyDelivery + })) + + const result = launchAgentInStructuredNewTab({ + worktreeId: 'wt-1', + agent: 'codex', + prompt: 'Fix it', + promptDelivery: 'submit-after-ready', + legacyLaunch + }) + + await expect(result.structuredSettlement).resolves.toEqual({ + kind: 'refused-then-legacy', + primaryTabId: 'tab-1', + promptDeliveryResult: legacyDelivery + }) + await expect(result.promptDeliveryResult).resolves.toBe(delivered) + expect(legacyLaunch).toHaveBeenCalledOnce() + }) + + it('counts an argv-carried prompt as delivered when the terminal launch returns no promise', async () => { + settleWith('refusal') + const legacyLaunch = vi.fn(() => ({ + tabId: 'tab-1', + startupPlan: {} as never, + pasteDraftAfterLaunch: false + })) + + const result = launchAgentInStructuredNewTab({ + worktreeId: 'wt-1', + agent: 'codex', + prompt: 'Fix it', + promptDelivery: 'auto-submit', + legacyLaunch + }) + + await expect(result.promptDeliveryResult).resolves.toEqual(delivered) + await expect(result.structuredSettlement).resolves.toMatchObject({ primaryTabId: 'tab-1' }) + }) + + it('reports a notified failure when the terminal launch has no startup plan', async () => { + settleWith('refusal') + + const result = launchAgentInStructuredNewTab({ + worktreeId: 'wt-1', + agent: 'codex', + prompt: 'Fix it', + promptDelivery: 'auto-submit', + legacyLaunch: () => null + }) + + await expect(result.promptDeliveryResult).resolves.toEqual(undelivered) + await expect(result.structuredSettlement).resolves.toMatchObject({ + kind: 'refused-then-legacy', + primaryTabId: null + }) + }) + + it('logs a failed settlement without re-entering the terminal launch', async () => { + const error = new Error('boom') + settleWith({ kind: 'failed', error }) + const legacyLaunch = vi.fn() + + const result = launchAgentInStructuredNewTab({ + worktreeId: 'wt-1', + agent: 'codex', + prompt: 'Fix it', + promptDelivery: 'submit-after-ready', + legacyLaunch + }) + + await expect(result.structuredSettlement).resolves.toEqual({ kind: 'failed', error }) + await expect(result.promptDeliveryResult).resolves.toEqual(undelivered) + expect(consoleError).toHaveBeenCalledWith('Structured agent launch failed', error) + expect(legacyLaunch).not.toHaveBeenCalled() + }) + + it('treats a thrown settle loop as a failed settlement', async () => { + const error = new Error('intent unavailable') + mocks.settleStructuredAgentLaunch.mockRejectedValue(error) + + const result = launchAgentInStructuredNewTab({ + worktreeId: 'wt-1', + agent: 'codex', + prompt: 'Fix it', + promptDelivery: 'submit-after-ready', + legacyLaunch: vi.fn() + }) + + await expect(result.structuredSettlement).resolves.toEqual({ kind: 'failed', error }) + await expect(result.promptDeliveryResult).resolves.toEqual(undelivered) + expect(consoleError).toHaveBeenCalledWith('Structured agent launch failed', error) + }) + + it('surfaces an unknown outcome silently and never falls back', async () => { + settleWith({ kind: 'visibility-unknown', sessionId: 'session-1' }) + const legacyLaunch = vi.fn() + + const result = launchAgentInStructuredNewTab({ + worktreeId: 'wt-1', + agent: 'codex', + prompt: 'Fix it', + promptDelivery: 'submit-after-ready', + legacyLaunch + }) + + await expect(result.structuredSettlement).resolves.toEqual({ + kind: 'visibility-unknown', + sessionId: 'session-1' + }) + await expect(result.promptDeliveryResult).resolves.toEqual(undelivered) + expect(consoleError).not.toHaveBeenCalled() + expect(legacyLaunch).not.toHaveBeenCalled() + }) + + it.each([ + ['no prompt', { prompt: '', promptDelivery: 'auto-submit' as const }], + ['a draft prompt', { prompt: 'Fix it', promptDelivery: 'draft' as const }] + ])('exposes no delivery promise for %s', async (_label, options) => { + settleWith({ kind: 'structured', sessionId: 'session-1' }) + + const result = launchAgentInStructuredNewTab({ + worktreeId: 'wt-1', + agent: 'codex', + ...options, + legacyLaunch: vi.fn() + }) + + expect(result.promptDeliveryResult).toBeUndefined() + await expect(result.structuredSettlement).resolves.toMatchObject({ kind: 'structured' }) + }) +}) diff --git a/src/renderer/src/lib/launch-agent-in-new-tab-structured.ts b/src/renderer/src/lib/launch-agent-in-new-tab-structured.ts new file mode 100644 index 00000000000..618048af0a7 --- /dev/null +++ b/src/renderer/src/lib/launch-agent-in-new-tab-structured.ts @@ -0,0 +1,85 @@ +import type { AgentSessionHandleProvider } from '../../../shared/agent-session-provider-handle' +import type { LaunchAgentInNewTabResult } from '@/lib/launch-agent-in-new-tab' +import { + settleStructuredAgentLaunch, + type StructuredAgentLaunchSettlement +} from '@/lib/structured-agent-launch-settlement' +import type { StructuredAgentLaunchOptions } from '@/lib/structured-agent-session-launch' +import type { StructuredPromptDeliveryResult } from '@/lib/structured-agent-session-launch-prompt' + +export type StructuredNewTabLaunchArgs = Pick< + StructuredAgentLaunchOptions, + 'promptDelivery' | 'onPromptDelivered' +> & { + worktreeId: string + agent: AgentSessionHandleProvider + /** Already trimmed; empty means no prompt. */ + prompt: string + /** The terminal-backed launch with the same arguments. Runs at most once, on definitive refusal. */ + legacyLaunch: () => LaunchAgentInNewTabResult +} + +export type StructuredNewTabLaunch = { + structuredSettlement: Promise + promptDeliveryResult?: Promise +} + +const UNDELIVERED: StructuredPromptDeliveryResult = { delivered: false, failureNotified: true } + +function promptDeliveryFromSettlement( + settlement: StructuredAgentLaunchSettlement +): Promise { + if (settlement.kind === 'structured' || settlement.kind === 'refused-then-legacy') { + return settlement.promptDeliveryResult ?? Promise.resolve(UNDELIVERED) + } + return Promise.resolve(UNDELIVERED) +} + +/** + * The new-tab launcher's structured branch. Returns synchronously so `launchAgentInNewTab` keeps + * its signature; the settlement carries what the launch actually did, and `promptDeliveryResult` + * follows it so a refusal reports the terminal fallback's delivery, not the refused structured one. + */ +export function launchAgentInStructuredNewTab( + args: StructuredNewTabLaunchArgs +): StructuredNewTabLaunch { + const hasPrompt = args.prompt.length > 0 + const structuredSettlement = settleStructuredAgentLaunch( + args.worktreeId, + args.agent, + { + prompt: args.prompt, + promptDelivery: args.promptDelivery, + onPromptDelivered: args.onPromptDelivered + }, + { + legacyFallback: async () => { + const fallback = args.legacyLaunch() + // Why: a legacy launch with no delivery promise still delivered an argv-carried or draft + // prompt; only a null launch (no startup plan) is a failure. + const promptDeliveryResult = + fallback?.promptDeliveryResult ?? + (hasPrompt + ? Promise.resolve({ delivered: Boolean(fallback), failureNotified: fallback === null }) + : undefined) + return { + primaryTabId: fallback?.tabId ?? null, + ...(promptDeliveryResult ? { promptDeliveryResult } : {}) + } + } + } + ).catch((error: unknown): StructuredAgentLaunchSettlement => ({ kind: 'failed', error })) + void structuredSettlement.then((settlement) => { + // Why: unknown already shows the launch badge and failed already toasted; this is the log + // line the old fire-and-forget fallback claim kept. + if (settlement.kind === 'failed') { + console.error('Structured agent launch failed', settlement.error) + } + }) + return { + structuredSettlement, + ...(hasPrompt && args.promptDelivery !== 'draft' + ? { promptDeliveryResult: structuredSettlement.then(promptDeliveryFromSettlement) } + : {}) + } +} diff --git a/src/renderer/src/lib/launch-agent-in-new-tab.ts b/src/renderer/src/lib/launch-agent-in-new-tab.ts index fdcabff8a9a..3043edc6692 100644 --- a/src/renderer/src/lib/launch-agent-in-new-tab.ts +++ b/src/renderer/src/lib/launch-agent-in-new-tab.ts @@ -28,7 +28,8 @@ import type { LaunchSource } from '../../../shared/telemetry-events' import { getConnectionIdFromState } from '@/lib/connection-context' import { resolveInitialNativeChatSessionOptions } from '@/components/native-chat/native-chat-launch-session-options' import { seedNativeChatAppliedSessionOptions } from '@/components/native-chat/native-chat-session-option-cache' -import { startStructuredAgentLaunch } from '@/lib/structured-agent-session-launch' +import { launchAgentInStructuredNewTab } from '@/lib/launch-agent-in-new-tab-structured' +import type { StructuredAgentLaunchSettlement } from '@/lib/structured-agent-launch-settlement' import { isAgentSessionHandleProvider } from '../../../shared/agent-session-provider-handle' import { resolveAgentLaunchRouteForWorkspace, @@ -64,6 +65,9 @@ export type LaunchAgentInNewTabResult = { /** The host will publish and focus a structured tab asynchronously. */ focusAfterMenuClose?: 'structured-session' promptDeliveryResult?: Promise<{ delivered: boolean; failureNotified: boolean }> + /** Structured route only: what the launch did once it settled, including whether the terminal + * fallback ran. The call itself stays synchronous. */ + structuredSettlement?: Promise } | null export function shouldQueueTerminalFocusAfterMenuClose( @@ -206,29 +210,22 @@ function launchAgentInNewTabInternal( initialSessionOptions: startupPlan.sessionOptions }) if (launchRoute === 'structured-native-chat' && isAgentSessionHandleProvider(agent)) { - const structuredLaunch = startStructuredAgentLaunch(worktreeId, agent, { + const structured = launchAgentInStructuredNewTab({ + worktreeId, + agent, prompt: trimmedPrompt, promptDelivery: viewModePromptDelivery, - onPromptDelivered + onPromptDelivered, + legacyLaunch: () => launchAgentInNewTabInternal(args, true) }) - void structuredLaunch - .claimDefinitiveRefusalFallback(() => { - const fallback = launchAgentInNewTabInternal(args, true) - return ( - fallback?.promptDeliveryResult ?? - (hasPrompt - ? { delivered: Boolean(fallback), failureNotified: fallback === null } - : undefined) - ) - }) - .catch((error) => console.error('Structured Codex fallback failed', error)) return { tabId: null, startupPlan, pasteDraftAfterLaunch: false, focusAfterMenuClose: 'structured-session', - ...(structuredLaunch.promptDeliveryResult - ? { promptDeliveryResult: structuredLaunch.promptDeliveryResult } + structuredSettlement: structured.structuredSettlement, + ...(structured.promptDeliveryResult + ? { promptDeliveryResult: structured.promptDeliveryResult } : {}) } } diff --git a/src/renderer/src/lib/launch-agent-structured-chat-guard.test.ts b/src/renderer/src/lib/launch-agent-structured-chat-guard.test.ts index 6bd2c646990..2e355a80f32 100644 --- a/src/renderer/src/lib/launch-agent-structured-chat-guard.test.ts +++ b/src/renderer/src/lib/launch-agent-structured-chat-guard.test.ts @@ -175,6 +175,10 @@ describe('structured chat adoption guard on the launch path', () => { focusAfterMenuClose: 'structured-session' }) expect(shouldQueueTerminalFocusAfterMenuClose(result!)).toBe(false) + await expect(result?.structuredSettlement).resolves.toEqual({ + kind: 'structured', + sessionId: 'codex-session-1' + }) expect(mockCreateStructuredCodexSessionLaunchIntent).toHaveBeenCalledWith('wt-1', 'codex') expect(mockLaunchStructuredCodexSession).toHaveBeenCalledWith( expect.objectContaining({ worktreeId: 'wt-1' }) @@ -230,7 +234,11 @@ describe('structured chat adoption guard on the launch path', () => { const result = launchAgentInNewTab({ agent: 'claude', worktreeId: 'wt-1' }) expect(result).toMatchObject({ tabId: null }) - await vi.waitFor(() => expect(mockCreateTab).toHaveBeenCalledOnce()) + await expect(result?.structuredSettlement).resolves.toEqual({ + kind: 'refused-then-legacy', + primaryTabId: 'tab-1' + }) + expect(mockCreateTab).toHaveBeenCalledOnce() expect(mockToastError).not.toHaveBeenCalled() }) @@ -277,10 +285,53 @@ describe('structured chat adoption guard on the launch path', () => { const result = launchAgentInNewTab({ agent: 'codex', worktreeId: 'wt-1' }) expect(result).toMatchObject({ tabId: null, pasteDraftAfterLaunch: false }) - await vi.waitFor(() => expect(mockCreateTab).toHaveBeenCalledOnce()) + await expect(result?.structuredSettlement).resolves.toEqual({ + kind: 'refused-then-legacy', + primaryTabId: 'tab-1' + }) + expect(mockCreateTab).toHaveBeenCalledOnce() + expect(mockCreateTab).toHaveBeenCalledWith( + 'wt-1', + undefined, + undefined, + expect.objectContaining({ launchAgent: 'codex' }) + ) expect(mockToastError).not.toHaveBeenCalled() }) + it('logs a fallback that throws and never re-enters the terminal launch', async () => { + const { StructuredAgentSessionCreateRefusalError } = + await import('./launch-structured-agent-session') + mockLaunchStructuredCodexSession.mockRejectedValueOnce( + new StructuredAgentSessionCreateRefusalError('provider unavailable') + ) + const tabError = new Error('no tab surface') + mockCreateTab.mockImplementationOnce(() => { + throw tabError + }) + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}) + const { launchAgentInNewTab } = await import('./launch-agent-in-new-tab') + + const result = launchAgentInNewTab({ + agent: 'codex', + worktreeId: 'wt-1', + prompt: 'start this task', + promptDelivery: 'submit-after-ready' + }) + + await expect(result?.structuredSettlement).resolves.toEqual({ + kind: 'failed', + error: tabError + }) + await expect(result?.promptDeliveryResult).resolves.toEqual({ + delivered: false, + failureNotified: true + }) + expect(mockCreateTab).toHaveBeenCalledOnce() + expect(consoleError).toHaveBeenCalledWith('Structured agent launch failed', tabError) + consoleError.mockRestore() + }) + it('reports prompt delivery from the definitive-refusal terminal fallback', async () => { const { StructuredAgentSessionCreateRefusalError } = await import('./launch-structured-agent-session') @@ -300,6 +351,10 @@ describe('structured chat adoption guard on the launch path', () => { delivered: true, failureNotified: false }) + await expect(result?.structuredSettlement).resolves.toMatchObject({ + kind: 'refused-then-legacy', + primaryTabId: 'tab-1' + }) expect(mockCreateTab).toHaveBeenCalledOnce() expect(mockPasteDraftWhenAgentReady).toHaveBeenCalledOnce() }) @@ -376,8 +431,13 @@ describe('structured chat adoption guard on the launch path', () => { ]) const { launchAgentInNewTab } = await import('./launch-agent-in-new-tab') - launchAgentInNewTab({ agent: 'codex', worktreeId: 'wt-1' }) + const unknown = launchAgentInNewTab({ agent: 'codex', worktreeId: 'wt-1' }) await vi.waitFor(() => expect(mockToastError).toHaveBeenCalledTimes(1)) + await expect(unknown?.structuredSettlement).resolves.toEqual({ + kind: 'visibility-unknown', + sessionId: firstIntent.sessionId + }) + expect(mockCreateTab).not.toHaveBeenCalled() launchAgentInNewTab({ agent: 'codex', worktreeId: 'wt-1' }) await vi.waitFor(() => expect(mockRefreshLocalStructuredSessionTabs).toHaveBeenCalledTimes(3)) diff --git a/src/renderer/src/lib/structured-agent-launch-settlement.test.ts b/src/renderer/src/lib/structured-agent-launch-settlement.test.ts index a4f3146dbdd..f137e59f172 100644 --- a/src/renderer/src/lib/structured-agent-launch-settlement.test.ts +++ b/src/renderer/src/lib/structured-agent-launch-settlement.test.ts @@ -96,6 +96,24 @@ describe('settleStructuredAgentLaunch', () => { expect(onStructuredReady).not.toHaveBeenCalled() }) + it('carries a fallback that opened a tab without activating a workspace', async () => { + fakeLaunch({ + launchResult: Promise.reject(new StructuredAgentSessionCreateRefusalError('unsupported')) + }) + const promptDeliveryResult = Promise.resolve({ delivered: true, failureNotified: false }) + const legacyFallback = vi + .fn() + .mockResolvedValue({ primaryTabId: 'new-tab', promptDeliveryResult }) + + await expect( + settleStructuredAgentLaunch('worktree-1', 'codex', {}, { legacyFallback }) + ).resolves.toEqual({ + kind: 'refused-then-legacy', + primaryTabId: 'new-tab', + promptDeliveryResult + }) + }) + it('fails a refusal that has no legacy equivalent', async () => { const error = new StructuredAgentSessionCreateRefusalError('unsupported') fakeLaunch({ launchResult: Promise.reject(error) }) diff --git a/src/renderer/src/lib/structured-agent-launch-settlement.ts b/src/renderer/src/lib/structured-agent-launch-settlement.ts index bfe6ae9ca2c..fd26619d438 100644 --- a/src/renderer/src/lib/structured-agent-launch-settlement.ts +++ b/src/renderer/src/lib/structured-agent-launch-settlement.ts @@ -8,7 +8,8 @@ import type { StructuredPromptDeliveryResult } from '@/lib/structured-agent-sess import type { ActivateAndRevealResult } from '@/lib/worktree-activation' export type StructuredAgentLegacyFallbackResult = { - activation: ActivateAndRevealResult | false + /** Absent when the fallback opened a tab in an already-active workspace instead of activating one. */ + activation?: ActivateAndRevealResult | false primaryTabId: string | null promptDeliveryResult?: Promise }