fix(coding-agents): stop opencode's install from replacing a JSONC config - #3843
Open
etnperlong wants to merge 5 commits into
Open
fix(coding-agents): stop opencode's install from replacing a JSONC config#3843etnperlong wants to merge 5 commits into
etnperlong wants to merge 5 commits into
Conversation
opencode reads its global config from either `~/.config/opencode/opencode.json` or `~/.config/opencode/opencode.jsonc`, but the installer hardcoded the `.json` name. A user whose config is `opencode.jsonc` therefore got a SECOND config file created beside their real one: their settings in the file they wrote, our plugin entry in a file they never made. `opencodeConfigPath` now probes the same way `kiloConfigPath` does — edit the candidate that already exists, and only create `opencode.json` when neither does. `.jsonc` is probed first because that is the name that carries comments, so a machine holding both most likely keeps the real config there. This is the path resolution only; the strict-JSON parse of a commented file is still broken and is fixed next.
…onfig
The opencode adapter read its config with `readJson`, whose strict `JSON.parse`
rejects the comments and trailing commas real configs carry — and whose `{}`
fallback then meant the whole file was rewritten as just our plugin key. A user
with providers, agent overrides and MCP servers configured lost all of it to an
install that reported success.
It now parses with `parseJsonc` and aborts on a file it cannot understand,
exactly as the kilo adapter already does: a config we merely failed to read must
never be replaced by our own key alone. The backup `writeJson` takes on first
touch was the only reason this was recoverable at all.
Note this applies `parseJsonc` to `opencode.json` too, not just `.jsonc` — the
comments turn up under both names, so the content decides the parser, not the
extension.
Parsing a JSONC config was only half the problem. `writeJson` round-trips the whole object through `JSON.stringify`, which can only emit strict JSON — so even a config we read correctly came back with every comment and trailing comma stripped and its formatting reflowed. For a hand-maintained config that reads as damage regardless of the data surviving. `writeJsonc` computes a minimal text edit with jsonc-parser instead, so anything it did not touch survives byte for byte, CRLF included. It sets or deletes ONE top-level key on purpose: a whole-object write is exactly what forces the re-serialize, and both JSONC callers only ever mutate `plugin`. Comments inside the edited value are still lost, since that value is re-emitted. jsonc-parser is added to tsup's noExternal list. installer.js is staged to ~/.hindsight/coding-agents as dist + skill + package.json and never node_modules, so an external import is unresolvable there — verified by running the built installer from a staged-layout copy, which failed with ERR_MODULE_NOT_FOUND before the change and runs after it. Unused until the next commit, which routes opencode through it.
…ments opencode's adapter now writes through `writeJsonc`, so installing or removing the plugin entry edits the `plugin` key and leaves the rest of the file exactly as the user wrote it — comments, trailing commas and formatting included. Before this it parsed correctly (previous commit) but still round-tripped through JSON.stringify on the way out, so a commented config came back stripped: the data survived, the file the user maintains did not. Uninstall takes the same path, passing `undefined` to drop the key when no other plugin entries remain, which is what the object-delete used to express.
…NC family
kilo had the same split as opencode: a JSONC-aware `parseJsonc` on the way in,
`writeJson` on the way out. So a commented `kilo.jsonc` was read correctly and
then written back stripped — the existing test even asserted it ("comments
dropped, DATA kept"), which is how the loss stayed invisible.
It now writes through `writeJsonc`, and that test asserts the comment survives.
The parity guard sweeps the JSONC hosts instead of trusting a per-harness test:
this bug existed in two adapters at once and would return with the third, since
the sibling that forgets is by construction the one nobody wrote a test for. It
drives a real install and uninstall against a commented config and checks the
file that came out. Verified it fails when either adapter is reverted to
`writeJson`.
Strict-JSON hosts (claude-code, cursor, …) and the TOML/YAML ones deliberately
stay on `writeJson`, and the list says so.
Strix Security ReviewNo security issues found. Updated for Reviewed by Strix |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
opencode reads its global config from either
~/.config/opencode/opencode.jsonor~/.config/opencode/opencode.jsonc. The installer knew only the first name, and read it withreadJson, whoseJSON.parserejects the comments and trailing commas a hand-maintained config carries, and whosecatchreturns{}. My config isopencode.jsonc, soinstall opencodewrote a second config file next to the real one. Had it been named.json, the{}fallback would have gone back to disk carrying only ourpluginkey, taking the providers, MCP servers and agent overrides with it. The<file>.hindsight-backupcopy thatwriteJsontakes on first touch is the only reason a config in that state is recoverable.Five commits, each one step:
opencodeConfigPathprobesopencode.jsoncthenopencode.json, edits whichever already exists, and createsopencode.jsononly when neither does. Same shape askiloConfigPath.parseJsoncand aborts with aSKIPPEDlog on a file it cannot read, rather than falling back to{}. kilo already did this.writeJsoncsets or deletes one top-level key throughjsonc-parser, so text it did not touch survives byte for byte. Parsing was only half of it: a config that read correctly still came back stripped, becausewriteJsonround-trips throughJSON.stringify.writeJsonc.jsonc-parsergoes in tsup'snoExternal.installer.jsis staged to~/.hindsight/coding-agentsas dist + skill + package.json and nevernode_modules, so an external import there cannot resolve; without it, re-runninginstallfrom the staged copy exits withERR_MODULE_NOT_FOUND. I hit that while testing and it is why the dependency is inlined.Comments inside the
pluginarray are still lost, since that value gets re-emitted. Everything outside it is kept.No issue filed for this. I ran into it installing the plugin on my own machine.
Type of change
Test plan
src/installer.test.ts: opencode gains coverage for editing an existing.jsonc, uninstalling from the same file, creating.jsonwhen neither exists, keeping a commented config's other keys, parsing a trailing-comma.json, and leaving an unparseable file alone. I checked these against the pre-fix installer: three of them fail there with the reported symptom.writeJsonmakes it fail, which is how I confirmed it would have caught the kilo half.npm test: 669 passed across 55 files.npx tsc --noEmitandnpm run buildclean../scripts/hooks/lint.shclean.opencode.jsonc(4.7 KB, four providers, ten agent overrides, comments throughout): no competingopencode.json, anddiffshows changes confined to thepluginarray.Notes for the reviewer
parseJsoncis now used foropencode.jsonas well as.jsonc. Comments turn up under both names, so the file content decides the parser rather than the extension. If you would rather keep strict parsing for.json, that is a one-line change, but it leaves the original data loss in place for anyone whose commented config uses that name.writeJsonctakes a single key instead of an object on purpose. A whole-object write is exactly what forces the re-serialize, and both callers only ever touchplugin.writeJson. The parity test lists the JSONC hosts explicitly so a new one has to be added there.OPENCODE_CONFIG_CANDIDATESis exported to mirrorKILO_CONFIG_CANDIDATES. Neither is imported anywhere else.