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] 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"; }