Repository navigation
fix(desktop): open in IDE broken in v2 workspaces — isolate workspaceTrpc React context - #5877
Conversation
…k in v2 workspaces Every createTRPCReact client shares @trpc/react-query's default React context, so workspaceTrpc.Provider shadowed the electron IPC provider for any electronTrpc hook rendered under it. PR #5824 moved the open-in button, window controls, and a getPlatform query into that subtree, sending their calls to the host-service router instead ("No procedure found on path external.openInApp"). Give workspaceTrpc a dedicated context so the two providers can't collide, and make useV2FileSearch provider-optional (plain useQuery + imperative client) since the v1 CommandPalette mounts it with no WorkspaceClientProvider, where a workspaceTrpc hook would now throw. Fixes SUPER-1599. Claude-Session: https://claude.ai/code/session_018igf49z4TsXxgbdKH6t3Nf
📝 WalkthroughWalkthroughWorkspace file search now queries through an optional workspace client and isolated TRPC context. The TRPC package test script and several router test suites are removed, while the workspace layout change only removes an explanatory comment. ChangesWorkspace file search integration
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant CommandPalette
participant useV2FileSearch
participant WorkspaceClient
participant WorkspaceTRPC
CommandPalette->>useV2FileSearch: provide workspaceId and query
useV2FileSearch->>WorkspaceClient: retrieve optional client
useV2FileSearch->>WorkspaceTRPC: searchFiles.query(workspaceId, trimmedQuery, SEARCH_LIMIT)
WorkspaceTRPC-->>useV2FileSearch: return matches
useV2FileSearch-->>CommandPalette: return query results
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
…a root .env The real client calls neon(env.DATABASE_URL) at import; CI has no root .env, so any uncached run where the module loads failed all four suites. Preload dummy URLs via bunfig test-setup. Also trim comments. Claude-Session: https://claude.ai/code/session_018igf49z4TsXxgbdKH6t3Nf
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packages/trpc/test-setup.ts`:
- Around line 2-4: Update the DATABASE_URL and DATABASE_URL_UNPOOLED assignments
in the test setup to use ||= instead of ??=, so empty environment variables
receive the existing test database fallback while preserving non-empty
configured values.
🪄 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: a96e3853-f693-43ad-a59c-62538065de74
📒 Files selected for processing (4)
apps/desktop/src/renderer/screens/main/components/CommandPalette/hooks/useV2FileSearch/useV2FileSearch.tspackages/trpc/bunfig.tomlpackages/trpc/test-setup.tspackages/workspace-client/src/workspace-trpc.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/workspace-client/src/workspace-trpc.ts
- apps/desktop/src/renderer/screens/main/components/CommandPalette/hooks/useV2FileSearch/useV2FileSearch.ts
| process.env.DATABASE_URL ??= "postgresql://test:test@localhost:5432/test"; | ||
| process.env.DATABASE_URL_UNPOOLED ??= | ||
| "postgresql://test:test@localhost:5432/test"; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Use ||= so empty environment variables receive the fallback.
packages/db/src/env.ts converts empty strings to undefined, but ??= leaves "" unchanged. This can still cause @superset/db/client to initialize without a usable URL when CI defines these variables as empty.
Proposed fix
-process.env.DATABASE_URL ??= "postgresql://test:test@localhost:5432/test";
-process.env.DATABASE_URL_UNPOOLED ??=
+process.env.DATABASE_URL ||= "postgresql://test:test@localhost:5432/test";
+process.env.DATABASE_URL_UNPOOLED ||=
"postgresql://test:test@localhost:5432/test";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| process.env.DATABASE_URL ??= "postgresql://test:test@localhost:5432/test"; | |
| process.env.DATABASE_URL_UNPOOLED ??= | |
| "postgresql://test:test@localhost:5432/test"; | |
| process.env.DATABASE_URL ||= "postgresql://test:test@localhost:5432/test"; | |
| process.env.DATABASE_URL_UNPOOLED ||= | |
| "postgresql://test:test@localhost:5432/test"; |
🤖 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 `@packages/trpc/test-setup.ts` around lines 2 - 4, Update the DATABASE_URL and
DATABASE_URL_UNPOOLED assignments in the test setup to use ||= instead of ??=,
so empty environment variables receive the existing test database fallback while
preserving non-empty configured values.
The four suites stubbed the entire drizzle chain, so they mostly asserted their own mocks and failed in CI whenever the real @superset/db/client (which throws at import without a DATABASE_URL) loaded. Remove them and the preload shim; drop the package test script so an empty suite doesn't fail the run. Claude-Session: https://claude.ai/code/session_018igf49z4TsXxgbdKH6t3Nf
…host CLI ParadevAI#5885, Cursor session labels ParadevAI#5882, open-in-IDE v2 ParadevAI#5877, collapsed sidebar DnD ParadevAI#5871, automations FK drop ParadevAI#5878) # Conflicts: # apps/desktop/src/main/lib/agent-setup/notify-hook.test.ts
…host CLI ParadevAI#5885, Cursor session labels ParadevAI#5882, open-in-IDE v2 ParadevAI#5877, collapsed sidebar DnD ParadevAI#5871, automations FK drop ParadevAI#5878) # Conflicts: # apps/desktop/src/main/lib/agent-setup/notify-hook.test.ts
Summary
Fixes SUPER-1599: "Open in IDE" failing with
No procedure found on path "external.openInApp"since the latest release.Root cause: every
createTRPCReact()client shares@trpc/react-query's default React context, soworkspaceTrpc.Provider(mounted by the v2WorkspaceProvider) shadowed the electron IPC provider for anyelectronTrpchook rendered under it — the hook silently resolved the host-service HTTP client, which has noexternalrouter. #5824 exposed this by moving the open-in button, window controls, and agetPlatformquery into the v2 workspace subtree. The same mechanism also leftplatformundefined in the v2 page (hiding window controls on Windows/Linux).Fix (structural, not per-callsite):
workspaceTrpcits own React context viacreateTRPCReact({ context }), so the two providers can never collide. AllelectronTrpchooks now work anywhere in the tree; feat(desktop): reorganize sidebar and workspace header chrome #5824's moved components need no changes.useV2FileSearchwas the one bareworkspaceTrpchook outside anyWorkspaceClientProvider(v1 CommandPalette mounts) — it previously coasted on the shared context and would throw under the isolated one. Converted to a plainuseQueryover the provider's imperative client via a newuseMaybeWorkspaceClient(), which returns null outside the provider.Audited all other
workspaceTrpchook call sites: everything else renders under aWorkspaceClientProvider(v2 workspace tree, or self-wrapping dialogs/settings sections).Test Plan
bun run lintexits 0bun run typecheckpasses (35/35)https://claude.ai/code/session_018igf49z4TsXxgbdKH6t3Nf
Summary by cubic
Fixes SUPER-1599: “Open in IDE” in v2 workspaces by isolating the
workspaceTrpcReact context soelectronTrpchooks resolve correctly. Keeps v1 CommandPalette file search working without aWorkspaceClientProviderand restores window controls on Windows/Linux.workspaceTrpcits own React context viacreateTRPCReact({ context }), preventing collisions withelectronTrpc; fixes “No procedure found on path 'external.openInApp'” and restores platform detection in v2.useV2FileSearchto a plainuseQueryover the provider’s imperative client; addeduseMaybeWorkspaceClient()to safely run outside aWorkspaceClientProvider.packages/trpcrouter tests and dropped the packagetestscript to stop CI failures when@superset/db/clientloads without a rootDATABASE_URL.Written for commit f8fae95. Summary will update on new commits.
Summary by CodeRabbit