acdl/.ciagent/archive/REVIEW.md

Nova v1.16 — Multi-Persona Code Review (final phase P21)

Reviewer: lead-developer (model: glm-5.2) Scope: v1.16 milestone — 22 tags (v1.15.5..v1.15.26), 20 execution phases + final. Squash-merged to main via milestone/v1.16-nova-simplification. Date: 2026-07-30

Historical note: REVIEW.md was reconstructed at v1.16 P21 (the v1.3–v1.15 reviews were not persisted or were overwritten per the established convention). The v1.16 review overwrites prior content.

Review approach

The v1.16 milestone is an NFR sweep (no new features). Each of the 20 execution phases shipped with a 4-layer verify (structural/behavioral/ security/quality) + run_ci.sh 3-stage PASS at every phase boundary. The final-phase review (P21) is a milestone-level cross-phase check, not a per-phase re-review (the per-phase verify already ran).

P0 issues (0)

No blocking issues found. The 4-layer verify at each phase boundary + the regression gate (D-118, 18V+4S at P9 + P21) are the structural controls. No P0 was auto-applied at P21.

P1 issues (0)

No P1 issues flagged. The grill binding decisions (G-111..G-113) were incorporated into the plan before execution; the regression gate (G-111) passed at both checkpoints (P9 + P21).

P2 issues (2 — post-hoc, non-blocking)

P2-1: Onboarding framing (E-002, deferred from grill)

[scope] .ciagent/PROJECT.md, .ciagent/ROADMAP.md

The grill escalation E-002 (confidence 0.55) flagged that the PROJECT.md framing "first self-service onboarding request path" may over-promise relative to a request-acceptance path that writes a pending row + generates an env-file + proves the role Terraform offline but never fulfills (no live role grant). The milestone is internally consistent with D-113 (request-path only) — the wording is the only risk. The ROADMAP/PROJECT use "request path" (not "request-fulfillment"), and the Out-of-Scope section explicitly defers real AWS provisioning. Accepted as-is — the framing is accurate for what was delivered (a request path, not a fulfillment path).

P2-2: REVIEW.md + AUDIT.md not updated during the run

[maintainability] .ciagent/REVIEW.md, .ciagent/AUDIT.md

REVIEW.md still held v1.11 content during the v1.16 run (the per-phase verify ran but wasn't persisted to REVIEW.md until P21). AUDIT.md held v1.15 content. Both are reconstructed at P21 (this review + the audit running now). This matches the established convention (REVIEW.md is overwritten at milestone complete; the per-phase verify commits are the record). Not a defect.

What is correct

• State-bucket drift fix (P1): adapter.py:117 now emits nova-tfstate-* (matching the live bucket renamed in v1.15 P4). The new test_adapt_emits_nova_state_bucket regression guard asserts this.
• Kyverno label fix (P1): require-resource-labels.yml enforces nova:* labels (consistent with nova_tagging.py hard-fail on acdl:*). No policy contradiction.
• Ingestor defense-in-depth (P10): fail-closed on missing IAM identity (401, not silent pass); env enum derived from core/environments/ (not hardcoded). The NOVA_LAMBDA_LOCAL_BYPASS env allows local/stub testing without blocking the fail-closed path.
• Payload validation (P11): 256 KB size cap + contract.schema.json validation before the DynamoDB write; aligned error/stackTrace caps (both 10000).
• Regression gate (G-111): CAP-013..016 return Skipped (not Decayed/Broken) for the post-teardown steady state (D-096). passed accepts Skipped. Gate passes at 18V+4S.
• Workflow generator (P8): sync_workflows.py + workflows-src/ single source; the byte-identity test is replaced with a generator- output test (--check exits 0). The 3 pairs are no longer hand-synced.
• Onboarding request path (P18-P20): schema + Lambda action (pending CMDB row, no AWS resources) + env-file autogen + offline-proven cross-account Terraform. Self-service message (no "contact the platform team"). Real AWS provisioning explicitly deferred (D-113/D-114).
• Splits (P12/P13): contract_resolver + regression_verify split with re-export shims; G-113 one-way import direction documented. All tests pass without modification (backwards compat preserved).
• DX (P15-P17): --help works + documents all 9 flags; workflows README catalogs all 7 workflows; getting-started is offline-first.
• Regression gate: 18 Verified + 4 Skipped at P9 + P21 (0 Decayed/ Broken). The 4 Skipped are the post-v1.11-teardown live-AWS caps.

Test coverage assessment

~635 tests pass (was ~620 at v1.15.4). New test files:

• tests/test_onboarding.py (3 tests — env-file generation)
• tests/test_onboarding_terraform.py (3 tests — terraform validate + tags)
• tests/test_docs_coverage.py (expanded — workflows README catalog)

New tests in existing files: test_adapt_emits_nova_state_bucket, test_onboarding_message_says_nova_not_acdl, test_no_identity_fails_closed, test_no_identity_passes_with_local_bypass, test_oversized_contract_rejected, test_schema_invalid_contract_rejected, TestNarrowedException (2 tests), TestOnboardConsumer (3 tests), TestOnboardingMessageSelfService (2 tests), test_sync_workflows_check_passes.

Verdict

PASS — 0 P0, 0 P1, 2 P2 (post-hoc, accepted). The v1.16 NFR milestone is complete. All 20 requirements (REQ-165..184) satisfied; regression gate 18V+4S; CI 3-stage PASS at every phase boundary. The onboarding request path is self-service; real AWS provisioning deferred. The state-bucket drift + Kyverno label contradiction (the two correctness regressions from the v1.15 rebrand) are fixed with regression guards.