fix(code-server,vscode-web): parse JSONC in extensions.json and .code-workspace#798
fix(code-server,vscode-web): parse JSONC in extensions.json and .code-workspace#798rodmk wants to merge 6 commits into
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR improves auto-installation of VS Code/code-server extensions by attempting to preprocess JSONC workspace/extension files (comments + trailing commas) before feeding them to jq, and updates the module README version pins accordingly.
Changes:
- Add a
strip_jsonc_for_extensions()helper in bothvscode-webandcode-servermodules, then use it when extracting extension recommendations viajq. - Make
jqparsing tolerant of missing recommendations arrays by defaulting to[]. - Bump module versions referenced in READMEs and add a
typosdictionary entry to avoid false positives.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| registry/coder/modules/vscode-web/run.sh | Adds JSONC preprocessing helper and uses it for workspace/extensions parsing before jq. |
| registry/coder/modules/vscode-web/README.md | Updates example module version pins to 1.5.1. |
| registry/coder/modules/code-server/run.sh | Adds JSONC preprocessing helper and uses it for .vscode/extensions.json parsing before jq. |
| registry/coder/modules/code-server/README.md | Updates example module version pins to 1.4.4. |
| .github/typos.toml | Adds ba to the allowed word list (sed label pattern). |
DevelopmentCats
left a comment
There was a problem hiding this comment.
I agree with you that the Copilot Comments were wrong.
This LGTM though thanks for the fix and quick response!! 😸
|
@rodmk can you resolve the conflicts. |
|
@35C4n0r Whoa, I didn't realize this hadn't been merged yet, it was approved months ago 😅 I'll resolve the conflicts promptly. |
# Conflicts: # registry/coder/modules/code-server/README.md # registry/coder/modules/code-server/run.sh # registry/coder/modules/vscode-web/README.md
|
Resolved the conflicts against the latest Validation: 🤖 Generated with Claude Code using Claude Opus 4.8 |
35C4n0r
left a comment
There was a problem hiding this comment.
Two P1 bugs found in the strip_jsonc_for_extensions shell function introduced by this PR. Both are verified with live tests. Each inline comment above includes a suggested fix or a plain diff.
Summary of required changes:
| # | File | Issue | Severity |
|---|---|---|---|
| 1 | code-server/run.sh lines 119–128 |
Line-comment pass runs before block-comment slurp — corrupts URLs in comments and settings; single-line files pass through unprocessed | P1 |
| 2 | vscode-web/run.sh lines 153–162 |
Same stage-ordering bug | P1 |
| 3 | code-server/run.sh line 137 |
RECOMMENDATIONS_QUERY not null-safe (vscode-web was fixed, code-server was not) |
P1 |
Generated by Coder Agents on behalf of @35C4n0r.
…fety
Address review feedback on the strip_jsonc_for_extensions helper.
The previous two-stage pipeline stripped line comments before slurping
the file for block-comment removal. This corrupted any // that lived
inside a block comment (e.g. a URL in /* see https://... */), leaving an
unterminated /* that broke jq. Single-line input was also dropped
entirely by the :a;N;$!ba slurp.
Reorder into three portable passes so each concern gets the right scope:
1. block comments (whole-file slurp) first, so a URL inside /* ... */
is removed as a unit before the line-comment pass sees its //
2. line comments per line (no non-portable [^\n] class); // preceded
by ':' is preserved so URLs in .code-workspace string values survive
3. trailing commas (whole-file slurp)
The slurp idiom is :a;$!{N;ba}, which is single-line safe. Verified on
GNU sed (Linux), BSD sed (macOS), and BusyBox sed (Alpine).
Also make the code-server recommendations query null-safe to match
vscode-web: .recommendations[] -> (.recommendations // [])[], so a valid
extensions.json without a recommendations key no longer errors.
Bump code-server 1.5.1 -> 1.5.2 and vscode-web 1.6.1 -> 1.6.2.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks for the careful review — all three findings were valid and are fixed in f29505a. I reproduced each one on BSD sed (macOS) first, which also turned up a portability issue in the suggested patch, so I took a slightly different route to fix the same bugs. Details below. 1 & 2 — line-comment pass running before the block-comment slurp (both files)Confirmed, both failure modes reproduce:
On the suggested fix: collapsing into one So I kept the fix portable with three ordered passes instead: strip_jsonc_for_extensions() {
sed -E ':a
$!{
N
ba
}
s#/[*]([^*]|[*]+[^*/])*[*]+/##g' "$1" \
| sed -E 's#^[[:space:]]*//.*##; s#([^:])//.*#\1#' \
| sed -E ':a
$!{
N
ba
}
s/,[^]}"]*([]}])/\1/g'
}
3 — code-server null-safetyFixed: On failure mode 2 (URL in a
|
|
@rodmk can you resolve the merge conflict. |
|
Yup sorry just noticed, doing that now. |
Resolve conflicts against the latest main: - code-server/run.sh: keep both coder#953's new settings-merge logic (base64 SETTINGS_B64/MACHINE_SETTINGS_B64 + merge_settings) and this branch's JSONC parsing (strip_jsonc_for_extensions) + null-safe recommendations query. Auto-merged cleanly. - code-server/README.md: version pins resolved to 1.5.2 (latest release tag v1.5.1 + patch), keeping upstream's coder#953 README content outside the version lines. - vscode-web/README.md: corrected version pins to 1.6.1 (latest release tag v1.6.0 + patch); the previous 1.6.2 was one ahead of the released baseline and would have failed the version-bump CI check. Validation after merge: bun run fmt:ci, shellcheck, terraform test (code-server 8/8, vscode-web 3/3), bun test (code-server 8/8, vscode-web 7/7), and the 25-case JSONC matrix (GNU/BSD/BusyBox sed) all pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
All set - no more conflicts for now. |
|
@rodmk can you add some tests in main.test.ts for both the modules to test the changes. |
Add container-based tests that drive the rendered coder_script through the auto_install_extensions path with a mock CLI that records --install-extension calls, so the actual strip_jsonc_for_extensions + jq queries are exercised. - code-server: reaches auto-install via use_cached + a pre-placed mock binary. Covers a JSONC .vscode/extensions.json (line/block/multi-line comments, a URL inside a block comment, trailing comma) and a file with no recommendations key (null-safe query must not make jq error). - vscode-web: early-exits on use_cached/offline, so curl/tar are stubbed to make the download a no-op and let the pre-placed mock binary survive. Covers the JSONC .vscode/extensions.json path and a .code-workspace whose settings hold a URL (the :// guard must keep it intact so jq can parse the workspace file). These fail on the pre-fix code (the old pipeline drops the extension after the URL-bearing block comment; the old code-server query errors on a missing key). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Added tests in One thing worth flagging: 🤖 Generated with Claude Code using Claude Opus 4.8 |
Fixes #708. Supersedes #707.
Description
auto_install_extensionsfails to parse.vscode/extensions.jsonand.code-workspacefiles that contain valid JSONC (block comments, line comments, trailing commas). VS Code officially supports these via its built-in JSONC parser with default options.This PR replaces the original
sed 's|//.*||g' | jqpipeline with a small, portablestrip_jsonc_for_extensionshelper (defined once per script) that strips JSONC features before handing the file tojq— no new dependencies, no GNU-only extensions.Type of Change
Module Information
Path:
registry/coder/modules/code-serverandregistry/coder/modules/vscode-webNew version: code-server
1.5.2, vscode-web1.6.2Breaking change: No
Problem
The original
sed 's|//.*||g' | jqpipeline only handled line comments; block comments and trailing commas brokejq. Naive fixes introduce two portability traps and one ordering bug:sed -z(slurp the whole file) is a GNU extension — it does not exist on BSD sed (macOS) and would break there.[^\n]inside a bracket expression is a GNU extension — BSD sed reads\nin a bracket as a literal backslash andn, so//[^\n]*silently corrupts any extension ID containing the lettern(e.g.dbaeumer.vscode-eslint).//that lives inside a block comment (e.g. a URL in/* see https://open-vsx.org */), leaving an unterminated/*that makesjqfail — installing zero extensions with no error surfaced. A single-line/minified file was also dropped entirely by the:a;N;$!baslurp idiom.Fix
strip_jsonc_for_extensionsruns three orderedsedpasses, each scoped to what it needs and portable across GNU (Linux), BSD (macOS), and BusyBox (Alpine) sed:/* ... */can span lines. Running first means a URL inside a block comment is removed as a unit before the line-comment pass ever sees its//.// ...stops at end of line without the non-portable[^\n]class. A//preceded by:is preserved, so URLs in string values (e.g."http.proxy": "https://…"in a.code-workspace) survive.The
:a;$!{N;ba}slurp idiom is single-line safe (it falls through on the last/only line), unlike:a;N;$!ba, which drops single-line input.Also makes the code-server recommendations query null-safe to match vscode-web (
.recommendations[]→(.recommendations // [])[]), so a validextensions.jsonwith norecommendationskey no longer errors.Known limitation
This is not a general-purpose JSONC parser. A non-URL
//inside a string value (e.g. a regex setting"a//b") can still be mis-stripped — only a real string-aware parser could distinguish it, and a hand-rolled parser was rejected in #707 as over-engineered. Extension IDs are restricted to[a-z0-9\-.]by vsce'snameRegex, and URLs (the common//-in-string case) are handled by the://guard, so the recommendations arrays this feature actually reads are safe.Alternatives considered
sed -z-zis a GNU extension — breaks on macOS BSD sedsed -Ewith//[^\n]*[^\n]in a bracket is not portable — BSD sed treats it as literal\+n, silently corrupting IDs containingnperl -0777 -pecommand -vguardsed(this PR)Testing & Validation
terraform test: code-server 8/8, vscode-web 3/3installs and runs code-servertest)prettier(withprettier-plugin-sh) andshellcheck --severity=warningclean;terraform fmtcleantemplatefile()renders bothrun.shfiles; rendered scripts passbash -nThe
strip_jsonc_for_extensionshelper was validated with a 25-case matrix (the original 19 JSONC cases plus new regression cases for URL-in-block-comment, single-line/minified input, IDs containingn, a line comment containing a quote before a trailing comma, and URL-valued.code-workspacesettings). The matrix was run against the exact bytes committed to bothrun.shfiles and passes 25/25 on GNU sed (Ubuntu), BSD sed (macOS), and BusyBox sed (Alpine).Related Issues
🤖 Generated with Claude Code using Claude Opus 4.8