From 730f46393fec4a71f2f2afe27353df2d3e17174b Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Sun, 15 Mar 2026 13:39:55 -0400 Subject: [PATCH 1/2] test(vcs): add tests for branch detection and HEAD event publishing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two tests pass (branch() reads current branch correctly). Two tests fail: BranchUpdated never fires because the FileWatcher event filter in vcs.ts has an inverted condition — it skips HEAD changes instead of filtering for them. --- packages/opencode/test/project/vcs.test.ts | 139 +++++++++++++++++++++ 1 file changed, 139 insertions(+) create mode 100644 packages/opencode/test/project/vcs.test.ts diff --git a/packages/opencode/test/project/vcs.test.ts b/packages/opencode/test/project/vcs.test.ts new file mode 100644 index 000000000000..17a1af27cace --- /dev/null +++ b/packages/opencode/test/project/vcs.test.ts @@ -0,0 +1,139 @@ +import { $ } from "bun" +import { afterEach, describe, expect, test } from "bun:test" +import fs from "fs/promises" +import path from "path" +import { ConfigProvider, Deferred, Effect, Fiber, Layer, ManagedRuntime, Option } from "effect" +import { tmpdir } from "../fixture/fixture" +import { FileWatcher, FileWatcherService } from "../../src/file/watcher" +import { InstanceContext } from "../../src/effect/instances" +import { Instance } from "../../src/project/instance" +import { GlobalBus } from "../../src/bus/global" +import { Vcs } from "../../src/project/vcs" + +// Skip in CI — native @parcel/watcher binding needed +const describeVcs = FileWatcher.hasNativeBinding() && !process.env.CI ? describe : describe.skip + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +const configLayer = ConfigProvider.layer( + ConfigProvider.fromUnknown({ + OPENCODE_EXPERIMENTAL_FILEWATCHER: "true", + OPENCODE_EXPERIMENTAL_DISABLE_FILEWATCHER: "false", + }), +) + +/** Boot watcher + vcs inside an Instance, return a disposable runtime */ +function withInstance(directory: string, body: () => Promise) { + return Instance.provide({ + directory, + fn: async () => { + const ctx = Layer.sync(InstanceContext, () => + InstanceContext.of({ directory: Instance.directory, project: Instance.project }), + ) + const layer = Layer.fresh(FileWatcherService.layer).pipe(Layer.provide(ctx), Layer.provide(configLayer)) + const rt = ManagedRuntime.make(layer) + try { + await rt.runPromise(FileWatcherService.use((s) => s.init())) + Vcs.init() + await Bun.sleep(200) + await body() + } finally { + await rt.dispose() + } + }, + }) +} + +type BranchEvent = { directory?: string; payload: { type: string; properties: { branch?: string } } } + +/** Wait for a Vcs.Event.BranchUpdated event on GlobalBus */ +function nextBranchUpdate(directory: string, timeout = 5000) { + return new Promise((resolve, reject) => { + const timer = setTimeout(() => { + GlobalBus.off("event", on) + reject(new Error("timed out waiting for BranchUpdated event")) + }, timeout) + + function on(evt: BranchEvent) { + if (evt.directory !== directory) return + if (evt.payload.type !== Vcs.Event.BranchUpdated.type) return + clearTimeout(timer) + GlobalBus.off("event", on) + resolve(evt.payload.properties.branch) + } + + GlobalBus.on("event", on) + }) +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +describeVcs("Vcs", () => { + afterEach(() => Instance.disposeAll()) + + test("branch() returns current branch name", async () => { + await using tmp = await tmpdir({ git: true }) + + await Instance.provide({ + directory: tmp.path, + fn: async () => { + await Vcs.init() + const branch = await Vcs.branch() + // tmpdir creates a git repo — branch should be "main" or "master" + expect(branch).toBeDefined() + expect(typeof branch).toBe("string") + }, + }) + }) + + test("branch() returns undefined for non-git directories", async () => { + await using tmp = await tmpdir() + + await Instance.provide({ + directory: tmp.path, + fn: async () => { + await Vcs.init() + const branch = await Vcs.branch() + expect(branch).toBeUndefined() + }, + }) + }) + + test("publishes BranchUpdated when .git/HEAD changes", async () => { + await using tmp = await tmpdir({ git: true }) + const branch = `test-${Math.random().toString(36).slice(2)}` + await $`git branch ${branch}`.cwd(tmp.path).quiet() + + await withInstance(tmp.path, async () => { + const pending = nextBranchUpdate(tmp.path) + + // Write .git/HEAD directly to simulate branch switch + const head = path.join(tmp.path, ".git", "HEAD") + await fs.writeFile(head, `ref: refs/heads/${branch}\n`) + + const updated = await pending + expect(updated).toBe(branch) + }) + }) + + test("branch() reflects the new branch after HEAD change", async () => { + await using tmp = await tmpdir({ git: true }) + const branch = `test-${Math.random().toString(36).slice(2)}` + await $`git branch ${branch}`.cwd(tmp.path).quiet() + + await withInstance(tmp.path, async () => { + const pending = nextBranchUpdate(tmp.path) + + const head = path.join(tmp.path, ".git", "HEAD") + await fs.writeFile(head, `ref: refs/heads/${branch}\n`) + + await pending + const current = await Vcs.branch() + expect(current).toBe(branch) + }) + }) +}) From bcc827e5313af131b0c4a8ed1a3d9cb430d562c5 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Sun, 15 Mar 2026 13:46:18 -0400 Subject: [PATCH 2/2] refactor(vcs): effectify VcsService as scoped service Convert Vcs from Instance.state namespace to an Effect ServiceMap.Service on the Instances LayerMap. Uses Instance.bind for the Bus.subscribe callback (same ALS pattern as FileWatcherService). Branch state is managed in the layer closure with scope-based cleanup. --- .../opencode/src/effect/instance-context.ts | 13 +++ packages/opencode/src/effect/instances.ts | 22 +++-- packages/opencode/src/file/watcher.ts | 2 +- packages/opencode/src/permission/service.ts | 2 +- packages/opencode/src/project/bootstrap.ts | 4 +- packages/opencode/src/project/vcs.ts | 86 ++++++++++--------- packages/opencode/src/server/server.ts | 5 +- packages/opencode/test/file/watcher.test.ts | 38 +++----- packages/opencode/test/fixture/instance.ts | 47 ++++++++++ packages/opencode/test/project/vcs.test.ts | 74 ++++++---------- 10 files changed, 162 insertions(+), 131 deletions(-) create mode 100644 packages/opencode/src/effect/instance-context.ts create mode 100644 packages/opencode/test/fixture/instance.ts diff --git a/packages/opencode/src/effect/instance-context.ts b/packages/opencode/src/effect/instance-context.ts new file mode 100644 index 000000000000..583b52d56217 --- /dev/null +++ b/packages/opencode/src/effect/instance-context.ts @@ -0,0 +1,13 @@ +import { ServiceMap } from "effect" +import type { Project } from "@/project/project" + +export declare namespace InstanceContext { + export interface Shape { + readonly directory: string + readonly project: Project.Info + } +} + +export class InstanceContext extends ServiceMap.Service()( + "opencode/InstanceContext", +) {} diff --git a/packages/opencode/src/effect/instances.ts b/packages/opencode/src/effect/instances.ts index d60d7935589a..2e6fbe167a37 100644 --- a/packages/opencode/src/effect/instances.ts +++ b/packages/opencode/src/effect/instances.ts @@ -1,24 +1,21 @@ import { Effect, Layer, LayerMap, ServiceMap } from "effect" import { registerDisposer } from "./instance-registry" +import { InstanceContext } from "./instance-context" import { ProviderAuthService } from "@/provider/auth-service" import { QuestionService } from "@/question/service" import { PermissionService } from "@/permission/service" import { FileWatcherService } from "@/file/watcher" +import { VcsService } from "@/project/vcs" import { Instance } from "@/project/instance" -import type { Project } from "@/project/project" -export declare namespace InstanceContext { - export interface Shape { - readonly directory: string - readonly project: Project.Info - } -} - -export class InstanceContext extends ServiceMap.Service()( - "opencode/InstanceContext", -) {} +export { InstanceContext } from "./instance-context" -export type InstanceServices = QuestionService | PermissionService | ProviderAuthService | FileWatcherService +export type InstanceServices = + | QuestionService + | PermissionService + | ProviderAuthService + | FileWatcherService + | VcsService function lookup(directory: string) { const project = Instance.project @@ -28,6 +25,7 @@ function lookup(directory: string) { Layer.fresh(PermissionService.layer), Layer.fresh(ProviderAuthService.layer), Layer.fresh(FileWatcherService.layer), + Layer.fresh(VcsService.layer), ).pipe(Layer.provide(ctx)) } diff --git a/packages/opencode/src/file/watcher.ts b/packages/opencode/src/file/watcher.ts index 651f15f8403d..16ee8f27c84e 100644 --- a/packages/opencode/src/file/watcher.ts +++ b/packages/opencode/src/file/watcher.ts @@ -1,6 +1,6 @@ import { BusEvent } from "@/bus/bus-event" import { Bus } from "@/bus" -import { InstanceContext } from "@/effect/instances" +import { InstanceContext } from "@/effect/instance-context" import { Instance } from "@/project/instance" import z from "zod" import { Log } from "../util/log" diff --git a/packages/opencode/src/permission/service.ts b/packages/opencode/src/permission/service.ts index b790158d163b..f20b19acf3ed 100644 --- a/packages/opencode/src/permission/service.ts +++ b/packages/opencode/src/permission/service.ts @@ -1,6 +1,6 @@ import { Bus } from "@/bus" import { BusEvent } from "@/bus/bus-event" -import { InstanceContext } from "@/effect/instances" +import { InstanceContext } from "@/effect/instance-context" import { ProjectID } from "@/project/schema" import { MessageID, SessionID } from "@/session/schema" import { PermissionTable } from "@/session/session.sql" diff --git a/packages/opencode/src/project/bootstrap.ts b/packages/opencode/src/project/bootstrap.ts index bd819dc280a1..da4a67dba706 100644 --- a/packages/opencode/src/project/bootstrap.ts +++ b/packages/opencode/src/project/bootstrap.ts @@ -7,7 +7,7 @@ import { Project } from "./project" import { Bus } from "../bus" import { Command } from "../command" import { Instance } from "./instance" -import { Vcs } from "./vcs" +import { VcsService } from "./vcs" import { Log } from "@/util/log" import { ShareNext } from "@/share/share-next" import { Snapshot } from "../snapshot" @@ -22,7 +22,7 @@ export async function InstanceBootstrap() { await LSP.init() await runPromiseInstance(FileWatcherService.use((service) => service.init())) File.init() - Vcs.init() + await runPromiseInstance(VcsService.use((s) => s.init())) Snapshot.init() Truncate.init() diff --git a/packages/opencode/src/project/vcs.ts b/packages/opencode/src/project/vcs.ts index 6eada6b675d4..4d1f7b766b18 100644 --- a/packages/opencode/src/project/vcs.ts +++ b/packages/opencode/src/project/vcs.ts @@ -1,11 +1,12 @@ import { BusEvent } from "@/bus/bus-event" import { Bus } from "@/bus" -import path from "path" import z from "zod" import { Log } from "@/util/log" import { Instance } from "./instance" +import { InstanceContext } from "@/effect/instance-context" import { FileWatcher } from "@/file/watcher" import { git } from "@/util/git" +import { Effect, Layer, ServiceMap } from "effect" const log = Log.create({ service: "vcs" }) @@ -27,50 +28,57 @@ export namespace Vcs { ref: "VcsInfo", }) export type Info = z.infer +} - async function currentBranch() { - const result = await git(["rev-parse", "--abbrev-ref", "HEAD"], { - cwd: Instance.worktree, - }) - if (result.exitCode !== 0) return - const text = result.text().trim() - if (!text) return - return text +export namespace VcsService { + export interface Service { + readonly init: () => Effect.Effect + readonly branch: () => Effect.Effect } +} - const state = Instance.state( - async () => { - if (Instance.project.vcs !== "git") { - return { branch: async () => undefined, unsubscribe: undefined } - } - let current = await currentBranch() - log.info("initialized", { branch: current }) +export class VcsService extends ServiceMap.Service()("@opencode/Vcs") { + static readonly layer = Layer.effect( + VcsService, + Effect.gen(function* () { + const instance = yield* InstanceContext + let current: string | undefined - const unsubscribe = Bus.subscribe(FileWatcher.Event.Updated, async (evt) => { - if (!evt.properties.file.endsWith("HEAD")) return - const next = await currentBranch() - if (next !== current) { - log.info("branch changed", { from: current, to: next }) - current = next - Bus.publish(Event.BranchUpdated, { branch: next }) + if (instance.project.vcs === "git") { + const currentBranch = async () => { + const result = await git(["rev-parse", "--abbrev-ref", "HEAD"], { + cwd: instance.project.worktree, + }) + if (result.exitCode !== 0) return undefined + const text = result.text().trim() + return text || undefined } - }) - return { - branch: async () => current, - unsubscribe, - } - }, - async (state) => { - state.unsubscribe?.() - }, - ) + current = yield* Effect.promise(() => currentBranch()) + log.info("initialized", { branch: current }) - export async function init() { - return state() - } + const unsubscribe = Bus.subscribe( + FileWatcher.Event.Updated, + Instance.bind(async (evt) => { + if (!evt.properties.file.endsWith("HEAD")) return + const next = await currentBranch() + if (next !== current) { + log.info("branch changed", { from: current, to: next }) + current = next + Bus.publish(Vcs.Event.BranchUpdated, { branch: next }) + } + }), + ) - export async function branch() { - return await state().then((s) => s.branch()) - } + yield* Effect.addFinalizer(() => Effect.sync(unsubscribe)) + } + + return VcsService.of({ + init: Effect.fn("VcsService.init")(function* () {}), + branch: Effect.fn("VcsService.branch")(function* () { + return current + }), + }) + }), + ) } diff --git a/packages/opencode/src/server/server.ts b/packages/opencode/src/server/server.ts index 55bcf2dfce16..677af4da87f0 100644 --- a/packages/opencode/src/server/server.ts +++ b/packages/opencode/src/server/server.ts @@ -14,7 +14,8 @@ import { LSP } from "../lsp" import { Format } from "../format" import { TuiRoutes } from "./routes/tui" import { Instance } from "../project/instance" -import { Vcs } from "../project/vcs" +import { Vcs, VcsService } from "../project/vcs" +import { runPromiseInstance } from "@/effect/runtime" import { Agent } from "../agent/agent" import { Skill } from "../skill/skill" import { Auth } from "../auth" @@ -330,7 +331,7 @@ export namespace Server { }, }), async (c) => { - const branch = await Vcs.branch() + const branch = await runPromiseInstance(VcsService.use((s) => s.branch())) return c.json({ branch, }) diff --git a/packages/opencode/test/file/watcher.test.ts b/packages/opencode/test/file/watcher.test.ts index 7fe53612d9fb..a2de6173346a 100644 --- a/packages/opencode/test/file/watcher.test.ts +++ b/packages/opencode/test/file/watcher.test.ts @@ -2,10 +2,10 @@ import { $ } from "bun" import { afterEach, describe, expect, test } from "bun:test" import fs from "fs/promises" import path from "path" -import { ConfigProvider, Deferred, Effect, Fiber, Layer, ManagedRuntime, Option } from "effect" +import { Deferred, Effect, Fiber, Option } from "effect" import { tmpdir } from "../fixture/fixture" +import { watcherConfigLayer, withServices } from "../fixture/instance" import { FileWatcher, FileWatcherService } from "../../src/file/watcher" -import { InstanceContext } from "../../src/effect/instances" import { Instance } from "../../src/project/instance" import { GlobalBus } from "../../src/bus/global" @@ -16,35 +16,21 @@ const describeWatcher = FileWatcher.hasNativeBinding() && !process.env.CI ? desc // Helpers // --------------------------------------------------------------------------- -const configLayer = ConfigProvider.layer( - ConfigProvider.fromUnknown({ - OPENCODE_EXPERIMENTAL_FILEWATCHER: "true", - OPENCODE_EXPERIMENTAL_DISABLE_FILEWATCHER: "false", - }), -) - type BusUpdate = { directory?: string; payload: { type: string; properties: WatcherEvent } } type WatcherEvent = { file: string; event: "add" | "change" | "unlink" } -/** Run `body` with a live FileWatcherService. Runtime is acquired/released via Effect.scoped. */ +/** Run `body` with a live FileWatcherService. */ function withWatcher(directory: string, body: Effect.Effect) { - return Instance.provide({ + return withServices( directory, - fn: () => - Effect.gen(function* () { - const ctx = Layer.sync(InstanceContext, () => - InstanceContext.of({ directory: Instance.directory, project: Instance.project }), - ) - const layer = Layer.fresh(FileWatcherService.layer).pipe(Layer.provide(ctx), Layer.provide(configLayer)) - const rt = yield* Effect.acquireRelease( - Effect.sync(() => ManagedRuntime.make(layer)), - (rt) => Effect.promise(() => rt.dispose()), - ) - yield* Effect.promise(() => rt.runPromise(FileWatcherService.use((s) => s.init()))) - yield* ready(directory) - yield* body - }).pipe(Effect.scoped, Effect.runPromise), - }) + FileWatcherService.layer, + async (rt) => { + await rt.runPromise(FileWatcherService.use((s) => s.init())) + await Effect.runPromise(ready(directory)) + await Effect.runPromise(body) + }, + { provide: [watcherConfigLayer] }, + ) } function listen(directory: string, check: (evt: WatcherEvent) => boolean, hit: (evt: WatcherEvent) => void) { diff --git a/packages/opencode/test/fixture/instance.ts b/packages/opencode/test/fixture/instance.ts new file mode 100644 index 000000000000..d322e1d9fbea --- /dev/null +++ b/packages/opencode/test/fixture/instance.ts @@ -0,0 +1,47 @@ +import { ConfigProvider, Layer, ManagedRuntime } from "effect" +import { InstanceContext } from "../../src/effect/instance-context" +import { Instance } from "../../src/project/instance" + +/** ConfigProvider that enables the experimental file watcher. */ +export const watcherConfigLayer = ConfigProvider.layer( + ConfigProvider.fromUnknown({ + OPENCODE_EXPERIMENTAL_FILEWATCHER: "true", + OPENCODE_EXPERIMENTAL_DISABLE_FILEWATCHER: "false", + }), +) + +/** + * Boot an Instance with the given service layers and run `body` with + * the ManagedRuntime. Cleanup is automatic — the runtime is disposed + * and Instance context is torn down when `body` completes. + * + * Layers may depend on InstanceContext (provided automatically). + * Pass extra layers via `options.provide` (e.g. ConfigProvider.layer). + */ +export function withServices( + directory: string, + layer: Layer.Layer, + body: (rt: ManagedRuntime.ManagedRuntime) => Promise, + options?: { provide?: Layer.Layer[] }, +) { + return Instance.provide({ + directory, + fn: async () => { + const ctx = Layer.sync(InstanceContext, () => + InstanceContext.of({ directory: Instance.directory, project: Instance.project }), + ) + let resolved: Layer.Layer = Layer.fresh(layer).pipe(Layer.provide(ctx)) as any + if (options?.provide) { + for (const l of options.provide) { + resolved = resolved.pipe(Layer.provide(l)) as any + } + } + const rt = ManagedRuntime.make(resolved) + try { + await body(rt) + } finally { + await rt.dispose() + } + }, + }) +} diff --git a/packages/opencode/test/project/vcs.test.ts b/packages/opencode/test/project/vcs.test.ts index 17a1af27cace..b5100585f5f0 100644 --- a/packages/opencode/test/project/vcs.test.ts +++ b/packages/opencode/test/project/vcs.test.ts @@ -2,13 +2,13 @@ import { $ } from "bun" import { afterEach, describe, expect, test } from "bun:test" import fs from "fs/promises" import path from "path" -import { ConfigProvider, Deferred, Effect, Fiber, Layer, ManagedRuntime, Option } from "effect" +import { Layer, ManagedRuntime } from "effect" import { tmpdir } from "../fixture/fixture" +import { watcherConfigLayer, withServices } from "../fixture/instance" import { FileWatcher, FileWatcherService } from "../../src/file/watcher" -import { InstanceContext } from "../../src/effect/instances" import { Instance } from "../../src/project/instance" import { GlobalBus } from "../../src/bus/global" -import { Vcs } from "../../src/project/vcs" +import { Vcs, VcsService } from "../../src/project/vcs" // Skip in CI — native @parcel/watcher binding needed const describeVcs = FileWatcher.hasNativeBinding() && !process.env.CI ? describe : describe.skip @@ -17,33 +17,21 @@ const describeVcs = FileWatcher.hasNativeBinding() && !process.env.CI ? describe // Helpers // --------------------------------------------------------------------------- -const configLayer = ConfigProvider.layer( - ConfigProvider.fromUnknown({ - OPENCODE_EXPERIMENTAL_FILEWATCHER: "true", - OPENCODE_EXPERIMENTAL_DISABLE_FILEWATCHER: "false", - }), -) - -/** Boot watcher + vcs inside an Instance, return a disposable runtime */ -function withInstance(directory: string, body: () => Promise) { - return Instance.provide({ +function withVcs( + directory: string, + body: (rt: ManagedRuntime.ManagedRuntime) => Promise, +) { + return withServices( directory, - fn: async () => { - const ctx = Layer.sync(InstanceContext, () => - InstanceContext.of({ directory: Instance.directory, project: Instance.project }), - ) - const layer = Layer.fresh(FileWatcherService.layer).pipe(Layer.provide(ctx), Layer.provide(configLayer)) - const rt = ManagedRuntime.make(layer) - try { - await rt.runPromise(FileWatcherService.use((s) => s.init())) - Vcs.init() - await Bun.sleep(200) - await body() - } finally { - await rt.dispose() - } + Layer.merge(FileWatcherService.layer, VcsService.layer), + async (rt) => { + await rt.runPromise(FileWatcherService.use((s) => s.init())) + await rt.runPromise(VcsService.use((s) => s.init())) + await Bun.sleep(200) + await body(rt) }, - }) + { provide: [watcherConfigLayer] }, + ) } type BranchEvent = { directory?: string; payload: { type: string; properties: { branch?: string } } } @@ -78,28 +66,19 @@ describeVcs("Vcs", () => { test("branch() returns current branch name", async () => { await using tmp = await tmpdir({ git: true }) - await Instance.provide({ - directory: tmp.path, - fn: async () => { - await Vcs.init() - const branch = await Vcs.branch() - // tmpdir creates a git repo — branch should be "main" or "master" - expect(branch).toBeDefined() - expect(typeof branch).toBe("string") - }, + await withVcs(tmp.path, async (rt) => { + const branch = await rt.runPromise(VcsService.use((s) => s.branch())) + expect(branch).toBeDefined() + expect(typeof branch).toBe("string") }) }) test("branch() returns undefined for non-git directories", async () => { await using tmp = await tmpdir() - await Instance.provide({ - directory: tmp.path, - fn: async () => { - await Vcs.init() - const branch = await Vcs.branch() - expect(branch).toBeUndefined() - }, + await withVcs(tmp.path, async (rt) => { + const branch = await rt.runPromise(VcsService.use((s) => s.branch())) + expect(branch).toBeUndefined() }) }) @@ -108,10 +87,9 @@ describeVcs("Vcs", () => { const branch = `test-${Math.random().toString(36).slice(2)}` await $`git branch ${branch}`.cwd(tmp.path).quiet() - await withInstance(tmp.path, async () => { + await withVcs(tmp.path, async () => { const pending = nextBranchUpdate(tmp.path) - // Write .git/HEAD directly to simulate branch switch const head = path.join(tmp.path, ".git", "HEAD") await fs.writeFile(head, `ref: refs/heads/${branch}\n`) @@ -125,14 +103,14 @@ describeVcs("Vcs", () => { const branch = `test-${Math.random().toString(36).slice(2)}` await $`git branch ${branch}`.cwd(tmp.path).quiet() - await withInstance(tmp.path, async () => { + await withVcs(tmp.path, async (rt) => { const pending = nextBranchUpdate(tmp.path) const head = path.join(tmp.path, ".git", "HEAD") await fs.writeFile(head, `ref: refs/heads/${branch}\n`) await pending - const current = await Vcs.branch() + const current = await rt.runPromise(VcsService.use((s) => s.branch())) expect(current).toBe(branch) }) })