diff --git a/packages/@n8n/api-types/src/dto/instance-ai/instance-ai-confirm-request.dto.ts b/packages/@n8n/api-types/src/dto/instance-ai/instance-ai-confirm-request.dto.ts index 2022696e0ad..39ca205abe7 100644 --- a/packages/@n8n/api-types/src/dto/instance-ai/instance-ai-confirm-request.dto.ts +++ b/packages/@n8n/api-types/src/dto/instance-ai/instance-ai-confirm-request.dto.ts @@ -83,6 +83,9 @@ const setupWorkflowApplyConfirmSchema = z.object({ kind: z.literal('setupWorkflowApply'), nodeCredentials: nodeCredentialsRecord, nodeParameters: nodeParametersRecord, + /** Nodes whose cards the user actively skipped in this panel. Without this the backend + * can only see that they're still unconfigured, which reads as "ask again". */ + skippedNodes: z.array(z.string()).optional(), }); /** Workflow-setup wizard: run a test-trigger against a specific node. Approval is implied; diff --git a/packages/@n8n/api-types/src/index.ts b/packages/@n8n/api-types/src/index.ts index 873be0407b6..7d6854a3a6b 100644 --- a/packages/@n8n/api-types/src/index.ts +++ b/packages/@n8n/api-types/src/index.ts @@ -293,6 +293,8 @@ export { buildRunWorkflowSessionGrantKey, buildUpdateWorkflowSessionGrantKey, buildDataTablesSessionGrantKey, + buildSetupSkipGrantKey, + parseSetupSkipGrants, buildFetchUrlGrantKey, FETCH_URL_ALLOW_ALL_GRANT_KEY, WEB_SEARCH_GRANT_KEY, diff --git a/packages/@n8n/api-types/src/schemas/__tests__/instance-ai.schema.test.ts b/packages/@n8n/api-types/src/schemas/__tests__/instance-ai.schema.test.ts index 9f0986afd4e..3f739a003c0 100644 --- a/packages/@n8n/api-types/src/schemas/__tests__/instance-ai.schema.test.ts +++ b/packages/@n8n/api-types/src/schemas/__tests__/instance-ai.schema.test.ts @@ -12,6 +12,8 @@ import { applyBranchReadOnlyOverrides, buildDataTablesSessionGrantKey, buildUpdateWorkflowSessionGrantKey, + buildSetupSkipGrantKey, + parseSetupSkipGrants, buildFetchUrlGrantKey, DEFAULT_INSTANCE_AI_PERMISSIONS, errorPayloadSchema, @@ -455,6 +457,25 @@ describe('workflow update session grant keys', () => { }); }); +describe('workflow-setup skip keys', () => { + it('round-trips credential types and ignores unrelated keys', () => { + const keys = new Set([ + buildSetupSkipGrantKey('slackApi'), + buildSetupSkipGrantKey('Wait for Form'), + 'executions:run:wf-1', + ]); + + expect(buildSetupSkipGrantKey('slackApi')).toBe('workflows:setup-skip:slackApi'); + expect(parseSetupSkipGrants(keys)).toEqual(new Set(['slackApi', 'Wait for Form'])); + }); + + it('does not collide with the workflow-update namespace', () => { + expect(parseSetupSkipGrants(new Set([buildUpdateWorkflowSessionGrantKey('wf-1')])).size).toBe( + 0, + ); + }); +}); + describe('domain-access grant keys', () => { it('builds and parses per-host grant keys round-trip', () => { const key = buildFetchUrlGrantKey('example.com'); diff --git a/packages/@n8n/api-types/src/schemas/instance-ai.schema.ts b/packages/@n8n/api-types/src/schemas/instance-ai.schema.ts index 456797e75bb..509f3770978 100644 --- a/packages/@n8n/api-types/src/schemas/instance-ai.schema.ts +++ b/packages/@n8n/api-types/src/schemas/instance-ai.schema.ts @@ -73,6 +73,35 @@ export function buildDataTablesSessionGrantKey(action: string): string { return `data-tables:${action}`; } +// --- Workflow-setup skips --- + +const SETUP_SKIP_GRANT_PREFIX = 'workflows:setup-skip:'; + +/** + * Builds the thread-level key recording that the user passed on a setup card. Unlike its + * siblings this records a *declined* decision rather than an approval, but it belongs on the + * same per-thread, per-user store: the setup flow reads it to stop re-opening the blocking + * setup card for something the user already skipped in this conversation. + * + * `subject` is opaque here — what a skip generalises to is the setup flow's call, so the + * kind-tagged subjects (`cred:`, `node::`) are built by + * `setup-skip-state.ts` in the instance-ai package. + */ +export function buildSetupSkipGrantKey(subject: string): string { + return `${SETUP_SKIP_GRANT_PREFIX}${subject}`; +} + +/** The skip subjects recorded for this thread, parsed out of persisted grant keys. */ +export function parseSetupSkipGrants(keys: ReadonlySet): Set { + const skipped = new Set(); + for (const key of keys) { + if (key.startsWith(SETUP_SKIP_GRANT_PREFIX)) { + skipped.add(key.slice(SETUP_SKIP_GRANT_PREFIX.length)); + } + } + return skipped; +} + // --- Domain-access grants ("always allow" for web access) --- // These keys mirror the research tool's action names (`fetch-url`, `web-search`) the same // way `executions:run:` mirrors the executions `run` action, so a persisted grant row @@ -485,6 +514,14 @@ export const workflowSetupNodeSchema = z.object({ 'Whether this node still requires user intervention. ' + 'False when credentials are set and valid, parameters are resolved, etc.', ), + credentialNeedsAction: z + .boolean() + .optional() + .describe( + 'Whether the credential slot itself is what needs intervention. False when the node has a ' + + 'resolvable credential and only a parameter is missing — that card asks about a parameter, ' + + 'not about the service, so a skip of it must not be generalised to the credential type.', + ), subnodeRootNode: z .object({ name: z.string(), diff --git a/packages/@n8n/instance-ai/docs/tools.md b/packages/@n8n/instance-ai/docs/tools.md index eb0ac62ff6a..a22464d7ec8 100644 --- a/packages/@n8n/instance-ai/docs/tools.md +++ b/packages/@n8n/instance-ai/docs/tools.md @@ -272,7 +272,9 @@ configure it interactively. |-------|------|----------|-------------| | `workflowId` | string | yes | Workflow to set up | -**Returns**: `{ completedNodes, skippedNodes, failedNodes }` +**Returns**: `{ completedNodes, nodesStillNeedingSetup, skippedByUser, failedNodes }` — +`nodesStillNeedingSetup` is what nobody has configured yet, `skippedByUser` what the user +actively dismissed and the agent must not re-open (see `reopenSkipped`). ### `publish-workflow` diff --git a/packages/@n8n/instance-ai/evaluations/__tests__/build-workflow-conversation-seed.test.ts b/packages/@n8n/instance-ai/evaluations/__tests__/build-workflow-conversation-seed.test.ts index 0dbddd8edf6..ca9367e9142 100644 --- a/packages/@n8n/instance-ai/evaluations/__tests__/build-workflow-conversation-seed.test.ts +++ b/packages/@n8n/instance-ai/evaluations/__tests__/build-workflow-conversation-seed.test.ts @@ -297,14 +297,12 @@ describe('buildWorkflow with an inline seed', () => { // and never hunts by name. it('sends the attached seed workflow with the opening message, using the REMAPPED id', async () => { const sendMessage = vi.fn().mockResolvedValue({ runId: 'run-1' }); - const restoreThread = vi - .fn() - .mockResolvedValue({ - restored: 1, - workflowIds: ['restored-wf-1'], - dataTableIds: [], - agentIds: [], - }); + const restoreThread = vi.fn().mockResolvedValue({ + restored: 1, + workflowIds: ['restored-wf-1'], + dataTableIds: [], + agentIds: [], + }); await buildWorkflow({ client: makeClient(restoreThread, { sendMessage }), ...baseConfig, @@ -339,14 +337,12 @@ describe('buildWorkflow with an inline seed', () => { // empty strings). The recorded turn names it instead. it('names the attached workflow in the RECORDED turn, so judges can see the hand-off', async () => { const sendMessage = vi.fn().mockResolvedValue({ runId: 'run-1' }); - const restoreThread = vi - .fn() - .mockResolvedValue({ - restored: 1, - workflowIds: ['restored-wf-1'], - dataTableIds: [], - agentIds: [], - }); + const restoreThread = vi.fn().mockResolvedValue({ + restored: 1, + workflowIds: ['restored-wf-1'], + dataTableIds: [], + agentIds: [], + }); vi.mocked(recordUserTurn).mockClear(); await buildWorkflow({ @@ -374,14 +370,12 @@ describe('buildWorkflow with an inline seed', () => { // hand-off case with follow-ups would otherwise audit plans and decide follow-ups // against a blank opening turn that never mentions the workflow. it('names the attached workflow in the script the user proxy reads', async () => { - const restoreThread = vi - .fn() - .mockResolvedValue({ - restored: 1, - workflowIds: ['restored-wf-1'], - dataTableIds: [], - agentIds: [], - }); + const restoreThread = vi.fn().mockResolvedValue({ + restored: 1, + workflowIds: ['restored-wf-1'], + dataTableIds: [], + agentIds: [], + }); proxyScripts.length = 0; await buildWorkflow({ @@ -406,14 +400,12 @@ describe('buildWorkflow with an inline seed', () => { // The schema refuses an `attach` the seed does not declare, so a miss here means // the restore/remap lost it. Running on would silently downgrade the case to a // find-it test and grade the wrong thing. - const restoreThread = vi - .fn() - .mockResolvedValue({ - restored: 1, - workflowIds: ['restored-wf-1'], - dataTableIds: [], - agentIds: [], - }); + const restoreThread = vi.fn().mockResolvedValue({ + restored: 1, + workflowIds: ['restored-wf-1'], + dataTableIds: [], + agentIds: [], + }); const result = await buildWorkflow({ client: makeClient(restoreThread, { sendMessage: vi.fn().mockResolvedValue({ runId: 'r' }) }), diff --git a/packages/@n8n/instance-ai/evaluations/__tests__/conversation-seed.test.ts b/packages/@n8n/instance-ai/evaluations/__tests__/conversation-seed.test.ts index 273bf0db99b..9d4bf18e73f 100644 --- a/packages/@n8n/instance-ai/evaluations/__tests__/conversation-seed.test.ts +++ b/packages/@n8n/instance-ai/evaluations/__tests__/conversation-seed.test.ts @@ -679,7 +679,45 @@ describe('transcriptPrefixFromSeed', () => { { kind: 'setup-wizard', completedNodes: [{ nodeName: 'Schedule', parametersSet: ['rule'] }], - skippedNodes: [{ nodeName: 'Slack', credentialType: 'slackApi' }], + // Seeded before the split: the pre-split `skippedNodes` key still parses. + nodesStillNeedingSetup: [{ nodeName: 'Slack', credentialType: 'slackApi' }], + reason: undefined, + }, + ]); + }); + + it('renders a skipped-only setup outcome, which carries neither completedNodes nor the old key', () => { + // The apply result splits "still unconfigured" from "the user declined this". A seed + // carrying only the latter used to fall through the guard and vanish from the transcript. + const turns = transcriptPrefixFromSeed([ + { + id: 'a1', + type: 'tool', + role: 'assistant', + content: [ + { + type: 'tool-call', + toolCallId: 'c1', + toolName: 'workflows[setup]', + state: 'resolved', + input: { action: 'setup', workflowId: 'wf1' }, + output: { + success: true, + skippedByUser: [ + { nodeName: 'Post to Slack', credentialType: 'slackApi', reopenWith: 'slackApi' }, + ], + }, + }, + ], + createdAt: '2026-01-01T00:00:00Z', + }, + ]); + expect(turns[0].steps).toEqual([ + { + kind: 'setup-wizard', + completedNodes: [], + nodesStillNeedingSetup: [], + skippedByUser: [{ nodeName: 'Post to Slack', credentialType: 'slackApi' }], reason: undefined, }, ]); 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 9556df3d101..6c3a6c82bd4 100644 --- a/packages/@n8n/instance-ai/evaluations/__tests__/event-parser.test.ts +++ b/packages/@n8n/instance-ai/evaluations/__tests__/event-parser.test.ts @@ -832,7 +832,7 @@ describe('seededTurnCounters', () => { const [counter] = seededTurnCounters([ seededTurn([ { kind: 'agent-text', text: 'hello' }, - { kind: 'setup-wizard', completedNodes: [], skippedNodes: [] }, + { kind: 'setup-wizard', completedNodes: [], nodesStillNeedingSetup: [] }, ]), ]); expect(counter.toolCallCount).toBe(1); // setup-wizard is a tool call; agent-text is not diff --git a/packages/@n8n/instance-ai/evaluations/__tests__/transcript-from-events.test.ts b/packages/@n8n/instance-ai/evaluations/__tests__/transcript-from-events.test.ts index f71b78ba963..a8f295a3da6 100644 --- a/packages/@n8n/instance-ai/evaluations/__tests__/transcript-from-events.test.ts +++ b/packages/@n8n/instance-ai/evaluations/__tests__/transcript-from-events.test.ts @@ -313,7 +313,8 @@ describe('buildTranscriptFromEvents', () => { expect(interactions[1]).toMatchObject({ kind: 'setup-wizard', completedNodes: [{ nodeName: 'Schedule', parametersSet: ['cron'] }], - skippedNodes: [{ nodeName: 'Slack', credentialType: 'slackApi' }], + // Recorded before the split, so the pre-split `skippedNodes` key still parses. + nodesStillNeedingSetup: [{ nodeName: 'Slack', credentialType: 'slackApi' }], }); }); diff --git a/packages/@n8n/instance-ai/evaluations/harness/conversation-seed.ts b/packages/@n8n/instance-ai/evaluations/harness/conversation-seed.ts index 678f4c2c327..7a3774be7c4 100644 --- a/packages/@n8n/instance-ai/evaluations/harness/conversation-seed.ts +++ b/packages/@n8n/instance-ai/evaluations/harness/conversation-seed.ts @@ -520,7 +520,14 @@ const interpretPlan: SeedStepInterpreter = (call) => { // rendering as the live `workflows` result). const interpretSetupWizard: SeedStepInterpreter = (call) => { const { output } = call; - if (!output || !(Array.isArray(output.completedNodes) || Array.isArray(output.skippedNodes))) { + // `skippedNodes` is the pre-split key, kept so seeded fixtures recorded then still parse. + const setupOutcomeKeys = [ + 'completedNodes', + 'nodesStillNeedingSetup', + 'skippedByUser', + 'skippedNodes', + ]; + if (!output || !setupOutcomeKeys.some((key) => Array.isArray(output[key]))) { return null; } return extractSetupWizardOutcome(output); diff --git a/packages/@n8n/instance-ai/evaluations/outcome/transcript-from-events.ts b/packages/@n8n/instance-ai/evaluations/outcome/transcript-from-events.ts index ce4e6f834c9..21bebda2cef 100644 --- a/packages/@n8n/instance-ai/evaluations/outcome/transcript-from-events.ts +++ b/packages/@n8n/instance-ai/evaluations/outcome/transcript-from-events.ts @@ -282,12 +282,28 @@ export function extractSetupWizardOutcome(result: Record): Tool const completed = Array.isArray(result.completedNodes) ? extractCompletedNodes(result.completedNodes) : []; - const skipped = Array.isArray(result.skippedNodes) - ? extractSkippedNodes(result.skippedNodes) + // `skippedNodes` was this list's name before it split into "still unconfigured" and + // "declined by the user"; traces and seeded fixtures recorded then still carry it. + const stillNeedingSetupRaw = Array.isArray(result.nodesStillNeedingSetup) + ? result.nodesStillNeedingSetup + : Array.isArray(result.skippedNodes) + ? result.skippedNodes + : undefined; + const stillNeedingSetup = stillNeedingSetupRaw ? extractSkippedNodes(stillNeedingSetupRaw) : []; + const skippedByUser = Array.isArray(result.skippedByUser) + ? extractSkippedNodes(result.skippedByUser) : []; - if (completed.length === 0 && skipped.length === 0) return null; + if (completed.length === 0 && stillNeedingSetup.length === 0 && skippedByUser.length === 0) { + return null; + } const reason = typeof result.reason === 'string' ? result.reason : undefined; - return { kind: 'setup-wizard', completedNodes: completed, skippedNodes: skipped, reason }; + return { + kind: 'setup-wizard', + completedNodes: completed, + nodesStillNeedingSetup: stillNeedingSetup, + ...(skippedByUser.length > 0 ? { skippedByUser } : {}), + reason, + }; } /** diff --git a/packages/@n8n/instance-ai/evaluations/report/workflow-report.ts b/packages/@n8n/instance-ai/evaluations/report/workflow-report.ts index 391e946a1f1..7465c0fa1ab 100644 --- a/packages/@n8n/instance-ai/evaluations/report/workflow-report.ts +++ b/packages/@n8n/instance-ai/evaluations/report/workflow-report.ts @@ -18,6 +18,7 @@ import type { BuildExpectationResult, ConversationMetrics, ExecutionScenarioResult, + SetupWizardSkippedNode, ToolInteraction, TranscriptStep, TranscriptTurn, @@ -1035,7 +1036,8 @@ function renderInteraction(interaction: ToolInteraction): string | null { return `
${summary}
    ${lines}
`; } case 'setup-wizard': { - const skipped = interaction.skippedNodes; + const skipped = interaction.nodesStillNeedingSetup; + const declined = interaction.skippedByUser ?? []; const needCreds = skipped.filter((s) => Boolean(s.credentialType)).length; const needParams = skipped.length - needCreds; const breakdown: string[] = []; @@ -1047,9 +1049,12 @@ function renderInteraction(interaction: ToolInteraction): string | null { } if (skipped.length > 0) { headerParts.push( - `${String(skipped.length)} skipped${breakdown.length > 0 ? ` (${breakdown.join(', ')})` : ''}`, + `${String(skipped.length)} unconfigured${breakdown.length > 0 ? ` (${breakdown.join(', ')})` : ''}`, ); } + if (declined.length > 0) { + headerParts.push(`${String(declined.length)} skipped by user`); + } const header = headerParts.length > 0 ? headerParts.join(', ') : 'nothing to apply'; const sections: string[] = []; @@ -1064,17 +1069,20 @@ function renderInteraction(interaction: ToolInteraction): string | null { `
    ${items}
`, ); } - if (skipped.length > 0) { - const items = skipped + const renderNodeList = (nodes: SetupWizardSkippedNode[], label: string) => { + const items = nodes .map( (s) => `
  • ${escapeHtml(s.nodeName)}${s.credentialType ? ` — needs ${escapeHtml(s.credentialType)} credential` : ' — needs parameters'}
  • `, ) .join(''); sections.push( - `
      ${items}
    `, + `
      ${items}
    `, ); - } + }; + if (skipped.length > 0) renderNodeList(skipped, 'unconfigured'); + // Separate section: re-asking for one of these is the failure a judge looks for. + if (declined.length > 0) renderNodeList(declined, 'skipped by user'); return `
    🛠 setup wizard — ${escapeHtml(header)}${sections.join('')}
    `; } case 'setup-card': { diff --git a/packages/@n8n/instance-ai/evaluations/types.ts b/packages/@n8n/instance-ai/evaluations/types.ts index e97e2de5d7f..6082bfda352 100644 --- a/packages/@n8n/instance-ai/evaluations/types.ts +++ b/packages/@n8n/instance-ai/evaluations/types.ts @@ -380,7 +380,10 @@ export type ToolInteraction = | { kind: 'setup-wizard'; completedNodes: SetupWizardCompletedNode[]; - skippedNodes: SetupWizardSkippedNode[]; + /** Left unconfigured — nobody has filled these in yet. */ + nodesStillNeedingSetup: SetupWizardSkippedNode[]; + /** Actively dismissed by the user, which the assistant must not ask about again. */ + skippedByUser?: SetupWizardSkippedNode[]; reason?: string; } | { diff --git a/packages/@n8n/instance-ai/evaluations/utils/conversation-text.ts b/packages/@n8n/instance-ai/evaluations/utils/conversation-text.ts index d2f1f60b882..0839b130bb3 100644 --- a/packages/@n8n/instance-ai/evaluations/utils/conversation-text.ts +++ b/packages/@n8n/instance-ai/evaluations/utils/conversation-text.ts @@ -1,7 +1,13 @@ import { isRecord } from '@n8n/utils/is-record'; import type { CaseSeed } from '../harness/schema'; -import type { ConversationTurn, ToolInteraction, TranscriptStep, TranscriptTurn } from '../types'; +import type { + ConversationTurn, + SetupWizardSkippedNode, + ToolInteraction, + TranscriptStep, + TranscriptTurn, +} from '../types'; /** Render a turn's out-of-band workflow attachment for a transcript/prompt, e.g. * `[attached workflow: Batch loop]`, or '' when it has none. The editor hands the @@ -246,12 +252,17 @@ function describeInteraction(interaction: ToolInteraction): string | null { ); parts.push(`configured ${configured.join('; ')}`); } - if (interaction.skippedNodes.length > 0) { - const skipped = interaction.skippedNodes.map( - (s) => - `${s.nodeName}${s.credentialType ? ` (needs ${s.credentialType} credential)` : ' (needs parameters)'}`, + const describeNeeds = (node: SetupWizardSkippedNode) => + `${node.nodeName}${node.credentialType ? ` (needs ${node.credentialType} credential)` : ' (needs parameters)'}`; + if (interaction.nodesStillNeedingSetup.length > 0) { + parts.push( + `still needs setup ${interaction.nodesStillNeedingSetup.map(describeNeeds).join(', ')}`, ); - parts.push(`skipped ${skipped.join(', ')}`); + } + // Kept distinct from the above: the judge cares whether the assistant re-asked for + // something the user declined, which reads the same as "unconfigured" if merged. + if (interaction.skippedByUser && interaction.skippedByUser.length > 0) { + parts.push(`user skipped ${interaction.skippedByUser.map(describeNeeds).join(', ')}`); } const body = parts.length > 0 ? parts.join('; ') : 'nothing to apply'; return `Setup wizard: ${body}${interaction.reason ? ` — ${interaction.reason}` : ''}`; diff --git a/packages/@n8n/instance-ai/skills/planned-task-runtime/SKILL.md b/packages/@n8n/instance-ai/skills/planned-task-runtime/SKILL.md index 00d7a528a36..43199ff816a 100644 --- a/packages/@n8n/instance-ai/skills/planned-task-runtime/SKILL.md +++ b/packages/@n8n/instance-ai/skills/planned-task-runtime/SKILL.md @@ -135,8 +135,9 @@ succeeds and any verified workflow dependency outcome has `workflows(action="setup")` with that workflowId before `complete-checkpoint`; the inline setup card appears automatically in the AI Assistant panel, so do not tell the user to open the editor, use the canvas, or click a Setup button. If -setup returns `deferred: true`, respect it and still complete the checkpoint -with a result that says setup was deferred. Do not call +setup returns `deferred: true`, or reports `skippedByUser`, respect it and still +complete the checkpoint with a result that says setup was deferred — never call +setup again for a credential the user skipped. Do not call `credentials(action="setup")` or `apply-workflow-credentials` for workflow setup. Then call `complete-checkpoint(taskId, status, result)` **exactly once** to report the outcome (`status: "succeeded"` on pass, `"failed"` on a verification 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 98f86fc50bb..bed3aa1da97 100644 --- a/packages/@n8n/instance-ai/skills/post-build-flow/SKILL.md +++ b/packages/@n8n/instance-ai/skills/post-build-flow/SKILL.md @@ -50,6 +50,9 @@ is to call `workflows(action="setup")` with the `workflowId` from the payload. D not verify, do not ask, do not write a message first — the inline setup card in the AI Assistant panel is the user-visible surface. If it returns `deferred: true`, respect the user's choice and do not retry with any other setup tool. +A result carrying `skippedByUser` names credentials the user already passed on: +never re-open setup for those, in this turn or any later one — see +[Credentials the user skipped](#credentials-the-user-skipped). After setup completes or is applied, follow [Mocked verification live-test follow-up](#mocked-verification-live-test-follow-up) if the payload or prior verification evidence says mocked credentials, @@ -153,6 +156,35 @@ If the user defers setup instead, don't hand them manual field-by-field credential instructions for the n8n editor — tell them to reopen setup when they're ready: the card pre-fills everything except their key. +### Credentials the user skipped + +Skipping is remembered for the whole conversation. A setup result may carry +`skippedByUser` (nodes and credential types the user passed on), and a build +outcome may carry `setupRequirement.status === "not_required"` with +`reason: "skipped-by-user"`. In both cases the blocking setup card is off the +table for those credentials — including after later edits, rebuilds, and +`` steps. Asking again is the single most common +complaint about this flow. + +Instead, in your normal message: + +- name what stays unconfigured and what happens at runtime (e.g. "the Slack post + will fail until a channel is selected; the email still sends"), +- offer to set it up whenever they want. + +Only once the user asks for a specific credential — "connect Slack now", "let's +do the Slack setup", or picking it out of an offer you made — call +`workflows(action="setup", reopenSkipped: ["slackApi"])`, naming just what they +asked for so the rest stays skipped. A generic "yes" to an unrelated question is +not an ask. + +Pass the `reopenWith` value the tool reported for that card, not the user's +wording — a credential type for a credential card, a node name for one that was +only missing a parameter. If nothing matches, setup answers with +`unknown_reopen_target` and the list you can choose from; pick from it or tell +the user what they named isn't part of this workflow. Don't fall back to +re-offering, the user already asked. + ## Publishing and testing **Publishing is never required for testing.** Both `executions(action="run")` and @@ -258,9 +290,13 @@ again. 4. When `workflows(action="setup")` opens the inline setup card, the card is the user-visible surface. Do not tell the user to open the editor, use the canvas, or click a Setup button; the user does not need to navigate anywhere. -5. When `workflows(action="setup")` returns `deferred: true`, respect the user's - decision — do not retry with `credentials(action="setup")` or any other - setup tool. The user chose to set things up later. +5. When `workflows(action="setup")` returns `deferred: true`, or reports + `skippedByUser`, or applies only part of the card, respect the user's + decision — do not retry with `credentials(action="setup")`, another + `workflows(action="setup")` call, or any other setup tool. `partial: true` + with `nodesStillNeedingSetup` is not permission to re-open the card in the + same turn: report what remains as described in + [Credentials the user skipped](#credentials-the-user-skipped). 6. After setup completes or is applied, follow [Mocked verification live-test follow-up](#mocked-verification-live-test-follow-up) when the latest verification evidence used mocks or simulations. If this diff --git a/packages/@n8n/instance-ai/skills/workflow-builder/SKILL.md b/packages/@n8n/instance-ai/skills/workflow-builder/SKILL.md index 1c4bde752dc..62eb1c4307f 100644 --- a/packages/@n8n/instance-ai/skills/workflow-builder/SKILL.md +++ b/packages/@n8n/instance-ai/skills/workflow-builder/SKILL.md @@ -286,7 +286,8 @@ When this turn is responsible for verification, do not stop after a successful save. The job is done when one of these is true: - The workflow is verified by structured tool evidence. -- Setup is required and `workflows(action="setup")` has been routed or deferred. +- Setup is required and `workflows(action="setup")` has been routed or deferred, + or the only setup left is for credentials the user skipped earlier. - A remediation guard says `shouldEdit: false`. - You are blocked after one repair attempt per unique failure signature. diff --git a/packages/@n8n/instance-ai/src/agent/system-prompt.ts b/packages/@n8n/instance-ai/src/agent/system-prompt.ts index f61c5118050..cf2a91c8275 100644 --- a/packages/@n8n/instance-ai/src/agent/system-prompt.ts +++ b/packages/@n8n/instance-ai/src/agent/system-prompt.ts @@ -167,7 +167,7 @@ Don't fabricate provider setup mechanics (credential field names, secret values, ## Safety - **Destructive operations** show a confirmation UI automatically — don't ask via text. -- **Credential setup** uses \`workflows(action="setup")\` when a workflowId is available — it opens the inline setup card in the AI Assistant panel and handles credentials, parameters, and triggers in one step. Use \`credentials(action="setup")\` only when the user explicitly asks to create a credential outside of any workflow context. Never call both tools for the same workflow. Never describe workflow setup as something the user starts from the canvas or editor. Setup cards are only open while the setup call is pending — once it returns a result, the card is resolved: describe the outcome (e.g. credentials selected and ready), never that a card is open or that the user still needs to authorize. When a skipped node carries \`parameterIssues\`, the connected credential can't reach the value that was configured (e.g. a model outside what the credential allows) — fix the value, then tell the user plainly which value didn't work and what you set instead. Never silently swap a model or other parameter without saying so. +- **Credential setup** uses \`workflows(action="setup")\` when a workflowId is available — it opens the inline setup card in the AI Assistant panel and handles credentials, parameters, and triggers in one step. Use \`credentials(action="setup")\` only when the user explicitly asks to create a credential outside of any workflow context. Never call both tools for the same workflow. Never describe workflow setup as something the user starts from the canvas or editor. Setup cards are only open while the setup call is pending — once it returns a result, the card is resolved: describe the outcome (e.g. credentials selected and ready), never that a card is open or that the user still needs to authorize. When a node in \`nodesStillNeedingSetup\` carries \`parameterIssues\`, the connected credential can't reach the value that was configured (e.g. a model outside what the credential allows) — fix the value, then tell the user plainly which value didn't work and what you set instead. Never silently swap a model or other parameter without saying so. Nodes listed under \`skippedByUser\` are different: the user chose to skip them, so never re-open the setup card for those — say what stays unconfigured and offer to set it up later. - **Error workflows are per workflow** — n8n has no global/instance-wide error workflow setting. Mention that only when the user explicitly asks about global error workflow behavior; build/assign steps live in \`workflow-builder\` and \`post-build-flow\`. - **Never expose credential secrets** — metadata only. diff --git a/packages/@n8n/instance-ai/src/runtime/run-state-registry.ts b/packages/@n8n/instance-ai/src/runtime/run-state-registry.ts index c22b5d5f985..d99d9209d78 100644 --- a/packages/@n8n/instance-ai/src/runtime/run-state-registry.ts +++ b/packages/@n8n/instance-ai/src/runtime/run-state-registry.ts @@ -67,6 +67,8 @@ export interface ConfirmationData { domainAccessAction?: string; action?: 'apply' | 'test-trigger'; nodeParameters?: Record>; + /** Workflow-setup cards the user actively skipped, by node name. */ + skippedNodes?: string[]; testTriggerNode?: string; answers?: Array<{ questionId: string; diff --git a/packages/@n8n/instance-ai/src/tools/__tests__/workflows.tool.test.ts b/packages/@n8n/instance-ai/src/tools/__tests__/workflows.tool.test.ts index db314ebf425..edb73718749 100644 --- a/packages/@n8n/instance-ai/src/tools/__tests__/workflows.tool.test.ts +++ b/packages/@n8n/instance-ai/src/tools/__tests__/workflows.tool.test.ts @@ -2018,6 +2018,364 @@ describe('workflows tool', () => { // Settled requests never count as pending, so the apply is not partial. expect(result).not.toHaveProperty('partial'); }); + + describe('credentials the user skipped', () => { + const slackRequest = { + node: { name: 'Post to Slack', type: 'n8n-nodes-base.slack' }, + credentialType: 'slackApi', + needsAction: true, + credentialNeedsAction: true, + }; + const sheetsRequest = { + node: { name: 'Log to Sheet', type: 'n8n-nodes-base.googleSheets' }, + credentialType: 'googleSheetsOAuth2Api', + needsAction: true, + credentialNeedsAction: true, + }; + /** Sheets is connected — this card only asks for a parameter on this one node. */ + const sheetsParamRequest = { + node: { name: 'Log to Sheet', type: 'n8n-nodes-base.googleSheets' }, + credentialType: 'googleSheetsOAuth2Api', + needsAction: true, + parameterIssues: { documentId: ['Placeholder "SPREADSHEET_ID"'] }, + }; + + /** Mirrors the service wiring: one mutable set, read and written through the context. */ + function createGrantAwareContext(granted: string[] = []) { + const sessionApprovedToolKeys = new Set(granted); + return createMockContext({ + sessionApprovedToolKeys, + grantSessionToolApproval: vi.fn(async (key: string) => { + await Promise.resolve(); + sessionApprovedToolKeys.add(key); + }), + revokeSessionToolApproval: vi.fn(async (key: string) => { + await Promise.resolve(); + sessionApprovedToolKeys.delete(key); + }), + }); + } + + it('leaves skipped credentials out of the setup card', async () => { + (analyzeWorkflow as Mock).mockResolvedValue([slackRequest, sheetsRequest]); + const context = createGrantAwareContext(['workflows:setup-skip:cred:slackApi']); + const suspend = vi.fn(); + + const tool = createWorkflowsTool(context, 'full'); + await executeTool(tool, { action: 'setup', workflowId: 'wf1' }, { + suspend, + resumeData: undefined, + } as never); + + expect(suspend.mock.calls[0][0]).toMatchObject({ setupRequests: [sheetsRequest] }); + }); + + it('reports instead of suspending when only skipped credentials remain', async () => { + (analyzeWorkflow as Mock).mockResolvedValue([slackRequest]); + const context = createGrantAwareContext(['workflows:setup-skip:cred:slackApi']); + const suspend = vi.fn(); + + const tool = createWorkflowsTool(context, 'full'); + const result = await executeTool(tool, { action: 'setup', workflowId: 'wf1' }, { + suspend, + resumeData: undefined, + } as never); + + expect(suspend).not.toHaveBeenCalled(); + expect(result).toMatchObject({ + success: true, + skippedByUser: [{ nodeName: 'Post to Slack', credentialType: 'slackApi' }], + }); + }); + + it('re-opens a skipped card when the user asks for it', async () => { + (analyzeWorkflow as Mock).mockResolvedValue([slackRequest]); + const context = createGrantAwareContext(['workflows:setup-skip:cred:slackApi']); + const suspend = vi.fn(); + + const tool = createWorkflowsTool(context, 'full'); + await executeTool( + tool, + { action: 'setup', workflowId: 'wf1', reopenSkipped: ['Post to Slack'] }, + { + suspend, + resumeData: undefined, + } as never, + ); + + expect(context.revokeSessionToolApproval).toHaveBeenCalledWith( + 'workflows:setup-skip:cred:slackApi', + ); + expect(suspend.mock.calls[0][0]).toMatchObject({ setupRequests: [slackRequest] }); + }); + + it('reports a skipped card and an out-of-scope one separately', async () => { + // The two filters answer different questions — "did this build touch it" and "did the + // user decline it" — and the agent has to say different things about each, so neither + // report may swallow the other. + (analyzeWorkflow as Mock).mockResolvedValue([slackRequest, sheetsRequest]); + const context = createGrantAwareContext(['workflows:setup-skip:cred:slackApi']); + 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: ['Post to Slack'] }), + }, + }; + const suspend = vi.fn(); + + const tool = createWorkflowsTool(context, 'full'); + const result = await executeTool(tool, { action: 'setup', workflowId: 'wf1' }, { + suspend, + resumeData: undefined, + } as never); + + // Slack is in scope but declined; Sheets is pending but untouched by this build. + expect(suspend).not.toHaveBeenCalled(); + expect(result).toMatchObject({ + success: true, + skippedByUser: [{ nodeName: 'Post to Slack', reopenWith: 'slackApi' }], + }); + expect(result).toHaveProperty('reason', expect.stringContaining('already skipped')); + expect(result).toHaveProperty('reason', expect.stringContaining('"Log to Sheet"')); + }); + + it('still hides a skipped card that the build did change', async () => { + (analyzeWorkflow as Mock).mockResolvedValue([slackRequest, sheetsRequest]); + const context = createGrantAwareContext(['workflows:setup-skip:cred:slackApi']); + 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: ['Post to Slack', 'Log to Sheet'], + }), + }, + }; + const suspend = vi.fn(); + + const tool = createWorkflowsTool(context, 'full'); + await executeTool(tool, { action: 'setup', workflowId: 'wf1' }, { + suspend, + resumeData: undefined, + } as never); + + // Being in scope does not override the user's decision. + expect(suspend.mock.calls[0][0]).toMatchObject({ setupRequests: [sheetsRequest] }); + }); + + it('reports a reopen request that names nothing in the workflow', async () => { + // Otherwise the guidance tells the caller to wait until the user asks — which is + // exactly what just happened — and the request disappears. + (analyzeWorkflow as Mock).mockResolvedValue([slackRequest]); + const context = createGrantAwareContext(['workflows:setup-skip:cred:slackApi']); + const suspend = vi.fn(); + + const tool = createWorkflowsTool(context, 'full'); + const result = await executeTool( + tool, + { action: 'setup', workflowId: 'wf1', reopenSkipped: ['Notion'] }, + { suspend, resumeData: undefined } as never, + ); + + expect(suspend).not.toHaveBeenCalled(); + expect(result).toMatchObject({ + error: 'unknown_reopen_target', + unmatchedReopen: ['Notion'], + reopenable: [{ nodeName: 'Post to Slack', reopenWith: 'slackApi' }], + }); + }); + + it('reports an unknown entry even when another one resolves', async () => { + // "connect Slack and Notion" with no Notion node: opening the Slack card and + // suspending would drop half of what the user asked for with nowhere to report it. + (analyzeWorkflow as Mock).mockResolvedValue([slackRequest, sheetsRequest]); + const context = createGrantAwareContext(['workflows:setup-skip:cred:slackApi']); + const suspend = vi.fn(); + + const tool = createWorkflowsTool(context, 'full'); + const result = await executeTool( + tool, + { action: 'setup', workflowId: 'wf1', reopenSkipped: ['slackApi', 'Notion'] }, + { suspend, resumeData: undefined } as never, + ); + + expect(suspend).not.toHaveBeenCalled(); + expect(result).toMatchObject({ + error: 'unknown_reopen_target', + unmatchedReopen: ['Notion'], + }); + // Nothing is un-skipped until the whole request is valid, so the retry is clean. + expect(context.revokeSessionToolApproval).not.toHaveBeenCalled(); + }); + + it('tells the caller to drop reopenSkipped when nothing is skipped', async () => { + (analyzeWorkflow as Mock).mockResolvedValue([slackRequest]); + const context = createGrantAwareContext(); + const suspend = vi.fn(); + + const tool = createWorkflowsTool(context, 'full'); + const result = await executeTool( + tool, + { action: 'setup', workflowId: 'wf1', reopenSkipped: ['Notion'] }, + { suspend, resumeData: undefined } as never, + ); + + expect(result).toMatchObject({ error: 'unknown_reopen_target', reopenable: [] }); + expect(result).toHaveProperty( + 'message', + expect.stringContaining('without `reopenSkipped`'), + ); + }); + + it('keeps a skipped parameter card from silencing that credential elsewhere', async () => { + // The Sheets credential works; the user passed on filling in the document id. A new + // node that genuinely needs the Sheets credential must still be asked about. + (analyzeWorkflow as Mock).mockResolvedValue([sheetsParamRequest]); + const context = createGrantAwareContext(); + + const tool = createWorkflowsTool(context, 'full'); + await executeTool(tool, { action: 'setup', workflowId: 'wf1' }, { + resumeData: { approved: false }, + } as never); + + expect(context.grantSessionToolApproval).toHaveBeenCalledWith( + 'workflows:setup-skip:node:wf1:Log to Sheet', + ); + expect(context.grantSessionToolApproval).not.toHaveBeenCalledWith( + 'workflows:setup-skip:cred:googleSheetsOAuth2Api', + ); + }); + + it('remembers everything still pending when the user skips the whole card', async () => { + (analyzeWorkflow as Mock).mockResolvedValue([slackRequest, sheetsRequest]); + const context = createGrantAwareContext(); + + const tool = createWorkflowsTool(context, 'full'); + const result = await executeTool(tool, { action: 'setup', workflowId: 'wf1' }, { + resumeData: { approved: false }, + } as never); + + expect(context.grantSessionToolApproval).toHaveBeenCalledWith( + 'workflows:setup-skip:cred:slackApi', + ); + expect(context.grantSessionToolApproval).toHaveBeenCalledWith( + 'workflows:setup-skip:cred:googleSheetsOAuth2Api', + ); + expect(result).toMatchObject({ success: true, deferred: true }); + }); + + it('separates a card the user skipped from one that is merely unconfigured', async () => { + // The Slack card was dismissed; the Sheets one was left half-filled. Reporting both + // as "still need configuration" is what made the agent re-open setup. + (analyzeWorkflow as Mock).mockResolvedValue([slackRequest, sheetsRequest]); + (applyNodeChanges as Mock).mockResolvedValue({ applied: [], failed: [] }); + (buildCompletedReport as Mock).mockReturnValue([]); + const context = createGrantAwareContext(); + + const tool = createWorkflowsTool(context, 'full'); + const result = await executeTool(tool, { action: 'setup', workflowId: 'wf1' }, { + resumeData: { + approved: true, + action: 'apply', + skippedNodes: ['Post to Slack'], + }, + } as never); + + expect(context.grantSessionToolApproval).toHaveBeenCalledWith( + 'workflows:setup-skip:cred:slackApi', + ); + expect(result).toMatchObject({ + partial: true, + nodesStillNeedingSetup: [{ nodeName: 'Log to Sheet' }], + skippedByUser: [{ nodeName: 'Post to Slack', credentialType: 'slackApi' }], + }); + }); + + it('forgets a skip once that credential is configured', async () => { + (analyzeWorkflow as Mock).mockResolvedValue([{ ...slackRequest, needsAction: false }]); + (applyNodeChanges as Mock).mockResolvedValue({ applied: ['Post to Slack'], failed: [] }); + (buildCompletedReport as Mock).mockReturnValue([ + { nodeName: 'Post to Slack', credentialType: 'slackApi' }, + ]); + const context = createGrantAwareContext(['workflows:setup-skip:cred:slackApi']); + + const tool = createWorkflowsTool(context, 'full'); + const result = await executeTool(tool, { action: 'setup', workflowId: 'wf1' }, { + resumeData: { + approved: true, + action: 'apply', + credentials: { 'Post to Slack': { slackApi: 'cred-1' } }, + }, + } as never); + + expect(context.revokeSessionToolApproval).toHaveBeenCalledWith( + 'workflows:setup-skip:cred:slackApi', + ); + expect(result).not.toHaveProperty('skippedByUser'); + }); + + it('keeps skipped cards out of the panel a trigger test rebuilds', async () => { + // The re-suspend re-derives the requests from scratch, so it has to partition again — + // otherwise testing a trigger mid-session puts back the card the user dismissed. + (analyzeWorkflow as Mock).mockResolvedValue([slackRequest, sheetsRequest]); + (applyNodeChanges as Mock).mockResolvedValue({ applied: [], failed: [] }); + const context = createGrantAwareContext(['workflows:setup-skip:cred:slackApi']); + (context.executionService.run as Mock).mockResolvedValue({ status: 'success' }); + const suspend = vi.fn(); + + const tool = createWorkflowsTool(context, 'full'); + await executeTool(tool, { action: 'setup', workflowId: 'wf1' }, { + suspend, + resumeData: { + approved: true, + action: 'test-trigger', + testTriggerNode: 'Log to Sheet', + }, + } as never); + + expect(suspend).toHaveBeenCalledTimes(1); + expect(suspend.mock.calls[0][0]).toMatchObject({ setupRequests: [sheetsRequest] }); + }); + + it('keeps another node Slack skip when only a parameter was completed', async () => { + // "Alert on Slack" was connected already and only needed a channel. Clearing the + // type-wide record here would re-open the card "Post to Slack" was skipped on. + const alertParamRequest = { + node: { name: 'Alert on Slack', type: 'n8n-nodes-base.slack' }, + credentialType: 'slackApi', + needsAction: false, + }; + (analyzeWorkflow as Mock).mockResolvedValue([slackRequest, alertParamRequest]); + (applyNodeChanges as Mock).mockResolvedValue({ applied: ['Alert on Slack'], failed: [] }); + (buildCompletedReport as Mock).mockReturnValue([ + { nodeName: 'Alert on Slack', parametersSet: ['channel'] }, + ]); + const context = createGrantAwareContext(['workflows:setup-skip:cred:slackApi']); + + const tool = createWorkflowsTool(context, 'full'); + await executeTool(tool, { action: 'setup', workflowId: 'wf1' }, { + resumeData: { + approved: true, + action: 'apply', + nodeParameters: { 'Alert on Slack': { channel: '#general' } }, + }, + } as never); + + expect(context.revokeSessionToolApproval).not.toHaveBeenCalledWith( + 'workflows:setup-skip:cred:slackApi', + ); + }); + }); }); describe('unpublish action', () => { diff --git a/packages/@n8n/instance-ai/src/tools/orchestration/__tests__/complete-checkpoint.tool.test.ts b/packages/@n8n/instance-ai/src/tools/orchestration/__tests__/complete-checkpoint.tool.test.ts index 7dcd7cfb70f..c83ea77648e 100644 --- a/packages/@n8n/instance-ai/src/tools/orchestration/__tests__/complete-checkpoint.tool.test.ts +++ b/packages/@n8n/instance-ai/src/tools/orchestration/__tests__/complete-checkpoint.tool.test.ts @@ -194,6 +194,44 @@ describe('createCompleteCheckpointTool', () => { }); }); + it('names the setup the user skipped when it lets the checkpoint through', async () => { + // The guard can't block on these, but completing silently would report the workflow as + // done with no mention that part of it can't run. + const service = makeService({ + getGraph: vi.fn().mockResolvedValue(makeSetupRequiredGraph()), + markCheckpointSucceeded: vi + .fn() + .mockResolvedValue({ ok: true, graph: { tasks: [], planRunId: 'r', status: 'active' } }), + }); + vi.mocked(analyzeWorkflow).mockResolvedValue([ + { + node: { name: 'Slack' } as SetupRequest['node'], + credentialType: 'slackApi', + needsAction: true, + credentialNeedsAction: true, + }, + ] as SetupRequest[]); + const tool = createCompleteCheckpointTool( + makeContext(service, { + domainContext: { + sessionApprovedToolKeys: new Set(['workflows:setup-skip:cred:slackApi']), + } as unknown as OrchestrationContext['domainContext'], + }), + ); + + const res = await executeTool(tool, { + taskId: 'verify-1', + status: 'succeeded', + result: 'Verified', + }); + + expect(res.ok).toBe(true); + expect(service.markCheckpointSucceeded).toHaveBeenCalled(); + expect(res.result).toContain('Slack (slackApi)'); + expect(res.result).toContain('skipped'); + expect(res.result).not.toContain('workflows(action="setup"'); + }); + it('marks a checkpoint failed via markCheckpointFailed', async () => { const service = makeService({ markCheckpointFailed: vi diff --git a/packages/@n8n/instance-ai/src/tools/orchestration/complete-checkpoint.tool.ts b/packages/@n8n/instance-ai/src/tools/orchestration/complete-checkpoint.tool.ts index 7646d634971..b5dabe00a1f 100644 --- a/packages/@n8n/instance-ai/src/tools/orchestration/complete-checkpoint.tool.ts +++ b/packages/@n8n/instance-ai/src/tools/orchestration/complete-checkpoint.tool.ts @@ -14,6 +14,10 @@ import { isRecord } from '@n8n/utils/is-record'; import { z } from 'zod'; import type { OrchestrationContext } from '../../types'; +import { + getSkippedSetupSubjects, + partitionSkippedSetupRequests, +} from '../workflows/setup-skip-state'; import { analyzeWorkflow } from '../workflows/setup-workflow.service'; const inputSchema = z.object({ @@ -57,7 +61,7 @@ function getChangedNodeNames(outcome: Record | undefined): stri async function rejectIfSetupStillRequired( context: OrchestrationContext, checkpointTaskId: string, -): Promise<{ ok: true } | { ok: false; result: string }> { +): Promise<{ ok: true; skippedNote?: string } | { ok: false; result: string }> { const graph = await context.plannedTaskService?.getGraph(context.threadId); if (!graph) return { ok: true }; @@ -89,14 +93,37 @@ async function rejectIfSetupStillRequired( }; } + const skippedNotes: string[] = []; + for (const { workflowId, changedNodeNames } of dependentWorkflows) { try { const setupRequests = await analyzeWorkflow(domainContext, workflowId); - const pendingRequests = setupRequests.filter( - (request) => - request.needsAction && - (!changedNodeNames || changedNodeNames.includes(request.node.name)), + // Credentials the user skipped can never be satisfied by another setup call, so + // counting them here would deadlock the checkpoint against a card that must not reopen. + const { pending, skippedByUser } = partitionSkippedSetupRequests( + setupRequests, + workflowId, + getSkippedSetupSubjects(domainContext), ); + // Nodes this build never touched don't gate the checkpoint either, and are not worth + // reporting: the user was never asked about them. + const blocks = (request: (typeof setupRequests)[number]) => + request.needsAction === true && + (!changedNodeNames || changedNodeNames.includes(request.node.name)); + const pendingRequests = pending.filter(blocks); + // Passing the guard silently would let the checkpoint report a workflow as done with + // no mention that parts of it can't run — the one thing the user needs to hear. + const skipped = skippedByUser.filter(blocks); + if (skipped.length > 0) { + const described = skipped + .map((request) => + request.credentialType + ? `${request.node.name} (${request.credentialType})` + : request.node.name, + ) + .join(', '); + skippedNotes.push(`workflow "${workflowId}": ${described}`); + } if (pendingRequests.length > 0) { const nodeNames = pendingRequests .map((request) => request.node.name) @@ -121,6 +148,16 @@ async function rejectIfSetupStillRequired( } } + if (skippedNotes.length > 0) { + return { + ok: true, + skippedNote: + `Setup the user skipped earlier is still outstanding — ${skippedNotes.join('; ')}. ` + + 'Do not re-open the setup card. Say in your summary which parts stay unconfigured and ' + + 'what that means when the workflow runs.', + }; + } + return { ok: true }; } @@ -139,9 +176,11 @@ export function createCompleteCheckpointTool(context: OrchestrationContext) { return { ok: false, result: 'Error: planned task service not available.' }; } + let skippedNote: string | undefined; if (input.status === 'succeeded') { const setupGuard = await rejectIfSetupStillRequired(context, input.taskId); if (!setupGuard.ok) return setupGuard; + skippedNote = setupGuard.skippedNote; } const settleResult = @@ -165,7 +204,9 @@ export function createCompleteCheckpointTool(context: OrchestrationContext) { if (settleResult.ok) { return { ok: true, - result: `Checkpoint ${input.taskId} marked ${input.status}.`, + result: + `Checkpoint ${input.taskId} marked ${input.status}.` + + (skippedNote ? ` ${skippedNote}` : ''), }; } diff --git a/packages/@n8n/instance-ai/src/tools/workflows.tool.ts b/packages/@n8n/instance-ai/src/tools/workflows.tool.ts index 7d445d04aa2..b442193eb24 100644 --- a/packages/@n8n/instance-ai/src/tools/workflows.tool.ts +++ b/packages/@n8n/instance-ai/src/tools/workflows.tool.ts @@ -24,7 +24,19 @@ import { rememberCurrentWorkflowChecksum, rememberObservedWorkflowChecksum, } from './workflows/observed-workflow-checksums'; +import { + completedSetupSubjects, + describeSkippedSetup, + forgetSkippedSetup, + getSkippedSetupSubjects, + partitionSkippedSetupRequests, + rememberSkippedSetup, + resolveReopenTargets, + setupSkipSubject, + SKIPPED_SETUP_GUIDANCE, +} from './workflows/setup-skip-state'; import { setupSuspendSchema, setupResumeSchema } from './workflows/setup-workflow.schema'; +import type { SetupRequest } from './workflows/setup-workflow.schema'; import { analyzeWorkflow, applyCredentialHints, @@ -143,11 +155,17 @@ const setupAction = z.object({ .describe( 'Set ONLY when the user explicitly chose a plain generic auth type (Bearer/Header/Query/Custom Auth) for a new credential, or the workflow pre-existed with it. Otherwise setup rejects new plain generic credentials on HTTP Request nodes in favor of Simplified Custom Auth.', ), + reopenSkipped: z + .array(z.string()) + .optional() + .describe( + 'Credential types (or node names) the user has just explicitly asked to configure after skipping them earlier — e.g. ["slackApi"] for "connect Slack now". Use the `reopenWith` value setup reported for that card. Anything the user skipped and did not ask about stays out of the card; without this, setup reports skipped credentials instead of re-opening them. An entry matching nothing in the workflow comes back as `unknown_reopen_target` with the list to choose from.', + ), includeAllNodes: z .boolean() .optional() .describe( - 'By default, setup after a build covers only the nodes that build changed. Set to true to cover every node in the workflow — ONLY when the user explicitly asked to set up the whole workflow or a node the last build did not touch.', + 'By default, setup after a build covers only the nodes that build changed. Set to true to cover every node in the workflow — ONLY when the user explicitly asked to set up the whole workflow or a node the last build did not touch. Cards the user skipped stay out unless named in `reopenSkipped`.', ), }); @@ -663,7 +681,15 @@ async function handleSetupTestTrigger( const refreshedRequests = await analyzeWorkflow(context, input.workflowId, { [testTriggerNode]: triggerTestResult, }); - applyCredentialHints(refreshedRequests, input.credentialHints); + // 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. + const { pending: refreshedPending } = partitionSkippedSetupRequests( + refreshedRequests, + input.workflowId, + getSkippedSetupSubjects(context), + ); + applyCredentialHints(refreshedPending, input.credentialHints); // Generate a new requestId so the frontend doesn't filter it // as already-resolved from the previous suspend cycle @@ -678,12 +704,47 @@ async function handleSetupTestTrigger( requestId: state.currentRequestId, message: 'Configure credentials for your workflow', severity: 'info' as const, - setupRequests: refreshedRequests, + setupRequests: refreshedPending, workflowId: input.workflowId, ...(projectId ? { projectId } : {}), }); } +/** + * Fold the panel's skip decisions into the thread's skip memory and return the requests + * that are now suppressed. Anything just configured wins over a skip of the same subject: + * two nodes can share a credential type, and a configured credential isn't a declined one. + */ +async function reconcileSetupSkips( + context: InstanceAiContext, + args: { + workflowId: string; + requests: readonly SetupRequest[]; + skippedNodeNames: readonly string[]; + /** The applied report — `credentialType` is set only where a credential was applied. */ + completed: ReadonlyArray<{ nodeName: string; credentialType?: string }>; + }, +): Promise { + const byNodeName = new Map(args.requests.map((request) => [request.node.name, request])); + + const completedSubjects = new Set(completedSetupSubjects(args.completed, args.workflowId)); + await forgetSkippedSetup(context, completedSubjects); + + const newlySkipped = args.skippedNodeNames + .map((name) => byNodeName.get(name)) + .filter((request): request is SetupRequest => request !== undefined) + .filter( + (request) => + request.needsAction && !completedSubjects.has(setupSkipSubject(request, args.workflowId)), + ); + await rememberSkippedSetup(context, newlySkipped, args.workflowId); + + const skipped = getSkippedSetupSubjects(context); + return args.requests.filter( + (request) => request.needsAction && skipped.has(setupSkipSubject(request, args.workflowId)), + ); +} + /** Setup state 4: apply credentials and parameters atomically and report the outcome. */ async function handleSetupApply( context: InstanceAiContext, @@ -725,13 +786,29 @@ async function handleSetupApply( const remainingRequests = await analyzeWorkflow(context, input.workflowId, undefined, { includeSettled: true, }); - const pendingRequests = remainingRequests.filter((r) => r.needsAction); const completedNodes = buildCompletedReport( resumeData.credentials, resumeData.nodeParameters, applyResult.applied, ); + // The user dismissing a card and a card merely being unconfigured look identical in + // the re-analysis, so the panel tells us which ones were dismissed. Record those for + // the rest of the thread, and drop the record for anything just configured — a + // credential type that now has a working credential is no longer a declined decision. + const skippedByUser = await reconcileSetupSkips(context, { + workflowId: input.workflowId, + requests: remainingRequests, + skippedNodeNames: resumeData.skippedNodes ?? [], + completed: completedNodes, + }); + const skippedSubjects = new Set( + skippedByUser.map((request) => setupSkipSubject(request, input.workflowId)), + ); + const pendingRequests = remainingRequests.filter( + (r) => r.needsAction && !skippedSubjects.has(setupSkipSubject(r, input.workflowId)), + ); + // Detect credentials that were applied but failed testing. const credTestFailures = collectCredentialTestFailures( remainingRequests, @@ -743,12 +820,22 @@ async function handleSetupApply( const allFailedNodes = [...(failedNodes ?? []), ...credTestFailures]; const mergedFailedNodes = allFailedNodes.length > 0 ? allFailedNodes : undefined; + // Reported separately from the nodes that still need setup: these must not be + // re-opened, so folding them into one list is what made the agent ask again. + const skippedByUserReport = + skippedByUser.length > 0 + ? { + skippedByUser: describeSkippedSetup(skippedByUser), + skippedByUserGuidance: SKIPPED_SETUP_GUIDANCE, + } + : {}; + if (pendingRequests.length > 0) { // Carry the parameter issues, not just the node name: a value the connected // credential can't reach (e.g. a model outside the free-credits allowlist) is // only actionable if the caller learns which value was wrong, so it can replace // it and say what it changed. - const skippedNodes = pendingRequests.map((r) => ({ + const nodesStillNeedingSetup = pendingRequests.map((r) => ({ nodeName: r.node.name, credentialType: r.credentialType, ...(r.parameterIssues && Object.keys(r.parameterIssues).length > 0 @@ -760,7 +847,8 @@ async function handleSetupApply( partial: true, reason: `Applied setup for ${String(validCompletedNodes.length)} node(s), ${String(pendingRequests.length)} node(s) still need configuration.`, completedNodes: validCompletedNodes, - skippedNodes, + nodesStillNeedingSetup, + ...skippedByUserReport, failedNodes: mergedFailedNodes, updatedNodes, updatedConnections, @@ -770,6 +858,7 @@ async function handleSetupApply( return { success: true, completedNodes: validCompletedNodes, + ...skippedByUserReport, failedNodes: mergedFailedNodes, updatedNodes, updatedConnections, @@ -831,17 +920,67 @@ async function handleSetup( if (resumeData === undefined || resumeData === null) { const allSetupRequests = await analyzeWorkflow(context, input.workflowId); + // The user asked to come back to something they skipped, so that decision no longer + // holds — drop it before partitioning so the card renders again. Scoped to what they + // named: anything else they skipped stays skipped. + // + // Validated as a whole before anything is forgotten or opened, like the credential-hint + // and plain-auth checks below. Honouring the entries that resolve and opening the card + // anyway would drop the rest of what the user asked for with nothing to report it: the + // card suspends, so this is the last point where the caller can still be told. + if (input.reopenSkipped && input.reopenSkipped.length > 0) { + // Matched against every analyzed node, not the build-scoped subset: the user named + // this card, so a node the last build happened not to touch is still a valid target + // and must not come back as "nothing matches". + const { subjects, unmatched } = resolveReopenTargets( + allSetupRequests, + input.workflowId, + input.reopenSkipped, + ); + if (unmatched.length > 0) { + const reopenable = describeSkippedSetup( + partitionSkippedSetupRequests( + allSetupRequests, + input.workflowId, + getSkippedSetupSubjects(context), + ).skippedByUser, + ); + const named = unmatched.map((entry) => `"${entry}"`).join(', '); + return { + error: 'unknown_reopen_target', + message: + reopenable.length > 0 + ? `Nothing in this workflow matches ${named}. Call setup again passing only \`reopenWith\` values from \`reopenable\`, and tell the user that what they named is not part of this workflow.` + : `Nothing in this workflow matches ${named}, and nothing in it is currently skipped. Call setup again without \`reopenSkipped\`.`, + unmatchedReopen: unmatched, + reopenable, + }; + } + await forgetSkippedSetup(context, subjects); + } + // 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 setupRequests = scopeNodeNames + const scopedRequests = scopeNodeNames ? allSetupRequests.filter((request) => scopeNodeNames.includes(request.node.name)) : 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 — + // an out-of-scope card is not something the user passed on. + const { pending: setupRequests, skippedByUser } = partitionSkippedSetupRequests( + scopedRequests, + input.workflowId, + getSkippedSetupSubjects(context), + ); + // Validated against the workflow's node URLs so a recipe can't set one of - // the workflow's own (action) endpoints as its probe testUrl. + // the workflow's own (action) endpoints as its probe testUrl. Checked against every + // analyzed node, not just the pending ones — narrowing it to what the card shows would + // let a skipped or out-of-scope node's endpoint through. const nodeUrls = allSetupRequests.map((request) => request.node.parameters?.url); const hintProblems = (input.credentialHints ?? []).flatMap((hint) => findSetupHintProblems(hint, { nodeUrls }).map((problem) => @@ -883,19 +1022,37 @@ async function handleSetup( } if (setupRequests.length === 0) { - const skippedNodeNames = scopeNodeNames + // Two different silences, and the agent has to say different things about them: cards + // 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)) : []; - if (skippedNodeNames.length > 0) { + const outOfScopeReason = + outOfScopeNodeNames.length > 0 + ? `Pre-existing node(s) ${outOfScopeNodeNames + .map((name) => `"${name}"`) + .join(', ')} have pending setup, but this change did not touch them — ` + + 'do not route the user to set them up now. Only if the user explicitly asks to set them up, call setup again with includeAllNodes: true.' + : undefined; + + if (skippedByUser.length > 0) { return { success: true, reason: - `No nodes changed by the latest build require setup. Pre-existing node(s) ${skippedNodeNames - .map((name) => `"${name}"`) - .join(', ')} have pending setup, but this change did not touch them — ` + - 'do not route the user to set them up now. Only if the user explicitly asks to set them up, call setup again with includeAllNodes: true.', + 'The only nodes that need setup are ones the user already skipped.' + + (outOfScopeReason ? ` ${outOfScopeReason}` : ''), + skippedByUser: describeSkippedSetup(skippedByUser), + skippedByUserGuidance: SKIPPED_SETUP_GUIDANCE, + }; + } + if (outOfScopeReason) { + return { + success: true, + reason: `No nodes changed by the latest build require setup. ${outOfScopeReason}`, }; } return { success: true, reason: 'No nodes require setup.' }; @@ -925,10 +1082,23 @@ async function handleSetup( await refreshWorkflowSourceFileBindingFromWorkflow(context, input.workflowId); state.preTestSnapshot = null; } + // 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, + ); + await rememberSkippedSetup(context, dismissed, input.workflowId); return { success: true, deferred: true, reason: 'User skipped workflow setup for now.', + ...(dismissed.length > 0 + ? { + skippedByUser: describeSkippedSetup(dismissed), + skippedByUserGuidance: SKIPPED_SETUP_GUIDANCE, + } + : {}), }; } diff --git a/packages/@n8n/instance-ai/src/tools/workflows/__tests__/setup-skip-state.test.ts b/packages/@n8n/instance-ai/src/tools/workflows/__tests__/setup-skip-state.test.ts new file mode 100644 index 00000000000..e88c6c56558 --- /dev/null +++ b/packages/@n8n/instance-ai/src/tools/workflows/__tests__/setup-skip-state.test.ts @@ -0,0 +1,250 @@ +import type { InstanceAiContext } from '../../../types'; +import { + completedSetupSubjects, + describeSkippedSetup, + forgetSkippedSetup, + getSkippedSetupSubjects, + partitionSkippedSetupRequests, + rememberSkippedSetup, + resolveReopenTargets, + setupSkipSubject, +} from '../setup-skip-state'; +import type { SetupRequest } from '../setup-workflow.schema'; + +const WF = 'wf-1'; + +function makeRequest(overrides: Partial & { name: string }): SetupRequest { + const { name, ...rest } = overrides; + return { + node: { name, type: 'n8n-nodes-base.slack' }, + needsAction: true, + isTrigger: false, + ...rest, + } as SetupRequest; +} + +/** A card asking for the credential itself. */ +function credentialRequest(name: string, credentialType: string): SetupRequest { + return makeRequest({ name, credentialType, credentialNeedsAction: true }); +} + +/** A card whose credential is connected and which only needs a parameter filled in. */ +function parameterRequest(name: string, credentialType?: string): SetupRequest { + return makeRequest({ + name, + ...(credentialType ? { credentialType } : {}), + parameterIssues: { + documentId: ['Placeholder "SPREADSHEET_ID" — please provide the real value'], + }, + }); +} + +/** Mirrors the service wiring: one mutable set, read and written through the context. */ +function createContext(granted: string[] = []) { + const sessionApprovedToolKeys = new Set(granted); + return { + sessionApprovedToolKeys, + grantSessionToolApproval: async (key: string) => { + await Promise.resolve(); + sessionApprovedToolKeys.add(key); + }, + revokeSessionToolApproval: async (key: string) => { + await Promise.resolve(); + sessionApprovedToolKeys.delete(key); + }, + } as unknown as InstanceAiContext; +} + +describe('setupSkipSubject', () => { + it('keys a credential card off the credential type so sibling nodes stay quiet too', () => { + expect(setupSkipSubject(credentialRequest('Post to Slack', 'slackApi'), WF)).toBe( + 'cred:slackApi', + ); + }); + + it('keys a parameter-only card off the node, even when the node has a credential type', () => { + // The user declined to fill in one field, not to use Google Sheets. + expect(setupSkipSubject(parameterRequest('Log to Sheet', 'googleSheetsOAuth2Api'), WF)).toBe( + 'node:wf-1:Log to Sheet', + ); + }); + + it('scopes a node-keyed skip to its workflow', () => { + const request = parameterRequest('HTTP Request'); + + expect(setupSkipSubject(request, 'wf-1')).not.toBe(setupSkipSubject(request, 'wf-2')); + }); +}); + +describe('skip bookkeeping', () => { + it('round-trips a skip through the grant store', async () => { + const context = createContext(); + const slack = credentialRequest('Post to Slack', 'slackApi'); + + await rememberSkippedSetup(context, [slack], WF); + expect(getSkippedSetupSubjects(context).has('cred:slackApi')).toBe(true); + + await forgetSkippedSetup(context, ['cred:slackApi']); + expect(getSkippedSetupSubjects(context).has('cred:slackApi')).toBe(false); + }); + + it('ignores unrelated grant keys', () => { + const context = createContext(['executions:run:wf-1', 'fetch-url:example.com']); + + expect(getSkippedSetupSubjects(context).size).toBe(0); + }); + + it('suppresses every node sharing a skipped credential type', () => { + const requests = [ + credentialRequest('Post to Slack', 'slackApi'), + credentialRequest('Alert on Slack', 'slackApi'), + credentialRequest('Log to Sheet', 'googleSheetsOAuth2Api'), + ]; + + const { pending, skippedByUser } = partitionSkippedSetupRequests( + requests, + WF, + new Set(['cred:slackApi']), + ); + + expect(pending.map((r) => r.node.name)).toEqual(['Log to Sheet']); + expect(skippedByUser.map((r) => r.node.name)).toEqual(['Post to Slack', 'Alert on Slack']); + }); + + it('does not let a skipped parameter card silence a node needing that credential', async () => { + const context = createContext(); + // Sheets is connected; the user passed on filling in the document id. + await rememberSkippedSetup( + context, + [parameterRequest('Log to Sheet', 'googleSheetsOAuth2Api')], + WF, + ); + + const { pending } = partitionSkippedSetupRequests( + [credentialRequest('Read another Sheet', 'googleSheetsOAuth2Api')], + WF, + getSkippedSetupSubjects(context), + ); + + expect(pending.map((r) => r.node.name)).toEqual(['Read another Sheet']); + }); + + it('does not let a skip leak into another workflow in the same thread', async () => { + const context = createContext(); + await rememberSkippedSetup(context, [parameterRequest('HTTP Request')], 'wf-1'); + + const { pending } = partitionSkippedSetupRequests( + [parameterRequest('HTTP Request')], + 'wf-2', + getSkippedSetupSubjects(context), + ); + + expect(pending.map((r) => r.node.name)).toEqual(['HTTP Request']); + }); + + it('is a no-op in contexts without grant persistence', async () => { + const context = {} as InstanceAiContext; + + await expect( + rememberSkippedSetup(context, [credentialRequest('Post to Slack', 'slackApi')], WF), + ).resolves.toBeUndefined(); + expect(getSkippedSetupSubjects(context).size).toBe(0); + }); +}); + +describe('completedSetupSubjects', () => { + it('clears the credential skip even though configuring it re-keys the card', async () => { + const context = createContext(); + await rememberSkippedSetup(context, [credentialRequest('Post to Slack', 'slackApi')], WF); + + // Post-apply the credential resolves, so the same card re-analyses as a parameter card — + // the subject it was recorded under is no longer the one it computes. Keying off what was + // applied instead of off the re-analysed state is what makes this land. + await forgetSkippedSetup( + context, + completedSetupSubjects([{ nodeName: 'Post to Slack', credentialType: 'slackApi' }], WF), + ); + + expect(getSkippedSetupSubjects(context).size).toBe(0); + }); + + it('keeps a type-wide skip when only a parameter was completed', async () => { + // "Alert on Slack" has a working credential and only needed a channel; that says nothing + // about the Slack credential "Post to Slack" is still waiting on. + const context = createContext(['workflows:setup-skip:cred:slackApi']); + + await forgetSkippedSetup(context, completedSetupSubjects([{ nodeName: 'Alert on Slack' }], WF)); + + expect(getSkippedSetupSubjects(context).has('cred:slackApi')).toBe(true); + }); + + it('clears the completed node own record', async () => { + const context = createContext(['workflows:setup-skip:node:wf-1:Log to Sheet']); + + await forgetSkippedSetup(context, completedSetupSubjects([{ nodeName: 'Log to Sheet' }], WF)); + + expect(getSkippedSetupSubjects(context).size).toBe(0); + }); +}); + +describe('resolveReopenTargets', () => { + const requests = [ + credentialRequest('Post to Slack', 'slackApi'), + parameterRequest('Log to Sheet', 'googleSheetsOAuth2Api'), + ]; + + it('matches a credential type case-insensitively', () => { + const { subjects, unmatched } = resolveReopenTargets(requests, WF, ['SlackApi']); + + expect(subjects).toContain('cred:slackApi'); + expect(unmatched).toEqual([]); + }); + + it('matches a node name', () => { + const { subjects, unmatched } = resolveReopenTargets(requests, WF, ['Log to Sheet']); + + expect(subjects).toContain('node:wf-1:Log to Sheet'); + expect(unmatched).toEqual([]); + }); + + it('drops the type-wide record when the user names a credential type', () => { + // Also covers a card re-keyed since the skip — a credential attached outside the panel + // would otherwise leave the original `cred:` record hiding it. + const { subjects } = resolveReopenTargets(requests, WF, ['slackApi']); + + expect(subjects).toContain('cred:slackApi'); + }); + + it('touches only the named node when the user names a node', () => { + // Reopening one node must not drag another node's declined credential back into the card. + const { subjects } = resolveReopenTargets(requests, WF, ['Log to Sheet']); + + expect(subjects).toEqual(['node:wf-1:Log to Sheet']); + }); + + it('reports what it could not match instead of silently leaving the card closed', () => { + const { subjects, unmatched } = resolveReopenTargets(requests, WF, ['Notion']); + + expect(subjects).toEqual([]); + expect(unmatched).toEqual(['Notion']); + }); +}); + +describe('describeSkippedSetup', () => { + it('names the value the caller passes back to reopen each card', () => { + expect( + describeSkippedSetup([ + credentialRequest('Post to Slack', 'slackApi'), + parameterRequest('Log to Sheet', 'googleSheetsOAuth2Api'), + ]), + ).toEqual([ + { nodeName: 'Post to Slack', credentialType: 'slackApi', reopenWith: 'slackApi' }, + { + nodeName: 'Log to Sheet', + credentialType: 'googleSheetsOAuth2Api', + // Not the credential type: reopening this asks for the parameter on this node. + reopenWith: 'Log to Sheet', + }, + ]); + }); +}); diff --git a/packages/@n8n/instance-ai/src/tools/workflows/__tests__/setup-workflow.service.test.ts b/packages/@n8n/instance-ai/src/tools/workflows/__tests__/setup-workflow.service.test.ts index a05e64b8138..e3effcf15f8 100644 --- a/packages/@n8n/instance-ai/src/tools/workflows/__tests__/setup-workflow.service.test.ts +++ b/packages/@n8n/instance-ai/src/tools/workflows/__tests__/setup-workflow.service.test.ts @@ -482,6 +482,28 @@ describe('buildSetupRequests', () => { expect(result[0].parameterIssues).toBeDefined(); }); + it('reports credentialNeedsAction only for the credential slot itself', async () => { + // A connected credential with an unfilled parameter needs action, but not about the + // service — skipping that card must not be read as "I don't want this credential". + (context.credentialService.list as Mock).mockResolvedValue([ + { id: 'cred-1', name: 'My Slack', updatedAt: '2025-01-01T00:00:00.000Z' }, + ]); + (context.credentialService.test as Mock).mockResolvedValue({ success: true }); + (context.nodeService as unknown as Record).getParameterIssues = vi + .fn() + .mockResolvedValue({ resource: ['Parameter "resource" is required'] }); + + const withCredential = await buildSetupRequests( + context, + makeNode({ credentials: { slackApi: { id: 'cred-1', name: 'My Slack' } } }), + ); + expect(withCredential[0].needsAction).toBe(true); + expect(withCredential[0].credentialNeedsAction).toBeFalsy(); + + const withoutCredential = await buildSetupRequests(context, makeNode()); + expect(withoutCredential[0].credentialNeedsAction).toBe(true); + }); + it('auto-applies the only credential when node has none', async () => { (context.credentialService.list as Mock).mockResolvedValue([ { id: 'cred-1', name: 'My Slack', updatedAt: '2025-01-01T00:00:00.000Z' }, diff --git a/packages/@n8n/instance-ai/src/tools/workflows/__tests__/workflow-build-routing.test.ts b/packages/@n8n/instance-ai/src/tools/workflows/__tests__/workflow-build-routing.test.ts index 0f0ae3077e2..cdb2296c54c 100644 --- a/packages/@n8n/instance-ai/src/tools/workflows/__tests__/workflow-build-routing.test.ts +++ b/packages/@n8n/instance-ai/src/tools/workflows/__tests__/workflow-build-routing.test.ts @@ -163,4 +163,38 @@ describe('withDeterministicRouting', () => { }); expect('workflowNeedsSetup' in outcome).toBe(false); }); + + it('does not route setup when the only pending credentials were skipped by the user', () => { + // Mocked credentials and placeholders both normally force setup — a skipped credential + // still produces both, so the gate has to sit in front of them. + const outcome = withDeterministicRouting({ + ...makeOutcome({ + mockedNodeNames: ['Post to Slack'], + mockedCredentialTypes: ['slackApi'], + mockedCredentialsByNode: { 'Post to Slack': ['slackApi'] }, + hasUnresolvedPlaceholders: true, + }), + onlySkippedSetupRemains: true, + }); + + expect(outcome.setupRequirement).toMatchObject({ + status: 'not_required', + reason: 'skipped-by-user', + }); + expect(outcome.verificationReadiness).toEqual({ status: 'ready' }); + expect('onlySkippedSetupRemains' in outcome).toBe(false); + }); + + it('still routes setup when a non-skipped credential is pending too', () => { + const outcome = withDeterministicRouting({ + ...makeOutcome({ + mockedNodeNames: ['Post to Slack', 'Log to Sheet'], + mockedCredentialTypes: ['slackApi', 'googleSheetsOAuth2Api'], + }), + workflowNeedsSetup: true, + onlySkippedSetupRemains: false, + }); + + expect(outcome.setupRequirement).toMatchObject({ status: 'required' }); + }); }); 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 bb34ffa9fc2..395be333d15 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 @@ -18,6 +18,7 @@ import { resolveCredentials, } from './resolve-credentials'; import { resolvedCredentialSchema } from './resolved-credential.schema'; +import { getSkippedSetupSubjects, partitionSkippedSetupRequests } from './setup-skip-state'; import { analyzeWorkflow, stripStaleCredentialsFromWorkflow } from './setup-workflow.service'; import { combineWarnings, @@ -195,7 +196,11 @@ const triggerNodeOutputSchema = z.object({ const verificationReadinessOutputSchema = workflowVerificationReadinessSchema; const setupRequirementOutputSchema = z.discriminatedUnion('status', [ - z.object({ status: z.literal('not_required') }), + z.object({ + status: z.literal('not_required'), + reason: z.literal('skipped-by-user').optional(), + guidance: z.string().optional(), + }), z.object({ status: z.literal('required'), reason: z.enum(['mocked-credentials', 'unresolved-placeholders', 'workflow-needs-setup']), @@ -941,9 +946,22 @@ export function createBuildWorkflowTool(context: InstanceAiContext) { operation: 'create' | 'update', ) => { const setupRequests = await analyzeWorkflow(context, saved.id); - const workflowNeedsSetup = setupRequests.some( - (request) => request.needsAction && isInSetupScope(request.node.name), - ); + // Two independent filters over the same list: `isInSetupScope` drops nodes this + // build never touched, the skip partition drops cards the user declined. A node + // only re-arms the setup follow-up when it survives both. + const { pending: pendingSetupRequests, skippedByUser: skippedSetupRequests } = + partitionSkippedSetupRequests( + setupRequests, + saved.id, + getSkippedSetupSubjects(context), + ); + const needsSetupInScope = (request: (typeof setupRequests)[number]) => + request.needsAction === true && isInSetupScope(request.node.name); + const workflowNeedsSetup = pendingSetupRequests.some(needsSetupInScope); + // Only the user's skip explains the silence — an out-of-scope node is not + // something they declined, and has its own reporting on the setup path. + const onlySkippedSetupRemains = + !workflowNeedsSetup && skippedSetupRequests.some(needsSetupInScope); const { nodeSimulationPlan, simulationFixtures, waitGateScripts } = await planVerificationSimulation({ workflow: json, @@ -1023,6 +1041,7 @@ export function createBuildWorkflowTool(context: InstanceAiContext) { ? mockResult.resolvedCredentialsByNode : undefined, workflowNeedsSetup, + onlySkippedSetupRemains, nodeSimulationPlan, simulationFixtures, waitGateScripts, diff --git a/packages/@n8n/instance-ai/src/tools/workflows/setup-skip-state.ts b/packages/@n8n/instance-ai/src/tools/workflows/setup-skip-state.ts new file mode 100644 index 00000000000..1669778cc92 --- /dev/null +++ b/packages/@n8n/instance-ai/src/tools/workflows/setup-skip-state.ts @@ -0,0 +1,204 @@ +/** + * Thread-scoped memory of the setup cards the user passed on. + * + * Skipping is a user decision, so it has to outlive the panel that collected it: the + * blocking setup card is opened both by the agent and by the deterministic + * `` routing, and the latter re-arms on every build. Without a + * record, a skipped credential is indistinguishable from one that is merely still + * unconfigured, and the next build re-opens the card the user just dismissed. + * + * Records live on the same per-thread, per-user store as the "always allow" grants, so they + * survive reload and are visible across mains. + */ +import { buildSetupSkipGrantKey, parseSetupSkipGrants } from '@n8n/api-types'; + +import type { SetupRequest } from './setup-workflow.schema'; +import type { InstanceAiContext } from '../../types'; + +/** + * Whether the card is asking for the credential itself rather than for a parameter. A node + * can need action with a perfectly good credential attached — an unfilled `documentId`, a + * placeholder left in a parameter — and that card says nothing about the service. + */ +function isCredentialSkip( + request: SetupRequest, +): request is SetupRequest & { credentialType: string } { + return request.credentialType !== undefined && request.credentialNeedsAction === true; +} + +/** + * A skip of a *credential* card generalises to its credential type and to the whole thread: + * declining to connect Slack should quiet every Slack node, in this workflow and the next. + * + * A skip of a *parameter* card can't be generalised that way — the user declined to fill one + * field on one node, not to use the service — so it's keyed by node, scoped to the workflow. + * Node names are only unique within a workflow, and a thread can build several. + */ +export function setupSkipSubject(request: SetupRequest, workflowId: string): string { + return isCredentialSkip(request) + ? `cred:${request.credentialType}` + : nodeSubject(request.node.name, workflowId); +} + +/** + * The string a caller passes to `reopenSkipped` to bring this card back — the credential type + * for a credential card, the node name for a parameter one. Kept out of the grant key on + * purpose: the key needs to be unambiguous, this needs to be what the user would say. + */ +export function setupSkipReopenToken(request: SetupRequest): string { + return isCredentialSkip(request) ? request.credentialType : request.node.name; +} + +function nodeSubject(nodeName: string, workflowId: string): string { + return `node:${workflowId}:${nodeName}`; +} + +/** Skip subjects recorded earlier in this thread. */ +export function getSkippedSetupSubjects(context: InstanceAiContext): ReadonlySet { + return parseSetupSkipGrants(context.sessionApprovedToolKeys ?? new Set()); +} + +export function isSetupRequestSkipped( + request: SetupRequest, + workflowId: string, + skipped: ReadonlySet, +): boolean { + return skipped.has(setupSkipSubject(request, workflowId)); +} + +/** Remember the given requests as skipped for the rest of the thread. */ +export async function rememberSkippedSetup( + context: InstanceAiContext, + requests: readonly SetupRequest[], + workflowId: string, +): Promise { + const subjects = new Set(requests.map((request) => setupSkipSubject(request, workflowId))); + for (const subject of subjects) { + await context.grantSessionToolApproval?.(buildSetupSkipGrantKey(subject)); + } +} + +/** + * Forget skips for the given subjects — a credential that just got configured, or one the + * user explicitly asked to come back to, is no longer a declined decision. + */ +export async function forgetSkippedSetup( + context: InstanceAiContext, + subjects: Iterable, +): Promise { + for (const subject of new Set(subjects)) { + await context.revokeSessionToolApproval?.(buildSetupSkipGrantKey(subject)); + } +} + +export interface ResolvedReopenTargets { + /** Skip subjects to forget. */ + subjects: string[]; + /** Entries that named nothing in this workflow — reported rather than silently dropped. */ + unmatched: string[]; +} + +/** + * Map what the caller named (`["slackApi"]`, `["Post to Slack"]`) onto skip subjects. + * Accepts either spelling and ignores case, because the model is relaying the user's words. + * + * Anything that matches nothing comes back in `unmatched`: a near-miss that silently left the + * card closed would strand the user, who has just asked for it in so many words, behind + * guidance telling the agent not to re-open it. + */ +export function resolveReopenTargets( + requests: readonly SetupRequest[], + workflowId: string, + requested: readonly string[], +): ResolvedReopenTargets { + const subjects = new Set(); + const unmatched: string[] = []; + for (const entry of requested) { + const wanted = entry.toLowerCase(); + const byCredentialType = requests.filter( + (request) => request.credentialType?.toLowerCase() === wanted, + ); + const byNodeName = requests.filter((request) => request.node.name.toLowerCase() === wanted); + const matches = byCredentialType.length > 0 ? byCredentialType : byNodeName; + if (matches.length === 0) { + unmatched.push(entry); + continue; + } + // Clear what the user named, at the granularity they named it. "Connect Slack" also drops + // the type-wide record — otherwise a card re-keyed since the skip (a credential attached + // outside the panel) would stay hidden. Naming one node touches only that node, so it + // can't drag another node's declined credential back into the card. + // + // Naming the type deliberately reaches the parameter cards on nodes using it, not just the + // credential ones: `cred:` is coarse by design (that is what quiets sibling nodes), and the + // user asking to set up Slack is asking about Slack. Restricting this to credential cards + // would make "set up Slack" fail outright whenever the only Slack card left is a parameter + // one, and split one request into two asks. + if (byCredentialType.length > 0) { + subjects.add(`cred:${byCredentialType[0].credentialType}`); + } + for (const match of matches) subjects.add(setupSkipSubject(match, workflowId)); + } + return { subjects: [...subjects], unmatched }; +} + +/** + * Subjects to clear for the cards the user just finished configuring, keyed off what was + * actually applied rather than off the node's re-analysed state. + * + * `credentialType` is set only when a credential of that type was applied to that node, so + * that is the one case where a type-wide `cred:` record stops being a declined decision. + * A node that only completed a *parameter* clears its own record and nothing else — its + * credential was already connected, so it says nothing about a credential another node is + * still waiting on. Reading the post-apply state instead would conflate the two, because + * attaching a credential is exactly what re-keys a card from `cred:` to `node:`. + */ +export function completedSetupSubjects( + completed: ReadonlyArray<{ nodeName: string; credentialType?: string }>, + workflowId: string, +): string[] { + const subjects = new Set(); + for (const entry of completed) { + subjects.add(nodeSubject(entry.nodeName, workflowId)); + if (entry.credentialType) subjects.add(`cred:${entry.credentialType}`); + } + return [...subjects]; +} + +export interface PartitionedSetupRequests { + /** Requests to put in front of the user. */ + pending: SetupRequest[]; + /** Requests suppressed because the user already skipped them. */ + skippedByUser: SetupRequest[]; +} + +export function partitionSkippedSetupRequests( + requests: readonly SetupRequest[], + workflowId: string, + skipped: ReadonlySet, +): PartitionedSetupRequests { + const pending: SetupRequest[] = []; + const skippedByUser: SetupRequest[] = []; + for (const request of requests) { + if (isSetupRequestSkipped(request, workflowId, skipped)) skippedByUser.push(request); + else pending.push(request); + } + return { pending, skippedByUser }; +} + +/** What the agent is told about cards it must not re-open. */ +export function describeSkippedSetup( + requests: readonly SetupRequest[], +): Array<{ nodeName: string; credentialType?: string; reopenWith: string }> { + return requests.map((request) => ({ + nodeName: request.node.name, + ...(request.credentialType ? { credentialType: request.credentialType } : {}), + reopenWith: setupSkipReopenToken(request), + })); +} + +export const SKIPPED_SETUP_GUIDANCE = + 'The user skipped these earlier in this conversation. Do not re-open the setup card for them: ' + + 'mention in your message what stays unconfigured and what that means at runtime, and offer to ' + + 'set it up when they want. Only after the user asks for a specific one, call setup again with ' + + '`reopenSkipped: [""]`.'; diff --git a/packages/@n8n/instance-ai/src/tools/workflows/setup-workflow.schema.ts b/packages/@n8n/instance-ai/src/tools/workflows/setup-workflow.schema.ts index 3eccb550be1..bfdbc90923e 100644 --- a/packages/@n8n/instance-ai/src/tools/workflows/setup-workflow.schema.ts +++ b/packages/@n8n/instance-ai/src/tools/workflows/setup-workflow.schema.ts @@ -27,5 +27,8 @@ export const setupResumeSchema = z.object({ action: z.enum(['apply', 'test-trigger']).optional(), credentials: z.record(z.record(z.string())).optional(), nodeParameters: z.record(z.record(z.unknown())).optional(), + /** Node names whose cards the user actively skipped, so the tool can tell a declined + * card apart from one that is merely still unconfigured. */ + skippedNodes: z.array(z.string()).optional(), testTriggerNode: z.string().optional(), }); diff --git a/packages/@n8n/instance-ai/src/tools/workflows/setup-workflow.service.ts b/packages/@n8n/instance-ai/src/tools/workflows/setup-workflow.service.ts index c7cc01c4efd..c47d88177dc 100644 --- a/packages/@n8n/instance-ai/src/tools/workflows/setup-workflow.service.ts +++ b/packages/@n8n/instance-ai/src/tools/workflows/setup-workflow.service.ts @@ -732,7 +732,7 @@ async function buildRequestForCredentialType( // rides along in credentialTestResult for display. // A parameter request needs action if issues remain. // A trigger-only request (no credential, no param issues) never blocks apply. - let needsAction = false; + let credentialNeedsAction = false; if (credentialType) { const existingOnNode = node.credentials?.[credentialType]; const boundId = @@ -743,11 +743,11 @@ async function buildRequestForCredentialType( isAiGatewayManagedCredential(existingOnNode) || (boundId !== undefined && existingCredentials.some((credential) => credential.id === boundId)); - needsAction = !isSettled; - } - if (hasParamIssues) { - needsAction = true; + credentialNeedsAction = !isSettled; } + // Tracked apart from `needsAction` because the two answer different questions: a node with a + // working credential and an unfilled parameter needs action, but not *about the credential*. + const needsAction = credentialNeedsAction || hasParamIssues; return { node: { @@ -777,6 +777,7 @@ async function buildRequestForCredentialType( ? { editableParameters: nodeCtx.editableParameters } : {}), needsAction, + ...(credentialNeedsAction ? { credentialNeedsAction } : {}), }; } diff --git a/packages/@n8n/instance-ai/src/tools/workflows/workflow-build-routing.ts b/packages/@n8n/instance-ai/src/tools/workflows/workflow-build-routing.ts index 1e323ff4b42..f3d5174b9ea 100644 --- a/packages/@n8n/instance-ai/src/tools/workflows/workflow-build-routing.ts +++ b/packages/@n8n/instance-ai/src/tools/workflows/workflow-build-routing.ts @@ -10,6 +10,9 @@ type WorkflowBuildRoutingInput = Omit< 'verificationReadiness' | 'setupRequirement' > & { workflowNeedsSetup?: boolean; + /** True when everything still needing setup belongs to a credential the user already + * skipped in this thread. Routing setup then would re-open the card they dismissed. */ + onlySkippedSetupRemains?: boolean; }; function hasSetupCredentials( @@ -76,6 +79,7 @@ function determineSetupRequirement( | 'mockedCredentialsByNode' | 'hasUnresolvedPlaceholders' | 'workflowNeedsSetup' + | 'onlySkippedSetupRemains' | 'changedNodeNames' >, ): WorkflowSetupRequirement { @@ -83,6 +87,18 @@ function determineSetupRequirement( return { status: 'not_required' }; } + // Checked before the reason-specific branches: a mocked credential or an unresolved + // placeholder on a node the user skipped is still a skipped node. The setup follow-up + // re-arms on every build, so without this gate any later edit re-opens the card. + if (outcome.onlySkippedSetupRemains) { + return { + status: 'not_required', + reason: 'skipped-by-user', + guidance: + 'The only remaining setup is for credentials the user skipped earlier in this conversation. Do not open the setup card: say what stays unconfigured and what that means at runtime, and offer to set it up when they want.', + }; + } + if (outcome.hasUnresolvedPlaceholders) { return { status: 'required', @@ -111,7 +127,7 @@ function determineSetupRequirement( } export function withDeterministicRouting(outcome: WorkflowBuildRoutingInput): WorkflowBuildOutcome { - const { workflowNeedsSetup, ...buildOutcome } = outcome; + const { workflowNeedsSetup, onlySkippedSetupRemains, ...buildOutcome } = outcome; return { ...buildOutcome, verificationReadiness: determineVerificationReadiness(outcome), diff --git a/packages/@n8n/instance-ai/src/types.ts b/packages/@n8n/instance-ai/src/types.ts index 45c6a3d270c..bc514a8f5af 100644 --- a/packages/@n8n/instance-ai/src/types.ts +++ b/packages/@n8n/instance-ai/src/types.ts @@ -1104,6 +1104,9 @@ export interface InstanceAiContext { /** Persist a thread-level "always allow" grant for the given key. Invoked by a tool when it * resumes from a `scope: 'session'` approval. No-op in contexts without persistence. */ grantSessionToolApproval?: (key: string) => Promise; + /** Drop a thread-level grant. Only for decisions meant to be reversible inside a thread — + * e.g. a skipped credential setup the user later asks to complete. */ + revokeSessionToolApproval?: (key: string) => Promise; /** When true, the instance is in read-only mode (source control branchReadOnly). */ branchReadOnly?: boolean; /** When `false`, callers must avoid surfacing node parameter values (or anything derived from them diff --git a/packages/@n8n/instance-ai/src/workflow-loop/__tests__/guidance.test.ts b/packages/@n8n/instance-ai/src/workflow-loop/__tests__/guidance.test.ts index 16731115fbb..c6c02c5228f 100644 --- a/packages/@n8n/instance-ai/src/workflow-loop/__tests__/guidance.test.ts +++ b/packages/@n8n/instance-ai/src/workflow-loop/__tests__/guidance.test.ts @@ -107,6 +107,21 @@ describe('formatWorkflowLoopGuidance', () => { ); }); + it('should not send the user back to the setup card for credentials they skipped', () => { + const action: WorkflowLoopAction = { + type: 'done', + summary: 'Built with mocks the user skipped', + mockedCredentialTypes: ['slackApi'], + hasUnresolvedPlaceholders: true, + workflowId: 'wf-skip-1', + setupSkippedByUser: true, + }; + const result = formatWorkflowLoopGuidance(action); + expect(result).not.toContain('workflows(action="setup")'); + expect(result).toContain('skipped earlier in this conversation'); + expect(result).toContain('offer'); + }); + it('should trigger workflow setup guidance when both mocked credentials and placeholders exist', () => { const action: WorkflowLoopAction = { type: 'done', diff --git a/packages/@n8n/instance-ai/src/workflow-loop/__tests__/workflow-loop-controller.test.ts b/packages/@n8n/instance-ai/src/workflow-loop/__tests__/workflow-loop-controller.test.ts index 3df23db7707..ce912dd413c 100644 --- a/packages/@n8n/instance-ai/src/workflow-loop/__tests__/workflow-loop-controller.test.ts +++ b/packages/@n8n/instance-ai/src/workflow-loop/__tests__/workflow-loop-controller.test.ts @@ -70,6 +70,37 @@ describe('createWorkItem', () => { // ── handleBuildOutcome ────────────────────────────────────────────────────── describe('handleBuildOutcome', () => { + it('carries a user-skipped setup requirement onto the state', () => { + const { state: next } = handleBuildOutcome( + makeState(), + [], + makeOutcome({ + workflowId: 'wf_123', + setupRequirement: { status: 'not_required', reason: 'skipped-by-user' }, + }), + ); + + expect(next.setupSkippedByUser).toBe(true); + }); + + it("clears a previous build's skipped-setup flag when setup is required again", () => { + // A credential added after the skip must still route setup, so the flag can't be sticky. + const { state: next } = handleBuildOutcome( + { ...makeState(), setupSkippedByUser: true }, + [], + makeOutcome({ + workflowId: 'wf_123', + setupRequirement: { + status: 'required', + reason: 'mocked-credentials', + guidance: 'Route the workflow through setup so the user can add real credentials.', + }, + }), + ); + + expect(next.setupSkippedByUser).toBe(false); + }); + it('transitions to verifying when submitted and testable', () => { const state = makeState(); const outcome = makeOutcome({ diff --git a/packages/@n8n/instance-ai/src/workflow-loop/guidance.ts b/packages/@n8n/instance-ai/src/workflow-loop/guidance.ts index dd3351074dc..3ccbed103df 100644 --- a/packages/@n8n/instance-ai/src/workflow-loop/guidance.ts +++ b/packages/@n8n/instance-ai/src/workflow-loop/guidance.ts @@ -22,6 +22,14 @@ export function formatWorkflowLoopGuidance( case 'continue_building': return `BUILD FAILED: ${action.reason}. Fix the workflow source file: ${formatSourceFileInstruction(action.sourceFilePath)}.`; case 'done': { + if (action.setupSkippedByUser) { + return ( + 'Workflow verified successfully. The credentials it still needs are ones the user ' + + 'skipped earlier in this conversation, so do NOT open the setup card again. Tell them ' + + 'which parts stay unconfigured and what that means when the workflow runs, and offer ' + + 'to set them up whenever they want.' + ); + } if (action.mockedCredentialTypes?.length || action.hasUnresolvedPlaceholders) { return ( 'Workflow verified successfully with temporary mock data. ' + diff --git a/packages/@n8n/instance-ai/src/workflow-loop/workflow-loop-controller.ts b/packages/@n8n/instance-ai/src/workflow-loop/workflow-loop-controller.ts index d940d8de916..4ca00ece77d 100644 --- a/packages/@n8n/instance-ai/src/workflow-loop/workflow-loop-controller.ts +++ b/packages/@n8n/instance-ai/src/workflow-loop/workflow-loop-controller.ts @@ -172,10 +172,16 @@ export function handleBuildOutcome( : undefined; const hasUnresolvedPlaceholders = outcome.hasUnresolvedPlaceholders ?? undefined; const sourceFilePath = outcome.sourceFilePath ?? normalizedState.sourceFilePath; + // Deliberately not carried over from the previous build: a credential added since then + // must still route setup, even if an earlier build had nothing but skipped ones left. + const setupSkippedByUser = + outcome.setupRequirement?.status === 'not_required' && + outcome.setupRequirement.reason === 'skipped-by-user'; const updatedState: WorkflowLoopState = { ...normalizedState, workflowId: outcome.workflowId ?? normalizedState.workflowId, ...(sourceFilePath ? { sourceFilePath } : {}), + setupSkippedByUser, lastTaskId: outcome.taskId, mockedCredentialTypes: mockedCredentialTypes ?? normalizedState.mockedCredentialTypes, hasUnresolvedPlaceholders: @@ -205,6 +211,7 @@ export function handleBuildOutcome( summary: outcome.summary, mockedCredentialTypes, hasUnresolvedPlaceholders: updatedState.hasUnresolvedPlaceholders, + setupSkippedByUser: updatedState.setupSkippedByUser, }, attempt, }; @@ -282,6 +289,7 @@ export function handleVerificationVerdict( summary: verdict.summary, mockedCredentialTypes: normalizedState.mockedCredentialTypes, hasUnresolvedPlaceholders: normalizedState.hasUnresolvedPlaceholders, + setupSkippedByUser: normalizedState.setupSkippedByUser, }, attempt, }; @@ -303,6 +311,7 @@ export function handleVerificationVerdict( summary: verdict.summary, mockedCredentialTypes: normalizedState.mockedCredentialTypes, hasUnresolvedPlaceholders: normalizedState.hasUnresolvedPlaceholders, + setupSkippedByUser: normalizedState.setupSkippedByUser, }, attempt, }; diff --git a/packages/@n8n/instance-ai/src/workflow-loop/workflow-loop-state.ts b/packages/@n8n/instance-ai/src/workflow-loop/workflow-loop-state.ts index e583e395639..402d5a56aa6 100644 --- a/packages/@n8n/instance-ai/src/workflow-loop/workflow-loop-state.ts +++ b/packages/@n8n/instance-ai/src/workflow-loop/workflow-loop-state.ts @@ -92,6 +92,12 @@ export const workflowLoopStateSchema = z.object({ preSaveSubmitFailures: z.number().int().min(0).optional(), postSubmitRemediationSubmitsUsed: z.number().int().min(0).optional(), lastRemediation: remediationMetadataSchema.optional(), + /** + * Set when the only setup this build needs is for credentials the user already skipped in + * this thread. Recomputed per build (never sticky) so a newly added credential still + * routes setup normally. + */ + setupSkippedByUser: z.boolean().optional(), /** * Set once the service has routed this work item to post-verification setup. * Guards the deterministic setup follow-up so it fires at most once per build. @@ -207,7 +213,13 @@ export const workflowVerificationReadinessSchema = z.discriminatedUnion('status' export type WorkflowVerificationReadiness = z.infer; export const workflowSetupRequirementSchema = z.discriminatedUnion('status', [ - z.object({ status: z.literal('not_required') }), + z.object({ + status: z.literal('not_required'), + // Only set when setup *would* have been routed but the user already skipped the + // credentials involved — kept so traces show why the follow-up went quiet. + reason: z.literal('skipped-by-user').optional(), + guidance: z.string().optional(), + }), z.object({ status: z.literal('required'), reason: z.enum(['mocked-credentials', 'unresolved-placeholders', 'workflow-needs-setup']), @@ -475,5 +487,6 @@ export type WorkflowLoopAction = summary: string; mockedCredentialTypes?: string[]; hasUnresolvedPlaceholders?: boolean; + setupSkippedByUser?: boolean; } | { type: 'blocked'; reason: string }; diff --git a/packages/cli/src/modules/instance-ai/instance-ai.service.ts b/packages/cli/src/modules/instance-ai/instance-ai.service.ts index d9b6e4fcf58..ea32657d158 100644 --- a/packages/cli/src/modules/instance-ai/instance-ai.service.ts +++ b/packages/cli/src/modules/instance-ai/instance-ai.service.ts @@ -647,6 +647,7 @@ function toConfirmationData(request: InstanceAiConfirmRequest): ConfirmationData action: 'apply', nodeCredentials: request.nodeCredentials, nodeParameters: request.nodeParameters, + skippedNodes: request.skippedNodes, }; case 'setupWorkflowTestTrigger': return { @@ -1024,6 +1025,27 @@ export class InstanceAiService { } } + /** + * Drop a per-user, thread-level grant. Used by decisions that are meant to be reversible + * within a thread — e.g. the user skipped a credential's setup, then later asks for it. + * Best-effort: a failed delete leaves the decision in place until the next run. + */ + private async revokeThreadSessionGrant( + threadId: string, + userId: string, + key: string, + ): Promise { + try { + await this.threadGrantRepo.revoke(threadId, userId, key); + } catch (error) { + this.logger.warn('Failed to revoke Instance AI session grant', { + threadId, + key, + error: getErrorMessage(error), + }); + } + } + /** Whether the AI service proxy is enabled for credit counting. */ isProxyEnabled(): boolean { return this.modelService.isProxyEnabled(); @@ -2448,6 +2470,10 @@ export class InstanceAiService { sessionGrants.add(key); }; context.grantSessionToolApproval = grantSessionToolApproval; + context.revokeSessionToolApproval = async (key: string) => { + await this.revokeThreadSessionGrant(threadId, user.id, key); + sessionGrants.delete(key); + }; // Domain-access approvals are stored as grant keys in `instance_ai_thread_grants` (via // the same load/persist path as above), so they survive restart and are visible @@ -5105,6 +5131,7 @@ export class InstanceAiService { ...(data.domainAccessAction ? { domainAccessAction: data.domainAccessAction } : {}), ...(data.action ? { action: data.action } : {}), ...(data.nodeParameters ? { nodeParameters: data.nodeParameters } : {}), + ...(data.skippedNodes ? { skippedNodes: data.skippedNodes } : {}), ...(data.testTriggerNode ? { testTriggerNode: data.testTriggerNode } : {}), ...(data.answers ? { answers: data.answers } : {}), ...(data.resourceDecision ? { resourceDecision: data.resourceDecision } : {}), diff --git a/packages/cli/src/modules/instance-ai/repositories/__tests__/instance-ai-thread-grant.repository.test.ts b/packages/cli/src/modules/instance-ai/repositories/__tests__/instance-ai-thread-grant.repository.test.ts index ccbd77b7bf6..9f06a989848 100644 --- a/packages/cli/src/modules/instance-ai/repositories/__tests__/instance-ai-thread-grant.repository.test.ts +++ b/packages/cli/src/modules/instance-ai/repositories/__tests__/instance-ai-thread-grant.repository.test.ts @@ -32,6 +32,21 @@ describe('InstanceAiThreadGrantRepository', () => { }); }); + describe('revoke', () => { + it('deletes the exact grant row', async () => { + const repo = buildRepo(); + repo.delete = vi.fn().mockResolvedValue({ affected: 1 }); + + await repo.revoke('thread-1', 'user-1', 'workflows:setup-skip:slackApi'); + + expect(repo.delete).toHaveBeenCalledWith({ + threadId: 'thread-1', + userId: 'user-1', + grantKey: 'workflows:setup-skip:slackApi', + }); + }); + }); + describe('findKeys', () => { it('returns the grant keys for the thread/user as a set', async () => { const repo = buildRepo(); diff --git a/packages/cli/src/modules/instance-ai/repositories/instance-ai-thread-grant.repository.ts b/packages/cli/src/modules/instance-ai/repositories/instance-ai-thread-grant.repository.ts index 8fea2c70c25..e2151ce5026 100644 --- a/packages/cli/src/modules/instance-ai/repositories/instance-ai-thread-grant.repository.ts +++ b/packages/cli/src/modules/instance-ai/repositories/instance-ai-thread-grant.repository.ts @@ -21,6 +21,11 @@ export class InstanceAiThreadGrantRepository extends Repository { + await this.delete({ threadId, userId, grantKey }); + } + /** The grant keys this user holds in this thread. */ async findKeys(threadId: string, userId: string): Promise> { const rows = await this.find({ diff --git a/packages/frontend/editor-ui/src/features/ai/instanceAi/workflowSetup/composables/useWorkflowSetupInputs.test.ts b/packages/frontend/editor-ui/src/features/ai/instanceAi/workflowSetup/composables/useWorkflowSetupInputs.test.ts index 22f6a678672..8bfb9adef36 100644 --- a/packages/frontend/editor-ui/src/features/ai/instanceAi/workflowSetup/composables/useWorkflowSetupInputs.test.ts +++ b/packages/frontend/editor-ui/src/features/ai/instanceAi/workflowSetup/composables/useWorkflowSetupInputs.test.ts @@ -255,6 +255,18 @@ describe('useWorkflowSetupInputs', () => { }); }); + it('reports skipped sections so the backend stops asking for them', () => { + // Without this the backend only sees an unconfigured node, which reads as "ask again". + const h = setupHarness(); + h.inputs.setCredential(h.sectionB, 'cred-2'); + h.inputs.markSectionSkipped(h.sectionA); + + expect(h.inputs.buildCompletedSetupPayload()).toEqual({ + nodeCredentials: { Slack: { slackApi: 'cred-2' } }, + skippedNodes: ['HTTP Request'], + }); + }); + it('seeds selections from current credentials and tests seeded credentials', async () => { addCredential({ id: 'current-cred', type: 'httpBasicAuth', name: 'Current credential' }); const section = makeWorkflowSetupSection({ @@ -566,6 +578,9 @@ describe('useWorkflowSetupInputs', () => { h.inputs.setCredential(groupedSection, 'cred-1'); h.inputs.markSectionSkipped(groupedSection); - expect(h.inputs.buildCompletedSetupPayload()).toEqual({}); + // No credentials applied, and every node behind the section is reported as skipped. + expect(h.inputs.buildCompletedSetupPayload()).toEqual({ + skippedNodes: ['Primary', 'Follower'], + }); }); }); diff --git a/packages/frontend/editor-ui/src/features/ai/instanceAi/workflowSetup/composables/useWorkflowSetupInputs.ts b/packages/frontend/editor-ui/src/features/ai/instanceAi/workflowSetup/composables/useWorkflowSetupInputs.ts index 22f2612b548..d4d3aa408a2 100644 --- a/packages/frontend/editor-ui/src/features/ai/instanceAi/workflowSetup/composables/useWorkflowSetupInputs.ts +++ b/packages/frontend/editor-ui/src/features/ai/instanceAi/workflowSetup/composables/useWorkflowSetupInputs.ts @@ -155,13 +155,29 @@ export function useWorkflowSetupInputs(deps: { const nodeCredentials = buildNodeCredentials(includeCredential); const nodeParameters = buildNodeParameters(includeParams); + const skippedNodes = buildSkippedNodeNames(); return { ...(Object.keys(nodeCredentials).length > 0 ? { nodeCredentials } : {}), ...(Object.keys(nodeParameters).length > 0 ? { nodeParameters } : {}), + ...(skippedNodes.length > 0 ? { skippedNodes } : {}), }; } + /** + * Every node behind a skipped section — a credential section can cover several nodes, and + * the backend keys the decision off node names. + */ + function buildSkippedNodeNames(): string[] { + const names = new Set(); + for (const section of deps.sections.value) { + if (!isSectionSkipped(section)) continue; + names.add(section.targetNodeName); + for (const target of section.credentialTargetNodes) names.add(target.name); + } + return [...names]; + } + function buildNodeCredentials( shouldInclude: (section: WorkflowSetupSection) => boolean, ): Record> { diff --git a/packages/frontend/editor-ui/src/features/ai/instanceAi/workflowSetup/workflowSetup.types.ts b/packages/frontend/editor-ui/src/features/ai/instanceAi/workflowSetup/workflowSetup.types.ts index 42ba568f684..5722fda3e85 100644 --- a/packages/frontend/editor-ui/src/features/ai/instanceAi/workflowSetup/workflowSetup.types.ts +++ b/packages/frontend/editor-ui/src/features/ai/instanceAi/workflowSetup/workflowSetup.types.ts @@ -40,6 +40,9 @@ export type WorkflowSetupStep = export interface WorkflowSetupApplyPayload { nodeCredentials?: Record>; nodeParameters?: Record; + /** Nodes the user skipped. Sent so the backend can tell a dismissed card apart from one + * that just isn't filled in yet, and stop asking for it later in the conversation. */ + skippedNodes?: string[]; } export type TerminalState = 'applying' | 'applied' | 'partial' | 'deferred';