From 065dc274ec46a9995c04d8908293d3502bcff67e Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Fri, 24 Jul 2026 21:23:44 -0400 Subject: [PATCH] fix(core): branch-keyed repository cache with gated reference readiness (#38759) --- packages/core/src/reference.ts | 6 +- packages/core/src/repository-cache.ts | 80 +++++++++---------- packages/core/src/repository.ts | 10 ++- packages/core/test/reference.test.ts | 2 +- packages/core/test/repository-cache.test.ts | 37 ++++++++- packages/core/test/repository.test.ts | 6 ++ .../test/server/httpapi-reference.test.ts | 2 +- 7 files changed, 90 insertions(+), 53 deletions(-) diff --git a/packages/core/src/reference.ts b/packages/core/src/reference.ts index 5303dbd955c..1e1ab9d20f7 100644 --- a/packages/core/src/reference.ts +++ b/packages/core/src/reference.ts @@ -58,7 +58,6 @@ const layer = Layer.effect( finalize: (draft) => Effect.gen(function* () { materialized.clear() - const seen = new Map() for (const [name, source] of draft.list()) { if (source.type === "local") { materialized.set( @@ -82,14 +81,11 @@ const layer = Layer.effect( 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( name, new Info({ name, - path: AbsolutePath.make(target), + path: AbsolutePath.make(Repository.cachePath(global.repos, repository, source.branch)), ...(source.description === undefined ? {} : { description: source.description }), ...(source.hidden === undefined ? {} : { hidden: source.hidden }), source, diff --git a/packages/core/src/repository-cache.ts b/packages/core/src/repository-cache.ts index 988236743b3..f166219125f 100644 --- a/packages/core/src/repository-cache.ts +++ b/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 { Context, Effect, Layer, Schema } from "effect" import { FSUtil } from "./fs-util" @@ -135,7 +142,7 @@ const layer: Layer.Layer new FetchFailedError({ repository, message: error.message }))) if (input.branch) { - const requestedBranch = input.branch yield* git.sync - .fetchBranch(existing, { branch: requestedBranch }) + .fetchBranch(existing, { branch: input.branch }) .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 - .resetHard(existing, yield* resetTarget(git, existing, input.branch)) + .resetHard(existing, target ? `origin/${target}` : "HEAD") .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], }) -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) { return error instanceof globalThis.Error ? error.message : String(error) } @@ -245,17 +252,4 @@ function cacheOperation(effect: Effect.Effect, 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" diff --git a/packages/core/src/repository.ts b/packages/core/src/repository.ts index dbc6a8fbcae..8ee5be600e3 100644 --- a/packages/core/src/repository.ts +++ b/packages/core/src/repository.ts @@ -118,8 +118,14 @@ export function isRemote(reference: Reference): reference is RemoteReference { 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 { diff --git a/packages/core/test/reference.test.ts b/packages/core/test/reference.test.ts index db61232310e..5ab215230ac 100644 --- a/packages/core/test/reference.test.ts +++ b/packages/core/test/reference.test.ts @@ -47,7 +47,7 @@ describe("Reference", () => { expect(yield* references.list()).toEqual([ new Reference.Info({ name: "sdk", - path: AbsolutePath.make(Repository.cachePath(Global.Path.repos, repository)), + path: AbsolutePath.make(Repository.cachePath(Global.Path.repos, repository, "main")), source, }), ]) diff --git a/packages/core/test/repository-cache.test.ts b/packages/core/test/repository-cache.test.ts index 2dd0ce25028..4433583bf78 100644 --- a/packages/core/test/repository-cache.test.ts +++ b/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 { Repository } from "@opencode-ai/core/repository" 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 { 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", () => withRemote((fixture) => Effect.gen(function* () { diff --git a/packages/core/test/repository.test.ts b/packages/core/test/repository.test.ts index 5b18f8b1d69..1a6af675d12 100644 --- a/packages/core/test/repository.test.ts +++ b/packages/core/test/repository.test.ts @@ -17,6 +17,12 @@ describe("Repository", () => { label: "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") }) diff --git a/packages/opencode/test/server/httpapi-reference.test.ts b/packages/opencode/test/server/httpapi-reference.test.ts index 9354d527da0..b187bd4eea2 100644 --- a/packages/opencode/test/server/httpapi-reference.test.ts +++ b/packages/opencode/test/server/httpapi-reference.test.ts @@ -51,7 +51,7 @@ describe("reference HttpApi", () => { }, { 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: { type: "git", repository: "Effect-TS/effect",