The adapter reads env.state_backend.bucket from the env JSON when present
(fallback to the computed nova-tfstate-{account_id}-{region} pattern for
backwards compat). dev.json bound to the real account 581513795199 +
bucket nova-tfstate-581513795199-us-east-1 (D-203). qa/prod/dr stay
placeholder (account_id 000000000000 — the pilot-readiness policy blocks
apply on placeholder, D-208). dynamodb added to the adapter test
EXPECTED_L1_KEYS + a resolution/emission test.
---ci---
project: acdl
phase: 3
milestone: v1.26
status: execute
wave: W3
---
Code review (correctness lens) found the same P5 mechanical-edit defect
in two more test files: the ACDL_* fallback delenv was replaced with a
duplicate NOVA_* delenv (leaving a dead duplicate line, a stale 'ACDL_*
fallback until P5' comment, and the ACDL_* var no longer cleaned).
- tests/test_route_halt_artifact.py: two sites (stderr-fallback +
outbox-fallback) each deleted NOVA_SOD_HALT_TOPIC_ARN twice.
- tests/test_adapter.py::test_default_remote_state_key: deleted
NOVA_REMOTE_STATE_KEY twice.
With core/env.py NOVA-only as of P5, a single NOVA_* delenv is the
correct precondition. Collapsed to one delenv per var + updated comments.
---ci---
project: acdl
phase: 5
milestone: v1.15
status: verify
lessons:
- P0 fix applied: duplicate monkeypatch.delenv('NOVA_*') in test_route_halt_artifact.py (2 sites) + test_adapter.py collapsed to a single delenv consistent with the P5 NOVA-only core/env.py.
---/ci---
---
ci---
project: acdl
phase: 70
milestone: v1.12
status: verify
---
/ci---
Code review (P70) flagged 2 P1 testing gaps:
1. No end-to-end terraform validate test for the microservice (the real
CAP-013 surface). A future refactor could re-break the dedup and the
suite would stay green.
2. No unit test for the _child_id helper / id_remap / dedup merge.
Added 6 tests (38 adapter tests total, 522 suite total):
- test_microservice_dedup_names_modules_by_child_id: asserts module 'alb'
+ 'service' appear, expanded sub-ids do NOT.
- test_microservice_dedup_rewrites_stack_outputs: service_arn -> module.service,
lb_arn -> module.alb.
- test_microservice_dedup_rewrites_cross_module_refs: lb_target_group_arn ->
module.alb.target_group_arn (not module.alb-targetgroup).
- test_microservice_emits_valid_terraform: end-to-end terraform init +
validate on the microservice main.tf (locks in CAP-013).
- test_single_resource_returns_id_verbatim / test_multi_resource_returns_common_prefix:
unit tests for _child_id.
P2 nits (noted, not fixed): the ci-vpc-apply/destroy 'if' uses != 'plan'
rather than == 'full' (stricter but not exploitable); _child_id docstring
could note commonprefix is character-wise. Both are post-hoc.
The ci.yml workflow's test job runs test_adapter.py which includes
test_s3_instance_emits_valid_terraform — this test runs terraform
init+validate as a subprocess. Previously Terraform was not installed
in the CI job, causing FileNotFoundError. Now both the test and
check-only jobs install Terraform 1.9.* via the HashiCorp apt repo.
Reverted the skip-when-terraform-missing logic in the test — Terraform
is now always available in CI.
---ci---
project: acdl
phase: P59
milestone: v1.11
status: execute
---/ci---
The test_s3_instance_emits_valid_terraform test runs terraform init+validate
as a subprocess. In CI, the ci.yml workflow doesn't install Terraform (only
the modules-lifecycle workflow does). The test now skips gracefully when
terraform is not on PATH, using shutil.which('terraform').
---ci---
project: acdl
phase: P59
milestone: v1.11
status: execute
---/ci---
---ci---
project: acdl
phase: 32
milestone: v1.8
status: execute
---/ci---
- All 11 L1 primitives now have deletion_protection NFR (boolean, default true).
- Adapter emits `lifecycle { prevent_destroy = true }` when NFR is true;
omits it when false. Default is true when NFR is absent.
- L2 composition resolver propagates inputs.deletion_protection to all
children NFRs. When false, all resources get deletion_protection=false.
- Stack schema updated with optional features object (deletion_protection,
uptime_enabled).
- Contract schema description updated to document deletion_protection
and uptime_enabled inputs.
Tests: +5 (307 -> 312). All pass.
---ci---
project: acdl
phase: 31
milestone: v1.8
status: execute
---/ci---
- New kms-key L1 primitive (aws:kms:key) with enable_key_rotation=true
(AWS-managed annual rotation, D-075). Registered in registry.json.
- Adapter TYPE_MAP expanded for aws:kms:key + aws:kms:alias.
- Adapter emits enable_key_rotation from NFR.
- S3 adapter emits server_side_encryption_configuration with KMS when
kms_key_arn provided; managed KMS fallback with stderr warning when not.
- All 10 existing L1 primitives now have encryption_enabled NFR (default true).
- s3, rds, ecr, ecs-service, ecs-cluster have kms_key_arn input.
- Both L2 compositions (static-assets, microservice) now include a kms-key
child + wires connecting kms_key_arn to children.
- L2 stack outputs include kms_key_arn.
Tests: +7 (300 -> 307). All pass. run_platform.sh --check-only green
(static-assets now resolves to 5 resources with the CMK).
---ci---
project: acdl
phase: 20
milestone: v1.5
status: review
verdict: READY TO SHIP
p0: 1 (fixed — contract path resolution in deploy workflow)
p1: 6 (flagged post-hoc)
---/ci---
Multi-persona review of v1.5 phase 20 (docs + reusable deploy workflow).
P0 (blocking) — AUTO-FIXED:
- C1: scripts/run_platform.sh contract path resolution broken in deploy
workflow. The reusable workflow invokes run_platform.sh from the consumer
workspace root with a relative contract path (.acdl/contract.yaml), but
run_platform.sh does `cd "$ROOT"` (platform repo) early, so the relative
path resolved against the platform repo and the pipeline could never run.
Fix (commit 75c2274): capture CALLER_CWD before cd "$ROOT"; resolve
caller-supplied relative paths against CALLER_CWD; default no-arg contract
stays relative to ROOT (preserves platform-local CI). Reproduced pre-fix;
verified post-fix.
P1 (important) — FLAGGED FOR POST-HOC REVIEW (do not block ship):
- C2: ref: v1.4 in the deploy workflow platform checkout — no v1.4 tag exists
(only v1.4.0 / v1.4.1). Operator must create a floating v1.4 tag or change
the ref to v1.4.1.
- C3: modules/l2/{static-asset,microservice}/README.md still use @v1 in their
Usage examples; missed by the v1.4 bump.
- S1: static-key override is not wired. ACDL_AWS_* env vars on the OIDC step
are not read by aws-actions/configure-aws-credentials@v4 (it reads AWS_*
or its own access-key/secret-key inputs). The README/CONSUMER_GUIDE claim
a working override that doesn't function as written. Needs a conditional
step or renamed env vars + input wiring.
- S2: README overstates ABAC repo:org/repo:ref:... scoping. The workflow
constructs a numeric role name (github.repository_id); the actual claim
enforcement lives in the IAM trust policy, not in this workflow.
- T1: no deploy-workflow triggers conformance test (CI workflow has one;
deploy doesn't). Minor — reusable workflows use workflow_call, not push
triggers, but the contract's triggers field is then unenforced.
- A1: terraform/spike/terraform.tf uploaded as artifact leaks the AWS account
ID via the state-backend bucket name. Recommend excluding terraform.tf or
gating artifact upload to non-public repos.
P2 (nits) — listed for awareness: floating-tag terminology imprecision (M1),
header comment "Gitea Actions" in the GitHub copy (M2, intentional byte-
identical), pip install split (P1-perf), comment drift in pipelines/deploy.yaml
header (C4), module README internal inconsistency (C5).
Verdict: READY TO SHIP. The one P0 is fixed. The 6 P1s are post-hoc items —
the deploy workflow is a scaffold whose first real consumer run requires
operator setup (tag, IAM role, secrets) that gates go-live. The P1s should
be addressed before any consumer invokes uses: acdl/.gitea/workflows/
deploy.yml@v1.4 in earnest.
Tests: 154 pass (19 new). run_ci.sh green.
Jon Chery