diff --git a/apps/desktop/src/main/__tests__/app-shell-chat-actions-fixture.ts b/apps/desktop/src/main/__tests__/app-shell-chat-actions-fixture.ts index f7f2bbc22f..ad52d98382 100644 --- a/apps/desktop/src/main/__tests__/app-shell-chat-actions-fixture.ts +++ b/apps/desktop/src/main/__tests__/app-shell-chat-actions-fixture.ts @@ -96,24 +96,21 @@ export function createActionsDeps() { return { uiLocale: 'en' as const, activeIdRef, - addPendingSessionAction: () => true, captureComposerImportOwner: () => ({ sessionId: undefined, navSection: 'sessions' as const, }), checkTaskSubmissionReadiness: async () => true, - clearPendingSessionAction: () => undefined, isNewChatSendSurfaceActive: () => true, isShellSurfaceOwnerActive: () => true, markSessionReadLocally: () => undefined, - messageRetryPendingRef: { current: new Set() }, + messageRetryPending: { claim: () => true, release: () => undefined }, refreshSessions: async () => [], activateSessionForFirstSend: async (sessionId: string) => { activeIdRef.current = sessionId; }, setActiveId: () => undefined, setMessageLoadErrorBySession: () => undefined, - setMessageRetryPendingBySession: () => undefined, setMessages: () => undefined, addTransientMessage: () => undefined, updateTransientMessage: () => undefined, diff --git a/apps/desktop/src/main/__tests__/app-shell-session-settings-actions.test.ts b/apps/desktop/src/main/__tests__/app-shell-session-settings-actions.test.ts index 6889c68786..7468c17afa 100644 --- a/apps/desktop/src/main/__tests__/app-shell-session-settings-actions.test.ts +++ b/apps/desktop/src/main/__tests__/app-shell-session-settings-actions.test.ts @@ -23,6 +23,7 @@ import type { LlmConnection } from '@maka/core/llm-connections'; import type { StoredMessage } from '@maka/core/session'; import type { DesktopSessionSummary } from '../../preload/bridge-contract.js'; import { createAppShellSessionSettingsActions } from '../../renderer/app-shell-session-settings-actions.js'; +import type { SessionPendingClaim } from '../../renderer/app-shell-session-ui-state.js'; function deferred() { let resolve!: (value: T) => void; @@ -56,6 +57,20 @@ function session(id: string): DesktopSessionSummary { }; } +/** The store's claim semantics over a plain map the assertions can read. */ +function pendingClaimOver(state: Record): SessionPendingClaim { + return { + claim(key) { + if (state[key] === true) return false; + state[key] = true; + return true; + }, + release(key) { + delete state[key]; + }, + }; +} + function createHarness(options: { confirm?: () => Promise; connections?: LlmConnection[]; @@ -65,8 +80,8 @@ function createHarness(options: { const activeIdRef = { current: 'session-a' as string | undefined }; const sessions = [session('session-a'), session('session-b')]; const sessionsRef = { current: sessions }; - const pending = new Set(); - const pendingBySession: Record = {}; + const permissionModePending: Record = {}; + const sessionModelPending: Record = {}; const modelCalls: string[] = []; const permissionCalls: string[] = []; const thinkingCalls: string[] = []; @@ -107,18 +122,12 @@ function createHarness(options: { activeIdRef, connections: options.connections ?? ([{ slug: 'e2e', name: 'E2E' }] as LlmConnection[]), messages: options.messages ?? [], - pendingPermissionModeChangesRef: { current: new Set() }, - pendingSessionModelChangesRef: { current: pending }, + permissionModePending: pendingClaimOver(permissionModePending), + sessionModelPending: pendingClaimOver(sessionModelPending), refreshSessions: async () => sessions, saveComposerDefaults: () => undefined, sessionsRef, setNewTaskPermissionMode: (mode) => void newTaskPermissionModes.push(mode), - setPendingPermissionModeBySession: () => undefined, - setPendingSessionModelBySession: (update) => { - const next = update(pendingBySession); - for (const key of Object.keys(pendingBySession)) delete pendingBySession[key]; - Object.assign(pendingBySession, next); - }, toastApi: { success: (title, description) => successes.push({ title, description }), error: (title, _description, _details, target) => { @@ -137,8 +146,8 @@ function createHarness(options: { modelCalls, modelResult, newTaskPermissionModes, - pending, - pendingBySession, + permissionModePending, + sessionModelPending, permissionCalls, sessionsRef, thinkingCalls, @@ -225,7 +234,7 @@ describe('AppShell session settings actions', () => { assert.deepEqual(harness.modelCalls, ['session-a']); assert.deepEqual(harness.thinkingCalls, []); - assert.equal(harness.pendingBySession['session-a'], true); + assert.equal(harness.sessionModelPending['session-a'], true); harness.modelResult.resolve(session('session-a')); await modelChange; @@ -312,7 +321,7 @@ describe('AppShell session settings actions', () => { assert.deepEqual(harness.modelCalls, ['session-a']); assert.deepEqual(harness.thinkingCalls, ['session-b']); - assert.deepEqual(harness.pending, new Set(['session-a', 'session-b'])); + assert.deepEqual(Object.keys(harness.sessionModelPending), ['session-a', 'session-b']); harness.thinkingResult.resolve(session('session-b')); await thinkingChange; @@ -332,11 +341,11 @@ describe('AppShell session settings actions', () => { assert.deepEqual(harness.thinkingCalls, ['session-a']); assert.deepEqual(harness.modelCalls, []); - assert.equal(harness.pendingBySession['session-a'], true); + assert.equal(harness.sessionModelPending['session-a'], true); harness.thinkingResult.resolve(session('session-a')); await thinkingChange; - assert.equal(harness.pendingBySession['session-a'], undefined); + assert.equal(harness.sessionModelPending['session-a'], undefined); }); it('releases the session owner after a failed mutation so the next action can run', async () => { @@ -346,8 +355,7 @@ describe('AppShell session settings actions', () => { harness.thinkingResult.reject(new Error('fixture failure')); await thinkingChange; - assert.equal(harness.pending.has('session-a'), false); - assert.equal(harness.pendingBySession['session-a'], undefined); + assert.equal(harness.sessionModelPending['session-a'], undefined); assert.equal(harness.errors.length, 1); assert.deepEqual(harness.errorTargets, [{ sessionId: 'session-a' }]); diff --git a/apps/desktop/src/main/__tests__/app-shell-stop-action.test.ts b/apps/desktop/src/main/__tests__/app-shell-stop-action.test.ts index e86ed76f1d..a16796aa15 100644 --- a/apps/desktop/src/main/__tests__/app-shell-stop-action.test.ts +++ b/apps/desktop/src/main/__tests__/app-shell-stop-action.test.ts @@ -39,10 +39,7 @@ test('removes exactly the transient messages the Host retracts while stopping', const stop = createAppShellStopAction({ uiLocale: 'en', activeIdRef: { current: 'session-1' }, - addPendingSessionAction: () => true, - clearPendingSessionAction: () => undefined, - setStopPendingBySession: () => undefined, - stopPendingRef: { current: new Set() }, + stopPending: { claim: () => true, release: () => undefined }, removeTransientMessage: (sessionId, messageId) => removed.push({ sessionId, messageId }), toastApi: { error() {} }, }); diff --git a/apps/desktop/src/main/__tests__/use-stable-actions.test.ts b/apps/desktop/src/main/__tests__/use-stable-actions.test.ts new file mode 100644 index 0000000000..b6c708f779 --- /dev/null +++ b/apps/desktop/src/main/__tests__/use-stable-actions.test.ts @@ -0,0 +1,74 @@ +/* + * 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 { strict as assert } from 'node:assert'; +import { afterEach, describe, it } from 'node:test'; +import { act, createElement, useState } from 'react'; +import { cleanupFakeDom, installReactRenderer } from './fake-dom.js'; +import { useStableActions } from '../../renderer/use-stable-actions.js'; + +/** + * Action identity is a contract in this renderer, not an implementation + * detail. AppShell's nine `useStableActions` call sites hand their results to + * consumers that list them in dependency arrays and pass them down as props; + * a per-render identity there re-arms effect timers and defeats `memo` on + * every commit — measured at 20 full re-renders of a 32-row sidebar for one + * session switch (#4109). + * + * Asserting it on the mechanism covers all nine by construction. What it + * cannot cover is a factory that goes through neither this hook nor a + * once-created object; that failure is invisible to types and to + * `useExhaustiveDependencies`, which is why the Session rail also carries an + * outcome budget in `session-rail-render-contract.spec.ts`. + */ +describe('useStableActions', () => { + afterEach(cleanupFakeDom); + + it('fixes action identities while delegating to the latest committed closures', () => { + const { root } = installReactRenderer(); + const identities: Array<{ report(): number }> = []; + let bump: ((next: number) => void) | undefined; + + function Probe(): null { + const [value, setValue] = useState(0); + bump = setValue; + // A factory whose closure genuinely captures a changing dep — the case + // the facade exists for. + identities.push( + useStableActions((deps: { value: number }) => ({ report: () => deps.value }), { value }), + ); + return null; + } + + act(() => { + root.render(createElement(Probe)); + }); + act(() => bump?.(1)); + act(() => bump?.(2)); + + assert.equal(identities.length, 3); + const [first] = identities; + assert.ok(first); + for (const actions of identities) { + assert.equal(actions, first, 'the facade itself is re-created'); + assert.equal(actions.report, first.report, 'a method identity changed between renders'); + } + assert.equal(first.report(), 2, 'the facade did not delegate to the latest committed render'); + }); +}); diff --git a/apps/desktop/src/renderer/app-shell-chat-actions.ts b/apps/desktop/src/renderer/app-shell-chat-actions.ts index fedf80b37e..300d5d5e9e 100644 --- a/apps/desktop/src/renderer/app-shell-chat-actions.ts +++ b/apps/desktop/src/renderer/app-shell-chat-actions.ts @@ -51,6 +51,7 @@ import { skillInvocationDisplayText, } from './skill-invocation-feedback.js'; import type { DesktopTranscriptRangeController } from './desktop-transcript-range-store.js'; +import type { SessionPendingClaim } from './app-shell-session-ui-state.js'; import { retainedAttachmentRefs, toComposerIngestItems, @@ -78,7 +79,6 @@ type ComposerImportOwner = { }; type RefBox = { current: T }; -type BooleanRecordUpdater = (updater: (current: Record) => Record) => void; type LiveTurnRecordUpdater = ( updater: (current: Record) => Record, ) => void; @@ -139,29 +139,18 @@ export interface AppShellChatActions { export function createAppShellChatActions(deps: { uiLocale: UiLocale; activeIdRef: RefBox; - addPendingSessionAction: ( - sessionId: string, - pendingRef: RefBox>, - setPendingBySession: BooleanRecordUpdater, - ) => boolean; captureComposerImportOwner: () => ComposerImportOwner; checkTaskSubmissionReadiness: () => Promise; - clearPendingSessionAction: ( - sessionId: string, - pendingRef: RefBox>, - setPendingBySession: BooleanRecordUpdater, - ) => void; isNewChatSendSurfaceActive: (owner: ComposerImportOwner) => boolean; /** The shell's one answer to "is this owner still the surface the user is * looking at". Both halves matter — the section AND the session id — which * is why the send path asks it instead of comparing the id itself. */ isShellSurfaceOwnerActive: (owner: ComposerImportOwner) => boolean; - messageRetryPendingRef: RefBox>; + messageRetryPending: SessionPendingClaim; refreshSessions: () => Promise; activateSessionForFirstSend: (sessionId: string) => Promise; setActiveId: (sessionId: string | undefined) => void; setMessageLoadErrorBySession: MessageLoadErrorUpdater; - setMessageRetryPendingBySession: BooleanRecordUpdater; setMessages: MessageListUpdater; addTransientMessage: ( sessionId: string, @@ -207,18 +196,15 @@ export function createAppShellChatActions(deps: { const { uiLocale, activeIdRef, - addPendingSessionAction, captureComposerImportOwner, checkTaskSubmissionReadiness, - clearPendingSessionAction, isNewChatSendSurfaceActive, isShellSurfaceOwnerActive, - messageRetryPendingRef, + messageRetryPending, refreshSessions, activateSessionForFirstSend, setActiveId, setMessageLoadErrorBySession, - setMessageRetryPendingBySession, setMessages, addTransientMessage, updateTransientMessage, @@ -817,7 +803,7 @@ export function createAppShellChatActions(deps: { } } async function retryMessages(sessionId: string) { - if (!addPendingSessionAction(sessionId, messageRetryPendingRef, setMessageRetryPendingBySession)) return; + if (!messageRetryPending.claim(sessionId)) return; try { if (activeIdRef.current !== sessionId) return; await transcriptRangeRef.current?.reload(); @@ -830,7 +816,7 @@ export function createAppShellChatActions(deps: { })); toastApi.error(copy.refreshFailedTitle, message, undefined, { sessionId }); } finally { - clearPendingSessionAction(sessionId, messageRetryPendingRef, setMessageRetryPendingBySession); + messageRetryPending.release(sessionId); } } diff --git a/apps/desktop/src/renderer/app-shell-effects.ts b/apps/desktop/src/renderer/app-shell-effects.ts index 6277ff143b..fa2fd09a1f 100644 --- a/apps/desktop/src/renderer/app-shell-effects.ts +++ b/apps/desktop/src/renderer/app-shell-effects.ts @@ -159,10 +159,7 @@ export function useAppShellBootstrapSubscriptions(options: { handleConnectionEvent: (event: ConnectionEvent) => void; openHelp: () => void; openSettings: () => void; - pendingPermissionModeChangesRef: RefBox>; - pendingSessionModelChangesRef: RefBox>; - pendingTurnActionTimersRef: RefBox>>; - pendingTurnActionsRef: RefBox>; + clearPendingTurnActions: () => void; projectPickerPendingRef: RefBox; projectPickerRequestRef: RefBox; refreshConnections: () => Promise; @@ -288,13 +285,7 @@ export function useAppShellBootstrapSubscriptions(options: { options.rendererMountedRef.current = false; options.projectPickerRequestRef.current += 1; options.projectPickerPendingRef.current = false; - for (const timeoutHandle of options.pendingTurnActionTimersRef.current.values()) { - clearTimeout(timeoutHandle); - } - options.pendingTurnActionTimersRef.current.clear(); - options.pendingTurnActionsRef.current.clear(); - options.pendingPermissionModeChangesRef.current.clear(); - options.pendingSessionModelChangesRef.current.clear(); + options.clearPendingTurnActions(); }); useEffect(() => { diff --git a/apps/desktop/src/renderer/app-shell-session-settings-actions.ts b/apps/desktop/src/renderer/app-shell-session-settings-actions.ts index 1b01820dce..35385308ab 100644 --- a/apps/desktop/src/renderer/app-shell-session-settings-actions.ts +++ b/apps/desktop/src/renderer/app-shell-session-settings-actions.ts @@ -28,9 +28,9 @@ import type { ThinkingLevel } from '@maka/core/model-thinking'; import type { UiLocale } from '@maka/core/ui-locale'; import type { DesktopSessionSummary } from '../preload/bridge-contract.js'; import { getShellCopy, localizedShellErrorMessage } from './locales/shell-copy.js'; +import type { SessionPendingClaim } from './app-shell-session-ui-state.js'; type RefBox = { current: T }; -type BooleanRecordUpdater = (updater: (current: Record) => Record) => void; type ToastApi = { success(title: string, description?: string): void; @@ -64,8 +64,8 @@ export function createAppShellSessionSettingsActions(deps: { activeIdRef: RefBox; connections: readonly LlmConnection[]; messages: readonly StoredMessage[]; - pendingPermissionModeChangesRef: RefBox>; - pendingSessionModelChangesRef: RefBox>; + permissionModePending: SessionPendingClaim; + sessionModelPending: SessionPendingClaim; refreshSessions: () => Promise; saveComposerDefaults: (patch: { model: { llmConnectionId: string; llmConnectionSlug: string; model: string }; @@ -73,8 +73,6 @@ export function createAppShellSessionSettingsActions(deps: { sessionsRef: RefBox; /** Persists the chat default; awaited so a failure surfaces as one. */ setNewTaskPermissionMode: (mode: ChatDefaultPermissionMode) => void | Promise; - setPendingPermissionModeBySession: BooleanRecordUpdater; - setPendingSessionModelBySession: BooleanRecordUpdater; toastApi: ToastApi; }): AppShellSessionSettingsActions { const { @@ -82,25 +80,16 @@ export function createAppShellSessionSettingsActions(deps: { activeIdRef, connections, messages, - pendingPermissionModeChangesRef, - pendingSessionModelChangesRef, + permissionModePending, + sessionModelPending, refreshSessions, saveComposerDefaults, sessionsRef, setNewTaskPermissionMode, - setPendingPermissionModeBySession, - setPendingSessionModelBySession, toastApi, } = deps; const copy = getShellCopy(uiLocale).sessionSettingsActions; - function omitSessionKey(current: Record, sessionId: string): Record { - if (!(sessionId in current)) return current; - const next = { ...current }; - delete next[sessionId]; - return next; - } - function modelLabel(connectionSlug: string, model: string): string { const connection = connections.find((entry) => entry.slug === connectionSlug); const displayName = connection?.models?.find((entry) => entry.id === model)?.displayName?.trim(); @@ -121,8 +110,14 @@ export function createAppShellSessionSettingsActions(deps: { ? sessionsRef.current.find((session) => session.id === sessionId)?.permissionMode : undefined; if (currentMode === mode) return true; + // No session means the chat default, which has no row to mark pending. It + // still needs a key of its own so two rapid switches cannot both run. const pendingKey = sessionId ?? '__global_permission_mode__'; - if (pendingPermissionModeChangesRef.current.has(pendingKey)) return false; + // Claimed before the confirm rather than after it: the dialog is part of + // the change, so a second click while it is open must not open a second + // one. The cost is that the control reads as pending while the user + // decides, which is what is actually true. + if (!permissionModePending.claim(pendingKey)) return false; if ( mode === 'bypass' && !(await toastApi.confirm({ @@ -133,15 +128,10 @@ export function createAppShellSessionSettingsActions(deps: { destructive: true, })) ) { + permissionModePending.release(pendingKey); return false; } - pendingPermissionModeChangesRef.current.add(pendingKey); - if (sessionId) - setPendingPermissionModeBySession((current) => ({ - ...current, - [sessionId]: true, - })); try { let nextMode = mode; if (sessionId) { @@ -165,8 +155,7 @@ export function createAppShellSessionSettingsActions(deps: { ); return false; } finally { - pendingPermissionModeChangesRef.current.delete(pendingKey); - if (sessionId) setPendingPermissionModeBySession((current) => omitSessionKey(current, sessionId)); + permissionModePending.release(pendingKey); } } @@ -179,12 +168,7 @@ export function createAppShellSessionSettingsActions(deps: { if (!sessionId) return; const previous = sessionsRef.current.find((session) => session.id === sessionId); const lastUsedModel = latestAssistantModelId(messages); - if (pendingSessionModelChangesRef.current.has(sessionId)) return; - pendingSessionModelChangesRef.current.add(sessionId); - setPendingSessionModelBySession((current) => ({ - ...current, - [sessionId]: true, - })); + if (!sessionModelPending.claim(sessionId)) return; try { const next = await window.maka.sessions.setModel(sessionId, input); if (activeIdRef.current === sessionId) { @@ -217,8 +201,7 @@ export function createAppShellSessionSettingsActions(deps: { ); } } finally { - pendingSessionModelChangesRef.current.delete(sessionId); - setPendingSessionModelBySession((current) => omitSessionKey(current, sessionId)); + sessionModelPending.release(sessionId); } } @@ -227,12 +210,7 @@ export function createAppShellSessionSettingsActions(deps: { if (!sessionId) return; const current = sessionsRef.current.find((session) => session.id === sessionId); if (current && current.thinkingLevel === level) return; - if (pendingSessionModelChangesRef.current.has(sessionId)) return; - pendingSessionModelChangesRef.current.add(sessionId); - setPendingSessionModelBySession((currentPending) => ({ - ...currentPending, - [sessionId]: true, - })); + if (!sessionModelPending.claim(sessionId)) return; try { await window.maka.sessions.setThinkingLevel(sessionId, level); if (activeIdRef.current === sessionId) { @@ -249,8 +227,7 @@ export function createAppShellSessionSettingsActions(deps: { ); } } finally { - pendingSessionModelChangesRef.current.delete(sessionId); - setPendingSessionModelBySession((currentPending) => omitSessionKey(currentPending, sessionId)); + sessionModelPending.release(sessionId); } } diff --git a/apps/desktop/src/renderer/app-shell-session-ui-state.ts b/apps/desktop/src/renderer/app-shell-session-ui-state.ts index bd3ba23947..ae0115175e 100644 --- a/apps/desktop/src/renderer/app-shell-session-ui-state.ts +++ b/apps/desktop/src/renderer/app-shell-session-ui-state.ts @@ -46,6 +46,20 @@ export interface MessageQueueUiState { type AppShellSessionUiStateMapKey = keyof AppShellSessionUiState; +/** The maps that record nothing but "an action is in flight for this key". */ +type BooleanMapKey = + | 'messageRetryPendingBySession' + | 'stopPendingBySession' + | 'pendingPermissionModeBySession' + | 'pendingSessionModelBySession'; + +export interface SessionPendingClaim { + /** Marks `key` in flight. Returns false — a no-op — if it already was. */ + claim(key: string): boolean; + /** Gives the claim back. Safe to call for a key that never held one. */ + release(key: string): void; +} + const SESSION_UI_MAP_KEYS = [ 'messageLoadErrorBySession', 'messageRetryPendingBySession', @@ -169,14 +183,41 @@ export function createAppShellSessionUiStateController( return (updater) => updateMap(key, updater); } + /** + * The in-flight claim on one of the pending maps: `claim` marks a key and + * reports whether it won, `release` gives it back. + * + * Both read and write the same map, which is what makes this the only + * representation of "an action is in flight". Each of these four used to be a + * `Set` ref for the duplicate guard beside a map for the rendered flag, + * synchronized by hand at every add and every `finally`, and cleared by two + * separate teardown paths that stayed aligned only by ordering. Nothing + * needed the ref: state replacement is synchronous, so a claim is visible to + * the next `getState()` in the same task. + */ + function createPendingClaim(key: BooleanMapKey): SessionPendingClaim { + return { + claim(claimKey: string): boolean { + if (currentState[key][claimKey] === true) return false; + updateMap(key, (current) => ({ ...current, [claimKey]: true })); + return true; + }, + release(claimKey: string): void { + updateMap(key, (current) => omitSessionKey(current, claimKey)); + }, + }; + } + return { getState: () => currentState, subscribe, liveTurnBySessionRef, sessionEventHealthBySessionRef, setMessageLoadErrorBySession: createMapSetter('messageLoadErrorBySession'), - setMessageRetryPendingBySession: createMapSetter('messageRetryPendingBySession'), - setStopPendingBySession: createMapSetter('stopPendingBySession'), + messageRetryPending: createPendingClaim('messageRetryPendingBySession'), + stopPending: createPendingClaim('stopPendingBySession'), + permissionModePending: createPendingClaim('pendingPermissionModeBySession'), + sessionModelPending: createPendingClaim('pendingSessionModelBySession'), setLiveTurnBySession: createMapSetter('liveTurnBySession'), setShellRunUpdatesBySession: createMapSetter('shellRunUpdatesBySession'), setInteractionBySession: createMapSetter('interactionBySession'), @@ -184,8 +225,6 @@ export function createAppShellSessionUiStateController( setSessionEventHealthBySession: ((updater) => { sessionEventHealthBySessionRef.current = updater(sessionEventHealthBySessionRef.current); }) satisfies StateUpdater>, - setPendingPermissionModeBySession: createMapSetter('pendingPermissionModeBySession'), - setPendingSessionModelBySession: createMapSetter('pendingSessionModelBySession'), /** * The authority said something about `turnId` — it started, failed to * start, or ended. Drop that arm's `unconfirmed` claim so a session list @@ -224,32 +263,14 @@ export type AppShellSessionUiStateController = ReturnType(null); - - if (!controllerRef.current) { - controllerRef.current = createAppShellSessionUiStateController(); - } - - const controller = controllerRef.current; - - return { - controller, - liveTurnBySessionRef: controller.liveTurnBySessionRef, - sessionEventHealthBySessionRef: controller.sessionEventHealthBySessionRef, - setMessageLoadErrorBySession: controller.setMessageLoadErrorBySession, - setMessageRetryPendingBySession: controller.setMessageRetryPendingBySession, - setStopPendingBySession: controller.setStopPendingBySession, - setLiveTurnBySession: controller.setLiveTurnBySession, - setShellRunUpdatesBySession: controller.setShellRunUpdatesBySession, - setInteractionBySession: controller.setInteractionBySession, - setMessageQueueBySession: controller.setMessageQueueBySession, - setSessionEventHealthBySession: controller.setSessionEventHealthBySession, - setPendingPermissionModeBySession: controller.setPendingPermissionModeBySession, - setPendingSessionModelBySession: controller.setPendingSessionModelBySession, - confirmLiveTurn: controller.confirmLiveTurn, - clearSessionUiState: controller.clearSessionUiState, - clearTurnTransientStateIfCurrent: controller.clearTurnTransientStateIfCurrent, - }; + controllerRef.current ??= createAppShellSessionUiStateController(); + return controllerRef.current; } diff --git a/apps/desktop/src/renderer/app-shell-stop-action.ts b/apps/desktop/src/renderer/app-shell-stop-action.ts index ea1fa70eaf..36525e8311 100644 --- a/apps/desktop/src/renderer/app-shell-stop-action.ts +++ b/apps/desktop/src/renderer/app-shell-stop-action.ts @@ -20,9 +20,9 @@ import type { UiLocale } from '@maka/core/ui-locale'; import { localizedShellErrorMessage } from './locales/shell-copy.js'; import { getDesktopConversationCopy } from './locales/conversation-copy.js'; +import type { SessionPendingClaim } from './app-shell-session-ui-state.js'; type RefBox = { current: T }; -type BooleanRecordUpdater = (updater: (current: Record) => Record) => void; type ToastApi = { error( @@ -36,35 +36,21 @@ type ToastApi = { export function createAppShellStopAction(deps: { uiLocale: UiLocale; activeIdRef: RefBox; - addPendingSessionAction: ( - sessionId: string, - pendingRef: RefBox>, - setPendingBySession: BooleanRecordUpdater, - ) => boolean; - clearPendingSessionAction: ( - sessionId: string, - pendingRef: RefBox>, - setPendingBySession: BooleanRecordUpdater, - ) => void; - setStopPendingBySession: BooleanRecordUpdater; - stopPendingRef: RefBox>; + stopPending: SessionPendingClaim; removeTransientMessage: (sessionId: string, messageId: string) => void; toastApi: ToastApi; }): () => Promise { const { uiLocale, activeIdRef, - addPendingSessionAction, - clearPendingSessionAction, - setStopPendingBySession, - stopPendingRef, + stopPending, removeTransientMessage, toastApi, } = deps; async function stop() { const sessionId = activeIdRef.current; - if (!sessionId || !addPendingSessionAction(sessionId, stopPendingRef, setStopPendingBySession)) return; + if (!sessionId || !stopPending.claim(sessionId)) return; try { const result = await window.maka.sessions.stop(sessionId, { source: 'stop_button' }); if (result?.kind === 'interrupted') { @@ -89,7 +75,7 @@ export function createAppShellStopAction(deps: { ); } } finally { - clearPendingSessionAction(sessionId, stopPendingRef, setStopPendingBySession); + stopPending.release(sessionId); } } diff --git a/apps/desktop/src/renderer/app-shell.tsx b/apps/desktop/src/renderer/app-shell.tsx index 5a30508f18..dab53a0d6f 100644 --- a/apps/desktop/src/renderer/app-shell.tsx +++ b/apps/desktop/src/renderer/app-shell.tsx @@ -201,7 +201,7 @@ import { type LiveContentSeed, } from './live-content-seed'; import { loadComposerDefaults, saveComposerDefaults } from './composer-defaults'; -import { useKeyedPendingRegistry } from './use-pending-action-registry'; +import { useTurnActionRegistry } from './use-turn-action-registry'; import { useComposerAttachments } from './use-composer-attachments'; import { useAppShellComposerQuotes } from './use-app-shell-composer-quotes'; import { useComposerMentions } from './use-composer-mentions'; @@ -353,22 +353,7 @@ function AppShellContent({ transcriptRangeRef, messageLoadPending, setMessageLoadPending, - messageRetryPendingRef, - stopPendingRef, sessionUiController, - liveTurnBySessionRef, - sessionEventHealthBySessionRef, - setMessageLoadErrorBySession, - setMessageRetryPendingBySession, - setStopPendingBySession, - setLiveTurnBySession, - confirmLiveTurn, - setShellRunUpdatesBySession, - setInteractionBySession, - setMessageQueueBySession, - setSessionEventHealthBySession, - setPendingPermissionModeBySession, - setPendingSessionModelBySession, } = useAppShellSessionWorkspace(toastApi); const interactionHydrationEpochRef = useRef(new Map()); const markInteractionChanged = useCallback((sessionId: string) => { @@ -894,54 +879,21 @@ function AppShellContent({ // mask. Per @kenji PR109d review: pending state prevents double-click // duplicate sibling turns by disabling the action button between // click and `sessions:changed turn-status-change` arriving. - // These de-dup registries (turn-footer actions and per-session permission-mode - // / model changes) share the same keyed-Set shape; see - // useKeyedPendingRegistry. Session-row mutations live in Session Navigation. - const turnActionRegistry = useKeyedPendingRegistry({ - trackState: true, - autoClearMs: 5000, - }); + // Session-row mutations live in Session Navigation; the per-session mode and + // model claims live in the session UI store. + const turnActionRegistry = useTurnActionRegistry(); const pendingTurnActions = turnActionRegistry.keys; - const permissionModeChangeRegistry = useKeyedPendingRegistry(); - const sessionModelChangeRegistry = useKeyedPendingRegistry(); const pendingKeyOf = (sessionId: string, turnId: string, actionId: string) => `${sessionId}:${turnId}:${actionId}`; - function omitSessionKey(current: Record, sessionId: string): Record { - if (!(sessionId in current)) return current; - const next = { ...current }; - delete next[sessionId]; - return next; - } - - function addPendingSessionAction( - sessionId: string, - pendingRef: { current: Set }, - setPendingBySession?: (updater: (current: Record) => Record) => void, - ): boolean { - if (pendingRef.current.has(sessionId)) return false; - pendingRef.current.add(sessionId); - setPendingBySession?.((current) => ({ ...current, [sessionId]: true })); - return true; - } - - function clearPendingSessionAction( - sessionId: string, - pendingRef: { current: Set }, - setPendingBySession?: (updater: (current: Record) => Record) => void, - ): void { - if (!pendingRef.current.has(sessionId)) return; - pendingRef.current.delete(sessionId); - setPendingBySession?.((current) => omitSessionKey(current, sessionId)); - } function clearSessionRendererState(sessionId: string): void { dropDisplayEvents(sessionId); + // `clearOwnedSessionState` ends in `clearSessionUiState`, which drops this + // session from every session-UI map — the four pending claims included. clearOwnedSessionState(sessionId); turnActionRegistry.clearForSession(sessionId); - permissionModeChangeRegistry.keysRef.current.delete(sessionId); planModeIntent.clear(sessionId); orchestrationModeIntent.clear(sessionId); - sessionModelChangeRegistry.keysRef.current.delete(sessionId); } const { @@ -953,14 +905,12 @@ function AppShellContent({ activeIdRef, connections, messages, - pendingPermissionModeChangesRef: permissionModeChangeRegistry.keysRef, - pendingSessionModelChangesRef: sessionModelChangeRegistry.keysRef, + permissionModePending: sessionUiController.permissionModePending, + sessionModelPending: sessionUiController.sessionModelPending, refreshSessions, saveComposerDefaults, sessionsRef, setNewTaskPermissionMode, - setPendingPermissionModeBySession, - setPendingSessionModelBySession, toastApi, }); @@ -1239,22 +1189,22 @@ function AppShellContent({ ) { return; } - setInteractionBySession((current) => reconcileInteractions(current, activeId, requests)); + sessionUiController.setInteractionBySession((current) => reconcileInteractions(current, activeId, requests)); }) .catch(() => {}); return () => { cancelled = true; }; - }, [activeId, setInteractionBySession]); + }, [activeId, sessionUiController.setInteractionBySession]); useEffect( () => window.maka.sessions.subscribeActiveInteractions(({ sessionId, interactions }) => { markInteractionChanged(sessionId); - setInteractionBySession((current) => + sessionUiController.setInteractionBySession((current) => reconcileInteractions(current, sessionId, interactions), ); }), - [markInteractionChanged, setInteractionBySession], + [markInteractionChanged, sessionUiController.setInteractionBySession], ); const activeBoundarySurface = deriveDesktopExecutionBoundarySurface( activeId, @@ -1754,25 +1704,22 @@ function AppShellContent({ } = useStableActions(createAppShellChatActions, { uiLocale, activeIdRef, - addPendingSessionAction, captureComposerImportOwner, checkTaskSubmissionReadiness: taskSubmissionReadyAtSend, - clearPendingSessionAction, isNewChatSendSurfaceActive, isShellSurfaceOwnerActive, - messageRetryPendingRef, + messageRetryPending: sessionUiController.messageRetryPending, refreshSessions, activateSessionForFirstSend, setActiveId, - setMessageLoadErrorBySession, - setMessageRetryPendingBySession, + setMessageLoadErrorBySession: sessionUiController.setMessageLoadErrorBySession, setMessages, addTransientMessage, updateTransientMessage, removeTransientMessage, transcriptRangeRef, - setLiveTurnBySession, - setInteractionBySession, + setLiveTurnBySession: sessionUiController.setLiveTurnBySession, + setInteractionBySession: sessionUiController.setInteractionBySession, onInteractionChanged: markInteractionChanged, onExecutionBoundaryChanged: reloadActiveExecutionBoundary, showModelSetupToast, @@ -1914,7 +1861,7 @@ function AppShellContent({ metadata?.workspaceFileReferences, sessionId ? retractedWorkspaceReferencesRef.current[sessionId] : undefined, ); - const liveTurn = sessionId ? liveTurnBySessionRef.current[sessionId] : undefined; + const liveTurn = sessionId ? sessionUiController.liveTurnBySessionRef.current[sessionId] : undefined; const runningTurnIds = sessionId ? sessionsRef.current.find((session) => session.id === sessionId)?.runningTurnIds : undefined; @@ -2176,10 +2123,7 @@ function AppShellContent({ const stop = createAppShellStopAction({ uiLocale, activeIdRef, - addPendingSessionAction, - clearPendingSessionAction, - setStopPendingBySession, - stopPendingRef, + stopPending: sessionUiController.stopPending, removeTransientMessage, toastApi, }); @@ -2196,12 +2140,12 @@ function AppShellContent({ } = useStableActions(createAppShellSessionEventHandlers, { uiLocale, activeIdRef, - liveTurnBySessionRef, + liveTurnBySessionRef: sessionUiController.liveTurnBySessionRef, refreshMessages, refreshSessions, - setLiveTurnBySession, - setInteractionBySession, - setMessageQueueBySession, + setLiveTurnBySession: sessionUiController.setLiveTurnBySession, + setInteractionBySession: sessionUiController.setInteractionBySession, + setMessageQueueBySession: sessionUiController.setMessageQueueBySession, projectQueuedTransientMessages, removeTransientMessage, displayBatch: sessionDisplayBatch, @@ -2249,16 +2193,13 @@ function AppShellContent({ applyE2eFixture, bootstrapSessions, clearPendingTurnActionsForSession: turnActionRegistry.clearForSession, - confirmLiveTurn, + confirmLiveTurn: sessionUiController.confirmLiveTurn, clearSessionRendererState, createSession, handleConnectionEvent, openHelp, openSettings, - pendingPermissionModeChangesRef: permissionModeChangeRegistry.keysRef, - pendingSessionModelChangesRef: sessionModelChangeRegistry.keysRef, - pendingTurnActionTimersRef: turnActionRegistry.timersRef, - pendingTurnActionsRef: turnActionRegistry.keysRef, + clearPendingTurnActions: turnActionRegistry.clearAll, projectPickerPendingRef, projectPickerRequestRef, refreshConnections: refreshConnectionProjections, @@ -2270,7 +2211,7 @@ function AppShellContent({ rendererMountedRef, setActiveId, setMessages, - setSessionEventHealthBySession, + setSessionEventHealthBySession: sessionUiController.setSessionEventHealthBySession, toastApi, }); useAppShellPersistenceEffects({ @@ -2312,11 +2253,11 @@ function AppShellContent({ handleEvent, beginObservationSeed, completeObservationSeed, - setMessageLoadErrorBySession, + setMessageLoadErrorBySession: sessionUiController.setMessageLoadErrorBySession, setMessageLoadPending, setMessages, transcriptRangeRef, - setSessionEventHealthBySession, + setSessionEventHealthBySession: sessionUiController.setSessionEventHealthBySession, toastApi, }); let newestDurablePromptSequence: number | null = null; @@ -2375,7 +2316,7 @@ function AppShellContent({ }) .catch((error) => { if (disposed || activeIdRef.current !== target.sessionId) return; - setMessageLoadErrorBySession((current) => ({ + sessionUiController.setMessageLoadErrorBySession((current) => ({ ...current, [target.sessionId]: localizedShellErrorMessage( error, @@ -2388,7 +2329,10 @@ function AppShellContent({ disposed = true; }; }, [activeId, searchScrollTarget?.nonce]); - useShellRunUpdates({ activeId, setShellRunUpdatesBySession }); + useShellRunUpdates({ + activeId, + setShellRunUpdatesBySession: sessionUiController.setShellRunUpdatesBySession, + }); useSessionEventHealthPolling({ activeId, activeInteraction, @@ -2397,8 +2341,8 @@ function AppShellContent({ hasInFlightLiveTools, refreshMessages, refreshSessions, - sessionEventHealthBySessionRef, - setSessionEventHealthBySession, + sessionEventHealthBySessionRef: sessionUiController.sessionEventHealthBySessionRef, + setSessionEventHealthBySession: sessionUiController.setSessionEventHealthBySession, }); function captureComposerImportOwner(): ComposerImportOwner { return { diff --git a/apps/desktop/src/renderer/session-workspace-actions.ts b/apps/desktop/src/renderer/session-workspace-actions.ts index bef2637567..503919e2d5 100644 --- a/apps/desktop/src/renderer/session-workspace-actions.ts +++ b/apps/desktop/src/renderer/session-workspace-actions.ts @@ -22,13 +22,15 @@ * list, and the transient (optimistic) message projection. * * Every dependency here is a ref box, a React state setter, or a method of the - * once-created session-UI controller — all fixed for the renderer's lifetime. - * The factory therefore runs ONCE (issue #1043's `useStableActions` exists for - * factories whose closures capture changing deps; these capture none, so the - * cheaper structural guarantee applies). Declaring these in the hook body - * instead handed every consumer a fresh identity per render, and - * `activateSession` alone rebuilt the Session rail's whole command chain, which - * defeated `SessionNavRow`'s `memo` on every commit. + * once-created session-UI controller — all fixed for the renderer's lifetime — + * so the factory runs ONCE and the identities follow structurally. + * + * Declaring these in the hook body instead handed every consumer a fresh + * identity per render, and `activateSession` alone rebuilt the Session rail's + * whole command chain, which defeated `SessionNavRow`'s `memo` on every commit. + * That is asserted in `session-workspace-action-identity.test.ts`; whether a + * factory earns its identity this way or through `useStableActions` is an + * implementation choice the test does not care about. */ import type { StoredMessage } from '@maka/core/session'; @@ -71,8 +73,6 @@ export function createSessionWorkspaceActions(deps: { transientMessagesBySessionRef: RefBox>>; transcriptRangeRef: RefBox; selectionRevisionRef: RefBox; - messageRetryPendingRef: RefBox>; - stopPendingRef: RefBox>; setActiveIdState: (next: string | undefined) => void; setMessagesState: (next: StoredMessage[]) => void; setTransientMessagesState: (next: TransientUserMessage[]) => void; @@ -85,8 +85,6 @@ export function createSessionWorkspaceActions(deps: { transientMessagesBySessionRef, transcriptRangeRef, selectionRevisionRef, - messageRetryPendingRef, - stopPendingRef, setActiveIdState, setMessagesState, setTransientMessagesState, @@ -220,8 +218,6 @@ export function createSessionWorkspaceActions(deps: { } function clearOwnedSessionState(sessionId: string): void { - messageRetryPendingRef.current.delete(sessionId); - stopPendingRef.current.delete(sessionId); transientMessagesBySessionRef.current.delete(sessionId); if (activeIdRef.current === sessionId) setTransientMessagesState([]); clearSessionUiState(sessionId); diff --git a/apps/desktop/src/renderer/stable-actions.ts b/apps/desktop/src/renderer/stable-actions.ts deleted file mode 100644 index 056c23bceb..0000000000 --- a/apps/desktop/src/renderer/stable-actions.ts +++ /dev/null @@ -1,44 +0,0 @@ -/* - * 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. - */ - -/** - * Delegating-actions facade behind `useStableActions` (issue #1043). - * - * App-shell action factories run in the render body, so every render allocates - * fresh handler identities. Any consumer that lists a handler in an effect dep - * array (the streaming-settle fallback timer did) tears down and re-arms on - * every render. The facade fixes the identity without freezing the closures: - * each method delegates to the latest committed render's factory result through - * `latestRef`, so calls always see fresh deps while consumers see one stable - * identity for the component's lifetime. - * - * Factories must return a constant key shape of function values — the - * facade's key set is fixed at creation. Kept React-free so the behavior is - * testable from `node:test`. - */ -export function createDelegatingActions(latestRef: { current: A }): A { - const facade: Record unknown> = {}; - for (const key of Object.keys(latestRef.current)) { - facade[key] = (...args: unknown[]) => { - const action = Reflect.get(latestRef.current, key) as unknown as (...args: unknown[]) => unknown; - return action(...args); - }; - } - return facade as unknown as A; -} diff --git a/apps/desktop/src/renderer/use-app-shell-session-workspace.ts b/apps/desktop/src/renderer/use-app-shell-session-workspace.ts index d607d11621..f8ccc337b6 100644 --- a/apps/desktop/src/renderer/use-app-shell-session-workspace.ts +++ b/apps/desktop/src/renderer/use-app-shell-session-workspace.ts @@ -39,11 +39,11 @@ type TransientUserMessage = TransientUserMessageProjection; export function useAppShellSessionWorkspace(toastApi: ToastApi) { const [activeId, setActiveIdState] = useState(); const activeIdRef = useRef(undefined); - const sessionUi = useAppShellSessionUiState(); + const sessionUiController = useAppShellSessionUiState(); const sessionList = useAppShellSessionList(toastApi, { activeIdRef, - liveTurnBySessionRef: sessionUi.liveTurnBySessionRef, - clearTurnTransientStateIfCurrent: sessionUi.clearTurnTransientStateIfCurrent, + liveTurnBySessionRef: sessionUiController.liveTurnBySessionRef, + clearTurnTransientStateIfCurrent: sessionUiController.clearTurnTransientStateIfCurrent, }); const selectionRevisionRef = useRef(0); const bootstrapSelectionLeaseRef = useRef | null>(null); @@ -55,8 +55,6 @@ export function useAppShellSessionWorkspace(toastApi: ToastApi) { ); const transcriptRangeRef = useRef(undefined); const [messageLoadPending, setMessageLoadPending] = useState(false); - const messageRetryPendingRef = useRef>(new Set()); - const stopPendingRef = useRef>(new Set()); const actionsRef = useRef(null); // Every dep below is a ref box, a state setter, or a method of the @@ -69,13 +67,11 @@ export function useAppShellSessionWorkspace(toastApi: ToastApi) { transientMessagesBySessionRef, transcriptRangeRef, selectionRevisionRef, - messageRetryPendingRef, - stopPendingRef, setActiveIdState, setMessagesState: setMessages, setTransientMessagesState: setTransientMessages, setMessageLoadPending, - clearSessionUiState: sessionUi.clearSessionUiState, + clearSessionUiState: sessionUiController.clearSessionUiState, }); const actions = actionsRef.current; @@ -99,21 +95,8 @@ export function useAppShellSessionWorkspace(toastApi: ToastApi) { transcriptRangeRef, messageLoadPending, setMessageLoadPending, - messageRetryPendingRef, - stopPendingRef, - sessionUiController: sessionUi.controller, - liveTurnBySessionRef: sessionUi.liveTurnBySessionRef, - sessionEventHealthBySessionRef: sessionUi.sessionEventHealthBySessionRef, - setMessageLoadErrorBySession: sessionUi.setMessageLoadErrorBySession, - setMessageRetryPendingBySession: sessionUi.setMessageRetryPendingBySession, - setStopPendingBySession: sessionUi.setStopPendingBySession, - setLiveTurnBySession: sessionUi.setLiveTurnBySession, - setShellRunUpdatesBySession: sessionUi.setShellRunUpdatesBySession, - setInteractionBySession: sessionUi.setInteractionBySession, - setMessageQueueBySession: sessionUi.setMessageQueueBySession, - setSessionEventHealthBySession: sessionUi.setSessionEventHealthBySession, - setPendingPermissionModeBySession: sessionUi.setPendingPermissionModeBySession, - setPendingSessionModelBySession: sessionUi.setPendingSessionModelBySession, - confirmLiveTurn: sessionUi.confirmLiveTurn, + // The store's own surface, not a copy of it. Consumers reach setters and + // claims through the controller. + sessionUiController, }; } diff --git a/apps/desktop/src/renderer/use-pending-action-registry.ts b/apps/desktop/src/renderer/use-pending-action-registry.ts deleted file mode 100644 index a16fbee0ed..0000000000 --- a/apps/desktop/src/renderer/use-pending-action-registry.ts +++ /dev/null @@ -1,122 +0,0 @@ -/* - * 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 { useRef, useState } from 'react'; - -type RefBox = { current: T }; - -/** - * Generic keyed-pending registry shared by AppShell's four hand-rolled - * de-dup sets (turn-footer actions, session-row actions, per-session - * permission-mode changes, per-session model changes). Each tracks a Set of - * in-flight keys so a second click while the first request is still settling - * is ignored, then clears the key when the backend confirms. - * - * The four instances differ only along two axes, both opt-in: - * - * - `trackState` — the turn-footer registry drives a React-visible disabled - * mask (the turn presentation derivation reads `keys`), so it mirrors the ref - * into state on every mutation. The three ref-only registries pass their - * `keysRef` straight to their action factories / the bootstrap cleanup - * effect and never re-render, so they skip the state mirror entirely. - * - `autoClearMs` — the turn-footer registry arms a per-key timeout so a - * button re-enables even if the confirming `sessions:changed` event is - * dropped. The others clear synchronously in their action's `finally`. - * - * `keysRef` is always a stable Set instance so the action factories that - * mutate it directly (session-row-actions, session-settings-actions) and the - * unmount cleanup (app-shell-effects) keep operating on the same object. - */ -export interface KeyedPendingRegistry { - /** - * Reactive snapshot of the pending keys. Only refreshed when the registry - * is created with `trackState: true`; otherwise stays the empty seed and - * callers read `keysRef` instead. - */ - keys: Set; - /** Stable mirror of the pending keys for synchronous reads / external mutation. */ - keysRef: RefBox>; - /** Per-key auto-clear timers. Empty unless the registry sets `autoClearMs`. */ - timersRef: RefBox>>; - /** - * Marks `key` pending. Returns false (a no-op) if it was already pending so - * callers can bail on a duplicate action. Bumps the reactive snapshot and - * arms the auto-clear timer when the registry is configured for them. - */ - addKey(key: string): boolean; - /** Clears a single pending key along with its timer and snapshot entry. */ - clearKey(key: string): void; - /** Clears every pending key prefixed with `${sessionId}:` (session teardown). */ - clearForSession(sessionId: string): void; - /** Clears the registry when its owning Runtime Host generation changes. */ - clearAll(): void; -} - -export function useKeyedPendingRegistry(options?: { - trackState?: boolean; - autoClearMs?: number; -}): KeyedPendingRegistry { - const trackState = options?.trackState ?? false; - const autoClearMs = options?.autoClearMs; - const [keys, setKeys] = useState>(() => new Set()); - const keysRef = useRef>(new Set()); - const timersRef = useRef>>(new Map()); - const controllerRef = useRef | null>(null); - - if (!controllerRef.current) { - const syncState = (): void => { - if (trackState) setKeys(new Set(keysRef.current)); - }; - const clearKey = (key: string): void => { - if (!keysRef.current.has(key)) return; - keysRef.current.delete(key); - const timeoutHandle = timersRef.current.get(key); - if (timeoutHandle) clearTimeout(timeoutHandle); - timersRef.current.delete(key); - syncState(); - }; - const addKey = (key: string): boolean => { - if (keysRef.current.has(key)) return false; - keysRef.current.add(key); - syncState(); - if (autoClearMs !== undefined) { - const timeoutHandle = setTimeout(() => clearKey(key), autoClearMs); - timersRef.current.set(key, timeoutHandle); - } - return true; - }; - const clearForSession = (sessionId: string): void => { - const prefix = `${sessionId}:`; - for (const key of Array.from(keysRef.current)) { - if (key.startsWith(prefix)) clearKey(key); - } - }; - const clearAll = (): void => { - for (const timeoutHandle of timersRef.current.values()) { - clearTimeout(timeoutHandle); - } - timersRef.current.clear(); - keysRef.current.clear(); - syncState(); - }; - controllerRef.current = { keysRef, timersRef, addKey, clearKey, clearForSession, clearAll }; - } - - return { keys, ...controllerRef.current }; -} diff --git a/apps/desktop/src/renderer/use-stable-actions.ts b/apps/desktop/src/renderer/use-stable-actions.ts index 57b73d27f6..18a901c654 100644 --- a/apps/desktop/src/renderer/use-stable-actions.ts +++ b/apps/desktop/src/renderer/use-stable-actions.ts @@ -18,7 +18,24 @@ */ import { useLayoutEffect, useRef, useState } from 'react'; -import { createDelegatingActions } from './stable-actions.js'; + +/** + * The facade itself: one method per key, each forwarding to whatever + * `latestRef` holds when it is called. + * + * Factories must return a constant key shape of function values — the key set + * is fixed at creation. + */ +function createDelegatingActions(latestRef: { current: A }): A { + const facade: Record unknown> = {}; + for (const key of Object.keys(latestRef.current)) { + facade[key] = (...args: unknown[]) => { + const action = Reflect.get(latestRef.current, key) as unknown as (...args: unknown[]) => unknown; + return action(...args); + }; + } + return facade as unknown as A; +} /** * Runs an app-shell action factory in the render body but returns a stable @@ -35,6 +52,9 @@ import { createDelegatingActions } from './stable-actions.js'; * * `createAppShellStopAction` is deliberately NOT wrapped: it returns a bare * function (no object to facade) and only feeds JSX props, never effect deps. + * + * The identity guarantee is asserted in `use-stable-actions.test.ts` rather + * than argued for in review — see that file for why this is a contract. */ export function useStableActions(factory: (deps: D) => A, deps: D): A { const actions = factory(deps); diff --git a/apps/desktop/src/renderer/use-turn-action-registry.ts b/apps/desktop/src/renderer/use-turn-action-registry.ts new file mode 100644 index 0000000000..a336323226 --- /dev/null +++ b/apps/desktop/src/renderer/use-turn-action-registry.ts @@ -0,0 +1,106 @@ +/* + * 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 { useRef, useState } from 'react'; + +type RefBox = { current: T }; + +/** + * How long a turn-footer button stays disabled without a confirming + * `sessions:changed` event. Long enough that the normal round trip wins the + * race, short enough that a dropped event does not disable the button forever. + */ +const AUTO_CLEAR_MS = 5000; + +/** + * The in-flight keys behind the turn-footer buttons' disabled mask. + * + * A key is `${sessionId}:${turnId}:${actionId}`, claimed on click and released + * when the Host confirms — so a second click while the first request settles is + * ignored. `keys` drives the mask through the turn presentation derivation; + * `keysRef` is the same content for synchronous reads. + * + * Deliberately concrete. This began as a generic registry with `trackState` and + * `autoClearMs` options, serving these keys plus per-session permission-mode + * and model changes. Those two moved to the session UI store, which already + * held their rendered flag, and the options had no variation left: the timers + * and the state mirror exist for the turn footer alone. + */ +export interface TurnActionRegistry { + /** Reactive snapshot of the pending keys, for the disabled mask. */ + keys: Set; + /** Stable mirror of the pending keys for synchronous reads. */ + keysRef: RefBox>; + /** + * Marks `key` pending and arms its auto-clear timer. Returns false (a no-op) + * if it was already pending, so callers can bail on a duplicate action. + */ + addKey(key: string): boolean; + /** Clears a single pending key along with its timer and snapshot entry. */ + clearKey(key: string): void; + /** Clears every pending key prefixed with `${sessionId}:` (session teardown). */ + clearForSession(sessionId: string): void; + /** Clears every key and timer (unmount, or a Runtime Host generation change). */ + clearAll(): void; +} + +export function useTurnActionRegistry(): TurnActionRegistry { + const [keys, setKeys] = useState>(() => new Set()); + const keysRef = useRef>(new Set()); + const timersRef = useRef>>(new Map()); + const controllerRef = useRef | null>(null); + + if (!controllerRef.current) { + const syncState = (): void => { + setKeys(new Set(keysRef.current)); + }; + const clearKey = (key: string): void => { + if (!keysRef.current.has(key)) return; + keysRef.current.delete(key); + const timeoutHandle = timersRef.current.get(key); + if (timeoutHandle) clearTimeout(timeoutHandle); + timersRef.current.delete(key); + syncState(); + }; + const addKey = (key: string): boolean => { + if (keysRef.current.has(key)) return false; + keysRef.current.add(key); + syncState(); + timersRef.current.set(key, setTimeout(() => clearKey(key), AUTO_CLEAR_MS)); + return true; + }; + const clearForSession = (sessionId: string): void => { + const prefix = `${sessionId}:`; + for (const key of Array.from(keysRef.current)) { + if (key.startsWith(prefix)) clearKey(key); + } + }; + const clearAll = (): void => { + for (const timeoutHandle of timersRef.current.values()) { + clearTimeout(timeoutHandle); + } + timersRef.current.clear(); + keysRef.current.clear(); + syncState(); + }; + controllerRef.current = { keysRef, addKey, clearKey, clearForSession, clearAll }; + } + + return { keys, ...controllerRef.current }; +}