From 9d23048bb54354d27b607fcde752b49b64c3220c Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Tue, 3 Nov 2026 00:31:43 +0000 Subject: [PATCH 1/2] test(server): reproduce #29753 review guide refusing Forgejo SSH-remote checkouts A Forgejo project whose origin is an SSH remote on a host other than the web UI keys its own repository under the SSH host, so "Write review guide" says the pull request is in another repository. This test reproduces #29753 and fails until the fix lands. Co-Authored-By: Claude Opus 5.5 --- .../PullRequestReviewGuideService.test.ts | 43 ++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/apps/server/src/pullRequest/PullRequestReviewGuideService.test.ts b/apps/server/src/pullRequest/PullRequestReviewGuideService.test.ts index a4d944a457286..057c78251ad85 100644 --- a/apps/server/src/pullRequest/PullRequestReviewGuideService.test.ts +++ b/apps/server/src/pullRequest/PullRequestReviewGuideService.test.ts @@ -158,7 +158,7 @@ const makeLayer = (input: { readonly stored: Deferred.Deferred; readonly enabled?: boolean; readonly currentHead?: Ref.Ref; - readonly identity?: typeof ACME_APP; + readonly identity?: typeof ACME_APP & { readonly webUrl?: string }; }) => PullRequestReviewGuideService.layer.pipe( Layer.provide( @@ -589,4 +589,45 @@ it.layer(layerBase, { excludeTestServices: true })("PullRequestReviewGuideServic }), ), ); + + it.effect("writes a guide for a Forgejo checkout whose SSH remote is not the web host", () => + Effect.scoped( + Effect.gen(function* () { + const repositories = yield* makeRepositories; + const stored = yield* Deferred.make(); + const context = yield* Layer.build( + makeLayer({ + ...repositories, + stored, + // The checkout's remote is not HTTP, so only the signed-in server's web URL names the host. + identity: { + ...ACME_APP, + provider: "forgejo", + canonicalKey: "git-ssh.internal/owner/repo", + displayName: "owner/repo", + owner: "owner", + name: "repo", + webUrl: "https://git.example.com/owner/repo", + }, + generate: () => Effect.succeed({ stops: [] }), + }), + ); + const service = Context.get( + context, + PullRequestReviewGuideService.PullRequestReviewGuideService, + ); + const forgejoReference: PullRequestRef = { + projectId, + host: "git.example.com", + repository: "owner/repo", + number: 7, + }; + const started = yield* service.generate(forgejoReference); + assert.isTrue(started.started); + yield* Deferred.await(stored); + const result = yield* service.get(forgejoReference); + assert.strictEqual(result.guide?.headSha, repositories.headSha); + }), + ), + ); }); From da3df0e6f0615ebc0f4b76e450bcafd120c06e47 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Tue, 3 Nov 2026 00:36:38 +0000 Subject: [PATCH 2/2] fix(forgejo): review guides work for checkouts whose SSH remote is not the web host Forgejo's changeRequestHost ignored the resolved web URL for a non-HTTP remote and keyed the project under the SSH host, so the review guide refused Forgejo pull requests as being in another repository. Prefer the web URL's host (port kept), as changeRequestUrl and the repository matchers already do. Fixes pingdotgg/t3code#29753 Co-Authored-By: Claude Opus 5.5 --- packages/source-control-core/src/shared/definition.ts | 1 + .../source-control-forgejo/src/shared/definition.test.ts | 7 +++++++ packages/source-control-forgejo/src/shared/definition.ts | 9 +++++---- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/packages/source-control-core/src/shared/definition.ts b/packages/source-control-core/src/shared/definition.ts index ed12267c25a09..6d8c0d91e24de 100644 --- a/packages/source-control-core/src/shared/definition.ts +++ b/packages/source-control-core/src/shared/definition.ts @@ -68,6 +68,7 @@ export type ChangeRequestHostIdentity = | { readonly canonicalKey?: string | undefined; readonly locator?: { readonly remoteUrl: string } | undefined; + readonly webUrl?: string | undefined; } | null | undefined; diff --git a/packages/source-control-forgejo/src/shared/definition.test.ts b/packages/source-control-forgejo/src/shared/definition.test.ts index 19628b44d3cdf..2af834e410f30 100644 --- a/packages/source-control-forgejo/src/shared/definition.test.ts +++ b/packages/source-control-forgejo/src/shared/definition.test.ts @@ -127,5 +127,12 @@ describe("Forgejo change request host", () => { expect( definition.changeRequestHost?.(identity("ssh://git@forge.example:2222/git/team/repo.git")), ).toBe("forge.example"); + expect( + definition.changeRequestHost?.( + identity("ssh://git@ssh.forge.example:2222/git/team/repo.git", { + webUrl: "http://forge.example:3000/git/team/repo", + }), + ), + ).toBe("forge.example:3000"); }); }); diff --git a/packages/source-control-forgejo/src/shared/definition.ts b/packages/source-control-forgejo/src/shared/definition.ts index c75b071c2ab80..4fb9750acc6ac 100644 --- a/packages/source-control-forgejo/src/shared/definition.ts +++ b/packages/source-control-forgejo/src/shared/definition.ts @@ -3,7 +3,6 @@ import { buildCommitUrl, changeRequestUrlBelow, type ChangeRequestHostIdentity, - type ChangeRequestProjectIdentity, defineSourceControl, isChangeRequestAuthorityOfProject, isChangeRequestInProjectRepository, @@ -31,8 +30,8 @@ const changeRequestHeadRef = (number: number) => `refs/pull/${number}/head`; * The repository's browser URL the server resolved from the signed-in account, which is the * only place a Forgejo mounted below a path says so. */ -function resolvedWebUrl(identity: ChangeRequestProjectIdentity): URL | null { - if (!identity.webUrl) return null; +function resolvedWebUrl(identity: ChangeRequestHostIdentity): URL | null { + if (!identity?.webUrl) return null; try { const url = new URL(identity.webUrl); return url.protocol === "http:" || url.protocol === "https:" ? url : null; @@ -41,8 +40,10 @@ function resolvedWebUrl(identity: ChangeRequestProjectIdentity): URL | null { } } -/** A server's identity includes its port, which only an HTTP remote names. */ +/** A server's identity includes its port, which only its web URL or an HTTP remote names. */ function changeRequestHost(identity: ChangeRequestHostIdentity): string { + const web = resolvedWebUrl(identity); + if (web) return web.host.toLowerCase(); try { const remote = new URL(identity?.locator?.remoteUrl ?? ""); if (remote.protocol === "http:" || remote.protocol === "https:") {