fix(desktop): return direct-lookup PR/issue regardless of state#4190
Conversation
… state Pasting a same-repo PR URL (or `#N`) into the v2 new-workspace modal returned an empty list when the PR was closed or merged, because the direct-lookup branch ran the same `!includeClosed && state !== open` filter as the text-search branch. The dropdown rendered "No open pull requests found." with no hint that the PR existed. Direct lookups are unambiguous — the user named a specific PR/issue and wants that one back. State filtering only makes sense for the browse/text-search branch where it bounds an open-ended result set. Drop the state guard from both gh and Octokit direct-lookup branches in searchPullRequests and searchGitHubIssues. Keep the entity-type guards (`/pull/` URL check, `issue.pull_request` field) that prevent issues from leaking into PR results and vice versa — those aren't about state.
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
💤 Files with no reviewable changes (2)
📝 WalkthroughWalkthroughTwo GitHub issue and pull-request lookup procedures are refactored. The issue lookup now explicitly detects and rejects PR results by checking URL paths or PR fields in direct lookups. The PR lookup removes state-based filtering in direct lookups, returning resolved PRs regardless of open/closed status, while search queries continue to respect the ChangesDirect Lookup Filtering
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 SummaryDirect-lookup paths for PRs and issues now bypass the
Confidence Score: 5/5Safe to merge — the change is a small, targeted removal of a filter that was incorrectly applied to unambiguous direct lookups. Both files make the same minimal, symmetrical change: dropping the state guard from the direct-lookup branch while leaving the text-search branch untouched. The entity-type guards that prevent PRs from surfacing in issue results (and vice versa) are fully preserved. The gh-CLI and Octokit fallback paths both receive the same treatment, so neither code path regresses. No new logic is introduced that could produce wrong data or unexpected side effects. No files require special attention.
|
| Filename | Overview |
|---|---|
| packages/host-service/src/trpc/router/workspace-creation/procedures/search-pull-requests.ts | Removes the includeClosed state guard from both the gh-CLI and Octokit direct-lookup branches so pasting a specific PR URL/number always returns that PR regardless of state; text-search branch is unchanged. |
| packages/host-service/src/trpc/router/workspace-creation/procedures/search-github-issues.ts | Same targeted removal of the includeClosed state guard from the direct-lookup branches; the /pull/ URL check and issue.pull_request entity-type guards are correctly retained. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User pastes query] --> B{normalizeGitHubQuery}
B -- repoMismatch --> C[Return repoMismatch error]
B -- isDirectLookup=true --> D[Direct Lookup Branch]
B -- isDirectLookup=false --> E[Text-Search Branch]
D --> F{Try gh CLI}
F -- success --> G["Return item (any state) ✅"]
F -- error --> H{Try Octokit}
H -- success --> I["Return item (any state) ✅"]
H -- error --> J[Rethrow → error toast]
E --> K{includeClosed?}
K -- false --> L[Filter: open only]
K -- true --> M[Filter: all states]
L --> N[Return filtered results]
M --> N
style G fill:#d4edda
style I fill:#d4edda
Reviews (1): Last reviewed commit: "fix(host-service): always return direct-..." | Re-trigger Greptile
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
Changes since v0.2.11: - workspaces: add `superset workspaces update <id> --name "..."` for renaming. Branch/host moves stay desktop-only — they require host-side git orchestration that isn't safe to drive from the cloud directly. (#4189) - organization: new `superset organization members list` command (also exposed via SDK + mcp-v2). Read-only — add/remove keep their existing UI flows. (#4197) - tasks: new `superset tasks statuses list` command surfacing the organization's configured task statuses. (#4197) - host-service: paste a closed/merged PR URL or `#N` into the v2 new-workspace modal and the dropdown actually returns the PR instead of "No open pull requests found." Direct lookups now ignore the open-only state filter that bounds text-search results. (#4190) Push cli-v0.2.12 after this lands to fire the release pipeline.
Summary
#N) into the v2 new-workspace modal returned an empty list when the PR was closed or merged. The direct-lookup branch was applying the same!includeClosed && state !== openfilter as the text-search branch, so the dropdown rendered "No open pull requests found." with no hint that the PR actually existed.searchPullRequestsandsearchGitHubIssues. Kept the entity-type guards (/pull/URL check,issue.pull_request) that prevent issues from leaking into PR results and vice versa.Test plan
#Nshorthand for a closed PR → resolvesSummary by cubic
Return the exact PR/issue when a same-repo URL or
#Nis pasted in the v2 new-workspace modal, even if it’s closed or merged. Text search still respects “Show closed”.searchPullRequestsandsearchGitHubIssues(GH and Octokit).Written for commit c4a8a8f. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes