From 79b464bf9db85bc50846fd7d63494b5d243be821 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Tue, 25 Aug 2026 13:51:57 -0700 Subject: [PATCH] fix(server): surface skill materialization failures instead of dropping the skill (#12146) 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 > - Runtime skill listing materializes each company skill's files before handing them to the agent's adapter > - A materialization failure was swallowed with catch-to-null, and the skill silently vanished from the runtime while the library still showed it installed > - Operators saw "installed", agents saw nothing, and nobody saw the cause; on claude-local a missing desired skill could even crash the prompt-bundle hasher > - This pull request turns both failure paths into structured "missing" entries with the real error and makes every adapter skip unmountable entries explicitly > - The benefit is that a broken skill shows up as broken, with its cause, instead of not existing ## Linked Issues or Issue Description **What happened?** A company skill whose runtime files fail to materialize (deleted source, missing stored SKILL.md copy, failed version snapshot) disappears from `listRuntimeSkillEntries` with no trace. Agent skill snapshots report a generic "not available" with no cause. On claude-local, a desired skill whose source path does not exist reaches the prompt-bundle hasher, whose `fs.lstat` throws and can fail the whole run. **Expected behavior** The skill appears with `sourceStatus: "missing"` and a `missingDetail` carrying the underlying error, snapshots and the UI show it as broken, and adapters skip it at mount time with a logged warning instead of crashing or dangling-symlinking. **Steps to reproduce** Install a local-path skill referenced by an agent, delete its source directory contents so the stored SKILL.md copy cannot be recovered, and start a run: before this change the skill vanishes from the runtime set silently; on claude-local a pinned-but-unmaterializable version can fail bundle preparation. ## What Changed - `server/src/services/company-skills.ts` `resolveRuntimeSkillSource`: both `.catch(() => null)` sites (version snapshot, runtime materialization) now return the structured `{status: "missing", source, detail}` shape the deliberate missing branch already used, with the underlying error message in `detail`. - `packages/adapter-utils/src/server-utils.ts`: `isPaperclipSkillSourceMissing` is exported with a doc comment. - `packages/adapters/claude-local/src/server/execute.ts`: missing desired skills are filtered out of the prompt bundle and each one logs a `[paperclip] Warning` with its detail to the run output. - `cursor-local`, `gemini-local`, `kimi-local`, `opencode-local`, `pi-local` `execute.ts`: mount loops (and the cursor/gemini injection calls) skip missing entries instead of symlinking a nonexistent path. ## Verification - `cd server && npx vitest run src/__tests__/company-skills-service.test.ts` — new test pins the missing-with-cause entry for a failed materialization. Nine pre-existing project-workspace tests in this file fail on my machine at clean `master` too (environment-specific); their count is unchanged by this PR. - `cd server && npx vitest run src/__tests__/heartbeat-runtime-skills.test.ts src/__tests__/claude-local-skill-sync.test.ts src/__tests__/cursor-local-skill-sync.test.ts src/__tests__/cursor-local-skill-injection.test.ts src/__tests__/gemini-local-skill-sync.test.ts` — 12 tests pass. - `cd packages/adapters/claude-local && npx vitest run` — 244 passed, 1 skipped. - `pnpm run typecheck` clean in server, adapter-utils, and all six touched adapters. ## Risks - Runtime skill entry lists grow by the previously dropped entries (now flagged missing). All shipped consumers either intersect with desired sets, already handle `sourceStatus: "missing"`, or now skip missing entries at mount time. The snapshot layer already understood the missing shape via the `materializeMissing: false` path, so downstream contracts are unchanged. ## Model Used - Claude Fable 5 (`claude-fable-5`, Anthropic) with extended thinking and tool use, via Claude Code. ## 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 - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --- .../adapter-utils/src/acpx-engine/execute.ts | 8 ++- packages/adapter-utils/src/server-utils.ts | 8 ++- .../claude-local/src/server/execute.ts | 15 ++++- .../codex-local/src/server/execute.ts | 10 +++- .../cursor-local/src/server/execute.ts | 6 +- .../gemini-local/src/server/execute.ts | 8 ++- .../adapters/kimi-local/src/server/execute.ts | 2 + .../opencode-local/src/server/execute.ts | 2 + .../adapters/pi-local/src/server/execute.ts | 2 + .../__tests__/company-skills-service.test.ts | 60 +++++++++++++++++++ server/src/services/company-skills.ts | 39 ++++++++++-- 11 files changed, 148 insertions(+), 12 deletions(-) diff --git a/packages/adapter-utils/src/acpx-engine/execute.ts b/packages/adapter-utils/src/acpx-engine/execute.ts index 9210025c4d..37c15eb9af 100644 --- a/packages/adapter-utils/src/acpx-engine/execute.ts +++ b/packages/adapter-utils/src/acpx-engine/execute.ts @@ -50,6 +50,7 @@ import { joinPromptSections, materializePaperclipSkillCopy, parseObject, + isPaperclipSkillSourceMissing, readPaperclipRuntimeSkillEntries, readPaperclipIssueWorkModeFromContext, renderPaperclipWakePrompt, @@ -880,7 +881,12 @@ async function resolveSelectedRuntimeSkills( const desiredSet = new Set(desiredSkillNames); return { allSkills, - selectedSkills: allSkills.filter((entry) => desiredSet.has(entry.key)), + // Missing-source entries never mount: buildSkillSetKey hashes each + // selected entry's path contents, and a nonexistent source would abort + // runtime construction over one broken skill. + selectedSkills: allSkills.filter( + (entry) => desiredSet.has(entry.key) && !isPaperclipSkillSourceMissing(entry), + ), desiredSkillNames, }; } diff --git a/packages/adapter-utils/src/server-utils.ts b/packages/adapter-utils/src/server-utils.ts index a013ada663..48d229fb47 100644 --- a/packages/adapter-utils/src/server-utils.ts +++ b/packages/adapter-utils/src/server-utils.ts @@ -322,7 +322,13 @@ function buildManagedSkillOrigin(): Pick< }; } -function isPaperclipSkillSourceMissing(entry: PaperclipSkillEntry) { +/** + * True when a runtime skill entry's files are unavailable (failed + * materialization, deleted version snapshot). Adapters must skip these at + * mount time: their `source` path does not exist, so symlinking produces a + * dangling link and content hashing throws. + */ +export function isPaperclipSkillSourceMissing(entry: PaperclipSkillEntry) { return entry.sourceStatus === "missing"; } diff --git a/packages/adapters/claude-local/src/server/execute.ts b/packages/adapters/claude-local/src/server/execute.ts index 7d2ca4bdb2..5eb6fae993 100644 --- a/packages/adapters/claude-local/src/server/execute.ts +++ b/packages/adapters/claude-local/src/server/execute.ts @@ -32,6 +32,7 @@ import { parseJson, applyPaperclipWorkspaceEnv, buildPaperclipEnv, + isPaperclipSkillSourceMissing, readPaperclipRuntimeSkillEntries, readPaperclipIssueWorkModeFromContext, joinPromptSections, @@ -506,9 +507,21 @@ export async function execute(ctx: AdapterExecutionContext): Promise desiredSkillNames.has(entry.key)); + const mountableSkillEntries = desiredSkillEntries.filter((entry) => !isPaperclipSkillSourceMissing(entry)); + for (const entry of desiredSkillEntries) { + if (!isPaperclipSkillSourceMissing(entry)) continue; + await onLog( + "stderr", + `[paperclip] Warning: skill "${entry.key}" is enabled for this agent but its files are unavailable and it was not mounted${entry.missingDetail ? `: ${entry.missingDetail}` : "."}\n`, + ); + } const promptBundle = await prepareClaudePromptBundle({ companyId: agent.companyId, - skills: claudeSkillEntries.filter((entry) => desiredSkillNames.has(entry.key)), + skills: mountableSkillEntries, instructionsContents: combinedInstructionsContents, onLog, }); diff --git a/packages/adapters/codex-local/src/server/execute.ts b/packages/adapters/codex-local/src/server/execute.ts index aa1e958729..48fa6705d1 100644 --- a/packages/adapters/codex-local/src/server/execute.ts +++ b/packages/adapters/codex-local/src/server/execute.ts @@ -40,6 +40,7 @@ import { ensurePaperclipSkillSymlink, ensurePathInEnv, refreshPaperclipWorkspaceEnvForExecution, + isPaperclipSkillSourceMissing, readPaperclipRuntimeSkillEntries, readPaperclipIssueWorkModeFromContext, resolvePaperclipDesiredSkillNames, @@ -502,7 +503,10 @@ export async function ensureCodexSkillsInjected( onLog: AdapterExecutionContext["onLog"], options: EnsureCodexSkillsInjectedOptions = {}, ) { - const allSkillsEntries = options.skillsEntries ?? await readPaperclipRuntimeSkillEntries({}, __moduleDir); + const allSkillsEntries = options.skillsEntries + ?? (await readPaperclipRuntimeSkillEntries({}, __moduleDir)).filter( + (entry) => !isPaperclipSkillSourceMissing(entry), + ); const desiredSkillNames = options.desiredSkillNames ?? allSkillsEntries.map((entry) => entry.key); const desiredSet = new Set(desiredSkillNames); @@ -633,7 +637,9 @@ export async function execute(ctx: AdapterExecutionContext): Promise 0 ? path.resolve(envConfig.CODEX_HOME.trim()) : null; - const codexSkillEntries = await readPaperclipRuntimeSkillEntries(config, __moduleDir); + const codexSkillEntries = (await readPaperclipRuntimeSkillEntries(config, __moduleDir)) + // A missing-source entry would become a dangling skill symlink; skip it. + .filter((entry) => !isPaperclipSkillSourceMissing(entry)); const desiredSkillNames = resolveCodexDesiredSkillNames(config, codexSkillEntries); if (!executionTargetIsRemote) { await ensureAbsoluteDirectory(cwd, { createIfMissing: true }); diff --git a/packages/adapters/cursor-local/src/server/execute.ts b/packages/adapters/cursor-local/src/server/execute.ts index 62dc9cbc0a..30f23950e8 100644 --- a/packages/adapters/cursor-local/src/server/execute.ts +++ b/packages/adapters/cursor-local/src/server/execute.ts @@ -36,6 +36,7 @@ import { ensurePaperclipSkillSymlink, ensurePathInEnv, refreshPaperclipWorkspaceEnvForExecution, + isPaperclipSkillSourceMissing, readPaperclipRuntimeSkillEntries, readPaperclipIssueWorkModeFromContext, resolvePaperclipDesiredSkillNames, @@ -128,6 +129,7 @@ async function buildCursorSkillsDir(config: Record): Promise desiredCursorSkillNames.includes(entry.key)), + skillsEntries: cursorSkillEntries.filter( + (entry) => desiredCursorSkillNames.includes(entry.key) && !isPaperclipSkillSourceMissing(entry), + ), }); } diff --git a/packages/adapters/gemini-local/src/server/execute.ts b/packages/adapters/gemini-local/src/server/execute.ts index 9591adc357..ce01307b96 100644 --- a/packages/adapters/gemini-local/src/server/execute.ts +++ b/packages/adapters/gemini-local/src/server/execute.ts @@ -38,6 +38,7 @@ import { joinPromptSections, ensurePathInEnv, refreshPaperclipWorkspaceEnvForExecution, + isPaperclipSkillSourceMissing, readPaperclipRuntimeSkillEntries, readPaperclipIssueWorkModeFromContext, resolvePaperclipDesiredSkillNames, @@ -199,6 +200,7 @@ async function buildGeminiSkillsDir( const desiredNames = new Set(resolvePaperclipDesiredSkillNames(config, availableEntries)); for (const entry of availableEntries) { if (!desiredNames.has(entry.key)) continue; + if (isPaperclipSkillSourceMissing(entry)) continue; await fs.symlink(entry.source, path.join(target, entry.runtimeName)); } return target; @@ -258,7 +260,11 @@ export async function execute(ctx: AdapterExecutionContext): Promise !isPaperclipSkillSourceMissing(entry)), + desiredGeminiSkillNames, + ); } const envConfig = parseObject(config.env); diff --git a/packages/adapters/kimi-local/src/server/execute.ts b/packages/adapters/kimi-local/src/server/execute.ts index a14a4fb7e1..4ec307bbca 100644 --- a/packages/adapters/kimi-local/src/server/execute.ts +++ b/packages/adapters/kimi-local/src/server/execute.ts @@ -31,6 +31,7 @@ import { joinPromptSections, ensurePathInEnv, refreshPaperclipWorkspaceEnvForExecution, + isPaperclipSkillSourceMissing, readPaperclipRuntimeSkillEntries, readPaperclipIssueWorkModeFromContext, resolvePaperclipDesiredSkillNames, @@ -176,6 +177,7 @@ async function buildKimiSkillsDir( const desiredNames = new Set(resolvePaperclipDesiredSkillNames(config, availableEntries)); for (const entry of availableEntries) { if (!desiredNames.has(entry.key)) continue; + if (isPaperclipSkillSourceMissing(entry)) continue; await fs.symlink(entry.source, path.join(target, entry.runtimeName)); } return target; diff --git a/packages/adapters/opencode-local/src/server/execute.ts b/packages/adapters/opencode-local/src/server/execute.ts index 4283f57d27..df3467511b 100644 --- a/packages/adapters/opencode-local/src/server/execute.ts +++ b/packages/adapters/opencode-local/src/server/execute.ts @@ -43,6 +43,7 @@ import { stringifyPaperclipWakePayload, DEFAULT_PAPERCLIP_AGENT_PROMPT_TEMPLATE, runChildProcess, + isPaperclipSkillSourceMissing, readPaperclipRuntimeSkillEntries, readPaperclipIssueWorkModeFromContext, resolvePaperclipDesiredSkillNames, @@ -214,6 +215,7 @@ async function buildOpenCodeSkillsDir(config: Record): Promise< const desiredNames = new Set(resolvePaperclipDesiredSkillNames(config, availableEntries)); for (const entry of availableEntries) { if (!desiredNames.has(entry.key)) continue; + if (isPaperclipSkillSourceMissing(entry)) continue; await fs.symlink(entry.source, path.join(target, entry.runtimeName)); } return target; diff --git a/packages/adapters/pi-local/src/server/execute.ts b/packages/adapters/pi-local/src/server/execute.ts index 97fab8b36a..9d3e50a35b 100644 --- a/packages/adapters/pi-local/src/server/execute.ts +++ b/packages/adapters/pi-local/src/server/execute.ts @@ -37,6 +37,7 @@ import { ensurePaperclipSkillSymlink, ensurePathInEnv, refreshPaperclipWorkspaceEnvForExecution, + isPaperclipSkillSourceMissing, readPaperclipRuntimeSkillEntries, readPaperclipIssueWorkModeFromContext, resolvePaperclipDesiredSkillNames, @@ -129,6 +130,7 @@ async function buildPiSkillsDir(config: Record): Promise { ); }); + it("surfaces a failed runtime materialization as a missing entry instead of dropping the skill", async () => { + const companyId = randomUUID(); + const skillId = randomUUID(); + const skillKey = `company/${companyId}/broken-coach`; + const missingSkillDir = path.join(await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-broken-skill-")), "gone"); + cleanupDirs.add(path.dirname(missingSkillDir)); + + await db.insert(companies).values({ + id: companyId, + name: "Paperclip", + issuePrefix: `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`, + requireBoardApprovalForNewAgents: false, + }); + // The inventory lists no SKILL.md and the on-disk source is gone, so the + // runtime materializer has no SKILL.md to write and throws. The entry must + // still appear, flagged missing with the real cause, so snapshots and the + // UI can show the skill as broken instead of silently dropping it. + await db.insert(companySkills).values({ + id: skillId, + companyId, + key: skillKey, + slug: "broken-coach", + name: "Broken Coach", + description: null, + markdown: "# Broken Coach\n", + sourceType: "local_path", + sourceLocator: missingSkillDir, + trustLevel: "markdown_only", + compatibility: "compatible", + fileInventory: [{ path: "notes.md", kind: "reference" }], + metadata: { sourceKind: "local_path" }, + }); + // An agent must reference the skill: inventory reconciliation deletes + // unused local-path skills whose source directory is gone, and this test + // is about the used-but-unmaterializable path. + await db.insert(agents).values({ + id: randomUUID(), + companyId, + name: "Runner", + role: "engineer", + status: "active", + adapterType: "codex_local", + adapterConfig: { + paperclipSkillSync: { + desiredSkills: [skillKey], + }, + }, + }); + + const entries = await svc.listRuntimeSkillEntries(companyId); + const entry = entries.find((candidate) => candidate.key === skillKey); + + expect(entry).toMatchObject({ + key: skillKey, + sourceStatus: "missing", + missingDetail: expect.stringContaining("Failed to materialize skill files"), + }); + expect(entry!.missingDetail).toContain("stored SKILL.md copy is missing"); + }); + it("falls back to stored markdown when reading SKILL.md from a missing local source", async () => { const companyId = randomUUID(); const skillId = randomUUID(); diff --git a/server/src/services/company-skills.ts b/server/src/services/company-skills.ts index cfa474d477..283943af83 100644 --- a/server/src/services/company-skills.ts +++ b/server/src/services/company-skills.ts @@ -5775,16 +5775,29 @@ export function companySkillService(db: Db) { ): Promise { const selectedVersionId = options.versionSelections?.get(skill.key) ?? null; if (selectedVersionId) { + const versionPath = path.resolve(resolveManagedSkillsRoot(companyId), "__versions__", skill.id, selectedVersionId); const version = await getVersion(companyId, skill.id, selectedVersionId); if (!version) { return { status: "missing", - source: path.resolve(resolveManagedSkillsRoot(companyId), "__versions__", skill.id, selectedVersionId), + source: versionPath, detail: "The selected skill version no longer exists.", }; } - const versionSource = await materializeVersionSnapshot(companyId, skill, version).catch(() => null); - return versionSource ? { status: "available", source: versionSource } : null; + // A failed snapshot materialization must surface as a "missing" entry + // with the real cause — a silent drop makes the skill vanish from the + // runtime while the library still shows it installed. + try { + const versionSource = await materializeVersionSnapshot(companyId, skill, version); + if (versionSource) return { status: "available", source: versionSource }; + return { status: "missing", source: versionPath, detail: "The selected skill version produced no files." }; + } catch (error) { + return { + status: "missing", + source: versionPath, + detail: `Failed to materialize the selected skill version: ${error instanceof Error ? error.message : String(error)}`, + }; + } } const source = await resolveExistingSkillDirectory(normalizeSkillDirectory(skill)); @@ -5801,8 +5814,24 @@ export function companySkillService(db: Db) { }; } - const materializedSource = await materializeRuntimeSkillFiles(companyId, skill).catch(() => null); - return materializedSource ? { status: "available", source: materializedSource } : null; + // Same contract as above: a materialization failure becomes a structured + // "missing" resolution carrying the underlying error, so snapshots and the + // UI can show the skill as broken instead of pretending it does not exist. + try { + const materializedSource = await materializeRuntimeSkillFiles(companyId, skill); + if (materializedSource) return { status: "available", source: materializedSource }; + return { + status: "missing", + source: resolveRuntimeSkillMaterializedPath(companyId, skill), + detail: buildMissingRuntimeSourceDetail(skill), + }; + } catch (error) { + return { + status: "missing", + source: resolveRuntimeSkillMaterializedPath(companyId, skill), + detail: `Failed to materialize skill files: ${error instanceof Error ? error.message : String(error)}`, + }; + } } async function listRuntimeSkillEntries(