From 2a8c55b05586948f769d8b3eab4af4f5c733cdf7 Mon Sep 17 00:00:00 2001 From: Nicolas Borges Date: Thu, 5 Nov 2026 16:46:32 -0500 Subject: [PATCH 1/2] fix(payments): preserve v0 provider names for migrated projects --- .../project/backends/cdk/credentials.test.ts | 70 +++++++++++++++++++ src/core/project/backends/cdk/credentials.ts | 31 +++++--- 2 files changed, 93 insertions(+), 8 deletions(-) diff --git a/src/core/project/backends/cdk/credentials.test.ts b/src/core/project/backends/cdk/credentials.test.ts index 1f3b56dfc..cf4423e88 100644 --- a/src/core/project/backends/cdk/credentials.test.ts +++ b/src/core/project/backends/cdk/credentials.test.ts @@ -752,6 +752,42 @@ describe("createCredentialProvisioner", () => { expect(subject.contents()[OLD_WALLET_PROVIDER]?.operation).toBeUndefined(); }); + test("ignores a recorded API key ARN when the credential changed to payment", async () => { + const subject = account( + {}, + {}, + { + AGENTCORE_CREDENTIAL_WALLET_API_KEY_ID: "key-1", + AGENTCORE_CREDENTIAL_WALLET_API_KEY_SECRET: "key-secret", + AGENTCORE_CREDENTIAL_WALLET_WALLET_SECRET: "wallet-secret", + }, + ); + const input = await project([COINBASE]); + input.spec = ProjectSpecSchema.parse({ + ...input.spec, + payments: [ + { + name: "MyPayments", + connectors: [{ name: "Coinbase", provider: "CoinbaseCDP", credentialName: "wallet" }], + }, + ], + }); + const recorded: DeployedCredentials = { + wallet: { + authorizerType: "ApiKeyCredentialProvider", + credentialProviderArn: "arn:apikey:wallet", + }, + }; + + expect(() => + assertPaymentConnectorCredentials(input, INPUT.targetName, recorded), + ).not.toThrow(); + const { result } = await run(subject.provision, input, { ...INPUT, recorded }); + + expect(Object.keys(subject.contents())).toEqual([WALLET_PROVIDER]); + expect(result.wallet?.authorizerType).toBe("PaymentCredentialProvider"); + }); + test("fails before writing when an old payment provider is not recorded for the target", async () => { const oldProvider = { credentialProviderArn: paymentArn(OLD_WALLET_PROVIDER) }; const subject = account({ [OLD_WALLET_PROVIDER]: oldProvider }); @@ -1155,6 +1191,40 @@ describe("projects migrated from v0", () => { expect(result["openai-key"]?.credentialProviderArn).toBe("arn:apikey:openai-key"); }); + test("a new target reuses the bare payment provider recorded by another target", async () => { + const arn = paymentArn("wallet"); + const subject = account({ wallet: { credentialProviderArn: arn } }); + const input = await migratedProject([COINBASE]); + + const defaultTarget = await run(subject.provision, input, { + ...INPUT, + targetName: "default", + recorded: { + wallet: { authorizerType: "PaymentCredentialProvider", credentialProviderArn: arn }, + }, + }); + const prodTarget = await run(subject.provision, input, { ...INPUT, targetName: "prod" }); + + expect(defaultTarget.result.wallet?.credentialProviderArn).toBe(arn); + expect(prodTarget.result.wallet?.credentialProviderArn).toBe(arn); + expect(Object.keys(subject.contents())).toEqual(["wallet"]); + expect(subject.contents().wallet?.operation).toBeUndefined(); + }); + + test("removes a migrated payment provider by its bare name without recorded state", async () => { + const subject = account({ wallet: { credentialProviderArn: paymentArn("wallet") } }); + + await collect( + createCredentialRemover(subject.client)(await migratedProject([]), { + ...INPUT, + providers: [COINBASE], + }), + ); + + expect(subject.deleted).toEqual([{ kind: "payment", name: "wallet" }]); + expect(subject.contents()).toEqual({}); + }); + test("never delete API key or OAuth providers, which v0 projects may share", async () => { const subject = account({ "openai-key": { credentialProviderArn: "arn:apikey:openai-key" } }); diff --git a/src/core/project/backends/cdk/credentials.ts b/src/core/project/backends/cdk/credentials.ts index 7f8d1cb0d..4d3d3194e 100644 --- a/src/core/project/backends/cdk/credentials.ts +++ b/src/core/project/backends/cdk/credentials.ts @@ -60,9 +60,10 @@ function previousProviderName( * providers keep the physical name in deployed state. New payment providers need * a name whose ARN CreatePaymentConnector can accept. * - * A raw v0 project without recorded payment state keeps v0's bare credential - * name. Other existing payment providers use the recorded physical name, so - * upgrading the CLI does not replace one that agent code may still use. + * Projects that use v0 credential names keep the bare payment credential name + * when no payment provider is recorded for this target. Existing payment + * providers use the recorded physical name, so upgrading the CLI does not + * replace one that agent code may still use. */ function providerName( project: Project, @@ -71,10 +72,10 @@ function providerName( recorded: DeployedCredentials, ): string { if (credential.authorizerType === "PaymentCredentialProvider") { - const existing = recorded[credential.name]; + const existing = recordedPaymentCredential(recorded, credential.name); if (existing) return paymentProviderNameFromArn(credential.name, existing.credentialProviderArn); - if (project.legacy) return credential.name; + if (keepsV0CredentialNames(project)) return credential.name; const parts = [project.name, targetName, credential.name]; const suffix = createHash("sha256").update(JSON.stringify(parts)).digest("hex").slice(0, 16); @@ -87,6 +88,17 @@ function providerName( const PROVIDER_NAME_MAX_LENGTH = 128; const CONNECTOR_PAYMENT_NAME_PATTERN = /^[a-zA-Z0-9.-]+$/; +function recordedPaymentCredential( + recorded: DeployedCredentials, + credentialName: string, +): DeployedCredential | undefined { + const existing = recorded[credentialName]; + if (existing?.authorizerType && existing.authorizerType !== "PaymentCredentialProvider") { + return undefined; + } + return existing; +} + function paymentProviderNameFromArn(credentialName: string, arn: string): string { const name = /\/paymentcredentialprovider\/([^/]+)$/.exec(arn)?.[1]; if (!name) { @@ -107,7 +119,7 @@ export function assertPaymentConnectorCredentials( for (const manager of project.spec.payments ?? []) { for (const connector of manager.connectors) { if (!connector.credentialName) continue; - const existing = recorded[connector.credentialName]; + const existing = recordedPaymentCredential(recorded, connector.credentialName); if (!existing) continue; const name = paymentProviderNameFromArn( connector.credentialName, @@ -216,7 +228,7 @@ export function providerRefFromRecorded( credential: CredentialProviderRef, recorded: DeployedCredentials, ): CredentialProviderRef { - const existing = recorded[credential.name]; + const existing = recordedPaymentCredential(recorded, credential.name); return existing && credential.authorizerType === "PaymentCredentialProvider" ? { name: credential.name, @@ -336,7 +348,10 @@ export function createCredentialProvisioner( for (const { credential, provider } of named) { // Lost target state must not turn an existing payment provider into an // untracked orphan while a newly named provider takes its place. - if (credential.authorizerType === "PaymentCredentialProvider" && !recorded[credential.name]) { + if ( + credential.authorizerType === "PaymentCredentialProvider" && + !recordedPaymentCredential(recorded, credential.name) + ) { const previousName = previousProviderName(project, targetName, credential.name); if ( previousName !== provider && From 8a350a31df2bbc83f1a768a499ceafe3e34477d8 Mon Sep 17 00:00:00 2001 From: Nicolas Borges Date: Thu, 5 Nov 2026 17:53:31 -0500 Subject: [PATCH 2/2] fix(payments): scope migrated names and protect shared providers Keep a migrated project's bare payment provider name when a connector can reference it. New underscored names use a connector-compatible physical name, while recorded providers keep their existing names. Leave a payment provider in place while another target records it. Preserve the lost-state error for an unrecorded underscored provider instead of adopting it. --- src/core/project/backends/cdk.test.ts | 129 ++++++++++++++++++ src/core/project/backends/cdk.ts | 73 +++++++++- .../project/backends/cdk/credentials.test.ts | 121 +++++++++++----- src/core/project/backends/cdk/credentials.ts | 84 +++++++----- 4 files changed, 338 insertions(+), 69 deletions(-) diff --git a/src/core/project/backends/cdk.test.ts b/src/core/project/backends/cdk.test.ts index 19d07cf28..c27fb626f 100644 --- a/src/core/project/backends/cdk.test.ts +++ b/src/core/project/backends/cdk.test.ts @@ -1004,6 +1004,48 @@ describe("CdkBackend.deploy", () => { ]); }); + test("teardown leaves a payment provider recorded by another target", async () => { + const input = await project(); + input.spec = ProjectSpecSchema.parse({ ...input.spec, migratedFrom: "v1" }); + await writeAssembly(input, [TARGET.name], { resources: METADATA_ONLY }); + const statePath = join(input.rootPath, DEPLOYED_STATE_RELATIVE_PATH); + await mkdir(dirname(statePath), { recursive: true }); + const wallet = { + credentialProviderArn: + "arn:aws:acps:us-east-1:111122223333:" + + "token-vault/default/paymentcredentialprovider/wallet", + authorizerType: "PaymentCredentialProvider", + }; + await writeFile( + statePath, + JSON.stringify({ + targets: { + default: { stackArn: "arn:stack:default", resources: { credentials: { wallet } } }, + prod: { stackArn: "arn:stack:prod", resources: { credentials: { wallet } } }, + }, + }), + ); + const handed: CredentialRemovalInput[] = []; + const subject = harness({ + removeCredentials: async function* (_project, removalInput) { + handed.push(removalInput); + yield { type: "step", message: "checked credential removals" }; + }, + }); + + const deployed = await collectDeploy( + subject.backend.deploy(input, deployInput({ confirmTeardown: async () => true })), + ); + + expect(handed[0]?.providers).toEqual([]); + expect(stepMessages(deployed.events)).toContain( + "Leaving credential provider 'wallet' in place: also recorded for target 'prod'", + ); + const state = JSON.parse(await Bun.file(statePath).text()); + expect(state.targets.default).toBeUndefined(); + expect(state.targets.prod.resources.credentials.wallet).toEqual(wallet); + }); + test("removes a recorded provider whose credential left the spec, after the stack update", async () => { const input = await project(); input.spec = ProjectSpecSchema.parse({ @@ -1073,6 +1115,93 @@ describe("CdkBackend.deploy", () => { expect(deployed.result).toEqual({ outputs: {} }); }); + test("deploy leaves a dropped payment provider used by another target", async () => { + const input = await project(); + input.spec = ProjectSpecSchema.parse({ ...input.spec, migratedFrom: "v1" }); + await writeAssembly(input, [TARGET.name]); + const statePath = join(input.rootPath, DEPLOYED_STATE_RELATIVE_PATH); + await mkdir(dirname(statePath), { recursive: true }); + const wallet = { + credentialProviderArn: + "arn:aws:bedrock-agentcore:us-east-1:111122223333:" + + "token-vault/default/paymentcredentialprovider/wallet", + authorizerType: "PaymentCredentialProvider", + }; + await writeFile( + statePath, + JSON.stringify({ + targets: { + default: { resources: { credentials: { wallet } } }, + prod: { resources: { credentials: { wallet } } }, + }, + }), + ); + const handed: CredentialRemovalInput[] = []; + const subject = harness({ + removeCredentials: async function* (_project, removalInput) { + handed.push(removalInput); + yield { type: "step", message: "checked credential removals" }; + }, + }); + + const deployed = await collectDeploy(subject.backend.deploy(input, deployInput())); + + expect(handed[0]?.providers).toEqual([]); + expect(stepMessages(deployed.events)).toContain( + "Leaving credential provider 'wallet' in place: also recorded for target 'prod'", + ); + const state = JSON.parse(await Bun.file(statePath).text()); + expect(state.targets.default.resources.credentials).toEqual({}); + expect(state.targets.prod.resources.credentials.wallet).toEqual(wallet); + }); + + test("deploy removes a provider when the other target's matching name is in another account", async () => { + const input = await project(); + input.spec = ProjectSpecSchema.parse({ ...input.spec, migratedFrom: "v1" }); + await writeAssembly(input, [TARGET.name]); + const statePath = join(input.rootPath, DEPLOYED_STATE_RELATIVE_PATH); + await mkdir(dirname(statePath), { recursive: true }); + const wallet = { + credentialProviderArn: + "arn:aws:bedrock-agentcore:us-east-1:111122223333:" + + "token-vault/default/paymentcredentialprovider/wallet", + authorizerType: "PaymentCredentialProvider", + }; + await writeFile( + statePath, + JSON.stringify({ + targets: { + default: { resources: { credentials: { wallet } } }, + prod: { + resources: { + credentials: { + wallet: { + ...wallet, + credentialProviderArn: + "arn:aws:bedrock-agentcore:us-west-2:444455556666:" + + "token-vault/default/paymentcredentialprovider/wallet", + }, + }, + }, + }, + }, + }), + ); + const handed: CredentialRemovalInput[] = []; + const subject = harness({ + removeCredentials: async function* (_project, removalInput) { + handed.push(removalInput); + yield { type: "step", message: "checked credential removals" }; + }, + }); + + await collectDeploy(subject.backend.deploy(input, deployInput())); + + expect(handed[0]?.providers).toEqual([ + { name: "wallet", authorizerType: "PaymentCredentialProvider", physicalName: "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 }); diff --git a/src/core/project/backends/cdk.ts b/src/core/project/backends/cdk.ts index b9e20e645..6599a0107 100644 --- a/src/core/project/backends/cdk.ts +++ b/src/core/project/backends/cdk.ts @@ -40,6 +40,7 @@ import { type CredentialProviderCalls, type CredentialProviderRef, type CredentialProvisioner, + type CredentialRemovalInput, type CredentialRemover, type DeployedCredentials, } from "./cdk/credentials"; @@ -54,6 +55,7 @@ import { removeTargetState, stackReferenceOf, updateTargetState, + type DeployedState, } from "./cdk/deployedState"; import { bootstrapStackReader, @@ -112,6 +114,32 @@ function stackExportName(stackName: string, ...parts: string[]): string { .join("-"); } +function targetsRecordingPaymentProvider( + state: DeployedState, + target: AwsDeploymentTarget, + provider: CredentialProviderRef, +): string[] { + if (provider.authorizerType !== "PaymentCredentialProvider") return []; + const physicalName = provider.physicalName ?? provider.name; + return Object.entries(state.targets).flatMap(([name, saved]) => { + if (name === target.name) return []; + const sharesProvider = Object.values(saved.resources?.credentials ?? {}).some((credential) => { + if (credential.authorizerType && credential.authorizerType !== "PaymentCredentialProvider") { + return false; + } + const arn = credential.credentialProviderArn; + const parts = arn.split(":"); + return ( + (parts[2] === "bedrock-agentcore" || parts[2] === "acps") && + parts[3] === target.region && + parts[4] === target.account && + arn.endsWith(`/paymentcredentialprovider/${physicalName}`) + ); + }); + return sharesProvider ? [name] : []; + }); +} + function findDeployedResourceId( stack: Stack, input: Pick, @@ -273,9 +301,8 @@ export class CdkBackend implements ProjectBackend { // Read before provisioning rewrites the credentials map: it is the only record // of what this target provisioned, so it is the only way to find a provider whose // credential has since left the spec. - const recorded = - (await readDeployedState(this.json, project.rootPath)).targets[target.name]?.resources - ?.credentials ?? {}; + const deployedState = await readDeployedState(this.json, project.rootPath); + const recorded = deployedState.targets[target.name]?.resources?.credentials ?? {}; assertPaymentConnectorCredentials(project, target.name, recorded); const orphaned = orphanedCredentials(recorded, project.spec.credentials); @@ -302,7 +329,15 @@ export class CdkBackend implements ProjectBackend { // with nothing in it as an instruction to delete the stack, and reports that // as an ordinary successful deploy. if ((await countDeployableResources(this.json, assemblyDirectory, artifact)) === 0) { - return yield* this.teardown({ project, artifact, input, options, orphaned, recorded }); + return yield* this.teardown({ + project, + artifact, + input, + options, + orphaned, + recorded, + deployedState, + }); } // Only after a real deploy is confirmed: a teardown (empty assembly) returns @@ -403,7 +438,7 @@ export class CdkBackend implements ProjectBackend { // After the stack update, since a resource in it may have been using the provider // until this deploy removed the reference. - yield* this.removeCredentials(project, { + yield* this.removeUnsharedCredentials(project, target, deployedState, { credentials, region: target.region, targetName: target.name, @@ -437,6 +472,7 @@ export class CdkBackend implements ProjectBackend { options, orphaned, recorded, + deployedState, }: { project: Project; artifact: StackArtifact; @@ -445,6 +481,7 @@ export class CdkBackend implements ProjectBackend { /** Providers recorded for this target before the deploy that the spec no longer declares. */ orphaned: CredentialProviderRef[]; recorded: DeployedCredentials; + deployedState: DeployedState; }): AsyncGenerator { const { target } = input; if (!(await this.describeStack(target.region, options.credentials, artifact.stackName))) { @@ -475,7 +512,7 @@ export class CdkBackend implements ProjectBackend { // After the stack, since a resource in it may still be using the provider. The // declared credentials are included because `agentcore remove all` empties the spec // before the deploy that gets here, so what it recorded is all that names them. - yield* this.removeCredentials(project, { + yield* this.removeUnsharedCredentials(project, target, deployedState, { credentials: options.credentials, region: target.region, targetName: target.name, @@ -490,6 +527,30 @@ export class CdkBackend implements ProjectBackend { return { outputs: {}, tornDown: true }; } + private async *removeUnsharedCredentials( + project: Project, + target: AwsDeploymentTarget, + deployedState: DeployedState, + input: CredentialRemovalInput, + ): AsyncGenerator { + const providers: CredentialProviderRef[] = []; + for (const provider of input.providers) { + const otherTargets = targetsRecordingPaymentProvider(deployedState, target, provider); + if (otherTargets.length === 0) { + providers.push(provider); + continue; + } + yield { + type: "step", + message: + `Leaving credential provider '${provider.physicalName ?? provider.name}' in place: ` + + `also recorded for target${otherTargets.length === 1 ? "" : "s"} ` + + otherTargets.map((name) => `'${name}'`).join(", "), + }; + } + yield* this.removeCredentials(project, { ...input, providers }); + } + /** * Runs one Toolkit operation with its progress streamed as `output` events. * The trailing lines are also kept so a failure can carry them: the Toolkit's diff --git a/src/core/project/backends/cdk/credentials.test.ts b/src/core/project/backends/cdk/credentials.test.ts index cf4423e88..38114e3ad 100644 --- a/src/core/project/backends/cdk/credentials.test.ts +++ b/src/core/project/backends/cdk/credentials.test.ts @@ -752,41 +752,43 @@ describe("createCredentialProvisioner", () => { expect(subject.contents()[OLD_WALLET_PROVIDER]?.operation).toBeUndefined(); }); - test("ignores a recorded API key ARN when the credential changed to payment", async () => { - const subject = account( - {}, - {}, - { - AGENTCORE_CREDENTIAL_WALLET_API_KEY_ID: "key-1", - AGENTCORE_CREDENTIAL_WALLET_API_KEY_SECRET: "key-secret", - AGENTCORE_CREDENTIAL_WALLET_WALLET_SECRET: "wallet-secret", - }, - ); - const input = await project([COINBASE]); - input.spec = ProjectSpecSchema.parse({ - ...input.spec, - payments: [ + for (const authorizerType of ["ApiKeyCredentialProvider", undefined] as const) { + test(`ignores an API key ARN (${authorizerType ?? "untyped"}) when changing to payment`, async () => { + const subject = account( + {}, + {}, { - name: "MyPayments", - connectors: [{ name: "Coinbase", provider: "CoinbaseCDP", credentialName: "wallet" }], + AGENTCORE_CREDENTIAL_WALLET_API_KEY_ID: "key-1", + AGENTCORE_CREDENTIAL_WALLET_API_KEY_SECRET: "key-secret", + AGENTCORE_CREDENTIAL_WALLET_WALLET_SECRET: "wallet-secret", }, - ], - }); - const recorded: DeployedCredentials = { - wallet: { - authorizerType: "ApiKeyCredentialProvider", - credentialProviderArn: "arn:apikey:wallet", - }, - }; + ); + const input = await project([COINBASE]); + input.spec = ProjectSpecSchema.parse({ + ...input.spec, + payments: [ + { + name: "MyPayments", + connectors: [{ name: "Coinbase", provider: "CoinbaseCDP", credentialName: "wallet" }], + }, + ], + }); + const recorded: DeployedCredentials = { + wallet: { + ...(authorizerType && { authorizerType }), + credentialProviderArn: "arn:apikey:wallet", + }, + }; - expect(() => - assertPaymentConnectorCredentials(input, INPUT.targetName, recorded), - ).not.toThrow(); - const { result } = await run(subject.provision, input, { ...INPUT, recorded }); + expect(() => + assertPaymentConnectorCredentials(input, INPUT.targetName, recorded), + ).not.toThrow(); + const { result } = await run(subject.provision, input, { ...INPUT, recorded }); - expect(Object.keys(subject.contents())).toEqual([WALLET_PROVIDER]); - expect(result.wallet?.authorizerType).toBe("PaymentCredentialProvider"); - }); + expect(Object.keys(subject.contents())).toEqual([WALLET_PROVIDER]); + expect(result.wallet?.authorizerType).toBe("PaymentCredentialProvider"); + }); + } test("fails before writing when an old payment provider is not recorded for the target", async () => { const oldProvider = { credentialProviderArn: paymentArn(OLD_WALLET_PROVIDER) }; @@ -1211,6 +1213,63 @@ describe("projects migrated from v0", () => { expect(subject.contents().wallet?.operation).toBeUndefined(); }); + test("a new underscored payment credential deploys with a connector-compatible name", async () => { + const subject = account( + {}, + {}, + { + AGENTCORE_CREDENTIAL_WALLET_US_API_KEY_ID: "key-1", + AGENTCORE_CREDENTIAL_WALLET_US_API_KEY_SECRET: "key-secret", + AGENTCORE_CREDENTIAL_WALLET_US_WALLET_SECRET: "wallet-secret", + }, + ); + const input = await migratedProject([{ ...COINBASE, name: "wallet_us" }]); + input.spec = ProjectSpecSchema.parse({ + ...input.spec, + payments: [ + { + name: "MyPayments", + connectors: [{ name: "Coinbase", provider: "CoinbaseCDP", credentialName: "wallet_us" }], + }, + ], + }); + + const { result } = await run(subject.provision, input); + const [name] = Object.keys(subject.contents()); + + expect(name).toMatch(/^example-dev-wallet-us-[a-f0-9]{16}$/); + expect(result.wallet_us?.authorizerType).toBe("PaymentCredentialProvider"); + }); + + test("an underscored payment credential keeps its recorded bare provider", async () => { + const arn = paymentArn("wallet_us"); + const subject = account({ wallet_us: { credentialProviderArn: arn } }); + const input = await migratedProject([{ ...COINBASE, name: "wallet_us" }]); + + const { result } = await run(subject.provision, input, { + ...INPUT, + recorded: { + wallet_us: { authorizerType: "PaymentCredentialProvider", credentialProviderArn: arn }, + }, + }); + + expect(result.wallet_us?.credentialProviderArn).toBe(arn); + expect(Object.keys(subject.contents())).toEqual(["wallet_us"]); + expect(subject.contents().wallet_us?.operation).toBeUndefined(); + }); + + test("does not adopt an unrecorded underscored provider", async () => { + const arn = paymentArn("wallet_us"); + const subject = account({ wallet_us: { credentialProviderArn: arn } }); + const input = await migratedProject([{ ...COINBASE, name: "wallet_us" }]); + + await expect(run(subject.provision, input)).rejects.toThrow( + /no provider recorded for target 'dev'.*provider 'wallet_us' exists in AWS/s, + ); + expect(Object.keys(subject.contents())).toEqual(["wallet_us"]); + expect(subject.contents().wallet_us?.operation).toBeUndefined(); + }); + test("removes a migrated payment provider by its bare name without recorded state", async () => { const subject = account({ wallet: { credentialProviderArn: paymentArn("wallet") } }); diff --git a/src/core/project/backends/cdk/credentials.ts b/src/core/project/backends/cdk/credentials.ts index 4d3d3194e..c23485e70 100644 --- a/src/core/project/backends/cdk/credentials.ts +++ b/src/core/project/backends/cdk/credentials.ts @@ -60,10 +60,10 @@ function previousProviderName( * providers keep the physical name in deployed state. New payment providers need * a name whose ARN CreatePaymentConnector can accept. * - * Projects that use v0 credential names keep the bare payment credential name - * when no payment provider is recorded for this target. Existing payment - * providers use the recorded physical name, so upgrading the CLI does not - * replace one that agent code may still use. + * Raw v0 projects keep their bare credential names. Migrated projects keep + * connector-compatible bare names. Existing payment providers use their + * recorded physical names, so upgrading the CLI does not replace one that + * agent code may still use. */ function providerName( project: Project, @@ -75,7 +75,12 @@ function providerName( const existing = recordedPaymentCredential(recorded, credential.name); if (existing) return paymentProviderNameFromArn(credential.name, existing.credentialProviderArn); - if (keepsV0CredentialNames(project)) return credential.name; + if ( + project.legacy || + (keepsV0CredentialNames(project) && CONNECTOR_PAYMENT_NAME_PATTERN.test(credential.name)) + ) { + return credential.name; + } const parts = [project.name, targetName, credential.name]; const suffix = createHash("sha256").update(JSON.stringify(parts)).digest("hex").slice(0, 16); @@ -96,11 +101,23 @@ function recordedPaymentCredential( if (existing?.authorizerType && existing.authorizerType !== "PaymentCredentialProvider") { return undefined; } + // Older state omits the kind; its ARN must still identify a payment provider. + if ( + existing?.authorizerType === undefined && + existing && + !paymentProviderNameInArn(existing.credentialProviderArn) + ) { + return undefined; + } return existing; } +function paymentProviderNameInArn(arn: string): string | undefined { + return /\/paymentcredentialprovider\/([^/]+)$/.exec(arn)?.[1]; +} + function paymentProviderNameFromArn(credentialName: string, arn: string): string { - const name = /\/paymentcredentialprovider\/([^/]+)$/.exec(arn)?.[1]; + const name = paymentProviderNameInArn(arn); if (!name) { throw new ProjectStateError( `Credential '${credentialName}' has an invalid recorded payment provider ARN '${arn}' ` + @@ -241,14 +258,15 @@ export function providerRefFromRecorded( /** * Deletes the given credentials' providers under their target-scoped names. * - * Every kind is deleted. A provider's name is scoped to this project and target, so - * nothing outside this target can be using it. A provider that is already gone is - * not an error, and a provider that cannot be deleted is reported rather than - * failing the deploy that already changed the stack. + * Every kind handed to the remover is deleted. The caller excludes payment + * providers still recorded by another target before reaching this function. + * A provider that is already gone is not an error, and a provider that cannot + * be deleted is reported rather than failing the deploy that already changed + * the stack. * * A v0 project's providers carry the bare credential name, so another project in the * account can be using the same API key or OAuth provider; those are never deleted. - * Payment providers are deleted, as v0's own teardown deleted them. + * Unshared payment providers are deleted, as v0's own teardown deleted them. */ export function createCredentialRemover(identity: ProviderDeletes): CredentialRemover { return async function* removeCredentials( @@ -341,32 +359,34 @@ export function createCredentialProvisioner( return { credential, provider }; }); + // Lost target state must not turn an existing payment provider into an + // untracked orphan while a newly named provider takes its place. + for (const { credential, provider } of named) { + if ( + credential.authorizerType !== "PaymentCredentialProvider" || + recordedPaymentCredential(recorded, credential.name) + ) { + continue; + } + const previousName = previousProviderName(project, targetName, credential.name); + if (previousName === provider) continue; + const previous = await undefinedWhenAbsent(() => + identity.getPaymentCredentialProvider(previousName, options), + ); + if (!previous) continue; + throw new ProjectStateError( + `Payment credential '${credential.name}' has no provider recorded for target ` + + `'${targetName}' in agentcore/.cli/deployed-state.json, but provider ` + + `'${previousName}' exists in AWS. Restore the target's deployed-state.json ` + + `entry, or delete that provider if unused, then re-deploy.`, + ); + } + // Resolve every credential before writing any: look up existing providers and // validate the secret each one needs. A missing secret then fails before the // first provider is written, not partway through the list. const plans: (NamedCredential & { provision: Provision })[] = []; for (const { credential, provider } of named) { - // Lost target state must not turn an existing payment provider into an - // untracked orphan while a newly named provider takes its place. - if ( - credential.authorizerType === "PaymentCredentialProvider" && - !recordedPaymentCredential(recorded, credential.name) - ) { - const previousName = previousProviderName(project, targetName, credential.name); - if ( - previousName !== provider && - (await undefinedWhenAbsent(() => - identity.getPaymentCredentialProvider(previousName, options), - )) - ) { - throw new ProjectStateError( - `Payment credential '${credential.name}' has no provider recorded for target ` + - `'${targetName}' in agentcore/.cli/deployed-state.json, but provider ` + - `'${previousName}' exists in AWS. Restore the target's deployed-state.json ` + - `entry, or delete that provider if unused, then re-deploy.`, - ); - } - } plans.push({ credential, provider,