[AZ-581] [AZ-582] [AZ-583] [AZ-584] Cumulative review batches 01-03

Every-K=3 cumulative slice over the test-implementation cycle so far.
Scope: tests/, _docs/ — production source not touched. 48/48 ACs traced;
4 Low findings (3 follow-up + 1 baseline-carried). Verdict: PASS_WITH_WARNINGS.
Continue to Batch 4 (AZ-585, AZ-586).

Co-authored-by: Cursor <cursoragent@cursor.com>

_docs/03_implementation/cumulative_review_batches_01-03_cycle1_report.md

@@ -0,0 +1,90 @@
+# Cumulative Code Review — Batches 01–03 (cycle 1)
+
+**Mode**: cumulative (every K=3 batches), test-implementation context
+**Date**: 2026-05-15
+**Scope**: union of files changed since the start of the test-implementation run (batches 1, 2, 3) — see "Scanned files" below
+**Verdict**: **PASS_WITH_WARNINGS** (0 Critical, 0 High, 0 Medium; 4 Low; 0 baseline-regressions)
+
+## Scanned files
+
+Every file touched during the cumulative window:
+
+```
+.gitignore (B1)
+Auth/JwtExtensions.cs (B1 — only stop-watch noise; no functional change)
+docker-compose.test.yml (B1, B2 — fixtures volume + e2e-consumer wiring)
+Dockerfile (B1 — image tag for SUT)
+README.md (B1)
+tests/Azaion.Missions.E2E.Tests/* (B1, B2, B3 — full test project)
+tests/Azaion.Missions.JwksMock/* (B1, B3 — mock service; expanded /sign contract)
+_docs/02_tasks/ (lifecycle moves only — 11 tasks todo → done)
+_docs/03_implementation/batch_*_report.md (the 3 batch reports)
+```
+
+**Files NOT in scope** (deliberate — not changed this cycle): every production source file under `Azaion.Missions.csproj`'s authoritative ownership (`Services/`, `Database/`, `Infrastructure/`, `Middleware/`, `Controllers/`, `Program.cs`). The test cycle is observation-only on production code.
+
+## Phase coverage
+
+| Phase | Status | Notes |
+|-------|--------|-------|
+| 1. Context loading | OK   | Read every batch report + task spec for AZ-576..AZ-584 |
+| 2. Spec compliance | OK   | 48 / 48 ACs across the 9 task specs have a directly-tracing test method (`[Trait("Traces", "AC-X.Y")]`) |
+| 3. Code quality   | OK   | All test methods follow Arrange / Act / Assert; no bare catch; no >50-line methods (largest: 50 lines in `ConfigDbStartupTests.NFT_RES_05_db_down`) |
+| 4. Security quick-scan | OK | All Npgsql calls parameterised; no hardcoded secrets; `ForeignKeypair` confined to test-only use |
+| 5. Performance scan | OK | 100-iteration TOCTOU race bounded; rotation tests use 90s polled deadlines (no unbounded waits) |
+| 6. Cross-task consistency | OK | See "Cross-batch consistency" section below |
+| 7. Architecture compliance | OK | See "Baseline Delta" — no new layering/Public-API violations introduced |
+
+## Baseline Delta
+
+Baseline at `_docs/02_document/architecture_compliance_baseline.md` (2026-05-14, verdict PASS_WITH_WARNINGS with 2 High already resolved via doc retag and 2 Low open).
+
+| Carried over from baseline | Resolved this cycle | Newly introduced this cycle |
+|----------------------------|---------------------|-----------------------------|
+| F3 — dead `using Azaion.Flights.Enums;` in `Database/Entities/Flight.cs` (Low) | — | 0 |
+| F4 — three empty scaffolding directories at repo root (Low) | — | 0 |
+
+**Why zero new architecture findings**: the cumulative window touched only `tests/` and `_docs/`. The production source tree (under `Azaion.Missions.csproj`) was not modified, so no new same-namespace imports, no new component boundaries crossed, no new layer-direction violations are possible.
+
+## Cross-batch consistency (Phase 6)
+
+The 22 NFT methods (B3) sit alongside the 26 FT methods (B2) and 1 sanity stub (B1). Verified shared patterns are followed across all three batches:
+
+1. **`TestBase` inheritance** — every test class extends `TestBase` for the shared `HttpClient` + `TokenMinter` instances. No bespoke per-test HTTP-client construction (would risk DNS caching surprises against `missions:8080`).
+2. **Token minting** — every protected-endpoint call uses `Tokens.MintDefaultAsync()` (or `Tokens.MintAsync(SignRequest)` for non-default issuer/audience/permissions/alg/kid). The only test-only signing path that bypasses the mock is `ForeignKeypair.Mint(...)` in NFT-SEC-02 — explicitly scoped, called out in batch 3 report.
+3. **Side-channel assertions** — every DB-side check goes through `DbAssertions` or a direct Npgsql connection built from `TestEnvironment.DbSideChannel`. No test holds onto a long-lived connection across iterations.
+4. **HTTP assertions** — every status-code assertion goes through `HttpAssertions.AssertStatusAsync` or `HttpAssertions.AssertProblemEnvelopeAsync`. No raw `Assert.Equal((int)HttpStatusCode.X, ...)` in test bodies.
+5. **Docker-gated tests** — every Docker-dependent test is `SkippableFact` / `SkippableTheory` with an explicit `Skip.IfNot(...)` reason. No silent pass paths.
+6. **Traceability** — every test method carries `[Trait("Traces", "AC-X.Y")]` and `[Trait("max_ms", "<N>")]`. Tests with spec divergence carry `[Trait("carry_forward", "...")]` so `dotnet test --filter "carry_forward~..."` finds every flip-when-resolved site.
+7. **Fixtures and collections** — destructive fixtures (`DROP TABLE`, JWKS rotation, compose restart) live in dedicated xUnit collections (`CascadeF3`, `CascadeF4`, `ErrorEnvelope500`, `JwksRotation`, `MigratorRestart`) so they never overlap by accident. The `MigratorRestart` collection is shared by AZ-583 and AZ-584 (`MigratorRestartTests`, `ConfigDbStartupTests`) to serialize their docker-compose access.
+
+## Duplicate-symbol scan
+
+No test method names collide across files. NFT-SEC-* and NFT-RES-* prefixes are unique per scenario. Helper classes (`TokenMinter`, `ForeignKeypair`, `MissionsContainerHelper`, `DockerLogs`, `DbAssertions`, `HttpAssertions`, `FixtureSql`, `StubSchema`) are single-purpose with non-overlapping methods.
+
+## Open Findings (Low, 4)
+
+| # | Severity | Category        | Location                                                       | Title                                                                  | Suggestion                                                                                                                                                                                                                                                |
+|---|----------|-----------------|----------------------------------------------------------------|------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
+| 1 | Low      | Coverage        | 7 SkippableFact/SkippableTheory methods across batch 3         | Docker-CLI-dependent tests skip in default e2e-consumer image          | Follow-up task to add `docker-cli` + `/var/run/docker.sock` bind to the `e2e-consumer` service. Out of scope for test implementation.                                                                                                                      |
+| 2 | Low      | Maintainability | `tests/.../Resilience/ConfigDbStartupTests.cs:DropAzaionDatabase` | Connection-string swap via `string.Replace("Database=azaion", ...)`    | Replace with `NpgsqlConnectionStringBuilder` so the swap survives case/ordering changes in the canonical conn string.                                                                                                                                     |
+| 3 | Low      | Maintainability | Root `Azaion.Missions.csproj` (pre-existing project layout)    | Sdk.Web globs pull `tests/**/*.cs` into the main project compilation    | Add `<Compile Remove="tests/**" />` to `Azaion.Missions.csproj` OR introduce a `.sln` with explicit project list. **Pre-existing — NOT introduced by this cycle.** Confirmed via `git log -- Azaion.Missions.csproj`.                                       |
+| 4 | Low      | Maintainability | `Database/Entities/Flight.cs:2`                                | Dead `using Azaion.Flights.Enums;` directive (baseline-carried)        | Resolve as part of the post-B6 cleanup — already tracked in the baseline report. **Carried from baseline, not new.**                                                                                                                                       |
+
+## Auto-Fix Gate decision
+
+All 4 findings are Low/Maintainability/Coverage — no Critical, High, or Medium present. Per the implement-skill Auto-Fix Gate matrix:
+
+- Findings #1 and #3 are **out of scope for the test-implementation cycle** (infrastructure / project file). They should be created as separate follow-up tasks rather than auto-fixed in this run.
+- Finding #2 is auto-fix-eligible but the test it lives in is `SkippableFact` (today skipping), so the swap fix has no observable behavioral consequence right now — recommend folding it into the docker-cli follow-up task.
+- Finding #4 is pre-existing (baseline-carried) and already tracked.
+
+Per the cumulative-review gate: PASS_WITH_WARNINGS → continue to next batch (Step 14 loop).
+
+## Recommendation
+
+Proceed to Batch 4 (AZ-585 + AZ-586). After Batch 4 completes the cycle ends; Step 7 (`test-run/SKILL.md`) owns the full-suite gate and will surface the SkippableFact reasons live during the `docker compose ... up e2e-consumer` invocation.
+
+## Sign-off
+
+Cumulative review batches 01–03, cycle 1: **PASS_WITH_WARNINGS**. No blocking findings. Loop to Step 14 → Batch 4.