golem15/msd-core
3
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

fix(#4053): quote decimal-shaped frontmatter scalars for spec YAML readers (#4165)

Browse Source 
* 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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YBicDMJyh3AH56ZFbUsyxC

* chore(changeset): add Fixed fragment for #4053

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Qc7VN4zTpTSDTS9JXM2cFB

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Tom Boucher <trekkie@nomorestars.com>
This commit is contained in:
aaka3207
2026-09-05 06:17:41 -05:00
committed by GitHub
parent 723ea08dc2
commit 294ec29857
5 changed files with 85 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
.changeset/quote-decimal-frontmatter-scalars.md Normal file
View File

@@ -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)

21
src/frontmatter.cts
View File

@@ -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}`);

59
tests/frontmatter.test.cjs
View File

@@ -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).

2
tests/state-transition.test.cjs
View File

@@ -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)"', () => {

4
tests/state.test.cjs
View File

@@ -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',