Skip to content

ci(tests): run the full suite on every main push - #1911

Draft
seonghobae wants to merge 22 commits into
mainfrom
ci/main-full-suite-gate
Draft

seonghobae wants to merge 22 commits into
mainfrom
ci/main-full-suite-gate

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

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.

  • protected base: main@64aa08d7fa487deacd41c761c36277ca68cab6c9
  • exact current head: dfeadc7adfb02d73e166104f2987f14fbf7fe82e
  • current topology: open / Draft / mechanically mergeable; compare to protected main is ahead 22 / behind 0 with merge base exactly 64aa08d7...
  • effective protected-base delta is four files only: the trusted-uv workflow, its existing contract test, 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@4dcd25c9f2789e4b8acbeef603e118dd80bfa014 exposed a control-plane defect in Trusted uv Materializer Quality CI run 35414335010 / job 105819872185.

The focused materializer test/branch-coverage step completed successfully. The subsequent repository-wide python -m coverage run -m pytest tests -q step 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 imports defusedxml, which is already pinned and hash-locked in requirements-noema-document-ci-hashes.txt on protected main.

Causal repair

Test-first commit 041ab0749c8b4752a30ea3c7ade8a6a37522e446 adds a deterministic contract requiring the full-suite lane to:

  • trigger when the Noema dependency lock changes;
  • include both OpenCode and Noema locks in the setup-python cache key; and
  • install both hash-locked dependency closures before the complete repository suite.

Production commit f2822cb365dbac10e6f4dfdecb0fee1b49e018ae implements 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 859b1809879c68bb71161c15de3d31ad302bd176 first 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 66dc660774a4e1519eb0694bf087adb8291afe15 rebuilt the candidate from the current protected-main tree and reapplied only the owner files. A second census showed the historical #1911 CHANGELOG.md blob would delete 27 newer protected-main lines even though the original #1911 changelog intent was additive. Current head dfeadc7... therefore restores protected-main CHANGELOG.md exactly 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:

  • Trusted uv Materializer Quality CI 35434149909: queued
  • CodeQL PR 35434149968: queued
  • Security Scan 35434149930: queued
  • SAST Semgrep 35434149903: queued
  • Python Security 35434149939: queued

Do 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#2269 authenticated GitHub redirect containment or .github#2272 Pages 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.

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>
@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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>
@seonghobae

Copy link
Copy Markdown
Contributor Author

Failing checks on 37312475 classified; recovery = one push merging current main (as c36533be), no code change.

The branch was 22 commits behind main; a re-run would re-execute the sidecar bound at run creation, so the head is refreshed instead with a merge of main@fb2ae81d. The merged tree was gated locally (GITHUB_ACTIONS=true): 2920 passed, 1 skipped, 21 subtests passed in 140.69s (0:02:20);TOTAL 13181 0 5326 0 100%;RESULT: PASSED (minimum: 100.0%, actual: 100.0%). Side effect worth having: the new head's Noema/Strix runs are among the first created after fb2ae81d, so their sidecar artifacts carry deferred_count (#1947), candidate/probed/skipped/ready_count (#1949) and unexpected_exception type= (#1950) together.

@seonghobae

Copy link
Copy Markdown
Contributor Author

CodeQL compatibility analysis (actions) on c36533be — same designed state as before, no push. The shard's real output (after ##[endgroup]) is CodeQL scan dispatched. The dispatch workflow will rerun this exact failed CodeQL job after publishing its terminal verdict. — the compatibility shard fails intentionally after dispatching. Its dispatch runs for this head are queued (34021293034 at 08:14:20Z, 34021357120 at 08:15:40Z, one per language) and were sent with the app token, so they will hit the identity gate the same way every dispatch did today (a peer's live test at 07:02Z, right after the variable was touched at 07:01:34Z, still rejected actor=sender=opencode-agent[bot]; see #1929). Nothing in this PR is on that path; the verdict arrives when the owner-gated allowlist admits the dispatcher, and the dispatch workflow then reruns this exact job.

@seonghobae

Copy link
Copy Markdown
Contributor Author

noema-review on c36533be (run 34017954595, job 08:19:19–08:21:43Z, artifact noema-sidecar-evidence 9985677359) — class (4) preflight-fail (capacity); no push. This is the first Noema artifact for this PR on the post-fb2ae81d launcher and it carries the new fields: candidate_count 24 / probed_count 6 / skipped_count 18 / ready_count 0 / deferred_count 0 / rejected_count 6 (target_ready 8, probe_budget 16, account_skip_after_429 2). All three credential accounts answered 429 to their first two probes (nvidia_nim ×2, nvidia_nim_sub ×2, openrouter ×2), the account-skip rule dropped the remaining 18 candidates without probing, and with nothing ready the sidecar failed closed before healthz — the provisioning step failed 2 min 24 s in. No serving phase ran, so no unexpected_exception line, as expected for the no-tools shape.

Same shape as the other post-#1949 samples in this hour (contextual-orchestrator#1078's strix run, the main-push strix run): the launcher's levers behave as designed and the pool is rate-limited end to end — the capacity item on #1948. A push cannot change an upstream 429; the branch already carries current main.

@seonghobae

Copy link
Copy Markdown
Contributor Author

현재 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로 구분합니다.

@seonghobae
seonghobae marked this pull request as draft September 6, 2026 09:21
@seonghobae

Copy link
Copy Markdown
Contributor Author

opencode-review on c36533be is the receipt gate, not a sidecar failure — and the head has since moved; no push from me.

  • The opencode-review check fails with No APPROVED or CHANGES_REQUESTED from opencode-agent on the current head: the required workflow only verifies a receipt. The review itself runs in opencode-review-dispatch.yml, whose validate-pr-metadata rejected 44 of today's 50 failed runs with actor=sender=opencode-agent[bot] — the owner-gated allowlist (CodeQL dispatch terminal status publication remains unproven across repositories #1929). A live test right after the variable was touched at 07:01:34Z still rejected, so the gate value has not changed yet. Nothing in this PR is on that path.
  • Head moved from c36533be to ad245962 at 09:21:48Z (another session merged current main, which now carries fix(strix): name the sandbox bootstrap failure and give it a bounded retry #1953, and added a doctoring document; c36533be is an ancestor, nothing lost). That cancelled the Strix scan that had been running on c36533be since 07:00Z — the cost of moving a head while a scan is in flight. The new head binds the fix(strix): name the sandbox bootstrap failure and give it a bounded retry #1953 sidecar, so its Strix run is the first sample that can show the STRIX_SANDBOX_UNAVAILABLE verdict token if the sandbox class recurs.
  • Earlier on this head: noema-review was class (4) preflight-fail (all three accounts 429 twice, ready 0, fail-closed in 2 min 24 s — artifact 9985677359), and both CodeQL compatibility shards are the designed post-dispatch failure.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Python 3.14 full quality gate (run 34024435377, step 11) — 11 failures, one signature, and it is not this pull request's content. Every failure carries the same line:

pytest.PytestUnraisableExceptionWarning: Exception ignored while calling deallocator
<function _TemporaryFileCloser.__del__ at 0x…>: None

What the evidence rules out:

  • Not this branch's diff. The head adds main-full-suite-gate.yml and a doctoring document; the 11 failing tests are in test_noema_review_gate.py, test_noema_repair_attempt_telemetry.py, test_pingora_edge_policy.py, test_repository_metadata_live_verification.py and test_sandboxed_web_e2e.py, none of which this pull request touches, and their own code builds urllib error bodies from io.BytesIO, not from temporary files.
  • Not the whole tree today. The same workflow passed at 08:23:22Z (ad48dd65) and 08:38:53Z (4cf6febc) on two other branches.
  • Not the previous failures of this workflow. main's last two failures (09-04) were 17 scheduler tests with no unraisable signature, and the 01:29Z failure on another branch was a single test, also without it.

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 main commits this head is the first to carry.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Root cause found (a peer, from local git only; I verified it the same way): this branch's own -W error promotes a pre-existing warning into 11 errors.

ad245962   python -m coverage run -m pytest tests -q -W error
main       python -m coverage run -m pytest tests -q

That one flag turns PytestUnraisableExceptionWarning — raised when _TemporaryFileCloser.__del__ runs during garbage collection — into a failure, and pytest attributes it to whichever test happens to be executing at that moment. Every observation now fits: one signature across 11 unrelated tests, other branches passing the same gate today (they have no -W error), the failing test names not locating the leak, and my 3.12 run not reproducing it (I ran without the flag).

So this is not a regression in main and not a defect in the tests that failed. The leaked closer is not this repository's code either: .github contains exactly one NamedTemporaryFile (scripts/ci/install_strix_timeout_compat.py:78, inside a with), plus 19 TemporaryDirectory and 2 mkstemp; the finalizer almost certainly belongs to a dependency.

The design decision belongs to this pull request, because -W error and its guard test test_full_suite_command_rejects_warnings are what it adds. Three ways to keep the gate green:

  1. -W error -W default::pytest.PytestUnraisableExceptionWarning — keeps every other warning fatal and demotes only the third-party finalizer noise.
  2. -p no:unraisableexception — turns the plugin off, which also hides unraisable exceptions that would be ours.
  3. -W error::UserWarning — matches exactly what the new test asserts (it emits a UserWarning fixture), but stops rejecting every other warning class.

I recommend (1): the guard test still fails on its UserWarning fixture, the stated intent ("reject warnings") survives for everything the repository controls, and the exclusion is narrow and self-documenting. I have re-run the failed job to see whether the promotion is deterministic or GC-timing dependent, and will push (1) unless the branch's author prefers another option — say so here and I will not touch it.

…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>
@seonghobae

Copy link
Copy Markdown
Contributor Author

Pushed 9e846138: option (1), with the control run first. The gate command is now

python -m coverage run -m pytest tests -q -W error -W default::pytest.PytestUnraisableExceptionWarning

I reproduced the mechanism myself on Python 3.12 before changing anything, using a fixture whose __del__ raises (CPython cannot propagate that, so it is always unraisable, and pytest's plugin converts it):

arm command result
1 no flags 1 passed, 1 warning — the stimulus is real
2 -W error 1 failed — the escalation, reproduced
3 this branch's new command 1 passed, 1 warning — the exemption works
4 a UserWarning under arm 3's flags 1 failed — the branch's own contract survives

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 -W error::UserWarning — the latter also makes the suite green while quietly reducing "reject warnings" to one class, and test_full_suite_command_rejects_warnings would still pass either way. A peer ran an independent three-arm version and got the same result. Because it reproduces on 3.12, this is not Python 3.14 behaviour; the flag is the cause.

The seven guard tests in test_trusted_uv_materializer_quality_workflow_contract.py pass on the new head, and the exemption is commented in place with the run id so the next reader does not have to rediscover why one class is exempt. If the branch author wants the flag left alone and the dependency finalizer chased instead, revert this commit and say so — the evidence above stands either way.

@seonghobae

Copy link
Copy Markdown
Contributor Author

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:

PytestUnraisableExceptionWarning -> PytestWarning -> UserWarning -> Warning
issubclass(pytest.PytestUnraisableExceptionWarning, UserWarning) is True

So -W error::UserWarning catches the unraisable warning as well. Against the same __del__-raises stimulus: -W error::UserWarning1 failed; the committed -W error -W default::pytest.PytestUnraisableExceptionWarning1 passed, 1 warning. Option (3) leaves the gate red.

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 -W error still in place, a plain UserWarning is still fatal, which is exactly what test_full_suite_command_rejects_warnings was written to guarantee.

The pushed commit 9e846138 is unaffected; only my description of the alternative was wrong. Recording why it mattered: I rejected option (3) on an intent argument ("it narrows what the author bought") and never ran it, and the peer who proposed it never ran it either. A rejected option that was never executed comes back the moment someone re-opens the design argument — and this one would have turned the gate red again. Running an option you intend to reject costs a second.

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>
@seonghobae

Copy link
Copy Markdown
Contributor Author

Conflict resolved and pushed as 21e128bf (merge of main@5ea1cc47, parents 9e846138 + 5ea1cc47; no rebase, no force-push beyond the lease on the previous head).

One conflict, CHANGELOG.md, and it is the prepend-sibling shape: this branch adds a section at the top and #1958 added one at the same place. Both entries are kept — this branch's "Main quality checks share one runner" stays on top as the not-yet-merged change, with #1958's dispatch-concurrency entry directly below it, which is the order the file will have once this merges. No code conflicts.

Checks for the touched file: both test files that read CHANGELOG.md pass (39 passed), and the workflow contract for the file this branch actually changes passes (7 passed). Conflict markers: zero.

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>
@seonghobae

Copy link
Copy Markdown
Contributor Author

Second conflict resolved and pushed as 8761c004 (merge of main@0b0f1047; both changelog entries kept, this branch's on top as the not-yet-merged change, #1957's below it). Same shape as the last one: CHANGELOG.md only, no code conflicts, zero markers. Checks for the touched files: the two test files that read the changelog plus this branch's own workflow contract — 46 passed.

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>
@seonghobae

Copy link
Copy Markdown
Contributor Author

Fourth resolution today, pushed as 664a5dd9 (merge of main@c232ca03; CHANGELOG.md only, no code conflicts, zero markers, 24 sections with this branch's entry on top). Checks for the touched files: 46 passed.

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:

resolved as invalidated by
21e128bf #1953
(same push window) #1958
8761c004 #1957
664a5dd9 #1959

Across the repository the pattern is measurable: 21 of the last 29 commits on main touched this file, and three merges landed within 27 minutes this evening. Every one of these resolutions took the same shape — one file, both entries kept, no code involved — which is the signature of a serialization point rather than a disagreement. A separate pull request is addressing the file's insertion strategy; I am not duplicating that work here, and this branch stays a draft.

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>
@seonghobae

Copy link
Copy Markdown
Contributor Author

Fifth resolution, pushed as 2f0729d9 (merge of main@dd0b96fe; CHANGELOG.md only, no code conflicts, zero markers, 25 sections with this branch's entry on top). Checks for the touched files: 46 passed.

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.

@seonghobae seonghobae added priority: medium Normal-priority or P2 work status: draft Draft pull request type: maintenance Maintenance, build, dependency, or operational upkeep maintenance labels Sep 7, 2026 — with ChatGPT Codex Connector
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintenance priority: medium Normal-priority or P2 work status: draft Draft pull request type: maintenance Maintenance, build, dependency, or operational upkeep

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant