Queue PR fixes v2

[arul28]

Reopens the PtM queue fixes after rollback #249 reverted the accidental landing of #241.

This branch reapplies #241 and adds follow-up hardening:

• lane worktree locking for PtM/fix/resolver flows
• iOS PtM authoring and persistence coverage for newer settings/runtime fields
• Work-tab-aligned model, reasoning, and permission plumbing for the full PtM pipeline and conflict resolver
• startup responsiveness fixes around project sync/icon loading
• stricter auto-merge readiness: blocks commented reviews with bodies, unresolved review threads, missing/pending/failing checks, and readiness fetch failures

Validated locally:

• npm --prefix apps/desktop run typecheck
• npm --prefix apps/desktop run test -- src/main/services/prs/pathToMergeOrchestrator.test.ts src/renderer/components/prs/shared/PrResolverLaunchControls.test.tsx src/renderer/components/app/TopBar.test.tsx
• npm --prefix apps/desktop run lint -- --quiet

Greptile Summary

This PR reapplies and hardens the PtM queue fixes from #241, adding the full pathToMergeOrchestrator state machine, lane worktree locking, iOS PtM authoring/persistence, and stricter auto-merge readiness checks. All three previously-flagged P1 issues have been addressed: stale COMMENTED reviews are now deduplicated via latestReviewByReviewer, the lock TTL has been raised to 75 min with heartbeat on every schedule call, and isPermanentCleanInventoryBlocker routes permanent CI blockers to pauseLoop instead of parkConverged.

Confidence Score: 4/5

Safe to merge; the one remaining P2 edge case (infinite parkConverged loop when autoMerge:true + GitHub auto-merge disabled) requires explicitly non-default settings to trigger

All three previous P1 findings are resolved. Only one P2 remains: isPermanentCleanInventoryBlocker's regex doesn't cover GitHub-CLI 'auto-merge not enabled' errors, leaving a residual indefinite polling loop for a non-default configuration. No P0 issues found across the 71-file change set.

apps/desktop/src/main/services/prs/pathToMergeOrchestrator.ts — isPermanentCleanInventoryBlocker gap noted above

Important Files Changed

Filename	Overview
apps/desktop/src/main/services/prs/pathToMergeOrchestrator.ts	New 1690-line PtM orchestrator implementing the full convergence loop with lock management, merge ladder, and at-cap policies; addresses all prior P1 review findings except one edge case in isPermanentCleanInventoryBlocker
apps/desktop/src/main/services/lanes/laneWorktreeLockService.ts	New lane worktree lock service; DEFAULT_LEASE_MS is 75 min (up from 45), release correctly counts rows before deleting, transaction logic looks correct
apps/desktop/src/main/services/prs/queueLandingService.ts	Adds retargetNextEntryIfNeeded for stacked PRs after landing; pipeline settings merge logic is correct; legacy field back-compat retained
apps/desktop/src/main/services/prs/issueInventoryService.ts	Adds new convergence runtime fields (ciRetryAttemptsUsed, pauseRepeatCount, etc.) with proper validation and DB persistence; DEFAULT_CONVERGENCE_RUNTIME_STATE covers all new fields
apps/desktop/src/main/services/prs/prService.ts	Extracts runPostMergeCleanup for reuse by merge ladder; adds acquireLaneMutationLock/releaseLaneMutationLock helpers; exposes retargetBase on service object
apps/desktop/src/main/services/conflicts/conflictService.ts	Adds laneWorktreeLockService integration; shared-lock (heartbeat) vs owned-lock (acquire+release in finally) split is correct
apps/desktop/src/main/services/prs/prIssueResolver.ts	Lock acquired for non-PtM dispatches (ownedLockToken) is released only on error, not after success; intentional — the 75 min TTL acts as the lease boundary
apps/desktop/src/shared/types/prs.ts	Adds PrAgentPermissionMode union, PipelineSettings fields, QueueAutomationConfig pipeline field with deprecated legacy mirrors; pipelineFromLegacyQueueConfig conversion is correct
apps/desktop/src/main/services/prs/resolverUtils.ts	mapPermissionMode now correctly handles PrAgentPermissionMode union including guarded_edit→edit and passthrough of all AgentChatPermissionMode values

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
Start([startPathToMerge]) --> AcquireLock[Acquire lane worktree lock\n ownerKind: path_to_merge]
AcquireLock -- locked by another --> BlockStart([blockedBy: LaneWorktreeLockBlocker])
AcquireLock -- acquired --> Schedule[schedule waitingOnReview\n heartbeatLoopLock on each schedule]
Schedule --> Iteration[runIterationInner]
Iteration --> CheckEnabled{autoConvergeEnabled?}
CheckEnabled -- no --> ClearTimer[clearTimer]
CheckEnabled -- yes --> Refresh[prService.refresh]
Refresh --> CheckMerged{pr.state?}
CheckMerged -- merged --> Cleanup[runPostMergeCleanup\n releaseLock\n stop]
CheckMerged -- closed --> PauseLoop([pauseLoop: PR closed\n releaseLock])
CheckMerged -- open --> EarlyMerge{earlyMergeOnGreen\n& CI passing\n& reviewClean?}
EarlyMerge -- yes autoMerge:false --> ParkConverged([parkConverged: click merge when ready\n schedules waitingOnReview])
EarlyMerge -- yes autoMerge:true --> MergeLadder[runMergeLadder]
MergeLadder -- merged REST/admin --> Done([releaseLock, stop])
MergeLadder -- auto_armed --> ParkAutoArmed([parkAutoArmed\n schedules waitingOnReview])
MergeLadder -- conflict --> ConflictStrategy[applyConflictStrategy]
ConflictStrategy --> Schedule
EarlyMerge -- no or blocked --> TerminalGate{CI + review\nterminal?}
TerminalGate -- no --> WarmingSchedule([schedule warming])
TerminalGate -- yes --> AtCap{currentRound >= maxRounds?}
AtCap -- no --> DispatchFix[launchPrIssueResolutionChat\n heartbeat shared lock]
DispatchFix --> Schedule
AtCap -- yes --> AtCapPolicy{decideAtCapAction}
AtCapPolicy -- stop --> HardCap([pauseLoop: hard cap\n releaseLock])
AtCapPolicy -- wait --> WarmingSchedule
AtCapPolicy -- dispatch_ci --> DispatchFix
AtCapPolicy -- merge_now --> CleanLadder[runMergeLadder\n clean inventory path]
CleanLadder -- merged --> Done
CleanLadder -- auto_armed --> ParkAutoArmed
CleanLadder -- conflict --> ConflictStrategy
CleanLadder -- blocked isPermanentBlocker --> PauseLoop2([pauseLoop: cannot auto-merge\n releaseLock])
CleanLadder -- blocked not permanent --> ParkConverged
CleanLadder -- failed --> PauseLoop2
Stop([stopPathToMerge]) --> ReleaseLock[releasePtmLock\n clearTimer\n autoConvergeEnabled: false]

Loading

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---### Issue 1 of 1
apps/desktop/src/main/services/prs/pathToMergeOrchestrator.ts:1526-1530
**`parkConverged` still loops when `autoMerge: true` but GitHub auto-merge is disabled on the repo**`isPermanentCleanInventoryBlocker` returns `false` when `checksStatus === "passing"` and the error string doesn't mention "CI". So if `gh pr merge --auto` fails with a non-CI message (e.g., "auto-merge not enabled for this repository"), the condition is false → `parkConverged` is called → another `waitingOnReview` wake-up is scheduled → the loop fires indefinitely every 30 minutes. The regex pattern `/\bCI\b|no completed successful CI checks|no CI checks/i` does not match the GitHub-CLI error for auto-merge being disabled at the org/repo level.

_{Reviews (3): Last reviewed commit: "Fix clean inventory merge blocker loop" | Re-trigger Greptile}

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
ade	Ignored	Preview	May 4, 2026 9:46pm

[coderabbitai]

Warning

Rate limit exceeded

@arul28 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 1 minute and 44 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 62fbf301-7d07-47aa-a9cd-bd002d1f9781

📥 Commits

Reviewing files that changed from the base of the PR and between cb6722c and 61866f8.

⛔ Files ignored due to path filters (3)

• docs/ARCHITECTURE.md is excluded by !docs/**
• docs/features/pull-requests/README.md is excluded by !docs/**
• docs/features/pull-requests/path-to-merge.md is excluded by !docs/**

📒 Files selected for processing (68)

• apps/ade-cli/README.md
• apps/ade-cli/src/bootstrap.ts
• apps/ade-cli/src/cli.test.ts
• apps/ade-cli/src/cli.ts
• apps/desktop/scripts/extractKvDbBootstrapSql.mjs
• apps/desktop/src/main/main.ts
• apps/desktop/src/main/services/chat/agentChatService.test.ts
• apps/desktop/src/main/services/chat/agentChatService.ts
• apps/desktop/src/main/services/conflicts/conflictService.ts
• apps/desktop/src/main/services/ipc/registerIpc.ts
• apps/desktop/src/main/services/lanes/laneWorktreeLockService.ts
• apps/desktop/src/main/services/prs/issueInventoryService.ts
• apps/desktop/src/main/services/prs/pathToMergeOrchestrator.test.ts
• apps/desktop/src/main/services/prs/pathToMergeOrchestrator.ts
• apps/desktop/src/main/services/prs/prIssueResolution.test.ts
• apps/desktop/src/main/services/prs/prIssueResolver.ts
• apps/desktop/src/main/services/prs/prMergeQueue.test.ts
• apps/desktop/src/main/services/prs/prRebase.test.ts
• apps/desktop/src/main/services/prs/prService.test.ts
• apps/desktop/src/main/services/prs/prService.ts
• apps/desktop/src/main/services/prs/queueLandingService.ts
• apps/desktop/src/main/services/prs/resolverUtils.ts
• apps/desktop/src/main/services/state/kvDb.laneWorktreeLockMigration.test.ts
• apps/desktop/src/main/services/state/kvDb.ts
• apps/desktop/src/main/services/sync/syncHostService.ts
• apps/desktop/src/main/services/sync/syncRemoteCommandService.test.ts
• apps/desktop/src/main/services/sync/syncRemoteCommandService.ts
• apps/desktop/src/main/services/sync/syncService.ts
• apps/desktop/src/preload/global.d.ts
• apps/desktop/src/preload/preload.ts
• apps/desktop/src/renderer/browserMock.ts
• apps/desktop/src/renderer/components/app/CommandPalette.tsx
• apps/desktop/src/renderer/components/app/TopBar.test.tsx
• apps/desktop/src/renderer/components/app/TopBar.tsx
• apps/desktop/src/renderer/components/automations/adeActionSchemas.ts
• apps/desktop/src/renderer/components/chat/AgentChatComposer.tsx
• apps/desktop/src/renderer/components/chat/AgentChatMessageList.test.tsx
• apps/desktop/src/renderer/components/chat/AgentChatMessageList.tsx
• apps/desktop/src/renderer/components/prs/PRsPage.tsx
• apps/desktop/src/renderer/components/prs/detail/PrDetailPane.issueResolver.test.tsx
• apps/desktop/src/renderer/components/prs/detail/PrDetailPane.tsx
• apps/desktop/src/renderer/components/prs/shared/PrAiResolverPanel.tsx
• apps/desktop/src/renderer/components/prs/shared/PrConvergencePanel.test.tsx
• apps/desktop/src/renderer/components/prs/shared/PrConvergencePanel.tsx
• apps/desktop/src/renderer/components/prs/shared/PrIssueResolverModal.tsx
• apps/desktop/src/renderer/components/prs/shared/PrPipelineSettings.tsx
• apps/desktop/src/renderer/components/prs/shared/PrResolverLaunchControls.test.tsx
• apps/desktop/src/renderer/components/prs/shared/PrResolverLaunchControls.tsx
• apps/desktop/src/renderer/components/prs/shared/prFormatters.ts
• apps/desktop/src/renderer/components/prs/state/PrsContext.test.tsx
• apps/desktop/src/renderer/components/prs/state/PrsContext.tsx
• apps/desktop/src/renderer/components/prs/tabs/GitHubTab.test.tsx
• apps/desktop/src/renderer/components/prs/tabs/GitHubTab.tsx
• apps/desktop/src/renderer/components/prs/tabs/QueueAutomateMergingModal.tsx
• apps/desktop/src/renderer/components/prs/tabs/QueueTab.tsx
• apps/desktop/src/renderer/components/prs/tabs/RebaseTab.tsx
• apps/desktop/src/renderer/components/prs/tabs/WorkflowsTab.tsx
• apps/desktop/src/shared/ipc.ts
• apps/desktop/src/shared/types/chat.ts
• apps/desktop/src/shared/types/conflicts.ts
• apps/desktop/src/shared/types/prs.ts
• apps/desktop/src/shared/types/sync.ts
• apps/ios/ADE/Models/RemoteModels.swift
• apps/ios/ADE/Resources/DatabaseBootstrap.sql
• apps/ios/ADE/Services/Database.swift
• apps/ios/ADE/Services/SyncService.swift
• apps/ios/ADE/Views/PRs/PrDetailOverviewTab.swift
• apps/ios/ADE/Views/PRs/PrDetailScreen.swift

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch ade/queue-pr-fixes-v2

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[arul28]

@codex review

[arul28]

@copilot review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:d54223aef7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Deduplicate reviewer history before blocking on comments

getMergeReadinessBlocker treats any historical commented review with a non-empty body as a hard blocker, but prService.getReviews() returns the full /pulls/{n}/reviews history (including superseded reviews from the same reviewer). If someone comments first and later approves, the stale comment entry still matches this filter and auto-merge stays blocked indefinitely even though the latest reviewer state is acceptable. The blocker check should run on each reviewer's latest submission only.

Useful? React with 👍 / 👎.

[Copilot]

@copilot review

Reviewed. CI is green. Key observations:

laneWorktreeLockService — SQLite BEGIN IMMEDIATE transaction wraps the acquire race correctly. The LaneWorktreeLockedError is thrown after COMMIT, so the catch block's rollback guard (if (!(error instanceof LaneWorktreeLockedError))) handles both paths correctly.

pathToMergeOrchestrator / getMergeReadinessBlocker — The double-check approach (cached PrSummary fields first, then full API calls for checks/review-threads/reviews) is correct and blocks on: changes_requested, pending reviews, missing/pending/failing checks, unresolved review threads, and commented reviews with bodies. Error on readiness fetch also blocks as intended.

mapPermissionMode (resolverUtils) — The passthrough for AgentChatPermissionMode values (default, plan, edit, full-auto, config-toml) is correct. The OpenAI guarded_edit → default special-case in mapPermissionModeForModelFamily is preserved.

runGh spawn — Arguments are passed as an array to spawn (not a shell string), so no shell injection risk even from user-controlled merge method or PR number.

main.ts wiring — defaultModelId: null / defaultReasoningEffort: null is intentional; the stale-flag recovery logic in resumeFromPersistedState handles the case where there's no persisted model ID by clearing the flag rather than warming a doomed wake-up.

Minor notes (non-blocking):

• hasBlockingCommentedReview will block any commented review with a body, even informational ones. This is per spec but worth documenting in the UI ("operator review needed" label is good).
• syncRemoteCommandService.ts allows "config-toml" as a valid permissionMode from iOS callers — fine for forward-compat, but iOS UI probably shouldn't expose it.
• The queue's retargetNextEntryIfNeeded is wrapped in try/catch and non-blocking, which is correct given that a failed retarget surfaces as a natural merge error on the next iteration.

[arul28]

@copilot review but do not make fixes

[Copilot]

@copilot review but do not make fixes

Review of the 3 commits (no fixes):

---

Commit 1 & 2 (already reviewed)
See prior reply — findings stand.

---

Commit 3: "Fix path-to-merge review blockers" (3a625dc)

laneWorktreeLockService — release now returns actual row count

The SELECT count(1) + DELETE pair is outside a transaction. On a busy process-per-lane setup this is a TOCTOU race: the count can be 1 immediately before a concurrent DELETE zeroes the table. In practice the worktree lock table has one row per worktree key and all callers discard the return value anyway (release(…) is called void everywhere in main.ts, prIssueResolver, conflictService, and the orchestrator), so the race has no observable consequence. The return value is only exercised by the new test. Harmless, but the count SELECT adds two extra statements per release call with no functional benefit.

The lease bump from 45 min → 75 min now comfortably covers the waitingOnReview phase (1800 s = 30 min). The heartbeat call in schedule() always uses the default 75 min renewal, so the lease length is consistent. ✓

pathToMergeOrchestrator — latestReviewByReviewer

Deduplication logic is correct. Reviews are keyed on reviewer.trim().toLowerCase(), which matches how fetchReviews in prService normalises unknown logins to "unknown" (not empty). The reviewer field in PrReview is typed as string (non-nullable) and fetchReviews always sets it to a non-empty fallback "unknown", so the if (!reviewer) anonymous-review branch will only be reached for test fakes with an empty string — not a real API path, but the fallback is correct.

Tie-breaking at equal timestamps (nextAt >= currentAt): when two reviews from the same reviewer have identical submittedAt, the last one in the array wins. The GitHub reviews API returns reviews in ascending chronological order, so array-last = most recent is the right tiebreak. ✓

dismissed state not explicitly handled: a dismissed review has state: "dismissed" in the type. latestReviewByReviewer includes it in latestReviews. It doesn't match changes_requested and hasBlockingCommentedReview returns false for it (state !== "commented"), so a dismissed review is silently ignored, which is correct — GitHub already treats dismissed changes_requested reviews as no longer blocking. ✓

Test coverage: two new tests added — (1) commented-then-approved same reviewer unblocks merge, (2) heartbeat is called on schedule. Both exercise the meaningful paths. One gap: no test for the equal-timestamp tiebreak where comment appears after approve.

pathToMergeOrchestrator — heartbeatLoopLock

Heartbeat fires inside schedule(), which is called each time the loop reschedules (after every iteration). This is fine for the 270 s and 720 s phases. For waitingOnReview (1800 s), the lock gets heartbeated once at schedule time and then again when the next iteration fires — a gap of exactly 1800 s. With the default 75 min (4500 s) lease, the lock won't expire mid-wait. ✓

main.ts — orchestrator.dispose() in teardown

Added to disposeContextResources, which clears all setTimeout handles. The order (before autoRebaseService?.dispose()) is fine since the orchestrator doesn't depend on autoRebaseService for cleanup. ✓

kvDb migration test

The new lane worktree locks describe block tests the corrected release return value. All four cases (missing ownerKind, missing token, present token, double-release) are covered. ✓

---

Summary: No functional regressions. The one structural note (COUNT + DELETE outside a transaction) is non-blocking given that all production callers ignore the return value.

[arul28]

@copilot review but do not make fixes