fixes after merge

packages/@aws-cdk/toolkit-lib/lib/api/refactoring/context.ts

@@ -5,12 +5,12 @@ import { ResourceLocation, ResourceMapping } from './cloudformation';
 import type { GraphDirection } from './digest';
 import { computeResourceDigests } from './digest';
 import { ToolkitError } from '../../toolkit/toolkit-error';
+import { equalSets } from '../../util/sets';
 import type { SDK } from '../aws-auth/sdk';
 import type { SdkProvider } from '../aws-auth/sdk-provider';
 import { EnvironmentResourcesRegistry } from '../environment';
 import type { IoHelper } from '../io/private';
 import { Mode } from '../plugin';
-import { equalSets } from '../../util/sets';
 
 /**
  * Represents a set of possible moves of a resource from one location
@@ -159,18 +159,6 @@ function resourceMoves(
   return Object.values(removeUnmovedResources(zip(digestsBefore, digestsAfter)));
 }
 
-/**
- * Whether two sets of resources have the same elements (uniquely identified by the digest), and
- * each element is in the same number of locations. The locations themselves may be different.
- */
-function isomorphic(a: Record<string, ResourceLocation[]>, b: Record<string, ResourceLocation[]>): boolean {
-  const sameKeys = equalSets(new Set(Object.keys(a)), new Set(Object.keys(b)));
-  return sameKeys && Object.entries(a).every(([digest, locations]) => locations.length === b[digest].length);
-}
-
-  return Object.values(removeUnmovedResources(zip(digestsBefore, digestsAfter)));
-}
-
 /**
  * Whether two sets of resources have the same elements (uniquely identified by the digest), and
  * each element is in the same number of locations. The locations themselves may be different.

packages/@aws-cdk/toolkit-lib/lib/api/refactoring/index.ts

@@ -6,8 +6,6 @@ import {
 } from '@aws-cdk/cloudformation-diff';
 import type * as cxapi from '@aws-cdk/cx-api';
 import type { StackSummary } from '@aws-sdk/client-cloudformation';
-import { minimatch } from 'minimatch';
-import { major } from 'semver';
 import { deserializeStructure, indexBy } from '../../util';
 import type { SdkProvider } from '../aws-auth/private';
 import { Mode } from '../plugin';

packages/@aws-cdk/toolkit-lib/lib/api/refactoring/stack-definitions.ts

@@ -15,37 +15,36 @@ export function generateStackDefinitions(
   // overwriting its CDKMetadata resource with the one from the deployed stack
   for (const localStack of localStacks) {
     const deployedStack = deployedStackMap.get(localStack.stackName);
-    if (deployedStack) {
-      const localTemplate = localStack.template;
-      const deployedTemplate = deployedStack.template;
-      localTemplate.Resources = localTemplate.Resources ?? {};
-      deployedTemplate.Resources = deployedTemplate.Resources ?? {};
+    const localTemplate = localStack.template;
+    const deployedTemplate = deployedStack?.template;
 
-      // We need to preserve whatever CDKMetadata we had before.
-      // Otherwise, CloudFormation will consider this a modification.
-      if (deployedTemplate.Resources?.CDKMetadata != null) {
-        localTemplate.Resources.CDKMetadata = deployedTemplate.Resources.CDKMetadata;
-      } else {
-        delete localTemplate.Resources.CDKMetadata;
-      }
+    // The CDKMetadata resource is never part of a refactor. So at this point we need
+    // to adjust the template we will send to the API to make sure it has the same CDKMetadata
+    // as the deployed template. And if the deployed template doesn't have any, we cannot
+    // send any either.
+    if (deployedTemplate?.Resources?.CDKMetadata != null) {
+      localTemplate.Resources = localTemplate.Resources ?? {};
+      localTemplate.Resources.CDKMetadata = deployedTemplate.Resources.CDKMetadata;
+    } else {
+      delete localTemplate.Resources?.CDKMetadata;
+    }
 
-      // For every resource in the local template, take the Metadata['aws:cdk:path'] from the corresponding resource in the deployed template.
-      // A corresponding resource is one that the local maps to (using the `mappings` parameter). If there is no entry mapping the local
-      // resource, use the same id
-      // TODO Remove this logic once CloudFormation starts allowing changes to the construct path.
-      //  But we need it for now, otherwise we won't be able to refactor anything.
-      for (const [logicalId, localResource] of Object.entries(localTemplate.Resources)) {
-        const mapping = mappings.find(
-          (m) => m.destination.stackName === localStack.stackName && m.destination.logicalResourceId === logicalId,
-        );
+    // For every resource in the local template, take the Metadata['aws:cdk:path'] from the corresponding resource in the deployed template.
+    // A corresponding resource is one that the local maps to (using the `mappings` parameter). If there is no entry mapping the local
+    // resource, use the same id
+    // TODO Remove this logic once CloudFormation starts allowing changes to the construct path.
+    //  But we need it for now, otherwise we won't be able to refactor anything.
+    for (const [logicalId, localResource] of Object.entries(localTemplate.Resources ?? {})) {
+      const mapping = mappings.find(
+        (m) => m.destination.stackName === localStack.stackName && m.destination.logicalResourceId === logicalId,
+      );
 
-        if (mapping != null) {
-          const deployed = deployedStackMap.get(mapping.source.stackName)!;
-          const deployedResource = deployed.template?.Resources?.[mapping.source.logicalResourceId]!;
-          if (deployedResource.Metadata != null || localResource.Metadata != null) {
-            localResource.Metadata = localResource.Metadata ?? {};
-            localResource.Metadata['aws:cdk:path'] = deployedResource?.Metadata?.['aws:cdk:path'];
-          }
+      if (mapping != null) {
+        const deployed = deployedStackMap.get(mapping.source.stackName)!;
+        const deployedResource = deployed.template?.Resources?.[mapping.source.logicalResourceId]!;
+        if (deployedResource.Metadata != null || localResource.Metadata != null) {
+          localResource.Metadata = localResource.Metadata ?? {};
+          localResource.Metadata['aws:cdk:path'] = deployedResource?.Metadata?.['aws:cdk:path'];
         }
       }
     }

packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts

@@ -11,7 +11,7 @@ import { NonInteractiveIoHost } from './non-interactive-io-host';
 import type { ToolkitServices } from './private';
 import { assemblyFromSource } from './private';
 import { ToolkitError } from './toolkit-error';
-import type { FeatureFlag, DeployResult, DestroyResult, RollbackResult } from './types';
+import type { DeployResult, DestroyResult, FeatureFlag, RollbackResult } from './types';
 import type {
   BootstrapEnvironments,
   BootstrapOptions,
@@ -64,11 +64,7 @@ import {
   formatEnvironmentSectionHeader,
   formatTypedMappings,
   groupStacks,
-  ManifestExcludeList,
-  usePrescribedMappings,
-  groupStacks,
 } from '../api/refactoring';
-import type { ResourceMapping } from '../api/refactoring/cloudformation';
 import type { CloudFormationStack } from '../api/refactoring/cloudformation';
 import { ResourceMapping, ResourceLocation } from '../api/refactoring/cloudformation';
 import { RefactoringContext } from '../api/refactoring/context';
@@ -1101,20 +1097,30 @@ export class Toolkit extends CloudAssemblySourceBuilder {
           refactorResult.ambiguousPaths = paths;
         }
 
+        await ioHelper.notify(IO.CDK_TOOLKIT_I8900.msg(refactorMessage, refactorResult));
+
+        if (options.dryRun || context.mappings.length === 0) {
+          // Nothing left to do.
+          continue;
+        }
+
         // In interactive mode (TTY) we need confirmation before proceeding
         if (process.stdout.isTTY && !await confirm(options.force ?? false)) {
-          await notifyInfo(chalk.red(`Refactoring canceled for environment aws://${environment.account}/${environment.region}\n`));
+          await ioHelper.defaults.info(chalk.red(`Refactoring canceled for environment aws://${environment.account}/${environment.region}\n`));
           continue;
         }
 
-        await notifyInfo('Refactoring...');
+        await ioHelper.defaults.info('Refactoring...');
         await context.execute(stackDefinitions, sdkProvider, ioHelper);
-        await notifyInfo('✅  Stack refactor complete');
+        await ioHelper.defaults.info('✅  Stack refactor complete');
 
         await ioHelper.notify(IO.CDK_TOOLKIT_I8900.msg(refactorMessage, refactorResult));
       } catch (e: any) {
         const message = `❌  Refactor failed: ${formatError(e)}`;
         await ioHelper.notify(IO.CDK_TOOLKIT_E8900.msg(message, { error: e }));
+
+        // Also debugging the error, because the API does not always return a user-friendly message
+        await ioHelper.defaults.debug(e.message);
       }
     }
 
@@ -1157,6 +1163,19 @@ export class Toolkit extends CloudAssemblySourceBuilder {
       }
     }
 
+    async function confirm(force: boolean): Promise<boolean> {
+      // 'force' is set to true is the equivalent of having pre-approval for any refactor
+      if (force) {
+        return true;
+      }
+
+      const question = 'Do you wish to refactor these resources?';
+      const response = await ioHelper.requestResponse(IO.CDK_TOOLKIT_I8910.req(question, {
+        responseDescription: '[Y]es/[n]o',
+      }, 'y'));
+      return ['y', 'yes'].includes(response.toLowerCase());
+    }
+
     function formatError(error: any): string {
       try {
         const payload = JSON.parse(error.message);

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

@@ -7,8 +7,6 @@ import {
   ListStacksCommand,
 } from '@aws-sdk/client-cloudformation';
 import { GetCallerIdentityCommand } from '@aws-sdk/client-sts';
-import { MappingSource, type RefactorOptions, Toolkit } from '../../lib';
-import { GetTemplateCommand, ListStacksCommand } from '@aws-sdk/client-cloudformation';
 import { type RefactorOptions, StackSelectionStrategy, Toolkit } from '../../lib';
 import { SdkProvider } from '../../lib/api/aws-auth/private';
 import { builderFixture, TestIoHost } from '../_helpers';
@@ -358,15 +356,6 @@ test('detects modifications to the infrastructure', async () => {
   );
 });
 
-test('fails when dry-run is false', async () => {
-  const cx = await builderFixture(toolkit, 'stack-with-bucket');
-  await expect(
-    toolkit.refactor(cx, {
-      dryRun: false,
-    }),
-  ).rejects.toThrow('Refactor is not available yet. Too see the proposed changes, use the --dry-run flag.');
-});
-
 test('overrides can be used to resolve ambiguities', async () => {
   // GIVEN
   mockCloudFormationClient.on(ListStacksCommand).resolves({

packages/aws-cdk/lib/cli/cli-type-registry.json

@@ -924,6 +924,11 @@
           "type": "boolean",
           "default": false,
           "desc": "If specified, the command will revert the refactor operation. This is only valid if a mapping file was provided."
+        },
+        "force": {
+          "type": "boolean",
+          "default": false,
+          "desc": "Whether to do the refactor without asking for confirmation"
         }
       }
     },