fix(host-service): gh CLI first-class for PR/issue search; surface failures in UI#4140
Conversation
…ilures in UI Drops the cloud `github_repositories` join dependency in `resolveGithubRepo`: v2 projects created without the GitHub App installed for the org had `github_repository_id = NULL`, which made `searchPullRequests` / `searchGitHubIssues` throw `BAD_REQUEST: Project has no linked GitHub repository`. The renderer's PR/issue dropdown swallowed the error as "No open pull requests", hiding the bug from users. Resolver now reads owner/name from local `projects.repoOwner`/`repoName` or parses the cloud `repoCloneUrl`, and returns the local `repoPath` for gh's cwd. Search procedures shell `gh pr list --search …` / `gh issue list --search …` first-class (matches v1 behavior), with Octokit fallback for hosts without `gh` installed/authenticated. The renderer now surfaces tRPC errors via toast and an inline selectable error in the dropdown empty-state, so similar upstream failures don't hide silently again. Plan: plans/20260506-gh-cli-first-class.md
|
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 gh CLI–first path for PR/issue lookup with Octokit fallback, injects an executable ChangesGitHub CLI–First Workspace Creation
Sequence DiagramsequenceDiagram
participant Desktop as Desktop UI
participant Backend as TRPC Backend
participant DB as Local Database
participant GhCli as GitHub CLI
participant Octokit as Octokit API
rect rgba(100, 200, 255, 0.5)
Note over Desktop,Backend: User searches for issues/PRs
Desktop->>Backend: searchIssues/searchPRs(query, projectId)
Backend->>DB: resolveGithubRepo(projectId)
DB-->>Backend: { owner, name, repoPath }
end
rect rgba(150, 255, 150, 0.5)
Note over Backend,GhCli: Try gh CLI path first
alt Direct lookup (e.g., "#123")
Backend->>GhCli: gh <issue|pr> view --json <fields> --repo owner/name --repo-path repoPath
GhCli-->>Backend: JSON result
else Search query
Backend->>GhCli: gh <issue|pr> list --json <fields> --search "<q>" --repo owner/name --repo-path repoPath
GhCli-->>Backend: JSON results
end
Backend-->>Desktop: Mapped results (if gh succeeded)
end
rect rgba(255, 200, 100, 0.5)
Note over Backend,Octokit: Fallback if gh fails
Backend->>Octokit: octokit search/list with owner/name
Octokit-->>Backend: Results or Error
Backend-->>Desktop: Mapped results or empty
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 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 fixes a long-standing bug where projects created without the GitHub App (
Confidence Score: 3/5The gh-first-class path and error surfacing are solid, but the new resolver silently drops the cloudProject.githubRepository fallback, meaning older projects with a githubRepository cloud record but no repoCloneUrl will break where they previously worked. The resolver change is the riskiest part of the PR. The existing integration test had to have repoCloneUrl added to its mock fixture because the new code never reads cloudProject.githubRepository. If any production cloud project has a github_repositories row but the repoCloneUrl column is not populated or not returned by the API, PR/issue search will throw instead of resolving. The gh-first-class path, ExecGh injection, and renderer error-surfacing changes are clean and well-tested. packages/host-service/src/trpc/router/workspace-creation/shared/project-helpers.ts — the cloud fallback path should also check cloudProject.githubRepository before throwing.
|
| Filename | Overview |
|---|---|
| packages/host-service/src/trpc/router/workspace-creation/shared/project-helpers.ts | Rewrites resolveGithubRepo to source owner/name from local DB then repoCloneUrl, dropping the githubRepository cloud join — but also drops the githubRepository fallback entirely, risking a regression for older projects. |
| packages/host-service/src/trpc/router/workspace-creation/procedures/search-pull-requests.ts | Adds gh pr view / gh pr list --search as the primary path with Octokit fallback; response shape is preserved and state normalization is correct. |
| packages/host-service/src/trpc/router/workspace-creation/procedures/search-github-issues.ts | Adds gh issue view / gh issue list --search as the primary path; missing PR-filter guard in the gh issue view direct-lookup path could theoretically leak a PR item into issue results. |
| apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/components/PRLinkCommand/PRLinkCommand.tsx | Surfaces search failures via Sonner toast + inline empty-state; the lastToastedError ref resets on every re-fetch, so a persistent error will re-toast on each dropdown open. |
| apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/components/GitHubIssueLinkCommand/GitHubIssueLinkCommand.tsx | Same toast + inline error pattern as PRLinkCommand; shares the same ref-reset re-toast behaviour on dropdown re-open. |
| packages/host-service/test/integration/workspace-creation-github.integration.test.ts | Adds regression tests for repoCloneUrl-based resolution and first-class gh path; fakeOctokit throws loudly on any call when gh succeeds, making accidental fallbacks immediately visible. |
| packages/host-service/test/helpers/createTestHost.ts | Adds execGh option to test host; defaults to a stub that always rejects so existing tests exercise the Octokit fallback rather than shelling out to a real gh. |
| packages/host-service/src/app.ts | Wires execGh onto CreateAppOptions and passes it into the router context, mirroring the existing github factory injection pattern. |
| packages/host-service/src/types.ts | Adds execGh: ExecGh to HostServiceContext; straightforward type extension. |
| packages/host-service/src/trpc/router/workspace-creation/utils/exec-gh.ts | Converts execGh from a plain async function to a typed const matching the new ExecGh interface; logic is unchanged. |
Sequence Diagram
sequenceDiagram
participant UI as PRLinkCommand / GitHubIssueLinkCommand
participant tRPC as searchPullRequests / searchGitHubIssues
participant DB as Local projects DB
participant Cloud as Cloud API (v2Project.get)
participant GH as gh CLI
participant Octokit as Octokit (fallback)
UI->>tRPC: query(projectId, query)
tRPC->>DB: projects.findFirst(projectId).sync()
alt local repoOwner + repoName
DB-->>tRPC: {owner, name, repoPath}
else local repoUrl parseable
DB-->>tRPC: parse repoUrl to {owner, name, repoPath}
else no local row
tRPC->>Cloud: v2Project.get(projectId)
Cloud-->>tRPC: {repoCloneUrl, ...}
tRPC->>tRPC: parseGitHubRemote(repoCloneUrl)
end
tRPC->>GH: gh pr view/list --repo owner/name --json ...
alt gh succeeds
GH-->>tRPC: JSON result
tRPC-->>UI: {pullRequests/issues}
else gh fails
GH-->>tRPC: error
tRPC->>Octokit: pulls.get / search.issuesAndPullRequests
alt Octokit succeeds
Octokit-->>tRPC: data
tRPC-->>UI: {pullRequests/issues}
else Octokit fails
Octokit-->>tRPC: error
tRPC-->>UI: empty list (silent)
end
end
alt error received by UI
UI->>UI: toast.error + inline error state
end
Comments Outside Diff (1)
-
apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/components/PRLinkCommand/PRLinkCommand.tsx, line 128-138 (link)Toast fires on every dropdown re-open when the error persists
The
lastToastedError.currentref is reset tonullwhenevererrortransitions to null — which happens at the start of every fresh query attempt (when the user closes and reopens the dropdown,enabledflips false→true and React Query enters the loading state witherror = null). On the next query failure the same message is seen as "new," triggering anothertoast.error. A user hitting a persistent backend error will get a new toast notification each time they open the PR picker.Prompt To Fix With AI
This is a comment left during a code review. Path: apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/components/PRLinkCommand/PRLinkCommand.tsx Line: 128-138 Comment: **Toast fires on every dropdown re-open when the error persists** The `lastToastedError.current` ref is reset to `null` whenever `error` transitions to null — which happens at the start of every fresh query attempt (when the user closes and reopens the dropdown, `enabled` flips false→true and React Query enters the loading state with `error = null`). On the next query failure the same message is seen as "new," triggering another `toast.error`. A user hitting a persistent backend error will get a new toast notification each time they open the PR picker. How can I resolve this? If you propose a fix, please make it concise.
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
packages/host-service/src/trpc/router/workspace-creation/shared/project-helpers.ts:65-84
**`githubRepository` fallback silently dropped — regression for existing projects**
The cloud path now only tries `repoCloneUrl`, discarding the `cloudProject.githubRepository` field that the old code relied on. Any project where the cloud API returns `githubRepository: { owner, name }` but `repoCloneUrl` is null or unparseable (e.g., older projects or edge-cases where the GitHub App row exists but the clone URL isn't backfilled) will now throw "Project has no resolvable GitHub repository" — a regression from the old behaviour that correctly resolved them.
The existing test suite had to have `repoCloneUrl` added to its mock fixture (see the new `repoCloneUrl: "https://github.com/octocat/hello.git"` line in the integration tests) precisely because the new resolver no longer reads `githubRepository` at all. Adding a third fallback — `if (cloudProject.githubRepository?.owner && cloudProject.githubRepository?.name)` — before the final `throw` would preserve backward compat without re-introducing the cloud dependency for the happy path.
### Issue 2 of 3
apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/components/PRLinkCommand/PRLinkCommand.tsx:128-138
**Toast fires on every dropdown re-open when the error persists**
The `lastToastedError.current` ref is reset to `null` whenever `error` transitions to null — which happens at the start of every fresh query attempt (when the user closes and reopens the dropdown, `enabled` flips false→true and React Query enters the loading state with `error = null`). On the next query failure the same message is seen as "new," triggering another `toast.error`. A user hitting a persistent backend error will get a new toast notification each time they open the PR picker.
### Issue 3 of 3
packages/host-service/src/trpc/router/workspace-creation/procedures/search-github-issues.ts:104-126
**`gh issue view` on a PR number may succeed and leak a PR into issue results**
The Octokit direct-lookup path filters results with `if (issue.pull_request) { return { issues: [] }; }` because GitHub's issues API returns PRs too. The `gh issue view` path has no equivalent guard. While `gh issue view <pr-number>` often fails with "Not an issue", some `gh` versions return issue-like JSON for PRs. If it succeeds, `ghIssueSchema` parses it without complaint and a PR item appears in issue search results.
Reviews (1): Last reviewed commit: "fix(host-service): gh CLI first-class fo..." | Re-trigger Greptile
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/components/PRLinkCommand/PRLinkCommand.tsx (1)
85-98: ⚡ Quick winConsider extracting the toast-on-error pattern to a shared hook.
The
lastToastedErrorref +useEffect+toast.errorblock here is a copy of the same pattern inGitHubIssueLinkCommand.tsx(lines 85-95). Extracting into a small hook (e.g.,useQueryErrorToast(error, prefix)) would centralize the dedup/reset semantics and keep both call sites a one-liner.♻️ Sketch of a shared hook
// e.g., renderer/hooks/useQueryErrorToast.ts (co-located near consumers) import { useEffect, useRef } from "react"; import { toast } from "@superset/ui/sonner"; export function useQueryErrorToast( error: unknown, formatMessage: (msg: string) => string, ) { const lastToasted = useRef<string | null>(null); useEffect(() => { const msg = error instanceof Error ? error.message : null; if (!msg) { lastToasted.current = null; return; } if (lastToasted.current === msg) return; lastToasted.current = msg; toast.error(formatMessage(msg)); }, [error, formatMessage]); }Usage:
- const lastToastedError = useRef<string | null>(null); - useEffect(() => { - const msg = error instanceof Error ? error.message : null; - if (!msg) { - lastToastedError.current = null; - return; - } - if (lastToastedError.current === msg) return; - lastToastedError.current = msg; - toast.error(`Couldn't load pull requests: ${msg}`); - }, [error]); + useQueryErrorToast(error, (msg) => `Couldn't load pull requests: ${msg}`);As per coding guidelines — "Co-locate utils, hooks, constants, config, and stories next to the file using them" — place the hook somewhere both consumers can share, e.g.
renderer/hooks/.🤖 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/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/components/PRLinkCommand/PRLinkCommand.tsx` around lines 85 - 98, Extract the duplicated toast-on-error logic (the lastToastedError ref + useEffect + toast.error block) into a shared hook named useQueryErrorToast(error, formatMessage) that encapsulates deduping by message, resetting when there's no message, and calling toast.error with a formatted message; then replace the block in PRLinkCommand (remove lastToastedError and the useEffect) and the same block in GitHubIssueLinkCommand with a single call to useQueryErrorToast(error, msg => `Couldn't load pull requests: ${msg}`) or an appropriate prefix, and ensure the hook's effect deps include error and formatMessage so semantics remain identical.packages/host-service/src/trpc/router/workspace-creation/procedures/search-github-issues.ts (1)
121-126: 💤 Low valueConsider a debug-level signal when gh is simply unavailable.
console.warnwill fire on every PR/issue search for users who don't haveghinstalled/authenticated, which can be noisy in logs. A small refinement: detect an unavailability error class once at startup or distinguish "gh missing/auth" (debug) from "gh ran but failed" (warn). Optional and easy to defer.🤖 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 121 - 126, The catch block in searchGitHubIssues currently logs every gh failure with console.warn (ghErr), which is noisy when gh is just not installed/authenticated; change this to detect gh availability once (e.g., run a lightweight check at module startup and set an isGhAvailable flag) or inspect ghErr to distinguish "gh missing/auth" vs runtime failures, then log missing/auth cases at debug level and keep real runtime failures as warnings; update the catch in workspaceCreation.searchGitHubIssues (ghErr) to use the new isGhAvailable/inspection logic and emit debug for missing/auth and warn for genuine gh execution errors.packages/host-service/test/integration/workspace-creation-github.integration.test.ts (2)
245-289: 💤 Low valueOptional: add an
issue viewbranch tofakeExecGhfor symmetry.
fakeExecGhcoverspr view,pr list, andissue list, but notissue view. There's no test exercisingsearchGitHubIssueswith a#Ndirect-lookup query against this suite today, so this is fine — but if someone adds one later, it will silently fall through to the line 288 default{}, fail schema parsing, and route to the (throwing) Octokit fakes, producing a misleading failure. A smallif (verb === "view" && args[0] === "issue")branch would future-proof the helper.🤖 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/test/integration/workspace-creation-github.integration.test.ts` around lines 245 - 289, The fakeExecGh test helper doesn't handle the "issue view" command, so calls like issue view will fall through to the default {} and cause misleading failures; add an if branch in fakeExecGh similar to the existing pr view branch — i.e., check if verb === "view" && args[0] === "issue" and return a single issue object with fields matching the issue view shape (number, title, url, state, author, etc.) so schema parsing and future tests (e.g., searchGitHubIssues direct-lookup) behave correctly.
146-218: ⚡ Quick winExplicitly pass
execGhin the cloud-dep regression suite to make test intent self-documenting.This suite intentionally exercises the Octokit fallback path by not passing
execGh, which relies oncreateTestHost's default (an async function that throws). While the default is intentional and well-documented, explicitly passingexecGh: async () => { throw new Error("gh unavailable in this suite"); }makes the dependency clear without requiring developers to cross-referencecreateTestHost.ts.♻️ Suggested change
host = await createTestHost({ githubFactory: async () => fakeOctokit, + execGh: async () => { + throw new Error("gh unavailable in this suite (forces Octokit fallback)"); + }, apiOverrides: {🤖 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/test/integration/workspace-creation-github.integration.test.ts` around lines 146 - 218, The test suite relies on createTestHost's default execGh (an async function that throws) but should make this explicit; in the beforeEach where createTestHost is called (the block that sets host and passes githubFactory and apiOverrides), add an explicit execGh property: execGh: async () => { throw new Error("gh unavailable in this suite"); } so the fallback-Octokit path tested by searchPullRequests/searchGitHubIssues is self-documenting; update the createTestHost(...) call in this suite accordingly.
🤖 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
`@packages/host-service/src/trpc/router/workspace-creation/procedures/search-pull-requests.ts`:
- Around line 198-204: The catch in the searchPullRequests procedure currently
swallows Octokit failures and returns { pullRequests: [] }, which prevents the
renderer's error UX from triggering; change the catch in the function
searchPullRequests (the try/catch around the Octokit fallback) to rethrow the
caught error (or throw a new error that includes context) instead of returning
an empty array so the procedure surfaces a thrown error to the caller; apply the
same change to the parallel catch in search-github-issues.ts to keep PR/issue UX
symmetric.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/components/PRLinkCommand/PRLinkCommand.tsx`:
- Around line 85-98: Extract the duplicated toast-on-error logic (the
lastToastedError ref + useEffect + toast.error block) into a shared hook named
useQueryErrorToast(error, formatMessage) that encapsulates deduping by message,
resetting when there's no message, and calling toast.error with a formatted
message; then replace the block in PRLinkCommand (remove lastToastedError and
the useEffect) and the same block in GitHubIssueLinkCommand with a single call
to useQueryErrorToast(error, msg => `Couldn't load pull requests: ${msg}`) or an
appropriate prefix, and ensure the hook's effect deps include error and
formatMessage so semantics remain identical.
In
`@packages/host-service/src/trpc/router/workspace-creation/procedures/search-github-issues.ts`:
- Around line 121-126: The catch block in searchGitHubIssues currently logs
every gh failure with console.warn (ghErr), which is noisy when gh is just not
installed/authenticated; change this to detect gh availability once (e.g., run a
lightweight check at module startup and set an isGhAvailable flag) or inspect
ghErr to distinguish "gh missing/auth" vs runtime failures, then log
missing/auth cases at debug level and keep real runtime failures as warnings;
update the catch in workspaceCreation.searchGitHubIssues (ghErr) to use the new
isGhAvailable/inspection logic and emit debug for missing/auth and warn for
genuine gh execution errors.
In
`@packages/host-service/test/integration/workspace-creation-github.integration.test.ts`:
- Around line 245-289: The fakeExecGh test helper doesn't handle the "issue
view" command, so calls like issue view will fall through to the default {} and
cause misleading failures; add an if branch in fakeExecGh similar to the
existing pr view branch — i.e., check if verb === "view" && args[0] === "issue"
and return a single issue object with fields matching the issue view shape
(number, title, url, state, author, etc.) so schema parsing and future tests
(e.g., searchGitHubIssues direct-lookup) behave correctly.
- Around line 146-218: The test suite relies on createTestHost's default execGh
(an async function that throws) but should make this explicit; in the beforeEach
where createTestHost is called (the block that sets host and passes
githubFactory and apiOverrides), add an explicit execGh property: execGh: async
() => { throw new Error("gh unavailable in this suite"); } so the
fallback-Octokit path tested by searchPullRequests/searchGitHubIssues is
self-documenting; update the createTestHost(...) call in this suite accordingly.
🪄 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: 47d47379-e882-44a4-913e-0d12505e54d0
📒 Files selected for processing (11)
apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/components/GitHubIssueLinkCommand/GitHubIssueLinkCommand.tsxapps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/components/PRLinkCommand/PRLinkCommand.tsxpackages/host-service/src/app.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/shared/project-helpers.tspackages/host-service/src/trpc/router/workspace-creation/utils/exec-gh.tspackages/host-service/src/types.tspackages/host-service/test/helpers/createTestHost.tspackages/host-service/test/integration/workspace-creation-github.integration.test.tsplans/20260506-gh-cli-first-class.md
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
`resolveGithubRepo` previously fell back to parsing `cloudProject.repoCloneUrl` when the local DB row's cached owner/name were missing. Both the cloud value and the cached columns are setup-time snapshots that drift (rename, fork, manual remote re-point); GitHub queries must always target wherever the local remote points right now. Resolver now reads `simpleGit(repoPath).getRemotes(true)` via the existing `resolveLocalRepo` helper and parses the live URL. Throws PROJECT_NOT_SETUP when the project has no local clone on this host (search isn't meaningful without one) and BAD_REQUEST when the local clone has no GitHub remote. Tests now seed real temp git working trees with `git remote add origin` instead of stubbing `v2Project.get.query` — only a real `.git` exercises the actual `git remote get-url` path. New test asserts cloud-vs-local divergence: when cloud says one repo and the local remote says another, the resolver follows the local remote.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/host-service/test/integration/workspace-creation-github.integration.test.ts (1)
421-431: 💤 Low valueOptional: add a
gh issue view#N`` direct-lookup test for symmetry.The PR-side has both
gh pr view(lines 382-402) andgh pr list(lines 404-419) coverage, but the issue side only coversgh issue list. AsearchGitHubIssues#Ntest that assertsgh issue viewis invoked would close the symmetry and lock in the direct-lookup path that mirrorspulls.get→gh issue view.🧪 Suggested additional test
test("searchGitHubIssues `#N` invokes `gh issue view` with cwd=repoPath", async () => { // Extend fakeExecGh to handle issue view, e.g.: // if (verb === "view" && args[0] === "issue") { return { number: Number(args[2]), ... }; } const result = await host.trpc.workspaceCreation.searchGitHubIssues.query({ projectId, query: "#42", }); expect(result.issues).toHaveLength(1); expect(result.issues[0].issueNumber).toBe(42); expect(ghCalls).toHaveLength(1); expect(ghCalls[0].args.slice(0, 5)).toEqual([ "issue", "view", "42", "--repo", "octocat/hello", ]); expect(ghCalls[0].cwd).toBe(realpathSync(repoDir)); });🤖 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/test/integration/workspace-creation-github.integration.test.ts` around lines 421 - 431, Add a new integration test that verifies the direct-lookup path for issues by calling searchGitHubIssues with a "#N" query (e.g., "#42"), extend the fakeExecGh test helper to handle the "issue view" invocation and return a matching issue payload, then assert the returned result.issues[0].issueNumber is 42, ghCalls length is 1, ghCalls[0].args begins with ["issue","view","42","--repo","octocat/hello"] and ghCalls[0].cwd equals realpathSync(repoDir); this mirrors the existing PR-view tests and locks in the gh issue view code path used by searchGitHubIssues.
🤖 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
`@packages/host-service/src/trpc/router/workspace-creation/shared/project-helpers.ts`:
- Around line 19-60: Two execGh call sites are missing the working-directory
option so GH commands run outside the local clone; update the execGh invocations
in the get-content procedures to pass the repo clone path by calling execGh(...,
{ cwd: repo.repoPath }). Specifically, locate the execGh calls in the
pull-requests get-content and issues get-content handlers and add the second
argument { cwd: repo.repoPath } (using the ResolvedGithubRepo.repoPath value) to
match the pattern used in search-pull-requests.ts and search-github-issues.ts.
---
Nitpick comments:
In
`@packages/host-service/test/integration/workspace-creation-github.integration.test.ts`:
- Around line 421-431: Add a new integration test that verifies the
direct-lookup path for issues by calling searchGitHubIssues with a "#N" query
(e.g., "#42"), extend the fakeExecGh test helper to handle the "issue view"
invocation and return a matching issue payload, then assert the returned
result.issues[0].issueNumber is 42, ghCalls length is 1, ghCalls[0].args begins
with ["issue","view","42","--repo","octocat/hello"] and ghCalls[0].cwd equals
realpathSync(repoDir); this mirrors the existing PR-view tests and locks in the
gh issue view code path used by searchGitHubIssues.
🪄 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: 5279fc4d-4f57-4b32-b4f5-a35e3f2f8864
📒 Files selected for processing (2)
packages/host-service/src/trpc/router/workspace-creation/shared/project-helpers.tspackages/host-service/test/integration/workspace-creation-github.integration.test.ts
Switch resolveGithubRepo from `resolveLocalRepo` (origin-first heuristic)
to `getGitHubRemotes` (canonical `git config --get-regexp ^remote\..*\.url$`)
plus an explicit preference order:
1. user-configured `projects.remoteName` (recorded at project setup —
stable, e.g. "upstream" when tracking an upstream fork)
2. `origin`
3. first GitHub remote on the repo
Also keeps the `git rev-parse --show-toplevel` canonicalization to
validate the path is a git repo and resolve through symlinks.
`getGitHubRemotes`'s underlying `git config` regex is preferred over
`git remote -v` because the latter appends partial-clone markers like
`[blob:none]` and is otherwise human-readable, not machine-stable.
New test asserts the remoteName preference: a repo with both `origin`
(user's fork) and `upstream` (real source), with `remoteName=upstream`
recorded at setup, must route searches against upstream — silently
defaulting to origin would surface the wrong PRs.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/host-service/test/integration/workspace-creation-github.integration.test.ts (2)
17-39: ⚡ Quick winDeduplicate the suite-3 seed by parameterizing
seedRepoFixture.The
beforeEachfor "prefers the user-configured remoteName" inlines the samemkdtemp+git init+addRemote+host.db.insert(projects)flow thatseedRepoFixturealready encapsulates, only adding a second remote and overridingremoteName. Extending the helper to accept additional remotes and aremoteNamekeeps the three suites symmetric and avoids drift if theprojectsrow schema changes again.♻️ Proposed refactor
async function seedRepoFixture( host: TestHost, projectId: string, - remoteUrl: string, + remoteUrl: string, + options?: { + remoteName?: string; + extraRemotes?: Array<{ name: string; url: string }>; + }, ): Promise<string> { const dir = mkdtempSync(join(tmpdir(), "ws-creation-github-test-")); const git = simpleGit(dir); await git.init(["--initial-branch=main"]); await git.addRemote("origin", remoteUrl); + for (const r of options?.extraRemotes ?? []) { + await git.addRemote(r.name, r.url); + } host.db .insert(projects) .values({ id: projectId, repoPath: dir, repoProvider: "github", repoOwner: null, repoName: null, repoUrl: null, - remoteName: "origin", + remoteName: options?.remoteName ?? "origin", }) .run(); return dir; }Then in suite 3:
- repoDir = mkdtempSync(join(tmpdir(), "ws-creation-github-test-")); - const git = simpleGit(repoDir); - await git.init(["--initial-branch=main"]); - await git.addRemote("origin", "https://github.com/me/cli.git"); - await git.addRemote("upstream", "https://github.com/upstream-org/cli.git"); - host.db - .insert(projects) - .values({ - id: projectId, - repoPath: repoDir, - repoProvider: "github", - repoOwner: null, - repoName: null, - repoUrl: null, - remoteName: "upstream", - }) - .run(); + repoDir = await seedRepoFixture( + host, + projectId, + "https://github.com/me/cli.git", + { + remoteName: "upstream", + extraRemotes: [ + { name: "upstream", url: "https://github.com/upstream-org/cli.git" }, + ], + }, + );Also applies to: 288-310
🤖 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/test/integration/workspace-creation-github.integration.test.ts` around lines 17 - 39, seedRepoFixture duplicates setup used in the "prefers the user-configured remoteName" beforeEach; refactor seedRepoFixture to accept optional parameters for additionalRemotes (e.g., array of {name,url}) and an override remoteName so the helper handles creating the repo dir (mkdtempSync), initializing git (simpleGit.init) and adding any remotes before inserting the projects row; then update the suite that inlines mkdtempSync+git.init+addRemote to call seedRepoFixture with the extra remote and remoteName override (and update other call sites at the noted range 288-310) so all three suites use the single parameterized helper (seedRepoFixture, mkdtempSync, simpleGit, and the projects insert).
361-499: ⚡ Quick winAdd a test for execGh failure → Octokit fallback path to validate the resiliency claim.
The PR introduces Octokit as a fallback when
execGhfails (gh missing, not authed, network error). The current suite tests the success path where gh is available and Octokit must not be called. Earlier suites test Octokit-only scenarios. However, no test exercises the critical transition: wiringexecGhto throw and verifying that Octokit is invoked as the fallback. Adding one negative-path test would lock in this resiliency guarantee.🤖 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/test/integration/workspace-creation-github.integration.test.ts` around lines 361 - 499, Add a negative-path integration test that verifies the execGh failure → Octokit fallback: update the test suite that uses fakeExecGh/fakeOctokit by adding a new test which configures createTestHost to use an execGh that throws (instead of the current fakeExecGh) and a spyable fakeOctokit (the existing fakeOctokit can be reused) then call host.trpc.workspaceCreation.searchPullRequests.query (and/or searchGitHubIssues.query) and assert that the Octokit methods (e.g., fakeOctokit.pulls.get or issues.get) were invoked and that the returned pull/issue data comes from Octokit; reference execGh, fakeExecGh, fakeOctokit, createTestHost, and host.trpc.workspaceCreation.searchPullRequests to locate where to add this failing-execGh test.
🤖 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
`@packages/host-service/src/trpc/router/workspace-creation/shared/project-helpers.ts`:
- Around line 57-67: The catch currently collapses all errors from
simpleGit(...).revparse into a generic TRPCError("Not a git repository"); update
the error handling in the revparse block so you inspect the caught error: if it
clearly indicates a non-git repo (e.g., message contains "not a git repository"
or exit code from rev-parse), throw the TRPCError with message `Not a git
repository: ${local.repoPath}` and include the original error as the cause;
otherwise rethrow or throw a TRPCError that preserves the original error as
cause (or returns the original error) so permission, missing git, or IO errors
are not misclassified. Target the revparse call and the TRPCError construction
in this file (simpleGit(local.repoPath).revparse and the TRPCError block) when
implementing this change.
---
Nitpick comments:
In
`@packages/host-service/test/integration/workspace-creation-github.integration.test.ts`:
- Around line 17-39: seedRepoFixture duplicates setup used in the "prefers the
user-configured remoteName" beforeEach; refactor seedRepoFixture to accept
optional parameters for additionalRemotes (e.g., array of {name,url}) and an
override remoteName so the helper handles creating the repo dir (mkdtempSync),
initializing git (simpleGit.init) and adding any remotes before inserting the
projects row; then update the suite that inlines mkdtempSync+git.init+addRemote
to call seedRepoFixture with the extra remote and remoteName override (and
update other call sites at the noted range 288-310) so all three suites use the
single parameterized helper (seedRepoFixture, mkdtempSync, simpleGit, and the
projects insert).
- Around line 361-499: Add a negative-path integration test that verifies the
execGh failure → Octokit fallback: update the test suite that uses
fakeExecGh/fakeOctokit by adding a new test which configures createTestHost to
use an execGh that throws (instead of the current fakeExecGh) and a spyable
fakeOctokit (the existing fakeOctokit can be reused) then call
host.trpc.workspaceCreation.searchPullRequests.query (and/or
searchGitHubIssues.query) and assert that the Octokit methods (e.g.,
fakeOctokit.pulls.get or issues.get) were invoked and that the returned
pull/issue data comes from Octokit; reference execGh, fakeExecGh, fakeOctokit,
createTestHost, and host.trpc.workspaceCreation.searchPullRequests to locate
where to add this failing-execGh test.
🪄 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: 7f7323e2-34c7-4057-a717-fbd4544e9f6c
📒 Files selected for processing (2)
packages/host-service/src/trpc/router/workspace-creation/shared/project-helpers.tspackages/host-service/test/integration/workspace-creation-github.integration.test.ts
Adds an integration test that scans `packages/host-service/src/**` and
fails CI when production code reads `cloudProject.repoCloneUrl` or the
cached `projects.repoOwner`/`repoName` columns. Those fields drift on
rename/fork/manual remote re-point — query code must derive owner/name
from the live local git remote via `resolveGithubRepo(ctx, projectId)`.
Allowlist covers the legitimate snapshot consumers:
- schema declarations (`db/schema.ts`)
- setup pipeline (`project/handlers.ts`, `project/project.ts`,
`project/utils/persist-project.ts`) — cloning needs the cloud URL
- the resolver itself (`workspace-creation/shared/project-helpers.ts`)
- the runtime PR poller and `git.ts`'s sidebar response shape
(TODO comments in the allowlist track the migration paths)
Migrate `git.getPullRequestThreads` to `resolveGithubRepo` since the
guard exposed it as a violation. The procedure was building octokit
graphql/issues.listComments queries from the cached `project.repoOwner`/
`repoName` columns; now reads the live remote like the search procedures
already do.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@packages/host-service/src/trpc/router/git/git.ts`:
- Around line 726-733: The current blanket catch around the call to
resolveGithubRepo(ctx, workspace.projectId) masks unexpected failures; change it
to only swallow the known expected errors (e.g., RepoNotFoundError /
NoGithubRemoteError or errors whose message matches "not set up" / "no GitHub
remote") and rethrow or log-and-throw all other errors so real regressions are
not hidden; update the try/catch to inspect the thrown error (instanceof or
error.message) for the expected cases and return the empty { reviewThreads: [],
conversationComments: [] } only for those, otherwise rethrow the error from
resolveGithubRepo.
In
`@packages/host-service/test/integration/no-snapshot-fields-for-queries.test.ts`:
- Around line 1-4: The test no-snapshot-fields-for-queries.test.ts is currently
placed under the top-level integration tests but should be co-located with the
guarded source that enforces the "no snapshot fields for queries" rule (the
router implementation under the trpc/router area); move this test file next to
that source file, update its imports (remove/adjust the glob/readFileSync
relative paths to use local relative imports to the router/module under test),
and ensure the test still exports the same test cases (using expect, test from
bun:test) so it runs as part of the module's local test suite.
- Around line 82-83: The comment filtering loop currently only skips lines
starting with "//" or "*" causing lines starting with "/*" or "*/" to be checked
by FORBIDDEN and produce false positives; update the condition that checks
trimmed prefixes (the trimmed.startsWith(...) guard in the test) to also skip
lines starting with "/*" and "*/" so comment block delimiters are ignored before
invoking FORBIDDEN.test(line).
🪄 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: b7a95702-9184-4d71-a54e-14dc757d8778
📒 Files selected for processing (2)
packages/host-service/src/trpc/router/git/git.tspackages/host-service/test/integration/no-snapshot-fields-for-queries.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@packages/host-service/src/trpc/router/workspace-creation/shared/project-helpers.ts`:
- Around line 62-66: The fallback using remotes.values().next().value collapses
to any (losing type safety), so change the fallback to use a typed array
snapshot (e.g. Array.from(remotes.values())[0] or [...remotes.values()][0]) so
the element retains the ResolvedGithubRepo type; update the expression that
assigns preferred (involving getGitHubRemotes, remotes, local.remoteName, and
preferred) to use the array-based fallback and, if helpful, annotate preferred
as ResolvedGithubRepo | undefined to keep compiler checks for preferred.owner
and preferred.name.
🪄 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: fe9d2e13-b5c0-4dc0-84c0-3b89cc614003
📒 Files selected for processing (7)
apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/components/PRLinkCommand/PRLinkCommand.tsxpackages/host-service/src/trpc/router/workspace-creation/procedures/search-pull-requests.tspackages/host-service/src/trpc/router/workspace-creation/shared/project-helpers.tspackages/host-service/src/trpc/router/workspace-creation/utils/exec-gh.tspackages/host-service/test/helpers/createTestHost.tspackages/host-service/test/integration/no-snapshot-fields-for-queries.test.tspackages/host-service/test/integration/workspace-creation-github.integration.test.ts
✅ Files skipped from review due to trivial changes (1)
- packages/host-service/src/trpc/router/workspace-creation/utils/exec-gh.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/host-service/test/helpers/createTestHost.ts
- packages/host-service/test/integration/no-snapshot-fields-for-queries.test.ts
- apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/components/PRLinkCommand/PRLinkCommand.tsx
- packages/host-service/src/trpc/router/workspace-creation/procedures/search-pull-requests.ts
- packages/host-service/test/integration/workspace-creation-github.integration.test.ts
- search-pull-requests / search-github-issues: rethrow when both gh and Octokit fail so the renderer toast fires instead of silent empty state (was masking the entire user-facing error UX). Verified with new tests asserting both procedures reject when both backends throw. - search-github-issues: filter PRs out of `gh issue view` results — gh returns PR data on `gh issue view <pr-number>` (verified by hitting cli/cli #13353), where Octokit's `issues.get` exposes `pull_request` for the same filter. Detect via the canonical URL containing `/pull/`. Mutation test confirms regression detection. - project-helpers: preserve `cause` on rev-parse failure so the underlying git error survives the tRPC envelope. - git.getPullRequestThreads: narrow blanket `catch` to TRPCError — expected resolver failures degrade silently, real bugs propagate. - no-snapshot-fields-for-queries guard: skip lines starting with `/*` too, so block-comment delimiters can't trip the regex. Skipping other review notes: - "githubRepository fallback dropped": resolver no longer reads either cloud field; reviewer was reasoning from the prior commit state. - "cwd missing in get-content procs": those procs use `--repo owner/name` which routes gh independent of cwd. No behavior change. - "co-locate guard test": cross-cutting invariant scan with no single source file to co-locate with.
…er-divergence describe The dropped test asserted only `expect(result.issues).toEqual([])` after calling `searchGitHubIssues` in the cloud-vs-local divergence scenario — proving nothing beyond 'didn't throw'. The sibling `searchPullRequests` test in the same describe already proves the resolver follows the local remote (URL contains cli/cli, not the cloud's somewhere/else value), and the resolver is shared, so the issues procedure inherits that guarantee. Also trim the now-unused `issues.get` field from the fake Octokit.
Changes since v0.2.10: - host-service: gh CLI is first-class for PR/issue search; failures surface in the UI instead of silently degrading. (#4140) - host-service: respawn lost terminal sessions on attach instead of dead-ending the pane after laptop sleep / daemon restart. (#4149) - host-service: don't crash startup when the shell env probe fails (e.g. user's .zshrc errors). (#4150) Also drop --draft from the CLI release workflow so the per-version release auto-publishes alongside the cli-latest rolling pointer. --prerelease stays (still required to keep desktop's auto-updater from picking up CLI releases on /releases/latest/). Push cli-v0.2.11 after this lands to fire the release pipeline.
Changes since v0.2.10: - host-service: gh CLI is first-class for PR/issue search; failures surface in the UI instead of silently degrading. (#4140) - host-service: respawn lost terminal sessions on attach instead of dead-ending the pane after laptop sleep / daemon restart. (#4149) - host-service: don't crash startup when the shell env probe fails (e.g. user's .zshrc errors). (#4150) Also drop --draft from the CLI release workflow so the per-version release auto-publishes alongside the cli-latest rolling pointer. --prerelease stays (still required to keep desktop's auto-updater from picking up CLI releases on /releases/latest/). Push cli-v0.2.11 after this lands to fire the release pipeline.
Summary
github_repositoriesjoin dependency inresolveGithubRepo. v2 projects created without the GitHub App installed for the org hadgithub_repository_id = NULL, which madeworkspaceCreation.searchPullRequests/searchGitHubIssuesthrowBAD_REQUEST: Project has no linked GitHub repositoryfor the rest of the project's life. Resolver now reads owner/name from localprojects.repoOwner/repoNameor parses the cloudrepoCloneUrl, and returns the localrepoPathso gh can run with the right cwd.ghCLI first-class for PR/issue search (gh pr list --search …,gh pr view,gh issue list --search …,gh issue view). Falls back to Octokit onghfailure (not installed, not authenticated, etc.).execGhis now injectable onCreateAppOptionsfor testability, mirroring thegithubfactory.PRLinkCommandandGitHubIssueLinkCommandnow readerrorfrom useQuery, fire a Sonnertoast.erroronce per error transition, and render the message inline in the dropdown empty-state withselect-text cursor-textso users can copy it for bug reports.Validation
app.asardist/main/host-service.js:31805–31818— confirmed identical pre-fixresolveGithubRepo.github_repository_id: null); pre-fix logic throws the user-reported BAD_REQUEST against it, post-fix resolves to{owner: "cli", name: "cli"}.code: -32600,httpStatus: 400,path: workspaceCreation.searchPullRequests). After applying the fix, search returns real cli/cli PRs andfs_usageshowsgh pr list --repo cli/cli ….workspace-creation-github.integration.test.ts:repoCloneUrlwhengithubRepositoryis null)gh pr view/gh pr list --search/gh issue list --searchare invoked with--repo octocat/hello)Plan:
plans/20260506-gh-cli-first-class.mdTest plan
bun run lintcleanbun run typecheckclean (29/29 packages)bun test packages/host-service/— 553 pass / 0 failgithub_repository_id: null— search returns PRs, no toastghremoved from PATH — Octokit fallback still returns PRs, no toastgithubRepositorycontinues to work (regression check)gh pr view --jsonfield set vs. existing v2 response shapeOut of scope (follow-ups)
getPullRequestThreads(graphql review threads + issues.listComments) togh api graphql/gh api .../comments --paginatefetchRepositoryPullRequests) togh api graphqlmergePRtogh pr mergegithub.*endpoints (getPRStatus,getPR,listPRs,getRepo,listDeployments,listDeploymentStatuses,getUser)Octokitfactory + drop@octokit/restonce nothing references itSummary by cubic
Make the GitHub CLI (
gh) the primary path for PR/issue search using the project’s local clone, and surface errors in the dropdown. Resolve the repo from the live local GitHub remote to fix “no linked repo” and remote mismatches.New Features
gh pr list/viewandgh issue list/viewwithcwdset to the local repo; fall back toOctokitifghisn’t available.execGhis injectable onCreateAppOptions.Bug Fixes
{ owner, name, repoPath }from the live local remote viagetGitHubRemoteswith preference:remoteName→origin→ first GitHub remote; canonicalize withgit rev-parse --show-toplevel.PROJECT_NOT_SETUPwhen there’s no local clone;BAD_REQUESTwhen the clone has no GitHub remote.gh issue viewresults (detect via URL) so issue searches don’t return PRs.git.getPullRequestThreadsnow usesresolveGithubRepo; expected resolver failures return empty data while unexpected errors propagate.repoCloneUrl, cachedrepoOwner/repoName) in GitHub queries.Written for commit b4090fa. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores