From 294ec2985784877039b9e4ce005375e95e3f3099 Mon Sep 17 00:00:00 2001 From: aaka3207 <16450271+aaka3207@users.noreply.github.com> Date: Sat, 5 Sep 2026 06:17:41 -0500 Subject: [PATCH] fix(#4053): quote decimal-shaped frontmatter scalars for spec YAML readers (#4165) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(frontmatter): quote decimal-shaped scalars so a spec YAML reader preserves them A decimal phase identifier written to STATE.md frontmatter (e.g. `current_phase: 22.10`) was emitted BARE, because `scalarNeedsDoubleQuoting` only asks whether a value can OPEN a plain scalar — which `22.10` can. A YAML-spec reader (js-yaml, the statusline, any external tool) then reloads bare `22.10` as the float 22.1, colliding with `22.1` and dropping the trailing zero. gsd's own tolerant line-scanner (`extractFrontmatter`) round-trips the raw text and so hid the defect; a spec reader does not. Fix: `reconstructFrontmatter`'s general scalar path now also quotes numeric- looking strings that are not plain all-digit integers (decimals, exponents, sexagesimal, hex/oct/bin) via `generalScalarNeedsNumericQuoting`, reusing the existing `YAML_NUMERIC_RE`. Every all-digit string — integer counts, phase numbers, and leading-zero fixtures like `02` — stays bare, so the state-rebuild idempotency baseline and the rest of the state corpus are unchanged. This also quotes `gsd_state_version: 1.0` on write, which matches the authoritative STATE.md template (`src/state.cts` already emits it quoted). Regression test drives the real write path and asserts, via js-yaml, that `22.1` and `22.10` no longer collide and read back string-typed; guards that integers and free-text stay unquoted. Fixes #4053 Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01YBicDMJyh3AH56ZFbUsyxC * chore(changeset): add Fixed fragment for #4053 Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01YBicDMJyh3AH56ZFbUsyxC * docs(frontmatter): trim the generalScalarNeedsNumericQuoting comment Cut the over-long doc block down to the essential why and drop the inline comment that repeated it. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_011ZKeSj55VakqQajtoBgCTC * docs(test): drop the #4053 explanatory comments from the touched tests The assertions speak for themselves; remove the added narrative comments. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_011ZKeSj55VakqQajtoBgCTC * fix(#4053): correct the trade-off comment, changeset PR number, and cover every claimed numeric form Review follow-ups (trek-e): - The doc comment claimed a plain integer round-trips harmlessly. That is false for leading-zero values (`02` -> 2, `017` -> 17 under js-yaml). Rewrite it to state the real, deliberate trade-off: all-digit strings stay bare because zero-padded ids (`plan: 01`, `phase: 02`) are the pervasive GSD convention and quoting them all is the blanket quoting #4053 asked to avoid; the loss is padding not identity (`02` and `2` normalize to the same phase, `22.1` and `22.10` do not). - Changeset carried the auto-closed draft's number (4151); correct to 4165. - Test exponent, hex, octal, binary and sexagesimal forms through js-yaml, and pin the leading-zero trade-off so the documented behaviour is asserted. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01Qc7VN4zTpTSDTS9JXM2cFB --------- Co-authored-by: Claude Opus 4.8 Co-authored-by: Tom Boucher --- .../quote-decimal-frontmatter-scalars.md | 5 ++ src/frontmatter.cts | 21 ++++++- tests/frontmatter.test.cjs | 59 ++++++++++++++++++- tests/state-transition.test.cjs | 2 +- tests/state.test.cjs | 4 +- 5 files changed, 85 insertions(+), 6 deletions(-) create mode 100644 .changeset/quote-decimal-frontmatter-scalars.md 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',