Chat 20260608 131523#563
Conversation
…p + iOS) Desktop: thin main-process scheduler injects watch turns into a visible chat agent per PR; removes standalone resolver + inventory digestion. iOS: migrate PR-detail surface to the agent-watcher model. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…523-a511f3ef # Conflicts: # apps/desktop/src/main/services/prs/pathToMergeOrchestrator.ts # docs/features/pull-requests/path-to-merge.md
Drop the deterministic orchestrator, issue inventory, convergence UI, and related IPC/sync/iOS surfaces so PR workflows rely on queue, merge rails, and agent-driven ship instead. Co-authored-by: Cursor <cursoragent@cursor.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
@copilot review but do not make fixes |
|
Warning Review limit reached
More reviews will be available in 43 minutes and 45 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (14)
📒 Files selected for processing (90)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
| @@ -2767,18 +2762,6 @@ app.whenReady().then(async () => { | |||
| error: error instanceof Error ? error.message : String(error), | |||
There was a problem hiding this comment.
path_to_merge locks are never released on session end after upgrade
The session-end handler previously released both pr_issue_resolution and conflict_resolution locks for the ending session. The pr_issue_resolution release was removed here alongside the PtM code. On upgrade, any in-flight PtM session whose lock carries owner_kind = 'path_to_merge' (set by the orchestrator's acquire call) won't be caught by either remaining release call, because neither matches. The lock then blocks new conflict_resolution acquisitions on that worktree until the heartbeat lease expires. Consider adding a one-time migration DELETE FROM lane_worktree_locks WHERE owner_kind IN ('path_to_merge', 'pr_issue_resolution') in kvDb.ts to clear these immediately on next DB open.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/main/main.ts
Line: 2754-2762
Comment:
**`path_to_merge` locks are never released on session end after upgrade**
The session-end handler previously released both `pr_issue_resolution` and `conflict_resolution` locks for the ending session. The `pr_issue_resolution` release was removed here alongside the PtM code. On upgrade, any in-flight PtM session whose lock carries `owner_kind = 'path_to_merge'` (set by the orchestrator's `acquire` call) won't be caught by either remaining release call, because neither matches. The lock then blocks new `conflict_resolution` acquisitions on that worktree until the heartbeat lease expires. Consider adding a one-time migration `DELETE FROM lane_worktree_locks WHERE owner_kind IN ('path_to_merge', 'pr_issue_resolution')` in `kvDb.ts` to clear these immediately on next DB open.
How can I resolve this? If you propose a fix, please make it concise.7d62d8a to
24ca19f
Compare
After removing the PtM pipeline, orphaned path_to_merge and pr_issue_resolution locks could block conflict_resolution until lease expiry. Co-authored-by: Cursor <cursoragent@cursor.com>
Summary
Describe the change.
What Changed
Key files and behaviors.
Validation
How you tested.
Risks
Anything to watch.
Greptile Summary
This PR removes the Path-to-Merge (PtM) orchestrator and the associated issue-inventory/convergence pipeline, retiring ~26,600 lines across desktop, iOS, CLI, and docs. The feature is cleanly excised from all layers: IPC handlers, preload bridge, renderer components, iOS views, RPC server tool specs, and shared types.
issueInventoryService,pathToMergeOrchestrator, and their DB tables (pr_issue_inventory,pr_pipeline_settings,pr_convergence_state) are removed from migrations; a one-time migration deletes orphanedpath_to_merge/pr_issue_resolutionlane-worktree locks on next DB open, addressing the previously flagged lock-release gap.PrConvergencePanel,PrIssueResolverModal,PrPipelineSettings,QueueAutomateMergingModal,PrsContextconvergence-state cache, and all related IPC bridge entries are removed in a consistent sweep across desktop and iOS.shouldAttemptAdminMergeForRestErroris moved from the deletedpathToMergeOrchestrator.tsintoresolverUtils.tswith identical logic;prService.tsimport updated accordingly.Confidence Score: 5/5
Safe to merge — the removal is thorough and internally consistent across all layers, and the one-time lock-cleanup migration directly addresses the previously flagged lock-release concern.
The sweep is well-coordinated: every call site, IPC handler, preload bridge entry, type declaration, and test fixture that referenced PtM or issue inventory was updated or removed. The new DB migration clears stale path_to_merge / pr_issue_resolution locks. The only gap is that resolverUtils.ts now has three exported functions whose unit tests were deleted alongside the removed files — not a runtime risk, but a coverage gap on the admin-merge gate path.
apps/desktop/src/main/services/prs/resolverUtils.ts — the relocated shouldAttemptAdminMergeForRestError and the two permission-mode mappers no longer have dedicated unit tests after prRebase.test.ts and pathToMergeOrchestrator.test.ts were deleted.
Important Files Changed
Comments Outside Diff (2)
apps/desktop/src/main/services/state/kvDb.migrations.test.ts, line 309-365 (link)path_to_merge/pr_issue_resolutionlocks no longer testedThe test was updated to use
conflict_resolutionthroughout, so the lane-worktree-lock migration no longer validates that rows with the now-removedowner_kindvalues are handled correctly. On upgrade from a version where the PtM orchestrator was mid-run,lane_worktree_locksrows withowner_kind = 'path_to_merge'orowner_kind = 'pr_issue_resolution'will remain in the database. The lock service'sacquirepath comparesowner_kindstrictly, so those unexpired rows will block anyconflict_resolutionoperation on the same worktree until the lease expires naturally (no migration sweep runs for these specific kinds). Adding a test case—or a migration delete step—for both removed owner-kinds would close this gap.Prompt To Fix With AI
apps/desktop/src/main/services/prs/pullRequestRowCleanup.ts, line 30-52 (link)The explicit
delete from pr_convergence_state / pr_pipeline_settings / pr_issue_inventorycalls were removed. For databases where the tables exist AND were created before the FK-cascade retrofit ran (i.e. theFK_CONSTRAINTSentries that are now also removed),PRAGMA foreign_keys = ONmay be active but the stored table SQL lacks theON DELETE CASCADEclause — so cascade silently does nothing, leaving orphaned rows behind. New installations never create these tables, so this only affects existing users on older schema generations. A lightweight migrationDELETE FROM pr_issue_inventory / pr_pipeline_settings / pr_convergence_state WHERE pr_id NOT IN (SELECT id FROM pull_requests)would purge the orphans without risk.Prompt To Fix With AI
Reviews (4): Last reviewed commit: "Clear legacy Path to Merge worktree lock..." | Re-trigger Greptile