Skip to content

feat(build): improve performance and reliability of devcontainer launch (#977) #472

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
stewartadam wants to merge 12 commits into
base: main
Choose a base branch
from
Open

feat(build): improve performance and reliability of devcontainer launch (#977) #472

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
08c796d
feat(devcontainer): improve performance and reliability of devcontain…
stewartadam Dec 3, 2025
a81c83b
Quote $USER in shell script
stewartadam Dec 3, 2025
680be18
Use braces for string with inline variable expansion
stewartadam Dec 3, 2025
4dbe8ab
ignore crt files dropped into .devcontainer folder
stewartadam Dec 3, 2025
89bcbf8
Apply suggestions from code review
stewartadam Dec 3, 2025
a8c2104
refactor(devcontainer): remove custom CA workarounds per PR feedback
stewartadam Jan 27, 2026
0b9cc24
set ps1 to lf only for cross-os compat
stewartadam Mar 11, 2026
dfdf4c8
incorporate PR feedback
stewartadam Mar 11, 2026
265d54b
fix(devcontainer): restore python 3.11 feature
stewartadam Mar 16, 2026
01aafaa
fix(devcontainer): move npm ci to post-install to inherit container user
stewartadam Mar 17, 2026
6ba4b24
fix(devcontainer): pipe error output to stderr
stewartadam Mar 17, 2026
8bd6188
Merge branch 'main' into stad/devcontainer-enhancements
stewartadam Mar 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .devcontainer/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
---
title: Dev Container
description: Pre-configured development environment for HVE Core with all required tools and extensions
Expand Down Expand Up @@ -100,7 +100,10 @@

Container won't build: Ensure Docker Desktop is running and you have sufficient disk space (5GB+).

Extensions not loading: Reload the window (`F1` → **Developer: Reload Window**).
1. Extensions not loading: Reload the window (`F1` → **Developer: Reload Window**).

2. HTTP/TLS errors during build: Machines with corporate firewalls performing TLS inspection should ensure they are using the default `desktop-linux` builder, which honors OS root certificate trust stores.
You can change the active builder back to `desktop-linux` by running `docker buildx use desktop-linux`.

For more help, see [SUPPORT.md](../SUPPORT.md).

Expand Down
45 changes: 37 additions & 8 deletions .devcontainer/devcontainer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,24 @@
{
"name": "HVE Core - Markdown Editing",
"image": "${localEnv:HVE_DEVCONTAINER_IMAGE:mcr.microsoft.com/devcontainers/base:2-jammy}",
Comment thread
rezatnoMsirhC marked this conversation as resolved.
// Rename the mount to /workspace for consistency, otherwise its mounted using
// whatever folder name the user cloned the repo as
"workspaceMount": "\"source=${localWorkspaceFolder}\",target=/workspace,type=bind",
Comment thread
stewartadam marked this conversation as resolved.
"workspaceFolder": "/workspace",
"mounts": [
// Put GitHub local user data in a volume
{
"type": "volume",
"source": "${devcontainerId}-userconfig",
"target": "/home/vscode/.config"
Comment thread
stewartadam marked this conversation as resolved.
},
// Put node modules into volume for better performance
{
"type": "volume",
"source": "${devcontainerId}-nodemodules",
"target": "/workspace/node_modules"
}
Comment thread
stewartadam marked this conversation as resolved.
],
"features": {
"ghcr.io/devcontainers/features/node:1": {
"version": "20"
Copy link
Copy Markdown
Collaborator

@chaosdinosaur chaosdinosaur Apr 22, 2026

Choose a reason for hiding this comment

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

🔴 High — Node.js version regression

The PR branch specifies Node.js "version": "20", but origin/main now uses "version": "24". Merging as-is would regress the Node.js version. The branch needs a rebase or merge from main to pick up this change.

Suggested change
"version": "20"
"version": "24"

Expand All @@ -9,28 +27,39 @@
"version": "3.11"
},
"ghcr.io/devcontainers/features/git:1": {},
"ghcr.io/devcontainers/features/github-cli:1": {},
"ghcr.io/devcontainers/features/azure-cli:1": {},
"ghcr.io/devcontainers/features/github-cli:1": {},
"ghcr.io/devcontainers/features/powershell:1": {}
},
"customizations": {
"vscode": {
"extensions": [
"streetsidesoftware.code-spell-checker",
"davidanson.vscode-markdownlint",
"yzhang.markdown-all-in-one",
"bierner.markdown-preview-github-styles",
"bierner.markdown-mermaid",
"bierner.markdown-preview-github-styles",
"bpruitt-goddard.mermaid-markdown-syntax-highlighting",
"charliermarsh.ruff",
"davidanson.vscode-markdownlint",
"github.vscode-pull-request-github",
"ms-python.python",
"ms-python.vscode-pylance",
"charliermarsh.ruff"
]
"streetsidesoftware.code-spell-checker",
"yzhang.markdown-all-in-one"
],
"settings": {
// Prevent extensions from stealing focus, see microsoft/vscode#205225
"workbench.view.showQuietly": {
"workbench.panel.output": true
}
}
}
},
// This is to ensure support for config includes is properly handled, see microsoft/vscode-remote-release/2084
"initializeCommand": {
Comment thread
stewartadam marked this conversation as resolved.
"extractGitGlobals": "(git config -l --global --include || true) > .gitconfig.global",
"extractGitLocals": "(git config -l --local --include || true) > .gitconfig.local"
Comment on lines +58 to +59
Copy link
Copy Markdown
Collaborator

@chaosdinosaur chaosdinosaur Apr 22, 2026

Choose a reason for hiding this comment

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

🟡 Medium — initializeCommand dumps full git config to disk

This writes the entire global and local git config to plaintext files in the workspace root. While post-attach.sh only reads a safe allow-list (user.name, user.email, user.signingkey, commit.gpgsign) and then deletes the files, there's a window where sensitive config values (credential helpers, proxy settings, insteadOf rewrites) are exposed on disk.

Consider filtering at the source to only extract the keys actually needed:

Suggested change
"extractGitGlobals": "(git config -l --global --include || true) > .gitconfig.global",
"extractGitLocals": "(git config -l --local --include || true) > .gitconfig.local"
"extractGitGlobals": "(git config --global --include user.name; git config --global --include user.email; git config --global --include user.signingkey; git config --global --include commit.gpgsign) > .gitconfig.global 2>/dev/null || true",
"extractGitLocals": "(git config --local --include user.name; git config --local --include user.email; git config --local --include user.signingkey; git config --local --include commit.gpgsign) > .gitconfig.local 2>/dev/null || true"

This eliminates the information disclosure window by never writing unneeded config values to disk.

},
"postAttachCommand": "/bin/bash .devcontainer/scripts/post-attach.sh",
"onCreateCommand": "bash .devcontainer/scripts/on-create.sh",
"updateContentCommand": "npm ci",
"postCreateCommand": "bash .devcontainer/scripts/post-create.sh",
Comment on lines 62 to 63
Copy link
Copy Markdown
Collaborator

@chaosdinosaur chaosdinosaur Apr 22, 2026

Choose a reason for hiding this comment

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

🟡 Medium — Was the updateContentCommand removal intentional?

The base branch uses updateContentCommand: "npm ci", which runs during both initial creation and content update events (branch switches, pulls while the container is running). This lifecycle hook is also parallelizable with other steps.

Moving npm ci exclusively into postCreateCommand (via post-create.sh) means it only runs on initial container creation, not on content updates. If package-lock.json changes on a branch switch inside a running container, developers will need to manually run npm ci.

I'd suggest keeping updateContentCommand: "npm ci" alongside the postCreateCommand call to maintain automatic dependency updates on content changes:

Suggested change
"onCreateCommand": "bash .devcontainer/scripts/on-create.sh",
"updateContentCommand": "npm ci",
"postCreateCommand": "bash .devcontainer/scripts/post-create.sh",
"onCreateCommand": "bash .devcontainer/scripts/on-create.sh",
"updateContentCommand": "npm ci",
"postCreateCommand": "bash .devcontainer/scripts/post-create.sh",

"remoteEnv": {
"HVE_GITHUB_RELEASES_URL": "${localEnv:HVE_GITHUB_RELEASES_URL}",
Expand Down
34 changes: 34 additions & 0 deletions .devcontainer/scripts/post-attach.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
#!/usr/bin/env bash
Comment thread
stewartadam marked this conversation as resolved.
# Copyright (c) Microsoft Corporation.
# SPDX-License-Identifier: MIT
#
# post-attach.sh
# Post-attach setup for HVE Core development container

set -euo pipefail

Comment thread
stewartadam marked this conversation as resolved.
# devcontainers copy your local gitconfig but do not parse conditional includes.
# This re-configures the devcontainer git identities based on the prior exported
# global and local git configurations *after* parsing host includes. See also:
# https://github.com/microsoft/vscode-remote-release/issues/2084#issuecomment-2289987894
copy_user_gitconfig() {
for conf in .gitconfig.global .gitconfig.local; do
if [[ -f "$conf" ]]; then
echo "*** Parsing ${conf##.gitconfig.} Git configuration export"
local key value
while IFS='=' read -r key value; do
case "$key" in
user.name | user.email | user.signingkey | commit.gpgsign)
echo "Set Git config ${key}=${value}"
git config --global "$key" "$value"
Comment thread
stewartadam marked this conversation as resolved.
;;
esac
done < "$conf"
rm -f "${conf}"
fi
done
}

# Main execution path

copy_user_gitconfig
30 changes: 30 additions & 0 deletions .devcontainer/scripts/post-create.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,36 @@ set -euo pipefail
main() {
echo "Creating logs directory..."
mkdir -p logs

fix_volume_ownerships
npm_clean_install
}

# Volume ownership is not set automatically due to a bug:
# https://github.com/microsoft/vscode-remote-release/issues/9931
#
# IMPORTANT: workaround requires Docker base image to have password-less sudo.
fix_volume_ownership() {
local volume_path="$1"

if [[ ! -d "$volume_path" ]]; then
echo "ERROR: the volume path provided '$volume_path' does not exist." >&2
exit 1
fi

echo "Setting volume ownership for $volume_path"
Comment thread
stewartadam marked this conversation as resolved.
sudo -n chown "${USER}":"${USER}" "$volume_path"
}
Copy link
Copy Markdown
Collaborator

@chaosdinosaur chaosdinosaur Apr 22, 2026

Choose a reason for hiding this comment

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

🔵 Informational — sudo -n dependency on base image

sudo -n chown requires passwordless sudo, which the current base image (mcr.microsoft.com/devcontainers/base:2-jammy) provides. The inline comment documenting this assumption is appreciated. Just flagging for awareness — if the base image ever changes, this assumption will need revalidation.


fix_volume_ownerships() {
echo "Applying volume ownership workaround (see microsoft/vscode-remote-release#9931)..."
fix_volume_ownership "/home/${USER}/.config"
fix_volume_ownership "/workspace/node_modules"
}
Copy link
Copy Markdown
Collaborator

@chaosdinosaur chaosdinosaur Apr 22, 2026

Choose a reason for hiding this comment

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

🟢 Low — Hardcoded /workspace path

This hardcodes /workspace/node_modules which couples the script to the workspaceFolder value in devcontainer.json. If the mount path ever changes, this line silently breaks.

Consider using "$PWD/node_modules" or a variable to keep the two in sync:

Suggested change
}
fix_volume_ownership "$PWD/node_modules"


npm_clean_install() {
echo "Running npm ci..."
npm ci
}

main "$@"
2 changes: 2 additions & 0 deletions .dockerignore
View file
Open in desktop
Copy link
Copy Markdown
Collaborator

@chaosdinosaur chaosdinosaur Apr 22, 2026

Choose a reason for hiding this comment

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

🔵 Informational — .dockerignore has no current effect

The devcontainer uses "image:" rather than a Dockerfile build, so .dockerignore has no current effect (it only applies to Docker build contexts). The file is harmless and good practice if the setup ever transitions to a Dockerfile-based build — just noting for transparency.

Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
**/.git/
**/node_modules/
11 changes: 10 additions & 1 deletion .gitattributes
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,17 @@
# Set the default behavior, in case core.autocrlf has not been set.
* text=auto

# Declare files that will always have LF line endings on checkout.
# Declare files that must have specific line endings on checkout.
## Windows scripts - must be CRLF
*.bat text eol=crlf
*.cmd text eol=crlf

## Cross-platform PowerShell scripts - use LF for shebang compatibility
*.ps1 text eol=lf
## Linux scripts - must be LF
*.sh text eol=lf
*.Dockerfile text eol=lf
Dockerfile* text eol=lf

# Denote all files that are truly binary and should not be modified.
*.docx binary
Expand Down
4 changes: 4 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -466,3 +466,7 @@ dependency-pinning-artifacts/
docs/docusaurus/build/
docs/docusaurus/.docusaurus/
docs/docusaurus/node_modules/

# devcontainer
/.gitconfig.global
/.gitconfig.local
Loading