From 5d42a7a79c7d36c966b92c9af72108480d250f34 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sat, 20 Jun 2026 02:27:23 -0700 Subject: [PATCH] enrich source-control errors Co-authored-by: codex --- apps/server/src/git/GitManager.test.ts | 16 ++ .../src/sourceControl/AzureDevOpsCli.test.ts | 23 +++ .../src/sourceControl/AzureDevOpsCli.ts | 128 ++++++++------ .../AzureDevOpsSourceControlProvider.test.ts | 40 +++++ .../AzureDevOpsSourceControlProvider.ts | 130 +++++++++++--- .../BitbucketSourceControlProvider.test.ts | 39 +++++ .../BitbucketSourceControlProvider.ts | 121 +++++++++++--- .../src/sourceControl/GitHubCli.test.ts | 25 +-- apps/server/src/sourceControl/GitHubCli.ts | 126 ++++++++------ .../GitHubSourceControlProvider.test.ts | 43 +++++ .../GitHubSourceControlProvider.ts | 158 ++++++++++++++---- .../src/sourceControl/GitLabCli.test.ts | 23 +-- apps/server/src/sourceControl/GitLabCli.ts | 147 +++++++++------- .../GitLabSourceControlProvider.test.ts | 44 +++++ .../GitLabSourceControlProvider.ts | 132 ++++++++++++--- .../SourceControlProvider.test.ts | 19 +++ .../sourceControl/SourceControlProvider.ts | 30 ++++ .../SourceControlProviderRegistry.test.ts | 70 ++++++-- .../SourceControlProviderRegistry.ts | 105 ++++++++---- packages/contracts/src/sourceControl.ts | 4 + 20 files changed, 1084 insertions(+), 339 deletions(-) create mode 100644 apps/server/src/sourceControl/SourceControlProvider.test.ts diff --git a/apps/server/src/git/GitManager.test.ts b/apps/server/src/git/GitManager.test.ts index 81b82d7de30..c7e03a7428b 100644 --- a/apps/server/src/git/GitManager.test.ts +++ b/apps/server/src/git/GitManager.test.ts @@ -173,6 +173,8 @@ function runGitSyncForFakeGh(cwd: string, args: readonly string[]): void { } throw new GitHubCli.GitHubCliError({ operation: "execute", + command: "gh", + cwd, detail: `Failed to simulate gh checkout with git ${args.join(" ")}: ${result.stderr?.trim() || "unknown error"}`, }); } @@ -480,6 +482,8 @@ function createGitHubCliWithFakeGh(scenario: FakeGhScenario = {}): { ? error : new GitHubCli.GitHubCliError({ operation: "execute", + command: "gh", + cwd: input.cwd, detail: error instanceof Error ? `Failed to simulate gh checkout: ${error.message}` @@ -496,6 +500,8 @@ function createGitHubCliWithFakeGh(scenario: FakeGhScenario = {}): { return Effect.fail( new GitHubCli.GitHubCliError({ operation: "execute", + command: "gh", + cwd: input.cwd, detail: `Unexpected repository lookup: ${repository}`, }), ); @@ -516,6 +522,8 @@ function createGitHubCliWithFakeGh(scenario: FakeGhScenario = {}): { return Effect.fail( new GitHubCli.GitHubCliError({ operation: "execute", + command: "gh", + cwd: input.cwd, detail: `Unexpected gh command: ${args.join(" ")}`, }), ); @@ -595,6 +603,8 @@ function createGitHubCliWithFakeGh(scenario: FakeGhScenario = {}): { Effect.fail( new GitHubCli.GitHubCliError({ operation: "createRepository", + command: "gh", + cwd: input.cwd, detail: `Unexpected repository create: ${input.repository}`, }), ), @@ -1333,6 +1343,8 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { ghScenario: { failWith: new GitHubCli.GitHubCliError({ operation: "execute", + command: "gh", + cwd: repoDir, detail: "GitHub CLI (`gh`) is required but not available on PATH.", }), }, @@ -2471,6 +2483,8 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { ghScenario: { failWith: new GitHubCli.GitHubCliError({ operation: "execute", + command: "gh", + cwd: repoDir, detail: "GitHub CLI (`gh`) is required but not available on PATH.", }), }, @@ -2500,6 +2514,8 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { ghScenario: { failWith: new GitHubCli.GitHubCliError({ operation: "execute", + command: "gh", + cwd: repoDir, detail: "GitHub CLI is not authenticated. Run `gh auth login` and retry.", }), }, diff --git a/apps/server/src/sourceControl/AzureDevOpsCli.test.ts b/apps/server/src/sourceControl/AzureDevOpsCli.test.ts index 52aedd1d760..8617f14e365 100644 --- a/apps/server/src/sourceControl/AzureDevOpsCli.test.ts +++ b/apps/server/src/sourceControl/AzureDevOpsCli.test.ts @@ -5,6 +5,7 @@ import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; import * as Option from "effect/Option"; import { ChildProcessSpawner } from "effect/unstable/process"; +import { VcsProcessExitError } from "@t3tools/contracts"; import * as VcsProcess from "../vcs/VcsProcess.ts"; import * as AzureDevOpsCli from "./AzureDevOpsCli.ts"; @@ -329,4 +330,26 @@ describe("AzureDevOpsCli.layer", () => { }); }).pipe(Effect.provide(layer)), ); + + it.effect("preserves VCS causes without copying upstream details into messages", () => + Effect.gen(function* () { + const cause = new VcsProcessExitError({ + operation: "AzureDevOpsCli.execute", + command: "az repos list --organization sensitive-upstream-detail", + cwd: "/repo", + exitCode: 1, + detail: "sensitive-upstream-detail", + }); + mockRun.mockReturnValueOnce(Effect.fail(cause)); + + const az = yield* AzureDevOpsCli.AzureDevOpsCli; + const error = yield* az.execute({ cwd: "/repo", args: ["repos", "list"] }).pipe(Effect.flip); + + assert.strictEqual(error.command, "az"); + assert.strictEqual(error.cwd, "/repo"); + assert.strictEqual(error.detail, "Azure DevOps CLI command failed."); + assert.strictEqual(error.cause, cause); + assert.equal(error.message.includes("sensitive-upstream-detail"), false); + }).pipe(Effect.provide(layer)), + ); }); diff --git a/apps/server/src/sourceControl/AzureDevOpsCli.ts b/apps/server/src/sourceControl/AzureDevOpsCli.ts index 442cae68934..ea0e5286872 100644 --- a/apps/server/src/sourceControl/AzureDevOpsCli.ts +++ b/apps/server/src/sourceControl/AzureDevOpsCli.ts @@ -3,7 +3,6 @@ import * as Effect from "effect/Effect"; import * as Layer from "effect/Layer"; import * as Result from "effect/Result"; import * as Schema from "effect/Schema"; -import * as SchemaIssue from "effect/SchemaIssue"; import { TrimmedNonEmptyString, type SourceControlRepositoryVisibility, @@ -14,7 +13,6 @@ import * as VcsProcess from "../vcs/VcsProcess.ts"; import { decodeAzureDevOpsPullRequestJson, decodeAzureDevOpsPullRequestListJson, - formatAzureDevOpsJsonDecodeError, type NormalizedAzureDevOpsPullRequestRecord, } from "./azureDevOpsPullRequests.ts"; import * as SourceControlProvider from "./SourceControlProvider.ts"; @@ -25,6 +23,8 @@ export class AzureDevOpsCliError extends Schema.TaggedErrorClass( schema: S, operation: "getRepositoryCloneUrls" | "getDefaultBranch" | "createRepository", invalidDetail: string, + cwd: string, ): Effect.Effect { return Schema.decodeEffect(Schema.fromJsonString(schema))(raw).pipe( Effect.mapError( (error) => new AzureDevOpsCliError({ operation, - detail: `${invalidDetail}: ${SchemaIssue.makeFormatterDefault()(error.issue)}`, + command: "az", + cwd, + detail: invalidDetail, cause: error, }), ), @@ -249,7 +255,14 @@ export const make = Effect.gen(function* () { cwd: input.cwd, timeoutMs: input.timeoutMs ?? DEFAULT_TIMEOUT_MS, }) - .pipe(Effect.mapError((error) => normalizeAzureDevOpsCliError("execute", error))); + .pipe( + Effect.mapError((error) => + AzureDevOpsCliError.fromVcsError( + { operation: "execute", command: "az", cwd: input.cwd }, + error, + ), + ), + ); const executeJson = (input: Parameters[0]) => execute({ @@ -286,7 +299,9 @@ export const make = Effect.gen(function* () { return Effect.fail( new AzureDevOpsCliError({ operation: "listPullRequests", - detail: `Azure DevOps CLI returned invalid PR list JSON: ${formatAzureDevOpsJsonDecodeError(decoded.failure)}`, + command: "az", + cwd: input.cwd, + detail: "Azure DevOps CLI returned invalid PR list JSON.", cause: decoded.failure, }), ); @@ -318,7 +333,9 @@ export const make = Effect.gen(function* () { return Effect.fail( new AzureDevOpsCliError({ operation: "getPullRequest", - detail: `Azure DevOps CLI returned invalid pull request JSON: ${formatAzureDevOpsJsonDecodeError(decoded.failure)}`, + command: "az", + cwd: input.cwd, + detail: "Azure DevOps CLI returned invalid pull request JSON.", cause: decoded.failure, }), ); @@ -341,6 +358,7 @@ export const make = Effect.gen(function* () { RawAzureDevOpsRepositorySchema, "getRepositoryCloneUrls", "Azure DevOps CLI returned invalid repository JSON.", + input.cwd, ), ), Effect.map(normalizeRepositoryCloneUrls), @@ -370,6 +388,7 @@ export const make = Effect.gen(function* () { RawAzureDevOpsRepositorySchema, "createRepository", "Azure DevOps CLI returned invalid repository JSON.", + input.cwd, ), ), Effect.map(normalizeRepositoryCloneUrls), @@ -407,6 +426,7 @@ export const make = Effect.gen(function* () { RawAzureDevOpsRepositorySchema, "getDefaultBranch", "Azure DevOps CLI returned invalid repository JSON.", + input.cwd, ), ), Effect.map((repo) => normalizeDefaultBranch(repo.defaultBranch)), diff --git a/apps/server/src/sourceControl/AzureDevOpsSourceControlProvider.test.ts b/apps/server/src/sourceControl/AzureDevOpsSourceControlProvider.test.ts index f007ecf7985..1341f4cc08d 100644 --- a/apps/server/src/sourceControl/AzureDevOpsSourceControlProvider.test.ts +++ b/apps/server/src/sourceControl/AzureDevOpsSourceControlProvider.test.ts @@ -46,6 +46,46 @@ it.effect("maps Azure DevOps PR summaries into provider-neutral change requests" }), ); +it.effect("adds change-request context while retaining Azure CLI causes", () => + Effect.gen(function* () { + const cause = new AzureDevOpsCli.AzureDevOpsCliError({ + operation: "execute", + command: "az", + cwd: "/repo", + detail: "Azure DevOps CLI command failed.", + cause: new Error("raw upstream detail that should remain in the cause"), + }); + const provider = yield* makeProvider({ + checkoutPullRequest: () => Effect.fail(cause), + }); + + const error = yield* provider + .checkoutChangeRequest({ cwd: "/repo", reference: "#42" }) + .pipe(Effect.flip); + + assert.deepStrictEqual( + { + provider: error.provider, + operation: error.operation, + command: error.command, + cwd: error.cwd, + reference: error.reference, + detail: error.detail, + }, + { + provider: "azure-devops", + operation: "checkoutChangeRequest", + command: "az", + cwd: "/repo", + reference: "#42", + detail: "Azure DevOps CLI command failed.", + }, + ); + assert.strictEqual(error.cause, cause); + assert.equal(error.message.includes("raw upstream detail"), false); + }), +); + it.effect("creates Azure DevOps PRs through provider-neutral input names", () => Effect.gen(function* () { let createInput: diff --git a/apps/server/src/sourceControl/AzureDevOpsSourceControlProvider.ts b/apps/server/src/sourceControl/AzureDevOpsSourceControlProvider.ts index 8cd5bd7522d..bf2ac982927 100644 --- a/apps/server/src/sourceControl/AzureDevOpsSourceControlProvider.ts +++ b/apps/server/src/sourceControl/AzureDevOpsSourceControlProvider.ts @@ -12,18 +12,6 @@ import { type SourceControlCliDiscoverySpec, } from "./SourceControlProviderDiscovery.ts"; -function providerError( - operation: string, - cause: AzureDevOpsCli.AzureDevOpsCliError, -): SourceControlProviderError { - return new SourceControlProviderError({ - provider: "azure-devops", - operation, - detail: cause.detail, - cause, - }); -} - function parseAzureAuth(input: SourceControlAuthProbeInput) { const account = input.stdout.trim().split(/\r?\n/)[0]?.trim(); @@ -101,13 +89,39 @@ export const make = Effect.gen(function* () { }) .pipe( Effect.map((items) => items.map(toChangeRequest)), - Effect.mapError((error) => providerError("listChangeRequests", error)), + Effect.mapError( + (error) => + new SourceControlProviderError({ + provider: "azure-devops", + operation: "listChangeRequests", + command: error.command, + cwd: input.cwd, + reference: SourceControlProvider.transportSafeSourceControlErrorValue( + input.headSelector, + ), + detail: error.detail, + cause: error, + }), + ), ); }, getChangeRequest: (input) => azure.getPullRequest(input).pipe( Effect.map(toChangeRequest), - Effect.mapError((error) => providerError("getChangeRequest", error)), + Effect.mapError( + (error) => + new SourceControlProviderError({ + provider: "azure-devops", + operation: "getChangeRequest", + command: error.command, + cwd: input.cwd, + reference: SourceControlProvider.transportSafeSourceControlErrorValue( + input.reference, + ), + detail: error.detail, + cause: error, + }), + ), ), createChangeRequest: (input) => { const source = SourceControlProvider.sourceControlRefFromInput(input); @@ -121,20 +135,71 @@ export const make = Effect.gen(function* () { title: input.title, bodyFile: input.bodyFile, }) - .pipe(Effect.mapError((error) => providerError("createChangeRequest", error))); + .pipe( + Effect.mapError( + (error) => + new SourceControlProviderError({ + provider: "azure-devops", + operation: "createChangeRequest", + command: error.command, + cwd: input.cwd, + reference: SourceControlProvider.transportSafeSourceControlErrorValue( + input.headSelector, + ), + detail: error.detail, + cause: error, + }), + ), + ); }, getRepositoryCloneUrls: (input) => - azure - .getRepositoryCloneUrls(input) - .pipe(Effect.mapError((error) => providerError("getRepositoryCloneUrls", error))), + azure.getRepositoryCloneUrls(input).pipe( + Effect.mapError( + (error) => + new SourceControlProviderError({ + provider: "azure-devops", + operation: "getRepositoryCloneUrls", + command: error.command, + cwd: input.cwd, + repository: SourceControlProvider.transportSafeSourceControlErrorValue( + input.repository, + ), + detail: error.detail, + cause: error, + }), + ), + ), createRepository: (input) => - azure - .createRepository(input) - .pipe(Effect.mapError((error) => providerError("createRepository", error))), + azure.createRepository(input).pipe( + Effect.mapError( + (error) => + new SourceControlProviderError({ + provider: "azure-devops", + operation: "createRepository", + command: error.command, + cwd: input.cwd, + repository: SourceControlProvider.transportSafeSourceControlErrorValue( + input.repository, + ), + detail: error.detail, + cause: error, + }), + ), + ), getDefaultBranch: (input) => - azure - .getDefaultBranch({ cwd: input.cwd }) - .pipe(Effect.mapError((error) => providerError("getDefaultBranch", error))), + azure.getDefaultBranch({ cwd: input.cwd }).pipe( + Effect.mapError( + (error) => + new SourceControlProviderError({ + provider: "azure-devops", + operation: "getDefaultBranch", + command: error.command, + cwd: input.cwd, + detail: error.detail, + cause: error, + }), + ), + ), checkoutChangeRequest: (input) => azure .checkoutPullRequest({ @@ -142,7 +207,22 @@ export const make = Effect.gen(function* () { reference: input.reference, ...(input.context !== undefined ? { remoteName: input.context.remoteName } : {}), }) - .pipe(Effect.mapError((error) => providerError("checkoutChangeRequest", error))), + .pipe( + Effect.mapError( + (error) => + new SourceControlProviderError({ + provider: "azure-devops", + operation: "checkoutChangeRequest", + command: error.command, + cwd: input.cwd, + reference: SourceControlProvider.transportSafeSourceControlErrorValue( + input.reference, + ), + detail: error.detail, + cause: error, + }), + ), + ), }); }); diff --git a/apps/server/src/sourceControl/BitbucketSourceControlProvider.test.ts b/apps/server/src/sourceControl/BitbucketSourceControlProvider.test.ts index 8530e163dc6..75ac877cd43 100644 --- a/apps/server/src/sourceControl/BitbucketSourceControlProvider.test.ts +++ b/apps/server/src/sourceControl/BitbucketSourceControlProvider.test.ts @@ -51,6 +51,45 @@ it.effect("maps Bitbucket PR summaries into provider-neutral change requests", ( }), ); +it.effect("adds repository context while retaining Bitbucket API causes", () => + Effect.gen(function* () { + const cause = new BitbucketApi.BitbucketApiError({ + operation: "getRepository", + detail: "upstream detail that should remain in the cause", + status: 503, + cause: new Error("raw upstream failure"), + }); + const provider = yield* makeProvider({ + getRepositoryCloneUrls: () => Effect.fail(cause), + }); + + const error = yield* provider + .getRepositoryCloneUrls({ cwd: "/repo", repository: "owner/repo" }) + .pipe(Effect.flip); + + assert.deepStrictEqual( + { + provider: error.provider, + operation: error.operation, + command: error.command, + cwd: error.cwd, + repository: error.repository, + detail: error.detail, + }, + { + provider: "bitbucket", + operation: "getRepositoryCloneUrls", + command: undefined, + cwd: "/repo", + repository: "owner/repo", + detail: "Failed to get repository clone URLs.", + }, + ); + assert.strictEqual(error.cause, cause); + assert.equal(error.message.includes(cause.detail), false); + }), +); + it.effect("lists Bitbucket PRs through provider-neutral input names", () => Effect.gen(function* () { let listInput: Parameters[0] | null = diff --git a/apps/server/src/sourceControl/BitbucketSourceControlProvider.ts b/apps/server/src/sourceControl/BitbucketSourceControlProvider.ts index 6c1d67434bf..974fbb94a39 100644 --- a/apps/server/src/sourceControl/BitbucketSourceControlProvider.ts +++ b/apps/server/src/sourceControl/BitbucketSourceControlProvider.ts @@ -8,18 +8,6 @@ import type { NormalizedBitbucketPullRequestRecord } from "./bitbucketPullReques import * as SourceControlProvider from "./SourceControlProvider.ts"; import type { SourceControlApiDiscoverySpec } from "./SourceControlProviderDiscovery.ts"; -function providerError( - operation: string, - cause: BitbucketApi.BitbucketApiError, -): SourceControlProviderError { - return new SourceControlProviderError({ - provider: "bitbucket", - operation, - detail: cause.detail, - cause, - }); -} - function toChangeRequest(summary: NormalizedBitbucketPullRequestRecord): ChangeRequest { return { provider: "bitbucket", @@ -60,13 +48,37 @@ export const make = Effect.gen(function* () { }) .pipe( Effect.map((items) => items.map(toChangeRequest)), - Effect.mapError((error) => providerError("listChangeRequests", error)), + Effect.mapError( + (error) => + new SourceControlProviderError({ + provider: "bitbucket", + operation: "listChangeRequests", + cwd: input.cwd, + reference: SourceControlProvider.transportSafeSourceControlErrorValue( + input.headSelector, + ), + detail: "Failed to list change requests.", + cause: error, + }), + ), ); }, getChangeRequest: (input) => bitbucket.getPullRequest(input).pipe( Effect.map(toChangeRequest), - Effect.mapError((error) => providerError("getChangeRequest", error)), + Effect.mapError( + (error) => + new SourceControlProviderError({ + provider: "bitbucket", + operation: "getChangeRequest", + cwd: input.cwd, + reference: SourceControlProvider.transportSafeSourceControlErrorValue( + input.reference, + ), + detail: "Failed to get change request.", + cause: error, + }), + ), ), createChangeRequest: (input) => { const source = SourceControlProvider.sourceControlRefFromInput(input); @@ -81,23 +93,72 @@ export const make = Effect.gen(function* () { title: input.title, bodyFile: input.bodyFile, }) - .pipe(Effect.mapError((error) => providerError("createChangeRequest", error))); + .pipe( + Effect.mapError( + (error) => + new SourceControlProviderError({ + provider: "bitbucket", + operation: "createChangeRequest", + cwd: input.cwd, + reference: SourceControlProvider.transportSafeSourceControlErrorValue( + input.headSelector, + ), + detail: "Failed to create change request.", + cause: error, + }), + ), + ); }, getRepositoryCloneUrls: (input) => - bitbucket - .getRepositoryCloneUrls(input) - .pipe(Effect.mapError((error) => providerError("getRepositoryCloneUrls", error))), + bitbucket.getRepositoryCloneUrls(input).pipe( + Effect.mapError( + (error) => + new SourceControlProviderError({ + provider: "bitbucket", + operation: "getRepositoryCloneUrls", + cwd: input.cwd, + repository: SourceControlProvider.transportSafeSourceControlErrorValue( + input.repository, + ), + detail: "Failed to get repository clone URLs.", + cause: error, + }), + ), + ), createRepository: (input) => - bitbucket - .createRepository(input) - .pipe(Effect.mapError((error) => providerError("createRepository", error))), + bitbucket.createRepository(input).pipe( + Effect.mapError( + (error) => + new SourceControlProviderError({ + provider: "bitbucket", + operation: "createRepository", + cwd: input.cwd, + repository: SourceControlProvider.transportSafeSourceControlErrorValue( + input.repository, + ), + detail: "Failed to create repository.", + cause: error, + }), + ), + ), getDefaultBranch: (input) => bitbucket .getDefaultBranch({ cwd: input.cwd, ...(input.context ? { context: input.context } : {}), }) - .pipe(Effect.mapError((error) => providerError("getDefaultBranch", error))), + .pipe( + Effect.mapError( + (error) => + new SourceControlProviderError({ + provider: "bitbucket", + operation: "getDefaultBranch", + cwd: input.cwd, + detail: "Failed to get default branch.", + cause: error, + }), + ), + ), checkoutChangeRequest: (input) => bitbucket .checkoutPullRequest({ @@ -106,7 +167,21 @@ export const make = Effect.gen(function* () { reference: input.reference, ...(input.force !== undefined ? { force: input.force } : {}), }) - .pipe(Effect.mapError((error) => providerError("checkoutChangeRequest", error))), + .pipe( + Effect.mapError( + (error) => + new SourceControlProviderError({ + provider: "bitbucket", + operation: "checkoutChangeRequest", + cwd: input.cwd, + reference: SourceControlProvider.transportSafeSourceControlErrorValue( + input.reference, + ), + detail: "Failed to check out change request.", + cause: error, + }), + ), + ), }); }); diff --git a/apps/server/src/sourceControl/GitHubCli.test.ts b/apps/server/src/sourceControl/GitHubCli.test.ts index e0e781bd8b5..7c8c9b037be 100644 --- a/apps/server/src/sourceControl/GitHubCli.test.ts +++ b/apps/server/src/sourceControl/GitHubCli.test.ts @@ -269,18 +269,15 @@ describe("GitHubCli.layer", () => { it.effect("surfaces a friendly error when the pull request is not found", () => Effect.gen(function* () { - mockRun.mockReturnValueOnce( - Effect.fail( - new VcsProcessExitError({ - operation: "GitHubCli.execute", - command: "gh pr view", - cwd: "/repo", - exitCode: 1, - detail: - "GraphQL: Could not resolve to a PullRequest with the number of 4888. (repository.pullRequest)", - }), - ), - ); + const cause = new VcsProcessExitError({ + operation: "GitHubCli.execute", + command: "gh pr view", + cwd: "/repo", + exitCode: 1, + detail: + "GraphQL: Could not resolve to a PullRequest with the number of 4888. (repository.pullRequest)", + }); + mockRun.mockReturnValueOnce(Effect.fail(cause)); const gh = yield* GitHubCli.GitHubCli; const error = yield* gh @@ -291,6 +288,10 @@ describe("GitHubCli.layer", () => { .pipe(Effect.flip); assert.equal(error.message.includes("Pull request not found"), true); + assert.strictEqual(error.command, "gh"); + assert.strictEqual(error.cwd, "/repo"); + assert.strictEqual(error.cause, cause); + assert.equal(error.message.includes(cause.detail), false); }).pipe(Effect.provide(layer)), ); }); diff --git a/apps/server/src/sourceControl/GitHubCli.ts b/apps/server/src/sourceControl/GitHubCli.ts index 836c7e1eb74..4cdf38ec2b8 100644 --- a/apps/server/src/sourceControl/GitHubCli.ts +++ b/apps/server/src/sourceControl/GitHubCli.ts @@ -3,7 +3,6 @@ import * as Effect from "effect/Effect"; import * as Layer from "effect/Layer"; import * as Result from "effect/Result"; import * as Schema from "effect/Schema"; -import * as SchemaIssue from "effect/SchemaIssue"; import { TrimmedNonEmptyString, @@ -15,19 +14,71 @@ import * as VcsProcess from "../vcs/VcsProcess.ts"; import { decodeGitHubPullRequestJson, decodeGitHubPullRequestListJson, - formatGitHubJsonDecodeError, } from "./gitHubPullRequests.ts"; const DEFAULT_TIMEOUT_MS = 30_000; export class GitHubCliError extends Schema.TaggedErrorClass()("GitHubCliError", { operation: Schema.String, + command: Schema.String, + cwd: Schema.String, detail: Schema.String, cause: Schema.optional(Schema.Defect()), }) { override get message(): string { return `GitHub CLI failed in ${this.operation}: ${this.detail}`; } + + static fromVcsError( + context: { + readonly operation: "execute"; + readonly command: "gh"; + readonly cwd: string; + }, + error: VcsError | unknown, + ): GitHubCliError { + const lower = errorText(error).toLowerCase(); + + if (lower.includes("command not found: gh") || lower.includes("enoent")) { + return new GitHubCliError({ + ...context, + detail: "GitHub CLI (`gh`) is required but not available on PATH.", + cause: error, + }); + } + + if ( + lower.includes("authentication failed") || + lower.includes("not logged in") || + lower.includes("gh auth login") || + lower.includes("no oauth token") + ) { + return new GitHubCliError({ + ...context, + detail: "GitHub CLI is not authenticated. Run `gh auth login` and retry.", + cause: error, + }); + } + + if ( + lower.includes("could not resolve to a pullrequest") || + lower.includes("repository.pullrequest") || + lower.includes("no pull requests found for branch") || + lower.includes("pull request not found") + ) { + return new GitHubCliError({ + ...context, + detail: "Pull request not found. Check the PR number or URL and try again.", + cause: error, + }); + } + + return new GitHubCliError({ + ...context, + detail: "GitHub CLI command failed.", + cause: error, + }); + } } export interface GitHubPullRequestSummary { @@ -110,54 +161,6 @@ function errorText(error: VcsError | unknown): string { return String(error); } -function normalizeGitHubCliError( - operation: "execute" | "stdout", - error: VcsError | unknown, -): GitHubCliError { - const text = errorText(error); - const lower = text.toLowerCase(); - - if (lower.includes("command not found: gh") || lower.includes("enoent")) { - return new GitHubCliError({ - operation, - detail: "GitHub CLI (`gh`) is required but not available on PATH.", - cause: error, - }); - } - - if ( - lower.includes("authentication failed") || - lower.includes("not logged in") || - lower.includes("gh auth login") || - lower.includes("no oauth token") - ) { - return new GitHubCliError({ - operation, - detail: "GitHub CLI is not authenticated. Run `gh auth login` and retry.", - cause: error, - }); - } - - if ( - lower.includes("could not resolve to a pullrequest") || - lower.includes("repository.pullrequest") || - lower.includes("no pull requests found for branch") || - lower.includes("pull request not found") - ) { - return new GitHubCliError({ - operation, - detail: "Pull request not found. Check the PR number or URL and try again.", - cause: error, - }); - } - - return new GitHubCliError({ - operation, - detail: text, - cause: error, - }); -} - const RawGitHubRepositoryCloneUrlsSchema = Schema.Struct({ nameWithOwner: TrimmedNonEmptyString, url: TrimmedNonEmptyString, @@ -216,13 +219,16 @@ function decodeGitHubJson( schema: S, operation: "listOpenPullRequests" | "getPullRequest" | "getRepositoryCloneUrls", invalidDetail: string, + cwd: string, ): Effect.Effect { return Schema.decodeEffect(Schema.fromJsonString(schema))(raw).pipe( Effect.mapError( (error) => new GitHubCliError({ operation, - detail: `${invalidDetail}: ${SchemaIssue.makeFormatterDefault()(error.issue)}`, + command: "gh", + cwd, + detail: invalidDetail, cause: error, }), ), @@ -241,7 +247,14 @@ export const make = Effect.gen(function* () { cwd: input.cwd, timeoutMs: input.timeoutMs ?? DEFAULT_TIMEOUT_MS, }) - .pipe(Effect.mapError((error) => normalizeGitHubCliError("execute", error))); + .pipe( + Effect.mapError((error) => + GitHubCliError.fromVcsError( + { operation: "execute", command: "gh", cwd: input.cwd }, + error, + ), + ), + ); return GitHubCli.of({ execute, @@ -271,7 +284,9 @@ export const make = Effect.gen(function* () { return Effect.fail( new GitHubCliError({ operation: "listOpenPullRequests", - detail: `GitHub CLI returned invalid PR list JSON: ${formatGitHubJsonDecodeError(decoded.failure)}`, + command: "gh", + cwd: input.cwd, + detail: "GitHub CLI returned invalid PR list JSON.", cause: decoded.failure, }), ); @@ -303,7 +318,9 @@ export const make = Effect.gen(function* () { return Effect.fail( new GitHubCliError({ operation: "getPullRequest", - detail: `GitHub CLI returned invalid pull request JSON: ${formatGitHubJsonDecodeError(decoded.failure)}`, + command: "gh", + cwd: input.cwd, + detail: "GitHub CLI returned invalid pull request JSON.", cause: decoded.failure, }), ); @@ -328,6 +345,7 @@ export const make = Effect.gen(function* () { RawGitHubRepositoryCloneUrlsSchema, "getRepositoryCloneUrls", "GitHub CLI returned invalid repository JSON.", + input.cwd, ), ), Effect.map(normalizeRepositoryCloneUrls), diff --git a/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts b/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts index 141672c91c5..c1aa8680b26 100644 --- a/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts +++ b/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts @@ -68,6 +68,49 @@ it.effect("maps GitHub PR summaries into provider-neutral change requests", () = }), ); +it.effect("adds safe request context while retaining GitHub CLI causes", () => + Effect.gen(function* () { + const cause = new GitHubCli.GitHubCliError({ + operation: "execute", + command: "gh", + cwd: "/repo", + detail: "Pull request not found. Check the PR number or URL and try again.", + cause: new Error("raw upstream detail that should remain in the cause"), + }); + const provider = yield* makeProvider({ + getPullRequest: () => Effect.fail(cause), + }); + + const error = yield* provider + .getChangeRequest({ + cwd: "/repo", + reference: "https://user:secret@github.com/pingdotgg/t3code/pull/42?token=secret#diff", + }) + .pipe(Effect.flip); + + assert.deepStrictEqual( + { + provider: error.provider, + operation: error.operation, + command: error.command, + cwd: error.cwd, + reference: error.reference, + detail: error.detail, + }, + { + provider: "github", + operation: "getChangeRequest", + command: "gh", + cwd: "/repo", + reference: "https://github.com/pingdotgg/t3code/pull/42", + detail: "Pull request not found. Check the PR number or URL and try again.", + }, + ); + assert.strictEqual(error.cause, cause); + assert.equal(error.message.includes("raw upstream detail"), false); + }), +); + it.effect("uses gh json listing for non-open change request state queries", () => Effect.gen(function* () { let executeArgs: ReadonlyArray = []; diff --git a/apps/server/src/sourceControl/GitHubSourceControlProvider.ts b/apps/server/src/sourceControl/GitHubSourceControlProvider.ts index b84d2504f93..60298888e6c 100644 --- a/apps/server/src/sourceControl/GitHubSourceControlProvider.ts +++ b/apps/server/src/sourceControl/GitHubSourceControlProvider.ts @@ -2,7 +2,6 @@ import * as Effect from "effect/Effect"; import * as Layer from "effect/Layer"; import * as Option from "effect/Option"; import * as Result from "effect/Result"; -import * as Schema from "effect/Schema"; import { SourceControlProviderError, type ChangeRequest, @@ -20,19 +19,6 @@ import { type SourceControlAuthProbeInput, type SourceControlCliDiscoverySpec, } from "./SourceControlProviderDiscovery.ts"; -const isSourceControlProviderError = Schema.is(SourceControlProviderError); - -function providerError( - operation: string, - cause: GitHubCli.GitHubCliError, -): SourceControlProviderError { - return new SourceControlProviderError({ - provider: "github", - operation, - detail: cause.detail, - cause, - }); -} function toChangeRequest(summary: GitHubCli.GitHubPullRequestSummary): ChangeRequest { return { @@ -122,7 +108,20 @@ export const make = Effect.gen(function* () { }) .pipe( Effect.map((items) => items.map(toChangeRequest)), - Effect.mapError((error) => providerError("listChangeRequests", error)), + Effect.mapError( + (error) => + new SourceControlProviderError({ + provider: "github", + operation: "listChangeRequests", + command: error.command, + cwd: input.cwd, + reference: SourceControlProvider.transportSafeSourceControlErrorValue( + input.headSelector, + ), + detail: error.detail, + cause: error, + }), + ), ); } @@ -162,6 +161,11 @@ export const make = Effect.gen(function* () { new SourceControlProviderError({ provider: "github", operation: "listChangeRequests", + command: "gh", + cwd: input.cwd, + reference: SourceControlProvider.transportSafeSourceControlErrorValue( + input.headSelector, + ), detail: "GitHub CLI returned invalid change request JSON.", cause: decoded.failure, }), @@ -169,11 +173,20 @@ export const make = Effect.gen(function* () { ), ); }), - Effect.mapError((error) => - isSourceControlProviderError(error) - ? error - : providerError("listChangeRequests", error), - ), + Effect.catchTags({ + GitHubCliError: (error) => + new SourceControlProviderError({ + provider: "github", + operation: "listChangeRequests", + command: error.command, + cwd: input.cwd, + reference: SourceControlProvider.transportSafeSourceControlErrorValue( + input.headSelector, + ), + detail: error.detail, + cause: error, + }), + }), ); }; @@ -183,7 +196,20 @@ export const make = Effect.gen(function* () { getChangeRequest: (input) => github.getPullRequest(input).pipe( Effect.map(toChangeRequest), - Effect.mapError((error) => providerError("getChangeRequest", error)), + Effect.mapError( + (error) => + new SourceControlProviderError({ + provider: "github", + operation: "getChangeRequest", + command: error.command, + cwd: input.cwd, + reference: SourceControlProvider.transportSafeSourceControlErrorValue( + input.reference, + ), + detail: error.detail, + cause: error, + }), + ), ), createChangeRequest: (input) => github @@ -194,23 +220,87 @@ export const make = Effect.gen(function* () { title: input.title, bodyFile: input.bodyFile, }) - .pipe(Effect.mapError((error) => providerError("createChangeRequest", error))), + .pipe( + Effect.mapError( + (error) => + new SourceControlProviderError({ + provider: "github", + operation: "createChangeRequest", + command: error.command, + cwd: input.cwd, + reference: SourceControlProvider.transportSafeSourceControlErrorValue( + input.headSelector, + ), + detail: error.detail, + cause: error, + }), + ), + ), getRepositoryCloneUrls: (input) => - github - .getRepositoryCloneUrls(input) - .pipe(Effect.mapError((error) => providerError("getRepositoryCloneUrls", error))), + github.getRepositoryCloneUrls(input).pipe( + Effect.mapError( + (error) => + new SourceControlProviderError({ + provider: "github", + operation: "getRepositoryCloneUrls", + command: error.command, + cwd: input.cwd, + repository: SourceControlProvider.transportSafeSourceControlErrorValue( + input.repository, + ), + detail: error.detail, + cause: error, + }), + ), + ), createRepository: (input) => - github - .createRepository(input) - .pipe(Effect.mapError((error) => providerError("createRepository", error))), + github.createRepository(input).pipe( + Effect.mapError( + (error) => + new SourceControlProviderError({ + provider: "github", + operation: "createRepository", + command: error.command, + cwd: input.cwd, + repository: SourceControlProvider.transportSafeSourceControlErrorValue( + input.repository, + ), + detail: error.detail, + cause: error, + }), + ), + ), getDefaultBranch: (input) => - github - .getDefaultBranch(input) - .pipe(Effect.mapError((error) => providerError("getDefaultBranch", error))), + github.getDefaultBranch(input).pipe( + Effect.mapError( + (error) => + new SourceControlProviderError({ + provider: "github", + operation: "getDefaultBranch", + command: error.command, + cwd: input.cwd, + detail: error.detail, + cause: error, + }), + ), + ), checkoutChangeRequest: (input) => - github - .checkoutPullRequest(input) - .pipe(Effect.mapError((error) => providerError("checkoutChangeRequest", error))), + github.checkoutPullRequest(input).pipe( + Effect.mapError( + (error) => + new SourceControlProviderError({ + provider: "github", + operation: "checkoutChangeRequest", + command: error.command, + cwd: input.cwd, + reference: SourceControlProvider.transportSafeSourceControlErrorValue( + input.reference, + ), + detail: error.detail, + cause: error, + }), + ), + ), }); }); diff --git a/apps/server/src/sourceControl/GitLabCli.test.ts b/apps/server/src/sourceControl/GitLabCli.test.ts index f7c3b3e4bf0..792e3a82b13 100644 --- a/apps/server/src/sourceControl/GitLabCli.test.ts +++ b/apps/server/src/sourceControl/GitLabCli.test.ts @@ -313,17 +313,14 @@ layer("GitLabCli.layer", (it) => { it.effect("surfaces a friendly error when the merge request is not found", () => Effect.gen(function* () { - mockedRun.mockReturnValueOnce( - Effect.fail( - new VcsProcessExitError({ - operation: "GitLabCli.execute", - command: "glab mr view 4888", - cwd: "/repo", - exitCode: 1, - detail: "GET 404 merge request not found", - }), - ), - ); + const cause = new VcsProcessExitError({ + operation: "GitLabCli.execute", + command: "glab mr view 4888", + cwd: "/repo", + exitCode: 1, + detail: "GET 404 merge request not found", + }); + mockedRun.mockReturnValueOnce(Effect.fail(cause)); const error = yield* Effect.gen(function* () { const glab = yield* GitLabCli.GitLabCli; @@ -334,6 +331,10 @@ layer("GitLabCli.layer", (it) => { }).pipe(Effect.flip); assert.equal(error.message.includes("Merge request not found"), true); + assert.strictEqual(error.command, "glab"); + assert.strictEqual(error.cwd, "/repo"); + assert.strictEqual(error.cause, cause); + assert.equal(error.message.includes(cause.detail), false); }), ); }); diff --git a/apps/server/src/sourceControl/GitLabCli.ts b/apps/server/src/sourceControl/GitLabCli.ts index c5fd7ee52f0..b34e72ffc95 100644 --- a/apps/server/src/sourceControl/GitLabCli.ts +++ b/apps/server/src/sourceControl/GitLabCli.ts @@ -4,16 +4,18 @@ import * as Layer from "effect/Layer"; import * as Option from "effect/Option"; import * as Result from "effect/Result"; import * as Schema from "effect/Schema"; -import * as SchemaIssue from "effect/SchemaIssue"; import type * as DateTime from "effect/DateTime"; -import { TrimmedNonEmptyString, type SourceControlRepositoryVisibility } from "@t3tools/contracts"; +import { + TrimmedNonEmptyString, + type SourceControlRepositoryVisibility, + type VcsError, +} from "@t3tools/contracts"; import * as VcsProcess from "../vcs/VcsProcess.ts"; import { decodeGitLabMergeRequestJson, decodeGitLabMergeRequestListJson, - formatGitLabJsonDecodeError, } from "./gitLabMergeRequests.ts"; import type * as SourceControlProvider from "./SourceControlProvider.ts"; @@ -21,12 +23,64 @@ const DEFAULT_TIMEOUT_MS = 30_000; export class GitLabCliError extends Schema.TaggedErrorClass()("GitLabCliError", { operation: Schema.String, + command: Schema.String, + cwd: Schema.String, detail: Schema.String, cause: Schema.optional(Schema.Defect()), }) { override get message(): string { return `GitLab CLI failed in ${this.operation}: ${this.detail}`; } + + static fromVcsError( + context: { + readonly operation: "execute"; + readonly command: "glab"; + readonly cwd: string; + }, + error: VcsError | unknown, + ): GitLabCliError { + const lower = errorText(error).toLowerCase(); + + if (lower.includes("command not found: glab") || isVcsProcessSpawnError(error)) { + return new GitLabCliError({ + ...context, + detail: "GitLab CLI (`glab`) is required but not available on PATH.", + cause: error, + }); + } + + if ( + lower.includes("authentication failed") || + lower.includes("not logged in") || + lower.includes("glab auth login") || + lower.includes("token") + ) { + return new GitLabCliError({ + ...context, + detail: "GitLab CLI is not authenticated. Run `glab auth login` and retry.", + cause: error, + }); + } + + if ( + lower.includes("merge request not found") || + lower.includes("not found") || + lower.includes("404") + ) { + return new GitLabCliError({ + ...context, + detail: "Merge request not found. Check the MR number or URL and try again.", + cause: error, + }); + } + + return new GitLabCliError({ + ...context, + detail: "GitLab CLI command failed.", + cause: error, + }); + } } export interface GitLabMergeRequestSummary { @@ -48,6 +102,17 @@ export interface GitLabRepositoryCloneUrls { readonly sshUrl: string; } +function errorText(error: VcsError | unknown): string { + if (typeof error === "object" && error !== null) { + const tag = "_tag" in error && typeof error._tag === "string" ? error._tag : ""; + const detail = "detail" in error && typeof error.detail === "string" ? error.detail : ""; + const message = "message" in error && typeof error.message === "string" ? error.message : ""; + return [tag, detail, message].filter(Boolean).join("\n"); + } + + return String(error); +} + export class GitLabCli extends Context.Service< GitLabCli, { @@ -112,56 +177,6 @@ function isVcsProcessSpawnError(error: unknown): boolean { ); } -function normalizeGitLabCliError(operation: "execute" | "stdout", error: unknown): GitLabCliError { - if (error instanceof Error) { - if (error.message.includes("Command not found: glab") || isVcsProcessSpawnError(error)) { - return new GitLabCliError({ - operation, - detail: "GitLab CLI (`glab`) is required but not available on PATH.", - cause: error, - }); - } - - const lower = error.message.toLowerCase(); - if ( - lower.includes("authentication failed") || - lower.includes("not logged in") || - lower.includes("glab auth login") || - lower.includes("token") - ) { - return new GitLabCliError({ - operation, - detail: "GitLab CLI is not authenticated. Run `glab auth login` and retry.", - cause: error, - }); - } - - if ( - lower.includes("merge request not found") || - lower.includes("not found") || - lower.includes("404") - ) { - return new GitLabCliError({ - operation, - detail: "Merge request not found. Check the MR number or URL and try again.", - cause: error, - }); - } - - return new GitLabCliError({ - operation, - detail: `GitLab CLI command failed: ${error.message}`, - cause: error, - }); - } - - return new GitLabCliError({ - operation, - detail: "GitLab CLI command failed.", - cause: error, - }); -} - const RawGitLabRepositoryCloneUrlsSchema = Schema.Struct({ path_with_namespace: TrimmedNonEmptyString, web_url: TrimmedNonEmptyString, @@ -192,13 +207,16 @@ function decodeGitLabJson( schema: S, operation: "getRepositoryCloneUrls" | "getDefaultBranch" | "createRepository", invalidDetail: string, + cwd: string, ): Effect.Effect { return Schema.decodeEffect(Schema.fromJsonString(schema))(raw).pipe( Effect.mapError( (error) => new GitLabCliError({ operation, - detail: `${invalidDetail}: ${SchemaIssue.makeFormatterDefault()(error.issue)}`, + command: "glab", + cwd, + detail: invalidDetail, cause: error, }), ), @@ -274,7 +292,14 @@ export const make = Effect.gen(function* () { cwd: input.cwd, timeoutMs: input.timeoutMs ?? DEFAULT_TIMEOUT_MS, }) - .pipe(Effect.mapError((error) => normalizeGitLabCliError("execute", error))); + .pipe( + Effect.mapError((error) => + GitLabCliError.fromVcsError( + { operation: "execute", command: "glab", cwd: input.cwd }, + error, + ), + ), + ); return GitLabCli.of({ execute, @@ -303,7 +328,9 @@ export const make = Effect.gen(function* () { return Effect.fail( new GitLabCliError({ operation: "listMergeRequests", - detail: `GitLab CLI returned invalid MR list JSON: ${formatGitLabJsonDecodeError(decoded.failure)}`, + command: "glab", + cwd: input.cwd, + detail: "GitLab CLI returned invalid MR list JSON.", cause: decoded.failure, }), ); @@ -327,7 +354,9 @@ export const make = Effect.gen(function* () { return Effect.fail( new GitLabCliError({ operation: "getMergeRequest", - detail: `GitLab CLI returned invalid merge request JSON: ${formatGitLabJsonDecodeError(decoded.failure)}`, + command: "glab", + cwd: input.cwd, + detail: "GitLab CLI returned invalid merge request JSON.", cause: decoded.failure, }), ); @@ -350,6 +379,7 @@ export const make = Effect.gen(function* () { RawGitLabRepositoryCloneUrlsSchema, "getRepositoryCloneUrls", "GitLab CLI returned invalid repository JSON.", + input.cwd, ), ), Effect.map(normalizeRepositoryCloneUrls), @@ -368,6 +398,7 @@ export const make = Effect.gen(function* () { RawGitLabNamespaceSchema, "createRepository", "GitLab CLI returned invalid namespace JSON.", + input.cwd, ), ), Effect.map((namespace) => namespace.id), @@ -402,6 +433,7 @@ export const make = Effect.gen(function* () { RawGitLabRepositoryCloneUrlsSchema, "createRepository", "GitLab CLI returned invalid repository JSON.", + input.cwd, ), ), Effect.map(normalizeRepositoryCloneUrls), @@ -440,6 +472,7 @@ export const make = Effect.gen(function* () { RawGitLabDefaultBranchSchema, "getDefaultBranch", "GitLab CLI returned invalid repository JSON.", + input.cwd, ), ), Effect.map((value) => value.default_branch ?? null), diff --git a/apps/server/src/sourceControl/GitLabSourceControlProvider.test.ts b/apps/server/src/sourceControl/GitLabSourceControlProvider.test.ts index 3dc61e132f3..6ab3f23b150 100644 --- a/apps/server/src/sourceControl/GitLabSourceControlProvider.test.ts +++ b/apps/server/src/sourceControl/GitLabSourceControlProvider.test.ts @@ -52,6 +52,50 @@ it.effect("maps GitLab MR summaries into provider-neutral change requests", () = }), ); +it.effect("adds repository context while retaining GitLab CLI causes", () => + Effect.gen(function* () { + const cause = new GitLabCli.GitLabCliError({ + operation: "execute", + command: "glab", + cwd: "/repo", + detail: "GitLab CLI command failed.", + cause: new Error("raw upstream detail that should remain in the cause"), + }); + const provider = yield* makeProvider({ + createRepository: () => Effect.fail(cause), + }); + + const error = yield* provider + .createRepository({ + cwd: "/repo", + repository: "owner/repo", + visibility: "private", + }) + .pipe(Effect.flip); + + assert.deepStrictEqual( + { + provider: error.provider, + operation: error.operation, + command: error.command, + cwd: error.cwd, + repository: error.repository, + detail: error.detail, + }, + { + provider: "gitlab", + operation: "createRepository", + command: "glab", + cwd: "/repo", + repository: "owner/repo", + detail: "GitLab CLI command failed.", + }, + ); + assert.strictEqual(error.cause, cause); + assert.equal(error.message.includes("raw upstream detail"), false); + }), +); + it.effect("lists GitLab MRs through provider-neutral input names", () => Effect.gen(function* () { let listInput: Parameters[0] | null = null; diff --git a/apps/server/src/sourceControl/GitLabSourceControlProvider.ts b/apps/server/src/sourceControl/GitLabSourceControlProvider.ts index d1aaf06309d..2cba12f1b3f 100644 --- a/apps/server/src/sourceControl/GitLabSourceControlProvider.ts +++ b/apps/server/src/sourceControl/GitLabSourceControlProvider.ts @@ -17,18 +17,6 @@ import { } from "./SourceControlProviderDiscovery.ts"; import { findAuthenticatedGitLabHost, parseGitLabAuthStatusHosts } from "./gitLabAuthStatus.ts"; -function providerError( - operation: string, - cause: GitLabCli.GitLabCliError, -): SourceControlProviderError { - return new SourceControlProviderError({ - provider: "gitlab", - operation, - detail: cause.detail, - cause, - }); -} - function toChangeRequest(summary: GitLabCli.GitLabMergeRequestSummary): ChangeRequest { return { provider: "gitlab", @@ -129,13 +117,39 @@ export const make = Effect.gen(function* () { }) .pipe( Effect.map((items) => items.map(toChangeRequest)), - Effect.mapError((error) => providerError("listChangeRequests", error)), + Effect.mapError( + (error) => + new SourceControlProviderError({ + provider: "gitlab", + operation: "listChangeRequests", + command: error.command, + cwd: input.cwd, + reference: SourceControlProvider.transportSafeSourceControlErrorValue( + input.headSelector, + ), + detail: error.detail, + cause: error, + }), + ), ); }, getChangeRequest: (input) => gitlab.getMergeRequest(input).pipe( Effect.map(toChangeRequest), - Effect.mapError((error) => providerError("getChangeRequest", error)), + Effect.mapError( + (error) => + new SourceControlProviderError({ + provider: "gitlab", + operation: "getChangeRequest", + command: error.command, + cwd: input.cwd, + reference: SourceControlProvider.transportSafeSourceControlErrorValue( + input.reference, + ), + detail: error.detail, + cause: error, + }), + ), ), createChangeRequest: (input) => { const source = SourceControlProvider.sourceControlRefFromInput(input); @@ -149,24 +163,88 @@ export const make = Effect.gen(function* () { title: input.title, bodyFile: input.bodyFile, }) - .pipe(Effect.mapError((error) => providerError("createChangeRequest", error))); + .pipe( + Effect.mapError( + (error) => + new SourceControlProviderError({ + provider: "gitlab", + operation: "createChangeRequest", + command: error.command, + cwd: input.cwd, + reference: SourceControlProvider.transportSafeSourceControlErrorValue( + input.headSelector, + ), + detail: error.detail, + cause: error, + }), + ), + ); }, getRepositoryCloneUrls: (input) => - gitlab - .getRepositoryCloneUrls(input) - .pipe(Effect.mapError((error) => providerError("getRepositoryCloneUrls", error))), + gitlab.getRepositoryCloneUrls(input).pipe( + Effect.mapError( + (error) => + new SourceControlProviderError({ + provider: "gitlab", + operation: "getRepositoryCloneUrls", + command: error.command, + cwd: input.cwd, + repository: SourceControlProvider.transportSafeSourceControlErrorValue( + input.repository, + ), + detail: error.detail, + cause: error, + }), + ), + ), createRepository: (input) => - gitlab - .createRepository(input) - .pipe(Effect.mapError((error) => providerError("createRepository", error))), + gitlab.createRepository(input).pipe( + Effect.mapError( + (error) => + new SourceControlProviderError({ + provider: "gitlab", + operation: "createRepository", + command: error.command, + cwd: input.cwd, + repository: SourceControlProvider.transportSafeSourceControlErrorValue( + input.repository, + ), + detail: error.detail, + cause: error, + }), + ), + ), getDefaultBranch: (input) => - gitlab - .getDefaultBranch(input) - .pipe(Effect.mapError((error) => providerError("getDefaultBranch", error))), + gitlab.getDefaultBranch(input).pipe( + Effect.mapError( + (error) => + new SourceControlProviderError({ + provider: "gitlab", + operation: "getDefaultBranch", + command: error.command, + cwd: input.cwd, + detail: error.detail, + cause: error, + }), + ), + ), checkoutChangeRequest: (input) => - gitlab - .checkoutMergeRequest(input) - .pipe(Effect.mapError((error) => providerError("checkoutChangeRequest", error))), + gitlab.checkoutMergeRequest(input).pipe( + Effect.mapError( + (error) => + new SourceControlProviderError({ + provider: "gitlab", + operation: "checkoutChangeRequest", + command: error.command, + cwd: input.cwd, + reference: SourceControlProvider.transportSafeSourceControlErrorValue( + input.reference, + ), + detail: error.detail, + cause: error, + }), + ), + ), }); }); diff --git a/apps/server/src/sourceControl/SourceControlProvider.test.ts b/apps/server/src/sourceControl/SourceControlProvider.test.ts new file mode 100644 index 00000000000..7e532488279 --- /dev/null +++ b/apps/server/src/sourceControl/SourceControlProvider.test.ts @@ -0,0 +1,19 @@ +import { assert, it } from "@effect/vitest"; + +import { transportSafeSourceControlErrorValue } from "./SourceControlProvider.ts"; + +it("removes URL credentials, query parameters, and fragments from error transport values", () => { + assert.strictEqual( + transportSafeSourceControlErrorValue( + "https://user:secret@example.test/org/repo/pull/42?token=secret#discussion", + ), + "https://example.test/org/repo/pull/42", + ); +}); + +it("normalizes control characters and bounds error transport values", () => { + assert.strictEqual( + transportSafeSourceControlErrorValue(` owner/repo\n\t${"x".repeat(300)} `), + `owner/repo ${"x".repeat(245)}`, + ); +}); diff --git a/apps/server/src/sourceControl/SourceControlProvider.ts b/apps/server/src/sourceControl/SourceControlProvider.ts index c2959ef878e..5f93dbcaa42 100644 --- a/apps/server/src/sourceControl/SourceControlProvider.ts +++ b/apps/server/src/sourceControl/SourceControlProvider.ts @@ -22,6 +22,36 @@ export interface SourceControlRefSelector { readonly repository?: string; } +const MAX_ERROR_TRANSPORT_VALUE_LENGTH = 256; + +/** + * Sanitizes user-provided source-control identifiers before attaching them to + * contract errors. This is intentionally narrower than request validation: it + * only strips URL secrets and bounds diagnostic values sent over transport. + */ +export function transportSafeSourceControlErrorValue(value: string): string { + let printable = ""; + for (const character of value) { + const codePoint = character.codePointAt(0); + printable += codePoint !== undefined && (codePoint < 32 || codePoint === 127) ? " " : character; + } + const normalized = printable.trim().replace(/\s+/gu, " "); + + let safe = normalized; + try { + const url = new URL(normalized); + url.username = ""; + url.password = ""; + url.search = ""; + url.hash = ""; + safe = url.toString(); + } catch { + // Plain repository and change-request identifiers are not URLs. + } + + return safe.slice(0, MAX_ERROR_TRANSPORT_VALUE_LENGTH); +} + export function parseSourceControlOwnerRef( headSelector: string, ): SourceControlRefSelector | undefined { diff --git a/apps/server/src/sourceControl/SourceControlProviderRegistry.test.ts b/apps/server/src/sourceControl/SourceControlProviderRegistry.test.ts index 6cea2d9a496..5c4d27e46f9 100644 --- a/apps/server/src/sourceControl/SourceControlProviderRegistry.test.ts +++ b/apps/server/src/sourceControl/SourceControlProviderRegistry.test.ts @@ -5,6 +5,7 @@ import * as Effect from "effect/Effect"; import * as Layer from "effect/Layer"; import * as Option from "effect/Option"; import { ChildProcessSpawner } from "effect/unstable/process"; +import { VcsRepositoryDetectionError } from "@t3tools/contracts"; import * as ServerConfig from "../config.ts"; import type * as VcsDriver from "../vcs/VcsDriver.ts"; @@ -38,6 +39,7 @@ function makeRegistry(input: { readonly url: string; }>; readonly process?: Partial; + readonly resolve?: VcsDriverRegistry.VcsDriverRegistry["Service"]["resolve"]; }) { const driver = { listRemotes: () => @@ -57,21 +59,23 @@ function makeRegistry(input: { const registryLayer = Layer.mock(VcsDriverRegistry.VcsDriverRegistry)({ get: () => Effect.succeed(driver as unknown as VcsDriver.VcsDriver["Service"]), - resolve: () => - Effect.succeed({ - kind: "git", - repository: { + resolve: + input.resolve ?? + (() => + Effect.succeed({ kind: "git", - rootPath: "/repo", - metadataPath: null, - freshness: { - source: "live-local" as const, - observedAt: TEST_EPOCH, - expiresAt: Option.none(), + repository: { + kind: "git", + rootPath: "/repo", + metadataPath: null, + freshness: { + source: "live-local" as const, + observedAt: TEST_EPOCH, + expiresAt: Option.none(), + }, }, - }, - driver: driver as unknown as VcsDriver.VcsDriver["Service"], - }), + driver: driver as unknown as VcsDriver.VcsDriver["Service"], + })), }); const processLayer = Layer.mock(VcsProcess.VcsProcess)({ @@ -120,6 +124,46 @@ it.effect("routes directly by provider kind for remote-first workflows", () => }), ); +it.effect("includes the request cwd when an unregistered provider is used", () => + Effect.gen(function* () { + const registry = yield* makeRegistry({ remotes: [] }); + const provider = yield* registry.get("unknown"); + + const error = yield* provider + .getChangeRequest({ cwd: "/repo", reference: "#42" }) + .pipe(Effect.flip); + + assert.strictEqual(error.provider, "unknown"); + assert.strictEqual(error.operation, "getChangeRequest"); + assert.strictEqual(error.cwd, "/repo"); + assert.strictEqual(error.reference, "#42"); + }), +); + +it.effect("retains VCS detection failures with structured cwd context", () => + Effect.gen(function* () { + const cause = new VcsRepositoryDetectionError({ + operation: "resolve", + cwd: "/repo", + detail: "raw VCS detection failure", + cause: new Error("raw nested failure"), + }); + const registry = yield* makeRegistry({ + remotes: [], + resolve: () => Effect.fail(cause), + }); + + const error = yield* registry.resolve({ cwd: "/repo" }).pipe(Effect.flip); + + assert.strictEqual(error.provider, "unknown"); + assert.strictEqual(error.operation, "detectProvider"); + assert.strictEqual(error.cwd, "/repo"); + assert.strictEqual(error.detail, "Failed to detect source control provider."); + assert.strictEqual(error.cause, cause); + assert.equal(error.message.includes(cause.message), false); + }), +); + it.effect("routes GitLab remotes to the GitLab provider", () => Effect.gen(function* () { const registry = yield* makeRegistry({ diff --git a/apps/server/src/sourceControl/SourceControlProviderRegistry.ts b/apps/server/src/sourceControl/SourceControlProviderRegistry.ts index b1f1ea7aae7..fb70d677e43 100644 --- a/apps/server/src/sourceControl/SourceControlProviderRegistry.ts +++ b/apps/server/src/sourceControl/SourceControlProviderRegistry.ts @@ -64,33 +64,62 @@ export class SourceControlProviderRegistry extends Context.Service< function unsupportedProvider( kind: SourceControlProviderKind, ): SourceControlProvider.SourceControlProvider["Service"] { - const unsupported = (operation: string) => - Effect.fail( + return SourceControlProvider.SourceControlProvider.of({ + kind, + listChangeRequests: (input) => new SourceControlProviderError({ provider: kind, - operation, + operation: "listChangeRequests", + cwd: input.cwd, + detail: `No ${kind} source control provider is registered.`, + }), + getChangeRequest: (input) => + new SourceControlProviderError({ + provider: kind, + operation: "getChangeRequest", + cwd: input.cwd, + reference: SourceControlProvider.transportSafeSourceControlErrorValue(input.reference), + detail: `No ${kind} source control provider is registered.`, + }), + createChangeRequest: (input) => + new SourceControlProviderError({ + provider: kind, + operation: "createChangeRequest", + cwd: input.cwd, + reference: SourceControlProvider.transportSafeSourceControlErrorValue(input.headSelector), + detail: `No ${kind} source control provider is registered.`, + }), + getRepositoryCloneUrls: (input) => + new SourceControlProviderError({ + provider: kind, + operation: "getRepositoryCloneUrls", + cwd: input.cwd, + repository: SourceControlProvider.transportSafeSourceControlErrorValue(input.repository), + detail: `No ${kind} source control provider is registered.`, + }), + createRepository: (input) => + new SourceControlProviderError({ + provider: kind, + operation: "createRepository", + cwd: input.cwd, + repository: SourceControlProvider.transportSafeSourceControlErrorValue(input.repository), + detail: `No ${kind} source control provider is registered.`, + }), + getDefaultBranch: (input) => + new SourceControlProviderError({ + provider: kind, + operation: "getDefaultBranch", + cwd: input.cwd, + detail: `No ${kind} source control provider is registered.`, + }), + checkoutChangeRequest: (input) => + new SourceControlProviderError({ + provider: kind, + operation: "checkoutChangeRequest", + cwd: input.cwd, + reference: SourceControlProvider.transportSafeSourceControlErrorValue(input.reference), detail: `No ${kind} source control provider is registered.`, }), - ); - - return SourceControlProvider.SourceControlProvider.of({ - kind, - listChangeRequests: () => unsupported("listChangeRequests"), - getChangeRequest: () => unsupported("getChangeRequest"), - createChangeRequest: () => unsupported("createChangeRequest"), - getRepositoryCloneUrls: () => unsupported("getRepositoryCloneUrls"), - createRepository: () => unsupported("createRepository"), - getDefaultBranch: () => unsupported("getDefaultBranch"), - checkoutChangeRequest: () => unsupported("checkoutChangeRequest"), - }); -} - -function providerDetectionError(operation: string, cwd: string, cause: unknown) { - return new SourceControlProviderError({ - provider: "unknown", - operation, - detail: `Failed to detect source control provider for ${cwd}.`, - cause, }); } @@ -180,12 +209,30 @@ export const makeWithProviders = Effect.fn("makeSourceControlProviderRegistryWit const detectProviderContext = Effect.fn("SourceControlProviderRegistry.detectProviderContext")( function* (cwd: string) { - const handle = yield* vcsRegistry - .resolve({ cwd }) - .pipe(Effect.mapError((error) => providerDetectionError("detectProvider", cwd, error))); - const remotes = yield* handle.driver - .listRemotes(cwd) - .pipe(Effect.mapError((error) => providerDetectionError("detectProvider", cwd, error))); + const handle = yield* vcsRegistry.resolve({ cwd }).pipe( + Effect.mapError( + (error) => + new SourceControlProviderError({ + provider: "unknown", + operation: "detectProvider", + cwd, + detail: "Failed to detect source control provider.", + cause: error, + }), + ), + ); + const remotes = yield* handle.driver.listRemotes(cwd).pipe( + Effect.mapError( + (error) => + new SourceControlProviderError({ + provider: "unknown", + operation: "detectProvider", + cwd, + detail: "Failed to detect source control provider.", + cause: error, + }), + ), + ); const context = selectProviderContext(remotes.remotes); return yield* refineUnknownRemoteProvider({ diff --git a/packages/contracts/src/sourceControl.ts b/packages/contracts/src/sourceControl.ts index 0ecf13c67e6..104aadd9161 100644 --- a/packages/contracts/src/sourceControl.ts +++ b/packages/contracts/src/sourceControl.ts @@ -155,6 +155,10 @@ export class SourceControlProviderError extends Schema.TaggedErrorClass