fix: properly parse JSONC in extensions.json and batch extension installs#707
fix: properly parse JSONC in extensions.json and batch extension installs#707rodmk wants to merge 3 commits intocoder:mainfrom
Conversation
85d1673 to
af3efc1
Compare
…alls The auto_install_extensions feature uses `sed | jq` to parse .vscode/extensions.json, but this file is JSONC (JSON with Comments), not strict JSON. The current parser fails on: - Block comments (/* */) - Trailing commas (common in VS Code-generated files) - URLs inside string values (sed mangles https:// to https:) Replace the sed|jq pipeline with a proper state-machine JSONC parser that runs on the bundled Node.js binary already shipped with both code-server and vscode-web. This eliminates the jq dependency for this feature. Also batch extension installs into a single invocation using repeated --install-extension flags instead of spawning one process per extension. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
af3efc1 to
a418556
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a418556ae4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| } | ||
|
|
||
| result = result.replace(/,(\s*[\]}])/g, "$1"); |
There was a problem hiding this comment.
Preserve string contents when removing trailing commas
The trailing-comma pass result.replace(/,(\s*[\]}])/g, "$1") is not string-aware, so it also rewrites valid string data that happens to contain ,] or ,}. In extensions.json, a recommendation like https://example.com/a,].vsix is mutated to https://example.com/a].vsix before JSON.parse, which leads to installing the wrong extension ID/URL (or a failed install); the same parser logic is duplicated for vscode-web, so both modules are affected.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This is a non-issue for valid inputs. The recommendations array in extensions.json only accepts extension identifiers in publisher.extensionName format, and both components are validated by vsce's nameRegex:
/^[a-z0-9][a-z0-9\-]*$/i
Only letters, digits, and hyphens are allowed — no commas, brackets, or braces. The characters ,] / ,} that would trigger this regex bug cannot appear in any valid extension identifier. The example https://example.com/a,].vsix is not a valid extensions.json entry (VSIX URLs are a CLI-only feature for --install-extension, not supported in recommendations[]).
That said, making the trailing-comma removal string-aware would be a nice hardening improvement — happy to do it if reviewers prefer, but it's not a correctness issue for this use case.
Generated with Claude Code
Move trailing-comma handling into the state-machine loop so it never modifies characters inside quoted strings. Adds a regression test for the edge case (though it can't occur in practice since extension IDs are restricted to [a-z0-9-] by vsce's nameRegex). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Update: string-aware trailing-comma removalAddressed the Codex review feedback by making the trailing-comma removal string-aware. Instead of a separate regex pass ( Also added a regression test for the edge case with a comment noting it's hardening only (extension IDs are restricted to All CI checks pass on the fork: rodmk#2 Generated with Claude Code |
Fixes #708.
Summary
sed | jqJSONC parsing with a proper state-machine parser using the bundled Node.js binary--install-extensionflags) instead of one process per extensionProblem
.vscode/extensions.jsonis JSONC (JSON with Comments), not strict JSON. Thesed 's|//.*||g' | jqpipeline fails on block comments, trailing commas, and mangles URLs in strings.Fix
parse_jsonc_extensions.js) — uses thenodebinary already bundled with code-server/vscode-web, no new dependencies--install-extensionflags in one call instead of N separate processesNote on duplication
parse_jsonc_extensions.jsis duplicated in bothcode-server/andvscode-web/. The repo structure validator requires every subdirectory undermodules/to be a full module, so a sharedlib/directory isn't possible. This matches existing patterns (e.g. JFrog.tftplduplication). Happy to refactor if there's a preferred way to share code across modules.Test plan
parse_jsonc_extensions.test.ts): comments, trailing commas, URL safety,.code-workspaceformat--install-extensionworks with code-serverextensions.json(with//,/* */, trailing commas) parsed and extensions installed in single batched call using code-server's bundled node🤖 Generated with Claude Code