Commit Graph

4 Commits

Author SHA1 Message Date
Tom Boucher
7ebcf41939 feat(3567): state.* router delegates via executeForCjs + Phase 5.0 worker fix (Phase 5.1 of #3524)
Phase 5.1 of the CJS↔SDK hard-seam migration (parent #3524). Migrates
the bin/lib/state-command-router.cjs handlers map to delegate every
canonical state subcommand through the executeForCjs synchronous
primitive (shipped in Phase 5.0, PR #3558).

## Bundled fix for Phase 5.0 worker defect

Discovered during Phase 5.1 implementation that the Phase 5.0
worker drops projectDir and workstream from
RuntimeBridgeExecuteInput. The dispatch closure at
sdk/src/runtime-bridge-sync/worker.ts:41-42 hardcoded projectDir
to '', so registry handlers that read .planning/ from projectDir
(every state.* handler) saw an empty path and failed. Phase 5.0's
pinning tests passed because they exercised commands that don't
depend on projectDir (generate-slug takes its arg directly;
unknown_command doesn't dispatch). Maintainer authorized bundling
the fix into this PR.

Fix: moved QueryNativeDirectAdapter construction inside the
dispatchNative lambda so request.projectDir and request.workstream
close over the per-request values. Per-request adapter construction
adds <1ms overhead; correctness wins. Regression test at
sdk/src/runtime-bridge-sync/projectdir-regression.test.ts demonstrates
RED before fix → GREEN after.

Phase 5.0's index.test.ts native_failure fixture was passing
because of the bug — it relied on projectDir = '' producing a
specific error path. Updated to use a /nonexistent-... path that
triggers ENOENT under realpath, producing native_failure as intended.

## What landed for Phase 5.1

- bin/lib/state-command-router.cjs migrated. Every subcommand
  entry in the handlers map dispatches via executeForCjs when SDK
  is available, with transparent fallback to the existing CJS
  handlers in state.cjs if (a) SDK is not built / not present, or
  (b) GSD_WORKSTREAM is set (the sync-bridge worker cannot serve
  workstream-scoped commands per the SDK transport architecture).
- Special cases preserved:
  - load --raw: SDK data formatted into key=value lines matching
    cmdStateLoad's exact format.
  - complete-phase: CJS-only (no SDK counterpart yet).
  - add-roadmap-evolution: stays on the unsupported list (SDK-only).
- Golden parity tests added for 12 previously-uncovered state
  subcommands: advance-plan, record-metric, update-progress,
  add-decision, add-blocker, resolve-blocker, record-session,
  signal-waiting, signal-resume, planned-phase, milestone-switch,
  prune.

## Design decisions worth reviewer visibility

1. Lazy SDK loading with CJS fallback. The migration routes via
   executeForCjs only when the SDK is loadable; otherwise falls
   back to the existing CJS handlers. Conservative for rollback —
   if the SDK build is broken on a deploy, state commands keep
   working via the CJS path. Trade-off: drift surface is not
   structurally eliminated yet — the CJS handlers remain reachable.

2. Workstream → CJS fallback. The SDK transport forces subprocess
   for workstream commands, but subprocess is disabled in the sync
   bridge. When GSD_WORKSTREAM is set, the entire state command
   falls back to CJS rather than failing. Workstream users continue
   running the CJS handlers; the SDK path is exercised only in the
   default (no workstream) case.

3. Two documented parity divergences. state.record-metric: CJS
   auto-creates ## Performance Metrics section when absent; SDK
   returns {recorded: false, reason}. Test requires fixture with
   the section present. state.prune: CJS counts phases from disk;
   SDK reads from frontmatter fields. Test asserts structural shape
   rather than exact equality.

## Numbers

- Full CJS suite: 9323/9323 pass (baseline 9323; +0 net because
  the 12 new parity tests are SDK-side vitest, not CJS-side).
- SDK vitest sync-bridge: 10/10 pass.
- Regression test: 3/3 pass (proved RED before fix, GREEN after).
- tests/state.test.cjs (the safety net): 104/104 pass unchanged.

## Performance

gsd-tools state load via the SDK path: 49ms first call (Worker
startup), 43-44ms steady-state median. Slower than the
Phase 5.0-measured 0.1ms because state.load does fs reads on top
of the bridge overhead. Still well within the budget for CJS
dispatcher overhead.

Closes #3567.
2026-05-15 14:34:25 -04:00
Tom Boucher
fcef212926 test: align runtime-bridge coverage header 2026-05-15 11:59:11 -04:00
Tom Boucher
3ca410b5cc test: pin runtime-bridge-sync error classifications 2026-05-15 11:52:56 -04:00
Tom Boucher
8090456b66 feat(3555): QueryRuntimeBridge.executeForCjs synchronous primitive (Phase 5.0 of #3524)
Phase 5.0 of the CJS↔SDK hard-seam migration (parent #3524).
Foundational PR. Ships ONLY the synchronous primitive on the SDK
runtime bridge plus pinning tests. Per-family CJS router migrations
(state.*, verify.*, init.*, phase.*, phases.*, validate.*,
roadmap.*, frontmatter.*, config.*) become follow-up enhancements
that each reuse this primitive.

## What landed

- sdk/src/runtime-bridge-sync/index.ts (155 lines) — public API.
  Exports executeForCjs(input: RuntimeBridgeExecuteInput):
  RuntimeBridgeSyncResult. Synchronous; lazily creates the
  synckit sync function on first call.
- sdk/src/runtime-bridge-sync/worker.ts (167 lines) — synckit
  worker. Constructs a native-only QueryRuntimeBridge (with
  allowFallbackToSubprocess: false), awaits its async execute,
  catches GSDToolsError / GSDError, maps classification to the
  six ADR-0001 canonical error kinds plus exit code.
- sdk/src/runtime-bridge-sync/index.test.ts (197 lines, 8 vitest
  pinning fixtures) — success path; unknown_command;
  native_failure (shape); validation_error (shape); shape
  invariants; idempotency.
- tests/runtime-bridge-sync-smoke.test.cjs (99 lines, 4
  node:test cases) — proves the primitive works from CJS
  callers via require().

## Decisions

1. Synchronous-bridging mechanism: synckit. Disqualified:
   - deasync: stagnant (68 open issues, single maintainer,
     last release Nov 2025), private Node API (process.binding('uv')),
     untested on Node 22, documented deadlocks with modern
     Promise chains.
   - Sync-native SDK refactor: technically infeasible —
     acquireStateLock in state-mutation.ts uses await setTimeout
     for retry backoff; making that fully sync requires either
     Atomics.wait (which IS synckit), busy-loop (degrades
     responsiveness), or breaking 100+ SDK consumers.
   Synckit (v0.11.12) is pure JS, actively maintained (last
   push today), stable public APIs (Atomics.wait +
   SharedArrayBuffer), Node 22 compatible, no native compile.

2. Native-only transport inside the worker. The sync bridge
   uses allowFallbackToSubprocess: false. Unknown commands
   surface as unknown_command instead of spawning gsd-sdk.
   Keeps the worker self-contained and predictable.

3. Worker path resolution. resolveWorkerPath() navigates ../..
   from the loaded module URL to land at
   dist/runtime-bridge-sync/worker.js — works under both
   vitest (loads src) and CJS consumers (load dist).

4. GSDError.Blocked → validation_error. ADR-0001's 6-kind
   taxonomy has no `blocked` kind; Blocked classification is
   mapped onto validation_error since the operational shape
   matches (prerequisite missing).

## Numbers

- 8 SDK vitest pinning tests pass.
- 4 CJS smoke tests pass.
- Full suite: 9286/9286 pass (baseline 9282; +4 from the
  new smoke test cases).
- SDK vitest unit: 1860/1860 pass.
- Performance: 80ms first-call cold latency (Worker startup +
  bridge construction); 0.1ms steady-state per-call latency
  (10-call average after warmup). Well within budget for CJS
  dispatcher overhead.

## Canonical error kind coverage

- unknown_command: covered with pinning fixture
- native_failure: shape coverage (handler that throws)
- validation_error: shape coverage (GSDError.Validation +
  GSDError.Blocked)
- internal_error: shape coverage only (eliciting TypeError
  reliably from a registered handler requires elaborate fixture)
- native_timeout: NOT pinned (no registered handler genuinely
  times out; classification logic present in worker)
- fallback_failure: NOT pinned (subprocess fallback disabled
  by design in sync bridge)

The classification logic is in the worker regardless; per-family
migration PRs will exercise the unpinned kinds incidentally.

## Wiring

- sdk/package.json: synckit ^0.11.12 added as runtime
  dependency (not devDependency — it's required at runtime
  whenever a CJS caller invokes executeForCjs).
- sdk/package-lock.json: regenerated.
- CONTEXT.md: new "Sync Runtime Bridge Module" entry added
  after Dispatch Policy Module. Existing "CJS Command Router
  Adapter Module" entry amended with one sentence pointing at
  the primitive and the per-family migration roadmap.

No generator/freshness check needed for this phase — the
primitive IS the SDK (not a generated CJS mirror).

Closes #3555.
2026-05-15 11:10:03 -04:00