fix: address follow-up stage 2 review feedback (#95)

packages/core/src/__tests__/lifecycle-manager.test.ts

@@ -702,6 +702,25 @@ describe("check (single session)", () => {
     expect(lm.getStates().get("app-1")).toBe("merged");
   });
 
+  it("treats closed PRs as done when the canonical lifecycle marks them complete", async () => {
+    const mockSCM = createMockSCM({ getPRState: vi.fn().mockResolvedValue("closed") });
+    const registry = createMockRegistry({
+      runtime: plugins.runtime,
+      agent: plugins.agent,
+      scm: mockSCM,
+    });
+
+    const lm = setupCheck("app-1", {
+      session: makeSession({ status: "pr_open", pr: makePR() }),
+      registry,
+    });
+
+    await lm.check("app-1");
+    expect(lm.getStates().get("app-1")).toBe("done");
+    const meta = readMetadataRaw(env.sessionsDir, "app-1");
+    expect(meta?.["status"]).toBe("done");
+  });
+
   it("detects mergeable when approved + CI green", async () => {
     const mockSCM = createMockSCM({
       getReviewDecision: vi.fn().mockResolvedValue("approved"),

packages/core/src/__tests__/recovery-validator.test.ts

@@ -202,7 +202,7 @@ describe("recovery validator", () => {
       project: config.projects.app,
       sessionsDir: getSessionsDir(config.configPath, projectPath),
       rawMetadata: {
-        worktree: projectPath,
+        worktree: join(rootDir, "missing-worktree"),
         status: "working",
         runtimeHandle: JSON.stringify({ id: "rt-1", runtimeName: "tmux", data: {} }),
       },
@@ -297,7 +297,7 @@ describe("recovery validator", () => {
       project: config.projects.app,
       sessionsDir: getSessionsDir(config.configPath, projectPath),
       rawMetadata: {
-        worktree: projectPath,
+        worktree: join(rootDir, "missing-worktree"),
         status: "working",
         runtimeHandle: JSON.stringify({ id: "rt-1", runtimeName: "tmux", data: {} }),
       },
@@ -381,4 +381,99 @@ describe("recovery validator", () => {
     expect(assessment.recoveryRule).toBe("skip");
     expect(assessment.action).toBe("skip");
   });
+
+  it("respects escalatePartial=false for non-disagreement partial sessions", async () => {
+    rootDir = join(tmpdir(), `ao-recovery-validator-${randomUUID()}`);
+    mkdirSync(rootDir, { recursive: true });
+    const projectPath = join(rootDir, "project");
+    mkdirSync(projectPath, { recursive: true });
+    writeFileSync(join(rootDir, "agent-orchestrator.yaml"), "projects: {}\n", "utf-8");
+
+    const mockRuntime: Runtime = {
+      name: "tmux",
+      create: vi.fn(),
+      destroy: vi.fn(),
+      sendMessage: vi.fn(),
+      getOutput: vi.fn(),
+      isAlive: vi.fn().mockResolvedValue(true),
+    };
+    const mockWorkspace: Workspace = {
+      name: "worktree",
+      create: vi.fn(),
+      destroy: vi.fn(),
+      list: vi.fn(),
+      exists: vi.fn().mockResolvedValue(false),
+    };
+    const mockAgent: Agent = {
+      name: "mock-agent",
+      processName: "mock-agent",
+      getLaunchCommand: vi.fn(),
+      getEnvironment: vi.fn(),
+      detectActivity: vi.fn(),
+      getActivityState: vi.fn(),
+      isProcessRunning: vi.fn().mockResolvedValue(true),
+      getSessionInfo: vi.fn(),
+    };
+    const registry: PluginRegistry = {
+      register: vi.fn(),
+      get: vi.fn().mockImplementation((slot: string) => {
+        if (slot === "runtime") return mockRuntime;
+        if (slot === "workspace") return mockWorkspace;
+        if (slot === "agent") return mockAgent;
+        return null;
+      }),
+      list: vi.fn().mockReturnValue([]),
+      loadBuiltins: vi.fn().mockResolvedValue(undefined),
+      loadFromConfig: vi.fn().mockResolvedValue(undefined),
+    };
+    const config: OrchestratorConfig = {
+      configPath: join(rootDir, "agent-orchestrator.yaml"),
+      port: 3000,
+      readyThresholdMs: 300_000,
+      power: { preventIdleSleep: false },
+      defaults: {
+        runtime: "tmux",
+        agent: "mock-agent",
+        workspace: "worktree",
+        notifiers: ["desktop"],
+      },
+      projects: {
+        app: {
+          name: "app",
+          repo: "org/repo",
+          path: projectPath,
+          defaultBranch: "main",
+          sessionPrefix: "app",
+        },
+      },
+      notifiers: {},
+      notificationRouting: {
+        urgent: ["desktop"],
+        action: ["desktop"],
+        warning: [],
+        info: [],
+      },
+      reactions: {},
+    };
+    const scanned: ScannedSession = {
+      sessionId: "app-1",
+      projectId: "app",
+      project: config.projects.app,
+      sessionsDir: getSessionsDir(config.configPath, projectPath),
+      rawMetadata: {
+        worktree: join(rootDir, "missing-worktree"),
+        status: "working",
+        runtimeHandle: JSON.stringify({ id: "rt-1", runtimeName: "tmux", data: {} }),
+      },
+    };
+
+    const assessment = await validateSession(scanned, config, registry, {
+      escalatePartial: false,
+    });
+
+    expect(assessment.classification).toBe("partial");
+    expect(assessment.signalDisagreement).toBe(false);
+    expect(assessment.recoveryRule).toBe("auto");
+    expect(assessment.action).toBe("cleanup");
+  });
 });

packages/core/src/lifecycle-manager.ts

@@ -387,26 +387,6 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
     return "stale";
   }
 
-  function buildDetectingResult(
-    session: Session,
-    evidence: string,
-    fallbackStatus: SessionStatus = SESSION_STATUS.STUCK,
-  ): DeterminedStatus {
-    const attempts = parseAttemptCount(session.metadata["detectingAttempts"]) + 1;
-    if (attempts > DETECTING_MAX_ATTEMPTS) {
-      return {
-        status: fallbackStatus,
-        evidence,
-        detectingAttempts: attempts,
-      };
-    }
-    return {
-      status: SESSION_STATUS.DETECTING,
-      evidence,
-      detectingAttempts: attempts,
-    };
-  }
-
   /** Determine current status for a session by polling plugins. */
   async function determineStatus(session: Session): Promise<DeterminedStatus> {
     const project = config.projects[session.projectId];
@@ -444,7 +424,7 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
       detectingAttempts = parseAttemptCount(session.metadata["detectingAttempts"]),
     ): DeterminedStatus => {
       session.lifecycle = lifecycle;
-      session.status = deriveLegacyStatus(lifecycle, session.status);
+      session.status = status;
       return {
         status,
         evidence,
@@ -699,7 +679,7 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
             lifecycle.pr.state = "closed";
             lifecycle.pr.reason = "closed_unmerged";
             setSessionState("done", "research_complete");
-            return commit(SESSION_STATUS.KILLED, "pr_closed", 0);
+            return commit(SESSION_STATUS.DONE, "pr_closed", 0);
           }
 
           lifecycle.pr.state = "open";
@@ -753,7 +733,7 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
           lifecycle.pr.state = "closed";
           lifecycle.pr.reason = "closed_unmerged";
           setSessionState("done", "research_complete");
-          return commit(SESSION_STATUS.KILLED, "pr_closed", 0);
+          return commit(SESSION_STATUS.DONE, "pr_closed", 0);
         }
 
         lifecycle.pr.state = "open";

packages/core/src/lifecycle-state.ts

@@ -266,15 +266,7 @@ export function deriveLegacyStatus(
     case "idle":
       return lifecycle.pr.state === "merged" ? "merged" : "idle";
     case "detecting":
-      if (
-        lifecycle.session.reason === "runtime_lost" ||
-        lifecycle.session.reason === "agent_process_exited" ||
-        lifecycle.runtime.state === "missing" ||
-        lifecycle.runtime.state === "exited"
-      ) {
-        return "killed";
-      }
-      return TERMINAL_COMPATIBILITY_STATUS.has(previousStatus) ? "working" : previousStatus;
+      return "detecting";
     case "working":
       if (lifecycle.pr.reason === "ci_failing") return "ci_failed";
       if (lifecycle.pr.reason === "changes_requested") return "changes_requested";
@@ -286,15 +278,6 @@ export function deriveLegacyStatus(
   }
 }
 
-const TERMINAL_COMPATIBILITY_STATUS: ReadonlySet<SessionStatus> = new Set([
-  "killed",
-  "terminated",
-  "done",
-  "cleanup",
-  "errored",
-  "merged",
-]);
-
 export function buildLifecycleMetadataPatch(
   lifecycle: CanonicalSessionLifecycle,
   previousStatus?: SessionStatus,

packages/core/src/recovery/validator.ts

@@ -110,7 +110,12 @@ export async function validateSession(
     runtimeProbeSucceeded &&
     processProbeSucceeded &&
     ((runtimeAlive && !agentProcessRunning) || (!runtimeAlive && agentProcessRunning));
-  const recoveryRule = determineRecoveryRule(classification, signalDisagreement, metadataStatus);
+  const recoveryRule = determineRecoveryRule(
+    classification,
+    signalDisagreement,
+    metadataStatus,
+    recoveryConfig,
+  );
   const action = determineAction(classification, metadataStatus, recoveryConfig, recoveryRule);
 
   return {
@@ -186,11 +191,15 @@ function determineRecoveryRule(
   classification: RecoveryClassification,
   signalDisagreement: boolean,
   metadataStatus: SessionStatus,
+  recoveryConfig: RecoveryConfig = DEFAULT_RECOVERY_CONFIG,
 ): "auto" | "human" | "skip" {
   if (classification === "unrecoverable") return "skip";
-  if (metadataStatus === "detecting" || signalDisagreement || classification === "partial") {
+  if (metadataStatus === "detecting" || signalDisagreement) {
     return "human";
   }
+  if (classification === "partial") {
+    return recoveryConfig.escalatePartial ? "human" : "auto";
+  }
   if (classification === "live" || classification === "dead") {
     return "auto";
   }