Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 71 additions & 0 deletions packages/cli/__tests__/commands/session.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
46 changes: 45 additions & 1 deletion packages/cli/__tests__/lib/session-utils.test.ts
Original file line number Diff line number Diff line change
@@ -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", () => {
Expand Down Expand Up @@ -103,3 +108,42 @@ describe("findProjectForSession", () => {
expect(findProjectForSession(config, "b-2")).toBe("beta");
});
});

describe("isOrchestratorSessionName", () => {
const makeConfig = (projects: Record<string, { sessionPrefix?: string }>): 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);
});
});
31 changes: 29 additions & 2 deletions packages/cli/src/commands/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand All @@ -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."));
Expand Down
28 changes: 27 additions & 1 deletion packages/cli/src/lib/session-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,37 @@ 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;
}
}
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");
}
95 changes: 95 additions & 0 deletions packages/core/src/__tests__/session-manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
23 changes: 16 additions & 7 deletions packages/core/src/session-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string> | null,
): boolean {
const canonicalOrchestratorId = `${project.sessionPrefix}-orchestrator`;
return (
sessionId === canonicalOrchestratorId ||
isOrchestratorSession({ id: sessionId, metadata: metadata ?? undefined })
);
}

function applyMetadataUpdatesToRaw(
raw: Record<string, string>,
updates: Partial<Record<string, string>>,
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down
Loading