From 6ebe6372ce2e24005ba2d708803c9ced7863da4d Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 7 Sep 2026 04:27:35 -0400 Subject: [PATCH] fix(#4243): anchor stateReplaceProgressPercent bold form to line start (#4474) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(#4243): anchor stateReplaceProgressPercent bold form to line start The bold branch of stateReplaceProgressPercent carried no ^ and no /m flag, so a bold percent-ish label quoted MID-SENTENCE inside prose — an Accumulated Context bullet mentioning **Progress:** — captured the machine-segment rewrite and destroyed the rest of its line, silently, while the real Progress line stayed stale (and the frontmatter moved on without it, breaking the #4213 surfaces-agree contract). Every caller (cmdStateUpdateProgress, syncCore's percent arm, applyPostSyncPreservation) feeds the whole document, so all three were exposed. Anchored to ^([ \t]*\*\*Progress:\*\*[ \t]*)([^\r\n]*)$ with /im — the exact idiom #4453 applied to stateReplaceField's bold branch (same-line confinement per #4010: the leading class is [ \t]*, deliberately not \s*, which can consume the newlines before the label into the match; $ is explicit-and-inert and documents end-of-line). #2177's recorded requirements all stand: frontmatter is stripped before matching, the suffix-preserving machine-segment swap is untouched, and bold-beats-plain priority now governs line-start forms, so an earlier free-text plain Progress: line still cannot capture the rewrite ahead of the real bold status line. Per the maintainer ruling (2026-09-07), #2177's incidental bold-anywhere matching was not load-bearing. * test(#4243): scope the C4 region check with splitLines, not a bare \n split lint:ci (local/no-crlf-fragile-split) flagged the free-text-plain-line row's content.split(/\n## /)[0] — a bare \n split on readFileSync content is CRLF-fragile under Windows autocrlf. Same scoping via splitLines() (src/text-lines.cts), which splits on \r?\n. * chore(#4243): backfill PR number in changeset --------- Co-authored-by: sim --- .changeset/serene-foxes-hum.md | 5 + src/state-transition.cts | 28 ++++- tests/state-transition.test.cjs | 209 ++++++++++++++++++++++++++++++++ tests/state.test.cjs | 198 ++++++++++++++++++++++++++++++ 4 files changed, 435 insertions(+), 5 deletions(-) create mode 100644 .changeset/serene-foxes-hum.md diff --git a/.changeset/serene-foxes-hum.md b/.changeset/serene-foxes-hum.md new file mode 100644 index 000000000..38f5ec9ff --- /dev/null +++ b/.changeset/serene-foxes-hum.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4474 +--- +**progress-percent bold fields no longer rewrite mid-sentence lookalikes** — anchored to line-start like #4243's stateReplaceField fix. (#4243 follow-up; supersedes the #2177 bold-anywhere reading per maintainer ruling) diff --git a/src/state-transition.cts b/src/state-transition.cts index e001c4d98..699190d61 100644 --- a/src/state-transition.cts +++ b/src/state-transition.cts @@ -55,11 +55,29 @@ export function formatProgressMachineSegment(percent: number): string { // `syncCore`'s call here. export function stateReplaceProgressPercent(content: string, percent: number): string | null { const body = stripFrontmatter(content); - // #2177: bold `**Progress:**` anywhere in the body wins outright; the plain - // `^Progress:` form is the fallback only when no bold line exists, so an - // earlier free-text line starting with `Progress:` cannot capture the - // rewrite ahead of the real status line. - const boldProgressPattern = /(\*\*Progress:\*\*[ \t]*)([^\r\n]*)/i; + // #2177: bold `**Progress:**` takes priority over the plain `^Progress:` + // form, so an earlier free-text line starting with `Progress:` cannot + // capture the rewrite ahead of the real status line. + // + // #4243 (follow-up to #4453, maintainer ruling 2026-09-07): the bold form + // is also ANCHORED to line start, with same-line leading whitespace only — + // the exact idiom #4453 applied to stateReplaceField's bold branch. The + // pre-fix pattern carried no `^` and no `m` flag, so a bold percent-ish + // label quoted MID-SENTENCE inside prose (an Accumulated Context bullet + // mentioning `**Progress:**`) captured the machine-segment rewrite and + // destroyed the rest of its line, silently, while the real Progress line + // stayed stale — every caller (cmdStateUpdateProgress, syncCore's percent + // arm, applyPostSyncPreservation) feeds the whole document. #2177's own + // recorded requirements are unaffected: the frontmatter is stripped before + // matching (its defect was the YAML `progress:` key shadowing the body + // line), the suffix-preserving machine-segment swap is untouched, and the + // bold-beats-plain priority now governs LINE-START forms. The leading class + // is `[ \t]*`, deliberately NOT `\s*` — `^\s*\*\*` can consume the newlines + // before the label into the match and drop them on rebuild (#4010's + // same-line confinement hazard). `$` is explicit-and-inert (`[^\r\n]*` + // never crosses line terminators) and documents that the match ends at + // end-of-line. + const boldProgressPattern = /^([ \t]*\*\*Progress:\*\*[ \t]*)([^\r\n]*)$/im; const plainProgressPattern = /^(Progress:[ \t]*)([^\r\n]*)/im; const pattern = boldProgressPattern.test(body) ? boldProgressPattern diff --git a/tests/state-transition.test.cjs b/tests/state-transition.test.cjs index 0ad4996ba..4221464ca 100644 --- a/tests/state-transition.test.cjs +++ b/tests/state-transition.test.cjs @@ -22,6 +22,7 @@ const { getPreserveWhenUnchangedFields, STATE_MD_SECTIONS, sliceCurrentPositionSection, + stateReplaceProgressPercent, } = require('../gsd-core/bin/lib/state-transition.cjs'); const { stateExtractField } = require('../gsd-core/bin/lib/state-document.cjs'); const { STATE_FIELD_SCHEMA } = require('../gsd-core/bin/lib/state-md-schema.cjs'); @@ -4572,3 +4573,211 @@ describe('#4129: resyncing measured write ratchets the progress block', () => { assert.deepStrictEqual(r.postFm.progress, { total_phases: 18, completed_phases: 2, percent: 11 }); }); }); + +// ───────────────────────────────────────────────────────────────────────────── +// #4243 follow-up: stateReplaceProgressPercent's bold branch is anchored to +// line start, exactly like stateReplaceField's #4453 fix. The pre-fix bold +// pattern carried no ^ and no /m, so a bold percent-ish label quoted +// MID-SENTENCE inside prose — an Accumulated Context bullet mentioning +// `**Progress:**` — captured the machine-segment rewrite and destroyed the +// rest of its line, silently, while the real Progress line stayed stale (the +// callers — cmdStateUpdateProgress, syncCore's percent arm, +// applyPostSyncPreservation — all feed the whole document). #2177's recorded +// protections (frontmatter stripped, suffix preserved, plain-form fallback, +// bold-beats-plain priority among LINE-START forms) are unchanged; per the +// maintainer ruling (2026-09-07), #2177's incidental bold-anywhere matching +// was not load-bearing. +// ───────────────────────────────────────────────────────────────────────────── +describe('stateReplaceProgressPercent — anchored bold form leaves prose lookalikes untouched (#4243 follow-up)', () => { + // The corruption shape: a bold label quoted for documentation purposes + // inside a bullet, with the real status line in the plain template form. + // The value after the label has NO percent, so the pre-fix whole-value + // replacement destroyed the rest of the sentence. + const PROSE_LINE = + '- [2026-07-15] Progress dashboard: the **Progress:** field is machine-managed by state update-progress; do not hand-edit.'; + + const SEGMENT = (bars) => `[${'█'.repeat(bars)}${'░'.repeat(10 - bars)}]`; + + // ROW 1 — the failing-first regression. The lookalike must survive + // byte-identically and the REAL plain line must take the update. + test('issue repro: mid-sentence **Progress:** lookalike survives, real plain line updates', () => { + const input = [ + '## Current Position', + '', + 'Phase: 1 of 1', + 'Plan: 2 of 2', + 'Status: Executing Phase 1', + '', + 'Progress: [█████░░░░░] 50% (1/2 plans done)', + '', + '## Accumulated Context', + '', + '### Decisions', + '', + PROSE_LINE, + '', + ].join('\n'); + const result = stateReplaceProgressPercent(input, 0); + assert.notEqual(result, null, 'the real plain Progress line must still match'); + assert.ok( + result.includes(PROSE_LINE), + `prose lookalike must survive byte-identically, got:\n${result}`, + ); + assert.ok( + result.includes(`Progress: ${SEGMENT(0)} 0% (1/2 plans done)`), + `the real plain line's machine segment must update with the suffix intact, got:\n${result}`, + ); + assert.ok( + !result.includes('the **Progress:** ['), + 'the rewrite must not bleed a machine segment into the prose occurrence', + ); + }); + + test('lookalike ordered BEFORE the real bold line: prose survives, line-start bold line updates', () => { + const input = [ + '## Accumulated Context', + '', + PROSE_LINE, + '', + '## Current Position', + '', + '**Progress:** [█████░░░░░] 50% (2/4 plans done; blocked on API keys)', + '', + ].join('\n'); + const result = stateReplaceProgressPercent(input, 75); + assert.notEqual(result, null); + assert.ok(result.includes(PROSE_LINE), `prose lookalike must survive, got:\n${result}`); + assert.ok( + result.includes(`**Progress:** ${SEGMENT(8)} 75% (2/4 plans done; blocked on API keys)`), + `the real line-start bold line's machine segment must update with the suffix intact, got:\n${result}`, + ); + }); + + test('lookalike whose value carries a percent: prose percent is not swapped, real plain line updates', () => { + const lookalike = '- The **Progress:** bar read 20% last week; see the archived thread.'; + const input = [ + 'Progress: [█████░░░░░] 50% (1/2 plans done)', + '', + '## Accumulated Context', + '', + lookalike, + '', + ].join('\n'); + const result = stateReplaceProgressPercent(input, 0); + assert.notEqual(result, null); + assert.ok( + result.includes(lookalike), + `the prose percent must not be swapped into a machine segment, got:\n${result}`, + ); + assert.ok( + result.includes(`Progress: ${SEGMENT(0)} 0% (1/2 plans done)`), + 'the real plain line must take the update', + ); + }); + + test('mid-sentence lookalike with no real line: returns null (honest absence), never a rewrite', () => { + const input = `Some prose sentence quoting a **Progress:** label mid-sentence, plus trailing words.`; + assert.equal(stateReplaceProgressPercent(input, 40), null); + }); + + test('mid-word lookalike with no real line: returns null', () => { + const input = 'Prose mentions text**Progress:**tail mid-word and nothing else.'; + assert.equal(stateReplaceProgressPercent(input, 40), null); + }); + + // Negative space: an INDENTED line-start bold line is still the status line + // (the leading class is same-line whitespace only, #4010's idiom), and the + // indent is preserved. + test('indented line-start bold line still updates, indent preserved', () => { + const input = ' **Progress:** [█████░░░░░] 50%'; + const result = stateReplaceProgressPercent(input, 100); + assert.equal(result, ` **Progress:** ${SEGMENT(10)} 100%`); + }); + + // Negative space + fix-shape pin: leading blank lines before the label are + // NOT swallowed. The anchor's leading class is same-line whitespace only + // (`[ \t]*`); the naive `^\s*` variant would consume the newlines into the + // match and drop them on rebuild (#4010 hazard, rejected in #4453). + test('leading blank lines before a line-start bold label survive byte-identically', () => { + const input = '\n\n**Progress:** [█████░░░░░] 50%'; + const result = stateReplaceProgressPercent(input, 40); + assert.equal(result, `\n\n**Progress:** ${SEGMENT(4)} 40%`); + }); + + // Negative space: #2177's priority is unchanged among LINE-START forms — a + // real bold status line still beats the plain form, so an earlier free-text + // plain `Progress:` line cannot capture the rewrite ahead of it. + test('line-start bold still beats the plain form; earlier free-text plain line untouched (#2177)', () => { + const freeText = 'Progress: tracked in the weekly thread, do not edit this line by hand'; + const input = `${freeText}\n\n**Progress:** [█████░░░░░] 50%\n`; + const result = stateReplaceProgressPercent(input, 75); + assert.notEqual(result, null); + assert.ok(result.includes(freeText), 'the free-text plain line must stay byte-identical'); + assert.ok( + result.includes(`**Progress:** ${SEGMENT(8)} 75%`), + 'the line-start bold status line is the one rewritten', + ); + }); + + // Negative space: first-occurrence-wins among line-start bold lines. + test('two line-start bold occurrences: only the first is replaced', () => { + const input = '**Progress:** [█████░░░░░] 50%\n**Progress:** [████░░░░░░] 40%'; + const result = stateReplaceProgressPercent(input, 10); + assert.equal(result, `**Progress:** ${SEGMENT(1)} 10%\n**Progress:** [████░░░░░░] 40%`); + }); + + // Negative space: the plain-form fallback (#2177's plain path) is unchanged + // when no bold line exists at all — machine-segment-only swap, suffix kept. + test('plain-form fallback unchanged: no bold anywhere, plain line updates with suffix intact', () => { + const input = 'Progress: [██░░░░░░░░] 20% (1/2 plans done; next: verification)'; + const result = stateReplaceProgressPercent(input, 50); + assert.notEqual(result, null); + assert.equal(result, `Progress: ${SEGMENT(5)} 50% (1/2 plans done; next: verification)`); + }); + + // Negative space: #2177's core — the YAML frontmatter `progress:` key is + // never a match target; the block survives byte-identically. + test('frontmatter progress: key never matched — block survives byte-identically (#2177)', () => { + const fm = [ + '---', + 'progress:', + ' total_plans: 2', + ' completed_plans: 1', + ' percent: 50', + '---', + '', + ].join('\n'); + const input = `${fm}# Project State\n\nProgress: [█████░░░░░] 50% (1/2 plans done)\n\n## Accumulated Context\n\n${PROSE_LINE}\n`; + const result = stateReplaceProgressPercent(input, 0); + assert.notEqual(result, null); + assert.ok( + result.startsWith(fm), + `the frontmatter block must survive byte-identically, got:\n${result}`, + ); + assert.ok(result.includes(' percent: 50'), 'the frontmatter percent line is untouched'); + assert.ok(result.includes(PROSE_LINE), 'the prose lookalike survives'); + assert.ok( + result.includes(`Progress: ${SEGMENT(0)} 0% (1/2 plans done)`), + 'the real plain line takes the update', + ); + }); + + test('CRLF document: lookalike survives with CRLF intact, real plain line updates', () => { + const input = [ + 'Progress: [█████░░░░░] 50% (1/2 plans done)', + '', + '## Accumulated Context', + '', + PROSE_LINE, + '', + ].join('\r\n'); + const result = stateReplaceProgressPercent(input, 0); + assert.notEqual(result, null); + assert.ok(result.includes(PROSE_LINE), `prose lookalike must survive, got:\n${result}`); + assert.ok(result.includes('\r\n'), 'CRLF endings must be preserved'); + assert.ok( + result.includes(`Progress: ${SEGMENT(0)} 0% (1/2 plans done)`), + 'the real plain line must take the update', + ); + }); +}); diff --git a/tests/state.test.cjs b/tests/state.test.cjs index e924f597b..75aa4c9d4 100644 --- a/tests/state.test.cjs +++ b/tests/state.test.cjs @@ -4141,6 +4141,204 @@ describe('#4243: begin-phase leaves prose lookalikes untouched, preserves unknow }); }); +// ───────────────────────────────────────────────────────────────────────────── +// #4243 follow-up — update-progress: the bold branch of +// stateReplaceProgressPercent is anchored to line start (same fix as #4453's +// stateReplaceField anchoring), so prose bold-percent lookalikes stay +// untouched and the real Progress line takes the machine-segment rewrite. +// ───────────────────────────────────────────────────────────────────────────── + +describe('#4243 follow-up: update-progress leaves prose **Progress:** lookalikes untouched', () => { + const PROSE_LINE = + '- [2026-07-15] Progress dashboard: the **Progress:** field is machine-managed by state update-progress; do not hand-edit.'; + + let tmpDir; + + beforeEach(() => { + tmpDir = createFixture(); + // Same #3217 free-form ROADMAP as the update-progress block above: a + // no-version ROADMAP is COMPLETE scope, so the percent is computed rather + // than withheld. + fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), '# Roadmap\n'); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + function seedOnePhaseTwoPlans() { + // 1 of 2 plans summarized, no *-VERIFICATION.md → min-capped 0% (the + // existing update-progress rows' derivation; exact value is incidental — + // what matters is WHERE the rewrite lands). + const phaseDir = path.join(tmpDir, '.planning', 'phases', '01'); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync(path.join(phaseDir, '01-01-PLAN.md'), '# Plan\n'); + fs.writeFileSync(path.join(phaseDir, '01-01-SUMMARY.md'), '# Summary\n'); + fs.writeFileSync(path.join(phaseDir, '01-02-PLAN.md'), '# Plan\n'); + } + + // The repro verbatim: the real status line in the plain template form, the + // lookalike quoted mid-sentence inside an Accumulated Context bullet whose + // value has no percent (the pre-fix whole-value replacement shape). + test('issue repro: prose lookalike is byte-identical, real plain line updates, surfaces agree', () => { + writeState(tmpDir, [ + '# Project State', + '', + '## Current Position', + 'Phase: 1 of 1', + 'Plan: 2 of 2', + 'Status: Executing Phase 1', + 'Last activity: 2026-08-01 — did a thing', + '', + 'Progress: [█████░░░░░] 50% (1/2 plans done)', + '', + '## Accumulated Context', + '', + '### Decisions', + '', + PROSE_LINE, + '', + ].join('\n')); + seedOnePhaseTwoPlans(); + + const result = runGsdTools('state update-progress', tmpDir); + assert.ok(result.success, `update-progress failed: ${result.error}`); + const out = JSON.parse(result.output); + assert.strictEqual(out.updated, true, 'the real plain Progress line must still match'); + + const content = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8'); + assert.ok( + content.includes(PROSE_LINE), + `prose lookalike must survive update-progress byte-identically, got:\n${content}`, + ); + // The whole-document Progress extractor (stateExtractField) is itself + // bold-anywhere on the read side — with a bold lookalike in prose it + // extracts the PROSE value, so it cannot witness this write. Assert on + // the Current Position section body instead (#4453's precedent for + // prose-lookalike fixtures), plus the frontmatter surface for agreement. + const pos = sectionMatchOf(content, 'Current Position'); + assert.ok(pos, 'Current Position section should exist'); + assert.match( + pos[1], + new RegExp(`^Progress: \\[${'░'.repeat(10)}\\] ${out.percent}% \\(1/2 plans done\\)$`, 'm'), + 'the real plain line takes the machine-segment rewrite with its suffix intact', + ); + const fm = frontmatterLib.extractFrontmatter(content); + assert.strictEqual( + Number(fm.progress && fm.progress.percent), + out.percent, + 'frontmatter percent must agree with the reported percent (#4213 surfaces-agree)', + ); + }); + + // Corruption shape 2: lookalike section ordered BEFORE the status line, + // real field in the line-start bold form. + test('lookalike before Current Position: prose survives, real line-start bold line updates', () => { + writeState(tmpDir, [ + '# Project State', + '', + '## Accumulated Context', + '', + PROSE_LINE, + '', + '## Current Position', + '', + '**Progress:** [█████░░░░░] 50% (2/4 plans done; blocked on API keys)', + '', + ].join('\n')); + seedOnePhaseTwoPlans(); + + const result = runGsdTools('state update-progress', tmpDir); + assert.ok(result.success, `update-progress failed: ${result.error}`); + const out = JSON.parse(result.output); + assert.strictEqual(out.updated, true); + + const content = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8'); + assert.ok( + content.includes(PROSE_LINE), + `prose lookalike must survive update-progress byte-identically, got:\n${content}`, + ); + // Read-side extractor is bold-anywhere (see C1 note): assert on the + // Current Position section body instead. + const pos = sectionMatchOf(content, 'Current Position'); + assert.ok(pos, 'Current Position section should exist'); + assert.match( + pos[1], + new RegExp(`^\\*\\*Progress:\\*\\* \\[${'░'.repeat(10)}\\] ${out.percent}% \\(2/4 plans done; blocked on API keys\\)$`, 'm'), + 'the real line-start bold line takes the machine-segment rewrite with its suffix intact', + ); + }); + + // Honest absence: with only a prose lookalike (no line-start Progress line + // at all), the command must report updated:false with the #3957 body-layer + // reason — not a false success that corrupts the prose. + test('lookalike only, no body Progress line: updated:false, file unchanged', () => { + const before = [ + '# Project State', + '', + '## Accumulated Context', + '', + PROSE_LINE, + '', + ].join('\n'); + writeState(tmpDir, before); + seedOnePhaseTwoPlans(); + + const result = runGsdTools('state update-progress', tmpDir); + assert.ok(result.success, `update-progress failed: ${result.error}`); + const out = JSON.parse(result.output); + assert.strictEqual(out.updated, false, 'a prose lookalike is not a Progress line'); + assert.strictEqual( + out.reason, + 'no Progress: line found in STATE.md body to update (frontmatter progress data is unaffected)', + ); + const content = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8'); + assert.ok(content.includes(PROSE_LINE), 'the prose must be untouched'); + }); + + // #2177 priority restated at CLI level among LINE-START forms: an earlier + // free-text plain `Progress:` line must not capture the rewrite ahead of + // the real bold status line. + test('free-text plain Progress: line above the real bold line stays byte-identical (#2177)', () => { + const freeText = 'Progress: tracked in the weekly thread, do not edit this line by hand'; + writeState(tmpDir, [ + '# Project State', + '', + freeText, + '', + '**Progress:** [█████░░░░░] 50%', + '', + '## Accumulated Context', + '', + PROSE_LINE, + '', + ].join('\n')); + seedOnePhaseTwoPlans(); + + const result = runGsdTools('state update-progress', tmpDir); + assert.ok(result.success, `update-progress failed: ${result.error}`); + const out = JSON.parse(result.output); + assert.strictEqual(out.updated, true); + + const content = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8'); + assert.ok(content.includes(freeText), 'the free-text plain line must stay byte-identical'); + assert.ok(content.includes(PROSE_LINE), 'the prose lookalike must stay byte-identical'); + // Read-side extractor is bold-anywhere (see the C1 note above); the free- + // text plain line and the bold status line live in the top-of-body region + // between the title and the first ## heading. Scope with splitLines + // (CRLF-safe per local/no-crlf-fragile-split), never a bare \n split. + const { splitLines } = require('../gsd-core/bin/lib/text-lines.cjs'); + const lines = splitLines(content); + const firstSectionIdx = lines.findIndex((l) => l.startsWith('## ')); + const beforeFirstSection = lines.slice(0, firstSectionIdx === -1 ? lines.length : firstSectionIdx).join('\n'); + assert.match( + beforeFirstSection, + new RegExp(`^\\*\\*Progress:\\*\\* \\[${'░'.repeat(10)}\\] ${out.percent}%$`, 'm'), + 'the line-start bold status line is the one rewritten', + ); + }); +}); + // ───────────────────────────────────────────────────────────────────────────── // Bug #1589 — progress counters not updated during plan execution // ─────────────────────────────────────────────────────────────────────────────