Skip to content

fix(code-server,vscode-web): parse JSONC in extensions.json and .code-workspace#798

Open
rodmk wants to merge 7 commits into
coder:mainfrom
rodmk:fix/jsonc-parsing-708
Open

fix(code-server,vscode-web): parse JSONC in extensions.json and .code-workspace#798
rodmk wants to merge 7 commits into
coder:mainfrom
rodmk:fix/jsonc-parsing-708

Conversation

@rodmk

@rodmk rodmk commented Mar 12, 2026

Copy link
Copy Markdown

Fixes #708. Supersedes #707.

Description

auto_install_extensions fails to parse .vscode/extensions.json and .code-workspace files 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' | jq pipeline with a small, portable strip_jsonc_for_extensions helper (defined once per script) that strips JSONC features before handing the file to jq — no new dependencies, no GNU-only extensions.

Type of Change

  • Bug fix

Module Information

Path: registry/coder/modules/code-server and registry/coder/modules/vscode-web
New version: code-server 1.5.2, vscode-web 1.6.2
Breaking change: No

Problem

The original sed 's|//.*||g' | jq pipeline only handled line comments; block comments and trailing commas broke jq. Naive fixes introduce two portability traps and one ordering bug:

  1. sed -z (slurp the whole file) is a GNU extension — it does not exist on BSD sed (macOS) and would break there.
  2. [^\n] inside a bracket expression is a GNU extension — BSD sed reads \n in a bracket as a literal backslash and n, so //[^\n]* silently corrupts any extension ID containing the letter n (e.g. dbaeumer.vscode-eslint).
  3. Stripping line comments before block comments corrupts a // that lives inside a block comment (e.g. a URL in /* see https://open-vsx.org */), leaving an unterminated /* that makes jq fail — installing zero extensions with no error surfaced. A single-line/minified file was also dropped entirely by the :a;N;$!ba slurp idiom.

Fix

strip_jsonc_for_extensions runs three ordered sed passes, each scoped to what it needs and portable across GNU (Linux), BSD (macOS), and BusyBox (Alpine) sed:

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'
}
  1. Block comments first — slurps the whole file so /* ... */ 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 //.
  2. Line comments per line — // ... 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.
  3. Trailing commas — slurps the whole file so a comma and its closing bracket may sit on different lines.

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 valid extensions.json with no recommendations key 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's nameRegex, and URLs (the common //-in-string case) are handled by the :// guard, so the recommendations arrays this feature actually reads are safe.

Alternatives considered

Approach Tradeoff
sed -z Simpler slurp, but -z is a GNU extension — breaks on macOS BSD sed
Single sed -E with //[^\n]* Works on GNU sed, but [^\n] in a bracket is not portable — BSD sed treats it as literal \ + n, silently corrupting IDs containing n
perl -0777 -pe Clean one-liner, but no module in the repo uses perl and Apple may remove it from future macOS
Node.js state machine Correct JSONC parsing, but rejected in #707 as over-engineered
Python3 Could parse JSONC natively, but not guaranteed on all workspaces and would need a command -v guard
Three-pass sed (this PR) Two extra pipes, but zero new dependencies, fully portable across GNU/BSD/BusyBox sed, and fixes comment/URL ordering + single-line + null-safety

Testing & Validation

  • terraform test: code-server 8/8, vscode-web 3/3
  • code-server + vscode-web TypeScript suites pass in CI (incl. the container installs and runs code-server test)
  • prettier (with prettier-plugin-sh) and shellcheck --severity=warning clean; terraform fmt clean
  • templatefile() renders both run.sh files; rendered scripts pass bash -n
  • Version bumped

The strip_jsonc_for_extensions helper 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 containing n, a line comment containing a quote before a trailing comma, and URL-valued .code-workspace settings). The matrix was run against the exact bytes committed to both run.sh files 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

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 both vscode-web and code-server modules, then use it when extracting extension recommendations via jq.
  • Make jq parsing tolerant of missing recommendations arrays by defaulting to [].
  • Bump module versions referenced in READMEs and add a typos dictionary 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).

Comment thread registry/coder/modules/vscode-web/run.sh Outdated
Comment thread registry/coder/modules/vscode-web/run.sh Outdated
Comment thread registry/coder/modules/code-server/run.sh Outdated
Comment thread registry/coder/modules/code-server/run.sh Outdated

@DevelopmentCats DevelopmentCats left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with you that the Copilot Comments were wrong.

This LGTM though thanks for the fix and quick response!! 😸

@35C4n0r

35C4n0r commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

@rodmk can you resolve the conflicts.

@rodmk

rodmk commented Jul 7, 2026

Copy link
Copy Markdown
Author

@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
@rodmk

rodmk commented Jul 7, 2026

Copy link
Copy Markdown
Author

Resolved the conflicts against the latest main. The only real overlap was in the code-server/vscode-web run.sh scripts: I merged the new WORKSPACE-file support together with this PR's JSONC parsing, so both commented extensions.json files and .code-workspace files work. Version bumps re-based onto the current releases — code-server1.5.1, vscode-web1.6.1.

Validation: fmt, README validation, golangci-lint, and the full code-server + vscode-web test suites all pass. (The one failing test is an unrelated nexus-repository timeout flake.)

🤖 Generated with Claude Code using Claude Opus 4.8

@35C4n0r 35C4n0r requested review from 35C4n0r and removed request for jdomeracki-coder July 7, 2026 16:18

@35C4n0r 35C4n0r left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread registry/coder/modules/code-server/run.sh Outdated
Comment thread registry/coder/modules/vscode-web/run.sh Outdated
Comment thread registry/coder/modules/code-server/run.sh
…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>
@rodmk rodmk changed the title fix(code-server,vscode-web): parse JSONC in extensions.json with sed fix(code-server,vscode-web): parse JSONC in extensions.json and .code-workspace Jul 9, 2026
@rodmk

rodmk commented Jul 9, 2026

Copy link
Copy Markdown
Author

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:

  • URL inside a block comment/* see https://open-vsx.org */ → the first-stage s|//.*||g strips //open-vsx.org */, leaving an unterminated /* see https: that jq rejects. Zero extensions, no error surfaced.
  • Single-line / minified file:a;N;$!ba drops single-line input entirely (on BSD it produced empty output).

On the suggested fix: collapsing into one sed -E with s#//[^\n]*##g fixes the block-comment case, but [^\n] inside a bracket expression is a GNU extension. On BSD sed (macOS, which this module supports via platform = "darwin") \n in a bracket is read as a literal backslash + n, so //[^\n]* stops at the first n and silently corrupts IDs — e.g. dbaeumer.vscode-eslint came out as nting extension. Verified locally.

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'
}
  • Block comments first (whole-file slurp) so a // inside /* ... */ is removed as a unit before the line-comment pass sees it.
  • Line comments per line — no [^\n]; a // preceded by : is preserved so URLs in string values survive.
  • Trailing commas last (slurp).
  • :a;$!{N;ba} is single-line safe (falls through on the last/only line), fixing the minified-file case.

3 — code-server null-safety

Fixed: RECOMMENDATIONS_QUERY is now (.recommendations // [])[] (and the workspace query (.extensions.recommendations // [])[]), matching vscode-web.

On failure mode 2 (URL in a .code-workspace setting value)

Agreed this is the fundamental limit you called out — a purely regex-based stripper can't reliably tell a // in a string from a comment. The ://-guard in the line-comment pass handles the realistic case (URL-valued settings such as "http.proxy": "https://proxy.corp:8080" now survive). A non-URL // inside a string value is still out of reach without a real parser, which #707 established as over-engineered; I documented this as a known limitation in the PR description.

Validation

The helper was run against a 25-case matrix (the original 19 JSONC cases + regressions for URL-in-block-comment, single-line input, IDs containing n, a quote inside a line comment before a trailing comma, and URL-valued .code-workspace settings), using the exact bytes committed to both run.sh files. 25/25 on GNU sed (Ubuntu), BSD sed (macOS), and BusyBox sed (Alpine). terraform test, the code-server/vscode-web TS suites, shellcheck, and prettier all pass. Versions bumped to code-server 1.5.2 / vscode-web 1.6.1.

🤖 Generated with Claude Code using Claude Opus 4.8

@35C4n0r

35C4n0r commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

@rodmk can you resolve the merge conflict.

@rodmk

rodmk commented Jul 9, 2026

Copy link
Copy Markdown
Author

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>
@rodmk

rodmk commented Jul 9, 2026

Copy link
Copy Markdown
Author

All set - no more conflicts for now.

@35C4n0r 35C4n0r requested a review from DevelopmentCats July 9, 2026 18:20
@35C4n0r

35C4n0r commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

@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>
@rodmk

rodmk commented Jul 9, 2026

Copy link
Copy Markdown
Author

Added tests in main.test.ts for both modules (b0fc629) that drive the rendered script through the auto_install_extensions path with a mock CLI recording --install-extension calls — covering JSONC comments (incl. a URL inside a block comment), trailing commas, the null-safe query, and a .code-workspace with a URL-valued setting. They fail on the pre-fix code.

One thing worth flagging: code-server reaches the install path cleanly via use_cached, but vscode-web early-exits on use_cached/offline, so its test stubs curl/tar to no-op the download and let the pre-placed mock binary survive. All green in CI.

🤖 Generated with Claude Code using Claude Opus 4.8

@35C4n0r 35C4n0r removed the request for review from DevelopmentCats July 16, 2026 09:05
@35C4n0r 35C4n0r added the version:patch Add to PRs requiring a patch version upgrade label Jul 16, 2026

@35C4n0r 35C4n0r left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also add test for the null-safe guard on a missing recommendations key.

Comment thread .github/typos.toml
mavrick = "mavrick" # Username
inh = "inh" # Option in setpriv command
exportfs = "exportfs" # nfs related binary
ba = "ba" # sed branch command in :a;N;$!ba pattern

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ba = "ba" # sed branch command in :a;N;$!ba pattern
ba = "ba" # sed branch command in :a;$!{N;ba} pattern

# 3. Trailing commas - slurps the whole file so a comma and its closing
# bracket may sit on different lines.
# The ':a;$!{N;ba}' slurp is single-line safe (it falls through on the last or
# only line), unlike ':a;N;$!ba', which drops single-line input entirely.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# only line), unlike ':a;N;$!ba', which drops single-line input entirely.
# only line).

# 3. Trailing commas - slurps the whole file so a comma and its closing
# bracket may sit on different lines.
# The ':a;$!{N;ba}' slurp is single-line safe (it falls through on the last or
# only line), unlike ':a;N;$!ba', which drops single-line input entirely.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# only line), unlike ':a;N;$!ba', which drops single-line input entirely.
# only line).

@35C4n0r

35C4n0r commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

@rodmk, can you address the above reviews.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

version:patch Add to PRs requiring a patch version upgrade

Projects

None yet

Development

Successfully merging this pull request may close these issues.

auto_install_extensions fails on JSONC comments and trailing commas

4 participants