From 2df8eda8a3baf8c624527995ae1adb4dc19a1071 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Sun, 3 May 2026 00:24:33 -0400 Subject: [PATCH] fix(cli): bridge Instance.current ALS in effectCmd handlers (regression from #25522) (#25546) --- packages/opencode/src/cli/effect-cmd.ts | 27 ++++++----- .../test/cli/effect-cmd-instance-als.test.ts | 48 +++++++++++++++++++ 2 files changed, 64 insertions(+), 11 deletions(-) create mode 100644 packages/opencode/test/cli/effect-cmd-instance-als.test.ts diff --git a/packages/opencode/src/cli/effect-cmd.ts b/packages/opencode/src/cli/effect-cmd.ts index b0f6de16b7..ada5f8677d 100644 --- a/packages/opencode/src/cli/effect-cmd.ts +++ b/packages/opencode/src/cli/effect-cmd.ts @@ -3,6 +3,7 @@ import { Effect, Schema } from "effect" import { AppRuntime, type AppServices } from "@/effect/app-runtime" import { InstanceStore } from "@/project/instance-store" import { InstanceRef } from "@/effect/instance-ref" +import { Instance } from "@/project/instance" import { cmd, type WithDoubleDash } from "./cmd/cmd" /** @@ -82,17 +83,21 @@ export const effectCmd = (opts: EffectCmdOpts) => return } const directory = opts.directory?.(args) ?? process.cwd() - await AppRuntime.runPromise( - InstanceStore.Service.use((store) => - store.provide( - { directory }, - Effect.gen(function* () { - const ctx = yield* InstanceRef - const body = opts.handler(args) - return ctx ? yield* body.pipe(Effect.ensuring(store.dispose(ctx))) : yield* body - }), - ), - ), + // Two-phase: load ctx, then run body inside Instance.current ALS. + // Effect's InstanceRef is provided via fiber context, but that context is + // lost across `await` inside `Effect.promise(async () => ...)` callbacks + // — when handlers re-enter Effect via `AppRuntime.runPromise(svc.method())` + // there, attach() falls back to Instance.current ALS, which Node preserves + // across awaits. Matches the pre-effectCmd `bootstrap()` behavior. + const { store, ctx } = await AppRuntime.runPromise( + InstanceStore.Service.use((store) => store.load({ directory }).pipe(Effect.map((ctx) => ({ store, ctx })))), ) + try { + await Instance.restore(ctx, () => + AppRuntime.runPromise(opts.handler(args).pipe(Effect.provideService(InstanceRef, ctx))), + ) + } finally { + await AppRuntime.runPromise(store.dispose(ctx)) + } }, }) diff --git a/packages/opencode/test/cli/effect-cmd-instance-als.test.ts b/packages/opencode/test/cli/effect-cmd-instance-als.test.ts new file mode 100644 index 0000000000..de6fed8daa --- /dev/null +++ b/packages/opencode/test/cli/effect-cmd-instance-als.test.ts @@ -0,0 +1,48 @@ +import { afterEach, expect, test } from "bun:test" +import { Effect } from "effect" +import fs from "fs/promises" +import { Instance } from "../../src/project/instance" +import { disposeAllInstances, provideTestInstance, tmpdir } from "../fixture/fixture" + +afterEach(async () => { + await disposeAllInstances() +}) + +// Regression for PR #25522: when an effectCmd handler does +// `yield* Effect.promise(async () => { ... await runPromise(svcMethod) ... })`, +// the inner runPromise creates a fresh fiber after `await` whose Effect context +// has lost the outer InstanceRef. Services that read `InstanceState.context` +// then fall back to `Instance.current` ALS, which must be installed at the JS +// callback boundary (Node ALS persists across awaits, Effect's fiber context +// does not). `provideTestInstance` mirrors effectCmd's load + ALS-restore wrap. +// Pins effect-cmd.ts directly: the pattern test below exercises the load + +// Instance.restore + dispose triple via the shared `provideTestInstance` fixture, +// so a regression that removed `Instance.restore` from effect-cmd.ts wouldn't +// fail it. This grep guards the actual production callsite. +test("effect-cmd.ts wraps the handler body in Instance.restore", async () => { + const source = await fs.readFile(new URL("../../src/cli/effect-cmd.ts", import.meta.url), "utf8") + expect(source).toContain("Instance.restore(ctx") +}) + +test("Instance.current reachable from inner runPromise inside Effect.promise(async)", async () => { + await using dir = await tmpdir({ git: true }) + await provideTestInstance({ + directory: dir.path, + fn: () => + Effect.runPromise( + Effect.promise(async () => { + await new Promise((r) => setTimeout(r, 5)) + const current = await Effect.runPromise( + Effect.sync(() => { + try { + return Instance.current + } catch { + return undefined + } + }), + ) + expect(current?.directory).toBe(dir.path) + }), + ), + }) +})