diff --git a/server/src/__tests__/routines-service.test.ts b/server/src/__tests__/routines-service.test.ts index f7898ac9053..e449bd30207 100644 --- a/server/src/__tests__/routines-service.test.ts +++ b/server/src/__tests__/routines-service.test.ts @@ -382,8 +382,25 @@ describeEmbeddedPostgres("routine service live-execution coalescing", () => { const [initialRevision] = await svc.listRevisions(routine.id); expect(initialRevision?.snapshot.routine.env).toEqual(routine.env); + await db.delete(companySecretBindings).where(eq(companySecretBindings.targetId, routine.id)); + const repaired = await svc.update(routine.id, { env: routine.env }, {}); + expect(repaired).not.toBeNull(); + const repairedBindings = await db + .select() + .from(companySecretBindings) + .where(eq(companySecretBindings.targetId, routine.id)); + expect(repairedBindings).toMatchObject([ + { + companyId, + secretId: secret.id, + targetType: "routine", + configPath: "env.ROUTINE_API_KEY", + }, + ]); + + const currentRoutine = repaired ?? routine; const runBefore = await svc.runRoutine(routine.id, { source: "manual" }); - expect(runBefore.routineRevisionId).toBe(routine.latestRevisionId); + expect(runBefore.routineRevisionId).toBe(currentRoutine.latestRevisionId); const updated = await svc.update( routine.id, @@ -395,7 +412,7 @@ describeEmbeddedPostgres("routine service live-execution coalescing", () => { }, {}, ); - expect(updated?.latestRevisionNumber).toBe(routine.latestRevisionNumber + 1); + expect(updated?.latestRevisionNumber).toBe(currentRoutine.latestRevisionNumber + 1); const runAfter = await svc.runRoutine(routine.id, { source: "manual" }); expect(runAfter.routineRevisionId).toBe(updated?.latestRevisionId); diff --git a/server/src/services/routines.ts b/server/src/services/routines.ts index b420ea51809..80e221766b7 100644 --- a/server/src/services/routines.ts +++ b/server/src/services/routines.ts @@ -1562,15 +1562,16 @@ export function routineService( const { routine } = await appendRoutineRevision(txDb, created, actor, { changeSummary: "Created routine", }); + if (env) { + await secretsSvc.syncEnvBindingsForTarget( + companyId, + { targetType: "routine", targetId: routine.id }, + env, + { db: tx }, + ); + } return routine; }); - if (env) { - await secretsSvc.syncEnvBindingsForTarget( - companyId, - { targetType: "routine", targetId: createdRoutine.id }, - env, - ); - } return createdRoutine; }, @@ -1672,6 +1673,14 @@ export function routineService( ) .then((rows) => rows[0] ?? null); if (latestRevision && snapshotsMatch(nextSnapshot, latestRevision.snapshot as RoutineRevisionSnapshotV1)) { + if (patch.env !== undefined) { + await secretsSvc.syncEnvBindingsForTarget( + locked.companyId, + { targetType: "routine", targetId: locked.id }, + candidate.env, + { db: tx }, + ); + } return locked; } } @@ -1701,15 +1710,16 @@ export function routineService( const { routine } = await appendRoutineRevision(txDb, updated, actor, { changeSummary: "Updated routine", }); + if (patch.env !== undefined) { + await secretsSvc.syncEnvBindingsForTarget( + routine.companyId, + { targetType: "routine", targetId: routine.id }, + routine.env, + { db: tx }, + ); + } return routine; }); - if (updatedRoutine && patch.env !== undefined) { - await secretsSvc.syncEnvBindingsForTarget( - updatedRoutine.companyId, - { targetType: "routine", targetId: updatedRoutine.id }, - updatedRoutine.env, - ); - } return updatedRoutine; }, @@ -2084,6 +2094,12 @@ export function routineService( changeSummary: `Restored from revision ${targetRevision.revisionNumber}`, restoredFromRevisionId: targetRevision.id, }); + await secretsSvc.syncEnvBindingsForTarget( + locked.companyId, + { targetType: "routine", targetId: locked.id }, + routineSnapshot.env, + { db: tx }, + ); return { routine: appended.routine, revision: appended.revision, @@ -2092,11 +2108,6 @@ export function routineService( secretMaterials: [...recreatedWebhookSecrets.values()].map((entry) => entry.secretMaterial), }; }); - await secretsSvc.syncEnvBindingsForTarget( - existingRoutine.companyId, - { targetType: "routine", targetId: existingRoutine.id }, - routineSnapshot.env, - ); return result; }, diff --git a/server/src/services/secrets.ts b/server/src/services/secrets.ts index 226e40f7bd3..ce9ee967812 100644 --- a/server/src/services/secrets.ts +++ b/server/src/services/secrets.ts @@ -61,6 +61,8 @@ const COMING_SOON_SECRET_PROVIDERS: ReadonlySet = new Set([ "gcp_secret_manager", "vault", ]); +type DbTransaction = Parameters[0]>[0]; +type SecretBindingDb = Pick; function remoteProviderHttpError(error: unknown, context: { companyId: string; @@ -294,8 +296,8 @@ export function secretService(db: Db) { fieldPath?: string; }; - async function getById(id: string) { - return db + async function getById(id: string, source: Pick = db) { + return source .select() .from(companySecrets) .where(eq(companySecrets.id, id)) @@ -401,8 +403,12 @@ export function secretService(db: Db) { }); } - async function assertSecretInCompany(companyId: string, secretId: string) { - const secret = await getById(secretId); + async function assertSecretInCompany( + companyId: string, + secretId: string, + source: Pick = db, + ) { + const secret = await getById(secretId, source); if (!secret) throw notFound("Secret not found"); if (secret.status === "deleted") throw notFound("Secret not found"); if (secret.companyId !== companyId) throw unprocessable("Secret must belong to same company"); @@ -2025,6 +2031,7 @@ export function secretService(db: Db) { companyId: string, target: { targetType: SecretBindingTargetType; targetId: string; pathPrefix?: string }, envValue: unknown, + options?: { db?: SecretBindingDb }, ) => { const record = asRecord(envValue) ?? {}; const refs: Array<{ @@ -2033,12 +2040,13 @@ export function secretService(db: Db) { versionSelector: SecretVersionSelector; }> = []; const pathPrefix = target.pathPrefix ?? "env"; + const bindingDb = options?.db ?? db; for (const [key, rawBinding] of Object.entries(record)) { const parsed = envBindingSchema.safeParse(rawBinding); if (!parsed.success) continue; const binding = canonicalizeBinding(parsed.data as EnvBinding); if (binding.type !== "secret_ref") continue; - await assertSecretInCompany(companyId, binding.secretId); + await assertSecretInCompany(companyId, binding.secretId, bindingDb); refs.push({ secretId: binding.secretId, configPath: `${pathPrefix}.${key}`, @@ -2046,8 +2054,8 @@ export function secretService(db: Db) { }); } - await db.transaction(async (tx) => { - await tx + const writeBindings = async (targetDb: SecretBindingDb) => { + await targetDb .delete(companySecretBindings) .where( and( @@ -2058,7 +2066,7 @@ export function secretService(db: Db) { ), ); if (refs.length === 0) return; - await tx.insert(companySecretBindings).values( + await targetDb.insert(companySecretBindings).values( refs.map((ref) => ({ companyId, secretId: ref.secretId, @@ -2069,7 +2077,13 @@ export function secretService(db: Db) { required: true, })), ); - }); + }; + + if (options?.db) { + await writeBindings(options.db); + } else { + await db.transaction(async (tx) => writeBindings(tx)); + } return refs; }, diff --git a/ui/src/components/RoutineHistoryTab.test.tsx b/ui/src/components/RoutineHistoryTab.test.tsx index bceb9d6fcf8..e55fb3e7c9f 100644 --- a/ui/src/components/RoutineHistoryTab.test.tsx +++ b/ui/src/components/RoutineHistoryTab.test.tsx @@ -389,6 +389,87 @@ describe("RoutineHistoryTab", () => { expect(container.textContent).toContain("1 key changed"); }); + it("labels secret-ref env diffs by changed secret instead of binding kind", async () => { + const current = createRevision({ + id: "revision-2", + revisionNumber: 2, + snapshot: snapshotV1({ + env: { GH_TOKEN: { type: "secret_ref", secretId: "secret-2", version: "latest" } }, + }), + }); + const old = createRevision({ + id: "revision-1", + revisionNumber: 1, + snapshot: snapshotV1({ + env: { GH_TOKEN: { type: "secret_ref", secretId: "secret-1", version: "latest" } }, + }), + }); + const secrets: CompanySecret[] = [ + { + id: "secret-1", + companyId: "company-1", + key: "old_token", + name: "old-token", + provider: "local_encrypted", + status: "active", + managedMode: "paperclip_managed", + externalRef: null, + providerConfigId: null, + providerMetadata: null, + latestVersion: 1, + description: null, + lastResolvedAt: null, + lastRotatedAt: null, + deletedAt: null, + createdByAgentId: null, + createdByUserId: null, + createdAt: new Date("2026-04-01T00:00:00.000Z"), + updatedAt: new Date("2026-04-01T00:00:00.000Z"), + }, + { + id: "secret-2", + companyId: "company-1", + key: "new_token", + name: "new-token", + provider: "local_encrypted", + status: "active", + managedMode: "paperclip_managed", + externalRef: null, + providerConfigId: null, + providerMetadata: null, + latestVersion: 1, + description: null, + lastResolvedAt: null, + lastRotatedAt: null, + deletedAt: null, + createdByAgentId: null, + createdByUserId: null, + createdAt: new Date("2026-04-01T00:00:00.000Z"), + updatedAt: new Date("2026-04-01T00:00:00.000Z"), + }, + ]; + mockRoutinesApi.listRevisions.mockResolvedValue([current, old]); + await render({ secrets }); + + const oldRow = container.querySelector( + "[data-testid='revision-row-1']", + ) as HTMLButtonElement | null; + await act(async () => { + oldRow?.click(); + }); + await flush(); + const compareButton = Array.from(container.querySelectorAll("button")).find( + (button) => button.textContent === "Compare with current", + ); + await act(async () => { + compareButton?.click(); + }); + await flush(); + + expect(container.textContent).toContain("Env GH_TOKEN secret"); + expect(container.textContent).not.toContain("Env GH_TOKEN binding kind"); + }); + it("invokes onRestored with the restore response so the editor can rehydrate (PAP-3588)", async () => { const current = createRevision({ id: "revision-2", revisionNumber: 2 }); const old = createRevision({ diff --git a/ui/src/components/RoutineHistoryTab.tsx b/ui/src/components/RoutineHistoryTab.tsx index 30782ae03ba..35bd4cfb470 100644 --- a/ui/src/components/RoutineHistoryTab.tsx +++ b/ui/src/components/RoutineHistoryTab.tsx @@ -1223,7 +1223,7 @@ function compareEnv( const newRef = asSecretRef(newBinding)!; if (oldRef.secretId !== newRef.secretId) { changes.push({ - field: `Env ${key} binding kind`, + field: `Env ${key} secret`, oldValue: describeEnvBinding(oldBinding, secrets), newValue: describeEnvBinding(newBinding, secrets), });