From 34633fa4ecbb5954fc5403711177d6952eb21673 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sat, 1 Aug 2026 12:59:09 -0400 Subject: [PATCH] fix(#2893): preserve prose below the JSON ledger on windows append/waive/fixed (#2975) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(#2893): add regression for append destroying prose below JSON ledger writeLedgerAtomic overwrites the entire file with renderLedger(ledger), dropping any prose below the JSON closing fence. The test creates a WINDOWS.md with prose sections below the ledger, appends an entry, and asserts the prose survives. * fix(#2893): preserve prose below the JSON ledger on append/waive/fixed writeLedgerAtomic was overwriting the entire WINDOWS.md with renderLedger(ledger), which reconstructs only the frontmatter + header + table + JSON block — silently destroying any prose a user wrote below the JSON closing fence. Now the writer reads the existing file before overwriting, extracts content after the closing fence, and appends it to the rendered ledger. First-write (no existing file) proceeds normally with no prose to preserve. * fix(#2893): address review — correct fence search + idempotency test BLOCKER from isolated adversarial review: indexOf(JSON_FENCE_CLOSE) matched the OPENING fence ('json' starts with ''), duplicating the entire JSON body as prose on every write. Now searches for the closing fence starting AFTER the opening fence, mirroring parseJsonBlock. Test hardened: non-empty initial ledger, second append (idempotency — prose appears exactly once, exactly one JSON fence open), parseLedger round-trip. * chore(#2893): add changeset fragment * chore(#2893): backfill changeset PR number 2975 --------- Co-authored-by: sim --- .changeset/clever-lemurs-snooze.md | 5 +++ src/broken-windows.cts | 28 +++++++++++++- tests/broken-windows.test.cjs | 60 ++++++++++++++++++++++++++++++ 3 files changed, 92 insertions(+), 1 deletion(-) create mode 100644 .changeset/clever-lemurs-snooze.md diff --git a/.changeset/clever-lemurs-snooze.md b/.changeset/clever-lemurs-snooze.md new file mode 100644 index 000000000..cd261c26c --- /dev/null +++ b/.changeset/clever-lemurs-snooze.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 2975 +--- +**`windows append`/`waive`/`fixed` no longer destroy prose below the JSON ledger** — the writer reconstructed the file from the parsed JSON ledger only, silently dropping any human-authored prose sections below the closing fence. The writer now preserves trailing prose across all write operations. (#2893) diff --git a/src/broken-windows.cts b/src/broken-windows.cts index 8c70b238a..032c4e6b7 100644 --- a/src/broken-windows.cts +++ b/src/broken-windows.cts @@ -721,7 +721,33 @@ function writeLedgerAtomic(cwd: string, ledger: Ledger): void { ensurePlanningDir(cwd); const p = ledgerPath(cwd); const tmp = `${p}.${process.pid}.tmp`; - fs.writeFileSync(tmp, renderLedger(ledger), 'utf8'); + + // #2893: preserve any prose below the JSON ledger block. renderLedger + // reconstructs frontmatter + header + table + JSON — it does not include + // trailing prose that users may have written below the closing fence. + // Without this, every append/waive/fixed silently destroys that prose. + 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(/^\n+/, ''); + } + } + } catch { + // File doesn't exist yet (first write) — no prose to preserve. + } + + const rendered = renderLedger(ledger); + const content = trailingProse ? `${rendered}${trailingProse}` : rendered; + fs.writeFileSync(tmp, content, 'utf8'); try { renameWithRetry(tmp, p); } catch (err) { diff --git a/tests/broken-windows.test.cjs b/tests/broken-windows.test.cjs index 00edad0b4..6e07b794f 100644 --- a/tests/broken-windows.test.cjs +++ b/tests/broken-windows.test.cjs @@ -603,6 +603,66 @@ describe('broken-windows CLI: windows append', () => { assert.equal(res.success, false); assert.match(res.error, /4-backtick|fence|invalid_text/i); }); + + // ─── #2893: append must not destroy prose below the JSON ledger ────────── + + test('#2893 — append preserves prose below the JSON ledger block', (t) => { + const tmp = createTempDir('bw-append-prose-'); + t.after(() => cleanup(tmp)); + + // Create a WINDOWS.md with a NON-EMPTY ledger + prose below the JSON block. + fs.mkdirSync(path.join(tmp, '.planning'), { recursive: true }); + const lp = path.join(tmp, '.planning', LEDGER_FILE_NAME); + const initial = renderLedger({ + schema_version: 1, open_count: 1, waived_count: 0, fixed_count: 0, total_count: 1, + last_updated: '2026-01-01T00:00:00Z', + entries: [{ id: 1, phase: '1', kind: 'stub', file: '', line: null, description: 'pre-existing', status: 'open', reason: '', recorded_at: '2026-01-01T00:00:00Z', resolved_at: null }], + }); + const prose = [ + '', + '## Investigation Notes', + '', + 'This window was opened because the flaky test in thread-status.test.ts', + 'turned out to be a real race condition against live data, not a pre-existing break.', + '', + '## ACPT-M03', + '', + 'Went red on a green that PREDATED the diff — checkpoint refused, then fixed.', + ].join('\n'); + fs.writeFileSync(lp, initial + prose, 'utf8'); + + // First append. + const res = runGsdTools( + ['windows', 'append', '--kind', 'stub', '--phase', '2', '--description', 'test entry'], + tmp, + ); + assert.equal(res.success, true, `stderr: ${res.error || ''}`); + assert.equal(JSON.parse(res.output).ok, true); + + // Second append — idempotency: prose must appear exactly once, not duplicated. + const res2 = runGsdTools( + ['windows', 'append', '--kind', 'todo', '--phase', '3', '--description', 'second entry'], + tmp, + ); + assert.equal(res2.success, true); + + const after = fs.readFileSync(lp, 'utf8'); + // Prose must survive. + assert.match(after, /Investigation Notes/, 'prose heading must survive'); + assert.match(after, /thread-status\.test\.ts/, 'prose body must survive'); + assert.match(after, /ACPT-M03/, 'second prose heading must survive'); + assert.match(after, /PREDATED the diff/, 'second prose body must survive'); + // Prose must appear exactly once (not duplicated by the second write). + assert.equal((after.match(/Investigation Notes/g) || []).length, 1, + 'prose heading must appear exactly once after two appends (idempotency)'); + // The old JSON body must NOT be duplicated as prose (the indexOf(open-fence) bug). + // Count JSON fence opens — there must be exactly one. + assert.equal((after.match(/````json/g) || []).length, 1, + 'exactly one JSON fence open must exist (no duplicated JSON body)'); + // The file must re-parse cleanly with the correct entry count. + const reParsed = parseLedger(after); + assert.equal(reParsed.entries.length, 3, 'ledger must have 3 entries after two appends'); + }); }); // ---------------------------------------------------------------------------