fix: edit an existing opencode.jsonc instead of creating a sibling opencode.json - #59
Conversation
…encode.json opencode reads and merges both opencode.json and opencode.jsonc (JSONC with comments and trailing commas, at the global ~/.config/opencode location and in the project root). setup wrote opencode.json unconditionally, so a user with an opencode.jsonc ended up with two config files. setup now targets the existing variant, preferring opencode.jsonc — the same order opencode itself uses when it edits its config — and edits it JSONC-aware: the entry is inserted textually so comments and formatting survive, and when the file can't be edited safely the outcome prints the exact snippet to add by hand instead of clobbering or duplicating. Nothing changes when neither variant exists: opencode.json is created as before, and repo-local writes remain behind the explicit --project flag. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017iuTu1C4JF3WLBodoKktHy
Per review: never print a snippet for the user to copy. The upsert now edits any valid JSONC in place using jsonc-parser — the library opencode itself edits its config with — so comments and the formatting of existing members survive, and the entry is inserted whether or not an mcp block exists. Only a genuinely malformed file (one opencode couldn't parse either) is refused, as a clear skipped outcome naming the file. The WriteOutcome.snippet plumbing is gone. The bundle aliases jsonc-parser to its ESM build; the UMD default passes require into its factory, which esbuild can't follow. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017iuTu1C4JF3WLBodoKktHy
There was a problem hiding this comment.
LGTM: the opencode setup now edits an existing opencode.jsonc/opencode.json in place via jsonc-parser, never creates a duplicate, and never falls back to a copy-paste snippet — verified against the live code, not just the diff.
Procedure followed (per REVIEW.md): full diff read hunk-by-hunk, callers of every changed export grepped (upsertJsoncEntry, opencodeConfigFile, WriteOutcome.needsManualStep), every PR-body claim checked against code and, where practical, against real execution; npm ci, npm run codegen, npm run typecheck, npm run lint, npm run test run locally on Node 22.x (matches one CI leg; CI already covers 20.x/24.x and all three are green), plus npm run build since the diff touches build.ts/packaging.
Verified claims (ran the built bundle end-to-end against real fixtures, not just unit tests):
- Existing
opencode.jsoncwith comments + trailing commas → MCP entry inserted, all comments and trailing commas preserved, new entry written first, siblingopencode.jsonnever created. - Only
opencode.json(plain JSON) exists → edited in place, no.jsonccreated. - Neither exists → creates
opencode.jsonas before (no behavior change for the common case). - Both exist →
.jsoncedited,.jsonleft untouched (matches opencode's ownglobalConfigFile()precedence per the PR's cited source). - Genuinely malformed JSONC (
{ "theme":) → file left byte-for-byte untouched, outcome isskippedwith a link to the manual-registration doc — confirmed nothing is ever printed as a copy-paste snippet, per the requirement in the linked Slack thread. jsonc-parser'smainfield really does point at the UMD build (lib/umd/main.js, which passesrequireinto its factory) whilemodulepoints at the ESM build (lib/esm/main.js) that the newaliastargets —npm run buildbundles cleanly and the resultingdist/polylane.mjsruns the scenarios above correctly.
Code review notes:
src/agents/registry.ts:upsertJsoncEntrycorrectly special-cases "file doesn't exist" (create, unchanged from before), "malformed" (skip,needsManualStep: true, no manual-step regression vs. the plain-JSONupsertJsonEntry), "key already present" (unchanged, byte-for-byte, tested), and "needs insertion" (modify+applyEdits, insertion index forced to 0 so existing members keep their exact bytes). ThereachedLeafwalk only rejects non-object intermediate nodes and otherwise defers entirely tojsonc-parser's own path-creation — good, that's the right layer to own JSONC structural edits rather than hand-rolling it.opencodeConfigFilemirrors opencode's own jsonc-then-json resolution order exactly (confirmed above).- Test coverage is thorough: heavy-comment files, trailing commas in nested blocks, sibling-server preservation with correct ordering, single-quoted-string malformed handling (matches opencode's own JSONC strictness), dry-run no-ops, and both user/project agent-table wiring paths.
test/setup.test.tspasses 318/318 locally (0 fail/skipped) — note the PR body says "323 passing"; the discrepancy is almost certainly just other unrelated tests added tomainsince this branch was cut, not a regression here (confirmed nothing in this diff's own new tests fails). - Scope is tight: 7 files, only touches the opencode path; no other agent's behavior changes. Backward compatible —
jsonc-parser's JSONC parsing is a strict superset of JSON, so existing plain-opencode.jsonusers get identical behavior to before, just via the new code path.
Nits (non-blocking):
isJsonObject/skip('existing file is not a JSON object')guards the root value, but I didn't see a dedicated test for a root that parses to a non-object (e.g. a top-level JSONC array or bare string) — the plain-JSONupsertJsonEntryhas the equivalent case tested; wouldn't block on it since the guard clause itself is straightforward and mirrored from the JSON version.- The PR body's "323 passing" vs. the 318 I see locally is worth a quick sanity check by the author next time a description is written, just so the number stays trustworthy for future reviewers skimming the description.
CI: all 3 check runs green (Test & typecheck on 20.x/22.x/24.x), no flakes to attribute.
Generated by Claude Code
There was a problem hiding this comment.
Auto-approved: Claude reviewed this PR and posted an LGTM verdict (see its review). A repo admin enabled this via the auto-approve workflow.
Generated by Claude Code
Requested by boris · Slack thread
Before: a user who keeps their opencode config in
opencode.jsoncran the installer (orpolylane setup) and found a brand-new siblingopencode.jsonnext to it — two config files, and their own one untouched.After:
setuplooks foropencode.jsonc/opencode.jsonand edits the one that exists in place — thepolylaneMCP entry is inserted with comments, trailing commas, and the formatting of existing members left intact. Only when no opencode config exists at all does it createopencode.jsonas before. Nothing is ever printed for the user to copy: any valid JSONC is edited directly; only a genuinely malformed file (one opencode itself couldn't parse) is refused, as a clear skipped outcome naming the file.This makes the opencode MCP-config write variant-aware and JSONC-aware instead of unconditionally creating
opencode.json.How. Verified against opencode's docs and source before encoding anything:
opencode.jsonandopencode.jsoncwhen present, merged — global from~/.config/opencode/and project files found walking up from the cwd to the git root (docs, config.ts, paths.ts).opencode.jsonc→opencode.json(globalConfigFile()in config.ts).opencodeConfigFile()mirrors that order at both the user and project level.jsonc-parser(modify+applyEdits) — the same zero-dependency library opencode uses to patch its own config — so any valid JSONC edits cleanly and comment-preserving, with the new entry inserted first so existing members keep their exact bytes. The bundle aliases it to its ESM build (the UMD default passesrequireinto its factory, which esbuild can't follow).polylane setup(what the curl installer runs) only writes user-level files under the home directory — the global~/.config/opencode/location opencode documents. Repo-local files are written only behind the explicit--projectflag, which stays the consent gate; non-interactive runs keep the user-level default and never prompt.Checks:
npm run typecheck,npm run lint,npm run test(318 passing, including heavy-comment + trailing-comma edits, nestedmcpblocks, single-quote files treated as malformed like opencode treats them, unchanged/skip paths, and both agent levels preferring an existingopencode.jsonc) andnpm run buildwith an end-to-endsetup --agent opencoderun against the produced bundle.Companion installer-side PR: coreplanelabs/polylanedotcom#137.