acdl/.ciagent/REVIEW.md

ACDL v1.1 Milestone — Multi-Persona Code Review

Reviewer: ci-code-reviewer (model: glm-5.2) Scope: v1.1 milestone — Phases 06–10 (tags v1.1.1..v1.1.5), diff v1.1.0..HEAD Date: 2026-07-21 Verdict: READY TO SHIP — 0 P0, 1 P1 (carried-forward), 0 P2 new

---

Lens 1 — Correctness

The schemas + Python modules + Terraform implement what the decisions + ARCHITECTURE.md committed. Spot-checks all pass.

Findings

• schemas/ir.schema.json (REQ-17): resources / relationships / composition (max-depth-5) / policy hooks (via PolicyCheckResult consumer) all present per §12.1. Substrate-agnostic: aws_s3_bucket appears ONLY in $comment and description strings (which explain the IR→Terraform mapping); it does NOT appear in any constraining keyword (enum/const/pattern/required). The schema body uses IR types (aws:s3:bucket). Correct.
• schemas/contract.schema.json (REQ-22, W3.E): per-env mandatory via allOf if/then — qa requires validation.e2eSuite+validation.loadTest; prod requires runbook+dashboard+oncall; dr requires drDrillRef. The profile:agentic conditional is if: {required:[profile], profile:{const:agentic}} → then: {required:[naturalLanguageIntent]} — this is the fixed form (requires profile to be present before checking const), not the Phase 07 initial bug. Verified: prod-missing-runbook rejected; agentic-without-NLI rejected; qa-without-validation rejected; dr-without-drDrillRef rejected; dev + agentic-with-NLI accepted. Correct.
• acdl_platform/confidence_signal.py (REQ-19, D-040): WEIGHTS sum to 1.0 (verified: 0.30+0.25+0.10+0.15+0.10+0.10 = 1.0). PENALTY["critical"] = None (hard-override sentinel). The critical-override short-circuit (if p is None: return Signal(0.0, "block", ...)) returns BEFORE the score = max(0.0, min(1.0, base - penalty)) clamp. Dev-warn→block flip present (if environment == "dev" and band == "warn": band = "block"). The policy input key is read as inputs.get("policy") (not policy_results) — matches the Phase 10 e2e run_spike_e2e.sh which passes inputs = {"policy": pcr, ...}. Adversarial test: a critical-fail PCR → score=0.0 band=block reasons=['CRITICAL_OVERRIDE:...']. Correct.
• acdl_platform/contract_resolver.py (REQ-27): resolve() loads YAML → validates against contract.schema.json → looks up L2 in registry → loads composition.json → maps wires → emits IR → validates against ir.schema.json. Wire mapping verified: contract.inputs.bucket_name → child.inputs.bucket_name via wires.bucket_name.{target:s3, input:bucket_name}. Resolved spike IR has resources[0].inputs = {bucket_name: acdl-spike-bucket, region: us-east-1}. Prod-missing-runbook raises jsonschema.ValidationError (not a generic ValueError). Correct.
• acdl_platform/outbox_writer.py (D-044, D-P10-3): SHA-256 over canonical JSON (sort_keys=True, separators=(",", ":")). prev_event_hash defaults to "GENESIS". DynamoDB item shape: PK contractId (S), SK eventType#eventTs (S), TTL expire_at (N, now+365d). Append-only (put_item only; 0 delete_item/update_item). Correct.
• adapters/terraform/adapter.py (REQ-26, D-P10-1): TYPE_MAP = {aws:s3:bucket -> aws_s3_bucket}. Backend key derived from stack name: spike/l2-static-asset/terraform.tfstate (verified). Unknown IR type raises ValueError. Resources array handling is shape-driven (iterates ir_instance["resources"]; works for both l1 and l2 IR). Correct.
• adapters/terraform/policy/checkov_adapter.py (REQ-18, D-043): RULE_MAP has exactly 11 Checkov rule IDs (CKV_AWS_41/45/46/20/57/24/25/1/40/7/33). The ACDL_TAG_NAMING SKIPPED record is appended (severity: info, result: skipped). Tolerates both Checkov JSON shapes — nested {framework: {results: {...}}} and legacy {framework: {passed_checks:...}} (the results = body.get("results", body) fallback). Correct.

Verdict: PASS — no issues.

---

Lens 2 — Testing

The verify scripts are real gates that fail on regression, not presence checks.

Findings

• scripts/verify_phase07.sh: Check 2 uses jsonschema.Draft202012Validator.check_schema(...) || fail — actually validates the 3 schemas as Draft 2020-12 (fails if a schema is broken). Check 8 cross-checks the spike contract against contract.schema.json via jsonschema.validate(...) || fail. Check 9 cross-checks a minimal IR against ir.schema.json. Every check has || fail. Real gate.
• scripts/verify_phase10.sh: 8 checks, each with || fail. Check (h) is the REQ-28 substrate-agnostic scan. Synthetic leak test performed: appended LEAK = "aws_s3_bucket" to acdl_platform/separation_of_duties.py and ran the Check (h) grep — it caught the leak (acdl_platform/separation_of_duties.py:44: LEAK = "aws_s3_bucket"), then reverted. The check also scans modules-ir/ JSON for aws_* resource-type VALUES (excluding description/$comment strings). Real gate.
• scripts/run_spike_e2e.sh + scripts/run_spike_plan.sh: touch real AWS — terraform init/validate/plan -lock=false + checkov + DynamoDB put_item/query. NOT stubbed (the spike key is loaded from gitignored .env.secrets). The e2e runner uses || fail on every step, so a DynamoDB outage or terraform failure exits 1 (verified: outbox write failure propagates via || fail "outbox write failed"). Real e2e.

Verdict: PASS — no issues.

---

Lens 3 — Security

AWS key handling (D-034/D-039), IAM least-privilege, gitignore discipline, no secrets in commits. All clean.

Findings

• No leaked key IDs in executable code: git log v1.1.0..HEAD -p | grep -iE "AKIA[A-Z0-9]{16}" | grep -v "^#" returns matches ONLY inside .ciagent/VERIFY.md (the Phase 09 narrative — the carried-forward P1-1). No .py, .tf, .json, .yaml, or .sh file contains an AKIA… key ID. Clean.
• No leaked secret keys: git log v1.1.0..HEAD -p | grep -iE "aws_secret_access_key.*=.*[A-Za-z0-9/+=]{40}" | grep -v "^#" returns nothing. Clean.
• terraform/bootstrap/spike_runner_policy.json (REQ-23): least-privilege. Allow actions: s3:{PutObject,GetObject,DeleteObject,ListBucket,GetBucketLocation,GetBucketVersioning}
  • dynamodb:{GetItem,PutItem,DeleteItem,UpdateItem,Query,Scan,DescribeTable}
  • sts:GetCallerIdentity. No iam:*, no ec2:*, no s3:CreateBucket, no s3:DeleteBucket, no terraform apply (apply is out of spike scope). DenyEverythingElse NotResource lists exactly 3 ARNs (state bucket + bucket objects + outbox table); everything else is denied. Correct.
• Gitignore discipline: .env.secrets, terraform/bootstrap/.bootstrap_state.json, terraform/spike/.terraform/, terraform/spike/.terraform.lock.hcl, terraform/spike/tfplan, terraform/spike/*.tfstate* all gitignored (git check-ignore confirms each). Correct.
• Outbox write is append-only: grep -c "delete_item|update_item" outbox_writer.py = 0 (only put_item). Correct.
• E2E runner is plan-only: grep -c "terraform apply" run_spike_e2e.sh = 0 (only init + validate + plan). Correct.

P1 (carried-forward, NOT auto-fixed)

• P1-1: The .ciagent/VERIFY.md Phase 09 narrative contained two AWS access key IDs — AKIA…SPIKE (the rotated spike key id) and AKIA…ROOT-DEACTIVATED (the deactivated root key id). Confirmed present in the v1.1 audit (grep -c returned 2). These are public identifiers, not secret pairs; they live in the .ciagent/ audit narrative, not in any executable code path. Recommended for a future hygiene redaction pass (replace with AKIA…SPIKE / AKIA…ROOT-DEACTIVATED placeholders). Non-blocking for v1.2 ship; flagged for post-hoc review.

Verdict: PASS — 1 carried-forward P1 (non-blocking).

---

Lens 4 — Performance

Not a concern for the spike (plan-only, single resource, no load). Skipped.

---

Lens 5 — Maintainability

The acdl_platform/ rename, substrate-agnostic boundary, and decision trail are all consistent.

Findings

• acdl_platform/ rename (Phase 08 prep, fixing the stdlib platform shadow): consistently applied across scripts/verify_phase06.sh, scripts/verify_phase07.sh, README.md, and the Python imports (import acdl_platform.confidence_signal as c in run_spike_e2e.sh). grep -l acdl_platform confirms all three files reference the renamed dir. Consistent.
• Decision trail: every schema/module cites its source. Sampled 3 files:
  • acdl_platform/confidence_signal.py cites REQ-19, D-040, ARCHITECTURE.md §8.
  • acdl_platform/contract_resolver.py cites ARCHITECTURE.md §12.8.
  • schemas/ir.schema.json cites ARCHITECTURE.md §12.1, §3, W3.D. Citations present.
• Spike-vs-v1.2 boundary documented in each design doc: acdl_platform/audit_ledger_design.md, acdl_platform/hitl_matrix_design.md, and .ciagent/PLAN.md all reference v1.2. Boundary documented.
• Substrate-agnostic boundary (REQ-28): the adapter is the only substrate-specific code. acdl_platform/ Python is clean (verified by the Check (h) grep + the synthetic leak test). modules-ir/ JSON data files contain only IR types (aws:s3:bucket); aws_s3_bucket appears only in description/$comment strings that explain the mapping. Boundary holds.

Verdict: PASS — no issues.

---

Lens 6 — Adversarial

Tried to break the spike. All failure modes handled correctly.

Findings

• contracts/spike.yaml with environment: prod (missing runbook): the contract schema rejects it via the allOf if/then (runbook is a required property when environment == "prod"). contract_resolver.py raises jsonschema.ValidationError (not a generic ValueError). Handled.
• IR instance with a resource type not in TYPE_MAP (e.g. aws:ec2:instance): the adapter raises ValueError("unknown IR type 'aws:ec2:instance' (adapter spike handles aws:s3:bucket only)"). Handled.
• Confidence signal gets a critical-fail PolicyCheckResult: hard-overrides to score=0.0, band=block, reasonCodes=['CRITICAL_OVERRIDE:...']. The short-circuit returns BEFORE the score clamp. Handled.
• Outbox write fails (DynamoDB unreachable): outbox_writer.py raises (boto3 put_item propagates the exception); run_spike_e2e.sh line 93 uses || fail "outbox write failed" → exit 1. Handled (no silent success).
• Missing confidence input (e.g. nfrs absent): compute() returns Signal(0.0, "block", {}, ["INPUT_MISSING:nfrs"]). Handled.

Verdict: PASS — no issues.

---

P0 / P1 / P2 Summary

Severity	Count	Action
P0	0	none (no auto-fix needed)
P1	1	P1-1 (carried-forward): two AWS access key IDs in .ciagent/VERIFY.md Phase 09 narrative — flagged for post-hoc hygiene redaction; non-blocking
P2	0	none

---

Milestone verdict

v1.1 milestone: READY TO SHIP

• 0 P0 issues (no blocking fixes).
• 1 P1 carried-forward (non-blocking; flagged for post-hoc review).
• All 5 lenses pass. REQ-16..28 satisfied. The IR commitments hold (REQ-28).
• Ready for the COMPLETE gate → ship v1.2.0 → audit.