fix: skip orchestrator sessions during cleanup (#144)

* fix: skip orchestrator sessions during cleanup

The cleanup command could kill the orchestrator's own tmux session
(named {prefix}-orchestrator) because it has no PR or issue, making
it eligible for the "dead runtime" cleanup heuristic.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: add role metadata for orchestrator session identification

Store role=orchestrator in metadata during spawnOrchestrator() and
check it in cleanup() for explicit identification. Keeps the name
fallback (endsWith -orchestrator) for pre-existing sessions.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: preserve role in restore path, add integration test

- restore() now preserves the role field when recreating metadata
  from archive (previously dropped, breaking role-based detection)
- Add integration test: orchestrator skipped while sibling session
  with same closed issue gets killed (verifies real tracker plugin)
- Verified: both unit and integration tests fail without the guard

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: persist role field in metadata, isolate role test from name fallback

Bugbot caught two real issues:
- writeMetadata() and readMetadata() never serialized the role field,
  so role=orchestrator was silently dropped on disk
- The role-based test used "app-orchestrator" as session name, so the
  name fallback caught it — role check was never actually exercised

Fix: add role to metadata read/write, rename test session to "app-99"
so only the role metadata path can protect it.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

packages/core/src/__tests__/plugin-integration.test.ts

@@ -304,6 +304,42 @@ describe("plugin integration", () => {
       expect(session.branch).toBe("feat/99");
     });
 
+    it("cleanup() never kills orchestrator sessions even when issue is closed", async () => {
+      const registry = createTestRegistry();
+      const sm = createSessionManager({ config, registry });
+
+      // Seed an orchestrator session with a closed issue — it should still be skipped
+      writeMetadata(sessionsDir, "app-orchestrator", {
+        worktree: "/tmp/mock-ws/app-orchestrator",
+        branch: "main",
+        status: "working",
+        role: "orchestrator",
+        issue: "99",
+        project: "my-app",
+        runtimeHandle: JSON.stringify(makeHandle("rt-orch")),
+      });
+
+      // Also seed a regular session with the same closed issue — it SHOULD be killed
+      writeMetadata(sessionsDir, "app-1", {
+        worktree: "/tmp/mock-ws/app-1",
+        branch: "feat/issue-99",
+        status: "working",
+        issue: "99",
+        project: "my-app",
+        runtimeHandle: JSON.stringify(makeHandle("rt-1")),
+      });
+
+      // Mock gh: issue is closed
+      mockGh({ state: "CLOSED" });
+
+      const result = await sm.cleanup("my-app");
+
+      // Regular session killed, orchestrator skipped
+      expect(result.killed).toContain("app-1");
+      expect(result.killed).not.toContain("app-orchestrator");
+      expect(result.skipped).toContain("app-orchestrator");
+    });
+
     it("cleanup() calls tracker-github isCompleted() and kills completed sessions", async () => {
       const registry = createTestRegistry();
       const sm = createSessionManager({ config, registry });

packages/core/src/__tests__/session-manager.test.ts

@@ -957,6 +957,70 @@ describe("cleanup", () => {
     expect(result.skipped).toContain("app-1");
   });
 
+  it("skips orchestrator sessions by role metadata", async () => {
+    const deadRuntime: Runtime = {
+      ...mockRuntime,
+      isAlive: vi.fn().mockResolvedValue(false),
+    };
+    const registryWithDead: PluginRegistry = {
+      ...mockRegistry,
+      get: vi.fn().mockImplementation((slot: string) => {
+        if (slot === "runtime") return deadRuntime;
+        if (slot === "agent") return mockAgent;
+        if (slot === "workspace") return mockWorkspace;
+        return null;
+      }),
+    };
+
+    // Session with role=orchestrator but a name that does NOT end in "-orchestrator"
+    // so only the role metadata check can protect it (not the name fallback)
+    writeMetadata(sessionsDir, "app-99", {
+      worktree: "/tmp",
+      branch: "main",
+      status: "working",
+      role: "orchestrator",
+      project: "my-app",
+      runtimeHandle: JSON.stringify(makeHandle("rt-orch")),
+    });
+
+    const sm = createSessionManager({ config, registry: registryWithDead });
+    const result = await sm.cleanup();
+
+    expect(result.killed).toHaveLength(0);
+    expect(result.skipped).toContain("app-99");
+  });
+
+  it("skips orchestrator sessions by name fallback (no role metadata)", async () => {
+    const deadRuntime: Runtime = {
+      ...mockRuntime,
+      isAlive: vi.fn().mockResolvedValue(false),
+    };
+    const registryWithDead: PluginRegistry = {
+      ...mockRegistry,
+      get: vi.fn().mockImplementation((slot: string) => {
+        if (slot === "runtime") return deadRuntime;
+        if (slot === "agent") return mockAgent;
+        if (slot === "workspace") return mockWorkspace;
+        return null;
+      }),
+    };
+
+    // Pre-existing orchestrator session without role field
+    writeMetadata(sessionsDir, "app-orchestrator", {
+      worktree: "/tmp",
+      branch: "main",
+      status: "working",
+      project: "my-app",
+      runtimeHandle: JSON.stringify(makeHandle("rt-orch")),
+    });
+
+    const sm = createSessionManager({ config, registry: registryWithDead });
+    const result = await sm.cleanup();
+
+    expect(result.killed).toHaveLength(0);
+    expect(result.skipped).toContain("app-orchestrator");
+  });
+
   it("kills sessions with dead runtimes", async () => {
     const deadRuntime: Runtime = {
       ...mockRuntime,

packages/core/src/metadata.ts

@@ -100,6 +100,7 @@ export function readMetadata(dataDir: string, sessionId: SessionId): SessionMeta
     agent: raw["agent"],
     createdAt: raw["createdAt"],
     runtimeHandle: raw["runtimeHandle"],
+    role: raw["role"],
     dashboardPort: raw["dashboardPort"] ? Number(raw["dashboardPort"]) : undefined,
     terminalWsPort: raw["terminalWsPort"] ? Number(raw["terminalWsPort"]) : undefined,
     directTerminalWsPort: raw["directTerminalWsPort"] ? Number(raw["directTerminalWsPort"]) : undefined,
@@ -143,6 +144,7 @@ export function writeMetadata(
   if (metadata.agent) data["agent"] = metadata.agent;
   if (metadata.createdAt) data["createdAt"] = metadata.createdAt;
   if (metadata.runtimeHandle) data["runtimeHandle"] = metadata.runtimeHandle;
+  if (metadata.role) data["role"] = metadata.role;
   if (metadata.dashboardPort !== undefined)
     data["dashboardPort"] = String(metadata.dashboardPort);
   if (metadata.terminalWsPort !== undefined)

packages/core/src/session-manager.ts

@@ -667,6 +667,7 @@ export function createSessionManager(deps: SessionManagerDeps): SessionManager {
         worktree: project.path,
         branch: project.defaultBranch,
         status: "working",
+        role: "orchestrator",
         tmuxName,
         project: orchestratorConfig.projectId,
         createdAt: new Date().toISOString(),
@@ -831,6 +832,17 @@ export function createSessionManager(deps: SessionManagerDeps): SessionManager {
 
     for (const session of sessions) {
       try {
+        // Never clean up orchestrator sessions — they manage the lifecycle.
+        // Check explicit role metadata first, fall back to naming convention
+        // for pre-existing sessions spawned before the role field was added.
+        if (
+          session.metadata["role"] === "orchestrator" ||
+          session.id.endsWith("-orchestrator")
+        ) {
+          result.skipped.push(session.id);
+          continue;
+        }
+
         const project = config.projects[session.projectId];
         if (!project) {
           result.skipped.push(session.id);
@@ -978,6 +990,7 @@ export function createSessionManager(deps: SessionManagerDeps): SessionManager {
         worktree: raw["worktree"] ?? "",
         branch: raw["branch"] ?? "",
         status: raw["status"] ?? "killed",
+        role: raw["role"],
         tmuxName: raw["tmuxName"],
         issue: raw["issue"],
         pr: raw["pr"],

packages/core/src/types.ts

@@ -978,6 +978,7 @@ export interface SessionMetadata {
   createdAt?: string;
   runtimeHandle?: string;
   restoredAt?: string;
+  role?: string; // "orchestrator" for orchestrator sessions
   dashboardPort?: number;
   terminalWsPort?: number;
   directTerminalWsPort?: number;