diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index a4d0499..57e5b99 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -384,7 +384,7 @@ variables into readable validation phases with deterministic map traversal. ## Stage 5 — Split Composition Envelope Phases And Remove Incidental Metadata -**Status:** Pending +**Status:** Completed ### Goal diff --git a/internal/config/composition.go b/internal/config/composition.go index 3be31c9..4222850 100644 --- a/internal/config/composition.go +++ b/internal/config/composition.go @@ -26,18 +26,13 @@ type compositionNode struct { value string path string sources []string - line int - column int fields []compositionField items []*compositionNode } type compositionField struct { - key string - value *compositionNode - order int - line int - column int + key string + value *compositionNode } // compositionValueRecord is a deterministic semantic leaf projection. Lists @@ -104,7 +99,7 @@ func buildCompositionNode(node *yaml.Node, source, path string) (*compositionNod } result := &compositionNode{ kind: node.Kind, tag: node.Tag, value: node.Value, path: path, - sources: []string{source}, line: node.Line, column: node.Column, + sources: []string{source}, } switch node.Kind { @@ -139,10 +134,7 @@ func buildCompositionNode(node *yaml.Node, source, path string) (*compositionNod if err != nil { return nil, err } - result.fields = append(result.fields, compositionField{ - key: key, value: child, order: len(result.fields), - line: keyNode.Line, column: keyNode.Column, - }) + result.fields = append(result.fields, compositionField{key: key, value: child}) } case yaml.SequenceNode: for index, childNode := range node.Content { @@ -263,7 +255,6 @@ func mergeAdditiveNodes(base, incoming *compositionNode) (*compositionNode, erro index := compositionFieldIndex(base.fields, incomingField.key) if index < 0 { field := cloneCompositionField(incomingField) - field.order = len(base.fields) base.fields = append(base.fields, field) continue } @@ -322,7 +313,6 @@ func mergeOverlayNodes(base, overlay *compositionNode) (*compositionNode, error) index := compositionFieldIndex(base.fields, overlayField.key) if index < 0 { field := cloneCompositionField(overlayField) - field.order = len(base.fields) base.fields = append(base.fields, field) continue } @@ -644,7 +634,7 @@ func cloneCompositionNode(node *compositionNode) *compositionNode { } clone := &compositionNode{ kind: node.kind, tag: node.tag, value: node.value, path: node.path, - sources: append([]string(nil), node.sources...), line: node.line, column: node.column, + sources: append([]string(nil), node.sources...), } for _, field := range node.fields { clone.fields = append(clone.fields, cloneCompositionField(field)) @@ -656,10 +646,7 @@ func cloneCompositionNode(node *compositionNode) *compositionNode { } func cloneCompositionField(field compositionField) compositionField { - return compositionField{ - key: field.key, value: cloneCompositionNode(field.value), order: field.order, - line: field.line, column: field.column, - } + return compositionField{key: field.key, value: cloneCompositionNode(field.value)} } func compositionFieldIndex(fields []compositionField, key string) int { diff --git a/internal/config/effective_pipeline.go b/internal/config/effective_pipeline.go index 140adb7..a14c4c0 100644 --- a/internal/config/effective_pipeline.go +++ b/internal/config/effective_pipeline.go @@ -197,9 +197,6 @@ func removeResolutionOnlyPipelineFields(document *compositionDocument) { return } scriptorium.fields = append(scriptorium.fields[:familyIndex], scriptorium.fields[familyIndex+1:]...) - for index := range scriptorium.fields { - scriptorium.fields[index].order = index - } } func effectivePipelineFieldPaths(node *compositionNode) []string { diff --git a/internal/config/pipeline_composition.go b/internal/config/pipeline_composition.go index a8835f0..a750a2b 100644 --- a/internal/config/pipeline_composition.go +++ b/internal/config/pipeline_composition.go @@ -180,14 +180,24 @@ func splitPipelineCompositionEnvelope(document *compositionDocument) (*compositi if err := validateCompositionDocument(document, "root pipeline"); err != nil { return nil, pipelineCompositionEnvelope{}, err } - root := cloneCompositionNode(document.root) + envelope, fieldIndex, err := decodePipelineCompositionEnvelope(document.root) + if err != nil { + return nil, pipelineCompositionEnvelope{}, err + } + if err := validatePipelineCompositionEnvelope(envelope); err != nil { + return nil, pipelineCompositionEnvelope{}, err + } + return pipelineBodyWithoutComposition(document, fieldIndex), envelope, nil +} + +func decodePipelineCompositionEnvelope(root *compositionNode) (pipelineCompositionEnvelope, int, error) { index := compositionFieldIndex(root.fields, "composition") if index < 0 { - return &compositionDocument{root: root, sources: append([]string(nil), document.sources...)}, pipelineCompositionEnvelope{}, nil + return pipelineCompositionEnvelope{}, -1, nil } envelopeNode := root.fields[index].value if envelopeNode.kind != yaml.MappingNode { - return nil, pipelineCompositionEnvelope{}, fmt.Errorf( + return pipelineCompositionEnvelope{}, -1, fmt.Errorf( "configuration source %s at composition: expected a mapping, got %s", formatCompositionSources(envelopeNode.sources), yamlKindName(envelopeNode.kind), ) @@ -198,14 +208,14 @@ func splitPipelineCompositionEnvelope(document *compositionDocument) (*compositi switch field.key { case "imports": if field.value.kind != yaml.SequenceNode { - return nil, pipelineCompositionEnvelope{}, fmt.Errorf( + return pipelineCompositionEnvelope{}, -1, fmt.Errorf( "configuration source %s at composition.imports: expected a list, got %s", formatCompositionSources(field.value.sources), yamlKindName(field.value.kind), ) } for itemIndex, item := range field.value.items { if item.kind != yaml.ScalarNode || item.tag != "!!str" { - return nil, pipelineCompositionEnvelope{}, fmt.Errorf( + return pipelineCompositionEnvelope{}, -1, fmt.Errorf( "configuration source %s at composition.imports[%d]: expected a string path", formatCompositionSources(item.sources), itemIndex, ) @@ -214,54 +224,62 @@ func splitPipelineCompositionEnvelope(document *compositionDocument) (*compositi } case "default_profile": if field.value.kind != yaml.ScalarNode || field.value.tag != "!!str" { - return nil, pipelineCompositionEnvelope{}, fmt.Errorf( + return pipelineCompositionEnvelope{}, -1, fmt.Errorf( "configuration source %s at composition.default_profile: expected a string profile name", formatCompositionSources(field.value.sources), ) } name, err := normalizePipelineProfileName(field.value.value, "composition.default_profile") if err != nil { - return nil, pipelineCompositionEnvelope{}, err + return pipelineCompositionEnvelope{}, -1, err } envelope.defaultProfile = &name case "profiles": if field.value.kind != yaml.MappingNode { - return nil, pipelineCompositionEnvelope{}, fmt.Errorf( + return pipelineCompositionEnvelope{}, -1, fmt.Errorf( "configuration source %s at composition.profiles: expected a mapping, got %s", formatCompositionSources(field.value.sources), yamlKindName(field.value.kind), ) } if len(field.value.fields) == 0 { - return nil, pipelineCompositionEnvelope{}, fmt.Errorf("composition.profiles must declare at least one named profile") + return pipelineCompositionEnvelope{}, -1, fmt.Errorf("composition.profiles must declare at least one named profile") } for _, profileField := range field.value.fields { name, err := normalizePipelineProfileName(profileField.key, "composition.profiles profile name") if err != nil { - return nil, pipelineCompositionEnvelope{}, err + return pipelineCompositionEnvelope{}, -1, err } profile, err := parsePipelineProfileDeclaration(name, profileField.value) if err != nil { - return nil, pipelineCompositionEnvelope{}, err + return pipelineCompositionEnvelope{}, -1, err } envelope.profiles = append(envelope.profiles, profile) } default: - return nil, pipelineCompositionEnvelope{}, fmt.Errorf( + return pipelineCompositionEnvelope{}, -1, fmt.Errorf( "configuration source %s at composition.%s: unknown composition field %q", formatCompositionSources(field.value.sources), field.key, field.key, ) } } + return envelope, index, nil +} + +func validatePipelineCompositionEnvelope(envelope pipelineCompositionEnvelope) error { if envelope.defaultProfile != nil && !pipelineProfileDeclared(envelope.profiles, *envelope.defaultProfile) { - return nil, pipelineCompositionEnvelope{}, fmt.Errorf( + return fmt.Errorf( "composition.default_profile %q does not name a declared profile", *envelope.defaultProfile, ) } - root.fields = append(root.fields[:index], root.fields[index+1:]...) - for fieldIndex := range root.fields { - root.fields[fieldIndex].order = fieldIndex + return nil +} + +func pipelineBodyWithoutComposition(document *compositionDocument, compositionIndex int) *compositionDocument { + root := cloneCompositionNode(document.root) + if compositionIndex >= 0 { + root.fields = append(root.fields[:compositionIndex], root.fields[compositionIndex+1:]...) } - return &compositionDocument{root: root, sources: append([]string(nil), document.sources...)}, envelope, nil + return &compositionDocument{root: root, sources: append([]string(nil), document.sources...)} } func parsePipelineProfileDeclaration(name string, node *compositionNode) (pipelineProfileDeclaration, error) { diff --git a/internal/config/pipeline_composition_test.go b/internal/config/pipeline_composition_test.go index 75ce016..2b35c00 100644 --- a/internal/config/pipeline_composition_test.go +++ b/internal/config/pipeline_composition_test.go @@ -9,6 +9,82 @@ import ( "testing" ) +func TestSplitPipelineCompositionEnvelopePreservesInputAndDeclarationOrder(t *testing.T) { + document, err := parseCompositionBytes("pipeline.yml", []byte(`composition: + imports: [first.yml, second.yml] + default_profile: production + profiles: + production: + overlay: production.yml + testing: + overlay: testing.yml +workspace: + root: /srv/narratio +`)) + if err != nil { + t.Fatal(err) + } + originalRoot := cloneCompositionNode(document.root) + originalSources := append([]string(nil), document.sources...) + + body, envelope, err := splitPipelineCompositionEnvelope(document) + if err != nil { + t.Fatalf("splitPipelineCompositionEnvelope() error = %v", err) + } + if !reflect.DeepEqual(document.root, originalRoot) || !reflect.DeepEqual(document.sources, originalSources) { + t.Fatalf("splitPipelineCompositionEnvelope() mutated its input") + } + if compositionFieldIndex(body.root.fields, "composition") >= 0 { + t.Fatalf("pipeline body still contains composition envelope") + } + if compositionFieldIndex(body.root.fields, "workspace") < 0 { + t.Fatalf("pipeline body lost ordinary root fields") + } + if want := []string{"first.yml", "second.yml"}; !reflect.DeepEqual(envelope.imports, want) { + t.Fatalf("imports = %#v, want %#v", envelope.imports, want) + } + if len(envelope.profiles) != 2 || envelope.profiles[0].name != "production" || envelope.profiles[1].name != "testing" { + t.Fatalf("profiles = %#v, want declaration order", envelope.profiles) + } +} + +func TestSplitPipelineCompositionEnvelopeRejectsMalformedShapesWithoutMutation(t *testing.T) { + tests := []struct { + name string + yaml string + want string + }{ + {name: "envelope", yaml: "composition: []\n", want: "expected a mapping"}, + {name: "imports mapping", yaml: "composition:\n imports: {}\n", want: "expected a list"}, + {name: "import value", yaml: "composition:\n imports: [1]\n", want: "expected a string path"}, + {name: "default profile", yaml: "composition:\n default_profile: {}\n", want: "expected a string profile name"}, + {name: "profiles sequence", yaml: "composition:\n profiles: []\n", want: "expected a mapping"}, + {name: "empty profiles", yaml: "composition:\n profiles: {}\n", want: "at least one named profile"}, + {name: "profile declaration", yaml: "composition:\n profiles:\n production: production.yml\n", want: "expected a mapping"}, + {name: "invalid profile name", yaml: "composition:\n profiles:\n ' production ':\n overlay: production.yml\n", want: "surrounding whitespace"}, + {name: "unknown field", yaml: "composition:\n unexpected: true\n", want: "unknown composition field"}, + {name: "unknown default", yaml: "composition:\n default_profile: testing\n profiles:\n production:\n overlay: production.yml\n", want: "does not name a declared profile"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + document, err := parseCompositionBytes("pipeline.yml", []byte(test.yaml)) + if err != nil { + t.Fatal(err) + } + originalRoot := cloneCompositionNode(document.root) + + _, _, err = splitPipelineCompositionEnvelope(document) + if err == nil || !strings.Contains(err.Error(), test.want) { + t.Fatalf("splitPipelineCompositionEnvelope() error = %v, want containing %q", err, test.want) + } + if !reflect.DeepEqual(document.root, originalRoot) { + t.Fatalf("splitPipelineCompositionEnvelope() mutated invalid input") + } + }) + } +} + func TestLoadPipelineCompositionImportsDisjointFieldsAndTracksOwnership(t *testing.T) { dir := t.TempDir() rootPath := writePipelineSource(t, dir, "pipeline.yml", `composition: