fix(desktop): keep layout fallback lifecycle-safe

dsh-plugin-desktop/src/client/apply-guard.ts

@@ -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
-}

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)
   }
 }

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
   }
 }

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<typeof vi.fn>): () => () => 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 <z5>')
       })
       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()