authorgravatar for julius0216@outlook.comJulius Marminge <julius0216@outlook.com> 2026-10-09 16:22:37-07:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-10-09 16:22:37-07:00
log51347aca3aa73112bc1e288ae45507ce515f42a2
treec3e20f2b9364265174035ec4ba8977d0488e6dcc
parent2ce3e4ae74deb63bda7aaf059949a12aaaefed56
signature Signed by PGP key B5690EEEBB952194

refactor(source-control): Forgejo identity and Azure DevOps addressing move into their packages (#17624)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

8 files changed, 162 insertions(+), 46 deletions(-)

apps/server/src/pullRequest/PullRequestService.test.ts+4-1
...@@ -463,8 +463,11 @@ function fakeProvider(...@@ -463,8 +463,11 @@ function fakeProvider(
463 setReaction: () => Effect.void,463 setReaction: () => Effect.void,
464 listReviewerCandidates: () => Effect.succeed({ candidates: [], truncated: false }),464 listReviewerCandidates: () => Effect.succeed({ candidates: [], truncated: false }),
465 setReviewerRequest: () => Effect.void,465 setReviewerRequest: () => Effect.void,
466 // GitHub's own merge message rewrite, which the service reads instead of the kind.466 // The hosts' own resolvers, which the service reads instead of the kind.
467 ...(kind === "github" ? { mergeMessageRewrite: (message: string) => message } : {}),467 ...(kind === "github" ? { mergeMessageRewrite: (message: string) => message } : {}),
468 ...(kind === "azure-devops"
469 ? { repositoryKey: ({ canonicalKey }: { readonly canonicalKey: string }) => canonicalKey }
470 : {}),
468 ...overrides,471 ...overrides,
469 };472 };
470}473}
apps/server/src/pullRequest/PullRequestService.ts+18-19
...@@ -369,6 +369,12 @@ export interface SupportedProject {...@@ -369,6 +369,12 @@ export interface SupportedProject {
369 * Unique where `repository` is not: Azure's is a bare name that repeats across an organisation.369 * Unique where `repository` is not: Azure's is a bare name that repeats across an organisation.
370 */370 */
371 readonly remote: string;371 readonly remote: string;
372 /**
373 * The host's own key for this checkout's repository (`api.repositoryKey`), normalized, or null
374 * where `owner/name` on the host identifies it. When set, only a reference whose key matches
375 * is served by this checkout.
376 */
377 readonly repositoryKey: string | null;
372}378}
373379
374/**380/**
...@@ -589,6 +595,7 @@ function withRateLimitBackoff(...@@ -589,6 +595,7 @@ function withRateLimitBackoff(
589 ...(api.mergeMessageRewrite === undefined595 ...(api.mergeMessageRewrite === undefined
590 ? {}596 ? {}
591 : { mergeMessageRewrite: api.mergeMessageRewrite }),597 : { mergeMessageRewrite: api.mergeMessageRewrite }),
598 ...(api.repositoryKey === undefined ? {} : { repositoryKey: api.repositoryKey }),
592 // Refused during a pause like any other read, except for the caller that asks for the599 // Refused during a pause like any other read, except for the caller that asks for the
593 // bypass: a lookup that failed is not held, so letting every background read through would600 // bypass: a lookup that failed is not held, so letting every background read through would
594 // spawn this host's CLI on each of them and re-extend the pause it was already in.601 // 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* () {...@@ -852,10 +859,9 @@ export const make = Effect.gen(function* () {
852 if (roots === undefined) viewerRoots.set(host, [project.workspaceRoot]);859 if (roots === undefined) viewerRoots.set(host, [project.workspaceRoot]);
853 else if (!roots.includes(project.workspaceRoot)) roots.push(project.workspaceRoot);860 else if (!roots.includes(project.workspaceRoot)) roots.push(project.workspaceRoot);
854 }861 }
855 const key = listCursorKey(862 const repositoryKey =
856 host,863 api?.repositoryKey?.({ canonicalKey: identity.canonicalKey }) ?? null;
857 kind === "azure-devops" ? identity.canonicalKey : repository,864 const key = listCursorKey(host, repositoryKey ?? repository);
858 );
859 if (seen.has(key)) continue;865 if (seen.has(key)) continue;
860 seen.add(key);866 seen.add(key);
861 if (api === null) {867 if (api === null) {
...@@ -870,10 +876,9 @@ export const make = Effect.gen(function* () {...@@ -870,10 +876,9 @@ export const make = Effect.gen(function* () {
870 api: withRateLimitBackoff(api, host, rateLimits),876 api: withRateLimitBackoff(api, host, rateLimits),
871 repository,877 repository,
872 host,878 host,
873 remote:879 remote: repositoryKey ?? normalizeGitRemoteUrl(`https://${host}/${repository}`),
874 kind === "azure-devops"880 repositoryKey:
875 ? identity.canonicalKey881 repositoryKey === null ? null : canonicalRepositoryKey(repositoryKey.toLowerCase()),
876 : normalizeGitRemoteUrl(`https://${host}/${repository}`),
877 });882 });
878 }883 }
879 return { supported, unimplemented, viewerRoots };884 return { supported, unimplemented, viewerRoots };
...@@ -922,25 +927,21 @@ export const make = Effect.gen(function* () {...@@ -922,25 +927,21 @@ export const make = Effect.gen(function* () {
922 const route =927 const route =
923 supported.find(928 supported.find(
924 (candidate) =>929 (candidate) =>
925 candidate.api.kind === "azure-devops" &&930 candidate.repositoryKey !== null && candidate.repositoryKey === repositoryKey,
926 candidate.project.repositoryIdentity != null &&
927 canonicalRepositoryKey(
928 candidate.project.repositoryIdentity.canonicalKey.toLowerCase(),
929 ) === repositoryKey,
930 ) ??931 ) ??
931 onHost.find(932 onHost.find(
932 (candidate) =>933 (candidate) =>
933 candidate.api.kind !== "azure-devops" &&934 candidate.repositoryKey === null &&
934 candidate.repository.toLowerCase() === repository.toLowerCase(),935 candidate.repository.toLowerCase() === repository.toLowerCase(),
935 ) ??936 ) ??
936 onHost.find((candidate) => candidate.api.kind !== "azure-devops");937 onHost.find((candidate) => candidate.repositoryKey === null);
937 if (route === undefined) {938 if (route === undefined) {
938 return Effect.fail(939 return Effect.fail(
939 new PullRequestUnavailableError({ reason: "provider-unsupported" }),940 new PullRequestUnavailableError({ reason: "provider-unsupported" }),
940 );941 );
941 }942 }
942 return Effect.succeed(943 return Effect.succeed(
943 route.api.kind === "azure-devops" ||944 route.repositoryKey !== null ||
944 route.repository.toLowerCase() === repository.toLowerCase()945 route.repository.toLowerCase() === repository.toLowerCase()
945 ? route946 ? route
946 : {947 : {
...@@ -2099,9 +2100,7 @@ export const make = Effect.gen(function* () {...@@ -2099,9 +2100,7 @@ export const make = Effect.gen(function* () {
2099 ),2100 ),
2100 Effect.mapError(toPullRequestError("runAction")),2101 Effect.mapError(toPullRequestError("runAction")),
2101 Effect.as(2102 Effect.as(
2102 project.api.kind === "azure-devops"2103 project.repositoryKey !== null ? input.repository.trim() : project.repository,
2103 ? input.repository.trim()
2104 : project.repository,
2105 ),2104 ),
2106 ),2105 ),
2107 ),2106 ),
apps/server/src/server.ts+14-26
...@@ -52,7 +52,6 @@ import * as ProviderHostLive from "./provider/ProviderHostLive.ts";...@@ -52,7 +52,6 @@ import * as ProviderHostLive from "./provider/ProviderHostLive.ts";
52import * as AcpRegistrySupport from "@t3tools/provider-acp-registry/server/AcpRegistrySupport";52import * as AcpRegistrySupport from "@t3tools/provider-acp-registry/server/AcpRegistrySupport";
53import * as CheckpointDiffQuery from "./checkpointing/CheckpointDiffQuery.ts";53import * as CheckpointDiffQuery from "./checkpointing/CheckpointDiffQuery.ts";
54import * as CheckpointStore from "./checkpointing/CheckpointStore.ts";54import * as CheckpointStore from "./checkpointing/CheckpointStore.ts";
55import * as ForgejoCli from "@t3tools/source-control-forgejo/server/ForgejoCli";
56import * as SourceControlBuiltInDrivers from "./sourceControl/builtInDrivers.ts";55import * as SourceControlBuiltInDrivers from "./sourceControl/builtInDrivers.ts";
57import * as TextGeneration from "./textGeneration/TextGeneration.ts";56import * as TextGeneration from "./textGeneration/TextGeneration.ts";
58import * as ProviderInstanceRegistryHydration from "./provider/ProviderInstanceRegistryHydration.ts";57import * as ProviderInstanceRegistryHydration from "./provider/ProviderInstanceRegistryHydration.ts";
...@@ -315,32 +314,21 @@ const layerRepositoryIdentityResolver = Layer.effect(...@@ -315,32 +314,21 @@ const layerRepositoryIdentityResolver = Layer.effect(
315 Effect.gen(function* () {314 Effect.gen(function* () {
316 const registry = yield* SourceControlProviderRegistry.SourceControlProviderRegistry;315 const registry = yield* SourceControlProviderRegistry.SourceControlProviderRegistry;
317 return yield* RepositoryIdentityResolver.make({316 return yield* RepositoryIdentityResolver.make({
317 // Each host that can refine an identity gets a turn; the first one that changes it wins.
318 refine: Effect.fn(function* (identity: RepositoryIdentity) {318 refine: Effect.fn(function* (identity: RepositoryIdentity) {
319 const remote = ForgejoCli.parseForgejoRemote(identity.locator.remoteUrl);319 for (const kind of SourceControlBuiltInDrivers.BUILT_IN_SOURCE_CONTROL_DRIVERS.map(
320 if (320 (driver) => driver.kind,
321 !remote ||321 )) {
322 !identity.rootPath ||322 const provider = yield* registry.get(kind);
323 (identity.provider !== undefined &&323 if (provider.refineRepositoryIdentity === undefined) continue;
324 identity.provider !== "unknown" &&324 const refined = yield* provider.refineRepositoryIdentity({
325 identity.provider !== "forgejo")325 identity,
326 )326 resolveContext: (input) =>
327 return identity;327 registry.resolveHandle(input).pipe(Effect.map((handle) => handle.context)),
328 const handle = yield* registry.resolveHandle({328 });
329 cwd: identity.rootPath,329 if (refined !== identity) return refined;
330 context: {330 }
331 provider: { kind: "unknown", name: "Unknown", baseUrl: "" },331 return identity;
332 remoteName: identity.locator.remoteName,
333 remoteUrl: identity.locator.remoteUrl,
334 },
335 });
336 if (handle.context?.provider.kind !== "forgejo") return identity;
337 const baseUrl = handle.context.provider.baseUrl.replace(/\/+$/, "");
338 const basePath = new URL(baseUrl).pathname.replace(/^\/+|\/+$/g, "");
339 const path =
340 !remote.ssh && basePath && remote.path.startsWith(`${basePath}/`)
341 ? remote.path.slice(basePath.length + 1)
342 : remote.path;
343 return { ...identity, provider: "forgejo", webUrl: `${baseUrl}/${path}` };
344 }),332 }),
345 });333 });
346 }),334 }),
packages/source-control-azure-devops/src/server/AzureDevOpsPullRequestProvider.ts+2
...@@ -298,6 +298,8 @@ export const make = Effect.gen(function* () {...@@ -298,6 +298,8 @@ export const make = Effect.gen(function* () {
298 const provider: PullRequestProviderApi = {298 const provider: PullRequestProviderApi = {
299 kind: "azure-devops",299 kind: "azure-devops",
300 capabilities: CAPABILITIES,300 capabilities: CAPABILITIES,
301 // The bare repository name repeats across an organization; the project path does not.
302 repositoryKey: ({ canonicalKey }) => canonicalKey,
301303
302 getViewer: (input) =>304 getViewer: (input) =>
303 cli.getViewer({ cwd: input.cwd }).pipe(Effect.mapError(fail("getViewer"))),305 cli.getViewer({ cwd: input.cwd }).pipe(Effect.mapError(fail("getViewer"))),
packages/source-control-core/src/server/PullRequestProvider.ts+13
...@@ -352,6 +352,19 @@ export interface PullRequestProviderApi {...@@ -352,6 +352,19 @@ export interface PullRequestProviderApi {
352 * project asks for it.352 * project asks for it.
353 */353 */
354 readonly mergeMessageRewrite?: (message: string) => string;354 readonly mergeMessageRewrite?: (message: string) => string;
355 /**
356 * The key a checkout's repository is known by on this host, where `owner/name` is not enough.
357 * Absent means `owner/name` on the host identifies it, and any checkout on the host can serve a
358 * change request in another repository there.
359 *
360 * A host that resolves it (Azure DevOps: `dev.azure.com/org/project/_git/repo`) derives its
361 * organization from the checkout, so only a checkout whose key matches the reference serves
362 * it. The key also names the repository in list cursors and on routing.
363 */
364 readonly repositoryKey?: (input: {
365 /** The checkout's canonical key, as its remote names it. */
366 readonly canonicalKey: string;
367 }) => string;
355368
356 /** The signed-in account, which is what involvement filtering compares against. */369 /** The signed-in account, which is what involvement filtering compares against. */
357 readonly getViewer: (input: {370 readonly getViewer: (input: {
packages/source-control-core/src/server/SourceControlProvider.ts+13
...@@ -2,6 +2,7 @@ import * as Context from "effect/Context";...@@ -2,6 +2,7 @@ import * as Context from "effect/Context";
2import * as Effect from "effect/Effect";2import * as Effect from "effect/Effect";
3import type * as Option from "effect/Option";3import type * as Option from "effect/Option";
4import type {4import type {
5 RepositoryIdentity,
5 ChangeRequest,6 ChangeRequest,
6 ChangeRequestState,7 ChangeRequestState,
7 SourceControlProviderError,8 SourceControlProviderError,
...@@ -135,6 +136,18 @@ export class SourceControlProvider extends Context.Service<...@@ -135,6 +136,18 @@ export class SourceControlProvider extends Context.Service<
135 * more than that (Forgejo's HTTP installation mount). Absent means the whole path.136 * more than that (Forgejo's HTTP installation mount). Absent means the whole path.
136 */137 */
137 readonly repositoryNameFromRemoteUrl?: (url: string) => string | null;138 readonly repositoryNameFromRemoteUrl?: (url: string) => string | null;
139 /**
140 * Fills in what a repository identity read from git cannot know, such as the browser URL of
141 * a host the remote URL does not name. `resolveContext` asks the registry which host (and
142 * base URL) serves a remote. Only consulted for identities this host may own.
143 */
144 readonly refineRepositoryIdentity?: (input: {
145 readonly identity: RepositoryIdentity;
146 readonly resolveContext: (input: {
147 readonly cwd: string;
148 readonly context: SourceControlProviderContext;
149 }) => Effect.Effect<SourceControlProviderContext | null, SourceControlProviderError>;
150 }) => Effect.Effect<RepositoryIdentity, SourceControlProviderError>;
138 /** Optional capability for issue and change-request subjects. */151 /** Optional capability for issue and change-request subjects. */
139 readonly resolveLink?: ResolveSourceControlLink;152 readonly resolveLink?: ResolveSourceControlLink;
140 readonly listChangeRequests: (input: {153 readonly listChangeRequests: (input: {
packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.test.ts+60
...@@ -245,6 +245,66 @@ it.effect.each([...@@ -245,6 +245,66 @@ it.effect.each([
245 },245 },
246);246);
247247
248it.effect("gives Forgejo identities the browser URL of the login that serves them", () =>
249 Effect.gen(function* () {
250 const provider = yield* ForgejoSourceControlProvider.make;
251 const refine = provider.refineRepositoryIdentity;
252 assert.isDefined(refine);
253 const identity = (remoteUrl: string, provider?: string) => ({
254 canonicalKey: "forge.test/team/repo",
255 locator: { source: "git-remote" as const, remoteName: "origin", remoteUrl },
256 rootPath: "/repo",
257 ...(provider === undefined ? {} : { provider }),
258 });
259 const served = (baseUrl: string) => () =>
260 Effect.succeed({
261 provider: { kind: "forgejo" as const, name: "Forgejo / Gitea", baseUrl },
262 remoteName: "origin",
263 remoteUrl: "",
264 });
265
266 // An installation mount in the HTTP path is the login's, not the repository's.
267 assert.deepStrictEqual(
268 yield* refine!({
269 identity: identity("http://forge.test:3000/git/team/repo.git"),
270 resolveContext: served("http://forge.test:3000/git/"),
271 }),
272 {
273 ...identity("http://forge.test:3000/git/team/repo.git"),
274 provider: "forgejo",
275 webUrl: "http://forge.test:3000/git/team/repo",
276 },
277 );
278 // An SSH alias keeps its whole path.
279 assert.strictEqual(
280 (yield* refine!({
281 identity: identity("ssh://git@ssh.forge.test/team/repo.git"),
282 resolveContext: served("http://forge.test:3000/git"),
283 })).webUrl,
284 "http://forge.test:3000/git/team/repo",
285 );
286 // Another host's identity, or a remote no Forgejo login serves, is left alone.
287 const github = identity("https://github.com/team/repo.git", "github");
288 assert.strictEqual(
289 yield* refine!({ identity: github, resolveContext: () => Effect.die("not asked") }),
290 github,
291 );
292 const unserved = identity("http://forge.test:3000/team/repo.git");
293 assert.strictEqual(
294 yield* refine!({ identity: unserved, resolveContext: () => Effect.succeed(null) }),
295 unserved,
296 );
297 }).pipe(
298 Effect.provide(
299 Layer.mergeAll(
300 Layer.succeed(FileSystem.FileSystem, FileSystem.makeNoop({})),
301 TestSourceControlHost.layer(),
302 Layer.mock(ForgejoCli.ForgejoCli)({}),
303 ),
304 ),
305 ),
306);
307
248it.effect("loads Forgejo pull request references from files and commits views", () =>308it.effect("loads Forgejo pull request references from files and commits views", () =>
249 Effect.gen(function* () {309 Effect.gen(function* () {
250 const provider = yield* ForgejoSourceControlProvider.make;310 const provider = yield* ForgejoSourceControlProvider.make;
packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.ts+38
...@@ -166,6 +166,43 @@ export function repositoryNameFromRemoteUrl(url: string): string | null {...@@ -166,6 +166,43 @@ export function repositoryNameFromRemoteUrl(url: string): string | null {
166 : path.split("/").slice(-2).join("/");166 : path.split("/").slice(-2).join("/");
167}167}
168168
169/**
170 * A Forgejo remote's URL can't say which server it belongs to (an SSH alias, an installation
171 * mount), so the identity's browser URL comes from the login that serves it.
172 */
173const refineRepositoryIdentity: NonNullable<
174 SourceControlProvider.SourceControlProvider["Service"]["refineRepositoryIdentity"]
175> = Effect.fn("ForgejoSourceControlProvider.refineRepositoryIdentity")(function* ({
176 identity,
177 resolveContext,
178}) {
179 const remote = ForgejoCli.parseForgejoRemote(identity.locator.remoteUrl);
180 if (
181 !remote ||
182 !identity.rootPath ||
183 (identity.provider !== undefined &&
184 identity.provider !== "unknown" &&
185 identity.provider !== "forgejo")
186 )
187 return identity;
188 const context = yield* resolveContext({
189 cwd: identity.rootPath,
190 context: {
191 provider: { kind: "unknown", name: "Unknown", baseUrl: "" },
192 remoteName: identity.locator.remoteName,
193 remoteUrl: identity.locator.remoteUrl,
194 },
195 });
196 if (context?.provider.kind !== "forgejo") return identity;
197 const baseUrl = context.provider.baseUrl.replace(/\/+$/, "");
198 const basePath = new URL(baseUrl).pathname.replace(/^\/+|\/+$/g, "");
199 const path =
200 !remote.ssh && basePath && remote.path.startsWith(`${basePath}/`)
201 ? remote.path.slice(basePath.length + 1)
202 : remote.path;
203 return { ...identity, provider: "forgejo", webUrl: `${baseUrl}/${path}` };
204});
205
169const RepositorySchema = Schema.Struct({206const RepositorySchema = Schema.Struct({
170 full_name: Schema.String,207 full_name: Schema.String,
171 clone_url: Schema.String,208 clone_url: Schema.String,
...@@ -237,6 +274,7 @@ export const make = Effect.gen(function* () {...@@ -237,6 +274,7 @@ export const make = Effect.gen(function* () {
237 return SourceControlProvider.SourceControlProvider.of({274 return SourceControlProvider.SourceControlProvider.of({
238 kind: "forgejo",275 kind: "forgejo",
239 repositoryNameFromRemoteUrl,276 repositoryNameFromRemoteUrl,
277 refineRepositoryIdentity,
240 listChangeRequests: (input) =>278 listChangeRequests: (input) =>
241 Effect.gen(function* () {279 Effect.gen(function* () {
242 const repo = yield* cli.resolveRepository(input);280 const repo = yield* cli.resolveRepository(input);