diff --git a/.changeset/quote-decimal-frontmatter-scalars.md b/.changeset/quote-decimal-frontmatter-scalars.md new file mode 100644 index 000000000..ef50f1c88 --- /dev/null +++ b/.changeset/quote-decimal-frontmatter-scalars.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4165 +--- +**Decimal-shaped frontmatter scalars (e.g. a `22.10` phase id) are now quoted on write**, so a spec-compliant YAML reader preserves them as the exact string instead of reloading `22.10` as the float `22.1` — which collided with `22.1`, a different phase. Exponent, hex, octal and binary forms are quoted likewise. All-digit values (integer counts and zero-padded ids like `02`) stay unquoted as a deliberate scoped trade-off; `gsd_state_version` is now written `"1.0"`, matching the quoted form in the STATE.md template. (#4053) diff --git a/src/frontmatter.cts b/src/frontmatter.cts index d13afd600..a301d1f63 100644 --- a/src/frontmatter.cts +++ b/src/frontmatter.cts @@ -913,6 +913,23 @@ function agentScalarNeedsDoubleQuoting(s: string): boolean { return false; } +/** + * #4053 — Quote a numeric-looking scalar that is not all-digit (`22.10`, + * `1.0`, `1e3`, `0x1F`) so a spec YAML reader keeps it a string: bare `22.10` + * reloads as the float 22.1 and collides with `22.1`, a different phase. + * + * All-digit strings stay bare as a deliberate, scoped trade-off — not a + * safety guarantee. A leading-zero value (`02`, `017`) also mis-parses under + * a spec reader (to 2, 17). It is left unquoted because zero-padded ids + * (`plan: 01`, `phase: 02`) are the pervasive GSD convention, quoting them + * all is the blanket quoting #4053 asked to avoid, and the loss is padding + * rather than identity: `02` and `2` normalize to the same phase; `22.1` and + * `22.10` do not. + */ +function generalScalarNeedsNumericQuoting(s: string): boolean { + return YAML_NUMERIC_RE.test(s) && !/^\d+$/.test(s); +} + function reconstructFrontmatter(obj: Frontmatter): string { const lines: string[] = []; // #3257: read the full-line-comment channel (set by parseGuardedYamlRegion when comments @@ -978,12 +995,12 @@ function reconstructFrontmatter(obj: Frontmatter): string { } else { // eslint-disable-next-line @typescript-eslint/no-base-to-string const sv = String(subval); - lines.push(` ${subkey}: ${sv.includes(':') || sv.includes('#') || scalarNeedsDoubleQuoting(sv) ? `"${escapeDoubleQuotedScalar(sv)}"` : sv}`); + lines.push(` ${subkey}: ${sv.includes(':') || sv.includes('#') || scalarNeedsDoubleQuoting(sv) || generalScalarNeedsNumericQuoting(sv) ? `"${escapeDoubleQuotedScalar(sv)}"` : sv}`); } } } else { const sv = String(value); - if (sv.includes(':') || sv.includes('#') || sv.startsWith('[') || sv.startsWith('{') || scalarNeedsDoubleQuoting(sv)) { + if (sv.includes(':') || sv.includes('#') || sv.startsWith('[') || sv.startsWith('{') || scalarNeedsDoubleQuoting(sv) || generalScalarNeedsNumericQuoting(sv)) { lines.push(`${key}: "${escapeDoubleQuotedScalar(sv)}"`); } else { lines.push(`${key}: ${sv}`); diff --git a/tests/frontmatter.test.cjs b/tests/frontmatter.test.cjs index 4a1652e1e..b408cf79a 100644 --- a/tests/frontmatter.test.cjs +++ b/tests/frontmatter.test.cjs @@ -249,6 +249,63 @@ describe('reconstructFrontmatter', () => { assert.ok(hashResult.includes('"value # note"'), 'should quote value with hash'); }); + describe('#4053 — decimal-shaped numeric scalars survive a spec YAML reader', () => { + const yaml = require('js-yaml'); + const lineFor = (obj, key) => + reconstructFrontmatter(obj).split('\n').find((l) => l.startsWith(`${key}:`)); + + test('a decimal phase id is quoted so js-yaml keeps it a string', () => { + assert.strictEqual(lineFor({ current_phase: '22.1' }, 'current_phase'), 'current_phase: "22.1"'); + assert.strictEqual(lineFor({ current_phase: '22.10' }, 'current_phase'), 'current_phase: "22.10"'); + assert.strictEqual(lineFor({ current_phase: '22.0' }, 'current_phase'), 'current_phase: "22.0"'); + }); + + test('"22.1" and "22.10" no longer collide under js-yaml', () => { + const load = (v) => yaml.load(reconstructFrontmatter({ current_phase: v })).current_phase; + const a = load('22.1'); + const b = load('22.10'); + assert.strictEqual(a, '22.1'); + assert.strictEqual(b, '22.10'); // was the float 22.1 before the fix + assert.notStrictEqual(a, b); + assert.strictEqual(typeof b, 'string'); // was 'number' before the fix + }); + + test('a nested decimal value is also quoted', () => { + assert.strictEqual( + reconstructFrontmatter({ progress: { ratio: '1.10' } }), + 'progress:\n ratio: "1.10"', + ); + }); + + test('plain integer phase ids and counts stay bare — no idempotency churn', () => { + assert.strictEqual(lineFor({ current_phase: '3' }, 'current_phase'), 'current_phase: 3'); + assert.strictEqual(lineFor({ current_phase: '10' }, 'current_phase'), 'current_phase: 10'); + assert.strictEqual( + reconstructFrontmatter({ progress: { completed_phases: '2', percent: '40' } }), + 'progress:\n completed_phases: 2\n percent: 40', + ); + }); + + test('exponent, hex, octal, binary and sexagesimal forms are quoted and survive js-yaml', () => { + for (const v of ['1e3', '0x1F', '0o17', '0b101', '12:30']) { + assert.strictEqual(lineFor({ k: v }, 'k'), `k: "${v}"`, v); + assert.strictEqual(yaml.load(reconstructFrontmatter({ k: v })).k, v, v); + } + }); + + test('a leading-zero all-digit id stays bare — the documented, scoped trade-off', () => { + assert.strictEqual(lineFor({ current_phase: '02' }, 'current_phase'), 'current_phase: 02'); + assert.strictEqual(lineFor({ plan: '01' }, 'plan'), 'plan: 01'); + }); + + test('free-text with no YAML-special characters stays unquoted — no blanket quoting', () => { + assert.strictEqual( + lineFor({ current_phase_name: 'Test Phase' }, 'current_phase_name'), + 'current_phase_name: Test Phase', + ); + }); + }); + test('serializes nested objects with proper indentation', () => { const result = reconstructFrontmatter({ tech: { added: 'prisma', patterns: 'repo' } }); assert.ok(result.includes('tech:'), 'should have parent key'); @@ -312,7 +369,7 @@ describe('reconstructFrontmatter', () => { `comment should survive reconstruct; got:\n${reconstructed}`, ); // data identity preserved alongside the comment. - assert.ok(reconstructed.includes('gsd_state_version: 1.0')); + assert.ok(reconstructed.includes('gsd_state_version: "1.0"')); assert.ok(reconstructed.includes('current_phase: 3')); assert.ok(reconstructed.includes('status: executing')); // the reconstructed output re-parses to the same data (idempotent round-trip). diff --git a/tests/state-transition.test.cjs b/tests/state-transition.test.cjs index c2ac1dcc7..8eedd6053 100644 --- a/tests/state-transition.test.cjs +++ b/tests/state-transition.test.cjs @@ -1879,7 +1879,7 @@ describe('ADR-1769 Phase 4: milestoneSwitch transition — milestone reset', () test('gsd_state_version is preserved across the reset', () => { const result = transitionCore(milestoneBody(), { kind: 'milestoneSwitch', version: 'v2.0', name: 'New Milestone' }, deps); - assert.ok(/gsd_state_version:\s*1\.0/.test(result.content), 'gsd_state_version must be preserved'); + assert.ok(/gsd_state_version:\s*"?1\.0"?/.test(result.content), 'gsd_state_version must be preserved'); }); test('Current Position section is reset to "Not started (defining requirements)"', () => { diff --git a/tests/state.test.cjs b/tests/state.test.cjs index ee531fec1..5e38d1f7e 100644 --- a/tests/state.test.cjs +++ b/tests/state.test.cjs @@ -842,7 +842,7 @@ describe('STATE.md frontmatter sync', () => { const content = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8'); assert.ok(content.startsWith('---\n'), 'should start with frontmatter delimiter'); - assert.ok(content.includes('gsd_state_version: 1.0'), 'should have version field'); + assert.ok(content.includes('gsd_state_version: "1.0"'), 'should have version field'); assert.ok(content.includes('current_phase: 02'), 'frontmatter should have current phase'); assert.ok(content.includes('**Current Phase:** 02'), 'body field should be preserved'); assert.ok(content.includes('**Status:** Executing Plan 1'), 'updated field in body'); @@ -7126,7 +7126,7 @@ describe('ADR-3408 §8.5 Matrix (#3471): stale-but-present, and the report resid }), tmp); const expected = [ - '---', 'gsd_state_version: 1.0', 'status: unknown', 'last_updated: "2023-11-14T22:13:20.000Z"', + '---', 'gsd_state_version: "1.0"', 'status: unknown', 'last_updated: "2023-11-14T22:13:20.000Z"', 'stopped_at: Phase 5, curated stop', 'paused_at: Phase 5, curated pause', 'current_phase: 5', 'current_phase_name: Curated Name', 'current_plan: 05-02-plan', 'last_activity_desc: curated activity desc',