feat(desktop): Docker サイドバー・検索強化・スプレッドシート改善・データベース設定編集など多数の機能追加#51
Merged
feat(desktop): Docker サイドバー・検索強化・スプレッドシート改善・データベース設定編集など多数の機能追加#51
Conversation
- Virtualize result list with @tanstack/react-virtual to avoid rendering all 500 items at once - Memoize SearchTreeNode, SearchFileGroup, SearchMatchItem with React.memo - Cache highlightSearchText output with useMemo per match item - Extract handleOpenFolderChange as useCallback to stabilize props for memoized components
…FilePreviewContent
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds workspace-scoped database definitions and encrypted credential storage, a Docker TRPC router and sidebar UI, a Cloudflare Worker deep-link service plus renderer open-link tooling/env, persistent webview runtime refactor, expanded search/replace tree UI, spreadsheet default-app integration, and multiple editor/path Superset link actions. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Worker as Cloudflare Worker
participant Main as Electron Main
participant DB as Local DB
participant Renderer as Renderer
User->>Worker: GET /open?... (click)
Worker->>Worker: build superset://open?... (preserve query)
Worker->>User: HTML response + redirect script (deep-link + fallback)
User->>Main: OS opens superset://open?... (Electron)
Main->>DB: query workspaces/projects to resolve repo/branch/file
Main->>Renderer: IPC/navigate path with resolved metadata
Renderer->>Renderer: open workspace viewer / file pane at line/column
sequenceDiagram
participant UI as Renderer UI
participant TRPC as TRPC Router
participant Workspace as Workspace Config
participant Postgres as Postgres Client
UI->>TRPC: discoverWorkspaceDatabases(worktreePath)
TRPC->>Workspace: loadWorkspaceDatabaseDefinitions(workspacePath)
Workspace-->>TRPC: return definitions + discovery items
UI->>TRPC: inspectPostgres({ connection: source })
TRPC->>Workspace: resolvePostgresConnectionStringFromSource(source)
TRPC->>Postgres: withPostgresClient(resolvedConnection, query...)
Postgres-->>TRPC: query results
TRPC-->>UI: results
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/desktop/src/renderer/stores/tabs/store.ts (1)
1757-1818:⚠️ Potential issue | 🟠 MajorNormalize the target URL before mutating browser history.
navigatePersistentWebview()sanitizes bare hosts/search text, but this branch records the rawurlinbrowser.currentUrlandhistoryfirst. For inputs likelocalhost:3000orexample.com, the laterupdateBrowserUrl()path sees a different committed URL and appends a second entry, so Back/Forward gets out of sync. If the normalized target is already loaded, the store can also stay on the raw value because no navigation event fires.Possible fix
-import { navigatePersistentWebview } from "renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/hooks/usePersistentWebview/runtime"; +import { + navigatePersistentWebview, + sanitizeUrl, +} from "renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/hooks/usePersistentWebview/runtime"; ... + const targetUrl = sanitizeUrl(url); const { history: prevHistory, historyIndex } = existingPane.browser; const history = prevHistory.slice(0, historyIndex + 1); history.push({ - url, + url: targetUrl, title: "", timestamp: Date.now(), }); ... browser: { ...existingPane.browser, - currentUrl: url, + currentUrl: targetUrl, history, historyIndex: history.length - 1, }, }, }; ... - navigatePersistentWebview(existingPane.id, url); + navigatePersistentWebview(existingPane.id, targetUrl);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/stores/tabs/store.ts` around lines 1757 - 1818, The openInBrowserPane branch mutates pane.browser.currentUrl and history with the raw input url before calling navigatePersistentWebview, causing double entries when navigatePersistentWebview normalizes URLs; fix by normalizing the target URL first (use the same normalization logic as navigatePersistentWebview/updateBrowserUrl) and then push that normalized URL into existingPane.browser.history and currentUrl so the stored history matches the committed navigation; update references in openInBrowserPane (existingPane.browser, history, historyIndex) to use the normalized value before calling navigatePersistentWebview.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/hooks/usePersistentWebview/usePersistentWebview.ts (1)
426-458:⚠️ Potential issue | 🟠 MajorParking the wrapper now drops the first browser events for hidden navigations.
The new reuse flow can call
navigatePersistentWebview()while this pane is still parked, but this cleanup detaches every webview listener before parking it. That guarantees the next hidden load missesdid-start-loading, and fast navigations can also lose title/favicon/error updates before the pane remounts. Either keep the listeners attached for the lifetime of the persistent webview and only remove them indestroyPersistentWebview(), or do a full state reconciliation when reclaiming.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/hooks/usePersistentWebview/usePersistentWebview.ts` around lines 426 - 458, The cleanup currently detaches all webview listeners (handleDomReady, handleDidStartLoading, handleDidStopLoading, handleDidNavigate, handleDidNavigateInPage, handlePageTitleUpdated, handlePageFaviconUpdated, handleDidFailLoad) before parking the wrapper in getPersistentWrapper/getHiddenContainer, which causes missed events for hidden navigations and fast loads triggered by navigatePersistentWebview; instead, remove the removeEventListener calls from this return cleanup (keep listeners attached while the persistent webview is parked) and move those listener removals into destroyPersistentWebview() so listeners remain for the lifetime of the persistent webview, or implement a full state reconciliation when reclaiming—update the code paths referencing navigatePersistentWebview, getPersistentWrapper, getHiddenContainer, and destroyPersistentWebview accordingly.
🧹 Nitpick comments (14)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/RepositoryPanel/RepositoryPanel.tsx (1)
306-333: Consider extracting sub-components to reduce complexity.This component has grown to ~1175 lines with 15+ state variables. The collapsible sections (Pull Requests, Workflows, Issue Composer) could be extracted into separate components to improve maintainability and testability. For example:
PullRequestsSectionfor lines 1016-1074WorkflowsSectionfor lines 1076-1168IssueComposerfor the issue creation formThis is an optional refactor that could be deferred.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/RepositoryPanel/RepositoryPanel.tsx` around lines 306 - 333, RepositoryPanel has grown large with 15+ state hooks (e.g., issueTitle, issueBody, pullRequestsOpen, workflowsOpen, issueComposerOpen, trackedWorkflowRuns) and multiple collapsible sections; extract the Pull Requests, Workflows and Issue Composer UI into separate components (e.g., PullRequestsSection, WorkflowsSection, IssueComposer) to reduce complexity. For each new component, move the related JSX and tightly-coupled state into the child (or lift minimal state to RepositoryPanel and pass as props/handlers), wire up handlers (e.g., setPullRequestsOpen, setWorkflowsOpen, setIssueTitle, setIssueBody, setIssueAssignees, setIssueLabels, setUploadedAssets, setTrackedWorkflowRuns) via props, and replace the original sections in RepositoryPanel with the new component imports so behavior remains identical. Ensure types for props reference existing types (UploadedIssueAsset, TrackedWorkflowRun) and preserve usages of electronTrpc/useTabsStore/trpcUtils by passing callbacks or utilities as needed.apps/desktop/src/lib/trpc/routers/docker/index.ts (2)
195-247: Consider adding a depth limit for directory traversal.The
findComposeFilesfunction recursively scans the workspace without a depth limit. While this is unlikely to cause issues in practice (most compose files are near the root), extremely deep directory structures could cause performance issues.♻️ Optional: Add depth tracking
+const MAX_COMPOSE_SEARCH_DEPTH = 10; + async function findComposeFiles( rootPath: string, ): Promise<ComposeFileSummary[]> { - const queue: string[] = [rootPath]; + const queue: Array<{ path: string; depth: number }> = [{ path: rootPath, depth: 0 }]; const composeFiles: ComposeFileSummary[] = []; while (queue.length > 0) { - const currentDir = queue.shift(); - if (!currentDir) { + const current = queue.shift(); + if (!current) { continue; } + const { path: currentDir, depth } = current; let entries: Dirent[]; try { entries = await readdir(currentDir, { withFileTypes: true }); } catch { continue; } for (const entry of entries) { if (entry.isSymbolicLink()) { continue; } const absolutePath = path.join(currentDir, entry.name); if (entry.isDirectory()) { - if (isIgnoredDirectory(entry.name)) { + if (isIgnoredDirectory(entry.name) || depth >= MAX_COMPOSE_SEARCH_DEPTH) { continue; } - queue.push(absolutePath); + queue.push({ path: absolutePath, depth: depth + 1 }); continue; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/docker/index.ts` around lines 195 - 247, The findComposeFiles function currently walks the directory tree without any depth limit; add a maxDepth (e.g., default to 6) and track depth during traversal by changing the queue to hold tuples/objects with {path, depth} (or add a depth parameter if you refactor to recursion) and only enqueue subdirectories when currentDepth < maxDepth; update references to findComposeFiles or provide a default param so callers remain unchanged and ensure the depth check is applied before pushing into queue (symbols: findComposeFiles, queue, isIgnoredDirectory).
499-507: JSON parse error may produce confusing error message.If
docker container inspectreturns malformed JSON (unlikely but possible),JSON.parse(stdout)will throw aSyntaxError. This gets caught and passed tonormalizeExecError, which will surfaceerror.message(e.g., "Unexpected token...") without context that it's a parsing issue.Consider wrapping with a more descriptive error:
♻️ Optional: Add explicit JSON parse error handling
const stdout = await execDocker( ["container", "inspect", "--format", "json", input.containerId], { cwd: getWorkspaceRootPath(input.workspaceId) }, ); - return JSON.parse(stdout) as unknown; + try { + return JSON.parse(stdout) as unknown; + } catch (parseError) { + throw new TRPCError({ + code: "INTERNAL_SERVER_ERROR", + message: "Failed to parse container inspection data", + }); + } } catch (error) { normalizeExecError(error); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/docker/index.ts` around lines 499 - 507, The JSON.parse(stdout) call after execDocker(["container", "inspect", ...], { cwd: getWorkspaceRootPath(input.workspaceId) }) can throw a SyntaxError which is currently caught and passed to normalizeExecError with no context; update the try/catch so parse errors are handled explicitly: wrap JSON.parse(stdout) in its own try/catch (or detect parse failure) and call normalizeExecError or rethrow a new Error that includes context like the containerId and that the failure was a JSON parse error (referencing execDocker, JSON.parse, input.containerId and normalizeExecError) so the surfaced message clearly indicates a parsing issue rather than a raw SyntaxError.apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/DockerView/DockerView.tsx (2)
374-405: Consider disabling buttons during any project mutation.The Start and Stop buttons for compose projects are only disabled when their respective mutations are pending. If a user clicks "Up" and then quickly clicks "Stop" before the first operation completes, both mutations could fire concurrently.
♻️ Optional: Disable both buttons during either mutation
<Button size="sm" variant="outline" className="h-7 px-2 whitespace-normal" onClick={() => startProjectMutation.mutate({ composeFilePath: group.absolutePath, workspaceId, }) } - disabled={startProjectMutation.isPending} + disabled={startProjectMutation.isPending || stopProjectMutation.isPending} > <LuPlay className="mr-1 size-3.5" /> Up </Button> <Button size="sm" variant="outline" className="h-7 px-2 whitespace-normal" onClick={() => stopProjectMutation.mutate({ composeFilePath: group.absolutePath, workspaceId, }) } - disabled={stopProjectMutation.isPending} + disabled={startProjectMutation.isPending || stopProjectMutation.isPending} > <LuSquareX className="mr-1 size-3.5" /> Stop </Button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/DockerView/DockerView.tsx` around lines 374 - 405, The Start and Stop buttons currently only check their own mutation pending state, allowing concurrent mutations; update both Button disabled props to disable when either startProjectMutation.isPending OR stopProjectMutation.isPending (i.e., use a shared condition like startProjectMutation.isPending || stopProjectMutation.isPending) so both buttons that call startProjectMutation.mutate and stopProjectMutation.mutate are disabled while any project mutation is in flight; locate the Button components around the startProjectMutation and stopProjectMutation calls (they use group.absolutePath and workspaceId) and apply the combined disabled condition.
249-273: Minor: Stale keys may accumulate inexpandedComposeGroups.The logic at line 267 checks if
Object.keys(previous).length === composeFiles.length, but this doesn't account for scenarios where a compose file is removed and a different one is added (same count but different keys). Thenextobject only contains current keys, but ifchangedis false and counts match,previous(with potentially stale keys) is returned.This doesn't cause functional issues since the stale keys are simply unused, but could be cleaned up for consistency. Consider simplifying by always returning
nextwhen the keys differ:♻️ Suggested simplification
setExpandedComposeGroups((previous) => { const next: Record<string, boolean> = {}; - let changed = false; for (const group of composeFiles) { const existing = previous[group.absolutePath]; next[group.absolutePath] = existing ?? true; - if (existing === undefined) { - changed = true; - } } - if (!changed && Object.keys(previous).length === composeFiles.length) { + // Return previous if keys and values are identical + const prevKeys = Object.keys(previous); + if ( + prevKeys.length === composeFiles.length && + prevKeys.every((k) => next[k] === previous[k]) + ) { return previous; } return next; });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/DockerView/DockerView.tsx` around lines 249 - 273, The updater for expandedComposeGroups can return a stale previous object when counts match but keys differ; in the setExpandedComposeGroups updater (inside DockerView's useEffect, using dockerListQuery.data?.composeFiles and variables composeFiles / previous / next), replace the current length-based short-circuit with a keys comparison or simply return next when the set of keys differs: build next as you do, then if Object.keys(previous).length === Object.keys(next).length and every key in next exists in previous return previous, otherwise return next (or always return next) to ensure stale keys are removed.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/SpreadsheetViewer/SpreadsheetViewer.tsx (1)
341-343: Keep the default-app action outside the success-only path.Because
isLoading/error/No sheets foundall return before this block, the new fallback action disappears exactly when the built-in preview cannot help. Hoisting the toolbar above those early returns would make the feature consistently available.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/SpreadsheetViewer/SpreadsheetViewer.tsx` around lines 341 - 343, The toolbar containing SpreadsheetDefaultAppButton is currently rendered after early returns (isLoading / error / "No sheets found") so the fallback action disappears; in the SpreadsheetViewer component hoist the toolbar div with SpreadsheetDefaultAppButton (preserving the absoluteFilePath prop) above the conditional return checks (isLoading, error, and the "No sheets found" branch) so it is rendered unconditionally while keeping the rest of the sheet-rendering logic intact.apps/open-link/package.json (1)
9-9: Redundant--emitDeclarationOnly falseflag.When using
--noEmit, the--emitDeclarationOnly falseflag is redundant since--noEmitalready prevents all output files including declarations.Suggested simplification
- "typecheck": "bunx tsc -p tsconfig.json --noEmit --emitDeclarationOnly false" + "typecheck": "bunx tsc -p tsconfig.json --noEmit"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/open-link/package.json` at line 9, The "typecheck" npm script includes a redundant flag "--emitDeclarationOnly false" alongside "--noEmit"; update the "typecheck" script in package.json (the "typecheck" entry) to remove the "--emitDeclarationOnly false" token so the command becomes bunx tsc -p tsconfig.json --noEmit, keeping the rest unchanged.apps/desktop/src/main/index.ts (2)
160-169: Redundant null check after guaranteed assignment.At line 165,
matchis assigned either the result ofcandidates.find(...)orcandidates[0]. Sincecandidates.length > 0is guaranteed by the check at line 156,candidates[0]will always exist. Therefore,matchcannot benullat line 167, making the check redundant.Suggested simplification
const match = (branchParam ? candidates.find( (candidate) => candidate.workspaceBranch === branchParam, ) : null) ?? candidates[0]; - if (!match) { - return null; - } - const params = new URLSearchParams();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/index.ts` around lines 160 - 169, The null check for match is redundant because match is assigned via (branchParam ? candidates.find(...) : null) ?? candidates[0], and candidates[0] is guaranteed to exist; remove the if (!match) { return null; } block and keep the existing assignment of match (using the nullish-coalescing fallback to candidates[0]) so downstream code can use match directly; reference symbols: match, branchParam, candidates.
128-154: Consider moving filter logic into the SQL query for better efficiency.The current implementation fetches all non-deleted workspaces and filters in JavaScript. For workspaces with many projects, moving the repo name and owner filtering into the SQL query (using
LIKEor computed columns) would reduce memory usage and improve performance.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/index.ts` around lines 128 - 154, The current post-query filter on candidates pulls all non-deleted workspaces and then filters by repo name and owner in JS; move that logic into the SQL query built from localDb.select to avoid loading extra rows. Update the query that selects from workspaces and innerJoin(projects) to add SQL WHERE conditions that compare a lower-cased basename of projectMainRepoPath to normalizedRepo.repo (or use LIKE/ends-with semantics) and, if normalizedRepo.owner exists, add a clause comparing lower(projectGithubOwner) to normalizedRepo.owner; keep the existing isNull(workspaces.deletingAt) and ordering. Target the query building around the same symbols: localDb.select(...).from(workspaces).innerJoin(projects, ...).where(...) so the filter runs in the database not in the later .filter callback that currently references projectMainRepoPath, projectGithubOwner, and normalizedRepo.apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/SearchView/components/SearchFileGroup/SearchFileGroup.tsx (2)
113-113: Consider memoizinglineMatchescomputation.
groupMatchesByLine(group)is called on every render. For files with many matches, this could be a performance concern, especially in a virtualized list where items may re-render frequently.♻️ Memoize lineMatches
+import { memo, useMemo } from "react"; -import { memo } from "react"; // Inside the component: - const lineMatches = groupMatchesByLine(group); + const lineMatches = useMemo(() => groupMatchesByLine(group), [group]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/SearchView/components/SearchFileGroup/SearchFileGroup.tsx` at line 113, The call to groupMatchesByLine(group) in the SearchFileGroup component is executed on every render and can be expensive for files with many matches; wrap this computation in React.useMemo so lineMatches is only recomputed when its relevant dependency changes (e.g., the group object or group.matches/line-related fields) — locate the usage of groupMatchesByLine and replace the direct call that sets lineMatches with a memoized value using useMemo inside SearchFileGroup, referencing group (or a stable key like group.id plus group.matches) in the dependency array.
117-226: Repeated variant conditionals reduce readability.The same
isTreeVariant ? ... : isListVariant ? ... : ...pattern appears ~8 times. Consider extracting variant-specific class sets for better maintainability.♻️ Extract variant classes
const variantClasses = { tree: { container: "rounded-sm", header: "group grid grid-cols-[minmax(0,1fr)_auto] items-center gap-1 py-0.5", trigger: "flex w-full min-w-0 items-center gap-1.5 rounded-sm px-1 py-1 text-left text-xs transition-colors hover:bg-accent/40", chevron: "shrink-0 text-muted-foreground", icon: "size-4 shrink-0", content: "ml-4 border-l border-border/50 pl-2", }, list: { container: "rounded-sm", header: "group grid grid-cols-[minmax(0,1fr)_auto] items-center gap-1 py-0.5", trigger: "flex w-full min-w-0 items-center gap-1.5 rounded-sm px-1 py-1 text-left text-xs transition-colors hover:bg-accent/40", chevron: "shrink-0 text-muted-foreground", icon: "size-4 shrink-0", content: "ml-4 border-l border-border/50 pl-2", }, default: { container: "rounded-md border border-border/60 bg-background/60", header: "group grid grid-cols-[minmax(0,1fr)_auto] items-center gap-1 px-1.5 py-1.5", trigger: "flex w-full min-w-0 items-start gap-2 rounded-md px-1 py-1 text-left transition-colors hover:bg-accent/40", chevron: "mt-0.5 shrink-0 text-muted-foreground", icon: "mt-0.5 size-4 shrink-0", content: "border-t border-border/50 px-1.5 py-1", }, } as const; // Usage: const classes = variantClasses[variant]; // <div className={classes.container}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/SearchView/components/SearchFileGroup/SearchFileGroup.tsx` around lines 117 - 226, Multiple repeated ternary class expressions using isTreeVariant/isListVariant reduce readability; extract a variant-to-class map and use it instead. Create a const (e.g., variantClasses) mapping keys (tree, list, default) to class sets for container, header, trigger, chevron, icon, content, then compute const classes = variantClasses[isTreeVariant ? "tree" : isListVariant ? "list" : "default"] and replace occurrences of the repeated ternaries with classes.container, classes.header, classes.trigger, classes.chevron, classes.icon, and classes.content in SearchFileGroup (references: isTreeVariant, isListVariant, CollapsibleTrigger/CollapsibleContent, FileIcon, matchCount/RowHoverActions).apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/SearchView/components/SearchMatchItem/SearchMatchItem.tsx (1)
40-63: Consider memoizinghoverActionsarray for stable references.The
hoverActionsarray is recreated on each render. While the component itself is memoized, ifRowHoverActionsdoes shallow comparison or uses the array identity, this could cause unnecessary re-renders of that child component.♻️ Optional: Memoize hoverActions
+ const hoverActions: RowHoverAction[] = useMemo( + () => [ + ...(isReplaceEnabled + ? [ + { + key: "replace", + label: "Replace match", + icon: <LuReplace className="size-3.5" />, + onClick: () => onReplace(lineMatch), + }, + ] + : []), + { + key: "copy-link", + label: "Copy Superset link", + icon: <LuLink className="size-3.5" />, + onClick: () => onCopyLink(lineMatch), + }, + { + key: "ignore", + label: "Ignore result", + icon: <LuEyeOff className="size-3.5" />, + onClick: () => onIgnore(lineMatch), + }, + ], + [isReplaceEnabled, lineMatch, onCopyLink, onIgnore, onReplace], + ); - const hoverActions: RowHoverAction[] = [ - ...(isReplaceEnabled - ? [ - { - key: "replace", - label: "Replace match", - icon: <LuReplace className="size-3.5" />, - onClick: () => onReplace(lineMatch), - }, - ] - : []), - { - key: "copy-link", - label: "Copy Superset link", - icon: <LuLink className="size-3.5" />, - onClick: () => onCopyLink(lineMatch), - }, - { - key: "ignore", - label: "Ignore result", - icon: <LuEyeOff className="size-3.5" />, - onClick: () => onIgnore(lineMatch), - }, - ];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/SearchView/components/SearchMatchItem/SearchMatchItem.tsx` around lines 40 - 63, hoverActions is recreated every render which can break shallow-equality checks in RowHoverActions; wrap the hoverActions array creation in useMemo (import from React) and return the same array identity unless its dependencies change (isReplaceEnabled, onReplace, onCopyLink, onIgnore, lineMatch) so that RowHoverActions sees a stable prop and avoids unnecessary re-renders. Ensure you reference the existing symbols hoverActions, RowHoverAction, isReplaceEnabled, onReplace, onCopyLink, onIgnore, and lineMatch when building the memo dependencies.apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/SearchView/SearchView.tsx (2)
750-842: Type assertions are necessary but could benefit from type guards.The type assertions
as (typeof treeResults)[number]andas (typeof groupedResults)[number]are required becauselistItemsis a union type. While this works correctly given theresultViewModecheck, consider extracting into separate render functions for cleaner code.♻️ Optional: Extract render logic for cleaner typing
// Define outside component or memoize inside const renderTreeItem = (item: SearchTreeNodeType) => ( <SearchTreeNode node={item} /* ...props */ /> ); const renderListItem = (item: SearchResultGroup) => ( <SearchFileGroup group={item} /* ...props */ /> ); // In render: {resultViewMode === "tree" ? renderTreeItem(item as SearchTreeNodeType) : renderListItem(item as SearchResultGroup)}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/SearchView/SearchView.tsx` around lines 750 - 842, The virtualized render uses unsafe assertions on listItems (as (typeof treeResults)[number] / (typeof groupedResults)[number]) in SearchView.tsx when choosing between SearchTreeNode and SearchFileGroup; extract the render branches into two typed helper functions (e.g., renderTreeItem(item: SearchTreeNodeType) and renderListItem(item: SearchResultGroup)) or implement a small type guard that narrows an item based on resultViewMode before rendering, then call the appropriate helper from the virtualizer.map so you can remove the inline casts and keep typesafe props for SearchTreeNode, SearchFileGroup, virtualizer.getVirtualItems, and any handlers (handleOpenGroupChange, handleOpenFolderChange, onOpenFileAtLine, etc.).
367-370: Unusual dependency pattern for resetting state.Using
void searchResultResetKeyas a statement to satisfy the linter while the actual dependency triggers the effect is unconventional. Consider a more explicit approach.♻️ Alternative: Use explicit dependency without void statement
useEffect(() => { - void searchResultResetKey; setIgnoredMatchIds({}); + // Reset ignored matches when search parameters change }, [searchResultResetKey]);The
searchResultResetKeyin the dependency array already triggers the effect - thevoidstatement inside the effect body is unnecessary.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/SearchView/SearchView.tsx` around lines 367 - 370, The effect in useEffect is using an unnecessary "void searchResultResetKey" statement to appease the linter; remove that line and let the effect be triggered by the searchResultResetKey dependency, keeping setIgnoredMatchIds({}) as the body. Ensure the dependency array still contains searchResultResetKey so the effect runs on changes; if your linter still complains about an unused variable, address it with an explanatory comment above the effect or a focused eslint rule rather than a void statement.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/lib/trpc/routers/databases/index.ts`:
- Around line 356-378: The current logic applies slice(limit) after merging
fileItems and configuredDatabases so configured items can be dropped; fix by
applying the limit only to filesystem hits: compute configuredDatabases via
discoverWorkspaceConfiguredDatabases, then calculate allowedFileLimit =
Math.max(0, limit - configuredDatabases.length) and slice fileItems to
fileItems.slice(0, allowedFileLimit) (using the existing files -> fileItems
mapping), then merge [...fileItemsLimited, ...configuredDatabases], sort by
relativePath, and return that; this guarantees configuredDatabases entries are
reserved and the limit only trims filesystem results.
In `@apps/desktop/src/lib/trpc/routers/databases/workspace-config.ts`:
- Around line 136-146: The current loadWorkspaceDatabaseCredentialStore function
masks all failures (readFile, decrypt, JSON.parse, schema.parse) by returning an
empty store; change it so only a missing file maps to { entries: {} } and all
other errors are rethrown (or wrapped) so they surface. Specifically, when
calling readFile(WORKSPACE_DATABASE_CREDENTIALS_FILE) handle the
file-not-found/ENOENT case and return { entries: {} }, but let errors from
decrypt(), JSON.parse, and workspaceDatabaseCredentialStoreSchema.parse()
propagate (or throw a new error with context) so broken credentials aren't
silently lost.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/`$workspaceId/page.tsx:
- Around line 192-199: The navigate call currently re-writes tabId/paneId into
the URL after consuming a file param, which can cause the previous tab to be
reactivated; update the logic around the navigate(...) in page.tsx to omit tabId
and paneId from the search object whenever a deep-link included a file param
(i.e., when the incoming "file" param was present/handled). Concretely, change
the code that builds the search object for navigate(...) (the keys searchTabId
and searchPaneId) so they are only included when no file param was consumed,
leaving them undefined/absent when file was present to avoid reintroducing old
tab focus.
- Around line 75-89: The current coercion for search.line and search.column
accepts zeros, negatives, decimals, and whitespace; change the ternaries that
set the line and column fields so they only accept positive integers (>= 1). For
both the line and column branches (the code referencing search.line and
search.column in page.tsx), when handling string input parse it to a Number,
then require Number.isInteger(value) && value >= 1 before returning the number;
otherwise return undefined. Keep the existing numeric branch but similarly
require typeof search.line/column === "number" &&
Number.isInteger(search.line/column) && search.line/column >= 1.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/components/EditorContextMenu/useEditorActions.ts`:
- Around line 85-153: The menu shows Superset actions even when links are
unbuildable; instead of always returning handleCopySupersetLink and
handleCopySupersetLinkWithLine, only return these callbacks when a link can
actually be built: check supersetLinkProject and call buildSupersetOpenLink(...)
(or a lightweight "isBuildable" check) with the current
branch/worktreePath/filePath (and selection startLine for the line variant) and
if it returns a truthy link, return the corresponding callback, otherwise return
undefined/null so the menu gating by callback presence hides the action. Ensure
you reference and gate on the same symbols used in the file:
handleCopySupersetLink, handleCopySupersetLinkWithLine, buildSupersetOpenLink,
supersetLinkProject (and getEditor()/selection for the line variant).
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/SpreadsheetViewer/SpreadsheetDiffViewer.tsx`:
- Around line 611-632: getDiffRowIndices currently only inspects per-cell
diffStatus and returns [] for empty sheets, so sheet-level changes
(sheet.sheetStatus like "added"/"removed"/"renamed") are ignored; update
getDiffRowIndices to also check sheet.sheetStatus and, when it indicates a
sheet-level change, include a representative index (e.g., push 0 or otherwise
ensure at least one index is returned) so callers building diffLocations treat
the sheet as changed; reference getDiffRowIndices and the sheet.sheetStatus
property when making this change.
- Line 513: The SpreadsheetDefaultAppButton currently always uses
absoluteFilePath which is wrong for historical or removed-file diffs; update
SpreadsheetDiffViewer to only render SpreadsheetDefaultAppButton when viewing
the working-tree file (e.g. when commitHash is undefined/null and the diff entry
is not a removed-file and the working-tree path exists), or alternatively wire
the button to a materialized blob path when a commitHash or removed flag is
present so it opens the exact blob being inspected; modify the render condition
around SpreadsheetDefaultAppButton (and the props you pass it) to check
commitHash and removed-file status (and/or use the blob identifier from the
diff) before showing the button.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/hooks/usePathActions.ts`:
- Around line 62-89: hasSupersetLink currently only checks supersetLinkProject
and shows the menu item even when buildSupersetOpenLink(...) would return null;
update the visibility check to mirror the copy action by calling
buildSupersetOpenLink({ project: supersetLinkProject, branch, filePath:
relativePath }) and treating the link as available only when that call returns a
non-null value. Change the boolean/computed used for the menu visibility (the
hasSupersetLink variable or getter in usePathActions) to use that
buildSupersetOpenLink result, and ensure any other place in usePathActions that
determines visibility (the other occurrence of hasSupersetLink) is updated the
same way so the menu item only appears when a valid link can actually be built.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/DatabasesView/DatabasesView.tsx`:
- Around line 1835-1840: The code currently skips the credential prompt when
item.hasSavedCredentials is true by directly calling
attachWorkspaceConfigPostgresConnection, which prevents users from rotating or
replacing saved Postgres passwords; change the flow so the credential prompt can
be forced even when hasSavedCredentials is true — e.g., add a forcePrompt
boolean param to attachWorkspaceConfigPostgresConnection (or a separate
attachWithPrompt wrapper) and call openWorkspaceCredentialPrompt when
forcePrompt is set or when the user triggers edit; also update the edit-mode
rendering logic (the component that hides the password input when
hasSavedCredentials is true) to respect a "force edit" flag so the password
field becomes editable when the credential prompt is shown or edit is explicitly
requested (ensure functions: attachWorkspaceConfigPostgresConnection,
openWorkspaceCredentialPrompt and the hasSavedCredentials checks are updated to
support this behavior).
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/DockerView/components/InspectCodeBlock/InspectCodeBlock.tsx`:
- Around line 12-14: createShikiTheme(activeTheme) is recreating a new theme
object each render which forces the useEffect that depends on theme to re-run;
wrap the theme creation in useMemo so theme is memoized based on activeTheme
(e.g., replace the direct createShikiTheme call with const theme = useMemo(() =>
activeTheme ? createShikiTheme(activeTheme) : undefined, [activeTheme]) ),
update places using theme (and the other occurrence noted) to use the memoized
value so re-highlighting only occurs when activeTheme actually changes;
reference useResolvedTheme, createShikiTheme, and the theme variable in
InspectCodeBlock.tsx when applying the change.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/FilesView.tsx`:
- Around line 150-161: The current Superset metadata lookup only uses projectId
derived from workspace?.project?.id (projectId const) so when the nested project
relation is missing we lose the Superset link; update the lookup to fallback to
workspace?.projectId (e.g., derive projectId = workspace?.projectId ??
workspace?.project?.id) and pass that into electronTrpc.projects.get.useQuery
and into supersetLinkProject so the Superset link is preserved for workspaces
that only expose projectId.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ProblemsView/ProblemsView.tsx`:
- Around line 307-335: The Tooltip currently doesn't show when the Button is
disabled because Radix applies pointer-events-none to disabled elements; to fix,
keep TooltipTrigger as the parent but wrap the <Button> (used in the
onClick/disabled logic) in a non-disabled wrapper element (e.g., a <span>) so
hover/focus still reach TooltipTrigger when refreshDiagnostics.isPending or
isFetching is true; update the JSX around TooltipTrigger/Button in ProblemsView
(the refresh button using refreshDiagnostics, isFetching, and LuRefreshCw) to
render TooltipTrigger asChild containing a span that contains the actual Button,
preserving the disabled prop and classNames but allowing the tooltip to appear
during loading.
In `@apps/desktop/src/renderer/stores/database-sidebar.ts`:
- Around line 172-179: The persisted renderer store is saving manual Postgres
connectionString in plain text; update addConnection and updateConnection so
that when input.dialect === "postgres" and input.source === "manual" you do not
write connectionString into the renderer-persisted object (remove the
connectionString assignment in the block that builds the stored entry). Instead,
store a reference/key in the renderer store and save the actual credential
(connectionString) using the existing encrypted credential flow used by
workspace-database-credentials.enc (same AES-256-GCM pattern) so manual Postgres
credentials are written to the encrypted credential store rather than to the
plain persisted state.
---
Outside diff comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/hooks/usePersistentWebview/usePersistentWebview.ts`:
- Around line 426-458: The cleanup currently detaches all webview listeners
(handleDomReady, handleDidStartLoading, handleDidStopLoading, handleDidNavigate,
handleDidNavigateInPage, handlePageTitleUpdated, handlePageFaviconUpdated,
handleDidFailLoad) before parking the wrapper in
getPersistentWrapper/getHiddenContainer, which causes missed events for hidden
navigations and fast loads triggered by navigatePersistentWebview; instead,
remove the removeEventListener calls from this return cleanup (keep listeners
attached while the persistent webview is parked) and move those listener
removals into destroyPersistentWebview() so listeners remain for the lifetime of
the persistent webview, or implement a full state reconciliation when
reclaiming—update the code paths referencing navigatePersistentWebview,
getPersistentWrapper, getHiddenContainer, and destroyPersistentWebview
accordingly.
In `@apps/desktop/src/renderer/stores/tabs/store.ts`:
- Around line 1757-1818: The openInBrowserPane branch mutates
pane.browser.currentUrl and history with the raw input url before calling
navigatePersistentWebview, causing double entries when navigatePersistentWebview
normalizes URLs; fix by normalizing the target URL first (use the same
normalization logic as navigatePersistentWebview/updateBrowserUrl) and then push
that normalized URL into existingPane.browser.history and currentUrl so the
stored history matches the committed navigation; update references in
openInBrowserPane (existingPane.browser, history, historyIndex) to use the
normalized value before calling navigatePersistentWebview.
---
Nitpick comments:
In `@apps/desktop/src/lib/trpc/routers/docker/index.ts`:
- Around line 195-247: The findComposeFiles function currently walks the
directory tree without any depth limit; add a maxDepth (e.g., default to 6) and
track depth during traversal by changing the queue to hold tuples/objects with
{path, depth} (or add a depth parameter if you refactor to recursion) and only
enqueue subdirectories when currentDepth < maxDepth; update references to
findComposeFiles or provide a default param so callers remain unchanged and
ensure the depth check is applied before pushing into queue (symbols:
findComposeFiles, queue, isIgnoredDirectory).
- Around line 499-507: The JSON.parse(stdout) call after
execDocker(["container", "inspect", ...], { cwd:
getWorkspaceRootPath(input.workspaceId) }) can throw a SyntaxError which is
currently caught and passed to normalizeExecError with no context; update the
try/catch so parse errors are handled explicitly: wrap JSON.parse(stdout) in its
own try/catch (or detect parse failure) and call normalizeExecError or rethrow a
new Error that includes context like the containerId and that the failure was a
JSON parse error (referencing execDocker, JSON.parse, input.containerId and
normalizeExecError) so the surfaced message clearly indicates a parsing issue
rather than a raw SyntaxError.
In `@apps/desktop/src/main/index.ts`:
- Around line 160-169: The null check for match is redundant because match is
assigned via (branchParam ? candidates.find(...) : null) ?? candidates[0], and
candidates[0] is guaranteed to exist; remove the if (!match) { return null; }
block and keep the existing assignment of match (using the nullish-coalescing
fallback to candidates[0]) so downstream code can use match directly; reference
symbols: match, branchParam, candidates.
- Around line 128-154: The current post-query filter on candidates pulls all
non-deleted workspaces and then filters by repo name and owner in JS; move that
logic into the SQL query built from localDb.select to avoid loading extra rows.
Update the query that selects from workspaces and innerJoin(projects) to add SQL
WHERE conditions that compare a lower-cased basename of projectMainRepoPath to
normalizedRepo.repo (or use LIKE/ends-with semantics) and, if
normalizedRepo.owner exists, add a clause comparing lower(projectGithubOwner) to
normalizedRepo.owner; keep the existing isNull(workspaces.deletingAt) and
ordering. Target the query building around the same symbols:
localDb.select(...).from(workspaces).innerJoin(projects, ...).where(...) so the
filter runs in the database not in the later .filter callback that currently
references projectMainRepoPath, projectGithubOwner, and normalizedRepo.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/SpreadsheetViewer/SpreadsheetViewer.tsx`:
- Around line 341-343: The toolbar containing SpreadsheetDefaultAppButton is
currently rendered after early returns (isLoading / error / "No sheets found")
so the fallback action disappears; in the SpreadsheetViewer component hoist the
toolbar div with SpreadsheetDefaultAppButton (preserving the absoluteFilePath
prop) above the conditional return checks (isLoading, error, and the "No sheets
found" branch) so it is rendered unconditionally while keeping the rest of the
sheet-rendering logic intact.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/RepositoryPanel/RepositoryPanel.tsx`:
- Around line 306-333: RepositoryPanel has grown large with 15+ state hooks
(e.g., issueTitle, issueBody, pullRequestsOpen, workflowsOpen,
issueComposerOpen, trackedWorkflowRuns) and multiple collapsible sections;
extract the Pull Requests, Workflows and Issue Composer UI into separate
components (e.g., PullRequestsSection, WorkflowsSection, IssueComposer) to
reduce complexity. For each new component, move the related JSX and
tightly-coupled state into the child (or lift minimal state to RepositoryPanel
and pass as props/handlers), wire up handlers (e.g., setPullRequestsOpen,
setWorkflowsOpen, setIssueTitle, setIssueBody, setIssueAssignees,
setIssueLabels, setUploadedAssets, setTrackedWorkflowRuns) via props, and
replace the original sections in RepositoryPanel with the new component imports
so behavior remains identical. Ensure types for props reference existing types
(UploadedIssueAsset, TrackedWorkflowRun) and preserve usages of
electronTrpc/useTabsStore/trpcUtils by passing callbacks or utilities as needed.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/DockerView/DockerView.tsx`:
- Around line 374-405: The Start and Stop buttons currently only check their own
mutation pending state, allowing concurrent mutations; update both Button
disabled props to disable when either startProjectMutation.isPending OR
stopProjectMutation.isPending (i.e., use a shared condition like
startProjectMutation.isPending || stopProjectMutation.isPending) so both buttons
that call startProjectMutation.mutate and stopProjectMutation.mutate are
disabled while any project mutation is in flight; locate the Button components
around the startProjectMutation and stopProjectMutation calls (they use
group.absolutePath and workspaceId) and apply the combined disabled condition.
- Around line 249-273: The updater for expandedComposeGroups can return a stale
previous object when counts match but keys differ; in the
setExpandedComposeGroups updater (inside DockerView's useEffect, using
dockerListQuery.data?.composeFiles and variables composeFiles / previous /
next), replace the current length-based short-circuit with a keys comparison or
simply return next when the set of keys differs: build next as you do, then if
Object.keys(previous).length === Object.keys(next).length and every key in next
exists in previous return previous, otherwise return next (or always return
next) to ensure stale keys are removed.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/SearchView/components/SearchFileGroup/SearchFileGroup.tsx`:
- Line 113: The call to groupMatchesByLine(group) in the SearchFileGroup
component is executed on every render and can be expensive for files with many
matches; wrap this computation in React.useMemo so lineMatches is only
recomputed when its relevant dependency changes (e.g., the group object or
group.matches/line-related fields) — locate the usage of groupMatchesByLine and
replace the direct call that sets lineMatches with a memoized value using
useMemo inside SearchFileGroup, referencing group (or a stable key like group.id
plus group.matches) in the dependency array.
- Around line 117-226: Multiple repeated ternary class expressions using
isTreeVariant/isListVariant reduce readability; extract a variant-to-class map
and use it instead. Create a const (e.g., variantClasses) mapping keys (tree,
list, default) to class sets for container, header, trigger, chevron, icon,
content, then compute const classes = variantClasses[isTreeVariant ? "tree" :
isListVariant ? "list" : "default"] and replace occurrences of the repeated
ternaries with classes.container, classes.header, classes.trigger,
classes.chevron, classes.icon, and classes.content in SearchFileGroup
(references: isTreeVariant, isListVariant,
CollapsibleTrigger/CollapsibleContent, FileIcon, matchCount/RowHoverActions).
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/SearchView/components/SearchMatchItem/SearchMatchItem.tsx`:
- Around line 40-63: hoverActions is recreated every render which can break
shallow-equality checks in RowHoverActions; wrap the hoverActions array creation
in useMemo (import from React) and return the same array identity unless its
dependencies change (isReplaceEnabled, onReplace, onCopyLink, onIgnore,
lineMatch) so that RowHoverActions sees a stable prop and avoids unnecessary
re-renders. Ensure you reference the existing symbols hoverActions,
RowHoverAction, isReplaceEnabled, onReplace, onCopyLink, onIgnore, and lineMatch
when building the memo dependencies.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/SearchView/SearchView.tsx`:
- Around line 750-842: The virtualized render uses unsafe assertions on
listItems (as (typeof treeResults)[number] / (typeof groupedResults)[number]) in
SearchView.tsx when choosing between SearchTreeNode and SearchFileGroup; extract
the render branches into two typed helper functions (e.g., renderTreeItem(item:
SearchTreeNodeType) and renderListItem(item: SearchResultGroup)) or implement a
small type guard that narrows an item based on resultViewMode before rendering,
then call the appropriate helper from the virtualizer.map so you can remove the
inline casts and keep typesafe props for SearchTreeNode, SearchFileGroup,
virtualizer.getVirtualItems, and any handlers (handleOpenGroupChange,
handleOpenFolderChange, onOpenFileAtLine, etc.).
- Around line 367-370: The effect in useEffect is using an unnecessary "void
searchResultResetKey" statement to appease the linter; remove that line and let
the effect be triggered by the searchResultResetKey dependency, keeping
setIgnoredMatchIds({}) as the body. Ensure the dependency array still contains
searchResultResetKey so the effect runs on changes; if your linter still
complains about an unused variable, address it with an explanatory comment above
the effect or a focused eslint rule rather than a void statement.
In `@apps/open-link/package.json`:
- Line 9: The "typecheck" npm script includes a redundant flag
"--emitDeclarationOnly false" alongside "--noEmit"; update the "typecheck"
script in package.json (the "typecheck" entry) to remove the
"--emitDeclarationOnly false" token so the command becomes bunx tsc -p
tsconfig.json --noEmit, keeping the rest unchanged.
🪄 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: c8b7e40b-1447-46a3-ab3d-130b332343e4
📒 Files selected for processing (60)
apps/desktop/electron.vite.config.tsapps/desktop/src/lib/trpc/routers/databases/index.tsapps/desktop/src/lib/trpc/routers/databases/workspace-config.tsapps/desktop/src/lib/trpc/routers/docker/index.tsapps/desktop/src/lib/trpc/routers/external/index.tsapps/desktop/src/lib/trpc/routers/index.tsapps/desktop/src/main/index.tsapps/desktop/src/renderer/env.renderer.tsapps/desktop/src/renderer/lib/superset-open-links.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/utils/workspace-navigation.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilesPane/components/WorkspaceFilePreview/components/WorkspaceFilePreviewContent/WorkspaceFilePreviewContent.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/extensions/components/ExtensionsSettings/ExtensionsSettings.tsxapps/desktop/src/renderer/screens/main/components/CommandPalette/CommandPalette.tsxapps/desktop/src/renderer/screens/main/components/CommandPalette/useCommandPalette.tsapps/desktop/src/renderer/screens/main/components/SearchDialog/SearchDialog.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/hooks/usePersistentWebview/runtime.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/hooks/usePersistentWebview/usePersistentWebview.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/DiffViewerContextMenu/DiffViewerContextMenu.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/FileEditorContextMenu/FileEditorContextMenu.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/FileViewerContent/FileViewerContent.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/SpreadsheetViewer/SpreadsheetDiffViewer.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/SpreadsheetViewer/SpreadsheetViewer.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/SpreadsheetViewer/components/SpreadsheetDefaultAppButton/SpreadsheetDefaultAppButton.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/SpreadsheetViewer/components/SpreadsheetDefaultAppButton/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/components/EditorContextMenu/EditorContextMenu.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/components/EditorContextMenu/useEditorActions.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/RepositoryPanel/RepositoryPanel.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ReviewPanel/ReviewPanel.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/RowHoverActions/RowHoverActions.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/hooks/usePathActions.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/DatabasesView/DatabasesView.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/DockerView/DockerView.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/DockerView/components/InspectCodeBlock/InspectCodeBlock.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/DockerView/components/InspectCodeBlock/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/DockerView/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/FilesView.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/components/FileSearchResultItem/FileSearchResultItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/components/FileTreeItem/FileTreeItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ProblemsView/ProblemsView.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/SearchView/SearchView.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/SearchView/components/SearchFileGroup/SearchFileGroup.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/SearchView/components/SearchMatchItem/SearchMatchItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/SearchView/components/SearchToolbar/SearchToolbar.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/SearchView/components/SearchTreeNode/SearchTreeNode.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/SearchView/components/SearchTreeNode/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/SearchView/types.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/SearchView/utils/searchPattern/searchPattern.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/utils/code-theme/diff-theme.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/utils/code-theme/shiki-theme.tsapps/desktop/src/renderer/stores/database-sidebar.tsapps/desktop/src/renderer/stores/sidebar-state.tsapps/desktop/src/renderer/stores/tabs/store.tsapps/open-link/package.jsonapps/open-link/src/index.tsapps/open-link/tsconfig.jsonapps/open-link/wrangler.jsoncpackages/ui/src/components/ai-elements/code-block.tsxpackages/ui/src/components/ui/command.tsx
- Reserve slots for workspace-config entries before applying discovery limit - Re-throw non-ENOENT errors in credential store loader to prevent data loss - Clear tabId/paneId from URL after opening file via deep link - Hide default-app button in SpreadsheetDiffViewer when commitHash is present - Restrict line/column URL params to positive integers only - Derive hasSupersetLink from actual buildSupersetOpenLink result - Memoize shiki theme in InspectCodeBlock to prevent unnecessary re-renders - Add projectId fallback in FilesView for workspaces without nested project relation - Conditionally return superset link actions only when link is buildable
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/hooks/usePathActions.ts (1)
141-149: Consider memoizinghasSupersetLinkto avoid recomputation on every render.
buildSupersetOpenLinkcreates aURLobject and performs string operations on each render. While not expensive, wrapping this inuseMemowould be more idiomatic and prevent unnecessary work when dependencies haven't changed.♻️ Suggested refactor
+import { useCallback, useMemo } from "react"; -import { useCallback } from "react";Then at the return statement:
+ const hasSupersetLink = useMemo( + () => + Boolean( + relativePath && + supersetLinkProject && + buildSupersetOpenLink({ + project: supersetLinkProject, + branch, + filePath: relativePath, + }), + ), + [relativePath, supersetLinkProject, branch], + ); + return { copyPath, copyRelativePath, copySupersetLink, revealInFinder, openInEditor, hasRelativePath: Boolean(relativePath), - hasSupersetLink: Boolean( - relativePath && - supersetLinkProject && - buildSupersetOpenLink({ - project: supersetLinkProject, - branch, - filePath: relativePath, - }), - ), + hasSupersetLink, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/hooks/usePathActions.ts` around lines 141 - 149, The computed boolean hasSupersetLink is re-evaluated on every render because it calls buildSupersetOpenLink inline; wrap the URL/string work in a React useMemo so the URL is only rebuilt when its inputs change (depend on relativePath, supersetLinkProject, and branch) and then derive hasSupersetLink from that memoized value (keep references to buildSupersetOpenLink, relativePath, supersetLinkProject, and branch to locate the code).apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/DockerView/components/InspectCodeBlock/InspectCodeBlock.tsx (1)
19-34: Consider adding error handling for the highlight promise.If
highlightCoderejects (e.g., due to an unsupported language or Shiki error), the promise rejection is unhandled. Currently, the component gracefully degrades to empty content, but adding a.catch()would make failure handling explicit.🔧 Optional: Add error handling
useEffect(() => { let cancelled = false; void highlightCode(code, language, false, { lightTheme: theme, darkTheme: theme, - }).then(([nextHtml]) => { - if (!cancelled) { - setHtml(nextHtml); - } - }); + }) + .then(([nextHtml]) => { + if (!cancelled) { + setHtml(nextHtml); + } + }) + .catch((error) => { + if (!cancelled) { + console.error("Failed to highlight code:", error); + } + }); return () => { cancelled = true; }; }, [code, language, theme]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/DockerView/components/InspectCodeBlock/InspectCodeBlock.tsx` around lines 19 - 34, The highlightCode promise in the useEffect (used to generate highlighted HTML for code/language with highlightCode(...).then(...)) lacks error handling; update the effect to attach a .catch(...) to the returned promise (or use try/catch with async/await) to handle rejections, check the cancelled flag before calling setHtml, and in the catch handler either setHtml to a safe fallback (e.g., raw escaped code or empty string) and/or log the error (use the existing logger or console.error) so failures from highlightCode (unsupported language or Shiki errors) are explicit rather than unhandled.apps/desktop/src/lib/trpc/routers/databases/workspace-config.ts (1)
360-371: Re-reading config preserves unknown fields but introduces a race window.The second
readFile(Line 360) re-reads the file to preserve any extra JSON fields not covered by the schema. While this is a reasonable approach for forward compatibility, there's a small TOCTOU window between the initialloadWorkspaceDatabaseDefinitionsread and this re-read.In a single-user desktop context this is unlikely to cause issues, but you could optionally refactor
loadWorkspaceDatabaseDefinitionsto also return the raw config object to avoid the second read.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/databases/workspace-config.ts` around lines 360 - 371, The current code re-reads the workspace config via readFile(configPath, "utf8") to build rawConfig/rawDefinitions/currentRawDefinition, creating a TOCTOU window with the earlier loadWorkspaceDatabaseDefinitions call; refactor loadWorkspaceDatabaseDefinitions to return both the parsed schema-safe definitions and the raw config object (or the raw "databases" array) so you can use that returned raw object instead of calling readFile again, then update the code that uses rawConfig/rawDefinitions/currentRawDefinition to consume the returned raw payload and remove the second readFile call.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/lib/trpc/routers/databases/workspace-config.ts`:
- Around line 114-128: The buildPostgresConnectionString function currently
concatenates input.host directly into the URL which breaks for IPv6 addresses;
update it to detect IPv6 hosts (e.g., input.host contains ':' and is not already
bracketed) and wrap the host in square brackets before composing the URL (e.g.,
use a local hostVar = input.host.startsWith('[') ? input.host :
`[${input.host}]`), then use hostVar in the returned string so the port
delimiter remains unambiguous; keep the existing auth, port and query handling
unchanged.
---
Nitpick comments:
In `@apps/desktop/src/lib/trpc/routers/databases/workspace-config.ts`:
- Around line 360-371: The current code re-reads the workspace config via
readFile(configPath, "utf8") to build
rawConfig/rawDefinitions/currentRawDefinition, creating a TOCTOU window with the
earlier loadWorkspaceDatabaseDefinitions call; refactor
loadWorkspaceDatabaseDefinitions to return both the parsed schema-safe
definitions and the raw config object (or the raw "databases" array) so you can
use that returned raw object instead of calling readFile again, then update the
code that uses rawConfig/rawDefinitions/currentRawDefinition to consume the
returned raw payload and remove the second readFile call.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/hooks/usePathActions.ts`:
- Around line 141-149: The computed boolean hasSupersetLink is re-evaluated on
every render because it calls buildSupersetOpenLink inline; wrap the URL/string
work in a React useMemo so the URL is only rebuilt when its inputs change
(depend on relativePath, supersetLinkProject, and branch) and then derive
hasSupersetLink from that memoized value (keep references to
buildSupersetOpenLink, relativePath, supersetLinkProject, and branch to locate
the code).
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/DockerView/components/InspectCodeBlock/InspectCodeBlock.tsx`:
- Around line 19-34: The highlightCode promise in the useEffect (used to
generate highlighted HTML for code/language with highlightCode(...).then(...))
lacks error handling; update the effect to attach a .catch(...) to the returned
promise (or use try/catch with async/await) to handle rejections, check the
cancelled flag before calling setHtml, and in the catch handler either setHtml
to a safe fallback (e.g., raw escaped code or empty string) and/or log the error
(use the existing logger or console.error) so failures from highlightCode
(unsupported language or Shiki errors) are explicit rather than unhandled.
🪄 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: 2c0f61c9-3616-4dd6-8d6e-8ac336631039
📒 Files selected for processing (8)
apps/desktop/src/lib/trpc/routers/databases/index.tsapps/desktop/src/lib/trpc/routers/databases/workspace-config.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/page.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/SpreadsheetViewer/SpreadsheetDiffViewer.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/components/EditorContextMenu/useEditorActions.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/hooks/usePathActions.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/DockerView/components/InspectCodeBlock/InspectCodeBlock.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/FilesView.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/FilesView.tsx
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/components/EditorContextMenu/useEditorActions.ts
- apps/desktop/src/lib/trpc/routers/databases/index.ts
- Fix SpreadsheetDiffViewer showing "No changes" for added/removed empty sheets - Add password rotation support for workspace-config Postgres connections - Migrate manual Postgres connectionString from plain localStorage to AES-256-GCM encrypted store - Add tRPC procedures for manual Postgres connection string CRUD - Add IPv6 bracket notation in Postgres URL construction (RFC3986)
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/stores/database-sidebar.ts (1)
237-283:⚠️ Potential issue | 🟠 MajorClear the migration scratch fields after a manual Postgres connection is rewritten.
Lines 386-400 create
_pendingConnectionStringfor legacy records, but Lines 237-283 spreadcurrentConnectionand never remove it. The migration effect inDatabasesView.tsxwill keep treating that record as pending and re-encrypt the same secret on every update. Clear_pendingConnectionStringand the deprecatedconnectionStringwhen persisting the updated entry.Possible fix
const updatedConnection: SavedDatabaseConnection = { ...currentConnection, label: input.label, group: input.group, dialect: input.dialect, source: input.source ?? "manual", + connectionString: undefined, + _pendingConnectionString: undefined, databasePath: input.dialect === "sqlite" ? input.databasePath : undefined,Also applies to: 386-400
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/stores/database-sidebar.ts` around lines 237 - 283, When rewriting a manual Postgres connection (where you build updatedConnection from currentConnection in the SavedDatabaseConnection flow), ensure you remove legacy migration scratch fields so the record is no longer treated as pending: explicitly set _pendingConnectionString to undefined and clear the deprecated connectionString when constructing updatedConnection (in the same block that creates updatedConnection from currentConnection), and do the same in the migration path around where _pendingConnectionString is created (lines ~386-400) so both update paths stop re-encrypting the secret.
♻️ Duplicate comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/SpreadsheetViewer/SpreadsheetDiffViewer.tsx (1)
514-516:⚠️ Potential issue | 🟠 MajorHide the default-app action for removed-file diffs too.
Line 514 only gates on
commitHash, so a working-treediffCategory === "removed"view still renders a button for a path that no longer exists.🩹 Minimal guard
- {!commitHash && ( + {!commitHash && diffCategory !== "removed" && ( <SpreadsheetDefaultAppButton absoluteFilePath={absoluteFilePath} /> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/SpreadsheetViewer/SpreadsheetDiffViewer.tsx` around lines 514 - 516, The current rendering in SpreadsheetDiffViewer shows SpreadsheetDefaultAppButton when commitHash is falsy, but it still renders for working-tree removed files; update the condition to also check that diffCategory !== "removed" before rendering the button (i.e., only render SpreadsheetDefaultAppButton for non-removed working-tree files), referencing the existing commitHash, diffCategory and absoluteFilePath props/variables used in that component.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/SpreadsheetViewer/SpreadsheetDiffViewer.tsx`:
- Around line 615-617: The current SpreadsheetDiffViewer early-return collapses
any "added"/"removed" sheet into a single synthetic location by returning [0];
change this to only do that for truly empty sheets: inside the branch that
checks sheet.sheetStatus === "added" || sheet.sheetStatus === "removed" (in
SpreadsheetDiffViewer.tsx) test the sheet's row count (e.g., sheet.rows?.length,
sheet.rowCount, or the existing property that holds row data) and only return
[0] when that count is 0; otherwise fall through and preserve the real per-row
diff locations so Prev/Next can navigate actual rows.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/DatabasesView/DatabasesView.tsx`:
- Around line 696-747: The icon-only Buttons that call onOpen, onEdit, and
onRemove (inside the DatabasesView component) need accessible names: add
aria-label attributes to each Button (e.g., aria-label="Open in Explorer",
aria-label="Edit connection", aria-label="Remove connection") so screen readers
can distinguish them; apply the same change to the other identical icon-only
Button instances in this file (the blocks that render filter/history-delete
buttons) to ensure all icon-only Buttons include appropriate aria-labels
matching their TooltipContent.
- Around line 587-600: The subtitle and edit-hydration break because manual
Postgres entries now only store connectionStringId; update
getConnectionSubtitle(SavedDatabaseConnection) to handle manual Postgres by
using getManualPostgresConnectionString(connection) (or parse a redacted host/db
summary from the manual getter) when connection.connectionString is missing but
connection.connectionStringId exists, and change the edit-form hydration logic
(the block currently hydrating from the deprecated connection.connectionString
around the edit dialog code) to call getManualPostgresConnectionString to
populate the edit fields instead of reading connection.connectionString; also
ensure the store retains a redacted host/database summary for list rendering so
the subtitle remains populated for workspace-config and manual entries.
- Around line 1727-1753: The current flow updates the stored definition via
updateWorkspaceDatabaseDefinitionMutation.mutateAsync while only writing
credentials when isResettingWorkspacePassword is true, which allows changing
nextUsername without updating the saved credential blob
(saveWorkspaceDatabaseCredentialsMutation), causing a mismatch; fix by detecting
when nextUsername.trim() differs from currentEditingConnection's saved username
while a saved credential exists and, instead of only mutating the definition,
force the credential-reset path: either disable editing of username when reusing
stored credentials or trigger saveWorkspaceDatabaseCredentialsMutation so the
credential blob is updated with the new username (i.e., if nextUsername changed
and !isResettingWorkspacePassword and saved credentials exist, prompt for
password/reset and call saveWorkspaceDatabaseCredentialsMutation with the new
username/password or block the edit until credentials are updated).
- Around line 231-233: The DSN composition double-wraps already-bracketed IPv6
hosts (e.g., “[::1]” becomes “[ [::1] ]”); change the host normalization so it
first detects and preserves existing surrounding brackets on input.host (or
strips them then re-wraps correctly) and only adds brackets when a bare IPv6
address is present; update the host variable logic (the input.host -> host
assignment used in the return that builds the Postgres URL with auth, host,
input.port, databaseName, query) to handle these cases so the final
`postgres://...@${host}:...` string never contains double brackets.
---
Outside diff comments:
In `@apps/desktop/src/renderer/stores/database-sidebar.ts`:
- Around line 237-283: When rewriting a manual Postgres connection (where you
build updatedConnection from currentConnection in the SavedDatabaseConnection
flow), ensure you remove legacy migration scratch fields so the record is no
longer treated as pending: explicitly set _pendingConnectionString to undefined
and clear the deprecated connectionString when constructing updatedConnection
(in the same block that creates updatedConnection from currentConnection), and
do the same in the migration path around where _pendingConnectionString is
created (lines ~386-400) so both update paths stop re-encrypting the secret.
---
Duplicate comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/SpreadsheetViewer/SpreadsheetDiffViewer.tsx`:
- Around line 514-516: The current rendering in SpreadsheetDiffViewer shows
SpreadsheetDefaultAppButton when commitHash is falsy, but it still renders for
working-tree removed files; update the condition to also check that diffCategory
!== "removed" before rendering the button (i.e., only render
SpreadsheetDefaultAppButton for non-removed working-tree files), referencing the
existing commitHash, diffCategory and absoluteFilePath props/variables used in
that component.
🪄 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: 4aa48db6-d0bd-469d-aebe-6b5939a12630
📒 Files selected for processing (5)
apps/desktop/src/lib/trpc/routers/databases/index.tsapps/desktop/src/lib/trpc/routers/databases/workspace-config.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/SpreadsheetViewer/SpreadsheetDiffViewer.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/DatabasesView/DatabasesView.tsxapps/desktop/src/renderer/stores/database-sidebar.ts
✅ Files skipped from review due to trivial changes (1)
- apps/desktop/src/lib/trpc/routers/databases/workspace-config.ts
- Fix getDiffRowIndices to only synthesise location for empty added/removed sheets - Guard against double-bracketing already-bracketed IPv6 hosts in Postgres URLs - Store host/port/databaseName/usernameHint on manual Postgres connections for display and edit hydration - Update getConnectionSubtitle to use host metadata instead of plain connectionString - Populate edit form from store metadata instead of deprecated connectionString field - Lock username field when retaining workspace-config credentials (prevent username/password mismatch) - Add aria-label to icon-only buttons (edit, remove, filter, history remove) - Wrap disabled refresh button in span to restore tooltip visibility (ProblemsView)
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/DatabasesView/DatabasesView.tsx (1)
1695-1722:⚠️ Potential issue | 🟠 MajorValidate the final Postgres URL before saving it.
connectionStringis treated as valid as long as it is non-empty. That lets malformed raw URLs through, and it also lets manual mode persist generated URLs with invalid ports likeabc. In both cases the broken DSN is written first and only fails later when queried. Parse/validate the final URL and block the save on failure.Also applies to: 1807-1830
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/DatabasesView/DatabasesView.tsx` around lines 1695 - 1722, The form currently accepts any non-empty connectionString (either raw via useConnectionString or generated by buildPostgresConnectionString), which lets malformed DSNs (bad URLs or non-numeric ports) be saved; update the save/submit flow that uses connectionString to actually parse and validate the final Postgres URL before proceeding: after computing connectionString (in the branches that use useConnectionString and buildPostgresConnectionString, referenced by the variables connectionString, useConnectionString, buildPostgresConnectionString), attempt to parse it (e.g., with URL parsing or a Postgres DSN validator), ensure required parts exist (host present, database optional fallback), and validate port is a numeric value in 1–65535; if parsing/validation fails call setFormError with a clear message and return (also apply the same validation to the other save block around currentEditingConnection/isWorkspaceConfigConnection), preventing the save when invalid.
♻️ Duplicate comments (3)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/SpreadsheetViewer/SpreadsheetDiffViewer.tsx (1)
514-516:⚠️ Potential issue | 🟠 MajorGuard the default-app action on
diffCategoryas well.
!commitHashonly hides the action for historical views. A working-treeremoveddiff still renders a button for a path that no longer exists, so the action can fail or open the wrong file.🩹 Minimal guard
- {!commitHash && ( + {!commitHash && diffCategory !== "removed" && ( <SpreadsheetDefaultAppButton absoluteFilePath={absoluteFilePath} /> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/SpreadsheetViewer/SpreadsheetDiffViewer.tsx` around lines 514 - 516, The UI currently shows SpreadsheetDefaultAppButton when !commitHash only, which still renders for working-tree removed diffs; update the conditional that renders SpreadsheetDefaultAppButton to also check the diffCategory (e.g., only render when diffCategory !== 'removed' or when diffCategory === DiffCategory.Modified/Added as appropriate) so the button is not shown for removed paths; modify the render guard around SpreadsheetDefaultAppButton (using the existing commitHash and diffCategory props/variables) to require both conditions before rendering.apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/DatabasesView/DatabasesView.tsx (2)
709-724:⚠️ Potential issue | 🟠 MajorAdd an accessible name to the explorer button.
This is still an icon-only button. The tooltip does not provide an accessible name, so assistive tech will announce it as unlabeled. Add
aria-label="Open in Explorer".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/DatabasesView/DatabasesView.tsx` around lines 709 - 724, The icon-only explorer button (the Button component wrapped by TooltipTrigger that calls onOpen and shows TooltipContent "Open in Explorer") lacks an accessible name; add an aria-label="Open in Explorer" (or equivalent accessibleName prop) directly to that Button element so screen readers announce it while keeping the existing Tooltip/TooltipContent unchanged.
1000-1013:⚠️ Potential issue | 🔴 CriticalDon't rebuild manual Postgres DSNs from redacted edit state.
Manual edit mode clears both the raw DSN and the password, but the save path still overwrites the encrypted connection string from those fields. A label/group-only edit will therefore strip the stored password and any non-modeled URL options. Load the real DSN for edit mode, or leave the encrypted value untouched unless the user explicitly replaces it.
Also applies to: 1790-1798
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/DatabasesView/DatabasesView.tsx` around lines 1000 - 1013, When toggling into manual Postgres edit mode the current code clears the raw DSN and password via setPathInput("") and setPostgresPassword(""), which causes the saved encrypted connection string to be overwritten with redacted fields on save; instead, when entering manual mode populate the host/port/user/db/ssl fields by parsing the existing real DSN (if available) but do NOT clear or overwrite the stored encrypted connection string/state (the variable updated by setPathInput) unless the user explicitly edits the connection string or password; in practice remove the unconditional setPathInput("") and setPostgresPassword(""), ensure you preserve the existing encrypted DSN field, and only replace it when explicit user edits occur (apply same change where fields are reset at the other occurrence noted).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/lib/trpc/routers/databases/workspace-config.ts`:
- Around line 268-285: The usernameHint currently uses definition.username which
can be blank even when hasSavedCredentials is true; change usernameHint to
prefer the saved credential username by using definition.username ??
credentialStore.entries[key]?.username (keep the rest of the returned object
intact). Locate the block that builds the returned object in the
workspace-config router (uses workspaceCredentialKey, getPostgresDatabaseName,
WORKSPACE_DATABASES_CONFIG_FILE) and update the usernameHint assignment
accordingly so the renderer gets the credentialStore's username when
definition.username is undefined or empty.
- Around line 295-303: The credential store read/modify/write in
loadWorkspaceDatabaseCredentialStore/saveWorkspaceDatabaseCredentialStore (and
the analogous manual connection store code) is racy: concurrent calls can
overwrite each other. Fix by introducing a per-store mutex keyed by the store
file (e.g., workspacePath or a constant store id) and acquire the mutex before
calling loadWorkspaceDatabaseCredentialStore, modify entries (using
workspaceCredentialKey), then call saveWorkspaceDatabaseCredentialStore and
release the mutex; apply the same mutex pattern to the manual connection store
to serialize writes and prevent lost updates.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/SpreadsheetViewer/SpreadsheetDiffViewer.tsx`:
- Around line 461-472: When activeSheetIndex changes and we pick a new
preferredIndex (using pendingJumpRef, diffLocations, findPreferredDiffIndex and
setCurrentDiffIdx), also invoke the same scroll/jump routine used by the
Prev/Next buttons so both panes scroll to that diff; specifically, after
setCurrentDiffIdx(preferredIndex) call the existing jump handler (the function
that Prev/Next uses — e.g., jumpToDiffIndex/jumpToDiff/handleJumpToDiff) or set
pendingJumpRef/current to trigger the existing jump logic so the newly selected
sheet is scrolled into view.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/DatabasesView/DatabasesView.tsx`:
- Around line 1221-1253: The current useEffect loop over pendingConnections
swallows errors from saveManualPostgresConnectionStringMutation.mutateAsync,
leaving connections unusable; change the catch to surface failures (e.g., log
the error via your logger or processLogger and/or call updateConnection to set a
migration failure flag) and keep a runtime fallback by preserving the original
_pendingConnectionString on the connection until migration succeeds;
specifically update the error handling around
saveManualPostgresConnectionStringMutation.mutateAsync in the useEffect (and
consider adding a migrationFailed or pendingRuntimeConnectionString property via
updateConnection for the connection id) so failures are visible and the UI/code
can continue to use the pending connection string until a successful migration.
- Around line 520-537: The button inside the Tooltip/TooltipTrigger (the element
with onClick calling onOpenDetail and icon LuSearch) is only revealed via
group-hover, making it effectively invisible to keyboard users; update its
className to also show on keyboard focus by adding a focus-visible or focus
class that sets opacity to 100 (e.g., include "focus-visible:opacity-100" or
"focus:opacity-100" alongside the existing "group-hover:opacity-100") so
tabbing/focusing the button makes it visible and operable for keyboard users.
---
Outside diff comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/DatabasesView/DatabasesView.tsx`:
- Around line 1695-1722: The form currently accepts any non-empty
connectionString (either raw via useConnectionString or generated by
buildPostgresConnectionString), which lets malformed DSNs (bad URLs or
non-numeric ports) be saved; update the save/submit flow that uses
connectionString to actually parse and validate the final Postgres URL before
proceeding: after computing connectionString (in the branches that use
useConnectionString and buildPostgresConnectionString, referenced by the
variables connectionString, useConnectionString, buildPostgresConnectionString),
attempt to parse it (e.g., with URL parsing or a Postgres DSN validator), ensure
required parts exist (host present, database optional fallback), and validate
port is a numeric value in 1–65535; if parsing/validation fails call
setFormError with a clear message and return (also apply the same validation to
the other save block around
currentEditingConnection/isWorkspaceConfigConnection), preventing the save when
invalid.
---
Duplicate comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/SpreadsheetViewer/SpreadsheetDiffViewer.tsx`:
- Around line 514-516: The UI currently shows SpreadsheetDefaultAppButton when
!commitHash only, which still renders for working-tree removed diffs; update the
conditional that renders SpreadsheetDefaultAppButton to also check the
diffCategory (e.g., only render when diffCategory !== 'removed' or when
diffCategory === DiffCategory.Modified/Added as appropriate) so the button is
not shown for removed paths; modify the render guard around
SpreadsheetDefaultAppButton (using the existing commitHash and diffCategory
props/variables) to require both conditions before rendering.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/DatabasesView/DatabasesView.tsx`:
- Around line 709-724: The icon-only explorer button (the Button component
wrapped by TooltipTrigger that calls onOpen and shows TooltipContent "Open in
Explorer") lacks an accessible name; add an aria-label="Open in Explorer" (or
equivalent accessibleName prop) directly to that Button element so screen
readers announce it while keeping the existing Tooltip/TooltipContent unchanged.
- Around line 1000-1013: When toggling into manual Postgres edit mode the
current code clears the raw DSN and password via setPathInput("") and
setPostgresPassword(""), which causes the saved encrypted connection string to
be overwritten with redacted fields on save; instead, when entering manual mode
populate the host/port/user/db/ssl fields by parsing the existing real DSN (if
available) but do NOT clear or overwrite the stored encrypted connection
string/state (the variable updated by setPathInput) unless the user explicitly
edits the connection string or password; in practice remove the unconditional
setPathInput("") and setPostgresPassword(""), ensure you preserve the existing
encrypted DSN field, and only replace it when explicit user edits occur (apply
same change where fields are reset at the other occurrence noted).
🪄 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: 97a2ccbf-63c8-4088-b95c-2b63fc308b56
📒 Files selected for processing (4)
apps/desktop/src/lib/trpc/routers/databases/workspace-config.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/SpreadsheetViewer/SpreadsheetDiffViewer.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/DatabasesView/DatabasesView.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ProblemsView/ProblemsView.tsx
- Fall back to credential-store username for usernameHint when definition.username is absent - Serialize credential-store and manual-connection-store writes with per-file mutex to prevent race conditions - Scroll to preferred diff row when switching sheets in SpreadsheetDiffViewer - Show value-detail button on keyboard focus (group-focus-within / focus-visible) - Surface migration errors to user via toast instead of silently swallowing them
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/SpreadsheetViewer/SpreadsheetDiffViewer.tsx (1)
515-517:⚠️ Potential issue | 🟡 MinorStill missing
diffCategory !== "removed"guard.The
!commitHashcheck was added, but for removed-file diffs, the working-tree file atabsoluteFilePathmay not exist. Per the past review, both conditions should be checked.🩹 Proposed fix
- {!commitHash && ( + {!commitHash && diffCategory !== "removed" && ( <SpreadsheetDefaultAppButton absoluteFilePath={absoluteFilePath} /> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/SpreadsheetViewer/SpreadsheetDiffViewer.tsx` around lines 515 - 517, The conditional rendering for SpreadsheetDefaultAppButton only checks commitHash but misses the removed-file case; update the JSX condition where SpreadsheetDefaultAppButton is rendered to require both !commitHash and diffCategory !== "removed" (use the existing diffCategory, commitHash, and absoluteFilePath identifiers) so the button is not shown when the diffCategory is "removed" and the working-tree file may not exist.apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/DatabasesView/DatabasesView.tsx (1)
709-724:⚠️ Potential issue | 🟡 MinorMissing
aria-labelon "Open in Explorer" button.The Edit and Remove buttons correctly have
aria-labelattributes, but the "Open in Explorer" button is missing one. This is inconsistent with the accessibility pattern applied to sibling buttons.Suggested fix
<Tooltip> <TooltipTrigger asChild> <Button variant="ghost" size="icon" type="button" + aria-label="Open in Explorer" className="size-7 shrink-0 opacity-70 group-hover:opacity-100" onClick={onOpen} > <LuExternalLink className="size-3.5" /> </Button> </TooltipTrigger>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/DatabasesView/DatabasesView.tsx` around lines 709 - 724, The "Open in Explorer" icon Button inside the Tooltip (the Button with props variant="ghost" size="icon" onClick={onOpen} containing <LuExternalLink />) is missing an accessible aria-label; add an aria-label (e.g., aria-label="Open in Explorer") to that Button to match the Edit/Remove buttons and ensure screen readers convey the button's purpose while keeping existing Tooltip/TooltipTrigger behavior.
🧹 Nitpick comments (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/DatabasesView/DatabasesView.tsx (2)
1610-1848: Consider extractinghandleAddConnectioninto smaller functions.This function spans ~240 lines and handles multiple connection types (sqlite, postgres), edit modes (workspace-config, manual), and form modes (connection string vs. individual fields). While functionally correct, extracting dialect-specific handlers (e.g.,
handleAddSqliteConnection,handleAddPostgresConnection) would improve readability and maintainability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/DatabasesView/DatabasesView.tsx` around lines 1610 - 1848, The handleAddConnection function is too large and should be split into smaller dialect- and mode-specific handlers; extract the sqlite branch into a handleAddSqliteConnection function and the postgres branch into handleAddPostgresConnection (with sub-helpers for workspace-config vs manual/save connection-string logic), move mutation calls like updateWorkspaceDatabaseDefinitionMutation.mutateAsync, saveWorkspaceDatabaseCredentialsMutation.mutateAsync, saveManualPostgresConnectionStringMutation.mutateAsync and helper logic (resolveSQLiteDatabasePath, buildPostgresConnectionString, parsePostgresConnectionString, guessConnectionLabel/guessPostgresLabel) into those new functions, and have handleAddConnection only gather form values, call the appropriate handler based on connectionType and useConnectionString, then call resetConnectionForm() and setIsAddConnectionOpen(false); ensure you preserve existing error handling and calls to updateConnection/addConnection/discoverQuery.refetch in the extracted functions.
1067-1079: Consider removing fallback object when query is disabled.The fallback
{ kind: "connectionString", connectionStringId: "" }(lines 1069-1072) is never used whenactivePostgresConnectionInputis null becauseenabledis false. However, this pattern could mask TypeScript errors if the schema changes. Consider using a non-null assertion or conditional input construction.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/DatabasesView/DatabasesView.tsx` around lines 1067 - 1079, Remove the unused fallback object passed into electronTrpc.databases.inspectPostgres.useQuery and instead only pass a valid input when the query is enabled: build the query args conditionally (or use a non-null assertion on activePostgresConnectionInput) so inspectPostgresQuery receives a definite Postgres connection input only when enabled; update the call around inspectPostgresQuery and the enabled predicate that references activeConnection?.dialect === "postgres" to avoid supplying the placeholder { kind: "connectionString", connectionStringId: "" } when activePostgresConnectionInput is null.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/DatabasesView/DatabasesView.tsx`:
- Around line 593-614: The subtitle fallback uses "postgres" as a host which is
misleading; update getConnectionSubtitle to stop using connection.host ??
"postgres" and instead pass an empty string (connection.host ?? "") or omit the
host when calling getWorkspaceConfigPostgresLabel so the label won't show a
bogus host; adjust both places where getWorkspaceConfigPostgresLabel is called
in getConnectionSubtitle (the workspace-config branch and the host branch) to
pass connection.host ?? "" and ensure the label builder handles an empty host
appropriately.
- Around line 1241-1248: The updateConnection call is not clearing the internal
_pendingConnectionString, so migrated connections keep that field and retrigger
the migration; update the payload passed to updateConnection (the call that
currently sets id, label, group, dialect, source, connectionStringId) to also
set _pendingConnectionString: undefined so the field is removed after successful
migration, or alternatively modify the store method updateConnection to clear
_pendingConnectionString when source === "manual" (ensure references to
connection and updateConnection are used so the pending value is dropped).
---
Duplicate comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/SpreadsheetViewer/SpreadsheetDiffViewer.tsx`:
- Around line 515-517: The conditional rendering for SpreadsheetDefaultAppButton
only checks commitHash but misses the removed-file case; update the JSX
condition where SpreadsheetDefaultAppButton is rendered to require both
!commitHash and diffCategory !== "removed" (use the existing diffCategory,
commitHash, and absoluteFilePath identifiers) so the button is not shown when
the diffCategory is "removed" and the working-tree file may not exist.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/DatabasesView/DatabasesView.tsx`:
- Around line 709-724: The "Open in Explorer" icon Button inside the Tooltip
(the Button with props variant="ghost" size="icon" onClick={onOpen} containing
<LuExternalLink />) is missing an accessible aria-label; add an aria-label
(e.g., aria-label="Open in Explorer") to that Button to match the Edit/Remove
buttons and ensure screen readers convey the button's purpose while keeping
existing Tooltip/TooltipTrigger behavior.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/DatabasesView/DatabasesView.tsx`:
- Around line 1610-1848: The handleAddConnection function is too large and
should be split into smaller dialect- and mode-specific handlers; extract the
sqlite branch into a handleAddSqliteConnection function and the postgres branch
into handleAddPostgresConnection (with sub-helpers for workspace-config vs
manual/save connection-string logic), move mutation calls like
updateWorkspaceDatabaseDefinitionMutation.mutateAsync,
saveWorkspaceDatabaseCredentialsMutation.mutateAsync,
saveManualPostgresConnectionStringMutation.mutateAsync and helper logic
(resolveSQLiteDatabasePath, buildPostgresConnectionString,
parsePostgresConnectionString, guessConnectionLabel/guessPostgresLabel) into
those new functions, and have handleAddConnection only gather form values, call
the appropriate handler based on connectionType and useConnectionString, then
call resetConnectionForm() and setIsAddConnectionOpen(false); ensure you
preserve existing error handling and calls to
updateConnection/addConnection/discoverQuery.refetch in the extracted functions.
- Around line 1067-1079: Remove the unused fallback object passed into
electronTrpc.databases.inspectPostgres.useQuery and instead only pass a valid
input when the query is enabled: build the query args conditionally (or use a
non-null assertion on activePostgresConnectionInput) so inspectPostgresQuery
receives a definite Postgres connection input only when enabled; update the call
around inspectPostgresQuery and the enabled predicate that references
activeConnection?.dialect === "postgres" to avoid supplying the placeholder {
kind: "connectionString", connectionStringId: "" } when
activePostgresConnectionInput is null.
🪄 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: ef7bc0ef-2c8b-4d87-ad72-86b5a7cba189
📒 Files selected for processing (3)
apps/desktop/src/lib/trpc/routers/databases/workspace-config.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/SpreadsheetViewer/SpreadsheetDiffViewer.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/DatabasesView/DatabasesView.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/lib/trpc/routers/databases/workspace-config.ts
- Use "unknown" instead of "postgres" as host fallback in getConnectionSubtitle - Clear _pendingConnectionString after successful migration to prevent repeated useEffect runs
- Skip credential store re-save when editing manual Postgres without changing password, preventing blank DSN overwrite - Explicitly clear _pendingConnectionString in updateConnection store to stop repeated migration loop - Reject deep-link file params that resolve outside the workspace root to prevent arbitrary local file access
This was referenced Apr 3, 2026
Merged
This was referenced Apr 11, 2026
MocA-Love
added a commit
that referenced
this pull request
Apr 14, 2026
2 tasks
This was referenced Apr 15, 2026
14 tasks
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.
#49
#50
変更概要
このPRは desktop アプリを中心に、複数の大規模機能追加・改善をまとめたものです。
主な変更点
Docker サイドバーの追加・改善
DockerView.tsx）InspectCodeBlock）を追加apps/desktop/src/lib/trpc/routers/docker/index.tsに Docker 用 tRPC ルーターを追加検索ビュー（SearchView）の大幅強化
@tanstack/react-virtualによる仮想スクロールで大量検索結果のパフォーマンスを大幅改善SearchTreeNodeコンポーネントを新規追加し、ツリー形式の検索結果表示に対応React.memo/useMemo/useCallbackによる再レンダリング最適化ワークスペースデータベース設定の読み込み・編集
feat(desktop): load workspace database configs— ワークスペース DB 設定の読み込み機能を追加workspace-config.tsルーターを新規追加し、ワークスペース DB 設定の CRUD に対応DatabasesView.tsxに設定編集 UI を追加database-sidebar.tsストアを拡張スプレッドシート改善
SpreadsheetDefaultAppButton）を追加WorkspaceFilePreviewContentでabsoluteFilePathを正しく渡すよう修正共有可能な Superset ファイルリンク
apps/open-link/に Cloudflare Workers ベースのリンク解決ワーカーを新規追加superset-open-links.ts）右サイドバーのタブ並び替えとオーバーフローメニュー
index.tsx）ブラウザペイン・クイックオープン改善
その他
RepositoryPanelの Biome lint 修正diff-theme.tsをリファクタリングしshiki-theme.tsを分離code-block.tsx・command.tsx（packages/ui）の改善Summary by CodeRabbit
New Features
Improvements