Skip to content

Commit 954c1db

Browse files
feat(gen2-migration): assessment validation, credential loading, and infra reorganization (#14719)
* feat(cli-internal): add assess, refactor, and generate-new commands Squash of all work on the migration-plan branch since diverging from gen2-migration. Includes the assess subcommand for migration readiness, the refactor command rebuild with category-specific forward/rollback refactorers, the generate-new infrastructure with Generator+Renderer pattern, unified validation model, SpinningLogger UX, and comprehensive unit tests. --- Prompt: squash all commits after the merge base with gen2-migration into one and commit * chore: fix test * test(cli-internal): replace null-as-any logger with noOpLogger helper Add a noOpLogger() test helper that creates a real SpinningLogger in debug mode, then replace all `null as any` logger arguments across 8 refactor test files with it. This improves type safety without changing test behavior since the logger methods are never exercised in these tests. All 30 tests pass. --- Prompt: In the refactor test directory there are lot of null as any being used to pass a spinning logger instance - change it to actually create a proper logger instance. * feat(cli-internal): print validation report on failure Plan.validate() now captures the report field from ValidationResult and renders a "Failed Validations Report" section before the summary table. Each failed validation shows its description in red followed by the report text. Also trims the drift report in _validations.ts. --- Prompt: The report property in ValidationResult is currently not used at all. We should use to print the validation report in case the validation failed. * refactor(cli-internal): classify auth stacks by resource type Replace description-JSON-based auth stack classification with resource-type detection. The new approach checks for the presence of an AWS::Cognito::UserPool resource instead of parsing the stack Description field, which is more reliable. Also rename fetchStackDescription to fetchStack and descriptionCache to stackCache for accuracy since the method returns the full Stack object. --- Prompt: commit what I did * refactor(cli-internal): split auth refactorers and improve resource mapping Split monolithic auth-forward/rollback/utils into separate files for Cognito and UserPoolGroups, enabling independent forward and rollback refactoring per auth sub-resource. Replace gen1LogicalIds map with abstract targetLogicalId() method on RollbackCategoryRefactorer, giving each subclass explicit control over logical ID resolution. Extract match() hook on ForwardCategoryRefactorer for type-matching customization. Thread DiscoveredResource through CategoryRefactorer base class so refactorers can use resource metadata (e.g. resourceName) for stack discovery instead of relying on shared utility functions. Minor fixes to migration app docs and sanitize script (trailing newline normalization). --- Prompt: commit what I have * revert(cli-internal): remove trailing-newline normalization from sanitize --- Prompt: I reset the changes. just commit. * chore: remove merge markers * chore: cleanup * feat(cli-internal): group plan operations by resource Thread DiscoveredResource through all resource-backed planners so each operation carries the resource it belongs to. Plan.describe() now groups operations under resource headers using the format "<resourceName> (<category>/<service>)", matching the assessment display style. Ungrouped operations (scaffolding, validations) render as a flat list. Changes: - Add optional `resource` field to AmplifyMigrationOperation - Update Plan.describe() to group by resource label - Thread DiscoveredResource into all generate-side planners (Auth, ReferenceAuth, Data, S3, DynamoDB, RestApi, Function, AnalyticsKinesis) replacing separate resourceName params - Tag refactor-side operations via CategoryRefactorer and its forward/rollback subclasses (already had this.resource) - Update all affected test files with DiscoveredResource objects --- Prompt: in the gen2-migration, i want to make the plan describe itself by listing the description of each operation per resource. * fix(cli-internal): tweak plan resource group display Change label format to "category/resourceName (service)", add cyan color to group headers, remove indentation on grouped items, and add blank lines between groups for readability. --- Prompt: i've made changes * feat(cli-internal): refine plan display and support UserPool Groups rollback Group all operations under labeled sections — resource-backed ops use "Resource: category/name (service)", ungrouped ops fall under "Project". Descriptions rendered in gray for visual hierarchy. Add auth:Cognito-UserPool-Groups support in refactor assess and rollback using AuthUserPoolGroupsRollbackRefactorer. --- Prompt: I've made more changes. commit * docs: add commit OOM prevention and scratch file cleanup Add NODE_OPTIONS="--max-old-space-size=8192" to the commit command example and instructions to delete the scratch commit message file after a successful commit. --- Prompt: add an instruction in AGENTS.md to delete the commit file after committing and always increase memory size to prevent lint failures * feat(cli-internal): add changeset preview and move table to refactor plan Enrich the refactor plan output with changeset reports and formatted move tables so operators can review exactly what each operation will change before executing. Key changes: - Auth cognito: explicit client matching (GEN1_WEB_CLIENT ↔ GEN2_WEB_CLIENT, GEN1_NATIVE_APP_CLIENT ↔ GEN2_NATIVE_APP_CLIENT) replacing negation-based logic. Exported shared constants. - Auth user pool groups: extracted RESOURCE_TYPES constant, use USER_POOL_GROUP_TYPE consistently. - category-refactorer: added changeset preview via CreateChangeSetCommand/DescribeChangeSetCommand, made updateSource/updateTarget/buildMoveOperations/beforeMovePlan async, enriched plan descriptions with changeset reports and move tables. - forward/rollback-category-refactorer: updated to async signatures, added move table formatting to descriptions. - Removed validateSingleResourcePerCategory from refactor.ts. - Plan output now uses numbered steps and bold labels. - New files: changeset-report.ts, template-diff.ts, move-table.ts (formatting utilities). - Test stubs updated for new CFN commands. --- Prompt: I've made changes - commit what i've done. dont run tests or anything, just commit. * fix(cli-internal): improve changeset report formatting Use full JSON path (Target.Path) instead of just the top-level property name so duplicate property names like RoleMappings are distinguishable. Show before/after values on separate lines for readability. Use bgGray chalk headers for operation descriptions. Minor spacing tweaks in plan output and move table. --- Prompt: I've made more changes. Commit them. not tests. * refactor(cli-internal): use cli-table3 for move table and fix changeset no-changes detection Replace hand-rolled box-drawing move table with cli-table3 (CLITable) to match existing patterns. Fix changeset no-changes detection: a CREATE_COMPLETE changeset with an empty Changes list is the actual no-changes case, not a waiter failure. formatChangeSetReport now returns undefined when there are no changes. Remove debug 'bubu' suffix from cfn-output-resolver. --- Prompt: commit * feat(cli-internal): validate stack updates via changeset during refactor plan Move changeset creation into the validation lifecycle of updateSource/updateTarget operations. formatChangeSetReport returns undefined when no changes are detected. The validation checks report === undefined (valid) and surfaces the changeset report on failure. The describe output shows the report regardless. Removed unused chalk and formatTemplateDiff imports. --- Prompt: Commit. Don't run tests yet. * test(cli-internal): fix gen2-migration refactor tests Add CreateChangeSetCommand/DescribeChangeSetCommand mocks to the CloudFormationMock framework and individual test files that call plan(). Update tests for API changes: renamed module paths (auth-forward → auth-cognito-forward), new abstract targetLogicalId method on RollbackCategoryRefactorer, async beforeMovePlan, updated error message format, and Cognito-UserPool-Groups now being supported. Remove dead auth-utils.test.ts for deleted module. All 376 gen2-migr * refactor(cli-internal): improve refactor workflow resilience Improve category refactorer resilience for partial failure recovery and multi-stack auth scenarios: - Handle empty change-sets gracefully when source/target templates match deployed state (partial failure recovery) - Support reusing existing holding stacks in forward path for auth's two-gen1-stack-to-one-gen2-stack mapping - Consolidate rollback restore-from-holding into a single operation instead of three separate ops - Add logging before stack update/move/refactor operations - Improve plan step formatting (remove extra blank lines, add trailing newline to move table) - Use clearer descriptions for empty change-set validation --- Prompt: commit my changes * refactor(cli-internal): add physicalResourceId to MoveMapping Move physicalResourceId onto MoveMapping so it is populated once during buildResourceMappings and carried through the entire refactor pipeline. This eliminates redundant fetchStackResources calls in buildMoveOperations and the separate physicalIds/types maps that were threaded to formatMoveTable. - buildResourceMappings is now async; forward fetches from gen1Env, rollback from gen2Branch. - buildBlueprint is now async to await buildResourceMappings. - Deleted move-table.ts; renderMappingTable is now a protected method on CategoryRefactorer accepting MoveMapping[]. --- Prompt: in category-refactorer - I want to add the physical resource id to MoveMapping. Also make formatMoveTable accept MoveMapping[] and remove the unnecessary maps being passed to it. Remove move-table.ts and put formatMoveTable into a protected method inside CategoryRefactorer. Rename formatMoveTable to renderMappingTable. * refactor(cli-internal): hoist computation out of callbacks and simplify error handling Move all non-mutating work out of execute/describe/validate callbacks so errors surface during planning before any mutations run. tryRefactorStack and tryUpdateStack now throw on failure instead of returning result objects, eliminating boilerplate checks at every call site. createChangeSetReport now cleans up its changeset via try/finally. Deleted unused legacy-custom-resource.ts and template-diff.ts. --- Prompt: hoist computation out of execute callbacks, make tryRefactorStack and tryUpdateStack throw on failure, createChangeSetReport should delete its changeset, remove legacy-custom-resource.ts and template-diff.ts. * refactor(cli-internal): consolidate CFN operations into Cfn class Introduce a Cfn class that centralizes all CloudFormation operations (update, refactor, createChangeSet, findStack, deleteStack, renderChangeSet) behind a single client instance. Replace custom polling with SDK waiters (waitUntilStackUpdateComplete, waitUntilStackRefactorCreate/ ExecuteComplete, waitUntilStackDeleteComplete). Delete refactorer.ts (re-export of Planner), holding-stack.ts, cfn-stack-updater.ts, cfn-stack-refactor-updater.ts, changeset-report.ts, and snap.ts. Move getHoldingStackName and HOLDING_STACK_SUFFIX into CategoryRefactorer. Inline snapshot writing into cfn.ts. --- Prompt: consolidate 3 CFN operations into a Cfn class, replace custom polling with SDK waiters, remove refactorer.ts, holding-stack.ts, snap.ts, changeset-report.ts, inline snap into cfn.ts, remove resolveStackName, move ensureOutputDirectory to constructor. * refactor(cli-internal): add SpinningLogger to Cfn class Cfn now accepts a SpinningLogger and logs info messages before every wait operation (stack update, refactor create/execute, source/destination verification, stack deletion). --- Prompt: the cfn class should accept the spinning logger and log info whenever it is waiting on something. * refactor(cli-internal): clean up refactorer operations Split rollback holding stack update into its own operation with a validation that the changeset only adds the placeholder. Split forward holding stack deletion into a separate operation. Remove redundant fetchStackResources calls by deriving physical IDs from blueprint mappings. Move description/header construction into describe callbacks and ResourceMapping construction into execute callbacks. Add Cfn.fetchTemplate method. Remove unused imports. --- Prompt: split holding stack operations, add validation, remove redundant fetches, move descriptions into describe callbacks, add Cfn.fetchTemplate. * refactor(cli-internal): harden refactor workflow for multi-stack moves Add stack-level deduplication to prevent duplicate updates when multiple refactorers target the same stack. Thread targetStackId through buildResourceMappings for better error messages. Rework forward beforeMove to incrementally build holding stack templates by fetching existing state. In rollback, defer template computation into the execute closure and add duplicate-resource detection. Remove non-null assertions on StackResource fields. --- Prompt: commit everything I did. don't run tests. * refactor(cli-internal): improve refactor logging, remove caching, add noop handling Add resource-scoped log prefixes to Cfn operations so each category/resource pair is identifiable in output. Remove StackFacade caching layer so every call fetches fresh state from CloudFormation. Introduce buildNoopOperation and suppress the Implications section when all operations are no-ops. In rollback, skip resources that already exist in the target stack instead of throwing. Reduce max wait time from 3600s to 900s and pre-check destination stack existence to select the correct waiter. --- Prompt: commit everything I did. Don't run tests. just commit. * fix(cli-internal): defer template resolution to execution time in refactor workflow RefactorBlueprint now carries only mappings and stack IDs. Templates are fetched and resolved fresh inside each operation's execute() closure, so sequential refactorers targeting the same stack always see current state. This fixes the stale template bug where the second auth refactorer (user-pool-groups) would operate on a Gen2 template that the first refactorer (cognito) had already mutated. updateSource/updateTarget use plan-time resolved stacks directly (still fresh since they run before any moves). updateSource now accepts mappings to determine if a placeholder is needed. move(), beforeMove() (forward), and afterMove() (rollback) all re-fetch and re-resolve templates at execution time. --- Prompt: defer template resolution to execution time in refactor workflow to fix stale template bug when two Gen1 stacks map to the same Gen2 stack. * refactor(cli-internal): move template manipulation into Cfn.refactor and use SDK ResourceMapping Cfn.refactor() now accepts ResourceMapping[] directly, fetches both stack templates, moves resources between them, and handles the full refactor lifecycle internally. This eliminates template manipulation from callers entirely. Replace custom MoveMapping with the SDK's ResourceMapping type throughout the workflow. Simplify move(), beforeMove() (forward), and afterMove() (rollback) to just pass resource mappings. Remove fetchHoldingStackTemplate, isPlaceholderOnlyChangeSet, and the holding stack changeset validation. Move placeholder logic into addPlaceHolderIfNeeded() at the top of plan(). Fix symmetricDifference check to compare .size === 0. --- Prompt: Read what i've done and commit it. * refactor(cli-internal): use SDK ResourceMapping and strip all DependsOn Replace custom MoveMapping with the SDK ResourceMapping type. Cfn.refactor() now accepts ResourceMapping[] directly, fetches both stack templates internally, and moves resources between them before calling the refactor API. Simplify resolveDependencies to unconditionally strip all DependsOn from templates. DependsOn only controls deployment ordering which is irrelevant during refactor since all resources already exist. This also eliminates the partial-view problem where each refactorer only resolved dependencies for its own resource types. Remove resourceIds computation from resolveSource/resolveTarget since resolveDependencies no longer needs it. --- Prompt: use SDK ResourceMapping, move template manipulation into Cfn.refactor, strip all DependsOn unconditionally. * fix(cli-internal): handle absent holding stacks and defer template fetch in rollback Check target stack existence before fetching its template in Cfn.refactor(). Use an empty holding template when the target stack doesn't exist yet (only holding stacks may be absent). Defer holdingTemplate fetch into the execute closure in rollback afterMove so it reads fresh state. Improve error messages to show resource spec instead of class name. --- Prompt: I've made changes. Commit. * refactor(cli-internal): share Cfn instance, enable rollback resolution, add resource logging Share a single Cfn instance across all refactorers. Move update dedup from StackFacade into the workflow: updateSource/updateTarget check cfn.isUpdateClaimed() and call cfn.claimUpdate() at plan time to prevent duplicate operations in the plan. Enable resolveSource/resolveTarget and updateSource/updateTarget in rollback (previously skipped). This fixes the Fn::GetAtt dangling reference error when rolling back after a Gen2 redeploy. Thread DiscoveredResource into Cfn.update(), refactor(), and deleteStack() for resource-scoped log prefixes. --- Prompt: share Cfn instance, enable rollback resolution, add resource logging, plan-time update dedup. * test(cli-internal): update refactor tests for new interfaces Update all refactor test files to match the new API: SDK ResourceMapping instead of MoveMapping, slim RefactorBlueprint with only mappings + stack IDs, shared Cfn instance passed to constructors, and resolveDependencies taking no resource IDs. Update rollback plan test to expect updateSource/updateTarget operations (rollback now resolves and updates both stacks). Remove holding-stack.test.ts (module no longer exists). Rewrite build-refactor-templates.test.ts as minimal constant tests since buildBlueprint was removed. Update refactor.md docs to reflect current architecture. --- Prompt: make all the tests compile, update docs. * fix(cli-internal): prevent test hangs with missing mocks and async fixes Add DeleteChangeSetCommand mock to OAuth test. Make testBuildResourceMappings async and await all callers since buildResourceMappings is async. Convert sync throw assertions to async rejects.toThrow. --- Prompt: make sure the test won't hang by configuring all necessary mocks. * test(cli-internal): fix all refactor tests — 17 suites, 92 tests pass Delete tests for removed modules (cfn-stack-updater, cfn-stack-refactor-updater, legacy-custom-resource). Fix category-plan-orchestration constructor calls with shared Cfn. Update rollback assertions to expect no-op when resources already exist in target. Add default DescribeStacksCommand mocks for holding stack lookups. Mock SDK waiters in snapshot tests to avoid 30s polling delays. Handle missing template files in test framework GetTemplateCommand mock. Update snapshot files for DependsOn stripping. Add early return in beforeMove for empty mappings. Replace symmetricDifference with simple every() check in addPlaceHolderIfNeeded. --- Prompt: start running tests and fix what's needed. * docs(cli-internal): update JSDoc for refactor workflow changes Remove stale "Cached" references from StackFacade. Fix class-level JSDoc in CategoryRefactorer and RollbackCategoryRefactorer to match current method names. Add JSDoc to buildNoopOperation, buildResourceMappings, match, targetLogicalId, updateStackClaims, and createInfrastructure. --- Prompt: compare our code against the gen2-migration branch and make all necessary JSDoc changes. * docs(cli-internal): fix remaining JSDoc inaccuracies Fix afterMove JSDoc to not mention holding stack deletion. Fix "resources resources" typo in UserPoolGroups rollback. --- Prompt: do another pass. * test(cli-internal): make CloudFormation mock stateful with template map Replace invocation counter with a _templateForStack map pre-populated from snapshot files. DescribeStacks and GetTemplate now read from the map. CreateStackRefactor writes both stack templates to the map on create. UpdateStack writes the new template body to the map. DescribeStacks throws ValidationError for stacks not in the map, and returns minimal metadata for dynamically created stacks that lack snapshot files. --- Prompt: commit my changes. * test(cli-internal): reorganize refactor tests to mirror source structure Move test files into subdirectories matching the source layout: resolvers/, auth/, workflow/. Merge auth-forward-mapping tests into auth/auth-cognito-forward.test.ts. Merge build-refactor-templates tests into workflow/category-refactorer.test.ts. Split default-resource-mappings tests into the forward and rollback workflow test files. Delete flat test files that were replaced. Update all import paths for the new directory depth. --- Prompt: reorganize refactor tests to mirror source structure. * chore: revert snapshots * fix(cli-internal): clone empty holding template to prevent cross-refactorer leak EMPTY_HOLDING_TEMPLATE was a shared mutable object. When Cfn.refactor() used it as the target template for a new holding stack, it mutated the shared object by adding resources. Subsequent refactorers that also needed a new holding stack would get the already-mutated object, causing auth resources to leak into the storage holding stack. Fix by cloning the constant before use. --- Prompt: inspect snapshot changes and explain auth resources in storage holding stack. * docs: add coding guideline for module-level mutable constants Add "Don't mutate module-level constant objects" to the Mutability section of CODING_GUIDELINES.md. This pattern caused a real bug where EMPTY_HOLDING_TEMPLATE was shared across refactorers and accumulated resources from previous calls. --- Prompt: review session and add needed guidelines. * chore(cli-internal): remove duplicate ResourceMapping, add dictionary words Remove local ResourceMapping interface from category-refactorer and use the SDK type directly. Add refactorer, refactorers, and changeset to the eslint spellcheck dictionary. --- Prompt: remove ResourceMapping interface, add dictionary words. * style(cli-internal): consistent property order in ResourceMapping objects Reorder ResourceMapping properties to StackName before LogicalResourceId for consistency across forward and rollback refactorers. Update snapshot mapping files accordingly. --- Prompt: I've made some more changes - commit. * chore: regen fitness tracker snapshots * chore: update snapshots * refactor(cli-internal): simplify beforeMove/afterMove to take stack ID Replace RefactorBlueprint parameter with gen2StackId string in beforeMove and afterMove. Both methods now fetch templates and build resource mappings independently from the blueprint, reading the actual stack state at plan time. This decouples holding stack operations from the main move mappings. Add info/debug helpers to CategoryRefactorer base class. Move empty-mappings guard from plan() into move(). Add debug logging to beforeMove and afterMove for traceability. Update fitness-tracker generate snapshots for resource naming. --- Prompt: commit my changes. * test(cli-internal): fix tests for beforeMove/afterMove signature change Update beforeMove tests to pass gen2StackId string and mock GetTemplateCommand for Gen2 template fetch. Update afterMove tests to pass gen2StackId string. Update plan orchestration tests for new operation counts (no early no-op return). --- Prompt: run all cli package tests and fix what's needed. * chore: recapture snapshot * chore: update snapshots * docs(cli-internal): update refactor.md for beforeMove/afterMove changes Update flowcharts and plan() lifecycle to reflect that beforeMove and afterMove now independently discover resources from stack templates rather than using blueprint mappings. --- Prompt: run the PR stage from AGENTS.md. * test(cli-internal): add buildResourceMappings edge case tests Add tests for multiple matching targets, empty source, resource already in target (rollback skip), and empty rollback source. Remove unused CFNResource import from category-refactorer test. --- Prompt: add missing buildResourceMappings tests. * test(cli-internal): add targetLogicalId and match tests for all refactorers Add targetLogicalId tests for auth-cognito-rollback, auth-user-pool-groups-rollback, storage-rollback, storage-dynamo-rollback, and analytics-rollback. Add GroupName-based match tests for auth-user-pool-groups-forward. Add forward edge case tests for duplicate targets and ambiguous same-type matching. Fix bare throw in auth-cognito-rollback to use AmplifyError. Remove unused AmplifyError import from storage-dynamo-rollback. --- Prompt: add targetLogicalId and match tests for all refactorers, fix bare throw. * docs(cli-internal): document resource mapping happy and unhappy paths Add Resource Mapping section to refactor.md explaining forward type-based matching with usedTargetIds dedup, rollback targetLogicalId-based mapping, and all error conditions. --- Prompt: explain happy and unhappy paths of building resource mappings in refactor.md. * fix(cli-internal): skip holding stack resources already present in beforeMove beforeMove now checks the holding stack template before building resource mappings. Resources that already exist in the holding stack are skipped to handle re-execution of forward after a partial failure. Fix test mock to return empty template for REVIEW_IN_PROGRESS holding stacks. --- Prompt: fix REVIEW_IN_PROGRESS holding stack test failure. * feat(cli-internal): delete holding stack after rollback if only placeholder remains After moving resources out of the holding stack during rollback, check if only the migration placeholder resource remains. If so, delete the holding stack. This cleanup happens at execution time since each refactorer moves its own resources independently. --- Prompt: commit what I did. * chore: bring back discussions from gen2-migration * chore: update snapshots * fix(cli-internal): align tests with gen2-migration merge changes Three test files were broken after merging the gen2-migration branch: - category-refactorer.test.ts: fetchSourceStackId now uses 'storage' + resourceName as the prefix. Changed resourceName from 'test' to 'avatars' to match the mock's 'storageavatars' logical ID. - backend.generator.test.ts: Removed ensureStorageStack tests since that method was replaced by createDynamoDBStack (which already has its own tests). - dynamodb.generator.test.ts: Constructor now takes a DiscoveredResource instead of a string. Updated two call sites to pass full DiscoveredResource objects. All 125 test suites (716 tests) pass. --- Prompt: I merged code from the gen2-migration branch and now some cli package tests are failing. run and fix. * fix(cli-internal): drop removed hasS3Bucket param from DynamoDB tests The DynamoDBGenerator constructor dropped the hasS3Bucket parameter but the test file still passed it as a 4th argument. Removed the extra argument from all constructor calls and deleted the now-irrelevant "creates per-table stack regardless of hasS3Bucket flag" test. All 125 test suites (715 tests) pass. --- Prompt: run tests and fix * refactor(cli-internal): remove hasS3Bucket param from DynamoDBGenerator The DynamoDBGenerator now creates per-table stacks via createDynamoDBStack, making the hasS3Bucket flag unnecessary. Removed the parameter from the constructor and updated the call site in generate.ts. --- Prompt: there are still uncomitted changes * feat(cli-internal): add feature-level assessment to assess command Introduce per-category Assessor classes that detect unsupported Gen1 sub-features (overrides.ts, custom-policies.json) and report them in a new Features table alongside the existing Resources table. Consolidate SupportResponse and FeatureSupport into a single SupportLevel type union. Stateless categories (function, AppSync, API Gateway) report 'not-applicable' for refactor. The assess command now orchestrates directly via assessors instead of delegating to the generate/refactor steps' assess() methods, which have been removed. Tables use CLITable for rendering. --- Prompt: Add feature-level assessment to the assess command. Detect overrides.ts and custom-policies.json usage per resource. Display a Features table with Feature, Path, Generate, and Refactor columns. Consolidate SupportResponse and FeatureSupport into a single SupportLevel type-union. * docs: add commit message path and case rules to AGENTS.md Document two pitfalls: the -F path in git commit resolves relative to cwd (not workspace root), and commitlint requires lowercase subject lines. --- Prompt: Add instructions to AGENTS.md about your commitlint issue and the commit message file path problem. * refactor(cli-internal): replace Assessment map with arrays Change _resources from a Map to an array and replace the two-call record() API with a single recordResource() that takes generate and refactor levels together. Rename the entries getter to resources. Both _resources and _features are now consistently arrays. --- Prompt: In Assessment class, why is _features an array but _resources a map? seems strange. * docs: add "keep parallel structures symmetric" guideline Add a coding guideline about maintaining symmetry between sibling fields, methods, and data paths that serve analogous roles. Uses the Assessment class (Map vs Array) as the concrete example. --- Prompt: Add a coding guideline about keeping parallel structures symmetric. * refactor(cli-internal): align assessment API symmetry Extract DiscoveredFeature interface with name/path fields, make recordResource take a single ResourceAssessment object (symmetric with recordFeature), rename parameter from assessment to resource, and move SupportLevel to _assessment.ts. Update symmetry coding guideline with method signature and parameter naming examples. --- Prompt: Fix assessment API symmetry issues: recordResource vs recordFeature argument count, parameter naming, and DiscoveredFeature extraction. Update coding guideline. * refactor(cli-internal): make assessor reusable by generate and refactor steps Extract assess(resource) as a public method on AmplifyMigrationAssessor that returns an Assessment for a single resource. Change Assessment.display() to render() returning a string. The assessor now takes Gen1App in the constructor instead of creating it internally, so generate/refactor steps can instantiate it with their own Gen1App and call assess() per resource to check for unsupported features during validation. --- Prompt: Make the generate and refactor steps able to call the assessor on a specific resource and inspect the assessment for unsupported features as a validation. * feat(cli-internal): add feature assessment validation to generate and refactor Both the generate and refactor steps now run feature assessment as a validation before execution. Each step instantiates AmplifyMigrationAssessor with its Gen1App, assesses all discovered resources, and checks for unsupported features relevant to its step (generate checks generate support, refactor checks refactor support). Unsupported features are reported as a validation failure listing each feature and its file path. --- Prompt: Add assessment validation to generate and refactor. * refactor(cli-internal): use per-resource assessment validations Replace bulk validateFeatures methods with per-resource assessment validations in both generate and refactor steps. Each resource is assessed individually during planning, and a validation operation is added only for resources with unsupported features. The validation report uses Assessment.render() instead of custom report building. --- Prompt: validateFeatures should accept a single resource and create a validation operation per resource. Use render() for the report instead of custom logic. * chore: merge cleanup * refactor(cli-internal): consolidate step constructor to accept Gen1App Replace the 7-parameter AmplifyMigrationStep constructor (logger, envName, appName, appId, rootStackName, region, context) with a 3-parameter version (logger, gen1App, context). Gen1App now encapsulates all app state and is created once in the top-level dispatcher. Key changes: - Add appName to Gen1App and Gen1CreateOptions - Rename SUPPORTED_RESOURCE_KEYS to KNOWN_RESOURCE_KEYS and 'unsupported' ResourceKey to 'UNKNOWN' - Move Gen1App.create() to the dispatcher, pass instance to steps - Refactor Assessment to support step-scoped validation via validFor() and reportFor() - Replace per-resource feature assessment ops with a single Assessment validation operation per step - Add geo assessors (Map, PlaceIndex, GeofenceCollection) - Update generate/refactor tests to use mockGen1App() helper instead of spying on Gen1App.create --- Prompt: I made some changes and now refactor.test.ts and generate.test.ts don't compile. Fix them by creating a proper mock for Gen1App. * test(cli-internal): add assessment validation unit tests Add forward/rollback unit tests to both generate.test.ts and refactor.test.ts that verify plan.validate() fails when the assessment contains unsupported resources (UNKNOWN key) and passes when all resources are supported. Uses geo:Map for refactor and auth:Cognito-UserPool-Groups for generate since these are assessed as supported and don't trigger deep generator/refactorer pipelines. Also renames describe blocks from 'execute()' to 'forward()' to match the actual method name, and adds cfn to the createInfrastructure mock. --- Prompt: Write a unit in refactor.test.ts that ensures that validates fail if assessment fails. Now do the same for generate. In refactor, the "does not throw for stateless-only resources" should assert on the fact plan.validate passed - like the other test. Also this is missing from generate. The test still use "describe('execute()', ()" even though the method is called "forward". * refactor(cli-internal): fix remaining test failures and simplify assessment Fix all broken tests caused by the Gen1App constructor refactor: - lock.test.ts: use Gen1App mock instead of 7-arg constructor - _assessment.test.ts: rewrite for new validFor/render(step) API - assess.test.ts: replace removed assessFeatures() with assess() - function.assessor.test.ts: fix expected support levels - rollback-category-refactorer.test.ts: update afterMove assertion to expect 1 operation (placeholder cleanup moved into execute) Also includes user changes to assessors, AwsClients, Gen1App, refactor step, and dispatcher simplification. --- Prompt: fix the pre-existing failure as well / commit everything I have so far * fix(cli-internal): fix all test failures from Gen1App/AwsClients refactor Update all test files to match the refactored constructor signatures: - CategoryRefactorer: replace (clients, region) args with gen1App mock across 10 refactor test files - AuthCognitoForwardRefactorer: replace (clients, region, appId, envName) with gen1App mock - AwsClients: make loadConfiguration a dynamic import to avoid FeatureFlags side-effect, cast through `as any` in tests for private constructor - Assessment: re-add resources/features getters for test access - Assessor tests: add fileExists mock for DynamoDB/RestApi, update feature name expectations to match KNOWN_FEATURES enum - Snapshot tests: add MigrationApp.createGen1App() to bypass Gen1App.create() which requires real AWS credentials - Update _assessment.test.ts snapshots for new table format All 134 suites / 744 tests pass. --- Prompt: Now run tests and fix * refactor(cli-internal): rename assessment module and assessor method Move _assessment.ts to assess/assessment.ts and rename the Assessor interface method from assess() to record(). Update all test imports and method calls to match. --- Prompt: I made some renames and moved stuff around. run tests and commit. * refactor(cli-internal): replace SupportLevel strings with Support type Replace bare 'supported'/'unsupported'/'not-applicable' string literals with a Support interface containing level and optional note. Each assessor now provides its own note for unsupported entries instead of relying on hardcoded labels in the renderer. Add supported(), unsupported(note), notApplicable() helper functions for concise assessor code. Update all assessor tests to assert on .level property and use Support objects in recordResource/recordFeature calls. --- Prompt: change the 'SupportLevel' type to "Support" that has "level" and "note" - use the note instead of the hardcoded "unsupportedLabel" we use now for refactor and generate. I want for each assessor to be able to record its own note. * fix(cli-internal): remove bogus default cases from exhaustive switches Replace default return values with eslint-disable for consistent-return. The compiler already guarantees exhaustiveness via the union type — a default case with a made-up value is misleading. --- Prompt: don't do that. There is a coding guideline against it. the violation was eslint being too restrictive, the compiler ensures that no other case can happen - just disable the violation for those lines. * chore(cli-internal): misc fixes and AGENTS.md PR stage update - Fix FunctionAssessor to record supported (was temp unsupported) - Fix GeoFenceCollectionAssessor note to standard message - Switch AwsClients dynamic import to require - Add "Final Review" step to AGENTS.md PR stage --- Prompt: commit everything and run the PR stage. * docs(cli-internal): update gen2-migration docs and add PR body Update gen2-migration.md for Gen1App facade, new constructor signature, and assessment notes. Rewrite assess.md for the new assessor architecture, Support type, and geo resources. Generate PR body. --- Prompt: commit everything and run the PR stage. * docs: refine AGENTS.md PR stage instructions Clarify PR stage purpose, rename "Final Review" to "Code Review", and add instruction to ask for target branch and inspect the full diff. --- Prompt: I've made some changes to AGENTS.md / commit them. * fix(cli-internal): code review fixes for assessment refactor Restore `as const` on KNOWN_RESOURCE_KEYS to preserve exhaustive switch type checking. Fix validateLockStatus() in refactor.ts passing rootStackName twice instead of (rootStackName, envName). Fix copy-paste JSDoc on three geo assessors that incorrectly said "DynamoDB storage resource". Fix typo "assessement". Convert all single-line JSDoc comments to multi-line format per coding guidelines. --- Prompt: Run the PR stage. * docs: write scratch files to repo root Update AGENTS.md to instruct that both .commit-message.ai-generated.txt and .pr-body.ai-generated.md must be written to the repository root, not inside package directories. This prevents stale files from accumulating in subdirectories. --- Prompt: Add a similar instruction for the commit message file. * refactor(cli-internal): centralize AWS SDK clients and validations Consolidate all direct AWS SDK client instantiations into AwsClients. Add STSClient to AwsClients. Update _validations.ts to accept a CloudFormationClient via constructor instead of creating its own. Update lock.ts, decommission.ts, and refactor.ts to use gen1App.clients.*. Make the dispatcher instantiate AmplifyGen2MigrationValidations once and pass it to every step via the base class constructor. Steps now use this.validations instead of creating their own instances. Updated all tests accordingly. --- Prompt: Make the dispatcher also instantiate AmplifyGen2MigrationValidations and pass it to every step. * refactor(cli-internal): simplify AmplifyGen2MigrationValidations constructor Accept Gen1App directly instead of disparate properties (rootStackName, envName, cloudFormation). Extract the instantiation in the dispatcher to a named constant. --- Prompt: AmplifyGen2MigrationValidations should directly accept a Gen1App instead of disparate properties from it. * refactor(cli-internal): move infra files to _infra/ and fix imports Move shared infrastructure files (_step, _operation, _plan, _spinning-logger, _validations, planner, aws-clients, cfn-template, categories, stateful-resources) into a new _infra/ subdirectory. Remove clone.ts and shift.ts stubs. Move refactor step from refactor/refactor.ts to refactor.ts. Fix all broken import paths across 51 files including drift-detection modules. Type AwsClients constructor with Partial<AwsSdkConfig> instead of any. Export AwsSdkConfig from amplify-provider-awscloudformation. --- Prompt: I've moved and renamed a bunch of files. Run yarn build and fix the compilation errors. * fix(cli-internal): skip empty tables in assessment render Only print the Resources and Features tables when they contain entries. Previously, empty tables with just headers were always rendered. --- Prompt: In the rendering of the assessment, lets only print the tables if they are not empty. * chore: remove unused `AwsClients` import from category refactorer Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> * refactor(cli-internal): type AwsClients config with AmplifyClientConfig Replace Partial<AwsSdkConfig> with AmplifyClientConfig from the SDK. Add customUserAgent to the client config for user-agent tracking. Move constructExeInfo into the try block. Fix stale import paths in test framework app.ts. --- Prompt: Ok commit what i've done. * fix(cli-internal): validate team-provider-info.json exists before reading Throw a clear error with resolution guidance when team-provider-info.json is missing, instead of letting stateManager throw a generic error. Use relative path in error messages for readability. --- Prompt: made some more changes. commit. * fix(cli-internal): validate environment exists in team-provider-info Add explicit null check for envInfo before accessing awscloudformation. Throws a clear error when the target environment is not found in team-provider-info.json, instead of crashing with 'Cannot read properties of undefined'. --- Prompt: the change I just made also resolves issue 14590 * fix(cli-internal): fix stale import paths in test files Update test imports for files moved to _infra/ (spinning-logger, validations, aws-clients, cfn-template). Update _validations.test.ts to use Gen1App mock with clients.cloudFormation instead of module-level CloudFormationClient mock. --- Prompt: run tests and fix --------- Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
1 parent 42432b8 commit 954c1db

File tree

116 files changed

+2158
-1618
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

116 files changed

+2158
-1618
lines changed

AGENTS.md

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,14 +48,20 @@ Verify your changes by following these guidelines:
4848
work prefixed with a "Prompt: " after a single line consisting of '---'. Make sure there are no empty lines before or after this line.
4949
Word wrap all paragraphs at 72 columns including the prompt. For the author of the commit, use the configured username in git with
5050
' (AI)' appended and the user email. For example, `git commit --author="John Doe (AI) <john@bigco.com>" -m "docs: update configuration guide"`.
51-
To avoid issues with multi-line commit messages, write the message to `.commit-message.ai-generated.txt` and use `-F`:
51+
To avoid issues with multi-line commit messages, write the message to `.commit-message.ai-generated.txt` **at the repository root** and use `-F` with the path relative to your cwd:
5252

5353
```bash
54-
NODE_OPTIONS="--max-old-space-size=8192" git commit --author="John Doe (AI) <john@bigco.com>" -F .commit-message.ai-generated.txt
54+
NODE_OPTIONS="--max-old-space-size=8192" git commit --author="John Doe (AI) <john@bigco.com>" -F <repo-root>/.commit-message.ai-generated.txt
5555
```
5656

5757
Always set `NODE_OPTIONS="--max-old-space-size=8192"` when committing to prevent OOM failures in the lint-staged hook.
58-
After a successful commit, delete the scratch file: `rm -f .commit-message.ai-generated.txt`.
58+
After a successful commit, delete the scratch file: `rm -f ../../.commit-message.ai-generated.txt` (adjust the relative path to point to the repo root).
59+
60+
- **CRITICAL: Always write `.commit-message.ai-generated.txt` to the repository root**, not inside a package directory. The `-F` path
61+
in `git commit -F` is resolved relative to the cwd, so adjust the relative path accordingly (e.g., `../../.commit-message.ai-generated.txt`
62+
when committing from `packages/amplify-cli/`). This prevents stale files from accumulating in package directories.
63+
- The commit message subject line must be lowercase (commitlint enforces `subject-case`). Write `feat(scope): add feature` not
64+
`feat(scope): Add feature`.
5965

6066
- Since this repo has a commit hook that takes quite a long time to run, don't immediately commit every
6167
change you were asked to do. Apply your judgment, if the diff is still fairly small just keep going.
@@ -68,7 +74,12 @@ Verify your changes by following these guidelines:
6874

6975
### 5. PR Stage
7076

71-
This stage prepares the PR description — the user is responsible for creating the actual PR.
77+
Creating the PR means making sure everything meets our high bar and is
78+
ready for peer review and merge. The user is responsible for actually
79+
creating the PR, you are just preparing it.
80+
81+
Ask the user which branch the PR will be targetting and inspect the entire diff.
82+
Then, run the following phases, taking into account all the changes that were made:
7283

7384
#### 5.1 Update Docs
7485

@@ -77,9 +88,16 @@ Documentation is updated at PR time — not per-commit — because code changes
7788
- Update the .md files in `docs/` that correspond to the code files you touched.
7889
- Update the appropriate skill files when a change impacts the contents of the skill.
7990

80-
#### 5.2 Create Body File
91+
#### 5.2 Code Review
92+
93+
Before creating the PR body, do a final pass over every file you touched:
94+
95+
- Verify all code follows [CODING_GUIDELINES](./CODING_GUIDELINES.md) — read the guidelines file and check every rule against the code you touched.
96+
- Update JSDoc on every public member that was added or changed. Be concise.
97+
98+
#### 5.3 Create Body File
8199

82-
When asked to create a PR, generate a body into `.pr-body.ai-generated.md` and follow these guidelines:
100+
When asked to create a PR, generate a body into `.pr-body.ai-generated.md` **at the repository root** (not inside a package directory) and follow these guidelines:
83101

84102
- Use the PR template in `.github/PULL_REQUEST_TEMPLATE.md` as the structure.
85103
- Focus on **why** the change is being made and **what** it accomplishes, not the implementation details that are obvious from the diff.

CODING_GUIDELINES.md

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,71 @@ Scattered client usage also risks inconsistent instantiation. For example, if AP
106106

107107
---
108108

109+
### Keep parallel structures symmetric
110+
111+
When a class, interface, or module has two or more fields, methods, or data paths that serve the same role for different concerns, they should use the same types, the same access patterns, and the same naming conventions. Asymmetry — one field is a `Map` while its sibling is an `Array`, one uses a setter while its sibling uses a constructor parameter, one is optional while its sibling is required — signals that the design has drifted or that one path was added as an afterthought without aligning it with the existing one.
112+
113+
Asymmetry is a code smell even when the code is functionally correct. A reader scanning the class sees two fields that should be peers but are shaped differently, and has to figure out _why_ — is there a semantic reason, or is it accidental? That investigation costs time and often reveals that the difference is accidental.
114+
115+
This applies to naming too. When two sibling interfaces represent the same kind of thing for different domains, their property names should follow the same pattern. If one wraps its identity in a typed object (`resource: DiscoveredResource`), the other should too (`feature: DiscoveredFeature`) — not flatten it into loose fields (`feature: string; path: string`). And within those identity objects, analogous fields should use the same naming convention — if one has `resourceName`, the other should have `name`, not repeat the type (`feature.feature` stutters, `feature.name` doesn't).
116+
117+
This extends to method signatures. Sibling methods that do the same thing for different domains should accept the same number of arguments in the same shape. If `recordFeature` takes a single `FeatureAssessment` object, `recordResource` should take a single `ResourceAssessment` object — not three positional arguments. And the parameter names should follow the same convention: if one is `feature: FeatureAssessment`, the other should be `resource: ResourceAssessment`, not `assessment: ResourceAssessment`.
118+
119+
```typescript
120+
// Bad — two collections serving the same role, different types
121+
class Assessment {
122+
private readonly _resources = new Map<string, ResourceAssessment>();
123+
private readonly _features: FeatureAssessment[] = [];
124+
}
125+
126+
// Good — both are arrays, same access pattern
127+
class Assessment {
128+
private readonly _resources: ResourceAssessment[] = [];
129+
private readonly _features: FeatureAssessment[] = [];
130+
}
131+
132+
// Bad — sibling interfaces with asymmetric identity shapes
133+
interface ResourceAssessment {
134+
readonly resource: DiscoveredResource; // wrapped in typed object
135+
readonly generate: SupportLevel;
136+
readonly refactor: SupportLevel;
137+
}
138+
interface FeatureAssessment {
139+
readonly feature: string; // flat string — asymmetric
140+
readonly path: string; // flat string — asymmetric
141+
readonly generate: SupportLevel;
142+
readonly refactor: SupportLevel;
143+
}
144+
145+
// Good — both wrap identity in a typed object, same pattern
146+
interface ResourceAssessment {
147+
readonly resource: DiscoveredResource;
148+
readonly generate: SupportLevel;
149+
readonly refactor: SupportLevel;
150+
}
151+
interface FeatureAssessment {
152+
readonly feature: DiscoveredFeature;
153+
readonly generate: SupportLevel;
154+
readonly refactor: SupportLevel;
155+
}
156+
157+
// Bad — sibling methods with asymmetric signatures
158+
class Assessment {
159+
recordResource(resource: DiscoveredResource, generate: SupportLevel, refactor: SupportLevel): void { ... }
160+
recordFeature(feature: FeatureAssessment): void { ... }
161+
}
162+
163+
// Good — both take a single typed object, parameter named after the type
164+
class Assessment {
165+
recordResource(resource: ResourceAssessment): void { ... }
166+
recordFeature(feature: FeatureAssessment): void { ... }
167+
}
168+
```
169+
170+
The test: look at sibling fields, sibling methods, sibling interfaces, or sibling parameters. If they serve analogous roles but differ in type, shape, naming pattern, or access pattern, ask whether the difference is justified by a real semantic distinction. If not, align them.
171+
172+
---
173+
109174
## Mutability & State Management
110175

111176
### Minimize mutability

docs/packages/amplify-cli/src/commands/gen2-migration.md

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,22 +26,17 @@ const rollingBack = (context.input.options ?? {})['rollback'] ?? false;
2626

2727
### Common Gen1 Configuration Extraction
2828

29-
Extracts shared Gen1 configuration (`appId`, `appName`, `envName`, `stackName`, `region`) once from state manager and Amplify service,
30-
then passes these values to step constructors. This establishes a single source of truth; subcommands should use the injected values
31-
rather than re-extracting them independently.
29+
Creates a `Gen1App` facade that encapsulates all Gen1 app state — AWS clients, environment config, and the cloud backend snapshot. `Gen1App.create(context)` reads `team-provider-info.json`, fetches the app from the Amplify service, downloads the cloud backend from S3, and reads `amplify-meta.json`. The resulting instance is passed to all step constructors.
3230

3331
```ts
34-
const appId = (Object.values(stateManager.getTeamProviderInfo())[0] as any).awscloudformation.AmplifyAppId;
35-
const envName = localEnvName ?? migratingEnvName;
36-
// ... extract other config values
37-
const implementation: AmplifyMigrationStep = new step.class(logger, envName, appName, appId, stackName, region, context);
32+
const gen1App = await Gen1App.create(context);
33+
const implementation: AmplifyMigrationStep = new step.class(logger, gen1App, context);
3834
```
3935

4036
### Subcommand Dispatching
4137

4238
Maps the subcommand name to its implementation class via the `STEPS` registry, then instantiates the step with extracted configuration.
43-
The `assess` subcommand is intercepted before the `STEPS` lookup — it creates an `AmplifyMigrationAssessor` instead of a step,
44-
calls `assess()` on the generate and refactor steps, and renders the result.
39+
The `assess` subcommand is intercepted before the `STEPS` lookup — it creates an `AmplifyMigrationAssessor` with the `Gen1App` instance, calls `assess()` to collect resource and feature support levels, and prints the report.
4540

4641
### Plan-Based Execution
4742

@@ -116,7 +111,7 @@ flowchart LR
116111

117112
[`src/commands/gen2-migration/_step.ts`](../../../packages/amplify-cli/src/commands/gen2-migration/_step.ts)
118113

119-
Abstract base class that defines the lifecycle contract for all migration steps. Each step returns a `Plan` from `forward()` and `rollback()`.
114+
Abstract base class that defines the lifecycle contract for all migration steps. Constructor takes `(logger, gen1App, context)` — the `Gen1App` facade provides all app state. Each step returns a `Plan` from `forward()` and `rollback()`.
120115

121116
### `AmplifyMigrationOperation`
122117

@@ -173,6 +168,10 @@ amplify gen2-migration <step> [options]
173168
- `SpinningLogger` is the only logger class — the deprecated `Logger` subclass has been removed. Import directly from `_spinning-logger.ts`.
174169
- Automatic rollback is enabled by default but can be disabled with `--no-rollback`.
175170
- The `--rollback` flag explicitly executes rollback operations for a step.
171+
- `Gen1App` is the single facade for all Gen1 app state. It is created once in the dispatcher via `Gen1App.create(context)` and passed to all steps. Steps access `gen1App.appId`, `gen1App.region`, `gen1App.envName`, etc. instead of individual constructor params.
172+
- `AwsClients` has a private constructor — use `AwsClients.create(context)` in production. Tests bypass this with `new (AwsClients as any)(...)`.
173+
- Assessment uses a `Support` type with `level` and `note` fields. Each assessor provides its own note for unsupported entries. Use the `supported()`, `unsupported(note)`, `notApplicable()` helpers.
174+
- `KNOWN_RESOURCE_KEYS` (in `gen1-app.ts`) defines all supported category:service pairs. Unknown resources get the `'UNKNOWN'` key.
176175

177176
**Common pitfalls:**
178177

0 commit comments

Comments
 (0)