fix: harden supervisor reset edge cases

Author: pallyoung

packages/server/src/__tests__/supervisor-manager.test.ts

@@ -798,6 +798,56 @@ describe("SupervisorManager cycle triggers", () => {
     expect(deps.supervisorRepo.findById(supervisor.id)?.objective).toBe("Initial objective");
   });
 
+  it("preserves a pause request while an objective change abort is in flight", async () => {
+    const supervisor = await manager.create({
+      sessionId: "sess-objective-pause-race",
+      workspaceId: "ws-1",
+      objective: "Initial objective",
+      evaluatorProviderId: "codex",
+    });
+
+    let observedSignal: AbortSignal | undefined;
+    vi.spyOn(getManagerInternals().evaluator, "evaluate").mockImplementationOnce(
+      async (_supervisor, _context, options) =>
+        await new Promise<SupervisorEvaluationResult>((_resolve, reject) => {
+          observedSignal = options?.signal;
+          options?.signal?.addEventListener(
+            "abort",
+            () => {
+              reject({
+                code: "supervisor_eval_aborted",
+                message: "Supervisor evaluator aborted",
+              });
+            },
+            { once: true }
+          );
+        })
+    );
+
+    await manager.triggerEvaluation(supervisor.id);
+
+    await waitFor(() => {
+      expect(observedSignal).toBeDefined();
+      expect(manager.get(supervisor.id)?.state).toBe("evaluating");
+    });
+
+    const updatedPromise = manager.update(supervisor.id, {
+      objective: "New objective",
+    });
+    const paused = await manager.pause(supervisor.id);
+
+    await waitFor(() => {
+      expect(observedSignal?.aborted).toBe(true);
+    });
+
+    const updated = await updatedPromise;
+
+    expect(paused.state).toBe("paused");
+    expect(updated.state).toBe("paused");
+    expect(updated.objective).toBe("New objective");
+    expect(manager.get(supervisor.id)?.state).toBe("paused");
+  });
+
   it("retries evaluator timeout up to the global retry budget", async () => {
     vi.useFakeTimers();
     deps.settingsRepo.get = vi.fn((key: string) => {

packages/server/src/supervisor/manager.ts

@@ -824,9 +824,12 @@ export class SupervisorManager {
         }
 
         if (this.pendingObjectiveUpdates.has(supervisorId)) {
+          const nextState: SupervisorState = this.pendingPauses.has(supervisorId)
+            ? "paused"
+            : "idle";
           const recoveredSupervisor = this.attachCycles(
             this.deps.supervisorRepo.update(supervisorId, {
-              state: "idle",
+              state: nextState,
               stopReason: null,
               errorReason: null,
               updatedAt: Date.now(),
@@ -838,7 +841,6 @@ export class SupervisorManager {
           this.broadcastState(recoveredSupervisor, "state_changed");
           this.deps.cycleRepo.pruneOldest(supervisorId, this.config.maxCyclesPerSession);
           this.scheduler.refresh();
-          this.pendingPauses.delete(supervisorId);
 
           return abortedCycle;
         }

packages/server/src/supervisor/target-store.atomic.test.ts

@@ -1,24 +1,33 @@
-import { mkdtempSync, rmSync } from "node:fs";
+import { existsSync, mkdtempSync, readdirSync, readFileSync, rmSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 
-const renameState = vi.hoisted(() => ({
-  callCount: 0,
-  failOnCall: 0,
+const fsState = vi.hoisted(() => ({
+  renameCallCount: 0,
+  failRenameOnCalls: [] as number[],
+  rmCallCount: 0,
+  failRmOnCalls: [] as number[],
 }));
 
 vi.mock("node:fs/promises", async () => {
   const actual = await vi.importActual<typeof import("node:fs/promises")>("node:fs/promises");
   return {
     ...actual,
     rename: vi.fn(async (from: string, to: string) => {
-      renameState.callCount += 1;
-      if (renameState.failOnCall !== 0 && renameState.callCount === renameState.failOnCall) {
+      fsState.renameCallCount += 1;
+      if (fsState.failRenameOnCalls.includes(fsState.renameCallCount)) {
         throw new Error("promote failed");
       }
       return actual.rename(from, to);
     }),
+    rm: vi.fn(async (...args: Parameters<typeof actual.rm>) => {
+      fsState.rmCallCount += 1;
+      if (fsState.failRmOnCalls.includes(fsState.rmCallCount)) {
+        throw new Error("cleanup failed");
+      }
+      return actual.rm(...args);
+    }),
   };
 });
 
@@ -37,8 +46,10 @@ describe("target store atomic reset", () => {
 
   beforeEach(() => {
     workspacePath = mkdtempSync(join(tmpdir(), "supervisor-target-store-atomic-"));
-    renameState.callCount = 0;
-    renameState.failOnCall = 0;
+    fsState.renameCallCount = 0;
+    fsState.failRenameOnCalls = [];
+    fsState.rmCallCount = 0;
+    fsState.failRmOnCalls = [];
   });
 
   afterEach(() => {
@@ -77,7 +88,7 @@ describe("target store atomic reset", () => {
       attemptCount: 1,
     });
 
-    renameState.failOnCall = 2;
+    fsState.failRenameOnCalls = [2];
 
     await expect(
       resetTargetFiles(workspacePath, {
@@ -110,4 +121,80 @@ describe("target store atomic reset", () => {
       },
     ]);
   });
+
+  it("keeps the promoted target live when backup cleanup fails", async () => {
+    await createTargetFiles(workspacePath, {
+      targetId: "tgt-1",
+      sessionId: "sess-1",
+      workspaceId: "ws-1",
+      objective: "Old objective",
+      createdAt: 1,
+    });
+
+    fsState.failRmOnCalls = [1];
+
+    await expect(
+      resetTargetFiles(workspacePath, {
+        targetId: "tgt-1",
+        sessionId: "sess-1",
+        workspaceId: "ws-1",
+        objective: "New objective",
+        createdAt: 3,
+      })
+    ).resolves.toBeUndefined();
+
+    const meta = await readTargetMeta(workspacePath, "tgt-1");
+    const memory = await loadTargetMemory(workspacePath, "tgt-1");
+    const cycles = await readTargetCycleRecords(workspacePath, "tgt-1");
+
+    expect(meta.objective).toBe("New objective");
+    expect(memory).toMatchObject({
+      targetId: "tgt-1",
+      planGenerated: false,
+      stalledCount: 0,
+      updatedAt: 3,
+    });
+    expect(cycles).toEqual([]);
+  });
+
+  it("preserves the backup target when both promotion and restore fail", async () => {
+    await createTargetFiles(workspacePath, {
+      targetId: "tgt-1",
+      sessionId: "sess-1",
+      workspaceId: "ws-1",
+      objective: "Old objective",
+      createdAt: 1,
+    });
+
+    fsState.failRenameOnCalls = [2, 3];
+
+    await expect(
+      resetTargetFiles(workspacePath, {
+        targetId: "tgt-1",
+        sessionId: "sess-1",
+        workspaceId: "ws-1",
+        objective: "New objective",
+        createdAt: 3,
+      })
+    ).rejects.toThrow("promote failed");
+
+    const targetsRoot = join(workspacePath, ".coder-studio", "supervisor", "targets");
+    const entries = readdirSync(targetsRoot);
+    const backupEntry = entries.find((entry) => entry.startsWith("tgt-1.backup-"));
+    const stagingEntry = entries.find((entry) => entry.startsWith("tgt-1.reset-"));
+
+    expect(existsSync(join(targetsRoot, "tgt-1"))).toBe(false);
+    expect(backupEntry).toBeDefined();
+    expect(stagingEntry).toBeDefined();
+
+    const backupMeta = JSON.parse(
+      readFileSync(join(targetsRoot, backupEntry!, "meta.json"), "utf-8")
+    ) as { objective: string };
+    const stagedMeta = JSON.parse(
+      readFileSync(join(targetsRoot, stagingEntry!, "meta.json"), "utf-8")
+    ) as { objective: string };
+
+    expect(backupMeta.objective).toBe("Old objective");
+    expect(stagedMeta.objective).toBe("New objective");
+  });
 });

packages/server/src/supervisor/target-store.ts

@@ -51,6 +51,19 @@ function hasCode(error: unknown, code: string): boolean {
   );
 }
 
+function errorMessage(error: unknown, fallback: string): string {
+  if (error instanceof Error) {
+    return error.message;
+  }
+  if (error && typeof error === "object" && "message" in error) {
+    const value = (error as { message?: unknown }).message;
+    if (typeof value === "string") {
+      return value;
+    }
+  }
+  return fallback;
+}
+
 async function writeJsonIfMissing(path: string, value: unknown): Promise<void> {
   try {
     await writeFile(path, JSON.stringify(value, null, 2) + "\n", {
@@ -154,15 +167,16 @@ export async function resetTargetFiles(
   await mkdir(parentDir, { recursive: true });
   const stagingDir = await mkdtemp(join(parentDir, `${input.targetId}.reset-`));
 
-  let movedExisting = false;
+  let backupCreated = false;
   let promoted = false;
+  let restored = false;
 
   try {
     await writeResetTargetFiles(stagingDir, input);
 
     try {
       await rename(dir, backupDir);
-      movedExisting = true;
+      backupCreated = true;
     } catch (error) {
       if (!hasCode(error, "ENOENT")) {
         throw error;
@@ -172,27 +186,36 @@ export async function resetTargetFiles(
     try {
       await rename(stagingDir, dir);
       promoted = true;
-    } catch (error) {
-      if (movedExisting) {
-        await rename(backupDir, dir);
-        movedExisting = false;
+    } catch (promoteError) {
+      if (backupCreated) {
+        try {
+          await rename(backupDir, dir);
+          backupCreated = false;
+          restored = true;
+        } catch (restoreError) {
+          throw new Error(
+            `Failed to promote target reset (${errorMessage(
+              promoteError,
+              "unknown promote error"
+            )}); restore also failed (${errorMessage(restoreError, "unknown restore error")})`
+          );
+        }
       }
-      throw error;
-    }
-
-    if (movedExisting) {
-      await rm(backupDir, { recursive: true, force: true });
-      movedExisting = false;
+      throw promoteError;
     }
   } catch (error) {
-    if (!promoted) {
+    if (restored || !backupCreated) {
       await rm(stagingDir, { recursive: true, force: true }).catch(() => {});
     }
-    if (movedExisting) {
-      await rm(backupDir, { recursive: true, force: true }).catch(() => {});
-    }
     throw error;
   }
+
+  if (backupCreated) {
+    await rm(backupDir, { recursive: true, force: true }).catch(() => {});
+  }
+  if (!promoted) {
+    await rm(stagingDir, { recursive: true, force: true }).catch(() => {});
+  }
 }
 
 export async function readTargetMeta(