From 6420048d7adc34d9126b32f4d1effdbec81da709 Mon Sep 17 00:00:00 2001 From: Mutasem Aldmour <4711238+mutdmour@users.noreply.github.com> Date: Thu, 27 Aug 2026 11:52:12 +0000 Subject: [PATCH] feat(core): Let workflow verification target a specific trigger (#37091) Co-authored-by: Claude Opus 5 (1M context) --- .../skills/post-build-flow/SKILL.md | 46 +++++--- .../verify-built-workflow.tool.test.ts | 33 ++++++ .../__tests__/analyze-result.test.ts | 103 ++++++++++++++++++ .../__tests__/scripted-gate-run.test.ts | 13 +++ .../verification/analyze-result.ts | 29 ++++- .../verification/scripted-gate-run.ts | 3 + .../tools/orchestration/verification/types.ts | 2 + .../verify-built-workflow.tool.ts | 15 +++ .../instance-ai.adapter.service.test.ts | 12 ++ .../instance-ai.adapter.service.ts | 9 +- 10 files changed, 246 insertions(+), 19 deletions(-) diff --git a/packages/@n8n/instance-ai/skills/post-build-flow/SKILL.md b/packages/@n8n/instance-ai/skills/post-build-flow/SKILL.md index 504403d79c8..8f6025185d3 100644 --- a/packages/@n8n/instance-ai/skills/post-build-flow/SKILL.md +++ b/packages/@n8n/instance-ai/skills/post-build-flow/SKILL.md @@ -225,9 +225,10 @@ to force data through the trigger. **Reserve `executions(action="run")` for runs the user explicitly asked for** (e.g. "run it now", "execute it against my real data"). Never call it on your own to re-test, expand coverage, or "prove the full chain" of a workflow you just -built or verified: re-run `verify-built-workflow` (with `fixtureOverrides` to -reach an unverified branch) instead, or report the partial coverage and let the -user decide whether to run it. +built or verified: re-run `verify-built-workflow` instead — with +`triggerNodeName` to reach another trigger's branch, or `fixtureOverrides` to +reach another branch within one trigger's run — or report the partial coverage +and let the user decide whether to run it. If `fixtureOverrides` is rejected with `invalid_fixture_override`, the target node was not classified as simulated in the build outcome. Do not retry the same override. If that node's data controls a branch that needs verification and you @@ -235,18 +236,30 @@ have the source file, load `workflow-builder`, declare representative `output` fixtures on the controlling upstream node, rebuild the same workflow, and verify again. -**Never edit a saved workflow to reach a branch.** Disabling or deleting nodes to -steer a test mutates the user's workflow and leaves it broken for as long as the -test runs — if it is published, its triggers fire against the broken version. -For a workflow with more than one trigger (`triggerNodes` has multiple entries): +**Never edit or copy a saved workflow to reach a branch.** Disabling, deleting, +or reordering nodes to steer a test mutates the user's workflow and leaves it +broken for as long as the test runs — if it is published, its triggers fire +against the broken version. Building a throwaway second workflow is no better: +the evidence is gathered against a copy that can drift from the workflow the +user keeps, and the copy is left behind whenever the cleanup delete fails. +For a workflow with more than one trigger (`triggerNodes` has multiple entries), +**verify once per trigger**: + +- Pass `triggerNodeName` to `verify-built-workflow` and call it once for each + entry in `triggerNodes`. Naming no trigger verifies only the auto-detected + one. An unresolvable name is rejected outright, so a rejected call means the + name is wrong — re-read `triggerNodes`, never fall back to editing. +- Each pass covers its own trigger's branch, so its `nodesNotReached` will list + the other triggers' nodes. That is expected, not a defect: coverage is the + **union** across passes. Only treat a node as unverified once no pass reached + it. +- Report per-trigger coverage — name each trigger and whether its branch ran. + Claim the workflow is verified only when every trigger's branch has a + successful pass. - When the user asked for a live run, pass `triggerNodeName` to - `executions(action="run")` — one run per trigger — and report each branch's - result. Naming no trigger runs only the auto-detected one. -- `verify-built-workflow` always exercises the auto-detected trigger, so it - covers one branch. Say which trigger was verified and which branches were not, - and offer the user a live run for the rest. Do not force coverage by editing - the workflow. + `executions(action="run")` the same way — one run per trigger — and report + each branch's result. ## After build-workflow succeeds @@ -268,7 +281,8 @@ For a workflow with more than one trigger (`triggerNodes` has multiple entries): - If `verificationReadiness.status === "ready"`, call `verify-built-workflow` with the `workflowId`, the `workItemId` when you - have it, and the trigger-appropriate `inputData` shape. + have it, and the trigger-appropriate `inputData` shape. When `triggerNodes` + has more than one entry, call it once per trigger with `triggerNodeName`. - If `verificationReadiness.status === "needs_setup"`, call `workflows(action="setup")` with the workflowId so the user can configure it through the inline setup card in the AI Assistant panel. @@ -291,7 +305,9 @@ For a workflow with more than one trigger (`triggerNodes` has multiple entries): were verified and which were not, and tell the user the unreached part needs a manual test. Do not start a live `executions(action="run")` yourself to reach those nodes; offer the user a test instead. Never claim - end-to-end verification when `nodesNotReached` is non-empty. + end-to-end verification when `nodesNotReached` is non-empty — except for + nodes another trigger's pass already reached, since per-trigger coverage + is the union across passes. - If the unreached nodes sit behind IF/Switch logic controlled by a live or nondeterministic upstream node, and alternate-branch verification is part of this turn's goal, first try one source-file repair: add representative diff --git a/packages/@n8n/instance-ai/src/tools/orchestration/__tests__/verify-built-workflow.tool.test.ts b/packages/@n8n/instance-ai/src/tools/orchestration/__tests__/verify-built-workflow.tool.test.ts index f1d98a43e16..78088f5a80a 100644 --- a/packages/@n8n/instance-ai/src/tools/orchestration/__tests__/verify-built-workflow.tool.test.ts +++ b/packages/@n8n/instance-ai/src/tools/orchestration/__tests__/verify-built-workflow.tool.test.ts @@ -599,6 +599,7 @@ async function runTool( includeData?: boolean; maxDataChars?: number; fixtureOverrides?: Record>>; + triggerNodeName?: string; }, ) { const tool = createVerifyBuiltWorkflowTool(ctx as unknown as OrchestrationContext); @@ -1496,3 +1497,35 @@ describe('verify-built-workflow tool — stale mocked-credential plan', () => { ); }); }); + +describe('verify-built-workflow tool — trigger selection', () => { + it('starts verification from the named trigger', async () => { + const { ctx } = makeContext(makeBuildOutcome(), { + executionId: 'exec-monthly', + status: 'success', + data: { 'Post Summary': [{ ok: true }] }, + }); + + await runTool(ctx, { + workItemId: 'wi-1', + workflowId: 'wf-1', + triggerNodeName: 'First of Month', + }); + + const run = vi.mocked(ctx.domainContext.executionService.run); + expect(run.mock.calls[0][2]).toMatchObject({ triggerNodeName: 'First of Month' }); + }); + + it('leaves the trigger auto-detected when no trigger is named', async () => { + const { ctx } = makeContext(makeBuildOutcome(), { + executionId: 'exec-auto', + status: 'success', + data: { 'Post Movers': [{ ok: true }] }, + }); + + await runTool(ctx, { workItemId: 'wi-1', workflowId: 'wf-1' }); + + const run = vi.mocked(ctx.domainContext.executionService.run); + expect(run.mock.calls[0][2]).toMatchObject({ triggerNodeName: undefined }); + }); +}); diff --git a/packages/@n8n/instance-ai/src/tools/orchestration/verification/__tests__/analyze-result.test.ts b/packages/@n8n/instance-ai/src/tools/orchestration/verification/__tests__/analyze-result.test.ts index b8091335635..a130d2ad230 100644 --- a/packages/@n8n/instance-ai/src/tools/orchestration/verification/__tests__/analyze-result.test.ts +++ b/packages/@n8n/instance-ai/src/tools/orchestration/verification/__tests__/analyze-result.test.ts @@ -321,3 +321,106 @@ describe('analyzeVerificationResult — workflow-pinned nodes', () => { ]); }); }); + +describe('analyzeVerificationResult — trigger-scoped coverage', () => { + const twoBranchOutcome = makeBuildOutcome({ + nodeSimulationPlan: [ + { + nodeName: 'Post Movers', + verdict: 'simulate', + reason: 'Sends a message', + confidence: 'high', + source: 'deterministic', + }, + { + nodeName: 'Post Summary', + verdict: 'simulate', + reason: 'Sends a message', + confidence: 'high', + source: 'deterministic', + }, + ], + }); + const weekdayPass = { + executionId: 'exec-weekday', + status: 'success', + executedNodeNames: ['Every Weekday 9am', 'Build Movers Message', 'Post Movers'], + lastNodeExecuted: 'Post Movers', + data: { 'Post Movers': [{ ok: true }] }, + } as unknown as ExecutionRunResult; + + it('attributes the unreached nodes to the other triggers when a trigger was named', () => { + const analysis = analyzeVerificationResult({ + result: weekdayPass, + buildOutcome: twoBranchOutcome, + simulatedNodes: [{ nodeName: 'Post Movers', reason: 'Sends a message' }], + stateBefore: undefined, + runId: 'run-1', + triggerNodeName: 'Every Weekday 9am', + }); + + expect(analysis.nodesNotReached).toEqual(['Post Summary']); + expect(analysis.coverageNote).toContain('Every Weekday 9am'); + expect(analysis.coverageNote).toContain('triggerNodeName'); + // The generic "a lookup returned nothing" cause would send the agent + // editing a workflow whose other branch is simply not on this path. + expect(analysis.coverageNote).not.toContain('lookup or query returned nothing'); + }); + + it('keeps the wait-gate guidance when a named-trigger pass halts at a gate', () => { + const gateOutcome = makeBuildOutcome({ + nodeSimulationPlan: [ + { + nodeName: 'Email Approval', + verdict: 'simulate', + reason: 'Send-and-wait gate on a loop', + confidence: 'high', + source: 'deterministic', + haltBranch: true, + }, + { + nodeName: 'Publish', + verdict: 'simulate', + reason: 'Sends a message', + confidence: 'high', + source: 'deterministic', + }, + ], + }); + const gatedPass = { + executionId: 'exec-gated', + status: 'success', + executedNodeNames: ['1st of Month', 'Format Draft', 'Email Approval'], + lastNodeExecuted: 'Email Approval', + data: { 'Email Approval': [] }, + } as unknown as ExecutionRunResult; + + const analysis = analyzeVerificationResult({ + result: gatedPass, + buildOutcome: gateOutcome, + simulatedNodes: [{ nodeName: 'Email Approval', reason: 'Send-and-wait gate on a loop' }], + haltedGateNames: ['Email Approval'], + stateBefore: undefined, + runId: 'run-1', + triggerNodeName: '1st of Month', + }); + + // Nodes behind the gate can never be covered by verification, so the + // gate guidance must survive alongside the per-trigger scoping. + expect(analysis.coverageNote).toContain('pauses at wait gate'); + expect(analysis.coverageNote).toContain('live end-to-end test'); + expect(analysis.coverageNote).toContain('1st of Month'); + }); + + it('keeps the generic zero-output guidance when no trigger was named', () => { + const analysis = analyzeVerificationResult({ + result: weekdayPass, + buildOutcome: twoBranchOutcome, + simulatedNodes: [{ nodeName: 'Post Movers', reason: 'Sends a message' }], + stateBefore: undefined, + runId: 'run-1', + }); + + expect(analysis.coverageNote).toContain('Seed matching test data'); + }); +}); diff --git a/packages/@n8n/instance-ai/src/tools/orchestration/verification/__tests__/scripted-gate-run.test.ts b/packages/@n8n/instance-ai/src/tools/orchestration/verification/__tests__/scripted-gate-run.test.ts index 4b5215e30ab..071fded9f3c 100644 --- a/packages/@n8n/instance-ai/src/tools/orchestration/verification/__tests__/scripted-gate-run.test.ts +++ b/packages/@n8n/instance-ai/src/tools/orchestration/verification/__tests__/scripted-gate-run.test.ts @@ -109,6 +109,19 @@ describe('runScriptedGateVerification', () => { expect(result.executedNodeNames).toEqual(expect.arrayContaining(['Publish', 'Revise'])); }); + it('starts every decision pass from the named trigger', async () => { + const run = vi + .fn() + .mockResolvedValueOnce(approvePassResult) + .mockResolvedValueOnce(declinePassResult); + + await runScriptedGateVerification({ ...makeArgs(run), triggerNodeName: 'First of Month' }); + + for (const call of run.mock.calls) { + expect((call as unknown[])[2]).toMatchObject({ triggerNodeName: 'First of Month' }); + } + }); + it('fails the merged analysis when any pass fails', async () => { const run = vi .fn() diff --git a/packages/@n8n/instance-ai/src/tools/orchestration/verification/analyze-result.ts b/packages/@n8n/instance-ai/src/tools/orchestration/verification/analyze-result.ts index 2e1edc81964..9fcd411dc7c 100644 --- a/packages/@n8n/instance-ai/src/tools/orchestration/verification/analyze-result.ts +++ b/packages/@n8n/instance-ai/src/tools/orchestration/verification/analyze-result.ts @@ -335,8 +335,20 @@ function buildCoverageNote( result: ExecutionRunResult, success: boolean, reachedHaltedGates: string[], + triggerNodeName: string | undefined, ): string | undefined { if (nodesNotReached.length === 0) return undefined; + // A trigger-scoped pass only ever reaches its own trigger's branch, so nodes + // on the other branches are expected to be unreached. Appended to whichever + // note applies rather than returned on its own, so it never displaces the + // wait-gate guidance — nodes behind a gate stay uncoverable either way. + const triggerScopeNote = triggerNodeName + ? ` This pass started from trigger "${triggerNodeName}", so it covers that trigger's branch ` + + "only — nodes on another trigger's branch are expected to be unreached here. Call " + + 'verify-built-workflow again with `triggerNodeName` set to each remaining trigger and ' + + 'treat coverage as the union of those passes. Do not edit, disable, reorder, or copy the ' + + 'workflow to reach them.' + : ''; if (success && reachedHaltedGates.length > 0) { return ( `Verification pauses at wait gate(s) ${reachedHaltedGates.join(', ')} — in a live run the ` + @@ -346,7 +358,18 @@ function buildCoverageNote( 'not edit the workflow or re-run verification to force coverage there; recommend a live ' + 'end-to-end test instead. Any unreached node NOT behind the gate did not receive input ' + 'items (usually an empty lookup or query) — seed matching test data and re-run before ' + - 'treating it as verified.' + 'treating it as verified.' + + triggerScopeNote + ); + } + if (success && triggerNodeName) { + return ( + `Partial coverage by design: ${String(nodesNotReached.length)} planned node(s) were not ` + + `reached: ${nodesNotReached.join(', ')}.` + + triggerScopeNote + + " Any unreached node that IS on this trigger's branch did not receive input items " + + '(usually an empty lookup or query) — seed matching test data and re-run before treating ' + + 'it as verified.' ); } const ending = result.lastNodeExecuted @@ -415,6 +438,8 @@ export function analyzeVerificationResult(args: { chatModelRelatedNodeNames?: ReadonlySet; /** Precomputed replacement suggestions and n8n-credits availability for chat-model recovery guidance. */ chatModelRecovery?: ChatModelRecoveryOptions; + /** Trigger this pass started from, when the caller named one. */ + triggerNodeName?: string; }): VerificationAnalysis { const { result, @@ -425,6 +450,7 @@ export function analyzeVerificationResult(args: { runId, chatModelRelatedNodeNames, chatModelRecovery, + triggerNodeName, } = args; const nodeErrors = result.nodeErrors ?? []; const reachedNames = new Set( @@ -485,6 +511,7 @@ export function analyzeVerificationResult(args: { result, success, (haltedGateNames ?? []).filter((name) => reachedNames.has(name)), + triggerNodeName, ), errorMessage, nodeErrors, diff --git a/packages/@n8n/instance-ai/src/tools/orchestration/verification/scripted-gate-run.ts b/packages/@n8n/instance-ai/src/tools/orchestration/verification/scripted-gate-run.ts index 829f043379a..3b156ac9448 100644 --- a/packages/@n8n/instance-ai/src/tools/orchestration/verification/scripted-gate-run.ts +++ b/packages/@n8n/instance-ai/src/tools/orchestration/verification/scripted-gate-run.ts @@ -30,6 +30,7 @@ export interface ScriptedGateRunArgs { executionService: VerificationExecutionService; workflowId: string; inputData?: Record; + triggerNodeName?: string; timeout?: number; abortSignal?: AbortSignal; buildOutcome: WorkflowBuildOutcome; @@ -65,6 +66,7 @@ export async function runScriptedGateVerification( for (const decision of script.decisions) { const result = await executionService.run(workflowId, args.inputData, { timeout: args.timeout, + triggerNodeName: args.triggerNodeName, verificationPinData: { ...basePins, [script.nodeName]: decision.items }, omitConnections: [script.cutEdge], isVerificationRun: true, @@ -74,6 +76,7 @@ export async function runScriptedGateVerification( result, buildOutcome, simulatedNodes: prepared.simulatedNodes, + triggerNodeName: args.triggerNodeName, stateBefore, runId, chatModelRelatedNodeNames, diff --git a/packages/@n8n/instance-ai/src/tools/orchestration/verification/types.ts b/packages/@n8n/instance-ai/src/tools/orchestration/verification/types.ts index 052dfe4421a..9bb94b822b2 100644 --- a/packages/@n8n/instance-ai/src/tools/orchestration/verification/types.ts +++ b/packages/@n8n/instance-ai/src/tools/orchestration/verification/types.ts @@ -5,6 +5,8 @@ export interface VerifyToolInput { workItemId?: string; workflowId: string; inputData?: Record; + /** Trigger to start from; omitted means the adapter auto-detects one. */ + triggerNodeName?: string; timeout?: number; includeData?: boolean; maxDataChars?: number; diff --git a/packages/@n8n/instance-ai/src/tools/orchestration/verify-built-workflow.tool.ts b/packages/@n8n/instance-ai/src/tools/orchestration/verify-built-workflow.tool.ts index 1a8e9150605..a0c9b88aab8 100644 --- a/packages/@n8n/instance-ai/src/tools/orchestration/verify-built-workflow.tool.ts +++ b/packages/@n8n/instance-ai/src/tools/orchestration/verify-built-workflow.tool.ts @@ -44,6 +44,18 @@ export const verifyBuiltWorkflowInputSchema = z.object({ "If you wrap a form payload in {formFields: {...}} the adapter will reject the call; the builder's " + 'downstream expressions reference $json., matching the flat production shape.', ), + triggerNodeName: z + .string() + .min(1) + .optional() + .describe( + 'Name of the trigger node to start verification from. REQUIRED when the workflow has ' + + 'more than one trigger: without it a single trigger is auto-detected and the other ' + + "triggers' branches are never verified. To cover every branch, call verify once per " + + "trigger. Trigger names come from build-workflow's `triggerNodes` or " + + 'workflows(action="get-as-code"). Never disable, delete, reorder, or re-save a workflow — ' + + 'and never build a throwaway copy — to reach a branch; use this instead.', + ), timeout: z .number() .int() @@ -181,6 +193,7 @@ export function createVerifyBuiltWorkflowTool(context: OrchestrationContext) { executionService: target.domainContext.executionService, workflowId, inputData: resolvedInput.inputData, + triggerNodeName: resolvedInput.triggerNodeName, timeout: resolvedInput.timeout, abortSignal: context.abortSignal, buildOutcome, @@ -195,6 +208,7 @@ export function createVerifyBuiltWorkflowTool(context: OrchestrationContext) { resolvedInput.inputData, { timeout: resolvedInput.timeout, + triggerNodeName: resolvedInput.triggerNodeName, verificationPinData: prepared.verificationPinData, isVerificationRun: true, abortSignal: context.abortSignal, @@ -207,6 +221,7 @@ export function createVerifyBuiltWorkflowTool(context: OrchestrationContext) { buildOutcome, simulatedNodes: prepared.simulatedNodes, haltedGateNames: prepared.haltedGateNames, + triggerNodeName: resolvedInput.triggerNodeName, stateBefore: target.stateBefore, runId: context.runId, chatModelRelatedNodeNames, diff --git a/packages/cli/src/modules/instance-ai/__tests__/instance-ai.adapter.service.test.ts b/packages/cli/src/modules/instance-ai/__tests__/instance-ai.adapter.service.test.ts index 2bc63376814..94173fe3482 100644 --- a/packages/cli/src/modules/instance-ai/__tests__/instance-ai.adapter.service.test.ts +++ b/packages/cli/src/modules/instance-ai/__tests__/instance-ai.adapter.service.test.ts @@ -3928,6 +3928,18 @@ describe('createExecutionAdapter run()', () => { expect(mockWorkflowRunner.run).not.toHaveBeenCalled(); }); + it('rejects an empty trigger name instead of silently auto-detecting', async () => { + const { adapter, mockWorkflowRunner } = createRunAdapterForTests({ + id: 'wf-1', + nodes: [triggerNode('Daily 8am'), triggerNode('Weekly 5pm')], + }); + + await expect(adapter.run('wf-1', undefined, { triggerNodeName: '' })).rejects.toThrow( + /Daily 8am.*Weekly 5pm/s, + ); + expect(mockWorkflowRunner.run).not.toHaveBeenCalled(); + }); + it('rejects a named node that is not a trigger', async () => { const { adapter, mockWorkflowRunner } = createRunAdapterForTests({ id: 'wf-1', diff --git a/packages/cli/src/modules/instance-ai/instance-ai.adapter.service.ts b/packages/cli/src/modules/instance-ai/instance-ai.adapter.service.ts index 63093e10ea9..9483acb6538 100644 --- a/packages/cli/src/modules/instance-ai/instance-ai.adapter.service.ts +++ b/packages/cli/src/modules/instance-ai/instance-ai.adapter.service.ts @@ -1325,9 +1325,12 @@ export class InstanceAiAdapterService { // Use the explicitly requested trigger node when provided — the only way to // pick a branch in a multi-trigger workflow — otherwise auto-detect. - const triggerNode = options?.triggerNodeName - ? resolveRequestedTriggerNode(nodes, options.triggerNodeName) - : findTriggerNode(nodes); + // Checked against undefined, not truthiness: an empty name is a caller + // mistake, and auto-detecting there would run a branch nobody asked for. + const triggerNode = + options?.triggerNodeName !== undefined + ? resolveRequestedTriggerNode(nodes, options.triggerNodeName) + : findTriggerNode(nodes); const timeoutMs = Math.min(options?.timeout ?? DEFAULT_TIMEOUT_MS, MAX_TIMEOUT_MS);