fix(server): surface skill materialization failures instead of dropping the skill (#12146)
## 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
This commit is contained in:
parent
18b6c788d5
commit
79b464bf9d
|
|
@ -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,
|
||||
};
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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";
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -32,6 +32,7 @@ import {
|
|||
parseJson,
|
||||
applyPaperclipWorkspaceEnv,
|
||||
buildPaperclipEnv,
|
||||
isPaperclipSkillSourceMissing,
|
||||
readPaperclipRuntimeSkillEntries,
|
||||
readPaperclipIssueWorkModeFromContext,
|
||||
joinPromptSections,
|
||||
|
|
@ -506,9 +507,21 @@ export async function execute(ctx: AdapterExecutionContext): Promise<AdapterExec
|
|||
);
|
||||
}
|
||||
}
|
||||
// Missing-source entries must never reach the bundle: their path does not
|
||||
// exist, so the bundle hasher would throw and fail the whole run over one
|
||||
// broken skill. Log each one instead so the cause lands in the run output.
|
||||
const desiredSkillEntries = claudeSkillEntries.filter((entry) => 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,
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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<AdapterExec
|
|||
typeof envConfig.CODEX_HOME === "string" && envConfig.CODEX_HOME.trim().length > 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 });
|
||||
|
|
|
|||
|
|
@ -36,6 +36,7 @@ import {
|
|||
ensurePaperclipSkillSymlink,
|
||||
ensurePathInEnv,
|
||||
refreshPaperclipWorkspaceEnvForExecution,
|
||||
isPaperclipSkillSourceMissing,
|
||||
readPaperclipRuntimeSkillEntries,
|
||||
readPaperclipIssueWorkModeFromContext,
|
||||
resolvePaperclipDesiredSkillNames,
|
||||
|
|
@ -128,6 +129,7 @@ async function buildCursorSkillsDir(config: Record<string, unknown>): Promise<st
|
|||
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;
|
||||
|
|
@ -234,7 +236,9 @@ export async function execute(ctx: AdapterExecutionContext): Promise<AdapterExec
|
|||
const desiredCursorSkillNames = resolvePaperclipDesiredSkillNames(config, cursorSkillEntries);
|
||||
if (!executionTargetIsRemote) {
|
||||
await ensureCursorSkillsInjected(onLog, {
|
||||
skillsEntries: cursorSkillEntries.filter((entry) => desiredCursorSkillNames.includes(entry.key)),
|
||||
skillsEntries: cursorSkillEntries.filter(
|
||||
(entry) => desiredCursorSkillNames.includes(entry.key) && !isPaperclipSkillSourceMissing(entry),
|
||||
),
|
||||
});
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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<AdapterExec
|
|||
const geminiSkillEntries = await readPaperclipRuntimeSkillEntries(config, __moduleDir);
|
||||
const desiredGeminiSkillNames = resolvePaperclipDesiredSkillNames(config, geminiSkillEntries);
|
||||
if (!executionTargetIsRemote) {
|
||||
await ensureGeminiSkillsInjected(onLog, geminiSkillEntries, desiredGeminiSkillNames);
|
||||
await ensureGeminiSkillsInjected(
|
||||
onLog,
|
||||
geminiSkillEntries.filter((entry) => !isPaperclipSkillSourceMissing(entry)),
|
||||
desiredGeminiSkillNames,
|
||||
);
|
||||
}
|
||||
|
||||
const envConfig = parseObject(config.env);
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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<string, unknown>): 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;
|
||||
|
|
|
|||
|
|
@ -37,6 +37,7 @@ import {
|
|||
ensurePaperclipSkillSymlink,
|
||||
ensurePathInEnv,
|
||||
refreshPaperclipWorkspaceEnvForExecution,
|
||||
isPaperclipSkillSourceMissing,
|
||||
readPaperclipRuntimeSkillEntries,
|
||||
readPaperclipIssueWorkModeFromContext,
|
||||
resolvePaperclipDesiredSkillNames,
|
||||
|
|
@ -129,6 +130,7 @@ async function buildPiSkillsDir(config: Record<string, unknown>): Promise<string
|
|||
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;
|
||||
|
|
|
|||
|
|
@ -1912,6 +1912,66 @@ describeEmbeddedPostgres("companySkillService.list", () => {
|
|||
);
|
||||
});
|
||||
|
||||
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();
|
||||
|
|
|
|||
|
|
@ -5775,16 +5775,29 @@ export function companySkillService(db: Db) {
|
|||
): Promise<RuntimeSkillSourceResolution | null> {
|
||||
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(
|
||||
|
|
|
|||
Loading…
Reference in New Issue