fix(github): GitHub routing takes the caller's host for a twin; the task filters pass theirs (#31263)

* fix(github): GitHub routing takes the caller's host for a twin; the task filters pass theirs

* fix(github): task filters take the host from their source row, not an id+path lookup
This commit is contained in:
Neil
2026-10-20 19:11:57 -07:00
committed by GitHub
parent 6e589bb4ba
commit 0c2b01367e
5 changed files with 80 additions and 25 deletions
@@ -268,6 +268,16 @@ describe('task page cache selectors', () => {
expect(nextPages[0]?.[1]).toBe(remote)
})
it('names each twin row host in source state, even when the twins share a path', () => {
const local = { id: 'repo-1', path: '/same/path' }
const ssh = { id: 'repo-1', path: '/same/path', connectionId: 'devbox' }
expect(buildTaskPageRepoSourceState([local, ssh], []).map((state) => state.hostId)).toEqual([
'local',
'ssh:devbox'
])
})
it('reconciles paged table rows with patched work-item cache entries', () => {
const stale = {
...workItem('pr-1', 'repo-1'),
@@ -54,12 +54,16 @@ export function TaskPageGitHubFilters({
githubTasksBusy
} = model
// Why: filter metadata for the primary slug is read from the host that owns the repo it came from.
const primaryGithubFilterRepoId =
perRepoSourceState.find((state) =>
activeGithubTaskKind === 'prs' ? state.sources?.prs : state.sources?.issues
)?.repoId ?? null
const primaryGithubFilterSource = perRepoSourceState.find((state) =>
activeGithubTaskKind === 'prs' ? state.sources?.prs : state.sources?.issues
)
const primaryGithubFilterRepoId = primaryGithubFilterSource?.repoId ?? null
// Why the host: a twin's two rows share the id, so only the source's row host names one.
const primaryGithubFilterHostId = primaryGithubFilterSource?.hostId ?? null
const primaryGithubFilterTarget = useAppStore(
useShallow((s) => getGitHubRepoRoutingTarget(s, primaryGithubFilterRepoId, null))
useShallow((s) =>
getGitHubRepoRoutingTarget(s, primaryGithubFilterRepoId, null, primaryGithubFilterHostId)
)
)
return (
// Why: top of the joined GitHub list card — pairs with the
@@ -128,6 +128,43 @@ describe('getGitHubRepoRoutingTarget', () => {
).toEqual({ kind: 'local' })
})
describe('a repo id on two servers', () => {
const twins: RepoRuntimeOwnerState = {
settings: { activeRuntimeEnvironmentId: 'runtime-a' },
repos: [
{ id: 'repo-1', connectionId: null, executionHostId: 'runtime:runtime-a' },
{ id: 'repo-1', connectionId: null, executionHostId: 'runtime:runtime-b' }
]
}
it('routes to the host the caller names, not the Active Server focus', () => {
expect(getGitHubRepoRoutingTarget(twins, 'repo-1', null, 'runtime:runtime-b')).toEqual({
kind: 'environment',
environmentId: 'runtime-b'
})
})
it('stays local when the caller names this computer', () => {
const withLocal: RepoRuntimeOwnerState = {
...twins,
repos: [
...(twins.repos ?? []),
{ id: 'repo-1', connectionId: null, executionHostId: 'local' }
]
}
expect(getGitHubRepoRoutingTarget(withLocal, 'repo-1', null, 'local')).toEqual({
kind: 'local'
})
})
it('falls back to the Active Server focus only when the caller names no host', () => {
expect(getGitHubRepoRoutingTarget(twins, 'repo-1', null)).toEqual({
kind: 'environment',
environmentId: 'runtime-a'
})
})
})
it('ignores runtime hosts on non-GitHub sources', () => {
expect(
resolveTarget(runtimeOwnedRepo, 'repo-1', {
@@ -1,4 +1,4 @@
import type { ParsedExecutionHost } from '../../../shared/execution-host'
import type { ExecutionHostId, ParsedExecutionHost } from '../../../shared/execution-host'
import { parseExecutionHostId } from '../../../shared/execution-host'
import type { TaskSourceContext } from '../../../shared/task-source-context'
import { runtimeTargetForOwnerEnvironment } from '@/runtime/runtime-client-target'
@@ -30,12 +30,14 @@ export function getGitHubSourceRuntimeTarget(
// Why: PR reads and actions run on the repo's explicit owner host (#6957, #7623);
// a local or absent source never downgrades a runtime-owned repo to local IPC,
// a runtime source still overrides, and the globally focused runtime is never
// used as a fallback — a repo without an explicit owner is a local repo.
// a runtime source still overrides, and a repo without an explicit owner is a
// local repo. Only a twin with no caller host falls back to the focused runtime.
export function getGitHubRepoRoutingTarget(
state: RepoRuntimeOwnerState,
repoId: string | null | undefined,
sourceContext: TaskSourceContext | null | undefined
sourceContext: TaskSourceContext | null | undefined,
/** The host of the row the caller is showing; picks between twins without a github source. */
hostId?: ExecutionHostId | null
): RuntimeClientTarget {
const sourceHost = getGitHubSourceRuntimeHost(sourceContext)
if (sourceHost) {
@@ -44,7 +46,7 @@ export function getGitHubRepoRoutingTarget(
return runtimeTargetForOwnerEnvironment(
sourceContext?.provider === 'github'
? getHostlessSourceRepoOwnerEnvironmentId(state.repos, repoId)
: getExplicitRuntimeOwnerEnvironmentId(state, repoId)
: getExplicitRuntimeOwnerEnvironmentId(state, repoId, hostId)
)
}
+17 -15
View File
@@ -1,7 +1,8 @@
import {
getRepoExecutionHostId,
getSettingsFocusedExecutionHostId,
parseExecutionHostId
parseExecutionHostId,
type ExecutionHostId
} from '../../../shared/execution-host'
import type { GlobalSettings } from '../../../shared/global-settings-types'
import type { Repo } from '../../../shared/repo-types'
@@ -23,22 +24,22 @@ function findRepoOwner(
}
/**
* GitHub routing's twin pick, kept until `getGitHubRepoRoutingTarget` can refuse: dropping it would
* send a twin on two servers to local IPC with this machine's credentials.
* GitHub routing's twin pick: the caller's host when it names one, else the Active Server focus.
* Why the focus fallback stays: the routing target can't refuse, and dropping it would send a twin
* on two servers to local IPC with this machine's credentials.
*/
function findFocusedTwinRepoOwner(
function findTwinRepoOwner(
state: RepoRuntimeOwnerState,
repoId: string
repoId: string,
hostId: ExecutionHostId | null | undefined
): Pick<Repo, 'id' | 'connectionId' | 'executionHostId'> | null {
const matchingRepos = state.repos?.filter((entry) => entry.id === repoId) ?? []
if (matchingRepos.length < 2) {
return matchingRepos[0] ?? null
}
const focusedHostId = getSettingsFocusedExecutionHostId(state.settings)
const focusedMatches = matchingRepos.filter(
(entry) => getRepoExecutionHostId(entry) === focusedHostId
)
return focusedMatches.length === 1 ? focusedMatches[0] : null
const pickHostId = hostId ?? getSettingsFocusedExecutionHostId(state.settings)
const hostMatches = matchingRepos.filter((entry) => getRepoExecutionHostId(entry) === pickHostId)
return hostMatches.length === 1 ? hostMatches[0] : null
}
/** Whether two rows (hosts) publish this repo id, so no host-less caller may pick one. */
@@ -77,17 +78,18 @@ export function getRuntimeEnvironmentIdForRepo(
: state.settings?.activeRuntimeEnvironmentId?.trim() || null
}
// Why: PR mutations must not fall back to the globally focused runtime — a
// repo without an explicit owner is a local repo, and routing it to the
// focused runtime sends the mutation to a host that does not own it (#6957).
// Why: a repo without an explicit owner is a local repo; routing it to the focused
// runtime sends the mutation to a host that does not own it (#6957). Only a twin with
// no caller host still reads focus (findTwinRepoOwner).
export function getExplicitRuntimeOwnerEnvironmentId(
state: RepoRuntimeOwnerState,
repoId: string | null | undefined
repoId: string | null | undefined,
hostId?: ExecutionHostId | null
): string | null {
if (!repoId) {
return null
}
const repo = findFocusedTwinRepoOwner(state, repoId)
const repo = findTwinRepoOwner(state, repoId, hostId)
const hasExplicitOwner = Boolean(repo?.executionHostId?.trim() || repo?.connectionId?.trim())
if (!repo || !hasExplicitOwner) {
return null