fix(cli-internal): address PR #14698 review comments

Remove unused buildNoopOperation/NO_OP_MESSAGE from _operation.ts
and _plan.ts. Restore deleted comment in _validations.ts. Rename
description to stack in forward resolveTarget. Extract resource
type constants in auth, analytics, and storage forward files and
reuse them in rollback files. Add REVIEW_IN_PROGRESS holding stack
check and duplicate logical ID error in rollback afterMove. Update
rollback test mock to differentiate Gen2 vs holding stack templates.

All 19 refactor test suites pass (118 tests).
---
Prompt: Address the comments in this PR: 14698

Author: iliapolo

packages/amplify-cli/src/__tests__/commands/gen2-migration/refactor/workflow/rollback-category-refactorer.test.ts

@@ -44,6 +44,15 @@ describe('RollbackCategoryRefactorer.afterMove', () => {
   afterEach(() => cfnMock.restore());
 
   it('returns operations to update holding stack and move resources back', async () => {
+    const gen2Template: CFNTemplate = {
+      AWSTemplateFormatVersion: '2010-09-09',
+      Description: 'gen2',
+      Resources: {
+        MigrationPlaceholder: { Type: 'AWS::CloudFormation::WaitConditionHandle', Properties: {} },
+      },
+      Outputs: {},
+    };
+
     const holdingTemplate: CFNTemplate = {
       AWSTemplateFormatVersion: '2010-09-09',
       Description: 'holding',
@@ -56,7 +65,13 @@ describe('RollbackCategoryRefactorer.afterMove', () => {
     cfnMock.on(DescribeStacksCommand).resolves({
       Stacks: [{ StackName: 'holding', StackStatus: 'UPDATE_COMPLETE', CreationTime: new Date() }],
     });
-    cfnMock.on(GetTemplateCommand).resolves({ TemplateBody: JSON.stringify(holdingTemplate) });
+    cfnMock.on(GetTemplateCommand).callsFake((input) => {
+      const stackName = input.StackName ?? '';
+      if (stackName.includes('holding')) {
+        return { TemplateBody: JSON.stringify(holdingTemplate) };
+      }
+      return { TemplateBody: JSON.stringify(gen2Template) };
+    });
     cfnMock.on(UpdateStackCommand).resolves({});
     cfnMock.on(CreateStackRefactorCommand).resolves({ StackRefactorId: 'refactor-123' });
     cfnMock.on(DescribeStackRefactorCommand).resolves({

packages/amplify-cli/src/commands/gen2-migration/_operation.ts

@@ -1,23 +1,5 @@
 import { DiscoveredResource } from './generate/_infra/gen1-app';
 
-export const NO_OP_MESSAGE = 'No-op\n';
-
-/**
- * Creates a no-op operation for a resource with nothing to migrate.
- */
-export function buildNoopOperation(resource: DiscoveredResource): AmplifyMigrationOperation {
-  return {
-    resource: resource,
-    validate: () => undefined,
-    execute: async () => {
-      return;
-    },
-    describe: async () => {
-      return [NO_OP_MESSAGE];
-    },
-  };
-}
-
 /**
  * Result of a validation check.
  */

packages/amplify-cli/src/commands/gen2-migration/_plan.ts

@@ -1,4 +1,4 @@
-import { AmplifyMigrationOperation, NO_OP_MESSAGE } from './_operation';
+import { AmplifyMigrationOperation } from './_operation';
 import { SpinningLogger } from './_spinning-logger';
 import { printer } from '@aws-amplify/amplify-prompts';
 import chalk from 'chalk';
@@ -74,8 +74,6 @@ export class Plan {
       grouped.get(label)!.push(...lines);
     }
 
-    let hasRealImplications = false;
-
     if (grouped.size > 0) {
       printer.info(chalk.bold(chalk.underline('Operations Summary')));
       printer.blankLine();
@@ -86,15 +84,12 @@ export class Plan {
         let step = 1;
         for (const description of descriptions) {
           printer.info(`${step}. ${description}`);
-          if (description !== NO_OP_MESSAGE) {
-            hasRealImplications = true;
-          }
           step++;
         }
       }
     }
 
-    if (hasRealImplications && this.implications.length > 0) {
+    if (grouped.size > 0 && this.implications.length > 0) {
       printer.info(chalk.bold(chalk.underline('Implications')));
       printer.blankLine();
       for (const implication of this.implications) {

packages/amplify-cli/src/commands/gen2-migration/_validations.ts

@@ -97,6 +97,7 @@ export class AmplifyGen2MigrationValidations {
     const statefulRemoves: Array<{ category: string; resourceType: string; physicalId: string }> = [];
     for (const change of changeSet.Changes) {
       if (change.Type === 'Resource' && change.ResourceChange?.Action === 'Remove' && change.ResourceChange?.ResourceType) {
+        // Skip deployment bucket only when explicitly requested (e.g., during decommission)
         if (
           deploymentBucketName &&
           change.ResourceChange.ResourceType === 'AWS::S3::Bucket' &&

packages/amplify-cli/src/commands/gen2-migration/refactor/analytics/analytics-forward.ts

@@ -1,6 +1,8 @@
 import { ForwardCategoryRefactorer } from '../workflow/forward-category-refactorer';
 
-export const ANALYTICS_RESOURCE_TYPES = ['AWS::Kinesis::Stream'];
+export const KINESIS_STREAM_TYPE = 'AWS::Kinesis::Stream';
+
+export const ANALYTICS_RESOURCE_TYPES = [KINESIS_STREAM_TYPE];
 
 /**
  * Forward refactorer for the analytics category (Kinesis).

packages/amplify-cli/src/commands/gen2-migration/refactor/analytics/analytics-rollback.ts

@@ -1,6 +1,6 @@
 import { CFNResource } from '../../cfn-template';
 import { RollbackCategoryRefactorer } from '../workflow/rollback-category-refactorer';
-import { ANALYTICS_RESOURCE_TYPES } from './analytics-forward';
+import { ANALYTICS_RESOURCE_TYPES, KINESIS_STREAM_TYPE } from './analytics-forward';
 
 /**
  * Rollback refactorer for the analytics category (Kinesis).
@@ -22,7 +22,7 @@ export class AnalyticsKinesisRollbackRefactorer extends RollbackCategoryRefactor
 
   protected targetLogicalId(sourceId: string, sourceResource: CFNResource): string | undefined {
     switch (sourceResource.Type) {
-      case 'AWS::Kinesis::Stream':
+      case KINESIS_STREAM_TYPE:
         return 'KinesisStream';
       default:
         return undefined;

packages/amplify-cli/src/commands/gen2-migration/refactor/auth/auth-cognito-forward.ts

@@ -20,13 +20,17 @@ export const GEN2_NATIVE_APP_CLIENT = 'UserPoolNativeAppClient';
 export const GEN2_WEB_CLIENT = 'UserPoolAppClient';
 
 export const USER_POOL_CLIENT_TYPE = 'AWS::Cognito::UserPoolClient';
+export const USER_POOL_TYPE = 'AWS::Cognito::UserPool';
+export const IDENTITY_POOL_TYPE = 'AWS::Cognito::IdentityPool';
+export const IDENTITY_POOL_ROLE_ATTACHMENT_TYPE = 'AWS::Cognito::IdentityPoolRoleAttachment';
+export const USER_POOL_DOMAIN_TYPE = 'AWS::Cognito::UserPoolDomain';
 
 export const RESOURCE_TYPES = [
-  'AWS::Cognito::UserPool',
+  USER_POOL_TYPE,
   USER_POOL_CLIENT_TYPE,
-  'AWS::Cognito::IdentityPool',
-  'AWS::Cognito::IdentityPoolRoleAttachment',
-  'AWS::Cognito::UserPoolDomain',
+  IDENTITY_POOL_TYPE,
+  IDENTITY_POOL_ROLE_ATTACHMENT_TYPE,
+  USER_POOL_DOMAIN_TYPE,
 ];
 
 /**

packages/amplify-cli/src/commands/gen2-migration/refactor/auth/auth-cognito-rollback.ts

@@ -8,6 +8,10 @@ import {
   GEN2_NATIVE_APP_CLIENT,
   GEN2_WEB_CLIENT,
   USER_POOL_CLIENT_TYPE,
+  USER_POOL_TYPE,
+  IDENTITY_POOL_TYPE,
+  IDENTITY_POOL_ROLE_ATTACHMENT_TYPE,
+  USER_POOL_DOMAIN_TYPE,
 } from './auth-cognito-forward';
 
 /**
@@ -41,13 +45,13 @@ export class AuthCognitoRollbackRefactorer extends RollbackCategoryRefactorer {
           message: `Unable to determine Gen1 logical ID for UserPoolClient '${sourceId}' — expected logical ID to contain '${GEN2_NATIVE_APP_CLIENT}' or '${GEN2_WEB_CLIENT}'`,
         });
       }
-      case 'AWS::Cognito::UserPool':
+      case USER_POOL_TYPE:
         return 'UserPool';
-      case 'AWS::Cognito::IdentityPool':
+      case IDENTITY_POOL_TYPE:
         return 'IdentityPool';
-      case 'AWS::Cognito::IdentityPoolRoleAttachment':
+      case IDENTITY_POOL_ROLE_ATTACHMENT_TYPE:
         return 'IdentityPoolRoleMap';
-      case 'AWS::Cognito::UserPoolDomain':
+      case USER_POOL_DOMAIN_TYPE:
         return 'UserPoolDomain';
       default:
         return undefined;

packages/amplify-cli/src/commands/gen2-migration/refactor/storage/storage-dynamo-forward.ts

@@ -1,5 +1,7 @@
 import { ForwardCategoryRefactorer } from '../workflow/forward-category-refactorer';
 
+export const DYNAMO_TABLE_TYPE = 'AWS::DynamoDB::Table';
+
 /**
  * Forward refactorer for DynamoDB storage resources.
  * Moves DynamoDB tables from Gen1 to Gen2.
@@ -15,6 +17,6 @@ export class StorageDynamoForwardRefactorer extends ForwardCategoryRefactorer {
   }
 
   protected resourceTypes(): string[] {
-    return ['AWS::DynamoDB::Table'];
+    return [DYNAMO_TABLE_TYPE];
   }
 }

packages/amplify-cli/src/commands/gen2-migration/refactor/storage/storage-dynamo-rollback.ts

@@ -1,5 +1,6 @@
 import { CFNResource } from '../../cfn-template';
 import { RollbackCategoryRefactorer } from '../workflow/rollback-category-refactorer';
+import { DYNAMO_TABLE_TYPE } from './storage-dynamo-forward';
 
 /**
  * Rollback refactorer for DynamoDB storage resources.
@@ -16,12 +17,12 @@ export class StorageDynamoRollbackRefactorer extends RollbackCategoryRefactorer
   }
 
   protected resourceTypes(): string[] {
-    return ['AWS::DynamoDB::Table'];
+    return [DYNAMO_TABLE_TYPE];
   }
 
   protected targetLogicalId(_sourceId: string, sourceResource: CFNResource): string | undefined {
     switch (sourceResource.Type) {
-      case 'AWS::DynamoDB::Table':
+      case DYNAMO_TABLE_TYPE:
         return 'DynamoDBTable';
       default:
         return undefined;