fix(service): retry only safe transients and keep existing targets

Fresh step invocation is limited to provider terminated/connection plus
a missing target, or an explicit runner failure. A deliverable already
on disk is accepted and never overwritten.

Co-authored-by: Cursor <cursoragent@cursor.com>

internal/agent/step_retry.go

@@ -44,6 +44,9 @@ type StepFailureSignals struct {
 	WorkDir       string
 	ExtraText     string
 	TargetMissing bool
+	// TargetExists is a non-empty deliverable already on disk. Retry must
+	// not overwrite it.
+	TargetExists bool
 }
 
 type FailureAssessment struct {
@@ -106,27 +109,25 @@ func AssessStepFailure(sig StepFailureSignals) FailureAssessment {
 	lower := strings.ToLower(reason)
 	trace := ScanTraceText(sig.ExtraText + "\n" + ReadWorkDirTraces(sig.WorkDir))
 	missing := sig.TargetMissing || strings.Contains(lower, "missing target deliverable")
-
-	if isPermanentReason(lower) && !trace.ProviderTransient {
-		return FailureAssessment{Class: FailurePermanent, Reason: "permanent: " + compactReason(reason)}
+	if sig.TargetExists {
+		return FailureAssessment{Class: FailurePermanent, Reason: "target-already-present: do not overwrite"}
 	}
-	if isPermanentReason(lower) && !missing {
+	if isPermanentReason(lower) {
 		return FailureAssessment{Class: FailurePermanent, Reason: "permanent: " + compactReason(reason)}
 	}
 
-	if sig.Result != nil && sig.Result.Retryable {
-		return FailureAssessment{Class: FailureTransient, Reason: "runner-retryable: " + compactReason(reason)}
+	runnerFailed := (sig.Result != nil && sig.Result.Retryable) || isRunnerExplicitFailure(lower)
+	provider := trace.ProviderTransient || hasProviderTermOrConn(lower)
+	if runnerFailed {
+		return FailureAssessment{Class: FailureTransient, Reason: "runner-explicit-failure: " + compactReason(reason)}
 	}
-	if trace.ProviderTransient {
-		kind := "provider-transient"
-		if missing && trace.Settled {
+	if provider && missing {
+		kind := "provider-transient-missing-target"
+		if trace.Settled {
 			kind = "settled-failure-missing-target"
 		}
 		return FailureAssessment{Class: FailureTransient, Reason: kind + ": " + compactReason(reason)}
 	}
-	if hasTransientKeyword(lower) {
-		return FailureAssessment{Class: FailureTransient, Reason: "provider-transient: " + compactReason(reason)}
-	}
 	if missing {
 		return FailureAssessment{Class: FailurePermanent, Reason: "permanent-missing: " + compactReason(reason)}
 	}
@@ -156,13 +157,18 @@ func isPermanentReason(lower string) bool {
 		strings.Contains(lower, "治理拦截")
 }
 
-func hasTransientKeyword(lower string) bool {
+func hasProviderTermOrConn(lower string) bool {
 	return strings.Contains(lower, "connection error") ||
 		strings.Contains(lower, "terminated") ||
-		strings.Contains(lower, "auto_retry") ||
 		strings.Contains(lower, "econnreset") ||
-		strings.Contains(lower, "socket hang up") ||
-		strings.Contains(lower, "pi remote runner call failed")
+		strings.Contains(lower, "socket hang up")
+}
+
+func isRunnerExplicitFailure(lower string) bool {
+	return strings.Contains(lower, "pi remote runner call failed") ||
+		strings.Contains(lower, "auto_retry_failed") ||
+		strings.Contains(lower, "auto_retry_end") ||
+		(hasProviderTermOrConn(lower) && !strings.Contains(lower, "missing target deliverable"))
 }
 
 func compactReason(s string) string {

internal/agent/step_retry_test.go

@@ -103,6 +103,22 @@ func TestAssessStepFailure_TransientVsPermanent(t *testing.T) {
 			sig:  StepFailureSignals{Result: &Result{Success: false, ErrorReason: "pi remote runner call failed: connection refused"}},
 			want: FailureTransient,
 		},
+		{
+			name: "existing target is never retried",
+			sig: StepFailureSignals{
+				Result:       &Result{Success: false, ErrorReason: "Connection error.", Retryable: true},
+				TargetExists: true,
+			},
+			want: FailurePermanent,
+		},
+		{
+			name: "provider signal without missing is not enough",
+			sig: StepFailureSignals{
+				Result:    &Result{Success: false, ErrorReason: "agent_not_settled"},
+				ExtraText: `{"type":"auto_retry_end","success":false}`,
+			},
+			want: FailurePermanent,
+		},
 	}
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {

internal/squad/step_retry_test.go

@@ -66,6 +66,11 @@ func (b *countingBackend) Execute(ctx context.Context, req agent.Request) (*agen
 	case "always-transient":
 		return &agent.Result{Success: false, ErrorReason: failText, Retryable: true}, nil
 
+	case "fail-after-writing-target":
+		body := []byte("# already on disk\n\ndo not overwrite\n")
+		_ = os.WriteFile(filepath.Join(req.WorkDir, "output.md"), body, 0o644)
+		return &agent.Result{Success: false, ErrorReason: "Connection error.", Retryable: true}, nil
+
 	default:
 		return &agent.Result{Success: false, ErrorReason: "unknown mode"}, nil
 	}
@@ -165,6 +170,28 @@ func TestWorkflowDriver_DoesNotRetryPermanentMissing(t *testing.T) {
 	}
 }
 
+func TestWorkflowDriver_DoesNotOverwriteExistingTarget(t *testing.T) {
+	wsDir, driver := prepareSingleWriterWS(t)
+	backend := &countingBackend{mode: "fail-after-writing-target"}
+	res, err := driver.Execute(context.Background(), wsDir, backend, squad.ExecuteOptions{Timeout: 5 * time.Second})
+	if err != nil {
+		t.Fatalf("existing target must be accepted, not retried: %v", err)
+	}
+	if !res.Success {
+		t.Fatalf("want success from existing file, got %s", res.ErrorReason)
+	}
+	if backend.calls != 1 {
+		t.Fatalf("must not re-invoke over an existing target, calls=%d", backend.calls)
+	}
+	body, err := os.ReadFile(filepath.Join(wsDir, "tasks", "01_draft", "output.md"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if string(body) != "# already on disk\n\ndo not overwrite\n" {
+		t.Fatalf("target was rewritten: %s", body)
+	}
+}
+
 func TestWorkflowDriver_BoundedTransientRetryThenFailClosed(t *testing.T) {
 	wsDir, driver := prepareSingleWriterWS(t)
 	backend := &countingBackend{mode: "always-transient", failText: "Connection error."}

internal/squad/workflow.go

@@ -511,6 +511,16 @@ func (d *WorkflowDriver) runOneStep(
 		ok       bool
 	)
 	for attempt := 1; attempt <= agent.MaxTransientStepInvocations; attempt++ {
+		onDisk, _ := os.ReadFile(prevOutPath)
+		produced := len(strings.TrimSpace(string(onDisk))) > 0 && string(onDisk) != prevContent
+		if attempt > 1 && produced {
+			ok = true
+			matErr = nil
+			if res != nil {
+				res.Success = true
+			}
+			break
+		}
 		if attempt > 1 && streamer != nil {
 			streamer.EmitPhase(
 				"step_retrying",
@@ -534,18 +544,24 @@ func (d *WorkflowDriver) runOneStep(
 			}
 		}
 
+		onDisk, _ = os.ReadFile(prevOutPath)
+		produced = len(strings.TrimSpace(string(onDisk))) > 0 && string(onDisk) != prevContent
 		newContent := ""
-		if res != nil {
+		if produced {
+			newContent = string(onDisk)
+		} else if res != nil {
 			if b, has := res.Patches[deliverableName]; has {
 				newContent = string(b)
 			}
 		}
 		if newContent == "" {
-			if b, rErr := os.ReadFile(prevOutPath); rErr == nil {
-				newContent = string(b)
-			}
+			newContent = string(onDisk)
+		}
+		ok = (execErr == nil && res != nil && res.Success && matErr == nil) || produced
+		if produced && res != nil {
+			res.Success = true
+			matErr = nil
 		}
-		ok = execErr == nil && res != nil && res.Success && matErr == nil
 		sim := OutputSimilarity(prevContent, newContent)
 		if mu != nil {
 			mu.Lock()
@@ -588,7 +604,8 @@ func (d *WorkflowDriver) runOneStep(
 			Err:           execErr,
 			Result:        res,
 			WorkDir:       subDir,
-			TargetMissing: matErr != nil || (res != nil && strings.Contains(res.ErrorReason, "missing target deliverable")),
+			TargetMissing: !produced && (matErr != nil || (res != nil && strings.Contains(res.ErrorReason, "missing target deliverable"))),
+			TargetExists:  produced,
 		})
 		if assess.Class != agent.FailureTransient || attempt >= agent.MaxTransientStepInvocations {
 			break