diff --git a/apps/server/src/git/GitManager.test.ts b/apps/server/src/git/GitManager.test.ts index 643e1e7a0d5b..0c4fc4d34a53 100644 --- a/apps/server/src/git/GitManager.test.ts +++ b/apps/server/src/git/GitManager.test.ts @@ -4384,6 +4384,91 @@ 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("fails closed when a fork 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, ghCalls } = 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", + isCrossRepository: true, + }, + }, + }); + + const error = yield* preparePullRequestThread(manager, { + cwd: repoDir, + reference: "26856", + mode: "worktree", + }).pipe(Effect.flip); + + expect(error._tag).toBe("GitPullRequestMaterializationError"); + expect(ghCalls.some((call) => call.startsWith("repo view "))).toBe(false); + expect(ghCalls).not.toContain("pr checkout 26856 --force"); + }), + ); + 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 2d8af0c9e8bb..d7e544206539 100644 --- a/apps/server/src/git/GitManager.ts +++ b/apps/server/src/git/GitManager.ts @@ -832,12 +832,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; } @@ -876,8 +891,24 @@ export const make = Effect.gen(function* () { cwd: string, pullRequest: ResolvedPullRequest & PullRequestHeadRemoteInfo, localBranch = pullRequest.headBranch, - ) => - materializePullRequestHeadBranchBase(cwd, pullRequest, localBranch).pipe( + ) => { + const headRepository = resolveHeadRepositoryNameWithOwner(pullRequest); + if (pullRequest.isCrossRepository === true && headRepository === null) { + return Effect.fail( + new GitPullRequestMaterializationError({ + cwd, + pullRequestNumber: pullRequest.number, + headRepository, + headBranch: pullRequest.headBranch, + localBranch, + cause: new Error( + "Cross-repository pull request is missing its head repository identity.", + ), + }), + ); + } + + return materializePullRequestHeadBranchBase(cwd, pullRequest, localBranch).pipe( Effect.catch((primaryCause) => gitCore .fetchPullRequestBranch({ @@ -904,6 +935,7 @@ export const make = Effect.gen(function* () { ), ), ); + }; const tempDir = process.env.TMPDIR ?? process.env.TEMP ?? process.env.TMP ?? "/tmp"; const canonicalizeExistingPath = (value: string) => fileSystem.realPath(value).pipe(Effect.orElseSucceed(() => value)); diff --git a/apps/server/src/pullRequest/AzureDevOpsPullRequestCli.test.ts b/apps/server/src/pullRequest/AzureDevOpsPullRequestCli.test.ts index d893924b3f2a..68b22be370dd 100644 --- a/apps/server/src/pullRequest/AzureDevOpsPullRequestCli.test.ts +++ b/apps/server/src/pullRequest/AzureDevOpsPullRequestCli.test.ts @@ -516,7 +516,7 @@ layer("AzureDevOpsPullRequestCli.layer", (it) => { }), ); - it.effect("reads the conversation through the REST API, pinned to a version", () => + it.effect("reads the authenticated conversation through the REST API, pinned to a version", () => Effect.gen(function* () { mockedExecute.mockReturnValueOnce( Effect.succeed( @@ -543,10 +543,18 @@ layer("AzureDevOpsPullRequestCli.layer", (it) => { }); assert.strictEqual(comments.length, 1); - expect(argsOfCall(0)).toContain("rest"); - expect(argsOfCall(0)).toContain( + assert.deepStrictEqual(argsOfCall(0), [ + "rest", + "--method", + "get", + "--url", "https://dev.azure.com/acme/platform/_apis/git/r/web/pullRequests/42/threads?api-version=7.1", - ); + "--resource", + "499b84ac-1321-427f-aa17-267ca6975798", + "--only-show-errors", + "--output", + "json", + ]); }), ); diff --git a/apps/server/src/pullRequest/AzureDevOpsPullRequestCli.ts b/apps/server/src/pullRequest/AzureDevOpsPullRequestCli.ts index fe87692e1cc3..8ed299215641 100644 --- a/apps/server/src/pullRequest/AzureDevOpsPullRequestCli.ts +++ b/apps/server/src/pullRequest/AzureDevOpsPullRequestCli.ts @@ -111,6 +111,7 @@ export type AzureDevOpsPullRequestCliError = /** The version every REST call below is pinned to, so a new default cannot reshape a response. */ const REST_API_VERSION = "7.1"; +const AZURE_DEVOPS_RESOURCE_ID = "499b84ac-1321-427f-aa17-267ca6975798"; const PULL_REQUEST_LIST_MAX_OUTPUT_BYTES = 16 * 1024 * 1024; export class AzureDevOpsPullRequestCli extends Context.Service< @@ -455,6 +456,8 @@ export const make = Effect.gen(function* () { "get", "--url", `${input.threadsUrl}?api-version=${REST_API_VERSION}`, + "--resource", + AZURE_DEVOPS_RESOURCE_ID, ], }).pipe( Effect.flatMap((result) => { diff --git a/apps/server/src/pullRequest/azureDevOpsPullRequestJson.ts b/apps/server/src/pullRequest/azureDevOpsPullRequestJson.ts index 55d9b544ab3d..38027b99523a 100644 --- a/apps/server/src/pullRequest/azureDevOpsPullRequestJson.ts +++ b/apps/server/src/pullRequest/azureDevOpsPullRequestJson.ts @@ -189,7 +189,7 @@ 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 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); diff --git a/apps/server/src/sourceControl/AzureDevOpsCli.test.ts b/apps/server/src/sourceControl/AzureDevOpsCli.test.ts index f0cb52003029..12d05117f0dd 100644 --- a/apps/server/src/sourceControl/AzureDevOpsCli.test.ts +++ b/apps/server/src/sourceControl/AzureDevOpsCli.test.ts @@ -129,6 +129,153 @@ 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("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( @@ -210,7 +357,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 +365,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 f05f4a7588c4..a91cfc466e67 100644 --- a/apps/server/src/sourceControl/AzureDevOpsCli.ts +++ b/apps/server/src/sourceControl/AzureDevOpsCli.ts @@ -461,10 +461,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) => @@ -476,7 +485,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 cacdd1a3cd97..8d1a1405274d 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 8a840c524eba..e86fed2f6c20 100644 --- a/apps/server/src/sourceControl/AzureDevOpsSourceControlProvider.ts +++ b/apps/server/src/sourceControl/AzureDevOpsSourceControlProvider.ts @@ -63,6 +63,9 @@ function toChangeRequest(summary: { readonly state: "open" | "closed" | "merged"; readonly isDraft?: boolean; readonly updatedAt: ChangeRequest["updatedAt"]; + readonly isCrossRepository: boolean; + readonly headRepositoryNameWithOwner: string | null; + readonly headRepositoryOwnerLogin: string | null; }): ChangeRequest { return { provider: "azure-devops", @@ -74,7 +77,9 @@ function toChangeRequest(summary: { state: summary.state, ...(summary.isDraft === true ? { isDraft: true } : {}), 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 8ac682399e1d..2b52884dccf6 100644 --- a/apps/server/src/sourceControl/azureDevOpsPullRequests.ts +++ b/apps/server/src/sourceControl/azureDevOpsPullRequests.ts @@ -16,22 +16,47 @@ export interface NormalizedAzureDevOpsPullRequestRecord { readonly state: "open" | "closed" | "merged"; readonly isDraft?: boolean; 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, @@ -40,13 +65,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)), + }), + ), + ), + }), + ), ), }); @@ -163,6 +192,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, @@ -174,6 +205,12 @@ function normalizeAzureDevOpsPullRequestRecord( updatedAt: (raw.closedDate ?? Option.none()).pipe( Option.orElse(() => raw.creationDate ?? Option.none()), ), + isCrossRepository: raw.forkSource != null, + headRepositoryNameWithOwner: + forkRepositoryName === null || forkProjectName === null + ? null + : `${forkProjectName}/${forkRepositoryName}`, + headRepositoryOwnerLogin: forkProjectName, }; }