Repository navigation
🤖 refactor: handlers with their own offline message use the live client - #6323
Conversation
2955c19 to
e7ef248
Compare
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7ef248b2b
ℹ️ 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".
e7ef248 to
7205990
Compare
7205990 to
9643ff1
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9643ff1fae
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7bd9413b78
ℹ️ 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".
925eaca to
3812064
Compare
3812064 to
af9db5f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af9db5fc45
ℹ️ 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".
…client (bulk delete, picker, submit, compaction, review hydration, Coder disconnect, messaging settings)
af9db5f to
1e0c92b
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
…gates # Conflicts: # src/browser/features/Settings/Sections/ProvidersSection.tsx
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ea93b749aa
ℹ️ 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".
… 1a) After 1a's squash merge, the stack sits on main, where #6323 made OnboardingWizardSplash call useLiveAPI(). The test's API mock exported only useAPI, so every wizard handoff test threw "useLiveAPI is not a function". The mock now returns null for it, as useLiveAPI does with no API.
… 1a) After 1a's squash merge, the stack sits on main, where #6323 made OnboardingWizardSplash call useLiveAPI(). The test's API mock exported only useAPI, so every wizard handoff test threw "useLiveAPI is not a function". The mock now returns null for it, as useLiveAPI does with no API.
…oading (coder#6331) ## Summary This PR is layer 5a of the reconnect stack (base: coder#6323). Right-sidebar panes and dialogs now keep what they show when Xum reconnects. Each new client re-runs their load effects. Before this PR, many of them first cleared their data or swapped it for a spinner, so the pane blanked or flashed although nothing had changed. Now a re-run keeps the shown data, replaces it only on success, and ignores a load cut off by the lost connection. A change of the selected workspace, process or session still clears the old data. This is a declared behavior change before the switch (PR 6): it also stops the flash at today's reconnects. ## Implementation | Site | Change | |---|---| | `PlanFileDialog` | Split into `PlanFileBody` (keyed by workspace, mounted only while open) and a `usePlanFile` hook. Offline keeps the plan. A cut-off read shows no error. The markdown is keyed by content: Streamdown does not re-render a block whose position in the source is the same. | | `SubagentTranscriptDialog` | The viewer is keyed by `taskId` and starts loading. A cut-off read shows no error. | | `BackgroundBashOutputDialog` | No reset on a re-run: the first read replaces the output, later reads append. Keyed by `workspaceId:processId`. | | `useDevToolsSubscription`, `useWorkflowRuns`, `WorkflowsTab` (`useWorkflowLauncher`) | Runs, scripts and loading state reset only for another workspace. | | `useGoalBoard`, `GoalDefaultsSection` (`useWorkspaceGoalOverride`) | Offline keeps the board. The override controls are disabled offline (read-only), and a failed write shows its error. An edit made during a reload wins over the older answer. Another workspace clears the override (before, it showed workspace A's override under B). | | `ProjectMCPOverview` | A load cut off by the drop no longer clears the server list. | | `InstructionsTab`, `AdditionalSystemContextScratchpad` | The empty state no longer flashes while loading. The editor is disabled offline (read-only). A save cut off by the lost connection shows its error. | | `BrowserTab` | Offline keeps the discovered sessions and page tabs. A cut-off listing shows no error. | | `WorkflowRunHeader`, `DevToolsTab` | Interrupt, Resume, Retry, Re-run and Clear are disabled without a live client. An expanded run with no steps shows its spinner until the detail loads. | | `RightSidebar` (`useLlmDebugLogsEnabled`, `useDesktopAvailable`) | Offline keeps the last values. Before, a read cut off by the drop set `false`, which removed the Debug or Desktop tab from the saved layout. | Not changed: `ReviewPanel` (its loading flag drives only the refresh spinner), `useReadMore` (the shown context stays, the loading row replaces only the button), `OutputTab` (the listed line is a Delete handler, not a loader). The new hooks and child components also keep each function inside the CRAP gate. ## Validation - New both-mode tests (`null-gap` today, `kept-gap` after the switch), 25 tests: `dialogsReconnect.test.tsx`, `feedsReconnect.test.tsx`, `loadersReconnect.test.tsx`, `BrowserTab.reconnect.test.tsx`, `RightSidebar.reconnect.test.tsx`, plus `offlinePanes.test.tsx` (each kept pane is read-only while offline and editable again after the reconnect). Each shows data, drops, reconnects with a reload that is still pending or fails, and checks that the data stays, then that the new data shows. The identity tests check that another workspace or process clears the old data. - I reverted each production file and ran its test: each fails. The MCP overview test fails only in `kept-gap`: in `null-gap` the effect cleanup runs before the cut-off call settles. - `make static-check` and `make crap-check` pass. `bun test` on the touched folders: 1120 pass, 1 fail (`HighlightedCode.test.tsx`: "a mounted block highlights once a worker is ready after a start time-out"). That test also fails on origin/main on my host. ## Known gaps (follow-ups, tracked in coder#6383) - `BrowserTab`'s connect effect still disconnects the browser bridge when there is no client. - `useReadMore`'s `readFileLines(...).then` has no `.catch`. A drop during the read leaves an unhandled rejection and a stuck loading row. - In today's mode, the transcript and process dialogs show "API unavailable" above the kept content while offline. ## Risks Medium, limited to the right sidebar and its dialogs. The main risk is stale data under a new identity. The identity tests cover the workspace, process and session keys. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$81.58`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=81.58 --> --------- Signed-off-by: Thomas Kosiewski <tk@coder.com>
Summary
This PR is layer 4b of the reconnect stack (base: #6317). Handlers that show their own offline message now check the live client (
useLiveAPI()). Nothing changes today: the live client equalsuseAPI().apiuntil PR 6. After PR 6,useAPI().apikeeps the last client while Xum reconnects. Without this PR, these handlers would then call the lost connection and show "Not connected to the Xum server. Reconnecting…" fromConnectionLostError(or log an error), instead of their own message.Implementation
ChatInputsend and edit (and the voice input client),ProjectCreateModal(create, clone, add existing),DirectoryPickerModal,ArchivedWorkspacesdelete worktree,WorkspaceMenuBarfork and stop runtime,ProjectSidebarfork and stop runtime,AssistantMessagefork,RetryBarriermanual retry,useResumeStreamProvidersSection(Codex, Mux Gateway, Copilot and Coder logins, custom providers),useMCPOAuthLogin(it now reads the live client itself instead of takingapifrom its 3 callers),OnboardingWizardSplashuseArtifactInteractionssend,McpAppFrametool calls (itsapiRefnow holds the live client)LegacyPlanImportBanner,useBrowserBridgeConnection,ImmersiveReviewViewcopy,useCompactAndRetry(offline it puts the command in the composer),UserMessagestaged downloadapiRefholdersRetryBarrierandAskUserQuestionToolCallauto-retry rollback: skips offline, as it does today whenapiis nullNot changed:
ProvidersSection's unmount cancel ref: offline, the cancel call fails fast and nothing waits for it.useDesktopConnection: its failure path already backs off.Validation
ArtifactsReconnect.test.tsx, both gap modes: Send while offline shows "Xum is reconnecting. Send again once it is back." and sends nothing. Thekept-gapcase fails without this PR.useResumeStream.test.tsx, both gap modes: resume while offline shows "Not connected to server" and calls nothing. Thekept-gapcase fails without this PR.make static-checkandmake crap-checkpass.bun teston the touched folders passes, exceptWorkspaceShell.test.tsx(1 test) andGeneralSection.test.tsx(2 tests). These also fail on origin/main on my host.Risks
Low. Before PR 6 the live client and
useAPI().apiare the same object in every render (API.test.tsxpins this), so every changed handler behaves as before.Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:high• Cost:$55.49