Repository navigation
sync-models: naive parseJsonc line-stripper breaks on block comments and string contents — multi-line /* */ or a ' // ' string fails the sync, a ', ]' string is silently corrupted #704
Description
Problem
bin/sync-models.mjs reads the OpenCode config with a naive line-based JSONC stripper, parseJsonc (lines 224–231), even though the same file already contains a correct comment-aware parser, parseJsoncTree (line 238). parseJsonc:
- drops only lines that start with
//or/*, - strips everything after the first whitespace +
//on each remaining line, whether or not it is inside a string, - removes trailing commas with a whole-text regex (
/,\s*([}\]])/g) that also matches inside strings.
Three verified failure modes follow (probe below, run against the verbatim function extracted from current main b382bfb8):
- A multi-line block comment makes the sync fail outright. Only the line starting with
/*is dropped; the continuation lines remain andJSON.parsethrows.main()catches this at the config read (lines 391–398) and exits 1 — the sync-models service fails on every run for a config OpenCode itself accepts (block comments are legal JSONC). - A string containing
//makes the sync fail outright. The line stripper cuts the string at the comment-like sequence:{"note": "docs // see guide"}is truncated mid-string andJSON.parsethrows "Unterminated string". - A string containing
, ]or, }is silently corrupted.{"note": "a, ] b"}parses successfully as{"note": "a] b"}— the comma is deleted from the value with no error. The objectparseJsoncreturns is the read model the sync decides its adds/updates/removals from (line 393), so a corrupted string can drive wrong edit decisions; the post-edit guard at lines 642–643 calls the same function, so it cannot catch this either.
Controls that work today and must keep working: a URL value ("https://api.example.com/v1" — the // is not preceded by whitespace), ordinary // line comments, and real trailing commas.
bin/sync-models.mjs also has no test file, unlike every sibling in bin/ (agy-proxy, antigravity-ls-proxy, clef-proxy, codex-proxy, design-code-proxy, muse-proxy and proxy-port all have *.test.mjs).
Evidence
bin/sync-models.mjs on current main (b382bfb8):
function parseJsonc(text) {
const cleanLines = text.split('\n').filter(l => {
const s = l.trim();
return !s.startsWith('//') && !s.startsWith('/*');
}).map(l => l.replace(/\s+\/\/.*$/, ''));
const cleaned = cleanLines.join('\n').replace(/,\s*([}\]])/g, '$1');
return JSON.parse(cleaned);
}Probe (function extracted verbatim with sed -n '224,231p', run under Node):
blockComment => THROWS Expected property name or '}' in JSON at position 2
stringWithCommentLike => THROWS Unterminated string in JSON at position 14
silentCorrupt => OK {"note":"a] b","a":1} (input was "a, ] b")
urlControl => OK {"baseURL":"https://api.example.com/v1","a":1}
trailingCommaControl => OK {"a":1}
Inputs: block comment = {\n/* multi\nline comment */\n"a": 1\n}; string cases as shown in the problem section.
Dedup: gh issue list --search "sync-models" returns only #591 (closed, Nix store config-path resolution) — no issue covers the parser. Cross-repo: sync-models exists only in this repo (no copy in the other three clones; OpenChamber's parseJsonc is the jsonc-parser library import in its VS Code package, unrelated code).
Why it matters
sync-models runs as a service against Drew's real ~/.config/opencode/opencode.jsonc, a hand-edited JSONC file. One block comment or one string with // in it stops model syncing entirely with a parse error that points at the config, not the parser; the , ] case is worse — it changes a config value silently and lets the sync act on the changed value.
Acceptance criteria
- The config read no longer uses the line-based stripper: parse the config with the file's own comment-aware machinery (
parseJsoncTree, deriving the value from the recorded tree) or an equivalent string-aware JSONC parse, and use the same parse for the post-edit guard at lines 642–643. - A config with a multi-line
/* … */block comment parses to the same object as the same config without the comment. - A string containing
//,, ]or, }round-trips byte-for-byte through the config parse. - Existing good cases keep working:
//line comments (whole-line and trailing), real trailing commas, URL values. - New
bin/sync-models.test.mjs(the script's first test file) pins the cases above; the parsing helpers need an export-for-test seam or equivalent, following the sibling proxies' test pattern. -
node --check bin/sync-models.mjspasses and a--dry-runagainst a fixture config exercising those cases completes without a parse error.
Activity
dezren39 commentedon Oct 26, 2026
Intake review
Still live (verified this run, fleet monitor):
bin/sync-models.mjson current main still carries the naiveparseJsoncverbatim at lines 224–231 (line-start comment filter +\s+//strip + whole-text trailing-comma regex), alongside the correctparseJsoncTreeat line 238 — and there is still nobin/sync-models.test.mjs(contents API 404), unlike every sibling proxy script.Related: queued for triage as pilot task 1071 (
queue/queuedis on this issue; this enrichment does not change its queue state). Sibling test-gap issues from the same review round: #688, #692, #694.Notes / risks: failure mode 3 (silent string corruption feeding sync edit decisions) is the dangerous one — a wrong read model can drive wrong config edits, and the post-edit guard calls the same broken function. Modes 1–2 fail loud; mode 3 does not.
Suggested approach: make
parseJsoncdelegate toparseJsoncTree(or replace its call sites), then add the missing test file pinning the three failure modes plus the working controls (URL values, line comments, real trailing commas).Labels normalized by the fleet monitor: kind/bug, priority/normal, effort/s.
dezren39 commentedon Oct 27, 2026
Triage complete — queue worker (Lane A), task 1071. All three failure modes reproduced on current main (ce1acfb), probing the verbatim
parseJsoncextracted frombin/sync-models.mjs:224-231:/* multi\n line\n comment */inside the config objectExpected property name or '}' in JSON at position 7. Only the/*line is dropped; interior lines survive as garbage. Inmain()(:391-398) this is caught and the sync exits 1.//"url": "https://example.com // path"Bad control character in string literal. The line stripper cuts the string at//, unterminating it. Sync exits 1., ]"note": "x, ]"{"note": "x]"}: the trailing-comma regex/,\s*([}\]])/gfires inside the string and eats,.//line commentRecommendation: replace the reading parse with the comment-aware scanner that already exists in the same file — with one materialization caveat.
parseJsoncTree(:238) parses all three cases correctly in the same probe (itsskip()is block/line-comment aware andreadStringis escape aware), BUT it returns a span tree for comment-preserving edits, not a plain value: object members carry key/value spans, scalars are spans, and the array branch records onlystart/end. So the fix is not a one-line swap at :393. Sound shapes for the fix: (a) extend the tree parser to materialize plain values (JSON.parse each scalar span; recurse objects; arrays need element values retained), or (b) reuse the scanner logic to strip comments string-safely, thenJSON.parse(keeping the legitimate trailing-comma tolerance). Guard behaviour to preserve:main()catches the parse error and exits 1 with[sync-models] Error parsing …— that stays; what changes is that legal configs stop tripping it, and string contents stop being rewritten. A fix packet has been enqueued (small_fix, this queue) — no parser change landed by this triage, per the packet.9 remaining items
dezren39 commentedon Oct 27, 2026
🤖 Check-in — fleet monitor
dezren39 commentedon Oct 27, 2026
🤖 Agent note · handed off
queue/queued+status/needs-triage. Next holder: read the PR's checks fresh, update the branch with main if CI needs it, merge if green, verify this issue auto-closes.Merge pull request #1236 from developing-today/queue/1245-704-parsejsonc