diff --git a/dsh-plugin-desktop/src/client/apply-guard.ts b/dsh-plugin-desktop/src/client/apply-guard.ts deleted file mode 100644 index 1963749834..0000000000 --- a/dsh-plugin-desktop/src/client/apply-guard.ts +++ /dev/null @@ -1,42 +0,0 @@ -import type { DesktopClientEnvironment } from './environment.ts' - -/** - * Whether a previous application of this entry still owns the page. - * - * The mode markers are claimed synchronously at `apply()` entry and cleared on - * failure, so any value here means another fiber's shell (advanced, extended, - * or framed) is alive. Re-application while it stands would stack a second - * frame over the live one, so the guard degrades to a no-op instead — the - * loader may restart this entry freely and the stale fiber keeps presenting - * until its own cleanup runs. - */ -export function isDesktopShellAlreadyApplied(doc: Document): boolean { - return doc.body.dataset.dshDesktopMode !== undefined -} - -/** - * Claim the mode markers synchronously for one entry application. - * - * Plain writes on purpose: effect scheduling is asynchronous, which would - * leave a re-entry window between two `apply()` calls. Every shell effect - * re-sets these markers inside its own lifetime and owns their cleanup; this - * early claim only closes the concurrency window. - */ -export function claimDesktopEntryMarkers(doc: Document, environment: DesktopClientEnvironment): void { - doc.body.dataset.dshDesktopMode = environment.mode - doc.body.dataset.dshDesktopPlatform = environment.platform - doc.body.dataset.dshDesktopMaterial = environment.material -} - -/** - * Roll the claimed markers back after a failed startup. - * - * Any step of the registration walk can throw; a marker left behind then - * would make `isDesktopShellAlreadyApplied` swallow every loader restart of - * this entry until a full page reload (#517). - */ -export function clearDesktopEntryMarkers(doc: Document): void { - delete doc.body.dataset.dshDesktopMode - delete doc.body.dataset.dshDesktopPlatform - delete doc.body.dataset.dshDesktopMaterial -} diff --git a/dsh-plugin-desktop/src/client/index.ts b/dsh-plugin-desktop/src/client/index.ts index f21715489f..25f6a12ba0 100644 --- a/dsh-plugin-desktop/src/client/index.ts +++ b/dsh-plugin-desktop/src/client/index.ts @@ -9,11 +9,6 @@ import type {} from '@deepseek-ai/dsh-client-ui-settings/client' import type {} from '@deepseek-ai/dsh-client-ui-theme/client' import type {} from '@deepseek-ai/dsh-client-ui-renderer/client' import { applyAdvancedShell } from './advanced-shell.ts' -import { - claimDesktopEntryMarkers, - clearDesktopEntryMarkers, - isDesktopShellAlreadyApplied, -} from './apply-guard.ts' import { startRendererBootReporter } from './boot-health.ts' import { applyDesktopSettings } from './desktop-settings.ts' import { installDesktopDirectoryPickerBridge } from './directory-picker.ts' @@ -84,49 +79,28 @@ export const inject = [ 'uiRenderer', ] -/** - * Register desktop-owned client surfaces for the current BrowserWindow mode. - * - * The mode markers are claimed synchronously before any effect walks, so a - * concurrent application observes the guard immediately; every registration - * step runs inside one try block and any failure clears the markers again — - * a stale marker would otherwise make the idempotency guard swallow every - * loader restart of this entry until the page is fully reloaded (#517). - * @param ctx - browser Cordis context. - */ +/** Register desktop-owned client surfaces for the current BrowserWindow mode. @param ctx - browser Cordis context. */ export function apply(ctx: ClientContext): void { const environment = parseDesktopClientEnvironment(window.location.search) if (!environment) return - if (isDesktopShellAlreadyApplied(document)) { - console.warn(`dsh-plugin-desktop: shell already applied for mode ${document.body.dataset.dshDesktopMode}; skipping re-application`) - return + ctx.effect( + () => provideDesktopWindow(ctx, desktopWindowService(environment)), + 'dsh-plugin-desktop: native window geometry service', + ) + const desktopSettings = applyDesktopSettings(ctx, environment) + ctx.effect( + () => startRendererBootReporter(ctx.loader), + 'dsh-plugin-desktop: renderer boot health report', + ) + if (environment.platform === 'win32') { + ctx.effect( + () => installDesktopDirectoryPickerBridge(), + 'dsh-plugin-desktop: native directory picker bridge', + ) } - // Synchronous plain writes: effect scheduling is asynchronous, which would - // leave a re-entry window between apply() calls. - claimDesktopEntryMarkers(document, environment) - try { - ctx.effect( - () => provideDesktopWindow(ctx, desktopWindowService(environment)), - 'dsh-plugin-desktop: native window geometry service', - ) - const desktopSettings = applyDesktopSettings(ctx, environment) - ctx.effect( - () => startRendererBootReporter(ctx.loader), - 'dsh-plugin-desktop: renderer boot health report', - ) - if (environment.platform === 'win32') { - ctx.effect( - () => installDesktopDirectoryPickerBridge(), - 'dsh-plugin-desktop: native directory picker bridge', - ) - } - if (environment.mode === 'advanced') applyAdvancedShell(ctx, environment) - if (environment.mode === 'extended') applyExtendedShell(ctx, environment, desktopSettings) - if (environment.platform !== 'linux' && environment.mode === 'compatibility') { - applyFramedShell(ctx, environment, desktopSettings) - } - } catch (cause) { - clearDesktopEntryMarkers(document) - throw cause + if (environment.mode === 'advanced') applyAdvancedShell(ctx, environment) + if (environment.mode === 'extended') applyExtendedShell(ctx, environment, desktopSettings) + if (environment.platform !== 'linux' && environment.mode === 'compatibility') { + applyFramedShell(ctx, environment, desktopSettings) } } diff --git a/dsh-plugin-desktop/src/client/layout-service.ts b/dsh-plugin-desktop/src/client/layout-service.ts index 0fd81649ec..b58f05d798 100644 --- a/dsh-plugin-desktop/src/client/layout-service.ts +++ b/dsh-plugin-desktop/src/client/layout-service.ts @@ -13,23 +13,23 @@ import type { DesktopLayoutState } from './layout-state.ts' * degrades to `false`: the caller keeps mode markers but leaves presentation * upstream. * - * Ownership is detected through the `provide` failure itself — reading the - * store (`reflect.get`) would create a cross-fiber trace dependency during - * application, which deadlocks the page. + * Ownership is detected through the effect's synchronous `provide` failure. + * Keeping the registration inside the owning effect preserves Cordis unload + * and re-apply semantics without a separate process-global marker. * @param ctx - active browser Cordis context. * @param layout - desktop-owned layout implementation. * @returns whether this fiber now owns the service. */ export function claimDesktopLayout(ctx: ClientContext, layout: DesktopLayoutState): boolean { try { - const dispose = ctx.reflect.provide('layout', layout) - // Cordis uses the factory result as the uninstall disposer, so the factory - // must return it — a void-returning factory fails the SyncEffect overload. - ctx.effect(() => () => { void dispose() }, 'desktop: layout service') + ctx.effect(() => { + const dispose = ctx.reflect.provide('layout', layout) + return () => { void dispose() } + }, 'desktop: layout service') return true } catch (cause) { - if (!(cause instanceof Error) || !cause.message.includes('has been registered')) throw cause - console.warn('dsh-plugin-desktop: layout service owned by upstream dsh-client-ui-layout; deferring to it') + if (!(cause instanceof Error) || !cause.message.includes('service "layout" has been registered')) throw cause + console.warn('dsh-plugin-desktop: layout service already owned; deferring desktop presentation') return false } } diff --git a/dsh-plugin-desktop/tests/client-layout-service.spec.ts b/dsh-plugin-desktop/tests/client-layout-service.spec.ts index e049bd0196..23cc33e800 100644 --- a/dsh-plugin-desktop/tests/client-layout-service.spec.ts +++ b/dsh-plugin-desktop/tests/client-layout-service.spec.ts @@ -1,9 +1,4 @@ import { afterEach, describe, expect, it, vi } from 'vitest' -import { - claimDesktopEntryMarkers, - clearDesktopEntryMarkers, - isDesktopShellAlreadyApplied, -} from '../src/client/apply-guard.ts' import { claimDesktopLayout } from '../src/client/layout-service.ts' import { applyAdvancedShell } from '../src/client/advanced-shell.ts' import { applyExtendedShell } from '../src/client/extended-shell.ts' @@ -69,50 +64,8 @@ function makeCtx() { } } -/** Extract the first registered effect factory under `noUncheckedIndexedAccess`. */ -function firstEffect(effect: ReturnType): () => () => void { - const call = effect.mock.calls[0]?.[0] - if (typeof call !== 'function') throw new Error('expected one registered effect factory') - return call as () => () => void -} - afterEach(() => { vi.unstubAllGlobals() }) -describe('entry marker guard', () => { - it('treats any live mode marker as an already-applied signal', () => { - const doc = () => ({ body: { dataset: { dshDesktopMode: 'advanced' } } }) as unknown as Document - expect(isDesktopShellAlreadyApplied(doc())).toBe(true) - expect(isDesktopShellAlreadyApplied({ body: { dataset: { dshDesktopMode: 'extended' } } } as unknown as Document)).toBe(true) - expect(isDesktopShellAlreadyApplied({ body: { dataset: { dshDesktopMode: 'compatibility' } } } as unknown as Document)).toBe(true) - expect(isDesktopShellAlreadyApplied({ body: { dataset: {} } } as unknown as Document)).toBe(false) - }) - - it('rolls claimed markers back when a startup step fails midway', () => { - const { dataset } = stubDocument() - const environment = { mode: 'advanced', platform: 'win32', material: 'off', micaSupported: false, version: '2.0.2' } - - claimDesktopEntryMarkers(document, environment as never) - - // The synchronous claim is what closes the re-entry window between two - // apply() calls — it must be visible before any registration step runs. - expect(dataset.dshDesktopMode).toBe('advanced') - expect(dataset.dshDesktopPlatform).toBe('win32') - - try { - throw new Error('startup failed') - } catch (cause) { - clearDesktopEntryMarkers(document) - expect(cause).toBeInstanceOf(Error) - } - - // A stale marker would make the guard swallow every loader restart of - // this entry until a full page reload — they must go. - expect(dataset.dshDesktopMode).toBeUndefined() - expect(dataset.dshDesktopPlatform).toBeUndefined() - expect(dataset.dshDesktopMaterial).toBeUndefined() - }) -}) - describe('claimDesktopLayout', () => { it('wins ownership when nothing else registered the service', () => { const ctx = makeCtx() @@ -127,8 +80,9 @@ describe('claimDesktopLayout', () => { // the registration for whoever applies next; the factory result is what // cordis registers for uninstall. expect(ctx.effect).toHaveBeenCalledWith(expect.any(Function), 'desktop: layout service') - const disposer = firstEffect(ctx.effect)() - disposer() + const disposer = ctx.effect.mock.results[0]?.value + expect(typeof disposer).toBe('function') + ;(disposer as () => void)() expect(dispose).toHaveBeenCalled() }) @@ -140,8 +94,10 @@ describe('claimDesktopLayout', () => { throw new Error('service "layout" has been registered at ') }) expect(claimDesktopLayout(ctx as never, {} as never)).toBe(false) - // No disposal effect may be registered for an ownership we never took. - expect(ctx.effect).not.toHaveBeenCalled() + // Cordis starts the effect synchronously, but the failed factory never + // produces a disposer that could be retained by the fiber. + expect(ctx.effect).toHaveBeenCalledOnce() + expect(ctx.effect.mock.results[0]?.type).toBe('throw') expect(warn).toHaveBeenCalled() } finally { warn.mockRestore() @@ -195,9 +151,12 @@ describe('applyAdvancedShell presentation ownership', () => { // none of the desktop-owned chrome styles — just the mode markers, // whose cleanup still runs through their own fiber effect. expect(ctx.slots.register).not.toHaveBeenCalled() - expect(ctx.effect).toHaveBeenCalledTimes(1) - const cleanup = firstEffect(ctx.effect)() - cleanup() + // Failed ownership attempt plus the surviving marker effect. + expect(ctx.effect).toHaveBeenCalledTimes(2) + expect(ctx.effect.mock.results[0]?.type).toBe('throw') + const cleanup = ctx.effect.mock.results[1]?.value + expect(typeof cleanup).toBe('function') + ;(cleanup as () => void)() expect(dataset.dshDesktopMode).toBeUndefined() expect(dataset.dshDesktopPlatform).toBeUndefined() expect(warn).toHaveBeenCalled() @@ -237,10 +196,10 @@ describe('applyExtendedShell presentation ownership', () => { applyExtendedShell(ctx as never, environmentFor('extended') as never) - // Only the framed-chrome style effect remains; the titlebar overlay is - // injected (it layers over whatever presents the root), while no second - // root frame is stacked over the upstream one. - expect(ctx.effect).toHaveBeenCalledTimes(1) + // The failed ownership attempt is followed only by the framed-chrome + // style effect; no second root frame is stacked over the existing one. + expect(ctx.effect).toHaveBeenCalledTimes(2) + expect(ctx.effect.mock.results[0]?.type).toBe('throw') expect(ctx.slots.register).not.toHaveBeenCalled() expect(ctx.slots.inject).toHaveBeenCalledTimes(1) expect(warn).toHaveBeenCalled()