Skip to content

test(pr): keep GitHub helpers working when gh rejects --slurp - #3572

Closed
att430 wants to merge 1 commit into
kunchenguid:mainfrom
att430:fm/ci-monitor-gh-slurp
Closed

test(pr): keep GitHub helpers working when gh rejects --slurp#3572
att430 wants to merge 1 commit into
kunchenguid:mainfrom
att430:fm/ci-monitor-gh-slurp

Conversation

@att430

@att430 att430 commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Intent

Stop every PR run from parking at a decision gate because the CI check reader uses a flag the installed gh does not have.

THE GAP: the validation pipeline's CI monitor calls gh api --slurp. The installed gh is 2.45.0 (Ubuntu package 2.45.0-1ubuntu0.3, released 2025-07-18), which predates that flag and rejects it with unknown flag: --slurp. The monitor therefore cannot read check results at all and parks the run at an ask-user gate claiming it cannot verify CI.

WHY IT MATTERS: the runs are fine and CI is green. The failure is purely that the tool cannot read the answer, but it surfaces as a decision gate that stops the pipeline and asks a human. Every PR-producing run hits this, and each one costs a firstmate turn to resolve by hand. It has already done so on comment-relay PR #6 and PR #7.

VERIFIED WORKAROUND, for context: gh-axi pr checks <n> -R <owner>/<repo> reads the same result without that flag and returned "1 passed, 0 failed, 1 total" immediately.

THE FIX is one of: upgrade gh past the version that introduced --slurp, or point the CI check read at a call the installed gh supports. ESTABLISH WHICH BEFORE CHANGING ANYTHING. Do not upgrade a system package as a first move without checking what else on this host depends on it. State the evidence for the route you pick, and if the honest answer is that the fix belongs upstream rather than in this repo, say so and report it rather than working around it locally.

SCOPE: own the whole problem (duplicate backlog items describe the same issue). Do not conflate this with pipeline agent-selection work; it is unrelated to any codex switch.

ACCEPTANCE CRITERIA:

  • A PR run on this host can read check results and reach a CI verdict without parking at an ask-user gate.
  • The route chosen is justified with evidence about what else on this host depends on gh, if the route is an upgrade.
  • Whatever you change is covered so the regression is caught if the flag/version assumption breaks again; follow the repo's existing test layout and conventions in tests/, and do not assert implementation-source bytes.
  • If the real fix is upstream, the deliverable is a precise, reproducible upstream report plus whatever local mitigation is safe, not a silent local hack.

CONSTRAINTS:

  • Do not upgrade a system package as a first move without checking dependents.
  • Do not silently wrap or shadow gh.
  • Load firstmate-coding-guidelines before editing firstmate shared tracked material and follow its knowledge-placement, one-owner, and repo style rules.
  • bin/*.sh must pass shellcheck; run bin/fm-lint.sh before treating a script change as done.

DECISIONS MADE WHILE DOING THE WORK (reviewer context):

  • The CI reader that calls gh api --paginate --slurp is no-mistakes internal/scm/github/github.go getWorkflowRunChecks, still present on no-mistakes v1.62.0. Firstmate itself does not call that flag.
  • Ubuntu Noble apt has no newer gh than 2.45.0; apt reverse-depends lists only optional sugarjar. A system gh upgrade is not available from archive.ubuntu.com and was rejected as a firstmate-side workaround.
  • The real fix belongs upstream. Filed CI monitor parks green PRs at ask-user: gh api --slurp is unknown on Ubuntu 24.04 gh 2.45.0 no-mistakes#942 with a reproducible report (Ubuntu 2.45.0 unknown flag, green PRs parking at ask-user, gh-axi workaround).
  • Local firstmate mitigation is a regression test, not a gh wrapper: forge-helper test fakes now reject gh api --slurp and gh pr checks --json (the other 2.45 gap), and an explicit test drives PR record, poll, and merge through that surface. That is intentional and must not be "fixed" by teaching firstmate to use --slurp.
  • Do not add a firstmate bootstrap gh version floor that would block the fleet on distro gh; firstmate's own GitHub helpers already work on 2.45.0.
  • This is unrelated to pipeline agent selection / any codex switch.

What Changed

  • Make GitHub CLI test doubles reject unsupported gh api --slurp and gh pr checks --json usage.
  • Add regression coverage proving PR recording, polling, and merging continue to work with the GitHub CLI 2.45-compatible command surface.

Risk Assessment

✅ Low: The change is limited to executable regression coverage for Firstmate’s GitHub helper paths, correctly rejects the unsupported gh 2.45.0 flags, and accompanies a precise upstream report for the actual no-mistakes defect.

Testing

No earlier baseline commands were supplied; both targeted PR-forge test scripts passed, including end-to-end PR registration, open/merged polling, and merge under a simulated gh 2.45 CLI that rejects the unsupported flags, with the reviewer-visible transcript captured as evidence.

Evidence: GitHub CLI 2.45 compatibility behavior transcript

Source: GitHub CLI 2.45 compatibility behavior transcript

FM_TEST_BEGIN 2026-09-02T23:18:18Z tests/fm-pr-check-security.test.sh family=pr-forge expected_gate_skip=none
ok - PR record, poll, and merge still work when gh rejects --slurp and pr checks --json
ok - raw-byte parser accepts canonical URLs and rejects the complete adversarial matrix
ok - GitLab merge requests are followed on any instance and never wake falsely
ok - validated merged polls notify once and retire before the next watcher cycle
ok - a repeat identical merged poll for an already-notified task is absorbed, never queued as a main-blocking row
ok - a failed upward merge report keeps its poll armed for repair and retry
ok - staged self-merge and poll interleavings are never silent
ok - a merge detected by the poll is reported upward from a secondmate home exactly once
ok - a different merged PR for the same task gets its own first notification
ok - merged poll retirement preserves every persistent secondmate lifecycle artifact
ok - queue, receipt, and every fixed-path removal crash point recover without loss or repeated execution
ok - open/red, closed-unmerged, malformed, and forge errors remain armed until an exact merged transition
ok - replacement, nonterminal, tampered, and custom results receive no deletion authority
ok - queue failure and untrusted receipts preserve canonical poll evidence
ok - GitHub and GitLab exact merged results share one retirement path
ok - PR and teardown entrypoints reject invalid arguments before every side effect
ok - valid direct and merge flows record exact metadata and reject multiline head metadata
ok - rejected metacharacter bytes remain inert at generation and watcher time
ok - static poll is silent except for one merged line and remains watcher-bounded
ok - interrupted atomic preparation cleans private temporaries and publishes nothing
ok - concurrent watchers observe only complete private poll publications
ok - poll publication paths refuse symlinks and directories
ok - live poll and custom-check artifacts require private single-link files
ok - post-rename poll validation faults revoke both names and allow a clean retry
ok - bootstrap does not rewrite unauthenticated checks or emit retired migration diagnostics
ok - watcher signals promptly stop custom checks and clean private state
ok - returned custom check descendants are drained on installed and fallback timeout paths
ok - teardown removes safe poll artifacts and refuses directory-shaped check files without traversal
FM_TEST_END 2026-09-02T23:21:48Z tests/fm-pr-check-security.test.sh exit=0 duration_ms=210221 gate_skip=false
FM_TEST_BEGIN 2026-09-02T23:21:48Z tests/fm-pr-merge.test.sh family=pr-forge expected_gate_skip=none
ok - fm-pr-merge reports exact queue retry flags after a zero-exit false success
ok - fm-pr-merge omits merge-queue retry guidance for a closed GitHub PR
ok - fm-pr-merge aggregates agreeing merge-queue rules
ok - fm-pr-merge reports ambiguity for conflicting merge-queue rules
ok - fm-pr-merge records pr= and pr_head= for a verified GitHub merge
ok - fm-pr-merge records pr= before the forge call can land the merge
ok - fm-pr-merge propagates a real merge failure without silently succeeding
ok - fm-pr-merge refuses a GitHub merge call that leaves the PR open and unqueued
ok - fm-pr-merge keeps PR bookkeeping when it cannot read a successful merge call's outcome
ok - fm-pr-merge refuses with the forge's own output quoted apart from its verdict
ok - fm-pr-merge quotes the forge output when it cannot read the outcome either
ok - fm-pr-merge does not echo back queue flags the caller already used
ok - fm-pr-merge still names retry flags when the caller used a different method
ok - fm-pr-merge names the queue requirement even when its method is unrecognised
ok - fm-pr-merge distinguishes unreadable branch rules from a base with no merge queue
ok - fm-pr-merge says nothing about a merge queue when the base branch has no queue rule
ok - fm-pr-merge says the merge queue was unobservable when only the gh-axi view answered
ok - fm-pr-merge explains an armed auto-merge that landed nothing on a queue-less base
ok - fm-pr-merge never reports auto-merge as armed when the merge command failed
ok - fm-pr-merge claims no acceptance for a failed merge command carrying queue flags
ok - fm-pr-merge falls back to the gh-axi view when gh's read fails
ok - fm-pr-merge names a landed state hiding behind a failed GitHub merge command
ok - fm-pr-merge reaches and verifies the gh-axi merge path without gh
ok - fm-pr-merge preserves bookkeeping when gh is absent and the fallback read fails
ok - fm-pr-merge verifies a genuinely merged GitHub pull request
ok - fm-pr-merge refuses to claim a merge when poll recording fails
ok - fm-pr-merge accepts and accurately reports a GitHub merge-queue entry
ok - fm-pr-merge explains how to retry with the required GitHub merge queue method
ok - fm-pr-merge forwards extra flags to gh-axi pr merge after the -- separator
ok - fm-pr-merge refuses before merging when task meta is missing
ok - fm-pr-merge refuses malformed PR URLs before calling gh-axi
ok - fm-pr-merge refuses unsafe PR URL segments before recording state
ok - fm-pr-merge refuses repo override args before recording state
ok - fm-pr-merge refuses a bundled short-option repo override and forwards other short flags
ok - fm-pr-merge does not add default --squash when the caller passes an explicit merge method
ok - fm-pr-merge respects --method=<value> as an explicit merge method
ok - fm-pr-merge parses a GitHub PR URL into gh-axi number and --repo arguments
ok - fm-pr-merge leaves GitHub extra-arg handling unchanged, including --sha
ok - fm-pr-merge merges a GitLab merge request through glab instead of refusing it
ok - fm-pr-merge takes the GitLab instance from the URL rather than assuming one
ok - fm-pr-merge imposes no merge method on GitLab, leaving the project's own one
ok - fm-pr-merge forwards extra flags to glab mr merge after the -- separator
ok - fm-pr-merge propagates a real glab merge failure without silently succeeding
ok - fm-pr-merge refuses on each GitLab pre-merge condition independently
ok - fm-pr-merge reports every failing GitLab condition, not only the first
ok - fm-pr-merge reports a stale recorded head and verifies the live one
ok - fm-pr-merge refuses an unreadable GitLab merge request state rather than merging blind
ok - fm-pr-merge refuses a GitLab head commit it cannot validate
ok - fm-pr-merge refuses before recording anything when glab or jq is absent
ok - fm-pr-merge refuses a GitLab head override before recording state
ok - a merge a secondmate home performs itself is reported upward exactly once
ok - a locally routed secondmate home reports the landed PR into its parent's own channel
ok - a landed GitLab merge request is reported upward on the same channel
ok - a queued GitLab merge stays silent and leaves confirmation to the armed poll
ok - a refused or failed merge reports no outcome
ok - a GitLab merge refused before the forge call reports no outcome
ok - a merge a main home performs itself leaves one durable wake naming the PR
ok - a queued GitHub merge stays silent and leaves confirmation to the armed poll
ok - distinct merged PRs for one task retain distinct captain-facing wakes
ok - an uncommitted marker retry preserves at least one durable outcome
ok - a secondmate home that cannot report upward says so instead of merging in silence
FM_TEST_END 2026-09-02T23:23:33Z tests/fm-pr-merge.test.sh exit=0 duration_ms=104862 gate_skip=false
FM_TEST_SUMMARY total=2 failed=0 skipped_gate=0 duration_ms=315199
FM_TEST_SUMMARY_FAMILY family=pr-forge count=2 duration_ms=315083 failed=0
FM_TEST_SLOWEST rank=1 script=tests/fm-pr-check-security.test.sh duration_ms=210221
FM_TEST_SLOWEST rank=2 script=tests/fm-pr-merge.test.sh duration_ms=104862

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

✅ **Review** - passed

✅ No issues found.

✅ **Test** - passed

✅ No issues found.

  • Inspected git diff 5fb0ce7628f240f9844f8b4bcd32ecd6155c3778..b3e0827514a9b274be3d3d56fdb113b1f226a94a to identify the behavioral compatibility surface.
  • Ran bin/fm-test-run.sh --per-script-timeout-secs 300 tests/fm-pr-check-security.test.sh tests/fm-pr-merge.test.sh.
  • Verified the evidence transcript was created and the worktree remained clean with git status --short.
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

Ubuntu 24.04 ships gh 2.45.0, which has no gh api --slurp (added in 2.48.0)
and no gh pr checks --json (2.50.0). Firstmate's own PR record, poll, and
merge helpers already use older gh surfaces; the fakes now reject those
later flags so a helper cannot grow the same CI-reader failure no-mistakes
hits on this distro.

The CI monitor that parks green PRs at ask-user lives in no-mistakes
(kunchenguid/no-mistakes#942), not in this repo. Do not upgrade the system
gh package as a firstmate-side workaround: Noble has no newer apt candidate,
and apt reverse-depends lists only optional sugarjar.
@greptile-apps

greptile-apps Bot commented Sep 2, 2026

Copy link
Copy Markdown

Confidence Score: 5/5

The PR appears safe to merge because it changes only test infrastructure and adds focused compatibility coverage without altering runtime behavior.

The updated fakes exercise the intended Ubuntu gh 2.45 command surface, and the new regression path verifies that PR recording, polling, and merging continue to function through supported commands and existing fallbacks.

Reviews (1): Last reviewed commit: "test(pr): keep GitHub helpers working wh..." | Re-trigger Greptile

@att430

att430 commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Withdrawing this - not pursuing upstream contributions. Closing so it does not sit in your review queue. Apologies for the noise.

@att430 att430 closed this Sep 3, 2026
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