refactor(core): Address knowledge file bytes by their persisted storage key (no-changelog)

The store now returns the key it wrote and reads back whatever key the row
records, instead of deriving it from agent and file id. That lets rows
converted from BinaryDataService keep resolving under their original key.

Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
Robin Braumann 2026-07-27 11:26:40 +02:00
parent bebce8f90b
commit 97363cc4e2
9 changed files with 95 additions and 52 deletions

View File

@ -20,6 +20,7 @@ const mobyDickFile: AgentKnowledgeFileReference = {
fileId: 'file-1',
agentId: 'agent-1',
storedAt: 'fs',
storageKey: 'agents/agent-1/knowledge-files/file-1/content',
displayName: 'moby-dick.txt',
mimeType: 'text/plain',
fileSizeBytes: 123,

View File

@ -41,18 +41,32 @@ describe('AgentKnowledgeFileStore', () => {
it('falls back to fs when execution data storage mode is database and round-trips bytes', async () => {
const body = Buffer.from('knowledge-bytes', 'utf-8');
const ref = { agentId: 'agent-1', fileId: 'file-1' };
const storedAt = await store.write(ref, body, {
const stored = await store.write({ agentId: 'agent-1', fileId: 'file-1' }, body, {
fileName: 'notes.txt',
mimeType: 'text/plain',
});
expect(storedAt).toBe('fs');
expect(stored).toEqual({
storedAt: 'fs',
storageKey: 'agents/agent-1/knowledge-files/file-1/content',
});
expect(logger.warn).toHaveBeenCalledWith(
"Execution data storage mode is 'database'; agent knowledge files will be stored on the local filesystem. In multi-main deployments this path must be on a shared filesystem.",
);
await expect(store.readAsBuffer({ ...ref, storedAt: 'fs' })).resolves.toEqual(body);
await expect(store.readAsBuffer(stored)).resolves.toEqual(body);
});
it('reads a blob stored under the legacy BinaryDataService key', async () => {
const body = Buffer.from('legacy-bytes', 'utf-8');
const legacyKey =
'agents/agent-1/knowledge-files/file-1/binary_data/2f1b3c4d-5e6f-4a7b-8c9d-0e1f2a3b4c5d';
const fsByteStore = new FsByteStore({ storagePath, reportError: () => {} });
await fsByteStore.write(legacyKey, body);
await expect(store.readAsBuffer({ storedAt: 'fs', storageKey: legacyKey })).resolves.toEqual(
body,
);
});
it('writes to a registered s3 location when modeTag is s3', async () => {
@ -65,37 +79,42 @@ describe('AgentKnowledgeFileStore', () => {
store.registerByteStore('s3', s3Store);
const body = Buffer.from('s3-bytes', 'utf-8');
const ref = { agentId: 'agent-1', fileId: 'file-2' };
const storedAt = await store.write(ref, body, { mimeType: 'text/plain' });
const stored = await store.write({ agentId: 'agent-1', fileId: 'file-2' }, body, {
mimeType: 'text/plain',
});
expect(storedAt).toBe('s3');
await expect(store.readAsBuffer({ ...ref, storedAt: 's3' })).resolves.toEqual(body);
expect(stored.storedAt).toBe('s3');
await expect(store.readAsBuffer(stored)).resolves.toEqual(body);
});
it('returns null for a missing blob and throws for an unregistered storedAt', async () => {
await expect(
store.readAsBuffer({ agentId: 'agent-1', fileId: 'missing', storedAt: 'fs' }),
store.readAsBuffer({
storedAt: 'fs',
storageKey: 'agents/a/knowledge-files/missing/content',
}),
).resolves.toBeNull();
await expect(
store.readAsBuffer({ agentId: 'agent-1', fileId: 'file-1', storedAt: 's3' }),
store.readAsBuffer({ storedAt: 's3', storageKey: 'agents/a/knowledge-files/f/content' }),
).rejects.toThrow(UnexpectedError);
});
it('deletes registered blobs and skips unregistered locations without throwing', async () => {
const kept = { agentId: 'agent-1', fileId: 'keep' };
const body = Buffer.from('delete-me', 'utf-8');
await store.write(kept, body, { mimeType: 'text/plain' });
const kept = await store.write({ agentId: 'agent-1', fileId: 'keep' }, body, {
mimeType: 'text/plain',
});
await expect(
store.delete([
{ ...kept, storedAt: 'fs' },
{ agentId: 'agent-1', fileId: 'orphan', storedAt: 's3' },
kept,
{ storedAt: 's3', storageKey: 'agents/agent-1/knowledge-files/orphan/content' },
]),
).resolves.toBeUndefined();
await expect(store.readAsBuffer({ ...kept, storedAt: 'fs' })).resolves.toBeNull();
await expect(store.readAsBuffer(kept)).resolves.toBeNull();
expect(logger.warn).toHaveBeenCalledWith(
'Skipped deleting agent knowledge files for unconfigured storage',
expect.objectContaining({ storedAt: 's3', count: 1 }),

View File

@ -146,6 +146,7 @@ function makeAgentFile(overrides: Partial<AgentFile> = {}): AgentFile {
id,
agentId,
storedAt: 'fs',
storageKey: `agents/${agentId}/knowledge-files/${id}/content`,
fileName,
mimeType: 'text/plain',
fileSizeBytes: 100,

View File

@ -65,6 +65,7 @@ function makeAgentFile(overrides: Partial<AgentFile> = {}): AgentFile {
id: 'file-1',
agentId,
storedAt: 'fs',
storageKey: `agents/${agentId}/knowledge-files/${overrides.id ?? 'file-1'}/content`,
fileName: 'first.txt',
mimeType: 'text/plain',
fileSizeBytes: 4,
@ -84,6 +85,7 @@ class InMemoryAgentFileRepository {
id: input.id ?? 'generated-id',
agentId: input.agentId ?? agentId,
storedAt: input.storedAt ?? 'fs',
storageKey: input.storageKey ?? '',
fileName: input.fileName ?? '',
mimeType: input.mimeType ?? '',
fileSizeBytes: input.fileSizeBytes ?? 0,
@ -143,9 +145,12 @@ describe('AgentKnowledgeService', () => {
agentFileRepository = new InMemoryAgentFileRepository();
agentKnowledgeSandboxService = mock<AgentKnowledgeSandboxService>();
agentKnowledgeFileStore = mock<AgentKnowledgeFileStore>();
agentKnowledgeFileStore.write.mockImplementation(async (_ref, content) => {
agentKnowledgeFileStore.write.mockImplementation(async (ref, content) => {
await drainIfStream(content);
return 'fs';
return {
storedAt: 'fs',
storageKey: `agents/${ref.agentId}/knowledge-files/${ref.fileId}/content`,
};
});
agentKnowledgeFileStore.delete.mockResolvedValue(undefined);
logger = mock<Logger>();
@ -239,9 +244,12 @@ describe('AgentKnowledgeService', () => {
await writeFile(secondPath, 'world');
agentKnowledgeFileStore.write
.mockReset()
.mockImplementationOnce(async (_ref, content) => {
.mockImplementationOnce(async (ref, content) => {
await drainIfStream(content);
return 'fs';
return {
storedAt: 'fs',
storageKey: `agents/${ref.agentId}/knowledge-files/${ref.fileId}/content`,
};
})
.mockImplementationOnce(async (_ref, content) => {
await drainIfStream(content);
@ -262,7 +270,7 @@ describe('AgentKnowledgeService', () => {
expect(agentFileRepository.all()).toEqual([]);
expect(agentKnowledgeFileStore.delete).toHaveBeenCalledWith([
expect.objectContaining({ agentId, storedAt: 'fs' }),
expect.objectContaining({ storedAt: 'fs', storageKey: expect.any(String) }),
]);
});
@ -351,7 +359,7 @@ describe('AgentKnowledgeService', () => {
expect(agentFileRepository.all()).toEqual([]);
expect(agentKnowledgeFileStore.delete).toHaveBeenCalledWith([
{ agentId, fileId: 'file-1', storedAt: 'fs' },
{ storedAt: 'fs', storageKey: `agents/${agentId}/knowledge-files/file-1/content` },
]);
});
@ -385,8 +393,8 @@ describe('AgentKnowledgeService', () => {
expect(agentFileRepository.all()).toEqual([]);
expect(agentKnowledgeFileStore.delete).toHaveBeenCalledWith([
{ agentId, fileId: 'file-1', storedAt: 'fs' },
{ agentId, fileId: 'file-2', storedAt: 's3' },
{ storedAt: 'fs', storageKey: `agents/${agentId}/knowledge-files/file-1/content` },
{ storedAt: 's3', storageKey: `agents/${agentId}/knowledge-files/file-2/content` },
]);
});

View File

@ -10,8 +10,8 @@ import { ErrorReporter, StorageConfig } from 'n8n-core';
import { UnexpectedError } from 'n8n-workflow';
import type { Readable } from 'node:stream';
type AgentKnowledgeFileRef = { agentId: string; fileId: string };
type StoredAgentKnowledgeFileRef = AgentKnowledgeFileRef & { storedAt: StorageLocation };
/** Where a knowledge file's bytes live, as recorded on its `agent_files` row. */
type StoredAgentKnowledgeFile = { storedAt: StorageLocation; storageKey: string };
@Service()
export class AgentKnowledgeFileFsByteStore extends FsByteStore {
@ -28,6 +28,9 @@ export class AgentKnowledgeFileFsByteStore extends FsByteStore {
* always available; `s3` and `az` are registered at module init when configured.
* When execution data storage mode is `database`, writes fall back to `fs`.
* In multi-main deployments that fallback path must be on a shared filesystem.
*
* Keys are generated on write and persisted, so files written by the former
* BinaryDataService layout keep resolving under their original key.
*/
@Service()
export class AgentKnowledgeFileStore {
@ -60,27 +63,28 @@ export class AgentKnowledgeFileStore {
}
async write(
ref: AgentKnowledgeFileRef,
ref: { agentId: string; fileId: string },
body: Buffer | Readable,
metadata: PreWriteBlobMetadata,
): Promise<StorageLocation> {
const loc = this.writeLocation;
await this.getByteStore(loc).write(this.key(ref), body, metadata);
return loc;
): Promise<StoredAgentKnowledgeFile> {
const storedAt = this.writeLocation;
const storageKey = this.newKeyFor(ref);
await this.getByteStore(storedAt).write(storageKey, body, metadata);
return { storedAt, storageKey };
}
async readAsBuffer(ref: StoredAgentKnowledgeFileRef): Promise<Buffer | null> {
return await this.getByteStore(ref.storedAt).read(this.key(ref));
async readAsBuffer(file: StoredAgentKnowledgeFile): Promise<Buffer | null> {
return await this.getByteStore(file.storedAt).read(file.storageKey);
}
async delete(refs: StoredAgentKnowledgeFileRef[]): Promise<void> {
if (refs.length === 0) return;
async delete(files: StoredAgentKnowledgeFile[]): Promise<void> {
if (files.length === 0) return;
const groups = new Map<StorageLocation, StoredAgentKnowledgeFileRef[]>();
for (const ref of refs) {
const group = groups.get(ref.storedAt) ?? [];
group.push(ref);
groups.set(ref.storedAt, group);
const groups = new Map<StorageLocation, StoredAgentKnowledgeFile[]>();
for (const file of files) {
const group = groups.get(file.storedAt) ?? [];
group.push(file);
groups.set(file.storedAt, group);
}
await Promise.all(
@ -93,12 +97,12 @@ export class AgentKnowledgeFileStore {
});
return;
}
await store.delete(group.map((ref) => this.key(ref)));
await store.delete(group.map((file) => file.storageKey));
}),
);
}
private key(ref: AgentKnowledgeFileRef) {
private newKeyFor(ref: { agentId: string; fileId: string }) {
return [
'agents',
encodeURIComponent(ref.agentId),

View File

@ -190,6 +190,7 @@ export interface AgentKnowledgeFileReference {
fileId: string;
agentId: string;
storedAt: StorageLocation;
storageKey: string;
displayName: string;
mimeType: string;
fileSizeBytes: number;

View File

@ -559,9 +559,8 @@ export class AgentKnowledgeSandboxService {
try {
const buffer = await this.agentKnowledgeFileStore.readAsBuffer({
agentId: file.agentId,
fileId: file.fileId,
storedAt: file.storedAt,
storageKey: file.storageKey,
});
if (!buffer) {
this.logger.warn('Failed to load agent knowledge file for mirror sync', {
@ -650,6 +649,7 @@ export class AgentKnowledgeSandboxService {
fileId: file.id,
agentId: file.agentId,
storedAt: file.storedAt,
storageKey: file.storageKey,
displayName: file.fileName,
mimeType: file.mimeType,
fileSizeBytes: file.fileSizeBytes,

View File

@ -104,7 +104,7 @@ export class AgentKnowledgeService {
await this.agentFileRepository.delete({ id: fileId, agentId });
await this.agentKnowledgeFileStore
.delete([{ agentId, fileId: file.id, storedAt: file.storedAt }])
.delete([{ storedAt: file.storedAt, storageKey: file.storageKey }])
.catch((error) => {
this.logger.warn('Failed to delete knowledge file blob', {
agentId,
@ -123,9 +123,8 @@ export class AgentKnowledgeService {
await this.agentKnowledgeFileStore
.delete(
files.map((file) => ({
agentId,
fileId: file.id,
storedAt: file.storedAt,
storageKey: file.storageKey,
})),
)
.catch((error) => {
@ -149,15 +148,15 @@ export class AgentKnowledgeService {
const storageFileName = storageFileNameForOriginalFileName(file.originalname);
const content = await this.prepareUploadContent(file);
const storedAt = await this.agentKnowledgeFileStore.write({ agentId, fileId }, content, {
const stored = await this.agentKnowledgeFileStore.write({ agentId, fileId }, content, {
fileName: storageFileName,
mimeType: file.mimetype,
});
try {
return await this.saveAgentFile(agentId, fileId, file, storedAt);
return await this.saveAgentFile(agentId, fileId, file, stored);
} catch (error) {
await this.agentKnowledgeFileStore.delete([{ agentId, fileId, storedAt }]).catch(() => {});
await this.agentKnowledgeFileStore.delete([stored]).catch(() => {});
if (isUniqueConstraintError(error)) {
throw this.duplicateFileNameError(file.originalname);
}
@ -169,12 +168,13 @@ export class AgentKnowledgeService {
agentId: string,
fileId: string,
file: Express.Multer.File,
storedAt: StorageLocation,
stored: { storedAt: StorageLocation; storageKey: string },
): Promise<AgentFile> {
const agentFile = this.agentFileRepository.create({
id: fileId,
agentId,
storedAt,
storedAt: stored.storedAt,
storageKey: stored.storageKey,
fileName: file.originalname,
mimeType: file.mimetype,
fileSizeBytes: file.size,
@ -200,7 +200,7 @@ export class AgentKnowledgeService {
private async cleanupUploadedFiles(files: AgentFile[]): Promise<void> {
for (const file of files) {
await this.agentKnowledgeFileStore
.delete([{ agentId: file.agentId, fileId: file.id, storedAt: file.storedAt }])
.delete([{ storedAt: file.storedAt, storageKey: file.storageKey }])
.catch(() => {});
await this.agentFileRepository.delete({ id: file.id, agentId: file.agentId }).catch(() => {});
}

View File

@ -7,6 +7,7 @@ import { Agent } from './agent.entity';
@Entity({ name: 'agent_files' })
@Index(['agentId', 'createdAt'])
@Index(['agentId', 'fileName'], { unique: true })
@Index(['agentId', 'storageKey'], { unique: true })
export class AgentFile extends WithTimestampsAndStringId {
@ManyToOne(() => Agent, { onDelete: 'CASCADE' })
@JoinColumn({ name: 'agentId' })
@ -22,6 +23,14 @@ export class AgentFile extends WithTimestampsAndStringId {
@Column({ type: 'varchar', length: 2, nullable: false, default: 'fs' })
storedAt: StorageLocation;
/**
* Byte-store key addressing the bytes within `storedAt`. Persisted rather
* than derived, so files written under the former BinaryDataService layout
* keep resolving after the storage migration.
*/
@Column({ type: 'text' })
storageKey: string;
// fileName/mimeType/fileSizeBytes are intentionally denormalized rather than
// joined from storage metadata: we keep the original user-facing values,
// which differ from the stored bytes for converted uploads (a PDF is stored