fix(features): enforce dependsOn constraints in overrideFeatureInstallOrder

e2e/tests/up-features/testdata/docker-features-override-violates-depends-on/.devcontainer.json

@@ -0,0 +1,8 @@
+{
+  "image": "ghcr.io/devsy-org/test-images/base:alpine",
+  "features": {
+    "./features/base": {},
+    "./features/consumer": {}
+  },
+  "overrideFeatureInstallOrder": ["./features/consumer", "./features/base"]
+}

e2e/tests/up-features/testdata/docker-features-override-violates-depends-on/features/base/devcontainer-feature.json

@@ -0,0 +1,5 @@
+{
+  "id": "base",
+  "version": "0.1.0",
+  "name": "Base feature"
+}

e2e/tests/up-features/testdata/docker-features-override-violates-depends-on/features/base/install.sh

@@ -0,0 +1,3 @@
+#!/bin/bash
+set -e
+echo "base" >>/tmp/feature-install-order.txt

e2e/tests/up-features/testdata/docker-features-override-violates-depends-on/features/consumer/devcontainer-feature.json

@@ -0,0 +1,8 @@
+{
+  "id": "consumer",
+  "version": "0.1.0",
+  "name": "Consumer feature",
+  "dependsOn": {
+    "./features/base": {}
+  }
+}

e2e/tests/up-features/testdata/docker-features-override-violates-depends-on/features/consumer/install.sh

@@ -0,0 +1,3 @@
+#!/bin/bash
+set -e
+echo "consumer" >>/tmp/feature-install-order.txt

e2e/tests/up-features/testdata/docker-features-valid-override/.devcontainer.json

@@ -0,0 +1,13 @@
+{
+  "image": "ghcr.io/devsy-org/test-images/base:alpine",
+  "features": {
+    "./features/base": {},
+    "./features/alpha": {},
+    "./features/consumer": {}
+  },
+  "overrideFeatureInstallOrder": [
+    "./features/alpha",
+    "./features/base",
+    "./features/consumer"
+  ]
+}

e2e/tests/up-features/testdata/docker-features-valid-override/features/alpha/devcontainer-feature.json

@@ -0,0 +1,5 @@
+{
+  "id": "alpha",
+  "version": "0.1.0",
+  "name": "Alpha feature"
+}

e2e/tests/up-features/testdata/docker-features-valid-override/features/alpha/install.sh

@@ -0,0 +1,3 @@
+#!/bin/bash
+set -e
+echo "alpha" >>/tmp/feature-install-order.txt

e2e/tests/up-features/testdata/docker-features-valid-override/features/base/devcontainer-feature.json

@@ -0,0 +1,5 @@
+{
+  "id": "base",
+  "version": "0.1.0",
+  "name": "Base feature"
+}

e2e/tests/up-features/testdata/docker-features-valid-override/features/base/install.sh

@@ -0,0 +1,3 @@
+#!/bin/bash
+set -e
+echo "base" >>/tmp/feature-install-order.txt

e2e/tests/up-features/testdata/docker-features-valid-override/features/consumer/devcontainer-feature.json

@@ -0,0 +1,8 @@
+{
+  "id": "consumer",
+  "version": "0.1.0",
+  "name": "Consumer feature",
+  "dependsOn": {
+    "./features/base": {}
+  }
+}

e2e/tests/up-features/testdata/docker-features-valid-override/features/consumer/install.sh

@@ -0,0 +1,3 @@
+#!/bin/bash
+set -e
+echo "consumer" >>/tmp/feature-install-order.txt

e2e/tests/up-features/up_features.go

@@ -550,4 +550,52 @@ var _ = ginkgo.Describe("testing up command", ginkgo.Label("up-features", "suite
 		framework.ExpectNoError(err)
 		framework.ExpectEqual(strings.TrimSpace(out), "ubuntu")
 	}, ginkgo.SpecTimeout(framework.TimeoutShort()))
+
+	ginkgo.It(
+		"should reject overrideFeatureInstallOrder that violates dependsOn",
+		ginkgo.Label("features", "override"),
+		func(ctx context.Context) {
+			f, err := setupDockerProvider(initialDir+"/bin", "docker")
+			framework.ExpectNoError(err)
+
+			tempDir, err := framework.CopyToTempDir(
+				"tests/up-features/testdata/docker-features-override-violates-depends-on",
+			)
+			framework.ExpectNoError(err)
+			ginkgo.DeferCleanup(framework.CleanupTempDir, initialDir, tempDir)
+
+			wsName := filepath.Base(tempDir)
+			ginkgo.DeferCleanup(f.DevsyWorkspaceDelete, wsName)
+
+			err = f.DevsyUp(ctx, tempDir)
+			framework.ExpectError(err)
+		},
+		ginkgo.SpecTimeout(framework.TimeoutShort()),
+	)
+
+	ginkgo.It(
+		"should respect overrideFeatureInstallOrder when it satisfies dependsOn",
+		ginkgo.Label("features", "override"),
+		func(ctx context.Context) {
+			f, err := setupDockerProvider(initialDir+"/bin", "docker")
+			framework.ExpectNoError(err)
+
+			tempDir, err := framework.CopyToTempDir(
+				"tests/up-features/testdata/docker-features-valid-override",
+			)
+			framework.ExpectNoError(err)
+			ginkgo.DeferCleanup(framework.CleanupTempDir, initialDir, tempDir)
+
+			wsName := filepath.Base(tempDir)
+			ginkgo.DeferCleanup(f.DevsyWorkspaceDelete, wsName)
+
+			err = f.DevsyUp(ctx, tempDir)
+			framework.ExpectNoError(err)
+
+			out, err := f.DevsySSH(ctx, wsName, "cat /tmp/feature-install-order.txt")
+			framework.ExpectNoError(err)
+			gomega.Expect(strings.TrimSpace(out)).To(gomega.Equal("alpha\nbase\nconsumer"))
+		},
+		ginkgo.SpecTimeout(framework.TimeoutShort()),
+	)
 })

pkg/devcontainer/feature/extend.go

@@ -441,46 +441,93 @@ func getSortedFeatureSets(
 	devContainer *config.DevContainerConfig,
 	featureSets []*config.FeatureSet,
 ) ([]*config.FeatureSet, error) {
-	orderedFeatureSets, err := getOrderedFeatureSets(featureSets)
-	if err != nil {
-		return nil, err
+	if len(devContainer.OverrideFeatureInstallOrder) == 0 {
+		return getOrderedFeatureSets(featureSets)
 	}
 
-	if len(devContainer.OverrideFeatureInstallOrder) == 0 {
-		return orderedFeatureSets, nil
+	featureLookup := buildFeatureLookupMap(featureSets)
+	priority := buildOverridePriority(devContainer.OverrideFeatureInstallOrder, featureLookup)
+
+	if err := validateOverrideOrder(priority, featureSets, featureLookup); err != nil {
+		return nil, err
 	}
 
-	return sortFeaturesByOverride(devContainer.OverrideFeatureInstallOrder, orderedFeatureSets), nil
+	return getOrderedFeatureSetsWithPriority(featureSets, priority)
 }
 
-func sortFeaturesByOverride(
+func buildOverridePriority(
 	overrideOrder []string,
-	featureSets []*config.FeatureSet,
-) []*config.FeatureSet {
-	orderedFeatures := make([]*config.FeatureSet, 0, len(featureSets))
-	seen := make(map[string]bool)
-
-	for _, overrideFeatureID := range overrideOrder {
-		feature := extractFeatureByID(featureSets, overrideFeatureID)
-		if feature == nil {
-			normalizedID := normalizeFeatureID(overrideFeatureID)
-			feature = extractFeatureByID(featureSets, normalizedID)
+	featureLookup map[string]*config.FeatureSet,
+) map[string]int {
+	priority := make(map[string]int)
+	for i, id := range overrideOrder {
+		if _, exists := featureLookup[id]; exists {
+			priority[id] = i
+			continue
 		}
-
-		if feature != nil && !seen[feature.ConfigID] {
-			orderedFeatures = append(orderedFeatures, feature)
-			seen[feature.ConfigID] = true
+		normalizedID := normalizeFeatureID(id)
+		if _, exists := featureLookup[normalizedID]; exists {
+			priority[normalizedID] = i
 		}
 	}
+	return priority
+}
 
+func validateOverrideOrder(
+	priority map[string]int,
+	featureSets []*config.FeatureSet,
+	featureLookup map[string]*config.FeatureSet,
+) error {
 	for _, feature := range featureSets {
-		if !seen[feature.ConfigID] {
-			orderedFeatures = append(orderedFeatures, feature)
-			seen[feature.ConfigID] = true
+		featurePriority, featureHasPriority := priority[feature.ConfigID]
+		if !featureHasPriority {
+			continue
+		}
+
+		for depID := range feature.Config.DependsOn {
+			depConfigID := resolveDepConfigID(depID, featureLookup)
+			if depConfigID == "" {
+				continue
+			}
+			depPriority, depHasPriority := priority[depConfigID]
+			if !depHasPriority {
+				continue
+			}
+			if featurePriority < depPriority {
+				return fmt.Errorf(
+					"overrideFeatureInstallOrder places %q (position %d)"+
+						" before its dependency %q (position %d)",
+					feature.ConfigID,
+					featurePriority,
+					depConfigID,
+					depPriority,
+				)
+			}
 		}
 	}
+	return nil
+}
 
-	return orderedFeatures
+func resolveDepConfigID(depID string, featureLookup map[string]*config.FeatureSet) string {
+	if _, exists := featureLookup[depID]; exists {
+		return depID
+	}
+	normalizedID := normalizeFeatureID(depID)
+	if _, exists := featureLookup[normalizedID]; exists {
+		return normalizedID
+	}
+	return ""
+}
+
+func getOrderedFeatureSetsWithPriority(
+	features []*config.FeatureSet,
+	priority map[string]int,
+) ([]*config.FeatureSet, error) {
+	dependencyGraph, err := buildFeatureDependencyGraph(features)
+	if err != nil {
+		return nil, err
+	}
+	return dependencyGraph.SortWithPriority(priority)
 }
 
 func extractFeatureByID(features []*config.FeatureSet, featureID string) *config.FeatureSet {

pkg/devcontainer/feature/extend_test.go

@@ -330,7 +330,7 @@ func (suite *ExtendTestSuite) TestComputeFeatureOrder_NoOverride() {
 	}
 }
 
-func (suite *ExtendTestSuite) TestComputeFeatureOrder_WithOverride() {
+func (suite *ExtendTestSuite) TestComputeFeatureOrder_OverrideViolatesDependsOn() {
 	devContainer := &config.DevContainerConfig{
 		DevContainerConfigBase: config.DevContainerConfigBase{
 			OverrideFeatureInstallOrder: []string{"feature-a", "feature-b"},
@@ -347,17 +347,34 @@ func (suite *ExtendTestSuite) TestComputeFeatureOrder_WithOverride() {
 		{ConfigID: "feature-b", Config: &config.FeatureConfig{DependsOn: config.DependsOnField{}}},
 	}
 
+	_, err := getSortedFeatureSets(devContainer, features)
+	suite.Error(err)
+	suite.Contains(err.Error(), "overrideFeatureInstallOrder")
+	suite.Contains(err.Error(), "dependency")
+}
+
+func (suite *ExtendTestSuite) TestComputeFeatureOrder_ValidOverride() {
+	devContainer := &config.DevContainerConfig{
+		DevContainerConfigBase: config.DevContainerConfigBase{
+			OverrideFeatureInstallOrder: []string{"feature-b", "feature-a"},
+		},
+	}
+
+	features := []*config.FeatureSet{
+		{
+			ConfigID: "feature-a",
+			Config: &config.FeatureConfig{
+				DependsOn: config.DependsOnField{"feature-b": map[string]any{}},
+			},
+		},
+		{ConfigID: "feature-b", Config: &config.FeatureConfig{DependsOn: config.DependsOnField{}}},
+	}
+
 	order, err := getSortedFeatureSets(devContainer, features)
 	suite.Require().NoError(err)
 	suite.Len(order, 2)
-	if order[0].ConfigID != "feature-a" || order[1].ConfigID != "feature-b" {
-		suite.Failf(
-			"Order mismatch",
-			"Expected [feature-a, feature-b], got [%s, %s]",
-			order[0].ConfigID,
-			order[1].ConfigID,
-		)
-	}
+	suite.Equal("feature-b", order[0].ConfigID)
+	suite.Equal("feature-a", order[1].ConfigID)
 }
 
 func (suite *ExtendTestSuite) TestComputeFeatureOrder_PartialOverride() {
@@ -387,28 +404,42 @@ func (suite *ExtendTestSuite) TestComputeFeatureOrder_PartialOverride() {
 	}
 }
 
-func (suite *ExtendTestSuite) TestApplyManualOrdering() {
-	automaticOrder := []*config.FeatureSet{
-		{ConfigID: "feature-a"},
-		{ConfigID: "feature-b"},
-		{ConfigID: "feature-c"},
+func (suite *ExtendTestSuite) TestBuildOverridePriority() {
+	features := []*config.FeatureSet{
+		{ConfigID: "feature-a", Config: &config.FeatureConfig{DependsOn: config.DependsOnField{}}},
+		{ConfigID: "feature-b", Config: &config.FeatureConfig{DependsOn: config.DependsOnField{}}},
+		{ConfigID: "feature-c", Config: &config.FeatureConfig{DependsOn: config.DependsOnField{}}},
 	}
+	lookup := buildFeatureLookupMap(features)
 
 	overrideOrder := []string{"feature-c", "feature-a"}
-	result := sortFeaturesByOverride(overrideOrder, automaticOrder)
-	expected := []string{"feature-c", "feature-a", "feature-b"}
-	suite.Len(result, 3)
-	for i, expectedID := range expected {
-		if result[i].ConfigID != expectedID {
-			suite.Failf(
-				"Position mismatch",
-				"Position %d: expected %s, got %s",
-				i,
-				expectedID,
-				result[i].ConfigID,
-			)
-		}
+	priority := buildOverridePriority(overrideOrder, lookup)
+
+	suite.Equal(0, priority["feature-c"])
+	suite.Equal(1, priority["feature-a"])
+	_, hasB := priority["feature-b"]
+	suite.False(hasB)
+}
+
+func (suite *ExtendTestSuite) TestOverridePriorityAffectsSortOrder() {
+	devContainer := &config.DevContainerConfig{
+		DevContainerConfigBase: config.DevContainerConfigBase{
+			OverrideFeatureInstallOrder: []string{"feature-c", "feature-a"},
+		},
 	}
+
+	features := []*config.FeatureSet{
+		{ConfigID: "feature-a", Config: &config.FeatureConfig{DependsOn: config.DependsOnField{}}},
+		{ConfigID: "feature-b", Config: &config.FeatureConfig{DependsOn: config.DependsOnField{}}},
+		{ConfigID: "feature-c", Config: &config.FeatureConfig{DependsOn: config.DependsOnField{}}},
+	}
+
+	order, err := getSortedFeatureSets(devContainer, features)
+	suite.Require().NoError(err)
+	suite.Len(order, 3)
+	suite.Equal("feature-c", order[0].ConfigID)
+	suite.Equal("feature-a", order[1].ConfigID)
+	suite.Equal("feature-b", order[2].ConfigID)
 }
 
 func (suite *ExtendTestSuite) TestExtractFeatureByID() {