From cb9ba7a3e2c644e24672a398b4f9c146126d735e Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Tue, 8 Sep 2026 18:33:22 +0000 Subject: [PATCH] fix(design-md): edits follow a symlinked DESIGN.md, keep the BOM and the majority line ending, refuse an unclosed fence - `mark`, `convert --write`, and the design binary's extraction replaced a symlinked DESIGN.md (a docs-site layout) with a regular file and left the real target untouched; both writers resolve the link first. - A single stray CRLF flipped a whole LF file to CRLF: the editors now keep the majority ending. A UTF-8 BOM broke format detection and ended up mid-file after `mark`; it is recognized and kept at byte 0. - Re-running `mark` on a marked file deleted the blank line after the marker (`\s*$` matched across the newline); the marker regexes use `[ \t]*`. - Fences: readers follow markdown (an unclosed fence runs to EOF); the text-level editors refuse such a file with DesignMdEditRefused (DESIGN_MD_EDIT_REFUSED) instead of splicing the wrong section, and the design binary reports that and leaves the file alone. - needsQuotes also quotes a scalar containing ` #` (an inline-comment shape parsed back as a truncated value). Co-Authored-By: Claude Fable 5.1 --- bin/gstack-design-md.ts | 4 ++- design/src/memory.ts | 17 ++++++++--- lib/design-md.ts | 65 +++++++++++++++++++++++++---------------- test/design-md.test.ts | 51 +++++++++++++++++++++++++++++--- 4 files changed, 103 insertions(+), 34 deletions(-) diff --git a/bin/gstack-design-md.ts b/bin/gstack-design-md.ts index c27ad1232..1d4ef5280 100755 --- a/bin/gstack-design-md.ts +++ b/bin/gstack-design-md.ts @@ -30,8 +30,10 @@ import { parseDesignMd, detectFormat, convertLegacy, renderDesignMd, tokensFlat, insertMarker, type DesignMdDoc, type FormatChoice, FORMAT_CHOICES } from '../lib/design-md'; +/** The file itself, through any symlink (a `DESIGN.md -> docs/DESIGN.md` layout must edit the target, never replace the link). */ function resolveFile(arg?: string): string { - return path.resolve(arg ?? 'DESIGN.md'); + const p = path.resolve(arg ?? 'DESIGN.md'); + try { return fs.realpathSync(p); } catch { return p; } } function load(file: string): { text: string; doc: DesignMdDoc } | null { diff --git a/design/src/memory.ts b/design/src/memory.ts index 7c845c89d..bb85a47b2 100644 --- a/design/src/memory.ts +++ b/design/src/memory.ts @@ -15,7 +15,7 @@ import fs from "fs"; import path from "path"; import { requireApiKey } from "./auth"; import { receiptedFetch } from "./receipted-fetch"; -import { parseDesignMd, detectFormat, renderDesignMd, spliceSection, specSkeleton, tokensFlat, slug } from "../../lib/design-md"; +import { parseDesignMd, detectFormat, renderDesignMd, spliceSection, specSkeleton, tokensFlat, slug, DesignMdEditRefused } from "../../lib/design-md"; import { atomicWriteSync } from "../../lib/fs-atomic"; /** The section the extraction owns in DESIGN.md (replaced on every run). */ @@ -118,15 +118,24 @@ export function updateDesignMd( extracted: ExtractedDesign, sourceMockup: string, ): void { - const designPath = path.join(repoRoot, "DESIGN.md"); + const linkPath = path.join(repoRoot, "DESIGN.md"); const timestamp = new Date().toISOString().split("T")[0]; const body = formatExtractedSection(extracted, sourceMockup, timestamp); - if (fs.existsSync(designPath)) { - atomicWriteSync(designPath, spliceSection(fs.readFileSync(designPath, "utf-8"), EXTRACTED_SECTION_HEADING, body)); + if (fs.existsSync(linkPath)) { + const designPath = fs.realpathSync(linkPath); // edit the file behind a symlink, never replace the link + let next: string; + try { + next = spliceSection(fs.readFileSync(designPath, "utf-8"), EXTRACTED_SECTION_HEADING, body); + } catch (err) { + if (err instanceof DesignMdEditRefused) { console.error(`DESIGN.md not updated: ${err.message}`); return; } + throw err; + } + atomicWriteSync(designPath, next); console.error(`Updated DESIGN.md with extracted design language`); return; } + const designPath = linkPath; const colors: Record = {}; for (const c of extracted.colors) { diff --git a/lib/design-md.ts b/lib/design-md.ts index c191f7f32..75e535142 100644 --- a/lib/design-md.ts +++ b/lib/design-md.ts @@ -14,9 +14,10 @@ // ──► upsertSection ──► body-only splice on the parsed doc (files gstack writes from scratch) // ──► renderDesignMd ──► marker, front matter, preamble, canonical sections in order, extras // text ──► spliceSection / insertMarker ──► text-level edits of a file the USER owns: one section -// body or one marker line changes, every other byte and -// the file's line endings are untouched (the `mark` verb, -// the design binary's extraction section) +// body or one marker line changes; the BOM, the majority +// line ending, and every other line survive (the `mark` +// verb, the design binary's extraction section). A file +// with an unclosed fence is refused (DesignMdEditRefused). // ──► tokensFlat ──► "colors.primary" → "#F59E0B"; {path} refs resolved to primitives // // Format marker (the user's one-time conversion answer, persisted in the file): @@ -52,9 +53,10 @@ export const FORMAT_CHOICES = ['spec', 'legacy-keep'] as const; export type FormatChoice = (typeof FORMAT_CHOICES)[number]; const MARKER_RE_BODY = FORMAT_MARKER_PREFIX.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') + '(' + FORMAT_CHOICES.join('|') + ')'; /** `` on line 1 (legacy files) */ -const LEGACY_MARKER_RE = new RegExp('^\\n?'); +const LEGACY_MARKER_RE = new RegExp('^\\n?'); /** `# gstack: design-md-format=...` as a YAML comment inside the front matter (spec files) */ -const YAML_MARKER_RE = new RegExp('^# ' + MARKER_RE_BODY + '\\s*$', 'm'); +// `[ \\t]*$`, never `\\s*$`: a multi-line match would swallow the blank line after the marker. +const YAML_MARKER_RE = new RegExp('^# ' + MARKER_RE_BODY + '[ \\t]*$', 'm'); /** The marker line inside front matter, newline included (renderDesignMd drops it before re-emitting the marker itself). */ const YAML_MARKER_LINE_RE = new RegExp(YAML_MARKER_RE.source + '\\n', 'm'); /** A front matter opener immediately followed by the marker line (insertMarker replaces the old choice). */ @@ -109,8 +111,11 @@ function parseYaml(text: string): { value: Record | null; error } } +/** A UTF-8 byte-order mark (Windows editors write one); text-level editors keep it at byte 0. */ +const BOM = '\uFEFF'; + export function parseDesignMd(text: string): DesignMdDoc { - const src = text.replace(/\r\n/g, '\n'); + const src = text.replace(/^\uFEFF/, '').replace(/\r\n/g, '\n'); let rest = src; let marker: FormatChoice | null = null; let frontmatterText: string | null = null; @@ -136,7 +141,7 @@ export function parseDesignMd(text: string): DesignMdDoc { } const lines = rest.split('\n'); - const heads = headingLines(lines); + const { heads } = headingLines(lines); const preambleLines = lines.slice(0, heads[0]?.index ?? lines.length); const sections: Section[] = heads.map((h, k) => { const canonical = canonicalFor(h.heading); @@ -147,23 +152,27 @@ export function parseDesignMd(text: string): DesignMdDoc { } /** - * The `## ` headings of a body, with code fences skipped. The one section- + * The `## ` headings of a body, with code fences skipped: the one section- * boundary rule, shared by parseDesignMd and spliceSection so they cannot drift. - * An unclosed fence is treated as prose (fence tracking off for that file): - * a stray ``` must never swallow every later section of a file gstack edits. + * Markdown semantics: an unclosed fence runs to the end of the file, so nothing + * after it is a heading. Readers accept that; spliceSection refuses to edit such + * a file (`unclosedFence`), because "which section" is ambiguous there. */ -function headingLines(lines: string[]): Array<{ index: number; heading: string }> { - const fences = lines.filter(l => /^```/.test(l)).length; - const trackFences = fences % 2 === 0; - const out: Array<{ index: number; heading: string }> = []; +function headingLines(lines: string[]): { heads: Array<{ index: number; heading: string }>; unclosedFence: boolean } { + const heads: Array<{ index: number; heading: string }> = []; let inFence = false; for (let i = 0; i < lines.length; i++) { - if (trackFences && /^```/.test(lines[i])) { inFence = !inFence; continue; } + if (/^```/.test(lines[i])) { inFence = !inFence; continue; } if (inFence) continue; const h = lines[i].match(/^## (.+?)\s*$/); - if (h) out.push({ index: i, heading: h[1] }); + if (h) heads.push({ index: i, heading: h[1] }); } - return out; + return { heads, unclosedFence: inFence }; +} + +/** Thrown by the text-level editors when the file cannot be edited safely (an unclosed code fence). The bins print it as DESIGN_MD_EDIT_REFUSED and leave the file unchanged. */ +export class DesignMdEditRefused extends Error { + constructor(reason: string) { super(`${SENTINEL.DESIGN_MD_EDIT_REFUSED}: ${reason}; file unchanged`); this.name = 'DesignMdEditRefused'; } } /** Does a section heading name the requested section? By canonical name when the request has one, else by exact (case-insensitive) heading. */ @@ -171,9 +180,11 @@ function headingMatches(heading: string, wanted: string, canonical: CanonicalSec return canonical ? canonicalFor(heading) === canonical : heading.trim().toLowerCase() === wanted.trim().toLowerCase(); } -/** The file's dominant line ending; text-level editors restore it so a CRLF file stays CRLF. */ +/** The file's majority line ending; text-level editors restore it so a CRLF file stays CRLF (a lone stray CRLF in an LF file does not flip the file). */ function eolOf(text: string): string { - return text.includes('\r\n') ? '\r\n' : '\n'; + const crlf = (text.match(/\r\n/g) ?? []).length; + const lf = (text.match(/\n/g) ?? []).length - crlf; + return crlf > lf ? '\r\n' : '\n'; } // ── Format detection ───────────────────────────────────────────────────────── @@ -209,7 +220,8 @@ export function detectFormat(doc: DesignMdDoc | null): { format: DesignMdFormat; function needsQuotes(s: string): boolean { // Control characters (an LLM-extracted font family with an embedded newline) must // go through the double-quoted form: a bare multi-line scalar does not parse back. - return s === '' || /[\x00-\x1f\x7f]/.test(s) || /^[\s#&*!|>'"%@`{[\]},:?-]|[:#]\s|\s$|^(true|false|null|yes|no|on|off|~)$|^[-+]?(\d+\.?\d*|\.\d+)([eE][-+]?\d+)?$/i.test(s); + // `\s#` too: a plain scalar ending in ` #F59E0B` would parse back as a comment. + return s === '' || /[\x00-\x1f\x7f]/.test(s) || /^[\s#&*!|>'"%@`{[\]},:?-]|[:#]\s|\s#|\s$|^(true|false|null|yes|no|on|off|~)$|^[-+]?(\d+\.?\d*|\.\d+)([eE][-+]?\d+)?$/i.test(s); } function yamlScalar(v: unknown): string { @@ -284,11 +296,13 @@ export function renderDesignMd(doc: DesignMdDoc, opts: RenderOptions = {}): stri * writes from scratch (convert, skeletons). */ export function spliceSection(text: string, heading: string, body: string): string { + const bom = text.startsWith(BOM) ? BOM : ''; const eol = eolOf(text); - const src = text.replace(/\r\n/g, '\n'); + const src = text.slice(bom.length).replace(/\r\n/g, '\n'); const canonical = canonicalFor(heading); const lines = src.split('\n'); - const heads = headingLines(lines); + const { heads, unclosedFence } = headingLines(lines); + if (unclosedFence) throw new DesignMdEditRefused('unclosed code fence (```) makes the section boundaries ambiguous'); const k = heads.findIndex(h => headingMatches(h.heading, heading, canonical)); const block = `## ${canonical ?? heading}\n\n${body.replace(/\s+$/, '')}\n`; let out: string; @@ -301,7 +315,7 @@ export function spliceSection(text: string, heading: string, body: string): stri const after = lines.slice(end).join('\n'); out = before + (before ? '\n' : '') + block + (after.trim() ? '\n' + after.replace(/^\n+/, '') : ''); } - return eol === '\n' ? out : out.replace(/\n/g, eol); + return bom + (eol === '\n' ? out : out.replace(/\n/g, eol)); } /** @@ -310,8 +324,9 @@ export function spliceSection(text: string, heading: string, body: string): stri * marker; every other byte is untouched. */ export function insertMarker(text: string, choice: FormatChoice): string { + const bom = text.startsWith(BOM) ? BOM : ''; const eol = eolOf(text); - const src = text.replace(/\r\n/g, '\n'); + const src = text.slice(bom.length).replace(/\r\n/g, '\n'); const stripped = src.replace(LEGACY_MARKER_RE, ''); let out: string; if (stripped.startsWith('---\n')) { @@ -320,7 +335,7 @@ export function insertMarker(text: string, choice: FormatChoice): string { } else { out = `\n` + stripped; } - return eol === '\n' ? out : out.replace(/\n/g, eol); + return bom + (eol === '\n' ? out : out.replace(/\n/g, eol)); } /** Replace or add a section; canonical names slot into spec order, extras append. Body-only: front matter bytes untouched. */ diff --git a/test/design-md.test.ts b/test/design-md.test.ts index 72c152ca8..3d494595f 100644 --- a/test/design-md.test.ts +++ b/test/design-md.test.ts @@ -15,7 +15,7 @@ import * as path from 'path'; import { spawnSync } from 'child_process'; import { parseDesignMd, detectFormat, renderDesignMd, upsertSection, convertLegacy, tokensFlat, - emitYamlBlock, specSkeleton, spliceSection, insertMarker, CANONICAL_SECTIONS, TOKEN_GROUPS, isLegacyGstackFormat, + emitYamlBlock, specSkeleton, spliceSection, insertMarker, DesignMdEditRefused, CANONICAL_SECTIONS, TOKEN_GROUPS, isLegacyGstackFormat, } from '../lib/design-md'; import { updateDesignMd, readDesignConstraints } from '../design/src/memory'; @@ -561,13 +561,38 @@ describe('text-level editors keep line endings and respect fences', () => { expect(spliceSection(SPEC_LF, 'Colors', 'Ink only.')).not.toContain('\r'); }); - test('a fenced ## inside a section does not end it; an unclosed fence is prose, so later sections survive', () => { + test('a fenced ## inside a section does not end it; an unclosed fence runs to EOF for readers and refuses the edit', () => { const src = '## A\n\nbody\n\n```md\n## Not a heading\n```\n\n## B\n\nb body\n'; expect(spliceSection(src, 'A', 'x')).toBe('## A\n\nx\n\n## B\n\nb body\n'); expect(parseDesignMd(src).sections.map(s => s.heading)).toEqual(['A', 'B']); const unclosed = '## A\n\nbody\n\n```\nunclosed\n\n## B\n\nb body\n'; - expect(spliceSection(unclosed, 'A', 'x')).toBe('## A\n\nx\n\n## B\n\nb body\n'); - expect(parseDesignMd(unclosed).sections.map(s => s.heading)).toEqual(['A', 'B']); + expect(parseDesignMd(unclosed).sections.map(s => s.heading)).toEqual(['A']); // markdown: everything after the fence is code + expect(() => spliceSection(unclosed, 'A', 'x')).toThrow(DesignMdEditRefused); + expect(() => spliceSection(unclosed, 'A', 'x')).toThrow(/DESIGN_MD_EDIT_REFUSED: unclosed code fence/); + // the design binary skips the write and says why, rather than editing an ambiguous file + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-design-md-fence-')); + fs.writeFileSync(path.join(dir, 'DESIGN.md'), unclosed); + updateDesignMd(dir, { colors: [{ name: 'Ink', hex: '#111111', usage: 'text' }], typography: [], spacing: [], layout: [], mood: '' }, 'm.png'); + expect(fs.readFileSync(path.join(dir, 'DESIGN.md'), 'utf-8')).toBe(unclosed); + fs.rmSync(dir, { recursive: true, force: true }); + }); + + test('re-marking a marked spec file changes nothing; a stray CRLF does not flip an LF file; a BOM stays at byte 0', () => { + const marked = insertMarker(SPEC_LF, 'spec'); + expect(insertMarker(marked, 'spec')).toBe(marked); // the old regex ate the blank line after the marker + expect(insertMarker(marked, 'legacy-keep')).toBe(marked.replace('design-md-format=spec', 'design-md-format=legacy-keep')); + const stray = SPEC_LF.replace('name: x\n', 'name: x\r\n'); + expect(spliceSection(stray, 'Colors', 'c2')).not.toContain('\r'); // majority LF wins; the one stray CRLF is normalized, nothing else flips + const bom = '\uFEFF' + SPEC_LF; + expect(insertMarker(bom, 'spec')).toBe('\uFEFF' + insertMarker(SPEC_LF, 'spec')); + expect(spliceSection(bom, 'Colors', 'c2')).toBe('\uFEFF' + spliceSection(SPEC_LF, 'Colors', 'c2')); + expect(parseDesignMd(bom).frontmatterText).not.toBeNull(); + expect(detectFormat(parseDesignMd(bom)).format).toBe('spec'); + }); + + test('a scalar with a space-hash (an inline comment shape) is quoted and parses back', () => { + const yaml = emitYamlBlock({ colors: { amber: 'amber #F59E0B' } }); + expect(Bun.YAML.parse(yaml)).toEqual({ colors: { amber: 'amber #F59E0B' } }); }); test('a token value with an embedded newline is quoted and parses back', () => { @@ -575,3 +600,21 @@ describe('text-level editors keep line endings and respect fences', () => { expect(Bun.YAML.parse(yaml)).toEqual({ typography: { body: { fontFamily: 'Foo\nBar', fontSize: '16px\tx' } } }); }); }); + +describe('bin/gstack-design-md.ts follows a symlinked DESIGN.md', () => { + test('mark edits the target file and leaves the link a link', () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-design-md-link-')); + fs.mkdirSync(path.join(dir, 'docs')); + const legacy = '# T\n\n## Product Context\n\np\n\n## Aesthetic Direction\n\na\n'; + fs.writeFileSync(path.join(dir, 'docs', 'DESIGN.md'), legacy); + fs.symlinkSync(path.join('docs', 'DESIGN.md'), path.join(dir, 'DESIGN.md')); + try { + const r = spawnSync(process.execPath, ['--no-env-file', 'run', BIN, 'mark', 'legacy-keep', 'DESIGN.md'], { cwd: dir, encoding: 'utf-8', timeout: 30_000 }); + expect(r.status).toBe(0); + expect(fs.lstatSync(path.join(dir, 'DESIGN.md')).isSymbolicLink()).toBe(true); + expect(fs.readFileSync(path.join(dir, 'docs', 'DESIGN.md'), 'utf-8')).toBe('\n' + legacy); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); +});