From f40dbd3e77de83f5c1c17a9672190aca684343ec Mon Sep 17 00:00:00 2001 From: Kieren-Foenander Date: Thu, 27 Aug 2026 17:12:59 +1000 Subject: [PATCH 1/2] fix(server): support Azure DevOps pull request responses - Read pull request threads through the Azure DevOps CLI - Handle nullable Azure fields and same-repository PR refs --- apps/server/src/git/GitManager.test.ts | 82 +++++++++++ apps/server/src/git/GitManager.ts | 27 +++- .../AzureDevOpsPullRequestCli.test.ts | 32 ++++- .../pullRequest/AzureDevOpsPullRequestCli.ts | 24 +++- .../AzureDevOpsPullRequestProvider.ts | 4 +- .../azureDevOpsPullRequestJson.test.ts | 13 +- .../pullRequest/azureDevOpsPullRequestJson.ts | 25 ++-- .../src/sourceControl/AzureDevOpsCli.test.ts | 136 +++++++++++++++++- .../src/sourceControl/AzureDevOpsCli.ts | 18 ++- .../AzureDevOpsSourceControlProvider.test.ts | 40 ++++++ .../AzureDevOpsSourceControlProvider.ts | 7 +- .../sourceControl/azureDevOpsPullRequests.ts | 73 +++++++--- 12 files changed, 424 insertions(+), 57 deletions(-) diff --git a/apps/server/src/git/GitManager.test.ts b/apps/server/src/git/GitManager.test.ts index 9e2cf15ecb72..c032714096a6 100644 --- a/apps/server/src/git/GitManager.test.ts +++ b/apps/server/src/git/GitManager.test.ts @@ -3679,6 +3679,88 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { }), ); + it.effect("prepares a worktree PR thread on a host that publishes no pull request head ref", () => + Effect.gen(function* () { + const repoDir = yield* makeTempDir("t3code-git-manager-"); + yield* initRepo(repoDir); + const remoteDir = yield* createBareRemote(); + yield* runGit(repoDir, ["remote", "add", "origin", remoteDir]); + yield* runGit(repoDir, ["push", "-u", "origin", "main"]); + yield* runGit(repoDir, ["checkout", "-b", "feature/pr-no-pull-ref"]); + NodeFS.writeFileSync(NodePath.join(repoDir, "azure.txt"), "azure\n"); + yield* runGit(repoDir, ["add", "azure.txt"]); + yield* runGit(repoDir, ["commit", "-m", "PR branch with no pull ref"]); + yield* runGit(repoDir, ["push", "origin", "feature/pr-no-pull-ref"]); + const headCommit = (yield* runGit(repoDir, ["rev-parse", "HEAD"])).stdout.trim(); + yield* runGit(repoDir, ["checkout", "main"]); + yield* runGit(repoDir, ["branch", "-D", "feature/pr-no-pull-ref"]); + + const { manager } = yield* makeManager({ + ghScenario: { + pullRequest: { + number: 26855, + title: "PR with no pull ref", + url: "https://dev.azure.com/acme/project/_git/repo/pullrequest/26855", + baseRefName: "main", + headRefName: "feature/pr-no-pull-ref", + state: "open", + isCrossRepository: false, + }, + }, + }); + + const result = yield* preparePullRequestThread(manager, { + cwd: repoDir, + reference: "26855", + mode: "worktree", + }); + + expect(result.worktreePath).not.toBeNull(); + expect(result.isOnPullRequestHead).toBe(true); + expect( + (yield* runGit(result.worktreePath as string, ["rev-parse", "HEAD"])).stdout.trim(), + ).toBe(headCommit); + }), + ); + + it.effect("does not use a remote branch when the pull request repository is unknown", () => + Effect.gen(function* () { + const repoDir = yield* makeTempDir("t3code-git-manager-"); + yield* initRepo(repoDir); + const remoteDir = yield* createBareRemote(); + yield* runGit(repoDir, ["remote", "add", "origin", remoteDir]); + yield* runGit(repoDir, ["push", "-u", "origin", "main"]); + yield* runGit(repoDir, ["checkout", "-b", "feature/ambiguous-head"]); + NodeFS.writeFileSync(NodePath.join(repoDir, "unrelated.txt"), "unrelated\n"); + yield* runGit(repoDir, ["add", "unrelated.txt"]); + yield* runGit(repoDir, ["commit", "-m", "Unrelated same-named branch"]); + yield* runGit(repoDir, ["push", "origin", "feature/ambiguous-head"]); + yield* runGit(repoDir, ["checkout", "main"]); + yield* runGit(repoDir, ["branch", "-D", "feature/ambiguous-head"]); + + const { manager } = yield* makeManager({ + ghScenario: { + pullRequest: { + number: 26856, + title: "PR with unknown head repository", + url: "https://github.com/pingdotgg/codething-mvp/pull/26856", + baseRefName: "main", + headRefName: "feature/ambiguous-head", + state: "open", + }, + }, + }); + + const error = yield* preparePullRequestThread(manager, { + cwd: repoDir, + reference: "26856", + mode: "worktree", + }).pipe(Effect.flip); + + expect(error._tag).toBe("GitPullRequestMaterializationError"); + }), + ); + it.effect("preserves fork upstream tracking when preparing a worktree PR thread", () => Effect.gen(function* () { const repoDir = yield* makeTempDir("t3code-git-manager-"); diff --git a/apps/server/src/git/GitManager.ts b/apps/server/src/git/GitManager.ts index 5ea4a0072d66..3deb65fa772d 100644 --- a/apps/server/src/git/GitManager.ts +++ b/apps/server/src/git/GitManager.ts @@ -776,12 +776,27 @@ export const make = Effect.gen(function* () { ) { const repositoryNameWithOwner = resolveHeadRepositoryNameWithOwner(pullRequest) ?? ""; - if (repositoryNameWithOwner.length === 0) { - yield* gitCore.fetchPullRequestBranch({ - cwd, - prNumber: pullRequest.number, - branch: localBranch, - }); + if (repositoryNameWithOwner.length === 0 && pullRequest.isCrossRepository === false) { + yield* gitCore + .fetchPullRequestBranch({ + cwd, + prNumber: pullRequest.number, + branch: localBranch, + }) + .pipe( + // Azure DevOps publishes no pull-request head ref for same-repository PRs. + Effect.catch(() => + Effect.gen(function* () { + const remoteName = yield* gitCore.resolvePrimaryRemoteName(cwd); + yield* gitCore.fetchRemoteBranch({ + cwd, + remoteName, + remoteBranch: pullRequest.headBranch, + localBranch, + }); + }), + ), + ); return; } diff --git a/apps/server/src/pullRequest/AzureDevOpsPullRequestCli.test.ts b/apps/server/src/pullRequest/AzureDevOpsPullRequestCli.test.ts index 5baf18a1ff6a..c18901577c2f 100644 --- a/apps/server/src/pullRequest/AzureDevOpsPullRequestCli.test.ts +++ b/apps/server/src/pullRequest/AzureDevOpsPullRequestCli.test.ts @@ -476,7 +476,7 @@ layer("AzureDevOpsPullRequestCli.layer", (it) => { }), ); - it.effect("reads the conversation through the REST API, pinned to a version", () => + it.effect("reads the conversation through the Azure DevOps extension, pinned to a version", () => Effect.gen(function* () { mockedExecute.mockReturnValueOnce( Effect.succeed( @@ -499,14 +499,34 @@ layer("AzureDevOpsPullRequestCli.layer", (it) => { const comments = yield* cli.listThreads({ cwd: "/w", - threadsUrl: "https://dev.azure.com/acme/platform/_apis/git/r/web/pullRequests/42/threads", + route: { + organization: "https://dev.azure.com/acme", + project: "-platform tools", + repository: "-web repo", + pullRequestId: 42, + }, }); assert.strictEqual(comments.length, 1); - expect(argsOfCall(0)).toContain("rest"); - expect(argsOfCall(0)).toContain( - "https://dev.azure.com/acme/platform/_apis/git/r/web/pullRequests/42/threads?api-version=7.1", - ); + assert.deepStrictEqual(argsOfCall(0), [ + "devops", + "invoke", + "--org", + "https://dev.azure.com/acme", + "--area", + "git", + "--resource", + "pullRequestThreads", + "--route-parameters", + "project=-platform tools", + "repositoryId=-web repo", + "pullRequestId=42", + "--api-version", + "7.1", + "--only-show-errors", + "--output", + "json", + ]); }), ); diff --git a/apps/server/src/pullRequest/AzureDevOpsPullRequestCli.ts b/apps/server/src/pullRequest/AzureDevOpsPullRequestCli.ts index 549a172b3646..167ebe5685c5 100644 --- a/apps/server/src/pullRequest/AzureDevOpsPullRequestCli.ts +++ b/apps/server/src/pullRequest/AzureDevOpsPullRequestCli.ts @@ -18,6 +18,7 @@ import { decodeThreadsJson, decodeViewerJson, type AzureDevOpsPullRequest, + type AzureDevOpsThreadsRoute, } from "./azureDevOpsPullRequestJson.ts"; import type { ProviderListCursor } from "./PullRequestProvider.ts"; @@ -146,10 +147,10 @@ export class AzureDevOpsPullRequestCli extends Context.Service< readonly number: number; }) => Effect.Effect; - /** Threads are not reachable through `az repos pr`, so they come from the REST API. */ + /** Uses `az devops invoke` so thread reads share the extension's authentication. */ readonly listThreads: (input: { readonly cwd: string; - readonly threadsUrl: string; + readonly route: AzureDevOpsThreadsRoute; }) => Effect.Effect, AzureDevOpsPullRequestCliError>; readonly runPullRequestAction: (input: { @@ -433,11 +434,20 @@ export const make = Effect.gen(function* () { executeJson({ cwd: input.cwd, args: [ - "rest", - "--method", - "get", - "--url", - `${input.threadsUrl}?api-version=${REST_API_VERSION}`, + "devops", + "invoke", + "--org", + input.route.organization, + "--area", + "git", + "--resource", + "pullRequestThreads", + "--route-parameters", + `project=${input.route.project}`, + `repositoryId=${input.route.repository}`, + `pullRequestId=${input.route.pullRequestId}`, + "--api-version", + REST_API_VERSION, ], }).pipe( Effect.flatMap((result) => { diff --git a/apps/server/src/pullRequest/AzureDevOpsPullRequestProvider.ts b/apps/server/src/pullRequest/AzureDevOpsPullRequestProvider.ts index 631fee971cc1..13a80dcff4bc 100644 --- a/apps/server/src/pullRequest/AzureDevOpsPullRequestProvider.ts +++ b/apps/server/src/pullRequest/AzureDevOpsPullRequestProvider.ts @@ -177,9 +177,9 @@ export const make = Effect.gen(function* () { cli.getPullRequest({ cwd: input.cwd, number: input.number }).pipe( Effect.mapError(fail("getChangeRequestActivity")), Effect.flatMap((pullRequest) => - (pullRequest.threadsUrl === null + (pullRequest.threads === null ? Effect.succeed({ comments: [], truncated: true }) - : cli.listThreads({ cwd: input.cwd, threadsUrl: pullRequest.threadsUrl }).pipe( + : cli.listThreads({ cwd: input.cwd, route: pullRequest.threads }).pipe( Effect.map((comments) => ({ comments, truncated: false })), Effect.orElseSucceed(() => ({ comments: [], truncated: true })), ) diff --git a/apps/server/src/pullRequest/azureDevOpsPullRequestJson.test.ts b/apps/server/src/pullRequest/azureDevOpsPullRequestJson.test.ts index a975c89f858c..4c08998d6c77 100644 --- a/apps/server/src/pullRequest/azureDevOpsPullRequestJson.test.ts +++ b/apps/server/src/pullRequest/azureDevOpsPullRequestJson.test.ts @@ -162,12 +162,15 @@ describe("decodePullRequestJson", () => { it("works out where the conversation lives from what Azure returned", () => { const detail = expectSuccess(decodePullRequestJson(asJson(pullRequest()))); - expect(detail?.threadsUrl).toBe( - "https://dev.azure.com/acme/platform/_apis/git/repositories/web/pullRequests/42/threads", - ); + expect(detail?.threads).toEqual({ + organization: "https://dev.azure.com/acme", + project: "platform", + repository: "web", + pullRequestId: 42, + }); }); - it("reports no conversation url when Azure said too little to build one", () => { + it("reports no conversation route when Azure said too little to build one", () => { // A web link places the pull request, but without the REST url and repository there is // nothing to hang a threads collection off. const detail = expectSuccess( @@ -184,7 +187,7 @@ describe("decodePullRequestJson", () => { ), ); - expect(detail?.threadsUrl).toBeNull(); + expect(detail?.threads).toBeNull(); }); it("returns nothing when Azure gave no way to place the pull request at all", () => { diff --git a/apps/server/src/pullRequest/azureDevOpsPullRequestJson.ts b/apps/server/src/pullRequest/azureDevOpsPullRequestJson.ts index 39ca4a551d27..ccba01387267 100644 --- a/apps/server/src/pullRequest/azureDevOpsPullRequestJson.ts +++ b/apps/server/src/pullRequest/azureDevOpsPullRequestJson.ts @@ -108,6 +108,13 @@ const RawViewerSchema = Schema.Struct({ ), }); +export interface AzureDevOpsThreadsRoute { + readonly organization: string; + readonly project: string; + readonly repository: string; + readonly pullRequestId: number; +} + export interface AzureDevOpsPullRequest { readonly number: number; readonly title: string; @@ -129,7 +136,7 @@ export interface AzureDevOpsPullRequest { readonly reviewRequestLogins: ReadonlyArray; readonly reviewers: ReadonlyArray; /** Where this pull request's threads live, when Azure said enough to work it out. */ - readonly threadsUrl: string | null; + readonly threads: AzureDevOpsThreadsRoute | null; /** Whether Azure is set to complete this on its own once its policies pass. */ readonly autoMergeEnabled: boolean; } @@ -177,15 +184,17 @@ function toMergeability(value: string | null | undefined): PullRequestMergeabili } /** - * The REST collection a pull request's threads hang from. Built from what Azure returned rather - * than from the local remote, whose shape differs between the modern, legacy and SSH forms. + * The route a pull request's threads hang from. Built from what Azure returned rather than from + * the local remote, whose shape differs between the modern, legacy and SSH forms. */ -function toThreadsUrl(raw: Schema.Schema.Type): string | null { - const base = azureDevOpsOrganizationBaseFromRestApiUrl(raw.url); +function toThreadsRoute( + raw: Schema.Schema.Type, +): AzureDevOpsThreadsRoute | null { + const organization = azureDevOpsOrganizationBaseFromRestApiUrl(raw.url); const project = trimmed(raw.repository?.project?.name); const repository = trimmed(raw.repository?.name); - if (base === null || project === null || repository === null) return null; - return `${base}/${encodeURIComponent(project)}/_apis/git/repositories/${encodeURIComponent(repository)}/pullRequests/${raw.pullRequestId}/threads`; + if (organization === null || project === null || repository === null) return null; + return { organization, project, repository, pullRequestId: raw.pullRequestId }; } /** @@ -230,7 +239,7 @@ function toPullRequest( body: raw.description ?? "", reviewRequestLogins: reviewers.map((reviewer) => reviewer.login), reviewers, - threadsUrl: toThreadsUrl(raw), + threads: toThreadsRoute(raw), autoMergeEnabled: (raw.autoCompleteSetBy ?? null) !== null, }; } diff --git a/apps/server/src/sourceControl/AzureDevOpsCli.test.ts b/apps/server/src/sourceControl/AzureDevOpsCli.test.ts index a5bb9a9e30af..fdf11e0273e0 100644 --- a/apps/server/src/sourceControl/AzureDevOpsCli.test.ts +++ b/apps/server/src/sourceControl/AzureDevOpsCli.test.ts @@ -129,6 +129,121 @@ describe("AzureDevOpsCli.layer", () => { }).pipe(Effect.provide(layer)), ); + it.effect("reads a pull request whose optional fields Azure answered with null", () => + Effect.gen(function* () { + mockRun.mockReturnValueOnce( + Effect.succeed( + processOutput( + // @effect-diagnostics-next-line preferSchemaOverJson:off + JSON.stringify({ + pullRequestId: 26855, + title: "Braze iam_click event", + url: "https://dev.azure.com/acme/411da70a/_apis/git/repositories/2697671b/pullRequests/26855", + repository: { + name: "repo", + webUrl: null, + project: { name: "project" }, + }, + sourceRefName: "refs/heads/feature/iam-click", + targetRefName: "refs/heads/main", + status: "active", + creationDate: "2026-01-02T00:00:00.000Z", + closedDate: null, + _links: null, + }), + ), + ), + ); + + const az = yield* AzureDevOpsCli.AzureDevOpsCli; + const result = yield* az.getPullRequest({ cwd: "/repo", reference: "26855" }); + + assert.strictEqual(result.number, 26855); + assert.strictEqual(result.headRefName, "feature/iam-click"); + assert.strictEqual( + result.url, + "https://dev.azure.com/acme/project/_git/repo/pullrequest/26855", + ); + }).pipe(Effect.provide(layer)), + ); + + it.effect("keeps listed pull requests whose optional fields Azure answered with null", () => + Effect.gen(function* () { + mockRun.mockReturnValueOnce( + Effect.succeed( + processOutput( + // @effect-diagnostics-next-line preferSchemaOverJson:off + JSON.stringify([ + { + pullRequestId: 7, + title: "Merged work", + url: "https://dev.azure.com/acme/411da70a/_apis/git/repositories/2697671b/pullRequests/7", + repository: { name: "repo", webUrl: null, project: { name: "project" } }, + sourceRefName: "refs/heads/feature/merged", + targetRefName: "refs/heads/main", + status: "completed", + closedDate: "2026-01-03T00:00:00.000Z", + _links: null, + }, + ]), + ), + ), + ); + + const az = yield* AzureDevOpsCli.AzureDevOpsCli; + const result = yield* az.listPullRequests({ + cwd: "/repo", + headSelector: "origin:feature/merged", + state: "merged", + limit: 10, + }); + + assert.strictEqual(result.length, 1); + assert.strictEqual(result[0]?.number, 7); + }).pipe(Effect.provide(layer)), + ); + + it.effect("preserves the source repository for pull requests from Azure forks", () => + Effect.gen(function* () { + mockRun.mockReturnValueOnce( + Effect.succeed( + processOutput( + // @effect-diagnostics-next-line preferSchemaOverJson:off + JSON.stringify({ + pullRequestId: 8, + title: "Forked work", + url: "https://dev.azure.com/acme/project/_apis/git/repositories/repo/pullRequests/8", + repository: { name: "repo", project: { name: "project" } }, + forkSource: { + name: "refs/heads/feature/forked", + repository: { name: "repo-fork", project: { name: "contributor-project" } }, + }, + sourceRefName: "refs/heads/feature/forked", + targetRefName: "refs/heads/main", + status: "active", + }), + ), + ), + ); + + const az = yield* AzureDevOpsCli.AzureDevOpsCli; + const result = yield* az.getPullRequest({ cwd: "/repo", reference: "8" }); + + assert.deepStrictEqual( + { + isCrossRepository: result.isCrossRepository, + headRepositoryNameWithOwner: result.headRepositoryNameWithOwner, + headRepositoryOwnerLogin: result.headRepositoryOwnerLogin, + }, + { + isCrossRepository: true, + headRepositoryNameWithOwner: "contributor-project/repo-fork", + headRepositoryOwnerLogin: "contributor-project", + }, + ); + }).pipe(Effect.provide(layer)), + ); + it.effect("lists pull requests with Azure status and source branch arguments", () => Effect.gen(function* () { mockRun.mockReturnValueOnce( @@ -210,7 +325,7 @@ describe("AzureDevOpsCli.layer", () => { const az = yield* AzureDevOpsCli.AzureDevOpsCli; const result = yield* az.getRepositoryCloneUrls({ cwd: "/repo", - repository: "repo", + repository: "project/repo", }); assert.deepStrictEqual(result, { @@ -218,6 +333,25 @@ describe("AzureDevOpsCli.layer", () => { url: "https://dev.azure.com/acme/project/_git/repo", sshUrl: "git@ssh.dev.azure.com:v3/acme/project/repo", }); + expect(mockRun).toHaveBeenCalledWith({ + operation: "AzureDevOpsCli.execute", + command: "az", + args: [ + "repos", + "show", + "--detect", + "true", + "--repository", + "repo", + "--project", + "project", + "--only-show-errors", + "--output", + "json", + ], + cwd: "/repo", + timeoutMs: 30_000, + }); }).pipe(Effect.provide(layer)), ); diff --git a/apps/server/src/sourceControl/AzureDevOpsCli.ts b/apps/server/src/sourceControl/AzureDevOpsCli.ts index 556dc4bf213d..a8b74300c6cc 100644 --- a/apps/server/src/sourceControl/AzureDevOpsCli.ts +++ b/apps/server/src/sourceControl/AzureDevOpsCli.ts @@ -459,10 +459,19 @@ export const make = Effect.gen(function* () { ), ), ), - getRepositoryCloneUrls: (input) => - executeJson({ + getRepositoryCloneUrls: (input) => { + const repository = parseRepositorySpecifier(input.repository); + return executeJson({ cwd: input.cwd, - args: ["repos", "show", "--detect", "true", "--repository", input.repository], + args: [ + "repos", + "show", + "--detect", + "true", + "--repository", + repository.name, + ...(repository.project ? ["--project", repository.project] : []), + ], }).pipe( Effect.map((result) => result.stdout.trim()), Effect.flatMap((raw) => @@ -474,7 +483,8 @@ export const make = Effect.gen(function* () { ), ), Effect.map(normalizeRepositoryCloneUrls), - ), + ); + }, createRepository: (input) => { const repository = parseRepositorySpecifier(input.repository); // Azure Repos access is governed by project/organization permissions. diff --git a/apps/server/src/sourceControl/AzureDevOpsSourceControlProvider.test.ts b/apps/server/src/sourceControl/AzureDevOpsSourceControlProvider.test.ts index 21db25e79912..89c5e16f9313 100644 --- a/apps/server/src/sourceControl/AzureDevOpsSourceControlProvider.test.ts +++ b/apps/server/src/sourceControl/AzureDevOpsSourceControlProvider.test.ts @@ -24,6 +24,9 @@ it.effect("maps Azure DevOps PR summaries into provider-neutral change requests" headRefName: "feature/source-control", state: "open", updatedAt: Option.none(), + isCrossRepository: false, + headRepositoryNameWithOwner: null, + headRepositoryOwnerLogin: null, }), }); @@ -42,10 +45,47 @@ it.effect("maps Azure DevOps PR summaries into provider-neutral change requests" state: "open", updatedAt: Option.none(), isCrossRepository: false, + headRepositoryNameWithOwner: null, + headRepositoryOwnerLogin: null, }); }), ); +it.effect("preserves Azure fork repository metadata for pull request checkout", () => + Effect.gen(function* () { + const provider = yield* makeProvider({ + getPullRequest: () => + Effect.succeed({ + number: 43, + title: "Forked change", + url: "https://dev.azure.com/acme/project/_git/repo/pullrequest/43", + baseRefName: "main", + headRefName: "feature/forked", + state: "open", + updatedAt: Option.none(), + isCrossRepository: true, + headRepositoryNameWithOwner: "contributor-project/repo-fork", + headRepositoryOwnerLogin: "contributor-project", + }), + }); + + const changeRequest = yield* provider.getChangeRequest({ cwd: "/repo", reference: "43" }); + + assert.deepStrictEqual( + { + isCrossRepository: changeRequest.isCrossRepository, + headRepositoryNameWithOwner: changeRequest.headRepositoryNameWithOwner, + headRepositoryOwnerLogin: changeRequest.headRepositoryOwnerLogin, + }, + { + isCrossRepository: true, + headRepositoryNameWithOwner: "contributor-project/repo-fork", + headRepositoryOwnerLogin: "contributor-project", + }, + ); + }), +); + it.effect("adds change-request context while retaining Azure CLI causes", () => Effect.gen(function* () { const cause = new AzureDevOpsCli.AzureDevOpsCommandFailedError({ diff --git a/apps/server/src/sourceControl/AzureDevOpsSourceControlProvider.ts b/apps/server/src/sourceControl/AzureDevOpsSourceControlProvider.ts index 2f147452f9ec..7185d156cd9f 100644 --- a/apps/server/src/sourceControl/AzureDevOpsSourceControlProvider.ts +++ b/apps/server/src/sourceControl/AzureDevOpsSourceControlProvider.ts @@ -62,6 +62,9 @@ function toChangeRequest(summary: { readonly headRefName: string; readonly state: "open" | "closed" | "merged"; readonly updatedAt: ChangeRequest["updatedAt"]; + readonly isCrossRepository: boolean; + readonly headRepositoryNameWithOwner: string | null; + readonly headRepositoryOwnerLogin: string | null; }): ChangeRequest { return { provider: "azure-devops", @@ -72,7 +75,9 @@ function toChangeRequest(summary: { headRefName: summary.headRefName, state: summary.state, updatedAt: summary.updatedAt, - isCrossRepository: false, + isCrossRepository: summary.isCrossRepository, + headRepositoryNameWithOwner: summary.headRepositoryNameWithOwner, + headRepositoryOwnerLogin: summary.headRepositoryOwnerLogin, }; } diff --git a/apps/server/src/sourceControl/azureDevOpsPullRequests.ts b/apps/server/src/sourceControl/azureDevOpsPullRequests.ts index 8c3c5c4de56b..c407d5160892 100644 --- a/apps/server/src/sourceControl/azureDevOpsPullRequests.ts +++ b/apps/server/src/sourceControl/azureDevOpsPullRequests.ts @@ -15,22 +15,47 @@ export interface NormalizedAzureDevOpsPullRequestRecord { readonly headRefName: string; readonly state: "open" | "closed" | "merged"; readonly updatedAt: Option.Option; + readonly isCrossRepository: boolean; + readonly headRepositoryNameWithOwner: string | null; + readonly headRepositoryOwnerLogin: string | null; } const AzureDevOpsPullRequestSchema = Schema.Struct({ pullRequestId: PositiveInt, title: TrimmedNonEmptyString, - url: Schema.optional(Schema.String), + url: Schema.optional(Schema.NullOr(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), - }), - ), - }), + 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)), + }), + ), + ), + }), + ), + ), + forkSource: Schema.optional( + Schema.NullOr( + Schema.Struct({ + repository: Schema.optional( + Schema.NullOr( + Schema.Struct({ + name: Schema.optional(Schema.NullOr(Schema.String)), + project: Schema.optional( + Schema.NullOr( + Schema.Struct({ name: Schema.optional(Schema.NullOr(Schema.String)) }), + ), + ), + }), + ), + ), + }), + ), ), sourceRefName: TrimmedNonEmptyString, targetRefName: TrimmedNonEmptyString, @@ -38,13 +63,17 @@ const AzureDevOpsPullRequestSchema = Schema.Struct({ creationDate: Schema.optional(Schema.OptionFromNullOr(Schema.DateTimeUtcFromString)), closedDate: Schema.optional(Schema.OptionFromNullOr(Schema.DateTimeUtcFromString)), _links: Schema.optional( - Schema.Struct({ - web: Schema.optional( - Schema.Struct({ - href: Schema.String, - }), - ), - }), + Schema.NullOr( + Schema.Struct({ + web: Schema.optional( + Schema.NullOr( + Schema.Struct({ + href: Schema.optional(Schema.NullOr(Schema.String)), + }), + ), + ), + }), + ), ), }); @@ -161,6 +190,8 @@ function normalizeAzureDevOpsPullRequestUrl( function normalizeAzureDevOpsPullRequestRecord( raw: Schema.Schema.Type, ): NormalizedAzureDevOpsPullRequestRecord { + const forkRepositoryName = trimOptionalString(raw.forkSource?.repository?.name); + const forkProjectName = trimOptionalString(raw.forkSource?.repository?.project?.name); return { number: raw.pullRequestId, title: raw.title, @@ -171,6 +202,14 @@ function normalizeAzureDevOpsPullRequestRecord( updatedAt: (raw.closedDate ?? Option.none()).pipe( Option.orElse(() => raw.creationDate ?? Option.none()), ), + isCrossRepository: raw.forkSource != null, + headRepositoryNameWithOwner: + forkRepositoryName === null + ? null + : forkProjectName === null + ? forkRepositoryName + : `${forkProjectName}/${forkRepositoryName}`, + headRepositoryOwnerLogin: forkProjectName, }; } From 8716bfbf2e895867ee3dd433c02cef17ea11f31a Mon Sep 17 00:00:00 2001 From: Kieren Foenander Date: Thu, 27 Aug 2026 18:35:50 +1000 Subject: [PATCH 2/2] fix(server): reject incomplete Azure fork identity --- .../src/sourceControl/AzureDevOpsCli.test.ts | 32 +++++++++++++++++++ .../sourceControl/azureDevOpsPullRequests.ts | 6 ++-- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/apps/server/src/sourceControl/AzureDevOpsCli.test.ts b/apps/server/src/sourceControl/AzureDevOpsCli.test.ts index fdf11e0273e0..46ef3ddd4290 100644 --- a/apps/server/src/sourceControl/AzureDevOpsCli.test.ts +++ b/apps/server/src/sourceControl/AzureDevOpsCli.test.ts @@ -244,6 +244,38 @@ describe("AzureDevOpsCli.layer", () => { }).pipe(Effect.provide(layer)), ); + it.effect("does not expose an ambiguous Azure fork repository without its project", () => + Effect.gen(function* () { + mockRun.mockReturnValueOnce( + Effect.succeed( + processOutput( + // @effect-diagnostics-next-line preferSchemaOverJson:off + JSON.stringify({ + pullRequestId: 9, + title: "Forked work with incomplete identity", + url: "https://dev.azure.com/acme/project/_apis/git/repositories/repo/pullRequests/9", + repository: { name: "repo", project: { name: "project" } }, + forkSource: { + name: "refs/heads/feature/forked", + repository: { name: "repo" }, + }, + sourceRefName: "refs/heads/feature/forked", + targetRefName: "refs/heads/main", + status: "active", + }), + ), + ), + ); + + const az = yield* AzureDevOpsCli.AzureDevOpsCli; + const result = yield* az.getPullRequest({ cwd: "/repo", reference: "9" }); + + assert.strictEqual(result.isCrossRepository, true); + assert.strictEqual(result.headRepositoryNameWithOwner, null); + assert.strictEqual(result.headRepositoryOwnerLogin, null); + }).pipe(Effect.provide(layer)), + ); + it.effect("lists pull requests with Azure status and source branch arguments", () => Effect.gen(function* () { mockRun.mockReturnValueOnce( diff --git a/apps/server/src/sourceControl/azureDevOpsPullRequests.ts b/apps/server/src/sourceControl/azureDevOpsPullRequests.ts index c407d5160892..0082ca956629 100644 --- a/apps/server/src/sourceControl/azureDevOpsPullRequests.ts +++ b/apps/server/src/sourceControl/azureDevOpsPullRequests.ts @@ -204,11 +204,9 @@ function normalizeAzureDevOpsPullRequestRecord( ), isCrossRepository: raw.forkSource != null, headRepositoryNameWithOwner: - forkRepositoryName === null + forkRepositoryName === null || forkProjectName === null ? null - : forkProjectName === null - ? forkRepositoryName - : `${forkProjectName}/${forkRepositoryName}`, + : `${forkProjectName}/${forkRepositoryName}`, headRepositoryOwnerLogin: forkProjectName, }; }