# 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 contains two AWS access
  key IDs — `AKIAYOZHMKZ7RK26N66W` (the rotated spike key id) and
  `AKIAYOZHMKZ772SINHFX` (the deactivated root key id). Confirmed still present
  (`grep -c` returns 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.