diff --git a/.changeset/brave-elks-climb.md b/.changeset/brave-elks-climb.md new file mode 100644 index 000000000..65a038096 --- /dev/null +++ b/.changeset/brave-elks-climb.md @@ -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) diff --git a/src/broken-windows.cts b/src/broken-windows.cts index 67fcae63c..d0a9d5793 100644 --- a/src/broken-windows.cts +++ b/src/broken-windows.cts @@ -19,9 +19,9 @@ * --- * # Broken Windows Ledger * - * ```json + * ````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: ` per line. Throws on any @@ -433,22 +511,17 @@ function parseFrontmatterStrict(raw: string): Record { 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. diff --git a/src/complexity-trigger.cts b/src/complexity-trigger.cts index 8aac67bab..30c19b12f 100644 --- a/src/complexity-trigger.cts +++ b/src/complexity-trigger.cts @@ -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); diff --git a/tests/broken-windows.test.cjs b/tests/broken-windows.test.cjs index 640f3f516..56ce91513 100644 --- a/tests/broken-windows.test.cjs +++ b/tests/broken-windows.test.cjs @@ -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-'); diff --git a/tests/complexity-trigger.test.cjs b/tests/complexity-trigger.test.cjs index 7d8760c47..9f873559c 100644 --- a/tests/complexity-trigger.test.cjs +++ b/tests/complexity-trigger.test.cjs @@ -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'); + }); +});