shoc-backend/QUALITY_GATES.md

8.9 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 locally. The architecture job in ci.yml calls that script (skipping G4/G5, which the Build and test job owns). Do not add a check to CI that is not also runnable locally through the script (and vice versa), except G4/G5 which the local script still runs.

Gate inventory

# Gate Protects (architecture §) Local command CI step
G1 Restore build integrity dotnet restore SeaHavenIndustries.sln ci.yml architecture → 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 ci.yml Build and test (local script still runs it)
G5 Full test suite behavior dotnet test SeaHavenIndustries.sln -c Release --no-build ci.yml Build and test (local script still runs it)
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.yml Build and test
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 ci.yml architecture → governance-check.sh
G11 Terraform static validation import configuration integrity commands below ci.yml architecture → governance-check.sh
G13 App/Terraform isolation separate delivery lanes python3 scripts/check_app_terraform_isolation.py ci.yml architecture (live classifier: PR + local)

How to run locally

# Complete local repository gate (G1–G5, G10, G11, G13):
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. runs Terraform fmt/validate for live/dev and live/staging (G11).
  9. rejects a diff that contains both terraform/ and deployable application files (G13). Live G13 skips on merge_group and push. The architecture job in CI sets GOVERNANCE_SKIP_BUILD_TEST=1 so steps 4–5 run only locally and in the Build and test job.

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 -input=false -lockfile=readonly, and terraform validate for both live/dev and live/staging from governance-check.sh on every CI event, including merge groups. 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.

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 local script proves G1–G5, G10, G11, and G13 ran (G4/G5 are skipped in the CI architecture job). 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 job in ci.yml together (or into the Build and test job for G4/G5-type checks). One source of truth for local and CI. The required merge-queue check is ci-complete.
  • 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).