fix(#4998): read the decoded milestone scalar, so a quoted milestone stops gaining an escape layer

.changeset/sharp-pandas-squeak.md

@@ -0,0 +1,5 @@
+---
+type: Fixed
+pr: 5234
+---
+**A quoted `STATE.md` milestone stays correct, and session fields are written whole and inside the session section** — a milestone containing `"` gained one more layer of quotes and backslashes in `state.json` on every `state.*` write. `state record-session` rewrote only the first line of a wrapped session field and orphaned the rest; it now leaves that field whole and lists it under `skipped`, inserts a missing field beside its siblings in the section's plain or bold form, and with no session heading appends a section that carries the values already recorded instead of `None`. A fenced example line in the session section is never taken for the field by a session writer (`record-session`, `phase complete`, `state update`/`patch`) or reader (the frontmatter sync, `state-snapshot`); a frontmatter-only `stopped_at` that `record-session` replaces is reported in `replacedRecord`, and a `**Stopped at**:` line it updates reads back. `state update` and `state patch` write every session field (Stopped At, Paused At, Resume File, Last session, Last Date) inside the session section rather than the first match anywhere, so on a `STATE.md` with no `## Session` or `## Session Continuity` section an update or patch of Resume File, Last session or Last Date now reports the field not found and leaves the document unchanged; `state patch` reports a field already holding the requested value as updated, as `state update` does. `state update`, `state patch` and `record-session` refuse a value containing CR, LF, U+2028 or U+2029 by name, as do the planning-document field and prose writers for a value that would add one, `state add-roadmap-evolution` flattens all four in a note, and the frontmatter writer quotes a value holding U+2028/U+2029. (#4998)

docs/CLI-TOOLS.md

@@ -137,6 +137,10 @@ node gsd-tools.cjs state signal-waiting --type TYPE --question "..." --options "
 node gsd-tools.cjs state signal-resume
 ```
 
+`state update`, `state patch` and `state record-session` take single-line values: a value containing a line break — CR, LF, U+2028 LINE SEPARATOR or U+2029 PARAGRAPH SEPARATOR, each of which starts a new line for the STATE.md readers — is refused with an error naming the field or flag, before anything is written.
+
+`state record-session` lists the fields it wrote under `updated`, and under `replacedRecord` any Stopped At / Resume File value the write displaced (the `None` placeholder is not a record). In the session section, a line inside a fenced code block is an example: no session writer (`record-session`, `state update`/`patch` of a session field, `phase complete`) changes it, and neither the frontmatter sync nor `state-snapshot` reads it as a field. A field whose current value continues onto the next line is left unchanged, because a single-line write would orphan the rest: it is listed under `skipped` as `{ "field", "reason": "wrapped_value", "continuation" }` with a warning on stderr, and when every field to update was skipped the call reports `"recorded": false`. With no session heading, the appended `## Session` section carries each field's existing value (a skipped field included); a carried value holding U+2028/U+2029 is left out and listed as `{ "field", "reason": "line_separator" }`, and when the document ends inside an unclosed code fence nothing is appended (it would land inside the fence) and each field not already written in place is listed with `"reason": "unclosed_fence"`. `state update` and `state patch` write a session field (Stopped At, Paused At, Resume File, Last session, Last Date) inside the session section — not a fenced example there, and not a copy in another section such as `## Session Continuity Archive` — and report it updated; a document whose session section lacks the field, or that has no session section, reports it not found and is left unchanged. `state patch`, like `state update`, reports a field that already holds the requested value as updated.
+
 ### State Snapshot
 
 Structured parse of the full STATE.md:

src/frontmatter.cts

@@ -949,6 +949,13 @@ function scalarNeedsDoubleQuoting(s: string): boolean {
   // escapeDoubleQuotedScalar path (which already emits the \uD800 escape) fixes the
   // round-trip.
   if (/[\uD800-\uDFFF]/.test(s)) return true;
+  // #4998: U+2028/U+2029 are not YAML line breaks, so a bare one round-trips
+  // through js-yaml, but every line-anchored reader of a planning document
+  // (`^key:\s*(.+)` with the `m` flag) ends the line there and reads only the
+  // part before it. (A value holding `:` was already quoted by the callers, so
+  // no key could be forged this way.) The quoted path escapes them (`\L`,
+  // `\P`), which no such reader splits on.
+  if (/[\u2028\u2029]/.test(s)) return true;
   return false;
 }
 

src/planning-document.cts

@@ -35,6 +35,16 @@ import frontmatterModule from './frontmatter.cjs';
 import { locateFrontmatterFence } from './frontmatter-fence.cjs';
 import type { Result } from './write-set.cjs';
 
+/**
+ * #4998: every character that ends a line for the planning-document readers.
+ * Their `^`/`$` run with the `m` flag, and a JavaScript multiline anchor
+ * matches at U+2028 LINE SEPARATOR and U+2029 PARAGRAPH SEPARATOR as well as
+ * at CR and LF — so a single-line value carrying either one forges a sibling
+ * line just as `\n` does. Every single-line writer, here (`setFieldValue`,
+ * `replaceProse`) and in `state-document.cts`, refuses a value matching this.
+ */
+export const FIELD_LINE_BREAK_RE = /[\r\n\u2028\u2029]/;
+
 const { extractFrontmatter, FRONTMATTER_UNPARSEABLE } = frontmatterModule;
 
 // ─── Types ────────────────────────────────────────────────────────────────────
@@ -712,13 +722,25 @@ export function setFieldValue(doc: PlanningDoc, id: NodeId, value: string): Resu
     return { ok: false, reason: `node kind '${node.kind}' is not writable this phase` };
   }
   // #4917 / ADR-4910 Decision 2 & 4: a boldField's token boundary is a LINE
-  // boundary, not just an offset range — a value containing \n or \r escapes
+  // boundary, not just an offset range — a value containing any
+  // FIELD_LINE_BREAK_RE character (#4998: U+2028/U+2029 too) escapes
   // the field's own span and reparses as sibling structure (a forged field)
   // once spliced back into the source. Decision 4 licenses refusal for any
   // value the grammar cannot represent; Phase 3 may widen this to escaping,
   // but Phase 1 refuses outright. Do not remove this as an over-restriction.
-  if (/[\r\n]/.test(value)) {
-    return { ok: false, reason: 'field value must not contain a line break (\\r or \\n)' };
+  // One carve-out (#4998, Codex review): a U+2028/U+2029 the field ALREADY
+  // carries may stay when everything from it on is byte-identical — a rewrite
+  // of the text before it (phase renumbering a `Depends on`) adds no line, and
+  // refusing it left the field silently unrenumbered.
+  const breakAt = value.search(FIELD_LINE_BREAK_RE);
+  if (breakAt !== -1) {
+    const current = doc.source.slice(node.valueSpan.start, node.valueSpan.end);
+    const currentBreak = current.search(FIELD_LINE_BREAK_RE);
+    const keepsExistingTail = !/[\r\n]/.test(value) && currentBreak !== -1
+      && value.slice(breakAt) === current.slice(currentBreak);
+    if (!keepsExistingTail) {
+      return { ok: false, reason: 'field value must not contain a line break (\\r, \\n, U+2028 or U+2029)' };
+    }
   }
 
   // #4917 / ADR-4910 Decision 4: "a value that cannot be represented in the
@@ -821,7 +843,7 @@ export function replaceProse(doc: PlanningDoc, from: string, to: string): Result
   if (typeof from !== 'string' || from.length === 0) {
     return { ok: false, reason: 'replaceProse: `from` must be a non-empty string' };
   }
-  if (typeof to !== 'string' || /[\r\n]/.test(from) || /[\r\n]/.test(to)) {
+  if (typeof to !== 'string' || FIELD_LINE_BREAK_RE.test(from) || FIELD_LINE_BREAK_RE.test(to)) {
     return { ok: false, reason: 'replaceProse: `from` and `to` must be single-line strings' };
   }
   if (doc.staged.size > 0) {

src/roadmap-parser.cts

@@ -59,6 +59,9 @@ import planningWorkspace = require('./planning-workspace.cjs');
 const { planningDir, resolvePhaseIdConvention } = planningWorkspace;
 import { platformReadSync } from './shell-command-projection.cjs';
 // eslint-disable-next-line @typescript-eslint/no-require-imports
+import frontmatterMod = require('./frontmatter.cjs');
+const { rawFrontmatterField } = frontmatterMod;
+// eslint-disable-next-line @typescript-eslint/no-require-imports
 import unusableInputMod = require('./unusable-input.cjs');
 const { UNUSABLE_REASON, warnUnusableInput } = unusableInputMod;
 import { tokenizeHeadings, stripTaggedBlocks, editOutsideTaggedBlocks, withSection, stripFencedCode, collectSection } from './markdown-sectionizer.cjs';
@@ -1015,6 +1018,20 @@ function classifyMilestoneScalar(raw: string | null | undefined): { explicitNull
   return { explicitNull: false, version: trimmed };
 }
 
+/**
+ * #4998: STATE.md's `milestone:` scalar as the frontmatter parser DECODES it.
+ * The raw `^milestone:` line read captured the on-disk YAML text — quotes and
+ * escapes included — and that text was re-serialized as the value, adding one
+ * escape layer per `state.*` write. Falls back to the legacy line read only
+ * when there is no parseable frontmatter carrying a string `milestone` key.
+ */
+function readStateMilestoneScalar(stateRaw: string): string | null {
+  const field = rawFrontmatterField(stateRaw, 'milestone');
+  if (field && typeof field.value === 'string') return field.value;
+  const m = stateRaw.match(/^milestone:\s*(.+)/m);
+  return m ? m[1] : null;
+}
+
 /**
  * Extract the current milestone section from ROADMAP.md by positive lookup,
  * carrying a `scope` discriminator (ADR-3180 Decision 2) alongside the value.
@@ -1055,9 +1072,9 @@ function extractCurrentMilestoneScoped(content: string, cwd?: string, ws?: strin
     const statePath = path.join(planningDir(cwd, ws), 'STATE.md');
     const stateRaw = platformReadSync(statePath);
     if (stateRaw !== null) {
-      const milestoneMatch = stateRaw.match(/^milestone:\s*(.+)/m);
-      if (milestoneMatch) {
-        const classified = classifyMilestoneScalar(milestoneMatch[1]);
+      const milestoneScalar = readStateMilestoneScalar(stateRaw);
+      if (milestoneScalar !== null) {
+        const classified = classifyMilestoneScalar(milestoneScalar);
         version = classified.version;
         stateAssertsNoMilestone = classified.explicitNull;
       }
@@ -1894,9 +1911,9 @@ function getMilestoneInfo(cwd?: string): { value: MilestoneInfo | null; scope: S
         const statePath = path.join(planningDir(cwd), 'STATE.md');
         const stateRaw = platformReadSync(statePath);
         if (stateRaw !== null) {
-          const m = stateRaw.match(/^milestone:\s*(.+)/m);
-          if (m) {
-            const classified = classifyMilestoneScalar(m[1]);
+          const milestoneScalar = readStateMilestoneScalar(stateRaw);
+          if (milestoneScalar !== null) {
+            const classified = classifyMilestoneScalar(milestoneScalar);
             explicitlyNoMilestone = classified.explicitNull;
             stateVersion = classified.version;
           }
@@ -2355,9 +2372,9 @@ function currentMilestoneRawRanges(
     const statePath = path.join(planningDir(cwd), 'STATE.md');
     const stateRaw = platformReadSync(statePath);
     if (stateRaw !== null) {
-      const milestoneMatch = stateRaw.match(/^milestone:\s*(.+)/m);
-      if (milestoneMatch) {
-        const classified = classifyMilestoneScalar(milestoneMatch[1]);
+      const milestoneScalar = readStateMilestoneScalar(stateRaw);
+      if (milestoneScalar !== null) {
+        const classified = classifyMilestoneScalar(milestoneScalar);
         version = classified.version;
         stateAssertsNoMilestone = classified.explicitNull;
       }
@@ -2412,6 +2429,7 @@ export = {
   getRoadmapPhaseInternal,
   getMilestoneInfo,
   classifyMilestoneScalar,
+  readStateMilestoneScalar,
   getMilestonePhaseFilter,
   currentMilestoneRawRanges,
   withPhaseSection,