fix(deploy): serialize secrets state updates for concurrent deploys

.changeset/serialize-secrets-state-writes.md

@@ -0,0 +1,5 @@
+---
+"@tailor-platform/sdk": patch
+---
+
+Fix concurrent deploys to the same workspace and application corrupting the local secrets hash state. Secret and auth-connection updates now hold a target-scoped lock while writing remote values and saving their hashes, so a later deploy no longer skips a secret update based on a hash that no longer matches the deployed value.

packages/sdk/src/cli/commands/deploy/auth-connection.test.ts

@@ -27,6 +27,7 @@ vi.mock("./secrets-state", async (importOriginal) => {
     ...actual,
     loadSecretsState: (...args: unknown[]) => mockLoadSecretsState(...args),
     saveSecretsState: (...args: unknown[]) => mockSaveSecretsState(...args),
+    withSecretsStateLock: (_scope: unknown, fn: () => Promise<unknown>) => fn(),
     // Deterministic hash so tests can pin the "unchanged" secret state.
     hashValue: () => "fixed-hash",
   };

packages/sdk/src/cli/commands/deploy/auth-connection.ts

@@ -9,7 +9,12 @@ import { logger } from "#/cli/shared/logger";
 import { createChangeSet } from "./change-set";
 import { buildMetaRequest, resourceTrn, sdkNameLabelKey, type WithLabel } from "./label";
 import { trackDesiredResourceOwnership, trackRemainingResourceOwner } from "./owned-resource";
-import { hashValue, loadSecretsState, saveSecretsState } from "./secrets-state";
+import {
+  hashValue,
+  loadSecretsState,
+  saveSecretsState,
+  withSecretsStateLock,
+} from "./secrets-state";
 import type { AuthConnectionConfig } from "#/types/auth-connection.generated";
 import type { OwnerConflict, UnmanagedResource } from "./confirm";
 import type { ApplyPhase } from "./phase";
@@ -255,28 +260,55 @@ export async function applyAuthConnections(
   const { changeSet, stateScope } = result;
 
   if (phase === "create-update") {
-    await Promise.all(
-      changeSet.creates.map(async (create) => {
-        await client.createAuthConnection(create.request);
-        await client.setMetadata(create.metaRequest);
-        logger.info(
-          `Connection "${create.name}" was created. Authorize it with:\n` +
-            `  tailor-sdk authconnection authorize --name ${create.name}\n` +
-            `Or via the Console: tailor-sdk authconnection open`,
+    if (changeSet.creates.length > 0 || changeSet.replaces.length > 0) {
+      await withSecretsStateLock(stateScope, async () => {
+        await Promise.all(
+          changeSet.creates.map(async (create) => {
+            await client.createAuthConnection(create.request);
+            await client.setMetadata(create.metaRequest);
+            logger.info(
+              `Connection "${create.name}" was created. Authorize it with:\n` +
+                `  tailor-sdk authconnection authorize --name ${create.name}\n` +
+                `Or via the Console: tailor-sdk authconnection open`,
+            );
+          }),
         );
-      }),
-    );
 
-    for (const replace of changeSet.replaces) {
-      const resp = await client.updateAuthConnection(replace.updateRequest);
-      if (resp.connection?.status === AuthConnection_Status.UNAUTHORIZED) {
-        logger.warn(
-          `Connection "${replace.name}" requires re-authorization. Authorize with:\n` +
-            `  tailor-sdk authconnection authorize --name ${replace.name}\n` +
-            `Or via the Console: tailor-sdk authconnection open`,
+        for (const replace of changeSet.replaces) {
+          const resp = await client.updateAuthConnection(replace.updateRequest);
+          if (resp.connection?.status === AuthConnection_Status.UNAUTHORIZED) {
+            logger.warn(
+              `Connection "${replace.name}" requires re-authorization. Authorize with:\n` +
+                `  tailor-sdk authconnection authorize --name ${replace.name}\n` +
+                `Or via the Console: tailor-sdk authconnection open`,
+            );
+          }
+          await client.setMetadata(replace.metaRequest);
+        }
+
+        const secretReplaces = changeSet.replaces.filter((replace) =>
+          replace.updateRequest.updateMask?.paths?.includes("oauth2.client_secret"),
         );
-      }
-      await client.setMetadata(replace.metaRequest);
+        if (changeSet.creates.length > 0 || secretReplaces.length > 0) {
+          const state = loadSecretsState(stateScope);
+          if (!state.connections) {
+            state.connections = {};
+          }
+          for (const create of changeSet.creates) {
+            const conn = create.request.connection;
+            if (conn?.config?.case === "oauth2") {
+              state.connections[create.name] = hashValue(conn.config.value.clientSecret ?? "");
+            }
+          }
+          for (const replace of secretReplaces) {
+            const conn = replace.updateRequest.connection;
+            if (conn?.config?.case === "oauth2") {
+              state.connections[replace.name] = hashValue(conn.config.value.clientSecret ?? "");
+            }
+          }
+          saveSecretsState(stateScope, state);
+        }
+      });
     }
 
     // Metadata-only updates: backfill the SDK ownership label on connections
@@ -286,44 +318,21 @@ export async function applyAuthConnections(
         await client.setMetadata(update.metaRequest);
       }),
     );
+  } else if (changeSet.deletes.length > 0) {
+    await withSecretsStateLock(stateScope, async () => {
+      await Promise.all(
+        changeSet.deletes.map(async (del) => {
+          await client.deleteAuthConnection(del.request);
+        }),
+      );
 
-    const secretReplaces = changeSet.replaces.filter((replace) =>
-      replace.updateRequest.updateMask?.paths?.includes("oauth2.client_secret"),
-    );
-    if (changeSet.creates.length > 0 || secretReplaces.length > 0) {
-      const state = loadSecretsState(stateScope);
-      if (!state.connections) {
-        state.connections = {};
-      }
-      for (const create of changeSet.creates) {
-        const conn = create.request.connection;
-        if (conn?.config?.case === "oauth2") {
-          state.connections[create.name] = hashValue(conn.config.value.clientSecret ?? "");
-        }
-      }
-      for (const replace of secretReplaces) {
-        const conn = replace.updateRequest.connection;
-        if (conn?.config?.case === "oauth2") {
-          state.connections[replace.name] = hashValue(conn.config.value.clientSecret ?? "");
-        }
-      }
-      saveSecretsState(stateScope, state);
-    }
-  } else {
-    await Promise.all(
-      changeSet.deletes.map(async (del) => {
-        await client.deleteAuthConnection(del.request);
-      }),
-    );
-
-    if (changeSet.deletes.length > 0) {
       const state = loadSecretsState(stateScope);
       if (state.connections) {
         for (const del of changeSet.deletes) {
           delete state.connections[del.name];
         }
         saveSecretsState(stateScope, state);
       }
-    }
+    });
   }
 }

packages/sdk/src/cli/commands/deploy/secret-manager.test.ts

@@ -21,6 +21,7 @@ vi.mock("./secrets-state", async (importOriginal) => {
     ...actual,
     loadSecretsState: (...args: unknown[]) => mockLoadSecretsState(...args),
     saveSecretsState: (...args: unknown[]) => mockSaveSecretsState(...args),
+    withSecretsStateLock: (_scope: unknown, fn: () => Promise<unknown>) => fn(),
   };
 });
 

packages/sdk/src/cli/commands/deploy/secret-manager.ts

@@ -7,7 +7,12 @@ import {
   trackDesiredResourceOwnership,
   trackRemainingResourceOwner,
 } from "./owned-resource";
-import { hashValue, loadSecretsState, saveSecretsState } from "./secrets-state";
+import {
+  hashValue,
+  loadSecretsState,
+  saveSecretsState,
+  withSecretsStateLock,
+} from "./secrets-state";
 import type { ApplyPhase, PlanContext } from "#/cli/commands/deploy/types";
 import type { Application } from "#/cli/services/application";
 import type { OwnerConflict, UnmanagedResource } from "./confirm";
@@ -337,57 +342,61 @@ export async function applySecretManager(
       );
     }
 
-    // Create new secrets
-    await Promise.all(
-      secretChangeSet.creates.map((create) =>
-        client.createSecretManagerSecret(secretCreateRequest(create)),
-      ),
-    );
+    const secretHashUpdates = [...secretChangeSet.creates, ...secretChangeSet.updates];
+    if (secretHashUpdates.length > 0) {
+      await withSecretsStateLock(stateScope, async () => {
+        // Create new secrets
+        await Promise.all(
+          secretChangeSet.creates.map((create) =>
+            client.createSecretManagerSecret(secretCreateRequest(create)),
+          ),
+        );
 
-    // Update existing secrets
-    await Promise.all(
-      secretChangeSet.updates.map((update) =>
-        client.updateSecretManagerSecret(secretUpdateRequest(update)),
-      ),
-    );
+        // Update existing secrets
+        await Promise.all(
+          secretChangeSet.updates.map((update) =>
+            client.updateSecretManagerSecret(secretUpdateRequest(update)),
+          ),
+        );
 
-    const secretHashUpdates = [...secretChangeSet.creates, ...secretChangeSet.updates];
-    if (application && secretHashUpdates.length > 0) {
-      const state = loadSecretsState(stateScope);
-      for (const secret of secretHashUpdates) {
-        if (!Object.hasOwn(state.vaults, secret.vaultName)) {
-          state.vaults[secret.vaultName] = {};
+        if (application) {
+          const state = loadSecretsState(stateScope);
+          for (const secret of secretHashUpdates) {
+            if (!Object.hasOwn(state.vaults, secret.vaultName)) {
+              state.vaults[secret.vaultName] = {};
+            }
+            assertDefined(state.vaults[secret.vaultName], "vault state entry missing")[
+              secret.secretName
+            ] = hashValue(secret.value);
+          }
+          saveSecretsState(stateScope, state);
         }
-        assertDefined(state.vaults[secret.vaultName], "vault state entry missing")[
-          secret.secretName
-        ] = hashValue(secret.value);
-      }
-      saveSecretsState(stateScope, state);
+      });
     }
-  } else {
-    // Delete orphan secrets
-    await Promise.all(
-      secretChangeSet.deletes.map((del) =>
-        client.deleteSecretManagerSecret({
-          workspaceId: del.workspaceId,
-          secretmanagerVaultName: del.vaultName,
-          secretmanagerSecretName: del.secretName,
-        }),
-      ),
-    );
+  } else if (secretChangeSet.deletes.length > 0 || vaultChangeSet.deletes.length > 0) {
+    await withSecretsStateLock(stateScope, async () => {
+      // Delete orphan secrets
+      await Promise.all(
+        secretChangeSet.deletes.map((del) =>
+          client.deleteSecretManagerSecret({
+            workspaceId: del.workspaceId,
+            secretmanagerVaultName: del.vaultName,
+            secretmanagerSecretName: del.secretName,
+          }),
+        ),
+      );
 
-    // Delete orphan vaults
-    await Promise.all(
-      vaultChangeSet.deletes.map((del) =>
-        client.deleteSecretManagerVault({
-          workspaceId: del.workspaceId,
-          secretmanagerVaultName: del.name,
-        }),
-      ),
-    );
+      // Delete orphan vaults
+      await Promise.all(
+        vaultChangeSet.deletes.map((del) =>
+          client.deleteSecretManagerVault({
+            workspaceId: del.workspaceId,
+            secretmanagerVaultName: del.name,
+          }),
+        ),
+      );
 
-    // Remove deleted secrets and vaults from hash state
-    if (secretChangeSet.deletes.length > 0 || vaultChangeSet.deletes.length > 0) {
+      // Remove deleted secrets and vaults from hash state
       const state = loadSecretsState(stateScope);
       for (const del of secretChangeSet.deletes) {
         if (Object.hasOwn(state.vaults, del.vaultName)) {
@@ -406,6 +415,6 @@ export async function applySecretManager(
         delete state.vaults[del.name];
       }
       saveSecretsState(stateScope, state);
-    }
+    });
   }
 }