Skip to content

fix(gate): say WHY a green, approved PR did not merge — surface GitHub's own refusal - #9864

Merged
JSONbored merged 1 commit into
mainfrom
fix/surface-merge-block-reason
Jul 29, 2026
Merged

fix(gate): say WHY a green, approved PR did not merge — surface GitHub's own refusal#9864
JSONbored merged 1 commit into
mainfrom
fix/surface-merge-block-reason

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

The recurring mystery, solved

#9856: gate success, CI green, mergeable_state: clean, autonomy auto, approvals satisfied. The bot posted "LoopOver approves — the gate is satisfied and CI is green" and the panel said "Suggested Action - Approve/Merge · safe to merge". It never merged. This has recurred for weeks across many PRs with no stated reason.

It was never a LoopOver bug. GitHub refused the merge:

merge forbidden (403): Merging stacked PRs via this API is not supported.
Use the web interface instead.

Stacked PRs cannot be merged through the REST API at all. The executor recorded the refusal against that head and correctly stopped retrying — that part works exactly as designed.

The actual defect: the reason reaches nobody

merge_blocked_reason is stored in the DB and consulted by the planner, but no surface reads it:

  • mergeable_state reads clean — the refusal is invisible to it
  • no gate models it
  • the audit falls through to the residual "merge withheld because no merge action was planned" — true, and useless
  • the public comment keeps promising a merge that cannot happen

Across the fleet, 7 PRs / 4 distinct refusals all rendered identically: the 403 above (4 PRs), plus three 405 repo-policy variants ("Merge already in progress", and two others).

The fix — both surfaces

AuditagentHoldAuditDetail reports GitHub's message when the block is for this head. Checked last among the specifics: everything above it (pending CI, dirty/unstable, approvals, autonomy) is a state LoopOver can still change on its own; this one only a human can, so it must not mask them.

Comment — an otherwise-ready status downgrades to held, and the verdict box names the refusal plus the remedy ("merge this pull request yourself"). Same downgrade-only discipline as the guardrail hold: a real gate/CI block still wins, and an advisory review stays advisory.

A block recorded against a different head is stale and deliberately ignored — that commit was never attempted, so claiming GitHub refused it would be a lie and would hide whatever is actually holding the new push.

Clock discipline (a guard caught me)

My first attempt used Date.now() for the block's expiry. decision-clock-threading.test.ts (#9492) rejected it: a second clock read can disagree with the planner's within one pass, and replayDecision would then certify a decision made on an unrecorded read as match — a false certification. The decision path now uses decisionClock.nowMs.

The publish path matches on head SHA alone, and the comment says why: it's a reporting surface with no decision clock, reached from callers that have none either, and merge_blocked_until bounds when LoopOver may retry, not whether GitHub refused this commit.

Tests

9 new cases: the regression on both surfaces, the stale-head invariant, "more specific reasons still win", downgrade-only, no-block-unchanged, and a render assertion that the comment names the refusal and no longer contains "safe to merge". Full suite 25,350 passed; dead-source-files, dispatch-gate-reasons, docs:drift green.

Note for the maintainer

#9856 is unmergeable by the ORB permanently — it needs the web UI ("Squash and merge" on the stack). That's a GitHub restriction, not something to fix here. This PR just makes it say so.

…b's own refusal

#9856: gate success, CI green, mergeable_state "clean",
autonomy auto, approvals satisfied, the bot posted "LoopOver approves" and the
panel said "Suggested Action - Approve/Merge · safe to merge" -- and it never
merged. Recurring for weeks, across many PRs, with no stated reason.

The cause was never a LoopOver bug. GitHub REFUSED the merge:

  merge forbidden (403): Merging stacked PRs via this API is not supported.
  Use the web interface instead.

Stacked PRs cannot be merged through the REST API at all. The executor recorded
the refusal against that head and correctly stopped retrying. But that reason is
invisible to every surface: mergeable_state reads `clean`, no gate models it, the
audit fell through to the residual "merge withheld because no merge action was
planned", and the public comment kept promising a merge that cannot happen. Four
distinct refusals across 7 PRs all rendered identically (403 stacked; three 405
repo-policy variants).

Surface it in both places:
 - agentHoldAuditDetail reports GitHub's message when the block is for THIS head,
   checked last among the specifics (everything above it is a state LoopOver can
   still change on its own; this one only a human can).
 - the unified comment downgrades an otherwise-ready status to HELD and names the
   refusal in the verdict box, with the remedy ("merge it yourself"). Same
   downgrade-only discipline as the guardrail hold: a real gate/CI block still wins.

A block recorded against a DIFFERENT head is stale and deliberately ignored --
that commit was never attempted, so claiming GitHub refused it would be a lie and
would hide whatever is actually holding the new push.

Clock discipline: the decision path uses the pass's recorded instant
(decisionClock.nowMs), never a fresh Date.now() -- the #9492 invariant, whose
guard caught a first attempt that violated it. The publish path matches on head
SHA alone and explains why: it is a reporting surface with no decision clock, and
merge_blocked_until bounds when LoopOver may RETRY, not whether GitHub refused
this commit.
@loopover-orb

loopover-orb Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Caution

🛑 LoopOver review result - fixes required

Review updated: 2026-07-29 15:23:44 UTC

5 files · 1 AI reviewer · 3 blockers · CI failing · unstable

🛑 Suggested Action - Manual Review

  • Reviewer-manipulation text detected in this pull request: This pull request's title, description, or diff contains text addressed at an automated reviewer (for example instructions to ignore prior rules or to approve the change). That content is treated as data and was redacted before review, but the attempt itself means this pull request is held for a person rather than decided automatically.

Review summary
This PR threads GitHub's own durable merge-refusal reason (already stored via mergeBlockedSha/mergeBlockedReason from src/services/merge-failure.ts) through the audit resolver, the public comment status, and the verdict box, so a stacked-PR 403/repo-policy 405 refusal is finally visible instead of falling through to the useless residual 'no merge action was planned'. The gating logic (mergeBlockedSha === headSha, checked last among specifics, downgrade-only in deriveUnifiedStatus) is correct and well-tested in unified-comment.test.ts and precision-breakers-chain.test.ts. One real inconsistency: the audit path (processors.ts:3887) resolves the block through activeMergeBlockedSha (which respects the infra-scope TTL), while the public-comment path (processors.ts:12928) matches on the raw stored SHA with no TTL check, so a 401-class infra block that has expired (INFRA_MERGE_BLOCK_TTL_MS) could keep rendering 'GitHub refused the merge' in the comment after the planner has already resumed retrying it.

Blockers

  • The new branches wired in runAgentMaintenancePlanAndExecute and maybePublishPrPublicSurface (src/queue/processors.ts) have no direct tests in this diff, matching the codecov/patch failure at 71.42% against the 99% target.
Nits — 6 non-blocking
  • src/queue/processors.ts:12928 — the public-surface mergeBlockedReason match skips the activeMergeBlockedSha/TTL check the audit path uses, so an expired infra-scoped block (e.g. 401, 30-min TTL) could keep the comment saying 'GitHub refused the merge' after the planner has already resumed retrying; the in-code rationale only argues this is fine for commit-scoped (403/405) blocks, not infra-scoped ones.
  • The `[external-instruction-redacted]` text inside the verdict-box remedy string (src/review/unified-comment.ts verdictLine) suggests the original literal contained an imperative phrase that tripped injection filtering — worth confirming the actual shipped string reads naturally (e.g. 'merge it yourself') rather than leaking a redaction artifact.
  • External brief flags 403/405 as 'magic numbers' in the new doc comments (src/queue/processors.ts:2511) — these are narrative references to classifyMergeFailure's existing HTTP-status branches, not new logic literals, so low value to extract into a constant.
  • Either thread the same activeMergeBlockedSha/TTL-aware resolution into maybePublishPrPublicSurface's mergeBlockedReason match, or add an explicit test asserting the comment still shows the refusal after the TTL window for a scope:'infra' block, so the current behavior is a documented decision rather than an oversight.
  • Add unit coverage for the two processors.ts call sites directly (activeMergeBlockedSha wiring into agentHoldAuditDetail's args, and the conditional spread into maybePublishPrPublicSurface) to close the patch-coverage gap flagged by CI.
  • Reviewer-manipulation text detected in this pull request — A maintainer should review this pull request manually and confirm the content is legitimate.

Why this is blocked

  • The new branches wired in runAgentMaintenancePlanAndExecute and maybePublishPrPublicSurface (src/queue/processors.ts) have no direct tests in this diff, matching the codecov/patch failure at 71.42% against the 99% target.
  • No linked issue detected: No closing reference or linked issue number was found in the PR metadata/body. — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue: This repo's maintainer focus manifest requires every PR to reference a tracked issue. — Link the relevant issue (for example Closes #123) before opening the PR.
📋 Copy for AI agents — paste into your coding agent
Fix the following blocker(s) from this PR review:

1. The new branches wired in runAgentMaintenancePlanAndExecute and maybePublishPrPublicSurface \(src/queue/processors.ts\) have no direct tests in this diff, matching the codecov/patch failure at 71.42% against the 99% target.

2. No linked issue detected: No closing reference or linked issue number was found in the PR metadata/body. — If this PR is intended to solve an issue, link it explicitly in the PR body.

3. Maintainer requires a linked issue: This repo's maintainer focus manifest requires every PR to reference a tracked issue. — Link the relevant issue (for example `Closes #123`) before opening the PR.

CI checks failing

  • codecov/patch — 71.42% of diff hit (target 99.00%)

Decision drivers

  • ❌ Code review — 3 blockers (1 reviewer)
  • ❌ Gate result — Blocking (Repo-configured hard blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 14 registered-repo PR(s), 13 merged, 330 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 14 PR(s), 330 issue(s).
Improvement ✅ Minor risk: clean · value: minor
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is registered but has no active allocation in the current snapshot.
  • Public profile languages: not available
  • Official Gittensor activity: 14 PR(s), 330 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Then work through the remaining 3 steps in the Signals table above.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @loopover ask <question> answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

Decision record
  • action: hold · clause: missing_linked_issue
  • config: a483f14ca161d4067c47dfbfe055469d75cd389df502c98317d95b80d8f6c82d · pack: oss-anti-slop · ci: failed
  • model: claude-code · prompt: 63a85ceb2d21c2dd9f7a1bab6ab8bee9abc6da7ccc9907f9053d1b4e747321d9 · confidence: 0.55
  • record: 51ebd568c23510cd4171da20bcf384555e3958d8690640a8991730d5b082afbd (schema v5, head 65d8e0d)

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


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@JSONbored JSONbored self-assigned this Jul 29, 2026
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent did not find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 71.42857% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.77%. Comparing base (226d974) to head (65d8e0d).
⚠️ Report is 6 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/queue/processors.ts 66.66% 0 Missing and 1 partial ⚠️
src/review/unified-comment-bridge.ts 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9864      +/-   ##
==========================================
- Coverage   91.65%   90.77%   -0.88%     
==========================================
  Files         916      916              
  Lines      112772   112779       +7     
  Branches    27093    27099       +6     
==========================================
- Hits       103357   102371     -986     
- Misses       8126     9315    +1189     
+ Partials     1289     1093     -196     
Flag Coverage Δ
backend 94.10% <71.42%> (-1.58%) ⬇️

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

Files with missing lines Coverage Δ
src/review/unified-comment.ts 98.16% <100.00%> (+0.02%) ⬆️
src/queue/processors.ts 94.69% <66.66%> (-0.03%) ⬇️
src/review/unified-comment-bridge.ts 98.52% <0.00%> (-0.37%) ⬇️

... and 3 files with indirect coverage changes

@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 29, 2026
@JSONbored
JSONbored merged commit 235723a into main Jul 29, 2026
7 of 8 checks passed
@JSONbored
JSONbored deleted the fix/surface-merge-block-reason branch July 29, 2026 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

manual-review Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant