diff --git a/src/renderer/src/lib/ensure-hooks-confirmed.ts b/src/renderer/src/lib/ensure-hooks-confirmed.ts index 8128e9ae8a0..181be47e34d 100644 --- a/src/renderer/src/lib/ensure-hooks-confirmed.ts +++ b/src/renderer/src/lib/ensure-hooks-confirmed.ts @@ -8,6 +8,7 @@ import { type IssueCommandReadResult } from '@/runtime/runtime-hooks-client' import { getRuntimeEnvironmentIdForRepo } from './repo-runtime-owner' +import { MODAL_DISMISSED_KEY } from '@/store/slices/modal-slot-dismissal' import { getRepoExecutionHostId, parseExecutionHostId, @@ -135,7 +136,10 @@ async function confirmScriptContent( scriptContent, contentHash, previouslyApproved, - onResolve: (decision: 'run' | 'skip') => resolve(decision) + onResolve: (decision: 'run' | 'skip') => resolve(decision), + // Why: the modal slot holds one entry; losing it must not strand the + // caller awaiting this decision. Dismissal is the same as declining. + [MODAL_DISMISSED_KEY]: () => resolve('skip') }) }) } diff --git a/src/renderer/src/lib/orca-yaml-trust-prompt-slot-eviction.test.ts b/src/renderer/src/lib/orca-yaml-trust-prompt-slot-eviction.test.ts new file mode 100644 index 00000000000..a910bf67d72 --- /dev/null +++ b/src/renderer/src/lib/orca-yaml-trust-prompt-slot-eviction.test.ts @@ -0,0 +1,110 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { StoreApi } from 'zustand/vanilla' +import type { AppState } from '@/store/types' +import { createUIStore } from '@/store/slices/ui-slice-test-harness' +import { __resetTrustPromptChainForTests, ensureHooksConfirmed } from './ensure-hooks-confirmed' +import { + createCompatibleRuntimeStatusResponseIfNeeded, + type RuntimeEnvironmentCallRequest +} from '@/runtime/runtime-compatibility-test-fixture' +import { clearRuntimeCompatibilityCacheForTests } from '@/runtime/runtime-rpc-client' + +const hooksCheckMock = vi.fn() +const readIssueCommandMock = vi.fn() +const runtimeEnvironmentTransportCallMock = vi.fn() + +// Why: the real UI slice owns the single modal slot, so the eviction under test +// has to come from it — a hand-rolled openModal double would decide the outcome. +function createStateBackedByRealModalSlot(): { + store: StoreApi + state: AppState +} { + const store = createUIStore() + store.setState({ + repos: [{ id: 'repo-1', displayName: 'Repo One' }], + trustedOrcaHooks: {} + } as unknown as Partial) + return { store, state: store.getState() } +} + +async function settleOrReport(promise: Promise): Promise { + return Promise.race([ + promise, + new Promise<'never-settled'>((resolve) => setTimeout(() => resolve('never-settled'), 100)) + ]) +} + +describe('orca.yaml trust prompt evicted from the modal slot', () => { + beforeEach(() => { + hooksCheckMock.mockReset() + readIssueCommandMock.mockReset() + runtimeEnvironmentTransportCallMock.mockReset() + runtimeEnvironmentTransportCallMock.mockImplementation((args: RuntimeEnvironmentCallRequest) => + createCompatibleRuntimeStatusResponseIfNeeded(args) + ) + clearRuntimeCompatibilityCacheForTests() + vi.stubGlobal('window', { + api: { + hooks: { check: hooksCheckMock, readIssueCommand: readIssueCommandMock }, + runtimeEnvironments: { call: runtimeEnvironmentTransportCallMock } + } + }) + __resetTrustPromptChainForTests() + hooksCheckMock.mockResolvedValue({ + hasHooks: true, + hooks: { scripts: { setup: 'pnpm install' } }, + mayNeedUpdate: false + }) + }) + + it('resolves the pending decision as skip when another modal takes the slot', async () => { + const { store, state } = createStateBackedByRealModalSlot() + + const decision = ensureHooksConfirmed(state, 'repo-1', 'setup') + await vi.waitFor(() => expect(store.getState().activeModal).toBe('confirm-orca-yaml-hooks')) + + store.getState().openModal('worktree-palette') + + await expect(settleOrReport(decision)).resolves.toBe('skip') + }) + + it('resolves the pending decision as skip when the slot is closed outright', async () => { + const { store, state } = createStateBackedByRealModalSlot() + + const decision = ensureHooksConfirmed(state, 'repo-1', 'setup') + await vi.waitFor(() => expect(store.getState().activeModal).toBe('confirm-orca-yaml-hooks')) + + store.getState().closeModal() + + await expect(settleOrReport(decision)).resolves.toBe('skip') + }) + + it('keeps later trust prompts working after one is evicted', async () => { + const { store, state } = createStateBackedByRealModalSlot() + + const evicted = ensureHooksConfirmed(state, 'repo-1', 'setup') + await vi.waitFor(() => expect(store.getState().activeModal).toBe('confirm-orca-yaml-hooks')) + store.getState().openModal('worktree-palette') + await settleOrReport(evicted) + + const next = ensureHooksConfirmed(state, 'repo-1', 'setup') + await vi.waitFor(() => expect(store.getState().activeModal).toBe('confirm-orca-yaml-hooks')) + const onResolve = store.getState().modalData.onResolve as (d: 'run' | 'skip') => void + onResolve('run') + + await expect(settleOrReport(next)).resolves.toBe('run') + }) + + it('leaves the answered decision alone when the dialog closes the slot itself', async () => { + const { store, state } = createStateBackedByRealModalSlot() + + const decision = ensureHooksConfirmed(state, 'repo-1', 'setup') + await vi.waitFor(() => expect(store.getState().activeModal).toBe('confirm-orca-yaml-hooks')) + + // Mirrors OrcaYamlTrustDialog: resolve first, then vacate the slot. + ;(store.getState().modalData.onResolve as (d: 'run' | 'skip') => void)('run') + store.getState().closeModal() + + await expect(settleOrReport(decision)).resolves.toBe('run') + }) +}) diff --git a/src/renderer/src/store/slices/modal-slot-dismissal.ts b/src/renderer/src/store/slices/modal-slot-dismissal.ts new file mode 100644 index 00000000000..540f963cc80 --- /dev/null +++ b/src/renderer/src/store/slices/modal-slot-dismissal.ts @@ -0,0 +1,13 @@ +/** + * The app has one modal slot, so opening a modal evicts whatever held it. A modal + * that owns an unsettled promise (a trust prompt awaiting a decision) hands over + * this callback so eviction settles it instead of stranding the awaiting caller. + */ +export const MODAL_DISMISSED_KEY = 'onModalDismissed' + +export function settleEvictedModalData(evicted: Record): void { + const onDismissed = evicted[MODAL_DISMISSED_KEY] + if (typeof onDismissed === 'function') { + ;(onDismissed as () => void)() + } +} diff --git a/src/renderer/src/store/slices/ui.ts b/src/renderer/src/store/slices/ui.ts index e89e990719d..e06734e4923 100644 --- a/src/renderer/src/store/slices/ui.ts +++ b/src/renderer/src/store/slices/ui.ts @@ -2,6 +2,7 @@ import type { StateCreator } from 'zustand' import type { AppState } from '../types' import { normalizeRightSidebarRoute } from '../right-sidebar-route' +import { settleEvictedModalData } from './modal-slot-dismissal' import { findPrevLiveNonTaskStackHistoryIndex, findPrevLiveWorktreeHistoryIndex @@ -1621,12 +1622,18 @@ export const createUISlice: StateCreator = (set, get) if (modal === 'add-repo' || modal === 'create-worktree') { get().recordFeatureInteraction?.('workspace-creation') } + const evicted = get().modalData set({ activeModal: modal, modalData: data }) + settleEvictedModalData(evicted) + }, + closeModal: () => { + const evicted = get().modalData + set({ activeModal: 'none', modalData: {} }) + settleEvictedModalData(evicted) }, - closeModal: () => set({ activeModal: 'none', modalData: {} }), featureTipsSeenIds: [], markFeatureTipsSeen: (ids) => set((s) => {