Skip to content

fix(github): retry the transient 405 'Merge already in progress' race (#5003) - #5042

Merged
JSONbored merged 1 commit into
mainfrom
fix/5003-merge-already-in-progress-retry
Jul 11, 2026
Merged

fix(github): retry the transient 405 'Merge already in progress' race (#5003)#5042
JSONbored merged 1 commit into
mainfrom
fix/5003-merge-already-in-progress-retry

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

Requirement #1 (Merge already in progress): classifyMergeFailure (src/services/merge-failure.ts) already has a documented precedent for exactly this shape — isBaseBranchMovedMessage special-cases the benign "Base branch was modified" 405 as retryable (terminal: false), while every other 405 falls through to terminal. "Merge already in progress" is the same class of race: another merge request for the same PR is already being processed by GitHub — not a policy rejection, resolves on its own once the in-flight merge settles. Added isMergeAlreadyInProgressMessage, matching the existing pattern exactly (same file, same shape, same MERGE_RETRY_CAP-bounded retry via handleMergeFailure in agent-action-executor.ts, unchanged).

Requirement #2 (rate-limit during assignee retry): confirmed ensurePullRequestAssignee's octokit client already threads githubRateLimitAdmissionKeyForInstallation(installationId) through makeInstallationOctokit — the same shared rate-limit-budget convention every other src/github/ call site uses. The one captured GITTENSORY-1M event is from 2026-07-09T21:41:51Z, on release gittensory-orb@0.4.0-beta.13after #4167's 403-handling fix (2026-07-08) had already deployed. Seer's own root-cause read ("high queue load causes excessive concurrent GitHub API calls per installation, exhausting the rate limit") matches: this is a single, isolated rate-limit event from ordinary contention during a burst, not a new bug — the call already follows the standard rate-limit-aware conventions, and GitHub's own limiter self-resets within the hour. No code change proposed for this half; documented in the issue.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked a currently open issue this PR resolves (Closes #5003).

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck
  • npm run test:coverage locally
  • npm run test:workers
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm run ui:openapi:check
  • npm run ui:lint
  • npm run ui:typecheck
  • npm run ui:build
  • npm audit --audit-level=moderate
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries

If any required check was skipped, explain why:

  • test:coverage (full unsharded): not run end-to-end — ran scoped vitest --coverage for test/unit/merge-failure.test.ts and confirmed via lcov that src/services/merge-failure.ts is at 100% line and branch coverage (LF:18 LH:18, BRF:24 BRH:24). Also ran test/unit/agent-action-executor.test.ts (176 tests combined) to confirm the executor's merge-retry wiring is unaffected.
  • actionlint / test:workers / build:mcp / test:mcp-pack / ui:openapi:check / ui:lint / ui:typecheck / ui:build / npm audit: not run — this change touches only src/services/merge-failure.ts (a pure classification function, no new API/schema/binding/dependency surface) and its tests; no workflow, MCP, UI, or dependency-manifest surface changed.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests. (N/A — no auth surface touched; the existing terminal-405 negative test is preserved and still passes.)
  • API/OpenAPI/MCP behavior is updated and tested where needed. (N/A.)
  • UI changes use live API data or real empty/error/loading states. (N/A.)
  • Visible UI changes include a UI Evidence section. (N/A.)
  • Public docs/changelogs are updated where needed. (N/A — internal engine behavior; changelog is not edited in a normal PR.)

Notes

Part of a batch of 13 bug fixes filed from a Sentry-issue triage this session (#4994#5006). This is #10 by priority.

…#5003)

classifyMergeFailure treated every 405 except the known "Base branch
was modified" TOCTOU race as terminal, holding the PR for a human.
"Merge already in progress" (GITTENSORY-1K, 2 Sentry events) is the
same class of benign race -- another merge request for the same PR is
already being processed by GitHub -- not a policy rejection, so it
resolves the same way: retry against the settled state instead of
holding.
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jul 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.28%. Comparing base (afc7993) to head (867df90).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5042   +/-   ##
=======================================
  Coverage   94.28%   94.28%           
=======================================
  Files         462      462           
  Lines       39365    39367    +2     
  Branches    14362    14363    +1     
=======================================
+ Hits        37117    37119    +2     
  Misses       1593     1593           
  Partials      655      655           
Flag Coverage Δ
shard-1 46.66% <0.00%> (-0.03%) ⬇️
shard-2 33.25% <0.00%> (-0.12%) ⬇️
shard-3 31.27% <100.00%> (-0.18%) ⬇️
shard-4 32.52% <0.00%> (-0.27%) ⬇️
shard-5 34.40% <0.00%> (+0.93%) ⬆️
shard-6 45.29% <0.00%> (+0.21%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/services/merge-failure.ts 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 11, 2026
@loopover-orb

loopover-orb Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Important

🟪🟪🟪🟪🟪🟪🟪🟪🟪🟪🟪🟪

🔍 Gittensory is reviewing…

AI analysis is in progress. This comment will update when the review is complete.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed · 🟪 Reviewing

@JSONbored
JSONbored merged commit 34437c9 into main Jul 11, 2026
18 checks passed
@JSONbored
JSONbored deleted the fix/5003-merge-already-in-progress-retry branch July 11, 2026 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Development

Successfully merging this pull request may close these issues.

fix(github): unhandled transient GitHub states in merge/assignee retries (3 Sentry events)

1 participant