From bbecc6a08b05d912d5a0f0221984e04a5c494710 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Thu, 27 Aug 2026 14:03:48 -0400 Subject: [PATCH] fix(#3740): ack writer searches the exact status-field shape the reader parses (#3940) * test(#3740): acknowledging a nested-marker status line must clear the entry * fix(#3740): narrow the ack status-field search to the marker-free shape the reader parses * chore(#3740): changeset fragment (pr number backfilled after PR creation) * chore(#3740): backfill changeset PR number (3940) --------- Co-authored-by: sim --- .changeset/sunny-herons-hum.md | 5 +++ src/uat.cts | 15 ++++++-- tests/uat.test.cjs | 62 ++++++++++++++++++++++++++++++++++ 3 files changed, 80 insertions(+), 2 deletions(-) create mode 100644 .changeset/sunny-herons-hum.md diff --git a/.changeset/sunny-herons-hum.md b/.changeset/sunny-herons-hum.md new file mode 100644 index 000000000..f38fe2d9b --- /dev/null +++ b/.changeset/sunny-herons-hum.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3940 +--- +**`audit-open acknowledge` no longer reports success on an entry it did not clear** — acknowledging a deferred item whose status is written as a nested list line now records a status the reader actually parses, so acknowledged entries drop out of audit counts instead of resurfacing forever. (#3740) diff --git a/src/uat.cts b/src/uat.cts index 89c468136..51112084a 100644 --- a/src/uat.cts +++ b/src/uat.cts @@ -2038,8 +2038,19 @@ function acknowledgeDeferredItem(content: string, targetText: string): Acknowled } const matchIndexInContent = sectionOffset + start; - const statusFieldRe = /^\s*(?:-\s+)?(\*+status:\*+|status:)/i; - const statusLineIdx = matchedLines.findIndex((rawLine) => statusFieldRe.test(rawLine.replace(/\r$/, ''))); + // #3740: the search must mirror the reader exactly. extractGapEntryFields + // strips a bullet marker on line 0 ALONE — a later `- ` line is a nested + // sub-list, never a field line — so a marker-prefixed match on any + // continuation line would rewrite a line no reader reads and report `ok` + // while the entry stays outstanding. Line 0 KEEPS the marker-optional + // form: the reader de-bullets it, so `- status: open` as the entry line is + // a real field there (and first-wins means the insert branch could not + // outrank it). Everything else falls through to the insert branch below, + // which the marker-free and no-status controls already round-trip. + const statusFieldRe = /^\s*(\*+status:\*+|status:)/i; + const statusFieldReLine0 = /^\s*(?:-\s+)?(\*+status:\*+|status:)/i; + const statusLineIdx = matchedLines.findIndex((rawLine, idx) => + (idx === 0 ? statusFieldReLine0 : statusFieldRe).test(rawLine.replace(/\r$/, ''))); // No CRLF-preservation branch here (WARNING 1, #3458 follow-up review): // every write goes through `platformWriteSync` → `normalizeContent`, which diff --git a/tests/uat.test.cjs b/tests/uat.test.cjs index 0523b1ef1..3a1e5a6dd 100644 --- a/tests/uat.test.cjs +++ b/tests/uat.test.cjs @@ -16,6 +16,8 @@ const { CHECKPOINT_LANGUAGE_ALIASES, resolveCheckpointFrame, parseDeferredItems, + parseDeferredItemsWithStatus, + acknowledgeDeferredItem, parseUatItems, parseUatItemsWithStats, } = require('../gsd-core/bin/lib/uat.cjs'); @@ -6572,3 +6574,63 @@ describe('parseUatItemsWithStats — result: line-scan boundary defects (#3078-C assert.strictEqual(withSeparatorResult.headingsSeen, plainLfResult.headingsSeen, describeAll()); }); }); + +// ──────────────────────────────────────────────────────────────────────── +// #3740: acknowledgeDeferredItem must agree with the field extractor on +// what a status-field line is. The writer's search regex was marker-optional +// while the reader (extractGapEntryFields) deliberately strips a bullet +// marker on line 0 ONLY — a later `- ` line is a nested sub-list, not a +// field. Pre-fix, a nested ` - status: open` line was rewritten and `ok` +// returned, but no reader ever saw the rewrite: the entry stayed outstanding +// after a "successful" acknowledgement. +// ──────────────────────────────────────────────────────────────────────── +describe('#3740: acknowledge round-trips through the reader (parse → acknowledge → parse)', () => { + function roundTrip(body) { + const content = '## Deferred Items\n\n' + body + '\n'; + const before = parseDeferredItemsWithStatus(content); + assert.equal(before.length, 1, `fixture must parse to exactly one entry, got ${before.length}`); + const ack = acknowledgeDeferredItem(content, before[0].name); + const after = parseDeferredItemsWithStatus(ack.content); + assert.equal(after.length, 1, 'acknowledged file must still parse to one entry'); + return { ack, before: before[0], after: after[0], content: ack.content }; + } + + test('nested-marker status line: ack returns ok and the entry is no longer outstanding', () => { + const r = roundTrip('- alpha\n - status: open'); + assert.equal(r.ack.status, 'ok'); + assert.equal(r.after.status, 'acknowledged', + '#3740: a nested `- status:` line is not a field line to the reader; the ack must take the insert branch the reader parses'); + }); + + test('nested-marker status line, CRLF variant: same outcome', () => { + const r = roundTrip('- alpha\r\n - status: open'); + assert.equal(r.ack.status, 'ok'); + assert.equal(r.after.status, 'acknowledged'); + }); + + test('line-0 entry-line status (`- status: open`) is still rewritten in place', () => { + // The reader de-bullets line 0, so a status field on the entry line IS a + // real field there — and first-wins means the insert branch could never + // outrank it. The search must keep matching this shape (#3740 review). + const r = roundTrip('- status: open\n reason: flaky'); + assert.equal(r.ack.status, 'ok'); + assert.equal(r.after.status, 'acknowledged'); + const statusLines = r.content.split('\n').filter((l) => /status:\s*/.test(l)); + assert.equal(statusLines.length, 1, `exactly one status line must remain, got ${JSON.stringify(statusLines)}`); + }); + + test('control: marker-free status line is still rewritten in place, not duplicated', () => { + const r = roundTrip('- alpha\n status: open'); + assert.equal(r.ack.status, 'ok'); + assert.equal(r.after.status, 'acknowledged'); + const statusLines = r.content.split('\n').filter((l) => /status:\s*/.test(l)); + assert.equal(statusLines.length, 1, `exactly one status line must remain, got ${JSON.stringify(statusLines)}`); + assert.match(statusLines[0], /status:\s*acknowledged/); + }); + + test('control: entry with no status line keeps the insert branch', () => { + const r = roundTrip('- alpha'); + assert.equal(r.ack.status, 'ok'); + assert.equal(r.after.status, 'acknowledged'); + }); +});