feat(desktop): V2 workspace modal PR search improvements#3356
Conversation
…ion to V2 workspace modal Move GitHub PR URL detection, `#123` shorthand normalization, and cross-repo validation into the host-service `searchPullRequests` endpoint so the V2 client stays thin. The client sends raw user input and reacts to a `repoMismatch` field in the response. Also adds debounce gap handling to avoid empty-state flash while typing.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a shared GitHub query normalizer for PRs and issues, integrates it into server workspace-creation search routes, updates client PR/Issue link commands to handle trimmed/debounced input and repo-mismatch feedback, and introduces tests and design docs describing the behavior. Changes
Sequence DiagramsequenceDiagram
participant User
participant Client as PRLinkCommand / GitHubIssueLinkCommand
participant Debounce as Debounce Timer
participant Server as workspace-creation Router
participant Normalizer as normalizeGitHubQuery
participant GitHub as GitHub API
User->>Client: Type or paste URL / number
Client->>Client: compute trimmedQuery, set isPendingDebounce
Client->>Debounce: schedule debouncedTrimmed update
Client->>Client: show loading (isPendingDebounce=true)
Debounce->>Client: debounce elapsed
Client->>Server: send debouncedTrimmed
Server->>Normalizer: normalizeGitHubQuery(raw, repo, kind)
Normalizer->>Normalizer: parse URL / extract number / compare repo
alt repo mismatch
Normalizer-->>Server: {query: "", repoMismatch: true, isDirectLookup: false}
Server-->>Client: {results: [], repoMismatch: "owner/name"}
Client->>Client: display "URL must match owner/repo."
else direct lookup
Normalizer-->>Server: {query: "<number>", repoMismatch: false, isDirectLookup: true}
Server->>GitHub: octokit.get(number)
GitHub-->>Server: entity data
Server-->>Client: {results: [entity]}
Client->>Client: render single result
else text search
Normalizer-->>Server: {query: "<text>", repoMismatch: false, isDirectLookup: false}
Server->>GitHub: search.issuesAndPullRequests(query)
GitHub-->>Server: search results
Server-->>Client: {results: [...]}
Client->>Client: render results
end
Client->>Client: isPendingDebounce=false
Client-->>User: show results or empty state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR ports V1's GitHub PR URL paste and cross-repo validation into the V2 workspace creation modal, moving all normalization logic server-side (
Confidence Score: 3/5Not safe to merge as-is — the headline feature (paste PR URL → find that PR) won’t reliably work because the number is fed into a title-text search. URL parsing, cross-repo rejection, debounce logic, and client display are all solid. But the core user-facing feature — resolving a pasted PR URL to the actual PR — is broken: passing a PR number to packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts — needs a dedicated numeric-query code path (octokit.pulls.get or dropping in:title when effectiveQuery is purely numeric) Important Files Changed
Reviews (1): Last reviewed commit: "feat(host-service,desktop): add PR URL p..." | Re-trigger Greptile |
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts (1)
580-593:⚠️ Potential issue | 🟠 MajorDelay
ctx.github()until after the mismatch short-circuit.
ctx.github()can throw when the machine has no GitHub token configured, so a cross-repo URL can still fail before this code returnsrepoMismatch. That makes the new validation path depend on auth even though it should exit without touching GitHub.Suggested fix
.query(async ({ ctx, input }) => { const repo = await resolveGithubRepo(ctx, input.projectId); - const octokit = await ctx.github(); const limit = input.limit ?? 30; // Normalize the query: detect GitHub PR URLs, strip `#` shorthand const raw = input.query?.trim() ?? ""; const normalized = normalizePullRequestQuery(raw, repo); @@ if (normalized.repoMismatch) { return { pullRequests: [], repoMismatch: `${repo.owner}/${repo.name}`, }; } const effectiveQuery = normalized.query; + const octokit = await ctx.github();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts` around lines 580 - 593, The code calls ctx.github() before checking normalized.repoMismatch which can throw if no GitHub token exists; move the ctx.github() invocation so it occurs after normalizing the query and the early-return check (i.e., after calling normalizePullRequestQuery and verifying normalized.repoMismatch) to ensure the repoMismatch short-circuit in resolveGithubRepo/normalizePullRequestQuery runs without requiring authentication; update references around resolveGithubRepo, normalizePullRequestQuery, normalized.repoMismatch and only obtain octokit via ctx.github() when you actually need to make GitHub API calls (e.g., after the repoMismatch check and before using octokit).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/components/PRLinkCommand/PRLinkCommand.tsx`:
- Around line 67-93: The query hook currently destructures isLoading as
isFetching (const { data, isLoading: isFetching } = useQuery) which hides React
Query's actual isFetching semantics; change the destructure to pull isFetching
directly (const { data, isFetching } = useQuery) and update the local isLoading
computation to reference that isFetching variable (instead of the aliased
isFetching from isLoading) so in-flight refetches set the pending state
correctly; ensure references to repoMismatch and pullRequests remain unchanged.
In `@apps/desktop/V2_PR_LINK_COMMAND_DESIGN.md`:
- Around line 7-8: Update the "context" section to remove the stale claims "This
is a frontend-only change" and "no backend work needed" and instead state that
PRLinkCommand now relies on the host service (searchPullRequests) to perform URL
normalization and repository validation; mention that normalization and repo
validation have been moved into the host service rather than being handled
entirely in the frontend, and reference PRLinkCommand and the host service's
searchPullRequests endpoint so readers know where the logic now lives.
In `@apps/desktop/V2_WORKSPACE_MODAL_GAPS.md`:
- Around line 90-117: Update the documentation section describing Gap `#7` to
reflect that PR URL normalization and cross-repo validation are now handled by
the backend: remove or rewrite the paragraph that claims V2 lacks client-side PR
URL parsing and cross-repo checks, and update the priority table row for "#7 PR
URL parsing / cross-repo validation" to mark it as resolved or lower impact;
reference the implemented behavior in PRLinkCommand and
workspaceCreation.searchPullRequests and the surfaced UI field repoMismatch as
the justification for the change.
---
Outside diff comments:
In
`@packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts`:
- Around line 580-593: The code calls ctx.github() before checking
normalized.repoMismatch which can throw if no GitHub token exists; move the
ctx.github() invocation so it occurs after normalizing the query and the
early-return check (i.e., after calling normalizePullRequestQuery and verifying
normalized.repoMismatch) to ensure the repoMismatch short-circuit in
resolveGithubRepo/normalizePullRequestQuery runs without requiring
authentication; update references around resolveGithubRepo,
normalizePullRequestQuery, normalized.repoMismatch and only obtain octokit via
ctx.github() when you actually need to make GitHub API calls (e.g., after the
repoMismatch check and before using octokit).
🪄 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: 347511f2-930a-4611-82fd-305f9e0ebb11
📒 Files selected for processing (4)
apps/desktop/V2_PR_LINK_COMMAND_DESIGN.mdapps/desktop/V2_WORKSPACE_MODAL_GAPS.mdapps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/components/PRLinkCommand/PRLinkCommand.tsxpackages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts
There was a problem hiding this comment.
3 issues found across 4 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="packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts">
<violation number="1" location="packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts:599">
P1: Numeric PR queries from URL/# normalization are still searched with `in:title`, so `#123`/PR URL often won't find PR #123 unless the title contains that number.</violation>
</file>
<file name="apps/desktop/V2_WORKSPACE_MODAL_GAPS.md">
<violation number="1" location="apps/desktop/V2_WORKSPACE_MODAL_GAPS.md:94">
P3: The gap analysis incorrectly says V2 lacks PR URL parsing/cross-repo validation, but this PR implements that behavior server-side. This makes the new doc inaccurate.</violation>
</file>
<file name="apps/desktop/V2_PR_LINK_COMMAND_DESIGN.md">
<violation number="1" location="apps/desktop/V2_PR_LINK_COMMAND_DESIGN.md:7">
P3: The design doc is internally inconsistent: it says the change is frontend-only, but the final design requires host-service changes.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
When query is a PR number (from URL parsing or `#` shorthand), use `octokit.pulls.get()` instead of `in:title` text search. The text search only matched if the number appeared in the PR title.
… and tests - Extract normalizePullRequestQuery into its own module for testability - Use octokit.pulls.get() for bare numbers (e.g. "3130") not just #shorthand - Remove `in:title` from text search so it also matches PR body - Add 36 test cases covering: URL tabs (/files, /changes, /commits, /checks), query params, hash fragments, www prefix, http, cross-repo mismatch, #shorthand, bare numbers, non-PR URLs, GitHub Enterprise
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts (1)
95-96: Consider moving import to the top of the file.The import statement is placed after function definitions (Line 95), which is unconventional. Consolidating imports at the top improves readability and aligns with typical TypeScript conventions.
Suggested fix
Move the import near the other local imports at the top of the file (around line 11):
import { protectedProcedure, router } from "../../index"; import { deduplicateBranchName } from "./utils/sanitize-branch"; import { normalizePullRequestQuery } from "./normalize-pull-request-query";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts` around lines 95 - 96, The import for normalizePullRequestQuery is located after function definitions; move the import statement for normalizePullRequestQuery into the top imports block alongside other local imports (e.g., next to protectedProcedure, router and deduplicateBranchName) so all imports are consolidated at the top of workspace-creation.ts and maintain conventional ordering.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts`:
- Around line 95-96: The import for normalizePullRequestQuery is located after
function definitions; move the import statement for normalizePullRequestQuery
into the top imports block alongside other local imports (e.g., next to
protectedProcedure, router and deduplicateBranchName) so all imports are
consolidated at the top of workspace-creation.ts and maintain conventional
ordering.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c9df2405-fa88-40cd-aa04-29011ebfb656
📒 Files selected for processing (4)
apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/components/PRLinkCommand/PRLinkCommand.tsxpackages/host-service/src/trpc/router/workspace-creation/normalize-pull-request-query.test.tspackages/host-service/src/trpc/router/workspace-creation/normalize-pull-request-query.tspackages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts
There was a problem hiding this comment.
2 issues found across 1 file (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="packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts">
<violation number="1" location="packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts:536">
P2: Issue search no longer filters to open issues, so closed issues are now returned in normal search results.</violation>
<violation number="2" location="packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts:609">
P2: Default PR results now include closed/merged PRs when the query is empty.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
- Drop stale "open" from empty states — search is no longer open-only - Issue limit 20 → 30 to match PRs - Issue heading shows result count like PRs - Both default headings say "Recent" instead of implying open-only - Consistent "Loading..." text
- Move 3 stray plan docs to repo-root `plans/` (V2 PR link design, normalize-github-query plan, voice agent plan) - Move 3 shipped plan docs from `apps/desktop/docs/` into `apps/desktop/plans/done/` - Mark Gap 7 resolved in V2_WORKSPACE_MODAL_GAPS.md (PR #3356) - Add AGENTS.md rule #7 documenting plan vs doc placement
- Move 3 stray plan docs to repo-root `plans/` (V2 PR link design, normalize-github-query plan, voice agent plan) - Move 3 shipped plan docs from `apps/desktop/docs/` into `apps/desktop/plans/done/` - Mark Gap 7 resolved in V2_WORKSPACE_MODAL_GAPS.md (PR #3356) - Add AGENTS.md rule #7 documenting plan vs doc placement
- Move 3 stray plan docs to repo-root `plans/` (V2 PR link design, normalize-github-query plan, voice agent plan) - Move 3 shipped plan docs from `apps/desktop/docs/` into `apps/desktop/plans/done/` - Mark Gap 7 resolved in V2_WORKSPACE_MODAL_GAPS.md (PR superset-sh#3356) - Add AGENTS.md rule #7 documenting plan vs doc placement
…#3356) * Gaps * Link support * feat(host-service,desktop): add PR URL parsing and cross-repo validation to V2 workspace modal Move GitHub PR URL detection, `#123` shorthand normalization, and cross-repo validation into the host-service `searchPullRequests` endpoint so the V2 client stays thin. The client sends raw user input and reacts to a `repoMismatch` field in the response. Also adds debounce gap handling to avoid empty-state flash while typing. * fix(host-service): use direct PR lookup for URL paste and #123 shorthand When query is a PR number (from URL parsing or `#` shorthand), use `octokit.pulls.get()` instead of `in:title` text search. The text search only matched if the number appeared in the PR title. * fix(host-service): direct PR lookup for numbers, broader text search, and tests - Extract normalizePullRequestQuery into its own module for testability - Use octokit.pulls.get() for bare numbers (e.g. "3130") not just #shorthand - Remove `in:title` from text search so it also matches PR body - Add 36 test cases covering: URL tabs (/files, /changes, /commits, /checks), query params, hash fragments, www prefix, http, cross-repo mismatch, #shorthand, bare numbers, non-PR URLs, GitHub Enterprise * feat(host-service,desktop): unify GitHub query normalization for PRs and issues Generalize normalizePullRequestQuery into normalizeGitHubQuery with a `kind` parameter ("pull" | "issue"). Single regex matches both /pull/:number and /issues/:number URLs. - Wire into searchGitHubIssues: URL paste, #N shorthand, bare number direct lookup via octokit.issues.get(), cross-repo validation - Filter out PRs from issues.get() (GitHub returns PRs as issues) - Remove redundant `in:title,body` from issue text search - Update GitHubIssueLinkCommand client: debounce gap handling, repoMismatch display - Cross-entity fallback: wrong URL kind falls through to text search - 50 tests covering PRs, issues, cross-entity, and edge cases * fix: address PR review feedback (items 1-4) 1. Delay ctx.github() until after repoMismatch short-circuit in both searchPullRequests and searchGitHubIssues — avoids auth errors when the query is a cross-repo URL 2. Lowercase urlPath before comparing entity kind — fixes case-sensitive mismatch when regex captures "PULL" or "Issues" from uppercase URLs 3. Use isFetching instead of isLoading from useQuery in both client components — correctly reflects background refetch state 4. Use debouncedTrimmed for issue list heading instead of raw searchQuery — prevents "Results" label on whitespace-only input * fix(desktop): match V1 issue search UX with client-side Fuse.js filtering V2 was sending every keystroke to the GitHub search API which was slow and couldn't match issue numbers reliably. V1 pre-fetches all open issues once and does instant client-side fuzzy search with Fuse.js (issue number weighted 3x, title weighted 2x). Now V2 does the same: pre-fetch all open issues on popover open, Fuse.js for text/number filtering, and only hits the server for URL paste and #N shorthand (which need cross-repo validation and direct lookup). * Revert "fix(desktop): match V1 issue search UX with client-side Fuse.js filtering" This reverts commit 86c6151. * fix(host-service): use search API for issue listing to avoid PR contamination The no-query path used octokit.issues.listForRepo() which returns PRs mixed with issues. With per_page: 30, most slots were consumed by PRs that got filtered out client-side, leaving very few actual issues. Switch to octokit.search.issuesAndPullRequests() with `is:issue is:open` so GitHub filters server-side and the full page is real issues. * refactor(host-service): collapse duplicate issue search paths into one query Both the text search and no-query paths were doing the same search API call with `is:issue is:open`. Merged into a single path that appends the effective query when present. * refactor(host-service): single search path for PRs, drop is:open from issues Collapse PR text search + no-query list into one search API call (same pattern as issues). Drop `is:open` from issue search so closed issues are findable — useful when linking context for workspace creation. Both endpoints now use one query: `repo:owner/name is:<type> <query>`. * fix(desktop): align PR and issue link command labels and limits - Drop stale "open" from empty states — search is no longer open-only - Issue limit 20 → 30 to match PRs - Issue heading shows result count like PRs - Both default headings say "Recent" instead of implying open-only - Consistent "Loading..." text * Lint
Summary
#123shorthand normalization, and cross-repo validation to the V2 workspace creation modal's PR link commandsearchPullRequestsendpoint (not client-side) to respect the V2 architecture boundaryrepoMismatchfield in the responseV2_WORKSPACE_MODAL_GAPS.md) and design doc (V2_PR_LINK_COMMAND_DESIGN.md)Host service changes
normalizePullRequestQuery()helper handles: full GitHub PR URLs → extract PR number + validate repo;#123→ strip#; plain text → pass throughsearchPullRequestsreturns{ repoMismatch: "owner/repo" }on cross-repo URL paste — no GitHub API call wastedClient changes
PRLinkCommandreadsrepoMismatchfrom response, shows "PR URL must match owner/repo." messageisPendingDebouncetracking smooths loading state during debounce windowTest plan
#123— searches by PR number 123Summary by cubic
Improves PR and issue search in the V2 workspace modal with shared server-side GitHub query normalization, cross-repo validation, and fast number-based direct lookups. Consolidates both endpoints to a single GitHub search path and allows linking closed issues, with smoother loading states and aligned UI labels/limits.
New Features
normalizeGitHubQueryforsearchPullRequestsandsearchGitHubIssues: parse PR/issue URLs, handle#123and bare numbers, validateowner/repo, wrong-entity URLs fall back to text, and short-circuit on cross-repo with{ repoMismatch }.octokit.pulls.get(), issues viaoctokit.issues.get()(filters out PRs). Docs added:V2_PR_LINK_COMMAND_DESIGN.md,V2_WORKSPACE_MODAL_GAPS.md,NORMALIZE_GITHUB_QUERY_PLAN.md.Bug Fixes
search.issuesAndPullRequestspath for both PRs and issues (repo:… is:<type> <query>), sorted by updated; droppedis:openso closed issues are findable; removed redundantin:qualifiers; filters still exclude PRs from issue results.isFetching, consistent “Loading…” text, result-count headings, “Recent PRs/Recent issues” defaults, issue limit increased to 30 to match PRs, repo-mismatch messages in PR/issue commands; delayctx.github()until after mismatch short-circuit, and case-insensitive URL kind matching.Written for commit fb79138. Summary will update on new commits.
Summary by CodeRabbit
New Features
Documentation
Tests