* fix(#2803): honor --default flag in SDK config-get handler The gsd-sdk query config-get handler ignored the --default <value> flag. Missing keys always threw 'Key not found' (exit 1), making 8 workflow sites that rely on config-get --default fall through to error paths. The CJS path (gsd-tools.cjs) honored --default since #1893; this ports that behavior to the SDK configGet handler. Regression test: tests/bug-2803-config-get-default-flag.test.cjs * fix(#2803): address CodeRabbit — require --default value, keep missing config.json as error, bash fence
This commit is contained in:
@@ -80,9 +80,22 @@ export function getAgentToModelMapForProfile(normalizedProfile: string): Record<
|
||||
* @throws GSDError with Validation classification if key missing or not found
|
||||
*/
|
||||
export const configGet: QueryHandler = async (args, projectDir, workstream) => {
|
||||
const keyPath = args[0];
|
||||
// Support --default <value> flag (#2803): return this value (exit 0) when the
|
||||
// key is absent, mirroring gsd-tools.cjs config-get behavior from #1893.
|
||||
const defaultIdx = args.indexOf('--default');
|
||||
let defaultValue: string | undefined;
|
||||
let filteredArgs = args;
|
||||
if (defaultIdx !== -1) {
|
||||
if (defaultIdx + 1 >= args.length) {
|
||||
throw new GSDError('Usage: config-get <key.path> [--default <value>]', ErrorClassification.Validation);
|
||||
}
|
||||
defaultValue = String(args[defaultIdx + 1]);
|
||||
filteredArgs = [...args.slice(0, defaultIdx), ...args.slice(defaultIdx + 2)];
|
||||
}
|
||||
|
||||
const keyPath = filteredArgs[0];
|
||||
if (!keyPath) {
|
||||
throw new GSDError('Usage: config-get <key.path>', ErrorClassification.Validation);
|
||||
throw new GSDError('Usage: config-get <key.path> [--default <value>]', ErrorClassification.Validation);
|
||||
}
|
||||
|
||||
const paths = planningPaths(projectDir, workstream);
|
||||
@@ -106,11 +119,13 @@ export const configGet: QueryHandler = async (args, projectDir, workstream) => {
|
||||
if (current === undefined || current === null || typeof current !== 'object') {
|
||||
// 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.
|
||||
if (defaultValue !== undefined) return { data: defaultValue };
|
||||
throw new GSDError(`Key not found: ${keyPath}`, ErrorClassification.Execution);
|
||||
}
|
||||
current = (current as Record<string, unknown>)[key];
|
||||
}
|
||||
if (current === undefined) {
|
||||
if (defaultValue !== undefined) return { data: defaultValue };
|
||||
throw new GSDError(`Key not found: ${keyPath}`, ErrorClassification.Execution);
|
||||
}
|
||||
|
||||
|
||||
132
tests/bug-2803-config-get-default-flag.test.cjs
Normal file
132
tests/bug-2803-config-get-default-flag.test.cjs
Normal file
@@ -0,0 +1,132 @@
|
||||
/**
|
||||
* Regression test for bug #2803
|
||||
*
|
||||
* `gsd-sdk query config-get <key> --default <value>` silently ignored the
|
||||
* --default flag. When the key was missing, the SDK threw "Error: Key not found"
|
||||
* and exited 1, identical to calling it without --default.
|
||||
*
|
||||
* The CJS path (gsd-tools.cjs config-get <key> --default <value>) honored
|
||||
* --default correctly since #1893. The SDK handler was never ported.
|
||||
*
|
||||
* Fix: configGet in sdk/src/query/config-query.ts now strips --default <value>
|
||||
* from args before key lookup and returns { data: defaultValue } instead of
|
||||
* throwing when the key is absent (config missing, key missing, or nested
|
||||
* object missing).
|
||||
*/
|
||||
|
||||
'use strict';
|
||||
|
||||
const { describe, test, beforeEach, afterEach } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('node:fs');
|
||||
const path = require('node:path');
|
||||
const { execFileSync } = require('node:child_process');
|
||||
const { createTempProject, cleanup } = require('./helpers.cjs');
|
||||
|
||||
const REPO_ROOT = path.join(__dirname, '..');
|
||||
const SDK_CLI = path.join(REPO_ROOT, 'sdk', 'dist', 'cli.js');
|
||||
|
||||
/**
|
||||
* Invoke `gsd-sdk query config-get <...args>` against a project dir.
|
||||
* Returns { exitCode, stdout, stderr }.
|
||||
*/
|
||||
function runConfigGet(args, projectDir) {
|
||||
const argv = ['query', 'config-get', ...args, '--project-dir', projectDir];
|
||||
let stdout = '';
|
||||
let stderr = '';
|
||||
let exitCode = 0;
|
||||
try {
|
||||
stdout = execFileSync(process.execPath, [SDK_CLI, ...argv], {
|
||||
encoding: 'utf-8',
|
||||
stdio: ['pipe', 'pipe', 'pipe'],
|
||||
env: { ...process.env, GSD_SESSION_KEY: '' },
|
||||
});
|
||||
} catch (err) {
|
||||
exitCode = err.status ?? 1;
|
||||
stdout = err.stdout?.toString() ?? '';
|
||||
stderr = err.stderr?.toString() ?? '';
|
||||
}
|
||||
let json = null;
|
||||
try { json = JSON.parse(stdout.trim()); } catch { /* ok */ }
|
||||
return { exitCode, stdout: stdout.trim(), stderr: stderr.trim(), json };
|
||||
}
|
||||
|
||||
describe('bug-2803: config-get --default flag honored in SDK', () => {
|
||||
let tmpDir;
|
||||
|
||||
beforeEach(() => {
|
||||
tmpDir = createTempProject('gsd-test-2803-');
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
cleanup(tmpDir);
|
||||
});
|
||||
|
||||
test('--default returns fallback value when key absent, exit 0', () => {
|
||||
// Write a config without the key
|
||||
fs.writeFileSync(
|
||||
path.join(tmpDir, '.planning', 'config.json'),
|
||||
JSON.stringify({ mode: 'balanced' })
|
||||
);
|
||||
|
||||
const result = runConfigGet(
|
||||
['workflow.test_command', '--default', 'FALLBACK'],
|
||||
tmpDir
|
||||
);
|
||||
|
||||
assert.strictEqual(result.exitCode, 0, 'should exit 0 when --default provided');
|
||||
assert.ok(result.json !== null, 'should emit JSON');
|
||||
assert.strictEqual(result.json, 'FALLBACK', 'data should be the default value');
|
||||
});
|
||||
|
||||
test('--default not consumed as key path', () => {
|
||||
// The key path should still be the first positional, not --default
|
||||
fs.writeFileSync(
|
||||
path.join(tmpDir, '.planning', 'config.json'),
|
||||
JSON.stringify({ mode: 'quality' })
|
||||
);
|
||||
|
||||
const result = runConfigGet(['mode', '--default', 'balanced'], tmpDir);
|
||||
|
||||
assert.strictEqual(result.exitCode, 0, 'should exit 0 for found key');
|
||||
assert.strictEqual(result.json, 'quality', 'should return actual value when key exists');
|
||||
});
|
||||
|
||||
test('--default with empty string value works', () => {
|
||||
fs.writeFileSync(
|
||||
path.join(tmpDir, '.planning', 'config.json'),
|
||||
JSON.stringify({})
|
||||
);
|
||||
|
||||
const result = runConfigGet(['missing.key', '--default', ''], tmpDir);
|
||||
|
||||
assert.strictEqual(result.exitCode, 0, 'should exit 0 with empty default');
|
||||
assert.strictEqual(result.json, '', 'data should be empty string');
|
||||
});
|
||||
|
||||
test('without --default: missing key still exits 1', () => {
|
||||
fs.writeFileSync(
|
||||
path.join(tmpDir, '.planning', 'config.json'),
|
||||
JSON.stringify({ mode: 'balanced' })
|
||||
);
|
||||
|
||||
const result = runConfigGet(['workflow.missing_key'], tmpDir);
|
||||
|
||||
assert.strictEqual(result.exitCode, 1, 'should exit 1 when key absent and no --default');
|
||||
});
|
||||
|
||||
test('--default with nested missing path', () => {
|
||||
fs.writeFileSync(
|
||||
path.join(tmpDir, '.planning', 'config.json'),
|
||||
JSON.stringify({ workflow: {} })
|
||||
);
|
||||
|
||||
const result = runConfigGet(
|
||||
['workflow.test_command', '--default', 'npm test'],
|
||||
tmpDir
|
||||
);
|
||||
|
||||
assert.strictEqual(result.exitCode, 0, 'should exit 0 with default for nested missing key');
|
||||
assert.strictEqual(result.json, 'npm test', 'data should be the default value');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user