From 915d49205aeae8d54c03c32cf4c0e50de0008e0a Mon Sep 17 00:00:00 2001 From: Kalven Schraut Date: Sat, 5 Sep 2026 00:30:48 -0500 Subject: [PATCH] feat(pull-requests): bound repository relationship reads --- .../AzureDevOpsPullRequestCli.test.ts | 29 ++ .../pullRequest/AzureDevOpsPullRequestCli.ts | 13 + .../AzureDevOpsPullRequestProvider.ts | 1 + .../BitbucketPullRequestApi.test.ts | 61 ++++ .../pullRequest/BitbucketPullRequestApi.ts | 20 +- .../BitbucketPullRequestProvider.ts | 1 + .../pullRequest/GitHubPullRequestCli.test.ts | 68 ++++ .../src/pullRequest/GitHubPullRequestCli.ts | 18 +- .../GitHubPullRequestProvider.test.ts | 24 ++ .../pullRequest/GitHubPullRequestProvider.ts | 41 +-- .../pullRequest/GiteaPullRequestApi.test.ts | 322 +++++++++++++++++- .../src/pullRequest/GiteaPullRequestApi.ts | 121 ++++++- .../pullRequest/GiteaPullRequestProvider.ts | 15 +- .../src/pullRequest/PullRequestProvider.ts | 37 ++ 14 files changed, 725 insertions(+), 46 deletions(-) diff --git a/apps/server/src/pullRequest/AzureDevOpsPullRequestCli.test.ts b/apps/server/src/pullRequest/AzureDevOpsPullRequestCli.test.ts index d893924b3f2a..4e1a502d1509 100644 --- a/apps/server/src/pullRequest/AzureDevOpsPullRequestCli.test.ts +++ b/apps/server/src/pullRequest/AzureDevOpsPullRequestCli.test.ts @@ -1,6 +1,7 @@ import { afterEach, assert, expect, it, vi } from "@effect/vitest"; import * as Effect from "effect/Effect"; import * as Layer from "effect/Layer"; +import * as Schema from "effect/Schema"; import { ChildProcessSpawner } from "effect/unstable/process"; import * as AzureDevOpsCli from "../sourceControl/AzureDevOpsCli.ts"; @@ -61,6 +62,34 @@ afterEach(() => { }); layer("AzureDevOpsPullRequestCli.layer", (it) => { + it.effect("bounds dependency discovery when full raw pages contain no usable rows", () => + Effect.gen(function* () { + mockedExecute.mockReturnValue( + Effect.succeed( + output( + yield* Schema.encodeEffect(Schema.fromJsonString(Schema.Unknown))( + Array.from({ length: 201 }, () => ({ pullRequestId: "invalid" })), + ), + ), + ), + ); + const cli = yield* AzureDevOpsPullRequestCli.AzureDevOpsPullRequestCli; + const batch = yield* cli.listPullRequests({ + cwd: "/w", + repository: "web", + state: "open", + involvement: "all", + viewer: "", + limit: 200, + relationshipOnly: true, + }); + expect(mockedExecute).toHaveBeenCalledTimes(4); + expect(batch.items).toHaveLength(0); + expect(batch.truncated).toBe(true); + expect(batch.cursorAdvance).toBe(804); + }), + ); + it.effect("asks for one row more than the page, to probe for a next page", () => Effect.gen(function* () { mockedExecute.mockReturnValueOnce(Effect.succeed(output(pullRequests(3, 1)))); diff --git a/apps/server/src/pullRequest/AzureDevOpsPullRequestCli.ts b/apps/server/src/pullRequest/AzureDevOpsPullRequestCli.ts index fe87692e1cc3..a82b21c1bb15 100644 --- a/apps/server/src/pullRequest/AzureDevOpsPullRequestCli.ts +++ b/apps/server/src/pullRequest/AzureDevOpsPullRequestCli.ts @@ -121,6 +121,7 @@ export class AzureDevOpsPullRequestCli extends Context.Service< }) => Effect.Effect; readonly listPullRequests: (input: { + readonly relationshipOnly?: boolean | undefined; readonly cwd: string; readonly repository: string; readonly state: PullRequestListState; @@ -294,6 +295,8 @@ export const make = Effect.gen(function* () { readonly skip: number; readonly cursorAdvance: number; readonly items: ReadonlyArray; + readonly relationshipOnly?: boolean | undefined; + readonly page: number; }): Effect.Effect< { readonly items: ReadonlyArray; @@ -363,8 +366,16 @@ export const make = Effect.gen(function* () { cursorAdvance: input.cursorAdvance + decoded.success.rawCount, }); } + if (input.relationshipOnly === true && input.page >= 4) { + return Effect.succeed({ + items, + truncated: true, + cursorAdvance: input.cursorAdvance + decoded.success.rawCount, + }); + } return listPullRequestPage({ ...input, + page: input.page + 1, skip: input.skip + decoded.success.rawCount, cursorAdvance: input.cursorAdvance + decoded.success.rawCount, items, @@ -411,6 +422,8 @@ export const make = Effect.gen(function* () { skip: input.cursor?.delivered ?? 0, cursorAdvance: 0, items: [], + page: 1, + relationshipOnly: input.relationshipOnly, }), getPullRequest: (input) => diff --git a/apps/server/src/pullRequest/AzureDevOpsPullRequestProvider.ts b/apps/server/src/pullRequest/AzureDevOpsPullRequestProvider.ts index 932e80fbda23..9255c5aac9af 100644 --- a/apps/server/src/pullRequest/AzureDevOpsPullRequestProvider.ts +++ b/apps/server/src/pullRequest/AzureDevOpsPullRequestProvider.ts @@ -145,6 +145,7 @@ export const make = Effect.gen(function* () { viewer: input.viewer, limit: input.limit, cursor: input.cursor, + relationshipOnly: input.relationshipOnly, }) .pipe( Effect.mapError(fail("listChangeRequests")), diff --git a/apps/server/src/pullRequest/BitbucketPullRequestApi.test.ts b/apps/server/src/pullRequest/BitbucketPullRequestApi.test.ts index 8945ecc5e1e2..f93a11b0add9 100644 --- a/apps/server/src/pullRequest/BitbucketPullRequestApi.test.ts +++ b/apps/server/src/pullRequest/BitbucketPullRequestApi.test.ts @@ -83,6 +83,32 @@ afterEach(() => { }); layer("BitbucketPullRequestApi.layer", (it) => { + it.effect("bounds dependency discovery and retains omissions from earlier pages", () => + Effect.gen(function* () { + const next = "https://api.bitbucket.org/2.0/repositories/acme/web/pullrequests?page=2"; + mockedRequest.mockReturnValueOnce( + Effect.succeed(response(valuePage([{ id: "invalid" }], next))), + ); + mockedRequest.mockReturnValueOnce(Effect.succeed(response(page(1, 7)))); + const api = yield* BitbucketPullRequestApi.BitbucketPullRequestApi; + const input = { + repository: "acme/web", + state: "open" as const, + limit: 200, + relationshipOnly: true, + }; + const batch = yield* api.listPullRequests(input); + expect(batch.items).toHaveLength(1); + expect(batch.truncated).toBe(true); + expect(mockedRequest).toHaveBeenCalledTimes(2); + mockedRequest.mockClear(); + mockedRequest.mockReturnValue(Effect.succeed(response(valuePage([{ id: "invalid" }], next)))); + const bounded = yield* api.listPullRequests(input); + expect(mockedRequest).toHaveBeenCalledTimes(4); + expect(bounded.truncated).toBe(true); + }), + ); + it.effect("asks for reviewers, newest first, at Bitbucket's page ceiling", () => Effect.gen(function* () { mockedRequest.mockReturnValueOnce(Effect.succeed(response(page(3, 1)))); @@ -144,6 +170,41 @@ layer("BitbucketPullRequestApi.layer", (it) => { }), ); + it.effect("marks a page with a skipped row incomplete even without a next cursor", () => + Effect.gen(function* () { + mockedRequest.mockReturnValueOnce( + Effect.succeed( + response( + valuePage([ + { id: "not a number" }, + { + id: 7, + title: "Usable row", + state: "OPEN", + created_on: "2026-06-16T05:04:32+00:00", + updated_on: "2026-06-16T05:04:33+00:00", + source: { branch: { name: "feat/page" } }, + destination: { branch: { name: "master" } }, + links: { html: { href: "https://bitbucket.org/acme/web/pull-requests/7" } }, + }, + ]), + ), + ), + ); + const api = yield* BitbucketPullRequestApi.BitbucketPullRequestApi; + + const batch = yield* api.listPullRequests({ + repository: "acme/web", + state: "open", + limit: 50, + relationshipOnly: true, + }); + + assert.strictEqual(batch.items.length, 1); + assert.isTrue(batch.truncated); + }), + ); + it.effect("counts the rows it walked past as more to come", () => Effect.gen(function* () { // Bitbucket pages in fifties whatever was asked for, so a request for ninety-nine reads a diff --git a/apps/server/src/pullRequest/BitbucketPullRequestApi.ts b/apps/server/src/pullRequest/BitbucketPullRequestApi.ts index 5b3149b0d75c..4d79ddb8881b 100644 --- a/apps/server/src/pullRequest/BitbucketPullRequestApi.ts +++ b/apps/server/src/pullRequest/BitbucketPullRequestApi.ts @@ -148,6 +148,7 @@ export class BitbucketPullRequestApi extends Context.Service< readonly getViewer: () => Effect.Effect; readonly listPullRequests: (input: { + readonly relationshipOnly?: boolean | undefined; readonly repository: string; readonly state: PullRequestListState; readonly limit: number; @@ -405,6 +406,8 @@ export const make = Effect.gen(function* () { readonly limit: number; readonly page: number; readonly collected: ReadonlyArray; + readonly relationshipOnly?: boolean | undefined; + readonly incomplete: boolean; }): Effect.Effect => bitbucket.request({ method: "GET", url: input.url }).pipe( Effect.flatMap((response) => { @@ -419,16 +422,25 @@ export const make = Effect.gen(function* () { } const collected = [...input.collected, ...decoded.success.items]; const next = decoded.success.next; - if (next === null || collected.length >= input.limit || input.page >= MAX_LIST_PAGES) { + const incomplete = + input.incomplete || decoded.success.rawCount !== decoded.success.items.length; + if ( + next === null || + collected.length >= input.limit || + input.page >= (input.relationshipOnly === true ? 4 : MAX_LIST_PAGES) + ) { return Effect.succeed({ items: collected.slice(0, input.limit), // Bitbucket pages in fifties whatever was asked for, so a walk that stopped on the // count rather than on the last page is holding rows it is about to drop. Those are // more results just as surely as another page would be. - truncated: next !== null || collected.length > input.limit, + truncated: + next !== null || + collected.length > input.limit || + (input.relationshipOnly === true && incomplete), }); } - return listPage({ ...input, url: next, page: input.page + 1, collected }); + return listPage({ ...input, url: next, page: input.page + 1, collected, incomplete }); }), ); @@ -562,6 +574,8 @@ export const make = Effect.gen(function* () { limit: input.limit, page: 1, collected: [], + incomplete: false, + relationshipOnly: input.relationshipOnly, }); }), diff --git a/apps/server/src/pullRequest/BitbucketPullRequestProvider.ts b/apps/server/src/pullRequest/BitbucketPullRequestProvider.ts index 47d41eeee6d9..5d2753d8b49e 100644 --- a/apps/server/src/pullRequest/BitbucketPullRequestProvider.ts +++ b/apps/server/src/pullRequest/BitbucketPullRequestProvider.ts @@ -133,6 +133,7 @@ export const make = Effect.gen(function* () { limit: input.limit, query: input.query, cursor: input.cursor, + relationshipOnly: input.relationshipOnly, }) .pipe( Effect.mapError(fail("listChangeRequests")), diff --git a/apps/server/src/pullRequest/GitHubPullRequestCli.test.ts b/apps/server/src/pullRequest/GitHubPullRequestCli.test.ts index 9ca19d3eda17..7efd7b928a25 100644 --- a/apps/server/src/pullRequest/GitHubPullRequestCli.test.ts +++ b/apps/server/src/pullRequest/GitHubPullRequestCli.test.ts @@ -183,6 +183,74 @@ afterEach(() => { }); layer("GitHubPullRequestCli.layer", (it) => { + it.effect( + "relationship discovery uses one bounded repository listing without search fallback", + () => + Effect.gen(function* () { + mockedExecute.mockReturnValueOnce(Effect.succeed(output("[]"))); + const cli = yield* GitHubPullRequestCli.GitHubPullRequestCli; + const batch = yield* cli.listPullRequests({ + cwd: "/w", + repository: "acme/web", + host: "github.com", + state: "open", + involvement: "all", + viewer: "reader", + limit: 200, + relationshipOnly: true, + }); + expect(batch).toEqual({ items: [], truncated: false, continues: false }); + expect(mockedExecute).toHaveBeenCalledTimes(1); + expect(callAt(0).args).not.toContain("--search"); + expect(callAt(0).args).toContain("201"); + }), + ); + + it.effect("a skipped relationship row keeps coverage partial without fetching replacements", () => + Effect.gen(function* () { + mockedExecute.mockReturnValueOnce( + Effect.succeed(output(pullRequests(201, 1, () => ({ number: "invalid" })))), + ); + const cli = yield* GitHubPullRequestCli.GitHubPullRequestCli; + const batch = yield* cli.listPullRequests({ + cwd: "/w", + repository: "acme/web", + host: "github.com", + state: "open", + involvement: "all", + viewer: "reader", + limit: 200, + relationshipOnly: true, + }); + expect(batch.items).toHaveLength(0); + expect(batch.truncated).toBe(true); + expect(mockedExecute).toHaveBeenCalledTimes(1); + }), + ); + + it.effect("caps a larger relationship request and reports the unread remainder", () => + Effect.gen(function* () { + mockedExecute.mockReturnValueOnce(Effect.succeed(output(pullRequests(201, 1)))); + const cli = yield* GitHubPullRequestCli.GitHubPullRequestCli; + const batch = yield* cli.listPullRequests({ + cwd: "/w", + repository: "acme/web", + host: "github.com", + state: "open", + involvement: "all", + viewer: "reader", + limit: 1_000, + relationshipOnly: true, + }); + + expect(batch.items).toHaveLength(201); + expect(batch.truncated).toBe(true); + expect(mockedExecute).toHaveBeenCalledTimes(1); + const args = callAt(0).args; + expect(args[args.indexOf("--limit") + 1]).toBe("201"); + }), + ); + it.effect("reads linked pull request status through one narrow request", () => Effect.gen(function* () { mockedGetPullRequest.mockReturnValueOnce( diff --git a/apps/server/src/pullRequest/GitHubPullRequestCli.ts b/apps/server/src/pullRequest/GitHubPullRequestCli.ts index 5d5c2062c08c..4faaba2c66cf 100644 --- a/apps/server/src/pullRequest/GitHubPullRequestCli.ts +++ b/apps/server/src/pullRequest/GitHubPullRequestCli.ts @@ -355,6 +355,7 @@ const DIFF_FILE_MAX_OUTPUT_BYTES = 1024 * 1024; /** A search-free fallback may scan older rows for local filters, but never the whole repository. */ const PULL_REQUEST_FALLBACK_MAX_ROWS = 1_000; +const RELATIONSHIP_ONLY_MAX_ROWS = 201; /** What the files API serves at most in one response, which is what one slice is made of. */ const DIFF_FILES_PAGE_SIZE = 100; @@ -411,6 +412,7 @@ export class GitHubPullRequestCli extends Context.Service< }) => Effect.Effect; readonly listPullRequests: (input: { + readonly relationshipOnly?: boolean | undefined; readonly cwd: string; readonly repository: string; readonly host: string; @@ -1504,6 +1506,7 @@ export const make = Effect.gen(function* () { : decoded.success.items.filter((item) => matchesUnsortedListing(item, input)); if ( !continues && + input.relationshipOnly !== true && items.length < input.limit && decoded.success.rawCount >= requestedRows && requestedRows < fallbackMaxRows @@ -1515,9 +1518,13 @@ export const make = Effect.gen(function* () { items: items.slice(0, input.limit), // One row over the page size is the probe for a next page, and it is // counted before decoding: a skipped malformed row must not end paging. - truncated: continues - ? decoded.success.rawCount > input.limit - : items.length > input.limit || decoded.success.rawCount >= requestedRows, + truncated: + input.relationshipOnly === true + ? decoded.success.rawCount >= requestedRows || + decoded.success.rawCount !== items.length + : continues + ? decoded.success.rawCount > input.limit + : items.length > input.limit || decoded.success.rawCount >= requestedRows, continues, }); } @@ -1548,6 +1555,11 @@ export const make = Effect.gen(function* () { // the same read rather than a wider one. Free text is the one thing the fallback cannot // judge locally — it lists rows, it does not search their text — so a query still rules // the fallback out: an empty answer under one is already the answer. + // Relationship discovery needs the repository collection, not its eventually indexed search. + // A single bounded CLI listing uses at most three 100-row underlying pages. + if (input.relationshipOnly === true) { + return read(false, Math.min(input.limit + 1, RELATIONSHIP_ONLY_MAX_ROWS)); + } const hasQuery = (input.query?.trim().length ?? 0) > 0; return read(true).pipe( Effect.flatMap((batch) => diff --git a/apps/server/src/pullRequest/GitHubPullRequestProvider.test.ts b/apps/server/src/pullRequest/GitHubPullRequestProvider.test.ts index 8fd0f09dd3ea..18a14d705a51 100644 --- a/apps/server/src/pullRequest/GitHubPullRequestProvider.test.ts +++ b/apps/server/src/pullRequest/GitHubPullRequestProvider.test.ts @@ -8,6 +8,30 @@ import * as GitHubPullRequestCli from "./GitHubPullRequestCli.ts"; import { gitHubViewerPermissions, loginAvatarUrl, make } from "./GitHubPullRequestProvider.ts"; import type { GitHubReviewThreadComments } from "./gitHubPullRequestJson.ts"; +it.effect("dependency listings do not spend host requests loading actor avatars", () => + Effect.gen(function* () { + const provider = yield* make.pipe( + Effect.provide( + Layer.mock(GitHubPullRequestCli.GitHubPullRequestCli)({ + listPullRequests: () => Effect.succeed({ items: [], truncated: false, continues: false }), + listActorAvatars: () => Effect.die("relationship read requested avatars"), + }), + ), + ); + const page = yield* provider.listChangeRequests({ + cwd: "/w", + host: "github.com", + repository: "acme/web", + state: "open", + involvement: "all", + viewer: "reader", + limit: 200, + relationshipOnly: true, + }); + expect(page).toEqual({ items: [], truncated: false, continues: false }); + }), +); + it.effect("uses one narrow read for a linked pull request summary", () => Effect.gen(function* () { let summaryReads = 0; diff --git a/apps/server/src/pullRequest/GitHubPullRequestProvider.ts b/apps/server/src/pullRequest/GitHubPullRequestProvider.ts index e75ef3547c04..b700e554b4ce 100644 --- a/apps/server/src/pullRequest/GitHubPullRequestProvider.ts +++ b/apps/server/src/pullRequest/GitHubPullRequestProvider.ts @@ -223,6 +223,7 @@ export const make = Effect.gen(function* () { involvement: input.involvement, viewer: input.viewer, limit: input.limit, + relationshipOnly: input.relationshipOnly, query: input.query, cursor: input.cursor, filters: input.filters, @@ -230,25 +231,27 @@ export const make = Effect.gen(function* () { .pipe( Effect.mapError(fail("listChangeRequests")), Effect.flatMap((page) => - cli - .listActorAvatars({ - cwd: input.cwd, - repository: input.repository, - host: input.host, - ids: [...new Set(page.items.flatMap((item) => item.authorId ?? []))], - }) - // A listing without faces is still a listing, so a failed lookup falls back to - // the initials rather than taking the rows down with it. - .pipe( - Effect.orElseSucceed(() => new Map()), - Effect.map((avatarsByLogin) => ({ - ...page, - items: page.items.map((item) => ({ - ...item, - author: withAvatar(item.author, avatarsByLogin, input.host), - })), - })), - ), + input.relationshipOnly + ? Effect.succeed(page) + : cli + .listActorAvatars({ + cwd: input.cwd, + repository: input.repository, + host: input.host, + ids: [...new Set(page.items.flatMap((item) => item.authorId ?? []))], + }) + // A listing without faces is still a listing, so a failed lookup falls back to + // the initials rather than taking the rows down with it. + .pipe( + Effect.orElseSucceed(() => new Map()), + Effect.map((avatarsByLogin) => ({ + ...page, + items: page.items.map((item) => ({ + ...item, + author: withAvatar(item.author, avatarsByLogin, input.host), + })), + })), + ), ), ), diff --git a/apps/server/src/pullRequest/GiteaPullRequestApi.test.ts b/apps/server/src/pullRequest/GiteaPullRequestApi.test.ts index 81c201979066..64c1b9205a34 100644 --- a/apps/server/src/pullRequest/GiteaPullRequestApi.test.ts +++ b/apps/server/src/pullRequest/GiteaPullRequestApi.test.ts @@ -144,12 +144,30 @@ it.effect("keeps a search hydration transport failure fatal", () => ); layer("GiteaPullRequestApi", (it) => { - it.effect("reconstructs auto-merge from the timeline when discovery is unavailable", () => Effect.gen(function* () { - mockedRequest.mockReturnValueOnce(Effect.fail(new GiteaApi.GiteaApiError({operation: "getFeatures", reason: "failed", detail: "temporarily unavailable"}))).mockReturnValueOnce(Effect.succeed(response([{id: 1, type: "pull_scheduled_merge"}]))); - const api = yield* GiteaPullRequestApi.make; - expect(yield* api.getAutoMergeEnabled({host: "forge.example.test", repository: "acme/web", number: 7})).toBe(true); - expect(callAt(1).path).toContain("/timeline?"); - })); + it.effect("reconstructs auto-merge from the timeline when discovery is unavailable", () => + Effect.gen(function* () { + mockedRequest + .mockReturnValueOnce( + Effect.fail( + new GiteaApi.GiteaApiError({ + operation: "getFeatures", + reason: "failed", + detail: "temporarily unavailable", + }), + ), + ) + .mockReturnValueOnce(Effect.succeed(response([{ id: 1, type: "pull_scheduled_merge" }]))); + const api = yield* GiteaPullRequestApi.make; + expect( + yield* api.getAutoMergeEnabled({ + host: "forge.example.test", + repository: "acme/web", + number: 7, + }), + ).toBe(true); + expect(callAt(1).path).toContain("/timeline?"); + }), + ); it.effect("approves only the current pull request's waiting workflow runs", () => Effect.gen(function* () { const pull = rawPullRequest(7); @@ -342,6 +360,227 @@ layer("GiteaPullRequestApi", (it) => { }), ); + it.effect("marks a bounded dependency read partial when an exhausted page skips a row", () => + Effect.gen(function* () { + mockedRequest.mockReturnValueOnce( + Effect.succeed( + response([rawPullRequest(1), { number: "broken" }], { "x-total-count": "2" }), + ), + ); + const api = yield* GiteaPullRequestApi.make; + const page = yield* api.listPullRequests({ + host: "forge.example.test", + repository: "acme/web", + state: "open", + involvement: "all", + viewer: "", + limit: 200, + relationshipOnly: true, + }); + + expect(page.items.map((item) => item.number)).toEqual([1]); + assert.strictEqual(page.consumed, 2); + assert.isTrue(page.truncated); + assert.strictEqual(mockedRequest.mock.calls.length, 1); + }), + ); + + it.effect("returns a partial dependency page after four raw pages", () => + Effect.gen(function* () { + mockedRequest.mockImplementation(() => + Effect.succeed( + response( + Array.from({ length: 50 }, () => ({ number: "broken" })), + { + "x-total-count": "250", + }, + ), + ), + ); + const api = yield* GiteaPullRequestApi.make; + const page = yield* api.listPullRequests({ + host: "forge.example.test", + repository: "acme/web", + state: "open", + involvement: "all", + viewer: "", + limit: 200, + relationshipOnly: true, + }); + + expect(page.items).toEqual([]); + assert.strictEqual(page.consumed, 200); + assert.isTrue(page.truncated); + assert.strictEqual(mockedRequest.mock.calls.length, 4); + expect(callAt(3).path).toContain("page=4"); + }), + ); + + it.effect("uses the stored branch label without treating an internal pull ref as live", () => + Effect.gen(function* () { + // Field projection follows the captured deleted-fork fixture. Keeping the source repository + // here models the separately source-verified deleted-branch case for an open pull request. + mockedRequest.mockReturnValueOnce( + Effect.succeed( + response( + [ + rawPullRequest(9, { + base: { + ref: "main", + repo_id: 5, + repo: { id: 5, full_name: "acme/web" }, + }, + head: { + ref: "refs/pull/9/head", + label: "fork-head", + repo_id: 6, + repo: { id: 6, full_name: "acme/web-fork" }, + }, + }), + ], + { "x-total-count": "1" }, + ), + ), + ); + const api = yield* GiteaPullRequestApi.make; + const page = yield* api.listPullRequests({ + host: "forge.example.test", + repository: "acme/web", + state: "open", + involvement: "all", + viewer: "", + limit: 200, + relationshipOnly: true, + }); + + assert.strictEqual(page.items[0]?.relationshipHeadBranch, "fork-head"); + assert.isFalse(page.items[0]?.headBranchAvailable); + assert.strictEqual(page.items[0]?.headRepositoryNameWithOwner, "acme/web-fork"); + assert.isFalse(page.truncated); + expect(callAt(0).path).toContain("sort=oldest"); + }), + ); + + it.effect("marks duplicate dependency identities and inconsistent counts partial", () => + Effect.gen(function* () { + mockedRequest.mockReturnValueOnce( + Effect.succeed(response([rawPullRequest(1), rawPullRequest(1)], { "x-total-count": "1" })), + ); + const api = yield* GiteaPullRequestApi.make; + const page = yield* api.listPullRequests({ + host: "forge.example.test", + repository: "acme/web", + state: "open", + involvement: "all", + viewer: "", + limit: 200, + relationshipOnly: true, + }); + + expect(page.items.map((item) => item.number)).toEqual([1]); + assert.strictEqual(page.consumed, 2); + assert.isTrue(page.truncated); + }), + ); + + it.effect("rejects a mismatched dependency target without claiming complete coverage", () => + Effect.gen(function* () { + mockedRequest.mockReturnValueOnce( + Effect.succeed( + response( + [ + rawPullRequest(1, { + base: { ref: "main", repo: { id: 9, full_name: "other/web" } }, + }), + ], + { "x-total-count": "1" }, + ), + ), + ); + const api = yield* GiteaPullRequestApi.make; + const page = yield* api.listPullRequests({ + host: "forge.example.test", + repository: "acme/web", + state: "open", + involvement: "all", + viewer: "", + limit: 200, + relationshipOnly: true, + }); + + expect(page.items).toEqual([]); + assert.isTrue(page.truncated); + }), + ); + + it.effect("keeps conflicting repository IDs partial even when full names agree", () => + Effect.gen(function* () { + mockedRequest.mockReturnValueOnce( + Effect.succeed( + response( + [ + rawPullRequest(1, { + base: { ref: "main", repo: { id: 9, full_name: "acme/web" } }, + head: { ref: "feature", label: "feature", repo: { id: 10, full_name: "acme/web" } }, + }), + ], + { "x-total-count": "1" }, + ), + ), + ); + const api = yield* GiteaPullRequestApi.make; + const page = yield* api.listPullRequests({ + host: "forge.example.test", + repository: "acme/web", + state: "open", + involvement: "all", + viewer: "", + limit: 200, + relationshipOnly: true, + }); + expect(page.items[0]?.headRepositoryNameWithOwner).toBe("acme/web"); + expect(page.items[0]?.headRepositoryId).toBe(10); + expect(page.truncated).toBe(true); + }), + ); + + it.effect("retains conflicting repo_id evidence when nested repository IDs are null", () => + Effect.gen(function* () { + mockedRequest.mockReturnValueOnce( + Effect.succeed( + response( + [ + rawPullRequest(1, { + base: { ref: "main", repo_id: 9, repo: { id: null, full_name: "acme/web" } }, + head: { + ref: "feature", + label: "feature", + repo_id: 10, + repo: { id: null, full_name: "acme/web" }, + }, + }), + ], + { "x-total-count": "1" }, + ), + ), + ); + const api = yield* GiteaPullRequestApi.make; + const page = yield* api.listPullRequests({ + host: "forge.example.test", + repository: "acme/web", + state: "open", + involvement: "all", + viewer: "", + limit: 200, + relationshipOnly: true, + }); + + expect(page.items[0]?.baseRepositoryId).toBe(9); + expect(page.items[0]?.headRepositoryId).toBe(10); + expect(page.truncated).toBe(true); + }), + ); + it.effect("walks later pages until involvement filtering fills the requested slice", () => Effect.gen(function* () { mockedRequest @@ -402,6 +641,77 @@ layer("GiteaPullRequestApi", (it) => { }), ); + it.effect("continues from a server-clamped relationship page", () => + Effect.gen(function* () { + mockedRequest.mockImplementation((request) => { + const url = new URL(request.path, "https://forge.example.test"); + const page = Number(url.searchParams.get("page")); + const first = (page - 1) * 20 + 1; + return Effect.succeed( + response( + Array.from({ length: 20 }, (_, index) => rawPullRequest(first + index)), + { + "x-total-count": "200", + ...(page < 10 + ? { + link: `; rel="next"`, + } + : {}), + }, + ), + ); + }); + const api = yield* GiteaPullRequestApi.make; + const page = yield* api.listPullRequests({ + host: "forge.example.test", + repository: "acme/web", + state: "open", + involvement: "all", + viewer: "reviewer", + limit: 200, + relationshipOnly: true, + cursor: { + updatedBefore: "2026-09-02T10:20:00.000Z", + delivered: 80, + }, + }); + + expect(page.items).toHaveLength(60); + assert.strictEqual(page.items[0]?.number, 81); + assert.strictEqual(page.items[59]?.number, 140); + assert.strictEqual(page.consumed, 60); + assert.isTrue(page.truncated); + expect(mockedRequest).toHaveBeenCalledTimes(4); + expect(callAt(0).path).toContain("page=1"); + expect(callAt(1).path).toContain("page=5"); + expect(callAt(3).path).toContain("page=7"); + }), + ); + + it.effect("keeps relationship discovery with a query on the bounded listing path", () => + Effect.gen(function* () { + mockedRequest.mockReturnValueOnce( + Effect.succeed(response([rawPullRequest(1)], { "x-total-count": "1" })), + ); + const api = yield* GiteaPullRequestApi.make; + const page = yield* api.listPullRequests({ + host: "forge.example.test", + repository: "acme/web", + state: "open", + involvement: "all", + viewer: "reviewer", + limit: 200, + query: "ignored for relationship discovery", + relationshipOnly: true, + }); + + expect(page.items.map((item) => item.number)).toEqual([1]); + expect(mockedRequest).toHaveBeenCalledTimes(1); + expect(callAt(0).path).toContain("/repos/acme/web/pulls?"); + expect(callAt(0).path).not.toContain("/issues?"); + }), + ); + it.effect("uses native issue search and hydrates its pull request summaries", () => Effect.gen(function* () { mockedRequest diff --git a/apps/server/src/pullRequest/GiteaPullRequestApi.ts b/apps/server/src/pullRequest/GiteaPullRequestApi.ts index 9885b7f44879..236256e4e7c9 100644 --- a/apps/server/src/pullRequest/GiteaPullRequestApi.ts +++ b/apps/server/src/pullRequest/GiteaPullRequestApi.ts @@ -43,6 +43,7 @@ import { dedupeChecks } from "./pullRequestChecks.ts"; const PAGE_SIZE = 50; const CONVERSATION_PAGES = 4; +const DEPENDENCY_PAGINATION_PAGES = 4; const MAX_PAGINATION_PAGES = 100; const DIFF_MAX_BYTES = 8 * 1024 * 1024; // Issue search rows do not carry branches or full state, so hydrate them without opening an @@ -56,6 +57,7 @@ const RawUser = Schema.Struct({ avatar_url: Schema.optional(Schema.NullOr(Schema.String)), }); const RawRepository = Schema.Struct({ + id: Schema.optional(Schema.NullOr(Schema.Int)), full_name: Schema.optional(Schema.String), allow_merge_commits: Schema.optional(Schema.Boolean), allow_squash_merge: Schema.optional(Schema.Boolean), @@ -74,7 +76,9 @@ const RawRepository = Schema.Struct({ }); const RawBranch = Schema.Struct({ ref: Schema.optional(Schema.String), + label: Schema.optional(Schema.NullOr(Schema.String)), sha: Schema.optional(Schema.String), + repo_id: Schema.optional(Schema.Int), repo: Schema.optional(Schema.NullOr(RawRepository)), }); const RawLabel = Schema.Struct({ @@ -215,9 +219,14 @@ export interface GiteaPullRequest { readonly url: string; readonly author: PullRequestActor | null; readonly headBranch: string; + readonly relationshipHeadBranch: string; + readonly headBranchAvailable: boolean; readonly headSha: string; readonly headRepositoryNameWithOwner: string | null; + readonly headRepositoryId: number | null; readonly baseBranch: string; + readonly baseRepositoryNameWithOwner: string | null; + readonly baseRepositoryId: number | null; readonly baseSha: string; readonly mergeBaseSha: string; readonly state: "open" | "closed" | "merged"; @@ -280,6 +289,7 @@ function actor(value: typeof RawUser.Type | null | undefined): PullRequestActor function pullRequest(value: RawPullRequest): GiteaPullRequest | null { const title = value.title.trim(); const headBranch = value.head.ref?.trim(); + const headLabel = value.head.label?.trim(); const baseBranch = value.base.ref?.trim(); const createdAt = iso(value.created_at); const updatedAt = iso(value.updated_at); @@ -296,9 +306,14 @@ function pullRequest(value: RawPullRequest): GiteaPullRequest | null { url: value.html_url, author: actor(value.user), headBranch, + relationshipHeadBranch: headLabel || headBranch, + headBranchAvailable: !headBranch.startsWith("refs/pull/"), headSha: value.head.sha?.trim() ?? "", headRepositoryNameWithOwner: value.head.repo?.full_name?.trim() || null, + headRepositoryId: value.head.repo?.id ?? value.head.repo_id ?? null, baseBranch, + baseRepositoryNameWithOwner: value.base.repo?.full_name?.trim() || null, + baseRepositoryId: value.base.repo?.id ?? value.base.repo_id ?? null, baseSha: value.base.sha?.trim() ?? "", mergeBaseSha: value.merge_base?.trim() ?? "", state: value.merged === true ? "merged" : value.state === "closed" ? "closed" : "open", @@ -465,6 +480,7 @@ export class GiteaPullRequestApi extends Context.Service< readonly limit: number; readonly query?: string; readonly cursor?: ProviderListCursor; + readonly relationshipOnly?: boolean; }) => Effect.Effect< { items: ReadonlyArray; @@ -973,8 +989,9 @@ export const make = Effect.gen(function* () { const listPullRequests: GiteaPullRequestApi["Service"]["listPullRequests"] = Effect.fn( "GiteaPullRequestApi.listPullRequests", )(function* (input) { + const relationshipOnly = input.relationshipOnly === true; const search = input.query?.trim(); - if (search !== undefined && search !== "") { + if (!relationshipOnly && search !== undefined && search !== "") { return yield* listSearchPullRequests({ ...input, query: search }); } const wanted = Math.max(1, input.limit); @@ -984,7 +1001,7 @@ export const make = Effect.gen(function* () { let page = 1; let path = query(`${basePath(input.repository)}/pulls`, { state: endpointState, - sort: "recentupdate", + sort: relationshipOnly ? "oldest" : "recentupdate", page, limit: PAGE_SIZE, ...(input.involvement === "authored" ? { poster: input.viewer } : {}), @@ -992,35 +1009,114 @@ export const make = Effect.gen(function* () { let rowsSeen = 0; let rowsSkipped = 0; let consumed = 0; + let relationshipEvidenceIncomplete = false; + let reportedRelationshipTotal: number | null = null; + const relationshipNumbers = new Set(); + const repositoryIdsByName = new Map(); + const expectedRepository = input.repository.trim().toLowerCase(); const collected: Array = []; - while (page <= MAX_PAGINATION_PAGES) { - const result = yield* readUnknownPage({ + const maxPages = relationshipOnly ? DEPENDENCY_PAGINATION_PAGES : MAX_PAGINATION_PAGES; + let pagesRead = 0; + let prefetchedPage: UnknownPage | null = null; + let next: string | null = null; + if (relationshipOnly && delivered > 0) { + prefetchedPage = yield* readUnknownPage({ operation: "listPullRequests", host: input.host, repository: input.repository, path, }); + pagesRead = 1; + const pageSize = prefetchedPage.rows.length; + if (pageSize > 0 && delivered >= pageSize) { + page = Math.floor(delivered / pageSize) + 1; + rowsSeen = (page - 1) * pageSize; + rowsSkipped = rowsSeen; + path = pathAtPage(path, page); + const pageTotal = totalCount(prefetchedPage.headers); + if (pageTotal === null) relationshipEvidenceIncomplete = true; + else reportedRelationshipTotal = pageTotal; + prefetchedPage = null; + } + } + while (pagesRead < maxPages || prefetchedPage !== null) { + const prefetched = prefetchedPage; + const result = + prefetched ?? + (yield* readUnknownPage({ + operation: "listPullRequests", + host: input.host, + repository: input.repository, + path, + })); + prefetchedPage = null; + if (prefetched === null) pagesRead += 1; rowsSeen += result.rows.length; + if (relationshipOnly) { + const pageTotal = totalCount(result.headers); + if (pageTotal === null) relationshipEvidenceIncomplete = true; + else if (reportedRelationshipTotal === null) reportedRelationshipTotal = pageTotal; + else if (pageTotal !== reportedRelationshipTotal) relationshipEvidenceIncomplete = true; + } const toSkip = Math.min(Math.max(0, delivered - rowsSkipped), result.rows.length); rowsSkipped += toSkip; const pageRows = result.rows.slice(toSkip); - const next = nextPagePath({ + next = nextPagePath({ path, page, pageRows: result.rows.length, rowsSeen, headers: result.headers, }); + if ( + relationshipOnly && + reportedRelationshipTotal !== null && + (rowsSeen > reportedRelationshipTotal || + (next === null && rowsSeen !== reportedRelationshipTotal)) + ) { + relationshipEvidenceIncomplete = true; + } for (const [index, row] of pageRows.entries()) { consumed += 1; const decoded = decodeRow(row); - if (Option.isNone(decoded)) continue; + if (Option.isNone(decoded)) { + relationshipEvidenceIncomplete = relationshipOnly || relationshipEvidenceIncomplete; + continue; + } const pr = pullRequest(decoded.value); - if (pr === null) continue; - if (!matchesPullRequest(pr, input.state, input.involvement, input.viewer)) continue; + if (pr === null) { + relationshipEvidenceIncomplete = relationshipOnly || relationshipEvidenceIncomplete; + continue; + } + if (relationshipOnly) { + const targetRepository = pr.baseRepositoryNameWithOwner?.toLowerCase(); + if (targetRepository === undefined || targetRepository !== expectedRepository) { + relationshipEvidenceIncomplete = true; + continue; + } + if (relationshipNumbers.has(pr.number)) { + relationshipEvidenceIncomplete = true; + continue; + } + relationshipNumbers.add(pr.number); + for (const [name, id] of [ + [pr.baseRepositoryNameWithOwner, pr.baseRepositoryId], + [pr.headRepositoryNameWithOwner, pr.headRepositoryId], + ] as const) { + if (name === null || id === null || id < 0) continue; + const key = name.toLowerCase(); + const observed = repositoryIdsByName.get(key); + if (observed === undefined) repositoryIdsByName.set(key, id); + else if (observed !== id) relationshipEvidenceIncomplete = true; + } + } + if (!matchesPullRequest(pr, input.state, input.involvement, input.viewer)) { + relationshipEvidenceIncomplete = relationshipOnly || relationshipEvidenceIncomplete; + continue; + } collected.push(pr); if (collected.length === wanted) { - if (page === MAX_PAGINATION_PAGES && next !== null) { + if (pagesRead === maxPages && next !== null && !relationshipOnly) { return yield* new GiteaPullRequestApiError({ operation: "listPullRequests", reason: "failed", @@ -1029,7 +1125,8 @@ export const make = Effect.gen(function* () { } return { items: collected, - truncated: index < pageRows.length - 1 || next !== null, + truncated: + relationshipEvidenceIncomplete || index < pageRows.length - 1 || next !== null, consumed, }; } @@ -1038,7 +1135,7 @@ export const make = Effect.gen(function* () { path = next; page += 1; } - if (page > MAX_PAGINATION_PAGES) { + if (next !== null && !relationshipOnly) { return yield* new GiteaPullRequestApiError({ operation: "listPullRequests", reason: "failed", @@ -1047,7 +1144,7 @@ export const make = Effect.gen(function* () { } return { items: collected, - truncated: false, + truncated: relationshipOnly && (relationshipEvidenceIncomplete || next !== null), consumed, }; }); diff --git a/apps/server/src/pullRequest/GiteaPullRequestProvider.ts b/apps/server/src/pullRequest/GiteaPullRequestProvider.ts index 108aa3ab9022..f970c0786905 100644 --- a/apps/server/src/pullRequest/GiteaPullRequestProvider.ts +++ b/apps/server/src/pullRequest/GiteaPullRequestProvider.ts @@ -93,13 +93,17 @@ export function giteaBaseComparison( return pullRequest.baseSha === pullRequest.mergeBaseSha ? "up-to-date" : "behind"; } -function toChangeRequest(pullRequest: GiteaPullRequestApi.GiteaPullRequest): ProviderChangeRequest { +function toChangeRequest( + pullRequest: GiteaPullRequestApi.GiteaPullRequest, + relationshipOnly = false, +): ProviderChangeRequest { return { number: pullRequest.number, title: pullRequest.title, url: pullRequest.url, author: pullRequest.author, - headBranch: pullRequest.headBranch, + headBranch: relationshipOnly ? pullRequest.relationshipHeadBranch : pullRequest.headBranch, + ...(relationshipOnly ? { headBranchAvailable: pullRequest.headBranchAvailable } : {}), headRepositoryNameWithOwner: pullRequest.headRepositoryNameWithOwner, baseBranch: pullRequest.baseBranch, state: pullRequest.state, @@ -162,11 +166,16 @@ export const make = Effect.gen(function* () { limit: input.limit, ...(input.query === undefined ? {} : { query: input.query }), ...(input.cursor === undefined ? {} : { cursor: input.cursor }), + ...(input.relationshipOnly === undefined + ? {} + : { relationshipOnly: input.relationshipOnly }), }) .pipe( Effect.mapError(fail("listChangeRequests")), Effect.map((page) => ({ - items: page.items.map(toChangeRequest), + items: page.items.map((pullRequest) => + toChangeRequest(pullRequest, input.relationshipOnly === true), + ), truncated: page.truncated, cursorAdvance: page.consumed, continues: true, diff --git a/apps/server/src/pullRequest/PullRequestProvider.ts b/apps/server/src/pullRequest/PullRequestProvider.ts index adf6f8d15b2b..28f059c53cff 100644 --- a/apps/server/src/pullRequest/PullRequestProvider.ts +++ b/apps/server/src/pullRequest/PullRequestProvider.ts @@ -70,6 +70,8 @@ export interface ProviderChangeRequest { readonly url: string; readonly author: PullRequestActor | null; readonly headBranch: string; + /** False when a provider can only expose a retained/internal ref, not a live source branch. */ + readonly headBranchAvailable?: boolean; readonly headRepositoryNameWithOwner?: string | null; readonly baseBranch: string; readonly state: PullRequestState; @@ -118,6 +120,29 @@ export interface ProviderChangeRequestPage { readonly continues: boolean; } +/** Optional host-native stack membership, independent from ordinary branch relationships. */ +export type ProviderNativeDependencyMembership = + | { + readonly status: "present"; + readonly id: string; + /** Host-defined order. Every member carries enough data to render a lightweight node. */ + readonly members: ReadonlyArray< + Pick< + ProviderChangeRequest, + | "number" + | "title" + | "url" + | "state" + | "isDraft" + | "headBranch" + | "baseBranch" + | "headRepositoryNameWithOwner" + > + >; + readonly coverage: "complete" | "partial"; + } + | { readonly status: "none" }; + /** * Where a repository's next slice starts, as the provider that has to ask for it needs it. Built * by the service out of the slice it just handed over, so the boundary that decides whether a row @@ -275,6 +300,8 @@ export interface PullRequestProviderApi { * what it gets for the fields a row carries. */ readonly filters?: PullRequestListFilters | undefined; + /** Skip decorations that do not contribute to repository-qualified branch relationships. */ + readonly relationshipOnly?: boolean | undefined; }, ) => Effect.Effect; @@ -304,6 +331,8 @@ export interface PullRequestProviderApi { readonly query?: string | undefined; readonly cursor?: ProviderListCursor | undefined; readonly filters?: PullRequestListFilters | undefined; + /** Skip decorations that do not contribute to repository-qualified branch relationships. */ + readonly relationshipOnly?: boolean | undefined; }) => Effect.Effect; /** @@ -324,6 +353,14 @@ export interface PullRequestProviderApi { input: ProviderRepositoryRef & { readonly number: number }, ) => Effect.Effect; + /** + * Explicit stack membership reported by the host. Optional and independent from branch-chain + * discovery: an absent implementation or failed read must not disable ordinary relationships. + */ + readonly getNativeDependencyMembership?: ( + input: ProviderRepositoryRef & { readonly number: number; readonly limit: number }, + ) => Effect.Effect; + /** * The cheap live fields used by linked threads. Optional because a provider without a narrow * endpoint can fall back to its full detail read at the service boundary.