mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-09 06:28:59 +02:00
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 <noreply@anthropic.com>
This commit is contained in:
Garry Tan
co-authored by
Claude Fable 5.1
Claude Fable 5.1
parent
8d709c8f29
commit
cb9ba7a3e2
@@ -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 {
|
||||
|
||||
+13
-4
@@ -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<string, string> = {};
|
||||
for (const c of extracted.colors) {
|
||||
|
||||
+40
-25
@@ -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('|') + ')';
|
||||
/** `<!-- gstack: design-md-format=... -->` on line 1 (legacy files) */
|
||||
const LEGACY_MARKER_RE = new RegExp('^<!--\\s*' + MARKER_RE_BODY + '\\s*-->\\n?');
|
||||
const LEGACY_MARKER_RE = new RegExp('^<!--[ \\t]*' + MARKER_RE_BODY + '[ \\t]*-->\\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<string, unknown> | 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 = `<!-- ${FORMAT_MARKER_PREFIX}${choice} -->\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. */
|
||||
|
||||
+47
-4
@@ -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('<!-- gstack: design-md-format=legacy-keep -->\n' + legacy);
|
||||
} finally {
|
||||
fs.rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user