diff --git a/packages/@n8n/instance-ai/src/workspace/__tests__/n8n-sandbox-sandbox.test.ts b/packages/@n8n/instance-ai/src/workspace/__tests__/n8n-sandbox-sandbox.test.ts new file mode 100644 index 00000000000..aad349a41b1 --- /dev/null +++ b/packages/@n8n/instance-ai/src/workspace/__tests__/n8n-sandbox-sandbox.test.ts @@ -0,0 +1,264 @@ +import { SandboxServiceError } from '@n8n/sandbox-client'; + +import { N8nSandboxServiceSandbox } from '../n8n-sandbox-sandbox'; + +// --------------------------------------------------------------------------- +// Mocks +// --------------------------------------------------------------------------- + +const mockCreateSandbox = jest.fn(); +const mockGetSandbox = jest.fn(); +const mockDeleteSandbox = jest.fn(); +const mockExec = jest.fn(); + +jest.mock('@n8n/sandbox-client', () => { + class MockSandboxServiceError extends Error { + readonly status: number; + + constructor(message: string, status: number) { + super(message); + this.name = 'SandboxServiceError'; + this.status = status; + } + } + + return { + SandboxServiceError: MockSandboxServiceError, + SandboxClient: class { + createSandbox = mockCreateSandbox; + getSandbox = mockGetSandbox; + deleteSandbox = mockDeleteSandbox; + exec = mockExec; + }, + }; +}); + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +function makeDefaultOptions() { + return { apiKey: 'key', serviceUrl: 'https://sandbox.test' }; +} + +function makeSandboxRecord(overrides: Record = {}) { + return { + id: 'sb-123', + status: 'running', + createdAt: 1700000000, + lastActiveAt: 1700000100, + ...overrides, + }; +} + +function makeExecResult(overrides: Record = {}) { + return { + exitCode: 0, + stdout: '', + stderr: '', + executionTimeMs: 42, + timedOut: false, + killed: false, + success: true, + ...overrides, + }; +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +beforeEach(() => { + jest.clearAllMocks(); + mockCreateSandbox.mockResolvedValue(makeSandboxRecord()); + mockGetSandbox.mockResolvedValue(makeSandboxRecord()); + mockDeleteSandbox.mockResolvedValue(undefined); + mockExec.mockResolvedValue(makeExecResult({ stdout: '/home/user\n' })); +}); + +describe('destroy()', () => { + it('calls deleteSandbox when sandbox exists', async () => { + const sandbox = new N8nSandboxServiceSandbox(makeDefaultOptions()); + await sandbox.start(); + await sandbox.destroy(); + + expect(mockDeleteSandbox).toHaveBeenCalledWith('sb-123'); + }); + + it('swallows 404 (sandbox already gone)', async () => { + mockDeleteSandbox.mockRejectedValue(new SandboxServiceError('not found', 404)); + + const sandbox = new N8nSandboxServiceSandbox(makeDefaultOptions()); + await sandbox.start(); + + await expect(sandbox.destroy()).resolves.toBeUndefined(); + }); + + it('re-throws non-404 errors', async () => { + mockDeleteSandbox.mockRejectedValue(new SandboxServiceError('server error', 500)); + + const sandbox = new N8nSandboxServiceSandbox(makeDefaultOptions()); + await sandbox.start(); + + await expect(sandbox.destroy()).rejects.toThrow('server error'); + }); + + it('is a no-op when no sandboxId is set', async () => { + const sandbox = new N8nSandboxServiceSandbox(makeDefaultOptions()); + await sandbox.destroy(); + + expect(mockDeleteSandbox).not.toHaveBeenCalled(); + }); +}); + +describe('start()', () => { + describe('fresh creation (no existing ID)', () => { + it('creates a new sandbox', async () => { + const sandbox = new N8nSandboxServiceSandbox(makeDefaultOptions()); + await sandbox.start(); + + expect(mockCreateSandbox).toHaveBeenCalledTimes(1); + expect(sandbox.id).toBe('sb-123'); + }); + }); + + describe('reconnect to existing ID', () => { + it('reconnects when sandbox exists', async () => { + const sandbox = new N8nSandboxServiceSandbox({ + ...makeDefaultOptions(), + id: 'existing-sb', + }); + mockGetSandbox.mockResolvedValue(makeSandboxRecord({ id: 'existing-sb' })); + + await sandbox.start(); + + expect(mockGetSandbox).toHaveBeenCalledWith('existing-sb'); + expect(mockCreateSandbox).not.toHaveBeenCalled(); + expect(sandbox.id).toBe('existing-sb'); + }); + + it('creates new when getSandbox returns 404', async () => { + mockGetSandbox.mockRejectedValue(new SandboxServiceError('not found', 404)); + mockCreateSandbox.mockResolvedValue(makeSandboxRecord({ id: 'new-sb' })); + + const sandbox = new N8nSandboxServiceSandbox({ + ...makeDefaultOptions(), + id: 'gone-sb', + }); + await sandbox.start(); + + expect(mockCreateSandbox).toHaveBeenCalledTimes(1); + expect(sandbox.id).toBe('new-sb'); + }); + + it('re-throws non-404 errors from getSandbox', async () => { + mockGetSandbox.mockRejectedValue(new SandboxServiceError('forbidden', 403)); + + const sandbox = new N8nSandboxServiceSandbox({ + ...makeDefaultOptions(), + id: 'existing-sb', + }); + + await expect(sandbox.start()).rejects.toThrow('forbidden'); + }); + }); +}); + +describe('getInstructions()', () => { + it('includes runtime description, working directory, and timeout', async () => { + const sandbox = new N8nSandboxServiceSandbox({ + ...makeDefaultOptions(), + timeout: 60_000, + }); + await sandbox.start(); + + const instructions = sandbox.getInstructions(); + expect(instructions).toContain('Cloud sandbox'); + expect(instructions).toContain('TypeScript'); + expect(instructions).toContain('Default working directory: /home/user/workspace'); + expect(instructions).toContain('60s'); + }); +}); + +describe('executeCommand() env merging', () => { + it('merges constructor env with per-command env', async () => { + const sandbox = new N8nSandboxServiceSandbox({ + ...makeDefaultOptions(), + env: { BASE_KEY: 'base' }, + }); + await sandbox.start(); + mockExec.mockResolvedValue(makeExecResult()); + + await sandbox.executeCommand('ls', [], { env: { CMD_KEY: 'cmd' } }); + + expect(mockExec).toHaveBeenLastCalledWith( + 'sb-123', + expect.objectContaining({ + env: { BASE_KEY: 'base', CMD_KEY: 'cmd' }, + }), + ); + }); + + it('per-command env overrides constructor env for same key', async () => { + const sandbox = new N8nSandboxServiceSandbox({ + ...makeDefaultOptions(), + env: { KEY: 'base' }, + }); + await sandbox.start(); + mockExec.mockResolvedValue(makeExecResult()); + + await sandbox.executeCommand('ls', [], { env: { KEY: 'override' } }); + + expect(mockExec).toHaveBeenLastCalledWith( + 'sb-123', + expect.objectContaining({ + env: { KEY: 'override' }, + }), + ); + }); + + it('filters out undefined values from env', async () => { + const sandbox = new N8nSandboxServiceSandbox({ + ...makeDefaultOptions(), + env: { KEEP: 'yes' }, + }); + await sandbox.start(); + mockExec.mockResolvedValue(makeExecResult()); + + await sandbox.executeCommand('ls', [], { env: { DROP: undefined } as NodeJS.ProcessEnv }); + + expect(mockExec).toHaveBeenLastCalledWith( + 'sb-123', + expect.objectContaining({ + env: { KEEP: 'yes' }, + }), + ); + }); + + it('passes undefined env when no env is configured', async () => { + const sandbox = new N8nSandboxServiceSandbox(makeDefaultOptions()); + await sandbox.start(); + mockExec.mockResolvedValue(makeExecResult()); + + await sandbox.executeCommand('ls'); + + expect(mockExec).toHaveBeenLastCalledWith( + 'sb-123', + expect.objectContaining({ env: undefined }), + ); + }); +}); + +describe('getInfo()', () => { + it('includes locally tracked createdAt and workingDirectory', async () => { + const sandbox = new N8nSandboxServiceSandbox(makeDefaultOptions()); + await sandbox.start(); + + const info = await sandbox.getInfo(); + expect(info.createdAt).toBeInstanceOf(Date); + expect(info.metadata).toEqual( + expect.objectContaining({ workingDirectory: '/home/user/workspace' }), + ); + expect(info.provider).toBe('n8n-sandbox'); + }); +}); diff --git a/packages/@n8n/instance-ai/src/workspace/builder-sandbox-factory.ts b/packages/@n8n/instance-ai/src/workspace/builder-sandbox-factory.ts index f9aa104d8b2..a9de9617afe 100644 --- a/packages/@n8n/instance-ai/src/workspace/builder-sandbox-factory.ts +++ b/packages/@n8n/instance-ai/src/workspace/builder-sandbox-factory.ts @@ -367,7 +367,7 @@ export class BuilderSandboxFactory { } private async createN8nSandbox( - _builderId: string, + builderId: string, context: InstanceAiContext, ): Promise { const config = this.assertIsN8nSandbox(); @@ -419,6 +419,13 @@ export class BuilderSandboxFactory { } catch (error) { // If any step after sandbox creation throws (workspace init, catalog // write, SDK link), destroy the remote sandbox so it isn't orphaned. + this.errorReporter?.error(error, { + tags: { + component: 'builder-sandbox-factory', + provider: 'n8n-sandbox', + }, + extra: { builderId }, + }); await destroySandbox(); throw error; } diff --git a/packages/@n8n/instance-ai/src/workspace/n8n-sandbox-sandbox.ts b/packages/@n8n/instance-ai/src/workspace/n8n-sandbox-sandbox.ts index 0072ad49503..a3712c3708a 100644 --- a/packages/@n8n/instance-ai/src/workspace/n8n-sandbox-sandbox.ts +++ b/packages/@n8n/instance-ai/src/workspace/n8n-sandbox-sandbox.ts @@ -5,7 +5,7 @@ import type { SandboxInfo, } from '@n8n/agents'; import { BaseSandbox } from '@n8n/agents'; -import { SandboxClient } from '@n8n/sandbox-client'; +import { SandboxClient, SandboxServiceError, type SandboxRecord } from '@n8n/sandbox-client'; import assert from 'node:assert/strict'; import { randomUUID } from 'node:crypto'; @@ -14,6 +14,7 @@ export interface N8nSandboxServiceSandboxOptions { apiKey?: string; serviceUrl?: string; timeout?: number; + env?: Record; } function shellEscape(value: string): string { @@ -37,8 +38,16 @@ export class N8nSandboxServiceSandbox extends BaseSandbox { private readonly client: SandboxClient; + private readonly timeout: number; + + private static readonly HOME_DIR = '/home/user'; + + private static readonly WORKSPACE_DIR = `${N8nSandboxServiceSandbox.HOME_DIR}/workspace`; + private sandboxId?: string; + private createdAt?: Date; + constructor(private readonly options: N8nSandboxServiceSandboxOptions) { super(); this.client = new SandboxClient({ @@ -46,6 +55,7 @@ export class N8nSandboxServiceSandbox extends BaseSandbox { baseUrl: options.serviceUrl, }); this.sandboxId = options.id; + this.timeout = options.timeout ?? 300_000; } get id(): string { @@ -54,17 +64,26 @@ export class N8nSandboxServiceSandbox extends BaseSandbox { override async start(): Promise { if (this.sandboxId) { - await this.client.getSandbox(this.sandboxId); - return; + const existing = await this.tryGetExistingSandbox(this.sandboxId); + if (existing) { + this.createdAt = new Date(existing.createdAt * 1000); + return; + } } const sandbox = await this.client.createSandbox(); this.sandboxId = sandbox.id; + this.createdAt = new Date(); } override async destroy(): Promise { if (!this.sandboxId) return; - await this.client.deleteSandbox(this.sandboxId); + try { + await this.client.deleteSandbox(this.sandboxId); + } catch (error) { + if (error instanceof SandboxServiceError && error.status === 404) return; + throw error; + } } override async stop(): Promise { @@ -80,14 +99,23 @@ export class N8nSandboxServiceSandbox extends BaseSandbox { name: this.name, provider: this.provider, status: this.status, - createdAt: new Date(sandbox.createdAt * 1000), + createdAt: this.createdAt ?? new Date(sandbox.createdAt * 1000), metadata: { lastActiveAt: new Date(sandbox.lastActiveAt * 1000).toISOString(), remoteStatus: sandbox.status, + workingDirectory: N8nSandboxServiceSandbox.WORKSPACE_DIR, }, }; } + override getInstructions(): string { + return [ + 'Cloud sandbox with isolated execution (TypeScript runtime).', + `Default working directory: ${N8nSandboxServiceSandbox.WORKSPACE_DIR}.`, + `Command timeout: ${Math.ceil(this.timeout / 1000)}s.`, + ].join(' '); + } + override async executeCommand( command: string, args: string[] = [], @@ -96,9 +124,9 @@ export class N8nSandboxServiceSandbox extends BaseSandbox { await this.ensureRunning(); const result = await this.client.exec(this.requireSandboxId(), { command: toShellCommand(command, args), - env: options?.env, + env: this.compactEnv(options?.env), workdir: options?.cwd, - timeoutMs: options?.timeout ?? this.options.timeout, + timeoutMs: options?.timeout ?? this.timeout, abortSignal: options?.abortSignal, onStdout: options?.onStdout, onStderr: options?.onStderr, @@ -121,6 +149,28 @@ export class N8nSandboxServiceSandbox extends BaseSandbox { return this.client; } + /** Returns the remote sandbox record, or `null` if it no longer exists (404). */ + private async tryGetExistingSandbox(sandboxId: string): Promise { + try { + return await this.client.getSandbox(sandboxId); + } catch (error) { + if (error instanceof SandboxServiceError && error.status === 404) return null; + throw error; + } + } + + /** Merges constructor-level env with per-command env, filtering out undefined values. */ + private compactEnv(env: NodeJS.ProcessEnv | undefined): Record | undefined { + const merged = { + ...this.options.env, + ...env, + }; + const entries = Object.entries(merged).filter( + (entry): entry is [string, string] => typeof entry[1] === 'string', + ); + return entries.length > 0 ? Object.fromEntries(entries) : undefined; + } + private requireSandboxId(): string { assert(this.sandboxId, 'Sandbox has not been created yet'); return this.sandboxId;