diff --git a/packages/client/ui-slots/src/deferred.ts b/packages/client/ui-slots/src/deferred.ts index 9b68c4d206..a5233812e6 100644 --- a/packages/client/ui-slots/src/deferred.ts +++ b/packages/client/ui-slots/src/deferred.ts @@ -36,6 +36,12 @@ export interface DeferredRegistration { * @param name - target slot name. * @param component - the component whose ledger presence marks "registered". * @param register - performs the actual registration; returns its disposer. + * @param onFailure - owns a registration failure that fires from a LATER + * ledger flush (a declaration landing after two providers deferred, say): + * the deferral first removes its own subscription, then hands the error + * over instead of throwing through the flush — the callback's chance to + * roll back sibling deferrals and surface the conflict on a loud channel. + * Absent, a late failure rethrows out of the flush. * @returns the deferral handle (dispose in the owning effect's disposer). * @throws the immediate registration's failure, after removing the * just-installed subscription — a throwing construction leaves nothing live. @@ -45,6 +51,7 @@ export function deferRegistration( name: string, component: unknown, register: () => () => void, + onFailure?: (error: unknown) => void, ): DeferredRegistration { let dispose: (() => void) | undefined const tryRegister = (): void => { @@ -52,7 +59,15 @@ export function deferRegistration( if (registry.entries(name).some(e => e.component === component)) return dispose = register() } - const unsubscribe = registry.subscribe(name, () => { tryRegister() }) + const unsubscribe = registry.subscribe(name, () => { + try { + tryRegister() + } catch (error) { + unsubscribe() + if (onFailure === undefined) throw error + onFailure(error) + } + }) try { tryRegister() } catch (error) { diff --git a/packages/client/ui-slots/tests/deferred.spec.ts b/packages/client/ui-slots/tests/deferred.spec.ts index 2153e52181..ba4d9b91ee 100644 --- a/packages/client/ui-slots/tests/deferred.spec.ts +++ b/packages/client/ui-slots/tests/deferred.spec.ts @@ -24,6 +24,27 @@ describe('deferRegistration', () => { expect(core.entries(HOLE)).toHaveLength(0) }) + it('hands a late registration failure to onFailure after unsubscribing itself', async () => { + const core = new SlotCore() + const component = (): null => null + const foreign = (): null => null + const failures: unknown[] = [] + // Nothing is declared yet: the deferral just subscribes and waits. + const register = vi.fn(() => core.register({ name: HOLE } as never, component as never)) + deferRegistration(core, HOLE, component, register, (error) => { failures.push(error) }) + // The declaration lands with a foreign occupant racing in first: the + // deferral's flush-time attempt fails, unsubscribes itself, and reports + // through onFailure instead of throwing out of the flush. + core.register({ name: 'root', children: { [HOLE]: { kind: 'single', scope: 'root' } } } as never, (() => null) as never) + const disposeForeign = core.register({ name: HOLE } as never, foreign as never) + await Promise.resolve() + expect(failures.map(String).join('')).toContain('already has a registration') + // Unsubscribed: freeing the hole must not resurrect the loser. + disposeForeign() + await Promise.resolve() + expect(core.entries(HOLE)).toHaveLength(0) + }) + it('drops its subscription when the immediate registration throws', async () => { const core = declared() const foreign = (): null => null diff --git a/packages/host/directory-picker-native/src/client/index.ts b/packages/host/directory-picker-native/src/client/index.ts index f58bcbc056..aa2936e1da 100644 --- a/packages/host/directory-picker-native/src/client/index.ts +++ b/packages/host/directory-picker-native/src/client/index.ts @@ -29,13 +29,21 @@ export function apply(ctx: ClientContext): void { ctx.effect(() => { // Constructing the pair can throw halfway (a declared hole already // occupied registers synchronously): roll the earlier deferral back so - // no live subscription outlives the failed fiber. + // no live subscription outlives the failed fiber. A conflict surfacing + // from a LATER ledger flush (holes declared after two flow providers + // activated) rolls the whole pair back the same way and re-raises on + // the global channel the boot's fail-loud handler owns — never a throw + // through the slot flush, never partial occupancy from this package. const deferred: ReturnType[] = [] + const lateConflict = (error: unknown): void => { + for (const entry of deferred) entry.dispose() + queueMicrotask(() => { throw error instanceof Error ? error : new Error(String(error)) }) + } try { deferred.push(deferRegistration(ctx.slots, 'conversation.hero.workspace.directoryFlow', NativeDirectoryFlow, () => - ctx.slots.register({ name: 'conversation.hero.workspace.directoryFlow', inject: injected }, NativeDirectoryFlow))) + ctx.slots.register({ name: 'conversation.hero.workspace.directoryFlow', inject: injected }, NativeDirectoryFlow), lateConflict)) deferred.push(deferRegistration(ctx.slots, 'sidebar.workspaces.directoryFlow', NativeDirectoryFlow, () => - ctx.slots.register({ name: 'sidebar.workspaces.directoryFlow', inject: injected }, NativeDirectoryFlow))) + ctx.slots.register({ name: 'sidebar.workspaces.directoryFlow', inject: injected }, NativeDirectoryFlow), lateConflict)) } catch (error) { for (const entry of deferred) entry.dispose() throw error diff --git a/packages/host/directory-picker-native/tests/client-flow.spec.tsx b/packages/host/directory-picker-native/tests/client-flow.spec.tsx index a91732fab4..ecddf84eb4 100644 --- a/packages/host/directory-picker-native/tests/client-flow.spec.tsx +++ b/packages/host/directory-picker-native/tests/client-flow.spec.tsx @@ -56,6 +56,47 @@ describe('directory-picker-native client half', () => { for (const hole of HOLES) expect(after.slots.entries(hole)).toHaveLength(1) }) + it('rolls back wholesale and reports loudly when a rival provider wins after deferred activation', async () => { + const b = await bench() + const rejections: unknown[] = [] + const onUnhandled = (reason: unknown): void => { rejections.push(reason) } + // queueMicrotask throws surface as uncaughtException, not a rejection. + process.on('unhandledRejection', onUnhandled) + process.on('uncaughtException', onUnhandled) + try { + // This provider activates BEFORE any hole exists: both deferrals wait. + // (Duplicate rows of the SAME package converge silently — the deferral + // skips a hole its own component already occupies; the conflict needs + // a rival provider.) + await b.ctx.plugin({ inject: [...inject], apply }).await() + b.declare() + // A rival occupies both holes ahead of the pending microtask flush. + b.slots.register({ name: HOLES[0] } as never, () => null) + b.slots.register({ name: HOLES[1] } as never, () => null) + await new Promise(resolve => setTimeout(resolve, 20)) + // The rival keeps both holes; this provider rolled back wholesale and + // surfaced the conflict on the fail-loud channel — no partial mix. + for (const hole of HOLES) expect(b.slots.entries(hole)).toHaveLength(1) + expect(rejections.map(String).join('\n')).toContain('already has a registration') + + // Non-Error conflicts wrap before the loud rethrow (same channel). + const c = await bench() + await c.ctx.plugin({ inject: [...inject], apply }).await() + const original = c.slots.register.bind(c.slots) + const slotsAny = c.slots as { register: typeof original } + slotsAny.register = ((options: never, component: never) => { + if ((options as { name?: string }).name === HOLES[0]) throw 'string conflict' + return original(options, component) + }) as typeof original + c.declare() + await new Promise(resolve => setTimeout(resolve, 20)) + expect(rejections.map(String).join('\n')).toContain('string conflict') + } finally { + process.off('unhandledRejection', onUnhandled) + process.off('uncaughtException', onUnhandled) + } + }) + it('rolls back the first deferral when the second hole is already occupied', async () => { const b = await bench() b.declare()