Repository navigation
fix: parity round 4 (override-config, worktree mounts, logs, local Features, upgrade) - #45
Merged
Merged
Conversation
… does TS getWorkspaceConfiguration (v0.89.0, utils.ts) computes the Git worktree common-dir mount together with the workspace mount, only when the config leaves workspaceFolder or workspaceMount unset, and places it relative to the computed /workspaces/<segments> folder. Go had backported an unreleased upstream change (#1261): it added the mount even when the config set both properties, and resolved its target relative to the custom workspaceMount's target. read-configuration then reported an additionalMountString the reference does not, and up mounted the common dir into the container where the reference does not. Restore the reference behavior, with parity cases for both commands. BREAKING CHANGE: a Git worktree workspace whose configuration sets both workspaceMount and workspaceFolder no longer gets the common-dir mount; add it to `mounts` when Git is needed in the container. With only workspaceMount set, it lands under /workspaces/<main repository>/.git. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TS getWorkspaceConfiguration and readDevContainerConfigFile (v0.89.0) test `'workspaceMount' in config`, so an empty or null workspaceMount counts as set: read-configuration reports it as written, up passes no workspace --mount, and with workspaceFolder also set the worktree common-dir mount is not computed either. Go treated an empty workspaceMount as absent and bind-mounted the workspace folder. Track the property's presence and follow the reference, with parity cases for read-configuration and up. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
--log-format json events HTML-escaped &, < and > and escaped U+2028 and U+2029, which TS createJSONLog writes as they are. The JSON.stringify encoder command results already use moves to internal/stringify so the log handler shares it, and a ts-oracle golden pins the JSON log's bytes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TS dockerComposeCLIConfig runs `docker compose version --short` through the logging exec, so the debug log shows the command's start and stop and then "Docker Compose version: <version>". The Go client ran the command silently. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TS readDockerComposeConfig runs `docker compose ... config` with an info-level log, so its "Run:" start line shows at the default log level, and then writes the command's output to the debug log. The Go client logged the command at debug and dropped the output. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TS readConfiguration calls generateFeaturesConfig whenever it reads the Features configuration, which traces "workspace root: <cwd>" and, for a configuration without Features, "No user features to update". The Go command skipped both, and skipped the step entirely without Features. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The reference declares upgrade's --log-level as a yargs choice of error, info, debug and trace, validated before its --feature/--target-version check. upgrade accepted any value and ran at the default level. BREAKING CHANGE: `upgrade --log-level` with a value other than error, info, debug or trace now exits 1 with an invalid-value error. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The reference's getConfig fails with "Dev container config (<path>) not found." when --config names a file that does not exist. upgrade reported the operating system's read error instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The reference's upgrade logs nothing of its own after writing the lockfile, and its log starts with the version header, written with the first event, which every run that loads the configuration logs. upgrade logged "Lockfile updated." and never the header. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The reference's generateFeaturesConfig resolves no Features config for
a configuration without Features, and upgrade throws "Failed to update
lockfile" on it, exit 1. upgrade wrote, or with --dry-run printed,
{"features":{}} and exited 0.
BREAKING CHANGE: `upgrade` on a configuration without Features exits 1
with "Failed to update lockfile" and writes no lockfile.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The reference's upgrade builds the lockfile from readFeaturesConfig, which resolves the Features as build does: the dependency graph is computed, so Features pulled in by dependsOn are locked too, with each entry's dependsOn, and a Feature that cannot be fetched fails the command. upgrade looked up only the configured Features' manifests, skipped any it could not fetch and wrote no dependencies. upgrade now uses build's Feature resolver without the lockfile. BREAKING CHANGE: `upgrade` exits 1 when a Feature cannot be fetched instead of leaving it out of the lockfile, and the lockfile now lists the Features other Features depend on. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The reference's upgrade, given --feature and --target-version, first rewrites the Feature's key in devcontainer.json to "<id without version>:<target>" (updateFeatureVersionInConfig), even with --dry-run, and generates the lockfile from the configuration read again. upgrade validated both flags but never applied them. The port keeps the reference's semantics: the first key, in file order, naming the Feature is read as a regular expression and replaced everywhere in the file's text. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TS processFeatureIdentifier resolves a local Feature next to its
configuration and refuses it unless it lies in the workspace's
.devcontainer folder: an absolute id ("/x", which getFeatureIdType also
classifies as local), a configuration at the workspace root
(.devcontainer.json), "../x" or any relative path containing "..". It
logs the reason as an error and the dependency graph fails the Feature
with the plain "ERR: Feature '<id>' could not be processed." Error, which
build and up report as their error result's message.
Go resolved the folder next to the configuration wherever it was, took an
absolute id for a legacy one, and wrapped the graph's error in the
command's description. build, up, read-configuration, resolve-dependencies
and features info's graph now resolve local Features through the same
check, against the workspace folder (or the working directory without
one), as TS's params.cwd.
upgrade, which resolves its Features as build does, refuses such a
Feature too instead of writing it to the lockfile.
BREAKING CHANGE: a local Feature outside the workspace's .devcontainer/ folder now fails build, up, read-configuration and upgrade; move it under .devcontainer/.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TS writeLockfile writes devcontainer-lock.json with fs.writeFile, which creates it 0666 less the umask; Go created it 0644, so under a umask other than 022 (002, say) the two CLIs gave the new lockfile different permissions. WriteLockfile, which build and up write the lockfile through, now creates it 0666 less the umask; a lockfile it rewrites keeps its mode, as with fs.writeFile. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TS upgrade truncates the lockfile with writeLocalFile(lockfilePath, ''), fs.writeFile, so a lockfile it creates gets 0666 minus the umask. Go created it with 0644 before WriteLockfile kept that mode. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With --override-config and a workspace that has no devcontainer.json, TS keeps <workspace>/.devcontainer/devcontainer.json as configFilePath (getDefaultDevContainerConfigPath) and reads the override in its place, so a relative Dockerfile, context or local Feature in the override resolves against .devcontainer. Go named the override itself, resolving those paths next to it. Without a workspace, TS's configuration is the override alone; Go discovered a devcontainer.json in the current directory and merged the override onto it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TS reads --override-config in place of the configuration, so a missing override fails as "Dev container config (<path>) not found." naming the configuration's path (--config, the workspace's configuration or its default path, else the override), and read-configuration exits 1 without a message. Go failed with the override's raw read error. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rkspace With --container-id or --id-label and no --workspace-folder there is no workspace, and TS's read-configuration takes --override-config as the configuration (configPath = overrideConfigFile), printing it with a file URI as configFilePath. Go loaded nothing and printed an empty configuration. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t a folder Without a workspace folder, TS getWorkspaceConfiguration keeps the configuration's own workspaceFolder and workspaceMount, and read-configuration prints them as the workspace, leaving out what is unset. Go printed an empty workspace whenever --container-id came without --workspace-folder, and an empty workspaceFolder with --id-label. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ainer
TS readConfiguration substitutes ${devcontainerId} in the configuration
from the found container's id labels (beforeContainerSubstitute) before
its environment; Go left it as written. A missing workspace folder or
config file goes into those labels as TS's template literal writes
undefined (devcontainer.local_folder=undefined), where Go used an empty
value, so the id Go computed for a container's metadata differed with
--container-id alone.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TS findContainerAndIdLabels runs `docker ps -q -a --filter label=…` to find the container by --id-label or by the workspace's labels, and a failure throws: read-configuration exits 1 with nothing on stdout, logging "Command failed: docker ps …". Go ignored the failed lookup, also without a usable Docker engine, and printed the configuration with exit 0. exec, run-user-commands and stop keep treating a failed lookup as no container. BREAKING CHANGE: read-configuration exits 1 when its container lookup fails, including without a reachable Docker engine. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A ${localEnv} or ${containerEnv} without a variable name is a TS
ContainerError, "'${localEnv}' in devcontainer.json can not be resolved
because no environment variable name is given.", reported as both
message and description. Go prefixed it with "apply host
substitutions: " in up, build, run-user-commands, set-up, exec, upgrade
and read-configuration, and set-up described it as "An error occurred
running user commands in the container.".
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TS exec reads the configuration before it looks at the container: the workspace's, --config's (also without --workspace-folder) or --override-config's, and one that is named but missing fails with "Dev container config (…) not found.", even with --container-id. Go read a configuration only with --workspace-folder, ignored --override-config, and with --container-id ran the command without the configuration it could not load. BREAKING CHANGE: exec fails on a missing or invalid --config/--override-config, also with --container-id. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A ${containerEnv} without a name is TS's ContainerError, naming the
configuration's file, and TS substitutes the container's environment
into the configuration and the merged configuration before it uses the
container. read-configuration now exits 1 with nothing on stdout; set-up
and run-user-commands report the ContainerError as message and
description; exec exits 1; and up fails with the container's id, even
with --skip-post-create. Go wrote an envelope with a "resolve
configuration container variables: " prefix and no file name, checked
only what it reported or exec's remoteEnv, and otherwise succeeded.
BREAKING CHANGE: a ${containerEnv} reference without a variable name now fails up, set-up, run-user-commands, exec and read-configuration.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TS findContainerAndIdLabels throws when `docker ps` fails: run-user- commands reports "Command failed: docker ps -q -a --filter label=…" as the message with "An error occurred running user commands in the container." as the description, and exec fails with it. Go reported "Dev container not found.", exec looked further for a Compose service, and run-user-commands looked the container up by the workspace folder alone, without the configuration's file label. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TS outdated writes JSON.stringify(outdated, undefined, process.stdout.isTTY ? ' ' : undefined): one unindented line when stdout is redirected or piped, two-space indentation on a terminal, the empty result included. Go always indented it, and wrote the result of a configuration without Features unindented even on a terminal. Both now follow stdout: indented only when it is a terminal (an isatty check, as Node's isTTY; /dev/null is not one). A consumer that parses the JSON sees the same document; one that matched the indented text must parse it instead, as docs/migration.md says. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TS loadVersionInfo reorders the resolved Features to the order the configuration lists them in before JSON.stringify writes them; Go wrote them from a map, sorted by id. The text table already followed the configuration; the JSON result now does too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TS applyFeatureConfigToFeature builds a Feature entry as
{...featureJson, ...feature}: devcontainer-feature.json's properties in
the order it lists them, then the ones processFeatureIdentifier and
fetchFeatures set that the metadata lacks - {id, name, value, included}
for a local or tarball Feature, {id, included, value} for an OCI one,
then cachePath and consecutiveId. read-configuration's
featuresConfiguration wrote the Go struct's order instead, with value
after consecutiveId and containerEnv and customizations last.
A Feature now records its metadata's property order and the source's
properties, and is written in that order. Features built elsewhere keep
the struct's order. key_order paths can now index arrays, so parity cases
pin a local and an OCI Feature entry.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TS outdated writes its text table from loadVersionInfo's result, which has no rows for a configuration without Features, so it prints the header row alone. Go printed "No features configured." instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
read-configuration's featuresConfiguration is TS's FeaturesConfig as
JSON.stringify writes it. Go wrote its own structs instead, so key order
and content differed from the reference:
- A Feature entry is now built as TS builds it: the metadata spread over
the source's {id, name, value, included} (local, tarball) or
{id, included, value} (OCI) when the dependencies are read, cachePath
and consecutiveId added, that spread over the devcontainer-feature.json
and the deprecated extensions and settings moved into
customizations.vscode (updateFromOldProperties). Every metadata
property is kept, those Go has no field for and an empty options
included, with JSON.parse's semantics: a repeated property keeps its
first position and last value, array-index properties come first, and
nested objects such as options keep the file's order. An OCI Feature
reads its layer's devcontainer-feature.json as JSONC, as jsonc.parse
does.
- A Feature set writes internalVersion before computedDigest.
- OCI sourceInformation follows tryGetOCIFeatureSet's order, its manifest
is JSON.parse of the body the registry served, and featureRef is
getRef's object for the reference as written, not the lockfile's
pinned digest.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TS processFeatureIdentifier starts an OCI Feature as {id: <reference
id>, ...} and buildDependencyGraph spreads the Feature's metadata over
it - the manifest's dev.containers.metadata annotation, else the layer's
devcontainer-feature.json - so a published Feature whose metadata names
another id (a renamed one, such as ghcr.io/codspace/features/color:1,
now new-color) takes that id. Its consecutiveId, the folder its content
is staged in and the rename check of its install wrapper follow it.
Go kept the reference's id, so read-configuration reported
color_0 where TS reports new-color_0, and the build staged the Feature
under that name.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Array-index properties are ordered by comparing their canonical decimal forms instead of parsing them again, and isArrayIndex reports only whether a key is one. The tests now hold the 2^32-1 bound with 4294967294 and 4294967296, and the truthiness of a number in the deprecated-property move. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
gitRepo inherited GIT_DIR and GIT_WORK_TREE, which git rebase --exec exports, so TestDailyChangedCommand and TestReleaseResolve committed their fixtures into the repository under rebase. gitRepo now clears the variables that locate a repository. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Round 4 closes the parity follow-ups left by #42, plus what checking them turned up. Every behavior was confirmed by running the TS reference (v0.89.0) and this CLI side by side. Each fix comes with a regression test that fails before the fix and passes after: a parity matrix case, a TS-recorded ts-oracle golden, or a unit test.
Breaking changes
Each commit has a
BREAKING CHANGE:footer and a note in docs/migration.md.fix(config)!): mounted only when the config leavesworkspaceMountorworkspaceFolderunset, and placed under/workspaces/<main repo>/.git, as in v0.89.0. This removes an earlier backport of unreleased upstream #1261.upgrade(fix(cli)!, three commits):--log-levelaccepts only TS's choices.Failed to update lockfile.buildresolves them: an unfetchable Feature fails, and dependencies are included in the lockfile.fix(features)!): a local Feature outside the workspace's.devcontainer/fails, with TS's error.read-configuration(fix(cli)!): exits 1 when the container lookup fails, including when no Docker engine is reachable.exec(fix(cli)!): reads--config/--override-configwithout a workspace folder, and fails on a missing or invalid config.${containerEnv}without a variable name (fix(cli)!): failsup,set-up,run-user-commands,execandread-configuration, as TS does.Output that changes for scripts (migration notes added)
--log-format jsonevents are no longer HTML-escaped (&<>, U+2028/9), matchingJSON.stringify. The sameinternal/stringifyhelper now serves results and logs.docker compose configlogs at info with its output, and the version detection is logged, as in TS.outdated:read-configurationfeaturesConfigurationis built as TS builds it:manifestis the registry body as served;featureRefis TS'sgetRefobject.upgrade:Lockfile updated..--configreportsDev container config (...) not found..--feature/--target-versionnow rewrite the config, as in TS. They used to be ignored.Other fixes
--override-configwithout a base config:configFilePathis.devcontainer/devcontainer.json, and relative paths resolve from there.read-configuration --container-id --override-configwithout a workspace now reads the override (it printed{}), prints the configuredworkspace, and resolves${devcontainerId}. The id labels writeundefinedas TS does, which also affects exec and run-user-commands.apply host substitutions:prefix.execandrun-user-commandsis reported as TS reports it.nullworkspaceMountis kept as written;upthen mounts no workspace.build/upandupgrade.consecutiveIdand the staging folder follow a renamed Feature.test(devtools): the git fixtures no longer inheritGIT_DIR. Undergit rebase --execthey had committed into the repository being rebased and setcore.bare=truein it.Validation
go vet+go test ./...on each of the 32 code commitsgo test -race ./...(Docker unreachable)task linttask vulngo tool mutate -base origin/main -j 4task spec:compliancetask parity:contracttask parity:semantictask parity:networktask parity:runtime(Docker, full matrix)task test:e2etask testdata:ts-oracle:checkLeft for a next round
cfg.Featuresis a Go map. This affects builds, and the order inside each Feature'svaluetoo. Fixing it means carrying ordered maps through the config model.workspaceFolderthat substitutes to"": TS reports"", and Go keeps the raw string.upgradestderr: the resolver's progress lines differ from TS's.versiongets the tag in Go's lockfile, while TS leaves the version out.git rev-parse: Go walks the tree for.git.docker ps/inspect: Go uses the Engine API.🤖 Generated with Claude Code