From dee35c09005fd37dece6c68918696e2b183a4ac2 Mon Sep 17 00:00:00 2001 From: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com> Date: Fri, 9 Oct 2026 15:29:34 -0700 Subject: [PATCH 1/2] refactor(source-control): Forgejo refines repository identities from its own package server.ts built RepositoryIdentityResolver with ~25 lines of Forgejo logic: parse a Forgejo remote, ask the registry which login serves it, and derive the repository's browser URL by stripping the installation mount. That logic moves into the Forgejo provider as `refineRepositoryIdentity`, a new optional SourceControlProvider capability. server.ts now asks each built-in driver's provider that has one, in list order, and keeps the first change; it no longer imports from the Forgejo package. The provider gets a `resolveContext` callback rather than the registry, so it stays decoupled from the server. The logic is moved verbatim. It had no direct test while it lived in server.ts; the package now has one covering the installation mount, an SSH alias, another host's identity, and a remote no Forgejo login serves. Co-Authored-By: Claude Opus 5.5 --- apps/server/src/server.ts | 40 +++++-------- .../src/server/SourceControlProvider.ts | 13 ++++ .../ForgejoSourceControlProvider.test.ts | 60 +++++++++++++++++++ .../server/ForgejoSourceControlProvider.ts | 38 ++++++++++++ 4 files changed, 125 insertions(+), 26 deletions(-) diff --git a/apps/server/src/server.ts b/apps/server/src/server.ts index 80c5a73ac4c1..e24cb335b487 100644 --- a/apps/server/src/server.ts +++ b/apps/server/src/server.ts @@ -52,7 +52,6 @@ import * as ProviderHostLive from "./provider/ProviderHostLive.ts"; import * as AcpRegistrySupport from "@t3tools/provider-acp-registry/server/AcpRegistrySupport"; import * as CheckpointDiffQuery from "./checkpointing/CheckpointDiffQuery.ts"; import * as CheckpointStore from "./checkpointing/CheckpointStore.ts"; -import * as ForgejoCli from "@t3tools/source-control-forgejo/server/ForgejoCli"; import * as SourceControlBuiltInDrivers from "./sourceControl/builtInDrivers.ts"; import * as TextGeneration from "./textGeneration/TextGeneration.ts"; import * as ProviderInstanceRegistryHydration from "./provider/ProviderInstanceRegistryHydration.ts"; @@ -315,32 +314,21 @@ const layerRepositoryIdentityResolver = Layer.effect( Effect.gen(function* () { const registry = yield* SourceControlProviderRegistry.SourceControlProviderRegistry; return yield* RepositoryIdentityResolver.make({ + // Each host that can refine an identity gets a turn; the first one that changes it wins. refine: Effect.fn(function* (identity: RepositoryIdentity) { - const remote = ForgejoCli.parseForgejoRemote(identity.locator.remoteUrl); - if ( - !remote || - !identity.rootPath || - (identity.provider !== undefined && - identity.provider !== "unknown" && - identity.provider !== "forgejo") - ) - return identity; - const handle = yield* registry.resolveHandle({ - cwd: identity.rootPath, - context: { - provider: { kind: "unknown", name: "Unknown", baseUrl: "" }, - remoteName: identity.locator.remoteName, - remoteUrl: identity.locator.remoteUrl, - }, - }); - if (handle.context?.provider.kind !== "forgejo") return identity; - const baseUrl = handle.context.provider.baseUrl.replace(/\/+$/, ""); - const basePath = new URL(baseUrl).pathname.replace(/^\/+|\/+$/g, ""); - const path = - !remote.ssh && basePath && remote.path.startsWith(`${basePath}/`) - ? remote.path.slice(basePath.length + 1) - : remote.path; - return { ...identity, provider: "forgejo", webUrl: `${baseUrl}/${path}` }; + for (const kind of SourceControlBuiltInDrivers.BUILT_IN_SOURCE_CONTROL_DRIVERS.map( + (driver) => driver.kind, + )) { + const provider = yield* registry.get(kind); + if (provider.refineRepositoryIdentity === undefined) continue; + const refined = yield* provider.refineRepositoryIdentity({ + identity, + resolveContext: (input) => + registry.resolveHandle(input).pipe(Effect.map((handle) => handle.context)), + }); + if (refined !== identity) return refined; + } + return identity; }), }); }), diff --git a/packages/source-control-core/src/server/SourceControlProvider.ts b/packages/source-control-core/src/server/SourceControlProvider.ts index 74b946651faa..44d4cf4e95c5 100644 --- a/packages/source-control-core/src/server/SourceControlProvider.ts +++ b/packages/source-control-core/src/server/SourceControlProvider.ts @@ -2,6 +2,7 @@ import * as Context from "effect/Context"; import * as Effect from "effect/Effect"; import type * as Option from "effect/Option"; import type { + RepositoryIdentity, ChangeRequest, ChangeRequestState, SourceControlProviderError, @@ -135,6 +136,18 @@ export class SourceControlProvider extends Context.Service< * more than that (Forgejo's HTTP installation mount). Absent means the whole path. */ readonly repositoryNameFromRemoteUrl?: (url: string) => string | null; + /** + * Fills in what a repository identity read from git cannot know, such as the browser URL of + * a host the remote URL does not name. `resolveContext` asks the registry which host (and + * base URL) serves a remote. Only consulted for identities this host may own. + */ + readonly refineRepositoryIdentity?: (input: { + readonly identity: RepositoryIdentity; + readonly resolveContext: (input: { + readonly cwd: string; + readonly context: SourceControlProviderContext; + }) => Effect.Effect; + }) => Effect.Effect; /** Optional capability for issue and change-request subjects. */ readonly resolveLink?: ResolveSourceControlLink; readonly listChangeRequests: (input: { diff --git a/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.test.ts b/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.test.ts index bdddd5c94b40..e3dae3a6c529 100644 --- a/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.test.ts +++ b/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.test.ts @@ -245,6 +245,66 @@ it.effect.each([ }, ); +it.effect("gives Forgejo identities the browser URL of the login that serves them", () => + Effect.gen(function* () { + const provider = yield* ForgejoSourceControlProvider.make; + const refine = provider.refineRepositoryIdentity; + assert.isDefined(refine); + const identity = (remoteUrl: string, provider?: string) => ({ + canonicalKey: "forge.test/team/repo", + locator: { source: "git-remote" as const, remoteName: "origin", remoteUrl }, + rootPath: "/repo", + ...(provider === undefined ? {} : { provider }), + }); + const served = (baseUrl: string) => () => + Effect.succeed({ + provider: { kind: "forgejo" as const, name: "Forgejo / Gitea", baseUrl }, + remoteName: "origin", + remoteUrl: "", + }); + + // An installation mount in the HTTP path is the login's, not the repository's. + assert.deepStrictEqual( + yield* refine!({ + identity: identity("http://forge.test:3000/git/team/repo.git"), + resolveContext: served("http://forge.test:3000/git/"), + }), + { + ...identity("http://forge.test:3000/git/team/repo.git"), + provider: "forgejo", + webUrl: "http://forge.test:3000/git/team/repo", + }, + ); + // An SSH alias keeps its whole path. + assert.strictEqual( + (yield* refine!({ + identity: identity("ssh://git@ssh.forge.test/team/repo.git"), + resolveContext: served("http://forge.test:3000/git"), + })).webUrl, + "http://forge.test:3000/git/team/repo", + ); + // Another host's identity, or a remote no Forgejo login serves, is left alone. + const github = identity("https://github.com/team/repo.git", "github"); + assert.strictEqual( + yield* refine!({ identity: github, resolveContext: () => Effect.die("not asked") }), + github, + ); + const unserved = identity("http://forge.test:3000/team/repo.git"); + assert.strictEqual( + yield* refine!({ identity: unserved, resolveContext: () => Effect.succeed(null) }), + unserved, + ); + }).pipe( + Effect.provide( + Layer.mergeAll( + Layer.succeed(FileSystem.FileSystem, FileSystem.makeNoop({})), + TestSourceControlHost.layer(), + Layer.mock(ForgejoCli.ForgejoCli)({}), + ), + ), + ), +); + it.effect("loads Forgejo pull request references from files and commits views", () => Effect.gen(function* () { const provider = yield* ForgejoSourceControlProvider.make; diff --git a/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.ts b/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.ts index 86196975f2a3..fbcc5fbee99d 100644 --- a/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.ts +++ b/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.ts @@ -166,6 +166,43 @@ export function repositoryNameFromRemoteUrl(url: string): string | null { : path.split("/").slice(-2).join("/"); } +/** + * A Forgejo remote's URL can't say which server it belongs to (an SSH alias, an installation + * mount), so the identity's browser URL comes from the login that serves it. + */ +const refineRepositoryIdentity: NonNullable< + SourceControlProvider.SourceControlProvider["Service"]["refineRepositoryIdentity"] +> = Effect.fn("ForgejoSourceControlProvider.refineRepositoryIdentity")(function* ({ + identity, + resolveContext, +}) { + const remote = ForgejoCli.parseForgejoRemote(identity.locator.remoteUrl); + if ( + !remote || + !identity.rootPath || + (identity.provider !== undefined && + identity.provider !== "unknown" && + identity.provider !== "forgejo") + ) + return identity; + const context = yield* resolveContext({ + cwd: identity.rootPath, + context: { + provider: { kind: "unknown", name: "Unknown", baseUrl: "" }, + remoteName: identity.locator.remoteName, + remoteUrl: identity.locator.remoteUrl, + }, + }); + if (context?.provider.kind !== "forgejo") return identity; + const baseUrl = context.provider.baseUrl.replace(/\/+$/, ""); + const basePath = new URL(baseUrl).pathname.replace(/^\/+|\/+$/g, ""); + const path = + !remote.ssh && basePath && remote.path.startsWith(`${basePath}/`) + ? remote.path.slice(basePath.length + 1) + : remote.path; + return { ...identity, provider: "forgejo", webUrl: `${baseUrl}/${path}` }; +}); + const RepositorySchema = Schema.Struct({ full_name: Schema.String, clone_url: Schema.String, @@ -237,6 +274,7 @@ export const make = Effect.gen(function* () { return SourceControlProvider.SourceControlProvider.of({ kind: "forgejo", repositoryNameFromRemoteUrl, + refineRepositoryIdentity, listChangeRequests: (input) => Effect.gen(function* () { const repo = yield* cli.resolveRepository(input); From 5012fa4b73ea7bbf44c75eaefdaa2a6123d00116 Mon Sep 17 00:00:00 2001 From: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com> Date: Fri, 9 Oct 2026 15:32:57 -0700 Subject: [PATCH 2/2] refactor(source-control): PullRequestService reads Azure DevOps addressing, not its kind PullRequestService checked `kind === "azure-devops"` in eight places: the list cursor key, the routing remote, which checkout serves a hosted reference, and the repository a completed action reports. All eight encode one fact: Azure DevOps names a repository by the checkout's full canonical key (`host/org/project/repo`), and derives its organization from that checkout. That fact is now a resolver on PullRequestProviderApi, `repositoryKey`, which the Azure DevOps provider implements. A host without one is identified by `owner/name`. The service resolves the key once per checkout into SupportedProject.repositoryKey and routes on it, so generic code asks "what is this repository's key?" rather than "is this Azure?". Behavior is unchanged. Co-Authored-By: Claude Opus 5.5 --- .../pullRequest/PullRequestService.test.ts | 5 ++- .../src/pullRequest/PullRequestService.ts | 37 +++++++++---------- .../server/AzureDevOpsPullRequestProvider.ts | 2 + .../src/server/PullRequestProvider.ts | 13 +++++++ 4 files changed, 37 insertions(+), 20 deletions(-) diff --git a/apps/server/src/pullRequest/PullRequestService.test.ts b/apps/server/src/pullRequest/PullRequestService.test.ts index 04d4c879a656..c74faa92f06d 100644 --- a/apps/server/src/pullRequest/PullRequestService.test.ts +++ b/apps/server/src/pullRequest/PullRequestService.test.ts @@ -463,8 +463,11 @@ function fakeProvider( setReaction: () => Effect.void, listReviewerCandidates: () => Effect.succeed({ candidates: [], truncated: false }), setReviewerRequest: () => Effect.void, - // GitHub's own merge message rewrite, which the service reads instead of the kind. + // The hosts' own resolvers, which the service reads instead of the kind. ...(kind === "github" ? { mergeMessageRewrite: (message: string) => message } : {}), + ...(kind === "azure-devops" + ? { repositoryKey: ({ canonicalKey }: { readonly canonicalKey: string }) => canonicalKey } + : {}), ...overrides, }; } diff --git a/apps/server/src/pullRequest/PullRequestService.ts b/apps/server/src/pullRequest/PullRequestService.ts index 3644fefe985a..b44d713c3158 100644 --- a/apps/server/src/pullRequest/PullRequestService.ts +++ b/apps/server/src/pullRequest/PullRequestService.ts @@ -369,6 +369,12 @@ export interface SupportedProject { * Unique where `repository` is not: Azure's is a bare name that repeats across an organisation. */ readonly remote: string; + /** + * The host's own key for this checkout's repository (`api.repositoryKey`), normalized, or null + * where `owner/name` on the host identifies it. When set, only a reference whose key matches + * is served by this checkout. + */ + readonly repositoryKey: string | null; } /** @@ -589,6 +595,7 @@ function withRateLimitBackoff( ...(api.mergeMessageRewrite === undefined ? {} : { mergeMessageRewrite: api.mergeMessageRewrite }), + ...(api.repositoryKey === undefined ? {} : { repositoryKey: api.repositoryKey }), // Refused during a pause like any other read, except for the caller that asks for the // bypass: a lookup that failed is not held, so letting every background read through would // spawn this host's CLI on each of them and re-extend the pause it was already in. @@ -852,10 +859,9 @@ export const make = Effect.gen(function* () { if (roots === undefined) viewerRoots.set(host, [project.workspaceRoot]); else if (!roots.includes(project.workspaceRoot)) roots.push(project.workspaceRoot); } - const key = listCursorKey( - host, - kind === "azure-devops" ? identity.canonicalKey : repository, - ); + const repositoryKey = + api?.repositoryKey?.({ canonicalKey: identity.canonicalKey }) ?? null; + const key = listCursorKey(host, repositoryKey ?? repository); if (seen.has(key)) continue; seen.add(key); if (api === null) { @@ -870,10 +876,9 @@ export const make = Effect.gen(function* () { api: withRateLimitBackoff(api, host, rateLimits), repository, host, - remote: - kind === "azure-devops" - ? identity.canonicalKey - : normalizeGitRemoteUrl(`https://${host}/${repository}`), + remote: repositoryKey ?? normalizeGitRemoteUrl(`https://${host}/${repository}`), + repositoryKey: + repositoryKey === null ? null : canonicalRepositoryKey(repositoryKey.toLowerCase()), }); } return { supported, unimplemented, viewerRoots }; @@ -922,25 +927,21 @@ export const make = Effect.gen(function* () { const route = supported.find( (candidate) => - candidate.api.kind === "azure-devops" && - candidate.project.repositoryIdentity != null && - canonicalRepositoryKey( - candidate.project.repositoryIdentity.canonicalKey.toLowerCase(), - ) === repositoryKey, + candidate.repositoryKey !== null && candidate.repositoryKey === repositoryKey, ) ?? onHost.find( (candidate) => - candidate.api.kind !== "azure-devops" && + candidate.repositoryKey === null && candidate.repository.toLowerCase() === repository.toLowerCase(), ) ?? - onHost.find((candidate) => candidate.api.kind !== "azure-devops"); + onHost.find((candidate) => candidate.repositoryKey === null); if (route === undefined) { return Effect.fail( new PullRequestUnavailableError({ reason: "provider-unsupported" }), ); } return Effect.succeed( - route.api.kind === "azure-devops" || + route.repositoryKey !== null || route.repository.toLowerCase() === repository.toLowerCase() ? route : { @@ -2099,9 +2100,7 @@ export const make = Effect.gen(function* () { ), Effect.mapError(toPullRequestError("runAction")), Effect.as( - project.api.kind === "azure-devops" - ? input.repository.trim() - : project.repository, + project.repositoryKey !== null ? input.repository.trim() : project.repository, ), ), ), diff --git a/packages/source-control-azure-devops/src/server/AzureDevOpsPullRequestProvider.ts b/packages/source-control-azure-devops/src/server/AzureDevOpsPullRequestProvider.ts index afd045636947..b30356bba7b5 100644 --- a/packages/source-control-azure-devops/src/server/AzureDevOpsPullRequestProvider.ts +++ b/packages/source-control-azure-devops/src/server/AzureDevOpsPullRequestProvider.ts @@ -298,6 +298,8 @@ export const make = Effect.gen(function* () { const provider: PullRequestProviderApi = { kind: "azure-devops", capabilities: CAPABILITIES, + // The bare repository name repeats across an organization; the project path does not. + repositoryKey: ({ canonicalKey }) => canonicalKey, getViewer: (input) => cli.getViewer({ cwd: input.cwd }).pipe(Effect.mapError(fail("getViewer"))), diff --git a/packages/source-control-core/src/server/PullRequestProvider.ts b/packages/source-control-core/src/server/PullRequestProvider.ts index 2500c8fae62a..85f5d1893ffb 100644 --- a/packages/source-control-core/src/server/PullRequestProvider.ts +++ b/packages/source-control-core/src/server/PullRequestProvider.ts @@ -352,6 +352,19 @@ export interface PullRequestProviderApi { * project asks for it. */ readonly mergeMessageRewrite?: (message: string) => string; + /** + * The key a checkout's repository is known by on this host, where `owner/name` is not enough. + * Absent means `owner/name` on the host identifies it, and any checkout on the host can serve a + * change request in another repository there. + * + * A host that resolves it (Azure DevOps: `dev.azure.com/org/project/_git/repo`) derives its + * organization from the checkout, so only a checkout whose key matches the reference serves + * it. The key also names the repository in list cursors and on routing. + */ + readonly repositoryKey?: (input: { + /** The checkout's canonical key, as its remote names it. */ + readonly canonicalKey: string; + }) => string; /** The signed-in account, which is what involvement filtering compares against. */ readonly getViewer: (input: {