From f96a88f13816408900a3fa63a2f0638afbcd5b04 Mon Sep 17 00:00:00 2001 From: Keith Hughitt Date: Mon, 14 Sep 2026 09:56:23 -0400 Subject: [PATCH 1/3] fix(tui): skip instance disposal on SIGUSR2 reload when config is unchanged The TUI forwards SIGUSR2 to the worker's reload RPC, which invalidated the global config cache and unconditionally disposed every instance, aborting all in-flight sessions. Theme switchers (Omarchy, Noctalia hooks, ...) signal on every wallpaper change, so a periodic rotation kills active agent work every time. Themes are discovered from themes/*.json and are not part of the global config, so a theme-only signal never needs an instance rebuild. Reload now re-reads the global config and disposes instances only when it actually changed; the configUpdate route behavior is unaffected. --- packages/opencode/src/cli/tui/worker.ts | 12 +-- .../opencode/src/server/global-lifecycle.ts | 16 ++++ .../test/server/global-lifecycle.test.ts | 82 +++++++++++++++++++ 3 files changed, 100 insertions(+), 10 deletions(-) create mode 100644 packages/opencode/test/server/global-lifecycle.test.ts diff --git a/packages/opencode/src/cli/tui/worker.ts b/packages/opencode/src/cli/tui/worker.ts index 4cf6b2d446b3..3f66b9b45f6e 100644 --- a/packages/opencode/src/cli/tui/worker.ts +++ b/packages/opencode/src/cli/tui/worker.ts @@ -2,14 +2,12 @@ import { Server } from "@/server/server" import { InstanceRuntime } from "@/project/instance-runtime" import { Rpc } from "@/util/rpc" import { upgrade } from "@/cli/upgrade" -import { Config } from "@/config/config" import { GlobalBus } from "@/bus/global" import { ServerAuth } from "@/server/auth" import { writeHeapSnapshot } from "node:v8" import { Heap } from "@/cli/heap" import { AppRuntime } from "@/effect/app-runtime" -import { Effect } from "effect" -import { disposeAllInstancesAndEmitGlobalDisposed } from "@/server/global-lifecycle" +import { reloadIfGlobalConfigChanged } from "@/server/global-lifecycle" Heap.start() @@ -61,13 +59,7 @@ export const rpc = { await upgrade().catch(() => {}) }, async reload() { - await AppRuntime.runPromise( - Effect.gen(function* () { - const cfg = yield* Config.Service - yield* cfg.invalidate() - yield* disposeAllInstancesAndEmitGlobalDisposed({ swallowErrors: true }) - }), - ) + await AppRuntime.runPromise(reloadIfGlobalConfigChanged()) }, async shutdown() { await InstanceRuntime.disposeAllInstances() diff --git a/packages/opencode/src/server/global-lifecycle.ts b/packages/opencode/src/server/global-lifecycle.ts index 12b7687bfe08..aa5d3286db69 100644 --- a/packages/opencode/src/server/global-lifecycle.ts +++ b/packages/opencode/src/server/global-lifecycle.ts @@ -1,4 +1,5 @@ import { GlobalBus } from "@/bus/global" +import { Config } from "@/config/config" import { InstanceStore } from "@/project/instance-store" import { Effect } from "effect" import { Event } from "./event" @@ -25,4 +26,19 @@ export const disposeAllInstancesAndEmitGlobalDisposed = Effect.fn("Server.dispos }, ) +// External reload signals (SIGUSR2 from theme switchers such as Omarchy or +// Noctalia hooks) usually mean "repaint": themes are discovered from +// themes/*.json and never touch the global config. Disposing instances +// aborts their in-flight sessions, so only pay that cost when the global +// config itself actually changed. Returns whether instances were disposed. +export const reloadIfGlobalConfigChanged = Effect.fn("Server.reloadIfGlobalConfigChanged")(function* () { + const config = yield* Config.Service + const before = yield* config.getGlobal() + yield* config.invalidate() + const next = yield* config.getGlobal() + if (JSON.stringify(before) === JSON.stringify(next)) return false + yield* disposeAllInstancesAndEmitGlobalDisposed({ swallowErrors: true }) + return true +}) + export * as GlobalLifecycle from "./global-lifecycle" diff --git a/packages/opencode/test/server/global-lifecycle.test.ts b/packages/opencode/test/server/global-lifecycle.test.ts new file mode 100644 index 000000000000..970ff9fe7a7f --- /dev/null +++ b/packages/opencode/test/server/global-lifecycle.test.ts @@ -0,0 +1,82 @@ +import { beforeEach, expect } from "bun:test" +import { Effect, Layer } from "effect" +import { InstanceStore } from "@/project/instance-store" +import { reloadIfGlobalConfigChanged } from "@/server/global-lifecycle" +import { TestConfig } from "../fixture/config" +import { testEffect } from "../lib/effect" + +// Models the real Config service contract: a cached global config that only +// re-reads the file on invalidate(). +function make() { + let cached: object = {} + let onDisk: object = {} + let disposals = 0 + const layer = Layer.mergeAll( + TestConfig.layer({ + getGlobal: () => Effect.sync(() => cached), + invalidate: () => + Effect.sync(() => { + cached = onDisk + }), + }), + Layer.succeed( + InstanceStore.Service, + InstanceStore.Service.of({ + load: () => Effect.die("unexpected load"), + reload: () => Effect.die("unexpected reload"), + dispose: () => Effect.die("unexpected dispose"), + disposeDirectory: () => Effect.die("unexpected disposeDirectory"), + disposeAll: () => + Effect.sync(() => { + disposals += 1 + }), + provide: (_input, effect) => effect, + }), + ), + ) + return { + layer, + get disposals() { + return disposals + }, + changeOnDisk(next: object) { + onDisk = next + }, + reset() { + cached = {} + onDisk = {} + disposals = 0 + }, + } +} + +const fx = make() +const it = testEffect(fx.layer) + +beforeEach(() => fx.reset()) + +it.effect("leaves instances running when the global config is unchanged", () => + Effect.gen(function* () { + const disposed = yield* reloadIfGlobalConfigChanged() + expect(disposed).toBe(false) + expect(fx.disposals).toBe(0) + }), +) + +it.effect("disposes instances when the global config changed on disk", () => + Effect.gen(function* () { + fx.changeOnDisk({ username: "alice" }) + const disposed = yield* reloadIfGlobalConfigChanged() + expect(disposed).toBe(true) + expect(fx.disposals).toBe(1) + }), +) + +it.effect("does not dispose again once the change is absorbed", () => + Effect.gen(function* () { + fx.changeOnDisk({ username: "alice" }) + expect(yield* reloadIfGlobalConfigChanged()).toBe(true) + expect(yield* reloadIfGlobalConfigChanged()).toBe(false) + expect(fx.disposals).toBe(1) + }), +) From 1fcafcbc92115b385565ee20896986ef332cf823 Mon Sep 17 00:00:00 2001 From: Keith Hughitt Date: Mon, 14 Sep 2026 17:55:30 -0400 Subject: [PATCH 2/3] fix(tui): cover instance inputs in the SIGUSR2 reload gate Review follow-ups: - Read failures can no longer wedge reload: the cached read runs through catchCause and invalidate() always executes. An unreadable config is treated as changed (the previous unconditional-dispose behavior), so a malformed file neither skips invalidation nor pins the cache. - The gate now also covers per-instance on-disk inputs (project config, agent/command/mode/plugin files, OPENCODE_CONFIG) via a content fingerprint captured at instance boot, so editing those and signaling still reloads the instance. Theme files stay excluded. - Config comparison is key-order insensitive (canonicalEquals), so pure key reordering no longer counts as a change. --- packages/opencode/src/cli/tui/worker.ts | 4 +- packages/opencode/src/config/fingerprint.ts | 81 ++++++++++++ .../opencode/src/project/instance-store.ts | 31 ++++- .../opencode/src/server/global-lifecycle.ts | 41 ++++-- .../opencode/test/config/fingerprint.test.ts | 119 ++++++++++++++++++ .../opencode/test/project/instance.test.ts | 26 ++++ .../test/server/global-lifecycle.test.ts | 73 +++++++++-- 7 files changed, 354 insertions(+), 21 deletions(-) create mode 100644 packages/opencode/src/config/fingerprint.ts create mode 100644 packages/opencode/test/config/fingerprint.test.ts diff --git a/packages/opencode/src/cli/tui/worker.ts b/packages/opencode/src/cli/tui/worker.ts index 3f66b9b45f6e..6cdbb8be7667 100644 --- a/packages/opencode/src/cli/tui/worker.ts +++ b/packages/opencode/src/cli/tui/worker.ts @@ -7,7 +7,7 @@ import { ServerAuth } from "@/server/auth" import { writeHeapSnapshot } from "node:v8" import { Heap } from "@/cli/heap" import { AppRuntime } from "@/effect/app-runtime" -import { reloadIfGlobalConfigChanged } from "@/server/global-lifecycle" +import { reloadIfConfigChanged } from "@/server/global-lifecycle" Heap.start() @@ -59,7 +59,7 @@ export const rpc = { await upgrade().catch(() => {}) }, async reload() { - await AppRuntime.runPromise(reloadIfGlobalConfigChanged()) + await AppRuntime.runPromise(reloadIfConfigChanged()) }, async shutdown() { await InstanceRuntime.disposeAllInstances() diff --git a/packages/opencode/src/config/fingerprint.ts b/packages/opencode/src/config/fingerprint.ts new file mode 100644 index 000000000000..5ae187caffc1 --- /dev/null +++ b/packages/opencode/src/config/fingerprint.ts @@ -0,0 +1,81 @@ +import { createHash } from "crypto" +import { Flag } from "@opencode-ai/core/flag/flag" +import { Glob } from "@opencode-ai/core/util/glob" +import { Effect } from "effect" +import fs from "fs/promises" +import path from "path" +import { ConfigParse } from "./parse" +import { ConfigPaths } from "./paths" + +// Order-insensitive structural equality for decoded config values: key order +// in a JSON object carries no meaning, so reordering keys must not count as a +// change. +export const canonicalEquals = (a: unknown, b: unknown): boolean => + JSON.stringify(canonicalize(a)) === JSON.stringify(canonicalize(b)) + +const canonicalize = (value: unknown): unknown => { + if (Array.isArray(value)) return value.map(canonicalize) + if (value && typeof value === "object") { + return Object.fromEntries( + Object.entries(value) + .sort(([x], [y]) => x.localeCompare(y)) + .map(([k, v]) => [k, canonicalize(v)]), + ) + } + return value +} + +// JSON/JSONC files are canonicalized so that pure key reorders hash equal; +// anything unparseable is hashed raw (a malformed file is still a state worth +// detecting, and fixing it changes the hash). +const normalize = (file: string, text: string): string => { + if (!file.endsWith(".json") && !file.endsWith(".jsonc")) return text + try { + return JSON.stringify(canonicalize(ConfigParse.jsonc(text, file))) + } catch { + return text + } +} + +// Content fingerprint of everything the instance config loader reads from disk +// for one project: the project opencode.json/jsonc chain, every config +// directory's agent/mode/command/plugin files, the filtered directories' +// opencode.json/jsonc, and the OPENCODE_CONFIG override file. Mirrors the +// reads in Config's instance loader. Theme files are deliberately excluded: +// they affect rendering only and must not force an instance rebuild. +export const hashInstanceInputs = Effect.fn("ConfigFingerprint.hashInstanceInputs")(function* ( + directory: string, + worktree?: string, +) { + const files: string[] = [] + for (const file of yield* ConfigPaths.files("opencode", directory, worktree).pipe(Effect.orDie)) { + files.push(file) + } + for (const dir of yield* ConfigPaths.directories(directory, worktree).pipe(Effect.orDie)) { + if (dir.endsWith(".opencode") || dir === Flag.OPENCODE_CONFIG_DIR) { + files.push(path.join(dir, "opencode.json"), path.join(dir, "opencode.jsonc")) + } + for (const pattern of [ + "{agent,agents}/**/*.md", + "{mode,modes}/*.md", + "{command,commands}/**/*.md", + "{plugin,plugins}/*.{ts,js}", + ]) { + files.push(...(yield* Effect.promise(() => Glob.scan(pattern, { cwd: dir, absolute: true, dot: true })))) + } + } + if (Flag.OPENCODE_CONFIG) files.push(Flag.OPENCODE_CONFIG) + + const hash = createHash("sha256") + for (const file of [...new Set(files)].sort()) { + const text = yield* Effect.promise(() => fs.readFile(file, "utf8").catch(() => undefined)) + if (text === undefined) continue + hash.update(file) + hash.update("\0") + hash.update(normalize(file, text)) + hash.update("\0") + } + return hash.digest("hex") +}) + +export * as ConfigFingerprint from "./fingerprint" diff --git a/packages/opencode/src/project/instance-store.ts b/packages/opencode/src/project/instance-store.ts index 720549ddaff7..633c1d4073de 100644 --- a/packages/opencode/src/project/instance-store.ts +++ b/packages/opencode/src/project/instance-store.ts @@ -6,6 +6,7 @@ import { WorkspaceContext } from "@/control-plane/workspace-context" import { InstanceRef } from "@/effect/instance-ref" import { disposeInstance as runDisposers } from "@/effect/instance-registry" import { FSUtil } from "@opencode-ai/core/fs-util" +import { ConfigFingerprint } from "@/config/fingerprint" import { Context, Deferred, Duration, Effect, Exit, Layer, Scope } from "effect" import { type InstanceContext } from "./instance-context" import { InstanceBootstrap } from "./bootstrap-service" @@ -24,6 +25,10 @@ export interface Interface { readonly disposeDirectory: (directory: string) => Effect.Effect readonly disposeAll: () => Effect.Effect readonly provide: (input: LoadInput, effect: Effect.Effect) => Effect.Effect + // True when any loaded instance's on-disk config inputs (project config, + // agent/command/mode/plugin files, OPENCODE_CONFIG) changed since that + // instance booted. A missing or unreadable baseline counts as changed. + readonly configChanged: () => Effect.Effect } export class Service extends Context.Service()("@opencode/InstanceStore") {} @@ -32,13 +37,15 @@ export const use = serviceUse(Service) interface Entry { readonly deferred: Deferred.Deferred + configHash?: string } -const layer: Layer.Layer = Layer.effect( +const layer: Layer.Layer = Layer.effect( Service, Effect.gen(function* () { const project = yield* Project.Service const bootstrap = yield* InstanceBootstrap.Service + const fsutil = yield* FSUtil.Service const scope = yield* Scope.Scope const cache = new Map() @@ -73,6 +80,11 @@ const layer: Layer.Layer undefined), + ) yield* Deferred.done(entry.deferred, exit).pipe(Effect.asVoid) }) @@ -186,6 +198,20 @@ const layer: Layer.Layer undefined), + ) + if (fresh === undefined || fresh !== entry.configHash) return true + } + return false + }) + const provide = (input: LoadInput, effect: Effect.Effect): Effect.Effect => load(input).pipe(Effect.flatMap((ctx) => effect.pipe(Effect.provideService(InstanceRef, ctx)))) @@ -198,6 +224,7 @@ const layer: Layer.Layer @@ -28,17 +29,37 @@ export const disposeAllInstancesAndEmitGlobalDisposed = Effect.fn("Server.dispos // External reload signals (SIGUSR2 from theme switchers such as Omarchy or // Noctalia hooks) usually mean "repaint": themes are discovered from -// themes/*.json and never touch the global config. Disposing instances -// aborts their in-flight sessions, so only pay that cost when the global -// config itself actually changed. Returns whether instances were disposed. -export const reloadIfGlobalConfigChanged = Effect.fn("Server.reloadIfGlobalConfigChanged")(function* () { +// themes/*.json and never touch the config. Disposing instances aborts their +// in-flight sessions, so only pay that cost when config inputs actually +// changed. Covers the global config and every loaded instance's on-disk +// inputs (project config, agent/command/mode/plugin files, OPENCODE_CONFIG). +// A read failure or a missing fingerprint baseline counts as changed, keeping +// the historical unconditional-dispose behavior for any case the gate cannot +// prove is clean. Returns whether instances were disposed. +export const reloadIfConfigChanged = Effect.fn("Server.reloadIfConfigChanged")(function* () { const config = yield* Config.Service - const before = yield* config.getGlobal() + const store = yield* InstanceStore.Service + // ConfigParse throws on malformed files, surfacing here as a defect rather + // than a typed error, so read via catchAllCause: a failed read must never + // block invalidation, and unprovable means changed. + const read = config.getGlobal().pipe( + Effect.map(Option.some), + Effect.catchCause((cause) => + Effect.logWarning("global config read failed during reload check", { cause: String(cause) }).pipe( + Effect.as(Option.none()), + ), + ), + ) + const before = yield* read yield* config.invalidate() - const next = yield* config.getGlobal() - if (JSON.stringify(before) === JSON.stringify(next)) return false - yield* disposeAllInstancesAndEmitGlobalDisposed({ swallowErrors: true }) - return true + const next = yield* read + const globalChanged = + Option.isNone(before) || Option.isNone(next) || !ConfigFingerprint.canonicalEquals(before.value, next.value) + if (globalChanged || (yield* store.configChanged())) { + yield* disposeAllInstancesAndEmitGlobalDisposed({ swallowErrors: true }) + return true + } + return false }) export * as GlobalLifecycle from "./global-lifecycle" diff --git a/packages/opencode/test/config/fingerprint.test.ts b/packages/opencode/test/config/fingerprint.test.ts new file mode 100644 index 000000000000..5966e46c4f98 --- /dev/null +++ b/packages/opencode/test/config/fingerprint.test.ts @@ -0,0 +1,119 @@ +import { expect } from "bun:test" +import { Effect } from "effect" +import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" +import { LayerNode } from "@opencode-ai/core/effect/layer-node" +import { FSUtil } from "@opencode-ai/core/fs-util" +import fs from "fs/promises" +import path from "path" +import { ConfigFingerprint } from "@/config/fingerprint" +import { tmpdirScoped } from "../fixture/fixture" +import { testEffect } from "../lib/effect" + +const it = testEffect(LayerNode.compile(LayerNode.group([CrossSpawnSpawner.node, FSUtil.node]))) + +const hash = (dir: string) => ConfigFingerprint.hashInstanceInputs(dir, dir) + +const write = (file: string, text: string) => Effect.promise(() => fs.writeFile(file, text)) + +// Point Global.Path.home at the temp project so the home-level .opencode +// lookup stays hermetic. +const withHome = (dir: string, effect: Effect.Effect) => + Effect.acquireUseRelease( + Effect.sync(() => { + const previous = process.env.OPENCODE_TEST_HOME + process.env.OPENCODE_TEST_HOME = dir + return previous + }), + () => effect, + (previous) => + Effect.sync(() => { + if (previous === undefined) delete process.env.OPENCODE_TEST_HOME + else process.env.OPENCODE_TEST_HOME = previous + }), + ) + +const inTmpProject = (fn: (dir: string) => Effect.Effect) => + Effect.gen(function* () { + const dir = yield* tmpdirScoped() + return yield* withHome(dir, fn(dir)) + }) + +it.effect("is stable across calls when nothing changed", () => + inTmpProject((dir) => + Effect.gen(function* () { + expect(yield* hash(dir)).toBe(yield* hash(dir)) + }), + ), +) + +it.effect("ignores key reordering in the project config", () => + inTmpProject((dir) => + Effect.gen(function* () { + const file = path.join(dir, "opencode.json") + yield* write(file, JSON.stringify({ username: "alice", logLevel: "DEBUG" })) + const before = yield* hash(dir) + yield* write(file, JSON.stringify({ logLevel: "DEBUG", username: "alice" })) + expect(yield* hash(dir)).toBe(before) + }), + ), +) + +it.effect("detects project config edits", () => + inTmpProject((dir) => + Effect.gen(function* () { + const file = path.join(dir, "opencode.json") + yield* write(file, JSON.stringify({ username: "alice" })) + const before = yield* hash(dir) + yield* write(file, JSON.stringify({ username: "bob" })) + expect(yield* hash(dir)).not.toBe(before) + }), + ), +) + +it.effect("detects added agent files", () => + inTmpProject((dir) => + Effect.gen(function* () { + const before = yield* hash(dir) + yield* Effect.promise(() => fs.mkdir(path.join(dir, ".opencode", "agent"), { recursive: true })) + yield* write(path.join(dir, ".opencode", "agent", "reviewer.md"), "review things") + expect(yield* hash(dir)).not.toBe(before) + }), + ), +) + +it.effect("ignores theme files, which are rendering-only", () => + inTmpProject((dir) => + Effect.gen(function* () { + const before = yield* hash(dir) + yield* Effect.promise(() => fs.mkdir(path.join(dir, ".opencode", "themes"), { recursive: true })) + yield* write(path.join(dir, ".opencode", "themes", "night.json"), JSON.stringify({ theme: {} })) + expect(yield* hash(dir)).toBe(before) + }), + ), +) + +it.effect("detects plugin file changes", () => + inTmpProject((dir) => + Effect.gen(function* () { + const plugin = path.join(dir, ".opencode", "plugin") + yield* Effect.promise(() => fs.mkdir(plugin, { recursive: true })) + const file = path.join(plugin, "x.ts") + yield* write(file, "export const a = 1") + const before = yield* hash(dir) + yield* write(file, "export const a = 2") + expect(yield* hash(dir)).not.toBe(before) + }), + ), +) + +it.effect("treats a malformed config as a state and detects fixing it", () => + inTmpProject((dir) => + Effect.gen(function* () { + const file = path.join(dir, "opencode.json") + yield* write(file, "{ not json") + const broken = yield* hash(dir) + yield* write(file, JSON.stringify({ username: "alice" })) + expect(yield* hash(dir)).not.toBe(broken) + }), + ), +) diff --git a/packages/opencode/test/project/instance.test.ts b/packages/opencode/test/project/instance.test.ts index f78b99ef7d9b..13fa5184926c 100644 --- a/packages/opencode/test/project/instance.test.ts +++ b/packages/opencode/test/project/instance.test.ts @@ -2,6 +2,8 @@ import { describe, expect } from "bun:test" import { LayerNode } from "@opencode-ai/core/effect/layer-node" import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" import { Deferred, Effect, Fiber, Layer } from "effect" +import fs from "fs/promises" +import path from "path" import { InstanceRef } from "../../src/effect/instance-ref" import { registerDisposer } from "../../src/effect/instance-registry" import { InstanceBootstrap } from "../../src/project/bootstrap" @@ -50,6 +52,30 @@ describe("InstanceStore", () => { }), ) + it.live("configChanged tracks on-disk config inputs", () => + Effect.gen(function* () { + const dir = yield* tmpdirScoped({ git: true }) + const home = yield* tmpdirScoped() + const previousHome = process.env.OPENCODE_TEST_HOME + process.env.OPENCODE_TEST_HOME = home + try { + const store = yield* InstanceStore.Service + yield* store.load({ directory: dir }) + + expect(yield* store.configChanged()).toBe(false) + + yield* Effect.promise(() => fs.mkdir(path.join(dir, ".opencode", "agent"), { recursive: true })) + yield* Effect.promise(() => + fs.writeFile(path.join(dir, ".opencode", "agent", "reviewer.md"), "review things"), + ) + expect(yield* store.configChanged()).toBe(true) + } finally { + if (previousHome === undefined) delete process.env.OPENCODE_TEST_HOME + else process.env.OPENCODE_TEST_HOME = previousHome + } + }), + ) + it.live("runs bootstrap with InstanceRef provided", () => Effect.gen(function* () { const dir = yield* tmpdirScoped({ git: true }) diff --git a/packages/opencode/test/server/global-lifecycle.test.ts b/packages/opencode/test/server/global-lifecycle.test.ts index 970ff9fe7a7f..4cacdbc657ed 100644 --- a/packages/opencode/test/server/global-lifecycle.test.ts +++ b/packages/opencode/test/server/global-lifecycle.test.ts @@ -1,22 +1,28 @@ import { beforeEach, expect } from "bun:test" import { Effect, Layer } from "effect" import { InstanceStore } from "@/project/instance-store" -import { reloadIfGlobalConfigChanged } from "@/server/global-lifecycle" +import { reloadIfConfigChanged } from "@/server/global-lifecycle" import { TestConfig } from "../fixture/config" import { testEffect } from "../lib/effect" // Models the real Config service contract: a cached global config that only -// re-reads the file on invalidate(). +// re-reads the file on invalidate(). `failReads` simulates a cached read that +// defects (e.g. a malformed config file) until the next invalidate. function make() { let cached: object = {} let onDisk: object = {} + let failReads = false + let invalidations = 0 let disposals = 0 + let inputsChanged = false const layer = Layer.mergeAll( TestConfig.layer({ - getGlobal: () => Effect.sync(() => cached), + getGlobal: () => (failReads ? Effect.die("cached read failed") : Effect.sync(() => cached)), invalidate: () => Effect.sync(() => { + invalidations += 1 cached = onDisk + failReads = false }), }), Layer.succeed( @@ -31,6 +37,7 @@ function make() { disposals += 1 }), provide: (_input, effect) => effect, + configChanged: () => Effect.sync(() => inputsChanged), }), ), ) @@ -39,13 +46,31 @@ function make() { get disposals() { return disposals }, + get invalidations() { + return invalidations + }, + // Instances were built with `config` and the file matches it. + set(config: object) { + cached = config + onDisk = config + }, + // The file was edited on disk after instances booted. changeOnDisk(next: object) { onDisk = next }, + failReads() { + failReads = true + }, + setInputsChanged(next: boolean) { + inputsChanged = next + }, reset() { cached = {} onDisk = {} + failReads = false + invalidations = 0 disposals = 0 + inputsChanged = false }, } } @@ -57,7 +82,7 @@ beforeEach(() => fx.reset()) it.effect("leaves instances running when the global config is unchanged", () => Effect.gen(function* () { - const disposed = yield* reloadIfGlobalConfigChanged() + const disposed = yield* reloadIfConfigChanged() expect(disposed).toBe(false) expect(fx.disposals).toBe(0) }), @@ -66,7 +91,7 @@ it.effect("leaves instances running when the global config is unchanged", () => it.effect("disposes instances when the global config changed on disk", () => Effect.gen(function* () { fx.changeOnDisk({ username: "alice" }) - const disposed = yield* reloadIfGlobalConfigChanged() + const disposed = yield* reloadIfConfigChanged() expect(disposed).toBe(true) expect(fx.disposals).toBe(1) }), @@ -75,8 +100,42 @@ it.effect("disposes instances when the global config changed on disk", () => it.effect("does not dispose again once the change is absorbed", () => Effect.gen(function* () { fx.changeOnDisk({ username: "alice" }) - expect(yield* reloadIfGlobalConfigChanged()).toBe(true) - expect(yield* reloadIfGlobalConfigChanged()).toBe(false) + expect(yield* reloadIfConfigChanged()).toBe(true) + expect(yield* reloadIfConfigChanged()).toBe(false) + expect(fx.disposals).toBe(1) + }), +) + +it.effect("still invalidates when the cached read fails, and recovers on the next reload", () => + Effect.gen(function* () { + fx.failReads() + // A failed cached read cannot prove the config is unchanged, so the + // historical disposal runs -- and, critically, the cache is invalidated + // rather than left broken. + expect(yield* reloadIfConfigChanged()).toBe(true) + expect(fx.invalidations).toBe(1) + expect(fx.disposals).toBe(1) + // After the underlying file is readable again, the gate works normally. + expect(yield* reloadIfConfigChanged()).toBe(false) + expect(fx.disposals).toBe(1) + }), +) + +it.effect("ignores pure key reordering in the global config", () => + Effect.gen(function* () { + fx.set({ username: "alice", logLevel: "DEBUG" }) + fx.changeOnDisk({ logLevel: "DEBUG", username: "alice" }) + const disposed = yield* reloadIfConfigChanged() + expect(disposed).toBe(false) + expect(fx.disposals).toBe(0) + }), +) + +it.effect("disposes instances when an instance's own config inputs changed", () => + Effect.gen(function* () { + fx.setInputsChanged(true) + const disposed = yield* reloadIfConfigChanged() + expect(disposed).toBe(true) expect(fx.disposals).toBe(1) }), ) From 40c5fd4899055923606f4befe3284adfca7d2963 Mon Sep 17 00:00:00 2001 From: Keith Hughitt Date: Mon, 14 Sep 2026 18:35:52 -0400 Subject: [PATCH 3/3] fix(tui): preserve reload semantics across fingerprint checks --- packages/opencode/src/config/fingerprint.ts | 37 +++++---- .../opencode/src/project/instance-store.ts | 29 ++++--- .../opencode/test/config/fingerprint.test.ts | 78 ++++++++++++++++++- .../opencode/test/project/instance.test.ts | 75 +++++++++++++++++- .../test/server/global-lifecycle.test.ts | 9 +++ 5 files changed, 198 insertions(+), 30 deletions(-) diff --git a/packages/opencode/src/config/fingerprint.ts b/packages/opencode/src/config/fingerprint.ts index 5ae187caffc1..2852573a3d47 100644 --- a/packages/opencode/src/config/fingerprint.ts +++ b/packages/opencode/src/config/fingerprint.ts @@ -4,12 +4,13 @@ import { Glob } from "@opencode-ai/core/util/glob" import { Effect } from "effect" import fs from "fs/promises" import path from "path" +import { isRecord } from "@/util/record" import { ConfigParse } from "./parse" import { ConfigPaths } from "./paths" +import { ConfigVariable } from "./variable" -// Order-insensitive structural equality for decoded config values: key order -// in a JSON object carries no meaning, so reordering keys must not count as a -// change. +// Most config objects are unordered, but permission maps use last-match +// precedence. Preserve their order, including permissions inside agent config. export const canonicalEquals = (a: unknown, b: unknown): boolean => JSON.stringify(canonicalize(a)) === JSON.stringify(canonicalize(b)) @@ -19,21 +20,24 @@ const canonicalize = (value: unknown): unknown => { return Object.fromEntries( Object.entries(value) .sort(([x], [y]) => x.localeCompare(y)) - .map(([k, v]) => [k, canonicalize(v)]), + .map(([k, v]) => [k, k === "permission" ? v : canonicalize(v)]), ) } return value } -// JSON/JSONC files are canonicalized so that pure key reorders hash equal; -// anything unparseable is hashed raw (a malformed file is still a state worth -// detecting, and fixing it changes the hash). -const normalize = (file: string, text: string): string => { +// Normalize expanded config so referenced file changes are visible, while +// harmless key reorders hash equal. Malformed expanded text is hashed as-is. +const normalize = async (file: string, text: string): Promise => { if (!file.endsWith(".json") && !file.endsWith(".jsonc")) return text + const expanded = await ConfigVariable.substitute({ type: "path", path: file, text }) try { - return JSON.stringify(canonicalize(ConfigParse.jsonc(text, file))) + const value = ConfigParse.jsonc(expanded, file) + // The loader inserts this editor hint itself after the baseline is read. + if (isRecord(value)) delete value.$schema + return JSON.stringify(canonicalize(value)) } catch { - return text + return expanded } } @@ -61,18 +65,25 @@ export const hashInstanceInputs = Effect.fn("ConfigFingerprint.hashInstanceInput "{command,commands}/**/*.md", "{plugin,plugins}/*.{ts,js}", ]) { - files.push(...(yield* Effect.promise(() => Glob.scan(pattern, { cwd: dir, absolute: true, dot: true })))) + files.push( + ...(yield* Effect.promise(() => Glob.scan(pattern, { cwd: dir, absolute: true, dot: true, symlink: true }))), + ) } } if (Flag.OPENCODE_CONFIG) files.push(Flag.OPENCODE_CONFIG) const hash = createHash("sha256") for (const file of [...new Set(files)].sort()) { - const text = yield* Effect.promise(() => fs.readFile(file, "utf8").catch(() => undefined)) + const text = yield* Effect.promise(() => + fs.readFile(file, "utf8").catch((error: NodeJS.ErrnoException) => { + if (error.code === "ENOENT") return undefined + throw error + }), + ) if (text === undefined) continue hash.update(file) hash.update("\0") - hash.update(normalize(file, text)) + hash.update(yield* Effect.promise(() => normalize(file, text))) hash.update("\0") } return hash.digest("hex") diff --git a/packages/opencode/src/project/instance-store.ts b/packages/opencode/src/project/instance-store.ts index 633c1d4073de..c4da51fac7f1 100644 --- a/packages/opencode/src/project/instance-store.ts +++ b/packages/opencode/src/project/instance-store.ts @@ -49,7 +49,17 @@ const layer: Layer.Layer() - const boot = (input: LoadInput & { directory: string }) => + const fingerprint = (ctx: InstanceContext) => + ConfigFingerprint.hashInstanceInputs(ctx.directory, ctx.worktree).pipe( + Effect.provideService(FSUtil.Service, fsutil), + Effect.catchCause((cause) => + Effect.logWarning("config fingerprint failed", { directory: ctx.directory, cause }).pipe( + Effect.as(undefined), + ), + ), + ) + + const boot = (input: LoadInput & { directory: string }, entry: Entry) => Effect.gen(function* () { const ctx: InstanceContext = input.project && input.worktree @@ -65,6 +75,9 @@ const layer: Layer.Layer Effect.gen(function* () { - const exit = yield* Effect.exit(boot({ ...input, directory })) + const exit = yield* Effect.exit(boot({ ...input, directory }, entry)) if (Exit.isFailure(exit)) yield* removeEntry(directory, entry) - else - entry.configHash = yield* ConfigFingerprint.hashInstanceInputs(directory, exit.value.worktree).pipe( - Effect.provideService(FSUtil.Service, fsutil), - Effect.orElseSucceed(() => undefined), - ) yield* Deferred.done(entry.deferred, exit).pipe(Effect.asVoid) }) @@ -199,14 +207,11 @@ const layer: Layer.Layer undefined), - ) + const fresh = yield* fingerprint(exit.value) if (fresh === undefined || fresh !== entry.configHash) return true } return false diff --git a/packages/opencode/test/config/fingerprint.test.ts b/packages/opencode/test/config/fingerprint.test.ts index 5966e46c4f98..717c363fc50b 100644 --- a/packages/opencode/test/config/fingerprint.test.ts +++ b/packages/opencode/test/config/fingerprint.test.ts @@ -1,11 +1,12 @@ import { expect } from "bun:test" -import { Effect } from "effect" +import { Effect, Exit } from "effect" import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" import { LayerNode } from "@opencode-ai/core/effect/layer-node" import { FSUtil } from "@opencode-ai/core/fs-util" import fs from "fs/promises" import path from "path" import { ConfigFingerprint } from "@/config/fingerprint" +import { Permission } from "@/permission" import { tmpdirScoped } from "../fixture/fixture" import { testEffect } from "../lib/effect" @@ -117,3 +118,78 @@ it.effect("treats a malformed config as a state and detects fixing it", () => }), ), ) + +it.live("detects permission reordering that changes the winning rule", () => + inTmpProject((dir) => + Effect.gen(function* () { + const before = { permission: { bash: { "git *": "deny", "*": "allow" } } } as const + const next = { permission: { bash: { "*": "allow", "git *": "deny" } } } as const + expect(Permission.evaluate("bash", "git push", Permission.fromConfig(before.permission)).action).toBe("allow") + expect(Permission.evaluate("bash", "git push", Permission.fromConfig(next.permission)).action).toBe("deny") + expect(ConfigFingerprint.canonicalEquals(before, next)).toBe(false) + expect(ConfigFingerprint.canonicalEquals({ agent: { build: before } }, { agent: { build: next } })).toBe(false) + + const file = path.join(dir, "opencode.json") + yield* write(file, JSON.stringify(before)) + const baseline = yield* hash(dir) + yield* write(file, JSON.stringify(next)) + expect(yield* hash(dir)).not.toBe(baseline) + }), + ), +) + +it.live("detects changes to a file referenced by project config", () => + inTmpProject((dir) => + Effect.gen(function* () { + yield* write( + path.join(dir, "opencode.json"), + JSON.stringify({ agent: { build: { prompt: "{file:prompt.txt}" } } }), + ) + const file = path.join(dir, "prompt.txt") + yield* write(file, "old prompt") + const before = yield* hash(dir) + yield* write(file, "new prompt") + expect(yield* hash(dir)).not.toBe(before) + }), + ), +) + +it.live("follows nested symlinked agent and command directories", () => + inTmpProject((dir) => + Effect.gen(function* () { + for (const kind of ["agents", "commands"]) { + const target = path.join(dir, kind, "nested") + const source = path.join(dir, ".opencode", kind) + yield* Effect.promise(() => fs.mkdir(target, { recursive: true })) + yield* Effect.promise(() => fs.mkdir(source, { recursive: true })) + yield* Effect.promise(() => fs.symlink(path.dirname(target), path.join(source, "shared"), "dir")) + const file = path.join(target, "review.md") + yield* write(file, "old prompt") + const before = yield* hash(dir) + yield* write(file, "new prompt") + expect(yield* hash(dir)).not.toBe(before) + } + }), + ), +) + +it.live("ignores schema metadata added by the config loader", () => + inTmpProject((dir) => + Effect.gen(function* () { + const file = path.join(dir, "opencode.json") + yield* write(file, JSON.stringify({ username: "alice" })) + const before = yield* hash(dir) + yield* write(file, JSON.stringify({ $schema: "https://opencode.ai/config.json", username: "alice" })) + expect(yield* hash(dir)).toBe(before) + }), + ), +) + +it.live("fails on unreadable config inputs instead of hashing an incomplete set", () => + inTmpProject((dir) => + Effect.gen(function* () { + yield* Effect.promise(() => fs.mkdir(path.join(dir, "opencode.json"))) + expect(Exit.isFailure(yield* hash(dir).pipe(Effect.exit))).toBe(true) + }), + ), +) diff --git a/packages/opencode/test/project/instance.test.ts b/packages/opencode/test/project/instance.test.ts index 13fa5184926c..5e1fa3a3943f 100644 --- a/packages/opencode/test/project/instance.test.ts +++ b/packages/opencode/test/project/instance.test.ts @@ -1,6 +1,7 @@ import { describe, expect } from "bun:test" import { LayerNode } from "@opencode-ai/core/effect/layer-node" import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" +import { FSUtil } from "@opencode-ai/core/fs-util" import { Deferred, Effect, Fiber, Layer } from "effect" import fs from "fs/promises" import path from "path" @@ -8,18 +9,34 @@ import { InstanceRef } from "../../src/effect/instance-ref" import { registerDisposer } from "../../src/effect/instance-registry" import { InstanceBootstrap } from "../../src/project/bootstrap" import { InstanceStore } from "../../src/project/instance-store" +import { Project } from "../../src/project/project" import { tmpdirScoped } from "../fixture/fixture" import { testEffect } from "../lib/effect" let bootstrapRun: Effect.Effect = Effect.void +let fingerprintFailure = false +const fingerprintFS = Layer.effect( + FSUtil.Service, + Effect.gen(function* () { + const fsutil = yield* FSUtil.Service + return FSUtil.Service.of({ + ...fsutil, + up: (input) => + Effect.suspend(() => + fingerprintFailure ? Effect.die(new Error("fingerprint filesystem failure")) : fsutil.up(input), + ), + }) + }), +).pipe(Layer.provide(LayerNode.compile(FSUtil.node))) const noopBootstrap = Layer.succeed( InstanceBootstrap.Service, InstanceBootstrap.Service.of({ run: Effect.suspend(() => bootstrapRun) }), ) const it = testEffect( - LayerNode.compile(LayerNode.group([InstanceStore.node, CrossSpawnSpawner.node]), [ + LayerNode.compile(LayerNode.group([InstanceStore.node, Project.node, CrossSpawnSpawner.node]), [ [InstanceStore.bootstrapNode, noopBootstrap], + [FSUtil.node, fingerprintFS], ]), ) @@ -34,6 +51,16 @@ const setBootstrap = (run: Effect.Effect) => }), ) +const failFingerprint = Effect.acquireRelease( + Effect.sync(() => { + fingerprintFailure = true + }), + () => + Effect.sync(() => { + fingerprintFailure = false + }), +) + const registerDisposerScoped = (disposer: (directory: string) => Promise) => Effect.acquireRelease( Effect.sync(() => registerDisposer(disposer)), @@ -41,6 +68,48 @@ const registerDisposerScoped = (disposer: (directory: string) => Promise) ) describe("InstanceStore", () => { + it.live("finishes loading when fingerprinting defects and treats the missing baseline as changed", () => + Effect.gen(function* () { + const dir = yield* tmpdirScoped({ git: true }) + const store = yield* InstanceStore.Service + const project = yield* Project.Service + const info = yield* project.fromDirectory(dir) + yield* failFingerprint + const ctx = yield* store.load({ directory: dir, worktree: info.sandbox, project: info.project }) + expect(ctx.directory).toBe(dir) + expect(yield* store.configChanged()).toBe(true) + }), + ) + + it.live("treats a fingerprint defect during reload checking as changed", () => + Effect.gen(function* () { + const dir = yield* tmpdirScoped({ git: true }) + const store = yield* InstanceStore.Service + yield* store.load({ directory: dir }) + yield* failFingerprint + expect(yield* store.configChanged()).toBe(true) + }), + ) + + it.live("detects config edited after bootstrap consumed it", () => + Effect.gen(function* () { + const dir = yield* tmpdirScoped({ git: true }) + const file = path.join(dir, "opencode.json") + yield* Effect.promise(() => fs.writeFile(file, JSON.stringify({ username: "old" }))) + let consumed = "" + yield* setBootstrap( + Effect.gen(function* () { + consumed = yield* Effect.promise(() => fs.readFile(file, "utf8")) + yield* Effect.promise(() => fs.writeFile(file, JSON.stringify({ username: "new" }))) + }), + ) + const store = yield* InstanceStore.Service + yield* store.load({ directory: dir }) + expect(JSON.parse(consumed).username).toBe("old") + expect(yield* store.configChanged()).toBe(true) + }), + ) + it.live("loads instance context", () => Effect.gen(function* () { const dir = yield* tmpdirScoped({ git: true }) @@ -65,9 +134,7 @@ describe("InstanceStore", () => { expect(yield* store.configChanged()).toBe(false) yield* Effect.promise(() => fs.mkdir(path.join(dir, ".opencode", "agent"), { recursive: true })) - yield* Effect.promise(() => - fs.writeFile(path.join(dir, ".opencode", "agent", "reviewer.md"), "review things"), - ) + yield* Effect.promise(() => fs.writeFile(path.join(dir, ".opencode", "agent", "reviewer.md"), "review things")) expect(yield* store.configChanged()).toBe(true) } finally { if (previousHome === undefined) delete process.env.OPENCODE_TEST_HOME diff --git a/packages/opencode/test/server/global-lifecycle.test.ts b/packages/opencode/test/server/global-lifecycle.test.ts index 4cacdbc657ed..0573260dc3ba 100644 --- a/packages/opencode/test/server/global-lifecycle.test.ts +++ b/packages/opencode/test/server/global-lifecycle.test.ts @@ -139,3 +139,12 @@ it.effect("disposes instances when an instance's own config inputs changed", () expect(fx.disposals).toBe(1) }), ) + +it.effect("reloads when permission key order changes precedence", () => + Effect.gen(function* () { + fx.set({ permission: { bash: { "git *": "deny", "*": "allow" } } }) + fx.changeOnDisk({ permission: { bash: { "*": "allow", "git *": "deny" } } }) + expect(yield* reloadIfConfigChanged()).toBe(true) + expect(fx.disposals).toBe(1) + }), +)