fix(toolkit-lib): clean up the diff change set when early validation fails

packages/@aws-cdk/toolkit-lib/lib/api/deployments/cfn-api.ts

@@ -223,33 +223,51 @@ async function createChangeSetAndCleanup(
   });
 
   await ioHelper.defaults.debug(format('Initiated creation of changeset: %s; waiting for it to finish creating...', changeSet.Id));
-  // Fetching all pages if we'll execute, so we can have the correct change count when monitoring.
-  const createdChangeSet = await new ChangeSetDescriber({
-    cfn: options.cfn,
-    ioHelper,
-    stackNameOrArn: changeSet.StackId ?? options.stack.stackName,
-    changeSetNameOrArn: changeSet.Id ?? options.changeSetName,
-  }).waitAndThrowOnProblem({
-    diagnoser: options.diagnoser,
-  });
 
-  await cleanupOldChangeset(
-    options.cfn,
-    ioHelper,
-    changeSet.Id ?? options.changeSetName,
-    changeSet.StackId ?? options.stack.stackName,
-  );
+  const changeSetId = changeSet.Id ?? options.changeSetName;
+  const stackId = changeSet.StackId ?? options.stack.stackName;
+
+  // Remove the change set (and, for a brand new stack, the empty stack that a
+  // CREATE change set leaves in REVIEW_IN_PROGRESS). This has to run whether the
+  // change set succeeds or fails validation: otherwise a change set that fails
+  // early validation is orphaned and leaves the stack stuck in REVIEW_IN_PROGRESS,
+  // which then blocks subsequent change set creation.
+  const cleanup = async () => {
+    await cleanupOldChangeset(options.cfn, ioHelper, changeSetId, stackId);
+
+    if (!options.exists) {
+      await ioHelper.defaults.debug(format('Deleting empty stack created by diff changeset: %s', stackId));
+      await options.cfn.deleteStack({
+        StackName: stackId,
+        ClientRequestToken: randomUUID(),
+      });
+    }
+  };
 
-  // If the stack didn't exist before, creating a CREATE changeset will have
-  // put it in REVIEW_IN_PROGRESS state. Delete the empty stack to clean up.
-  if (!options.exists) {
-    await ioHelper.defaults.debug(format('Deleting empty stack created by diff changeset: %s', changeSet.StackId ?? options.stack.stackName));
-    await options.cfn.deleteStack({
-      StackName: changeSet.StackId ?? options.stack.stackName,
-      ClientRequestToken: randomUUID(),
+  let createdChangeSet: ChangeSetReport;
+  try {
+    // Fetching all pages if we'll execute, so we can have the correct change count when monitoring.
+    createdChangeSet = await new ChangeSetDescriber({
+      cfn: options.cfn,
+      ioHelper,
+      stackNameOrArn: stackId,
+      changeSetNameOrArn: changeSetId,
+    }).waitAndThrowOnProblem({
+      diagnoser: options.diagnoser,
     });
+  } catch (e) {
+    // Best-effort cleanup so a failed change set doesn't leak; don't let a
+    // cleanup failure mask the original creation/validation error.
+    try {
+      await cleanup();
+    } catch (cleanupError) {
+      await ioHelper.defaults.debug(format('Failed to clean up change set after a creation error: %s', cleanupError));
+    }
+    throw e;
   }
 
+  await cleanup();
+
   return createdChangeSet;
 }
 

packages/@aws-cdk/toolkit-lib/test/actions/diff.test.ts

@@ -1,5 +1,5 @@
 import * as path from 'path';
-import { CreateChangeSetCommand, DeleteStackCommand, DescribeChangeSetCommand, DescribeStacksCommand, GetTemplateCommand, ListStacksCommand } from '@aws-sdk/client-cloudformation';
+import { CreateChangeSetCommand, DeleteChangeSetCommand, DeleteStackCommand, DescribeChangeSetCommand, DescribeStacksCommand, GetTemplateCommand, ListStacksCommand } from '@aws-sdk/client-cloudformation';
 import { GetParameterCommand } from '@aws-sdk/client-ssm';
 import chalk from 'chalk';
 import { DiffMethod } from '../../lib/actions/diff';
@@ -443,6 +443,42 @@ describe('diff', () => {
       })).rejects.toThrow(/Could not create a change set, and '--method=change-set' was specified/);
     });
 
+    test('ChangeSet diff cleans up the failed change set and empty stack when validation fails', async () => {
+      // GIVEN - a new stack whose CREATE change set fails early validation
+      // (e.g. a resource that already exists), which would otherwise leave the
+      // change set orphaned and the stack stuck in REVIEW_IN_PROGRESS.
+      jest.spyOn(deployments.Deployments.prototype, 'stackExists').mockResolvedValue(false);
+      mockCloudFormationClient.on(DescribeStacksCommand).resolves({ Stacks: [] });
+      mockSSMClient.on(GetParameterCommand).resolves({ Parameter: { Value: '99' } });
+      mockCloudFormationClient.on(CreateChangeSetCommand).resolves({
+        Id: 'arn:aws:cloudformation:us-east-1:123456789012:changeSet/cdk-diff-change-set/abc',
+        StackId: 'arn:aws:cloudformation:us-east-1:123456789012:stack/Stack1/def',
+      });
+      mockCloudFormationClient.on(DescribeChangeSetCommand).resolves({
+        Status: 'FAILED',
+        StatusReason: "Resource of type 'AWS::S3::Bucket' with identifier 'mybucket' already exists.",
+        ExecutionStatus: 'UNAVAILABLE',
+        Changes: [],
+      });
+
+      // WHEN - the diff falls back to a template diff (default fallbackToTemplate = true)
+      const cx = await cdkOutFixture(toolkit, 'stack-with-bucket');
+      await toolkit.diff(cx, {
+        stacks: { strategy: StackSelectionStrategy.ALL_STACKS },
+        method: DiffMethod.ChangeSet(),
+      });
+
+      // THEN - the failed change set is deleted and the empty stack is cleaned up
+      const deleteChangeSetCalls = mockCloudFormationClient.commandCalls(DeleteChangeSetCommand);
+      expect(deleteChangeSetCalls.length).toBeGreaterThan(0);
+      expect(deleteChangeSetCalls[0].args[0].input).toEqual(expect.objectContaining({
+        ChangeSetName: expect.stringContaining('cdk-diff-change-set'),
+      }));
+
+      const deleteStackCalls = mockCloudFormationClient.commandCalls(DeleteStackCommand);
+      expect(deleteStackCalls.length).toBeGreaterThan(0);
+    });
+
     test('ChangeSet diff method creates changeset for new stacks when fallBackToTemplate = false', async () => {
       // GIVEN - stack doesn't exist
       jest.spyOn(deployments.Deployments.prototype, 'stackExists').mockResolvedValue(false);