fix(coding-agents): stop opencode's install from replacing a JSONC config (#3843)

opencode loads its global config from either `~/.config/opencode/opencode.json` or `opencode.jsonc`. The installer knew only the first name and read it with `readJson`, whose strict `JSON.parse` rejects the comments and trailing commas a hand-maintained config carries and whose `catch` returns `{}`. A user whose config is `opencode.jsonc` got a second config file created beside their real one; had it been named `.json`, the `{}` fallback would have been written back carrying only our `plugin` key, taking their providers, MCP servers and agent overrides with it.

Three layers change:

- `opencodeConfigPath` probes `opencode.jsonc` then `opencode.json`, edits whichever exists, and creates `opencode.json` only when neither does — the same shape as `kiloConfigPath`.
- The adapter parses with `parseJsonc` and aborts with a `SKIPPED` log on a config it cannot read, rather than falling back to `{}`.
- `writeJsonc` sets or deletes one top-level key through `jsonc-parser`, so text it did not touch survives byte for byte, and a CRLF file stays CRLF. Parsing was only half the problem: a config that read correctly still came back stripped, because `writeJson` round-trips through `JSON.stringify`.

kilo had the JSONC-aware read and the same lossy write, and its test asserted the loss; it now writes through `writeJsonc` too, with a parity guard sweeping both JSONC hosts through a real install and uninstall.

`jsonc-parser` goes in tsup's `noExternal`: `installer.js` is staged to `~/.hindsight/coding-agents` as dist + skill + package.json and never `node_modules`, so an external import there cannot resolve and re-running `install` from the staged copy would exit with `ERR_MODULE_NOT_FOUND`.

Comments inside the `plugin` array are still lost, since that value is re-emitted; everything outside it is kept.

Author: etnperlong

hindsight-integrations/coding-agents/package-lock.json

@@ -75,6 +75,7 @@
   "dependencies": {
     "@modelcontextprotocol/sdk": "^1.29.0",
     "@vectorize-io/hindsight-all": "^0.8.6",
+    "jsonc-parser": "^3.3.1",
     "zod": "^4.4.3"
   },
   "devDependencies": {

hindsight-integrations/coding-agents/package.json

@@ -11,7 +11,7 @@ import {
 import { tmpdir } from "node:os";
 import { dirname, join } from "node:path";
 import { pathToFileURL } from "node:url";
-import { INSTALLERS, MARKER, run, type InstallCtx } from "./installer";
+import { INSTALLERS, MARKER, parseJsonc, run, type InstallCtx } from "./installer";
 
 // Every test gets a FRESH temp dir as ctx.home (never the real $HOME) and a stubbed
 // claudeMcp so the real `claude` CLI is never executed. run() is always called with
@@ -463,9 +463,13 @@ describe("kilo installer", () => {
     writeFileSync(jsonc, '{\n  // my config\n  "$schema": "https://app.kilo.ai/config.json"\n}\n');
     run(["install", "kilo"], ctx);
     expect(existsSync(join(kiloDir(ctx), "kilo.json"))).toBe(false);
-    const cfg = readJson(jsonc);
+    const text = readFileSync(jsonc, "utf8");
+    const cfg = parseJsonc(text)!;
     expect(cfg.plugin).toEqual([entryOf(ctx)]);
-    expect(cfg.$schema).toBe("https://app.kilo.ai/config.json"); // comments dropped, DATA kept
+    expect(cfg.$schema).toBe("https://app.kilo.ai/config.json");
+    // The comment used to be dropped here: the read was JSONC-aware but the write re-serialized
+    // the parsed object, so a commented config came back stripped.
+    expect(text).toContain("// my config");
   });
 
   it("refuses to clobber a config it cannot parse", () => {
@@ -488,7 +492,124 @@ describe("kilo installer", () => {
 });
 
 describe("opencode installer", () => {
-  const cfgPath = (ctx: InstallCtx) => join(ctx.home, ".config", "opencode", "opencode.json");
+  const ocDir = (ctx: InstallCtx) => join(ctx.home, ".config", "opencode");
+  const cfgPath = (ctx: InstallCtx) => join(ocDir(ctx), "opencode.json");
+
+  // opencode loads `opencode.json` OR `opencode.jsonc` from ~/.config/opencode. Creating the
+  // .json variant next to an existing .jsonc leaves the user with two configs — their settings in
+  // one, our plugin entry in the other — so the install has to edit whichever already exists.
+  it("edits an existing opencode.jsonc instead of creating a competing opencode.json", () => {
+    const ctx = makeCtx();
+    const jsonc = join(ocDir(ctx), "opencode.jsonc");
+    mkdirSync(ocDir(ctx), { recursive: true });
+    writeFileSync(jsonc, '{\n  "$schema": "https://opencode.ai/config.json"\n}\n');
+    run(["install", "opencode"], ctx);
+    expect(existsSync(cfgPath(ctx))).toBe(false);
+    expect(readJson(jsonc).plugin).toEqual([ctx.pkgRoot]);
+  });
+
+  it("uninstall edits the same opencode.jsonc the install wrote to", () => {
+    const ctx = makeCtx();
+    const jsonc = join(ocDir(ctx), "opencode.jsonc");
+    mkdirSync(ocDir(ctx), { recursive: true });
+    writeFileSync(jsonc, "{}\n");
+    run(["install", "opencode"], ctx);
+    run(["uninstall", "opencode"], ctx);
+    expect(readJson(jsonc).plugin).toBeUndefined();
+    expect(existsSync(cfgPath(ctx))).toBe(false);
+  });
+
+  it("creates opencode.json when neither candidate exists", () => {
+    const ctx = makeCtx();
+    run(["install", "opencode"], ctx);
+    expect(existsSync(cfgPath(ctx))).toBe(true);
+    expect(existsSync(join(ocDir(ctx), "opencode.jsonc"))).toBe(false);
+  });
+
+  // The reported data loss: a commented config went through strict JSON.parse, which threw, and
+  // readJson's {} fallback meant the whole file was rewritten as just our plugin key. Every
+  // provider, agent override and MCP entry in it was gone.
+  it("keeps the rest of a commented config instead of replacing it with just our key", () => {
+    const ctx = makeCtx();
+    const jsonc = join(ocDir(ctx), "opencode.jsonc");
+    mkdirSync(ocDir(ctx), { recursive: true });
+    writeFileSync(
+      jsonc,
+      `{\n  // where memory lives\n  "share": "disabled",\n  "provider": {\n    "openai": { "name": "gw" },\n  },\n}\n`
+    );
+    run(["install", "opencode"], ctx);
+    const cfg = parseJsonc(readFileSync(jsonc, "utf8"))!;
+    expect(cfg.plugin).toEqual([ctx.pkgRoot]);
+    expect(cfg.share).toBe("disabled"); // survived — this is what used to be wiped
+    expect(cfg.provider).toEqual({ openai: { name: "gw" } });
+  });
+
+  // Reading the file correctly is only half of it: re-serializing the parsed object would drop
+  // every comment the user wrote, so a config that survived would still come back damaged.
+  it("leaves the user's comments and formatting in place", () => {
+    const ctx = makeCtx();
+    const jsonc = join(ocDir(ctx), "opencode.jsonc");
+    mkdirSync(ocDir(ctx), { recursive: true });
+    writeFileSync(
+      jsonc,
+      `{\n  /** Providers **/\n  "provider": {\n    // via the gateway\n    "openai": { "name": "gw" },\n  },\n}\n`
+    );
+    run(["install", "opencode"], ctx);
+    const text = readFileSync(jsonc, "utf8");
+    expect(text).toContain("/** Providers **/");
+    expect(text).toContain("// via the gateway");
+    expect(text).toContain('"openai": { "name": "gw" },');
+  });
+
+  it("uninstall drops the plugin key without reformatting the file", () => {
+    const ctx = makeCtx();
+    const jsonc = join(ocDir(ctx), "opencode.jsonc");
+    mkdirSync(ocDir(ctx), { recursive: true });
+    writeFileSync(jsonc, `{\n  /** mine **/\n  "share": "disabled",\n}\n`);
+    run(["install", "opencode"], ctx);
+    run(["uninstall", "opencode"], ctx);
+    const text = readFileSync(jsonc, "utf8");
+    expect(text).toContain("/** mine **/");
+    expect(parseJsonc(text)).toEqual({ share: "disabled" });
+  });
+
+  // Trailing commas are as common as comments in a hand-written config, and JSON.parse rejects
+  // both — so a .json file carrying them hit the very same wipe.
+  it("parses a trailing-comma opencode.json rather than clobbering it", () => {
+    const ctx = makeCtx();
+    mkdirSync(ocDir(ctx), { recursive: true });
+    writeFileSync(
+      cfgPath(ctx),
+      `{\n  "model": "openai/gpt-5",\n  "plugin": [\n    "other",\n  ],\n}\n`
+    );
+    run(["install", "opencode"], ctx);
+    // Read back with the JSONC parser: the trailing commas are PRESERVED by the write, so the
+    // result is still not strict JSON — which is the point.
+    const cfg = parseJsonc(readFileSync(cfgPath(ctx), "utf8"))!;
+    expect(cfg.model).toBe("openai/gpt-5");
+    expect(cfg.plugin).toEqual(["other", ctx.pkgRoot]);
+  });
+
+  it("refuses to clobber a config it cannot parse", () => {
+    const ctx = makeCtx();
+    mkdirSync(ocDir(ctx), { recursive: true });
+    const broken = '{ "provider": { unquoted } }';
+    writeFileSync(cfgPath(ctx), broken);
+    const logs: string[] = [];
+    ctx.log = (m) => logs.push(m);
+    run(["install", "opencode"], ctx);
+    expect(readFileSync(cfgPath(ctx), "utf8")).toBe(broken);
+    expect(logs.join("\n")).toContain("SKIPPED");
+  });
+
+  it("uninstall leaves an unparseable config untouched", () => {
+    const ctx = makeCtx();
+    mkdirSync(ocDir(ctx), { recursive: true });
+    const broken = '{ "provider": { unquoted } }';
+    writeFileSync(cfgPath(ctx), broken);
+    run(["uninstall", "opencode"], ctx);
+    expect(readFileSync(cfgPath(ctx), "utf8")).toBe(broken);
+  });
 
   it("install adds ctx.pkgRoot to the plugin array exactly once, even across reinstalls", () => {
     const ctx = makeCtx();
@@ -758,6 +879,42 @@ describe("MCP registrations name the calling harness", () => {
   });
 });
 
+/**
+ * A JSONC-configured host must survive the install with its comments intact.
+ *
+ * Swept over the family rather than asserted per harness: opencode and kilo each read with
+ * `parseJsonc` and then wrote with `writeJson`, which re-serializes and strips exactly what the
+ * JSONC-aware read preserved. The sibling that forgets is by construction the one nobody wrote a
+ * test for, so this drives the real install and checks the file that came out.
+ */
+describe("JSONC hosts keep their comments through an install", () => {
+  // Each entry is the config file the harness edits when it already exists, with the comment we
+  // expect to still be there afterwards. Hosts absent from this list are strict-JSON by design
+  // (claude-code's settings.json, cursor's hooks.json, …) or not JSON at all (grok/dsh: TOML/YAML).
+  const JSONC_HOSTS: { harness: string; relPath: string[] }[] = [
+    { harness: "opencode", relPath: [".config", "opencode", "opencode.jsonc"] },
+    { harness: "kilo", relPath: [".config", "kilo", "kilo.jsonc"] },
+  ];
+
+  it.each(JSONC_HOSTS)("$harness", ({ harness, relPath }) => {
+    const ctx = makeCtx();
+    const path = join(ctx.home, ...relPath);
+    mkdirSync(dirname(path), { recursive: true });
+    writeFileSync(path, `{\n  // keep me\n  "share": "disabled",\n}\n`);
+
+    expect(run(["install", harness], ctx)).toBe(0);
+    const afterInstall = readFileSync(path, "utf8");
+    expect(afterInstall).toContain("// keep me");
+    expect(parseJsonc(afterInstall)!.share).toBe("disabled");
+    expect(parseJsonc(afterInstall)!.plugin).toHaveLength(1);
+
+    expect(run(["uninstall", harness], ctx)).toBe(0);
+    const afterUninstall = readFileSync(path, "utf8");
+    expect(afterUninstall).toContain("// keep me");
+    expect(parseJsonc(afterUninstall)).toEqual({ share: "disabled" });
+  });
+});
+
 /**
  * `all` is an explicit target rather than the default for a bare command: wiring every detected
  * agent rewrites a lot of a machine's config and should not happen by accident.

hindsight-integrations/coding-agents/src/installer.test.ts

@@ -38,6 +38,7 @@ import { homedir, tmpdir } from "node:os";
 import { isatty } from "node:tty";
 import { dirname, join } from "node:path";
 import { fileURLToPath, pathToFileURL } from "node:url";
+import { applyEdits, modify } from "jsonc-parser";
 import { HOOK_HARNESSES, type HookHarnessName } from "./harness/hook-lifecycle";
 import { importLocalHistory } from "./core/history";
 import { detectLlm, hasRustToolchain, hasUvx, type LlmChoice } from "./core/daemon";
@@ -96,7 +97,10 @@ function readJson(path: string): Record<string, any> {
 }
 
 /**
- * Parse a JSONC file (JSON with comments), as Kilo's `kilo.jsonc` may be.
+ * Parse a JSONC file (JSON with comments), as Kilo's `kilo.jsonc` and opencode's `opencode.jsonc`
+ * may be. Applied to `opencode.json` too: comments and trailing commas turn up in the wild under
+ * that name as well, and the host reads it either way — so what decides the parser here is the
+ * content the installer might meet, not the extension.
  *
  * Returns null — NOT {} — when the file exists but can't be parsed. readJson's {} fallback is safe
  * for a strict-JSON host (an unparseable file is a broken file), but here a config we merely failed
@@ -118,10 +122,46 @@ export function parseJsonc(text: string): Record<string, any> | null {
 
 function writeJson(path: string, value: unknown): void {
   mkdirSync(dirname(path), { recursive: true });
+  backupOnce(path);
+  writeFileSync(path, JSON.stringify(value, null, 2) + "\n");
+}
+
+/** The first time we touch an existing file, keep a copy of what the user had. */
+function backupOnce(path: string): void {
   if (existsSync(path) && !existsSync(`${path}.hindsight-backup`)) {
     copyFileSync(path, `${path}.hindsight-backup`);
   }
-  writeFileSync(path, JSON.stringify(value, null, 2) + "\n");
+}
+
+/**
+ * Set (or, with `undefined`, delete) ONE top-level key, leaving the rest of the file's text alone.
+ *
+ * `writeJson` round-trips through `JSON.stringify`, which can only emit strict JSON — so writing a
+ * JSONC host's config with it silently strips every comment and trailing comma the user wrote, and
+ * reflows their formatting. Parsing the file was only half the problem: even a config we read
+ * correctly came back stripped. `jsonc-parser` computes a minimal text edit instead, so everything
+ * we did not touch survives byte for byte.
+ *
+ * Limited to a single key on purpose. A whole-object write is what forces a re-serialize, and both
+ * callers only ever mutate `plugin`; a general "merge this object" helper could not preserve
+ * anything. Comments INSIDE the edited value are still lost — the array is re-emitted — but the
+ * rest of the document, which is all a user notices, is not.
+ */
+function writeJsonc(path: string, key: string, value: unknown): void {
+  const text = existsSync(path) ? readFileSync(path, "utf8") : "";
+  const edits = modify(text, [key], value, {
+    formattingOptions: {
+      tabSize: 2,
+      insertSpaces: true,
+      // Keep a CRLF file CRLF: rewriting every line ending would turn a one-line change into a
+      // whole-file diff for anyone on Windows.
+      eol: text.includes("\r\n") ? "\r\n" : "\n",
+    },
+  });
+  const next = applyEdits(text, edits);
+  mkdirSync(dirname(path), { recursive: true });
+  backupOnce(path);
+  writeFileSync(path, next.endsWith("\n") ? next : `${next}\n`);
 }
 
 /** Hook-array merge for claude/codex-style files: drop our old entries, append the new one. */
@@ -243,26 +283,51 @@ function runClinePlugin(args: string[]): boolean {
   }
 }
 
+/**
+ * opencode reads its global config from EITHER `opencode.json` or `opencode.jsonc` under
+ * `~/.config/opencode`; both names are documented. So the installer must edit the one that is
+ * already there: hardcoding `opencode.json` meant a user whose config is `opencode.jsonc` got a
+ * SECOND config file created next to their real one — their settings in the file they wrote, our
+ * plugin entry in a file they never made.
+ *
+ * `.jsonc` is probed first because that is the name that carries comments, and a machine holding
+ * both is far likelier to have the commented one as the real config.
+ */
+export const OPENCODE_CONFIG_CANDIDATES = ["opencode.jsonc", "opencode.json"];
+
+function opencodeConfigPath(c: InstallCtx): string {
+  const dir = join(c.home, ".config", "opencode");
+  const existing = OPENCODE_CONFIG_CANDIDATES.map((f) => join(dir, f)).find((p) => existsSync(p));
+  return existing ?? join(dir, "opencode.json");
+}
+
 const opencode: HarnessInstaller = {
   name: "opencode",
   detect: (c) => onPath("opencode") || existsSync(join(c.home, ".config", "opencode")),
   install(c) {
-    const path = join(c.home, ".config", "opencode", "opencode.json");
-    const cfg = readJson(path);
-    const plugins: string[] = Array.isArray(cfg.plugin) ? cfg.plugin : [];
-    cfg.plugin = [...plugins.filter((p) => !String(p).includes(MARKER)), c.pkgRoot];
-    writeJson(path, cfg);
+    const path = opencodeConfigPath(c);
+    let cfg: Record<string, any> = {};
+    if (existsSync(path)) {
+      const parsed = parseJsonc(readFileSync(path, "utf8"));
+      if (!parsed) {
+        c.log?.(`opencode: SKIPPED — could not parse ${path}; add the plugin entry manually`);
+        return;
+      }
+      cfg = parsed;
+    }
+    const plugins: unknown[] = Array.isArray(cfg.plugin) ? cfg.plugin : [];
+    // writeJsonc, not writeJson: this config routinely carries comments, and re-serializing the
+    // parsed object would strip every one of them even though the read succeeded.
+    writeJsonc(path, "plugin", [...plugins.filter((p) => !String(p).includes(MARKER)), c.pkgRoot]);
     c.log?.(`opencode: plugin registered in ${path}`);
   },
   uninstall(c) {
-    const path = join(c.home, ".config", "opencode", "opencode.json");
+    const path = opencodeConfigPath(c);
     if (!existsSync(path)) return;
-    const cfg = readJson(path);
-    if (Array.isArray(cfg.plugin)) {
-      cfg.plugin = cfg.plugin.filter((p: string) => !String(p).includes(MARKER));
-      if (!cfg.plugin.length) delete cfg.plugin;
-      writeJson(path, cfg);
-    }
+    const cfg = parseJsonc(readFileSync(path, "utf8"));
+    if (!cfg || !Array.isArray(cfg.plugin)) return;
+    const kept = cfg.plugin.filter((p: unknown) => !String(p).includes(MARKER));
+    writeJsonc(path, "plugin", kept.length ? kept : undefined);
     c.log?.("opencode: plugin entry removed");
   },
 };
@@ -348,18 +413,18 @@ const kilo: HarnessInstaller = {
     }
     const entry = pathToFileURL(join(c.dist, "kilo.js")).href;
     const plugins: unknown[] = Array.isArray(cfg.plugin) ? cfg.plugin : [];
-    cfg.plugin = [...plugins.filter((p) => !String(p).includes(MARKER)), entry];
-    writeJson(path, cfg);
+    // writeJsonc for the same reason the read uses parseJsonc: kilo.jsonc is a commented file, and
+    // re-serializing the parsed object would strip what we just took care to read.
+    writeJsonc(path, "plugin", [...plugins.filter((p) => !String(p).includes(MARKER)), entry]);
     c.log?.(`kilo: plugin registered in ${path}`);
   },
   uninstall(c) {
     const path = kiloConfigPath(c);
     if (!existsSync(path)) return;
     const cfg = parseJsonc(readFileSync(path, "utf8"));
     if (!cfg || !Array.isArray(cfg.plugin)) return;
-    cfg.plugin = cfg.plugin.filter((p: unknown) => !String(p).includes(MARKER));
-    if (!cfg.plugin.length) delete cfg.plugin;
-    writeJson(path, cfg);
+    const kept = cfg.plugin.filter((p: unknown) => !String(p).includes(MARKER));
+    writeJsonc(path, "plugin", kept.length ? kept : undefined);
     c.log?.("kilo: plugin entry removed");
   },
 };

hindsight-integrations/coding-agents/src/installer.ts

@@ -58,5 +58,13 @@ export default defineConfig({
   // hindsight-all (the local-daemon lifecycle manager) is inlined for the same reason: hooks are
   // wired by absolute path to ONE dist file and never load the package's node_modules. It has zero
   // dependencies of its own, so inlining costs almost nothing.
-  noExternal: [/^@modelcontextprotocol\/sdk/, /^zod/, /^@vectorize-io\/hindsight-all/],
+  // jsonc-parser likewise: installer.js is staged to ~/.hindsight/coding-agents as dist + skill +
+  // package.json — never node_modules — so an external import there is unresolvable, and re-running
+  // `install` from the staged copy (the upgrade path) dies with ERR_MODULE_NOT_FOUND.
+  noExternal: [
+    /^@modelcontextprotocol\/sdk/,
+    /^zod/,
+    /^@vectorize-io\/hindsight-all/,
+    /^jsonc-parser/,
+  ],
 });