mirror of
https://github.com/stablyai/orca.git
synced 2026-10-17 08:03:14 +00:00
fix: keep originCandidate on raw origin remote for fork detection
resolvePrWorkItemSource resolved the raw origin candidate via getOwnerRepo, which is now upstream-first. For fork repos that collapsed originCandidate onto upstreamCandidate, hiding the issue-source selector in the renderer. Resolve it via getOriginOwnerRepo so the raw origin remote is preserved, and refresh two comments that still claimed PR lookups target origin. Co-Authored-By: Claude <noreply@anthropic.com> Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
co-authored by
Claude
Orca
parent
62c9b87909
commit
7a9b4b219f
@@ -36,6 +36,10 @@ vi.mock('./gh-utils', async () => {
|
||||
execFileAsync: execFileAsyncMock,
|
||||
ghExecFileAsync: ghExecFileAsyncMock,
|
||||
getOwnerRepo: getOwnerRepoMock,
|
||||
// Why: resolvePrWorkItemSource resolves the raw origin candidate via
|
||||
// getOriginOwnerRepo; map it to the same mock these tests use for the
|
||||
// origin PR repo.
|
||||
getOriginOwnerRepo: getOwnerRepoMock,
|
||||
getIssueOwnerRepo: getIssueOwnerRepoMock,
|
||||
getOwnerRepoForRemote: getOwnerRepoForRemoteMock,
|
||||
resolveIssueSource: resolveIssueSourceMock,
|
||||
|
||||
@@ -50,6 +50,9 @@ vi.mock('./gh-utils', () => ({
|
||||
? {}
|
||||
: { cwd: context.repoPath, ...(context.wslDistro ? { wslDistro: context.wslDistro } : {}) },
|
||||
getOwnerRepo: getOwnerRepoMock,
|
||||
// Why: resolvePrWorkItemSource resolves the raw origin candidate via
|
||||
// getOriginOwnerRepo; map it to the same mock these tests use for origin.
|
||||
getOriginOwnerRepo: getOwnerRepoMock,
|
||||
getIssueOwnerRepo: getIssueOwnerRepoMock,
|
||||
getOwnerRepoForRemote: getOwnerRepoForRemoteMock,
|
||||
resolveIssueSource: resolveIssueSourceMock,
|
||||
|
||||
@@ -993,8 +993,12 @@ async function resolvePrWorkItemSource(
|
||||
connectionId?: string | null,
|
||||
localGitOptions: LocalGitExecOptions = {}
|
||||
): Promise<ResolvedPrWorkItemSource> {
|
||||
// Why: originCandidate must stay the raw `origin` remote (not upstream-first),
|
||||
// so the renderer can compare it against upstream to decide whether to show the
|
||||
// issue-source selector. getOwnerRepo is now upstream-first, so use the
|
||||
// origin-specific resolver here.
|
||||
const [originCandidate, upstreamCandidate] = await Promise.all([
|
||||
getOwnerRepo(repoPath, connectionId, localGitOptions),
|
||||
getOriginOwnerRepo(repoPath, connectionId, localGitOptions),
|
||||
getOwnerRepoForRemote(repoPath, 'upstream', connectionId, localGitOptions)
|
||||
])
|
||||
const source =
|
||||
@@ -1898,12 +1902,13 @@ export async function getWorkItem(
|
||||
return issue
|
||||
}
|
||||
} catch (err) {
|
||||
// Why: the issue lookup now targets `upstream` while the PR lookup targets `origin`,
|
||||
// so a transient upstream failure (5xx, rate limit, network flake) on issue #N would
|
||||
// silently fall through to origin's PR #N — potentially a completely unrelated item.
|
||||
// Only fall through when the issue genuinely doesn't exist (404); re-throw everything
|
||||
// else so the outer catch returns null and the caller sees a real failure instead of
|
||||
// a wrong item. classifyGhError centralizes the 404/"not found" pattern-matching.
|
||||
// Why: issue and PR lookups both resolve upstream-first, so a transient
|
||||
// upstream failure (5xx, rate limit, network flake) on issue #N would
|
||||
// silently fall through to PR #N — a different item that happens to share
|
||||
// the number. Only fall through when the issue genuinely doesn't exist
|
||||
// (404); re-throw everything else so the outer catch returns null and the
|
||||
// caller sees a real failure instead of a wrong item. classifyGhError
|
||||
// centralizes the 404/"not found" pattern-matching.
|
||||
const stderr = err instanceof Error ? err.message : String(err)
|
||||
if (classifyGhError(stderr).type !== 'not_found') {
|
||||
throw err
|
||||
|
||||
@@ -801,9 +801,9 @@ async function getWorkItemParticipants(
|
||||
connectionId?: string | null,
|
||||
localGitOptions: LocalGitExecOptions = {}
|
||||
): Promise<GitHubAssignableUser[]> {
|
||||
// Why: issues in a fork live on the upstream remote, so participants must be
|
||||
// resolved via getIssueOwnerRepo to stay consistent with getIssueBodyAndComments.
|
||||
// PRs remain tied to origin via getOwnerRepo.
|
||||
// Why: both issues and PRs in a fork live on the upstream remote, so
|
||||
// participants are resolved via getIssueOwnerRepo/getOwnerRepo, which both
|
||||
// resolve upstream-first, staying consistent with getIssueBodyAndComments.
|
||||
const ownerRepo =
|
||||
item.type === 'issue'
|
||||
? await getIssueOwnerRepo(repoPath, connectionId, ...localGitOptionArgs(localGitOptions))
|
||||
|
||||
Reference in New Issue
Block a user