fix: address CodeRabbit findings on query/transport error handling

This commit is contained in:
Tom Boucher
2026-05-04 21:47:10 -04:00
parent 1642f47908
commit 40acf1f02e
6 changed files with 38 additions and 10 deletions

View File

@@ -13,4 +13,9 @@ describe('GSDToolsError constructors', () => {
expect(err.classification).toEqual({ kind: 'failure' });
expect(err.exitCode).toBe(1);
});
it('defaults direct constructor to failure classification', () => {
const err = new GSDToolsError('boom', 'state', ['load'], 1, 'stderr');
expect(err.classification).toEqual({ kind: 'failure' });
});
});

View File

@@ -22,7 +22,7 @@ export class GSDToolsError extends Error {
) {
super(message, options);
this.name = 'GSDToolsError';
this.classification = options?.classification;
this.classification = options?.classification ?? failureClassification();
}
static timeout(
@@ -61,5 +61,5 @@ export class GSDToolsError extends Error {
);
}
public readonly classification?: GSDToolsErrorClassification;
public readonly classification: GSDToolsErrorClassification;
}

View File

@@ -31,4 +31,9 @@ describe('formatQueryRawOutput', () => {
expect(formatQueryRawOutput('state begin-phase', { updated: ['x'] })).toBe('true');
expect(formatQueryRawOutput('state begin-phase', { updated: [] })).toBe('false');
});
it('never returns undefined for non-JSON top-level values', () => {
expect(formatQueryRawOutput('commit', undefined)).toBe('undefined');
expect(formatQueryRawOutput('commit', Symbol('x'))).toBe('Symbol(x)');
});
});

View File

@@ -1,5 +1,10 @@
import { formatStateLoadRawStdout } from './query/state-project-load.js';
function safeStringify(value: unknown): string {
const out = JSON.stringify(value, null, 2);
return out ?? String(value);
}
/**
* Raw output projection for native query results.
* Owns CLI-facing string contracts for raw mode commands.
@@ -11,7 +16,7 @@ export function formatQueryRawOutput(registryCommand: string, data: unknown): st
if (registryCommand === 'commit') {
if (data == null || typeof data !== 'object' || Array.isArray(data)) {
return JSON.stringify(data, null, 2);
return safeStringify(data);
}
const d = data as Record<string, unknown>;
if (d.committed === true) {
@@ -32,12 +37,12 @@ export function formatQueryRawOutput(registryCommand: string, data: unknown): st
}
return r || 'nothing';
}
return JSON.stringify(data, null, 2);
return safeStringify(data);
}
if (registryCommand === 'config-set') {
if (data == null || typeof data !== 'object' || Array.isArray(data)) {
return JSON.stringify(data, null, 2);
return safeStringify(data);
}
const d = data as Record<string, unknown>;
if ((d.updated === true || d.set === true) && d.key !== undefined) {
@@ -50,12 +55,12 @@ export function formatQueryRawOutput(registryCommand: string, data: unknown): st
}
return `${d.key}=${String(v)}`;
}
return JSON.stringify(data, null, 2);
return safeStringify(data);
}
if (registryCommand === 'state.begin-phase' || registryCommand === 'state begin-phase') {
if (data == null || typeof data !== 'object' || Array.isArray(data)) {
return JSON.stringify(data, null, 2);
return safeStringify(data);
}
const d = data as Record<string, unknown>;
const u = d.updated as string[] | undefined;
@@ -65,5 +70,5 @@ export function formatQueryRawOutput(registryCommand: string, data: unknown): st
if (typeof data === 'string') {
return data;
}
return JSON.stringify(data, null, 2);
return safeStringify(data);
}

View File

@@ -64,6 +64,17 @@ describe('QuerySubprocessAdapter', () => {
await expect(adapter.execJson('state', ['load'])).resolves.toEqual({ from: 'file' });
});
it('execJson resolves relative @file output against projectDir', async () => {
const relDir = join(dir, '.planning');
await mkdir(relDir, { recursive: true });
const relFile = join(relDir, 'out.json');
await writeFile(relFile, JSON.stringify({ from: 'relative-file' }));
const script = await createScript('file-relative.cjs', `process.stdout.write('@file:.planning/out.json');`);
const adapter = createAdapter(script);
await expect(adapter.execJson('state', ['load'])).resolves.toEqual({ from: 'relative-file' });
});
it('execRaw returns trimmed stdout', async () => {
const script = await createScript('raw.cjs', `process.stdout.write(' hello ');`);
const adapter = createAdapter(script);

View File

@@ -1,4 +1,5 @@
import { execFile } from 'node:child_process';
import { isAbsolute, resolve } from 'node:path';
import { readFile } from 'node:fs/promises';
import { timeoutMessage } from './query-failure-classification.js';
import type { QueryToolsErrorFactory } from './query-tools-error-factory.js';
@@ -131,11 +132,12 @@ export class QuerySubprocessAdapter {
let jsonStr = trimmed;
if (jsonStr.startsWith('@file:')) {
const filePath = jsonStr.slice(6).trim();
const resolvedPath = isAbsolute(filePath) ? filePath : resolve(this.deps.projectDir, filePath);
try {
jsonStr = await readFile(filePath, 'utf-8');
jsonStr = await readFile(resolvedPath, '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}`);
throw new Error(`Failed to read gsd-tools @file: indirection at "${resolvedPath}": ${reason}`);
}
}