fix(desktop): exit fullscreen before hiding window on macOS close (#1249)

* fix(desktop): exit fullscreen before hiding window on macOS close (#1215)

When a preview is in 演示 → 全屏 mode, the macOS close handler called
window.hide() directly, leaving the OS fullscreen Space orphaned as a
black screen — the window vanished but the Space stayed up.

Extract hideWindowExitingFullscreen as the named invariant ("hide,
but first leave fullscreen so the OS Space tears down with the window")
and route the darwin close handler through it. The hide is deferred
until 'leave-full-screen' fires so we don't race the OS Space teardown.

Bootstraps Vitest on apps/desktop with a single test under
tests/main/hide-window-exiting-fullscreen.test.ts that exercises the
helper through a structural mock — the bug shape is pure logic, no real
Electron window required. Spec was red against a hide-only helper and
green after the leave-full-screen sequencing.

* docs(agents): codify bug follow-up workflow

Distill the spec-first / cheapest-layer / scope-discipline /
invariant-shaped-fix / baseline-diff playbook used recently on #135 and
#1215 into a top-level subsection of root AGENTS.md, framed as a default
action shape with explicit room for case-by-case judgment rather than a
hard rule. Includes a single pointer back to the worked example spec.

* docs(agents): require staged human verification for visible bugs

Add the human-verification gate as a sixth bullet in the Bug follow-up
workflow. UI / platform-native / animation symptoms can pass green specs
and still ship the visible regression — proven by #1215, where the
desktop unit test green-lighted the helper logic but only a side-by-side
buggy-vs-fix run on a real macOS Space proved the black-screen actually
went away.

Reinforces the production-API-only seed constraint while we're there:
source-level backdoors prove a fake flow, not the real one, so they
invalidate the verification.

* fix(desktop): defer hide across the fullscreen-enter transition (#1215)

mrcfps observed on PR #1249 that the close handler only catches windows
already in fullscreen — Electron's enter-full-screen event is async on
macOS, so isFullScreen() can still read false during the OS Space
transition triggered by requestFullscreen(). A close in that window
took the plain hide() path and stranded the same black Space the fix
was meant to eliminate.

Track in-flight fullscreen entry from webContents.enter-html-full-screen
(set) and BrowserWindow.{enter,leave}-full-screen (clear), and surface
it through WindowFullscreenSurface.isEnteringFullscreen. The helper now
parks on enter-full-screen until the OS confirms the Space, then runs
the existing exit-then-hide path.

Adds a regression test ("waits out a fullscreen-enter transition before
exiting and hiding") that goes red against the previous helper.

AGENTS.md

@@ -88,6 +88,19 @@ This file is the single source of truth for agents entering this repository. Rea
 - Stamp/namespace changes must validate two concurrent namespaces and run desktop `inspect eval` plus `inspect screenshot` for each namespace.
 - Path/log changes must run `pnpm tools-dev logs --namespace <name> --json` and confirm log paths are under `.tmp/tools-dev/<namespace>/...`.
 
+## Bug follow-up workflow
+
+The following is a working playbook for routine bug follow-ups, distilled from recent practice. Treat it as a default action shape, not a contract — production reality always has edges these bullets can't anticipate, so use judgment when the situation doesn't fit cleanly.
+
+- **Lead with a red spec.** Default to encoding the bug as a falsifiable test that goes red before any source change, so the fix is anchored in observable behavior rather than source-code intuition. If a red spec can't be written cheaply, that's usually a signal to clarify scope rather than push forward on a guess.
+- **Try the cheapest layer first.** Reach for the lightest test layer that can still see the symptom (e2e Vitest at the daemon HTTP boundary → app-local Vitest → Playwright UI → platform-native harnesses), and drop down only when the cheaper layer can't.
+- **Hold the spec's scope.** Defects discovered outside the bug's described boundary belong in a follow-up — their own red spec, their own PR — not in this fix. List them in the PR body's "Adjacent issues" section with the rationale and move on.
+- **Let the fix read as an invariant.** Prefer a named helper whose docblock describes what must hold over a bolt-on `if` guard with apologetic history-comments. The call site should read as intent.
+- **Diff against the baseline.** When neighboring suites have pre-existing failures, stash or check out upstream before claiming "no new failures."
+- **Stage human verification for visible bugs.** When the symptom needs an eye to confirm — UI, platform-native behavior, animations, race conditions a unit test can't see — green specs alone aren't acceptance. Stand up a buggy-vs-fix comparison the reviewer can drive themselves (typical shape: two namespaced runtimes, one on `main`, one on the fix branch), and seed any required data only through production HTTP APIs; source-level test backdoors invalidate the verification because they prove a fake flow rather than the real one.
+
+For a worked example of one full loop (red e2e spec → fix → green), see `e2e/tests/dialog/stop-reconciles-message.test.ts` (issue #135).
+
 # Common commands
 
 ```bash

apps/desktop/package.json

@@ -15,7 +15,8 @@
   },
   "scripts": {
     "build": "tsc -p tsconfig.json",
-    "typecheck": "tsc -p tsconfig.json --noEmit"
+    "test": "vitest run -c vitest.config.ts",
+    "typecheck": "tsc -p tsconfig.json --noEmit && tsc -p tsconfig.tests.json --noEmit"
   },
   "dependencies": {
     "@open-design/platform": "workspace:*",
@@ -25,7 +26,8 @@
   "devDependencies": {
     "@types/node": "24.12.2",
     "electron": "41.3.0",
-    "typescript": "6.0.3"
+    "typescript": "6.0.3",
+    "vitest": "^2.1.8"
   },
   "engines": {
     "node": "~24"

apps/desktop/src/main/runtime.ts

@@ -627,6 +627,58 @@ function ensureWindowVisible(window: BrowserWindow): void {
   window.focus();
 }
 
+/**
+ * Surface of {@link BrowserWindow} consumed by
+ * {@link hideWindowExitingFullscreen} — declared structurally so the
+ * helper can be exercised with a plain mock in unit tests without
+ * standing up an actual Electron window.
+ *
+ * `isEnteringFullscreen` covers the Electron-asynchronous gap between
+ * the renderer asking for fullscreen and the OS confirming via
+ * 'enter-full-screen': the caller is expected to track this from the
+ * window's enter/leave listeners (see the close handler in
+ * {@link configureWindow}) and surface it here.
+ */
+export type WindowFullscreenSurface = {
+  hide: () => void;
+  isFullScreen: () => boolean;
+  isSimpleFullScreen: () => boolean;
+  isEnteringFullscreen: () => boolean;
+  setFullScreen: (flag: boolean) => void;
+  setSimpleFullScreen: (flag: boolean) => void;
+  once: (event: 'enter-full-screen' | 'leave-full-screen', listener: () => void) => unknown;
+};
+
+/**
+ * Hide the window, first leaving any active fullscreen so macOS doesn't
+ * orphan the fullscreen Space as a black screen. The hide is deferred
+ * until 'leave-full-screen' fires; if the Space transition is still
+ * flipping in (`isEnteringFullscreen`), defer further until
+ * 'enter-full-screen' settles before starting the exit. Plain hides
+ * race the OS Space teardown and leave the user staring at a black
+ * desktop until they switch Spaces by hand.
+ */
+export function hideWindowExitingFullscreen(window: WindowFullscreenSurface): void {
+  if (window.isSimpleFullScreen()) {
+    window.once('leave-full-screen', () => window.hide());
+    window.setSimpleFullScreen(false);
+    return;
+  }
+  if (window.isFullScreen()) {
+    window.once('leave-full-screen', () => window.hide());
+    window.setFullScreen(false);
+    return;
+  }
+  if (window.isEnteringFullscreen()) {
+    window.once('enter-full-screen', () => {
+      window.once('leave-full-screen', () => window.hide());
+      window.setFullScreen(false);
+    });
+    return;
+  }
+  window.hide();
+}
+
 // PPTX is rendered by the agent into the project folder and reaches the
 // renderer through a normal `<a download>` link to /api/projects/:id/raw/*.
 // Without this hook Electron writes the bytes straight to the OS Downloads
@@ -867,11 +919,43 @@ export async function createDesktopRuntime(options: DesktopRuntimeOptions): Prom
   });
 
   if (process.platform === "darwin") {
+    // Track the in-flight fullscreen-enter window so the close handler can
+    // tell mid-transition apart from "definitely not fullscreen". HTML
+    // requestFullscreen() emits enter-html-full-screen on webContents
+    // before the OS Space transition completes; the BrowserWindow
+    // enter-full-screen event fires once the OS confirms.
+    let enteringFullscreen = false;
+    window.webContents.on("enter-html-full-screen", () => {
+      enteringFullscreen = true;
+    });
+    window.webContents.on("leave-html-full-screen", () => {
+      enteringFullscreen = false;
+    });
+    window.on("enter-full-screen", () => {
+      enteringFullscreen = false;
+    });
+    window.on("leave-full-screen", () => {
+      enteringFullscreen = false;
+    });
     window.on("close", (event) => {
-      if (!stopped) {
-        event.preventDefault();
-        window.hide();
-      }
+      if (stopped) return;
+      event.preventDefault();
+      hideWindowExitingFullscreen({
+        hide: () => window.hide(),
+        isFullScreen: () => window.isFullScreen(),
+        isSimpleFullScreen: () => window.isSimpleFullScreen(),
+        isEnteringFullscreen: () => enteringFullscreen,
+        setFullScreen: (flag) => window.setFullScreen(flag),
+        setSimpleFullScreen: (flag) => window.setSimpleFullScreen(flag),
+        // BrowserWindow.once is heavily overloaded; both event names are
+        // valid (BrowserWindow emits enter-full-screen and
+        // leave-full-screen on macOS) but TypeScript can't pick a single
+        // overload for the union, so narrow at the call site.
+        once: (event, listener) =>
+          event === 'enter-full-screen'
+            ? window.once('enter-full-screen', listener)
+            : window.once('leave-full-screen', listener),
+      });
     });
   }
 

apps/desktop/tests/main/hide-window-exiting-fullscreen.test.ts

@@ -0,0 +1,143 @@
+// Issue #1215 — clicking the macOS red X while the preview is in fullscreen
+// presentation mode hides the window but leaves the OS Space stuck as a
+// black screen. The close handler is allowed to call window.hide(), but it
+// must first arrange for the window to leave fullscreen so the Space tears
+// down with the window.
+//
+// This spec exercises the contract through hideWindowExitingFullscreen with
+// a structural mock — the bug shape is pure logic (sequence of calls), so
+// no real Electron window is required.
+
+import { describe, expect, test } from 'vitest';
+
+import {
+  hideWindowExitingFullscreen,
+  type WindowFullscreenSurface,
+} from '../../src/main/runtime.js';
+
+type MockCalls = string[];
+
+function createMockWindow(initial: {
+  fullScreen?: boolean;
+  simpleFullScreen?: boolean;
+  enteringFullscreen?: boolean;
+}): {
+  window: WindowFullscreenSurface;
+  calls: MockCalls;
+  emitEnterFullscreen: () => void;
+  emitLeaveFullscreen: () => void;
+} {
+  const calls: MockCalls = [];
+  let enterListener: (() => void) | null = null;
+  let leaveListener: (() => void) | null = null;
+  let fullScreen = initial.fullScreen ?? false;
+  let simpleFullScreen = initial.simpleFullScreen ?? false;
+  const enteringFullscreen = initial.enteringFullscreen ?? false;
+  const window: WindowFullscreenSurface = {
+    hide: () => calls.push('hide'),
+    isFullScreen: () => fullScreen,
+    isSimpleFullScreen: () => simpleFullScreen,
+    isEnteringFullscreen: () => enteringFullscreen,
+    setFullScreen: (flag) => {
+      calls.push(`setFullScreen(${flag})`);
+      fullScreen = flag;
+    },
+    setSimpleFullScreen: (flag) => {
+      calls.push(`setSimpleFullScreen(${flag})`);
+      simpleFullScreen = flag;
+    },
+    once: (event, listener) => {
+      calls.push(`once(${event})`);
+      if (event === 'enter-full-screen') enterListener = listener;
+      if (event === 'leave-full-screen') leaveListener = listener;
+      return undefined;
+    },
+  };
+  return {
+    calls,
+    emitEnterFullscreen: () => {
+      const fn = enterListener;
+      enterListener = null;
+      // Real Electron flips isFullScreen() to true around the same moment
+      // 'enter-full-screen' fires, so model that here too.
+      fullScreen = true;
+      fn?.();
+    },
+    emitLeaveFullscreen: () => {
+      const fn = leaveListener;
+      leaveListener = null;
+      fn?.();
+    },
+    window,
+  };
+}
+
+describe('hideWindowExitingFullscreen', () => {
+  test('hides directly when the window is not in fullscreen', () => {
+    const { window, calls } = createMockWindow({});
+    hideWindowExitingFullscreen(window);
+    expect(calls).toEqual(['hide']);
+  });
+
+  test('exits native fullscreen first, defers hide until leave-full-screen fires', () => {
+    const { window, calls, emitLeaveFullscreen } = createMockWindow({ fullScreen: true });
+    hideWindowExitingFullscreen(window);
+
+    // Before the OS confirms the Space has torn down, the window must NOT
+    // be hidden — hiding mid-transition is what leaves the black Space.
+    expect(calls).toEqual(['once(leave-full-screen)', 'setFullScreen(false)']);
+
+    emitLeaveFullscreen();
+    expect(calls).toEqual([
+      'once(leave-full-screen)',
+      'setFullScreen(false)',
+      'hide',
+    ]);
+  });
+
+  test('exits simpleFullScreen first, defers hide until leave-full-screen fires', () => {
+    const { window, calls, emitLeaveFullscreen } = createMockWindow({ simpleFullScreen: true });
+    hideWindowExitingFullscreen(window);
+
+    expect(calls).toEqual(['once(leave-full-screen)', 'setSimpleFullScreen(false)']);
+
+    emitLeaveFullscreen();
+    expect(calls).toEqual([
+      'once(leave-full-screen)',
+      'setSimpleFullScreen(false)',
+      'hide',
+    ]);
+  });
+
+  // The macOS Space transition between requestFullscreen() and the
+  // 'enter-full-screen' event is asynchronous; isFullScreen() can still
+  // read false during that window. If we hide unconditionally in that gap
+  // the OS strands the still-transitioning Space as a black screen — the
+  // exact symptom #1215 reported. Wait for the enter to settle before
+  // pivoting to the exit-then-hide path.
+  test('waits out a fullscreen-enter transition before exiting and hiding', () => {
+    const { window, calls, emitEnterFullscreen, emitLeaveFullscreen } = createMockWindow({
+      enteringFullscreen: true,
+    });
+    hideWindowExitingFullscreen(window);
+
+    // While the Space is still flipping in, only an enter-listener is
+    // registered — no hide, no setFullScreen.
+    expect(calls).toEqual(['once(enter-full-screen)']);
+
+    emitEnterFullscreen();
+    expect(calls).toEqual([
+      'once(enter-full-screen)',
+      'once(leave-full-screen)',
+      'setFullScreen(false)',
+    ]);
+
+    emitLeaveFullscreen();
+    expect(calls).toEqual([
+      'once(enter-full-screen)',
+      'once(leave-full-screen)',
+      'setFullScreen(false)',
+      'hide',
+    ]);
+  });
+});

apps/desktop/tsconfig.tests.json

@@ -0,0 +1,16 @@
+{
+  "extends": "./tsconfig.json",
+  "compilerOptions": {
+    "noEmit": true,
+    "rootDir": "."
+  },
+  "include": [
+    "tests/**/*.ts",
+    "tests/**/*.tsx",
+    "src/**/*.ts"
+  ],
+  "exclude": [
+    "node_modules",
+    "dist"
+  ]
+}

apps/desktop/vitest.config.ts

@@ -0,0 +1,9 @@
+import { defineConfig } from 'vitest/config';
+
+export default defineConfig({
+  test: {
+    environment: 'node',
+    include: ['tests/**/*.test.{ts,tsx,js,mjs,cjs}'],
+    testTimeout: 10_000,
+  },
+});

pnpm-lock.yaml

@@ -103,6 +103,9 @@ importers:
       typescript:
         specifier: 6.0.3
         version: 6.0.3
+      vitest:
+        specifier: ^2.1.8
+        version: 2.1.9(@types/node@24.12.2)(jsdom@29.1.1)
 
   apps/landing-page:
     dependencies: