fix(mobile): close and reopen issues on older servers when their lookup proves the repository (#38829)

This commit is contained in:
Kelvin Amoaba
2026-11-06 05:02:24 +00:00
committed by GitHub
parent c94caf66e0
commit 24ed22e16f
6 changed files with 105 additions and 12 deletions
@@ -127,6 +127,49 @@ it('a disconnect/reconnect cannot regain authority with an identical connected-a
expect(a.writes).toEqual([])
expect(a.client.stateListeners()).toBe(0)
})
it.each([
['sends the ordinary update when its lookup lands on this issue', '1.4.247', 'fork', 1],
['refuses when its lookup lands on another repository', '1.4.247', 'upstream', 0],
['refuses before its writes followed its lookup', '1.4.220', 'fork', 0]
])('a server without exact targets %s', async (_name, appVersion, owner, sent) => {
const frames: { method: string; params?: unknown; envelope?: unknown }[] = []
const client = createFakeRpcClient()
client.sendRequest = async (method, params, options) => {
frames.push({ method, params, envelope: options?.envelope })
const result =
method === 'status.get'
? {
runtimeId: 'old',
hostDescriptor: { installationId: 'old' },
appVersion,
capabilities: []
}
: method === 'github.workItem'
? {
type: 'issue',
number: 93,
url: `https://github.corp/${owner}/widgets/issues/93`,
labels: []
}
: { ok: true }
return { id: String(frames.length), ok: true, result, _meta: { runtimeId: 'old' } }
}
const params = { repo: 'id:clone', number: 93, updates: { state: 'closed' as const } }
const pending = withGitHubTargetOperation({
client,
params,
ownerRepo: repository,
issueTarget: { repository, type: 'issue', number: 93 },
send: (selected, exact, options) =>
githubIssueUpdate.request(selected, { ...params, ...exact }, options)
})
await (sent
? expect(pending).resolves.toBeDefined()
: expect(pending).rejects.toThrow('Update Orca on the executing server'))
expect(frames.filter((frame) => frame.method === 'github.updateIssue')).toEqual(
sent ? [{ method: 'github.updateIssue', params, envelope: undefined }] : []
)
})
it('names the relaying desktop when only it lacks the exact-target check', async () => {
const desktop = physical('d', false),
server = physical('s')
+42 -3
View File
@@ -1,6 +1,11 @@
import type { RpcClient, SendRequestOptions } from '../transport/rpc-client'
import type { GitHubOwnerRepo } from '../../../src/shared/github/pull-request-types'
import { GITHUB_EXACT_OPERATION_UNSUPPORTED } from '../../../src/shared/github/work-item-target'
import { hasReachedAppVersion } from '../../../src/shared/app-version'
import {
GITHUB_EXACT_OPERATION_UNSUPPORTED,
matchesGitHubWorkItemTarget,
type GitHubWorkItemTarget
} from '../../../src/shared/github/work-item-target'
import {
captureGitHubInvocation,
GITHUB_DESKTOP_UPDATE_REQUIRED,
@@ -9,11 +14,15 @@ import {
githubOperationResponse
} from '../transport/github-target-invocation'
import { openRpcRequestBudget } from '../transport/rpc-request-budget'
import { readGitHubWorkItemByNumber } from './mobile-task-source-search-operations'
import {
GITHUB_CANONICAL_DETAILS_RUNTIME_CAPABILITY,
GITHUB_EXACT_DETAILS_READ_INTENT_RUNTIME_CAPABILITY
} from '../../../src/shared/task-source-runtime-capabilities'
// Before this release an unaddressed issue write ignored the issue source its own number lookup honors.
const ISSUE_WRITE_FOLLOWS_LOOKUP_VERSION = '1.4.221'
export async function withGitHubTargetOperation<T>(args: {
client: RpcClient
params: { repo: string }
@@ -21,6 +30,8 @@ export async function withGitHubTargetOperation<T>(args: {
read?: boolean
/** An addressed details read's intent: false for an ordinary load, true for the user's explicit Refresh. */
canonical?: boolean
/** A write's item, which a server without exact targets may still update unaddressed once its own lookup proves it. */
issueTarget?: GitHubWorkItemTarget
send: (
client: RpcClient,
exact: { ownerRepo?: GitHubOwnerRepo; canonical?: boolean },
@@ -70,8 +81,13 @@ export async function withGitHubTargetOperation<T>(args: {
)
const canonicalOnly =
!explicitIntent && capabilities.includes(GITHUB_CANONICAL_DETAILS_RUNTIME_CAPABILITY)
const sendOwner = canonicalOnly ? args.canonical === true : supports && source !== null
if ((!supports || !source) && !args.read) {
const exact = supports && source !== null
const sendOwner = canonicalOnly ? args.canonical === true : exact
if (
!exact &&
!args.read &&
!(await serverFollowsIssueLookup(client, status, args.params.repo, args.issueTarget, options))
) {
// Name the build to update: the relaying desktop when only it lacks the check, else the executing server.
throw new Error(
serverSupports && source && !relaySupports
@@ -103,6 +119,29 @@ export async function withGitHubTargetOperation<T>(args: {
}
}
// Without ownerRepo a server writes to the issue its own number lookup finds, so a lookup landing on the target proves it.
async function serverFollowsIssueLookup(
client: RpcClient,
status: Record<string, unknown>,
repo: string,
target: GitHubWorkItemTarget | undefined,
options: () => SendRequestOptions
): Promise<boolean> {
if (
target?.type !== 'issue' ||
typeof status.appVersion !== 'string' ||
!hasReachedAppVersion(status.appVersion, ISSUE_WRITE_FOLLOWS_LOOKUP_VERSION)
) {
return false
}
const item = await readGitHubWorkItemByNumber(
client,
{ repo, number: target.number, type: 'issue' },
options()
)
return matchesGitHubWorkItemTarget(item, target)
}
async function invocationStatus(client: RpcClient, options: SendRequestOptions) {
const invocation = captureGitHubInvocation(client)
try {
@@ -1,3 +1,5 @@
import type { GitHubWorkItem } from '../../../src/shared/github/work-item-types'
import type { RpcClient, SendRequestOptions } from '../transport/rpc-client'
import { bindDeferredRpcOperation, defineRpcOperation } from '../transport/rpc-operation'
import { rpcResultVariant } from '../transport/rpc-operation-result-reader'
import { githubPrRepoSlugSchema } from '../session/github-pr-read-reply-schema'
@@ -72,6 +74,17 @@ export const githubWorkItemByNumberRead = bindDeferredRpcOperation(
})
)
export async function readGitHubWorkItemByNumber(
client: RpcClient,
params: { repo: string; number: number; type?: 'issue' | 'pr' },
// A rest tuple keeps the request's recorded arity when no options are given.
...options: [SendRequestOptions?]
): Promise<GitHubWorkItem | null> {
const reply = await githubWorkItemByNumberRead.request(client, params, ...options)
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the lookup decodes the same `workItemRow`, so the assertion only fills in members the row omits, each read through a guard or interpolated as text; what the schema adds is that a non-object payload is now a named incompatible reply rather than a spread over a string.
return githubWorkItemByNumberRead.interpret(reply) as GitHubWorkItem | null
}
export const githubWorkItemBySlugRead = bindDeferredRpcOperation(
defineRpcOperation({
name: 'github.work-item-by-owner-repo',
@@ -12,8 +12,8 @@ import type { RpcClient } from '../transport/rpc-client'
import { isMethodNotFoundRefusal } from '../transport/rpc-acceptance-policies'
import {
githubRepoSlugRead,
githubWorkItemByNumberRead,
githubWorkItemBySlugRead,
readGitHubWorkItemByNumber,
gitlabWorkItemByPathRead
} from './mobile-task-source-search-operations'
import { githubRepoIdentityKey } from '../../../src/shared/github/repository-identity-key'
@@ -132,12 +132,7 @@ export async function lookupGitHubItemByNumber(
repoId: string,
number: number
): Promise<GitHubWorkItem | null> {
const reply = await githubWorkItemByNumberRead.request(client, {
repo: `id:${repoId}`,
number
})
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the lookup decodes the same `workItemRow`, so the assertion only fills in members the row omits, each read through a guard or interpolated as text; what the schema adds is that a non-object payload is now a named incompatible reply rather than a spread over a string.
const item = githubWorkItemByNumberRead.interpret(reply) as GitHubWorkItem | null
const item = await readGitHubWorkItemByNumber(client, { repo: `id:${repoId}`, number })
return item ? { ...item, repoId } : null
}
@@ -116,6 +116,7 @@ export function useMobileTasksGitlabGithubStatusActions<
client,
params,
ownerRepo: target.repository,
issueTarget: target,
send: (selected, exact, options) =>
githubIssueUpdate.request(selected, { ...params, ...exact }, options)
})
@@ -300,13 +300,15 @@ export function useMobileTasksProjectFileMergeActions<Model extends ProjectFileM
const nextState = item.source.state === 'closed' ? 'open' : 'closed'
const isCurrent = claimPublication(item)
try {
const target = requireGitHubTaskTarget(item.source)
const updated =
item.source.type === 'issue'
? githubIssueUpdate.interpret(
await withGitHubTargetOperation({
client,
params: { repo: `id:${item.source.repoId}` },
ownerRepo: requireGitHubTaskTarget(item.source).repository,
ownerRepo: target.repository,
issueTarget: target,
send: (selected, exact, options) =>
githubIssueUpdate.request(
selected,
@@ -324,7 +326,7 @@ export function useMobileTasksProjectFileMergeActions<Model extends ProjectFileM
await githubPullRequestStateUpdate.request(client, {
repo: `id:${item.source.repoId}`,
prNumber: item.source.number,
prRepo: requireGitHubTaskTarget(item.source).repository,
prRepo: target.repository,
updates: { state: nextState }
})
)