diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index c1f2d4c37..51df114ae 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -12,6 +12,7 @@ - Fixed idle prompt-cache warming rebuilding expired caches when its timer or an extension decision is delayed. - Improved crash diagnostics with hints identifying loaded extensions that appear in the stack trace. - Fixed text files beginning with `GIF` being misclassified as images and omitted from `read` and CLI `@file` input ([#9755](https://github.com/earendil-works/pi/issues/9755)). +- Fixed malformed prompt template frontmatter being silently ignored instead of reported as a resource warning ([#9830](https://github.com/earendil-works/pi/pull/9830)). ## [0.86.1] - 2026-09-20 diff --git a/packages/coding-agent/src/core/prompt-templates.ts b/packages/coding-agent/src/core/prompt-templates.ts index e0fe89e53..f5b21a665 100644 --- a/packages/coding-agent/src/core/prompt-templates.ts +++ b/packages/coding-agent/src/core/prompt-templates.ts @@ -3,6 +3,7 @@ import { basename, dirname, join, resolve, sep } from "path"; import { CONFIG_DIR_NAME } from "../config.ts"; import { parseFrontmatter } from "../utils/frontmatter.ts"; import { resolvePath } from "../utils/paths.ts"; +import type { ResourceDiagnostic } from "./diagnostics.ts"; import { createSyntheticSourceInfo, type SourceInfo } from "./source-info.ts"; /** @@ -101,45 +102,66 @@ export function substituteArgs(content: string, args: string[]): string { ); } -function loadTemplateFromFile(filePath: string, sourceInfo: SourceInfo): PromptTemplate | null { +function loadTemplateFromFile( + filePath: string, + sourceInfo: SourceInfo, +): { template: PromptTemplate | null; diagnostics: ResourceDiagnostic[] } { + const diagnostics: ResourceDiagnostic[] = []; + let rawContent: string; try { - const rawContent = readFileSync(filePath, "utf-8"); - const { frontmatter, body } = parseFrontmatter>(rawContent); + rawContent = readFileSync(filePath, "utf-8"); + } catch (error) { + const message = error instanceof Error ? error.message : "failed to read prompt template file"; + diagnostics.push({ type: "warning", message, path: filePath }); + return { template: null, diagnostics }; + } - const name = basename(filePath).replace(/\.md$/, ""); + let frontmatter: Record; + let body: string; + try { + ({ frontmatter, body } = parseFrontmatter(rawContent)); + } catch (error) { + const message = error instanceof Error ? error.message : "failed to parse prompt template file"; + diagnostics.push({ type: "warning", message, path: filePath }); + return { template: null, diagnostics }; + } - // Get description from frontmatter or first non-empty line - let description = frontmatter.description || ""; - if (!description) { - const firstLine = body.split("\n").find((line) => line.trim()); - if (firstLine) { - // Truncate if too long - description = firstLine.slice(0, 60); - if (firstLine.length > 60) description += "..."; - } + const name = basename(filePath).replace(/\.md$/, ""); + + // Get description from frontmatter or first non-empty line + let description = typeof frontmatter.description === "string" ? frontmatter.description : ""; + if (!description) { + const firstLine = body.split("\n").find((line) => line.trim()); + if (firstLine) { + // Truncate if too long + description = firstLine.slice(0, 60); + if (firstLine.length > 60) description += "..."; } + } - return { + const argumentHint = typeof frontmatter["argument-hint"] === "string" ? frontmatter["argument-hint"] : undefined; + return { + template: { name, description, - ...(frontmatter["argument-hint"] && { argumentHint: frontmatter["argument-hint"] }), + ...(argumentHint && { argumentHint }), content: body, sourceInfo, filePath, - }; - } catch { - return null; - } + }, + diagnostics, + }; } /** * Scan a directory for .md files (non-recursive) and load them as prompt templates. */ -function loadTemplatesFromDir(dir: string, getSourceInfo: (filePath: string) => SourceInfo): PromptTemplate[] { +function loadTemplatesFromDir(dir: string, getSourceInfo: (filePath: string) => SourceInfo): LoadPromptTemplatesResult { const templates: PromptTemplate[] = []; + const diagnostics: ResourceDiagnostic[] = []; if (!existsSync(dir)) { - return templates; + return { templates, diagnostics }; } try { @@ -161,17 +183,18 @@ function loadTemplatesFromDir(dir: string, getSourceInfo: (filePath: string) => } if (isFile && entry.name.endsWith(".md")) { - const template = loadTemplateFromFile(fullPath, getSourceInfo(fullPath)); - if (template) { - templates.push(template); + const result = loadTemplateFromFile(fullPath, getSourceInfo(fullPath)); + if (result.template) { + templates.push(result.template); } + diagnostics.push(...result.diagnostics); } } } catch { - return templates; + return { templates, diagnostics }; } - return templates; + return { templates, diagnostics }; } export interface LoadPromptTemplatesOptions { @@ -185,19 +208,29 @@ export interface LoadPromptTemplatesOptions { includeDefaults: boolean; } +export interface LoadPromptTemplatesResult { + templates: PromptTemplate[]; + diagnostics: ResourceDiagnostic[]; +} + /** * Load all prompt templates from: * 1. Global: agentDir/prompts/ * 2. Project: cwd/{CONFIG_DIR_NAME}/prompts/ * 3. Explicit prompt paths */ -export function loadPromptTemplates(options: LoadPromptTemplatesOptions): PromptTemplate[] { +export function loadPromptTemplates(options: LoadPromptTemplatesOptions): LoadPromptTemplatesResult { const resolvedCwd = resolvePath(options.cwd); const resolvedAgentDir = resolvePath(options.agentDir); const promptPaths = options.promptPaths; const includeDefaults = options.includeDefaults; const templates: PromptTemplate[] = []; + const diagnostics: ResourceDiagnostic[] = []; + const addResult = (result: LoadPromptTemplatesResult): void => { + templates.push(...result.templates); + diagnostics.push(...result.diagnostics); + }; const globalPromptsDir = join(resolvedAgentDir, "prompts"); const projectPromptsDir = resolve(resolvedCwd, CONFIG_DIR_NAME, "prompts"); @@ -233,8 +266,8 @@ export function loadPromptTemplates(options: LoadPromptTemplatesOptions): Prompt }; if (includeDefaults) { - templates.push(...loadTemplatesFromDir(globalPromptsDir, getSourceInfo)); - templates.push(...loadTemplatesFromDir(projectPromptsDir, getSourceInfo)); + addResult(loadTemplatesFromDir(globalPromptsDir, getSourceInfo)); + addResult(loadTemplatesFromDir(projectPromptsDir, getSourceInfo)); } // 3. Load explicit prompt paths @@ -247,19 +280,21 @@ export function loadPromptTemplates(options: LoadPromptTemplatesOptions): Prompt try { const stats = statSync(resolvedPath); if (stats.isDirectory()) { - templates.push(...loadTemplatesFromDir(resolvedPath, getSourceInfo)); + addResult(loadTemplatesFromDir(resolvedPath, getSourceInfo)); } else if (stats.isFile() && resolvedPath.endsWith(".md")) { - const template = loadTemplateFromFile(resolvedPath, getSourceInfo(resolvedPath)); - if (template) { - templates.push(template); + const result = loadTemplateFromFile(resolvedPath, getSourceInfo(resolvedPath)); + if (result.template) { + templates.push(result.template); } + diagnostics.push(...result.diagnostics); } - } catch { - // Ignore read failures + } catch (error) { + const message = error instanceof Error ? error.message : "failed to read prompt template path"; + diagnostics.push({ type: "warning", message, path: resolvedPath }); } } - return templates; + return { templates, diagnostics }; } /** diff --git a/packages/coding-agent/src/core/resource-loader.ts b/packages/coding-agent/src/core/resource-loader.ts index 04102425b..6babd0178 100644 --- a/packages/coding-agent/src/core/resource-loader.ts +++ b/packages/coding-agent/src/core/resource-loader.ts @@ -697,13 +697,17 @@ export class DefaultResourceLoader implements ResourceLoader { if (this.noPromptTemplates && promptPaths.length === 0) { promptsResult = { prompts: [], diagnostics: [] }; } else { - const allPrompts = loadPromptTemplates({ + const loaded = loadPromptTemplates({ cwd: this.cwd, agentDir: this.agentDir, promptPaths, includeDefaults: false, }); - promptsResult = this.dedupePrompts(allPrompts); + const deduped = this.dedupePrompts(loaded.templates); + promptsResult = { + prompts: deduped.prompts, + diagnostics: [...loaded.diagnostics, ...deduped.diagnostics], + }; } const resolvedPrompts = this.promptsOverride ? this.promptsOverride(promptsResult) : promptsResult; this.prompts = resolvedPrompts.prompts.map((prompt) => ({ diff --git a/packages/coding-agent/test/prompt-templates.test.ts b/packages/coding-agent/test/prompt-templates.test.ts index 97bfe985c..eaf59c12a 100644 --- a/packages/coding-agent/test/prompt-templates.test.ts +++ b/packages/coding-agent/test/prompt-templates.test.ts @@ -8,7 +8,7 @@ * - Edge cases and integration between parsing and substitution */ -import { mkdirSync, rmSync, writeFileSync } from "fs"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "fs"; import { tmpdir } from "os"; import { join } from "path"; import { afterAll, describe, expect, test } from "vitest"; @@ -515,7 +515,7 @@ argument-hint: "" You are given one or more GitHub PR URLs: $@`, ); - const templates = loadPromptTemplates({ + const { templates } = loadPromptTemplates({ cwd: process.cwd(), agentDir: getAgentDir(), promptPaths: [testDir], @@ -538,7 +538,7 @@ argument-hint: "[instructions]" Wrap it. Additional instructions: $ARGUMENTS`, ); - const templates = loadPromptTemplates({ + const { templates } = loadPromptTemplates({ cwd: process.cwd(), agentDir: getAgentDir(), promptPaths: [testDir], @@ -560,7 +560,7 @@ description: Audit changelog entries before release Audit changelog entries for all commits since the last release.`, ); - const templates = loadPromptTemplates({ + const { templates } = loadPromptTemplates({ cwd: process.cwd(), agentDir: getAgentDir(), promptPaths: [testDir], @@ -582,7 +582,7 @@ argument-hint: "" Do something`, ); - const templates = loadPromptTemplates({ + const { templates } = loadPromptTemplates({ cwd: process.cwd(), agentDir: getAgentDir(), promptPaths: [testDir], @@ -604,7 +604,7 @@ argument-hint: "" Analyze GitHub issue(s): $ARGUMENTS`, ); - const templates = loadPromptTemplates({ + const { templates } = loadPromptTemplates({ cwd: process.cwd(), agentDir: getAgentDir(), promptPaths: [testDir], @@ -622,3 +622,33 @@ Analyze GitHub issue(s): $ARGUMENTS`, } catch {} }); }); + +describe("loadPromptTemplates - diagnostics", () => { + // Regression test for #9354. + test("reports invalid YAML frontmatter and keeps valid siblings", () => { + const testDir = mkdtempSync(join(tmpdir(), "pi-test-prompts-invalid-")); + const invalidPromptPath = join(testDir, "invalid.md"); + try { + writeFileSync(invalidPromptPath, "---\ndescription: Broken: unquoted colon\n---\nDo something.\n"); + writeFileSync(join(testDir, "valid.md"), "Valid prompt content."); + + const { templates, diagnostics } = loadPromptTemplates({ + cwd: process.cwd(), + agentDir: getAgentDir(), + promptPaths: [testDir], + includeDefaults: false, + }); + + expect(templates.map((template) => template.name)).toEqual(["valid"]); + expect(diagnostics).toEqual([ + expect.objectContaining({ + type: "warning", + path: invalidPromptPath, + message: expect.stringContaining("line 1, column 14"), + }), + ]); + } finally { + rmSync(testDir, { recursive: true, force: true }); + } + }); +}); diff --git a/packages/coding-agent/test/resource-loader.test.ts b/packages/coding-agent/test/resource-loader.test.ts index 145eca3b2..435bd5e22 100644 --- a/packages/coding-agent/test/resource-loader.test.ts +++ b/packages/coding-agent/test/resource-loader.test.ts @@ -98,6 +98,28 @@ Prompt content.`, expect(prompts.some((p) => p.name === "test-prompt")).toBe(true); }); + // Regression test for #9354. + it("should report invalid prompt frontmatter while loading valid siblings", async () => { + const promptsDir = join(agentDir, "prompts"); + const invalidPromptPath = join(promptsDir, "invalid.md"); + mkdirSync(promptsDir, { recursive: true }); + writeFileSync(invalidPromptPath, "---\ndescription: Broken: unquoted colon\n---\nDo something.\n"); + writeFileSync(join(promptsDir, "valid.md"), "Valid prompt content."); + + const loader = new DefaultResourceLoader({ cwd, agentDir }); + await loader.reload(); + + const { prompts, diagnostics } = loader.getPrompts(); + expect(prompts.map((prompt) => prompt.name)).toEqual(["valid"]); + expect(diagnostics).toEqual([ + expect.objectContaining({ + type: "warning", + path: invalidPromptPath, + message: expect.stringContaining("line 1, column 14"), + }), + ]); + }); + it("should prefer project resources over user on name collisions", async () => { const userPromptsDir = join(agentDir, "prompts"); const projectPromptsDir = join(cwd, ".pi", "prompts");