Revert PR #241: queue pr fixes#249
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
Warning Rate limit exceeded
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 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (35)
✨ 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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @@ -4576,44 +4574,6 @@ if (typeof window !== "undefined" && shouldInstallBrowserMock(window)) { | |||
| convergenceStateDelete: async (prId: string) => { | |||
There was a problem hiding this comment.
This revert removes the pipelineSettingsGet, pipelineSettingsSave, and pipelineSettingsDelete mock implementations from browserMock.ts, but those IPC methods still exist in ipc.ts (prsPipelineSettingsGet/Save/Delete), preload.ts, and global.d.ts. Any renderer component that calls window.ade.prs.pipelineSettingsGet(prId) in browser-mock mode (tests, Storybook) will receive undefined instead of a function and throw at call time. The pre-PR #241 baseline appears to have been missing these mocks as well, but if any component added during or after #241 relies on them in mock mode, it will break.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/browserMock.ts
Line: 4574
Comment:
**Missing browser mock implementations for pipeline settings IPC**
This revert removes the `pipelineSettingsGet`, `pipelineSettingsSave`, and `pipelineSettingsDelete` mock implementations from `browserMock.ts`, but those IPC methods still exist in `ipc.ts` (`prsPipelineSettingsGet/Save/Delete`), `preload.ts`, and `global.d.ts`. Any renderer component that calls `window.ade.prs.pipelineSettingsGet(prId)` in browser-mock mode (tests, Storybook) will receive `undefined` instead of a function and throw at call time. The pre-PR #241 baseline appears to have been missing these mocks as well, but if any component added during or after #241 relies on them in mock mode, it will break.
How can I resolve this? If you propose a fix, please make it concise.
|
|
||
| alter table queue_landing_state add column updated_at text; | ||
|
|
||
| delete from queue_landing_state; | ||
|
|
||
| insert into kv (key, value) values (?, ?) on conflict(key) do update set value = excluded.value; | ||
|
|
||
| create table if not exists rebase_dismissed ( |
There was a problem hiding this comment.
PR #241 introduced insert into kv (key, value) values (?, ?) on conflict(key) do update set value = excluded.value — raw ? placeholders in a plain SQL bootstrap file that SQLite executes as-is. SQLite would treat ? as a syntax error (or bind as NULL in some contexts), meaning the QUEUE_OVERHAUL_WIPE_MARKER guard was never recorded on iOS, and every app launch that hit this migration path would re-run the destructive delete from queue_landing_state. The revert correctly removes both lines. iOS users who upgraded to the #241 build may have had their queue state wiped on every cold start.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/ios/ADE/Resources/DatabaseBootstrap.sql
Line: 708-711
Comment:
**Broken parameterized SQL removed by revert**
PR #241 introduced `insert into kv (key, value) values (?, ?) on conflict(key) do update set value = excluded.value` — raw `?` placeholders in a plain SQL bootstrap file that SQLite executes as-is. SQLite would treat `?` as a syntax error (or bind as `NULL` in some contexts), meaning the `QUEUE_OVERHAUL_WIPE_MARKER` guard was never recorded on iOS, and every app launch that hit this migration path would re-run the destructive `delete from queue_landing_state`. The revert correctly removes both lines. iOS users who upgraded to the #241 build may have had their queue state wiped on every cold start.
How can I resolve this? If you propose a fix, please make it concise.
1 participant
This reverts the accidental merge of #241 into main.\n\nPR #241 was merged by Path to Merge before it was ready. This revert preserves history and removes the merged changes from main via a normal follow-up PR.\n\nMerge commit reverted: d42fbc4\nRevert commit: 09354c8
Greptile Summary
This PR cleanly reverts PR #241 ("queue pr fixes"), which was accidentally merged before it was ready. The revert removes the Path-to-Merge orchestrator (~1,052 lines), the Queue Automate Merging Modal (~723 lines), stacked-PR chain-base queue logic, and extended
PipelineSettingstype fields, restoring the codebase to its pre-#241 state across desktop, iOS, and docs.DatabaseBootstrap.sqlremoval surfaces a latent P1 bug introduced by queue pr fixes #241: a raw parameterizedINSERT INTO kv … VALUES (?, ?)in a plain SQL file would have caused SQLite to either error or bindNULL, meaning theQUEUE_OVERHAUL_WIPE_MARKERguard was never written andqueue_landing_statewould have been deleted on every cold start for iOS users on that build.browserMock.tslosespipelineSettingsGet/Save/Deletemock stubs while those IPC routes remain live; this restores the pre-queue pr fixes #241 baseline but leaves mock-mode callers without implementations.Confidence Score: 4/5
Safe to merge — the revert is complete and consistent across all layers, with one surfaced P1 bug (iOS SQL) that was already in the wild from the reverted PR
The revert is thorough and internally consistent: IPC constants, preload bindings, type declarations, service wiring, DB migrations, iOS models and SQL, and UI components are all reverted in lockstep. The P1 finding (iOS parameterized SQL) is a bug that was introduced by #241 and is fixed by this revert, not introduced by it. A P2 observation about missing browser mock stubs restores the pre-#241 baseline.
apps/ios/ADE/Resources/DatabaseBootstrap.sql (P1 bug surfaced), apps/desktop/src/renderer/browserMock.ts (missing pipelineSettings mock stubs)
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD subgraph "Reverted (PR #241 removed)" PtM["PathToMergeOrchestrator\n(~1052 lines deleted)"] QAM["QueueAutomateMergingModal\n(~723 lines deleted)"] Chain["Stacked-chain base logic\nin createQueuePrs()"] Retarget["retargetBase() +\nretargetNextEntryIfNeeded()"] PipelineExt["Extended PipelineSettings:\nconflictStrategy, autoAgentSettings\nforceFinalizeMode, earlyMergeOnGreen"] DBMigrations["DB Migrations:\npr_pipeline_settings new columns\npr_convergence_state.ptm_args_json\nQUEUE_OVERHAUL_WIPE_MARKER"] end subgraph "Restored state (post-revert)" Land["land() with inline\npost-merge cleanup"] Queue["Queue: all PRs target\nsame branch (no chaining)"] PipelineSimple["PipelineSettings:\nautoMerge, mergeMethod\nmaxRounds, onRebaseNeeded"] IPC["IPC/Preload/global.d.ts\nconsistently cleaned up"] end PtM --> Land QAM --> Queue Chain --> Queue Retarget --> Land PipelineExt --> PipelineSimple DBMigrations --> PipelineSimplePrompt To Fix All With AI
Reviews (1): Last reviewed commit: "Revert "Merge pull request #241 from aru..." | Re-trigger Greptile