ci(tests): run the full suite on every main push - #1911
seonghobae wants to merge 22 commits into
Conversation
No workflow ran `tests/` on an unfiltered `main` push. Verified directly: - `agent-review-runtime-quality-ci.yml` has only a `pull_request` trigger (no `push:` key at all) with a narrow paths list. - `opencode-review-dispatch.yml` triggers solely on `repository_dispatch: types: [opencode-review]`. - `trusted-uv-materializer-quality-ci.yml` does run on `push: branches: [main]` but filters to the materialize/uv surface. - Seven workflows do push to main unfiltered (security scanners, SBOM, scorecard, secret-scan, the merge scheduler) and none of them run `tests/`. So merging a change to, say, `pr_review_merge_scheduler_core.py` or `opencode-review.yml` triggered no full-suite run, a suite-breaking merge landed silently on `main`, and the breakage first appeared as a red check on the next unrelated pull request. That is the failure mode behind #1823, #1826, #1828, #1892, and #1895, and behind the repair PRs #1829, #1874, and #1883. The new workflow deliberately carries no `paths` filter, since the point is to catch merges no path list anticipated. It is not in the organization required-workflow ruleset and is not injected into sibling repositories, so it costs one runner slot per `main` push in this repository only; successive pushes coalesce through its concurrency group instead of stacking. Root cause found by a peer session; this is the prescription half, kept separate from that session's documentation of the gap. actionlint: clean. Full suite with this file present: 2883 passed, 1 skipped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks 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 |
A read-only Codex audit (a different model family, run per AGENTS.md's verification discipline) caught two claims that were broader than the evidence: - "Every other workflow that runs the full suite is either PR-only or carries a narrow paths filter" missed `repository-metadata-reconcile.yml`, which runs an unrestricted `pytest -q` on an hourly schedule. That does not contradict this workflow's reason to exist — a schedule is not a push, so a broken merge still sits undetected until the schedule fires — but the sweeping phrasing was wrong. - The materializer workflow does not watch "only the materialize/uv surface": its push paths also cover `tests/conftest.py`, `pyproject.toml`, the tooling requirements lock, and the repository branch-coverage tests. The same audit confirmed the two things that would have made this gate inert if wrong: `coverage report` enforces `fail_under = 100` from pyproject.toml without the flag (coverage 7.15.4 exits 2 below threshold), and no-argument `interrogate` reads `fail-under = 100` and the `tests` exclusion from the same file (interrogate 1.7.0). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Failing checks on
The branch was 22 commits behind |
|
|
|
Same shape as the other post-#1949 samples in this hour (contextual-orchestrator#1078's strix run, the |
|
현재 c36533beb63ba980c2c520fc9fe5c4c448937c8b와 main43024633 기준에서 중복을 확인했습니다. 새 main-full-suite-gate와 기존 trusted-uv-materializer-quality-ci의 full-quality-gate가 materializer main 변경에 동일한 전체 pytest/coverage를 각각 실행합니다. 기존 파일의 3.10 compile/import와 3.14 전체 품질 검사를 한 runner로 순차 통합하고, main push만 경로 필터를 없애며 전체 docstring 검증을 승계하는 최소 후보를 격리 worktree에서 준비합니다. PR 경로 필터, 두 Python 버전, 읽기 전용 권한과 SHA pin, PR/head 및 push/SHA 구분은 유지합니다. 삭제되는 별도 3.10 check 이름은 현재 main 보호 규칙의 required contexts에 없음을 직접 확인했으며 보호 설정은 바꾸지 않습니다. 새 workflow의 기능을 완전히 승계한 뒤 해당69줄 파일만 제거하고 Git 이력은 보존합니다. 기존 PR/원격 head를 일반 갱신 직전에 재조회하며 활성 작성자가 있으면 알려 주세요. #1879 응답 정리 수정의 보호 병합 전에는 경고 엄격 전체 검사 결과를 별도 prerequisite로 구분합니다. |
|
|
|
What the evidence rules out:
The signature itself is a finalizer running during garbage collection, which pytest attributes to whichever test happens to be executing at that moment — so the failing test names do not locate the leaked object, and a deterministic reproduction needs a Python 3.14 interpreter (the toolchain available to me here is 3.12). I am not pushing a speculative change on that basis. The cheap discriminator is a re-run of this one workflow: it runs no review sidecar, so re-running does not resurrect stale trusted code the way re-running a review job would. Triggered now. If it passes, this was finalizer timing; if it fails identically, the leak is real and I will bisect it against the recent |
|
Root cause found (a peer, from local git only; I verified it the same way): this branch's own That one flag turns So this is not a regression in The design decision belongs to this pull request, because
I recommend (1): the guard test still fails on its |
…e gate -W error made the whole suite reject every warning, which is what this branch set out to buy. It also promoted PytestUnraisableExceptionWarning, raised when a dependency's `_TemporaryFileCloser.__del__` runs during garbage collection, into a hard failure. pytest attributes an unraisable warning to whichever test happens to be executing when the collector runs, so the gate failed 11 unrelated tests (run 34024435377) and named none of them accurately. Keep -W error and exempt that one class back to a printed warning. Verified as a four-arm control on Python 3.12, so this is not a 3.14-specific behaviour: the warning is raised with no flags (1 passed, 1 warning); -W error alone turns it into a failure; this command returns it to a warning; and a UserWarning is still fatal under it, so test_full_suite_command_rejects_warnings keeps asserting what it was written to assert. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Pushed I reproduced the mechanism myself on Python 3.12 before changing anything, using a fixture whose
Arm 1 matters as much as arm 2: without it a negative result would have meant "no stimulus", not "no escalation". Arm 4 is why this is option (1) and not The seven guard tests in |
|
Correction to the comment above: option (3) does not work at all, so "both options make the suite green" was wrong. A peer caught it and I confirmed it here: So That also changes what arm 4 proves. It is not "which of two working options to prefer" — it is the independent evidence that option (1) keeps the branch's own contract: with the blanket The pushed commit |
Both changelog entries kept: this branch's shared-runner quality gate stays on top as the not-yet-merged change, with #1958's dispatch-concurrency entry below it in merge order. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Conflict resolved and pushed as One conflict, Checks for the touched file: both test files that read Running note for whoever picks this up: the branch is a draft, so nothing here flips it. |
Changelog prepend siblings again: this branch's entry stays on top as the not-yet-merged change, with #1957's entry below it in merge order. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Second conflict resolved and pushed as This is the third prepend collision on this branch today (#1953, #1958, #1957 each landed a section at the top while this one sat open). If it keeps costing round trips, the durable fix is to give the file a stable insertion point — a marker line the tooling inserts under — rather than having every branch write to line 1. Not something to change inside this pull request. |
Fourth changelog prepend collision on this branch today; both entries kept, this branch's on top as the not-yet-merged change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Fourth resolution today, pushed as The four collisions on this one branch, each caused by a different merge landing a section at line 1 while this pull request sat open:
Across the repository the pattern is measurable: 21 of the last 29 commits on |
Fifth changelog prepend collision today; both entries kept, this branch's on top as the not-yet-merged change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Fifth resolution, pushed as Five collisions on this branch today, one per merge that landed while it stayed open: #1953, #1958, #1957, #1959, #1960. Each was the same shape — one file, both entries kept, no code involved. |
Current owner state — 2026-09-19 KST
This Draft is the canonical owner lane for the repository-wide post-merge/full-suite quality gate carried by
trusted-uv-materializer-quality-ci.yml.main@64aa08d7fa487deacd41c761c36277ca68cab6c9dfeadc7adfb02d73e166104f2987f14fbf7fe82e64aa08d7...docs/doctoring/agent-review-runtime-quality-workflow-consolidation-20260903.md, and the new dependency-closure regression. No unrelated current-main source is replaced.Exact consumer RED that reopened this owner lane
.github#2269@4dcd25c9f2789e4b8acbeef603e118dd80bfa014exposed a control-plane defect in Trusted uv Materializer Quality CI run35414335010/ job105819872185.The focused materializer test/branch-coverage step completed successfully. The subsequent repository-wide
python -m coverage run -m pytest tests -qstep failed during test import with:ModuleNotFoundError: No module named 'defusedxml'This is not a materializer integrity/download failure and is not a #2269 Strix/CodeQL networking defect. The workflow runs the complete repository test suite but installed only
requirements-opencode-review-ci-hashes.txt; the Noema document surface importsdefusedxml, which is already pinned and hash-locked inrequirements-noema-document-ci-hashes.txton protected main.Causal repair
Test-first commit
041ab0749c8b4752a30ea3c7ade8a6a37522e446adds a deterministic contract requiring the full-suite lane to:Production commit
f2822cb365dbac10e6f4dfdecb0fee1b49e018aeimplements exactly that contract. No unpinned dependency install, warning suppression, test exclusion,continue-on-error, or gate weakening is introduced.Protected-main reconciliation repair
The old #1911 branch was hundreds of commits behind current protected main. An ordinary two-parent reconciliation commit
859b1809879c68bb71161c15de3d31ad302bd176first made current main an ancestor, but retaining the stale branch tree exposed a repair finding: the PR diff expanded to 141 files. That candidate was not accepted as the final tree.Without force-push or destructive rebase, ordinary-forward commit
66dc660774a4e1519eb0694bf087adb8291afe15rebuilt the candidate from the current protected-main tree and reapplied only the owner files. A second census showed the historical #1911CHANGELOG.mdblob would delete 27 newer protected-main lines even though the original #1911 changelog intent was additive. Current headdfeadc7...therefore restores protected-mainCHANGELOG.mdexactly and drops that stale collateral.The resulting protected-base diff is again bounded to four owner files. Historical commits remain in ancestry; only the current tree is proposed for integration.
Current hosted evidence
Fresh natural exact-head runs for
dfeadc7...are not terminal and are therefore not acceptance evidence:35434149909: queued35434149968: queued35434149930: queued35434149903: queued35434149939: queuedDo not transfer GREEN, approvals, or receipts from predecessor heads. Any exact-current failure is an RCA input for this owner lane.
Ownership boundary
#1911 owns the repository-wide/full-suite quality workflow and its dependency closure. It does not absorb
.github#2269authenticated GitHub redirect containment or.github#2272Pages shell-input security work. Those sibling source deltas remain in their own owner lanes and must later converge losslessly under the normal central stack.Keep this PR Draft until the exact final head has terminal hosted evidence and the live protected-branch review requirements are satisfied. No force-push, destructive rebase, self-approval, administrator bypass, synthetic status, source-neutral wake commit, unchanged-head blind rerun, or gate weakening.