diff --git a/docs/integrations/comparison-bundle.md b/docs/integrations/comparison-bundle.md index a874f22..ea210c5 100644 --- a/docs/integrations/comparison-bundle.md +++ b/docs/integrations/comparison-bundle.md @@ -88,6 +88,10 @@ case-variant, or duplicate fields; multiple JSON values; extra entries; symlinks; and future or otherwise unsupported versions. Treat a bundle that fails recognition as an ordinary directory, not as a compatible bundle. +When replacing a recognized bundle, cancellation observed before the new +bundle is installed preserves the prior bundle rather than committing the +replacement. + The manifest contains safe operational provenance, but `data-package.yml` and the generated Markdown can contain sensitive weather or location context. Do not assume these artifacts are safe for public distribution. Handle retention, diff --git a/docs/internal/comparison-publication.md b/docs/internal/comparison-publication.md index daba804..76179ed 100644 --- a/docs/internal/comparison-publication.md +++ b/docs/internal/comparison-publication.md @@ -18,8 +18,9 @@ symlinks and non-directories, accepts a missing or empty directory, and permits replacement only for a recognized current bundle. Publication rechecks that authorization immediately before it writes a private sibling staging directory. For replacement, it moves the prior bundle to a private sibling backup, -reauthorizes that moved entry, and restores it if installing the new bundle -fails. +reauthorizes that moved entry, checks for cancellation, and restores it if +cancellation or installing the new bundle prevents replacement. If guarded +restoration fails, the error retains the prior bundle's recovery path. Planning also validates the final component and the bounded fixed names used for private staging and backup siblings. A destination that cannot form those diff --git a/docs/operations.md b/docs/operations.md index 4491ae2..e647303 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -123,9 +123,12 @@ rechecked immediately before an atomic publish. A missing or empty directory is usable. A nonempty directory can be replaced only when `--replace` is given and it is recognized as a current Weatherreporter comparison bundle; ordinary directories, symlinks, and unsafe destinations are rejected. Cancellation and -all failures before publication preserve an existing bundle. Profile failures -are different: the command publishes a complete partial bundle, with failed -profiles represented in the manifest and no Markdown file for those profiles. +all failures before publication preserve an existing bundle, including a +cancellation observed while a replacement is being prepared. If guarded +restoration cannot complete, the error names the retained sibling bundle for +manual recovery. Profile failures are different: the command publishes a +complete partial bundle, with failed profiles represented in the manifest and +no Markdown file for those profiles. Comparison preflight also checks that private publication siblings can be formed. An infeasible destination name is rejected before a missing parent directory is created. diff --git a/internal/comparison/publish.go b/internal/comparison/publish.go index 9a2d1f9..eab5eb8 100644 --- a/internal/comparison/publish.go +++ b/internal/comparison/publish.go @@ -550,6 +550,9 @@ func publish(ctx context.Context, plan DestinationPlan, bundle LogicalBundle, op if err := authorizeMovedDestination(currentPlan, backupDirectory); err != nil { return PublicationResult{}, restoreMovedDestination(operations, backupDirectory, currentPlan.Target, err) } + if err := ctx.Err(); err != nil { + return PublicationResult{}, restoreMovedDestination(operations, backupDirectory, currentPlan.Target, err) + } if err := operations.rename(temporaryDirectory, currentPlan.Target); err != nil { return PublicationResult{}, restoreMovedDestination(operations, backupDirectory, currentPlan.Target, fmt.Errorf("replace comparison destination %q: %w", currentPlan.Target, err)) } diff --git a/internal/comparison/publish_test.go b/internal/comparison/publish_test.go index f565404..2491971 100644 --- a/internal/comparison/publish_test.go +++ b/internal/comparison/publish_test.go @@ -685,6 +685,154 @@ func TestPublishCancellationBeforeCommitLeavesNoDestination(t *testing.T) { } } +func TestPublishRestoresPriorDestinationWhenCanceledAfterBackup(t *testing.T) { + for _, test := range []struct { + name string + prepare func(t *testing.T, workingDirectory, target string) + }{ + { + name: "empty directory", + prepare: func(t *testing.T, _, target string) { + t.Helper() + if err := os.Mkdir(target, 0o700); err != nil { + t.Fatal(err) + } + }, + }, + { + name: "recognized bundle", + prepare: func(t *testing.T, workingDirectory, target string) { + t.Helper() + plan, err := PlanDestination(workingDirectory, target, false) + if err != nil { + t.Fatal(err) + } + if _, err := Publish(context.Background(), plan, testBundle()); err != nil { + t.Fatal(err) + } + }, + }, + } { + t.Run(test.name, func(t *testing.T) { + workingDirectory := t.TempDir() + target := filepath.Join(workingDirectory, "comparison-daily") + test.prepare(t, workingDirectory, target) + before := directorySnapshot(t, target) + + plan, err := PlanDestination(workingDirectory, target, true) + if err != nil { + t.Fatal(err) + } + ctx, cancel := context.WithCancel(context.Background()) + next := testBundle() + next.Reports[0].Markdown = []byte("# Replacement\n") + publication, err := publish(ctx, plan, next, publishOperations{rename: func(oldPath, newPath string) error { + if err := os.Rename(oldPath, newPath); err != nil { + return err + } + if oldPath == target { + cancel() + } + return nil + }}) + if publication.Committed || !errors.Is(err, context.Canceled) { + t.Fatalf("publish() result/error = %#v/%v, want canceled uncommitted publication", publication, err) + } + if after := directorySnapshot(t, target); !equalSnapshots(before, after) { + t.Fatalf("restored destination = %#v, want %#v", after, before) + } + assertOnlyDestinationEntry(t, workingDirectory, filepath.Base(target)) + }) + } +} + +func TestPublishRetainsBackupWhenCancellationRestorationFails(t *testing.T) { + workingDirectory := t.TempDir() + target := filepath.Join(workingDirectory, "comparison-daily") + initialPlan, err := PlanDestination(workingDirectory, target, false) + if err != nil { + t.Fatal(err) + } + if _, err := Publish(context.Background(), initialPlan, testBundle()); err != nil { + t.Fatal(err) + } + plan, err := PlanDestination(workingDirectory, target, true) + if err != nil { + t.Fatal(err) + } + + ctx, cancel := context.WithCancel(context.Background()) + restoreCause := errors.New("restore failed") + var backupPath string + calls := 0 + publication, err := publish(ctx, plan, testBundle(), publishOperations{rename: func(oldPath, newPath string) error { + calls++ + if calls == 2 { + return restoreCause + } + if err := os.Rename(oldPath, newPath); err != nil { + return err + } + if calls == 1 { + backupPath = newPath + cancel() + } + return nil + }}) + if publication.Committed || !errors.Is(err, context.Canceled) || !errors.Is(err, restoreCause) { + t.Fatalf("publish() result/error = %#v/%v, want cancellation and restoration failure", publication, err) + } + if info, statErr := os.Stat(backupPath); statErr != nil || !info.IsDir() { + t.Fatalf("backup stat = %v, %v, want retained directory", info, statErr) + } + if _, statErr := os.Lstat(target); !errors.Is(statErr, os.ErrNotExist) { + t.Fatalf("destination stat error = %v, want not exist", statErr) + } + assertOnlyDestinationEntry(t, workingDirectory, filepath.Base(backupPath)) +} + +func TestPublishReturnsCommittedBundleWhenCanceledAfterInstall(t *testing.T) { + workingDirectory := t.TempDir() + target := filepath.Join(workingDirectory, "comparison-daily") + initialPlan, err := PlanDestination(workingDirectory, target, false) + if err != nil { + t.Fatal(err) + } + if _, err := Publish(context.Background(), initialPlan, testBundle()); err != nil { + t.Fatal(err) + } + next := testBundle() + next.Reports[0].Markdown = []byte("# Next\n") + plan, err := PlanDestination(workingDirectory, target, true) + if err != nil { + t.Fatal(err) + } + + ctx, cancel := context.WithCancel(context.Background()) + publication, err := publish(ctx, plan, next, publishOperations{rename: func(oldPath, newPath string) error { + if err := os.Rename(oldPath, newPath); err != nil { + return err + } + if newPath == target && oldPath != target { + cancel() + } + return nil + }}) + if !publication.Committed || err != nil { + t.Fatalf("publish() result/error = %#v/%v, want committed bundle", publication, err) + } + if err := ctx.Err(); !errors.Is(err, context.Canceled) { + t.Fatalf("context error = %v, want cancellation", err) + } + if got := string(readFile(t, filepath.Join(target, next.Reports[0].Path))); got != string(next.Reports[0].Markdown) { + t.Fatalf("published report = %q, want %q", got, next.Reports[0].Markdown) + } + if _, err := RecognizeBundle(target); err != nil { + t.Fatalf("committed bundle recognition error = %v", err) + } + assertOnlyDestinationEntry(t, workingDirectory, filepath.Base(target)) +} + func assertDestinationErrorKind(t *testing.T, workingDirectory, target string, replace bool, want DestinationErrorKind) { t.Helper() _, err := PlanDestination(workingDirectory, target, replace)