puter/paseo
1
0
Fork 0
mirror of https://github.com/getpaseo/paseo.git synced 2026-11-02 22:52:58 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(forges): preserve pull request repository identity

Browse Source 
Treat bounded discovery as inconclusive and resolve redirected repositories before selecting requests.
This commit is contained in:
Mohamed Boudra
2026-11-02 06:35:23 +01:00
parent dc7ade5264
commit f1e262a142
6 changed files with 293 additions and 53 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/server/src/services/gitea-service.test.ts
+24
View File
@@ -2097,6 +2097,30 @@ describe("createGiteaService", () => {
]);
});
it("discovers the repository from tea when origin and a head path are unavailable", async () => {
const pr = currentPullRequestApi({
number: 8,
state: "closed",
headRef: "topic",
headSha: "1".repeat(40),
merged: true,
});
const { service } = makeService(
(args) => {
if (args[0] === "api" && args[1] === "repos/{owner}/{repo}")
return ok(JSON.stringify({ full_name: "example-user/sample-repo" }));
if (args[0] === "pr" && args[1] === "list") return ok("[]");
if (args[0] === "api" && args[1].includes("pulls?state=all"))
return ok(JSON.stringify([pr]));
throw new Error(`unexpected call: ${args.join(" ")}`);
},
{ resolveRemoteUrl: async () => null },
);
expect(
(await service.getCurrentPullRequestStatus({ cwd: "/repo", headRef: "topic" }))?.number,
).toBe(8);
});
it("shows a merged PR when the checkout head differs", async () => {
const stale = currentPullRequestApi({
number: 8,
packages/server/src/services/gitea-service.ts
+8
View File
@@ -1638,6 +1638,14 @@ export function createGiteaService(options: CreateGiteaServiceOptions = {}): For
(input.headRepositoryOwner
? null
: repoIdentity && `${repoIdentity.owner}/${repoIdentity.name}`);
if (!expectedPath && !input.headRepositoryOwner && !repoIdentity) {
const repository = await runJson(
["api", "repos/{owner}/{repo}"],
{ cwd: input.cwd },
GiteaPullRequestRepoSchema,
);
expectedPath = repository.full_name ?? null;
}
if (!expectedPath && input.headRepositoryOwner && input.changeRequestNumber) {
const hint = await runJson(
["api", `${repoPath}/pulls/${input.changeRequestNumber}`],
packages/server/src/services/github-service.test.ts
+130 -3
View File
@@ -18,6 +18,7 @@ import {
} from "./github-service.js";
import { isPlatform } from "../test-utils/platform.js";
import { CheckoutPrStatusResponseSchema } from "@getpaseo/protocol/messages";
import { ForgeDiscoveryIncompleteError } from "./forge-service.js";
const EXPECTED_GITHUB_FAST_POLL_MS = 20_000;
const EXPECTED_GITHUB_SLOW_POLL_MS = 120_000;
@@ -1874,6 +1875,82 @@ describe("ForgeService", () => {
service.dispose?.();
});
it("uses canonical repository identity for batched polls after a transfer", async () => {
const node = batchPollPrNodeJson({
state: "OPEN",
mergedAt: null,
headRepositoryOwner: { login: "new-owner" },
headRepository: { name: "repo" },
});
const runner = createScriptedRunner([
batchPollStatusJson({
t0: batchPollRepositoryJson([node], { owner: { login: "new-owner" }, name: "repo" }),
}),
batchPollStatusJson({ t0: batchPollChecksAliasJson([]) }),
]);
const service = createGitHubService({
ttlMs: 0,
runner: runner.runner,
resolveGhPath: async () => "/usr/bin/gh",
resolveRepoHost: async () => null,
resolveRepoSlug: async () => "old-owner/repo",
});
const statuses: Array<CurrentPullRequestStatus | null> = [];
const subscription = service.retainCurrentPullRequestStatusPoll?.({
cwd: "/ws-a",
headRef: "feat-a",
headRepositoryPath: "old-owner/repo",
onStatus: (status) => statuses.push(status),
});
await vi.advanceTimersByTimeAsync(0);
await flushMicrotasks();
expect(statuses).toEqual([expect.objectContaining({ number: 42, state: "open" })]);
subscription?.unsubscribe();
service.dispose?.();
});
it("resolves a transferred fork during batched polling", async () => {
const node = batchPollPrNodeJson({
state: "OPEN",
mergedAt: null,
headRepositoryOwner: { login: "new-fork" },
headRepository: { name: "repo" },
});
const pr = currentPullRequestJson({
url: "https://github.com/base/repo/pull/42",
headRefName: "feat-a",
headRepositoryOwner: { login: "new-fork" },
headRepository: { name: "repo" },
});
const runner = createScriptedRunner([
batchPollStatusJson({ t0: batchPollRepositoryJson([node]) }),
pr,
JSON.stringify([JSON.parse(pr)]),
JSON.stringify({ full_name: "new-fork/repo" }),
currentPullRequestGithubFactsJson(),
]);
const service = createGitHubService({
ttlMs: 0,
runner: runner.runner,
resolveGhPath: async () => "/usr/bin/gh",
resolveRepoHost: async () => null,
resolveRepoSlug: async () => "base/repo",
});
const statuses: Array<CurrentPullRequestStatus | null> = [];
const subscription = service.retainCurrentPullRequestStatusPoll?.({
cwd: "/ws-a",
headRef: "feat-a",
headRepositoryOwner: "old-fork",
headRepositoryPath: "old-fork/repo",
onStatus: (status) => statuses.push(status),
});
await vi.advanceTimersByTimeAsync(0);
await flushMicrotasks();
expect(statuses).toEqual([expect.objectContaining({ number: 42, state: "open" })]);
subscription?.unsubscribe();
service.dispose?.();
});
it("recovers healthy aliases from a partially failed batch response", async () => {
let now = 0;
const partialBody = batchPollStatusJson({
@@ -3747,12 +3824,62 @@ describe("ForgeService", () => {
runner: runner.runner,
resolveGhPath: async () => "/usr/bin/gh",
});
expect(
await service.getCurrentPullRequestStatus({ cwd: "/repo", headRef: "feature/fork" }),
).toBeNull();
await expect(
service.getCurrentPullRequestStatus({ cwd: "/repo", headRef: "feature/fork" }),
).rejects.toBeInstanceOf(ForgeDiscoveryIncompleteError);
expect(runner.calls.filter((call) => call.args[0] === "api")).toHaveLength(1);
});
it("matches a repository transferred while the checkout retains its old origin URL", async () => {
const pr = currentPullRequestJson({
url: "https://github.com/new-owner/repo/pull/42",
headRepositoryOwner: { login: "new-owner" },
headRepository: { name: "repo" },
});
const runner = createScriptedRunner([
pr,
JSON.stringify({ owner: { login: "new-owner" }, name: "repo", parent: null }),
JSON.stringify([JSON.parse(pr)]),
]);
const service = createGitHubService({
runner: runner.runner,
resolveGhPath: async () => "/usr/bin/gh",
resolveRepoSlug: async () => "old-owner/repo",
});
expect(
await service.getCurrentPullRequestStatus({
cwd: "/repo",
headRef: "feature/fork",
headRepositoryPath: "old-owner/repo",
}),
).toMatchObject({ number: 42 });
});
it("matches a transferred fork using its canonical repository identity", async () => {
const pr = currentPullRequestJson({
url: "https://github.com/base/repo/pull/42",
headRepositoryOwner: { login: "new-fork" },
headRepository: { name: "repo" },
});
const runner = createScriptedRunner([
pr,
JSON.stringify([JSON.parse(pr)]),
JSON.stringify({ full_name: "new-fork/repo" }),
]);
const service = createGitHubService({
runner: runner.runner,
resolveGhPath: async () => "/usr/bin/gh",
});
expect(
await service.getCurrentPullRequestStatus({
cwd: "/repo",
headRef: "feature/fork",
headRepositoryOwner: "old-fork",
headRepositoryPath: "old-fork/repo",
}),
).toMatchObject({ number: 42 });
});
it("propagates a failed fork PR lookup instead of reporting no pull request", async () => {
const rateLimitError = new GitHubCommandError({
args: ["api", "repos/parentOwner/parentRepo/pulls"],
packages/server/src/services/github-service.ts
+78 -11
View File
@@ -29,6 +29,7 @@ import {
normalizeForgeSearchKinds,
parseOptionalTime,
matchesHeadRepositoryEndpoint,
ForgeDiscoveryIncompleteError,
} from "./forge-service.js";
import type {
CheckAnnotation,
@@ -666,6 +667,7 @@ const BatchPollPrNodeSchema = z.object({
const BatchPollRepositorySchema = z.object({
owner: z.object({ login: z.string().optional() }).nullable().optional().catch(null),
name: z.string().optional(),
isFork: z.boolean().optional().catch(false),
parent: z
.object({ owner: z.object({ login: z.string() }), name: z.string() })
@@ -797,6 +799,7 @@ function buildBatchPullRequestStatusQuery(
owner {
login
}
name
isFork
parent {
owner {
@@ -1108,6 +1111,7 @@ interface GitHubBatchPollEntry {
owner: string;
name: string;
headRepositoryOwner?: string;
canonicalHeadRepositoryPath?: string;
}
interface PendingBatchPullRequestChecks {
@@ -1179,7 +1183,12 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G
const frozenPullRequestChecksCache = new Map<string, unknown[]>();
const pollRepositoryRedirects = new Map<
string,
{ owner: string; name: string; headRepositoryOwner: string }
{
owner: string;
name: string;
headRepositoryOwner: string;
canonicalHeadRepositoryPath: string;
}
>();
const pollPausedUntilByHost = new Map<string, number>();
const rateLimitResetLoads = new Map<string, Promise<number | null>>();
@@ -1754,6 +1763,7 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G
owner: repository.data.parent.owner.login,
name: repository.data.parent.name,
headRepositoryOwner: forkOwner,
canonicalHeadRepositoryPath: `${forkOwner}/${repository.data.name ?? entry.originName}`,
});
distribution.redirected.push(entry);
continue;
@@ -1764,8 +1774,7 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G
}
const node = selectBatchPollNode(entry, repository.data);
if (!node) {
// A fork's PR can live in the parent repository.
if (repository.data.isFork) {
if (shouldRetryUnmatchedBatch(entry, repository.data)) {
distribution.legacyFallback.push(entry.target);
} else {
finalizeBatchPollTarget({
@@ -1778,10 +1787,7 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G
}
continue;
}
const frozenKey =
node.state !== "OPEN" && node.headRefOid.length > 0
? `${entry.owner}/${entry.name}#${node.number}@${node.headRefOid}`
: null;
const frozenKey = frozenPullRequestKey(entry, node);
const cachedRollup = frozenKey ? frozenPullRequestChecksCache.get(frozenKey) : undefined;
if (frozenKey && cachedRollup) {
rememberFrozenPullRequestChecks(frozenKey, cachedRollup);
@@ -1804,6 +1810,24 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G
return distribution;
}
function shouldRetryUnmatchedBatch(
entry: GitHubBatchPollEntry,
repository: BatchPollRepository,
): boolean {
return Boolean(
repository.isFork ||
(entry.target.headRepositoryOwner &&
entry.target.headRepositoryPath &&
repository.pullRequests.nodes.length > 0),
);
}
function frozenPullRequestKey(entry: GitHubBatchPollEntry, node: BatchPollPrNode): string | null {
return node.state !== "OPEN" && node.headRefOid.length > 0
? `${entry.owner}/${entry.name}#${node.number}@${node.headRefOid}`
: null;
}
function addressGitHubBatchEntry(entry: GitHubBatchPollEntry): GitHubBatchPollEntry {
const redirect = pollRepositoryRedirects.get(batchRepositoryRedirectKey(entry));
return redirect
@@ -1812,6 +1836,7 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G
owner: redirect.owner,
name: redirect.name,
headRepositoryOwner: redirect.headRepositoryOwner,
canonicalHeadRepositoryPath: redirect.canonicalHeadRepositoryPath,
}
: entry;
}
@@ -1857,7 +1882,12 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G
headRef: entry.target.headRef,
headRepositoryOwner,
headRepositoryPath:
entry.target.headRepositoryPath ?? `${entry.originOwner}/${entry.originName}`,
!entry.target.headRepositoryOwner &&
entry.target.headRepositoryPath?.toLowerCase() ===
`${entry.originOwner}/${entry.originName}`.toLowerCase()
? (entry.canonicalHeadRepositoryPath ??
`${repository.owner?.login ?? entry.originOwner}/${repository.name ?? entry.originName}`)
: (entry.target.headRepositoryPath ?? `${entry.originOwner}/${entry.originName}`),
});
return match ? (nodeByCandidate.get(match) ?? null) : null;
}
@@ -2220,6 +2250,10 @@ export function createGitHubService(options: CreateGitHubServiceOptions = {}): G
headSha: input.headSha,
headRepositoryOwner: input.headRepositoryOwner,
headRepositoryPath: input.headRepositoryPath,
originRepositoryPath:
input.headRepositoryPath && !input.headRepositoryOwner
? await resolveRepoSlugCached(input.cwd)
: null,
changeRequestNumber: input.changeRequestNumber,
run,
});
@@ -3035,6 +3069,7 @@ interface CurrentPullRequestLookup {
headSha?: string;
headRepositoryOwner?: string;
headRepositoryPath?: string;
originRepositoryPath?: string | null;
changeRequestNumber?: number;
run: (args: string[], options: GitHubCommandRunnerOptions) => Promise<string>;
}
@@ -3087,7 +3122,11 @@ async function resolvePullRequestWithoutOwner(
const parentOwner = repo?.parent?.owner?.login;
const parentName = repo?.parent?.name;
if (!forkOwner || !forkName) return null;
const headRepositoryPath = options.headRepositoryPath ?? `${forkOwner}/${forkName}`;
const headRepositoryPath =
options.headRepositoryPath &&
options.headRepositoryPath.toLowerCase() !== options.originRepositoryPath?.toLowerCase()
? options.headRepositoryPath
: `${forkOwner}/${forkName}`;
options = { ...options, headRepositoryPath };
const expectedOwner = headRepositoryPath.split("/")[0];
if (!expectedOwner) return null;
@@ -3113,12 +3152,37 @@ async function resolveListedPullRequest(
if (candidates.length >= BATCH_PR_CANDIDATE_LIMIT && options.headRepositoryPath) {
return resolveCrowdedPullRequests(options, options.headRepositoryPath);
}
const match = pickPullRequestCandidate({
let match = pickPullRequestCandidate({
candidates: viewMatch ? [viewMatch, ...candidates] : candidates,
headRef: options.headRef,
headRepositoryOwner,
headRepositoryPath: options.headRepositoryPath,
});
if (
!match &&
options.headRepositoryOwner &&
options.headRepositoryPath &&
candidates.some((candidate) => isCandidateForHeadRef(candidate, options.headRef))
) {
const args = ["api", `repos/${options.headRepositoryPath}`];
const stdout = await options.run(args, { cwd: options.cwd });
const repository = parseGitHubJsonOutput(
stdout,
z.object({ full_name: z.string().optional() }),
{ args, cwd: options.cwd, emptyFallback: "{}" },
);
if (
repository.full_name &&
repository.full_name.toLowerCase() !== options.headRepositoryPath.toLowerCase()
) {
match = pickPullRequestCandidate({
candidates: viewMatch ? [viewMatch, ...candidates] : candidates,
headRef: options.headRef,
headRepositoryOwner: repository.full_name.split("/")[0],
headRepositoryPath: repository.full_name,
});
}
}
return match?.status ?? null;
}
@@ -3221,7 +3285,10 @@ async function listForkPullRequestNumbers(options: {
parseOptionalTime(b.updated_at) - parseOptionalTime(a.updated_at) ||
b.number - a.number,
);
if (refs.length >= FORK_PR_PAGE_SIZE && !matching.some((ref) => ref.state === "open")) return [];
if (refs.length >= FORK_PR_PAGE_SIZE && !matching.some((ref) => ref.state === "open"))
throw new ForgeDiscoveryIncompleteError(
"GitHub pull request discovery exceeded its candidate window",
);
return matching.slice(0, FORK_PR_VIEW_LIMIT).map((ref) => ref.number);
}
packages/server/src/services/gitlab-service.test.ts
+30
View File
@@ -262,6 +262,36 @@ const DISCUSSIONS = [
];
describe("createGitLabService", () => {
it.each([true, false])(
"discovers the project without an origin remote (head path supplied: %s)",
async (hasHeadPath) => {
const mr = { ...OPEN_MR, source_branch: "topic" };
const { service } = makeService(
(args) => {
if (args[1] === "list") return ok(JSON.stringify([mr]));
if (args[1] === "view") return ok(JSON.stringify(mr));
if (
args[1] === "projects/:fullpath" ||
args[1] === "projects/example-group%2Fexample-project"
)
return ok(
JSON.stringify({ id: 101, path_with_namespace: "example-group/example-project" }),
);
return ok("{}");
},
{ resolveRemoteUrl: async () => null },
);
expect(
(
await service.getCurrentPullRequestStatus({
cwd: "/repo",
headRef: "topic",
...(hasHeadPath ? { headRepositoryPath: "example-group/example-project" } : {}),
})
)?.number,
).toBe(14);
},
);
it("requires native clone URL proof for mixed transport ports and isolates cached endpoints", async () => {
const mr = { ...OPEN_MR, source_branch: "topic" };
let cloneUrl: string | undefined =
packages/server/src/services/gitlab-service.ts
+23 -39
View File
@@ -988,22 +988,24 @@ export function createGitLabService(options: CreateGitLabServiceOptions = {}): F
}
async function resolveExpectedHeadProject(
cwd: string,
headRepositoryPath?: string,
headRepositoryOwner?: string,
changeRequestNumber?: number,
force = false,
input: Parameters<ForgeService["getCurrentPullRequestStatus"]>[0],
): Promise<string | null> {
const remote = await resolveRemoteUrl(cwd);
const remotePath = remote ? parseGitRemoteLocation(remote)?.path : null;
if (!remotePath) return null;
if (headRepositoryPath) return headRepositoryPath;
if (!headRepositoryOwner) return remotePath;
if (!changeRequestNumber) return null;
const hint = await viewMergeRequest(cwd, String(changeRequestNumber));
if (input.headRepositoryPath) return input.headRepositoryPath;
if (!input.headRepositoryOwner) {
const remote = await resolveRemoteUrl(input.cwd);
return (
(remote ? parseGitRemoteLocation(remote)?.path : null) ??
(await readProject(input.cwd, ":fullpath", input.force === true)).path_with_namespace
);
}
if (!input.changeRequestNumber) return null;
const hint = await viewMergeRequest(input.cwd, String(input.changeRequestNumber));
if (hint.source_project_id == null) return null;
const path = (await readProject(cwd, hint.source_project_id, force)).path_with_namespace;
return path.split("/")[0]?.toLowerCase() === headRepositoryOwner.toLowerCase() ? path : null;
const path = (await readProject(input.cwd, hint.source_project_id, input.force === true))
.path_with_namespace;
return path.split("/")[0]?.toLowerCase() === input.headRepositoryOwner.toLowerCase()
? path
: null;
}
async function readProject(cwd: string, projectId: number | string, force = false) {
@@ -1011,7 +1013,7 @@ export function createGitLabService(options: CreateGitLabServiceOptions = {}): F
const cached = force ? undefined : projectPaths.get(key);
if (cached) return cached;
const source = await runJson(
["api", `projects/${encodeURIComponent(projectId)}`],
["api", `projects/${projectId === ":fullpath" ? projectId : encodeURIComponent(projectId)}`],
{ cwd },
z.object({
id: z.number().optional(),
@@ -1034,7 +1036,7 @@ export function createGitLabService(options: CreateGitLabServiceOptions = {}): F
const remote = await resolveRemoteUrl(input.cwd);
const originPath = remote ? parseGitRemoteLocation(remote)?.path : null;
const targetId =
input.expectedPath.toLowerCase() === originPath?.toLowerCase()
!originPath || input.expectedPath.toLowerCase() === originPath.toLowerCase()
? input.mergeRequests.find(
(mr) =>
extractProjectPath(mr.references?.full)?.toLowerCase() ===
@@ -1054,23 +1056,13 @@ export function createGitLabService(options: CreateGitLabServiceOptions = {}): F
}
async function resolveCurrentMergeRequest(
cwd: string,
headRef: string,
headRepositoryPath?: string,
headRepositoryOwner?: string,
changeRequestNumber?: number,
force = false,
headRepositoryEndpoint?: import("./forge-service.js").HeadRepositoryEndpoint,
input: Parameters<ForgeService["getCurrentPullRequestStatus"]>[0],
): Promise<GitLabMergeRequest | null> {
const { cwd, headRef, headRepositoryEndpoint } = input;
const force = input.force === true;
const mergeRequests = await listMergeRequestsBySourceBranch(cwd, headRef);
if (mergeRequests.length === 0) return null;
const expectedPath = await resolveExpectedHeadProject(
cwd,
headRepositoryPath,
headRepositoryOwner,
changeRequestNumber,
force,
);
const expectedPath = await resolveExpectedHeadProject(input);
if (!expectedPath) return null;
const expectedProjectId = await resolveExpectedProjectId({
cwd,
@@ -1266,15 +1258,7 @@ export function createGitLabService(options: CreateGitLabServiceOptions = {}): F
async getCurrentPullRequestStatus(input): Promise<CurrentPullRequestStatus | null> {
try {
const mr = await resolveCurrentMergeRequest(
input.cwd,
input.headRef,
input.headRepositoryPath,
input.headRepositoryOwner,
input.changeRequestNumber,
input.force === true,
input.headRepositoryEndpoint,
);
const mr = await resolveCurrentMergeRequest(input);
if (!mr) {
return null;
}