From 0f5f4617296308ba75680a58af0c1ed89bb9ab7d Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Fri, 10 Jul 2026 08:28:37 -0500 Subject: [PATCH] Avoid startup crash when Reflection Coach assets are missing (#9351) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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 --- server/src/__tests__/built-in-agents.test.ts | 55 ++++++++- server/src/services/built-in-agents.ts | 115 +++++++++++++++++-- 2 files changed, 158 insertions(+), 12 deletions(-) diff --git a/server/src/__tests__/built-in-agents.test.ts b/server/src/__tests__/built-in-agents.test.ts index b42ef25822..3ee9ce13ce 100644 --- a/server/src/__tests__/built-in-agents.test.ts +++ b/server/src/__tests__/built-in-agents.test.ts @@ -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; let tempDb: Awaited> | null = null; diff --git a/server/src/services/built-in-agents.ts b/server/src/services/built-in-agents.ts index e7191f3817..ae3179457c 100644 --- a/server/src/services/built-in-agents.ts +++ b/server/src/services/built-in-agents.ts @@ -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(); +const warnedBuiltInTextReadErrors = new Set(); + +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([