feat(gen2-migration): assessment validation, credential loading, and infra reorganization

AGENTS.md

@@ -48,14 +48,21 @@ Verify your changes by following these guidelines:
   work prefixed with a "Prompt: " after a single line consisting of '---'. Make sure there are no empty lines before or after this line.
   Word wrap all paragraphs at 72 columns including the prompt. For the author of the commit, use the configured username in git with
   ' (AI)' appended and the user email. For example, `git commit --author="John Doe (AI) <john@bigco.com>" -m "docs: update configuration guide"`.
-  To avoid issues with multi-line commit messages, write the message to `.commit-message.ai-generated.txt` and use `-F`:
+  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:
 
   ```bash
-  NODE_OPTIONS="--max-old-space-size=8192" git commit --author="John Doe (AI) <john@bigco.com>" -F .commit-message.ai-generated.txt
+  # From packages/amplify-cli/:
+  NODE_OPTIONS="--max-old-space-size=8192" git commit --author="John Doe (AI) <john@bigco.com>" -F ../../.commit-message.ai-generated.txt
   ```
 
   Always set `NODE_OPTIONS="--max-old-space-size=8192"` when committing to prevent OOM failures in the lint-staged hook.
-  After a successful commit, delete the scratch file: `rm -f .commit-message.ai-generated.txt`.
+  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).
+
+- **CRITICAL: Always write `.commit-message.ai-generated.txt` to the repository root**, not inside a package directory. The `-F` path
+  in `git commit -F` is resolved relative to the cwd, so adjust the relative path accordingly (e.g., `../../.commit-message.ai-generated.txt`
+  when committing from `packages/amplify-cli/`). This prevents stale files from accumulating in package directories.
+- The commit message subject line must be lowercase (commitlint enforces `subject-case`). Write `feat(scope): add feature` not
+  `feat(scope): Add feature`.
 
 - Since this repo has a commit hook that takes quite a long time to run, don't immediately commit every
   change you were asked to do. Apply your judgment, if the diff is still fairly small just keep going.
@@ -68,7 +75,12 @@ Verify your changes by following these guidelines:
 
 ### 5. PR Stage
 
-This stage prepares the PR description — the user is responsible for creating the actual PR.
+Creating the PR means making sure everything meets our high bar and is
+ready for peer review and merge. The user is responsible for actually
+creating the PR, you are just preparing it.
+
+Ask the user which branch the PR will be targetting and inspect the entire diff.
+Then, run the following phases, taking into account all the changes that were made:
 
 #### 5.1 Update Docs
 
@@ -77,9 +89,16 @@ Documentation is updated at PR time — not per-commit — because code changes
 - Update the .md files in `docs/` that correspond to the code files you touched.
 - Update the appropriate skill files when a change impacts the contents of the skill.
 
-#### 5.2 Create Body File
+#### 5.2 Code Review
+
+Before creating the PR body, do a final pass over every file you touched:
+
+- Verify all code follows [CODING_GUIDELINES](./CODING_GUIDELINES.md) — read the guidelines file and check every rule against the code you touched.
+- Update JSDoc on every public member that was added or changed. Be concise.
+
+#### 5.3 Create Body File
 
-When asked to create a PR, generate a body into `.pr-body.ai-generated.md` and follow these guidelines:
+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:
 
 - Use the PR template in `.github/PULL_REQUEST_TEMPLATE.md` as the structure.
 - 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

@@ -106,6 +106,71 @@ Scattered client usage also risks inconsistent instantiation. For example, if AP
 
 ---
 
+### Keep parallel structures symmetric
+
+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.
+
+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.
+
+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).
+
+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`.
+
+```typescript
+// Bad — two collections serving the same role, different types
+class Assessment {
+  private readonly _resources = new Map<string, ResourceAssessment>();
+  private readonly _features: FeatureAssessment[] = [];
+}
+
+// Good — both are arrays, same access pattern
+class Assessment {
+  private readonly _resources: ResourceAssessment[] = [];
+  private readonly _features: FeatureAssessment[] = [];
+}
+
+// Bad — sibling interfaces with asymmetric identity shapes
+interface ResourceAssessment {
+  readonly resource: DiscoveredResource; // wrapped in typed object
+  readonly generate: SupportLevel;
+  readonly refactor: SupportLevel;
+}
+interface FeatureAssessment {
+  readonly feature: string; // flat string — asymmetric
+  readonly path: string; // flat string — asymmetric
+  readonly generate: SupportLevel;
+  readonly refactor: SupportLevel;
+}
+
+// Good — both wrap identity in a typed object, same pattern
+interface ResourceAssessment {
+  readonly resource: DiscoveredResource;
+  readonly generate: SupportLevel;
+  readonly refactor: SupportLevel;
+}
+interface FeatureAssessment {
+  readonly feature: DiscoveredFeature;
+  readonly generate: SupportLevel;
+  readonly refactor: SupportLevel;
+}
+
+// Bad — sibling methods with asymmetric signatures
+class Assessment {
+  recordResource(resource: DiscoveredResource, generate: SupportLevel, refactor: SupportLevel): void { ... }
+  recordFeature(feature: FeatureAssessment): void { ... }
+}
+
+// Good — both take a single typed object, parameter named after the type
+class Assessment {
+  recordResource(resource: ResourceAssessment): void { ... }
+  recordFeature(feature: FeatureAssessment): void { ... }
+}
+```
+
+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.
+
+---
+
 ## Mutability & State Management
 
 ### Minimize mutability

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

@@ -26,22 +26,17 @@ const rollingBack = (context.input.options ?? {})['rollback'] ?? false;
 
 ### Common Gen1 Configuration Extraction
 
-Extracts shared Gen1 configuration (`appId`, `appName`, `envName`, `stackName`, `region`) once from state manager and Amplify service,
-then passes these values to step constructors. This establishes a single source of truth; subcommands should use the injected values
-rather than re-extracting them independently.
+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.
 
 ```ts
-const appId = (Object.values(stateManager.getTeamProviderInfo())[0] as any).awscloudformation.AmplifyAppId;
-const envName = localEnvName ?? migratingEnvName;
-// ... extract other config values
-const implementation: AmplifyMigrationStep = new step.class(logger, envName, appName, appId, stackName, region, context);
+const gen1App = await Gen1App.create(context);
+const implementation: AmplifyMigrationStep = new step.class(logger, gen1App, context);
 ```
 
 ### Subcommand Dispatching
 
 Maps the subcommand name to its implementation class via the `STEPS` registry, then instantiates the step with extracted configuration.
-The `assess` subcommand is intercepted before the `STEPS` lookup — it creates an `AmplifyMigrationAssessor` instead of a step,
-calls `assess()` on the generate and refactor steps, and renders the result.
+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.
 
 ### Plan-Based Execution
 
@@ -116,7 +111,7 @@ flowchart LR
 
 [`src/commands/gen2-migration/_step.ts`](../../../packages/amplify-cli/src/commands/gen2-migration/_step.ts)
 
-Abstract base class that defines the lifecycle contract for all migration steps. Each step returns a `Plan` from `forward()` and `rollback()`.
+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()`.
 
 ### `AmplifyMigrationOperation`
 
@@ -173,6 +168,10 @@ amplify gen2-migration <step> [options]
 - `SpinningLogger` is the only logger class — the deprecated `Logger` subclass has been removed. Import directly from `_spinning-logger.ts`.
 - Automatic rollback is enabled by default but can be disabled with `--no-rollback`.
 - The `--rollback` flag explicitly executes rollback operations for a step.
+- `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.
+- `AwsClients` has a private constructor — use `AwsClients.create(context)` in production. Tests bypass this with `new (AwsClients as any)(...)`.
+- 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.
+- `KNOWN_RESOURCE_KEYS` (in `gen1-app.ts`) defines all supported category:service pairs. Unknown resources get the `'UNKNOWN'` key.
 
 **Common pitfalls:**