* 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 <sim@local>
This commit is contained in:
5
.changeset/clever-lemurs-snooze.md
Normal file
5
.changeset/clever-lemurs-snooze.md
Normal file
@@ -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)
|
||||
@@ -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) {
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user