forked from farhoodlabs/paperclip
fix(skills): atomic deleteBySource + PAT secret cleanup on skill deletion
- Pre-check all skills for agent usage before deleting any in deleteBySource to prevent partial/failed deletions - Delete (rotate to empty) the skill-pat:<skillId> secret when a skill is deleted to prevent orphaned PAT secrets Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -2397,6 +2397,17 @@ export function companySkillService(db: Db) {
|
|||||||
// Clean up materialized runtime files
|
// Clean up materialized runtime files
|
||||||
await fs.rm(resolveRuntimeSkillMaterializedPath(companyId, skill), { recursive: true, force: true });
|
await fs.rm(resolveRuntimeSkillMaterializedPath(companyId, skill), { recursive: true, force: true });
|
||||||
|
|
||||||
|
// Delete associated PAT secret if present
|
||||||
|
const meta = skill.metadata as Record<string, unknown> | null;
|
||||||
|
const secretId = typeof meta?.sourceAuthSecretId === "string" ? meta.sourceAuthSecretId : null;
|
||||||
|
if (secretId) {
|
||||||
|
try {
|
||||||
|
await secretsSvc.remove(secretId);
|
||||||
|
} catch {
|
||||||
|
// Best-effort: don't fail the skill deletion if secret cleanup fails
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
return skill;
|
return skill;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -2454,6 +2465,28 @@ export function companySkillService(db: Db) {
|
|||||||
.where(and(eq(companySkills.companyId, companyId), eq(companySkills.sourceLocator, sourceLocator)));
|
.where(and(eq(companySkills.companyId, companyId), eq(companySkills.sourceLocator, sourceLocator)));
|
||||||
if (rows.length === 0) return [];
|
if (rows.length === 0) return [];
|
||||||
|
|
||||||
|
// Pre-check all skills for agent usage before deleting any (atomicity)
|
||||||
|
const skills = rows.map(toCompanySkill);
|
||||||
|
for (const skill of skills) {
|
||||||
|
const usedByAgents = await usage(companyId, skill.key);
|
||||||
|
if (usedByAgents.length > 0) {
|
||||||
|
const agentNames = usedByAgents.map((agent) => agent.name).sort((left, right) => left.localeCompare(right));
|
||||||
|
throw unprocessable(
|
||||||
|
`Cannot delete skills from "${sourceLocator}" because skill "${skill.name}" is still used by ${agentNames.join(", ")}. Detach it from those agents first.`,
|
||||||
|
{
|
||||||
|
skillId: skill.id,
|
||||||
|
skillKey: skill.key,
|
||||||
|
usedByAgents: usedByAgents.map((agent) => ({
|
||||||
|
id: agent.id,
|
||||||
|
name: agent.name,
|
||||||
|
urlKey: agent.urlKey,
|
||||||
|
adapterType: agent.adapterType,
|
||||||
|
})),
|
||||||
|
},
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
const deleted: CompanySkill[] = [];
|
const deleted: CompanySkill[] = [];
|
||||||
for (const row of rows) {
|
for (const row of rows) {
|
||||||
const result = await deleteSkill(companyId, row.id);
|
const result = await deleteSkill(companyId, row.id);
|
||||||
|
|||||||
Reference in New Issue
Block a user