fix(#2544): exit 1 on missing key in config-get (#2588)

The configGet query handler previously threw GSDError with
ErrorClassification.Validation, which maps to exit code 10. Callers
using `if ! gsd-sdk query config-get key; then fallback; fi` could
not detect missing keys through the exit code alone, because exit 10
is still truthy-failure but the intent (and documented UNIX
convention — cf. `git config --get`) is exit 1 for absent key.

Change the classification for the two 'Key not found' throw sites to
ErrorClassification.Execution so the CLI exits 1 on missing key.
Usage/schema errors (no key argument, malformed JSON, missing
config.json) remain Validation.

Closes #2544
This commit is contained in:
Tom Boucher
2026-04-22 12:04:03 -04:00
committed by GitHub
parent 2404b40a15
commit 7032f44633
2 changed files with 40 additions and 3 deletions

View File

@@ -6,7 +6,7 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import { mkdtemp, writeFile, mkdir, rm } from 'node:fs/promises';
import { join } from 'node:path';
import { tmpdir } from 'node:os';
import { GSDError } from '../errors.js';
import { GSDError, ErrorClassification, exitCodeFor } from '../errors.js';
// ─── Test setup ─────────────────────────────────────────────────────────────
@@ -58,6 +58,41 @@ describe('configGet', () => {
await expect(configGet(['nonexistent.key'], tmpDir)).rejects.toThrow(GSDError);
});
it('throws GSDError that maps to exit code 1 for missing key (bug #2544)', async () => {
const { configGet } = await import('./config-query.js');
await writeFile(
join(tmpDir, '.planning', 'config.json'),
JSON.stringify({ model_profile: 'quality' }),
);
try {
await configGet(['nonexistent.key'], tmpDir);
throw new Error('expected configGet to throw for missing key');
} catch (err) {
expect(err).toBeInstanceOf(GSDError);
const gsdErr = err as GSDError;
// UNIX convention: missing config key should exit 1 (like `git config --get`).
// Validation (exit 10) is the previous buggy classification — see issue #2544.
expect(gsdErr.classification).toBe(ErrorClassification.Execution);
expect(exitCodeFor(gsdErr.classification)).toBe(1);
}
});
it('throws GSDError that maps to exit code 1 when traversing into non-object (bug #2544)', async () => {
const { configGet } = await import('./config-query.js');
await writeFile(
join(tmpDir, '.planning', 'config.json'),
JSON.stringify({ model_profile: 'quality' }),
);
try {
await configGet(['model_profile.subkey'], tmpDir);
throw new Error('expected configGet to throw');
} catch (err) {
expect(err).toBeInstanceOf(GSDError);
const gsdErr = err as GSDError;
expect(exitCodeFor(gsdErr.classification)).toBe(1);
}
});
it('reads raw config without merging defaults', async () => {
const { configGet } = await import('./config-query.js');
// Write config with only model_profile -- no workflow section

View File

@@ -104,12 +104,14 @@ export const configGet: QueryHandler = async (args, projectDir, _workstream) =>
let current: unknown = config;
for (const key of keys) {
if (current === undefined || current === null || typeof current !== 'object') {
throw new GSDError(`Key not found: ${keyPath}`, ErrorClassification.Validation);
// UNIX convention (cf. `git config --get`): missing key exits 1, not 10.
// See issue #2544 — callers use `if ! gsd-sdk query config-get k; then` patterns.
throw new GSDError(`Key not found: ${keyPath}`, ErrorClassification.Execution);
}
current = (current as Record<string, unknown>)[key];
}
if (current === undefined) {
throw new GSDError(`Key not found: ${keyPath}`, ErrorClassification.Validation);
throw new GSDError(`Key not found: ${keyPath}`, ErrorClassification.Execution);
}
return { data: current };