Repository navigation
fix(features): resolve Features in declaration order; stop treating http:// ids as tarballs - #76
Merged
Merged
Conversation
The reference builds its Feature list from Object.keys(config.features) followed by the --additional-features ids the configuration does not name, so integer-like ids come first, ascending, and the others keep their declaration order. Go sorted the ids, and features resolve-dependencies took Go map order. That decides which Feature is processed first: the first failing Feature reported, the order of the log lines and of the dependency graph's worklist. The loader now records the order of the features keys across the configuration and its override, mergeAdditionalFeatures appends the additional ids after them, and every resolution (up, build, read-configuration, upgrade, features resolve-dependencies) follows it. build no longer checks every id for a legacy one before resolving: the reference reports the first failing Feature in that order, after the image pull. Co-Authored-By: Claude Opus 5.5 <[email protected]>
The reference's getFeatureIdType takes only https:// ids as a direct tarball. An http:// id falls through to the OCI classification, where getRef rejects it without any request; the GitHub Releases fallback then refuses an id that is not owner/repo/feature, logs "Could not resolve Feature '<id>'", and the dependency graph fails with "ERR: Feature '<id>' could not be processed. You may not have permission ...". Go downloaded the tarball over plain HTTP. Any reference the OCI parser rejects and that fallback refuses now leaves the Feature unprocessed the same way, after the reference's warning and error lines. A reference the fallback would fetch from GitHub Releases keeps failing with the parse error, since this CLI does not implement that fallback. BREAKING CHANGE: an http:// Feature id no longer downloads a tarball; build, up, read-configuration and upgrade exit 1 with "ERR: Feature '<id>' could not be processed. ...". Serve the tarball over https:// (NODE_EXTRA_CA_CERTS or SSL_CERT_FILE can name a private CA) or publish the Feature to an OCI registry. Co-Authored-By: Claude Opus 5.5 <[email protected]>
aledbf
force-pushed
the
feature-resolution-order
branch
from
November 13, 2026 15:57
f1370a2 to
4217127
Compare
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.
Two drifts from the reference (TypeScript Dev Container CLI v0.90.0) in how Features are resolved.
1. Feature resolution order
What TS does.
generateFeaturesConfig→userFeaturesToArray(containerFeaturesConfiguration.ts) iteratesObject.keys(config.features), then appends the--additional-featuresids the configuration does not name, in their ownObject.keysorder. JS property order puts integer-like keys first, ascending, then the others in declaration order.features resolve-dependenciesuses the same list. That order decides which Feature is processed first: the first failing Feature reported, the log lines, and the dependency graph's worklist (the mermaid output).Before.
sortedUserFeaturessorted the ids (up, build, read-configuration, upgrade, compose).features resolve-dependenciestook Go map order, which is random.buildalso rancheckLegacyFeaturesbefore resolving anything: it reported a legacy id even when a Feature declared ahead of it fails first, and even before the image pull (TS reportsdocker pullfailing there).After.
featureskeys across the configuration and its override (DevContainer.FeatureOrder,features.KeyOrderof the concatenated member names, matching howdeepMergeConfigmerges).mergeAdditionalFeaturesappends the additional-only ids after them, in the additional JSON's JS order.features.UserFeaturesToArraytakes that order and is used everywheresortedUserFeatureswas, plusfeatures resolve-dependencies. It falls back to sorted order only for ids no order names (maps built by callers).checkLegacyFeaturesis removed. The legacy error still comes fromresolveFeatureID, in resolution order.Call sites checked against TS:
outdatedand theupgrade --featurerewrite already used the file's declaration order (orderedFeatureKeys). Integer-like ids are never OCI references, so they are unaffected. The disallowed-Features check (featureIDsInOrder) is unchanged.2.
http://Feature idsWhat TS does.
getFeatureIdTypetakes onlyhttps://asdirect-tarball. Anhttp://…id falls through tofetchOCIFeatureManifestIfExistsFromUserIdentifier. TheregetRefrejects it without a request (Path '/host/…' … failed validation), because the path always starts with/. TS then logsCould not resolve Feature manifest for '<id>'…and returnsgithub-repo.processFeatureIdentifier's GitHub branch refuses any id that is notowner/repo/featureand logsCould not resolve Feature '<id>'. Ensure the Feature is published…. It returnsundefined, andbuildDependencyGraphthrowsERR: Feature '<id>' could not be processed. You may not have permission to access this Feature, or may not be logged in. If the issue persists, report this to the Feature author.Before. Go downloaded the tarball over plain HTTP:
ClassifyIDacceptedhttp://.After.
ClassifyIDaccepts onlyhttps://. Whenoci.ParseRefrejects a reference that TS's GitHub Releases fallback would also refuse (more than one@, or notowner/repo/<[A-Za-z0-9_-]*>), the Feature is left unprocessed after logging the reference's warning and error lines, so the user sees the reference's error. A reference the fallback would fetch from GitHub Releases still fails with the parse error, because this CLI does not implement that fallback (already documented).This is a breaking change: a configuration with a working
http://Feature now fails. docs/migration.md has the migration: serve the tarball over HTTPS, usingNODE_EXTRA_CA_CERTS/SSL_CERT_FILEfor a private CA, or publish to an OCI registry. The commit usesfix(features)!:with aBREAKING CHANGE:footer.Tests
Each new test was checked against origin/main and fails there:
internal/config:TestLoadOrdersFeatureIDsAsJavaScriptEnumeratesThemcovers integer-like ids first and an override adding Features after the base's.TestLoadKeepsContainerEnvAndFeatureOptionOrderis extended forFeatureOrder.internal/features:TestUserFeaturesToArrayFollowsTheOrderThenSortsTheRest.TestClassifyIDnow expectshttp://to be OCI.internal/cli:TestReadConfigurationReportsTheFirstFeatureInDeclarationOrdercovers declaration order, integer-like ids, an override adding a Feature, and additional Features after the config's, including integer-like ones.TestReadConfigurationReportsAnAdditionalIntegerLikeIDAfterTheConfigs.TestFeaturesResolveDependenciesListsFeaturesInDeclarationOrder.TestMergeAdditionalFeaturesAppendsTheirOrderAfterTheConfigs.TestPlainHTTPFeatureIDIsNotATarball: no request is made, and both processors log the reference's lines.TestUnparseableFeatureReferences: the fallback's refusals, and the owner/repo/feature parse error that is kept.TestResolveFeatureIDRefusesLegacyIDskeeps the assertions of the removedTestCheckLegacyFeatures.gradleis dropped there because it never reachesresolveFeatureID;TestFetchFeatureSetsMovesDeprecatedFeaturesIntoOptionscovers it.serveTrustedTLS).getFeatureIdType("http://example.com/devcontainer-feature-node.tgz")returnsgithub-repo(generated withtask testdata:ts-oracle). The Go adapter maps a referenceoci.ParseRefrejects togithub-repo.features.resolve-dependencies-declaration-order-success(contract): mermaid order of 5 local Features.build.features-first-failure-in-declaration-order-error(runtime):../zetarefused before legacyzz.build.features-integer-like-ids-first-error(runtime):9before10.build.additional-features-after-config-error(runtime).build.features-http-id-not-a-tarball-error(runtime).Checks
All checks ran locally in the worktree. The shared host has a stray
/tmp/.gitand its Docker is shared with other runs.go build ./...,go vet ./...: passinternal/config,internal/features,internal/cli, withTMPDIR=/var/tmp/…): pass. The only failures are the parity cases that the/tmp/.gitworkspace breaks.go tool task lint: pass (0 issues; ctxlife, testquality, refs, mutate -stale-exempts, deadgate and taskgate all OK)TMPDIR=/var/tmp/… BASE=origin/main go tool task mutate: 195 refused, 0 survived, 5 did not buildTMPDIR=/var/tmp/… go tool task test:race: passgo tool task testdata:ts-oracleandtestdata:ts-oracle:check: pass (13 goldens)go tool task parity:contract: 115 matched. The 2 that failed areread-configuration.features-configuration-local-feature-{key-order,metadata}-success, the known/tmp/.gitcases.go tool task spec:compliance: passgo tool task test:e2e -- -timeout 40m: pass. The default 5m timeout ran out insideTestE2E_CABundleon the loaded shared Docker; that test passes alone./tmp/.git:read-configuration.merged-image-registry-features-success,read-configuration.features-configuration-oci-feature-key-order-success,read-configuration.feature-host-variables-merged-successandfeatures.test-single-scenario-success.upfailures were Docker contention. Rerun alone, 6 of 9 matched. The other 3 failed only on their cleanup command timing out (exit=-1), with no output difference.PARITY_COMMAND=build,features,read-configuration,upgraderun hit its 1h timeout on the shared Docker.🤖 Generated with Claude Code