Report comparison cleanup recovery state
This commit is contained in:
@@ -87,27 +87,42 @@ func TestCompareDetailedPublishesPartialBundleAndReturnsAggregateError(t *testin
|
||||
}
|
||||
|
||||
func TestCompareDetailedRetainsCommittedPathsWhenBackupCleanupFails(t *testing.T) {
|
||||
bundle := generationBundle(t)
|
||||
backupPath := filepath.Join(t.TempDir(), ".comparison-daily.backup-retained")
|
||||
cleanupCause := errors.New("backup cleanup failed")
|
||||
publish := func(context.Context, comparison.DestinationPlan, comparison.LogicalBundle) (comparison.PublicationResult, error) {
|
||||
return comparison.PublicationResult{Committed: true, RetainedBackupPath: backupPath}, &comparison.PublicationCleanupError{RetainedBackupPath: backupPath, Err: cleanupCause}
|
||||
}
|
||||
for _, test := range []struct {
|
||||
name string
|
||||
state comparison.BackupRecoveryState
|
||||
path bool
|
||||
}{
|
||||
{name: "complete recovery bundle", state: comparison.BackupRecoveryComplete, path: true},
|
||||
{name: "partial remnants", state: comparison.BackupRecoveryPartial, path: true},
|
||||
{name: "absent backup", state: comparison.BackupRecoveryAbsent},
|
||||
} {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
bundle := generationBundle(t)
|
||||
recoveryPath := ""
|
||||
if test.path {
|
||||
recoveryPath = filepath.Join(t.TempDir(), ".comparison-daily.backup-recovery")
|
||||
}
|
||||
cleanupCause := errors.New("backup cleanup failed")
|
||||
publish := func(context.Context, comparison.DestinationPlan, comparison.LogicalBundle) (comparison.PublicationResult, error) {
|
||||
return comparison.PublicationResult{Committed: true, RecoveryState: test.state, RecoveryPath: recoveryPath}, &comparison.PublicationCleanupError{RecoveryState: test.state, RecoveryPath: recoveryPath, Err: cleanupCause}
|
||||
}
|
||||
|
||||
result, err := compareDetailed(context.Background(), ComparisonRequest{
|
||||
Config: comparisonConfig(), Report: ReportDaily, ProfileIDs: []string{"weather-light", "weather-deep"},
|
||||
WorkingDir: t.TempDir(), Date: generationTime("2026-05-29T12:00:00-05:00"),
|
||||
Clock: timeutil.FixedClock{Time: generationTime("2026-05-29T08:30:00-05:00")},
|
||||
Collector: &generationCollector{bundle: &bundle}, Executor: &generationExecutor{},
|
||||
}, publish)
|
||||
var cleanupErr *comparison.PublicationCleanupError
|
||||
if result == nil || !errors.As(err, &cleanupErr) || !errors.Is(err, cleanupCause) || cleanupErr.RetainedBackupPath != backupPath || !filepath.IsAbs(result.ManifestPath) || !filepath.IsAbs(result.DataPackagePath) {
|
||||
t.Fatalf("CompareDetailed() result/error = %#v/%v", result, err)
|
||||
}
|
||||
for _, profile := range result.Results {
|
||||
if profile.Status == comparison.StatusSucceeded && !filepath.IsAbs(profile.ReportPath) {
|
||||
t.Fatalf("published profile result = %#v", profile)
|
||||
}
|
||||
result, err := compareDetailed(context.Background(), ComparisonRequest{
|
||||
Config: comparisonConfig(), Report: ReportDaily, ProfileIDs: []string{"weather-light", "weather-deep"},
|
||||
WorkingDir: t.TempDir(), Date: generationTime("2026-05-29T12:00:00-05:00"),
|
||||
Clock: timeutil.FixedClock{Time: generationTime("2026-05-29T08:30:00-05:00")},
|
||||
Collector: &generationCollector{bundle: &bundle}, Executor: &generationExecutor{},
|
||||
}, publish)
|
||||
var cleanupErr *comparison.PublicationCleanupError
|
||||
if result == nil || !errors.As(err, &cleanupErr) || !errors.Is(err, cleanupCause) || cleanupErr.RecoveryState != test.state || cleanupErr.RecoveryPath != recoveryPath || !filepath.IsAbs(result.ManifestPath) || !filepath.IsAbs(result.DataPackagePath) {
|
||||
t.Fatalf("CompareDetailed() result/error = %#v/%v", result, err)
|
||||
}
|
||||
for _, profile := range result.Results {
|
||||
if profile.Status == comparison.StatusSucceeded && !filepath.IsAbs(profile.ReportPath) {
|
||||
t.Fatalf("published profile result = %#v", profile)
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -223,9 +223,9 @@ func TestCompareCommandReportsCommittedBundleWhenCleanupFails(t *testing.T) {
|
||||
{Position: 1, ProfileID: "weather-light", ModelName: "light", Status: comparison.StatusSucceeded, ValidationStatus: promptexec.ValidationPassed, ReportPath: filepath.Join(outputDirectory, "01-weather-light.md")},
|
||||
{Position: 2, ProfileID: "weather-deep", ModelName: "deep", Status: comparison.StatusSucceeded, ValidationStatus: promptexec.ValidationPassed, ReportPath: filepath.Join(outputDirectory, "02-weather-deep.md")},
|
||||
})
|
||||
backupPath := filepath.Join(workingDir, ".comparison-daily.backup-retained")
|
||||
recoveryPath := filepath.Join(workingDir, ".comparison-daily.backup-retained")
|
||||
cleanupCause := errors.New("filesystem cleanup detail")
|
||||
cleanupErr := &comparison.PublicationCleanupError{RetainedBackupPath: backupPath, Err: cleanupCause}
|
||||
cleanupErr := &comparison.PublicationCleanupError{RecoveryState: comparison.BackupRecoveryComplete, RecoveryPath: recoveryPath, Err: cleanupCause}
|
||||
runner := comparisonRunner(t, workingDir)
|
||||
runner.compareDetailed = func(context.Context, app.ComparisonRequest) (*app.ComparisonResult, error) {
|
||||
return result, cleanupErr
|
||||
@@ -240,10 +240,10 @@ func TestCompareCommandReportsCommittedBundleWhenCleanupFails(t *testing.T) {
|
||||
if err := json.Unmarshal(stdout.Bytes(), &summary); err != nil {
|
||||
t.Fatalf("decode summary: %v\n%s", err, stdout.String())
|
||||
}
|
||||
if summary.Status != summaryStatusFailed || summary.Error == nil || summary.Error.Category != "publication_cleanup" || summary.Error.Message != "comparison published but cleanup did not complete" || summary.ManifestPath != result.ManifestPath || summary.DataPackagePath != result.DataPackagePath || summary.Results[0].ReportPath == "" {
|
||||
if summary.Status != summaryStatusFailed || summary.Error == nil || summary.Error.Category != "publication_cleanup" || summary.Error.Message != "comparison published but a complete prior bundle remains" || summary.ManifestPath != result.ManifestPath || summary.DataPackagePath != result.DataPackagePath || summary.Results[0].ReportPath == "" {
|
||||
t.Fatalf("summary = %#v", summary)
|
||||
}
|
||||
for _, unsafe := range []string{cleanupCause.Error(), backupPath} {
|
||||
for _, unsafe := range []string{cleanupCause.Error(), recoveryPath} {
|
||||
if strings.Contains(stdout.String(), unsafe) {
|
||||
t.Fatalf("summary contains unsafe recovery detail %q: %s", unsafe, stdout.String())
|
||||
}
|
||||
|
||||
@@ -225,7 +225,7 @@ func safeComparisonSummaryError(err error) *comparison.SafeError {
|
||||
}
|
||||
var cleanupErr *comparison.PublicationCleanupError
|
||||
if errors.As(err, &cleanupErr) {
|
||||
safe := comparison.NewSafeError("publication_cleanup", "comparison published but cleanup did not complete")
|
||||
safe := comparison.NewSafeError("publication_cleanup", publicationCleanupMessage(cleanupErr.RecoveryState))
|
||||
return &safe
|
||||
}
|
||||
var destinationErr *comparison.DestinationError
|
||||
@@ -249,6 +249,19 @@ func safeComparisonSummaryError(err error) *comparison.SafeError {
|
||||
return &safe
|
||||
}
|
||||
|
||||
func publicationCleanupMessage(state comparison.BackupRecoveryState) string {
|
||||
switch state {
|
||||
case comparison.BackupRecoveryComplete:
|
||||
return "comparison published but a complete prior bundle remains"
|
||||
case comparison.BackupRecoveryPartial:
|
||||
return "comparison published but partial cleanup remnants remain"
|
||||
case comparison.BackupRecoveryAbsent:
|
||||
return "comparison published but no prior bundle remains"
|
||||
default:
|
||||
return "comparison published but cleanup recovery state is unknown"
|
||||
}
|
||||
}
|
||||
|
||||
func comparisonAggregateErrorMessage(err error) (string, bool) {
|
||||
const prefix = "comparison completed with "
|
||||
const suffix = " failed profiles"
|
||||
|
||||
@@ -179,12 +179,36 @@ func TestSafeComparisonSummaryErrorClassifiesWrappedFailures(t *testing.T) {
|
||||
message: "comparison destination preflight failed",
|
||||
},
|
||||
{
|
||||
name: "publication cleanup",
|
||||
name: "complete cleanup recovery",
|
||||
err: fmt.Errorf("outer wrapper: %w", &comparison.PublicationCleanupError{
|
||||
RetainedBackupPath: "/tmp/" + unsafeDetail, Err: fmt.Errorf("%w: %s", context.Canceled, unsafeDetail),
|
||||
RecoveryState: comparison.BackupRecoveryComplete, RecoveryPath: "/tmp/" + unsafeDetail, Err: fmt.Errorf("%w: %s", context.Canceled, unsafeDetail),
|
||||
}),
|
||||
category: "publication_cleanup",
|
||||
message: "comparison published but cleanup did not complete",
|
||||
message: "comparison published but a complete prior bundle remains",
|
||||
},
|
||||
{
|
||||
name: "partial cleanup remnants",
|
||||
err: fmt.Errorf("outer wrapper: %w", &comparison.PublicationCleanupError{
|
||||
RecoveryState: comparison.BackupRecoveryPartial, RecoveryPath: "/tmp/" + unsafeDetail, Err: fmt.Errorf("%w: %s", context.Canceled, unsafeDetail),
|
||||
}),
|
||||
category: "publication_cleanup",
|
||||
message: "comparison published but partial cleanup remnants remain",
|
||||
},
|
||||
{
|
||||
name: "absent cleanup recovery",
|
||||
err: fmt.Errorf("outer wrapper: %w", &comparison.PublicationCleanupError{
|
||||
RecoveryState: comparison.BackupRecoveryAbsent, Err: fmt.Errorf("%w: %s", context.Canceled, unsafeDetail),
|
||||
}),
|
||||
category: "publication_cleanup",
|
||||
message: "comparison published but no prior bundle remains",
|
||||
},
|
||||
{
|
||||
name: "unknown cleanup recovery",
|
||||
err: fmt.Errorf("outer wrapper: %w", &comparison.PublicationCleanupError{
|
||||
RecoveryState: comparison.BackupRecoveryUnknown, Err: fmt.Errorf("%w: %s", context.Canceled, unsafeDetail),
|
||||
}),
|
||||
category: "publication_cleanup",
|
||||
message: "comparison published but cleanup recovery state is unknown",
|
||||
},
|
||||
{
|
||||
name: "unknown",
|
||||
|
||||
@@ -445,22 +445,44 @@ func unrecognizedBundleError(action string, err error) error {
|
||||
return fmt.Errorf("%w: %s: %v", ErrUnrecognizedBundle, action, err)
|
||||
}
|
||||
|
||||
// BackupRecoveryState describes what remains of a prior bundle after cleanup
|
||||
// reports an error.
|
||||
type BackupRecoveryState string
|
||||
|
||||
const (
|
||||
BackupRecoveryComplete BackupRecoveryState = "complete"
|
||||
BackupRecoveryPartial BackupRecoveryState = "partial"
|
||||
BackupRecoveryAbsent BackupRecoveryState = "absent"
|
||||
BackupRecoveryUnknown BackupRecoveryState = "unknown"
|
||||
)
|
||||
|
||||
// PublicationResult describes the durable state of a publication attempt.
|
||||
type PublicationResult struct {
|
||||
Committed bool
|
||||
RetainedBackupPath string
|
||||
Committed bool
|
||||
RecoveryState BackupRecoveryState
|
||||
RecoveryPath string
|
||||
}
|
||||
|
||||
// PublicationCleanupError reports that a committed bundle could not remove its
|
||||
// prior sibling backup. The new bundle remains installed and the backup path
|
||||
// is retained for operator recovery.
|
||||
// PublicationCleanupError reports that a committed bundle could not completely
|
||||
// remove its prior sibling backup. The new bundle remains installed; recovery
|
||||
// fields describe the state observed after cleanup failed.
|
||||
type PublicationCleanupError struct {
|
||||
RetainedBackupPath string
|
||||
Err error
|
||||
RecoveryState BackupRecoveryState
|
||||
RecoveryPath string
|
||||
Err error
|
||||
}
|
||||
|
||||
func (err *PublicationCleanupError) Error() string {
|
||||
return fmt.Sprintf("remove comparison backup %q: %v", err.RetainedBackupPath, err.Err)
|
||||
switch err.RecoveryState {
|
||||
case BackupRecoveryComplete:
|
||||
return fmt.Sprintf("remove comparison backup %q: %v; complete recovery bundle remains", err.RecoveryPath, err.Err)
|
||||
case BackupRecoveryPartial:
|
||||
return fmt.Sprintf("remove comparison backup %q: %v; partial remnants remain", err.RecoveryPath, err.Err)
|
||||
case BackupRecoveryAbsent:
|
||||
return fmt.Sprintf("remove comparison backup: %v; no recovery bundle remains", err.Err)
|
||||
default:
|
||||
return fmt.Sprintf("remove comparison backup: %v; recovery state is unknown", err.Err)
|
||||
}
|
||||
}
|
||||
|
||||
func (err *PublicationCleanupError) Unwrap() error {
|
||||
@@ -558,12 +580,26 @@ func publish(ctx context.Context, plan DestinationPlan, bundle LogicalBundle, op
|
||||
}
|
||||
temporaryDirectory = ""
|
||||
if err := operations.removeAll(backupDirectory); err != nil {
|
||||
cleanupErr := &PublicationCleanupError{RetainedBackupPath: backupDirectory, Err: err}
|
||||
return PublicationResult{Committed: true, RetainedBackupPath: backupDirectory}, cleanupErr
|
||||
recoveryState, recoveryPath := inspectBackupRecovery(backupDirectory)
|
||||
cleanupErr := &PublicationCleanupError{RecoveryState: recoveryState, RecoveryPath: recoveryPath, Err: err}
|
||||
return PublicationResult{Committed: true, RecoveryState: recoveryState, RecoveryPath: recoveryPath}, cleanupErr
|
||||
}
|
||||
return PublicationResult{Committed: true}, nil
|
||||
}
|
||||
|
||||
func inspectBackupRecovery(backupDirectory string) (BackupRecoveryState, string) {
|
||||
if _, err := os.Lstat(backupDirectory); err != nil {
|
||||
if errors.Is(err, os.ErrNotExist) {
|
||||
return BackupRecoveryAbsent, ""
|
||||
}
|
||||
return BackupRecoveryUnknown, ""
|
||||
}
|
||||
if _, err := RecognizeBundle(backupDirectory); err == nil {
|
||||
return BackupRecoveryComplete, backupDirectory
|
||||
}
|
||||
return BackupRecoveryPartial, backupDirectory
|
||||
}
|
||||
|
||||
func authorizeMovedDestination(plan DestinationPlan, backupDirectory string) error {
|
||||
backupPlan, err := PlanDestination(plan.WorkingDirectory, backupDirectory, plan.Replace)
|
||||
if err != nil {
|
||||
|
||||
@@ -577,7 +577,7 @@ func TestPublishReportsCommittedBundleWhenBackupCleanupFails(t *testing.T) {
|
||||
},
|
||||
})
|
||||
var cleanupErr *PublicationCleanupError
|
||||
if !result.Committed || result.RetainedBackupPath != backupPath || !filepath.IsAbs(backupPath) || !errors.As(err, &cleanupErr) || cleanupErr.RetainedBackupPath != backupPath || !errors.Is(err, cleanupCause) {
|
||||
if !result.Committed || result.RecoveryState != BackupRecoveryComplete || result.RecoveryPath != backupPath || !filepath.IsAbs(backupPath) || !errors.As(err, &cleanupErr) || cleanupErr.RecoveryState != BackupRecoveryComplete || cleanupErr.RecoveryPath != backupPath || !errors.Is(err, cleanupCause) {
|
||||
t.Fatalf("publish() result/error = %#v/%v", result, err)
|
||||
}
|
||||
if _, err := RecognizeBundle(target); err != nil {
|
||||
@@ -592,6 +592,82 @@ func TestPublishReportsCommittedBundleWhenBackupCleanupFails(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestPublishReportsPartialRecoveryAfterBackupCleanupFailure(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)
|
||||
}
|
||||
cleanupCause := errors.New("backup removal failed")
|
||||
var backupPath string
|
||||
result, err := publish(context.Background(), plan, testBundle(), publishOperations{
|
||||
rename: os.Rename,
|
||||
removeAll: func(path string) error {
|
||||
backupPath = path
|
||||
if err := os.Remove(filepath.Join(path, ManifestFilename)); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return cleanupCause
|
||||
},
|
||||
})
|
||||
var cleanupErr *PublicationCleanupError
|
||||
if !result.Committed || result.RecoveryState != BackupRecoveryPartial || result.RecoveryPath != backupPath || !errors.As(err, &cleanupErr) || cleanupErr.RecoveryState != BackupRecoveryPartial || cleanupErr.RecoveryPath != backupPath || !errors.Is(err, cleanupCause) {
|
||||
t.Fatalf("publish() result/error = %#v/%v", result, err)
|
||||
}
|
||||
if _, err := RecognizeBundle(target); err != nil {
|
||||
t.Fatalf("new bundle recognition error = %v", err)
|
||||
}
|
||||
if _, err := RecognizeBundle(backupPath); err == nil {
|
||||
t.Fatal("partially removed backup was recognized")
|
||||
}
|
||||
}
|
||||
|
||||
func TestPublishReportsAbsentRecoveryAfterBackupCleanupFailure(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)
|
||||
}
|
||||
cleanupCause := errors.New("backup removal failed")
|
||||
var backupPath string
|
||||
result, err := publish(context.Background(), plan, testBundle(), publishOperations{
|
||||
rename: os.Rename,
|
||||
removeAll: func(path string) error {
|
||||
backupPath = path
|
||||
if err := os.RemoveAll(path); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return cleanupCause
|
||||
},
|
||||
})
|
||||
var cleanupErr *PublicationCleanupError
|
||||
if !result.Committed || result.RecoveryState != BackupRecoveryAbsent || result.RecoveryPath != "" || !errors.As(err, &cleanupErr) || cleanupErr.RecoveryState != BackupRecoveryAbsent || cleanupErr.RecoveryPath != "" || !errors.Is(err, cleanupCause) {
|
||||
t.Fatalf("publish() result/error = %#v/%v", result, err)
|
||||
}
|
||||
if _, err := RecognizeBundle(target); err != nil {
|
||||
t.Fatalf("new bundle recognition error = %v", err)
|
||||
}
|
||||
if _, err := os.Lstat(backupPath); !errors.Is(err, os.ErrNotExist) {
|
||||
t.Fatalf("backup stat error = %v, want not exist", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPublishRestoresExistingBundleAfterReplacementFailure(t *testing.T) {
|
||||
workingDirectory := t.TempDir()
|
||||
target := filepath.Join(workingDirectory, "comparison-daily")
|
||||
|
||||
Reference in New Issue
Block a user