From 69b0856d42edbe571230c9f2a5f6a60df9c68b25 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Tue, 8 Sep 2026 18:55:37 +0000 Subject: [PATCH] fix(design-md): markdown edge cases: rule-opened legacy files, spaced fences, ~~~ blocks, duplicate headings, YAML 1.2 numerics - insertMarker keyed on "starts with ---", so a legacy file opening with a horizontal rule got a `# gstack:` line rendered as a heading that the parser then never read back (the conversion question re-asked every run). It keys on parsed front matter. - A closing front-matter fence with trailing spaces (`--- `) made a valid spec file `unknown`; the closer is any whole `---` line. - `~~~` fences hid nothing, so a `## ` inside one was a section boundary and a splice corrupted the fence; both fence kinds are tracked and only the same kind closes an opener. - convertLegacy silently kept the first of two `## Layout` bodies (and one of `## Color` / `## Colors`); it refuses with DESIGN_MD_CONVERT_REFUSED and the bin leaves the file and writes no backup. - needsQuotes covers 0x / 0o / .inf / .nan (YAML 1.2 numerics that changed type on round-trip); emitYamlBlock throws on an object inside an array instead of writing "[object Object]". - The design binary coerces the model's extraction JSON at the parse boundary (null names, missing arrays) so the paid call's result survives. Co-Authored-By: Claude Fable 5.1 --- bin/gstack-design-md.ts | 10 +++++-- design/src/memory.ts | 21 +++++++++++---- lib/design-md.ts | 39 ++++++++++++++++++--------- test/design-md.test.ts | 59 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 110 insertions(+), 19 deletions(-) diff --git a/bin/gstack-design-md.ts b/bin/gstack-design-md.ts index 1d4ef5280..ec2f419f2 100755 --- a/bin/gstack-design-md.ts +++ b/bin/gstack-design-md.ts @@ -28,7 +28,7 @@ import { SENTINEL } from '../lib/design-detect-contract'; import { atomicWriteSync } from '../lib/fs-atomic'; import { parseDesignMd, detectFormat, convertLegacy, renderDesignMd, tokensFlat, insertMarker, - type DesignMdDoc, type FormatChoice, FORMAT_CHOICES } from '../lib/design-md'; + type DesignMdDoc, type FormatChoice, FORMAT_CHOICES, DesignMdEditRefused } 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 { @@ -67,7 +67,13 @@ export function main(argv = process.argv.slice(2)): number { process.stderr.write(`${SENTINEL.DESIGN_MD_FORMAT}: ${format}${reason ? ` (${reason})` : ''}; convert only accepts a legacy gstack DESIGN.md\n`); return 1; } - const out = renderDesignMd(convertLegacy(loaded.doc), { emitFrontmatter: true }); + let out: string; + try { + out = renderDesignMd(convertLegacy(loaded.doc), { emitFrontmatter: true }); + } catch (err) { + if (err instanceof DesignMdEditRefused) { process.stderr.write(`${SENTINEL.DESIGN_MD_CONVERT_REFUSED}: ${err.message.replace(/^[A-Z_]+: /, '')}\n`); return 2; } + throw err; + } if (flags.has('--write')) { fs.writeFileSync(`${file}.legacy.bak`, loaded.text); atomicWriteSync(file, out); diff --git a/design/src/memory.ts b/design/src/memory.ts index bb85a47b2..378e55348 100644 --- a/design/src/memory.ts +++ b/design/src/memory.ts @@ -84,7 +84,18 @@ Extract real values from what you see. Be specific about hex colors and font siz const data = await response.json() as any; const content = data.choices?.[0]?.message?.content?.trim() || ""; - return JSON.parse(content) as ExtractedDesign; + // The model's JSON is unvalidated: default the arrays and coerce the strings so a null name + // cannot throw after the paid vision call. + const raw = JSON.parse(content) as Partial>; + const list = (v: unknown) => (Array.isArray(v) ? v : []); + const str = (v: unknown) => (v === null || v === undefined ? "" : String(v)); + return { + colors: list(raw.colors).map((c) => ({ name: str((c as Record)?.name), hex: str((c as Record)?.hex), usage: str((c as Record)?.usage) })), + typography: list(raw.typography).map((t) => ({ role: str((t as Record)?.role), family: str((t as Record)?.family), size: str((t as Record)?.size), weight: str((t as Record)?.weight) })), + spacing: list(raw.spacing).map(str), + layout: list(raw.layout).map(str), + mood: str(raw.mood), + }; } catch (err: any) { console.error(`Design extraction error: ${err.message}`); return defaultDesign(); @@ -138,13 +149,13 @@ export function updateDesignMd( const designPath = linkPath; const colors: Record = {}; - for (const c of extracted.colors) { - const key = slug(c.name); + for (const c of extracted.colors ?? []) { + const key = slug(String(c.name ?? "")); if (key !== "token" && /^#[0-9a-fA-F]{3,8}$/.test(c.hex) && !(key in colors)) colors[key] = c.hex; } const typography: Record> = {}; - for (const t of extracted.typography) { - const role = slug(t.role); + for (const t of extracted.typography ?? []) { + const role = slug(String(t.role ?? "")); if (role === "token" || typography[role]) continue; const entry: Record = { fontFamily: t.family }; if (t.size) entry.fontSize = t.size; diff --git a/lib/design-md.ts b/lib/design-md.ts index 75e535142..833fae84e 100644 --- a/lib/design-md.ts +++ b/lib/design-md.ts @@ -128,15 +128,16 @@ export function parseDesignMd(text: string): DesignMdDoc { rest = rest.slice(legacyMarker[0].length); } if (rest.startsWith('---\n')) { - const end = rest.indexOf('\n---', 4); - if (end !== -1 && (rest[end + 4] === '\n' || end + 4 === rest.length)) { - frontmatterText = rest.slice(4, end + 1); + // The closing fence is a whole line of `---` (trailing spaces allowed, an editor artifact); a value line like `---x` is not one. + const close = /^---[ \t]*$/m.exec(rest.slice(4)); + if (close) { + frontmatterText = rest.slice(4, 4 + close.index); const m = frontmatterText.match(YAML_MARKER_RE); if (m) marker = m[1] as FormatChoice; const parsed = parseYaml(frontmatterText); frontmatter = parsed.value; frontmatterError = parsed.error; - rest = rest.slice(end + 5); + rest = rest.slice(4 + close.index + close[0].length + 1); } } @@ -160,14 +161,16 @@ export function parseDesignMd(text: string): DesignMdDoc { */ function headingLines(lines: string[]): { heads: Array<{ index: number; heading: string }>; unclosedFence: boolean } { const heads: Array<{ index: number; heading: string }> = []; - let inFence = false; + let fence: string | null = null; // the opener's characters (``` or ~~~); only the same kind closes it for (let i = 0; i < lines.length; i++) { - if (/^```/.test(lines[i])) { inFence = !inFence; continue; } - if (inFence) continue; + const f = lines[i].match(/^(```|~~~)/); + if (f && fence === null) { fence = f[1]; continue; } + if (f && fence === f[1]) { fence = null; continue; } + if (fence !== null) continue; const h = lines[i].match(/^## (.+?)\s*$/); if (h) heads.push({ index: i, heading: h[1] }); } - return { heads, unclosedFence: inFence }; + return { heads, unclosedFence: fence !== null }; } /** 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. */ @@ -220,8 +223,9 @@ 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. - // `\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); + // `\s#` too: a plain scalar ending in ` #F59E0B` would parse back as a comment. YAML 1.2 also + // reads 0x1F / 0o17 / .inf / .nan as numbers, so those shapes are quoted as well. + return s === '' || /[\x00-\x1f\x7f]/.test(s) || /^[\s#&*!|>'"%@`{[\]},:?-]|[:#]\s|\s#|\s$|^(true|false|null|yes|no|on|off|~)$|^[-+]?(\d+\.?\d*|\.\d+)([eE][-+]?\d+)?$|^0[xob][0-9a-f_]+$|^[-+]?\.(inf|nan)$/i.test(s); } function yamlScalar(v: unknown): string { @@ -242,7 +246,10 @@ export function emitYamlBlock(obj: Record, indent = 0): string out.push(emitYamlBlock(v as Record, indent + 2)); } else if (Array.isArray(v)) { out.push(`${pad}${key}:`); - for (const item of v) out.push(`${pad} - ${yamlScalar(item)}`); + for (const item of v) { + if (item !== null && typeof item === 'object') throw new TypeError('emitYamlBlock: array items must be scalars (a nested object would be written as "[object Object]")'); + out.push(`${pad} - ${yamlScalar(item)}`); + } } else { out.push(`${pad}${key}: ${yamlScalar(v)}`); } @@ -329,7 +336,8 @@ export function insertMarker(text: string, choice: FormatChoice): string { 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')) { + // Front matter, not "starts with ---": a legacy file opening with a horizontal rule gets the HTML comment. + if (parseDesignMd(stripped).frontmatterText !== null) { const withoutOld = stripped.replace(FRONTMATTER_OPEN_WITH_MARKER_RE, '---\n'); out = withoutOld.replace(/^---\n/, `---\n# ${FORMAT_MARKER_PREFIX}${choice}\n`); } else { @@ -431,6 +439,13 @@ function firstFontName(value: string): string | undefined { * its original order. Idempotent: converting the render again changes nothing. */ export function convertLegacy(doc: DesignMdDoc, opts: { name?: string } = {}): DesignMdDoc { + // A heading the conversion consumes must be unique, or a second body would be silently dropped. + const counts = new Map(); + for (const s of doc.sections) counts.set(s.heading.trim().toLowerCase(), (counts.get(s.heading.trim().toLowerCase()) ?? 0) + 1); + for (const h of [...LEGACY_HEADINGS, 'Typography', 'Layout', 'Colors']) { + if ((counts.get(h.toLowerCase()) ?? 0) > 1) throw new DesignMdEditRefused(`legacy heading "## ${h}" appears more than once`); + } + if (counts.has('color') && counts.has('colors')) throw new DesignMdEditRefused('both "## Color" and "## Colors" are present'); const title = doc.preamble.match(/^#\s+(.+)$/m)?.[1]?.trim(); const name = opts.name ?? (title ? title.replace(/^Design System\s*[—–-]\s*/i, '').trim() : 'Design System'); const fm: Record = { name }; diff --git a/test/design-md.test.ts b/test/design-md.test.ts index 3d494595f..15ecdff9b 100644 --- a/test/design-md.test.ts +++ b/test/design-md.test.ts @@ -618,3 +618,62 @@ describe('bin/gstack-design-md.ts follows a symlinked DESIGN.md', () => { } }); }); + +describe('adversarial round: markdown edge cases the editors must survive', () => { + test('a legacy file that opens with a horizontal rule gets the HTML-comment marker, and the marker is read back', () => { + const legacy = '---\n\n# Design\n\n## Product Context\n\np\n\n## Aesthetic Direction\n\na\n'; + expect(detectFormat(parseDesignMd(legacy)).format).toBe('legacy'); + const marked = insertMarker(legacy, 'legacy-keep'); + expect(marked).toBe('\n' + legacy); + expect(parseDesignMd(marked).marker).toBe('legacy-keep'); + }); + + test('a closing front-matter fence with trailing spaces still closes; a `---x` value line does not', () => { + const doc = parseDesignMd('---\nname: x\ncolors:\n a: "#fff"\n--- \n\n## Overview\n\no\n'); + expect(doc.frontmatterText).toBe('name: x\ncolors:\n a: "#fff"\n'); + expect(detectFormat(doc).format).toBe('spec'); + expect(doc.sections.map(s => s.heading)).toEqual(['Overview']); + const odd = parseDesignMd('---\nname: x\ndescription: ---x\ncolors:\n a: "#fff"\n---\n\n## Overview\n\no\n'); + expect(odd.frontmatter).toEqual({ name: 'x', description: '---x', colors: { a: '#fff' } }); + }); + + test('~~~ fences hide headings like ``` fences, and only the same kind closes an opener', () => { + const src = '## Overview\n\n~~~\n## Fake\n```\nstill inside\n~~~\n\n## Colors\n\nc\n'; + expect(parseDesignMd(src).sections.map(s => s.heading)).toEqual(['Overview', 'Colors']); + expect(spliceSection(src, 'Overview', 'NEW')).toBe('## Overview\n\nNEW\n\n## Colors\n\nc\n'); + }); + + test('YAML 1.2 numeric shapes and nested array items are handled by the emitter', () => { + const yaml = emitYamlBlock({ typography: { body: { fontSize: '0x1F', fontWeight: '.inf', lineHeight: '0o17' } } }); + expect(Bun.YAML.parse(yaml)).toEqual({ typography: { body: { fontSize: '0x1F', fontWeight: '.inf', lineHeight: '0o17' } } }); + expect(() => emitYamlBlock({ components: [{ a: 1 }] } as never)).toThrow(/array items must be scalars/); + }); + + test('convert refuses a legacy file whose consumed heading repeats, instead of dropping a body', () => { + const dup = '# T\n\n## Product Context\n\np\n\n## Aesthetic Direction\n\na\n\n## Layout\n\nl1\n\n## Layout\n\nl2\n'; + expect(() => convertLegacy(parseDesignMd(dup))).toThrow(/appears more than once/); + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-design-md-dup-')); + try { + fs.writeFileSync(path.join(dir, 'DESIGN.md'), dup); + const r = runBin(['convert', 'DESIGN.md', '--write'], dir); + expect(r.code).toBe(2); + expect(r.err).toContain('DESIGN_MD_CONVERT_REFUSED: legacy heading "## Layout" appears more than once'); + expect(fs.readFileSync(path.join(dir, 'DESIGN.md'), 'utf-8')).toBe(dup); + expect(fs.existsSync(path.join(dir, 'DESIGN.md.legacy.bak'))).toBe(false); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + test('the design binary tolerates unvalidated extraction output (null names, missing arrays)', () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-design-md-null-')); + try { + updateDesignMd(dir, { colors: [{ name: null, hex: '#111111', usage: null }], typography: [{ role: null, family: 'X', size: null, weight: null }], spacing: [], layout: [], mood: '' } as never, 'm.png'); + const out = fs.readFileSync(path.join(dir, 'DESIGN.md'), 'utf-8'); + expect(out.startsWith('---\n')).toBe(true); + expect(Bun.YAML.parse(parseDesignMd(out).frontmatterText!)).toBeTruthy(); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); +});