Skip to content

feat(bin): add fm-pr-status.sh GitLab merge request status reporter - #11

Merged
alandomio merged 6 commits into
mainfrom
fm/fm-mr-status-script
Sep 5, 2026
Merged

alandomio merged 6 commits into
mainfrom
fm/fm-mr-status-script

Conversation

@alandomio

Copy link
Copy Markdown
Owner

Intent

Promote a working local script (~/.local/bin/mrstat) into firstmate's bin/ as a house-style fm-.sh tool: a one-line-per-merge-request GitLab status reporter that reports merged/approved/conflicted state and, most importantly, whether the reported pipeline actually ran against the real branch head rather than GitLab's often-misleading merge-result run against a synthetic commit (which can show green over a failed or never-tested head). This is a promotion/hardening job, not a rewrite - the five-verdict logic (green(head), FAILED(head), manual(head), NO-HEAD-RUN, and the green-badge-on-unverified-head trap), the control-character scrubbing before JSON parsing, and reading approval from detailed_merge_status (never the approved flag, which can be true with zero approvals required) must all survive intact. Two deliberate hardening changes were required: (1) the original script hardcoded one operator's organization's repo shortnames (e.g. peter-park/...) into an 'expand_repo' function with a catch-all guess; since bin/ is shared tracked material in a repo that other people install as a template, this could not ship. Shortnames now resolve from local, gitignored state instead - specifically from git remotes of existing clones under projects/, the same registry firstmate already uses for project management - and an unresolvable shortname fails with a clear error naming what it looked for, never guessing an org prefix. A full group/project path (containing '/') is still accepted verbatim. (2) Forge coverage: the tool is deliberately kept GitLab-only, with the reason stated in the script's header, rather than left as an unstated deviation from firstmate's usual forge-agnostic fm-pr- family (fm-pr-check.sh/fm-pr-merge.sh/fm-pr-poll.sh already handle both forges) - because the entire reason this tool exists is the GitLab-specific merge-result-vs-head gap, and GitHub has no equivalent hazard (its checks attach directly to the head commit), so a GitHub code path would add surface without hardening any of the actual traps this task is about. The script is named bin/fm-pr-status.sh, fitting the existing fm-pr-* family, with a house-style header stating the merge-result trap, a --help flag (via the same awk-over-own-header pattern other fm-.sh scripts use), and is shellcheck-clean via bin/fm-lint.sh. Tests live at tests/fm-pr-status.test.sh, following the ~40 existing test files' conventions (sourcing tests/lib.sh, a fake glab in a fakebin dir intercepting calls, no real network), and assert the parsing/verdict logic against fixture JSON: all five verdicts, the control-character-in-response case, and the zero-approvals-required case (approved:false/approved_by:[] but detailed_merge_status:mergeable must report 'ok', not NOT-APPROVED). Explicit scope boundary: only this one script and its test - the existing fm-pr-.sh scripts, fm-lint.sh, and anything under data/state/config were not touched. docs/scripts.md got one added row for the new script, matching its existing inventory-table convention.

What Changed

  • Added bin/fm-pr-status.sh: prints one compact line per GitLab merge request (state, approval, pipeline, head sha, notes) and never trusts the head_pipeline badge — it reads the merge request's pipeline list to decide whether a run existed against the real branch head, reporting green(head) / FAILED(head) / manual(head) / NO-HEAD-RUN / UNVERIFIED and disclosing a merge-result badge, conflicts, draft state, and failed source=external statuses only as notes. It scrubs control characters before parsing API responses, reads approval from detailed_merge_status rather than the approved flag, resolves bare repo shortnames from the origin remote of a local clone under projects/<name> (failing with a message naming what it looked for instead of guessing an org prefix) while accepting a full group/project path verbatim, and exits non-zero when a merge request or head-run lookup could not be read. It is GitLab-only by design, with that reasoning stated in the header, and carries a --help flag rendered from its own header.
  • Added tests/fm-pr-status.test.sh: 29 fixture cases driven through a fake glab with no network, covering all five verdicts, merge-result vs. detached vs. push pipeline ref forms, full-sha comparison, external-status handling, control characters in merge request text, the zero-approvals-required shape (approved:false/approved_by:[] with detailed_merge_status:mergeable reporting ok), unreadable/failing API responses, shortname resolution from HTTPS and scp-style remotes, and the --help output.
  • Added one row for the new script to the docs/scripts.md inventory table.

Risk Assessment

✅ Low: The change adds one self-contained new script plus its test file and a single docs row, touches no existing code path (grep confirms fm-pr-status is referenced nowhere else in the repo), and the three fix rounds each landed a regression case that pins the corrected behavior; the remaining findings are header-wording accuracy, one uncovered new branch, and a subprocess-coupling cleanup, none of which affect a verdict on realistic GitLab data.

Testing

Ran the change's targeted suite tests/fm-pr-status.test.sh (29 cases, all pass) and, because a passing suite alone does not show the operator-facing behavior, drove the script end-to-end through a stand-in glab replaying recorded GitLab API bodies to capture a real CLI transcript. The transcript demonstrates the tool's whole reason to exist — a green merge-result badge on an untested head renders as NO-HEAD-RUN with the badge disclosed only in the notes — plus green/failed/manual head verdicts, MERGED, UNVERIFIED on an unreadable pipelines lookup with a non-zero exit, approval read from detailed_merge_status in both misleading directions, a control-character body that breaks a naive parse but not the tool, a red external status kept out of the CI verdict yet disclosed, shortname resolution from projects/<name> with a clear failure for an unknown one, and a --help text that matches the verdicts and columns actually printed. No failures, no flakiness, worktree left clean.

Evidence: fm-pr-status.sh CLI transcript (annotated, incl. --help and glab call log)

Source: fm-pr-status.sh CLI transcript (annotated, incl. --help and glab call log)

fm-pr-status.sh - end-user CLI transcript
=========================================
Recorded against a stand-in `glab` that replays recorded GitLab API bodies
(merge_requests/<iid> and merge_requests/<iid>/pipelines). No network.
Shortnames resolve from projects/<name> clones, as they do in real use.

Fixture set (one merge request per row):
  !101 head run green            !105 already merged
  !102 GREEN MERGE-RESULT BADGE, !106 draft, head run blocked on manual jobs
       head never tested         !107 pipelines lookup returns HTTP 403
  !103 head run failed, conflicts,
       description carries a raw 0x02 control character
  !104 head CI green + failed Atlantis external status

$ fm-pr-status.sh --repo gateway 101 102 103 104 105 106 107
gateway!101                         opened  ok            green(head)   9f3c0ab7  
gateway!102                         opened  ok            NO-HEAD-RUN   1c7d55ea  - badge is merge-result (success on aa00bb11)
gateway!103                         opened  -             FAILED(head)  4b81ff20  - CONFLICTS
gateway!104                         opened  NOT-APPROVED  green(head)   77e2a4c0  - external status red: atlantis/plan
gateway!105                         MERGED  on 2026-08-28
gateway!106                         opened  draft         manual(head)  5d10c8ba  - DRAFT - cannot merge until marked ready
gateway!107                         opened  ok            UNVERIFIED    c4419ab0  - badge is merge-result (success on bb99cc88); head-run lookup failed - the pipeline list could not be read
$ echo $?
1   # non-zero: !107's head-run lookup could not be read

Read the rows against what a naive badge reader sees:

  !102  head_pipeline.status = success, source = merge_request_event,
        badge sha aa00bb11 != head sha 1c7d55ea
        -> the badge is GREEN. The tool prints NO-HEAD-RUN and discloses the
           badge only in the notes, under its own status and origin.
           This is the entire trap the tool exists to close.

  !104  head_pipeline.status = success, source = push, sha == head sha
        -> green(head) is correct for CI, and the failed source=external
           Atlantis status is disclosed as its own note rather than folded
           into (or hidden behind) the CI verdict.

  !103  the fixture's description carries a raw 0x02 byte; `json.loads` on the
        unscrubbed bytes raises "Invalid control character at: line 1 column
        96". The row parses anyway - the response is scrubbed first.

  !104  detailed_merge_status = not_approved with approved:true and an empty
        approved_by -> NOT-APPROVED. !101 is the mirror case: approved:false,
        approved_by:[] but detailed_merge_status = mergeable -> "ok", because
        zero approvals were REQUIRED. Approval never reads the `approved` flag.

  !107  the pipelines call fails; the row says UNVERIFIED, never NO-HEAD-RUN -
        "could not look" is not "never tested" - and the run exits non-zero.

$ fm-pr-status.sh gateway!101 gateway!102        # repo!iid form
gateway!101                         opened  ok            green(head)   9f3c0ab7  
gateway!102                         opened  ok            NO-HEAD-RUN   1c7d55ea  - badge is merge-result (success on aa00bb11)

$ fm-pr-status.sh peterpark/platform/gateway!104  # a full path is used verbatim
gateway!104                         opened  NOT-APPROVED  green(head)   77e2a4c0  - external status red: atlantis/plan

$ fm-pr-status.sh acme-widgets!7                  # unresolvable shortname
error: unknown repo 'acme-widgets' - no clone at projects/acme-widgets; pass a full group/project path instead
exit=1  # names what it looked for; never guesses an org prefix

$ GLAB_CALL_LOG=... fm-pr-status.sh --repo gateway 101 104   # API calls issued
  projects/peterpark%2Fplatform%2Fgateway/merge_requests/101
  projects/peterpark%2Fplatform%2Fgateway/merge_requests/101/pipelines?per_page=30
  projects/peterpark%2Fplatform%2Fgateway/merge_requests/104
  projects/peterpark%2Fplatform%2Fgateway/merge_requests/104/pipelines?per_page=30
  # the pipeline list is read on every row, including !101 where the badge
  # already matches the head - otherwise a red external status there is invisible.

$ fm-pr-status.sh --help
fm-pr-status.sh - one compact line per GitLab merge request: merged,
approved, conflicted, and whether its reported pipeline is ACTUALLY green.

The trap this exists to close: GitLab's head_pipeline is frequently a
MERGE-RESULT run against a synthetic merge commit, not the branch head, so a
green badge can sit on top of a head that failed or was never tested at all.
This script never reports the badge. It establishes whether a run existed
against the real head and reports one of:
  green(head)     a run on the real branch head succeeded - trustworthy
  FAILED(head)    a run on the real head failed, whatever the badge says
  manual(head)    the head's run is blocked on manual jobs - not "passed"
  <status>(head)  any other head-run status, printed under its own name
  NO-HEAD-RUN     nothing ever ran against the head; "never tested" is not
                  "passed"
  UNVERIFIED      the head-run lookup itself could not be read, so no
                  pipeline verdict is claimed at all

There is deliberately no green verdict for a merge-result run: a green badge
over an unverified head prints NO-HEAD-RUN, and the badge appears only in the
notes under its own status and origin ("badge is merge-result (success on
<sha>)"), so a green merge-result run is never mistakable for a green head.

A verdict is only ever printed from data that was actually read. When a call
fails or returns something unreadable the row says UNREACHABLE (the merge
request itself could not be read) or UNVERIFIED (the head-run lookup could
not be read) and the run exits non-zero, rather than reporting "never tested"
for a response nobody managed to look at.

Three further traps stay encoded because they were each got wrong by hand:
  - merge request descriptions routinely carry raw control characters that
    break `jq`/`json.load` mid-parse; the API response is scrubbed first.
  - approval is read from detailed_merge_status, not the `approved` flag:
    approved==true with an empty approved_by list means zero approvals were
    REQUIRED, not that anyone signed off.
  - a merge request's pipeline list mixes real CI runs with source=external
    entries that third-party tools (Atlantis and friends) post through the
    commit status API. Only non-external runs decide a verdict; a red
    external status is disclosed in the notes on its own terms, and a head
    carrying nothing but external entries is still NO-HEAD-RUN.

GitLab-only by design: the merge-result-vs-head gap above is this script's
entire reason to exist, and GitHub has no equivalent - its checks attach
directly to the head commit, so there is no comparable trap to close, and a
GitHub path would add an unrelated, un-hardened code path for no verdict this
tool needs to make. fm-pr-check.sh / fm-pr-merge.sh / fm-pr-poll.sh remain
the forge-agnostic family for arming and landing a PR/MR once it is ready.

A repo argument containing "/" is used verbatim as a GitLab project path.
A bare shortname instead resolves from a local clone at projects/<name> (the
same registry firstmate already keeps for project management): its origin
remote is read and its path extracted, regardless of host, so this script
carries no hardcoded organization. An unresolvable shortname fails with a
clear message naming what it looked for rather than guessing a prefix.

Usage:
  fm-pr-status.sh <repo> <iid> [<iid>...]
  fm-pr-status.sh <repo!iid> [<repo!iid>...]
  fm-pr-status.sh --repo group/subgroup/project <iid>...
  fm-pr-status.sh -h | --help

Output columns: repo!num  state  approval  pipeline  head-sha  [- notes]
A merged merge request prints just "repo!num  MERGED  on <date>", and an
unreadable one just "repo!num  UNREACHABLE (<why>)". Conflicts, draft state,
what the badge actually is, and any failed external status are all disclosed
as their own notes - never folded into the pipeline verdict.

Requires: glab (authenticated against the target GitLab host), python3.
Evidence: Operator view: one row per merge request
$ fm-pr-status.sh --repo gateway 101 102 103 104 105 106 107
gateway!101 opened ok green(head) 9f3c0ab7
gateway!102 opened ok NO-HEAD-RUN 1c7d55ea - badge is merge-result (success on aa00bb11)
gateway!103 opened - FAILED(head) 4b81ff20 - CONFLICTS
gateway!104 opened NOT-APPROVED green(head) 77e2a4c0 - external status red: atlantis/plan
gateway!105 MERGED on 2026-08-28
gateway!106 opened draft manual(head) 5d10c8ba - DRAFT - cannot merge until marked ready
gateway!107 opened ok UNVERIFIED c4419ab0 - badge is merge-result (success on bb99cc88); head-run lookup failed - the pipeline list could not be read
$ echo $?
1

!102 is the trap: head_pipeline.status is "success" (a merge_request_event run on synthetic
commit aa00bb11, not head 1c7d55ea) and the tool still refuses to call it green.
!104 shows a failed source=external Atlantis status disclosed beside a genuinely green head CI run.
Evidence: Targeted suite result
$ bash tests/fm-pr-status.test.sh -> exit 0
ok - case A: green(head) verdict and zero-approvals-required mergeable status
ok - case B: FAILED(head) verdict and not_approved read from merge status
ok - case C: manual(head) verdict and a real conflict
ok - case D: NO-HEAD-RUN with a failed merge-result badge and a draft note
ok - case E: a green badge on an unverified head is still reported as NO-HEAD-RUN
ok - case F: a merged merge request short-circuits to MERGED
ok - case G: a raw control character in the response does not break parsing
ok - case H: a source branch containing 'merge' is not misread as a merge-result run
ok - case I: a refs/merge-requests/<iid>/head run is recognised as a head run
ok - case J: shas are compared in full, not by their displayed 8-char prefix
ok - case K: an MR with no pipeline reports that, not a phantom merge-result run
ok - case L: an unparseable response reports UNREACHABLE and exits non-zero
ok - case M: a parseable non-MR error body reports UNREACHABLE and exits non-zero
ok - case N: a non-zero glab exit reports UNREACHABLE and exits non-zero
ok - case O: a shortname resolves from an https origin remote of projects/<name>
ok - case P: a shortname resolves from an scp-style origin remote of projects/<name>
ok - case Q: an unresolvable shortname fails clearly and never guesses an org prefix
ok - case R: a full group/project path is used verbatim without resolution
ok - case S: a failing pipelines lookup reports UNVERIFIED, never NO-HEAD-RUN
ok - case T: a non-list pipelines body reports UNVERIFIED, never NO-HEAD-RUN
ok - case U: with head_pipeline null, no arm invents a merge-result badge
ok - case V: a missing head sha is reported as unreachable, not as none(head)
ok - case W: failed external statuses never substitute for a CI verdict
ok - case X: external entries are excluded from the CI verdict but still disclosed
ok - case Y: a failed external status is never dropped on the badge-matches-head path
ok - case Z: a stale push badge is described as a push run, not a merge-result run
ok - case AA: unnamed external entries are identified individually, not by their shared branch
ok - case AB: the head's CI status comes from the newest run, not the first listed
ok - case AC: --help describes the verdicts and columns the tool really emits
- Outcome: 🔧 1 issue found → auto-fixed ✅ across 2 runs (14m10s)

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

⚠️ **Review** - 5 infos
  • ⚠️ bin/fm-pr-status.sh:154 - The head-run filter &#34;/merge&#34; not in str(r.get(&#34;ref&#34;,&#34;&#34;)) uses a substring test, but a plain source-branch ref can contain that substring. Concrete path: an MR from branch fix/merge-conflicts whose head_pipeline is a merge-result run (different sha) takes the else branch at line 143; the pipelines list contains the real head run {sha: &lt;head sha&gt;, ref: &#34;fix/merge-conflicts&#34;}, which the filter discards, so the tool prints NO-HEAD-RUN with note "only a merge-result run exists" for a head that is actually green. Wrong verdict, no error. Merge-result refs are refs/merge-requests/&lt;iid&gt;/merge and detached-head refs are refs/merge-requests/&lt;iid&gt;/head, so match str(r.get(&#34;ref&#34;,&#34;&#34;)).startswith(&#34;refs/merge-requests/&#34;) and endswith(&#34;/merge&#34;) instead of a substring. This whole matcher is also untested: cases D and E are the only ones reaching this code and both feed [], so no test ever exercises the sha-prefix match or the ref filter against real pipeline records.
  • ⚠️ bin/fm-pr-status.sh:114 - The JSON-parse fallback fabricates a head verdict instead of reporting that nothing could be read. except: print(&#34;? ? - - none ? ? -&#34;) sets sha="-" and pipe_sha="-", which compare equal at line 136, so pipe=&#34;none&#34; falls to the *) arm and the row prints none(head) — an assertion that a run existed against the real head. Reachable with any non-empty unparseable response, and also with a well-formed non-MR body: glab api prints the error body on stdout for a non-2xx (mistyped iid, revoked token), which parses fine and defaults every field to the same - / - / none shape. The UNREACHABLE row at line 106 and these rows also leave STATUS at 0, so a caller cannot tell a read failure from a real report. Have the parser emit a distinct sentinel (and check glab's exit status) so the row prints UNREACHABLE and sets STATUS=1, rather than a head-attributed verdict.
  • ⚠️ bin/fm-pr-status.sh:160 - An MR with no pipeline at all produces a factually wrong note. With head_pipeline: null, the parser yields pipe_sha="-" and pipe="none"; that never equals the real sha, so the else branch runs, the pipelines lookup returns nothing, and the output reads NO-HEAD-RUN - only a merge-result run exists (none on -). The verdict is right but the note claims a merge-result run that does not exist, in a tool whose entire value is not mis-stating pipeline provenance. Special-case pipe_sha="-" with a note like "no pipeline recorded for this merge request".
  • ⚠️ bin/fm-pr-status.sh:14 - --help documents two outputs the script cannot produce. Line 14 lists a fifth verdict green(merge) only a merge-result run is green; the head is unverified, but no code path ever assigns green(merge): the green-badge-on-unverified-head case resolves to NO-HEAD-RUN plus a note (which tests/fm-pr-status.test.sh case E asserts as the intended behavior). Line 43 likewise documents Output columns: repo!num state approval conflicts pipeline head-sha [notes], but the printf at line 180 emits six fields with conflicts folded into notes, not a conflicts column. Both were carried verbatim from ~/.local/bin/mrstat, but the header is now the tracked --help contract of a shared tool. This needs the author's call because the fix is a choice between changing the documented verdict labels/columns and changing the emitted ones — both user-visible.
  • ⚠️ tests/fm-pr-status.test.sh:60 - Cases A, B and C pass for the wrong reason. Their fixtures give the MR sha and the head_pipeline sha different values (aaaaaaaa1111 vs aaaaaaaa2222, and likewise bbbb/cccc), which models a merge-result run on a different commit — but the parser truncates both to 8 characters, so aaaaaaaa == aaaaaaaa and the equality branch at line 136 is taken. The green(head)/FAILED(head)/manual(head) assertions therefore never exercise a genuine head-run fixture, and any change to the truncation width or a switch to full-sha comparison would flip all three to the else branch and break them for a reason unrelated to the behavior they claim to cover. Make the two shas identical in A/B/C (as the real API returns), and compare full shas in the script, truncating only for display.
  • ℹ️ bin/fm-pr-status.sh:63 - fm_prstat_repo_path — the headline hardening of this promotion (replacing the hardcoded peter-park/... expand_repo and its catch-all guess) — has no test coverage at all: every test case passes a g/x!N full path, which returns at line 66 before any resolution logic runs. Untested behaviors include the projects/<name> clone lookup, the scp-style vs URL remote parsing at lines 77-85, the .git suffix strip, and specifically the guarantee the intent calls out — that an unresolvable shortname fails with a clear error and never guesses an org prefix. A regression re-introducing a guessed prefix would pass the suite. A case with a temp FM_PROJECTS_OVERRIDE holding a real git init clone plus one missing-shortname case would cover it.

🔧 Fix: fix head-run detection and unreadable responses in fm-pr-status
3 issues (2 warnings, 1 info) still open:

  • ⚠️ bin/fm-pr-status.sh:163 - The head-run lookup is the one remaining call that turns a read failure into a verdict. glab api .../pipelines?per_page=30 runs mid-pipeline, so its exit status is discarded (the pipeline's status is python3's), and the parser exits silently on both an unparseable body (except Exception: raise SystemExit(0), line 172) and a well-formed non-list body (if not isinstance(rs,list), line 173). Concrete path: an MR whose head_pipeline is a merge-result run (so the else branch at line 160 is taken) queried with a token that can read merge requests but not pipelines — glab prints {&#34;message&#34;:&#34;403 Forbidden&#34;} on stdout and exits non-zero; json.load succeeds, rs is a dict, python exits 0 printing nothing, hp is empty, and line 183 prints NO-HEAD-RUN - only a merge-result run exists (success on &lt;sha&gt;) with STATUS left at 0. That is a positive assertion that nothing ever ran against the head, made from a response that was never read — the exact class of fabrication review-2 closed for the merge_requests call at lines 109-142, still reachable on this sibling call. Apply the same boundary that call already uses: capture glab's exit status into a variable before the parse (e.g. raw=$(glab api ...); rc=$?), have the parser emit a distinct UNREADABLE sentinel for the unparseable/non-list cases instead of silent SystemExit, and on either condition route through fm_prstat_unreachable (or a distinct 'could not read pipelines for this merge request' note) so STATUS becomes 1 rather than emitting a head-attributed verdict.
  • ⚠️ bin/fm-pr-status.sh:182 - The fix round guarded only one of the four arms of the case &#34;${hp:-}&#34; block against the pipe_sha=&#34;-&#34; state, so the other three still print a note claiming a merge-result run that does not exist. Line 184 proves the code knows head_pipeline can be absent, but lines 181, 182 and 189 word their notes as if a merge-result badge always exists. Concrete path: an MR with head_pipeline: null (pipe_sha="-", pipe="none", pipe_short="-") whose /pipelines list contains a run on the real head sha with status failed — a shape that occurs whenever GitLab has not associated a head_pipeline while a branch pipeline for the head sha is still listed on the MR. sha != pipe_sha, so the else branch runs; hp="failed"; line 182 emits FAILED(head) ... - badge shows none on merge-result -. The verdict is right but the note asserts a merge-result run with status "none" on commit "-", which is the same false provenance claim review-3 removed from the NO-HEAD-RUN arm, in the one tool whose entire value is not mis-stating pipeline provenance. Same for line 181 ("badge is merge-result; head run is green too") and line 189 ("badge is merge-result -"). Fix at the shared boundary rather than per arm: compute the badge disclosure once before the case (empty when pipe_sha="-", otherwise "badge is merge-result $pipe ($pipe_short)") and append it to each arm's note, so no arm can word a claim the badge state does not support.
  • ℹ️ bin/fm-pr-status.sh:153 - The head-vs-badge equality test compares parser sentinels, not just real shas. The parser defaults both fields to "-" (d.get(&#34;sha&#34;) or &#34;-&#34; and hp.get(&#34;sha&#34;) or &#34;-&#34;, lines 130-131), so a response with no sha and no head_pipeline makes the two "-" strings compare equal, takes the equality branch, falls to the *) arm at line 158 and prints none(head) with head-sha column "-": a head-attributed verdict derived from two absent values. The presence check at line 124 only requires iid and state, so sha being absent does not route to UNREACHABLE. Reachability depends on GitLab ever omitting sha on a single-MR response, which I could not prove from source here, but the guard is one line and matches the sentinel-awareness the code already applies at line 184: require [ &#34;$sha&#34; != &#34;-&#34; ] before treating the shas as equal, so a missing sha falls through to the head lookup (or to UNREACHABLE) rather than being read as a matching run.

🔧 Fix: exclude external statuses from CI verdicts; surface lookup failures
5 issues (2 warnings, 3 infos) still open:

  • ⚠️ bin/fm-pr-status.sh:170 - The fast path silently drops failed external statuses on the head, contradicting captain-1's accepted requirement that a red external status is "never silently drop[ped]". When sha == pipe_sha and pipe_source != external, lines 170-176 compute the verdict from head_pipeline alone and never call the pipelines list, so ext_red is never computed. Concrete reachable state: MR head sha X; a CI pipeline on X (source=push, id 100, success) and an Atlantis external pipeline on X (source=external, id 99, failed). GitLab's head_pipeline is the highest-id pipeline for the head sha, so it is the push pipeline; the fast path fires and the row prints green(head) with empty notes - the failed Atlantis check is invisible. This is exactly the platform-infra !1740 shape with the two pipelines ordered the other way (CI re-run after Atlantis posted). The test suite does not cover it: case X drives the same combination but with head_pipeline on a different sha, so it exercises the else branch only and passes with this gap open. The header's own trap list (lines 28-32) promises "a red external status is disclosed in the notes on its own terms" without qualifying it to one code path. Marked ask-user because the remedy, not the defect, needs authorization: the only way to see external entries here is to issue the /pipelines call on every row including the currently zero-extra-call fast path, which adds one API request per merge request and makes notes appear on rows that today print none - a cost/behavior tradeoff that is the author's call, not a mechanical correction.
  • ⚠️ bin/fm-pr-status.sh:218 - The badge disclosure asserts merge-result provenance from evidence that does not establish it. Line 218 reaches badge=&#34;badge is merge-result ($pipe on $pipe_short)&#34; for every head_pipeline that merely has a sha differing from the MR head and a source that is not external - the branch tests pipe_sha != sha, never what kind of run the badge is, even though pipe_source was added to the parser in this same fix round (line 143) and is already used one line above at 215. Concrete input: MR head sha X, and head_pipeline = a stale push pipeline {sha: W, status: success, source: push} left over from the previous commit because the new commit created no pipeline (a [skip ci] commit, or rules that match no job for a docs-only change). sha != pipe_sha, pipe_source is push, pipe_sha != "-", so the row prints NO-HEAD-RUN ... - badge is merge-result (success on &lt;W&gt;). The verdict is right; the note states a provenance the data contradicts, in the one tool whose entire purpose is not misstating pipeline provenance - the same class of false claim review-3 and review-8 removed from the other note arms. The same wrong wording appears for source=schedule/web/api/trigger and for a head_pipeline whose source field is absent (parser defaults it to "-"). Fix from data already in hand: word the badge from pipe_source (e.g. merge-result only when pipe_source = merge_request_event, otherwise "badge is a $pipe_source run"), and for full precision have the MR parser also emit head_pipeline.ref so the existing is_merge_result predicate at lines 185-189 - which already encodes the exact refs/merge-requests/<iid>/merge test - is the single place that decides what counts as a merge-result run.
  • ℹ️ bin/fm-pr-status.sh:214 - The pipe_sha = &#34;-&#34; badge text overstates what is absent. It says "no pipeline recorded for this merge request", but the only thing absent is the head_pipeline badge; the pipelines list can and does carry runs for the same merge request. Case U in tests/fm-pr-status.test.sh asserts precisely that contradiction as the intended output: a fixture with head_pipeline: null plus a real failed CI pipeline on the head prints FAILED(head) ... - no pipeline recorded for this merge request, a row that names a pipeline result and denies a pipeline exists in the same breath. Reword to what is actually missing (e.g. "no pipeline badge recorded on this merge request" or "GitLab records no head pipeline here") and update case U's assertion string with it.
  • ℹ️ bin/fm-pr-status.sh:203 - The external-entry identifier falls back to a value that does not identify the entry. str(r.get(&#34;name&#34;) or r.get(&#34;ref&#34;) or r.get(&#34;id&#34;) or &#34;external&#34;) uses ref - the branch name - as its first fallback, and ref is populated on every pipeline entry, so id is unreachable. The fallback exists because name can be absent, and for pipelines created through the commit status API it generally is (the pipeline entity's name comes from pipeline metadata, which a commit-status-created pipeline does not set). With two failed external entries on the same branch and no name, the note reads external status red: feature/w, feature/w: a duplicated string that names the branch rather than which check failed, weakening the "its own clearly-named distinct signal" the disclosure is for. Note that the case W fixture supplies &#34;name&#34;:&#34;atlantis/plan&#34; on the pipeline object, so the suite asserts the best-case output and never exercises the fallback. Minimal correction: prefer the pipeline id over ref when name is absent (name or &#34;#&#34;+str(id)), so distinct entries stay distinguishable. Resolving the actual check names would require the /repository/commits/&lt;sha&gt;/statuses endpoint - out of scope here, and not what this finding asks for.
  • ℹ️ bin/fm-pr-status.sh:206 - if ci == &#34;-&#34;: ci=str(r.get(&#34;status&#34;) or &#34;?&#34;) takes the first matching non-external, non-merge-result entry in list order and treats it as the head's run, which is only correct if the /merge_requests/:iid/pipelines response is ordered newest-first. The code never sorts and never reads id, so it depends on an ordering that endpoint does not document. Concrete input: a head sha with two CI pipelines - feat(supervise): make watcher liveness daemon-owned and always-on kunchenguid/firstmate#100 failed (an earlier run) and feat(bin): open local review files in tmux and wezterm surfaces kunchenguid/firstmate#105 success (a manual "Run pipeline" after the fix) - arriving in ascending id order. ci latches onto failed and the row prints FAILED(head) for a head whose latest CI run is green, without erroring. One line makes the intent explicit rather than inherited: sort the entries by id descending before the loop (rs = sorted((r for r in rs if isinstance(r, dict)), key=lambda r: r.get(&#34;id&#34;) or 0, reverse=True)), so "the head's run" always means the most recent one.

🔧 Fix: always read pipeline list; name badge provenance honestly
5 infos still open:

  • ℹ️ bin/fm-pr-status.sh:14 - The header (printed verbatim by --help and by a bare invocation) lists green(merge) as one of "five verdicts", but no code path ever emits that string — grep -rn &#39;green(merge)&#39; bin/ tests/ matches only this comment line. Concrete state: fixture E (MR head 11112222aaaa, head_pipeline {sha: 22223333bbbb, status: success, source: merge_request_event, ref: refs/merge-requests/5/merge}, empty pipelines list) is exactly the green-badge-on-unverified-head case, and the row prints NO-HEAD-RUN … - badge is merge-result (success on 22223333). So the help text names an output token users will grep for and never see. The intent itself words the fifth item as "the green-badge-on-unverified-head trap", not as a verdict string, so the code conforms to the acceptance criteria and only line 14's framing is wrong: reword it to describe the trap and the row it actually produces (NO-HEAD-RUN with the green badge disclosed in the notes) rather than presenting green(merge) as an emitted verdict.
  • ℹ️ bin/fm-pr-status.sh:240 - This round introduced a fast-path arm that sets STATUS=1 while printing a fully-determined head verdict, which the header's exit-code contract (lines 16-20) does not cover: it states that when a call fails "the row says UNREACHABLE … or UNVERIFIED … and the run exits non-zero". Concrete input: MR head sha X with head_pipeline {sha: X, status: success, source: push}, and the /pipelines call returning non-zero (a token with read_api on merge requests but no pipeline read). The fast path at 233 fires, prints green(head), then lines 240-243 attach "external statuses could not be checked" and fail the run — a non-zero exit on a row that is neither UNREACHABLE nor UNVERIFIED. The verdict and the note are both honest; only the header now under-describes when the run exits non-zero. Smallest remedy is one clause in the header stating that a row whose head verdict is sound but whose external-status check could not be read also fails the run.
  • ℹ️ bin/fm-pr-status.sh:220 - Badge naming is a pure function of pipe_sha, pipe_source, and pipe_ref — all three already sit in bash locals after the MR parse — yet pipe_kind is computed inside the pipelines-lookup subprocess and read back with sed -n 3p. That coupling makes badge wording depend on a subprocess that can fail, and the [ -n &#34;$pipe_kind&#34; ] || pipe_kind=&#34;-&#34; fallback then conflates "the lookup produced no output" with "the badge's source field was absent". Concrete state: head_pipeline: null (so pipe_sha=&#34;-&#34;, pipe=&#34;none&#34;) plus any uncaught exception inside the pipelines script (e.g. a non-integer id making the sorted key comparison raise, since stderr is discarded and nothing is printed) yields plook=&#34;&#34;, pipe_kind=&#34;-&#34;, and a row reading UNVERIFIED … - badge is a run of unrecorded origin (none on -); head-run lookup failed — asserting a badge of unknown origin exists for an MR whose own response said there is no badge at all. Deriving the four pipe_kind cases in bash (they need only a refs/merge-requests/*/merge glob for the merge-result arm) removes the subprocess round-trip, the sed -n 3p extraction, and the ambiguous fallback in one move.
  • ℹ️ bin/fm-pr-status.sh:177 - per_page=30 caps the head-run lookup at one page, which is only sufficient if the endpoint returns entries newest-first — the exact assumption the comment added two lines below at 195-196 explicitly refuses to make ("the endpoint does not document an order"). Under that stated assumption the two are inconsistent: a long-lived MR that accumulates more than 30 pipeline entries (each push producing a branch run plus a merged-result run, plus one external entry per commit from an Atlantis-style integration) can have the head's CI run fall outside the 30 returned, and the row then prints NO-HEAD-RUN for a head that was genuinely tested — the precise false verdict this tool exists to prevent, with no error and no note. Either filter server-side for the head (/projects/:id/pipelines?sha=&lt;head&gt;, which makes the page size irrelevant to correctness) or state in the comment that the code does rely on the endpoint's id-descending order, so the sort at 197 and the page cap rest on the same premise.
  • ℹ️ tests/fm-pr-status.test.sh:505 - Every other branch this fix round introduced got a regression case that fails against the pre-fix code (Y for the always-fetch path, Z for badge provenance, AA for the external identifier, AB for the newest-run sort, U for the reworded absent-badge note), but the fast-path lookup-failure arm at bin/fm-pr-status.sh:240-243 has none. Reaching it needs sha == pipe_sha, pipe_source != external, and a failing pipelines call; the existing failure cases S and T both use a badge sha that differs from the head (bbbb2222dddd / bbbb4444dddd), so they only exercise the UNVERIFIED arm at 244. Nothing currently pins the two things that arm decides: that the head verdict is still printed rather than downgraded to UNVERIFIED, and that the run exits non-zero anyway. A case Y variant with FM_TEST_GLAB_PIPELINES_RC=1 asserting green(head), the "external statuses could not be checked" note, and expect_code 1 covers it.
🔧 **Test** - 1 issue found → auto-fixed ✅
  • ⚠️ bin/fm-pr-status.sh:14 - The --help output describes two things the tool never does. (1) It lists green(merge) only a merge-result run is green; the head is unverified as one of the five verdicts the script "returns", but no code path assigns that string — the only verdicts assignable are green(head), FAILED(head), manual(head), $status(head), NO-HEAD-RUN and UNVERIFIED (bin/fm-pr-status.sh:235-254). The situation that line describes now renders as NO-HEAD-RUN with the badge disclosed in the notes; my transcript row !4 is exactly that case. A user who sees the trap fire and greps the help for green(merge) finds a verdict that is never printed. (2) Line 54 states Output columns: repo!num state approval conflicts pipeline head-sha [notes] — seven columns — but the row printf emits six: repo!num, state, approval, verdict, head-sha, notes. There is no conflicts column; conflicts are reported inside the notes as CONFLICTS (transcript row !3). Marked ask-user because the remedy is your call, not a mechanical correction: for (1), either reword the help to describe the trap as a NO-HEAD-RUN row with the badge disclosed, or restore a distinct green(merge) verdict — the round-3 decisions moved the wording toward honest badge disclosure, so I did not assume which you intended. This does not affect any verdict the tool computes; the behavior itself is correct and fully covered by tests/fm-pr-status.test.sh (case E, case K).
  • bash tests/fm-pr-status.test.sh — all 28 fixture cases (A–AB) pass, exit 0
  • Manual end-to-end CLI run: fm-pr-status.sh platform-infra 1 2 3 4 5 6 7 8 99 against a fake glab shim on PATH serving per-iid fixture JSON (no network), exercising green(head), FAILED(head), manual(head), NO-HEAD-RUN (green merge-result badge), NO-HEAD-RUN (external-only head), MERGED, UNVERIFIED, control-char-scrubbed green(head), UNREACHABLE — and asserting the run exits 1
  • Manual verification of the zero-approvals-required trap: fixture with approved:false/approved_by:[] + detailed_merge_status:mergeable renders approval as ok, not NOT-APPROVED (row !1)
  • Manual verification of control-character handling: MR description containing raw 0x01/0x02 still parses to a green(head) row (row !8)
  • Manual shortname-resolution check with a real git init clone at projects/platform-infra: logged every glab api path issued, confirming platform-infra → projects/acme%2Finfra%2Fplatform-infra/merge_requests/1 from the clone's origin remote, --repo acme/infra/other-thing used verbatim, and an unregistered peter-park-billing!42 producing zero API calls and a clear error (exit 1)
  • fm-pr-status.sh --help and bare fm-pr-status.sh (no args) — help renders via the awk-over-own-header pattern, exit 0
  • grep -n &#39;verdict=&#39; bin/fm-pr-status.sh cross-checked against the emitted rows to confirm which verdict strings the code can actually assign

🔧 Fix: correct --help verdict list and output columns; cover with test
✅ Re-checked - no issues remain.

  • bash tests/fm-pr-status.test.sh — all 29 cases (A–AC) pass, exit 0
  • Manual CLI run: fm-pr-status.sh --repo gateway 101 102 103 104 105 106 107 against recorded GitLab bodies via a stand-in glab on PATH (no network), covering green(head), NO-HEAD-RUN under a green merge-result badge, FAILED(head)+CONFLICTS, green(head)+red external status, MERGED, manual(head)+DRAFT, and UNVERIFIED on a 403 pipelines lookup; exit code 1 as designed
  • Manual CLI run: fm-pr-status.sh gateway!101 gateway!102 and fm-pr-status.sh peterpark/platform/gateway!104 — shortname resolution from a projects/gateway clone's origin remote vs. a full path used verbatim
  • Manual CLI run: fm-pr-status.sh acme-widgets!7 — unresolvable shortname error message and exit 1
  • Manual check: python3 -c &#34;json.loads(open(&#39;103.mr.json&#39;,&#39;rb&#39;).read().decode())&#34; on the unscrubbed fixture raises JSONDecodeError: Invalid control character, while the same body renders a correct row through the tool
  • Manual check: logged every glab api path issued for a two-row run, confirming the pipelines list is fetched on every merge request including the badge-matches-head fast path
  • fm-pr-status.sh --help — verdict list and Output columns: line compared against the verdicts and field count the fixture rows actually print
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

Alan Domio added 6 commits September 3, 2026 12:49
Promotes a validated local script (mrstat) into bin/ as fm-pr-status.sh: one
line per merge request naming merged/approved/conflicted state and whether
the pipeline actually ran against the real branch head, since GitLab's
head_pipeline is often a merge-result run against a synthetic commit that can
show green over a failed or untested head.

Hardening over the original: the hardcoded organization shortname map is
removed and replaced with resolution from local projects/<name> clones (no
org baked into shared bin/ code); GitLab-only scope is stated and justified
in the header rather than left as an unstated deviation from the
forge-agnostic fm-pr-* family; and a fixture-driven test covers the five
pipeline verdicts, the control-character scrubbing trap, and the
zero-approvals-required merge-status case.
@alandomio
alandomio merged commit e8613e9 into main Sep 5, 2026
13 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.

1 participant