From 51e453cfe03dfe5c54b111c9196f6acea000b6f6 Mon Sep 17 00:00:00 2001 From: Lex Date: Fri, 14 Aug 2026 12:09:05 +0800 Subject: [PATCH 1/2] fix(core): fail closed on git transport errors --- packages/core/src/git.ts | 3 +- packages/core/test/project.test.ts | 65 +++++++++++++++++++++++++++++- 2 files changed, 65 insertions(+), 3 deletions(-) diff --git a/packages/core/src/git.ts b/packages/core/src/git.ts index 86708d5f9a..b757641ae4 100644 --- a/packages/core/src/git.ts +++ b/packages/core/src/git.ts @@ -953,8 +953,7 @@ interface Result { } function run(cwd: string, proc: AppProcess.Interface) { - return (args: string[]) => - execute(cwd, proc)(args).pipe(Effect.catch(() => Effect.succeed({ exitCode: 1, text: "", stderr: "" }))) + return (args: string[]) => execute(cwd, proc)(args).pipe(Effect.orDie) } function execute(cwd: string, proc: AppProcess.Interface) { diff --git a/packages/core/test/project.test.ts b/packages/core/test/project.test.ts index 8c52f4a98a..0c175eba7a 100644 --- a/packages/core/test/project.test.ts +++ b/packages/core/test/project.test.ts @@ -2,11 +2,13 @@ import { describe, expect } from "bun:test" import { $ } from "bun" import fs from "fs/promises" import path from "path" -import { Effect, Layer, Schema } from "effect" +import { Cause, Effect, Exit, Layer, Schema } from "effect" +import { ChildProcess } from "effect/unstable/process" import { ProjectV2 } from "@opencode-ai/core/project" import { Database } from "@opencode-ai/core/database/database" import { FSUtil } from "@opencode-ai/core/fs-util" import { Git } from "@opencode-ai/core/git" +import { AppProcess } from "@opencode-ai/core/process" import { AbsolutePath } from "@opencode-ai/core/schema" import { Hash } from "@opencode-ai/core/util/hash" import { ProjectDirectories } from "@opencode-ai/core/project/directories" @@ -14,6 +16,34 @@ import { tmpdir } from "./fixture/tmpdir" import { testEffect } from "./lib/effect" const it = testEffect(Layer.mergeAll(ProjectV2.defaultLayer, Database.defaultLayer, ProjectDirectories.defaultLayer)) +const unavailableProject = projectWithProcessFailure(() => true) +const unavailableRootCommits = projectWithProcessFailure( + (command) => command._tag === "StandardCommand" && command.command === "git" && command.args[0] === "rev-list", +) + +function projectWithProcessFailure(matches: (command: ChildProcess.Command) => boolean) { + const unavailableProcess = Layer.effect( + AppProcess.Service, + Effect.gen(function* () { + const process = yield* AppProcess.Service + return AppProcess.Service.of({ + ...process, + run: (command, options) => + matches(command) + ? Effect.fail(new AppProcess.AppProcessError({ command: "git" })) + : process.run(command, options), + }) + }), + ).pipe(Layer.provide(AppProcess.defaultLayer)) + const git = Git.layer.pipe(Layer.provide(FSUtil.defaultLayer), Layer.provide(unavailableProcess)) + return testEffect( + ProjectV2.layer.pipe( + Layer.provide(FSUtil.defaultLayer), + Layer.provide(git), + Layer.provide(ProjectDirectories.defaultLayer), + ), + ) +} function remoteID(remote: string) { return ProjectV2.ID.make(Hash.fast(`git-remote:${remote}`)) @@ -42,6 +72,39 @@ async function rootCommit(dir: string) { } describe("ProjectV2.resolve", () => { + unavailableProject.effect("fails closed instead of returning global when git cannot start", () => + Effect.gen(function* () { + const tmp = yield* Effect.acquireRelease( + Effect.promise(() => tmpdir()), + (tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()), + ) + yield* Effect.promise(() => fs.mkdir(path.join(tmp.path, ".git"))) + const project = yield* ProjectV2.Service + + const exit = yield* project.resolve(abs(tmp.path)).pipe(Effect.exit) + + expect(Exit.isFailure(exit)).toBe(true) + expect(Exit.isSuccess(exit) ? exit.value.id : undefined).not.toBe(ProjectV2.ID.global) + }), + ) + + unavailableRootCommits.live("fails closed when root commit lookup cannot start", () => + Effect.gen(function* () { + const tmp = yield* Effect.acquireRelease( + Effect.promise(() => tmpdir()), + (tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()), + ) + yield* Effect.promise(() => initRepo(tmp.path, { commit: true })) + const project = yield* ProjectV2.Service + + const exit = yield* project.resolve(abs(tmp.path)).pipe(Effect.exit) + + expect(Exit.isFailure(exit)).toBe(true) + if (Exit.isFailure(exit)) expect(Cause.squash(exit.cause)).toBeInstanceOf(AppProcess.AppProcessError) + expect(Exit.isSuccess(exit) ? exit.value.id : undefined).not.toBe(ProjectV2.ID.global) + }), + ) + it.live("returns global for non-git directory", () => Effect.gen(function* () { const tmp = yield* Effect.acquireRelease( From 64c3a5e04b1d169aaefa9eea71ac73ac4b908e54 Mon Sep 17 00:00:00 2001 From: Lex Date: Fri, 14 Aug 2026 12:09:52 +0800 Subject: [PATCH 2/2] fix(memory): expose global config decisions --- packages/opencode/src/memory/config.ts | 10 +++- packages/opencode/src/memory/memory.ts | 11 ++++- .../memory/memory-global-identity.test.ts | 8 ++-- packages/opencode/test/memory/memory.test.ts | 47 +++++++++++++++++++ 4 files changed, 70 insertions(+), 6 deletions(-) diff --git a/packages/opencode/src/memory/config.ts b/packages/opencode/src/memory/config.ts index 4d1b641edf..db8162af5e 100644 --- a/packages/opencode/src/memory/config.ts +++ b/packages/opencode/src/memory/config.ts @@ -122,7 +122,15 @@ export const layer = Layer.effect( const file = join(globalConfigDir(), "memory.jsonc") const found = yield* readFirst(globalCandidates()) if (found) { - if (yield* readConfig(found)) return false + const existing = yield* readConfig(found) + if (existing) { + yield* Effect.logWarning("global MEMORY config write declined — preserving existing valid config", { + path: found.path, + existingModel: existing.model, + requestedModel: config.model, + }) + return false + } yield* flock.withLock(MemoryFile.atomicWrite(fs, found.path, serialize(config)), writeLockKey(found.path)) return true } diff --git a/packages/opencode/src/memory/memory.ts b/packages/opencode/src/memory/memory.ts index a9a3fe0b3f..6aa9e9b12e 100644 --- a/packages/opencode/src/memory/memory.ts +++ b/packages/opencode/src/memory/memory.ts @@ -158,7 +158,16 @@ export const layer: Layer.Layer< : yield* selectConfiguration(yield* availableModels(), undefined, conversationModel) if (existing?.config.model === config.model) return const created = yield* configStore.writeGlobal(config, existing?.path) - if (created) yield* Effect.logInfo("global MEMORY config initialized", { model: config.model }) + if (!created) return + if (!existing) { + yield* Effect.logInfo("global MEMORY config initialized", { model: config.model }) + return + } + yield* Effect.logInfo("global MEMORY model replaced", { + path: existing.path, + previousModel: existing.config.model, + model: config.model, + }) }) const initUnsafe = Effect.fn("Memory.initUnsafe")(function* (conversationModel?: string) { diff --git a/packages/opencode/test/memory/memory-global-identity.test.ts b/packages/opencode/test/memory/memory-global-identity.test.ts index ad94ce4a24..687b7a416f 100644 --- a/packages/opencode/test/memory/memory-global-identity.test.ts +++ b/packages/opencode/test/memory/memory-global-identity.test.ts @@ -35,8 +35,8 @@ import { testEffect } from "../lib/effect" // bun test runs all files in one process, sequentially, sharing one // XDG_CONFIG_HOME — so a test file that runs before this one and triggers // global-memory initialization leaves a VALID memory.jsonc whose model this -// file's fake provider does not know; writeGlobal then silently no-ops over -// it and search fails closed with "unavailable" (dev CI, deterministic). +// file's fake provider does not know; writeGlobal then preserves it with a +// warning and search fails closed with "unavailable" (dev CI, deterministic). // Pin a private config dir per file so the global file can never be // contaminated by earlier files. const pinnedConfigDir = path.join(os.tmpdir(), `opencode-memory-global-identity-${process.pid}`) @@ -185,8 +185,8 @@ describe("MEM-PR01-R1-03: memory is inert once the identity row is retired", () yield* project.setInitialized(info.id) yield* configStore.writeGlobal(baseConfig) - // Tripwire: writeGlobal silently no-ops over a pre-existing VALID - // config, so a contaminated global dir would leave a foreign model + // Tripwire: writeGlobal preserves a pre-existing VALID config with + // a warning, so a contaminated global dir would leave a foreign model // here and every search would fail closed. expect((yield* configStore.loadGlobal())?.config.model).toBe("test/memory-on") diff --git a/packages/opencode/test/memory/memory.test.ts b/packages/opencode/test/memory/memory.test.ts index eaf2185b04..730889688b 100644 --- a/packages/opencode/test/memory/memory.test.ts +++ b/packages/opencode/test/memory/memory.test.ts @@ -3,6 +3,7 @@ import { SessionV1 } from "@opencode-ai/core/v1/session" import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" import { EffectFlock } from "@opencode-ai/core/util/effect-flock" import { Deferred, Duration, Effect, Fiber, Layer } from "effect" +import { logLines } from "effect/testing/TestConsole" import fs from "node:fs/promises" import path from "node:path" import { Config } from "@/config/config" @@ -502,6 +503,42 @@ describe("memory config and YAML store", () => { ) }), ) + + it.live("warns when preserving an existing valid global configuration", () => + Effect.gen(function* () { + const memoryConfig = yield* MemoryConfig.Service + const global = yield* tmpdirScoped() + const previous = process.env.OPENCODE_CONFIG_DIR + const existing = { ...config, model: "test/existing" } + const requested = { ...config, model: "test/requested" } + + yield* Effect.acquireUseRelease( + Effect.sync(() => { + process.env.OPENCODE_CONFIG_DIR = global + }), + () => + Effect.gen(function* () { + expect(yield* memoryConfig.writeGlobal(existing)).toBe(true) + expect(yield* memoryConfig.writeGlobal(requested)).toBe(false) + expect((yield* memoryConfig.loadGlobal())?.config).toEqual(existing) + + const logs = JSON.stringify(yield* logLines) + expect(logs).toContain("WARN") + expect(logs).toContain("global MEMORY config write declined — preserving existing valid config") + expect(logs).toContain(path.join(global, "memory.jsonc")) + expect(logs).toContain("existingModel") + expect(logs).toContain("test/existing") + expect(logs).toContain("requestedModel") + expect(logs).toContain("test/requested") + }), + () => + Effect.sync(() => { + if (previous === undefined) delete process.env.OPENCODE_CONFIG_DIR + else process.env.OPENCODE_CONFIG_DIR = previous + }), + ) + }), + ) }) describe("memory controller policy", () => { @@ -1360,6 +1397,9 @@ describe("memory bootstrap", () => { injection: { max_topics: 3, max_tokens: 1_200 }, }) expect(bootstrap.state.modelCalls).toBe(0) + const logs = JSON.stringify(yield* logLines) + expect(logs).toContain("global MEMORY config initialized") + expect(logs).not.toContain("global MEMORY model replaced") }), { git: true }, ) @@ -1449,6 +1489,13 @@ describe("memory bootstrap", () => { turn_interval: 7, }) expect(bootstrap.state.modelCalls).toBe(0) + const logs = JSON.stringify(yield* logLines) + expect(logs).toContain("global MEMORY model replaced") + expect(logs).toContain("previousModel") + expect(logs).toContain("removed/model") + expect(logs).toContain("test/compaction") + expect(logs).toContain("/global/memory.jsonc") + expect(logs).not.toContain("global MEMORY config initialized") }), { git: true }, )