From 02af1235747cdbc003fca0a199e9e0bdbc55affc Mon Sep 17 00:00:00 2001 From: Mutasem Aldmour <4711238+mutdmour@users.noreply.github.com> Date: Fri, 17 Jul 2026 19:30:19 +0200 Subject: [PATCH] feat: Add typed seed data tables with rows on execution scenarios (#34420) Co-authored-by: Claude Opus 4.8 (1M context) --- packages/@n8n/api-types/src/index.ts | 2 + .../src/schemas/instance-ai.schema.ts | 49 +++- .../build-workflow-seed-cleanup.test.ts | 131 ++++++++++ ...xecution-scenario-seed-data-tables.test.ts | 92 +++++++ .../__tests__/scenario-seed-tables.test.ts | 203 +++++++++++++++ .../@n8n/instance-ai/evaluations/cli/index.ts | 1 + .../evaluations/clients/n8n-client.ts | 32 ++- .../instance-ai/evaluations/harness/runner.ts | 243 +++++++++++++++++- .../instance-ai/evaluations/harness/schema.ts | 7 + .../@n8n/instance-ai/evaluations/types.ts | 10 +- .../__tests__/instance-ai.controller.test.ts | 30 ++- ...thread-restore.service.integration.test.ts | 161 ++++++++++++ .../__tests__/thread-restore.service.test.ts | 68 +++++ .../eval/thread-restore.service.ts | 49 +++- .../instance-ai/instance-ai.controller.ts | 42 ++- 15 files changed, 1092 insertions(+), 28 deletions(-) create mode 100644 packages/@n8n/instance-ai/evaluations/__tests__/build-workflow-seed-cleanup.test.ts create mode 100644 packages/@n8n/instance-ai/evaluations/__tests__/execution-scenario-seed-data-tables.test.ts create mode 100644 packages/@n8n/instance-ai/evaluations/__tests__/scenario-seed-tables.test.ts create mode 100644 packages/cli/src/modules/instance-ai/eval/__tests__/thread-restore.service.integration.test.ts diff --git a/packages/@n8n/api-types/src/index.ts b/packages/@n8n/api-types/src/index.ts index 1ec8d86d970..e4bae2c8fd9 100644 --- a/packages/@n8n/api-types/src/index.ts +++ b/packages/@n8n/api-types/src/index.ts @@ -339,6 +339,7 @@ export { INSTANCE_AI_MEMORY_TASK_WAIT_TIMEOUT_MS, AI_GATEWAY_MANAGED_TAG, InstanceAiEvalRestoreThreadRequest, + InstanceAiEvalSeedDataTableRowsRequest, instanceAiGatewayKeySchema, InstanceAiGatewayEventsQuery, InstanceAiEventsQuery, @@ -355,6 +356,7 @@ export { InstanceAiGatewayCapabilitiesDto, InstanceAiGatewayCreateCredentialDto, InstanceAiFilesystemResponseDto, + instanceAiEvalSeedDataTableSchema, applyBranchReadOnlyOverrides, normalizeInstanceAiThreadSource, } from './schemas/instance-ai.schema'; diff --git a/packages/@n8n/api-types/src/schemas/instance-ai.schema.ts b/packages/@n8n/api-types/src/schemas/instance-ai.schema.ts index 06670ff3368..5ef3ba5a9e2 100644 --- a/packages/@n8n/api-types/src/schemas/instance-ai.schema.ts +++ b/packages/@n8n/api-types/src/schemas/instance-ai.schema.ts @@ -1631,10 +1631,17 @@ export type InstanceAiEvalSeedWorkflow = z.infer; export class InstanceAiEvalRestoreThreadRequest extends Z.class({ threadId: z.string().uuid(), - /** Native agent message log (ISO `createdAt`), stored verbatim. */ - messages: z.array(z.record(z.unknown())).min(1).max(1000), + /** Native agent message log (ISO `createdAt`), stored verbatim. May be empty + * when the request only seeds data tables (TRUST-311 scenario seeding). */ + messages: z.array(z.record(z.unknown())).max(1000), /** Data tables the workflows reference; recreated first so ids can be rewritten. */ dataTables: z.array(instanceAiEvalSeedDataTableSchema).max(20).optional(), /** Workflows the history references; recreated (node credentials stripped). */ workflows: z.array(instanceAiEvalSeedWorkflowSchema).max(50).optional(), + /** Append a unique suffix to each seed data table's name (default true — safe + * for id-remapped seed workflows). False keeps the EXACT declared name so a + * freshly-built workflow's by-name references resolve. */ + uniquifyNames: z.boolean().optional(), +}) {} + +/** + * Reset an existing data table's rows to exactly `rows` (clear-then-insert). + * Unlike restore-thread (which CREATES tables), this targets a table that + * already exists by id — used for the per-scenario row seeding of a case whose + * tables were created empty before the build turn (TRUST-311 follow-up). The + * table is scoped to the thread's project server-side. + */ +export class InstanceAiEvalSeedDataTableRowsRequest extends Z.class({ + threadId: z.string().uuid(), + /** Id of the (already existing) data table whose rows are reset. */ + tableId: z.string().min(8).max(64), + /** The exact row set the table should hold after seeding (may be empty to + * clear it). Cell values are validated against each column's type on insert. */ + rows: z + .array(z.record(z.string(), z.union([z.string(), z.number(), z.boolean(), z.null()]))) + .max(1000), }) {} diff --git a/packages/@n8n/instance-ai/evaluations/__tests__/build-workflow-seed-cleanup.test.ts b/packages/@n8n/instance-ai/evaluations/__tests__/build-workflow-seed-cleanup.test.ts new file mode 100644 index 00000000000..811673fde26 --- /dev/null +++ b/packages/@n8n/instance-ai/evaluations/__tests__/build-workflow-seed-cleanup.test.ts @@ -0,0 +1,131 @@ +import { vi } from 'vitest'; + +import type { N8nClient } from '../clients/n8n-client'; +import type { EvalLogger } from '../harness/logger'; +import { buildWorkflow } from '../harness/runner'; +import { buildAgentOutcome } from '../outcome/workflow-discovery'; +import type { ExecutionScenario } from '../types'; + +// The chat loop is network/SSE machinery irrelevant to this test: stub it so a +// single-turn build reaches the pre-build seed + outcome steps without real I/O. +// (vi.mock is hoisted above the imports, so the runner picks up these stubs.) +vi.mock('../harness/chat-loop', () => ({ + SSE_SETTLE_DELAY_MS: 0, + startSseConnection: vi.fn().mockResolvedValue(undefined), + waitForAllActivity: vi.fn().mockResolvedValue(undefined), + runMultiTurnConversation: vi.fn().mockResolvedValue(undefined), + recordUserTurn: vi.fn(), +})); + +// Force a "workflow built" outcome by default so the build succeeds; individual +// tests override it to simulate a build-step failure after pre-seeding. +vi.mock('../outcome/workflow-discovery', () => ({ + buildAgentOutcome: vi.fn().mockResolvedValue({ + workflowsCreated: [{ id: 'built-wf-1', name: 'Built', nodeCount: 3, active: false }], + executionsRun: [], + dataTablesCreated: ['built-dt-1'], + finalText: 'done', + workflowJsons: [{ id: 'built-wf-1', name: 'Built', nodes: [], connections: {} }], + }), + extractWorkflowIdsFromMessages: vi.fn().mockReturnValue([]), +})); + +const silentLogger: EvalLogger = { + info: () => {}, + verbose: () => {}, + success: () => {}, + warn: () => {}, + error: () => {}, + isVerbose: false, +}; + +function scenarioWithSeedTable(): ExecutionScenario { + return { + name: 'scenario', + description: 'd', + dataSetup: 'setup', + successCriteria: 'ok', + seedDataTables: [ + { + id: 'job-applications-1234', + name: 'Job Applications', + columns: [{ name: 'id', type: 'string' as const }], + rows: [{ id: 'row_001' }], + }, + ], + }; +} + +function makeClient(overrides: Partial> = {}): N8nClient { + return { + getPersonalProjectId: vi.fn().mockResolvedValue('project-1'), + ensureThread: vi.fn().mockResolvedValue(undefined), + setThreadCredentialAllowlist: vi.fn().mockResolvedValue(undefined), + sendMessage: vi.fn().mockResolvedValue(undefined), + getThreadMessages: vi.fn().mockResolvedValue({ messages: [] }), + // Pre-build scenario-table creation returns the real id under the name. + restoreThread: vi + .fn() + .mockResolvedValue({ restored: 0, workflowIds: [], dataTableIds: ['scenario-dt-1'] }), + ...overrides, + } as unknown as N8nClient; +} + +const baseConfig = { + conversation: [{ role: 'user' as const, text: 'build a workflow' }], + executionScenarios: [scenarioWithSeedTable()], + skipWorkflowChecks: true, + preRunWorkflowIds: new Set(), + claimedWorkflowIds: new Set(), + logger: silentLogger, +}; + +// TRUST-311 follow-up: scenario data tables are created (empty) BEFORE the build +// turn. These pin the failure/cleanup contract of that pre-build seeding: +// - a create failure fails the build as a harness problem (framework_issue), +// and there is nothing built to leak; +// - if a LATER build step fails, the already-created tables are still handed to +// cleanup (folded into restoredDataTableIds) rather than leaking. +describe('buildWorkflow scenario-seed data table lifecycle', () => { + it('fails the build and flags seedingFailed when pre-build table creation fails', async () => { + const client = makeClient({ + restoreThread: vi.fn().mockRejectedValue(new Error('seed insert failed')), + }); + + const build = await buildWorkflow({ client, ...baseConfig }); + + expect(build.success).toBe(false); + // A pre-seed failure is a harness problem, not an agent build failure — flag + // it so the CLI attributes framework_issue, not build_failure. + expect(build.seedingFailed).toBe(true); + // The build never ran, so there is nothing built to leak. + expect(build.createdWorkflowIds).toEqual([]); + expect(build.createdDataTableIds).toEqual([]); + }); + + it('hands the pre-created scenario tables to cleanup when a later build step fails', async () => { + vi.mocked(buildAgentOutcome).mockRejectedValueOnce(new Error('workflow discovery failed')); + const client = makeClient(); // pre-seed succeeds → scenario-dt-1 created + + const build = await buildWorkflow({ client, ...baseConfig }); + + expect(build.success).toBe(false); + // The pre-created table must still be returned so the caller's cleanup + // deletes it instead of leaking it into the shared project. + expect(build.createdDataTableIds).toContain('scenario-dt-1'); + }); + + it('returns the built workflow, both tables, and the name→id map on success', async () => { + const client = makeClient(); + + const build = await buildWorkflow({ client, ...baseConfig }); + + expect(build.success).toBe(true); + expect(build.createdWorkflowIds).toContain('built-wf-1'); + expect(build.createdDataTableIds).toEqual( + expect.arrayContaining(['built-dt-1', 'scenario-dt-1']), + ); + // The name→real-id map lets each scenario reseed rows into the bound table. + expect(build.seededScenarioTableIdsByName).toEqual({ 'Job Applications': 'scenario-dt-1' }); + }); +}); diff --git a/packages/@n8n/instance-ai/evaluations/__tests__/execution-scenario-seed-data-tables.test.ts b/packages/@n8n/instance-ai/evaluations/__tests__/execution-scenario-seed-data-tables.test.ts new file mode 100644 index 00000000000..a05cd4c6b30 --- /dev/null +++ b/packages/@n8n/instance-ai/evaluations/__tests__/execution-scenario-seed-data-tables.test.ts @@ -0,0 +1,92 @@ +import { EvalTestCaseSchema } from '../harness/schema'; + +// TRUST-311: an execution scenario can declare typed seed data tables (with rows) +// so a string id like `row_001` lands in a `string` column instead of being +// rejected by a `number` column. The field reuses api-types' +// `instanceAiEvalSeedDataTableSchema`, extended to carry optional `rows`. +describe('executionScenarios[].seedDataTables', () => { + function caseWith(seedDataTables: unknown) { + return { + complexity: 'simple' as const, + tags: [], + conversation: [{ role: 'user' as const, text: 'do the thing' }], + executionScenarios: [ + { + name: 'scenario-1', + description: 'seeded scenario', + dataSetup: 'a job application already exists', + successCriteria: 'the workflow reads it', + seedDataTables, + }, + ], + }; + } + + it('preserves a typed seed table with a string id column and a string-id row', () => { + const seedDataTables = [ + { + id: 'job-applications-tbl', + name: 'Job Applications', + columns: [ + { name: 'id', type: 'string' as const }, + { name: 'is_active', type: 'boolean' as const }, + ], + rows: [{ id: 'row_001', is_active: true }], + }, + ]; + + const result = EvalTestCaseSchema.safeParse(caseWith(seedDataTables)); + + expect(result.success).toBe(true); + if (!result.success) return; + expect(result.data.executionScenarios?.[0].seedDataTables).toEqual(seedDataTables); + }); + + it('is optional — a scenario without seed tables still parses', () => { + const result = EvalTestCaseSchema.safeParse({ + complexity: 'simple' as const, + tags: [], + conversation: [{ role: 'user' as const, text: 'do the thing' }], + executionScenarios: [ + { + name: 'scenario-1', + description: 'plain scenario', + dataSetup: 'nothing', + successCriteria: 'ok', + }, + ], + }); + + expect(result.success).toBe(true); + if (!result.success) return; + expect(result.data.executionScenarios?.[0].seedDataTables).toBeUndefined(); + }); + + it('rejects a too-short table id at load time (restore needs >=8 chars to remap)', () => { + const result = EvalTestCaseSchema.safeParse( + caseWith([ + { + id: 'short', + name: 'Job Applications', + columns: [{ name: 'id', type: 'string' as const }], + }, + ]), + ); + + expect(result.success).toBe(false); + }); + + it('rejects an unknown column type', () => { + const result = EvalTestCaseSchema.safeParse( + caseWith([ + { + id: 'job-applications-tbl', + name: 'Job Applications', + columns: [{ name: 'id', type: 'uuid' }], + }, + ]), + ); + + expect(result.success).toBe(false); + }); +}); diff --git a/packages/@n8n/instance-ai/evaluations/__tests__/scenario-seed-tables.test.ts b/packages/@n8n/instance-ai/evaluations/__tests__/scenario-seed-tables.test.ts new file mode 100644 index 00000000000..e3e2ae16db6 --- /dev/null +++ b/packages/@n8n/instance-ai/evaluations/__tests__/scenario-seed-tables.test.ts @@ -0,0 +1,203 @@ +import { vi } from 'vitest'; +import type { Mock } from 'vitest'; + +import type { N8nClient } from '../clients/n8n-client'; +import type { EvalLogger } from '../harness/logger'; +import { + buildSeededTablesNote, + dedupeScenarioSeedTables, + reseedScenarioTables, + scenariosRequireSerialSeeding, +} from '../harness/runner'; +import type { ExecutionScenario } from '../types'; + +// TRUST-311 follow-up: scenario data tables are created EMPTY before the build +// turn (so the agent discovers the real table and binds its real id), then row- +// seeded per scenario just before that scenario executes (so build-time row +// mutations don't leak across scenarios, and scenarios can carry different rows). +// These unit the pure pieces; the pre-build/per-scenario wiring is integration. + +const silentLogger: EvalLogger = { + info: () => {}, + verbose: () => {}, + success: () => {}, + warn: () => {}, + error: () => {}, + isVerbose: false, +}; + +function scenario(overrides: Partial = {}): ExecutionScenario { + return { + name: 'scenario', + description: 'd', + dataSetup: 'setup', + successCriteria: 'ok', + ...overrides, + }; +} + +const jobApplications = { + id: 'job-applications-1234', + name: 'Job Applications', + columns: [{ name: 'application_id', type: 'string' as const }], + rows: [{ application_id: 'row_001' }], +}; + +describe('dedupeScenarioSeedTables', () => { + it('returns the union of scenario seed tables deduped by name (first wins)', () => { + const dup = { + ...jobApplications, + id: 'applications-2222', + rows: [{ application_id: 'row_002' }], + }; + const tables = dedupeScenarioSeedTables( + [scenario({ seedDataTables: [jobApplications] }), scenario({ seedDataTables: [dup] })], + silentLogger, + ); + + expect(tables).toEqual([jobApplications]); // first declaration wins + }); + + it('returns an empty array when no scenario declares a seed table', () => { + expect( + dedupeScenarioSeedTables([scenario(), scenario({ seedDataTables: [] })], silentLogger), + ).toEqual([]); + }); + + it('warns when a later scenario redeclares the same name with a DIFFERENT shape', () => { + const warn = vi.fn(); + const logger = { ...silentLogger, warn }; + const conflicting = { + ...jobApplications, + id: 'applications-2222', + rows: [{ application_id: 'row_002' }], + }; + + dedupeScenarioSeedTables( + [ + scenario({ seedDataTables: [jobApplications] }), + scenario({ seedDataTables: [conflicting] }), + ], + logger, + ); + + expect(warn).toHaveBeenCalledWith(expect.stringContaining('Job Applications')); + }); + + it('does not warn when the same name is redeclared identically', () => { + const warn = vi.fn(); + const logger = { ...silentLogger, warn }; + + dedupeScenarioSeedTables( + [ + scenario({ seedDataTables: [jobApplications] }), + scenario({ seedDataTables: [{ ...jobApplications }] }), + ], + logger, + ); + + expect(warn).not.toHaveBeenCalled(); + }); + + it('throws when the deduped union exceeds the 20-table cap', () => { + const tables = Array.from({ length: 21 }, (_, i) => ({ + id: `table-id-${String(i).padStart(4, '0')}`, + name: `Table ${String(i)}`, + columns: [{ name: 'application_id', type: 'string' as const }], + })); + + expect(() => + dedupeScenarioSeedTables([scenario({ seedDataTables: tables })], silentLogger), + ).toThrow(/20/); + }); +}); + +describe('buildSeededTablesNote', () => { + it('is empty when there are no tables', () => { + expect(buildSeededTablesNote([])).toBe(''); + }); + + it('names each table and its columns so the agent binds the real table', () => { + const note = buildSeededTablesNote([jobApplications]); + + expect(note).toContain('Job Applications'); + expect(note).toContain('application_id'); + expect(note).toContain('string'); + }); +}); + +describe('scenariosRequireSerialSeeding', () => { + it('is true when any scenario declares seed tables', () => { + expect( + scenariosRequireSerialSeeding([scenario(), scenario({ seedDataTables: [jobApplications] })]), + ).toBe(true); + }); + + it('is false when no scenario declares seed tables', () => { + expect(scenariosRequireSerialSeeding([scenario(), scenario({ seedDataTables: [] })])).toBe( + false, + ); + }); +}); + +function makeClient(seedDataTableRows: Mock): N8nClient { + return { seedDataTableRows } as unknown as N8nClient; +} + +describe('reseedScenarioTables', () => { + it('clears + seeds each declared table by its bound real id', async () => { + const seedDataTableRows = vi.fn().mockResolvedValue(undefined); + const client = makeClient(seedDataTableRows); + + await reseedScenarioTables( + client, + scenario({ seedDataTables: [jobApplications] }), + 'thread-1', + { 'Job Applications': 'dt-real-1' }, + silentLogger, + ); + + expect(seedDataTableRows).toHaveBeenCalledWith('thread-1', 'dt-real-1', jobApplications.rows); + }); + + it('seeds an empty row set when a table declares no rows', async () => { + const seedDataTableRows = vi.fn().mockResolvedValue(undefined); + const client = makeClient(seedDataTableRows); + const noRows = { ...jobApplications, rows: undefined }; + + await reseedScenarioTables( + client, + scenario({ seedDataTables: [noRows] }), + 'thread-1', + { 'Job Applications': 'dt-real-1' }, + silentLogger, + ); + + expect(seedDataTableRows).toHaveBeenCalledWith('thread-1', 'dt-real-1', []); + }); + + it('does nothing when the scenario declares no seed tables', async () => { + const seedDataTableRows = vi.fn(); + const client = makeClient(seedDataTableRows); + + await reseedScenarioTables(client, scenario(), 'thread-1', {}, silentLogger); + + expect(seedDataTableRows).not.toHaveBeenCalled(); + }); + + it('throws when a declared table was not pre-seeded (missing from the id map)', async () => { + const seedDataTableRows = vi.fn(); + const client = makeClient(seedDataTableRows); + + await expect( + reseedScenarioTables( + client, + scenario({ seedDataTables: [jobApplications] }), + 'thread-1', + {}, // Job Applications not in the map + silentLogger, + ), + ).rejects.toThrow(/Job Applications/); + expect(seedDataTableRows).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/@n8n/instance-ai/evaluations/cli/index.ts b/packages/@n8n/instance-ai/evaluations/cli/index.ts index 9234c9e3372..236f3b7c296 100644 --- a/packages/@n8n/instance-ai/evaluations/cli/index.ts +++ b/packages/@n8n/instance-ai/evaluations/cli/index.ts @@ -652,6 +652,7 @@ async function runWithLangSmith(config: RunConfig): Promise<{ seedFile: buildArgs.seedFile, priorConversation: buildArgs.priorConversation, seedThread: buildArgs.seedThread, + executionScenarios: buildArgs.executionScenarios, createdCredentialIds: lane.createdCredentialIds, timeoutMs: buildArgs.timeoutMs, preRunWorkflowIds: lane.preRunWorkflowIds, diff --git a/packages/@n8n/instance-ai/evaluations/clients/n8n-client.ts b/packages/@n8n/instance-ai/evaluations/clients/n8n-client.ts index 021140d6584..6252456b348 100644 --- a/packages/@n8n/instance-ai/evaluations/clients/n8n-client.ts +++ b/packages/@n8n/instance-ai/evaluations/clients/n8n-client.ts @@ -635,6 +635,13 @@ export class N8nClient { * referenced workflows are recreated (node credentials stripped server-side) * and the native message log is written verbatim, so the thread continues * as if the conversation really happened. + * + * `uniquifyNames` (default true) appends a unique suffix to each seed data + * table's name to dodge the per-project unique-name constraint — safe when the + * seed workflow references tables by id (id-remap). Pass false to keep the + * EXACT declared names, so a freshly-built workflow's by-name references + * resolve (TRUST-311 scenario seeding). `messages`/`workflows` may be empty to + * seed only data tables. * POST /rest/instance-ai/eval/restore-thread */ async restoreThread( @@ -642,14 +649,37 @@ export class N8nClient { messages: Array>, workflows: InstanceAiEvalSeedWorkflow[], dataTables: InstanceAiEvalSeedDataTable[] = [], + options: { uniquifyNames?: boolean } = {}, ): Promise<{ restored: number; workflowIds: string[]; dataTableIds: string[] }> { + const body: Record = { threadId, messages, workflows, dataTables }; + if (options.uniquifyNames !== undefined) body.uniquifyNames = options.uniquifyNames; const result = await this.fetch('/rest/instance-ai/eval/restore-thread', { method: 'POST', - body: { threadId, messages, workflows, dataTables }, + body, }); return RestoreThreadEnvelope.parse(result).data; } + /** + * Reset an existing data table's rows to exactly `rows` (clear-then-insert), + * for the per-scenario row seeding of a case that pre-created its tables + * before the build turn (TRUST-311). Unlike `restoreThread` (which CREATES + * tables), this targets a table that already exists by id, so a scenario can + * declare its own row state without disturbing the table the built workflow + * bound. `threadId` scopes the table to the run's project server-side. + * POST /rest/instance-ai/eval/seed-data-table-rows + */ + async seedDataTableRows( + threadId: string, + tableId: string, + rows: Array>, + ): Promise { + await this.fetch('/rest/instance-ai/eval/seed-data-table-rows', { + method: 'POST', + body: { threadId, tableId, rows }, + }); + } + // -- Data tables --------------------------------------------------------- /** diff --git a/packages/@n8n/instance-ai/evaluations/harness/runner.ts b/packages/@n8n/instance-ai/evaluations/harness/runner.ts index e930455c0d6..34cfd7ed35b 100644 --- a/packages/@n8n/instance-ai/evaluations/harness/runner.ts +++ b/packages/@n8n/instance-ai/evaluations/harness/runner.ts @@ -6,7 +6,11 @@ // LLM-mocked HTTP, checklist verification, and result aggregation. // --------------------------------------------------------------------------- -import type { InstanceAiConfirmRequest, InstanceAiEvalExecutionResult } from '@n8n/api-types'; +import type { + InstanceAiConfirmRequest, + InstanceAiEvalExecutionResult, + InstanceAiEvalSeedDataTable, +} from '@n8n/api-types'; import { isRecord } from '@n8n/utils/is-record'; import crypto from 'node:crypto'; import { mkdir, writeFile } from 'node:fs/promises'; @@ -327,6 +331,7 @@ export async function runWorkflowTestCase( seedFile: testCase.seedFile, priorConversation: testCase.priorConversation, seedThread: testCase.seedThread, + executionScenarios: testCase.executionScenarios, createdCredentialIds: config.createdCredentialIds, timeoutMs, preRunWorkflowIds: config.preRunWorkflowIds, @@ -444,9 +449,21 @@ export async function runWorkflowTestCase( result.buildTrace = build.buildTrace; const testCaseArtifactName = deriveTestCaseArtifactName(testCase); + const scenarios = testCase.executionScenarios ?? []; + // Rows for a case's pre-seeded scenario tables are swapped in per scenario + // (TRUST-311). All scenarios of a case share one table per name, so seeding + // must run serially — concurrent scenarios would race on the shared rows. + const seedContext = + build.seededScenarioTableIdsByName && build.threadId + ? { threadId: build.threadId, tableIdsByName: build.seededScenarioTableIdsByName } + : undefined; + const scenarioConcurrency = scenariosRequireSerialSeeding(scenarios) + ? 1 + : MAX_CONCURRENT_SCENARIOS; + const scenarioStart = Date.now(); const scenariosPromise = runWithConcurrency( - testCase.executionScenarios ?? [], + scenarios, async (scenario) => { for (let attempt = 1; ; attempt++) { try { @@ -460,6 +477,7 @@ export async function runWorkflowTestCase( testCaseArtifactName, build.buildTrace, config.pinAiRoots, + seedContext, ); } catch (error: unknown) { const errorMessage = extractErrorMessage(error); @@ -488,7 +506,7 @@ export async function runWorkflowTestCase( } } }, - MAX_CONCURRENT_SCENARIOS, + scenarioConcurrency, ); const [scenarioResults, expectationResults] = await Promise.all([ @@ -526,6 +544,9 @@ interface MultiTurnDriverConfig { logger: EvalLogger; proxyResponses?: Map; followUpMessagesOut?: string[]; + /** Appended to the FIRST sent message only (pre-seeded-table hint); the + * recorded turn and the proxy's conversation keep the clean prompt. */ + openingMessageSuffix?: string; } async function driveMultiTurnConversation( @@ -551,7 +572,10 @@ async function driveMultiTurnConversation( }; recordUserTurn(config.events, openingMessage); - await config.client.sendMessage(config.threadId, openingMessage); + await config.client.sendMessage( + config.threadId, + openingMessage + (config.openingMessageSuffix ?? ''), + ); await runMultiTurnConversation({ client: config.client, @@ -582,6 +606,11 @@ export interface BuildResult { /** IDs to pass to cleanupBuild() */ createdWorkflowIds: string[]; createdDataTableIds: string[]; + /** Maps each scenario seed table's declared NAME to the real id it was created + * under (empty) before the build turn, so each scenario can reset+seed its + * rows into the table the built workflow actually bound (TRUST-311 follow-up). + * Absent when the case declares no scenario seed tables. */ + seededScenarioTableIdsByName?: Record; /** Non-workflow artifact refs (agent, config-eval) captured from the SSE stream, * fed to the build-expectations judge context. Empty/undefined for prebuilt runs. */ artifactRefs?: ArtifactRef[]; @@ -625,6 +654,9 @@ export interface BuildWorkflowConfig { /** Reproduce a real conversation from its LangSmith trace (seed = before the * last user message, live = that message). */ seedThread?: SeedThreadRef; + /** Execution scenarios whose declared `seedDataTables` are created + row-seeded + * after a successful build, before any scenario runs (TRUST-311). */ + executionScenarios?: ExecutionScenario[]; timeoutMs?: number; preRunWorkflowIds: Set; claimedWorkflowIds: Set; @@ -681,6 +713,17 @@ export async function buildWorkflow(config: BuildWorkflowConfig): Promise = {}; + let scenarioSeedTablesNote = ''; + // Ids the build itself produced (the agent's workflow + any data tables it + // made). Tracked here so a throw AFTER the build lands — scenario-table + // seeding, workflow checks — still hands them to the caller's cleanup rather + // than leaking them into the shared eval project. + let builtWorkflowIds: string[] = []; + let builtDataTableIds: string[] = []; let seededTranscript: TranscriptTurn[] = []; let seedingFailed = false; @@ -787,6 +830,43 @@ export async function buildWorkflow(config: BuildWorkflowConfig): Promise 0) { + const schemasOnly = scenarioSeedTables.map((table) => ({ ...table, rows: undefined })); + const { dataTableIds } = await client.restoreThread(threadId, [], [], schemasOnly, { + uniquifyNames: false, + }); + // restoreThread returns ids in input order; a length mismatch means we + // can't safely map names to ids, so fail rather than mis-seed. + if (dataTableIds.length !== scenarioSeedTables.length) { + throw new Error( + `Pre-seeding created ${String(dataTableIds.length)} data table(s) but the case declares ${String(scenarioSeedTables.length)}; cannot map names to ids.`, + ); + } + scenarioSeedTables.forEach((table, index) => { + scenarioTableIdsByName[table.name] = dataTableIds[index]; + }); + restoredDataTableIds = [...restoredDataTableIds, ...dataTableIds]; + scenarioSeedTablesNote = buildSeededTablesNote(scenarioSeedTables); + logger.info( + ` Pre-seeded ${String(dataTableIds.length)} scenario data table schema(s)${config.laneTag ?? ''}`, + ); + } + } catch (error: unknown) { + seedingFailed = true; + throw error; + } + const ssePromise = startSseConnection(client, threadId, events, abortController.signal).catch( () => {}, ); @@ -807,10 +887,13 @@ export async function buildWorkflow(config: BuildWorkflowConfig): Promise wf.id); + builtDataTableIds = outcome.dataTablesCreated; if (outcome.workflowsCreated.length === 0) { // Answer-only cases (no execution scenarios, no outcome expectations) @@ -925,6 +1010,9 @@ export async function buildWorkflow(config: BuildWorkflowConfig): Promise wf.id), createdDataTableIds: [...outcome.dataTablesCreated, ...restoredDataTableIds], + seededScenarioTableIdsByName: scenarioTableIdsByName, artifactRefs: eventOutcome.artifactRefs, conversationMetrics, events, @@ -949,8 +1038,8 @@ export async function buildWorkflow(config: BuildWorkflowConfig): Promise { return await runScenario( client, @@ -991,6 +1081,129 @@ export async function executeScenario( testCaseName, buildTrace, pinAiRoots, + seedContext, + ); +} + +/** Per-scenario row-seeding context: the run's thread and the name→real-id map + * of the tables created empty before the build turn (TRUST-311 follow-up). */ +export interface ScenarioSeedContext { + threadId: string; + tableIdsByName: Record; +} + +/** Max distinct scenario seed tables per case — mirrors the restore-thread + * DTO's `dataTables` cap, since the whole union is sent in one call. */ +const MAX_SEED_DATA_TABLES = 20; + +/** + * Deduplicate the data tables an execution-scenario case declares + * (`seedDataTables`) into the union a case shares across its scenarios + * (TRUST-311). A table name is unique per project and the built workflow binds + * it by name, so a case shares ONE table per name across its scenarios; the + * first declaration wins. A later same-name declaration with a different shape + * (columns/rows) is dropped with a warning — the by-name binding can only + * resolve to one table, so keeping the first silently would be data loss for the + * author. Throws if the distinct-name union exceeds the restore-thread DTO's cap + * (the whole union is created in one call). The returned tables carry their + * declared `rows`, but the pre-build creation seeds only the schema — rows are + * reset+seeded per scenario (`reseedScenarioTables`). + */ +export function dedupeScenarioSeedTables( + scenarios: ExecutionScenario[], + logger: EvalLogger, +): InstanceAiEvalSeedDataTable[] { + const byName = new Map(); + for (const scenario of scenarios) { + for (const table of scenario.seedDataTables ?? []) { + const existing = byName.get(table.name); + if (existing) { + if (!sameSeedTableShape(existing, table)) { + logger.warn( + ` Scenario seed table "${table.name}" is declared more than once with different columns/rows; keeping the first declaration and ignoring the rest.`, + ); + } + continue; + } + byName.set(table.name, table); + } + } + if (byName.size > MAX_SEED_DATA_TABLES) { + throw new Error( + `A case declares ${String(byName.size)} distinct scenario seed data tables, exceeding the ${String(MAX_SEED_DATA_TABLES)}-table restore limit; reduce the number of distinct table names.`, + ); + } + return [...byName.values()]; +} + +/** + * A note appended to the build's opening message naming the data tables that + * already exist in the workspace (created empty before the build turn) so the + * agent discovers and binds the REAL table (via the Data Table node's + * list/schema) instead of creating a duplicate — the production-faithful flow + * where the user's table pre-exists (TRUST-311 follow-up). Empty when the case + * declares no scenario seed tables. + */ +export function buildSeededTablesNote(tables: InstanceAiEvalSeedDataTable[]): string { + if (tables.length === 0) return ''; + const lines = tables.map((table) => { + const columns = table.columns.map((column) => `${column.name}: ${column.type}`).join(', '); + return `- "${table.name}" (columns: ${columns})`; + }); + return `\n\nThe following data table(s) already exist in this workspace — reuse them (look them up with the Data Table node's list/schema) instead of creating new ones:\n${lines.join('\n')}`; +} + +/** + * True when any scenario declares seed tables. All of a case's scenarios share + * one table per name, so their per-scenario row reset+seed + * (`reseedScenarioTables`) must run serially — concurrent scenarios would race + * on the shared table's rows. Callers gate scenario concurrency to 1 for such + * cases. + */ +export function scenariosRequireSerialSeeding(scenarios: ExecutionScenario[]): boolean { + return scenarios.some((scenario) => (scenario.seedDataTables?.length ?? 0) > 0); +} + +/** + * Reset + row-seed a scenario's declared data tables into their pre-seeded real + * ids, just before that scenario executes (TRUST-311). Clears whatever rows a + * prior scenario — or a build-time self-verification execution — left, then + * inserts this scenario's declared rows, so each scenario runs against exactly + * the state it declared (and scenarios may carry different rows for the same + * table). `tableIdsByName` maps the declared table name to the real id created + * before the build turn; a name missing from it means the table was never + * pre-seeded, which is a harness bug, so throw rather than silently skip. + */ +export async function reseedScenarioTables( + client: N8nClient, + scenario: ExecutionScenario, + threadId: string, + tableIdsByName: Record, + logger: EvalLogger, +): Promise { + for (const table of scenario.seedDataTables ?? []) { + const tableId = tableIdsByName[table.name]; + if (!tableId) { + throw new Error( + `Scenario "${scenario.name}" declares seed table "${table.name}" that was not pre-seeded before the build; cannot bind its rows.`, + ); + } + await client.seedDataTableRows(threadId, tableId, table.rows ?? []); + logger.verbose( + ` [${scenario.name}] reseeded data table "${table.name}" (${String((table.rows ?? []).length)} row(s))`, + ); + } +} + +/** Two seed tables bind the same way iff their columns + rows match (the id + * differs per declaration and is cosmetic under by-name seeding). */ +function sameSeedTableShape( + a: InstanceAiEvalSeedDataTable, + b: InstanceAiEvalSeedDataTable, +): boolean { + return ( + JSON.stringify({ columns: a.columns, rows: a.rows }) === + JSON.stringify({ columns: b.columns, rows: b.rows }) ); } @@ -1173,10 +1386,26 @@ async function runScenario( testCaseName?: string, buildTrace?: BuildTrace, pinAiRoots?: string[], + seedContext?: ScenarioSeedContext, ): Promise { const pinNodes = pinAiRoots && pinAiRoots.length > 0 ? pinAiRoots : undefined; const targetWorkflowId = selectScenarioWorkflowId(scenario, workflowId, workflowJsons, logger); + // Reset + seed this scenario's declared rows into the tables the build bound, + // just before it runs — clears any prior scenario's (or build-time) rows so + // each scenario runs against exactly the state it declared (TRUST-311). Runs + // serially per case (see scenariosRequireSerialSeeding) to avoid racing on the + // shared table. + if (seedContext) { + await reseedScenarioTables( + client, + scenario, + seedContext.threadId, + seedContext.tableIdsByName, + logger, + ); + } + const execStart = Date.now(); let evalResult = await client.executeWithLlmMock( targetWorkflowId, diff --git a/packages/@n8n/instance-ai/evaluations/harness/schema.ts b/packages/@n8n/instance-ai/evaluations/harness/schema.ts index 17052ad7f25..2ab5cae2425 100644 --- a/packages/@n8n/instance-ai/evaluations/harness/schema.ts +++ b/packages/@n8n/instance-ai/evaluations/harness/schema.ts @@ -1,3 +1,4 @@ +import { instanceAiEvalSeedDataTableSchema } from '@n8n/api-types'; import { z } from 'zod'; import { SUPPORTED_CREDENTIAL_TYPES } from '../credentials/seeder'; @@ -26,6 +27,12 @@ const ExecutionScenarioSchema = z.object({ dataSetup: z.string(), successCriteria: z.string(), requires: z.string().optional(), + /** Typed data tables to seed before this scenario executes (TRUST-311). + * Unlike free-text `dataSetup`, this declares each column's type, so a string + * id (`row_001`) can be seeded into a `string` column rather than being + * rejected by a `number` column. Reuses the api-types seed-table schema + * (extended with optional `rows`). */ + seedDataTables: z.array(instanceAiEvalSeedDataTableSchema).max(20).optional(), }); const evalTestCaseObjectSchema = z diff --git a/packages/@n8n/instance-ai/evaluations/types.ts b/packages/@n8n/instance-ai/evaluations/types.ts index d19bd93aabb..0c1103a0577 100644 --- a/packages/@n8n/instance-ai/evaluations/types.ts +++ b/packages/@n8n/instance-ai/evaluations/types.ts @@ -2,7 +2,11 @@ // Shared types for the instance-ai workflow test case evaluator // --------------------------------------------------------------------------- -import type { InstanceAiEvalExecutionResult, InstanceAiRunDebugResponse } from '@n8n/api-types'; +import type { + InstanceAiEvalExecutionResult, + InstanceAiEvalSeedDataTable, + InstanceAiRunDebugResponse, +} from '@n8n/api-types'; import type { CheckOutcome } from './binaryChecks/types'; import type { WorkflowResponse } from './clients/n8n-client'; @@ -182,6 +186,10 @@ export interface ExecutionScenario { dataSetup: string; /** Criteria the LLM verifier checks against the execution result */ successCriteria: string; + /** Typed data tables to create + row-seed before the scenario executes + * (TRUST-311). Seeded under their exact declared names so the built + * workflow's by-name data-table references resolve. */ + seedDataTables?: InstanceAiEvalSeedDataTable[]; } export interface ConversationTurn { diff --git a/packages/cli/src/modules/instance-ai/__tests__/instance-ai.controller.test.ts b/packages/cli/src/modules/instance-ai/__tests__/instance-ai.controller.test.ts index ed5205de416..8329314737c 100644 --- a/packages/cli/src/modules/instance-ai/__tests__/instance-ai.controller.test.ts +++ b/packages/cli/src/modules/instance-ai/__tests__/instance-ai.controller.test.ts @@ -750,7 +750,9 @@ describe('InstanceAiController', () => { dataTables: [dataTable], } as InstanceAiEvalRestoreThreadRequest); - expect(evalThreadRestore.restoreDataTables).toHaveBeenCalledWith([dataTable], 'project-1'); + expect(evalThreadRestore.restoreDataTables).toHaveBeenCalledWith([dataTable], 'project-1', { + uniquifyNames: true, + }); expect(evalThreadRestore.restoreWorkflows).toHaveBeenCalledWith( [seedWorkflow], 'project-1', @@ -759,6 +761,32 @@ describe('InstanceAiController', () => { expect(result).toMatchObject({ dataTableIds: ['dt-new'] }); }); + it('seeds data tables only (no messages) under exact names when uniquifyNames is false (TRUST-311)', async () => { + memoryService.checkThreadOwnership.mockResolvedValue('owned'); + memoryService.getThreadProjectId.mockResolvedValue('project-1'); + evalThreadRestore.restoreDataTables.mockResolvedValue(new Map([['dt-old-1234', 'dt-new']])); + + const dataTable = { + id: 'dt-old-1234', + name: 'Job Applications', + columns: [{ name: 'application_id', type: 'string' as const }], + rows: [{ application_id: 'row_001' }], + }; + const result = await controller.restoreEvalThread(req, res, { + threadId: THREAD_ID, + messages: [], + dataTables: [dataTable], + uniquifyNames: false, + } as InstanceAiEvalRestoreThreadRequest); + + expect(evalThreadRestore.restoreDataTables).toHaveBeenCalledWith([dataTable], 'project-1', { + uniquifyNames: false, + }); + // No messages to restore — the message write is skipped. + expect(memoryService.restoreThreadMessages).not.toHaveBeenCalled(); + expect(result).toMatchObject({ restored: 0, dataTableIds: ['dt-new'] }); + }); + it('should roll back created workflows and data tables when a later step fails', async () => { memoryService.checkThreadOwnership.mockResolvedValue('owned'); memoryService.getThreadProjectId.mockResolvedValue('project-1'); diff --git a/packages/cli/src/modules/instance-ai/eval/__tests__/thread-restore.service.integration.test.ts b/packages/cli/src/modules/instance-ai/eval/__tests__/thread-restore.service.integration.test.ts new file mode 100644 index 00000000000..896b16e6eae --- /dev/null +++ b/packages/cli/src/modules/instance-ai/eval/__tests__/thread-restore.service.integration.test.ts @@ -0,0 +1,161 @@ +import { createTeamProject, testDb, testModules } from '@n8n/backend-test-utils'; +import type { Project } from '@n8n/db'; +import { SharedWorkflowRepository, WorkflowRepository } from '@n8n/db'; +import { Container } from '@n8n/di'; + +import { DataTableService } from '@/modules/data-table/data-table.service'; +import { DataTableValidationError } from '@/modules/data-table/errors/data-table-validation.error'; +import { mockDataTableSizeValidator } from '@/modules/data-table/__tests__/test-helpers'; + +import { EvalThreadRestoreService } from '../thread-restore.service'; + +// TRUST-311: an eval scenario can seed typed data-table rows through the restore +// path. A string id like `row_001` must land in an explicitly `string`-typed +// column (no DataTableValidationError), where a `number` column would reject it. +beforeAll(async () => { + await testModules.loadModules(['data-table']); + await testDb.init(); + mockDataTableSizeValidator(); +}); + +beforeEach(async () => { + await testDb.truncate(['DataTable', 'DataTableColumn']); +}); + +afterAll(async () => { + await testDb.terminate(); +}); + +describe('EvalThreadRestoreService.restoreDataTables (seed rows)', () => { + let service: EvalThreadRestoreService; + let dataTableService: DataTableService; + let project: Project; + + beforeAll(() => { + dataTableService = Container.get(DataTableService); + service = new EvalThreadRestoreService( + Container.get(WorkflowRepository), + Container.get(SharedWorkflowRepository), + dataTableService, + ); + }); + + beforeEach(async () => { + project = await createTeamProject(); + }); + + afterEach(async () => { + await dataTableService.deleteDataTableAll(); + }); + + it('seeds a string-id row into a string column without a validation error', async () => { + const idMap = await service.restoreDataTables( + [ + { + id: 'job-applications-1234', + name: 'Job Applications', + columns: [ + { name: 'application_id', type: 'string' }, + { name: 'is_active', type: 'boolean' }, + ], + rows: [ + { application_id: 'row_001', is_active: true }, + { application_id: 'row_002', is_active: false }, + ], + }, + ], + project.id, + ); + + const newId = idMap.get('job-applications-1234'); + expect(newId).toBeDefined(); + + const { count, data } = await dataTableService.getManyRowsAndCount(newId!, project.id, {}); + expect(count).toBe(2); + expect(data).toEqual([ + expect.objectContaining({ application_id: 'row_001', is_active: true }), + expect.objectContaining({ application_id: 'row_002', is_active: false }), + ]); + }); + + it('rejects (and rolls back) a string id declared as a number column', async () => { + await expect( + service.restoreDataTables( + [ + { + id: 'job-applications-9999', + name: 'Job Applications', + columns: [{ name: 'application_id', type: 'number' }], + rows: [{ application_id: 'row_001' }], + }, + ], + project.id, + ), + ).rejects.toThrow(DataTableValidationError); + + // Rolled back: no table survives the failed seed. + const tables = await dataTableService.getManyAndCount({ filter: { projectId: project.id } }); + expect(tables.count).toBe(0); + }); +}); + +// TRUST-311 follow-up: scenario tables are created empty before the build turn, +// then row-seeded per scenario. reseedDataTableRows must REPLACE (clear then +// insert) the existing rows, not append — so each scenario runs against exactly +// the state it declared, regardless of what a prior scenario or the build left. +describe('EvalThreadRestoreService.reseedDataTableRows', () => { + let service: EvalThreadRestoreService; + let dataTableService: DataTableService; + let project: Project; + let tableId: string; + + beforeAll(() => { + dataTableService = Container.get(DataTableService); + service = new EvalThreadRestoreService( + Container.get(WorkflowRepository), + Container.get(SharedWorkflowRepository), + dataTableService, + ); + }); + + beforeEach(async () => { + project = await createTeamProject(); + const created = await dataTableService.createDataTable(project.id, { + name: 'Job Applications', + columns: [{ name: 'application_id', type: 'string' }], + }); + tableId = created.id; + }); + + afterEach(async () => { + await dataTableService.deleteDataTableAll(); + }); + + it('seeds rows into an empty pre-created table', async () => { + await service.reseedDataTableRows(tableId, project.id, [{ application_id: 'row_001' }]); + + const { count, data } = await dataTableService.getManyRowsAndCount(tableId, project.id, {}); + expect(count).toBe(1); + expect(data).toEqual([expect.objectContaining({ application_id: 'row_001' })]); + }); + + it('replaces the prior scenario rows rather than appending them', async () => { + await service.reseedDataTableRows(tableId, project.id, [ + { application_id: 'row_001' }, + { application_id: 'row_002' }, + ]); + await service.reseedDataTableRows(tableId, project.id, [{ application_id: 'row_003' }]); + + const { count, data } = await dataTableService.getManyRowsAndCount(tableId, project.id, {}); + expect(count).toBe(1); + expect(data).toEqual([expect.objectContaining({ application_id: 'row_003' })]); + }); + + it('clears the table when seeded with no rows', async () => { + await service.reseedDataTableRows(tableId, project.id, [{ application_id: 'row_001' }]); + await service.reseedDataTableRows(tableId, project.id, []); + + const { count } = await dataTableService.getManyRowsAndCount(tableId, project.id, {}); + expect(count).toBe(0); + }); +}); diff --git a/packages/cli/src/modules/instance-ai/eval/__tests__/thread-restore.service.test.ts b/packages/cli/src/modules/instance-ai/eval/__tests__/thread-restore.service.test.ts index 36893c47b46..4ece1515e76 100644 --- a/packages/cli/src/modules/instance-ai/eval/__tests__/thread-restore.service.test.ts +++ b/packages/cli/src/modules/instance-ai/eval/__tests__/thread-restore.service.test.ts @@ -136,6 +136,74 @@ describe('EvalThreadRestoreService', () => { expect(dataTableService.insertRows).not.toHaveBeenCalled(); }); + it('seeds declared rows into the recreated table (TRUST-311)', async () => { + dataTableService.createDataTable.mockResolvedValue(mock({ id: 'dt-new' })); + + await service.restoreDataTables( + [ + { + id: 'job-applications-1234', + name: 'Job Applications', + columns: [{ name: 'id', type: 'string' }], + rows: [{ id: 'row_001' }, { id: 'row_002' }], + }, + ], + 'project-1', + ); + + // The table is created schema-only, then its rows are inserted against + // the freshly created id (not the seed id). + expect(dataTableService.insertRows).toHaveBeenCalledWith('dt-new', 'project-1', [ + { id: 'row_001' }, + { id: 'row_002' }, + ]); + }); + + it('rolls back the created table when row seeding fails', async () => { + dataTableService.createDataTable.mockResolvedValue(mock({ id: 'dt-new' })); + dataTableService.insertRows.mockRejectedValueOnce(new Error('row insert failed')); + + await expect( + service.restoreDataTables( + [ + { + id: 'job-applications-1234', + name: 'Job Applications', + columns: [{ name: 'id', type: 'string' }], + rows: [{ id: 'row_001' }], + }, + ], + 'project-1', + ), + ).rejects.toThrow('row insert failed'); + + expect(dataTableService.deleteDataTable).toHaveBeenCalledWith('dt-new', 'project-1'); + }); + + it('creates the table under its EXACT name when uniquifyNames is false (TRUST-311)', async () => { + dataTableService.createDataTable.mockResolvedValue(mock({ id: 'dt-new' })); + + await service.restoreDataTables( + [ + { + id: 'job-applications-1234', + name: 'Job Applications', + columns: [{ name: 'application_id', type: 'string' }], + rows: [{ application_id: 'row_001' }], + }, + ], + 'project-1', + { uniquifyNames: false }, + ); + + const [, dto] = dataTableService.createDataTable.mock.calls[0]; + // No ` [seed ]` suffix — the built workflow references it by this name. + expect(dto.name).toBe('Job Applications'); + expect(dataTableService.insertRows).toHaveBeenCalledWith('dt-new', 'project-1', [ + { application_id: 'row_001' }, + ]); + }); + it('rejects a too-short table id without creating anything (unsafe to string-replace)', async () => { await expect( service.restoreDataTables( diff --git a/packages/cli/src/modules/instance-ai/eval/thread-restore.service.ts b/packages/cli/src/modules/instance-ai/eval/thread-restore.service.ts index 287eb078638..2ec0f8e9ded 100644 --- a/packages/cli/src/modules/instance-ai/eval/thread-restore.service.ts +++ b/packages/cli/src/modules/instance-ai/eval/thread-restore.service.ts @@ -40,15 +40,26 @@ export class EvalThreadRestoreService { ) {} /** - * Recreate each seed data table (schema only — no rows) and map its seed id to - * the freshly created one. Tables are created under a uniquified name (names - * are unique per project; the id, which the workflow references, is what - * matters). Rolls back tables already created if a later one fails. + * Recreate each seed data table and map its seed id to the freshly created + * one. Tables are created under a uniquified name (names are unique per + * project; the id, which the workflow references, is what matters). A table + * declaring `rows` is seeded with them against its declared column types + * (TRUST-311) — free-text `dataSetup` can't declare types, so a string id + * like `row_001` would otherwise be rejected by a `number` column. Rolls back + * tables already created if a later table (or its rows) fails. + * + * `uniquifyNames` (default true) appends a unique suffix to each name to dodge + * the per-project unique-name constraint — safe when the seed workflow + * references tables by id (id-remap). Pass false to keep the EXACT declared + * name, so a freshly-built workflow's by-name references resolve (TRUST-311 + * scenario seeding). */ async restoreDataTables( dataTables: InstanceAiEvalSeedDataTable[], projectId: string, + options: { uniquifyNames?: boolean } = {}, ): Promise> { + const uniquifyNames = options.uniquifyNames ?? true; const idMap = new Map(); try { for (const table of dataTables) { @@ -59,13 +70,20 @@ export class EvalThreadRestoreService { `Seed data table id "${table.id}" is too short to remap safely (need ≥8 chars)`, ); } - const suffix = ` [seed ${randomUUID().slice(0, 8)}]`; - const name = `${table.name.slice(0, 128 - suffix.length)}${suffix}`; + let name = table.name; + if (uniquifyNames) { + const suffix = ` [seed ${randomUUID().slice(0, 8)}]`; + name = `${table.name.slice(0, 128 - suffix.length)}${suffix}`; + } const created = await this.dataTableService.createDataTable(projectId, { name, columns: table.columns, }); + // Map before seeding rows so a row-insert failure rolls this table back too. idMap.set(table.id, created.id); + if (table.rows && table.rows.length > 0) { + await this.dataTableService.insertRows(created.id, projectId, table.rows); + } } } catch (error) { await this.deleteDataTables([...idMap.values()], projectId); @@ -74,6 +92,25 @@ export class EvalThreadRestoreService { return idMap; } + /** + * Reset an existing data table's rows to exactly `rows` (clear-then-insert). + * Used for the per-scenario row seeding of a case whose tables were created + * empty before the build turn (TRUST-311 follow-up): the table already exists + * (the built workflow bound its id), so we only swap the rows a scenario + * declares — clearing whatever a prior scenario or a build-time execution + * left. Rows are validated against each column's type by `insertRows`. + */ + async reseedDataTableRows( + tableId: string, + projectId: string, + rows: NonNullable, + ): Promise { + await this.dataTableService.clearRows(tableId, projectId); + if (rows.length > 0) { + await this.dataTableService.insertRows(tableId, projectId, rows); + } + } + /** Best-effort delete (rollback of a failed restore). */ async deleteDataTables(dataTableIds: string[], projectId: string): Promise { for (const id of dataTableIds) { diff --git a/packages/cli/src/modules/instance-ai/instance-ai.controller.ts b/packages/cli/src/modules/instance-ai/instance-ai.controller.ts index 1e74d48eabc..4bf940d3462 100644 --- a/packages/cli/src/modules/instance-ai/instance-ai.controller.ts +++ b/packages/cli/src/modules/instance-ai/instance-ai.controller.ts @@ -16,6 +16,7 @@ import { InstanceAiEvalExecutionRequest, InstanceAiEvalCredentialAllowlistRequest, InstanceAiEvalRestoreThreadRequest, + InstanceAiEvalSeedDataTableRowsRequest, normalizeInstanceAiThreadSource, } from '@n8n/api-types'; import type { InstanceAiAgentNode, InstanceAiEvent } from '@n8n/api-types'; @@ -974,11 +975,12 @@ export class InstanceAiController { const idMap = await this.evalThreadRestore.restoreDataTables( payload.dataTables ?? [], projectId, + { uniquifyNames: payload.uniquifyNames ?? true }, ); const dataTableIds = [...idMap.values()]; // Roll back everything we created if a later step fails, so a partial // restore doesn't leak workflows/tables into the shared eval project. - let restored: number; + let restored = 0; let createdWorkflowIds: string[] = []; try { createdWorkflowIds = await this.evalThreadRestore.restoreWorkflows( @@ -986,11 +988,14 @@ export class InstanceAiController { projectId, idMap, ); - ({ restored } = await this.memoryService.restoreThreadMessages( - req.user.id, - payload.threadId, - payload.messages, - )); + // A data-table-only seed (TRUST-311) sends no messages — skip the write. + if (payload.messages.length > 0) { + ({ restored } = await this.memoryService.restoreThreadMessages( + req.user.id, + payload.threadId, + payload.messages, + )); + } } catch (error) { await this.evalThreadRestore.deleteWorkflows(createdWorkflowIds); await this.evalThreadRestore.deleteDataTables(dataTableIds, projectId); @@ -1005,6 +1010,31 @@ export class InstanceAiController { }; } + /** + * Reset an existing data table's rows to exactly the supplied set + * (clear-then-insert). The eval harness pre-creates a case's scenario data + * tables empty before the build turn (so the agent binds the real table id), + * then calls this per scenario to swap in that scenario's rows (TRUST-311 + * follow-up). Auth + project scoping mirror restore-thread: the table must be + * in the thread's project. + */ + @Post('/eval/seed-data-table-rows') + @GlobalScope('instanceAi:eval') + async seedEvalDataTableRows( + req: AuthenticatedRequest, + _res: Response, + @Body payload: InstanceAiEvalSeedDataTableRowsRequest, + ) { + this.requireInstanceAiEnabled(); + await this.assertThreadAccess(req.user.id, payload.threadId); + const projectId = await this.memoryService.getThreadProjectId(payload.threadId); + if (!projectId) { + throw new BadRequestError('Thread is not bound to a project'); + } + await this.evalThreadRestore.reseedDataTableRows(payload.tableId, projectId, payload.rows); + return { ok: true, tableId: payload.tableId, rowCount: payload.rows.length }; + } + // ── Gateway endpoints (daemon ↔ server) ────────────────────────────────── @Post('/gateway/create-link')