From ec2dce4f240c2282c3f985ba3eea8c2e58a85b9b Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 20 Sep 2026 23:34:11 -0700 Subject: [PATCH] refactor(native-chat): drop the reconnect vocabulary the resume action left behind MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resuming became one action — reattach and ask the agent to carry on — so the notification helpers no longer need to be told which action they are reporting. Every caller passed `continue`; the `reconnect` branch, its helper and its catalog keys are gone. The dialog and the launch path had grown two copies of the same call: same RPC, same response shape, same announce-and-settle. That now lives once in the store module that owns the offer, which also takes the dismiss call, leaving the modal presentational. The two copies had drifted — only the dialog's caught a malformed payload — and the unified one keeps the defensive reading. No behaviour change. `agentSession.restartResume` stays: it is a published wire method even though nothing in the app calls it. --- ...ured-agent-session-restart-continuation.ts | 10 +-- ...tured-agent-session-restart-resume-host.ts | 8 +- ...structured-agent-session-restart-resume.ts | 21 ++--- .../NativeChatResumeOnRestartModal.test.tsx | 8 +- .../NativeChatResumeOnRestartModal.tsx | 79 +++++-------------- ...ative-chat-restart-action-notifications.ts | 74 +++++------------ .../native-chat-resume-on-restart-dialog.ts | 14 ++-- .../native-chat-resume-on-restart-store.ts | 76 ++++++++++++------ .../src/i18n/en-runtime-required.json | 9 +-- src/renderer/src/i18n/locales/en.json | 11 --- .../agent-session-restart-continuation.ts | 11 ++- 11 files changed, 128 insertions(+), 193 deletions(-) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-restart-continuation.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-restart-continuation.ts index 447d59a5609..22dfcdff882 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-restart-continuation.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-restart-continuation.ts @@ -1,11 +1,9 @@ // Asking an interrupted agent to carry on, on the user's opt-in. // -// Reattaching and continuing are still SEPARATE operations: `resume` reattaches and sends nothing; -// this adds one message on top of it. What changed is WHO may ask. Resuming from the restart prompt -// comes here, and so does an opted-in launch, so this module is no longer unreachable from a -// setting — do not restate that old guarantee. It is acceptable because the work is the user's own, -// the message asks the agent to verify its last action before repeating it, and the launch toast -// reports what happened. +// Reattaching and continuing are SEPARATE operations: `resume` reattaches and sends nothing; this +// adds one message on top of it. Both the restart prompt and an opted-in launch come here, so a +// SETTING can reach this send — acceptable because the work is the user's own, the message asks the +// agent to verify its last action before repeating it, and the launch toast reports what happened. import type { AgentJournalMessageItem } from '../../../shared/agent-session-journal-types' import type { AgentSessionMutationEnvelope } from '../../../shared/agent-session-wire' diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-restart-resume-host.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-restart-resume-host.ts index 373235b4372..bc73f037f28 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-restart-resume-host.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-restart-resume-host.ts @@ -40,11 +40,9 @@ export type StructuredAgentSessionRestartResumeSurfaces = { /** The resume-capable hold; see the runner for why a hold and not a send. */ hold: (sessionId: string, holderId: string) => Promise release: (sessionId: string, holderId: string) => void - /** The host's own send. Reached ONLY from `continueAfterRestart`; `resume` still never calls it, - * so reattaching on its own sends nothing. That is no longer a guarantee about SETTINGS, though: - * a launch the user opted into calls `continueAfterRestart` directly, which is acceptable - * because the work is the user's own, the message asks the agent to verify its last action - * before repeating it, and the launch toast reports what happened. + /** The host's own send. Reached ONLY from `continueAfterRestart`, so reattaching on its own sends + * nothing — but an opted-in launch calls `continueAfterRestart` too, so this is not a guarantee + * about settings; see the continuation module for why that is acceptable. * * Typed against the wire result rather than a hand-written subset: a narrower local shape hid * `value.submission` here once, and the continuation reads it. */ diff --git a/src/main/runtime/rpc/methods/structured-agent-session-restart-resume.ts b/src/main/runtime/rpc/methods/structured-agent-session-restart-resume.ts index b6ae6a8f4f4..3c1f6cfb937 100644 --- a/src/main/runtime/rpc/methods/structured-agent-session-restart-resume.ts +++ b/src/main/runtime/rpc/methods/structured-agent-session-restart-resume.ts @@ -1,8 +1,9 @@ -// `agentSession.restartResumable` / `agentSession.restartResume` — the restart-resume offer. +// The restart-resume offer: list it, act on it, or turn it down. // -// Both reach for records on disk this process may not have opened yet, so they build the host the -// way hold and reveal do. Listing is read-only and takes nothing live; resuming goes through the -// host's single resume path, which re-derives eligibility rather than trusting the ids it is given. +// Each method reaches for records on disk this process may not have opened yet, so each builds the +// host the way hold and reveal do. Listing is read-only and takes nothing live; acting goes through +// the host's single resume path, which re-derives eligibility rather than trusting the ids it is +// given. import { defineMethod } from '../core' import { @@ -32,9 +33,9 @@ export const STRUCTURED_AGENT_SESSION_RESTART_RESUME_METHODS = [ } }), defineMethod({ - // Reattach AND ask each reattached agent to carry on — what the desktop prompt now calls - // resuming, and what an opted-in launch runs without asking. Still a separate method from - // `restartResume`, which sends nothing, but no longer one that only a button can reach. + // Reattach AND ask each reattached agent to carry on — what the desktop prompt calls resuming, + // and what an opted-in launch runs without asking. Separate from `restartResume`, which sends + // nothing, but reachable from a setting rather than only from a button. name: 'agentSession.restartContinue', params: RestartResumeParams, handler: async (params, ctx) => { @@ -47,9 +48,9 @@ export const STRUCTURED_AGENT_SESSION_RESTART_RESUME_METHODS = [ } }), defineMethod({ - // Reattach only, no send. The desktop prompt stopped calling this once its single action became - // resume-and-continue, but it stays: it is a published wire method, and its absence is what an - // older or non-desktop client would be met with. + // Reattach only, no send. No Orca surface calls it now — the desktop prompt's single action is + // resume-and-continue — but it is a PUBLISHED wire method, so dropping it is a wire removal an + // older or non-desktop client would meet as an unknown method. name: 'agentSession.restartResume', params: RestartResumeParams, handler: async (params, ctx) => { diff --git a/src/renderer/src/components/NativeChatResumeOnRestartModal.test.tsx b/src/renderer/src/components/NativeChatResumeOnRestartModal.test.tsx index d9ee8c285e4..eab773f2f66 100644 --- a/src/renderer/src/components/NativeChatResumeOnRestartModal.test.tsx +++ b/src/renderer/src/components/NativeChatResumeOnRestartModal.test.tsx @@ -111,8 +111,7 @@ it('keeps next-launch preference out of the current resume action', async () => expect(rpc).toHaveBeenCalledTimes(2) }) -// One primary action and one way out of it. The vacuous plain-reconnect button, the Not now button -// and the "what gets sent" popover are gone; the body copy carries the transparency now. +// One primary action and one way out of it; the body copy carries the transparency. it('offers exactly Dismiss all and the resume action', async () => { rpc.mockImplementation(async (_target, method) => method === 'agentSession.restartResumable' ? { sessions: offered } : { results: [] } @@ -127,9 +126,8 @@ it('offers exactly Dismiss all and the resume action', async () => { ]) }) -// Snoozing saves the preference like every other way out of the dialog, and calls NOTHING: the -// offer is the host's and stays exactly where it was. Closing the dialog is the only snooze left -// now that Not now is gone, so it has to keep doing all of that. +// Closing is the only snooze, so it carries the whole of one: saves the preference like every +// other way out, and calls NOTHING — the offer is the host's and stays exactly where it was. it('snoozes to the status-bar offer when the dialog is closed', async () => { rpc.mockImplementation(async (_target, method) => method === 'agentSession.restartResumable' ? { sessions: offered } : { results: [] } diff --git a/src/renderer/src/components/NativeChatResumeOnRestartModal.tsx b/src/renderer/src/components/NativeChatResumeOnRestartModal.tsx index ade29869642..2411f63a670 100644 --- a/src/renderer/src/components/NativeChatResumeOnRestartModal.tsx +++ b/src/renderer/src/components/NativeChatResumeOnRestartModal.tsx @@ -11,15 +11,8 @@ import { DialogTitle } from './ui/dialog' import { useAppStore } from '../store' -import { callStructuredAgentSession } from '@/runtime/structured-agent-session-client' import { translate } from '@/i18n/i18n' import { ResumeOnRestartGroups } from './NativeChatResumeOnRestartGroups' -import { - announceRestartDismissUnconfirmed, - announceRestartResults, - announceRestartUnconfirmed, - type RestartActionOutcome -} from './native-chat-restart-action-notifications' import { selectedResumeSessionIds } from './native-chat-resume-on-restart-grouping' import { consumeNativeChatResumeOnRestartDialogRequest, @@ -27,8 +20,8 @@ import { subscribeNativeChatResumeOnRestartDialog } from './native-chat-resume-on-restart-dialog' import { - clearNativeChatRestartOffer, - settleNativeChatRestartOffer, + continueNativeChatRestartOffer, + dismissNativeChatRestartOffer, useNativeChatRestartOffer } from './native-chat-resume-on-restart-store' @@ -37,24 +30,17 @@ import { * * The list is the point. Resuming a chat that was not working starts a provider the user never * asked for and puts a misleading row in front of them, so they see exactly which chats the last - * teardown recorded as mid-turn and decide. The checkbox is the opt-in to skipping this prompt in - * future — it removes the PROMPT, never a safety check: automatic mode calls the same RPC, which - * re-derives the same predicate and staggers the same way. + * teardown recorded as mid-turn and decide. The checkbox removes the PROMPT, never a safety check: + * automatic mode calls the same RPC, which re-derives the same predicate and staggers the same way. * * Resuming reattaches each session where it stopped AND asks that agent to carry on, which is the * only reason the prompt is worth showing: reattaching alone is what simply opening the chat does. * The user's own prompt is never re-sent, and every string here has to keep saying so. * - * Closing is a SNOOZE: the host keeps the offer and the status bar keeps a way back to it, so - * looking around before deciding cannot cost the recovery. Dismiss all is the only path that spends - * it, and even that loses nothing — opening a chat takes a resume-capable hold, which re-acquires - * the provider at the same cursor and retires the offer for it. + * Closing is a SNOOZE, so looking around before deciding cannot cost the recovery. Dismiss all is + * the only path that spends the offer. */ -// Structured sessions run on the machine hosting the runtime; both launch resolvers refuse anything -// else, so there is no remote target to aim this at. -const LOCAL = { kind: 'local' } as const - export function NativeChatResumeOnRestartModal(): React.JSX.Element | null { const structuredEnabled = useAppStore( (store) => store.settings?.experimentalStructuredNativeChat === true @@ -70,14 +56,9 @@ export function NativeChatResumeOnRestartModal(): React.JSX.Element | null { const updateSettings = useAppStore((store) => store.updateSettings) const [dontAskAgain, setDontAskAgain] = useState(false) const [busy, setBusy] = useState(false) - /** - * Which of the OFFERED chats to leave out. Tracked as EXCLUSIONS rather than a selection because - * the list is the host's and arrives — and shrinks — under an open dialog; a stored selection - * would need seeding from an effect every time it changed. - * - * This changes which eligible chats are acted on, never what is eligible: the ids below are - * intersected back against the host's own list, and the host re-derives the predicate regardless. - */ + /** Which of the OFFERED chats to leave out. Tracked as EXCLUSIONS rather than a selection because + * the list is the host's and arrives — and shrinks — under an open dialog; a stored selection + * would need seeding from an effect every time it changed. */ const [excluded, setExcluded] = useState>(() => new Set()) const selected = useMemo( () => @@ -108,30 +89,12 @@ export function NativeChatResumeOnRestartModal(): React.JSX.Element | null { } }, [dontAskAgain, updateSettings]) - /** - * The one action: reattach, then ask each agent to carry on. - * - * `restartContinue` is the host method behind it — reattaching without a send is a separate RPC - * this surface no longer calls, because opening the chat already does exactly that. - */ const resume = useCallback( async (sessionIds: string[]): Promise => { setBusy(true) try { void persistPreference() - const result = await callStructuredAgentSession<{ - /** Which chats the host actually reattached, and so which claims it spent. Optional - * because the payload is unvalidated: a shape this side did not expect must not turn a - * delivered continuation into a failure report. */ - resumed?: RestartActionOutcome[] - continued: RestartActionOutcome[] - }>(LOCAL, 'agentSession.restartContinue', { sessionIds }) - announceRestartResults(sessionIds, result.continued, 'continue') - // Resuming spends the claims, so the offer has to shrink with it — otherwise the status bar - // keeps counting chats the host has already handed back. - settleNativeChatRestartOffer((result.resumed ?? []).map((entry) => entry.sessionId)) - } catch { - announceRestartUnconfirmed(sessionIds.length, 'continue') + await continueNativeChatRestartOffer(sessionIds) } finally { setBusy(false) consumeNativeChatResumeOnRestartDialogRequest() @@ -146,18 +109,14 @@ export function NativeChatResumeOnRestartModal(): React.JSX.Element | null { void persistPreference() }, [persistPreference]) - /** The only path that spends the markers. */ const dismissAll = useCallback(async (): Promise => { setBusy(true) void persistPreference() - // The dismissal is the user's and lands here, whatever the host answers. A write Orca cannot - // confirm is reported rather than allowed to trap the dialog open behind a rejected promise. + // Bookkeeping never gates the user's own action: the dialog closes here whatever the host + // answers, rather than being trapped open behind a rejected promise. consumeNativeChatResumeOnRestartDialogRequest() try { - await callStructuredAgentSession(LOCAL, 'agentSession.restartResumableDismiss', {}) - clearNativeChatRestartOffer() - } catch { - announceRestartDismissUnconfirmed() + await dismissNativeChatRestartOffer() } finally { setBusy(false) } @@ -180,8 +139,8 @@ export function NativeChatResumeOnRestartModal(): React.JSX.Element | null { } }} > - {/* Height is capped, never the data: seeing WHICH chats would be resumed is the whole - point, so the list scrolls inside the dialog while the header and primary action stay. */} + {/* Height is capped, never the data: the list scrolls inside the dialog so the header and + the primary action stay put however many chats were interrupted. */} @@ -194,8 +153,8 @@ export function NativeChatResumeOnRestartModal(): React.JSX.Element | null { )} - {/* Carries the transparency an explainer popover used to hide behind an icon: what the - agent is told, and what is NOT re-sent. */} + {/* The transparency, in the copy rather than behind a disclosure: what the agent is + told, and what is NOT re-sent. */} {interruptedByUpdate ? translate( @@ -250,8 +209,8 @@ export function NativeChatResumeOnRestartModal(): React.JSX.Element | null { - {/* Two controls, and they are opposites: one spends the offer, one acts on it. Closing the - dialog is neither — it snoozes, so it needs no button of its own. */} + {/* Two controls, and they are opposites: one spends the offer, one acts on it. Closing is + neither — it snoozes, so it needs no button of its own. */} {/* Quiet, not destructive: this spends an offer, and opening a chat still reattaches it. */}