diff --git a/src/main/github/auth-diagnose-routing.test.ts b/src/main/github/auth-diagnose-routing.test.ts new file mode 100644 index 00000000000..dc1839a492e --- /dev/null +++ b/src/main/github/auth-diagnose-routing.test.ts @@ -0,0 +1,44 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { ghExecFileAsyncMock, gitExecFileAsyncMock } = vi.hoisted(() => ({ + ghExecFileAsyncMock: vi.fn(), + gitExecFileAsyncMock: vi.fn() +})) + +vi.mock('../git/runner', () => ({ + ghExecFileAsync: ghExecFileAsyncMock, + gitExecFileAsync: gitExecFileAsyncMock +})) + +import { _resetOwnerRepoCache } from './gh-utils' +import { diagnoseGhAuth } from './auth-diagnose' + +describe('diagnoseGhAuth routing', () => { + beforeEach(() => { + ghExecFileAsyncMock.mockReset() + gitExecFileAsyncMock.mockReset() + _resetOwnerRepoCache() + }) + + it('checks auth status for the repo GitHub Enterprise host', async () => { + gitExecFileAsyncMock + .mockRejectedValueOnce(new Error('upstream missing')) + .mockResolvedValueOnce({ stdout: 'https://ghe.acme.internal/acme/orca.git\n' }) + ghExecFileAsyncMock.mockResolvedValueOnce({ + stdout: '', + stderr: `ghe.acme.internal + ✓ Logged in to ghe.acme.internal account octocat (keyring) + - Active account: true + - Token scopes: 'project', 'read:org', 'repo' +` + }) + + const result = await diagnoseGhAuth({ repoPath: '/repo' }) + + expect(result.activeAccount?.host).toBe('ghe.acme.internal') + expect(ghExecFileAsyncMock).toHaveBeenCalledWith( + ['auth', 'status', '--hostname', 'ghe.acme.internal'], + { cwd: '/repo' } + ) + }) +}) diff --git a/src/main/github/auth-diagnose.ts b/src/main/github/auth-diagnose.ts index aeaf4b9e284..b9635ba24a0 100644 --- a/src/main/github/auth-diagnose.ts +++ b/src/main/github/auth-diagnose.ts @@ -13,6 +13,8 @@ */ import { ghExecFileAsync } from '../git/runner' import type { GhAuthDiagnostic, GhAuthAccount } from '../../shared/github-auth-types' +import type { GitHubRepoTarget } from '../../shared/github-project-types' +import { getGitHubApiHostForRepo, ghRepoExecOptions, githubRepoContext } from './gh-utils' // Required scopes for ProjectV2 GraphQL access in Orca. `project` is the // scope that gates ProjectV2 reads/writes; the others are needed for the @@ -90,13 +92,23 @@ export function parseAuthStatus(text: string): GhAuthAccount[] { return accounts } -export async function diagnoseGhAuth(): Promise { +export async function diagnoseGhAuth(args: GitHubRepoTarget = {}): Promise { + // Why (issue #1715): pass --hostname for the repo's git remote host when + // known so auth status reports scopes for the GHES/github.com account that + // will actually serve ProjectV2 calls. + const ghHost = args.repoPath + ? await getGitHubApiHostForRepo(args.repoPath, args.connectionId) + : null + const ghOptions = args.repoPath + ? ghRepoExecOptions(githubRepoContext(args.repoPath, args.connectionId)) + : {} + const ghArgs = ['auth', 'status', ...(ghHost ? ['--hostname', ghHost] : [])] let raw = '' let ghAvailable = true try { // `gh auth status` exits non-zero when no host is logged in but still // prints the same diagnostic text we want, so capture both streams. - const { stdout, stderr } = await ghExecFileAsync(['auth', 'status']) + const { stdout, stderr } = await ghExecFileAsync(ghArgs, ghOptions) raw = `${stdout}\n${stderr}` } catch (err) { const stderr = diff --git a/src/main/github/gh-utils.test.ts b/src/main/github/gh-utils.test.ts index bc04b2bdc5c..ff633533f83 100644 --- a/src/main/github/gh-utils.test.ts +++ b/src/main/github/gh-utils.test.ts @@ -22,6 +22,7 @@ import { _resetOwnerRepoCache, classifyGhError, classifyListIssuesError, + getGitHubApiHostForRepo, getIssueOwnerRepo, getOwnerRepo, getOwnerRepoForRemote, @@ -89,6 +90,28 @@ describe('github owner/repo resolution', () => { expect(parseGitHubOwnerRepo('https://ghe.acme.internal/acme/orca.git')).toBeNull() }) + it('resolves a GitHub Enterprise host for gh api routing', async () => { + gitExecFileAsyncMock + .mockRejectedValueOnce(new Error('upstream missing')) + .mockResolvedValueOnce({ stdout: 'https://ghe.acme.internal/acme/orca.git\n' }) + + await expect(getGitHubApiHostForRepo('/repo')).resolves.toBe('ghe.acme.internal') + expect(gitExecFileAsyncMock).toHaveBeenNthCalledWith(1, ['remote', 'get-url', 'upstream'], { + cwd: '/repo' + }) + expect(gitExecFileAsyncMock).toHaveBeenNthCalledWith(2, ['remote', 'get-url', 'origin'], { + cwd: '/repo' + }) + }) + + it('does not route gh api to known non-GitHub hosts', async () => { + gitExecFileAsyncMock + .mockRejectedValueOnce(new Error('upstream missing')) + .mockResolvedValueOnce({ stdout: 'git@gitlab.com:acme/orca.git\n' }) + + await expect(getGitHubApiHostForRepo('/repo')).resolves.toBeNull() + }) + it('keeps getOwnerRepo origin-based', async () => { gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: 'git@github.com:fork/orca.git\n' diff --git a/src/main/github/gh-utils.ts b/src/main/github/gh-utils.ts index 44f25a0ed2c..c3187b0aa0c 100644 --- a/src/main/github/gh-utils.ts +++ b/src/main/github/gh-utils.ts @@ -7,6 +7,7 @@ import { gitExecFileAsync, ghExecFileAsync, extractExecError } from '../git/runn export const execFileAsync = promisify(execFile) export { ghExecFileAsync, gitExecFileAsync, extractExecError } export { classifyGhError, classifyListIssuesError } from './gh-error-classification' +export { getGitHubApiHostForRepo } from './github-api-host-resolution' export { _getOwnerRepoCacheSize, _resetOwnerRepoCache, diff --git a/src/main/github/github-api-host-resolution.ts b/src/main/github/github-api-host-resolution.ts new file mode 100644 index 00000000000..2775b04b5ed --- /dev/null +++ b/src/main/github/github-api-host-resolution.ts @@ -0,0 +1,63 @@ +import { + normalizeGitHubApiHost, + parseGitHubRemoteIdentity, + preferredGitHubApiHost +} from './github-remote-identity-parsing' +import { getRemoteUrlForRepo, githubRepoContext } from './github-repository-identity' + +// Why (issue #1715): mirror the owner/repo positive-cache window so a repo's +// resolved gh API host and its owner/repo lookups expire on the same cadence. +const REPO_HOST_CACHE_TTL_MS = 30_000 + +const repoHostCache = new Map() + +/** @internal - exposed for tests only */ +export function _resetRepoHostCache(): void { + repoHostCache.clear() +} + +// Why (issue #1715): in multi-host setups gh must target the repo's own host. +// Prefer the upstream remote's host, then origin's, so forks of a GHE repo +// route to the enterprise host rather than the user's default github.com. +export async function getGitHubApiHostForRepo( + repoPath: string, + connectionId?: string | null +): Promise { + const context = githubRepoContext(repoPath, connectionId) + const cacheKey = `${context.connectionId ?? 'local'}\0${context.repoPath}\0api-host` + const cached = repoHostCache.get(cacheKey) + if (cached && cached.expiresAt > Date.now()) { + return cached.value + } + if (cached) { + repoHostCache.delete(cacheKey) + } + + let fallback: string | null = null + for (const remoteName of ['upstream', 'origin']) { + try { + const remoteUrl = await getRemoteUrlForRepo(context, remoteName) + const identity = remoteUrl ? parseGitHubRemoteIdentity(remoteUrl) : null + const host = identity ? normalizeGitHubApiHost(identity.host) : null + if (!host) { + continue + } + if (preferredGitHubApiHost(host)) { + repoHostCache.set(cacheKey, { + value: host, + expiresAt: Date.now() + REPO_HOST_CACHE_TTL_MS + }) + return host + } + fallback ??= host + } catch { + // ignore missing remotes or non-git paths + } + } + + repoHostCache.set(cacheKey, { + value: fallback, + expiresAt: Date.now() + REPO_HOST_CACHE_TTL_MS + }) + return fallback +} diff --git a/src/main/github/github-remote-identity-parsing.ts b/src/main/github/github-remote-identity-parsing.ts index e7bd21242f1..0fd1ba39656 100644 --- a/src/main/github/github-remote-identity-parsing.ts +++ b/src/main/github/github-remote-identity-parsing.ts @@ -47,3 +47,35 @@ export function parseGitHubOwnerRepo(remoteUrl: string): GitHubOwnerRepo | null } return { owner: identity.owner, repo: identity.repo } } + +// Why (issue #1715): validate and normalize a git remote host into a `gh api +// --hostname` value. ProjectV2 is GitHub-only, so reject hosts that obviously +// belong to other providers rather than mis-routing them to a GHES API. +export function normalizeGitHubApiHost(host: string): string | null { + const normalized = host.trim().toLowerCase() + // Reject a leading dash so the host can never be mistaken for a gh CLI flag. + if (!/^[a-z0-9]([a-z0-9.-]*)?(?::[0-9]+)?$/.test(normalized)) { + return null + } + const hostname = normalized.split(':')[0] + if ( + hostname === 'gitlab.com' || + hostname.endsWith('.gitlab.com') || + hostname === 'bitbucket.org' || + hostname.endsWith('.bitbucket.org') || + hostname === 'dev.azure.com' || + hostname.endsWith('.visualstudio.com') + ) { + return null + } + return normalized +} + +// Why (issue #1715): a heuristic for which remote host most likely hosts the +// GitHub API. github.com is canonical; otherwise a host containing "github" or +// "ghe" is almost certainly a GitHub Enterprise instance and is preferred over +// an unrecognized host (which is only used as a fallback). +export function preferredGitHubApiHost(host: string): boolean { + const hostname = host.split(':')[0] + return hostname === 'github.com' || hostname.includes('github') || hostname.includes('ghe') +} diff --git a/src/main/github/github-repository-identity.ts b/src/main/github/github-repository-identity.ts index aa106972cdf..65f2e634251 100644 --- a/src/main/github/github-repository-identity.ts +++ b/src/main/github/github-repository-identity.ts @@ -8,6 +8,7 @@ import { type GitHubRemoteIdentity } from './github-remote-identity-parsing' import { isStableMissingGitRemoteError } from './stable-missing-git-remote-error' +import { _resetRepoHostCache } from './github-api-host-resolution' export type OwnerRepo = GitHubOwnerRepo @@ -66,6 +67,7 @@ const ownerRepoInFlight = new Map>() export function _resetOwnerRepoCache(): void { ownerRepoCache.clear() ownerRepoInFlight.clear() + _resetRepoHostCache() } /** @internal - exposed for tests only */ diff --git a/src/main/github/project-view-routing.test.ts b/src/main/github/project-view-routing.test.ts new file mode 100644 index 00000000000..8d18e259753 --- /dev/null +++ b/src/main/github/project-view-routing.test.ts @@ -0,0 +1,149 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { ghExecFileAsyncMock, gitExecFileAsyncMock, rateLimitGuardMock, noteRateLimitSpendMock } = + vi.hoisted(() => ({ + ghExecFileAsyncMock: vi.fn(), + gitExecFileAsyncMock: vi.fn(), + rateLimitGuardMock: vi.fn(() => ({ blocked: false })), + noteRateLimitSpendMock: vi.fn() + })) + +vi.mock('../git/runner', () => ({ + ghExecFileAsync: ghExecFileAsyncMock, + gitExecFileAsync: gitExecFileAsyncMock, + extractExecError: (err: unknown) => ({ + stderr: err && typeof err === 'object' && 'stderr' in err ? String(err.stderr) : '', + stdout: err && typeof err === 'object' && 'stdout' in err ? String(err.stdout) : '' + }) +})) + +vi.mock('./rate-limit', () => ({ + rateLimitGuard: rateLimitGuardMock, + noteRateLimitSpend: noteRateLimitSpendMock +})) + +import { _resetOwnerRepoCache } from './gh-utils' +import { + listAccessibleProjects, + updateIssueBySlug, + updateProjectItemFieldValue +} from './project-view' + +function discoveryResponses(): { stdout: string; stderr: string }[] { + return [ + { + stdout: JSON.stringify({ + data: { + viewer: { + login: 'octocat', + projectsV2: { pageInfo: { hasNextPage: false }, nodes: [] } + } + } + }), + stderr: '' + }, + { + stdout: JSON.stringify({ + data: { + viewer: { + organizations: { pageInfo: { hasNextPage: false }, nodes: [] } + } + } + }), + stderr: '' + } + ] +} + +describe('GitHub Project gh routing', () => { + beforeEach(() => { + ghExecFileAsyncMock.mockReset() + gitExecFileAsyncMock.mockReset() + rateLimitGuardMock.mockReset() + rateLimitGuardMock.mockReturnValue({ blocked: false }) + noteRateLimitSpendMock.mockReset() + _resetOwnerRepoCache() + }) + + it('passes --hostname for GitHub Enterprise repo targets', async () => { + gitExecFileAsyncMock + .mockRejectedValueOnce(new Error('upstream missing')) + .mockResolvedValueOnce({ stdout: 'https://ghe.acme.internal/acme/orca.git\n' }) + ghExecFileAsyncMock + .mockResolvedValueOnce(discoveryResponses()[0]) + .mockResolvedValueOnce(discoveryResponses()[1]) + + await expect(listAccessibleProjects({ repoPath: '/repo' })).resolves.toMatchObject({ + ok: true + }) + + expect(ghExecFileAsyncMock).toHaveBeenCalled() + for (const [argv, options] of ghExecFileAsyncMock.mock.calls) { + expect(argv).toEqual(expect.arrayContaining(['--hostname', 'ghe.acme.internal'])) + expect(options).toMatchObject({ cwd: '/repo' }) + } + }) + + it('does not pass known non-GitHub remote hosts as gh api hostnames', async () => { + gitExecFileAsyncMock + .mockRejectedValueOnce(new Error('upstream missing')) + .mockResolvedValueOnce({ stdout: 'git@gitlab.com:acme/orca.git\n' }) + ghExecFileAsyncMock + .mockResolvedValueOnce(discoveryResponses()[0]) + .mockResolvedValueOnce(discoveryResponses()[1]) + + await expect(listAccessibleProjects({ repoPath: '/repo' })).resolves.toMatchObject({ + ok: true + }) + + const firstArgv = ghExecFileAsyncMock.mock.calls[0][0] as string[] + expect(firstArgv).not.toContain('--hostname') + expect(ghExecFileAsyncMock.mock.calls[0][1]).toMatchObject({ cwd: '/repo' }) + }) + + it('passes --hostname for Project field and slug mutations', async () => { + gitExecFileAsyncMock + .mockRejectedValueOnce(new Error('upstream missing')) + .mockResolvedValueOnce({ stdout: 'https://ghe.acme.internal/acme/orca.git\n' }) + ghExecFileAsyncMock.mockResolvedValueOnce({ + stdout: JSON.stringify({ data: { updateProjectV2ItemFieldValue: {} } }), + stderr: '' + }) + + await expect( + updateProjectItemFieldValue({ + repoPath: '/repo', + projectId: 'PVT_project', + itemId: 'PVTI_item', + fieldId: 'PVTF_field', + value: { kind: 'text', text: 'Ready' } + }) + ).resolves.toEqual({ ok: true }) + + expect(ghExecFileAsyncMock.mock.calls[0][0]).toEqual( + expect.arrayContaining(['--hostname', 'ghe.acme.internal']) + ) + + ghExecFileAsyncMock.mockClear() + gitExecFileAsyncMock.mockClear() + _resetOwnerRepoCache() + gitExecFileAsyncMock + .mockRejectedValueOnce(new Error('upstream missing')) + .mockResolvedValueOnce({ stdout: 'https://ghe.acme.internal/acme/orca.git\n' }) + ghExecFileAsyncMock.mockResolvedValueOnce({ stdout: '{}', stderr: '' }) + + await expect( + updateIssueBySlug({ + repoPath: '/repo', + owner: 'acme', + repo: 'orca', + number: 12, + updates: { title: 'New title' } + }) + ).resolves.toEqual({ ok: true }) + + expect(ghExecFileAsyncMock.mock.calls[0][0]).toEqual( + expect.arrayContaining(['--hostname', 'ghe.acme.internal']) + ) + }) +}) diff --git a/src/main/github/project-view.test.ts b/src/main/github/project-view.test.ts index 00a8844bb34..c0e27597548 100644 --- a/src/main/github/project-view.test.ts +++ b/src/main/github/project-view.test.ts @@ -136,7 +136,8 @@ describe('parseProjectPaste', () => { kind: 'org', owner: 'acme', number: 42, - viewNumber: 3 + viewNumber: 3, + host: 'github.com' }) }) @@ -144,7 +145,23 @@ describe('parseProjectPaste', () => { expect(parseProjectPaste('https://github.com/users/octocat/projects/1')).toEqual({ kind: 'user', owner: 'octocat', - number: 1 + number: 1, + host: 'github.com' + }) + }) + + it('parses GitHub Enterprise project URL hosts', () => { + expect(parseProjectPaste('https://ghe/orgs/acme/projects/2')).toEqual({ + kind: 'org', + owner: 'acme', + number: 2, + host: 'ghe' + }) + expect(parseProjectPaste('https://ghe.acme.internal:8443/users/octocat/projects/3')).toEqual({ + kind: 'user', + owner: 'octocat', + number: 3, + host: 'ghe.acme.internal:8443' }) }) @@ -152,6 +169,10 @@ describe('parseProjectPaste', () => { expect(parseProjectPaste('https://github.com/orgs/co_op/projects/1')).toBeNull() }) + it('rejects URLs whose host has invalid characters', () => { + expect(parseProjectPaste('https://github.com@evil.example/orgs/acme/projects/1')).toBeNull() + }) + it('returns null for empty input', () => { expect(parseProjectPaste('')).toBeNull() expect(parseProjectPaste(' ')).toBeNull() diff --git a/src/main/github/project-view.ts b/src/main/github/project-view.ts index 64835133d71..1e3037fea88 100644 --- a/src/main/github/project-view.ts +++ b/src/main/github/project-view.ts @@ -19,6 +19,8 @@ import { isValidOwnerSlug, assertSlug, assertPositiveInt, + targetToGhApiRoute, + type GhApiRoute, type GhGraphqlErrorShape, type GraphqlVars } from './project-view/internals' @@ -41,6 +43,7 @@ import type { GitHubProjectViewError, GitHubProjectViewLayout, GitHubProjectViewSummary, + ListAccessibleProjectsArgs, ListAccessibleProjectsResult, ListProjectViewsArgs, ListProjectViewsResult, @@ -638,12 +641,15 @@ function ownerQueryRoot(ownerType: GitHubProjectOwnerType): string { return ownerType === 'organization' ? 'organization' : 'user' } -async function fetchProjectViewsPage(args: { - owner: string - ownerType: GitHubProjectOwnerType - projectNumber: number - after: string | null -}): Promise< +async function fetchProjectViewsPage( + args: { + owner: string + ownerType: GitHubProjectOwnerType + projectNumber: number + after: string | null + }, + route: GhApiRoute +): Promise< | { ok: true project: { id: string; title: string; url: string } @@ -686,7 +692,8 @@ async function fetchProjectViewsPage(args: { } const res = await runGraphql>( query, - vars + vars, + route ) if (!res.ok) { return res @@ -709,7 +716,8 @@ async function fetchProjectViewsPage(args: { async function fetchViewFieldsContinuation( viewId: string, - after: string + after: string, + route: GhApiRoute ): Promise< { ok: true; fields: RawProjectV2Field[] } | { ok: false; error: GitHubProjectViewError } > { @@ -745,7 +753,7 @@ async function fetchViewFieldsContinuation( nodes?: (RawProjectV2Field | null)[] } } | null - }>(query, { viewId, after: cursor }) + }>(query, { viewId, after: cursor }, route) if (!res.ok) { return res } @@ -847,15 +855,18 @@ type RawItemsPage = { // Why: runGraphql returns the classified error but not the raw GraphQL // errors; for the parent-field retry decision we need those. This variant // returns the raw envelope so callers can re-inspect. -async function fetchItemsPageWithRaw(args: { - owner: string - ownerType: GitHubProjectOwnerType - projectNumber: number - query: string - first: number - after: string | null - includeParent: boolean -}): Promise< +async function fetchItemsPageWithRaw( + args: { + owner: string + ownerType: GitHubProjectOwnerType + projectNumber: number + query: string + first: number + after: string | null + includeParent: boolean + }, + route: GhApiRoute +): Promise< | { ok: true; page: RawItemsPage } | { ok: false @@ -887,7 +898,13 @@ async function fetchItemsPageWithRaw(args: { } ${FIELD_CONFIG_FRAGMENT} ` - const argsArr: string[] = ['api', 'graphql', '-f', `query=${query}`] + const argsArr: string[] = [ + 'api', + ...(route.hostname ? ['--hostname', route.hostname] : []), + 'graphql', + '-f', + `query=${query}` + ] argsArr.push('-f', `owner=${args.owner}`) argsArr.push('-F', `num=${args.projectNumber}`) argsArr.push('-f', `q=${args.query}`) @@ -912,7 +929,10 @@ async function fetchItemsPageWithRaw(args: { let stderr = '' let execFailed = false try { - const r = await ghExecFileAsync(argsArr, { encoding: 'utf-8' }) + const r = await ghExecFileAsync(argsArr, { + encoding: 'utf-8', + ...(route.cwd ? { cwd: route.cwd } : {}) + }) stdout = r.stdout stderr = r.stderr } catch (err) { @@ -978,12 +998,15 @@ async function fetchItemsPageWithRaw(args: { } } -async function fetchAllItems(args: { - owner: string - ownerType: GitHubProjectOwnerType - projectNumber: number - query: string -}): Promise< +async function fetchAllItems( + args: { + owner: string + ownerType: GitHubProjectOwnerType + projectNumber: number + query: string + }, + route: GhApiRoute +): Promise< | { ok: true; rows: GitHubProjectRow[]; totalCount: number; parentFieldDropped: boolean } | { ok: false; error: GitHubProjectViewError; totalCount?: number } > { @@ -1017,15 +1040,18 @@ async function fetchAllItems(args: { parentFieldProbeInFlight.set(scopeKey, probe) probePromise = (async () => { try { - const result = await fetchItemsPageWithRaw({ - owner: args.owner, - ownerType: args.ownerType, - projectNumber: args.projectNumber, - query: args.query, - first: ITEM_PAGE_SIZE, - after: null, - includeParent: true - }) + const result = await fetchItemsPageWithRaw( + { + owner: args.owner, + ownerType: args.ownerType, + projectNumber: args.projectNumber, + query: args.query, + first: ITEM_PAGE_SIZE, + after: null, + includeParent: true + }, + route + ) // Why: flip parentFieldRetriedByOwner BEFORE resolveProbe()/clearing // parentFieldProbeInFlight so siblings that awoke on `inFlight.catch()` // observe the updated flag. Doing this in the outer block (after @@ -1043,15 +1069,18 @@ async function fetchAllItems(args: { })() first = await probePromise } else { - first = await fetchItemsPageWithRaw({ - owner: args.owner, - ownerType: args.ownerType, - projectNumber: args.projectNumber, - query: args.query, - first: ITEM_PAGE_SIZE, - after: null, - includeParent - }) + first = await fetchItemsPageWithRaw( + { + owner: args.owner, + ownerType: args.ownerType, + projectNumber: args.projectNumber, + query: args.query, + first: ITEM_PAGE_SIZE, + after: null, + includeParent + }, + route + ) } if (!first.ok && includeParent && errorsIndicateParentField(first.rawErrors, first.stderr)) { // Retry the whole table without parent. Mark this owner as retried so @@ -1066,15 +1095,18 @@ async function fetchAllItems(args: { ) markParentFieldWarningLogged(scopeKey) } - first = await fetchItemsPageWithRaw({ - owner: args.owner, - ownerType: args.ownerType, - projectNumber: args.projectNumber, - query: args.query, - first: ITEM_PAGE_SIZE, - after: null, - includeParent: false - }) + first = await fetchItemsPageWithRaw( + { + owner: args.owner, + ownerType: args.ownerType, + projectNumber: args.projectNumber, + query: args.query, + first: ITEM_PAGE_SIZE, + after: null, + includeParent: false + }, + route + ) } if (!first.ok) { return { ok: false, error: first.error } @@ -1133,15 +1165,18 @@ async function fetchAllItems(args: { } } while (hasNext) { - const next = await fetchItemsPageWithRaw({ - owner: args.owner, - ownerType: args.ownerType, - projectNumber: args.projectNumber, - query: args.query, - first: ITEM_PAGE_SIZE, - after: cursor as string, - includeParent - }) + const next = await fetchItemsPageWithRaw( + { + owner: args.owner, + ownerType: args.ownerType, + projectNumber: args.projectNumber, + query: args.query, + first: ITEM_PAGE_SIZE, + after: cursor as string, + includeParent + }, + route + ) if (!next.ok) { return { ok: false, error: next.error, totalCount } } @@ -1174,12 +1209,15 @@ async function fetchAllItems(args: { // ─── Cheap count-only query (for unsupported_layout) ────────────────── -async function fetchItemsCountOnly(args: { - owner: string - ownerType: GitHubProjectOwnerType - projectNumber: number - query: string -}): Promise { +async function fetchItemsCountOnly( + args: { + owner: string + ownerType: GitHubProjectOwnerType + projectNumber: number + query: string + }, + route: GhApiRoute +): Promise { const root = ownerQueryRoot(args.ownerType) const query = ` query($owner:String!, $num:Int!, $q:String!) { @@ -1192,7 +1230,7 @@ async function fetchItemsCountOnly(args: { ` const res = await runGraphql< Record - >(query, { owner: args.owner, num: args.projectNumber, q: args.query }) + >(query, { owner: args.owner, num: args.projectNumber, q: args.query }, route) if (!res.ok) { return null } @@ -1219,6 +1257,7 @@ export async function getProjectViewTable( error: { type: 'validation_error', message: 'Invalid ownerType.' } } } + const ghRoute = await targetToGhApiRoute(args) // Paginate views until a match is found. let cursor: string | null = null @@ -1227,12 +1266,15 @@ export async function getProjectViewTable( let matchStrength: 'id' | 'number' | 'name' | 'default' | null = null const viewsSeen: RawProjectView[] = [] while (true) { - const page = await fetchProjectViewsPage({ - owner: args.owner, - ownerType: args.ownerType, - projectNumber: args.projectNumber, - after: cursor - }) + const page = await fetchProjectViewsPage( + { + owner: args.owner, + ownerType: args.ownerType, + projectNumber: args.projectNumber, + after: cursor + }, + ghRoute + ) if (!page.ok) { return { ok: false, error: page.error } } @@ -1287,7 +1329,7 @@ export async function getProjectViewTable( let extraFields: RawProjectV2Field[] = [] const fieldsPi = selectedRaw.fields?.pageInfo if (fieldsPi?.hasNextPage === true && typeof fieldsPi.endCursor === 'string' && selectedRaw.id) { - const cont = await fetchViewFieldsContinuation(selectedRaw.id, fieldsPi.endCursor) + const cont = await fetchViewFieldsContinuation(selectedRaw.id, fieldsPi.endCursor, ghRoute) if (!cont.ok) { return { ok: false, error: cont.error } } @@ -1310,12 +1352,15 @@ export async function getProjectViewTable( // Unsupported layout: return without paginating items; attempt a cheap // count-only query best-effort. if (selectedView.layout !== 'TABLE_LAYOUT') { - const count = await fetchItemsCountOnly({ - owner: args.owner, - ownerType: args.ownerType, - projectNumber: args.projectNumber, - query: effectiveQuery - }) + const count = await fetchItemsCountOnly( + { + owner: args.owner, + ownerType: args.ownerType, + projectNumber: args.projectNumber, + query: effectiveQuery + }, + ghRoute + ) return { ok: false, error: { @@ -1327,12 +1372,15 @@ export async function getProjectViewTable( } // Fetch items. - const items = await fetchAllItems({ - owner: args.owner, - ownerType: args.ownerType, - projectNumber: args.projectNumber, - query: effectiveQuery - }) + const items = await fetchAllItems( + { + owner: args.owner, + ownerType: args.ownerType, + projectNumber: args.projectNumber, + query: effectiveQuery + }, + ghRoute + ) if (!items.ok) { return { ok: false, @@ -1386,7 +1434,10 @@ type RawViewerDiscovery = { } } -export async function listAccessibleProjects(): Promise { +export async function listAccessibleProjects( + args: ListAccessibleProjectsArgs = {} +): Promise { + const ghRoute = await targetToGhApiRoute(args) const viewerProjects: GitHubProjectSummary[] = [] const orgProjects: GitHubProjectSummary[] = [] // Why: per-org failures are collected so the picker can render a "some orgs @@ -1421,7 +1472,7 @@ export async function listAccessibleProjects(): Promise(query, vars) + const res = await runGraphql(query, vars, ghRoute) if (!res.ok) { // Why: a viewer-level failure is structural — if we can't list the // user's own projects, we have nothing to build on. Propagate as a @@ -1496,7 +1547,7 @@ export async function listAccessibleProjects(): Promise(query, vars) + const res = await runGraphql(query, vars, ghRoute) if (!res.ok) { // Why: the org-listing query itself failed (not a nested projectsV2). // Record it as a partial failure against a synthetic `*` owner so the @@ -1560,8 +1611,8 @@ export async function listAccessibleProjects(): Promise= 1 ? { viewNumber } : {}) @@ -1608,9 +1665,16 @@ export function parseProjectPaste(input: string): ParsedPaste | null { return null } +function normalizeProjectUrlHost(host: string): string | null { + const normalized = host.trim().toLowerCase() + // Reject a leading dash so a pasted host can never be mistaken for a gh flag. + return /^[a-z0-9]([a-z0-9.-]*)?(?::[0-9]+)?$/.test(normalized) ? normalized : null +} + async function resolveOwnerType( owner: string, - preferred: GitHubProjectOwnerType | null + preferred: GitHubProjectOwnerType | null, + route: GhApiRoute ): Promise< | { ok: true; ownerType: GitHubProjectOwnerType; title: string } | { ok: false; error: GitHubProjectViewError } @@ -1638,7 +1702,7 @@ async function resolveOwnerType( } const res = await runGraphql< Record - >(query, vars) + >(query, vars, route) if (!res.ok) { return { ok: false, error: res.error } } @@ -1718,8 +1782,16 @@ export async function resolveProjectRef( } const preferred: GitHubProjectOwnerType | null = parsed.kind === 'org' ? 'organization' : parsed.kind === 'user' ? 'user' : null + const baseRoute = await targetToGhApiRoute(args) + // Why: pasted project URLs carry an explicit host. Prefer that over the + // active repo hint so users can paste a GHES URL even when the active repo + // context is absent or points at another GitHub host. + const ghRoute = + parsed.kind === 'bare' + ? baseRoute + : { ...baseRoute, hostname: parsed.host.trim().toLowerCase() } // Verify by fetching project title. - const ownerRes = await resolveOwnerType(parsed.owner, preferred) + const ownerRes = await resolveOwnerType(parsed.owner, preferred, ghRoute) if (!ownerRes.ok) { return { ok: false, error: ownerRes.error } } @@ -1732,7 +1804,7 @@ export async function resolveProjectRef( ` const res = await runGraphql< Record - >(query, { owner: parsed.owner, num: parsed.number }) + >(query, { owner: parsed.owner, num: parsed.number }, ghRoute) if (!res.ok) { return { ok: false, error: res.error } } @@ -1771,15 +1843,19 @@ export async function listProjectViews( if (args.ownerType !== 'organization' && args.ownerType !== 'user') { return { ok: false, error: { type: 'validation_error', message: 'Invalid ownerType.' } } } + const ghRoute = await targetToGhApiRoute(args) const summaries: GitHubProjectViewSummary[] = [] let cursor: string | null = null while (true) { - const page = await fetchProjectViewsPage({ - owner: args.owner, - ownerType: args.ownerType, - projectNumber: args.projectNumber, - after: cursor - }) + const page = await fetchProjectViewsPage( + { + owner: args.owner, + ownerType: args.ownerType, + projectNumber: args.projectNumber, + after: cursor + }, + ghRoute + ) if (!page.ok) { return { ok: false, error: page.error } } diff --git a/src/main/github/project-view/internals.ts b/src/main/github/project-view/internals.ts index 52509c30652..0ceebfc9eb8 100644 --- a/src/main/github/project-view/internals.ts +++ b/src/main/github/project-view/internals.ts @@ -6,14 +6,51 @@ classification semantics. */ // for gh calls. The legacy plain `execFileAsync` is NOT used here — routing // every gh call through the runner gives us transient-5xx retry, WSL path // translation, and a single hook point for future quota tracking. -import { acquire, release } from '../gh-utils' +import { + acquire, + getGitHubApiHostForRepo, + ghRepoExecOptions, + githubRepoContext, + release +} from '../gh-utils' import { extractExecError, ghExecFileAsync } from '../../git/runner' import { rateLimitGuard, noteRateLimitSpend, type RateLimitBucketKind } from '../rate-limit' -import type { GitHubProjectViewError } from '../../../shared/github-project-types' +import type { GitHubProjectViewError, GitHubRepoTarget } from '../../../shared/github-project-types' export { acquire, release, extractExecError, ghExecFileAsync, rateLimitGuard, noteRateLimitSpend } export type { RateLimitBucketKind } +export type GhApiRoute = { cwd?: string; hostname?: string } + +// Why (issue #1715): `gh api graphql` does not reliably infer the API host +// from cwd for hostless GraphQL calls, so resolve the repo's git remote host +// and pass it as `gh api --hostname`. Keep cwd for placeholder expansion and +// for commands that still consult local repo context. +export async function targetToGhApiRoute( + target: GitHubRepoTarget | undefined +): Promise { + if (!target?.repoPath) { + return {} + } + const cwd = ghRepoExecOptions(githubRepoContext(target.repoPath, target.connectionId)).cwd + const hostname = await getGitHubApiHostForRepo(target.repoPath, target.connectionId) + return { + ...(cwd ? { cwd } : {}), + ...(hostname ? { hostname } : {}) + } +} + +function normalizeGhApiRoute(route?: GhApiRoute | string): GhApiRoute { + if (!route) { + return {} + } + return typeof route === 'string' ? { cwd: route } : route +} + +function ghApiArgs(route: GhApiRoute, endpoint: string): string[] { + return ['api', ...(route.hostname ? ['--hostname', route.hostname] : []), endpoint] +} + // ─── Slug validation ────────────────────────────────────────────────── // Why: GitHub usernames/org logins disallow `_`, `.`, leading `-`. Repo names @@ -269,7 +306,7 @@ export type GraphqlVars = Record export async function runGraphql( query: string, vars: GraphqlVars, - cwd?: string + route?: GhApiRoute | string ): Promise< | { ok: true; data: T } | { ok: false; error: GitHubProjectViewError; raw: { stderr: string; stdout: string } } @@ -281,7 +318,8 @@ export async function runGraphql( // Why: build argv as an array. `-f` for strings (including numbers passed // as strings), `-F` coerces to typed. We use `-f` uniformly and coerce in // the query via Int! casts, because `gh` can confuse empty strings. - const args: string[] = ['api', 'graphql', '-f', `query=${query}`] + const ghRoute = normalizeGhApiRoute(route) + const args: string[] = [...ghApiArgs(ghRoute, 'graphql'), '-f', `query=${query}`] for (const [k, v] of Object.entries(vars)) { if (typeof v === 'number' || typeof v === 'boolean') { args.push('-F', `${k}=${String(v)}`) @@ -294,7 +332,7 @@ export async function runGraphql( try { const { stdout, stderr } = await ghExecFileAsync(args, { encoding: 'utf-8', - ...(cwd ? { cwd } : {}) + ...(ghRoute.cwd ? { cwd: ghRoute.cwd } : {}) }) try { const parsed = JSON.parse(stdout) as { data?: T; errors?: GhGraphqlErrorShape[] } @@ -338,7 +376,7 @@ export async function runGraphql( export async function runRest( args: string[], - cwd?: string, + route?: GhApiRoute | string, bucket: RateLimitBucketKind = 'core', options?: { expectEmpty?: boolean } ): Promise<{ ok: true; data: T } | { ok: false; error: GitHubProjectViewError }> { @@ -348,11 +386,15 @@ export async function runRest( } await acquire() noteRateLimitSpend(bucket) + const ghRoute = normalizeGhApiRoute(route) try { - const { stdout, stderr } = await ghExecFileAsync(['api', ...args], { - encoding: 'utf-8', - ...(cwd ? { cwd } : {}) - }) + const { stdout, stderr } = await ghExecFileAsync( + [...ghApiArgs(ghRoute, args[0]), ...args.slice(1)], + { + encoding: 'utf-8', + ...(ghRoute.cwd ? { cwd: ghRoute.cwd } : {}) + } + ) // Why: 204/empty-body endpoints (DELETE label, DELETE comment) return no // body. Treat empty stdout as success rather than misclassifying the // unparseable response as 'unknown' — which the caller would otherwise diff --git a/src/main/github/project-view/mutations.test.ts b/src/main/github/project-view/mutations.test.ts index 9d8cabca7f8..683aaa3181b 100644 --- a/src/main/github/project-view/mutations.test.ts +++ b/src/main/github/project-view/mutations.test.ts @@ -21,6 +21,7 @@ vi.mock('./internals', () => ({ rateLimitedError: () => ({ type: 'rate_limited', message: 'rate limited' }), runGraphql: vi.fn(), runRest: runRestMock, + targetToGhApiRoute: vi.fn(async () => ({})), validateSlugArgs: (owner: string, repo: string) => owner && repo ? { ok: true } : { ok: false, error: { type: 'validation_error' } }, assertPositiveInt: (value: number, name: string) => diff --git a/src/main/github/project-view/mutations.ts b/src/main/github/project-view/mutations.ts index e2da3dae068..6a0e257f181 100644 --- a/src/main/github/project-view/mutations.ts +++ b/src/main/github/project-view/mutations.ts @@ -14,6 +14,7 @@ import { runRest, validateSlugArgs, assertPositiveInt, + targetToGhApiRoute, type GraphqlVars } from './internals' import type { GitHubAssignableUser, GitHubWorkItemDetails, PRComment } from '../../../shared/types' @@ -122,7 +123,8 @@ export async function updateProjectItemFieldValue( fieldId: args.fieldId, value: valVar.val } - const res = await runGraphql(query, vars) + const route = await targetToGhApiRoute(args) + const res = await runGraphql(query, vars, route) if (!res.ok) { return { ok: false, error: res.error } } @@ -144,11 +146,15 @@ export async function clearProjectItemFieldValue( }) { projectV2Item { id } } } ` - const res = await runGraphql(query, { - projectId: args.projectId, - itemId: args.itemId, - fieldId: args.fieldId - }) + const res = await runGraphql( + query, + { + projectId: args.projectId, + itemId: args.itemId, + fieldId: args.fieldId + }, + await targetToGhApiRoute(args) + ) if (!res.ok) { return { ok: false, error: res.error } } @@ -182,6 +188,7 @@ export async function updateIssueBySlug( addAssignees, removeAssignees } = args.updates + const route = await targetToGhApiRoute(args) if (duplicateOf !== undefined && (state !== 'closed' || stateReason !== 'duplicate')) { return { @@ -229,7 +236,12 @@ export async function updateIssueBySlug( } await acquire() try { - await ghExecFileAsync(stateArgs, { encoding: 'utf-8' }) + // Why (issue #1715): `gh issue close/reopen` has no --hostname flag, so + // run it with the repo's cwd to let gh infer the GHES/github.com host. + await ghExecFileAsync(stateArgs, { + encoding: 'utf-8', + ...(route.cwd ? { cwd: route.cwd } : {}) + }) } catch (err) { const { stderr, stdout } = extractExecError(err) return { ok: false, error: classifyProjectError(stderr, stdout) } @@ -247,7 +259,7 @@ export async function updateIssueBySlug( if (body !== undefined) { patchArgs.push('--raw-field', `body=${body}`) } - const r = await runRest(patchArgs) + const r = await runRest(patchArgs, route) if (!r.ok) { return { ok: false, error: r.error } } @@ -263,7 +275,7 @@ export async function updateIssueBySlug( const addCount = addLabels?.length ?? 0 if (removeCount > 1) { type RawLabelResp = { name?: string }[] - const fetched = await runRest(['-X', 'GET', `${base}/labels`]) + const fetched = await runRest(['-X', 'GET', `${base}/labels`], route) if (!fetched.ok) { return { ok: false, error: fetched.error } } @@ -281,7 +293,7 @@ export async function updateIssueBySlug( // body — GitHub does NOT interpret that as "clear labels". The // dedicated DELETE endpoint is the documented way to remove all // labels in a single call. - const r = await runRest(['-X', 'DELETE', `${base}/labels`], undefined, 'core', { + const r = await runRest(['-X', 'DELETE', `${base}/labels`], route, 'core', { expectEmpty: true }) if (!r.ok && r.error.type !== 'not_found') { @@ -292,7 +304,7 @@ export async function updateIssueBySlug( for (const name of currentNames) { putArgs.push('--raw-field', `labels[]=${name}`) } - const r = await runRest(putArgs) + const r = await runRest(putArgs, route) if (!r.ok) { return { ok: false, error: r.error } } @@ -303,7 +315,7 @@ export async function updateIssueBySlug( for (const l of addLabels ?? []) { restArgs.push('--raw-field', `labels[]=${l}`) } - const r = await runRest(restArgs) + const r = await runRest(restArgs, route) if (!r.ok) { return { ok: false, error: r.error } } @@ -311,7 +323,7 @@ export async function updateIssueBySlug( if (removeCount === 1) { const r = await runRest( ['-X', 'DELETE', `${base}/labels/${encodeURIComponent(removeLabels![0])}`], - undefined, + route, 'core', { expectEmpty: true } ) @@ -328,7 +340,7 @@ export async function updateIssueBySlug( for (const u of addAssignees) { restArgs.push('--raw-field', `assignees[]=${u}`) } - const r = await runRest(restArgs) + const r = await runRest(restArgs, route) if (!r.ok) { return { ok: false, error: r.error } } @@ -338,7 +350,7 @@ export async function updateIssueBySlug( for (const u of removeAssignees) { restArgs.push('--raw-field', `assignees[]=${u}`) } - const r = await runRest(restArgs) + const r = await runRest(restArgs, route) if (!r.ok) { return { ok: false, error: r.error } } @@ -384,7 +396,7 @@ export async function updatePullRequestBySlug( // No fields to update — nothing to do. return { ok: true } } - const r = await runRest(patchArgs) + const r = await runRest(patchArgs, await targetToGhApiRoute(args)) if (!r.ok) { return { ok: false, error: r.error } } @@ -425,13 +437,16 @@ export async function addIssueCommentBySlug( if (typeof args.body !== 'string' || !args.body.trim()) { return { ok: false, error: { type: 'validation_error', message: 'Comment body required.' } } } - const r = await runRest([ - '-X', - 'POST', - `repos/${args.owner}/${args.repo}/issues/${args.number}/comments`, - '--raw-field', - `body=${args.body}` - ]) + const r = await runRest( + [ + '-X', + 'POST', + `repos/${args.owner}/${args.repo}/issues/${args.number}/comments`, + '--raw-field', + `body=${args.body}` + ], + await targetToGhApiRoute(args) + ) if (!r.ok) { return { ok: false, error: r.error } } @@ -452,13 +467,16 @@ export async function updateIssueCommentBySlug( if (typeof args.body !== 'string' || !args.body.trim()) { return { ok: false, error: { type: 'validation_error', message: 'Comment body required.' } } } - const r = await runRest([ - '-X', - 'PATCH', - `repos/${args.owner}/${args.repo}/issues/comments/${args.commentId}`, - '--raw-field', - `body=${args.body}` - ]) + const r = await runRest( + [ + '-X', + 'PATCH', + `repos/${args.owner}/${args.repo}/issues/comments/${args.commentId}`, + '--raw-field', + `body=${args.body}` + ], + await targetToGhApiRoute(args) + ) if (!r.ok) { return { ok: false, error: r.error } } @@ -478,7 +496,7 @@ export async function deleteIssueCommentBySlug( } const r = await runRest( ['-X', 'DELETE', `repos/${args.owner}/${args.repo}/issues/comments/${args.commentId}`], - undefined, + await targetToGhApiRoute(args), 'core', { expectEmpty: true } ) @@ -501,14 +519,22 @@ export async function listLabelsBySlug( if (guard.blocked) { return { ok: false, error: rateLimitedError(guard) } } + const route = await targetToGhApiRoute(args) await acquire() // Why: `--paginate` may fan out to multiple pages; we can only reasonably // estimate a 1-call spend up front. The next probe will reconcile. noteRateLimitSpend('core') try { const { stdout } = await ghExecFileAsync( - ['api', '--paginate', `repos/${args.owner}/${args.repo}/labels`, '--jq', '.[].name'], - { encoding: 'utf-8' } + [ + 'api', + ...(route.hostname ? ['--hostname', route.hostname] : []), + '--paginate', + `repos/${args.owner}/${args.repo}/labels`, + '--jq', + '.[].name' + ], + { encoding: 'utf-8', ...(route.cwd ? { cwd: route.cwd } : {}) } ) return { ok: true, @@ -539,18 +565,20 @@ export async function listAssignableUsersBySlug( if (guard.blocked) { return { ok: false, error: rateLimitedError(guard) } } + const route = await targetToGhApiRoute(args) await acquire() noteRateLimitSpend('core') try { const { stdout } = await ghExecFileAsync( [ 'api', + ...(route.hostname ? ['--hostname', route.hostname] : []), '--paginate', `repos/${args.owner}/${args.repo}/assignees`, '--jq', '.[] | {login: .login, name: null, avatarUrl: .avatar_url}' ], - { encoding: 'utf-8' } + { encoding: 'utf-8', ...(route.cwd ? { cwd: route.cwd } : {}) } ) for (const line of stdout .trim() @@ -614,7 +642,7 @@ export async function listIssueTypesBySlug( } | null)[] } | null } | null - }>(query, { owner: args.owner, repo: args.repo }) + }>(query, { owner: args.owner, repo: args.repo }, await targetToGhApiRoute(args)) if (!res.ok) { // Why: repos without issue types respond with a GraphQL error claiming the // `issueTypes` field is unknown. Map that to an empty list so the UI shows @@ -659,7 +687,8 @@ export async function updateIssueTypeBySlug( `query($owner:String!, $repo:String!, $num:Int!) { repository(owner:$owner, name:$repo) { issue(number:$num) { id } } }`, - { owner: args.owner, repo: args.repo, num: args.number } + { owner: args.owner, repo: args.repo, num: args.number }, + await targetToGhApiRoute(args) ) if (!lookup.ok) { return { ok: false, error: lookup.error } @@ -689,7 +718,7 @@ export async function updateIssueTypeBySlug( const vars: GraphqlVars = args.issueTypeId ? { issueId, issueTypeId: args.issueTypeId } : { issueId } - const res = await runGraphql(query, vars) + const res = await runGraphql(query, vars, await targetToGhApiRoute(args)) if (!res.ok) { return { ok: false, error: res.error } } @@ -810,7 +839,11 @@ export async function getWorkItemDetailsBySlug( }) | null } | null - }>(query, { owner: args.owner, repo: args.repo, num: args.number }) + }>( + query, + { owner: args.owner, repo: args.repo, num: args.number }, + await targetToGhApiRoute(args) + ) if (!res.ok) { return { ok: false, error: res.error } } diff --git a/src/main/ipc/github.ts b/src/main/ipc/github.ts index 2a808f60325..9830fee3b29 100644 --- a/src/main/ipc/github.ts +++ b/src/main/ipc/github.ts @@ -93,6 +93,8 @@ import type { ClearProjectItemFieldArgs, DeleteIssueCommentBySlugArgs, GetProjectViewTableArgs, + GitHubRepoTarget, + ListAccessibleProjectsArgs, ListAssignableUsersBySlugArgs, ListIssueTypesBySlugArgs, ListLabelsBySlugArgs, @@ -1122,7 +1124,7 @@ export function registerGitHubHandlers(store: Store, stats: StatsCollector): voi getRateLimit(args?.force ? { force: true } : undefined) ) - ipcMain.handle('gh:diagnoseAuth', () => diagnoseGhAuth()) + ipcMain.handle('gh:diagnoseAuth', (_event, args?: GitHubRepoTarget) => diagnoseGhAuth(args)) // ── GitHub ProjectV2 view handlers ───────────────────────────────── // Why: registered unconditionally so enabling the experimental flag at @@ -1130,7 +1132,9 @@ export function registerGitHubHandlers(store: Store, stats: StatsCollector): voi // Handlers never throw across IPC — every failure mode resolves through the // GitHubProjectViewError envelope. - ipcMain.handle('gh:listAccessibleProjects', () => listAccessibleProjects()) + ipcMain.handle('gh:listAccessibleProjects', (_event, args?: ListAccessibleProjectsArgs) => + listAccessibleProjects(args) + ) ipcMain.handle('gh:resolveProjectRef', (_event, args: ResolveProjectRefArgs) => resolveProjectRef(args) diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 4c421345834..dc83b4ff472 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -525,6 +525,7 @@ import { import type { ClearProjectItemFieldArgs, GetProjectViewTableArgs, + ListAccessibleProjectsArgs, ListAssignableUsersBySlugArgs, ListIssueTypesBySlugArgs, ListLabelsBySlugArgs, @@ -11073,8 +11074,10 @@ export class OrcaRuntimeService { ) } - async listGitHubProjects(): Promise>> { - return listAccessibleProjects() + async listGitHubProjects( + args?: ListAccessibleProjectsArgs + ): Promise>> { + return listAccessibleProjects(args) } async listGitHubLabelsBySlug( diff --git a/src/main/runtime/rpc/methods/github.ts b/src/main/runtime/rpc/methods/github.ts index 6c8b8bd9995..59ef765f53d 100644 --- a/src/main/runtime/rpc/methods/github.ts +++ b/src/main/runtime/rpc/methods/github.ts @@ -45,10 +45,6 @@ const SlugRepo = z.object({ repo: requiredString('Missing repo') }) -const SlugAssignableUsers = SlugRepo.extend({ - seedLogins: z.array(z.string()).optional() -}) - const PrForBranch = RepoSelector.extend({ branch: requiredString('Missing branch'), linkedPRNumber: z.number().int().positive().nullable().optional(), @@ -198,7 +194,22 @@ const PRReviewCommentReply = RepoSelector.extend({ const ProjectOwnerType = z.enum(['organization', 'user']) -const ProjectViewTable = z.object({ +// Why (issue #1715): repo target lets the renderer hint which gh host to +// route to in multi-host setups. Both fields optional — callers without a +// repo context (e.g. paste-to-add from a foreign host) accept gh's globally +// active host. `repoPath` lets main resolve the git remote host for +// `gh api --hostname`; local repos also use it as cwd for repo placeholders. +const RepoTarget = z.object({ + repoPath: OptionalString, + connectionId: OptionalString.nullable().optional() +}) + +const RoutedSlugRepo = RepoTarget.merge(SlugRepo) +const RoutedSlugAssignableUsers = RoutedSlugRepo.extend({ + seedLogins: z.array(z.string()).optional() +}) + +const ProjectViewTable = RepoTarget.extend({ owner: requiredString('Missing owner'), ownerType: ProjectOwnerType, projectNumber: z.number().int().positive(), @@ -208,44 +219,40 @@ const ProjectViewTable = z.object({ queryOverride: OptionalString }) -const ProjectWorkItemDetailsBySlug = SlugRepo.extend({ +const ProjectWorkItemDetailsBySlug = RoutedSlugRepo.extend({ number: z.number().int().positive(), type: z.enum(['issue', 'pr']) }) -const ProjectRef = z.object({ +const ProjectRef = RepoTarget.extend({ input: requiredString('Missing project reference') }) -const ProjectViews = z.object({ +const ProjectViews = RepoTarget.extend({ owner: requiredString('Missing owner'), ownerType: ProjectOwnerType, projectNumber: z.number().int().positive() }) -const ProjectItemField = z.object({ +const ProjectItemField = RepoTarget.extend({ projectId: requiredString('Missing project ID'), itemId: requiredString('Missing item ID'), fieldId: requiredString('Missing field ID'), value: z.any() }) -const ClearProjectItemField = z.object({ +const ClearProjectItemField = RepoTarget.extend({ projectId: requiredString('Missing project ID'), itemId: requiredString('Missing item ID'), fieldId: requiredString('Missing field ID') }) -const SlugIssueUpdate = z.object({ - owner: requiredString('Missing owner'), - repo: requiredString('Missing repo'), +const SlugIssueUpdate = RoutedSlugRepo.extend({ number: z.number().int().positive(), updates: IssueUpdate }) -const SlugPullRequestUpdate = z.object({ - owner: requiredString('Missing owner'), - repo: requiredString('Missing repo'), +const SlugPullRequestUpdate = RoutedSlugRepo.extend({ number: z.number().int().positive(), updates: z.object({ state: z.enum(['open', 'closed']).optional(), @@ -254,30 +261,22 @@ const SlugPullRequestUpdate = z.object({ }) }) -const SlugIssueTypeUpdate = z.object({ - owner: requiredString('Missing owner'), - repo: requiredString('Missing repo'), +const SlugIssueTypeUpdate = RoutedSlugRepo.extend({ number: z.number().int().positive(), issueTypeId: z.string().nullable() }) -const SlugIssueComment = z.object({ - owner: requiredString('Missing owner'), - repo: requiredString('Missing repo'), +const SlugIssueComment = RoutedSlugRepo.extend({ number: z.number().int().positive(), body: requiredString('Comment body required') }) -const SlugIssueCommentEdit = z.object({ - owner: requiredString('Missing owner'), - repo: requiredString('Missing repo'), +const SlugIssueCommentEdit = RoutedSlugRepo.extend({ commentId: z.number().int().positive(), body: requiredString('Comment body required') }) -const SlugIssueCommentDelete = z.object({ - owner: requiredString('Missing owner'), - repo: requiredString('Missing repo'), +const SlugIssueCommentDelete = RoutedSlugRepo.extend({ commentId: z.number().int().positive() }) @@ -542,22 +541,22 @@ export const GITHUB_METHODS: RpcMethod[] = [ }), defineMethod({ name: 'github.project.listAccessible', - params: z.object({}), - handler: async (_params, { runtime }) => runtime.listGitHubProjects() + params: RepoTarget, + handler: async (params, { runtime }) => runtime.listGitHubProjects(params) }), defineMethod({ name: 'github.project.listLabelsBySlug', - params: SlugRepo, + params: RoutedSlugRepo, handler: async (params, { runtime }) => runtime.listGitHubLabelsBySlug(params) }), defineMethod({ name: 'github.project.listAssignableUsersBySlug', - params: SlugAssignableUsers, + params: RoutedSlugAssignableUsers, handler: async (params, { runtime }) => runtime.listGitHubAssignableUsersBySlug(params) }), defineMethod({ name: 'github.project.listIssueTypesBySlug', - params: SlugRepo, + params: RoutedSlugRepo, handler: async (params, { runtime }) => runtime.listGitHubIssueTypesBySlug(params) }), defineMethod({ diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index 7ae75a59aa3..71867482831 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -202,6 +202,8 @@ import type { GetProjectViewTableResult, GitHubProjectCommentMutationResult, GitHubProjectMutationResult, + GitHubRepoTarget, + ListAccessibleProjectsArgs, ListAccessibleProjectsResult, ListAssignableUsersBySlugArgs, ListAssignableUsersBySlugResult, @@ -1385,9 +1387,11 @@ export type PreloadApi = { * shell and silently shadows the keyring credential — in that case * `gh auth refresh` is a no-op and the UI must say so. */ - diagnoseAuth: () => Promise + diagnoseAuth: (args?: GitHubRepoTarget) => Promise // ── ProjectV2 (GitHub Projects) ───────────────────────────────── - listAccessibleProjects: () => Promise + listAccessibleProjects: ( + args?: ListAccessibleProjectsArgs + ) => Promise resolveProjectRef: (args: ResolveProjectRefArgs) => Promise listProjectViews: (args: ListProjectViewsArgs) => Promise getProjectViewTable: (args: GetProjectViewTableArgs) => Promise diff --git a/src/preload/index.ts b/src/preload/index.ts index a3adca161ba..a795be0cca6 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -90,6 +90,8 @@ import type { GetProjectViewTableResult, GitHubProjectCommentMutationResult, GitHubProjectMutationResult, + GitHubRepoTarget, + ListAccessibleProjectsArgs, ListAccessibleProjectsResult, ListAssignableUsersBySlugArgs, ListAssignableUsersBySlugResult, @@ -1247,11 +1249,14 @@ const api = { rateLimit: (args?: { force?: boolean }): Promise => ipcRenderer.invoke('gh:rateLimit', args), - diagnoseAuth: (): Promise => ipcRenderer.invoke('gh:diagnoseAuth'), + diagnoseAuth: (args?: GitHubRepoTarget): Promise => + ipcRenderer.invoke('gh:diagnoseAuth', args), // ── ProjectV2 (GitHub Projects) ─────────────────────────────────── - listAccessibleProjects: (): Promise => - ipcRenderer.invoke('gh:listAccessibleProjects'), + listAccessibleProjects: ( + args?: ListAccessibleProjectsArgs + ): Promise => + ipcRenderer.invoke('gh:listAccessibleProjects', args), resolveProjectRef: (args: ResolveProjectRefArgs): Promise => ipcRenderer.invoke('gh:resolveProjectRef', args), listProjectViews: (args: ListProjectViewsArgs): Promise => diff --git a/src/renderer/src/components/GitHubItemDialog.tsx b/src/renderer/src/components/GitHubItemDialog.tsx index 1ff788a6c95..cce115f9642 100644 --- a/src/renderer/src/components/GitHubItemDialog.tsx +++ b/src/renderer/src/components/GitHubItemDialog.tsx @@ -180,6 +180,7 @@ import { getGithubWorkItemWorkspaceAttachmentLabel } from '@/lib/github-work-item-workspace-attachment' import { startFixChecksAgent } from '@/lib/fix-checks-agent-launch' +import { activeGitHubRepoTargetFromState } from '@/lib/github-active-repo-target' import { activateAndRevealWorktree } from '@/lib/worktree-activation' import { buildFixBrokenChecksPrompt, getBrokenChecks } from '@/components/pr-checks-fix-prompt' import type { @@ -734,6 +735,25 @@ function PRReviewersPanel({ : repoOwnerSettings, [repoOwnerSettings, sourceContext] ) + // Why (issue #1715): the slug-addressed reviewer/label/assignee lookups need + // the repo-host routing hint so they resolve against the host that owns the + // dialog's repo (github.com vs GHES). + const repoForTarget = useAppStore((s) => + repoPath + ? (s.repos.find((repo) => repo.path === repoPath) ?? null) + : s.activeRepoId + ? (s.repos.find((repo) => repo.id === s.activeRepoId) ?? null) + : null + ) + const repoTarget = useMemo( + () => + repoForTarget + ? { repoPath: repoForTarget.path, connectionId: repoForTarget.connectionId ?? null } + : repoPath + ? { repoPath } + : {}, + [repoForTarget, repoPath] + ) const reviewerInputRef = useRef(null) const reviewerInputFocusFrameRef = useRef(null) const reviewerPanelMountedRef = useRef(true) @@ -808,7 +828,8 @@ function PRReviewersPanel({ open && reviewSlug ? reviewSlug.owner : null, open && reviewSlug ? reviewSlug.repo : null, reviewerSeedUsers.map((user) => user.login), - sourceSettings + sourceSettings, + repoTarget ) const reviewerMetadataByPath = useRepoAssignees( open && !reviewSlug ? repoPath : null, @@ -4891,8 +4912,10 @@ async function runIssueUpdate(args: { updates: Parameters[0]['updates'] }): Promise { if (args.projectOrigin) { + const state = useAppStore.getState() const target = getActiveRuntimeTarget(getGitHubMutationSettings(args.repoId)) const updateArgs = { + ...activeGitHubRepoTargetFromState(state), owner: args.projectOrigin.owner, repo: args.projectOrigin.repo, number: args.number, @@ -4944,8 +4967,10 @@ async function runWorkItemBodyUpdate(args: { if (!targetSlug) { throw new Error('No GitHub repository context available for this pull request.') } + const state = useAppStore.getState() const target = getActiveRuntimeTarget(getGitHubMutationSettings(args.item.repoId)) const updateArgs = { + ...activeGitHubRepoTargetFromState(state), owner: targetSlug.owner, repo: targetSlug.repo, number: args.item.number, @@ -4987,8 +5012,10 @@ async function runPullRequestStateUpdate(args: { updates: { state: 'open' | 'closed' } }): Promise { if (args.projectOrigin) { + const state = useAppStore.getState() const target = getActiveRuntimeTarget(getGitHubMutationSettings(args.repoId)) const updateArgs = { + ...activeGitHubRepoTargetFromState(state), owner: args.projectOrigin.owner, repo: args.projectOrigin.repo, number: args.number, @@ -5142,6 +5169,27 @@ function GHEditSection({ : repoOwnerSettings, [repoOwnerSettings, sourceContext] ) + // Why (issue #1715): the slug-addressed label/assignee lookups need the + // repo-host routing hint so they resolve against the host that owns the + // dialog's repo (github.com vs GHES). + const repoForTarget = useAppStore((s) => + repoId + ? (s.repos.find((repo) => repo.id === repoId) ?? null) + : repoPath + ? (s.repos.find((repo) => repo.path === repoPath) ?? null) + : s.activeRepoId + ? (s.repos.find((repo) => repo.id === s.activeRepoId) ?? null) + : null + ) + const repoTarget = useMemo( + () => + repoForTarget + ? { repoPath: repoForTarget.path, connectionId: repoForTarget.connectionId ?? null } + : repoPath + ? { repoPath } + : {}, + [repoForTarget, repoPath] + ) const { isPending, run } = useImmediateMutation() // Why: when the dialog opens from a Project view, mutations route through // *BySlug IPCs and we must keep `projectViewCache` in sync alongside @@ -5168,7 +5216,7 @@ function GHEditSection({ projectOrigin ? null : repoId, sourceSettings ) - const repoLabelsBySlug = useRepoLabelsBySlug(slugOwner, slugRepo, sourceSettings) + const repoLabelsBySlug = useRepoLabelsBySlug(slugOwner, slugRepo, sourceSettings, repoTarget) const repoLabels = projectOrigin ? repoLabelsBySlug : repoLabelsByPath const repositoryLabelsUrl = useMemo(() => getGitHubRepositoryLabelsUrl(item.url), [item.url]) const repoAssigneesByPath = useRepoAssignees( @@ -5176,7 +5224,13 @@ function GHEditSection({ projectOrigin ? null : repoId, sourceSettings ) - const repoAssigneesBySlug = useRepoAssigneesBySlug(slugOwner, slugRepo, assignees, sourceSettings) + const repoAssigneesBySlug = useRepoAssigneesBySlug( + slugOwner, + slugRepo, + assignees, + sourceSettings, + repoTarget + ) const repoAssignees = projectOrigin ? repoAssigneesBySlug : repoAssigneesByPath const hasAttachedWorkspace = attachedWorkspaceLabel !== null && attachedWorkspaceLabel !== undefined diff --git a/src/renderer/src/components/github-item-dialog-source-boundary.test.ts b/src/renderer/src/components/github-item-dialog-source-boundary.test.ts index 83c8b3cac47..101e6f906c0 100644 --- a/src/renderer/src/components/github-item-dialog-source-boundary.test.ts +++ b/src/renderer/src/components/github-item-dialog-source-boundary.test.ts @@ -44,7 +44,11 @@ describe('GitHubItemDialog source host boundaries', () => { expect(section).toContain('getTaskSourceRuntimeSettings(sourceContext)') expect(section).toContain('useRepoLabels(') - expect(section).toContain('useRepoLabelsBySlug(slugOwner, slugRepo, sourceSettings)') + // Why (issue #1715): the slug lookups gained a trailing repoTarget arg for + // multi-host gh routing; sourceSettings must still be the source-routing arg. + expect(section).toContain( + 'useRepoLabelsBySlug(slugOwner, slugRepo, sourceSettings, repoTarget)' + ) expect(section).toContain('useRepoAssignees(') expect(section).toContain('useRepoAssigneesBySlug(') expect(section).toContain('sourceSettings') diff --git a/src/renderer/src/components/github-project/GhAuthErrorHelp.tsx b/src/renderer/src/components/github-project/GhAuthErrorHelp.tsx index 9131094bdc7..562a8780714 100644 --- a/src/renderer/src/components/github-project/GhAuthErrorHelp.tsx +++ b/src/renderer/src/components/github-project/GhAuthErrorHelp.tsx @@ -8,11 +8,12 @@ * a one-shot diagnostic and rewrites the suggested fix to match what gh * is actually doing: env-shadow vs. plain missing-scope vs. not-installed. */ -import { useEffect, useState } from 'react' +import { useEffect, useMemo, useState } from 'react' import { Copy, ExternalLink, RotateCw } from 'lucide-react' import { toast } from 'sonner' import { Button } from '@/components/ui/button' -import type { GitHubProjectViewError } from '@/../../shared/github-project-types' +import { useAppStore } from '@/store' +import type { GitHubProjectViewError, GitHubRepoTarget } from '@/../../shared/github-project-types' import type { GhAuthDiagnostic } from '@/../../shared/github-auth-types' import { translate } from '@/i18n/i18n' @@ -248,10 +249,29 @@ export function GhAuthErrorHelp({ variant?: 'block' | 'banner' }): React.JSX.Element { const [diag, setDiag] = useState(null) + // Why (issue #1715): pass the active repo as a host hint so `gh auth status` + // reads scope/auth info for the host that owns the repo instead of gh's + // globally-active host. Without this, multi-host setups (github.com + GHES) + // see remediation tailored to the wrong host's account. + const activeRepoId = useAppStore((s) => s.activeRepoId) + const repos = useAppStore((s) => s.repos) + const activeRepoTarget = useMemo(() => { + const repo = activeRepoId ? repos.find((r) => r.id === activeRepoId) : null + if (!repo) { + return {} + } + return { repoPath: repo.path, connectionId: repo.connectionId ?? null } + }, [activeRepoId, repos]) + const repoPath = activeRepoTarget.repoPath + const connectionId = activeRepoTarget.connectionId useEffect(() => { let cancelled = false + const target: GitHubRepoTarget = { + ...(repoPath ? { repoPath } : {}), + ...(connectionId !== undefined ? { connectionId } : {}) + } window.api.gh - .diagnoseAuth() + .diagnoseAuth(target) .then((d) => { if (!cancelled) { setDiag(d) @@ -262,7 +282,7 @@ export function GhAuthErrorHelp({ return () => { cancelled = true } - }, []) + }, [repoPath, connectionId]) const remedy = buildRemediation(error.message, error.type, diag) const docsUrl = remedy.docsUrl diff --git a/src/renderer/src/components/github-project/ProjectCell.tsx b/src/renderer/src/components/github-project/ProjectCell.tsx index b9642d0c2e8..85ca4983284 100644 --- a/src/renderer/src/components/github-project/ProjectCell.tsx +++ b/src/renderer/src/components/github-project/ProjectCell.tsx @@ -16,6 +16,7 @@ import { useAppStore } from '@/store' import { callRuntimeRpc, getActiveRuntimeTarget } from '@/runtime/runtime-rpc-client' import { useRepoSlugIndex } from '@/lib/repo-slug-index' import { getSettingsForRepoRuntimeOwner } from '@/lib/repo-runtime-owner' +import { activeGitHubRepoTargetFromState } from '@/lib/github-active-repo-target' import type { GitHubIssueType, GitHubProjectField, @@ -323,6 +324,15 @@ function IssueTypeCell({ const [open, setOpen] = useState(false) const [options, setOptions] = useState([]) const [loading, setLoading] = useState(false) + // Why (issue #1715): route the issue-type lookup to the host that owns the + // active repo. Memoized on primitive ids so a fresh target per render + // doesn't refire the effect. + const activeRepoId = useAppStore((s) => s.activeRepoId) + const repos = useAppStore((s) => s.repos) + const repoTarget = React.useMemo( + () => activeGitHubRepoTargetFromState({ activeRepoId, repos }), + [activeRepoId, repos] + ) const [owner, repo] = (row.content.repository ?? '').split('/') const { lookupSlug } = useRepoSlugIndex() const matchedRepo = useMemo( @@ -340,15 +350,16 @@ function IssueTypeCell({ let cancelled = false setLoading(true) const target = getActiveRuntimeTarget(matchedRepo ? ownerSettings : sourceSettings) + const args = { ...repoTarget, owner, repo } const request = target.kind === 'environment' ? callRuntimeRpc( target, 'github.project.listIssueTypesBySlug', - { owner, repo }, + args, { timeoutMs: 30_000 } ) - : window.api.gh.listIssueTypesBySlug({ owner, repo }) + : window.api.gh.listIssueTypesBySlug(args) request .then((res) => { if (cancelled) { @@ -366,7 +377,7 @@ function IssueTypeCell({ return () => { cancelled = true } - }, [matchedRepo, open, owner, ownerSettings, repo, sourceSettings]) + }, [matchedRepo, open, owner, ownerSettings, repo, sourceSettings, repoTarget]) const trigger = ( @@ -828,6 +839,14 @@ function AssigneesCell({ }): React.JSX.Element { const assignees = row.content.assignees const [open, setOpen] = useState(false) + // Why (issue #1715): route the assignee lookup to the host that owns the + // active repo. Memoized on primitive ids to keep the IPC stable. + const activeRepoId = useAppStore((s) => s.activeRepoId) + const repos = useAppStore((s) => s.repos) + const repoTarget = React.useMemo( + () => activeGitHubRepoTargetFromState({ activeRepoId, repos }), + [activeRepoId, repos] + ) const [owner, repo] = (row.content.repository ?? '').split('/') @@ -849,7 +868,8 @@ function AssigneesCell({ open ? owner : null, open ? repo : null, seedKey ? seedKey.split(',') : [], - sourceSettings + sourceSettings, + repoTarget ) const labelContent = @@ -943,9 +963,22 @@ function LabelsCell({ }): React.JSX.Element { const labels = row.content.labels const [open, setOpen] = useState(false) + // Why (issue #1715): route the label lookup to the host that owns the + // active repo. Memoized on primitive ids to keep the IPC stable. + const activeRepoId = useAppStore((s) => s.activeRepoId) + const repos = useAppStore((s) => s.repos) + const repoTarget = React.useMemo( + () => activeGitHubRepoTargetFromState({ activeRepoId, repos }), + [activeRepoId, repos] + ) const [owner, repo] = (row.content.repository ?? '').split('/') - const metadata = useRepoLabelsBySlug(open ? owner : null, open ? repo : null, sourceSettings) + const metadata = useRepoLabelsBySlug( + open ? owner : null, + open ? repo : null, + sourceSettings, + repoTarget + ) const labelContent = labels.length === 0 ? null : labels.map((l) => ) diff --git a/src/renderer/src/components/github-project/ProjectPicker.tsx b/src/renderer/src/components/github-project/ProjectPicker.tsx index de2f2710dd2..50f832293c3 100644 --- a/src/renderer/src/components/github-project/ProjectPicker.tsx +++ b/src/renderer/src/components/github-project/ProjectPicker.tsx @@ -20,6 +20,7 @@ import type { GitHubProjectSummary, GitHubProjectViewError, GitHubProjectViewSummary, + GitHubRepoTarget, ListAccessibleProjectsResult, ListProjectViewsResult, ResolveProjectRefResult @@ -47,6 +48,12 @@ type Props = { title?: string } | null onSelect: (selection: ResolvedProjectSelection) => void + // Why (issue #1715): repo target hint so the picker's gh calls land on the + // right host in multi-host setups (e.g. github.com + GHES). The wrapping + // ProjectViewWrapper derives this from the active worktree's owning repo; + // main resolves the gh API host from that repo's git remote. Omit both to + // accept gh's globally-active host (legitimate when no repo context exists). + repoTarget?: GitHubRepoTarget } const BROWSE_CACHE_TTL_MS = 5 * 60_000 @@ -56,6 +63,9 @@ type BrowseCacheEntry = { partialFailures?: { owner: string; message: string }[] } +// Why (issue #1715): the cache is keyed by both the runtime scope AND the +// repo-target routing key so a GHES vs github.com switch (or runtime switch) +// can't serve projects discovered against the wrong host. const browseCacheByRuntimeScope = new Map() function getProjectPickerRuntimeScope( @@ -65,51 +75,72 @@ function getProjectPickerRuntimeScope( return target.kind === 'environment' ? `runtime:${target.environmentId}` : 'local' } +function repoTargetCacheKey(repoTarget: GitHubRepoTarget): string { + return `${repoTarget.connectionId ?? 'local'}:${repoTarget.repoPath ?? ''}` +} + async function listAccessibleProjectsForRuntime( - settings: Parameters[0] + settings: Parameters[0], + repoTarget: GitHubRepoTarget ): Promise { const target = getActiveRuntimeTarget(settings) return target.kind === 'environment' ? callRuntimeRpc( target, 'github.project.listAccessible', - {}, + repoTarget, { timeoutMs: 60_000 } ) - : window.api.gh.listAccessibleProjects() + : window.api.gh.listAccessibleProjects(repoTarget) } async function listProjectViewsForRuntime( settings: Parameters[0], - args: { owner: string; ownerType: GitHubProjectOwnerType; projectNumber: number } + args: { owner: string; ownerType: GitHubProjectOwnerType; projectNumber: number }, + repoTarget: GitHubRepoTarget ): Promise { const target = getActiveRuntimeTarget(settings) + const payload = { ...args, ...repoTarget } return target.kind === 'environment' - ? callRuntimeRpc(target, 'github.project.listViews', args, { + ? callRuntimeRpc(target, 'github.project.listViews', payload, { timeoutMs: 30_000 }) - : window.api.gh.listProjectViews(args) + : window.api.gh.listProjectViews(payload) } async function resolveProjectRefForRuntime( settings: Parameters[0], - input: string + input: string, + repoTarget: GitHubRepoTarget ): Promise { const target = getActiveRuntimeTarget(settings) + const payload = { input, ...repoTarget } return target.kind === 'environment' - ? callRuntimeRpc( - target, - 'github.project.resolveRef', - { input }, - { timeoutMs: 30_000 } - ) - : window.api.gh.resolveProjectRef({ input }) + ? callRuntimeRpc(target, 'github.project.resolveRef', payload, { + timeoutMs: 30_000 + }) + : window.api.gh.resolveProjectRef(payload) } -export default function ProjectPicker({ activeProject, onSelect }: Props): React.JSX.Element { +export default function ProjectPicker({ + activeProject, + onSelect, + repoTarget +}: Props): React.JSX.Element { const settings = useAppStore((s) => s.settings) const updateSettings = useAppStore((s) => s.updateSettings) const mountedRef = useMountedRef() + // Why (issue #1715): stable wire payload for gh routing. Memoized on the + // primitive fields so a new object identity on every render doesn't bust + // useCallback caches (loadBrowse, handleChooseProject, handlePaste). + const ghRepoTarget = useMemo( + () => ({ + ...(repoTarget?.repoPath ? { repoPath: repoTarget.repoPath } : {}), + ...(repoTarget?.connectionId !== undefined ? { connectionId: repoTarget.connectionId } : {}) + }), + [repoTarget?.repoPath, repoTarget?.connectionId] + ) + const ghRepoTargetKey = useMemo(() => repoTargetCacheKey(ghRepoTarget), [ghRepoTarget]) const projectSettings: GitHubProjectSettings = useMemo( () => settings?.githubProjects ?? { @@ -125,7 +156,8 @@ export default function ProjectPicker({ activeProject, onSelect }: Props): React const [query, setQuery] = useState('') const [browseLoading, setBrowseLoading] = useState(false) const [browseError, setBrowseError] = useState(null) - const browseCache = browseCacheByRuntimeScope.get(getProjectPickerRuntimeScope(settings)) + const browseCacheKey = `${getProjectPickerRuntimeScope(settings)}:${ghRepoTargetKey}` + const browseCache = browseCacheByRuntimeScope.get(browseCacheKey) const [browseProjects, setBrowseProjects] = useState( () => browseCache?.projects ?? [] ) @@ -146,7 +178,7 @@ export default function ProjectPicker({ activeProject, onSelect }: Props): React const [viewLoading, setViewLoading] = useState(false) const loadBrowse = useCallback(async () => { - const cacheKey = getProjectPickerRuntimeScope(settings) + const cacheKey = `${getProjectPickerRuntimeScope(settings)}:${ghRepoTargetKey}` const cached = browseCacheByRuntimeScope.get(cacheKey) ?? null if (cached && Date.now() - cached.fetchedAt < BROWSE_CACHE_TTL_MS) { setBrowseProjects(cached.projects) @@ -156,7 +188,7 @@ export default function ProjectPicker({ activeProject, onSelect }: Props): React setBrowseLoading(true) setBrowseError(null) try { - const res = await listAccessibleProjectsForRuntime(settings) + const res = await listAccessibleProjectsForRuntime(settings, ghRepoTarget) if (res.ok) { browseCacheByRuntimeScope.set(cacheKey, { fetchedAt: Date.now(), @@ -186,7 +218,21 @@ export default function ProjectPicker({ activeProject, onSelect }: Props): React setBrowseLoading(false) } } - }, [mountedRef, settings]) + }, [mountedRef, settings, ghRepoTarget, ghRepoTargetKey]) + + // Why (issue #1715): when the repo-target routing key changes (repo switch), + // rehydrate from the matching cache entry or clear so stale projects from + // the previous host don't linger. + useEffect(() => { + const cached = browseCacheByRuntimeScope.get(browseCacheKey) + if (cached) { + setBrowseProjects(cached.projects) + setPartialFailures(cached.partialFailures ?? []) + return + } + setBrowseProjects([]) + setPartialFailures([]) + }, [browseCacheKey]) useEffect(() => { if (open && !viewPickFor) { @@ -282,11 +328,15 @@ export default function ProjectPicker({ activeProject, onSelect }: Props): React }) setViewLoading(true) try { - const res = await listProjectViewsForRuntime(settings, { - owner: selection.owner, - ownerType: selection.ownerType, - projectNumber: selection.number - }) + const res = await listProjectViewsForRuntime( + settings, + { + owner: selection.owner, + ownerType: selection.ownerType, + projectNumber: selection.number + }, + ghRepoTarget + ) if (!mountedRef.current) { return } @@ -336,7 +386,7 @@ export default function ProjectPicker({ activeProject, onSelect }: Props): React } } }, - [commitSelection, mountedRef, projectSettings.lastViewByProject, settings] + [commitSelection, mountedRef, projectSettings.lastViewByProject, settings, ghRepoTarget] ) const handlePaste = useCallback(async () => { @@ -353,7 +403,7 @@ export default function ProjectPicker({ activeProject, onSelect }: Props): React setPasteError(null) setPasteBusy(true) try { - const res = await resolveProjectRefForRuntime(settings, input) + const res = await resolveProjectRefForRuntime(settings, input, ghRepoTarget) if (!mountedRef.current) { return } @@ -376,7 +426,7 @@ export default function ProjectPicker({ activeProject, onSelect }: Props): React setPasteBusy(false) } } - }, [handleChooseProject, mountedRef, pasteInput, settings]) + }, [handleChooseProject, mountedRef, pasteInput, settings, ghRepoTarget]) const canSubmitPasteInput = !pasteBusy && hasBoundedGitHubProjectRefInputText(pasteInput) @@ -822,7 +872,10 @@ function parseProjectInput( } try { const url = new URL(input) - if (url.hostname !== 'github.com') { + // Why (issue #1715): accept any host so GHES project URLs parse. The host + // itself is re-parsed by the main process resolver for explicit host + // routing. Require at least one dot to reject obvious garbage hosts. + if (!url.hostname.includes('.')) { return null } const parts = url.pathname.split('/').filter(Boolean) diff --git a/src/renderer/src/components/github-project/ProjectViewWrapper.tsx b/src/renderer/src/components/github-project/ProjectViewWrapper.tsx index f9c8a4ceee7..82b7ebb5491 100644 --- a/src/renderer/src/components/github-project/ProjectViewWrapper.tsx +++ b/src/renderer/src/components/github-project/ProjectViewWrapper.tsx @@ -41,6 +41,7 @@ import type { GitHubProjectTable, GitHubProjectViewError, GitHubProjectViewSummary, + GitHubRepoTarget, ListProjectViewsResult } from '../../../../shared/github-project-types' import type { GitHubWorkItem } from '../../../../shared/types' @@ -71,14 +72,18 @@ const ORCA_FEATURE_REQUEST_URL = 'https://github.com/stablyai/orca/issues/new' function listProjectViewsForRuntime( settings: Parameters[0], - args: { owner: string; ownerType: 'organization' | 'user'; projectNumber: number } + args: { owner: string; ownerType: 'organization' | 'user'; projectNumber: number }, + repoTarget: GitHubRepoTarget ): Promise { const target = getActiveRuntimeTarget(settings) + // Why (issue #1715): merge the repo-routing hint into the IPC/RPC payload + // so listProjectViews lands on the host that owns the active repo. + const payload = { ...repoTarget, ...args } return target.kind === 'environment' - ? callRuntimeRpc(target, 'github.project.listViews', args, { + ? callRuntimeRpc(target, 'github.project.listViews', payload, { timeoutMs: 30_000 }) - : window.api.gh.listProjectViews(args) + : window.api.gh.listProjectViews(payload) } function getProjectViewSourceScope(settings: Parameters[0]): string { @@ -96,11 +101,26 @@ export default function ProjectViewWrapper({ selectedRepoIds }: Props): React.JS const patchProjectRowIssueType = useAppStore((s) => s.patchProjectRowIssueType) const addRepoFromStore = useAppStore((s) => s.addRepo) const repos = useAppStore((s) => s.repos) + const activeRepoId = useAppStore((s) => s.activeRepoId) const { lookupSlug, ready: slugIndexReady } = useRepoSlugIndex() const mountedRef = useMountedRef() const activeProject = settings?.githubProjects?.activeProject ?? null const projectViewSourceScope = useMemo(() => getProjectViewSourceScope(settings), [settings]) + // Why (issue #1715): pass the active repo as a gh-host hint to all + // project-related gh calls (picker, project view fetch, mutations). gh + // resolves the API host from that repo's git remote; without this, GHES + // users hit github.com and see misleading scope errors. + const activeRepoTarget = useMemo(() => { + const repo = activeRepoId ? repos.find((r) => r.id === activeRepoId) : null + if (!repo) { + return {} + } + return { + repoPath: repo.path, + connectionId: repo.connectionId ?? null + } + }, [activeRepoId, repos]) const lastViewByProject = useMemo( () => settings?.githubProjects?.lastViewByProject ?? {}, [settings?.githubProjects?.lastViewByProject] @@ -192,7 +212,8 @@ export default function ProjectViewWrapper({ selectedRepoIds }: Props): React.JS activeProject.number, viewId, queryOverride, - projectViewSourceScope + projectViewSourceScope, + activeRepoTarget ) if (projectViewCache[cacheKey]?.data) { return @@ -213,7 +234,8 @@ export default function ProjectViewWrapper({ selectedRepoIds }: Props): React.JS projectViewCache, doFetch, appliedQueryByView, - projectViewSourceScope + projectViewSourceScope, + activeRepoTarget ]) // Load the project's view list whenever the active project changes so the @@ -228,11 +250,15 @@ export default function ProjectViewWrapper({ selectedRepoIds }: Props): React.JS return } let cancelled = false - void listProjectViewsForRuntime(settings, { - owner: activeProject.owner, - ownerType: activeProject.ownerType, - projectNumber: activeProject.number - }) + void listProjectViewsForRuntime( + settings, + { + owner: activeProject.owner, + ownerType: activeProject.ownerType, + projectNumber: activeProject.number + }, + activeRepoTarget + ) .then((res) => { if (cancelled) { return @@ -254,7 +280,7 @@ export default function ProjectViewWrapper({ selectedRepoIds }: Props): React.JS return () => { cancelled = true } - }, [activeProject, viewListByProject, settings, projectViewSourceScope]) + }, [activeProject, viewListByProject, settings, projectViewSourceScope, activeRepoTarget]) const handleSwitchView = useCallback( async (viewId: string) => { @@ -328,9 +354,16 @@ export default function ProjectViewWrapper({ selectedRepoIds }: Props): React.JS activeProject.number, viewId, currentAppliedOverride, - projectViewSourceScope + projectViewSourceScope, + activeRepoTarget ) - }, [activeProject, lastViewByProject, currentAppliedOverride, projectViewSourceScope]) + }, [ + activeProject, + lastViewByProject, + currentAppliedOverride, + projectViewSourceScope, + activeRepoTarget + ]) const table: GitHubProjectTable | null = currentCacheKey ? (projectViewCache[currentCacheKey]?.data ?? null) @@ -764,6 +797,7 @@ export default function ProjectViewWrapper({ selectedRepoIds }: Props): React.JS : null } onSelect={handleSelect} + repoTarget={activeRepoTarget} /> {currentProjectViewKey ? ( // Why: render the search input whenever a view is selected — even diff --git a/src/renderer/src/components/github-project/slug-dialog/AssigneesEditor.tsx b/src/renderer/src/components/github-project/slug-dialog/AssigneesEditor.tsx index ded043e3d68..317f98f3ced 100644 --- a/src/renderer/src/components/github-project/slug-dialog/AssigneesEditor.tsx +++ b/src/renderer/src/components/github-project/slug-dialog/AssigneesEditor.tsx @@ -2,6 +2,8 @@ import React, { useMemo, useState } from 'react' import { Popover, PopoverContent, PopoverTrigger } from '@/components/ui/popover' import { cn } from '@/lib/utils' import { useRepoAssigneesBySlug } from '@/hooks/useGitHubSlugMetadata' +import { useAppStore } from '@/store' +import { activeGitHubRepoTargetFromState } from '@/lib/github-active-repo-target' import type { GlobalSettings } from '../../../../../shared/types' import { translate } from '@/i18n/i18n' @@ -21,6 +23,16 @@ export function AssigneesEditor({ onChange: (add: string[], remove: string[]) => void | Promise }): React.JSX.Element { const [open, setOpen] = useState(false) + // Why (issue #1715): route the assignee lookup to the host that owns the + // active repo so GHES projects don't fall through to gh's global host. + // Memoized on the primitive id so a fresh target object per render doesn't + // refire the metadata IPC. + const activeRepoId = useAppStore((s) => s.activeRepoId) + const repos = useAppStore((s) => s.repos) + const repoTarget = useMemo( + () => activeGitHubRepoTargetFromState({ activeRepoId, repos }), + [activeRepoId, repos] + ) // Why: stabilize the assignee seed identity. `selected` is a fresh array on // every parent render — depending on it directly would refire the IPC for // every unrelated re-render while the popover is open. @@ -29,7 +41,8 @@ export function AssigneesEditor({ open ? owner : null, open ? repo : null, seedKey ? seedKey.split(',') : [], - sourceSettings + sourceSettings, + repoTarget ) return ( !disabled && setOpen(o)}> diff --git a/src/renderer/src/components/github-project/slug-dialog/Comments.tsx b/src/renderer/src/components/github-project/slug-dialog/Comments.tsx index 95470618a81..979181d5b51 100644 --- a/src/renderer/src/components/github-project/slug-dialog/Comments.tsx +++ b/src/renderer/src/components/github-project/slug-dialog/Comments.tsx @@ -14,6 +14,7 @@ import { useRepoSlugIndex } from '@/lib/repo-slug-index' import { getSettingsForRepoRuntimeOwner } from '@/lib/repo-runtime-owner' import type { GlobalSettings, PRComment } from '../../../../../shared/types' import type { + GitHubRepoTarget, GitHubProjectCommentMutationResult, GitHubProjectMutationResult } from '../../../../../shared/github-project-types' @@ -37,6 +38,21 @@ function useRuntimeSettingsForSlug(owner: string, repo: string) { ) } +// Why (issue #1715): the comment mutations also need the repo-host routing +// hint so they land on the host that owns the active repo. +function useActiveRepoTarget(): GitHubRepoTarget { + const activeRepo = useAppStore((s) => + s.activeRepoId ? (s.repos.find((repo) => repo.id === s.activeRepoId) ?? null) : null + ) + return useMemo( + () => + activeRepo + ? { repoPath: activeRepo.path, connectionId: activeRepo.connectionId ?? null } + : {}, + [activeRepo] + ) +} + export function CommentsList({ owner, repo, @@ -52,6 +68,7 @@ export function CommentsList({ }): React.JSX.Element { const fallbackRuntimeSettings = useRuntimeSettingsForSlug(owner, repo) const runtimeSettings = sourceSettings ?? fallbackRuntimeSettings + const repoTarget = useActiveRepoTarget() return (
{comments.length === 0 ? ( @@ -71,6 +88,7 @@ export function CommentsList({ onDelete={async () => { const target = getRuntimeTarget(runtimeSettings) const args = { + ...repoTarget, owner, repo, commentId: c.id @@ -92,6 +110,7 @@ export function CommentsList({ onEdit={async (next) => { const target = getRuntimeTarget(runtimeSettings) const args = { + ...repoTarget, owner, repo, commentId: c.id, @@ -201,6 +220,7 @@ export function NewCommentForm({ const [submitting, setSubmitting] = useState(false) const fallbackRuntimeSettings = useRuntimeSettingsForSlug(owner, repo) const runtimeSettings = sourceSettings ?? fallbackRuntimeSettings + const repoTarget = useActiveRepoTarget() const canSubmitComment = hasBoundedCommentBodyText(draft) return (
@@ -234,7 +254,7 @@ export function NewCommentForm({ setSubmitting(true) try { const target = getRuntimeTarget(runtimeSettings) - const args = { owner, repo, number, body: bodyState.body } + const args = { ...repoTarget, owner, repo, number, body: bodyState.body } const res = target ? await callRuntimeRpc( target, diff --git a/src/renderer/src/components/github-project/slug-dialog/LabelsEditor.tsx b/src/renderer/src/components/github-project/slug-dialog/LabelsEditor.tsx index 1c87fc3fd28..eee717cfe79 100644 --- a/src/renderer/src/components/github-project/slug-dialog/LabelsEditor.tsx +++ b/src/renderer/src/components/github-project/slug-dialog/LabelsEditor.tsx @@ -1,7 +1,9 @@ -import React, { useState } from 'react' +import React, { useMemo, useState } from 'react' import { Popover, PopoverContent, PopoverTrigger } from '@/components/ui/popover' import { cn } from '@/lib/utils' import { useRepoLabelsBySlug } from '@/hooks/useGitHubSlugMetadata' +import { useAppStore } from '@/store' +import { activeGitHubRepoTargetFromState } from '@/lib/github-active-repo-target' import type { GlobalSettings } from '../../../../../shared/types' import { translate } from '@/i18n/i18n' @@ -21,7 +23,21 @@ export function LabelsEditor({ onChange: (add: string[], remove: string[]) => void | Promise }): React.JSX.Element { const [open, setOpen] = useState(false) - const metadata = useRepoLabelsBySlug(open ? owner : null, open ? repo : null, sourceSettings) + // Why (issue #1715): route the label lookup to the host that owns the active + // repo. Memoized on the primitive id so a fresh target object per render + // doesn't refire the metadata IPC. + const activeRepoId = useAppStore((s) => s.activeRepoId) + const repos = useAppStore((s) => s.repos) + const repoTarget = useMemo( + () => activeGitHubRepoTargetFromState({ activeRepoId, repos }), + [activeRepoId, repos] + ) + const metadata = useRepoLabelsBySlug( + open ? owner : null, + open ? repo : null, + sourceSettings, + repoTarget + ) return ( !disabled && setOpen(o)}> diff --git a/src/renderer/src/components/github-project/slug-dialog/SlugDialogBody.tsx b/src/renderer/src/components/github-project/slug-dialog/SlugDialogBody.tsx index ad7f9b5eb62..d97d1ec6116 100644 --- a/src/renderer/src/components/github-project/slug-dialog/SlugDialogBody.tsx +++ b/src/renderer/src/components/github-project/slug-dialog/SlugDialogBody.tsx @@ -27,6 +27,16 @@ export function SlugDialogBody({ const { owner, repo, number, type, cacheKey } = projectOrigin const patchProjectIssueOrPr = useAppStore((s) => s.patchProjectIssueOrPr) const projectViewCache = useAppStore((s) => s.projectViewCache) + const activeRepo = useAppStore((s) => + s.activeRepoId ? (s.repos.find((candidate) => candidate.id === s.activeRepoId) ?? null) : null + ) + const repoTarget = useMemo( + () => + activeRepo + ? { repoPath: activeRepo.path, connectionId: activeRepo.connectionId ?? null } + : {}, + [activeRepo] + ) // Why: the Project row is the source of truth for the list-side columns; // reading it reactively here keeps the dialog in sync with optimistic @@ -57,6 +67,9 @@ export function SlugDialogBody({ setError(null) setDetails(null) const target = getActiveRuntimeTarget(sourceSettings) + // Why (issue #1715): thread the repo-host routing hint through both the + // runtime RPC and the local IPC so the details lookup lands on the host + // that owns the active repo (github.com vs GHES). const request = target.kind === 'environment' ? callRuntimeRpc< @@ -64,10 +77,10 @@ export function SlugDialogBody({ >( target, 'github.project.workItemDetailsBySlug', - { owner, repo, number, type }, + { ...repoTarget, owner, repo, number, type }, { timeoutMs: 30_000 } ) - : window.api.gh.projectWorkItemDetailsBySlug({ owner, repo, number, type }) + : window.api.gh.projectWorkItemDetailsBySlug({ ...repoTarget, owner, repo, number, type }) request .then((res) => { if (rid !== requestIdRef.current) { @@ -91,7 +104,7 @@ export function SlugDialogBody({ } setLoading(false) }) - }, [owner, repo, number, type, sourceSettings]) + }, [owner, repo, number, type, sourceSettings, repoTarget]) const title = row?.content.title ?? details?.item.title ?? '' const url = row?.content.url ?? details?.item.url ?? null diff --git a/src/renderer/src/hooks/useGitHubSlugMetadata.ts b/src/renderer/src/hooks/useGitHubSlugMetadata.ts index 26022191fcc..12265b7f5fc 100644 --- a/src/renderer/src/hooks/useGitHubSlugMetadata.ts +++ b/src/renderer/src/hooks/useGitHubSlugMetadata.ts @@ -9,6 +9,7 @@ import { useEffect, useRef, useState } from 'react' import { callRuntimeRpc, getActiveRuntimeTarget } from '@/runtime/runtime-rpc-client' import type { GitHubAssignableUser, GlobalSettings } from '../../../shared/types' import type { + GitHubRepoTarget, ListAssignableUsersBySlugResult, ListLabelsBySlugResult } from '../../../shared/github-project-types' @@ -33,10 +34,15 @@ export function clearGitHubSlugMetadataCache(): void { clearMetadataRequestStore(slugAssigneeStore) } +function repoTargetKey(repoTarget?: GitHubRepoTarget): string { + return `${repoTarget?.connectionId ?? 'local'}:${repoTarget?.repoPath ?? ''}` +} + export function useRepoLabelsBySlug( owner: string | null, repo: string | null, - settings?: Pick | null + settings?: Pick | null, + repoTarget?: GitHubRepoTarget ): MetadataState { const [state, setState] = useState>({ data: [], @@ -50,10 +56,11 @@ export function useRepoLabelsBySlug( return } const target = getActiveRuntimeTarget(settings) + const routeKey = repoTargetKey(repoTarget) const key = target.kind === 'environment' - ? `runtime:${target.environmentId}:${owner}/${repo}` - : `${owner}/${repo}` + ? `runtime:${target.environmentId}:${routeKey}:${owner}/${repo}` + : `${routeKey}:${owner}/${repo}` const cached = getFreshMetadata(slugLabelStore, key) if (cached) { @@ -82,10 +89,10 @@ export function useRepoLabelsBySlug( ? callRuntimeRpc( target, 'github.project.listLabelsBySlug', - { owner, repo }, + { ...repoTarget, owner, repo }, { timeoutMs: 30_000 } ) - : window.api.gh.listLabelsBySlug({ owner, repo }) + : window.api.gh.listLabelsBySlug({ ...repoTarget, owner, repo }) ).then((res) => { if (!res.ok) { throw new Error(res.error.message) @@ -110,7 +117,7 @@ export function useRepoLabelsBySlug( error: err instanceof Error ? err.message : 'Failed to load labels' })) }) - }, [owner, repo, settings]) + }, [owner, repo, settings, repoTarget]) return state } @@ -119,7 +126,8 @@ export function useRepoAssigneesBySlug( owner: string | null, repo: string | null, seedLogins?: string[], - settings?: Pick | null + settings?: Pick | null, + repoTarget?: GitHubRepoTarget ): MetadataState { const [state, setState] = useState>({ data: [], @@ -137,10 +145,11 @@ export function useRepoAssigneesBySlug( return } const target = getActiveRuntimeTarget(settings) + const routeKey = repoTargetKey(repoTarget) const key = target.kind === 'environment' - ? `runtime:${target.environmentId}:${owner}/${repo}#${seedKey}` - : `${owner}/${repo}#${seedKey}` + ? `runtime:${target.environmentId}:${routeKey}:${owner}/${repo}#${seedKey}` + : `${routeKey}:${owner}/${repo}#${seedKey}` const cached = getFreshMetadata(slugAssigneeStore, key) if (cached) { @@ -165,6 +174,7 @@ export function useRepoAssigneesBySlug( error: null })) const args = { + ...repoTarget, owner, repo, ...(seedKey ? { seedLogins: seedKey.split(',') } : {}) @@ -202,7 +212,7 @@ export function useRepoAssigneesBySlug( error: err instanceof Error ? err.message : 'Failed to load assignees' })) }) - }, [owner, repo, seedKey, settings]) + }, [owner, repo, seedKey, settings, repoTarget]) return state } diff --git a/src/renderer/src/lib/github-active-repo-target.ts b/src/renderer/src/lib/github-active-repo-target.ts new file mode 100644 index 00000000000..3e904f844b3 --- /dev/null +++ b/src/renderer/src/lib/github-active-repo-target.ts @@ -0,0 +1,12 @@ +import type { AppState } from '@/store/types' +import type { GitHubRepoTarget } from '../../../shared/github-project-types' + +export function activeGitHubRepoTargetFromState( + state: Pick +): GitHubRepoTarget { + const repo = state.activeRepoId ? state.repos.find((r) => r.id === state.activeRepoId) : null + if (!repo) { + return {} + } + return { repoPath: repo.path, connectionId: repo.connectionId ?? null } +} diff --git a/src/renderer/src/store/slices/github.ts b/src/renderer/src/store/slices/github.ts index 28618d3a8cd..1cf00aaa58e 100644 --- a/src/renderer/src/store/slices/github.ts +++ b/src/renderer/src/store/slices/github.ts @@ -30,7 +30,8 @@ import type { GitHubProjectMutationResult, GitHubProjectRow, GitHubProjectTable, - GitHubProjectViewError + GitHubProjectViewError, + GitHubRepoTarget } from '../../../../shared/github-project-types' import { isGitHubWorkItemsSshRemoteRequiredError, @@ -61,6 +62,7 @@ import { getTaskSourceRuntimeSettings, type TaskSourceContext } from '../../../../shared/task-source-context' +import { activeGitHubRepoTargetFromState } from '../../lib/github-active-repo-target' // ─── ProjectV2 cache types ──────────────────────────────────────────── // Why: declared separately from CacheEntry (not a generified E parameter) @@ -422,9 +424,14 @@ export function projectViewCacheKey( projectNumber: number, resolvedViewId: string, queryOverride?: string, - sourceScope = 'local' + sourceScope = 'local', + // Why (issue #1715): the repo-host routing hint participates in the cache + // key so the same owner/project on github.com vs GHES can't reuse each + // other's cached table. + repoTarget?: GitHubRepoTarget ): string { - return `github-project:${sourceScope}:${ownerType}:${owner}:${projectNumber}:${resolvedViewId}${queryOverrideKeyPart(queryOverride)}` + const targetKey = `${repoTarget?.connectionId ?? 'local'}:${repoTarget?.repoPath ?? ''}` + return `github-project:${sourceScope}:${targetKey}:${ownerType}:${owner}:${projectNumber}:${resolvedViewId}${queryOverrideKeyPart(queryOverride)}` } function projectViewRequestKey(args: GetProjectViewTableArgs, sourceScope: string): string { @@ -438,7 +445,8 @@ function projectViewRequestKey(args: GetProjectViewTableArgs, sourceScope: strin : args.viewName ? `name:${args.viewName}` : 'default' - return `${sourceScope}:${args.ownerType}:${args.owner}:${args.projectNumber}:${selector}${queryOverrideKeyPart(args.queryOverride)}` + const targetKey = `${args.connectionId ?? 'local'}:${args.repoPath ?? ''}` + return `${sourceScope}:${targetKey}:${args.ownerType}:${args.owner}:${args.projectNumber}:${selector}${queryOverrideKeyPart(args.queryOverride)}` } function projectViewSourceScope(settings: AppState['settings']): string { @@ -2023,18 +2031,23 @@ export const createGitHubSlice: StateCreator = (s fetchProjectViewTable: async (args, options) => { const target = getActiveRuntimeTarget(get().settings) const sourceScope = projectViewSourceScope(get().settings) - const requestKey = projectViewRequestKey(args, sourceScope) + // Why (issue #1715): include the repo-host routing hint in both the + // request and cache identity so same owner/project numbers on different + // GitHub hosts cannot reuse each other's data. + const payload: GetProjectViewTableArgs = { ...activeGitHubRepoTargetFromState(get()), ...args } + const requestKey = projectViewRequestKey(payload, sourceScope) // Fast path: when the caller supplies `viewId`, we already know the // resolved cache key and can serve a fresh entry directly. - const maybeKnownKey = args.viewId + const maybeKnownKey = payload.viewId ? projectViewCacheKey( - args.ownerType, - args.owner, - args.projectNumber, - args.viewId, - args.queryOverride, - sourceScope + payload.ownerType, + payload.owner, + payload.projectNumber, + payload.viewId, + payload.queryOverride, + sourceScope, + payload ) : null if (!options?.force && maybeKnownKey) { @@ -2064,10 +2077,10 @@ export const createGitHubSlice: StateCreator = (s ? await callRuntimeRpc( target, 'github.project.viewTable', - args, + payload, { timeoutMs: 60_000 } ) - : await window.api.gh.getProjectViewTable(args) + : await window.api.gh.getProjectViewTable(payload) if (envelope.ok) { const table = envelope.data const key = projectViewCacheKey( @@ -2075,8 +2088,9 @@ export const createGitHubSlice: StateCreator = (s table.project.owner, table.project.number, table.selectedView.id, - args.queryOverride, - sourceScope + payload.queryOverride, + sourceScope, + payload ) set((s) => ({ projectViewCache: withBoundedCacheEntry(s.projectViewCache, key, { @@ -2159,25 +2173,25 @@ export const createGitHubSlice: StateCreator = (s applyRowPatch(set, cacheKey, rowId, optimisticRow) const target = getActiveRuntimeTarget(settingsForProjectViewCacheKey(get().settings, cacheKey)) + // Why (issue #1715): route the ProjectV2 field-update GraphQL to the + // host that owns the active repo so GHES projects don't fall through + // to gh's globally-active host (github.com) and 404 / mis-auth. + const updatePayload = { + ...activeGitHubRepoTargetFromState(state), + projectId: table.project.id, + itemId: rowId, + fieldId, + value + } const result = target.kind === 'environment' ? await callRuntimeRpc( target, 'github.project.updateItemField', - { - projectId: table.project.id, - itemId: rowId, - fieldId, - value - }, + updatePayload, { timeoutMs: 30_000 } ) - : await window.api.gh.updateProjectItemField({ - projectId: table.project.id, - itemId: rowId, - fieldId, - value - }) + : await window.api.gh.updateProjectItemField(updatePayload) if (!result.ok) { rollbackRowIfPresent(set, get, cacheKey, rowId, previousRow) } @@ -2217,23 +2231,23 @@ export const createGitHubSlice: StateCreator = (s applyRowPatch(set, cacheKey, rowId, optimisticRow) const target = getActiveRuntimeTarget(settingsForProjectViewCacheKey(get().settings, cacheKey)) + // Why (issue #1715): same host-routing concern as updateProjectFieldValue + // — clear-field GraphQL must hit the host that owns the active repo. + const clearPayload = { + ...activeGitHubRepoTargetFromState(state), + projectId: table.project.id, + itemId: rowId, + fieldId + } const result = target.kind === 'environment' ? await callRuntimeRpc( target, 'github.project.clearItemField', - { - projectId: table.project.id, - itemId: rowId, - fieldId - }, + clearPayload, { timeoutMs: 30_000 } ) - : await window.api.gh.clearProjectItemField({ - projectId: table.project.id, - itemId: rowId, - fieldId - }) + : await window.api.gh.clearProjectItemField(clearPayload) if (!result.ok) { rollbackRowIfPresent(set, get, cacheKey, rowId, previousRow) } @@ -2327,11 +2341,15 @@ export const createGitHubSlice: StateCreator = (s settingsForProjectViewCacheKey(get().settings, cacheKey) ) ) + // Why (issue #1715): the repo-host routing hint must also flow to the + // slug-addressed mutation so it lands on the host that owns the repo. + const repoTarget = activeGitHubRepoTargetFromState(state) if ( previousRow.itemType === 'PULL_REQUEST' && (updates.title !== undefined || updates.body !== undefined) ) { const args = { + ...repoTarget, owner, repo, number, @@ -2363,6 +2381,7 @@ export const createGitHubSlice: StateCreator = (s (updates.title !== undefined || updates.body !== undefined))) ) { const args = { + ...repoTarget, owner, repo, number, @@ -2455,7 +2474,11 @@ export const createGitHubSlice: StateCreator = (s settingsForProjectViewCacheKey(get().settings, cacheKey) ) ) + // Why (issue #1715): also thread the repo-host routing hint into the + // slug-addressed issue-type mutation. + const repoTarget = activeGitHubRepoTargetFromState(state) const args = { + ...repoTarget, owner, repo, number, diff --git a/src/shared/github-project-types.ts b/src/shared/github-project-types.ts index 3067aa95260..91c8b55af80 100644 --- a/src/shared/github-project-types.ts +++ b/src/shared/github-project-types.ts @@ -327,7 +327,20 @@ export type ListIssueTypesBySlugResult = // ─── IPC arg shapes (shared between main, preload, renderer) ────────── -export type GetProjectViewTableArgs = { +/** + * Repo target for routing `gh` to the correct host in multi-host setups + * (e.g. github.com + GHES). Main resolves the repo's git remote host and + * passes it to `gh api --hostname`; local repos also keep `cwd: repoPath` + * for repo-context placeholders. When `connectionId` is set the path is read + * through the SSH git provider and local cwd is omitted. Both optional — + * calls with no repo context accept gh's globally-active host. See issue #1715. + */ +export type GitHubRepoTarget = { + repoPath?: string + connectionId?: string | null +} + +export type GetProjectViewTableArgs = GitHubRepoTarget & { owner: string ownerType: GitHubProjectOwnerType projectNumber: number @@ -343,77 +356,77 @@ export type GetProjectViewTableArgs = { queryOverride?: string } -export type ProjectWorkItemDetailsBySlugArgs = { +export type ProjectWorkItemDetailsBySlugArgs = GitHubRepoTarget & { owner: string repo: string number: number type: 'issue' | 'pr' } -export type UpdateProjectItemFieldArgs = { +export type UpdateProjectItemFieldArgs = GitHubRepoTarget & { projectId: string itemId: string fieldId: string value: GitHubProjectFieldMutationValue } -export type ClearProjectItemFieldArgs = { +export type ClearProjectItemFieldArgs = GitHubRepoTarget & { projectId: string itemId: string fieldId: string } -export type UpdateIssueBySlugArgs = { +export type UpdateIssueBySlugArgs = GitHubRepoTarget & { owner: string repo: string number: number updates: GitHubIssueUpdate & { body?: string } } -export type UpdatePullRequestBySlugArgs = { +export type UpdatePullRequestBySlugArgs = GitHubRepoTarget & { owner: string repo: string number: number updates: { title?: string; body?: string; state?: 'open' | 'closed' } } -export type AddIssueCommentBySlugArgs = { +export type AddIssueCommentBySlugArgs = GitHubRepoTarget & { owner: string repo: string number: number body: string } -export type UpdateIssueCommentBySlugArgs = { +export type UpdateIssueCommentBySlugArgs = GitHubRepoTarget & { owner: string repo: string commentId: number body: string } -export type DeleteIssueCommentBySlugArgs = { +export type DeleteIssueCommentBySlugArgs = GitHubRepoTarget & { owner: string repo: string commentId: number } -export type ListLabelsBySlugArgs = { +export type ListLabelsBySlugArgs = GitHubRepoTarget & { owner: string repo: string } -export type ListAssignableUsersBySlugArgs = { +export type ListAssignableUsersBySlugArgs = GitHubRepoTarget & { owner: string repo: string seedLogins?: string[] } -export type ListIssueTypesBySlugArgs = { +export type ListIssueTypesBySlugArgs = GitHubRepoTarget & { owner: string repo: string } -export type UpdateIssueTypeBySlugArgs = { +export type UpdateIssueTypeBySlugArgs = GitHubRepoTarget & { owner: string repo: string number: number @@ -421,12 +434,14 @@ export type UpdateIssueTypeBySlugArgs = { issueTypeId: string | null } -export type ResolveProjectRefArgs = { +export type ResolveProjectRefArgs = GitHubRepoTarget & { input: string } -export type ListProjectViewsArgs = { +export type ListProjectViewsArgs = GitHubRepoTarget & { owner: string ownerType: GitHubProjectOwnerType projectNumber: number } + +export type ListAccessibleProjectsArgs = GitHubRepoTarget