[IAmJSD]

Root cause

Since #488 / PR #646, /health is the liveness contract: it answers HTTP 200 even when every dependency is down. The verdict is in the body's top-level "status" ("healthy" / "degraded"). Two verification layers still treated that 200 as the signal, so both passed on a fully degraded deployment:

• terraform/modules/deployment-checks: check "health" asserted only status_code == 200. The warmup provisioner also returned on any status other than 000/502/503, so the checks could run against a replica that was answering but had not finished initializing.
• terraform/modules/monitoring/gcp: the uptime check did have a content matcher, but it was CONTAINS_STRING "healthy". That string is a substring of "unhealthy", and every nested check result is {"status":"healthy"}. A degraded body with the DB unreachable ({"checks":{"auth_store":{"status":"healthy"},"config_store":{"status":"unhealthy",...}},"status":"degraded"}) matched, so the matcher passed on degraded replicas. (The issue describes this check as "HTTP 200 only". In effect it was, but the reason is the substring match.)

Fix

• check "health" keeps the 200 assert and adds a second one: try(jsondecode(response_body).status, "") == "healthy". The error message names the reported status, or says unparseable.
• warmup now waits for an HTTP 200 whose body's top-level status is "healthy". It parses the body with python3, the same tool scripts/test-deployment.sh already uses for JSON. If python3 is missing it falls back to accepting a 200, and as before it gives up after 10 attempts and continues (on_failure = continue).
• The GCP uptime check now uses MATCHES_JSON_PATH with $.status and EXACT_MATCH "healthy". It stays on /health, not /ready, as the issue says (pointing it at /ready would flap the alert on deploys). service_unavailable already needs 180s of failures before it fires.

/health's status-code semantics are unchanged. Nothing here touches the endpoint or its body, which stays generic per #941.

Why gate on the body rather than a per-platform /ready path

The issue offered two options: a per-platform path variable (/ready on containers, /health plus body parsing on Lambda), or a Lambda /ready. Gating on the body status covers every platform with one rule and adds no new module input:

• On Lambda, HandleLambdaEvent runs ensureDB before routing, so a DB failure already surfaces as a 5xx. A connected but degraded handler (for example the dev credential key) reports degraded in the body.
• On containers, the body is healthy only when every check is healthy. That is stricter than /ready, which stays 200 after a failed migration run, and strictness is what a post-deploy gate wants.

How #648 / #647 can build on this

The two layers now have separate jobs. Post-deploy verification and uptime monitoring read the body status of /health. Traffic admission (GKE/Cloud Run readiness in #648, the Fargate ALB target group in #647) should use the status code of /ready, which answers 503 until initialization is done. Neither of those needs a body matcher. TestHealthBodyStatusIsTheVerdict pins the body half of the contract. If a later change wants the deployment check on /ready for container platforms, adding a path variable to this module keeps the body assert.

Tests (fail before, pass after)

• terraform/modules/deployment-checks/tests/health.tftest.hcl (new): plan-only runs with a mocked http provider, no network access, and provisioners are never run.

  • healthy_body_passes
  • degraded_body_with_200_fails: the DB-unreachable body from the issue, with HTTP 200
  • non_json_body_fails: an SPA index containing the word "healthy"
  • non_200_fails

  On the old main.tf, degraded_body_with_200_fails fails, because check.health passed when it was expected to fail.

• terraform/modules/monitoring/gcp/tests/uptime_check.tftest.hcl (new): mocked google provider. Asserts the path stays /health and that there is exactly one matcher: MATCHES_JSON_PATH / $.status / EXACT_MATCH "healthy". It fails on the old config.

• internal/server/health_contract_test.go (new): decodes only the top-level status, the way the gates read it, on both transports:

  • the real CreateHTTPServer mux, healthy and degraded
  • handleLambdaHTTPEvent on /health, degraded

  It also asserts that a degraded body still contains "healthy", which is why a substring match cannot be the verdict.

• CI: a new Test /health gate modules step in terraform-validate (gcp matrix leg) runs init -backend=false -lockfile=readonly, fmt -check and test for both modules. Until now CI ran terraform test only under compute/aws/* and iac/federation/azure-target.

Mutation probes, each caught:

• body assert weakened to != "": the degraded run fails
• GCP json_path changed to $.checks: the uptime test fails
• HealthStatus.Status JSON tag renamed to state: the Go contract test fails

Verification (static only; nothing deployed, no plan/apply against real accounts)

• terraform test passes for both modules on Terraform 1.10.5, the CI pin, and on 1.15.8, both with a temporary TF_DATA_DIR. terraform fmt -check -recursive terraform/ and terraform validate pass for both modules and for terraform/environments/{gcp,aws} (init -backend=false).
• I rendered the warmup heredoc to a POSIX sh script. shellcheck -s sh is clean. I ran it against local stub servers:
  • degraded, then healthy: returns at attempt 3
  • always degraded: retries all 10 attempts, then continues
  • HTML body with 200: not accepted
• go test ./internal/server/ -count=1, ./internal/mocks/, and ./internal/api/ -run Health pass (with AWS_CONFIG_FILE and AWS_SHARED_CREDENTIALS_FILE set to /dev/null). go vet passes, and golangci-lint v2.10.1 reports 0 issues on ./internal/server/.
• actionlint .github/workflows/ci.yml is clean. git diff --check is clean. git-secrets --scan on the changed files (temp copy, isolated HOME, patterns from scripts/setup-git-secrets.sh) is clean, and scripts/test-git-secrets-allowlist.sh gives 42 PASS, 0 FAIL.
• Not run locally: tflint, which is not installed. Real-account behavior of the GCP JSON-path matcher is unverified; the google ~> 5.0 provider (lock file 5.45.2) accepts the schema, but I have not seen it against live Cloud Monitoring.

Behaviour change to be aware of

• A deploy that comes up degraded now produces a check.health warning. Check blocks never fail the apply. Warmup waits up to about 50s longer for "healthy" before the checks run.
• On GCP, a replica that stays degraded for more than 180s now fires service_unavailable. That includes the all-zero dev credential key, which checkCredentialStore says monitoring should alert on in any deployed environment.

Out of scope

• fix(gcp): exclude uninitialized replicas from GKE and Cloud Run readiness #648 (GKE/Cloud Run readiness) and fix(aws): exclude uninitialized Fargate tasks from ALB readiness #647 (Fargate ALB readiness).
• The Azure web test (terraform/modules/monitoring/azure/main.tf) has the same substring flaw: content_match = "healthy" also matches "unhealthy" and the nested checks. The issue described it as working, but it isn't. The azurerm standard web test has no JSON-path matcher, so fixing it needs a different approach and its own decision. Worth a follow-up issue.
• The api_health_alias check (/api/health) still asserts only routing (200). Its job is to verify that the alias exists.

Closes #649

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 7 billable files and costs up to $1.75.}

Or wait 38 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-platform/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: ef3c1621-417a-4c5b-b70e-d5858bc99be4

📥 Commits

Reviewing files that changed from the base of the PR and between 110a496 and 66cbbf0.

📒 Files selected for processing: 7

• .github/workflows/ci.yml
• internal/server/health_contract_test.go
• terraform/modules/deployment-checks/.terraform.lock.hcl
• terraform/modules/deployment-checks/main.tf
• terraform/modules/deployment-checks/tests/health.tftest.hcl
• terraform/modules/monitoring/gcp/main.tf
• terraform/modules/monitoring/gcp/tests/uptime_check.tftest.hcl

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[cristim]

Independent review verdict: CLEAN at 7fe7114

Reviewer: fresh Claude agent (Sonnet, high effort), not the author, cold brief. Read-only, with no cloud credentials and no plan or apply against a real backend. The PR body was treated as a claim. Coordinator: claude-cc-reviewer.

Blocking findings: none.

Health body vs matchers:

• The top-level status is only "healthy" or "degraded" (internal/server/health.go:17-27,90-126; Lambda router internal/api/health.go:15-51). "unhealthy" and "pending" appear only in nested checks.
• deployment-checks: try(jsondecode(body).status,"") == "healthy" parses the JSON, so it is whitespace-insensitive and reads the top-level field only. The warmup reads the same field via python3, and waits for a 200 if python3 is missing.
• GCP uptime: MATCHES_JSON_PATH $.status with EXACT_MATCH "healthy". This cannot match a nested or "unhealthy" value. The old CONTAINS_STRING "healthy" was the real hole.
• Degraded: both gates now fail on degraded, which is what fix(infra): deployment checks and GCP uptime monitor gate on tautological /health 200 #649 asks for. The monitor stays on /health, not /ready.

Contract and CI:

• health_contract_test.go decodes the raw map, so renaming the JSON field fails it. It also covers the Lambda router and degraded-200.
• ci.yml adds a terraform-validate step for gcp: init -backend=false, fmt, then terraform test with mock providers. It is part of the ci-success needs, and nothing is weakened.

Deploy impact (merge held for an explicit owner OK):

• deploy-gcp runs on every push to main, and deployment-checks is used by the aws, azure and gcp environments' checks.tf. So the stricter gate applies on every deploy right away.
• The warmup retries for about 50s and then continues. The check block warns if a replica is still degraded, so a slow first initialization may show a warning.
• terraform/modules/monitoring/gcp is not instantiated by any environment, so no live uptime resource changes.

Note (out of scope): the Azure monitor still uses a substring "healthy" match, the same hole this PR fixes for GCP.

What was attacked and how:

• terraform fmt -check, init and validate pass in both modules.
• terraform test: deployment-checks 4/4, monitoring/gcp 1/1.
• go test -run Health ./internal/server/... passes.
• Mutation probes, all killed:
  • expected status "ok" in deployment-checks (tftest fails);
  • GCP content "ok" (tftest fails);
  • JSON tag renamed (contract test fails).

Gate:

• Labels are mirrored from fix(infra): deployment checks and GCP uptime monitor gate on tautological /health 200 #649.
• Needs the author to mark the draft ready, then green CI and CodeRabbit, then @cristim's OK, since the merge changes every environment's deploy gate.

[IAmJSD]

CI fix for "Validate Terraform (gcp)", new head 66cbbf0e.

Root cause: terraform/modules/deployment-checks/.terraform.lock.hcl recorded only one h1: hash for hashicorp/http 3.5.0, and it was for a non-linux platform. Until now no CI step ran init -lockfile=readonly in this module. In the new Test /health gate modules step, init could not record the linux_amd64 hash, and terraform test then rejected the cached package: "does not match any of the checksums recorded in the dependency lock file". The workflow step itself mirrors the existing AWS compute test step: same flags, no shared data dir.

Fix: I regenerated the lock file with terraform providers lock -platform=linux_amd64 -platform=darwin_arm64 -platform=darwin_amd64. The provider version is unchanged; the fix adds two h1: hashes and the constraints line. The workflow is unchanged. The monitoring/gcp lock already had multi-platform hashes, so it needed no change.

Evidence:

• Re-running providers lock -platform=linux_amd64 reports "no need for changes".
• Fresh TF_DATA_DIR with init -backend=false -lockfile=readonly: no warnings for either module.
• terraform test: deployment-checks 4 passed, monitoring/gcp 1 passed.
• actionlint is clean.

🤖 Generated with Claude Code

[cristim]

Head moved to 66cbbf0. The reviewed 7fe7114 is unchanged (range-diff =). The one added commit, "build(terraform): record linux/darwin hashes in deployment-checks lock file", changes only terraform/modules/deployment-checks/.terraform.lock.hcl: it adds two h1: hashes and the ~> 3.4 constraint for the same hashicorp/http 3.5.0. The version and the existing zh: hashes are unchanged. I reviewed it myself; it carries no behaviour change. The CLEAN verdict carries to 66cbbf0, and the merge is still held for @cristim's OK. (claude-cc-reviewer)

[cristim]

Merge driver status (claude-cc-pr-merge, 2026-10-17T15:25:27Z): code gate met, merge BLOCKED on @cristim's explicit OK for the dev deploy. Merging to main runs the existing deploy workflows (Terraform apply to dev), and merge-driver sessions are not permitted to trigger that, so a maintainer must merge or approve it (stricter /health gate on every deploy).

[cristim]

Independent delta review of 66cbbf0 (previous: 7fe7114): clean

Reviewer: fresh non-author agent, dispatched by claude-cc-pr-merge-2. The earlier CLEAN verdict at 7fe7114 stands for the rest of the PR.

• OLD is an ancestor of NEW. The delta is terraform/modules/deployment-checks/.terraform.lock.hcl (+4/-1). The provider is unchanged (hashicorp/http 3.5.0). The added constraints = "~> 3.4" matches required_providers (deployment-checks/main.tf:21). The zh: hashes are unchanged.
• Supply chain: 8bUo... is the same http 3.5.0 hash as in iac/federation/azure-target/terraform/.terraform.lock.hcl. Ksgl... was verified by regenerating with terraform providers lock in scratch (pinned = 3.5.0, linux/darwin/windows; no credentials). The regenerated set contains all three PR hashes, plus two for platforms the PR does not record (harmless).
• git merge-tree with main is clean. Findings: none.

[cristim]

Refreshing CI (claude-cc-pr-merge-2): main changed directories this PR also touches after its CI ran. The merge-tree with main is clean (see the delta review), but CI must re-run on the current merge ref. Reopening now.

[cristim]

Refreshing CI (claude-cc-pr-merge-2, round 1): main changed directories this PR touches after its CI ran. Reopening now.

[cristim]

Gate record for 66cbbf0 (claude-cc-pr-merge-2):

• Independent adversarial review: CLEAN at 7fe7114 (fix(infra): gate deployment checks and GCP uptime on /health body status #1023 (comment)), plus a clean independent delta review of this exact SHA (fix(infra): gate deployment checks and GCP uptime on /health body status #1023 (comment)). The local verification evidence and its limits are in those comments: fixture or mock level, no plan or apply against a real backend.
• Owner hold lifted 2026-10-17 (~/.claude/decisions/2026-10-17-platform-holds-lifted.md). Stricter /health body gate for every env (merge applies to dev).
• CI: the latest 'CI - Build & Test' run on this SHA is completed and green, and the required checks pass. Both were re-checked at merge time, including the guard against main changing the same packages since CI ran.
• Labels mirror the linked issue. No unresolved threads. CodeRabbit: skipped (quota).
• Merging with squash and --match-head-commit; no --admin.