From 776eba2278dbf21cd7a52ed2b471f94817158e37 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 7 Oct 2026 03:22:23 -0700 Subject: [PATCH 1/8] feat(agent-launch): a click's follow-up survives a window reload, once --- .../vitest-sqlite-database-runtime-files.mjs | 1 + .../agent-launch-follow-up-record.test.ts | 160 +++++++++++ .../runtime/agent-launch-follow-up-record.ts | 113 ++++++++ .../agent-launch-owed-prompt-record.ts | 10 +- .../agent-session-operation-admission.ts | 5 +- src/main/runtime/orca-runtime.ts | 5 + .../methods/agent-launch-follow-ups.test.ts | 151 ++++++++++ .../rpc/methods/agent-launch-follow-ups.ts | 57 ++++ .../methods/agent-launch-owed-prompt-host.ts | 3 + .../rpc/methods/agent-launch-replay.ts | 29 +- src/main/runtime/rpc/methods/agent-launch.ts | 4 + src/main/runtime/rpc/methods/index.ts | 2 + src/main/runtime/runtime-notifier-contract.ts | 2 + src/main/window/runtime-window-lifecycle.ts | 1 + .../ui-bridge-terminal-and-session-tabs.ts | 8 + src/preload/api/ui-command-event-api.ts | 2 + .../src/app-shell/use-app-shell-services.ts | 2 + .../use-recorded-launch-follow-ups.ts | 15 + .../src/components/editor/NotesSendMenu.tsx | 7 + .../editor/ReviewNotesSendMenuContent.tsx | 5 + .../SourceControlAgentActionDialog.tsx | 6 +- .../checks-panel/active-content.tsx | 14 +- .../use-checks-panel-ai-acknowledgement.tsx | 245 +++-------------- .../pr-comment-groups-in-flight.ts | 23 ++ .../pr-comment-resolution-after-delivery.ts | 223 +++++++++++++++ .../pr-comments-list-selection.ts | 16 +- .../review-comments-resolution-follow-up.ts | 28 ++ .../runSourceControlAgentActionStart.ts | 14 +- .../useSourceControlAgentActionDialog.ts | 2 + .../useSourceControlAgentActionStart.ts | 7 +- .../components/tab-bar/QuickLaunchButton.tsx | 8 +- .../src/lib/agent-launch-follow-ups.test.ts | 172 ++++++++++++ .../src/lib/agent-launch-follow-ups.ts | 260 ++++++++++++++++++ .../src/lib/agent-launch-through-host.ts | 12 +- .../src/lib/launch-agent-in-new-tab.ts | 10 +- .../launch-agent-new-tab-host-route.test.ts | 65 ++++- .../lib/launch-agent-new-tab-host-route.ts | 46 +++- src/shared/agent-launch-follow-up.ts | 57 ++++ src/shared/agent-launch-owed-prompt.ts | 16 ++ src/shared/agent-launch-runtime-capability.ts | 7 +- src/shared/agent-session-operation-ledger.ts | 31 +-- .../rpc-contract/agent-launch-params.ts | 17 ++ .../rpc-params-catalog.generated.ts | 3 +- 43 files changed, 1617 insertions(+), 247 deletions(-) create mode 100644 src/main/runtime/agent-launch-follow-up-record.test.ts create mode 100644 src/main/runtime/agent-launch-follow-up-record.ts create mode 100644 src/main/runtime/rpc/methods/agent-launch-follow-ups.test.ts create mode 100644 src/main/runtime/rpc/methods/agent-launch-follow-ups.ts create mode 100644 src/renderer/src/app-shell/use-recorded-launch-follow-ups.ts create mode 100644 src/renderer/src/components/right-sidebar/pr-comment-groups-in-flight.ts create mode 100644 src/renderer/src/components/right-sidebar/pr-comment-resolution-after-delivery.ts create mode 100644 src/renderer/src/components/right-sidebar/review-comments-resolution-follow-up.ts create mode 100644 src/renderer/src/lib/agent-launch-follow-ups.test.ts create mode 100644 src/renderer/src/lib/agent-launch-follow-ups.ts create mode 100644 src/shared/agent-launch-follow-up.ts create mode 100644 src/shared/agent-launch-owed-prompt.ts diff --git a/config/scripts/vitest-sqlite-database-runtime-files.mjs b/config/scripts/vitest-sqlite-database-runtime-files.mjs index 26ec187e4554..ea5844dcce79 100644 --- a/config/scripts/vitest-sqlite-database-runtime-files.mjs +++ b/config/scripts/vitest-sqlite-database-runtime-files.mjs @@ -123,6 +123,7 @@ export const SQLITE_DATABASE_RUNTIME_INCLUDE = [ 'src/main/runtime/orchestration/worker-dispatch-settlement.test.ts', 'src/main/runtime/orchestration/worker-start-unobserved-prompt-settlement.test.ts', 'src/main/runtime/rpc/methods/agent-launch-instant-tab.test.ts', + 'src/main/runtime/rpc/methods/agent-launch-follow-ups.test.ts', 'src/main/runtime/rpc/methods/agent-launch-restart-replay.test.ts', 'src/main/runtime/rpc/methods/native-chat-opencode-semantic-pages.test.ts', 'src/main/runtime/rpc/methods/native-chat-opencode-wire-page-budget.test.ts', diff --git a/src/main/runtime/agent-launch-follow-up-record.test.ts b/src/main/runtime/agent-launch-follow-up-record.test.ts new file mode 100644 index 000000000000..56a7b8240c1b --- /dev/null +++ b/src/main/runtime/agent-launch-follow-up-record.test.ts @@ -0,0 +1,160 @@ +import { describe, expect, it } from 'vitest' +import { + agentSessionOperationKey, + type AgentSessionOperationRow +} from '../../shared/agent-session-operation-ledger' +import type { AgentLaunchResult } from '../../shared/agent-launch-intent' +import { takeLaunchFollowUpsInto } from './agent-launch-follow-up-record' + +const DESKTOP = 'trusted-local:desktop' +const FOLLOW_UP = { kind: 'review-notes-delivered', version: 1, payload: { noteIds: ['n1'] } } + +function launch(promptOutcome: 'handed-to-terminal' | 'not-delivered', unobserved = false) { + const result: AgentLaunchResult = { + outcome: { kind: 'terminal', handle: 'term_1', paneKey: 'tab:leaf' }, + worktreeId: 'wt-1', + receipt: { mode: 'terminal', preferred: 'terminal', reason: 'user_default', detail: 'x' }, + prompt: + promptOutcome === 'handed-to-terminal' && unobserved + ? { delivery: 'submit', outcome: 'handed-to-terminal', composerUnobserved: true } + : { delivery: 'submit', outcome: promptOutcome } + } + return result +} + +function row( + operationId: string, + overrides: Partial = {} +): AgentSessionOperationRow { + return { + callerKey: DESKTOP, + operationId, + fingerprint: 'fp', + operationTimestamp: 1, + recordedAt: 1, + expiresAt: 10_000, + outcome: { status: 'succeeded', sessionId: '', launch: launch('handed-to-terminal') }, + launchFollowUp: FOLLOW_UP, + ...overrides + } +} + +function rows(...list: AgentSessionOperationRow[]) { + return { + operations: new Map(list.map((r) => [agentSessionOperationKey(r.callerKey, r.operationId), r])) + } +} + +const take = ( + state: ReturnType, + args: { callerKey?: string; operationId?: string; running?: string[] } = {} +) => + takeLaunchFollowUpsInto(state, { + callerKey: args.callerKey ?? DESKTOP, + ...(args.operationId ? { operationId: args.operationId } : {}), + now: 100, + isLaunchRunning: (key) => (args.running ?? []).includes(key) + }) + +describe('taking a click’s follow-up off its launch’s row', () => { + it('takes it once its prompt was handed over, removing it in the same write', () => { + const state = rows(row('op-1')) + expect(take(state)).toEqual({ + taken: [ + { + operationId: 'op-1', + followUp: FOLLOW_UP, + promptHandedOver: true, + composerUnobserved: false + } + ], + pending: [] + }) + expect(state.operations.get(agentSessionOperationKey(DESKTOP, 'op-1'))).not.toHaveProperty( + 'launchFollowUp' + ) + // Taken once: whoever took it is the one runner. + expect(take(state)).toEqual({ taken: [], pending: [] }) + }) + + it('says when the host wrote the prompt without seeing the composer', () => { + const state = rows( + row('op-1', { + outcome: { + status: 'succeeded', + sessionId: '', + launch: launch('handed-to-terminal', true) + } + }) + ) + expect(take(state).taken[0]).toMatchObject({ promptHandedOver: true, composerUnobserved: true }) + }) + + it('reports a follow-up whose prompt is still owed, read-only', () => { + const state = rows( + row('op-1', { + promptDelivery: { state: 'owed', text: 't', agent: 'claude', deadline: 9_000 } + }) + ) + expect(take(state)).toEqual({ + taken: [], + pending: [{ operationId: 'op-1', followUp: FOLLOW_UP }] + }) + expect(state.operations.get(agentSessionOperationKey(DESKTOP, 'op-1'))?.launchFollowUp).toEqual( + FOLLOW_UP + ) + }) + + it('takes, with nothing to run, a launch that never handed its prompt over', () => { + const state = rows( + row('not-delivered', { + outcome: { status: 'succeeded', sessionId: '', launch: launch('not-delivered') } + }), + row('failed', { outcome: { status: 'failed', code: 'worktree_not_found' } }), + // Claimed, then the host died before it recorded anything. + row('died', { outcome: { status: 'unknown' } }) + ) + expect(take(state).taken.map((t) => [t.operationId, t.promptHandedOver])).toEqual([ + ['not-delivered', false], + ['failed', false], + ['died', false] + ]) + }) + + it('waits on a prompt whose state this build cannot read, rather than dropping its follow-up', () => { + const state = rows(Object.assign(row('op-1'), { promptDelivery: { state: 'paused' } })) + expect(take(state)).toEqual({ + taken: [], + pending: [{ operationId: 'op-1', followUp: FOLLOW_UP }] + }) + }) + + it('waits on a launch this process is still running', () => { + const state = rows(row('op-1', { outcome: { status: 'unknown' } })) + expect(take(state, { running: [agentSessionOperationKey(DESKTOP, 'op-1')] })).toEqual({ + taken: [], + pending: [{ operationId: 'op-1', followUp: FOLLOW_UP }] + }) + }) + + it('never shows or takes another caller’s follow-ups', () => { + const state = rows(row('op-1')) + expect(take(state, { callerKey: 'device-1' })).toEqual({ taken: [], pending: [] }) + expect(take(state, { callerKey: 'trusted-local:runtime' })).toEqual({ taken: [], pending: [] }) + expect(take(state).taken).toHaveLength(1) + }) + + it('takes only the named launch when the click asks for its own', () => { + const state = rows(row('op-1'), row('op-2')) + expect(take(state, { operationId: 'op-2' }).taken.map((t) => t.operationId)).toEqual(['op-2']) + expect(take(state).taken.map((t) => t.operationId)).toEqual(['op-1']) + }) + + it('ignores an expired row and a value this build cannot read', () => { + const state = rows( + row('expired', { expiresAt: 50 }), + Object.assign(row('malformed'), { launchFollowUp: { kind: 'x' } }) + ) + expect(take(state)).toEqual({ taken: [], pending: [] }) + }) +}) diff --git a/src/main/runtime/agent-launch-follow-up-record.ts b/src/main/runtime/agent-launch-follow-up-record.ts new file mode 100644 index 000000000000..5a136dd160c0 --- /dev/null +++ b/src/main/runtime/agent-launch-follow-up-record.ts @@ -0,0 +1,113 @@ +/** + * Taking a click's follow-up off its launch's row (`agent-launch-follow-up`). + * + * Taken once its prompt has settled, in the same write that removes it, so whoever takes it is the + * one runner: a window that crashes between taking and running loses the follow-up, never repeats + * it. A follow-up still waiting on its prompt is reported, never removed. + */ + +import { + isAgentLaunchFollowUp, + type AgentLaunchFollowUpTake +} from '../../shared/agent-launch-follow-up' +import { isAgentLaunchResult } from '../../shared/agent-launch-intent' +import { + agentSessionOperationKey, + type AgentSessionOperationRow +} from '../../shared/agent-session-operation-ledger' +import { launchPromptUnsettled, readOwedLaunchPrompt } from './agent-launch-owed-prompt-record' + +type OperationRows = { operations: Map } + +/** Whether the launch has handed its prompt over, or never will; null while it still may. */ +function settledPrompt( + row: AgentSessionOperationRow, + isLaunchRunning: (operationKey: string) => boolean +): { handedOver: boolean; composerUnobserved: boolean } | null { + if (launchPromptUnsettled(row)) { + return null + } + const { outcome } = row + if (outcome.status === 'succeeded') { + const prompt = isAgentLaunchResult(outcome.launch) ? outcome.launch.prompt : undefined + return prompt?.outcome === 'handed-to-terminal' + ? { handedOver: true, composerUnobserved: prompt.composerUnobserved === true } + : { handedOver: false, composerUnobserved: false } + } + // A claimed launch this process no longer runs died before it recorded anything: its prompt was + // never handed over, and waiting for it would hold its follow-up until the row expires. + if ( + outcome.status !== 'failed' && + isLaunchRunning(agentSessionOperationKey(row.callerKey, row.operationId)) + ) { + return null + } + return { handedOver: false, composerUnobserved: false } +} + +export function takeLaunchFollowUpsInto( + state: OperationRows, + args: { + callerKey: string + operationId?: string + now: number + isLaunchRunning: (operationKey: string) => boolean + } +): AgentLaunchFollowUpTake { + const take: AgentLaunchFollowUpTake = { taken: [], pending: [] } + let next: Map | null = null + for (const [key, row] of state.operations) { + const followUp: unknown = row.launchFollowUp + if ( + row.callerKey !== args.callerKey || + row.expiresAt <= args.now || + (args.operationId !== undefined && row.operationId !== args.operationId) || + !isAgentLaunchFollowUp(followUp) + ) { + continue + } + const settled = settledPrompt(row, args.isLaunchRunning) + if (!settled) { + const owed = readOwedLaunchPrompt(row) + take.pending.push({ + operationId: row.operationId, + followUp, + ...(owed?.state === 'owed' ? { deadline: owed.deadline } : {}) + }) + continue + } + take.taken.push({ + operationId: row.operationId, + followUp, + promptHandedOver: settled.handedOver, + composerUnobserved: settled.composerUnobserved + }) + const { launchFollowUp: _taken, ...rest } = row + next ??= new Map(state.operations) + next.set(key, rest) + } + if (next) { + state.operations = next + } + return take +} + +/** The launches whose follow-up now waits only on its caller to take it. */ +export function listSettledLaunchFollowUps( + rows: Iterable, + callerKey: string, + now: number +): string[] { + const settled: string[] = [] + for (const row of rows) { + if ( + row.callerKey === callerKey && + row.expiresAt > now && + isAgentLaunchFollowUp(row.launchFollowUp) && + !launchPromptUnsettled(row) + ) { + settled.push(row.operationId) + } + } + return settled +} diff --git a/src/main/runtime/agent-launch-owed-prompt-record.ts b/src/main/runtime/agent-launch-owed-prompt-record.ts index 81f4614968f2..f5b358386d9d 100644 --- a/src/main/runtime/agent-launch-owed-prompt-record.ts +++ b/src/main/runtime/agent-launch-owed-prompt-record.ts @@ -11,10 +11,10 @@ import { agentSessionOperationKey, - type AgentLaunchOwedPrompt, type AgentSessionOperationOutcome, type AgentSessionOperationRow } from '../../shared/agent-session-operation-ledger' +import type { AgentLaunchOwedPrompt } from '../../shared/agent-launch-owed-prompt' import { isTuiAgent } from '../../shared/tui-agent-config' import type { AgentSessionRecordStore } from './agent-session-record-store' @@ -136,6 +136,14 @@ export function clearOwedLaunchPromptInto(state: OperationRows, ref: OperationRe }) } +/** + * Whether the launch's prompt is still on its way: any value, even one this build cannot read, + * since only the settle (W3) removes the field. What depends on the prompt waits for this. + */ +export function launchPromptUnsettled(row: AgentSessionOperationRow): boolean { + return row.promptDelivery !== undefined +} + /** Unexpired rows that still owe a prompt or may be writing one. */ export function listOwedLaunchPromptRows( rows: Iterable, diff --git a/src/main/runtime/agent-session-operation-admission.ts b/src/main/runtime/agent-session-operation-admission.ts index 1de4c13d1134..ece58e41c22c 100644 --- a/src/main/runtime/agent-session-operation-admission.ts +++ b/src/main/runtime/agent-session-operation-admission.ts @@ -21,6 +21,7 @@ import { import type { AgentSessionMutationEnvelope } from '../../shared/agent-session-wire' import type { AgentSessionRecord } from '../../shared/agent-session-record' import type { AgentSessionStoreState } from './agent-session-store-state' +import type { AgentLaunchFollowUp } from '../../shared/agent-launch-follow-up' import { settleLaunchPromptInto, type LaunchPromptSettlement @@ -191,7 +192,7 @@ export function admitAgentSessionGlobalOperationInto( export function claimAgentSessionOperationInto( state: { operations: Map }, - args: { callerKey: string; operationId: string; ownedPane?: AgentSessionOperationOwnedPane } + args: Parameters[1] ): AgentSessionOperationClaim { const claimed = claimAgentSessionOperation(state.operations, args) state.operations = claimed.rows @@ -204,6 +205,8 @@ export type ClaimAfterAdmission = (decision: AgentSessionOperationDecision) => b /** An admission whose claimant laid out a pane before its effect; recorded only if the claim wins. */ export type AgentSessionOperationClaimingAdmission = AgentSessionOperationAdmission & { ownedPane?: AgentSessionOperationOwnedPane + /** What the click owes once its prompt lands; recorded only if the claim wins. */ + launchFollowUp?: AgentLaunchFollowUp } /** Admission and, when `claimAfter` says so, the claim, in one transaction: the same swap as diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 2ed451fc59bf..d1234bbdca39 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -64,6 +64,11 @@ class OrcaRuntimeService extends OrcaRuntimeWithResolveWaiter { this.notifier?.agentLaunchPaneVerdict?.({ ...pane, verdict }) } + /** Tells the window a launch it recorded a follow-up for has settled its prompt. */ + reportAgentLaunchPromptSettled(operationId: string): void { + this.notifier?.agentLaunchPromptSettled?.({ operationId }) + } + private readonly agentLaunchRecordWarmup = createAgentLaunchRecordWarmupGate({ isOpen: () => peekOpenedAgentSessionRecordStore() !== null, open: () => this.openAgentSessionRecordStore() diff --git a/src/main/runtime/rpc/methods/agent-launch-follow-ups.test.ts b/src/main/runtime/rpc/methods/agent-launch-follow-ups.test.ts new file mode 100644 index 000000000000..4bf9df83e545 --- /dev/null +++ b/src/main/runtime/rpc/methods/agent-launch-follow-ups.test.ts @@ -0,0 +1,151 @@ +/** + * A click's follow-up on its launch's record, against the real durable ledger: recorded with the + * launch, taken once by the caller that made it, and never seen by any other caller. + */ + +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { AGENT_LAUNCH_RUNTIME_CAPABILITY } from '../../../../shared/agent-launch-runtime-capability' +import type { AgentSessionRecordStore } from '../../agent-session-record-store' +import { openTestAgentSessionRecordStore } from '../../agent-session-record-store-test-harness' +import type { RpcContext } from '../core' +import { DESKTOP_RPC_CALLER } from '../rpc-caller-identity' +import { + methodNamed, + rpcContext, + runtimeStub, + setAgentLaunchRecordStore, + type AgentLaunchRuntimeStub +} from './agent-launch.test-fixture' + +const deliverTerminalPrompt = vi.hoisted(() => + vi.fn(async (args: { beginPromptWrite?: () => Promise }): Promise => { + await args.beginPromptWrite?.() + return true + }) +) +vi.mock('./agent-launch-terminal-prompt', () => ({ + deliverTerminalAgentLaunchPrompt: deliverTerminalPrompt +})) + +const { AGENT_LAUNCH_METHODS } = await import('./agent-launch') +const { AGENT_LAUNCH_FOLLOW_UP_METHODS } = await import('./agent-launch-follow-ups') +const AGENT_LAUNCH_REPLAY = methodNamed(AGENT_LAUNCH_METHODS, 'agent.launchReplay') +const TAKE = methodNamed(AGENT_LAUNCH_FOLLOW_UP_METHODS, 'agent.takeLaunchFollowUps') + +const OPERATION_ID = `${Date.now()}-000000000000000000000000000000f1` +const PANE_KEY = '9b1deb4d-3b7d-4bad-9bdd-2b0d7b3dcb6d:3f2504e0-4f89-41d3-9a0c-0305e82c3301' +const FOLLOW_UP = { kind: 'review-notes-delivered', version: 1, payload: { worktreeId: 'wt-7' } } +const LAUNCH = { + agent: 'claude', + target: { kind: 'existing', worktree: 'id:wt-7' }, + prompt: { text: 'fix the notes', delivery: 'submit', transport: 'paste' }, + followUp: FOLLOW_UP, + operationId: OPERATION_ID +} +const DESKTOP: Partial = { + caller: DESKTOP_RPC_CALLER, + clientKind: 'runtime', + clientCapabilities: [AGENT_LAUNCH_RUNTIME_CAPABILITY] +} +const PHONE: Partial = { + clientKind: 'mobile', + pairedDeviceId: 'device-1', + clientCapabilities: [AGENT_LAUNCH_RUNTIME_CAPABILITY] +} + +let directory: string +let store: AgentSessionRecordStore + +function host(): AgentLaunchRuntimeStub { + return runtimeStub({ settings: {}, terminalPaneKey: PANE_KEY, lineCarriesPrompt: false }) +} + +function launch( + runtime: AgentLaunchRuntimeStub, + params: unknown = LAUNCH, + context: Partial = DESKTOP +) { + return AGENT_LAUNCH_REPLAY.handler( + AGENT_LAUNCH_REPLAY.params.parse(params), + rpcContext(runtime, context) + ) +} + +function take(runtime: AgentLaunchRuntimeStub, context: Partial, operationId?: string) { + return TAKE.handler( + TAKE.params.parse(operationId ? { operationId } : {}), + rpcContext(runtime, context) + ) +} + +beforeEach(async () => { + deliverTerminalPrompt.mockClear() + directory = await mkdtemp(join(tmpdir(), 'orca-agent-launch-follow-ups-')) + store = await openTestAgentSessionRecordStore(directory) + setAgentLaunchRecordStore(store) +}) + +afterEach(async () => { + setAgentLaunchRecordStore(null) + await rm(directory, { recursive: true, force: true }) +}) + +describe('a click’s follow-up on its launch’s record', () => { + it('is recorded with the launch and taken once, by the caller that made it', async () => { + const runtime = host() + await launch(runtime) + + // Another caller, on a connection that proves it is someone else, sees nothing. + await expect(take(runtime, PHONE)).resolves.toEqual({ taken: [], pending: [] }) + await expect(take(runtime, DESKTOP, OPERATION_ID)).resolves.toEqual({ + taken: [ + { + operationId: OPERATION_ID, + followUp: FOLLOW_UP, + promptHandedOver: true, + composerUnobserved: false + } + ], + pending: [] + }) + await expect(take(runtime, DESKTOP)).resolves.toEqual({ taken: [], pending: [] }) + }) + + it('is recorded for the desktop only: a phone, which never takes one, records nothing', async () => { + const runtime = host() + await launch(runtime, LAUNCH, PHONE) + await expect(take(runtime, PHONE)).resolves.toEqual({ taken: [], pending: [] }) + expect(store.listOperationRows().some((row) => row.launchFollowUp !== undefined)).toBe(false) + }) + + it('outlives a host restart until it is taken', async () => { + await launch(host()) + store = await openTestAgentSessionRecordStore(directory) + setAgentLaunchRecordStore(store) + + const after = await take(host(), DESKTOP) + expect(after.taken.map((entry) => entry.operationId)).toEqual([OPERATION_ID]) + }) + + it('is not recorded over the size cap, and the launch still runs', async () => { + const runtime = host() + const huge = { ...FOLLOW_UP, payload: { blob: 'x'.repeat(300 * 1024) } } + + const result = await launch(runtime, { ...LAUNCH, followUp: huge }) + + expect(result.prompt).toEqual({ delivery: 'submit', outcome: 'handed-to-terminal' }) + await expect(take(runtime, DESKTOP)).resolves.toEqual({ taken: [], pending: [] }) + }) + + it('is not part of what the launch is: a retry that names another follow-up replays it', async () => { + const runtime = host() + const first = await launch(runtime) + await expect( + launch(runtime, { ...LAUNCH, followUp: { ...FOLLOW_UP, version: 2 } }) + ).resolves.toEqual(first) + expect(runtime.createTerminal).toHaveBeenCalledOnce() + }) +}) diff --git a/src/main/runtime/rpc/methods/agent-launch-follow-ups.ts b/src/main/runtime/rpc/methods/agent-launch-follow-ups.ts new file mode 100644 index 000000000000..0bc98a693e09 --- /dev/null +++ b/src/main/runtime/rpc/methods/agent-launch-follow-ups.ts @@ -0,0 +1,57 @@ +/** + * `agent.takeLaunchFollowUps`: the calling client takes the follow-ups its own launches recorded + * (`agent-launch-follow-up-record`). Scoped by the connection's caller identity, so a phone or the + * CLI can never see, or take, the desktop's. + */ + +import { AgentTakeLaunchFollowUps } from '../../../../shared/rpc-contract/agent-launch-params' +import type { AgentLaunchFollowUpTake } from '../../../../shared/agent-launch-follow-up' +import { + listSettledLaunchFollowUps, + takeLaunchFollowUpsInto +} from '../../agent-launch-follow-up-record' +import type { OrcaRuntimeService } from '../../orca-runtime' +import { defineMethod } from '../core' +import { DESKTOP_RPC_CALLER, rpcCallerOperationKey } from '../rpc-caller-identity' +import { activeAgentLaunchesFor } from './agent-launch-active-operations' +import { agentLaunchOperationCallerKey } from './agent-launch-replay' + +/** + * Tells the desktop window which of its recorded follow-ups now wait only on it, so a window that + * reloaded mid-launch takes each as its prompt settles, not at its next start. Only the desktop + * records follow-ups. One the window is not waiting on is ignored there. + */ +export function announceSettledLaunchFollowUps(runtime: OrcaRuntimeService): void { + const store = runtime.openedAgentSessionRecordStore() + if (!store) { + return + } + const desktop = rpcCallerOperationKey(DESKTOP_RPC_CALLER) + for (const operationId of listSettledLaunchFollowUps( + store.listOperationRows(), + desktop, + Date.now() + )) { + runtime.reportAgentLaunchPromptSettled(operationId) + } +} + +export const AGENT_LAUNCH_FOLLOW_UP_METHODS = [ + defineMethod({ + name: 'agent.takeLaunchFollowUps', + params: AgentTakeLaunchFollowUps, + handler: async (params, context): Promise => { + const callerKey = agentLaunchOperationCallerKey(context) + const store = await context.runtime.openAgentSessionRecordStore() + const active = activeAgentLaunchesFor(context.runtime) + return store.transactOperations((draft) => + takeLaunchFollowUpsInto(draft, { + callerKey, + ...(params.operationId ? { operationId: params.operationId } : {}), + now: Date.now(), + isLaunchRunning: (operationKey) => active.has(operationKey) + }) + ) + } + }) +] diff --git a/src/main/runtime/rpc/methods/agent-launch-owed-prompt-host.ts b/src/main/runtime/rpc/methods/agent-launch-owed-prompt-host.ts index 6051b74cbee4..cec67b1f5d7d 100644 --- a/src/main/runtime/rpc/methods/agent-launch-owed-prompt-host.ts +++ b/src/main/runtime/rpc/methods/agent-launch-owed-prompt-host.ts @@ -17,6 +17,7 @@ import { activeAgentLaunchesFor } from './agent-launch-active-operations' import { deliverTerminalAgentLaunchPrompt } from './agent-launch-terminal-prompt' import type { AgentLaunchResult } from '../../../../shared/agent-launch-intent' import type { LaunchedTerminal } from './agent-launch-replay' +import { announceSettledLaunchFollowUps } from './agent-launch-follow-ups' function resumeDeps( runtime: OrcaRuntimeService, @@ -90,6 +91,7 @@ export async function resumeOwedAgentLaunchPrompts(runtime: OrcaRuntimeService): if (store && (await resumeOwedLaunchPrompts(resumeDeps(runtime, store)))) { sweepAgainSoon(runtime) } + announceSettledLaunchFollowUps(runtime) } catch (error) { console.warn('[agent-launch] could not resume owed launch prompts', error) } @@ -112,5 +114,6 @@ export async function settleOwedLaunchPromptBeforeReplay( if (resumed === 'retry') { sweepAgainSoon(runtime) } + announceSettledLaunchFollowUps(runtime) } } diff --git a/src/main/runtime/rpc/methods/agent-launch-replay.ts b/src/main/runtime/rpc/methods/agent-launch-replay.ts index d62daeddb02c..c7269077aac3 100644 --- a/src/main/runtime/rpc/methods/agent-launch-replay.ts +++ b/src/main/runtime/rpc/methods/agent-launch-replay.ts @@ -32,6 +32,10 @@ import type { RpcContext } from '../core' import { rpcCallerOperationKey } from '../rpc-caller-identity' import type { AgentLaunchParams } from './agent-launch-schemas' import { isDesktopLaunchCaller } from './agent-launch-desktop-caller' +import { + agentLaunchFollowUpFits, + type AgentLaunchFollowUp +} from '../../../../shared/agent-launch-follow-up' import { OWED_LAUNCH_PROMPT_DEADLINE_MS, type OwedLaunchPrompt, @@ -222,8 +226,17 @@ export async function admitAgentLaunchOperation( const callerKey = agentLaunchOperationCallerKey(context) // The ledger alone: admitting a terminal launch has no use for the chat host. const store = await context.runtime.openAgentSessionRecordStore() + // Temporary, desktop only like its owed prompt: no other caller takes follow-ups. + const launchFollowUp = isDesktopLaunchCaller(callerKey) ? recordableFollowUp(params) : undefined const { decision: admitted, claim } = await store.admitAndClaimOperation( - { callerKey, operationId, fingerprint, now, ...(ownedPane ? { ownedPane } : {}) }, + { + callerKey, + operationId, + fingerprint, + now, + ...(ownedPane ? { ownedPane } : {}), + ...(launchFollowUp ? { launchFollowUp } : {}) + }, // A fresh row, or a replayed one no one has answered yet, leaves the right to run open. (decision) => decision.decision === 'admit' || @@ -293,6 +306,20 @@ export async function admitAgentLaunchOperation( } } +/** A follow-up over the cap is not recorded: it runs live, as before, and never fails the launch. */ +function recordableFollowUp(params: AgentLaunchParams): AgentLaunchFollowUp | undefined { + if (!params.followUp) { + return undefined + } + if (!agentLaunchFollowUpFits(params.followUp)) { + console.warn( + `[agent-launch] a ${params.followUp.kind} follow-up is over the record's size cap; it runs live only` + ) + return undefined + } + return params.followUp +} + /** The text a terminal's first write still owes: a submit nothing has delivered yet, which the * provisional answer marks `unconfirmed` (`settledAtCreation`). */ function owedTerminalPrompt( diff --git a/src/main/runtime/rpc/methods/agent-launch.ts b/src/main/runtime/rpc/methods/agent-launch.ts index e069a17cb9c9..eeacc7977602 100644 --- a/src/main/runtime/rpc/methods/agent-launch.ts +++ b/src/main/runtime/rpc/methods/agent-launch.ts @@ -58,6 +58,7 @@ import { launchedTerminal, settleOwedLaunchPromptBeforeReplay } from './agent-launch-owed-prompt-host' +import { announceSettledLaunchFollowUps } from './agent-launch-follow-ups' import { clientRendersStructuredAgent } from './structured-agent-session-policy' import { resolveUnlaunchedIntent } from './agent-launch-intent-resolution' import { @@ -270,6 +271,9 @@ async function executeAdmittedAgentLaunch( // Bookkeeping: a failure leaves the first write, whose owed prompt replays as `unconfirmed` (or as // `unknown` to a caller that cannot read it), never as `not-delivered`. await settleQuietly(admission.settle(result)) + if (params.followUp) { + announceSettledLaunchFollowUps(context.runtime) + } return result } diff --git a/src/main/runtime/rpc/methods/index.ts b/src/main/runtime/rpc/methods/index.ts index 267b201edf90..6cfd1cc56933 100644 --- a/src/main/runtime/rpc/methods/index.ts +++ b/src/main/runtime/rpc/methods/index.ts @@ -50,6 +50,7 @@ import { STRUCTURED_AGENT_SESSION_AGENTS_METHODS } from './structured-agent-sess import { ARTIFACT_METHODS } from './artifacts' import { AGENT_HOOK_METHODS } from './agent-hooks' import { AGENT_LAUNCH_METHODS } from './agent-launch' +import { AGENT_LAUNCH_FOLLOW_UP_METHODS } from './agent-launch-follow-ups' // Why: a flat manifest keeps registration order explicit and provides one // grep-point for "what methods does the RPC server expose?" — useful when @@ -66,6 +67,7 @@ export const ALL_RPC_METHODS = [ ...STRUCTURED_AGENT_SESSION_METHODS, ...STRUCTURED_AGENT_SESSION_AGENTS_METHODS, ...AGENT_LAUNCH_METHODS, + ...AGENT_LAUNCH_FOLLOW_UP_METHODS, ...TERMINAL_METHODS, ...TERMINAL_ORPHAN_METHODS, ...BROWSER_CORE_METHODS, diff --git a/src/main/runtime/runtime-notifier-contract.ts b/src/main/runtime/runtime-notifier-contract.ts index 8a6b3d2bc467..883bded9b56e 100644 --- a/src/main/runtime/runtime-notifier-contract.ts +++ b/src/main/runtime/runtime-notifier-contract.ts @@ -86,6 +86,8 @@ export type RuntimeNotifier = { ): Promise /** A launch pane's fate, for the window to keep on its tab or act on. */ agentLaunchPaneVerdict?(event: AgentLaunchPaneVerdictEvent): void + /** A desktop launch whose click recorded a follow-up has settled its prompt: the window takes it. */ + agentLaunchPromptSettled?(event: { operationId: string }): void resolveLegacyWorkerTerminalRecovery?( paneKey: string, resolution: 'adopted' | 'exited' | 'rolled_back', diff --git a/src/main/window/runtime-window-lifecycle.ts b/src/main/window/runtime-window-lifecycle.ts index 857ed79d787e..80e348b347c1 100644 --- a/src/main/window/runtime-window-lifecycle.ts +++ b/src/main/window/runtime-window-lifecycle.ts @@ -147,6 +147,7 @@ export function registerRuntimeWindowLifecycle( publishAgentLaunchTab: (request) => requestAgentLaunchTabPublishFromRenderer(mainWindow, request), agentLaunchPaneVerdict: (event) => send('ui:agentLaunchPaneVerdict', event), + agentLaunchPromptSettled: (event) => send('ui:agentLaunchPromptSettled', event), resolveLegacyWorkerTerminalRecovery: (paneKey, resolution, ptyId) => send('agentStatus:legacyWorkerTerminalRecovery', { paneKey, diff --git a/src/preload/api/ui-bridge-terminal-and-session-tabs.ts b/src/preload/api/ui-bridge-terminal-and-session-tabs.ts index 458a50c05841..521064bd370d 100644 --- a/src/preload/api/ui-bridge-terminal-and-session-tabs.ts +++ b/src/preload/api/ui-bridge-terminal-and-session-tabs.ts @@ -116,6 +116,14 @@ export const uiTerminalAndSessionTabsApi = { ipcRenderer.on('ui:agentLaunchPaneVerdict', listener) return () => ipcRenderer.removeListener('ui:agentLaunchPaneVerdict', listener) }, + onAgentLaunchPromptSettled: ( + callback: (data: { operationId: string }) => void + ): (() => void) => { + const listener = (_event: Electron.IpcRendererEvent, data: { operationId: string }) => + callback(data) + ipcRenderer.on('ui:agentLaunchPromptSettled', listener) + return () => ipcRenderer.removeListener('ui:agentLaunchPromptSettled', listener) + }, onSplitTerminal: ( callback: (data: { tabId: string diff --git a/src/preload/api/ui-command-event-api.ts b/src/preload/api/ui-command-event-api.ts index 9a9c8524ed3e..96c728c3ba8d 100644 --- a/src/preload/api/ui-command-event-api.ts +++ b/src/preload/api/ui-command-event-api.ts @@ -188,6 +188,8 @@ export type UiCommandEventApi = { onPublishAgentLaunchTab: (callback: (data: AgentLaunchTabPublishRequest) => void) => () => void replyAgentLaunchTabPublish: (reply: AgentLaunchTabPublishReply) => void onAgentLaunchPaneVerdict: (callback: (data: AgentLaunchPaneVerdictEvent) => void) => () => void + /** A launch whose click recorded a follow-up settled its prompt (`agent-launch-follow-ups`). */ + onAgentLaunchPromptSettled?: (callback: (data: { operationId: string }) => void) => () => void onSplitTerminal: ( callback: (data: { tabId: string diff --git a/src/renderer/src/app-shell/use-app-shell-services.ts b/src/renderer/src/app-shell/use-app-shell-services.ts index d64925a8a3b3..5eff2534295b 100644 --- a/src/renderer/src/app-shell/use-app-shell-services.ts +++ b/src/renderer/src/app-shell/use-app-shell-services.ts @@ -17,6 +17,7 @@ import { useGitStatusPolling } from '../components/right-sidebar/useGitStatusPol import { useOsc52ClipboardDefaultOnNotice } from '../components/terminal-pane/osc52-clipboard-default-on-notice' import { useWebSessionTabsSync } from '../runtime/web-session-tabs-sync' import { useLocalStructuredSessionTabsSync } from '../runtime/local-structured-session-tabs-sync' +import { useRecordedLaunchFollowUps } from './use-recorded-launch-follow-ups' import { useHostStructuredAgentsSync } from '../runtime/host-structured-agents-sync' import { useRemoteRuntimeRecoveryTriggers } from '../runtime/use-remote-runtime-recovery-triggers' import { useTerminalViewerColorPublication } from './use-terminal-viewer-color-publication' @@ -42,6 +43,7 @@ export function useAppShellServices(): void { useRadixBodyPointerEventsRecovery() useWebSessionTabsSync() useLocalStructuredSessionTabsSync() + useRecordedLaunchFollowUps() useHostStructuredAgentsSync() // Subscribe to IPC push events useIpcEvents() diff --git a/src/renderer/src/app-shell/use-recorded-launch-follow-ups.ts b/src/renderer/src/app-shell/use-recorded-launch-follow-ups.ts new file mode 100644 index 000000000000..8e78078c3b34 --- /dev/null +++ b/src/renderer/src/app-shell/use-recorded-launch-follow-ups.ts @@ -0,0 +1,15 @@ +import { useEffect } from 'react' +import { useAppStore } from '../store' +import { runRecordedLaunchFollowUps } from '../lib/agent-launch-follow-ups' + +/** As soon as the window has its worktrees and their notes, and before its terminals are back, it + * holds what launches still on their way act on and runs what they recorded for it while it was + * gone (`agent-launch-follow-ups`): a note shown as sendable meanwhile could be sent twice. */ +export function useRecordedLaunchFollowUps(): void { + const ready = useAppStore((state) => state.workspaceSessionReady) + useEffect(() => { + if (ready) { + void runRecordedLaunchFollowUps() + } + }, [ready]) +} diff --git a/src/renderer/src/components/editor/NotesSendMenu.tsx b/src/renderer/src/components/editor/NotesSendMenu.tsx index bb8cac0511a6..d553c931ab30 100644 --- a/src/renderer/src/components/editor/NotesSendMenu.tsx +++ b/src/renderer/src/components/editor/NotesSendMenu.tsx @@ -22,6 +22,7 @@ import { useNotesInFlightVersion } from '@/lib/notes-send-in-flight' import { translate } from '@/i18n/i18n' +import { reviewNotesDeliveredFollowUp } from '@/lib/agent-launch-follow-ups' const ENABLED_SEND_TOOLTIP = 'Send notes to an agent' @@ -60,6 +61,8 @@ export type NotesSendMenuProps = { // so it reads exact time rather than a render clock that can lag or freeze. openRequestExpiresAt?: number | null onOpenRequestHandled?: () => void + /** Must remove these notes from `worktreeId` (`clearDeliveredDiffComments`): a new agent's launch + * records exactly that, so a window that reloads mid-launch still removes them once. */ onDelivered: (notes: readonly TNote[]) => void } @@ -278,6 +281,7 @@ export function NotesSendMenu({ promptDelivery="submit-after-ready" launchSource="notes_send" onPromptDelivered={() => markDelivered(scope.notes)} + durableFollowUp={reviewNotesDeliveredFollowUp(worktreeId, scope.notes)} onPromptHandedOff={holdInFlight(scope.notes)} /> @@ -296,6 +300,9 @@ export function NotesSendMenu({ markDelivered(defaultScope.notes) } }} + {...(defaultScope + ? { durableFollowUp: reviewNotesDeliveredFollowUp(worktreeId, defaultScope.notes) } + : {})} onPromptHandedOff={holdInFlight(defaultScope?.notes ?? [])} /> )} diff --git a/src/renderer/src/components/editor/ReviewNotesSendMenuContent.tsx b/src/renderer/src/components/editor/ReviewNotesSendMenuContent.tsx index 935d98dd9861..de079fe769b2 100644 --- a/src/renderer/src/components/editor/ReviewNotesSendMenuContent.tsx +++ b/src/renderer/src/components/editor/ReviewNotesSendMenuContent.tsx @@ -34,6 +34,7 @@ import { useWorktreeAgentRows } from '@/components/sidebar/useWorktreeAgentRows' import type { LaunchSource } from '../../../../shared/telemetry-events' import { agentRowDisplayDotState } from '@/lib/agent-row-dot-state' import { translate } from '@/i18n/i18n' +import type { AgentLaunchFollowUp } from '../../../../shared/agent-launch-follow-up' type OrderedSendTarget = { target: NotesSendAgentTarget @@ -47,6 +48,7 @@ export function ReviewNotesSendMenuContent({ promptDelivery = 'submit-after-ready', launchSource = 'notes_send', onPromptDelivered, + durableFollowUp, onPromptHandedOff }: { worktreeId: string @@ -55,6 +57,8 @@ export function ReviewNotesSendMenuContent({ promptDelivery?: 'auto-submit' | 'draft' | 'submit-after-ready' launchSource?: LaunchSource onPromptDelivered?: () => void + /** What `onPromptDelivered` does, recorded on a new agent's launch so a reload still runs it. */ + durableFollowUp?: AgentLaunchFollowUp /** Given each send's own result the moment its prompt is handed to an agent. */ onPromptHandedOff?: (delivered: Promise) => void }): React.JSX.Element { @@ -212,6 +216,7 @@ export function ReviewNotesSendMenuContent({ promptDelivery={promptDelivery} launchSource={launchSource} onPromptDelivered={onPromptDelivered} + {...(durableFollowUp ? { durableFollowUp } : {})} onPromptHandedOff={onPromptHandedOff} disabled={!hasPrompt} /> diff --git a/src/renderer/src/components/right-sidebar/SourceControlAgentActionDialog.tsx b/src/renderer/src/components/right-sidebar/SourceControlAgentActionDialog.tsx index 1e508b917145..6057f12d2980 100644 --- a/src/renderer/src/components/right-sidebar/SourceControlAgentActionDialog.tsx +++ b/src/renderer/src/components/right-sidebar/SourceControlAgentActionDialog.tsx @@ -15,6 +15,7 @@ import type { LaunchSource } from '../../../../shared/telemetry-events' import type { SourceControlAiWriteTarget } from '../../../../shared/source-control-ai-recipe-save' import { SourceControlAgentActionDialogForm } from './SourceControlAgentActionDialogForm' import { useSourceControlAgentActionDialog } from './useSourceControlAgentActionDialog' +import type { AgentLaunchFollowUp } from '../../../../shared/agent-launch-follow-up' export type SourceControlAgentActionDialogProps = { open: boolean @@ -46,7 +47,10 @@ export type SourceControlAgentActionDialogProps = { onLaunchAccepted?: () => void /** Fires when an accepted launch later failed to deliver its prompt. */ onLaunchAborted?: () => void - onLaunched?: () => void + onLaunched?: (launch?: { followUpDeferred?: boolean }) => void + /** What `onLaunched` does that a reload must not turn into a second send, recorded on the launch + * so a window that reloads mid-launch still runs it once (`agent-launch-follow-ups`). */ + durableFollowUp?: AgentLaunchFollowUp startLabel?: string onStart?: (args: { agent: TuiAgent diff --git a/src/renderer/src/components/right-sidebar/checks-panel/active-content.tsx b/src/renderer/src/components/right-sidebar/checks-panel/active-content.tsx index 767913b65593..905f92a3325b 100644 --- a/src/renderer/src/components/right-sidebar/checks-panel/active-content.tsx +++ b/src/renderer/src/components/right-sidebar/checks-panel/active-content.tsx @@ -21,6 +21,7 @@ import { translate } from '@/i18n/i18n' import type { ChecksPanelReview } from '../checks-panel-review' import type { ChecksPanelHostedReviewModifierDestination } from '../checks-panel-hosted-review-click-routing' import type { ChecksPanelActiveContentModel } from './active-content-props' +import { reviewCommentsResolutionFollowUp } from '@/lib/agent-launch-follow-ups' type ReviewHeaderComponentProps = { review: ChecksPanelReview isRefreshing: boolean @@ -331,9 +332,18 @@ export function ChecksPanelActiveContent({ // Why: claims the ack payload when the tab exists; the host writes still wait for delivery. onLaunchAccepted={handleLaunchAccepted} onLaunchAborted={handleLaunchAborted} - onLaunched={() => { + // Why: resolving threads and posting replies must survive a reload mid-launch, once. + {...(agentComposerState?.actionId === 'resolveComments' && + agentComposerState.commentResolution + ? { + durableFollowUp: reviewCommentsResolutionFollowUp( + agentComposerState.commentResolution + ) + } + : {})} + onLaunched={(launch) => { // Why: prompt delivery succeeded — the only point at which host replies/resolves may run. - consumeClaimedCommentResolutionAfterDeliveryRef.current() + consumeClaimedCommentResolutionAfterDeliveryRef.current(launch) if (agentComposerState?.actionId === 'resolveConflicts') { toast.success( translate( diff --git a/src/renderer/src/components/right-sidebar/checks-panel/use-checks-panel-ai-acknowledgement.tsx b/src/renderer/src/components/right-sidebar/checks-panel/use-checks-panel-ai-acknowledgement.tsx index d1441f09ff42..886ea7181fc1 100644 --- a/src/renderer/src/components/right-sidebar/checks-panel/use-checks-panel-ai-acknowledgement.tsx +++ b/src/renderer/src/components/right-sidebar/checks-panel/use-checks-panel-ai-acknowledgement.tsx @@ -1,24 +1,14 @@ import { useCallback, useEffect, useRef } from 'react' -import { toast } from 'sonner' -import { mergePRCommentIntoList } from '@/store/github/pr-comment-cache' import { - acknowledgePRCommentsAfterAiLaunch, - attachPRReviewReplyParent, - canPostPRReviewThreadReply, checksPanelReviewStableKey, - hasPRCommentGroupNeedingReply, - resolvePRReviewReplyThreadId, setPendingPRCommentAiAck, takePendingPRCommentAiAck } from '../pr-comments-ai-launch-ack' import type { ChecksPanelControllerState } from './use-checks-panel-controller-state' import type { ChecksPanelReviewDataState } from './use-checks-panel-review-data' import type { ChecksPanelPollingState } from './use-checks-panel-polling' -import { buildSnapshottedThreadResolver } from '../pr-comment-snapshotted-thread-resolver' -import { markPRCommentThreadResolved } from '../pr-comment-thread-resolution' -import { resolveGitLabMRDiscussionForChecks } from './gitlab-review-client' +import { acknowledgeCommentResolutionAfterDelivery } from '../pr-comment-resolution-after-delivery' import { clearPRCommentsListSelection } from '../pr-comments-list-selection' -import { translate } from '@/i18n/i18n' import type { ChecksAgentComposerState } from './panel-state-types' import type { ChecksPanelReview } from '../checks-panel-review' @@ -87,169 +77,22 @@ export function useChecksPanelAiAcknowledgement(model: ChecksPanelAiAcknowledgem clearSentCommentSelection(resolution.reviewContextKey) // Why: ignore headSha churn; only abort resolve/UI refresh if the user left this PR/panel. const launchStableKey = checksPanelReviewStableKey(resolution.reviewContextKey) - // Why: the host calls keep the snapshotted target, but every UI mutation must belong - // to the review the panel is showing now — otherwise replies land in another PR's list. - const isPanelStillOnLaunchReview = (): boolean => - checksPanelReviewStableKey(asyncResultKeyRef.current) === launchStableKey - const githubTarget = resolution.githubTarget - const canReplyOnHost = resolution.provider === 'github' && githubTarget != null - // Why: only GitHub posts fixing replies today; a GitLab MR reaching replied=0 is expected, - // and a missing reply target only matters when something in the selection needs a reply. - let lastHostError = - resolution.provider === 'github' && - githubTarget == null && - hasPRCommentGroupNeedingReply(resolution.selectedGroups) - ? translate( - 'auto.components.right.sidebar.ChecksPanel.7e4b2a19c0', - 'Could not resolve the GitHub PR to reply on.' - ) - : undefined - const resolveSnapshottedThread = buildSnapshottedThreadResolver({ - provider: resolution.provider, - githubResolveTarget: resolution.githubResolveTarget, - gitlabTarget: resolution.gitlabTarget, - resolveReviewThread, - resolveGitLabDiscussion: (args) => - resolveGitLabMRDiscussionForChecks({ ...args, settings }), - isPanelStillOnLaunchReview, - onResolvedOptimistically: (threadId) => { - setComments((prev) => markPRCommentThreadResolved(prev, threadId, true)) + await acknowledgeCommentResolutionAfterDelivery(resolution, { + actions: { + resolveReviewThread, + addPRReviewCommentReply, + addPRConversationComment, + settings }, - onResolveFailed: ({ threadId, error }) => { - lastHostError = - error || - translate( - 'auto.components.right.sidebar.ChecksPanel.430f1a62d4', - 'Could not resolve the selected thread on the host.' - ) - console.warn('Post-launch thread resolve failed:', threadId, error) - } - }) - const counts = await acknowledgePRCommentsAfterAiLaunch({ - groups: resolution.selectedGroups, - deps: { - resolveThread: resolveSnapshottedThread, - canReply: canReplyOnHost, - replyInThread: async (comment, body) => { - if (!githubTarget || !canPostPRReviewThreadReply(comment)) { - return false - } - try { - const parentThreadId = - resolvePRReviewReplyThreadId({ - parent: comment, - existingComments: commentsRef.current - }) ?? comment.threadId - const result = await addPRReviewCommentReply( - githubTarget.repoPath, - githubTarget.prNumber, - comment.id, - body, - { - repoId: githubTarget.repoId, - prRepo: githubTarget.prRepo, - threadId: parentThreadId, - path: comment.path, - line: comment.line - } - ) - if (result.ok) { - // Why: force threadId/path onto the optimistic row so the sidebar groups it - // under the parent immediately (API payload may omit them). - if (isPanelStillOnLaunchReview()) { - setComments((prev) => - mergePRCommentIntoList( - prev, - attachPRReviewReplyParent(result.comment, { - ...comment, - threadId: parentThreadId - }) - ) - ) - } - return true - } - lastHostError = result.error - console.warn('In-thread fixing reply failed:', result.error) - return false - } catch (err) { - lastHostError = err instanceof Error ? err.message : String(err) - console.warn('Failed to post in-thread fixing reply for review comment:', err) - return false - } - }, - // Why: CodeRabbit / review-summary / conversation comments have no nested-reply - // API, so the ack sends one combined body for all of them. - replyAsConversation: async (body) => { - if (!githubTarget) { - return false - } - try { - const result = await addPRConversationComment( - githubTarget.repoPath, - githubTarget.prNumber, - body, - { - repoId: githubTarget.repoId, - prRepo: githubTarget.prRepo - } - ) - if (result.ok) { - if (isPanelStillOnLaunchReview()) { - setComments((prev) => mergePRCommentIntoList(prev, result.comment)) - } - return true - } - lastHostError = result.error - console.warn('Conversation fixing reply failed:', result.error) - return false - } catch (err) { - lastHostError = err instanceof Error ? err.message : String(err) - console.warn('Failed to post conversation fixing reply for review comment:', err) - return false - } - } - } + view: { + isStillOnLaunchReview: () => + checksPanelReviewStableKey(asyncResultKeyRef.current) === launchStableKey, + existingComments: () => commentsRef.current, + setComments, + refresh: () => refreshCommentsAfterBulkResolve(resolution.provider) + }, + refreshCache: async () => {} }) - - if (isPanelStillOnLaunchReview()) { - await refreshCommentsAfterBulkResolve(resolution.provider) - } - - // Why: surface the underlying API error when replies were possible but none landed. - // Resolvable threads are acked by resolving, so replied=0 is correct when nothing needed one. - const repliedNoneDespiteHostSupport = - canReplyOnHost && - counts.replied === 0 && - hasPRCommentGroupNeedingReply(resolution.selectedGroups) - if (counts.failed > 0 || repliedNoneDespiteHostSupport || lastHostError) { - toast.error( - translate( - 'auto.components.right.sidebar.ChecksPanel.f273f2271c', - 'Started the agent. Marked {{value0}} resolved, replied to {{value1}}, skipped {{value2}}, failed {{value3}}.{{value4}}', - { - value0: counts.resolved, - value1: counts.replied, - value2: counts.skipped, - value3: counts.failed, - value4: lastHostError ? ` ${lastHostError}` : '' - } - ) - ) - return - } - toast.success( - translate( - 'auto.components.right.sidebar.ChecksPanel.aa95b81a3a', - 'Started the agent. Marked {{value0}} resolved, replied to {{value1}}, skipped {{value2}}, failed {{value3}}.', - { - value0: counts.resolved, - value1: counts.replied, - value2: counts.skipped, - value3: counts.failed - } - ) - ) }, [ addPRConversationComment, @@ -302,37 +145,33 @@ export function useChecksPanelAiAcknowledgement(model: ChecksPanelAiAcknowledgem ]) /** Prompt reached the agent: only now may Orca write to the host. */ - const consumeClaimedCommentResolutionAfterDelivery = useCallback((): void => { - const resolution = - claimedCommentResolutionRef.current ?? - takePendingPRCommentAiAck() ?? - pendingCommentResolutionRef.current - claimedCommentResolutionRef.current = null - pendingCommentResolutionRef.current = null - commentResolutionLaunchAcceptedRef.current = false - if (!resolution) { - setCommentResolutionAckBusyNow(false) - return - } - setCommentResolutionAckBusyNow(true) - void resolveSelectedThreadsAfterLaunch(resolution) - .catch((err) => { - console.warn('Failed to resolve/reply on selected review comments after AI launch:', err) - toast.error( - translate( - 'auto.components.right.sidebar.ChecksPanel.495b2f8c4b', - 'Started the agent, but could not resolve or reply on the selected comments.' - ) - ) - }) - .finally(() => setCommentResolutionAckBusyNow(false)) - }, [ - resolveSelectedThreadsAfterLaunch, - setCommentResolutionAckBusyNow, - claimedCommentResolutionRef, - pendingCommentResolutionRef, - commentResolutionLaunchAcceptedRef - ]) + const consumeClaimedCommentResolutionAfterDelivery = useCallback( + (launch?: { followUpDeferred?: boolean }): void => { + const resolution = + claimedCommentResolutionRef.current ?? + takePendingPRCommentAiAck() ?? + pendingCommentResolutionRef.current + claimedCommentResolutionRef.current = null + pendingCommentResolutionRef.current = null + commentResolutionLaunchAcceptedRef.current = false + // Left on the launch's record for the next start to run: posting here would post twice. + if (!resolution || launch?.followUpDeferred) { + setCommentResolutionAckBusyNow(false) + return + } + setCommentResolutionAckBusyNow(true) + void resolveSelectedThreadsAfterLaunch(resolution).finally(() => + setCommentResolutionAckBusyNow(false) + ) + }, + [ + resolveSelectedThreadsAfterLaunch, + setCommentResolutionAckBusyNow, + claimedCommentResolutionRef, + pendingCommentResolutionRef, + commentResolutionLaunchAcceptedRef + ] + ) // Why: auto-start can capture a stale callback; always call the latest consumer. const consumeClaimedCommentResolutionAfterDeliveryRef = useRef( consumeClaimedCommentResolutionAfterDelivery diff --git a/src/renderer/src/components/right-sidebar/pr-comment-groups-in-flight.ts b/src/renderer/src/components/right-sidebar/pr-comment-groups-in-flight.ts new file mode 100644 index 000000000000..e8222d9fc9fd --- /dev/null +++ b/src/renderer/src/components/right-sidebar/pr-comment-groups-in-flight.ts @@ -0,0 +1,23 @@ +import { + holdNotesForSend, + isNoteInFlight, + useNotesInFlightVersion +} from '@/lib/notes-send-in-flight' + +// The holds notes use, for review threads a launch still on its way will resolve: a thread shown as +// selectable meanwhile could be sent twice. +const key = (groupId: string): string => `pr-comment-group:${groupId}` + +export function holdPRCommentGroupsForSend( + groupIds: readonly string[], + settled: Promise +): void { + holdNotesForSend(groupIds.map(key), settled) +} + +export function isPRCommentGroupInFlight(groupId: string): boolean { + return isNoteInFlight(key(groupId)) +} + +/** Changes whenever a hold starts or ends, for memos that filter by `isPRCommentGroupInFlight`. */ +export const usePRCommentGroupsInFlightVersion = useNotesInFlightVersion diff --git a/src/renderer/src/components/right-sidebar/pr-comment-resolution-after-delivery.ts b/src/renderer/src/components/right-sidebar/pr-comment-resolution-after-delivery.ts new file mode 100644 index 000000000000..572d8b4beb00 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/pr-comment-resolution-after-delivery.ts @@ -0,0 +1,223 @@ +import { toast } from 'sonner' +import { translate } from '@/i18n/i18n' +import { mergePRCommentIntoList } from '@/store/github/pr-comment-cache' +import type { AppState } from '@/store/types' +import type { PRComment } from '../../../../shared/github/comment-types' +import { + acknowledgePRCommentsAfterAiLaunch, + attachPRReviewReplyParent, + canPostPRReviewThreadReply, + hasPRCommentGroupNeedingReply, + resolvePRReviewReplyThreadId, + type PendingPRCommentAiAck +} from './pr-comments-ai-launch-ack' +import { buildSnapshottedThreadResolver } from './pr-comment-snapshotted-thread-resolver' +import { markPRCommentThreadResolved } from './pr-comment-thread-resolution' +import { resolveGitLabMRDiscussionForChecks } from './checks-panel/gitlab-review-client' + +export type CommentResolutionAfterDeliveryDeps = { + actions: Pick< + AppState, + 'resolveReviewThread' | 'addPRReviewCommentReply' | 'addPRConversationComment' | 'settings' + > + /** The Checks panel showing this review now; absent when none is (a window that reloaded). */ + view?: { + isStillOnLaunchReview: () => boolean + existingComments: () => readonly PRComment[] + setComments: (update: (previous: PRComment[]) => PRComment[]) => void + refresh: () => Promise + } + /** With no panel: refresh the cached comments, so a panel opened later shows them resolved. */ + refreshCache: () => Promise +} + +/** + * Resolves the threads a Checks "Resolve comments" launch sent and posts its "fixing" replies, + * once its prompt reached the agent. The one implementation, for the panel's own click and for a + * window that reloaded mid-launch and runs the click's recorded follow-up. + */ +export async function acknowledgeCommentResolutionAfterDelivery( + resolution: PendingPRCommentAiAck, + deps: CommentResolutionAfterDeliveryDeps +): Promise { + try { + await acknowledge(resolution, deps) + } catch (error) { + console.warn('Failed to resolve/reply on selected review comments after AI launch:', error) + toast.error( + translate( + 'auto.components.right.sidebar.ChecksPanel.495b2f8c4b', + 'Started the agent, but could not resolve or reply on the selected comments.' + ) + ) + } +} + +async function acknowledge( + resolution: PendingPRCommentAiAck, + { actions, view, refreshCache }: CommentResolutionAfterDeliveryDeps +): Promise { + // Why: the host calls keep the snapshotted target, but every UI mutation must belong to the + // review the panel is showing now — otherwise replies land in another PR's list. + const isPanelStillOnLaunchReview = (): boolean => view?.isStillOnLaunchReview() ?? false + const githubTarget = resolution.githubTarget + const canReplyOnHost = resolution.provider === 'github' && githubTarget != null + // Why: only GitHub posts fixing replies today; a GitLab MR reaching replied=0 is expected, + // and a missing reply target only matters when something in the selection needs a reply. + let lastHostError = + resolution.provider === 'github' && + githubTarget == null && + hasPRCommentGroupNeedingReply(resolution.selectedGroups) + ? translate( + 'auto.components.right.sidebar.ChecksPanel.7e4b2a19c0', + 'Could not resolve the GitHub PR to reply on.' + ) + : undefined + const resolveSnapshottedThread = buildSnapshottedThreadResolver({ + provider: resolution.provider, + githubResolveTarget: resolution.githubResolveTarget, + gitlabTarget: resolution.gitlabTarget, + resolveReviewThread: actions.resolveReviewThread, + resolveGitLabDiscussion: (args) => + resolveGitLabMRDiscussionForChecks({ ...args, settings: actions.settings }), + isPanelStillOnLaunchReview, + onResolvedOptimistically: (threadId) => { + view?.setComments((prev) => markPRCommentThreadResolved(prev, threadId, true)) + }, + onResolveFailed: ({ threadId, error }) => { + lastHostError = + error || + translate( + 'auto.components.right.sidebar.ChecksPanel.430f1a62d4', + 'Could not resolve the selected thread on the host.' + ) + console.warn('Post-launch thread resolve failed:', threadId, error) + } + }) + const counts = await acknowledgePRCommentsAfterAiLaunch({ + groups: resolution.selectedGroups, + deps: { + resolveThread: resolveSnapshottedThread, + canReply: canReplyOnHost, + replyInThread: async (comment, body) => { + if (!githubTarget || !canPostPRReviewThreadReply(comment)) { + return false + } + try { + const parentThreadId = + resolvePRReviewReplyThreadId({ + parent: comment, + existingComments: view?.existingComments() ?? [] + }) ?? comment.threadId + const result = await actions.addPRReviewCommentReply( + githubTarget.repoPath, + githubTarget.prNumber, + comment.id, + body, + { + repoId: githubTarget.repoId, + prRepo: githubTarget.prRepo, + threadId: parentThreadId, + path: comment.path, + line: comment.line + } + ) + if (result.ok) { + // Why: force threadId/path onto the optimistic row so the sidebar groups it + // under the parent immediately (API payload may omit them). + if (isPanelStillOnLaunchReview()) { + view?.setComments((prev) => + mergePRCommentIntoList( + prev, + attachPRReviewReplyParent(result.comment, { + ...comment, + threadId: parentThreadId + }) + ) + ) + } + return true + } + lastHostError = result.error + console.warn('In-thread fixing reply failed:', result.error) + return false + } catch (err) { + lastHostError = err instanceof Error ? err.message : String(err) + console.warn('Failed to post in-thread fixing reply for review comment:', err) + return false + } + }, + // Why: CodeRabbit / review-summary / conversation comments have no nested-reply + // API, so the ack sends one combined body for all of them. + replyAsConversation: async (body) => { + if (!githubTarget) { + return false + } + try { + const result = await actions.addPRConversationComment( + githubTarget.repoPath, + githubTarget.prNumber, + body, + { repoId: githubTarget.repoId, prRepo: githubTarget.prRepo } + ) + if (result.ok) { + if (isPanelStillOnLaunchReview()) { + view?.setComments((prev) => mergePRCommentIntoList(prev, result.comment)) + } + return true + } + lastHostError = result.error + console.warn('Conversation fixing reply failed:', result.error) + return false + } catch (err) { + lastHostError = err instanceof Error ? err.message : String(err) + console.warn('Failed to post conversation fixing reply for review comment:', err) + return false + } + } + } + }) + + if (view) { + if (isPanelStillOnLaunchReview()) { + await view.refresh() + } + } else { + await refreshCache() + } + + // Why: surface the underlying API error when replies were possible but none landed. + // Resolvable threads are acked by resolving, so replied=0 is correct when nothing needed one. + const repliedNoneDespiteHostSupport = + canReplyOnHost && + counts.replied === 0 && + hasPRCommentGroupNeedingReply(resolution.selectedGroups) + if (counts.failed > 0 || repliedNoneDespiteHostSupport || lastHostError) { + toast.error( + translate( + 'auto.components.right.sidebar.ChecksPanel.f273f2271c', + 'Started the agent. Marked {{value0}} resolved, replied to {{value1}}, skipped {{value2}}, failed {{value3}}.{{value4}}', + { + value0: counts.resolved, + value1: counts.replied, + value2: counts.skipped, + value3: counts.failed, + value4: lastHostError ? ` ${lastHostError}` : '' + } + ) + ) + return + } + toast.success( + translate( + 'auto.components.right.sidebar.ChecksPanel.aa95b81a3a', + 'Started the agent. Marked {{value0}} resolved, replied to {{value1}}, skipped {{value2}}, failed {{value3}}.', + { + value0: counts.resolved, + value1: counts.replied, + value2: counts.skipped, + value3: counts.failed + } + ) + ) +} diff --git a/src/renderer/src/components/right-sidebar/pr-comments-list-selection.ts b/src/renderer/src/components/right-sidebar/pr-comments-list-selection.ts index b82c2b4c2770..01c3e58a3d77 100644 --- a/src/renderer/src/components/right-sidebar/pr-comments-list-selection.ts +++ b/src/renderer/src/components/right-sidebar/pr-comments-list-selection.ts @@ -5,6 +5,10 @@ import { type PRCommentGroup } from '../../../../shared/pr-comment-groups' import { isPRCommentGroupQueueableForAI } from '@/lib/pr-comment-action-state' +import { + isPRCommentGroupInFlight, + usePRCommentGroupsInFlightVersion +} from './pr-comment-groups-in-flight' import type { PRComment } from '../../../../shared/github/comment-types' export type PRCommentsListSelection = { @@ -155,9 +159,17 @@ export function usePRCommentsListSelection( // Why: selectable groups come from the unfiltered list so switching the // audience filter doesn't silently drop already-selected comments. const canonicalGroups = useMemo(() => groupPRComments(comments), [comments]) + const inFlightVersion = usePRCommentGroupsInFlightVersion() + // A thread a launch still on its way will resolve is not offered again (`agent-launch-follow-ups`). const selectableGroups = useMemo( - () => canonicalGroups.filter(isPRCommentGroupQueueableForAI), - [canonicalGroups] + () => + canonicalGroups.filter( + (group) => + isPRCommentGroupQueueableForAI(group) && + !isPRCommentGroupInFlight(getPRCommentGroupId(group)) + ), + // oxlint-disable-next-line react-hooks/exhaustive-deps -- inFlightVersion re-reads the holds. + [canonicalGroups, inFlightVersion] ) const selectableGroupsById = useMemo(() => { const map = new Map() diff --git a/src/renderer/src/components/right-sidebar/review-comments-resolution-follow-up.ts b/src/renderer/src/components/right-sidebar/review-comments-resolution-follow-up.ts new file mode 100644 index 000000000000..b718aa27d4d4 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/review-comments-resolution-follow-up.ts @@ -0,0 +1,28 @@ +import { useAppStore } from '@/store' +import type { PendingPRCommentAiAck } from './pr-comments-ai-launch-ack' +import { acknowledgeCommentResolutionAfterDelivery } from './pr-comment-resolution-after-delivery' + +/** + * The Checks panel's "resolve the selected comments" follow-up, for a window that reloaded while + * its launch ran (`agent-launch-follow-ups`): the panel's own implementation, with no panel on + * screen, then a fresh fetch of the PR's comments so a panel opened later shows them resolved. + */ +export function runReviewCommentsResolutionFollowUp( + resolution: PendingPRCommentAiAck +): Promise { + const store = useAppStore.getState() + const target = resolution.githubResolveTarget + return acknowledgeCommentResolutionAfterDelivery(resolution, { + actions: store, + refreshCache: async () => { + // GitHub's cached comments; GitLab's are not refreshed here. + if (resolution.provider === 'github' && target) { + await useAppStore.getState().fetchPRComments(target.repoPath, target.prNumber, { + force: true, + repoId: target.repoId, + ...(target.prRepo ? { prRepo: target.prRepo } : {}) + }) + } + } + }) +} diff --git a/src/renderer/src/components/right-sidebar/runSourceControlAgentActionStart.ts b/src/renderer/src/components/right-sidebar/runSourceControlAgentActionStart.ts index c01d43da511e..10ea4c3522fb 100644 --- a/src/renderer/src/components/right-sidebar/runSourceControlAgentActionStart.ts +++ b/src/renderer/src/components/right-sidebar/runSourceControlAgentActionStart.ts @@ -14,6 +14,7 @@ import { toast } from 'sonner' import { translate } from '@/i18n/i18n' import { sourceControlActionRecipeMatchesTarget } from './source-control-action-recipe-match' import { resolveSourceControlAgentSaveTarget } from './source-control-agent-action-dialog-support' +import type { AgentLaunchFollowUp } from '../../../../shared/agent-launch-follow-up' type RunSourceControlAgentActionStartArgs = { selectedAgent: TuiAgent @@ -52,7 +53,9 @@ type RunSourceControlAgentActionStartArgs = { onLaunchAccepted?: () => void /** Fires when a launch that already reported onLaunchAccepted failed to deliver its prompt. */ onLaunchAborted?: () => void - onLaunched?: () => void + onLaunched?: (launch?: { followUpDeferred?: boolean }) => void + /** Recorded on a host launch so a reload mid-launch still runs `onLaunched`'s host writes once. */ + durableFollowUp?: AgentLaunchFollowUp onClose: () => void } @@ -77,10 +80,13 @@ export async function runSourceControlAgentActionStart({ onLaunchAccepted, onLaunchAborted, onLaunched, + durableFollowUp, onClose }: RunSourceControlAgentActionStartArgs): Promise { let launched = false let launchFailureNotified = false + // The recorded follow-up was left for the next start to take: running it here could run it twice. + let followUpDeferred = false let launchAcceptedNotified = false // Why: `undefined` is what makes the launch fall back to the global Agents arguments; // an empty string would beat that fallback and silently suppress them. @@ -111,7 +117,8 @@ export async function runSourceControlAgentActionStart({ agentArgs: launchAgentArgs, promptDelivery, launchPlatform, - launchSource + launchSource, + ...(durableFollowUp ? { durableFollowUp } : {}) }) launched = Boolean(result) if (result?.surface.kind === 'local-terminal') { @@ -127,6 +134,7 @@ export async function runSourceControlAgentActionStart({ const deliveryResult = await result.promptDeliveryResult launched = deliveryResult.delivered launchFailureNotified = deliveryResult.failureNotified + followUpDeferred = deliveryResult.followUpDeferred === true } catch (error) { console.error('promptDeliveryResult rejected', error) launched = false @@ -173,7 +181,7 @@ export async function runSourceControlAgentActionStart({ console.error('onSaveAgentDefault failed', error) } } - onLaunched?.() + onLaunched?.(followUpDeferred ? { followUpDeferred } : undefined) onClose() return true } diff --git a/src/renderer/src/components/right-sidebar/useSourceControlAgentActionDialog.ts b/src/renderer/src/components/right-sidebar/useSourceControlAgentActionDialog.ts index 60e7a45bb1bb..a0efcea3aa56 100644 --- a/src/renderer/src/components/right-sidebar/useSourceControlAgentActionDialog.ts +++ b/src/renderer/src/components/right-sidebar/useSourceControlAgentActionDialog.ts @@ -44,6 +44,7 @@ export function useSourceControlAgentActionDialog({ onLaunchAccepted, onLaunchAborted, onLaunched, + durableFollowUp, onStart }: SourceControlAgentActionDialogProps): UseSourceControlAgentActionDialogResult { const settings = useAppStore((state) => state.settings) @@ -212,6 +213,7 @@ export function useSourceControlAgentActionDialog({ onLaunchAccepted, onLaunchAborted, onLaunched, + ...(durableFollowUp ? { durableFollowUp } : {}), onClose: closeDialog }) diff --git a/src/renderer/src/components/right-sidebar/useSourceControlAgentActionStart.ts b/src/renderer/src/components/right-sidebar/useSourceControlAgentActionStart.ts index 4ca1519080cf..b977befa801a 100644 --- a/src/renderer/src/components/right-sidebar/useSourceControlAgentActionStart.ts +++ b/src/renderer/src/components/right-sidebar/useSourceControlAgentActionStart.ts @@ -12,6 +12,7 @@ import { buildSourceControlAgentDeliveryPlan } from './buildSourceControlAgentDe import type { SourceControlAgentActionDeliveryPlanState } from './SourceControlAgentActionDialogForm' import { runSourceControlAgentActionStart } from './runSourceControlAgentActionStart' import { buildSourceControlAgentConnectionErrorPlan } from './source-control-agent-action-dialog-support' +import type { AgentLaunchFollowUp } from '../../../../shared/agent-launch-follow-up' type UseSourceControlAgentActionStartArgs = { selectedAgent: TuiAgent | null @@ -50,7 +51,8 @@ type UseSourceControlAgentActionStartArgs = { ) => void | Promise onLaunchAccepted?: () => void onLaunchAborted?: () => void - onLaunched?: () => void + onLaunched?: (launch?: { followUpDeferred?: boolean }) => void + durableFollowUp?: AgentLaunchFollowUp onClose: () => void } @@ -95,6 +97,7 @@ export function useSourceControlAgentActionStart({ onLaunchAccepted, onLaunchAborted, onLaunched, + durableFollowUp, onClose }: UseSourceControlAgentActionStartArgs): UseSourceControlAgentActionStartResult { const [deliveryPlan, setDeliveryPlan] = useState({ @@ -172,6 +175,7 @@ export function useSourceControlAgentActionStart({ onLaunchAccepted, onLaunchAborted, onLaunched, + ...(durableFollowUp ? { durableFollowUp } : {}), onClose: () => { resetDeliveryPlan() onClose() @@ -196,6 +200,7 @@ export function useSourceControlAgentActionStart({ onLaunchAborted, onLaunchAccepted, onLaunched, + durableFollowUp, onSaveAgentDefault, onStart, promptDelivery, diff --git a/src/renderer/src/components/tab-bar/QuickLaunchButton.tsx b/src/renderer/src/components/tab-bar/QuickLaunchButton.tsx index 3f15fe434181..81acf636b506 100644 --- a/src/renderer/src/components/tab-bar/QuickLaunchButton.tsx +++ b/src/renderer/src/components/tab-bar/QuickLaunchButton.tsx @@ -17,6 +17,7 @@ import { } from '../../../../shared/tui-agent-selection' import { translate } from '@/i18n/i18n' import { newAgentPromptOutcome } from '@/lib/new-agent-prompt-outcome' +import type { AgentLaunchFollowUp } from '../../../../shared/agent-launch-follow-up' export type QuickLaunchAgentMenuItemsProps = { worktreeId: string @@ -37,6 +38,8 @@ export type QuickLaunchAgentMenuItemsProps = { launchSource?: LaunchSource /** Called after a prompt is queued into the agent, or immediately for argv prompt launches. */ onPromptDelivered?: () => void + /** What `onPromptDelivered` does, recorded on the launch so a reload mid-launch still runs it. */ + durableFollowUp?: AgentLaunchFollowUp /** Given the launch's own delivery result while the prompt is still on its way. */ onPromptHandedOff?: (delivered: Promise) => void /** Nothing to send: e.g. every note is already on its way, so no agent is started. */ @@ -108,6 +111,7 @@ function QuickLaunchAgentMenuItemsInner({ promptDelivery, launchSource, onPromptDelivered, + durableFollowUp, onPromptHandedOff, disabled = false }: QuickLaunchAgentMenuItemsProps): React.JSX.Element | null { @@ -145,7 +149,8 @@ function QuickLaunchAgentMenuItemsInner({ ...(prompt !== undefined ? { prompt } : {}), ...(promptDelivery !== undefined ? { promptDelivery } : {}), ...(launchSource !== undefined ? { launchSource } : {}), - ...(onPromptDelivered !== undefined ? { onPromptDelivered } : {}) + ...(onPromptDelivered !== undefined ? { onPromptDelivered } : {}), + ...(durableFollowUp !== undefined ? { durableFollowUp } : {}) }) if (!result) { toast.error( @@ -202,6 +207,7 @@ function QuickLaunchAgentMenuItemsInner({ promptDelivery, launchSource, onPromptDelivered, + durableFollowUp, onPromptHandedOff, disabled ] diff --git a/src/renderer/src/lib/agent-launch-follow-ups.test.ts b/src/renderer/src/lib/agent-launch-follow-ups.test.ts new file mode 100644 index 000000000000..8099c6023c06 --- /dev/null +++ b/src/renderer/src/lib/agent-launch-follow-ups.test.ts @@ -0,0 +1,172 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { + AgentLaunchFollowUp, + AgentLaunchFollowUpTake +} from '../../../shared/agent-launch-follow-up' +import { AGENT_LAUNCH_FOLLOW_UPS_RUNTIME_CAPABILITY } from '../../../shared/agent-launch-runtime-capability' + +const state = vi.hoisted(() => { + const capabilities: string[] = [] + const answers: unknown[] = [] + const calls: unknown[] = [] + const held: { keys: readonly unknown[]; settled: Promise }[] = [] + return { + capabilities, + answers, + calls, + held, + clearDeliveredDiffComments: vi.fn(async () => true), + runResolution: vi.fn(async () => {}) + } +}) + +vi.mock('@/store', () => ({ + useAppStore: { + getState: () => ({ clearDeliveredDiffComments: state.clearDeliveredDiffComments }) + } +})) +vi.mock('@/runtime/local-runtime-capabilities', () => ({ + ensureLocalRuntimeCapabilities: async () => state.capabilities, + readLocalRuntimeCapabilitiesOrUnknown: () => state.capabilities +})) +vi.mock('@/runtime/runtime-rpc-client', () => ({ + callRuntimeRpc: vi.fn(async (_target: unknown, method: string, params: unknown) => { + state.calls.push([method, params]) + const answer = state.answers.shift() + if (answer instanceof Error) { + throw answer + } + return answer ?? { taken: [], pending: [] } + }) +})) +vi.mock('@/lib/notes-send-in-flight', () => ({ + diffCommentSendKey: (note: { id: string }) => note.id, + holdNotesForSend: (keys: readonly unknown[], settled: Promise) => + state.held.push({ keys, settled }) +})) +vi.mock('@/components/right-sidebar/review-comments-resolution-follow-up', () => ({ + runReviewCommentsResolutionFollowUp: state.runResolution +})) + +const { + recordableLaunchFollowUp, + reviewCommentsResolutionFollowUp, + reviewNotesDeliveredFollowUp, + runRecordedLaunchFollowUps +} = await import('./agent-launch-follow-ups') + +const NOTE = { id: 'n1', body: 'fix this', filePath: 'a.ts', lineNumber: 3 } +const NOTES = reviewNotesDeliveredFollowUp('wt-1', [NOTE]) +const RESOLUTION = reviewCommentsResolutionFollowUp({ + reviewContextKey: 'pr-1', + provider: 'github', + selectedGroups: [] +}) + +function taken(operationId: string, followUp: AgentLaunchFollowUp, promptHandedOver = true) { + return { operationId, followUp, promptHandedOver, composerUnobserved: false } +} + +beforeEach(() => { + state.capabilities = [AGENT_LAUNCH_FOLLOW_UPS_RUNTIME_CAPABILITY] + state.answers = [] + state.calls = [] + state.held = [] + state.clearDeliveredDiffComments.mockClear() + state.runResolution.mockClear() +}) +afterEach(() => vi.restoreAllMocks()) + +describe('a window that reloaded mid-launch runs what its launches recorded', () => { + it('runs each follow-up whose prompt was handed over, once', async () => { + const take: AgentLaunchFollowUpTake = { + taken: [taken('op-1', NOTES), taken('op-2', RESOLUTION)], + pending: [] + } + state.answers.push(take) + await runRecordedLaunchFollowUps() + expect(state.clearDeliveredDiffComments).toHaveBeenCalledExactlyOnceWith('wt-1', [NOTE]) + expect(state.runResolution).toHaveBeenCalledOnce() + expect(state.calls).toEqual([['agent.takeLaunchFollowUps', {}]]) + }) + + it('runs nothing for a launch whose prompt never arrived', async () => { + state.answers.push({ taken: [taken('op-1', NOTES, false)], pending: [] }) + await runRecordedLaunchFollowUps() + expect(state.clearDeliveredDiffComments).not.toHaveBeenCalled() + }) + + it('discards a kind or version this build does not know, and a payload it cannot read', async () => { + state.answers.push({ + taken: [ + taken('op-1', { ...NOTES, version: 2 }), + taken('op-2', { kind: 'unknown-kind', version: 1, payload: {} }), + taken('op-3', { ...NOTES, payload: { worktreeId: 'wt-1' } }) + ], + pending: [] + }) + await runRecordedLaunchFollowUps() + expect(state.clearDeliveredDiffComments).not.toHaveBeenCalled() + }) + + it('keeps notes still waiting on their prompt unsendable, then runs them once taken', async () => { + state.answers.push( + { taken: [], pending: [{ operationId: 'op-1', followUp: NOTES }] }, + { taken: [], pending: [{ operationId: 'op-1', followUp: NOTES }] }, + { taken: [taken('op-1', NOTES)], pending: [] } + ) + let released = false + const run = runRecordedLaunchFollowUps({ wait: async () => {}, pollMs: 1 }) + await vi.waitFor(() => expect(state.held).toHaveLength(1)) + void state.held[0]!.settled.then(() => (released = true)) + await run + expect(state.held[0]!.keys).toEqual(['n1']) + expect(released).toBe(true) + expect(state.clearDeliveredDiffComments).toHaveBeenCalledOnce() + // The re-ask names the launch, so a click this window makes meanwhile stays its own. + expect(state.calls.slice(1)).toEqual([ + ['agent.takeLaunchFollowUps', { operationId: 'op-1' }], + ['agent.takeLaunchFollowUps', { operationId: 'op-1' }] + ]) + }) + + it('stops asking at its bound and leaves the follow-up on the record', async () => { + const pending = { taken: [], pending: [{ operationId: 'op-1', followUp: NOTES }] } + state.answers.push(pending, pending, pending, pending) + let now = 0 + vi.spyOn(Date, 'now').mockImplementation(() => now) + await runRecordedLaunchFollowUps({ + maxWaitMs: 3, + pollMs: 1, + wait: async (ms) => { + now += ms + } + }) + expect(state.clearDeliveredDiffComments).not.toHaveBeenCalled() + expect(state.calls).toHaveLength(4) + await expect(state.held[0]!.settled).resolves.toBeUndefined() + }) + + it('asks nothing of a host that does not record follow-ups', async () => { + state.capabilities = [] + await runRecordedLaunchFollowUps() + expect(state.calls).toEqual([]) + }) + + it('treats a host that cannot answer as nothing recorded', async () => { + state.answers.push(new Error('method_not_found')) + await expect(runRecordedLaunchFollowUps()).resolves.toBeUndefined() + expect(state.clearDeliveredDiffComments).not.toHaveBeenCalled() + }) +}) + +describe('which follow-ups a launch records', () => { + it('only on a host that keeps them, and only under the size cap', () => { + expect(recordableLaunchFollowUp(NOTES)).toBe(NOTES) + expect(recordableLaunchFollowUp(undefined)).toBeUndefined() + const huge = reviewNotesDeliveredFollowUp('wt-1', [{ ...NOTE, body: 'x'.repeat(300 * 1024) }]) + expect(recordableLaunchFollowUp(huge)).toBeUndefined() + state.capabilities = [] + expect(recordableLaunchFollowUp(NOTES)).toBeUndefined() + }) +}) diff --git a/src/renderer/src/lib/agent-launch-follow-ups.ts b/src/renderer/src/lib/agent-launch-follow-ups.ts new file mode 100644 index 000000000000..f452e824af98 --- /dev/null +++ b/src/renderer/src/lib/agent-launch-follow-ups.ts @@ -0,0 +1,260 @@ +/** + * The follow-ups a desktop click records on its launch (`agent-launch-follow-up`), and the window's + * side of taking them: the live click takes its own before it runs it, and a window that reloaded + * mid-launch takes the rest at startup. Whoever takes one runs it; a kind or version this build does + * not know is discarded, never misread. + * + * Only the follow-ups a reload would otherwise turn into a second send are recorded: notes deleted + * once sent, and review threads resolved with replies. A toast is not worth a record. + */ + +import { useAppStore } from '@/store' +import { callRuntimeRpc } from '@/runtime/runtime-rpc-client' +import { + ensureLocalRuntimeCapabilities, + readLocalRuntimeCapabilitiesOrUnknown +} from '@/runtime/local-runtime-capabilities' +import { diffCommentSendKey, holdNotesForSend } from '@/lib/notes-send-in-flight' +import type { DiffCommentDeliverySnapshot } from '@/store/slices/diffComments' +import type { PendingPRCommentAiAck } from '@/components/right-sidebar/pr-comments-ai-launch-ack' +import { runReviewCommentsResolutionFollowUp } from '@/components/right-sidebar/review-comments-resolution-follow-up' +import { holdPRCommentGroupsForSend } from '@/components/right-sidebar/pr-comment-groups-in-flight' +import { getPRCommentGroupId, type PRCommentGroup } from '../../../shared/pr-comment-groups' +import type { PRComment } from '../../../shared/github/comment-types' +import { + agentLaunchFollowUpFits, + type AgentLaunchFollowUp, + type AgentLaunchFollowUpTake +} from '../../../shared/agent-launch-follow-up' +import { AGENT_LAUNCH_FOLLOW_UPS_RUNTIME_CAPABILITY } from '../../../shared/agent-launch-runtime-capability' + +const REVIEW_NOTES_DELIVERED = 'review-notes-delivered' +const REVIEW_COMMENTS_RESOLUTION = 'review-comments-resolution' + +export function reviewNotesDeliveredFollowUp( + worktreeId: string, + notes: readonly DiffCommentDeliverySnapshot[] +): AgentLaunchFollowUp { + // Only what the removal matches on, so a note carries no more than it must. + const snapshots = notes.map( + ({ id, body, filePath, lineNumber, startLine, selectedText, source }) => ({ + id, + body, + filePath, + lineNumber, + ...(startLine !== undefined ? { startLine } : {}), + ...(selectedText !== undefined ? { selectedText } : {}), + ...(source !== undefined ? { source } : {}) + }) + ) + return { kind: REVIEW_NOTES_DELIVERED, version: 1, payload: { worktreeId, notes: snapshots } } +} + +export function reviewCommentsResolutionFollowUp( + resolution: PendingPRCommentAiAck +): AgentLaunchFollowUp { + // Without comment bodies: resolving and replying never read them, and they are other people's + // words that would otherwise sit in the launch record. + const withoutBody = (comment: PRComment): PRComment => ({ ...comment, body: '' }) + const selectedGroups = resolution.selectedGroups.map((group): PRCommentGroup => + group.kind === 'thread' + ? { ...group, root: withoutBody(group.root), replies: group.replies.map(withoutBody) } + : { ...group, comment: withoutBody(group.comment) } + ) + return { + kind: REVIEW_COMMENTS_RESOLUTION, + version: 1, + payload: { ...resolution, selectedGroups } + } +} + +function isNotesPayload( + value: unknown +): value is { worktreeId: string; notes: DiffCommentDeliverySnapshot[] } { + return ( + typeof value === 'object' && + value !== null && + 'worktreeId' in value && + typeof value.worktreeId === 'string' && + 'notes' in value && + Array.isArray(value.notes) && + value.notes.every( + (note: unknown) => + typeof note === 'object' && + note !== null && + 'id' in note && + typeof note.id === 'string' && + 'body' in note && + typeof note.body === 'string' + ) + ) +} + +function isResolutionPayload(value: unknown): value is PendingPRCommentAiAck { + return ( + typeof value === 'object' && + value !== null && + 'reviewContextKey' in value && + typeof value.reviewContextKey === 'string' && + 'provider' in value && + typeof value.provider === 'string' && + 'selectedGroups' in value && + Array.isArray(value.selectedGroups) + ) +} + +/** How to run a recorded follow-up, and keep what it acts on unsendable until it runs. */ +function readFollowUp(followUp: AgentLaunchFollowUp): { + run: () => Promise + holdUntil?: (settled: Promise) => void +} | null { + if (followUp.kind === REVIEW_NOTES_DELIVERED && followUp.version === 1) { + const payload = followUp.payload + if (!isNotesPayload(payload)) { + return null + } + return { + run: async () => { + await useAppStore.getState().clearDeliveredDiffComments(payload.worktreeId, payload.notes) + }, + holdUntil: (settled) => holdNotesForSend(payload.notes.map(diffCommentSendKey), settled) + } + } + if (followUp.kind === REVIEW_COMMENTS_RESOLUTION && followUp.version === 1) { + const payload = followUp.payload + return isResolutionPayload(payload) + ? { + run: () => runReviewCommentsResolutionFollowUp(payload), + holdUntil: (settled) => + holdPRCommentGroupsForSend(payload.selectedGroups.map(getPRCommentGroupId), settled) + } + : null + } + return null +} + +/** Whether this window's host records follow-ups; an older one runs them live, as before. */ +export function hostRecordsLaunchFollowUps(): boolean { + return ( + readLocalRuntimeCapabilitiesOrUnknown()?.includes(AGENT_LAUNCH_FOLLOW_UPS_RUNTIME_CAPABILITY) ?? + false + ) +} + +/** The follow-up a launch request may carry: recorded only where the host keeps it and it fits. */ +export function recordableLaunchFollowUp( + followUp: AgentLaunchFollowUp | undefined +): AgentLaunchFollowUp | undefined { + return followUp && hostRecordsLaunchFollowUps() && agentLaunchFollowUpFits(followUp) + ? followUp + : undefined +} + +/** Null when the host could not answer: the caller treats that as nothing recorded. */ +export async function takeLaunchFollowUps( + operationId?: string +): Promise { + try { + return await callRuntimeRpc( + { kind: 'local' }, + 'agent.takeLaunchFollowUps', + operationId ? { operationId } : {} + ) + } catch (error) { + console.warn('[agent-launch] could not take launch follow-ups', error) + return null + } +} + +async function runTaken(take: AgentLaunchFollowUpTake['taken']): Promise { + for (const entry of take) { + const followUp = entry.promptHandedOver ? readFollowUp(entry.followUp) : null + await followUp?.run().catch((error: unknown) => { + console.warn(`[agent-launch] the ${entry.followUp.kind} follow-up failed`, error) + }) + } +} + +/** How long past the host's own deadline a waiting window takes once more, then lets go. */ +const PAST_DEADLINE_GRACE_MS = 15_000 +/** For a pending entry whose deadline the host could not say: its owed-prompt deadline. */ +const FALLBACK_WAIT_MS = 5 * 60_000 + +export type RecordedLaunchFollowUpClock = { + now: () => number + /** Runs `run` after `ms`; returns its cancel. */ + schedule: (ms: number, run: () => void) => () => void + /** The host's word that a launch settled its prompt; returns its unsubscribe. */ + onSettled: (listener: (operationId: string) => void) => () => void +} + +const WINDOW_CLOCK: RecordedLaunchFollowUpClock = { + now: () => Date.now(), + schedule: (ms, run) => { + const timer = setTimeout(run, ms) + return () => clearTimeout(timer) + }, + onSettled: (listener) => + window.api.ui.onAgentLaunchPromptSettled?.((event) => listener(event.operationId)) ?? (() => {}) +} + +/** + * Startup: holds what launches still on their way act on, before anything else, then runs the + * follow-ups launches finished while this window was gone. Each one still pending is taken when + * the host says its prompt settled, or once more just past the host's own deadline; one still + * pending then stays on its row for the next start, and nothing is held any more. + */ +export async function runRecordedLaunchFollowUps( + clock: RecordedLaunchFollowUpClock = WINDOW_CLOCK +): Promise { + const capabilities = await ensureLocalRuntimeCapabilities() + if (!capabilities?.includes(AGENT_LAUNCH_FOLLOW_UPS_RUNTIME_CAPABILITY)) { + return + } + const first = await takeLaunchFollowUps() + if (!first) { + return + } + const waiting = new Map void>() + for (const { operationId, followUp } of first.pending) { + let release: () => void = () => {} + readFollowUp(followUp)?.holdUntil?.(new Promise((resolve) => (release = resolve))) + waiting.set(operationId, release) + } + await runTaken(first.taken) + if (waiting.size === 0) { + return + } + await new Promise((done) => { + const cancels: (() => void)[] = [] + const finish = (operationId: string): void => { + waiting.get(operationId)?.() + waiting.delete(operationId) + if (waiting.size === 0) { + cancels.forEach((cancel) => cancel()) + done() + } + } + const takeOne = async (operationId: string, last: boolean): Promise => { + if (!waiting.has(operationId)) { + return + } + const take = await takeLaunchFollowUps(operationId) + if (take && take.pending.length === 0) { + await runTaken(take.taken) + finish(operationId) + } else if (last) { + finish(operationId) + } + } + cancels.push(clock.onSettled((operationId) => void takeOne(operationId, false))) + for (const { operationId, deadline } of first.pending) { + const waitMs = (deadline ?? clock.now() + FALLBACK_WAIT_MS) - clock.now() + cancels.push( + clock.schedule(Math.max(0, waitMs) + PAST_DEADLINE_GRACE_MS, () => { + void takeOne(operationId, true) + }) + ) + } + }) +} diff --git a/src/renderer/src/lib/agent-launch-through-host.ts b/src/renderer/src/lib/agent-launch-through-host.ts index 085368fb7723..b23066c7959a 100644 --- a/src/renderer/src/lib/agent-launch-through-host.ts +++ b/src/renderer/src/lib/agent-launch-through-host.ts @@ -30,6 +30,7 @@ import type { TuiAgent } from '../../../shared/tui-agent' import type { LaunchSource } from '../../../shared/telemetry-events' import type { Tab } from '../../../shared/tab-types' import type { SessionOptionValue } from '../../../shared/native-chat-session-options' +import type { AgentLaunchFollowUp } from '../../../shared/agent-launch-follow-up' export type HostAgentLaunchArgs = { agent: TuiAgent @@ -40,6 +41,8 @@ export type HostAgentLaunchArgs = { prompt: string /** What the host pastes and submits once the agent is ready; it can differ from `prompt`. */ hostPrompt?: string + /** Recorded on the launch, so a window that reloads mid-launch still runs it once. */ + followUp?: AgentLaunchFollowUp /** Absent uses the settings default; `null` means no arguments. */ agentArgs?: string | null cwd?: string @@ -113,6 +116,7 @@ function launchParams(args: HostAgentLaunchArgs) { } } : {}), + ...(args.followUp ? { followUp: args.followUp } : {}), ...(args.agentArgs !== undefined ? { agentArgs: args.agentArgs } : {}), ...(args.cwd ? { cwd: args.cwd } : {}), ...stringSessionOptions(args.sessionOptions), @@ -191,6 +195,7 @@ export function windowMakesHostLaunchTab(): boolean { export function launchAgentThroughHost(args: HostAgentLaunchArgs): { tabId: string + operationId: string outcome: Promise } { const store = useAppStore.getState() @@ -198,10 +203,11 @@ export function launchAgentThroughHost(args: HostAgentLaunchArgs): { const leafId = createBrowserUuid() // Before the tab exists, so its first mount already waits. const releaseHold = holdAgentLaunchPaneSpawn(tabId, leafId) + // A new click is a new operation; the pane is this click's too. + const operationId = createAgentSessionOperationId() const send = callRuntimeRpc({ kind: 'local' }, 'agent.launchReplay', { ...launchParams(args), - // A new click is a new operation; the pane is this click's too. - operationId: createAgentSessionOperationId(), + operationId, paneKey: makePaneKey(tabId, leafId) }) store.createTab(args.worktreeId, args.groupId, undefined, { @@ -218,5 +224,5 @@ export function launchAgentThroughHost(args: HostAgentLaunchArgs): { // Why: without it an activated launch can stay hidden behind an editor. store.setActiveTabType('terminal', args.worktreeId) persistAgentLaunchTabOrder(args.worktreeId, tabId) - return { tabId, outcome: settleLaunch(args, { tabId, leafId }, send, releaseHold) } + return { tabId, operationId, outcome: settleLaunch(args, { tabId, leafId }, send, releaseHold) } } 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 9cb7ccbbdefe..3d46dc5ed3a7 100644 --- a/src/renderer/src/lib/launch-agent-in-new-tab.ts +++ b/src/renderer/src/lib/launch-agent-in-new-tab.ts @@ -7,7 +7,8 @@ import { seedNativeChatLaunchDraftForAgentTab } from '@/lib/agent-launch-prompt- import { pasteAgentLaunchPromptOnceReady } from '@/lib/launch-agent-tab-prompt-paste' import { launchNewTabPromptThroughHost, - newTabPromptLaunchesThroughHost + newTabPromptLaunchesThroughHost, + type NewTabPromptDeliveryResult } from '@/lib/launch-agent-new-tab-host-route' import { initialAgentTabViewModeProps } from '@/lib/native-chat-initial-view-mode' import { isNativeChatTranscriptLocalReadable } from '@/lib/native-chat-transcript-readability' @@ -20,6 +21,7 @@ import { } from '../../../shared/tui-agent-launch-defaults' import { TUI_AGENT_CONFIG } from '../../../shared/tui-agent-config' import type { TuiAgent } from '../../../shared/tui-agent' +import type { AgentLaunchFollowUp } from '../../../shared/agent-launch-follow-up' import type { LaunchSource } from '../../../shared/telemetry-events' import { resolveAgentLaunchExecutionContext } from '@/lib/launch-agent-execution-context' import { resolveInitialNativeChatSessionOptions } from '@/components/native-chat/native-chat-launch-session-options' @@ -69,6 +71,9 @@ export type LaunchAgentInNewTabArgs = LaunchAgentInNewTabRequest & { * terminal route, whose readiness signal the client watches itself. */ onPromptDeliveryUnconfirmed?: () => void + /** What `onPromptDelivered` does, recorded on a host launch so a reload mid-launch still runs it + * once (`agent-launch-follow-ups`). */ + durableFollowUp?: AgentLaunchFollowUp /** Keep terminal launches in a floating workspace from taking global selection. */ activate?: boolean /** The launch seeds a workspace being opened, so its PTY spawn must not reshuffle Recent. */ @@ -91,7 +96,7 @@ export type LaunchAgentInNewTabResult = { surface: AgentLaunchSurface startupPlan: AgentStartupPlan pasteDraftAfterLaunch: boolean - promptDeliveryResult?: Promise<{ delivered: boolean; failureNotified: boolean }> + promptDeliveryResult?: Promise /** Structured route only: what the launch did once it settled. The call stays synchronous. */ structuredSettlement?: Promise } | null @@ -265,6 +270,7 @@ function launchAgentInNewTabInternal(args: LaunchAgentInNewTabArgs): LaunchAgent ...(pendingActivationSpawn ? { pendingActivationSpawn: true } : {}), ...(initialViewModeProps.viewMode ? { viewMode: initialViewModeProps.viewMode } : {}), pasteContent: pasteDraftAfterLaunch, + ...(args.durableFollowUp ? { durableFollowUp: args.durableFollowUp } : {}), ...(onPromptDelivered ? { onPromptDelivered } : {}), ...(onPromptDeliveryUnconfirmed ? { onPromptDeliveryUnconfirmed } : {}) }) diff --git a/src/renderer/src/lib/launch-agent-new-tab-host-route.test.ts b/src/renderer/src/lib/launch-agent-new-tab-host-route.test.ts index 1d0f12c5e153..9f28915b0262 100644 --- a/src/renderer/src/lib/launch-agent-new-tab-host-route.test.ts +++ b/src/renderer/src/lib/launch-agent-new-tab-host-route.test.ts @@ -20,6 +20,21 @@ vi.mock('@/lib/command-code-prompt-status-seed', () => ({ })) const toast = vi.hoisted(() => ({ error: vi.fn() })) vi.mock('sonner', () => ({ toast })) +const followUps = vi.hoisted(() => { + const order: string[] = [] + return { + hostRecords: true, + order, + takeLaunchFollowUps: vi.fn(async (_operationId?: string) => ({ taken: [], pending: [] })) + } +}) +vi.mock('@/lib/agent-launch-follow-ups', () => ({ + recordableLaunchFollowUp: (followUp: unknown) => (followUps.hostRecords ? followUp : undefined), + takeLaunchFollowUps: async (operationId?: string) => { + followUps.order.push('take') + return followUps.takeLaunchFollowUps(operationId) + } +})) const { launchNewTabPromptThroughHost, newTabPromptLaunchesThroughHost } = await import('./launch-agent-new-tab-host-route') @@ -29,7 +44,7 @@ const TAB = '9b1deb4d-3b7d-4bad-9bdd-2b0d7b3dcb6d' function deferredOutcome() { let resolve!: (outcome: HostAgentLaunchOutcome) => void const promise = new Promise((done) => (resolve = done)) - host.launchAgentThroughHost.mockReturnValue({ tabId: TAB, outcome: promise }) + host.launchAgentThroughHost.mockReturnValue({ tabId: TAB, operationId: 'op-1', outcome: promise }) return resolve } @@ -47,6 +62,8 @@ function launch( beforeEach(() => { vi.clearAllMocks() + followUps.hostRecords = true + followUps.order = [] notice.wasNotified.mockReturnValue(true) host.windowMakesHostLaunchTab.mockReturnValue(true) }) @@ -151,6 +168,52 @@ describe('an AI button launched through the host, which delivers its prompt', () }) }) +describe('a click whose follow-up is recorded on its launch', () => { + const FOLLOW_UP = { kind: 'review-notes-delivered', version: 1, payload: {} } + + it('records it, then takes it off the record before it runs, so a reload never runs it again', async () => { + deferredOutcome()({ + kind: 'started', + prompt: { delivery: 'submit', outcome: 'handed-to-terminal' } + }) + await launchNewTabPromptThroughHost({ + agent: 'claude', + worktreeId: 'wt-1', + prompt: 'p', + pasteContent: 'p', + durableFollowUp: FOLLOW_UP, + onPromptDelivered: () => followUps.order.push('follow-up') + }).promptDeliveryResult + expect(host.launchAgentThroughHost).toHaveBeenCalledWith( + expect.objectContaining({ followUp: FOLLOW_UP }) + ) + expect(followUps.takeLaunchFollowUps).toHaveBeenCalledWith('op-1') + expect(followUps.order).toEqual(['take', 'follow-up']) + }) + + it('on a host that does not record follow-ups, runs it live and records nothing, as before', async () => { + followUps.hostRecords = false + deferredOutcome()({ + kind: 'started', + prompt: { delivery: 'submit', outcome: 'handed-to-terminal' } + }) + const onPromptDelivered = vi.fn() + await launchNewTabPromptThroughHost({ + agent: 'claude', + worktreeId: 'wt-1', + prompt: 'p', + pasteContent: 'p', + durableFollowUp: FOLLOW_UP, + onPromptDelivered + }).promptDeliveryResult + expect(host.launchAgentThroughHost).toHaveBeenCalledWith( + expect.not.objectContaining({ followUp: expect.anything() }) + ) + expect(followUps.takeLaunchFollowUps).not.toHaveBeenCalled() + expect(onPromptDelivered).toHaveBeenCalledOnce() + }) +}) + describe('which new agent tabs start through the host', () => { it('an AI button whose prompt is pasted once ready, in a terminal this window makes', () => { expect( diff --git a/src/renderer/src/lib/launch-agent-new-tab-host-route.ts b/src/renderer/src/lib/launch-agent-new-tab-host-route.ts index c868aa795e07..a19ad2054a5d 100644 --- a/src/renderer/src/lib/launch-agent-new-tab-host-route.ts +++ b/src/renderer/src/lib/launch-agent-new-tab-host-route.ts @@ -13,6 +13,15 @@ import { seedCommandCodeSubmittedPromptStatus } from '@/lib/command-code-prompt- import { isNativeChatSupportedAgent } from '@/lib/native-chat-supported-agent' import type { AgentLaunchPromptReceipt } from '../../../shared/agent-launch-intent' import type { TuiAgent } from '../../../shared/tui-agent' +import type { AgentLaunchFollowUp } from '../../../shared/agent-launch-follow-up' +import { recordableLaunchFollowUp, takeLaunchFollowUps } from '@/lib/agent-launch-follow-ups' + +/** `followUpDeferred`: the click's recorded follow-up was left for the next start; don't run it. */ +export type NewTabPromptDeliveryResult = { + delivered: boolean + failureNotified: boolean + followUpDeferred?: boolean +} /** * Whether a new agent tab starts through the host's `agent.launch`: an AI button's launch, whose @@ -64,15 +73,26 @@ function seedChatCopy(tabId: string, agent: TuiAgent, text: string, createdAt: n * What the host's answer means for the click: the follow-ups run on a prompt it handed to the * agent, and a prompt it could not hand over gets main's own "wasn't sent" notice. */ -function settleHostPrompt( +async function settleHostPrompt( args: HostAgentLaunchArgs & { onPromptDelivered?: () => void onPromptDeliveryUnconfirmed?: () => void }, - tabId: string, + launch: { tabId: string; operationId: string; followUpRecorded: boolean }, receipt: AgentLaunchPromptReceipt | undefined, seeded: boolean -): { delivered: boolean; failureNotified: boolean } { +): Promise { + const { tabId } = launch + // Runs only once this click took it off the record: a take that failed leaves it there, for the + // next start to run, and running it here too would run it twice. + let followUpRunsHere = true + if (launch.followUpRecorded) { + const take = await takeLaunchFollowUps(launch.operationId) + followUpRunsHere = + take?.taken.some( + (entry) => entry.operationId === launch.operationId && entry.promptHandedOver + ) ?? false + } if (receipt?.outcome === 'handed-to-terminal') { if (receipt.composerUnobserved) { args.onPromptDeliveryUnconfirmed?.() @@ -81,6 +101,9 @@ function settleHostPrompt( // Command Code has no prompt-submit hook; seed working when the prompt is submitted. seedCommandCodeSubmittedPromptStatus(args.worktreeId, tabId, args.prompt) } + if (!followUpRunsHere) { + return { delivered: true, failureNotified: false, followUpDeferred: true } + } args.onPromptDelivered?.() return { delivered: true, failureNotified: false } } @@ -112,6 +135,8 @@ export function launchNewTabPromptThroughHost( pasteContent: string onPromptDelivered?: () => void onPromptDeliveryUnconfirmed?: () => void + /** What `onPromptDelivered` does, recorded so a reload mid-launch still runs it once. */ + durableFollowUp?: AgentLaunchFollowUp } ): { tabId: string @@ -121,9 +146,15 @@ export function launchNewTabPromptThroughHost( pasteContent, onPromptDelivered: _delivered, onPromptDeliveryUnconfirmed: _u, + durableFollowUp, ...launch } = args - const { tabId, outcome } = launchAgentThroughHost({ ...launch, hostPrompt: pasteContent }) + const followUp = recordableLaunchFollowUp(durableFollowUp) + const { tabId, operationId, outcome } = launchAgentThroughHost({ + ...launch, + hostPrompt: pasteContent, + ...(followUp ? { followUp } : {}) + }) // Stamped at the click: the chat view matches the agent's turn to a copy made before it. const clickedAt = Date.now() const promptDeliveryResult = outcome.then((launched) => { @@ -131,7 +162,12 @@ export function launchNewTabPromptThroughHost( // Seeded once the host started the agent, as main's paste seeded it: a launch that never // started leaves no chat copy behind. const seeded = seedChatCopy(tabId, args.agent, pasteContent, clickedAt) - return settleHostPrompt(args, tabId, launched.prompt, seeded) + return settleHostPrompt( + args, + { tabId, operationId, followUpRecorded: followUp !== undefined }, + launched.prompt, + seeded + ) } // The pane, or this notice for a tab that went, already says why: never a second notice. showLaunchNotStartedNotice(launched, args.prompt) diff --git a/src/shared/agent-launch-follow-up.ts b/src/shared/agent-launch-follow-up.ts new file mode 100644 index 000000000000..71853db79f86 --- /dev/null +++ b/src/shared/agent-launch-follow-up.ts @@ -0,0 +1,57 @@ +/** + * What a desktop click still owes once its launch's prompt has landed: notes to mark sent, review + * threads to resolve. Recorded on the launch's own row (the operation ledger) so a window that + * reloads mid-launch can still run it, once. It dies when the window takes it, or with the row. + * + * Opaque to the host. `kind` and `version` say how the window reads `payload`; a window that does + * not know the pair discards it rather than misreading a shape another build wrote. + */ +export type AgentLaunchFollowUp = { + kind: string + version: number + payload: unknown +} + +/** Bigger than any notes or comment selection a click sends; anything larger stays live-only. */ +export const AGENT_LAUNCH_FOLLOW_UP_MAX_BYTES = 256 * 1024 + +const encoder = new TextEncoder() + +/** Whether the follow-up may be recorded; one that is not still runs live, as before. */ +export function agentLaunchFollowUpFits(followUp: AgentLaunchFollowUp): boolean { + try { + return encoder.encode(JSON.stringify(followUp)).byteLength <= AGENT_LAUNCH_FOLLOW_UP_MAX_BYTES + } catch { + return false + } +} + +export function isAgentLaunchFollowUp(value: unknown): value is AgentLaunchFollowUp { + return ( + typeof value === 'object' && + value !== null && + 'kind' in value && + typeof value.kind === 'string' && + 'version' in value && + typeof value.version === 'number' && + 'payload' in value + ) +} + +/** What the window's take returns for one launch. */ +export type TakenAgentLaunchFollowUp = { + operationId: string + followUp: AgentLaunchFollowUp + /** The launch handed its prompt to the agent; false when it never did, and nothing may run. */ + promptHandedOver: boolean + /** The host wrote the prompt without seeing the agent's composer ready. */ + composerUnobserved: boolean +} + +export type AgentLaunchFollowUpTake = { + /** Removed from the record: this caller now owns running them. */ + taken: TakenAgentLaunchFollowUp[] + /** Still waiting on their prompt; left on the record, read-only. `deadline`: when the host stops + * trying to deliver it, for a waiting window to stop holding what it acts on. */ + pending: { operationId: string; followUp: AgentLaunchFollowUp; deadline?: number }[] +} diff --git a/src/shared/agent-launch-owed-prompt.ts b/src/shared/agent-launch-owed-prompt.ts new file mode 100644 index 000000000000..472103dbb014 --- /dev/null +++ b/src/shared/agent-launch-owed-prompt.ts @@ -0,0 +1,16 @@ +import type { TuiAgent } from './tui-agent' + +/** + * `owed`: no byte of the prompt has been written, so a host that restarts may still deliver it. + * `writing`: the write may have begun, so nothing may write it again. + */ +export type AgentLaunchOwedPrompt = + | { + state: 'owed' + text: string + agent: TuiAgent + deadline: number + /** The PTY the launch started its agent in: a resume pastes into that one only. */ + terminal: { ptyId: string; incarnationId: string | null } | null + } + | { state: 'writing'; since: number } diff --git a/src/shared/agent-launch-runtime-capability.ts b/src/shared/agent-launch-runtime-capability.ts index 060766828e27..7a0aa4afbf23 100644 --- a/src/shared/agent-launch-runtime-capability.ts +++ b/src/shared/agent-launch-runtime-capability.ts @@ -38,6 +38,10 @@ export const AGENT_LAUNCH_PROMPT_UNCONFIRMED_RUNTIME_CAPABILITY = // may publish the launch's tab before it admits the launch. export const AGENT_LAUNCH_PLACEMENT_RUNTIME_CAPABILITY = 'agent.launch.placement.v1' as const +// Host-advertised: records a launch's `followUp` on its row and answers `agent.takeLaunchFollowUps`. +// A window on a host without it runs its follow-ups live, on the launch's answer, as before. +export const AGENT_LAUNCH_FOLLOW_UPS_RUNTIME_CAPABILITY = 'agent.launch.follow-ups.v1' as const + // Client-advertised only: the client reads a listed launch tab with no terminal yet as "not // started", never as proof its agent runs. The host publishes a paired caller's tab before the // spawn only for a client that says so. @@ -54,5 +58,6 @@ export const AGENT_LAUNCH_RUNTIME_CAPABILITIES = [ AGENT_LAUNCH_REPLAY_REQUIRED_RUNTIME_CAPABILITY, AGENT_LAUNCH_PROMPT_CARRY_RUNTIME_CAPABILITY, AGENT_LAUNCH_PROMPT_UNCONFIRMED_RUNTIME_CAPABILITY, - AGENT_LAUNCH_PLACEMENT_RUNTIME_CAPABILITY + AGENT_LAUNCH_PLACEMENT_RUNTIME_CAPABILITY, + AGENT_LAUNCH_FOLLOW_UPS_RUNTIME_CAPABILITY ] as const diff --git a/src/shared/agent-session-operation-ledger.ts b/src/shared/agent-session-operation-ledger.ts index 8324ec95364b..41b17fe8e956 100644 --- a/src/shared/agent-session-operation-ledger.ts +++ b/src/shared/agent-session-operation-ledger.ts @@ -26,7 +26,8 @@ import { isAgentSessionConversationCommandResult, type AgentSessionConversationCommandResult } from './agent-session-conversation-command' -import type { TuiAgent } from './tui-agent' +import type { AgentLaunchOwedPrompt } from './agent-launch-owed-prompt' +import type { AgentLaunchFollowUp } from './agent-launch-follow-up' export const AGENT_SESSION_DURABLE_OPERATION_PER_CLIENT_LIMIT = 512 export const AGENT_SESSION_DURABLE_OPERATION_GLOBAL_LIMIT = 4_096 @@ -93,23 +94,11 @@ export type AgentSessionOperationRow = { * Not checked by `isAgentSessionOperationRow`, like `ownedPane`. */ promptDelivery?: AgentLaunchOwedPrompt + /** What the click owes once its prompt lands (`agent-launch-follow-up`): written with the claim, + * gone when its caller takes it or with the row. Unchecked by the row validator, like `ownedPane`. */ + launchFollowUp?: AgentLaunchFollowUp } -/** - * `owed`: no byte of the prompt has been written, so a host that restarts may still deliver it. - * `writing`: the write may have begun, so nothing may write it again. - */ -export type AgentLaunchOwedPrompt = - | { - state: 'owed' - text: string - agent: TuiAgent - deadline: number - /** The PTY the launch started its agent in: a resume pastes into that one only. */ - terminal: { ptyId: string; incarnationId: string | null } | null - } - | { state: 'writing'; since: number } - /** Unexpired rows naming this pane as theirs. */ export function listAgentSessionOperationRowsOwningPane( rows: Iterable, @@ -220,7 +209,12 @@ export type AgentSessionOperationClaim = */ export function claimAgentSessionOperation( rows: ReadonlyMap, - args: { callerKey: string; operationId: string; ownedPane?: AgentSessionOperationOwnedPane } + args: { + callerKey: string + operationId: string + ownedPane?: AgentSessionOperationOwnedPane + launchFollowUp?: AgentLaunchFollowUp + } ): { rows: Map; claim: AgentSessionOperationClaim } { const key = agentSessionOperationKey(args.callerKey, args.operationId) const existing = rows.get(key) @@ -233,7 +227,8 @@ export function claimAgentSessionOperation( const claimed: AgentSessionOperationRow = { ...existing, outcome: { status: 'unknown' }, - ...(args.ownedPane ? { ownedPane: args.ownedPane } : {}) + ...(args.ownedPane ? { ownedPane: args.ownedPane } : {}), + ...(args.launchFollowUp ? { launchFollowUp: args.launchFollowUp } : {}) } const next = new Map(rows) next.set(key, claimed) diff --git a/src/shared/rpc-contract/agent-launch-params.ts b/src/shared/rpc-contract/agent-launch-params.ts index bdac99d5219f..15d4fbecdd4e 100644 --- a/src/shared/rpc-contract/agent-launch-params.ts +++ b/src/shared/rpc-contract/agent-launch-params.ts @@ -83,6 +83,18 @@ export const AgentLaunchFields = z.object({ transport: z.enum(['paste']).optional() }) .optional(), + /** + * What the caller still owes once the prompt lands (`agent-launch-follow-up`), kept on the launch's + * record so a reloaded window can run it once. Opaque to the host, and not part of the + * fingerprint: it is what the caller does next, not what the launch does. + */ + followUp: z + .object({ + kind: z.string().min(1).max(80), + version: z.number().int().min(1), + payload: z.unknown() + }) + .optional(), /** A chat seeds the options it accepts; a terminal launch reads the model, effort and mode. */ sessionOptions: z.record(z.string(), z.string()).optional(), reuseTerminal: z.object({ handle: z.string().min(1, 'Missing terminal handle') }).optional(), @@ -167,3 +179,8 @@ export type AgentLaunchParams = z.infer export const AgentLaunchReplay = AgentLaunchFields.required({ operationId: true }).superRefine( refuseSessionIdForAnotherAgent ) + +/** One launch's follow-up when the click is still on screen; all of the caller's after a reload. */ +export const AgentTakeLaunchFollowUps = z.object({ + operationId: z.string().min(1).optional() +}) diff --git a/src/shared/rpc-contract/rpc-params-catalog.generated.ts b/src/shared/rpc-contract/rpc-params-catalog.generated.ts index b8af9f409373..579bbe080548 100644 --- a/src/shared/rpc-contract/rpc-params-catalog.generated.ts +++ b/src/shared/rpc-contract/rpc-params-catalog.generated.ts @@ -38,7 +38,7 @@ import { SelectDataAccountParams } from './accounts-params' import { PrepareCodexForWslPaneParams } from './agent-hooks-params' -import { AgentLaunch, AgentLaunchReplay } from './agent-launch-params' +import { AgentLaunch, AgentLaunchReplay, AgentTakeLaunchFollowUps } from './agent-launch-params' import { CreateAgentSessionParams, EnsureAgentSessionParams } from './agent-session-params' import { AiVaultListSessionsParams, @@ -588,6 +588,7 @@ export const RPC_PARAMS_BY_METHOD = { 'accounts.unsubscribe': AccountsUnsubscribeParams, 'agent.launch': AgentLaunch, 'agent.launchReplay': AgentLaunchReplay, + 'agent.takeLaunchFollowUps': AgentTakeLaunchFollowUps, 'agentHooks.prepareCodexForWslPane': PrepareCodexForWslPaneParams, 'agentSession.acknowledgeAttention': AcknowledgeAttentionParams, 'agentSession.agents': AgentsParams, From 8ab49dfd20d80c729e145e33d6f42dc55251a8b9 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 7 Oct 2026 03:33:37 -0700 Subject: [PATCH 2/8] fix(agent-launch): never let a settle announcement throw; cover each follow-up path The host's word that a launch settled is bookkeeping: a missing notifier or an unreadable store leaves the follow-up for the window's next look, never fails the launch. Tests now cover the live click's take, the push and the deadline take, the held Checks threads, the shared resolution logic with and without a panel, the startup hook, and the notes and Checks wiring. --- .../agent-launch-follow-up-record.test.ts | 4 +- .../methods/agent-launch-follow-ups.test.ts | 13 +- .../rpc/methods/agent-launch-follow-ups.ts | 32 ++- .../rpc/methods/agent-launch.test-fixture.ts | 1 + .../use-recorded-launch-follow-ups.test.tsx | 25 ++ .../components/editor/NotesSendMenu.test.tsx | 23 ++ .../ReviewNotesSendMenuContent.test.tsx | 6 + .../panel-content-rendering.test.tsx | 217 ++++++++++++------ ...-comment-resolution-after-delivery.test.ts | 121 ++++++++++ .../pr-comments-list-selection.test.tsx | 21 ++ .../runSourceControlAgentActionStart.test.ts | 19 ++ .../QuickLaunchButton.launch-status.test.tsx | 22 ++ .../src/lib/agent-launch-follow-ups.test.ts | 169 +++++++++++--- .../launch-agent-new-tab-host-route.test.ts | 48 +++- 14 files changed, 599 insertions(+), 122 deletions(-) create mode 100644 src/renderer/src/app-shell/use-recorded-launch-follow-ups.test.tsx create mode 100644 src/renderer/src/components/right-sidebar/pr-comment-resolution-after-delivery.test.ts diff --git a/src/main/runtime/agent-launch-follow-up-record.test.ts b/src/main/runtime/agent-launch-follow-up-record.test.ts index 56a7b8240c1b..2738d66dae67 100644 --- a/src/main/runtime/agent-launch-follow-up-record.test.ts +++ b/src/main/runtime/agent-launch-follow-up-record.test.ts @@ -93,12 +93,12 @@ describe('taking a click’s follow-up off its launch’s row', () => { it('reports a follow-up whose prompt is still owed, read-only', () => { const state = rows( row('op-1', { - promptDelivery: { state: 'owed', text: 't', agent: 'claude', deadline: 9_000 } + promptDelivery: { state: 'owed', text: 't', agent: 'claude', deadline: 9_000, terminal: null } }) ) expect(take(state)).toEqual({ taken: [], - pending: [{ operationId: 'op-1', followUp: FOLLOW_UP }] + pending: [{ operationId: 'op-1', followUp: FOLLOW_UP, deadline: 9_000 }] }) expect(state.operations.get(agentSessionOperationKey(DESKTOP, 'op-1'))?.launchFollowUp).toEqual( FOLLOW_UP diff --git a/src/main/runtime/rpc/methods/agent-launch-follow-ups.test.ts b/src/main/runtime/rpc/methods/agent-launch-follow-ups.test.ts index 4bf9df83e545..3566d65ccc3d 100644 --- a/src/main/runtime/rpc/methods/agent-launch-follow-ups.test.ts +++ b/src/main/runtime/rpc/methods/agent-launch-follow-ups.test.ts @@ -31,7 +31,8 @@ vi.mock('./agent-launch-terminal-prompt', () => ({ })) const { AGENT_LAUNCH_METHODS } = await import('./agent-launch') -const { AGENT_LAUNCH_FOLLOW_UP_METHODS } = await import('./agent-launch-follow-ups') +const { AGENT_LAUNCH_FOLLOW_UP_METHODS, announceSettledLaunchFollowUps } = + await import('./agent-launch-follow-ups') const AGENT_LAUNCH_REPLAY = methodNamed(AGENT_LAUNCH_METHODS, 'agent.launchReplay') const TAKE = methodNamed(AGENT_LAUNCH_FOLLOW_UP_METHODS, 'agent.takeLaunchFollowUps') @@ -114,6 +115,16 @@ describe('a click’s follow-up on its launch’s record', () => { await expect(take(runtime, DESKTOP)).resolves.toEqual({ taken: [], pending: [] }) }) + it('tells the window once the launch settled its prompt, so a reloaded window takes it', async () => { + const runtime = host() + await launch(runtime) + expect(runtime.reportAgentLaunchPromptSettled).toHaveBeenCalledExactlyOnceWith(OPERATION_ID) + await take(runtime, DESKTOP, OPERATION_ID) + // Taken: a later sweep has nothing to announce. + announceSettledLaunchFollowUps(runtime) + expect(runtime.reportAgentLaunchPromptSettled).toHaveBeenCalledOnce() + }) + it('is recorded for the desktop only: a phone, which never takes one, records nothing', async () => { const runtime = host() await launch(runtime, LAUNCH, PHONE) diff --git a/src/main/runtime/rpc/methods/agent-launch-follow-ups.ts b/src/main/runtime/rpc/methods/agent-launch-follow-ups.ts index 0bc98a693e09..99ff18046148 100644 --- a/src/main/runtime/rpc/methods/agent-launch-follow-ups.ts +++ b/src/main/runtime/rpc/methods/agent-launch-follow-ups.ts @@ -21,18 +21,26 @@ import { agentLaunchOperationCallerKey } from './agent-launch-replay' * reloaded mid-launch takes each as its prompt settles, not at its next start. Only the desktop * records follow-ups. One the window is not waiting on is ignored there. */ -export function announceSettledLaunchFollowUps(runtime: OrcaRuntimeService): void { - const store = runtime.openedAgentSessionRecordStore() - if (!store) { - return - } - const desktop = rpcCallerOperationKey(DESKTOP_RPC_CALLER) - for (const operationId of listSettledLaunchFollowUps( - store.listOperationRows(), - desktop, - Date.now() - )) { - runtime.reportAgentLaunchPromptSettled(operationId) +export function announceSettledLaunchFollowUps( + runtime: Pick & + Partial> +): void { + try { + const store = runtime.openedAgentSessionRecordStore() + if (!store) { + return + } + const desktop = rpcCallerOperationKey(DESKTOP_RPC_CALLER) + for (const operationId of listSettledLaunchFollowUps( + store.listOperationRows(), + desktop, + Date.now() + )) { + runtime.reportAgentLaunchPromptSettled?.(operationId) + } + } catch (error) { + // Bookkeeping: a missed word leaves the follow-up for the window's next look. + console.warn('[agent-launch] could not announce settled launch follow-ups', error) } } diff --git a/src/main/runtime/rpc/methods/agent-launch.test-fixture.ts b/src/main/runtime/rpc/methods/agent-launch.test-fixture.ts index 81ddb34a06b5..5f9dd17c81c6 100644 --- a/src/main/runtime/rpc/methods/agent-launch.test-fixture.ts +++ b/src/main/runtime/rpc/methods/agent-launch.test-fixture.ts @@ -136,6 +136,7 @@ export function runtimeStub(options: AgentLaunchRuntimeStubOptions = {}) { }), showTerminal: vi.fn(async (handle: string) => ({ handle, worktreeId: 'wt-7' })), getTerminalHandleForPaneKey: vi.fn((paneKey: string) => handlesByPaneKey.get(paneKey) ?? null), + reportAgentLaunchPromptSettled: vi.fn((_operationId: string) => {}), getTerminalPtyIdentity: vi.fn( options.ptyIdentity ?? ((_handle: string) => ({ ptyId: 'pty-1', incarnationId: null })) ), diff --git a/src/renderer/src/app-shell/use-recorded-launch-follow-ups.test.tsx b/src/renderer/src/app-shell/use-recorded-launch-follow-ups.test.tsx new file mode 100644 index 000000000000..9685865ece76 --- /dev/null +++ b/src/renderer/src/app-shell/use-recorded-launch-follow-ups.test.tsx @@ -0,0 +1,25 @@ +// @vitest-environment happy-dom +import { act, renderHook } from '@testing-library/react' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const run = vi.hoisted(() => vi.fn(async () => {})) +vi.mock('../lib/agent-launch-follow-ups', () => ({ runRecordedLaunchFollowUps: run })) + +const { useAppStore } = await import('../store') +const { useRecordedLaunchFollowUps } = await import('./use-recorded-launch-follow-ups') + +beforeEach(() => { + run.mockClear() + useAppStore.setState({ workspaceSessionReady: false }) +}) + +describe('a window starting up takes what its launches recorded for it', () => { + it('once its worktrees and notes are loaded, and only once', () => { + const { rerender } = renderHook(() => useRecordedLaunchFollowUps()) + expect(run).not.toHaveBeenCalled() + act(() => useAppStore.setState({ workspaceSessionReady: true })) + expect(run).toHaveBeenCalledOnce() + rerender() + expect(run).toHaveBeenCalledOnce() + }) +}) diff --git a/src/renderer/src/components/editor/NotesSendMenu.test.tsx b/src/renderer/src/components/editor/NotesSendMenu.test.tsx index b87588508952..07f24d63b60f 100644 --- a/src/renderer/src/components/editor/NotesSendMenu.test.tsx +++ b/src/renderer/src/components/editor/NotesSendMenu.test.tsx @@ -3,6 +3,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import { buildNotesSendTargetModeId, NotesSendMenu } from './NotesSendMenu' import type { DiffCommentDeliverySnapshot } from '@/store/slices/diffComments' import { resetNotesInFlightForTests } from '@/lib/notes-send-in-flight' +import { reviewNotesDeliveredFollowUp } from '@/lib/agent-launch-follow-ups' type ReactElementLike = { type: unknown @@ -345,6 +346,28 @@ describe('NotesSendMenu', () => { }) }) + it('records on a new agent’s launch that the notes it sends are removed once delivered', () => { + const tree = renderMenu() + expect(findByType(tree, 'ReviewNotesSendMenuContent').props.durableFollowUp).toEqual( + reviewNotesDeliveredFollowUp('wt-1', [note('note-1')]) + ) + }) + + it('records each scope’s own notes when the menu offers several', () => { + const tree = renderMenu({ + scopes: [ + { id: 'all', label: 'All', notes: [note('a'), note('b')], formatPrompt: () => 'all' }, + { id: 'one', label: 'One', notes: [note('b')], formatPrompt: () => 'one' } + ] + }) + expect( + findAllByType(tree, 'ReviewNotesSendMenuContent').map((entry) => entry.props.durableFollowUp) + ).toEqual([ + reviewNotesDeliveredFollowUp('wt-1', [note('a'), note('b')]), + reviewNotesDeliveredFollowUp('wt-1', [note('b')]) + ]) + }) + it('switches running-agent target mode when a different scope is focused', () => { const tree = renderMenu({ defaultScopeId: 'file', diff --git a/src/renderer/src/components/editor/ReviewNotesSendMenuContent.test.tsx b/src/renderer/src/components/editor/ReviewNotesSendMenuContent.test.tsx index af4c1e984457..feaecdea0ebb 100644 --- a/src/renderer/src/components/editor/ReviewNotesSendMenuContent.test.tsx +++ b/src/renderer/src/components/editor/ReviewNotesSendMenuContent.test.tsx @@ -871,6 +871,12 @@ describe('ReviewNotesSendMenuContent', () => { }) }) + it('hands the new-agent launcher what to record on its launch', () => { + const durableFollowUp = { kind: 'review-notes-delivered', version: 1, payload: {} } + const launcher = findByType(render({ durableFollowUp }), 'QuickLaunchAgentMenuItems') + expect(launcher.props.durableFollowUp).toBe(durableFollowUp) + }) + // Every note already on its way leaves an empty prompt: no agent may start with no text. it('disables the new-agent launcher when there is nothing left to send', () => { expect(findByType(render({ prompt: '' }), 'QuickLaunchAgentMenuItems').props.disabled).toBe( diff --git a/src/renderer/src/components/right-sidebar/checks-panel/panel-content-rendering.test.tsx b/src/renderer/src/components/right-sidebar/checks-panel/panel-content-rendering.test.tsx index 5b587fbe4784..396a9510c102 100644 --- a/src/renderer/src/components/right-sidebar/checks-panel/panel-content-rendering.test.tsx +++ b/src/renderer/src/components/right-sidebar/checks-panel/panel-content-rendering.test.tsx @@ -5,15 +5,103 @@ import { afterEach, describe, expect, it, vi } from 'vitest' import type { ChecksPanelActiveContentModel } from './active-content-props' import type { ChecksPanelEmptyContentModel } from './empty-content-props' import { ChecksPanelReviewHeader } from '../ChecksPanel' +import { reviewCommentsResolutionFollowUp } from '@/lib/agent-launch-follow-ups' vi.mock('../HostedReviewActions', () => ({ default: () => null })) -vi.mock('../SourceControlAgentActionDialog', () => ({ SourceControlAgentActionDialog: () => null })) +const dialogProps = vi.hoisted(() => { + const props: { last: Record | null } = { last: null } + return props +}) +vi.mock('../SourceControlAgentActionDialog', () => ({ + SourceControlAgentActionDialog: (props: Record) => { + dialogProps.last = props + return null + } +})) import { ChecksPanelActiveContent } from './active-content' import { ChecksPanelEmptyContent } from './empty-content' afterEach(cleanup) +function activeContentModel() { + return { + activeConnectionId: null, + activeConflictReview: null, + activeGitLabReview: null, + activeReview: { + provider: 'github', + number: 42, + state: 'open', + title: 'Preserve mounted panel behavior', + url: 'https://github.com/orca/app/pull/42', + status: 'success', + updatedAt: '2026-08-23T00:00:00.000Z', + mergeable: 'MERGEABLE' + }, + activeSourceControlLaunchPlatform: 'darwin', + activeWorktree: null, + activeWorktreeId: 'worktree-1', + agentComposerState: null, + aiActionDisabledReason: undefined, + canTargetPRComments: false, + checks: [], + checksLoading: false, + claimedCommentResolutionRef: { current: null }, + commentResolutionLaunchAcceptedRef: { current: false }, + comments: [], + commentsDisabledReason: undefined, + commentsLoading: false, + commentsSelectionClearRequest: null, + conflictDetailsRefreshing: false, + consumeClaimedCommentResolutionAfterDeliveryRef: { current: vi.fn() }, + detachedHeadDisplay: null, + editingTitle: false, + getGitLabProjectRef: vi.fn(() => null), + handleAddPRComment: vi.fn(), + handleCancelEdit: vi.fn(), + handleDeleteComment: vi.fn(), + handleEditComment: vi.fn(), + handleFixChecksWithAI: vi.fn(), + handleLaunchAborted: vi.fn(), + handleLaunchAccepted: vi.fn(), + handleLinkAnotherReview: vi.fn(), + handleLoadCheckDetails: vi.fn(), + handleOpenPR: vi.fn(), + handleRefresh: vi.fn(), + handleOpenStackPR: vi.fn(), + handleReplyToComment: vi.fn(), + handleResolve: vi.fn(), + handleResolveCommentsWithAI: vi.fn(), + handleResolveConflictsWithAI: vi.fn(), + handleSaveTitle: vi.fn(), + handleSetReaction: vi.fn(), + handleStartEdit: vi.fn(), + handleTitleKeyDown: vi.fn(), + handleUnlinkReview: vi.fn(), + isFixingChecksWithAI: false, + isRefreshing: false, + isResolvingConflictsWithAI: false, + linkedGitLabMR: null, + pendingCommentResolutionRef: { current: null }, + pr: null, + prRefreshState: undefined, + repo: null, + refreshHostedReviewAfterMutation: vi.fn(), + resolveCommentsWithAIDisabledReason: undefined, + saveLaunchActionDefault: vi.fn(), + setAgentComposerState: vi.fn(), + setChecksPanelContentRef: vi.fn(), + settings: null, + sourceControlAiActionsVisible: false, + stateRequestKey: 'review-42', + titleDraft: '', + setTitleDraft: vi.fn(), + titleInputRef: { current: null }, + titleSaving: false + } satisfies ChecksPanelActiveContentModel +} + describe('checks panel concrete content', () => { it('renders the no-workspace empty state with its guidance', () => { const model = { activeWorktree: null } as ChecksPanelEmptyContentModel @@ -97,81 +185,7 @@ describe('checks panel concrete content', () => { }) it('renders the active review header and empty check/comment sections with accessible actions', () => { - const model = { - activeConnectionId: null, - activeConflictReview: null, - activeGitLabReview: null, - activeReview: { - provider: 'github', - number: 42, - state: 'open', - title: 'Preserve mounted panel behavior', - url: 'https://github.com/orca/app/pull/42', - status: 'success', - updatedAt: '2026-08-23T00:00:00.000Z', - mergeable: 'MERGEABLE' - }, - activeSourceControlLaunchPlatform: 'darwin', - activeWorktree: null, - activeWorktreeId: 'worktree-1', - agentComposerState: null, - aiActionDisabledReason: undefined, - canTargetPRComments: false, - checks: [], - checksLoading: false, - claimedCommentResolutionRef: { current: null }, - commentResolutionLaunchAcceptedRef: { current: false }, - comments: [], - commentsDisabledReason: undefined, - commentsLoading: false, - commentsSelectionClearRequest: null, - conflictDetailsRefreshing: false, - consumeClaimedCommentResolutionAfterDeliveryRef: { current: vi.fn() }, - detachedHeadDisplay: null, - editingTitle: false, - getGitLabProjectRef: vi.fn(() => null), - handleAddPRComment: vi.fn(), - handleCancelEdit: vi.fn(), - handleDeleteComment: vi.fn(), - handleEditComment: vi.fn(), - handleFixChecksWithAI: vi.fn(), - handleLaunchAborted: vi.fn(), - handleLaunchAccepted: vi.fn(), - handleLinkAnotherReview: vi.fn(), - handleLoadCheckDetails: vi.fn(), - handleOpenPR: vi.fn(), - handleRefresh: vi.fn(), - handleOpenStackPR: vi.fn(), - handleReplyToComment: vi.fn(), - handleResolve: vi.fn(), - handleResolveCommentsWithAI: vi.fn(), - handleResolveConflictsWithAI: vi.fn(), - handleSaveTitle: vi.fn(), - handleSetReaction: vi.fn(), - handleStartEdit: vi.fn(), - handleTitleKeyDown: vi.fn(), - handleUnlinkReview: vi.fn(), - isFixingChecksWithAI: false, - isRefreshing: false, - isResolvingConflictsWithAI: false, - linkedGitLabMR: null, - pendingCommentResolutionRef: { current: null }, - pr: null, - prRefreshState: undefined, - repo: null, - refreshHostedReviewAfterMutation: vi.fn(), - resolveCommentsWithAIDisabledReason: undefined, - saveLaunchActionDefault: vi.fn(), - setAgentComposerState: vi.fn(), - setChecksPanelContentRef: vi.fn(), - settings: null, - sourceControlAiActionsVisible: false, - stateRequestKey: 'review-42', - titleDraft: '', - setTitleDraft: vi.fn(), - titleInputRef: { current: null }, - titleSaving: false - } satisfies ChecksPanelActiveContentModel + const model = activeContentModel() render( @@ -185,4 +199,55 @@ describe('checks panel concrete content', () => { 'Open on GitHub' ) }) + + it('records a Resolve comments launch’s thread resolution, and hands the click its outcome', () => { + const commentResolution = { + reviewContextKey: 'review-42', + provider: 'github' as const, + selectedGroups: [] + } + const model = { + ...activeContentModel(), + agentComposerState: { + actionId: 'resolveComments' as const, + title: 'Resolve comments', + description: '', + prompt: 'fix', + launchSource: 'task_page' as const, + commentResolution + } + } + render( + + ) + + expect(dialogProps.last?.durableFollowUp).toEqual( + reviewCommentsResolutionFollowUp(commentResolution) + ) + const onLaunched = dialogProps.last?.onLaunched + if (typeof onLaunched !== 'function') { + throw new Error('no onLaunched') + } + onLaunched({ followUpDeferred: true }) + expect(model.consumeClaimedCommentResolutionAfterDeliveryRef.current).toHaveBeenCalledWith({ + followUpDeferred: true + }) + }) + + it('records nothing for an AI action that resolves no comments', () => { + const model = { + ...activeContentModel(), + agentComposerState: { + actionId: 'fixChecks' as const, + title: 'Fix checks', + description: '', + prompt: 'fix', + launchSource: 'task_page' as const + } + } + render( + + ) + expect(dialogProps.last?.durableFollowUp).toBeUndefined() + }) }) diff --git a/src/renderer/src/components/right-sidebar/pr-comment-resolution-after-delivery.test.ts b/src/renderer/src/components/right-sidebar/pr-comment-resolution-after-delivery.test.ts new file mode 100644 index 000000000000..6293a9a2bc6b --- /dev/null +++ b/src/renderer/src/components/right-sidebar/pr-comment-resolution-after-delivery.test.ts @@ -0,0 +1,121 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { PRComment } from '../../../../shared/github/comment-types' +import type { PendingPRCommentAiAck, PRCommentAiLaunchAckDeps } from './pr-comments-ai-launch-ack' + +const mocks = vi.hoisted(() => ({ + ack: vi.fn(), + toast: { error: vi.fn(), success: vi.fn() } +})) +vi.mock('sonner', () => ({ toast: mocks.toast })) +vi.mock('./pr-comments-ai-launch-ack', async (importOriginal) => ({ + ...(await importOriginal>()), + acknowledgePRCommentsAfterAiLaunch: mocks.ack +})) + +const { acknowledgeCommentResolutionAfterDelivery } = + await import('./pr-comment-resolution-after-delivery') + +const RESOLUTION: PendingPRCommentAiAck = { + reviewContextKey: 'pr-1', + provider: 'github', + selectedGroups: [], + githubTarget: { repoPath: '/repo', repoId: 'r1', prNumber: 1, prRepo: { owner: 'o', repo: 'r' } } +} +const POSTED: PRComment = { + id: 9, + author: 'me', + authorAvatarUrl: '', + body: 'Fixing.', + createdAt: '2026-01-01T00:00:00Z', + url: 'https://example.test/c/9' +} + +function actions() { + return { + resolveReviewThread: vi.fn(async () => true), + addPRReviewCommentReply: vi.fn(), + addPRConversationComment: vi.fn(async () => ({ ok: true as const, comment: POSTED })), + settings: null + } +} + +function view(onReview: boolean) { + return { + isStillOnLaunchReview: () => onReview, + existingComments: () => [], + setComments: vi.fn(), + refresh: vi.fn(async () => {}) + } +} + +/** Acknowledges by posting one conversation reply, as a selection of plain comments would. */ +function ackPostsOneReply(counts = { resolved: 0, replied: 1, skipped: 0, failed: 0 }) { + mocks.ack.mockImplementationOnce(async ({ deps }: { deps: PRCommentAiLaunchAckDeps }) => { + await deps.replyAsConversation('Fixing.') + return counts + }) +} + +beforeEach(() => vi.clearAllMocks()) + +describe('resolving a Checks launch’s comments once its prompt reached the agent', () => { + it('shows the replies in the panel still on that review, then refreshes it', async () => { + ackPostsOneReply() + const panel = view(true) + const refreshCache = vi.fn(async () => {}) + await acknowledgeCommentResolutionAfterDelivery(RESOLUTION, { + actions: actions(), + view: panel, + refreshCache + }) + expect(panel.setComments).toHaveBeenCalledOnce() + expect(panel.refresh).toHaveBeenCalledOnce() + expect(refreshCache).not.toHaveBeenCalled() + expect(mocks.toast.success).toHaveBeenCalledOnce() + }) + + it('leaves a panel that moved to another review alone', async () => { + ackPostsOneReply() + const panel = view(false) + await acknowledgeCommentResolutionAfterDelivery(RESOLUTION, { + actions: actions(), + view: panel, + refreshCache: vi.fn(async () => {}) + }) + expect(panel.setComments).not.toHaveBeenCalled() + expect(panel.refresh).not.toHaveBeenCalled() + }) + + it('with no panel, as after a reload, posts the same replies and refreshes the cached comments', async () => { + ackPostsOneReply() + const host = actions() + const refreshCache = vi.fn(async () => {}) + await acknowledgeCommentResolutionAfterDelivery(RESOLUTION, { actions: host, refreshCache }) + expect(host.addPRConversationComment).toHaveBeenCalledOnce() + expect(refreshCache).toHaveBeenCalledOnce() + expect(mocks.toast.success).toHaveBeenCalledOnce() + }) + + it('reports what failed with the counts', async () => { + ackPostsOneReply({ resolved: 1, replied: 0, skipped: 0, failed: 1 }) + await acknowledgeCommentResolutionAfterDelivery(RESOLUTION, { + actions: actions(), + refreshCache: vi.fn(async () => {}) + }) + expect(mocks.toast.error).toHaveBeenCalledExactlyOnceWith(expect.stringContaining('failed 1')) + expect(mocks.toast.success).not.toHaveBeenCalled() + }) + + it('says so once when acknowledging throws, never rejecting its caller', async () => { + mocks.ack.mockRejectedValueOnce(new Error('offline')) + await expect( + acknowledgeCommentResolutionAfterDelivery(RESOLUTION, { + actions: actions(), + refreshCache: vi.fn(async () => {}) + }) + ).resolves.toBeUndefined() + expect(mocks.toast.error).toHaveBeenCalledExactlyOnceWith( + 'Started the agent, but could not resolve or reply on the selected comments.' + ) + }) +}) diff --git a/src/renderer/src/components/right-sidebar/pr-comments-list-selection.test.tsx b/src/renderer/src/components/right-sidebar/pr-comments-list-selection.test.tsx index 482b732029a2..1c382bb48940 100644 --- a/src/renderer/src/components/right-sidebar/pr-comments-list-selection.test.tsx +++ b/src/renderer/src/components/right-sidebar/pr-comments-list-selection.test.tsx @@ -6,6 +6,7 @@ import { act, StrictMode, Suspense, type ReactNode } from 'react' import { createRoot, type Root } from 'react-dom/client' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { TooltipProvider } from '@/components/ui/tooltip' +import { holdPRCommentGroupsForSend } from './pr-comment-groups-in-flight' vi.mock('@/components/ui/dropdown-menu', () => { // Radix keeps the selection callback on the group, so the mocked items need it too. @@ -291,6 +292,26 @@ describe('PRCommentsList comment resolution selection', () => { ) }) + it('does not offer again a thread a launch still on its way will resolve', async () => { + let settle: () => void = () => {} + holdPRCommentGroupsForSend(['thread:thread-1'], new Promise((done) => (settle = done))) + const comments = [ + comment({ id: 1, threadId: 'thread-1', path: 'src/a.ts', isResolved: false }), + comment({ id: 2, threadId: 'thread-2', path: 'src/b.ts', isResolved: false }) + ] + const onResolveSelectedCommentsWithAI = vi.fn() + renderList({ comments, onResolveSelectedCommentsWithAI }) + clickButton('Send unresolved PR comments') + expect(onResolveSelectedCommentsWithAI).toHaveBeenCalledExactlyOnceWith([ + expect.objectContaining({ threadId: 'thread-2' }) + ]) + + await act(async () => settle()) + onResolveSelectedCommentsWithAI.mockClear() + clickButton('Send unresolved PR comments') + expect(onResolveSelectedCommentsWithAI.mock.calls[0]?.[0]).toHaveLength(2) + }) + it('lets a user queue one eligible comment thread for the agent from the visible row action', () => { const onResolveSelectedCommentsWithAI = vi.fn() renderList({ diff --git a/src/renderer/src/components/right-sidebar/runSourceControlAgentActionStart.test.ts b/src/renderer/src/components/right-sidebar/runSourceControlAgentActionStart.test.ts index 3d315f42414d..76290a2c8edd 100644 --- a/src/renderer/src/components/right-sidebar/runSourceControlAgentActionStart.test.ts +++ b/src/renderer/src/components/right-sidebar/runSourceControlAgentActionStart.test.ts @@ -72,6 +72,25 @@ describe('runSourceControlAgentActionStart', () => { expect(mocks.toastError).not.toHaveBeenCalled() }) + it('records the follow-up on the launch, and tells onLaunched when it was left for the next start', async () => { + const durableFollowUp = { kind: 'review-comments-resolution', version: 1, payload: {} } + mocks.launchAgentInNewTab.mockReturnValue({ + surface: { kind: 'local-terminal', tabId: 'tab-1' }, + promptDeliveryResult: Promise.resolve({ + delivered: true, + failureNotified: false, + followUpDeferred: true + }) + }) + + await runSourceControlAgentActionStart(buildArgs({ durableFollowUp })) + + expect(mocks.launchAgentInNewTab).toHaveBeenCalledWith( + expect.objectContaining({ durableFollowUp }) + ) + expect(mocks.onLaunched).toHaveBeenCalledExactlyOnceWith({ followUpDeferred: true }) + }) + it('notifies onLaunchAccepted as soon as the agent tab is created, before prompt delivery', async () => { let resolveDelivery: (value: { delivered: boolean diff --git a/src/renderer/src/components/tab-bar/QuickLaunchButton.launch-status.test.tsx b/src/renderer/src/components/tab-bar/QuickLaunchButton.launch-status.test.tsx index 370ca704b87b..534de6534e3c 100644 --- a/src/renderer/src/components/tab-bar/QuickLaunchButton.launch-status.test.tsx +++ b/src/renderer/src/components/tab-bar/QuickLaunchButton.launch-status.test.tsx @@ -194,6 +194,28 @@ describe('QuickLaunchAgentMenuItems launches', () => { await expect(onPromptHandedOff.mock.calls[0][0]).resolves.toEqual({ delivered: true }) }) + it('records the menu’s follow-up on the launch it starts', () => { + launchMock.mockReturnValue({ + surface: { kind: 'local-agent-session', tabId: 'tab-1', sessionId: 'codex-session' }, + promptDeliveryResult: Promise.resolve({ delivered: true, failureNotified: false }) + }) + const durableFollowUp = { kind: 'review-notes-delivered', version: 1, payload: {} } + + render( + + ) + fireEvent.click(document.querySelector('[title="Launch Codex in a new terminal"]')!) + + expect(launchMock).toHaveBeenCalledWith(expect.objectContaining({ durableFollowUp })) + }) + it('starts no agent when the menu has nothing left to send', () => { launchMock.mockClear() render( diff --git a/src/renderer/src/lib/agent-launch-follow-ups.test.ts b/src/renderer/src/lib/agent-launch-follow-ups.test.ts index 8099c6023c06..6c2dfc794014 100644 --- a/src/renderer/src/lib/agent-launch-follow-ups.test.ts +++ b/src/renderer/src/lib/agent-launch-follow-ups.test.ts @@ -10,11 +10,13 @@ const state = vi.hoisted(() => { const answers: unknown[] = [] const calls: unknown[] = [] const held: { keys: readonly unknown[]; settled: Promise }[] = [] + const heldThreads: { keys: readonly unknown[]; settled: Promise }[] = [] return { capabilities, answers, calls, held, + heldThreads, clearDeliveredDiffComments: vi.fn(async () => true), runResolution: vi.fn(async () => {}) } @@ -47,6 +49,10 @@ vi.mock('@/lib/notes-send-in-flight', () => ({ vi.mock('@/components/right-sidebar/review-comments-resolution-follow-up', () => ({ runReviewCommentsResolutionFollowUp: state.runResolution })) +vi.mock('@/components/right-sidebar/pr-comment-groups-in-flight', () => ({ + holdPRCommentGroupsForSend: (keys: readonly unknown[], settled: Promise) => + state.heldThreads.push({ keys, settled }) +})) const { recordableLaunchFollowUp, @@ -57,12 +63,58 @@ const { const NOTE = { id: 'n1', body: 'fix this', filePath: 'a.ts', lineNumber: 3 } const NOTES = reviewNotesDeliveredFollowUp('wt-1', [NOTE]) +const COMMENT = { + id: 7, + author: 'reviewer', + authorAvatarUrl: '', + body: 'please rename this', + createdAt: '2026-01-01T00:00:00Z', + url: 'https://example.test/c/7', + threadId: 'thread-1', + path: 'a.ts', + line: 3 +} const RESOLUTION = reviewCommentsResolutionFollowUp({ reviewContextKey: 'pr-1', provider: 'github', - selectedGroups: [] + selectedGroups: [{ kind: 'thread', threadId: 'thread-1', root: COMMENT, replies: [COMMENT] }] }) +/** A clock the test turns by hand: timers fire on `advance`, the host's word on `settle`. */ +function manualClock() { + let now = 0 + const timers: { at: number; run: () => void }[] = [] + const listeners = new Set<(operationId: string) => void>() + return { + clock: { + now: () => now, + schedule: (ms: number, run: () => void) => { + const timer = { at: now + ms, run } + timers.push(timer) + return () => timers.splice(timers.indexOf(timer), 1) + }, + onSettled: (listener: (operationId: string) => void) => { + listeners.add(listener) + return () => listeners.delete(listener) + } + }, + advance(ms: number) { + now += ms + for (const timer of timers.filter((t) => t.at <= now)) { + timers.splice(timers.indexOf(timer), 1) + timer.run() + } + }, + settle: (operationId: string) => listeners.forEach((listener) => listener(operationId)), + get listeners() { + return listeners.size + }, + get timers() { + return timers.length + } + } +} + function taken(operationId: string, followUp: AgentLaunchFollowUp, promptHandedOver = true) { return { operationId, followUp, promptHandedOver, composerUnobserved: false } } @@ -72,6 +124,7 @@ beforeEach(() => { state.answers = [] state.calls = [] state.held = [] + state.heldThreads = [] state.clearDeliveredDiffComments.mockClear() state.runResolution.mockClear() }) @@ -84,7 +137,7 @@ describe('a window that reloaded mid-launch runs what its launches recorded', () pending: [] } state.answers.push(take) - await runRecordedLaunchFollowUps() + await runRecordedLaunchFollowUps(manualClock().clock) expect(state.clearDeliveredDiffComments).toHaveBeenCalledExactlyOnceWith('wt-1', [NOTE]) expect(state.runResolution).toHaveBeenCalledOnce() expect(state.calls).toEqual([['agent.takeLaunchFollowUps', {}]]) @@ -92,7 +145,7 @@ describe('a window that reloaded mid-launch runs what its launches recorded', () it('runs nothing for a launch whose prompt never arrived', async () => { state.answers.push({ taken: [taken('op-1', NOTES, false)], pending: [] }) - await runRecordedLaunchFollowUps() + await runRecordedLaunchFollowUps(manualClock().clock) expect(state.clearDeliveredDiffComments).not.toHaveBeenCalled() }) @@ -105,62 +158,118 @@ describe('a window that reloaded mid-launch runs what its launches recorded', () ], pending: [] }) - await runRecordedLaunchFollowUps() + await runRecordedLaunchFollowUps(manualClock().clock) expect(state.clearDeliveredDiffComments).not.toHaveBeenCalled() }) - it('keeps notes still waiting on their prompt unsendable, then runs them once taken', async () => { + it('holds what a launch still on its way acts on before running anything else', async () => { + const order: string[] = [] + state.clearDeliveredDiffComments.mockImplementationOnce(async () => { + order.push(`ran with ${state.held.length + state.heldThreads.length} held`) + return true + }) + state.answers.push({ + taken: [taken('op-1', NOTES)], + pending: [ + { operationId: 'op-2', followUp: NOTES, deadline: 60_000 }, + { operationId: 'op-3', followUp: RESOLUTION, deadline: 60_000 } + ] + }) + const t = manualClock() + void runRecordedLaunchFollowUps(t.clock) + await vi.waitFor(() => expect(order).toEqual(['ran with 2 held'])) + expect(state.held[0]!.keys).toEqual(['n1']) + expect(state.heldThreads[0]!.keys).toEqual(['thread:thread-1']) + }) + + it('runs a pending follow-up when the host says its prompt settled, and lets go', async () => { state.answers.push( - { taken: [], pending: [{ operationId: 'op-1', followUp: NOTES }] }, - { taken: [], pending: [{ operationId: 'op-1', followUp: NOTES }] }, + { taken: [], pending: [{ operationId: 'op-1', followUp: NOTES, deadline: 60_000 }] }, { taken: [taken('op-1', NOTES)], pending: [] } ) + const t = manualClock() + const run = runRecordedLaunchFollowUps(t.clock) + await vi.waitFor(() => expect(t.listeners).toBe(1)) let released = false - const run = runRecordedLaunchFollowUps({ wait: async () => {}, pollMs: 1 }) - await vi.waitFor(() => expect(state.held).toHaveLength(1)) void state.held[0]!.settled.then(() => (released = true)) + t.settle('op-1') await run - expect(state.held[0]!.keys).toEqual(['n1']) expect(released).toBe(true) expect(state.clearDeliveredDiffComments).toHaveBeenCalledOnce() - // The re-ask names the launch, so a click this window makes meanwhile stays its own. - expect(state.calls.slice(1)).toEqual([ - ['agent.takeLaunchFollowUps', { operationId: 'op-1' }], - ['agent.takeLaunchFollowUps', { operationId: 'op-1' }] - ]) + // The take names the launch, so a click this window makes meanwhile stays its own. + expect(state.calls.slice(1)).toEqual([['agent.takeLaunchFollowUps', { operationId: 'op-1' }]]) + expect(t.listeners).toBe(0) + expect(t.timers).toBe(0) }) - it('stops asking at its bound and leaves the follow-up on the record', async () => { - const pending = { taken: [], pending: [{ operationId: 'op-1', followUp: NOTES }] } - state.answers.push(pending, pending, pending, pending) - let now = 0 - vi.spyOn(Date, 'now').mockImplementation(() => now) - await runRecordedLaunchFollowUps({ - maxWaitMs: 3, - pollMs: 1, - wait: async (ms) => { - now += ms - } - }) + it('keeps waiting when the word comes for a launch still owed', async () => { + const stillPending = { + taken: [], + pending: [{ operationId: 'op-1', followUp: NOTES, deadline: 60_000 }] + } + state.answers.push(stillPending, stillPending) + const t = manualClock() + let finished = false + void runRecordedLaunchFollowUps(t.clock).then(() => (finished = true)) + await vi.waitFor(() => expect(t.listeners).toBe(1)) + t.settle('op-1') + await vi.waitFor(() => expect(state.calls).toHaveLength(2)) + expect(finished).toBe(false) + expect(t.listeners).toBe(1) + }) + + it('takes once more just past the host’s deadline, then lets go of a launch still pending', async () => { + const stillPending = { + taken: [], + pending: [{ operationId: 'op-1', followUp: NOTES, deadline: 60_000 }] + } + state.answers.push(stillPending, stillPending) + const t = manualClock() + const run = runRecordedLaunchFollowUps(t.clock) + await vi.waitFor(() => expect(t.timers).toBe(1)) + t.advance(60_000 + 14_999) + expect(state.calls).toHaveLength(1) + t.advance(1) + await run + expect(state.calls).toHaveLength(2) expect(state.clearDeliveredDiffComments).not.toHaveBeenCalled() - expect(state.calls).toHaveLength(4) await expect(state.held[0]!.settled).resolves.toBeUndefined() + expect(t.listeners).toBe(0) + }) + + it('runs a follow-up the last take finds settled', async () => { + state.answers.push( + { taken: [], pending: [{ operationId: 'op-1', followUp: NOTES, deadline: 60_000 }] }, + { taken: [taken('op-1', NOTES)], pending: [] } + ) + const t = manualClock() + const run = runRecordedLaunchFollowUps(t.clock) + await vi.waitFor(() => expect(t.timers).toBe(1)) + t.advance(75_000) + await run + expect(state.clearDeliveredDiffComments).toHaveBeenCalledOnce() }) it('asks nothing of a host that does not record follow-ups', async () => { state.capabilities = [] - await runRecordedLaunchFollowUps() + await runRecordedLaunchFollowUps(manualClock().clock) expect(state.calls).toEqual([]) }) it('treats a host that cannot answer as nothing recorded', async () => { state.answers.push(new Error('method_not_found')) - await expect(runRecordedLaunchFollowUps()).resolves.toBeUndefined() + await expect(runRecordedLaunchFollowUps(manualClock().clock)).resolves.toBeUndefined() expect(state.clearDeliveredDiffComments).not.toHaveBeenCalled() }) }) describe('which follow-ups a launch records', () => { + it('records review threads without the comments’ words', () => { + const payload = RESOLUTION.payload + expect(JSON.stringify(payload)).not.toContain('please rename this') + expect(JSON.stringify(payload)).toContain('thread-1') + }) + it('only on a host that keeps them, and only under the size cap', () => { expect(recordableLaunchFollowUp(NOTES)).toBe(NOTES) expect(recordableLaunchFollowUp(undefined)).toBeUndefined() diff --git a/src/renderer/src/lib/launch-agent-new-tab-host-route.test.ts b/src/renderer/src/lib/launch-agent-new-tab-host-route.test.ts index 9f28915b0262..8cef7e00b9ed 100644 --- a/src/renderer/src/lib/launch-agent-new-tab-host-route.test.ts +++ b/src/renderer/src/lib/launch-agent-new-tab-host-route.test.ts @@ -1,5 +1,6 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import type { HostAgentLaunchOutcome } from './agent-launch-through-host' +import type { AgentLaunchFollowUpTake } from '../../../shared/agent-launch-follow-up' const host = vi.hoisted(() => ({ launchAgentThroughHost: vi.fn(), @@ -25,7 +26,12 @@ const followUps = vi.hoisted(() => { return { hostRecords: true, order, - takeLaunchFollowUps: vi.fn(async (_operationId?: string) => ({ taken: [], pending: [] })) + takeLaunchFollowUps: vi.fn( + async (_operationId?: string): Promise => ({ + taken: [], + pending: [] + }) + ) } }) vi.mock('@/lib/agent-launch-follow-ups', () => ({ @@ -171,7 +177,29 @@ describe('an AI button launched through the host, which delivers its prompt', () describe('a click whose follow-up is recorded on its launch', () => { const FOLLOW_UP = { kind: 'review-notes-delivered', version: 1, payload: {} } + function launchWithFollowUp(onPromptDelivered: () => void) { + return launchNewTabPromptThroughHost({ + agent: 'claude', + worktreeId: 'wt-1', + prompt: 'p', + pasteContent: 'p', + durableFollowUp: FOLLOW_UP, + onPromptDelivered + }).promptDeliveryResult + } + it('records it, then takes it off the record before it runs, so a reload never runs it again', async () => { + followUps.takeLaunchFollowUps.mockResolvedValueOnce({ + taken: [ + { + operationId: 'op-1', + followUp: FOLLOW_UP, + promptHandedOver: true, + composerUnobserved: false + } + ], + pending: [] + }) deferredOutcome()({ kind: 'started', prompt: { delivery: 'submit', outcome: 'handed-to-terminal' } @@ -191,6 +219,24 @@ describe('a click whose follow-up is recorded on its launch', () => { expect(followUps.order).toEqual(['take', 'follow-up']) }) + it('leaves it to the next start when this click could not take it, and says so', async () => { + // Taken by a reloaded window already, or the host could not answer: it runs there, never here. + for (const take of [{ taken: [], pending: [] }, null]) { + followUps.takeLaunchFollowUps.mockResolvedValueOnce(take) + deferredOutcome()({ + kind: 'started', + prompt: { delivery: 'submit', outcome: 'handed-to-terminal' } + }) + const onPromptDelivered = vi.fn() + await expect(launchWithFollowUp(onPromptDelivered)).resolves.toEqual({ + delivered: true, + failureNotified: false, + followUpDeferred: true + }) + expect(onPromptDelivered).not.toHaveBeenCalled() + } + }) + it('on a host that does not record follow-ups, runs it live and records nothing, as before', async () => { followUps.hostRecords = false deferredOutcome()({ From d65948d785f7ba265662e40cddcafb25862e5044 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 7 Oct 2026 03:59:23 -0700 Subject: [PATCH 3/8] fix(agent-launch): wait on recorded follow-ups one take at a time, and reach an open Checks panel - The window's startup take neither opens nor creates the record store when no launch recorded a follow-up. - The host says a launch settled however it ended (failed, tab closed), once it no longer reads as running, so a reloaded window stops holding notes and threads for a launch that failed. - One waiter per window: it listens before startup's first take, takes each launch once at a time (a second, empty answer never releases a hold another take is still running), and also holds a click's follow-up its own take missed, taking it when the host says so. - A reload's comment resolution goes through the Checks panel open on that review, as the click would, re-reading it from GitHub or GitLab. - A recorded comment keeps the one-line snippet its "Fixing:" reply quotes. --- .../agent-session-journal/journal-database.ts | 2 +- ...agent-launch-persisted-obligations.test.ts | 8 + .../methods/agent-launch-follow-ups.test.ts | 23 ++ .../rpc/methods/agent-launch-follow-ups.ts | 13 +- src/main/runtime/rpc/methods/agent-launch.ts | 8 +- .../use-recorded-launch-follow-ups.test.tsx | 2 +- .../use-recorded-launch-follow-ups.ts | 2 +- .../checks-panel/open-checks-panel-views.ts | 43 +++ ...e-checks-panel-ai-acknowledgement.test.tsx | 49 +++ .../use-checks-panel-ai-acknowledgement.tsx | 13 + .../pr-comment-fixing-reply-body.ts | 2 +- .../pr-comment-resolution-after-delivery.ts | 10 +- ...view-comments-resolution-follow-up.test.ts | 58 ++++ .../review-comments-resolution-follow-up.ts | 9 +- .../lib/agent-launch-follow-up-waiter.test.ts | 324 ++++++++++++++++++ .../src/lib/agent-launch-follow-up-waiter.ts | 190 ++++++++++ .../src/lib/agent-launch-follow-ups.test.ts | 212 ++---------- .../src/lib/agent-launch-follow-ups.ts | 112 +----- .../launch-agent-new-tab-host-route.test.ts | 45 ++- .../lib/launch-agent-new-tab-host-route.ts | 33 +- 20 files changed, 831 insertions(+), 327 deletions(-) create mode 100644 src/renderer/src/components/right-sidebar/checks-panel/open-checks-panel-views.ts create mode 100644 src/renderer/src/components/right-sidebar/checks-panel/use-checks-panel-ai-acknowledgement.test.tsx create mode 100644 src/renderer/src/components/right-sidebar/review-comments-resolution-follow-up.test.ts create mode 100644 src/renderer/src/lib/agent-launch-follow-up-waiter.test.ts create mode 100644 src/renderer/src/lib/agent-launch-follow-up-waiter.ts diff --git a/src/main/native-chat/agent-session-journal/journal-database.ts b/src/main/native-chat/agent-session-journal/journal-database.ts index 71209526b563..ede4964bad76 100644 --- a/src/main/native-chat/agent-session-journal/journal-database.ts +++ b/src/main/native-chat/agent-session-journal/journal-database.ts @@ -51,7 +51,7 @@ export function journalDatabaseHoldsAgentSessions(dbPath: string): boolean { } /** A launch-record field whose presence means the host still owes that launch work. */ -export type LaunchOperationObligation = 'promptDelivery' +export type LaunchOperationObligation = 'promptDelivery' | 'launchFollowUp' /** Whether any unexpired launch record holds `field`, read-only: an expired one is skipped by * everything that reads it, and nothing prunes it until the store next opens. */ diff --git a/src/main/runtime/agent-launch-persisted-obligations.test.ts b/src/main/runtime/agent-launch-persisted-obligations.test.ts index 1760a77f8c7c..c4060ae59b7a 100644 --- a/src/main/runtime/agent-launch-persisted-obligations.test.ts +++ b/src/main/runtime/agent-launch-persisted-obligations.test.ts @@ -79,6 +79,14 @@ describe('whether a restarted host owes a launch its prompt, read before opening expect(hasPersistedLaunchObligation(directory, 'promptDelivery', 50_000)).toBe(false) }) + it('finds a recorded follow-up the window has not taken yet', async () => { + const row = launchRow() + row.launchFollowUp = { kind: 'review-notes-delivered', version: 1, payload: {} } + await leaveLaunchRecords(row) + expect(hasPersistedLaunchObligation(directory, 'launchFollowUp')).toBe(true) + expect(hasPersistedLaunchObligation(directory, 'promptDelivery')).toBe(false) + }) + it('never creates the database of a profile that has none', () => { expect(hasPersistedLaunchObligation(directory, 'promptDelivery')).toBe(false) expect(existsSync(journalDatabasePath(directory))).toBe(false) diff --git a/src/main/runtime/rpc/methods/agent-launch-follow-ups.test.ts b/src/main/runtime/rpc/methods/agent-launch-follow-ups.test.ts index 3566d65ccc3d..8eced054d051 100644 --- a/src/main/runtime/rpc/methods/agent-launch-follow-ups.test.ts +++ b/src/main/runtime/rpc/methods/agent-launch-follow-ups.test.ts @@ -26,6 +26,10 @@ const deliverTerminalPrompt = vi.hoisted(() => return true }) ) +const persisted = vi.hoisted(() => ({ followUps: true })) +vi.mock('../../agent-launch-persisted-obligations', () => ({ + hasPersistedLaunchObligation: () => persisted.followUps +})) vi.mock('./agent-launch-terminal-prompt', () => ({ deliverTerminalAgentLaunchPrompt: deliverTerminalPrompt })) @@ -83,6 +87,7 @@ function take(runtime: AgentLaunchRuntimeStub, context: Partial, ope } beforeEach(async () => { + persisted.followUps = true deliverTerminalPrompt.mockClear() directory = await mkdtemp(join(tmpdir(), 'orca-agent-launch-follow-ups-')) store = await openTestAgentSessionRecordStore(directory) @@ -125,6 +130,24 @@ describe('a click’s follow-up on its launch’s record', () => { expect(runtime.reportAgentLaunchPromptSettled).toHaveBeenCalledOnce() }) + it('tells the window when the launch failed too, so it stops holding what the click sent', async () => { + const runtime = host() + runtime.createTerminal.mockRejectedValueOnce(new Error('spawn failed')) + await expect(launch(runtime)).rejects.toThrow() + expect(runtime.reportAgentLaunchPromptSettled).toHaveBeenCalledExactlyOnceWith(OPERATION_ID) + await expect(take(runtime, DESKTOP, OPERATION_ID)).resolves.toMatchObject({ + taken: [{ operationId: OPERATION_ID, promptHandedOver: false }] + }) + }) + + it('answers a window load with nothing, never opening the store, when no launch recorded one', async () => { + persisted.followUps = false + const runtime = host() + runtime.openedAgentSessionRecordStore.mockReturnValue(null) + await expect(take(runtime, DESKTOP)).resolves.toEqual({ taken: [], pending: [] }) + expect(runtime.openAgentSessionRecordStore).not.toHaveBeenCalled() + }) + it('is recorded for the desktop only: a phone, which never takes one, records nothing', async () => { const runtime = host() await launch(runtime, LAUNCH, PHONE) diff --git a/src/main/runtime/rpc/methods/agent-launch-follow-ups.ts b/src/main/runtime/rpc/methods/agent-launch-follow-ups.ts index 99ff18046148..e48319491341 100644 --- a/src/main/runtime/rpc/methods/agent-launch-follow-ups.ts +++ b/src/main/runtime/rpc/methods/agent-launch-follow-ups.ts @@ -12,6 +12,8 @@ import { } from '../../agent-launch-follow-up-record' import type { OrcaRuntimeService } from '../../orca-runtime' import { defineMethod } from '../core' +import { hasPersistedLaunchObligation } from '../../agent-launch-persisted-obligations' +import { getProfileUserDataPath } from '../../../orca-profiles/profile-storage-paths' import { DESKTOP_RPC_CALLER, rpcCallerOperationKey } from '../rpc-caller-identity' import { activeAgentLaunchesFor } from './agent-launch-active-operations' import { agentLaunchOperationCallerKey } from './agent-launch-replay' @@ -50,7 +52,16 @@ export const AGENT_LAUNCH_FOLLOW_UP_METHODS = [ params: AgentTakeLaunchFollowUps, handler: async (params, context): Promise => { const callerKey = agentLaunchOperationCallerKey(context) - const store = await context.runtime.openAgentSessionRecordStore() + // Every window load asks: a profile whose launches recorded none must not open the store at + // startup, nor create its database. + const store = + context.runtime.openedAgentSessionRecordStore() ?? + (hasPersistedLaunchObligation(getProfileUserDataPath(), 'launchFollowUp') + ? await context.runtime.openAgentSessionRecordStore() + : null) + if (!store) { + return { taken: [], pending: [] } + } const active = activeAgentLaunchesFor(context.runtime) return store.transactOperations((draft) => takeLaunchFollowUpsInto(draft, { diff --git a/src/main/runtime/rpc/methods/agent-launch.ts b/src/main/runtime/rpc/methods/agent-launch.ts index eeacc7977602..ba2674d4743d 100644 --- a/src/main/runtime/rpc/methods/agent-launch.ts +++ b/src/main/runtime/rpc/methods/agent-launch.ts @@ -271,9 +271,6 @@ async function executeAdmittedAgentLaunch( // Bookkeeping: a failure leaves the first write, whose owed prompt replays as `unconfirmed` (or as // `unknown` to a caller that cannot read it), never as `not-delivered`. await settleQuietly(admission.settle(result)) - if (params.followUp) { - announceSettledLaunchFollowUps(context.runtime) - } return result } @@ -298,6 +295,11 @@ function runReplaySafeAgentLaunch( if (activeAgentLaunches.get(key)?.promise === promise) { activeAgentLaunches.delete(key) } + // Every way a launch ends, once it no longer reads as running: a window that reloaded mid-launch + // holds its follow-up's notes and threads until it hears. + if (params.followUp) { + announceSettledLaunchFollowUps(context.runtime) + } }) activeAgentLaunches.set(key, { fingerprint, promise }) return promise diff --git a/src/renderer/src/app-shell/use-recorded-launch-follow-ups.test.tsx b/src/renderer/src/app-shell/use-recorded-launch-follow-ups.test.tsx index 9685865ece76..fd1eebbb1081 100644 --- a/src/renderer/src/app-shell/use-recorded-launch-follow-ups.test.tsx +++ b/src/renderer/src/app-shell/use-recorded-launch-follow-ups.test.tsx @@ -3,7 +3,7 @@ import { act, renderHook } from '@testing-library/react' import { beforeEach, describe, expect, it, vi } from 'vitest' const run = vi.hoisted(() => vi.fn(async () => {})) -vi.mock('../lib/agent-launch-follow-ups', () => ({ runRecordedLaunchFollowUps: run })) +vi.mock('../lib/agent-launch-follow-up-waiter', () => ({ runRecordedLaunchFollowUps: run })) const { useAppStore } = await import('../store') const { useRecordedLaunchFollowUps } = await import('./use-recorded-launch-follow-ups') diff --git a/src/renderer/src/app-shell/use-recorded-launch-follow-ups.ts b/src/renderer/src/app-shell/use-recorded-launch-follow-ups.ts index 8e78078c3b34..bcec9e7345eb 100644 --- a/src/renderer/src/app-shell/use-recorded-launch-follow-ups.ts +++ b/src/renderer/src/app-shell/use-recorded-launch-follow-ups.ts @@ -1,6 +1,6 @@ import { useEffect } from 'react' import { useAppStore } from '../store' -import { runRecordedLaunchFollowUps } from '../lib/agent-launch-follow-ups' +import { runRecordedLaunchFollowUps } from '../lib/agent-launch-follow-up-waiter' /** As soon as the window has its worktrees and their notes, and before its terminals are back, it * holds what launches still on their way act on and runs what they recorded for it while it was diff --git a/src/renderer/src/components/right-sidebar/checks-panel/open-checks-panel-views.ts b/src/renderer/src/components/right-sidebar/checks-panel/open-checks-panel-views.ts new file mode 100644 index 000000000000..409bd632e876 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/checks-panel/open-checks-panel-views.ts @@ -0,0 +1,43 @@ +import type { PRComment } from '../../../../../shared/github/comment-types' +import { + checksPanelReviewStableKey, + type PendingPRCommentAiAck +} from '../pr-comments-ai-launch-ack' +import type { CommentResolutionAfterDeliveryDeps } from '../pr-comment-resolution-after-delivery' + +/** What a mounted Checks panel lends a resolution that runs without its click (after a reload). */ +export type OpenChecksPanelView = { + /** The review it shows now, as `checksPanelReviewStableKey` keys it. */ + stableKey: () => string + existingComments: () => readonly PRComment[] + setComments: (update: (previous: PRComment[]) => PRComment[]) => void + /** Re-reads the review's comments from its provider, as the panel's own click does. */ + refresh: (provider: PendingPRCommentAiAck['provider']) => Promise +} + +const views = new Set() + +export function registerOpenChecksPanelView(view: OpenChecksPanelView): () => void { + views.add(view) + return () => { + views.delete(view) + } +} + +/** The open panel showing this resolution's review, as its `view`; none when no panel shows it. */ +export function openChecksPanelViewFor( + resolution: PendingPRCommentAiAck +): CommentResolutionAfterDeliveryDeps['view'] { + const launchKey = checksPanelReviewStableKey(resolution.reviewContextKey) + for (const view of views) { + if (view.stableKey() === launchKey) { + return { + isStillOnLaunchReview: () => view.stableKey() === launchKey, + existingComments: view.existingComments, + setComments: view.setComments, + refresh: () => view.refresh(resolution.provider) + } + } + } + return undefined +} diff --git a/src/renderer/src/components/right-sidebar/checks-panel/use-checks-panel-ai-acknowledgement.test.tsx b/src/renderer/src/components/right-sidebar/checks-panel/use-checks-panel-ai-acknowledgement.test.tsx new file mode 100644 index 000000000000..0c620dfa3143 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/checks-panel/use-checks-panel-ai-acknowledgement.test.tsx @@ -0,0 +1,49 @@ +// @vitest-environment happy-dom +import { renderHook } from '@testing-library/react' +import { describe, expect, it, vi } from 'vitest' +import { useChecksPanelAiAcknowledgement } from './use-checks-panel-ai-acknowledgement' +import { openChecksPanelViewFor } from './open-checks-panel-views' + +function model(reviewKey: string) { + return { + addPRConversationComment: vi.fn(), + addPRReviewCommentReply: vi.fn(), + asyncResultKeyRef: { current: reviewKey }, + claimedCommentResolutionRef: { current: null }, + commentsRef: { current: [] }, + commentsSelectionClearTokenRef: { current: 0 }, + pendingCommentResolutionRef: { current: null }, + resolveReviewThread: vi.fn(), + setCommentResolutionAckBusyNow: vi.fn(), + setComments: vi.fn(), + setCommentsSelectionClearRequest: vi.fn(), + settings: null, + commentResolutionLaunchAcceptedRef: { current: false }, + fetchComments: vi.fn(async () => {}), + fetchGitLabDetails: vi.fn(async () => {}) + } satisfies Parameters[0] +} + +const resolution = (provider: 'github' | 'gitlab') => ({ + reviewContextKey: 'repo-1::pr::42::sha-1', + provider, + selectedGroups: [] +}) + +describe('a mounted Checks panel, for a resolution that runs without its click', () => { + it('is found while it shows that review, and re-reads it from the review’s provider', async () => { + const panel = model('repo-1::pr::42::sha-2') + const { unmount } = renderHook(() => useChecksPanelAiAcknowledgement(panel)) + + await openChecksPanelViewFor(resolution('github'))?.refresh() + expect(panel.fetchComments).toHaveBeenCalledExactlyOnceWith({ force: true }) + await openChecksPanelViewFor(resolution('gitlab'))?.refresh() + expect(panel.fetchGitLabDetails).toHaveBeenCalledExactlyOnceWith({ commitAsCurrent: true }) + + panel.asyncResultKeyRef.current = 'repo-1::pr::7::sha-1' + expect(openChecksPanelViewFor(resolution('github'))).toBeUndefined() + panel.asyncResultKeyRef.current = 'repo-1::pr::42::sha-2' + unmount() + expect(openChecksPanelViewFor(resolution('github'))).toBeUndefined() + }) +}) diff --git a/src/renderer/src/components/right-sidebar/checks-panel/use-checks-panel-ai-acknowledgement.tsx b/src/renderer/src/components/right-sidebar/checks-panel/use-checks-panel-ai-acknowledgement.tsx index 886ea7181fc1..fb8f438fdd20 100644 --- a/src/renderer/src/components/right-sidebar/checks-panel/use-checks-panel-ai-acknowledgement.tsx +++ b/src/renderer/src/components/right-sidebar/checks-panel/use-checks-panel-ai-acknowledgement.tsx @@ -9,6 +9,7 @@ import type { ChecksPanelReviewDataState } from './use-checks-panel-review-data' import type { ChecksPanelPollingState } from './use-checks-panel-polling' import { acknowledgeCommentResolutionAfterDelivery } from '../pr-comment-resolution-after-delivery' import { clearPRCommentsListSelection } from '../pr-comments-list-selection' +import { registerOpenChecksPanelView } from './open-checks-panel-views' import type { ChecksAgentComposerState } from './panel-state-types' import type { ChecksPanelReview } from '../checks-panel-review' @@ -72,6 +73,18 @@ export function useChecksPanelAiAcknowledgement(model: ChecksPanelAiAcknowledgem [fetchComments, fetchGitLabDetails] ) + // Lends this panel to a resolution that runs without its click, after a window reload. + useEffect( + () => + registerOpenChecksPanelView({ + stableKey: () => checksPanelReviewStableKey(asyncResultKeyRef.current), + existingComments: () => commentsRef.current, + setComments, + refresh: refreshCommentsAfterBulkResolve + }), + [asyncResultKeyRef, commentsRef, setComments, refreshCommentsAfterBulkResolve] + ) + const resolveSelectedThreadsAfterLaunch = useCallback( async (resolution: NonNullable) => { clearSentCommentSelection(resolution.reviewContextKey) diff --git a/src/renderer/src/components/right-sidebar/pr-comment-fixing-reply-body.ts b/src/renderer/src/components/right-sidebar/pr-comment-fixing-reply-body.ts index 5a12b6bc17a7..d41d75f59f4b 100644 --- a/src/renderer/src/components/right-sidebar/pr-comment-fixing-reply-body.ts +++ b/src/renderer/src/components/right-sidebar/pr-comment-fixing-reply-body.ts @@ -23,7 +23,7 @@ export function buildPRCommentConversationReplyBody( const ACK_SNIPPET_MAX_LENGTH = 72 /** First readable line of a comment body, minus HTML comments and markdown markers. */ -function summarizePRCommentBody(body: string): string { +export function summarizePRCommentBody(body: string): string { const cleaned = body.replace(//g, ' ') let start = 0 while (start <= cleaned.length) { diff --git a/src/renderer/src/components/right-sidebar/pr-comment-resolution-after-delivery.ts b/src/renderer/src/components/right-sidebar/pr-comment-resolution-after-delivery.ts index 572d8b4beb00..cff15cd76e90 100644 --- a/src/renderer/src/components/right-sidebar/pr-comment-resolution-after-delivery.ts +++ b/src/renderer/src/components/right-sidebar/pr-comment-resolution-after-delivery.ts @@ -27,7 +27,7 @@ export type CommentResolutionAfterDeliveryDeps = { setComments: (update: (previous: PRComment[]) => PRComment[]) => void refresh: () => Promise } - /** With no panel: refresh the cached comments, so a panel opened later shows them resolved. */ + /** With no panel on that review: refresh the cached comments, so one opened later shows them. */ refreshCache: () => Promise } @@ -178,13 +178,7 @@ async function acknowledge( } }) - if (view) { - if (isPanelStillOnLaunchReview()) { - await view.refresh() - } - } else { - await refreshCache() - } + await (view && isPanelStillOnLaunchReview() ? view.refresh() : refreshCache()) // Why: surface the underlying API error when replies were possible but none landed. // Resolvable threads are acked by resolving, so replied=0 is correct when nothing needed one. diff --git a/src/renderer/src/components/right-sidebar/review-comments-resolution-follow-up.test.ts b/src/renderer/src/components/right-sidebar/review-comments-resolution-follow-up.test.ts new file mode 100644 index 000000000000..061e22dc634d --- /dev/null +++ b/src/renderer/src/components/right-sidebar/review-comments-resolution-follow-up.test.ts @@ -0,0 +1,58 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { checksPanelReviewStableKey, type PendingPRCommentAiAck } from './pr-comments-ai-launch-ack' + +const store = vi.hoisted(() => ({ + fetchPRComments: vi.fn(async () => []), + resolveReviewThread: vi.fn(async () => true), + addPRReviewCommentReply: vi.fn(), + addPRConversationComment: vi.fn(), + settings: null +})) +vi.mock('@/store', () => ({ useAppStore: { getState: () => store } })) +vi.mock('sonner', () => ({ toast: { error: vi.fn(), success: vi.fn() } })) + +const { runReviewCommentsResolutionFollowUp } = + await import('./review-comments-resolution-follow-up') +const { registerOpenChecksPanelView } = await import('./checks-panel/open-checks-panel-views') + +function resolution(provider: 'github' | 'gitlab'): PendingPRCommentAiAck { + return { + reviewContextKey: 'repo-1::pr::42::sha-1', + provider, + selectedGroups: [], + githubResolveTarget: { repoPath: '/repo', repoId: 'repo-1', prNumber: 42 } + } +} + +function panelOn(reviewContextKey: string) { + const panel = { + stableKey: () => checksPanelReviewStableKey(reviewContextKey), + existingComments: () => [], + setComments: vi.fn(), + refresh: vi.fn(async (_provider: string) => {}) + } + return { panel, unregister: registerOpenChecksPanelView(panel) } +} + +beforeEach(() => vi.clearAllMocks()) + +describe('a reload’s comment resolution, with a Checks panel already open', () => { + it.each(['github', 'gitlab'] as const)( + 're-reads that panel on that review (%s), as its own click does', + async (provider) => { + const { panel, unregister } = panelOn('repo-1::pr::42::sha-2') + await runReviewCommentsResolutionFollowUp(resolution(provider)) + expect(panel.refresh).toHaveBeenCalledExactlyOnceWith(provider) + expect(store.fetchPRComments).not.toHaveBeenCalled() + unregister() + } + ) + + it('leaves a panel on another review alone and refreshes the cached comments instead', async () => { + const { panel, unregister } = panelOn('repo-1::pr::7::sha-1') + await runReviewCommentsResolutionFollowUp(resolution('github')) + expect(panel.refresh).not.toHaveBeenCalled() + expect(store.fetchPRComments).toHaveBeenCalledOnce() + unregister() + }) +}) diff --git a/src/renderer/src/components/right-sidebar/review-comments-resolution-follow-up.ts b/src/renderer/src/components/right-sidebar/review-comments-resolution-follow-up.ts index b718aa27d4d4..b1263778ea87 100644 --- a/src/renderer/src/components/right-sidebar/review-comments-resolution-follow-up.ts +++ b/src/renderer/src/components/right-sidebar/review-comments-resolution-follow-up.ts @@ -1,19 +1,24 @@ import { useAppStore } from '@/store' import type { PendingPRCommentAiAck } from './pr-comments-ai-launch-ack' import { acknowledgeCommentResolutionAfterDelivery } from './pr-comment-resolution-after-delivery' +import { openChecksPanelViewFor } from './checks-panel/open-checks-panel-views' /** * The Checks panel's "resolve the selected comments" follow-up, for a window that reloaded while - * its launch ran (`agent-launch-follow-ups`): the panel's own implementation, with no panel on - * screen, then a fresh fetch of the PR's comments so a panel opened later shows them resolved. + * its launch ran (`agent-launch-follow-ups`): the panel's own implementation, through the Checks + * panel open on that review if there is one, else a fresh fetch of the PR's comments so a panel + * opened later shows them resolved. */ export function runReviewCommentsResolutionFollowUp( resolution: PendingPRCommentAiAck ): Promise { const store = useAppStore.getState() const target = resolution.githubResolveTarget + // A panel restored on that review keeps its comments in its own state: update it as its click does. + const view = openChecksPanelViewFor(resolution) return acknowledgeCommentResolutionAfterDelivery(resolution, { actions: store, + ...(view ? { view } : {}), refreshCache: async () => { // GitHub's cached comments; GitLab's are not refreshed here. if (resolution.provider === 'github' && target) { diff --git a/src/renderer/src/lib/agent-launch-follow-up-waiter.test.ts b/src/renderer/src/lib/agent-launch-follow-up-waiter.test.ts new file mode 100644 index 000000000000..e2baca6dc2df --- /dev/null +++ b/src/renderer/src/lib/agent-launch-follow-up-waiter.test.ts @@ -0,0 +1,324 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { + AgentLaunchFollowUp, + AgentLaunchFollowUpTake +} from '../../../shared/agent-launch-follow-up' +import { AGENT_LAUNCH_FOLLOW_UPS_RUNTIME_CAPABILITY } from '../../../shared/agent-launch-runtime-capability' + +const state = vi.hoisted(() => { + const capabilities: string[] = [] + const answers: unknown[] = [] + const calls: unknown[] = [] + const held: { keys: readonly unknown[]; settled: Promise }[] = [] + const heldThreads: { keys: readonly unknown[]; settled: Promise }[] = [] + return { + capabilities, + answers, + calls, + held, + heldThreads, + clearDeliveredDiffComments: vi.fn(async () => true), + runResolution: vi.fn(async () => {}) + } +}) + +vi.mock('@/store', () => ({ + useAppStore: { + getState: () => ({ clearDeliveredDiffComments: state.clearDeliveredDiffComments }) + } +})) +vi.mock('@/runtime/local-runtime-capabilities', () => ({ + ensureLocalRuntimeCapabilities: async () => state.capabilities, + readLocalRuntimeCapabilitiesOrUnknown: () => state.capabilities +})) +vi.mock('@/runtime/runtime-rpc-client', () => ({ + callRuntimeRpc: vi.fn(async (_target: unknown, method: string, params: unknown) => { + state.calls.push([method, params]) + const answer = state.answers.shift() + if (answer instanceof Error) { + throw answer + } + return answer ?? { taken: [], pending: [] } + }) +})) +vi.mock('@/lib/notes-send-in-flight', () => ({ + diffCommentSendKey: (note: { id: string }) => note.id, + holdNotesForSend: (keys: readonly unknown[], settled: Promise) => + state.held.push({ keys, settled }) +})) +vi.mock('@/components/right-sidebar/review-comments-resolution-follow-up', () => ({ + runReviewCommentsResolutionFollowUp: state.runResolution +})) +vi.mock('@/components/right-sidebar/pr-comment-groups-in-flight', () => ({ + holdPRCommentGroupsForSend: (keys: readonly unknown[], settled: Promise) => + state.heldThreads.push({ keys, settled }) +})) + +const { reviewCommentsResolutionFollowUp, reviewNotesDeliveredFollowUp } = + await import('./agent-launch-follow-ups') +const { runRecordedLaunchFollowUps, waitForRecordedLaunchFollowUp } = + await import('./agent-launch-follow-up-waiter') + +const NOTE = { id: 'n1', body: 'fix this', filePath: 'a.ts', lineNumber: 3 } +const NOTES = reviewNotesDeliveredFollowUp('wt-1', [NOTE]) +const COMMENT = { + id: 7, + author: 'reviewer', + authorAvatarUrl: '', + body: 'please rename this\n\nThe private details of why, which no reply quotes.', + createdAt: '2026-01-01T00:00:00Z', + url: 'https://example.test/c/7', + threadId: 'thread-1', + path: 'a.ts', + line: 3 +} +const RESOLUTION = reviewCommentsResolutionFollowUp({ + reviewContextKey: 'pr-1', + provider: 'github', + selectedGroups: [{ kind: 'thread', threadId: 'thread-1', root: COMMENT, replies: [COMMENT] }] +}) + +/** A clock the test turns by hand: timers fire on `advance`, the host's word on `settle`. */ +function manualClock() { + let now = 0 + const timers: { at: number; run: () => void }[] = [] + const listeners = new Set<(operationId: string) => void>() + return { + clock: { + now: () => now, + schedule: (ms: number, run: () => void) => { + const timer = { at: now + ms, run } + timers.push(timer) + return () => timers.splice(timers.indexOf(timer), 1) + }, + onSettled: (listener: (operationId: string) => void) => { + listeners.add(listener) + return () => listeners.delete(listener) + } + }, + advance(ms: number) { + now += ms + for (const timer of timers.filter((t) => t.at <= now)) { + timers.splice(timers.indexOf(timer), 1) + timer.run() + } + }, + settle: (operationId: string) => listeners.forEach((listener) => listener(operationId)), + get listeners() { + return listeners.size + }, + get timers() { + return timers.length + } + } +} + +function taken(operationId: string, followUp: AgentLaunchFollowUp, promptHandedOver = true) { + return { operationId, followUp, promptHandedOver, composerUnobserved: false } +} + +beforeEach(() => { + state.capabilities = [AGENT_LAUNCH_FOLLOW_UPS_RUNTIME_CAPABILITY] + state.answers = [] + state.calls = [] + state.held = [] + state.heldThreads = [] + state.clearDeliveredDiffComments.mockClear() + state.runResolution.mockClear() +}) +afterEach(() => vi.restoreAllMocks()) + +describe('a window that reloaded mid-launch runs what its launches recorded', () => { + it('runs each follow-up whose prompt was handed over, once', async () => { + const take: AgentLaunchFollowUpTake = { + taken: [taken('op-1', NOTES), taken('op-2', RESOLUTION)], + pending: [] + } + state.answers.push(take) + await runRecordedLaunchFollowUps(manualClock().clock) + expect(state.clearDeliveredDiffComments).toHaveBeenCalledExactlyOnceWith('wt-1', [NOTE]) + expect(state.runResolution).toHaveBeenCalledOnce() + expect(state.calls).toEqual([['agent.takeLaunchFollowUps', {}]]) + }) + + it('runs nothing for a launch whose prompt never arrived', async () => { + state.answers.push({ taken: [taken('op-1', NOTES, false)], pending: [] }) + await runRecordedLaunchFollowUps(manualClock().clock) + expect(state.clearDeliveredDiffComments).not.toHaveBeenCalled() + }) + + it('discards a kind or version this build does not know, and a payload it cannot read', async () => { + state.answers.push({ + taken: [ + taken('op-1', { ...NOTES, version: 2 }), + taken('op-2', { kind: 'unknown-kind', version: 1, payload: {} }), + taken('op-3', { ...NOTES, payload: { worktreeId: 'wt-1' } }) + ], + pending: [] + }) + await runRecordedLaunchFollowUps(manualClock().clock) + expect(state.clearDeliveredDiffComments).not.toHaveBeenCalled() + }) + + it('holds what a launch still on its way acts on before running anything else', async () => { + const order: string[] = [] + state.clearDeliveredDiffComments.mockImplementationOnce(async () => { + order.push(`ran with ${state.held.length + state.heldThreads.length} held`) + return true + }) + state.answers.push({ + taken: [taken('op-1', NOTES)], + pending: [ + { operationId: 'op-2', followUp: NOTES, deadline: 60_000 }, + { operationId: 'op-3', followUp: RESOLUTION, deadline: 60_000 } + ] + }) + const t = manualClock() + void runRecordedLaunchFollowUps(t.clock) + await vi.waitFor(() => expect(order).toEqual(['ran with 2 held'])) + expect(state.held[0]!.keys).toEqual(['n1']) + expect(state.heldThreads[0]!.keys).toEqual(['thread:thread-1']) + }) + + it('runs a pending follow-up when the host says its prompt settled, and lets go', async () => { + state.answers.push( + { taken: [], pending: [{ operationId: 'op-1', followUp: NOTES, deadline: 60_000 }] }, + { taken: [taken('op-1', NOTES)], pending: [] } + ) + const t = manualClock() + const run = runRecordedLaunchFollowUps(t.clock) + await vi.waitFor(() => expect(t.listeners).toBe(1)) + let released = false + void state.held[0]!.settled.then(() => (released = true)) + t.settle('op-1') + await run + expect(released).toBe(true) + expect(state.clearDeliveredDiffComments).toHaveBeenCalledOnce() + // The take names the launch, so a click this window makes meanwhile stays its own. + expect(state.calls.slice(1)).toEqual([['agent.takeLaunchFollowUps', { operationId: 'op-1' }]]) + expect(t.listeners).toBe(0) + expect(t.timers).toBe(0) + }) + + it('keeps waiting when the word comes for a launch still owed', async () => { + const stillPending = { + taken: [], + pending: [{ operationId: 'op-1', followUp: NOTES, deadline: 60_000 }] + } + state.answers.push(stillPending, stillPending) + const t = manualClock() + let finished = false + void runRecordedLaunchFollowUps(t.clock).then(() => (finished = true)) + await vi.waitFor(() => expect(t.listeners).toBe(1)) + t.settle('op-1') + await vi.waitFor(() => expect(state.calls).toHaveLength(2)) + expect(finished).toBe(false) + expect(t.listeners).toBe(1) + }) + + it('takes once more just past the host’s deadline, then lets go of a launch still pending', async () => { + const stillPending = { + taken: [], + pending: [{ operationId: 'op-1', followUp: NOTES, deadline: 60_000 }] + } + state.answers.push(stillPending, stillPending) + const t = manualClock() + const run = runRecordedLaunchFollowUps(t.clock) + await vi.waitFor(() => expect(t.timers).toBe(1)) + t.advance(60_000 + 14_999) + expect(state.calls).toHaveLength(1) + t.advance(1) + await run + expect(state.calls).toHaveLength(2) + expect(state.clearDeliveredDiffComments).not.toHaveBeenCalled() + await expect(state.held[0]!.settled).resolves.toBeUndefined() + expect(t.listeners).toBe(0) + }) + + it('runs a follow-up the last take finds settled', async () => { + state.answers.push( + { taken: [], pending: [{ operationId: 'op-1', followUp: NOTES, deadline: 60_000 }] }, + { taken: [taken('op-1', NOTES)], pending: [] } + ) + const t = manualClock() + const run = runRecordedLaunchFollowUps(t.clock) + await vi.waitFor(() => expect(t.timers).toBe(1)) + t.advance(75_000) + await run + expect(state.clearDeliveredDiffComments).toHaveBeenCalledOnce() + }) + + it('asks nothing of a host that does not record follow-ups', async () => { + state.capabilities = [] + await runRecordedLaunchFollowUps(manualClock().clock) + expect(state.calls).toEqual([]) + }) + + it('treats a host that cannot answer as nothing recorded', async () => { + state.answers.push(new Error('method_not_found')) + await expect(runRecordedLaunchFollowUps(manualClock().clock)).resolves.toBeUndefined() + expect(state.clearDeliveredDiffComments).not.toHaveBeenCalled() + }) +}) + +describe('waiting on a launch’s follow-up, one take at a time', () => { + it('keeps the host’s word that arrives while startup’s first take is still asking', async () => { + let answerFirst: (take: AgentLaunchFollowUpTake) => void = () => {} + state.answers.push(new Promise((resolve) => (answerFirst = resolve)), { + taken: [taken('op-1', NOTES)], + pending: [] + }) + const t = manualClock() + const run = runRecordedLaunchFollowUps(t.clock) + await vi.waitFor(() => expect(t.listeners).toBe(1)) + t.settle('op-1') + answerFirst({ + taken: [], + pending: [{ operationId: 'op-1', followUp: NOTES, deadline: 60_000 }] + }) + await run + expect(state.clearDeliveredDiffComments).toHaveBeenCalledOnce() + expect(state.calls).toHaveLength(2) + }) + + it('never lets an empty answer release the hold while another take still runs it', async () => { + let finishRun: () => void = () => {} + state.clearDeliveredDiffComments.mockImplementationOnce( + () => new Promise((resolve) => (finishRun = () => resolve(true))) + ) + state.answers.push( + { taken: [], pending: [{ operationId: 'op-1', followUp: NOTES, deadline: 60_000 }] }, + { taken: [taken('op-1', NOTES)], pending: [] } + ) + const t = manualClock() + const run = runRecordedLaunchFollowUps(t.clock) + await vi.waitFor(() => expect(t.listeners).toBe(1)) + let released = false + void state.held[0]!.settled.then(() => (released = true)) + // Every sweep says so again: two words for one launch. + t.settle('op-1') + t.settle('op-1') + await vi.waitFor(() => expect(state.clearDeliveredDiffComments).toHaveBeenCalledOnce()) + await Promise.resolve() + expect(released).toBe(false) + expect(state.calls).toHaveLength(2) + finishRun() + await run + expect(released).toBe(true) + // The second word found it already run, and asked nothing. + expect(state.calls).toHaveLength(2) + }) + + it('holds a click’s follow-up its own take missed, and runs it when the host says so', async () => { + state.answers.push({ taken: [taken('op-1', RESOLUTION)], pending: [] }) + const t = manualClock() + const waited = waitForRecordedLaunchFollowUp('op-1', RESOLUTION, undefined, t.clock) + expect(state.heldThreads[0]!.keys).toEqual(['thread:thread-1']) + expect(t.timers).toBe(1) + t.settle('op-1') + await waited + expect(state.runResolution).toHaveBeenCalledOnce() + expect(t.listeners).toBe(0) + expect(t.timers).toBe(0) + }) +}) diff --git a/src/renderer/src/lib/agent-launch-follow-up-waiter.ts b/src/renderer/src/lib/agent-launch-follow-up-waiter.ts new file mode 100644 index 000000000000..183e8aa0b51e --- /dev/null +++ b/src/renderer/src/lib/agent-launch-follow-up-waiter.ts @@ -0,0 +1,190 @@ +/** + * The follow-ups this window waits on (`agent-launch-follow-ups`): ones its launches recorded that + * the host has not settled yet, found at startup after a reload, or left behind by a click whose own + * take missed. Each one's notes or threads stay unsendable while it waits. It is taken when the host + * says its prompt settled, or once more just past the host's own deadline, then let go. + * + * One take per launch runs at a time: a second answer that finds it already gone must not release + * the hold while the first take is still running it. + */ + +import { ensureLocalRuntimeCapabilities } from '@/runtime/local-runtime-capabilities' +import type { AgentLaunchFollowUp } from '../../../shared/agent-launch-follow-up' +import { AGENT_LAUNCH_FOLLOW_UPS_RUNTIME_CAPABILITY } from '../../../shared/agent-launch-runtime-capability' +import { + readLaunchFollowUp, + runTakenLaunchFollowUps, + takeLaunchFollowUps +} from './agent-launch-follow-ups' + +/** How long past the host's own deadline a waiting window takes once more, then lets go. */ +const PAST_DEADLINE_GRACE_MS = 15_000 +/** For a launch whose deadline the host could not say: its owed-prompt deadline. */ +const FALLBACK_WAIT_MS = 5 * 60_000 + +export type RecordedLaunchFollowUpClock = { + now: () => number + /** Runs `run` after `ms`; returns its cancel. */ + schedule: (ms: number, run: () => void) => () => void + /** The host's word that a launch settled its prompt; returns its unsubscribe. */ + onSettled: (listener: (operationId: string) => void) => () => void +} + +const WINDOW_CLOCK: RecordedLaunchFollowUpClock = { + now: () => Date.now(), + schedule: (ms, run) => { + const timer = setTimeout(run, ms) + return () => clearTimeout(timer) + }, + onSettled: (listener) => + window.api.ui.onAgentLaunchPromptSettled?.((event) => listener(event.operationId)) ?? (() => {}) +} + +type Waiting = { done: Promise; finish: () => void } + +class LaunchFollowUpWaiter { + private readonly waiting = new Map() + private readonly takes = new Map>() + /** Words heard for launches not waited on yet, kept only while startup's first take runs. */ + private heard: Set | null = null + private unsubscribe: (() => void) | null = null + + constructor(private readonly clock: RecordedLaunchFollowUpClock) {} + + /** Listens before anything is taken, so a word sent while startup asks is not lost. */ + startCollecting(): void { + this.heard = new Set() + this.listen() + } + + stopCollecting(): void { + this.heard = null + this.stopListeningWhenIdle() + } + + /** Holds what the follow-up acts on until it is run, or the wait gives up. */ + wait(operationId: string, followUp: AgentLaunchFollowUp, deadline?: number): Promise { + const existing = this.waiting.get(operationId) + if (existing) { + return existing.done + } + let release: () => void = () => {} + const done = new Promise((resolve) => (release = resolve)) + readLaunchFollowUp(followUp)?.holdUntil?.(done) + const waitMs = (deadline ?? this.clock.now() + FALLBACK_WAIT_MS) - this.clock.now() + const cancelTimer = this.clock.schedule(Math.max(0, waitMs) + PAST_DEADLINE_GRACE_MS, () => { + void this.take(operationId, true) + }) + this.waiting.set(operationId, { + done, + finish: () => { + cancelTimer() + release() + } + }) + this.listen() + if (this.heard?.has(operationId)) { + void this.take(operationId, false) + } + return done + } + + private listen(): void { + this.unsubscribe ??= this.clock.onSettled((operationId) => { + if (this.waiting.has(operationId)) { + void this.take(operationId, false) + } else { + this.heard?.add(operationId) + } + }) + } + + private stopListeningWhenIdle(): void { + if (this.waiting.size === 0 && this.heard === null) { + this.unsubscribe?.() + this.unsubscribe = null + } + } + + private take(operationId: string, last: boolean): Promise { + const next = (this.takes.get(operationId) ?? Promise.resolve()).then(() => + this.takeNow(operationId, last) + ) + this.takes.set(operationId, next) + void next.finally(() => { + if (this.takes.get(operationId) === next) { + this.takes.delete(operationId) + } + }) + return next + } + + private async takeNow(operationId: string, last: boolean): Promise { + if (!this.waiting.has(operationId)) { + return + } + const take = await takeLaunchFollowUps(operationId) + if (take && !take.pending.some((entry) => entry.operationId === operationId)) { + await runTakenLaunchFollowUps(take.taken) + this.finish(operationId) + } else if (last) { + this.finish(operationId) + } + } + + private finish(operationId: string): void { + this.waiting.get(operationId)?.finish() + this.waiting.delete(operationId) + this.stopListeningWhenIdle() + } +} + +const waiters = new WeakMap() + +function waiterFor(clock: RecordedLaunchFollowUpClock): LaunchFollowUpWaiter { + let waiter = waiters.get(clock) + if (!waiter) { + waiter = new LaunchFollowUpWaiter(clock) + waiters.set(clock, waiter) + } + return waiter +} + +/** + * Startup: holds what launches still on their way act on, before anything else, then runs the + * follow-ups launches finished while this window was gone, and waits on the rest. + */ +export async function runRecordedLaunchFollowUps( + clock: RecordedLaunchFollowUpClock = WINDOW_CLOCK +): Promise { + const capabilities = await ensureLocalRuntimeCapabilities() + if (!capabilities?.includes(AGENT_LAUNCH_FOLLOW_UPS_RUNTIME_CAPABILITY)) { + return + } + const waiter = waiterFor(clock) + waiter.startCollecting() + try { + const first = await takeLaunchFollowUps() + if (!first) { + return + } + const waits = first.pending.map(({ operationId, followUp, deadline }) => + waiter.wait(operationId, followUp, deadline) + ) + waiter.stopCollecting() + await runTakenLaunchFollowUps(first.taken) + await Promise.all(waits) + } finally { + waiter.stopCollecting() + } +} + +/** A click's follow-up still on the record after its own take missed: waited on as startup's are. */ +export function waitForRecordedLaunchFollowUp( + operationId: string, + followUp: AgentLaunchFollowUp, + deadline?: number, + clock: RecordedLaunchFollowUpClock = WINDOW_CLOCK +): Promise { + return waiterFor(clock).wait(operationId, followUp, deadline) +} diff --git a/src/renderer/src/lib/agent-launch-follow-ups.test.ts b/src/renderer/src/lib/agent-launch-follow-ups.test.ts index 6c2dfc794014..2dc35524c455 100644 --- a/src/renderer/src/lib/agent-launch-follow-ups.test.ts +++ b/src/renderer/src/lib/agent-launch-follow-ups.test.ts @@ -1,9 +1,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import type { - AgentLaunchFollowUp, - AgentLaunchFollowUpTake -} from '../../../shared/agent-launch-follow-up' import { AGENT_LAUNCH_FOLLOW_UPS_RUNTIME_CAPABILITY } from '../../../shared/agent-launch-runtime-capability' +import type { PRCommentGroup } from '../../../shared/pr-comment-groups' +import { buildPRCommentBatchConversationReplyBody } from '@/components/right-sidebar/pr-comment-fixing-reply-body' const state = vi.hoisted(() => { const capabilities: string[] = [] @@ -54,12 +52,8 @@ vi.mock('@/components/right-sidebar/pr-comment-groups-in-flight', () => ({ state.heldThreads.push({ keys, settled }) })) -const { - recordableLaunchFollowUp, - reviewCommentsResolutionFollowUp, - reviewNotesDeliveredFollowUp, - runRecordedLaunchFollowUps -} = await import('./agent-launch-follow-ups') +const { recordableLaunchFollowUp, reviewCommentsResolutionFollowUp, reviewNotesDeliveredFollowUp } = + await import('./agent-launch-follow-ups') const NOTE = { id: 'n1', body: 'fix this', filePath: 'a.ts', lineNumber: 3 } const NOTES = reviewNotesDeliveredFollowUp('wt-1', [NOTE]) @@ -67,7 +61,7 @@ const COMMENT = { id: 7, author: 'reviewer', authorAvatarUrl: '', - body: 'please rename this', + body: 'please rename this\n\nThe private details of why, which no reply quotes.', createdAt: '2026-01-01T00:00:00Z', url: 'https://example.test/c/7', threadId: 'thread-1', @@ -80,45 +74,6 @@ const RESOLUTION = reviewCommentsResolutionFollowUp({ selectedGroups: [{ kind: 'thread', threadId: 'thread-1', root: COMMENT, replies: [COMMENT] }] }) -/** A clock the test turns by hand: timers fire on `advance`, the host's word on `settle`. */ -function manualClock() { - let now = 0 - const timers: { at: number; run: () => void }[] = [] - const listeners = new Set<(operationId: string) => void>() - return { - clock: { - now: () => now, - schedule: (ms: number, run: () => void) => { - const timer = { at: now + ms, run } - timers.push(timer) - return () => timers.splice(timers.indexOf(timer), 1) - }, - onSettled: (listener: (operationId: string) => void) => { - listeners.add(listener) - return () => listeners.delete(listener) - } - }, - advance(ms: number) { - now += ms - for (const timer of timers.filter((t) => t.at <= now)) { - timers.splice(timers.indexOf(timer), 1) - timer.run() - } - }, - settle: (operationId: string) => listeners.forEach((listener) => listener(operationId)), - get listeners() { - return listeners.size - }, - get timers() { - return timers.length - } - } -} - -function taken(operationId: string, followUp: AgentLaunchFollowUp, promptHandedOver = true) { - return { operationId, followUp, promptHandedOver, composerUnobserved: false } -} - beforeEach(() => { state.capabilities = [AGENT_LAUNCH_FOLLOW_UPS_RUNTIME_CAPABILITY] state.answers = [] @@ -130,144 +85,33 @@ beforeEach(() => { }) afterEach(() => vi.restoreAllMocks()) -describe('a window that reloaded mid-launch runs what its launches recorded', () => { - it('runs each follow-up whose prompt was handed over, once', async () => { - const take: AgentLaunchFollowUpTake = { - taken: [taken('op-1', NOTES), taken('op-2', RESOLUTION)], - pending: [] - } - state.answers.push(take) - await runRecordedLaunchFollowUps(manualClock().clock) - expect(state.clearDeliveredDiffComments).toHaveBeenCalledExactlyOnceWith('wt-1', [NOTE]) - expect(state.runResolution).toHaveBeenCalledOnce() - expect(state.calls).toEqual([['agent.takeLaunchFollowUps', {}]]) - }) - - it('runs nothing for a launch whose prompt never arrived', async () => { - state.answers.push({ taken: [taken('op-1', NOTES, false)], pending: [] }) - await runRecordedLaunchFollowUps(manualClock().clock) - expect(state.clearDeliveredDiffComments).not.toHaveBeenCalled() - }) - - it('discards a kind or version this build does not know, and a payload it cannot read', async () => { - state.answers.push({ - taken: [ - taken('op-1', { ...NOTES, version: 2 }), - taken('op-2', { kind: 'unknown-kind', version: 1, payload: {} }), - taken('op-3', { ...NOTES, payload: { worktreeId: 'wt-1' } }) - ], - pending: [] - }) - await runRecordedLaunchFollowUps(manualClock().clock) - expect(state.clearDeliveredDiffComments).not.toHaveBeenCalled() - }) +function isRecordedResolution(value: unknown): value is { selectedGroups: PRCommentGroup[] } { + return typeof value === 'object' && value !== null && 'selectedGroups' in value +} - it('holds what a launch still on its way acts on before running anything else', async () => { - const order: string[] = [] - state.clearDeliveredDiffComments.mockImplementationOnce(async () => { - order.push(`ran with ${state.held.length + state.heldThreads.length} held`) - return true - }) - state.answers.push({ - taken: [taken('op-1', NOTES)], - pending: [ - { operationId: 'op-2', followUp: NOTES, deadline: 60_000 }, - { operationId: 'op-3', followUp: RESOLUTION, deadline: 60_000 } +describe('which follow-ups a launch records', () => { + it('records of each comment only the snippet its "Fixing:" reply quotes', () => { + expect(JSON.stringify(RESOLUTION.payload)).not.toContain('private details') + expect(JSON.stringify(RESOLUTION.payload)).toContain('thread-1') + // A reply built after a reload reads as the click's would, long first lines included. + const long = { ...COMMENT, id: 8, body: `${'word '.repeat(40)}\nmore` } + const recorded = reviewCommentsResolutionFollowUp({ + reviewContextKey: 'pr-1', + provider: 'github', + selectedGroups: [ + { kind: 'standalone', comment: COMMENT }, + { kind: 'standalone', comment: long } ] - }) - const t = manualClock() - void runRecordedLaunchFollowUps(t.clock) - await vi.waitFor(() => expect(order).toEqual(['ran with 2 held'])) - expect(state.held[0]!.keys).toEqual(['n1']) - expect(state.heldThreads[0]!.keys).toEqual(['thread:thread-1']) - }) - - it('runs a pending follow-up when the host says its prompt settled, and lets go', async () => { - state.answers.push( - { taken: [], pending: [{ operationId: 'op-1', followUp: NOTES, deadline: 60_000 }] }, - { taken: [taken('op-1', NOTES)], pending: [] } - ) - const t = manualClock() - const run = runRecordedLaunchFollowUps(t.clock) - await vi.waitFor(() => expect(t.listeners).toBe(1)) - let released = false - void state.held[0]!.settled.then(() => (released = true)) - t.settle('op-1') - await run - expect(released).toBe(true) - expect(state.clearDeliveredDiffComments).toHaveBeenCalledOnce() - // The take names the launch, so a click this window makes meanwhile stays its own. - expect(state.calls.slice(1)).toEqual([['agent.takeLaunchFollowUps', { operationId: 'op-1' }]]) - expect(t.listeners).toBe(0) - expect(t.timers).toBe(0) - }) - - it('keeps waiting when the word comes for a launch still owed', async () => { - const stillPending = { - taken: [], - pending: [{ operationId: 'op-1', followUp: NOTES, deadline: 60_000 }] - } - state.answers.push(stillPending, stillPending) - const t = manualClock() - let finished = false - void runRecordedLaunchFollowUps(t.clock).then(() => (finished = true)) - await vi.waitFor(() => expect(t.listeners).toBe(1)) - t.settle('op-1') - await vi.waitFor(() => expect(state.calls).toHaveLength(2)) - expect(finished).toBe(false) - expect(t.listeners).toBe(1) - }) - - it('takes once more just past the host’s deadline, then lets go of a launch still pending', async () => { - const stillPending = { - taken: [], - pending: [{ operationId: 'op-1', followUp: NOTES, deadline: 60_000 }] + }).payload + if (!isRecordedResolution(recorded)) { + throw new Error('unexpected payload') } - state.answers.push(stillPending, stillPending) - const t = manualClock() - const run = runRecordedLaunchFollowUps(t.clock) - await vi.waitFor(() => expect(t.timers).toBe(1)) - t.advance(60_000 + 14_999) - expect(state.calls).toHaveLength(1) - t.advance(1) - await run - expect(state.calls).toHaveLength(2) - expect(state.clearDeliveredDiffComments).not.toHaveBeenCalled() - await expect(state.held[0]!.settled).resolves.toBeUndefined() - expect(t.listeners).toBe(0) - }) - - it('runs a follow-up the last take finds settled', async () => { - state.answers.push( - { taken: [], pending: [{ operationId: 'op-1', followUp: NOTES, deadline: 60_000 }] }, - { taken: [taken('op-1', NOTES)], pending: [] } + const comments = recorded.selectedGroups.flatMap((group) => + group.kind === 'standalone' ? [group.comment] : [] + ) + expect(buildPRCommentBatchConversationReplyBody(comments)).toBe( + buildPRCommentBatchConversationReplyBody([COMMENT, long]) ) - const t = manualClock() - const run = runRecordedLaunchFollowUps(t.clock) - await vi.waitFor(() => expect(t.timers).toBe(1)) - t.advance(75_000) - await run - expect(state.clearDeliveredDiffComments).toHaveBeenCalledOnce() - }) - - it('asks nothing of a host that does not record follow-ups', async () => { - state.capabilities = [] - await runRecordedLaunchFollowUps(manualClock().clock) - expect(state.calls).toEqual([]) - }) - - it('treats a host that cannot answer as nothing recorded', async () => { - state.answers.push(new Error('method_not_found')) - await expect(runRecordedLaunchFollowUps(manualClock().clock)).resolves.toBeUndefined() - expect(state.clearDeliveredDiffComments).not.toHaveBeenCalled() - }) -}) - -describe('which follow-ups a launch records', () => { - it('records review threads without the comments’ words', () => { - const payload = RESOLUTION.payload - expect(JSON.stringify(payload)).not.toContain('please rename this') - expect(JSON.stringify(payload)).toContain('thread-1') }) it('only on a host that keeps them, and only under the size cap', () => { diff --git a/src/renderer/src/lib/agent-launch-follow-ups.ts b/src/renderer/src/lib/agent-launch-follow-ups.ts index f452e824af98..388d8038572e 100644 --- a/src/renderer/src/lib/agent-launch-follow-ups.ts +++ b/src/renderer/src/lib/agent-launch-follow-ups.ts @@ -10,15 +10,13 @@ import { useAppStore } from '@/store' import { callRuntimeRpc } from '@/runtime/runtime-rpc-client' -import { - ensureLocalRuntimeCapabilities, - readLocalRuntimeCapabilitiesOrUnknown -} from '@/runtime/local-runtime-capabilities' +import { readLocalRuntimeCapabilitiesOrUnknown } from '@/runtime/local-runtime-capabilities' import { diffCommentSendKey, holdNotesForSend } from '@/lib/notes-send-in-flight' import type { DiffCommentDeliverySnapshot } from '@/store/slices/diffComments' import type { PendingPRCommentAiAck } from '@/components/right-sidebar/pr-comments-ai-launch-ack' import { runReviewCommentsResolutionFollowUp } from '@/components/right-sidebar/review-comments-resolution-follow-up' import { holdPRCommentGroupsForSend } from '@/components/right-sidebar/pr-comment-groups-in-flight' +import { summarizePRCommentBody } from '@/components/right-sidebar/pr-comment-fixing-reply-body' import { getPRCommentGroupId, type PRCommentGroup } from '../../../shared/pr-comment-groups' import type { PRComment } from '../../../shared/github/comment-types' import { @@ -53,13 +51,16 @@ export function reviewNotesDeliveredFollowUp( export function reviewCommentsResolutionFollowUp( resolution: PendingPRCommentAiAck ): AgentLaunchFollowUp { - // Without comment bodies: resolving and replying never read them, and they are other people's - // words that would otherwise sit in the launch record. - const withoutBody = (comment: PRComment): PRComment => ({ ...comment, body: '' }) + // Each comment keeps only the one-line snippet the batched "Fixing:" reply quotes (which reads + // the same from it); the rest is other people's words that would otherwise sit in the record. + const snippetOnly = (comment: PRComment): PRComment => ({ + ...comment, + body: summarizePRCommentBody(comment.body) + }) const selectedGroups = resolution.selectedGroups.map((group): PRCommentGroup => group.kind === 'thread' - ? { ...group, root: withoutBody(group.root), replies: group.replies.map(withoutBody) } - : { ...group, comment: withoutBody(group.comment) } + ? { ...group, root: snippetOnly(group.root), replies: group.replies.map(snippetOnly) } + : { ...group, comment: snippetOnly(group.comment) } ) return { kind: REVIEW_COMMENTS_RESOLUTION, @@ -104,7 +105,7 @@ function isResolutionPayload(value: unknown): value is PendingPRCommentAiAck { } /** How to run a recorded follow-up, and keep what it acts on unsendable until it runs. */ -function readFollowUp(followUp: AgentLaunchFollowUp): { +export function readLaunchFollowUp(followUp: AgentLaunchFollowUp): { run: () => Promise holdUntil?: (settled: Promise) => void } | null { @@ -166,95 +167,14 @@ export async function takeLaunchFollowUps( } } -async function runTaken(take: AgentLaunchFollowUpTake['taken']): Promise { +/** Runs each taken follow-up whose prompt was handed over, once; a failure is only logged. */ +export async function runTakenLaunchFollowUps( + take: AgentLaunchFollowUpTake['taken'] +): Promise { for (const entry of take) { - const followUp = entry.promptHandedOver ? readFollowUp(entry.followUp) : null + const followUp = entry.promptHandedOver ? readLaunchFollowUp(entry.followUp) : null await followUp?.run().catch((error: unknown) => { console.warn(`[agent-launch] the ${entry.followUp.kind} follow-up failed`, error) }) } } - -/** How long past the host's own deadline a waiting window takes once more, then lets go. */ -const PAST_DEADLINE_GRACE_MS = 15_000 -/** For a pending entry whose deadline the host could not say: its owed-prompt deadline. */ -const FALLBACK_WAIT_MS = 5 * 60_000 - -export type RecordedLaunchFollowUpClock = { - now: () => number - /** Runs `run` after `ms`; returns its cancel. */ - schedule: (ms: number, run: () => void) => () => void - /** The host's word that a launch settled its prompt; returns its unsubscribe. */ - onSettled: (listener: (operationId: string) => void) => () => void -} - -const WINDOW_CLOCK: RecordedLaunchFollowUpClock = { - now: () => Date.now(), - schedule: (ms, run) => { - const timer = setTimeout(run, ms) - return () => clearTimeout(timer) - }, - onSettled: (listener) => - window.api.ui.onAgentLaunchPromptSettled?.((event) => listener(event.operationId)) ?? (() => {}) -} - -/** - * Startup: holds what launches still on their way act on, before anything else, then runs the - * follow-ups launches finished while this window was gone. Each one still pending is taken when - * the host says its prompt settled, or once more just past the host's own deadline; one still - * pending then stays on its row for the next start, and nothing is held any more. - */ -export async function runRecordedLaunchFollowUps( - clock: RecordedLaunchFollowUpClock = WINDOW_CLOCK -): Promise { - const capabilities = await ensureLocalRuntimeCapabilities() - if (!capabilities?.includes(AGENT_LAUNCH_FOLLOW_UPS_RUNTIME_CAPABILITY)) { - return - } - const first = await takeLaunchFollowUps() - if (!first) { - return - } - const waiting = new Map void>() - for (const { operationId, followUp } of first.pending) { - let release: () => void = () => {} - readFollowUp(followUp)?.holdUntil?.(new Promise((resolve) => (release = resolve))) - waiting.set(operationId, release) - } - await runTaken(first.taken) - if (waiting.size === 0) { - return - } - await new Promise((done) => { - const cancels: (() => void)[] = [] - const finish = (operationId: string): void => { - waiting.get(operationId)?.() - waiting.delete(operationId) - if (waiting.size === 0) { - cancels.forEach((cancel) => cancel()) - done() - } - } - const takeOne = async (operationId: string, last: boolean): Promise => { - if (!waiting.has(operationId)) { - return - } - const take = await takeLaunchFollowUps(operationId) - if (take && take.pending.length === 0) { - await runTaken(take.taken) - finish(operationId) - } else if (last) { - finish(operationId) - } - } - cancels.push(clock.onSettled((operationId) => void takeOne(operationId, false))) - for (const { operationId, deadline } of first.pending) { - const waitMs = (deadline ?? clock.now() + FALLBACK_WAIT_MS) - clock.now() - cancels.push( - clock.schedule(Math.max(0, waitMs) + PAST_DEADLINE_GRACE_MS, () => { - void takeOne(operationId, true) - }) - ) - } - }) -} diff --git a/src/renderer/src/lib/launch-agent-new-tab-host-route.test.ts b/src/renderer/src/lib/launch-agent-new-tab-host-route.test.ts index 8cef7e00b9ed..6f95b9792c71 100644 --- a/src/renderer/src/lib/launch-agent-new-tab-host-route.test.ts +++ b/src/renderer/src/lib/launch-agent-new-tab-host-route.test.ts @@ -26,6 +26,7 @@ const followUps = vi.hoisted(() => { return { hostRecords: true, order, + wait: vi.fn(async (_operationId: string, _followUp: unknown, _deadline?: number) => {}), takeLaunchFollowUps: vi.fn( async (_operationId?: string): Promise => ({ taken: [], @@ -42,6 +43,10 @@ vi.mock('@/lib/agent-launch-follow-ups', () => ({ } })) +vi.mock('@/lib/agent-launch-follow-up-waiter', () => ({ + waitForRecordedLaunchFollowUp: followUps.wait +})) + const { launchNewTabPromptThroughHost, newTabPromptLaunchesThroughHost } = await import('./launch-agent-new-tab-host-route') @@ -219,21 +224,31 @@ describe('a click whose follow-up is recorded on its launch', () => { expect(followUps.order).toEqual(['take', 'follow-up']) }) - it('leaves it to the next start when this click could not take it, and says so', async () => { - // Taken by a reloaded window already, or the host could not answer: it runs there, never here. - for (const take of [{ taken: [], pending: [] }, null]) { - followUps.takeLaunchFollowUps.mockResolvedValueOnce(take) - deferredOutcome()({ - kind: 'started', - prompt: { delivery: 'submit', outcome: 'handed-to-terminal' } - }) - const onPromptDelivered = vi.fn() - await expect(launchWithFollowUp(onPromptDelivered)).resolves.toEqual({ - delivered: true, - failureNotified: false, - followUpDeferred: true - }) - expect(onPromptDelivered).not.toHaveBeenCalled() + it.each([ + ['another take returned it, and runs it there', { taken: [], pending: [] }, undefined], + [ + 'the host has not settled its record, so it waits until the host says so', + { taken: [], pending: [{ operationId: 'op-1', followUp: FOLLOW_UP, deadline: 9_000 }] }, + { deadline: 9_000 } + ], + ['its take failed, so it waits as a reloaded window would', null, { deadline: undefined }] + ])('never runs a follow-up its own take missed: %s', async (_, take, waits) => { + followUps.takeLaunchFollowUps.mockResolvedValueOnce(take) + deferredOutcome()({ + kind: 'started', + prompt: { delivery: 'submit', outcome: 'handed-to-terminal' } + }) + const onPromptDelivered = vi.fn() + await expect(launchWithFollowUp(onPromptDelivered)).resolves.toEqual({ + delivered: true, + failureNotified: false, + followUpDeferred: true + }) + expect(onPromptDelivered).not.toHaveBeenCalled() + if (waits) { + expect(followUps.wait).toHaveBeenCalledExactlyOnceWith('op-1', FOLLOW_UP, waits.deadline) + } else { + expect(followUps.wait).not.toHaveBeenCalled() } }) diff --git a/src/renderer/src/lib/launch-agent-new-tab-host-route.ts b/src/renderer/src/lib/launch-agent-new-tab-host-route.ts index a19ad2054a5d..0d852cdb75f9 100644 --- a/src/renderer/src/lib/launch-agent-new-tab-host-route.ts +++ b/src/renderer/src/lib/launch-agent-new-tab-host-route.ts @@ -15,6 +15,7 @@ import type { AgentLaunchPromptReceipt } from '../../../shared/agent-launch-inte import type { TuiAgent } from '../../../shared/tui-agent' import type { AgentLaunchFollowUp } from '../../../shared/agent-launch-follow-up' import { recordableLaunchFollowUp, takeLaunchFollowUps } from '@/lib/agent-launch-follow-ups' +import { waitForRecordedLaunchFollowUp } from '@/lib/agent-launch-follow-up-waiter' /** `followUpDeferred`: the click's recorded follow-up was left for the next start; don't run it. */ export type NewTabPromptDeliveryResult = { @@ -78,20 +79,20 @@ async function settleHostPrompt( onPromptDelivered?: () => void onPromptDeliveryUnconfirmed?: () => void }, - launch: { tabId: string; operationId: string; followUpRecorded: boolean }, + launch: { tabId: string; operationId: string; followUp: AgentLaunchFollowUp | undefined }, receipt: AgentLaunchPromptReceipt | undefined, seeded: boolean ): Promise { const { tabId } = launch - // Runs only once this click took it off the record: a take that failed leaves it there, for the - // next start to run, and running it here too would run it twice. + // Runs here only when this click's own take returned it; one another take returned runs there. let followUpRunsHere = true - if (launch.followUpRecorded) { + let stillRecorded: { deadline?: number } | null = null + if (launch.followUp) { const take = await takeLaunchFollowUps(launch.operationId) - followUpRunsHere = - take?.taken.some( - (entry) => entry.operationId === launch.operationId && entry.promptHandedOver - ) ?? false + const mine = (entry: { operationId: string }): boolean => + entry.operationId === launch.operationId + followUpRunsHere = take?.taken.some((entry) => mine(entry) && entry.promptHandedOver) ?? false + stillRecorded = take ? (take.pending.find(mine) ?? null) : {} } if (receipt?.outcome === 'handed-to-terminal') { if (receipt.composerUnobserved) { @@ -102,6 +103,15 @@ async function settleHostPrompt( seedCommandCodeSubmittedPromptStatus(args.worktreeId, tabId, args.prompt) } if (!followUpRunsHere) { + if (launch.followUp && stillRecorded) { + // The take failed, or the host has not settled the record: hold what it acts on, and take + // it when the host says so, as a reloaded window would. + void waitForRecordedLaunchFollowUp( + launch.operationId, + launch.followUp, + stillRecorded.deadline + ) + } return { delivered: true, failureNotified: false, followUpDeferred: true } } args.onPromptDelivered?.() @@ -162,12 +172,7 @@ export function launchNewTabPromptThroughHost( // Seeded once the host started the agent, as main's paste seeded it: a launch that never // started leaves no chat copy behind. const seeded = seedChatCopy(tabId, args.agent, pasteContent, clickedAt) - return settleHostPrompt( - args, - { tabId, operationId, followUpRecorded: followUp !== undefined }, - launched.prompt, - seeded - ) + return settleHostPrompt(args, { tabId, operationId, followUp }, launched.prompt, seeded) } // The pane, or this notice for a tab that went, already says why: never a second notice. showLaunchNotStartedNotice(launched, args.prompt) From 84cf1d5c8efb6baf3ca6b0b33af0649feaffce9d Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 7 Oct 2026 04:12:10 -0700 Subject: [PATCH 4/8] fix(agent-launch): ask again soon for a click's follow-up its own take missed --- .../lib/agent-launch-follow-up-waiter.test.ts | 15 ++++++++++++- .../src/lib/agent-launch-follow-up-waiter.ts | 21 ++++++++++++++++--- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/src/renderer/src/lib/agent-launch-follow-up-waiter.test.ts b/src/renderer/src/lib/agent-launch-follow-up-waiter.test.ts index e2baca6dc2df..abcbe4510e55 100644 --- a/src/renderer/src/lib/agent-launch-follow-up-waiter.test.ts +++ b/src/renderer/src/lib/agent-launch-follow-up-waiter.test.ts @@ -314,11 +314,24 @@ describe('waiting on a launch’s follow-up, one take at a time', () => { const t = manualClock() const waited = waitForRecordedLaunchFollowUp('op-1', RESOLUTION, undefined, t.clock) expect(state.heldThreads[0]!.keys).toEqual(['thread:thread-1']) - expect(t.timers).toBe(1) t.settle('op-1') await waited expect(state.runResolution).toHaveBeenCalledOnce() expect(t.listeners).toBe(0) expect(t.timers).toBe(0) }) + + it('asks again a few seconds after a click’s take failed, not at the deadline', async () => { + // The host's word came before the click's own take, so none will come again. + state.answers.push({ taken: [taken('op-1', RESOLUTION)], pending: [] }) + const t = manualClock() + const waited = waitForRecordedLaunchFollowUp('op-1', RESOLUTION, undefined, t.clock) + t.advance(2_999) + expect(state.calls).toEqual([]) + t.advance(1) + await waited + expect(state.calls).toEqual([['agent.takeLaunchFollowUps', { operationId: 'op-1' }]]) + expect(state.runResolution).toHaveBeenCalledOnce() + expect(t.timers).toBe(0) + }) }) diff --git a/src/renderer/src/lib/agent-launch-follow-up-waiter.ts b/src/renderer/src/lib/agent-launch-follow-up-waiter.ts index 183e8aa0b51e..48873a59b444 100644 --- a/src/renderer/src/lib/agent-launch-follow-up-waiter.ts +++ b/src/renderer/src/lib/agent-launch-follow-up-waiter.ts @@ -21,6 +21,8 @@ import { const PAST_DEADLINE_GRACE_MS = 15_000 /** For a launch whose deadline the host could not say: its owed-prompt deadline. */ const FALLBACK_WAIT_MS = 5 * 60_000 +/** A click's own take that missed is asked again once this soon: the host's word came before it. */ +const CLICK_RETAKE_MS = 3_000 export type RecordedLaunchFollowUpClock = { now: () => number @@ -63,7 +65,12 @@ class LaunchFollowUpWaiter { } /** Holds what the follow-up acts on until it is run, or the wait gives up. */ - wait(operationId: string, followUp: AgentLaunchFollowUp, deadline?: number): Promise { + wait( + operationId: string, + followUp: AgentLaunchFollowUp, + deadline?: number, + retakeAfterMs?: number + ): Promise { const existing = this.waiting.get(operationId) if (existing) { return existing.done @@ -75,10 +82,15 @@ class LaunchFollowUpWaiter { const cancelTimer = this.clock.schedule(Math.max(0, waitMs) + PAST_DEADLINE_GRACE_MS, () => { void this.take(operationId, true) }) + const cancelRetake = + retakeAfterMs === undefined + ? () => {} + : this.clock.schedule(retakeAfterMs, () => void this.take(operationId, false)) this.waiting.set(operationId, { done, finish: () => { cancelTimer() + cancelRetake() release() } }) @@ -179,12 +191,15 @@ export async function runRecordedLaunchFollowUps( } } -/** A click's follow-up still on the record after its own take missed: waited on as startup's are. */ +/** + * A click's follow-up still on the record after its own take missed: waited on as startup's are, + * and asked for again once soon, since the host's word that it settled came before that take. + */ export function waitForRecordedLaunchFollowUp( operationId: string, followUp: AgentLaunchFollowUp, deadline?: number, clock: RecordedLaunchFollowUpClock = WINDOW_CLOCK ): Promise { - return waiterFor(clock).wait(operationId, followUp, deadline) + return waiterFor(clock).wait(operationId, followUp, deadline, CLICK_RETAKE_MS) } From 128843a351f327f7f180910ad993dbb49427b034 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 7 Oct 2026 04:27:51 -0700 Subject: [PATCH 5/8] fix(agent-launch): a launch past the record's cap runs its click's follow-up live --- .../src/lib/agent-launch-through-host.test.ts | 13 +++++++++---- .../src/lib/agent-launch-through-host.ts | 11 +++++++---- .../lib/launch-agent-new-tab-host-route.test.ts | 16 ++++++++++++++++ .../src/lib/launch-agent-new-tab-host-route.ts | 9 ++++++++- 4 files changed, 40 insertions(+), 9 deletions(-) diff --git a/src/renderer/src/lib/agent-launch-through-host.test.ts b/src/renderer/src/lib/agent-launch-through-host.test.ts index 1967e6d8097c..8e0db6a5ff12 100644 --- a/src/renderer/src/lib/agent-launch-through-host.test.ts +++ b/src/renderer/src/lib/agent-launch-through-host.test.ts @@ -81,14 +81,17 @@ function lastPaneKey(): string { return String(lastParams().paneKey) } -function launch() { +const FOLLOW_UP = { kind: 'review-notes-delivered', version: 1, payload: {} } + +function launch(extra: { followUp?: typeof FOLLOW_UP } = {}) { return launchAgentThroughHost({ agent: 'claude', worktreeId: WT, groupId: store.getState().activeGroupIdByWorktree[WT], prompt: 'fix the failing checks', agentArgs: null, - launchSource: 'source_control_recovery' + launchSource: 'source_control_recovery', + ...extra }) } @@ -211,20 +214,22 @@ describe('a desktop launch through the host', () => { callRuntimeRpc .mockRejectedValueOnce(rpcError('agent_session_operation_capacity')) .mockReturnValueOnce(unrecorded.promise) - const { tabId, outcome } = launch() + const { tabId, outcome } = launch({ followUp: FOLLOW_UP }) const paneKey = lastPaneKey() await vi.waitFor(() => expect(callRuntimeRpc).toHaveBeenCalledTimes(2)) const [, method, params] = callRuntimeRpc.mock.calls[1]! expect(method).toBe('agent.launch') expect(params).not.toHaveProperty('operationId') + // Nothing records it: the click keeps its follow-up and runs it live. + expect(params).not.toHaveProperty('followUp') // The same pane, held until the answer. expect(params.paneKey).toBe(paneKey) const leafId = launchTab(tabId)!.agentLaunchPane!.leafId expect(agentLaunchPaneSpawnHold(tabId, leafId)).not.toBeNull() unrecorded.resolve(terminalResult(paneKey)) - await expect(outcome).resolves.toEqual({ kind: 'started' }) + await expect(outcome).resolves.toEqual({ kind: 'started', unrecorded: true }) }) it('takes its tab back when the unrecorded launch fails before the host revealed it', async () => { diff --git a/src/renderer/src/lib/agent-launch-through-host.ts b/src/renderer/src/lib/agent-launch-through-host.ts index b23066c7959a..0cd2b5afeaac 100644 --- a/src/renderer/src/lib/agent-launch-through-host.ts +++ b/src/renderer/src/lib/agent-launch-through-host.ts @@ -59,8 +59,9 @@ export type HostAgentLaunchArgs = { /** What became of the launch, as this window must tell it. */ export type HostAgentLaunchOutcome = /** The agent was started in this tab's pane, which is attached to it; `prompt` is what the host - * says became of the text it was given. */ - | { kind: 'started'; prompt?: AgentLaunchPromptReceipt } + * says became of the text it was given. `unrecorded`: launched past the record's cap, so the + * host keeps no follow-up for it and the click runs its own. */ + | { kind: 'started'; prompt?: AgentLaunchPromptReceipt; unrecorded?: true } /** The pane shows how the launch ended: couldn't start, or couldn't confirm it started. */ | { kind: 'pane-says' } /** The user closed the tab while it started, and with it the launch: nothing more to say. */ @@ -137,12 +138,14 @@ async function launchWithoutRecord( pane: { tabId: string; leafId: string } ): Promise { try { - return outcomeFromResult( + const { followUp: _unrecorded, ...unrecordedArgs } = args + const outcome = outcomeFromResult( await callRuntimeRpc({ kind: 'local' }, 'agent.launch', { - ...launchParams(args), + ...launchParams(unrecordedArgs), paneKey: makePaneKey(pane.tabId, pane.leafId) }) ) + return outcome.kind === 'started' ? { ...outcome, unrecorded: true } : outcome } catch (error) { const code = error instanceof RuntimeRpcCallError ? error.code : undefined // A pane the host never revealed would open as a shell; one it did holds the agent's terminal. diff --git a/src/renderer/src/lib/launch-agent-new-tab-host-route.test.ts b/src/renderer/src/lib/launch-agent-new-tab-host-route.test.ts index 6f95b9792c71..36d4730e33f5 100644 --- a/src/renderer/src/lib/launch-agent-new-tab-host-route.test.ts +++ b/src/renderer/src/lib/launch-agent-new-tab-host-route.test.ts @@ -252,6 +252,22 @@ describe('a click whose follow-up is recorded on its launch', () => { } }) + it('runs it live, taking nothing, when the launch went past the record’s cap unrecorded', async () => { + deferredOutcome()({ + kind: 'started', + unrecorded: true, + prompt: { delivery: 'submit', outcome: 'handed-to-terminal' } + }) + const onPromptDelivered = vi.fn() + await expect(launchWithFollowUp(onPromptDelivered)).resolves.toEqual({ + delivered: true, + failureNotified: false + }) + expect(onPromptDelivered).toHaveBeenCalledOnce() + expect(followUps.takeLaunchFollowUps).not.toHaveBeenCalled() + expect(followUps.wait).not.toHaveBeenCalled() + }) + it('on a host that does not record follow-ups, runs it live and records nothing, as before', async () => { followUps.hostRecords = false deferredOutcome()({ diff --git a/src/renderer/src/lib/launch-agent-new-tab-host-route.ts b/src/renderer/src/lib/launch-agent-new-tab-host-route.ts index 0d852cdb75f9..03e2b6c5ccc5 100644 --- a/src/renderer/src/lib/launch-agent-new-tab-host-route.ts +++ b/src/renderer/src/lib/launch-agent-new-tab-host-route.ts @@ -172,7 +172,14 @@ export function launchNewTabPromptThroughHost( // Seeded once the host started the agent, as main's paste seeded it: a launch that never // started leaves no chat copy behind. const seeded = seedChatCopy(tabId, args.agent, pasteContent, clickedAt) - return settleHostPrompt(args, { tabId, operationId, followUp }, launched.prompt, seeded) + // A launch past the record's cap kept no follow-up: the click runs its own, as main did. + const recorded = launched.unrecorded ? undefined : followUp + return settleHostPrompt( + args, + { tabId, operationId, followUp: recorded }, + launched.prompt, + seeded + ) } // The pane, or this notice for a tab that went, already says why: never a second notice. showLaunchNotStartedNotice(launched, args.prompt) From aa27e9f8d37757c41e177d8eaf73cb449f8c78cb Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 8 Oct 2026 14:30:04 -0700 Subject: [PATCH 6/8] Wait for host settlement before finishing launch follow-ups --- .../vitest-sqlite-database-runtime-files.mjs | 1 + ...y-foreground-inspection-cheap-tier.test.ts | 26 ++ src/main/providers/local-pty-provider.ts | 6 +- ...t-launch-follow-up-late-settlement.test.ts | 388 ++++++++++++++++++ ...nt-launch-follow-up-window.test-fixture.ts | 90 ++++ .../methods/agent-launch-follow-ups.test.ts | 28 ++ .../rpc/methods/agent-launch-follow-ups.ts | 13 +- src/main/runtime/rpc/methods/agent-launch.ts | 5 +- src/main/window/runtime-window-lifecycle.ts | 2 + .../lib/agent-launch-follow-up-waiter.test.ts | 64 ++- .../src/lib/agent-launch-follow-up-waiter.ts | 19 +- 11 files changed, 625 insertions(+), 17 deletions(-) create mode 100644 src/main/runtime/rpc/methods/agent-launch-follow-up-late-settlement.test.ts create mode 100644 src/main/runtime/rpc/methods/agent-launch-follow-up-window.test-fixture.ts diff --git a/config/scripts/vitest-sqlite-database-runtime-files.mjs b/config/scripts/vitest-sqlite-database-runtime-files.mjs index 0be98d22bf6e..2f5259d1d971 100644 --- a/config/scripts/vitest-sqlite-database-runtime-files.mjs +++ b/config/scripts/vitest-sqlite-database-runtime-files.mjs @@ -123,6 +123,7 @@ export const SQLITE_DATABASE_RUNTIME_INCLUDE = [ 'src/main/runtime/orchestration/worker-start-unobserved-prompt-settlement.test.ts', 'src/main/runtime/rpc/methods/agent-launch-instant-tab.test.ts', 'src/main/runtime/rpc/methods/agent-launch-follow-ups.test.ts', + 'src/main/runtime/rpc/methods/agent-launch-follow-up-late-settlement.test.ts', 'src/main/runtime/rpc/methods/agent-launch-restart-replay.test.ts', 'src/main/runtime/rpc/methods/native-chat-opencode-semantic-pages.test.ts', 'src/main/runtime/rpc/methods/native-chat-opencode-wire-page-budget.test.ts', diff --git a/src/main/providers/local-pty-foreground-inspection-cheap-tier.test.ts b/src/main/providers/local-pty-foreground-inspection-cheap-tier.test.ts index 0d906dbd6610..8347c0ee4681 100644 --- a/src/main/providers/local-pty-foreground-inspection-cheap-tier.test.ts +++ b/src/main/providers/local-pty-foreground-inspection-cheap-tier.test.ts @@ -20,6 +20,8 @@ vi.mock('./agent-foreground-process', () => ({ })) import { getLocalPtyForegroundProcess } from './local-pty-foreground-inspection' +import { LocalPtyProvider } from './local-pty-provider' +import { PROCESS_TABLE_EVIDENCE_BUDGET_MS } from '../../shared/process-table-snapshot-reader' import { ptyLastRecognizedForeground, ptyProcesses, ptyShellPath } from './local-pty-provider-state' const SHELL_PID = 4242 @@ -82,6 +84,7 @@ describe('local POSIX provider cheap-tier revalidation', () => { }) afterEach(() => { + vi.useRealTimers() ptyProcesses.delete(ID) ptyShellPath.delete(ID) ptyLastRecognizedForeground.delete(ID) @@ -135,4 +138,27 @@ describe('local POSIX provider cheap-tier revalidation', () => { expect(await getLocalPtyForegroundProcess(ID)).toBe('claude') expect(resolveMock).toHaveBeenCalledTimes(2) }) + + it('stops a direct provider inspection at the existing evidence budget when its full capture hangs', async () => { + vi.useFakeTimers() + fullSnapshotMock.mockImplementationOnce(() => new Promise(() => {})) + const rejected = vi.fn() + void new LocalPtyProvider().inspectProcess(ID).catch(rejected) + + await vi.advanceTimersByTimeAsync(PROCESS_TABLE_EVIDENCE_BUDGET_MS - 1) + expect(fullSnapshotMock).toHaveBeenCalledOnce() + expect(rejected).not.toHaveBeenCalled() + await vi.advanceTimersByTimeAsync(1) + expect(rejected).toHaveBeenCalledExactlyOnceWith( + expect.objectContaining({ reason: 'capture_over_budget' }) + ) + }) + + it('retains a direct provider’s positive foreground and child evidence when capture completes', async () => { + await expect(new LocalPtyProvider().inspectProcess(ID)).resolves.toEqual({ + foregroundProcess: 'claude', + hasChildProcesses: true, + childProcessEvidence: 'children' + }) + }) }) diff --git a/src/main/providers/local-pty-provider.ts b/src/main/providers/local-pty-provider.ts index 9c2f3c80c9cd..fca6566a7cc9 100644 --- a/src/main/providers/local-pty-provider.ts +++ b/src/main/providers/local-pty-provider.ts @@ -1,4 +1,5 @@ import type * as pty from 'node-pty' +import { withEvidenceBudget } from '../../shared/process-table-snapshot-reader' import type { IPtyProvider, PtyProcessInfo, PtySpawnOptions, PtySpawnResult } from './types' import { WRITE_ACCEPTED, @@ -141,7 +142,10 @@ export class LocalPtyProvider implements IPtyProvider { async inspectProcess(id: string): Promise { const proc = ptyProcesses.get(id) - const foregroundProcess = await getLocalPtyForegroundProcess(id) + const foregroundProcess = + process.platform === 'win32' + ? await getLocalPtyForegroundProcess(id) + : await withEvidenceBudget(getLocalPtyForegroundProcess(id)) const childProcessEvidence = await inspectLocalPtyChildProcesses(id) // Neither asynchronous inspection may publish a replacement pane's identity. if (ptyProcesses.get(id) !== proc) { diff --git a/src/main/runtime/rpc/methods/agent-launch-follow-up-late-settlement.test.ts b/src/main/runtime/rpc/methods/agent-launch-follow-up-late-settlement.test.ts new file mode 100644 index 000000000000..cbdbf9dc500f --- /dev/null +++ b/src/main/runtime/rpc/methods/agent-launch-follow-up-late-settlement.test.ts @@ -0,0 +1,388 @@ +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, expect, it, vi } from 'vitest' +import type { AgentLaunchFollowUpTake } from '../../../../shared/agent-launch-follow-up' +import { + AGENT_SESSION_MAX_NEW_OPERATION_AGE_MS, + AGENT_SESSION_OPERATION_FUTURE_SKEW_MS +} from '../../../../shared/agent-session-host-authority' +import { + AGENT_LAUNCH_RUNTIME_CAPABILITY, + AGENT_LAUNCH_FOLLOW_UPS_RUNTIME_CAPABILITY +} from '../../../../shared/agent-launch-runtime-capability' +import { openTestAgentSessionRecordStore } from '../../agent-session-record-store-test-harness' +import { createAgentPromptSubmissionRuntime } from '../../agent-prompt-submission-runtime-test-fixture' +import { DESKTOP_RPC_CALLER } from '../rpc-caller-identity' +import { + methodNamed, + rpcContext, + runtimeStub, + setAgentLaunchRecordStore +} from './agent-launch.test-fixture' +import { createLaunchFollowUpWindow } from './agent-launch-follow-up-window.test-fixture' + +const windowState = vi.hoisted(() => { + const holds: Promise[] = [] + return { + take: vi.fn<(params: unknown) => Promise>(), + clearNotes: vi.fn(async () => true), + resolveThreads: vi.fn(async () => {}), + holds + } +}) +vi.mock('../../../git/worktree', () => ({ + listWorktrees: async () => [ + { + path: '/tmp/worktree-a', + head: 'abc', + branch: 'feature', + isBare: false, + isMainWorktree: false + } + ], + listWorktreesStrict: async () => [ + { + path: '/tmp/worktree-a', + head: 'abc', + branch: 'feature', + isBare: false, + isMainWorktree: false + } + ] +})) +vi.mock('electron', () => ({ ipcMain: { on: vi.fn(), removeListener: vi.fn() } })) +vi.mock('../../../ipc/worktree-change-invalidators', () => ({ + runWorktreeChangeInvalidators: vi.fn(), + registerWorktreeChangeInvalidator: vi.fn() +})) +vi.mock('../../../window/mobile-markdown-request-relay', () => ({ + requestMobileMarkdownFromRenderer: vi.fn() +})) +vi.mock('../../../window/session-tab-close-request-relay', () => ({ + requestSessionTabCloseFromRenderer: vi.fn() +})) +vi.mock('../../../window/terminal-tab-close-request-relay', () => ({ + requestTerminalTabCloseFromRenderer: vi.fn() +})) +vi.mock('@/store', () => ({ + useAppStore: { getState: () => ({ clearDeliveredDiffComments: windowState.clearNotes }) } +})) +vi.mock('@/runtime/local-runtime-capabilities', () => ({ + ensureLocalRuntimeCapabilities: async () => [AGENT_LAUNCH_FOLLOW_UPS_RUNTIME_CAPABILITY], + readLocalRuntimeCapabilitiesOrUnknown: () => [AGENT_LAUNCH_FOLLOW_UPS_RUNTIME_CAPABILITY] +})) +vi.mock('@/runtime/runtime-rpc-client', () => ({ + callRuntimeRpc: (_target: unknown, _method: string, params: unknown) => windowState.take(params) +})) +vi.mock('@/lib/notes-send-in-flight', () => ({ + diffCommentSendKey: (note: { id: string }) => note.id, + holdNotesForSend: (_keys: unknown, done: Promise) => windowState.holds.push(done) +})) +vi.mock('@/components/right-sidebar/pr-comment-groups-in-flight', () => ({ + holdPRCommentGroupsForSend: (_keys: unknown, done: Promise) => + windowState.holds.push(done) +})) +vi.mock('@/components/right-sidebar/review-comments-resolution-follow-up', () => ({ + runReviewCommentsResolutionFollowUp: windowState.resolveThreads +})) + +const { AGENT_LAUNCH_METHODS } = await import('./agent-launch') +const { AGENT_LAUNCH_FOLLOW_UP_METHODS, announceSettledLaunchFollowUps } = + await import('./agent-launch-follow-ups') +const { runRecordedLaunchFollowUps } = + await import('../../../../renderer/src/lib/agent-launch-follow-up-waiter') +const { reviewNotesDeliveredFollowUp, reviewCommentsResolutionFollowUp } = + await import('../../../../renderer/src/lib/agent-launch-follow-ups') +const LAUNCH = methodNamed(AGENT_LAUNCH_METHODS, 'agent.launchReplay') +const TAKE = methodNamed(AGENT_LAUNCH_FOLLOW_UP_METHODS, 'agent.takeLaunchFollowUps') +const DESKTOP = { + caller: DESKTOP_RPC_CALLER, + clientKind: 'runtime' as const, + clientCapabilities: [AGENT_LAUNCH_RUNTIME_CAPABILITY, AGENT_LAUNCH_FOLLOW_UPS_RUNTIME_CAPABILITY] +} +const PANE_KEY = '9b1deb4d-3b7d-4bad-9bdd-2b0d7b3dcb6d:3f2504e0-4f89-41d3-9a0c-0305e82c3301' +const EXPIRED_HANDOFF_MS = + AGENT_SESSION_MAX_NEW_OPERATION_AGE_MS + AGENT_SESSION_OPERATION_FUTURE_SKEW_MS + 1 + +let directory: string +beforeEach(async () => { + vi.useFakeTimers({ toFake: ['Date'] }) + windowState.take.mockReset() + windowState.clearNotes.mockClear() + windowState.resolveThreads.mockClear() + windowState.holds = [] + directory = await mkdtemp(join(tmpdir(), 'orca-late-launch-follow-up-')) + setAgentLaunchRecordStore(await openTestAgentSessionRecordStore(directory)) +}) +afterEach(async () => { + vi.restoreAllMocks() + vi.useRealTimers() + setAgentLaunchRecordStore(null) + await rm(directory, { recursive: true, force: true }) +}) + +it.each([ + { + kind: 'notes', + cancelNavigation: false, + lookupFails: false, + elapsedMs: 315_001, + rowState: 'live' + }, + { + kind: 'checks', + cancelNavigation: false, + lookupFails: false, + elapsedMs: 315_001, + rowState: 'live' + }, + { + kind: 'notes', + cancelNavigation: true, + lookupFails: false, + elapsedMs: 315_001, + rowState: 'live' + }, + { + kind: 'checks', + cancelNavigation: true, + lookupFails: false, + elapsedMs: 315_001, + rowState: 'live' + }, + { kind: 'notes', cancelNavigation: false, lookupFails: true, elapsedMs: 1_000, rowState: 'live' }, + { + kind: 'checks', + cancelNavigation: false, + lookupFails: true, + elapsedMs: 1_000, + rowState: 'live' + }, + { + kind: 'notes', + cancelNavigation: false, + lookupFails: true, + elapsedMs: 315_001, + rowState: 'live' + }, + { + kind: 'checks', + cancelNavigation: false, + lookupFails: true, + elapsedMs: 315_001, + rowState: 'live' + }, + { + kind: 'notes', + cancelNavigation: false, + lookupFails: false, + elapsedMs: EXPIRED_HANDOFF_MS, + rowState: 'expired' + }, + { + kind: 'checks', + cancelNavigation: false, + lookupFails: false, + elapsedMs: EXPIRED_HANDOFF_MS, + rowState: 'expired' + }, + { + kind: 'notes', + cancelNavigation: false, + lookupFails: false, + elapsedMs: EXPIRED_HANDOFF_MS, + rowState: 'pruned' + }, + { + kind: 'checks', + cancelNavigation: false, + lookupFails: false, + elapsedMs: EXPIRED_HANDOFF_MS, + rowState: 'pruned' + } +] as const)( + 'settles $kind after a guarded handoff at $elapsedMs ms ($rowState row, canceled navigation: $cancelNavigation, failed lookup: $lookupFails)', + async ({ kind, cancelNavigation, lookupFails, elapsedMs, rowState }) => { + const rig = await createAgentPromptSubmissionRuntime(() => {}, 'opencode') + vi.spyOn(rig.runtime, 'waitForFreshWorkerComposer').mockResolvedValue({ + handle: rig.handle, + condition: 'tui-idle', + satisfied: true, + status: 'running', + exitCode: null + }) + let allowForeground: (value: 'agent') => void = () => {} + let started: () => void = () => {} + const guardStarted = new Promise((resolve) => { + started = resolve + }) + const foreground = new Promise<'agent'>((resolve) => { + allowForeground = resolve + }) + vi.mocked(rig.runtime.readLaunchedAgentForeground).mockImplementationOnce(() => { + started() + return foreground + }) + const runtime = Object.assign( + runtimeStub({ settings: {}, terminalPaneKey: PANE_KEY, lineCarriesPrompt: false }), + { + getTerminalPromptRequestBinding: rig.runtime.getTerminalPromptRequestBinding.bind( + rig.runtime + ), + waitForFreshWorkerComposer: rig.runtime.waitForFreshWorkerComposer.bind(rig.runtime), + waitForTerminal: rig.runtime.waitForTerminal.bind(rig.runtime), + waitForAgentLaunchFallback: rig.runtime.waitForAgentLaunchFallback.bind(rig.runtime), + readLaunchedAgentForeground: rig.runtime.readLaunchedAgentForeground.bind(rig.runtime), + subscribeToTerminalData: rig.runtime.subscribeToTerminalData.bind(rig.runtime), + sendTerminalAgentPrompt: rig.runtime.sendTerminalAgentPrompt.bind(rig.runtime) + } + ) + runtime.createTerminal.mockResolvedValue({ handle: rig.handle, paneKey: PANE_KEY }) + const context = rpcContext(runtime, DESKTOP) + const t = createLaunchFollowUpWindow(runtime, context.runtime) + windowState.take.mockImplementation((params) => + TAKE.handler(TAKE.params.parse(params), context) + ) + const note = { id: 'n1', body: 'fix this', filePath: 'a.ts', lineNumber: 3 } + const followUp = + kind === 'notes' + ? reviewNotesDeliveredFollowUp('wt-7', [note]) + : reviewCommentsResolutionFollowUp({ + reviewContextKey: 'pr-1', + provider: 'github', + selectedGroups: [ + { + kind: 'comment', + comment: { + id: 7, + author: 'reviewer', + authorAvatarUrl: '', + body: 'fix this', + createdAt: '2026-01-01T00:00:00Z', + url: 'https://example.test/c/7' + } + } + ] + }) + const operationId = `${Date.now()}-000000000000000000000000000000f${kind === 'notes' ? '1' : '2'}` + const running = LAUNCH.handler( + LAUNCH.params.parse({ + agent: 'opencode', + target: { kind: 'existing', worktree: 'id:wt-7' }, + prompt: { text: 'fix this', delivery: 'submit', transport: 'paste' }, + followUp, + operationId + }), + context + ) + await guardStarted + let finished = false + const windowRun = runRecordedLaunchFollowUps(t.clock).then(() => { + finished = true + }) + try { + await vi.waitFor(() => expect(windowState.holds).toHaveLength(1)) + const probeElapsed = Math.min(elapsedMs, 315_001) + t.advance(probeElapsed) + const probeIndex = elapsedMs > 315_000 ? 1 : 0 + await vi.waitFor(() => expect(windowState.take).toHaveBeenCalledTimes(probeIndex + 1)) + await vi.waitFor(async () => { + const probe = await windowState.take.mock.results[probeIndex]!.value + expect(probe.pending).toMatchObject([{ operationId }]) + }) + await Promise.resolve() + expect(finished).toBe(false) + expect(t.listenerCount).toBe(1) + expect(rig.writes).toEqual([]) + if (rowState !== 'live') { + t.advance(elapsedMs - probeElapsed) + const store = runtime.openedAgentSessionRecordStore() + if (!store) { + throw new Error('missing launch store') + } + const row = store.listOperationRows().find((entry) => entry.operationId === operationId) + expect(row?.expiresAt).toBeLessThan(Date.now()) + if (rowState === 'pruned') { + await store.admitOperation({ + callerKey: 'unrelated', + operationId: `${Date.now()}-000000000000000000000000000000f3`, + fingerprint: 'unrelated-operation', + now: Date.now() + }) + expect(store.listOperationRows().some((entry) => entry.operationId === operationId)).toBe( + false + ) + } + } + if (cancelNavigation) { + t.webContents.emit('did-start-navigation', {}, 'https://example.test/reload', false, true) + } + if (lookupFails) { + windowState.take.mockRejectedValueOnce(new Error('runtime_unavailable')) + } + allowForeground('agent') + const result = await running + expect(result.prompt?.outcome).toBe('handed-to-terminal') + if (cancelNavigation) { + expect(t.send).not.toHaveBeenCalled() + expect(finished).toBe(false) + t.webContents.emit('did-stop-loading') + expect(t.send).toHaveBeenCalledExactlyOnceWith('ui:agentLaunchPromptSettled', { + operationId + }) + announceSettledLaunchFollowUps(runtime) + announceSettledLaunchFollowUps(runtime) + } + await vi.waitFor(() => expect(finished).toBe(true)) + await windowRun + expect(rig.writes).toEqual(['\x1b[200~fix this\x1b[201~', '\r']) + expect(runtime.reportAgentLaunchPromptSettled).toHaveBeenCalledWith(operationId) + if (rowState !== 'live') { + expect(windowState.clearNotes).not.toHaveBeenCalled() + expect(windowState.resolveThreads).not.toHaveBeenCalled() + await expect(windowState.holds[0]).resolves.toBeUndefined() + expect(t.listenerCount).toBe(0) + announceSettledLaunchFollowUps(runtime) + await runRecordedLaunchFollowUps(t.clock) + await expect(TAKE.handler(TAKE.params.parse({ operationId }), context)).resolves.toEqual({ + taken: [], + pending: [] + }) + expect(windowState.clearNotes).not.toHaveBeenCalled() + expect(windowState.resolveThreads).not.toHaveBeenCalled() + return + } + if (lookupFails) { + expect(windowState.clearNotes).not.toHaveBeenCalled() + expect(windowState.resolveThreads).not.toHaveBeenCalled() + await expect(windowState.holds[0]).resolves.toBeUndefined() + expect(t.listenerCount).toBe(0) + expect(runtime.openedAgentSessionRecordStore()?.listOperationRows()).toEqual( + expect.arrayContaining([ + expect.objectContaining({ operationId, launchFollowUp: followUp }) + ]) + ) + await runRecordedLaunchFollowUps(t.clock) + } + expect( + kind === 'notes' ? windowState.clearNotes : windowState.resolveThreads + ).toHaveBeenCalledOnce() + announceSettledLaunchFollowUps(runtime) + announceSettledLaunchFollowUps(runtime) + await Promise.resolve() + expect( + kind === 'notes' ? windowState.clearNotes : windowState.resolveThreads + ).toHaveBeenCalledOnce() + await expect(windowState.holds[0]).resolves.toBeUndefined() + expect(t.listenerCount).toBe(0) + await expect(TAKE.handler(TAKE.params.parse({ operationId }), context)).resolves.toEqual({ + taken: [], + pending: [] + }) + } finally { + allowForeground('agent') + await running + } + } +) diff --git a/src/main/runtime/rpc/methods/agent-launch-follow-up-window.test-fixture.ts b/src/main/runtime/rpc/methods/agent-launch-follow-up-window.test-fixture.ts new file mode 100644 index 000000000000..596d3da00301 --- /dev/null +++ b/src/main/runtime/rpc/methods/agent-launch-follow-up-window.test-fixture.ts @@ -0,0 +1,90 @@ +import { EventEmitter } from 'node:events' +import type { BrowserWindow } from 'electron' +import { vi } from 'vitest' +import type { OrcaRuntimeService } from '../../orca-runtime' +import type { RuntimeNotifier } from '../../runtime-notifier-contract' +import { registerRuntimeWindowLifecycle } from '../../../window/runtime-window-lifecycle' +import type { AgentLaunchRuntimeStub } from './agent-launch.test-fixture' + +export function createLaunchFollowUpWindow( + runtime: AgentLaunchRuntimeStub, + contextRuntime: OrcaRuntimeService +) { + let now = Date.now() + const timers = new Set<{ at: number; run: () => void }>() + const listeners = new Set<(operationId: string) => void>() + const attached: { notifier: RuntimeNotifier | null } = { notifier: null } + const settle = (operationId: string) => listeners.forEach((listener) => listener(operationId)) + Object.assign(runtime, { + attachWindow: vi.fn(), + markGraphReloadFailed: vi.fn(), + markRendererReloading: vi.fn(() => ({})), + markRendererReloadCancelled: vi.fn(() => true), + setNotifier: (notifier: RuntimeNotifier | null) => { + attached.notifier = notifier + } + }) + const send = vi.fn((channel: string, event: unknown) => { + if ( + channel === 'ui:agentLaunchPromptSettled' && + event && + typeof event === 'object' && + 'operationId' in event && + typeof event.operationId === 'string' + ) { + settle(event.operationId) + } + }) + const webContents = Object.assign(new EventEmitter(), { + isDestroyed: () => false, + isLoadingMainFrame: () => false, + getURL: () => 'https://example.test/', + send + }) + const mainWindow = Object.assign(new EventEmitter(), { + id: 1, + isDestroyed: () => false, + webContents + }) + registerRuntimeWindowLifecycle( + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: registration only uses the event and notification members implemented above. + mainWindow as unknown as BrowserWindow, + contextRuntime + ) + runtime.reportAgentLaunchPromptSettled.mockImplementation((operationId) => { + attached.notifier?.agentLaunchPromptSettled?.({ operationId }) + }) + return { + webContents, + send, + clock: { + now: () => now, + schedule: (ms: number, run: () => void) => { + const timer = { at: now + ms, run } + timers.add(timer) + return () => { + timers.delete(timer) + } + }, + onSettled: (listener: (operationId: string) => void) => { + listeners.add(listener) + return () => { + listeners.delete(listener) + } + } + }, + advance(ms: number) { + now += ms + vi.setSystemTime(now) + for (const timer of timers) { + if (timer.at <= now) { + timers.delete(timer) + timer.run() + } + } + }, + get listenerCount() { + return listeners.size + } + } +} diff --git a/src/main/runtime/rpc/methods/agent-launch-follow-ups.test.ts b/src/main/runtime/rpc/methods/agent-launch-follow-ups.test.ts index bd1a04cf5f0e..8ed99a2021b7 100644 --- a/src/main/runtime/rpc/methods/agent-launch-follow-ups.test.ts +++ b/src/main/runtime/rpc/methods/agent-launch-follow-ups.test.ts @@ -251,6 +251,34 @@ describe('a click’s follow-up on its launch’s record', () => { await expect(take(runtime, DESKTOP)).resolves.toEqual({ taken: [], pending: [] }) }) + it('does not announce another caller’s completed launch with the same operation id as a pending desktop launch', async () => { + const runtime = host() + let complete: (value: boolean) => void = () => {} + let started: () => void = () => {} + const promptStarted = new Promise((resolve) => { + started = resolve + }) + deliverTerminalPrompt.mockImplementationOnce(() => { + started() + return new Promise((resolve) => { + complete = resolve + }) + }) + const running = launch(runtime) + await promptStarted + try { + expect((await take(runtime, DESKTOP)).pending).toMatchObject([{ operationId: OPERATION_ID }]) + await launch(runtime, LAUNCH, PHONE) + expect(runtime.reportAgentLaunchPromptSettled).not.toHaveBeenCalled() + } finally { + complete(true) + await running + } + expect(runtime.reportAgentLaunchPromptSettled).toHaveBeenCalledExactlyOnceWith(OPERATION_ID) + expect((await take(runtime, DESKTOP)).taken).toHaveLength(1) + await expect(take(runtime, DESKTOP)).resolves.toEqual({ taken: [], pending: [] }) + }) + it('is not recorded over the size cap, and the launch still runs', async () => { const runtime = host() const huge = { ...FOLLOW_UP, payload: { blob: 'x'.repeat(300 * 1024) } } diff --git a/src/main/runtime/rpc/methods/agent-launch-follow-ups.ts b/src/main/runtime/rpc/methods/agent-launch-follow-ups.ts index cf76b4b30343..efab9f683e2a 100644 --- a/src/main/runtime/rpc/methods/agent-launch-follow-ups.ts +++ b/src/main/runtime/rpc/methods/agent-launch-follow-ups.ts @@ -25,21 +25,28 @@ import { agentLaunchOperationCallerKey } from './agent-launch-replay' */ export function announceSettledLaunchFollowUps( runtime: Pick & - Partial> + Partial>, + completed?: { callerKey: string; operationId: string } ): void { try { + const desktop = rpcCallerOperationKey(DESKTOP_RPC_CALLER) + const completedId = completed?.callerKey === desktop ? completed.operationId : undefined + if (completedId) { + runtime.reportAgentLaunchPromptSettled?.(completedId) + } const store = runtime.openedAgentSessionRecordStore() if (!store) { return } - const desktop = rpcCallerOperationKey(DESKTOP_RPC_CALLER) for (const operationId of listSettledLaunchFollowUps( store.listOperationRows(), desktop, Date.now(), (operationKey) => activeAgentLaunchesFor(runtime).has(operationKey) )) { - runtime.reportAgentLaunchPromptSettled?.(operationId) + if (operationId !== completedId) { + runtime.reportAgentLaunchPromptSettled?.(operationId) + } } } catch (error) { // Bookkeeping: a missed word leaves the follow-up for the window's next look. diff --git a/src/main/runtime/rpc/methods/agent-launch.ts b/src/main/runtime/rpc/methods/agent-launch.ts index 98827ab78798..3296015f9c71 100644 --- a/src/main/runtime/rpc/methods/agent-launch.ts +++ b/src/main/runtime/rpc/methods/agent-launch.ts @@ -278,7 +278,10 @@ function runReplaySafeAgentLaunch( // Every way a launch ends, once it no longer reads as running: a window that reloaded mid-launch // holds its follow-up's notes and threads until it hears. if (params.followUp) { - announceSettledLaunchFollowUps(context.runtime) + announceSettledLaunchFollowUps(context.runtime, { + callerKey, + operationId: params.operationId + }) } }) activeAgentLaunches.set(key, { fingerprint, promise }) diff --git a/src/main/window/runtime-window-lifecycle.ts b/src/main/window/runtime-window-lifecycle.ts index 8566a104966a..b731e12c5749 100644 --- a/src/main/window/runtime-window-lifecycle.ts +++ b/src/main/window/runtime-window-lifecycle.ts @@ -13,6 +13,7 @@ import type { TerminalTabCreateReply } from '../../shared/terminal-reveal-identi import { runWorktreeChangeInvalidators } from '../ipc/worktree-change-invalidators' import { requestAgentLaunchTabPublishFromRenderer } from './agent-launch-tab-publish-relay' import type { OrcaRuntimeService } from '../runtime/orca-runtime' +import { announceSettledLaunchFollowUps } from '../runtime/rpc/methods/agent-launch-follow-ups' import { requestMobileMarkdownFromRenderer } from './mobile-markdown-request-relay' import { registerRendererDocumentNavigation } from './renderer-document-navigation' import { createRuntimeRendererNotificationSender } from './runtime-renderer-notification-sender' @@ -230,6 +231,7 @@ export function registerRuntimeWindowLifecycle( return () => { if (fence && runtime.markRendererReloadCancelled(mainWindow.id, fence)) { rendererNotifications.onMainFrameReloadCancelled() + announceSettledLaunchFollowUps(runtime) } } }) diff --git a/src/renderer/src/lib/agent-launch-follow-up-waiter.test.ts b/src/renderer/src/lib/agent-launch-follow-up-waiter.test.ts index abcbe4510e55..b3fa7075957c 100644 --- a/src/renderer/src/lib/agent-launch-follow-up-waiter.test.ts +++ b/src/renderer/src/lib/agent-launch-follow-up-waiter.test.ts @@ -216,20 +216,39 @@ describe('a window that reloaded mid-launch runs what its launches recorded', () expect(t.listeners).toBe(1) }) - it('takes once more just past the host’s deadline, then lets go of a launch still pending', async () => { + it('takes once more past an older host’s deadline and stays subscribed while the host reports pending', async () => { const stillPending = { taken: [], pending: [{ operationId: 'op-1', followUp: NOTES, deadline: 60_000 }] } - state.answers.push(stillPending, stillPending) + state.answers.push(stillPending, stillPending, { taken: [taken('op-1', NOTES)], pending: [] }) const t = manualClock() const run = runRecordedLaunchFollowUps(t.clock) await vi.waitFor(() => expect(t.timers).toBe(1)) t.advance(60_000 + 14_999) expect(state.calls).toHaveLength(1) t.advance(1) + await vi.waitFor(() => expect(state.calls).toHaveLength(2)) + expect(t.listeners).toBe(1) + expect(state.clearDeliveredDiffComments).not.toHaveBeenCalled() + t.settle('op-1') + await run + expect(state.calls).toHaveLength(3) + expect(state.clearDeliveredDiffComments).toHaveBeenCalledOnce() + await expect(state.held[0]!.settled).resolves.toBeUndefined() + expect(t.listeners).toBe(0) + }) + + it('releases the hold at the existing last probe when the host cannot answer', async () => { + state.answers.push( + { taken: [], pending: [{ operationId: 'op-1', followUp: NOTES, deadline: 60_000 }] }, + new Error('runtime_unavailable') + ) + const t = manualClock() + const run = runRecordedLaunchFollowUps(t.clock) + await vi.waitFor(() => expect(t.timers).toBe(1)) + t.advance(75_000) await run - expect(state.calls).toHaveLength(2) expect(state.clearDeliveredDiffComments).not.toHaveBeenCalled() await expect(state.held[0]!.settled).resolves.toBeUndefined() expect(t.listeners).toBe(0) @@ -262,6 +281,28 @@ describe('a window that reloaded mid-launch runs what its launches recorded', () }) describe('waiting on a launch’s follow-up, one take at a time', () => { + it('releases a failed lookup for a settlement heard during startup, without applying the follow-up', async () => { + let answerFirst: (take: AgentLaunchFollowUpTake) => void = () => {} + state.answers.push( + new Promise((resolve) => (answerFirst = resolve)), + new Error('runtime_unavailable') + ) + const t = manualClock() + let finished = false + const run = runRecordedLaunchFollowUps(t.clock).then(() => { + finished = true + }) + await vi.waitFor(() => expect(t.listeners).toBe(1)) + t.settle('op-1') + answerFirst({ taken: [], pending: [{ operationId: 'op-1', followUp: NOTES }] }) + await vi.waitFor(() => expect(finished).toBe(true)) + await run + expect(state.clearDeliveredDiffComments).not.toHaveBeenCalled() + await expect(state.held[0]!.settled).resolves.toBeUndefined() + expect(t.listeners).toBe(0) + expect(t.timers).toBe(0) + }) + it('keeps the host’s word that arrives while startup’s first take is still asking', async () => { let answerFirst: (take: AgentLaunchFollowUpTake) => void = () => {} state.answers.push(new Promise((resolve) => (answerFirst = resolve)), { @@ -334,4 +375,21 @@ describe('waiting on a launch’s follow-up, one take at a time', () => { expect(state.runResolution).toHaveBeenCalledOnce() expect(t.timers).toBe(0) }) + + it('keeps a failed click retake held until its existing final probe', async () => { + state.answers.push(new Error('runtime_unavailable'), { + taken: [taken('op-1', RESOLUTION)], + pending: [] + }) + const t = manualClock() + const waited = waitForRecordedLaunchFollowUp('op-1', RESOLUTION, undefined, t.clock) + t.advance(3_000) + await vi.waitFor(() => expect(state.calls).toHaveLength(1)) + expect(t.listeners).toBe(1) + expect(state.runResolution).not.toHaveBeenCalled() + t.advance(312_000) + await waited + expect(state.runResolution).toHaveBeenCalledOnce() + expect(t.listeners).toBe(0) + }) }) diff --git a/src/renderer/src/lib/agent-launch-follow-up-waiter.ts b/src/renderer/src/lib/agent-launch-follow-up-waiter.ts index 9e4733d2b665..c0026866071e 100644 --- a/src/renderer/src/lib/agent-launch-follow-up-waiter.ts +++ b/src/renderer/src/lib/agent-launch-follow-up-waiter.ts @@ -2,7 +2,8 @@ * The follow-ups this window waits on (`agent-launch-follow-ups`): ones its launches recorded that * the host has not settled yet, found at startup after a reload, or left behind by a click whose own * take missed. Each one's notes or threads stay unsendable while it waits. It is taken when the host - * says its prompt settled, or once more just past the host's own deadline, then let go. + * says its prompt settled, or once more just past an older host's deadline. A host that still + * reports the launch pending keeps the wait subscribed until its settlement. * * One take per launch runs at a time: a second answer that finds it already gone must not release * the hold while the first take is still running it. @@ -17,9 +18,9 @@ import { takeLaunchFollowUps } from './agent-launch-follow-ups' -/** How long past the host's own deadline a waiting window takes once more, then lets go. */ +/** How long past an older host's deadline a waiting window takes once more. */ const PAST_DEADLINE_GRACE_MS = 15_000 -/** Bounds this window's hold if a live launch never announces settlement. */ +/** When to ask once more if the host supplied no deadline. */ const FALLBACK_WAIT_MS = 5 * 60_000 /** A click's own take that missed is asked again once this soon: the host's word came before it. */ const CLICK_RETAKE_MS = 3_000 @@ -96,7 +97,7 @@ class LaunchFollowUpWaiter { }) this.listen() if (this.heard?.has(operationId)) { - void this.take(operationId, false) + void this.take(operationId, true) } return done } @@ -104,7 +105,7 @@ class LaunchFollowUpWaiter { private listen(): void { this.unsubscribe ??= this.clock.onSettled((operationId) => { if (this.waiting.has(operationId)) { - void this.take(operationId, false) + void this.take(operationId, true) } else { this.heard?.add(operationId) } @@ -118,9 +119,9 @@ class LaunchFollowUpWaiter { } } - private take(operationId: string, last: boolean): Promise { + private take(operationId: string, finishIfUnavailable: boolean): Promise { const next = (this.takes.get(operationId) ?? Promise.resolve()).then(() => - this.takeNow(operationId, last) + this.takeNow(operationId, finishIfUnavailable) ) this.takes.set(operationId, next) void next.finally(() => { @@ -131,7 +132,7 @@ class LaunchFollowUpWaiter { return next } - private async takeNow(operationId: string, last: boolean): Promise { + private async takeNow(operationId: string, finishIfUnavailable: boolean): Promise { if (!this.waiting.has(operationId)) { return } @@ -139,7 +140,7 @@ class LaunchFollowUpWaiter { if (take && !take.pending.some((entry) => entry.operationId === operationId)) { await runTakenLaunchFollowUps(take.taken) this.finish(operationId) - } else if (last) { + } else if (finishIfUnavailable && !take) { this.finish(operationId) } } From abb28899162c4358bd230eac88345556cfc62c4d Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 8 Oct 2026 14:52:12 -0700 Subject: [PATCH 7/8] Type launch ownership and keep integration imports within test projects --- .../methods/agent-launch-active-operations.ts | 7 ++- ...t-launch-follow-up-late-settlement.test.ts | 59 +++++++++++++++++-- 2 files changed, 58 insertions(+), 8 deletions(-) diff --git a/src/main/runtime/rpc/methods/agent-launch-active-operations.ts b/src/main/runtime/rpc/methods/agent-launch-active-operations.ts index d86d912681cb..38004869d12c 100644 --- a/src/main/runtime/rpc/methods/agent-launch-active-operations.ts +++ b/src/main/runtime/rpc/methods/agent-launch-active-operations.ts @@ -4,15 +4,18 @@ */ import type { AgentLaunchResult } from '../../../../shared/agent-launch-intent' +import type { OrcaRuntimeService } from '../../orca-runtime' + +type AgentLaunchOwner = Pick type ActiveAgentLaunch = { fingerprint: string promise: Promise } -const activeAgentLaunchesByRuntime = new WeakMap>() +const activeAgentLaunchesByRuntime = new WeakMap>() -export function activeAgentLaunchesFor(runtime: object): Map { +export function activeAgentLaunchesFor(runtime: AgentLaunchOwner): Map { const existing = activeAgentLaunchesByRuntime.get(runtime) if (existing) { return existing diff --git a/src/main/runtime/rpc/methods/agent-launch-follow-up-late-settlement.test.ts b/src/main/runtime/rpc/methods/agent-launch-follow-up-late-settlement.test.ts index cbdbf9dc500f..f5afe209dd35 100644 --- a/src/main/runtime/rpc/methods/agent-launch-follow-up-late-settlement.test.ts +++ b/src/main/runtime/rpc/methods/agent-launch-follow-up-late-settlement.test.ts @@ -2,7 +2,12 @@ import { mkdtemp, rm } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, beforeEach, expect, it, vi } from 'vitest' -import type { AgentLaunchFollowUpTake } from '../../../../shared/agent-launch-follow-up' +import type { HostedReviewInfo } from '../../../../shared/hosted-review' +import type { PRCommentGroup } from '../../../../shared/pr-comment-groups' +import type { + AgentLaunchFollowUp, + AgentLaunchFollowUpTake +} from '../../../../shared/agent-launch-follow-up' import { AGENT_SESSION_MAX_NEW_OPERATION_AGE_MS, AGENT_SESSION_OPERATION_FUTURE_SKEW_MS @@ -90,10 +95,52 @@ vi.mock('@/components/right-sidebar/review-comments-resolution-follow-up', () => const { AGENT_LAUNCH_METHODS } = await import('./agent-launch') const { AGENT_LAUNCH_FOLLOW_UP_METHODS, announceSettledLaunchFollowUps } = await import('./agent-launch-follow-ups') -const { runRecordedLaunchFollowUps } = - await import('../../../../renderer/src/lib/agent-launch-follow-up-waiter') -const { reviewNotesDeliveredFollowUp, reviewCommentsResolutionFollowUp } = - await import('../../../../renderer/src/lib/agent-launch-follow-ups') +type RendererFollowUpWaiter = { + runRecordedLaunchFollowUps: ( + clock: ReturnType['clock'] + ) => Promise +} +type RendererFollowUpBuilders = { + reviewNotesDeliveredFollowUp: ( + worktreeId: string, + notes: readonly { id: string; body: string; filePath: string; lineNumber: number }[] + ) => AgentLaunchFollowUp + reviewCommentsResolutionFollowUp: (resolution: { + reviewContextKey: string + provider: HostedReviewInfo['provider'] + selectedGroups: PRCommentGroup[] + }) => AgentLaunchFollowUp +} +function isRendererFollowUpWaiter(value: unknown): value is RendererFollowUpWaiter { + return ( + typeof value === 'object' && + value !== null && + 'runRecordedLaunchFollowUps' in value && + typeof value.runRecordedLaunchFollowUps === 'function' + ) +} +function isRendererFollowUpBuilders(value: unknown): value is RendererFollowUpBuilders { + return ( + typeof value === 'object' && + value !== null && + 'reviewNotesDeliveredFollowUp' in value && + typeof value.reviewNotesDeliveredFollowUp === 'function' && + 'reviewCommentsResolutionFollowUp' in value && + typeof value.reviewCommentsResolutionFollowUp === 'function' + ) +} +// Load the real renderer through Vitest without pulling it into the node compiler project. +const waiterModule = await vi.importActual( + '../../../../renderer/src/lib/agent-launch-follow-up-waiter' +) +const buildersModule = await vi.importActual( + '../../../../renderer/src/lib/agent-launch-follow-ups' +) +if (!isRendererFollowUpWaiter(waiterModule) || !isRendererFollowUpBuilders(buildersModule)) { + throw new Error('missing renderer launch follow-up exports') +} +const { runRecordedLaunchFollowUps } = waiterModule +const { reviewNotesDeliveredFollowUp, reviewCommentsResolutionFollowUp } = buildersModule const LAUNCH = methodNamed(AGENT_LAUNCH_METHODS, 'agent.launchReplay') const TAKE = methodNamed(AGENT_LAUNCH_FOLLOW_UP_METHODS, 'agent.takeLaunchFollowUps') const DESKTOP = { @@ -253,7 +300,7 @@ it.each([ provider: 'github', selectedGroups: [ { - kind: 'comment', + kind: 'standalone', comment: { id: 7, author: 'reviewer', From c6dbbe4bf19453b13e7f4865659d2514cd4314a8 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 8 Oct 2026 16:42:54 -0700 Subject: [PATCH 8/8] fix(agent-launch): declare the workspace permission on the follow-up take method --- src/main/runtime/rpc/methods/agent-launch-follow-ups.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/runtime/rpc/methods/agent-launch-follow-ups.ts b/src/main/runtime/rpc/methods/agent-launch-follow-ups.ts index efab9f683e2a..fb65a5e3236d 100644 --- a/src/main/runtime/rpc/methods/agent-launch-follow-ups.ts +++ b/src/main/runtime/rpc/methods/agent-launch-follow-ups.ts @@ -57,6 +57,7 @@ export function announceSettledLaunchFollowUps( export const AGENT_LAUNCH_FOLLOW_UP_METHODS = [ defineMethod({ name: 'agent.takeLaunchFollowUps', + permission: 'workspace', params: AgentTakeLaunchFollowUps, handler: async (params, context): Promise => { const callerKey = agentLaunchOperationCallerKey(context)