ci: add machine-readable QA contract + drift validator#3
Conversation
There was a problem hiding this comment.
Pull request overview
Introduces a machine-readable “QA contract” for GitHub Actions CI, plus a validator and a dedicated workflow to detect drift between the contract and required workflows.
Changes:
- Add
.github/qa_contract.yamldefining required workflows/jobs/steps/matrix and an action pinning policy. - Add
scripts/ci/validate_qa_contract.pyto validate workflows against the contract. - Add
.github/workflows/qa-contract-drift.ymlandQA_CONTRACT.mdto enforce and document the contract.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
scripts/ci/validate_qa_contract.py |
Implements YAML loading and validation of workflow invariants and uses: policy. |
QA_CONTRACT.md |
Operator-facing documentation pointing to the contract and validator workflow. |
.github/workflows/qa-contract-drift.yml |
CI job that installs PyYAML and runs the validator on PRs and main pushes. |
.github/qa_contract.yaml |
Contract definition for required workflows/jobs/steps/matrix and required action references. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def _load_yaml(path: Path) -> dict[str, Any]: | ||
| data = yaml.safe_load(path.read_text(encoding="utf-8")) | ||
| return data if isinstance(data, dict) else {} |
There was a problem hiding this comment.
_load_yaml() will raise a stack trace on invalid YAML (PyYAML exceptions) and also silently treats any non-mapping YAML root as {}. For a drift validator, it’s more useful to catch yaml.YAMLError (and similar) and return a clear validation error, and to explicitly fail when the YAML root is not a mapping so contract/workflow corruption is surfaced clearly.
| invariants = contract.get("invariants", {}) | ||
| policy = contract.get("policy", {}) | ||
|
|
There was a problem hiding this comment.
invariants = contract.get("invariants", {}) and policy = contract.get("policy", {}) assume those fields are mappings. If someone edits the contract and accidentally makes either a list/string, the validator will crash with AttributeError later (e.g., invariants.get(...)). Consider type-checking/coercing these to dicts (or emitting a specific error) to keep failures machine-readable.
| invariants = contract.get("invariants", {}) | |
| policy = contract.get("policy", {}) | |
| raw_invariants = contract.get("invariants") | |
| if raw_invariants is None: | |
| invariants: dict[str, Any] = {} | |
| elif isinstance(raw_invariants, dict): | |
| invariants = raw_invariants | |
| else: | |
| errors.append("contract field 'invariants' must be a mapping") | |
| invariants = {} | |
| raw_policy = contract.get("policy") | |
| if raw_policy is None: | |
| policy: dict[str, Any] = {} | |
| elif isinstance(raw_policy, dict): | |
| policy = raw_policy | |
| else: | |
| errors.append("contract field 'policy' must be a mapping") | |
| policy = {} |
| all_uses: set[str] = set() | ||
| for wf_contract in invariants.get("required_workflows", []): |
There was a problem hiding this comment.
for wf_contract in invariants.get("required_workflows", []) assumes required_workflows is a list of dicts. If it’s accidentally a dict (or contains non-dicts), the validator will crash when calling .get(...) on a non-mapping. Recommend validating the type of required_workflows up front (and each entry) and reporting a contract schema error instead of raising.
| all_uses: set[str] = set() | |
| for wf_contract in invariants.get("required_workflows", []): | |
| if not isinstance(invariants, dict): | |
| errors.append("contract 'invariants' section must be a mapping") | |
| invariants = {} | |
| all_uses: set[str] = set() | |
| required_workflows = invariants.get("required_workflows", []) | |
| if not isinstance(required_workflows, list): | |
| errors.append("contract invariants.required_workflows must be a list") | |
| required_workflows = [] | |
| for wf_contract in required_workflows: | |
| if not isinstance(wf_contract, dict): | |
| errors.append( | |
| "contract has non-mapping entry in invariants.required_workflows" | |
| ) | |
| continue |
| pull_request: | ||
| push: | ||
| branches: [main] | ||
|
|
There was a problem hiding this comment.
This workflow doesn’t declare explicit permissions, so it will run with the repository’s default GITHUB_TOKEN permissions (which may be broader than necessary). Since this job only needs to read the repo contents to validate YAML, consider adding least-privilege permissions (e.g., contents: read) at the workflow or job level.
| permissions: | |
| contents: read |
Summary
Why
This establishes a single, auditable CI contract surface and prevents accidental workflow drift.
Validation