Repository navigation
fix(OMN-15410): collect the 4 remaining uncollected pytest roots, shrink meta-guard baseline to zero - #2555
Conversation
…ink meta-guard baseline to zero OMN-15378 (#2542) wired scripts/deploy-agent/tests into CI and added a meta-guard that inventoried the test roots CI never collects. It recorded four more roots of the identical class as KNOWN_UNCOLLECTED_DEBT. This collects all four and closes the two seams that let the class exist. Roots now collected (366 tests that had never run in CI): scripts/ci/tests, scripts/tests, scripts/runtime_build/tests, src/omnibase_infra/services/observability/agent_actions/tests Rot found by running them for the first time: 4 tests in the agent_actions root have been RED since 2026-04-02. OMN-7227 (#1127) made ConfigAgentActionsConsumer.kafka_bootstrap_servers required; these four constructions never supplied it and raised ValidationError for ~17 weeks with zero signal. Fixed test-side, supplying the same literal the already-passing _make_health_consumer helper in the same module uses, not by relaxing the field, since fail-fast on missing Kafka wiring is what OMN-7227 bought. Mechanism, not just a baseline edit: * pyproject.toml testpaths is now the single source of truth for what CI collects, and ci.yml full-suite step passes NO positional path so it inherits that list verbatim. It previously hardcoded "pytest tests/", which made the workflow, not pyproject, the real definition of the suite: a root added to testpaths would still never have run. * scripts/ci/detect_test_paths.py gains COLLOCATED_TEST_ROOTS so a NARROWED smart-selection run can reach the collocated roots from a diff touching their code, rather than only via an unrelated full-suite escalation. * ModelTestSelection TestPath widened to admit a collocated tests/ dir; the final path component must still be "tests", so the selector still cannot emit a source directory. * The guard now derives collected roots from testpaths, rejects a testpaths entry with no directory behind it (pytest exit 5), fails closed on an empty testpaths, asserts ci.yml passes no positional path, and asserts testpaths vs COLLOCATED_TEST_ROOTS parity in both directions. * KNOWN_UNCOLLECTED_DEBT is now EMPTY: the guard has converged to zero known uncollected roots, its stated OMN-15378 goal. docker/runners/runner-image.lock.json regenerated: runner_image_identity binds pyproject.toml bytes, so the testpaths edit re-binds the identity (digest-only change, image_version unchanged at v6). Caught by tests/ci/test_runner_image_identity.py, which is exactly the gate working. Refs: OMN-15410, OMN-15378, OMN-14880
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 38 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
Comment |
|
| Verdict | Meaning | Blocks merge? |
|---|---|---|
passed |
No critical findings | No |
blocked |
CRITICAL findings found | Yes |
degraded |
All models unavailable (infra) | No (pilot) |
Powered by omniintelligence.review_pairing.cli_review — node-based adversarial review via HandlerLlmCliSubprocess (OMN-8468/OMN-8524)
#5556) * evidence: OCC companion pass 1 for OmniNode-ai/omnibase_infra#2555 * evidence: OCC companion self-bind for #5556 --------- Co-authored-by: node-occ-companion-effect <occ-companion-effect@omninode.ai>
…lity leg) Peer PR #2553 (OMN-15378 follow-up) merged to dev at 03:48:40Z while this PR was open, touching the same guard module and its test file. Hand-merged, not blind-rebased: - validate_test_root_collection.py docstring: kept #2553's accurate three-legs description of STANDALONE_PROJECT_ROOTS verification (it added real code -- _workflow_runs_on_pull_request -- that the OMN-15410 text under-described), plus this PR's ci.yml-seam and selector-seam items, plus the KNOWN_UNCOLLECTED_DEBT-is-now-empty fact. - test_validate_test_root_collection.py: both sides added a docstring item 5. #2553's continues from item 4 (same STANDALONE_PROJECT_ROOTS subject) so it stays 5; the OMN-15410 item becomes 6. Neither was dropped. PAIR_INCOMPATIBLE break found and fixed (both PRs were individually green): #2553's three new synthetic-repo tests built a tmp_path repo with no root pyproject.toml and no tests/ dir. OMN-15410 makes testpaths the single source of truth, so collected_roots() fails closed without one -- 3 tests went RED on merge. Fixed the FIXTURE, not the guard: a repo with no root pyproject.toml is not a faithful stand-in for any real repo, and weakening the fail-closed check would reintroduce the silent-empty-collection failure mode this PR exists to kill. testpaths lists only tests/ so scripts/widget-agent/tests stays uncollected and the standalone-registration path under test is still the thing exercised. RED-proof that the merge kept #2553 load-bearing: mutating _workflow_runs_on_pull_request to return True unconditionally makes test_registered_workflow_that_never_runs_on_a_pr_fails_closed FAIL (assert 0 == 1); restored and re-verified green. 101 passed across the guard, selector and CI-summary suites on .200.
…losed (#2569) omnibase_infra dev requires exactly one status check ("CI Summary", strict=false), and that context is a poller that reads actions/runs/${RUN_ID}/jobs -- its OWN ci.yml run. Every check emitted by a different workflow file is therefore invisible to it AND absent from branch protection: enforced by neither layer. On a real merged head (#2567, 0fca3b5) that is 59 distinct cross-workflow check-run names against 40 inside the run's suite. Proven consequence, not inferred: PR #2555 merged 2026-07-30T04:25:09Z with "CI Summary" = success while "deploy-gate / deploy-gate" = failure on the same head SHA. All 53 in-run jobs really were green, so the run-scoped checks structurally could not catch it. Rather than re-fan 59 required contexts -- which discards the single-umbrella design of OMN-4497/OMN-14127, and wedges the branch the moment a listed context does not report on some PR shape -- the umbrella now covers them: EXPECTED_EXTERNAL_CONTEXTS is asserted against the PR head's commits/{sha}/check-runs, requiring present + completed + success. Missing or still-running is PENDING, which the caller deadline converts to FAILURE. This is the presence assertion OMN-14456 AC4 asked for and never got. Membership is measured, not guessed. Each of the 17 admitted contexts reported on 16/16 of the dev PRs merged 2026-07-29T23:04Z..2026-07-30T14:54Z, scored at merge time (started_at <= mergedAt; post-merge runs are a retrospective artifact that produced three phantom failures on the first pass). Replaying those 16 payloads through the shipped resolver yields 15 green and exactly one block -- #2555 on deploy-gate, the real defect. Four measured exclusions are recorded as data in MEASURED_NOT_ENFORCED_CONTEXTS rather than silently dropped: a path-filtered context at 1/16, a flaky publisher EFFECT at 10/16, a judgment gate at 14/16, and occ-preflight/eligibility (already a STRICT gate, and the one name observed on both sides of the suite boundary). Resolution is latest-wins by (started_at, id), deliberately matching GitHub's own required-context rule. A stricter "any red fails" variant was measured and rejected: check-runs accumulate on a SHA forever, so it makes every transient red permanent and deletes rerun as a recovery path -- it blocks 6 of the same 16 PRs, 5 of them already rerun-green. The residual it leaves, duplicate producers of one context name, is tracked in OMN-15112. The gate is bound in this same change, not merely defined: ci.yml's poll step fetches the check-runs and passes --check-runs-file/--event-name, and a test fails if that wiring is removed. A failed fetch removes the file rather than leaving it stale, and an absent file reads as "no context observed" -> PENDING -> fail-closed. --event-name defaults to pull_request so a forgotten argument enforces rather than skips; merge_group/workflow_dispatch assert nothing. Measured headroom: the slowest asserted context terminalized 24.9 min after CI Summary started, against a 90 min poll deadline. No branch-protection mutation: dev still requires only ["CI Summary"]. What changes is what that context means. Refs: OMN-15496, OMN-14430, OMN-15108, OMN-14456, OMN-14127, OMN-4497, OMN-15112, OMN-14909 Co-authored-by: jonahgabriel <jonahgabriel@users.noreply.github.com>
OMN-15410 — collect the 4 remaining uncollected pytest roots, shrink the meta-guard baseline to zero
Closes the residual class OMN-15378 (#2542) named but deliberately left open. Related: OMN-15378 (class definition + meta-guard), OMN-14880 (same class in omnimarket).
RED before
The guard shipped by #2542 passes today only because the four roots sit in its
KNOWN_UNCOLLECTED_DEBTallowlist:366 tests had never run in CI. Running them for the first time surfaced real rot: 4 tests in the agent_actions root fail.
git log -Lpins the RED-since date: 2026-04-02, OMN-7227 / #1127 replaceddefault="localhost:19092"withField(...). That PR's diff touchedconfig.pyonly — it never updated these four constructions, because nothing could see them. ~17 weeks RED with zero signal.What changed
The rot (4 tests). Supplied
kafka_bootstrap_servers="localhost:19092"— the same literal the already-passing_make_health_consumerhelper in the same module uses. Fixed test-side, not by relaxing the field: fail-fast on missing Kafka wiring is exactly what OMN-7227 bought. Nothing deleted; all four assert live product behavior (DLQ defaults, the 127.0.0.1 health-bind default).Mechanism, not just a baseline edit. Two seams let this class exist, and both are now guarded:
pyproject.tomltestpathsis the single source of truth. The ci.yml full-suite step used to runuv run pytest tests/— a hardcoded positional path that made the workflow, not pyproject, the real definition of the suite. A root added totestpathswould still never have run. The step now passes no positional path and inherits the list verbatim; the guard fails closed if one reappears (use--ignoreto exclude, never a positional include).COLLOCATED_TEST_ROOTSinscripts/ci/detect_test_paths.py. The full suite is only one of two pytest steps. A narrowed smart-selection run reaches only what the selector maps, sotestpathsalone would have made these roots reachable only via unrelated full-suite escalations. Each root is now selectable from a diff touching its own code. Parity betweentestpathsand this mapping is asserted in both directions.Supporting changes:
ModelTestSelection.TestPathwidened to admit a collocatedtests/dir (final path component must still betests, so the selector still cannot emit a source directory); the guard now derives roots fromtestpaths, rejects atestpathsentry with no directory behind it (pytest exit 5), and fails closed on an emptytestpaths(barepytestwould otherwise collect the whole repo including.venv).KNOWN_UNCOLLECTED_DEBTis now EMPTY — the guard has converged to zero known uncollected roots, its stated OMN-15378 goal.docker/runners/runner-image.lock.jsonregenerated.runner_image_identitybindspyproject.tomlbytes, so thetestpathsedit re-binds the identity. Digest-only change;image_versionunchanged at v6. This was caught bytests/ci/test_runner_image_identity.py— the gate working as designed, not a workaround.GREEN after
All gates run on
.200(stickybeatz-studio) per rule 11a, via patch-transfer: edited locally,git diff | ssh … git apply, then sha256-verified all 9 files byte-identical on the remote before running anything. No gate here was run against an unedited copy.RED-proofs are live, not just synthetic. Both new seams were demonstrated failing before being restored:
Each also has a committed synthetic RED test (
test_positional_pytest_args_detects_reintroduced_path,test_full_suite_check_reports_reintroduced_positional_path,test_synthetic_missing_testpaths_entry_is_a_violation,test_collected_roots_fails_closed_on_empty_testpaths,test_selected_path_contract_admits_collocated_roots_but_not_source_dirs).Honest note on the local full suite
A full-suite run on
.200completed once (27,013 passed) but three subsequent runs — including two on the unmodified base commit — died with an xdistINTERNALERROR: KeyError <WorkerController gwN>afternode down: Not properly terminated. That instability reproduces on base and is a.200host/xdist issue, not this diff. Attribution was therefore settled deterministically instead: the 60 non-green node IDs from the completed run were re-run serially on both worktrees.e2050a20:12 passed, 48 skipped2 failed, 10 passed, 48 skipped← the runner-identity regression, root-caused and fixed above12 passed, 48 skipped— identical to baselineThe remaining 48 skips / xdist-only errors are
.200environment artifacts (no Postgres/Kafka/LINEAR creds) present on base. Hosted CI is the authoritative full-suite surface for this PR.pre-commit run --files <the 9 changed files>: 60 passed, 29 skipped, 0 failed, working tree clean. (--all-filesreports pre-existing repo-wide failures — URL Authority Gate, SPDX, env-var checks — none of which touch any file in this diff.)Files
pyproject.tomltestpaths= the 5 collected roots; single source of truth.github/workflows/ci.ymltests/scripts/ci/detect_test_paths.pyCOLLOCATED_TEST_ROOTSselector mappingscripts/ci/test_selection_models.pyTestPathadmits collocated roots, stilltests-terminalscripts/validation/validate_test_root_collection.pysrc/.../agent_actions/tests/test_hardening.pytests/ci/test_validate_test_root_collection.pytests/unit/scripts/ci/test_detect_test_paths.pydocker/runners/runner-image.lock.jsonSeams
pyproject.toml testpaths↔ ci.yml full-suite argv — asserted bycheck_full_suite_invocationpyproject.toml testpaths↔COLLOCATED_TEST_ROOTS— asserted both directions bycheck_collocated_selector_coverageCOLLOCATED_TEST_ROOTSvalues ↔ModelTestSelection.TestPathpattern — asserted bytest_selected_path_contract_admits_collocated_roots_but_not_source_dirspyproject.tomlbytes ↔runner-image.lock.json— asserted by the pre-existingtests/ci/test_runner_image_identity.pyUpdate 2026-07-30T04:1xZ — conflict with #2553 resolved; terminal CI state
Peer conflict, hand-merged. OMN-15378 follow-up #2553 merged to dev at 03:48:40Z while this PR was open, touching the same guard module and its test file. Merge commit
0dbedbca, resolved by hand (not a blind rebase):validate_test_root_collection.pydocstring — kept fix(OMN-15378): gate deploy-agent tests through CI Summary — advisory job becomes mechanism #2553's three-legs description ofSTANDALONE_PROJECT_ROOTSverification (it landed real code,_workflow_runs_on_pull_request, that this PR's text under-described), plus this PR's ci.yml-seam and selector-seam items, plus theKNOWN_UNCOLLECTED_DEBT-is-empty fact.test_validate_test_root_collection.py— both sides added a docstring item5.; fix(OMN-15378): gate deploy-agent tests through CI Summary — advisory job becomes mechanism #2553's continues from item 4 (same subject) so it stays5., the OMN-15410 item becomes6.. Neither dropped.import yaml(both sides added it) was removed byruff --fix.PAIR_INCOMPATIBLE break found and fixed — both PRs were individually green. #2553's three new synthetic-repo tests build a
tmp_pathrepo with no rootpyproject.tomland notests/dir. OMN-15410 makestestpathsthe single source of truth, socollected_roots()fails closed without one — 3 tests went RED only on merge. Fixed the fixture, not the guard: a repo with no rootpyproject.tomlis not a faithful stand-in for any real repo, and relaxing the fail-closed check would reintroduce the silent-empty-collection mode this PR exists to kill.testpathslists onlytestssoscripts/widget-agent/testsstays uncollected and the standalone-registration path under test is still what's exercised.RED-proof that the merge kept #2553 load-bearing: mutating
_workflow_runs_on_pull_requesttoreturn Trueunconditionally makestest_registered_workflow_that_never_runs_on_a_pr_fails_closedFAIL (assert 0 == 1); mutation reverted and re-verified green. 101 passed across the guard, selector, and CI-summary suites on.200.Terminal CI state
mergeStateStatus=UNSTABLE, 111 pass / 1 fail / 6 skipping. The one failure isdeploy-gate / deploy-gate, which is not load-bearing on this branch — live readback:gh api repos/OmniNode-ai/omnibase_infra/branches/dev/protection/required_status_checks --jq '.contexts'→["CI Summary"]deploy-gateappears nowhere inCI Summary's strict or skippable gate lists (job 90775808771)CI Summarypass ·OCC Companion Merged Gate (OMN-15214)pass ·CI Tests Gatepass · all 15/15 test splits passdeploy-gate is red for two independent, already-ticketed reasons, neither fixable in this repo:
src/.../agent_actions/tests/test_hardening.pyas a "runtime path" — filed OMN-15463 (5th recurrence of the over-broad-matcher class; the shared validator lives in omniclaude and is used by 3 repos).dod-deploy-assessmentincontracts/OMN-15410.yamlis refused by the OMN-14443 falsifiability ratchet (commands found: ['gh','grep']) — that is OMN-15388's exact class. Not repaired here: implementer ≠ evidence author (OMN-14055).Evidence companion OCC#5556 merged 03:48:09Z. It was not born admissible — its first head failed the OMN-9350 lint (
occ-self-bind-pr-5556embedded PR #5556 with nocheck_valuereferencing that literal, the OMN-15382 class-3 mis-binding, also live on #5542/#5546/#5558 — OMN-15407's class). The producer self-healed on re-run by emitting a literal pin; no manual remediation by this lane.Evidence-Ticket: OMN-15410
Evidence-Source: OCC#5556