fix(server): guard DAG cycle check against filtered-out optional dependencies (#6915)

This commit is contained in:
Justin Leider
2026-07-29 02:27:06 +02:00
committed by GitHub
parent a816707e6e
commit ecfce6ea85
2 changed files with 47 additions and 1 deletions
+11 -1
View File
@@ -75,6 +75,16 @@ func (c dagCompiler) compileByDependsOn() ([]*backend_types.Stage, error) {
}
func dfsVisit(steps map[string]*dagCompilerStep, name string, visited map[string]struct{}, path []string) error {
step, exists := steps[name]
if !exists {
// `name` is an optional dependency on a step that was filtered out. convertDAGToStages
// drops those edges, but it resolves one step at a time and runs this walk in the same
// loop, so the walk can reach a step whose dependsOn is not resolved yet and follow an
// edge to a filtered-out step. There is nothing to traverse, and a missing *required*
// dependency is still reported by convertDAGToStages when it reaches that step.
return nil
}
if _, ok := visited[name]; ok {
return &ErrStepDependencyCycle{path: path}
}
@@ -82,7 +92,7 @@ func dfsVisit(steps map[string]*dagCompilerStep, name string, visited map[string
visited[name] = struct{}{}
path = append(path, name)
for _, dep := range steps[name].dependsOn {
for _, dep := range step.dependsOn {
if err := dfsVisit(steps, dep.Name, visited, path); err != nil {
return err
}
@@ -162,6 +162,42 @@ func TestOptionalStepDependency(t *testing.T) {
assert.Len(t, stages, 2, "should produce 2 stages (build then deploy)")
})
t.Run("missing optional step dep on a step that has dependents", func(t *testing.T) {
// convertDAGToStages resolves one step at a time and runs the cycle check in the
// same loop, so if it reaches `notify` before `deploy`, the walk follows deploy's
// still-unresolved edge to the absent `lint`. Map iteration order decides, so run
// this enough times to cover both orders.
for range 100 {
steps := map[string]*dagCompilerStep{
"build": {
position: 0,
name: "build",
step: &backend_types.Step{Name: "build"},
},
"deploy": {
position: 1,
name: "deploy",
step: &backend_types.Step{Name: "deploy"},
dependsOn: constraint.DependsOn{
{Name: "build"},
{Name: "lint", Optional: true},
},
},
"notify": {
position: 2,
name: "notify",
step: &backend_types.Step{Name: "notify"},
dependsOn: constraint.DependsOn{
{Name: "deploy"},
},
},
}
stages, err := convertDAGToStages(steps)
assert.NoError(t, err)
assert.Len(t, stages, 3, "build, then deploy, then notify")
}
})
t.Run("missing required step dep still errors", func(t *testing.T) {
steps := map[string]*dagCompilerStep{
"deploy": {