瀏覽代碼

fix(core): branch-keyed repository cache with gated reference readiness (#38759)

Kit Langton 1 月之前
父節點
當前提交
065dc274ec

+ 1 - 5
packages/core/src/reference.ts

@@ -58,7 +58,6 @@ const layer = Layer.effect(
       finalize: (draft) =>
       finalize: (draft) =>
         Effect.gen(function* () {
         Effect.gen(function* () {
           materialized.clear()
           materialized.clear()
-          const seen = new Map<string, string | undefined>()
           for (const [name, source] of draft.list()) {
           for (const [name, source] of draft.list()) {
             if (source.type === "local") {
             if (source.type === "local") {
               materialized.set(
               materialized.set(
@@ -82,14 +81,11 @@ const layer = Layer.effect(
                 continue
                 continue
               }
               }
             }
             }
-            const target = Repository.cachePath(global.repos, repository)
-            if (seen.has(target) && seen.get(target) !== source.branch) continue
-            seen.set(target, source.branch)
             materialized.set(
             materialized.set(
               name,
               name,
               new Info({
               new Info({
                 name,
                 name,
-                path: AbsolutePath.make(target),
+                path: AbsolutePath.make(Repository.cachePath(global.repos, repository, source.branch)),
                 ...(source.description === undefined ? {} : { description: source.description }),
                 ...(source.description === undefined ? {} : { description: source.description }),
                 ...(source.hidden === undefined ? {} : { hidden: source.hidden }),
                 ...(source.hidden === undefined ? {} : { hidden: source.hidden }),
                 source,
                 source,

+ 36 - 42
packages/core/src/repository-cache.ts

@@ -1,3 +1,10 @@
+/**
+ * Local tracking checkouts for remote Git references, one per remote and
+ * branch. Each checkout permanently tracks a single ref: the requested branch
+ * when the cache key has one, otherwise the remote default branch. Content
+ * follows "newest wins": refresh fetches and hard-resets, so readers may
+ * observe the checkout move underneath them.
+ */
 import path from "path"
 import path from "path"
 import { Context, Effect, Layer, Schema } from "effect"
 import { Context, Effect, Layer, Schema } from "effect"
 import { FSUtil } from "./fs-util"
 import { FSUtil } from "./fs-util"
@@ -135,7 +142,7 @@ const layer: Layer.Layer<Service, never, FSUtil.Service | Git.Service | EffectFl
           if (input.branch) yield* validateBranch(input.branch)
           if (input.branch) yield* validateBranch(input.branch)
 
 
           const repository = input.reference.label
           const repository = input.reference.label
-          const localPath = Repository.cachePath(global.repos, input.reference)
+          const localPath = Repository.cachePath(global.repos, input.reference, input.branch)
           const cloneTarget = Repository.parse(input.reference.remote) ?? input.reference
           const cloneTarget = Repository.parse(input.reference.remote) ?? input.reference
 
 
           return yield* flock
           return yield* flock
@@ -143,21 +150,24 @@ const layer: Layer.Layer<Service, never, FSUtil.Service | Git.Service | EffectFl
               Effect.gen(function* () {
               Effect.gen(function* () {
                 yield* cacheOperation(fs.ensureDir(path.dirname(localPath)), "ensure cache directory", localPath)
                 yield* cacheOperation(fs.ensureDir(path.dirname(localPath)), "ensure cache directory", localPath)
 
 
-                const exists = yield* fs.existsSafe(localPath)
                 const existing = yield* git.repo.discover(AbsolutePath.make(localPath))
                 const existing = yield* git.repo.discover(AbsolutePath.make(localPath))
                 const origin = existing ? yield* git.remote.get(existing) : undefined
                 const origin = existing ? yield* git.remote.get(existing) : undefined
                 const originReference = origin ? Repository.parse(origin) : undefined
                 const originReference = origin ? Repository.parse(origin) : undefined
-                const reuse = Boolean(existing && originReference && Repository.same(originReference, cloneTarget))
-                if (exists && !reuse) {
+                // Discovery walks upward, so an enclosing repository with a
+                // matching origin could masquerade as the cache entry; reuse
+                // requires the checkout to live exactly at the cache path.
+                const worktree = existing ? yield* fs.resolve(localPath) : undefined
+                const reuse = Boolean(
+                  existing &&
+                    existing.worktree === worktree &&
+                    originReference &&
+                    Repository.same(originReference, cloneTarget),
+                )
+                if (!reuse && (yield* fs.existsSafe(localPath))) {
                   yield* cacheOperation(fs.remove(localPath, { recursive: true }), "remove stale cache", localPath)
                   yield* cacheOperation(fs.remove(localPath, { recursive: true }), "remove stale cache", localPath)
                 }
                 }
 
 
-                const currentBranch = reuse && existing ? yield* git.history.branch(existing) : undefined
-                const status = statusForRepository({
-                  reuse,
-                  refresh: input.refresh,
-                  branchMatches: input.branch ? currentBranch === input.branch : undefined,
-                })
+                const status = !reuse ? ("cloned" as const) : input.refresh ? ("refreshed" as const) : ("cached" as const)
 
 
                 if (status === "cloned") {
                 if (status === "cloned") {
                   yield* git.repo
                   yield* git.repo
@@ -177,25 +187,28 @@ const layer: Layer.Layer<Service, never, FSUtil.Service | Git.Service | EffectFl
                     .pipe(Effect.mapError((error) => new FetchFailedError({ repository, message: error.message })))
                     .pipe(Effect.mapError((error) => new FetchFailedError({ repository, message: error.message })))
 
 
                   if (input.branch) {
                   if (input.branch) {
-                    const requestedBranch = input.branch
                     yield* git.sync
                     yield* git.sync
-                      .fetchBranch(existing, { branch: requestedBranch })
+                      .fetchBranch(existing, { branch: input.branch })
                       .pipe(Effect.mapError((error) => new FetchFailedError({ repository, message: error.message })))
                       .pipe(Effect.mapError((error) => new FetchFailedError({ repository, message: error.message })))
+                  }
 
 
-                    yield* git.sync.checkoutRemoteBranch(existing, { branch: requestedBranch }).pipe(
-                      Effect.mapError(
-                        (error) =>
-                          new CheckoutFailedError({
-                            repository,
-                            branch: requestedBranch,
-                            message: error.message,
-                          }),
-                      ),
-                    )
+                  // Checking out the tracked ref before resetting keeps the
+                  // checkout self-healing even if it was left on another
+                  // branch.
+                  const branch = input.branch ?? (yield* git.history.defaultRemoteBranch(existing))
+                  if (branch) {
+                    yield* git.sync
+                      .checkoutRemoteBranch(existing, { branch })
+                      .pipe(
+                        Effect.mapError(
+                          (error) => new CheckoutFailedError({ repository, branch, message: error.message }),
+                        ),
+                      )
                   }
                   }
 
 
+                  const target = branch ?? (yield* git.history.branch(existing))
                   yield* git.sync
                   yield* git.sync
-                    .resetHard(existing, yield* resetTarget(git, existing, input.branch))
+                    .resetHard(existing, target ? `origin/${target}` : "HEAD")
                     .pipe(Effect.mapError((error) => new ResetFailedError({ repository, message: error.message })))
                     .pipe(Effect.mapError((error) => new ResetFailedError({ repository, message: error.message })))
                 }
                 }
 
 
@@ -229,12 +242,6 @@ export const node = makeGlobalNode({
   deps: [EffectFlock.node, FSUtil.node, Git.node, Global.node],
   deps: [EffectFlock.node, FSUtil.node, Git.node, Global.node],
 })
 })
 
 
-function statusForRepository(input: { reuse: boolean; refresh?: boolean; branchMatches?: boolean }) {
-  if (!input.reuse) return "cloned" as const
-  if (input.branchMatches === false || input.refresh) return "refreshed" as const
-  return "cached" as const
-}
-
 function errorMessage(error: unknown) {
 function errorMessage(error: unknown) {
   return error instanceof globalThis.Error ? error.message : String(error)
   return error instanceof globalThis.Error ? error.message : String(error)
 }
 }
@@ -245,17 +252,4 @@ function cacheOperation<A, E, R>(effect: Effect.Effect<A, E, R>, operation: stri
   )
   )
 }
 }
 
 
-const resetTarget = Effect.fnUntraced(function* (
-  git: Git.Interface,
-  repository: Git.Repository,
-  requestedBranch?: string,
-) {
-  if (requestedBranch) return `origin/${requestedBranch}`
-  const remoteHead = yield* git.history.defaultRemoteBranch(repository)
-  if (remoteHead) return `origin/${remoteHead}`
-  const currentBranch = yield* git.history.branch(repository)
-  if (currentBranch) return `origin/${currentBranch}`
-  return "HEAD"
-})
-
 export * as RepositoryCache from "./repository-cache"
 export * as RepositoryCache from "./repository-cache"

+ 8 - 2
packages/core/src/repository.ts

@@ -118,8 +118,14 @@ export function isRemote(reference: Reference): reference is RemoteReference {
   return !isFile(reference)
   return !isFile(reference)
 }
 }
 
 
-export function cachePath(root: string, reference: Reference): string {
-  return path.join(root, ...reference.host.split(":"), ...reference.segments)
+/**
+ * Checkouts are keyed by remote and branch: a branch-specific reference gets
+ * its own directory so branchless refreshes can never move it. The branch is
+ * percent-encoded because valid branch names may contain `/`.
+ */
+export function cachePath(root: string, reference: Reference, branch?: string): string {
+  const base = path.join(root, ...reference.host.split(":"), ...reference.segments)
+  return branch ? `${base}@${encodeURIComponent(branch)}` : base
 }
 }
 
 
 export function cacheIdentity(reference: Reference): string {
 export function cacheIdentity(reference: Reference): string {

+ 1 - 1
packages/core/test/reference.test.ts

@@ -47,7 +47,7 @@ describe("Reference", () => {
       expect(yield* references.list()).toEqual([
       expect(yield* references.list()).toEqual([
         new Reference.Info({
         new Reference.Info({
           name: "sdk",
           name: "sdk",
-          path: AbsolutePath.make(Repository.cachePath(Global.Path.repos, repository)),
+          path: AbsolutePath.make(Repository.cachePath(Global.Path.repos, repository, "main")),
           source,
           source,
         }),
         }),
       ])
       ])

+ 36 - 1
packages/core/test/repository-cache.test.ts

@@ -8,7 +8,7 @@ import { LayerNode } from "@opencode-ai/core/effect/layer-node"
 import { Global } from "@opencode-ai/core/global"
 import { Global } from "@opencode-ai/core/global"
 import { Repository } from "@opencode-ai/core/repository"
 import { Repository } from "@opencode-ai/core/repository"
 import { RepositoryCache } from "@opencode-ai/core/repository-cache"
 import { RepositoryCache } from "@opencode-ai/core/repository-cache"
-import { git, gitRemote } from "./fixture/git"
+import { branch, git, gitRemote } from "./fixture/git"
 import { tmpdir } from "./fixture/tmpdir"
 import { tmpdir } from "./fixture/tmpdir"
 import { testEffect } from "./lib/effect"
 import { testEffect } from "./lib/effect"
 
 
@@ -66,6 +66,41 @@ describe("RepositoryCache", () => {
     ),
     ),
   )
   )
 
 
+  it.live("keeps branch checkouts isolated from branchless refreshes", () =>
+    withRemote((fixture) =>
+      Effect.gen(function* () {
+        yield* Effect.promise(() => branch(fixture.source, "feature", "two\n"))
+        const cache = yield* RepositoryCache.Service
+
+        const featured = yield* cache.ensure({ reference: fixture.reference, branch: "feature" })
+        expect(featured.branch).toBe("feature")
+        expect(featured.localPath.endsWith("repo@feature")).toBe(true)
+        expect(yield* read(path.join(featured.localPath, "README.md"))).toBe("two\n")
+
+        const refreshed = yield* cache.ensure({ reference: fixture.reference, refresh: true })
+        expect(refreshed.localPath).not.toBe(featured.localPath)
+        expect(yield* read(path.join(refreshed.localPath, "README.md"))).toBe("one\n")
+
+        const cached = yield* cache.ensure({ reference: fixture.reference, branch: "feature" })
+        expect(cached.status).toBe("cached")
+        expect(yield* read(path.join(cached.localPath, "README.md"))).toBe("two\n")
+      }).pipe(Effect.provide(cacheLayer(fixture.root))),
+    ),
+  )
+
+  it.live("does not mistake an enclosing repository for the cache checkout", () =>
+    withRemote((fixture) =>
+      Effect.gen(function* () {
+        yield* Effect.promise(() => git(fixture.root, "clone", fixture.remote, path.join(fixture.root, "repos")))
+
+        const result = yield* (yield* RepositoryCache.Service).ensure({ reference: fixture.reference })
+
+        expect(result.status).toBe("cloned")
+        expect(yield* read(path.join(result.localPath, "README.md"))).toBe("one\n")
+      }).pipe(Effect.provide(cacheLayer(fixture.root))),
+    ),
+  )
+
   it.live("returns typed validation and clone failures", () =>
   it.live("returns typed validation and clone failures", () =>
     withRemote((fixture) =>
     withRemote((fixture) =>
       Effect.gen(function* () {
       Effect.gen(function* () {

+ 6 - 0
packages/core/test/repository.test.ts

@@ -17,6 +17,12 @@ describe("Repository", () => {
       label: "owner/repo",
       label: "owner/repo",
     })
     })
     expect(Repository.cachePath("/cache", reference)).toBe(path.join("/cache", "github.com", "owner", "repo"))
     expect(Repository.cachePath("/cache", reference)).toBe(path.join("/cache", "github.com", "owner", "repo"))
+    expect(Repository.cachePath("/cache", reference, "main")).toBe(
+      path.join("/cache", "github.com", "owner", "repo@main"),
+    )
+    expect(Repository.cachePath("/cache", reference, "feature/x")).toBe(
+      path.join("/cache", "github.com", "owner", "repo@feature%2Fx"),
+    )
     expect(Repository.cacheIdentity(reference)).toBe("github.com/owner/repo")
     expect(Repository.cacheIdentity(reference)).toBe("github.com/owner/repo")
   })
   })
 
 

+ 1 - 1
packages/opencode/test/server/httpapi-reference.test.ts

@@ -51,7 +51,7 @@ describe("reference HttpApi", () => {
       },
       },
       {
       {
         name: "effect",
         name: "effect",
-        path: path.join(Global.Path.repos, "github.com", "Effect-TS", "effect"),
+        path: path.join(Global.Path.repos, "github.com", "Effect-TS", "effect@main"),
         source: {
         source: {
           type: "git",
           type: "git",
           repository: "Effect-TS/effect",
           repository: "Effect-TS/effect",