fix: auto-resume failed sessions with silent workspace-restore fallback (#696)

apps/backend/internal/orchestrator/task_operations.go

@@ -32,6 +32,11 @@ type PromptResult struct {
 // error-recovery state (WAITING_FOR_INPUT with a non-empty ErrorMessage).
 const resumeReasonErrorRecovery = "error_recovery"
 
+// resumeReasonFailedSessionResumable is the resume reason returned when a
+// FAILED session is auto-resumed because its runtime is Resumable. Distinct
+// from "agent_not_running" so log filtering can isolate FAILED auto-resumes.
+const resumeReasonFailedSessionResumable = "failed_session_resumable"
+
 var ErrAgentPromptInProgress = errors.New("agent is currently processing a prompt")
 var ErrSessionResetInProgress = errors.New("session reset in progress")
 
@@ -1037,7 +1042,18 @@ func (s *Service) GetTaskSessionStatus(ctx context.Context, taskID, sessionID st
 
 	// 3. Session can be resumed if it has a resume token
 	if resumeToken != "" {
-		// Don't auto-resume terminal sessions — they failed/completed for a reason.
+		// Auto-resume FAILED sessions when the runtime is resumable: PR #670 made
+		// terminal-state resume safe (cleanup + retry), so recover transparently
+		// before surfacing the error. Frontend falls back to restore_workspace if
+		// the resume itself fails.
+		if session.State == models.TaskSessionStateFailed && running != nil && running.Resumable {
+			out := s.validateResumeEligibility(session, resp)
+			if out.NeedsResume {
+				out.ResumeReason = resumeReasonFailedSessionResumable
+			}
+			return out, nil
+		}
+		// Don't auto-resume other terminal sessions (CANCELLED stays stopped, COMPLETED is done).
 		if !isActiveSessionState(session.State) {
 			resp.IsAgentRunning = false
 			resp.IsResumable = false

apps/backend/internal/orchestrator/task_operations_resume_test.go

@@ -109,6 +109,168 @@ func TestGetTaskSessionStatus_AutoResumesNormalWaitingSession(t *testing.T) {
 	}
 }
 
+// TestGetTaskSessionStatus_AutoResumesFailedSessionWithResumeToken verifies the
+// failed-but-recoverable path: a FAILED session that still has a resumable
+// runtime + resume token reports NeedsResume=true so the frontend retries
+// transparently instead of showing the historical error to the user.
+func TestGetTaskSessionStatus_AutoResumesFailedSessionWithResumeToken(t *testing.T) {
+	ctx := context.Background()
+	repo := setupTestRepo(t)
+	seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateFailed)
+
+	session, err := repo.GetTaskSession(ctx, "session1")
+	if err != nil {
+		t.Fatalf("failed to load session: %v", err)
+	}
+	session.AgentProfileID = "profile1"
+	session.ErrorMessage = "execution already running"
+	if err := repo.UpdateTaskSession(ctx, session); err != nil {
+		t.Fatalf("failed to update session: %v", err)
+	}
+
+	now := time.Now().UTC()
+	if err := repo.UpsertExecutorRunning(ctx, &models.ExecutorRunning{
+		ID:          "er1",
+		SessionID:   "session1",
+		TaskID:      "task1",
+		Status:      "ready",
+		Resumable:   true,
+		ResumeToken: "acp-session-123",
+		CreatedAt:   now,
+		UpdatedAt:   now,
+	}); err != nil {
+		t.Fatalf("failed to upsert executor running: %v", err)
+	}
+
+	taskRepo := newMockTaskRepo()
+	taskRepo.tasks["task1"] = &v1.Task{ID: "task1", State: v1.TaskStateReview}
+	agentMgr := &mockAgentManager{}
+	svc := createTestServiceWithAgent(repo, newMockStepGetter(), taskRepo, agentMgr)
+	svc.executor = executor.NewExecutor(agentMgr, repo, testLogger(), executor.ExecutorConfig{})
+
+	resp, err := svc.GetTaskSessionStatus(ctx, "task1", "session1")
+	if err != nil {
+		t.Fatalf("GetTaskSessionStatus returned error: %v", err)
+	}
+	if !resp.NeedsResume {
+		t.Fatal("expected NeedsResume=true so frontend auto-resumes a failed session")
+	}
+	if !resp.IsResumable {
+		t.Fatal("expected IsResumable=true for failed session with resumable runtime")
+	}
+	if resp.NeedsWorkspaceRestore {
+		t.Fatal("expected NeedsWorkspaceRestore=false when auto-resuming")
+	}
+	if resp.ResumeReason != resumeReasonFailedSessionResumable {
+		t.Fatalf("expected ResumeReason=%q for FAILED auto-resume, got %q",
+			resumeReasonFailedSessionResumable, resp.ResumeReason)
+	}
+}
+
+// TestGetTaskSessionStatus_FailedSessionWithoutResumableFlagFallsBack verifies
+// that a FAILED session whose runtime is NOT resumable still goes through the
+// workspace-restore path (no auto-resume attempt to avoid certain failure).
+func TestGetTaskSessionStatus_FailedSessionWithoutResumableFlagFallsBack(t *testing.T) {
+	ctx := context.Background()
+	repo := setupTestRepo(t)
+	seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateFailed)
+
+	// Seed a worktree so canRestoreWorkspace returns true.
+	now := time.Now().UTC()
+	if err := repo.CreateTaskSessionWorktree(ctx, &models.TaskSessionWorktree{
+		ID:             "wt1",
+		SessionID:      "session1",
+		WorktreeID:     "wid1",
+		RepositoryID:   "repo1",
+		WorktreePath:   "/tmp/worktrees/session1",
+		WorktreeBranch: "feature/test",
+		CreatedAt:      now,
+	}); err != nil {
+		t.Fatalf("failed to create worktree: %v", err)
+	}
+
+	if err := repo.UpsertExecutorRunning(ctx, &models.ExecutorRunning{
+		ID:          "er1",
+		SessionID:   "session1",
+		TaskID:      "task1",
+		Status:      "ready",
+		Resumable:   false,
+		ResumeToken: "acp-session-123",
+		CreatedAt:   now,
+		UpdatedAt:   now,
+	}); err != nil {
+		t.Fatalf("failed to upsert executor running: %v", err)
+	}
+
+	taskRepo := newMockTaskRepo()
+	taskRepo.tasks["task1"] = &v1.Task{ID: "task1", State: v1.TaskStateReview}
+	agentMgr := &mockAgentManager{}
+	svc := createTestServiceWithAgent(repo, newMockStepGetter(), taskRepo, agentMgr)
+	svc.executor = executor.NewExecutor(agentMgr, repo, testLogger(), executor.ExecutorConfig{})
+
+	resp, err := svc.GetTaskSessionStatus(ctx, "task1", "session1")
+	if err != nil {
+		t.Fatalf("GetTaskSessionStatus returned error: %v", err)
+	}
+	if resp.NeedsResume {
+		t.Fatal("expected NeedsResume=false when runtime is not Resumable")
+	}
+	if !resp.NeedsWorkspaceRestore {
+		t.Fatal("expected NeedsWorkspaceRestore=true as fallback")
+	}
+}
+
+// TestGetTaskSessionStatus_CancelledSessionStaysWorkspaceRestore verifies that
+// CANCELLED sessions are NOT auto-resumed (the user explicitly stopped them).
+func TestGetTaskSessionStatus_CancelledSessionStaysWorkspaceRestore(t *testing.T) {
+	ctx := context.Background()
+	repo := setupTestRepo(t)
+	seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateCancelled)
+
+	now := time.Now().UTC()
+	if err := repo.CreateTaskSessionWorktree(ctx, &models.TaskSessionWorktree{
+		ID:             "wt1",
+		SessionID:      "session1",
+		WorktreeID:     "wid1",
+		RepositoryID:   "repo1",
+		WorktreePath:   "/tmp/worktrees/session1",
+		WorktreeBranch: "feature/test",
+		CreatedAt:      now,
+	}); err != nil {
+		t.Fatalf("failed to create worktree: %v", err)
+	}
+
+	if err := repo.UpsertExecutorRunning(ctx, &models.ExecutorRunning{
+		ID:          "er1",
+		SessionID:   "session1",
+		TaskID:      "task1",
+		Status:      "ready",
+		Resumable:   true,
+		ResumeToken: "acp-session-123",
+		CreatedAt:   now,
+		UpdatedAt:   now,
+	}); err != nil {
+		t.Fatalf("failed to upsert executor running: %v", err)
+	}
+
+	taskRepo := newMockTaskRepo()
+	taskRepo.tasks["task1"] = &v1.Task{ID: "task1", State: v1.TaskStateReview}
+	agentMgr := &mockAgentManager{}
+	svc := createTestServiceWithAgent(repo, newMockStepGetter(), taskRepo, agentMgr)
+	svc.executor = executor.NewExecutor(agentMgr, repo, testLogger(), executor.ExecutorConfig{})
+
+	resp, err := svc.GetTaskSessionStatus(ctx, "task1", "session1")
+	if err != nil {
+		t.Fatalf("GetTaskSessionStatus returned error: %v", err)
+	}
+	if resp.NeedsResume {
+		t.Fatal("expected NeedsResume=false for CANCELLED — user stopped intentionally")
+	}
+	if !resp.NeedsWorkspaceRestore {
+		t.Fatal("expected NeedsWorkspaceRestore=true for CANCELLED with worktree")
+	}
+}
+
 func TestGetTaskSessionStatus_NoAutoResumeWithResumeTokenOnError(t *testing.T) {
 	ctx := context.Background()
 	repo := setupTestRepo(t)

apps/web/hooks/domains/session/use-session-resumption.test.ts

@@ -0,0 +1,158 @@
+import { describe, it, expect, vi, beforeEach } from "vitest";
+
+const mockRequest = vi.fn();
+
+vi.mock("@/lib/ws/connection", () => ({
+  getWebSocketClient: () => ({ request: mockRequest }),
+}));
+
+import {
+  resumeWithSilentFallback,
+  type ResumeStateSetter,
+  type ResumptionState,
+} from "./use-session-resumption";
+
+type SetterCalls = {
+  resumptionStates: ResumptionState[];
+  errors: (string | null)[];
+  worktreePaths: (string | null)[];
+  worktreeBranches: (string | null)[];
+  taskSessionStates: string[];
+};
+
+function createSetters(): { setters: ResumeStateSetter; calls: SetterCalls } {
+  const calls: SetterCalls = {
+    resumptionStates: [],
+    errors: [],
+    worktreePaths: [],
+    worktreeBranches: [],
+    taskSessionStates: [],
+  };
+  const setters: ResumeStateSetter = {
+    setResumptionState: (s: ResumptionState) => {
+      calls.resumptionStates.push(s);
+    },
+    setError: (e: string | null) => {
+      calls.errors.push(e);
+    },
+    setWorktreePath: (p: string | null) => {
+      calls.worktreePaths.push(p);
+    },
+    setWorktreeBranch: (b: string | null) => {
+      calls.worktreeBranches.push(b);
+    },
+    setTaskSession: (s: { state: string }) => {
+      calls.taskSessionStates.push(s.state);
+    },
+  };
+  return { setters, calls };
+}
+
+describe("resumeWithSilentFallback", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    // tryLaunch logs caught errors via console.error; silence in tests so the
+    // expected error paths don't pollute the test output.
+    vi.spyOn(console, "error").mockImplementation(() => {});
+  });
+
+  it("uses resume on first try when it succeeds, never calling restore_workspace", async () => {
+    mockRequest.mockResolvedValueOnce({
+      success: true,
+      task_id: "t1",
+      session_id: "s1",
+      state: "STARTING",
+      worktree_path: "/wt/foo",
+      worktree_branch: "feature/foo",
+    });
+    const { setters, calls } = createSetters();
+
+    await resumeWithSilentFallback("t1", "s1", null, setters);
+
+    expect(mockRequest).toHaveBeenCalledTimes(1);
+    expect(mockRequest).toHaveBeenCalledWith(
+      "session.launch",
+      expect.objectContaining({ intent: "resume", session_id: "s1" }),
+      expect.any(Number),
+    );
+    expect(calls.resumptionStates).toContain("resumed");
+    expect(calls.errors).not.toContain(expect.any(String));
+    expect(calls.worktreePaths).toContain("/wt/foo");
+  });
+
+  it("falls back to restore_workspace silently when resume returns success=false", async () => {
+    // 1st call: resume fails. 2nd call: restore_workspace succeeds.
+    mockRequest
+      .mockResolvedValueOnce({ success: false, task_id: "t1", state: "FAILED" })
+      .mockResolvedValueOnce({
+        success: true,
+        task_id: "t1",
+        session_id: "s1",
+        state: "FAILED",
+        worktree_path: "/wt/foo",
+      });
+    const { setters, calls } = createSetters();
+
+    await resumeWithSilentFallback("t1", "s1", null, setters);
+
+    expect(mockRequest).toHaveBeenCalledTimes(2);
+    expect(mockRequest).toHaveBeenNthCalledWith(
+      1,
+      "session.launch",
+      expect.objectContaining({ intent: "resume" }),
+      expect.any(Number),
+    );
+    expect(mockRequest).toHaveBeenNthCalledWith(
+      2,
+      "session.launch",
+      expect.objectContaining({ intent: "restore_workspace" }),
+      expect.any(Number),
+    );
+    // Final state is "resumed" (from successful restore), no error surfaced.
+    expect(calls.resumptionStates.at(-1)).toBe("resumed");
+    expect(calls.errors.filter((e) => typeof e === "string")).toHaveLength(0);
+  });
+
+  it("falls back to restore_workspace silently when resume throws", async () => {
+    mockRequest
+      .mockRejectedValueOnce(new Error("ws timeout"))
+      .mockResolvedValueOnce({ success: true, task_id: "t1", session_id: "s1", state: "FAILED" });
+    const { setters, calls } = createSetters();
+
+    await resumeWithSilentFallback("t1", "s1", null, setters);
+
+    expect(mockRequest).toHaveBeenCalledTimes(2);
+    expect(calls.resumptionStates.at(-1)).toBe("resumed");
+    expect(calls.errors.filter((e) => typeof e === "string")).toHaveLength(0);
+  });
+
+  it("surfaces an error only when BOTH resume and restore_workspace fail", async () => {
+    mockRequest
+      .mockResolvedValueOnce({ success: false, task_id: "t1", state: "FAILED" })
+      .mockResolvedValueOnce({ success: false, task_id: "t1", state: "FAILED" });
+    const { setters, calls } = createSetters();
+
+    await resumeWithSilentFallback("t1", "s1", null, setters);
+
+    expect(mockRequest).toHaveBeenCalledTimes(2);
+    expect(calls.resumptionStates.at(-1)).toBe("error");
+    expect(calls.errors.at(-1)).toBe(
+      "Failed to resume session — workspace restore also unavailable",
+    );
+  });
+
+  it("surfaces an error when both resume and restore_workspace throw", async () => {
+    mockRequest
+      .mockRejectedValueOnce(new Error("ws closed"))
+      .mockRejectedValueOnce(new Error("still closed"));
+    const { setters, calls } = createSetters();
+
+    await resumeWithSilentFallback("t1", "s1", null, setters);
+
+    expect(mockRequest).toHaveBeenCalledTimes(2);
+    expect(calls.resumptionStates.at(-1)).toBe("error");
+    expect(calls.errors.at(-1)).toBe(
+      "Failed to resume session — workspace restore also unavailable",
+    );
+  });
+});

apps/web/hooks/domains/session/use-session-resumption.ts

@@ -44,7 +44,7 @@ type ResumeResponse = {
   error?: string;
 };
 
-type ResumeStateSetter = {
+export type ResumeStateSetter = {
   setResumptionState: (s: ResumptionState) => void;
   setError: (e: string | null) => void;
   setWorktreePath: (p: string | null) => void;
@@ -117,6 +117,83 @@ async function resumeViaLaunch(
   );
 }
 
+/** Attempt resume, silently falling back to restore_workspace on any failure.
+ *  Used for sessions where the backend reports needs_resume=true — typically
+ *  WAITING_FOR_INPUT after restart, or FAILED with a resumable token. The user
+ *  only sees an error banner if BOTH attempts fail; otherwise they just see
+ *  the session reload (resumed) or the workspace come back read-only.
+ *  Exported for unit tests. */
+export async function resumeWithSilentFallback(
+  taskId: string,
+  sessionId: string,
+  session: SessionLike,
+  setters: ResumeStateSetter,
+): Promise<void> {
+  setters.setResumptionState("resuming");
+  if (
+    await tryLaunch(
+      buildResumeRequest(taskId, sessionId).request,
+      taskId,
+      sessionId,
+      session,
+      setters,
+    )
+  ) {
+    return;
+  }
+  // Resume failed (returned success=false OR threw). Fall back to read-only
+  // workspace restore so the user keeps file/terminal/git access.
+  if (
+    await tryLaunch(
+      buildRestoreWorkspaceRequest(taskId, sessionId).request,
+      taskId,
+      sessionId,
+      session,
+      setters,
+    )
+  ) {
+    return;
+  }
+  setters.setResumptionState("error");
+  setters.setError("Failed to resume session — workspace restore also unavailable");
+}
+
+/** Run a single launch attempt; returns true on success, false on any failure.
+ *  Logs caught errors to the console so silent fallback paths remain debuggable
+ *  (errors otherwise vanish into the implicit `false` return). */
+async function tryLaunch(
+  request: import("@/lib/services/session-launch-service").LaunchSessionRequest,
+  taskId: string,
+  sessionId: string,
+  session: SessionLike,
+  setters: ResumeStateSetter,
+): Promise<boolean> {
+  try {
+    const resp = await launchSession(request);
+    if (!resp.success) return false;
+    applyResumeResponse(
+      {
+        success: true,
+        state: resp.state,
+        worktree_path: resp.worktree_path,
+        worktree_branch: resp.worktree_branch,
+      },
+      taskId,
+      sessionId,
+      session,
+      setters,
+    );
+    return true;
+  } catch (err) {
+    console.error("[tryLaunch] session launch failed", {
+      intent: request.intent,
+      sessionId,
+      err,
+    });
+    return false;
+  }
+}
+
 type CheckAndResumeParams = {
   taskId: string;
   sessionId: string;
@@ -177,7 +254,7 @@ async function checkAndResume({
     if (status.is_agent_running) {
       setters.setResumptionState("running");
     } else if (status.needs_resume && status.is_resumable) {
-      await resumeViaLaunch(taskId, sessionId, session, setters, buildResumeRequest);
+      await resumeWithSilentFallback(taskId, sessionId, session, setters);
     } else if (status.needs_workspace_restore) {
       await resumeViaLaunch(taskId, sessionId, session, setters, buildRestoreWorkspaceRequest);
     } else {