diff --git a/packages/opencode/src/cli/error.ts b/packages/opencode/src/cli/error.ts index 569b186d55c..d7120aa5e98 100644 --- a/packages/opencode/src/cli/error.ts +++ b/packages/opencode/src/cli/error.ts @@ -28,7 +28,7 @@ export function FormatError(input: unknown) { return `Directory "${input.data.dir}" in ${input.data.path} is not valid. Rename the directory to "${input.data.suggestion}" or remove it. This is a common typo.` } if (ConfigMarkdown.FrontmatterError.isInstance(input)) { - return `Failed to parse frontmatter in ${input.data.path}:\n${input.data.message}` + return input.data.message } if (Config.InvalidError.isInstance(input)) return [ diff --git a/packages/opencode/src/config/config.ts b/packages/opencode/src/config/config.ts index 134358ec3ba..322ce273ab8 100644 --- a/packages/opencode/src/config/config.ts +++ b/packages/opencode/src/config/config.ts @@ -235,7 +235,7 @@ export namespace Config { })) { const md = await ConfigMarkdown.parse(item).catch((err) => { const message = ConfigMarkdown.FrontmatterError.isInstance(err) - ? `${err.data.path}: ${err.data.message}` + ? err.data.message : `Failed to parse command ${item}` Bus.publish(Session.Event.Error, { error: new NamedError.Unknown({ message }).toObject() }) log.error("failed to load command", { command: item, err }) @@ -274,7 +274,7 @@ export namespace Config { })) { const md = await ConfigMarkdown.parse(item).catch((err) => { const message = ConfigMarkdown.FrontmatterError.isInstance(err) - ? `${err.data.path}: ${err.data.message}` + ? err.data.message : `Failed to parse agent ${item}` Bus.publish(Session.Event.Error, { error: new NamedError.Unknown({ message }).toObject() }) log.error("failed to load agent", { agent: item, err }) @@ -312,7 +312,7 @@ export namespace Config { })) { const md = await ConfigMarkdown.parse(item).catch((err) => { const message = ConfigMarkdown.FrontmatterError.isInstance(err) - ? `${err.data.path}: ${err.data.message}` + ? err.data.message : `Failed to parse mode ${item}` Bus.publish(Session.Event.Error, { error: new NamedError.Unknown({ message }).toObject() }) log.error("failed to load mode", { mode: item, err }) diff --git a/packages/opencode/src/config/markdown.ts b/packages/opencode/src/config/markdown.ts index f20842c41a9..d1eeeac3821 100644 --- a/packages/opencode/src/config/markdown.ts +++ b/packages/opencode/src/config/markdown.ts @@ -14,8 +14,60 @@ export namespace ConfigMarkdown { return Array.from(template.matchAll(SHELL_REGEX)) } + export function preprocessFrontmatter(content: string): string { + const match = content.match(/^---\r?\n([\s\S]*?)\r?\n---/) + if (!match) return content + + const frontmatter = match[1] + const lines = frontmatter.split("\n") + const result: string[] = [] + + for (const line of lines) { + // skip comments and empty lines + if (line.trim().startsWith("#") || line.trim() === "") { + result.push(line) + continue + } + + // skip lines that are continuations (indented) + if (line.match(/^\s+/)) { + result.push(line) + continue + } + + // match key: value pattern + const kvMatch = line.match(/^([a-zA-Z_][a-zA-Z0-9_]*)\s*:\s*(.*)$/) + if (!kvMatch) { + result.push(line) + continue + } + + const key = kvMatch[1] + const value = kvMatch[2].trim() + + // skip if value is empty, already quoted, or uses block scalar + if (value === "" || value === ">" || value === "|" || value.startsWith('"') || value.startsWith("'")) { + result.push(line) + continue + } + + // if value contains a colon, convert to block scalar + if (value.includes(":")) { + result.push(`${key}: |`) + result.push(` ${value}`) + continue + } + + result.push(line) + } + + const processed = result.join("\n") + return content.replace(frontmatter, () => processed) + } + export async function parse(filePath: string) { - const template = await Bun.file(filePath).text() + const raw = await Bun.file(filePath).text() + const template = preprocessFrontmatter(raw) try { const md = matter(template) @@ -24,7 +76,7 @@ export namespace ConfigMarkdown { throw new FrontmatterError( { path: filePath, - message: `Failed to parse YAML frontmatter: ${err instanceof Error ? err.message : String(err)}`, + message: `${filePath}: Failed to parse YAML frontmatter: ${err instanceof Error ? err.message : String(err)}`, }, { cause: err }, ) diff --git a/packages/opencode/src/skill/skill.ts b/packages/opencode/src/skill/skill.ts index 95a599a5478..6ae0e9fe887 100644 --- a/packages/opencode/src/skill/skill.ts +++ b/packages/opencode/src/skill/skill.ts @@ -48,7 +48,7 @@ export namespace Skill { const addSkill = async (match: string) => { const md = await ConfigMarkdown.parse(match).catch((err) => { const message = ConfigMarkdown.FrontmatterError.isInstance(err) - ? `${err.data.path}: ${err.data.message}` + ? err.data.message : `Failed to parse skill ${match}` Bus.publish(Session.Event.Error, { error: new NamedError.Unknown({ message }).toObject() }) log.error("failed to load skill", { skill: match, err }) diff --git a/packages/opencode/test/config/fixtures/empty-frontmatter.md b/packages/opencode/test/config/fixtures/empty-frontmatter.md new file mode 100644 index 00000000000..95d5a80ed1c --- /dev/null +++ b/packages/opencode/test/config/fixtures/empty-frontmatter.md @@ -0,0 +1,4 @@ +--- +--- + +Content diff --git a/packages/opencode/test/config/fixtures/frontmatter.md b/packages/opencode/test/config/fixtures/frontmatter.md new file mode 100644 index 00000000000..27822d6218f --- /dev/null +++ b/packages/opencode/test/config/fixtures/frontmatter.md @@ -0,0 +1,28 @@ +--- +description: "This is a description wrapped in quotes" +# field: this is a commented out field that should be ignored +occupation: This man has the following occupation: Software Engineer +title: 'Hello World' +name: John "Doe" + +family: He has no 'family' +summary: > + This is a summary +url: https://example.com:8080/path?query=value +time: The time is 12:30:00 PM +nested: First: Second: Third: Fourth +quoted_colon: "Already quoted: no change needed" +single_quoted_colon: 'Single quoted: also fine' +mixed: He said "hello: world" and then left +empty: +dollar: Use $' and $& for special patterns +--- + +Content that should not be parsed: + +fake_field: this is not yaml +another: neither is this +time: 10:30:00 AM +url: https://should-not-be-parsed.com:3000 + +The above lines look like YAML but are just content. diff --git a/packages/opencode/test/config/fixtures/no-frontmatter.md b/packages/opencode/test/config/fixtures/no-frontmatter.md new file mode 100644 index 00000000000..39c9f3681a2 --- /dev/null +++ b/packages/opencode/test/config/fixtures/no-frontmatter.md @@ -0,0 +1 @@ +Content diff --git a/packages/opencode/test/config/markdown.test.ts b/packages/opencode/test/config/markdown.test.ts index 90a997d1f91..b4263ee6b5c 100644 --- a/packages/opencode/test/config/markdown.test.ts +++ b/packages/opencode/test/config/markdown.test.ts @@ -1,89 +1,192 @@ -import { expect, test } from "bun:test" +import { expect, test, describe } from "bun:test" import { ConfigMarkdown } from "../../src/config/markdown" -const template = `This is a @valid/path/to/a/file and it should also match at -the beginning of a line: +describe("ConfigMarkdown: normal template", () => { + const template = `This is a @valid/path/to/a/file and it should also match at + the beginning of a line: -@another-valid/path/to/a/file + @another-valid/path/to/a/file -but this is not: + but this is not: - - Adds a "Co-authored-by:" footer which clarifies which AI agent - helped create this commit, using an appropriate \`noreply@...\` - or \`noreply@anthropic.com\` email address. + - Adds a "Co-authored-by:" footer which clarifies which AI agent + helped create this commit, using an appropriate \`noreply@...\` + or \`noreply@anthropic.com\` email address. -We also need to deal with files followed by @commas, ones -with @file-extensions.md, even @multiple.extensions.bak, -hidden directories like @.config/ or files like @.bashrc -and ones at the end of a sentence like @foo.md. + We also need to deal with files followed by @commas, ones + with @file-extensions.md, even @multiple.extensions.bak, + hidden directories like @.config/ or files like @.bashrc + and ones at the end of a sentence like @foo.md. -Also shouldn't forget @/absolute/paths.txt with and @/without/extensions, -as well as @~/home-files and @~/paths/under/home.txt. + Also shouldn't forget @/absolute/paths.txt with and @/without/extensions, + as well as @~/home-files and @~/paths/under/home.txt. -If the reference is \`@quoted/in/backticks\` then it shouldn't match at all.` + If the reference is \`@quoted/in/backticks\` then it shouldn't match at all.` -const matches = ConfigMarkdown.files(template) + const matches = ConfigMarkdown.files(template) -test("should extract exactly 12 file references", () => { - expect(matches.length).toBe(12) -}) + test("should extract exactly 12 file references", () => { + expect(matches.length).toBe(12) + }) -test("should extract valid/path/to/a/file", () => { - expect(matches[0][1]).toBe("valid/path/to/a/file") -}) + test("should extract valid/path/to/a/file", () => { + expect(matches[0][1]).toBe("valid/path/to/a/file") + }) -test("should extract another-valid/path/to/a/file", () => { - expect(matches[1][1]).toBe("another-valid/path/to/a/file") -}) + test("should extract another-valid/path/to/a/file", () => { + expect(matches[1][1]).toBe("another-valid/path/to/a/file") + }) -test("should extract paths ignoring comma after", () => { - expect(matches[2][1]).toBe("commas") -}) + test("should extract paths ignoring comma after", () => { + expect(matches[2][1]).toBe("commas") + }) -test("should extract a path with a file extension and comma after", () => { - expect(matches[3][1]).toBe("file-extensions.md") -}) + test("should extract a path with a file extension and comma after", () => { + expect(matches[3][1]).toBe("file-extensions.md") + }) -test("should extract a path with multiple dots and comma after", () => { - expect(matches[4][1]).toBe("multiple.extensions.bak") -}) + test("should extract a path with multiple dots and comma after", () => { + expect(matches[4][1]).toBe("multiple.extensions.bak") + }) -test("should extract hidden directory", () => { - expect(matches[5][1]).toBe(".config/") -}) + test("should extract hidden directory", () => { + expect(matches[5][1]).toBe(".config/") + }) -test("should extract hidden file", () => { - expect(matches[6][1]).toBe(".bashrc") -}) + test("should extract hidden file", () => { + expect(matches[6][1]).toBe(".bashrc") + }) -test("should extract a file ignoring period at end of sentence", () => { - expect(matches[7][1]).toBe("foo.md") -}) + test("should extract a file ignoring period at end of sentence", () => { + expect(matches[7][1]).toBe("foo.md") + }) -test("should extract an absolute path with an extension", () => { - expect(matches[8][1]).toBe("/absolute/paths.txt") -}) + test("should extract an absolute path with an extension", () => { + expect(matches[8][1]).toBe("/absolute/paths.txt") + }) -test("should extract an absolute path without an extension", () => { - expect(matches[9][1]).toBe("/without/extensions") -}) + test("should extract an absolute path without an extension", () => { + expect(matches[9][1]).toBe("/without/extensions") + }) + + test("should extract an absolute path in home directory", () => { + expect(matches[10][1]).toBe("~/home-files") + }) -test("should extract an absolute path in home directory", () => { - expect(matches[10][1]).toBe("~/home-files") + test("should extract an absolute path under home directory", () => { + expect(matches[11][1]).toBe("~/paths/under/home.txt") + }) + + test("should not match when preceded by backtick", () => { + const backtickTest = "This `@should/not/match` should be ignored" + const backtickMatches = ConfigMarkdown.files(backtickTest) + expect(backtickMatches.length).toBe(0) + }) + + test("should not match email addresses", () => { + const emailTest = "Contact user@example.com for help" + const emailMatches = ConfigMarkdown.files(emailTest) + expect(emailMatches.length).toBe(0) + }) }) -test("should extract an absolute path under home directory", () => { - expect(matches[11][1]).toBe("~/paths/under/home.txt") +describe("ConfigMarkdown: frontmatter parsing", async () => { + const parsed = await ConfigMarkdown.parse(import.meta.dir + "/fixtures/frontmatter.md") + + test("should parse without throwing", () => { + expect(parsed).toBeDefined() + expect(parsed.data).toBeDefined() + expect(parsed.content).toBeDefined() + }) + + test("should extract description field", () => { + expect(parsed.data.description).toBe("This is a description wrapped in quotes") + }) + + test("should extract occupation field with colon in value", () => { + expect(parsed.data.occupation).toBe("This man has the following occupation: Software Engineer\n") + }) + + test("should extract title field with single quotes", () => { + expect(parsed.data.title).toBe("Hello World") + }) + + test("should extract name field with embedded quotes", () => { + expect(parsed.data.name).toBe('John "Doe"') + }) + + test("should extract family field with embedded single quotes", () => { + expect(parsed.data.family).toBe("He has no 'family'") + }) + + test("should extract multiline summary field", () => { + expect(parsed.data.summary).toBe("This is a summary\n") + }) + + test("should not include commented fields in data", () => { + expect(parsed.data.field).toBeUndefined() + }) + + test("should extract URL with port", () => { + expect(parsed.data.url).toBe("https://example.com:8080/path?query=value\n") + }) + + test("should extract time with colons", () => { + expect(parsed.data.time).toBe("The time is 12:30:00 PM\n") + }) + + test("should extract value with multiple colons", () => { + expect(parsed.data.nested).toBe("First: Second: Third: Fourth\n") + }) + + test("should preserve already double-quoted values with colons", () => { + expect(parsed.data.quoted_colon).toBe("Already quoted: no change needed") + }) + + test("should preserve already single-quoted values with colons", () => { + expect(parsed.data.single_quoted_colon).toBe("Single quoted: also fine") + }) + + test("should extract value with quotes and colons mixed", () => { + expect(parsed.data.mixed).toBe('He said "hello: world" and then left\n') + }) + + test("should handle empty values", () => { + expect(parsed.data.empty).toBeNull() + }) + + test("should handle dollar sign replacement patterns literally", () => { + expect(parsed.data.dollar).toBe("Use $' and $& for special patterns") + }) + + test("should not parse fake yaml from content", () => { + expect(parsed.data.fake_field).toBeUndefined() + expect(parsed.data.another).toBeUndefined() + }) + + test("should extract content after frontmatter without modification", () => { + expect(parsed.content).toContain("Content that should not be parsed:") + expect(parsed.content).toContain("fake_field: this is not yaml") + expect(parsed.content).toContain("url: https://should-not-be-parsed.com:3000") + }) }) -test("should not match when preceded by backtick", () => { - const backtickTest = "This `@should/not/match` should be ignored" - const backtickMatches = ConfigMarkdown.files(backtickTest) - expect(backtickMatches.length).toBe(0) +describe("ConfigMarkdown: frontmatter parsing w/ empty frontmatter", async () => { + const result = await ConfigMarkdown.parse(import.meta.dir + "/fixtures/empty-frontmatter.md") + + test("should parse without throwing", () => { + expect(result).toBeDefined() + expect(result.data).toEqual({}) + expect(result.content.trim()).toBe("Content") + }) }) -test("should not match email addresses", () => { - const emailTest = "Contact user@example.com for help" - const emailMatches = ConfigMarkdown.files(emailTest) - expect(emailMatches.length).toBe(0) +describe("ConfigMarkdown: frontmatter parsing w/ no frontmatter", async () => { + const result = await ConfigMarkdown.parse(import.meta.dir + "/fixtures/no-frontmatter.md") + + test("should parse without throwing", () => { + expect(result).toBeDefined() + expect(result.data).toEqual({}) + expect(result.content.trim()).toBe("Content") + }) })