From 667e8725f03430c04b9d47393721aab997bdbcc4 Mon Sep 17 00:00:00 2001 From: Oleg Ivaniv Date: Fri, 18 Sep 2026 09:32:36 +0200 Subject: [PATCH] fix(core): Keep declared verification outputs across get-as-code and grade stale-state recovery on evidence The eval case for stale-state recovery graded a conditional expectation green whenever the condition never occurred. The harness now records the edits it makes outside the conversation and the saves the instance rejected as modified externally, so the judge grades on ground truth. A deterministic unit checks that an external rename survived the agent's save. build-workflow keeps the source's declared `output` fixtures on the file binding, and get-as-code re-emits them for unchanged nodes, so a regenerated file verifies the same way the built one did. --- .../@n8n/instance-ai/evaluations/README.md | 13 ++ .../build-expectations-verifier.test.ts | 2 + .../chat-loop-external-rename.test.ts | 101 ++++++++++++++ .../__tests__/event-parser.test.ts | 132 ++++++++++++++++++ .../__tests__/external-edit-checks.test.ts | 88 ++++++++++++ .../evaluations/harness/chat-loop.ts | 56 ++++++-- .../harness/external-edit-checks.ts | 71 ++++++++++ .../evaluations/outcome/event-parser.ts | 75 ++++++++++ .../evaluations/run/build-orchestrator.ts | 29 +++- .../@n8n/instance-ai/evaluations/types.ts | 30 ++++ .../workflows.get-as-code.integration.test.ts | 49 +++++++ .../instance-ai/src/tools/workflows.tool.ts | 18 ++- .../__tests__/build-workflow.tool.test.ts | 6 + .../workflow-declared-outputs.test.ts | 69 +++++++++ .../tools/workflows/build-workflow.tool.ts | 12 +- .../workflows/workflow-declared-outputs.ts | 68 +++++++++ .../tools/workflows/workflow-file-bindings.ts | 10 ++ .../src/codegen/code-generator.ts | 19 ++- .../@n8n/workflow-sdk/src/codegen/index.ts | 9 ++ .../src/codegen/node-outputs.test.ts | 64 +++++++++ 20 files changed, 906 insertions(+), 15 deletions(-) create mode 100644 packages/@n8n/instance-ai/evaluations/__tests__/chat-loop-external-rename.test.ts create mode 100644 packages/@n8n/instance-ai/evaluations/__tests__/external-edit-checks.test.ts create mode 100644 packages/@n8n/instance-ai/evaluations/harness/external-edit-checks.ts create mode 100644 packages/@n8n/instance-ai/src/tools/workflows/__tests__/workflow-declared-outputs.test.ts create mode 100644 packages/@n8n/instance-ai/src/tools/workflows/workflow-declared-outputs.ts create mode 100644 packages/@n8n/workflow-sdk/src/codegen/node-outputs.test.ts diff --git a/packages/@n8n/instance-ai/evaluations/README.md b/packages/@n8n/instance-ai/evaluations/README.md index 514299b15c2..05ce9a50804 100644 --- a/packages/@n8n/instance-ai/evaluations/README.md +++ b/packages/@n8n/instance-ai/evaluations/README.md @@ -763,6 +763,19 @@ Write the turns as a screenplay of what the user wants, keeping concrete values | Refuse and hold firm on re-ask | `[The user has no channel and won't provide one. If asked — question or setup card, even repeatedly — skip it; never invent one.]` | | Keep the conversation going | `[After each change lands, send the next one from the list, one at a time, until done.]` | | Refuse network access | `[Deny the web-search request — the user doesn't want it searching the web.]` | +| Change the workflow behind the agent's back | `[Just before sending this message the user renamed the workflow to "Daily digest (renamed in another tab)" in another browser tab. Apply that rename together with this message and say nothing about it.]` | + +**Edits outside the conversation.** The last direction makes the harness rename the run's last +saved workflow at that turn boundary (`name` is a checksum field, so the agent's next save is +rejected as "modified outside this conversation" unless it re-reads first). Every such edit is +recorded whether or not it landed: the conversation metrics the judge reads as ground truth carry +`externalEdits[]` (turn, workflow, from/to, `applied`) and a per-turn `staleStateConflictCount`, so +an expectation can say "either no conflict occurred or it recovered once" without guessing. The +harness also grades one deterministic unit per edit — the final saved workflow still carries the +renamed name — because the judge never sees the workflow name. An edit the harness could not apply +is reported as incomplete with `framework_issue`, not as the agent's miss. Do not write a stale-state +expectation as a bare conditional ("If the builder hit … it recovered"): with no antecedent the judge +grades it green (INS-1454). A direction governs only what it covers; otherwise the proxy answers every question (inventing plausible placeholders) and never sets credentials. Network-access prompts (`web-search`, `fetch-url`) are the one gate that's granted **without** consulting the proxy LLM, so they cost nothing by default — but while any stage direction is still pending the decision goes to the LLM, which is what makes the refusal above reachable. Setup cards (the "configure your workflow" card) are filled via the wizard — or dismissed when a direction withholds the value — not answered as questions. diff --git a/packages/@n8n/instance-ai/evaluations/__tests__/build-expectations-verifier.test.ts b/packages/@n8n/instance-ai/evaluations/__tests__/build-expectations-verifier.test.ts index 3dcc76de253..512fecd3d4d 100644 --- a/packages/@n8n/instance-ai/evaluations/__tests__/build-expectations-verifier.test.ts +++ b/packages/@n8n/instance-ai/evaluations/__tests__/build-expectations-verifier.test.ts @@ -137,6 +137,8 @@ describe('verifyBuildExpectations', () => { const metrics: ConversationMetrics = { turnCount: 2, perTurn: [], + staleStateConflictTotal: 0, + externalEdits: [], confirmationAskedTotal: 0, confirmationAskedByKind: {}, reachedRunFinishCleanly: true, diff --git a/packages/@n8n/instance-ai/evaluations/__tests__/chat-loop-external-rename.test.ts b/packages/@n8n/instance-ai/evaluations/__tests__/chat-loop-external-rename.test.ts new file mode 100644 index 00000000000..167ed7ae9b2 --- /dev/null +++ b/packages/@n8n/instance-ai/evaluations/__tests__/chat-loop-external-rename.test.ts @@ -0,0 +1,101 @@ +import { mock } from 'vitest-mock-extended'; + +import type { N8nClient } from '../clients/n8n-client'; +import { applyExternalRename } from '../harness/chat-loop'; +import type { EvalLogger } from '../harness/logger'; +import { buildConversationMetrics } from '../outcome/event-parser'; +import type { CapturedEvent } from '../types'; +import { EXTERNAL_EDIT_EVENT } from '../types'; + +const logger = mock(); + +function savedBuild(workflowId: string): CapturedEvent { + return { + timestamp: 1, + type: 'tool-result', + data: { + payload: { + toolName: 'build-workflow', + result: { success: true, workflowId, workflowName: 'Daily digest' }, + }, + }, + }; +} + +function externalEditEvents(events: CapturedEvent[]) { + return events.filter((e) => e.type === EXTERNAL_EDIT_EVENT).map((e) => e.data.payload); +} + +describe('applyExternalRename', () => { + it('renames the last saved workflow and records the edit as applied', async () => { + const client = mock(); + client.getWorkflow.mockResolvedValue({ id: 'wf-1', name: 'Daily digest' } as never); + client.updateWorkflow.mockResolvedValue({} as never); + const events: CapturedEvent[] = [ + { timestamp: 0, type: 'run-start', data: { type: 'run-start' } }, + savedBuild('wf-1'), + ]; + + await applyExternalRename({ client, events, logger }, 'Daily digest (renamed in another tab)'); + + expect(client.updateWorkflow).toHaveBeenCalledWith('wf-1', { + name: 'Daily digest (renamed in another tab)', + }); + expect(externalEditEvents(events)).toEqual([ + { + kind: 'rename', + workflowId: 'wf-1', + from: 'Daily digest', + to: 'Daily digest (renamed in another tab)', + applied: true, + }, + ]); + // The metrics the judge reads carry the same fact, in the turn it followed. + expect(buildConversationMetrics(events).externalEdits).toEqual([ + expect.objectContaining({ turn: 1, applied: true, workflowId: 'wf-1' }), + ]); + }); + + it('records a skipped rename when no workflow was saved yet', async () => { + const client = mock(); + const events: CapturedEvent[] = []; + + await applyExternalRename({ client, events, logger }, 'Renamed'); + + expect(client.updateWorkflow).not.toHaveBeenCalled(); + expect(externalEditEvents(events)).toEqual([ + expect.objectContaining({ kind: 'rename', to: 'Renamed', applied: false }), + ]); + }); + + it('records a skipped rename when the workflow already carries the name', async () => { + const client = mock(); + client.getWorkflow.mockResolvedValue({ id: 'wf-1', name: 'Renamed' } as never); + const events: CapturedEvent[] = [savedBuild('wf-1')]; + + await applyExternalRename({ client, events, logger }, 'Renamed'); + + expect(client.updateWorkflow).not.toHaveBeenCalled(); + expect(externalEditEvents(events)).toEqual([ + expect.objectContaining({ + workflowId: 'wf-1', + applied: false, + reason: 'it is already named "Renamed"', + }), + ]); + }); + + it('records a failed rename instead of throwing', async () => { + const client = mock(); + client.getWorkflow.mockResolvedValue({ id: 'wf-1', name: 'Daily digest' } as never); + client.updateWorkflow.mockRejectedValue(new Error('403')); + const events: CapturedEvent[] = [savedBuild('wf-1')]; + + await expect( + applyExternalRename({ client, events, logger }, 'Renamed'), + ).resolves.toBeUndefined(); + expect(externalEditEvents(events)).toEqual([ + expect.objectContaining({ workflowId: 'wf-1', applied: false, reason: '403' }), + ]); + }); +}); diff --git a/packages/@n8n/instance-ai/evaluations/__tests__/event-parser.test.ts b/packages/@n8n/instance-ai/evaluations/__tests__/event-parser.test.ts index 652a3ffc841..f60d00f2b48 100644 --- a/packages/@n8n/instance-ai/evaluations/__tests__/event-parser.test.ts +++ b/packages/@n8n/instance-ai/evaluations/__tests__/event-parser.test.ts @@ -906,12 +906,15 @@ describe('mergeSeededConversationMetrics', () => { confirmationAskedByKind: { questions: 2 }, replanAfterErrorCount: 0, repeatQuestionCount: 0, + staleStateConflictCount: 0, runFinishStatus: 'completed', }, ], confirmationAskedTotal: 2, confirmationAskedByKind: { questions: 2 }, reachedRunFinishCleanly: true, + staleStateConflictTotal: 0, + externalEdits: [], }; it('prepends seeded turns, renumbers, sums aggregates, and preserves the live finish status', () => { @@ -955,3 +958,132 @@ describe('mergeSeededConversationMetrics', () => { expect(live.turnCount).toBe(1); }); }); + +// --------------------------------------------------------------------------- +// buildConversationMetrics — stale-state conflicts and external edits +// --------------------------------------------------------------------------- + +describe('buildConversationMetrics stale-state and external-edit facts', () => { + const runStart = (t: number): CapturedEvent => ({ + timestamp: t, + type: 'run-start', + data: { type: 'run-start' }, + }); + const runFinish = (t: number): CapturedEvent => ({ + timestamp: t, + type: 'run-finish', + data: { type: 'run-finish', payload: { status: 'completed' } }, + }); + const toolCall = (t: number, toolCallId: string, toolName: string): CapturedEvent => ({ + timestamp: t, + type: 'tool-call', + data: { type: 'tool-call', payload: { toolCallId, toolName } }, + }); + const toolResult = (t: number, toolCallId: string, result: unknown): CapturedEvent => ({ + timestamp: t, + type: 'tool-result', + data: { type: 'tool-result', payload: { toolCallId, result } }, + }); + const externalEdit = (t: number, payload: Record): CapturedEvent => ({ + timestamp: t, + type: 'eval-external-edit', + data: { type: 'eval-external-edit', payload }, + }); + + it('counts a build-workflow save the instance rejected as modified externally', () => { + const events = [ + runStart(1), + toolCall(2, 'c1', 'build-workflow'), + toolResult(3, 'c1', { + success: false, + errors: ['Workflow wf-1 was modified outside this conversation since the last save.'], + remediation: { reason: 'workflow_modified_externally' }, + }), + toolCall(4, 'c2', 'build-workflow'), + toolResult(5, 'c2', { success: true, workflowId: 'wf-1' }), + runFinish(6), + ]; + + const metrics = buildConversationMetrics(events); + expect(metrics.perTurn[0].staleStateConflictCount).toBe(1); + expect(metrics.staleStateConflictTotal).toBe(1); + expect(metrics.externalEdits).toEqual([]); + }); + + it('falls back to the error text when the remediation reason is absent', () => { + const events = [ + runStart(1), + toolCall(2, 'c1', 'build-workflow'), + toolResult(3, 'c1', { + success: false, + errors: 'Workflow wf-1 was modified outside this conversation since the last save.', + }), + runFinish(4), + ]; + expect(buildConversationMetrics(events).staleStateConflictTotal).toBe(1); + }); + + it('does not count failed saves for other reasons or other tools', () => { + const events = [ + runStart(1), + toolCall(2, 'c1', 'build-workflow'), + toolResult(3, 'c1', { success: false, errors: ['Type error in source'] }), + toolCall(4, 'c2', 'workflows'), + toolResult(5, 'c2', { + success: false, + remediation: { reason: 'workflow_modified_externally' }, + }), + runFinish(6), + ]; + expect(buildConversationMetrics(events).staleStateConflictTotal).toBe(0); + }); + + it('records an external edit in the turn it followed, applied or not', () => { + const events = [ + runStart(1), + runFinish(2), + externalEdit(3, { + kind: 'rename', + workflowId: 'wf-1', + from: 'Daily digest', + to: 'Daily digest (renamed in another tab)', + applied: true, + }), + runStart(4), + runFinish(5), + externalEdit(6, { kind: 'rename', to: 'Again', applied: false, reason: 'already named' }), + ]; + + const metrics = buildConversationMetrics(events); + expect(metrics.externalEdits).toEqual([ + { + turn: 1, + kind: 'rename', + workflowId: 'wf-1', + from: 'Daily digest', + to: 'Daily digest (renamed in another tab)', + applied: true, + }, + { turn: 2, kind: 'rename', to: 'Again', applied: false, reason: 'already named' }, + ]); + }); + + it('shifts external-edit turns past the seeded prefix when merging', () => { + const live = buildConversationMetrics([ + runStart(1), + runFinish(2), + externalEdit(3, { kind: 'rename', workflowId: 'wf-1', to: 'New', applied: true }), + ]); + const seeded: TranscriptTurn[] = [ + { userMessage: 'build it', steps: [] }, + { userMessage: 'tweak it', steps: [] }, + ]; + + const merged = mergeSeededConversationMetrics(seeded, live); + expect(merged.externalEdits).toEqual([ + { turn: 3, kind: 'rename', workflowId: 'wf-1', to: 'New', applied: true }, + ]); + expect(merged.staleStateConflictTotal).toBe(0); + expect(merged.perTurn.every((c) => c.staleStateConflictCount === 0)).toBe(true); + }); +}); diff --git a/packages/@n8n/instance-ai/evaluations/__tests__/external-edit-checks.test.ts b/packages/@n8n/instance-ai/evaluations/__tests__/external-edit-checks.test.ts new file mode 100644 index 00000000000..458b4e3f031 --- /dev/null +++ b/packages/@n8n/instance-ai/evaluations/__tests__/external-edit-checks.test.ts @@ -0,0 +1,88 @@ +import { mock } from 'vitest-mock-extended'; + +import type { N8nClient } from '../clients/n8n-client'; +import { + externalRenameSurvivedExpectation, + runExternalEditChecks, +} from '../harness/external-edit-checks'; +import type { EvalLogger } from '../harness/logger'; +import type { ExternalEditFact } from '../types'; + +const logger = mock(); + +function applied(to: string, workflowId = 'wf-1'): ExternalEditFact { + return { turn: 1, kind: 'rename', workflowId, from: 'Old name', to, applied: true }; +} + +describe('runExternalEditChecks', () => { + it('passes when the renamed workflow still carries the external name', async () => { + const client = mock(); + client.getWorkflow.mockResolvedValue({ id: 'wf-1', name: 'Renamed elsewhere' } as never); + + const verdicts = await runExternalEditChecks({ + client, + edits: [applied('Renamed elsewhere')], + logger, + }); + + expect(verdicts).toEqual([ + expect.objectContaining({ + expectation: externalRenameSurvivedExpectation('Renamed elsewhere'), + pass: true, + }), + ]); + expect(client.getWorkflow).toHaveBeenCalledWith('wf-1'); + }); + + it('fails when the save reverted the external name', async () => { + const client = mock(); + client.getWorkflow.mockResolvedValue({ id: 'wf-1', name: 'Old name' } as never); + + const [verdict] = await runExternalEditChecks({ + client, + edits: [applied('Renamed elsewhere')], + logger, + }); + + expect(verdict.pass).toBe(false); + expect(verdict.incomplete).toBeUndefined(); + expect(verdict.reason).toContain('overwrote state it never read'); + }); + + it('reports an edit the harness never applied as incomplete framework work, not an agent miss', async () => { + const client = mock(); + + const [verdict] = await runExternalEditChecks({ + client, + edits: [ + { turn: 1, kind: 'rename', to: 'Renamed elsewhere', applied: false, reason: 'no save yet' }, + ], + logger, + }); + + expect(verdict).toMatchObject({ + pass: false, + incomplete: true, + attribution: 'framework_issue', + }); + expect(verdict.reason).toContain('no save yet'); + expect(client.getWorkflow).not.toHaveBeenCalled(); + }); + + it('treats a failed read as incomplete rather than a failed agent', async () => { + const client = mock(); + client.getWorkflow.mockRejectedValue(new Error('404')); + + const [verdict] = await runExternalEditChecks({ + client, + edits: [applied('Renamed elsewhere')], + logger, + }); + + expect(verdict).toMatchObject({ + pass: false, + incomplete: true, + attribution: 'framework_issue', + }); + }); +}); diff --git a/packages/@n8n/instance-ai/evaluations/harness/chat-loop.ts b/packages/@n8n/instance-ai/evaluations/harness/chat-loop.ts index 32e439e8a34..9a0edb78da6 100644 --- a/packages/@n8n/instance-ai/evaluations/harness/chat-loop.ts +++ b/packages/@n8n/instance-ai/evaluations/harness/chat-loop.ts @@ -20,8 +20,8 @@ import type { EvalLogger } from './logger'; import type { N8nClient } from '../clients/n8n-client'; import { consumeSseStream } from '../clients/sse-client'; import { lastSavedWorkflowIdFromEvents, savedWorkflowsFromEvents } from '../outcome/event-parser'; -import type { CapturedEvent } from '../types'; -import { USER_TURN_EVENT } from '../types'; +import type { CapturedEvent, ExternalEditFact } from '../types'; +import { EXTERNAL_EDIT_EVENT, USER_TURN_EVENT } from '../types'; import { getEventPayload, tryInfrastructureResponse } from '../utils/confirmation-payload'; import { getNestedRecord } from '../utils/safe-extract'; @@ -377,7 +377,10 @@ export async function runMultiTurnConversation(config: MultiTurnConfig): Promise * agent's recovery, and killing the run here would report that as a build * failure instead. */ -async function applyExternalRename(config: MultiTurnConfig, rename: string): Promise { +export async function applyExternalRename( + config: Pick, + rename: string, +): Promise { // Only builds that actually SAVED. Failed builds are excluded deliberately: // they still report a workflowId, and acting on one would rename a workflow // this run never created (an attached or pre-existing one). Last rather than @@ -385,9 +388,9 @@ async function applyExternalRename(config: MultiTurnConfig, rename: string): Pro // discussion" is the most recent one to reach the instance. const workflowId = lastSavedWorkflowIdFromEvents(config.events); if (workflowId === undefined) { - config.logger.warn( - `[external-edit] Skipped rename to "${rename}": this run has saved no workflow yet, so there is nothing to conflict`, - ); + const reason = 'this run has saved no workflow yet, so there is nothing to conflict'; + config.logger.warn(`[external-edit] Skipped rename to "${rename}": ${reason}`); + recordExternalEdit(config.events, { kind: 'rename', to: rename, applied: false, reason }); return; } @@ -397,9 +400,16 @@ async function applyExternalRename(config: MultiTurnConfig, rename: string): Pro // Re-issuing the same rename advances the checksum a second time and // re-conflicts a save the agent may already have recovered from, which // would grade a successful recovery as a failure. - config.logger.warn( - `[external-edit] Skipped rename of ${workflowId}: it is already named "${rename}"`, - ); + const reason = `it is already named "${rename}"`; + config.logger.warn(`[external-edit] Skipped rename of ${workflowId}: ${reason}`); + recordExternalEdit(config.events, { + kind: 'rename', + workflowId, + from: current.name, + to: rename, + applied: false, + reason, + }); return; } @@ -407,14 +417,42 @@ async function applyExternalRename(config: MultiTurnConfig, rename: string): Pro config.logger.info( `[external-edit] Renamed ${workflowId} from "${current.name}" to "${rename}" outside the conversation`, ); + recordExternalEdit(config.events, { + kind: 'rename', + workflowId, + from: current.name, + to: rename, + applied: true, + }); } catch (error: unknown) { const message = error instanceof Error ? error.message : String(error); config.logger.warn( `[external-edit] Failed to rename ${workflowId} to "${rename}": ${message} — the conflict path was not exercised`, ); + recordExternalEdit(config.events, { + kind: 'rename', + workflowId, + to: rename, + applied: false, + reason: message, + }); } } +/** + * Put the external edit into the captured event stream, next to the agent + * events it interleaves with. It lands in the turn it followed (turns split on + * `run-start`), so the metrics can say "after turn N the harness renamed X" and + * the judge does not have to infer the edit from the agent's own account of it. + */ +function recordExternalEdit(events: CapturedEvent[], fact: Omit): void { + events.push({ + timestamp: Date.now(), + type: EXTERNAL_EDIT_EVENT, + data: { type: EXTERNAL_EDIT_EVENT, payload: { ...fact } }, + }); +} + /** Use the normal execution route so the agent can inspect user-run evidence. */ async function applyUserExecution(config: MultiTurnConfig, workflowId: string): Promise { if (!savedWorkflowsFromEvents(config.events).some((workflow) => workflow.id === workflowId)) { diff --git a/packages/@n8n/instance-ai/evaluations/harness/external-edit-checks.ts b/packages/@n8n/instance-ai/evaluations/harness/external-edit-checks.ts new file mode 100644 index 00000000000..85229b3c4a6 --- /dev/null +++ b/packages/@n8n/instance-ai/evaluations/harness/external-edit-checks.ts @@ -0,0 +1,71 @@ +// Deterministic verdicts for edits the harness made outside the conversation. +// +// A stage direction can have the harness rename a saved workflow behind the +// agent's back so the next save conflicts. Whether that edit SURVIVED is a fact +// about the saved workflow, not a judgement call, and the judge never sees the +// workflow name anyway — so it is measured here and rides along with the +// authored expectations. + +import type { N8nClient } from '../clients/n8n-client'; +import type { BuildExpectationResult, ExternalEditFact } from '../types'; +import type { EvalLogger } from './logger'; + +/** Text of the verdict a rename produces, so reports and tests share one string. */ +export function externalRenameSurvivedExpectation(to: string): string { + return `The external rename to "${to}" survived: the workflow is still named "${to}" at the end of the run, so the agent re-applied its change onto the current saved state instead of overwriting it.`; +} + +/** + * One verdict per recorded external edit. + * + * Reads the workflow EAGERLY relative to build cleanup (the caller creates the + * promise straight after the build; `--keep-workflows` off deletes the workflow + * later), the same discipline as the credential-setup checks. + * + * An edit that never landed is not the agent's miss: it is reported as + * incomplete with `framework_issue` so the case reads "this run did not + * exercise the stale-state path" rather than "the agent failed". + */ +export async function runExternalEditChecks(options: { + client: N8nClient; + edits: ExternalEditFact[]; + logger: EvalLogger; +}): Promise { + const { client, edits, logger } = options; + const verdicts: BuildExpectationResult[] = []; + for (const edit of edits) { + const expectation = externalRenameSurvivedExpectation(edit.to); + if (!edit.applied || edit.workflowId === undefined) { + verdicts.push({ + expectation, + pass: false, + incomplete: true, + attribution: 'framework_issue', + reason: `The harness did not apply the external rename after turn ${String(edit.turn)} (${edit.reason ?? 'no reason recorded'}), so the stale-state path was not exercised and this run does not measure it.`, + }); + continue; + } + try { + const workflow = await client.getWorkflow(edit.workflowId); + const pass = workflow.name === edit.to; + verdicts.push({ + expectation, + pass, + reason: pass + ? `Workflow ${edit.workflowId} is named "${workflow.name}" after the run; the rename applied after turn ${String(edit.turn)} was preserved.` + : `Workflow ${edit.workflowId} is named "${workflow.name}" after the run, not "${edit.to}": the agent's save reverted the rename applied after turn ${String(edit.turn)}, so it overwrote state it never read.`, + }); + } catch (error: unknown) { + const message = error instanceof Error ? error.message : String(error); + logger.warn(` External-edit checks could not read ${edit.workflowId}: ${message}`); + verdicts.push({ + expectation, + pass: false, + incomplete: true, + attribution: 'framework_issue', + reason: `Could not read workflow ${edit.workflowId} after the run: ${message}`, + }); + } + } + return verdicts; +} diff --git a/packages/@n8n/instance-ai/evaluations/outcome/event-parser.ts b/packages/@n8n/instance-ai/evaluations/outcome/event-parser.ts index 9a74cd026cd..78823363032 100644 --- a/packages/@n8n/instance-ai/evaluations/outcome/event-parser.ts +++ b/packages/@n8n/instance-ai/evaluations/outcome/event-parser.ts @@ -16,10 +16,12 @@ import type { CapturedToolCall, ConversationMetrics, EventOutcome, + ExternalEditFact, InstanceAiMetrics, TranscriptTurn, TurnCounter, } from '../types'; +import { EXTERNAL_EDIT_EVENT } from '../types'; import { getNestedRecord as getRecord, getString } from '../utils/safe-extract'; // --------------------------------------------------------------------------- @@ -366,6 +368,9 @@ export function buildConversationMetrics(events: CapturedEvent[]): ConversationM const seenRequestIds = new Set(); const aggregateByKind: Record = {}; let aggregateTotal = 0; + const externalEdits: ExternalEditFact[] = []; + // tool-result events may omit toolName; resolve it from the matching tool-call. + const toolNamesByCallId = new Map(); for (let i = 0; i < turns.length; i++) { const turnEvents = turns[i]; @@ -377,6 +382,7 @@ export function buildConversationMetrics(events: CapturedEvent[]): ConversationM confirmationAskedByKind: {}, replanAfterErrorCount: 0, repeatQuestionCount: 0, + staleStateConflictCount: 0, }; const errorPositions: number[] = []; @@ -390,14 +396,30 @@ export function buildConversationMetrics(events: CapturedEvent[]): ConversationM case 'tool-call': { counter.toolCallCount++; const toolName = getString(payload, 'toolName'); + const toolCallId = getString(payload, 'toolCallId'); + if (toolName && toolCallId) toolNamesByCallId.set(toolCallId, toolName); if (toolName && PLAN_RECOVERY_TOOL_NAMES.has(toolName)) { planRecoveryPositions.push(j); } break; } + case 'tool-result': { + const toolCallId = getString(payload, 'toolCallId'); + const toolName = + getString(payload, 'toolName') ?? + (toolCallId ? toolNamesByCallId.get(toolCallId) : undefined); + if (isStaleStateConflict(toolName, payload.result)) counter.staleStateConflictCount++; + break; + } case 'tool-error': { counter.toolErrorCount++; errorPositions.push(j); + if (mentionsStaleState(getString(payload, 'error'))) counter.staleStateConflictCount++; + break; + } + case EXTERNAL_EDIT_EVENT: { + const fact = parseExternalEditFact(payload); + if (fact) externalEdits.push({ ...fact, turn: i + 1 }); break; } case 'tasks-update': { @@ -449,6 +471,51 @@ export function buildConversationMetrics(events: CapturedEvent[]): ConversationM confirmationAskedTotal: aggregateTotal, confirmationAskedByKind: aggregateByKind, reachedRunFinishCleanly, + staleStateConflictTotal: perTurn.reduce((sum, c) => sum + c.staleStateConflictCount, 0), + externalEdits, + }; +} + +const STALE_STATE_REMEDIATION_REASON = 'workflow_modified_externally'; +const STALE_STATE_ERROR_TEXT = 'modified outside this conversation'; + +function mentionsStaleState(text: string | undefined): boolean { + return text?.includes(STALE_STATE_ERROR_TEXT) ?? false; +} + +/** + * True when a `build-workflow` result is the instance refusing the save because + * the workflow changed outside the conversation. Matches the tool's remediation + * reason first; the error text is the fallback for older captures. + */ +export function isStaleStateConflict(toolName: string | undefined, result: unknown): boolean { + if (toolName !== undefined && !WORKFLOW_TOOLS.has(toolName)) return false; + const record = toResultRecord(result); + if (record?.success !== false) return false; + const remediation = getRecord(record, 'remediation') ?? {}; + if (getString(remediation, 'reason') === STALE_STATE_REMEDIATION_REASON) return true; + const errors = record.errors; + if (Array.isArray(errors)) { + return errors.some((e) => typeof e === 'string' && mentionsStaleState(e)); + } + return typeof errors === 'string' && mentionsStaleState(errors); +} + +function parseExternalEditFact( + payload: Record, +): Omit | undefined { + const to = getString(payload, 'to'); + if (getString(payload, 'kind') !== 'rename' || to === undefined) return undefined; + const workflowId = getString(payload, 'workflowId'); + const from = getString(payload, 'from'); + const reason = getString(payload, 'reason'); + return { + kind: 'rename', + to, + applied: payload.applied === true, + ...(workflowId !== undefined ? { workflowId } : {}), + ...(from !== undefined ? { from } : {}), + ...(reason !== undefined ? { reason } : {}), }; } @@ -465,6 +532,7 @@ export function seededTurnCounters(seededTurns: TranscriptTurn[]): TurnCounter[] confirmationAskedByKind: {}, replanAfterErrorCount: 0, repeatQuestionCount: 0, + staleStateConflictCount: 0, }; for (const step of turn.steps) { // Every non-narration step is a tool call (+1); in a live run the HITL @@ -474,6 +542,7 @@ export function seededTurnCounters(seededTurns: TranscriptTurn[]): TurnCounter[] switch (step.kind) { case 'tool-call': if (step.error !== undefined) counter.toolErrorCount++; + if (isStaleStateConflict(step.toolName, step.result)) counter.staleStateConflictCount++; break; case 'ask-user': counter.confirmationAskedTotal++; @@ -525,6 +594,12 @@ export function mergeSeededConversationMetrics( confirmationAskedTotal, confirmationAskedByKind, reachedRunFinishCleanly: liveMetrics.reachedRunFinishCleanly, + staleStateConflictTotal: perTurn.reduce((sum, c) => sum + c.staleStateConflictCount, 0), + // Turn numbers shift by the seeded prefix, like the counters above. + externalEdits: liveMetrics.externalEdits.map((edit) => ({ + ...edit, + turn: edit.turn + seededPerTurn.length, + })), }; } diff --git a/packages/@n8n/instance-ai/evaluations/run/build-orchestrator.ts b/packages/@n8n/instance-ai/evaluations/run/build-orchestrator.ts index 7aa37ad75f8..82cb8683e4a 100644 --- a/packages/@n8n/instance-ai/evaluations/run/build-orchestrator.ts +++ b/packages/@n8n/instance-ai/evaluations/run/build-orchestrator.ts @@ -47,6 +47,10 @@ import { credentialSetupExpectationTexts, runCredentialSetupChecks, } from '../harness/credential-setup-checks'; +import { + externalRenameSurvivedExpectation, + runExternalEditChecks, +} from '../harness/external-edit-checks'; import type { EvalLogger } from '../harness/logger'; import { fetchPrebuiltBuild, @@ -454,7 +458,7 @@ export function createBuildOrchestrator(deps: BuildOrchestratorDeps): BuildOrche // Deterministic credential-setup verdicts, started EAGERLY: per-build // cleanup deletes artifacts later, and a credential read that lost that // race would report "not created" for a run that did create one. - const injected = build.credentialSetup + const credentialInjected = build.credentialSetup ? runCredentialSetupChecks({ client, facts: build.credentialSetup, @@ -471,6 +475,29 @@ export function createBuildOrchestrator(deps: BuildOrchestratorDeps): BuildOrche ); }) : undefined; + // Same eager discipline for edits the harness made outside the conversation: + // whether the rename survived is read from the saved workflow before cleanup. + const externalEdits = build.conversationMetrics?.externalEdits ?? []; + const externalEditVerdicts = + externalEdits.length > 0 + ? runExternalEditChecks({ client, edits: externalEdits, logger }).catch( + (error: unknown) => { + const reason = error instanceof Error ? error.message : String(error); + logger.warn(` External-edit checks failed: ${reason}`); + return allFailVerdicts( + externalEdits.map((edit) => externalRenameSurvivedExpectation(edit.to)), + `External-edit checks could not run: ${reason}`, + ); + }, + ) + : undefined; + const injected = + credentialInjected || externalEditVerdicts + ? Promise.all([credentialInjected, externalEditVerdicts]).then(([a, b]) => [ + ...(a ?? []), + ...(b ?? []), + ]) + : undefined; const { expectations, transcript, unjudged } = selectAuthorExpectations({ testCase, transcript: build.transcript, diff --git a/packages/@n8n/instance-ai/evaluations/types.ts b/packages/@n8n/instance-ai/evaluations/types.ts index f5de59c2dd3..a276be2b77e 100644 --- a/packages/@n8n/instance-ai/evaluations/types.ts +++ b/packages/@n8n/instance-ai/evaluations/types.ts @@ -100,15 +100,43 @@ export interface TurnCounter { confirmationAskedByKind: Record; replanAfterErrorCount: number; repeatQuestionCount: number; + /** Saves the instance rejected because the workflow changed outside the + * conversation (`build-workflow` answered `workflow_modified_externally`). + * Ground truth for stale-state expectations: a judge cannot tell "recovered" + * from "never happened" without it. */ + staleStateConflictCount: number; runFinishStatus?: string; } +/** + * A change the HARNESS made to a saved workflow from outside the conversation + * (a stage direction such as "renamed in another tab"). Recorded whether or not + * it landed, so a case that relies on the edit can see when it did not happen. + */ +export interface ExternalEditFact { + /** Agent turn the edit followed (1-based): applied after that turn settled, + * before the next user message. */ + turn: number; + kind: 'rename'; + workflowId?: string; + from?: string; + to: string; + applied: boolean; + /** Why the edit was skipped or failed, when `applied` is false. */ + reason?: string; +} + export interface ConversationMetrics { turnCount: number; perTurn: TurnCounter[]; confirmationAskedTotal: number; confirmationAskedByKind: Record; reachedRunFinishCleanly: boolean; + /** Sum of `perTurn[].staleStateConflictCount`. */ + staleStateConflictTotal: number; + /** Edits the harness applied outside the conversation, in order. Empty for + * the ordinary case. */ + externalEdits: ExternalEditFact[]; } // --------------------------------------------------------------------------- @@ -407,6 +435,8 @@ export interface TranscriptTurn { * each emit their own `run-start`) under the message that triggered them. * Ignored by the metric/outcome consumers (unknown type → default case). */ export const USER_TURN_EVENT = 'eval-user-turn'; +/** Harness-injected marker for an edit made outside the conversation; payload is an `ExternalEditFact` minus `turn`. */ +export const EXTERNAL_EDIT_EVENT = 'eval-external-edit'; export type TranscriptStep = ToolInteraction | { kind: 'agent-text'; text: string }; diff --git a/packages/@n8n/instance-ai/src/tools/__tests__/workflows.get-as-code.integration.test.ts b/packages/@n8n/instance-ai/src/tools/__tests__/workflows.get-as-code.integration.test.ts index 4b4161b9213..4d10392e9c8 100644 --- a/packages/@n8n/instance-ai/src/tools/__tests__/workflows.get-as-code.integration.test.ts +++ b/packages/@n8n/instance-ai/src/tools/__tests__/workflows.get-as-code.integration.test.ts @@ -102,4 +102,53 @@ describe('workflows get-as-code integration', () => { workflowChecksum: 'checksum-current', }); }); + + it('re-emits the verification output the last build declared for unchanged nodes', async () => { + const files = new Map(); + const context = makeContext(makeManagedWorkflow(), files); + const filePath = 'src/workflows/managed.workflow.ts'; + await saveWorkflowSourceFileBinding(context, { + filePath, + workflowId: 'wf-managed', + workflowVersionId: 'v-stale', + workflowChecksum: 'checksum-stale', + declaredOutputFixtures: { + Slack: { nodeType: 'n8n-nodes-base.slack', items: [{ ok: true, ts: '1.2' }] }, + // A node that no longer exists in the saved workflow is not re-emitted. + Gone: { nodeType: 'n8n-nodes-base.set', items: [{ x: 1 }] }, + }, + }); + const tool = createWorkflowsTool(context); + + const result = await executeTool(tool, { + action: 'get-as-code', + workflowId: 'wf-managed', + }); + + expect(result.error).toBeUndefined(); + expect(result.code).toContain("output: [{ ok: true, ts: '1.2' }]"); + expect(result.code).not.toContain('x: 1'); + expect(files.get(filePath)).toBe(result.code); + }); + + it('emits no output when the node changed type since the fixture was declared', async () => { + const files = new Map(); + const context = makeContext(makeManagedWorkflow(), files); + await saveWorkflowSourceFileBinding(context, { + filePath: 'src/workflows/managed.workflow.ts', + workflowId: 'wf-managed', + declaredOutputFixtures: { + Slack: { nodeType: 'n8n-nodes-base.httpRequest', items: [{ ok: true }] }, + }, + }); + const tool = createWorkflowsTool(context); + + const result = await executeTool(tool, { + action: 'get-as-code', + workflowId: 'wf-managed', + }); + + expect(result.error).toBeUndefined(); + expect(result.code).not.toContain('output:'); + }); }); diff --git a/packages/@n8n/instance-ai/src/tools/workflows.tool.ts b/packages/@n8n/instance-ai/src/tools/workflows.tool.ts index 7adcdc45785..de8dd4bcdbc 100644 --- a/packages/@n8n/instance-ai/src/tools/workflows.tool.ts +++ b/packages/@n8n/instance-ai/src/tools/workflows.tool.ts @@ -61,7 +61,9 @@ import { summarizeWorkflowStructure, } from './workflows/summarize-workflow'; import { validateWorkflowConfig } from './workflows/validate-workflow.service'; +import { nodeOutputsForRegeneration } from './workflows/workflow-declared-outputs'; import { + findWorkflowSourceFileBindingsForWorkflow, refreshWorkflowSourceFileBindingFromSave, refreshWorkflowSourceFileBindingFromWorkflow, } from './workflows/workflow-file-bindings'; @@ -799,7 +801,7 @@ async function handleGetAsCode( input: Extract, ) { const { generateWorkflowCode, buildImports } = await import('@n8n/workflow-sdk'); - const toCode = (json: WorkflowJSON): string => { + const toCode = (json: WorkflowJSON, nodeOutputs?: Record): string => { // Emit node ids: this code is edited and built back into the same saved workflow, // and carrying the ids through is what keeps node identity stable. Positions stay // out: build-workflow restores the saved layout by id, so a position in the file @@ -808,6 +810,7 @@ async function handleGetAsCode( workflow: json, includeNodeIds: true, includePositions: false, + ...(nodeOutputs ? { nodeOutputs } : {}), }); // The file must build as-is, so it carries the import line codegen omits. const importLine = buildImports(body); @@ -832,7 +835,18 @@ async function handleGetAsCode( } const { json, saved } = await readConsistentWorkflowSnapshot(context, input.workflowId); - const code = toCode(json); + // The saved workflow carries no verification `output` fixtures; the last build + // kept them on the binding. Re-emit them so a regenerated file verifies the + // same way the built one did instead of attempting live calls. + const priorBindings = await findWorkflowSourceFileBindingsForWorkflow( + context, + input.workflowId, + ); + const nodeOutputs = nodeOutputsForRegeneration( + priorBindings.map((binding) => binding.declaredOutputFixtures), + json.nodes, + ); + const code = toCode(json, nodeOutputs); const nodeCount = json.nodes?.length ?? 0; const base = { workflowId: input.workflowId, name: json.name, nodeCount }; diff --git a/packages/@n8n/instance-ai/src/tools/workflows/__tests__/build-workflow.tool.test.ts b/packages/@n8n/instance-ai/src/tools/workflows/__tests__/build-workflow.tool.test.ts index e27fd34a574..5e1b28e3789 100644 --- a/packages/@n8n/instance-ai/src/tools/workflows/__tests__/build-workflow.tool.test.ts +++ b/packages/@n8n/instance-ai/src/tools/workflows/__tests__/build-workflow.tool.test.ts @@ -2601,6 +2601,12 @@ describe('createBuildWorkflowTool', () => { }); expect(outcome?.simulationFixtures).toEqual({ 'Get Berlin Weather': rainyOutput }); expect(outcome?.verificationPinData).toBeUndefined(); + // Kept on the binding so get-as-code can re-emit it; the saved workflow has no copy. + await expect(getWorkflowSourceFileBinding(context, filePath)).resolves.toMatchObject({ + declaredOutputFixtures: { + 'Get Berlin Weather': { nodeType: 'n8n-nodes-base.httpRequest', items: rainyOutput }, + }, + }); }); it('warns when a chat-model node uses a provider without a stored credential while another LLM credential exists', async () => { diff --git a/packages/@n8n/instance-ai/src/tools/workflows/__tests__/workflow-declared-outputs.test.ts b/packages/@n8n/instance-ai/src/tools/workflows/__tests__/workflow-declared-outputs.test.ts new file mode 100644 index 00000000000..c057f81bd2c --- /dev/null +++ b/packages/@n8n/instance-ai/src/tools/workflows/__tests__/workflow-declared-outputs.test.ts @@ -0,0 +1,69 @@ +import type { WorkflowJSON } from '@n8n/workflow-sdk'; + +import { + declaredOutputFixturesForBinding, + MAX_DECLARED_OUTPUT_FIXTURES_CHARS, + nodeOutputsForRegeneration, +} from '../workflow-declared-outputs'; + +const nodes: WorkflowJSON['nodes'] = [ + { + id: 'http-1', + name: 'Fetch', + type: 'n8n-nodes-base.httpRequest', + typeVersion: 4.5, + position: [0, 0], + parameters: {}, + }, + { + id: 'slack-1', + name: 'Post', + type: 'n8n-nodes-base.slack', + typeVersion: 2.7, + position: [200, 0], + parameters: {}, + }, +]; + +describe('declaredOutputFixturesForBinding', () => { + it('keeps fixtures for known nodes with their type and drops the rest', () => { + expect( + declaredOutputFixturesForBinding( + { Fetch: [{ summary: 'ok' }], Ghost: [{ x: 1 }], Post: [] }, + nodes, + ), + ).toEqual({ Fetch: { nodeType: 'n8n-nodes-base.httpRequest', items: [{ summary: 'ok' }] } }); + }); + + it('returns undefined when nothing was declared or nothing survives', () => { + expect(declaredOutputFixturesForBinding(undefined, nodes)).toBeUndefined(); + expect(declaredOutputFixturesForBinding({ Ghost: [{ x: 1 }] }, nodes)).toBeUndefined(); + }); + + it('drops a fixture set too large for thread metadata', () => { + const big = { Fetch: [{ blob: 'x'.repeat(MAX_DECLARED_OUTPUT_FIXTURES_CHARS) }] }; + expect(declaredOutputFixturesForBinding(big, nodes)).toBeUndefined(); + }); +}); + +describe('nodeOutputsForRegeneration', () => { + it('re-emits only for nodes that still exist under the same name and type', () => { + const fixtures = { + Fetch: { nodeType: 'n8n-nodes-base.httpRequest', items: [{ summary: 'ok' }] }, + Post: { nodeType: 'n8n-nodes-base.httpRequest', items: [{ stale: true }] }, + Gone: { nodeType: 'n8n-nodes-base.set', items: [{ x: 1 }] }, + }; + expect(nodeOutputsForRegeneration([fixtures], nodes)).toEqual({ + Fetch: [{ summary: 'ok' }], + }); + }); + + it('lets a later binding override an earlier one and returns undefined when empty', () => { + const older = { Fetch: { nodeType: 'n8n-nodes-base.httpRequest', items: [{ v: 1 }] } }; + const newer = { Fetch: { nodeType: 'n8n-nodes-base.httpRequest', items: [{ v: 2 }] } }; + expect(nodeOutputsForRegeneration([older, undefined, newer], nodes)).toEqual({ + Fetch: [{ v: 2 }], + }); + expect(nodeOutputsForRegeneration([undefined], nodes)).toBeUndefined(); + }); +}); diff --git a/packages/@n8n/instance-ai/src/tools/workflows/build-workflow.tool.ts b/packages/@n8n/instance-ai/src/tools/workflows/build-workflow.tool.ts index 7ca0de89d8a..ffa7f860caf 100644 --- a/packages/@n8n/instance-ai/src/tools/workflows/build-workflow.tool.ts +++ b/packages/@n8n/instance-ai/src/tools/workflows/build-workflow.tool.ts @@ -58,6 +58,7 @@ import { trackWorkflowSourceBuild, type BuildTelemetryStage, } from './workflow-build-telemetry'; +import { declaredOutputFixturesForBinding } from './workflow-declared-outputs'; import { bindSourceFileToExistingWorkflow, getWorkflowSourceFileBinding, @@ -1497,12 +1498,21 @@ export function createBuildWorkflowTool(context: InstanceAiContext) { const runId = buildContext?.runId ?? context.runId; const workflowName = json.name || 'workflow'; const summary = `${operation === 'update' ? 'Updated' : 'Created'} ${isSupportingWorkflow ? 'supporting ' : ''}workflow "${workflowName}" (${saved.id}).`; + // Declared `output` fixtures live in source only, so keep them on the + // binding: a later get-as-code regenerates the source from the saved + // workflow and would otherwise silently drop them. + const { declaredOutputFixtures: _previousFixtures, ...bindingWithoutFixtures } = binding; + const keptFixtures = declaredOutputFixturesForBinding( + compiled.declaredOutputFixtures, + json.nodes, + ); binding = await saveWorkflowSourceFileBinding(context, { - ...binding, + ...bindingWithoutFixtures, workflowId: saved.id, workflowVersionId: saved.versionId, ...(saved.checksum ? { workflowChecksum: saved.checksum } : {}), sourceHash, + ...(keptFixtures ? { declaredOutputFixtures: keptFixtures } : {}), }); // Trace-only compiled-JSON event for eval seed reconstruction — never part // of the tool result, so it never enters the agent's context. diff --git a/packages/@n8n/instance-ai/src/tools/workflows/workflow-declared-outputs.ts b/packages/@n8n/instance-ai/src/tools/workflows/workflow-declared-outputs.ts new file mode 100644 index 00000000000..65dd4ab2528 --- /dev/null +++ b/packages/@n8n/instance-ai/src/tools/workflows/workflow-declared-outputs.ts @@ -0,0 +1,68 @@ +import type { WorkflowJSON } from '@n8n/workflow-sdk'; + +/** + * Verification fixtures a workflow's source declared as node `output`, keyed by + * node name. The saved workflow does not store them (they are source-only, see + * `declaredOutputFixtures` on the compile result), so once a build lands they + * survive only here, on the file binding. `nodeType` guards re-emission: a + * fixture describes one node's shape, and a node that changed type outside the + * conversation should not inherit it. + */ +export type DeclaredOutputFixtures = Record< + string, + { nodeType: string; items: Array> } +>; + +/** Thread metadata is read on every turn, so a fixture set that would bloat it is dropped instead. */ +export const MAX_DECLARED_OUTPUT_FIXTURES_CHARS = 32_000; + +type Nodes = WorkflowJSON['nodes']; + +function nodeTypesByName(nodes: Nodes | undefined): Map { + const byName = new Map(); + for (const node of nodes ?? []) { + if (node.name && node.type) byName.set(node.name, node.type); + } + return byName; +} + +/** + * The fixtures to keep on the binding after a successful build, or undefined + * when there are none worth keeping (no declared output, or too large). + */ +export function declaredOutputFixturesForBinding( + declared: NonNullable | undefined, + nodes: Nodes | undefined, +): DeclaredOutputFixtures | undefined { + if (!declared) return undefined; + const types = nodeTypesByName(nodes); + const kept: DeclaredOutputFixtures = {}; + for (const [nodeName, items] of Object.entries(declared)) { + const nodeType = types.get(nodeName); + if (!nodeType || !Array.isArray(items) || items.length === 0) continue; + kept[nodeName] = { nodeType, items }; + } + if (Object.keys(kept).length === 0) return undefined; + if (JSON.stringify(kept).length > MAX_DECLARED_OUTPUT_FIXTURES_CHARS) return undefined; + return kept; +} + +/** + * Fixtures to re-emit when source is regenerated from the saved workflow: only + * for nodes that still exist under the same name and type. Later entries win + * when several bindings carry fixtures for the same node. + */ +export function nodeOutputsForRegeneration( + fixtureSets: Array, + nodes: Nodes | undefined, +): Record | undefined { + const types = nodeTypesByName(nodes); + const outputs: Record = {}; + for (const fixtures of fixtureSets) { + for (const [nodeName, fixture] of Object.entries(fixtures ?? {})) { + if (types.get(nodeName) !== fixture.nodeType) continue; + outputs[nodeName] = fixture.items; + } + } + return Object.keys(outputs).length > 0 ? outputs : undefined; +} diff --git a/packages/@n8n/instance-ai/src/tools/workflows/workflow-file-bindings.ts b/packages/@n8n/instance-ai/src/tools/workflows/workflow-file-bindings.ts index 61e297e1971..e037ff81610 100644 --- a/packages/@n8n/instance-ai/src/tools/workflows/workflow-file-bindings.ts +++ b/packages/@n8n/instance-ai/src/tools/workflows/workflow-file-bindings.ts @@ -8,12 +8,22 @@ import { normalizeWorkspaceRelativePath } from '../../workspace/workspace-paths' const METADATA_KEY = 'instanceAiWorkflowSourceFiles'; +const declaredOutputFixturesSchema = z.record( + z.string(), + z.object({ + nodeType: z.string(), + items: z.array(z.record(z.string(), z.unknown())), + }), +); + const workflowSourceFileBindingSchema = z.object({ filePath: z.string(), workflowId: z.string().optional(), workflowVersionId: z.string().optional(), workflowChecksum: z.string().optional(), sourceHash: z.string().optional(), + /** Node `output` fixtures the last built source declared; see workflow-declared-outputs.ts. */ + declaredOutputFixtures: declaredOutputFixturesSchema.optional(), }); const workflowSourceFileBindingsSchema = z.record(z.string(), workflowSourceFileBindingSchema); diff --git a/packages/@n8n/workflow-sdk/src/codegen/code-generator.ts b/packages/@n8n/workflow-sdk/src/codegen/code-generator.ts index dbba50439ac..c0fc467b45c 100644 --- a/packages/@n8n/workflow-sdk/src/codegen/code-generator.ts +++ b/packages/@n8n/workflow-sdk/src/codegen/code-generator.ts @@ -70,6 +70,13 @@ export interface GenerateCodeOptions extends ExecutionContextOptions { includeNodeIds?: boolean; /** Emit saved node positions. On by default; see `GenerateWorkflowCodeOptions`. */ includePositions?: boolean; + /** + * Verification fixtures to emit verbatim as each node's `output`, keyed by node + * name. Takes precedence over the sample derived from `executionSchema`. The + * saved workflow does not carry these (they live in source only), so a caller + * that regenerates source for editing passes the ones it kept. + */ + nodeOutputs?: Map; } /** @@ -90,6 +97,8 @@ interface GenerationContext { workflowStatusJSDoc?: string; valuesExcluded?: boolean; pinnedNodes?: Set; + /** Declared verification outputs to re-emit, keyed by node name. */ + nodeOutputs?: Map; /** Graph node names allowed to emit their id; absent when id emission is off */ nodesWithEmittableId?: ReadonlySet; /** Leave `position` out of every node, subnode, and sticky note. */ @@ -549,9 +558,14 @@ function generateNodeConfig(node: SemanticNode, ctx: GenerationContext): string parts.push(`${innerIndent}config: {}`); } - // Add output from execution schema if available (for data flow awareness) const nodeName = node.json.name; - if (nodeName && ctx.nodeSchemas?.has(nodeName)) { + // Declared verification output wins: it is what the source said, not a sample. + const declaredOutput = nodeName ? ctx.nodeOutputs?.get(nodeName) : undefined; + if (declaredOutput && declaredOutput.length > 0) { + const items = declaredOutput.map((item) => formatValue(item, ctx)).join(', '); + parts.push(`${innerIndent}output: [${items}]`); + } else if (nodeName && ctx.nodeSchemas?.has(nodeName)) { + // Add output from execution schema if available (for data flow awareness) const schema = ctx.nodeSchemas.get(nodeName)!; const outputSample = schemaToOutputSample(schema, ctx.valuesExcluded); if (outputSample && Object.keys(outputSample).length > 0) { @@ -1419,6 +1433,7 @@ export function generateCode( workflowStatusJSDoc: executionContext?.workflowStatusJSDoc, valuesExcluded: executionContext?.valuesExcluded, pinnedNodes: executionContext?.pinnedNodes, + nodeOutputs: executionContext?.nodeOutputs, nodesWithEmittableId: executionContext?.includeNodeIds ? collectNodesWithEmittableId(graph) : undefined, diff --git a/packages/@n8n/workflow-sdk/src/codegen/index.ts b/packages/@n8n/workflow-sdk/src/codegen/index.ts index 2f1b94ed049..ed184566f8c 100644 --- a/packages/@n8n/workflow-sdk/src/codegen/index.ts +++ b/packages/@n8n/workflow-sdk/src/codegen/index.ts @@ -60,6 +60,12 @@ export interface GenerateWorkflowCodeOptions { * in the file invites a layout edit. */ includePositions?: boolean; + /** + * Verification fixtures to emit as each node's `output`, keyed by node name and + * copied verbatim. Saved workflows do not store them, so a caller that regenerates + * source for editing passes the ones it kept from the last build. + */ + nodeOutputs?: Record; } // Re-export individual functions for testing and extension @@ -122,6 +128,7 @@ export function generateWorkflowCode(input: WorkflowJSON | GenerateWorkflowCodeO pinnedNodes, includeNodeIds, includePositions, + nodeOutputs, } = isOptionsObject(input) ? input : { @@ -133,6 +140,7 @@ export function generateWorkflowCode(input: WorkflowJSON | GenerateWorkflowCodeO pinnedNodes: undefined, includeNodeIds: undefined, includePositions: undefined, + nodeOutputs: undefined, }; // Phase 1: Build semantic graph @@ -169,5 +177,6 @@ export function generateWorkflowCode(input: WorkflowJSON | GenerateWorkflowCodeO pinnedNodes: pinnedNodes ? new Set(pinnedNodes) : undefined, includeNodeIds, includePositions, + nodeOutputs: nodeOutputs ? new Map(Object.entries(nodeOutputs)) : undefined, }); } diff --git a/packages/@n8n/workflow-sdk/src/codegen/node-outputs.test.ts b/packages/@n8n/workflow-sdk/src/codegen/node-outputs.test.ts new file mode 100644 index 00000000000..840a9a65e7d --- /dev/null +++ b/packages/@n8n/workflow-sdk/src/codegen/node-outputs.test.ts @@ -0,0 +1,64 @@ +import { generateWorkflowCode } from './index'; +import type { WorkflowJSON } from '../types/base'; + +function workflow(): WorkflowJSON { + return { + name: 'Status summary', + nodes: [ + { + id: 'trigger-1', + name: 'Every Morning', + type: 'n8n-nodes-base.scheduleTrigger', + typeVersion: 1.2, + position: [0, 0], + parameters: { rule: { interval: [{ field: 'days', triggerAtHour: 8 }] } }, + }, + { + id: 'http-1', + name: 'Fetch Status Summary', + type: 'n8n-nodes-base.httpRequest', + typeVersion: 4.5, + position: [200, 0], + parameters: { method: 'GET', url: 'https://status.example.com/api/summary' }, + }, + ], + connections: { + 'Every Morning': { main: [[{ node: 'Fetch Status Summary', type: 'main', index: 0 }]] }, + }, + }; +} + +describe('generateWorkflowCode nodeOutputs', () => { + it('emits declared verification output verbatim for the named node only', () => { + const code = generateWorkflowCode({ + workflow: workflow(), + nodeOutputs: { + 'Fetch Status Summary': [{ summary: 'All systems operational.' }], + }, + }); + + expect(code).toContain("output: [{ summary: 'All systems operational.' }]"); + // Only one node declared output; the trigger gets none. + expect(code.match(/output:/g)).toHaveLength(1); + }); + + it('emits several items and ignores names that are not in the workflow', () => { + const code = generateWorkflowCode({ + workflow: workflow(), + nodeOutputs: { + 'Fetch Status Summary': [{ a: 1 }, { a: 2 }], + 'Not A Node': [{ b: 1 }], + }, + }); + + expect(code).toContain('output: [{ a: 1 }, { a: 2 }]'); + expect(code).not.toContain('b: 1'); + }); + + it('emits nothing for an empty item list and without the option', () => { + expect( + generateWorkflowCode({ workflow: workflow(), nodeOutputs: { 'Fetch Status Summary': [] } }), + ).not.toContain('output:'); + expect(generateWorkflowCode(workflow())).not.toContain('output:'); + }); +});