From 7ebcf419395adf6354c6101ec1902a30ea362c3a Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 15 May 2026 14:34:25 -0400 Subject: [PATCH] feat(3567): state.* router delegates via executeForCjs + Phase 5.0 worker fix (Phase 5.1 of #3524) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../bin/lib/state-command-router.cjs | 335 +++++++++++++++--- sdk/src/golden/golden.integration.test.ts | 229 ++++++++++++ sdk/src/runtime-bridge-sync/index.test.ts | 18 +- .../projectdir-regression.test.ts | 151 ++++++++ sdk/src/runtime-bridge-sync/worker.ts | 38 +- 5 files changed, 692 insertions(+), 79 deletions(-) create mode 100644 sdk/src/runtime-bridge-sync/projectdir-regression.test.ts diff --git a/get-shit-done/bin/lib/state-command-router.cjs b/get-shit-done/bin/lib/state-command-router.cjs index a2ead23a7..ee472bb0d 100644 --- a/get-shit-done/bin/lib/state-command-router.cjs +++ b/get-shit-done/bin/lib/state-command-router.cjs @@ -2,10 +2,86 @@ const { STATE_SUBCOMMANDS } = require('./command-aliases.generated.cjs'); const { routeCjsCommandFamily } = require('./cjs-command-router-adapter.cjs'); +const { output } = require('./core.cjs'); + +// ─── SDK bridge (Phase 5.1) ───────────────────────────────────────────────── +// executeForCjs requires the compiled dist artifact. The require() path is +// resolved from gsd-tools.cjs's node_modules rather than this file's location, +// so we use a lazy require to avoid resolution errors on environments that +// haven't built the SDK yet. +let _executeForCjs = null; +let _formatStateLoadRawStdout = null; + +function tryLoadSdk() { + if (_executeForCjs !== null) return true; + try { + const bridgeModule = require('@gsd-build/sdk/dist/runtime-bridge-sync/index.js'); + _executeForCjs = bridgeModule.executeForCjs; + // formatStateLoadRawStdout is in state-project-load (re-exported from SDK dist) + const loadModule = require('@gsd-build/sdk/dist/query/state-project-load.js'); + _formatStateLoadRawStdout = loadModule.formatStateLoadRawStdout; + return true; + } catch { + return false; + } +} + +/** + * Dispatch a subcommand via the SDK sync bridge. + * + * Returns true if dispatched successfully, false if the SDK is unavailable. + * The caller must still handle result.ok=false as a hard error. + * + * @param {string} registryCommand - Registry command name (e.g. 'state.json') + * @param {string[]} registryArgs - Args for the registry handler + * @param {string} cwd - Project directory + * @param {boolean} raw - Raw output mode + * @param {Function} error - Error reporter + * @param {Function} [rawFormatter] - Optional raw output formatter (for state.load) + * @returns {boolean} true if handled, false to fall through to CJS + */ +function dispatchViaSdk(registryCommand, registryArgs, legacyArgs, cwd, raw, error, rawFormatter) { + if (!tryLoadSdk()) return false; + + const result = _executeForCjs({ + registryCommand, + registryArgs, + legacyCommand: 'state', + legacyArgs, + mode: raw ? 'raw' : 'json', + projectDir: cwd, + // workstream: not threaded here — GSDTransport forces subprocess for workstream + // requests and subprocess is disabled in the worker. Workstream commands fall + // back to the CJS path (see routeStateCommand guard below). + }); + + if (!result.ok) { + error(result.errorDetails && result.errorDetails.message + ? result.errorDetails.message + : `state ${registryCommand} failed (${result.errorKind})`); + return true; // handled (error was reported) + } + + if (raw && rawFormatter) { + const rawText = rawFormatter(result.data); + const fs = require('fs'); + fs.writeSync(1, rawText); + } else { + output(result.data); + } + return true; +} /** * Manifest-backed state subcommand router. * Keeps gsd-tools.cjs thin while preserving existing command semantics. + * + * Phase 5.1: handlers that have SDK equivalents are dispatched via + * executeForCjs (the sync bridge). CJS fallback is retained for: + * - complete-phase: no SDK counterpart. + * - Any command when GSD_WORKSTREAM is active (GSDTransport forces subprocess + * for workstream requests; subprocess is disabled in the sync bridge worker). + * - Any command when the SDK is not available (build not present). */ function routeStateCommand({ state, args, cwd, raw, parseNamedArgs, error }) { const parsePlans = (plans) => { @@ -17,6 +93,26 @@ function routeStateCommand({ state, args, cwd, raw, parseNamedArgs, error }) { return parsedPlans; }; + // Workstream guard: if GSD_WORKSTREAM is set, the sync bridge worker cannot + // handle the request (GSDTransport.subprocessReason returns 'workstream_forced' + // and subprocess is disabled in the worker). Fall back to CJS path for all + // workstream-scoped state commands. + const activeWorkstream = process.env.GSD_WORKSTREAM; + const sdkAvailable = !activeWorkstream && tryLoadSdk(); + + // Helper: build SDK-backed handler that falls through to CJS on SDK failure. + // cjsFallback is called when SDK is unavailable or when the subcommand has no + // SDK counterpart. + function sdkHandler(registryCommand, registryArgs, legacyArgs, rawFormatter, cjsFallback) { + if (!sdkAvailable) return cjsFallback; + return () => { + const handled = dispatchViaSdk( + registryCommand, registryArgs, legacyArgs, cwd, raw, error, rawFormatter, + ); + if (!handled) cjsFallback(); + }; + } + routeCjsCommandFamily({ args, subcommands: ['load', 'complete-phase', ...STATE_SUBCOMMANDS.filter((s) => s !== 'load')], @@ -27,70 +123,191 @@ function routeStateCommand({ state, args, cwd, raw, parseNamedArgs, error }) { error, unknownMessage: (subcommand, available) => `Unknown state subcommand: "${subcommand}". Available: ${available.join(', ')}`, handlers: { - load: () => state.cmdStateLoad(cwd, raw), - json: () => state.cmdStateJson(cwd, raw), - update: () => state.cmdStateUpdate(cwd, args[2], args[3]), - get: () => state.cmdStateGet(cwd, args[2], raw), - patch: () => { - const patches = {}; - for (let i = 2; i < args.length; i += 2) { - const key = args[i].replace(/^--/, ''); - const value = args[i + 1]; - if (key && value !== undefined) { - patches[key] = value; + load: sdkHandler( + 'state.load', + [], + args.slice(1), + _formatStateLoadRawStdout, + () => state.cmdStateLoad(cwd, raw), + ), + json: sdkHandler( + 'state.json', + [], + args.slice(1), + null, + () => state.cmdStateJson(cwd, raw), + ), + get: sdkHandler( + 'state.get', + args.slice(2), + args.slice(1), + null, + () => state.cmdStateGet(cwd, args[2], raw), + ), + update: sdkHandler( + 'state.update', + args.slice(2), + args.slice(1), + null, + () => state.cmdStateUpdate(cwd, args[2], args[3]), + ), + patch: sdkHandler( + 'state.patch', + args.slice(2), + args.slice(1), + null, + () => { + const patches = {}; + for (let i = 2; i < args.length; i += 2) { + const key = args[i].replace(/^--/, ''); + const value = args[i + 1]; + if (key && value !== undefined) { + patches[key] = value; + } } - } - state.cmdStatePatch(cwd, patches, raw); - }, - 'advance-plan': () => state.cmdStateAdvancePlan(cwd, raw), - 'record-metric': () => { - const { phase: p, plan, duration, tasks, files } = parseNamedArgs(args, ['phase', 'plan', 'duration', 'tasks', 'files']); - state.cmdStateRecordMetric(cwd, { phase: p, plan, duration, tasks, files }, raw); - }, - 'update-progress': () => state.cmdStateUpdateProgress(cwd, raw), - 'add-decision': () => { - const { phase: p, summary, 'summary-file': summary_file, rationale, 'rationale-file': rationale_file } = parseNamedArgs(args, ['phase', 'summary', 'summary-file', 'rationale', 'rationale-file']); - state.cmdStateAddDecision(cwd, { phase: p, summary, summary_file, rationale: rationale || '', rationale_file }, raw); - }, - 'add-blocker': () => { - const { text, 'text-file': text_file } = parseNamedArgs(args, ['text', 'text-file']); - state.cmdStateAddBlocker(cwd, { text, text_file }, raw); - }, - 'resolve-blocker': () => state.cmdStateResolveBlocker(cwd, parseNamedArgs(args, ['text']).text, raw), - 'record-session': () => { - const { 'stopped-at': stopped_at, 'resume-file': resume_file } = parseNamedArgs(args, ['stopped-at', 'resume-file']); - state.cmdStateRecordSession(cwd, { stopped_at, resume_file: resume_file || 'None' }, raw); - }, - 'begin-phase': () => { - const { phase: p, name, plans } = parseNamedArgs(args, ['phase', 'name', 'plans']); - state.cmdStateBeginPhase(cwd, p, name, parsePlans(plans), raw); - }, - 'signal-waiting': () => { - const { type, question, options, phase: p } = parseNamedArgs(args, ['type', 'question', 'options', 'phase']); - state.cmdSignalWaiting(cwd, type, question, options, p, raw); - }, - 'signal-resume': () => state.cmdSignalResume(cwd, raw), - 'planned-phase': () => { - const { phase: p, plans } = parseNamedArgs(args, ['phase', 'name', 'plans']); - state.cmdStatePlannedPhase(cwd, p, parsePlans(plans), raw); - }, - validate: () => state.cmdStateValidate(cwd, raw), - sync: () => { - const { verify } = parseNamedArgs(args, [], ['verify']); - state.cmdStateSync(cwd, { verify }, raw); - }, - prune: () => { - const { 'keep-recent': keepRecent, 'dry-run': dryRun } = parseNamedArgs(args, ['keep-recent'], ['dry-run']); - state.cmdStatePrune(cwd, { keepRecent: keepRecent || '3', dryRun: !!dryRun }, raw); - }, + state.cmdStatePatch(cwd, patches, raw); + }, + ), + 'advance-plan': sdkHandler( + 'state.advance-plan', + [], + args.slice(1), + null, + () => state.cmdStateAdvancePlan(cwd, raw), + ), + 'record-metric': sdkHandler( + 'state.record-metric', + args.slice(2), + args.slice(1), + null, + () => { + const { phase: p, plan, duration, tasks, files } = parseNamedArgs(args, ['phase', 'plan', 'duration', 'tasks', 'files']); + state.cmdStateRecordMetric(cwd, { phase: p, plan, duration, tasks, files }, raw); + }, + ), + 'update-progress': sdkHandler( + 'state.update-progress', + [], + args.slice(1), + null, + () => state.cmdStateUpdateProgress(cwd, raw), + ), + 'add-decision': sdkHandler( + 'state.add-decision', + args.slice(2), + args.slice(1), + null, + () => { + const { phase: p, summary, 'summary-file': summary_file, rationale, 'rationale-file': rationale_file } = parseNamedArgs(args, ['phase', 'summary', 'summary-file', 'rationale', 'rationale-file']); + state.cmdStateAddDecision(cwd, { phase: p, summary, summary_file, rationale: rationale || '', rationale_file }, raw); + }, + ), + 'add-blocker': sdkHandler( + 'state.add-blocker', + args.slice(2), + args.slice(1), + null, + () => { + const { text, 'text-file': text_file } = parseNamedArgs(args, ['text', 'text-file']); + state.cmdStateAddBlocker(cwd, { text, text_file }, raw); + }, + ), + 'resolve-blocker': sdkHandler( + 'state.resolve-blocker', + args.slice(2), + args.slice(1), + null, + () => state.cmdStateResolveBlocker(cwd, parseNamedArgs(args, ['text']).text, raw), + ), + 'record-session': sdkHandler( + 'state.record-session', + args.slice(2), + args.slice(1), + null, + () => { + const { 'stopped-at': stopped_at, 'resume-file': resume_file } = parseNamedArgs(args, ['stopped-at', 'resume-file']); + state.cmdStateRecordSession(cwd, { stopped_at, resume_file: resume_file || 'None' }, raw); + }, + ), + 'begin-phase': sdkHandler( + 'state.begin-phase', + args.slice(2), + args.slice(1), + null, + () => { + const { phase: p, name, plans } = parseNamedArgs(args, ['phase', 'name', 'plans']); + state.cmdStateBeginPhase(cwd, p, name, parsePlans(plans), raw); + }, + ), + 'signal-waiting': sdkHandler( + 'state.signal-waiting', + args.slice(2), + args.slice(1), + null, + () => { + const { type, question, options, phase: p } = parseNamedArgs(args, ['type', 'question', 'options', 'phase']); + state.cmdSignalWaiting(cwd, type, question, options, p, raw); + }, + ), + 'signal-resume': sdkHandler( + 'state.signal-resume', + [], + args.slice(1), + null, + () => state.cmdSignalResume(cwd, raw), + ), + 'planned-phase': sdkHandler( + 'state.planned-phase', + args.slice(2), + args.slice(1), + null, + () => { + const { phase: p, plans } = parseNamedArgs(args, ['phase', 'name', 'plans']); + state.cmdStatePlannedPhase(cwd, p, parsePlans(plans), raw); + }, + ), + validate: sdkHandler( + 'state.validate', + [], + args.slice(1), + null, + () => state.cmdStateValidate(cwd, raw), + ), + sync: sdkHandler( + 'state.sync', + args.slice(2), + args.slice(1), + null, + () => { + const { verify } = parseNamedArgs(args, [], ['verify']); + state.cmdStateSync(cwd, { verify }, raw); + }, + ), + prune: sdkHandler( + 'state.prune', + args.slice(2), + args.slice(1), + null, + () => { + const { 'keep-recent': keepRecent, 'dry-run': dryRun } = parseNamedArgs(args, ['keep-recent'], ['dry-run']); + state.cmdStatePrune(cwd, { keepRecent: keepRecent || '3', dryRun: !!dryRun }, raw); + }, + ), + // complete-phase: CJS-only — no SDK counterpart. 'complete-phase': () => { const { phase: p } = parseNamedArgs(args, ['phase']); state.cmdStateCompletePhase(cwd, raw, p || args[2]); }, - 'milestone-switch': () => { - const { milestone, name } = parseNamedArgs(args, ['milestone', 'name']); - state.cmdStateMilestoneSwitch(cwd, milestone, name, raw); - }, + 'milestone-switch': sdkHandler( + 'state.milestone-switch', + args.slice(2), + args.slice(1), + null, + () => { + const { milestone, name } = parseNamedArgs(args, ['milestone', 'name']); + state.cmdStateMilestoneSwitch(cwd, milestone, name, raw); + }, + ), }, }); } diff --git a/sdk/src/golden/golden.integration.test.ts b/sdk/src/golden/golden.integration.test.ts index e13f9e76c..47216490d 100644 --- a/sdk/src/golden/golden.integration.test.ts +++ b/sdk/src/golden/golden.integration.test.ts @@ -312,6 +312,235 @@ describe('Golden file tests', () => { const sdkResult = await registry.dispatch('state.sync', ['--verify'], tmpDir); expect(sdkResult.data).toEqual(gsdOutput); }); + + // ─── Phase 5.1: 12 additional state subcommand parity tests ──────────── + + it('state.advance-plan matches gsd-tools.cjs', async () => { + // Setup: add compound Plan field so advance-plan can parse it + const statePath = join(tmpDir, '.planning', 'STATE.md'); + const content = await readFile(statePath, 'utf-8'); + await writeFile(statePath, content + '\nPlan: 2 of 3\n', 'utf-8'); + const gsdOutput = await captureGsdToolsOutput('state', ['advance-plan'], tmpDir); + // Restore and re-apply for SDK call + await writeFile(statePath, content + '\nPlan: 2 of 3\n', 'utf-8'); + const registry = createRegistry(); + const sdkResult = await registry.dispatch('state.advance-plan', [], tmpDir); + // Both advance plan: compare shape (times may differ slightly but structure matches) + expect(typeof sdkResult.data).toBe('object'); + const sdkData = sdkResult.data as Record; + const gsdData = gsdOutput as Record; + expect(sdkData.advanced).toBe(gsdData.advanced); + if (sdkData.advanced) { + expect(typeof sdkData.current_plan).toBe('number'); + expect(typeof sdkData.previous_plan).toBe('number'); + } + }); + + it('state.update-progress matches gsd-tools.cjs', async () => { + // Both update the progress bar. Phase dir is empty so percent=0. + const gsdOutput = await captureGsdToolsOutput('state', ['update-progress'], tmpDir); + // Restore state for SDK call (CJS mutates the file) + const statePath = join(tmpDir, '.planning', 'STATE.md'); + await writeFile(statePath, MINIMAL_STATE, 'utf-8'); + const registry = createRegistry(); + const sdkResult = await registry.dispatch('state.update-progress', [], tmpDir); + expect(sdkResult.data).toEqual(gsdOutput); + }); + + it('state.add-decision matches gsd-tools.cjs', async () => { + // Setup: add a Decisions section to STATE.md body + const statePath = join(tmpDir, '.planning', 'STATE.md'); + const withDecisions = MINIMAL_STATE + '\n## Decisions\n\nNone yet.\n'; + await writeFile(statePath, withDecisions, 'utf-8'); + const argv = ['add-decision', '--phase', '10', '--summary', 'SDK parity decision']; + const gsdOutput = await captureGsdToolsOutput('state', argv, tmpDir); + await writeFile(statePath, withDecisions, 'utf-8'); + const registry = createRegistry(); + const sdkResult = await registry.dispatch('state.add-decision', ['--phase', '10', '--summary', 'SDK parity decision'], tmpDir); + expect(sdkResult.data).toEqual(gsdOutput); + }); + + it('state.add-blocker matches gsd-tools.cjs', async () => { + // Setup: add a Blockers section to STATE.md body + const statePath = join(tmpDir, '.planning', 'STATE.md'); + const withBlockers = MINIMAL_STATE + '\n## Blockers\n\nNone\n'; + await writeFile(statePath, withBlockers, 'utf-8'); + const argv = ['add-blocker', '--text', 'SDK parity blocker']; + const gsdOutput = await captureGsdToolsOutput('state', argv, tmpDir); + await writeFile(statePath, withBlockers, 'utf-8'); + const registry = createRegistry(); + const sdkResult = await registry.dispatch('state.add-blocker', ['--text', 'SDK parity blocker'], tmpDir); + expect(sdkResult.data).toEqual(gsdOutput); + }); + + it('state.resolve-blocker matches gsd-tools.cjs', async () => { + // Setup: add a Blockers section that has a blocker entry to remove + const statePath = join(tmpDir, '.planning', 'STATE.md'); + const withBlocker = MINIMAL_STATE + '\n## Blockers\n\n- SDK parity blocker to resolve\n'; + await writeFile(statePath, withBlocker, 'utf-8'); + const argv = ['resolve-blocker', '--text', 'SDK parity blocker to resolve']; + const gsdOutput = await captureGsdToolsOutput('state', argv, tmpDir); + await writeFile(statePath, withBlocker, 'utf-8'); + const registry = createRegistry(); + const sdkResult = await registry.dispatch('state.resolve-blocker', ['--text', 'SDK parity blocker to resolve'], tmpDir); + expect(sdkResult.data).toEqual(gsdOutput); + }); + + it('state.record-session matches gsd-tools.cjs', async () => { + // Setup: add session fields to STATE.md body + const statePath = join(tmpDir, '.planning', 'STATE.md'); + const withSession = MINIMAL_STATE + '\nLast session: 2026-05-01T00:00:00.000Z\n'; + await writeFile(statePath, withSession, 'utf-8'); + const argv = ['record-session', '--stopped-at', 'plan 2 done']; + const gsdOutput = await captureGsdToolsOutput('state', argv, tmpDir); + // SDK writes timestamp — compare shape not exact value + const registry = createRegistry(); + await writeFile(statePath, withSession, 'utf-8'); + const sdkResult = await registry.dispatch('state.record-session', ['--stopped-at', 'plan 2 done'], tmpDir); + const sdkData = sdkResult.data as Record; + const gsdData = gsdOutput as Record; + // Both should agree on recorded:true/false shape + expect(sdkData.recorded).toBe(gsdData.recorded); + if (sdkData.recorded && gsdData.recorded) { + expect(Array.isArray(sdkData.updated)).toBe(true); + expect(Array.isArray(gsdData.updated)).toBe(true); + expect((sdkData.updated as string[]).sort()).toEqual((gsdData.updated as string[]).sort()); + } + }); + + it('state.signal-waiting matches gsd-tools.cjs', async () => { + const argv = ['signal-waiting', '--type', 'decision_point', '--question', 'Which SDK approach?', '--phase', '10']; + const gsdOutput = await captureGsdToolsOutput('state', argv, tmpDir); + const registry = createRegistry(); + const sdkResult = await registry.dispatch('state.signal-waiting', ['--type', 'decision_point', '--question', 'Which SDK approach?', '--phase', '10'], tmpDir); + const sdkData = sdkResult.data as Record; + const gsdData = gsdOutput as Record; + // Both write WAITING.json — compare structural fields, not timestamp or exact paths + expect(sdkData.signaled).toBe(gsdData.signaled); + expect(typeof sdkData.path).toBe('string'); + expect(typeof gsdData.path).toBe('string'); + }); + + it('state.signal-resume matches gsd-tools.cjs', async () => { + // First signal so resume has something to remove + const gsdDir2 = join(tmpdir(), `gsd-golden-state-resume-gsd-${Date.now()}`); + const sdkDir2 = join(tmpdir(), `gsd-golden-state-resume-sdk-${Date.now()}`); + try { + await setupMinimalStateProject(gsdDir2); + await setupMinimalStateProject(sdkDir2); + // Signal in both dirs first + await captureGsdToolsOutput('state', ['signal-waiting', '--type', 'review'], gsdDir2); + const registry1 = createRegistry(); + await registry1.dispatch('state.signal-waiting', ['--type', 'review'], sdkDir2); + // Now resume + const gsdOutput = await captureGsdToolsOutput('state', ['signal-resume'], gsdDir2); + const registry2 = createRegistry(); + const sdkResult = await registry2.dispatch('state.signal-resume', [], sdkDir2); + expect(sdkResult.data).toEqual(gsdOutput); + } finally { + await rm(gsdDir2, { recursive: true, force: true }); + await rm(sdkDir2, { recursive: true, force: true }); + } + }); + + it('state.planned-phase matches gsd-tools.cjs', async () => { + const argv = ['planned-phase', '--phase', '11', '--plans', '4']; + const gsdOutput = await captureGsdToolsOutput('state', argv, tmpDir); + const statePath = join(tmpDir, '.planning', 'STATE.md'); + await writeFile(statePath, MINIMAL_STATE, 'utf-8'); + const registry = createRegistry(); + const sdkResult = await registry.dispatch('state.planned-phase', ['--phase', '11', '--plans', '4'], tmpDir); + expect(sdkResult.data).toEqual(gsdOutput); + }); + + it('state.milestone-switch matches gsd-tools.cjs', async () => { + const gsdDir2 = join(tmpdir(), `gsd-golden-state-ms-gsd-${Date.now()}`); + const sdkDir2 = join(tmpdir(), `gsd-golden-state-ms-sdk-${Date.now()}`); + try { + await setupMinimalStateProject(gsdDir2); + await setupMinimalStateProject(sdkDir2); + const argv = ['milestone-switch', '--milestone', 'v4.0', '--name', 'Next Milestone']; + const gsdOutput = await captureGsdToolsOutput('state', argv, gsdDir2); + const registry = createRegistry(); + const sdkResult = await registry.dispatch('state.milestone-switch', ['--milestone', 'v4.0', '--name', 'Next Milestone'], sdkDir2); + // Both return {switched:true, milestone, name} — compare structural shape + const sdkData = sdkResult.data as Record; + const gsdData = gsdOutput as Record; + expect(sdkData.switched).toBe(gsdData.switched); + expect(sdkData.version).toBe(gsdData.version); + expect(sdkData.name).toBe(gsdData.name); + } finally { + await rm(gsdDir2, { recursive: true, force: true }); + await rm(sdkDir2, { recursive: true, force: true }); + } + }); + + it('state.prune dry-run matches gsd-tools.cjs', async () => { + // Prune needs a parseable current_phase. Use fresh dirs with a STATE.md + // whose frontmatter includes current_phase so both CJS and SDK agree. + // CJS extracts current phase from disk-counted phases (result: 0 phases → "Only 0 phases..."), + // SDK extracts from frontmatter current_phase field. + // Use only 2 keepRecent phases, leaving phases dir empty so CJS reports "Only 0 phases" + // and SDK also bails early (current_phase=10, cutoff=7, but no phases to scan → same reason). + // Align via a fixture that has current_phase in frontmatter AND no phases on disk. + const gsdDir2 = join(tmpdir(), `gsd-golden-prune-gsd-${Date.now()}`); + const sdkDir2 = join(tmpdir(), `gsd-golden-prune-sdk-${Date.now()}`); + // Minimal state — no phases on disk, prune returns "Only N phases — nothing to prune" + try { + await setupMinimalStateProject(gsdDir2); + await setupMinimalStateProject(sdkDir2); + const argv = ['prune', '--keep-recent', '3', '--dry-run']; + const gsdOutput = await captureGsdToolsOutput('state', argv, gsdDir2); + const registry = createRegistry(); + const sdkResult = await registry.dispatch('state.prune', ['--keep-recent', '3', '--dry-run'], sdkDir2); + // Both should return pruned:false. Exact reason may differ (CJS: phase count from disk; + // SDK: phase count from frontmatter). Compare just the structural result. + const sdkData = sdkResult.data as Record; + const gsdData = gsdOutput as Record; + expect(sdkData.pruned).toBe(false); + expect(gsdData.pruned).toBe(false); + expect(typeof sdkData.reason).toBe('string'); + expect(typeof gsdData.reason).toBe('string'); + } finally { + await rm(gsdDir2, { recursive: true, force: true }); + await rm(sdkDir2, { recursive: true, force: true }); + } + }); + + it('state.record-metric matches gsd-tools.cjs (no-metrics-section → divergence documented)', async () => { + // Divergence: CJS auto-creates the Performance Metrics section when absent; + // SDK returns { recorded: false, reason: '...' }. We test both via fresh dirs + // and add a metrics section to align behavior for parity. + const gsdDir2 = join(tmpdir(), `gsd-golden-state-metric-gsd-${Date.now()}`); + const sdkDir2 = join(tmpdir(), `gsd-golden-state-metric-sdk-${Date.now()}`); + try { + const metricsState = MINIMAL_STATE + [ + '', + '## Performance Metrics', + '', + '| Phase | Plan | Duration | Notes |', + '|-------|------|----------|-------|', + '', + ].join('\n'); + await mkdir(join(gsdDir2, '.planning', 'phases'), { recursive: true }); + await writeFile(join(gsdDir2, '.planning', 'STATE.md'), metricsState, 'utf-8'); + await writeFile(join(gsdDir2, '.planning', 'ROADMAP.md'), '# Roadmap\n', 'utf-8'); + await writeFile(join(gsdDir2, '.planning', 'config.json'), '{"model_profile":"balanced"}', 'utf-8'); + await mkdir(join(sdkDir2, '.planning', 'phases'), { recursive: true }); + await writeFile(join(sdkDir2, '.planning', 'STATE.md'), metricsState, 'utf-8'); + await writeFile(join(sdkDir2, '.planning', 'ROADMAP.md'), '# Roadmap\n', 'utf-8'); + await writeFile(join(sdkDir2, '.planning', 'config.json'), '{"model_profile":"balanced"}', 'utf-8'); + + const argv = ['record-metric', '--phase', '10', '--plan', '1', '--duration', '45m', '--tasks', '12', '--files', '8']; + const gsdOutput = await captureGsdToolsOutput('state', argv, gsdDir2); + const registry = createRegistry(); + const sdkResult = await registry.dispatch('state.record-metric', ['--phase', '10', '--plan', '1', '--duration', '45m', '--tasks', '12', '--files', '8'], sdkDir2); + expect(sdkResult.data).toEqual(gsdOutput); + } finally { + await rm(gsdDir2, { recursive: true, force: true }); + await rm(sdkDir2, { recursive: true, force: true }); + } + }); }); describe('phase mutations (subprocess parity)', () => { diff --git a/sdk/src/runtime-bridge-sync/index.test.ts b/sdk/src/runtime-bridge-sync/index.test.ts index 642c409cc..4c297a429 100644 --- a/sdk/src/runtime-bridge-sync/index.test.ts +++ b/sdk/src/runtime-bridge-sync/index.test.ts @@ -81,19 +81,21 @@ describe('executeForCjs - sync primitive', () => { it('native_failure: handler execution failure is classified as native_failure', () => { // generate-slug with no args throws a GSDError (validation) — that maps to validation_error. - // We need a command that throws a plain Error. The 'current-timestamp' command with - // an invalid format that causes a runtime failure should work. Instead, let's directly - // test the bridge's behavior when the execution policy throws a non-TypeError GSDToolsError. + // We need a command that throws a plain Error (GSDToolsError classification.kind='failure'). // - // We'll use 'frontmatter.get' with a non-existent file path that causes a file read failure. - // That should result in native_failure. + // Phase 5.1 fix note: the Phase 5.0 fixture used projectDir='/tmp' with an absolute + // path arg that started with /tmp — after the worker fix threads projectDir correctly, + // frontmatter.get returns a soft ok:true error instead of throwing (path escape check + // passes, then realpath on the nonexistent path returns ok:true with error field). + // Updated fixture: use a completely nonexistent projectDir so resolvePathUnderProject + // calls realpath('/nonexistent...') and throws ENOENT, which is classified as native_failure. const result = executeForCjs({ registryCommand: 'frontmatter.get', - registryArgs: ['/tmp/__definitely_does_not_exist_abc123/file.md'], + registryArgs: ['file.md'], legacyCommand: 'frontmatter get', - legacyArgs: ['/tmp/__definitely_does_not_exist_abc123/file.md'], + legacyArgs: ['file.md'], mode: 'json', - projectDir: '/tmp', + projectDir: '/nonexistent-absolutely-does-not-exist-project-dir', }); expect(result.ok).toBe(false); diff --git a/sdk/src/runtime-bridge-sync/projectdir-regression.test.ts b/sdk/src/runtime-bridge-sync/projectdir-regression.test.ts new file mode 100644 index 000000000..3ae741446 --- /dev/null +++ b/sdk/src/runtime-bridge-sync/projectdir-regression.test.ts @@ -0,0 +1,151 @@ +/** + * Regression test for the Phase 5.0 worker bug: projectDir and workstream were + * dropped from RuntimeBridgeExecuteInput before being forwarded to + * registry.dispatch(). The worker constructed a module-scoped + * QueryNativeDirectAdapter with a hardcoded projectDir='' — meaning any handler + * that reads .planning/ (e.g. state.*) would either fail silently or read from + * the process CWD rather than the requested project directory. + * + * Fix (Phase 5.1): the adapter is now constructed per-request inside + * dispatchNative so request.projectDir and request.workstream close over the + * correct values. + * + * These tests must: + * - FAIL against the unfixed worker (projectDir='', handler sees wrong dir). + * - PASS against the fixed worker (projectDir threaded correctly). + * + * NOTE: executeForCjs uses a compiled dist/ worker (see index.ts comments). + * The tests here call executeForCjs, which requires the worker to be rebuilt + * before the fix is observable. Run `npm run build` in sdk/ first. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { mkdir, writeFile, rm } from 'node:fs/promises'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import { executeForCjs } from './index.js'; + +// ─── Fixture STATE.md with parseable frontmatter ────────────────────────── + +const FIXTURE_STATE = `--- +gsd_state_version: 1.0 +milestone: v9.1 +milestone_name: Regression Test Milestone +status: executing +--- + +# Project State + +## Current Position + +Phase: 9 (Regression Tests) — EXECUTING +Plan: 1 of 2 +Status: Executing Phase 9 +Last activity: 2026-05-15 -- Regression test started + +Progress: [█████░░░░░] 50% +`; + +// ─── Helpers ─────────────────────────────────────────────────────────────── + +let tmpDir: string; + +beforeAll(async () => { + tmpDir = join( + tmpdir(), + `gsd-projectdir-regression-${Date.now()}-${Math.random().toString(36).slice(2)}`, + ); + await mkdir(join(tmpDir, '.planning'), { recursive: true }); + await writeFile(join(tmpDir, '.planning', 'STATE.md'), FIXTURE_STATE, 'utf-8'); +}); + +afterAll(async () => { + await rm(tmpDir, { recursive: true, force: true }); +}); + +// ─── Tests ───────────────────────────────────────────────────────────────── + +describe('executeForCjs projectDir regression (Phase 5.0 bug)', () => { + it('threads projectDir to the handler: state.json returns frontmatter data from the tmpdir fixture', () => { + // This test FAILS against the unfixed worker because projectDir='' causes + // the handler to look for .planning/STATE.md relative to '' (process CWD), + // which does not have a STATE.md fixture. The handler returns { error: 'STATE.md not found' }. + // + // With the fix, projectDir=tmpDir is forwarded and the handler reads the fixture. + const result = executeForCjs({ + registryCommand: 'state.json', + registryArgs: [], + legacyCommand: 'state', + legacyArgs: ['json'], + mode: 'json', + projectDir: tmpDir, + }); + + expect(result.ok).toBe(true); + if (!result.ok) return; // narrow for TS + + const data = result.data as Record; + + // The handler should have found the fixture and returned parsed frontmatter. + // Key assertions: these fields come from FIXTURE_STATE and are absent from + // any STATE.md that might exist at ''. + expect(data).not.toHaveProperty('error'); + expect(data.milestone).toBe('v9.1'); + expect(data.milestone_name).toBe('Regression Test Milestone'); + expect(data.status).toBe('executing'); + }); + + it('negative: nonexistent projectDir returns ok:true with {error} (handler-level not-found)', () => { + // A completely nonexistent directory: handler cannot find .planning/STATE.md + // and returns a structured error payload rather than throwing. This is the + // expected "soft failure" shape for state.json on a missing project. + const result = executeForCjs({ + registryCommand: 'state.json', + registryArgs: [], + legacyCommand: 'state', + legacyArgs: ['json'], + mode: 'json', + projectDir: '/nonexistent-gsd-project-regression-test-dir', + }); + + // The handler returns { data: { error: 'STATE.md not found' } } — ok:true + // because it is a domain-level not-found, not a dispatch error. + expect(result.ok).toBe(true); + if (!result.ok) return; + + const data = result.data as Record; + expect(data).toHaveProperty('error'); + expect(String(data.error)).toMatch(/STATE\.md not found/i); + }); + + it('workstream transport contract: GSDTransport forces subprocess for workstream requests (subprocess disabled in worker → ok:false)', () => { + // This test documents an architectural constraint, not a bug. + // + // GSDTransport.subprocessReason() returns 'workstream_forced' when + // request.workstream is set (gsd-transport.ts line ~72). The worker has + // subprocess disabled (allowFallbackToSubprocess=false), so a workstream + // request always surfaces as ok:false / internal_error. + // + // This is the expected contract for the sync bridge worker: workstream + // scoped commands cannot run natively in the worker and must be invoked + // via the async bridge or gsd-tools.cjs subprocess fallback instead. + // + // This test is here to document + pin the behavior, not to assert a fix. + const result = executeForCjs({ + registryCommand: 'state.json', + registryArgs: [], + legacyCommand: 'state', + legacyArgs: ['json'], + mode: 'json', + projectDir: tmpDir, + workstream: 'some-workstream', + }); + + // Workstream forces subprocess; subprocess disabled → ok:false. + expect(result.ok).toBe(false); + if (result.ok) return; + // The error surfaces as internal_error because 'Subprocess fallback disabled' + // does not match the unknown_command classifier pattern. + expect(['internal_error', 'unknown_command']).toContain(result.errorKind); + }); +}); diff --git a/sdk/src/runtime-bridge-sync/worker.ts b/sdk/src/runtime-bridge-sync/worker.ts index 0d6a601cb..b3c5f0e21 100644 --- a/sdk/src/runtime-bridge-sync/worker.ts +++ b/sdk/src/runtime-bridge-sync/worker.ts @@ -36,21 +36,27 @@ function getBridge(): QueryRuntimeBridge { const NATIVE_TIMEOUT_MS = 30_000; // 30 s ceiling for any single handler const nativeErrorFactory = createQueryNativeErrorFactory(NATIVE_TIMEOUT_MS); - const nativeDirectAdapter = new QueryNativeDirectAdapter({ - timeoutMs: NATIVE_TIMEOUT_MS, - dispatch: (registryCommand, registryArgs) => - registry.dispatch(registryCommand, registryArgs, ''), - ...nativeErrorFactory, - }); - + // Build a per-request adapter inside dispatchNative so that projectDir and + // workstream from the request close over the correct values. The Phase 5.0 + // bug was a module-scoped adapter that hardcoded projectDir = '' — any + // handler reading .planning/ (e.g. state.*) received an empty path and + // silently failed or read from the process CWD. Constructing per-request + // adds microseconds; correctness wins. (fix for latent bug, Phase 5.1) const transport = new GSDTransport(registry, { - dispatchNative: (request) => - nativeDirectAdapter.dispatchResult( + dispatchNative: (request) => { + const adapter = new QueryNativeDirectAdapter({ + timeoutMs: NATIVE_TIMEOUT_MS, + dispatch: (registryCommand, registryArgs) => + registry.dispatch(registryCommand, registryArgs, request.projectDir, request.workstream), + ...nativeErrorFactory, + }); + return adapter.dispatchResult( request.legacyCommand, request.legacyArgs, request.registryCommand, request.registryArgs, - ), + ); + }, // Subprocess fallback stubs — never called because allowFallbackToSubprocess=false execSubprocessJson: () => Promise.reject(new Error('Subprocess fallback disabled in sync bridge worker')), @@ -60,10 +66,18 @@ function getBridge(): QueryRuntimeBridge { const executionPolicy = new QueryExecutionPolicy(transport); - // Hotpath adapter stubs — dispatchHotpath is not called by executeForCjs + // Hotpath adapter: construct a stub that satisfies the QueryRuntimeBridge + // constructor. executeForCjs does not invoke dispatchHotpath so this + // adapter is never actually called. We still need a valid instance because + // QueryRuntimeBridge requires one at construction time. + const stubDirectAdapter = new QueryNativeDirectAdapter({ + timeoutMs: NATIVE_TIMEOUT_MS, + dispatch: () => Promise.reject(new Error('stub: hotpath direct adapter not used')), + ...nativeErrorFactory, + }); const hotpathAdapter = new QueryNativeHotpathAdapter( () => true, - nativeDirectAdapter, + stubDirectAdapter, () => Promise.reject(new Error('hotpath json fallback disabled')), () => Promise.reject(new Error('hotpath raw fallback disabled')), );