diff --git a/apps/server/src/pullRequest/AzureDevOpsPullRequestProvider.ts b/apps/server/src/pullRequest/AzureDevOpsPullRequestProvider.ts index 8461e57d5685..932e80fbda23 100644 --- a/apps/server/src/pullRequest/AzureDevOpsPullRequestProvider.ts +++ b/apps/server/src/pullRequest/AzureDevOpsPullRequestProvider.ts @@ -81,6 +81,7 @@ function toChangeRequest(pullRequest: AzureDevOpsPullRequest): ProviderChangeReq url: pullRequest.url, author: pullRequest.author, headBranch: pullRequest.headBranch, + headRepositoryNameWithOwner: pullRequest.headRepositoryNameWithOwner, baseBranch: pullRequest.baseBranch, state: pullRequest.state, isDraft: pullRequest.isDraft, @@ -94,6 +95,9 @@ function toChangeRequest(pullRequest: AzureDevOpsPullRequest): ProviderChangeReq reviewRequestLogins: pullRequest.reviewRequestLogins, // Azure keeps labels on work items rather than on the pull request. labels: [], + ...(pullRequest.isCrossRepository === undefined + ? {} + : { isCrossRepository: pullRequest.isCrossRepository }), }; } diff --git a/apps/server/src/pullRequest/GitHubPullRequestCli.test.ts b/apps/server/src/pullRequest/GitHubPullRequestCli.test.ts index 1e6ca0ed43a6..9ca19d3eda17 100644 --- a/apps/server/src/pullRequest/GitHubPullRequestCli.test.ts +++ b/apps/server/src/pullRequest/GitHubPullRequestCli.test.ts @@ -2519,7 +2519,7 @@ layer("GitHubPullRequestCli.layer", (it) => { expect(detail.body).toBe("Core body"); expect(activity.author?.login).toBe("octocat"); expect(callAt(0).args.at(-1)).toBe( - "number,title,url,author,headRefName,baseRefName,state,isDraft,mergeable,reviewDecision,additions,deletions,createdAt,updatedAt,mergedAt,reviewRequests,labels,statusCheckRollup,body,changedFiles,closedAt,isCrossRepository,headRepositoryOwner,headRefOid,autoMergeRequest", + "number,title,url,author,headRefName,baseRefName,isCrossRepository,headRepository,headRepositoryOwner,state,isDraft,mergeable,reviewDecision,additions,deletions,createdAt,updatedAt,mergedAt,reviewRequests,labels,statusCheckRollup,body,changedFiles,closedAt,headRefOid,autoMergeRequest", ); expect(callAt(1).args.at(-1)).toBe("author,comments,reviews,commits"); }), diff --git a/apps/server/src/pullRequest/GitLabPullRequestCli.test.ts b/apps/server/src/pullRequest/GitLabPullRequestCli.test.ts index 014d91a02740..2a7174fe70ca 100644 --- a/apps/server/src/pullRequest/GitLabPullRequestCli.test.ts +++ b/apps/server/src/pullRequest/GitLabPullRequestCli.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 GitLabCli from "../sourceControl/GitLabCli.ts"; @@ -126,6 +127,44 @@ layer("GitLabPullRequestCli.layer", (it) => { }), ); + it.effect("uses the requested repository for a standard same-project REST row", () => + Effect.gen(function* () { + // Standard GitLab REST rows commonly carry only the numeric project ids. Equal ids prove + // this is the requested project; the request path supplies its qualified repository name. + mockedExecute.mockReturnValueOnce( + Effect.succeed( + output( + yield* Schema.encodeEffect(Schema.fromJsonString(Schema.Unknown))([ + { + iid: 7, + title: "Merge request 7", + web_url: "https://gitlab.com/acme/web/-/merge_requests/7", + source_branch: "feat/page", + target_branch: "main", + source_project_id: 100, + target_project_id: 100, + created_at: "2026-07-01T00:00:00Z", + updated_at: "2026-07-02T00:00:00Z", + }, + ]), + ), + ), + ); + const cli = yield* GitLabPullRequestCli.GitLabPullRequestCli; + + const batch = yield* cli.listMergeRequests({ + cwd: "/w", + repository: "acme/web", + state: "open", + involvement: "all", + viewer: "bilal", + limit: 10, + }); + + expect(batch.items[0]?.headRepositoryNameWithOwner).toBe("acme/web"); + }), + ); + it.effect("walks pages at a fixed size, because GitLab pages by offset", () => Effect.gen(function* () { mockedExecute @@ -869,6 +908,30 @@ layer("GitLabPullRequestCli.layer", (it) => { }), ); + it.effect("uses the requested repository for a standard same-project detail row", () => + Effect.gen(function* () { + mockedExecute.mockReturnValueOnce( + Effect.succeed( + output( + mergeRequestJson({ + source_project_id: 100, + target_project_id: 100, + }), + ), + ), + ); + const cli = yield* GitLabPullRequestCli.GitLabPullRequestCli; + + const detail = yield* cli.getMergeRequestDetail({ + cwd: "/w", + repository: "acme/web", + number: 7, + }); + + expect(detail.headRepositoryNameWithOwner).toBe("acme/web"); + }), + ); + it.effect("fails the read when GitLab returns something unreadable", () => Effect.gen(function* () { mockedExecute.mockReturnValueOnce(Effect.succeed(output('{"message":"404 Not Found"}'))); diff --git a/apps/server/src/pullRequest/GitLabPullRequestCli.ts b/apps/server/src/pullRequest/GitLabPullRequestCli.ts index f291cbd89c7f..ac0a0b29b028 100644 --- a/apps/server/src/pullRequest/GitLabPullRequestCli.ts +++ b/apps/server/src/pullRequest/GitLabPullRequestCli.ts @@ -596,7 +596,7 @@ export const make = Effect.gen(function* () { cursorAdvance: input.cursorAdvance, }); } - const decoded = decodeMergeRequestListJson(raw); + const decoded = decodeMergeRequestListJson(raw, input.repository); if (!Result.isSuccess(decoded)) { return Effect.fail( new GitLabMergeRequestReadError({ @@ -914,7 +914,7 @@ export const make = Effect.gen(function* () { ])}`, }).pipe( Effect.flatMap((result) => { - const decoded = decodeMergeRequestDetailJson(result.stdout.trim()); + const decoded = decodeMergeRequestDetailJson(result.stdout.trim(), input.repository); return Result.isSuccess(decoded) ? Effect.succeed(decoded.success) : Effect.fail( diff --git a/apps/server/src/pullRequest/GitLabPullRequestProvider.test.ts b/apps/server/src/pullRequest/GitLabPullRequestProvider.test.ts index 5d36d58dfc2b..45fd619a2399 100644 --- a/apps/server/src/pullRequest/GitLabPullRequestProvider.test.ts +++ b/apps/server/src/pullRequest/GitLabPullRequestProvider.test.ts @@ -68,6 +68,7 @@ describe("getChangeRequest base freshness", () => { url: "https://gitlab.com/acme/web/-/merge_requests/7", author: null, headBranch: "feat/page", + headRepositoryNameWithOwner: null, baseBranch: "main", state: "open" as const, isDraft: false, diff --git a/apps/server/src/pullRequest/azureDevOpsPullRequestJson.test.ts b/apps/server/src/pullRequest/azureDevOpsPullRequestJson.test.ts index 6f55eb937bc9..63077e742705 100644 --- a/apps/server/src/pullRequest/azureDevOpsPullRequestJson.test.ts +++ b/apps/server/src/pullRequest/azureDevOpsPullRequestJson.test.ts @@ -26,6 +26,8 @@ function pullRequest(overrides: Record = {}): Record { author: { login: "bilal@acme.dev", name: "Bilal Hassan" }, // Azure prefixes its refs, which no other host does. headBranch: "feat/page", + headRepositoryNameWithOwner: "platform/web", baseBranch: "main", state: "open", isDraft: false, @@ -144,6 +147,30 @@ describe("decodePullRequestJson", () => { ]); }); + it("uses Azure's fork source identity and leaves deleted sources unknown", () => { + const fork = expectSuccess( + decodePullRequestJson( + asJson( + pullRequest({ + forkSource: { + repository: { name: "web", project: { name: "contributor" } }, + }, + }), + ), + ), + ); + expect(fork).toMatchObject({ + headRepositoryNameWithOwner: "contributor/web", + isCrossRepository: true, + }); + + const deleted = expectSuccess( + decodePullRequestJson(asJson(pullRequest({ forkSource: { repository: null } }))), + ); + expect(deleted?.headRepositoryNameWithOwner).toBeNull(); + expect(deleted?.isCrossRepository).toBeUndefined(); + }); + it("reads auto-complete from whoever armed it, and its absence as nobody", () => { const armed = expectSuccess( decodePullRequestJson( diff --git a/apps/server/src/pullRequest/azureDevOpsPullRequestJson.ts b/apps/server/src/pullRequest/azureDevOpsPullRequestJson.ts index 55d9b544ab3d..51e8c39d424d 100644 --- a/apps/server/src/pullRequest/azureDevOpsPullRequestJson.ts +++ b/apps/server/src/pullRequest/azureDevOpsPullRequestJson.ts @@ -14,6 +14,8 @@ import { decodeJsonResult } from "@t3tools/shared/schemaJson"; import { azureDevOpsOrganizationBaseFromRestApiUrl, + azureDevOpsHeadRepositoryNameWithOwner, + azureDevOpsRepositoryNameWithOwner, azureDevOpsPullRequestWebUrl, } from "../sourceControl/azureDevOpsPullRequests.ts"; @@ -30,6 +32,18 @@ const RawIdentitySchema = Schema.Struct({ imageUrl: Schema.optional(Schema.NullOr(Schema.String)), }); +const RawRepositorySchema = Schema.Struct({ + name: Schema.optional(Schema.NullOr(Schema.String)), + webUrl: Schema.optional(Schema.NullOr(Schema.String)), + project: Schema.optional( + Schema.NullOr(Schema.Struct({ name: Schema.optional(Schema.NullOr(Schema.String)) })), + ), +}); + +const RawForkSourceSchema = Schema.Struct({ + repository: Schema.optional(Schema.NullOr(RawRepositorySchema)), +}); + const RawPullRequestSchema = Schema.Struct({ pullRequestId: Schema.Int, title: Schema.String, @@ -61,17 +75,9 @@ const RawPullRequestSchema = Schema.Struct({ creationDate: TrimmedNonEmptyString, closedDate: Schema.optional(Schema.NullOr(Schema.String)), url: Schema.optional(Schema.NullOr(Schema.String)), - repository: Schema.optional( - Schema.NullOr( - Schema.Struct({ - name: Schema.optional(Schema.NullOr(Schema.String)), - webUrl: Schema.optional(Schema.NullOr(Schema.String)), - project: Schema.optional( - Schema.NullOr(Schema.Struct({ name: Schema.optional(Schema.NullOr(Schema.String)) })), - ), - }), - ), - ), + repository: Schema.optional(Schema.NullOr(RawRepositorySchema)), + /** Non-null only when Azure identifies a pull request source fork. */ + forkSource: Schema.optional(Schema.NullOr(RawForkSourceSchema)), _links: Schema.optional( Schema.NullOr( Schema.Struct({ @@ -123,6 +129,10 @@ export interface AzureDevOpsPullRequest { readonly url: string; readonly author: PullRequestActor | null; readonly headBranch: string; + /** The qualified source repository, or null if Azure did not resolve it. */ + readonly headRepositoryNameWithOwner: string | null; + /** Derived from the source and target repository identities when both are known. */ + readonly isCrossRepository?: boolean; readonly baseBranch: string; readonly state: PullRequestState; readonly isDraft: boolean; @@ -154,6 +164,10 @@ function normalizeRefName(refName: string): string { return refName.trim().replace(/^refs\/heads\//, ""); } +function normalizeRepositoryIdentity(value: string): string { + return value.trim().toLowerCase(); +} + /** A login has to compare against `az account show`, which reports an email. */ function toActor(raw: Schema.Schema.Type | null | undefined) { const login = trimmed(raw?.uniqueName) ?? trimmed(raw?.displayName); @@ -224,6 +238,15 @@ function toAutoMergeMethod( function toPullRequest( raw: Schema.Schema.Type, ): AzureDevOpsPullRequest | null { + const headRepositoryNameWithOwner = azureDevOpsHeadRepositoryNameWithOwner(raw); + const targetRepositoryNameWithOwner = azureDevOpsRepositoryNameWithOwner(raw.repository); + const isCrossRepository = + headRepositoryNameWithOwner !== undefined && + headRepositoryNameWithOwner !== null && + targetRepositoryNameWithOwner !== null + ? normalizeRepositoryIdentity(headRepositoryNameWithOwner) !== + normalizeRepositoryIdentity(targetRepositoryNameWithOwner) + : undefined; const autoMergeMethod = toAutoMergeMethod(raw); const reviewers = (raw.reviewers ?? []).flatMap((reviewer) => { const actor = toActor(reviewer); @@ -249,6 +272,8 @@ function toPullRequest( url, author: toActor(raw.createdBy), headBranch, + headRepositoryNameWithOwner: headRepositoryNameWithOwner ?? null, + ...(typeof isCrossRepository === "boolean" ? { isCrossRepository } : {}), baseBranch, state: toState(raw), isDraft: raw.isDraft ?? false, diff --git a/apps/server/src/pullRequest/bitbucketPullRequestJson.test.ts b/apps/server/src/pullRequest/bitbucketPullRequestJson.test.ts index 356c5da96f33..e4c5ca45d9c5 100644 --- a/apps/server/src/pullRequest/bitbucketPullRequestJson.test.ts +++ b/apps/server/src/pullRequest/bitbucketPullRequestJson.test.ts @@ -106,6 +106,34 @@ describe("decodePullRequestPageJson", () => { }); describe("decodePullRequestJson", () => { + it("keeps a deleted source repository unresolved on detail responses", () => { + const decoded = expectSuccess( + decodePullRequestJson( + JSON.stringify( + pullRequest({ + source: { branch: { name: "feat/deleted" }, repository: null }, + }), + ), + ), + ); + + expect(decoded.headRepositoryNameWithOwner).toBeNull(); + }); + + it("does not treat a bare source repository name as a qualified identity", () => { + const decoded = expectSuccess( + decodePullRequestJson( + JSON.stringify( + pullRequest({ + source: { branch: { name: "feat/unqualified" }, repository: { full_name: "web" } }, + }), + ), + ), + ); + + expect(decoded.headRepositoryNameWithOwner).toBeNull(); + }); + it("reads reviewers as review requests", () => { const decoded = expectSuccess( decodePullRequestJson( diff --git a/apps/server/src/pullRequest/bitbucketPullRequestJson.ts b/apps/server/src/pullRequest/bitbucketPullRequestJson.ts index f0bc8a71a827..38cc11c1a866 100644 --- a/apps/server/src/pullRequest/bitbucketPullRequestJson.ts +++ b/apps/server/src/pullRequest/bitbucketPullRequestJson.ts @@ -203,6 +203,11 @@ function trimmed(value: string | null | undefined): string | null { return text.length > 0 ? text : null; } +function qualifiedRepository(value: string | null | undefined): string | null { + const repository = trimmed(value); + return repository?.includes("/") ? repository : null; +} + /** * Bitbucket stamps times as `+00:00` with microseconds. The page sorts change requests from * every host against each other as plain strings, so they are normalized to the same `Z` form @@ -293,7 +298,7 @@ function toPullRequest(raw: Schema.Schema.Type): Bi url: raw.links.html.href, author: toActor(raw.author), headBranch: raw.source.branch.name, - headRepositoryNameWithOwner: raw.source.repository?.full_name ?? null, + headRepositoryNameWithOwner: qualifiedRepository(raw.source.repository?.full_name), baseBranch: raw.destination.branch.name, state: toState(raw), isDraft: raw.draft ?? false, @@ -328,10 +333,15 @@ export interface BitbucketPage { readonly next: string | null; } +export interface BitbucketPullRequestPage extends BitbucketPage { + /** Rows before decoding; a skipped row cannot silently certify a complete relationship read. */ + readonly rawCount: number; +} + /** Malformed entries are skipped rather than failing the page, as on the other hosts. */ export function decodePullRequestPageJson( raw: string, -): Result.Result, DecodeFailure> { +): Result.Result { const decoded = decodePage(raw); if (!Result.isSuccess(decoded)) { return Result.fail(decoded.failure); @@ -343,7 +353,11 @@ export function decodePullRequestPageJson( items.push(toPullRequest(item.value)); } } - return Result.succeed({ items, next: trimmed(decoded.success.next) }); + return Result.succeed({ + items, + next: trimmed(decoded.success.next), + rawCount: decoded.success.values.length, + }); } export function decodePullRequestJson( diff --git a/apps/server/src/pullRequest/gitHubPullRequestJson.test.ts b/apps/server/src/pullRequest/gitHubPullRequestJson.test.ts index f6f5957f5875..d4654f223d70 100644 --- a/apps/server/src/pullRequest/gitHubPullRequestJson.test.ts +++ b/apps/server/src/pullRequest/gitHubPullRequestJson.test.ts @@ -73,6 +73,56 @@ describe("pull request list decoding", () => { expect(entry?.reviewRequestLogins).toEqual(["octocat"]); }); + it("maps a qualified head repository on list responses", () => { + const [entry] = expectSuccess( + decodePullRequestListJson( + listJson([ + { + headRepository: { nameWithOwner: "contributor/web", name: "web" }, + headRepositoryOwner: { login: "contributor" }, + isCrossRepository: true, + }, + ]), + ), + ).items; + + expect(entry).toMatchObject({ + headRepositoryNameWithOwner: "contributor/web", + isCrossRepository: true, + }); + }); + + it("uses the owner and repository fields when older gh output lacks nameWithOwner", () => { + const [entry] = expectSuccess( + decodePullRequestListJson( + listJson([ + { + headRepository: { name: "web" }, + headRepositoryOwner: { login: "acme" }, + }, + ]), + ), + ).items; + + expect(entry?.headRepositoryNameWithOwner).toBe("acme/web"); + }); + + it("does not turn a bare or deleted head repository into the target identity", () => { + const entries = expectSuccess( + decodePullRequestListJson( + listJson([ + { headRepository: { name: "web" }, headRepositoryOwner: null }, + { headRepository: null, headRepositoryOwner: null, isCrossRepository: true }, + ]), + ), + ).items; + + expect(entries.map((entry) => entry.headRepositoryNameWithOwner)).toEqual([ + undefined, + undefined, + ]); + }); + it("normalizes the review decision and reports nothing for one GitHub does not summarize", () => { const batch = expectSuccess( decodePullRequestListJson( @@ -216,6 +266,43 @@ describe("pull request detail decoding", () => { ], }); + it("maps the qualified head repository on detail responses", () => { + const raw = JSON.parse(detailJson) as Record; + const detail = expectSuccess( + decodePullRequestDetailJson( + JSON.stringify({ + ...raw, + isCrossRepository: true, + headRepository: { nameWithOwner: "contributor/web", name: "web" }, + headRepositoryOwner: { login: "contributor" }, + }), + ), + ); + + expect(detail).toMatchObject({ + headRepositoryNameWithOwner: "contributor/web", + headRepositoryOwner: "contributor", + isCrossRepository: true, + }); + }); + + it("leaves a deleted head repository unresolved on detail responses", () => { + const raw = JSON.parse(detailJson) as Record; + const detail = expectSuccess( + decodePullRequestDetailJson( + JSON.stringify({ + ...raw, + isCrossRepository: true, + headRepository: null, + headRepositoryOwner: null, + }), + ), + ); + + expect(detail.headRepositoryNameWithOwner).toBeUndefined(); + expect(detail.headRepositoryOwner).toBeNull(); + }); + it("maps check-run status and commit-status state onto one vocabulary", () => { const detail = expectSuccess(decodePullRequestDetailJson(detailJson)); expect(detail.checks.map((check) => [check.name, check.status])).toEqual([ diff --git a/apps/server/src/pullRequest/gitHubPullRequestJson.ts b/apps/server/src/pullRequest/gitHubPullRequestJson.ts index 7887a81617d6..00b54d22b6aa 100644 --- a/apps/server/src/pullRequest/gitHubPullRequestJson.ts +++ b/apps/server/src/pullRequest/gitHubPullRequestJson.ts @@ -83,6 +83,17 @@ const RawCheckSchema = Schema.Struct({ completedAt: Schema.optional(Schema.NullOr(Schema.String)), }); +/** The repository a head ref belongs to, when GitHub can still resolve it. */ +const RawHeadRepositorySchema = Schema.Struct({ + nameWithOwner: Schema.optional(Schema.NullOr(Schema.String)), + name: Schema.optional(Schema.NullOr(Schema.String)), +}); + +/** GitHub keeps the owner separate from the repository object on `gh pr` JSON output. */ +const RawHeadRepositoryOwnerSchema = Schema.Struct({ + login: Schema.optional(Schema.NullOr(Schema.String)), +}); + const RawListItemSchema = Schema.Struct({ number: Schema.Int, title: Schema.String, @@ -90,6 +101,10 @@ const RawListItemSchema = Schema.Struct({ author: Schema.optional(Schema.NullOr(RawActorSchema)), headRefName: Schema.String, baseRefName: Schema.String, + /** Whether GitHub explicitly says that the head is from another repository. */ + isCrossRepository: Schema.optional(Schema.Boolean), + headRepository: Schema.optional(Schema.NullOr(RawHeadRepositorySchema)), + headRepositoryOwner: Schema.optional(Schema.NullOr(RawHeadRepositoryOwnerSchema)), state: Schema.optional(Schema.NullOr(Schema.String)), isDraft: Schema.optional(Schema.Boolean), mergeable: Schema.optional(Schema.NullOr(Schema.String)), @@ -362,10 +377,6 @@ const RawCommitSchema = Schema.Struct({ const RawDetailSchema = Schema.Struct({ ...RawListItemSchema.fields, - /** GitHub's explicit distinction between a fork head and a branch in the base repository. */ - isCrossRepository: Schema.optional(Schema.Boolean), - /** Names the fork a pull request came from, which is what qualifies its head ref. */ - headRepositoryOwner: Schema.optional(Schema.NullOr(Schema.Struct({ login: Schema.String }))), /** The exact head revision, used to find workflow runs that GitHub has not started yet. */ headRefOid: Schema.optional(Schema.NullOr(Schema.String)), body: Schema.optional(Schema.String), @@ -618,9 +629,9 @@ export function decodeActorAvatarsJson( } export const PULL_REQUEST_LIST_JSON_FIELDS = - "number,title,url,author,headRefName,baseRefName,state,isDraft,mergeable,reviewDecision,additions,deletions,createdAt,updatedAt,mergedAt,reviewRequests,labels,statusCheckRollup"; + "number,title,url,author,headRefName,baseRefName,isCrossRepository,headRepository,headRepositoryOwner,state,isDraft,mergeable,reviewDecision,additions,deletions,createdAt,updatedAt,mergedAt,reviewRequests,labels,statusCheckRollup"; -export const PULL_REQUEST_DETAIL_JSON_FIELDS = `${PULL_REQUEST_LIST_JSON_FIELDS},body,changedFiles,closedAt,isCrossRepository,headRepositoryOwner,headRefOid,autoMergeRequest`; +export const PULL_REQUEST_DETAIL_JSON_FIELDS = `${PULL_REQUEST_LIST_JSON_FIELDS},body,changedFiles,closedAt,headRefOid,autoMergeRequest`; export const PULL_REQUEST_ACTIVITY_JSON_FIELDS = "author,comments,reviews,commits"; /** GitHub's own ceiling on a connection page, which is what both thread reads ask for. */ @@ -1026,6 +1037,10 @@ export interface GitHubPullRequestListItem { readonly url: string; readonly author: PullRequestActor | null; readonly headBranch: string; + /** The repository-qualified head ref, omitted when GitHub could not resolve the source. */ + readonly headRepositoryNameWithOwner?: string | null; + /** True only when GitHub explicitly says the head belongs to another repository. */ + readonly isCrossRepository?: boolean; readonly baseBranch: string; readonly state: PullRequestState; readonly isDraft: boolean; @@ -1045,8 +1060,6 @@ export interface GitHubPullRequestListItem { } export interface GitHubPullRequestDetail extends GitHubPullRequestListItem { - /** True only when GitHub says the head belongs to another repository. */ - readonly isCrossRepository?: boolean; /** The owner of the head branch's repository; null where `gh` did not say. */ readonly headRepositoryOwner: string | null; readonly headSha?: string | null; @@ -1085,6 +1098,39 @@ function trimmed(value: string | null | undefined): string | null { return text.length > 0 ? text : null; } +/** + * GitHub normally gives `nameWithOwner`; older `gh` versions expose the name and owner as two + * fields. A bare repository name is not a repository identity, so it is discarded when neither + * field can qualify it. + */ +function headRepositoryNameWithOwner(raw: { + readonly headRepository?: Schema.Schema.Type | null | undefined; + readonly headRepositoryOwner?: + | Schema.Schema.Type + | null + | undefined; +}): string | null { + const explicit = trimmed(raw.headRepository?.nameWithOwner); + if (explicit?.includes("/")) return explicit; + const owner = trimmed(raw.headRepositoryOwner?.login); + const name = trimmed(raw.headRepository?.name); + return owner !== null && name !== null ? `${owner}/${name}` : null; +} + +function headRepositoryOwner(raw: { + readonly headRepository?: Schema.Schema.Type | null | undefined; + readonly headRepositoryOwner?: + | Schema.Schema.Type + | null + | undefined; +}): string | null { + return ( + trimmed(raw.headRepositoryOwner?.login) ?? + headRepositoryNameWithOwner(raw)?.split("/", 1)[0] ?? + null + ); +} + /** * Null once a connection has nothing further, which is what ends every walk below. GitHub sends * an `endCursor` on a page that is also the last one, so the flag is what decides, not the @@ -1395,6 +1441,7 @@ function toCommits( } function toListItem(raw: Schema.Schema.Type): GitHubPullRequestListItem { + const repository = headRepositoryNameWithOwner(raw); return { authorId: trimmed(raw.author?.id), number: raw.number, @@ -1402,6 +1449,10 @@ function toListItem(raw: Schema.Schema.Type): GitHubPu url: raw.url, author: toActor(raw.author), headBranch: raw.headRefName, + ...(repository === null ? {} : { headRepositoryNameWithOwner: repository }), + ...(typeof raw.isCrossRepository === "boolean" + ? { isCrossRepository: raw.isCrossRepository } + : {}), baseBranch: raw.baseRefName, state: toState(raw), isDraft: raw.isDraft ?? false, @@ -1425,7 +1476,7 @@ function toDetail(raw: Schema.Schema.Type): GitHubPullRe ...(typeof raw.isCrossRepository === "boolean" ? { isCrossRepository: raw.isCrossRepository } : {}), - headRepositoryOwner: trimmed(raw.headRepositoryOwner?.login), + headRepositoryOwner: headRepositoryOwner(raw), headSha: trimmed(raw.headRefOid), body: raw.body ?? "", changedFiles: raw.changedFiles ?? 0, diff --git a/apps/server/src/pullRequest/gitLabMergeRequestJson.test.ts b/apps/server/src/pullRequest/gitLabMergeRequestJson.test.ts index 4438aa0c521a..a57e46baef8e 100644 --- a/apps/server/src/pullRequest/gitLabMergeRequestJson.test.ts +++ b/apps/server/src/pullRequest/gitLabMergeRequestJson.test.ts @@ -79,6 +79,60 @@ describe("decodeMergeRequestListJson", () => { }); }); + it("keeps the source project identity for a cross-project merge request", () => { + const batch = expectSuccess( + decodeMergeRequestListJson( + listJson([ + { + source_project_id: 12, + target_project_id: 11, + source_project: { path_with_namespace: "contributor/web" }, + target_project: { path_with_namespace: "acme/web" }, + }, + ]), + ), + ); + + expect(batch.items[0]).toMatchObject({ + headRepositoryNameWithOwner: "contributor/web", + }); + }); + + it("uses the target project only when matching project ids prove a same-repository source", () => { + const batch = expectSuccess( + decodeMergeRequestListJson( + listJson([ + { + source_project_id: 11, + target_project_id: 11, + target_project: { path_with_namespace: "acme/web" }, + }, + ]), + ), + ); + + expect(batch.items[0]).toMatchObject({ + headRepositoryNameWithOwner: "acme/web", + }); + }); + + it("leaves a deleted or redacted source project unqualified", () => { + const batch = expectSuccess( + decodeMergeRequestListJson( + listJson([ + { + source_project_id: 12, + target_project_id: 11, + source_project: null, + target_project: { path_with_namespace: "acme/web" }, + }, + ]), + ), + ); + + expect(batch.items[0]?.headRepositoryNameWithOwner).toBeNull(); + }); + it("reports no line counts, which GitLab does not expose", () => { const batch = expectSuccess(decodeMergeRequestListJson(listJson([{}]))); @@ -137,6 +191,21 @@ describe("decodeMergeRequestListJson", () => { }); describe("decodeMergeRequestDetailJson", () => { + it("maps the source project identity on detail responses too", () => { + const detail = expectSuccess( + decodeMergeRequestDetailJson( + detailJson({ + source_project_id: 12, + target_project_id: 11, + source_project: { path_with_namespace: "contributor/web" }, + target_project: { path_with_namespace: "acme/web" }, + }), + ), + ); + + expect(detail.headRepositoryNameWithOwner).toBe("contributor/web"); + }); + it("reads the description, file count and pipeline", () => { const detail = expectSuccess( decodeMergeRequestDetailJson( diff --git a/apps/server/src/pullRequest/gitLabMergeRequestJson.ts b/apps/server/src/pullRequest/gitLabMergeRequestJson.ts index 4b3c797136f8..4cb7bfbefb26 100644 --- a/apps/server/src/pullRequest/gitLabMergeRequestJson.ts +++ b/apps/server/src/pullRequest/gitLabMergeRequestJson.ts @@ -42,6 +42,21 @@ const RawPipelineSchema = Schema.Struct({ source: Schema.optional(Schema.NullOr(Schema.String)), }); +const GitLabProjectReferenceSchema = Schema.Struct({ + path: Schema.optional(Schema.String), + path_with_namespace: Schema.optional(Schema.String), + pathWithNamespace: Schema.optional(Schema.String), + namespace: Schema.optional( + Schema.NullOr( + Schema.Struct({ + path: Schema.optional(Schema.String), + full_path: Schema.optional(Schema.String), + fullPath: Schema.optional(Schema.String), + }), + ), + ), +}); + const RawMergeRequestSchema = Schema.Struct({ iid: Schema.Int, title: Schema.String, @@ -50,6 +65,11 @@ const RawMergeRequestSchema = Schema.Struct({ author: Schema.optional(Schema.NullOr(RawUserSchema)), source_branch: Schema.String, target_branch: Schema.String, + /** Present on REST list and detail responses; absent on older or redacted installations. */ + source_project_id: Schema.optional(Schema.NullOr(Schema.Number)), + target_project_id: Schema.optional(Schema.NullOr(Schema.Number)), + source_project: Schema.optional(Schema.NullOr(GitLabProjectReferenceSchema)), + target_project: Schema.optional(Schema.NullOr(GitLabProjectReferenceSchema)), state: Schema.optional(Schema.NullOr(Schema.String)), draft: Schema.optional(Schema.Boolean), work_in_progress: Schema.optional(Schema.Boolean), @@ -203,6 +223,8 @@ export interface GitLabMergeRequestListItem { readonly url: string; readonly author: PullRequestActor | null; readonly headBranch: string; + /** The repository-qualified head ref, or null when GitLab omitted source project metadata. */ + readonly headRepositoryNameWithOwner: string | null; readonly baseBranch: string; readonly state: PullRequestState; readonly isDraft: boolean; @@ -246,6 +268,26 @@ function trimmed(value: string | null | undefined): string | null { return text.length > 0 ? text : null; } +function projectPathWithNamespace( + project: Schema.Schema.Type | null | undefined, +): string | null { + const explicit = trimmed(project?.path_with_namespace) ?? trimmed(project?.pathWithNamespace); + if (explicit?.includes("/")) return explicit; + + const projectPath = trimmed(project?.path); + const namespacePath = + trimmed(project?.namespace?.full_path) ?? + trimmed(project?.namespace?.fullPath) ?? + trimmed(project?.namespace?.path); + return projectPath !== null && namespacePath !== null ? `${namespacePath}/${projectPath}` : null; +} + +/** A repository supplied with the request is safe to use only in its qualified form. */ +function requestedRepositoryPath(repository: string | undefined): string | null { + const path = trimmed(repository); + return path?.includes("/") ? path : null; +} + function toActor(raw: Schema.Schema.Type | null | undefined) { const login = trimmed(raw?.username); return login === null @@ -339,13 +381,23 @@ function toChecks( function toListItem( raw: Schema.Schema.Type, + repository?: string, ): GitLabMergeRequestListItem { + const targetProjectPath = projectPathWithNamespace(raw.target_project); + const sourceProjectPath = + projectPathWithNamespace(raw.source_project) ?? + (typeof raw.source_project_id === "number" && + typeof raw.target_project_id === "number" && + raw.source_project_id === raw.target_project_id + ? (targetProjectPath ?? requestedRepositoryPath(repository)) + : null); return { number: raw.iid, title: raw.title, url: raw.web_url, author: toActor(raw.author), headBranch: raw.source_branch, + headRepositoryNameWithOwner: sourceProjectPath, baseBranch: raw.target_branch, state: toState(raw), isDraft: raw.draft ?? raw.work_in_progress ?? false, @@ -362,8 +414,11 @@ function toListItem( }; } -function toDetail(raw: Schema.Schema.Type): GitLabMergeRequestDetail { - const listItem = toListItem(raw); +function toDetail( + raw: Schema.Schema.Type, + repository?: string, +): GitLabMergeRequestDetail { + const listItem = toListItem(raw, repository); const autoMerge = raw.merge_when_pipeline_succeeds == null && raw.auto_merge_enabled == null ? undefined @@ -425,6 +480,7 @@ export interface GitLabMergeRequestListBatch { * must not blank the whole list. */ export function decodeMergeRequestListJson( raw: string, + repository?: string, ): Result.Result { const decoded = decodeUnknownList(raw); if (!Result.isSuccess(decoded)) { @@ -435,7 +491,7 @@ export function decodeMergeRequestListJson( for (const [rawIndex, entry] of decoded.success.entries()) { const item = decodeMergeRequestEntry(entry); if (Exit.isSuccess(item)) { - items.push(toListItem(item.value)); + items.push(toListItem(item.value, repository)); rawIndexes.push(rawIndex); } } @@ -444,10 +500,11 @@ export function decodeMergeRequestListJson( export function decodeMergeRequestDetailJson( raw: string, + repository?: string, ): Result.Result { const decoded = decodeMergeRequest(raw); return Result.isSuccess(decoded) - ? Result.succeed(toDetail(decoded.success)) + ? Result.succeed(toDetail(decoded.success, repository)) : Result.fail(decoded.failure); } diff --git a/apps/server/src/sourceControl/AzureDevOpsCli.test.ts b/apps/server/src/sourceControl/AzureDevOpsCli.test.ts index f0cb52003029..5cbf4ae420f6 100644 --- a/apps/server/src/sourceControl/AzureDevOpsCli.test.ts +++ b/apps/server/src/sourceControl/AzureDevOpsCli.test.ts @@ -48,6 +48,9 @@ describe("AzureDevOpsCli.layer", () => { status: "active", creationDate: "2026-01-02T00:00:00.000Z", closedDate: null, + repository: { name: "repo", project: { name: "project" } }, + // Azure uses a JSON null fork marker for a same-repository pull request. + forkSource: null, _links: { web: { href: "https://dev.azure.com/acme/project/_git/repo/pullrequest/42", @@ -70,6 +73,8 @@ describe("AzureDevOpsCli.layer", () => { assert.strictEqual(result.baseRefName, "main"); assert.strictEqual(result.headRefName, "feature/source-control"); assert.strictEqual(result.state, "open"); + assert.strictEqual(result.headRepositoryNameWithOwner, "project/repo"); + assert.isFalse(result.isCrossRepository); assert.deepStrictEqual(result.updatedAt._tag, Option.some(1)._tag); assert.deepStrictEqual(mockRun.mock.calls.at(-1)?.[0], { operation: "AzureDevOpsCli.execute", @@ -143,6 +148,10 @@ describe("AzureDevOpsCli.layer", () => { targetRefName: "refs/heads/main", status: "completed", closedDate: "2026-01-03T00:00:00.000Z", + repository: { name: "repo", project: { name: "project" } }, + forkSource: { + repository: { name: "repo", project: { name: "fork-project" } }, + }, _links: { web: { href: "https://dev.azure.com/acme/project/_git/repo/pullrequest/7", @@ -163,6 +172,8 @@ describe("AzureDevOpsCli.layer", () => { }); assert.strictEqual(result[0]?.state, "merged"); + assert.strictEqual(result[0]?.headRepositoryNameWithOwner, "fork-project/repo"); + assert.isTrue(result[0]?.isCrossRepository); expect(mockRun).toHaveBeenCalledWith({ operation: "AzureDevOpsCli.execute", command: "az", @@ -188,6 +199,37 @@ describe("AzureDevOpsCli.layer", () => { }).pipe(Effect.provide(layer)), ); + it.effect("keeps an unresolved fork source unknown instead of using the target repository", () => + Effect.gen(function* () { + mockRun.mockReturnValueOnce( + Effect.succeed( + processOutput( + // @effect-diagnostics-next-line preferSchemaOverJson:off + JSON.stringify({ + pullRequestId: 43, + title: "Deleted fork source", + sourceRefName: "refs/heads/feature/deleted", + targetRefName: "refs/heads/main", + status: "active", + creationDate: "2026-01-02T00:00:00.000Z", + repository: { name: "repo", project: { name: "project" } }, + forkSource: { repository: null }, + _links: { + web: { href: "https://dev.azure.com/acme/project/_git/repo/pullrequest/43" }, + }, + }), + ), + ), + ); + + const az = yield* AzureDevOpsCli.AzureDevOpsCli; + const result = yield* az.getPullRequest({ cwd: "/repo", reference: "43" }); + + assert.isNull(result.headRepositoryNameWithOwner); + assert.isUndefined(result.isCrossRepository); + }).pipe(Effect.provide(layer)), + ); + it.effect("reads repository clone URLs", () => Effect.gen(function* () { mockRun.mockReturnValueOnce( diff --git a/apps/server/src/sourceControl/AzureDevOpsSourceControlProvider.test.ts b/apps/server/src/sourceControl/AzureDevOpsSourceControlProvider.test.ts index cacdd1a3cd97..29efcc3fa530 100644 --- a/apps/server/src/sourceControl/AzureDevOpsSourceControlProvider.test.ts +++ b/apps/server/src/sourceControl/AzureDevOpsSourceControlProvider.test.ts @@ -24,6 +24,8 @@ it.effect("maps Azure DevOps PR summaries into provider-neutral change requests" headRefName: "feature/source-control", state: "open", updatedAt: Option.none(), + headRepositoryNameWithOwner: "project/repo", + isCrossRepository: false, }), }); @@ -41,6 +43,7 @@ it.effect("maps Azure DevOps PR summaries into provider-neutral change requests" headRefName: "feature/source-control", state: "open", updatedAt: Option.none(), + headRepositoryNameWithOwner: "project/repo", isCrossRepository: false, }); }), diff --git a/apps/server/src/sourceControl/AzureDevOpsSourceControlProvider.ts b/apps/server/src/sourceControl/AzureDevOpsSourceControlProvider.ts index 8a840c524eba..1a90021063b3 100644 --- a/apps/server/src/sourceControl/AzureDevOpsSourceControlProvider.ts +++ b/apps/server/src/sourceControl/AzureDevOpsSourceControlProvider.ts @@ -63,6 +63,8 @@ function toChangeRequest(summary: { readonly state: "open" | "closed" | "merged"; readonly isDraft?: boolean; readonly updatedAt: ChangeRequest["updatedAt"]; + readonly headRepositoryNameWithOwner?: string | null; + readonly isCrossRepository?: boolean; }): ChangeRequest { return { provider: "azure-devops", @@ -74,7 +76,12 @@ function toChangeRequest(summary: { state: summary.state, ...(summary.isDraft === true ? { isDraft: true } : {}), updatedAt: summary.updatedAt, - isCrossRepository: false, + ...(summary.headRepositoryNameWithOwner === undefined + ? {} + : { headRepositoryNameWithOwner: summary.headRepositoryNameWithOwner }), + ...(summary.isCrossRepository === undefined + ? {} + : { isCrossRepository: summary.isCrossRepository }), }; } diff --git a/apps/server/src/sourceControl/BitbucketApi.test.ts b/apps/server/src/sourceControl/BitbucketApi.test.ts index 856d043a7fce..88ee1f2f5c40 100644 --- a/apps/server/src/sourceControl/BitbucketApi.test.ts +++ b/apps/server/src/sourceControl/BitbucketApi.test.ts @@ -198,6 +198,77 @@ it.effect("parses pull request responses from the Bitbucket REST API", () => { }).pipe(Effect.provide(layer)); }); +it.effect("does not guess the target repository when Bitbucket omits the source", () => { + const { execute, layer } = makeLayer({ + response: () => + Response.json({ + ...bitbucketPullRequest, + source: { + branch: { name: "feature/deleted" }, + repository: null, + }, + }), + }); + + return Effect.gen(function* () { + const bitbucket = yield* BitbucketApi.BitbucketApi; + const result = yield* bitbucket.getPullRequest({ cwd: "/repo", reference: "#42" }); + + assert.isUndefined(result.headRepositoryNameWithOwner); + assert.isUndefined(result.isCrossRepository); + assert.strictEqual( + execute.mock.calls[0]?.[0].url, + "https://api.test.local/2.0/repositories/pingdotgg/t3code/pullrequests/42", + ); + }).pipe(Effect.provide(layer)); +}); + +it.effect("does not expose a bare source repository name as an identity", () => { + const { layer } = makeLayer({ + response: () => + Response.json({ + ...bitbucketPullRequest, + source: { + ...bitbucketPullRequest.source, + repository: { full_name: "t3code" }, + }, + }), + }); + + return Effect.gen(function* () { + const bitbucket = yield* BitbucketApi.BitbucketApi; + const result = yield* bitbucket.getPullRequest({ cwd: "/repo", reference: "#42" }); + + assert.isUndefined(result.headRepositoryNameWithOwner); + assert.isUndefined(result.isCrossRepository); + }).pipe(Effect.provide(layer)); +}); + +it.effect("compares Bitbucket repository identities without case sensitivity", () => { + const { layer } = makeLayer({ + response: () => + Response.json({ + ...bitbucketPullRequest, + source: { + ...bitbucketPullRequest.source, + repository: { full_name: "PINGDOTGG/t3code", workspace: { slug: "PINGDOTGG" } }, + }, + destination: { + ...bitbucketPullRequest.destination, + repository: { full_name: "pingdotgg/t3code", workspace: { slug: "pingdotgg" } }, + }, + }), + }); + + return Effect.gen(function* () { + const bitbucket = yield* BitbucketApi.BitbucketApi; + const result = yield* bitbucket.getPullRequest({ cwd: "/repo", reference: "#42" }); + + assert.strictEqual(result.headRepositoryNameWithOwner, "PINGDOTGG/t3code"); + assert.isUndefined(result.isCrossRepository); + }).pipe(Effect.provide(layer)); +}); + it.effect("lists pull requests with Bitbucket state and source branch query params", () => { const { execute, layer } = makeLayer({ response: () => diff --git a/apps/server/src/sourceControl/GitLabCli.test.ts b/apps/server/src/sourceControl/GitLabCli.test.ts index eb56b434b2f8..12f132502888 100644 --- a/apps/server/src/sourceControl/GitLabCli.test.ts +++ b/apps/server/src/sourceControl/GitLabCli.test.ts @@ -152,6 +152,199 @@ layer("GitLabCli.layer", (it) => { }), ); + it.effect("uses matching project ids for same-repository MR source identity", () => + Effect.gen(function* () { + mockedRun.mockReturnValueOnce( + Effect.succeed( + processOutput( + // @effect-diagnostics-next-line preferSchemaOverJson:off + JSON.stringify({ + iid: 44, + title: "Same repository MR", + web_url: "https://gitlab.com/acme/web/-/merge_requests/44", + target_branch: "main", + source_branch: "feature/same-repo", + source_project_id: 100, + target_project_id: 100, + target_project: { path_with_namespace: "acme/web" }, + state: "opened", + }), + ), + ), + ); + + const glab = yield* GitLabCli.GitLabCli; + const result = yield* glab.getMergeRequest({ cwd: "/repo", reference: "44" }); + + assert.deepStrictEqual(result, { + number: 44, + title: "Same repository MR", + url: "https://gitlab.com/acme/web/-/merge_requests/44", + baseRefName: "main", + headRefName: "feature/same-repo", + state: "open", + isCrossRepository: false, + headRepositoryNameWithOwner: "acme/web", + headRepositoryOwnerLogin: "acme", + }); + }), + ); + + it.effect("uses the requested repository for IDs-only same-repository MRs", () => + Effect.gen(function* () { + mockedRun.mockReturnValueOnce( + Effect.succeed( + processOutput( + // @effect-diagnostics-next-line preferSchemaOverJson:off + JSON.stringify({ + iid: 46, + title: "IDs-only same repository MR", + web_url: "https://gitlab.com/acme/web/-/merge_requests/46", + target_branch: "main", + source_branch: "feature/ids-only", + source_project_id: 100, + target_project_id: 100, + state: "opened", + }), + ), + ), + ); + + const glab = yield* GitLabCli.GitLabCli; + const result = yield* glab.getMergeRequest({ + cwd: "/repo", + reference: "46", + repository: "acme/web", + }); + + assert.deepStrictEqual(result, { + number: 46, + title: "IDs-only same repository MR", + url: "https://gitlab.com/acme/web/-/merge_requests/46", + baseRefName: "main", + headRefName: "feature/ids-only", + state: "open", + isCrossRepository: false, + headRepositoryNameWithOwner: "acme/web", + headRepositoryOwnerLogin: "acme", + }); + expect(mockedRun).toHaveBeenCalledWith( + expect.objectContaining({ + command: "glab", + cwd: "/repo", + args: ["mr", "view", "46", "--repo", "acme/web", "--output", "json"], + }), + ); + }), + ); + + it.effect("does not reuse the target project after GitLab redacts a fork source", () => + Effect.gen(function* () { + mockedRun.mockReturnValueOnce( + Effect.succeed( + processOutput( + // @effect-diagnostics-next-line preferSchemaOverJson:off + JSON.stringify([ + { + iid: 45, + title: "Deleted source", + web_url: "https://gitlab.com/acme/web/-/merge_requests/45", + target_branch: "main", + source_branch: "feature/deleted", + source_project_id: 101, + target_project_id: 100, + source_project: null, + target_project: { path_with_namespace: "acme/web" }, + }, + ]), + ), + ), + ); + + const glab = yield* GitLabCli.GitLabCli; + const result = yield* glab.listMergeRequests({ + cwd: "/repo", + headSelector: "feature/deleted", + state: "open", + }); + + assert.strictEqual(result[0]?.number, 45); + assert.isUndefined(result[0]?.headRepositoryNameWithOwner); + // Project ids still prove that this was cross-repository even after the source was deleted. + assert.isTrue(result[0]?.isCrossRepository); + }), + ); + + it.effect("binds IDs-only list identity to the requested self-hosted repository", () => + Effect.gen(function* () { + mockedRun.mockReturnValueOnce( + Effect.succeed( + processOutput( + // @effect-diagnostics-next-line preferSchemaOverJson:off + JSON.stringify([ + { + iid: 47, + title: "Same project", + web_url: "https://forge.test/acme/web/-/merge_requests/47", + target_branch: "main", + source_branch: "feature/list", + source_project_id: 100, + target_project_id: 100, + }, + ]), + ), + ), + ); + const glab = yield* GitLabCli.GitLabCli; + const result = yield* glab.listMergeRequests({ + cwd: "/different-checkout", + headSelector: "feature/list", + state: "open", + repository: "acme/web", + repositoryUrl: "https://forge.test/acme/web", + }); + assert.strictEqual(result[0]?.headRepositoryNameWithOwner, "acme/web"); + expect(mockedRun).toHaveBeenCalledWith( + expect.objectContaining({ + args: expect.arrayContaining(["--repo", "https://forge.test/acme/web"]), + }), + ); + }), + ); + + it.effect("does not borrow workspace repository identity for an explicit MR URL", () => + Effect.gen(function* () { + const reference = "https://another.test/other/project/-/merge_requests/48"; + mockedRun.mockReturnValueOnce( + Effect.succeed( + processOutput( + // @effect-diagnostics-next-line preferSchemaOverJson:off + JSON.stringify({ + iid: 48, + title: "Explicit MR", + web_url: reference, + target_branch: "main", + source_branch: "feature/url", + source_project_id: 200, + target_project_id: 200, + }), + ), + ), + ); + const glab = yield* GitLabCli.GitLabCli; + const result = yield* glab.getMergeRequest({ + cwd: "/repo", + reference, + repository: "acme/web", + repositoryUrl: "https://forge.test/acme/web", + }); + assert.isUndefined(result.headRepositoryNameWithOwner); + expect(mockedRun).toHaveBeenCalledWith( + expect.objectContaining({ args: ["mr", "view", reference, "--output", "json"] }), + ); + }), + ); + it.effect("reads repository clone URLs", () => Effect.gen(function* () { mockedRun.mockReturnValueOnce( diff --git a/apps/server/src/sourceControl/GitLabCli.ts b/apps/server/src/sourceControl/GitLabCli.ts index ab8dfbb5f334..ce1cadb2b35a 100644 --- a/apps/server/src/sourceControl/GitLabCli.ts +++ b/apps/server/src/sourceControl/GitLabCli.ts @@ -275,6 +275,10 @@ export class GitLabCli extends Context.Service< readonly cwd: string; readonly headSelector: string; readonly source?: SourceControlProvider.SourceControlRefSelector; + /** Qualified path used to recover same-project identity when GitLab omits project paths. */ + readonly repository?: string; + /** Sanitized repository URL used to keep glab on the same GitLab host. */ + readonly repositoryUrl?: string; readonly state: "open" | "closed" | "merged" | "all"; readonly limit?: number; }) => Effect.Effect, GitLabCliError>; @@ -282,6 +286,10 @@ export class GitLabCli extends Context.Service< readonly getMergeRequest: (input: { readonly cwd: string; readonly reference: string; + /** Qualified path used to recover same-project identity when GitLab omits project paths. */ + readonly repository?: string; + /** Sanitized repository URL used to keep glab on the same GitLab host. */ + readonly repositoryUrl?: string; }) => Effect.Effect; readonly getRepositoryCloneUrls: (input: { @@ -382,6 +390,28 @@ function sourceProjectIdentifier( return source?.repository ?? source?.owner ?? null; } +function repositoryForMergeRequest(input: { + readonly reference?: string; + readonly repository?: string; +}): string | null { + // An explicit URL selects its own repository rather than the workspace remote. + if (input.reference !== undefined && URL.canParse(input.reference.trim())) return null; + const repository = input.repository?.trim(); + return repository?.includes("/") ? repository : null; +} + +function repositoryArgs(input: { + readonly reference?: string; + readonly repository?: string; + readonly repositoryUrl?: string; +}): ReadonlyArray { + if (repositoryForMergeRequest(input) === null) { + return []; + } + const selector = input.repositoryUrl?.trim() || input.repository?.trim(); + return selector === undefined || selector.length === 0 ? [] : ["--repo", selector]; +} + function toSummaryWithOptionalUpdatedAt( record: GitLabMergeRequestSummary & { readonly updatedAt: Option.Option; @@ -464,6 +494,7 @@ export const make = Effect.gen(function* () { ...stateArgs(input.state), "--per-page", String(input.limit ?? 20), + ...repositoryArgs(input), "--output", "json", ], @@ -472,7 +503,12 @@ export const make = Effect.gen(function* () { Effect.flatMap((raw) => raw.length === 0 ? Effect.succeed([]) - : Effect.sync(() => decodeGitLabMergeRequestListJson(raw)).pipe( + : Effect.sync(() => + decodeGitLabMergeRequestListJson( + raw, + repositoryForMergeRequest(input) ?? undefined, + ), + ).pipe( Effect.flatMap((decoded) => { if (!Result.isSuccess(decoded)) { return Effect.fail( @@ -494,11 +530,13 @@ export const make = Effect.gen(function* () { executeMergeRequest({ cwd: input.cwd, reference: input.reference, - args: ["mr", "view", input.reference, "--output", "json"], + args: ["mr", "view", input.reference, ...repositoryArgs(input), "--output", "json"], }).pipe( Effect.map((result) => result.stdout.trim()), Effect.flatMap((raw) => - Effect.sync(() => decodeGitLabMergeRequestJson(raw)).pipe( + Effect.sync(() => + decodeGitLabMergeRequestJson(raw, repositoryForMergeRequest(input) ?? undefined), + ).pipe( Effect.flatMap((decoded) => { if (!Result.isSuccess(decoded)) { return Effect.fail( diff --git a/apps/server/src/sourceControl/GitLabSourceControlProvider.test.ts b/apps/server/src/sourceControl/GitLabSourceControlProvider.test.ts index 0d06e0665214..d8a66fa1d6e3 100644 --- a/apps/server/src/sourceControl/GitLabSourceControlProvider.test.ts +++ b/apps/server/src/sourceControl/GitLabSourceControlProvider.test.ts @@ -121,6 +121,62 @@ it.effect("lists GitLab MRs through provider-neutral input names", () => }), ); +it.effect("passes the target repository from remote context to GitLab MR reads", () => + Effect.gen(function* () { + let listInput: Parameters[0] | null = null; + let getInput: Parameters[0] | null = null; + const provider = yield* makeProvider({ + listMergeRequests: (input) => { + listInput = input; + return Effect.succeed([]); + }, + getMergeRequest: (input) => { + getInput = input; + return Effect.succeed({ + number: 42, + title: "Same repository MR", + url: "https://gitlab.com/group/project/-/merge_requests/42", + baseRefName: "main", + headRefName: "feature/provider", + }); + }, + }); + const context = { + provider: { + kind: "gitlab", + name: "GitLab", + baseUrl: "https://gitlab.com", + }, + remoteName: "origin", + remoteUrl: "https://gitlab.com/group/project.git", + } as const; + + yield* provider.listChangeRequests({ + cwd: "/repo", + context, + headSelector: "feature/provider", + state: "all", + limit: 10, + }); + yield* provider.getChangeRequest({ cwd: "/repo", context, reference: "42" }); + + assert.deepStrictEqual(listInput, { + cwd: "/repo", + headSelector: "feature/provider", + repository: "group/project", + repositoryUrl: "https://gitlab.com/group/project", + state: "all", + limit: 10, + }); + assert.deepStrictEqual(getInput, { + cwd: "/repo", + reference: "42", + repository: "group/project", + repositoryUrl: "https://gitlab.com/group/project", + }); + }), +); + it.effect("creates GitLab MRs through provider-neutral input names", () => Effect.gen(function* () { let createInput: Parameters[0] | null = diff --git a/apps/server/src/sourceControl/GitLabSourceControlProvider.ts b/apps/server/src/sourceControl/GitLabSourceControlProvider.ts index 2ec1f9b9a228..3a90338fc951 100644 --- a/apps/server/src/sourceControl/GitLabSourceControlProvider.ts +++ b/apps/server/src/sourceControl/GitLabSourceControlProvider.ts @@ -17,6 +17,49 @@ import { } from "./SourceControlProviderDiscovery.ts"; import { findAuthenticatedGitLabHost, parseGitLabAuthStatusHosts } from "./gitLabAuthStatus.ts"; +interface GitLabRepositoryRequest { + readonly nameWithOwner: string; + readonly selector: string; +} + +function repositoryFromRemoteUrl( + remoteUrl: string | undefined, + baseUrl: string | undefined, +): GitLabRepositoryRequest | undefined { + const trimmed = remoteUrl?.trim() ?? ""; + if (trimmed.length === 0) { + return undefined; + } + + const scpPath = /^[^@\s/:]+@[^:\s]+:(.+)$/u.exec(trimmed)?.[1]; + let path = scpPath; + if (path === undefined) { + try { + const url = new URL(trimmed); + if (url.protocol !== "http:" && url.protocol !== "https:" && url.protocol !== "ssh:") { + return undefined; + } + path = url.pathname; + } catch { + return undefined; + } + } + + const repository = path + .replace(/^\/+|\/+$/gu, "") + .replace(/\.git$/iu, "") + .trim(); + if (!repository.includes("/")) { + return undefined; + } + + const host = baseUrl?.trim().replace(/\/+$/gu, ""); + return { + nameWithOwner: repository, + selector: host ? `${host}/${repository}` : repository, + }; +} + function toChangeRequest(summary: GitLabCli.GitLabMergeRequestSummary): ChangeRequest { return { provider: "gitlab", @@ -108,11 +151,18 @@ export const make = Effect.gen(function* () { kind: "gitlab", listChangeRequests: (input) => { const source = SourceControlProvider.sourceControlRefFromInput(input); + const repository = repositoryFromRemoteUrl( + input.context?.remoteUrl, + input.context?.provider.baseUrl, + ); return gitlab .listMergeRequests({ cwd: input.cwd, headSelector: input.headSelector, ...(source ? { source } : {}), + ...(repository + ? { repository: repository.nameWithOwner, repositoryUrl: repository.selector } + : {}), state: input.state, ...(input.limit !== undefined ? { limit: input.limit } : {}), }) @@ -134,24 +184,40 @@ export const make = Effect.gen(function* () { ), ); }, - getChangeRequest: (input) => - gitlab.getMergeRequest(input).pipe( - Effect.map(toChangeRequest), - Effect.mapError( - (error) => - new SourceControlProviderError({ - provider: "gitlab", - operation: "getChangeRequest", - command: error.command, - cwd: input.cwd, - reference: SourceControlProvider.transportSafeSourceControlErrorValue( - input.reference, - ), - detail: error.detail, - cause: error, - }), - ), - ), + getChangeRequest: (input) => { + const repository = repositoryFromRemoteUrl( + input.context?.remoteUrl, + input.context?.provider.baseUrl, + ); + return gitlab + .getMergeRequest({ + cwd: input.cwd, + reference: input.reference, + ...(repository + ? { + repository: repository.nameWithOwner, + repositoryUrl: repository.selector, + } + : {}), + }) + .pipe( + Effect.map(toChangeRequest), + Effect.mapError( + (error) => + new SourceControlProviderError({ + provider: "gitlab", + operation: "getChangeRequest", + command: error.command, + cwd: input.cwd, + reference: SourceControlProvider.transportSafeSourceControlErrorValue( + input.reference, + ), + detail: error.detail, + cause: error, + }), + ), + ); + }, createChangeRequest: (input) => { const source = SourceControlProvider.sourceControlRefFromInput(input); return gitlab diff --git a/apps/server/src/sourceControl/azureDevOpsPullRequests.ts b/apps/server/src/sourceControl/azureDevOpsPullRequests.ts index 8ac682399e1d..c823f94f75fd 100644 --- a/apps/server/src/sourceControl/azureDevOpsPullRequests.ts +++ b/apps/server/src/sourceControl/azureDevOpsPullRequests.ts @@ -16,23 +16,35 @@ export interface NormalizedAzureDevOpsPullRequestRecord { readonly state: "open" | "closed" | "merged"; readonly isDraft?: boolean; readonly updatedAt: Option.Option; + /** The qualified source repository; omitted when Azure did not expose fork metadata. */ + readonly headRepositoryNameWithOwner?: string | null; + /** Derived only from two known repository identities. */ + readonly isCrossRepository?: boolean; } +const AzureDevOpsRepositorySchema = Schema.Struct({ + name: Schema.optional(Schema.NullOr(Schema.String)), + webUrl: Schema.optional(Schema.NullOr(Schema.String)), + project: Schema.optional( + Schema.NullOr( + Schema.Struct({ + name: Schema.optional(Schema.NullOr(Schema.String)), + }), + ), + ), +}); + +const AzureDevOpsForkSourceSchema = Schema.Struct({ + repository: Schema.optional(Schema.NullOr(AzureDevOpsRepositorySchema)), +}); + const AzureDevOpsPullRequestSchema = Schema.Struct({ pullRequestId: PositiveInt, title: TrimmedNonEmptyString, url: Schema.optional(Schema.String), - repository: Schema.optional( - Schema.Struct({ - name: Schema.optional(Schema.String), - webUrl: Schema.optional(Schema.String), - project: Schema.optional( - Schema.Struct({ - name: Schema.optional(Schema.String), - }), - ), - }), - ), + repository: Schema.optional(Schema.NullOr(AzureDevOpsRepositorySchema)), + /** Azure sets this only for a pull request whose source is a fork. */ + forkSource: Schema.optional(Schema.NullOr(AzureDevOpsForkSourceSchema)), sourceRefName: TrimmedNonEmptyString, targetRefName: TrimmedNonEmptyString, status: Schema.String, @@ -55,6 +67,41 @@ function trimOptionalString(value: string | null | undefined): string | null { return trimmed.length > 0 ? trimmed : null; } +type AzureDevOpsRepositoryReference = { + readonly name?: string | null | undefined; + readonly project?: { readonly name?: string | null | undefined } | null | undefined; +}; + +/** Azure's stable repository identity is its project and repository name. */ +export function azureDevOpsRepositoryNameWithOwner( + repository: AzureDevOpsRepositoryReference | null | undefined, +): string | null { + const project = trimOptionalString(repository?.project?.name); + const name = trimOptionalString(repository?.name); + return project !== null && name !== null ? `${project}/${name}` : null; +} + +/** + * `forkSource` is Azure's explicit fork marker. An omitted marker is an unverified projection; + * only a JSON null marker certifies that the source is the target repository. + */ +export function azureDevOpsHeadRepositoryNameWithOwner(input: { + readonly repository?: AzureDevOpsRepositoryReference | null | undefined; + readonly forkSource?: + | { readonly repository?: AzureDevOpsRepositoryReference | null | undefined } + | null + | undefined; +}): string | null | undefined { + if (input.forkSource === undefined) return undefined; + return azureDevOpsRepositoryNameWithOwner( + input.forkSource === null ? input.repository : input.forkSource.repository, + ); +} + +function normalizeRepositoryIdentity(value: string): string { + return value.trim().toLowerCase(); +} + function normalizeRefName(refName: string): string { return refName.trim().replace(/^refs\/heads\//, ""); } @@ -163,6 +210,15 @@ function normalizeAzureDevOpsPullRequestUrl( function normalizeAzureDevOpsPullRequestRecord( raw: Schema.Schema.Type, ): NormalizedAzureDevOpsPullRequestRecord { + const headRepositoryNameWithOwner = azureDevOpsHeadRepositoryNameWithOwner(raw); + const targetRepositoryNameWithOwner = azureDevOpsRepositoryNameWithOwner(raw.repository); + const isCrossRepository = + headRepositoryNameWithOwner !== undefined && + headRepositoryNameWithOwner !== null && + targetRepositoryNameWithOwner !== null + ? normalizeRepositoryIdentity(headRepositoryNameWithOwner) !== + normalizeRepositoryIdentity(targetRepositoryNameWithOwner) + : undefined; return { number: raw.pullRequestId, title: raw.title, @@ -171,6 +227,8 @@ function normalizeAzureDevOpsPullRequestRecord( headRefName: normalizeRefName(raw.sourceRefName), state: normalizeAzureDevOpsPullRequestState(raw.status), ...(raw.isDraft === true ? { isDraft: true } : {}), + ...(headRepositoryNameWithOwner === undefined ? {} : { headRepositoryNameWithOwner }), + ...(typeof isCrossRepository === "boolean" ? { isCrossRepository } : {}), updatedAt: (raw.closedDate ?? Option.none()).pipe( Option.orElse(() => raw.creationDate ?? Option.none()), ), diff --git a/apps/server/src/sourceControl/bitbucketPullRequests.ts b/apps/server/src/sourceControl/bitbucketPullRequests.ts index 3b07334a8040..6a52300071ba 100644 --- a/apps/server/src/sourceControl/bitbucketPullRequests.ts +++ b/apps/server/src/sourceControl/bitbucketPullRequests.ts @@ -60,6 +60,15 @@ function trimOptionalString(value: string | null | undefined): string | null { return trimmed.length > 0 ? trimmed : null; } +function qualifiedRepository(value: string | null | undefined): string | null { + const repository = trimOptionalString(value); + return repository?.includes("/") ? repository : null; +} + +function normalizeRepositoryIdentity(value: string): string { + return value.trim().toLowerCase(); +} + function repositoryOwner(repository: Schema.Schema.Type) { return ( trimOptionalString(repository.workspace?.slug) ?? @@ -83,15 +92,16 @@ function normalizeBitbucketPullRequestState(state: string | null | undefined) { export function normalizeBitbucketPullRequestRecord( raw: Schema.Schema.Type, ): NormalizedBitbucketPullRequestRecord { - const headRepositoryNameWithOwner = trimOptionalString(raw.source.repository?.full_name); - const baseRepositoryNameWithOwner = trimOptionalString(raw.destination.repository?.full_name); + const headRepositoryNameWithOwner = qualifiedRepository(raw.source.repository?.full_name); + const baseRepositoryNameWithOwner = qualifiedRepository(raw.destination.repository?.full_name); const headRepositoryOwnerLogin = raw.source.repository ? repositoryOwner(raw.source.repository) : null; const isCrossRepository = headRepositoryNameWithOwner !== null && baseRepositoryNameWithOwner !== null && - headRepositoryNameWithOwner !== baseRepositoryNameWithOwner; + normalizeRepositoryIdentity(headRepositoryNameWithOwner) !== + normalizeRepositoryIdentity(baseRepositoryNameWithOwner); return { number: raw.id, diff --git a/apps/server/src/sourceControl/gitLabMergeRequests.ts b/apps/server/src/sourceControl/gitLabMergeRequests.ts index 3b032e245bbc..fe492663135f 100644 --- a/apps/server/src/sourceControl/gitLabMergeRequests.ts +++ b/apps/server/src/sourceControl/gitLabMergeRequests.ts @@ -22,6 +22,7 @@ export interface NormalizedGitLabMergeRequestRecord { } const GitLabProjectReferenceSchema = Schema.Struct({ + path: Schema.optional(Schema.String), path_with_namespace: Schema.optional(Schema.String), pathWithNamespace: Schema.optional(Schema.String), namespace: Schema.optional( @@ -75,15 +76,16 @@ function projectPathWithNamespace( const explicit = trimOptionalString(project?.path_with_namespace) ?? trimOptionalString(project?.pathWithNamespace); - if (explicit) { + if (explicit?.includes("/")) { return explicit; } + const projectPath = trimOptionalString(project?.path); const namespacePath = trimOptionalString(project?.namespace?.full_path) ?? trimOptionalString(project?.namespace?.fullPath) ?? trimOptionalString(project?.namespace?.path); - return namespacePath; + return projectPath !== null && namespacePath !== null ? `${namespacePath}/${projectPath}` : null; } function ownerLoginFromPathWithNamespace(pathWithNamespace: string | null): string | null { @@ -91,11 +93,24 @@ function ownerLoginFromPathWithNamespace(pathWithNamespace: string | null): stri return trimOptionalString(owner); } +/** A repository supplied with the request is safe to use only in its qualified form. */ +function requestedRepositoryPath(repository: string | undefined): string | null { + const path = trimOptionalString(repository); + return path?.includes("/") ? path : null; +} + function normalizeGitLabMergeRequestRecord( raw: Schema.Schema.Type, + repository?: string, ): NormalizedGitLabMergeRequestRecord { - const sourceProjectPath = projectPathWithNamespace(raw.source_project); const targetProjectPath = projectPathWithNamespace(raw.target_project); + const sourceProjectPath = + projectPathWithNamespace(raw.source_project) ?? + (typeof raw.source_project_id === "number" && + typeof raw.target_project_id === "number" && + raw.source_project_id === raw.target_project_id + ? (targetProjectPath ?? requestedRepositoryPath(repository)) + : null); const isCrossRepository = typeof raw.source_project_id === "number" && typeof raw.target_project_id === "number" ? raw.source_project_id !== raw.target_project_id @@ -127,6 +142,7 @@ export const formatGitLabJsonDecodeError = formatSchemaError; export function decodeGitLabMergeRequestListJson( raw: string, + repository?: string, ): Result.Result< ReadonlyArray, Cause.Cause @@ -139,7 +155,7 @@ export function decodeGitLabMergeRequestListJson( if (Exit.isFailure(decodedEntry)) { continue; } - mergeRequests.push(normalizeGitLabMergeRequestRecord(decodedEntry.value)); + mergeRequests.push(normalizeGitLabMergeRequestRecord(decodedEntry.value, repository)); } return Result.succeed(mergeRequests); } @@ -148,10 +164,11 @@ export function decodeGitLabMergeRequestListJson( export function decodeGitLabMergeRequestJson( raw: string, + repository?: string, ): Result.Result> { const result = decodeGitLabMergeRequest(raw); if (Result.isSuccess(result)) { - return Result.succeed(normalizeGitLabMergeRequestRecord(result.success)); + return Result.succeed(normalizeGitLabMergeRequestRecord(result.success, repository)); } return Result.fail(result.failure); }