fix(sdk): make stub-leak detection TDZ-safe + defer to next microtask (#1287) (#1398)

* fix(sdk): make stub-leak detection TDZ-safe + defer to next microtask (#1287)

`bun run scripts/start-grpc.ts` crashed at startup with:

    ReferenceError: Cannot access 'QueryEngine' before initialization.
        at detectStubLeaks (src/entrypoints/sdk/index.ts:29:33)
        at src/entrypoints/sdk/index.ts:47:1

The detector ran at module-load time and read each critical import
directly. When the start script's circular-import chain reached the SDK
barrel before `QueryEngine.js` had finished initializing its own export
bindings, the QueryEngine reference at line 29 hit the temporal dead
zone and threw. Stub-leak detection is meant to catch `__stub: true`
markers from the esbuild plugin — TDZ is a different bug class (an
uninitialized binding can't carry `__stub`), so the detector should
treat the access failure as 'nothing to check here' rather than
crashing the entire SDK entry.

Two changes:

1. Wrap each import read in safelyAccess(() => binding) so a TDZ
   ReferenceError on one returns undefined and the loop continues.
   Real stub markers still surface as the explicit SDK init error.
2. Defer detectStubLeaks() from module-load to queueMicrotask, so
   every same-tick init in the circular chain (start-grpc.ts → SDK
   index → QueryEngine → ... → SDK index) completes before we read
   bindings. Microtask runs before any actual SDK usage, so a real
   stub leak still surfaces well before the first query() call.

Tests (3): SDK barrel imports without throwing, anti-regression on
real __stub: true bindings, TDZ-shaped access returns undefined.

* test(sdk): exercise the real stub-leak detector with stubbed fixtures (#1287)

The regression test asserted only that a local object literal had
__stub === true and re-implemented safelyAccess inline, so it never ran
the real detector: removing queueMicrotask(detectStubLeaks), dropping the
loop, or swallowing the __stub case would all still pass.

Split the detection primitives (safelyAccess + the critical-import scan)
into src/entrypoints/sdk/stubLeakDetection.ts and have the SDK entry point
import them. The test now feeds stub-shaped fixtures through the real
checkCriticalImportsForStubs / safelyAccess and asserts: a real
__stub: true binding throws the explicit SDK init error; non-stub modules
pass; a TDZ ReferenceError is tolerated (skipped) without crashing; a stub
behind a skipped TDZ access is still caught; and the SDK barrel import
never throws on its own load. Detector runtime behavior is unchanged.

src/entrypoints/sdk/index.ts

@@ -12,6 +12,11 @@ import type { CallToolResult, ToolAnnotations } from '@modelcontextprotocol/sdk/
 import { QueryEngine } from '../../QueryEngine.js'
 import { getTools } from '../../tools.js'
 import { init } from '../init.js'
+import {
+  checkCriticalImportsForStubs,
+  safelyAccess,
+  type CriticalImport,
+} from './stubLeakDetection.js'
 
 // ============================================================================
 // Stub leak detection
@@ -22,29 +27,48 @@ import { init } from '../init.js'
  * runtime. The esbuild sdk-missing-stub plugin marks every stub with
  * `__stub: true`. We check core SDK modules that should NEVER be stubs.
  * If any resolved to a stub, it means a TUI dependency leaked through.
+ *
+ * Each access is wrapped in a TDZ-safe getter so a circular import that
+ * leaves one binding uninitialized at the moment we run cannot crash the
+ * whole detector (#1287 — `npm run dev:grpc` tripped the check at
+ * `QueryEngine` via a TDZ ReferenceError from the script's import order).
+ * TDZ is a separate bug class than stub-leak: an uninitialized binding
+ * can't carry `__stub: true`, so the access failure is treated as "nothing
+ * to check here". Real stub leaks still throw the explicit SDK init error.
+ *
+ * The detection primitives live in ./stubLeakDetection so the real path can
+ * be unit-tested directly (see tests/sdk/stub-leak-detect.test.ts).
  */
 function detectStubLeaks(): void {
-  const criticalImports: Array<{ name: string; mod: Record<string, unknown> }> = [
+  const criticalImports: CriticalImport[] = [
     // QueryEngine is the core SDK engine — must never be a stub
-    { name: 'QueryEngine', mod: QueryEngine as unknown as Record<string, unknown> },
+    {
+      name: 'QueryEngine',
+      get: () =>
+        safelyAccess(() => QueryEngine as unknown as Record<string, unknown>),
+    },
     // These are imported by this file and must be real modules, not stubs
-    { name: 'getTools', mod: getTools as unknown as Record<string, unknown> },
-    { name: 'init', mod: init as unknown as Record<string, unknown> },
+    {
+      name: 'getTools',
+      get: () =>
+        safelyAccess(() => getTools as unknown as Record<string, unknown>),
+    },
+    {
+      name: 'init',
+      get: () => safelyAccess(() => init as unknown as Record<string, unknown>),
+    },
   ]
 
-  for (const { name, mod } of criticalImports) {
-    if ('__stub' in mod && mod.__stub === true) {
-      throw new Error(
-        `SDK init error: "${name}" resolved to a build stub at runtime. ` +
-        `This means a TUI/CLI dependency leaked into the SDK bundle. ` +
-        `Report this at https://github.com/Gitlawb/openclaude/issues`,
-      )
-    }
-  }
+  checkCriticalImportsForStubs(criticalImports)
 }
 
-// Run leak detection once at module load time.
-detectStubLeaks()
+// Run leak detection on the next microtask so every same-tick module init
+// (including circular-dep neighbors like scripts/start-grpc.ts → SDK index
+// → QueryEngine → ... → SDK index) completes before we read the bindings.
+// Module-load-time invocation would race the binding initialization order
+// and surface as a TDZ ReferenceError (#1287). The safelyAccess wrapper
+// inside detectStubLeaks belt-and-braces this for any remaining cycles.
+queueMicrotask(detectStubLeaks)
 
 // ============================================================================
 // Re-exports from shared types

src/entrypoints/sdk/stubLeakDetection.ts

@@ -0,0 +1,51 @@
+/**
+ * Stub-leak detection helpers for the SDK barrel.
+ *
+ * The esbuild sdk-missing-stub plugin marks every build stub with
+ * `__stub: true`. Core SDK modules (QueryEngine, getTools, init) must never
+ * resolve to a stub at runtime — if one does, a TUI/CLI dependency leaked into
+ * the SDK bundle. These helpers are split out of the SDK entry point so the
+ * real detection path can be unit-tested directly with stub-shaped fixtures
+ * (the entry point only runs the check as an import side effect).
+ */
+
+export type CriticalImport = {
+  name: string
+  get: () => Record<string, unknown> | undefined
+}
+
+/**
+ * Invoke `fn`, swallowing a throw and returning undefined. The throw we care
+ * about is a TDZ ReferenceError from a circular import that left the binding
+ * uninitialized at call time (#1287). TDZ is a different bug class than a stub
+ * leak: an uninitialized binding can't carry `__stub: true`, so treat the
+ * access failure as "nothing to check here" instead of crashing the detector.
+ */
+export function safelyAccess<T>(fn: () => T): T | undefined {
+  try {
+    return fn()
+  } catch {
+    return undefined
+  }
+}
+
+/**
+ * Throw an explicit SDK init error if any critical import resolved to a build
+ * stub (`__stub: true`). Accessors that throw (TDZ) or resolve to undefined are
+ * skipped without short-circuiting the loop, so a real stub on a later import
+ * is still caught.
+ */
+export function checkCriticalImportsForStubs(
+  criticalImports: CriticalImport[],
+): void {
+  for (const { name, get } of criticalImports) {
+    const mod = get()
+    if (mod && '__stub' in mod && mod.__stub === true) {
+      throw new Error(
+        `SDK init error: "${name}" resolved to a build stub at runtime. ` +
+          `This means a TUI/CLI dependency leaked into the SDK bundle. ` +
+          `Report this at https://github.com/Gitlawb/openclaude/issues`,
+      )
+    }
+  }
+}

tests/sdk/stub-leak-detect.test.ts

@@ -0,0 +1,90 @@
+import { describe, expect, test } from 'bun:test'
+import {
+  checkCriticalImportsForStubs,
+  safelyAccess,
+  type CriticalImport,
+} from '../../src/entrypoints/sdk/stubLeakDetection.ts'
+
+// Pin issue #1287: stub-leak detection must not throw a ReferenceError when one
+// of the bindings under inspection is still in the temporal dead zone (e.g.
+// mid-circular-import). TDZ is a different bug class than a stub leak — an
+// uninitialized binding can't carry `__stub: true`, so the detector treats the
+// access failure as "skip" rather than crashing the whole SDK entry.
+//
+// These tests drive the real detection primitives directly (the SDK barrel only
+// runs them as an import side effect via queueMicrotask), so a regression that
+// removes the loop, drops the throw, or swallows the `__stub` case is caught.
+
+describe('SDK stub-leak detection (issue #1287)', () => {
+  test('throws the SDK init error when a critical import resolves to a real __stub: true binding', () => {
+    const criticalImports: CriticalImport[] = [
+      { name: 'QueryEngine', get: () => ({ __stub: true }) },
+    ]
+    expect(() => checkCriticalImportsForStubs(criticalImports)).toThrow(
+      /SDK init error: "QueryEngine" resolved to a build stub/,
+    )
+  })
+
+  test('does not throw when every critical import resolves to a real (non-stub) module', () => {
+    const criticalImports: CriticalImport[] = [
+      { name: 'QueryEngine', get: () => ({ run: () => undefined }) },
+      { name: 'getTools', get: () => ({ default: () => [] }) },
+      { name: 'init', get: () => ({}) },
+    ]
+    expect(() => checkCriticalImportsForStubs(criticalImports)).not.toThrow()
+  })
+
+  test('tolerates a TDZ ReferenceError from an uninitialized binding (anti-#1287)', () => {
+    // A binding still in the temporal dead zone throws on access; safelyAccess
+    // swallows it so the detector skips that import instead of crashing.
+    const criticalImports: CriticalImport[] = [
+      {
+        name: 'QueryEngine',
+        get: () =>
+          safelyAccess(() => {
+            throw new ReferenceError(
+              "Cannot access 'QueryEngine' before initialization.",
+            )
+          }),
+      },
+    ]
+    expect(() => checkCriticalImportsForStubs(criticalImports)).not.toThrow()
+  })
+
+  test('a stub on a later import is still caught after a skipped TDZ access', () => {
+    // The TDZ skip must not short-circuit the loop: a real stub behind a
+    // not-yet-initialized binding is still detected.
+    const criticalImports: CriticalImport[] = [
+      {
+        name: 'QueryEngine',
+        get: () =>
+          safelyAccess(() => {
+            throw new ReferenceError('tdz')
+          }),
+      },
+      { name: 'getTools', get: () => ({ __stub: true }) },
+    ]
+    expect(() => checkCriticalImportsForStubs(criticalImports)).toThrow(
+      /"getTools" resolved to a build stub/,
+    )
+  })
+
+  test('safelyAccess returns the value on success and undefined on throw', () => {
+    expect(safelyAccess(() => 42)).toBe(42)
+    expect(
+      safelyAccess(() => {
+        throw new Error('boom')
+      }),
+    ).toBeUndefined()
+  })
+
+  test('importing the SDK barrel never throws synchronously on its own load', async () => {
+    // queueMicrotask defers the real detector to the next tick so circular-dep
+    // module init completes first; the bare import must always succeed.
+    const sdk = await import('../../src/entrypoints/sdk/index.ts')
+    expect(sdk).toBeDefined()
+    // Yield so any queued microtask runs, then re-confirm nothing threw.
+    await new Promise(resolve => setTimeout(resolve, 0))
+    expect(sdk).toBeDefined()
+  })
+})