mirror of
https://github.com/stablyai/orca.git
synced 2026-10-23 08:03:39 +00:00
fix(workspaces): settle an orca.yaml trust prompt when the modal slot is taken
The app has one modal slot, so any openModal/closeModal evicts whatever held it. A pending orca.yaml trust prompt owns the promise that quick create awaits, and eviction dropped its resolver: that submit never settled, and because the trust prompts are serialized on a module-global chain, every later create/remove in the session silently did nothing. Modal data can now carry an onModalDismissed callback that the slot invokes when it evicts an entry; the trust prompt uses it to resolve as 'skip', which is what dismissing the dialog already means.
This commit is contained in:
@@ -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')
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
@@ -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<AppState>
|
||||
state: AppState
|
||||
} {
|
||||
const store = createUIStore()
|
||||
store.setState({
|
||||
repos: [{ id: 'repo-1', displayName: 'Repo One' }],
|
||||
trustedOrcaHooks: {}
|
||||
} as unknown as Partial<AppState>)
|
||||
return { store, state: store.getState() }
|
||||
}
|
||||
|
||||
async function settleOrReport<T>(promise: Promise<T>): Promise<T | 'never-settled'> {
|
||||
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')
|
||||
})
|
||||
})
|
||||
@@ -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<string, unknown>): void {
|
||||
const onDismissed = evicted[MODAL_DISMISSED_KEY]
|
||||
if (typeof onDismissed === 'function') {
|
||||
;(onDismissed as () => void)()
|
||||
}
|
||||
}
|
||||
@@ -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<AppState, [], [], UISlice> = (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) => {
|
||||
|
||||
Reference in New Issue
Block a user