CLI sorts Feature options fewest-first, against spec

Open Beginner friendly
#755 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
76/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Quiet
Tech stack
typescript
Domain
cli

Research direction

Start in devcontainers/cli at src/spec-configuration/containerFeaturesOrder.ts, focusing on optionsCompareTo and its option-count ordering. Run the existing container-features order test at src/test/container-features/containerFeaturesOrder.test.ts with the configs/feature-dependencies/dependsOn/local-with-options fixture, then update the expected order so the greatest number of user-defined options comes first.

Written by the indexing model from the issue text.

Description

Note on repo choice: Per the devcontainers/cli CONTRIBUTING guide, issues concerning the CLI should be filed in this spec repository, so I've opened it here even though it's about devcontainers/cli behavior. Please let me know if you'd prefer it moved to the devcontainers/cli repo instead.

Summary

When sorting Features within an installation round, the spec says Features are ordered by the greatest number of user-defined options first, but containerFeaturesOrder.ts orders by the fewest options first.

Specification

Features spec — Definition: Round Stable Sort:

Greatest number of user-defined options (note omitting an option will default that value to the Feature's default value and is not considered a user-defined option)

Implementation

optionsCompareTo returns aKeys.length - bKeys.length, which is negative when a has fewer options, so the Feature with fewer options sorts earlier:

https://github.com/devcontainers/cli/blob/f683c29f64a20109b4453e5149807e390ff65133/src/spec-configuration/containerFeaturesOrder.ts#L123-L125

This is the option tiebreaker for every source type (OCI, file-path, direct-tarball, legacy).

Test evidence

The existing test valid dependsOn with round sorting based on options (fixture configs/feature-dependencies/dependsOn/local-with-options) pins the fewest-first behavior — the ./b variant with {} (zero options) is expected first, ahead of the two-option variants:

https://github.com/devcontainers/cli/blob/f683c29f64a20109b4453e5149807e390ff65133/src/test/container-features/containerFeaturesOrder.test.ts#L135-L209

So the implementation and its tests are internally consistent; only the behavior disagrees with the spec.

Proposed fix

Sort by the greatest option count to match the spec (bKeys.length - aKeys.length), with the fixture/expected order updated accordingly. This changes the install order of the same Feature requested with differing option sets within a round.

Dominant language
No language data
Stars
5.7k
Forks
497
No merged PRs in 30d

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from devcontainers/spec

All issues in devcontainers/spec

Similar issues

More CLI issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.