-
Notifications
You must be signed in to change notification settings - Fork 962
feat(desktop): V2 workspace modal PR search improvements #3356
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
5d01a32
Gaps
Kitenite d54976b
Link support
Kitenite 06f3c2a
feat(host-service,desktop): add PR URL parsing and cross-repo validat…
Kitenite b11ce4a
fix(host-service): use direct PR lookup for URL paste and #123 shorthand
Kitenite 76892ce
fix(host-service): direct PR lookup for numbers, broader text search,…
Kitenite ca4adde
feat(host-service,desktop): unify GitHub query normalization for PRs …
Kitenite e4a8be5
fix: address PR review feedback (items 1-4)
Kitenite 86c6151
fix(desktop): match V1 issue search UX with client-side Fuse.js filte…
Kitenite 8730928
Revert "fix(desktop): match V1 issue search UX with client-side Fuse.…
Kitenite 7acd4e3
fix(host-service): use search API for issue listing to avoid PR conta…
Kitenite 106c39e
refactor(host-service): collapse duplicate issue search paths into on…
Kitenite ab589ee
refactor(host-service): single search path for PRs, drop is:open from…
Kitenite 28ce5e3
fix(desktop): align PR and issue link command labels and limits
Kitenite fb79138
Lint
Kitenite File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,143 @@ | ||
| # V2 PRLinkCommand — Design Doc | ||
|
|
||
| > Porting V1's GitHub PR URL paste + cross-repo validation into the V2 workspace creation modal. | ||
|
|
||
| ## Context | ||
|
|
||
| V2's `PRLinkCommand` uses the host-service `searchPullRequests` endpoint for text-based PR search. V1 additionally supports pasting full GitHub PR URLs and validates them against the selected project's repo. This is a frontend-only change — no backend work needed. | ||
|
|
||
|
Kitenite marked this conversation as resolved.
|
||
| ### File References | ||
|
|
||
| | | Path | | ||
| |---|---| | ||
| | **V1 PRLinkCommand** | `src/renderer/components/NewWorkspaceModal/components/PromptGroup/components/PRLinkCommand/PRLinkCommand.tsx` | | ||
| | **V2 PRLinkCommand** | `src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/components/PRLinkCommand/PRLinkCommand.tsx` | | ||
| | **V2 PromptGroup** | `…/DashboardNewWorkspaceForm/PromptGroup/PromptGroup.tsx` | | ||
| | **V2 ProjectOption type** | `…/DashboardNewWorkspaceForm/PromptGroup/types.ts` | | ||
| | **V2 ModalContent** | `…/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceModalContent/DashboardNewWorkspaceModalContent.tsx` | | ||
|
|
||
| --- | ||
|
|
||
| ## Gap A: No GitHub PR URL Paste Support | ||
|
|
||
| **V1**: User pastes `https://github.com/owner/repo/pull/123` into the search field. V1 parses it with `parseGitHubPullRequestUrl()`, extracts the PR number, and uses that as the search query. | ||
|
|
||
| **V2**: Only does plain text search via `client.workspaceCreation.searchPullRequests`. Pasting a URL returns no results. | ||
|
|
||
| ## Gap B: No Cross-Repo Validation | ||
|
|
||
| **V1**: Receives `githubOwner` and `repoName` as props. Compares the parsed URL's `owner/repo` against these values. If they don't match, it blocks selection and shows: _"PR URL must match owner/repo."_ | ||
|
|
||
| **V2**: `PRLinkCommand` has no `githubOwner`/`repoName` props. However, this data **is already available** — `ProjectOption` in `types.ts` has `githubOwner` and `githubRepoName`, and `DashboardNewWorkspaceModalContent` resolves them from the `githubRepositories` collection. The data just isn't threaded through to `PRLinkCommand`. | ||
|
|
||
| ## Gap C: No Debounce Loading State | ||
|
|
||
| **V1**: Tracks `isPendingDebounce` by comparing `trimmedQuery !== debouncedTrimmed`. Shows loading state during the debounce window instead of briefly flashing "No results" before the query fires. | ||
|
|
||
| **V2**: No debounce gap handling. There's a brief flash of empty state between typing and the debounced query firing. | ||
|
|
||
| --- | ||
|
|
||
| ## Research: V1 vs VS Code vs GitHub Desktop | ||
|
|
||
| ### V1 (Superset) | ||
|
|
||
| A single regex that only matches full HTTPS GitHub.com PR URLs: | ||
|
|
||
| ``` | ||
| /^https?:\/\/(?:www\.)?github\.com\/([\w.-]+)\/([\w.-]+)\/pull\/(\d+)(?:[/?#].*)?$/i | ||
| ``` | ||
|
|
||
| **Handles:** `https://github.com/owner/repo/pull/123`, trailing slashes, query params, hash fragments, `www.` prefix. | ||
|
|
||
| **Does not handle:** | ||
| - Shorthand like `#123` or bare `123` — always requires the full URL | ||
| - `owner/repo#123` cross-reference syntax | ||
| - GitHub Enterprise domains | ||
| - SSH-style URLs | ||
|
|
||
| The parsed result extracts the PR number, which becomes the search query. If the URL's `owner/repo` doesn't match the selected project, selection is blocked with _"PR URL must match owner/repo."_ | ||
|
|
||
| ### VS Code | ||
|
|
||
| VS Code does **not** parse PR URLs from user input in search fields at all. It solves a different problem: | ||
|
|
||
| 1. **Git remote URL parsing** (`parseRemoteUrl` in `gitService.ts`) — normalizes SSH shorthand, aliases, ports, HTTPS URLs into canonical form. Supports GitHub Enterprise via `ghe.com` host matching. | ||
|
|
||
| 2. **PR detection via branch** (`PullRequestDetectionService`) — discovers PRs by querying GitHub API with the current branch name, not by parsing URLs. Uses exponential backoff retry. | ||
|
|
||
| Not applicable to our use case — VS Code never asks users to paste PR URLs. | ||
|
|
||
| ### GitHub Desktop | ||
|
|
||
| GitHub Desktop also **does not** support pasting PR URLs in its search. Its approach: | ||
|
|
||
| 1. **PR list is pre-fetched** — all open PRs for the current repo are loaded from the GitHub API upfront. No server-side search endpoint. | ||
|
|
||
| 2. **Client-side fuzzy filtering** (`pull-request-list.tsx:358-366`) — uses `fuzzaldrin-plus` library for fuzzy matching. Each PR is indexed as: | ||
| ```typescript | ||
| text: [pr.title, `#${pr.pullRequestNumber} opened ${timeAgo} by ${author}`] | ||
| ``` | ||
| So typing `#123` or `123` or part of a title all work through fuzzy match — no URL parsing needed. | ||
|
|
||
| 3. **Git remote parsing** (`remote-parsing.ts`) — parses repository URLs via multiple regex patterns covering HTTPS, SSH (`git@`), SSH with GHE domains (`*.ghe.com`), `git:` protocol, and `ssh://` protocol. Also supports `owner/repo` shorthand via `parseRepositoryIdentifier()`. | ||
|
|
||
| 4. **Cross-repo validation** (`repository-matching.ts:74-119`) — `repositoryMatchesRemote()` compares a PR's GitHub repository against local git remotes by parsing both URLs and comparing hostname, owner, and name (case-insensitive). Uses the same `parseRemote()` for both sides. | ||
|
|
||
| **Key takeaway:** GitHub Desktop sidesteps the URL paste problem entirely by pre-loading all PRs and doing client-side fuzzy search. The `#123` syntax works naturally because the subtitle string includes `#${prNumber}`. Their remote parsing is comprehensive (5 regex patterns, GHE support) but only used for git remotes, not browser URLs. | ||
|
|
||
| ### Other Superset Parsers | ||
|
|
||
| The codebase has several git remote URL parsers, none for PR URLs: | ||
|
|
||
| | Utility | Location | Purpose | | ||
| |---------|----------|---------| | ||
| | `parseGitHubRemote` | `packages/host-service` | SSH + HTTPS git remote → `{ owner, name }` | | ||
| | `normalizeGitHubRepoUrl` | `apps/desktop/.../changes/utils` | Git remote → normalized HTTPS URL | | ||
| | `normalizeGitHubUrl` | `apps/desktop/.../repo-context` | Git remote → `owner/repo` string | | ||
|
|
||
| ### Recommendation for V2 | ||
|
|
||
| Neither VS Code nor GitHub Desktop solve this exact problem — both avoid PR URL parsing in search fields entirely (VS Code uses branch detection, GitHub Desktop uses pre-fetched fuzzy search). | ||
|
|
||
| V1's regex is the right approach for our use case: parsing browser URLs pasted by users into a server-side search field. It covers all realistic browser-pasted URLs. | ||
|
|
||
| V2 should improve on V1 in one way: **strip `#` from shorthand like `#123`**. GitHub Desktop gets this for free via fuzzy matching (the subtitle includes `#123`). Our backend does server-side search, so we should normalize `#123` → `123` before sending the query to ensure it reliably matches by number. V1 sends `#123` as-is and hopes the backend text search handles it. | ||
|
|
||
| GitHub Enterprise isn't supported anywhere in the codebase, so adding it here would be inconsistent. The `owner/repo#123` cross-reference syntax adds complexity for a pattern nobody uses in a PR link popover. | ||
|
|
||
| --- | ||
|
|
||
| ## Design (Final — server-side normalization) | ||
|
|
||
| All URL parsing, `#` shorthand stripping, and cross-repo validation happen in the host service. The client sends raw user input and reacts to a `repoMismatch` field in the response. | ||
|
|
||
| ### 1. Host service: `normalizePullRequestQuery` helper | ||
|
|
||
| Added to `packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts`. | ||
|
|
||
| Handles three cases: | ||
| - **Full GitHub PR URL** → parse with regex, extract PR number, validate owner/repo against the project's linked repo. Return `{ repoMismatch: true }` if different. | ||
| - **`#123` shorthand** → strip the leading `#`, search by number. | ||
| - **Plain text** → pass through as-is. | ||
|
|
||
| The `searchPullRequests` procedure calls this before querying GitHub. On repo mismatch it returns early with `{ pullRequests: [], repoMismatch: "owner/repo" }` — no GitHub API call made. | ||
|
|
||
| ### 2. Client: thin — send raw query, react to `repoMismatch` | ||
|
|
||
| `PRLinkCommand` sends the raw `debouncedTrimmed` string to the host service. No URL parsing, no `githubOwner`/`repoName` props needed. | ||
|
|
||
| On response, reads `data.repoMismatch` (a string like `"owner/repo"` or absent). Shows _"PR URL must match owner/repo."_ in the empty state when present. | ||
|
|
||
| ### 3. Client: debounce gap handling | ||
|
|
||
| Tracks `isPendingDebounce` (`trimmedQuery !== debouncedTrimmed`) to show loading state during the debounce window instead of flashing "No results". | ||
|
|
||
| --- | ||
|
|
||
| ## Files Modified | ||
|
|
||
| | File | Change | | ||
| |------|--------| | ||
| | `packages/host-service/…/workspace-creation.ts` | Add `normalizePullRequestQuery` helper + wire into `searchPullRequests` procedure | | ||
| | `apps/desktop/…/PRLinkCommand/PRLinkCommand.tsx` (V2) | Add `isPendingDebounce`, read `repoMismatch` from response, update empty state messaging | | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,117 @@ | ||
| # V2 Workspace Creation Modal — Gap Analysis vs V1 | ||
|
|
||
| > Generated 2026-04-11. Compares V2 (`DashboardNewWorkspaceModal`) against V1 (`NewWorkspaceModal`). | ||
|
|
||
| ## File References | ||
|
|
||
| | | Path | | ||
| |---|---| | ||
| | **V1 PromptGroup** | `src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx` | | ||
| | **V2 PromptGroup** | `src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/PromptGroup.tsx` | | ||
| | **V2 Submit Hook** | `…/PromptGroup/hooks/useSubmitWorkspace/useSubmitWorkspace.ts` | | ||
| | **V2 Draft Context** | `…/DashboardNewWorkspaceModal/DashboardNewWorkspaceDraftContext.tsx` | | ||
|
|
||
| --- | ||
|
|
||
| ## Gaps | ||
|
|
||
| ### 1. Project Picker — "Open project" / "New project" actions | ||
|
|
||
| **V1**: Project picker includes a separator and two extra items — "Open project" (`onImportRepo`) and "New project" (`onNewProject`). | ||
|
|
||
| **V2**: Only lists existing projects with search. No way to import a repo or create a new project from within the modal. | ||
|
|
||
| **V1 ref**: `PromptGroup.tsx:246-268` | ||
| **V2 ref**: `ProjectPickerPill.tsx` (entire file) | ||
|
|
||
| --- | ||
|
|
||
| ### 2. Branch Picker — Worktree awareness and Open/Create actions | ||
|
|
||
| **V1** has a fully worktree-aware branch picker (`CompareBaseBranchPickerInline`) with: | ||
| - All / Worktrees filter toggle (tabs with counts) | ||
| - Differentiated icons: local branch, remote branch, openable worktree, active workspace | ||
| - "external" badge for external worktrees | ||
| - Active workspace detection with `GoArrowUpRight` icon | ||
| - Hover actions: "Open" button to navigate to existing workspace/worktree, "Create" button to create alongside an existing one | ||
| - Keyboard hint labels (Enter / Cmd+Enter) | ||
|
|
||
| **V2** has a simplified `CompareBaseBranchPicker` — flat list of branches with `GoGitBranch` icons, "default" and "workspace" badges. No open/create actions, no worktree filter toggle, no differentiated icons. | ||
|
|
||
| **V1 ref**: `PromptGroup.tsx:275-530` (`CompareBaseBranchPickerInline`) | ||
| **V2 ref**: `CompareBaseBranchPicker/CompareBaseBranchPicker.tsx` | ||
|
|
||
| --- | ||
|
|
||
| ### 3. AI Branch Name Generation | ||
|
|
||
| **V1**: On submit, calls `electronTrpc.workspaces.generateBranchName.mutateAsync()` with a 30-second timeout. Falls back to random name on timeout/failure with toast feedback. Shows "generating-branch" pending status. | ||
|
|
||
| **V2**: `resolveNames()` resolves names synchronously. No AI generation call. | ||
|
|
||
| **V1 ref**: `PromptGroup.tsx:600-608, 759-809` | ||
| **V2 ref**: `useSubmitWorkspace.ts` → `resolveNames()` | ||
|
|
||
| --- | ||
|
|
||
| ### 4. GitHub Issue Content Auto-Fetching | ||
|
|
||
| **V1**: On submit, fetches full GitHub issue content via `projects.getIssueContent.query()`, sanitizes it (HTML entity escaping, URL validation, 50KB body limit), converts to markdown, and attaches as file attachments alongside user-uploaded files. | ||
|
|
||
| **V2**: Only sends `githubIssueUrls` as string URLs in `linkedContext`. Does not fetch or attach issue content. | ||
|
|
||
| **V1 ref**: `PromptGroup.tsx:832-943` | ||
| **V2 ref**: `useSubmitWorkspace.ts` → `mapLinkedContext()` | ||
|
|
||
| --- | ||
|
|
||
| ### 5. Agent Launch Request Building | ||
|
|
||
| **V1**: Builds a full `AgentLaunchRequest` via `buildPromptAgentLaunchRequest()` with agent config, prompt, converted files, and task slug. Passes this to `createWorkspace.mutateAsyncWithPendingSetup()`. | ||
|
|
||
| **V2**: No `AgentLaunchRequest` is built. The prompt is sent as part of `composer` but agent config resolution, file bundling, and task slug mapping are missing. | ||
|
|
||
| **V1 ref**: `PromptGroup.tsx:695-708, 945-959` | ||
| **V2 ref**: `useSubmitWorkspace.ts:96-110` | ||
|
|
||
| --- | ||
|
|
||
| ### 6. Dedicated "Create from PR" Flow | ||
|
|
||
| **V1**: When a PR is linked, submit uses a separate code path — `createFromPr.mutateAsyncWithSetup()` — that creates the workspace from the PR's branch and metadata. | ||
|
|
||
| **V2**: Sends `linkedPrUrl` as part of `linkedContext`. The PR is treated as context rather than driving branch creation. No separate mutation. | ||
|
|
||
| **V1 ref**: `PromptGroup.tsx:963-982` | ||
| **V2 ref**: `useSubmitWorkspace.ts:79` → `mapLinkedContext()` | ||
|
|
||
| --- | ||
|
|
||
| ### 7. PR URL Parsing and Cross-Repo Validation | ||
|
|
||
| **V1**: `PRLinkCommand` parses pasted GitHub PR URLs (`github.com/:owner/:repo/pull/:number`), detects cross-repository links, and shows an error ("PR URL must match {repo}") for mismatched repos. | ||
|
|
||
| **V2**: `PRLinkCommand` uses host-service `searchPullRequests` endpoint only. No client-side URL parsing or cross-repo validation. | ||
|
Kitenite marked this conversation as resolved.
|
||
|
|
||
| **V1 ref**: `PRLinkCommand.tsx:37-53, 86-97` | ||
| **V2 ref**: `PRLinkCommand.tsx` (V2 version) | ||
|
|
||
| --- | ||
|
|
||
| ## Not a Gap (V2 advantage) | ||
|
|
||
| **Branch name preview**: V2 shows a live `branchPreview` in the branch name input placeholder using `slugifyForBranch(trimmedPrompt)`. V1 shows a static `"branch name"` placeholder. V2 is better here. | ||
|
|
||
| --- | ||
|
|
||
| ## Priority Assessment | ||
|
|
||
| | # | Gap | Impact | Effort | | ||
| |---|-----|--------|--------| | ||
| | 5 | Agent launch request building | High — agents won't receive full config/prompt/files | Medium | | ||
| | 3 | AI branch name generation | High — branch names won't be meaningful | Low | | ||
| | 4 | GitHub issue content fetching | Medium — issues linked as URLs only, not rich context | Medium | | ||
| | 6 | Dedicated "create from PR" flow | Medium — PR workspaces may not set up branches properly | Medium | | ||
| | 2 | Branch picker worktree awareness | Medium — can't discover/open existing worktrees | High | | ||
| | 1 | Project picker open/new actions | Low — can do this outside the modal | Low | | ||
| | 7 | PR URL parsing / cross-repo validation | Low — server search covers most cases | Low | | ||
|
Kitenite marked this conversation as resolved.
|
||
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
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.