mirror of
https://github.com/badlogic/pi-mono.git
synced 2026-11-05 23:37:25 +00:00
fix(coding-agent): report invalid prompt frontmatter (#9830)
Fixes #9354 Co-authored-by: Geet Khosla <geetkhosla@gmail.com>
This commit is contained in:
parent
1e0fe20497
commit
b6419322e6
5 changed files with 136 additions and 44 deletions
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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<Record<string, string>>(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<string, unknown>;
|
||||
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 };
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -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) => ({
|
||||
|
|
|
|||
|
|
@ -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: "<PR-URL>"
|
|||
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: "<issue>"
|
|||
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 });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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");
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue