Repository navigation
provider-catalog-sync: stripJsonc's trailing-comma pass is not string-aware — a comma before } or ] inside a string literal is silently deleted #3430
Description
provider-catalog-sync: stripJsonc's trailing-comma pass is not string-aware — a comma before } or ] inside a string literal is silently deleted
Evidence (verified this run, main a17ae13)
bin/provider-catalog-sync.mjs:
:88-122—stripJsonccarefully tracks string state (instr/esc) while stripping//and/* */comments, so comment markers inside strings survive.:121— the final trailing-comma fix is a plain regex over the whole output, with no string awareness:return out.join('').replace(/,(\s*[}\]])/g, '$1');. A comma that sits immediately before a}or]inside a string literal matches the pattern and is deleted.:125-127—parseJsoncisJSON.parse(stripJsonc(text)), so the corrupted value is what every caller sees. There is no error: the parse succeeds with the wrong string.
Probe (laptop, ~/git/fleet/probe-2026-11-01-laneB-jsoncstr/; real provider-catalog-sync.mjs copied from the laptop clone at the same HEAD, sha256 08e742c0… matched to sandbox main a17ae136; node probe.mjs, log run.log):
| Input | parseJsonc result |
Verdict |
|---|---|---|
{"a": 1,} (control) |
{"a":1} |
correct — real trailing comma removed |
// hi\n{"a": [1, 2,], "b": {"c": 3,}} (control) |
{"a":[1,2],"b":{"c":3}} |
correct |
{"url": "https://example.com//x"} (control) |
url unchanged | correct — comment pass is string-aware |
{"note": "hello, world"} (control) |
note unchanged | correct |
{"note": "x,}", "n": 1} |
{"note":"x}","n":1} |
comma deleted inside the string |
{"note": "a,]", "n": 1} |
{"note":"a]","n":1} |
comma deleted |
{"note": "a, }", "n": 1} |
{"note":"a }","n":1} |
comma deleted |
{"list": ["x, ]", "y"]} |
{"list":["x ]","y"]} |
comma deleted |
nested {"provider":{"p":{"models":{"m":{"variants":{"v":{"headers":"a,}"}}}}}}} |
headers becomes "a}" |
comma deleted at depth |
stripJsonc('{"note": "x,}", "n": 1}') returns {"note": "x}", "n": 1} — the corruption is in the stripper itself, before JSON.parse.
Why it matters
runSync reads the live OpenCode config through parseJsonc (:462-465). Keys are unaffected, so enumeration and classification look right, but values are silently wrong wherever a config string ends in ,} / ,] / , } / , ] — e.g. a variant header, a prompt fragment, or an option value cloned by makeEntry (:404-439), which deep-clones the nearest sibling's variants from the parsed config and writes them into the new catalog entry inserted into the file. The corrupted string is then committed to the config as if it were the operator's own text. assertCatalogOnly (:619) cannot catch it: it compares parseJsonc(before) with parseJsonc(after), and both sides go through the same lossy stripper. The existing test at bin/provider-catalog-sync.test.mjs:45 pins comments + trailing commas but has no case with a ,+bracket sequence inside a string.
Acceptance criteria
parseJsoncreturns string values byte-for-byte: a comma inside a string literal is never removed, including,},,],, },, ]and the same shapes at any nesting depth.- Real trailing commas (outside strings) in objects and arrays are still removed, and comment stripping stays string-aware (the current controls keep passing).
- The trailing-comma removal is done in the same string-aware pass as comment stripping (or an equivalent state machine) — not as a whole-text regex.
- Tests pin: the control cases above, plus string cases
x,},a,],a, }, an array elementx, ], and a nested variant-style value.
Distinct from #2827 (enumerateProxy /v1/models shape — live-source side), #2235 (this script's entry-point symlink check), and #2832 (sync-models, a different file's JSONC handling). Sibling lesson: blankComments in the same file (:132-178) already does a same-length string-aware pass — the fix can follow that pattern.
Activity
developing-today commentedon Nov 1, 2026
CODE-1311
dezren39 commentedon Nov 1, 2026
🤖 Agent note · filed
5 remaining items
dezren39 commentedon Nov 2, 2026
Queue task 6082 checkpoint (2026-11-02): confirmed the defect and scoped fix in
bin/provider-catalog-sync.mjs— comment stripping already tracks strings, but a trailing-comma regex runs over the full stripped text. I’m adding pinning tests for the listed literal comma cases, real trailing commas, and the existing URL/comment behavior, then I’ll runnode --test bin/provider-catalog-sync.test.mjson this laptop. No files changed yet; branch isqueue/6082-3430-stripjsonc-string-aware.dezren39 commentedon Nov 2, 2026
Queue task 6082 checkpoint (2026-11-02): fix complete, PR open — no merge.
Changed:
bin/provider-catalog-sync.mjs:stripJsonckeeps its string-aware comment strip, but the finalreplace(/,(\s*[}\]])/g)is gone — replaced with a string-aware trailing-comma scanner over the stripped text (skips"..."literals with the sameinstr/esctracking). Real,}/,]outside strings still removed; in-stringx,},a,],a, },x, ], nestedheaders: "a,}"survive verbatim.bin/provider-catalog-sync.test.mjs: added pinning tests for both directions (in-string preservation + real trailing-comma removal,////* */stripping,https://string case).Tests (laptop, this worktree):
node --test bin/provider-catalog-sync.test.mjs— 25 pass, 0 fail.Branch/PR:
queue/6082-3430-stripjsonc-string-aware@ 98688e4, pushed.No quota/402 wall hit; no blocker.
dezren39 commentedon Nov 2, 2026
🤖 Conductor binding · manual dispatch wave (Drew-ordered queue unblock) 2026-11-02
small_fix— "Fix provider-catalog-sync: stripJsonc's trailing-comma pass is not string-aware — a comma before } or ] inside a string literal is silently deleted #3430: make stripJsonc trailing-comma removal string-aware") ↔ issue provider-catalog-sync: stripJsonc's trailing-comma pass is not string-aware — a comma before } or ] inside a string literal is silently deleted #3430ses_e5e6323ede9ffej9vV2pOibWD8(laptop OpenChamber, agent build)opencode-route/workhorse(route id only; pooled route, no single-plan direct seat). Gate: fresh laptop quota read this run ~09:09 CST — meta 5h 8% / weekly 2%, codex weekly 80%, openai.alternate 5h 0% / weekly 0% — Workhorse OPEN; plan quotas only, never PAYG.~/git/fleet/6082-3430-stripjsonc-string-awareon the laptop, branchqueue/6082-3430-stripjsonc-string-aware(from origin/main d1de7ee)queuedin the fleet queue (there is no targeted claim/binding verb yet — code#1337 / openchamber#1272); the dispatch is recorded in the task's progress. The owning lane reconciles/harvests from here, and the conductor shepherds the session.dezren39 commentedon Nov 2, 2026
Queue task 6082 picked up (lane 8ac61616). Revalidated: issue OPEN, no superseding PR, no held/Drew-owned flags. Resuming from prior session checkpoint: fix + PR #4422 already exist — verifying fresh and harvesting (merge + close) rather than restarting.
Merge pull request #4422 from developing-today/queue/6082-3430-stripj…
dezren39 commentedon Nov 2, 2026
Queue task 6082 done. Fix merged via PR #4422 (merge 5b04802): stripJsonc trailing-comma removal is now string-aware; in-string commas survive verbatim, real trailing commas still removed, comment stripping unchanged. Verified: node --test bin/provider-catalog-sync.test.mjs 25 pass / 0 fail + independent probe of all four acceptance cases. Closing as fixed.