diff --git a/config/scripts/vitest-sqlite-database-runtime-files.mjs b/config/scripts/vitest-sqlite-database-runtime-files.mjs index a2bbfd4d0c94..a6540aa40304 100644 --- a/config/scripts/vitest-sqlite-database-runtime-files.mjs +++ b/config/scripts/vitest-sqlite-database-runtime-files.mjs @@ -25,6 +25,7 @@ export const SQLITE_DATABASE_RUNTIME_INCLUDE = [ 'src/main/opencode-usage/scanner.test.ts', 'src/main/rate-limits/opencode-go-api-key-source.test.ts', 'src/main/rate-limits/opencode-go-managed-account-source.test.ts', + 'src/main/runtime/agent-launch-persisted-obligations.test.ts', 'src/main/runtime/agent-session-record-store-slot.test.ts', 'src/main/runtime/claude-structured-exit-mid-response.test.ts', 'src/main/runtime/claude-structured-failed-start-resume.test.ts', @@ -121,6 +122,8 @@ 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-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/native-chat/agent-session-journal/journal-database.ts b/src/main/native-chat/agent-session-journal/journal-database.ts index 0dee9537b3f1..f65285c2b0c6 100644 --- a/src/main/native-chat/agent-session-journal/journal-database.ts +++ b/src/main/native-chat/agent-session-journal/journal-database.ts @@ -52,6 +52,36 @@ export function journalDatabaseHoldsAgentSessions(dbPath: string): boolean { } } +/** A launch-record field whose presence means the host still owes that launch work. */ +export type LaunchOperationObligation = '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. */ +export function journalDatabaseHoldsLaunchObligation( + dbPath: string, + field: LaunchOperationObligation, + now: number +): boolean { + const db = new Database(dbPath, { readonly: true, fileMustExist: true }) + try { + const table = db + .prepare( + "SELECT 1 FROM sqlite_master WHERE type = 'table' AND name = 'agent_session_operations'" + ) + .get() + return ( + table !== undefined && + db + .prepare( + `SELECT 1 FROM agent_session_operations WHERE json_extract(row_json, '$.${field}') IS NOT NULL AND json_extract(row_json, '$.expiresAt') > ? LIMIT 1` + ) + .get(now) !== undefined + ) + } finally { + db.close() + } +} + export function openJournalDatabase(dbPath: string): OpenJournalDatabase { const probe = new Database(dbPath) let stored: number 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..20bba67abd4a --- /dev/null +++ b/src/main/runtime/agent-launch-follow-up-record.test.ts @@ -0,0 +1,194 @@ +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 { + listSettledLaunchFollowUps, + 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' | 'unconfirmed', + 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 provisional prompt as pending only while its original launch runs', () => { + const state = rows( + row('op-1', { + outcome: { status: 'succeeded', sessionId: '', launch: launch('unconfirmed') } + }) + ) + const key = agentSessionOperationKey(DESKTOP, 'op-1') + expect(take(state, { running: [key] })).toEqual({ + taken: [], + pending: [{ operationId: 'op-1', followUp: FOLLOW_UP }] + }) + expect(state.operations.get(key)?.launchFollowUp).toEqual(FOLLOW_UP) + expect(listSettledLaunchFollowUps(state.operations.values(), DESKTOP, 100, () => true)).toEqual( + [] + ) + expect(take(state).taken[0]).toMatchObject({ promptHandedOver: false }) + }) + + it('takes a final handoff even while its operation promise is finishing', () => { + const state = rows(row('op-1')) + const key = agentSessionOperationKey(DESKTOP, 'op-1') + expect(listSettledLaunchFollowUps(state.operations.values(), DESKTOP, 100, () => true)).toEqual( + ['op-1'] + ) + expect(take(state, { running: [key] }).taken[0]).toMatchObject({ promptHandedOver: true }) + }) + + 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.each(['owed', 'writing', 'paused'])( + 'ignores an old %s prompt field and consumes an interrupted follow-up without running it', + (state) => { + const saved = Object.assign( + row('op-1', { + outcome: { status: 'succeeded', sessionId: '', launch: launch('unconfirmed') } + }), + { promptDelivery: { state, text: 'old text' } } + ) + const operations = rows(saved) + expect(take(operations).taken[0]).toMatchObject({ promptHandedOver: false }) + expect(take(operations)).toEqual({ taken: [], pending: [] }) + } + ) + + it('consumes a bare terminal creation without authorizing the follow-up', () => { + const result = launch('handed-to-terminal') + delete result.prompt + const state = rows( + row('op-1', { outcome: { status: 'succeeded', sessionId: '', launch: result } }) + ) + expect(take(state).taken[0]).toMatchObject({ promptHandedOver: false }) + }) + + 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..04455130b489 --- /dev/null +++ b/src/main/runtime/agent-launch-follow-up-record.ts @@ -0,0 +1,114 @@ +/** + * 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' + +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 { + const { outcome } = row + if (outcome.status === 'succeeded') { + const prompt = isAgentLaunchResult(outcome.launch) ? outcome.launch.prompt : undefined + if ( + prompt?.outcome === 'unconfirmed' && + isLaunchRunning(agentSessionOperationKey(row.callerKey, row.operationId)) + ) { + return null + } + 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) { + take.pending.push({ + operationId: row.operationId, + followUp + }) + 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, + isLaunchRunning: (operationKey: string) => boolean +): string[] { + const settled: string[] = [] + for (const row of rows) { + if ( + row.callerKey === callerKey && + row.expiresAt > now && + isAgentLaunchFollowUp(row.launchFollowUp) && + settledPrompt(row, isLaunchRunning) !== null + ) { + settled.push(row.operationId) + } + } + return settled +} diff --git a/src/main/runtime/agent-launch-persisted-obligations.test.ts b/src/main/runtime/agent-launch-persisted-obligations.test.ts new file mode 100644 index 000000000000..1e6434f73488 --- /dev/null +++ b/src/main/runtime/agent-launch-persisted-obligations.test.ts @@ -0,0 +1,94 @@ +import { existsSync } from 'node:fs' +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { journalDatabasePath } from '../native-chat/agent-session-journal/journal-host-database' +import { closeTestJournalHostDatabase } from '../native-chat/agent-session-journal/journal-host-database-test-support' +import { hasPersistedLaunchObligation } from './agent-launch-persisted-obligations' +import { + agentSessionOperationKey, + type AgentSessionOperationRow +} from '../../shared/agent-session-operation-ledger' +import { + editPersistedTestAgentSessionStore, + openTestAgentSessionRecordStore +} from './agent-session-record-store-test-harness' +import { hasPersistedStructuredAgentSessionStore } from './structured-agent-session-runtime' + +const REF = { callerKey: 'trusted-local:desktop', operationId: 'op-1' } + +let directory: string + +beforeEach(async () => { + directory = await mkdtemp(join(tmpdir(), 'orca-launch-obligations-')) +}) +afterEach(async () => { + closeTestJournalHostDatabase(directory) + await rm(directory, { recursive: true, force: true }) +}) + +function launchRow(launchFollowUp?: AgentSessionOperationRow['launchFollowUp']) { + const row: AgentSessionOperationRow = { + ...REF, + fingerprint: 'fp', + operationTimestamp: 1, + recordedAt: 1, + expiresAt: Number.MAX_SAFE_INTEGER, + outcome: { status: 'succeeded', sessionId: '', launch: { stub: true } }, + ...(launchFollowUp ? { launchFollowUp } : {}) + } + return row +} + +/** Leaves launch records behind, and no chat, as a profile that only launched terminal agents. */ +async function leaveLaunchRecords(row: AgentSessionOperationRow): Promise { + await openTestAgentSessionRecordStore(directory) + await editPersistedTestAgentSessionStore(directory, (persisted) => { + persisted.operations = { [agentSessionOperationKey(REF.callerKey, REF.operationId)]: row } + }) + closeTestJournalHostDatabase(directory) +} + +const FOLLOW_UP = { kind: 'review-notes-delivered', version: 1, payload: {} } + +describe('whether launch records have a follow-up left for the window', () => { + it('finds a follow-up in a profile that only ever launched terminal agents', async () => { + await leaveLaunchRecords(launchRow(FOLLOW_UP)) + expect(hasPersistedLaunchObligation(directory, 'launchFollowUp')).toBe(true) + expect(hasPersistedStructuredAgentSessionStore(directory)).toBe(false) + }) + + it('finds nothing once the window took every follow-up', async () => { + await leaveLaunchRecords(launchRow()) + expect(hasPersistedLaunchObligation(directory, 'launchFollowUp')).toBe(false) + }) + + it('skips a follow-up whose row has expired', async () => { + const row = launchRow(FOLLOW_UP) + row.expiresAt = 50_000 + await leaveLaunchRecords(row) + expect(hasPersistedLaunchObligation(directory, 'launchFollowUp', 49_999)).toBe(true) + expect(hasPersistedLaunchObligation(directory, 'launchFollowUp', 50_000)).toBe(false) + }) + + it('ignores an old owed prompt without a follow-up', async () => { + await leaveLaunchRecords( + Object.assign(launchRow(), { + promptDelivery: { + state: 'owed', + text: 'fix it', + agent: 'claude', + deadline: 9_000, + terminal: null + } + }) + ) + expect(hasPersistedLaunchObligation(directory, 'launchFollowUp')).toBe(false) + }) + + it('never creates the database of a profile that has none', () => { + expect(hasPersistedLaunchObligation(directory, 'launchFollowUp')).toBe(false) + expect(existsSync(journalDatabasePath(directory))).toBe(false) + }) +}) diff --git a/src/main/runtime/agent-launch-persisted-obligations.ts b/src/main/runtime/agent-launch-persisted-obligations.ts new file mode 100644 index 000000000000..ef1c91a76be2 --- /dev/null +++ b/src/main/runtime/agent-launch-persisted-obligations.ts @@ -0,0 +1,29 @@ +import { existsSync } from 'node:fs' +import { + journalDatabaseHoldsLaunchObligation, + type LaunchOperationObligation +} from '../native-chat/agent-session-journal/journal-database' +import { journalDatabasePath } from '../native-chat/agent-session-journal/journal-host-database' + +/** + * Whether this profile's launch records still owe work of this kind, read without opening the + * record store: a profile with no database, or none owed, neither opens nor creates one at startup. + * A terminal-only profile has launch records and no chat, so the chat-store check cannot answer. + */ +export function hasPersistedLaunchObligation( + stateDirectory: string, + field: LaunchOperationObligation, + now: number = Date.now(), + fileExists: (path: string) => boolean = existsSync +): boolean { + const databasePath = journalDatabasePath(stateDirectory) + if (!fileExists(databasePath)) { + return false + } + try { + return journalDatabaseHoldsLaunchObligation(databasePath, field, now) + } catch { + // A database that cannot be read cannot say it owes nothing. + return true + } +} diff --git a/src/main/runtime/agent-session-operation-admission.ts b/src/main/runtime/agent-session-operation-admission.ts index c7498ea33ee3..3eafd1397489 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' export type AgentSessionOperationAdmission = { callerKey: string @@ -187,7 +188,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 @@ -200,6 +201,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/agent-session-record-store.ts b/src/main/runtime/agent-session-record-store.ts index dc44c1c86e74..acc66430dead 100644 --- a/src/main/runtime/agent-session-record-store.ts +++ b/src/main/runtime/agent-session-record-store.ts @@ -309,6 +309,9 @@ export class AgentSessionRecordStore { await this.transact((draft) => settleAgentSessionOperationInto(draft, args)) } + /** Takes recorded launch follow-ups in one serialized ledger write. */ + transactOperations = (apply: (draft: AgentSessionStoreState) => T) => this.transact(apply) + /** The same settlement, committed by the journal write that makes it true. It changes only the * ledger, so no record listener is owed. */ operationOutcomeReceipt = (args: AgentSessionOperationSettlement): JournalOperationReceipt => diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index afd4be369cfa..4e65b5c5ce3e 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -97,6 +97,11 @@ class OrcaRuntimeService extends OrcaRuntimeWithMigrationCatalog { 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-active-operations.ts b/src/main/runtime/rpc/methods/agent-launch-active-operations.ts index 9801f4237eb9..38004869d12c 100644 --- a/src/main/runtime/rpc/methods/agent-launch-active-operations.ts +++ b/src/main/runtime/rpc/methods/agent-launch-active-operations.ts @@ -6,19 +6,16 @@ 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< - OrcaRuntimeService, - Map ->() +const activeAgentLaunchesByRuntime = new WeakMap>() -export function activeAgentLaunchesFor( - runtime: OrcaRuntimeService -): 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 new file mode 100644 index 000000000000..f5afe209dd35 --- /dev/null +++ b/src/main/runtime/rpc/methods/agent-launch-follow-up-late-settlement.test.ts @@ -0,0 +1,435 @@ +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 { 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 +} 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') +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 = { + 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: 'standalone', + 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 new file mode 100644 index 000000000000..8ed99a2021b7 --- /dev/null +++ b/src/main/runtime/rpc/methods/agent-launch-follow-ups.test.ts @@ -0,0 +1,300 @@ +/** + * 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, + AGENT_LAUNCH_PROMPT_UNCONFIRMED_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 (): Promise => true)) +const persisted = vi.hoisted(() => ({ followUps: true })) +vi.mock('../../agent-launch-persisted-obligations', () => ({ + hasPersistedLaunchObligation: () => persisted.followUps +})) +vi.mock('./agent-launch-terminal-prompt', () => ({ + deliverTerminalAgentLaunchPrompt: deliverTerminalPrompt +})) + +const { AGENT_LAUNCH_METHODS } = await import('./agent-launch') +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') + +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, + AGENT_LAUNCH_PROMPT_UNCONFIRMED_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 () => { + persisted.followUps = true + deliverTerminalPrompt.mockReset() + deliverTerminalPrompt.mockResolvedValue(true) + 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('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('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) + 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('keeps a reloaded window pending until the live host finishes its one prompt', 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 + // Flush the provisional receipt fired before the live prompt started. + await store.recordOperationOutcome({ + callerKey: 'absent', + operationId: 'absent', + outcome: { status: 'unknown' } + }) + + const expectedPending = { + taken: [], + pending: [{ operationId: OPERATION_ID, followUp: FOLLOW_UP }] + } + await expect(take(runtime, DESKTOP)).resolves.toEqual(expectedPending) + announceSettledLaunchFollowUps(runtime) + expect(runtime.reportAgentLaunchPromptSettled).not.toHaveBeenCalled() + const replay = launch(runtime) + expect(deliverTerminalPrompt).toHaveBeenCalledOnce() + + complete(true) + const [result, retry] = await Promise.all([running, replay]) + expect(retry).toEqual(result) + expect(runtime.reportAgentLaunchPromptSettled).toHaveBeenCalledExactlyOnceWith(OPERATION_ID) + expect((await take(runtime, DESKTOP)).taken[0]).toMatchObject({ promptHandedOver: true }) + await expect(take(runtime, DESKTOP)).resolves.toEqual({ taken: [], pending: [] }) + expect(deliverTerminalPrompt).toHaveBeenCalledOnce() + }) + + it('consumes a restarted interrupted launch without executing its follow-up or pasting later', async () => { + let started: () => void = () => {} + const promptStarted = new Promise((resolve) => { + started = resolve + }) + deliverTerminalPrompt.mockImplementationOnce(() => { + started() + return new Promise(() => {}) + }) + void launch(host()) + await promptStarted + await store.recordOperationOutcome({ + callerKey: 'absent', + operationId: 'absent', + outcome: { status: 'unknown' } + }) + store = await openTestAgentSessionRecordStore(directory) + setAgentLaunchRecordStore(store) + + const restarted = host() + const result = await launch(restarted) + expect(result.prompt).toEqual({ delivery: 'submit', outcome: 'unconfirmed' }) + expect((await take(restarted, DESKTOP)).taken[0]).toMatchObject({ promptHandedOver: false }) + await expect(take(restarted, DESKTOP)).resolves.toEqual({ taken: [], pending: [] }) + expect(restarted.createTerminal).not.toHaveBeenCalled() + expect(deliverTerminalPrompt).toHaveBeenCalledOnce() + }) + + it('does not authorize a follow-up on bare terminal creation', async () => { + const runtime = host() + await launch(runtime, { ...LAUNCH, prompt: undefined }) + + expect((await take(runtime, DESKTOP)).taken[0]).toMatchObject({ promptHandedOver: false }) + expect(deliverTerminalPrompt).not.toHaveBeenCalled() + }) + + it('allows concurrent callers to take a settled follow-up only once', async () => { + const runtime = host() + await launch(runtime) + const results = await Promise.all([take(runtime, DESKTOP), take(runtime, DESKTOP)]) + expect(results.flatMap((result) => result.taken)).toHaveLength(1) + 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) } } + + 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..fb65a5e3236d --- /dev/null +++ b/src/main/runtime/rpc/methods/agent-launch-follow-ups.ts @@ -0,0 +1,85 @@ +/** + * `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 { 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' + +/** + * 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: Pick & + 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 + } + for (const operationId of listSettledLaunchFollowUps( + store.listOperationRows(), + desktop, + Date.now(), + (operationKey) => activeAgentLaunchesFor(runtime).has(operationKey) + )) { + if (operationId !== completedId) { + 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) + } +} + +export const AGENT_LAUNCH_FOLLOW_UP_METHODS = [ + defineMethod({ + name: 'agent.takeLaunchFollowUps', + permission: 'workspace', + params: AgentTakeLaunchFollowUps, + handler: async (params, context): Promise => { + const callerKey = agentLaunchOperationCallerKey(context) + // 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, { + callerKey, + ...(params.operationId ? { operationId: params.operationId } : {}), + now: Date.now(), + isLaunchRunning: (operationKey) => active.has(operationKey) + }) + ) + } + }) +] diff --git a/src/main/runtime/rpc/methods/agent-launch-replay.ts b/src/main/runtime/rpc/methods/agent-launch-replay.ts index 290152cd1b59..64ec410c890c 100644 --- a/src/main/runtime/rpc/methods/agent-launch-replay.ts +++ b/src/main/runtime/rpc/methods/agent-launch-replay.ts @@ -31,7 +31,11 @@ import { resolveAgentSessionReplayOutcome } from '../../../native-chat/agent-ses 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' /** * The ledger namespace of whoever the transport says is calling. A transport that could not name its * caller gets no replay safety at all, rather than a namespace shared with strangers. @@ -208,8 +212,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: 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' || @@ -264,6 +277,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 +} + function refusal( operationId: string, code: AgentSessionOperationRefusalCode | 'agent_session_operation_unknown', 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 d53b9c48c7ef..9160a649152d 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/main/runtime/rpc/methods/agent-launch.ts b/src/main/runtime/rpc/methods/agent-launch.ts index 72e3ac32702c..2773aa2515b5 100644 --- a/src/main/runtime/rpc/methods/agent-launch.ts +++ b/src/main/runtime/rpc/methods/agent-launch.ts @@ -54,6 +54,7 @@ import { } from './agent-launch-caller-selection' import { agentLaunchWorkspaceFactory } from './agent-launch-worktree-creation' import { activeAgentLaunchesFor } from './agent-launch-active-operations' +import { announceSettledLaunchFollowUps } from './agent-launch-follow-ups' import { assertAgentLaunchTargetAuthorized } from './agent-launch-target-authorization' import { clientRendersStructuredAgent } from './structured-agent-session-policy' import { resolveUnlaunchedIntent } from './agent-launch-intent-resolution' @@ -275,6 +276,14 @@ 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, { + callerKey, + operationId: params.operationId + }) + } }) activeAgentLaunches.set(key, { fingerprint, promise }) return promise diff --git a/src/main/runtime/rpc/methods/index.ts b/src/main/runtime/rpc/methods/index.ts index 3370db3f2de5..011fe418d0db 100644 --- a/src/main/runtime/rpc/methods/index.ts +++ b/src/main/runtime/rpc/methods/index.ts @@ -55,6 +55,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 @@ -75,6 +76,7 @@ export const ALL_RPC_METHODS = [ ...STRUCTURED_AGENT_SESSION_VISUAL_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 d13da3976f1d..332621f68cfb 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 95abe7c35158..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' @@ -147,6 +148,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, @@ -229,6 +231,7 @@ export function registerRuntimeWindowLifecycle( return () => { if (fence && runtime.markRendererReloadCancelled(mainWindow.id, fence)) { rendererNotifications.onMainFrameReloadCancelled() + announceSettledLaunchFollowUps(runtime) } } }) 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 a9555fae256d..f0a2afa667a6 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,12 @@ 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 02dce476eeca..9a725b07345e 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 ebe655728115..385473b9a40f 100644 --- a/src/renderer/src/app-shell/use-app-shell-services.ts +++ b/src/renderer/src/app-shell/use-app-shell-services.ts @@ -16,6 +16,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 { useHostModelCatalogSnapshotsSync } from '../runtime/host-model-catalog-snapshots-sync' import { useRemoteRuntimeRecoveryTriggers } from '../runtime/use-remote-runtime-recovery-triggers' @@ -43,6 +44,7 @@ export function useAppShellServices(): void { useRadixBodyPointerEventsRecovery() useWebSessionTabsSync() useLocalStructuredSessionTabsSync() + useRecordedLaunchFollowUps() useHostStructuredAgentsSync() useHostModelCatalogSnapshotsSync() // Subscribe to IPC push events 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..fd1eebbb1081 --- /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-up-waiter', () => ({ 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/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..bcec9e7345eb --- /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-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 + * 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.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/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.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/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/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/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/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 d1441f09ff42..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 @@ -1,24 +1,15 @@ 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 { registerOpenChecksPanelView } from './open-checks-panel-views' import type { ChecksAgentComposerState } from './panel-state-types' import type { ChecksPanelReview } from '../checks-panel-review' @@ -82,174 +73,39 @@ 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) // 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 +158,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-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-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.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-comment-resolution-after-delivery.ts b/src/renderer/src/components/right-sidebar/pr-comment-resolution-after-delivery.ts new file mode 100644 index 000000000000..cff15cd76e90 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/pr-comment-resolution-after-delivery.ts @@ -0,0 +1,217 @@ +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 on that review: refresh the cached comments, so one opened later shows them. */ + 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 + } + } + } + }) + + 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. + 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.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/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.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 new file mode 100644 index 000000000000..b1263778ea87 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/review-comments-resolution-follow-up.ts @@ -0,0 +1,33 @@ +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, 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) { + 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.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/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.launch-status.test.tsx b/src/renderer/src/components/tab-bar/QuickLaunchButton.launch-status.test.tsx index becabddc7b83..b4fa2ede3761 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 @@ -226,6 +226,28 @@ describe('QuickLaunchAgentMenuItems launches', () => { }) }) + it('records the menu’s follow-up on the launch it starts', () => { + launchMock.mockReturnValue({ + surface: { kind: 'host-published' }, + 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/components/tab-bar/QuickLaunchButton.tsx b/src/renderer/src/components/tab-bar/QuickLaunchButton.tsx index 861e239d95c1..874645278365 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' import { activeAgentNotesSendFailureMessage } from '@/lib/active-agent-note-send-result' export type QuickLaunchAgentMenuItemsProps = { @@ -38,6 +39,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. */ @@ -109,6 +112,7 @@ function QuickLaunchAgentMenuItemsInner({ promptDelivery, launchSource, onPromptDelivered, + durableFollowUp, onPromptHandedOff, disabled = false }: QuickLaunchAgentMenuItemsProps): React.JSX.Element | null { @@ -147,6 +151,7 @@ function QuickLaunchAgentMenuItemsInner({ ...(promptDelivery !== undefined ? { promptDelivery } : {}), ...(launchSource !== undefined ? { launchSource } : {}), ...(onPromptDelivered !== undefined ? { onPromptDelivered } : {}), + ...(durableFollowUp !== undefined ? { durableFollowUp } : {}), // Notes keep their text until it goes out, so the new chat's composer never gets a copy. ...(onPromptHandedOff ? { promptKeptByCaller: true as const } : {}) }) @@ -214,6 +219,7 @@ function QuickLaunchAgentMenuItemsInner({ promptDelivery, launchSource, onPromptDelivered, + durableFollowUp, onPromptHandedOff, disabled ] 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..b3fa7075957c --- /dev/null +++ b/src/renderer/src/lib/agent-launch-follow-up-waiter.test.ts @@ -0,0 +1,395 @@ +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 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, { 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.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('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)), { + 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']) + 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) + }) + + 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 new file mode 100644 index 000000000000..c0026866071e --- /dev/null +++ b/src/renderer/src/lib/agent-launch-follow-up-waiter.ts @@ -0,0 +1,206 @@ +/** + * 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 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. + */ + +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 an older host's deadline a waiting window takes once more. */ +const PAST_DEADLINE_GRACE_MS = 15_000 +/** 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 + +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, + retakeAfterMs?: 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) + }) + const cancelRetake = + retakeAfterMs === undefined + ? () => {} + : this.clock.schedule(retakeAfterMs, () => void this.take(operationId, false)) + this.waiting.set(operationId, { + done, + finish: () => { + cancelTimer() + cancelRetake() + release() + } + }) + this.listen() + if (this.heard?.has(operationId)) { + void this.take(operationId, true) + } + return done + } + + private listen(): void { + this.unsubscribe ??= this.clock.onSettled((operationId) => { + if (this.waiting.has(operationId)) { + void this.take(operationId, true) + } 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, finishIfUnavailable: boolean): Promise { + const next = (this.takes.get(operationId) ?? Promise.resolve()).then(() => + this.takeNow(operationId, finishIfUnavailable) + ) + 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, finishIfUnavailable: 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 (finishIfUnavailable && !take) { + 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, + * 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, CLICK_RETAKE_MS) +} 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..2dc35524c455 --- /dev/null +++ b/src/renderer/src/lib/agent-launch-follow-ups.test.ts @@ -0,0 +1,125 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +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[] = [] + 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 { 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]) +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] }] +}) + +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()) + +function isRecordedResolution(value: unknown): value is { selectedGroups: PRCommentGroup[] } { + return typeof value === 'object' && value !== null && 'selectedGroups' in value +} + +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 } + ] + }).payload + if (!isRecordedResolution(recorded)) { + throw new Error('unexpected payload') + } + const comments = recorded.selectedGroups.flatMap((group) => + group.kind === 'standalone' ? [group.comment] : [] + ) + expect(buildPRCommentBatchConversationReplyBody(comments)).toBe( + buildPRCommentBatchConversationReplyBody([COMMENT, long]) + ) + }) + + 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..388d8038572e --- /dev/null +++ b/src/renderer/src/lib/agent-launch-follow-ups.ts @@ -0,0 +1,180 @@ +/** + * 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 { 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 { + 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 { + // 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: snippetOnly(group.root), replies: group.replies.map(snippetOnly) } + : { ...group, comment: snippetOnly(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. */ +export function readLaunchFollowUp(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 + } +} + +/** 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 ? readLaunchFollowUp(entry.followUp) : null + await followUp?.run().catch((error: unknown) => { + console.warn(`[agent-launch] the ${entry.followUp.kind} follow-up failed`, error) + }) + } +} diff --git a/src/renderer/src/lib/agent-launch-through-host.ts b/src/renderer/src/lib/agent-launch-through-host.ts index 183276e33941..a26cdbd907b9 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), @@ -160,6 +164,7 @@ export function windowMakesHostLaunchTab(): boolean { export function launchAgentThroughHost(args: HostAgentLaunchArgs): { tabId: string + operationId: string outcome: Promise } { const store = useAppStore.getState() @@ -167,10 +172,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, { @@ -187,5 +193,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 c1de2a9ff509..5a795b9a48f2 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 { getRuntimeEnvironmentIdForWorktree } from '@/lib/worktree-runtime-owner' import { isWebRuntimeSessionActive } from '@/runtime/web-runtime-session' @@ -18,6 +19,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 { launchStructuredAgentFromNewTab } from '@/lib/launch-agent-in-new-tab-structured-route' @@ -69,14 +71,15 @@ 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. */ pendingActivationSpawn?: boolean /** Lets a workspace reveal itself before the selected surface opens. */ - beforeSurfaceOpen?: ( - surface: { kind: 'local-terminal' } | { kind: 'host-published' } - ) => boolean | void + beforeSurfaceOpen?: (surface: { kind: 'local-terminal' | 'host-published' }) => boolean | void /** Opens instead of this agent's terminal when the host declines its structured chat. */ onStructuredHostDeclined?: () => StructuredLaunchTerminal } @@ -90,7 +93,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 @@ -256,6 +259,7 @@ function launchAgentInNewTabInternal(args: LaunchAgentInNewTabArgs): LaunchAgent quickCommandLabel, ...(pendingActivationSpawn ? { pendingActivationSpawn: true } : {}), 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..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 @@ -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(), @@ -20,6 +21,31 @@ 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, + wait: vi.fn(async (_operationId: string, _followUp: unknown, _deadline?: number) => {}), + takeLaunchFollowUps: vi.fn( + async (_operationId?: string): Promise => ({ + 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) + } +})) + +vi.mock('@/lib/agent-launch-follow-up-waiter', () => ({ + waitForRecordedLaunchFollowUp: followUps.wait +})) const { launchNewTabPromptThroughHost, newTabPromptLaunchesThroughHost } = await import('./launch-agent-new-tab-host-route') @@ -29,7 +55,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 +73,8 @@ function launch( beforeEach(() => { vi.clearAllMocks() + followUps.hostRecords = true + followUps.order = [] notice.wasNotified.mockReturnValue(true) host.windowMakesHostLaunchTab.mockReturnValue(true) }) @@ -151,6 +179,102 @@ 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' } + }) + 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.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() + } + }) + + 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..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 @@ -13,6 +13,16 @@ 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' +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 = { + 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 +74,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; followUp: AgentLaunchFollowUp | undefined }, receipt: AgentLaunchPromptReceipt | undefined, seeded: boolean -): { delivered: boolean; failureNotified: boolean } { +): Promise { + const { tabId } = launch + // Runs here only when this click's own take returned it; one another take returned runs there. + let followUpRunsHere = true + let stillRecorded: { deadline?: number } | null = null + if (launch.followUp) { + const take = await takeLaunchFollowUps(launch.operationId) + 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) { args.onPromptDeliveryUnconfirmed?.() @@ -81,6 +102,18 @@ function settleHostPrompt( // Command Code has no prompt-submit hook; seed working when the prompt is submitted. 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?.() return { delivered: true, failureNotified: false } } @@ -112,6 +145,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 +156,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 +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, 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) diff --git a/src/shared/agent-launch-follow-up.ts b/src/shared/agent-launch-follow-up.ts new file mode 100644 index 000000000000..3d5cdc388028 --- /dev/null +++ b/src/shared/agent-launch-follow-up.ts @@ -0,0 +1,56 @@ +/** + * 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 a live launch; left on the record. Older hosts may include a deadline. */ + pending: { operationId: string; followUp: AgentLaunchFollowUp; deadline?: number }[] +} diff --git a/src/shared/agent-launch-runtime-capability.ts b/src/shared/agent-launch-runtime-capability.ts index b751c13b9c48..8ee8c7932ecf 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 + // Host-advertised: accepts `target.kind: 'create-folder-workspace'`. An older host refuses the // unknown target kind, so a client offers the new-folder-workspace launch only on this. It is not // enough on its own: the target also needs `folderWorkspace.create` authorization, which a @@ -62,5 +66,6 @@ export const AGENT_LAUNCH_RUNTIME_CAPABILITIES = [ AGENT_LAUNCH_PROMPT_CARRY_RUNTIME_CAPABILITY, AGENT_LAUNCH_PROMPT_UNCONFIRMED_RUNTIME_CAPABILITY, AGENT_LAUNCH_PLACEMENT_RUNTIME_CAPABILITY, + AGENT_LAUNCH_FOLLOW_UPS_RUNTIME_CAPABILITY, AGENT_LAUNCH_CREATE_FOLDER_WORKSPACE_RUNTIME_CAPABILITY ] as const diff --git a/src/shared/agent-session-operation-ledger.ts b/src/shared/agent-session-operation-ledger.ts index 8c34edc7bcb0..6129afcd019f 100644 --- a/src/shared/agent-session-operation-ledger.ts +++ b/src/shared/agent-session-operation-ledger.ts @@ -30,6 +30,7 @@ import { isAgentSessionConversationCommandResult, type AgentSessionConversationCommandResult } from './agent-session-conversation-command' +import type { AgentLaunchFollowUp } from './agent-launch-follow-up' export type AgentSessionOperationOutcome = | { status: 'pending' } @@ -87,6 +88,9 @@ export type AgentSessionOperationRow = { * malformed value costs that pane its verdict, never the row. */ ownedPane?: AgentSessionOperationOwnedPane + /** 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 } /** Unexpired rows naming this pane as theirs. */ @@ -198,7 +202,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) @@ -211,7 +220,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 f33c06e3b09b..a8d2ac5a19a3 100644 --- a/src/shared/rpc-contract/agent-launch-params.ts +++ b/src/shared/rpc-contract/agent-launch-params.ts @@ -89,6 +89,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(), @@ -173,3 +185,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 ce0481c1f171..ad2376de265f 100644 --- a/src/shared/rpc-contract/rpc-params-catalog.generated.ts +++ b/src/shared/rpc-contract/rpc-params-catalog.generated.ts @@ -40,7 +40,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 { AttachmentReadParams, AttachmentUploadAppendParams, @@ -604,6 +604,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,