gr pr merge: a finished check with a `skipped` conclusion is classified as still-running, so the checks gate can never pass

[laynepenney]

What happens

gr pr merge refuses with checks still running on a PR whose only check has finished. There is no state the PR can reach that clears the gate, so the check gate on that PR is permanently unpassable and the only way forward is to waive it.

Measured

On a PR whose sole check was a workflow that concluded skipped:

$ gh pr checks <N>
sync-status   skipping   0   ...
$ gh pr view <N> --json statusCheckRollup
sync-status: COMPLETED SKIPPED
$ gh pr view <N> --json mergeStateStatus
CLEAN

GitHub reports the check as COMPLETED, and GitHub's own merge-state says CLEAN. gr pr merge reports:

⚠ Some PRs have issues:
  - <repo> PR #<N>: checks still running [checks]

Two sources disagree, and the one that is wrong is ours.

Root cause

src/platform/github.rs:830-834 classifies a check run by matching on its conclusion alone:

let check_state = match cr.conclusion.as_deref() {
    Some("success") => CheckState::Success,
    Some("failure") | Some("timed_out") => CheckState::Failure,
    Some("cancelled") => CheckState::Failure,
    _ => CheckState::Pending, // "in_progress", "queued", "neutral", or null
};

Two things are wrong here, and the second is the interesting one.

1. status is available and unused. The API returns status (queued / in_progress / completed) alongside conclusion, and status is the field that actually answers "is this still running." It is read at line 849 for display and never used for classification. Because the match sees only conclusion, it cannot distinguish "no conclusion yet" from "a conclusion I do not recognize", and it defaults both to Pending.

2. The catch-all is open-ended. GitHub's documented conclusions include neutral, skipped, action_required, and stale in addition to the four matched above. All four are terminal, and all four land in _ => Pending. So do any conclusions GitHub adds in future. The comment on that line enumerates "in_progress", "queued", "neutral", or null as the intended cases and does not mention skipped — the arm is catching a case nobody decided rather than one anyone chose.

Why this is worse than a wrong label

A check gate that cannot pass does not fail loudly; it converts itself into a waiver prompt. The operator's only recourse is to waive, and having waived once on a PR where the waiver was clearly correct, waiving becomes the reflex on PRs where it is not. A gate that always fails teaches people to route around it, which costs more than not having had the gate.

The specific hazard is that skipped is common and often permanent: path-filtered workflows, if: conditions, and matrix exclusions all produce a skipped conclusion on every run for a given PR. Those repos will never see this gate pass.

Scope

The same catch-all shape is present in the other platform adapters:

• src/platform/gitlab.rs — "success" => Success, "failed" | "canceled" => Failure, _ => Pending. GitLab pipeline statuses include skipped and manual, which are terminal and would land in the catch-all.
• src/platform/bitbucket.rs — _ => "pending".

Measured on GitHub only. GitLab and Bitbucket are reported from reading the match arms, not from a live run against those platforms, and their status vocabularies have not been checked against their docs. Someone should confirm before fixing those two.

The structural root

CheckState (src/platform/types.rs:151) has exactly three variants — Pending, Success, Failure. There is no variant meaning "finished, with a conclusion this adapter does not recognize." So an adapter meeting an unfamiliar conclusion is forced to pick one of the three, and every adapter has independently picked Pending. The missing arm is a symptom; the missing state is the cause.

Worth noting that the consuming enum is richer: CheckStatus in src/cli/commands/pr/merge.rs:21 has Passing, Failing, Pending, and Unknown, and merge.rs already handles Unknown by warning and proceeding rather than blocking. But the mapping at merge.rs:41-43 is total over the three CheckState variants, so Unknown is unreachable from any platform adapter. The handling exists; nothing can produce it.

Suggested direction

1. Classify on status first: anything not completed is Pending, regardless of conclusion.
2. For completed runs, match conclusions explicitly. skipped and neutral are non-blocking (GitHub itself does not treat them as failures — see the CLEAN merge-state above); action_required and stale are arguably blocking.
3. Give CheckState a variant for completed-but-unrecognized and route it to the existing CheckStatus::Unknown path, so a conclusion added by a platform in future degrades to a warning rather than to a permanent block.

Point 3 is the one worth discussing before implementing — it is a change to a shared type used by four adapters, and the alternative (map unrecognized-completed to Failure) is safer in the short term but has the same route-around-the-gate problem this issue is about.

Test note

A regression test wants a check-run fixture with status: "completed" and conclusion: "skipped" asserting the gate passes, plus a discriminating sibling with status: "in_progress" and conclusion: null asserting it still blocks. Without the second, a fix that simply mapped everything to non-pending would go green.