feat(desktop): unify tasks/PRs/issues view with project filter#4220
Merged
Conversation
Adds Type tabs (All/Tasks/PRs/Issues) and a Project filter to the Tasks page, so PRs and GitHub issues for a project surface alongside tasks instead of needing a separate sidebar entry. Row clicks open externally and "Add to workspace" seeds the new-workspace draft to launch the existing modal pre-filled with the linked PR or issue.
Contributor
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds type/project filtering and multi-panel views for tasks/PRs/issues, implements infinite-scrolling PR and issue lists with detail pages, refactors GitHub search procedures to return paginated shapes (CLI + Octokit), introduces selective IndexedDB-backed React Query persistence, adds filter UI components and tests, and bumps React Query-related package versions. ChangesTasks Dashboard Multi-View with Pagination
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
Splits the Tasks "All" view into three resizable, collapsible panes
(Tasks · Pull requests · Issues), each scrolling independently and
remembering its layout via autoSaveId. Per-pane minimize button and a
vertical-text rail click target for re-expanding.
Backs PR/issue lists with `gh api search/issues` + page cursor so the
top-right indicator shows real totals ("30 of 412 pull requests") and
new pages stream in via IntersectionObserver as the user scrolls.
Toolbar uses container queries for breakpoints — labels collapse to
icon-only as the toolbar narrows, so it no longer overflows on narrow
windows. Status + assignee filters are now scoped to the dedicated
Tasks tab.
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
Adds /tasks/pr/$prNumber and /tasks/issue/$issueNumber routes that fetch the full body via existing host-service procedures (pullRequests.getContent, issues.getContent) and render it with MarkdownRenderer. Click on a PR or issue row in the All / PRs / Issues view now opens the preview instead of the GitHub URL; the external- link icon in the row still opens GitHub directly. The preview header carries an "Add to workspace" button so users can seed a new-workspace draft from the detail page, mirroring the row action.
Phase 1: tunes React Query staleness on the four tasks-page queries (PR list, issue list, PR detail, issue detail) — `staleTime: 30s`, `gcTime: 10m`, `placeholderData: keepPreviousData` so toggling a filter or navigating away and back keeps the prior list rendered while new data is fetched in the background. Phase 2: wraps the renderer's QueryClient with PersistQueryClient- Provider, persisting only tasks-page queries (whitelisted by queryKey prefix) to IndexedDB via idb-keyval. First paint after relaunch renders from the rehydrated cache; the persister auto-writes successful queries on a default throttle. Buster + 24h maxAge guard against stale shapes.
Greptile SummaryThis PR unifies the Tasks page by adding Type tabs (All / Tasks / PRs / Issues) and a Project filter, letting users browse GitHub PRs and issues alongside Linear tasks without a separate navigation entry. New
Confidence Score: 3/5Safe to merge for the new features; the back-navigation bug will be immediately noticeable when using the PR/Issues tabs. The PR/issue navigation flow drops the PullRequestsContent.tsx and GitHubIssuesContent.tsx — both
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/PullRequestsContent/PullRequestsContent.tsx | New component for paginated PR list; handleOpenPreview loses type and other URL params when navigating to the detail page, breaking the Back button. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/GitHubIssuesContent/GitHubIssuesContent.tsx | New component for paginated issues list; same handleOpenPreview navigation bug as PullRequestsContent — type param dropped on detail route entry. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/TasksView.tsx | Orchestrates type/project routing; showLinearCTA correctly gates on typeTab === "tasks" only; task navigation uses buildSearch({}) correctly. |
| packages/host-service/src/trpc/router/workspace-creation/procedures/search-pull-requests.ts | Migrated from gh pr list to gh api search/issues with pagination; total_count/hasNextPage logic relies on raw API count before optional client-side filter. |
| packages/host-service/src/trpc/router/workspace-creation/procedures/search-github-issues.ts | Migrated from gh issue list to gh api search/issues; same totalCount/filter discrepancy noted in search-pull-requests.ts. |
| apps/desktop/src/renderer/providers/ElectronTRPCProvider/ElectronTRPCProvider.tsx | Adds IndexedDB-backed query persistence via idb-keyval; whitelist correctly scoped to tasks/PR/issue keys; PERSIST_BUSTER requires manual bump on schema changes. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/pr/$prNumber/page.tsx | New PR detail page; backSearch correctly reconstructs all URL params from TasksLayoutRoute.useSearch(), but depends on navigate callers passing them through (which they currently don't for type). |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/issue/$issueNumber/page.tsx | New issue detail page; same observation as the PR detail page regarding backSearch depending on callers forwarding all params. |
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/PullRequestsContent/PullRequestsContent.tsx:155-163
**Back navigation loses `type` filter**
`handleOpenPreview` only forwards `project` in the `search` object when navigating to the detail page. The `type` param (e.g. `"prs"`) — and any other URL params like `tab`, `assignee`, or `search` — are absent from the destination URL. The detail page's `backSearch` is built from `TasksLayoutRoute.useSearch()`, so `search.type` is `undefined` on that page; clicking Back lands the user on `/tasks?project=…` with the Type tab reset to "All" instead of "PRs".
The same issue affects `GitHubIssuesContent` where `handleOpenPreview` also only passes `project`.
### Issue 2 of 3
apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/GitHubIssuesContent/GitHubIssuesContent.tsx:120-128
**Back navigation loses `type` filter (same pattern as `PullRequestsContent`)**
`handleOpenPreview` passes only `{ project: projectFilter }` to the issue detail route. The `type` param (`"issues"`) is stripped from the URL, so `TasksLayoutRoute.useSearch().type` is `undefined` on the detail page and the Back button sends the user to `/tasks?project=…` with Type tab defaulting to "All".
### Issue 3 of 3
packages/host-service/src/trpc/router/workspace-creation/procedures/search-pull-requests.ts:87-102
**`totalCount` may include PR items filtered out client-side**
After switching to `gh api search/issues`, `hasNextPage = page * perPage < parsed.total_count` uses the raw GitHub API `total_count`. The subsequent `.filter((item) => !!item.pull_request)` can silently drop items if GitHub ever leaks non-PR results, causing the displayed `N of M` count and `hasNextPage` sentinel to diverge from the items actually rendered. The same pattern exists in `search-github-issues.ts`. In practice the `is:pr` / `is:issue` qualifiers should make this a no-op filter, but if it does fire the UI will show an infinite load spinner at the bottom of an already-complete list.
Reviews (1): Last reviewed commit: "feat(desktop): cache tasks/PR/issue list..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+155
to
+163
| </span> | ||
| </div> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| const isInitialLoad = isFetching && pullRequests.length === 0; | ||
| const countLabel = isInitialLoad | ||
| ? "Loading…" |
There was a problem hiding this comment.
type filter
handleOpenPreview only forwards project in the search object when navigating to the detail page. The type param (e.g. "prs") — and any other URL params like tab, assignee, or search — are absent from the destination URL. The detail page's backSearch is built from TasksLayoutRoute.useSearch(), so search.type is undefined on that page; clicking Back lands the user on /tasks?project=… with the Type tab reset to "All" instead of "PRs".
The same issue affects GitHubIssuesContent where handleOpenPreview also only passes project.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/PullRequestsContent/PullRequestsContent.tsx
Line: 155-163
Comment:
**Back navigation loses `type` filter**
`handleOpenPreview` only forwards `project` in the `search` object when navigating to the detail page. The `type` param (e.g. `"prs"`) — and any other URL params like `tab`, `assignee`, or `search` — are absent from the destination URL. The detail page's `backSearch` is built from `TasksLayoutRoute.useSearch()`, so `search.type` is `undefined` on that page; clicking Back lands the user on `/tasks?project=…` with the Type tab reset to "All" instead of "PRs".
The same issue affects `GitHubIssuesContent` where `handleOpenPreview` also only passes `project`.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+120
to
+128
| resetDraft(); | ||
| updateDraft({ | ||
| selectedProjectId: projectFilter, | ||
| linkedIssues: [linkedIssue], | ||
| }); | ||
| openModal(projectFilter); | ||
| }; | ||
|
|
||
| const handleOpenUrl = (url: string) => { |
There was a problem hiding this comment.
type filter (same pattern as PullRequestsContent)
handleOpenPreview passes only { project: projectFilter } to the issue detail route. The type param ("issues") is stripped from the URL, so TasksLayoutRoute.useSearch().type is undefined on the detail page and the Back button sends the user to /tasks?project=… with Type tab defaulting to "All".
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/GitHubIssuesContent/GitHubIssuesContent.tsx
Line: 120-128
Comment:
**Back navigation loses `type` filter (same pattern as `PullRequestsContent`)**
`handleOpenPreview` passes only `{ project: projectFilter }` to the issue detail route. The `type` param (`"issues"`) is stripped from the URL, so `TasksLayoutRoute.useSearch().type` is `undefined` on the detail page and the Back button sends the user to `/tasks?project=…` with Type tab defaulting to "All".
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+87
to
+102
| .optional(), | ||
| }); | ||
|
|
||
| const searchIssuesResponseSchema = z.object({ | ||
| total_count: z.number(), | ||
| items: z.array(searchIssuesItemSchema), | ||
| }); | ||
|
|
||
| async function ghApiSearchPullRequests( | ||
| execGh: ExecGh, | ||
| repo: ResolvedGithubRepo, | ||
| query: string, | ||
| includeClosed: boolean, | ||
| limit: number, | ||
| ): Promise<PullRequestResult[]> { | ||
| page: number, | ||
| perPage: number, | ||
| ): Promise<{ |
There was a problem hiding this comment.
totalCount may include PR items filtered out client-side
After switching to gh api search/issues, hasNextPage = page * perPage < parsed.total_count uses the raw GitHub API total_count. The subsequent .filter((item) => !!item.pull_request) can silently drop items if GitHub ever leaks non-PR results, causing the displayed N of M count and hasNextPage sentinel to diverge from the items actually rendered. The same pattern exists in search-github-issues.ts. In practice the is:pr / is:issue qualifiers should make this a no-op filter, but if it does fire the UI will show an infinite load spinner at the bottom of an already-complete list.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/host-service/src/trpc/router/workspace-creation/procedures/search-pull-requests.ts
Line: 87-102
Comment:
**`totalCount` may include PR items filtered out client-side**
After switching to `gh api search/issues`, `hasNextPage = page * perPage < parsed.total_count` uses the raw GitHub API `total_count`. The subsequent `.filter((item) => !!item.pull_request)` can silently drop items if GitHub ever leaks non-PR results, causing the displayed `N of M` count and `hasNextPage` sentinel to diverge from the items actually rendered. The same pattern exists in `search-github-issues.ts`. In practice the `is:pr` / `is:issue` qualifiers should make this a no-op filter, but if it does fire the UI will show an infinite load spinner at the bottom of an already-complete list.
How can I resolve this? If you propose a fix, please make it concise.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/host-service/src/trpc/router/workspace-creation/procedures/search-github-issues.ts (1)
208-216:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winState casing is inconsistent between
ghand Octokit fallback paths.The
ghpaths normalizestateto lowercase (issue.state.toLowerCase()at lines 59 and 117), but the Octokit fallback returnsissue.stateanditem.stateraw at lines 214 and 240. In practice Octokit returns lowercase already, butgh issue viewreturns"OPEN"/"CLOSED". Consumers comparingstateagainst"open"will see different behavior depending on which backend served the request. Apply the same.toLowerCase()(or a sharednormalizeIssueStatehelper, mirroringnormalizePullRequestState) on the Octokit path for consistency.🔧 Proposed fix
- return { - issues: [ - { - issueNumber: issue.number, - title: issue.title, - url: issue.html_url, - state: issue.state, - authorLogin: issue.user?.login ?? null, - }, - ], + return { + issues: [ + { + issueNumber: issue.number, + title: issue.title, + url: issue.html_url, + state: issue.state.toLowerCase(), + authorLogin: issue.user?.login ?? null, + }, + ],- const issues = data.items - .filter((item) => !item.pull_request) - .map((item) => ({ - issueNumber: item.number, - title: item.title, - url: item.html_url, - state: item.state, - authorLogin: item.user?.login ?? null, - })); + const issues = data.items + .filter((item) => !item.pull_request) + .map((item) => ({ + issueNumber: item.number, + title: item.title, + url: item.html_url, + state: item.state.toLowerCase(), + authorLogin: item.user?.login ?? null, + }));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/host-service/src/trpc/router/workspace-creation/procedures/search-github-issues.ts` around lines 208 - 216, The Octokit fallback branch returns raw issue/item.state which can be uppercase from `gh` whereas other paths call `.toLowerCase()`; update the Octokit path in search-github-issues (the branch that builds the returned `issues` array with `issue` and `item`) to normalize state consistently—either call `.toLowerCase()` on `issue.state`/`item.state` or invoke a shared `normalizeIssueState` (mirror `normalizePullRequestState`) so the returned `state` is always lowercase.apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/TasksView.tsx (1)
192-212:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUsers get trapped on the "Tasks" tab when Linear isn't connected.
When
showLinearCTAistrue(typeTab === "tasks"+ no Linear), theTasksTopBar— which contains the type tabs — is completely hidden. The user has no way to switch to "PRs" or "Issues" tabs without manually editing the URL. Since navigation usesreplace: true, the browser back-stack doesn't help either.The simplest fix is to render
TasksTopBarunconditionally and letLinearCTAappear only in the content area:🐛 Proposed fix
-{!showLinearCTA && ( - <TasksTopBar - currentTab={currentTab} - onTabChange={handleTabChange} - searchQuery={searchQuery} - onSearchChange={handleSearchChange} - assigneeFilter={assigneeFilter} - onAssigneeFilterChange={handleAssigneeFilterChange} - selectedTasks={selectedTasks} - onClearSelection={handleClearSelection} - viewMode={viewMode} - onViewModeChange={setViewMode} - typeTab={typeTab} - onTypeTabChange={handleTypeTabChange} - projectFilter={projectFilter} - onProjectFilterChange={handleProjectFilterChange} - /> -)} +<TasksTopBar + currentTab={currentTab} + onTabChange={handleTabChange} + searchQuery={searchQuery} + onSearchChange={handleSearchChange} + assigneeFilter={assigneeFilter} + onAssigneeFilterChange={handleAssigneeFilterChange} + selectedTasks={selectedTasks} + onClearSelection={handleClearSelection} + viewMode={viewMode} + onViewModeChange={setViewMode} + typeTab={typeTab} + onTypeTabChange={handleTypeTabChange} + projectFilter={projectFilter} + onProjectFilterChange={handleProjectFilterChange} +/>Note: task-specific controls in
TasksTopBar(showTaskOnlyControls = typeTab === "tasks") will still render when the LinearCTA is shown, but that's acceptable since they're already gated ontypeTab. If a fully clean state is needed,TasksTopBarcan additionally accept adisabledorshowOnlyNavigationprop.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/TasksView.tsx` around lines 192 - 212, When showLinearCTA is true the current code hides TasksTopBar (which contains the type tabs), trapping users on the Tasks tab; change rendering so TasksTopBar is rendered unconditionally (keep passing currentTab, handleTabChange, typeTab, etc.), and only conditionally render LinearCTA in the content area instead of replacing the whole top bar. Leave task-specific controls inside TasksTopBar gated by showTaskOnlyControls = typeTab === "tasks" (or add a disabled/showOnlyNavigation prop later if needed) so navigation remains available while LinearCTA is shown.
🧹 Nitpick comments (3)
packages/host-service/src/trpc/router/workspace-creation/procedures/search-pull-requests.ts (1)
145-178: 💤 Low valueDirect-lookup hardcodes
page: 1even wheninput.pagediffers.For
isDirectLookup, the response returnspage: 1regardless ofinput.page. This is harmless becausehasNextPage: falseandpullRequests.length === 1mean the renderer won't paginate, but it's inconsistent with the search path which echoes backinput.page. Consider returningpage: input.page ?? 1for symmetry, or simply documenting the behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/host-service/src/trpc/router/workspace-creation/procedures/search-pull-requests.ts` around lines 145 - 178, The direct-lookup branch in the searchPullRequests procedure returns page: 1 unconditionally; change it to echo the requested page by returning page: input.page ?? 1 (or the local page variable) so the response matches the non-direct-search path and remains consistent with the input.page value when normalized.isDirectLookup is true.apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/pr/$prNumber/page.tsx (1)
125-127: 💤 Low valueConsider extracting branch summary logic for clarity.
The ternary expression correctly handles cross-repository PRs, but extracting it to a named helper would improve readability:
function formatBranchSummary(data: typeof data): string | null { if (!data.branch) return null; const head = data.headRepositoryOwner && data.isCrossRepository ? `${data.headRepositoryOwner}:${data.branch}` : data.branch; return `${head} → ${data.baseBranch}`; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/pr/`$prNumber/page.tsx around lines 125 - 127, Extract the inline ternary that builds branchSummary into a named helper to improve readability: create a function (e.g., formatBranchSummary) that accepts the existing data object, returns null when data.branch is falsy, computes head as data.headRepositoryOwner && data.isCrossRepository ? `${data.headRepositoryOwner}:${data.branch}` : data.branch, and returns `${head} → ${data.baseBranch}`; then replace the current branchSummary assignment with a call to formatBranchSummary(data).apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/TasksView.tsx (1)
50-76: ⚡ Quick winImprove compile-time type safety by explicitly typing
buildSearchreturn value.The
/tasksroute defines aTasksSearchschema inlayout.tsxwithvalidateSearchfor runtime validation. However,buildSearchreturnsRecord<string, string>, which bypasses TypeScript compile-time checking. Although runtime validation catches issues, explicitly typing the return to matchTasksSearchenables TypeScript to catch type mismatches at thenavigate()call site.♻️ Proposed refactor
-const search: Record<string, string> = {}; +const search: { + tab?: "all" | "active" | "backlog"; + assignee?: string; + search?: string; + type?: "all" | "tasks" | "prs" | "issues"; + project?: string; +} = {};Alternatively, import
TasksSearchfromlayout.tsxand use it directly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/TasksView.tsx` around lines 50 - 76, Update buildSearch to return a typed object matching the route schema instead of Record<string,string>: import the TasksSearch type from layout.tsx and change the useCallback signature and the local search variable to use Partial<TasksSearch> (or TasksSearch if all keys must be present). Keep the same logic for conditionally adding keys but ensure each assigned property matches the TasksSearch property types so TypeScript can validate calls to navigate() (referencing buildSearch and the local search variable).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/desktop/package.json`:
- Around line 106-109: Update the package dependency entry for
"@tanstack/react-query" in apps/desktop/package.json from "^5.90.19" to
"^5.100.9" so it matches the peer requirement of
"@tanstack/react-query-persist-client": "^5.100.9"; after changing the version
string for the "@tanstack/react-query" field, regenerate your lockfile
(npm/yarn/pnpm install) so the lockfile reflects the bumped version.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/GitHubIssuesContent/GitHubIssuesContent.tsx`:
- Around line 205-209: The repo mismatch error message rendered in the
conditional block that checks repoMismatch (the JSX fragment containing "Issue
URL must match {repoMismatch}") isn't selectable; update that div to include the
required CSS utility classes by adding "select-text cursor-text" to its
className so the displayed {repoMismatch} value can be copied by users; locate
the conditional rendering that uses repoMismatch in GitHubIssuesContent (the JSX
block with {repoMismatch && (...)} ) and append those classes to the existing
"px-4 py-3 text-sm text-muted-foreground" class string.
- Around line 128-130: The renderer's handleOpenUrl currently uses window.open
which creates an Electron window; replace it with a tRPC call to a new
main-process procedure that calls Electron's shell.openExternal. Add a procedure
(e.g., shell.openExternal) to the main process router that accepts a URL string
and calls require('electron').shell.openExternal(url) (with error handling),
expose that procedure on the router used by the renderer, then update
handleOpenUrl in GitHubIssuesContent.tsx to call the tRPC procedure (e.g.,
trpc.shell.openExternal.mutate or .query) instead of window.open and
handle/rethrow errors appropriately. Ensure the procedure name matches what the
renderer client imports and remove the window.open usage.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/PullRequestsContent/PullRequestsContent.tsx`:
- Around line 30-36: Extract the duplicated state logic from normalizeState (in
PullRequestsContent.tsx) and resolveState (in pr/$prNumber/page.tsx) into a
single shared utility function (e.g., normalizePRState) exported from a new
module (suggested name: renderer/utils/pr-state.ts); the new function should
accept (state: string, isDraft: boolean): PRState, re-use the exact logic (draft
-> "draft", merged -> "merged", closed -> "closed", else "open"), and both files
should import and call normalizePRState instead of their local implementations,
ensuring you also import the PRState type from
renderer/screens/main/components/PRIcon/PRIcon.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/TasksTopBar.tsx`:
- Around line 218-224: The placeholder logic in TasksTopBar uses a ternary on
typeTab and falls back to "Search tasks..." which is misleading for the "all"
tab; update the placeholder expression in the TasksTopBar component (the
placeholder prop that references typeTab) to explicitly handle typeTab === "all"
(e.g., "Search all items..." or "Search tasks, PRs, and issues...") instead of
falling through to the tasks-only text so that the All tab shows an accurate
placeholder.
---
Outside diff comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/TasksView.tsx`:
- Around line 192-212: When showLinearCTA is true the current code hides
TasksTopBar (which contains the type tabs), trapping users on the Tasks tab;
change rendering so TasksTopBar is rendered unconditionally (keep passing
currentTab, handleTabChange, typeTab, etc.), and only conditionally render
LinearCTA in the content area instead of replacing the whole top bar. Leave
task-specific controls inside TasksTopBar gated by showTaskOnlyControls =
typeTab === "tasks" (or add a disabled/showOnlyNavigation prop later if needed)
so navigation remains available while LinearCTA is shown.
In
`@packages/host-service/src/trpc/router/workspace-creation/procedures/search-github-issues.ts`:
- Around line 208-216: The Octokit fallback branch returns raw issue/item.state
which can be uppercase from `gh` whereas other paths call `.toLowerCase()`;
update the Octokit path in search-github-issues (the branch that builds the
returned `issues` array with `issue` and `item`) to normalize state
consistently—either call `.toLowerCase()` on `issue.state`/`item.state` or
invoke a shared `normalizeIssueState` (mirror `normalizePullRequestState`) so
the returned `state` is always lowercase.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/TasksView.tsx`:
- Around line 50-76: Update buildSearch to return a typed object matching the
route schema instead of Record<string,string>: import the TasksSearch type from
layout.tsx and change the useCallback signature and the local search variable to
use Partial<TasksSearch> (or TasksSearch if all keys must be present). Keep the
same logic for conditionally adding keys but ensure each assigned property
matches the TasksSearch property types so TypeScript can validate calls to
navigate() (referencing buildSearch and the local search variable).
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/pr/`$prNumber/page.tsx:
- Around line 125-127: Extract the inline ternary that builds branchSummary into
a named helper to improve readability: create a function (e.g.,
formatBranchSummary) that accepts the existing data object, returns null when
data.branch is falsy, computes head as data.headRepositoryOwner &&
data.isCrossRepository ? `${data.headRepositoryOwner}:${data.branch}` :
data.branch, and returns `${head} → ${data.baseBranch}`; then replace the
current branchSummary assignment with a call to formatBranchSummary(data).
In
`@packages/host-service/src/trpc/router/workspace-creation/procedures/search-pull-requests.ts`:
- Around line 145-178: The direct-lookup branch in the searchPullRequests
procedure returns page: 1 unconditionally; change it to echo the requested page
by returning page: input.page ?? 1 (or the local page variable) so the response
matches the non-direct-search path and remains consistent with the input.page
value when normalized.isDirectLookup is true.
🪄 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: 14540d8f-5c8b-43eb-a51a-e59c6a4a8c8e
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (28)
apps/desktop/package.jsonapps/desktop/src/renderer/providers/ElectronTRPCProvider/ElectronTRPCProvider.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/TasksView.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/AllModePanels/AllModePanels.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/AllModePanels/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/CollapsedColumnRail/CollapsedColumnRail.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/CollapsedColumnRail/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/GitHubIssuesContent/GitHubIssuesContent.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/GitHubIssuesContent/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/PullRequestsContent/PullRequestsContent.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/PullRequestsContent/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksColumn/TasksColumn.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksColumn/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/TasksTopBar.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/components/AssigneeFilter/AssigneeFilter.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/components/ProjectFilter/ProjectFilter.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/components/ProjectFilter/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/components/StatusFilter/StatusFilter.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/components/StatusFilter/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/issue/$issueNumber/page.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/layout.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/page.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/pr/$prNumber/page.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/stores/tasks-filter-state.tspackages/host-service/src/trpc/router/workspace-creation/procedures/search-github-issues.tspackages/host-service/src/trpc/router/workspace-creation/procedures/search-pull-requests.tspackages/host-service/src/trpc/router/workspace-creation/schemas.tspackages/host-service/test/integration/workspace-creation-github.integration.test.ts
Comment on lines
+128
to
+130
| const handleOpenUrl = (url: string) => { | ||
| window.open(url, "_blank", "noopener,noreferrer"); | ||
| }; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for existing openExternal / setWindowOpenHandler patterns in the codebase
rg -n --type=ts "openExternal\|setWindowOpenHandler\|open.*external\|openUrl" \
-g "!node_modules" -g "!dist" -A 2 -B 2Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
cat -n apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/GitHubIssuesContent/GitHubIssuesContent.tsx | sed -n '120,140p'Repository: superset-sh/superset
Length of output: 683
🏁 Script executed:
find apps/desktop -name "main.ts" -o -name "main.js" -o -name "preload.ts" -o -name "preload.js" | head -20Repository: superset-sh/superset
Length of output: 102
🏁 Script executed:
rg -n "setWindowOpenHandler\|BrowserWindow\|preload" apps/desktop --type=ts --type=js -l | head -20Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
cat -n apps/desktop/src/main/windows/main.ts | head -100Repository: superset-sh/superset
Length of output: 4252
🏁 Script executed:
rg -n "trpc\|router\|procedure" apps/desktop/src/main --type=ts -A 3 | head -50Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
find apps/desktop/src -name "*trpc*" -o -name "*router*" | grep -E "main|preload"Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
cat -n apps/desktop/src/main/windows/main.ts | sed -n '100,250p'Repository: superset-sh/superset
Length of output: 5370
🏁 Script executed:
find apps/desktop/src -path "*/trpc/*" -name "*.ts" | head -20Repository: superset-sh/superset
Length of output: 1193
🏁 Script executed:
cat -n apps/desktop/src/main/windows/main.ts | sed -n '250,350p'Repository: superset-sh/superset
Length of output: 2741
🏁 Script executed:
cat -n apps/desktop/src/main/preload/index.tsRepository: superset-sh/superset
Length of output: 135
🏁 Script executed:
find apps/desktop/src/main -name "preload*" -type fRepository: superset-sh/superset
Length of output: 46
🏁 Script executed:
rg -n "window.open\|setWindowOpenHandler" apps/desktop/src/main --type=tsRepository: superset-sh/superset
Length of output: 46
🏁 Script executed:
tree -L 3 apps/desktop/src/main/ 2>/dev/null | head -50Repository: superset-sh/superset
Length of output: 1413
🏁 Script executed:
find apps/desktop -name "*preload*" -type f 2>/dev/nullRepository: superset-sh/superset
Length of output: 46
🏁 Script executed:
ls -la apps/desktop/src/main/Repository: superset-sh/superset
Length of output: 678
🏁 Script executed:
ls -la apps/desktop/src/Repository: superset-sh/superset
Length of output: 567
🏁 Script executed:
find apps/desktop/src -name "*preload*" 2>/dev/nullRepository: superset-sh/superset
Length of output: 89
🏁 Script executed:
ls -la apps/desktop/src/preload/Repository: superset-sh/superset
Length of output: 233
🏁 Script executed:
cat -n apps/desktop/src/preload/index.tsRepository: superset-sh/superset
Length of output: 2952
🏁 Script executed:
rg -n "shell\.openExternal\|open.*url\|handleOpenUrl" apps/desktop/src --type=ts -B 2 -A 2 | head -40Repository: superset-sh/superset
Length of output: 46
Use tRPC to open external URLs instead of window.open.
In Electron, window.open(url, "_blank") opens a new Electron window, not the system browser, unless the main process explicitly intercepts it with setWindowOpenHandler calling shell.openExternal. No such handler exists in this codebase, and shell is not exposed in the preload or via tRPC.
Add a tRPC procedure (e.g., shell.openExternal) to the main process router and call it from the renderer instead of using window.open.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/GitHubIssuesContent/GitHubIssuesContent.tsx`
around lines 128 - 130, The renderer's handleOpenUrl currently uses window.open
which creates an Electron window; replace it with a tRPC call to a new
main-process procedure that calls Electron's shell.openExternal. Add a procedure
(e.g., shell.openExternal) to the main process router that accepts a URL string
and calls require('electron').shell.openExternal(url) (with error handling),
expose that procedure on the router used by the renderer, then update
handleOpenUrl in GitHubIssuesContent.tsx to call the tRPC procedure (e.g.,
trpc.shell.openExternal.mutate or .query) instead of window.open and
handle/rethrow errors appropriately. Ensure the procedure name matches what the
renderer client imports and remove the window.open usage.
Removes the "All" type tab from the tasks page along with the resizable panel layout (AllModePanels, CollapsedColumnRail, TasksColumn). Default type tab is now "tasks"; tab bar is Tasks / PRs / Issues. Cleans up the URL search-param validation, store default, and TasksView render. SafeImage now allows data: and http(s):// sources so images embedded in PR / issue / task markdown render. file://, absolute paths, and UNC paths are still blocked. Adds a refresh icon button to the PR and issue section headers — calls the infinite query's refetch and spins while fetching.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/desktop/src/renderer/components/MarkdownRenderer/components/SafeImage/SafeImage.tsx (1)
61-64: ⚡ Quick winHarden external image loads with
referrerPolicyNow that remote URLs are allowed, add
referrerPolicy="no-referrer"to reduce metadata leakage to third-party hosts.Suggested fix
<img src={src} alt={alt} + referrerPolicy="no-referrer" className={className ?? "max-w-full h-auto rounded-md my-4"} />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/components/MarkdownRenderer/components/SafeImage/SafeImage.tsx` around lines 61 - 64, The <img> tag in SafeImage (component SafeImage) currently renders external src without a referrer policy; update the JSX for the img element (the <img src={src} alt={alt} className={...} />) to include referrerPolicy="no-referrer" so remote image requests do not leak referrer metadata to third-party hosts.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/desktop/src/renderer/components/MarkdownRenderer/components/SafeImage/SafeImage.tsx`:
- Around line 29-30: The URL check in SafeImage (the function that inspects the
`lower` variable in SafeImage.tsx) currently allows both "https://" and
"http://"; update it to disallow plain "http://" and only allow "https://" and
"data:" schemes instead. Locate the conditional that returns true for
lower.startsWith("https://") || lower.startsWith("http://") and change the logic
so it returns true only when the string starts with "https://" or "data:" (and
returns false otherwise), ensuring any other schemes including "http://" are
rejected.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/components/MarkdownRenderer/components/SafeImage/SafeImage.tsx`:
- Around line 61-64: The <img> tag in SafeImage (component SafeImage) currently
renders external src without a referrer policy; update the JSX for the img
element (the <img src={src} alt={alt} className={...} />) to include
referrerPolicy="no-referrer" so remote image requests do not leak referrer
metadata to third-party hosts.
🪄 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: 09608631-aad7-420c-b46b-67fa77d32a91
📒 Files selected for processing (8)
apps/desktop/src/renderer/components/MarkdownRenderer/components/SafeImage/SafeImage.tsxapps/desktop/src/renderer/components/MarkdownRenderer/styles/tufte/config.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/TasksView.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/GitHubIssuesContent/GitHubIssuesContent.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/PullRequestsContent/PullRequestsContent.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/TasksTopBar.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/layout.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/stores/tasks-filter-state.ts
💤 Files with no reviewable changes (1)
- apps/desktop/src/renderer/components/MarkdownRenderer/styles/tufte/config.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/GitHubIssuesContent/GitHubIssuesContent.tsx
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/TasksView.tsx
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
6 issues found across 25 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/components/AssigneeFilter/AssigneeFilter.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/components/AssigneeFilter/AssigneeFilter.tsx:133">
P2: This icon-only state relies on `title` for naming; add an explicit `aria-label` so the assignee button remains accessible when the visible text is hidden.</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/GitHubIssuesContent/GitHubIssuesContent.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/GitHubIssuesContent/GitHubIssuesContent.tsx:241">
P2: Guard the row key handler so Enter/Space on nested buttons does not also trigger row navigation.</violation>
</file>
<file name="packages/host-service/src/trpc/router/workspace-creation/procedures/search-github-issues.ts">
<violation number="1" location="packages/host-service/src/trpc/router/workspace-creation/procedures/search-github-issues.ts:159">
P2: Direct-lookup responses hardcode `page: 1` instead of returning the requested page, causing inconsistent pagination state across response branches.</violation>
</file>
<file name="apps/desktop/src/renderer/providers/ElectronTRPCProvider/ElectronTRPCProvider.tsx">
<violation number="1" location="apps/desktop/src/renderer/providers/ElectronTRPCProvider/ElectronTRPCProvider.tsx:66">
P1: Missing `query.state.status === 'success'` check in `shouldDehydrateQuery`. Without this, queries in pending or error states will be persisted to IndexedDB. On next app load, these restore with empty/broken data. The default dehydration behavior intentionally filters to successful queries only — this override drops that safeguard.
Use `defaultShouldDehydrateQuery(query)` (from `@tanstack/react-query`) alongside the prefix check to preserve the built-in status guard.</violation>
</file>
<file name="apps/desktop/src/renderer/components/MarkdownRenderer/components/SafeImage/SafeImage.tsx">
<violation number="1" location="apps/desktop/src/renderer/components/MarkdownRenderer/components/SafeImage/SafeImage.tsx:29">
P2: Avoid allowing `http://` image sources; permit only `https://` (and `data:`) to prevent insecure remote fetches.</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/PullRequestsContent/PullRequestsContent.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/PullRequestsContent/PullRequestsContent.tsx:249">
P2: PR row activation opens the internal preview route instead of the PR URL, which breaks the expected row-click behavior.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
- SafeImage: drop http://, allow only data: and https:// (cleartext is no longer needed; GitHub user-attachments are https). - ElectronTRPCProvider: compose shouldDehydrateQuery with defaultShouldDehydrateQuery so pending/error queries are excluded from the persisted IndexedDB cache. - search-pull-requests / search-github-issues: direct-lookup branches now echo the requested page instead of hardcoding 1, matching the search branches. - PullRequestsContent / GitHubIssuesContent: short-circuit row keydown when the event target isn't the row itself, so Enter/Space on a nested action button no longer also navigates to the preview. repoMismatch text gets select-text cursor-text so the repo string is copyable (per AGENTS.md). - AssigneeFilter / ProjectFilter / StatusFilter: add aria-label to the icon-only popover triggers (title alone is tooltip-only on many screen readers). - Bump @tanstack/react-query to ^5.100.9 to satisfy @tanstack/react-query-persist-client peer requirement. - Extract shared normalizePRState helper next to the PRState type so the PR list and detail page can't drift.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/host-service/src/trpc/router/workspace-creation/procedures/search-github-issues.ts (1)
208-221: 💤 Low valueState casing is normalized in the
ghpath but not in the Octokit fallback.
ghDirectLookup(line 59) andghApiSearchIssues(line 117) emitstate.toLowerCase(), but the Octokit branches at lines 214 and 240 forwardissue.state/item.stateas-is. GitHub's REST API today returns lowercase states so this is functionally equivalent, but the asymmetry is a future-proofing footgun (any consumer doing exact-match comparisons onstatewould silently break if the gh CLI vs. REST drift). Recommend lowercasing in both Octokit branches for parity with theIssueResultcontract used by theghpath.♻️ Proposed normalization
- state: issue.state, + state: issue.state.toLowerCase(), authorLogin: issue.user?.login ?? null,- state: item.state, + state: item.state.toLowerCase(), authorLogin: item.user?.login ?? null,Also applies to: 234-242
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/host-service/src/trpc/router/workspace-creation/procedures/search-github-issues.ts` around lines 208 - 221, The Octokit fallback branches return issue.state/item.state without normalization while ghDirectLookup and ghApiSearchIssues emit state.toLowerCase(); update the Octokit result construction in search-github-issues.ts (the branches that build the IssueResult objects from variables named issue and item) to call .toLowerCase() on the state before returning (e.g., state: issue.state?.toLowerCase() ?? issue.state and state: item.state?.toLowerCase() ?? item.state) so the returned IssueResult always uses lowercase state for parity with gh paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/pr/`$prNumber/page.tsx:
- Around line 200-211: The anchor that renders the LuExternalLink icon (the
conditional block using the url variable) lacks an accessible name; add an
aria-label attribute to that <a> element (e.g., aria-label="Open in GitHub" or
include the repo/PR identifier if available) so screen readers and keyboard
users get meaningful context while keeping the existing title, target, rel and
icon intact.
In
`@packages/host-service/src/trpc/router/workspace-creation/procedures/search-github-issues.ts`:
- Around line 120-122: Clamp GitHub's reported total to the API cap before
computing pagination: in ghApiSearchIssues and the Octokit search branch compute
an effectiveTotal = Math.min(parsed.total_count, 1000) and use that
effectiveTotal when setting hasNextPage (instead of parsed.total_count) and when
returning totalCount so the client stops at the real 1,000-item limit; ensure
the existing page, perPage and hasNextPage logic uses effectiveTotal for the
comparison (page * perPage < effectiveTotal).
---
Nitpick comments:
In
`@packages/host-service/src/trpc/router/workspace-creation/procedures/search-github-issues.ts`:
- Around line 208-221: The Octokit fallback branches return
issue.state/item.state without normalization while ghDirectLookup and
ghApiSearchIssues emit state.toLowerCase(); update the Octokit result
construction in search-github-issues.ts (the branches that build the IssueResult
objects from variables named issue and item) to call .toLowerCase() on the state
before returning (e.g., state: issue.state?.toLowerCase() ?? issue.state and
state: item.state?.toLowerCase() ?? item.state) so the returned IssueResult
always uses lowercase state for parity with gh paths.
🪄 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: 90671b64-c897-4ce9-a3a4-2d5a433539ec
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
apps/desktop/package.jsonapps/desktop/src/renderer/components/MarkdownRenderer/components/SafeImage/SafeImage.tsxapps/desktop/src/renderer/providers/ElectronTRPCProvider/ElectronTRPCProvider.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/GitHubIssuesContent/GitHubIssuesContent.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/PullRequestsContent/PullRequestsContent.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/components/AssigneeFilter/AssigneeFilter.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/components/ProjectFilter/ProjectFilter.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/components/StatusFilter/StatusFilter.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/pr/$prNumber/page.tsxapps/desktop/src/renderer/screens/main/components/PRIcon/index.tsapps/desktop/src/renderer/screens/main/components/PRIcon/normalizePRState.tspackages/host-service/src/trpc/router/workspace-creation/procedures/search-github-issues.tspackages/host-service/src/trpc/router/workspace-creation/procedures/search-pull-requests.ts
✅ Files skipped from review due to trivial changes (3)
- apps/desktop/src/renderer/screens/main/components/PRIcon/normalizePRState.ts
- apps/desktop/src/renderer/screens/main/components/PRIcon/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/GitHubIssuesContent/GitHubIssuesContent.tsx
🚧 Files skipped from review as they are similar to previous changes (7)
- apps/desktop/package.json
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/components/AssigneeFilter/AssigneeFilter.tsx
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/components/ProjectFilter/ProjectFilter.tsx
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/components/StatusFilter/StatusFilter.tsx
- apps/desktop/src/renderer/providers/ElectronTRPCProvider/ElectronTRPCProvider.tsx
- packages/host-service/src/trpc/router/workspace-creation/procedures/search-pull-requests.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/PullRequestsContent/PullRequestsContent.tsx
Comment on lines
+200
to
+211
| <div className="ml-auto flex items-center gap-1"> | ||
| {url && ( | ||
| <a | ||
| href={url} | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| className="text-muted-foreground hover:text-foreground transition-colors p-2" | ||
| title="Open in GitHub" | ||
| > | ||
| <LuExternalLink className="w-4 h-4" /> | ||
| </a> | ||
| )} |
Contributor
There was a problem hiding this comment.
Add aria-label to the icon-only external link
The "Open in GitHub" anchor renders only an icon with no accessible label—title is a tooltip, not an accessible name. Screen readers and keyboard users get no meaningful context.
♿ Proposed fix
<a
href={url}
target="_blank"
rel="noopener noreferrer"
className="text-muted-foreground hover:text-foreground transition-colors p-2"
title="Open in GitHub"
+ aria-label="Open pull request in GitHub"
>
<LuExternalLink className="w-4 h-4" />
</a>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <div className="ml-auto flex items-center gap-1"> | |
| {url && ( | |
| <a | |
| href={url} | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| className="text-muted-foreground hover:text-foreground transition-colors p-2" | |
| title="Open in GitHub" | |
| > | |
| <LuExternalLink className="w-4 h-4" /> | |
| </a> | |
| )} | |
| <div className="ml-auto flex items-center gap-1"> | |
| {url && ( | |
| <a | |
| href={url} | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| className="text-muted-foreground hover:text-foreground transition-colors p-2" | |
| title="Open in GitHub" | |
| aria-label="Open pull request in GitHub" | |
| > | |
| <LuExternalLink className="w-4 h-4" /> | |
| </a> | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/pr/`$prNumber/page.tsx
around lines 200 - 211, The anchor that renders the LuExternalLink icon (the
conditional block using the url variable) lacks an accessible name; add an
aria-label attribute to that <a> element (e.g., aria-label="Open in GitHub" or
include the repo/PR identifier if available) so screen readers and keyboard
users get meaningful context while keeping the existing title, target, rel and
icon intact.
Comment on lines
+120
to
122
| const hasNextPage = page * perPage < parsed.total_count; | ||
| return { items, totalCount: parsed.total_count, hasNextPage }; | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
GitHub REST search/issues API maximum results limit per query
💡 Result:
In GitHub REST API search for issues (GET /search/issues), the maximum total results returned per search query is 1,000 results. This is independent of pagination; pagination just lets you fetch the first pages up to that cap. [1] For the REST “list issues” endpoint for a specific repository (GET /repos/{owner}/{repo}/issues), you can paginate with per_page up to 100 results per page (max 100). You must keep requesting subsequent pages to get more than 100 issues. [2] How to interpret your example URL (repos/{owner}/{repo}/issues?per_page=100): - per_page=100 means “100 per request/page”. You cannot increase that beyond 100. [2] - To get all issues, you paginate via the page parameter (or the Link header) and aggregate client-side. [2][3] If you’re using the REST search endpoint (GET /search/issues?q=...), even with pagination you will not get more than 1,000 total matching results for that query. [1][4]
Citations:
- 1: https://docs.github.com/rest/search/search
- 2: https://docs.github.com/rest/issues/issues
- 3: https://docs.github.com/rest/using-the-rest-api/using-pagination-in-the-rest-api
- 4: There is a limit of 1000 results per search. PyGithub/PyGithub#824
🏁 Script executed:
cat -n packages/host-service/src/trpc/router/workspace-creation/procedures/search-github-issues.ts | head -250Repository: superset-sh/superset
Length of output: 7793
hasNextPage may over-report past GitHub's 1,000-result search cap.
GitHub's Search API (/search/issues) returns total_count for the full match set but caps actually-returnable items at 1,000 total across all pages. With the current calc page * perPage < parsed.total_count (line 120 in ghApiSearchIssues and line 243 in the Octokit search path), when total_count > 1000, hasNextPage will keep returning true past page Math.ceil(1000/perPage), and the desktop useInfiniteQuery consumer will keep fetching pages that come back empty. Clamp the effective total to 1,000 for both the gh CLI search and Octokit search paths so pagination terminates at the API's actual limit.
♻️ Suggested clamp (apply in both `ghApiSearchIssues` and the Octokit search branch)
- const hasNextPage = page * perPage < parsed.total_count;
- return { items, totalCount: parsed.total_count, hasNextPage };
+ const effectiveTotal = Math.min(parsed.total_count, 1000);
+ const hasNextPage = page * perPage < effectiveTotal;
+ return { items, totalCount: parsed.total_count, hasNextPage };- const hasNextPage = page * limit < data.total_count;
+ const effectiveTotal = Math.min(data.total_count, 1000);
+ const hasNextPage = page * limit < effectiveTotal;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const hasNextPage = page * perPage < parsed.total_count; | |
| return { items, totalCount: parsed.total_count, hasNextPage }; | |
| } | |
| const effectiveTotal = Math.min(parsed.total_count, 1000); | |
| const hasNextPage = page * perPage < effectiveTotal; | |
| return { items, totalCount: parsed.total_count, hasNextPage }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/host-service/src/trpc/router/workspace-creation/procedures/search-github-issues.ts`
around lines 120 - 122, Clamp GitHub's reported total to the API cap before
computing pagination: in ghApiSearchIssues and the Octokit search branch compute
an effectiveTotal = Math.min(parsed.total_count, 1000) and use that
effectiveTotal when setting hasNextPage (instead of parsed.total_count) and when
returning totalCount so the client stops at the real 1,000-item limit; ensure
the existing page, perPage and hasNextPage logic uses effectiveTotal for the
comparison (page * perPage < effectiveTotal).
Previous commit bumped only apps/desktop, leaving admin/mobile/web/ workspace-client on ^5.90.19. Bun resolved both major-incompatible ranges, duplicating @tanstack/query-core (5.100.9 + 5.95.2). The two QueryClient types are nominal, so passing electronQueryClient (now 5.100.9) into ChatPane and other call sites typed against the older copy failed turbo typecheck. Sherif also flagged the version split. Aligning everything to ^5.100.9 collapses the duplicate and matches the @tanstack/react-query-persist-client peer requirement.
Contributor
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/mobile/package.json">
<violation number="1" location="apps/mobile/package.json:51">
P3: This mobile dependency bump appears unrelated to the desktop feature scope and should be split into a separate PR for safer review and rollback.
(Based on your team's feedback about keeping PRs focused and avoiding unrelated changes in the same diff.) [FEEDBACK_USED]</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Tip: cubic used a learning from your PR history. Let your coding agent read cubic learnings directly with the cubic MCP.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
linkedPR/linkedIssues+selectedProjectId) and opens the existingDashboardNewWorkspaceModalpre-filled.type,project) persist in URL + the existing tasks-filter zustand store, so deep-links and back/forward work.Implementation notes
PullRequestsContent,GitHubIssuesContent,ProjectFilter(Command-style dropdown readingcollections.v2Projects).workspaceCreation.searchPullRequests/searchGitHubIssues(the same procedures used by the New Workspace modal's link pickers).prsorissues, task-only controls (status tabs, assignee, New task, view toggle) are hidden; the Linear CTA only blocks when type isall/tasks.all, all three sections render with section headers under the same project + search filters.Test plan
?type=prs&project=<id>→ state restores from URL.Follow-ups (not in this PR)
Summary by cubic
Unifies the Tasks page with tabs for Tasks, PRs, and Issues plus a Project filter. Adds in‑app PR/issue previews, infinite‑scroll lists with real totals and refresh, persisted IndexedDB caching, and allows https images in markdown.
New Features
LinearCTA) show only on Tasks;type/projectpersist in URL/store.gh api search/issueswith true totals + pagination (infinite scroll), a “Show closed/merged” toggle, and a header refresh; row click opens an in‑app preview; “Add to workspace” pre‑fillsselectedProjectId+linkedPR/linkedIssues./tasks/pr/$prNumberand/tasks/issue/$issueNumberrender full markdown bodies with an “Add to workspace” header action.@tanstack/react-query-persist-client+@tanstack/query-async-storage-persister; usesstaleTime: 30s,gcTime: 10m, andkeepPreviousData.SafeImagenow allowsdata:andhttps://sources; blockshttp://,file://, and unsafe paths.Bug Fixes
tasks,pull-request-detail,issue-detail.@tanstack/react-queryto^5.100.9across all packages to match persist‑client peers and avoid duplicatequery-core.Written for commit a17df59. Summary will update on new commits.
Summary by CodeRabbit
New Features
Improvements