From c95c9301ae2ba2da8037c3fb55dd198f7cfa8ca0 Mon Sep 17 00:00:00 2001 From: Omid Mirzaei Date: Tue, 8 Sep 2026 13:19:38 +0400 Subject: [PATCH 1/2] fix: scope facility doctor agent checks to YAML frontmatter Doctor was matching name, model, triggers, and forbidden keys across the prompt body, so a valid prompt could fail and a wrong frontmatter name could hide behind a line in the prompt. --- packages/cli/src/doctor.mjs | 29 +++++++++++---- packages/cli/test/init.test.mjs | 63 +++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 6 deletions(-) diff --git a/packages/cli/src/doctor.mjs b/packages/cli/src/doctor.mjs index 891e7e41..431f3ba6 100644 --- a/packages/cli/src/doctor.mjs +++ b/packages/cli/src/doctor.mjs @@ -53,17 +53,34 @@ function checkAgent(dir, name) { const path = join(dir, relative); if (!existsSync(path)) return failed(relative, "missing"); const source = readFileSync(path, "utf8").replace(/\r\n?/g, "\n"); - if (!source.startsWith("---\n") || !/\n---\n[\s\S]*\S/.test(source)) return failed(relative, "invalid frontmatter or empty prompt"); - if (!new RegExp(`^name:\\s*${escapeRegExp(name)}\\s*$`, "m").test(source)) return failed(relative, `name must be ${name}`); - if (!/^engine:\s*(?:claude_code|codex)\s*$/m.test(source)) return failed(relative, "engine must be claude_code or codex"); - if (!/^model:\s*\S+\s*$/m.test(source)) return failed(relative, "model is missing"); - if (!/^triggers:\s*$/m.test(source) || !/^\s{2}- type:\s*(?:manual|schedule|github)\s*$/m.test(source)) { + const parsed = splitFrontmatter(source); + if (!parsed || !parsed.prompt.trim()) return failed(relative, "invalid frontmatter or empty prompt"); + const { frontmatter } = parsed; + if (!new RegExp(`^name:\\s*${escapeRegExp(name)}\\s*$`, "m").test(frontmatter)) { + return failed(relative, `name must be ${name}`); + } + if (!/^engine:\s*(?:claude_code|codex)\s*$/m.test(frontmatter)) { + return failed(relative, "engine must be claude_code or codex"); + } + if (!/^model:\s*\S/m.test(frontmatter)) return failed(relative, "model is missing"); + if ( + !/^triggers:\s*$/m.test(frontmatter) || + !/^\s{2}- type:\s*(?:manual|schedule|github|mcp|ui)\s*$/m.test(frontmatter) + ) { return failed(relative, "at least one supported trigger is required"); } - if (/^(?:permissions|sandbox|tools):/m.test(source)) return failed(relative, "per-agent access controls are not supported"); + if (/^(?:permissions|sandbox|tools):/m.test(frontmatter)) { + return failed(relative, "per-agent access controls are not supported"); + } return passed(relative, "valid agent manifest"); } +function splitFrontmatter(source) { + const match = /^---\n([\s\S]*?)\n---(?:\n|$)([\s\S]*)$/.exec(source); + if (!match) return null; + return { frontmatter: match[1] ?? "", prompt: match[2] ?? "" }; +} + function passed(label, detail) { return { label, ok: true, detail }; } diff --git a/packages/cli/test/init.test.mjs b/packages/cli/test/init.test.mjs index 8fc2c656..1fb8256d 100644 --- a/packages/cli/test/init.test.mjs +++ b/packages/cli/test/init.test.mjs @@ -198,6 +198,69 @@ test("local doctor validates the 0.12 contract and preserves its JSON output", ( assert.match(invalidPort.stdout, /between 1 and 65535/); }); +test("doctor inspects agent frontmatter only and accepts quoted models and mcp/ui triggers", (t) => { + const dir = makeTargetRepo(); + t.after(() => rmSync(dir, { recursive: true, force: true })); + const init = runCli( + ["init", "--yes", `--dir=${dir}`, "--repo=acme/demo-app", "--start=npm run dev"], + dir, + ); + assert.equal(init.status, 0, init.stdout + init.stderr); + + const builderPath = join(dir, ".agents/builder.md"); + const original = readFileSync(builderPath, "utf8"); + + writeFileSync( + builderPath, + `${original.trimEnd()}\n\npermissions:\n contents: read\n`, + ); + const promptPermissions = runCli(["doctor", `--dir=${dir}`, "--json"], dir); + assert.equal(promptPermissions.status, 0, promptPermissions.stdout + promptPermissions.stderr); + + writeFileSync( + builderPath, + original.replace(/^name: builder$/m, "name: not-builder") + "\nname: builder\n", + ); + const hiddenName = runCli(["doctor", `--dir=${dir}`, "--json"], dir); + assert.equal(hiddenName.status, 1); + assert.match(hiddenName.stdout, /name must be builder/); + + writeFileSync( + builderPath, + original.replace(/^model: .+$/m, 'model: "gpt-5.6-sol with spaces"'), + ); + const quotedModel = runCli(["doctor", `--dir=${dir}`, "--json"], dir); + assert.equal(quotedModel.status, 0, quotedModel.stdout + quotedModel.stderr); + + writeFileSync( + builderPath, + [ + "---", + "name: builder", + "description: Implements a complete story.", + "engine: codex", + "model: gpt-5.6-sol", + "enabled: true", + "triggers:", + " - type: mcp", + " - type: ui", + "---", + "", + "# Builder", + "", + "Complete the story.", + "", + ].join("\n"), + ); + const interactiveOnly = runCli(["doctor", `--dir=${dir}`, "--json"], dir); + assert.equal(interactiveOnly.status, 0, interactiveOnly.stdout + interactiveOnly.stderr); + + writeFileSync(builderPath, original.replace(/^enabled: true$/m, "permissions: {}\nenabled: true")); + const frontmatterPermissions = runCli(["doctor", `--dir=${dir}`, "--json"], dir); + assert.equal(frontmatterPermissions.status, 1); + assert.match(frontmatterPermissions.stdout, /per-agent access controls are not supported/); +}); + test("local commands reject unknown and valueless flags and legacy commands", () => { const unknown = runCli(["doctor", "--jsoon"]); assert.equal(unknown.status, 1); From 3d3677d2b1e43cb65d9ebcbab61f82ad27ed485d Mon Sep 17 00:00:00 2001 From: Omid Mirzaei Date: Tue, 15 Sep 2026 11:00:25 +0400 Subject: [PATCH 2/2] fix: reject comment-only and unclosed doctor model values The relaxed model check treated any non-whitespace after model: as valid, so a YAML comment or an unclosed quoted scalar passed locally while the server rejected both. --- packages/cli/src/doctor.mjs | 40 ++++++++++++++++++++++++++++++++- packages/cli/test/init.test.mjs | 10 +++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/doctor.mjs b/packages/cli/src/doctor.mjs index 431f3ba6..4287f6d6 100644 --- a/packages/cli/src/doctor.mjs +++ b/packages/cli/src/doctor.mjs @@ -62,7 +62,7 @@ function checkAgent(dir, name) { if (!/^engine:\s*(?:claude_code|codex)\s*$/m.test(frontmatter)) { return failed(relative, "engine must be claude_code or codex"); } - if (!/^model:\s*\S/m.test(frontmatter)) return failed(relative, "model is missing"); + if (!agentModel(frontmatter)) return failed(relative, "model is missing or invalid"); if ( !/^triggers:\s*$/m.test(frontmatter) || !/^\s{2}- type:\s*(?:manual|schedule|github|mcp|ui)\s*$/m.test(frontmatter) @@ -81,6 +81,44 @@ function splitFrontmatter(source) { return { frontmatter: match[1] ?? "", prompt: match[2] ?? "" }; } +const MODEL_MAX = 160; + +function agentModel(frontmatter) { + const match = /^model:\s*(.*)$/m.exec(frontmatter); + if (!match) return null; + const value = yamlStringScalar(match[1]); + if (value === null || value.length < 1 || value.length > MODEL_MAX) return null; + return value; +} + +function yamlStringScalar(raw) { + const source = raw.trim(); + if (!source || source.startsWith("#")) return null; + if (source.startsWith('"')) return yamlDoubleQuoted(source); + if (source.startsWith("'")) return yamlSingleQuoted(source); + if (source.startsWith("|") || source.startsWith(">")) return null; + const comment = /[\t ]#/.exec(source); + const plain = (comment ? source.slice(0, comment.index) : source).trim(); + return plain || null; +} + +function yamlDoubleQuoted(source) { + const match = /^("(?:\\.|[^"\\\n])*")\s*(?:#.*)?$/.exec(source); + if (!match) return null; + try { + const parsed = JSON.parse(match[1]); + return typeof parsed === "string" ? parsed : null; + } catch { + return null; + } +} + +function yamlSingleQuoted(source) { + const match = /^('(?:[^']|'')*')\s*(?:#.*)?$/.exec(source); + if (!match) return null; + return match[1].slice(1, -1).replaceAll("''", "'"); +} + function passed(label, detail) { return { label, ok: true, detail }; } diff --git a/packages/cli/test/init.test.mjs b/packages/cli/test/init.test.mjs index 1fb8256d..43916ac6 100644 --- a/packages/cli/test/init.test.mjs +++ b/packages/cli/test/init.test.mjs @@ -232,6 +232,16 @@ test("doctor inspects agent frontmatter only and accepts quoted models and mcp/u const quotedModel = runCli(["doctor", `--dir=${dir}`, "--json"], dir); assert.equal(quotedModel.status, 0, quotedModel.stdout + quotedModel.stderr); + writeFileSync(builderPath, original.replace(/^model: .+$/m, "model: # choose a model")); + const commentModel = runCli(["doctor", `--dir=${dir}`, "--json"], dir); + assert.equal(commentModel.status, 1); + assert.match(commentModel.stdout, /model is missing or invalid/); + + writeFileSync(builderPath, original.replace(/^model: .+$/m, 'model: "unclosed')); + const unclosedModel = runCli(["doctor", `--dir=${dir}`, "--json"], dir); + assert.equal(unclosedModel.status, 1); + assert.match(unclosedModel.stdout, /model is missing or invalid/); + writeFileSync( builderPath, [