diff --git a/.changeset/loud-jade-canyon.md b/.changeset/loud-jade-canyon.md new file mode 100644 index 000000000..0919d9c39 --- /dev/null +++ b/.changeset/loud-jade-canyon.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3387 +--- +**Full-line `#` comments in `.planning/STATE.md` (and every frontmatter surface) now survive a mutating write** — `parseYamlRegion` carries column-0 comments through to `reconstructFrontmatter` via a Symbol-keyed channel, and `syncStateFrontmatter` propagates that channel across its fresh-rebuild of the frontmatter object, so a comment like `# NOTE: current_phase is hand-maintained` is no longer silently destroyed on the next `state` verb. Comment-less frontmatter is unchanged; data identity (keys/values/arrays/nested) is preserved alongside the comments. (#3257) diff --git a/src/frontmatter.cts b/src/frontmatter.cts index 985ae629c..ee77e4e39 100644 --- a/src/frontmatter.cts +++ b/src/frontmatter.cts @@ -93,6 +93,20 @@ function isFrontmatterShaped(region: string): boolean { )); } +/** + * #3257: a Symbol-keyed channel that carries full-line (column-0 `#`) YAML + * comments through a parse → reconstruct round-trip. Comments are otherwise + * unrepresentable on the Frontmatter object (Record) and were + * silently dropped by reconstructFrontmatter. The Symbol is invisible to + * Object.entries / Object.keys / JSON.stringify / for-in, so every existing + * reader is unchanged; only reconstructFrontmatter reads it. Leading comments + * are attached to the top-level key that follows them; comments after the last + * key go to `trailing`. Only set when a comment is actually seen, so comment-less + * frontmatter parses byte-identically to before. + */ +const FULL_LINE_COMMENTS = Symbol('fullLineComments'); +type FullLineCommentChannel = { leading: Record; trailing: string[] }; + /** * Parse one already-delimited YAML region into a Frontmatter object. * @@ -104,6 +118,10 @@ function parseYamlRegion(yaml: string): Frontmatter { const frontmatter: Frontmatter = {}; const lines = yaml.split(/\r?\n/); + // #3257: pending column-0 full-line comments, attached to the next top-level key. + let pendingComments: string[] = []; + let commentChannel: FullLineCommentChannel | undefined; + // Stack to track nested objects: [{obj, key, indent}] type StackEntry = { obj: Record | unknown[]; key: string | null; indent: number }; const stack: StackEntry[] = [{ obj: frontmatter, key: null, indent: -1 }]; @@ -112,6 +130,12 @@ function parseYamlRegion(yaml: string): Frontmatter { // Skip empty lines if (line.trim() === '') continue; + // #3257: capture column-0 full-line comments; attach them to the next top-level key. + if (/^#/.test(line)) { + pendingComments.push(line); + continue; + } + // Calculate indentation (number of leading spaces) const indentMatch = line.match(/^(\s*)/); const indent = indentMatch ? indentMatch[1].length : 0; @@ -127,6 +151,12 @@ function parseYamlRegion(yaml: string): Frontmatter { const keyMatch = line.match(/^(\s*)([a-zA-Z0-9_-]+):\s*(.*)/); if (keyMatch) { const key = keyMatch[2]; + // #3257: attach any pending comments to this (top-level) key. + if (pendingComments.length) { + if (!commentChannel) commentChannel = { leading: {}, trailing: [] }; + commentChannel.leading[key] = pendingComments; + pendingComments = []; + } const value = keyMatch[3].trim(); if (value === '' || value === '[') { @@ -168,6 +198,15 @@ function parseYamlRegion(yaml: string): Frontmatter { } } + // #3257: trailing comments (after the last key) + attach the channel if any comment was seen. + if (pendingComments.length) { + if (!commentChannel) commentChannel = { leading: {}, trailing: [] }; + commentChannel.trailing = pendingComments; + } + if (commentChannel) { + (frontmatter as Record)[FULL_LINE_COMMENTS as unknown as symbol] = commentChannel; + } + return frontmatter; } @@ -282,8 +321,14 @@ function scalarNeedsDoubleQuoting(s: string): boolean { function reconstructFrontmatter(obj: Frontmatter): string { const lines: string[] = []; + // #3257: read the full-line-comment channel (set by parseYamlRegion when comments + // were present). Object.entries skips the Symbol key, so the data loop is unchanged. + const commentChannel = (obj as Record)[FULL_LINE_COMMENTS as unknown as symbol] as FullLineCommentChannel | undefined; for (const [key, value] of Object.entries(obj)) { if (value === null || value === undefined) continue; + // #3257: re-emit this key's leading full-line comments before the key itself. + const leading = commentChannel?.leading[key]; + if (leading) for (const c of leading) lines.push(c); if (Array.isArray(value)) { if (value.length === 0) { lines.push(`${key}: []`); @@ -343,9 +388,34 @@ function reconstructFrontmatter(obj: Frontmatter): string { } } } + // #3257: re-emit any trailing full-line comments (those after the last key). + if (commentChannel?.trailing?.length) { + for (const c of commentChannel.trailing) lines.push(c); + } return lines.join('\n'); } +/** + * #3257: copy the full-line-comment channel from `source` onto `target`, filtering + * `leading` to keys still present in `target` (a deleted key's annotation goes with + * it — AC5). No-op when `source` carries no channel. Consumers that rebuild their + * target object fresh (syncStateFrontmatter builds derivedFm via buildStateFrontmatter + * and copies keys with Object.keys, which skips the Symbol) MUST call this before + * reconstructFrontmatter, or the channel parseYamlRegion attached to the extracted + * source is lost. + */ +function propagateCommentChannel(source: Frontmatter, target: Frontmatter): void { + const channel = (source as Record)[FULL_LINE_COMMENTS as unknown as symbol] as FullLineCommentChannel | undefined; + if (!channel) return; + const filtered: FullLineCommentChannel = { leading: {}, trailing: channel.trailing }; + for (const [key, comments] of Object.entries(channel.leading)) { + if (key in target) filtered.leading[key] = comments; + } + if (filtered.trailing.length || Object.keys(filtered.leading).length) { + (target as Record)[FULL_LINE_COMMENTS as unknown as symbol] = filtered; + } +} + /** * Slice a frontmatter YAML body into per-top-level-key raw text segments. Each segment * runs from a column-0 `key:` line through the line before the next column-0 key (or the @@ -826,4 +896,5 @@ export = { cmdFrontmatterSet, cmdFrontmatterMerge, cmdFrontmatterValidate, + propagateCommentChannel, }; diff --git a/src/state.cts b/src/state.cts index dd7e6d4db..a7226114e 100644 --- a/src/state.cts +++ b/src/state.cts @@ -27,7 +27,7 @@ const { planningDir, planningPaths } = planningWorkspace; import { realClock } from './clock.cjs'; // eslint-disable-next-line @typescript-eslint/no-require-imports import frontmatter = require('./frontmatter.cjs'); -const { extractFrontmatter, reconstructFrontmatter, stripFrontmatter } = frontmatter; +const { extractFrontmatter, reconstructFrontmatter, stripFrontmatter, propagateCommentChannel } = frontmatter; // eslint-disable-next-line @typescript-eslint/no-require-imports import scanPhasePlans = require('./plan-scan.cjs'); // eslint-disable-next-line @typescript-eslint/no-require-imports @@ -2362,6 +2362,12 @@ function syncStateFrontmatter(content: string, cwd: string | undefined, authorit } } + // #3257: propagate full-line frontmatter comments from the extracted source onto the + // rebuilt derivedFm (buildStateFrontmatter + the Object.keys carry-forward above both + // skip the Symbol-keyed channel, so without this the comments would be lost here even + // though parseYamlRegion/reconstructFrontmatter preserve them in isolation). + propagateCommentChannel(existingFm as unknown as Frontmatter, derivedFm as unknown as Frontmatter); + const yamlStr = reconstructFrontmatter(derivedFm as unknown as Frontmatter); return `---\n${yamlStr}\n---\n\n${body}`; } diff --git a/tests/frontmatter.test.cjs b/tests/frontmatter.test.cjs index a0e09ace9..009646c67 100644 --- a/tests/frontmatter.test.cjs +++ b/tests/frontmatter.test.cjs @@ -295,6 +295,47 @@ describe('reconstructFrontmatter', () => { const extracted2 = extractFrontmatter(roundTrip); assert.deepStrictEqual(extracted2, extracted1, 'round-trip should preserve multiple data types'); }); + + test('#3257: full-line comments survive an extract→reconstruct round-trip', () => { + const original = '---\ngsd_state_version: 1.0\n# NOTE: current_phase is hand-maintained here\ncurrent_phase: 3\nstatus: executing\n---'; + const extracted = extractFrontmatter(original); + assert.strictEqual(extracted['gsd_state_version'], '1.0'); + assert.strictEqual(extracted['current_phase'], '3'); + + const reconstructed = reconstructFrontmatter(extracted); + // #3257: the comment must survive in place (between gsd_state_version and current_phase). + assert.ok( + reconstructed.includes('# NOTE: current_phase is hand-maintained here'), + `comment should survive reconstruct; got:\n${reconstructed}`, + ); + // data identity preserved alongside the comment. + assert.ok(reconstructed.includes('gsd_state_version: 1.0')); + assert.ok(reconstructed.includes('current_phase: 3')); + assert.ok(reconstructed.includes('status: executing')); + // the reconstructed output re-parses to the same data (idempotent round-trip). + const reextracted = extractFrontmatter(`---\n${reconstructed}\n---`); + assert.strictEqual(reextracted['current_phase'], '3'); + }); + + test('#3257: leading (before first key) and trailing (after last key) comments survive', () => { + const original = '---\n# top comment\na: 1\nb: 2\n# trailing comment\n---'; + const extracted = extractFrontmatter(original); + const reconstructed = reconstructFrontmatter(extracted); + assert.ok(reconstructed.includes('# top comment'), `leading comment lost:\n${reconstructed}`); + assert.ok(reconstructed.includes('# trailing comment'), `trailing comment lost:\n${reconstructed}`); + }); + + test('#3257: multiple consecutive comments survive in order', () => { + const original = '---\na: 1\n# first note\n# second note\nb: 2\n---'; + const extracted = extractFrontmatter(original); + const reconstructed = reconstructFrontmatter(extracted); + assert.ok(reconstructed.includes('# first note') && reconstructed.includes('# second note'), + `consecutive comments lost:\n${reconstructed}`); + const aIdx = reconstructed.indexOf('a: 1'); + const firstIdx = reconstructed.indexOf('# first note'); + const secondIdx = reconstructed.indexOf('# second note'); + assert.ok(aIdx < firstIdx && firstIdx < secondIdx, `order wrong (a:${aIdx} first:${firstIdx} second:${secondIdx})`); + }); }); // ─── spliceFrontmatter ────────────────────────────────────────────────────── diff --git a/tests/state.test.cjs b/tests/state.test.cjs index aea8966fd..aeee20566 100644 --- a/tests/state.test.cjs +++ b/tests/state.test.cjs @@ -915,6 +915,39 @@ team: platform assert.ok(content.includes('status: executing'), 'schema-owned status still preserved'); }); + test('#3257: full-line frontmatter comments survive a mutating state verb', () => { + // syncStateFrontmatter rebuilds frontmatter via buildStateFrontmatter (fresh object) + // + an Object.keys carry-forward. Without propagating the comment channel, the + // comment is lost HERE even though the parse→reconstruct pair preserves it in + // isolation. This is the e2e path the issue is filed against. + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + `--- +status: executing +milestone: v1.0 +# NOTE: current_phase is hand-maintained here while the roadmap is in flux +current_phase: 3 +--- + +# Project State + +**Current Phase:** 03 +**Current Plan:** 03-02 +` + ); + + // Any writeStateMd triggers syncStateFrontmatter. + runGsdTools('state update "Current Plan" "03-03"', tmpDir); + + const content = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8'); + assert.ok( + content.includes('# NOTE: current_phase is hand-maintained here while the roadmap is in flux'), + `full-line frontmatter comment must survive a mutating verb; got:\n${content}`, + ); + // The mutation itself still applied. + assert.ok(content.includes('03-03'), 'the state update still took effect'); + }); + test('round-trip: write then read via state json', () => { fs.writeFileSync( path.join(tmpDir, '.planning', 'STATE.md'),