refactor(ci): extract shared quality-gate reusable workflow for 2 of 8 duplicated files - #1683
Conversation
…8 duplicated files An audit of the 8 .github/workflows/*-quality-ci.yml files that share a bootstrap-templated skeleton found only one pair -- javascript-coverage-quality-ci.yml and organization-commercial-readiness-loop-quality-ci.yml -- where the shared shape (exact-head checkout, an identical pinned six-package requirements heredoc, coverage run --branch + --fail-under=100, compileall, git diff --exit-code) was genuinely the same logic, differing only in timeout, pytest target, and coverage --include path. Extract that into a new workflow_call-only exact-head-coverage-quality-gate.yml and turn both callers into thin uses:/with: wrappers. Verified first that no branch-protection required status check or the org's required-workflow ruleset references either caller's job name, so restructuring them is safe. Updated the contract tests that pinned the old inline text and added one for the new gate's own contract and both callers' input wiring. The other 6 files each encode a genuinely different policy (harden-runner presence, a docstring gate, exact-head-verification mechanics, multi-Python-version matrices with non-shared extra logic, or no coverage --fail-under step at all) so templatizing them would weaken what they individually enforce. Left untouched. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 4 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (7)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| - name: Checkout exact source revision | ||
| uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 | ||
| with: | ||
| ref: ${{ github.event.pull_request.head.sha || github.sha }} |
There was a problem hiding this comment.
📝 Info: Exact-head context survives delegation
Reusable calls retain the caller’s pull-request context. github.event.pull_request.head.sha therefore still binds checkout and verification to the exact head.
Was this helpful? React with 👍 or 👎 to provide feedback.
| python -m coverage run --branch -m pytest --import-mode=importlib ${{ inputs.pytest_target }} -q | ||
| python -m coverage report \ | ||
| --include='${{ inputs.coverage_include }}' \ | ||
| --show-missing \ | ||
| --fail-under=100 | ||
| python -m compileall -q ${{ inputs.compileall_targets }} |
There was a problem hiding this comment.
📝 Info: Caller arguments preserve command behavior
Folded inputs become space-separated shell words. The organization globs still expand into the same pytest and compileall arguments as before.
Was this helpful? React with 👍 or 👎 to provide feedback.
Resolve CHANGELOG.md conflict by keeping both this PR's quality-CI consolidation entry and main's hourly-review-repair-callers entry (#1673), newest first. Also fold in a fix for a Devin Review finding on the new exact-head-coverage-quality-gate.yml reusable workflow: route pytest_target/coverage_include/compileall_targets through step-level env vars instead of interpolating ${{ inputs.* }} directly into the run: script, matching this repo's own established convention (test_verifier_is_data_only_and_workflow_never_executes_downloaded_evidence in test_exact_artifact_sbom_attestation_contract.py already enforces this for exact-artifact-sbom-attestation.yml). Verified glob/word-split behavior for the two callers is unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Resolve CHANGELOG.md conflict by keeping both this PR's quality-CI consolidation entry and main's active_workflow_runs caching entry (ADR-0022), newest first. Full suite after merge: 2591 passed, 1 skipped, 100% branch coverage, 100% docstrings, actionlint clean on the workflow files this PR touches. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Resolve CHANGELOG.md conflict by keeping both this PR's quality-CI consolidation entry and main's fail-closed stale-workflow-cancellation entry, newest first. Full suite after merge: 2625 passed, 1 skipped, 100% branch coverage, 100% docstrings. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
javascript-coverage-quality-ci.yml's pytest_target is the whole tests directory, so it already executes tests/test_exact_head_coverage_quality_gate_contract.py -- but that file was missing from the workflow's own path trigger, so an edit scoped only to that test could merge without the gate that runs it ever firing (Devin Review finding on PR #1683). Added the file to the JS caller's path list, not the org-loop caller's: org-loop's pytest_target is a narrower glob that never matches this filename, so adding it there would trigger a job that doesn't actually exercise the test. Pinned this with a new contract test. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
A sibling session here — this PR was Generated by Claude Code Generated by Claude Code |
Summary
An audit flagged 8
.github/workflows/*-quality-ci.ymlfiles as sharing anear-identical bootstrap template. This PR does the real work of figuring out
which parts of that duplication are safe to templatize and which aren't, and
acts only on the safe part.
(
exact-head-coverage-contract,exact-head-policy) appears in thisrepo's branch-protection required status checks (
gh api repos/ContextualWisdomLab/.github/branches/main/protection) or in theorg's required-workflow ruleset (which per
docs/CWL-MASTER-CONTEXT.mdonly governs Strix/OpenCode Review/the PR Review Merge Scheduler) — so
restructuring these two files can't silently break a required check,
here or in a sibling repo.
javascript-coverage-quality-ci.ymland
organization-commercial-readiness-loop-quality-ci.ymlhad byte-for-byte identical logic (same pinned six-package requirements
heredoc, same
coverage run --branch -m pytest --import-mode=importlib+coverage report --fail-under=100+compileall+git diff --exit-codeshape) differing only in
timeout-minutes, the pytest target, and thecoverage
--includepath. Extracted that shape into a newworkflow_call-onlyexact-head-coverage-quality-gate.ymlreusable workflow (4 required inputs:
timeout_minutes,pytest_target,coverage_include,compileall_targets) and turned both callers intothin
uses:/with:wrappers, following this repo's existingworkflow_callconvention (deploy-pages.yml,pr-review-fix-scheduler.yml).agent-mention-router-quality-ci.yml,exact-artifact-sbom-attestation-quality.yml,noema-token-lifetime-quality-ci.yml,opencode-rust-coverage-toolchain-quality-ci.yml,strix-changed-path-quality-ci.yml, andtrusted-uv-materializer-quality-ci.ymllook similar at a glance but eachenforces a genuinely different policy: harden-runner presence, a
docstring/interrogate gate, different exact-head-verification mechanics
(noema has no
ref:pin at all), multi-Python-version matrices withnon-shared extra logic (a tomli-fallback exercise, a compile-only Python
3.10 contract), or (strix) no
coverage --fail-understep at all —delegating instead to a bash gate script. Forcing these into one template
would either weaken what they individually enforce or need enough
per-caller toggles to defeat the point of sharing. This mirrors the
precedent already documented in this repo for ruling out consolidating
the agent-mention dispatch pair and the noema/opencode/strix
"cancel superseded runs" jobs.
Contract tests updated
tests/test_organization_commercial_readiness_loop_policy.pyandtests/test_organization_commercial_readiness_loop_import_contract.pynowcheck the coverage/exact-head mechanics against the shared gate file and
the subsystem wiring (
coverage_include, fixtures path) against thecaller, instead of the old inline text.
tests/test_exact_head_coverage_quality_gate_contract.pyto pin thenew gate's
workflow_call-only contract, its required/typed inputs, andboth callers' distinct input wiring.
javascript-coverage-quality-ci.ymlhad no dedicated contract test beforethis change; behavior is now covered indirectly through the new gate test
plus the org-loop tests exercising the same shared file.
Test plan
PYTHONPATH=. coverage run -m pytest tests -q— 2603 passed, 1 skippedcoverage report --show-missing— 100% branch coverage onscripts/ciinterrogate— 100% docstringsactionlinton the 3 changed workflow files — clean🤖 Generated with Claude Code