Fix routine env binding sync review issues

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);

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;
     },
 

server/src/services/secrets.ts

@@ -61,6 +61,8 @@ const COMING_SOON_SECRET_PROVIDERS: ReadonlySet<SecretProvider> = new Set([
   "gcp_secret_manager",
   "vault",
 ]);
+type DbTransaction = Parameters<Parameters<Db["transaction"]>[0]>[0];
+type SecretBindingDb = Pick<Db | DbTransaction, "select" | "delete" | "insert">;
 
 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 | DbTransaction, "select"> = 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 | DbTransaction, "select"> = 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;
     },