Skip to content

fix(ship): merge-train gates on CI — fetch check-runs in sync [ROA-274]#107

Merged
prellr merged 1 commit into
mainfrom
fix/merge-train-ci-gating
May 16, 2026
Merged

fix(ship): merge-train gates on CI — fetch check-runs in sync [ROA-274]#107
prellr merged 1 commit into
mainfrom
fix/merge-train-ci-gating

Conversation

@prellr
Copy link
Copy Markdown
Owner

@prellr prellr commented May 16, 2026

Summary

Closes ROA-274. The Ship Hub merge-train never gated on CI — it merged PR #100 with red frontend lint, breaking main for ~12h.

Root cause: upsertPR (sync) hardcoded ci_status="" (a never-finished "Phase 1" TODO); ci_status was only ever set by webhooks, which don't deliver on this self-hosted fork. releaseEligibilityReason only blocks non-empty, non-"success" — so empty → eligible → red PRs merge.

Fix — make sync self-sufficient

  • github.GetCIStatus: one combined rollup of the legacy commit-status API + the check-runs API. GitHub Actions reports as check-runs, which the legacy /status endpoint is blind to (why ci_status was always "" here). Failure-dominant: failure > pending > success > "".
  • upsertPR: resolves ci_status for open PRs only (closed/merged skip — bounds the rate-limit cost the original TODO worried about). Best-effort: a fetch error logs + leaves ""; next sync corrects it; one PR's lookup never fails the whole sync.
  • releaseEligibilityReason unchanged: once ci_status is correct, the existing != "" && != "success" already blocks pending/failure. CI-less repos (no statuses, no check-runs → "") stay mergeable by design (RoastConsole / saf-roast-macos has zero workflows — must not false-block). Fail-closed behavior emerges from correct data; no logic change needed there.

Test plan

  • go build ./... + go vet clean
  • ship + github suites pass
  • New: TestGetCIStatus_* (5 rollup cases), TestUpsertPR_{OpenPR_Persists,MergedPR_Skips,CIFetchError_Tolerates}, TestReleaseEligibilityReason_CIGate (failure/success/empty)
  • Pre-existing handler failures (cross-UTC-midnight flakes + a squad-migration gap from the batch-sync) verified byte-identical on a clean tree — zero new failures
  • CI green
  • Post-merge: a PR with a failing Actions check is now blocked by the merge-train (the feat: add ship repo sections [ROA-264] #100 scenario can't recur)

Reuses the existing gh.CheckRun type. release.go deliberately untouched.

🤖 Generated with Claude Code

The merge-train never gated on CI. upsertPR (sync) hardcoded
ci_status="" ("Phase 1 leaves it blank" TODO that never landed);
ci_status was only ever set by webhooks, which don't deliver on this
self-hosted fork. releaseEligibilityReason only blocks non-empty
non-"success", so empty → eligible → red PRs merged (e.g. PR #100
broke main lint for 12h).

Fix — make sync self-sufficient:
- github.GetCIStatus: combined rollup of the LEGACY commit-status API
  + the check-runs API (GitHub Actions reports as check-runs, which
  the legacy /status endpoint is blind to). Failure-dominant: failure
  > pending > success > "". CI-less repo → "" (stays mergeable by
  design — RoastConsole/saf-roast-macos has zero workflows).
- upsertPR: resolve ci_status for OPEN PRs only (closed/merged skip —
  bounds the rate-limit cost the original TODO worried about).
  Best-effort: a fetch error logs + leaves "" and the next sync
  corrects it; one PR's CI lookup never fails the whole sync.
- releaseEligibilityReason UNCHANGED: once ci_status is correctly
  populated, the existing != "" && != "success" check already blocks
  pending/failure and leaves CI-less repos mergeable. Fail-closed
  behavior emerges from correct data; no logic change needed.

Reuses the existing gh.CheckRun type (webhook.go). Regression tests:
GetCIStatus rollup (5 cases), upsertPR persists/skips/error-tolerates
ci_status, releaseEligibilityReason CI gate (failure/success/empty).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@prellr prellr merged commit e3c7593 into main May 16, 2026
2 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