diff --git a/packages/skills-catalog/catalog/bundled/paperclip-operations/reflection-coach/SKILL.md b/packages/skills-catalog/catalog/bundled/paperclip-operations/reflection-coach/SKILL.md index 2489b96ec9a..c6786939d70 100644 --- a/packages/skills-catalog/catalog/bundled/paperclip-operations/reflection-coach/SKILL.md +++ b/packages/skills-catalog/catalog/bundled/paperclip-operations/reflection-coach/SKILL.md @@ -164,13 +164,12 @@ From a reflection issue (assigned to the target's manager or the requester): Server-enforced mutation target keys: -- Agent instructions: `reflection-coach:agent-instructions:` -- Agent/tool description fields: `reflection-coach:agent-description:` -- Existing company skill: `reflection-coach:company-skill:` -- New local company skill by slug: `reflection-coach:company-skill-slug:` -- Imported skill source: `reflection-coach:company-skill-import:` -- Catalog skill install/update: `reflection-coach:company-skill-catalog:` -- Project workspace skill scan: `reflection-coach:company-skills:scan-projects` +- Agent instructions: `agent::instructions` +- Agent/tool description fields: `agent::profile` +- Existing company skill: `skill:` +- New local company skill by slug: `skill-slug:` +- Imported or catalog skill source: `skill-import:` +- Project workspace skill scan: `skills:scan-projects` ### 10) Apply only after acceptance, in a follow-up run diff --git a/packages/skills-catalog/generated/catalog.json b/packages/skills-catalog/generated/catalog.json index 24d6856ab18..791750aa2ac 100644 --- a/packages/skills-catalog/generated/catalog.json +++ b/packages/skills-catalog/generated/catalog.json @@ -2,7 +2,7 @@ "schemaVersion": 1, "packageName": "@paperclipai/skills-catalog", "packageVersion": "0.3.1", - "generatedAt": "2026-07-08T20:34:14.622Z", + "generatedAt": "2026-07-09T15:02:07.574Z", "skills": [ { "id": "paperclipai:bundled:docs:doc-maintenance", @@ -102,11 +102,11 @@ { "path": "SKILL.md", "kind": "skill", - "sizeBytes": 12120, - "sha256": "4050db7eb27559727c972673f419cd35335f082ee7f54ade636ada8ccd3e2e11" + "sizeBytes": 11903, + "sha256": "ca167eac8d1e89cadc8009b61a204368507e7edb7f2da0688fea6dba8223e189" } ], - "contentHash": "sha256:292fa8b1ce949cae972633fe6c283a628c87bce6f64d000d421b0a6644e84d4e" + "contentHash": "sha256:20381a898f05ceb668e305708dd33a03ce36aef0e00b923c01ae20ab785f04d2" }, { "id": "paperclipai:bundled:paperclip-operations:task-planning", diff --git a/server/src/__tests__/built-in-agents.test.ts b/server/src/__tests__/built-in-agents.test.ts index 88ca06ab3bd..6775477ac71 100644 --- a/server/src/__tests__/built-in-agents.test.ts +++ b/server/src/__tests__/built-in-agents.test.ts @@ -947,7 +947,7 @@ describeEmbeddedPostgres("built-in agents", () => { ].join("\n"), target: { type: "custom", - key: `reflection-coach:agent-instructions:${target.id}`, + key: `agent:${target.id}:instructions`, revisionId: "proposal-v1", label: "Target Coder AGENTS.md diff", }, @@ -1003,7 +1003,7 @@ describeEmbeddedPostgres("built-in agents", () => { ].join("\n"), target: { type: "custom", - key: `reflection-coach:agent-instructions:${target.id}`, + key: `agent:${target.id}:instructions`, revisionId: "proposal-v2", label: "Target Coder AGENTS.md rejected diff", }, diff --git a/server/src/__tests__/reflection-coach-mutation-gate.test.ts b/server/src/__tests__/change-consent-gate.test.ts similarity index 76% rename from server/src/__tests__/reflection-coach-mutation-gate.test.ts rename to server/src/__tests__/change-consent-gate.test.ts index 9c07fe512af..c5ba95b9441 100644 --- a/server/src/__tests__/reflection-coach-mutation-gate.test.ts +++ b/server/src/__tests__/change-consent-gate.test.ts @@ -13,14 +13,14 @@ import { startEmbeddedPostgresTestDatabase, } from "./helpers/embedded-postgres.js"; import { - reflectionCoachCompanySkillTargetKey, - reflectionCoachMutationGateService, -} from "../services/reflection-coach-mutation-gate.js"; + changeConsentGateService, + skillChangeTargetKey, +} from "../services/change-consent-gate.js"; const embeddedPostgresSupport = getEmbeddedPostgresTestSupport(); const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; -describeEmbeddedPostgres("reflectionCoachMutationGateService", () => { +describeEmbeddedPostgres("changeConsentGateService", () => { let db!: ReturnType; let tempDb: Awaited> | null = null; @@ -47,7 +47,7 @@ describeEmbeddedPostgres("reflectionCoachMutationGateService", () => { const sourceRunId = randomUUID(); const proposalIssueId = randomUUID(); const skillId = randomUUID(); - const targetKey = reflectionCoachCompanySkillTargetKey(skillId); + const targetKey = skillChangeTargetKey(skillId); await db.insert(companies).values({ id: companyId, @@ -64,12 +64,6 @@ describeEmbeddedPostgres("reflectionCoachMutationGateService", () => { adapterConfig: {}, runtimeConfig: {}, permissions: { canCreateSkills: true }, - metadata: { - paperclipBuiltInAgent: { - key: "reflection-coach", - featureKeys: ["reflection-coach"], - }, - }, }); await db.insert(heartbeatRuns).values({ id: sourceRunId, @@ -94,7 +88,7 @@ describeEmbeddedPostgres("reflectionCoachMutationGateService", () => { it("rejects Reflection Coach skill mutation without an accepted bound interaction", async () => { const { companyId, coachId, targetKey } = await seedGateFixture(); - await expect(reflectionCoachMutationGateService(db).assertAllowed({ + await expect(changeConsentGateService(db).assertConsented({ companyId, actorAgentId: coachId, actorRunId: randomUUID(), @@ -127,7 +121,7 @@ describeEmbeddedPostgres("reflectionCoachMutationGateService", () => { resolvedAt: new Date(), }); - await expect(reflectionCoachMutationGateService(db).assertAllowed({ + await expect(changeConsentGateService(db).assertConsented({ companyId, actorAgentId: coachId, actorRunId: sourceRunId, @@ -160,7 +154,37 @@ describeEmbeddedPostgres("reflectionCoachMutationGateService", () => { resolvedAt: new Date(), }); - await expect(reflectionCoachMutationGateService(db).assertAllowed({ + await expect(changeConsentGateService(db).assertConsented({ + companyId, + actorAgentId: coachId, + actorRunId: randomUUID(), + targetKeys: [targetKey], + })).resolves.toBe(true); + }); + + it("allows legacy Reflection Coach target keys for durable accepted interactions", async () => { + const { companyId, coachId, sourceRunId, proposalIssueId, skillId, targetKey } = await seedGateFixture(); + await db.insert(issueThreadInteractions).values({ + id: randomUUID(), + companyId, + issueId: proposalIssueId, + kind: "request_confirmation", + status: "accepted", + continuationPolicy: "wake_assignee_on_accept", + sourceRunId, + createdByAgentId: coachId, + payload: { + version: 1, + prompt: "Apply this Reflection Coach skill diff?", + detailsMarkdown: "```diff\n+Tighten the workflow.\n```", + target: { type: "custom", key: `reflection-coach:company-skill:${skillId}`, revisionId: "proposal-v1" }, + }, + result: { version: 1, outcome: "accepted" }, + resolvedByUserId: "board-user", + resolvedAt: new Date(), + }); + + await expect(changeConsentGateService(db).assertConsented({ companyId, actorAgentId: coachId, actorRunId: randomUUID(), diff --git a/server/src/__tests__/company-skills-routes.test.ts b/server/src/__tests__/company-skills-routes.test.ts index a9e198b9b1a..8f097a2d5df 100644 --- a/server/src/__tests__/company-skills-routes.test.ts +++ b/server/src/__tests__/company-skills-routes.test.ts @@ -75,7 +75,7 @@ const mockLogActivity = vi.hoisted(() => vi.fn()); const mockTrackSkillImported = vi.hoisted(() => vi.fn()); const mockGetTelemetryClient = vi.hoisted(() => vi.fn()); const mockReflectionCoachMutationGate = vi.hoisted(() => ({ - assertAllowed: vi.fn(), + assertConsented: vi.fn(), })); function registerModuleMocks() { @@ -108,13 +108,13 @@ function registerModuleMocks() { vi.doMock("../services/skills-catalog.js", () => mockCatalogService); - vi.doMock("../services/reflection-coach-mutation-gate.js", async () => { - const actual = await vi.importActual( - "../services/reflection-coach-mutation-gate.js", + vi.doMock("../services/change-consent-gate.js", async () => { + const actual = await vi.importActual( + "../services/change-consent-gate.js", ); return { ...actual, - reflectionCoachMutationGateService: () => mockReflectionCoachMutationGate, + changeConsentGateService: () => mockReflectionCoachMutationGate, }; }); @@ -154,7 +154,7 @@ describe("company skill mutation permissions", () => { vi.doUnmock("../services/agents.js"); vi.doUnmock("../services/company-skills.js"); vi.doUnmock("../services/skills-catalog.js"); - vi.doUnmock("../services/reflection-coach-mutation-gate.js"); + vi.doUnmock("../services/change-consent-gate.js"); vi.doUnmock("../services/index.js"); vi.doUnmock("../routes/company-skills.js"); vi.doUnmock("../routes/authz.js"); @@ -604,7 +604,7 @@ describe("company skill mutation permissions", () => { mockLogActivity.mockResolvedValue(undefined); mockAccessService.canUser.mockResolvedValue(true); mockAccessService.hasPermission.mockResolvedValue(false); - mockReflectionCoachMutationGate.assertAllowed.mockResolvedValue(false); + mockReflectionCoachMutationGate.assertConsented.mockResolvedValue(false); }); it("allows local board operators to mutate company skills", async () => { @@ -1175,7 +1175,7 @@ describe("company skill mutation permissions", () => { }, }, }); - mockReflectionCoachMutationGate.assertAllowed.mockRejectedValue(forbidden("gate required", { + mockReflectionCoachMutationGate.assertConsented.mockRejectedValue(forbidden("gate required", { code: "reflection_coach_mutation_gate_required", })); @@ -1189,11 +1189,11 @@ describe("company skill mutation permissions", () => { .send({ name: "Reflection Draft", slug: "reflection-draft", markdown: "# Draft" }); expect(res.status, JSON.stringify(res.body)).toBe(403); - expect(mockReflectionCoachMutationGate.assertAllowed).toHaveBeenCalledWith({ + expect(mockReflectionCoachMutationGate.assertConsented).toHaveBeenCalledWith({ companyId: "company-1", actorAgentId: "reflection-coach", actorRunId: "run-apply", - targetKeys: ["reflection-coach:company-skill-slug:reflection-draft"], + targetKeys: ["skill-slug:reflection-draft"], }); expect(mockCompanySkillService.createLocalSkill).not.toHaveBeenCalled(); }); diff --git a/server/src/built-ins/agents/reflection-coach/AGENTS.md b/server/src/built-ins/agents/reflection-coach/AGENTS.md index 6bbba77d36b..3aaf98954b0 100644 --- a/server/src/built-ins/agents/reflection-coach/AGENTS.md +++ b/server/src/built-ins/agents/reflection-coach/AGENTS.md @@ -31,13 +31,12 @@ You may be granted permission to create and update skills, update agent AGENTS.m Server-enforced target keys: -- `reflection-coach:agent-instructions:` -- `reflection-coach:agent-description:` -- `reflection-coach:company-skill:` -- `reflection-coach:company-skill-slug:` -- `reflection-coach:company-skill-import:` -- `reflection-coach:company-skill-catalog:` -- `reflection-coach:company-skills:scan-projects` +- `agent::instructions` +- `agent::profile` +- `skill:` +- `skill-slug:` +- `skill-import:` +- `skills:scan-projects` ## Execution contract diff --git a/server/src/built-ins/agents/reflection-coach/routines/recent-agent-reflection.md b/server/src/built-ins/agents/reflection-coach/routines/recent-agent-reflection.md index 97ab0957ac5..db73f36e2b5 100644 --- a/server/src/built-ins/agents/reflection-coach/routines/recent-agent-reflection.md +++ b/server/src/built-ins/agents/reflection-coach/routines/recent-agent-reflection.md @@ -66,7 +66,8 @@ This routine is **paused by default** and spends no tokens until an operator ena ## Hard limits for this routine - Proposal-only. This routine must not edit any agent's live AGENTS.md, skill assignments, or tool descriptions directly. -- Any actual instruction/skill/tool-description change requires a displayed diff and an **accepted** task interaction (`request_confirmation`, `request_checkbox_confirmation`, or `ask_user_questions`), applied only in a separate follow-up run. +- Any actual instruction/skill/tool-description change requires a displayed diff and an **accepted** `request_confirmation` task interaction, applied only in a separate follow-up run. +- Mutation confirmations must bind the exact resource key they will apply, using `agent::instructions`, `agent::profile`, `skill:`, `skill-slug:`, `skill-import:`, or `skills:scan-projects`. - Keep every read company-scoped. Do not cross company boundaries. - Every proposed rule needs linked issue/comment evidence or it is dropped. No scoring without trajectories. - Respect the size caps: AGENTS.md +20% max per proposal, skills 15KB max, tool descriptions 500 chars max. diff --git a/server/src/routes/agents.ts b/server/src/routes/agents.ts index a88d8c32e8d..08770367685 100644 --- a/server/src/routes/agents.ts +++ b/server/src/routes/agents.ts @@ -101,10 +101,11 @@ import { readObject } from "../lib/objects.js"; import { listInvalidOrgChainDescendantIds } from "../services/agent-invokability.js"; import { readBuiltInAgentMarker } from "../services/built-in-agent-metadata.js"; import { - reflectionCoachAgentDescriptionTargetKey, - reflectionCoachAgentInstructionsTargetKey, - reflectionCoachMutationGateService, -} from "../services/reflection-coach-mutation-gate.js"; + agentInstructionsChangeTargetKey, + agentProfileChangeTargetKey, + changeConsentGateService, + touchesAgentProfileChangeConsentFields, +} from "../services/change-consent-gate.js"; const RUN_LOG_DEFAULT_LIMIT_BYTES = 256_000; const RUN_LOG_MAX_LIMIT_BYTES = 1024 * 1024; @@ -1335,37 +1336,43 @@ export function agentRoutes( return; } - const allowedByReflectionCoachGate = await reflectionCoachMutationGateService(db).assertAllowed({ - companyId: targetAgent.companyId, - actorAgentId: req.actor.agentId, - actorRunId: req.actor.runId ?? null, - targetKeys: [reflectionCoachAgentInstructionsTargetKey(targetAgent.id)], - }); - if (allowedByReflectionCoachGate) return; + if (await isReflectionCoachActor(req, targetAgent.companyId)) { + const allowedByReflectionCoachGate = await changeConsentGateService(db).assertConsented({ + companyId: targetAgent.companyId, + actorAgentId: req.actor.agentId, + actorRunId: req.actor.runId ?? null, + targetKeys: [agentInstructionsChangeTargetKey(targetAgent.id)], + }); + if (allowedByReflectionCoachGate) return; + } throw forbidden( "Only board-authenticated callers can manage instructions path or bundle configuration", ); } + async function isReflectionCoachActor(req: Request, companyId: string) { + if (req.actor.type !== "agent" || !req.actor.agentId) return false; + const actorAgent = await svc.getById(req.actor.agentId); + if (!actorAgent || actorAgent.companyId !== companyId) { + throw forbidden("Agent key cannot access another company"); + } + return readBuiltInAgentMarker(actorAgent.metadata)?.key === "reflection-coach"; + } + async function assertReflectionCoachAgentDescriptionMutationGate( req: Request, targetAgent: { id: string; companyId: string }, ) { - await reflectionCoachMutationGateService(db).assertAllowed({ + if (!(await isReflectionCoachActor(req, targetAgent.companyId))) return; + await changeConsentGateService(db).assertConsented({ companyId: targetAgent.companyId, actorAgentId: req.actor.type === "agent" ? req.actor.agentId : null, actorRunId: req.actor.runId ?? null, - targetKeys: [reflectionCoachAgentDescriptionTargetKey(targetAgent.id)], + targetKeys: [agentProfileChangeTargetKey(targetAgent.id)], }); } - function touchesAgentDescription(patchData: Record) { - return ["name", "role", "title", "capabilities"].some((key) => - Object.prototype.hasOwnProperty.call(patchData, key), - ); - } - function assertNoAgentInstructionsConfigMutation( req: Request, adapterConfig: Record | null | undefined, @@ -2966,7 +2973,7 @@ export function agentRoutes( }, ); } - if (touchesAgentDescription(patchData)) { + if (touchesAgentProfileChangeConsentFields(patchData)) { await assertReflectionCoachAgentDescriptionMutationGate(req, existing); } diff --git a/server/src/routes/company-skills.ts b/server/src/routes/company-skills.ts index 8ef1a2c2c25..f1169d077e3 100644 --- a/server/src/routes/company-skills.ts +++ b/server/src/routes/company-skills.ts @@ -34,14 +34,14 @@ import { import { forbidden } from "../errors.js"; import { assertAuthenticated, assertCompanyAccess, getActorInfo } from "./authz.js"; import { getTelemetryClient } from "../telemetry.js"; +import { readBuiltInAgentMarker } from "../services/built-in-agent-metadata.js"; import { - reflectionCoachCompanySkillCatalogTargetKey, - reflectionCoachCompanySkillImportTargetKey, - reflectionCoachCompanySkillScanTargetKey, - reflectionCoachCompanySkillSlugTargetKey, - reflectionCoachCompanySkillTargetKey, - reflectionCoachMutationGateService, -} from "../services/reflection-coach-mutation-gate.js"; + changeConsentGateService, + skillChangeTargetKey, + skillImportChangeTargetKey, + skillSlugChangeTargetKey, + skillsScanProjectsChangeTargetKey, +} from "../services/change-consent-gate.js"; type SkillTelemetryInput = { key: string; @@ -106,8 +106,38 @@ export function companySkillRoutes(db: Db) { return { type: "system" as const }; } + function skillMutationTargets(input: { + skillId?: string | null; + slug?: unknown; + source?: unknown; + catalogSkillId?: unknown; + scanProjects?: boolean; + }) { + const targetKeys: string[] = []; + const skillId = asString(input.skillId); + const slug = asString(input.slug); + const source = asString(input.source); + const catalogSkillId = asString(input.catalogSkillId); + if (skillId) targetKeys.push(skillChangeTargetKey(skillId)); + if (slug) targetKeys.push(skillSlugChangeTargetKey(slug)); + if (source) targetKeys.push(skillImportChangeTargetKey(source)); + if (catalogSkillId) targetKeys.push(skillImportChangeTargetKey(catalogSkillId)); + if (input.scanProjects) targetKeys.push(skillsScanProjectsChangeTargetKey()); + return targetKeys; + } + + async function isReflectionCoachActor(req: Request, companyId: string) { + if (req.actor.type !== "agent" || !req.actor.agentId) return false; + const actorAgent = await agents.getById(req.actor.agentId); + if (!actorAgent || actorAgent.companyId !== companyId) { + throw forbidden("Agent key cannot access another company"); + } + return readBuiltInAgentMarker(actorAgent.metadata)?.key === "reflection-coach"; + } + async function assertReflectionCoachSkillMutationGate(req: Request, companyId: string, targetKeys: string[]) { - await reflectionCoachMutationGateService(db).assertAllowed({ + if (!(await isReflectionCoachActor(req, companyId))) return; + await changeConsentGateService(db).assertConsented({ companyId, actorAgentId: req.actor.type === "agent" ? req.actor.agentId : null, actorRunId: req.actor.runId ?? null, @@ -626,9 +656,7 @@ export function companySkillRoutes(db: Db) { const companyId = req.params.companyId as string; const skillId = req.params.skillId as string; await assertCanMutateCompanySkills(req, companyId); - await assertReflectionCoachSkillMutationGate(req, companyId, [ - reflectionCoachCompanySkillTargetKey(skillId), - ]); + await assertReflectionCoachSkillMutationGate(req, companyId, skillMutationTargets({ skillId })); const result = await svc.createVersion(companyId, skillId, req.body, skillActor(req)); const actor = getActorInfo(req); await logActivity(db, { @@ -697,12 +725,10 @@ export function companySkillRoutes(db: Db) { const companyId = req.params.companyId as string; const skillId = req.params.skillId as string; await assertCanMutateCompanySkills(req, companyId); - await assertReflectionCoachSkillMutationGate(req, companyId, [ - reflectionCoachCompanySkillTargetKey(skillId), - ...(typeof req.body.slug === "string" && req.body.slug.trim() - ? [reflectionCoachCompanySkillSlugTargetKey(req.body.slug.trim())] - : []), - ]); + await assertReflectionCoachSkillMutationGate(req, companyId, skillMutationTargets({ + skillId, + slug: req.body.slug, + })); const result = await svc.forkSkill(companyId, skillId, req.body, skillActor(req)); const actor = getActorInfo(req); await logActivity(db, { @@ -833,11 +859,9 @@ export function companySkillRoutes(db: Db) { async (req, res) => { const companyId = req.params.companyId as string; await assertCanMutateCompanySkills(req, companyId); - await assertReflectionCoachSkillMutationGate(req, companyId, [ - ...(typeof req.body.slug === "string" && req.body.slug.trim() - ? [reflectionCoachCompanySkillSlugTargetKey(req.body.slug.trim())] - : []), - ]); + await assertReflectionCoachSkillMutationGate(req, companyId, skillMutationTargets({ + slug: req.body.slug, + })); const result = await svc.createLocalSkill(companyId, req.body, skillActor(req)); const actor = getActorInfo(req); @@ -867,9 +891,7 @@ export function companySkillRoutes(db: Db) { const companyId = req.params.companyId as string; const skillId = req.params.skillId as string; await assertCanMutateCompanySkills(req, companyId); - await assertReflectionCoachSkillMutationGate(req, companyId, [ - reflectionCoachCompanySkillTargetKey(skillId), - ]); + await assertReflectionCoachSkillMutationGate(req, companyId, skillMutationTargets({ skillId })); const result = await svc.updateSkill(companyId, skillId, req.body); const actor = getActorInfo(req); @@ -900,9 +922,7 @@ export function companySkillRoutes(db: Db) { const companyId = req.params.companyId as string; const skillId = req.params.skillId as string; await assertCanMutateCompanySkills(req, companyId); - await assertReflectionCoachSkillMutationGate(req, companyId, [ - reflectionCoachCompanySkillTargetKey(skillId), - ]); + await assertReflectionCoachSkillMutationGate(req, companyId, skillMutationTargets({ skillId })); const result = await svc.updateFile( companyId, skillId, @@ -968,9 +988,7 @@ export function companySkillRoutes(db: Db) { const companyId = req.params.companyId as string; await assertCanMutateCompanySkills(req, companyId); const source = String(req.body.source ?? ""); - await assertReflectionCoachSkillMutationGate(req, companyId, [ - reflectionCoachCompanySkillImportTargetKey(source), - ]); + await assertReflectionCoachSkillMutationGate(req, companyId, skillMutationTargets({ source })); const result = await svc.importFromSource(companyId, source); const actor = getActorInfo(req); @@ -1010,12 +1028,10 @@ export function companySkillRoutes(db: Db) { async (req, res) => { const companyId = req.params.companyId as string; await assertCanMutateCompanySkills(req, companyId); - await assertReflectionCoachSkillMutationGate(req, companyId, [ - reflectionCoachCompanySkillCatalogTargetKey(String(req.body.catalogSkillId ?? "")), - ...(typeof req.body.slug === "string" && req.body.slug.trim() - ? [reflectionCoachCompanySkillSlugTargetKey(req.body.slug.trim())] - : []), - ]); + await assertReflectionCoachSkillMutationGate(req, companyId, skillMutationTargets({ + catalogSkillId: req.body.catalogSkillId, + slug: req.body.slug, + })); const result = await svc.installFromCatalog(companyId, req.body); const actor = getActorInfo(req); @@ -1048,9 +1064,7 @@ export function companySkillRoutes(db: Db) { async (req, res) => { const companyId = req.params.companyId as string; await assertCanMutateCompanySkills(req, companyId); - await assertReflectionCoachSkillMutationGate(req, companyId, [ - reflectionCoachCompanySkillScanTargetKey(), - ]); + await assertReflectionCoachSkillMutationGate(req, companyId, skillMutationTargets({ scanProjects: true })); const result = await svc.scanProjectWorkspaces(companyId, req.body); const actor = getActorInfo(req); @@ -1082,9 +1096,7 @@ export function companySkillRoutes(db: Db) { const companyId = req.params.companyId as string; const skillId = req.params.skillId as string; await assertCanMutateCompanySkills(req, companyId); - await assertReflectionCoachSkillMutationGate(req, companyId, [ - reflectionCoachCompanySkillTargetKey(skillId), - ]); + await assertReflectionCoachSkillMutationGate(req, companyId, skillMutationTargets({ skillId })); const result = await svc.deleteSkill(companyId, skillId); if (!result) { res.status(404).json({ error: "Skill not found" }); @@ -1116,9 +1128,7 @@ export function companySkillRoutes(db: Db) { const companyId = req.params.companyId as string; const skillId = req.params.skillId as string; await assertCanMutateCompanySkills(req, companyId); - await assertReflectionCoachSkillMutationGate(req, companyId, [ - reflectionCoachCompanySkillTargetKey(skillId), - ]); + await assertReflectionCoachSkillMutationGate(req, companyId, skillMutationTargets({ skillId })); const result = await svc.auditSkill(companyId, skillId); if (!result) { res.status(404).json({ error: "Skill not found" }); @@ -1155,9 +1165,7 @@ export function companySkillRoutes(db: Db) { const companyId = req.params.companyId as string; const skillId = req.params.skillId as string; await assertCanMutateCompanySkills(req, companyId); - await assertReflectionCoachSkillMutationGate(req, companyId, [ - reflectionCoachCompanySkillTargetKey(skillId), - ]); + await assertReflectionCoachSkillMutationGate(req, companyId, skillMutationTargets({ skillId })); const before = await svc.getById(companyId, skillId); const result = await svc.installUpdate(companyId, skillId, req.body); if (!result) { @@ -1198,9 +1206,7 @@ export function companySkillRoutes(db: Db) { const companyId = req.params.companyId as string; const skillId = req.params.skillId as string; await assertCanMutateCompanySkills(req, companyId); - await assertReflectionCoachSkillMutationGate(req, companyId, [ - reflectionCoachCompanySkillTargetKey(skillId), - ]); + await assertReflectionCoachSkillMutationGate(req, companyId, skillMutationTargets({ skillId })); const before = await svc.getById(companyId, skillId); const result = await svc.resetSkill(companyId, skillId, req.body); if (!result) { diff --git a/server/src/services/reflection-coach-mutation-gate.ts b/server/src/services/change-consent-gate.ts similarity index 53% rename from server/src/services/reflection-coach-mutation-gate.ts rename to server/src/services/change-consent-gate.ts index 1025f250f09..dc5eab1217d 100644 --- a/server/src/services/reflection-coach-mutation-gate.ts +++ b/server/src/services/change-consent-gate.ts @@ -1,36 +1,39 @@ import type { Db } from "@paperclipai/db"; -import { agents, issueThreadInteractions } from "@paperclipai/db"; +import { issueThreadInteractions } from "@paperclipai/db"; import { and, desc, eq, or, sql } from "drizzle-orm"; import type { RequestConfirmationPayload, RequestConfirmationResult } from "@paperclipai/shared"; import { forbidden } from "../errors.js"; -import { readBuiltInAgentMarker } from "./built-in-agent-metadata.js"; -export function reflectionCoachAgentInstructionsTargetKey(agentId: string) { - return `reflection-coach:agent-instructions:${agentId}`; +export const AGENT_PROFILE_CHANGE_CONSENT_FIELDS = ["name", "role", "title", "capabilities"] as const; + +export function agentInstructionsChangeTargetKey(agentId: string) { + return `agent:${agentId}:instructions`; } -export function reflectionCoachAgentDescriptionTargetKey(agentId: string) { - return `reflection-coach:agent-description:${agentId}`; +export function agentProfileChangeTargetKey(agentId: string) { + return `agent:${agentId}:profile`; } -export function reflectionCoachCompanySkillTargetKey(skillId: string) { - return `reflection-coach:company-skill:${skillId}`; +export function skillChangeTargetKey(skillId: string) { + return `skill:${skillId}`; } -export function reflectionCoachCompanySkillSlugTargetKey(slug: string) { - return `reflection-coach:company-skill-slug:${slug}`; +export function skillSlugChangeTargetKey(slug: string) { + return `skill-slug:${slug}`; } -export function reflectionCoachCompanySkillImportTargetKey(source: string) { - return `reflection-coach:company-skill-import:${source}`; +export function skillImportChangeTargetKey(source: string) { + return `skill-import:${source}`; } -export function reflectionCoachCompanySkillCatalogTargetKey(catalogSkillId: string) { - return `reflection-coach:company-skill-catalog:${catalogSkillId}`; +export function skillsScanProjectsChangeTargetKey() { + return "skills:scan-projects"; } -export function reflectionCoachCompanySkillScanTargetKey() { - return "reflection-coach:company-skills:scan-projects"; +export function touchesAgentProfileChangeConsentFields(patchData: Record) { + return AGENT_PROFILE_CHANGE_CONSENT_FIELDS.some((key) => + Object.prototype.hasOwnProperty.call(patchData, key), + ); } function readNonEmptyString(value: unknown) { @@ -44,9 +47,52 @@ function payloadHasDisplayedDiff(payload: RequestConfirmationPayload) { return /(^|\n)[+-][^\n]+/.test(details); } -export function reflectionCoachMutationGateService(db: Db) { +function legacyTargetKeysFor(targetKey: string) { + if (targetKey.startsWith("agent:") && targetKey.endsWith(":instructions")) { + const agentId = targetKey.slice("agent:".length, -":instructions".length); + if (agentId) return [`reflection-coach:agent-instructions:${agentId}`]; + } + if (targetKey.startsWith("agent:") && targetKey.endsWith(":profile")) { + const agentId = targetKey.slice("agent:".length, -":profile".length); + if (agentId) return [`reflection-coach:agent-description:${agentId}`]; + } + if (targetKey.startsWith("skill:")) { + const skillId = targetKey.slice("skill:".length); + if (skillId) return [`reflection-coach:company-skill:${skillId}`]; + } + if (targetKey.startsWith("skill-slug:")) { + const slug = targetKey.slice("skill-slug:".length); + if (slug) return [`reflection-coach:company-skill-slug:${slug}`]; + } + if (targetKey.startsWith("skill-import:")) { + const source = targetKey.slice("skill-import:".length); + if (source) { + return [ + `reflection-coach:company-skill-import:${source}`, + `reflection-coach:company-skill-catalog:${source}`, + ]; + } + } + if (targetKey === "skills:scan-projects") { + return ["reflection-coach:company-skills:scan-projects"]; + } + return []; +} + +function expandTargetKeysForLegacyCompatibility(targetKeys: string[]) { + const expanded = new Set(); + for (const targetKey of targetKeys) { + expanded.add(targetKey); + for (const legacyTargetKey of legacyTargetKeysFor(targetKey)) { + expanded.add(legacyTargetKey); + } + } + return [...expanded]; +} + +export function changeConsentGateService(db: Db) { return { - assertAllowed: async (input: { + assertConsented: async (input: { companyId: string; actorAgentId: string | null | undefined; actorRunId: string | null | undefined; @@ -55,23 +101,6 @@ export function reflectionCoachMutationGateService(db: Db) { const actorAgentId = readNonEmptyString(input.actorAgentId); if (!actorAgentId) return false; - const actorAgent = await db - .select({ - id: agents.id, - companyId: agents.companyId, - metadata: agents.metadata, - }) - .from(agents) - .where(eq(agents.id, actorAgentId)) - .then((rows) => rows[0] ?? null); - - if (!actorAgent || actorAgent.companyId !== input.companyId) { - throw forbidden("Agent key cannot access another company"); - } - - const marker = readBuiltInAgentMarker(actorAgent.metadata); - if (marker?.key !== "reflection-coach") return false; - const actorRunId = readNonEmptyString(input.actorRunId); if (!actorRunId) { throw forbidden("Reflection Coach mutations require a run id", { @@ -85,9 +114,10 @@ export function reflectionCoachMutationGateService(db: Db) { code: "reflection_coach_mutation_target_required", }); } + const queryTargetKeys = expandTargetKeysForLegacyCompatibility(targetKeys); const targetKeyPredicate = or( - ...targetKeys.map((targetKey) => + ...queryTargetKeys.map((targetKey) => sql`${issueThreadInteractions.payload}->'target'->>'key' = ${targetKey}`, ), ); @@ -114,7 +144,7 @@ export function reflectionCoachMutationGateService(db: Db) { const payload = row.payload as RequestConfirmationPayload; const result = row.result as RequestConfirmationResult | null; return payload.target?.type === "custom" - && targetKeys.includes(payload.target.key) + && queryTargetKeys.includes(payload.target.key) && result?.outcome === "accepted" && payloadHasDisplayedDiff(payload) && Boolean(row.sourceRunId)