shoc-backend/QUALITY_GATES.md
Adam Moussa 8f4fa36647
refactor(cd): ship Elastic Beanstalk versions from GitHub on main
Keep application and Terraform changes in separate PRs so a merge cannot race an HCP apply against an app deploy.
2026-09-17 15:54:31 -04:00

8.4 KiB
Raw Blame History

Quality Gates (canonical)

Status: canonical. This document owns how the architecture and code-quality rules are enforced: the gate list, the local command, the CI mapping, and the pass/fail/skip reporting semantics. The meaning of each rule lives in ARCHITECTURE_AND_CODE_QUALITY.md.

CI and local run the same check. Every gate below is executed by scripts/governance-check.sh. The architecture-quality workflow calls that script so a green run means the same thing locally and in CI. Do not add a check to CI that is not also runnable locally through the script (and vice versa).

Gate inventory

# Gate Protects (architecture §) Local command CI step
G1 Restore build integrity dotnet restore SeaHavenIndustries.sln architecture-quality → governance-check.sh
G2 Architecture boundary tests §1, §3 (dependency direction) dotnet test Api.SeaHavenIndustries.Tests ... --filter FullyQualifiedName~ArchitectureTests --no-restore same script
G3 Changed-file formatting §1 (conventions) dotnet format SeaHavenIndustries.sln --no-restore --verify-no-changes --include <changed .cs> same script
G4 Release build compile correctness dotnet build SeaHavenIndustries.sln -c Release --no-restore architecture-quality and ci
G5 Full test suite behavior dotnet test SeaHavenIndustries.sln -c Release --no-build architecture-quality and ci
G6 Migration gates §7 see §"Migration gates" below on-demand / release
G7 Cancellation forwarding §6 behavior tests on changed I/O paths + analyzer review-enforced on changed paths
G8 Error disclosure §5 SanitizedErrorsTests (part of G5) ci
G9 Board-backed regression review framework REVIEW_AND_PR_FRAMEWORK.md inventory review-enforced
G10 Terraform import plan safety live infrastructure adoption python scripts/test-terraform-import-plan-check.py architecture-quality → governance-check.sh
G11 Terraform static validation import configuration integrity commands below ci-terraform on PRs to main or dev
G13 App/Terraform isolation separate delivery lanes python3 scripts/check_app_terraform_isolation.py architecture-quality → governance-check.sh

How to run locally

# Complete local repository gate (mirrors the architecture-quality workflow):
bash scripts/governance-check.sh

# By default it compares against the default branch for changed-file formatting.
# Override the comparison point:
BASE_REF=origin/main bash scripts/governance-check.sh
BASE_REF=main         bash scripts/governance-check.sh

The script:

  1. dotnet restore (G1)
  2. runs the ArchitectureTests filter with --no-restore (G2)
  3. computes changed .cs files vs BASE_REF (default origin/main) and runs dotnet format --verify-no-changes --include ... (G3). When there are no changed C# files it skips G3 with an explicit "skipped: no changed C#" line.
  4. builds the complete solution in Release with no restore (G4).
  5. runs the complete solution test suite in Release with no rebuild (G5).
  6. verifies that the Terraform plan guard rejects create, delete, replacement, unmanaged resource types, and updates not allowlisted by exact address (G10).
  7. runs the release-tag, commit-check, and isolation unit tests.
  8. rejects a diff that contains both terraform/ and deployable application files (G13).

G10 permits only exact approved resource address/type pairs for the environment-owned boundary: Elastic Beanstalk environment, IAM role/inline policy/managed-policy attachment/ instance profile, Secrets Manager secret metadata, and Route 53 record. SSM /shoc-backend/<env>/deploy/* parameters are created after adoption and are not part of the import allowlist. Initial mode permits no update. Controlled mode requires one --allow-update-address argument per reviewed in-place update. Every invocation also requires --environment dev or --environment staging; an empty or incomplete environment plan fails.

G11 runs terraform fmt -check -recursive terraform, terraform init -backend=false, and terraform validate for both live/dev and live/staging on every PR to main or dev. Org-baseline CloudFormation owns the HCP role substrate, and Terraform owns the environment GitHub deploy roles, so no backend CDK or bootstrap root remains in the matrix. HCP plan/apply roles stay in org-baseline; this repository never manages hcptf-* roles.

The former G12 version-only HCP apply guard is not part of the repository gate. scripts/check-terraform-release-plan.py remains only while .github/workflows/deploy.yml is still present.

G13 fails when the same diff contains both terraform/ and deployable application files. Workflow, documentation, and gate-script changes may share a PR with either side.

Migration gates (G6)

Provider-real, not reflection-derived. Run against the configured provider:

  • Actual-provider discovery — confirm the runtime provider matches the configured connection (e.g., SQL Server / Npgsql) from configuration, not from package guesses.
  • Snapshot consistency — dotnet ef migrations script --idempotent (or a model diff) shows no phantom operations; ModelSnapshot matches the model.
  • Empty-db apply — dotnet ef database update against a fresh empty database applies the full history cleanly.
  • Supported-prior upgrade — upgrade from each still-supported prior migration applies cleanly.
  • Rollback — report not guaranteed unless a real downgrade was exercised against the provider. Never claim "EF can roll it back" untested.

Migration gates are run on-demand and gated for releases; they are not in the per-PR fast path because they require a database. A PR that adds/changes a migration must document the G6 evidence in the PR description.

Cancellation forwarding (G7)

For changed async I/O paths, the requirement (§6) is verified forwarding, not a signature checkbox. Acceptable evidence, any one of:

  • a behavior test asserting the CancellationToken reaches the data service / EF call; or
  • an analyzer diagnostic confirming propagation on the changed method.

A parameter that exists but is swallowed/default-ed is a fail. If an analyzer is unavailable for a path, a behavior test is required; silence is not acceptable. Review enforces this on changed paths.

Architecture discovery must fail on zero (G2 invariants)

Each reflection invariant in ArchitectureTests asserts the discovered set is non-empty. A future namespace move that makes reflection return zero types will fail the gate rather than silently pass. The current invariants:

  1. Controllers inject neither DbContext, concrete data services, nor concrete service implementations.
  2. Business services inject neither DbContext nor IConfiguration.
  3. Business services depend on interfaces (not concrete service/data-service implementations) for their data and persistence dependencies.

What this does not claim

  • Reflection over constructors proves wiring shape, not runtime behavior. Runtime/semantic properties (cancellation actually cancelling, a query executing in SQL, a transaction actually committing atomically) are proven by behavior tests or provider evidence, never by reflection.
  • A passing script proves G1–G5 ran in sequence. It does not prove the provider-backed or review-owned gates G6–G9.

Pass / fail / skip / not-run reporting

Every gate result is reported as exactly one of:

  • pass — ran and succeeded, with evidence.
  • fail — ran and failed; cite the failing test/command and output.
  • skipped — intentionally not run for this change (e.g., no changed C# for G3); name the gate and the reason.
  • not-run — could not run (environment/dependency missing); name the blocker.

A PR is not "green" if any applicable gate is fail, skipped-without-justification, or not-run. Never infer a pass from silence.

Adding or changing a gate

  • Any new executable check goes into scripts/governance-check.sh and the architecture-quality workflow together (or into the full CI for G4/G5-type checks). One source of truth for local and CI.
  • A new rule documents its meaning in ARCHITECTURE_AND_CODE_QUALITY.md and its command/mapping here.
  • Explicit, documented exceptions only — no wildcard suppressions (see ARCHITECTURE_AND_CODE_QUALITY.md §10).