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 <noreply@anthropic.com>
* 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 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
dbdbaa54ea
commit
1194697b3b
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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=<url> 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) {
|
||||
|
|
|
|||
Loading…
Reference in New Issue