fix(backlog): sync PR branch with main before respawning a fix session#163
Merged
Conversation
…a fix session AutoReopenForPRFix respawns an autonomous session whenever ReconcilePRPending detects a CI failure, blocking review, or merge conflict on a pr_pending item, but it only ever told the new session about the failure via a text note — it never actually resynced the branch with main first. A CI failure caused by drift from main (a fix landing on main after the PR's branch was created, or the branch simply going stale) wasted the respawned session's effort diagnosing something a merge would have fixed outright, and left the PR exposed to the same "developed a hard conflict with nobody proactively resyncing it" pattern that produced PR #157. AutoReopenForPRFix now merges main into the worktree behind the currently open PR (found via the most recent completed work ItemSession) before building the fix context: - a clean merge is pushed straight to the PR's branch on origin, so the live PR is resynced even before the new session starts; - a merge conflict is aborted immediately (via `git merge --abort`, leaving the worktree clean) and the conflicting file paths are folded into the fix context so resolving them against main becomes part of the fix; - an already-up-to-date branch is a silent no-op. Adds session/git.MergeMainIntoWorktree (fetch + merge + conflict abort/report, following the existing session/git/ops.go conventions) and BacklogService.syncPRBranchWithMain, wired in as a best-effort step ahead of the existing respawn — any sync failure is logged and swallowed, never blocking the fix spawn. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
✅ Registry ValidationTest Coverage: 8/172 features have
|
Contributor
E2E RPC Latency |
Contributor
Go Benchmarks (Tier 1) |
Contributor
🎬 E2E Feature Demos2 shard(s) recorded feature flows for this PR. recordings shard 1 Demo preview opens directly in browser (single-file HTML). Raw WebM recordings in ZIP. Expires after 30 days. |
Contributor
Frontend Terminal Throughput |
Code review on the branch-sync fix (PR #163) surfaced two MAJOR issues, now fixed, plus test-coverage gaps, now filled: - session/git/ops.go: MergeMainIntoWorktree detected an up-to-date merge by substring-matching git's merge output ("Already up to date"), which is locale- and git-version-fragile (older git prints "Already up-to-date."). Now compares HEAD before/after the merge via the package's existing getHeadCommitSHA helper instead. - backlog_service_triage.go: the push-failure note told the next fix session to "push it before continuing," but that session gets a brand-new worktree (SpawnSessionFromItem always creates one fresh on reopen), not the one the merge landed in — the instruction wasn't actionable from there. The note now names the branch and gives an explicit `git -C <repoPath> push origin <branch>` command against the shared repo checkout, whose branch ref survives worktree cleanup. - Added missing coverage: fetch failure and non-conflict merge failure in session/git/ops_test.go; push failure after a successful merge and a swallowed sync-fetch failure in backlog_service_triage_test.go. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
✅ Registry ValidationTest Coverage: 8/172 features have
|
4 tasks
…-agent-a0878367c2523521d
Contributor
✅ Registry ValidationTest Coverage: 8/172 features have
|
Contributor
📊 Feature E2E CoverageFeature coverage report unavailable
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
AutoReopenForPRFixnow proactively mergesmaininto the PR's branch before respawning a fix session, instead of only describing the CI failure in a text note.Context
ReconcilePRPendingdetects CI failures, blocking reviews, and merge conflicts onpr_pendingitems and callsAutoReopenForPRFixto respawn an autonomous fix session. Previously this only told the respawned session about the failure via a text note — it never verified the branch was actually in sync withmain. For CI failures caused by drift (a fix landing onmainafter the branch was created, or the branch simply going stale) this wasted the respawned session's effort diagnosing something a merge would resolve immediately, and left the PR exposed to the same "drifted from main until it hit a hard conflict" pattern that produced PR #157. The fix makes the sync a programmatic guarantee rather than a prompt instruction, per this codebase's established pattern (seedocs/tasks/backlog-feature-improvement.md).Changes
session/git/ops.go: newMergeMainIntoWorktree(worktreePath, mainBranch)— fetches and mergesmainBranch, reportingUpToDate/Merged/Conflicted. Up-to-date detection comparesHEADbefore/after the merge (not merge-output text matching, which is locale/git-version fragile). On conflict it aborts the merge (git merge --abort) so the worktree is always left clean, and reports the conflicting file paths. Non-conflict merge failures (e.g. uncommitted local changes) and fetch failures are propagated as errors rather than misreported as a result state.server/services/backlog_service_triage.go: newBacklogService.syncPRBranchWithMain, called fromAutoReopenForPRFixbefore building the fix context. Finds the worktree behind the currently open PR (most recent completed workItemSession), mergesmaininto it, and pushes the merge when it brings in new commits. Best-effort — any failure is logged and swallowed, never blocking the fix spawn. The push-failure note gives an explicitgit -C <repoPath> push origin <branch>command against the shared repo checkout (not the worktree, which gets cleaned up once the new fix session is spawned) since worktree cleanup never deletes the underlying branch ref.session/git/ops_test.go(5 tests: up-to-date, clean merge, conflict+abort, fetch failure, non-conflict merge failure) and 7 tests inserver/services/backlog_service_triage_test.gocoveringAutoReopenForPRFix's sync-and-push, push-failure, conflict-folds-into-context, sync-fetch-failure-swallowed, and up-to-date no-op paths end to end.Impact
session/gitand the backlog PR-fix respawn path (server/services/backlog_service_triage.go). Does not touchsession/unfinished/gogitstore/or the VCS widget — those are owned by other in-flight work.Testing
go test ./session/git/...(5MergeMainIntoWorktreetests: up-to-date, clean merge, conflict, fetch failure, non-conflict merge failure)go test ./server/services/...(full package, including 7 newAutoReopenForPRFixsync tests)go test ./session/...(full package)golangci-lint run ./session/git/... ./server/services/...go vet ./session/git/... ./server/services/...,gofmt -lon all changed filesReviewer Notes
syncPRBranchWithMain's worktree-lookup (findMostRecentSessions+GetWorktreeDataBySessionUUID) and the abort-on-conflict behavior inMergeMainIntoWorktree.SpawnSessionFromItem's reopen path always creates a brand-new branch/worktree off the shared repo's local HEAD (buildRevisionTitleappends-rN; seeresolveSessionPath/CreateBacklogWorktree) rather than continuing the branch this PR just merged+pushed, andpushAndCreatePR's "reuse existing PR" check never verifies the new branch matches the tracked PR's head. This diff's sync still has real, standalone value — it keeps the currently open PR in sync withmainon GitHub immediately (which can resolve or shrink a main-drift-caused CI failure before the new session even starts) — but for CI failures caused by the PR's own diff (the common case), the actual fix commits land on an unrelated branch. Full resolution requires redesigning branch continuity across reopen iterations, which is shared withAutoReopenAfterFailedReviewand deserves its own investigation rather than a rushed change here.ReconcilePRPending's sequential per-item loop — worst case (~210s: 60s fetch + 60s merge + 30s abort + 60s push) could stall that tick if the remote is slow/hung. Bounded, not unbounded, and matches the existing timeout style used elsewhere in this file; a sharedcontext.Contextdeadline threaded throughMergeMainIntoWorktreewould tighten this but wasn't done here to avoid a larger signature change under review-cycle time pressure."main"(this repo's convention); no fallback tomaster/other default branches.