From 1194697b3bb46461a69d375caeb8ae423462949f Mon Sep 17 00:00:00 2001 From: Jayesh Sharma Date: Thu, 12 Mar 2026 14:46:39 +0530 Subject: [PATCH] fix: skip PR auto-detection for orchestrator sessions (#442) * fix: skip PR auto-detection for orchestrator sessions Orchestrator sessions sit on the base branch (e.g. master) and should never own a PR. When the lifecycle worker ran detectPR for these sessions, any PR whose head branch matched master (e.g. a master->prod deploy PR) would get incorrectly attached and keep re-attaching after manual cleanup. Add a role=orchestrator check to the PR auto-detection guard so orchestrator sessions are skipped entirely. Co-Authored-By: Claude Opus 4.6 * fix: add session ID suffix fallback for orchestrator PR auto-detection skip Pre-existing orchestrator sessions spawned before the role metadata field was added lack role=orchestrator. Mirror the session-manager pattern by also checking session.id.endsWith("-orchestrator") as a fallback. Co-Authored-By: Claude Opus 4.6 --------- Co-authored-by: Claude Opus 4.6 --- .../src/__tests__/lifecycle-manager.test.ts | 110 ++++++++++++++++++ packages/core/src/lifecycle-manager.ts | 11 +- 2 files changed, 120 insertions(+), 1 deletion(-) diff --git a/packages/core/src/__tests__/lifecycle-manager.test.ts b/packages/core/src/__tests__/lifecycle-manager.test.ts index 42dc5ba7c..172301559 100644 --- a/packages/core/src/__tests__/lifecycle-manager.test.ts +++ b/packages/core/src/__tests__/lifecycle-manager.test.ts @@ -507,6 +507,116 @@ describe("check (single session)", () => { expect(lm.getStates().get("app-1")).toBe("working"); }); + it("skips PR auto-detection for orchestrator sessions", async () => { + const mockSCM: SCM = { + name: "mock-scm", + detectPR: vi.fn().mockResolvedValue(makePR()), + getPRState: vi.fn().mockResolvedValue("open"), + mergePR: vi.fn(), + closePR: vi.fn(), + getCIChecks: vi.fn(), + getCISummary: vi.fn().mockResolvedValue("passing"), + getReviews: vi.fn(), + getReviewDecision: vi.fn().mockResolvedValue("none"), + getPendingComments: vi.fn(), + getAutomatedComments: vi.fn(), + getMergeability: vi.fn(), + }; + + const registryWithSCM: PluginRegistry = { + ...mockRegistry, + get: vi.fn().mockImplementation((slot: string) => { + if (slot === "runtime") return mockRuntime; + if (slot === "agent") return mockAgent; + if (slot === "scm") return mockSCM; + return null; + }), + }; + + writeMetadata(sessionsDir, "app-1", { + worktree: "/tmp", + branch: "master", + status: "working", + project: "my-app", + role: "orchestrator", + }); + + const realSessionManager = createSessionManager({ + config, + registry: registryWithSCM, + }); + const session = await realSessionManager.get("app-1"); + + expect(session).not.toBeNull(); + vi.mocked(mockSessionManager.get).mockResolvedValue(session); + + const lm = createLifecycleManager({ + config, + registry: registryWithSCM, + sessionManager: mockSessionManager, + }); + + await lm.check("app-1"); + + expect(mockSCM.detectPR).not.toHaveBeenCalled(); + expect(lm.getStates().get("app-1")).toBe("working"); + }); + + it("skips PR auto-detection for orchestrator sessions identified by ID suffix (fallback)", async () => { + const mockSCM: SCM = { + name: "mock-scm", + detectPR: vi.fn().mockResolvedValue(makePR()), + getPRState: vi.fn().mockResolvedValue("open"), + mergePR: vi.fn(), + closePR: vi.fn(), + getCIChecks: vi.fn(), + getCISummary: vi.fn().mockResolvedValue("passing"), + getReviews: vi.fn(), + getReviewDecision: vi.fn().mockResolvedValue("none"), + getPendingComments: vi.fn(), + getAutomatedComments: vi.fn(), + getMergeability: vi.fn(), + }; + + const registryWithSCM: PluginRegistry = { + ...mockRegistry, + get: vi.fn().mockImplementation((slot: string) => { + if (slot === "runtime") return mockRuntime; + if (slot === "agent") return mockAgent; + if (slot === "scm") return mockSCM; + return null; + }), + }; + + // Session has no role metadata but ID ends with "-orchestrator" + writeMetadata(sessionsDir, "app-orchestrator", { + worktree: "/tmp", + branch: "master", + status: "working", + project: "my-app", + }); + + const realSessionManager = createSessionManager({ + config, + registry: registryWithSCM, + }); + const session = await realSessionManager.get("app-orchestrator"); + + expect(session).not.toBeNull(); + vi.mocked(mockSessionManager.get).mockResolvedValue(session); + + const lm = createLifecycleManager({ + config, + registry: registryWithSCM, + sessionManager: mockSessionManager, + }); + + await lm.check("app-orchestrator"); + + expect(mockSCM.detectPR).not.toHaveBeenCalled(); + expect(lm.getStates().get("app-orchestrator")).toBe("working"); + }); + it("detects merged PR", async () => { const mockSCM: SCM = { name: "mock-scm", diff --git a/packages/core/src/lifecycle-manager.ts b/packages/core/src/lifecycle-manager.ts index 0ca4438c9..46ab8e3e7 100644 --- a/packages/core/src/lifecycle-manager.ts +++ b/packages/core/src/lifecycle-manager.ts @@ -355,7 +355,16 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan // 3. Auto-detect PR by branch if metadata.pr is missing. // This is critical for agents without auto-hook systems (Codex, Aider, // OpenCode) that can't reliably write pr= to metadata on their own. - if (!session.pr && scm && session.branch && session.metadata["prAutoDetect"] !== "off") { + // Skip orchestrator sessions — they sit on the base branch (e.g. master) + // and should never own a PR. + if ( + !session.pr && + scm && + session.branch && + session.metadata["prAutoDetect"] !== "off" && + session.metadata["role"] !== "orchestrator" && + !session.id.endsWith("-orchestrator") + ) { try { const detectedPR = await scm.detectPR(session, project); if (detectedPR) {