Complete diagnostics retention semantics

internal/core/diagnostics/retention_test.go

@@ -0,0 +1,72 @@
+package diagnostics
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+func TestShouldRetainRunDirectoryMatrix(t *testing.T) {
+	tests := []struct {
+		name  string
+		input RetentionDecisionInput
+		want  bool
+	}{
+		{name: "always success keeps", input: RetentionDecisionInput{RetentionMode: "always", RunSucceeded: true}, want: true},
+		{name: "always failure keeps", input: RetentionDecisionInput{RetentionMode: "always", RunSucceeded: false}, want: true},
+		{name: "never success keeps", input: RetentionDecisionInput{RetentionMode: "never", RunSucceeded: true}, want: true},
+		{name: "never failure keeps", input: RetentionDecisionInput{RetentionMode: "never", RunSucceeded: false}, want: true},
+		{name: "auto success no skips removes", input: RetentionDecisionInput{RetentionMode: "auto", RunSucceeded: true, HasSkippedCorrections: false}, want: false},
+		{name: "auto success skips keeps", input: RetentionDecisionInput{RetentionMode: "auto", RunSucceeded: true, HasSkippedCorrections: true}, want: true},
+		{name: "auto failure keeps", input: RetentionDecisionInput{RetentionMode: "auto", RunSucceeded: false}, want: true},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			got := ShouldRetainRunDirectory(tc.input)
+			if got != tc.want {
+				t.Fatalf("unexpected retain decision: got=%v want=%v", got, tc.want)
+			}
+		})
+	}
+}
+
+func TestApplyRetentionRemovesWhenDecisionSaysRemove(t *testing.T) {
+	workDir := t.TempDir()
+	runPath := filepath.Join(workDir, "run-test")
+	if err := os.Mkdir(runPath, 0o755); err != nil {
+		t.Fatalf("mkdir run path: %v", err)
+	}
+
+	runDir := &RunDirectory{path: runPath, retention: "auto"}
+	err := runDir.ApplyRetention(RetentionDecisionInput{RunSucceeded: true, HasSkippedCorrections: false})
+	if err != nil {
+		t.Fatalf("ApplyRetention failed: %v", err)
+	}
+
+	if _, err := os.Stat(runPath); !os.IsNotExist(err) {
+		t.Fatalf("expected run directory removed, stat err=%v", err)
+	}
+}
+
+func TestApplyRetentionReturnsRemovalError(t *testing.T) {
+	parent := t.TempDir()
+	runPath := filepath.Join(parent, "run-test")
+	if err := os.Mkdir(runPath, 0o755); err != nil {
+		t.Fatalf("mkdir run path: %v", err)
+	}
+
+	// Make parent non-writable so removing child fails.
+	if err := os.Chmod(parent, 0o500); err != nil {
+		t.Fatalf("chmod parent: %v", err)
+	}
+	t.Cleanup(func() {
+		_ = os.Chmod(parent, 0o700)
+	})
+
+	runDir := &RunDirectory{path: runPath, retention: "auto"}
+	err := runDir.ApplyRetention(RetentionDecisionInput{RunSucceeded: true, HasSkippedCorrections: false})
+	if err == nil {
+		t.Fatalf("expected removal error, got nil")
+	}
+}

internal/core/diagnostics/run_dir.go

@@ -21,6 +21,31 @@ type RunDirectory struct {
 	createdAt time.Time
 }
 
+type RetentionDecisionInput struct {
+	RetentionMode         string
+	RunSucceeded          bool
+	HasSkippedCorrections bool
+}
+
+func ShouldRetainRunDirectory(input RetentionDecisionInput) bool {
+	// Failed runs are always retained.
+	if !input.RunSucceeded {
+		return true
+	}
+
+	switch input.RetentionMode {
+	case "always":
+		return true
+	case "never":
+		return true
+	case "auto":
+		return input.HasSkippedCorrections
+	default:
+		// Be conservative for unknown values.
+		return true
+	}
+}
+
 // InvocationMetadata captures non-secret invocation details for diagnostics.
 type InvocationMetadata struct {
 	Operation      string    `json:"operation"`
@@ -187,20 +212,14 @@ func (r *RunDirectory) WriteChunkingSummary(summary *chunking.DetailedSummary) e
 	return nil
 }
 
-// TODO: In later phases, implement full "auto" semantics based on skip/report outcomes.
-func (r *RunDirectory) ApplyRetention(runSucceeded bool) error {
-	if !runSucceeded {
-		// Failed runs are always retained.
-		return nil
+func (r *RunDirectory) ApplyRetention(input RetentionDecisionInput) error {
+	decision := input
+	if decision.RetentionMode == "" {
+		decision.RetentionMode = r.retention
 	}
 
-	switch r.retention {
-	case "always":
-		return nil
-	case "never", "auto":
-		return os.RemoveAll(r.path)
-	default:
-		// Retain by default for unknown modes; config validation should prevent this.
+	if ShouldRetainRunDirectory(decision) {
 		return nil
 	}
+	return os.RemoveAll(r.path)
 }