* fix(#1581): config-set no longer silently coerces Infinity/project_code The value parser used !isNaN(Number(val)), which admits Infinity/-Infinity; JSON.stringify then renders those as null on disk while the CLI echoed the non-finite value (output ≠ disk). Leading-zero strings like project_code '007' were also silently number-coerced to 7. - config.cts parser: Number.isFinite instead of !isNaN, so Infinity falls through to the JSON branch (rejected) and stays a string. - project_code: always persisted as a string (identifier; leading zeros matter), bypassing number coercion. - context_window: new per-key validator — must be a finite positive integer (rejects Infinity/0/negatives/non-integers with a non-zero exit). - tests/config.test.cjs: #1581 regression (Infinity rejected, 0 rejected, 200000 accepted finite, project_code '007' string-preserved, granularity numeric coercion unchanged). Closes #1581 * docs(#1581): backfill changeset pr 2023
This commit is contained in:
5
.changeset/1581-config-set-coercion.md
Normal file
5
.changeset/1581-config-set-coercion.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 2023
|
||||
---
|
||||
**`config-set` no longer silently coerces values into something the disk never sees** — `Number.isFinite` replaced `!isNaN` in the value parser so `Infinity`/`-Infinity` are no longer coerced to non-finite numbers that `JSON.stringify` then renders as `null` on disk while the CLI echoes `Infinity` (output ≠ disk). `context_window` now has a per-key validator requiring a finite positive integer (rejects `Infinity`, `0`, negatives, non-integers with a non-zero exit), and `project_code` is always persisted as a string so a leading-zero code like `007` survives verbatim instead of collapsing to `7`. Numeric coercion for genuine numeric keys (e.g. `granularity 42`) is unchanged. (#1581)
|
||||
@@ -586,11 +586,22 @@ function cmdConfigSet(cwd: string, keyPath: string | undefined, value: string |
|
||||
let parsedValue: unknown = val;
|
||||
if (val === 'true') parsedValue = true;
|
||||
else if (val === 'false') parsedValue = false;
|
||||
else if (!isNaN(Number(val)) && val !== '') parsedValue = Number(val);
|
||||
// #1581: Number.isFinite (not !isNaN) so 'Infinity'/'-Infinity' are NOT
|
||||
// coerced to non-finite numbers that JSON.stringify later renders as `null`
|
||||
// (disk=null while the CLI echoed 'Infinity'). They fall through to the
|
||||
// JSON branch (which rejects them) and stay strings, then per-key validators
|
||||
// reject them with a non-zero exit.
|
||||
else if (Number.isFinite(Number(val)) && val !== '') parsedValue = Number(val);
|
||||
else if (typeof val === 'string' && (val.startsWith('[') || val.startsWith('{'))) {
|
||||
try { parsedValue = JSON.parse(val); } catch { /* keep as string */ }
|
||||
}
|
||||
|
||||
// #1581: project_code is an identifier string — never number-coerce it. A
|
||||
// leading-zero code like '007' must persist verbatim (not collapse to 7).
|
||||
if (kp === 'project_code') {
|
||||
parsedValue = val;
|
||||
}
|
||||
|
||||
const VALID_CONTEXT_VALUES = ['dev', 'research', 'review'];
|
||||
if (kp === 'context') assertEnumValue(parsedValue, val, VALID_CONTEXT_VALUES, 'context value');
|
||||
|
||||
@@ -603,6 +614,15 @@ function cmdConfigSet(cwd: string, keyPath: string | undefined, value: string |
|
||||
}
|
||||
}
|
||||
|
||||
// #1581: context_window must be a finite positive integer. 'Infinity' is no
|
||||
// longer number-coerced (see the parse block above) so it reaches here as a
|
||||
// string and is rejected; '0', negatives, and non-integers are also rejected.
|
||||
if (kp === 'context_window') {
|
||||
if (typeof parsedValue !== 'number' || !Number.isFinite(parsedValue) || !Number.isInteger(parsedValue) || parsedValue < 1) {
|
||||
error(`Invalid context_window '${val}'. Must be a positive integer (token count).`, ERROR_REASON.USAGE);
|
||||
}
|
||||
}
|
||||
|
||||
// Post-planning gap checker (#2493)
|
||||
if (kp === 'workflow.post_planning_gaps') {
|
||||
if (typeof parsedValue !== 'boolean') {
|
||||
|
||||
@@ -692,6 +692,61 @@ describe('config-new-project command', () => {
|
||||
});
|
||||
});
|
||||
|
||||
// ─── config-set silent coercion (#1581) ──────────────────────────────────────
|
||||
|
||||
describe('config-set — no silent coercion (#1581)', () => {
|
||||
let tmpDir;
|
||||
|
||||
beforeEach(() => { tmpDir = createTempProject(); });
|
||||
afterEach(() => { cleanup(tmpDir); });
|
||||
|
||||
test('context_window Infinity is rejected (no null-on-disk / output≠disk divergence)', () => {
|
||||
const result = runGsdTools('config-set context_window Infinity', tmpDir);
|
||||
assert.strictEqual(result.success, false, 'Infinity must be rejected');
|
||||
assert.match(result.error, /context_window/i);
|
||||
// The old bug number-coerced Infinity, then JSON.stringify rendered it as
|
||||
// `null` on disk while the CLI echoed 'Infinity'. The fix rejects before
|
||||
// any write, so config.json is left untouched (no null entry written).
|
||||
assert.doesNotThrow(() => {
|
||||
const configPath = path.join(tmpDir, '.planning', 'config.json');
|
||||
if (!fs.existsSync(configPath)) return; // never written — the rejection path
|
||||
const cfg = JSON.parse(fs.readFileSync(configPath, 'utf-8'));
|
||||
assert.ok(cfg.context_window !== null && cfg.context_window !== Infinity,
|
||||
'context_window must not be written as null/Infinity');
|
||||
});
|
||||
});
|
||||
|
||||
test('context_window 0 is rejected (must be a positive integer)', () => {
|
||||
const result = runGsdTools('config-set context_window 0', tmpDir);
|
||||
assert.strictEqual(result.success, false, '0 must be rejected');
|
||||
assert.match(result.error, /positive integer/i);
|
||||
});
|
||||
|
||||
test('context_window <positive integer> is accepted and persisted as a finite number', () => {
|
||||
const result = runGsdTools('config-set context_window 200000', tmpDir);
|
||||
assert.ok(result.success, `Command failed: ${result.error}`);
|
||||
const config = readConfig(tmpDir);
|
||||
assert.strictEqual(config.context_window, 200000);
|
||||
assert.strictEqual(typeof config.context_window, 'number');
|
||||
assert.ok(Number.isFinite(config.context_window), 'must be finite on disk');
|
||||
});
|
||||
|
||||
test('project_code 007 persists as the string "007" (leading zero preserved, not coerced to 7)', () => {
|
||||
const result = runGsdTools('config-set project_code 007', tmpDir);
|
||||
assert.ok(result.success, `Command failed: ${result.error}`);
|
||||
const config = readConfig(tmpDir);
|
||||
assert.strictEqual(config.project_code, '007');
|
||||
assert.strictEqual(typeof config.project_code, 'string',
|
||||
'project_code is an identifier string — must not be number-coerced');
|
||||
});
|
||||
|
||||
test('regression guard: numeric coercion still works for numeric keys (granularity 42)', () => {
|
||||
const result = runGsdTools('config-set granularity 42', tmpDir);
|
||||
assert.ok(result.success, `Command failed: ${result.error}`);
|
||||
assert.strictEqual(readConfig(tmpDir).granularity, 42);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── config-set (research_before_questions and discuss_mode) ──────────────────
|
||||
|
||||
describe('config-set research_before_questions and discuss_mode', () => {
|
||||
|
||||
Reference in New Issue
Block a user