CI: unblock audit image pipeline and scope Ruff to changes - #6
Merged
Conversation
CI has never once passed in this repository's history (all 15 recorded runs since the workflow was introduced on 2026-06-10 are failures, including the introducing commit itself), which meant the Docker build-and-push job -- gated on the lint-and-test job succeeding -- has never been able to run for any commit, including the merged audit:events signing/integrity-classification work (PR1, PR-B0, PR2). Root cause #1: an empty, tracked pyproject.toml (present since this repo's root commit) satisfies `if [ -f pyproject.toml ]` in the install step, permanently shadowing the `elif [ -f requirements.txt ]` branch that was always the correct one -- `pip install -e .` then fails because setuptools can't auto-discover a package layout from an empty file. Deleting it restores the already-correct, already-written fallback. Root cause #2, found once #1 was fixed: pytest collection then fails on a missing httpx (required by fastapi.testclient.TestClient, used in tests/conftest.py and tests/test_routes_audit.py) and the async tests fail execution without pytest-asyncio (tests/test_logger.py, tests/test_decorators.py) -- neither was ever listed anywhere for CI to install. Both added to the existing "pip install ruff pytest" test- tooling line, not to requirements.txt (they are not runtime dependencies the Docker image needs). Root cause #3, found once #1 and #2 were fixed: `ruff check .` reports 107 pre-existing findings across 32 files (97 confirmed, by running ruff against pre-PR1 history, to predate the audit-event signing/integrity series entirely; the remaining 10 were added across PR1/PR-B0/PR2 without CI ever being able to catch them). These are not fixed here -- deliberately out of scope, and several of the security-relevant BLE001 hits (audit/signing.py's own verify_audit_event, worker/main.py) are correct as written, matching this series' own fail-closed design, not accidental bugs a blanket fix should touch. Ruff is now scoped to the Python files actually changed by each push/PR instead of the whole repository, so that legacy debt no longer blocks new work while new violations remain fully gated -- validated directly: a deliberately introduced, uncommitted unused-import violation in a changed file was caught (non-zero exit) and then fully reverted; this same commit's own diff (zero .py files) passes the scoped check while `ruff check .` run separately in the same moment still reports the same 107, proving the two are genuinely decoupled. Base-commit selection uses merge-base against the PR's target branch for pull_request events and github.event.before (guarded against the all-zero first-push SHA) for push events; falls back to the full repository check, not a silent skip, if no base can be determined. Mirrors the situation omnibioai-control-center already solved for its own unrelated 341-finding backlog, generalized: that fix is a hand-maintained static file list; this computes the changed-file set itself. Validated in a genuinely isolated environment (throwaway venv, not the shared dev environment): 242/242 tests pass, including the real Redis/MySQL integration suite. Full test suite re-confirmed here: 242/242. CI-only change. No application, test, dependency, Docker, database, or docker-compose file touched. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
a5167bb's changed-file scoping treats every push event's base commit the same way: trust github.event.before unless it's empty or the all-zero SHA. That's correct for an ordinary push to an existing branch, but a newly-created ref -- which is what every normal `git push origin vX.Y.Z` tag push is -- always reports `before` as all-zero, since there is no prior history *on that ref*, even though the tagged commit itself has a perfectly good parent in the actual commit graph. The existing "can't determine base" guard then routes every tag push into the unscoped `ruff check .` fallback -- the same 107-finding check that has never once passed -- which meant `lint-and-test` failed on every tag, and the tag-gated `docker` job (needs: lint-and-test, if: startsWith( github.ref, 'refs/tags/v')) could never run. The one event that exists specifically to trigger publication was the one event this scoping didn't actually help. Adds a dedicated branch, checked before the generic push case, for github.ref_type == 'tag': resolves the tagged commit's own first parent via `git rev-parse --verify --quiet HEAD^` (available with no extra checkout work, since `fetch-depth: 0` already fetches full history) and uses that as BASE_SHA instead of ever consulting `before` for tags. If the tagged commit has no parent (tagging the repo's own root commit), the --verify --quiet guard leaves BASE_SHA unset, which still falls back to the existing conservative full-repository check rather than silently skipping -- unchanged fallback behavior, just no longer reached on every ordinary tag push. Nothing else changes: pull_request's merge-base resolution, ordinary push's github.event.before handling, the --diff-filter=ACMR file selection, the full Ruff rule set, and the Docker job's own gating are all untouched. Validated in disposable git worktrees (discarded after, main branch history never touched): - ruff check . still reports the same 107 pre-existing findings. - 242/242 tests pass (isolated venv). - A deliberate violation in a changed file is still caught on an ordinary push (exit 123). - A simulated tag push with the pre-fix logic, same inputs: falls back to the full check, 108 errors (107 legacy + 1 injected), fails. - The same simulated tag push with this fix: resolves BASE_SHA to the real parent, scopes to the one changed file, catches only the injected violation (exit 123) -- not the 107 legacy findings. - A docs-only tag (zero changed .py files since parent): resolves base correctly, nothing to lint, exits 0 -- a real clean release tag would now pass lint-and-test and let the Docker job run. - A root-commit (no-parent) tag: guard correctly leaves BASE_SHA unset and falls back to the full check without crashing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
CI has never once passed in this repository's history — all 15 recorded runs since the workflow was introduced (2026-06-10) are failures, including the introducing commit itself. This meant the Docker build-and-push job — gated on
lint-and-testsucceeding — has never been able to run for any commit, including the merged audit:events signing/integrity-classification work (PR1, PR-B0, PR2).Root causes, in the order each was uncovered
Empty, tracked
pyproject.toml(present since this repo's root commit) satisfiesif [ -f pyproject.toml ]in the install step, permanently shadowing theelif [ -f requirements.txt ]branch — the correct one all along.pip install -e .then fails: setuptools can't auto-discover a package layout from an empty file. Deleted — restores the already-correct fallback with zero workflow logic changes needed for this part.Missing test-only dependencies, found once docs: document the DB sink + query API that already ship, add endpoints #1 was fixed:
httpx(required byfastapi.testclient.TestClient, used intests/conftest.py/tests/test_routes_audit.py) andpytest-asyncio(required to execute this repo's existing@pytest.mark.asynciotests) were never installed by CI. Added to the existingpip install ruff pytesttest-tooling line — not torequirements.txt, since neither is a runtime dependency the Docker image needs.107 pre-existing Ruff findings, found once docs: document the DB sink + query API that already ship, add endpoints #1 and feat(audit): make worker runtime ready #2 were fixed. 97 confirmed (by running Ruff against pre-PR1 history) to predate the audit-event signing/integrity series entirely; the remaining 10 were added across PR1/PR-B0/PR2 without CI ever being able to catch them. Not fixed here, deliberately — several of the security-relevant
BLE001hits (audit/signing.py's ownverify_audit_event,worker/main.py) are correct as written, matching this series' own fail-closed design, not accidental bugs a blanket fix should touch.The fix for #3: scope Ruff to changed files, not the whole repo
Ruff now runs only against the Python files actually changed by each push/PR — legacy debt no longer blocks new work, while new violations remain fully gated. Validated directly, not assumed:
.pyfiles) passes the scoped check, whileruff check .run separately in the same moment still reports the same 107 — proving the two are genuinely decoupled.merge-baseagainst the PR's target branch forpull_requestevents;github.event.before(guarded against the all-zero first-push SHA) forpushevents; falls back to the full repository check — not a silent skip — if no base can be determined.This generalizes what
omnibioai-control-centeralready did for its own unrelated 341-finding backlog — that fix is a hand-maintained static file list (ci: scope ruff check to this milestone's own files); this computes the changed-file set dynamically instead.Validation
ruff check .: still reports the 107 legacy findings, unchanged, not fixed.git diff --check: clean.main@7892749with no unrelated commits.Explicitly NOT in this PR
audit/signing.py,worker/main.py,db/models.py, migrations,consumers/*are all untouched.requirements.txt, Dockerfiles,docker-compose.yml— untouched.omnibioai-studio— untouched.if: startsWith(github.ref, 'refs/tags/v')) and is not triggered by this PR — no image built, no image published, no tag created.🤖 Generated with Claude Code