From 3007c56eb50fddf55abf8ea815d4f85fc7abcf3e Mon Sep 17 00:00:00 2001 From: Jiawei Zhao Date: Tue, 1 Sep 2026 13:26:57 +0800 Subject: [PATCH] fix(desktop): skip archived session queries Automatic Skills and Plan refreshes could reach the Runtime Host after a session entered archival, producing avoidable session errors. CLOSES #4430 Signed-off-by: Jiawei Zhao Generated-by: OpenAI Codex --- apps/desktop/renderer-architecture.json | 4 +- .../bootstrap-selection-lease.test.ts | 33 +++- .../session-navigation-controller.test.ts | 2 +- ...n-navigation-row-actions-revisions.test.ts | 86 ++++++++- .../session-navigation-session-purge.test.ts | 1 + .../main/__tests__/session-query-gate.test.ts | 176 ++++++++++++++++++ apps/desktop/src/renderer/app-shell.tsx | 7 +- .../src/renderer/bootstrap-selection-lease.ts | 13 +- .../src/renderer/composer-mentions.tsx | 20 +- .../controller/session-row-actions.ts | 122 +++++++----- .../use-session-navigation-controller.ts | 5 +- .../features/session-navigation/ports.ts | 2 +- apps/desktop/src/renderer/plan-mode-panel.tsx | 50 ++++- .../src/renderer/session-catalog-state.ts | 41 ++++ 14 files changed, 487 insertions(+), 75 deletions(-) create mode 100644 apps/desktop/src/main/__tests__/session-query-gate.test.ts diff --git a/apps/desktop/renderer-architecture.json b/apps/desktop/renderer-architecture.json index b4d193049f..67dc47efb3 100644 --- a/apps/desktop/renderer-architecture.json +++ b/apps/desktop/renderer-architecture.json @@ -770,7 +770,7 @@ "useNewTaskChoice": 1, "useOnboardingSnapshot": 1, "usePlanModeState": 1, - "useRef": 24, + "useRef": 23, "useSessionCollaborationDialog": 1, "useSessionEventHealthPolling": 1, "useSessionNavigationReads": 1, @@ -891,7 +891,7 @@ "react": 1 }, "importSpecifiers": 148, - "nonTriviaTokens": 15602 + "nonTriviaTokens": 15600 }, "src/renderer/use-app-shell-composer-quotes.ts": { "importDeclarations": 2, diff --git a/apps/desktop/src/main/__tests__/bootstrap-selection-lease.test.ts b/apps/desktop/src/main/__tests__/bootstrap-selection-lease.test.ts index 3179171fa3..6d67625ee4 100644 --- a/apps/desktop/src/main/__tests__/bootstrap-selection-lease.test.ts +++ b/apps/desktop/src/main/__tests__/bootstrap-selection-lease.test.ts @@ -30,10 +30,10 @@ import { writeNewTaskReloadDraft, } from '../../renderer/new-task-reload-intent.js'; -type Summary = { id: string; lastMessageAt?: number }; +type Summary = { id: string; lastMessageAt?: number; isArchived: boolean }; -function session(id: string, lastMessageAt?: number): Summary { - return { id, lastMessageAt }; +function session(id: string, lastMessageAt?: number, isArchived = false): Summary { + return { id, lastMessageAt, isArchived }; } function harness(activeId?: string) { @@ -88,6 +88,33 @@ describe('bootstrap selection lease', () => { assert.equal(state.activeId(), undefined); }); + for (const { name, initialActiveId, sessions, expected } of [ + { + name: 'the freshest session is archived', + initialActiveId: undefined, + sessions: [session('archived', 2, true), session('active', 1)], + expected: 'active', + }, + { + name: 'the bootstrap-owned selection is archived', + initialActiveId: 'archived', + sessions: [session('archived', 2, true), session('active', 1)], + expected: 'active', + }, + { + name: 'every session is archived', + initialActiveId: 'archived', + sessions: [session('archived', 1, true)], + expected: undefined, + }, + ] as const) { + it(`skips archived sessions when ${name}`, () => { + const state = harness(initialActiveId); + assert.equal(state.lease.reconcile(sessions), true); + assert.equal(state.activeId(), expected); + }); + } + it('does not reconcile after release', () => { const state = harness(); state.lease.release(); diff --git a/apps/desktop/src/main/__tests__/session-navigation-controller.test.ts b/apps/desktop/src/main/__tests__/session-navigation-controller.test.ts index 77ae054556..ee4e0c0fd6 100644 --- a/apps/desktop/src/main/__tests__/session-navigation-controller.test.ts +++ b/apps/desktop/src/main/__tests__/session-navigation-controller.test.ts @@ -108,7 +108,7 @@ function ports( return { activeIdRef: { current: activeSessionId }, sessionsRef: { current: sessions }, - pendingSessionRowActionsRef: { current: new Set() }, + acquireAutomaticQueryBlock: () => ({ release: () => undefined }), activateSession: (sessionId) => calls.push(`activate:${sessionId ?? 'none'}`), clearActiveMessages: () => calls.push('clear-messages'), clearSessionRendererState: (sessionId) => calls.push(`clear:${sessionId}`), diff --git a/apps/desktop/src/main/__tests__/session-navigation-row-actions-revisions.test.ts b/apps/desktop/src/main/__tests__/session-navigation-row-actions-revisions.test.ts index 7e7a59f9ef..11c6e004e4 100644 --- a/apps/desktop/src/main/__tests__/session-navigation-row-actions-revisions.test.ts +++ b/apps/desktop/src/main/__tests__/session-navigation-row-actions-revisions.test.ts @@ -90,14 +90,22 @@ describe('revision-family session row actions', () => { }); const branch = summary('branch', { parentSessionId: 'root', branchOfTurnId: 'turn-1' }); const activeIdRef = { current: 'root' as string | undefined }; + const service = createService(calls); const actions = createSessionNavigationRowActions({ uiLocale: 'en', activeIdRef, + acquireAutomaticQueryBlock: (ids) => { + calls.push(`acquire:${ids.join(',')}`); + return { release: () => calls.push('release') }; + }, clearActiveMessages: () => undefined, clearSessionRendererState: (id) => { cleared.push(id); }, pendingSessionRowActionsRef: { current: new Set() }, - refreshSessions: async () => [root, version, branch], - service: createService(calls), + refreshSessions: async () => { + calls.push('refresh'); + return [root, version, branch]; + }, + service, sessionsRef: { current: [root, version, branch] }, setActiveId: (id) => { selections.push(id); activeIdRef.current = id; }, toastApi: { @@ -115,17 +123,84 @@ describe('revision-family session row actions', () => { assert.deepEqual(calls, [ 'flag:version:true:true', + 'refresh', 'rename:branch:Independent branch:true', + 'refresh', + 'acquire:root,version', 'archive:version:true', + 'refresh', + 'release', // The delete asks the Host how many subtasks it would archive before the // confirm, then removes. 'preview:root', + 'acquire:root,version', // `root` is not archived, so the delete states no archived premise — // requiring one would refuse every delete from the rail. 'remove:root:true:false', + 'refresh', + 'release', ]); assert.deepEqual(selections, [undefined, undefined]); assert.deepEqual(cleared, ['root', 'version', 'root', 'version']); + + service.archive = async () => { throw new Error('archive failed'); }; + await actions.archiveSession('root'); + assert.deepEqual(calls.slice(-2), ['acquire:root,version', 'release']); + }); + + it('holds one query block through a bulk archive refresh', async () => { + const calls: string[] = []; + const root = summary('root'); + const version = summary('version', { + revisionRootSessionId: 'root', + revisionParentSessionId: 'root', + }); + const other = summary('other'); + let rejectRefresh = false; + const actions = createSessionNavigationRowActions({ + uiLocale: 'en', + activeIdRef: { current: undefined }, + acquireAutomaticQueryBlock: (ids) => { + calls.push(`acquire:${ids.join(',')}`); + return { release: () => calls.push('release') }; + }, + clearActiveMessages: () => undefined, + clearSessionRendererState: () => undefined, + pendingSessionRowActionsRef: { current: new Set() }, + refreshSessions: async () => { + calls.push('refresh'); + if (rejectRefresh) throw new Error('refresh failed'); + return []; + }, + service: createService(calls), + sessionsRef: { current: [root, version, other] }, + setActiveId: () => undefined, + toastApi: { + success: () => undefined, + error: () => undefined, + confirm: async () => true, + }, + }); + + await actions.archiveSelected(['version', 'other']); + + assert.deepEqual(calls, [ + 'acquire:root,version,other', + 'archive:version:true', + 'archive:other:true', + 'refresh', + 'release', + ]); + + calls.length = 0; + rejectRefresh = true; + await assert.rejects(actions.archiveSelected(['other']), /refresh failed/); + assert.deepEqual(calls, [ + 'acquire:other', + 'archive:other:true', + 'refresh', + 'release', + ]); }); }); @@ -138,9 +213,11 @@ function deleteHarness( const calls: string[] = []; const confirms: Array<{ title: string; description: string }> = []; const successes: Array<{ title: string; description?: string }> = []; + let leaseReleased = false; const actions = createSessionNavigationRowActions({ uiLocale: 'en', activeIdRef: { current: undefined }, + acquireAutomaticQueryBlock: () => ({ release: () => { leaseReleased = true; } }), clearActiveMessages: () => undefined, clearSessionRendererState: () => undefined, pendingSessionRowActionsRef: { current: new Set() }, @@ -154,7 +231,7 @@ function deleteHarness( confirm: async (options) => { confirms.push({ title: options.title, description: options.description }); return true; }, }, }); - return { actions, calls, confirms, successes }; + return { actions, calls, confirms, successes, wasLeaseReleased: () => leaseReleased }; } describe('delete confirm warns off the Host preview, toast reports the Host count', () => { @@ -219,7 +296,7 @@ describe('delete confirm warns off the Host preview, toast reports the Host coun }); it('stays silent on the toast when a concurrent restore calls the delete off', async () => { - const { actions, confirms, successes } = deleteHarness( + const { actions, confirms, successes, wasLeaseReleased } = deleteHarness( [summary('parent', { name: 'hi' })], 'restored', 0, @@ -232,5 +309,6 @@ describe('delete confirm warns off the Host preview, toast reports the Host coun assert.match(confirms[0].description, /kept and moved to Archived/); // But nothing was deleted, so nothing moved to the archive. assert.deepEqual(successes, [{ title: 'hi was restored, so it was kept', description: undefined }]); + assert.equal(wasLeaseReleased(), true); }); }); diff --git a/apps/desktop/src/main/__tests__/session-navigation-session-purge.test.ts b/apps/desktop/src/main/__tests__/session-navigation-session-purge.test.ts index 83a5f7099b..0387541cf1 100644 --- a/apps/desktop/src/main/__tests__/session-navigation-session-purge.test.ts +++ b/apps/desktop/src/main/__tests__/session-navigation-session-purge.test.ts @@ -142,6 +142,7 @@ function createActions(input: { return createSessionNavigationRowActions({ uiLocale: 'en', activeIdRef: input.activeIdRef, + acquireAutomaticQueryBlock: () => ({ release: () => undefined }), clearActiveMessages: () => undefined, clearSessionRendererState: (id) => { input.harness.cleared.push(id); diff --git a/apps/desktop/src/main/__tests__/session-query-gate.test.ts b/apps/desktop/src/main/__tests__/session-query-gate.test.ts new file mode 100644 index 0000000000..7aa2765c8a --- /dev/null +++ b/apps/desktop/src/main/__tests__/session-query-gate.test.ts @@ -0,0 +1,176 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import assert from 'node:assert/strict'; +import { afterEach, test } from 'node:test'; +import { deferred } from '@maka/core/test-only/async-primitives'; +import type { PlanSessionState } from '@maka/core/plan'; +import type { SessionSummary } from '@maka/core/session'; +import { LocaleProvider, ToastProvider } from '@maka/ui'; +import { act, createElement } from 'react'; +import type { DesktopSessionSummary } from '../../preload/bridge-contract.js'; +import { + ComposerMentionsProvider, + useComposerMentionsContext, +} from '../../renderer/composer-mentions.js'; +import { + usePlanModeState, + type PlanModeState, +} from '../../renderer/plan-mode-panel.js'; +import { createSessionCatalogController } from '../../renderer/session-catalog-state.js'; +import { cleanupFakeDom, installReactRenderer } from './fake-dom.js'; + +afterEach(cleanupFakeDom); + +test('query blocking pauses, resumes, and fences automatic Skills and Plan reads', async () => { + const { root } = installReactRenderer(); + const session = { id: 'session', isArchived: false } as SessionSummary; + const catalog = createSessionCatalogController(); + catalog.commitSessions([{ ...session, isArchived: true } as DesktopSessionSummary]); + const firstSkillQuery = deferred(); + const firstPlanQuery = deferred(); + const stalePlanState = { + schemaVersion: 1, + sessionId: session.id, + storeVersion: 1, + proposals: [], + executions: [], + } satisfies PlanSessionState; + const freshPlanState = { ...stalePlanState, storeVersion: 2 } satisfies PlanSessionState; + let skillQueries = 0; + let planQueries = 0; + let revisionRequests = 0; + let planState: PlanSessionState | undefined; + let planMode: PlanModeState | undefined; + let skillsUnavailable = false; + + (globalThis.window as unknown as { maka: unknown }).maka = { + skills: { + listInvocable: async () => { + skillQueries += 1; + return skillQueries === 1 ? firstSkillQuery.promise : []; + }, + }, + sessions: { + getPlanState: async () => { + planQueries += 1; + return planQueries === 1 ? firstPlanQuery.promise : freshPlanState; + }, + requestPlanRevision: async () => { + revisionRequests += 1; + }, + subscribeChanges: () => () => undefined, + subscribeEvents: () => () => undefined, + subscribePlanChanges: () => () => undefined, + }, + mcp: { subscribeChanges: () => () => undefined }, + }; + + function QueryProbe() { + const plan = usePlanModeState(session, catalog); + planMode = plan; + planState = plan.state; + skillsUnavailable = useComposerMentionsContext()?.mentionSkillsUnavailable ?? false; + return null; + } + + await act(async () => { + root.render(createElement(LocaleProvider, { + locale: 'en', + children: createElement(ToastProvider, { + children: createElement(ComposerMentionsProvider, { + skillCatalogRevision: 0, + sessionId: session.id, + automaticQueryGate: catalog, + children: createElement(QueryProbe), + }), + }), + })); + await Promise.resolve(); + }); + + assert.deepEqual([skillQueries, planQueries], [0, 0]); + + await act(async () => { + catalog.commitSessions([session as DesktopSessionSummary]); + await Promise.resolve(); + }); + + let lease!: ReturnType; + let overlappingLease!: ReturnType; + await act(async () => { + lease = catalog.acquireAutomaticQueryBlock([session.id]); + overlappingLease = catalog.acquireAutomaticQueryBlock([session.id]); + firstSkillQuery.reject(new Error('session archived')); + firstPlanQuery.resolve(stalePlanState); + await Promise.resolve(); + }); + + assert.equal(planState, undefined); + assert.equal(skillsUnavailable, false); + + await act(async () => { + lease.release(); + await Promise.resolve(); + }); + assert.deepEqual([skillQueries, planQueries], [1, 1]); + + await act(async () => { + overlappingLease.release(); + await Promise.resolve(); + }); + assert.deepEqual([skillQueries, planQueries], [2, 2]); + assert.deepEqual(planState, freshPlanState); + + let mutationLease!: ReturnType; + await act(async () => { + mutationLease = catalog.acquireAutomaticQueryBlock([session.id]); + await planMode?.requestRevision('proposal'); + }); + assert.equal(revisionRequests, 1); + assert.equal(planQueries, 3); + + await act(async () => { + mutationLease.release(); + await Promise.resolve(); + }); + assert.deepEqual([skillQueries, planQueries], [3, 4]); +}); + +test('query block membership is part of the catalog snapshot', () => { + const catalog = createSessionCatalogController(); + let notifications = 0; + const unsubscribe = catalog.subscribe(() => { + notifications += 1; + }); + + const first = catalog.acquireAutomaticQueryBlock(['session']); + assert.equal(catalog.getState().automaticQueryBlockedSessionIds.has('session'), true); + assert.equal(notifications, 1); + + const nested = catalog.acquireAutomaticQueryBlock(['session']); + first.release(); + assert.equal(catalog.getState().automaticQueryBlockedSessionIds.has('session'), true); + assert.equal(notifications, 1); + + nested.release(); + assert.equal(catalog.getState().automaticQueryBlockedSessionIds.has('session'), false); + assert.equal(notifications, 2); + unsubscribe(); +}); diff --git a/apps/desktop/src/renderer/app-shell.tsx b/apps/desktop/src/renderer/app-shell.tsx index 3e17e55038..8418f699bd 100644 --- a/apps/desktop/src/renderer/app-shell.tsx +++ b/apps/desktop/src/renderer/app-shell.tsx @@ -353,6 +353,7 @@ function AppShellContent({ transcriptRangeRef, messageLoadPending, setMessageLoadPending, + sessionCatalogController, sessionUiController, } = useAppShellSessionWorkspace(toastApi); const activeCatalogSession = sessions.find((session) => session.id === activeId); @@ -1226,7 +1227,7 @@ function AppShellContent({ ? sessionSettingIntent.overlays.permissionMode[activeId] ?? activeBoundarySurface.permissionMode : activeBoundarySurface.permissionMode; - const planMode = usePlanModeState(sharedSessionActive ? undefined : activeSessionForView); + const planMode = usePlanModeState(sharedSessionActive ? undefined : activeSessionForView, sessionCatalogController); const planConversationItems = (planMode.state?.proposals ?? []).map((proposal) => ({ id: proposal.proposalId, afterTurnId: proposal.turnId, @@ -1566,6 +1567,7 @@ function AppShellContent({ const composerMentionsSurface: ComposerMentionsSurface = { skillCatalogRevision: moduleHub.selectors.skillCatalogRevision, sessionId: ownerActiveId, + automaticQueryGate: sessionCatalogController, projectPath: activeId ? ownerActiveId ? projectInfo?.projectPath @@ -1632,7 +1634,6 @@ function AppShellContent({ useLayoutEffect(() => { openSessionInChatRef.current = openSession; }, [openSession]); - const pendingSessionRowActionsRef = useRef(new Set()); const sessionNavigationCommandsRef = useRef(null); // Built inline: the rail reads these through a ref published on commit, so // their identity carries no information and this object never has to be @@ -1640,7 +1641,7 @@ function AppShellContent({ const sessionNavigationPorts: SessionNavigationPorts = { activeIdRef, sessionsRef, - pendingSessionRowActionsRef, + acquireAutomaticQueryBlock: sessionCatalogController.acquireAutomaticQueryBlock, activateSession: setActiveId, clearActiveMessages, clearSessionRendererState, diff --git a/apps/desktop/src/renderer/bootstrap-selection-lease.ts b/apps/desktop/src/renderer/bootstrap-selection-lease.ts index 3a1cacb766..b393b196fa 100644 --- a/apps/desktop/src/renderer/bootstrap-selection-lease.ts +++ b/apps/desktop/src/renderer/bootstrap-selection-lease.ts @@ -17,12 +17,14 @@ * under the License. */ -export interface BootstrapSelectionLease { +type BootstrapSelectionSummary = { id: string; lastMessageAt?: number; isArchived: boolean }; + +export interface BootstrapSelectionLease { reconcile(sessions: readonly Summary[]): boolean; release(): void; } -export function createBootstrapSelectionLease(options: { +export function createBootstrapSelectionLease(options: { readActiveId: () => string | undefined; readSelectionRevision: () => number; select: (sessionId: string | undefined) => void; @@ -37,11 +39,12 @@ export function createBootstrapSelectionLease !session.isArchived); const current = options.readActiveId(); - const next = current && sessions.some((session) => session.id === current) + const next = current && activeSessions.some((session) => session.id === current) ? current - : sessions[0]?.lastMessageAt - ? sessions[0].id + : activeSessions[0]?.lastMessageAt + ? activeSessions[0].id : undefined; if (next !== current) options.select(next); ownedRevision = options.readSelectionRevision(); diff --git a/apps/desktop/src/renderer/composer-mentions.tsx b/apps/desktop/src/renderer/composer-mentions.tsx index 4a4eeb62fc..97d8375eea 100644 --- a/apps/desktop/src/renderer/composer-mentions.tsx +++ b/apps/desktop/src/renderer/composer-mentions.tsx @@ -59,6 +59,10 @@ export interface ComposerMentionsSurface { /** Invalidates Runtime's invocable projection after installed Skills settle. */ skillCatalogRevision: number; sessionId?: string; + automaticQueryGate: { + subscribe(listener: () => void): () => void; + isAutomaticQueryBlocked(sessionId: string): boolean; + }; projectPath?: string; newSessionModel?: { llmConnectionSlug: string; model: string }; newSessionCollaborationMode?: 'agent' | 'plan'; @@ -78,6 +82,7 @@ function useComposerMentions(options: ComposerMentionsSurface): ComposerMentions const { projectPath, sessionId, + automaticQueryGate, skillCatalogRevision, newSessionModel, newSessionCollaborationMode, @@ -130,8 +135,11 @@ function useComposerMentions(options: ComposerMentionsSurface): ComposerMentions useEffect(() => { let cancelled = false; let requestVersion = 0; + const blocked = () => !!sessionId && automaticQueryGate.isAutomaticQueryBlocked(sessionId); + let queryBlocked = blocked(); const refresh = () => { const version = ++requestVersion; + if (queryBlocked) return; setCatalog((previous) => previous.contextKey === contextKey ? // A same-context refresh keeps both its settled verdict and the @@ -161,7 +169,7 @@ function useComposerMentions(options: ComposerMentionsSurface): ComposerMentions : Promise.resolve([]); void request.then( (next) => { - if (cancelled || version !== requestVersion) return; + if (cancelled || version !== requestVersion || queryBlocked) return; setCatalog((previous) => ({ contextKey, loading: false, @@ -179,12 +187,18 @@ function useComposerMentions(options: ComposerMentionsSurface): ComposerMentions () => { // Fail soft: an unavailable projection leaves `/` with no suggestions. // Direct `/skill:` input still reaches the same Runtime resolver. - if (cancelled || version !== requestVersion) return; + if (cancelled || version !== requestVersion || queryBlocked) return; setCatalog({ contextKey, loading: false, settled: 'empty', skills: EMPTY_SKILLS }); }, ); }; refresh(); + const unsubscribeQueryGate = automaticQueryGate.subscribe(() => { + const next = blocked(); + if (next === queryBlocked) return; + queryBlocked = next; + refresh(); + }); const unsubscribeSessions = window.maka.sessions.subscribeChanges((event) => { if ( sessionId && @@ -203,12 +217,14 @@ function useComposerMentions(options: ComposerMentionsSurface): ComposerMentions return () => { cancelled = true; requestVersion += 1; + unsubscribeQueryGate(); unsubscribeSessions(); unsubscribeContext(); }; }, [ projectPath, sessionId, + automaticQueryGate, skillCatalogRevision, newSessionModel?.llmConnectionSlug, newSessionModel?.model, diff --git a/apps/desktop/src/renderer/features/session-navigation/controller/session-row-actions.ts b/apps/desktop/src/renderer/features/session-navigation/controller/session-row-actions.ts index a7fa25efb0..f715eb6d31 100644 --- a/apps/desktop/src/renderer/features/session-navigation/controller/session-row-actions.ts +++ b/apps/desktop/src/renderer/features/session-navigation/controller/session-row-actions.ts @@ -90,6 +90,7 @@ export interface SessionNavigationRowActions { export function createSessionNavigationRowActions(deps: { uiLocale: UiLocale; activeIdRef: RefObject; + acquireAutomaticQueryBlock(sessionIds: readonly string[]): { release(): void }; clearActiveMessages: () => void; clearSessionRendererState: (sessionId: string) => void; pendingSessionRowActionsRef: RefObject>; @@ -102,6 +103,7 @@ export function createSessionNavigationRowActions(deps: { const { uiLocale, activeIdRef, + acquireAutomaticQueryBlock, clearActiveMessages, clearSessionRendererState, pendingSessionRowActionsRef, @@ -113,6 +115,26 @@ export function createSessionNavigationRowActions(deps: { } = deps; const copy = getShellCopy(uiLocale).sessionRowActions; + async function withAutomaticQueryBlockOn( + sessionIds: readonly string[], + action: () => Promise, + ): Promise { + const lease = acquireAutomaticQueryBlock(sessionIds); + try { + return await action(); + } finally { + lease.release(); + } + } + + async function withAutomaticQueryBlock( + sessionId: string, + action: (familyIds: readonly string[]) => Promise, + ): Promise { + const familyIds = revisionFamilySessionIds(sessionsRef.current, sessionId); + return withAutomaticQueryBlockOn(familyIds, () => action(familyIds)); + } + async function runSessionRowAction( sessionId: string, actionId: 'flag' | 'archive' | 'rename' | 'delete', @@ -146,14 +168,15 @@ export function createSessionNavigationRowActions(deps: { async function archiveSession(sessionId: string) { return runSessionRowAction(sessionId, 'archive', copy.archiveFailedTitle, async () => { - const familyIds = revisionFamilySessionIds(sessionsRef.current, sessionId); - await service.archive(sessionId, { revisionFamily: true }); - if (activeIdRef.current && familyIds.includes(activeIdRef.current)) { - setActiveId(undefined); - clearActiveMessages(); - } - for (const id of familyIds) clearSessionRendererState(id); - await refreshSessions(); + await withAutomaticQueryBlock(sessionId, async (familyIds) => { + await service.archive(sessionId, { revisionFamily: true }); + if (activeIdRef.current && familyIds.includes(activeIdRef.current)) { + setActiveId(undefined); + clearActiveMessages(); + } + for (const id of familyIds) clearSessionRendererState(id); + await refreshSessions(); + }); }); } @@ -206,10 +229,16 @@ export function createSessionNavigationRowActions(deps: { if (!ok) return; // The confirm named an archived task, so a restore revokes it. An active // task has no such premise to lose. - const { disposition, archivedSubtaskCount } = await removeSessionFamily(sessionId, { - requireArchived: session?.isArchived === true, - }); - await refreshSessions(); + const { disposition, archivedSubtaskCount } = await withAutomaticQueryBlock( + sessionId, + async () => { + const outcome = await removeSessionFamily(sessionId, { + requireArchived: session?.isArchived === true, + }); + await refreshSessions(); + return outcome; + }, + ); // `restored` means nothing was deleted, so no subtask moved either. On a // real delete the count is the Host's executed number, not an estimate. if (disposition === 'restored') toastApi.success(copy.deleteRestoredTitle(name)); @@ -360,40 +389,45 @@ export function createSessionNavigationRowActions(deps: { * a run of them is what a sweep exists to avoid. */ async function archiveSessions(sessionIds: readonly string[]): Promise { - const failed: string[] = []; - let firstFailure: SessionArchiveOutcome['firstFailure']; - let archived = 0; - for (const sessionId of sessionIds) { - const key = `${sessionId}:archive`; - if ( - Array.from(pendingSessionRowActionsRef.current).some((pending) => - pending.startsWith(`${sessionId}:`), - ) - ) { - failed.push(sessionId); - continue; - } - pendingSessionRowActionsRef.current.add(key); - try { - const familyIds = revisionFamilySessionIds(sessionsRef.current, sessionId); - await service.archive(sessionId, { revisionFamily: true }); - if (activeIdRef.current && familyIds.includes(activeIdRef.current)) { - setActiveId(undefined); - clearActiveMessages(); + const familyIds = [ + ...new Set(sessionIds.flatMap((id) => revisionFamilySessionIds(sessionsRef.current, id))), + ]; + return withAutomaticQueryBlockOn(familyIds, async () => { + const failed: string[] = []; + let firstFailure: SessionArchiveOutcome['firstFailure']; + let archived = 0; + for (const sessionId of sessionIds) { + const key = `${sessionId}:archive`; + if ( + Array.from(pendingSessionRowActionsRef.current).some((pending) => + pending.startsWith(`${sessionId}:`), + ) + ) { + failed.push(sessionId); + continue; + } + pendingSessionRowActionsRef.current.add(key); + try { + const familyIds = revisionFamilySessionIds(sessionsRef.current, sessionId); + await service.archive(sessionId, { revisionFamily: true }); + if (activeIdRef.current && familyIds.includes(activeIdRef.current)) { + setActiveId(undefined); + clearActiveMessages(); + } + for (const id of familyIds) clearSessionRendererState(id); + archived += 1; + } catch (error) { + failed.push(sessionId); + firstFailure ??= { error, sessionId }; + } finally { + pendingSessionRowActionsRef.current.delete(key); } - for (const id of familyIds) clearSessionRendererState(id); - archived += 1; - } catch (error) { - failed.push(sessionId); - firstFailure ??= { error, sessionId }; - } finally { - pendingSessionRowActionsRef.current.delete(key); } - } - // Once, after the whole sweep. Refreshing per task would re-render the rail - // under the user's cursor for every id in the selection. - await refreshSessions(); - return { archived, failed, firstFailure }; + // Once, after the whole sweep. Refreshing per task would re-render the + // rail under the user's cursor for every id in the selection. + await refreshSessions(); + return { archived, failed, firstFailure }; + }); } /** diff --git a/apps/desktop/src/renderer/features/session-navigation/controller/use-session-navigation-controller.ts b/apps/desktop/src/renderer/features/session-navigation/controller/use-session-navigation-controller.ts index c67a460271..b2f02dad9a 100644 --- a/apps/desktop/src/renderer/features/session-navigation/controller/use-session-navigation-controller.ts +++ b/apps/desktop/src/renderer/features/session-navigation/controller/use-session-navigation-controller.ts @@ -98,6 +98,7 @@ export function useSessionNavigationController( // be upstream of the rail, where a single ordinary `function` declaration // anywhere in the chain silently undoes the whole thing (#4109). const portsRef = useRef(ports); + const pendingSessionRowActionsRef = useRef(new Set()); useLayoutEffect(() => { portsRef.current = ports; }); @@ -107,10 +108,12 @@ export function useSessionNavigationController( createSessionNavigationRowActions({ uiLocale: locale, activeIdRef: portsRef.current.activeIdRef, + acquireAutomaticQueryBlock: (sessionIds) => + portsRef.current.acquireAutomaticQueryBlock(sessionIds), clearActiveMessages: () => portsRef.current.clearActiveMessages(), clearSessionRendererState: (sessionId) => portsRef.current.clearSessionRendererState(sessionId), - pendingSessionRowActionsRef: portsRef.current.pendingSessionRowActionsRef, + pendingSessionRowActionsRef, refreshSessions: () => portsRef.current.refreshSessions(), service, sessionsRef: portsRef.current.sessionsRef, diff --git a/apps/desktop/src/renderer/features/session-navigation/ports.ts b/apps/desktop/src/renderer/features/session-navigation/ports.ts index 8461fa502d..ea4abb1549 100644 --- a/apps/desktop/src/renderer/features/session-navigation/ports.ts +++ b/apps/desktop/src/renderer/features/session-navigation/ports.ts @@ -112,7 +112,7 @@ export interface SessionNavigationServices { export interface SessionNavigationPorts { activeIdRef: RefObject; sessionsRef: RefObject>; - pendingSessionRowActionsRef: RefObject>; + acquireAutomaticQueryBlock(sessionIds: readonly string[]): { release(): void }; activateSession(sessionId: string | undefined): void; clearActiveMessages(): void; clearSessionRendererState(sessionId: string): void; diff --git a/apps/desktop/src/renderer/plan-mode-panel.tsx b/apps/desktop/src/renderer/plan-mode-panel.tsx index 37c5b3d850..a024b36088 100644 --- a/apps/desktop/src/renderer/plan-mode-panel.tsx +++ b/apps/desktop/src/renderer/plan-mode-panel.tsx @@ -36,7 +36,13 @@ export interface PlanModeState { abandon(executionId: string, title: string): Promise; } -export function usePlanModeState(session: SessionSummary | undefined): PlanModeState { +export function usePlanModeState( + session: SessionSummary | undefined, + automaticQueryGate: { + subscribe(listener: () => void): () => void; + isAutomaticQueryBlocked(sessionId: string): boolean; + }, +): PlanModeState { const toastApi = useToast(); const copy = getPlanModeCopy(useUiLocale()); const [state, setState] = useState(); @@ -55,17 +61,41 @@ export function usePlanModeState(session: SessionSummary | undefined): PlanModeS turnId: string; } | undefined>(undefined); - const refresh = useCallback(async () => { - if (!session) return; - setState(await window.maka.sessions.getPlanState(session.id)); - }, [session?.id]); + const refresh = useCallback(async (options: { + automatic?: boolean; + isCurrent?: () => boolean; + } = {}) => { + const { automatic = true, isCurrent = () => true } = options; + if (!session || (automatic && automaticQueryGate.isAutomaticQueryBlocked(session.id))) return; + const next = await window.maka.sessions.getPlanState(session.id); + if (isCurrent() && (!automatic || !automaticQueryGate.isAutomaticQueryBlocked(session.id))) { + setState(next); + } + }, [automaticQueryGate, session?.id]); useEffect(() => { setState(undefined); setError(undefined); - if (!session) return; - const refreshOrReport = () => void refresh().catch((cause) => setError(message(cause))); + if (!session) { + return; + } + let requestVersion = 0; + const blocked = () => automaticQueryGate.isAutomaticQueryBlocked(session.id); + let queryBlocked = blocked(); + const refreshOrReport = () => { + const version = ++requestVersion; + if (queryBlocked) return; + void refresh({ isCurrent: () => version === requestVersion }).catch((cause) => { + if (version === requestVersion && !queryBlocked) setError(message(cause)); + }); + }; refreshOrReport(); + const unsubscribeQueryGate = automaticQueryGate.subscribe(() => { + const next = blocked(); + if (next === queryBlocked) return; + queryBlocked = next; + refreshOrReport(); + }); const unsubscribeEvents = window.maka.sessions.subscribeEvents(session.id, (event: SessionEvent) => { if ( event.type === 'plan_submitted' @@ -80,17 +110,19 @@ export function usePlanModeState(session: SessionSummary | undefined): PlanModeS refreshOrReport, ); return () => { + requestVersion += 1; + unsubscribeQueryGate(); unsubscribeEvents(); unsubscribePlanChanges(); }; - }, [session?.id, session?.collaborationMode, refresh]); + }, [automaticQueryGate, session?.id, session?.collaborationMode]); const run = useCallback(async (action: () => Promise): Promise => { setPending(true); setError(undefined); try { await action(); - await refresh(); + await refresh({ automatic: false }); } catch (cause) { setError(message(cause)); } finally { diff --git a/apps/desktop/src/renderer/session-catalog-state.ts b/apps/desktop/src/renderer/session-catalog-state.ts index 294a73beef..60c1695963 100644 --- a/apps/desktop/src/renderer/session-catalog-state.ts +++ b/apps/desktop/src/renderer/session-catalog-state.ts @@ -40,6 +40,7 @@ export interface SessionCatalogState { readonly sessions: readonly DesktopSessionSummary[]; readonly revision: number; readonly activeSessionId: string | undefined; + readonly automaticQueryBlockedSessionIds: ReadonlySet; } export function createSessionCatalogController() { @@ -47,11 +48,51 @@ export function createSessionCatalogController() { sessions: [], revision: 0, activeSessionId: undefined, + automaticQueryBlockedSessionIds: new Set(), }); + const automaticQueryBlockCounts = new Map(); + const publishAutomaticQueryBlocks = () => { + const current = state.getState(); + const next = new Set(automaticQueryBlockCounts.keys()); + if ( + current.automaticQueryBlockedSessionIds.size === next.size + && [...next].every((id) => current.automaticQueryBlockedSessionIds.has(id)) + ) { + return; + } + state.replaceState({ ...current, automaticQueryBlockedSessionIds: next }); + }; return { getState: state.getState, subscribe: state.subscribe, + isAutomaticQueryBlocked(sessionId: string): boolean { + return ( + state.getState().automaticQueryBlockedSessionIds.has(sessionId) || + state.getState().sessions.some((session) => session.id === sessionId && session.isArchived) + ); + }, + acquireAutomaticQueryBlock(sessionIds: readonly string[]): { release(): void } { + const ids = [...new Set(sessionIds)]; + for (const id of ids) { + automaticQueryBlockCounts.set(id, (automaticQueryBlockCounts.get(id) ?? 0) + 1); + } + publishAutomaticQueryBlocks(); + + let released = false; + return { + release(): void { + if (released) return; + released = true; + for (const id of ids) { + const count = automaticQueryBlockCounts.get(id) ?? 0; + if (count <= 1) automaticQueryBlockCounts.delete(id); + else automaticQueryBlockCounts.set(id, count - 1); + } + publishAutomaticQueryBlocks(); + }, + }; + }, commitSessions(next: readonly DesktopSessionSummary[]): void { const current = state.getState(); state.replaceState({ ...current, sessions: next, revision: current.revision + 1 });