acdl/.ciagent/REVIEW.md

ACDL v1.2 Milestone — Multi-Persona Code Review

Reviewer: ci-code-reviewer (model: glm-5.2) Scope: v1.2 milestone — Phases 11–16 (tags v1.2.1..v1.2.6), diff v1.2.0..HEAD Date: 2026-07-21 Verdict: READY TO SHIP — 1 P0 (operator action, non-code), 1 P1 (adapter hardening for v1.3)

---

Summary

v1.2 hardens the v1.1 spike, simplifies the setup, rewrites the docs, and takes the platform to a real ECS Fargate microservice deployment. 6 phases shipped (v1.2.1–v1.2.6): research + README, NFR hardening + simplification, 6 ECS L1s + adapter generalization, l2-microservice + contract schema + resolver wiring, consumer repo + terraform apply (blocked by IAM), capstone e2e.

P0 issues

P0-IAM (operator action, NOT a code fix)

The terraform apply (Phase 15) is blocked by the live IAM policy. The Phase 12 spike_runner_policy.json expansion (ECS/ECR/ELB/IAM/EC2) was committed to the repo but never pushed to the live AWS account — the root key was deactivated per D-034, and the acdl-spike-runner user cannot self-elevate via iam:PutUserPolicy.

Unblock step (operator):

ACDL_BOOTSTRAP_AWS_ACCESS_KEY_ID=<root-or-admin-key> \
ACDL_BOOTSTRAP_AWS_SECRET_ACCESS_KEY=<root-or-admin-secret> \
python3 terraform/bootstrap/create_iam_user.py

This re-PUTs the expanded policy (idempotent). Then terraform apply (plan is valid, 13 to add) → live ECS Fargate service → HTTP 200.

Why this is not a code fix: the code + plan are correct + verified (terraform validate + terraform plan succeed). The blocker is purely the live IAM policy state, which requires a privileged credential that was deliberately deactivated (D-034 closure).

P1 issues

P1-1 (adapter hardening, deferred to v1.3)

The adapter's ECS/ALB/VPC emission includes several resource-type-specific defaults (desired_count = 1, launch_type = "FARGATE", target_type = "ip", load_balancer_type = "application", tags = { Name = ... }, family = "app"). These are pragmatic for the v1.2 spike but should be parameterized via the L1 interfaces in v1.3 (the adapter should remain a thin translator; these defaults belong in the L1 contract, not the adapter).

Per-lens review

Correctness

• The contract→IR→adapter pipeline produces valid HCL (terraform validate passes; terraform plan succeeds with 13 to add).
• The v1.1 S3 regression passes (byte-identical main.tf) across all adapter changes (ref emission, JSON-string detection, ECS service network_configuration/load_balancer, listener default_action, target group defaults, VPC tags, IGW emission, managed_policy_arns).
• The intra_refs mechanism (L1-declared refs between sub-resources of the same L1) correctly resolves subnet→vpc.vpc_id + routetable→vpc.vpc_id.
• The resolver's array-form wires + child→child ref: emission are backward-compatible (v1.1 single-object wires still work).

Testing

• 6 per-phase verify scripts (verify_phase11.sh..verify_phase16.sh), all green.
• The capstone verify (verify_phase16.sh) exercises every v1.2 deliverable + the v1.1 regression + NFR + docs + L1 catalog + outbox.
• The terraform apply + HTTP 200 check are the operator's post-unblock step (documented in Phase 15/16 VERIFY).

Security

• No credentials introduced. The P1-1 AWS key ID redaction (carried from v1.1) is closed — no live key IDs in .ciagent/.
• The IAM blocker is a security positive: least-privilege enforced; the policy push requires a deliberate privileged action.
• The assume_role_policy in the contract is the standard ECS task execution trust policy (not a secret).

Performance

• N/A (this milestone is about correctness + simplification, not perf).

Maintainability

• run_platform.sh consolidates two scripts (D-048) — one entry point.
• The adapter's TYPE_MAP + INPUT_MAP + OUTPUT_MAP tables make adding future L1s a table-extension, not new emit logic.
• The intra_refs mechanism is a clean L1-declared extension.

Adversarial

• The terraform apply failure was investigated thoroughly: the subagent attempted one fix (adapter HCL correctness), then correctly identified the IAM root cause + documented the unblock step. No half-applied AWS state (all 5 creates failed at the API; state is empty).
• The TERRAFORM_APPLY_BLOCKED + MILESTONE_CAPSTONE_VERIFIED evidence events truthfully record the state (not faking success).

Conclusion

v1.2 is READY TO SHIP. The 1 P0 is an operator action (not a code fix), and the 1 P1 is deferred to v1.3. The milestone's code is complete + verified: the platform flow works end-to-end up to terraform plan (13 to add), and the one remaining step (terraform apply → live ECS service) is the operator's IAM policy push. Ship tag: v1.3.0 (feature milestone, next minor per ship.md — v1.1 shipped v1.2.0).