From 3ea3020a76d70d90cdafdd5e15fa2baa02fa31ca Mon Sep 17 00:00:00 2001 From: Chris Farhood Date: Sun, 3 May 2026 11:02:16 -0400 Subject: [PATCH] fix(secrets): include skill metadata references in usages, route, and delete dialog secretsSvc.usages() previously only scanned agent env bindings. Skills can reference a secret via metadata.sourceAuthSecretId (set by the PAT auth feature), so removing a secret without checking those references could orphan a sibling skill's PAT pointer. - Extend usages() to return { agents, skills } with both reference kinds. - Update remove() to block when either array is non-empty. - /secrets/:id/usages now responds with { agents, skills }. - The delete dialog displays both kinds inline so the operator knows which references to detach first. --- server/src/routes/secrets.ts | 4 ++-- server/src/services/secrets.ts | 42 +++++++++++++++++++++++++-------- ui/src/api/secrets.ts | 5 +++- ui/src/pages/CompanySecrets.tsx | 38 ++++++++++++++++++++--------- 4 files changed, 65 insertions(+), 24 deletions(-) diff --git a/server/src/routes/secrets.ts b/server/src/routes/secrets.ts index c7748079..862851fe 100644 --- a/server/src/routes/secrets.ts +++ b/server/src/routes/secrets.ts @@ -141,8 +141,8 @@ export function secretRoutes(db: Db) { return; } assertCompanyAccess(req, existing.companyId); - const agents = await svc.usages(existing.companyId, id); - res.json({ agents }); + const used = await svc.usages(existing.companyId, id); + res.json({ agents: used.agents, skills: used.skills }); }); router.delete("/secrets/:id", async (req, res) => { diff --git a/server/src/services/secrets.ts b/server/src/services/secrets.ts index 77155de7..0b69b262 100644 --- a/server/src/services/secrets.ts +++ b/server/src/services/secrets.ts @@ -1,6 +1,6 @@ -import { and, desc, eq } from "drizzle-orm"; +import { and, desc, eq, sql } from "drizzle-orm"; import type { Db } from "@paperclipai/db"; -import { companySecrets, companySecretVersions } from "@paperclipai/db"; +import { companySecrets, companySecretVersions, companySkills } from "@paperclipai/db"; import type { AgentEnvConfig, EnvBinding, SecretProvider } from "@paperclipai/shared"; import { envBindingSchema } from "@paperclipai/shared"; import { conflict, notFound, unprocessable } from "../errors.js"; @@ -46,6 +46,8 @@ export function secretService(db: Db) { }; type SecretUsageAgent = { id: string; name: string; envKeys: string[] }; + type SecretUsageSkill = { id: string; name: string; slug: string }; + type SecretUsages = { agents: SecretUsageAgent[]; skills: SecretUsageSkill[] }; async function getById(id: string) { return db @@ -55,10 +57,10 @@ export function secretService(db: Db) { .then((rows) => rows[0] ?? null); } - async function usages(companyId: string, secretId: string): Promise { + async function usages(companyId: string, secretId: string): Promise { const agents = agentService(db); const allAgents = await agents.list(companyId); - const out: SecretUsageAgent[] = []; + const agentRefs: SecretUsageAgent[] = []; for (const agent of allAgents) { const config = asRecord(agent.adapterConfig); if (!config) continue; @@ -73,10 +75,28 @@ export function secretService(db: Db) { } } if (matchingKeys.length > 0) { - out.push({ id: agent.id, name: agent.name, envKeys: matchingKeys }); + agentRefs.push({ id: agent.id, name: agent.name, envKeys: matchingKeys }); } } - return out; + + const skillRows = await db + .select({ + id: companySkills.id, + name: companySkills.name, + slug: companySkills.slug, + }) + .from(companySkills) + .where(and( + eq(companySkills.companyId, companyId), + sql`${companySkills.metadata} ->> 'sourceAuthSecretId' = ${secretId}`, + )); + const skillRefs: SecretUsageSkill[] = skillRows.map((row) => ({ + id: row.id, + name: row.name, + slug: row.slug, + })); + + return { agents: agentRefs, skills: skillRefs }; } async function getByName(companyId: string, name: string) { @@ -315,11 +335,13 @@ export function secretService(db: Db) { const secret = await getById(secretId); if (!secret) return null; const used = await usages(secret.companyId, secretId); - if (used.length > 0) { - const names = used.map((agent) => agent.name).sort((left, right) => left.localeCompare(right)); + if (used.agents.length > 0 || used.skills.length > 0) { + const agentNames = used.agents.map((agent) => agent.name); + const skillNames = used.skills.map((skill) => skill.name); + const names = [...agentNames, ...skillNames].sort((left, right) => left.localeCompare(right)); throw unprocessable( - `Cannot delete secret "${secret.name}" while it is still used by ${names.join(", ")}. Detach it from those agents first.`, - { secretId, usedByAgents: used }, + `Cannot delete secret "${secret.name}" while it is still used by ${names.join(", ")}. Detach it from those references first.`, + { secretId, usedByAgents: used.agents, usedBySkills: used.skills }, ); } await db.delete(companySecrets).where(eq(companySecrets.id, secretId)); diff --git a/ui/src/api/secrets.ts b/ui/src/api/secrets.ts index 397bfd5f..ba30328e 100644 --- a/ui/src/api/secrets.ts +++ b/ui/src/api/secrets.ts @@ -23,5 +23,8 @@ export const secretsApi = { ) => api.patch(`/secrets/${id}`, data), remove: (id: string) => api.delete<{ ok: true }>(`/secrets/${id}`), usages: (id: string) => - api.get<{ agents: { id: string; name: string; envKeys: string[] }[] }>(`/secrets/${id}/usages`), + api.get<{ + agents: { id: string; name: string; envKeys: string[] }[]; + skills: { id: string; name: string; slug: string }[]; + }>(`/secrets/${id}/usages`), }; diff --git a/ui/src/pages/CompanySecrets.tsx b/ui/src/pages/CompanySecrets.tsx index 56ec4354..65834276 100644 --- a/ui/src/pages/CompanySecrets.tsx +++ b/ui/src/pages/CompanySecrets.tsx @@ -421,7 +421,9 @@ function EditSecretDialog({ ); } -type DeleteUsage = { id: string; name: string; envKeys: string[] }; +type DeleteUsageAgent = { id: string; name: string; envKeys: string[] }; +type DeleteUsageSkill = { id: string; name: string; slug: string }; +type DeleteBlock = { agents: DeleteUsageAgent[]; skills: DeleteUsageSkill[] }; function DeleteSecretDialog({ secret, @@ -432,7 +434,7 @@ function DeleteSecretDialog({ onClose: () => void; onDeleted: () => void; }) { - const [blockedBy, setBlockedBy] = useState(null); + const [blockedBy, setBlockedBy] = useState(null); const [errorMessage, setErrorMessage] = useState(null); const remove = useMutation({ @@ -441,12 +443,16 @@ function DeleteSecretDialog({ onError: (error: unknown) => { if (error instanceof ApiError) { const body = error.body as - | { details?: { usedByAgents?: DeleteUsage[] } } + | { details?: { usedByAgents?: DeleteUsageAgent[]; usedBySkills?: DeleteUsageSkill[] } } | null | undefined; - const usedByAgents = body?.details?.usedByAgents; - if (Array.isArray(usedByAgents) && usedByAgents.length > 0) { - setBlockedBy(usedByAgents); + const usedByAgents = body?.details?.usedByAgents ?? []; + const usedBySkills = body?.details?.usedBySkills ?? []; + if ( + (Array.isArray(usedByAgents) && usedByAgents.length > 0) + || (Array.isArray(usedBySkills) && usedBySkills.length > 0) + ) { + setBlockedBy({ agents: usedByAgents, skills: usedBySkills }); setErrorMessage(error.message); return; } @@ -457,6 +463,8 @@ function DeleteSecretDialog({ }, }); + const totalBlockers = blockedBy ? blockedBy.agents.length + blockedBy.skills.length : 0; + return ( !open && onClose()}> @@ -464,25 +472,33 @@ function DeleteSecretDialog({ Delete "{secret.name}" This permanently deletes the secret and all of its versions. Agents - that reference it will fail until the binding is removed. + and skills that reference it will fail until the binding is removed. - {blockedBy && blockedBy.length > 0 ? ( + {blockedBy && totalBlockers > 0 ? (

Cannot delete — still referenced by:

    - {blockedBy.map((agent) => ( -
  • + {blockedBy.agents.map((agent) => ( +
  • {agent.name}{" "} + agent{" "} ({agent.envKeys.join(", ")})
    • ))} + {blockedBy.skills.map((skill) => ( +
  • + {skill.name}{" "} + skill{" "} + ({skill.slug}) +
    • + ))}

- Detach this secret from those agents' environment variables first. + Detach this secret from those references first.

) : errorMessage ? (