Skip to content

fix(features): resolve Features in declaration order; stop treating http:// ids as tarballs - #76

Merged
aledbf merged 2 commits into
mainfrom
feature-resolution-order
Nov 13, 2026
Merged

aledbf merged 2 commits into
mainfrom
feature-resolution-order

Conversation

@aledbf

@aledbf aledbf commented Nov 13, 2026 •

Copy link
Copy Markdown
Member

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) iterates Object.keys(config.features), then appends the --additional-features ids the configuration does not name, in their own Object.keys order. JS property order puts integer-like keys first, ascending, then the others in declaration order. features resolve-dependencies uses 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. sortedUserFeatures sorted the ids (up, build, read-configuration, upgrade, compose). features resolve-dependencies took Go map order, which is random. build also ran checkLegacyFeatures before resolving anything: it reported a legacy id even when a Feature declared ahead of it fails first, and even before the image pull (TS reports docker pull failing there).

After.

  • The loader records the order of the features keys across the configuration and its override (DevContainer.FeatureOrder, features.KeyOrder of the concatenated member names, matching how deepMergeConfig merges).
  • mergeAdditionalFeatures appends the additional-only ids after them, in the additional JSON's JS order.
  • features.UserFeaturesToArray takes that order and is used everywhere sortedUserFeatures was, plus features resolve-dependencies. It falls back to sorted order only for ids no order names (maps built by callers).
  • checkLegacyFeatures is removed. The legacy error still comes from resolveFeatureID, in resolution order.

Call sites checked against TS: outdated and the upgrade --feature rewrite 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 ids

What TS does. getFeatureIdType takes only https:// as direct-tarball. An http://… id falls through to fetchOCIFeatureManifestIfExistsFromUserIdentifier. There getRef rejects it without a request (Path '/host/…' … failed validation), because the path always starts with /. TS then logs Could not resolve Feature manifest for '<id>'… and returns github-repo. processFeatureIdentifier's GitHub branch refuses any id that is not owner/repo/feature and logs Could not resolve Feature '<id>'. Ensure the Feature is published…. It returns undefined, and buildDependencyGraph throws ERR: 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: ClassifyID accepted http://.

After. ClassifyID accepts only https://. When oci.ParseRef rejects a reference that TS's GitHub Releases fallback would also refuse (more than one @, or not owner/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, using NODE_EXTRA_CA_CERTS/SSL_CERT_FILE for a private CA, or publish to an OCI registry. The commit uses fix(features)!: with a BREAKING CHANGE: footer.

Tests

Each new test was checked against origin/main and fails there:

  • internal/config: TestLoadOrdersFeatureIDsAsJavaScriptEnumeratesThem covers integer-like ids first and an override adding Features after the base's. TestLoadKeepsContainerEnvAndFeatureOptionOrder is extended for FeatureOrder.
  • internal/features: TestUserFeaturesToArrayFollowsTheOrderThenSortsTheRest. TestClassifyID now expects http:// to be OCI.
  • internal/cli:
    • TestReadConfigurationReportsTheFirstFeatureInDeclarationOrder covers 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.
    • TestResolveFeatureIDRefusesLegacyIDs keeps the assertions of the removed TestCheckLegacyFeatures. gradle is dropped there because it never reaches resolveFeatureID; TestFetchFeatureSetsMovesDeprecatedFeaturesIntoOptions covers it.
    • The tests that served tarball Features over plain HTTP now use a trusted TLS test server (serveTrustedTLS).
  • ts-oracle: getFeatureIdType("http://example.com/devcontainer-feature-node.tgz") returns github-repo (generated with task testdata:ts-oracle). The Go adapter maps a reference oci.ParseRef rejects to github-repo.
  • Parity cases (docs/parity/parity-matrix.yaml). All match TS and all fail with the origin/main binary:
    • features.resolve-dependencies-declaration-order-success (contract): mermaid order of 5 local Features.
    • build.features-first-failure-in-declaration-order-error (runtime): ../zeta refused before legacy zz.
    • build.features-integer-like-ids-first-error (runtime): 9 before 10.
    • 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/.git and its Docker is shared with other runs.

  • go build ./..., go vet ./...: pass
  • Affected package tests (internal/config, internal/features, internal/cli, with TMPDIR=/var/tmp/…): pass. The only failures are the parity cases that the /tmp/.git workspace 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 build
  • TMPDIR=/var/tmp/… go tool task test:race: pass
  • go tool task testdata:ts-oracle and testdata:ts-oracle:check: pass (13 goldens)
  • go tool task parity:contract: 115 matched. The 2 that failed are read-configuration.features-configuration-local-feature-{key-order,metadata}-success, the known /tmp/.git cases.
  • go tool task spec:compliance: pass
  • go tool task test:e2e -- -timeout 40m: pass. The default 5m timeout ran out inside TestE2E_CABundle on the loaded shared Docker; that test passes alone.
  • Runtime parity: the 74 Feature/lockfile/upgrade cases across build, up, read-configuration, features and upgrade, including the 4 new build cases.
    • 61 matched.
    • 4 failed the same way with the origin/main binary, all from /tmp/.git: read-configuration.merged-image-registry-features-success, read-configuration.features-configuration-oci-feature-key-order-success, read-configuration.feature-host-variables-merged-success and features.test-single-scenario-success.
    • The compose up failures 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.
    • The full PARITY_COMMAND=build,features,read-configuration,upgrade run hit its 1h timeout on the shared Docker.
  • CI on this PR: all green, including parity-runtime (0), parity-runtime (1), e2e and mutate.

🤖 Generated with Claude Code

aledbf and others added 2 commits November 13, 2026 12:55
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
aledbf force-pushed the feature-resolution-order branch from f1370a2 to 4217127 Compare November 13, 2026 15:57
@aledbf
aledbf merged commit 2353da4 into main Nov 13, 2026
13 checks passed
@aledbf
aledbf deleted the feature-resolution-order branch November 13, 2026 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant