Add workspace hover cards with GitHub PR status#233
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded@saddlepaddle has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 15 minutes and 53 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughAdds a GitHub PR status fetcher using the gh CLI with 30s caching, exposes two TRPC endpoints to surface worktree and GitHub info, extends DB schema with check items and PR fields, and introduces a hover-card UI (plus components) to display PR details and checks for workspace tabs. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant UI as Workspace Tab
participant Hover as HoverCard
participant TRPC as Workspaces Router
participant GHUtil as GitHub Utility
participant GHCli as gh/git CLI
User->>UI: hover over workspace tab
UI->>Hover: open hovercard
Hover->>TRPC: getWorktreeInfo(workspaceId)
TRPC->>GHUtil: fetchGitHubPRStatus(worktreePath)
GHUtil->>GHCli: git rev-parse HEAD (get branch/ref)
GHCli-->>GHUtil: ref/hash
GHUtil->>GHCli: gh repo view --json url
GHCli-->>GHUtil: repo URL JSON
GHUtil->>GHCli: gh pr view <ref> --json <fields>
GHCli-->>GHUtil: PR + checks JSON
GHUtil->>GHUtil: validate/parse/map checks, compute checksStatus
GHUtil-->>TRPC: GitHubStatus | null
TRPC-->>Hover: worktreeInfo + githubStatus
Hover->>User: render PR details, badges, checks, and links
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–60 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts (2)
500-531: Consider adding request deduplication or throttling for frequent hovers.The endpoint always fetches fresh data on every hover. If users rapidly hover over workspace tabs (e.g., moving mouse across many tabs), this could trigger many concurrent
ghCLI calls. Consider:
- Using tRPC's built-in deduplication via
staleTimeon the client side- Adding a short cache TTL check before fetching (e.g., skip fetch if last refresh was < 30s ago)
Example server-side throttle:
getGitHubStatus: publicProcedure .input(z.object({ workspaceId: z.string() })) .query(async ({ input }) => { const workspace = db.data.workspaces.find( (w) => w.id === input.workspaceId, ); if (!workspace) { return null; } const worktree = db.data.worktrees.find( (wt) => wt.id === workspace.worktreeId, ); if (!worktree) { return null; } + // Return cached data if recently fetched (< 30s ago) + const CACHE_TTL_MS = 30_000; + if ( + worktree.githubStatus && + Date.now() - worktree.githubStatus.lastRefreshed < CACHE_TTL_MS + ) { + return worktree.githubStatus; + } + // Always fetch fresh data on hover const freshStatus = await fetchGitHubPRStatus(worktree.path);
550-551: Path extraction could yield empty string for trailing-slash paths.
worktree.path.split("/").pop()returns an empty string if the path ends with/(e.g.,/path/to/worktree/). Consider using Node'spath.basenamewhich handles this correctly.+import { basename } from "node:path"; // ... - const worktreeName = worktree.path.split("/").pop() ?? worktree.branch; + const worktreeName = basename(worktree.path) || worktree.branch;apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/WorkspaceHoverCard.tsx (2)
11-11: Consider re-exporting types from a shared location.The renderer imports the
CheckItemtype directly frommain/lib/db/schemas. While this works for TypeScript types (no runtime Node.js dependency), it creates a coupling between renderer and main process code paths. Consider re-exporting shared types from ashared/orlib/location to keep the boundary cleaner.
122-281: Multiple helper components in the same file.The coding guidelines state "One component per file." However, these are tightly-coupled internal subcomponents (
PRStatusBadge,ChecksSummary,ChecksList,CheckItemRow,ReviewStatus) that are only used withinWorkspaceHoverCardContent. Keeping them co-located is reasonable for maintainability. Consider extracting to acomponents/subdirectory if they grow in complexity or are reused elsewhere.apps/desktop/src/lib/trpc/routers/workspaces/utils/github.ts (4)
94-98: Potential shell injection via branch name.While branch names from
git rev-parseare typically safe, maliciously crafted branch names (e.g., containing backticks or$()) could lead to command injection. Consider using a shell-safe approach.Use an array-based approach or quote the branch name properly:
- const { stdout } = await execAsync( - `gh pr view ${branch} --json number,title,url,state,isDraft,mergedAt,additions,deletions,reviewDecision,statusCheckRollup`, - { cwd: worktreePath }, - ); + const { stdout } = await execAsync( + `gh pr view "${branch.replace(/"/g, '\\"')}" --json number,title,url,state,isDraft,mergedAt,additions,deletions,reviewDecision,statusCheckRollup`, + { cwd: worktreePath }, + );Alternatively, consider using
execFile(which doesn't spawn a shell) with argument arrays for better security.
115-125: Fragile error detection via string matching.Checking for
"no pull requests found"in the error message is brittle—GitHub CLI could change wording or localization could affect it.Consider checking the exit code or using a more robust pattern:
} catch (error) { - // "no pull requests found" is not an error - just no PR - if ( - error instanceof Error && - error.message.includes("no pull requests found") - ) { + // gh returns exit code 1 with "no pull requests found" when no PR exists + if (error instanceof Error && /no pull requests? found/i.test(error.message)) { return null; } // Re-throw other errors to be caught by parent throw error; }This at least handles singular/plural variations and case differences.
160-174:ACTION_REQUIREDconclusion is not explicitly handled.The
GHCheckContexttype includesACTION_REQUIREDas a possible conclusion (line 18), but it's not mapped inparseChecks. It falls through to"pending", which may be the intended behavior but is implicit.Consider explicitly handling
ACTION_REQUIREDfor clarity:} else if (rawStatus === "CANCELLED") { status = "cancelled"; + } else if (rawStatus === "ACTION_REQUIRED") { + status = "pending"; // Requires manual intervention, treat as pending } else { status = "pending"; }
48-70: Silent error handling may hinder debugging.Both
fetchGitHubPRStatusandgetRepoUrlcatch all errors and returnnullsilently. While graceful degradation is good for UX, this could make debugging harder when issues occur.Consider adding debug-level logging for unexpected errors:
} catch (error) { - // Any error (gh not installed, not auth'd, etc.) - return null + // Any error (gh not installed, not auth'd, etc.) - log and return null + if (process.env.NODE_ENV === "development") { + console.debug("[github] fetchGitHubPRStatus failed:", error); + } return null; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/desktop/src/lib/trpc/routers/workspaces/utils/github.ts(1 hunks)apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts(2 hunks)apps/desktop/src/main/lib/db/schemas.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/WorkspaceHoverCard.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItemContextMenu.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
apps/desktop/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
For Electron interprocess communication, ALWAYS use tRPC as defined in
src/lib/trpc
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/index.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/WorkspaceHoverCard.tsxapps/desktop/src/lib/trpc/routers/workspaces/utils/github.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItemContextMenu.tsxapps/desktop/src/main/lib/db/schemas.tsapps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: Please use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/index.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/WorkspaceHoverCard.tsxapps/desktop/src/lib/trpc/routers/workspaces/utils/github.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItemContextMenu.tsxapps/desktop/src/main/lib/db/schemas.tsapps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid usinganytype - use explicit types instead for type safety
Use camelCase for variable and function names following existing codebase patterns
Keep diffs minimal with targeted edits only - avoid unnecessary changes when making modifications
Follow existing patterns and match the codebase style when writing new code
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/index.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/WorkspaceHoverCard.tsxapps/desktop/src/lib/trpc/routers/workspaces/utils/github.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItemContextMenu.tsxapps/desktop/src/main/lib/db/schemas.tsapps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules (fs, path, os, net, etc.) in renderer process code - browser environment only
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/index.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/WorkspaceHoverCard.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItemContextMenu.tsx
**/components/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
**/components/**/*.tsx: Create one folder per component with structure:ComponentName/ComponentName.tsx+index.tsfor barrel export
Co-locate tests next to the component file they test (e.g.,ComponentName.test.tsx)
Co-locate dependencies (utils, hooks, constants, config, stories) next to the file using them
Use nestedcomponents/subdirectory within a parent component only if a sub-component is used 2+ times within that parent; otherwise keep it in the parent'scomponents/folder
One component per file - avoid multi-component files
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/WorkspaceHoverCard.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItemContextMenu.tsx
apps/desktop/src/renderer/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Call IPC methods using
window.ipcRenderer.invoke()with object parameters - TypeScript will infer the exact response type automatically
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/WorkspaceHoverCard.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItemContextMenu.tsx
apps/desktop/src/lib/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules in shared code like
src/lib/electron-router-dom.ts- this code runs in both main and renderer processes
Files:
apps/desktop/src/lib/trpc/routers/workspaces/utils/github.tsapps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts
apps/desktop/src/main/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Node.js modules (fs, path, os, net, etc.) can be used in main process code only
Files:
apps/desktop/src/main/lib/db/schemas.ts
🧬 Code graph analysis (3)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/WorkspaceHoverCard.tsx (1)
apps/desktop/src/main/lib/db/schemas.ts (1)
CheckItem(17-21)
apps/desktop/src/lib/trpc/routers/workspaces/utils/github.ts (1)
apps/desktop/src/main/lib/db/schemas.ts (2)
GitHubStatus(23-38)CheckItem(17-21)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItemContextMenu.tsx (1)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/WorkspaceHoverCard.tsx (1)
WorkspaceHoverCardContent(20-120)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (10)
apps/desktop/src/main/lib/db/schemas.ts (2)
17-21: LGTM!The
CheckIteminterface is well-defined with appropriate status union types covering GitHub CI states. The optionalurlfield correctly handles checks that may not have associated links.
30-34: LGTM!The extended
GitHubStatus.prfields are comprehensive and align well with GitHub's PR data model. The distinction betweenchecksStatus(aggregate) andchecks(individual items) provides flexibility for both summary and detailed views.apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/index.ts (1)
1-1: LGTM!Clean barrel export following the component folder structure guideline.
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsx (1)
80-84: LGTM!Clean prop addition enabling the context menu to access workspace-specific data for the hover card feature.
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/WorkspaceHoverCard.tsx (2)
20-120: LGTM!The
WorkspaceHoverCardContentcomponent is well-structured with clear conditional rendering for loading, PR present, and no-PR states. Good use of TRPC queries with properenabledguards.
283-302: LGTM!The
formatRelativeTimehelper is simple and effective. It avoids external dependencies for a straightforward use case.apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItemContextMenu.tsx (2)
8-15: LGTM on imports and module structure.The imports are properly organized, using the UI library's hover-card primitives and the local
WorkspaceHoverCardContentcomponent. This follows the existing codebase patterns.
38-55: Clean composition of HoverCard with ContextMenu.The nested trigger pattern (
HoverCardTriggerwrappingContextMenuTrigger) withasChildis the correct approach for combining these Radix-based primitives. The delay values (400ms open, 100ms close) provide a good balance between responsiveness and avoiding accidental triggers.apps/desktop/src/lib/trpc/routers/workspaces/utils/github.ts (2)
1-5: LGTM on imports.Using Node.js built-in modules is appropriate here since this file is in the tRPC routers directory which runs in the main process.
180-208: LGTM oncomputeChecksStatuslogic.The function correctly prioritizes failure over pending, and treats empty/missing rollups as "none". The handling of
null/undefinedstatus as pending is appropriate for in-progress checks.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ReviewStatus/ReviewStatus.tsx (1)
1-18: Tighten config typing to keep status mapping in syncImplementation is clear. To make this future-proof if you extend the status union, you can tie the config type directly to
ReviewStatusProps["status"]so missing mappings become a TS error:-export function ReviewStatus({ status }: ReviewStatusProps) { - const config = { +export function ReviewStatus({ status }: ReviewStatusProps) { + const config: Record< + ReviewStatusProps["status"], + { label: string; className: string } + > = { approved: { label: "Approved", className: "text-emerald-500" }, @@ const { label, className } = config[status];Purely a type-safety nicety; the current runtime behavior is fine.
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/components/CheckItemRow/CheckItemRow.tsx (1)
1-42: Status-to-icon mapping is solid; consider stronger typing for future changesThe visual mapping and link handling look good, and covering all
CheckItemstatuses makes this easy to scan.For extra type safety if
CheckItem["status"]ever grows, you can tiestatusConfigto that union so missing mappings fail at compile time:-export function CheckItemRow({ check }: CheckItemRowProps) { - const statusConfig = { +export function CheckItemRow({ check }: CheckItemRowProps) { + const statusConfig: Record< + CheckItem["status"], + { icon: typeof Check; className: string } + > = { success: { icon: Check, className: "text-emerald-500" }, failure: { icon: X, className: "text-destructive-foreground" }, pending: { icon: LoaderCircle, className: "text-amber-500" }, skipped: { icon: Minus, className: "text-muted-foreground" }, cancelled: { icon: Minus, className: "text-muted-foreground" }, };Optional, but helps keep things in sync if backend statuses evolve.
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/PRStatusBadge/PRStatusBadge.tsx (1)
1-27: PRStatusBadge mapping is clear; small type-safety improvement possibleThe badge styling/labels are straightforward and match the state union. Similar to
ReviewStatus, you can bind the maps to the prop union so adding a new state forces you to update both:export function PRStatusBadge({ state }: PRStatusBadgeProps) { - const styles = { + const styles: Record<PRStatusBadgeProps["state"], string> = { open: "bg-emerald-500/15 text-emerald-500", draft: "bg-muted text-muted-foreground", merged: "bg-violet-500/15 text-violet-500", closed: "bg-destructive/15 text-destructive-foreground", }; - const labels = { + const labels: Record<PRStatusBadgeProps["state"], string> = { open: "Open", draft: "Draft", merged: "Merged", closed: "Closed", };Not required, but makes the component safer to extend later.
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/WorkspaceHoverCard.tsx (1)
18-27: Consider handling query error states.Both tRPC queries handle loading state but silently fail on errors. If
getGitHubStatuserrors (e.g.,ghCLI unavailable), the component falls through to the "No PR" state, which may be acceptable as a graceful fallback. However,getWorktreeInfoerrors would leave the header section empty without feedback.If this graceful degradation is intentional, consider adding a brief inline comment for future maintainers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
AGENTS.md(2 hunks)apps/desktop/package.json(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/WorkspaceHoverCard.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/ChecksList.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/components/CheckItemRow/CheckItemRow.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/components/CheckItemRow/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksSummary/ChecksSummary.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksSummary/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/PRStatusBadge/PRStatusBadge.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/PRStatusBadge/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ReviewStatus/ReviewStatus.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ReviewStatus/index.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksSummary/index.ts
- apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ReviewStatus/index.ts
🧰 Additional context used
📓 Path-based instructions (7)
apps/desktop/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
For Electron interprocess communication, ALWAYS use tRPC as defined in
src/lib/trpc
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ReviewStatus/ReviewStatus.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/index.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksSummary/ChecksSummary.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/components/CheckItemRow/index.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/components/CheckItemRow/CheckItemRow.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/PRStatusBadge/index.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/PRStatusBadge/PRStatusBadge.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/ChecksList.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/WorkspaceHoverCard.tsx
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: Please use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ReviewStatus/ReviewStatus.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/index.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksSummary/ChecksSummary.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/components/CheckItemRow/index.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/components/CheckItemRow/CheckItemRow.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/PRStatusBadge/index.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/PRStatusBadge/PRStatusBadge.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/ChecksList.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/WorkspaceHoverCard.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid usinganytype - use explicit types instead for type safety
Use camelCase for variable and function names following existing codebase patterns
Keep diffs minimal with targeted edits only - avoid unnecessary changes when making modifications
Follow existing patterns and match the codebase style when writing new code
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ReviewStatus/ReviewStatus.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/index.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksSummary/ChecksSummary.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/components/CheckItemRow/index.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/components/CheckItemRow/CheckItemRow.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/PRStatusBadge/index.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/PRStatusBadge/PRStatusBadge.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/ChecksList.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/WorkspaceHoverCard.tsx
**/components/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
**/components/**/*.tsx: Create one folder per component with structure:ComponentName/ComponentName.tsx+index.tsfor barrel export
Co-locate tests next to the component file they test (e.g.,ComponentName.test.tsx)
Co-locate dependencies (utils, hooks, constants, config, stories) next to the file using them
Use nestedcomponents/subdirectory within a parent component only if a sub-component is used 2+ times within that parent; otherwise keep it in the parent'scomponents/folder
One component per file - avoid multi-component files
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ReviewStatus/ReviewStatus.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksSummary/ChecksSummary.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/components/CheckItemRow/CheckItemRow.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/PRStatusBadge/PRStatusBadge.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/ChecksList.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/WorkspaceHoverCard.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules (fs, path, os, net, etc.) in renderer process code - browser environment only
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ReviewStatus/ReviewStatus.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/index.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksSummary/ChecksSummary.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/components/CheckItemRow/index.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/components/CheckItemRow/CheckItemRow.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/PRStatusBadge/index.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/PRStatusBadge/PRStatusBadge.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/ChecksList.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/WorkspaceHoverCard.tsx
apps/desktop/src/renderer/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Call IPC methods using
window.ipcRenderer.invoke()with object parameters - TypeScript will infer the exact response type automatically
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ReviewStatus/ReviewStatus.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksSummary/ChecksSummary.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/components/CheckItemRow/CheckItemRow.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/PRStatusBadge/PRStatusBadge.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/ChecksList.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/WorkspaceHoverCard.tsx
AGENTS.md
📄 CodeRabbit inference engine (CLAUDE.md)
Document agent responsibilities and interactions in AGENTS.md
Files:
AGENTS.md
🧠 Learnings (13)
📚 Learning: 2025-11-24T21:32:21.725Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/CLAUDE.md:0-0
Timestamp: 2025-11-24T21:32:21.725Z
Learning: Applies to apps/desktop/**/AGENTS.md : Document agent responsibilities, capabilities, and interaction patterns in AGENTS.md
Applied to files:
AGENTS.md
📚 Learning: 2025-11-24T21:32:17.800Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T21:32:17.800Z
Learning: Applies to AGENTS.md : Document agent responsibilities and interactions in AGENTS.md
Applied to files:
AGENTS.md
📚 Learning: 2025-11-28T01:03:47.963Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T01:03:47.963Z
Learning: Applies to **/components/**/*.tsx : Co-locate dependencies (utils, hooks, constants, config, stories) next to the file using them
Applied to files:
AGENTS.md
📚 Learning: 2025-11-28T01:03:47.963Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T01:03:47.963Z
Learning: Applies to **/components/**/*.tsx : Create one folder per component with structure: `ComponentName/ComponentName.tsx` + `index.ts` for barrel export
Applied to files:
AGENTS.md
📚 Learning: 2025-11-28T01:03:47.963Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T01:03:47.963Z
Learning: Applies to **/components/**/*.tsx : Co-locate tests next to the component file they test (e.g., `ComponentName.test.tsx`)
Applied to files:
AGENTS.md
📚 Learning: 2025-11-28T01:03:47.963Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T01:03:47.963Z
Learning: Applies to **/components/**/*.tsx : Use nested `components/` subdirectory within a parent component only if a sub-component is used 2+ times within that parent; otherwise keep it in the parent's `components/` folder
Applied to files:
AGENTS.md
📚 Learning: 2025-11-28T01:03:47.963Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T01:03:47.963Z
Learning: Applies to **/components/**/*.tsx : One component per file - avoid multi-component files
Applied to files:
AGENTS.md
📚 Learning: 2025-11-28T01:03:47.963Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T01:03:47.963Z
Learning: Applies to packages/ui/src/**/*.tsx : Use shadcn/ui components and TailwindCSS v4 for all UI component styling in the shared UI package
Applied to files:
AGENTS.md
📚 Learning: 2025-11-28T01:03:47.963Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T01:03:47.963Z
Learning: Applies to packages/db/src/**/*.ts : Define all database schema in `packages/db/src/` using Drizzle ORM
Applied to files:
AGENTS.md
📚 Learning: 2025-11-28T01:03:47.963Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T01:03:47.963Z
Learning: Applies to **/*.{ts,tsx} : Follow existing patterns and match the codebase style when writing new code
Applied to files:
AGENTS.md
📚 Learning: 2025-11-28T01:03:47.963Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T01:03:47.963Z
Learning: Applies to **/*.{ts,tsx} : Use camelCase for variable and function names following existing codebase patterns
Applied to files:
AGENTS.md
📚 Learning: 2025-11-24T21:33:13.267Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.267Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary
Applied to files:
AGENTS.md
📚 Learning: 2025-11-24T21:33:13.267Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.267Z
Learning: Applies to apps/desktop/**/*.test.{ts,tsx,js,jsx} : Tests should be repeatable
Applied to files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/WorkspaceHoverCard.tsx
🧬 Code graph analysis (5)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ReviewStatus/ReviewStatus.tsx (1)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ReviewStatus/index.ts (1)
ReviewStatus(1-1)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksSummary/ChecksSummary.tsx (2)
apps/desktop/src/main/lib/db/schemas.ts (1)
CheckItem(17-21)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksSummary/index.ts (1)
ChecksSummary(1-1)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/components/CheckItemRow/CheckItemRow.tsx (2)
apps/desktop/src/main/lib/db/schemas.ts (1)
CheckItem(17-21)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/components/CheckItemRow/index.ts (1)
CheckItemRow(1-1)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/PRStatusBadge/PRStatusBadge.tsx (1)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/PRStatusBadge/index.ts (1)
PRStatusBadge(1-1)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/ChecksList.tsx (1)
apps/desktop/src/main/lib/db/schemas.ts (1)
CheckItem(17-21)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (9)
apps/desktop/package.json (1)
55-55: New date utility dependency looks appropriate for hover-card time formattingAdding a date utility here makes sense for the worktree age/relative time display in the hover card; no issues from a package-layout perspective. Please just confirm that your Electron/Vite setup is happy with the current major version of this library (ESM expectations, tree-shaking, etc.).
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/index.ts (1)
1-1: Barrel export is consistent with component structureRe-exporting
ChecksListfrom anindex.tsmatches the component-folder guideline and keeps imports clean.apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/PRStatusBadge/index.ts (1)
1-1: PRStatusBadge barrel export matches existing patternsThis keeps consumer imports tidy and matches how other hover-card subcomponents are exposed.
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/WorkspaceHoverCard.tsx (1)
32-113: LGTM!The component structure is well-organized with clear state handling for loading, PR present, and no-PR scenarios. The conditional rendering logic is clean and the sub-components are properly composed.
AGENTS.md (2)
80-103: LGTM!The expanded project structure documentation provides clear guidance on organizing hooks, utils, stores, and providers alongside components. This aligns well with the co-location principles in the coding guidelines.
141-143: LGTM!Good addition documenting the shadcn/ui exception for kebab-case files. This prevents confusion when developers encounter the different naming convention in UI component directories.
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksSummary/ChecksSummary.tsx (2)
9-42: LGTM!The component logic is clean with proper handling of all status states. The passing/total calculation correctly excludes skipped and cancelled checks, and the config-driven icon mapping is a nice pattern.
2-2: The import ofCheckItemfrommain/lib/db/schemasin renderer code is acceptable—it's a type-only import (erased at runtime) and uses the alias correctly per AGENTS.md guidelines. While moving this type tosrc/shared/types.tswould be a reasonable refactoring since shared types are co-located there, AGENTS.md contains no architectural rule requiring this change. The current pattern is consistent with how other database schemas are structured.apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/components/CheckItemRow/index.ts (1)
1-1: LGTM! Standard barrel export pattern.The re-export follows common TypeScript/React conventions, allowing cleaner imports of the CheckItemRow component.
| interface ChecksListProps { | ||
| checks: CheckItem[]; | ||
| } | ||
|
|
||
| export function ChecksList({ checks }: ChecksListProps) { | ||
| const [expanded, setExpanded] = useState(false); | ||
|
|
||
| // Filter out skipped/cancelled for display count, but show all when expanded | ||
| const relevantChecks = checks.filter( | ||
| (c) => c.status !== "skipped" && c.status !== "cancelled", | ||
| ); | ||
|
|
||
| if (relevantChecks.length === 0) return null; | ||
|
|
||
| return ( | ||
| <div className="text-xs"> | ||
| <button | ||
| type="button" | ||
| onClick={() => setExpanded(!expanded)} | ||
| className="flex items-center gap-1 text-muted-foreground hover:text-foreground transition-colors" | ||
| > | ||
| {expanded ? ( | ||
| <ChevronDown className="size-3" /> | ||
| ) : ( | ||
| <ChevronRight className="size-3" /> | ||
| )} | ||
| <span>{expanded ? "Hide checks" : "Show checks"}</span> | ||
| </button> | ||
|
|
||
| {expanded && ( | ||
| <div className="mt-1.5 space-y-1 pl-1"> | ||
| {relevantChecks.map((check) => ( | ||
| <CheckItemRow key={check.name} check={check} /> | ||
| ))} | ||
| </div> | ||
| )} | ||
| </div> | ||
| ); |
There was a problem hiding this comment.
Checks filtering contradicts the comment and likely intended UX
Right now:
- You filter out
skipped/cancelledintorelevantChecks. - You return
nullwhenrelevantChecks.length === 0. - You also render the expanded list from
relevantChecks.
So skipped/cancelled checks are never shown, even when expanded, and a list containing only those statuses disappears entirely. That conflicts with the comment “show all when expanded” and can hide real CI jobs.
If you want the expanded list to show every check while still bailing out only when there are literally no checks, a minimal adjustment would be:
- // Filter out skipped/cancelled for display count, but show all when expanded
- const relevantChecks = checks.filter(
- (c) => c.status !== "skipped" && c.status !== "cancelled",
- );
-
- if (relevantChecks.length === 0) return null;
+ if (checks.length === 0) return null;
@@
- {expanded && (
- <div className="mt-1.5 space-y-1 pl-1">
- {relevantChecks.map((check) => (
- <CheckItemRow key={check.name} check={check} />
- ))}
- </div>
- )}
+ {expanded && (
+ <div className="mt-1.5 space-y-1 pl-1">
+ {checks.map((check) => (
+ <CheckItemRow key={check.name} check={check} />
+ ))}
+ </div>
+ )}If the intent is instead to always hide skipped/cancelled, I’d at least update the comment to avoid misleading future readers.
Also, if there’s any chance of duplicate check.name values within a PR, consider using a more stable key (e.g. a backend-provided id) when/if it becomes available.
- Add hover card to workspace tabs showing worktree info and GitHub PR status - Fetch PR data via `gh` CLI with graceful fallback if unavailable - Display worktree name, age, rebase warning, PR number/status/title - Show diff stats (+additions/-deletions) with GitHub-style colors - Add expandable checks list showing individual CI job statuses with links - Show review status (approved/changes requested/pending) - Include "View on GitHub" button with GitHub logo 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Break down the monolithic WorkspaceHoverCard.tsx into separate components following the repo's co-location pattern: - PRStatusBadge: Badge showing PR state (open/draft/merged/closed) - ReviewStatus: Shows review decision status - ChecksSummary: Summary of CI checks (X/Y passing) - ChecksList: Expandable list of individual checks with CheckItemRow - utils.ts: Shared formatRelativeTime utility 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Update AGENTS.md with expanded project structure guidelines - Add shadcn/ui exception section for kebab-case files - Move CheckItemRow to proper nested folder structure - Replace custom formatRelativeTime with date-fns formatDistanceToNow - Delete unnecessary utils.ts wrapper file 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
saddlepaddle
force-pushed
the
silver-glacier-88
branch
from
e47e42c to
3c5b3a0
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/ChecksList.tsx (1)
13-41: Filtering behavior contradicts the comment.The comment states "show all when expanded" but the expanded list only renders
relevantChecks(excluding skipped/cancelled). This was already flagged in a previous review. Either update the comment to match the behavior, or change the expanded list to render allchecksas the comment suggests.
🧹 Nitpick comments (3)
apps/desktop/src/lib/trpc/routers/workspaces/utils/github.ts (2)
95-98: Consider shell-escaping the branch name to prevent command injection.While git branch names have restrictions that limit most attack vectors, directly interpolating
${branch}into a shell command could be problematic if a branch name contains shell metacharacters. Consider using an array-based exec or explicit escaping.- const { stdout } = await execAsync( - `gh pr view ${branch} --json number,title,url,state,isDraft,mergedAt,additions,deletions,reviewDecision,statusCheckRollup`, - { cwd: worktreePath }, - ); + const { stdout } = await execAsync( + `gh pr view "${branch}" --json number,title,url,state,isDraft,mergedAt,additions,deletions,reviewDecision,statusCheckRollup`, + { cwd: worktreePath }, + );Alternatively, use
execFilewith an argument array for safer subprocess invocation:import { execFile } from "node:child_process"; const execFileAsync = promisify(execFile); const { stdout } = await execFileAsync( "gh", ["pr", "view", branch, "--json", "number,title,url,state,isDraft,mergedAt,additions,deletions,reviewDecision,statusCheckRollup"], { cwd: worktreePath }, );
159-174:ACTION_REQUIREDconclusion falls through topending.GitHub's
ACTION_REQUIREDconclusion typically indicates the check needs manual intervention. The current logic treats it aspending, which may be reasonable, but consider whether mapping it tofailurewould better communicate that user action is needed.} else if (rawStatus === "CANCELLED") { status = "cancelled"; + } else if (rawStatus === "ACTION_REQUIRED") { + status = "failure"; // Requires manual intervention } else { status = "pending"; }apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksSummary/ChecksSummary.tsx (1)
12-15: Optional: compute passing/total in a single pass and consider label tweakYou can slightly simplify/optimize by deriving
passingandtotalin one pass, and optionally make the label a bit clearer:- const passing = checks.filter((c) => c.status === "success").length; - const total = checks.filter( - (c) => c.status !== "skipped" && c.status !== "cancelled", - ).length; + const { passing, total } = checks.reduce( + (acc, c) => { + if (c.status !== "skipped" && c.status !== "cancelled") { + acc.total += 1; + if (c.status === "success") acc.passing += 1; + } + return acc; + }, + { passing: 0, total: 0 }, + );If you want a bit more explicit copy, you could also switch the
labelto something like"Checks"→"No checks"whentotal === 0, or"1/3 checks passing"whentotal > 0, but that’s purely stylistic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (19)
AGENTS.md(2 hunks)apps/desktop/package.json(1 hunks)apps/desktop/src/lib/trpc/routers/workspaces/utils/github.ts(1 hunks)apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts(2 hunks)apps/desktop/src/main/lib/db/schemas.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/WorkspaceHoverCard.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/ChecksList.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/components/CheckItemRow/CheckItemRow.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/components/CheckItemRow/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksSummary/ChecksSummary.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksSummary/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/PRStatusBadge/PRStatusBadge.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/PRStatusBadge/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ReviewStatus/ReviewStatus.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ReviewStatus/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItemContextMenu.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (12)
- apps/desktop/package.json
- apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/PRStatusBadge/index.ts
- apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts
- apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ReviewStatus/ReviewStatus.tsx
- apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/index.ts
- apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ReviewStatus/index.ts
- apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/index.ts
- apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/PRStatusBadge/PRStatusBadge.tsx
- apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/components/CheckItemRow/CheckItemRow.tsx
- apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/components/CheckItemRow/index.ts
- apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksSummary/index.ts
- apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItem.tsx
🧰 Additional context used
📓 Path-based instructions (9)
apps/desktop/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
For Electron interprocess communication, ALWAYS use tRPC as defined in
src/lib/trpc
Files:
apps/desktop/src/main/lib/db/schemas.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/github.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/WorkspaceHoverCard.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItemContextMenu.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/ChecksList.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksSummary/ChecksSummary.tsx
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: Please use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary
Files:
apps/desktop/src/main/lib/db/schemas.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/github.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/WorkspaceHoverCard.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItemContextMenu.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/ChecksList.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksSummary/ChecksSummary.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid usinganytype - use explicit types instead for type safety
Use camelCase for variable and function names following existing codebase patterns
Keep diffs minimal with targeted edits only - avoid unnecessary changes when making modifications
Follow existing patterns and match the codebase style when writing new code
Files:
apps/desktop/src/main/lib/db/schemas.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/github.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/WorkspaceHoverCard.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItemContextMenu.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/ChecksList.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksSummary/ChecksSummary.tsx
apps/desktop/src/main/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Node.js modules (fs, path, os, net, etc.) can be used in main process code only
Files:
apps/desktop/src/main/lib/db/schemas.ts
apps/desktop/src/lib/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules in shared code like
src/lib/electron-router-dom.ts- this code runs in both main and renderer processes
Files:
apps/desktop/src/lib/trpc/routers/workspaces/utils/github.ts
**/components/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
**/components/**/*.tsx: Create one folder per component with structure:ComponentName/ComponentName.tsx+index.tsfor barrel export
Co-locate tests next to the component file they test (e.g.,ComponentName.test.tsx)
Co-locate dependencies (utils, hooks, constants, config, stories) next to the file using them
Use nestedcomponents/subdirectory within a parent component only if a sub-component is used 2+ times within that parent; otherwise keep it in the parent'scomponents/folder
One component per file - avoid multi-component files
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/WorkspaceHoverCard.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItemContextMenu.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/ChecksList.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksSummary/ChecksSummary.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules (fs, path, os, net, etc.) in renderer process code - browser environment only
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/WorkspaceHoverCard.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItemContextMenu.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/ChecksList.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksSummary/ChecksSummary.tsx
apps/desktop/src/renderer/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Call IPC methods using
window.ipcRenderer.invoke()with object parameters - TypeScript will infer the exact response type automatically
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/WorkspaceHoverCard.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItemContextMenu.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/ChecksList.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksSummary/ChecksSummary.tsx
AGENTS.md
📄 CodeRabbit inference engine (CLAUDE.md)
Document agent responsibilities and interactions in AGENTS.md
Files:
AGENTS.md
🧠 Learnings (12)
📚 Learning: 2025-11-24T21:33:13.267Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.267Z
Learning: Applies to apps/desktop/**/*.test.{ts,tsx,js,jsx} : Tests should be repeatable
Applied to files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/WorkspaceHoverCard.tsx
📚 Learning: 2025-11-24T21:32:21.725Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/CLAUDE.md:0-0
Timestamp: 2025-11-24T21:32:21.725Z
Learning: Applies to apps/desktop/**/AGENTS.md : Document agent responsibilities, capabilities, and interaction patterns in AGENTS.md
Applied to files:
AGENTS.md
📚 Learning: 2025-11-28T01:03:47.963Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T01:03:47.963Z
Learning: Applies to **/components/**/*.tsx : Co-locate dependencies (utils, hooks, constants, config, stories) next to the file using them
Applied to files:
AGENTS.md
📚 Learning: 2025-11-28T01:03:47.963Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T01:03:47.963Z
Learning: Applies to **/components/**/*.tsx : Create one folder per component with structure: `ComponentName/ComponentName.tsx` + `index.ts` for barrel export
Applied to files:
AGENTS.md
📚 Learning: 2025-11-28T01:03:47.963Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T01:03:47.963Z
Learning: Applies to **/components/**/*.tsx : Co-locate tests next to the component file they test (e.g., `ComponentName.test.tsx`)
Applied to files:
AGENTS.md
📚 Learning: 2025-11-28T01:03:47.963Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T01:03:47.963Z
Learning: Applies to **/components/**/*.tsx : Use nested `components/` subdirectory within a parent component only if a sub-component is used 2+ times within that parent; otherwise keep it in the parent's `components/` folder
Applied to files:
AGENTS.md
📚 Learning: 2025-11-28T01:03:47.963Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T01:03:47.963Z
Learning: Applies to **/components/**/*.tsx : One component per file - avoid multi-component files
Applied to files:
AGENTS.md
📚 Learning: 2025-11-28T01:03:47.963Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T01:03:47.963Z
Learning: Applies to packages/ui/src/**/*.tsx : Use shadcn/ui components and TailwindCSS v4 for all UI component styling in the shared UI package
Applied to files:
AGENTS.md
📚 Learning: 2025-11-28T01:03:47.963Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T01:03:47.963Z
Learning: Applies to packages/db/src/**/*.ts : Define all database schema in `packages/db/src/` using Drizzle ORM
Applied to files:
AGENTS.md
📚 Learning: 2025-11-28T01:03:47.963Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T01:03:47.963Z
Learning: Applies to **/*.{ts,tsx} : Follow existing patterns and match the codebase style when writing new code
Applied to files:
AGENTS.md
📚 Learning: 2025-11-28T01:03:47.963Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T01:03:47.963Z
Learning: Applies to **/*.{ts,tsx} : Use camelCase for variable and function names following existing codebase patterns
Applied to files:
AGENTS.md
📚 Learning: 2025-11-24T21:33:13.267Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.267Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary
Applied to files:
AGENTS.md
🧬 Code graph analysis (5)
apps/desktop/src/lib/trpc/routers/workspaces/utils/github.ts (1)
apps/desktop/src/main/lib/db/schemas.ts (2)
GitHubStatus(25-40)CheckItem(19-23)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/WorkspaceHoverCard.tsx (4)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/PRStatusBadge/PRStatusBadge.tsx (1)
PRStatusBadge(5-27)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksSummary/ChecksSummary.tsx (1)
ChecksSummary(9-43)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ReviewStatus/ReviewStatus.tsx (1)
ReviewStatus(5-18)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/ChecksList.tsx (1)
ChecksList(10-44)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItemContextMenu.tsx (2)
packages/ui/src/components/hover-card.tsx (3)
HoverCard(42-42)HoverCardTrigger(42-42)HoverCardContent(42-42)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/WorkspaceHoverCard.tsx (1)
WorkspaceHoverCardContent(15-115)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/ChecksList.tsx (2)
apps/desktop/src/main/lib/db/schemas.ts (1)
CheckItem(19-23)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksList/components/CheckItemRow/CheckItemRow.tsx (1)
CheckItemRow(8-42)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksSummary/ChecksSummary.tsx (2)
apps/desktop/src/main/lib/db/schemas.ts (1)
CheckItem(19-23)apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksSummary/index.ts (1)
ChecksSummary(1-1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (8)
apps/desktop/src/main/lib/db/schemas.ts (1)
19-36: Well-structured type definitions for GitHub PR status.The
CheckIteminterface andGitHubStatus.prextensions are properly typed with appropriate union types that cover the expected GitHub states. The separation betweenchecksStatus(aggregate) andchecks(individual items) provides good flexibility for the UI layer.AGENTS.md (2)
80-103: Good documentation of modular project structure.The expanded structure example clearly illustrates the co-location patterns for hooks, utils, stores, and providers. This aligns well with the coding guidelines.
141-143: Helpful exception documentation for shadcn/ui components.Documenting that shadcn/ui components use kebab-case single files (instead of the standard folder structure) prevents future confusion and explains the intentional divergence from the normal pattern.
apps/desktop/src/lib/trpc/routers/workspaces/utils/github.ts (1)
48-70: Clean error handling with graceful fallback.The function properly swallows errors when
ghis unavailable or not authenticated, returningnullto allow the UI to gracefully degrade. The separation of concerns betweengetRepoUrlandgetPRForCurrentBranchis well-structured.apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/WorkspaceHoverCard.tsx (2)
18-27: Well-structured data fetching with conditional enabling.The TRPC queries properly use
enabled: !!workspaceIdto prevent fetching when no workspace is selected. The separation between worktree info and GitHub status allows for independent loading states.
57-112: Comprehensive state handling in PR section.The component properly handles all UI states: loading (
isLoadingGithub), PR exists (pr), no PR but GitHub status available (githubStatuswithoutpr), and no GitHub status at all. The checks and review status are appropriately shown only for open PRs.apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceItemContextMenu.tsx (1)
39-55: Clean integration of HoverCard with existing ContextMenu.The composition properly nests
HoverCardaroundContextMenu, usingasChildto avoid extra DOM wrappers. The delay values (400ms open, 100ms close) provide good UX by preventing accidental triggers while still feeling responsive.apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/WorkspaceHoverCard/components/ChecksSummary/ChecksSummary.tsx (1)
1-43: ChecksSummary component looks correct and aligns with guidelines
- Props are well-typed (
CheckItem[]and narrowstatusunion), noany.status === "none"guard cleanly prevents invalid config indexing and avoids rendering noise.- Count logic (passing vs total excluding skipped/cancelled) is reasonable, and icon/color mappings match the status semantics.
- Renderer file correctly uses a TS alias import and does not pull in Node APIs.
No blocking issues from this file.
- Validate gh CLI output with Zod schemas instead of type assertions - Derive TypeScript types from Zod schemas using z.infer - Add 30-second TTL cache to reduce GitHub API calls 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/desktop/src/lib/trpc/routers/workspaces/utils/github.ts (1)
9-28: Consider usingz.string()instead of strict enums for future-proofing.GitHub may introduce new check states or conclusions over time (e.g.,
EXPECTED,STALE,STARTUP_FAILURE). If theghCLI returns an unexpected value,z.parse()will throw and the entire PR fetch fails. While the parent catch returnsnull, this could unexpectedly hide valid PR data.You could use
z.string()and handle unknown values inparseChecks/computeChecksStatus, or use.catch()to provide defaults.const GHCheckContextSchema = z.object({ __typename: z.string(), name: z.string().optional(), context: z.string().optional(), - state: z.enum(["SUCCESS", "FAILURE", "PENDING", "ERROR"]).optional(), + state: z.string().optional(), conclusion: z - .enum([ - "SUCCESS", - "FAILURE", - "CANCELLED", - "SKIPPED", - "TIMED_OUT", - "ACTION_REQUIRED", - "NEUTRAL", - ]) + .string() .nullable() .optional(), detailsUrl: z.string().optional(), targetUrl: z.string().optional(), });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src/lib/trpc/routers/workspaces/utils/github.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
apps/desktop/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
For Electron interprocess communication, ALWAYS use tRPC as defined in
src/lib/trpc
Files:
apps/desktop/src/lib/trpc/routers/workspaces/utils/github.ts
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: Please use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary
Files:
apps/desktop/src/lib/trpc/routers/workspaces/utils/github.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid usinganytype - use explicit types instead for type safety
Use camelCase for variable and function names following existing codebase patterns
Keep diffs minimal with targeted edits only - avoid unnecessary changes when making modifications
Follow existing patterns and match the codebase style when writing new code
Files:
apps/desktop/src/lib/trpc/routers/workspaces/utils/github.ts
apps/desktop/src/lib/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules in shared code like
src/lib/electron-router-dom.ts- this code runs in both main and renderer processes
Files:
apps/desktop/src/lib/trpc/routers/workspaces/utils/github.ts
🧬 Code graph analysis (1)
apps/desktop/src/lib/trpc/routers/workspaces/utils/github.ts (1)
apps/desktop/src/main/lib/db/schemas.ts (2)
GitHubStatus(25-40)CheckItem(19-23)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (7)
apps/desktop/src/lib/trpc/routers/workspaces/utils/github.ts (7)
1-7: LGTM!Imports are appropriate for main-process code, and the alias usage follows tsconfig patterns.
55-57: LGTM!Simple and effective caching strategy. The 30-second TTL is reasonable for hover card data. For a desktop app with a limited number of worktrees, the unbounded cache size is acceptable.
64-97: LGTM!The function correctly implements caching with TTL check, handles the fetch flow appropriately, and the error handling aligns with the documented graceful fallback behavior.
99-109: LGTM!The command string is static with no user input interpolation, and the error handling provides graceful fallback.
155-171: LGTM!Both mapping functions correctly handle all expected states with clear logic.
173-205: LGTM!Correctly handles both CheckRun and StatusContext types from GitHub's API with appropriate status mapping. The fallback to "pending" for unrecognized states is a reasonable default.
207-235: LGTM!The aggregation logic correctly prioritizes failure over pending, and treats skipped/cancelled checks as non-blocking, which aligns with GitHub's semantics.
| const { stdout } = await execAsync( | ||
| `gh pr view ${branch} --json number,title,url,state,isDraft,mergedAt,additions,deletions,reviewDecision,statusCheckRollup`, | ||
| { cwd: worktreePath }, | ||
| ); |
There was a problem hiding this comment.
Command injection risk: escape or avoid shell interpolation of branch name.
The branch variable is directly interpolated into a shell command. Git branch names can contain shell metacharacters ($, backticks) that would be interpreted by the shell. A malicious branch name like test$(malicious_command) could execute arbitrary code.
Use execFile to avoid shell interpretation, or properly quote the argument:
+import { execFile } from "node:child_process";
+const execFileAsync = promisify(execFile);
async function getPRForCurrentBranch(
worktreePath: string,
): Promise<GitHubStatus["pr"]> {
try {
const { stdout: branchName } = await execAsync(
"git rev-parse --abbrev-ref HEAD",
{ cwd: worktreePath },
);
const branch = branchName.trim();
- const { stdout } = await execAsync(
- `gh pr view ${branch} --json number,title,url,state,isDraft,mergedAt,additions,deletions,reviewDecision,statusCheckRollup`,
- { cwd: worktreePath },
- );
+ const { stdout } = await execFileAsync(
+ "gh",
+ [
+ "pr",
+ "view",
+ branch,
+ "--json",
+ "number,title,url,state,isDraft,mergedAt,additions,deletions,reviewDecision,statusCheckRollup",
+ ],
+ { cwd: worktreePath },
+ );Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/desktop/src/lib/trpc/routers/workspaces/utils/github.ts around lines
122-125 the code interpolates the branch name directly into a shell command
which allows command injection; change to invoke the GitHub CLI without shell
interpolation by using an execFile-style call (or the execAsync equivalent that
accepts an args array) and pass "pr", "view", branch, and the --json string as
separate arguments, keeping the cwd option; this ensures the branch name is not
interpreted by a shell and prevents execution of embedded metacharacters.
- reviewDecision can be empty string (not just null)
- statusCheckRollup is a flat array, not { contexts: [...] }
- Add missing fields: status, startedAt, completedAt, workflowName
- conclusion can be empty string when check is in progress
- Treat empty string as pending in checksStatus computation
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Security fix: - Replace exec() with execFile() to prevent shell injection via branch names - Pass gh CLI arguments as array instead of interpolated string Structure: - Move github.ts to github/github.ts with index.ts barrel export - Extract Zod schemas to co-located types.ts 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 participant
Summary
ghCLI with graceful fallback when unavailableTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Schema
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.