Revert "[codex] Fix renderer stress degradation"#4535
Merged
Conversation
This reverts commit 6fe97bc.
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (84)
📝 WalkthroughWalkthroughThis PR removes the renderer stress testing infrastructure (scripts, CDP port handling, window bridges), simplifies terminal addon management by removing image/WebGL controllers, eliminates ChangesRenderer Stress Testing Infrastructure Removal
Terminal Addon Management Refactoring
Hook Signature Simplification (Removal of
Workspace Navigation Store and Router Integration
Browser Pane and Tab Bar UI Optimization
Changes Sidebar Virtualization and Collapsible Refactoring
Pull Request Runtime and Git Query Simplification
Workspace Run Definition Type Tightening
Miscellaneous Optimizations and Cleanups
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
Contributor
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
|
Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
Greptile SummaryThis PR reverts #4500 ("[codex] Fix renderer stress degradation"), removing approximately 6,700 lines of changes across 84 files and restoring the pre-#4500 baseline.
Confidence Score: 4/5Safe to merge as an intentional rollback; all behavioural changes are deliberate restorations of pre-#4500 code The revert is clean and mechanically correct. Three behaviours that #4500 specifically addressed are now absent again: the active tab will not scroll into view when the tab bar overflows; concurrent project-creation requests can race on ensureMainWorkspaceStrict and surface a constraint-rejection error; and in-flight pull-request sync tasks are not drained before the SQLite handle is closed. None of these were new breakage introduced accidentally — they are the known pre-#4500 state — but they are worth tracking as follow-on work. TabBar.tsx (missing active-tab scroll-into-view), ensure-main-workspace.ts (concurrent call race), app.ts (fire-and-forget stop on shutdown)
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/lib/terminal/terminal-runtime.ts | Removes output-queuing, WebGL/image addon lifecycle management, and RAF-deferred terminal disposal; restores direct terminal.dispose() and simpler resize scheduler |
| packages/panes/src/react/components/Workspace/components/TabBar/TabBar.tsx | Removes virtual tab windowing and active-tab auto-scroll; replaces scroll-metrics tracking with OverflowFadeContainer — active tab no longer scrolls into view when tab bar overflows |
| packages/host-service/src/trpc/router/project/utils/ensure-main-workspace.ts | Removes in-flight request deduplication map for concurrent ensureMainWorkspaceStrict calls; a DB unique index on (projectId, hostId) still prevents actual duplicates |
| packages/host-service/src/app.ts | Removes await from pullRequestRuntime.stop() and removes filesystem.close() call during shutdown; background tasks are no longer drained before process exit |
| packages/host-service/src/runtime/pull-requests/pull-requests.ts | Reverts stop() to synchronous, removing backgroundTasks tracking and drainInFlightWork; in-flight sync tasks are no longer awaited on shutdown |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["PR #4500 Changes Being Reverted"] --> B["Stress Testing Tooling"]
A --> C["Terminal Optimizations"]
A --> D["Tab Bar"]
A --> E["Backend Infrastructure"]
B --> B1["stress-renderer.ts removed"]
B --> B2["useRendererStressWorkspaceBridge.ts removed"]
C --> C1["Output queuing removed"]
C --> C2["WebGL context-loss recovery removed"]
D --> D1["Virtual tab windowing removed"]
D --> D2["Active tab auto-scroll removed"]
E --> E1["gitConfigWrite retry removed"]
E --> E2["ensureMainWorkspace dedup removed"]
E --> E3["PullRequestRuntime drain removed"]
Comments Outside Diff (1)
-
packages/host-service/src/trpc/router/project/utils/ensure-main-workspace.ts, line 57-80 (link)Concurrent calls to
ensureMainWorkspaceStrictcan now raceThe
mainWorkspaceEnsuresInFlightdeduplication map from [codex] Fix renderer stress degradation #4500 is removed. If two requests for the same(projectId, repoPath)reachensureMainWorkspaceStrictconcurrently (e.g. the startup sweep and a user-triggered project-creation handler both firing), both will pass the "does a main workspace exist?" check before either commits, and both will callctx.api.v2Workspace.create.mutate. The DB unique index on(projectId, hostId) WHERE type='main'will reject the second insert, surfacing an unhandled error fromensureMainWorkspaceStrict(the strict variant does not swallow errors). The non-strict wrapperensureMainWorkspacewill catch it and log a warning, but any caller that relied onensureMainWorkspaceStrictsucceeding (e.g. the project-creation saga inhandlers.ts) will receive an unexpected rejection.Prompt To Fix With AI
This is a comment left during a code review. Path: packages/host-service/src/trpc/router/project/utils/ensure-main-workspace.ts Line: 57-80 Comment: **Concurrent calls to `ensureMainWorkspaceStrict` can now race** The `mainWorkspaceEnsuresInFlight` deduplication map from #4500 is removed. If two requests for the same `(projectId, repoPath)` reach `ensureMainWorkspaceStrict` concurrently (e.g. the startup sweep and a user-triggered project-creation handler both firing), both will pass the "does a main workspace exist?" check before either commits, and both will call `ctx.api.v2Workspace.create.mutate`. The DB unique index on `(projectId, hostId) WHERE type='main'` will reject the second insert, surfacing an unhandled error from `ensureMainWorkspaceStrict` (the strict variant does not swallow errors). The non-strict wrapper `ensureMainWorkspace` will catch it and log a warning, but any caller that relied on `ensureMainWorkspaceStrict` succeeding (e.g. the project-creation saga in `handlers.ts`) will receive an unexpected rejection. How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
packages/panes/src/react/components/Workspace/components/TabBar/TabBar.tsx:189-230
**Active tab no longer scrolls into view on overflow**
The revert removes the `useLayoutEffect` that computed `nextScrollLeft` and imperatively set `scrollContainerRef.current.scrollLeft` whenever `activeTabIndex` changed. With `OverflowFadeContainer`, the tab list is a regular flex row with native `overflow-x: auto`, so the active tab stays at whatever scroll position it already occupies. If a user has 12+ tabs and switches to one that is off-screen (e.g. via keyboard shortcut or programmatic tab selection), the newly active tab will not scroll into view — users will see an empty-looking area with no visible selection indicator until they scroll manually.
### Issue 2 of 3
packages/host-service/src/app.ts:215-220
**`pullRequestRuntime.stop()` is now fire-and-forget on shutdown**
Before #4500, `stop()` was synchronous (no background tasks to drain). The reverted `PullRequestRuntimeManager.stop()` still clears timers and the git-watcher subscription synchronously, so new events stop arriving — but any `syncWorkspaceBranches` or `refreshEligibleProjects` coroutines already in flight (`void`-dispatched) will continue running against the database after the shutdown sequence moves on to the next cleanup step. If those tasks write rows while `ownsDb` teardown is in progress there is a window for a write-after-close error on the SQLite handle.
### Issue 3 of 3
packages/host-service/src/trpc/router/project/utils/ensure-main-workspace.ts:57-80
**Concurrent calls to `ensureMainWorkspaceStrict` can now race**
The `mainWorkspaceEnsuresInFlight` deduplication map from #4500 is removed. If two requests for the same `(projectId, repoPath)` reach `ensureMainWorkspaceStrict` concurrently (e.g. the startup sweep and a user-triggered project-creation handler both firing), both will pass the "does a main workspace exist?" check before either commits, and both will call `ctx.api.v2Workspace.create.mutate`. The DB unique index on `(projectId, hostId) WHERE type='main'` will reject the second insert, surfacing an unhandled error from `ensureMainWorkspaceStrict` (the strict variant does not swallow errors). The non-strict wrapper `ensureMainWorkspace` will catch it and log a warning, but any caller that relied on `ensureMainWorkspaceStrict` succeeding (e.g. the project-creation saga in `handlers.ts`) will receive an unexpected rejection.
Reviews (1): Last reviewed commit: "Revert "[codex] Fix renderer stress degr..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
189
to
230
| @@ -363,15 +224,12 @@ export function TabBar<TData>({ | |||
| /> | |||
| )} | |||
| {!hasHorizontalOverflow && ( | |||
| <div | |||
| className="absolute top-0 flex h-full w-10 items-center justify-center" | |||
| style={{ left: totalTabsWidth }} | |||
| > | |||
| <div className="flex h-full w-10 shrink-0 items-center justify-center"> | |||
| <AddTabButton renderAddTabMenu={renderAddTabMenu} /> | |||
| </div> | |||
| )} | |||
There was a problem hiding this comment.
The revert removes the useLayoutEffect that computed nextScrollLeft and imperatively set scrollContainerRef.current.scrollLeft whenever activeTabIndex changed. With OverflowFadeContainer, the tab list is a regular flex row with native overflow-x: auto, so the active tab stays at whatever scroll position it already occupies. If a user has 12+ tabs and switches to one that is off-screen (e.g. via keyboard shortcut or programmatic tab selection), the newly active tab will not scroll into view — users will see an empty-looking area with no visible selection indicator until they scroll manually.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/panes/src/react/components/Workspace/components/TabBar/TabBar.tsx
Line: 189-230
Comment:
**Active tab no longer scrolls into view on overflow**
The revert removes the `useLayoutEffect` that computed `nextScrollLeft` and imperatively set `scrollContainerRef.current.scrollLeft` whenever `activeTabIndex` changed. With `OverflowFadeContainer`, the tab list is a regular flex row with native `overflow-x: auto`, so the active tab stays at whatever scroll position it already occupies. If a user has 12+ tabs and switches to one that is off-screen (e.g. via keyboard shortcut or programmatic tab selection), the newly active tab will not scroll into view — users will see an empty-looking area with no visible selection indicator until they scroll manually.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
215
to
220
| console.warn("[host-service] revokeAllSessions failed:", err); | ||
| } | ||
| try { | ||
| await pullRequestRuntime.stop(); | ||
| pullRequestRuntime.stop(); | ||
| } catch (err) { | ||
| console.warn("[host-service] pullRequestRuntime.stop failed:", err); |
There was a problem hiding this comment.
pullRequestRuntime.stop() is now fire-and-forget on shutdown
Before #4500, stop() was synchronous (no background tasks to drain). The reverted PullRequestRuntimeManager.stop() still clears timers and the git-watcher subscription synchronously, so new events stop arriving — but any syncWorkspaceBranches or refreshEligibleProjects coroutines already in flight (void-dispatched) will continue running against the database after the shutdown sequence moves on to the next cleanup step. If those tasks write rows while ownsDb teardown is in progress there is a window for a write-after-close error on the SQLite handle.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/host-service/src/app.ts
Line: 215-220
Comment:
**`pullRequestRuntime.stop()` is now fire-and-forget on shutdown**
Before #4500, `stop()` was synchronous (no background tasks to drain). The reverted `PullRequestRuntimeManager.stop()` still clears timers and the git-watcher subscription synchronously, so new events stop arriving — but any `syncWorkspaceBranches` or `refreshEligibleProjects` coroutines already in flight (`void`-dispatched) will continue running against the database after the shutdown sequence moves on to the next cleanup step. If those tasks write rows while `ownsDb` teardown is in progress there is a window for a write-after-close error on the SQLite handle.
How can I resolve this? If you propose a fix, please make it concise.
This was referenced May 14, 2026
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.
Reverts #4500
Summary by cubic
Reverts the renderer stress tooling and related optimizations. Removes stress scripts and experimental code paths, simplifies terminal/runtime logic, and drops the
git.getDiffStatsAPI in favor of existing status calls.Refactors
SUPERSET_RENDERER_STRESS_CDP_PORTdev flag fromapps/desktop.@xtermaddons directly.git.getDiffStatsand related helpers/tests; simplified PR runtime lifecycle and main workspace ensure logic; usesimple-gitdirectly for config writes.Migration
git.getDiffStatscalls withgit.getStatus(theuseDiffStatshook now proxies status).SUPERSET_RENDERER_STRESS_CDP_PORTfrom local envs.useGitStatus(workspaceId),useHybridSearch(tasks),useOpenInExternalEditor(workspaceId),useChangeset({ workspaceId, ref }),useSidebarDiffRef(workspaceId),useReviewTab({ workspaceId, ... }),useChangesTab({ workspaceId, ... }).navigateToV2Workspace(id, navigate)directly; pending navigation state is gone.Written for commit 127fd18. Summary will update on new commits.
Summary by CodeRabbit
Release Notes