fix(workspaces): make trust prompt settlement one-shot

This commit is contained in:
Brennan Benson
2026-08-19 16:40:49 -07:00
parent a322eef916
commit 64d94b180a
4 changed files with 119 additions and 13 deletions
+11 -4
View File
@@ -129,6 +129,14 @@ async function confirmScriptContent(
const previouslyApproved = Boolean(existingHash)
return new Promise<'run' | 'skip'>((resolve) => {
let settled = false
const settle = (decision: 'run' | 'skip'): void => {
if (settled) {
return
}
settled = true
resolve(decision)
}
state.openModal('confirm-orca-yaml-hooks', {
repoId,
repoName,
@@ -136,10 +144,9 @@ async function confirmScriptContent(
scriptContent,
contentHash,
previouslyApproved,
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')
onResolve: settle,
// Why: eviction must fail closed without stranding the singleton trust queue.
[MODAL_DISMISSED_KEY]: () => settle('skip')
})
})
}
@@ -13,8 +13,7 @@ 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.
// Why: a hand-rolled openModal double would decide the eviction outcome under test.
function createStateBackedByRealModalSlot(): {
store: StoreApi<AppState>
state: AppState
@@ -59,13 +58,21 @@ describe('orca.yaml trust prompt evicted from the modal slot', () => {
it('resolves the pending decision as skip when another modal takes the slot', async () => {
const { store, state } = createStateBackedByRealModalSlot()
const runSetup = vi.fn()
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')
const result = await settleOrReport(decision)
if (result === 'run') {
runSetup()
}
expect(result).toBe('skip')
expect(runSetup).not.toHaveBeenCalled()
expect(store.getState().trustedOrcaHooks).toEqual({})
})
it('resolves the pending decision as skip when the slot is closed outright', async () => {
@@ -101,7 +108,7 @@ describe('orca.yaml trust prompt evicted from the modal slot', () => {
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.
// Mirrors the trust dialog: resolve first, then vacate the slot.
;(store.getState().modalData.onResolve as (d: 'run' | 'skip') => void)('run')
store.getState().closeModal()
@@ -1,8 +1,4 @@
/**
* 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.
*/
// Modal owners with pending work use this hook to settle when the singleton slot evicts them.
export const MODAL_DISMISSED_KEY = 'onModalDismissed'
export function settleEvictedModalData(evicted: Record<string, unknown>): void {
@@ -0,0 +1,96 @@
import { describe, expect, it, vi } from 'vitest'
import { createUIStore } from './ui-slice-test-harness'
import { MODAL_DISMISSED_KEY } from './modal-slot-dismissal'
function dismissalData(onDismissed: () => void): Record<string, unknown> {
return { [MODAL_DISMISSED_KEY]: onDismissed }
}
describe('UI modal slot dismissal', () => {
it('dismisses replaced and closed entries once', () => {
const store = createUIStore()
const firstDismissed = vi.fn()
const secondDismissed = vi.fn()
store.getState().openModal('worktree-palette', dismissalData(firstDismissed))
store.getState().openModal('quick-open', dismissalData(secondDismissed))
store.getState().closeModal()
store.getState().closeModal()
expect(firstDismissed).toHaveBeenCalledOnce()
expect(secondDismissed).toHaveBeenCalledOnce()
})
it('dismisses each entry across consecutive replacements', () => {
const store = createUIStore()
const firstDismissed = vi.fn()
const secondDismissed = vi.fn()
const thirdDismissed = vi.fn()
store.getState().openModal('worktree-palette', dismissalData(firstDismissed))
store.getState().openModal('quick-open', dismissalData(secondDismissed))
store.getState().openModal('add-repo', dismissalData(thirdDismissed))
store.getState().closeModal()
expect(firstDismissed).toHaveBeenCalledOnce()
expect(secondDismissed).toHaveBeenCalledOnce()
expect(thirdDismissed).toHaveBeenCalledOnce()
})
it('updates the slot before notifying the evicted entry', () => {
const store = createUIStore()
const observedModal = vi.fn(() => store.getState().activeModal)
store.getState().openModal('worktree-palette', dismissalData(observedModal))
store.getState().openModal('quick-open')
expect(observedModal).toHaveReturnedWith('quick-open')
})
it('ignores modal data without a callable dismissal hook', () => {
const store = createUIStore()
store.getState().openModal('worktree-palette', { [MODAL_DISMISSED_KEY]: 'invalid' })
expect(() => store.getState().openModal('quick-open')).not.toThrow()
expect(store.getState().activeModal).toBe('quick-open')
})
it('settles reentrant replacement and close paths once', () => {
const store = createUIStore()
const replacementDismissed = vi.fn()
const reentrantDismissed = vi.fn()
const firstDismissed = vi.fn(() => {
store.getState().openModal('quick-open', dismissalData(reentrantDismissed))
})
store.getState().openModal('worktree-palette', dismissalData(firstDismissed))
store.getState().openModal('add-repo', dismissalData(replacementDismissed))
store.getState().closeModal()
expect(firstDismissed).toHaveBeenCalledOnce()
expect(replacementDismissed).toHaveBeenCalledOnce()
expect(reentrantDismissed).toHaveBeenCalledOnce()
expect(store.getState().activeModal).toBe('none')
})
it('performs one bounded notification per transition under repeated replacement', () => {
const store = createUIStore()
const dismissed = new Set<number>()
const transitionCount = 1_000
for (let id = 0; id < transitionCount; id += 1) {
store.getState().openModal(
'worktree-palette',
dismissalData(() => {
expect(dismissed.has(id)).toBe(false)
dismissed.add(id)
})
)
}
store.getState().closeModal()
expect(dismissed.size).toBe(transitionCount)
expect(store.getState().modalData).toEqual({})
})
})