Measure auto-rebase behind-ness with git, not mergeStateStatus (CROW-944)

[dhilgaertner]

Closes #944

The bug

Auto-rebase decided "is this branch behind its base?" from one GitHub field, mergeStateStatus == "BEHIND" (IssueTracker.swift:3599). That field is single-valued — it reports the highest-priority reason the merge button isn't green, not a set of flags — so a PR that is behind base and anything else reports the other value:

Also true	mergeStateStatus	Behind-ness visible?
Required review pending	BLOCKED	✗
Merge conflict	DIRTY	✗ (caught by the CONFLICTING clause, by luck)
Draft	DRAFT	✗
Base moved, GitHub mid-recompute	UNKNOWN	✗

BLOCKED is the expensive one: it is the normal state of a PR waiting on its reviewer — exactly the window in which a busy base drifts ahead. On a repo with strict_required_status_checks_policy the PR then stalled short of merge until a human pressed Update branch, and the rebase Crow could have done for free during review was serialized behind approval instead.

The fix

shouldAttemptAutoRebase becomes a candidate filter (OPEN and not CLEAN); git answers the real question.

• GitManager.behindBase — one fetch + rev-list --count origin/<branch>..origin/<base>. It compares the two remote refs, not HEAD: behind-ness is a property of the PR head, and a HEAD-based probe would read a locally-rebased-but-unpushed branch as up to date and swallow the out-of-sync-ahead deferral that tells you to push. Three-valued (.upToDate / .behind(n) / .unknown) so a fetch flake falls through to a full attempt rather than silently skipping.
• It runs before prHasCrowAuthoredCommit, so the widened candidate set costs a git fetch per head state rather than a provider API call.
• RebaseOutcome.alreadyUpToDate, checked after the fast-forward reconcile. Without it a no-op rebase (exit 0) plus a no-op force-push ("Everything up-to-date", exit 0) returned .rebasedAndPushed and fired a phantom "Branch rebased" notification.

The trap this created, and how it's handled

Widening + the per-head latch means a PR probed while merely BLOCKED-and-not-yet-behind burns its one attempt — and the base moving afterwards is invisible in headRefOid, so it would never be looked at again. Worse than the original bug. autoRebaseUpToDateHeads re-arms the latch when GitHub's view of that same head changes, and only then, so a persistent git/GitHub disagreement can't hot-loop.

Three fixes around it

A wedged branch now says so. out-of-sync-diverged backed off at a 15-minute cap forever, in silence — 64 such lines in my crowd-automation.log. After 5 consecutive deferrals (where autoRebaseDeferralBackoff saturates; a test pins the two together) it publishes AutoRebaseState(.blocked) and fires a new autoRebaseStuck notification. Escalating is not giving up — Crow keeps retrying at the cap, so a human fix lands next cycle. It deliberately does not self-heal a diverged worktree (by definition that holds commits origin lacks; a reset would destroy them) and does not hand off to the agent the way the conflict path does, for the same reason.

attemptUpdateBranch stops latching and lying. Its no-backend and no-trailer guards returned above the defer, so the PR stayed in autoMergeInFlight for the process lifetime. Worse than "silent": nothing wrote autoMergePermanentSkips, so evaluateAutoMerge's guard fell back to .inFlight — the UI said "Crow is working on this PR's auto-merge right now." Forever. The defer now covers every path; suppression is autoUpdateBranchAttempted's job, which returns before any dispatch, so this costs no extra backend calls. A failed gh pr update-branch gets bounded retries rather than latching until a head commit that a failed update is precisely what didn't change, and both maps are now pruned (autoUpdateBranchAttempted never was).

The precedence continue yields only while auto-merge can still act. It was unconditional, so once auto-merge spent its one-shot per-head attempt and gave up, nobody fixed the branch. The autoMergeInFlight disjunct is load-bearing: the key is inserted before the async attempt and applyAutoMerge runs synchronously right before applyAutoRebase in the same poll, so !contains alone is already false in the very poll auto-merge dispatched.

Visibility

auto_rebase_state rides list-sessions-live beside auto_merge_state, rendered as a ⟲ chip (orange waiting / red stuck), inserted before the ⛙ auto-merge chip — chronological, and it keeps autoIco (last icon) meaning "auto-merge" for every existing #888 assertion. The justification is sharper than auto-merge's: prStatusJSON never ships mergeStateStatus, so a wedged branch rendered as a fully green pill.

Decisions stated in the code

• Drafts stay eligible, as since CROW-318 — a rebase only rewrites the session's own branch and can never merge (shouldAttemptAutoMerge keeps its own draft guard). DRAFT masked behind-ness permanently, so this is where the change bites most.
• Every open GitLab MR becomes a candidate — that backend never populates mergeStateStatus. Wanted (MRs do fall behind, nothing else handles them), bounded to one probe per head, but a deliberate scope expansion rather than a side effect.
• shouldUpdateBranchBeforeMerge is NOT widened. So a crow:merge PR that is BLOCKED-but-behind now routes to auto-rebase (force-push) rather than auto-merge (merge commit). On a repo with "dismiss stale reviews on push", that dismisses approvals. Flagging it rather than letting someone discover it via a dismissed review.

Out of scope: gh pr update-branch --rebase, and inverting precedence wholesale.

Testing

make test green except CrowDaemon's 2 pre-existing app.js-source-shape failures (refreshTerminalsRebindsTheActiveTerminalToTheFreshRow, bootCatchResetsTheHistory) — verified identical with app.js stashed to HEAD. make parity passes at 12 events; make docs regenerated and committed.

⚠️ CrowEngine is not in CI's Linux allow-list and row.test.js is omitted from test:ci (10 pre-existing CROW-802 failures), so most of the new tests won't run in CI. Verified locally:

• CrowGit 27 tests (the CI-covered ones): alreadyUpToDate, behindBase × 4 incl. the remote-vs-HEAD distinction and .unknown-not-.upToDate on a bad ref.
• CrowEngine 664 tests. shouldAttemptAutoRebase against BLOCKED/DIRTY/DRAFT/UNKNOWN/UNSTABLE/HAS_HOOKS/CLEAN; rejectsUnknownState inverted with a comment explaining why rather than deleted; escalation threshold pinned to the backoff curve; verdict permanent ⟺ blocked invariant. New IssueTrackerBehindPRHandoffTests drives real dispatch bookkeeping for the precedence hand-off, both autoMergeInFlight leak paths, pruning, and the up-to-date re-check.
• row.test.js: 57 passed / 10 failed, baseline was 43 / 10 — 14 new assertions, same 10 pre-existing failures. (Found and fixed a real bug doing this: the harness's own T.prStatusInline shim dropped the new 4th argument.)

Not yet exercised end-to-end against a live BLOCKED-and-behind PR — that's the one acceptance criterion no test reaches.

🤖 Generated with Claude Code

[dgershman]

Code & Security Review

Critical Issues (if any)

None.

Security Review

Strengths:

• Git operations use Process argument arrays (no shell interpolation), so branch/base ref names from the provider are not a command-injection vector (GitManager.swift:335-341).
• Auto-rebase only force-pushes after prHasCrowAuthoredCommit and excludes review/manager sessions (IssueTracker.swift:3836-3838, 4035-4038).
• rebaseOntoBase refuses dirty worktrees, verifies HEAD branch, fast-forwards only when strictly behind remote, and pushes with --force-with-lease (GitManager.swift:171-289).
• UI surfaces daemon message via textContent / title attributes, not innerHTML (app.js:1192-1196, 2748-2756); git stderr in verdicts is truncated to 200 chars (IssueTracker.swift:358).

Concerns:

• No material security issues identified. Residual risk is operational: force-push on Crow-authored branches can dismiss approvals on repos with “dismiss stale reviews on push” — documented in the PR and an accepted tradeoff vs. shouldUpdateBranchBeforeMerge staying BEHIND-only.

Code Quality

• Yellow — re-arm signal may miss pure base drift: autoRebaseUpToDateHeads re-arms the per-head latch only when mergeStateStatus changes (IssueTracker.swift:3912-3915). GitHub keeps mergeStateStatus at BLOCKED while a review is pending even when the base moves ahead, so a PR probed while BLOCKED and git-up-to-date can burn its one latch, then miss a later base drift with no headRefOid change. The test at IssueTrackerBehindPRHandoffTests.swift:204-219 covers BLOCKED → BEHIND flips but not stable-BLOCKED base drift. Consider a secondary re-arm keyed on fetched origin/<base> SHA or a bounded periodic re-probe.
• Green: behindBase correctly compares remote refs (not HEAD), returns .unknown on failure rather than .upToDate, and alreadyUpToDate prevents phantom notifications (GitManager.swift:100-135, 232-252).
• Green: attemptUpdateBranch in-flight leak fix and bounded retry mirror the rebase path (IssueTracker.swift:3663-3718); precedence hand-off tests exercise real dispatch bookkeeping (IssueTrackerBehindPRHandoffTests.swift).
• Green: AutoRebaseState UI chip, RPC payload, and autoRebaseStuck notification close the visibility gap for wedged branches (app.js:1982-2006, RPCHandlers.swift:977-983, CrowDaemon.swift:685-688).
• Green: swift build succeeds; CrowGit (27 tests) and IssueTrackerAutoRebaseTests (33 tests) pass locally.

Summary Table

Color	Meaning	Verdict effect
Red	Must fix	Request changes
Yellow	Should fix	Request changes
Green	Consider	Approve allowed

Recommendation: Request Changes — driven by [0 Red, 1 Yellow, 4 Green] findings.

---

🐦‍⬛ Reviewed by Crow via Cursor

[dhilgaertner]

Thanks — the Yellow finding is correct, and tracing it, it's worse than "may miss": that re-arm could never fire for the case CROW-944 is about.

mergeStateStatus is single-valued, which is the whole premise of the ticket — BLOCKED outranks BEHIND. So when the base drifts under a review-pending PR the field doesn't flip to BEHIND, it just stays BLOCKED. headRefOid doesn't move either. A status-change re-arm therefore only fires when BLOCKED finally clears, i.e. once the PR is approved — precisely the serialization this ticket exists to remove. My fix only ever helped PRs that were already behind the first time they were seen, which is the minority path.

Fixed in 25cb27b.

I went with your second suggestion (bounded periodic re-probe) rather than the origin/<base> SHA, because the SHA is circular here: you need a git fetch to learn the base SHA, and the fetch is the cost the latch exists to avoid. PRRecord carries baseRefName but no base OID, so there's no free signal from the poll either.

AutoRebaseUpToDateHead now records a recheckAt alongside the status, and shouldRecheckUpToDateHead re-arms on either signal:

nonisolated static func shouldRecheckUpToDateHead(
    _ seen: AutoRebaseUpToDateHead, currentStatus: String, now: Date
) -> Bool {
    seen.mergeStateStatus != currentStatus || now >= seen.recheckAt
}

Interval is 900s, matching autoRebaseDeferralMaxDelay — four local git fetches an hour per open PR with a worktree, comfortably inside how long CI takes so the detection lag never becomes the critical path, and still bounded enough that a persistent git/GitHub disagreement re-probes on the interval rather than every poll (the hot-loop the status gate was guarding against).

Two tests: aStableBlockedHeadIsRecheckedOnTheIntervalSoBaseDriftIsCaught for the exact scenario you described, and recheckPolicyIsGatedByBothStatusAndClock for the pure policy. I confirmed both fail with the clock condition removed, so they're real regression cover rather than tests that pass either way. docs/automation.md updated — the old prose described the insufficient behavior.

make test is green except CrowDaemon's 2 pre-existing app.js-source-shape failures (verified identical with app.js stashed to HEAD); make parity passes.

[dgershman]

Code & Security Review

Critical Issues (if any)

None.

Security Review

Strengths:

• GitManager.run passes argv arrays to Process (no shell interpolation), and behindBase/rebaseOntoBase inherit the existing --force-with-lease + dirty-tree guards.
• BaseBehindState.unknown correctly falls through to a full attempt instead of silently skipping work — avoids a class of "optimization drops real rebases" bugs.
• AutoRebaseVerdict.gaveUp truncates git stderr to 200 chars before it reaches RPC/UI; prStatusPart renders via textContent/title, not innerHTML (app.js:2748-2756, IssueTracker.swift:387-396).
• Crow-authorship gate still applies before any rewrite; review/manager sessions remain excluded.

Concerns:

• None blocking. Operational note (not a vulnerability): crow:merge PRs that are BLOCKED but git-behind now route through auto-rebase (force-push) rather than gh pr update-branch, which can dismiss approvals on repos with "dismiss stale reviews on push." This is documented and intentional — worth awareness for operators.

Code Quality

• Root-cause fix is sound: widen shouldAttemptAutoRebase to a cheap candidate filter, then let GitManager.behindBase answer the real question using remote refs (not HEAD), with RebaseOutcome.alreadyUpToDate preventing phantom notifications.
• The autoRebaseUpToDateHeads + 900s recheck interval correctly handles base drift invisible in both headRefOid and mergeStateStatus (IssueTracker.swift:3938-3953) — without this, widening the candidate set would be worse than the original bug.
• attemptUpdateBranch defer fix (IssueTracker.swift:3706-3751) closes a real autoMergeInFlight leak; bounded retry on failed update-branch mirrors the rebase path.
• Precedence hand-off only yields while auto-merge can still act (IssueTracker.swift:3929-3935), with thorough coverage in IssueTrackerBehindPRHandoffTests.
• UI/RPC wiring (AutoRebaseState, list-sessions-live, ⟲ chip) mirrors AutoMergeState conventions; stuck escalation + autoRebaseStuck notification address the prior silent 15-minute log loop.
• Tests are extensive: CrowGit behindBase/alreadyUpToDate (13/13), IssueTrackerAutoRebaseTests (33/33), IssueTrackerBehindPRHandoffTests (14/14), AutoRebaseStatePayloadTests (5/5). swift build clean.

Summary Table

Color	Meaning	Verdict effect
Red	Must fix	Request changes
Yellow	Should fix	Request changes
Green	Consider	Approve allowed

#	Finding	Color
1	Remote-ref behindBase probe + three-valued result prevents both false skips and false positives	Green
2	autoRebaseUpToDateHeads time-based re-arm closes the widened-candidate latch trap	Green
3	attemptUpdateBranch in-flight leak + bounded failure retries	Green
4	crow:merge + BLOCKED-but-behind may force-push (dismiss reviews on some repos) — documented trade-off	Green
5	New CrowEngine/web tests won't run in Linux CI (pre-existing CROW-802); verified locally	Green

Recommendation: Approve — driven by [0 Red, 0 Yellow, 5 Green] findings.

---

🐦‍⬛ Reviewed by Crow via Cursor