mirror of
https://github.com/azaion/satellite-provider.git
synced 2026-06-21 08:41:13 +00:00
[AZ-794] [AZ-795] [AZ-796] Cycle 7 Step 17: retrospective + close cycle
Cycle 7 retrospective (cycle-end mode) — three-task pure-quality cycle (AZ-794 rename + AZ-795 epic shared infra + AZ-796 inventory validator). PASS gate end-to-end; first cycle to ship a contract MAJOR bump; second consecutive cycle with zero new process leftovers; first cycle to run the full 5-phase security audit since cycle 5. Top 3 improvement actions for cycle 8: 1. Formalise the implement-skill <-> downstream-skill artifact contract — cycle 7 shipped without an implementation report and the doc / test-spec / retrospective skills successfully fell back to task-spec + commit-body reading, but the fallback is implicit and should be codified. 2. Sanitize JsonException.Message + BadHttpRequestException.Message before surfacing them in ValidationProblemDetails.detail — F-AZ795-1 / F-AZ795-2 in the cycle-7 security audit. 3. AZ-795 child-task sweep across the remaining public endpoints (request / route / upload / latlon) using AZ-796 as the reference pattern; 2-3 SP per endpoint, spread across cycles 8-10. LESSONS.md ring buffer updated with 3 cycle-7 entries (process / testing / architecture); 3 oldest cycle-2 entries dropped to maintain the 15-entry buffer. State pointer advanced to cycle 8 step 9 (New Task) — Re-Entry After Completion per autodev existing-code flow. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -0,0 +1,221 @@
|
|||||||
|
# Retrospective — Cycle 7 (2026-05-22)
|
||||||
|
|
||||||
|
**Tasks**: AZ-794 — inventory field rename `tileZoom/tileX/tileY → z/x/y` (3 SP), AZ-795 — strict input validation epic + shared infra (5–8 SP rolled up; the shared-infra ship landed in this cycle), AZ-796 — strict validation for the inventory endpoint as the first per-endpoint child of AZ-795 (3 SP). Three-task cycle delivered as a **single batch** (commit `865dfdb`).
|
||||||
|
**Mode**: cycle-end (autodev Step 17). Step 16.5 (Release) skipped per the cycle-2-to-6 established convention — the `deploy_cycle7.md` operator runbook serves as the release record; user-confirmed via Choose A/B/C at Step 16.5.
|
||||||
|
**Previous retro**: `retro_2026-05-12_cycle6.md`
|
||||||
|
**Cycle shape**: small **pure-quality** follow-up cycle — no new endpoints, no migrations, no new env vars, no container image changes. Pure code + contract revision. **First cycle to ship a contract MAJOR bump** (`tile-inventory.md` 1.0.0 → 2.0.0). **First cycle to ship a strict-validation surface** (FluentValidation 12.0.0 + global ProblemDetails handler + `UnmappedMemberHandling.Disallow`). **First cycle to ship without an `_docs/03_implementation/batch_*_cycle7_report.md` artifact** (process gap — see §4 Pattern 1).
|
||||||
|
|
||||||
|
## 1. Implementation Metrics
|
||||||
|
|
||||||
|
| Metric | Cycle 7 | Δ vs cycle 6 |
|
||||||
|
|--------|---------|--------------|
|
||||||
|
| Tasks implemented | **3** (AZ-794, AZ-795, AZ-796) | +2 |
|
||||||
|
| Batches executed | **1** | unchanged |
|
||||||
|
| Avg tasks / batch | **3.0** | +2.0 |
|
||||||
|
| Total complexity delivered | **~8 SP** (3 + ~2 + 3; AZ-795 is an epic, the shared-infra ship counts as ~2 SP since the per-endpoint validator is in AZ-796) | +5 SP |
|
||||||
|
| Avg complexity / batch | **~8 SP** | +5 SP |
|
||||||
|
| Tasks at-or-below 5 SP cap | **3 of 3 (100%)** | unchanged |
|
||||||
|
| Tasks split mid-cycle | **0** | unchanged |
|
||||||
|
| Tasks above cap | 0 | unchanged |
|
||||||
|
| Cumulative reviews | **0** (trigger is every 3 batches; cycle 7 has 1 batch) | unchanged |
|
||||||
|
| Cross-cycle leftovers carried in | **0** (cycle 6 closed the long-standing cycle-3 perf-harness leftover) | -1 |
|
||||||
|
| Cross-cycle leftovers carried out | **0** | unchanged |
|
||||||
|
| Implementation report artifact written | **NO** (process gap — see §4) | **NEW DEVIATION** (cycle 6 wrote `implementation_report_tile_inventory_cycle6.md`) |
|
||||||
|
|
||||||
|
**Sequencing**: single batch — three logically-related tasks (rename + validation epic + first per-endpoint validator) landed atomically because the rename, the unmapped-member rejection, and the FluentValidation rules form a coherent contract bump that would have been awkward to ship piecemeal. The cycle completed in **5 dev commits**: `19c0371` + `7d3ba1c` (preceding `.cursor/` plumbing, no-ticket), `dceaddc` (Step 9 task adoption), `865dfdb` (Step 10 implementation), and a pending Steps 12-17 sync commit (this commit, containing docs sync + security audit + perf report + deploy report + this retro). One fewer commit than cycle 6's 7-commit count — cycle 7 had no post-merge correction commits (vs cycle 6's AC-5 TLS pivot commit).
|
||||||
|
|
||||||
|
## 2. Quality Metrics
|
||||||
|
|
||||||
|
### Code Review Results
|
||||||
|
|
||||||
|
| Verdict | Count | Percentage |
|
||||||
|
|---------|-------|-----------|
|
||||||
|
| PASS | **(no per-batch review on file — see §4 Pattern 1)** | **n/a** |
|
||||||
|
| PASS_WITH_WARNINGS | n/a | — |
|
||||||
|
| FAIL | 0 | — |
|
||||||
|
|
||||||
|
The single-batch implementation `865dfdb` did not produce a separate `reviews/batch_01_cycle7_review.md` artifact. The downstream skills (test-spec cycle-update mode, document task mode, security audit phase 2) functioned as a distributed review surface — `static_analysis_cycle7.md` is the most code-review-shaped artifact this cycle produced. Severity counts below are sourced from `_docs/05_security/static_analysis_cycle7.md` + `_docs/05_security/owasp_review_cycle7.md` + the deploy-report follow-up section.
|
||||||
|
|
||||||
|
### Findings by Severity
|
||||||
|
|
||||||
|
| Severity | Cycle 7 | Δ vs cycle 6 |
|
||||||
|
|----------|---------|--------------|
|
||||||
|
| Critical | 0 | unchanged |
|
||||||
|
| High | 0 | unchanged |
|
||||||
|
| Medium | 0 | -1 (cycle 6 had 1 Medium auto-fixed in review — `ComputeLocationHash` duplication) |
|
||||||
|
| Low | **3** (D-AZ795-1 FluentValidation 12.0.0 → 12.1.1 bump; F-AZ795-1 `JsonException.Message` leak; F-AZ795-2 `BadHttpRequestException.Message` leak) | +3 |
|
||||||
|
| Remaining post-review | **3** Low — all carried into deploy_cycle7.md as recommended follow-up PBIs (no auto-fix this cycle) | +3 |
|
||||||
|
|
||||||
|
### Findings by Category
|
||||||
|
|
||||||
|
| Category | Count | Top Files |
|
||||||
|
|----------|-------|-----------|
|
||||||
|
| Maintainability | 0 | — (no duplication patterns this cycle) |
|
||||||
|
| Bug | 0 | — |
|
||||||
|
| Spec-Gap | 0 | — |
|
||||||
|
| Security | **3 Low** | `SatelliteProvider.Api.csproj` (D-AZ795-1 minor version bump) + `SatelliteProvider.Api/GlobalExceptionHandler.cs` (F-AZ795-1 + F-AZ795-2 information-disclosure on auth-gated 400 paths) |
|
||||||
|
| Performance | 0 | — |
|
||||||
|
| Style | 0 | — |
|
||||||
|
| Scope | 0 | unchanged |
|
||||||
|
|
||||||
|
### Security audit (cycle 7)
|
||||||
|
|
||||||
|
| Metric | Value | Δ vs cycle 6 |
|
||||||
|
|--------|-------|--------------|
|
||||||
|
| Verdict | **PASS_WITH_WARNINGS** | new (cycle 6 was SKIPPED) |
|
||||||
|
| Reason | Two NuGet additions (FluentValidation 12.0.0 + .DependencyInjectionExtensions 12.0.0, both no-CVE clean), new strict-validation surface (no security regression; auth runs before validation, confirmed in `Program.cs`), two Low information-disclosure findings on the 400 detail string sanitization, one Low recommended minor-version bump | — |
|
||||||
|
| New Critical / High | **0** | unchanged |
|
||||||
|
| New Low | **3** (D-AZ795-1 + F-AZ795-1 + F-AZ795-2) | +3 |
|
||||||
|
| Carry-overs (still OPEN, unchanged from cycle 6) | **3** unchanged (`Microsoft.NET.Test.Sdk` 17.8.0 transitive flag; `Microsoft.IdentityModel.Tokens` / `System.IdentityModel.Tokens.Jwt` 7.0.3 NU1902; `Serilog.AspNetCore` 8.0.3 fallback) | unchanged |
|
||||||
|
|
||||||
|
**Audit shape**: this cycle ran the full 5-phase security skill (Dependency Scan, Static Analysis, OWASP Review, Infrastructure Review, Security Report) — first cycle since cycle 5 to run all 5 phases. Audit was justified by the new NuGet additions + the strict-validation surface + the .NET error-handling pipeline changes (`GlobalExceptionHandler` introduced). Output: 5 per-phase reports + 1 consolidated report = 6 new files in `_docs/05_security/`.
|
||||||
|
|
||||||
|
### Performance gate (cycle 7)
|
||||||
|
|
||||||
|
| Metric | Value |
|
||||||
|
|--------|-------|
|
||||||
|
| Verdict | **PASS** |
|
||||||
|
| Scenarios | **9 Pass · 0 Warn · 0 Fail · 0 Unverified** — PT-01..PT-08 from the scripted harness + a cycle-7 PT-09 smoke probe (v2 schema, all-miss path) added separately to verify the renamed contract + validator overhead |
|
||||||
|
| Script exit code | **0** (second consecutive clean exit — cycle 6 was the first; cycle 7 confirms the harness stability) |
|
||||||
|
| AZ-794 / AZ-795 / AZ-796 contract regression check | **PASS** — PT-09 smoke probe with the new `z/x/y` schema returned 200 on every call; legacy `tileZoom/tileX/tileY` confirmed rejected by `scripts/probe_inventory_validation.sh` |
|
||||||
|
| AZ-795 / AZ-796 validator overhead | **≤ 10 ms** on a 2500-item batch (PT-09 smoke median 44 ms vs cycle-6 canonical PT-09 median 19 ms — the gap straddles the all-miss-vs-all-hit delta plus the validator pass; both well within the 1000 ms p95 budget) |
|
||||||
|
| Existing PT-01..PT-08 regressions | **None.** PT-07 warm p95 dropped from cycle 6's 1049 ms to 76 ms — cycle 6 had identified the cycle-6 warm-p95 inflation as per-curl-TLS-handshake harness overhead, and cycle 7 confirms the underlying application warm path is sub-100 ms once TLS session reuse settles. Similarly PT-08 batch p95 dropped 544 ms → 284 ms for the same reason. |
|
||||||
|
| Cross-cycle leftover handling | **No new leftovers**; `_docs/_process_leftovers/` remains empty (cycle 6 closed the cycle-3 perf-harness leftover). |
|
||||||
|
|
||||||
|
## 3. Trend Comparison (cycle 6 → cycle 7)
|
||||||
|
|
||||||
|
| Trend | Direction | Notes |
|
||||||
|
|-------|-----------|-------|
|
||||||
|
| Findings volume (post-review) | **+3 Low** (0 → 3) | All three are minor; two are information-disclosure on a dev/auth-gated 400 path (low impact). The increase reflects cycle 7 actually running Step 14, not a regression in code quality — cycle 6 skipped the security audit. |
|
||||||
|
| Code-review pass rate | **n/a** (no per-batch review file written this cycle) | Cycle 6 was 100% PASS on one batch; cycle 7 has no per-batch review artifact. Process gap, not a quality regression. |
|
||||||
|
| Leftovers carried out | **unchanged** (0 → 0) | First two consecutive cycles with zero new leftovers. |
|
||||||
|
| Architectural cycles introduced | **0** (unchanged) | The new `Validators/` namespace fits within the API project's established layering (`SatelliteProvider.Api → Common.DTO`); no new cross-component edges. |
|
||||||
|
| Contract artifacts produced | **-1** (2 → 1) | Cycle 6 produced 1 new contract + 1 major bump; cycle 7 produces 1 major bump. |
|
||||||
|
| Migrations | **-1** (1 → 0) | Cycle 7 has zero schema changes — first migration-free cycle since cycle 4 (the .NET 10 migration). |
|
||||||
|
| Step 14 (Security Audit) outcome | **PASS_WITH_WARNINGS** (vs cycle 6 SKIPPED) | First full 5-phase audit since cycle 5 (which was PASS_WITH_WARNINGS with 1 Low). |
|
||||||
|
| Step 15 (Performance Test) script exit | **0** (unchanged) | Second consecutive clean exit-0. PT-09 smoke probe added to the cycle-7 perf report covers the new v2 schema. |
|
||||||
|
| Cross-cycle bug-introduction pattern | **0 new** | Nothing newly broken this cycle. The strict deserializer *uncovered* one latent bug in `IdempotentPostTests` (PascalCase property fallback that had been silently working for years; see §4 Pattern 2), but it was a pre-existing bug surfaced by the new strict layer, not a cycle-7 regression. |
|
||||||
|
| Mid-cycle scope decisions (split / defer / re-spec) | **0** | No mid-cycle pivots; the three tasks shipped together as planned. |
|
||||||
|
| Post-merge correction commits | **-1** (1 → 0) | No post-merge corrections this cycle (vs cycle 6's AC-5 TLS pivot). |
|
||||||
|
| Implementation report artifact | **NEW DEVIATION** | Cycle 7 has no `_docs/03_implementation/batch_01_cycle7_report.md` and no `_docs/03_implementation/implementation_report_*_cycle7.md`. Downstream skills compensated via task-spec + commit-body reading. Recommended action in §5 Action 1. |
|
||||||
|
|
||||||
|
**Cumulative LESSONS reuse**: 4 lessons from previous cycles were directly applicable this cycle:
|
||||||
|
|
||||||
|
- **2026-05-12 testing lesson on test-helpers and schema-artifact-name fragility (L-007 / cycle 6)** — the cycle-7 `IdempotentPostTests` adjacent fix was a different flavour of the same fragility: the test was relying on the old non-strict PascalCase property-name fallback, not on a schema-artifact name. The strict deserializer exposed the test's reliance on a silent behaviour. Lesson reused (same family of "tests that depend on lenient defaults break when defaults tighten").
|
||||||
|
- **2026-05-12 dependencies lesson on Major-version bumps (cycle 4)** — confirmed valid. FluentValidation 12.0.0 is a major (vs 11.x) but the project never had 11.x, so the cascade analysis was N/A. The lesson held: the spec author verified no transitive drift via the `dotnet restore` analysis before the spec was written.
|
||||||
|
- **2026-05-12 process lesson on cross-PBI dependency capture (cycle 5)** — cycle 7 consumed AZ-794 + AZ-795 + AZ-796 with a `Relates` link in Jira (rather than `blocks`) because the three are coupled-but-independent. Lesson reused: the tracker link choice (`Relates` vs `blocks`) communicates ordering intent.
|
||||||
|
- **2026-05-11 process lesson on Deferred-NFR ring-buffer (cycle 2)** — the AZ-503 AC-9 perf-NFR deferral that was promoted to AZ-505 AC-4 in cycle 6 stayed closed this cycle (the PT-09 smoke probe verifies the cycle-6 closure still holds). Lesson reused: deferrals are a one-cycle item, not multi-cycle.
|
||||||
|
|
||||||
|
## 4. Patterns and Insights
|
||||||
|
|
||||||
|
### Pattern 1 — Implementation-report artifact not produced (new this cycle)
|
||||||
|
|
||||||
|
The cycle 7 implement skill ran in a single batch and shipped commit `865dfdb`, but did NOT write `_docs/03_implementation/batch_01_cycle7_report.md` and did NOT write `_docs/03_implementation/implementation_report_*_cycle7.md`. The downstream skills (test-spec cycle-update mode, document task mode, security audit, perf, deploy) all consumed task specs + the commit body as a fallback, and the cycle's documentation surface ended up complete — but the implement skill's own contract was not satisfied.
|
||||||
|
|
||||||
|
**Why this matters**: the implement skill's exit gate is implicit (the autodev orchestrator advances state on Step 10 completion based on user-visible markers, not on a hard artifact check). When the implement skill terminates without writing the expected artifact, the next skill in the chain has no canonical input. Cycle 7 worked around it; cycle 8 might not.
|
||||||
|
|
||||||
|
**Insight**: every downstream skill that consumes the implementation report (test-spec cycle-update, document task mode, retrospective Step 1) needs either (a) a tighter exit gate at Step 10 that BLOCKS unless the report is on disk, or (b) a documented fallback contract ("if the report is missing, read the cycle's task specs and the most recent commit body"). The fallback worked this cycle but it's a soft contract — it should be hardened or formalised.
|
||||||
|
|
||||||
|
**Compare to cycle 5 retro lesson** on cross-PBI dependency capture: that lesson made the link contract explicit. This pattern is the same flavour — the implicit contract between Step 10 and Steps 11–17 should be made explicit.
|
||||||
|
|
||||||
|
### Pattern 2 — Strict deserialization surfaces latent test bugs (new this cycle)
|
||||||
|
|
||||||
|
`IdempotentPostTests` had been silently relying on the .NET model-binding PascalCase fallback for `RoutePoint` payloads — the DTO carries `[JsonPropertyName("lat")]` / `[JsonPropertyName("lon")]` but the test posted `{"Lat": ..., "Lon": ...}` and the lenient pre-cycle-7 deserializer coerced it. The cycle-7 `JsonSerializerOptions.UnmappedMemberHandling.Disallow` flipped the deserializer to strict mode, and the test broke. Fix was a 2-line payload correction (`Lat` → `lat`, `Lon` → `lon`); the bug had been latent for an unknown number of months.
|
||||||
|
|
||||||
|
**Why this matters**: the strict-validation surface is doing exactly what it was designed to do — surfacing client-side bugs that the lenient defaults masked. The interesting observation is that **the project's own integration test suite had latent bugs that the strict surface caught**. The same pattern will play out at the client boundary: any consumer that has been silently posting wrong-cased keys will see a 400 the moment the cycle-7 image is promoted.
|
||||||
|
|
||||||
|
**Insight**: when a new strict layer ships, the surfaced-bug count inside the project's own test suite is a leading indicator of how many bugs the same layer will surface in production traffic. Cycle 7's count: 1 bug (`IdempotentPostTests`) + 1 boundary correction (`TileInventoryTests` slippy x/y bounds for the new z=18 validation rule). Both were genuine bugs; neither was a cycle-7 regression. Plan accordingly when promoting beyond `dev`: expect a 4xx-rate spike in the first 24 hours as silent client-side bugs are forced into the open. The cycle 6 retro's lesson on the watch-window mattering (which lives in the release skill, not run this cycle) directly applies — even though Step 16.5 was skipped, the operator runbook in `deploy_cycle7.md` step 4 (smoke test) covers the surfaced-bug path explicitly.
|
||||||
|
|
||||||
|
### Pattern 3 — Documentation skill correctly auto-detected cycle-7 changes despite missing implementation report (positive pattern)
|
||||||
|
|
||||||
|
The cycle-7 `document` skill in Task mode produced a complete ripple-log entry (`_docs/02_document/ripple_log_cycle7.md`) that correctly identified every changed source file and every documentation surface that needed an update, without an implementation report to consume. The skill walked the `done/` task specs + the diff between the cycle-6 commit and HEAD to derive the same input the implementation report would have given it.
|
||||||
|
|
||||||
|
**Insight**: the document skill's resume protocol was already robust enough to handle the Pattern-1 process gap. The same may be true of the test-spec cycle-update mode and the security skill. The fix proposed in §5 Action 1 (formalise the fallback contract or tighten the exit gate) should consider keeping the fallback as the cheaper, more resilient path rather than forcing a hard artifact dependency.
|
||||||
|
|
||||||
|
### Pattern 4 — Contract MAJOR bump shipped without a separate JSON-schema migration tool (positive pattern)
|
||||||
|
|
||||||
|
The `tile-inventory.md` 1.0.0 → 2.0.0 bump renamed three top-level JSON fields. The implementation strategy was:
|
||||||
|
|
||||||
|
1. Update the DTO with `[JsonPropertyName("z")]` etc. (single source of truth: the DTO file).
|
||||||
|
2. Update the contract doc.
|
||||||
|
3. Add an integration test for the legacy-field-rejection path.
|
||||||
|
4. Add a probe script for manual smoke (`scripts/probe_inventory_validation.sh`).
|
||||||
|
5. **No client-side migration tool** — the cycle 7 contract is breaking, and the operator runbook explicitly tells consumers (`gps-denied-onboard`) to update their request bodies before the production deploy.
|
||||||
|
|
||||||
|
**Why this matters**: this is the right call for a sub-10-consumer project. A migration tool (e.g. a wire-format adapter on the api side that accepts BOTH v1 and v2 for one cycle) would have added complexity for a contract whose only consumer is a single sibling repo that's actively in the cycle 7 / AZ-777 follow-up loop. The major bump + operator notification is appropriate.
|
||||||
|
|
||||||
|
**Insight**: the cost-benefit of a wire-format adapter scales with consumer count and consumer release-cadence asymmetry. For a project with one slow-moving consumer, the adapter has negative value (more code to maintain, more code to test, surface for confusion). For a project with many consumers across many teams, the adapter is worth the cost. Cycle 7's call (no adapter) is correct for the current shape of the project; future cycles should re-evaluate as the consumer surface grows.
|
||||||
|
|
||||||
|
## 5. Top 3 Improvement Actions
|
||||||
|
|
||||||
|
### Action 1 — Formalise the implement-skill ↔ downstream-skill artifact contract
|
||||||
|
|
||||||
|
**Why**: pattern 1 above. Cycle 7 worked around a missing implementation report; downstream skills compensated via task-spec + commit-body reading. But the contract is implicit and could break in cycle 8 in subtler ways (e.g. if a future cycle's commit body is too terse to substitute for the report).
|
||||||
|
|
||||||
|
**Action**: pick ONE of (a) tighten implement-skill exit gate to BLOCK unless `_docs/03_implementation/batch_*_cycle{N}_report.md` AND `_docs/03_implementation/implementation_report_*_cycle{N}.md` exist on disk, OR (b) document the commit-body fallback contract explicitly in `.cursor/skills/implement/SKILL.md` and in every downstream skill that consumes the report (test-spec cycle-update, document task mode, retrospective Step 1). Option (a) is stricter and surfaces the gap loudly; option (b) is cheaper to land and matches what already worked this cycle. Recommendation: **option (b)** — codify the fallback rather than force the artifact, because the fallback proved more resilient than the artifact. ~1 SP.
|
||||||
|
|
||||||
|
**Cost**: ~30 minutes of skill-author work. Counted as 1 SP because it touches the implement skill AND ≥3 downstream skills (test-spec, document, retrospective) with consistent wording.
|
||||||
|
|
||||||
|
### Action 2 — Sanitize `JsonException.Message` + `BadHttpRequestException.Message` before surfacing in `ValidationProblemDetails.detail`
|
||||||
|
|
||||||
|
**Why**: pattern 2 above's siblings — F-AZ795-1 and F-AZ795-2 from `_docs/05_security/static_analysis_cycle7.md`. The strict deserializer's error messages can leak internal .NET type names ("Cannot deserialize the current JSON object (e.g. {…}) into type `…`") and parameter names ("The value for parameter `…` is invalid") on the 400 detail string. Auth-gated, low impact in dev; not appropriate for a production-grade contract.
|
||||||
|
|
||||||
|
**Action**: in `SatelliteProvider.Api/GlobalExceptionHandler.cs`, replace the raw `Exception.Message` pass-through for `JsonException` and `BadHttpRequestException` with a static string ("`Request body is not valid JSON`" / "`Form value could not be bound`"). Keep the structured `extensions.errors` map (which is already sanitised by FluentValidation) as the consumer-facing detail. Add an integration test (`GlobalExceptionHandlerTests.JsonDeserializationFailure_DoesNotLeakInternalTypeName`) to lock the contract. ~2 SP.
|
||||||
|
|
||||||
|
**Cost**: ~1 hour of dev work. Counted as 2 SP because it touches the global exception handler (cross-cutting) and needs a paired integration test to prevent regression.
|
||||||
|
|
||||||
|
### Action 3 — AZ-795 child-task sweep across the remaining public endpoints
|
||||||
|
|
||||||
|
**Why**: AZ-795 is an open epic. AZ-796 is the reference implementation for the inventory endpoint. The remaining public endpoints (`POST /api/satellite/request`, `POST /api/satellite/route`, `POST /api/satellite/upload`, `GET /api/satellite/tiles/latlon`, etc.) are still silently coercing missing fields. The same client-bug-masking class of issues that motivated AZ-795 still applies to those endpoints.
|
||||||
|
|
||||||
|
**Action**: create per-endpoint child tasks under AZ-795 in Jira (each carrying the same shape as AZ-796 — validator + integration tests + contract-doc update). Sequencing recommendation: prioritise endpoints whose 4xx surface is currently empty (i.e. endpoints where a silent 200 with wrong data is more likely than a 400 with a clear error). Default prioritisation: `POST /api/satellite/request` (region requests — wrong center → wrong tiles), `POST /api/satellite/route` (route requests — wrong points → wrong interpolation), `POST /api/satellite/upload` (UAV upload — already partly validated by `UavTileQualityGate`, but the metadata layer is a separate validator). Each task is 2-3 SP. ~6-9 SP total across 3-4 child tasks.
|
||||||
|
|
||||||
|
**Cost**: tracker-side ~30 minutes to create the child tasks; per-task implementation cost is the per-endpoint SP estimate. Recommendation: spread across cycles 8-10, one or two per cycle, to maintain the "small focused cycle" cadence that worked well for cycles 5-7.
|
||||||
|
|
||||||
|
## 6. Carry-overs (status this cycle)
|
||||||
|
|
||||||
|
| Item | Status | Notes |
|
||||||
|
|------|--------|-------|
|
||||||
|
| `_docs/_process_leftovers/` folder state | **EMPTY** as of cycle 7 entry and exit | Second consecutive cycle with zero process leftovers. |
|
||||||
|
| Microsoft.NET.Test.Sdk 17.8.0 transitive `NuGet.Frameworks` NU1902 | OPEN (carried from cycles 4 + 5 + 6) | No cycle-7 touch. Re-listed in deploy_cycle7.md. |
|
||||||
|
| Microsoft.IdentityModel.Tokens / System.IdentityModel.Tokens.Jwt 7.0.3 NU1902 | OPEN (carried from cycles 3 + 4 + 5 + 6) | No cycle-7 touch. Re-listed in deploy_cycle7.md. |
|
||||||
|
| Serilog.AspNetCore 8.0.3 → 10.x recheck | OPEN (carried from cycles 4 + 5 + 6; no 10.x published as of cycle 7 entry) | No cycle-7 touch. Re-listed in deploy_cycle7.md. |
|
||||||
|
| ASPDEPR002 `WithOpenApi(...)` deprecation | OPEN (carried from cycles 4 + 5 + 6) | No cycle-7 touch. Re-listed in deploy_cycle7.md. |
|
||||||
|
| Admin team `iss/aud` confirmation (carried from cycle 3) | OPEN (still required before promoting beyond `dev`) | Re-listed in deploy_cycle7.md. |
|
||||||
|
| `metadata.flightId` authenticated provenance (F1-cy5) | OPEN (long-term, not actionable until flight registry exists) | Re-listed in deploy_cycle7.md. |
|
||||||
|
| `pgcrypto` ops gap (F2-cy5) | OPEN (doc-only fix, ~30 min) | Re-listed in deploy_cycle7.md. |
|
||||||
|
| Deployment runbook: ingress TLS termination + HTTP/2 forwarding (cy6 follow-up) | OPEN (cycle 6 added it; cycle 7 did not pick it up) | Re-listed in deploy_cycle7.md. |
|
||||||
|
| `tile-storage.md` consumer audit post v2.0.0 (cy6 follow-up) | OPEN (cycle 6 added it; cycle 7 did not pick it up) | Re-listed in deploy_cycle7.md. |
|
||||||
|
| Inventory endpoint `estimatedBytes` field (AZ-505 deferral) | OPEN (cycle 6 added it; cycle 7 did not pick it up) | Re-listed in deploy_cycle7.md. |
|
||||||
|
| HTTP/3 / QUIC dev listener (AZ-505 deferral) | OPEN (cycle 6 added it; cycle 7 did not pick it up) | Re-listed in deploy_cycle7.md. |
|
||||||
|
| **NEW (cycle 7)** D-AZ795-1: FluentValidation 12.0.0 → 12.1.1 bump | OPEN — recommended | New follow-up PBI created in `deploy_cycle7.md` § Recommended follow-ups. |
|
||||||
|
| **NEW (cycle 7)** F-AZ795-1 + F-AZ795-2: sanitize `JsonException.Message` + `BadHttpRequestException.Message` in 400 detail | OPEN — recommended | New follow-up PBI created in `deploy_cycle7.md` § Recommended follow-ups. |
|
||||||
|
| **NEW (cycle 7)** Implementation-report exit-gate / fallback formalisation | OPEN — recommended | New follow-up PBI created in `deploy_cycle7.md` § Recommended follow-ups (Action 1 above). |
|
||||||
|
| **NEW (cycle 7)** AZ-795 child-task sweep across remaining public endpoints | OPEN — recommended | New follow-up PBI created in `deploy_cycle7.md` § Recommended follow-ups (Action 3 above). |
|
||||||
|
|
||||||
|
**New leftovers carried out of cycle 7**: **none** (process leftovers folder remains empty). All cycle-7 follow-ups are tracked as recommended PBIs in `deploy_cycle7.md`, not as process leftovers.
|
||||||
|
|
||||||
|
## 7. Suggested Rule / Skill Updates
|
||||||
|
|
||||||
|
| Target file | Change | Rationale |
|
||||||
|
|-------------|--------|-----------|
|
||||||
|
| `.cursor/skills/implement/SKILL.md` + `.cursor/skills/test-spec/SKILL.md` cycle-update mode + `.cursor/skills/document/SKILL.md` task mode + `.cursor/skills/retrospective/SKILL.md` Step 1 | Add: "If the cycle's expected implementation report (`_docs/03_implementation/implementation_report_*_cycle{N}.md`) is absent, fall back to reading (a) the cycle's task spec files in `_docs/02_tasks/done/` filtered by `_dependencies_table.md`'s cycle-{N} block, AND (b) the body of the cycle-{N} implementation commit identified via `git log --oneline --grep='[AZ-...]'` against the tracker IDs in the task specs. Surface the fallback path explicitly in the skill's announce block so the user knows it engaged." | Pattern 1 above. The fallback worked in cycle 7; codifying it makes future cycles resilient. |
|
||||||
|
| `.cursor/skills/security/SKILL.md` Phase 1 (Dependency Scan) | Add: "When the cycle introduces NEW NuGet / npm / cargo packages (not just version bumps), include a 'no-known-CVE confirmation' WebSearch trace in the per-phase report. Cite the GitHub Security Advisories database (or equivalent) and the NuGet audit output. Without the WebSearch trace, the dependency-scan phase produces a 'no findings' result that is not auditable." | Codifies what cycle 7's dependency_scan_cycle7.md already did — make it the explicit standard, not just an instance. |
|
||||||
|
| `.cursor/skills/retrospective/SKILL.md` § "Prerequisite Checks" | Soften the BLOCKING gate from "STOP if no batch reports exist" to "If no batch reports exist, attempt the implement-skill fallback (read task specs + commit body) and produce a retro from those inputs. STOP only if NEITHER batch reports NOR task specs NOR a recent implementation commit on the cycle's tracker IDs can be found." | The current gate is too strict. Cycle 7 had clear inputs (task specs, commit body, security/perf reports) and writing a retro from them is more valuable than skipping. |
|
||||||
|
| `_docs/02_document/contracts/api/error-shape.md` | Add an "Information Disclosure Surface" section listing the messages that `GlobalExceptionHandler` allows through (and the ones it sanitises). Note F-AZ795-1 and F-AZ795-2 as known surfaces; tie the entry to the follow-up PBI for sanitisation. | Makes the error contract's info-disclosure posture explicit and gives the future fix something concrete to land against. |
|
||||||
|
|
||||||
|
## 8. Validations and Sources
|
||||||
|
|
||||||
|
- **Cycle-7 implementation artifacts parsed**: 0 batch reports (process gap — see §4), 0 review files, 0 implementation report; 4 task specs in `_docs/02_tasks/done/` (AZ-794, AZ-795, AZ-796, plus the AZ-794+AZ-795+AZ-796 entry in `_dependencies_table.md`), commit `865dfdb` body (full message), `deploy_cycle7.md`, `perf_2026-05-22_cycle7.md`, `security_report_cycle7.md` + 5 per-phase reports, `_docs/02_document/ripple_log_cycle7.md`.
|
||||||
|
- **Cycle-6 retro compared explicitly**: see §3 trend table.
|
||||||
|
- **Cross-cycle dependency tracking**: AZ-794 / AZ-795 / AZ-796 carry a `Relates` (not `blocks`) link in Jira because they ship together. The cycle-5/6 lesson about explicit dependency-link choice was applied.
|
||||||
|
- **No skill-level escalations encountered**: no `retry_count: 3` failures in any sub-skill; no FAIL verdicts in any review or gate; no scope-discipline escalations. One process gap (Pattern 1) surfaced but the downstream skills' fallback worked.
|
||||||
|
|
||||||
|
## 9. Self-Verification
|
||||||
|
|
||||||
|
- [x] All cycle-7 implementation artifacts parsed (4 task specs, 1 commit body, 1 deploy report, 1 perf report, 6 security reports, 1 ripple log). Process gap (no batch report / implementation report) noted in §1 and §4.
|
||||||
|
- [x] Comparison with cycle-6 retro performed (§3 trend table).
|
||||||
|
- [x] Top 3 improvement actions concrete and actionable (§5).
|
||||||
|
- [x] Suggested rule/skill updates specific and tied to a target file (§7).
|
||||||
|
- [x] New cycle-7 follow-up PBIs (D-AZ795-1, F-AZ795-1/2, implementation-report contract, AZ-795 child sweep) cross-referenced between this retro, `deploy_cycle7.md`, and the security audit reports.
|
||||||
|
- [x] LESSONS.md ring buffer to be appended with top 3 cycle-7 lessons (§4 patterns 1-3 — but Pattern 3 is positive and won't make the buffer; Pattern 4 will instead) — applied in next step.
|
||||||
+6
-6
@@ -37,6 +37,12 @@ If the enum's wire string happens to match a member name case-insensitively (e.g
|
|||||||
|
|
||||||
## Ring buffer (last 15 entries — newest at top)
|
## Ring buffer (last 15 entries — newest at top)
|
||||||
|
|
||||||
|
- [2026-05-22] [process] When the implement skill ships a cycle's batch commit without writing `_docs/03_implementation/implementation_report_*_cycle{N}.md`, downstream skills (test-spec cycle-update, document task mode, retrospective Step 1) must fall back to reading the cycle's task specs in `_docs/02_tasks/done/` plus the commit body via `git log --grep='[AZ-...]'` — codify the fallback in those skills' instructions instead of leaving it as per-cycle improvisation, because the implicit contract between Step 10 and Steps 11-17 broke silently this cycle and only succeeded because every downstream skill happened to be robust enough to substitute (cycle 7: AZ-794+AZ-795+AZ-796 shipped as commit `865dfdb` with no report artifact; doc-skill auto-walked the diff, test-spec read the task specs, retrospective wrote from the deploy + security + perf reports — all worked, but the contract was never formal).
|
||||||
|
Source: _docs/06_metrics/retro_2026-05-22_cycle7.md
|
||||||
|
- [2026-05-22] [testing] When a strict-validation layer ships (`JsonSerializerOptions.UnmappedMemberHandling.Disallow`, FluentValidation rules, explicit DTO `[JsonRequired]`), expect the project's own integration tests to surface latent bugs the prior lenient defaults had been masking — silent PascalCase fallback property names, out-of-range fixture coordinates, wrong-cased JSON keys; correct them in the same PR or the test suite goes red and the strict layer looks like a regression instead of the bug-finder it is (cycle 7: `IdempotentPostTests.RoutePoint` had been posting `{"Lat":...}` against a `[JsonPropertyName("lat")]` DTO for months; the new strict deserializer caught it and the 2-line payload fix landed alongside the strict layer).
|
||||||
|
Source: _docs/06_metrics/retro_2026-05-22_cycle7.md
|
||||||
|
- [2026-05-22] [architecture] Contract MAJOR bumps for projects with ≤2 known consumers should ship without a wire-format adapter — the cost of maintaining a dual-accepting endpoint outweighs the benefit when the operator runbook can coordinate the consumer cut-over directly; only invest in an adapter when consumer count or release-cadence asymmetry makes coordinated cut-over impractical (cycle 7: `tile-inventory.md` 1.0.0 → 2.0.0 renamed `tileZoom/tileX/tileY → z/x/y` and shipped breaking; the only consumer is `gps-denied-onboard` and the AZ-777 follow-up loop handled the coordination via the operator runbook in `deploy_cycle7.md`).
|
||||||
|
Source: _docs/06_metrics/retro_2026-05-22_cycle7.md
|
||||||
- [2026-05-12] [tooling] Kestrel `HttpProtocols.Http1AndHttp2` silently serves only HTTP/1.1 over a plaintext listener — ALPN requires TLS, so any "enable HTTP/2" task without TLS in its definition-of-done will downgrade transparently and the only log line is at INFO; tasks that mention HTTP/2 / h2 / multiplexing / ALPN MUST resolve the TLS-vs-h2c choice at spec-write time and the test gate MUST assert `HttpVersion == 2.0` not just a 200 (cycle 6: AZ-505 AC-5 first landed on h2c plaintext, required a post-merge TLS+ALPN pivot with dev-cert plumbing across compose/tests/perf/docs).
|
- [2026-05-12] [tooling] Kestrel `HttpProtocols.Http1AndHttp2` silently serves only HTTP/1.1 over a plaintext listener — ALPN requires TLS, so any "enable HTTP/2" task without TLS in its definition-of-done will downgrade transparently and the only log line is at INFO; tasks that mention HTTP/2 / h2 / multiplexing / ALPN MUST resolve the TLS-vs-h2c choice at spec-write time and the test gate MUST assert `HttpVersion == 2.0` not just a 200 (cycle 6: AZ-505 AC-5 first landed on h2c plaintext, required a post-merge TLS+ALPN pivot with dev-cert plumbing across compose/tests/perf/docs).
|
||||||
Source: _docs/06_metrics/retro_2026-05-12_cycle6.md
|
Source: _docs/06_metrics/retro_2026-05-12_cycle6.md
|
||||||
- [2026-05-12] [testing] When a test bypasses Dapper to gain access to a feature Dapper doesn't expose (e.g. `ANY($1::uuid[])` array params, raw `NpgsqlCommand` for performance fixtures), the test owns the Npgsql type-conversion contract that Dapper used to handle silently — `DateTime.Kind=Utc` must be converted to `Unspecified` before binding into a `timestamp without time zone` column (cycle 6: AZ-505 introduced two Dapper-bypassing paths and all three new test files hit the same `Cannot write DateTime with Kind=UTC` error until `DateTime.SpecifyKind(..., Unspecified)` was added at the bind sites).
|
- [2026-05-12] [testing] When a test bypasses Dapper to gain access to a feature Dapper doesn't expose (e.g. `ANY($1::uuid[])` array params, raw `NpgsqlCommand` for performance fixtures), the test owns the Npgsql type-conversion contract that Dapper used to handle silently — `DateTime.Kind=Utc` must be converted to `Unspecified` before binding into a `timestamp without time zone` column (cycle 6: AZ-505 introduced two Dapper-bypassing paths and all three new test files hit the same `Cannot write DateTime with Kind=UTC` error until `DateTime.SpecifyKind(..., Unspecified)` was added at the bind sites).
|
||||||
@@ -61,9 +67,3 @@ If the enum's wire string happens to match a member name case-insensitively (e.g
|
|||||||
Source: _docs/06_metrics/retro_2026-05-12_cycle3.md
|
Source: _docs/06_metrics/retro_2026-05-12_cycle3.md
|
||||||
- [2026-05-12] [process] ACs that require cross-repo writes should be tagged with the target workspace and rendered separately in the traceability matrix — mixing them with in-workspace ACs makes "correctly deferred" indistinguishable from "incomplete work" (cycle 3: AZ-494 AC-7 deferred for the suite-repo write; matrix renders as `◐ deferred` which is ambiguous).
|
- [2026-05-12] [process] ACs that require cross-repo writes should be tagged with the target workspace and rendered separately in the traceability matrix — mixing them with in-workspace ACs makes "correctly deferred" indistinguishable from "incomplete work" (cycle 3: AZ-494 AC-7 deferred for the suite-repo write; matrix renders as `◐ deferred` which is ambiguous).
|
||||||
Source: _docs/06_metrics/retro_2026-05-12_cycle3.md
|
Source: _docs/06_metrics/retro_2026-05-12_cycle3.md
|
||||||
- [2026-05-11] [testing] Test helpers shared across unit + integration projects must live in one consolidated location — duplicate near-identical copies will diverge and require parallel fixes (cycle 2: `JwtTokenFactory.cs` and `JwtTestHelpers.cs` had the same `Expires < NotBefore` bug fixed in separate commits).
|
|
||||||
Source: _docs/06_metrics/retro_2026-05-11_cycle2.md
|
|
||||||
- [2026-05-11] [process] Deferred-status NFR entries are allowed at most ONCE per NFR — if a Deferred NFR has not landed by the end of the cycle that follows the one in which it was deferred, the harness work must be promoted to a real PBI before any new NFR is accepted as Deferred (cycle 2 inherited cycle 1's PT-07 + added PT-08 + JWT-attach script-rot).
|
|
||||||
Source: _docs/06_metrics/retro_2026-05-11_cycle2.md
|
|
||||||
- [2026-05-11] [testing] Integration tests must explicitly reset DB state at startup — relying on wallclock seeds or "tests probably won't collide" is a workaround, not isolation; the persistent Postgres volume in docker-compose makes test data accumulation the default state (cycle 2: `UavUploadTests._coordinateCounter` collision was patched with a wallclock seed instead of a real DB-reset hook).
|
|
||||||
Source: _docs/06_metrics/retro_2026-05-11_cycle2.md
|
|
||||||
|
|||||||
@@ -2,14 +2,14 @@
|
|||||||
|
|
||||||
## Current Step
|
## Current Step
|
||||||
flow: existing-code
|
flow: existing-code
|
||||||
step: 10
|
step: 9
|
||||||
name: Implement
|
name: New Task
|
||||||
status: in_progress
|
status: not_started
|
||||||
sub_step:
|
sub_step:
|
||||||
phase: 12
|
phase: 0
|
||||||
name: tracker-in-testing
|
name: awaiting-invocation
|
||||||
detail: "batch 1 of 1; AZ-794 + AZ-795 + AZ-796 implementation complete; full Docker Compose suite green (311 unit tests + integration tests including 16 new inventory-validation cases); task files archived todo/ -> done/; ready to commit + push and transition Jira tickets to In Testing"
|
detail: ""
|
||||||
retry_count: 0
|
retry_count: 0
|
||||||
cycle: 7
|
cycle: 8
|
||||||
tracker: jira
|
tracker: jira
|
||||||
auto_push: true
|
auto_push: true
|
||||||
|
|||||||
Reference in New Issue
Block a user