From 05fbf7eb783617945f28e2ddfeb765cc49c73d5c Mon Sep 17 00:00:00 2001 From: Aiden Cline Date: Thu, 15 Jan 2026 12:27:48 -0600 Subject: [PATCH 1/3] test: add new tests --- .../opencode/test/config/markdown.test.ts | 183 ++++++++++++------ 1 file changed, 120 insertions(+), 63 deletions(-) diff --git a/packages/opencode/test/config/markdown.test.ts b/packages/opencode/test/config/markdown.test.ts index 90a997d1f91..330ad57ff1a 100644 --- a/packages/opencode/test/config/markdown.test.ts +++ b/packages/opencode/test/config/markdown.test.ts @@ -1,89 +1,146 @@ -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 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 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 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", async () => { + const template = `--- +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 +--- + +Content +` + + const matter = await import("gray-matter") + const parsed = matter.default(template) + + 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 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() + expect(parsed.data.occupation).toBeUndefined() + }) + + test("should extract content after frontmatter", () => { + expect(parsed.content.trim()).toBe("Content") + }) }) From 8cb0f199eeedb9b575fa47e77b4784d496d12f96 Mon Sep 17 00:00:00 2001 From: Aiden Cline Date: Thu, 15 Jan 2026 12:46:30 -0600 Subject: [PATCH 2/3] fix: ensure markdown processor can handle the colons that arent technically valid yaml --- packages/opencode/src/config/markdown.ts | 54 +++++++++++++- .../opencode/test/config/markdown.test.ts | 70 +++++++++++++++++-- 2 files changed, 117 insertions(+), 7 deletions(-) diff --git a/packages/opencode/src/config/markdown.ts b/packages/opencode/src/config/markdown.ts index f20842c41a9..1ec809586a7 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) diff --git a/packages/opencode/test/config/markdown.test.ts b/packages/opencode/test/config/markdown.test.ts index 330ad57ff1a..1d2e353489e 100644 --- a/packages/opencode/test/config/markdown.test.ts +++ b/packages/opencode/test/config/markdown.test.ts @@ -94,20 +94,36 @@ describe("ConfigMarkdown: frontmatter parsing", async () => { const template = `--- 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 +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 +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. ` const matter = await import("gray-matter") - const parsed = matter.default(template) + const preprocessed = ConfigMarkdown.preprocessFrontmatter(template) + const parsed = matter.default(preprocessed) test("should parse without throwing", () => { expect(parsed).toBeDefined() @@ -119,6 +135,10 @@ Content 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") }) @@ -137,10 +157,48 @@ Content test("should not include commented fields in data", () => { expect(parsed.data.field).toBeUndefined() - expect(parsed.data.occupation).toBeUndefined() }) - test("should extract content after frontmatter", () => { - expect(parsed.content.trim()).toBe("Content") + 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") }) }) From 83fba42e2fb0f7158dfd4460442ac6b9c99c24ab Mon Sep 17 00:00:00 2001 From: Aiden Cline Date: Thu, 15 Jan 2026 13:04:56 -0600 Subject: [PATCH 3/3] tweak tests and cleanup code --- packages/opencode/src/cli/error.ts | 2 +- packages/opencode/src/config/config.ts | 6 +-- packages/opencode/src/config/markdown.ts | 2 +- packages/opencode/src/skill/skill.ts | 2 +- .../test/config/fixtures/empty-frontmatter.md | 4 ++ .../test/config/fixtures/frontmatter.md | 28 ++++++++++ .../test/config/fixtures/no-frontmatter.md | 1 + .../opencode/test/config/markdown.test.ts | 54 ++++++++----------- 8 files changed, 60 insertions(+), 39 deletions(-) create mode 100644 packages/opencode/test/config/fixtures/empty-frontmatter.md create mode 100644 packages/opencode/test/config/fixtures/frontmatter.md create mode 100644 packages/opencode/test/config/fixtures/no-frontmatter.md 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 1ec809586a7..d1eeeac3821 100644 --- a/packages/opencode/src/config/markdown.ts +++ b/packages/opencode/src/config/markdown.ts @@ -76,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 1d2e353489e..b4263ee6b5c 100644 --- a/packages/opencode/test/config/markdown.test.ts +++ b/packages/opencode/test/config/markdown.test.ts @@ -91,39 +91,7 @@ describe("ConfigMarkdown: normal template", () => { }) describe("ConfigMarkdown: frontmatter parsing", async () => { - const template = `--- -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. -` - - const matter = await import("gray-matter") - const preprocessed = ConfigMarkdown.preprocessFrontmatter(template) - const parsed = matter.default(preprocessed) + const parsed = await ConfigMarkdown.parse(import.meta.dir + "/fixtures/frontmatter.md") test("should parse without throwing", () => { expect(parsed).toBeDefined() @@ -202,3 +170,23 @@ The above lines look like YAML but are just content. expect(parsed.content).toContain("url: https://should-not-be-parsed.com:3000") }) }) + +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") + }) +}) + +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") + }) +})