Skip to content

P0: main CI red — CI failure on f66803c #3060

Description

@github-actions

P0: main went red

Workflow: CI concluded failure on main.

Failing run: https://github.com/edobry/minsky/actions/runs/31992444196
HEAD SHA: f66803cfb2b8ac71c5e05feb20bbf8fed08a859b
Head commit: fix(mt#2312): Name the cause of zero check_runs, and stop blaming behind

Summary

The merge gate's CI-presence floor (mt#1309) denied every zero-check_runs PR with the same message: "This is the GitHub Actions webhook-miss class", prescribing an empty-commit nudge. Zero check_runs has two causes and they need opposite recoveries. When the cause is a merge conflict, GitHub never built refs/pull/N/merge, so no pull_request workflow could dispatch — the nudge cannot work, and it re-heads the branch, invalidating any existing reviewer APPROVE and costing another round.

That is not hypothetical: mem#537 records three recurrences (R1 2026-06-04, R2 and R3 on 2026-08-16/17), each of which paid both costs.

The gate now reads the PR's own merge state on the zero path and names the cause.

What planning falsified before any code was written

Two premises this task asserted turned out to be false. Both would have shipped as defects.

behind does not suppress CI dispatch

The spec's SC2 grouped mergeable_state ∈ {dirty, behind, unknown} as "unmergeable", and both memories it was written from (mem#321, mem#537) say GitHub "cannot form refs/pull/N/merge" for dirty or behind.

Measured: PR #3042's base was main's tip at 02:24:45Z. By 02:41:52Z main was 13 commits ahead of that base (compare/84255d5a...02361c4f6ahead_by: 13). The push at 02:43:36Z — with the branch 13 commits behind — dispatched the full 20-check set. Four other open PRs read in the same window (#3050, #3051, #3052, #2945) were all behind, all mergeable: true, all carrying 13–20 check runs.

Only a real conflict stops the merge ref forming. behind has mergeable: true; it blocks the merge button under branch protection, which is a different thing. Grouping it with dirty would send an agent to session_update for what is actually a webhook miss — the same class of wrong answer this gate exists to stop.

blocked is the trap in the other direction, and the spec had it right: with required checks configured, a genuine webhook miss presents as blocked precisely because the required checks are missing. It is a symptom of the zero, not a cause.

So the discriminator is mergeable alone. mergeable_state is reported for triage and never decides. This matches what mt#4182 shipped for session_pr_checks (eabb96d2b, merged earlier today), which branches on mergeable === false and explicitly rejected a broader predicate for the same reason. The two surfaces must not disagree about which state means "CI could not have dispatched."

Both memories are corrected in this task's scope (mem#537 ## The cheap discriminator, mem#321's 2026-06-05 banner).

The merge state is not "one cheap added field"

The spec said the PR object is already fetched, so mergeability is "one cheap added field/call." The gate resolves its PR through resolvePrRefByBranchfetchPrMetaByBranch, which runs gh pr list --head. Probed directly:

$ gh pr list --repo edobry/minsky --head task/mt-4191 --json number,mergeable,mergeStateStatus
[{"mergeStateStatus":"UNKNOWN","mergeable":"UNKNOWN","number":3052}]
$ gh pr view 3052 --repo edobry/minsky --json mergeable,mergeStateStatus
{"mergeStateStatus":"BEHIND","mergeable":"MERGEABLE"}

The list endpoint does not compute mergeability — GitHub runs that as a background job which a GET on the individual PR starts. Extending PR_META_JSON_FIELDS (the route the spec proposed) would have returned UNKNOWN on every call: a change that compiles, typechecks, ships, and silently classifies every PR as inconclusive forever, with no error anywhere.

The change

classifyZeroCheckRuns(state) — a pure function, three outcomes:

merge state cause prescribed recovery
mergeable === false unmergeable-branch session_update + resolve; explicitly warns off the empty commit and names its cost (re-heads the branch, invalidates APPROVE)
mergeable === true (any mergeable_state) webhook-miss the pre-mt#2312 message, unchanged
mergeable === null, or the read failed inconclusive lists both causes and their opposite recoveries rather than asserting either

readMergeState is the IO half, kept separate so the classifier stays pure and directly testable. It re-reads once when mergeable is null, because the first GET is what starts GitHub's job — without that, the common case would report inconclusive and the discriminator would be useless.

getMergeState is passed to evaluateCheckRunsPresence as a thunk, so the extra single-PR read happens only on the zero path — which is already denying the merge. An ordinary merge pays nothing. A test asserts this by passing a thunk that throws.

fetchPullRequestMergeStateRaw joins the existing raw fetchers in pr-context.ts, with the gh pr list finding recorded in its doc comment so the cheaper-looking route is not re-attempted.

The test file was split — require-review-before-merge.test.ts crossed the 1500-line max-lines ceiling once these landed, so the zero-path tests live in require-review-before-merge.zero-check-runs.test.ts.

Execution evidence:

$ bun test --preload ./tests/setup.ts --timeout=15000 \
    ./.minsky/hooks/require-review-before-merge.test.ts \
    ./.minsky/hooks/require-review-before-merge.zero-check-runs.test.ts
 149 pass
 0 fail
 307 expect() calls
Ran 149 tests across 2 files. [58.00ms]

AT1 — a conflicted PR (mergeable: false, dirty) classifies merge-conflict; the message contains session_update and Do NOT push an empty commit, and does NOT contain wake the webhook.
AT2 — a behind but mergeable PR classifies webhook-miss, not unmergeable. This is the regression test for the corrected premise.
AT3 — clean classifies webhook-miss (unchanged).
AT4 — blocked classifies webhook-miss (unchanged) — the required checks are missing because CI never ran.
AT5 — unresolved mergeability classifies inconclusive; the message contains both session_update and empty commit.
AT6 — see ## Live verification below. The unit half is a fixture of PR #3031's shape; the live half exercises the real gh api call and parse.

SC1 — the read is a single-PR GET. fetchPullRequestMergeStateRaw calls gh api repos/<repo>/pulls/<n>; the gh pr list route is ruled out in its doc comment with the probe output above.
SC2 — covered by AT1.
SC3 — covered by AT2, AT3, AT4. The webhook-miss text is byte-identical to the pre-change message on the assertions the mt#1309 tests already pinned (mt#1309 / PR #763 lineage, noFiles/noStage, /merge-coordination step 7a), which all still pass.
SC4 — readMergeState re-reads once on null; parseMergeStateResponse treats null as a KNOWN not-yet-computed value rather than a parse failure (its own test), and an unresolved second read renders inconclusive (AT5).
SC5 — classifyZeroCheckRuns is pure and unit-tested per branch, plus a test asserting mergeable_state alone never decides (same string, opposite mergeable, opposite result).

Full suite, typecheck and lint:

$ MINSKY_PREPUSH_FULL_SUITE=1 bun scripts/run-tests-gated.ts
Ran 13988 tests across 942 files. [196.30s]
run-tests-gated.ts: all test steps passed.

$ validate_typecheck  → 0 errors across 8 projects
$ validate_lint       → 0 errors, 0 warnings across 3720 files

Negative control 1 — the discriminator is load-bearing:

classifyZeroCheckRuns was short-circuited to return "webhook-miss" (the pre-mt#2312 behavior) and the suite re-run. 8 tests went red, all of them mt#2312's; every pre-existing mt#1309 assertion still passed, which is correct — the webhook-miss path is unchanged for a mergeable PR.

(fail) classifyZeroCheckRuns (mt#2312) > AT1: a conflicted PR is the merge-conflict cause
(fail) classifyZeroCheckRuns (mt#2312) > AT5: unresolved mergeability is inconclusive, not guessed at
(fail) classifyZeroCheckRuns (mt#2312) > an unreadable merge state is inconclusive
(fail) classifyZeroCheckRuns (mt#2312) > mergeable_state alone never decides — only `mergeable` does
(fail) evaluateCheckRunsPresence zero-path messages (mt#2312) > the conflict message prescribes session_update and warns OFF the empty commit
(fail) evaluateCheckRunsPresence zero-path messages (mt#2312) > the conflict message names the invalidated-approval cost
(fail) evaluateCheckRunsPresence zero-path messages (mt#2312) > the inconclusive message lists BOTH causes and their opposite recoveries
(fail) evaluateCheckRunsPresence zero-path messages (mt#2312) > an unreadable merge state surfaces the fetch error in the inconclusive message
 141 pass / 8 fail

Negative control 2 — the behind correction is load-bearing:

Control 1 does not discriminate AT2/AT3/AT4: they assert webhook-miss, which that control returns unconditionally, so all three passed while proving nothing about the premise this task exists to correct. So a second control was run, implementing the spec's original premise — {dirty, behind} both treated as unmergeable. Exactly the three behind-specific tests went red, and nothing else.

(fail) classifyZeroCheckRuns (mt#2312) > AT2: a BEHIND but mergeable PR is a webhook miss, NOT unmergeable
(fail) classifyZeroCheckRuns (mt#2312) > mergeable_state alone never decides — only `mergeable` does
(fail) evaluateCheckRunsPresence zero-path messages (mt#2312) > a behind-but-mergeable PR still gets the webhook-miss recovery
 146 pass / 3 fail

Both controls were restored and the suite re-verified green.

What the controls do not buy, stated rather than assumed: they prove the tests can fail for the reverted behavior. The defect CLASS is "a denial message that asserts one cause for an ambiguous symptom." This PR covers the zero-check_runs member. The sibling members in the same file — the required-checks gate's "no matching run" denial (mt#1938) also prescribes the webhook-wake recovery unconditionally — are NOT covered here and remain as they were; that is a separate surface with its own tests, out of scope for this task.

Live verification

The unit tests all feed the classifier a hand-built MergeState, so nothing above exercises the actual gh api invocation or the --jq expression — the part most likely to be wrong in a way typechecking cannot see. Run against the live API from the session:

$ bun -e 'import { readMergeState, classifyZeroCheckRuns } from "./.minsky/hooks/require-review-before-merge.ts"; ...'
PR #3052: {"known":true,"mergeable":true,"mergeableState":"behind"} -> webhook-miss
PR #3051: {"known":true,"mergeable":true,"mergeableState":"behind"} -> webhook-miss

The fetch, the parse and the classification all work end-to-end against real PRs — and the result independently re-confirms the corrected premise: a live behind PR classifies as a webhook miss, which is the right answer, because both of those PRs carry a full check set.

AT6's conflicted-PR half is not live-exercised: producing it means deliberately conflicting a real PR against main, which is a destructive change to shared state that this task's scope does not authorize. The conflicted shape is covered by AT1's fixture, which is PR #3031's shape verbatim — an observed real occurrence recorded in mem#537 R2.

Scope note

.claude/hooks/** is regenerated from .minsky/hooks/** and recompiled in this PR (bun run src/cli.ts compile, verified by git status plus a grep for the new symbol in the generated output, not by the exit code).

Co-Authored-By: minsky-ai[bot] <minsky-ai[bot]@users.noreply.github.com>

What this means

A push to main triggered CI and the workflow above did not conclude success. Per
CLAUDE.md user preference ("main must never be broken"), this is severity-1.

Diagnostic checklist

  1. Open the failing run URL above; identify which job/step failed.
  2. Check whether the offending PR was merged with a known-failing required check
    (operator-API bypass via gh api PUT /merge despite enforce_admins).
  3. Confirm enforce_admins is currently enabled:
    gh api repos/edobry/minsky/branches/main/protection --jq .enforce_admins.enabled
    
    Expected: true post-mt#1938. If false, that is itself a separate finding.

Recovery

  1. Open a hotfix branch off current main.
  2. Apply the smallest fix that turns CI green (often a formatter pass or a config
    flip).
  3. Land via the standard Minsky session flow:
    tasks_create → session_start → session_commit → session_pr_create → /review-pr → session_pr_merge.
  4. Verify the post-merge main build is green within ~5 minutes.
  5. Close this issue with a link to the hotfix PR.

Cross-references

Metadata

Metadata

Assignees

No one assigned

    Labels

    main-redAuto-filed by .github/workflows/main-watch.yml when main CI fails (mt#1938)p0Severity 0: production breakage requiring immediate attention

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions