feat(gen2-migration): implement holding stack for Gen2 resource retention during refactor

docs/amplify-cli/src/gen2-migration/refactor.md

@@ -227,9 +227,115 @@ const resourceMapPath = this.resourceMappings.split(FILE_PROTOCOL_PREFIX)[1];
 - Multiple resources of the same type (e.g., multiple DynamoDB tables) are matched arbitrarily by type alone—the mapping may not preserve correct resource correspondence without explicit `--resourceMappings`
 - The `--resourceMappings` CLI option is currently disabled (commented out in code)
 - Auth with OAuth providers is known to be broken—fails on deployment after refactor when trying to replace IdP that already exists
-- The `rollback()` method is not implemented—it only logs 'Not implemented'. Manual intervention required on failure
 - The refactor operation has a 60-minute timeout (300 attempts × 12s)—very large stacks may timeout
 
+## Holding Stack (Gen2 Resource Retention)
+
+During forward migration, Gen2 stateful resources are moved to a temporary "holding stack" instead of being deleted. This preserves test data that customers may have created while testing the Gen2 deployment.
+
+### How It Works
+
+**Forward Migration:**
+1. Gen1 stack is pre-processed (references resolved)
+2. A holding stack is created: `{gen2CategoryStackName}-holding`
+3. Gen2 stateful resources are moved to the holding stack via StackRefactor
+4. Gen1 resources are moved to Gen2 stack via StackRefactor
+
+**Rollback:**
+1. Resources are moved from Gen2 back to Gen1 (existing logic)
+2. If a holding stack exists, resources are restored from holding stack to Gen2
+3. The empty holding stack is deleted
+
+**Cleanup:**
+After successful migration, run `amplify gen2-migration cleanup` to delete any remaining holding stacks.
+
+### Holding Stack Structure
+
+```yaml
+AWSTemplateFormatVersion: '2010-09-09'
+Description: 'Temporary holding stack for Gen2 migration'
+Metadata:
+  AmplifyMigration:
+    SourceCategoryStack: 'arn:aws:cloudformation:...'
+    Category: 'auth'
+Resources:
+  # Gen2 resources moved here via StackRefactor
+  UserPool:
+    Type: AWS::Cognito::UserPool
+    ...
+```
+
+### Key Design Decisions
+
+- **One holding stack per category**: Matches the per-category refactor flow
+- **Logical IDs preserved**: Gen2 logical IDs are kept in the holding stack, simplifying rollback
+- **Graceful rollback**: If holding stack is missing during rollback, a warning is logged and rollback continues
+- **Standalone stack**: Not nested under Gen1 or Gen2 to avoid CDK interference
+
+### Related Files
+
+| File | Purpose |
+|------|---------|
+| `holding-stack.ts` | Utilities for creating, finding, and deleting holding stacks |
+| `cleanup.ts` | Cleanup command implementation |
+
+## Known Issues — Holding Stack Implementation
+
+### ~~Critical: No recovery from partial failure (resume scenario)~~ FIXED
+
+`recoverFromHoldingStack()` in `CategoryTemplateGenerator` now detects an existing holding stack, populates `gen2ResourcesToRemove` from its contents, and returns the current Gen2 template. `processGen2Stack` calls this before attempting a new holding stack move. The redundant `generateGen2ResourceRemovalTemplate` fallback was removed.
+
+### Critical: Rollback after failed Gen1→Gen2 refactor doesn't restore holding stack
+
+In `template-generator.ts` `generateCategoryTemplates`, when the second StackRefactor (Gen1→Gen2) fails, the code calls `rollbackGen2Stack` with the original Gen2 template. But the Gen2 stack was already modified by the first StackRefactor (resources moved to holding stack). Updating the Gen2 stack with a template that references resources now owned by the holding stack will fail.
+
+**File:** `generators/template-generator.ts` — failure handler in `generateCategoryTemplates`
+
+**Fix:** Call `restoreGen2ResourcesFromHoldingStack()` before `rollbackGen2Stack()`:
+
+```typescript
+if (!isRollback && oldDestinationTemplate) {
+  await categoryTemplateGenerator.restoreGen2ResourcesFromHoldingStack();
+  await this.rollbackGen2Stack(category, destinationCategoryStackId, destinationStackParameters, oldDestinationTemplate);
+}
+```
+
+### ~~Medium: `[object Object]` displayed for implications~~ FIXED
+
+Implications display now calls `executeImplications()` / `rollbackImplications()` instead of `execute()` / `rollback()`.
+
+### Medium: Cleanup matches any stack ending with `-holding` in the account
+
+`findHoldingStacks` in `cleanup.ts` matches ALL stacks ending with `-holding`, not just Amplify migration holding stacks. This could delete unrelated stacks.
+
+**File:** `cleanup.ts` — `findHoldingStacks`
+
+**Fix:** After matching the suffix, read the stack template and verify `Metadata.AmplifyMigration` exists before including it.
+
+### ~~Medium: `deleteHoldingStack` crashes when stack is already gone~~ FIXED
+
+`deleteHoldingStack` now catches "does not exist" from `pollStackForCompletionState` and treats it as successful deletion.
+
+### ~~Low: Redundant fallback to `generateGen2ResourceRemovalTemplate`~~ FIXED
+
+Removed as part of the `processGen2Stack` rewrite. The method now throws directly if no resources are found.
+
+### Low: No stack name length validation
+
+`getHoldingStackName` appends `-holding` (8 chars) without checking the 128-character CloudFormation stack name limit.
+
+**File:** `holding-stack.ts` — `getHoldingStackName`
+
+**Fix:** Validate `stackName.length + HOLDING_STACK_SUFFIX.length <= 128` and throw if exceeded.
+
+### Cosmetic: Misleading rollback log messages
+
+In `generateCategoryTemplates`, the rollback path always logs "Restoring..." and "Restored...successfully" even when no holding stack exists. `restoreGen2ResourcesFromHoldingStack` handles this gracefully (returns early), but the surrounding log messages are misleading.
+
+**File:** `generators/template-generator.ts` — rollback path in `generateCategoryTemplates`
+
+**Fix:** Move log messages inside `restoreGen2ResourcesFromHoldingStack` or check return value.
+
 ## AI Development Notes
 
 **Important considerations:**
@@ -244,7 +350,7 @@ const resourceMapPath = this.resourceMappings.split(FILE_PROTOCOL_PREFIX)[1];
 - The `--to` parameter is required and must point to a valid Gen2 stack name—`InputValidationError` is thrown if missing
 - Resource mappings file must use `file://` protocol prefix (e.g., `file:///path/to/mappings.json`)—relative paths without protocol will fail
 - User pool groups and auth resources are in the same stack in Gen2 but different stacks in Gen1—the module handles this with 'auth-user-pool-group' category
-- Don't assume rollback works—the `rollback()` method is not implemented and only logs a message
+- Holding stacks must be cleaned up after successful migration using `amplify gen2-migration cleanup`
 
 **Testing guidance:**
 Test with deployed Amplify Gen1 projects that have auth and storage categories. Verify the assessment correctly identifies resources to migrate. Test with `--resourceMappings` to verify custom mapping support. Test OAuth migrations with social login providers (Google, Facebook, Sign In With Apple). Verify rollback behavior when refactor fails mid-operation. Test rollback operation (Gen2→Gen1) to ensure bidirectional support works.

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

@@ -3,7 +3,6 @@ import { $TSContext, AmplifyError } from '@aws-amplify/amplify-cli-core';
 import { AmplifyMigrationStep } from './gen2-migration/_step';
 import { AmplifyMigrationOperation } from './gen2-migration/_operation';
 import { printer, prompter } from '@aws-amplify/amplify-prompts';
-import { AmplifyMigrationCleanupStep } from './gen2-migration/cleanup';
 import { AmplifyMigrationDecommissionStep } from './gen2-migration/decommission';
 import { AmplifyMigrationGenerateStep } from './gen2-migration/generate';
 import { AmplifyMigrationLockStep } from './gen2-migration/lock';
@@ -14,10 +13,6 @@ import { AmplifyClient, GetAppCommand } from '@aws-sdk/client-amplify';
 import chalk from 'chalk';
 
 const STEPS = {
-  cleanup: {
-    class: AmplifyMigrationCleanupStep,
-    description: 'Not Implemented',
-  },
   clone: {
     class: AmplifyMigrationCloneStep,
     description: 'Not Implemented',

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

@@ -7,10 +7,13 @@ import {
   DescribeChangeSetCommand,
   DeleteChangeSetCommand,
   DescribeChangeSetOutput,
+  ListStacksCommand,
+  StackStatus,
   waitUntilChangeSetCreateComplete,
 } from '@aws-sdk/client-cloudformation';
 import { removeEnvFromCloud } from '../../extensions/amplify-helpers/remove-env-from-cloud';
 import { invokeDeleteEnvParamsFromService } from '../../extensions/amplify-helpers/invoke-delete-env-params';
+import { deleteHoldingStack, HOLDING_STACK_SUFFIX } from './refactor/holding-stack';
 
 export class AmplifyMigrationDecommissionStep extends AmplifyMigrationStep {
   public async executeImplications(): Promise<string[]> {
@@ -35,33 +38,65 @@ export class AmplifyMigrationDecommissionStep extends AmplifyMigrationStep {
   }
 
   public async execute(): Promise<AmplifyMigrationOperation[]> {
-    return [
-      {
-        describe: async () => {
-          return ['Delete the Gen1 environment'];
-        },
-        execute: async () => {
-          this.logger.info(`Starting decommission of environment: ${this.currentEnvName}`);
-
-          this.logger.info('Preparing to delete Gen1 resources...');
-
-          this.logger.info('Deleting Gen1 resources from the cloud. This will take a few minutes.');
-          await removeEnvFromCloud(this.context, this.currentEnvName, true);
+    const cfnClient = new CloudFormationClient({ region: this.region });
+    const holdingStacks = await this.findHoldingStacks(cfnClient);
 
-          this.logger.info('Cleaning up SSM parameters...');
-          await invokeDeleteEnvParamsFromService(this.context, this.currentEnvName);
+    const operations: AmplifyMigrationOperation[] = [];
 
-          this.logger.info('Successfully decommissioned Gen1 environment from the cloud');
-          this.logger.info(`Environment '${this.currentEnvName}' has been completely removed from AWS`);
+    if (holdingStacks.length > 0) {
+      operations.push({
+        describe: async () => holdingStacks.map((name) => `Delete holding stack: ${name}`),
+        execute: async () => {
+          for (const stackName of holdingStacks) {
+            this.logger.info(`Deleting holding stack: ${stackName}`);
+            await deleteHoldingStack(cfnClient, stackName);
+            this.logger.info(`Deleted holding stack: ${stackName}`);
+          }
         },
+      });
+    }
+
+    operations.push({
+      describe: async () => ['Delete the Gen1 environment'],
+      execute: async () => {
+        this.logger.info(`Starting decommission of environment: ${this.currentEnvName}`);
+        this.logger.info('Preparing to delete Gen1 resources...');
+        this.logger.info('Deleting Gen1 resources from the cloud. This will take a few minutes.');
+        await removeEnvFromCloud(this.context, this.currentEnvName, true);
+        this.logger.info('Cleaning up SSM parameters...');
+        await invokeDeleteEnvParamsFromService(this.context, this.currentEnvName);
+        this.logger.info('Successfully decommissioned Gen1 environment from the cloud');
+        this.logger.info(`Environment '${this.currentEnvName}' has been completely removed from AWS`);
       },
-    ];
+    });
+
+    return operations;
   }
 
   public async rollback(): Promise<AmplifyMigrationOperation[]> {
     throw new Error('Not Implemented');
   }
 
+  private async findHoldingStacks(cfnClient: CloudFormationClient): Promise<string[]> {
+    const holdingStacks: string[] = [];
+    let nextToken: string | undefined;
+    do {
+      const response = await cfnClient.send(
+        new ListStacksCommand({
+          NextToken: nextToken,
+          StackStatusFilter: [StackStatus.CREATE_COMPLETE, StackStatus.UPDATE_COMPLETE, StackStatus.ROLLBACK_COMPLETE],
+        }),
+      );
+      for (const stack of response.StackSummaries ?? []) {
+        if (stack.StackName?.endsWith(HOLDING_STACK_SUFFIX) && stack.StackName.includes(this.appId)) {
+          holdingStacks.push(stack.StackName);
+        }
+      }
+      nextToken = response.NextToken;
+    } while (nextToken);
+    return holdingStacks;
+  }
+
   private async createChangeSet(): Promise<DescribeChangeSetOutput> {
     const cfn = new CloudFormationClient({});
     const changeSetName = `decommission-${Date.now()}`;

packages/amplify-cli/src/commands/gen2-migration/refactor/cfn-stack-refactor-updater.ts

@@ -87,7 +87,10 @@ export async function tryRefactorStack(
   const sourceStackStatus = await pollStackForCompletionState(cfnClient, sourceStackName);
   assert(sourceStackStatus === CFNStackStatus.UPDATE_COMPLETE, `${sourceStackName} was not updated successfully.`);
   const destinationStackStatus = await pollStackForCompletionState(cfnClient, destinationStackName);
-  assert(destinationStackStatus === CFNStackStatus.UPDATE_COMPLETE, `${destinationStackName} was not updated successfully.`);
+  assert(
+    destinationStackStatus === CFNStackStatus.UPDATE_COMPLETE || destinationStackStatus === CFNStackStatus.CREATE_COMPLETE,
+    `${destinationStackName} was not updated successfully.`,
+  );
 
   return [true, undefined];
 }