From 8b0ce4d134bfedca30a6b017b32ff0504a2a34d2 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Thu, 18 Jun 2026 15:40:24 +0000 Subject: [PATCH] Add takeover backend regressions and close roadmap --- docs/roadmap/implementation.md | 34 ++--- docs/roadmap/takeover.md | 229 --------------------------------- internal/app/run_test.go | 118 +++++++++++++++-- 3 files changed, 126 insertions(+), 255 deletions(-) delete mode 100644 docs/roadmap/takeover.md diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index d9a98cd..f5815a1 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -1,14 +1,14 @@ # Managed Destination Takeover Implementation Roadmap -This is the active staged implementation plan for -`docs/roadmap/takeover.md`. The feature roadmap defines the target policy and -end-state semantics; this document defines the implementation sequence for an -LLM coding agent to follow stage by stage. +This is the completed staged implementation plan for the takeover feature. The +detailed feature roadmap was removed after implementation; current behavior is +documented outside `docs/roadmap/`. This document records the implementation +sequence used by LLM coding agents stage by stage. -Future behavior must remain under `docs/roadmap/` until each stage is -implemented. Preparatory internal stages should not update user-facing current -docs. Current-behavior docs should be updated when takeover behavior is wired -for operator-facing use. +Future behavior must remain under `docs/roadmap/` until implemented. +Preparatory internal stages should not update user-facing current docs. +Current-behavior docs should be updated when behavior is wired for +operator-facing use. ## Current Baseline @@ -55,7 +55,8 @@ changing publish behavior. Source roadmap reference: -- `docs/roadmap/takeover.md`: Configuration, Policy Semantics, Safety Rules. +- Completed takeover feature roadmap: Configuration, Policy Semantics, Safety + Rules. Implementation scope: @@ -96,7 +97,7 @@ takeover eligibility without parsing human-readable reason strings. Source roadmap reference: -- `docs/roadmap/takeover.md`: Policy Semantics, Relationship To Existing +- Completed takeover feature roadmap: Policy Semantics, Relationship To Existing Policies. Implementation scope: @@ -140,7 +141,7 @@ Goal: implement `takeover.mode` for single-owner destination state. Source roadmap reference: -- `docs/roadmap/takeover.md`: Policy Semantics, Publish Planning, Destination +- Completed takeover feature roadmap: Policy Semantics, Publish Planning, Destination State Results, Safety Rules. Implementation scope: @@ -207,7 +208,7 @@ conflicts. Source roadmap reference: -- `docs/roadmap/takeover.md`: Policy Semantics, Publish Planning, Destination +- Completed takeover feature roadmap: Policy Semantics, Publish Planning, Destination State Results, Safety Rules. Implementation scope: @@ -262,7 +263,7 @@ feature. Source roadmap reference: -- `docs/roadmap/takeover.md`: Documentation Impact, Publish Planning, +- Completed takeover feature roadmap: Documentation Impact, Publish Planning, Relationship To Existing Policies. Implementation scope: @@ -313,7 +314,7 @@ after implementation. Source roadmap reference: -- `docs/roadmap/takeover.md`: Goals, Non-Goals, Safety Rules. +- Completed takeover feature roadmap: Goals, Non-Goals, Safety Rules. Implementation scope: @@ -328,9 +329,8 @@ Implementation scope: - Add tests for archive-style `preserve_relative` destinations using `takeover.mode: same_source` where strict source identity is desired. - Run final consistency searches. -- Once behavior and docs are complete, either remove `docs/roadmap/takeover.md` - or reduce it to future-only material according to the documentation policy. - If no future takeover work remains, remove the roadmap file. +- Remove the completed detailed takeover roadmap when no future takeover work + remains. Tests: diff --git a/docs/roadmap/takeover.md b/docs/roadmap/takeover.md deleted file mode 100644 index 02f6fcc..0000000 --- a/docs/roadmap/takeover.md +++ /dev/null @@ -1,229 +0,0 @@ -# Managed Destination Takeover Roadmap - -This roadmap defines planned replacement policy for distributor-managed -destination content when the existing destination state does not describe the -same source bundle as the current publication. - -Current behavior is intentionally conservative. A destination may be replaced -automatically when its existing state describes the same source id and an older -source creation time. If the existing state belongs to a different source id, -pipeline id, destination id, or shared-root owner, `distributor` reports a -conflict unless an explicit forced replacement workflow is selected where -supported. - -That strict default was useful for the initial implementation, but it is too -rigid for producers that intentionally publish newer generated content to the -same destination path from different producer tasks or bundle ids. `distributor` -is primarily a distribution tool: by default, a valid producer request should -publish the requested bundle unless doing so would claim unmanaged content or -cross an ownership boundary that is likely to surprise the operator. - -## Goals - -- Add a destination-level policy that defines when normal, non-force publication - may replace existing distributor-managed content whose source or owner - identity differs from the current source. -- Default the policy to `same_pipeline`, so a pipeline may distribute the latest - valid bundle to its configured destination paths without requiring every - producer task to reuse one source manifest id. -- Preserve strong safety boundaries for unmanaged content, invalid destination - state, and cross-pipeline ownership by default. -- Keep reconciliation policy focused on how managed output sets are updated - after replacement is allowed. -- Keep transfer policy focused on same-source comparison outcomes and explicit - forced replacement behavior. -- Preserve clear dry-run and JSON output so takeover behavior is visible before - files are changed. - -## Non-Goals - -- Do not allow automatic adoption of unmanaged destination files. -- Do not allow invalid `.distributor.json` state to be overwritten without the - existing explicit force workflow. -- Do not make producers select destination ownership or takeover policy through - source manifests or HTTP upload requests. -- Do not replace `state.mode`, `reconciliation.mode`, `path_mapping.mode`, or - `transfer` policy. -- Do not introduce broad synchronization behavior outside configured - destination bundle paths. - -## Configuration - -Add destination-level takeover policy: - -```yaml -destinations: - - id: latest - backend: local - path: /srv/reports/weather/latest/tomorrow - path_mapping: - mode: fixed - takeover: - mode: same_pipeline -``` - -Accepted `takeover.mode` values: - -- `same_pipeline`: default. Normal publication may replace valid - distributor-managed state or managed output paths owned by the same pipeline, - even when the previous source id or destination id differs. -- `same_source`: normal publication may replace only when the destination state - has the same source manifest id as the current source. This is closest to the - current strict managed replacement behavior. -- `any_managed`: normal publication may replace any valid distributor-managed - state or managed output path in the selected destination bundle path, - regardless of pipeline id, destination id, or source id. -- `never`: normal publication never performs takeover replacement for identity - or source conflicts. Same-source older/newer behavior remains governed by - `transfer`, and explicit forced replacement remains governed by `--force` and - `transfer.on_conflict`. - -The field is destination-local because different destinations from the same -pipeline can have different ownership expectations. Archive destinations often -want stricter behavior, while latest-style fixed destinations often want -`same_pipeline` or, after explicit operator review, `any_managed`. - -## Policy Semantics - -Takeover policy applies only after destination state has been parsed and -validated as distributor-managed state. - -For single-owner state: - -- `same_pipeline` permits replacement when the existing state `pipeline_id` - matches the current pipeline id. The previous `destination_id` and source id - may differ. -- `same_source` permits replacement only when the existing source manifest id - matches the current source manifest id. -- `any_managed` permits replacement of any valid single-owner destination state. -- `never` does not permit takeover replacement. - -For shared-root state: - -- Takeover is evaluated per managed output path that collides with the current - plan and per current owner record when one already exists. -- `same_pipeline` permits the current destination owner to take over output - paths owned by another destination under the same pipeline. -- `same_source` permits takeover only when the colliding output owner records - the same source manifest id as the current source. -- `any_managed` permits takeover of colliding managed output paths owned by any - owner in the shared root. -- `never` preserves the current owner-scoped conflict behavior. - -Takeover never applies to: - -- destination content with no valid `.distributor.json`; -- invalid destination state; -- planned paths that exist in storage but are not recorded as managed; -- source digest mismatches or invalid source manifests; -- destination newer/same-created digest comparisons for the same source id, - except where existing transfer policy already permits replacement. - -## Relationship To Existing Policies - -`takeover.mode` answers this question: - -> May this destination normally replace valid managed content whose owner or -> source identity differs from the current publication? - -`reconciliation.mode` continues to answer: - -> Once replacement is allowed, should omitted managed outputs be removed -> (`replace`) or retained (`merge`)? - -`transfer` continues to answer: - -> What should happen for same-source comparisons, destination-newer state, and -> explicit forced conflict replacement? - -`--force` remains an operator workflow for exceptional cases. Takeover policy is -not a replacement for forced replacement of unmanaged content or invalid state. - -## Publish Planning - -Add a new planned action for successful takeover replacement, tentatively named -`replace_takeover`. - -`replace_takeover` should execute through the same bounded managed replacement -machinery as other managed replacements, with one additional source-identity -safety rule: when takeover crosses source id or owner identity, omitted outputs -from the previous source must not be retained merely because the destination is -configured with `reconciliation.mode: merge`. Retaining omitted outputs is valid -only when those outputs still belong to the same source identity being -published. This keeps destination state from claiming that outputs derived from -an old source belong to the new source manifest. - -- single-owner `replace` deletes only managed outputs recorded in existing - state plus `.distributor.json`, then writes the new outputs and state; -- single-owner `merge` retains omitted managed outputs only for same-source - updates; cross-source takeover behaves as a managed replacement for the - affected owner; -- shared-root `replace` deletes only output records taken over by the current - owner or omitted from the current owner according to the shared-root plan; -- shared-root `merge` preserves unrelated non-conflicting managed outputs and - updates ownership for paths explicitly taken over, but does not retain omitted - outputs from a different source under the taking-over owner. - -Text output, JSON output, dry-run output, and summary counters should identify -takeover replacements separately from `replace_older` and `force_replace`. - -## Destination State Results - -After a successful takeover, destination state must describe the current -pipeline, destination, source manifest, outputs, links, reconciliation policy, -and state mode. - -For single-owner state, the destination bundle path has one current owner after -publication: the publishing pipeline and destination. - -For shared-root state, only the affected owner records and output records are -changed. Unrelated owners and non-conflicting outputs remain recorded. - -No source manifest schema change is required. No destination state schema change -is expected unless implementation discovers that takeover metadata must be -persisted for recovery or audit. The preferred initial design is to make -takeover a planning decision, not a new persisted state concept. - -## Documentation Impact - -When this feature is implemented, current-behavior docs should explain the new -policy where operators and maintainers already look for publication safety, -state comparison, and configuration behavior: - -- `docs/config.md`: document `takeover.mode`, default `same_pipeline`, accepted - values, examples for archive and latest destinations, and interaction with - `transfer`, `reconciliation`, and `--force`. -- `docs/operations.md`: explain takeover dry-runs, normal replacement safety, - shared-root behavior, and recovery guidance. -- `docs/troubleshooting.md`: update conflict guidance for - `destination source id differs from source`, pipeline/destination mismatch, - and shared-root ownership conflicts. -- `docs/integrations/destination-state.md`: describe how takeover affects - comparison and state rewriting without changing the source manifest contract. -- `docs/internal/publish.md`, `docs/internal/state.md`, and - `docs/internal/config.md`: document package responsibilities and invariants. -- `docs/policy/architecture.md`: clarify that the default posture is to - distribute valid producer bundles through configured destinations while - protecting unmanaged content and cross-pipeline ownership by default. -- `docs/policy/development.md`: add `takeover.mode` to the list of config - changes that must be validated and documented when publish safety changes. - -## Safety Rules - -- Takeover must be dry-runnable. -- Takeover must operate only inside the resolved destination bundle path. -- Takeover must never delete parent paths, sibling paths, or unmanaged files. -- Takeover must never claim storage content that is not recorded in valid - destination state. -- `any_managed` should be documented as an intentional cross-pipeline ownership - policy for tightly controlled destinations, not as the recommended default for - archives. -- Archive-style destinations should generally use `same_source` if each bundle - id represents a distinct immutable artifact. - -## Implementation Reference - -The staged implementation plan for this feature lives in -`docs/roadmap/implementation.md`. This document remains the feature contract and -policy reference; implementation sequencing, package-by-package work, and test -commands belong in the implementation roadmap. diff --git a/internal/app/run_test.go b/internal/app/run_test.go index aa1f58e..7d5f010 100644 --- a/internal/app/run_test.go +++ b/internal/app/run_test.go @@ -684,9 +684,13 @@ func TestRunFixedPathReplacesOlderManagedState(t *testing.T) { }, }) - if err := Run(context.Background(), RunOptions{ConfigPath: configPath}); err != nil { + var stdout bytes.Buffer + if err := Run(context.Background(), RunOptions{ConfigPath: configPath, Stdout: &stdout}); err != nil { t.Fatalf("second Run() error = %v", err) } + if !strings.Contains(stdout.String(), "action=replace_takeover takeover_mode=same_pipeline") { + t.Fatalf("stdout = %q, want same-pipeline takeover", stdout.String()) + } testutil.AssertFile(t, filepath.Join(destinationRoot, "report.md"), "# Report\nNew.\n") destinationState := readStateFile(t, filepath.Join(destinationRoot, storage.StateFileName)) if destinationState.Source.Manifest.ID != "reports.new" { @@ -694,6 +698,67 @@ func TestRunFixedPathReplacesOlderManagedState(t *testing.T) { } } +func TestRunPreserveRelativeSameSourceTakeoverAllowsOwnerMismatch(t *testing.T) { + sourceRoot := t.TempDir() + destinationRoot := t.TempDir() + sourceManifest := writeSourceBundle(t, sourceRoot, "daily/report", testBundleOptions{ + ID: "reports.same", + Files: []testFile{{Path: "report.md", Data: "# Report\nNew.\n"}}, + }) + testutil.WriteDestinationState(t, destinationRoot, "daily/report", sourceManifest, testutil.DestinationStateOptions{ + PipelineID: "other", + }) + if err := os.WriteFile(filepath.Join(destinationRoot, "daily", "report", "report.md"), []byte("# Report\nOld.\n"), 0o600); err != nil { + t.Fatalf("write old report: %v", err) + } + + var stdout bytes.Buffer + err := Run(context.Background(), RunOptions{ + ConfigPath: writeSameSourcePreserveRelativeConfig(t, sourceRoot, destinationRoot), + Stdout: &stdout, + }) + if err != nil { + t.Fatalf("Run() error = %v", err) + } + if !strings.Contains(stdout.String(), "action=replace_takeover takeover_mode=same_source") { + t.Fatalf("stdout = %q, want same-source takeover", stdout.String()) + } + testutil.AssertFile(t, filepath.Join(destinationRoot, "daily", "report", "report.md"), "# Report\nNew.\n") + destinationState := readStateFile(t, filepath.Join(destinationRoot, "daily", "report", storage.StateFileName)) + if destinationState.PipelineID != "reports" || destinationState.Source.Manifest.ID != "reports.same" { + t.Fatalf("state owner/source = %s/%s source=%s, want reports/archive reports.same", destinationState.PipelineID, destinationState.DestinationID, destinationState.Source.Manifest.ID) + } +} + +func TestRunPreserveRelativeSameSourceRefusesDifferentSource(t *testing.T) { + sourceRoot := t.TempDir() + destinationRoot := t.TempDir() + sourceManifest := writeSourceBundle(t, sourceRoot, "daily/report", testBundleOptions{ + ID: "reports.same", + Files: []testFile{{Path: "report.md", Data: "# Report\nNew.\n"}}, + }) + destinationManifest := sourceManifest + destinationManifest.ID = "reports.other" + testutil.WriteDestinationState(t, destinationRoot, "daily/report", destinationManifest, testutil.DestinationStateOptions{ + PipelineID: "other", + }) + if err := os.WriteFile(filepath.Join(destinationRoot, "daily", "report", "report.md"), []byte("# Report\nOld.\n"), 0o600); err != nil { + t.Fatalf("write old report: %v", err) + } + + err := Run(context.Background(), RunOptions{ + ConfigPath: writeSameSourcePreserveRelativeConfig(t, sourceRoot, destinationRoot), + }) + if err == nil || !strings.Contains(err.Error(), "fail_conflict") { + t.Fatalf("Run() error = %v, want fail_conflict", err) + } + testutil.AssertFile(t, filepath.Join(destinationRoot, "daily", "report", "report.md"), "# Report\nOld.\n") + destinationState := readStateFile(t, filepath.Join(destinationRoot, "daily", "report", storage.StateFileName)) + if destinationState.PipelineID != "other" || destinationState.Source.Manifest.ID != "reports.other" { + t.Fatalf("state owner/source = %s/%s source=%s, want unchanged other/archive reports.other", destinationState.PipelineID, destinationState.DestinationID, destinationState.Source.Manifest.ID) + } +} + func TestRunFixedPathSkipsWhenDestinationStateIsNewer(t *testing.T) { sourceRoot := t.TempDir() destinationRoot := t.TempDir() @@ -777,14 +842,6 @@ func TestRunFixedPathRemoteBackendsUseBackendRoots(t *testing.T) { {Path: "summary.txt", Data: "Old summary\n"}, }, }) - writeSourceBundle(t, localSourceRoot, "new", testBundleOptions{ - ID: "reports.new", - Created: testutil.DefaultCreated.Add(time.Hour), - Files: []testFile{ - {Path: "report.md", Data: "# Report\nNew.\n"}, - {Path: "summary.txt", Data: "New summary\n"}, - }, - }) s3Destination := fake.New() sshDestination := fake.New() cfg := config.Config{Pipelines: []config.Pipeline{{ @@ -816,6 +873,30 @@ func TestRunFixedPathRemoteBackendsUseBackendRoots(t *testing.T) { if err := runConfigWithBackendFactory(context.Background(), cfg, RunOptions{}, provider); err != nil { t.Fatalf("Run() error = %v", err) } + testutil.AssertFakeFile(t, s3Destination, "report.md", "# Report\nOld.\n") + testutil.AssertFakeFile(t, sshDestination, "summary.txt", "Old summary\n") + + writeSourceBundle(t, localSourceRoot, "new", testBundleOptions{ + ID: "reports.new", + Created: testutil.DefaultCreated.Add(time.Hour), + Files: []testFile{ + {Path: "report.md", Data: "# Report\nNew.\n"}, + {Path: "summary.txt", Data: "New summary\n"}, + }, + }) + var stdout bytes.Buffer + if err := runConfigWithBackendFactory(context.Background(), cfg, RunOptions{Stdout: &stdout}, provider); err != nil { + t.Fatalf("second Run() error = %v", err) + } + for _, want := range []string{ + "destination=object-latest backend=s3 path_mapping=fixed target=. action=replace_takeover takeover_mode=same_pipeline", + "destination=ssh-latest backend=ssh path_mapping=fixed target=. action=replace_takeover takeover_mode=same_pipeline", + "replace_takeover=2", + } { + if !strings.Contains(stdout.String(), want) { + t.Fatalf("stdout = %q, want substring %q", stdout.String(), want) + } + } testutil.AssertFakeFile(t, s3Destination, "report.md", "# Report\nNew.\n") testutil.AssertFakeFile(t, s3Destination, "summary.txt", "New summary\n") testutil.AssertFakeMissing(t, s3Destination, "new/report.md") @@ -1892,6 +1973,25 @@ pipelines: `) } +func writeSameSourcePreserveRelativeConfig(t *testing.T, sourceRoot, destinationRoot string) string { + t.Helper() + return writeConfigFile(t, ` +pipelines: + - id: reports + source: + backend: local + path: `+sourceRoot+` + destinations: + - id: archive + backend: local + path: `+destinationRoot+` + path_mapping: + mode: preserve_relative + takeover: + mode: same_source +`) +} + func writeFanoutConfig(t *testing.T, sourceRoot, firstDestination, secondDestination string) string { t.Helper() return testutil.WriteFanoutLocalConfig(t, sourceRoot, firstDestination, secondDestination)