fix(credentials): name payment providers so connectors can reference them

src/core/project/backends/cdk.test.ts

@@ -945,6 +945,78 @@ describe("CdkBackend.deploy", () => {
     expect(deployed.result).toEqual({ outputs: {} });
   });
 
+  test("removes a payment provider recorded under its underscored name once its replacement deployed", async () => {
+    const input = await project();
+    input.spec = ProjectSpecSchema.parse({
+      name: "example",
+      version: 2,
+      credentials: [
+        { authorizerType: "PaymentCredentialProvider", name: "wallet", provider: "CoinbaseCDP" },
+      ],
+    });
+    await writeAssembly(input, [TARGET.name]);
+    const statePath = join(input.rootPath, DEPLOYED_STATE_RELATIVE_PATH);
+    await mkdir(dirname(statePath), { recursive: true });
+    const arn = (name: string) =>
+      `arn:aws:bedrock-agentcore:${TARGET.region}:${TARGET.account}:token-vault/default/paymentcredentialprovider/${name}`;
+    await writeFile(
+      statePath,
+      JSON.stringify({
+        targets: {
+          default: {
+            stackArn: "arn:stack:default",
+            resources: {
+              credentials: {
+                wallet: {
+                  credentialProviderArn: arn("example_default_wallet"),
+                  authorizerType: "PaymentCredentialProvider",
+                },
+              },
+            },
+          },
+        },
+      }),
+    );
+    const provisionCredentials: CredentialProvisioner = async function* () {
+      yield { type: "step", message: "preparing credential provider 'example-default-wallet'" };
+      return {
+        wallet: {
+          credentialProviderArn: arn("example-default-wallet"),
+          authorizerType: "PaymentCredentialProvider",
+        },
+      };
+    };
+    const handed: CredentialRemovalInput[] = [];
+    const removeCredentials: CredentialRemover = async function* (_project, removalInput) {
+      handed.push(removalInput);
+      yield { type: "step", message: "removing credential provider 'example_default_wallet'" };
+    };
+    const subject = harness({ provisionCredentials, removeCredentials });
+
+    const deployed = await collectDeploy(subject.backend.deploy(input, deployInput()));
+
+    // The credential is still declared, but the stack now uses the hyphenated provider,
+    // and no connector could ever reference the underscored one.
+    expect(handed).toEqual([
+      {
+        credentials: subject.credentials,
+        region: TARGET.region,
+        targetName: TARGET.name,
+        providers: [
+          {
+            name: "wallet",
+            authorizerType: "PaymentCredentialProvider",
+            legacyProvider: "example_default_wallet",
+          },
+        ],
+      },
+    ]);
+    const messages = stepMessages(deployed.events);
+    expect(
+      messages.indexOf("deploying AgentCore-example-default-0 (usually takes a few minutes)"),
+    ).toBeLessThan(messages.indexOf("removing credential provider 'example_default_wallet'"));
+  });
+
   test("says to add a resource when there is no stack to remove either", async () => {
     const input = await project();
     await writeAssembly(input, [TARGET.name], { resources: METADATA_ONLY });

src/core/project/backends/cdk.ts

@@ -273,7 +273,7 @@ export class CdkBackend implements ProjectBackend {
     const recorded =
       (await readDeployedState(this.json, project.rootPath)).targets[target.name]?.resources
         ?.credentials ?? {};
-    const orphaned = orphanedCredentials(recorded, project.spec.credentials);
+    const orphaned = orphanedCredentials(project, target.name, recorded);
 
     // Credential providers aren't stack resources; the synthesized app reads their
     // ARNs from deployed-state.json, so they must exist and be recorded before synth.

src/core/project/backends/cdk/credentials.test.ts

@@ -56,10 +56,17 @@ const COINBASE = {
 /**
  * The names Identity sees: project, target, then the spec credential name. Spec names
  * and the `.env.local` variables derived from them are untouched by the scoping.
+ * Payment names are joined with hyphens, which a payment connector can reference.
  */
 const OPENAI_PROVIDER = "example_dev_openai-key";
 const OAUTH_PROVIDER = "example_dev_my-oauth";
-const WALLET_PROVIDER = "example_dev_wallet";
+const WALLET_PROVIDER = "example-dev-wallet";
+
+/** The service's pattern for a payment connector's `credentialProviderArn`. */
+const PAYMENT_CONNECTOR_CREDENTIAL_ARN =
+  /^arn:[^:]+:(acps|bedrock-agentcore):[A-Za-z0-9-]{1,64}:[0-9]{12}:token-vault\/[a-zA-Z0-9-.]+\/paymentcredentialprovider\/[a-zA-Z0-9-.]+$/;
+const paymentProviderArn = (name: string) =>
+  `arn:aws:bedrock-agentcore:${REGION}:123456789012:token-vault/default/paymentcredentialprovider/${name}`;
 
 const tempDirectories: string[] = [];
 afterEach(async () => {
@@ -716,7 +723,7 @@ describe("createCredentialProvisioner", () => {
   });
 
   test("updates an existing StripePrivy payment provider", async () => {
-    const subject = account({ example_dev_pay: { credentialProviderArn: "arn:existing" } });
+    const subject = account({ "example-dev-pay": { credentialProviderArn: "arn:existing" } });
     const input = await project(
       [{ authorizerType: "PaymentCredentialProvider", name: "pay", provider: "StripePrivy" }],
       "AGENTCORE_CREDENTIAL_PAY_APP_ID='app-1'\n" +
@@ -727,9 +734,9 @@ describe("createCredentialProvisioner", () => {
 
     await run(subject.provision, input);
 
-    expect(subject.contents()["example_dev_pay"]?.operation).toBe("updated");
-    expect(subject.configFor("example_dev_pay")).toEqual({
-      name: "example_dev_pay",
+    expect(subject.contents()["example-dev-pay"]?.operation).toBe("updated");
+    expect(subject.configFor("example-dev-pay")).toEqual({
+      name: "example-dev-pay",
       credentialProviderVendor: "StripePrivy",
       providerConfigurationInput: {
         stripePrivyConfiguration: {
@@ -742,6 +749,27 @@ describe("createCredentialProvisioner", () => {
     });
   });
 
+  test("names a payment provider so its connector can reference it", async () => {
+    const subject = account();
+    const input = await project(
+      [{ ...COINBASE, name: "my_wallet" }],
+      "AGENTCORE_CREDENTIAL_MY_WALLET_API_KEY_ID='key-1'\n" +
+        "AGENTCORE_CREDENTIAL_MY_WALLET_API_KEY_SECRET='key-secret'\n" +
+        "AGENTCORE_CREDENTIAL_MY_WALLET_WALLET_SECRET='wallet-secret'\n",
+    );
+
+    const { result } = await run(subject.provision, input);
+
+    // The underscores of the scope and of the credential name both become hyphens;
+    // the spec name, and the variables derived from it, keep theirs.
+    expect(Object.keys(subject.contents())).toEqual(["example-dev-my-wallet"]);
+    expect(Object.keys(result)).toEqual(["my_wallet"]);
+    expect(paymentProviderArn("example-dev-my-wallet")).toMatch(PAYMENT_CONNECTOR_CREDENTIAL_ARN);
+    expect(paymentProviderArn("example_dev_my_wallet")).not.toMatch(
+      PAYMENT_CONNECTOR_CREDENTIAL_ARN,
+    );
+  });
+
   test("names every payment variable that is unset", async () => {
     const subject = account();
     const input = await project([COINBASE], "AGENTCORE_CREDENTIAL_WALLET_API_KEY_ID='key-1'\n");
@@ -934,10 +962,53 @@ describe("orphanedCredentials", () => {
     };
     const input = await project([COINBASE]);
 
-    expect(orphanedCredentials(recorded, input.spec.credentials)).toEqual([
+    expect(orphanedCredentials(input, INPUT.targetName, recorded)).toEqual([
       { name: "openai-key", authorizerType: "ApiKeyCredentialProvider" },
     ]);
   });
+
+  test("keeps a payment provider recorded under its underscored name, by that name", async () => {
+    // Created by a CLI that joined payment names with underscores, which no connector
+    // can reference. The deploy creates the hyphenated one, so the old one goes,
+    // whether or not its credential is still declared.
+    const recorded: DeployedCredentials = {
+      wallet: recordedAs("PaymentCredentialProvider", {
+        credentialProviderArn: paymentProviderArn("example_dev_wallet"),
+      }),
+      dropped: recordedAs("PaymentCredentialProvider", {
+        credentialProviderArn: paymentProviderArn("example_dev_dropped"),
+      }),
+      current: recordedAs("PaymentCredentialProvider", {
+        credentialProviderArn: paymentProviderArn("example-dev-current"),
+      }),
+    };
+    const input = await project([COINBASE, { ...COINBASE, name: "current" }]);
+
+    expect(orphanedCredentials(input, INPUT.targetName, recorded)).toEqual([
+      {
+        name: "wallet",
+        authorizerType: "PaymentCredentialProvider",
+        legacyProvider: "example_dev_wallet",
+      },
+      {
+        name: "dropped",
+        authorizerType: "PaymentCredentialProvider",
+        legacyProvider: "example_dev_dropped",
+      },
+    ]);
+  });
+
+  test("leaves a v0 payment provider's bare name alone", async () => {
+    const recorded: DeployedCredentials = {
+      wallet: recordedAs("PaymentCredentialProvider", {
+        credentialProviderArn: paymentProviderArn("wallet"),
+      }),
+    };
+
+    expect(
+      orphanedCredentials(await migratedProject([COINBASE]), INPUT.targetName, recorded),
+    ).toEqual([]);
+  });
 });
 
 describe("createCredentialRemover", () => {
@@ -973,6 +1044,21 @@ describe("createCredentialRemover", () => {
     expect(subject.optionsSeen).toEqual([OPTIONS, OPTIONS, OPTIONS]);
   });
 
+  test("deletes a payment provider by the name it was recorded under", async () => {
+    const subject = account({
+      example_dev_wallet: { credentialProviderArn: paymentProviderArn("example_dev_wallet") },
+      [WALLET_PROVIDER]: { credentialProviderArn: paymentProviderArn(WALLET_PROVIDER) },
+    });
+
+    const { events } = await remove(subject, [
+      { ...COINBASE, legacyProvider: "example_dev_wallet" },
+    ]);
+
+    expect(subject.deleted).toEqual([{ kind: "payment", name: "example_dev_wallet" }]);
+    expect(Object.keys(subject.contents())).toEqual([WALLET_PROVIDER]);
+    expect(stepMessages(events)).toEqual(["removing credential provider 'example_dev_wallet'"]);
+  });
+
   test("treats a provider that is already gone as removed", async () => {
     const notFound = new ResourceNotFoundException({ $metadata: {}, message: "not found" });
     const subject = account({}, { deleteFails: notFound });

src/core/project/backends/cdk/credentials.ts

@@ -49,13 +49,46 @@ function keepsV0CredentialNames(project: Project): boolean {
  * The Identity name a credential's provider is created under. Scoped to the project
  * and target so two targets in one account and region never share a provider.
  *
+ * A payment provider is scoped with hyphens, and the credential name's underscores
+ * become hyphens too: a payment connector's `credentialProviderArn` only accepts
+ * letters, digits, `-` and `.` in the provider name, and Identity names do not
+ * allow `.`, so an underscore anywhere in the name leaves the connector unable to
+ * reference its provider. Two credentials cannot meet on one name this way: names
+ * that differ only in `_` and `-` derive the same `.env.local` variables, which the
+ * spec already refuses.
+ *
  * A v0 project, and one migrated from v0, keeps the bare credential name v0 created
  * its providers under: the agent code it deployed looks them up by that name, and a
  * scoped name would create a second set of providers beside the ones still in use.
  */
-function providerName(project: Project, targetName: string, credentialName: string): string {
-  if (keepsV0CredentialNames(project)) return credentialName;
-  return `${project.name}_${targetName}_${credentialName}`;
+function providerName(
+  project: Project,
+  targetName: string,
+  { name, authorizerType }: CredentialProviderRef,
+): string {
+  if (keepsV0CredentialNames(project)) return name;
+  if (authorizerType === "PaymentCredentialProvider") {
+    return `${project.name}-${targetName}-${name.replaceAll("_", "-")}`;
+  }
+  return `${project.name}_${targetName}_${name}`;
+}
+
+/**
+ * The name CLIs before the hyphenated scheme gave a payment provider, when the
+ * recorded ARN still points at it. Such a provider can never back a connector, so
+ * the deploy that creates its replacement deletes it like any other orphan.
+ */
+function legacyPaymentProviderName(
+  project: Project,
+  targetName: string,
+  name: string,
+  { credentialProviderArn, authorizerType }: DeployedCredential,
+): string | undefined {
+  if (keepsV0CredentialNames(project) || authorizerType !== "PaymentCredentialProvider") {
+    return undefined;
+  }
+  const legacy = `${project.name}_${targetName}_${name}`;
+  return credentialProviderArn.endsWith(`/${legacy}`) ? legacy : undefined;
 }
 
 const PROVIDER_NAME_MAX_LENGTH = 128;
@@ -109,7 +142,10 @@ export type CredentialProvisioner = (
 ) => AsyncGenerator<ProjectEvent, DeployedCredentials>;
 
 /** A credential's spec name and provider kind, enough to name and delete its provider. */
-export type CredentialProviderRef = Pick<Credential, "name" | "authorizerType">;
+export type CredentialProviderRef = Pick<Credential, "name" | "authorizerType"> & {
+  /** The name a payment provider was recorded under before the hyphenated scheme. */
+  legacyProvider?: string;
+};
 
 export type CredentialRemovalInput = CredentialProvisionInput & {
   /** The credentials whose providers to delete. The caller decides the set. */
@@ -129,15 +165,24 @@ export type CredentialRemover = (
  * A recorded entry without `authorizerType` was written by a CLI that named providers
  * by the bare credential name, so nothing exists under the scoped name and the entry
  * is skipped.
+ *
+ * A payment provider recorded under its underscored name is orphaned even while its
+ * credential is still declared: the deploy creates its hyphenated replacement, and
+ * the old one is deleted by the name it was recorded under.
  */
 export function orphanedCredentials(
+  project: Project,
+  targetName: string,
   recorded: DeployedCredentials,
-  declared: readonly Credential[],
 ): CredentialProviderRef[] {
-  const names = new Set(declared.map(({ name }) => name));
-  return Object.entries(recorded).flatMap(([name, { authorizerType }]) =>
-    authorizerType && !names.has(name) ? [{ name, authorizerType }] : [],
-  );
+  const names = new Set(project.spec.credentials.map(({ name }) => name));
+  return Object.entries(recorded).flatMap(([name, deployed]) => {
+    const { authorizerType } = deployed;
+    if (!authorizerType) return [];
+    const legacyProvider = legacyPaymentProviderName(project, targetName, name, deployed);
+    if (legacyProvider) return [{ name, authorizerType, legacyProvider }];
+    return names.has(name) ? [] : [{ name, authorizerType }];
+  });
 }
 
 /**
@@ -158,8 +203,9 @@ export function createCredentialRemover(identity: ProviderDeletes): CredentialRe
     { region, credentials, targetName, providers },
   ) {
     const options: CoreOptions = { region, credentials };
-    for (const { name, authorizerType } of providers) {
-      const provider = providerName(project, targetName, name);
+    for (const ref of providers) {
+      const { authorizerType } = ref;
+      const provider = ref.legacyProvider ?? providerName(project, targetName, ref);
       if (keepsV0CredentialNames(project) && authorizerType !== "PaymentCredentialProvider") {
         yield {
           type: "step",
@@ -227,7 +273,7 @@ export function createCredentialProvisioner(
     // Every name is composed and checked before the first lookup, so one that is
     // too long fails the deploy before it touches Identity.
     const named: NamedCredential[] = declared.map((credential) => {
-      const provider = providerName(project, targetName, credential.name);
+      const provider = providerName(project, targetName, credential);
       if (provider.length > PROVIDER_NAME_MAX_LENGTH) {
         throw new ProjectStateError(
           `Credential '${credential.name}' would create a credential provider named ` +