Fix derived publish destinations with explicit requiredness

docs/releases/v2.1.1.md

@@ -0,0 +1,33 @@
+# Narratio v2.1.1
+
+This release fixes configuration loading for publish rules that explicitly set
+requiredness while relying on a derived destination.
+
+## Summary
+
+Publish rules with a derivable source and omitted `dest` now load correctly
+whether `required` is omitted, explicitly true, or explicitly false. Previously,
+explicit requiredness could trigger ConfigKit's `resolve.unrecorded_change`
+diagnostic instead of accepting valid configuration.
+
+## Compatibility
+
+This is a backward-compatible bookkeeping correction. Publish schemas,
+destination derivation, requiredness defaults, and YAML syntax support are
+unchanged. Explicit destinations and explicit false values remain intact.
+No ConfigKit upgrade or publication/storage behavior change is involved.
+
+## Upgrade
+
+No special upgrade action or configuration migration is required. Operators
+need not add a derivable `dest`, remove explicit `required` values, or change
+flow-style YAML to block-style YAML. See the
+[configuration reference](../config.md#publish-configuration-summary).
+
+## Changes
+
+- Recorded derived publish destinations at ConfigKit's atomic sequence boundary
+  while retaining field-level resolution records.
+- Added real load/validation and safe effective-configuration regression coverage
+  for built-in and configured-artifact sources, explicit requiredness, and an
+  explicit destination that must remain unchanged.

docs/roadmap/fix-publish-output-destination-resolution.md

@@ -0,0 +1,171 @@
+# Bugfix Request: Preserve Publish Destination Derivation with Explicit Rules
+
+Status: Implemented; release target v2.1.1
+
+## Summary
+
+Narratio should successfully derive a publish destination when a valid
+`publish.outputs` rule omits `dest`, regardless of whether that rule explicitly
+sets `required`. Narratio v2.1.0 currently rejects this valid configuration in
+some cases with a ConfigKit `resolve.unrecorded_change` diagnostic.
+
+This is a configuration-resolution bookkeeping defect, not a YAML formatting
+restriction. The fix should preserve the existing public configuration
+contract: operators may omit a derivable destination and may independently set
+`required` to either `true` or `false`.
+
+## Observed Behavior
+
+A pipeline containing a rule equivalent to the following fails during native
+configuration loading:
+
+```yaml
+publish:
+  outputs: [{"source": "narratio.transcript.final", "required": true}]
+```
+
+Narratio reports:
+
+```text
+error [resolve.unrecorded_change] /publish/outputs: application resolution changed a value without a covering record
+```
+
+The same failure is possible with ordinary block-style YAML when `required` is
+explicit and `dest` is omitted. Flow-style YAML is valid input and is not the
+cause of the defect.
+
+The existing destination-derivation tests do not expose this case because
+their rules also omit `required`. Narratio then supplies the default value for
+`required` and records a change covering the complete `publish.outputs`
+sequence. That broader record happens to cover the destination mutation as
+well, masking the missing resolution record that occurs when `required` is
+already explicit.
+
+## Expected Contract
+
+The current configuration contract remains unchanged:
+
+- `publish.outputs[].source` is required;
+- `publish.outputs[].dest` may be omitted when Narratio can derive it from the
+  selected built-in or configured artifact source;
+- `publish.outputs[].required` defaults to `true`, but an operator may set it
+  explicitly to either `true` or `false`; and
+- equivalent valid YAML representations have equivalent configuration
+  semantics.
+
+For example, both rules below should load and validate, retain their explicit
+requiredness, and receive the canonical derived destination:
+
+```yaml
+publish:
+  outputs:
+    - source: narratio.transcript.final
+      required: true
+    - source: narratio.transcript.final_markdown
+      required: false
+```
+
+## Root Cause
+
+ConfigKit intentionally treats sequences as atomic when comparing values
+before and after application resolution. Changing a field within one
+`publish.outputs` entry is therefore observed as a change to the
+`/publish/outputs` sequence.
+
+Narratio currently derives each missing destination by recording a
+normalization at the individual `publish.outputs[index].dest` path. That child
+record does not cover the atomic parent-sequence change. A separate
+container-level record is currently created only when Narratio also defaults
+an omitted `required` value, so configurations with explicit `required` values
+lose that incidental coverage.
+
+## Recommended Fix
+
+Make Narratio's publish-output resolution bookkeeping conform to ConfigKit's
+atomic-sequence contract. Whenever resolution derives one or more destinations
+inside `publish.outputs`, the recorded resolution changes must cover the
+`/publish/outputs` container. Destination derivation must not depend on an
+unrelated `required` default being applied in the same list.
+
+The implementation may retain more specific provenance where useful, but it
+must provide valid container-level coverage for the sequence mutation. Do not
+change ConfigKit's sequence comparison semantics merely to accommodate this
+call site.
+
+## Scope
+
+- Correct destination derivation for publish rules that explicitly set
+  `required` and omit a derivable `dest`.
+- Preserve explicit `required: true` and `required: false` values.
+- Preserve canonical destination derivation for built-in transcript sources
+  and configured artifact sources.
+- Add focused regression coverage through Narratio's real configuration load
+  and validation boundary.
+- Prepare a backward-compatible patch release after the fix passes the normal
+  repository validation.
+
+## Non-goals
+
+- Changing the `publish.outputs` schema, defaults, source vocabulary, or
+  destination derivation rules.
+- Requiring operators or configuration generators to supply `dest` when it is
+  derivable.
+- Requiring operators to omit explicit `required: true` values.
+- Treating flow-style YAML or JSON-compatible YAML values as unsupported.
+- Changing ConfigKit's documented atomic treatment of sequences.
+- Adding storage, publication, or network integration behavior.
+
+## Regression Coverage
+
+Add one focused package-level behavioral test that loads and validates a
+pipeline with omitted destinations and explicit requiredness. A representative
+list should include both `required: true` and `required: false`, then assert
+that:
+
+- loading and validation succeed;
+- each destination is derived from its source; and
+- each explicit `required` value is preserved.
+
+Using the compact flow-style representation that exposed the integration
+failure is reasonable, while the existing block-style cases continue to cover
+omitted `required`. The test should exercise the public configuration behavior,
+not ConfigKit call choreography or Narratio's private recording mechanism.
+
+No network service, object store, credential, or test double is required.
+
+## Target End State
+
+All valid publish rules with derivable sources behave consistently across
+these cases:
+
+| `dest` | `required` | Result |
+| --- | --- | --- |
+| omitted | omitted | destination derived; required defaults to `true` |
+| omitted | `true` | destination derived; explicit `true` retained |
+| omitted | `false` | destination derived; explicit `false` retained |
+| explicit | either value | explicit destination and requiredness retained |
+
+`narratio config validate`, `narratio config show`, and ordinary pipeline
+loading accept the corrected cases without an unrecorded-change diagnostic.
+No operator configuration migration or workaround is required.
+
+## Release
+
+This is a backward-compatible correction to documented configuration behavior
+and should be released as Narratio `v2.1.1`. The release note should identify
+the corrected explicit-required/derived-destination interaction and state that
+no special upgrade action is required.
+
+## Acceptance Criteria
+
+- The focused regression test fails against v2.1.0 behavior and passes after
+  the fix.
+- Publish rules with explicit `required: true` or `required: false` and omitted
+  derivable destinations load and validate successfully.
+- Derived destinations and explicit requiredness appear correctly in the safe
+  effective configuration.
+- Existing destination derivation, publish validation, imports, profiles, and
+  configuration inspection tests remain green.
+- The complete repository validation and release-candidate checks pass.
+- The correction is published as the immutable `v2.1.1` tag with the required
+  release note.

internal/config/pipeline_configkit.go

@@ -209,11 +209,17 @@ func resolvePipelineConfig(cfg *PipelineConfig, resolution *configkit.Resolution
 		resolution.Record(configkit.Keys("notarius"), configkit.ChangeNormalization, "canonical Notarius configuration")
 	}
 	if cfg.Publish != nil {
-		for _, derived := range deriveMissingPublishDestinations(cfg.Publish, cfg.Scriptorium, cfg.Notarius) {
+		destinations := deriveMissingPublishDestinations(cfg.Publish, cfg.Scriptorium, cfg.Notarius)
+		for _, derived := range destinations {
 			output := cfg.Publish.Outputs[derived.index]
 			configkit.Normalize(resolution, configkit.Keys("publish", "outputs").Index(derived.index).Key("dest"), &output.Dest, derived.dest, "derived from publish source")
 			cfg.Publish.Outputs[derived.index] = output
 		}
+		if len(destinations) > 0 {
+			// ConfigKit compares sequences atomically; child records alone do not
+			// cover the list mutation when requiredness was already explicit.
+			resolution.Record(configkit.Keys("publish", "outputs"), configkit.ChangeNormalization, "derived publish destinations")
+		}
 	}
 	return nil
 }

internal/config/storage_publish_test.go

@@ -3,6 +3,8 @@ package config
 import (
 	"strings"
 	"testing"
+
+	"gopkg.in/yaml.v3"
 )
 
 func TestStorageS3DefaultsAndValidation(t *testing.T) {
@@ -549,6 +551,52 @@ publish:
 	}
 }
 
+func TestPublishDerivedDestinationsPreserveExplicitRequiredness(t *testing.T) {
+	pipelinePath, sessionPath := writeConfigFiles(t, testPipelineBaseYAML+`
+scriptorium:
+  artifacts:
+    session_recap:
+      enabled: true
+      prompt_id: dnd.session_recap
+      output_path: artifacts/session_recap.md
+publish:
+  outputs:
+    - {source: narratio.transcript.final, required: true}
+    - {source: narratio.transcript.final_markdown, required: false}
+    - {source: narratio.artifact.session_recap, required: false}
+    - {source: narratio.transcript.final_trimmed, dest: custom/trimmed.json, required: true}
+`, testSessionBaseYAML)
+	cfg, err := Load(pipelinePath, sessionPath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := Validate(cfg); err != nil {
+		t.Fatal(err)
+	}
+	effective, err := MarshalEffectivePipeline(cfg.Pipeline)
+	if err != nil {
+		t.Fatal(err)
+	}
+	var projected struct {
+		Publish PublishConfig `yaml:"publish"`
+	}
+	if err := yaml.Unmarshal(effective, &projected); err != nil {
+		t.Fatal(err)
+	}
+	wantDest := []string{"transcripts/final.json", "transcripts/final.md", "artifacts/session_recap.md", "custom/trimmed.json"}
+	wantRequired := []bool{true, false, false, true}
+	for label, publish := range map[string]*PublishConfig{"loaded": cfg.Pipeline.Publish, "effective": &projected.Publish} {
+		if len(publish.Outputs) != len(wantDest) {
+			t.Fatalf("%s output count = %d, want %d", label, len(publish.Outputs), len(wantDest))
+		}
+		for i, output := range publish.Outputs {
+			if output.Dest != wantDest[i] || output.Required == nil || *output.Required != wantRequired[i] {
+				t.Errorf("%s output %d = %#v, want destination %q and required %t", label, i, output, wantDest[i], wantRequired[i])
+			}
+		}
+	}
+}
+
 func TestScriptoriumOutputPathsIgnoreInvalidKeys(t *testing.T) {
 	scriptorium := &ScriptoriumConfig{Artifacts: map[string]ScriptoriumArtifactConfig{
 		" artifact": {OutputPath: "artifacts/invalid.md"},