Explorar el Código

refactor(repository): type cache failures (#28188)

Shoubhit Dash hace 3 meses
padre
commit
94828eb44b

+ 141 - 12
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<InvalidRepositoryError>()(
+  "RepositoryCacheInvalidRepositoryError",
+  {
+    repository: Schema.String,
+    message: Schema.String,
+  },
+) {}
+
+export class InvalidBranchError extends Schema.TaggedErrorClass<InvalidBranchError>()(
+  "RepositoryCacheInvalidBranchError",
+  {
+    branch: Schema.String,
+    message: Schema.String,
+  },
+) {}
+
+export class CloneFailedError extends Schema.TaggedErrorClass<CloneFailedError>()("RepositoryCacheCloneFailedError", {
+  repository: Schema.String,
+  message: Schema.String,
+}) {}
+
+export class FetchFailedError extends Schema.TaggedErrorClass<FetchFailedError>()("RepositoryCacheFetchFailedError", {
+  repository: Schema.String,
+  message: Schema.String,
+}) {}
+
+export class CheckoutFailedError extends Schema.TaggedErrorClass<CheckoutFailedError>()(
+  "RepositoryCacheCheckoutFailedError",
+  {
+    repository: Schema.String,
+    branch: Schema.String,
+    message: Schema.String,
+  },
+) {}
+
+export class ResetFailedError extends Schema.TaggedErrorClass<ResetFailedError>()("RepositoryCacheResetFailedError", {
+  repository: Schema.String,
+  message: Schema.String,
+}) {}
+
+export class LockFailedError extends Schema.TaggedErrorClass<LockFailedError>()("RepositoryCacheLockFailedError", {
+  localPath: Schema.String,
+  message: Schema.String,
+}) {}
+
+export class CacheOperationError extends Schema.TaggedErrorClass<CacheOperationError>()("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<Result, unknown>
+  ensure: (input: EnsureInput) => Effect.Effect<Result, Error>
 }
 
 export class Service extends Context.Service<Service, Interface>()("@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}`,
+            })
           }
         }
 

+ 7 - 4
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<typeof Parameters, Metadata, Repository
       parameters: Parameters,
       execute: (params: Schema.Schema.Type<typeof Parameters>, ctx: Tool.Context<Metadata>) =>
         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<typeof Parameters, Metadata, Repository
               ...(result.head ? [`HEAD: ${result.head}`] : []),
             ].join("\n"),
           }
-        }).pipe(Effect.orDie),
+        }).pipe(
+          Effect.catchIf(RepositoryCache.isError, (error) => Effect.fail(new Error(error.message))),
+          Effect.orDie,
+        ),
     } satisfies Tool.DefWithoutID<typeof Parameters, Metadata>
   }),
 )

+ 17 - 0
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",
+          )
+        }
+      }),
+    ),
+  )
 })