From 2397336cbbd6aaa037eecfaf58275cbe13ea045d Mon Sep 17 00:00:00 2001 From: Jon Chery Date: Tue, 28 Jul 2026 12:03:10 +0000 Subject: [PATCH] =?UTF-8?q?verify(P57):=20code=20review=20=E2=80=94=203=20?= =?UTF-8?q?P0=20auto-fixed,=202=20P1+=20flagged?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Multi-persona review of the contract surface redesign (031887e + 10b87a6). P0-1 (auto-fixed): scripts/run_platform.sh:437 read the uptime_enabled feature flag from the OLD top-level contract.inputs.uptime_enabled path, which P57 removed. With the new contract shape c.get('inputs',{}) returns {} so the flag silently always defaulted to True — a consumer setting uptime_enabled:false under infrastructure..inputs could NOT disable uptime monitoring. Fixed to scan infrastructure..inputs.uptime_enabled (any module false wins). P0-2 (auto-fixed): docs/consumer-guide.md:417,472 documented the ${contract.module} interpolation token, but P57 dropped the `module` field. _expand_vars fails loud (D-081) on unknown tokens, so a consumer following the documented bucket_name example (acdl-${env.environment}-${contract.module}-...) hit a hard ValueError at resolve time. Replaced with ${contract.id} (the surviving short acronym field) in both the example and the interpolation reference table. P0-3 (auto-fixed): core/regression_verify.py CAP-006 and tests/test_consumer_guide_per_env_section.py both asserted the dropped ${contract.module} token. Updated CAP-006 to use ${contract.id} and the doc test to assert ${contract.id} present / ${contract.module} absent. P1+ flags (post-hoc): - P1: _namespace_resources does not rewrite ref: targets in stack.outputs[].from for cross-module refs (within-module is handled; multi-module refs across fragments are not wired today, but no contract uses them yet). - P1: _latest_version raises ValueError (not a clear message) on a malformed semver string in the registry; the schema pins version to ^\d+\.\d+\.\d+$ so this is unreachable from a contract, but registry authors have no guardrail. - P2: docs/consumer-guide.md:407 example path uses .yaml extension while the repo-wide rename standardized on .yml (consumer-repo paths, not platform, so non-blocking). ---ci--- project: acdl phase: 57 milestone: v1.10.2 status: verify lessons: - P0 fix applied: uptime_enabled read path migrated to infrastructure..inputs (was stale top-level contract.inputs) - P0 fix applied: docs + tests migrated off dropped ${contract.module} interpolation token to ${contract.id} ---/ci--- --- core/regression_verify.py | 10 +++++++--- docs/consumer-guide.md | 8 ++++---- scripts/run_platform.sh | 20 ++++++++++++++++++-- tests/test_consumer_guide_per_env_section.py | 5 ++++- 4 files changed, 33 insertions(+), 10 deletions(-) diff --git a/core/regression_verify.py b/core/regression_verify.py index 941b3e5..c85d3f8 100755 --- a/core/regression_verify.py +++ b/core/regression_verify.py @@ -192,13 +192,17 @@ def _check_adapter_emits_terraform() -> Tuple[Status, str]: def _check_interpolation() -> Tuple[Status, str]: - """CAP-006: contract interpolation expands ${env.*} / ${contract.*}.""" + """CAP-006: contract interpolation expands ${env.*} / ${contract.*}. + + P57: the contract's `module` field was dropped in favor of `id` + (short acronym) + `infrastructure` map; the interpolation check uses + `contract.id` (the surviving field).""" return _check_subprocess([ "python3", "-c", "import sys; sys.path.insert(0,'.'); " "from core.contract_resolver import _expand_vars; " - "ctx={'env':{'environment':'qa','account_id':'123'},'contract':{'module':'ms'}}; " - "assert _expand_vars('acdl-${env.environment}-${contract.module}', ctx)=='acdl-qa-ms'; " + "ctx={'env':{'environment':'qa','account_id':'123'},'contract':{'id':'assets'}}; " + "assert _expand_vars('acdl-${env.environment}-${contract.id}', ctx)=='acdl-qa-assets'; " "print('interpolation ok')", ]) diff --git a/docs/consumer-guide.md b/docs/consumer-guide.md index 427b1f4..0110d14 100644 --- a/docs/consumer-guide.md +++ b/docs/consumer-guide.md @@ -404,8 +404,8 @@ input). Promotion = running the matching job. ### Two shapes (both supported) **Shape 1 — per-environment contract files:** a consumer repo has one -contract per environment (e.g. `.acdl/static-assets.dev.yaml`, -`.acdl/static-assets.qa.yaml`, …). Each sets `environment:` to its own +contract per environment (e.g. `.acdl/static-assets.dev.yml`, +`.acdl/static-assets.qa.yml`, …). Each sets `environment:` to its own name and uses interpolation so env-specific values differ automatically: ```yaml @@ -414,7 +414,7 @@ id: assets infrastructure: static-assets: inputs: - bucket_name: acdl-${env.environment}-${contract.module}-${env.account_id}-${env.region} + bucket_name: acdl-${env.environment}-${contract.id}-${env.account_id}-${env.region} region: ${env.region} version: 1.0.0 name: static-assets @@ -469,7 +469,7 @@ duties check blocks a prod promotion when `approver_qa == approver_prod` | `${env.account_id}` | the environment's AWS account id | `123456789012` | | `${env.state_backend.bucket}` | the environment's state bucket | `acdl-qa-state` | | `${env.network.vpc_cidr}` | the environment's VPC CIDR | `10.1.0.0/16` | -| `${contract.module}` | the contract's module name | `static-assets` | +| `${contract.id}` | the contract's operational acronym | `assets` | | `${contract.environment}` | the contract's environment field | `qa` | | `${contract.inputs.}` | a contract input value | (as declared) | diff --git a/scripts/run_platform.sh b/scripts/run_platform.sh index b2b876b..48e16b0 100755 --- a/scripts/run_platform.sh +++ b/scripts/run_platform.sh @@ -431,10 +431,26 @@ echo "" echo "=== Step 9b: deploy uptime monitoring (separate state) ===" # The uptime stack is deployed by default after the L2 module. It uses a # separate terraform state ($WORK/uptime-tf). Endpoints from the L2 outputs -# are passed as monitored_endpoints. The feature flag (inputs.uptime_enabled, +# are passed as monitored_endpoints. The feature flag (uptime_enabled, # default true) controls whether this step runs. +# +# P57 contract shape: uptime_enabled is a per-module input under +# infrastructure..inputs.uptime_enabled (the old top-level +# contract.inputs.uptime_enabled was removed). Scan every module's inputs; +# any module setting uptime_enabled=false disables the uptime step (one +# contract = one logical stack, so a single false wins). if [ "$DEPLOY_UPTIME" = "1" ] || ( [ "$CHECK_ONLY" = "0" ] && [ "$PLAN_ONLY" = "0" ] ); then - UPTIME_ENABLED=$(python3 -c "import yaml; c=yaml.safe_load(open('$CONTRACT')); print(c.get('inputs',{}).get('uptime_enabled', True))" 2>/dev/null || echo "True") + UPTIME_ENABLED=$(python3 -c " +import yaml +c = yaml.safe_load(open('$CONTRACT')) +infra = c.get('infrastructure', {}) +# Default true; a module may override to false. +for m, entry in infra.items(): + if isinstance(entry, dict) and entry.get('inputs', {}).get('uptime_enabled') is False: + print('False'); break +else: + print('True') +" 2>/dev/null || echo "True") if [ "$UPTIME_ENABLED" = "True" ] || [ "$UPTIME_ENABLED" = "true" ]; then echo "uptime: feature flag enabled — constructing uptime contract" UPTIME_DIR="$WORK/uptime-tf" diff --git a/tests/test_consumer_guide_per_env_section.py b/tests/test_consumer_guide_per_env_section.py index 76b0e81..7300895 100644 --- a/tests/test_consumer_guide_per_env_section.py +++ b/tests/test_consumer_guide_per_env_section.py @@ -37,7 +37,10 @@ def test_consumer_guide_has_interpolation_reference(): text = GUIDE.read_text() assert "${env.environment}" in text assert "${env.account_id}" in text - assert "${contract.module}" in text + # P57: contract.module was dropped in favor of contract.id (short acronym) + assert "${contract.id}" in text + # The old token must not survive the P57 contract redesign + assert "${contract.module}" not in text def test_consumer_guide_states_no_field_editing():