Skip to content

ci(dependabot): auto-merge gate no longer counts by-design skipped pytest jobs as failures - #3149

Merged
vybe merged 3 commits into
devfrom
fix/dependabot-automerge-skipped-gate
Oct 2, 2026
Merged

vybe merged 3 commits into
devfrom
fix/dependabot-automerge-skipped-gate

Conversation

@trinity-ability

@trinity-ability trinity-ability commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #2994

Problem

dependabot-auto-merge.yml's Wait for build + pytest to pass step greps every check-run named build or pytest* and treats any completed non-success conclusion as a failed gate. Since the #2945 tiering every PR carries pytest (base, inline fallback) and pytest (push, absolute failures) as completed / skipped (they run on other events), so the gate has answered a gating check failed on every Dependabot PR since mid-September — with build and pytest (head) green right beside them. The Dependabot queue filled with green patch/minor bumps nobody had looked at; today's merge train merged #3100, #2967, #2965 and #2830 by hand after re-running their flaky journey-smoke and reading the auto-merge log.

Fix

One predicate: skipped is exempt. Every other non-success conclusion (failure, cancelled, timed_out, action_required) still blocks.

Test

tests/unit/test_dependabot_automerge_skipped_gate.py extracts the step's run: from the workflow YAML and executes it under bash with gh stubbed to answer the rows — the script that runs on the runner is the script under test. With the fix reverted, exactly the skipped-rows case fails (verified in a python:3.13-slim container; the test skips on macOS's bash 3.2, which has no mapfile).

Lane C (.github/workflows/), not deploy-dev.yml. No behaviour change for majors — they are still held for a human by the eligibility step.

🤖 Generated with Claude Code

…d pytest jobs as failures

dependabot-auto-merge.yml waits on every check-run named build or
pytest* and treated any completed non-success conclusion as a failed
gate. Since the #2945 tiering every PR carries 'pytest (base, inline
fallback)' and 'pytest (push, absolute failures)' as completed/skipped
— they run on other events — so the gate answered 'a gating check
failed' on every Dependabot PR from mid-September on, with build and
pytest (head) green beside them, and the queue filled with unreviewed
green patch/minor bumps (3100, 2967, 2965, 2830 merged by hand today).

Exempt 'skipped' only. A behavioural test executes the step's script
under bash with gh stubbed: skipped+success passes, failure/cancelled/
timed_out/action_required still block. The test skips on a bash
without mapfile (macOS /bin/bash 3.2); ubuntu-latest runs it.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
pytest (head) runs its suite with '|| true' and is green whatever the tests
do; the unit verdict is the 'regression diff' job, which the ^build$|pytest
filter never selected. With skipped rows no longer failing the gate, a
patch/minor bump that breaks unit tests would have auto-merged. New test runs
the workflow's own jq filter under real jq.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@vybe

vybe commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

merge-train: on this train. I pushed one commit (7c0c3e745) and added Fixes #2994 to the body.

Why: pytest (head) runs its suite with || true and reports green whatever happens. The unit verdict is regression diff, which the ^build$|pytest filter never selected. With skipped rows no longer failing the gate, a patch/minor bump that breaks unit tests would have auto-merged.

What changed:

  • The filter now also selects ^regression diff$.
  • New test: test_the_regression_verdict_is_a_gating_check runs the workflow's own jq filter under real jq.

Not addressed here: the job is guarded on github.actor rather than the PR author's login. That's older than this PR.

@vybe

vybe commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

Correction to the note above: this PR rides the next train (with 3148, 3157's sibling batch), not this one. The pushed fix and keyword stand.

`regression diff` needs the pytest jobs, so GitHub creates its check-run
only after `pytest (head)` finishes. In that window every gating row is
green or skipped, and the gate read "nothing pending" and passed without
the unit verdict. A missing `regression diff` row is now pending, and a
poll where every row is skipped no longer counts as green.

merge-train: mechanical fix on our own PR, per the merge-train note.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@vybe

vybe commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

merge-train: pushed a5497edbf. This is our own PR, so the finding was fixed here instead of sending it back.

Finding: regression diff has needs: [test, baseline, base-run], so GitHub creates its check-run only after pytest (head) finishes. In that window every gating row is green or skipped. The gate read "nothing pending" and passed without the unit verdict, and the PR's own test asserted green on exactly that row set.

Fix:

  • The jq projection now carries the check name.
  • A missing regression diff row counts as pending, bounded by the existing 30-minute ceiling.
  • A poll where every row is skipped no longer counts as green.
  • Two new tests use a stub that changes its answer between polls, so they prove the gate did not decide on the first poll.

The suite is 8/8 locally (bash 3.2 with a mapfile shim), and 8/8 fail against the previous workflow.

Not addressed:

  • gh pr merge --auto merges a later head without re-gating it. --match-head-commit is one option.
  • The 5-minute "no rows → exit 0" fail-open.

Both pre-date this PR.

@vybe vybe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merge-train: batch validated on train PR #3179 (all checks green).

@vybe
vybe merged commit 021cd26 into dev Oct 2, 2026
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants