From 2adf6e08cd55f3671cd6b2bc476dac225f90facd Mon Sep 17 00:00:00 2001 From: i-trytoohard <193449657+i-trytoohard@users.noreply.github.com> Date: Fri, 8 May 2026 02:55:30 +0530 Subject: [PATCH] fix(session-manager): refuse to restore sessions with merged/closed PRs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a session's PR has reached a terminal state (merged or closed) and the session is later restored, two things conspire to attach the session to an unrelated PR: 1. The original head branch is typically deleted on merge, so on workspace restore `session.branch` may drift to the project default branch (or be rewritten by an agent-side `git checkout main`). 2. With branch == default branch, downstream `detectPR` matches whatever PR happens to share that head ref name (including fork PRs). Fix 1 (scm-github) and Fix 2 (lifecycle) defend in depth, but the cleanest guarantee is to refuse the restore up-front: a session whose PR has already merged/closed has no remaining work to do. If the user wants follow-up work, they should spawn a new session. The previous behavior — silently clearing terminal `lifecycle.pr.*` fields on restore and resetting `session.state` to "working" — is removed because it's now unreachable: the new check at step 3a throws before we get there. Invariants preserved: - Workers with `pr.state` of "open"/"none" still restore normally. - `isRestorable` semantics unchanged for terminated/done/runtime-lost. - Errors surface via `SessionNotRestorableError`, the same channel used by other restore-blockers (workspace missing, OpenCode mapping, etc). Refs #1724. --- .../__tests__/session-manager/restore.test.ts | 6 ++-- packages/core/src/session-manager.ts | 29 ++++++++++++------- 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/packages/core/src/__tests__/session-manager/restore.test.ts b/packages/core/src/__tests__/session-manager/restore.test.ts index f955bb509..bf9691b4a 100644 --- a/packages/core/src/__tests__/session-manager/restore.test.ts +++ b/packages/core/src/__tests__/session-manager/restore.test.ts @@ -197,12 +197,13 @@ describe("restore", () => { expect(mockRuntime.create).toHaveBeenCalled(); }); - it("allows restoring merged sessions", async () => { + it("refuses to restore sessions whose PR has merged (would silently re-spawn against the default branch)", async () => { const ws = "/tmp/mock-ws/app-1"; writeMetadata(sessionsDir, "app-1", { worktree: ws, branch: "main", status: "merged", + pr: "https://github.com/acme/repo/pull/1723", project: "my-app", runtimeHandle: makeHandle("rt-old"), }); @@ -222,8 +223,7 @@ describe("restore", () => { }; const sm = createSessionManager({ config, registry }); - const restored = await sm.restore("app-1"); - expect(restored.id).toBe("app-1"); + await expect(sm.restore("app-1")).rejects.toThrow(/is merged/); }); it("throws SessionNotRestorableError for working sessions", async () => { diff --git a/packages/core/src/session-manager.ts b/packages/core/src/session-manager.ts index 1cec42a94..8ebe2aca8 100644 --- a/packages/core/src/session-manager.ts +++ b/packages/core/src/session-manager.ts @@ -2801,6 +2801,23 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM throw new SessionNotRestorableError(sessionId, reason); } + // 3a. Refuse to silently revive a session whose PR has reached a terminal + // state. The original branch has typically been deleted on merge, so + // `session.branch` may have drifted to the project default branch + // between the kill and the restore. Re-spawning would associate the + // session with whatever PR `detectPR` finds on that branch — including + // unrelated fork PRs (see #1724). If the user wants a fresh session + // in this workspace, they should spawn a new one. + const prState = session.lifecycle.pr.state; + if (prState === "merged" || prState === "closed") { + const prRef = session.lifecycle.pr.url ?? session.pr?.url ?? raw["pr"] ?? "(unknown)"; + throw new SessionNotRestorableError( + sessionId, + `PR ${prRef} is ${prState}; the session's work is complete. ` + + `Spawn a new session for follow-up work instead of restoring.`, + ); + } + // 4. Validate required plugins (plugins already resolved above for enrichment) if (!plugins.runtime) { throw new Error(`Runtime plugin '${project.runtime ?? config.defaults.runtime}' not found`); @@ -2967,16 +2984,8 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM restoredLifecycle.runtime.handle = handle; restoredLifecycle.runtime.lastObservedAt = now; - // Reset terminal PR state so the lifecycle manager doesn't immediately - // re-terminate the session. The old PR is done — if the agent creates - // a new one, PR auto-detect will pick it up. - if (restoredLifecycle.pr.state === "merged" || restoredLifecycle.pr.state === "closed") { - restoredLifecycle.pr.state = "none"; - restoredLifecycle.pr.reason = "cleared_on_restore"; - restoredLifecycle.pr.number = null; - restoredLifecycle.pr.url = null; - restoredLifecycle.pr.lastObservedAt = null; - } + // Terminal PR state (merged/closed) is rejected up-front in step 3a, so + // by here the lifecycle.pr is non-terminal and is preserved as-is. updateMetadata(sessionsDir, sessionId, { ...buildLifecycleMetadataPatch(restoredLifecycle),