From 8a94c5b0e645eda26acc0c9afe19bc4e7541b492 Mon Sep 17 00:00:00 2001 From: harshitsinghbhandari <24b4506@iitb.ac.in> Date: Sat, 18 Apr 2026 12:11:17 +0530 Subject: [PATCH] fix(core): address PR #1300 lifecycle regressions Fix legacy merged-session rehydration so merged PRs stay non-restorable after v2 synthesis. Also validate canonical lifecycle payloads before normalizing them so malformed statePayload data falls back to synthesized metadata instead of corrupting lifecycle state. --- .../src/__tests__/lifecycle-state.test.ts | 37 +++++++ .../__tests__/session-manager/restore.test.ts | 14 +++ packages/core/src/lifecycle-state.ts | 96 +++++++++++++++++-- 3 files changed, 140 insertions(+), 7 deletions(-) diff --git a/packages/core/src/__tests__/lifecycle-state.test.ts b/packages/core/src/__tests__/lifecycle-state.test.ts index d96579ae3..8b195a046 100644 --- a/packages/core/src/__tests__/lifecycle-state.test.ts +++ b/packages/core/src/__tests__/lifecycle-state.test.ts @@ -63,6 +63,21 @@ describe("deriveLegacyStatus", () => { }); describe("parseCanonicalLifecycle", () => { + it("rehydrates legacy merged sessions with a merged PR state", () => { + const parsed = parseCanonicalLifecycle({ + status: "merged", + pr: "https://github.com/org/repo/pull/42", + createdAt: "2025-01-01T00:00:00.000Z", + }); + + expect(parsed.session.state).toBe("idle"); + expect(parsed.session.reason).toBe("merged_waiting_decision"); + expect(parsed.pr.state).toBe("merged"); + expect(parsed.pr.reason).toBe("merged"); + expect(parsed.pr.number).toBe(42); + expect(deriveLegacyStatus(parsed, "merged")).toBe("merged"); + }); + it("preserves explicit null payload fields instead of rehydrating stale flat metadata", () => { const lifecycle = createInitialCanonicalLifecycle("worker", new Date("2025-01-01T00:00:00Z")); lifecycle.session.state = "working"; @@ -86,4 +101,26 @@ describe("parseCanonicalLifecycle", () => { expect(parsed.runtime.handle).toBeNull(); expect(parsed.runtime.tmuxName).toBeNull(); }); + + it("falls back to synthesized lifecycle when a v2 payload is malformed", () => { + const parsed = parseCanonicalLifecycle({ + status: "review_pending", + pr: "https://github.com/org/repo/pull/42", + stateVersion: "2", + statePayload: JSON.stringify({ + version: 2, + session: { + kind: "worker", + state: "working", + reason: 123, + }, + }), + }); + + expect(parsed.session.state).toBe("working"); + expect(parsed.session.reason).toBe("task_in_progress"); + expect(parsed.pr.state).toBe("open"); + expect(parsed.pr.reason).toBe("in_progress"); + expect(parsed.pr.number).toBe(42); + }); }); diff --git a/packages/core/src/__tests__/session-manager/restore.test.ts b/packages/core/src/__tests__/session-manager/restore.test.ts index eebe1be01..e3f68c34d 100644 --- a/packages/core/src/__tests__/session-manager/restore.test.ts +++ b/packages/core/src/__tests__/session-manager/restore.test.ts @@ -176,6 +176,20 @@ describe("restore", () => { await expect(sm.restore("app-1")).rejects.toThrow(SessionNotRestorableError); }); + it("throws SessionNotRestorableError for legacy merged sessions with a PR URL", async () => { + writeMetadata(sessionsDir, "app-1", { + worktree: "/tmp", + branch: "main", + status: "merged", + pr: "https://github.com/org/my-app/pull/10", + project: "my-app", + createdAt: "2025-01-01T00:00:00.000Z", + }); + + const sm = createSessionManager({ config, registry: mockRegistry }); + await expect(sm.restore("app-1")).rejects.toThrow(SessionNotRestorableError); + }); + it("throws SessionNotRestorableError for working sessions", async () => { writeMetadata(sessionsDir, "app-1", { worktree: "/tmp", diff --git a/packages/core/src/lifecycle-state.ts b/packages/core/src/lifecycle-state.ts index a28923acb..3e7972aa5 100644 --- a/packages/core/src/lifecycle-state.ts +++ b/packages/core/src/lifecycle-state.ts @@ -10,6 +10,7 @@ import type { SessionKind, SessionStatus, } from "./types.js"; +import { z } from "zod"; import { parsePrFromUrl } from "./utils/pr.js"; import { safeJsonParse, validateStatus } from "./utils/validation.js"; @@ -20,6 +21,84 @@ interface ParseCanonicalLifecycleOptions { createdAt?: Date; } +const TimestampSchema = z.string().nullable(); + +const RuntimeHandleSchema = z.object({ + id: z.string(), + runtimeName: z.string(), + data: z.record(z.unknown()), +}); + +const CanonicalSessionLifecycleSchema = z.object({ + version: z.literal(2), + session: z.object({ + kind: z.enum(["worker", "orchestrator"]), + state: z.enum([ + "not_started", + "working", + "idle", + "needs_input", + "stuck", + "detecting", + "done", + "terminated", + ]), + reason: z.enum([ + "spawn_requested", + "agent_acknowledged", + "task_in_progress", + "pr_created", + "pr_closed_waiting_decision", + "fixing_ci", + "resolving_review_comments", + "awaiting_user_input", + "awaiting_external_review", + "research_complete", + "merged_waiting_decision", + "manually_killed", + "runtime_lost", + "agent_process_exited", + "probe_failure", + "error_in_process", + ]), + startedAt: TimestampSchema, + completedAt: TimestampSchema, + terminatedAt: TimestampSchema, + lastTransitionAt: z.string(), + }), + pr: z.object({ + state: z.enum(["none", "open", "merged", "closed"]), + reason: z.enum([ + "not_created", + "in_progress", + "ci_failing", + "review_pending", + "changes_requested", + "approved", + "merge_ready", + "merged", + "closed_unmerged", + ]), + number: z.number().int().nullable(), + url: z.string().nullable(), + lastObservedAt: TimestampSchema, + }), + runtime: z.object({ + state: z.enum(["unknown", "alive", "exited", "missing", "probe_failed"]), + reason: z.enum([ + "spawn_incomplete", + "process_running", + "process_missing", + "tmux_missing", + "manual_kill_requested", + "probe_error", + ]), + lastObservedAt: TimestampSchema, + handle: RuntimeHandleSchema.nullable(), + tmuxName: z.string().nullable(), + }), +}); + function normalizeTimestamp(value: unknown, fallback: string | null = null): string | null { if (typeof value !== "string") return fallback; const parsed = Date.parse(value); @@ -98,7 +177,7 @@ function synthesizeSessionState( } } -function synthesizePRState(meta: Record): { +function synthesizePRState(meta: Record, status: SessionStatus): { state: CanonicalPRState; reason: CanonicalPRReason; number: number | null; @@ -110,8 +189,8 @@ function synthesizePRState(meta: Record): { } const parsed = parsePrFromUrl(prUrl); return { - state: "open", - reason: "in_progress", + state: status === "merged" ? "merged" : "open", + reason: status === "merged" ? "merged" : "in_progress", number: parsed?.number ?? null, url: prUrl, }; @@ -154,7 +233,7 @@ function synthesizeCanonicalLifecycle( normalizeTimestamp(meta["createdAt"], new Date().toISOString()) ?? new Date().toISOString(); const sessionState = synthesizeSessionState(status); - const pr = synthesizePRState(meta); + const pr = synthesizePRState(meta, status); const runtime = synthesizeRuntimeState(meta, options.runtimeHandle ?? null); return { @@ -289,10 +368,11 @@ export function parseCanonicalLifecycle( ): CanonicalSessionLifecycle { const parsed = meta["statePayload"] && meta["stateVersion"] === "2" - ? safeJsonParse(meta["statePayload"]) + ? safeJsonParse(meta["statePayload"]) : null; - if (parsed?.version === 2) { - return normalizePayloadLifecycle(parsed, meta, options); + const validated = CanonicalSessionLifecycleSchema.safeParse(parsed); + if (validated.success) { + return normalizePayloadLifecycle(validated.data, meta, options); } return synthesizeCanonicalLifecycle(meta, options); } @@ -345,6 +425,8 @@ export function deriveLegacyStatus( return "idle"; case "working": return "working"; + default: + return previousStatus; } }