diff --git a/packages/opencode/src/reference/repository-cache.ts b/packages/opencode/src/reference/repository-cache.ts index 1f3ec7bf68..3679f0a047 100644 --- a/packages/opencode/src/reference/repository-cache.ts +++ b/packages/opencode/src/reference/repository-cache.ts @@ -1,5 +1,5 @@ import path from "path" -import { Context, Effect, Layer } from "effect" +import { Context, Effect, Layer, Schema } from "effect" import { AppFileSystem } from "@opencode-ai/core/filesystem" import { Flock } from "@opencode-ai/core/util/flock" import { Git } from "@/git" @@ -8,6 +8,7 @@ import { sameRepositoryReference, parseRepositoryReference, validateRepositoryBranch, + isRemoteRepositoryReference, type RemoteReference, } from "@/util/repository" @@ -27,8 +28,69 @@ export type EnsureInput = { branch?: string } +export class InvalidRepositoryError extends Schema.TaggedErrorClass()( + "RepositoryCacheInvalidRepositoryError", + { + repository: Schema.String, + message: Schema.String, + }, +) {} + +export class InvalidBranchError extends Schema.TaggedErrorClass()( + "RepositoryCacheInvalidBranchError", + { + branch: Schema.String, + message: Schema.String, + }, +) {} + +export class CloneFailedError extends Schema.TaggedErrorClass()("RepositoryCacheCloneFailedError", { + repository: Schema.String, + message: Schema.String, +}) {} + +export class FetchFailedError extends Schema.TaggedErrorClass()("RepositoryCacheFetchFailedError", { + repository: Schema.String, + message: Schema.String, +}) {} + +export class CheckoutFailedError extends Schema.TaggedErrorClass()( + "RepositoryCacheCheckoutFailedError", + { + repository: Schema.String, + branch: Schema.String, + message: Schema.String, + }, +) {} + +export class ResetFailedError extends Schema.TaggedErrorClass()("RepositoryCacheResetFailedError", { + repository: Schema.String, + message: Schema.String, +}) {} + +export class LockFailedError extends Schema.TaggedErrorClass()("RepositoryCacheLockFailedError", { + localPath: Schema.String, + message: Schema.String, +}) {} + +export class CacheOperationError extends Schema.TaggedErrorClass()("RepositoryCacheOperationError", { + operation: Schema.String, + path: Schema.String, + message: Schema.String, +}) {} + +export type Error = + | InvalidRepositoryError + | InvalidBranchError + | CloneFailedError + | FetchFailedError + | CheckoutFailedError + | ResetFailedError + | LockFailedError + | CacheOperationError + export interface Interface { - ensure: (input: EnsureInput) => Effect.Effect + ensure: (input: EnsureInput) => Effect.Effect } export class Service extends Context.Service()("@opencode/RepositoryCache") {} @@ -55,6 +117,45 @@ function resetTarget(input: { return "HEAD" } +function errorMessage(error: unknown) { + return error instanceof globalThis.Error ? error.message : String(error) +} + +export function isError(error: unknown): error is Error { + return ( + error instanceof InvalidRepositoryError || + error instanceof InvalidBranchError || + error instanceof CloneFailedError || + error instanceof FetchFailedError || + error instanceof CheckoutFailedError || + error instanceof ResetFailedError || + error instanceof LockFailedError || + error instanceof CacheOperationError + ) +} + +export const parseRemoteReference = Effect.fn("RepositoryCache.parseRemoteReference")(function* (repository: string) { + const reference = parseRepositoryReference(repository) + if (!reference) { + return yield* new InvalidRepositoryError({ + repository, + message: "Repository must be a git URL, host/path reference, or GitHub owner/repo shorthand", + }) + } + if (!isRemoteRepositoryReference(reference)) { + return yield* new InvalidRepositoryError({ repository, message: "Local file repositories are not supported" }) + } + return reference +}) + +export const validateBranch = Effect.fn("RepositoryCache.validateBranch")(function* (branch: string) { + try { + validateRepositoryBranch(branch) + } catch (error) { + return yield* new InvalidBranchError({ branch, message: errorMessage(error) }) + } +}) + const ensureWithServices = Effect.fn("RepositoryCache.ensureWithServices")(function* ( input: EnsureInput, services: { @@ -62,7 +163,7 @@ const ensureWithServices = Effect.fn("RepositoryCache.ensureWithServices")(funct git: Git.Interface }, ) { - if (input.branch) validateRepositoryBranch(input.branch) + if (input.branch) yield* validateBranch(input.branch) const repository = input.reference.label const remote = input.reference.remote @@ -70,10 +171,20 @@ const ensureWithServices = Effect.fn("RepositoryCache.ensureWithServices")(funct const cloneTarget = parseRepositoryReference(remote) ?? input.reference return yield* Effect.acquireUseRelease( - Effect.promise((signal) => Flock.acquire(`repo-clone:${localPath}`, { signal })), + Effect.promise((signal) => Flock.acquire(`repo-clone:${localPath}`, { signal })).pipe( + Effect.catch((error: unknown) => + Effect.fail(new LockFailedError({ localPath, message: errorMessage(error) || `Failed to lock ${localPath}` })), + ), + ), () => Effect.gen(function* () { - yield* services.fs.ensureDir(path.dirname(localPath)).pipe(Effect.orDie) + yield* services.fs.ensureDir(path.dirname(localPath)).pipe( + Effect.catch((error: unknown) => + Effect.fail( + new CacheOperationError({ operation: "ensure cache directory", path: localPath, message: errorMessage(error) }), + ), + ), + ) const exists = yield* services.fs.existsSafe(localPath) const hasGitDir = yield* services.fs.existsSafe(path.join(localPath, ".git")) @@ -83,7 +194,13 @@ const ensureWithServices = Effect.fn("RepositoryCache.ensureWithServices")(funct const originReference = origin?.exitCode === 0 ? parseRepositoryReference(origin.text().trim()) : undefined const reuse = hasGitDir && Boolean(originReference && sameRepositoryReference(originReference, cloneTarget)) if (exists && !reuse) { - yield* services.fs.remove(localPath, { recursive: true }).pipe(Effect.orDie) + yield* services.fs.remove(localPath, { recursive: true }).pipe( + Effect.catch((error: unknown) => + Effect.fail( + new CacheOperationError({ operation: "remove stale cache", path: localPath, message: errorMessage(error) }), + ), + ), + ) } const currentBranch = hasGitDir ? yield* services.git.branch(localPath) : undefined @@ -99,14 +216,20 @@ const ensureWithServices = Effect.fn("RepositoryCache.ensureWithServices")(funct { cwd: path.dirname(localPath) }, ) if (clone.exitCode !== 0) { - throw new Error(clone.stderr.toString().trim() || clone.text().trim() || `Failed to clone ${repository}`) + return yield* new CloneFailedError({ + repository, + message: clone.stderr.toString().trim() || clone.text().trim() || `Failed to clone ${repository}`, + }) } } if (status === "refreshed") { const fetch = yield* services.git.run(["fetch", "--all", "--prune"], { cwd: localPath }) if (fetch.exitCode !== 0) { - throw new Error(fetch.stderr.toString().trim() || fetch.text().trim() || `Failed to refresh ${repository}`) + return yield* new FetchFailedError({ + repository, + message: fetch.stderr.toString().trim() || fetch.text().trim() || `Failed to refresh ${repository}`, + }) } if (input.branch) { @@ -114,9 +237,12 @@ const ensureWithServices = Effect.fn("RepositoryCache.ensureWithServices")(funct cwd: localPath, }) if (checkout.exitCode !== 0) { - throw new Error( - checkout.stderr.toString().trim() || checkout.text().trim() || `Failed to checkout ${input.branch}`, - ) + return yield* new CheckoutFailedError({ + repository, + branch: input.branch, + message: + checkout.stderr.toString().trim() || checkout.text().trim() || `Failed to checkout ${input.branch}`, + }) } } @@ -130,7 +256,10 @@ const ensureWithServices = Effect.fn("RepositoryCache.ensureWithServices")(funct const reset = yield* services.git.run(["reset", "--hard", target], { cwd: localPath }) if (reset.exitCode !== 0) { - throw new Error(reset.stderr.toString().trim() || reset.text().trim() || `Failed to reset ${repository}`) + return yield* new ResetFailedError({ + repository, + message: reset.stderr.toString().trim() || reset.text().trim() || `Failed to reset ${repository}`, + }) } } diff --git a/packages/opencode/src/tool/repo_clone.ts b/packages/opencode/src/tool/repo_clone.ts index 8515f27661..3c5a6e8933 100644 --- a/packages/opencode/src/tool/repo_clone.ts +++ b/packages/opencode/src/tool/repo_clone.ts @@ -1,7 +1,7 @@ import { Effect, Schema } from "effect" import DESCRIPTION from "./repo_clone.txt" import * as Tool from "./tool" -import { parseRemoteRepositoryReference, repositoryCachePath, validateRepositoryBranch } from "@/util/repository" +import { repositoryCachePath } from "@/util/repository" import { RepositoryCache } from "@/reference/repository-cache" export const Parameters = Schema.Struct({ @@ -36,8 +36,8 @@ export const RepoCloneTool = Tool.define, ctx: Tool.Context) => Effect.gen(function* () { - const reference = parseRemoteRepositoryReference(params.repository) - if (params.branch) validateRepositoryBranch(params.branch) + const reference = yield* RepositoryCache.parseRemoteReference(params.repository) + if (params.branch) yield* RepositoryCache.validateBranch(params.branch) const repository = reference.label const remote = reference.remote @@ -68,7 +68,10 @@ export const RepoCloneTool = Tool.define Effect.fail(new Error(error.message))), + Effect.orDie, + ), } satisfies Tool.DefWithoutID }), ) diff --git a/packages/opencode/test/tool/repo_clone.test.ts b/packages/opencode/test/tool/repo_clone.test.ts index cba0885477..2d7c70efcf 100644 --- a/packages/opencode/test/tool/repo_clone.test.ts +++ b/packages/opencode/test/tool/repo_clone.test.ts @@ -225,4 +225,21 @@ describe("tool.repo_clone", () => { }), ), ) + + it.live("rejects invalid branch inputs", () => + provideTmpdirInstance((_dir) => + Effect.gen(function* () { + const tool = yield* init() + const result = yield* tool.execute({ repository: "owner/repo", branch: "bad..branch" }, ctx).pipe(Effect.exit) + + expect(Exit.isFailure(result)).toBe(true) + if (Exit.isFailure(result)) { + const error = Cause.squash(result.cause) + expect(error instanceof Error ? error.message : String(error)).toContain( + "Branch must contain only alphanumeric characters", + ) + } + }), + ), + ) })