Repository navigation
fix(features): keep JS property order in metadata, options and compose env - #75
Merged
Merged
Conversation
…e env The reference CLI writes these objects in JS property order (array-index keys ascending, then the others where each was first set); this CLI sorted them: - each entry's containerEnv in the devcontainer.metadata label, both a base image's entries as its label lists them and the configuration's as devcontainer.json declares it; - a Feature's value in read-configuration's featuresConfiguration; - the options of features resolve-dependencies' installOrder; - the environment of the compose override, which is the merged containerEnv, where a key keeps the position it first had (Object.assign over the base image, Features and configuration). An imagemeta.Entry now records its containerEnv's member order when it is decoded or built from the configuration, keeps it through host substitution, and MergeConfiguration carries the merged order. A Feature's value and the resolve-dependencies options are written through features.JSValue with the recorded option order. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…rder Every reader of MergedConfig.ContainerEnvOrder goes through features.PropertyOrder, which already keeps a key's first position, so deduplicating it in MergeConfiguration had no effect any test could see. Co-Authored-By: Claude Opus 5.5 <[email protected]>
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.
What TS does
The reference CLI (v0.90.0) writes these objects as JavaScript enumerates them: integer-index keys ascending, then the others in the order each was first set.
devcontainer.metadatalabel:getDevcontainerMetadataputs the base image's entries in the label asJSON.parseread them from its label, and the config's entry ispick(devContainerConfig.raw, …). So eachcontainerEnvkeeps its own declaration order.read-configurationfeaturesConfiguration: a Feature'svalueis the user's options object from devcontainer.json, or thedependsOnoptions.features resolve-dependencies:installOrder[].optionsisfeatures[0].value, the same object.generateFeaturesComposeOverrideContent, dockerCompose.ts): the override writesenvironmentfromObject.keys(mergedConfig.containerEnv). That object isObject.assign({}, ...entries.map(e => e.containerEnv)), so a key stays where it first appeared and takes the value of the last entry that sets it.Before / after
Config:
containerEnv: {"ZED":"1","10":"x","ALPHA":"2","2":"y"}; Feature options:{"zz":"1","10":"t","a":"b","2":"s"}; base image label:containerEnv: {"ZB":"1","10":"b","AB":"2","2":"a"}.{"10","2","AB","ZB"}{"2","10","ZB","AB"}{"10","2","ALPHA","ZED"}{"2","10","ZED","ALPHA"}{"2","10","a","zz"}(sorted){"2","10","zz","a"}{"10","2","a","zz"}{"2","10","zz","a"}ZB,10,AB+ configZED,AB,2,ALPHA)10, 2, AB, ALPHA, ZB, ZED2, 10, ZB, AB, ZED, ALPHAHow
The change reuses the #68 machinery and adds no second ordering mechanism:
imagemeta.Entry.ContainerEnvOrderis recorded on decode (jsonc.MemberNames). The config entries also set it, fromcfg.ContainerEnvOrder(configMetadataEntry) and from the loader's order (configLabelEntry). It surviveshostMetadataSubstitution, which round-trips the entries through maps.Entry.MarshalJSONwritescontainerEnvinfeatures.PropertyOrder.MergeConfigurationcollectsMergedConfig.ContainerEnvOrderwithfeatures.KeyOrder, which keeps the first position. The compose override iterates over that order.features.JSValue(v, declared)returns toJS output with a map's keys inPropertyOrder.Feature.MarshalJSONuses it forvaluewithValueOrder, and resolve-dependencies uses it foroptions.docs/DIVERGENCES.md: the label bullet now says thatcontainerEnvkeeps TS order.Tests
Each of these fails before the fix.
internal/imagemeta/testdata/ts-oracle/imagemeta.json, regenerated withtask testdata:ts-oracle):getDevcontainerMetadataLabelcase with per-entry containerEnv orders that include integer-like keys;mergeConfigurationContainerEnv(Object.entriesof the merged containerEnv) with a case where a key keeps its first position.TestFeatureMarshalJSONWritesTheValueInPropertyOrderTestJSValueOrdersAMapsKeysTestMetadataEntriesKeepTheContainerEnvOrder(configLabelEntry, host substitution, configMetadataEntry)TestGenerateComposeFeaturesOverrideWritesTheMergedEnvInPropertyOrder(label decode, then merge, then override)read-configuration.features-configuration-value-key-order-success(key_orderon the Feature's value)features.resolve-dependencies-option-key-order-success(stdout_text, user options and dependsOn options)build.metadata-label-container-env-order-success(image label)up.compose-container-env-order-success(override environment, container label, image label)TestEntryKeyOrdersNameEveryPropertynow skipsjson:"-"fields;ContainerEnvOrderis not a label property.Checks
go build ./...: passgo test ./internal/features ./internal/imagemeta ./internal/config: pass../internal/cli: pass, except parity contract cases that fail only on this machine because a stray/tmp/.gitmakes Go putworkspaceFolderat/workspaces/tmp. Those are the threeread-configuration.features-configuration-*cases below, plusoverride-config-*,key-orderandjson-stringify-escapes. They all differ only onworkspace.workspaceFolder.go tool task lint: passBASE=origin/main go tool task mutate: after the second commit, 160 refused, 0 survived, 7 did not build. The first commit's CI mutate run found a survivor: deduplicating the merged order inMergeConfigurationwas redundant withPropertyOrder. The second commit removes that step.TMPDIR=/var/tmp/… go tool task test:race: pass, with two exceptions:TestEntryKeyOrdersNameEveryPropertyfailed on the newjson:"-"field. It is fixed in this PR, andgo test -race ./internal/imagemetapasses.TestMetadataInteropGoRoundTripfailed ondocker inspect. Its fixed tagdcc-interop-rw006:testis shared with other agents on this Docker host. It passes when rerun alone.go tool task testdata:ts-oracle:check: passgo tool task parity:contract: pass, except the/tmp/.gitcasesread-configuration.features-configuration-local-feature-{key-order,metadata}-successand the newread-configuration.features-configuration-value-key-order-success. The new case fails for the sameworkspaceFolderreason and passes when its workspace is outside/tmp. CI has no/tmp/.git.go tool task spec:compliance: passPARITY_COMMAND=build,upruntime parity (run withgo test … -timeout 180m, because the task's 60m limit expired on this shared, loaded Docker host): 130 pass, 26 fail. Of the failures:remoteWorkspaceFolder/workspaceFolderbecause of the local/tmp/.git(up.dotfiles-local-*,up.workspace-secrets,up.config-file-fspath-*,up.override-config-scheme,up.include-configurations-key-order,up.image-with-parallel-initialize-command,up.container-entrypoint-and-command,up.read-only-workspace-dockerfile).up.compose-dockerfile-with-features-reuse-stopped-container-{with,without}-persisted-overrides,up.compose-with-name{,-custom-yaml,-devcontainer-env},up.image,up.compose-dockerfile-with-features,up.compose-{dockerfile-without,image-with}-features-omit-config-remote-env-from-metadata,build.no-cache-features,build.cache-to-local.build.metadata-label-container-env-order-successandup.compose-container-env-order-successpass.go tool task test:e2e: pass🤖 Generated with Claude Code