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
6 changes: 5 additions & 1 deletion src/agents/shared/cleanup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,12 @@ export function cleanupAgentResources(
clearWatchdogCleanup();

const isLocalMode = process.env.CASCADE_LOCAL_MODE === 'true';
// Preserve the workspace when the router will commit this container to a snapshot image.
// If the workspace is deleted before process exit, docker commit captures an empty
// /workspace and the snapshot fails to provide any reuse benefit.
const isSnapshotEnabled = process.env.CASCADE_SNAPSHOT_ENABLED === 'true';

if (repoDir && !isLocalMode && !skipRepoDeletion) {
if (repoDir && !isLocalMode && !skipRepoDeletion && !isSnapshotEnabled) {
try {
cleanupTempDir(repoDir);
} catch (err) {
Expand Down
9 changes: 7 additions & 2 deletions src/router/container-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,12 @@ export async function spawnWorker(job: Job<CascadeJob>): Promise<void> {
// A snapshot is being reused when snapshotEnabled and the image differs from the base image.
const snapshotReuse = snapshotEnabled && workerImage !== routerConfig.workerImage;

const workerEnv = await buildWorkerEnvWithProjectId(job, projectId, snapshotReuse);
const workerEnv = await buildWorkerEnvWithProjectId(
job,
projectId,
snapshotReuse,
snapshotEnabled,
);
const hasCredentials = workerEnv.some((e) => e.startsWith('CASCADE_CREDENTIAL_KEYS='));

logger.info('[WorkerManager] Spawning worker:', {
Expand Down Expand Up @@ -418,7 +423,7 @@ export async function spawnWorker(job: Job<CascadeJob>): Promise<void> {
{ jobId, staleImage: workerImage },
);
invalidateSnapshot(projectId, workItemId);
const fallbackEnv = await buildWorkerEnvWithProjectId(job, projectId, false);
const fallbackEnv = await buildWorkerEnvWithProjectId(job, projectId, false, snapshotEnabled);
const fallbackConfig: ContainerLaunchConfig = {
workerImage: routerConfig.workerImage,
snapshotEnabled,
Expand Down
11 changes: 11 additions & 0 deletions src/router/worker-env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,15 @@ function appendOptionalEnvVars(env: string[]): void {
*
* @param snapshotReuse - When true, injects CASCADE_SNAPSHOT_REUSE=true so the
* worker knows to refresh an existing workspace instead of cloning from scratch.
* @param snapshotEnabled - When true, injects CASCADE_SNAPSHOT_ENABLED=true so the
* worker preserves its workspace on exit (the router will commit the container to
* a snapshot image after the worker exits).
*/
export async function buildWorkerEnvWithProjectId(
job: Job<CascadeJob>,
projectId: string | null,
snapshotReuse = false,
snapshotEnabled = false,
): Promise<string[]> {
const env: string[] = [
`JOB_ID=${job.id}`,
Expand All @@ -106,6 +110,13 @@ export async function buildWorkerEnvWithProjectId(
env.push('CASCADE_SNAPSHOT_REUSE=true');
}

// Signal that this container will be committed to a snapshot image after exit.
// The worker must NOT delete its workspace so the directory is present when the
// router calls docker commit (after container.wait() resolves).
if (snapshotEnabled) {
env.push('CASCADE_SNAPSHOT_ENABLED=true');
}

// Resolve project credentials in the router and set as individual env vars.
// NOTE: CREDENTIAL_MASTER_KEY is intentionally NOT passed to workers.
if (projectId) {
Expand Down
120 changes: 120 additions & 0 deletions tests/unit/agents/cleanup.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';

// ---------------------------------------------------------------------------
// Module-level mocks
// ---------------------------------------------------------------------------

const mockCleanupTempDir = vi.fn();
const mockCleanupLogFile = vi.fn();
const mockCleanupLogDirectory = vi.fn();
const mockClearWatchdogCleanup = vi.fn();

vi.mock('../../../src/utils/repo.js', () => ({
cleanupTempDir: (...args: unknown[]) => mockCleanupTempDir(...args),
}));

vi.mock('../../../src/utils/fileLogger.js', () => ({
cleanupLogFile: (...args: unknown[]) => mockCleanupLogFile(...args),
cleanupLogDirectory: (...args: unknown[]) => mockCleanupLogDirectory(...args),
}));

vi.mock('../../../src/utils/lifecycle.js', () => ({
clearWatchdogCleanup: () => mockClearWatchdogCleanup(),
}));

vi.mock('../../../src/utils/logging.js', () => ({
logger: { warn: vi.fn(), info: vi.fn(), error: vi.fn(), debug: vi.fn() },
}));

// ---------------------------------------------------------------------------
// Imports (after mocks)
// ---------------------------------------------------------------------------

import { cleanupAgentResources } from '../../../src/agents/shared/cleanup.js';

// ---------------------------------------------------------------------------
// Helpers
// ---------------------------------------------------------------------------

function makeFakeFileLogger() {
return {
logPath: '/tmp/cascade-test.log',
engineLogPath: '/tmp/cascade-test-engine.log',
llmCallLogger: { logDir: '/tmp/cascade-test-llm' },
} as never;
}

// ---------------------------------------------------------------------------
// Tests
// ---------------------------------------------------------------------------

describe('cleanupAgentResources', () => {
beforeEach(() => {
mockCleanupTempDir.mockClear();
mockCleanupLogFile.mockClear();
mockCleanupLogDirectory.mockClear();
mockClearWatchdogCleanup.mockClear();
process.env.CASCADE_LOCAL_MODE = undefined;
process.env.CASCADE_SNAPSHOT_ENABLED = undefined;
});

afterEach(() => {
process.env.CASCADE_LOCAL_MODE = undefined;
process.env.CASCADE_SNAPSHOT_ENABLED = undefined;
});

it('deletes workspace and log files on a normal run', () => {
cleanupAgentResources('/workspace/cascade-proj-123', makeFakeFileLogger());
expect(mockCleanupTempDir).toHaveBeenCalledWith('/workspace/cascade-proj-123');
expect(mockCleanupLogFile).toHaveBeenCalledTimes(2);
expect(mockCleanupLogDirectory).toHaveBeenCalledTimes(1);
});

it('skips workspace deletion when CASCADE_SNAPSHOT_ENABLED=true', () => {
process.env.CASCADE_SNAPSHOT_ENABLED = 'true';
cleanupAgentResources('/workspace/cascade-proj-123', makeFakeFileLogger());
expect(mockCleanupTempDir).not.toHaveBeenCalled();
});

it('still cleans up log files when CASCADE_SNAPSHOT_ENABLED=true', () => {
process.env.CASCADE_SNAPSHOT_ENABLED = 'true';
cleanupAgentResources('/workspace/cascade-proj-123', makeFakeFileLogger());
expect(mockCleanupLogFile).toHaveBeenCalledTimes(2);
expect(mockCleanupLogDirectory).toHaveBeenCalledTimes(1);
});

it('skips workspace deletion when skipRepoDeletion=true', () => {
cleanupAgentResources('/workspace/cascade-proj-123', makeFakeFileLogger(), true);
expect(mockCleanupTempDir).not.toHaveBeenCalled();
});

it('skips workspace deletion when CASCADE_LOCAL_MODE=true', () => {
process.env.CASCADE_LOCAL_MODE = 'true';
cleanupAgentResources('/workspace/cascade-proj-123', makeFakeFileLogger());
expect(mockCleanupTempDir).not.toHaveBeenCalled();
// Log files are also skipped in local mode
expect(mockCleanupLogFile).not.toHaveBeenCalled();
});

it('does nothing when repoDir is null', () => {
cleanupAgentResources(null, makeFakeFileLogger());
expect(mockCleanupTempDir).not.toHaveBeenCalled();
// Log files are still cleaned up
expect(mockCleanupLogFile).toHaveBeenCalledTimes(2);
});

it('always calls clearWatchdogCleanup', () => {
cleanupAgentResources(null, makeFakeFileLogger());
expect(mockClearWatchdogCleanup).toHaveBeenCalledOnce();
});

it('logs a warning when cleanupTempDir throws', () => {
mockCleanupTempDir.mockImplementation(() => {
throw new Error('permission denied');
});
// Should not throw
expect(() =>
cleanupAgentResources('/workspace/cascade-proj-123', makeFakeFileLogger()),
).not.toThrow();
});
});
32 changes: 32 additions & 0 deletions tests/unit/router/worker-env.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -299,3 +299,35 @@ describe('buildWorkerEnvWithProjectId — snapshotReuse flag', () => {
expect(env).toContain('REDIS_URL=redis://localhost:6379');
});
});

// ---------------------------------------------------------------------------
// buildWorkerEnvWithProjectId — snapshotEnabled flag
// ---------------------------------------------------------------------------

describe('buildWorkerEnvWithProjectId — snapshotEnabled flag', () => {
beforeEach(() => {
mockGetAllProjectCredentials.mockResolvedValue({});
});

it('omits CASCADE_SNAPSHOT_ENABLED when snapshotEnabled=false (default)', async () => {
const env = await buildWorkerEnvWithProjectId(makeJob() as never, 'proj-1');
expect(env.some((e) => e.startsWith('CASCADE_SNAPSHOT_ENABLED='))).toBe(false);
});

it('includes CASCADE_SNAPSHOT_ENABLED=true when snapshotEnabled=true', async () => {
const env = await buildWorkerEnvWithProjectId(makeJob() as never, 'proj-1', false, true);
expect(env).toContain('CASCADE_SNAPSHOT_ENABLED=true');
});

it('can combine CASCADE_SNAPSHOT_REUSE and CASCADE_SNAPSHOT_ENABLED', async () => {
const env = await buildWorkerEnvWithProjectId(makeJob() as never, 'proj-1', true, true);
expect(env).toContain('CASCADE_SNAPSHOT_REUSE=true');
expect(env).toContain('CASCADE_SNAPSHOT_ENABLED=true');
});

it('omits CASCADE_SNAPSHOT_ENABLED when snapshotReuse=true but snapshotEnabled=false', async () => {
const env = await buildWorkerEnvWithProjectId(makeJob() as never, 'proj-1', true, false);
expect(env).toContain('CASCADE_SNAPSHOT_REUSE=true');
expect(env.some((e) => e.startsWith('CASCADE_SNAPSHOT_ENABLED='))).toBe(false);
});
});
Loading