From 49f849ecce3667abab7ce9e6aaa42dfd2e3c45d7 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Fri, 9 Oct 2026 16:22:36 -0700 Subject: [PATCH] refactor(source-control): GitManager reads provider resolvers, not host kinds (#17617) Co-authored-by: Claude Opus 5.5 --- apps/server/src/git/GitManager.test.ts | 35 +++++-- apps/server/src/git/GitManager.ts | 92 ++++++++----------- .../SourceControlProviderRegistry.ts | 3 +- ...ts => gitHubChangeRequestTemplate.test.ts} | 4 +- .../src/server/SourceControlHost.ts | 2 + .../src/server/SourceControlProvider.ts | 39 ++++++++ .../ForgejoSourceControlProvider.test.ts | 80 ++++++++-------- .../server/ForgejoSourceControlProvider.ts | 9 ++ packages/source-control-github/package.json | 4 + .../src/server/GitHubSourceControlProvider.ts | 13 +++ .../src/server/gitHubChangeRequestTemplate.ts | 16 +++- 11 files changed, 190 insertions(+), 107 deletions(-) rename apps/server/src/sourceControl/{PrTemplateDetection.test.ts => gitHubChangeRequestTemplate.test.ts} (97%) rename apps/server/src/sourceControl/PrTemplateDetection.ts => packages/source-control-github/src/server/gitHubChangeRequestTemplate.ts (88%) diff --git a/apps/server/src/git/GitManager.test.ts b/apps/server/src/git/GitManager.test.ts index 24dd7e1d54ea0e550f3d4f3d4fda73784162bf0a..45f4ae96a1330fe6d968d126945232ec96f61be9 100644 --- a/apps/server/src/git/GitManager.test.ts +++ b/apps/server/src/git/GitManager.test.ts @@ -44,6 +44,7 @@ import { } from "@t3tools/contracts"; import * as DateTime from "effect/DateTime"; import { decodeGitHubPullRequestListJson } from "@t3tools/source-control-github/server/gitHubPullRequests"; +import * as GitHubChangeRequestTemplate from "@t3tools/source-control-github/server/gitHubChangeRequestTemplate"; import * as GitLabCli from "@t3tools/source-control-gitlab/server/GitLabCli"; import type * as SourceControlHost from "@t3tools/source-control-core/server/SourceControlHost"; import * as TestSourceControlHost from "@t3tools/source-control-testing/TestSourceControlHost"; @@ -55,6 +56,7 @@ import * as VcsProjectConfig from "../vcs/VcsProjectConfig.ts"; import * as VcsStatusBroadcaster from "../vcs/VcsStatusBroadcaster.ts"; import * as BackgroundPolicy from "../background/BackgroundPolicy.ts"; import * as GitWorkflowService from "./GitWorkflowService.ts"; +import * as ForgejoSourceControlProvider from "@t3tools/source-control-forgejo/server/ForgejoSourceControlProvider"; import * as GitLabSourceControlProvider from "@t3tools/source-control-gitlab/server/GitLabSourceControlProvider"; import { ForgejoPullRequestSchema, @@ -545,6 +547,13 @@ function createGitHubProviderWithFakeGh(scenario: FakeGhScenario = {}): { return { service: { kind: "github", + // The GitHub provider's own lookup rule and template convention, which GitManager reads + // instead of the kind. + headBranchProbe: ({ headSelectors }) => ({ + headSelectors: headSelectors.filter((selector) => !selector.includes(":")), + limit: 100, + }), + listChangeRequests: (input) => input.state === "open" ? execute({ @@ -745,7 +754,18 @@ function makeManager(input?: { ); const layerSourceControlRegistry = Layer.effect( SourceControlProviderRegistry.SourceControlProviderRegistry, - Effect.succeed(input?.sourceControlProvider ?? fakeGitHubProvider).pipe( + Effect.gen(function* () { + // GitHub reads its PR template with git, which the fake gh cannot answer; give the fake + // the package's real reader over the test repository's git. + const git = yield* GitVcsDriver.GitVcsDriver; + return ( + input?.sourceControlProvider ?? { + ...fakeGitHubProvider, + readChangeRequestTemplate: ({ cwd, treeish }: { cwd: string; treeish: string }) => + GitHubChangeRequestTemplate.detect(cwd, treeish, git.execute), + } + ); + }).pipe( Effect.map((provider) => SourceControlProviderRegistry.SourceControlProviderRegistry.of({ resolveLink: (input) => provider.resolveLink?.(input), @@ -769,9 +789,12 @@ function makeManager(input?: { runForThread: () => Effect.succeed({ status: "no-script" as const }), }, ), - layerVcsDriver, layerServerSettings, - ).pipe(Layer.provideMerge(layerSourceControlRegistry), Layer.provideMerge(NodeServices.layer)); + ).pipe( + Layer.provideMerge(layerSourceControlRegistry), + Layer.provideMerge(layerVcsDriver), + Layer.provideMerge(NodeServices.layer), + ); // Built into the test's scope: the manager reads these stores after this returns. const layerStores = Layer.merge(ProjectionStore.layer, ProjectStore.layer).pipe( Layer.provideMerge(SqlitePersistence.layerMemory), @@ -4232,7 +4255,7 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { }; const repository = GitManager.parseRepositoryNameWithOwnerFromRemoteUrl( `https://forgejo.example/forgejo/${owner}/project.git`, - "forgejo", + { repositoryNameFromRemoteUrl: ForgejoSourceControlProvider.repositoryNameFromRemoteUrl }, ); expect(repository).toBe(`${owner}/project`); const context = { @@ -4253,13 +4276,13 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { expect( GitManager.parseRepositoryNameWithOwnerFromRemoteUrl( "git@forgejo.example:maria/project.git", - "forgejo", + { repositoryNameFromRemoteUrl: ForgejoSourceControlProvider.repositoryNameFromRemoteUrl }, ), ).toBe("maria/project"); expect( GitManager.parseRepositoryNameWithOwnerFromRemoteUrl( "https://gitlab.example/group/maria/project.git", - "gitlab", + {}, ), ).toBe("group/maria/project"); }), diff --git a/apps/server/src/git/GitManager.ts b/apps/server/src/git/GitManager.ts index 593910ba9a4abd6dd1c11fc7b9762bb03129b1c2..8ca0b6b976a94138c55e50bd758f09cec39afb62 100644 --- a/apps/server/src/git/GitManager.ts +++ b/apps/server/src/git/GitManager.ts @@ -35,7 +35,6 @@ import { ModelSelection, type ProjectId, SourceControlProviderError, - type SourceControlProviderKind, type SourceControlWritingStyleSettings, type ThreadId, type ThreadPullRequestKey, @@ -81,8 +80,8 @@ import { detachStackFrame } from "./detachStackFrame.ts"; import * as ServerSettings from "../serverSettings.ts"; import type { GitManagerServiceError } from "@t3tools/contracts"; import * as GitVcsDriver from "../vcs/GitVcsDriver.ts"; +import * as SourceControlProvider from "@t3tools/source-control-core/server/SourceControlProvider"; import * as SourceControlProviderRegistry from "../sourceControl/SourceControlProviderRegistry.ts"; -import { detectPrTemplate } from "../sourceControl/PrTemplateDetection.ts"; import type { ChangeRequest } from "@t3tools/contracts"; export interface GitActionProgressReporter { @@ -314,27 +313,20 @@ function resolvePullRequestWorktreeLocalBranchName( return `t3code/pr-${pullRequest.number}/${suffix}`; } +/** The repository's `owner/name` from a remote URL, as `provider` reads its remote paths. */ export function parseRepositoryNameWithOwnerFromRemoteUrl( url: string | null, - providerKind?: ChangeRequest["provider"], + provider?: Pick< + SourceControlProvider.SourceControlProvider["Service"], + "repositoryNameFromRemoteUrl" + >, ): string | null { const trimmed = url?.trim() ?? ""; - if (trimmed.length === 0) { - return null; - } - - const match = - /^(?:[^@/\s]+@[^:/\s]+:|(?:ssh|https?|git):\/\/[^/]+\/)((?:[^/\s]+\/)+[^/\s]+?)(?:\.git)?\/?$/iu.exec( - trimmed, - ); - const repositoryNameWithOwner = match?.[1]?.trim() ?? ""; - // Forgejo HTTP paths can include an installation mount; its API always names owner/repo. - if (providerKind === "forgejo" && /^https?:\/\//iu.test(trimmed)) { - return repositoryNameWithOwner.length > 0 - ? repositoryNameWithOwner.split("/").slice(-2).join("/") - : null; - } - return repositoryNameWithOwner.length > 0 ? repositoryNameWithOwner : null; + if (trimmed.length === 0) return null; + return ( + provider?.repositoryNameFromRemoteUrl?.(trimmed) ?? + SourceControlProvider.repositoryPathFromRemoteUrl(trimmed) + ); } function parseRepositoryOwnerLogin(nameWithOwner: string | null): string | null { @@ -625,24 +617,18 @@ function parseCustomCommitMessage(raw: string): { subject: string; body: string }; } -// Without the owner selector, a bare branch name also lists same-named -// branches on other forks (`main`, `patch-1`), so GitHub probes ask for a full -// page and let matchesBranchHeadContext pick the right head. gh fetches up to -// 100 in one request, and GitHub prices a first:100 connection like first:1. -const GITHUB_HEAD_BRANCH_PROBE_LIMIT = 100; - -// `gh pr list --head` filters on the head ref name alone and accepts anything, so an -// `owner:branch` or `remote:branch` selector silently lists zero pull requests -// while spending a GraphQL call. Git branch names cannot contain ":", and the -// bare head branch is always among the selectors, so GitHub probes skip them and -// leave the owner check to matchesBranchHeadContext. -function probeableHeadSelectors( - providerKind: SourceControlProviderKind, +/** The selectors and page size `provider` asks about when looking up a branch's change requests. */ +function headBranchProbe( + provider: SourceControlProvider.SourceControlProvider["Service"], headSelectors: ReadonlyArray, -): ReadonlyArray { - return providerKind === "github" - ? headSelectors.filter((selector) => !selector.includes(":")) - : headSelectors; + state: "open" | "all", +) { + return ( + provider.headBranchProbe?.({ headSelectors, state }) ?? { + headSelectors, + limit: state === "open" ? 1 : 20, + } + ); } function appendUnique(values: string[], next: string | null | undefined): void { @@ -1430,15 +1416,14 @@ export const make = Effect.gen(function* () { /^https?:\/\//iu.test(remoteUrl) && (repositoryNameWithOwner?.split("/").length ?? 0) > 2 ) { - const detected = detectSourceControlProviderFromGitRemoteUrl(remoteUrl); - const kind = - detected?.kind === "unknown" - ? yield* sourceControlProvider(cwd).pipe( - Effect.map((provider) => provider.kind), - Effect.orElseSucceed(() => undefined), - ) - : detected?.kind; - repositoryNameWithOwner = parseRepositoryNameWithOwnerFromRemoteUrl(remoteUrl, kind); + const kind = detectSourceControlProviderFromGitRemoteUrl(remoteUrl)?.kind; + const provider = + kind === undefined + ? undefined + : yield* ( + kind === "unknown" ? sourceControlProvider(cwd) : sourceControlProviders.get(kind) + ).pipe(Effect.orElseSucceed(() => undefined)); + repositoryNameWithOwner = parseRepositoryNameWithOwnerFromRemoteUrl(remoteUrl, provider); } return { remoteUrlKey: remoteUrl ? normalizeGitRemoteUrl(remoteUrl) : null, @@ -1701,13 +1686,13 @@ export const make = Effect.gen(function* () { >, ) { const provider = yield* sourceControlProvider(cwd); - const headSelectors = probeableHeadSelectors(provider.kind, headContext.headSelectors); - for (const headSelector of headSelectors) { + const probe = headBranchProbe(provider, headContext.headSelectors, "open"); + for (const headSelector of probe.headSelectors) { const pullRequests = yield* provider.listChangeRequests({ cwd, headSelector, state: "open", - limit: provider.kind === "github" ? GITHUB_HEAD_BRANCH_PROBE_LIMIT : 1, + limit: probe.limit, }); const normalizedPullRequests = pullRequests.map(toPullRequestInfo); @@ -1733,12 +1718,13 @@ export const make = Effect.gen(function* () { const parsedByNumber = new Map(); const provider = yield* sourceControlProvider(cwd); - for (const headSelector of probeableHeadSelectors(provider.kind, headContext.headSelectors)) { + const probe = headBranchProbe(provider, headContext.headSelectors, "all"); + for (const headSelector of probe.headSelectors) { const pullRequests = yield* provider.listChangeRequests({ cwd, headSelector, state: "all", - limit: provider.kind === "github" ? GITHUB_HEAD_BRANCH_PROBE_LIMIT : 20, + limit: probe.limit, }); for (const pr of pullRequests.map(toPullRequestInfo)) { @@ -2129,8 +2115,10 @@ export const make = Effect.gen(function* () { const rangeContext = yield* gitCore.readRangeContext(cwd, baseRangeRef); const policy = yield* resolveStylePolicy(cwd, settings); const changeRequestTemplate = - settings.style.followChangeRequestTemplates && provider.kind === "github" - ? Option.getOrUndefined(yield* detectPrTemplate(cwd, baseRangeRef, gitCore.execute)) + settings.style.followChangeRequestTemplates && provider.readChangeRequestTemplate + ? Option.getOrUndefined( + yield* provider.readChangeRequestTemplate({ cwd, treeish: baseRangeRef }), + ) : undefined; const generated = yield* textGeneration.generatePrContent({ diff --git a/apps/server/src/sourceControl/SourceControlProviderRegistry.ts b/apps/server/src/sourceControl/SourceControlProviderRegistry.ts index 153e8c21b1133f3984ea1c8b3e335eb701a89c78..7af90663dc930cf1484b1f75468621c6a95dc1c8 100644 --- a/apps/server/src/sourceControl/SourceControlProviderRegistry.ts +++ b/apps/server/src/sourceControl/SourceControlProviderRegistry.ts @@ -158,8 +158,7 @@ function bindProviderContext( } return SourceControlProvider.SourceControlProvider.of({ - kind: provider.kind, - ...(provider.resolveLink ? { resolveLink: provider.resolveLink } : {}), + ...provider, listChangeRequests: (input) => provider.listChangeRequests({ ...input, diff --git a/apps/server/src/sourceControl/PrTemplateDetection.test.ts b/apps/server/src/sourceControl/gitHubChangeRequestTemplate.test.ts similarity index 97% rename from apps/server/src/sourceControl/PrTemplateDetection.test.ts rename to apps/server/src/sourceControl/gitHubChangeRequestTemplate.test.ts index 53972f019f240c1cf901dbf14d78ac6a304f1bef..1c7c34d2ab806bd65baff9754fec45dc63136253 100644 --- a/apps/server/src/sourceControl/PrTemplateDetection.test.ts +++ b/apps/server/src/sourceControl/gitHubChangeRequestTemplate.test.ts @@ -9,7 +9,7 @@ import * as Path from "effect/Path"; import * as ServerConfig from "../config.ts"; import * as GitVcsDriver from "../vcs/GitVcsDriver.ts"; import * as VcsProcess from "../vcs/VcsProcess.ts"; -import { detectPrTemplate } from "./PrTemplateDetection.ts"; +import * as GitHubChangeRequestTemplate from "@t3tools/source-control-github/server/gitHubChangeRequestTemplate"; import { symlinksSupported } from "@t3tools/shared/testing/symlinks"; const SINGLE_TEMPLATE_PATHS = [ @@ -80,7 +80,7 @@ const commitTemplates = (cwd: string) => const detectTemplate = (cwd: string, treeish = "HEAD") => Effect.gen(function* () { const git = yield* GitVcsDriver.GitVcsDriver; - return yield* detectPrTemplate(cwd, treeish, git.execute); + return yield* GitHubChangeRequestTemplate.detect(cwd, treeish, git.execute); }); it.effect.each(SINGLE_TEMPLATE_PATHS)("recognizes $0", (relativePath) => diff --git a/packages/source-control-core/src/server/SourceControlHost.ts b/packages/source-control-core/src/server/SourceControlHost.ts index a46db0fccf277030e2b226810963554d00406307..442dc47d0c68286a9995578fcf7d2c77d2579dbe 100644 --- a/packages/source-control-core/src/server/SourceControlHost.ts +++ b/packages/source-control-core/src/server/SourceControlHost.ts @@ -83,6 +83,8 @@ export class SourceControlHost extends Context.Service< readonly operation: string; readonly cwd: string; readonly args: ReadonlyArray; + readonly maxOutputBytes?: number; + readonly appendTruncationMarker?: boolean; }) => Effect.Effect; readonly resolveCommit: (input: { readonly cwd: string; diff --git a/packages/source-control-core/src/server/SourceControlProvider.ts b/packages/source-control-core/src/server/SourceControlProvider.ts index 2bcabceb00e349262672352bc65f76d835335fc9..74b946651faa9d3301af3d71056ef214c1c140f7 100644 --- a/packages/source-control-core/src/server/SourceControlProvider.ts +++ b/packages/source-control-core/src/server/SourceControlProvider.ts @@ -1,5 +1,6 @@ import * as Context from "effect/Context"; import * as Effect from "effect/Effect"; +import type * as Option from "effect/Option"; import type { ChangeRequest, ChangeRequestState, @@ -92,10 +93,48 @@ export function sourceControlRefFromInput(input: { return input.source ?? parseSourceControlOwnerRef(input.headSelector); } +/** The repository path a remote URL names (`owner/name`, or deeper for nested groups). */ +export function repositoryPathFromRemoteUrl(url: string | null): string | null { + const trimmed = url?.trim() ?? ""; + if (trimmed.length === 0) { + return null; + } + + const match = + /^(?:[^@/\s]+@[^:/\s]+:|(?:ssh|https?|git):\/\/[^/]+\/)((?:[^/\s]+\/)+[^/\s]+?)(?:\.git)?\/?$/iu.exec( + trimmed, + ); + const path = match?.[1]?.trim() ?? ""; + return path.length > 0 ? path : null; +} + export class SourceControlProvider extends Context.Service< SourceControlProvider, { readonly kind: SourceControlProviderKind; + /** + * How to look up a branch's change requests: which head selectors to ask about, and how many + * results to read per selector. The caller checks the owner against what comes back, so a + * host that cannot search `owner:branch` can drop those selectors without losing anything. + * Absent asks about every selector, reading 1 for the open lookup and 20 for any state. + */ + readonly headBranchProbe?: (input: { + readonly headSelectors: ReadonlyArray; + readonly state: "open" | "all"; + }) => { readonly headSelectors: ReadonlyArray; readonly limit: number }; + /** + * The repository's change request template at `treeish`, for change request content + * generation to follow. Absent means the host has no template convention. + */ + readonly readChangeRequestTemplate?: (input: { + readonly cwd: string; + readonly treeish: string; + }) => Effect.Effect>; + /** + * The repository's `owner/name` from a remote URL, for a host whose remote paths can carry + * more than that (Forgejo's HTTP installation mount). Absent means the whole path. + */ + readonly repositoryNameFromRemoteUrl?: (url: string) => string | null; /** 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 679a4e69028c296ad0a9cbada6ac242a164d307b..bdddd5c94b406dece81ec7dd6dd89e795475b91c 100644 --- a/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.test.ts +++ b/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.test.ts @@ -1,13 +1,12 @@ import { assert, it } from "@effect/vitest"; import * as Effect from "effect/Effect"; -import type * as Context from "effect/Context"; import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; import * as Option from "effect/Option"; import * as Schema from "effect/Schema"; import * as ChildProcessSpawner from "effect/process/ChildProcessSpawner"; -import * as FetchHttpClient from "effect/http/FetchHttpClient"; import * as HttpClient from "effect/http/HttpClient"; +import * as HttpClientRequest from "effect/http/HttpClientRequest"; import * as HttpClientResponse from "effect/http/HttpClientResponse"; import { VcsProcessSpawnError } from "@t3tools/contracts"; import { firstNonEmptyLine } from "@t3tools/source-control-core/server/discovery"; @@ -20,38 +19,45 @@ import * as ForgejoSourceControlProvider from "./ForgejoSourceControlProvider.ts const processOutput = TestSourceControlHost.processOutput; const encodeJson = Schema.encodeSync(Schema.fromJsonString(Schema.Unknown)); +const decodeJson = Schema.decodeUnknownSync(Schema.fromJsonString(Schema.Unknown)); + +/** The JSON body a request sends. Forgejo encodes bodies as UTF-8 text. */ +const requestJson = (request: HttpClientRequest.HttpClientRequest): unknown => { + assert.strictEqual(request.body._tag, "Uint8Array"); + return request.body._tag === "Uint8Array" + ? decodeJson(new TextDecoder().decode(request.body.body)) + : undefined; +}; const encodeJsonEffect = Schema.encodeEffect(Schema.fromJsonString(Schema.Unknown)); it.effect("submits a Forgejo review without sending its summary in the preliminary GET", () => { const methods: string[] = []; - const fetchReview = async ( - ...[input, init]: Parameters> - ) => { - const request = new Request(input instanceof Request ? input.url : String(input), { - ...(init?.method === undefined ? {} : { method: init.method }), - ...(init?.headers === undefined ? {} : { headers: init.headers }), - ...(init?.body === undefined ? {} : { body: init.body }), - }); + const reviewClient = HttpClient.make((request) => { methods.push(request.method); if (request.method === "GET") { assert.strictEqual(request.url, "https://forgejo.test/api/v1/repos/maria/project/pulls/42"); - return new Response( - encodeJson({ - number: 42, - title: "Review target", - body: "", - html_url: "https://forgejo.test/maria/project/pulls/42", - user: { login: "maria" }, - state: "open", - merged: false, - head: { ref: "feature", sha: "head", repo: null }, - base: { ref: "main", sha: "base", repo: null }, - created_at: "2026-09-13T00:00:00Z", - updated_at: "2026-09-13T00:00:00Z", - closed_at: null, - merged_at: null, - labels: [], - }), + return Effect.succeed( + HttpClientResponse.fromWeb( + request, + new Response( + encodeJson({ + number: 42, + title: "Review target", + body: "", + html_url: "https://forgejo.test/maria/project/pulls/42", + user: { login: "maria" }, + state: "open", + merged: false, + head: { ref: "feature", sha: "head", repo: null }, + base: { ref: "main", sha: "base", repo: null }, + created_at: "2026-09-13T00:00:00Z", + updated_at: "2026-09-13T00:00:00Z", + closed_at: null, + merged_at: null, + labels: [], + }), + ), + ), ); } assert.strictEqual(request.method, "POST"); @@ -59,14 +65,16 @@ it.effect("submits a Forgejo review without sending its summary in the prelimina request.url, "https://forgejo.test/api/v1/repos/maria/project/pulls/42/reviews", ); - assert.deepStrictEqual(JSON.parse(await request.text()), { + assert.deepStrictEqual(requestJson(request), { event: "COMMENT", body: "Review summary", commit_id: "head", comments: [], }); - return new Response('{"id":1}', { status: 200 }); - }; + return Effect.succeed( + HttpClientResponse.fromWeb(request, new Response('{"id":1}', { status: 200 })), + ); + }); return Effect.gen(function* () { const cli = yield* ForgejoCli.make; const provider = yield* ForgejoPullRequestProvider.make.pipe( @@ -83,11 +91,7 @@ it.effect("submits a Forgejo review without sending its summary in the prelimina }); assert.deepStrictEqual(methods, ["GET", "POST"]); }).pipe( - Effect.provideService( - FetchHttpClient.Fetch, - Object.assign(fetchReview, { preconnect: () => undefined }), - ), - Effect.provide(FetchHttpClient.layer), + Effect.provideService(HttpClient.HttpClient, reviewClient), Effect.provideService( FileSystem.FileSystem, FileSystem.makeNoop({ @@ -837,11 +841,7 @@ it.effect("prefers fj for HTTP and ported SSH aliases on root servers", () => { } assert.strictEqual(request.method, "POST"); assert.strictEqual(request.headers.authorization, "token test-token"); - assert.strictEqual(request.body._tag, "Uint8Array"); - if (request.body._tag === "Uint8Array") - assert.deepStrictEqual(JSON.parse(new TextDecoder().decode(request.body.body)), { - body: "verified through fj", - }); + assert.deepStrictEqual(requestJson(request), { body: "verified through fj" }); return Effect.succeed( HttpClientResponse.fromWeb(request, new Response('{"id":99}', { status: 201 })), ); diff --git a/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.ts b/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.ts index cbcb3405f4ec4f0f99c781f32bf68f8c9d539f0d..86196975f2a38e1ecf481037b4eebf8cc7c40ce7 100644 --- a/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.ts +++ b/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.ts @@ -158,6 +158,14 @@ export const makeDiscovery = Effect.gen(function* () { } satisfies SourceControlManagedCliDiscoverySpec; }); +/** HTTP paths can include an installation mount; Forgejo's API always names owner/repo. */ +export function repositoryNameFromRemoteUrl(url: string): string | null { + const path = SourceControlProvider.repositoryPathFromRemoteUrl(url); + return path === null || !/^https?:\/\//iu.test(url.trim()) + ? path + : path.split("/").slice(-2).join("/"); +} + const RepositorySchema = Schema.Struct({ full_name: Schema.String, clone_url: Schema.String, @@ -228,6 +236,7 @@ export const make = Effect.gen(function* () { }); return SourceControlProvider.SourceControlProvider.of({ kind: "forgejo", + repositoryNameFromRemoteUrl, listChangeRequests: (input) => Effect.gen(function* () { const repo = yield* cli.resolveRepository(input); diff --git a/packages/source-control-github/package.json b/packages/source-control-github/package.json index 739d6e8b5f3f4d1d0d33bd66b72b7abcfaa42302..46a8965976fd32098b969578cb4c37ace3a48f7d 100644 --- a/packages/source-control-github/package.json +++ b/packages/source-control-github/package.json @@ -15,6 +15,10 @@ "types": "./src/server/gitHubAuthStatus.ts", "import": "./src/server/gitHubAuthStatus.ts" }, + "./server/gitHubChangeRequestTemplate": { + "types": "./src/server/gitHubChangeRequestTemplate.ts", + "import": "./src/server/gitHubChangeRequestTemplate.ts" + }, "./server/gitHubConditionalChecks": { "types": "./src/server/gitHubConditionalChecks.ts", "import": "./src/server/gitHubConditionalChecks.ts" diff --git a/packages/source-control-github/src/server/GitHubSourceControlProvider.ts b/packages/source-control-github/src/server/GitHubSourceControlProvider.ts index 0f442f65d712813c2bc97aadf3a152d79ac4d207..f906f3e629d46dfbd61e2c1617a8a67cffa604a8 100644 --- a/packages/source-control-github/src/server/GitHubSourceControlProvider.ts +++ b/packages/source-control-github/src/server/GitHubSourceControlProvider.ts @@ -23,6 +23,7 @@ import { decodeJsonResult } from "@t3tools/shared/schemaJson"; import { isSshRemoteUrl } from "@t3tools/shared/sourceControl"; import * as GitHubApi from "./GitHubApi.ts"; +import * as GitHubChangeRequestTemplate from "./gitHubChangeRequestTemplate.ts"; import { decodeGitHubPullRequestEntries, type NormalizedGitHubPullRequestRecord, @@ -939,6 +940,18 @@ export const make = Effect.gen(function* () { return SourceControlProvider.SourceControlProvider.of({ kind: "github", + // `gh pr list --head` filters on the head ref name alone and accepts anything, so an + // `owner:branch` or `remote:branch` selector silently lists zero pull requests while + // spending a GraphQL call; the bare branch is always among the selectors. Without the owner, + // a bare branch also lists same-named branches on other forks (`main`, `patch-1`), so read a + // full page and let the owner check pick the right head. gh fetches up to 100 in one + // request, and GitHub prices a first:100 connection like first:1. + headBranchProbe: ({ headSelectors }) => ({ + headSelectors: headSelectors.filter((selector) => !selector.includes(":")), + limit: 100, + }), + readChangeRequestTemplate: ({ cwd, treeish }) => + GitHubChangeRequestTemplate.detect(cwd, treeish, git.execute), resolveLink: (input) => { // Automatic enrichment must not send ambient CLI credentials to a host from message text. if (input.url.host !== "github.com") return undefined; diff --git a/apps/server/src/sourceControl/PrTemplateDetection.ts b/packages/source-control-github/src/server/gitHubChangeRequestTemplate.ts similarity index 88% rename from apps/server/src/sourceControl/PrTemplateDetection.ts rename to packages/source-control-github/src/server/gitHubChangeRequestTemplate.ts index 6872708fec941e8dc864af3d68648bf03f8deeb0..e6915c6bf01eef93cbd47daa99403c5fd0a5b385 100644 --- a/apps/server/src/sourceControl/PrTemplateDetection.ts +++ b/packages/source-control-github/src/server/gitHubChangeRequestTemplate.ts @@ -1,8 +1,14 @@ +/** + * GitHub's pull request template convention: the first single template found at a known path, + * or the only Markdown file in a template directory, read from the committed base tree. + * + * @module source-control-github/server/gitHubChangeRequestTemplate + */ import type { GitCommandError } from "@t3tools/contracts"; import * as Effect from "effect/Effect"; import * as Option from "effect/Option"; -import type * as GitVcsDriver from "../vcs/GitVcsDriver.ts"; +import type * as SourceControlHost from "@t3tools/source-control-core/server/SourceControlHost"; const TEMPLATE_MAX_BYTES = 8_000; const TREE_LIST_MAX_BYTES = 100_000; @@ -25,7 +31,7 @@ const TEMPLATE_DIRECTORIES = [ const TREE_PATHS = [...TEMPLATE_PATHS, ...TEMPLATE_DIRECTORIES] as const; -type ExecuteGit = GitVcsDriver.GitVcsDriver["Service"]["execute"]; +type ExecuteGit = SourceControlHost.SourceControlHost["Service"]["git"]["execute"]; interface TemplateTreeEntry { readonly objectId: string; @@ -66,7 +72,7 @@ function readTemplateBlob(input: { }): Effect.Effect, GitCommandError> { return input .executeGit({ - operation: "PrTemplateDetection.readTemplateBlob", + operation: "gitHubChangeRequestTemplate.readTemplateBlob", cwd: input.cwd, args: ["cat-file", "blob", input.entry.objectId], maxOutputBytes: TEMPLATE_MAX_BYTES, @@ -125,7 +131,7 @@ function readTemplateDirectory(input: { }); } -export const detectPrTemplate = Effect.fn("detectPrTemplate")(function* ( +export const detect = Effect.fn("gitHubChangeRequestTemplate.detect")(function* ( cwd: string, treeish: string, executeGit: ExecuteGit, @@ -134,7 +140,7 @@ export const detectPrTemplate = Effect.fn("detectPrTemplate")(function* ( // Worktree paths can be replaced between validation and open. Read regular blobs from the // committed base tree so repository-controlled symlinks and path races never reach the host filesystem. const result = yield* executeGit({ - operation: "PrTemplateDetection.listTemplates", + operation: "gitHubChangeRequestTemplate.listTemplates", cwd, args: ["ls-tree", "-r", "-z", "--full-tree", treeish, "--", ...TREE_PATHS], maxOutputBytes: TREE_LIST_MAX_BYTES, -- 2.54.0