8.9 KiB
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.shlocally. Thearchitecturejob inci.ymlcalls that script (skipping G4/G5, which theBuild and testjob 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:
dotnet restore(G1)- runs the
ArchitectureTestsfilter with--no-restore(G2) - computes changed
.csfiles vsBASE_REF(defaultorigin/main) and runsdotnet format --verify-no-changes --include ...(G3). When there are no changed C# files it skips G3 with an explicit "skipped: no changed C#" line. - builds the complete solution in Release with no restore (G4).
- runs the complete solution test suite in Release with no rebuild (G5).
- verifies that the Terraform plan guard rejects create, delete, replacement, unmanaged resource types, and updates not allowlisted by exact address (G10).
- runs the release-tag, commit-check, and isolation unit tests.
- runs Terraform fmt/validate for
live/devandlive/staging(G11). - rejects a diff that contains both
terraform/and deployable application files (G13). Live G13 skips onmerge_groupandpush. The architecture job in CI setsGOVERNANCE_SKIP_BUILD_TEST=1so 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;ModelSnapshotmatches the model. - Empty-db apply —
dotnet ef database updateagainst 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
CancellationTokenreaches 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:
- Controllers inject neither
DbContext, concrete data services, nor concrete service implementations. - Business services inject neither
DbContextnorIConfiguration. - 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.shand thearchitecturejob inci.ymltogether (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 isci-complete. - A new rule documents its meaning in
ARCHITECTURE_AND_CODE_QUALITY.mdand its command/mapping here. - Explicit, documented exceptions only — no wildcard suppressions (see
ARCHITECTURE_AND_CODE_QUALITY.md§10).