From a45eb322bb54b5ccb612ad009910a273de17a79d Mon Sep 17 00:00:00 2001
From: harshitsinghbhandari <24b4506@iitb.ac.in>
Date: Fri, 17 Apr 2026 10:44:35 +0530
Subject: [PATCH 1/3] fix: decouple PR truth from worker session policy
---
.changeset/stage4-pr-policy.md | 6 ++
.../src/__tests__/lifecycle-manager.test.ts | 55 +++++++++++++++++--
.../src/__tests__/plugin-integration.test.ts | 4 +-
.../session-manager/lifecycle.test.ts | 14 ++---
.../__tests__/session-manager/spawn.test.ts | 4 +-
packages/core/src/lifecycle-manager.ts | 54 ++++++++++++++----
packages/core/src/lifecycle-state.ts | 22 +++++---
packages/core/src/session-manager.ts | 2 +-
packages/core/src/types.ts | 1 +
packages/web/src/components/SessionCard.tsx | 19 +++++++
.../dashboard-page-data.fast-path.test.ts | 10 ++--
.../web/src/lib/__tests__/serialize.test.ts | 21 +++++++
packages/web/src/lib/dashboard-page-data.ts | 12 +---
packages/web/src/lib/serialize.ts | 18 +++++-
packages/web/src/lib/services.ts | 4 +-
15 files changed, 191 insertions(+), 55 deletions(-)
create mode 100644 .changeset/stage4-pr-policy.md
diff --git a/.changeset/stage4-pr-policy.md b/.changeset/stage4-pr-policy.md
new file mode 100644
index 000000000..3887a4016
--- /dev/null
+++ b/.changeset/stage4-pr-policy.md
@@ -0,0 +1,6 @@
+---
+"@aoagents/ao-core": patch
+"@aoagents/ao-web": patch
+---
+
+Decouple canonical session state from PR state so workers stay idle while waiting on reviews or merged/closed PR decisions, stop cleanup from auto-killing merged PR sessions, and make the dashboard/rendered labels follow canonical PR truth instead of inferring it from legacy lifecycle aliases.
diff --git a/packages/core/src/__tests__/lifecycle-manager.test.ts b/packages/core/src/__tests__/lifecycle-manager.test.ts
index 006553729..0ee07143b 100644
--- a/packages/core/src/__tests__/lifecycle-manager.test.ts
+++ b/packages/core/src/__tests__/lifecycle-manager.test.ts
@@ -375,7 +375,7 @@ describe("check (single session)", () => {
runtime: plugins.runtime,
agent: plugins.agent,
});
- vi.mocked(registryWithoutAgent.get).mockImplementation((slot: string, name?: string) => {
+ vi.mocked(registryWithoutAgent.get).mockImplementation((slot: string, _name?: string) => {
if (slot === "runtime") return plugins.runtime;
if (slot === "agent") return null;
return null;
@@ -584,6 +584,38 @@ describe("check (single session)", () => {
expect(lm.getStates().get("app-1")).toBe("ci_failed");
});
+ it("keeps canonical session state idle while waiting on external review", async () => {
+ const mockSCM = createMockSCM({ getReviewDecision: vi.fn().mockResolvedValue("pending") });
+ const registry = createMockRegistry({
+ runtime: plugins.runtime,
+ agent: plugins.agent,
+ scm: mockSCM,
+ });
+ const session = makeSession({ status: "pr_open", pr: makePR() });
+ vi.mocked(mockSessionManager.get).mockResolvedValue(session);
+
+ writeMetadata(env.sessionsDir, "app-1", {
+ worktree: "/tmp",
+ branch: session.branch ?? "main",
+ status: session.status,
+ project: "my-app",
+ pr: session.pr?.url,
+ runtimeHandle: session.runtimeHandle ? JSON.stringify(session.runtimeHandle) : undefined,
+ });
+
+ const lm = createLifecycleManager({
+ config,
+ registry,
+ sessionManager: mockSessionManager,
+ });
+
+ await lm.check("app-1");
+
+ expect(lm.getStates().get("app-1")).toBe("review_pending");
+ expect(session.lifecycle.session.state).toBe("idle");
+ expect(session.lifecycle.session.reason).toBe("awaiting_external_review");
+ });
+
it("skips PR auto-detection when metadata disables it", async () => {
const mockSCM = createMockSCM({ detectPR: vi.fn().mockResolvedValue(makePR()) });
const registry = createMockRegistry({
@@ -702,23 +734,36 @@ 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 () => {
+ it("keeps closed PR sessions idle and emits a PR-closed notification", async () => {
const mockSCM = createMockSCM({ getPRState: vi.fn().mockResolvedValue("closed") });
+ const notifier = createMockNotifier();
const registry = createMockRegistry({
runtime: plugins.runtime,
agent: plugins.agent,
scm: mockSCM,
+ notifier,
});
+ const session = makeSession({ status: "pr_open", pr: makePR() });
const lm = setupCheck("app-1", {
- session: makeSession({ status: "pr_open", pr: makePR() }),
+ session,
registry,
+ configOverride: {
+ ...config,
+ notificationRouting: {
+ ...config.notificationRouting,
+ info: ["desktop"],
+ },
+ },
});
await lm.check("app-1");
- expect(lm.getStates().get("app-1")).toBe("done");
+ expect(lm.getStates().get("app-1")).toBe("idle");
const meta = readMetadataRaw(env.sessionsDir, "app-1");
- expect(meta?.["status"]).toBe("done");
+ expect(meta?.["status"]).toBe("idle");
+ expect(meta?.["statePayload"]).toContain('"state":"closed"');
+ expect(meta?.["statePayload"]).toContain('"reason":"pr_closed_waiting_decision"');
+ expect(notifier.notify).toHaveBeenCalledWith(expect.objectContaining({ type: "pr.closed" }));
});
it("detects mergeable when approved + CI green", async () => {
diff --git a/packages/core/src/__tests__/plugin-integration.test.ts b/packages/core/src/__tests__/plugin-integration.test.ts
index 6b64aef1c..663575a4f 100644
--- a/packages/core/src/__tests__/plugin-integration.test.ts
+++ b/packages/core/src/__tests__/plugin-integration.test.ts
@@ -392,7 +392,7 @@ describe("plugin integration", () => {
// -------------------------------------------------------------------------
describe("SessionManager + SCM", () => {
- it("cleanup() calls scm-github getPRState() and kills merged PR sessions", async () => {
+ it("cleanup() calls scm-github getPRState() but keeps merged PR sessions alive", async () => {
const registry = createTestRegistry();
const sm = createSessionManager({ config, registry });
@@ -412,7 +412,7 @@ describe("plugin integration", () => {
const result = await sm.cleanup("my-app");
- expect(result.killed).toContain("app-1");
+ expect(result.skipped).toContain("app-1");
// Verify gh CLI was called for PR state check
expect(ghMock).toHaveBeenCalledWith(
"gh",
diff --git a/packages/core/src/__tests__/session-manager/lifecycle.test.ts b/packages/core/src/__tests__/session-manager/lifecycle.test.ts
index 7a24277db..6a656cf5f 100644
--- a/packages/core/src/__tests__/session-manager/lifecycle.test.ts
+++ b/packages/core/src/__tests__/session-manager/lifecycle.test.ts
@@ -229,11 +229,11 @@ describe("kill", () => {
});
describe("cleanup", () => {
- it("kills sessions with merged PRs", async () => {
+ it("kills sessions with closed PRs", async () => {
const mockSCM: SCM = {
name: "mock-scm",
detectPR: vi.fn(),
- getPRState: vi.fn().mockResolvedValue("merged"),
+ getPRState: vi.fn().mockResolvedValue("closed"),
mergePR: vi.fn(),
closePR: vi.fn(),
getCIChecks: vi.fn(),
@@ -272,7 +272,7 @@ describe("cleanup", () => {
expect(result.skipped).toHaveLength(0);
});
- it("deletes mapped OpenCode session during cleanup", async () => {
+ it("deletes mapped OpenCode session during cleanup for closed PRs", async () => {
const deleteLogPath = join(tmpDir, "opencode-delete.log");
const mockBin = installMockOpencode(tmpDir, "[]", deleteLogPath);
process.env.PATH = `${mockBin}:${originalPath ?? ""}`;
@@ -280,7 +280,7 @@ describe("cleanup", () => {
const mockSCM: SCM = {
name: "mock-scm",
detectPR: vi.fn(),
- getPRState: vi.fn().mockResolvedValue("merged"),
+ getPRState: vi.fn().mockResolvedValue("closed"),
mergePR: vi.fn(),
closePR: vi.fn(),
getCIChecks: vi.fn(),
@@ -322,14 +322,14 @@ describe("cleanup", () => {
expect(deleteLog).toContain("session delete ses_cleanup");
});
- it("treats missing mapped OpenCode session as already cleaned", async () => {
+ it("treats missing mapped OpenCode session as already cleaned for closed PRs", async () => {
const mockBin = installMockOpencodeWithNotFoundDelete(tmpDir, "[]");
process.env.PATH = `${mockBin}:${originalPath ?? ""}`;
const mockSCM: SCM = {
name: "mock-scm",
detectPR: vi.fn(),
- getPRState: vi.fn().mockResolvedValue("merged"),
+ getPRState: vi.fn().mockResolvedValue("closed"),
mergePR: vi.fn(),
closePR: vi.fn(),
getCIChecks: vi.fn(),
@@ -392,7 +392,7 @@ describe("cleanup", () => {
expect(result.killed).toContain("app-6");
const deleteLog = readFileSync(deleteLogPath, "utf-8");
expect(deleteLog).toContain("session delete ses_archived");
- });
+ }, 15_000);
it("does not skip archived cleanup for matching session IDs in other projects", async () => {
const deleteLogPath = join(tmpDir, "opencode-delete-archived-cross-project.log");
diff --git a/packages/core/src/__tests__/session-manager/spawn.test.ts b/packages/core/src/__tests__/session-manager/spawn.test.ts
index 222039714..968ce9149 100644
--- a/packages/core/src/__tests__/session-manager/spawn.test.ts
+++ b/packages/core/src/__tests__/session-manager/spawn.test.ts
@@ -576,7 +576,7 @@ describe("spawn", () => {
const metadata = readMetadataRaw(sessionsDir, session.id);
expect(metadata?.["opencodeSessionId"]).toBeUndefined();
- });
+ }, 15_000);
it("throws for unknown project", async () => {
const sm = createSessionManager({ config, registry: mockRegistry });
@@ -1865,7 +1865,7 @@ describe("spawn", () => {
}),
}),
);
- });
+ }, 15_000);
it("throws for unknown project", async () => {
const sm = createSessionManager({ config, registry: mockRegistry });
diff --git a/packages/core/src/lifecycle-manager.ts b/packages/core/src/lifecycle-manager.ts
index 3d567e41b..a5f74b1a5 100644
--- a/packages/core/src/lifecycle-manager.ts
+++ b/packages/core/src/lifecycle-manager.ts
@@ -142,6 +142,19 @@ function statusToEventType(_from: SessionStatus | undefined, to: SessionStatus):
}
}
+function prStateToEventType(
+ from: Session["lifecycle"]["pr"]["state"],
+ to: Session["lifecycle"]["pr"]["state"],
+): EventType | null {
+ if (from === to) return null;
+ switch (to) {
+ case "closed":
+ return "pr.closed";
+ default:
+ return null;
+ }
+}
+
/** Map event type to reaction config key. */
function eventToReactionKey(eventType: EventType): string | null {
switch (eventType) {
@@ -683,8 +696,8 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
if (cachedData.state === PR_STATE.CLOSED) {
lifecycle.pr.state = "closed";
lifecycle.pr.reason = "closed_unmerged";
- setSessionState("done", "research_complete");
- return commit(SESSION_STATUS.DONE, "pr_closed", 0);
+ setSessionState("idle", "pr_closed_waiting_decision");
+ return commit(SESSION_STATUS.IDLE, "pr_closed", 0);
}
lifecycle.pr.state = "open";
@@ -701,18 +714,18 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
if (cachedData.reviewDecision === "approved" || cachedData.reviewDecision === "none") {
if (cachedData.mergeable) {
lifecycle.pr.reason = "merge_ready";
- setSessionState("working", "awaiting_external_review");
+ setSessionState("idle", "awaiting_external_review");
return commit(SESSION_STATUS.MERGEABLE, "merge_ready", 0);
}
if (cachedData.reviewDecision === "approved") {
lifecycle.pr.reason = "approved";
- setSessionState("working", "awaiting_external_review");
+ setSessionState("idle", "awaiting_external_review");
return commit(SESSION_STATUS.APPROVED, "review_approved", 0);
}
}
if (cachedData.reviewDecision === "pending") {
lifecycle.pr.reason = "review_pending";
- setSessionState("working", "awaiting_external_review");
+ setSessionState("idle", "awaiting_external_review");
return commit(SESSION_STATUS.REVIEW_PENDING, "review_pending", 0);
}
@@ -723,7 +736,7 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
}
lifecycle.pr.reason = "in_progress";
- setSessionState("working", "pr_created");
+ setSessionState("idle", "pr_created");
return commit(SESSION_STATUS.PR_OPEN, "pr_open", 0);
}
@@ -737,8 +750,8 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
if (prState === PR_STATE.CLOSED) {
lifecycle.pr.state = "closed";
lifecycle.pr.reason = "closed_unmerged";
- setSessionState("done", "research_complete");
- return commit(SESSION_STATUS.DONE, "pr_closed", 0);
+ setSessionState("idle", "pr_closed_waiting_decision");
+ return commit(SESSION_STATUS.IDLE, "pr_closed", 0);
}
lifecycle.pr.state = "open";
@@ -759,18 +772,18 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
const mergeReady = await scm.getMergeability(session.pr);
if (mergeReady.mergeable) {
lifecycle.pr.reason = "merge_ready";
- setSessionState("working", "awaiting_external_review");
+ setSessionState("idle", "awaiting_external_review");
return commit(SESSION_STATUS.MERGEABLE, "merge_ready", 0);
}
if (reviewDecision === "approved") {
lifecycle.pr.reason = "approved";
- setSessionState("working", "awaiting_external_review");
+ setSessionState("idle", "awaiting_external_review");
return commit(SESSION_STATUS.APPROVED, "review_approved", 0);
}
}
if (reviewDecision === "pending") {
lifecycle.pr.reason = "review_pending";
- setSessionState("working", "awaiting_external_review");
+ setSessionState("idle", "awaiting_external_review");
return commit(SESSION_STATUS.REVIEW_PENDING, "review_pending", 0);
}
@@ -781,7 +794,7 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
}
lifecycle.pr.reason = "in_progress";
- setSessionState("working", "pr_created");
+ setSessionState("idle", "pr_created");
return commit(SESSION_STATUS.PR_OPEN, "pr_open", 0);
} catch {
// Keep current status on SCM failure.
@@ -1463,6 +1476,7 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
const tracked = states.get(session.id);
const oldStatus =
tracked ?? ((session.metadata?.["status"] as SessionStatus | undefined) || session.status);
+ const previousPRState = session.lifecycle.pr.state;
const assessment = await determineStatus(session);
const newStatus = assessment.status;
const lifecycleChanged = session.metadata["statePayload"] !== JSON.stringify(session.lifecycle);
@@ -1572,6 +1586,22 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
}
}
+ const prEventType = prStateToEventType(previousPRState, session.lifecycle.pr.state);
+ if (prEventType) {
+ const prEvent = createEvent(prEventType, {
+ sessionId: session.id,
+ projectId: session.projectId,
+ message: `${session.id}: PR ${previousPRState} → ${session.lifecycle.pr.state}`,
+ data: {
+ oldPRState: previousPRState,
+ newPRState: session.lifecycle.pr.state,
+ prNumber: session.lifecycle.pr.number,
+ prUrl: session.lifecycle.pr.url,
+ },
+ });
+ await notifyHuman(prEvent, inferPriority(prEventType));
+ }
+
// Pin first quality summary for title stability
if (
session.agentInfo?.summary &&
diff --git a/packages/core/src/lifecycle-state.ts b/packages/core/src/lifecycle-state.ts
index a347d768b..d1e0719f3 100644
--- a/packages/core/src/lifecycle-state.ts
+++ b/packages/core/src/lifecycle-state.ts
@@ -250,8 +250,20 @@ export function parseCanonicalLifecycle(
export function deriveLegacyStatus(
lifecycle: CanonicalSessionLifecycle,
- previousStatus: SessionStatus = "working",
+ _previousStatus: SessionStatus = "working",
): SessionStatus {
+ if (lifecycle.pr.state === "merged") {
+ return "merged";
+ }
+ if (lifecycle.pr.state === "open") {
+ if (lifecycle.pr.reason === "ci_failing") return "ci_failed";
+ if (lifecycle.pr.reason === "changes_requested") return "changes_requested";
+ if (lifecycle.pr.reason === "review_pending") return "review_pending";
+ if (lifecycle.pr.reason === "approved") return "approved";
+ if (lifecycle.pr.reason === "merge_ready") return "mergeable";
+ return "pr_open";
+ }
+
switch (lifecycle.session.state) {
case "not_started":
return "spawning";
@@ -264,16 +276,10 @@ export function deriveLegacyStatus(
case "terminated":
return "terminated";
case "idle":
- return lifecycle.pr.state === "merged" ? "merged" : "idle";
+ return "idle";
case "detecting":
return "detecting";
case "working":
- if (lifecycle.pr.reason === "ci_failing") return "ci_failed";
- if (lifecycle.pr.reason === "changes_requested") return "changes_requested";
- if (lifecycle.pr.reason === "review_pending") return "review_pending";
- if (lifecycle.pr.reason === "approved") return "approved";
- if (lifecycle.pr.reason === "merge_ready") return "mergeable";
- if (lifecycle.pr.state === "open") return "pr_open";
return "working";
}
}
diff --git a/packages/core/src/session-manager.ts b/packages/core/src/session-manager.ts
index bbfe5ab6e..fe158ebd7 100644
--- a/packages/core/src/session-manager.ts
+++ b/packages/core/src/session-manager.ts
@@ -1876,7 +1876,7 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM
if (session.pr && plugins.scm) {
try {
const prState = await plugins.scm.getPRState(session.pr);
- if (prState === PR_STATE.MERGED || prState === PR_STATE.CLOSED) {
+ if (prState === PR_STATE.CLOSED) {
shouldKill = true;
}
} catch {
diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts
index fd9b23bc1..bab548771 100644
--- a/packages/core/src/types.ts
+++ b/packages/core/src/types.ts
@@ -41,6 +41,7 @@ export type CanonicalSessionReason =
| "agent_acknowledged"
| "task_in_progress"
| "pr_created"
+ | "pr_closed_waiting_decision"
| "fixing_ci"
| "resolving_review_comments"
| "awaiting_user_input"
diff --git a/packages/web/src/components/SessionCard.tsx b/packages/web/src/components/SessionCard.tsx
index 04181e43b..e32740f8f 100644
--- a/packages/web/src/components/SessionCard.tsx
+++ b/packages/web/src/components/SessionCard.tsx
@@ -53,6 +53,25 @@ function getDoneStatusInfo(session: DashboardSession): {
};
}
+ if (prState === "closed") {
+ return {
+ label: "closed",
+ pillClass: "done-status-pill--exited",
+ icon: (
+
+ ),
+ };
+ }
+
if (status === "killed" || status === "terminated") {
return {
label: status,
diff --git a/packages/web/src/lib/__tests__/dashboard-page-data.fast-path.test.ts b/packages/web/src/lib/__tests__/dashboard-page-data.fast-path.test.ts
index 7ceb794d8..b212213f6 100644
--- a/packages/web/src/lib/__tests__/dashboard-page-data.fast-path.test.ts
+++ b/packages/web/src/lib/__tests__/dashboard-page-data.fast-path.test.ts
@@ -60,15 +60,15 @@ describe("getDashboardPageData fast path", () => {
hoisted.enrichSessionsMetadataFastMock.mockResolvedValue(undefined);
});
- it("runs fast enrichment, uses cache-only PR hydration, and infers merged/closed state for terminal cache misses even without SCM", async () => {
+ it("runs fast enrichment, uses cache-only PR hydration, and preserves canonical PR state on cache misses even without SCM", async () => {
const noPrCore = { id: "session-no-pr", status: "working", pr: null };
- const closedCore = { id: "session-closed", status: "killed", pr: { number: 2 } };
- const mergedCore = { id: "session-merged", status: "merged", pr: { number: 3 } };
+ const closedCore = { id: "session-closed", status: "idle", pr: { number: 2 } };
+ const mergedCore = { id: "session-merged", status: "idle", pr: { number: 3 } };
const allSessions = [noPrCore, closedCore, mergedCore];
const dashboardNoPr = { id: "session-no-pr", pr: null };
- const dashboardClosed = { id: "session-closed", pr: { state: "open", enriched: false } };
- const dashboardMerged = { id: "session-merged", pr: { state: "open", enriched: false } };
+ const dashboardClosed = { id: "session-closed", pr: { state: "closed", enriched: false } };
+ const dashboardMerged = { id: "session-merged", pr: { state: "merged", enriched: false } };
hoisted.getServicesMock.mockResolvedValue({
config: { projects: { docs: { id: "docs" } } },
diff --git a/packages/web/src/lib/__tests__/serialize.test.ts b/packages/web/src/lib/__tests__/serialize.test.ts
index 749734582..a2dfa7d47 100644
--- a/packages/web/src/lib/__tests__/serialize.test.ts
+++ b/packages/web/src/lib/__tests__/serialize.test.ts
@@ -149,6 +149,27 @@ describe("sessionToDashboard", () => {
});
});
+ it("should seed dashboard PR state from canonical lifecycle truth", () => {
+ const lifecycle = createInitialCanonicalLifecycle("worker", new Date("2025-01-01T00:00:00Z"));
+ lifecycle.session.state = "idle";
+ lifecycle.session.reason = "merged_waiting_decision";
+ lifecycle.session.startedAt = lifecycle.session.lastTransitionAt;
+ lifecycle.pr.state = "merged";
+ lifecycle.pr.reason = "merged";
+ lifecycle.runtime.state = "alive";
+ lifecycle.runtime.reason = "process_running";
+
+ const coreSession = createCoreSession({
+ status: "idle",
+ lifecycle,
+ pr: createPRInfo(),
+ });
+
+ const dashboard = sessionToDashboard(coreSession);
+
+ expect(dashboard.pr?.state).toBe("merged");
+ });
+
it("should use agentInfo summary with summaryIsFallback false", () => {
const coreSession = createCoreSession({
agentInfo: {
diff --git a/packages/web/src/lib/dashboard-page-data.ts b/packages/web/src/lib/dashboard-page-data.ts
index efb6e2419..3c5a18d7b 100644
--- a/packages/web/src/lib/dashboard-page-data.ts
+++ b/packages/web/src/lib/dashboard-page-data.ts
@@ -1,7 +1,7 @@
import "server-only";
import { cache } from "react";
-import { TERMINAL_STATUSES, type DashboardSession, type DashboardOrchestratorLink } from "@/lib/types";
+import { type DashboardSession, type DashboardOrchestratorLink } from "@/lib/types";
import { getServices, getSCM } from "@/lib/services";
import {
sessionToDashboard,
@@ -84,16 +84,6 @@ export const getDashboardPageData = cache(async function getDashboardPageData(pr
await enrichSessionPR(pageData.sessions[i], scm, core.pr, { cacheOnly: true });
}
- // For cache-miss PRs, infer terminal PR state from lifecycle status
- // to avoid showing merged/closed PRs as "open" until client refresh.
- const sessionPR = pageData.sessions[i].pr;
- if (sessionPR && !sessionPR.enriched && TERMINAL_STATUSES.has(core.status)) {
- if (core.status === "merged") {
- sessionPR.state = "merged";
- } else if (core.status === "killed") {
- sessionPR.state = "closed";
- }
- }
}
} catch {
pageData.sessions = [];
diff --git a/packages/web/src/lib/serialize.ts b/packages/web/src/lib/serialize.ts
index e6d569f37..4bbdc49da 100644
--- a/packages/web/src/lib/serialize.ts
+++ b/packages/web/src/lib/serialize.ts
@@ -75,7 +75,12 @@ export function sessionToDashboard(session: Session): DashboardSession {
summaryIsFallback: agentSummary ? (session.agentInfo?.summaryIsFallback ?? false) : false,
createdAt: session.createdAt.toISOString(),
lastActivityAt: session.lastActivityAt.toISOString(),
- pr: session.pr ? basicPRToDashboard(session.pr) : null,
+ pr: session.pr
+ ? {
+ ...basicPRToDashboard(session.pr),
+ state: normalizeDashboardPRState(session.lifecycle.pr.state),
+ }
+ : null,
metadata: session.metadata,
};
}
@@ -137,6 +142,17 @@ function basicPRToDashboard(pr: PRInfo): DashboardPR {
};
}
+function normalizeDashboardPRState(state: Session["lifecycle"]["pr"]["state"]): DashboardPR["state"] {
+ switch (state) {
+ case "merged":
+ return "merged";
+ case "closed":
+ return "closed";
+ default:
+ return "open";
+ }
+}
+
/**
* Enrich a DashboardSession's PR with live data from the SCM plugin.
* Uses cache to reduce API calls and handles rate limit errors gracefully.
diff --git a/packages/web/src/lib/services.ts b/packages/web/src/lib/services.ts
index cccae0125..bdd5541a8 100644
--- a/packages/web/src/lib/services.ts
+++ b/packages/web/src/lib/services.ts
@@ -129,7 +129,9 @@ async function labelIssuesForVerification(
): Promise {
const mergedSessions = sessions.filter(
(s) =>
- s.status === "merged" && s.issueId && !processedIssues.has(`${s.projectId}:${s.issueId}`),
+ s.lifecycle.pr.state === "merged" &&
+ s.issueId &&
+ !processedIssues.has(`${s.projectId}:${s.issueId}`),
);
for (const session of mergedSessions) {
From 7ccbe5b260e61f82901ac7adb759cf0f5229d306 Mon Sep 17 00:00:00 2001
From: harshitsinghbhandari <24b4506@iitb.ac.in>
Date: Fri, 17 Apr 2026 11:02:06 +0530
Subject: [PATCH 2/3] fix: preserve session truth over PR aliases (#118)
---
.../src/__tests__/lifecycle-manager.test.ts | 41 ++++++++++++++++
.../src/__tests__/lifecycle-state.test.ts | 49 +++++++++++++++++++
packages/core/src/config.ts | 7 +++
packages/core/src/lifecycle-manager.ts | 41 +++++++++++-----
packages/core/src/lifecycle-state.ts | 29 ++++++-----
5 files changed, 143 insertions(+), 24 deletions(-)
create mode 100644 packages/core/src/__tests__/lifecycle-state.test.ts
diff --git a/packages/core/src/__tests__/lifecycle-manager.test.ts b/packages/core/src/__tests__/lifecycle-manager.test.ts
index 0ee07143b..a5ded0160 100644
--- a/packages/core/src/__tests__/lifecycle-manager.test.ts
+++ b/packages/core/src/__tests__/lifecycle-manager.test.ts
@@ -766,6 +766,47 @@ describe("check (single session)", () => {
expect(notifier.notify).toHaveBeenCalledWith(expect.objectContaining({ type: "pr.closed" }));
});
+ it("routes closed PR transitions through the pr-closed reaction key", async () => {
+ const notifier = createMockNotifier();
+ const mockSCM = createMockSCM({ getPRState: vi.fn().mockResolvedValue("closed") });
+ const registry = createMockRegistry({
+ runtime: plugins.runtime,
+ agent: plugins.agent,
+ scm: mockSCM,
+ notifier,
+ });
+
+ const session = makeSession({ status: "pr_open", pr: makePR() });
+ const lm = setupCheck("app-1", {
+ session,
+ registry,
+ configOverride: {
+ ...config,
+ reactions: {
+ ...config.reactions,
+ "pr-closed": {
+ auto: true,
+ action: "notify",
+ priority: "action",
+ },
+ },
+ notificationRouting: {
+ ...config.notificationRouting,
+ action: ["desktop"],
+ },
+ },
+ });
+
+ await lm.check("app-1");
+
+ expect(notifier.notify).toHaveBeenCalledWith(
+ expect.objectContaining({
+ type: "reaction.triggered",
+ data: expect.objectContaining({ reactionKey: "pr-closed" }),
+ }),
+ );
+ });
+
it("detects mergeable when approved + CI green", async () => {
const mockSCM = createMockSCM({
getReviewDecision: vi.fn().mockResolvedValue("approved"),
diff --git a/packages/core/src/__tests__/lifecycle-state.test.ts b/packages/core/src/__tests__/lifecycle-state.test.ts
new file mode 100644
index 000000000..32bae2f35
--- /dev/null
+++ b/packages/core/src/__tests__/lifecycle-state.test.ts
@@ -0,0 +1,49 @@
+import { describe, expect, it } from "vitest";
+import { createInitialCanonicalLifecycle, deriveLegacyStatus } from "../lifecycle-state.js";
+
+function createOpenPRLifecycle() {
+ const lifecycle = createInitialCanonicalLifecycle("worker", new Date("2025-01-01T00:00:00Z"));
+ lifecycle.session.startedAt = lifecycle.session.lastTransitionAt;
+ lifecycle.pr.state = "open";
+ lifecycle.pr.reason = "review_pending";
+ lifecycle.pr.number = 42;
+ lifecycle.pr.url = "https://github.com/org/repo/pull/42";
+ lifecycle.pr.lastObservedAt = lifecycle.session.lastTransitionAt;
+ lifecycle.runtime.state = "alive";
+ lifecycle.runtime.reason = "process_running";
+ return lifecycle;
+}
+
+describe("deriveLegacyStatus", () => {
+ it("preserves urgent session states ahead of open PR aliases", () => {
+ const needsInput = createOpenPRLifecycle();
+ needsInput.session.state = "needs_input";
+ needsInput.session.reason = "awaiting_user_input";
+
+ const stuck = createOpenPRLifecycle();
+ stuck.session.state = "stuck";
+ stuck.session.reason = "probe_failure";
+
+ const terminated = createOpenPRLifecycle();
+ terminated.session.state = "terminated";
+ terminated.session.reason = "manually_killed";
+
+ expect(deriveLegacyStatus(needsInput)).toBe("needs_input");
+ expect(deriveLegacyStatus(stuck)).toBe("stuck");
+ expect(deriveLegacyStatus(terminated)).toBe("terminated");
+ });
+
+ it("keeps PR-oriented aliases for idle workers with open PRs", () => {
+ const reviewPending = createOpenPRLifecycle();
+ reviewPending.session.state = "idle";
+ reviewPending.session.reason = "awaiting_external_review";
+
+ const mergeReady = createOpenPRLifecycle();
+ mergeReady.session.state = "idle";
+ mergeReady.session.reason = "awaiting_external_review";
+ mergeReady.pr.reason = "merge_ready";
+
+ expect(deriveLegacyStatus(reviewPending)).toBe("review_pending");
+ expect(deriveLegacyStatus(mergeReady)).toBe("mergeable");
+ });
+});
diff --git a/packages/core/src/config.ts b/packages/core/src/config.ts
index c5b3f7eed..862355b3c 100644
--- a/packages/core/src/config.ts
+++ b/packages/core/src/config.ts
@@ -548,6 +548,13 @@ function validateProjectUniqueness(config: OrchestratorConfig): void {
/** Apply default reactions */
function applyDefaultReactions(config: OrchestratorConfig): OrchestratorConfig {
const defaults: Record = {
+ "pr-closed": {
+ auto: true,
+ action: "notify",
+ priority: "action",
+ message:
+ "A PR was closed without merging. Decide whether to learn from the closure, resume the work, or terminate the session.",
+ },
"ci-failed": {
auto: true,
action: "send-to-agent",
diff --git a/packages/core/src/lifecycle-manager.ts b/packages/core/src/lifecycle-manager.ts
index a5f74b1a5..897b215f4 100644
--- a/packages/core/src/lifecycle-manager.ts
+++ b/packages/core/src/lifecycle-manager.ts
@@ -158,6 +158,8 @@ function prStateToEventType(
/** Map event type to reaction config key. */
function eventToReactionKey(eventType: EventType): string | null {
switch (eventType) {
+ case "pr.closed":
+ return "pr-closed";
case "ci.failing":
return "ci-failed";
case "review.changes_requested":
@@ -1588,18 +1590,33 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
const prEventType = prStateToEventType(previousPRState, session.lifecycle.pr.state);
if (prEventType) {
- const prEvent = createEvent(prEventType, {
- sessionId: session.id,
- projectId: session.projectId,
- message: `${session.id}: PR ${previousPRState} → ${session.lifecycle.pr.state}`,
- data: {
- oldPRState: previousPRState,
- newPRState: session.lifecycle.pr.state,
- prNumber: session.lifecycle.pr.number,
- prUrl: session.lifecycle.pr.url,
- },
- });
- await notifyHuman(prEvent, inferPriority(prEventType));
+ let reactionHandledNotify = false;
+ const reactionKey = eventToReactionKey(prEventType);
+
+ if (reactionKey) {
+ const reactionConfig = getReactionConfigForSession(session, reactionKey);
+ if (reactionConfig && reactionConfig.action) {
+ if (reactionConfig.auto !== false || reactionConfig.action === "notify") {
+ await executeReaction(session.id, session.projectId, reactionKey, reactionConfig);
+ reactionHandledNotify = true;
+ }
+ }
+ }
+
+ if (!reactionHandledNotify) {
+ const prEvent = createEvent(prEventType, {
+ sessionId: session.id,
+ projectId: session.projectId,
+ message: `${session.id}: PR ${previousPRState} → ${session.lifecycle.pr.state}`,
+ data: {
+ oldPRState: previousPRState,
+ newPRState: session.lifecycle.pr.state,
+ prNumber: session.lifecycle.pr.number,
+ prUrl: session.lifecycle.pr.url,
+ },
+ });
+ await notifyHuman(prEvent, inferPriority(prEventType));
+ }
}
// Pin first quality summary for title stability
diff --git a/packages/core/src/lifecycle-state.ts b/packages/core/src/lifecycle-state.ts
index d1e0719f3..0b07901e3 100644
--- a/packages/core/src/lifecycle-state.ts
+++ b/packages/core/src/lifecycle-state.ts
@@ -252,6 +252,23 @@ export function deriveLegacyStatus(
lifecycle: CanonicalSessionLifecycle,
_previousStatus: SessionStatus = "working",
): SessionStatus {
+ switch (lifecycle.session.state) {
+ case "not_started":
+ return "spawning";
+ case "needs_input":
+ return "needs_input";
+ case "stuck":
+ return "stuck";
+ case "done":
+ return "done";
+ case "terminated":
+ return "terminated";
+ case "detecting":
+ return "detecting";
+ default:
+ break;
+ }
+
if (lifecycle.pr.state === "merged") {
return "merged";
}
@@ -265,20 +282,8 @@ export function deriveLegacyStatus(
}
switch (lifecycle.session.state) {
- case "not_started":
- return "spawning";
- case "needs_input":
- return "needs_input";
- case "stuck":
- return "stuck";
- case "done":
- return "done";
- case "terminated":
- return "terminated";
case "idle":
return "idle";
- case "detecting":
- return "detecting";
case "working":
return "working";
}
From b5c969c6b4ed3f5a908240e4cfe50c8bea3b8f66 Mon Sep 17 00:00:00 2001
From: harshitsinghbhandari <24b4506@iitb.ac.in>
Date: Fri, 17 Apr 2026 11:39:13 +0530
Subject: [PATCH 3/3] fix: address PR review policy regressions (#118)
---
.../src/__tests__/lifecycle-manager.test.ts | 79 +++++++++++++++++++
.../src/__tests__/lifecycle-state.test.ts | 10 +++
packages/core/src/lifecycle-manager.ts | 6 +-
packages/core/src/lifecycle-state.ts | 12 ++-
packages/core/src/session-manager.ts | 2 +-
packages/web/src/lib/dashboard-page-data.ts | 4 +-
6 files changed, 105 insertions(+), 8 deletions(-)
diff --git a/packages/core/src/__tests__/lifecycle-manager.test.ts b/packages/core/src/__tests__/lifecycle-manager.test.ts
index a5ded0160..f56df07bb 100644
--- a/packages/core/src/__tests__/lifecycle-manager.test.ts
+++ b/packages/core/src/__tests__/lifecycle-manager.test.ts
@@ -1649,6 +1649,42 @@ describe("reactions", () => {
expect(metadata?.["lastMergeConflictDispatched"]).toBeFalsy();
});
+ it("clears merge conflict tracking when PR is closed", async () => {
+ config.reactions = {
+ "merge-conflicts": {
+ auto: true,
+ action: "send-to-agent",
+ message: "Resolve merge conflicts.",
+ },
+ };
+
+ const getMergeability = vi.fn();
+ const mockSCM = createMockSCM({
+ getPRState: vi.fn().mockResolvedValue("closed"),
+ getMergeability,
+ });
+ const registry = createMockRegistry({
+ runtime: plugins.runtime,
+ agent: plugins.agent,
+ scm: mockSCM,
+ });
+
+ const lm = setupCheck("app-1", {
+ session: makeSession({
+ status: "pr_open",
+ pr: makePR(),
+ metadata: { lastMergeConflictDispatched: "true" },
+ }),
+ registry,
+ });
+
+ await lm.check("app-1");
+
+ const metadata = readMetadataRaw(env.sessionsDir, "app-1");
+ expect(metadata?.["lastMergeConflictDispatched"]).toBeFalsy();
+ expect(getMergeability).not.toHaveBeenCalled();
+ });
+
it("notifies humans on significant transitions without reaction config", async () => {
const notifier = createMockNotifier();
const mockSCM = createMockSCM({ getPRState: vi.fn().mockResolvedValue("merged") });
@@ -2154,6 +2190,49 @@ describe("rate limiting optimizations", () => {
await lm.check("app-1");
expect(getPendingMock).toHaveBeenCalledTimes(1);
});
+
+ it("clears review backlog tracking when PR is closed", async () => {
+ const getPendingMock = vi.fn();
+ const getAutomatedMock = vi.fn();
+ const mockSCM = createMockSCM({
+ getPRState: vi.fn().mockResolvedValue("closed"),
+ getPendingComments: getPendingMock,
+ getAutomatedComments: getAutomatedMock,
+ });
+ const registry = createMockRegistry({
+ runtime: plugins.runtime,
+ agent: plugins.agent,
+ scm: mockSCM,
+ });
+
+ const lm = setupCheck("app-1", {
+ session: makeSession({
+ status: "pr_open",
+ pr: makePR(),
+ metadata: {
+ lastPendingReviewFingerprint: "fingerprint",
+ lastPendingReviewDispatchHash: "dispatch",
+ lastPendingReviewDispatchAt: "2025-01-01T00:00:00.000Z",
+ lastAutomatedReviewFingerprint: "auto-fingerprint",
+ lastAutomatedReviewDispatchHash: "auto-dispatch",
+ lastAutomatedReviewDispatchAt: "2025-01-01T00:00:00.000Z",
+ },
+ }),
+ registry,
+ });
+
+ await lm.check("app-1");
+
+ const metadata = readMetadataRaw(env.sessionsDir, "app-1");
+ expect(metadata?.["lastPendingReviewFingerprint"]).toBeFalsy();
+ expect(metadata?.["lastPendingReviewDispatchHash"]).toBeFalsy();
+ expect(metadata?.["lastPendingReviewDispatchAt"]).toBeFalsy();
+ expect(metadata?.["lastAutomatedReviewFingerprint"]).toBeFalsy();
+ expect(metadata?.["lastAutomatedReviewDispatchHash"]).toBeFalsy();
+ expect(metadata?.["lastAutomatedReviewDispatchAt"]).toBeFalsy();
+ expect(getPendingMock).not.toHaveBeenCalled();
+ expect(getAutomatedMock).not.toHaveBeenCalled();
+ });
});
describe("summary pinning", () => {
it("pins first quality summary when pinnedSummary not set", async () => {
diff --git a/packages/core/src/__tests__/lifecycle-state.test.ts b/packages/core/src/__tests__/lifecycle-state.test.ts
index 32bae2f35..f8168784f 100644
--- a/packages/core/src/__tests__/lifecycle-state.test.ts
+++ b/packages/core/src/__tests__/lifecycle-state.test.ts
@@ -33,6 +33,16 @@ describe("deriveLegacyStatus", () => {
expect(deriveLegacyStatus(terminated)).toBe("terminated");
});
+ it("preserves prior terminal legacy statuses for terminated sessions", () => {
+ const terminated = createOpenPRLifecycle();
+ terminated.session.state = "terminated";
+ terminated.session.reason = "manually_killed";
+
+ expect(deriveLegacyStatus(terminated, "killed")).toBe("killed");
+ expect(deriveLegacyStatus(terminated, "cleanup")).toBe("cleanup");
+ expect(deriveLegacyStatus(terminated, "errored")).toBe("errored");
+ });
+
it("keeps PR-oriented aliases for idle workers with open PRs", () => {
const reviewPending = createOpenPRLifecycle();
reviewPending.session.state = "idle";
diff --git a/packages/core/src/lifecycle-manager.ts b/packages/core/src/lifecycle-manager.ts
index 897b215f4..f37b4375d 100644
--- a/packages/core/src/lifecycle-manager.ts
+++ b/packages/core/src/lifecycle-manager.ts
@@ -1033,7 +1033,7 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
const humanReactionKey = "changes-requested";
const automatedReactionKey = "bugbot-comments";
- if (TERMINAL_STATUSES.has(newStatus)) {
+ if (TERMINAL_STATUSES.has(newStatus) || session.lifecycle.pr.state !== "open") {
clearReactionTracker(session.id, humanReactionKey);
clearReactionTracker(session.id, automatedReactionKey);
lastReviewBacklogCheckAt.delete(session.id);
@@ -1365,8 +1365,8 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
const conflictReactionKey = "merge-conflicts";
- // Clear tracking when PR is closed/merged
- if (newStatus === "merged" || newStatus === "killed") {
+ // Clear tracking when PR is no longer open.
+ if (session.lifecycle.pr.state !== "open" || newStatus === "killed") {
clearReactionTracker(session.id, conflictReactionKey);
updateSessionMetadata(session, {
lastMergeConflictDispatched: "",
diff --git a/packages/core/src/lifecycle-state.ts b/packages/core/src/lifecycle-state.ts
index 0b07901e3..9b1266bea 100644
--- a/packages/core/src/lifecycle-state.ts
+++ b/packages/core/src/lifecycle-state.ts
@@ -250,8 +250,18 @@ export function parseCanonicalLifecycle(
export function deriveLegacyStatus(
lifecycle: CanonicalSessionLifecycle,
- _previousStatus: SessionStatus = "working",
+ previousStatus: SessionStatus = "working",
): SessionStatus {
+ if (
+ lifecycle.session.state === "terminated" &&
+ (previousStatus === "cleanup" ||
+ previousStatus === "errored" ||
+ previousStatus === "killed" ||
+ previousStatus === "terminated")
+ ) {
+ return previousStatus;
+ }
+
switch (lifecycle.session.state) {
case "not_started":
return "spawning";
diff --git a/packages/core/src/session-manager.ts b/packages/core/src/session-manager.ts
index fe158ebd7..a8de6c6c4 100644
--- a/packages/core/src/session-manager.ts
+++ b/packages/core/src/session-manager.ts
@@ -1872,7 +1872,7 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM
const plugins = resolvePlugins(project);
let shouldKill = false;
- // Check if PR is merged
+ // Check if PR was closed without merging.
if (session.pr && plugins.scm) {
try {
const prState = await plugins.scm.getPRState(session.pr);
diff --git a/packages/web/src/lib/dashboard-page-data.ts b/packages/web/src/lib/dashboard-page-data.ts
index 3c5a18d7b..52b6354d8 100644
--- a/packages/web/src/lib/dashboard-page-data.ts
+++ b/packages/web/src/lib/dashboard-page-data.ts
@@ -73,8 +73,7 @@ export const getDashboardPageData = cache(async function getDashboardPageData(pr
FAST_METADATA_ENRICH_TIMEOUT_MS,
);
- // PR cache hits only (in-memory lookup, no SCM API calls)
- // TERMINAL_STATUSES includes merged, killed, cleanup, done, terminated, errored
+ // PR cache hits only (in-memory lookup, no SCM API calls).
for (let i = 0; i < coreSessions.length; i++) {
const core = coreSessions[i];
if (!core.pr) continue;
@@ -83,7 +82,6 @@ export const getDashboardPageData = cache(async function getDashboardPageData(pr
if (scm) {
await enrichSessionPR(pageData.sessions[i], scm, core.pr, { cacheOnly: true });
}
-
}
} catch {
pageData.sessions = [];