From 4c57ace2f64c09f4d8dfee3ebca2f5dd38644011 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sun, 30 Aug 2026 15:10:38 +0000 Subject: [PATCH] Add semantic configuration profile comparison --- docs/cli.md | 26 +- docs/config.md | 13 +- docs/internal/configuration.md | 13 +- docs/operations.md | 8 + docs/roadmap/implementation.md | 2 +- internal/app/config_commands.go | 8 +- internal/app/config_commands_test.go | 161 +++++++++++- internal/app/config_diff.go | 302 ++++++++++++++++++++++ internal/config/composition.go | 69 +++++ internal/config/effective_pipeline.go | 35 +++ internal/config/load.go | 81 +++++- internal/config/pipeline_composition.go | 70 ++++- internal/config/pipeline_profiles_test.go | 84 ++++++ 13 files changed, 847 insertions(+), 25 deletions(-) create mode 100644 internal/app/config_diff.go diff --git a/docs/cli.md b/docs/cli.md index 702efe5..151222f 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -20,7 +20,7 @@ Top-level commands: - `publish `: force-run publish. - `clean ` or `clean --all`: remove local work/spool state. - `session `: session helper commands. -- `config `: validate or display resolved pipeline configuration. +- `config `: validate, display, source-trace, or compare resolved pipeline configuration. Session subcommands: @@ -78,12 +78,13 @@ Commands with additional positionals keep their command-specific order: ## Command Reference -### `config validate`, `config show`, and `config sources` +### `config validate`, `config show`, `config sources`, and `config diff` ```bash narratio config validate [--config ] [--campaign | --campaign-file ] [--profile ] narratio config show [--config ] [--campaign | --campaign-file ] [--profile ] narratio config sources [--config ] [--campaign | --campaign-file ] [--profile ] +narratio config diff [--config ] [--campaign | --campaign-file ] ``` These commands resolve the selected profile, defaults, ordinary paths, and—if @@ -112,6 +113,27 @@ legacy-player, and generated family ownership. A generated party member has one family record and one party record at the same logical path. The output never reads or prints secret values. +`config diff` resolves both supplied profile names from one parsed root source +set and compares their fully resolved, secret-free effective mappings. It does +not accept `--profile`; the two positional names must be distinct, declared +profiles. When party-driven families are present, both profiles must resolve to +the same selected campaign and party. Use `--campaign-file` if profile-specific +campaign configuration would otherwise select different files. + +Equal profiles print `no differences`. Otherwise, sorted tab-separated records +use one of these forms, with compact JSON values: + +```text +added +removed +changed +``` + +Mappings are flattened to their logical field paths; lists remain one atomic +value. The command compares defaulted concrete artifacts and publish rules, not +profile names, source-file layout, or formatting. It succeeds when differences +are found, making it suitable for review and migration checks. + ### `version` ```bash diff --git a/docs/config.md b/docs/config.md index c0ddad0..4ada2ee 100644 --- a/docs/config.md +++ b/docs/config.md @@ -48,6 +48,9 @@ the durable copied session input. sources` use the same `--config`, `--campaign`, `--campaign-file`, and `--profile` selection rules as pipeline commands, but do not select, discover, or load a session. They do not read credential values or create runtime state. +`narratio config diff ` uses the same pipeline and +campaign selectors, resolves each named profile independently from one parsed +root source set, and does not accept a separate `--profile` flag. Campaign selection is optional only when the resolved pipeline has no `scriptorium.artifact_families`. When families are declared, Narratio selects a @@ -60,9 +63,17 @@ composition and family declarations omitted. `config sources` prints a stable source projection instead of effective values: root/import/profile/default ownership plus campaign/party and generated-family records. Canonical derived players trace to the party; a legacy configured players file is explicitly -marked as a legacy player source. The [CLI reference](cli.md#config-validate-config-show-and-config-sources) +marked as a legacy player source. The [CLI reference](cli.md#config-validate-config-show-config-sources-and-config-diff) owns command syntax and output conventions. +`config diff` compares normalized field values rather than YAML text or source +ownership. It emits sorted `added`, `removed`, and `changed` records, uses +compact deterministic JSON values, treats lists atomically, and reports `no +differences` when the complete effective configurations are equal. Concrete +family members and generated publish rules participate after expansion; moving +an equal value between eligible root/import sources does not create a +difference. + ### Identity segments Campaign IDs (`campaign_id` and `default_campaign_id`), session IDs, previous diff --git a/docs/internal/configuration.md b/docs/internal/configuration.md index b4ae1e5..bc6de6b 100644 --- a/docs/internal/configuration.md +++ b/docs/internal/configuration.md @@ -53,7 +53,7 @@ a temporary downloaded session file therefore cannot invalidate the resolved pipeline or campaign context. The application also has a separate read-only inspection resolver for `config -validate` and `config show`. It uses the same production root/profile and +validate`, `config show`, and `config sources`. It uses the same production root/profile and campaign selection functions, but never routes through session discovery, remote-session download, secret loading, adapter composition, workspace initialization, manifest access, or cleanup. A pipeline with retained artifact @@ -79,6 +79,17 @@ derived players point to the party source, while legacy players retain a dedicated legacy-player role. The application command only joins these sorted records with selection metadata and never reparses configuration files. +`config diff` uses a paired profile loader that parses the root, imports, and +declared overlays once, then clones the additive base before independently +selecting, decoding, defaulting, and finalizing each profile. When campaign +resolution is needed, the command loads one selected campaign and party and +expands both effective pipelines from that same party value. The configuration +owner projects each normalized effective mapping into sorted logical paths; +mapping leaves are compared individually while sequence values remain atomic. +Values are compact deterministic JSON representations for command output, not +raw YAML fragments, ownership records, or secret material. A differing digest +with no projected difference is treated as an internal consistency error. + Campaign context construction also reads and classifies the campaign-owned party source through `ParseParty`. A canonical party retains its raw bytes and normalized roster in runtime-only `ResolvedParty` provenance, while a legacy diff --git a/docs/operations.md b/docs/operations.md index 812c791..bb9d508 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -75,6 +75,14 @@ and effective configuration digest. Status distinguishes the current resolved value from the last value persisted in the session manifest, which helps diagnose profile switches without changing resume authority. +Before switching an operational profile, compare its effective meaning with the +current selection through `narratio config diff `. +The command is read-only and succeeds whether it finds differences or not. Its +sorted records describe defaulted, expanded concrete configuration—not source +file layout—so it can be used to review model, artifact, and publish changes +without creating a session or run. Select the same campaign explicitly when +profiles could resolve different campaign paths; see the [CLI reference](cli.md#config-validate-config-show-config-sources-and-config-diff) for syntax and record format. + ## Stage Execution and Continuation Behavior Canonical stage order: diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index b7dc6af..e75c1ff 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -1048,7 +1048,7 @@ generated configuration without leaking secrets or internal YAML machinery. ## Stage 19 — Semantic `config diff` -**Status: Pending** +**Status: Completed** ### Goal diff --git a/internal/app/config_commands.go b/internal/app/config_commands.go index 5d0a779..f308aec 100644 --- a/internal/app/config_commands.go +++ b/internal/app/config_commands.go @@ -30,7 +30,7 @@ type inspectionConfig struct { // Config dispatches read-only pipeline configuration commands. func Config(ctx context.Context, args []string, out io.Writer) error { if len(args) == 0 { - return fmt.Errorf("config: expected subcommand: validate|show|sources") + return fmt.Errorf("config: expected subcommand: validate|show|sources|diff") } if args[0] == "--help" || args[0] == "-h" { writeConfigCommandUsage(out) @@ -43,6 +43,8 @@ func Config(ctx context.Context, args []string, out io.Writer) error { return ConfigShow(ctx, args[1:], out) case "sources": return ConfigSources(ctx, args[1:], out) + case "diff": + return ConfigDiff(ctx, args[1:], out) default: return fmt.Errorf("config: unknown subcommand %q", args[0]) } @@ -234,9 +236,9 @@ func inspectionProfile(pipeline *config.PipelineConfig) string { } func writeConfigCommandUsage(out io.Writer) { - fmt.Fprintln(out, "Usage: narratio config ") + fmt.Fprintln(out, "Usage: narratio config ") fmt.Fprintln(out) - fmt.Fprintln(out, "Use config validate to check an effective pipeline, config show to print normalized YAML, or config sources to report ownership.") + fmt.Fprintln(out, "Use config validate to check an effective pipeline, config show to print normalized YAML, config sources to report ownership, or config diff to compare two profiles.") } func writeInspectionSourceHeader(out io.Writer, resolved *inspectionConfig) error { diff --git a/internal/app/config_commands_test.go b/internal/app/config_commands_test.go index 37d07f7..c06e4d2 100644 --- a/internal/app/config_commands_test.go +++ b/internal/app/config_commands_test.go @@ -21,12 +21,13 @@ func TestConfigCommandHelpAndDispatch(t *testing.T) { want string code int }{ - {name: "top level help", args: []string{"config", "--help"}, want: "Usage: narratio config "}, + {name: "top level help", args: []string{"config", "--help"}, want: "Usage: narratio config "}, {name: "validate help", args: []string{"config", "validate", "--help"}, want: "Usage: narratio config validate"}, {name: "show help", args: []string{"config", "show", "--help"}, want: "Usage: narratio config show"}, {name: "sources help", args: []string{"config", "sources", "--help"}, want: "Usage: narratio config sources"}, - {name: "unknown", args: []string{"config", "diff"}, want: `config: unknown subcommand "diff"`, code: 1}, - {name: "missing", args: []string{"config"}, want: "config: expected subcommand: validate|show|sources", code: 1}, + {name: "diff help", args: []string{"config", "diff", "--help"}, want: "Usage: narratio config diff"}, + {name: "unknown", args: []string{"config", "unknown"}, want: `config: unknown subcommand "unknown"`, code: 1}, + {name: "missing", args: []string{"config"}, want: "config: expected subcommand: validate|show|sources|diff", code: 1}, {name: "session flag", args: []string{"config", "validate", "--session", "session.yml"}, want: "flag provided but not defined", code: 1}, {name: "stage flag", args: []string{"config", "show", "--from", "prepare"}, want: "flag provided but not defined", code: 1}, {name: "force flag", args: []string{"config", "show", "--force"}, want: "flag provided but not defined", code: 1}, @@ -94,6 +95,14 @@ whisperx: if strings.Contains(sourcesOut.String(), secret) { t.Fatalf("sources output leaked a raw secret: %q", sourcesOut.String()) } + writeInspectionProfiles(t, pipelinePath, "whisperx:\n language: en\n", "whisperx:\n language: fr\n") + var diffOut bytes.Buffer + if err := ConfigDiff(context.Background(), []string{"production", "testing", "--config", pipelinePath}, &diffOut); err != nil { + t.Fatalf("ConfigDiff() error = %v", err) + } + if strings.Contains(diffOut.String(), secret) { + t.Fatalf("diff output leaked a raw secret: %q", diffOut.String()) + } if calls != 0 { t.Fatalf("inspection invoked configured external endpoint %d time(s)", calls) } @@ -102,6 +111,152 @@ whisperx: } } +func TestConfigDiffReportsSemanticProfileChanges(t *testing.T) { + workspaceRoot := t.TempDir() + pipelinePath, _, _ := writeValidConfigFiles(t, workspaceRoot) + writeInspectionProfiles(t, pipelinePath, `workspace: + cleanup_after_publish: true +whisperx: + language: en +audita: + modules: [glossary, grammar] +scriptorium: + artifacts: + production_only: + enabled: false + output_path: artifacts/production.md +`, `workspace: + cleanup_after_publish: false +whisperx: + language: fr +audita: + modules: [] +scriptorium: + artifacts: + testing_only: + enabled: false + output_path: artifacts/testing.md +`) + + args := []string{"production", "testing", "--config", pipelinePath} + var first, second bytes.Buffer + if err := ConfigDiff(context.Background(), args, &first); err != nil { + t.Fatalf("ConfigDiff() error = %v", err) + } + if err := ConfigDiff(context.Background(), args, &second); err != nil { + t.Fatalf("second ConfigDiff() error = %v", err) + } + if first.String() != second.String() { + t.Fatalf("diff output is not deterministic:\nfirst=%q\nsecond=%q", first.String(), second.String()) + } + output := first.String() + for _, want := range []string{ + "changed\taudita.modules\t[\"glossary\",\"grammar\"]\t[]\n", + "changed\tworkspace.cleanup_after_publish\ttrue\tfalse\n", + "changed\twhisperx.language\t\"en\"\t\"fr\"\n", + "removed\tscriptorium.artifacts.production_only.enabled\tfalse\n", + "added\tscriptorium.artifacts.testing_only.enabled\tfalse\n", + } { + if !strings.Contains(output, want) { + t.Fatalf("diff output missing %q:\n%s", want, output) + } + } + if strings.Index(output, "audita.modules") > strings.Index(output, "workspace.cleanup_after_publish") { + t.Fatalf("diff records are not sorted by path:\n%s", output) + } + var reversed bytes.Buffer + if err := ConfigDiff(context.Background(), []string{"testing", "production", "--config", pipelinePath}, &reversed); err != nil { + t.Fatalf("reversed ConfigDiff() error = %v", err) + } + if !strings.Contains(reversed.String(), "changed\twhisperx.language\t\"fr\"\t\"en\"\n") { + t.Fatalf("reversed diff did not independently resolve profiles:\n%s", reversed.String()) + } +} + +func TestConfigDiffComparesExpandedFamiliesAndPublishRules(t *testing.T) { + pipelinePath, campaignPath := writeInspectionFamilyConfig(t) + writeInspectionProfiles(t, pipelinePath, `scriptorium: + artifact_families: + character_note: + enabled: false + member_vars: + character_name: character.name + publish: + enabled: true + required: false +`, `scriptorium: + artifact_families: + character_note: + enabled: true + prompt_id: dnd.character_note + member_vars: + character_name: character.class_summary + publish: + enabled: true + required: true +`) + + var out bytes.Buffer + if err := ConfigDiff(context.Background(), []string{ + "production", "testing", "--config", pipelinePath, "--campaign-file", campaignPath, + }, &out); err != nil { + t.Fatalf("ConfigDiff() error = %v", err) + } + output := out.String() + for _, want := range []string{ + "changed\tscriptorium.artifacts.character_note_arannis.enabled\tfalse\ttrue\n", + "changed\tscriptorium.artifacts.character_note_arannis.vars.character_name\t\"Arannis\"\t\"wizard\"\n", + "changed\tpublish.outputs\t", + } { + if !strings.Contains(output, want) { + t.Fatalf("family diff output missing %q:\n%s", want, output) + } + } +} + +func TestConfigDiffReportsEqualityAndRejectsInvalidInput(t *testing.T) { + workspaceRoot := t.TempDir() + pipelinePath, campaignPath, _ := writeValidConfigFiles(t, workspaceRoot) + writeInspectionProfiles(t, pipelinePath, "whisperx:\n language: en\n", "whisperx:\n language: en\n") + + var equal bytes.Buffer + if err := ConfigDiff(context.Background(), []string{"production", "testing", "--config", pipelinePath}, &equal); err != nil { + t.Fatalf("ConfigDiff() equal profiles error = %v", err) + } + if got := equal.String(); got != "no differences\n" { + t.Fatalf("equal diff output = %q", got) + } + + for _, args := range [][]string{ + {"production", "testing", "extra", "--config", pipelinePath}, + {"production", "--config", pipelinePath}, + {"production", "production", "--config", pipelinePath}, + {"unknown", "testing", "--config", pipelinePath}, + {"production", "testing", "--config", pipelinePath, "--profile", "production"}, + {"production", "testing", "--config", pipelinePath, "--config", pipelinePath}, + {"production", "testing", "--config", pipelinePath, "--campaign", "sample-campaign", "--campaign-file", campaignPath}, + } { + if err := ConfigDiff(context.Background(), args, io.Discard); err == nil { + t.Fatalf("ConfigDiff(%q) succeeded, want error", args) + } + } +} + +func writeInspectionProfiles(t *testing.T, pipelinePath, production, testing string) { + t.Helper() + data, err := os.ReadFile(pipelinePath) + if err != nil { + t.Fatal(err) + } + composition := "composition:\n default_profile: production\n profiles:\n production:\n overlay: production.yml\n testing:\n overlay: testing.yml\n" + if err := os.WriteFile(pipelinePath, append([]byte(composition), data...), 0o644); err != nil { + t.Fatal(err) + } + dir := filepath.Dir(pipelinePath) + mustWriteTestFile(t, filepath.Join(dir, "production.yml"), production) + mustWriteTestFile(t, filepath.Join(dir, "testing.yml"), testing) +} + func TestConfigCommandsSelectProfilesAndCampaigns(t *testing.T) { workspaceRoot := t.TempDir() pipelinePath, campaignPath, _ := writeValidConfigFiles(t, workspaceRoot) diff --git a/internal/app/config_diff.go b/internal/app/config_diff.go new file mode 100644 index 0000000..9f4b05c --- /dev/null +++ b/internal/app/config_diff.go @@ -0,0 +1,302 @@ +package app + +import ( + "context" + "errors" + "flag" + "fmt" + "io" + "sort" + "strings" + + "gitea.maximumdirect.net/eric/narratio/internal/config" +) + +type configDiffFlags struct { + pipelinePath singletonStringFlag + campaignPath singletonStringFlag + campaignFilePath singletonStringFlag +} + +type configDiffRequest struct { + leftProfile string + rightProfile string + flags configDiffFlags +} + +type configDiffResolved struct { + left *inspectionConfig + right *inspectionConfig +} + +type configDiffRecord struct { + Kind string + Path string + Left string + Right string +} + +// ConfigDiff compares two explicitly selected profiles through the same +// read-only configuration resolution boundary used by config validate, show, +// and sources. Differences describe normalized effective values rather than +// the layout of root, import, or overlay files. +func ConfigDiff(_ context.Context, args []string, out io.Writer) error { + request, err := parseConfigDiffRequest(args, out) + if err != nil { + if errors.Is(err, flag.ErrHelp) { + return nil + } + return err + } + resolved, err := resolveConfigDiff(request) + if err != nil { + return fmt.Errorf("config diff: %w", err) + } + leftValues, err := config.EffectivePipelineValues(resolved.left.Pipeline) + if err != nil { + return fmt.Errorf("config diff: project left effective configuration: %w", err) + } + rightValues, err := config.EffectivePipelineValues(resolved.right.Pipeline) + if err != nil { + return fmt.Errorf("config diff: project right effective configuration: %w", err) + } + records, err := diffEffectivePipelineValues(leftValues, rightValues) + if err != nil { + return fmt.Errorf("config diff: %w", err) + } + leftDigest := config.EffectivePipelineDigest(resolved.left.Pipeline) + rightDigest := config.EffectivePipelineDigest(resolved.right.Pipeline) + if len(records) == 0 { + if leftDigest != rightDigest { + return fmt.Errorf("internal error: effective pipeline digests differ without a semantic difference") + } + _, err := fmt.Fprintln(out, "no differences") + return err + } + for _, record := range records { + switch record.Kind { + case "added": + if _, err := fmt.Fprintf(out, "added\t%s\t%s\n", record.Path, record.Right); err != nil { + return err + } + case "removed": + if _, err := fmt.Fprintf(out, "removed\t%s\t%s\n", record.Path, record.Left); err != nil { + return err + } + case "changed": + if _, err := fmt.Fprintf(out, "changed\t%s\t%s\t%s\n", record.Path, record.Left, record.Right); err != nil { + return err + } + default: + return fmt.Errorf("internal error: unsupported difference kind %q", record.Kind) + } + } + return nil +} + +func parseConfigDiffRequest(args []string, out io.Writer) (configDiffRequest, error) { + profiles, flagArgs := splitConfigDiffArguments(args) + fs := flag.NewFlagSet("config diff", flag.ContinueOnError) + fs.SetOutput(out) + var request configDiffRequest + request.flags.pipelinePath.name = "config" + request.flags.campaignPath.name = "campaign" + request.flags.campaignFilePath.name = "campaign-file" + fs.Var(&request.flags.pipelinePath, "config", "path to pipeline.yml (optional; defaults searched)") + fs.Var(&request.flags.campaignPath, "campaign", "campaign ID") + fs.Var(&request.flags.campaignFilePath, "campaign-file", "path to campaign.yml") + fs.Usage = func() { + fmt.Fprintln(out, "Usage: narratio config diff [--config ] [--campaign | --campaign-file ]") + fmt.Fprintln(out) + fmt.Fprintln(out, "Compares fully resolved profiles without a session, workspace, manifest, adapters, or external services.") + fmt.Fprintln(out) + fmt.Fprintln(out, "Flags:") + fs.PrintDefaults() + } + if err := fs.Parse(flagArgs); err != nil { + return configDiffRequest{}, fmt.Errorf("config diff: invalid flags: %w", err) + } + profiles = append(profiles, fs.Args()...) + if len(profiles) != 2 || strings.TrimSpace(profiles[0]) == "" || strings.TrimSpace(profiles[1]) == "" { + return configDiffRequest{}, fmt.Errorf("config diff: exactly two non-empty profile names are required") + } + if request.flags.campaignPath.set && request.flags.campaignFilePath.set { + return configDiffRequest{}, fmt.Errorf("config diff: --campaign and --campaign-file are mutually exclusive") + } + request.leftProfile = profiles[0] + request.rightProfile = profiles[1] + return request, nil +} + +// splitConfigDiffArguments accepts the documented positional-first syntax and +// also permits flags before or between profile names. Values belonging to the +// supported string flags stay with their flag so they are not mistaken for +// profile names. +func splitConfigDiffArguments(args []string) (profiles, flagArgs []string) { + for index := 0; index < len(args); index++ { + argument := args[index] + if !strings.HasPrefix(argument, "-") || argument == "-" { + profiles = append(profiles, argument) + continue + } + flagArgs = append(flagArgs, argument) + name, hasValue := strings.CutPrefix(argument, "--") + if !hasValue { + continue + } + name, _, hasInlineValue := strings.Cut(name, "=") + if hasInlineValue || !configDiffStringFlag(name) || index+1 >= len(args) { + continue + } + index++ + flagArgs = append(flagArgs, args[index]) + } + return profiles, flagArgs +} + +func configDiffStringFlag(name string) bool { + switch name { + case "config", "campaign", "campaign-file", "profile": + return true + default: + return false + } +} + +func resolveConfigDiff(request configDiffRequest) (*configDiffResolved, error) { + pipelinePath, err := resolvePipelineConfigPath(request.flags.pipelinePath.value) + if err != nil { + return nil, err + } + leftPipeline, rightPipeline, err := config.LoadPipelineProfilePair(pipelinePath, request.leftProfile, request.rightProfile) + if err != nil { + return nil, err + } + needsCampaign := pipelineHasArtifactFamilies(leftPipeline) || pipelineHasArtifactFamilies(rightPipeline) + hasCampaignSelection := request.flags.campaignPath.set || request.flags.campaignFilePath.set + if !needsCampaign && !hasCampaignSelection { + if err := validateInspectionPipeline(pipelinePath, leftPipeline); err != nil { + return nil, err + } + if err := validateInspectionPipeline(pipelinePath, rightPipeline); err != nil { + return nil, err + } + return &configDiffResolved{ + left: &inspectionConfig{PipelinePath: pipelinePath, Pipeline: leftPipeline}, + right: &inspectionConfig{PipelinePath: pipelinePath, Pipeline: rightPipeline}, + }, nil + } + + campaignPath, err := resolveSharedDiffCampaignPath(leftPipeline, rightPipeline, request.flags) + if err != nil { + if needsCampaign { + return nil, fmt.Errorf("pipeline.scriptorium.artifact_families requires one shared campaign: %w", err) + } + return nil, err + } + campaign, err := config.LoadCampaign(campaignPath) + if err != nil { + return nil, err + } + if request.flags.campaignPath.set && !request.flags.campaignFilePath.set { + if got := config.CampaignID(campaign); got != request.flags.campaignPath.value { + return nil, fmt.Errorf("campaign config %q invalid: campaign_id %q does not match selected campaign %q", campaignPath, got, request.flags.campaignPath.value) + } + } + leftLoaded, err := config.LoadPipelineCampaign(pipelinePath, leftPipeline, campaignPath, campaign) + if err != nil { + return nil, err + } + rightLoaded, err := config.LoadPipelineCampaignWithParty(pipelinePath, rightPipeline, campaignPath, campaign, leftLoaded.Party) + if err != nil { + return nil, err + } + if err := validateInspectionPipeline(leftLoaded.PipelinePath, leftLoaded.Pipeline); err != nil { + return nil, err + } + if err := validateInspectionPipeline(rightLoaded.PipelinePath, rightLoaded.Pipeline); err != nil { + return nil, err + } + if err := validateInspectionCampaign(campaignPath, campaign); err != nil { + return nil, err + } + return &configDiffResolved{ + left: &inspectionConfig{ + PipelinePath: leftLoaded.PipelinePath, + Pipeline: leftLoaded.Pipeline, + CampaignPath: leftLoaded.CampaignPath, + Campaign: campaign, + Party: leftLoaded.Party, + }, + right: &inspectionConfig{ + PipelinePath: rightLoaded.PipelinePath, + Pipeline: rightLoaded.Pipeline, + CampaignPath: rightLoaded.CampaignPath, + Campaign: campaign, + Party: rightLoaded.Party, + }, + }, nil +} + +func resolveSharedDiffCampaignPath(left, right *config.PipelineConfig, flags configDiffFlags) (string, error) { + leftPath, leftErr := resolveCampaignConfigPath(left, flags.campaignPath.value, flags.campaignFilePath.value) + if leftErr != nil { + return "", leftErr + } + rightPath, rightErr := resolveCampaignConfigPath(right, flags.campaignPath.value, flags.campaignFilePath.value) + if rightErr != nil { + return "", rightErr + } + if leftPath != rightPath { + return "", fmt.Errorf("profile selections resolve different campaign files; specify --campaign-file to compare one campaign") + } + return leftPath, nil +} + +func diffEffectivePipelineValues(left, right []config.EffectivePipelineValueRecord) ([]configDiffRecord, error) { + leftByPath, err := effectivePipelineValueMap("left", left) + if err != nil { + return nil, err + } + rightByPath, err := effectivePipelineValueMap("right", right) + if err != nil { + return nil, err + } + paths := make([]string, 0, len(leftByPath)+len(rightByPath)) + seen := make(map[string]struct{}, len(leftByPath)+len(rightByPath)) + for path := range leftByPath { + seen[path] = struct{}{} + paths = append(paths, path) + } + for path := range rightByPath { + if _, exists := seen[path]; !exists { + paths = append(paths, path) + } + } + sort.Strings(paths) + records := make([]configDiffRecord, 0) + for _, path := range paths { + leftValue, leftExists := leftByPath[path] + rightValue, rightExists := rightByPath[path] + switch { + case !leftExists: + records = append(records, configDiffRecord{Kind: "added", Path: path, Right: rightValue}) + case !rightExists: + records = append(records, configDiffRecord{Kind: "removed", Path: path, Left: leftValue}) + case leftValue != rightValue: + records = append(records, configDiffRecord{Kind: "changed", Path: path, Left: leftValue, Right: rightValue}) + } + } + return records, nil +} + +func effectivePipelineValueMap(side string, records []config.EffectivePipelineValueRecord) (map[string]string, error) { + values := make(map[string]string, len(records)) + for _, record := range records { + if _, exists := values[record.Path]; exists { + return nil, fmt.Errorf("internal error: %s effective configuration has duplicate path %q", side, record.Path) + } + values[record.Path] = record.Value + } + return values, nil +} diff --git a/internal/config/composition.go b/internal/config/composition.go index d133f55..3be31c9 100644 --- a/internal/config/composition.go +++ b/internal/config/composition.go @@ -397,6 +397,22 @@ func (document *compositionDocument) semanticRecords() ([]compositionValueRecord return records, nil } +// compactSemanticRecords returns the same logical atomic paths as +// semanticRecords, but represents values as compact JSON-compatible YAML +// values rather than the typed structural form used for digesting. It is the +// stable human-facing projection for semantic comparisons. +func (document *compositionDocument) compactSemanticRecords() ([]compositionValueRecord, error) { + if err := validateCompositionDocument(document, "document"); err != nil { + return nil, err + } + var records []compositionValueRecord + if err := appendCompactCompositionRecords(document.root, &records); err != nil { + return nil, err + } + sort.Slice(records, func(i, j int) bool { return records[i].Path < records[j].Path }) + return records, nil +} + func appendCompositionRecords(node *compositionNode, records *[]compositionValueRecord) error { if node.kind == yaml.MappingNode && len(node.fields) > 0 { for _, field := range node.fields { @@ -421,6 +437,59 @@ func appendCompositionRecords(node *compositionNode, records *[]compositionValue return nil } +func appendCompactCompositionRecords(node *compositionNode, records *[]compositionValueRecord) error { + if node.kind == yaml.MappingNode && len(node.fields) > 0 { + for _, field := range node.fields { + if err := appendCompactCompositionRecords(field.value, records); err != nil { + return err + } + } + return nil + } + value, err := compactCompositionValue(node) + if err != nil { + return err + } + encoded, err := json.Marshal(value) + if err != nil { + return fmt.Errorf("serialize configuration value at %s: %w", displayCompositionPath(node.path), err) + } + *records = append(*records, compositionValueRecord{ + Path: node.path, Kind: node.kind, Value: string(encoded), + Sources: append([]string(nil), node.sources...), + }) + return nil +} + +func compactCompositionValue(node *compositionNode) (any, error) { + switch node.kind { + case yaml.MappingNode: + values := make(map[string]any, len(node.fields)) + for _, field := range node.fields { + value, err := compactCompositionValue(field.value) + if err != nil { + return nil, err + } + values[field.key] = value + } + return values, nil + case yaml.SequenceNode: + values := make([]any, 0, len(node.items)) + for _, item := range node.items { + value, err := compactCompositionValue(item) + if err != nil { + return nil, err + } + values = append(values, value) + } + return values, nil + case yaml.ScalarNode: + return canonicalScalarValue(node) + default: + return nil, fmt.Errorf("configuration at %s has unsupported YAML kind %s", displayCompositionPath(node.path), yamlKindName(node.kind)) + } +} + type canonicalCompositionField struct { Key string `json:"key"` Value any `json:"value"` diff --git a/internal/config/effective_pipeline.go b/internal/config/effective_pipeline.go index bc79c68..4a7be01 100644 --- a/internal/config/effective_pipeline.go +++ b/internal/config/effective_pipeline.go @@ -44,6 +44,41 @@ type EffectivePipelineSourceRecord struct { Source string } +// EffectivePipelineValueRecord identifies one normalized, secret-free +// effective configuration value. Values use deterministic compact JSON +// representations so command output can be compared without raw YAML layout. +type EffectivePipelineValueRecord struct { + Path string + Value string +} + +// EffectivePipelineValues projects a fully resolved pipeline into sorted +// logical configuration values. Mappings are flattened, while sequences remain +// atomic values. The projection uses the same normalized effective mapping as +// config show and therefore excludes composition and family declarations. +func EffectivePipelineValues(cfg *PipelineConfig) ([]EffectivePipelineValueRecord, error) { + if cfg == nil { + return nil, fmt.Errorf("pipeline config is required") + } + data, err := MarshalEffectivePipeline(cfg) + if err != nil { + return nil, err + } + document, err := parseCompositionBytes("effective pipeline", data) + if err != nil { + return nil, err + } + records, err := document.compactSemanticRecords() + if err != nil { + return nil, err + } + values := make([]EffectivePipelineValueRecord, 0, len(records)) + for _, record := range records { + values = append(values, EffectivePipelineValueRecord{Path: record.Path, Value: record.Value}) + } + return values, nil +} + // EffectivePipelineSources projects pipeline ownership after defaults and // optional family expansion. It never includes effective values or raw secret // material, only logical paths and source identifiers. diff --git a/internal/config/load.go b/internal/config/load.go index f2eb805..66176be 100644 --- a/internal/config/load.go +++ b/internal/config/load.go @@ -27,10 +27,72 @@ type PipelineLoadOptions struct { // LoadPipelineWithOptions loads pipeline configuration with strict field // checking and optional named-profile selection. func LoadPipelineWithOptions(path string, opts PipelineLoadOptions) (*PipelineConfig, error) { - cfg, err := loadComposedPipeline(path, opts) + sources, err := loadPipelineCompositionSources(path) if err != nil { return nil, fmt.Errorf("load pipeline config: %w", err) } + if _, err := selectPipelineProfile(sources.envelope, opts); err != nil { + return nil, fmt.Errorf("load pipeline config: %w", err) + } + if err := sources.loadOverlays(); err != nil { + return nil, fmt.Errorf("load pipeline config: %w", err) + } + cfg, err := sources.resolve(opts) + if err != nil { + return nil, fmt.Errorf("load pipeline config: %w", err) + } + return finalizeLoadedPipeline(path, cfg) +} + +// LoadPipelineProfilePair resolves two explicit named profiles from one parsed +// pipeline root and its declared source set. Each result is independently +// decoded, defaulted, and finalized so later resolution can safely mutate one +// effective pipeline without affecting the other. +func LoadPipelineProfilePair(path, leftProfile, rightProfile string) (*PipelineConfig, *PipelineConfig, error) { + if _, err := normalizePipelineProfileName(leftProfile, "left profile selection"); err != nil { + return nil, nil, fmt.Errorf("load left pipeline profile: %w", err) + } + if _, err := normalizePipelineProfileName(rightProfile, "right profile selection"); err != nil { + return nil, nil, fmt.Errorf("load right pipeline profile: %w", err) + } + if leftProfile == rightProfile { + return nil, nil, fmt.Errorf("load pipeline profiles: left and right profile selections must differ") + } + sources, err := loadPipelineCompositionSources(path) + if err != nil { + return nil, nil, fmt.Errorf("load pipeline config: %w", err) + } + leftOptions := PipelineLoadOptions{Profile: &leftProfile} + rightOptions := PipelineLoadOptions{Profile: &rightProfile} + if _, err := selectPipelineProfile(sources.envelope, leftOptions); err != nil { + return nil, nil, fmt.Errorf("load left pipeline profile: %w", err) + } + if _, err := selectPipelineProfile(sources.envelope, rightOptions); err != nil { + return nil, nil, fmt.Errorf("load right pipeline profile: %w", err) + } + if err := sources.loadOverlays(); err != nil { + return nil, nil, fmt.Errorf("load pipeline config: %w", err) + } + left, err := sources.resolve(leftOptions) + if err != nil { + return nil, nil, fmt.Errorf("load left pipeline profile: %w", err) + } + right, err := sources.resolve(rightOptions) + if err != nil { + return nil, nil, fmt.Errorf("load right pipeline profile: %w", err) + } + left, err = finalizeLoadedPipeline(path, left) + if err != nil { + return nil, nil, err + } + right, err = finalizeLoadedPipeline(path, right) + if err != nil { + return nil, nil, err + } + return left, right, nil +} + +func finalizeLoadedPipeline(path string, cfg *PipelineConfig) (*PipelineConfig, error) { cfg.resolution.publishDeclared = cfg.Publish != nil applyPipelineDefaults(cfg) if err := resolveNotariusPaths(cfg, path); err != nil { @@ -201,6 +263,23 @@ func LoadPipelineCampaign(pipelinePath string, pipeline *PipelineConfig, campaig if err != nil { return LoadedPipelineCampaign{}, err } + return LoadPipelineCampaignWithParty(pipelinePath, pipeline, campaignPath, campaign, party) +} + +// LoadPipelineCampaignWithParty combines an already loaded pipeline and +// campaign with one already resolved campaign-owned party. It is useful when +// more than one independently resolved pipeline must be expanded against the +// exact same party document. +func LoadPipelineCampaignWithParty(pipelinePath string, pipeline *PipelineConfig, campaignPath string, campaign *CampaignConfig, party ResolvedParty) (LoadedPipelineCampaign, error) { + if pipeline == nil { + return LoadedPipelineCampaign{}, fmt.Errorf("pipeline config is required") + } + if campaign == nil { + return LoadedPipelineCampaign{}, fmt.Errorf("campaign config is required") + } + if party.Mode == "" { + return LoadedPipelineCampaign{}, fmt.Errorf("resolved campaign party is required") + } if party.Mode == PartyModeCanonical { if err := validateCanonicalPartySelection(campaign, nil); err != nil { return LoadedPipelineCampaign{}, err diff --git a/internal/config/pipeline_composition.go b/internal/config/pipeline_composition.go index a692e22..c3e381a 100644 --- a/internal/config/pipeline_composition.go +++ b/internal/config/pipeline_composition.go @@ -46,7 +46,20 @@ type pipelineProfileDeclaration struct { overlay string } -func loadComposedPipeline(path string, opts PipelineLoadOptions) (*PipelineConfig, error) { +// pipelineCompositionSources retains one validated root source set. Each +// selected profile is resolved from a cloned base document so callers can +// safely compare or otherwise resolve multiple profiles without rereading or +// mutating the source set. +type pipelineCompositionSources struct { + rootPath string + envelope pipelineCompositionEnvelope + imports []loadedPipelineImport + overlays []loadedPipelineProfileOverlay + overlaysLoaded bool + base *compositionDocument +} + +func loadPipelineCompositionSources(path string) (*pipelineCompositionSources, error) { rootPath, err := filepath.Abs(path) if err != nil { return nil, fmt.Errorf("resolve root pipeline path %q: %w", path, err) @@ -72,14 +85,6 @@ func loadComposedPipeline(path string, opts PipelineLoadOptions) (*PipelineConfi if err != nil { return nil, err } - selection, err := selectPipelineProfile(envelope, opts) - if err != nil { - return nil, err - } - overlays, err := loadPipelineProfileOverlays(rootPath, envelope.profiles, imports) - if err != nil { - return nil, err - } documents := make([]*compositionDocument, 0, len(imports)+1) documents = append(documents, baseRoot) for _, imported := range imports { @@ -89,8 +94,47 @@ func loadComposedPipeline(path string, opts PipelineLoadOptions) (*PipelineConfi if err != nil { return nil, err } + return &pipelineCompositionSources{ + rootPath: rootPath, + envelope: envelope, + imports: imports, + base: merged, + }, nil +} + +func (sources *pipelineCompositionSources) loadOverlays() error { + if sources == nil { + return fmt.Errorf("pipeline composition sources are required") + } + if sources.overlaysLoaded { + return nil + } + overlays, err := loadPipelineProfileOverlays(sources.rootPath, sources.envelope.profiles, sources.imports) + if err != nil { + return err + } + sources.overlays = overlays + sources.overlaysLoaded = true + return nil +} + +func (sources *pipelineCompositionSources) resolve(opts PipelineLoadOptions) (*PipelineConfig, error) { + if sources == nil || sources.base == nil { + return nil, fmt.Errorf("pipeline composition sources are required") + } + if !sources.overlaysLoaded { + return nil, fmt.Errorf("pipeline profile overlays have not been loaded") + } + selection, err := selectPipelineProfile(sources.envelope, opts) + if err != nil { + return nil, err + } + merged := &compositionDocument{ + root: cloneCompositionNode(sources.base.root), + sources: append([]string(nil), sources.base.sources...), + } if selection != nil { - overlay, ok := loadedProfileOverlay(overlays, selection.name) + overlay, ok := loadedProfileOverlay(sources.overlays, selection.name) if !ok { return nil, fmt.Errorf("selected profile %q overlay was not loaded", selection.name) } @@ -106,7 +150,7 @@ func loadComposedPipeline(path string, opts PipelineLoadOptions) (*PipelineConfi return nil, err } var cfg PipelineConfig - if err := decodeStrictYAMLFromReader("pipeline", rootPath, strings.NewReader(string(rendered)), &cfg); err != nil { + if err := decodeStrictYAMLFromReader("pipeline", sources.rootPath, strings.NewReader(string(rendered)), &cfg); err != nil { return nil, fmt.Errorf("assembled pipeline sources %s: %w", formatCompositionSources(merged.sources), err) } records, err := merged.semanticRecords() @@ -114,11 +158,11 @@ func loadComposedPipeline(path string, opts PipelineLoadOptions) (*PipelineConfi return nil, err } metadata := &pipelineResolutionMetadata{ - rootPath: rootPath, + rootPath: sources.rootPath, sources: append([]string(nil), merged.sources...), selectedProfile: selection, } - for _, imported := range imports { + for _, imported := range sources.imports { metadata.imports = append(metadata.imports, imported.path) } for _, record := range records { diff --git a/internal/config/pipeline_profiles_test.go b/internal/config/pipeline_profiles_test.go index e05994f..ce88e7a 100644 --- a/internal/config/pipeline_profiles_test.go +++ b/internal/config/pipeline_profiles_test.go @@ -318,6 +318,43 @@ whisperx: } } +func TestEffectivePipelineValuesIgnoreEquivalentSourceLayout(t *testing.T) { + dir := t.TempDir() + monolithicPath := writePipelineSource(t, dir, "monolithic.yml", `workspace: + root: /srv/narratio +whisperx: + transcribe_url: https://transcription.example.com/transcribe + language: en +`) + composedPath := writePipelineSource(t, dir, "pipeline.yml", `composition: + imports: [workspace.yml] +whisperx: + language: en + transcribe_url: https://transcription.example.com/transcribe +`) + writePipelineSource(t, dir, "workspace.yml", "workspace:\n root: /srv/narratio\n") + + monolithic, err := LoadPipeline(monolithicPath) + if err != nil { + t.Fatal(err) + } + composed, err := LoadPipeline(composedPath) + if err != nil { + t.Fatal(err) + } + monolithicValues, err := EffectivePipelineValues(monolithic) + if err != nil { + t.Fatal(err) + } + composedValues, err := EffectivePipelineValues(composed) + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(monolithicValues, composedValues) { + t.Fatalf("equivalent effective values differ:\nmonolithic=%#v\ncomposed=%#v", monolithicValues, composedValues) + } +} + func TestPipelineEffectiveDigestExcludesProfileIdentity(t *testing.T) { dir := t.TempDir() rootPath := writeProfilePipeline(t, dir, profileComposition("production", "production", "testing")) @@ -340,6 +377,53 @@ func TestPipelineEffectiveDigestExcludesProfileIdentity(t *testing.T) { } } +func TestLoadPipelineProfilePairResolvesIndependentEffectivePipelines(t *testing.T) { + dir := t.TempDir() + rootPath := writeProfilePipeline(t, dir, `composition: + imports: [base.yml] + default_profile: production + profiles: + production: + overlay: production.yml + testing: + overlay: testing.yml +whisperx: + transcribe_url: https://transcription.example.com/transcribe +`) + writePipelineSource(t, dir, "base.yml", "workspace:\n root: /srv/base\n") + writePipelineSource(t, dir, "production.yml", "workspace:\n root: /srv/production\nwhisperx:\n language: en\n") + writePipelineSource(t, dir, "testing.yml", "workspace:\n root: /srv/testing\nwhisperx:\n language: fr\n") + + production, testing, err := LoadPipelineProfilePair(rootPath, "production", "testing") + if err != nil { + t.Fatal(err) + } + if production.Workspace.Root != "/srv/production" || production.WhisperX.Language != "en" { + t.Fatalf("production pair result = %#v", production) + } + if testing.Workspace.Root != "/srv/testing" || testing.WhisperX.Language != "fr" { + t.Fatalf("testing pair result = %#v", testing) + } + production.Workspace.Root = "/mutated-left" + if testing.Workspace.Root != "/srv/testing" { + t.Fatalf("right profile shared mutable state with left: %q", testing.Workspace.Root) + } + testingFirst, productionSecond, err := LoadPipelineProfilePair(rootPath, "testing", "production") + if err != nil { + t.Fatal(err) + } + if testingFirst.Workspace.Root != "/srv/testing" || productionSecond.Workspace.Root != "/srv/production" { + t.Fatalf("reversed profile pair = testing=%q production=%q", testingFirst.Workspace.Root, productionSecond.Workspace.Root) + } + + if _, _, err := LoadPipelineProfilePair(rootPath, "production", "production"); err == nil || !strings.Contains(err.Error(), "must differ") { + t.Fatalf("equal profile pair error = %v, want selection rejection", err) + } + if _, _, err := LoadPipelineProfilePair(rootPath, "unknown", "testing"); err == nil || !strings.Contains(err.Error(), "not declared") { + t.Fatalf("unknown profile pair error = %v, want selection rejection", err) + } +} + func TestLoadWithSessionOptionsCarriesExplicitProfilePresence(t *testing.T) { dir := t.TempDir() rootPath := writeProfilePipeline(t, dir, profileComposition("production", "production", "testing"))