feat(vscode-desktop): pre-apply machine settings (#207) by davidoo62 · Pull Request #1220 · coder/registry

davidoo62 · 2026-10-29T21:31:51Z

Closes #207

Description

Completes the resolution of #207 by adding remote machine settings pre-configuration support to the vscode-desktop module.

While PR #1094 introduced the extensions pre-installation capability, issue #207 also requires pre-applying VS Code machine settings before the user's first connection to the workspace. This PR implements that capability natively in the vscode-desktop module.

Type of Change

Feature/enhancement
Documentation

Module Information

Path: registry/coder/modules/vscode-desktop
New version: v1.4.0
Breaking change: [ ] Yes [x] No

What Changed

Variables Added

Variable	Type	Default	Description
`settings`	`any`	`{}`	Map of machine-level settings to apply to VS Code Desktop
`settings_file`	`string`	`"$HOME/.vscode-server/data/Machine/settings.json"`	Target settings.json file path

Implementation Details

Finite, login-blocking startup execution: When settings is configured, a coder_script resource (apply_settings) runs on workspace start (start_blocks_login = true, timeout 300s).
Zero-overhead default: If settings is empty or unset, no additional script resource is created (count = 0).
Safe, robust JSONC merging:
- Strips block comments, line comments, and trailing commas from existing settings files before parsing.
- Deep-merges new configured settings into existing machine settings without deleting user-defined keys.
- Atomically updates symlinks without breaking user symlink targets.
- Leaves original file untouched if parsing or merging encounters any fatal error.
Documentation & Tests:
- Added documentation and configuration examples in README.md.
- Added test assertions in main.tftest.hcl and main.test.ts.

Usage Example

module "vscode" {
  count    = data.coder_workspace.me.start_count
  source   = "registry.coder.com/coder/vscode-desktop/coder"
  version  = "1.4.0"
  agent_id = coder_agent.main.id

  extensions = [
    "ms-python.python",
    "esbenp.prettier-vscode@12.4.0",
  ]

  settings = {
    "editor.fontSize"              = 14
    "editor.formatOnSave"          = true
    "workbench.colorCustomizations" = {
      "editor.background" = "#1e1e1e"
    }
  }
}

Testing & Validation

Test plan in main.tftest.hcl covering default (no script) and configured (script created, blocks login)
Test cases added to main.test.ts
Verified merge-settings.py against valid JSON, JSONC with comments, trailing commas, and deep object merging
Verified symlink preservation
Backward-compatible — existing extensions, folder, and open_recent behaviors unchanged

## Summary Completes the resolution of coder#207 by adding remote machine settings support to the `vscode-desktop` module: - Add `settings` map variable to configure machine-level settings before first user connection - Add `settings_file` variable pointing to `~/.vscode-server/data/Machine/settings.json` by default - Add finite, login-blocking `apply_settings` coder_script executing during workspace startup - Safely merge configured settings with existing settings (supporting comments and trailing commas) - Atomically update symlinks without destroying user symlink targets - Document settings usage and bump version examples to v1.4.0 - Add tests in `main.tftest.hcl` and `main.test.ts`

DevelopmentCats · 2026-11-02T15:13:40Z

/review

DevelopmentCats

Full review with the context of registry/coder/modules/vscode-desktop. I verified behavior rather than only reading it: rendered scripts/apply-settings.sh.tftpl and executed it end to end (create, merge into JSONC, symlink write-through, dangling symlink, malformed settings), ran scripts/merge-settings.py against comments, block comments, and trailing commas, ran shellcheck (severity=warning) on the template, and probed the Terraform typing edge cases in terraform console.

What works well

Injection safety: settings and settings_file are base64 encoded before being embedded, so user input cannot break out of the shell quoting.
Atomicity: mktemp in the target directory plus mv gives a same-filesystem atomic replace, and the original file is left untouched when parsing or merging fails (verified).
The JSONC stripper handles strings, escaped quotes, BOMs (utf-8-sig), and unterminated block comments correctly.
Deep merge preserves the key order of existing settings and appends new keys (verified).
Zero overhead default: no script resource is created when settings is empty (covered in both test suites).
Version bump 1.3.0 -> 1.4.0 (minor for a feature) matches the AGENTS.md convention, and README examples are updated everywhere.

Blocking

1. CI is red: Prettier fails on two files

The "Check for typos and unformatted code" check fails on registry/coder/modules/vscode-desktop/main.test.ts (stray blank line at EOF) and registry/coder/modules/vscode-desktop/README.md (alignment of the settings example). Please run bun run fmt and push.

2. README claims comments are preserved, they are not

registry/coder/modules/vscode-desktop/README.md line 84 says existing settings "including JSONC comments and trailing commas" are preserved. They are accepted, but comments are stripped and the file is rewritten as plain JSON (verified: a file with // and /* */ comments and a trailing comma comes back without them). Real user impact: someone whose settings.json is managed in a dotfiles repo silently loses their comments on first start. See the inline suggestion to reword; optionally log a notice when comments were stripped.

Should fix

3. Dangling symlink blocks login for the full timeout

registry/coder/modules/vscode-desktop/scripts/resolve-settings-path.py line 6 uses resolve(strict=True). If settings_file is a symlink whose target does not exist yet, this raises, the script exits 1, and because start_blocks_login = true the user is blocked from logging in for the full 300s on every start. With strict=False a dangling symlink resolves to its target path, the existing [ ! -e ] branch then creates the file, and the symlink stays intact.

4. `settings` accepts non-map values, the failure only surfaces at runtime

registry/coder/modules/vscode-desktop/main.tf types settings as any with no validation. I confirmed in terraform console that a string passes local.has_settings ("foo" != {} is true), so a mistaken settings = "editor.fontSize" reaches the runtime: jsonencode produces a JSON string and the script either writes an invalid settings.json (new file, verified: exits 0 with a bare JSON string on disk) or crashes in merge() with AttributeError (existing file), blocking login for the timeout. A validation block catches this at plan time. I also checked that map(any) is not an option here: Terraform rejects the README's mixed number/bool example with "all map elements must have the same type", so validation is the right tool.

5. Empty `settings.json` hard-fails

A 0-byte settings.json raises JSONDecodeError and fails the startup script (verified), blocking login for 300s. Treating an empty file as {} is a cheap safeguard.

6. No automated test exercises the merge or symlink path

The trickiest logic (JSONC stripping, merge, symlink write-through) ships without runtime coverage, while the extensions flow has container tests in main.test.ts. Suggest a container test that writes a JSONC settings file plus a symlink, runs the rendered script, and asserts the merged output and intact symlink. Happy to sketch one if useful.

Nits

apply-settings.sh.tftpl forces chmod 600 on merge, silently overwriting existing permissions (e.g. 644). Consider preserving the original mode (stat -c %a) or documenting the change.
The trap is registered after both mktemp calls, so if the second mktemp fails the first temp file leaks. Register the trap right after the first mktemp.
expand_home only handles $HOME, not ~/. A settings_file of "~/x" creates a literal ~ directory relative to the agent cwd. Worth a README note or tilde support.
On first creation the file is written as compact single-line JSON (jsonencode), on merge it is pretty printed with indent=2. Consider pretty printing both for consistency.
timeout = 300 is generous for a purely local merge with no network. 60s would still be plenty and shrinks the worst-case login block when the script fails (e.g. missing python3). Also consider documenting in the README that this script blocks login on start, like the extensions section does.

Questions

AGENTS.md asks for coder-utils orchestration when reworking module scripts (ordering, materialization under $HOME/.coder-modules/coder/vscode-desktop/, logs). This PR adds a hand-rolled coder_script with inline Python and no on-disk logs. Deliberate exception for consistency with vscode-desktop-core? If it stays, consider writing a log line under the module logs dir.
Path assumption worth confirming: extensions use a custom extensions_dir ($HOME/.vscode-server/extensions), while settings target the default ~/.vscode-server/data/Machine/settings.json. Please confirm the server launched by the Coder remote extension reads machine settings from that default data dir.

Residual risk: I could not run the full bun check suite here (the TS tests require Docker and the container tooling is unavailable in my review environment), so CI remains the source of truth for those. Everything else above was verified directly.

DevelopmentCats · 2026-11-02T15:27:58Z

+}
+```
+
+Existing settings in `settings.json` (including JSONC comments and trailing commas) are preserved, and user symlinks are respected and atomically updated.


Verified: comments are not preserved. The pipeline strips comments and trailing commas and rewrites the file with json.dump, so the output is plain JSON, and users lose comments they may have in a dotfiles-managed settings.json. Suggested reword so the docs match behavior:

Suggested change

Existing settings in `settings.json` (including JSONC comments and trailing commas) are preserved, and user symlinks are respected and atomically updated.

Existing keys and values in `settings.json` are preserved and merged on startup. The file is rewritten as plain JSON, so JSONC comments and trailing commas are accepted but not kept. Symlinks are resolved and their targets are updated atomically.

DevelopmentCats · 2026-11-02T15:27:58Z

+    );
+  });
 });
+


Trailing blank line at EOF, Prettier flags this file (CI check failure). Empty suggestion removes the line:

Suggested change

DevelopmentCats · 2026-11-02T15:27:58Z

+
+
+try:
+    print(Path(sys.argv[1]).resolve(strict=True))


A dangling symlink (target not created yet) makes resolve(strict=True) raise OSError, the script exits 1, and with start_blocks_login = true the user is blocked from logging in for the full 300s on every start. With strict=False the symlink resolves to its target path, the existing [ ! -e ] branch creates the file, and the symlink stays intact.

Suggested change

print(Path(sys.argv[1]).resolve(strict=True))

print(Path(sys.argv[1]).resolve(strict=False))

DevelopmentCats · 2026-11-02T15:27:58Z

+variable "settings" {
+  type        = any
+  description = "A map of settings to apply to VS Code Desktop's Machine settings. These settings are merged with any existing machine settings on startup."
+  default     = {}
+}


settings has no plan-time guard. A string value passes local.has_settings (verified: "foo" != {} is true in Terraform) and then either writes an invalid settings file or crashes merge-settings.py at runtime, blocking login. This validation rejects non-maps at plan time:

Suggested change

variable "settings" {

type = any

description = "A map of settings to apply to VS Code Desktop's Machine settings. These settings are merged with any existing machine settings on startup."

default = {}

}

variable "settings" {

description = "A map of settings to apply to VS Code Desktop's Machine settings. These settings are merged with any existing machine settings on startup."

type = any

default = {}

validation {

condition = var.settings == null ? true : can([for key, value in var.settings : value])

error_message = "settings must be a map of setting names to values."

}

}

Note map(any) is not a substitute: Terraform rejects mixed-type maps like the README example with "all map elements must have the same type".

DevelopmentCats · 2026-11-02T15:27:59Z

+        source = source_file.read()
+    return json.loads(strip_trailing_commas(strip_jsonc_comments(source)))


A 0-byte settings.json currently raises JSONDecodeError and fails the startup script (verified), blocking login for 300s. Treating an empty file as an empty object is a cheap safeguard:

Suggested change

source = source_file.read()

return json.loads(strip_trailing_commas(strip_jsonc_comments(source)))

source = source_file.read()

if not source.strip():

return {}

return json.loads(strip_trailing_commas(strip_jsonc_comments(source)))

DevelopmentCats requested changes Nov 2, 2026

View reviewed changes

Navigation Menu

Repository navigation

feat(vscode-desktop): pre-apply machine settings (#207) - #1220

feat(vscode-desktop): pre-apply machine settings (#207)#1220
davidoo62 wants to merge 1 commit into
coder:mainfrom
davidoo62:feat/vscode-desktop-preinstall-extensions

davidoo62 commented Oct 29, 2026

Uh oh!

DevelopmentCats commented Nov 2, 2026

Uh oh!

DevelopmentCats left a comment

Uh oh!

DevelopmentCats Nov 2, 2026

Uh oh!

DevelopmentCats Nov 2, 2026

Uh oh!

DevelopmentCats Nov 2, 2026

Uh oh!

DevelopmentCats Nov 2, 2026

Uh oh!

DevelopmentCats Nov 2, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	Existing settings in `settings.json` (including JSONC comments and trailing commas) are preserved, and user symlinks are respected and atomically updated.
	Existing keys and values in `settings.json` are preserved and merged on startup. The file is rewritten as plain JSON, so JSONC comments and trailing commas are accepted but not kept. Symlinks are resolved and their targets are updated atomically.

	print(Path(sys.argv[1]).resolve(strict=True))
	print(Path(sys.argv[1]).resolve(strict=False))

		source = source_file.read()
		return json.loads(strip_trailing_commas(strip_jsonc_comments(source)))



		try:
		print(Path(sys.argv[1]).resolve(strict=True))

Conversation

davidoo62 commented Oct 29, 2026

Description

Type of Change

Module Information

What Changed

Variables Added

Implementation Details

Usage Example

Testing & Validation

Uh oh!

DevelopmentCats commented Nov 2, 2026

Uh oh!

DevelopmentCats left a comment

Choose a reason for hiding this comment

What works well

Blocking

1. CI is red: Prettier fails on two files

2. README claims comments are preserved, they are not

Should fix

3. Dangling symlink blocks login for the full timeout

4. settings accepts non-map values, the failure only surfaces at runtime

5. Empty settings.json hard-fails

6. No automated test exercises the merge or symlink path

Nits

Questions

Uh oh!

DevelopmentCats Nov 2, 2026

Choose a reason for hiding this comment

Uh oh!

DevelopmentCats Nov 2, 2026

Choose a reason for hiding this comment

Uh oh!

DevelopmentCats Nov 2, 2026

Choose a reason for hiding this comment

Uh oh!

DevelopmentCats Nov 2, 2026

Choose a reason for hiding this comment

Uh oh!

DevelopmentCats Nov 2, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

4. `settings` accepts non-map values, the failure only surfaces at runtime

5. Empty `settings.json` hard-fails