From 0e7e49bdc955f5b55ba77e46206705ca410ce99e Mon Sep 17 00:00:00 2001 From: oleg Date: Mon, 10 Aug 2026 16:09:06 +0200 Subject: [PATCH] fix(core): Reattach n8n-sandbox thread sandboxes after restart, idle eviction, and across mains (#35953) --- packages/@n8n/agents/package.json | 2 +- .../sandbox/create-workspace.test.ts | 15 +++ .../sandbox/n8n-sandbox-sandbox.test.ts | 45 +++++---- .../src/workspace/sandbox/create-workspace.ts | 1 + .../workspace/sandbox/n8n-sandbox-sandbox.ts | 31 ++---- .../agents/src/workspace/sandbox/types.ts | 6 ++ packages/@n8n/instance-ai/docs/sandboxing.md | 2 +- .../instance-ai-sandbox.service.test.ts | 97 +++++++++++++++++++ .../sandbox/instance-ai-sandbox.service.ts | 83 ++++++++++++++-- pnpm-lock.yaml | 10 +- 10 files changed, 232 insertions(+), 60 deletions(-) diff --git a/packages/@n8n/agents/package.json b/packages/@n8n/agents/package.json index a3ff85ac677..bc0f2ab0834 100644 --- a/packages/@n8n/agents/package.json +++ b/packages/@n8n/agents/package.json @@ -108,7 +108,7 @@ "@daytona/sdk": "catalog:", "@modelcontextprotocol/sdk": "catalog:", "@n8n/ai-utilities": "workspace:*", - "@n8n/sandbox-client": "0.0.4", + "@n8n/sandbox-client": "0.1.0", "@n8n/utils": "workspace:*", "@openrouter/ai-sdk-provider": "catalog:", "@opentelemetry/exporter-trace-otlp-http": ">=0.50.0", diff --git a/packages/@n8n/agents/src/__tests__/workspace/sandbox/create-workspace.test.ts b/packages/@n8n/agents/src/__tests__/workspace/sandbox/create-workspace.test.ts index 63bde164396..c529311d4a6 100644 --- a/packages/@n8n/agents/src/__tests__/workspace/sandbox/create-workspace.test.ts +++ b/packages/@n8n/agents/src/__tests__/workspace/sandbox/create-workspace.test.ts @@ -135,6 +135,21 @@ describe('createSandbox', () => { expect(result?.provider).toBe('n8n-sandbox'); }); + it('passes the configured id through to the N8nSandboxServiceSandbox', async () => { + const config: SandboxConfig = { + enabled: true, + provider: 'n8n-sandbox', + serviceUrl: 'https://sandbox.example.com', + id: '11111111-1111-4111-8111-111111111111', + timeout: 45_000, + }; + + const result = await createSandbox(config); + + expect(result).toBeInstanceOf(N8nSandboxServiceSandbox); + expect(result?.id).toBe('11111111-1111-4111-8111-111111111111'); + }); + it('does not include sandbox secrets in unsupported provider errors', async () => { const config = { enabled: true, diff --git a/packages/@n8n/agents/src/__tests__/workspace/sandbox/n8n-sandbox-sandbox.test.ts b/packages/@n8n/agents/src/__tests__/workspace/sandbox/n8n-sandbox-sandbox.test.ts index e81a7c9413e..9fade2cd8e9 100644 --- a/packages/@n8n/agents/src/__tests__/workspace/sandbox/n8n-sandbox-sandbox.test.ts +++ b/packages/@n8n/agents/src/__tests__/workspace/sandbox/n8n-sandbox-sandbox.test.ts @@ -113,46 +113,45 @@ describe('destroy()', () => { describe('start()', () => { describe('fresh creation (no existing ID)', () => { - it('creates a new sandbox', async () => { + it('creates a new sandbox with a service-generated id', async () => { const sandbox = new N8nSandboxServiceSandbox(makeDefaultOptions()); await sandbox.start(); expect(mockCreateSandbox).toHaveBeenCalledTimes(1); + expect(mockCreateSandbox).toHaveBeenCalledWith(undefined); expect(sandbox.id).toBe('sb-123'); }); }); - describe('reconnect to existing ID', () => { - it('reconnects when sandbox exists', async () => { + describe('create-or-reconnect with a configured id', () => { + it('passes the configured id so the service creates or reconnects', async () => { + mockCreateSandbox.mockResolvedValue(makeSandboxRecord({ id: 'existing-sb' })); + 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'); + expect(mockCreateSandbox).toHaveBeenCalledWith({ id: 'existing-sb' }); + expect(sandbox.id).toBe('existing-sb'); }); - it('re-throws non-404 errors from getSandbox', async () => { - mockGetSandbox.mockRejectedValue(new SandboxServiceError('forbidden', 403)); + it('reuses the last known id when restarted after a stop', async () => { + mockCreateSandbox.mockResolvedValue(makeSandboxRecord({ id: 'sb-123' })); + + const sandbox = new N8nSandboxServiceSandbox(makeDefaultOptions()); + await sandbox.start(); + await sandbox.stop(); + await sandbox.start(); + + expect(mockCreateSandbox).toHaveBeenLastCalledWith({ id: 'sb-123' }); + expect(sandbox.id).toBe('sb-123'); + }); + + it('re-throws createSandbox errors', async () => { + mockCreateSandbox.mockRejectedValue(new SandboxServiceError('forbidden', 403)); const sandbox = new N8nSandboxServiceSandbox({ ...makeDefaultOptions(), diff --git a/packages/@n8n/agents/src/workspace/sandbox/create-workspace.ts b/packages/@n8n/agents/src/workspace/sandbox/create-workspace.ts index 662e4e38f94..9205cd5d99b 100644 --- a/packages/@n8n/agents/src/workspace/sandbox/create-workspace.ts +++ b/packages/@n8n/agents/src/workspace/sandbox/create-workspace.ts @@ -58,6 +58,7 @@ function buildSandbox( if (provider === 'n8n-sandbox') { return new N8nSandboxServiceSandbox({ + id: config.id, apiKey: config.apiKey, serviceUrl: config.serviceUrl, timeout: config.timeout ?? 300_000, diff --git a/packages/@n8n/agents/src/workspace/sandbox/n8n-sandbox-sandbox.ts b/packages/@n8n/agents/src/workspace/sandbox/n8n-sandbox-sandbox.ts index 83a79f39ecf..c94c6d55847 100644 --- a/packages/@n8n/agents/src/workspace/sandbox/n8n-sandbox-sandbox.ts +++ b/packages/@n8n/agents/src/workspace/sandbox/n8n-sandbox-sandbox.ts @@ -1,4 +1,4 @@ -import { SandboxClient, SandboxServiceError, type SandboxRecord } from '@n8n/sandbox-client'; +import { SandboxClient, SandboxServiceError } from '@n8n/sandbox-client'; import assert from 'node:assert/strict'; import { randomUUID } from 'node:crypto'; @@ -6,6 +6,7 @@ import { BaseSandbox } from './base-sandbox'; import type { CommandResult, ExecuteCommandOptions, ProviderStatus, SandboxInfo } from '../types'; export interface N8nSandboxServiceSandboxOptions { + /** Lowercase UUID to create or reconnect to; the service generates one when omitted. */ id?: string; apiKey?: string; serviceUrl?: string; @@ -59,17 +60,15 @@ export class N8nSandboxServiceSandbox extends BaseSandbox { } override async start(): Promise { - if (this.sandboxId) { - const existing = await this.tryGetExistingSandbox(this.sandboxId); - if (existing) { - this.createdAt = new Date(existing.createdAt * 1000); - return; - } - } - - const sandbox = await this.client.createSandbox(); + // With an id, the service creates-or-reconnects: an existing live sandbox + // is returned as-is, a stale one is replaced under the same id. This is + // what lets a restarted process (or another main) reattach to a thread's + // sandbox instead of orphaning it. + const sandbox = await this.client.createSandbox( + this.sandboxId ? { id: this.sandboxId } : undefined, + ); this.sandboxId = sandbox.id; - this.createdAt = new Date(); + this.createdAt = new Date(sandbox.createdAt * 1000); } override async destroy(): Promise { @@ -145,16 +144,6 @@ 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 = { diff --git a/packages/@n8n/agents/src/workspace/sandbox/types.ts b/packages/@n8n/agents/src/workspace/sandbox/types.ts index 8d9924ef89b..008d88725df 100644 --- a/packages/@n8n/agents/src/workspace/sandbox/types.ts +++ b/packages/@n8n/agents/src/workspace/sandbox/types.ts @@ -92,6 +92,12 @@ export interface N8nSandboxConfig extends SandboxConfigBase { provider: 'n8n-sandbox'; serviceUrl: string; apiKey?: string; + /** + * Sandbox id to create or reconnect to. Must be a lowercase UUID — the + * sandbox service rejects any other format. When omitted the service + * generates a random id, which no other process can rediscover later. + */ + id?: string; } export type SandboxConfig = DisabledSandboxConfig | DaytonaSandboxConfig | N8nSandboxConfig; diff --git a/packages/@n8n/instance-ai/docs/sandboxing.md b/packages/@n8n/instance-ai/docs/sandboxing.md index 6a68dc831ec..7d1de86fdde 100644 --- a/packages/@n8n/instance-ai/docs/sandboxing.md +++ b/packages/@n8n/instance-ai/docs/sandboxing.md @@ -191,7 +191,7 @@ graph TB style Build2 fill:#dcfce7,stroke:#16a34a ``` -- **Thread-scoped workspace.** The service can maintain a single workspace per conversation thread, reused across messages. This workspace is destroyed on server shutdown. +- **Thread-scoped workspace.** The service can maintain a single workspace per conversation thread, reused across messages. This workspace is destroyed on server shutdown. Its sandbox identity is derived deterministically from the thread ID (a name for Daytona, a UUIDv5 for the n8n sandbox service), so a restarted process — or another main in a multi-main deployment — reattaches to the same remote sandbox instead of creating a duplicate. - **Per-builder ephemeral workspace.** Each time the workflow builder is invoked, it gets its own isolated workspace. Multiple concurrent builders in the same thread do not share a workspace. The provider sandbox is deleted after the builder finishes (best-effort). ### Pre-warmed images diff --git a/packages/cli/src/modules/instance-ai/__tests__/instance-ai-sandbox.service.test.ts b/packages/cli/src/modules/instance-ai/__tests__/instance-ai-sandbox.service.test.ts index 818c3a441b2..9a8db664596 100644 --- a/packages/cli/src/modules/instance-ai/__tests__/instance-ai-sandbox.service.test.ts +++ b/packages/cli/src/modules/instance-ai/__tests__/instance-ai-sandbox.service.test.ts @@ -447,6 +447,42 @@ describe('InstanceAiSandboxService', () => { expect(setupSandboxWorkspace).toHaveBeenCalledTimes(1); }); + it('assigns a deterministic thread-scoped UUID for the n8n-sandbox provider', async () => { + const n8nSandboxConfig: Overrides['config'] = { + sandboxEnabled: true, + sandboxProvider: 'n8n-sandbox', + n8nSandboxServiceUrl: 'https://env.sandbox', + }; + const workspace = { init: vi.fn(async () => {}), destroy: vi.fn(async () => {}) }; + (createSandbox as Mock).mockResolvedValue({ id: 'sandbox-1' }); + (createWorkspace as Mock).mockReturnValue(workspace); + (setupSandboxWorkspace as Mock).mockResolvedValue(undefined); + + const { service } = createSandboxService({ config: n8nSandboxConfig }); + await service.getOrCreateWorkspace('thread-1', fakeUser, {} as InstanceAiContext); + + const uuidPattern = /^[0-9a-f]{8}-[0-9a-f]{4}-5[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/; + expect(createSandbox).toHaveBeenCalledWith( + expect.objectContaining({ + provider: 'n8n-sandbox', + id: expect.stringMatching(uuidPattern), + }), + expect.any(Object), + ); + + // A fresh service instance (e.g. after a restart or on another main) + // derives the same id for the same thread, and a different one for + // another thread. + const { service: restartedService } = createSandboxService({ config: n8nSandboxConfig }); + await restartedService.getOrCreateWorkspace('thread-1', fakeUser, {} as InstanceAiContext); + await restartedService.getOrCreateWorkspace('thread-2', fakeUser, {} as InstanceAiContext); + + const ids = (createSandbox as Mock).mock.calls.map((call) => (call[0] as { id?: string }).id); + expect(ids).toHaveLength(3); + expect(ids[1]).toBe(ids[0]); + expect(ids[2]).not.toBe(ids[0]); + }); + it('threads Daytona name prefixes and labels through sandbox creation', async () => { const { service } = createSandboxService({ config: { @@ -859,5 +895,66 @@ describe('InstanceAiSandboxService', () => { await expect(service.destroySandbox('missing-thread')).resolves.toBeUndefined(); }); + + it('deletes the uncached remote sandbox for the n8n-sandbox provider', async () => { + // No prior getOrCreateWorkspace call: simulates a thread deleted after + // a restart or idle eviction, when the in-process cache has no entry. + const { service } = createSandboxService({ + config: { + sandboxEnabled: true, + sandboxProvider: 'n8n-sandbox', + n8nSandboxServiceUrl: 'https://env.sandbox', + }, + }); + const sandbox = { destroy: vi.fn(async () => {}) }; + (createSandbox as Mock).mockResolvedValue(sandbox); + + await service.destroySandbox('thread-1'); + + expect(createSandbox).toHaveBeenCalledWith( + expect.objectContaining({ + provider: 'n8n-sandbox', + id: expect.stringMatching(/^[0-9a-f-]{36}$/), + }), + expect.any(Object), + ); + expect(sandbox.destroy).toHaveBeenCalledTimes(1); + }); + + it('does not attempt an uncached destroy for the daytona provider', async () => { + const { service } = createSandboxService({ + config: { sandboxEnabled: true, sandboxProvider: 'daytona' }, + }); + + await service.destroySandbox('thread-1'); + + expect(createSandbox).not.toHaveBeenCalled(); + }); + + it('swallows uncached destroy errors and logs a warning', async () => { + const { service, logger } = createSandboxService({ + config: { + sandboxEnabled: true, + sandboxProvider: 'n8n-sandbox', + n8nSandboxServiceUrl: 'https://env.sandbox', + }, + }); + (createSandbox as Mock).mockResolvedValue({ + destroy: vi.fn(async () => { + throw new Error('service unreachable'); + }), + }); + + await expect(service.destroySandbox('thread-1', 'custom_reason')).resolves.toBeUndefined(); + + expect(logger.warn).toHaveBeenCalledWith( + 'Failed to destroy sandbox', + expect.objectContaining({ + threadId: 'thread-1', + reason: 'custom_reason', + error: 'service unreachable', + }), + ); + }); }); }); diff --git a/packages/cli/src/modules/instance-ai/sandbox/instance-ai-sandbox.service.ts b/packages/cli/src/modules/instance-ai/sandbox/instance-ai-sandbox.service.ts index 5e83e910662..81a7c4ef133 100644 --- a/packages/cli/src/modules/instance-ai/sandbox/instance-ai-sandbox.service.ts +++ b/packages/cli/src/modules/instance-ai/sandbox/instance-ai-sandbox.service.ts @@ -14,6 +14,7 @@ import { import type { ErrorReporter } from 'n8n-core'; import { OperationalError, UnexpectedError } from 'n8n-workflow'; import { nanoid } from 'nanoid'; +import { v5 as uuidv5 } from 'uuid'; import { N8N_VERSION } from '@/constants'; import { callAiServiceWithRetry } from '@/utils/ai-service-retry'; @@ -88,8 +89,29 @@ function buildThreadScopedSandboxLabels( return labels; } +/** + * Fixed UUIDv5 namespace for deriving thread-scoped n8n-sandbox ids. Never + * change it: any main must be able to recompute the id of a sandbox created + * by an older process to reattach to it. + */ +const N8N_SANDBOX_THREAD_ID_NAMESPACE = '5e6c2f7a-93a1-4b0e-8f27-c1d6a3b9e514'; + +/** The n8n sandbox service only accepts lowercase UUID ids, so hash the thread-scoped name into a stable UUIDv5. */ +function buildThreadScopedSandboxUuid(threadId: string): string { + return uuidv5(getThreadScopedSandboxName(threadId), N8N_SANDBOX_THREAD_ID_NAMESPACE); +} + +/** + * Give the sandbox a deterministic, thread-derived identity so any process — + * after a restart, a cache eviction, or on another main — resolves the same + * remote sandbox instead of creating a duplicate and orphaning the old one. + */ function withThreadScopedSandboxIdentity(config: SandboxConfig, threadId: string): SandboxConfig { - if (!config.enabled || config.provider !== 'daytona') return config; + if (!config.enabled) return config; + + if (config.provider === 'n8n-sandbox') { + return { ...config, id: buildThreadScopedSandboxUuid(threadId) }; + } const name = buildThreadScopedSandboxName(threadId, config.namePrefix); return { @@ -148,16 +170,18 @@ export type InstanceAiSandboxServiceOptions = { * * Each conversation thread gets a single shared sandbox + workspace, created * lazily on first use and reused across runs and background tasks. Sandbox - * names are deterministic (derived from the thread ID) so a restarted process - * — or another main in a multi-main deployment — reconnects to the same remote + * identities are deterministic (derived from the thread ID — a name for + * Daytona, a UUIDv5 for the n8n sandbox service) so a restarted process — or + * another main in a multi-main deployment — reconnects to the same remote * sandbox instead of spawning a duplicate. An in-process TTL drops idle cache - * entries so the map cannot grow without bound; provider auto-stop reclaims the - * remote sandbox itself, so an idle eviction never destroys live work. + * entries so the map cannot grow without bound; the provider reclaims the + * remote sandbox itself (Daytona auto-stop, sandbox-service idle reaping), so + * an idle eviction never destroys live work. */ export class InstanceAiSandboxService { /** * Shared runtime workspaces keyed by thread ID. This is only an in-process - * cache; deterministic sandbox names let providers reconnect after restart + * cache; deterministic sandbox identities let providers reconnect after restart * or from another main when the thread uses the workspace again. */ private readonly sandboxes = new Map(); @@ -443,7 +467,10 @@ export class InstanceAiSandboxService { /** Destroy and remove the shared runtime workspace for a thread. */ async destroySandbox(threadId: string, reason = 'thread_cleanup'): Promise { const entry = this.sandboxes.get(threadId); - if (!entry?.sandbox) return; + if (!entry?.sandbox) { + await this.destroyUncachedSandbox(threadId, reason); + return; + } this.evictSandboxEntry(threadId, entry); try { @@ -457,6 +484,42 @@ export class InstanceAiSandboxService { } } + /** + * Delete the remote sandbox for a thread with no cache entry (after a + * restart or an idle eviction). Only the n8n-sandbox provider supports + * this: its id is recomputable from the thread id, and a delete for an id + * that never existed is a cheap 404. Daytona is left to its own + * auto-stop/auto-delete lifecycle. + */ + private async destroyUncachedSandbox(threadId: string, reason: string): Promise { + try { + const base = this.getSandboxConfigFromEnv(); + if (!base.enabled || base.provider !== 'n8n-sandbox') return; + + const settings = await this.options.settingsService.resolveN8nSandboxConfig(); + const config = withThreadScopedSandboxIdentity( + { + ...base, + serviceUrl: settings.serviceUrl ?? base.serviceUrl, + apiKey: settings.apiKey ?? base.apiKey, + }, + threadId, + ); + // Constructing the adapter makes no remote calls; destroy() issues the delete. + const sandbox = await createSandbox(config, { + logger: this.logger, + errorReporter: this.options.errorReporter, + }); + await sandbox?.destroy?.(); + } catch (error) { + this.logger.warn('Failed to destroy sandbox', { + threadId, + reason, + error: error instanceof Error ? error.message : String(error), + }); + } + } + private get sandboxTtlMs(): number { return this.instanceAiConfig?.builderSandboxTtlMs ?? DEFAULT_SANDBOX_TTL_MS; } @@ -487,8 +550,10 @@ export class InstanceAiSandboxService { if (this.sandboxTtlMs <= 0) return; if (entry.cleanupTimer) clearTimeout(entry.cleanupTimer); - // Provider auto-stop handles remote Daytona sandboxes. This timer only - // drops our in-process cache entry so the map cannot grow indefinitely. + // The provider reclaims the remote sandbox (Daytona auto-stop, sandbox-service + // idle reaping), and the deterministic identity lets a later request reattach + // while it is still alive. This timer only drops our in-process cache entry + // so the map cannot grow indefinitely. const delay = Math.max(0, entry.expiresAt - Date.now()); entry.cleanupTimer = setTimeout(() => { const current = this.sandboxes.get(threadId); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 86aa6eb1f95..c95fcdef1b6 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -995,8 +995,8 @@ importers: specifier: workspace:* version: link:../ai-utilities '@n8n/sandbox-client': - specifier: 0.0.4 - version: 0.0.4 + specifier: 0.1.0 + version: 0.1.0 '@n8n/utils': specifier: workspace:* version: link:../utils @@ -10135,8 +10135,8 @@ packages: resolution: {integrity: sha512-qmDvJIjcNsZ6tXWy2G9yuCgMPTTn35GMA3dPpSLm7QJVpbQzYdw0ALy1bKoivXnEM3U93/OrK+/M719b+fg84Q==} engines: {node: '>=18'} - '@n8n/sandbox-client@0.0.4': - resolution: {integrity: sha512-yaCVqz2MCHfmNkPQObiXiM9x3hzy/S+Ey2+WIe3TazFsXz1A02cZudt7eO1MW1J0O7IKU1mqe1xPmchHN3+6Vw==} + '@n8n/sandbox-client@0.1.0': + resolution: {integrity: sha512-9pADrO61nwOVrfaEZJM6lFkp6YQ0Qnt0OSArncxABoaYj3C2XvdfDbXByKn71tsE6zqmnFFmYI1S0ccshHASeQ==} engines: {node: '>=22.16', pnpm: '>=10.22.0'} '@n8n/typeorm@0.3.20-16': @@ -27646,7 +27646,7 @@ snapshots: outvariant: 1.4.3 strict-event-emitter: 0.5.1 - '@n8n/sandbox-client@0.0.4': + '@n8n/sandbox-client@0.1.0': dependencies: axios: 1.18.0(patch_hash=149e256a2a7b632497650b32816716ced972ab02a5ab00fbd8a5158a51722c49)(debug@4.4.3) transitivePeerDependencies: