mirror of
https://github.com/n8n-io/n8n.git
synced 2026-08-28 09:12:12 +08:00
feat(core): Let workflow verification target a specific trigger (#37091)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
||||
|
||||
+33
@@ -599,6 +599,7 @@ async function runTool(
|
||||
includeData?: boolean;
|
||||
maxDataChars?: number;
|
||||
fixtureOverrides?: Record<string, Array<Record<string, unknown>>>;
|
||||
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 });
|
||||
});
|
||||
});
|
||||
|
||||
+103
@@ -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');
|
||||
});
|
||||
});
|
||||
|
||||
+13
@@ -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()
|
||||
|
||||
@@ -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<string>;
|
||||
/** 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,
|
||||
|
||||
@@ -30,6 +30,7 @@ export interface ScriptedGateRunArgs {
|
||||
executionService: VerificationExecutionService;
|
||||
workflowId: string;
|
||||
inputData?: Record<string, unknown>;
|
||||
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,
|
||||
|
||||
@@ -5,6 +5,8 @@ export interface VerifyToolInput {
|
||||
workItemId?: string;
|
||||
workflowId: string;
|
||||
inputData?: Record<string, unknown>;
|
||||
/** Trigger to start from; omitted means the adapter auto-detects one. */
|
||||
triggerNodeName?: string;
|
||||
timeout?: number;
|
||||
includeData?: boolean;
|
||||
maxDataChars?: number;
|
||||
|
||||
@@ -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.<field>, 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,
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user