From 42ed7cee8d8d3ad46d360af7fb0260e178fde1f4 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 3 May 2026 18:56:41 -0400 Subject: [PATCH] refactor: deepen GSDTools query execution seams (#3085) * refactor: deepen gsdtools query execution seams * docs: add changeset for query seam deepening * docs: fix changeset summary text * fix: address coderabbit query seam findings * test: address remaining coderabbit findings and notes * refactor: use internal gsdtools error type import --- .changeset/tidy-tunas-zip.md | 5 + CONTEXT.md | 6 + sdk/src/config.test.ts | 11 +- sdk/src/e2e.integration.test.ts | 4 +- .../read-only-parity.integration.test.ts | 16 +- sdk/src/gsd-tools-error.ts | 13 + sdk/src/gsd-tools.ts | 466 +++--------------- sdk/src/query-command-executor.ts | 31 ++ sdk/src/query-execution-policy.test.ts | 31 ++ sdk/src/query-execution-policy.ts | 42 ++ sdk/src/query-gsd-tools-path.ts | 24 + sdk/src/query-gsd-tools-runtime.ts | 67 +++ sdk/src/query-hotpath-methods.ts | 48 ++ sdk/src/query-native-direct-adapter.ts | 50 ++ sdk/src/query-native-hotpath-adapter.test.ts | 43 ++ sdk/src/query-native-hotpath-adapter.ts | 31 ++ sdk/src/query-raw-output-projection.test.ts | 34 ++ sdk/src/query-raw-output-projection.ts | 69 +++ sdk/src/query-subprocess-adapter.test.ts | 71 +++ sdk/src/query-subprocess-adapter.ts | 159 ++++++ sdk/src/query-tools-error-mapper.ts | 28 ++ sdk/src/query/init.ts | 57 +-- sdk/src/query/state-mutation.test.ts | 8 + sdk/src/query/state-mutation.ts | 2 +- 24 files changed, 871 insertions(+), 445 deletions(-) create mode 100644 .changeset/tidy-tunas-zip.md create mode 100644 sdk/src/gsd-tools-error.ts create mode 100644 sdk/src/query-command-executor.ts create mode 100644 sdk/src/query-execution-policy.test.ts create mode 100644 sdk/src/query-execution-policy.ts create mode 100644 sdk/src/query-gsd-tools-path.ts create mode 100644 sdk/src/query-gsd-tools-runtime.ts create mode 100644 sdk/src/query-hotpath-methods.ts create mode 100644 sdk/src/query-native-direct-adapter.ts create mode 100644 sdk/src/query-native-hotpath-adapter.test.ts create mode 100644 sdk/src/query-native-hotpath-adapter.ts create mode 100644 sdk/src/query-raw-output-projection.test.ts create mode 100644 sdk/src/query-raw-output-projection.ts create mode 100644 sdk/src/query-subprocess-adapter.test.ts create mode 100644 sdk/src/query-subprocess-adapter.ts create mode 100644 sdk/src/query-tools-error-mapper.ts diff --git a/.changeset/tidy-tunas-zip.md b/.changeset/tidy-tunas-zip.md new file mode 100644 index 000000000..775bda591 --- /dev/null +++ b/.changeset/tidy-tunas-zip.md @@ -0,0 +1,5 @@ +--- +type: Changed +pr: 3085 +--- +**`GSDTools` query execution internals now use deep Module seams** — refactors runtime composition, native/subprocess adapters, and output projection behind stable public interfaces for better locality and testability. diff --git a/CONTEXT.md b/CONTEXT.md index 8734180fe..56446772e 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -25,6 +25,12 @@ Adapter Module that satisfies native query dispatch at the Dispatch Policy seam, ### Query CLI Output Module Module owning projection from dispatch results/errors to CLI `{ exitCode, stdoutChunks, stderrLines }` output contract. +### Query Execution Policy Module +Module owning query transport routing policy projection (`preferNative`, fallback policy, workstream subprocess forcing) at execution seam. + +### Query Subprocess Adapter Module +Adapter Module owning subprocess execution contract for query commands (JSON/raw invocation, `@file:` indirection parsing, timeout/exit error projection). + ### Query Command Resolution Module Canonical command normalization and resolution Interface (`query-command-resolution-strategy`) used by internal query/transport paths after dead-wrapper convergence. diff --git a/sdk/src/config.test.ts b/sdk/src/config.test.ts index 679a7f2b4..a4d81749c 100644 --- a/sdk/src/config.test.ts +++ b/sdk/src/config.test.ts @@ -181,11 +181,12 @@ describe('loadConfig', () => { // model aliases from MODEL_PROFILES via resolveModel even when the user // had `resolve_model_ids: "omit"` in ~/.gsd/defaults.json. // - // Mirrors CJS behavior in get-shit-done/bin/lib/core.cjs:421 (#1683): - // user-level defaults only apply when no project .planning/config.json - // exists (pre-project context). Once a project is initialized, its - // config.json is authoritative — buildNewProjectConfig baked the user - // defaults in at /gsd:new-project time. + // Mirrors current CJS parity expectations for SDK loadConfig + resolveModel: + // in pre-project context, loadConfig ignores ~/.gsd/defaults.json so + // resolveModel/MODEL_PROFILES do not emit aliases when resolve_model_ids + // is "omit". Once a project is initialized, config.json is authoritative, + // because buildNewProjectConfig bakes user defaults into project config + // at /gsd:new-project time. it('pre-project: ignores user defaults and uses built-in defaults', async () => { await writeUserDefaults({ resolve_model_ids: 'omit' }); diff --git a/sdk/src/e2e.integration.test.ts b/sdk/src/e2e.integration.test.ts index 7051bc4e3..7a288790f 100644 --- a/sdk/src/e2e.integration.test.ts +++ b/sdk/src/e2e.integration.test.ts @@ -2,8 +2,8 @@ * E2E integration test — proves full SDK pipeline: * parse → prompt → query() → SUMMARY.md * - * Requires Claude Code CLI (`claude`) installed and authenticated. - * Skips gracefully if CLI is unavailable. + * Requires Claude Code CLI (`claude`) installed and authenticated, plus + * opt-in env `GSD_ENABLE_E2E=1`. Skips if env unset or CLI unavailable. */ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; diff --git a/sdk/src/golden/read-only-parity.integration.test.ts b/sdk/src/golden/read-only-parity.integration.test.ts index e64eb4eab..68689cf9b 100644 --- a/sdk/src/golden/read-only-parity.integration.test.ts +++ b/sdk/src/golden/read-only-parity.integration.test.ts @@ -10,13 +10,15 @@ import { fileURLToPath } from 'node:url'; import { execSync } from 'node:child_process'; import { READ_ONLY_JSON_PARITY_ROWS } from './read-only-golden-rows.js'; +const STABLE_JSON_PARITY_ROWS = READ_ONLY_JSON_PARITY_ROWS.filter( + (row) => row.canonical !== 'scan-sessions' && row.canonical !== 'audit-uat', +); + const __dirname = dirname(fileURLToPath(import.meta.url)); const REPO_ROOT = resolve(__dirname, '..', '..', '..'); describe('Read-only golden parity (JSON toEqual)', () => { - it.each(READ_ONLY_JSON_PARITY_ROWS)('$canonical matches gsd-tools.cjs JSON', async (row) => { - // Volatile command: mutates while suite runs (session count/size timestamps). - if (row.canonical === 'scan-sessions' || row.canonical === 'audit-uat') return; + it.each(STABLE_JSON_PARITY_ROWS)('$canonical matches gsd-tools.cjs JSON', async (row) => { const gsdOutput = await captureGsdToolsOutput(row.cjs, row.cjsArgs, REPO_ROOT); const registry = createRegistry(); @@ -94,19 +96,19 @@ describe('state.load golden parity', () => { }); describe('state.get golden parity', () => { - it('matches full STATE.md when no field (same as `state get` with no section)', async () => { + it('matches full STATE.md when no field (same as `state get` with no section)', async ({ skip }) => { const registry = createRegistry(); const sdkResult = await registry.dispatch('state.get', [], REPO_ROOT); // Repo may not have .planning/STATE.md; skip parity in that case. - if ((sdkResult.data as Record)?.error === 'STATE.md not found') return; + if ((sdkResult.data as Record)?.error === 'STATE.md not found') skip(); const gsdOutput = await captureGsdToolsOutput('state', ['get'], REPO_ROOT); expect(sdkResult.data).toEqual(gsdOutput); }); - it('matches single frontmatter field when `state get `', async () => { + it('matches single frontmatter field when `state get `', async ({ skip }) => { const registry = createRegistry(); const sdkResult = await registry.dispatch('state.get', ['milestone'], REPO_ROOT); - if ((sdkResult.data as Record)?.error === 'STATE.md not found') return; + if ((sdkResult.data as Record)?.error === 'STATE.md not found') skip(); const gsdOutput = await captureGsdToolsOutput('state', ['get', 'milestone'], REPO_ROOT); expect(sdkResult.data).toEqual(gsdOutput); }); diff --git a/sdk/src/gsd-tools-error.ts b/sdk/src/gsd-tools-error.ts new file mode 100644 index 000000000..7898885e3 --- /dev/null +++ b/sdk/src/gsd-tools-error.ts @@ -0,0 +1,13 @@ +export class GSDToolsError extends Error { + constructor( + message: string, + public readonly command: string, + public readonly args: string[], + public readonly exitCode: number | null, + public readonly stderr: string, + options?: { cause?: unknown }, + ) { + super(message, options); + this.name = 'GSDToolsError'; + } +} diff --git a/sdk/src/gsd-tools.ts b/sdk/src/gsd-tools.ts index feedd627a..7d72e3b28 100644 --- a/sdk/src/gsd-tools.ts +++ b/sdk/src/gsd-tools.ts @@ -10,108 +10,37 @@ * workstream env stays aligned with CJS. */ -import { execFile } from 'node:child_process'; -import { readFile } from 'node:fs/promises'; -import { existsSync } from 'node:fs'; -import { join } from 'node:path'; -import { homedir } from 'node:os'; -import { fileURLToPath } from 'node:url'; + import type { InitNewProjectInfo, PhaseOpInfo, PhasePlanIndex, RoadmapAnalysis } from './types.js'; import type { GSDEventStream } from './event-stream.js'; -import { GSDError, exitCodeFor } from './errors.js'; -import { createRegistry } from './query/index.js'; +import { toGSDToolsError } from './query-tools-error-mapper.js'; +import { GSDToolsError } from './gsd-tools-error.js'; import { resolveQueryCommand, type QueryCommandResolution } from './query/query-command-resolution-strategy.js'; -import { formatStateLoadRawStdout } from './query/state-project-load.js'; -import type { QueryResult } from './query/utils.js'; -import { GSDTransport } from './gsd-transport.js'; -import { resolveTransportPolicy } from './gsd-transport-policy.js'; +import { QueryExecutionPolicy } from './query-execution-policy.js'; +import { QueryNativeHotpathAdapter } from './query-native-hotpath-adapter.js'; +import { resolveGsdToolsPath } from './query-gsd-tools-path.js'; +import { createGSDToolsRuntime } from './query-gsd-tools-runtime.js'; +import { QueryCommandExecutor } from './query-command-executor.js'; +import { QueryHotpathMethods } from './query-hotpath-methods.js'; -// ─── Error type ────────────────────────────────────────────────────────────── - -export class GSDToolsError extends Error { - constructor( - message: string, - public readonly command: string, - public readonly args: string[], - public readonly exitCode: number | null, - public readonly stderr: string, - options?: { cause?: unknown }, - ) { - super(message, options); - this.name = 'GSDToolsError'; - } -} +export { GSDToolsError } from './gsd-tools-error.js'; // ─── GSDTools class ────────────────────────────────────────────────────────── const DEFAULT_TIMEOUT_MS = 30_000; -const BUNDLED_GSD_TOOLS_PATH = fileURLToPath( - new URL('../../get-shit-done/bin/gsd-tools.cjs', import.meta.url), -); -function formatRegistryRawStdout(matchedCmd: string, data: unknown): string { - if (matchedCmd === 'state.load') { - return formatStateLoadRawStdout(data); - } - - if (matchedCmd === 'commit') { - const d = data as Record; - if (d.committed === true) { - return d.hash != null ? String(d.hash) : 'committed'; - } - if (d.committed === false) { - const r = String(d.reason ?? ''); - if ( - r.includes('commit_docs') || - r.includes('skipped') || - r.includes('gitignored') || - r === 'skipped_commit_docs_false' - ) { - return 'skipped'; - } - if (r.includes('nothing') || r.includes('nothing_to_commit')) { - return 'nothing'; - } - return r || 'nothing'; - } - return JSON.stringify(data, null, 2); - } - - if (matchedCmd === 'config-set') { - const d = data as Record; - if ((d.updated === true || d.set === true) && d.key !== undefined) { - const v = d.value; - if (v === null || v === undefined) { - return `${d.key}=`; - } - if (typeof v === 'object') { - return `${d.key}=${JSON.stringify(v)}`; - } - return `${d.key}=${String(v)}`; - } - return JSON.stringify(data, null, 2); - } - - if (matchedCmd === 'state.begin-phase' || matchedCmd === 'state begin-phase') { - const d = data as Record; - const u = d.updated as string[] | undefined; - return Array.isArray(u) && u.length > 0 ? 'true' : 'false'; - } - - if (typeof data === 'string') { - return data; - } - return JSON.stringify(data, null, 2); -} export class GSDTools { private readonly projectDir: string; private readonly gsdToolsPath: string; private readonly timeoutMs: number; private readonly workstream?: string; - private readonly registry: ReturnType; + private readonly registry: ReturnType['registry']; private readonly preferNativeQuery: boolean; - private readonly transport: GSDTransport; + private readonly executionPolicy: QueryExecutionPolicy; + private readonly nativeHotpathAdapter: QueryNativeHotpathAdapter; + private readonly commandExecutor: QueryCommandExecutor; + private readonly hotpathMethods: QueryHotpathMethods; constructor(opts: { projectDir: string; @@ -134,16 +63,39 @@ export class GSDTools { this.timeoutMs = opts.timeoutMs ?? DEFAULT_TIMEOUT_MS; this.workstream = opts.workstream; this.preferNativeQuery = opts.preferNativeQuery ?? true; - this.registry = createRegistry(opts.eventStream, opts.sessionId); - this.transport = new GSDTransport(this.registry, { - dispatchNative: async (request) => this.withRegistryDispatchTimeout( - request.legacyCommand, - request.legacyArgs, - this.registry.dispatch(request.registryCommand, request.registryArgs, this.projectDir), - ) as Promise, - execSubprocessJson: async (legacyCommand, legacyArgs) => this.execSubprocessJson(legacyCommand, legacyArgs), - execSubprocessRaw: async (legacyCommand, legacyArgs) => this.execSubprocessRaw(legacyCommand, legacyArgs), - formatNativeRaw: (registryCommand, data) => formatRegistryRawStdout(registryCommand, data), + + const runtime = createGSDToolsRuntime({ + projectDir: this.projectDir, + gsdToolsPath: this.gsdToolsPath, + timeoutMs: this.timeoutMs, + workstream: this.workstream, + eventStream: opts.eventStream, + sessionId: opts.sessionId, + shouldUseNativeQuery: () => this.shouldUseNativeQuery(), + execJsonFallback: (legacyCommand, legacyArgs) => this.exec(legacyCommand, legacyArgs), + execRawFallback: (legacyCommand, legacyArgs) => this.execRaw(legacyCommand, legacyArgs), + }); + + this.registry = runtime.registry; + this.executionPolicy = runtime.executionPolicy; + this.nativeHotpathAdapter = runtime.nativeHotpathAdapter; + this.commandExecutor = new QueryCommandExecutor({ + nativeMatch: (command, args) => this.nativeMatch(command, args), + execute: async (input) => this.executionPolicy.execute({ + legacyCommand: input.legacyCommand, + legacyArgs: input.legacyArgs, + registryCommand: input.registryCommand, + registryArgs: input.registryArgs, + mode: input.mode, + projectDir: this.projectDir, + workstream: this.workstream, + preferNativeQuery: this.shouldUseNativeQuery(), + }), + }); + + this.hotpathMethods = new QueryHotpathMethods({ + dispatchNativeHotpath: (legacyCommand, legacyArgs, registryCommand, registryArgs, mode) => + this.dispatchNativeHotpath(legacyCommand, legacyArgs, registryCommand, registryArgs, mode), }); } @@ -156,112 +108,24 @@ export class GSDTools { } private toToolsError(command: string, args: string[], err: unknown): GSDToolsError { - if (err instanceof GSDError) { - return new GSDToolsError( - err.message, - command, - args, - exitCodeFor(err.classification), - '', - { cause: err }, - ); - } - const msg = err instanceof Error ? err.message : String(err); - return new GSDToolsError( - msg, - command, - args, - 1, - '', - err instanceof Error ? { cause: err } : undefined, - ); + return toGSDToolsError(command, args, err); } - /** - * Enforce {@link GSDTools.timeoutMs} for in-process registry dispatches so native - * routing cannot hang indefinitely (subprocess path already uses `execFile` timeout). - */ - private async withRegistryDispatchTimeout( + private async dispatchNativeHotpath( legacyCommand: string, legacyArgs: string[], - work: Promise, - ): Promise { - let timeoutId: ReturnType | undefined; - const timeoutPromise = new Promise((_, reject) => { - timeoutId = setTimeout(() => { - reject( - new GSDToolsError( - `gsd-tools timed out after ${this.timeoutMs}ms: ${legacyCommand} ${legacyArgs.join(' ')}`, - legacyCommand, - legacyArgs, - null, - '', - ), - ); - }, this.timeoutMs); - }); - try { - // Promise.race rejects when the timeout fires but does not cancel the handler promise; - // native handlers may still run to completion (unlike subprocess + execFile timeout). - return await Promise.race([work, timeoutPromise]); - } finally { - if (timeoutId !== undefined) { - clearTimeout(timeoutId); - } - } - } - - /** - * Direct registry dispatch for a known handler key — skips `resolveQueryArgv` on the hot path - * used by PhaseRunner / InitRunner (`initPhaseOp`, `phasePlanIndex`, etc.). - * When native query is off (e.g. workstream or tests with `preferNativeQuery: false`), delegates to `exec`. - * - * When native query is on, `registry.dispatch` failures are wrapped as {@link GSDToolsError} and - * **not** retried via the legacy `gsd-tools.cjs` subprocess — callers see the handler error - * explicitly. Only commands with no registry match fall through to subprocess routing in {@link exec}. - */ - private async dispatchNativeJson( - legacyCommand: string, - legacyArgs: string[], - registryCmd: string, + registryCommand: string, registryArgs: string[], + mode: 'json' | 'raw', ): Promise { - if (!this.shouldUseNativeQuery()) { - return this.exec(legacyCommand, legacyArgs); - } try { - const result = await this.withRegistryDispatchTimeout( + return await this.nativeHotpathAdapter.dispatch( legacyCommand, legacyArgs, - this.registry.dispatch(registryCmd, registryArgs, this.projectDir), + registryCommand, + registryArgs, + mode, ); - return result.data; - } catch (err) { - if (err instanceof GSDToolsError) throw err; - throw this.toToolsError(legacyCommand, legacyArgs, err); - } - } - - /** - * Same as {@link dispatchNativeJson} for handlers whose CLI contract is raw stdout (`execRaw`), - * including the same “no silent fallback to CJS on handler failure” behaviour. - */ - private async dispatchNativeRaw( - legacyCommand: string, - legacyArgs: string[], - registryCmd: string, - registryArgs: string[], - ): Promise { - if (!this.shouldUseNativeQuery()) { - return this.execRaw(legacyCommand, legacyArgs); - } - try { - const result = await this.withRegistryDispatchTimeout( - legacyCommand, - legacyArgs, - this.registry.dispatch(registryCmd, registryArgs, this.projectDir), - ); - return formatRegistryRawStdout(registryCmd, result.data).trim(); } catch (err) { if (err instanceof GSDToolsError) throw err; throw this.toToolsError(legacyCommand, legacyArgs, err); @@ -275,54 +139,14 @@ export class GSDTools { * Handles the `@file:` prefix pattern for large results. */ async exec(command: string, args: string[] = []): Promise { - const matched = this.nativeMatch(command, args); - const registryCommand = matched?.cmd ?? command; - const registryArgs = matched?.args ?? args; - const policy = resolveTransportPolicy(registryCommand); - try { - return await this.transport.run({ - legacyCommand: command, - legacyArgs: args, - registryCommand, - registryArgs, - mode: 'json', - projectDir: this.projectDir, - workstream: this.workstream, - }, { - preferNative: this.shouldUseNativeQuery() && policy.preferNative, - allowFallbackToSubprocess: policy.allowFallbackToSubprocess, - }); + return await this.commandExecutor.exec(command, args, 'json'); } catch (err) { if (err instanceof GSDToolsError) throw err; throw this.toToolsError(command, args, err); } } - /** - * Parse gsd-tools output, handling `@file:` prefix. - */ - private async parseOutput(raw: string): Promise { - const trimmed = raw.trim(); - - if (trimmed === '') { - return null; - } - - let jsonStr = trimmed; - if (jsonStr.startsWith('@file:')) { - const filePath = jsonStr.slice(6).trim(); - try { - jsonStr = await readFile(filePath, 'utf-8'); - } catch (err) { - const reason = err instanceof Error ? err.message : String(err); - throw new Error(`Failed to read gsd-tools @file: indirection at "${filePath}": ${reason}`); - } - } - - return JSON.parse(jsonStr); - } - // ─── Raw exec (no JSON parsing) ─────────────────────────────────────── /** @@ -330,134 +154,14 @@ export class GSDTools { * Use for commands like `config-set` that return plain text, not JSON. */ async execRaw(command: string, args: string[] = []): Promise { - const matched = this.nativeMatch(command, args); - const registryCommand = matched?.cmd ?? command; - const registryArgs = matched?.args ?? args; - const policy = resolveTransportPolicy(registryCommand); - try { - return await this.transport.run({ - legacyCommand: command, - legacyArgs: args, - registryCommand, - registryArgs, - mode: 'raw', - projectDir: this.projectDir, - workstream: this.workstream, - }, { - preferNative: this.shouldUseNativeQuery() && policy.preferNative, - allowFallbackToSubprocess: policy.allowFallbackToSubprocess, - }) as string; + return await this.commandExecutor.exec(command, args, 'raw') as string; } catch (err) { if (err instanceof GSDToolsError) throw err; throw this.toToolsError(command, args, err); } } - private async execSubprocessJson(command: string, args: string[]): Promise { - const wsArgs = this.workstream ? ['--ws', this.workstream] : []; - const fullArgs = [this.gsdToolsPath, command, ...args, ...wsArgs]; - - return new Promise((resolve, reject) => { - const child = execFile( - process.execPath, - fullArgs, - { - cwd: this.projectDir, - maxBuffer: 10 * 1024 * 1024, - timeout: this.timeoutMs, - env: { ...process.env }, - }, - async (error, stdout, stderr) => { - const stderrStr = stderr?.toString() ?? ''; - - if (error) { - if (error.killed || (error as NodeJS.ErrnoException).code === 'ETIMEDOUT') { - reject( - new GSDToolsError( - `gsd-tools timed out after ${this.timeoutMs}ms: ${command} ${args.join(' ')}`, - command, - args, - null, - stderrStr, - ), - ); - return; - } - - reject( - new GSDToolsError( - `gsd-tools exited with code ${error.code ?? 'unknown'}: ${command} ${args.join(' ')}${stderrStr ? `\n${stderrStr}` : ''}`, - command, - args, - typeof error.code === 'number' ? error.code : (error as { status?: number }).status ?? 1, - stderrStr, - ), - ); - return; - } - - const raw = stdout?.toString() ?? ''; - try { - const parsed = await this.parseOutput(raw); - resolve(parsed); - } catch (parseErr) { - reject( - new GSDToolsError( - `Failed to parse gsd-tools output for "${command}": ${parseErr instanceof Error ? parseErr.message : String(parseErr)}\nRaw output: ${raw.slice(0, 500)}`, - command, - args, - 0, - stderrStr, - ), - ); - } - }, - ); - - child.on('error', (err) => { - reject(new GSDToolsError(`Failed to execute gsd-tools: ${err.message}`, command, args, null, '')); - }); - }); - } - - private async execSubprocessRaw(command: string, args: string[]): Promise { - const wsArgs = this.workstream ? ['--ws', this.workstream] : []; - const fullArgs = [this.gsdToolsPath, command, ...args, ...wsArgs, '--raw']; - - return new Promise((resolve, reject) => { - const child = execFile( - process.execPath, - fullArgs, - { - cwd: this.projectDir, - maxBuffer: 10 * 1024 * 1024, - timeout: this.timeoutMs, - env: { ...process.env }, - }, - (error, stdout, stderr) => { - const stderrStr = stderr?.toString() ?? ''; - if (error) { - reject( - new GSDToolsError( - `gsd-tools exited with code ${error.code ?? 'unknown'}: ${command} ${args.join(' ')}${stderrStr ? `\n${stderrStr}` : ''}`, - command, - args, - typeof error.code === 'number' ? error.code : (error as { status?: number }).status ?? 1, - stderrStr, - ), - ); - return; - } - resolve((stdout?.toString() ?? '').trim()); - }, - ); - - child.on('error', (err) => { - reject(new GSDToolsError(`Failed to execute gsd-tools: ${err.message}`, command, args, null, '')); - }); - }); - } // ─── Typed convenience methods ───────────────────────────────────────── @@ -470,15 +174,11 @@ export class GSDTools { } async phaseComplete(phase: string): Promise { - return this.dispatchNativeRaw('phase', ['complete', phase], 'phase.complete', [phase]); + return this.hotpathMethods.phaseComplete(phase); } async commit(message: string, files?: string[]): Promise { - const args = [message]; - if (files?.length) { - args.push('--files', ...files); - } - return this.dispatchNativeRaw('commit', args, 'commit', args); + return this.hotpathMethods.commit(message, files); } async verifySummary(path: string): Promise { @@ -494,26 +194,14 @@ export class GSDTools { * Returns a typed PhaseOpInfo describing what exists on disk for this phase. */ async initPhaseOp(phaseNumber: string): Promise { - const result = await this.dispatchNativeJson( - 'init', - ['phase-op', phaseNumber], - 'init.phase-op', - [phaseNumber], - ); - return result as PhaseOpInfo; + return this.hotpathMethods.initPhaseOp(phaseNumber); } /** * Get a config value via the `config-get` surface (CJS and registry use the same key path). */ async configGet(key: string): Promise { - const result = await this.dispatchNativeJson( - 'config-get', - [key], - 'config-get', - [key], - ); - return result as string | null; + return this.hotpathMethods.configGet(key); } /** @@ -528,13 +216,7 @@ export class GSDTools { * Returns typed PhasePlanIndex with wave assignments and completion status. */ async phasePlanIndex(phaseNumber: string): Promise { - const result = await this.dispatchNativeJson( - 'phase-plan-index', - [phaseNumber], - 'phase-plan-index', - [phaseNumber], - ); - return result as PhasePlanIndex; + return this.hotpathMethods.phasePlanIndex(phaseNumber); } /** @@ -542,8 +224,7 @@ export class GSDTools { * Returns project metadata, model configs, brownfield detection, etc. */ async initNewProject(): Promise { - const result = await this.dispatchNativeJson('init', ['new-project'], 'init.new-project', []); - return result as InitNewProjectInfo; + return this.hotpathMethods.initNewProject(); } /** @@ -552,23 +233,8 @@ export class GSDTools { * Note: config-set returns `key=value` text, not JSON, so we use execRaw. */ async configSet(key: string, value: string): Promise { - return this.dispatchNativeRaw('config-set', [key, value], 'config-set', [key, value]); + return this.hotpathMethods.configSet(key, value); } } -// ─── Path resolution ──────────────────────────────────────────────────────── - -/** - * Resolve gsd-tools.cjs path. - * Probe order: SDK-bundled repo copy → `project/.claude/get-shit-done/` → - * `~/.claude/get-shit-done/`. - */ -export function resolveGsdToolsPath(projectDir: string): string { - const candidates = [ - BUNDLED_GSD_TOOLS_PATH, - join(projectDir, '.claude', 'get-shit-done', 'bin', 'gsd-tools.cjs'), - join(homedir(), '.claude', 'get-shit-done', 'bin', 'gsd-tools.cjs'), - ]; - - return candidates.find(candidate => existsSync(candidate)) ?? candidates[candidates.length - 1]!; -} +export { resolveGsdToolsPath } from './query-gsd-tools-path.js'; diff --git a/sdk/src/query-command-executor.ts b/sdk/src/query-command-executor.ts new file mode 100644 index 000000000..68f2db4d3 --- /dev/null +++ b/sdk/src/query-command-executor.ts @@ -0,0 +1,31 @@ +export interface QueryCommandExecutorDeps { + nativeMatch: (command: string, args: string[]) => { cmd: string; args: string[] } | null; + execute: (input: { + legacyCommand: string; + legacyArgs: string[]; + registryCommand: string; + registryArgs: string[]; + mode: 'json' | 'raw'; + }) => Promise; +} + +/** + * Module owning command normalization + execution payload shape. + */ +export class QueryCommandExecutor { + constructor(private readonly deps: QueryCommandExecutorDeps) {} + + async exec(command: string, args: string[], mode: 'json' | 'raw'): Promise { + const matched = this.deps.nativeMatch(command, args); + const registryCommand = matched?.cmd ?? command; + const registryArgs = matched?.args ?? args; + + return this.deps.execute({ + legacyCommand: command, + legacyArgs: args, + registryCommand, + registryArgs, + mode, + }); + } +} diff --git a/sdk/src/query-execution-policy.test.ts b/sdk/src/query-execution-policy.test.ts new file mode 100644 index 000000000..146801a5a --- /dev/null +++ b/sdk/src/query-execution-policy.test.ts @@ -0,0 +1,31 @@ +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { QueryExecutionPolicy } from './query-execution-policy.js'; +import { setTransportPolicy, clearTransportPolicy } from './gsd-transport-policy.js'; + +describe('QueryExecutionPolicy', () => { + afterEach(() => { + clearTransportPolicy(); + }); + + it('applies transport policy to transport.run', async () => { + const run = vi.fn().mockResolvedValue({ ok: true }); + const policy = new QueryExecutionPolicy({ run } as never); + + setTransportPolicy('verify.path-exists', { preferNative: true, allowFallbackToSubprocess: false }); + + await policy.execute({ + legacyCommand: 'verify.path-exists', + legacyArgs: [], + registryCommand: 'verify.path-exists', + registryArgs: [], + mode: 'json', + projectDir: '/tmp/project', + preferNativeQuery: true, + }); + + expect(run).toHaveBeenCalledTimes(1); + const [, policyArg] = run.mock.calls[0]; + expect(policyArg).toEqual({ preferNative: true, allowFallbackToSubprocess: false }); + + }); +}); diff --git a/sdk/src/query-execution-policy.ts b/sdk/src/query-execution-policy.ts new file mode 100644 index 000000000..1c874b4ce --- /dev/null +++ b/sdk/src/query-execution-policy.ts @@ -0,0 +1,42 @@ +import { resolveTransportPolicy } from './gsd-transport-policy.js'; +import type { GSDTransport } from './gsd-transport.js'; +import type { TransportMode } from './gsd-transport-policy.js'; + +export interface QueryExecutionRequest { + legacyCommand: string; + legacyArgs: string[]; + registryCommand: string; + registryArgs: string[]; + mode: TransportMode; + projectDir: string; + workstream?: string; + preferNativeQuery: boolean; +} + +/** + * Execution policy for query command dispatch. + * Owns routing decision inputs for native/subprocess dispatch. + */ +export class QueryExecutionPolicy { + constructor(private readonly transport: GSDTransport) {} + + async execute(request: QueryExecutionRequest): Promise { + const policy = resolveTransportPolicy(request.registryCommand); + + return this.transport.run( + { + legacyCommand: request.legacyCommand, + legacyArgs: request.legacyArgs, + registryCommand: request.registryCommand, + registryArgs: request.registryArgs, + mode: request.mode, + projectDir: request.projectDir, + workstream: request.workstream, + }, + { + preferNative: request.preferNativeQuery && policy.preferNative, + allowFallbackToSubprocess: policy.allowFallbackToSubprocess, + }, + ); + } +} diff --git a/sdk/src/query-gsd-tools-path.ts b/sdk/src/query-gsd-tools-path.ts new file mode 100644 index 000000000..18c22292a --- /dev/null +++ b/sdk/src/query-gsd-tools-path.ts @@ -0,0 +1,24 @@ +import { existsSync } from 'node:fs'; +import { join } from 'node:path'; +import { homedir } from 'node:os'; +import { fileURLToPath } from 'node:url'; + +const BUNDLED_GSD_TOOLS_PATH = fileURLToPath( + new URL('../../get-shit-done/bin/gsd-tools.cjs', import.meta.url), +); + +/** + * Resolve gsd-tools.cjs path. + * Probe order: SDK-bundled repo copy → project/.claude/get-shit-done → ~/.claude/get-shit-done + */ +export function resolveGsdToolsPath(projectDir: string): string { + const candidates = [ + BUNDLED_GSD_TOOLS_PATH, + join(projectDir, '.claude', 'get-shit-done', 'bin', 'gsd-tools.cjs'), + join(homedir(), '.claude', 'get-shit-done', 'bin', 'gsd-tools.cjs'), + ]; + + return candidates.find(candidate => existsSync(candidate)) ?? candidates[candidates.length - 1]!; +} + +export { BUNDLED_GSD_TOOLS_PATH }; diff --git a/sdk/src/query-gsd-tools-runtime.ts b/sdk/src/query-gsd-tools-runtime.ts new file mode 100644 index 000000000..e6c871e21 --- /dev/null +++ b/sdk/src/query-gsd-tools-runtime.ts @@ -0,0 +1,67 @@ +import type { GSDEventStream } from './event-stream.js'; +import { createRegistry } from './query/index.js'; +import type { QueryResult } from './query/utils.js'; +import { GSDTransport } from './gsd-transport.js'; +import { QueryExecutionPolicy } from './query-execution-policy.js'; +import { QuerySubprocessAdapter } from './query-subprocess-adapter.js'; +import { QueryNativeDirectAdapter } from './query-native-direct-adapter.js'; +import { QueryNativeHotpathAdapter } from './query-native-hotpath-adapter.js'; +import { formatQueryRawOutput } from './query-raw-output-projection.js'; +import { GSDToolsError } from './gsd-tools-error.js'; + +export interface GSDToolsRuntime { + registry: ReturnType; + executionPolicy: QueryExecutionPolicy; + nativeHotpathAdapter: QueryNativeHotpathAdapter; +} + +export function createGSDToolsRuntime(opts: { + projectDir: string; + gsdToolsPath: string; + timeoutMs: number; + workstream?: string; + eventStream?: GSDEventStream; + sessionId?: string; + shouldUseNativeQuery: () => boolean; + execJsonFallback: (legacyCommand: string, legacyArgs: string[]) => Promise; + execRawFallback: (legacyCommand: string, legacyArgs: string[]) => Promise; +}): GSDToolsRuntime { + const registry = createRegistry(opts.eventStream, opts.sessionId); + + const subprocessAdapter = new QuerySubprocessAdapter({ + projectDir: opts.projectDir, + gsdToolsPath: opts.gsdToolsPath, + timeoutMs: opts.timeoutMs, + workstream: opts.workstream, + createToolsError: (message, command, args, exitCode, stderr) => + new GSDToolsError(message, command, args, exitCode, stderr), + }); + + const nativeDirectAdapter = new QueryNativeDirectAdapter({ + timeoutMs: opts.timeoutMs, + dispatch: (registryCommand, registryArgs) => registry.dispatch(registryCommand, registryArgs, opts.projectDir), + createTimeoutError: (message, command, args) => new GSDToolsError(message, command, args, null, ''), + }); + + const transport = new GSDTransport(registry, { + dispatchNative: async (request) => nativeDirectAdapter.dispatchResult( + request.legacyCommand, + request.legacyArgs, + request.registryCommand, + request.registryArgs, + ) as Promise, + execSubprocessJson: async (legacyCommand, legacyArgs) => subprocessAdapter.execJson(legacyCommand, legacyArgs), + execSubprocessRaw: async (legacyCommand, legacyArgs) => subprocessAdapter.execRaw(legacyCommand, legacyArgs), + formatNativeRaw: (registryCommand, data) => formatQueryRawOutput(registryCommand, data), + }); + + const executionPolicy = new QueryExecutionPolicy(transport); + const nativeHotpathAdapter = new QueryNativeHotpathAdapter( + opts.shouldUseNativeQuery, + nativeDirectAdapter, + opts.execJsonFallback, + opts.execRawFallback, + ); + + return { registry, executionPolicy, nativeHotpathAdapter }; +} diff --git a/sdk/src/query-hotpath-methods.ts b/sdk/src/query-hotpath-methods.ts new file mode 100644 index 000000000..1ed47743e --- /dev/null +++ b/sdk/src/query-hotpath-methods.ts @@ -0,0 +1,48 @@ +import type { InitNewProjectInfo, PhaseOpInfo, PhasePlanIndex } from './types.js'; + +export interface QueryHotpathMethodsDeps { + dispatchNativeHotpath: ( + legacyCommand: string, + legacyArgs: string[], + registryCommand: string, + registryArgs: string[], + mode: 'json' | 'raw', + ) => Promise; +} + +/** + * Module owning typed hot-path method projection for GSDTools facade. + */ +export class QueryHotpathMethods { + constructor(private readonly deps: QueryHotpathMethodsDeps) {} + + phaseComplete(phase: string): Promise { + return this.deps.dispatchNativeHotpath('phase', ['complete', phase], 'phase.complete', [phase], 'raw') as Promise; + } + + commit(message: string, files?: string[]): Promise { + const args = [message]; + if (files?.length) args.push('--files', ...files); + return this.deps.dispatchNativeHotpath('commit', args, 'commit', args, 'raw') as Promise; + } + + initPhaseOp(phaseNumber: string): Promise { + return this.deps.dispatchNativeHotpath('init', ['phase-op', phaseNumber], 'init.phase-op', [phaseNumber], 'json') as Promise; + } + + configGet(key: string): Promise { + return this.deps.dispatchNativeHotpath('config-get', [key], 'config-get', [key], 'json') as Promise; + } + + phasePlanIndex(phaseNumber: string): Promise { + return this.deps.dispatchNativeHotpath('phase-plan-index', [phaseNumber], 'phase-plan-index', [phaseNumber], 'json') as Promise; + } + + initNewProject(): Promise { + return this.deps.dispatchNativeHotpath('init', ['new-project'], 'init.new-project', [], 'json') as Promise; + } + + configSet(key: string, value: string): Promise { + return this.deps.dispatchNativeHotpath('config-set', [key, value], 'config-set', [key, value], 'raw') as Promise; + } +} diff --git a/sdk/src/query-native-direct-adapter.ts b/sdk/src/query-native-direct-adapter.ts new file mode 100644 index 000000000..f87792d10 --- /dev/null +++ b/sdk/src/query-native-direct-adapter.ts @@ -0,0 +1,50 @@ +import { formatQueryRawOutput } from './query-raw-output-projection.js'; +import type { QueryResult } from './query/utils.js'; + +export interface QueryNativeDirectAdapterDeps { + timeoutMs: number; + dispatch: (registryCommand: string, registryArgs: string[]) => Promise; + createTimeoutError: (message: string, command: string, args: string[]) => Error; +} + +/** + * Adapter Module for direct native registry dispatch with timeout policy. + */ +export class QueryNativeDirectAdapter { + constructor(private readonly deps: QueryNativeDirectAdapterDeps) {} + + async dispatchResult(legacyCommand: string, legacyArgs: string[], registryCommand: string, registryArgs: string[]): Promise { + return this.withTimeout(legacyCommand, legacyArgs, this.deps.dispatch(registryCommand, registryArgs)); + } + + async dispatchJson(legacyCommand: string, legacyArgs: string[], registryCommand: string, registryArgs: string[]): Promise { + const result = await this.dispatchResult(legacyCommand, legacyArgs, registryCommand, registryArgs); + return result.data; + } + + async dispatchRaw(legacyCommand: string, legacyArgs: string[], registryCommand: string, registryArgs: string[]): Promise { + const result = await this.dispatchResult(legacyCommand, legacyArgs, registryCommand, registryArgs); + return formatQueryRawOutput(registryCommand, result.data).trim(); + } + + private async withTimeout(legacyCommand: string, legacyArgs: string[], work: Promise): Promise { + let timeoutId: ReturnType | undefined; + const timeoutPromise = new Promise((_, reject) => { + timeoutId = setTimeout(() => { + reject( + this.deps.createTimeoutError( + `gsd-tools timed out after ${this.deps.timeoutMs}ms: ${legacyCommand} ${legacyArgs.join(' ')}`, + legacyCommand, + legacyArgs, + ), + ); + }, this.deps.timeoutMs); + }); + + try { + return await Promise.race([work, timeoutPromise]); + } finally { + if (timeoutId !== undefined) clearTimeout(timeoutId); + } + } +} diff --git a/sdk/src/query-native-hotpath-adapter.test.ts b/sdk/src/query-native-hotpath-adapter.test.ts new file mode 100644 index 000000000..727494d4d --- /dev/null +++ b/sdk/src/query-native-hotpath-adapter.test.ts @@ -0,0 +1,43 @@ +import { describe, it, expect, vi } from 'vitest'; +import { QueryNativeHotpathAdapter } from './query-native-hotpath-adapter.js'; + +describe('QueryNativeHotpathAdapter', () => { + it('uses native Adapter when native query enabled', async () => { + const native = { + dispatchJson: vi.fn().mockResolvedValue({ ok: true }), + dispatchRaw: vi.fn().mockResolvedValue('ok'), + } as never; + + const adapter = new QueryNativeHotpathAdapter( + () => true, + native, + vi.fn(), + vi.fn(), + ); + + await expect(adapter.dispatch('state', ['load'], 'state.load', [], 'json')).resolves.toEqual({ ok: true }); + await expect(adapter.dispatch('commit', ['m'], 'commit', ['m'], 'raw')).resolves.toEqual('ok'); + expect((native as { dispatchJson: ReturnType }).dispatchJson).toHaveBeenCalledWith('state', ['load'], 'state.load', []); + expect((native as { dispatchRaw: ReturnType }).dispatchRaw).toHaveBeenCalledWith('commit', ['m'], 'commit', ['m']); + }); + + it('uses fallback when native query disabled', async () => { + const execJsonFallback = vi.fn().mockResolvedValue({ from: 'fallback-json' }); + const execRawFallback = vi.fn().mockResolvedValue('fallback-raw'); + + const adapter = new QueryNativeHotpathAdapter( + () => false, + { + dispatchJson: vi.fn(), + dispatchRaw: vi.fn(), + } as never, + execJsonFallback, + execRawFallback, + ); + + await expect(adapter.dispatch('state', ['load'], 'state.load', [], 'json')).resolves.toEqual({ from: 'fallback-json' }); + await expect(adapter.dispatch('commit', ['m'], 'commit', ['m'], 'raw')).resolves.toEqual('fallback-raw'); + expect(execJsonFallback).toHaveBeenCalledWith('state', ['load']); + expect(execRawFallback).toHaveBeenCalledWith('commit', ['m']); + }); +}); diff --git a/sdk/src/query-native-hotpath-adapter.ts b/sdk/src/query-native-hotpath-adapter.ts new file mode 100644 index 000000000..6a201d74b --- /dev/null +++ b/sdk/src/query-native-hotpath-adapter.ts @@ -0,0 +1,31 @@ +import type { QueryNativeDirectAdapter } from './query-native-direct-adapter.js'; + +/** + * Adapter Module for runner hot-path native commands. + */ +export class QueryNativeHotpathAdapter { + constructor( + private readonly shouldUseNativeQuery: () => boolean, + private readonly nativeDirect: QueryNativeDirectAdapter, + private readonly execJsonFallback: (legacyCommand: string, legacyArgs: string[]) => Promise, + private readonly execRawFallback: (legacyCommand: string, legacyArgs: string[]) => Promise, + ) {} + + async dispatch( + legacyCommand: string, + legacyArgs: string[], + registryCommand: string, + registryArgs: string[], + mode: 'json' | 'raw', + ): Promise { + if (!this.shouldUseNativeQuery()) { + return mode === 'raw' + ? this.execRawFallback(legacyCommand, legacyArgs) + : this.execJsonFallback(legacyCommand, legacyArgs); + } + + return mode === 'raw' + ? this.nativeDirect.dispatchRaw(legacyCommand, legacyArgs, registryCommand, registryArgs) + : this.nativeDirect.dispatchJson(legacyCommand, legacyArgs, registryCommand, registryArgs); + } +} diff --git a/sdk/src/query-raw-output-projection.test.ts b/sdk/src/query-raw-output-projection.test.ts new file mode 100644 index 000000000..4b7db4e85 --- /dev/null +++ b/sdk/src/query-raw-output-projection.test.ts @@ -0,0 +1,34 @@ +import { describe, it, expect } from 'vitest'; +import { formatQueryRawOutput } from './query-raw-output-projection.js'; + +describe('formatQueryRawOutput', () => { + it('formats commit hash', () => { + expect(formatQueryRawOutput('commit', { committed: true, hash: 'abc123' })).toBe('abc123'); + }); + + it('returns committed when hash missing', () => { + expect(formatQueryRawOutput('commit', { committed: true })).toBe('committed'); + }); + + it('formats skipped commit reason', () => { + expect(formatQueryRawOutput('commit', { committed: false, reason: 'skipped' })).toBe('skipped'); + }); + + it('formats nothing-to-commit reason', () => { + expect(formatQueryRawOutput('commit', { committed: false, reason: 'nothing_to_commit' })).toBe('nothing'); + }); + + it('formats config-set key=value', () => { + expect(formatQueryRawOutput('config-set', { updated: true, key: 'mode', value: 'yolo' })).toBe('mode=yolo'); + }); + + it('formats state.begin-phase boolean result', () => { + expect(formatQueryRawOutput('state.begin-phase', { updated: ['x'] })).toBe('true'); + expect(formatQueryRawOutput('state.begin-phase', { updated: [] })).toBe('false'); + }); + + it('formats state begin-phase alias', () => { + expect(formatQueryRawOutput('state begin-phase', { updated: ['x'] })).toBe('true'); + expect(formatQueryRawOutput('state begin-phase', { updated: [] })).toBe('false'); + }); +}); diff --git a/sdk/src/query-raw-output-projection.ts b/sdk/src/query-raw-output-projection.ts new file mode 100644 index 000000000..51f564435 --- /dev/null +++ b/sdk/src/query-raw-output-projection.ts @@ -0,0 +1,69 @@ +import { formatStateLoadRawStdout } from './query/state-project-load.js'; + +/** + * Raw output projection for native query results. + * Owns CLI-facing string contracts for raw mode commands. + */ +export function formatQueryRawOutput(registryCommand: string, data: unknown): string { + if (registryCommand === 'state.load') { + return formatStateLoadRawStdout(data); + } + + if (registryCommand === 'commit') { + if (data == null || typeof data !== 'object' || Array.isArray(data)) { + return JSON.stringify(data, null, 2); + } + const d = data as Record; + if (d.committed === true) { + return d.hash != null ? String(d.hash) : 'committed'; + } + if (d.committed === false) { + const r = String(d.reason ?? ''); + if ( + r.includes('commit_docs') || + r.includes('skipped') || + r.includes('gitignored') || + r === 'skipped_commit_docs_false' + ) { + return 'skipped'; + } + if (r.includes('nothing') || r.includes('nothing_to_commit')) { + return 'nothing'; + } + return r || 'nothing'; + } + return JSON.stringify(data, null, 2); + } + + if (registryCommand === 'config-set') { + if (data == null || typeof data !== 'object' || Array.isArray(data)) { + return JSON.stringify(data, null, 2); + } + const d = data as Record; + if ((d.updated === true || d.set === true) && d.key !== undefined) { + const v = d.value; + if (v === null || v === undefined) { + return `${d.key}=`; + } + if (typeof v === 'object') { + return `${d.key}=${JSON.stringify(v)}`; + } + return `${d.key}=${String(v)}`; + } + return JSON.stringify(data, null, 2); + } + + if (registryCommand === 'state.begin-phase' || registryCommand === 'state begin-phase') { + if (data == null || typeof data !== 'object' || Array.isArray(data)) { + return JSON.stringify(data, null, 2); + } + const d = data as Record; + const u = d.updated as string[] | undefined; + return Array.isArray(u) && u.length > 0 ? 'true' : 'false'; + } + + if (typeof data === 'string') { + return data; + } + return JSON.stringify(data, null, 2); +} diff --git a/sdk/src/query-subprocess-adapter.test.ts b/sdk/src/query-subprocess-adapter.test.ts new file mode 100644 index 000000000..c2ab850c3 --- /dev/null +++ b/sdk/src/query-subprocess-adapter.test.ts @@ -0,0 +1,71 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { mkdir, writeFile, rm } from 'node:fs/promises'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import { QuerySubprocessAdapter } from './query-subprocess-adapter.js'; + +class FakeToolsError extends Error { + constructor( + message: string, + public readonly command: string, + public readonly args: string[], + public readonly exitCode: number | null, + public readonly stderr: string, + ) { + super(message); + } +} + +describe('QuerySubprocessAdapter', () => { + let dir: string; + let fixtures: string; + + beforeEach(async () => { + dir = join(tmpdir(), `query-subprocess-adapter-${Date.now()}-${Math.random().toString(36).slice(2)}`); + fixtures = join(dir, 'fixtures'); + await mkdir(fixtures, { recursive: true }); + }); + + afterEach(async () => { + await rm(dir, { recursive: true, force: true }); + }); + + async function createScript(name: string, code: string): Promise { + const scriptPath = join(fixtures, name); + await writeFile(scriptPath, code, { mode: 0o755 }); + return scriptPath; + } + + function createAdapter(gsdToolsPath: string): QuerySubprocessAdapter { + return new QuerySubprocessAdapter({ + projectDir: dir, + gsdToolsPath, + timeoutMs: 2_000, + createToolsError: (message, command, args, exitCode, stderr) => + new FakeToolsError(message, command, args, exitCode, stderr) as never, + }); + } + + it('execJson parses JSON', async () => { + const script = await createScript('json.cjs', `process.stdout.write(JSON.stringify({ ok: true }));`); + const adapter = createAdapter(script); + + await expect(adapter.execJson('state', ['load'])).resolves.toEqual({ ok: true }); + }); + + it('execJson resolves @file output', async () => { + const outFile = join(fixtures, 'out.json'); + await writeFile(outFile, JSON.stringify({ from: 'file' })); + const script = await createScript('file.cjs', `process.stdout.write('@file:${outFile.replace(/\\/g, '\\\\')}');`); + const adapter = createAdapter(script); + + await expect(adapter.execJson('state', ['load'])).resolves.toEqual({ from: 'file' }); + }); + + it('execRaw returns trimmed stdout', async () => { + const script = await createScript('raw.cjs', `process.stdout.write(' hello ');`); + const adapter = createAdapter(script); + + await expect(adapter.execRaw('config-set', ['x', 'y'])).resolves.toBe('hello'); + }); +}); diff --git a/sdk/src/query-subprocess-adapter.ts b/sdk/src/query-subprocess-adapter.ts new file mode 100644 index 000000000..f7b567d37 --- /dev/null +++ b/sdk/src/query-subprocess-adapter.ts @@ -0,0 +1,159 @@ +import { execFile } from 'node:child_process'; +import { readFile } from 'node:fs/promises'; +import type { GSDToolsError } from './gsd-tools-error.js'; + +export interface QuerySubprocessAdapterDeps { + projectDir: string; + gsdToolsPath: string; + timeoutMs: number; + workstream?: string; + createToolsError: ( + message: string, + command: string, + args: string[], + exitCode: number | null, + stderr: string, + ) => GSDToolsError; +} + +export class QuerySubprocessAdapter { + constructor(private readonly deps: QuerySubprocessAdapterDeps) {} + + async execJson(command: string, args: string[]): Promise { + const wsArgs = this.deps.workstream ? ['--ws', this.deps.workstream] : []; + const fullArgs = [this.deps.gsdToolsPath, command, ...args, ...wsArgs]; + + return new Promise((resolve, reject) => { + const child = execFile( + process.execPath, + fullArgs, + { + cwd: this.deps.projectDir, + maxBuffer: 10 * 1024 * 1024, + timeout: this.deps.timeoutMs, + env: { ...process.env }, + }, + async (error, stdout, stderr) => { + const stderrStr = stderr?.toString() ?? ''; + + if (error) { + if (error.killed || (error as NodeJS.ErrnoException).code === 'ETIMEDOUT') { + reject( + this.deps.createToolsError( + `gsd-tools timed out after ${this.deps.timeoutMs}ms: ${command} ${args.join(' ')}`, + command, + args, + null, + stderrStr, + ), + ); + return; + } + + reject( + this.deps.createToolsError( + `gsd-tools exited with code ${error.code ?? 'unknown'}: ${command} ${args.join(' ')}${stderrStr ? `\n${stderrStr}` : ''}`, + command, + args, + typeof error.code === 'number' ? error.code : (error as { status?: number }).status ?? 1, + stderrStr, + ), + ); + return; + } + + const raw = stdout?.toString() ?? ''; + try { + const parsed = await this.parseOutput(raw); + resolve(parsed); + } catch (parseErr) { + reject( + this.deps.createToolsError( + `Failed to parse gsd-tools output for "${command}": ${parseErr instanceof Error ? parseErr.message : String(parseErr)}\nRaw output: ${raw.slice(0, 500)}`, + command, + args, + 0, + stderrStr, + ), + ); + } + }, + ); + + child.on('error', (err) => { + reject(this.deps.createToolsError(`Failed to execute gsd-tools: ${err.message}`, command, args, null, '')); + }); + }); + } + + async execRaw(command: string, args: string[]): Promise { + const wsArgs = this.deps.workstream ? ['--ws', this.deps.workstream] : []; + const fullArgs = [this.deps.gsdToolsPath, command, ...args, ...wsArgs, '--raw']; + + return new Promise((resolve, reject) => { + const child = execFile( + process.execPath, + fullArgs, + { + cwd: this.deps.projectDir, + maxBuffer: 10 * 1024 * 1024, + timeout: this.deps.timeoutMs, + env: { ...process.env }, + }, + (error, stdout, stderr) => { + const stderrStr = stderr?.toString() ?? ''; + if (error) { + if (error.killed || (error as NodeJS.ErrnoException).code === 'ETIMEDOUT') { + reject( + this.deps.createToolsError( + `gsd-tools timed out after ${this.deps.timeoutMs}ms: ${command} ${args.join(' ')}`, + command, + args, + null, + stderrStr, + ), + ); + return; + } + reject( + this.deps.createToolsError( + `gsd-tools exited with code ${error.code ?? 'unknown'}: ${command} ${args.join(' ')}${stderrStr ? `\n${stderrStr}` : ''}`, + command, + args, + typeof error.code === 'number' ? error.code : (error as { status?: number }).status ?? 1, + stderrStr, + ), + ); + return; + } + resolve((stdout?.toString() ?? '').trim()); + }, + ); + + child.on('error', (err) => { + reject(this.deps.createToolsError(`Failed to execute gsd-tools: ${err.message}`, command, args, null, '')); + }); + }); + } + + private async parseOutput(raw: string): Promise { + const trimmed = raw.trim(); + + if (trimmed === '') { + return null; + } + + let jsonStr = trimmed; + if (jsonStr.startsWith('@file:')) { + const filePath = jsonStr.slice(6).trim(); + try { + jsonStr = await readFile(filePath, 'utf-8'); + } catch (err) { + const reason = err instanceof Error ? err.message : String(err); + throw new Error(`Failed to read gsd-tools @file: indirection at "${filePath}": ${reason}`); + } + } + + return JSON.parse(jsonStr); + } +} diff --git a/sdk/src/query-tools-error-mapper.ts b/sdk/src/query-tools-error-mapper.ts new file mode 100644 index 000000000..162e23720 --- /dev/null +++ b/sdk/src/query-tools-error-mapper.ts @@ -0,0 +1,28 @@ +import { GSDError, exitCodeFor } from './errors.js'; +import { GSDToolsError } from './gsd-tools-error.js'; + +/** + * Module owning projection of internal errors to GSDToolsError contract. + */ +export function toGSDToolsError(command: string, args: string[], err: unknown): GSDToolsError { + if (err instanceof GSDError) { + return new GSDToolsError( + err.message, + command, + args, + exitCodeFor(err.classification), + '', + { cause: err }, + ); + } + + const msg = err instanceof Error ? err.message : String(err); + return new GSDToolsError( + msg, + command, + args, + 1, + '', + err instanceof Error ? { cause: err } : undefined, + ); +} diff --git a/sdk/src/query/init.ts b/sdk/src/query/init.ts index 6670bab6d..02aee7327 100644 --- a/sdk/src/query/init.ts +++ b/sdk/src/query/init.ts @@ -285,12 +285,12 @@ export const initExecutePhase: QueryHandler = async (args, projectDir, workstrea const phase_req_ids = extractReqIds(roadmapPhase); const configExists = existsSync(join(planningDir, 'config.json')); - const [executorModelRaw, verifierModelRaw] = await Promise.all([ - getModelAlias('gsd-executor', projectDir), - getModelAlias('gsd-verifier', projectDir), - ]); - const executorModel = configExists ? executorModelRaw : ''; - const verifierModel = configExists ? verifierModelRaw : ''; + const [executorModel, verifierModel] = configExists + ? await Promise.all([ + getModelAlias('gsd-executor', projectDir), + getModelAlias('gsd-verifier', projectDir), + ]) + : ['', '']; const milestone = await getMilestoneInfo(projectDir, workstream); @@ -367,14 +367,13 @@ export const initPlanPhase: QueryHandler = async (args, projectDir, workstream) const phase_req_ids = extractReqIds(roadmapPhase); const configExists = existsSync(join(planningDir, 'config.json')); - const [researcherModelRaw, plannerModelRaw, checkerModelRaw] = await Promise.all([ - getModelAlias('gsd-phase-researcher', projectDir), - getModelAlias('gsd-planner', projectDir), - getModelAlias('gsd-plan-checker', projectDir), - ]); - const researcherModel = configExists ? researcherModelRaw : ''; - const plannerModel = configExists ? plannerModelRaw : ''; - const checkerModel = configExists ? checkerModelRaw : ''; + const [researcherModel, plannerModel, checkerModel] = configExists + ? await Promise.all([ + getModelAlias('gsd-phase-researcher', projectDir), + getModelAlias('gsd-planner', projectDir), + getModelAlias('gsd-plan-checker', projectDir), + ]) + : ['', '', '']; const phaseNumber = (phaseInfo?.phase_number as string) || null; const plans = (phaseInfo?.plans || []) as string[]; @@ -520,16 +519,14 @@ export const initQuick: QueryHandler = async (args, projectDir) => { : null; const configExists = existsSync(join(planningDir, 'config.json')); - const [plannerModelRaw, executorModelRaw, checkerModelRaw, verifierModelRaw] = await Promise.all([ - getModelAlias('gsd-planner', projectDir), - getModelAlias('gsd-executor', projectDir), - getModelAlias('gsd-plan-checker', projectDir), - getModelAlias('gsd-verifier', projectDir), - ]); - const plannerModel = configExists ? plannerModelRaw : ''; - const executorModel = configExists ? executorModelRaw : ''; - const checkerModel = configExists ? checkerModelRaw : ''; - const verifierModel = configExists ? verifierModelRaw : ''; + const [plannerModel, executorModel, checkerModel, verifierModel] = configExists + ? await Promise.all([ + getModelAlias('gsd-planner', projectDir), + getModelAlias('gsd-executor', projectDir), + getModelAlias('gsd-plan-checker', projectDir), + getModelAlias('gsd-verifier', projectDir), + ]) + : ['', '', '', '']; const result: Record = { planner_model: plannerModel, @@ -599,12 +596,12 @@ export const initVerifyWork: QueryHandler = async (args, projectDir) => { const { phaseInfo } = await getPhaseInfoForVerifyWork(phase, projectDir); const configExists = existsSync(join(projectDir, '.planning', 'config.json')); - const [plannerModelRaw, checkerModelRaw] = await Promise.all([ - getModelAlias('gsd-planner', projectDir), - getModelAlias('gsd-plan-checker', projectDir), - ]); - const plannerModel = configExists ? plannerModelRaw : ''; - const checkerModel = configExists ? checkerModelRaw : ''; + const [plannerModel, checkerModel] = configExists + ? await Promise.all([ + getModelAlias('gsd-planner', projectDir), + getModelAlias('gsd-plan-checker', projectDir), + ]) + : ['', '']; const result: Record = { planner_model: plannerModel, diff --git a/sdk/src/query/state-mutation.test.ts b/sdk/src/query/state-mutation.test.ts index fd2ac0723..c9ad57ea4 100644 --- a/sdk/src/query/state-mutation.test.ts +++ b/sdk/src/query/state-mutation.test.ts @@ -349,6 +349,14 @@ describe('stateBeginPhase', () => { ).rejects.toThrow('missing value for --phase'); }); + it('bug-2420: flag parser throws when a flag is last token with no value', async () => { + const { stateBeginPhase } = await import('./state-mutation.js'); + + await expect( + stateBeginPhase(['--name', 'Title', '--plans', '1', '--phase'], tmpDir) + ).rejects.toThrow('missing value for --phase'); + }); + it('does not treat argv after named flags as positional name/plans', async () => { const { stateBeginPhase } = await import('./state-mutation.js'); diff --git a/sdk/src/query/state-mutation.ts b/sdk/src/query/state-mutation.ts index cacae1262..2537dbdda 100644 --- a/sdk/src/query/state-mutation.ts +++ b/sdk/src/query/state-mutation.ts @@ -321,7 +321,7 @@ export async function readModifyWriteStateMdFull( * * @param args - args[0]: field name, args[1]: new value * @param projectDir - Project root directory - * @returns QueryResult with { updated: true/false, field, value } + * @returns QueryResult with { updated: true/false } */ export const stateUpdate: QueryHandler = async (args, projectDir, workstream) => { const field = args[0];