Skip to content

fix(ci): act on a Unit Tests verdict only for the commit it covers (#1643) - #1644

Closed
JarryShaw wants to merge 1 commit into
mainfrom
fix/1643-workflow-run-verdict-sha
Closed

JarryShaw wants to merge 1 commit into
mainfrom
fix/1643-workflow-run-verdict-sha

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Oct 13, 2026 •

Copy link
Copy Markdown
Owner

Please follow the guide below

What is the purpose of your pull request?

Tick the commit type your subject line carries.

  • fix — corrects a defect
  • feat — adds a feature
  • perf — changes performance, not behaviour
  • refactor — changes neither behaviour nor performance
  • test — tests only
  • docs — documentation only
  • ci — workflows or build tooling
  • release — bumps the version or rolls up a distribution
  • chore — anything else

Description of your pull request and other information

Cause. A workflow_run checks out github.sha, "Last commit on default branch" (GitHub, Events that trigger workflows), not github.event.workflow_run.head_sha. Conda Update 38325136710 bumped 2e941bb under 9083163's verdict.

Fix. Each workflow gets a verdict job (cron-conda.yml:61, cron-vendor.yml:67, deploy-pages.yml:66) running decide (util/verdict_covers_checkout.py:77), and the acting job's workflow_run clause also needs verified == 'true' (cron-conda.yml:112, cron-vendor.yml:116, deploy-pages.yml:115). A stale verdict skips cleanly; schedule runs, Pages' pull-request builds and PR-raised runs run the same jobs as before. For workflow_run the workflow-level group is keyed on the verdict's sha (cron-conda.yml:37, cron-vendor.yml:43, deploy-pages.yml:41), as GitHub cancels "any existing pending job or workflow in the same concurrency group".

Bot commits. A tip passes on top of its verdict only if every commit after it is by github-actions[bot] and touches only conda/build and conda/requirements.txt (util/verdict_covers_checkout.py:58), which is all the 137 conda bumps and 9 build resets on main touch. Vendor bumps rewrite pcapkit/: e58618b's run 36355214959 failed 6 jobs (each Python leg "12 failed", plus the aggregator) where its parent's run 36354135989 failed none, so they wait for their own run. That run exists: since #401 (bb90090, 2026-09-15) bots push with a deploy key, and all 17 bot commits since have a push run of Unit Tests; 5d5d8af, 48bc113 and e4d0823, from before it, have none (GITHUB_TOKEN events "will not create a new workflow run"). This needs the verdict checkout's fetch-depth at 2 or more; it is 20 (cron-conda.yml:80, cron-vendor.yml:86, deploy-pages.yml:85).

Not covered. The verdict covers the checkout only. Push Changes git pull --rebases onto the live tip (cron-conda.yml:197) and Conda Tag git pulls and tags HEAD (:253-254), so a human commit landing after the event can be tagged, and once #1641 lets Conda Tag and Conda deployment run, packaged unverified. #1641 is being reworked to close this before it lands: Conda Tag will tag the sha Push Changes pushed, and Push Changes will refuse a rebase that brought in non-bot commits. A stale Vendor Update run also still concludes success, so Create Release triggers on it as before: create-release.yml:179 is its only conclusion check, and its own Release test gate (:362) is what protects it.

Tests. New tests/project/test_workflow_run_verdict.py, 25 tests: 15 run the script in throwaway repos rebuilt from the 2026-10-13 wave, 9 evaluate the three workflows, and 1 pins that those repos run no git maintenance. Their .gitconfig sets maintenance.auto=false, gc.auto=0 and both autoDetach=false (:100-103). Since git 2.54 a commit's detached auto maintenance repacks once objects/17 holds two loose objects, and on the runner's git 2.55.0 that repack raced TemporaryDirectory cleanup in run 38349290407 ("Directory not empty: 'objects'", Python 3.10). On a local git 2.55.0 build, test_a_merge_on_top_is_refused failed that way in 302 of 800 runs (8 in parallel) before the change and 0 of 800 after. With origin/main's workflows swapped in, all 9 fail (unittest failures=22); at fetch-depth: 1, test_the_verdict_checkout_reaches_past_a_bot_commit fails in all three. A merge whose side branch runs past the depth-20 checkout (e23817b brought in 76 commits) leaves a parentless boundary commit; it is refused with exit 0 (util/verdict_covers_checkout.py:92-93), and test_a_merge_deeper_than_the_checkout_is_refused_not_failed fails with ValueError under a bare strip(). make test is not claimed: the whole suite was not run, only 14 test files, one at a time, all passing.

With #1641. The two PRs merge without conflicts, but 4 of #1641's TestCondaBumpIsTaggedAndUploaded tests then fail, because its cron-conda.yml model has no verdict job. Land this one first: #1641 is being rebased onto it and carries the reconcile, which brings test_release_gates to 103 passed on the merged tree.

Closes #1643

@JarryShaw JarryShaw added fix Pull requests that fix a defect (fix: subject prefix) ci Pull requests that change CI or workflow configuration (ci: subject prefix) review: good-to-go Cross-review at the current head says ready; CI state is separate labels Oct 13, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at 9d4aaca45. Sonnet cross-review, three rounds. Round 2 found that the verdict job failed red, instead of skipping, on a shallow clone's parentless boundary commit; that is fixed at util/verdict_covers_checkout.py:92-93. Round 3 replayed decide() against 13 real Conda Update runs and found both stale ones from the 2026-10-13 wave refused. I re-ran it myself: 24 tests OK, 22 failures with origin/main's three workflows swapped in. Please land this before #1641, which I'm reworking on top of it.

@github-actions

github-actions Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Coverage: 89.66% (unit tier, Python 3.14, c2ee06a1e, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 3 20 3 95.16%
pcapkit/const 18858 954 2366 868 91.06%
pcapkit/corekit 2366 67 760 35 96.48%
pcapkit/dumpkit 275 3 100 3 98.40%
pcapkit/foundation 2884 145 1010 35 94.14%
pcapkit/interface 138 7 52 5 93.68%
pcapkit/protocols 17166 226 4580 191 98.06%
pcapkit/toolkit 1036 168 386 5 82.35%
pcapkit/utilities 494 4 146 4 98.75%
pcapkit/vendor 4409 2342 1006 157 43.25%

Per-file detail: the coverage-html artifact of this run.

@JarryShaw

Copy link
Copy Markdown
Owner Author

Not ready yet: the Python 3.10 leg is red. The failure is in the new test's teardown, not in its body. test_a_merge_on_top_is_refused passed, then TemporaryDirectory cleanup hit OSError: [Errno 39] Directory not empty: 'objects' (job 115137780473). Something still wrote to the throwaway repo's .git/objects during rmtree; my working guess is a detached auto-gc or maintenance process. A worker is establishing the cause and fixing the harness, not muting the error. That will amend this PR, so the verdict resets.

…1643)

Conda Update, Vendor Update and GitHub Pages check out github.sha, main's
tip when the workflow_run is raised, not the commit the completed Unit Tests
run verified. In a merge wave an older commit's success authorised work on an
untested tip: Conda Update 38325136710 bumped 2e941bb under 9083163's
verdict.

- Add a `verdict` job to each, running util/verdict_covers_checkout.py. It
  accepts the tip when it is the verified commit, or that commit plus
  github-actions[bot] commits to conda/build and conda/requirements.txt only.
  The acting job's workflow_run clause now also needs `verified == 'true'`.
- Key the workflow-level concurrency group on the verdict's sha for a
  workflow_run, so a stale run that skips cannot cancel a pending run whose
  verdict matches. Other events keep their groups.
- workflows.rst: describe the check and what it leaves uncovered, and
  renumber the three `uses:` citations.

tests/project/test_workflow_run_verdict.py: 25 pass. With origin/main's three
workflows swapped in, 9 fail (failures=22, subtests counted); with the verdict
checkout at fetch-depth 1, 1 fails in all three workflows. A merge whose side
branch runs past the shallow checkout is refused with exit 0, not a crash.
Its throwaway repos run no git maintenance: since git 2.54 a commit's detached
auto repack raced their cleanup ("Directory not empty", git 2.55.0).

Closes #1643
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at c2ee06a1e. The red 3.10 leg was a test-harness race. Git 2.55, which the runner had, defaults auto maintenance to a geometric repack, and that repack starts once two loose objects share one fan-out directory. The test's second bump commit always hits that, so a detached git repack outlived the test and wrote into .git while TemporaryDirectory was deleting it.

The fix turns off maintenance.auto/gc.auto and detaching in the harness's isolated .gitconfig. test_the_harness_git_runs_no_maintenance pins it. With git 2.55 the race went from 302 of 800 runs failing to 0 of 1200. Only the test file changed. Sonnet reproduced the race and the fix independently. Locally on git 2.47 I get 25 OK.

@JarryShaw

Copy link
Copy Markdown
Owner Author

Resolve conflicts

@JarryShaw

Copy link
Copy Markdown
Owner Author

Resolved by closing: this change is already on main. #1641 was stacked on this PR, so its squash merge 2c4683851 carried this commit too. All six files this PR touches are byte-identical on main and at #1641's head 9ac44a063, and #1643 closed with that merge. Merging main in would leave this PR with an empty diff, so I'm closing it as landed. Reopen it if you'd rather keep it.

@JarryShaw JarryShaw closed this Oct 13, 2026
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Oct 13, 2026
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci Pull requests that change CI or workflow configuration (ci: subject prefix) fix Pull requests that fix a defect (fix: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

fix(ci): workflow_run jobs build main's tip under an older commit's verdict

1 participant