fix(secrets): keep the deferred protection report non-fatal, and pin the wiring

Deferring the report moved it off `whenReady`'s promise chain. A throw there was
an unhandled rejection the app survives; inside `setImmediate` it is an uncaught
exception, and `installUncaughtPipeErrorGuard` re-throws those fatally — so a
diagnostic the module documents as deliberately not fatal could kill the app.
Wrap the deferred call so it degrades to a warn. Serve keeps the inline posture.

Nothing outside index.ts referenced the scheduler, so reverting the call site,
or flipping `deferUntilFirstWindow`, left the whole suite green — including the
headless-serve regression an earlier review already caught once. Pin the wiring
as source text, following the host-port-bootstrap-wiring idiom with every anchor
bounded, and pin the two module gates that were only jointly covered.

src/main/host/deferred-secret-protection-report.test.ts

@@ -2,7 +2,7 @@ import { mkdtempSync, rmSync } from 'node:fs'
 import { tmpdir } from 'node:os'
 import { join } from 'node:path'
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
-import { setSecretStore } from '../../shared/secret-store'
+import { _resetSecretStoreForTests, setSecretStore } from '../../shared/secret-store'
 
 type Listener = (...args: unknown[]) => void
 
@@ -69,6 +69,7 @@ describe('scheduleSecretProtectionGapReport', () => {
 
   afterEach(() => {
     vi.useRealTimers()
+    _resetSecretStoreForTests()
     rmSync(dir, { recursive: true, force: true })
   })
 
@@ -126,12 +127,38 @@ describe('scheduleSecretProtectionGapReport', () => {
     schedule()
     const window = createWindow()
     window.reveal()
+    drain()
+    // Why assert the timer is gone and not just the probe count: the once-guard alone makes
+    // the count right, so without this the fallback could stay armed for 15s past the reveal
+    // and nothing here would notice.
+    expect(vi.getTimerCount()).toBe(0)
     vi.runAllTimers()
     vi.advanceTimersByTime(60_000)
     vi.runAllTimers()
     expect(probes).toBe(1)
   })
 
+  it('does not let the fallback timer hold the process open', () => {
+    // Why: the report is diagnostics; a referenced 15s timer would keep a quitting app alive
+    // for up to 15s after its last window closed.
+    const timers: ReturnType<typeof setTimeout>[] = []
+    const original = globalThis.setTimeout
+    const spy = vi.spyOn(globalThis, 'setTimeout').mockImplementation(((
+      ...args: Parameters<typeof setTimeout>
+    ) => {
+      const timer = original(...args)
+      timers.push(timer)
+      return timer
+    }) as typeof setTimeout)
+    try {
+      schedule()
+    } finally {
+      spy.mockRestore()
+    }
+    expect(timers).toHaveLength(1)
+    expect(timers[0]?.hasRef()).toBe(false)
+  })
+
   it('does not probe again when the window reveals after the fallback already reported', () => {
     // Why this order and not the reverse: a GPU that presents late reveals the window
     // after the fallback has already reported, and a second blocking keyring probe would
@@ -146,6 +173,26 @@ describe('scheduleSecretProtectionGapReport', () => {
     expect(probes).toBe(1)
   })
 
+  it('does not turn a throwing report into a fatal main-process error', () => {
+    // Why: deferred, this runs off whenReady's promise chain, where a throw is an
+    // uncaughtException the main-process guard re-throws fatally (#9441) — not the
+    // unhandled rejection the app survives. A diagnostic must never end the process.
+    let thrown = 0
+    scheduleSecretProtectionGapReport({
+      dataFile,
+      log: () => {
+        thrown += 1
+        throw new Error('log sink is broken')
+      },
+      deferUntilFirstWindow: true
+    })
+    const window = createWindow()
+    window.reveal()
+    expect(() => drain()).not.toThrow()
+    expect(probes).toBe(1)
+    expect(thrown).toBeGreaterThan(0)
+  })
+
   it('reports inline in headless serve, before the runtime is advertised as ready', () => {
     // Why not deferred: serve opens no window, so the fallback would fire only after
     // clients could already have paired, and a frozen main thread reads as a dead host.

src/main/host/deferred-secret-protection-report.ts

@@ -12,6 +12,12 @@ import { reportSecretProtectionGap } from './secret-protection-report'
  * (STA-5765). Run before the first window, that reads to the user as "the app will not
  * open"; the window is gated behind a diagnostic whose result nothing on the startup
  * path consumes.
+ *
+ * Why every platform and not just Linux: deferring costs nothing here — no caller reads the
+ * result, so the only thing that moves is when a console line appears. macOS pays the same
+ * shape against the Keychain, and a probe that has to answer before a window exists is wrong
+ * on any host. A deferral that withholds a real secret has to be scoped to the platform that
+ * needs it; this one withholds nothing.
  */
 
 // Why a fallback as well as the window event: `ready-to-show` can fail to fire at all
@@ -40,6 +46,25 @@ export function scheduleSecretProtectionGapReport({
     return
   }
 
+  // Why swallow here and not in reportSecretProtectionGap: deferred, this no longer runs on
+  // whenReady's promise chain, where a throw was an unhandled rejection the app survives
+  // (#9441). Off that chain it is an uncaughtException, and installUncaughtPipeErrorGuard
+  // re-throws those fatally — killing the app over a diagnostic the module documents as
+  // deliberately not fatal. Serve still reports inline and keeps the old posture.
+  const report = (): void => {
+    try {
+      reportSecretProtectionGap(options)
+    } catch (error) {
+      try {
+        ;(options.log ?? console.warn)(
+          `[secrets] could not run the deferred protection report: ${String(error)}`
+        )
+      } catch {
+        // A throwing log sink must not be what ends the process either.
+      }
+    }
+  }
+
   let ran = false
   const run = (): void => {
     if (ran) {
@@ -49,9 +74,7 @@ export function scheduleSecretProtectionGapReport({
     clearTimeout(fallback)
     // Why setImmediate: keep the blocking keyring probe off the event handler that
     // reveals the window, so the reveal paints first.
-    setImmediate(() => {
-      reportSecretProtectionGap(options)
-    })
+    setImmediate(report)
   }
 
   const fallback = setTimeout(run, REPORT_FALLBACK_MS)

src/main/startup/secret-protection-report-deferral-wiring.test.ts

@@ -0,0 +1,63 @@
+import { readFileSync } from 'node:fs'
+import { join } from 'node:path'
+import { describe, expect, it } from 'vitest'
+
+/**
+ * Guards the one line that decides whether the OS keyring gates the first window.
+ *
+ * The report is diagnostics nothing on the startup path reads, but `describeProtectionGap()`
+ * is a blocking D-Bus round trip on Linux, and a present-but-locked keyring never answers —
+ * 76s to first window on Ubuntu 24.04 (STA-5765). Both directions of the one flag here are
+ * silent: `true` gives headless serve a deferral it must never have (serve opens no window,
+ * so only the fallback fires, after clients may have paired, and a frozen main thread reads
+ * as a dead host); `false` puts the blocking probe back in front of the window. Deleting the
+ * call entirely restores the original regression.
+ *
+ * Source-level because that is the property: this runs once inside `app.whenReady()` during
+ * startup, so there is no seam to assert against at runtime.
+ */
+describe('secret protection report deferral wiring', () => {
+  const source = readFileSync(join(process.cwd(), 'src/main/index.ts'), 'utf8')
+
+  const SCHEDULE = 'scheduleSecretProtectionGapReport({'
+
+  it('arms the deferred report exactly once and never calls the blocking one directly', () => {
+    expect(source.split(SCHEDULE).length - 1, `${SCHEDULE} should appear exactly once`).toBe(1)
+    expect(source).toContain(
+      "import { scheduleSecretProtectionGapReport } from './host/deferred-secret-protection-report'"
+    )
+    // Why also assert the absence: re-importing the blocking entry point reinstates the
+    // pre-window probe without touching the call site the next test pins. Note the scheduling
+    // name is `...GapReport(`, so it does not match this substring.
+    expect(source.split('reportSecretProtectionGap(').length - 1).toBe(0)
+  })
+
+  it('defers on desktop and reports inline in headless serve', () => {
+    const start = source.indexOf(SCHEDULE)
+    // Why bound every anchor: an unresolved one is -1, and the slice below would then run to
+    // EOF and pass against unrelated code.
+    expect(start).toBeGreaterThanOrEqual(0)
+    const end = source.indexOf('\n  })', start)
+    expect(end).toBeGreaterThan(start)
+    const call = source.slice(start, end)
+
+    expect(call).toContain('deferUntilFirstWindow: !isServeMode')
+    // Why assert the constants are absent too: `!isServeMode` being present does not stop a
+    // later property in the same literal from overriding it.
+    expect(call).not.toContain('deferUntilFirstWindow: true')
+    expect(call).not.toContain('deferUntilFirstWindow: false')
+  })
+
+  it('arms the report after the profile exists and inside app readiness', () => {
+    // Why: the report remembers what it last said beside the profile data file, so arming it
+    // before the profile is resolved would key the state off a path that does not exist yet.
+    // Anchored on code, never a comment — a reworded comment silently becomes -1.
+    const ready = source.indexOf('app.whenReady().then(')
+    const profile = source.indexOf('const activeOrcaProfile = ensureActiveOrcaProfile()')
+    const schedule = source.indexOf(SCHEDULE)
+
+    expect(ready).toBeGreaterThanOrEqual(0)
+    expect(profile).toBeGreaterThan(ready)
+    expect(schedule).toBeGreaterThan(profile)
+  })
+})