fix(core): Keep pending setup for nodes that n8n Assistant added in an earlier build (#41070)

packages/@n8n/instance-ai/src/tools/__tests__/workflows.tool.test.ts

@@ -3473,6 +3473,78 @@ describe('workflows tool', () => {
 				expect(suspend.mock.calls[0][0]).toMatchObject({ setupRequests: [sheetsRequest] });
 			});
 
+			describe('nodes the build left out of setup', () => {
+				// The build changed only the Sheets node; Slack is pre-existing and unconfigured.
+				function createScopedContext() {
+					const context = createGrantAwareContext();
+					context.runId = 'run-1';
+					(context as { workflowBuildContext?: unknown }).workflowBuildContext = {
+						threadId: 't1',
+						runId: 'run-1',
+						taskId: 'task-1',
+						workItemId: 'wi-1',
+						workflowTaskService: {
+							getLatestBuildOutcomeForWorkflow: vi
+								.fn()
+								.mockResolvedValue({ runId: 'run-1', changedNodeNames: ['Log to Sheet'] }),
+						},
+					};
+					return context;
+				}
+
+				beforeEach(() => {
+					(analyzeWorkflow as Mock).mockResolvedValue([slackRequest, sheetsRequest]);
+					(applyNodeChanges as Mock).mockResolvedValue({ applied: [], failed: [] });
+					(buildCompletedReport as Mock).mockReturnValue([]);
+				});
+
+				it('does not record them as skipped when the user skips the card', async () => {
+					const context = createScopedContext();
+
+					const tool = createWorkflowsTool(context);
+					const result = await executeTool(tool, { action: 'setup', workflowId: 'wf1' }, {
+						resumeData: { approved: false },
+					} as never);
+
+					expect(context.grantSessionToolApproval).not.toHaveBeenCalledWith(
+						'workflows:setup-skip:cred:slackApi',
+					);
+					expect(result).toMatchObject({ skippedByUser: [{ nodeName: 'Log to Sheet' }] });
+				});
+
+				it('does not report them as still needing setup after apply', async () => {
+					const context = createScopedContext();
+
+					const tool = createWorkflowsTool(context);
+					const result = await executeTool(tool, { action: 'setup', workflowId: 'wf1' }, {
+						resumeData: { approved: true, action: 'apply' },
+					} as never);
+
+					expect(result).toMatchObject({
+						partial: true,
+						nodesStillNeedingSetup: [{ nodeName: 'Log to Sheet' }],
+					});
+				});
+
+				it('keeps them out of the panel a trigger test rebuilds', async () => {
+					const context = createScopedContext();
+					(context.executionService.run as Mock).mockResolvedValue({ status: 'success' });
+					const suspend = vi.fn();
+
+					const tool = createWorkflowsTool(context);
+					await executeTool(tool, { action: 'setup', workflowId: 'wf1' }, {
+						suspend,
+						resumeData: {
+							approved: true,
+							action: 'test-trigger',
+							testTriggerNode: 'Log to Sheet',
+						},
+					} as never);
+
+					expect(suspend.mock.calls[0][0]).toMatchObject({ setupRequests: [sheetsRequest] });
+				});
+			});
+
 			it('revalidates the credential test destination after a trigger test', async () => {
 				const fixture = templatedSetupFixture({ testUrl: 'https://status.example.net/me' });
 				(analyzeWorkflow as Mock).mockResolvedValue([fixture.request]);

packages/@n8n/instance-ai/src/tools/workflows.tool.ts

@@ -1189,11 +1189,11 @@ async function handleSetupTestTrigger(
 			appliedCredentialIds: appliedCredentialIdList(resumeData.credentials, preTestApply.applied),
 		},
 	);
-	// Re-derived from scratch, so it has to be partitioned again: this is the second path that
-	// builds the panel, and without it a trigger test mid-session puts back the cards state 1
-	// left out.
+	// Re-derived from scratch, so it has to be scoped and partitioned again: this is the second
+	// path that builds the panel, and without it a trigger test mid-session puts back the cards
+	// state 1 left out. The destination check below still sees every node.
 	const { pending: refreshedPending } = partitionSkippedSetupRequests(
-		refreshedRequests,
+		await filterToSetupScope(context, input, refreshedRequests),
 		input.workflowId,
 		getSkippedSetupSubjects(context),
 	);
@@ -1319,12 +1319,17 @@ async function handleSetupApply(
 		// Filter by needsAction to distinguish "render this card" from
 		// "this still requires user intervention". Settled requests are kept so
 		// a just-applied credential whose test failed stays reportable below —
-		// a bound credential is settled for routing even when its test fails.
-		const remainingRequests = await analyzeWorkflow(context, input.workflowId, undefined, {
-			includeSettled: true,
-			...preferNewCredentialOptions(input, resumeData.credentials),
-			appliedCredentialIds: appliedCredentialIdList(resumeData.credentials, applyResult.applied),
-		});
+		// a bound credential is settled for routing even when its test fails. Scoped like the
+		// card: a node it left out is neither still pending nor skipped by the user.
+		const remainingRequests = await filterToSetupScope(
+			context,
+			input,
+			await analyzeWorkflow(context, input.workflowId, undefined, {
+				includeSettled: true,
+				...preferNewCredentialOptions(input, resumeData.credentials),
+				appliedCredentialIds: appliedCredentialIdList(resumeData.credentials, applyResult.applied),
+			}),
+		);
 		const completedNodes = buildCompletedReport(
 			resumeData.credentials,
 			resumeData.nodeParameters,
@@ -1444,6 +1449,23 @@ async function resolveSetupScopeNodeNames(
 	}
 }
 
+/**
+ * Narrow requests to the build-bound setup scope. Every path that builds or reports the setup
+ * card goes through here, so a node the card left out cannot come back through a trigger test,
+ * the apply report, or a skip the user never made.
+ */
+async function filterToSetupScope(
+	context: InstanceAiContext,
+	input: Extract<Input, { action: 'setup' }>,
+	requests: SetupRequest[],
+): Promise<SetupRequest[]> {
+	if (input.includeAllNodes) return requests;
+	const scopeNodeNames = await resolveSetupScopeNodeNames(context, input.workflowId);
+	return scopeNodeNames
+		? requests.filter((request) => scopeNodeNames.includes(request.node.name))
+		: requests;
+}
+
 /**
  * Coverage disclosure for the workflow's latest verification, or undefined when
  * it was fully verified or no claim exists. Absent evidence never blocks: a
@@ -1689,12 +1711,7 @@ async function handleSetup(
 
 		// Setup after a build covers only the nodes that build changed —
 		// pre-existing, unrelated nodes must not surface in the setup card.
-		const scopeNodeNames = input.includeAllNodes
-			? undefined
-			: await resolveSetupScopeNodeNames(context, input.workflowId);
-		const scopedRequests = scopeNodeNames
-			? allSetupRequests.filter((request) => scopeNodeNames.includes(request.node.name))
-			: allSetupRequests;
+		const scopedRequests = await filterToSetupScope(context, input, allSetupRequests);
 
 		// Two reasons a card stays out, applied in order: this build never touched the node, or
 		// the user declined it. Partitioning the scoped list keeps them apart in the report —
@@ -1774,11 +1791,9 @@ async function handleSetup(
 			// the user declined, and pre-existing nodes this build never touched. Both can hold at
 			// once, so neither branch may swallow the other. Named "out of scope" rather than
 			// "skipped" because in this file a skip is specifically a user decision.
-			const outOfScopeNodeNames = scopeNodeNames
-				? allSetupRequests
-						.map((request) => request.node.name)
-						.filter((name) => !scopeNodeNames.includes(name))
-				: [];
+			const outOfScopeNodeNames = allSetupRequests
+				.filter((request) => !scopedRequests.includes(request))
+				.map((request) => request.node.name);
 			const outOfScopeReason =
 				outOfScopeNodeNames.length > 0
 					? `Pre-existing node(s) ${outOfScopeNodeNames
@@ -1837,10 +1852,11 @@ async function handleSetup(
 		}
 		// Re-analyze rather than remembering what was suspended: the closure state doesn't
 		// survive a resume in another process, and a skip that silently fails to persist is
-		// the whole bug. Everything still needing setup is what the user just dismissed.
-		const dismissed = (await analyzeWorkflow(context, input.workflowId)).filter(
-			(request) => request.needsAction,
-		);
+		// the whole bug. Everything in scope still needing setup is what the user just
+		// dismissed; a node the card left out was never offered, so it is not a skip.
+		const dismissed = (
+			await filterToSetupScope(context, input, await analyzeWorkflow(context, input.workflowId))
+		).filter((request) => request.needsAction);
 		await rememberSkippedSetup(context, dismissed, input.workflowId);
 		return {
 			success: true,

packages/@n8n/instance-ai/src/tools/workflows/__tests__/build-workflow.tool.test.ts

@@ -1612,6 +1612,90 @@ describe('createBuildWorkflowTool', () => {
 		});
 	});
 
+	describe('setup scope across rebuilds', () => {
+		const pendingWebhook = {
+			node: {
+				id: 'webhook-1',
+				name: 'Webhook',
+				type: 'n8n-nodes-base.webhook',
+				typeVersion: 2,
+				parameters: {},
+				position: [0, 0],
+			},
+			parameterIssues: { path: ['Missing webhook path'] },
+			isTrigger: true,
+			needsAction: true,
+		} as SetupRequest;
+
+		// The first build creates the workflow, so every node is in scope. The rebuild
+		// leaves the pending node untouched.
+		async function rebuildUnchanged({
+			workItemId,
+			nextWorkItemId = workItemId,
+			nextRunId,
+			failInBetween = false,
+		}: {
+			workItemId?: string;
+			nextWorkItemId?: string;
+			nextRunId?: string;
+			failInBetween?: boolean;
+		} = {}) {
+			vi.mocked(analyzeWorkflow).mockResolvedValue([pendingWebhook]);
+			const { context, filePath } = makeContext({ source: 'workflow source' });
+			const tool = createBuildWorkflowTool(context);
+			await executeTool<BuildToolOutput>(tool, { filePath, workItemId, name: 'Webhook flow' });
+			vi.mocked(context.workflowService.getAsWorkflowJSON).mockResolvedValue({
+				...structuredClone(generatedWorkflow),
+				name: 'Webhook flow',
+			});
+			if (failInBetween) {
+				vi.mocked(compileWorkflowSource).mockResolvedValueOnce({
+					success: false,
+					reason: 'workflow_json_invalid',
+					editable: true,
+					errors: ['Workflow JSON must include name, nodes, and connections.'],
+					summary: 'Workflow JSON source is missing required workflow fields.',
+				});
+				expect(await executeTool<BuildToolOutput>(tool, { filePath })).toMatchObject({
+					success: false,
+				});
+			}
+			if (nextRunId) context.runId = nextRunId;
+			return await executeTool<BuildToolOutput>(tool, { filePath, workItemId: nextWorkItemId });
+		}
+
+		it.each([false, true])(
+			'keeps pending setup for a node an earlier build in the same run created (failed rebuild in between: %s)',
+			async (failInBetween) => {
+				expect(await rebuildUnchanged({ failInBetween })).toMatchObject({
+					success: true,
+					setupRequirement: { status: 'required' },
+				});
+			},
+		);
+
+		it.each([
+			['a later run of the same work item', 'required', { workItemId: 'wi-1', nextRunId: 'run-2' }],
+			[
+				'another work item in the same run',
+				'required',
+				{ workItemId: 'wi-1', nextWorkItemId: 'wi-2' },
+			],
+			[
+				'another work item in a later run',
+				'not_required',
+				{ workItemId: 'wi-1', nextWorkItemId: 'wi-2', nextRunId: 'run-2' },
+			],
+			// Without a work item id, the file path stands in for it on every build.
+			['a later run without a work item id', 'not_required', { nextRunId: 'run-2' }],
+		])('rebuild in %s: setup %s', async (_label, status, options) => {
+			expect(await rebuildUnchanged(options)).toMatchObject({
+				success: true,
+				setupRequirement: { status },
+			});
+		});
+	});
+
 	it('updates the workflow bound to the source file and remembers the binding', async () => {
 		const { context, filePath, trackTelemetry } = makeContext({ source: 'workflow source' });
 		const tool = createBuildWorkflowTool(context);

packages/@n8n/instance-ai/src/tools/workflows/build-workflow.tool.ts

@@ -1483,13 +1483,28 @@ export function createBuildWorkflowTool(context: InstanceAiContext) {
 						(t): t is { nodeName: string; nodeType: string } =>
 							Boolean(t.nodeName) && Boolean(t.nodeType),
 					);
-				// Setup routing is scoped to nodes this build actually changed:
-				// pre-existing nodes the build merely round-tripped must not route
-				// the user into setup for an unrelated edit. Undefined = unscoped
-				// (new workflow, or prior state unreadable).
-				const changedNodeNames = savedWorkflowSnapshot
+				// Setup routing is scoped to nodes this work item's builds actually
+				// changed: pre-existing nodes the build merely round-tripped must not
+				// route the user into setup for an unrelated edit. Earlier builds of the
+				// same work item (or the same run) stay in scope, so a fix that leaves a
+				// new node alone keeps its pending setup. Undefined = unscoped (workflow
+				// created by this work item, or prior state unreadable).
+				const runId = buildContext?.runId ?? context.runId;
+				// The file path is the fallback work item id. It stays the same across
+				// work items, so only the run can match then.
+				const scopeWorkItemId = resolvedWorkItemId === filePath ? undefined : resolvedWorkItemId;
+				const priorScope = binding.setupScope;
+				const sameWork =
+					(scopeWorkItemId !== undefined && priorScope?.workItemId === scopeWorkItemId) ||
+					(runId !== undefined && priorScope?.runId === runId);
+				const buildChanges = savedWorkflowSnapshot
 					? computeChangedNodeNames(json, savedWorkflowSnapshot)
 					: undefined;
+				const changedNodeNames = !sameWork
+					? buildChanges
+					: priorScope?.nodeNames && buildChanges
+						? [...new Set([...priorScope.nodeNames, ...buildChanges])]
+						: undefined;
 				const isInSetupScope = (nodeName: string | undefined) =>
 					changedNodeNames === undefined ||
 					(nodeName !== undefined && changedNodeNames.includes(nodeName));
@@ -1555,7 +1570,6 @@ export function createBuildWorkflowTool(context: InstanceAiContext) {
 						scriptedGateCount: waitGateScripts?.length ?? 0,
 						savedWorkflowId: saved.id,
 					});
-					const runId = buildContext?.runId ?? context.runId;
 					const workflowName = json.name || 'workflow';
 					const summary = `${operation === 'update' ? 'Updated' : 'Created'} ${isSupportingWorkflow ? 'supporting ' : ''}workflow "${workflowName}" (${saved.id}).`;
 					binding = await saveWorkflowSourceFileBinding(context, {
@@ -1565,6 +1579,7 @@ export function createBuildWorkflowTool(context: InstanceAiContext) {
 						...(saved.checksum ? { workflowChecksum: saved.checksum } : {}),
 						sourceHash,
 						setupPending: undefined,
+						setupScope: { workItemId: scopeWorkItemId, runId, nodeNames: changedNodeNames },
 						...(setupPreferences
 							? {
 									setupPreferences: {

packages/@n8n/instance-ai/src/tools/workflows/workflow-file-bindings.ts

@@ -18,6 +18,14 @@ const workflowSourceFileBindingSchema = z.object({
 	/** False when the source was read while parameter values were hidden. */
 	parameterValuesIncluded: z.boolean().optional(),
 	setupPending: z.boolean().optional(),
+	/** Setup scope of the builds so far in this work item or run. `nodeNames` undefined = every node. */
+	setupScope: z
+		.object({
+			workItemId: z.string().optional(),
+			runId: z.string().optional(),
+			nodeNames: z.array(z.string()).optional(),
+		})
+		.optional(),
 	setupPreferences: z
 		.object({
 			runId: z.string().optional(),

packages/@n8n/instance-ai/src/workflow-loop/workflow-loop-state.ts

@@ -430,11 +430,12 @@ export const workflowBuildOutcomeSchema = z.object({
 	/** Whether any node parameters contain unresolved placeholder values. */
 	hasUnresolvedPlaceholders: z.boolean().optional(),
 	/**
-	 * Nodes this build added or modified relative to the previously saved
-	 * workflow. Setup routing and `workflows(action="setup")` requests are
-	 * scoped to these nodes so editing one node never routes pre-existing,
-	 * unrelated nodes into setup. Absent when the build created the workflow
-	 * or the prior state was unreadable — every node is then in scope.
+	 * Nodes that this build and earlier builds of the same work item or run added
+	 * or modified, each relative to the workflow saved before it. Setup routing and
+	 * `workflows(action="setup")` requests are scoped to these nodes so editing one
+	 * node never routes pre-existing, unrelated nodes into setup. Absent when one of
+	 * those builds created the workflow or the prior state was unreadable — every
+	 * node is then in scope.
 	 */
 	changedNodeNames: z.array(z.string()).optional(),
 	/**