From baa6e70792a0732200c670415c969169bd087965 Mon Sep 17 00:00:00 2001 From: adil Date: Sat, 11 Apr 2026 21:10:05 +0530 Subject: [PATCH 1/3] fix(core): skip liveness check on spawning sessions in enrichSessionWithRuntimeState MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #1035 — race condition where lifecycle poll fires before tmux fully initializes, causing isAlive() to return false and permanently marking the session as "killed". The lifecycle-manager already guards against this (canProbeRuntimeIdentity skips spawning sessions), but enrichSessionWithRuntimeState in session-manager did not. Adding the same `status !== "spawning"` guard prevents the false kill during the spawn window. Co-Authored-By: Claude Opus 4.6 (1M context) --- packages/core/src/session-manager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/session-manager.ts b/packages/core/src/session-manager.ts index dc821e08d..4e2c38a25 100644 --- a/packages/core/src/session-manager.ts +++ b/packages/core/src/session-manager.ts @@ -887,7 +887,7 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM // Fabricated handles (constructed as fallback for external sessions) should // NOT override status to "killed" — we don't know if the session ever had // a tmux session, and we'd clobber meaningful statuses like "pr_open". - if (handleFromMetadata && session.runtimeHandle && plugins.runtime) { + if (handleFromMetadata && session.runtimeHandle && plugins.runtime && session.status !== "spawning") { try { const alive = await plugins.runtime.isAlive(session.runtimeHandle); if (!alive) { From 8ff258213b92d678d91e40f7f309ff4c572082f6 Mon Sep 17 00:00:00 2001 From: adil Date: Sat, 11 Apr 2026 21:20:57 +0530 Subject: [PATCH 2/3] fix(core): update comment and add regression test for spawning guard Address review feedback: - Update comment in enrichSessionWithRuntimeState to reflect the spawning exception instead of saying "regardless of session status" - Add regression test verifying isAlive() is not called for spawning sessions and status remains "spawning" (not falsely set to "killed") Refs #1035 Co-Authored-By: Claude Opus 4.6 (1M context) --- .../src/__tests__/session-manager.test.ts | 28 +++++++++++++++++++ packages/core/src/session-manager.ts | 9 ++++-- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/packages/core/src/__tests__/session-manager.test.ts b/packages/core/src/__tests__/session-manager.test.ts index ae9e1ef45..91b1b5fff 100644 --- a/packages/core/src/__tests__/session-manager.test.ts +++ b/packages/core/src/__tests__/session-manager.test.ts @@ -281,3 +281,31 @@ describe("deleteSession retry loop", () => { expect(deleteCallCount).toBe(1); }); }); + +describe("spawning session liveness (#1035)", () => { + it("does not call runtime.isAlive for spawning sessions, preventing false 'killed' status", async () => { + // Write a session in "spawning" status with a persisted runtime handle + writeMetadata(sessionsDir, "app-spawn", { + worktree: "/tmp/ws", + branch: "main", + status: "spawning", + project: "my-app", + agent: "opencode", + runtimeHandle: JSON.stringify(makeHandle("rt-spawn")), + }); + + // Make isAlive return false — if it were called, the session would become "killed" + vi.mocked(ctx.mockRuntime.isAlive).mockResolvedValue(false); + + const sm = createSessionManager({ config, registry: mockRegistry }); + const sessions = await sm.list(); + const spawning = sessions.find((s) => s.id === "app-spawn"); + + // isAlive must NOT have been called for the spawning session + expect(ctx.mockRuntime.isAlive).not.toHaveBeenCalled(); + + // Status must remain "spawning", not "killed" + expect(spawning).toBeDefined(); + expect(spawning!.status).toBe("spawning"); + }); +}); diff --git a/packages/core/src/session-manager.ts b/packages/core/src/session-manager.ts index 4e2c38a25..5d6932124 100644 --- a/packages/core/src/session-manager.ts +++ b/packages/core/src/session-manager.ts @@ -881,9 +881,12 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM plugins: ReturnType, handleFromMetadata: boolean, ): Promise { - // Check runtime liveness first — regardless of session status. - // This fixes #1081: terminal statuses (merged, done, etc.) should not force - // activity to "exited" if the agent process is still alive. + // Check runtime liveness first — for all statuses except "spawning". + // Skip spawning sessions because tmux may not be fully initialized yet, + // and a false-negative from isAlive() would permanently mark the session + // as "killed" (see #1035). + // This also fixes #1081: terminal statuses (merged, done, etc.) should not + // force activity to "exited" if the agent process is still alive. // Fabricated handles (constructed as fallback for external sessions) should // NOT override status to "killed" — we don't know if the session ever had // a tmux session, and we'd clobber meaningful statuses like "pr_open". From a7b0c8567a06bcc1178168eb1866055adf0eeafb Mon Sep 17 00:00:00 2001 From: adil Date: Wed, 15 Apr 2026 03:09:44 +0530 Subject: [PATCH 3/3] fix(core): guard lifecycle-manager poll path against spawning race condition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review feedback from @illegalcall: 1. Fix canProbeRuntimeIdentity in determineStatus() — the previous guard (hasPersistedRuntimeIdentity || status !== spawning) was ineffective because spawn writes runtimeHandle to metadata before leaving "spawning" status. Simplified to just check status !== spawning. 2. Guard getActivityState exited→killed transition — agent plugins return "exited" before the process starts, so spawning sessions were being marked killed via the agent activity path too. 3. Add lifecycle-manager tests covering both paths: runtime.isAlive false with persisted handle, and agent reporting "exited" activity during spawning. Refs #1035 Co-Authored-By: Claude Opus 4.6 (1M context) --- .../src/__tests__/lifecycle-manager.test.ts | 38 +++++++++++++++++++ packages/core/src/lifecycle-manager.ts | 11 +++--- 2 files changed, 44 insertions(+), 5 deletions(-) diff --git a/packages/core/src/__tests__/lifecycle-manager.test.ts b/packages/core/src/__tests__/lifecycle-manager.test.ts index 01e6a4b1b..e0ff3a0a5 100644 --- a/packages/core/src/__tests__/lifecycle-manager.test.ts +++ b/packages/core/src/__tests__/lifecycle-manager.test.ts @@ -135,6 +135,44 @@ describe("check (single session)", () => { expect(plugins.runtime.isAlive).not.toHaveBeenCalled(); }); + it("does not kill a spawning session even when runtimeHandle IS persisted in metadata (#1035)", async () => { + vi.mocked(plugins.runtime.isAlive).mockResolvedValue(false); + + const lm = setupCheck("app-1", { + session: makeSession({ + status: "spawning", + runtimeHandle: { id: "app-1", runtimeName: "mock", data: {} }, + metadata: {}, + }), + // runtimeHandle IS in metadata — this is the production scenario + }); + + await lm.check("app-1"); + + expect(lm.getStates().get("app-1")).toBe("working"); + expect(plugins.runtime.isAlive).not.toHaveBeenCalled(); + }); + + it("does not kill a spawning session when agent reports exited activity (#1035)", async () => { + vi.mocked(plugins.agent.getActivityState).mockResolvedValue({ + state: "exited" as ActivityState, + timestamp: new Date(), + }); + + const lm = setupCheck("app-1", { + session: makeSession({ + status: "spawning", + runtimeHandle: { id: "app-1", runtimeName: "mock", data: {} }, + metadata: {}, + }), + }); + + await lm.check("app-1"); + + // Should transition to working, not killed + expect(lm.getStates().get("app-1")).toBe("working"); + }); + it("still probes a working session when it relies on a synthesized runtime handle", async () => { vi.mocked(plugins.runtime.isAlive).mockResolvedValue(false); diff --git a/packages/core/src/lifecycle-manager.ts b/packages/core/src/lifecycle-manager.ts index 99105fd42..79a083be9 100644 --- a/packages/core/src/lifecycle-manager.ts +++ b/packages/core/src/lifecycle-manager.ts @@ -371,11 +371,12 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan // Track activity state across steps so stuck detection can run after PR checks let detectedIdleTimestamp: Date | null = null; - const hasPersistedRuntimeIdentity = - typeof session.metadata["runtimeHandle"] === "string" || - typeof session.metadata["tmuxName"] === "string"; + // Never probe runtime/agent liveness while a session is still spawning — + // tmux may not be initialized and the agent process hasn't started yet. + // A persisted runtimeHandle alone is insufficient to gate on because spawn + // writes it to metadata before leaving "spawning" status (#1035). const canProbeRuntimeIdentity = - hasPersistedRuntimeIdentity || session.status !== SESSION_STATUS.SPAWNING; + session.status !== SESSION_STATUS.SPAWNING; // 1. Check if runtime is alive if (session.runtimeHandle && canProbeRuntimeIdentity) { @@ -417,7 +418,7 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan const activityState = await agent.getActivityState(session, config.readyThresholdMs); if (activityState) { if (activityState.state === "waiting_input") return "needs_input"; - if (activityState.state === "exited") return "killed"; + if (activityState.state === "exited" && canProbeRuntimeIdentity) return "killed"; if ( (activityState.state === "idle" || activityState.state === "blocked") &&