fix(desktop): consolidate v2 workspace context behind a single provider#4067
Merged
Conversation
The v2 workspace tree had two independent useLiveQuery lookups for the
workspace — one in the layout (driving WorkspaceTrpcProvider mounting)
and one in the page (driving WorkspaceContent rendering). During a
workspaceId change they could disagree for a render: the layout would
strip the provider while the page still rendered TerminalPane, and
useWorkspaceClient threw "must be used within WorkspaceClientProvider".
Layout now owns the existence check (creating / error / not-found
states render directly, no Outlet without a provider). A new
WorkspaceProvider takes the resolved workspace, derives hostUrl once
from useLocalHostService, and mounts WorkspaceTrpcProvider with a key
of `${workspace.id}:${hostUrl}` — so the same component decides what
to render and what context to provide.
Descendants now read workspace + hostUrl via useWorkspace() instead of
accepting workspaceId as a prop. This eliminates the same drift class
at the inner-hook layer (a future caller could otherwise pass a stale
URL param while the trpc client is bound to a different workspace).
Contributor
📝 WalkthroughWalkthroughA ChangesWorkspace Context and Provider Foundation
Hook Refactors
Component and State Provider Updates
Sequence DiagramsequenceDiagram
participant Layout as V2WorkspaceLayout
participant Provider as WorkspaceProvider
participant Page as V2WorkspacePage
participant Hooks as Workspace Hooks<br/>(usePaneRegistry, etc.)
participant Context as WorkspaceContext
Layout->>Layout: Query workspace via useLiveQuery
Layout->>Provider: Render children wrapped
Provider->>Context: Compute hostUrl, provide workspace & hostUrl
Provider->>Page: Render page under provider
Page->>Context: Call useWorkspace() to get workspace
Context-->>Page: Return workspace object
Page->>Hooks: Call hooks (now param-less or reduced params)
Hooks->>Context: Each hook calls useWorkspace()
Context-->>Hooks: Return workspace for deriving workspaceId
Hooks-->>Page: Return hook results using derived workspace.id
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes The refactoring follows a consistent pattern across multiple hooks (reducing cognitive overhead), but the PR spans several distinct layers—provider creation, layout integration, page refactoring, and six separate hook updates—each requiring careful verification of context consumption and parameter removal. Logic density is moderate; the changes are safe and mechanical in nature but distributed across enough files and different file types (providers, hooks, components) to warrant careful attention to context correctness and hook dependencies. Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Contributor
Greptile SummaryThis PR consolidates the v2 workspace's provider hierarchy by introducing a new Confidence Score: 4/5Safe to merge — the crash is correctly fixed and no new logic bugs are introduced. The architectural change is sound: the layout now guarantees WorkspaceTrpcProvider is always mounted before any page content renders, closing the race condition. All P2 findings are minor (a missing loading state during host-url resolution, and a pre-existing redundant query). No P0 or P1 issues found. WorkspaceProvider.tsx — the blank-div early return when activeHostUrl is null has no user feedback; worth a follow-up polish pass.
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/providers/WorkspaceProvider/WorkspaceProvider.tsx | New file — defines WorkspaceContext and co-locates WorkspaceTrpcProvider; context is placed outside the keyed trpc provider, which is safe but has a minor blank-div gap when activeHostUrl is null |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/layout.tsx | Layout now owns all workspace lifecycle states (creating/error/not-found) and only renders WorkspaceProvider + Outlet when a valid workspace row exists; ensureWorkspaceInSidebar consolidated here with a lastEnsuredWorkspaceIdRef guard |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsx | Significantly simplified — duplicate workspace lookup and state rendering removed; pane store reset still works via WorkspaceTrpcProvider's key prop upstream |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useWorkspaceFileNavigation/useWorkspaceFileNavigation.ts | Removed workspaceId prop, reads from useWorkspace(); still fires a redundant workspaceTrpc.workspace.get query for worktreePath when the full workspace object is already in context |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2WorkspacePaneLayout/useV2WorkspacePaneLayout.ts | Removed projectId/workspaceId params and ensureWorkspaceInSidebar call (moved to layout); now reads workspace from context |
Sequence Diagram
sequenceDiagram
participant Router
participant Layout as V2WorkspaceLayout
participant WP as WorkspaceProvider
participant Page as V2WorkspacePage
Router->>Layout: navigate to workspace route
Layout->>Layout: useLiveQuery for workspace row
alt workspace not found or in-flight
Layout-->>Router: WorkspaceNotFoundState / WorkspaceCreatingState
else workspace resolved
Layout->>WP: render WorkspaceProvider
WP->>WP: derive hostUrl from useLocalHostService
WP->>WP: mount WorkspaceContext.Provider + WorkspaceTrpcProvider
WP->>Page: render Outlet
Page->>WP: useWorkspace() reads context
Page->>WP: workspaceTrpc hooks use trpc client
end
Note over Layout,Page: Workspace switch triggers new key on WorkspaceTrpcProvider, remounting Page and all child hooks atomically
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/providers/WorkspaceProvider/WorkspaceProvider.tsx:35-37
**Silent blank div when `activeHostUrl` is null**
When the workspace is local (`workspace.hostId === machineId`) but `activeHostUrl` hasn't resolved yet, `hostUrl` is falsy and `WorkspaceProvider` returns an unstyled blank div with no loading indicator or user feedback. The layout only guards `!workspace` before delegating to `WorkspaceProvider`, so this path is reachable during normal initialization. Consider forwarding a loading/pending state upward or rendering a skeleton here rather than an empty `<div>`.
### Issue 2 of 2
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useWorkspaceFileNavigation/useWorkspaceFileNavigation.ts:41-44
**Redundant trpc query for data already in context**
`useWorkspace()` now provides a full `SelectV2Workspace` row (fetched via `useLiveQuery` in the layout). If `SelectV2Workspace` includes `worktreePath`, `workspaceTrpc.workspace.get.useQuery` is making an extra round-trip to the host service to retrieve a value already available as `workspace.worktreePath`. Worth checking whether the trpc query is needed for freshness or whether the context value suffices.
Reviews (1): Last reviewed commit: "fix(desktop): consolidate v2 workspace c..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+35
to
+37
| if (!hostUrl) { | ||
| return <div className="flex h-full w-full" />; | ||
| } |
Contributor
There was a problem hiding this comment.
activeHostUrl is null
When the workspace is local (workspace.hostId === machineId) but activeHostUrl hasn't resolved yet, hostUrl is falsy and WorkspaceProvider returns an unstyled blank div with no loading indicator or user feedback. The layout only guards !workspace before delegating to WorkspaceProvider, so this path is reachable during normal initialization. Consider forwarding a loading/pending state upward or rendering a skeleton here rather than an empty <div>.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/providers/WorkspaceProvider/WorkspaceProvider.tsx
Line: 35-37
Comment:
**Silent blank div when `activeHostUrl` is null**
When the workspace is local (`workspace.hostId === machineId`) but `activeHostUrl` hasn't resolved yet, `hostUrl` is falsy and `WorkspaceProvider` returns an unstyled blank div with no loading indicator or user feedback. The layout only guards `!workspace` before delegating to `WorkspaceProvider`, so this path is reachable during normal initialization. Consider forwarding a loading/pending state upward or rendering a skeleton here rather than an empty `<div>`.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
41
to
+44
| recentFiles: RecentFile[]; | ||
| openFilePaths: Set<string>; | ||
| } { | ||
| const { workspace } = useWorkspace(); |
Contributor
There was a problem hiding this comment.
useWorkspace() now provides a full SelectV2Workspace row (fetched via useLiveQuery in the layout). If SelectV2Workspace includes worktreePath, workspaceTrpc.workspace.get.useQuery is making an extra round-trip to the host service to retrieve a value already available as workspace.worktreePath. Worth checking whether the trpc query is needed for freshness or whether the context value suffices.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useWorkspaceFileNavigation/useWorkspaceFileNavigation.ts
Line: 41-44
Comment:
**Redundant trpc query for data already in context**
`useWorkspace()` now provides a full `SelectV2Workspace` row (fetched via `useLiveQuery` in the layout). If `SelectV2Workspace` includes `worktreePath`, `workspaceTrpc.workspace.get.useQuery` is making an extra round-trip to the host service to retrieve a value already available as `workspace.worktreePath`. Worth checking whether the trpc query is needed for freshness or whether the context value suffices.
How can I resolve this? If you propose a fix, please make it concise.![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/layout.tsx`:
- Around line 51-74: The current guard returns the blank placeholder before
evaluating inFlight, causing create/error states to be hidden; change the order
so you first check workspaceId, then if no workspace evaluate inFlight states
(inspect inFlight?.state for "creating" or "error" and render
WorkspaceCreatingState, WorkspaceCreateErrorState, or WorkspaceNotFoundState
using inFlight.snapshot/startedAt/error/workspaceId), and only after handling
inFlight fall back to the query-readiness check (isReady/workspaces) to render
the blank loading container; update the logic around workspaceId, isReady,
workspaces, workspace, and inFlight in layout.tsx accordingly.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/providers/WorkspaceProvider/WorkspaceProvider.tsx`:
- Around line 35-37: The current readiness branch returns an empty container
when hostUrl is falsy which unmounts the workspace subtree; instead preserve the
last known good host URL so descendants stay mounted. In WorkspaceProvider (the
component using useLocalHostService()/activeHostUrl and the hostUrl variable)
add a local state or ref (e.g., lastHostUrl) that is initialized from hostUrl
and only updates when hostUrl is non-empty, then render the provider subtree
using lastHostUrl (or keep the readiness UI in the layout) so transient drops of
hostUrl do not unmount children.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f8cd2831-6ea4-4584-90da-8b5413f2ce94
📒 Files selected for processing (13)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/V2NotificationStatusIndicator/V2NotificationStatusIndicator.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useClearActivePaneAttention/useClearActivePaneAttention.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useDirtyTabCloseGuard/useDirtyTabCloseGuard.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/ChatPaneTitle/ChatPaneTitle.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2PresetExecution/useV2PresetExecution.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2WorkspacePaneLayout/useV2WorkspacePaneLayout.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useWorkspaceFileNavigation/useWorkspaceFileNavigation.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/state/fileDocumentStore/FileDocumentStoreProvider.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/layout.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/providers/WorkspaceProvider/WorkspaceProvider.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/providers/WorkspaceProvider/index.ts
💤 Files with no reviewable changes (1)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/ChatPaneTitle/ChatPaneTitle.tsx
Comment on lines
+51
to
+74
| if (!workspaceId || !isReady || !workspaces) { | ||
| return <div className="flex h-full w-full" />; | ||
| } | ||
|
|
||
| if (!workspace || !hostUrl) { | ||
| return <Outlet />; | ||
| if (!workspace) { | ||
| if (inFlight?.state === "creating") { | ||
| return ( | ||
| <WorkspaceCreatingState | ||
| name={inFlight.snapshot.name} | ||
| branch={inFlight.snapshot.branch} | ||
| startedAt={inFlight.startedAt} | ||
| /> | ||
| ); | ||
| } | ||
| if (inFlight?.state === "error") { | ||
| return ( | ||
| <WorkspaceCreateErrorState | ||
| workspaceId={workspaceId} | ||
| name={inFlight.snapshot.name} | ||
| error={inFlight.error ?? "Unknown error"} | ||
| /> | ||
| ); | ||
| } | ||
| return <WorkspaceNotFoundState workspaceId={workspaceId} />; |
Contributor
There was a problem hiding this comment.
Render the in-flight create/error states before the query-readiness placeholder.
Line 51 short-circuits to the blank container before the inFlight branches run. In the create-from-task flow, the workspace-create store can already know this workspace is "creating" or "error" while useLiveQuery() is still warming up, so users get a blank page instead of the new state component. Check workspaceId first, then prefer inFlight states before falling back to the query-loading placeholder.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/layout.tsx`
around lines 51 - 74, The current guard returns the blank placeholder before
evaluating inFlight, causing create/error states to be hidden; change the order
so you first check workspaceId, then if no workspace evaluate inFlight states
(inspect inFlight?.state for "creating" or "error" and render
WorkspaceCreatingState, WorkspaceCreateErrorState, or WorkspaceNotFoundState
using inFlight.snapshot/startedAt/error/workspaceId), and only after handling
inFlight fall back to the query-readiness check (isReady/workspaces) to render
the blank loading container; update the logic around workspaceId, isReady,
workspaces, workspace, and inFlight in layout.tsx accordingly.
Comment on lines
+35
to
+37
| if (!hostUrl) { | ||
| return <div className="flex h-full w-full" />; | ||
| } |
Contributor
There was a problem hiding this comment.
Don't tear down the workspace subtree when hostUrl is briefly unavailable.
Line 35 returns a blank container instead of keeping the provider tree mounted. If useLocalHostService() transiently drops activeHostUrl during local-host discovery/reconnect, this unmounts the entire workspace after the layout has already resolved it, which can reset pane/document state. Keep this readiness branch in the layout, or preserve the last resolved hostUrl so descendants stay mounted.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/providers/WorkspaceProvider/WorkspaceProvider.tsx`
around lines 35 - 37, The current readiness branch returns an empty container
when hostUrl is falsy which unmounts the workspace subtree; instead preserve the
last known good host URL so descendants stay mounted. In WorkspaceProvider (the
component using useLocalHostService()/activeHostUrl and the hostUrl variable)
add a local state or ref (e.g., lastHostUrl) that is initialized from hostUrl
and only updates when hostUrl is non-empty, then render the provider subtree
using lastHostUrl (or keep the readiness UI in the layout) so transient drops of
hostUrl do not unmount children.
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
MocA-Love
added a commit
to MocA-Love/superset
that referenced
this pull request
May 21, 2026
superset-sh#4101 import agents as v2 terminal presets with live link: - useV2PresetExecution.ts: drop upstream useV2AgentConfigs + useWorkspace import. useWorkspace arrives with superset-sh#4067 (cycle 44a WorkspaceProvider); fork keeps its workspaceId/projectId props until then. - V2AgentsSettings.tsx: adopt upstream initialAgentPresetId prop and useV2AgentConfigs re-import (fork retains the hook elsewhere). - PresetEditorDialog.tsx / V2PresetsSection.tsx: adopt upstream Switch / Link / ExternalLink additions for live-link UX. - useMigrateV1PresetsToV2/: re-delete fork-removed hook; drop import from V2PresetsSection.tsx (call sites were stub-only). - PresetEditorDialog.tsx: dedupe Switch import (one survived from each side of the merge). superset-sh#4107 v2 preset commands in new tabs (skipped): introduces useV2TerminalLauncher hook that takes a launcher prop through useV2PresetExecution / useDefaultContextMenuActions / useDefaultPaneActions / useWorkspacePaneOpeners. This pattern is tightly coupled to superset-sh#4067's WorkspaceProvider (workspaceId/projectId stop being passed down by page.tsx and instead come from useWorkspace()). Deferred to cycle 44a so the launcher and provider land together. superset-sh#4112 include agent args (skipped): depends on superset-sh#4107. superset-sh#4057 / superset-sh#4083 fix-compat: - RunInWorkspacePopoverV2.tsx / OpenInWorkspaceV2.tsx: drop iconUrl from project map and ProjectThumbnail props; pass githubOwner={null} until cycle 41 (superset-sh#3823 v2 project icon settings) lands. Phase 4 cycle 39 net: 3 cherry-picks landed (superset-sh#4057, superset-sh#4083, superset-sh#4101); 2 deferred to cycle 44a (superset-sh#4107, superset-sh#4112). useV2PresetExecution preserves fork's workspaceId/projectId calling convention.
MocA-Love
added a commit
to MocA-Love/superset
that referenced
this pull request
May 21, 2026
…perset-sh#4067 adoption) Take only the WorkspaceProvider scaffolding from upstream superset-sh#4067, and keep all inner v2-workspace hooks / page.tsx on the fork's existing workspaceId-props signature. Full superset-sh#4067 (drift-class fix at the inner-hook layer) requires migrating page.tsx + 8 hooks to useWorkspace() in lockstep, which would also delete ~1000 lines of fork-only WorkspaceContent logic (createWorkspaceMemo, browser shortcut bridge, hotkey labels, custom command palette wiring). That migration is deferred to a future cycle 44c. layout.tsx changes: - Drop the inline hostUrl derivation (useLocalHostService + buildHostRoutingKey + getHostServiceHeaders/WsToken). Those move into WorkspaceProvider, which now owns workspace + hostUrl context for the subtree. - Preserve fork's syncedWorkspace ?? inFlight.cloudRow fallback so the v2 create-flow still works during the Electric sync race. - Preserve fork's WorkspaceHostOfflineState / WorkspaceHostIncompatibleState / useRemoteHostStatus gates so the upstream WorkspaceProvider is only mounted when the remote host is reachable. - Adopt upstream's WorkspaceCreatingState / WorkspaceCreateErrorState / WorkspaceNotFoundState branches via the in-flight tracker. - Final return becomes <WorkspaceProvider workspace={workspace}><Outlet /></WorkspaceProvider>. WorkspaceProvider/WorkspaceProvider.tsx + index.ts: pristine upstream copy. Provides { workspace, hostUrl } context and wraps WorkspaceTrpcProvider internally. useWorkspace() is now available to any descendant. No other files touched (cycle 44c will migrate inner hooks).
MocA-Love
added a commit
to MocA-Love/superset
that referenced
this pull request
May 21, 2026
…uperset-sh#4067 完成) Complete the superset-sh#4067 (consolidate v2 workspace context behind a single provider) migration started in cycle 44a. The 7 internal hooks / components that previously took workspaceId / projectId props now read from useWorkspace() in the WorkspaceProvider tree. Files migrated (upstream verbatim overwrite — no fork-specific logic in any of them): - V2NotificationStatusIndicator: drop workspaceId prop, use useWorkspace - useClearActivePaneAttention: drop workspaceId param - useDirtyTabCloseGuard: drop workspaceId param - ChatPaneTitle: drop workspaceId pass-through to V2NotificationStatusIndicator - useV2WorkspacePaneLayout: drop projectId / workspaceId params + drop the ensureWorkspaceInSidebar effect (layout.tsx already owns it since cycle 44a). FORK NOTE: restore localWorkspaceState in the return value (upstream dropped it but the fork's page.tsx still reads localWorkspaceState.rightSidebarOpen for the right-sidebar portal-slot wiring from cycle 09). - useWorkspaceFileNavigation: drop workspaceId param - FileDocumentStoreProvider: drop workspaceId prop usePaneRegistry.tsx: hand-merged. Adopted upstream's new signature ({ onOpenFile, onRevealPath }: UsePaneRegistryOptions) + `const { workspace } = useWorkspace(); const workspaceId = workspace.id;` shim at the top of the body so all downstream usages of workspaceId keep working. Preserved fork-only: - FilePaneTabTitleStatic spreadsheet bypass - FilePaneTabTitleWithDocument displayName prop (memo title fallback) - 'FORK NOTE: v2 desktop runs on Windows too' comment on getFileName - 'FORK NOTE: memo files carry a derived displayName' comment - displayName / pinned / isActive prop wiring Adopted upstream-only: - listSessions.invalidate({ workspaceId }) targeted invalidation - V2NotificationStatusIndicator no-workspaceId callsite page.tsx: drop the inlined useClearActivePaneAttention definition (~32 lines) and import the hook from ./hooks/useClearActivePaneAttention, switch all hook call sites to the new prop-less signatures, and drop workspaceId props from <V2NotificationStatusIndicator> and <FileDocumentStoreProvider>. The fork-only ~1000-line WorkspaceContent logic (createWorkspaceMemo / browser-shortcut bridge / hotkey labels / custom command palette wiring / scratch/tearoff route handling) is untouched. After this cycle, useWorkspace() is the canonical workspace context source inside v2-workspace/$workspaceId, and the drift class superset-sh#4067 exists to fix is resolved at every layer where workspaceId travels.
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.
Summary
useWorkspaceClient must be used within WorkspaceClientProvider. The v2 layout and the workspace page each ran their ownuseLiveQueryfor the workspace; during a switch they could disagree for a render — the layout would stripWorkspaceTrpcProviderwhile the page still renderedWorkspaceContent.WorkspaceProviderthat takes the resolved workspace, deriveshostUrlonce fromuseLocalHostService, and mountsWorkspaceTrpcProviderinternally. The layout owns existence: creating / error / not-found states render in the layout itself, never via<Outlet />without a provider.workspaceIdprops on inner hooks and v2-only components (useV2WorkspacePaneLayout,useV2PresetExecution,usePaneRegistry,useWorkspaceFileNavigation,useDirtyTabCloseGuard,useClearActivePaneAttention,V2NotificationStatusIndicator,FileDocumentStoreProvider) withuseWorkspace(), so future callers can't drift the trpc client from the workspaceId used in queries / storage keys.Test plan
useWorkspaceClientcrash./v2-workspace/<id>for a workspace that doesn't exist — seeWorkspaceNotFoundState.WorkspaceCreatingStateand transition into the workspace tree once the row lands.WorkspaceCreateErrorState.Summary by cubic
Consolidates v2 workspace context behind a single provider to fix workspace-switch crashes and keep the TRPC client in sync. The layout now mounts the provider and handles existence states to avoid mismatched renders.
Bug Fixes
<Outlet />without a provider.Refactors
WorkspaceProviderthat deriveshostUrlonce and mountsWorkspaceTrpcProviderwith a${workspace.id}:${hostUrl}key.workspaceIdprops withuseWorkspace()across v2 hooks/components (pane layout, presets, file nav, dirty-close guard, notifications,FileDocumentStoreProvider).WorkspaceProvider.Written for commit 5c8d1a5. Summary will update on new commits.
Summary by CodeRabbit