mirror of
https://github.com/n8n-io/n8n.git
synced 2026-09-17 17:42:47 +08:00
fix(ai-builder): Prevent snapshot staging races during concurrent sandbox creation (#33342)
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
a46b001f90
commit
7d946c3031
@@ -38,7 +38,8 @@ describe('snapshot-image-context', () => {
|
||||
).resolves.toBe('# Schedule');
|
||||
});
|
||||
|
||||
it('reuses a hash-keyed staging directory for the same cache key', async () => {
|
||||
it('stages once per cache key and reuses the directory read-only', async () => {
|
||||
// Same key ⇒ same content in prod, so the repeat reuses the first staging (filesB ignored).
|
||||
const filesA = new Map([[`${WORKSPACE_ROOT}/package.json`, '{"name":"a"}']]);
|
||||
const filesB = new Map([[`${WORKSPACE_ROOT}/package.json`, '{"name":"b"}']]);
|
||||
const cacheKey = 'aaaaaaaaaaaa-bbbbbbbbbbbb';
|
||||
@@ -49,24 +50,36 @@ describe('snapshot-image-context', () => {
|
||||
|
||||
expect(second.stagingDir).toBe(first.stagingDir);
|
||||
await expect(readFile(join(first.stagingDir, 'package.json'), 'utf-8')).resolves.toBe(
|
||||
'{"name":"b"}',
|
||||
'{"name":"a"}',
|
||||
);
|
||||
});
|
||||
|
||||
it('removes stale files when reusing a cache-keyed staging directory', async () => {
|
||||
it('serves concurrent stagings for the same cache key from a single directory', async () => {
|
||||
const cacheKey = 'cccccccccccc-dddddddddddd';
|
||||
const filesWithExtra = new Map([
|
||||
[`${WORKSPACE_ROOT}/package.json`, '{"name":"a"}'],
|
||||
[`${WORKSPACE_ROOT}/skills/removed/SKILL.md`, '# Removed'],
|
||||
const files = new Map([
|
||||
[`${WORKSPACE_ROOT}/package.json`, '{"name":"concurrent"}'],
|
||||
[`${WORKSPACE_ROOT}/skills/post-build-flow/SKILL.md`, '# Post build'],
|
||||
[`${WORKSPACE_ROOT}/knowledge-base/templates/example.ts`, 'export const x = 1;'],
|
||||
]);
|
||||
const filesWithoutExtra = new Map([[`${WORKSPACE_ROOT}/package.json`, '{"name":"b"}']]);
|
||||
|
||||
const first = await stageWorkspaceFilesForImage(filesWithExtra, WORKSPACE_ROOT, cacheKey);
|
||||
const second = await stageWorkspaceFilesForImage(filesWithoutExtra, WORKSPACE_ROOT, cacheKey);
|
||||
tempDirs.push(first.stagingDir);
|
||||
const results = await Promise.all(
|
||||
Array.from(
|
||||
{ length: 8 },
|
||||
async () => await stageWorkspaceFilesForImage(files, WORKSPACE_ROOT, cacheKey),
|
||||
),
|
||||
);
|
||||
tempDirs.push(results[0].stagingDir);
|
||||
|
||||
expect(second.stagingDir).toBe(first.stagingDir);
|
||||
await expect(access(join(first.stagingDir, 'skills/removed/SKILL.md'))).rejects.toThrow();
|
||||
// All callers share one directory and every file is intact — no rm clobbered a write.
|
||||
for (const result of results) {
|
||||
expect(result.stagingDir).toBe(results[0].stagingDir);
|
||||
}
|
||||
await expect(
|
||||
readFile(join(results[0].stagingDir, 'skills/post-build-flow/SKILL.md'), 'utf-8'),
|
||||
).resolves.toBe('# Post build');
|
||||
await expect(
|
||||
readFile(join(results[0].stagingDir, 'knowledge-base/templates/example.ts'), 'utf-8'),
|
||||
).resolves.toBe('export const x = 1;');
|
||||
});
|
||||
|
||||
it('disposeSnapshotImageContext removes the staging directory', async () => {
|
||||
|
||||
@@ -8,6 +8,13 @@ export interface SnapshotImageContext {
|
||||
stagingDir: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Dedupe staging by key: the destructive `rm`+`mkdir`+`writeFile` runs once per
|
||||
* key, and concurrent callers await it and reuse the directory read-only. Same key
|
||||
* ⇒ same content, so reuse both avoids re-writing and prevents rm/write races.
|
||||
*/
|
||||
const stagingByCacheKey = new Map<string, Promise<SnapshotImageContext>>();
|
||||
|
||||
function workspaceRelativePath(filePath: string, workspaceRoot: string): string {
|
||||
const root = workspaceRoot.endsWith('/') ? workspaceRoot : `${workspaceRoot}/`;
|
||||
|
||||
@@ -20,35 +27,52 @@ function workspaceRelativePath(filePath: string, workspaceRoot: string): string
|
||||
return filePath.slice(root.length);
|
||||
}
|
||||
|
||||
/**
|
||||
* Writes sandbox workspace files to a host directory for Daytona `addLocalDir` / COPY.
|
||||
* When `cacheKey` is set, reuses `os.tmpdir()/n8n-snapshot-context-<cacheKey>` so repeated
|
||||
* image builds do not accumulate temp directories.
|
||||
*/
|
||||
export async function stageWorkspaceFilesForImage(
|
||||
async function writeStagedFiles(
|
||||
stagingDir: string,
|
||||
files: Map<string, string>,
|
||||
workspaceRoot: string,
|
||||
cacheKey?: string,
|
||||
): Promise<SnapshotImageContext> {
|
||||
let stagingDir: string;
|
||||
|
||||
if (cacheKey) {
|
||||
stagingDir = join(tmpdir(), `${SNAPSHOT_CONTEXT_PREFIX}${cacheKey}`);
|
||||
|
||||
await rm(stagingDir, { recursive: true, force: true });
|
||||
await mkdir(stagingDir, { recursive: true });
|
||||
} else {
|
||||
stagingDir = await mkdtemp(join(tmpdir(), `${SNAPSHOT_CONTEXT_PREFIX}temp-`));
|
||||
}
|
||||
|
||||
): Promise<void> {
|
||||
for (const [filePath, content] of files) {
|
||||
const targetPath = join(stagingDir, workspaceRelativePath(filePath, workspaceRoot));
|
||||
|
||||
await mkdir(dirname(targetPath), { recursive: true });
|
||||
await writeFile(targetPath, content, 'utf-8');
|
||||
}
|
||||
}
|
||||
|
||||
return { stagingDir };
|
||||
/**
|
||||
* Writes sandbox workspace files to a host directory for Daytona `addLocalDir` / COPY.
|
||||
* With a `cacheKey`, stages into `os.tmpdir()/n8n-snapshot-context-<cacheKey>` once per key
|
||||
* (see `stagingByCacheKey`) so repeated and concurrent builds share one directory.
|
||||
*/
|
||||
export async function stageWorkspaceFilesForImage(
|
||||
files: Map<string, string>,
|
||||
workspaceRoot: string,
|
||||
cacheKey?: string,
|
||||
): Promise<SnapshotImageContext> {
|
||||
if (!cacheKey) {
|
||||
const stagingDir = await mkdtemp(join(tmpdir(), `${SNAPSHOT_CONTEXT_PREFIX}temp-`));
|
||||
await writeStagedFiles(stagingDir, files, workspaceRoot);
|
||||
return { stagingDir };
|
||||
}
|
||||
|
||||
const inFlightOrDone = stagingByCacheKey.get(cacheKey);
|
||||
if (inFlightOrDone) return await inFlightOrDone;
|
||||
|
||||
const staging = (async () => {
|
||||
const stagingDir = join(tmpdir(), `${SNAPSHOT_CONTEXT_PREFIX}${cacheKey}`);
|
||||
await rm(stagingDir, { recursive: true, force: true });
|
||||
await mkdir(stagingDir, { recursive: true });
|
||||
await writeStagedFiles(stagingDir, files, workspaceRoot);
|
||||
return { stagingDir };
|
||||
})().catch((error) => {
|
||||
// Evict a rejected staging so the next caller retries.
|
||||
stagingByCacheKey.delete(cacheKey);
|
||||
throw error;
|
||||
});
|
||||
|
||||
stagingByCacheKey.set(cacheKey, staging);
|
||||
return await staging;
|
||||
}
|
||||
|
||||
export async function disposeSnapshotImageContext(stagingDir: string): Promise<void> {
|
||||
|
||||
@@ -26,7 +26,7 @@ import {
|
||||
builderTemplatesOptionsFromEnv,
|
||||
} from './builder-templates-service';
|
||||
import { PACKAGE_JSON, TSCONFIG_JSON, BUILD_MJS } from './sandbox-setup';
|
||||
import { disposeSnapshotImageContext, stageWorkspaceFilesForImage } from './snapshot-image-context';
|
||||
import { stageWorkspaceFilesForImage } from './snapshot-image-context';
|
||||
import { buildRuntimeSkillWorkspaceBundle } from '../skills/materialize-runtime-skills';
|
||||
import { loadInstanceAiRuntimeSkillSource } from '../skills/runtime-skills';
|
||||
|
||||
@@ -52,8 +52,6 @@ export class SnapshotManager {
|
||||
|
||||
private knowledgeBaseBundlePromise: Promise<KnowledgeBaseWorkspaceBundle> | null = null;
|
||||
|
||||
private stagingDir: string | null = null;
|
||||
|
||||
constructor(
|
||||
private readonly baseImage: string | undefined,
|
||||
private readonly logger: Logger,
|
||||
@@ -88,7 +86,6 @@ export class SnapshotManager {
|
||||
DAYTONA_WORKSPACE_ROOT,
|
||||
cacheKey,
|
||||
);
|
||||
this.stagingDir = stagingDir;
|
||||
|
||||
const { Image } = loadDaytona();
|
||||
const layoutDirs = SNAPSHOT_WORKSPACE_LAYOUT_DIRS.map(
|
||||
@@ -179,15 +176,14 @@ export class SnapshotManager {
|
||||
});
|
||||
}
|
||||
|
||||
/** Invalidate cached image (e.g., when base image changes). */
|
||||
/**
|
||||
* Invalidate the in-memory image/bundle caches. The shared staging dir is owned
|
||||
* by the per-key cache in `stageWorkspaceFilesForImage` and intentionally not
|
||||
* removed here (in-flight creations may still be reading it).
|
||||
*/
|
||||
invalidate(): void {
|
||||
const stagingDir = this.stagingDir;
|
||||
this.cachedImage = null;
|
||||
this.runtimeSkillBundlePromise = null;
|
||||
this.knowledgeBaseBundlePromise = null;
|
||||
this.stagingDir = null;
|
||||
if (stagingDir) {
|
||||
void disposeSnapshotImageContext(stagingDir);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user