From f5565e9d39d4d3791b6d20bc7e124d68fbb4332f Mon Sep 17 00:00:00 2001 From: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com> Date: Fri, 9 Oct 2026 15:02:51 -0700 Subject: [PATCH 1/2] refactor(source-control): GitManager reads provider resolvers, not host kinds GitManager branched on `provider.kind === "github"` and `"forgejo"` for four host behaviors. Each one is now a resolver on SourceControlProvider that the host's own package implements: - headBranchProbe: which head selectors to ask about and how many results to read. GitHub drops `owner:branch` selectors and reads 100; a host without one asks about every selector, reading 1 open / 20 any state. - readChangeRequestTemplate: the repository's PR template. GitHub's convention (`.github/pull_request_template.md` and friends) moves from apps/server/src/sourceControl/PrTemplateDetection.ts into the GitHub package as gitHubChangeRequestTemplate, reading through the host's git. - repositoryNameFromRemoteUrl: Forgejo keeps only owner/repo from an HTTP path with an installation mount. Generic code asks the provider a question instead of testing a flag that only one host ever sets. The reasoning for GitHub's values moves into the GitHub package. The pure remote-path parse moves to core as repositoryPathFromRemoteUrl. bindProviderContext spreads the provider, so new resolvers pass through. SourceControlHost.git.execute accepts output limits, which the template reader needs. Behavior is unchanged: every host keeps the values it had. 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 ++++++++ .../server/ForgejoSourceControlProvider.ts | 9 ++ packages/source-control-github/package.json | 4 + .../src/server/GitHubSourceControlProvider.ts | 13 +++ .../src/server/gitHubChangeRequestTemplate.ts | 16 +++- 10 files changed, 150 insertions(+), 67 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 24dd7e1d54ea..45f4ae96a133 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 593910ba9a4a..8ca0b6b976a9 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 153e8c21b113..7af90663dc93 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 53972f019f24..1c7c34d2ab80 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 a46db0fccf27..442dc47d0c68 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 2bcabceb00e3..74b946651faa 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.ts b/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.ts index cbcb3405f4ec..86196975f2a3 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 739d6e8b5f3f..46a8965976fd 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 0f442f65d712..f906f3e629d4 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 6872708fec94..e6915c6bf01e 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, From 3fd41c7ddaab96d0259976799aadd219d1c502c7 Mon Sep 17 00:00:00 2001 From: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com> Date: Fri, 9 Oct 2026 15:13:03 -0700 Subject: [PATCH 2/2] test(source-control): fake Forgejo HTTP with HttpClient.make, decode with Schema The moved Forgejo review test faked HTTP with an async fetch function behind FetchHttpClient and read request bodies with JSON.parse. It now provides HttpClient.make and decodes bodies through Schema, with the same assertions. Co-Authored-By: Claude Opus 5.5 --- .../ForgejoSourceControlProvider.test.ts | 80 +++++++++---------- 1 file changed, 40 insertions(+), 40 deletions(-) diff --git a/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.test.ts b/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.test.ts index 679a4e69028c..bdddd5c94b40 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 })), );