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
|
tstapler
added a commit
that referenced
this pull request
Jul 26, 2026
…tor gap (#163) * fix(terminal): correctly scan OSC/DCS escape sequences to stop render artifacts stripANSIBytes and sanitizeUTF8Bytes treated any ASCII letter as the end of an escape sequence. That's only true for CSI (ESC[...letter); OSC (ESC]...BEL or ESC\) and DCS/PM/APC/SOS (ESC{P,^,_,X}...ESC\ or 0x9C) terminate differently, and their payloads (window titles, hyperlink URLs, shell-integration marks) almost always contain a letter before the real terminator. Claude Code's newer renderer emits more of these OSC sequences, so their payload tails were leaking through as literal text in the web terminal and throwing off cursor-column math. Add a shared scanEscapeSequence helper (mirrors the correct boundary logic already used by pkg/analytics/escape_code_parser.go) and rewire both duplicated stripANSIBytes definitions plus sanitizeUTF8Bytes to consume whole sequences atomically instead of stopping at the first letter. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(terminal): widen CSI final-byte range to 0x40-0x7E, cap OSC/DCS scan size Code review on PR #156 found that the new scanCSI only accepted A-Z/a-z as CSI terminators, missing real ECMA-48-valid final bytes like '@' (0x40, Insert Character) and '~' (0x7E, used by many real xterm sequences e.g. function/navigation keys). Confirmed empirically: stripANSIBytes("\x1b[5@Hello") leaked "@hello" instead of "Hello" — the exact bug class this PR exists to eliminate, just for a different final byte. Widened to the full 0x40-0x7E range and aligned the malformed-CSI fallback with pkg/analytics' semantics (give up and consume only the ESC, rather than swallowing partially-scanned params). Also: - Widened pkg/analytics/escape_code_parser.go's parseCSI terminator range to match (it was cited as the reference implementation for this fix but had the same narrower gap for '~' and other non-letter finals in 0x5B-0x60/0x7B-0x7E). - Added a size cap (mirroring escape_code_parser.go's existing 65536 bound) to scanUntilTerminator so an unterminated/adversarial OSC or DCS payload can't force an unbounded scan. - Pre-size the bytes.Buffer in stripANSIBytes/sanitizeUTF8Bytes with Grow(len(b)) to avoid reallocation growth in this hot path. - Removed the now-vestigial (*StateGenerator).stripANSIBytes wrapper method now that its only caller can use the shared free function directly. - Added regression tests for all of the above, including a mid-buffer (start > 0) case and the new size-cap behavior. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * refactor: remove dead MOSH-style terminal state-sync infrastructure Investigating the terminal-artifacts report (PR #156) surfaced that server/terminal (the package that PR fixed) has zero callers anywhere in the repo — it's pre-integration code for a "state" streaming mode that was designed but never wired into the live rendering path. Tracing further found this same abandoned effort spans three separate, never- fully-connected implementations, plus the client-side code built to consume them: - server/terminal (StateGenerator/DeltaGenerator) — zero callers. - server/ssp (Mosh-inspired Coordinator) + session/framebuffer (its diff engine) — zero callers. - session/terminal_state.go (regex-based TerminalState) — only called from SessionService.StreamTerminal, which browsers never reach: the WebSocket bridge (connectrpc_websocket.go) intercepts the same RPC procedure and reimplements it with plain raw-byte passthrough. - server/compression (LZMA) — only exercised by transport_e2e_test.go, itself a spec-as-test for streaming modes the server never actually implements (its own docstring says so). - Frontend: StateApplicator/DeltaApplicator/EchoOverlay/lzma.ts and the predictive-echo/SSP-negotiation logic in useTerminalFlowControl.ts — all gated behind `sspNegotiated`, which never becomes true since the server never sends an SSPNegotiation message. Confirmed sendInput (the working path) is always used in practice, not sendInputWithEcho. - The streaming-mode selector (config field, proto field, dropdown UI) that drove all of the above: the value was accepted and logged but never branched on by the code that actually sends output. Simplified SessionService.StreamTerminal to plain raw output (removing its session.TerminalState dependency) rather than deleting it outright, since the RPC interface still requires an implementation for any future non-browser Connect client. Also widened the same CSI final-byte gap (missing '@'/'~' etc.) in pkg/analytics/escape_code_parser.go that PR #156 fixed in server/terminal, since it was cited as that fix's reference implementation and had the identical bug. Left ADR-002 (which documented the abandoned "state" mode decision for external sessions) in place with its status marked Superseded, rather than deleting it, to preserve the historical record. Verification: go build/vet, full Go test suite, npx tsc --noEmit, and the full Jest suite (2776 tests) all pass. go mod tidy dropped the now- unused github.com/ulikunitz/xz dependency. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(terminal): resolve CSI terminator gap and code-review follow-ups BUG-025: widen the CSI final-byte character class from [a-zA-Z] to [@-~] (0x40-0x7E per ECMA-48) everywhere it was copy-pasted narrow, so sequences terminating on '@' or '~' no longer leak raw escape bytes past rate-limit/activity detection and tmux banner filtering. Consolidates the fix into one shared, tested module per language (pkg/ansi for Go, web-app/src/lib/terminal/stripAnsi.ts for TypeScript) instead of five independently duplicated regexes, per code-review findings. Also reserves and deletes the orphaned MOSH oneof message types left behind in events.proto by the earlier dead-code removal. Adding real end-to-end test coverage for StreamTerminal's simplified output path surfaced two pre-existing concurrency bugs under -race, now fixed: - the output goroutine could race Connect's own stream-close write since nothing joined it before the handler returned - session.Instance.Started() read the started field without its lock while start() wrote it outside the lock Also adds a test pinning the WebSocket-vs-general-handler routing precedence for the StreamTerminal path, previously only documented in a comment. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(terminal): close residual races flagged by PR #163 code review A /github:pr-ship code-review gate on PR #163 (4 parallel agents: testing, code quality, architecture, security) found that both concurrency fixes from the previous commit were real but incomplete: - StreamTerminal's output goroutine was bounded-and-abandoned on timeout rather than actually interrupted, so it could still call stream.Send() after the handler returned if the PTY read blocked past the 2s bound. Fixed by dup'ing the PTY fd so this goroutine gets its own independent *os.File/poll.FD, safe to set a real 250ms read deadline on without racing the instance's other PTY consumers (response stream, command executor) that share the original fd. - session.Instance.started was fixed at exactly the two sites this session's new test exercised, leaving ~30 other unguarded read/write sites across the package equally racy. Converted the field to atomic.Bool everywhere instead, which is race-free by construction rather than by per-site lock-discipline audit. Also: added a deterministic unit test for waitWithTimeout's timeout branch (previously only indirectly exercised), and exported ansi.IsCSIFinalByte as a byte-range twin to CSIFinalByteClass so pkg/analytics/escape_code_parser.go's manual byte-scanning CSI check (a 6th independent reimplementation the prior consolidation missed) now derives from the same shared source of truth. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(session): close remaining actor/i.mu synchronization gaps post-merge Follow-up to the main-merge commit: these edits were made while chasing -race failures during conflict resolution but landed after that commit's git add, so they weren't included in it. - Instance.Preview, InstanceStatusManager.GetStatus: read Status via Snapshot() instead of a bare/RLock-guarded field read, matching the pattern already used by GetStatus/IsActive/etc. Both were caught by go test ./... -race (transitionToLocked writes i.Status outside i.mu). - pkg/ansi: tighten csiRegex/csiFinalByteMin/csiFinalByteMax to unexported now that no caller outside the package needs them directly. - instance_fork_test.go, instance_workspace_test.go: fix two `started:` struct-literal fields / bare assignments that go vet flagged after the bool -> atomic.Bool conversion. - instance_timestamp_test.go: wrap the test instance in a LiveInstance so TestInstance_TimestampConcurrency's concurrent UpdateTerminalTimestamps calls are actually actor-serialized, instead of silently taking the no-actor synchronous fallback (send()'s documented behavior when liveInstance is nil) with zero cross-goroutine synchronization. - instance_test.go: TestFromInstanceDataWithMissingWorktree now uses ForceStatus instead of a bare `instance.Status = Paused` write, so the Paused() check after it doesn't read a stale cached Snapshot(). - instance_serialization.go: ToInstanceData now builds its snapshot via sendSyncErr instead of reading the cached Snapshot(). Storage.UpdateInstance and SaveInstancesSync mutate Instance fields (Tags, Category) directly and expect the next ToInstanceData() call to see it; Snapshot()'s cache only refreshes when an actor command republishes it, so those callers were reading stale data (TestStorage_UpdateInstance, TestStorage_SaveInstancesSync). sendSyncErr gets a fresh buildSnapshot(s.inst) while still serializing against any concurrently-running actor commands. - instance_approval.go: simplify snap.ReviewState.TimeSince* to snap.TimeSince* (staticcheck QF1008 — ReviewState is embedded in InstanceSnapshot). - session_service_stream_terminal_test.go: wire session.NewRegistry + SetRegistry, matching server/dependencies.go's production wiring, so CreateSession actually wraps the instance in a LiveInstance during the test. go test ./... -race: 0 data races (verified across multiple full-suite runs). * fix(server): make PTY fd duplication cross-platform for windows/amd64 build syscall.Dup (used by StreamTerminal to give its reader goroutine an independent PTY fd) doesn't exist on Windows, breaking the release build matrix (windows/amd64) added by .github/workflows/build.yml — the failure first surfaced on PR #163's post-merge CI run, caused fail-fast cancellation of the whole Build matrix. Move the dup behind a dupPTYFile helper, split by build tag like the existing execSyscall precedent (exec_unix.go/exec_windows.go): syscall.Dup on Unix, windows.DuplicateHandle (golang.org/x/sys/windows, already a direct dependency) on Windows. Verified go build ./... for darwin/{amd64,arm64}, linux/{amd64,arm64}, and windows/amd64. Also investigated two other CI failures from the same run: - TestActorNoLeak/TestActorStopIdempotent (goleak "unexpected goroutines" from TestMain's own background helpers) — could not reproduce locally (isolated run or full `go test ./session -race`), consistent with a CI-timing-sensitive flake in this newly-merged upstream test, not something introduced here. - A TmuxSession.ptmx data race (GetPTY's unguarded read racing closePTYAndAttachCmd's unguarded close/reconnect) reproduced locally under -race -count=15 on TestStreamTerminal_SendsRawOutput. Pre-existing in session/tmux (t.ptmx is read/written without synchronization in ~15 call sites in tmux.go); fixing it properly needs a TmuxSession-level mutex refactor, out of scope for this merge-conflict PR. Flagged as a known follow-up, not fixed here. * fix(session): stop TestActorNoLeak/TestActorStopIdempotent flaking on unrelated leaks Reproduced the CI-only failure (2/2 remote runs) locally by constraining GOMAXPROCS=2, which changes goroutine scheduling enough to surface it — a plain -race/-count run on this fast dev machine never hit it. Two contributing issues, both fixed: 1. TestApprove_FromHibernated_Succeeds and TestApprove_AllSourceStatuses's Hibernated->Active case call Approve(), which (via transitionToLocked) dispatches resumeFromHibernationLocked's real Start()/StartController()/ StartSessionDriver() work in a background goroutine. StartSessionDriver's ticker loop has no external cancellation and runs for driverTotalTimeout (tens of minutes) — these tests never stop it, so it leaks for the rest of the test binary's life. Fixed by pre-setting driverRunning (the same CAS guard TestStartSessionDriver_Idempotent already relies on) so StartSessionDriver is a no-op; the tests aren't exercising driver dispatch, just the state transition. 2. The deeper issue: TestActorNoLeak/TestActorStopIdempotent used a bare process-wide goleak.VerifyNone(), which asserts "no goroutine is alive anywhere in the process" rather than these tests' actual claim ("Stop() joins the goroutine THIS test started"). In the large session package's shared test binary, ANY earlier, unrelated test that leaves its own background goroutine running (a ResponseStream poller, an exec.Cmd wait, etc.) trips this on unrelated grounds, nondeterministically depending on run order and scheduling — confirmed locally: after fixing #1, a -count=8 stress run still surfaced two more pre-existing unrelated leaks (MangleCorrelator.StartEviction, os.Process.pidWait) before either test assertion even reached the actor-specific logic. Auditing and fixing every leak source across this large, pre-existing test package is a separate, much larger effort - fixed the actual fragility instead: goleak.IgnoreCurrent() baselines the goroutine set before the actor work, so pre-existing goroutines from other tests are excluded and only a genuine actor-goroutine leak fails the test. Verified: GOMAXPROCS=2 go test ./session -race -v -count=8 — 0 failures (was reproducing on every run before this fix). * fix(scanner): register 3 missing backlog GitHub RPCs in methodToID tools/scanner's TestMethodToIDCompleteness failed in CI (Run scanner unit tests step of PR #163's Test job): SearchGitHubRepos, ListGitHubIssues, and ImportGitHubIssue exist in backlog.proto (and have real, if currently unimplemented/stubbed, server handlers - see the separate TestSearchGitHubRepos_*/TestListGitHubIssues_* skips) but were never added to proto_scanner.go's methodToID feature-registry map. Landed via the same upstream fork-sync merge as the actor-pattern refactor; nothing in this PR added these RPCs, just the first CI run to reach this check since the merge. Ran make registry-generate to regenerate the per-feature JSON files this map drives (docs/registry/features/backend/backlog/{search-github-repos, list-github-issues,import-github-issue}.json), which also cleaned up a stale duplicate (top-level DeleteBacklogItem.json, superseded by the already-committed backlog/delete-item.json from PR #159's merge - unrelated to this fix, just picked up by the same regenerate pass). registry-diff divergence back to 0.0%. * fix(scanner): keep GitHub RPC feature IDs stable to avoid a registry-diff false positive Follow-up to 94ce8333f: mapping SearchGitHubRepos/ListGitHubIssues/ ImportGitHubIssue to new kebab-case backlog:* IDs relocated their registry files (docs/registry/features/backend/backlog/*.json), which two things then treated as brand-new, untested RPCs: - The "Check new RPCs have tests" CI gate diffs registry files against origin/main by path; a relocated file is indistinguishable from a genuinely new one there. - SearchGitHubRepos and ListGitHubIssues already have real, committed tests (server/services/backlog_github_rpc_test.go) and tested:true/testIds on origin/main under their old top-level path/id — relocating them would have discarded that (loadExisting only preserves tested/testIds/ lastModified when the output *path* is unchanged). Fixed by mapping these three to their own method name (matching ScanProto's pre-existing fallback when methodToID has no entry), so the generated id/path is byte-identical to what's already committed. Restored SearchGitHubRepos.json/ListGitHubIssues.json/ImportGitHubIssue.json from origin/main and re-ran make registry-generate: markerFound correctly recalculates to false (no // +api: marker exists — these RPCs are still unimplemented stubs, that part hasn't changed), while tested/testIds/ lastModified are preserved via loadExisting's existing-file logic. ImportGitHubIssue.json (tested:false on both branches, no marker either side) comes out byte-identical to origin/main; the other two only change markerFound (true, which was stale, to false) and lastModified, with tested:true and their real testIds untouched - so neither trips the "new RPC has no test" per-file check (tested is true, and ImportGitHubIssue doesn't show up as changed in the origin/main diff at all). registry-diff: 0.0% divergence. cd tools/scanner && go test ./...: pass. * chore(tests): untrack tests/e2e/node_modules and fix flaky accessibility waits node_modules under tests/e2e was accidentally committed since it wasn't covered by .gitignore. Also swap networkidle waits in accessibility.spec.ts for element-based waits, since this app polls continuously and network activity never settles for the required window (matches alias.spec.ts). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: include tests/e2e/node_modules/ in .gitignore (missed in prior commit) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: bump otel past CVE-2026-41178, fix PreviewFullHistory race, refresh stale docs - security: bump go.opentelemetry.io/otel, /sdk, /trace from v1.43.0 to v1.44.0. v1.43.0 falls inside CVE-2026-41178's affected range (baggage-header-parsing DoS); the app registers the baggage propagator globally and otelhttp wraps the network- exposed HTTP server as the outermost layer, ahead of auth middleware. go mod tidy confirmed otelhttp v0.67.0 and otlptracegrpc v1.39.0 remain compatible with the bumped otel core, so those were left untouched; golang.org/x/sys picked up a transitive bump to v0.45.0. go mod tidy also dropped an unused gvisor.dev/gvisor indirect require (checklocks is installed via `go install ...@latest` in the Makefile, not consumed as a module dependency). - fix(session): PreviewFullHistory() read i.Status directly, unsynchronized with transitionToLocked's writes (which happen inside the actor's own serialization, not under i.mu) - the same hazard already fixed on Preview() a few lines above. Extracted the shared check into a new previewBlocked() helper used by both Preview() and PreviewFullHistory() so the two can't diverge again. - docs(session): updated ReviewState's file-header and UpdateTimestamps doc comments, which still described the old "protected by Instance.mu" discipline. UpdateTerminalTimestamps (the sole call path) now runs inside the actor's i.send() closure instead of holding i.mu; the comments now describe that. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Tyler Stapler <tystapler@gmail.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
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.