fix(#3657): tolerate commonmark fence widths in ledger readers (#3733)

* test(#3657): fence-width tolerance regression rows

* test(#3657): fix pure-row fixtures to use appendWindow result shape

* fix(#3657): tolerate commonmark fence widths in ledger readers

* fix(#3657): restore throw-block indentation in parseJsonBlock

* chore(#3657): backfill changeset pr number

---------

Co-authored-by: sim <sim@local>
This commit is contained in:
Tom Boucher
2026-08-21 03:45:06 -04:00
committed by GitHub
parent 72819a4616
commit 1f76861202
5 changed files with 380 additions and 31 deletions

View File

@@ -0,0 +1,5 @@
---
type: Fixed
pr: 3733
---
**`windows` ledger commands survive a formatter pass** — the WINDOWS.md ledger's JSON block is written with a four-backtick fence, which Prettier and other CommonMark formatters legally narrow to three; the reader then rejected the file and every `gsd-tools windows` subcommand (status/append/waive/fixed) failed with "Ledger missing JSON code block". The reader now accepts any CommonMark-legal fence width (the writer still emits four), resolves the real block past fences planted in entry descriptions, and preserves user prose below the ledger; the refactor-trigger proposal reader gets the same fence tolerance. (#3657)

View File

@@ -19,9 +19,9 @@
* ---
* # Broken Windows Ledger
* <human-readable prose>
* ```json
* ````json
* [ <entries array, canonical JSON> ]
* ```
* ````
*
* Frontmatter holds scalar counts (the FAST path the ship gate reads via jq
* without parsing JSON). The JSON code block is the AUTHORITATIVE entries
@@ -379,6 +379,84 @@ const JSON_FENCE_OPEN = '````json';
const JSON_FENCE_CLOSE = '````';
const FORBIDDEN_BACKTICK_RUN = '````';
// Reader-side fence tolerance (#3657): CommonMark formatters (Prettier et al.)
// normalize the written 4-backtick fence down to the shortest legal width (3)
// whenever the block body holds no backtick run — and a canonical-JSON ledger
// body never does. Both widths are valid CommonMark, so the reader locates the
// block by a line-anchored 3+ fence and closes on a run at least as wide as
// the opening one (CommonMark: a shorter run does not close). The writer above
// is unchanged — 4 backticks stay what renderLedger emits (#1950 review H1).
type JsonBlockSpan = { bodyStart: number; bodyEnd: number; afterClose: number };
type JsonBlockLookup =
| { ok: true; span: JsonBlockSpan }
| { ok: false; reason: 'missing-open' | 'unterminated' };
/**
* Locate the entries JSON block by CommonMark fence rules rather than a fixed
* literal width. Both parseJsonBlock (strict) and writeLedgerAtomic's #2893
* prose preservation (lenient) go through this one function so read tolerance
* and splice tolerance cannot drift (#3657). A backtick-only line can never
* occur inside a body: JSON.stringify renders strings single-line-escaped, so
* an inline run inside a description is never a close-fence candidate.
*
* Disambiguation (#3657 security review): an entry description may contain
* newlines and 3-backtick runs (append validation rejects only 4+ runs), and
* renderTable renders descriptions into the prose ABOVE the JSON block — so
* hostile or accidental text can plant a second json fence above the real
* one. renderLedger always emits the entries block as the FINAL fenced
* section, so spans are scanned in REVERSE: prefer the latest span whose
* entries length equals the frontmatter total_count (the real block always
* satisfies it — parseLedger cross-checks that invariant), else the latest
* span whose body is a JSON array, else the first span so corrupt bodies keep
* their fail-closed parse errors. A mirror planted below with identical
* length and identical entries is indistinguishable by construction — and
* harmless.
*/
function locateJsonBlock(raw: string, expectedTotal?: number): JsonBlockLookup {
const spans: JsonBlockSpan[] = [];
for (const open of raw.matchAll(/^(`{3,})json[ \t]*\r?$/gm)) {
const width = open[1].length;
const bodyStart = (open.index ?? 0) + open[0].length;
for (const close of raw.slice(bodyStart).matchAll(/^(`{3,})[ \t]*\r?$/gm)) {
if (close[1].length < width) continue;
const bodyEnd = bodyStart + (close.index ?? 0);
const closeLineEnd = raw.indexOf('\n', bodyEnd);
spans.push({
bodyStart,
bodyEnd,
afterClose: closeLineEnd === -1 ? raw.length : closeLineEnd + 1,
});
break; // CommonMark: the first qualifying close ends this fence block
}
}
if (spans.length === 0) {
const sawOpen = /^(`{3,})json[ \t]*\r?$/m.test(raw);
return { ok: false, reason: sawOpen ? 'unterminated' : 'missing-open' };
}
const parseBody = (s: JsonBlockSpan): unknown => {
try {
return JSON.parse(raw.slice(s.bodyStart, s.bodyEnd).trim());
} catch {
return undefined;
}
};
if (expectedTotal !== undefined) {
for (let i = spans.length - 1; i >= 0; i--) {
const body = parseBody(spans[i]);
if (Array.isArray(body) && body.length === expectedTotal) {
return { ok: true, span: spans[i] };
}
}
}
for (let i = spans.length - 1; i >= 0; i--) {
if (Array.isArray(parseBody(spans[i]))) {
return { ok: true, span: spans[i] };
}
}
return { ok: true, span: spans[0] };
}
/**
* Minimal strict frontmatter parser for flat scalar keys. Only supports the
* shape this module emits: `key: <number|string>` per line. Throws on any
@@ -433,22 +511,17 @@ function parseFrontmatterStrict(raw: string): Record<string, number | string> {
return out;
}
function parseJsonBlock(raw: string): WindowEntry[] {
const start = raw.indexOf(JSON_FENCE_OPEN);
if (start === -1) {
function parseJsonBlock(raw: string, expectedTotal?: number): WindowEntry[] {
const span = locateJsonBlock(raw, expectedTotal);
if (!span.ok) {
throw new WindowsError(
REASON.WINDOWS_LEDGER_MALFORMED,
'Ledger missing JSON code block for entries.',
span.reason === 'missing-open'
? 'Ledger missing JSON code block for entries.'
: 'Ledger JSON code block not terminated.',
);
}
const end = raw.indexOf(JSON_FENCE_CLOSE, start + JSON_FENCE_OPEN.length);
if (end === -1) {
throw new WindowsError(
REASON.WINDOWS_LEDGER_MALFORMED,
'Ledger JSON code block not terminated.',
);
}
const jsonText = raw.slice(start + JSON_FENCE_OPEN.length, end).trim();
const jsonText = raw.slice(span.span.bodyStart, span.span.bodyEnd).trim();
let parsed: unknown;
try {
parsed = JSON.parse(jsonText);
@@ -556,7 +629,7 @@ export function parseLedger(raw: string): Ledger {
);
}
const entries = parseJsonBlock(raw);
const entries = parseJsonBlock(raw, typeof fm.total_count === 'number' ? fm.total_count : undefined);
const ledger: Ledger = {
schema_version: SCHEMA_VERSION,
open_count: typeof fm.open_count === 'number' ? fm.open_count : 0,
@@ -734,17 +807,16 @@ function writeLedgerAtomic(cwd: string, ledger: Ledger): void {
let trailingProse = '';
try {
const existing = fs.readFileSync(p, 'utf8');
// #2893: search for the CLOSING fence starting AFTER the opening fence,
// mirroring parseJsonBlock — indexOf(JSON_FENCE_CLOSE) alone would match
// the opening fence ('````json' starts with '````').
const openIdx = existing.indexOf(JSON_FENCE_OPEN);
if (openIdx !== -1) {
const fenceEnd = existing.indexOf(JSON_FENCE_CLOSE, openIdx + JSON_FENCE_OPEN.length);
if (fenceEnd !== -1) {
const afterFence = existing.slice(fenceEnd + JSON_FENCE_CLOSE.length);
// Drop leading newlines; keep the rest as prose.
trailingProse = afterFence.replace(/^(?:\r?\n)+/, '');
}
// #2893: search for the CLOSING fence starting AFTER the opening fence.
// The span is located with the same tolerant + disambiguated fence rules
// parseJsonBlock uses (#3657), so a formatter-normalized 3-backtick ledger
// keeps its prose too — a literal-width search here would find no block
// and silently drop everything below the ledger on the next write.
const span = locateJsonBlock(existing, ledger.total_count);
if (span.ok) {
const afterFence = existing.slice(span.span.afterClose);
// Drop leading newlines; keep the rest as prose.
trailingProse = afterFence.replace(/^(?:\r?\n)+/, '');
}
} catch {
// File doesn't exist yet (first write) — no prose to preserve.

View File

@@ -874,6 +874,29 @@ export function reanchorBaseline(
const PROPOSAL_JSON_FENCE_OPEN = '````json';
const PROPOSAL_JSON_FENCE_CLOSE = '````';
// Reader-side fence tolerance (#3657 — same defect class as the WINDOWS.md
// ledger): CommonMark formatters (Prettier et al.) narrow the written
// 4-backtick fence to the shortest legal width (3) whenever the body holds no
// backtick run, and a canonical-JSON candidates array never does. Locate the
// block by a line-anchored 3+ fence and close on a run at least as wide
// (CommonMark: a shorter run does not close). The writer above is unchanged.
// This module is a leaf (CONTEXT.md — imports only node:fs/node:path), so the
// span logic is local rather than imported from broken-windows.
const PROPOSAL_FENCE_OPEN_RE = /^(`{3,})json[ \t]*\r?$/m;
function locateProposalJsonBlock(text: string): { jsonText: string } | null {
const open = text.match(PROPOSAL_FENCE_OPEN_RE);
if (!open || open.index === undefined) return null;
const width = open[1].length;
const bodyStart = open.index + open[0].length;
for (const close of text.slice(bodyStart).matchAll(/^(`{3,})[ \t]*\r?$/gm)) {
if (close[1].length < width) continue;
const bodyEnd = close.index ?? 0;
return { jsonText: text.slice(bodyStart, bodyStart + bodyEnd).trim() };
}
return null;
}
export function renderProposal(p: Proposal): string {
const fm = [
'---',
@@ -931,11 +954,9 @@ export function parseProposal(text: string): Proposal | null {
fm[m[1]] = m[2].trim();
}
const jsonStart = text.indexOf(PROPOSAL_JSON_FENCE_OPEN);
if (jsonStart === -1) return null;
const jsonEnd = text.indexOf(PROPOSAL_JSON_FENCE_CLOSE, jsonStart + PROPOSAL_JSON_FENCE_OPEN.length);
if (jsonEnd === -1) return null;
const jsonText = text.slice(jsonStart + PROPOSAL_JSON_FENCE_OPEN.length, jsonEnd).trim();
const span = locateProposalJsonBlock(text);
if (span === null) return null;
const jsonText = span.jsonText;
let candidates: unknown;
try {
candidates = JSON.parse(jsonText);

View File

@@ -399,6 +399,222 @@ describe('broken-windows: parseLedger fail-closed', () => {
// CLI: gsd-tools windows status (acceptance: clean-ship on empty)
// ---------------------------------------------------------------------------
// ---------------------------------------------------------------------------
// #3657: fence-width tolerant read (formatter-normalized ledgers)
// ---------------------------------------------------------------------------
// The formatter itself is never spawned here: the input class is "a ledger a
// CommonMark formatter already normalized" (Prettier narrows the written
// 4-backtick fence to the shortest legal width — 3 — because a canonical-JSON
// body never contains a backtick run). Narrowing a rendered ledger's fences
// reproduces that state deterministically.
describe('broken-windows: fence-width tolerant read (#3657)', () => {
/** Narrow a rendered ledger text's fences to `width` backticks. */
function narrowFences(raw, width = 3) {
return raw
.replace(/^````json$/m, '`'.repeat(width) + 'json')
.replace(/^````$/m, '`'.repeat(width));
}
/** Rendered ledger with its fences narrowed to `width` backticks. */
function renderNarrowed(ledger, width = 3) {
return narrowFences(renderLedger(ledger), width);
}
/** Narrow the fences of an on-disk ledger in place (the formatter's effect). */
function narrowLedgerOnDisk(p, width = 3) {
fs.writeFileSync(p, narrowFences(fs.readFileSync(p, 'utf8'), width), 'utf8');
}
/** Ledger with one open stub entry, built through the pure API. */
function ledgerWithEntry(description) {
const { ledger } = appendWindow(
emptyLedger('2026-07-19T00:00:00Z'),
{ kind: 'stub', phase: '2', description },
{ now: '2026-07-19T12:00:00Z' }
);
return ledger;
}
test('parseLedger accepts a formatter-narrowed 3-backtick JSON fence (#3657)', () => {
const parsed = parseLedger(renderNarrowed(ledgerWithEntry('narrowed fence entry')));
assert.equal(parsed.entries.length, 1);
assert.equal(parsed.entries[0].description, 'narrowed fence entry');
assert.equal(parsed.open_count, 1);
});
test('windows status recovers on a formatter-normalized ledger (#3657)', (t) => {
const tmp = createTempDir();
t.after(() => cleanup(tmp));
const r0 = runGsdTools(
['windows', 'append', '--kind', 'todo', '--phase', '2', '--description', 'normalized ledger entry'],
tmp
);
assert.ok(r0.success, `seed append failed: ${r0.error || ''}`);
narrowLedgerOnDisk(path.join(tmp, '.planning', LEDGER_FILE_NAME));
const res = runGsdTools(['windows', 'status', '--raw'], tmp);
assert.ok(res.success, `status must recover on a normalized ledger: ${res.error || ''}`);
const obj = JSON.parse(res.output);
assert.equal(obj.ok, true);
assert.equal(obj.ledger.open_count, 1);
});
test('windows append/waive/fixed recover on a normalized ledger and re-emit the 4-fence writer form (#3657)', (t) => {
const tmp = createTempDir();
t.after(() => cleanup(tmp));
const ledgerPath = path.join(tmp, '.planning', LEDGER_FILE_NAME);
const r0 = runGsdTools(
['windows', 'append', '--kind', 'todo', '--phase', '2', '--description', 'first'],
tmp
);
assert.ok(r0.success, `seed append failed: ${r0.error || ''}`);
narrowLedgerOnDisk(ledgerPath);
const rAppend = runGsdTools(
['windows', 'append', '--kind', 'todo', '--phase', '2', '--description', 'second'],
tmp
);
assert.ok(rAppend.success, `append must recover on a normalized ledger: ${rAppend.error || ''}`);
narrowLedgerOnDisk(ledgerPath);
const rWaive = runGsdTools(['windows', 'waive', '1', 'duplicate of second'], tmp);
assert.ok(rWaive.success, `waive must recover on a normalized ledger: ${rWaive.error || ''}`);
narrowLedgerOnDisk(ledgerPath);
const rFixed = runGsdTools(['windows', 'fixed', '2'], tmp);
assert.ok(rFixed.success, `fixed must recover on a normalized ledger: ${rFixed.error || ''}`);
// Writer contract unchanged: after any write the ledger is back on the
// 4-backtick fence form renderLedger emits (#1950 review H1).
const after = fs.readFileSync(ledgerPath, 'utf8');
assert.match(after, /^````json$/m, 'rewritten ledger must re-emit the 4-backtick writer fence');
assert.doesNotMatch(after, /^```json$/m, 'the 3-backtick form is a formatter artifact, never written');
const status = runGsdTools(['windows', 'status', '--raw'], tmp);
assert.ok(status.success, `final status failed: ${status.error || ''}`);
assert.equal(JSON.parse(status.output).ledger.open_count, 0);
});
test('windows append preserves trailing prose on a normalized ledger (#2893 via #3657)', (t) => {
const tmp = createTempDir();
t.after(() => cleanup(tmp));
const ledgerPath = path.join(tmp, '.planning', LEDGER_FILE_NAME);
const r0 = runGsdTools(
['windows', 'append', '--kind', 'todo', '--phase', '2', '--description', 'prose carrier'],
tmp
);
assert.ok(r0.success, `seed append failed: ${r0.error || ''}`);
// User prose below the closing fence (#2893), then a formatter pass.
const withProse = fs.readFileSync(ledgerPath, 'utf8') + 'Manual notes below the ledger.\n';
fs.writeFileSync(ledgerPath, withProse, 'utf8');
narrowLedgerOnDisk(ledgerPath);
const rAppend = runGsdTools(
['windows', 'append', '--kind', 'todo', '--phase', '2', '--description', 'second'],
tmp
);
assert.ok(rAppend.success, `append on normalized ledger failed: ${rAppend.error || ''}`);
const after = fs.readFileSync(ledgerPath, 'utf8');
assert.ok(
after.includes('Manual notes below the ledger.'),
'trailing prose must survive a write to a formatter-normalized ledger'
);
});
test('renderLedger keeps the 4-backtick writer fence (#3657)', () => {
const out = renderLedger(emptyLedger());
assert.match(out, /^````json$/m, 'writer must keep the #1950 H1 4-backtick open fence');
assert.match(out, /^````$/m, 'writer must keep the 4-backtick close fence');
});
test('fence tolerance does not loosen malformed-ledger fail-closed (#3657)', () => {
const frontmatter = [
'---',
'schema_version: 1',
'open_count: 0',
'waived_count: 0',
'fixed_count: 0',
'total_count: 0',
'last_updated: 2026-07-19T00:00:00Z',
'---',
].join('\n');
const noBlock = [frontmatter, '', '# Broken Windows Ledger', '', 'prose only', ''].join('\n');
assert.throws(() => parseLedger(noBlock), reasonIs(REASON.WINDOWS_LEDGER_MALFORMED));
assert.throws(() => parseLedger(noBlock), /missing JSON code block/);
const body = JSON.stringify([]);
const unterminated = [frontmatter, '', '```json', body, ''].join('\n');
assert.throws(() => parseLedger(unterminated), reasonIs(REASON.WINDOWS_LEDGER_MALFORMED));
assert.throws(() => parseLedger(unterminated), /not terminated/);
});
test('reader accepts 3+ widths and rejects a shorter closing run (#3657)', () => {
const ledger = ledgerWithEntry('width boundary entry');
const five = renderNarrowed(ledger, 5);
const parsedFive = parseLedger(five);
assert.equal(parsedFive.entries.length, 1, 'a 5-backtick fence is valid CommonMark and must parse');
// CommonMark: the closing run must be at least as long as the opening run.
const shortClose = renderLedger(ledger).replace(/^````$/m, '```');
assert.throws(
() => parseLedger(shortClose),
reasonIs(REASON.WINDOWS_LEDGER_MALFORMED),
'a 3-backtick line must not close a 4-backtick block'
);
});
test('3-backtick run inside a description never terminates the block (#1950 H1 under #3657 tolerance)', () => {
const description = 'see ```js x``` inline';
const ledger = ledgerWithEntry(description);
const parsed4 = parseLedger(renderLedger(ledger));
assert.equal(parsed4.entries[0].description, description, '4-fence roundtrip keeps the inline run');
// A hand-narrowed 3-fence file: the inline ``` sits inside a JSON string on
// a content line, so the line-anchored close scan must skip it.
const parsed3 = parseLedger(renderNarrowed(ledger));
assert.equal(parsed3.entries[0].description, description);
});
test('fence tolerance is CRLF-safe (#3116 sibling)', () => {
const crlf = renderNarrowed(ledgerWithEntry('crlf narrowed entry')).replace(/\n/g, '\r\n');
const parsed = parseLedger(crlf);
assert.equal(parsed.entries.length, 1);
assert.equal(parsed.entries[0].description, 'crlf narrowed entry');
});
test('a json fence planted in a description never hijacks or bricks the ledger (#3657 security)', () => {
// renderTable renders descriptions into the prose ABOVE the JSON block,
// and append validation rejects only 4+ backtick runs (#1950 H1) — so a
// hostile or accidental description can plant a second json fence above
// the real one. The reader must resolve to the REAL block: renderLedger
// always emits it as the final fenced section, and the counts cross-check
// pins it. Both the smuggled-entries variant and the empty-array (brick)
// variant must fail to influence the parse.
const plantedBodies = [
'[{"id":99,"kind":"stub","phase":"9","file":"","line":null,"description":"SMUGGLED","status":"open","reason":"","recorded_at":"t","resolved_at":null}]',
'[]',
];
for (const body of plantedBodies) {
const hostile = `see old snapshot:\n\`\`\`json\n${body}\n\`\`\`\nend`;
const ledger = ledgerWithEntry(hostile);
const rendered = renderLedger(ledger);
const parsed = parseLedger(rendered);
assert.equal(parsed.entries.length, 1, `planted fence must not replace the entries: ${body.slice(0, 12)}`);
assert.equal(parsed.entries[0].id, 1);
assert.notEqual(parsed.entries[0].description, 'SMUGGLED');
assert.ok(parsed.entries[0].description.includes('see old snapshot'));
// Same file after a formatter narrows every fence to three backticks.
const parsedNarrowed = parseLedger(narrowFences(rendered));
assert.equal(parsedNarrowed.entries[0].id, 1, 'narrowed planted ledger still resolves the real block');
assert.notEqual(parsedNarrowed.entries[0].description, 'SMUGGLED');
}
});
});
describe('broken-windows CLI: windows status', () => {
test('status on a project with no ledger returns open_count=0 (backward-compat baseline)', (t) => {
const tmp = createTempDir('bw-status-empty-');

View File

@@ -958,3 +958,38 @@ describe('complexity-trigger: typed surface', () => {
assert.equal(SCHEMA_VERSION, 1);
});
});
describe('complexity-trigger: proposal fence-width tolerant read (#3657)', () => {
test('parseProposal accepts a formatter-narrowed 3-backtick fence (#3657)', () => {
const { renderProposal, parseProposal } = require('../gsd-core/bin/lib/complexity-trigger.cjs');
const proposal = {
schema_version: 1,
status: 'proposed',
phase: '2',
target_file: 'src/a.ts',
target_function: 'handleThing',
score: 7,
baseline: 5,
delta: 2,
metric: 'decision-points',
recorded_at: '2026-07-19T00:00:00Z',
resolved_at: null,
reason: 'score above threshold',
candidates: [{ name: 'handleThing', score: 7 }],
};
const rendered = renderProposal(proposal);
const parsed4 = parseProposal(rendered);
assert.notEqual(parsed4, null, 'writer form must round-trip');
// Same artifact after a CommonMark formatter narrows the fence to the
// shortest legal width — the identical #3657 defect class.
const narrowed = rendered
.replace(/^````json$/m, '```json')
.replace(/^````$/m, '```');
const parsed3 = parseProposal(narrowed);
assert.notEqual(parsed3, null, 'a formatter-narrowed proposal must parse');
assert.equal(parsed3.status, 'proposed');
assert.equal(parsed3.candidates.length, 1);
assert.equal(parsed3.candidates[0].name, 'handleThing');
});
});