Avoid startup crash when Reflection Coach assets are missing (#9351)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The server has built-in agent definitions that are loaded during startup and used to provision optional operational agents such as Reflection Coach > - Reflection Coach stores richer stock instructions, a routine description, and a bundled skill as markdown assets outside the TypeScript module body > - A deployed server can fail before it is healthy if one of those copied markdown assets is absent from `server/dist` > - A recent build fix preserves those assets during normal server builds, but runtime should still degrade gracefully if a packaged asset is missing or unreadable > - This pull request adds resilient loading for built-in Reflection Coach assets and keeps a minimal compiled fallback available > - The benefit is that a missing optional built-in agent file no longer turns into a process-wide startup crash ## Linked Issues or Issue Description Bug fix. No public GitHub issue found for this exact startup crash. - Related public PR: #9339 - What happened: the server could throw `ENOENT` while importing the built-in agent service if `server/dist/built-ins/agents/reflection-coach/AGENTS.md` was missing from a deployed build. - Expected behavior: the server should keep starting, log that the built-in asset was missing, and use safe fallback text for the optional built-in agent resource. - Steps to reproduce: build the server, remove the compiled Reflection Coach `AGENTS.md` asset from `server/dist`, then import/start the server path that loads built-in agent definitions. - Paperclip version/commit: reproduced against a deployed build containing the Reflection Coach built-in agent assets; fixed against current `master` after #9339. - Deployment mode: Node server deployment using compiled `server/dist` output. ## What Changed - Added built-in agent text loading that checks the compiled asset path first, then source/package fallback paths, then a minimal compiled-in fallback string. - Added fallback text for Reflection Coach instructions, routine description, and bundled skill content so startup does not depend on optional markdown assets being present. - Added regression coverage for readable candidate selection and missing-file fallback behavior. ## Verification - `pnpm -w exec vitest run server/src/__tests__/built-in-agents.test.ts` — 1 test file passed, 24 tests passed. - `pnpm --filter @paperclipai/server build` — server TypeScript build completed and copied `src/built-ins` into `dist/built-ins`. - Manual smoke: temporarily moved `server/dist/built-ins/agents/reflection-coach/AGENTS.md`, imported `server/dist/services/built-in-agents.js` through the repo-pinned `tsx` runtime, and confirmed Reflection Coach definitions still loaded with output `reflection-coach:3732`; the asset was restored afterward. ## Risks Low risk. The normal path still uses the full packaged markdown assets. The fallback path is only used when those files are missing or unreadable, and it logs a warning so packaging drift remains visible. ## Model Used OpenAI GPT-5 Codex coding agent, with repository tool access and shell-based verification. Exact context window was not exposed in this runtime. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Dotta
GitHub
parent
cc81eefb60
commit
0f5f461729
|
|
@ -1,5 +1,8 @@
|
|||
import { randomUUID } from "node:crypto";
|
||||
import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest";
|
||||
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import path from "node:path";
|
||||
import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest";
|
||||
import { eq } from "drizzle-orm";
|
||||
import {
|
||||
activityLog,
|
||||
|
|
@ -32,6 +35,7 @@ import {
|
|||
builtInAgentService,
|
||||
deriveBuiltInAgentStatus,
|
||||
listBuiltInAgentDefinitions,
|
||||
readBuiltInTextWithFallback,
|
||||
reconcileBuiltInAgentsOnStartup,
|
||||
validateBuiltInAgentDefinitions,
|
||||
} from "../services/built-in-agents.ts";
|
||||
|
|
@ -51,6 +55,55 @@ if (!embeddedPostgresSupport.supported) {
|
|||
);
|
||||
}
|
||||
|
||||
describe("built-in agent asset loading", () => {
|
||||
it("uses the first readable candidate path", () => {
|
||||
const dir = mkdtempSync(path.join(tmpdir(), "paperclip-built-in-agent-"));
|
||||
try {
|
||||
const first = path.join(dir, "missing.md");
|
||||
const second = path.join(dir, "asset.md");
|
||||
writeFileSync(second, "asset text", "utf8");
|
||||
|
||||
expect(readBuiltInTextWithFallback(`asset:${randomUUID()}`, [first, second], "fallback text")).toBe("asset text");
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("falls back instead of throwing when built-in agent files are missing", () => {
|
||||
const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
const label = `missing:${randomUUID()}`;
|
||||
|
||||
try {
|
||||
expect(readBuiltInTextWithFallback(label, [path.join(tmpdir(), label, "AGENTS.md")], "fallback text")).toBe(
|
||||
"fallback text",
|
||||
);
|
||||
expect(warn).toHaveBeenCalledWith(expect.stringContaining(`Built-in agent asset ${label} was not readable`));
|
||||
} finally {
|
||||
warn.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it("warns about non-missing read errors before falling back", () => {
|
||||
const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
const dir = mkdtempSync(path.join(tmpdir(), "paperclip-built-in-agent-"));
|
||||
const label = "unreadable:" + randomUUID();
|
||||
|
||||
try {
|
||||
const directoryPath = path.join(dir, "asset.md");
|
||||
mkdirSync(directoryPath);
|
||||
|
||||
expect(readBuiltInTextWithFallback(label, [directoryPath], "fallback text")).toBe("fallback text");
|
||||
expect(warn).toHaveBeenCalledWith(expect.stringContaining("read error on " + directoryPath + ":"));
|
||||
expect(warn).toHaveBeenCalledWith(
|
||||
expect.stringContaining("Built-in agent asset " + label + " was not readable"),
|
||||
);
|
||||
} finally {
|
||||
warn.mockRestore();
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describeEmbeddedPostgres("built-in agents", () => {
|
||||
let db!: ReturnType<typeof createDb>;
|
||||
let tempDb: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>> | null = null;
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
import { createHash } from "node:crypto";
|
||||
import { readFileSync } from "node:fs";
|
||||
import { createRequire } from "node:module";
|
||||
import path from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import { readPaperclipSkillSyncPreference, writePaperclipSkillSyncPreference } from "@paperclipai/adapter-utils/server-utils";
|
||||
|
|
@ -138,20 +139,112 @@ export interface RequiredBuiltInAgent {
|
|||
|
||||
const BUILT_IN_AGENT_KEY_PATTERN = /^[a-z][a-z0-9_-]*$/;
|
||||
|
||||
const BUILT_INS_DIR = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../built-ins/agents");
|
||||
const moduleDir = path.dirname(fileURLToPath(import.meta.url));
|
||||
const require = createRequire(import.meta.url);
|
||||
const BUILT_INS_DIR = path.resolve(moduleDir, "../built-ins/agents");
|
||||
const SOURCE_BUILT_INS_DIR = path.resolve(moduleDir, "../../src/built-ins/agents");
|
||||
|
||||
function readBuiltInText(relativePath: string) {
|
||||
return readFileSync(path.join(BUILT_INS_DIR, relativePath), "utf8");
|
||||
const FALLBACK_REFLECTION_COACH_INSTRUCTIONS = [
|
||||
"# Reflection Coach",
|
||||
"",
|
||||
"You are Paperclip's built-in Reflection Coach.",
|
||||
"Review recent agent execution records, identify evidence-backed improvement patterns, and propose the smallest durable instruction, skill, or tool-description change.",
|
||||
"Do not apply changes in the same run. Present a reviewable diff and wait for the required Paperclip issue-thread approval before any follow-up applies it.",
|
||||
"",
|
||||
].join("\n");
|
||||
|
||||
const FALLBACK_REFLECTION_COACH_ROUTINE = [
|
||||
"Review recent agent work for coaching opportunities.",
|
||||
"",
|
||||
"Select recent target agents, inspect their work history and current instructions, then propose small, review-gated improvements that would prevent repeated misses.",
|
||||
"",
|
||||
].join("\n");
|
||||
|
||||
const FALLBACK_REFLECTION_COACH_SKILL = [
|
||||
"---",
|
||||
"name: reflection-coach",
|
||||
"description: Reflect on another agent's recent execution record and propose the smallest review-gated improvement.",
|
||||
"key: paperclipai/bundled/paperclip-operations/reflection-coach",
|
||||
"---",
|
||||
"",
|
||||
"# Reflection Coach",
|
||||
"",
|
||||
"Review another agent's recent execution record, name evidence-backed patterns, and propose the smallest durable improvement as a reviewable diff. Do not hot-swap instructions or skills in the same run.",
|
||||
"",
|
||||
].join("\n");
|
||||
|
||||
const warnedBuiltInTextFallbacks = new Set<string>();
|
||||
const warnedBuiltInTextReadErrors = new Set<string>();
|
||||
|
||||
function resolvePackageRoot(packageName: string) {
|
||||
try {
|
||||
return path.dirname(require.resolve(`${packageName}/package.json`));
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
const REFLECTION_COACH_INSTRUCTIONS = readBuiltInText("reflection-coach/AGENTS.md");
|
||||
const REFLECTION_COACH_ROUTINE = readBuiltInText("reflection-coach/routines/recent-agent-reflection.md");
|
||||
const REFLECTION_COACH_SKILL = readFileSync(
|
||||
path.resolve(
|
||||
path.dirname(fileURLToPath(import.meta.url)),
|
||||
"../../../packages/skills-catalog/catalog/bundled/paperclip-operations/reflection-coach/SKILL.md",
|
||||
),
|
||||
"utf8",
|
||||
export function readBuiltInTextWithFallback(
|
||||
label: string,
|
||||
candidatePaths: string[],
|
||||
fallbackText: string,
|
||||
) {
|
||||
const attemptedPaths = candidatePaths.filter((candidatePath) => candidatePath.trim().length > 0);
|
||||
for (const candidatePath of attemptedPaths) {
|
||||
try {
|
||||
return readFileSync(candidatePath, "utf8");
|
||||
} catch (error) {
|
||||
const code = (error as NodeJS.ErrnoException).code;
|
||||
if (code !== "ENOENT") {
|
||||
const codeLabel = code || String(error);
|
||||
const warningKey = [label, candidatePath, codeLabel].join(":");
|
||||
if (!warnedBuiltInTextReadErrors.has(warningKey)) {
|
||||
warnedBuiltInTextReadErrors.add(warningKey);
|
||||
console.warn(
|
||||
"[paperclip] Built-in agent asset " + label + " read error on " + candidatePath + ": " + codeLabel,
|
||||
);
|
||||
}
|
||||
}
|
||||
// Try every known runtime/source path before falling back to compiled text.
|
||||
}
|
||||
}
|
||||
|
||||
if (!warnedBuiltInTextFallbacks.has(label)) {
|
||||
warnedBuiltInTextFallbacks.add(label);
|
||||
console.warn(
|
||||
`[paperclip] Built-in agent asset ${label} was not readable; using bundled fallback text. `
|
||||
+ `Checked: ${attemptedPaths.join(", ")}`,
|
||||
);
|
||||
}
|
||||
return fallbackText;
|
||||
}
|
||||
|
||||
function readBuiltInText(relativePath: string, fallbackText: string) {
|
||||
return readBuiltInTextWithFallback(
|
||||
relativePath,
|
||||
[path.join(BUILT_INS_DIR, relativePath), path.join(SOURCE_BUILT_INS_DIR, relativePath)],
|
||||
fallbackText,
|
||||
);
|
||||
}
|
||||
|
||||
const skillsCatalogRoot = resolvePackageRoot("@paperclipai/skills-catalog");
|
||||
const REFLECTION_COACH_INSTRUCTIONS = readBuiltInText("reflection-coach/AGENTS.md", FALLBACK_REFLECTION_COACH_INSTRUCTIONS);
|
||||
const REFLECTION_COACH_ROUTINE = readBuiltInText(
|
||||
"reflection-coach/routines/recent-agent-reflection.md",
|
||||
FALLBACK_REFLECTION_COACH_ROUTINE,
|
||||
);
|
||||
const REFLECTION_COACH_SKILL = readBuiltInTextWithFallback(
|
||||
"reflection-coach/SKILL.md",
|
||||
[
|
||||
path.resolve(
|
||||
moduleDir,
|
||||
"../../../packages/skills-catalog/catalog/bundled/paperclip-operations/reflection-coach/SKILL.md",
|
||||
),
|
||||
...(skillsCatalogRoot
|
||||
? [path.join(skillsCatalogRoot, "catalog/bundled/paperclip-operations/reflection-coach/SKILL.md")]
|
||||
: []),
|
||||
],
|
||||
FALLBACK_REFLECTION_COACH_SKILL,
|
||||
);
|
||||
|
||||
const DEFINITIONS = validateBuiltInAgentDefinitions([
|
||||
|
|
|
|||
Loading…
Reference in New Issue