Skip to content

fix(hosted-review-sitter): key GitHub conflict work on the live base tip - #123

Merged
zpyoung merged 1 commit into
mainfrom
zpyoung/sitter-stale-base-sha
Oct 19, 2026
Merged

zpyoung merged 1 commit into
mainfrom
zpyoung/sitter-stale-base-sha

Conversation

@zpyoung

@zpyoung zpyoung commented Oct 19, 2026 •

Copy link
Copy Markdown
Owner
Files Added Deleted Net
Test 2 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​149 0 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​149
Prod 1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​4 $\color{#cf222e}{\Huge{\mathbf{−}}}$​3 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​1

ELI5

The PR sitter decides whether a pull request has merge conflicts by comparing it to the base branch. On GitHub it was comparing against an old snapshot of the base branch, so when it tried to fix a conflict there was nothing to merge, the fix "failed", and the sitter got stuck repeating the same complaint forever. It now compares against the base branch as it is right now.

What Changed

Before: on a GitHub PR whose base branch moved after the PR's last push, GitHub reports the PR as conflicting, but the sitter sends its conflict worker to merge the commit the PR was opened against. That merge is a no-op ("Already up to date"), the worker reports failure, and every later tick raises the same "worker never produced a verifiable outcome" deviation. Seen on zpyoung/quirk#52 (recorded base 00d339c, live main 3df5f34).

After: the sitter dispatches conflict resolution against the base branch's current tip, so the worker merges the commit that actually conflicts. Already-wedged sitters recover on their next tick, because the new base gives the attempt a new identity.

Mechanism: the GitHub reader read pullRequest.baseRefOid, which GitHub refreshes only when the PR is pushed, while mergeable is computed against the live tip. It now reads baseRef { target { oid } } in the same GraphQL query. If the base ref can't be read, the base SHA is left empty, which marks the snapshot's evidence incomplete, instead of quietly falling back to the stale commit. That matches the GitLab reader, which already looks up the target branch's tip.

Why

  • Same query, no extra API call, and the base tip is read in the same response as mergeable, so the two can't disagree. A separate REST lookup (the GitLab shape) would cost a request per tick and could race the GraphQL read.
  • The base SHA also feeds update-branch (expected state, merge-parent verification) and the local conflict simulation for UNKNOWN mergeability, and the live tip is the right value for all of them.
  • I didn't make a no-op conflict preparation retryable. Re-running the same (head, base) merge gives the same result every time, so retrying would just spawn agents for nothing. The real fix is giving the attempt the correct base, and once the base moves the attempt gets a new fingerprint anyway. A new decision test pins that behavior.
  • Remote wire compatibility: the snapshot shape is unchanged. Only the value of baseSha on GitHub changes, and it now means the same thing it already meant on GitLab.

Linked Issue

Orca ledger bug-317.

Visual Proof

N/A. This changes background sitter behavior and has no UI.

Testing

  • I manually tested these changes locally

  • Automated tests added/updated, or explained why not below

  • New provider-github-read.test.ts: snapshot base is the live tip, UNKNOWN mergeability is simulated against the live tip, and a missing base ref leaves the base empty and not ready. All three failed before the fix with the stale SHA.

  • New case in decision.test.ts: an unverifiable conflict preparation isn't re-dispatched for the same base, and a moved base gets a fresh dispatch.

  • pnpm test:sandbox over src/main/fork-hosted-review-sitter/ and src/shared/fork-hosted-review-sitter/ (135 tests) green, pnpm typecheck:node clean, check:code-quality:changed clean.

Review

Agent skill upstream boundary

  • Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

All files are under the fork-owned **/fork-hosted-review-sitter/** glob, so no manifest change is needed. SSH: no change, since the GitHub read runs through gh the same way as before.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

GitHub refreshes a pull request's baseRefOid only when the PR is pushed,
but computes mergeability against the base branch's current tip. The sitter
read baseRefOid, so after the base advanced it dispatched conflict
preparation against an old base commit the head already contained. The
worker's merge was a no-op, the attempt settled unverifiable, and because
(head, base) never changed the sitter raised the same deviation every tick.

Read baseRef.target.oid in the same GraphQL query instead, matching the
GitLab reader, which already resolves the target branch tip. An unreadable
base ref now leaves baseSha empty (evidence incomplete) rather than falling
back to the stale commit.

A no-op conflict preparation stays non-retryable for the same (head, base):
rerunning that merge is deterministic, and a moved base already produces a
fresh attempt fingerprint.
Copilot AI balanced review requested due to automatic review settings October 19, 2026 16:44

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your trial has ended. Reactivate Greptile to resume code reviews.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@zpyoung
zpyoung merged commit 6b096e3 into main Oct 19, 2026
31 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.

2 participants