diff --git a/packages/cli/__tests__/commands/session.test.ts b/packages/cli/__tests__/commands/session.test.ts index 7387160ff..8909add8d 100644 --- a/packages/cli/__tests__/commands/session.test.ts +++ b/packages/cli/__tests__/commands/session.test.ts @@ -565,6 +565,77 @@ describe("session cleanup", () => { expect(output).toContain("Cleaned: app-2"); }); + it("suppresses orchestrator cleanup output while preserving worker cleanup output", async () => { + mockSessionManager.cleanup.mockResolvedValue({ + killed: ["app-orchestrator", "app-2"], + skipped: [], + errors: [{ sessionId: "app-orchestrator", error: "should never surface" }], + } satisfies CleanupResult); + + await program.parseAsync(["node", "test", "session", "cleanup"]); + + const output = consoleSpy.mock.calls.map((c) => String(c[0])).join("\n"); + const errOutput = vi + .mocked(console.error) + .mock.calls.map((c) => String(c[0])) + .join("\n"); + + expect(output).toContain("Cleaned: app-2"); + expect(output).not.toContain("app-orchestrator"); + expect(output).toContain("Cleanup complete. 1 sessions cleaned"); + expect(errOutput).not.toContain("app-orchestrator"); + }); + + it("treats orchestrator-only cleanup results as no-op output", async () => { + mockSessionManager.cleanup.mockResolvedValue({ + killed: ["app-orchestrator"], + skipped: [], + errors: [], + } satisfies CleanupResult); + + await program.parseAsync(["node", "test", "session", "cleanup"]); + + const output = consoleSpy.mock.calls.map((c) => String(c[0])).join("\n"); + expect(output).toContain("No sessions to clean up"); + expect(output).not.toContain("app-orchestrator"); + }); + + it("suppresses orchestrators in cleanup dry-run output", async () => { + mockSessionManager.cleanup.mockResolvedValue({ + killed: ["app-orchestrator", "app-3"], + skipped: [], + errors: [], + } satisfies CleanupResult); + + await program.parseAsync(["node", "test", "session", "cleanup", "--dry-run"]); + + const output = consoleSpy.mock.calls.map((c) => String(c[0])).join("\n"); + expect(output).toContain("Would kill app-3"); + expect(output).not.toContain("app-orchestrator"); + expect(output).toContain("1 session would be cleaned"); + }); + + it("suppresses project-prefixed orchestrator cleanup results", async () => { + mockSessionManager.cleanup.mockResolvedValue({ + killed: ["my-app:app-orchestrator", "my-app:app-4"], + skipped: [], + errors: [{ sessionId: "my-app:app-orchestrator", error: "should never surface" }], + } satisfies CleanupResult); + + await program.parseAsync(["node", "test", "session", "cleanup"]); + + const output = consoleSpy.mock.calls.map((c) => String(c[0])).join("\n"); + const errOutput = vi + .mocked(console.error) + .mock.calls.map((c) => String(c[0])) + .join("\n"); + + expect(output).toContain("Cleaned: my-app:app-4"); + expect(output).not.toContain("my-app:app-orchestrator"); + expect(output).toContain("Cleanup complete. 1 sessions cleaned"); + expect(errOutput).not.toContain("my-app:app-orchestrator"); + }); + it("skips sessions without metadata", async () => { // No metadata files exist — list returns empty, cleanup returns empty mockSessionManager.cleanup.mockResolvedValue({ diff --git a/packages/cli/__tests__/lib/session-utils.test.ts b/packages/cli/__tests__/lib/session-utils.test.ts index 4a3f2ac39..5d42dadab 100644 --- a/packages/cli/__tests__/lib/session-utils.test.ts +++ b/packages/cli/__tests__/lib/session-utils.test.ts @@ -1,5 +1,10 @@ import { describe, it, expect } from "vitest"; -import { escapeRegex, matchesPrefix, findProjectForSession } from "../../src/lib/session-utils.js"; +import { + escapeRegex, + matchesPrefix, + findProjectForSession, + isOrchestratorSessionName, +} from "../../src/lib/session-utils.js"; import type { OrchestratorConfig } from "@composio/ao-core"; describe("escapeRegex", () => { @@ -103,3 +108,42 @@ describe("findProjectForSession", () => { expect(findProjectForSession(config, "b-2")).toBe("beta"); }); }); + +describe("isOrchestratorSessionName", () => { + const makeConfig = (projects: Record): OrchestratorConfig => + ({ + dataDir: "/tmp", + worktreeDir: "/tmp/wt", + port: 3000, + defaults: { runtime: "tmux", agent: "claude-code", workspace: "worktree", notifiers: [] }, + projects: Object.fromEntries( + Object.entries(projects).map(([id, p]) => [ + id, + { name: id, repo: "", path: "", defaultBranch: "main", ...p }, + ]), + ), + notifiers: {}, + notificationRouting: {}, + reactions: {}, + }) as OrchestratorConfig; + + it("matches the canonical orchestrator ID for a known project", () => { + const config = makeConfig({ "my-app": { sessionPrefix: "app" } }); + expect(isOrchestratorSessionName(config, "app-orchestrator", "my-app")).toBe(true); + }); + + it("matches orchestrator IDs across configured projects without an explicit project", () => { + const config = makeConfig({ "my-app": { sessionPrefix: "app" } }); + expect(isOrchestratorSessionName(config, "app-orchestrator")).toBe(true); + }); + + it("falls back to suffix detection for legacy orchestrator names", () => { + const config = makeConfig({ "my-app": { sessionPrefix: "app" } }); + expect(isOrchestratorSessionName(config, "legacy-orchestrator")).toBe(true); + }); + + it("does not classify worker session IDs as orchestrators", () => { + const config = makeConfig({ "my-app": { sessionPrefix: "app" } }); + expect(isOrchestratorSessionName(config, "app-12", "my-app")).toBe(false); + }); +}); diff --git a/packages/cli/src/commands/session.ts b/packages/cli/src/commands/session.ts index a6c00b19b..0c6cf88d5 100644 --- a/packages/cli/src/commands/session.ts +++ b/packages/cli/src/commands/session.ts @@ -5,6 +5,7 @@ import { loadConfig, SessionNotRestorableError, WorkspaceMissingError } from "@c import { git, getTmuxActivity, tmux } from "../lib/shell.js"; import { formatAge } from "../lib/format.js"; import { getSessionManager } from "../lib/create-session-manager.js"; +import { isOrchestratorSessionName } from "../lib/session-utils.js"; export function registerSession(program: Command): void { const session = program @@ -145,10 +146,31 @@ export function registerSession(program: Command): void { const sm = await getSessionManager(config); + const filterCleanupIds = (ids: string[]): string[] => + ids.filter((entry) => { + const separator = entry.indexOf(":"); + const entryProjectId = separator === -1 ? opts.project : entry.slice(0, separator); + const sessionId = separator === -1 ? entry : entry.slice(separator + 1); + return !isOrchestratorSessionName(config, sessionId, entryProjectId); + }); + + const filterCleanupErrors = (errors: Array<{ sessionId: string; error: string }>) => + errors.filter(({ sessionId }) => { + const separator = sessionId.indexOf(":"); + const entryProjectId = separator === -1 ? opts.project : sessionId.slice(0, separator); + const normalizedSessionId = separator === -1 ? sessionId : sessionId.slice(separator + 1); + return !isOrchestratorSessionName(config, normalizedSessionId, entryProjectId); + }); + if (opts.dryRun) { // Dry-run delegates to sm.cleanup() with dryRun flag so it uses the // same live checks (PR state, runtime alive, tracker) as actual cleanup. - const result = await sm.cleanup(opts.project, { dryRun: true }); + const rawResult = await sm.cleanup(opts.project, { dryRun: true }); + const result = { + ...rawResult, + killed: filterCleanupIds(rawResult.killed), + errors: filterCleanupErrors(rawResult.errors), + }; if (result.errors.length > 0) { for (const { sessionId, error } of result.errors) { @@ -171,7 +193,12 @@ export function registerSession(program: Command): void { } } } else { - const result = await sm.cleanup(opts.project); + const rawResult = await sm.cleanup(opts.project); + const result = { + ...rawResult, + killed: filterCleanupIds(rawResult.killed), + errors: filterCleanupErrors(rawResult.errors), + }; if (result.killed.length === 0 && result.errors.length === 0) { console.log(chalk.dim(" No sessions to clean up.")); diff --git a/packages/cli/src/lib/session-utils.ts b/packages/cli/src/lib/session-utils.ts index 44d35a738..2837afdb2 100644 --- a/packages/cli/src/lib/session-utils.ts +++ b/packages/cli/src/lib/session-utils.ts @@ -14,7 +14,9 @@ export function findProjectForSession( config: OrchestratorConfig, sessionName: string, ): string | null { - for (const [id, project] of Object.entries(config.projects)) { + for (const [id, project] of Object.entries(config.projects) as Array< + [string, OrchestratorConfig["projects"][string]] + >) { const prefix = project.sessionPrefix || id; if (matchesPrefix(sessionName, prefix)) { return id; @@ -22,3 +24,27 @@ export function findProjectForSession( } return null; } + +export function isOrchestratorSessionName( + config: OrchestratorConfig, + sessionName: string, + projectId?: string, +): boolean { + if (projectId) { + const project = config.projects[projectId]; + if (project && sessionName === `${project.sessionPrefix || projectId}-orchestrator`) { + return true; + } + } + + for (const [id, project] of Object.entries(config.projects) as Array< + [string, OrchestratorConfig["projects"][string]] + >) { + const prefix = project.sessionPrefix || id; + if (sessionName === `${prefix}-orchestrator`) { + return true; + } + } + + return sessionName.endsWith("-orchestrator"); +} diff --git a/packages/core/src/__tests__/session-manager.test.ts b/packages/core/src/__tests__/session-manager.test.ts index f2569af70..b764cc66a 100644 --- a/packages/core/src/__tests__/session-manager.test.ts +++ b/packages/core/src/__tests__/session-manager.test.ts @@ -2289,6 +2289,101 @@ describe("cleanup", () => { expect(result.skipped).toContain("app-orchestrator"); }); + it("never cleans the canonical orchestrator session even with stale worker-like metadata", async () => { + const deleteLogPath = join(tmpDir, "opencode-delete-orchestrator.log"); + const mockBin = installMockOpencode("[]", deleteLogPath); + process.env.PATH = `${mockBin}:${originalPath ?? ""}`; + + const deadRuntime: Runtime = { + ...mockRuntime, + isAlive: vi.fn().mockResolvedValue(false), + }; + const mockSCM: SCM = { + name: "mock-scm", + detectPR: vi.fn(), + getPRState: vi.fn().mockResolvedValue("merged"), + mergePR: vi.fn(), + closePR: vi.fn(), + getCIChecks: vi.fn(), + getCISummary: vi.fn(), + getReviews: vi.fn(), + getReviewDecision: vi.fn(), + getPendingComments: vi.fn(), + getAutomatedComments: vi.fn(), + getMergeability: vi.fn(), + }; + const mockTracker: Tracker = { + name: "mock-tracker", + getIssue: vi.fn().mockResolvedValue({ + id: "INT-42", + title: "Issue", + description: "", + url: "https://example.com/INT-42", + state: "closed", + labels: [], + }), + isCompleted: vi.fn().mockResolvedValue(true), + issueUrl: vi.fn().mockReturnValue("https://example.com/INT-42"), + branchName: vi.fn().mockReturnValue("feat/INT-42"), + generatePrompt: vi.fn().mockResolvedValue(""), + }; + const registryWithSignals: PluginRegistry = { + ...mockRegistry, + get: vi.fn().mockImplementation((slot: string) => { + if (slot === "runtime") return deadRuntime; + if (slot === "agent") return mockAgent; + if (slot === "workspace") return mockWorkspace; + if (slot === "scm") return mockSCM; + if (slot === "tracker") return mockTracker; + return null; + }), + }; + + writeMetadata(sessionsDir, "app-orchestrator", { + worktree: "/tmp", + branch: "main", + status: "ci_failed", + project: "my-app", + issue: "INT-42", + pr: "https://github.com/org/repo/pull/10", + agent: "opencode", + opencodeSessionId: "ses_orchestrator_active", + runtimeHandle: JSON.stringify(makeHandle("rt-orchestrator")), + }); + + const sm = createSessionManager({ config, registry: registryWithSignals }); + const result = await sm.cleanup(); + + expect(result.killed).not.toContain("app-orchestrator"); + expect(result.skipped).toContain("app-orchestrator"); + expect(existsSync(deleteLogPath)).toBe(false); + }); + + it("never cleans archived orchestrator mappings even when metadata looks stale", async () => { + const deleteLogPath = join(tmpDir, "opencode-delete-archived-orchestrator.log"); + const mockBin = installMockOpencode("[]", deleteLogPath); + process.env.PATH = `${mockBin}:${originalPath ?? ""}`; + + writeMetadata(sessionsDir, "app-orchestrator", { + worktree: "/tmp", + branch: "main", + status: "killed", + project: "my-app", + agent: "opencode", + opencodeSessionId: "ses_orchestrator_archived", + pr: "https://github.com/org/repo/pull/88", + runtimeHandle: JSON.stringify(makeHandle("rt-orchestrator")), + }); + deleteMetadata(sessionsDir, "app-orchestrator", true); + + const sm = createSessionManager({ config, registry: mockRegistry }); + const result = await sm.cleanup(); + + expect(result.killed).not.toContain("app-orchestrator"); + expect(result.skipped).toContain("app-orchestrator"); + expect(existsSync(deleteLogPath)).toBe(false); + }); + it("kills sessions with dead runtimes", async () => { const deadRuntime: Runtime = { ...mockRuntime, diff --git a/packages/core/src/session-manager.ts b/packages/core/src/session-manager.ts index 99741a770..f22514607 100644 --- a/packages/core/src/session-manager.ts +++ b/packages/core/src/session-manager.ts @@ -363,6 +363,18 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM return raw["role"] === "orchestrator" || sessionId.endsWith("-orchestrator"); } + function isCleanupProtectedSession( + project: ProjectConfig, + sessionId: string, + metadata?: Record | null, + ): boolean { + const canonicalOrchestratorId = `${project.sessionPrefix}-orchestrator`; + return ( + sessionId === canonicalOrchestratorId || + isOrchestratorSession({ id: sessionId, metadata: metadata ?? undefined }) + ); + } + function applyMetadataUpdatesToRaw( raw: Record, updates: Partial>, @@ -1643,16 +1655,13 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM 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 (isOrchestratorSession(session)) { + const project = config.projects[session.projectId]; + if (!project) { pushSkipped(session.projectId, session.id); continue; } - const project = config.projects[session.projectId]; - if (!project) { + if (isCleanupProtectedSession(project, session.id, session.metadata)) { pushSkipped(session.projectId, session.id); continue; } @@ -1718,7 +1727,7 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM const archived = readArchivedMetadataRaw(sessionsDir, archivedId); if (!archived) continue; - if (isOrchestratorSession({ id: archivedId, metadata: archived })) { + if (isCleanupProtectedSession(project, archivedId, archived)) { pushSkipped(projectKey, archivedId); continue; }