refactor(sdk): extract GSDTools transport seam + policy (#3058)

* refactor(sdk): extract gsdtools transport seam with per-command policy

* fix(sdk): address CodeRabbit transport policy and timeout findings

* fix(sdk): harden raw transport formatting and raw-path coverage
This commit is contained in:
Tom Boucher
2026-05-03 08:20:05 -04:00
committed by GitHub
parent eb365f7336
commit 0f98952a3d
7 changed files with 562 additions and 116 deletions

View File

@@ -0,0 +1,5 @@
---
type: Fixed
pr: 3058
---
**GSD transport raw-mode handling and timeout fallback hardened** — fixes undefined raw formatting edge case and adds raw-path coverage to prevent regressions.

View File

@@ -1,5 +1,6 @@
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import { GSDTools, GSDToolsError, resolveGsdToolsPath } from './gsd-tools.js';
import { setTransportPolicy, clearTransportPolicy } from './gsd-transport-policy.js';
import { mkdir, writeFile, rm } from 'node:fs/promises';
import { existsSync } from 'node:fs';
import { join } from 'node:path';
@@ -22,6 +23,7 @@ describe('GSDTools', () => {
});
afterEach(async () => {
clearTransportPolicy();
await rm(tmpDir, { recursive: true, force: true });
});
@@ -162,6 +164,41 @@ describe('GSDTools', () => {
expect(gsdErr.message).toContain('timed out');
}
}, 10_000);
it('uses subprocess fallback when native handler throws and policy allows fallback', async () => {
const scriptPath = await createScript(
'fallback-ok.cjs',
`process.stdout.write(JSON.stringify({ from: 'subprocess-fallback' }));`,
);
const tools = new GSDTools({ projectDir: tmpDir, gsdToolsPath: scriptPath });
setTransportPolicy('verify.path-exists', { allowFallbackToSubprocess: true });
const result = await tools.exec('verify.path-exists', []);
expect(result).toEqual({ from: 'subprocess-fallback' });
});
it('preserves GSDToolsError contract when native handler throws and fallback disabled', async () => {
const scriptPath = await createScript(
'should-not-run.cjs',
`process.stdout.write(JSON.stringify({ should: 'not-run' }));`,
);
const tools = new GSDTools({ projectDir: tmpDir, gsdToolsPath: scriptPath });
setTransportPolicy('verify.path-exists', { allowFallbackToSubprocess: false });
try {
await tools.exec('verify.path-exists', []);
expect.fail('Should have thrown');
} catch (err) {
expect(err).toBeInstanceOf(GSDToolsError);
const gsdErr = err as GSDToolsError;
expect(gsdErr.command).toBe('verify.path-exists');
expect(gsdErr.args).toEqual([]);
expect(gsdErr.stderr).toBe('');
expect(typeof gsdErr.exitCode === 'number').toBe(true);
}
});
});
// ─── Typed method tests ────────────────────────────────────────────────

View File

@@ -23,6 +23,9 @@ import { createRegistry } from './query/index.js';
import { resolveQueryArgv } from './query/registry.js';
import { normalizeQueryCommand } from './query/normalize-query-command.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';
// ─── Error type ──────────────────────────────────────────────────────────────
@@ -109,6 +112,7 @@ export class GSDTools {
private readonly workstream?: string;
private readonly registry: ReturnType<typeof createRegistry>;
private readonly preferNativeQuery: boolean;
private readonly transport: GSDTransport;
constructor(opts: {
projectDir: string;
@@ -132,6 +136,16 @@ export class GSDTools {
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<QueryResult>,
execSubprocessJson: async (legacyCommand, legacyArgs) => this.execSubprocessJson(legacyCommand, legacyArgs),
execSubprocessRaw: async (legacyCommand, legacyArgs) => this.execSubprocessRaw(legacyCommand, legacyArgs),
formatNativeRaw: (registryCommand, data) => formatRegistryRawStdout(registryCommand, data),
});
}
private shouldUseNativeQuery(): boolean {
@@ -262,101 +276,30 @@ export class GSDTools {
/**
* Execute a gsd-tools command and return parsed JSON output.
* Handles the `@file:` prefix pattern for large results.
*
* With native query enabled, a matching registry handler runs in-process;
* if that handler throws, the error is surfaced (no automatic fallback to `gsd-tools.cjs`).
*/
async exec(command: string, args: string[] = []): Promise<unknown> {
if (this.shouldUseNativeQuery()) {
const matched = this.nativeMatch(command, args);
if (matched) {
try {
const result = await this.withRegistryDispatchTimeout(
command,
args,
this.registry.dispatch(matched.cmd, matched.args, this.projectDir),
);
return result.data;
} catch (err) {
if (err instanceof GSDToolsError) throw err;
throw this.toToolsError(command, args, err);
}
}
}
const matched = this.nativeMatch(command, args);
const registryCommand = matched?.cmd ?? command;
const registryArgs = matched?.args ?? args;
const policy = resolveTransportPolicy(registryCommand);
const wsArgs = this.workstream ? ['--ws', this.workstream] : [];
const fullArgs = [this.gsdToolsPath, command, ...args, ...wsArgs];
return new Promise<unknown>((resolve, reject) => {
const child = execFile(
process.execPath,
fullArgs,
{
cwd: this.projectDir,
maxBuffer: 10 * 1024 * 1024, // 10MB
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,
'',
),
);
try {
return await this.transport.run({
legacyCommand: command,
legacyArgs: args,
registryCommand,
registryArgs,
mode: policy.outputMode,
projectDir: this.projectDir,
workstream: this.workstream,
}, {
preferNative: this.shouldUseNativeQuery() && policy.preferNative,
allowFallbackToSubprocess: policy.allowFallbackToSubprocess,
});
});
} catch (err) {
if (err instanceof GSDToolsError) throw err;
throw this.toToolsError(command, args, err);
}
}
/**
@@ -390,23 +333,98 @@ export class GSDTools {
* Use for commands like `config-set` that return plain text, not JSON.
*/
async execRaw(command: string, args: string[] = []): Promise<string> {
if (this.shouldUseNativeQuery()) {
const matched = this.nativeMatch(command, args);
if (matched) {
try {
const result = await this.withRegistryDispatchTimeout(
command,
args,
this.registry.dispatch(matched.cmd, matched.args, this.projectDir),
);
return formatRegistryRawStdout(matched.cmd, result.data).trim();
} catch (err) {
if (err instanceof GSDToolsError) throw err;
throw this.toToolsError(command, args, err);
}
}
}
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;
} catch (err) {
if (err instanceof GSDToolsError) throw err;
throw this.toToolsError(command, args, err);
}
}
private async execSubprocessJson(command: string, args: string[]): Promise<unknown> {
const wsArgs = this.workstream ? ['--ws', this.workstream] : [];
const fullArgs = [this.gsdToolsPath, command, ...args, ...wsArgs];
return new Promise<unknown>((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<string> {
const wsArgs = this.workstream ? ['--ws', this.workstream] : [];
const fullArgs = [this.gsdToolsPath, command, ...args, ...wsArgs, '--raw'];
@@ -439,15 +457,7 @@ export class GSDTools {
);
child.on('error', (err) => {
reject(
new GSDToolsError(
`Failed to execute gsd-tools: ${err.message}`,
command,
args,
null,
'',
),
);
reject(new GSDToolsError(`Failed to execute gsd-tools: ${err.message}`, command, args, null, ''));
});
});
}

View File

@@ -0,0 +1,34 @@
import { describe, it, expect, afterEach } from 'vitest';
import { resolveTransportPolicy, setTransportPolicy, clearTransportPolicy } from './gsd-transport-policy.js';
describe('gsd-transport-policy', () => {
afterEach(() => {
clearTransportPolicy();
});
it('uses legacy-safe defaults for unknown command', () => {
const policy = resolveTransportPolicy('unknown-cmd');
expect(policy.preferNative).toBe(true);
expect(policy.allowFallbackToSubprocess).toBe(true);
expect(policy.outputMode).toBe('json');
});
it('applies built-in raw output override', () => {
const policy = resolveTransportPolicy('config-set');
expect(policy.outputMode).toBe('raw');
expect(policy.allowFallbackToSubprocess).toBe(true);
});
it('applies verify-summary alias raw overrides', () => {
expect(resolveTransportPolicy('verify-summary').outputMode).toBe('raw');
expect(resolveTransportPolicy('verify.summary').outputMode).toBe('raw');
expect(resolveTransportPolicy('verify summary').outputMode).toBe('raw');
});
it('supports per-command override updates', () => {
setTransportPolicy('state', { allowFallbackToSubprocess: false, outputMode: 'raw' });
const policy = resolveTransportPolicy('state');
expect(policy.allowFallbackToSubprocess).toBe(false);
expect(policy.outputMode).toBe('raw');
});
});

View File

@@ -0,0 +1,54 @@
export type TransportMode = 'json' | 'raw';
export interface TransportPolicy {
preferNative: boolean;
allowFallbackToSubprocess: boolean;
outputMode: TransportMode;
}
const DEFAULT_POLICY: TransportPolicy = {
preferNative: true,
allowFallbackToSubprocess: true,
outputMode: 'json',
};
const BUILTIN_COMMAND_POLICY: Record<string, Partial<TransportPolicy>> = {
// raw stdout contracts
commit: { outputMode: 'raw' },
'config-set': { outputMode: 'raw' },
'verify-summary': { outputMode: 'raw' },
'verify.summary': { outputMode: 'raw' },
'verify summary': { outputMode: 'raw' },
// native-first/hard-fail examples (can expand later)
// 'state.load': { allowFallbackToSubprocess: false, outputMode: 'raw' },
};
const COMMAND_POLICY_OVERRIDES: Record<string, Partial<TransportPolicy>> = {};
export function resolveTransportPolicy(command: string): TransportPolicy {
const override = {
...(BUILTIN_COMMAND_POLICY[command] ?? {}),
...(COMMAND_POLICY_OVERRIDES[command] ?? {}),
};
return {
preferNative: override.preferNative ?? DEFAULT_POLICY.preferNative,
allowFallbackToSubprocess:
override.allowFallbackToSubprocess ?? DEFAULT_POLICY.allowFallbackToSubprocess,
outputMode: override.outputMode ?? DEFAULT_POLICY.outputMode,
};
}
export function setTransportPolicy(command: string, override: Partial<TransportPolicy>): void {
COMMAND_POLICY_OVERRIDES[command] = { ...(COMMAND_POLICY_OVERRIDES[command] ?? {}), ...override };
}
export function clearTransportPolicy(command?: string): void {
if (command) {
delete COMMAND_POLICY_OVERRIDES[command];
return;
}
for (const key of Object.keys(COMMAND_POLICY_OVERRIDES)) {
delete COMMAND_POLICY_OVERRIDES[key];
}
}

View File

@@ -0,0 +1,236 @@
import { describe, it, expect, vi } from 'vitest';
import { QueryRegistry } from './query/registry.js';
import { GSDTransport } from './gsd-transport.js';
describe('GSDTransport', () => {
it('uses native adapter when command registered and policy prefers native', async () => {
const registry = new QueryRegistry();
registry.register('state.load', async () => ({ data: { ok: true } }));
const adapters = {
dispatchNative: vi.fn(async () => ({ data: { ok: true } })),
execSubprocessJson: vi.fn(async () => ({ ok: false })),
execSubprocessRaw: vi.fn(async () => 'subprocess'),
};
const transport = new GSDTransport(registry, adapters);
const result = await transport.run({
legacyCommand: 'state',
legacyArgs: ['load'],
registryCommand: 'state.load',
registryArgs: [],
mode: 'json',
projectDir: '/tmp',
}, {
preferNative: true,
allowFallbackToSubprocess: true,
});
expect(result).toEqual({ ok: true });
expect(adapters.dispatchNative).toHaveBeenCalledOnce();
expect(adapters.execSubprocessJson).not.toHaveBeenCalled();
});
it('falls back to subprocess when native throws and policy allows fallback', async () => {
const registry = new QueryRegistry();
registry.register('state.load', async () => ({ data: { ok: true } }));
const adapters = {
dispatchNative: vi.fn(async () => {
throw new Error('native failed');
}),
execSubprocessJson: vi.fn(async () => ({ ok: 'fallback' })),
execSubprocessRaw: vi.fn(async () => 'fallback-raw'),
};
const transport = new GSDTransport(registry, adapters);
const result = await transport.run({
legacyCommand: 'state',
legacyArgs: ['load'],
registryCommand: 'state.load',
registryArgs: [],
mode: 'json',
projectDir: '/tmp',
}, {
preferNative: true,
allowFallbackToSubprocess: true,
});
expect(result).toEqual({ ok: 'fallback' });
expect(adapters.dispatchNative).toHaveBeenCalledOnce();
expect(adapters.execSubprocessJson).toHaveBeenCalledOnce();
});
it('hard-fails when native throws and fallback disabled', async () => {
const registry = new QueryRegistry();
registry.register('state.load', async () => ({ data: { ok: true } }));
const adapters = {
dispatchNative: vi.fn(async () => {
throw new Error('native failed');
}),
execSubprocessJson: vi.fn(async () => ({ ok: 'fallback' })),
execSubprocessRaw: vi.fn(async () => 'fallback-raw'),
};
const transport = new GSDTransport(registry, adapters);
await expect(transport.run({
legacyCommand: 'state',
legacyArgs: ['load'],
registryCommand: 'state.load',
registryArgs: [],
mode: 'json',
projectDir: '/tmp',
}, {
preferNative: true,
allowFallbackToSubprocess: false,
})).rejects.toThrow('native failed');
expect(adapters.execSubprocessJson).not.toHaveBeenCalled();
});
it('does not fallback after timeout-like native error', async () => {
const registry = new QueryRegistry();
registry.register('state.load', async () => ({ data: { ok: true } }));
const adapters = {
dispatchNative: vi.fn(async () => {
throw new Error('gsd-tools timed out after 500ms: state load');
}),
execSubprocessJson: vi.fn(async () => ({ ok: 'fallback' })),
execSubprocessRaw: vi.fn(async () => 'fallback-raw'),
};
const transport = new GSDTransport(registry, adapters);
await expect(transport.run({
legacyCommand: 'state',
legacyArgs: ['load'],
registryCommand: 'state.load',
registryArgs: [],
mode: 'json',
projectDir: '/tmp',
}, {
preferNative: true,
allowFallbackToSubprocess: true,
})).rejects.toThrow('timed out after');
expect(adapters.execSubprocessJson).not.toHaveBeenCalled();
});
it('formats native raw output via formatNativeRaw when provided', async () => {
const registry = new QueryRegistry();
registry.register('commit', async () => ({ data: { hash: 'abc123' } }));
const adapters = {
dispatchNative: vi.fn(async () => ({ data: { hash: 'abc123' } })),
execSubprocessJson: vi.fn(async () => ({ ok: false })),
execSubprocessRaw: vi.fn(async () => 'subprocess-raw'),
formatNativeRaw: vi.fn(() => 'raw-native-output'),
};
const transport = new GSDTransport(registry, adapters);
const result = await transport.run({
legacyCommand: 'commit',
legacyArgs: ['msg'],
registryCommand: 'commit',
registryArgs: ['msg'],
mode: 'raw',
projectDir: '/tmp',
}, {
preferNative: true,
allowFallbackToSubprocess: true,
});
expect(result).toBe('raw-native-output');
expect(adapters.formatNativeRaw).toHaveBeenCalledOnce();
expect(adapters.execSubprocessRaw).not.toHaveBeenCalled();
});
it('falls back to internal raw formatter when formatNativeRaw missing', async () => {
const registry = new QueryRegistry();
registry.register('commit', async () => ({ data: undefined }));
const adapters = {
dispatchNative: vi.fn(async () => ({ data: undefined })),
execSubprocessJson: vi.fn(async () => ({ ok: false })),
execSubprocessRaw: vi.fn(async () => 'subprocess-raw'),
};
const transport = new GSDTransport(registry, adapters);
const result = await transport.run({
legacyCommand: 'commit',
legacyArgs: ['msg'],
registryCommand: 'commit',
registryArgs: ['msg'],
mode: 'raw',
projectDir: '/tmp',
}, {
preferNative: true,
allowFallbackToSubprocess: true,
});
expect(result).toBe('');
expect(adapters.execSubprocessRaw).not.toHaveBeenCalled();
});
it('forces subprocess when workstream present', async () => {
const registry = new QueryRegistry();
registry.register('state.load', async () => ({ data: { ok: true } }));
const adapters = {
dispatchNative: vi.fn(async () => ({ data: { ok: true } })),
execSubprocessJson: vi.fn(async () => ({ ok: 'ws-subprocess' })),
execSubprocessRaw: vi.fn(async () => 'ws-subprocess-raw'),
};
const transport = new GSDTransport(registry, adapters);
const result = await transport.run({
legacyCommand: 'state',
legacyArgs: ['load'],
registryCommand: 'state.load',
registryArgs: [],
mode: 'json',
projectDir: '/tmp',
workstream: 'ws-1',
}, {
preferNative: true,
allowFallbackToSubprocess: true,
});
expect(result).toEqual({ ok: 'ws-subprocess' });
expect(adapters.dispatchNative).not.toHaveBeenCalled();
expect(adapters.execSubprocessJson).toHaveBeenCalledOnce();
});
it('forces raw subprocess path when workstream present and mode is raw', async () => {
const registry = new QueryRegistry();
registry.register('commit', async () => ({ data: { hash: 'abc' } }));
const adapters = {
dispatchNative: vi.fn(async () => ({ data: { hash: 'abc' } })),
execSubprocessJson: vi.fn(async () => ({ ok: 'json-subprocess' })),
execSubprocessRaw: vi.fn(async () => 'raw-subprocess'),
};
const transport = new GSDTransport(registry, adapters);
const result = await transport.run({
legacyCommand: 'commit',
legacyArgs: ['msg'],
registryCommand: 'commit',
registryArgs: ['msg'],
mode: 'raw',
projectDir: '/tmp',
workstream: 'ws-1',
}, {
preferNative: true,
allowFallbackToSubprocess: true,
});
expect(result).toBe('raw-subprocess');
expect(adapters.dispatchNative).not.toHaveBeenCalled();
expect(adapters.execSubprocessRaw).toHaveBeenCalledOnce();
expect(adapters.execSubprocessJson).not.toHaveBeenCalled();
});
});

70
sdk/src/gsd-transport.ts Normal file
View File

@@ -0,0 +1,70 @@
import type { QueryResult } from './query/utils.js';
import type { QueryRegistry } from './query/registry.js';
import type { TransportMode } from './gsd-transport-policy.js';
export interface TransportRequest {
legacyCommand: string;
legacyArgs: string[];
registryCommand: string;
registryArgs: string[];
mode: TransportMode;
projectDir: string;
workstream?: string;
}
export interface TransportAdapters {
dispatchNative: (request: TransportRequest) => Promise<QueryResult>;
execSubprocessJson: (legacyCommand: string, legacyArgs: string[]) => Promise<unknown>;
execSubprocessRaw: (legacyCommand: string, legacyArgs: string[]) => Promise<string>;
formatNativeRaw?: (registryCommand: string, data: unknown) => string;
}
export interface TransportPolicyLike {
preferNative: boolean;
allowFallbackToSubprocess: boolean;
}
function isTimeoutLikeError(error: unknown): boolean {
if (!(error instanceof Error)) return false;
if (error.name === 'TimeoutError' || error.name === 'AbortError') return true;
return error.message.includes('timed out after');
}
export class GSDTransport {
constructor(
private readonly registry: QueryRegistry,
private readonly adapters: TransportAdapters,
) {}
async run(request: TransportRequest, policy: TransportPolicyLike): Promise<unknown> {
const forceSubprocess = Boolean(request.workstream);
if (!forceSubprocess && policy.preferNative && this.registry.has(request.registryCommand)) {
try {
const native = await this.adapters.dispatchNative(request);
if (request.mode === 'raw') {
if (this.adapters.formatNativeRaw) {
return this.adapters.formatNativeRaw(request.registryCommand, native.data).trim();
}
return this.toRaw(native.data);
}
return native.data;
} catch (error) {
if (isTimeoutLikeError(error)) throw error;
if (!policy.allowFallbackToSubprocess) throw error;
}
}
if (request.mode === 'raw') {
return this.adapters.execSubprocessRaw(request.legacyCommand, request.legacyArgs);
}
return this.adapters.execSubprocessJson(request.legacyCommand, request.legacyArgs);
}
private toRaw(data: unknown): string {
if (typeof data === 'string') return data.trim();
const json = JSON.stringify(data, null, 2);
if (json == null) return '';
return json.trim();
}
}