ci: run classifier tests in Required Merge Gate - #160
Conversation
The merge-profile classifier decides which validation profile routes a change, but its unit suite was not executed by any workflow. Core Checks cannot protect the lightweight Required Merge Gate: if the classifier is broken in a way that still classifies successfully, the gate routes to a lightweight profile and passes while Core Checks fails elsewhere. Run test_classify_merge_profile.py in the gate's own classify-changes job, before the step that produces the routing outputs, so a failing suite fails classification and the gate shell rejects the run. Add ci/checks/test_classifier_ci_contract.py, which parses the workflow with an `on`-preserving loader and executes the extracted step and gate shell: - test_workflow_executes_all_classifier_tests runs the actual step script and observes every loader-discovered method (24) reported ok, with the step unconditional, ordered before routing outputs and free of continue-on-error. - test_suite_failure_reaches_required_gate injects a failure into one test in a temporary copy of the suite, shows the extracted step exits nonzero while still executing every method, then drives the extracted gate shell with modeled dependency results: failed, skipped and cancelled classification exit nonzero before any check polling, while a successful classification keeps github-metadata and rpc-docs profiles usable. The gh check-run API is simulated for polling only.
An adversarial mutation review found four ways the Required Merge Gate workflow could be changed so a failing classifier suite no longer fails the required check, none of which the contract test detected: * CLASSIFY_CHANGES_RESULT bound to a literal instead of needs.classify-changes.result * VALIDATION_PROFILE bound to a literal instead of needs.classify-changes.outputs.profile * job-level continue-on-error on classify-changes, which reports the job as a success when its steps fail * the gate job's `if` conditioned on needs.classify-changes.result, which skips the gate on classifier failure and reports a skipped job as a passing required check The root cause was that the test only checked which env keys the gate step declared and then supplied the modeled values itself. It now evaluates each modeled env binding against a fake `needs` context and requires the binding to be exactly needs.<job>.result or needs.<job>.outputs.<name> of a modeled dependency, and a new focused test asserts that classify-changes has neither `if` nor `continue-on-error` and that the gate's `if` uses always() without referencing any needs.* result. Also mark the file executable to match its shebang and the sibling ci/checks tests, as lint-files.py requires.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f5dfc27c37
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Install the distribution's PyYAML package and run the workflow contract with its system Python in the unconditional classification job. Contract failures now stop routing outputs and reach the Required Merge Gate. Verify the actual command in Ubuntu 24.04 with missing, filtered, and reordered classifier-test steps and an injected contract-test failure.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7c716a18f6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5207610c39
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
1 participant
Summary
Run all 24 merge-profile classifier tests and the 5 workflow contract tests in the Required Merge Gate's own
classify-changesjob, before it produces routing outputs. A failing suite now fails the job and required gate instead of allowing a regression to route itself to insufficient validation. Addresses #157.Add an executable workflow contract that runs the actual step and gate shell, verifies every classifier test executes, and checks failure propagation and successful lightweight routing. Install
python3-yamland run the contract with/usr/bin/python3so the interpreter uses the distribution-provided PyYAML package. Classification policy and unrelated gate requirements remain unchanged.Testing
.... A documented classifier fixture passes the actual workflow contract; filtering it out still fails. The updated regression fails against the previous parser.python3 ci/checks/test_classify_merge_profile.py -v: 24 tests passed.python3 ci/checks/test_classifier_ci_contract.py -v: 5 tests passed, including an intentionally failing assertion in the real classifier suite and failure/skipped/cancelled dependency results.1.x.xbase.Target Branch
mainor a maintainer-requested release branch such as0.1.x. Targets1.x.xas requested.Risk / Review Notes
Notes: CI wiring and its regression test only. Both the classifier suite and workflow contract run unconditionally before routing outputs; the classification job installs PyYAML for the contract. Issue closure should follow merge and verification of hosted gate behavior because this PR targets a maintenance branch.
Docs / Process Impact
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.