fix(spec): verification fixes for HIP-0025 resource sequencing

- Prevent duplicate edges in DAG (corrupted in-degree count)
- Deduplicate dependency edges in ParseResourceGroups
- Fix disabled subchart detection logic contradicting documentation
- Fix findSubchart alias lookup to use parent chart's Dependencies
- Set SequencingInfo before deployment for failure recovery
- Handle zero Timeout without immediate deadline failure
- Restore chart dependency Enabled values in lint rule
- Add 'resource in multiple groups' lint check
- Update template delimiters to include chart path per HIP spec
- Replace custom contains helpers with strings.Contains

pkg/action/install.go

@@ -549,7 +549,14 @@ func (i *Install) performSequencedInstall(ctx context.Context, chrt *chart.Chart
 		waitForJobs:      i.WaitForJobs,
 		timeout:          i.Timeout,
 		readinessTimeout: readinessTimeout,
-		deadline:         time.Now().Add(i.Timeout),
+		deadline:         computeDeadline(i.Timeout),
+	}
+
+	// Set SequencingInfo before deployment so that failure recovery (e.g.,
+	// rollback-on-failure) can also use sequenced deletion order.
+	rel.SequencingInfo = &release.SequencingInfo{
+		Enabled:  true,
+		Strategy: string(i.WaitStrategy),
 	}
 
 	if err := sd.deployChartLevel(ctx, chrt, manifests); err != nil {
@@ -563,11 +570,6 @@ func (i *Install) performSequencedInstall(ctx context.Context, chrt *chart.Chart
 		}
 	}
 
-	rel.SequencingInfo = &release.SequencingInfo{
-		Enabled:  true,
-		Strategy: string(i.WaitStrategy),
-	}
-
 	if len(i.Description) > 0 {
 		rel.SetStatus(rcommon.StatusDeployed, i.Description)
 	} else {

pkg/action/rollback.go

@@ -361,7 +361,7 @@ func (r *Rollback) performSequencedRollback(ctx context.Context, currentRelease,
 		waitForJobs:      r.WaitForJobs,
 		timeout:          r.Timeout,
 		readinessTimeout: readinessTimeout,
-		deadline:         time.Now().Add(r.Timeout),
+		deadline:         computeDeadline(r.Timeout),
 		upgradeMode:      true,
 		currentResources: current,
 		// upgradeCSAFieldManager: always true for rollback (same as existing performRollback)

pkg/action/sequencing.go

@@ -30,6 +30,15 @@ import (
 	releaseutil "helm.sh/helm/v4/pkg/release/v1/util"
 )
 
+// computeDeadline returns a deadline based on the given timeout duration.
+// If timeout is zero or negative, it returns the zero time (no deadline).
+func computeDeadline(timeout time.Duration) time.Time {
+	if timeout <= 0 {
+		return time.Time{}
+	}
+	return time.Now().Add(timeout)
+}
+
 // GroupManifestsByDirectSubchart groups manifests by the direct subchart they belong to.
 // The parent chart's own manifests (templates directly under `<chartName>/templates/`) are
 // returned under the empty string key "".
@@ -383,13 +392,27 @@ func (s *sequencedDeployment) waitForResources(resources kube.ResourceList) erro
 }
 
 // findSubchart finds the subchart chart object within chrt's direct dependencies by name or alias.
+// It uses the parent chart's Metadata.Dependencies to resolve aliases, since the alias is stored
+// on the Dependency struct in Chart.yaml, not on the subchart's own Metadata.
 func findSubchart(chrt *chartv2.Chart, nameOrAlias string) *chartv2.Chart {
+	// Build a map from subchart chart name → alias from the parent's dependency declarations.
+	aliasMap := make(map[string]string) // chart name → effective name (alias or name)
+	if chrt.Metadata != nil {
+		for _, dep := range chrt.Metadata.Dependencies {
+			effective := dep.Name
+			if dep.Alias != "" {
+				effective = dep.Alias
+			}
+			aliasMap[dep.Name] = effective
+		}
+	}
+
 	for _, dep := range chrt.Dependencies() {
-		alias := dep.Metadata.Annotations["alias"]
-		if alias == "" {
-			alias = dep.Name()
+		effective := dep.Name()
+		if alias, ok := aliasMap[dep.Name()]; ok {
+			effective = alias
 		}
-		if alias == nameOrAlias || dep.Name() == nameOrAlias {
+		if effective == nameOrAlias || dep.Name() == nameOrAlias {
 			return dep
 		}
 	}

pkg/action/sequencing_test.go

@@ -122,10 +122,10 @@ func TestBuildManifestYAML(t *testing.T) {
 		t.Error("expected non-empty YAML output")
 	}
 	// Should contain content from both manifests
-	if !contains(result, "name: cm1") {
+	if !strings.Contains(result, "name: cm1") {
 		t.Error("expected cm1 in output")
 	}
-	if !contains(result, "name: cm2") {
+	if !strings.Contains(result, "name: cm2") {
 		t.Error("expected cm2 in output")
 	}
 }
@@ -137,20 +137,6 @@ func TestBuildManifestYAML_Empty(t *testing.T) {
 	}
 }
 
-// contains checks if s contains substr.
-func contains(s, substr string) bool {
-	return len(s) >= len(substr) && (s == substr || len(s) > 0 && containsStr(s, substr))
-}
-
-func containsStr(s, substr string) bool {
-	for i := 0; i <= len(s)-len(substr); i++ {
-		if s[i:i+len(substr)] == substr {
-			return true
-		}
-	}
-	return false
-}
-
 // TestInstallRelease_StoresSequencingInfo verifies that a sequenced install stores
 // SequencingInfo in the release record.
 func TestInstallRelease_StoresSequencingInfo(t *testing.T) {

pkg/action/upgrade.go

@@ -608,12 +608,18 @@ func (u *Upgrade) performSequencedUpgrade(ctx context.Context, chrt *chartv2.Cha
 		waitForJobs:           u.WaitForJobs,
 		timeout:               u.Timeout,
 		readinessTimeout:      readinessTimeout,
-		deadline:              time.Now().Add(u.Timeout),
+		deadline:              computeDeadline(u.Timeout),
 		upgradeMode:           true,
 		currentResources:      current,
 		upgradeCSAFieldManager: upgradeCSAFieldManager,
 	}
 
+	// Set SequencingInfo before deployment so that failure recovery can use sequenced order.
+	upgradedRelease.SequencingInfo = &release.SequencingInfo{
+		Enabled:  true,
+		Strategy: string(u.WaitStrategy),
+	}
+
 	if err := sd.deployChartLevel(ctx, chrt, manifests); err != nil {
 		return u.failRelease(upgradedRelease, nil, err)
 	}
@@ -644,11 +650,6 @@ func (u *Upgrade) performSequencedUpgrade(ctx context.Context, chrt *chartv2.Cha
 
 	currentRelease.Info.Status = rcommon.StatusSuperseded
 	u.cfg.recordRelease(currentRelease)
-
-	upgradedRelease.SequencingInfo = &release.SequencingInfo{
-		Enabled:  true,
-		Strategy: string(u.WaitStrategy),
-	}
 	upgradedRelease.Info.Status = rcommon.StatusDeployed
 	if len(u.Description) > 0 {
 		upgradedRelease.Info.Description = u.Description

pkg/chart/v2/lint/rules/sequencing.go

@@ -70,9 +70,17 @@ func validateSubchartSequencing(c *chart.Chart) error {
 
 	// In lint context, conditions/tags cannot be evaluated. Treat all deps as
 	// enabled to catch cycles that would manifest with any values configuration.
-	for _, dep := range c.Metadata.Dependencies {
+	// Save and restore original Enabled values to avoid mutating the chart.
+	origEnabled := make([]bool, len(c.Metadata.Dependencies))
+	for i, dep := range c.Metadata.Dependencies {
+		origEnabled[i] = dep.Enabled
 		dep.Enabled = true
 	}
+	defer func() {
+		for i, dep := range c.Metadata.Dependencies {
+			dep.Enabled = origEnabled[i]
+		}
+	}()
 
 	dag, err := chartutil.BuildSubchartDAG(c)
 	if err != nil {
@@ -149,6 +157,26 @@ func validateRenderedSequencingAnnotations(linter *support.Linter, c *chart.Char
 		})
 	}
 
+	// Check for resources assigned to multiple different groups.
+	resourceGroups := make(map[string]string) // "kind/name" → group
+	for _, m := range manifests {
+		if m.Head == nil || m.Head.Metadata == nil {
+			continue
+		}
+		group, hasGroup := m.Head.Metadata.Annotations[releaseutil.AnnotationResourceGroup]
+		if !hasGroup || group == "" {
+			continue
+		}
+		key := m.Head.Kind + "/" + m.Head.Metadata.Name
+		if prev, exists := resourceGroups[key]; exists && prev != group {
+			linter.RunLinterRule(support.ErrorSev, linter.ChartDir,
+				fmt.Errorf("resource %q is assigned to multiple resource-groups (%q and %q); each resource must belong to exactly one group",
+					key, prev, group))
+		} else {
+			resourceGroups[key] = group
+		}
+	}
+
 	// Check partial readiness annotations.
 	for _, m := range manifests {
 		if m.Head == nil || m.Head.Metadata == nil {

pkg/chart/v2/util/dag.go

@@ -29,6 +29,7 @@ import (
 type DAG struct {
 	nodes    map[string]struct{}
 	edges    map[string][]string // from -> []to (dependents)
+	edgeSet  map[string]struct{} // "from\x00to" -> exists (dedup guard)
 	inDegree map[string]int      // node -> number of prerequisites
 }
 
@@ -37,6 +38,7 @@ func NewDAG() *DAG {
 	return &DAG{
 		nodes:    make(map[string]struct{}),
 		edges:    make(map[string][]string),
+		edgeSet:  make(map[string]struct{}),
 		inDegree: make(map[string]int),
 	}
 }
@@ -63,6 +65,12 @@ func (d *DAG) AddEdge(from, to string) error {
 	if _, ok := d.nodes[to]; !ok {
 		return fmt.Errorf("unknown node %q", to)
 	}
+	// Silently skip duplicate edges to prevent in-degree corruption.
+	key := from + "\x00" + to
+	if _, exists := d.edgeSet[key]; exists {
+		return nil
+	}
+	d.edgeSet[key] = struct{}{}
 	d.edges[from] = append(d.edges[from], to)
 	d.inDegree[to]++
 	return nil

pkg/chart/v2/util/dag_test.go

@@ -201,6 +201,33 @@ func TestDAGGetBatches_NodesWithoutEdges(t *testing.T) {
 	}
 }
 
+func TestDAGAddEdge_DuplicateIdempotent(t *testing.T) {
+	d := NewDAG()
+	d.AddNode("a")
+	d.AddNode("b")
+	if err := d.AddEdge("a", "b"); err != nil {
+		t.Fatal(err)
+	}
+	// Adding the same edge again should be a no-op.
+	if err := d.AddEdge("a", "b"); err != nil {
+		t.Fatal(err)
+	}
+	batches, err := d.GetBatches()
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	// Should be 2 batches: [a] then [b], NOT corrupted by double in-degree.
+	if len(batches) != 2 {
+		t.Fatalf("expected 2 batches, got %d: %v", len(batches), batches)
+	}
+	if !containsExactly(batches[0], "a") {
+		t.Errorf("batch 0 should be [a], got %v", batches[0])
+	}
+	if !containsExactly(batches[1], "b") {
+		t.Errorf("batch 1 should be [b], got %v", batches[1])
+	}
+}
+
 // helper: checks slice contains exactly these elements (order-independent)
 func containsExactly(slice []string, items ...string) bool {
 	if len(slice) != len(items) {

pkg/chart/v2/util/subchart_dag.go

@@ -70,15 +70,11 @@ func BuildSubchartDAG(c *chart.Chart) (*DAG, error) {
 		if dep.Alias != "" {
 			effective = dep.Alias
 		}
+		// A dependency is disabled when Enabled is false AND there's no condition or
+		// tag that could re-enable it at runtime. Conditions and tags are evaluated
+		// at install-time from user-supplied values, so we can't resolve them here.
+		// If Enabled is false but a condition/tag is set, treat as potentially enabled.
 		disabled := !dep.Enabled && dep.Condition == "" && len(dep.Tags) == 0
-		// A dependency with Enabled=false explicitly is disabled.
-		// We also check: if Enabled is true OR there's a condition/tag, treat as
-		// potentially enabled (conditions are runtime values we can't evaluate here).
-		// For DAG purposes: if Enabled is explicitly false and no condition/tag,
-		// treat as disabled. Otherwise treat as enabled.
-		if !dep.Enabled {
-			disabled = true
-		}
 		byName[effective] = &depInfo{dep: dep, disabled: disabled}
 	}
 

pkg/cmd/template.go

@@ -280,14 +280,16 @@ func renderOrderedTemplate(manifest string, out io.Writer) error {
 	}
 
 	// Output each batch's groups with START/END delimiters.
-	// m.Content already contains the "# Source:" comment from rel.Manifest.
+	// Format per HIP-0025: ## START resource-group: <chart-path> <group-name>
+	// where chart-path is derived from the manifest source (e.g., "foo" or "foo/bar").
 	for _, batch := range batches {
 		for _, groupName := range batch {
-			fmt.Fprintf(out, "## START resource-group: %s\n", groupName)
+			chartPath := chartPathFromGroup(result.Groups[groupName])
+			fmt.Fprintf(out, "## START resource-group: %s %s\n", chartPath, groupName)
 			for _, m := range result.Groups[groupName] {
 				fmt.Fprintf(out, "---\n%s\n", m.Content)
 			}
-			fmt.Fprintf(out, "## END resource-group: %s\n", groupName)
+			fmt.Fprintf(out, "## END resource-group: %s %s\n", chartPath, groupName)
 		}
 	}
 
@@ -299,6 +301,34 @@ func renderOrderedTemplate(manifest string, out io.Writer) error {
 	return nil
 }
 
+// chartPathFromGroup extracts the chart/subchart path from the first manifest's
+// "# Source:" comment in its content.
+// For "# Source: mychart/templates/foo.yaml" → "mychart".
+// For "# Source: mychart/charts/sub/templates/bar.yaml" → "mychart/sub".
+func chartPathFromGroup(manifests []releaseutil.Manifest) string {
+	if len(manifests) == 0 {
+		return ""
+	}
+	content := manifests[0].Content
+	// Look for "# Source: <path>" line.
+	for _, line := range strings.Split(content, "\n") {
+		line = strings.TrimSpace(line)
+		if strings.HasPrefix(line, "# Source: ") {
+			source := strings.TrimPrefix(line, "# Source: ")
+			// Extract chart path up to "/templates/"
+			idx := strings.Index(source, "/templates/")
+			if idx < 0 {
+				return source
+			}
+			p := source[:idx]
+			// Collapse "parent/charts/sub" into "parent/sub" per HIP spec.
+			p = strings.ReplaceAll(p, "/charts/", "/")
+			return p
+		}
+	}
+	return ""
+}
+
 func isTestHook(h *release.Hook) bool {
 	return slices.Contains(h.Events, release.HookTest)
 }

pkg/cmd/testdata/output/template-ordered-delimiters.txt

@@ -1,4 +1,4 @@
-## START resource-group: databases
+## START resource-group: sequenced-chart databases
 ---
 # Source: sequenced-chart/templates/aa-databases-configmap.yaml
 apiVersion: v1
@@ -9,8 +9,8 @@ metadata:
     helm.sh/resource-group: databases
 data:
   host: localhost
-## END resource-group: databases
-## START resource-group: app
+## END resource-group: sequenced-chart databases
+## START resource-group: sequenced-chart app
 ---
 # Source: sequenced-chart/templates/bb-app-configmap.yaml
 apiVersion: v1
@@ -22,7 +22,7 @@ metadata:
     helm.sh/depends-on/resource-groups: '["databases"]'
 data:
   db_host: localhost
-## END resource-group: app
+## END resource-group: sequenced-chart app
 ---
 # Source: sequenced-chart/templates/cc-unsequenced-configmap.yaml
 apiVersion: v1

pkg/release/v1/util/resource_group.go

@@ -121,8 +121,17 @@ func ParseResourceGroups(manifests []Manifest) (ResourceGroupResult, []string) {
 		// Only record deps if we haven't already moved this group to unsequenced.
 		if _, ok := result.Groups[p.groupName]; ok {
 			if len(p.deps) > 0 {
-				existing := result.GroupDeps[p.groupName]
-				result.GroupDeps[p.groupName] = append(existing, p.deps...)
+				// Deduplicate deps to prevent in-degree corruption in the DAG.
+				seen := make(map[string]struct{})
+				for _, d := range result.GroupDeps[p.groupName] {
+					seen[d] = struct{}{}
+				}
+				for _, d := range p.deps {
+					if _, exists := seen[d]; !exists {
+						result.GroupDeps[p.groupName] = append(result.GroupDeps[p.groupName], d)
+						seen[d] = struct{}{}
+					}
+				}
 			}
 		}
 	nextPending: