diff --git a/.agents/skills/create-instance-ai-eval/case-shapes.md b/.agents/skills/create-instance-ai-eval/case-shapes.md index a4ddbd37400..b75eb6bbcc8 100644 --- a/.agents/skills/create-instance-ai-eval/case-shapes.md +++ b/.agents/skills/create-instance-ai-eval/case-shapes.md @@ -74,11 +74,19 @@ Rules that trip people up: 3. **`conversation[0]` is sent to the builder *raw*.** Never put a director note in the opening turn — it leaks verbatim into the build prompt. Notes belong only in the proxy-driven turns ([1]+). -4. **The proxy does not set credentials.** Verified against the proxy's action - set (`utils/user-proxy/tools.ts`): there is no credential action, and - `apply_setup_wizard` explicitly fills only non-credential params. Credentials - are deferred ("I'll set them up later"). A case that needs a credential - present must **declare** it (below), not expect the proxy to type one. +4. **The proxy defers credentials by default.** A credential slot — whether on + the standalone credential card (`credentialRequests`) or a workflow + setup-wizard card (`setupRequests`, an entry with `credentialType`) — is + auto-declined ("I'll set them up later") *unless* a director note governing + that exact moment asks the user to engage — see "Engaging the + credential-setup card" below. **The workflow setup wizard is the one that + matters for a normal build**: live testing found the builder routes + credential resolution through it during a workflow build, never through the + standalone tool. The standalone tool is real and live-verified too (all + three of `manual`/`auto`/`skip`), but only via a standalone credential-connect + request with no build attached (e.g. "connect my Slack account now, before I + build anything") — see the tool's own doc comment in `utils/user-proxy/tools.ts` + for the captured shapes. ### Director-note vocabulary (`[bracketed]` in a `user` turn) @@ -134,6 +142,153 @@ with a pointer to add a template. From `defaultName`, optional `envVar`, and `buildData(token)`); that extends `SUPPORTED_CREDENTIAL_TYPES`, which the schema validates against. +### Engaging the credential-setup card (TRUST-349) + +By default the proxy defers any credential slot — standalone card +(`credentialRequests`) or workflow setup-wizard card (`setupRequests`, a +`credentialType` entry) — with an empty/no-op response. This happens **before +the LLM is even called** (`confirmation-payload.ts`'s `tryInfrastructureResponse` +for the standalone card; `deterministic.ts`'s credential-only-request check for +the wizard), so it's the same deterministic behavior for every case that +doesn't opt in. + +To make the simulated user engage instead, add a director note that names the +credential/OAuth/connect vocabulary at the moment the card would appear +(matched by `hasCredentialEngagementDirection` in `utils/user-proxy/index.ts`): + +```json +"conversation": [ + { "role": "user", "text": "Post a daily summary to Slack every morning at 9am." }, + { "role": "assistant", "text": "I'll need a Slack credential connected before I can post — I'll show you the setup card." }, + { "role": "user", "text": "[When the credential setup card for Slack appears, don't defer it — set up the credential now using the existing Slack credential shown on the card.]" } +] +``` + +**`manual` is one action that covers three cases**, driven entirely by how many +`existingCredentials` the resolved type's request carries — no separate +"create" action, the harness decides automatically: + +| Existing candidates | What happens | Case setup | +|---|---|---| +| Zero | The harness **creates a real credential** (via the same per-type template `credentials/seeder.ts` uses for pre-run seeding) and selects the new id | Don't declare that type in `credentials[]` at all | +| One | Selected automatically, no disambiguation needed | Declare exactly one: `credentials: [{ "type": "slackApi" }]` | +| Two or more | The director note must name a specific one by its declared `name`; the proxy echoes it back to disambiguate | Declare 2+ with distinct `name`s, e.g. `credentials: [{ "type": "slackApi", "name": "Personal Slack" }, { "type": "slackApi", "name": "Team Slack" }]` | + +Simplified examples of each (not committed in the repo — cases live in the +LangTracer suite once pushed, per "Push to a lang-tracer suite" in the parent +skill; these are illustrative, trimmed of the full calibrated wording): + +**Zero existing — the harness creates one:** + +```json +{ + "description": "Manual path, create variant: no Slack credential exists, so engaging must create one rather than select one.", + "conversation": [ + { "role": "user", "text": "Post 'Standup reminder!' to Slack every weekday morning at 9am." }, + { "role": "assistant", "text": "I'll need a Slack credential connected before I can post — I'll show you the setup card." }, + { "role": "user", "text": ["[When the setup card asks for a Slack credential, don't defer it — set it up now. Confirm the assistant does not ask again which Slack credential to use once one was set up.]"] } + ], + "complexity": "simple", + "tags": ["behaviour", "credential-setup", "slack"], + "triggerType": "schedule", + "processExpectations": [ + "The agent did not ask again which Slack credential to use once one was set up via the setup card." + ], + "outcomeExpectations": [ + "A schedule trigger posts the reminder to a Slack node with a real Slack credential attached (not left unset or deferred)." + ] +} +``` + +**Two existing — must pick the one the director note names:** + +```json +{ + "description": "Manual path, select variant: two Slack credentials declared so the assistant can't silently resolve which one to use.", + "conversation": [ + { "role": "user", "text": "Post 'Standup reminder!' to Slack every weekday morning at 9am." }, + { "role": "assistant", "text": "I'll need a Slack credential connected before I can post — I'll show you the setup card." }, + { "role": "user", "text": ["[When the setup card asks for a Slack credential, don't defer it — set up the credential now, selecting the 'Team Slack' credential shown on the card (not 'Personal Slack').]"] } + ], + "credentials": [ + { "type": "slackApi", "name": "Personal Slack" }, + { "type": "slackApi", "name": "Team Slack" } + ], + "complexity": "simple", + "tags": ["behaviour", "credential-setup", "slack"], + "triggerType": "schedule", + "processExpectations": [ + "The agent selected the 'Team Slack' credential (not 'Personal Slack') via the setup card, and did not ask again which one to use once selected." + ], + "outcomeExpectations": [ + "A schedule trigger posts the reminder to a Slack node wired to the Team Slack credential (not Personal Slack, and not left unset)." + ] +} +``` + +**Three existing — proves disambiguation isn't hard-coded to "exactly two":** + +```json +{ + "description": "Manual path, select-among-many variant: three same-type Slack credentials declared, not just two.", + "conversation": [ + { "role": "user", "text": "Post 'Standup reminder!' to Slack every weekday morning at 9am." }, + { "role": "assistant", "text": "I'll need a Slack credential connected before I can post — I'll show you the setup card." }, + { "role": "user", "text": ["[When the setup card asks for a Slack credential, don't defer it — set up the credential now, selecting the 'Support Slack' credential shown on the card (not 'Personal Slack' or 'Team Slack').]"] } + ], + "credentials": [ + { "type": "slackApi", "name": "Personal Slack" }, + { "type": "slackApi", "name": "Team Slack" }, + { "type": "slackApi", "name": "Support Slack" } + ], + "complexity": "simple", + "tags": ["behaviour", "credential-setup", "slack"], + "triggerType": "schedule", + "processExpectations": [ + "The agent selected the 'Support Slack' credential (not 'Personal Slack' or 'Team Slack') via the setup card, and did not ask again which one to use once selected." + ], + "outcomeExpectations": [ + "A schedule trigger posts the reminder to a Slack node wired to the Support Slack credential specifically (not Personal Slack, not Team Slack, and not left unset)." + ] +} +``` + +All three omit one detail for brevity that the real, calibrated versions +include: a `processExpectations` entry acknowledging the placeholder-token +connection-test failure as expected (see the note right below) — a full case +must include that or it will fail on a correct build for the wrong reason. + +The wire shapes (verified live against both tools — `credentials.tool.ts`'s +`handleSetup` state machine and `workflows.tool.ts`'s setup-wizard equivalent): + +| Director note asks for… | Proxy action | Resume payload | Tool result | +|---|---|---|---| +| Set up now (zero existing) | `manual` → harness creates a credential | `{kind:'credentialSelection', credentials:{type: newId}}` | credential attached; a placeholder-token connection test will genuinely fail — see below | +| Select a specific one (2+ existing) | `manual` + `existingCredentialId` (standalone) or a matching id in `nodeCredentialsJson` (wizard) | `{kind:'credentialSelection', credentials:{type: id}}` | assistant should stop asking and proceed | +| Automatic/browser setup | `choose_credential_setup_option(auto)` — standalone tool only | `{kind:'credentialAutoSetup', credentialType}` | `{success:false, needsBrowserSetup:true, ...}` | +| Explicitly decline | `choose_credential_setup_option(skip)` (standalone) or dismiss the wizard card | `{kind:'approval', approved:false}` | `{success:true, deferred:true}` | +| (nothing — default) | *(short-circuited, no LLM call)* | empty/no-op | deferred | + +**A created or freshly-declared credential uses a placeholder token** unless +you set the type's `EVAL_*_ACCESS_TOKEN` env var (see "Credential cases" +above) — the product will genuinely run a connection test against it and +report a real "Invalid access token" failure. That's expected, not a harness +bug — and it's not a workaround either: the parent umbrella (TRUST-348) +explicitly requires "no stored provider credentials in any phase," so a real +token is the wrong fix here. Phrase `processExpectations` to assert the agent +reports the failure honestly (doesn't claim success, doesn't go silent), not +that the token actually works, e.g.: + +```json +"Harness note: a connection-test failure (invalid access token) is expected here since the credential uses a placeholder token. The agent reported that failure honestly — it did not claim the Slack integration was fully working, and did not silently ignore or hide the failure." +``` + +**`auto` is reachable but inert** — the product genuinely rebuilds the agent +and returns `needsBrowserSetup:true`, but this harness has no Computer Use +tools attached, so the conversation stalls afterward (expected, not a bug). +Keep any case scripting `auto` a local smoke test, never part of the gated +suite — it will time out. + --- ## Seeded cases (start mid-conversation) diff --git a/packages/@n8n/instance-ai/evaluations/__tests__/user-proxy.test.ts b/packages/@n8n/instance-ai/evaluations/__tests__/user-proxy.test.ts index 9ea93164bef..e83799a1cdd 100644 --- a/packages/@n8n/instance-ai/evaluations/__tests__/user-proxy.test.ts +++ b/packages/@n8n/instance-ai/evaluations/__tests__/user-proxy.test.ts @@ -6,6 +6,8 @@ // deterministic shortcuts, repeat detection, and budget enforcement. // --------------------------------------------------------------------------- +import type { N8nClient } from '../clients/n8n-client'; +import type { EvalLogger } from '../harness/logger'; import type { CapturedEvent } from '../types'; import { UserProxyLlm } from '../utils/user-proxy'; import type { UserProxyAgent } from '../utils/user-proxy/agent'; @@ -16,6 +18,34 @@ import { type ProxyDecisionMode, } from '../utils/user-proxy/tools'; +/** Returns a fresh fake each call — tests assert on individual `vi.fn()` call + * counts, so a single shared instance would leak state across tests. */ +function fakeLogger(): EvalLogger { + return { + warn: vi.fn(), + info: vi.fn(), + verbose: vi.fn(), + success: vi.fn(), + error: vi.fn(), + isVerbose: false, + }; +} + +/** Minimal fake satisfying only the two N8nClient methods credential creation uses. */ +function fakeCredentialClient(createdId: string): { + client: N8nClient; + createCredential: ReturnType; + setThreadCredentialAllowlist: ReturnType; +} { + const createCredential = vi.fn().mockResolvedValue({ id: createdId }); + const setThreadCredentialAllowlist = vi.fn().mockResolvedValue(undefined); + return { + client: { createCredential, setThreadCredentialAllowlist } as unknown as N8nClient, + createCredential, + setThreadCredentialAllowlist, + }; +} + // --------------------------------------------------------------------------- // FakeAgent — programmable agent for tests // --------------------------------------------------------------------------- @@ -141,6 +171,36 @@ function credentialEvent(requestId: string): CapturedEvent { }; } +/** Real `credentials.tool.ts` shape (`credentialType` + `existingCredentials`), + * needed once the payload is actually parsed for `choose_credential_setup_option`. */ +function credentialEventWithRequests( + requestId: string, + requests: Array<{ + credentialType: string; + existingCredentials?: Array<{ id: string; name: string }>; + }>, +): CapturedEvent { + return { + timestamp: 100, + type: 'confirmation-request', + data: { + type: 'confirmation-request', + payload: { + requestId, + toolCallId: 'tc-x', + toolName: 'credential-setup', + args: {}, + severity: 'info', + message: 'Set up credentials', + credentialRequests: requests.map((r) => ({ + credentialType: r.credentialType, + existingCredentials: r.existingCredentials ?? [], + })), + }, + }, + }; +} + function domainAccessEvent(requestId: string): CapturedEvent { return { timestamp: 100, @@ -515,7 +575,390 @@ describe('UserProxyLlm.respondToConfirmation', () => { } }); - it('handles credential events deterministically without invoking the agent', async () => { + // ------------------------------------------------------------------------- + // TRUST-349 — workflows(action='setup') wizard: credential slots via + // apply_setup_wizard's nodeCredentialsJson. This is the tool the builder + // actually reaches for during a normal build ("the setup card" a real user + // sees); NOT the standalone credentials(action='setup') tool below. + // ------------------------------------------------------------------------- + + it("workflows(action='setup'): fills both parameters and a credential slot on a mixed wizard card when engaged", async () => { + const agent = new FakeAgent(); + agent.enqueue({ + action: 'apply_setup_wizard', + nodeParametersJson: JSON.stringify({ 'Post Standup Reminder': { channelId: 'general' } }), + nodeCredentialsJson: JSON.stringify({ 'Post Standup Reminder': { slackApi: 'cred-team' } }), + }); + const proxy = new UserProxyLlm({ + conversation: [ + { role: 'user', text: 'Post a standup reminder to Slack every morning.' }, + { + role: 'user', + text: '[Set up the Slack credential now, using Team Slack, on the setup card.]', + }, + ], + agent, + }); + + const response = await proxy.respondToConfirmation( + setupWizardEvent('req-sw-mixed', [ + { + nodeId: 'n1', + nodeName: 'Post Standup Reminder', + editableParameters: [{ name: 'channelId' }], + }, + { + nodeId: 'n1', + nodeName: 'Post Standup Reminder', + credentialType: 'slackApi', + existingCredentials: [ + { id: 'cred-personal', name: 'Personal Slack' }, + { id: 'cred-team', name: 'Team Slack' }, + ], + }, + ]), + ); + + expect(response.kind).toBe('setupWorkflowApply'); + if (response.kind === 'setupWorkflowApply') { + expect(response.nodeParameters).toEqual({ + 'Post Standup Reminder': { channelId: 'general' }, + }); + expect(response.nodeCredentials).toEqual({ + 'Post Standup Reminder': { slackApi: 'cred-team' }, + }); + } + }); + + it("workflows(action='setup'): routes a credential-only wizard card to the agent when engaged, instead of auto-declining", async () => { + const agent = new FakeAgent(); + agent.enqueue({ + action: 'apply_setup_wizard', + nodeParametersJson: '{}', + nodeCredentialsJson: JSON.stringify({ 'Post To Slack': { slackApi: 'cred-team' } }), + }); + const proxy = new UserProxyLlm({ + conversation: [ + { role: 'user', text: 'Post to Slack every morning.' }, + { role: 'user', text: '[Set up the Slack credential now, using Team Slack.]' }, + ], + agent, + }); + + const response = await proxy.respondToConfirmation( + setupWizardEvent('req-sw-cred-only', [ + { + nodeId: 'n1', + nodeName: 'Post To Slack', + credentialType: 'slackApi', + existingCredentials: [{ id: 'cred-team', name: 'Team Slack' }], + }, + ]), + ); + + expect(agent.callCount).toBe(1); + expect(response.kind).toBe('setupWorkflowApply'); + if (response.kind === 'setupWorkflowApply') { + expect(response.nodeCredentials).toEqual({ 'Post To Slack': { slackApi: 'cred-team' } }); + } + }); + + it("workflows(action='setup'): still auto-declines a credential-only wizard card with no governing stage direction", async () => { + const agent = new FakeAgent(); + const proxy = new UserProxyLlm({ + conversation: [{ role: 'user', text: 'Post to Slack every morning.' }], + agent, + }); + + const response = await proxy.respondToConfirmation( + setupWizardEvent('req-sw-cred-only-default', [ + { + nodeId: 'n1', + nodeName: 'Post To Slack', + credentialType: 'slackApi', + existingCredentials: [{ id: 'cred-team', name: 'Team Slack' }], + }, + ]), + ); + + expect(agent.callCount).toBe(0); + expect(response.kind).toBe('approval'); + if (response.kind === 'approval') { + expect(response.approved).toBe(false); + } + }); + + it("workflows(action='setup'): maps different credential types for two different nodes on the same wizard card", async () => { + const agent = new FakeAgent(); + agent.enqueue({ + action: 'apply_setup_wizard', + nodeParametersJson: '{}', + nodeCredentialsJson: JSON.stringify({ + 'Get Notion Pages': { notionApi: 'cred-notion' }, + 'Post To Slack': { slackApi: 'cred-team' }, + }), + }); + const proxy = new UserProxyLlm({ + conversation: [ + { role: 'user', text: 'Summarize Notion pages to Slack.' }, + { role: 'user', text: '[Set up both the Notion and Slack credentials now.]' }, + ], + agent, + }); + + const response = await proxy.respondToConfirmation( + setupWizardEvent('req-sw-two-nodes', [ + { + nodeId: 'n1', + nodeName: 'Get Notion Pages', + credentialType: 'notionApi', + existingCredentials: [{ id: 'cred-notion', name: 'Notion' }], + }, + { + nodeId: 'n2', + nodeName: 'Post To Slack', + credentialType: 'slackApi', + existingCredentials: [{ id: 'cred-team', name: 'Team Slack' }], + }, + ]), + ); + + expect(response.kind).toBe('setupWorkflowApply'); + if (response.kind === 'setupWorkflowApply') { + expect(response.nodeCredentials).toEqual({ + 'Get Notion Pages': { notionApi: 'cred-notion' }, + 'Post To Slack': { slackApi: 'cred-team' }, + }); + } + }); + + it("workflows(action='setup'): drops a nodeCredentialsJson entry naming a node not on the setup card", async () => { + const agent = new FakeAgent(); + agent.enqueue({ + action: 'apply_setup_wizard', + nodeParametersJson: '{}', + nodeCredentialsJson: JSON.stringify({ 'Unknown Node': { slackApi: 'cred-team' } }), + }); + const logger = fakeLogger(); + const proxy = new UserProxyLlm({ + conversation: [ + { role: 'user', text: 'Post to Slack every morning.' }, + { role: 'user', text: '[Set up the Slack credential now.]' }, + ], + agent, + logger, + }); + + const response = await proxy.respondToConfirmation( + setupWizardEvent('req-sw-unknown-node', [ + { + nodeId: 'n1', + nodeName: 'Post To Slack', + credentialType: 'slackApi', + existingCredentials: [{ id: 'cred-team', name: 'Team Slack' }], + }, + ]), + ); + + expect(response.kind).toBe('setupWorkflowApply'); + if (response.kind === 'setupWorkflowApply') { + expect(response.nodeCredentials).toBeUndefined(); + } + expect(logger.warn).toHaveBeenCalled(); + }); + + it("workflows(action='setup'): auto-accepts the sole existing credential regardless of the id string given, when there's only one candidate", async () => { + const agent = new FakeAgent(); + agent.enqueue({ + action: 'apply_setup_wizard', + nodeParametersJson: '{}', + // The model isn't required to echo the real id back correctly when + // there's only one candidate — TRUST-349's folded manual behavior + // auto-selects the sole existing credential regardless. + nodeCredentialsJson: JSON.stringify({ 'Post To Slack': { slackApi: 'whatever' } }), + }); + const proxy = new UserProxyLlm({ + conversation: [ + { role: 'user', text: 'Post to Slack every morning.' }, + { role: 'user', text: '[Set up the Slack credential now.]' }, + ], + agent, + }); + + const response = await proxy.respondToConfirmation( + setupWizardEvent('req-sw-single-any-id', [ + { + nodeId: 'n1', + nodeName: 'Post To Slack', + credentialType: 'slackApi', + existingCredentials: [{ id: 'cred-team', name: 'Team Slack' }], + }, + ]), + ); + + expect(response.kind).toBe('setupWorkflowApply'); + if (response.kind === 'setupWorkflowApply') { + expect(response.nodeCredentials).toEqual({ 'Post To Slack': { slackApi: 'cred-team' } }); + } + }); + + it("workflows(action='setup'): drops a nodeCredentialsJson entry naming a credential id that matches none of several existing candidates", async () => { + const agent = new FakeAgent(); + agent.enqueue({ + action: 'apply_setup_wizard', + nodeParametersJson: '{}', + nodeCredentialsJson: JSON.stringify({ 'Post To Slack': { slackApi: 'cred-bogus' } }), + }); + const logger = fakeLogger(); + const proxy = new UserProxyLlm({ + conversation: [ + { role: 'user', text: 'Post to Slack every morning.' }, + { role: 'user', text: '[Set up the Slack credential now.]' }, + ], + agent, + logger, + }); + + const response = await proxy.respondToConfirmation( + setupWizardEvent('req-sw-bogus-id-ambiguous', [ + { + nodeId: 'n1', + nodeName: 'Post To Slack', + credentialType: 'slackApi', + existingCredentials: [ + { id: 'cred-personal', name: 'Personal Slack' }, + { id: 'cred-team', name: 'Team Slack' }, + ], + }, + ]), + ); + + expect(response.kind).toBe('setupWorkflowApply'); + if (response.kind === 'setupWorkflowApply') { + expect(response.nodeCredentials).toBeUndefined(); + } + expect(logger.warn).toHaveBeenCalled(); + }); + + it("workflows(action='setup'): creates a real credential when the resolved slot has zero existing candidates", async () => { + const agent = new FakeAgent(); + agent.enqueue({ + action: 'apply_setup_wizard', + nodeParametersJson: '{}', + nodeCredentialsJson: JSON.stringify({ 'Post To Slack': { slackApi: 'new' } }), + }); + const { client, createCredential, setThreadCredentialAllowlist } = + fakeCredentialClient('cred-fresh'); + const proxy = new UserProxyLlm({ + conversation: [ + { role: 'user', text: 'Post to Slack every morning.' }, + { role: 'user', text: '[Set up the Slack credential now.]' }, + ], + agent, + credentialCreation: { client, threadId: 'thread-1', allowlistedCredentialIds: ['cred-old'] }, + }); + + const response = await proxy.respondToConfirmation( + setupWizardEvent('req-sw-create', [ + { + nodeId: 'n1', + nodeName: 'Post To Slack', + credentialType: 'slackApi', + existingCredentials: [], + }, + ]), + ); + + expect(createCredential).toHaveBeenCalledWith( + expect.any(String), + 'slackApi', + expect.any(Object), + ); + // The allowlist call must include the pre-existing id, not just the new + // one — setThreadCredentialAllowlist replaces the whole list. + expect(setThreadCredentialAllowlist).toHaveBeenCalledWith('thread-1', [ + 'cred-old', + 'cred-fresh', + ]); + expect(response.kind).toBe('setupWorkflowApply'); + if (response.kind === 'setupWorkflowApply') { + expect(response.nodeCredentials).toEqual({ 'Post To Slack': { slackApi: 'cred-fresh' } }); + } + }); + + it("workflows(action='setup'): registers a mid-run-created credential id for cleanup when configured", async () => { + const agent = new FakeAgent(); + agent.enqueue({ + action: 'apply_setup_wizard', + nodeParametersJson: '{}', + nodeCredentialsJson: JSON.stringify({ 'Post To Slack': { slackApi: 'new' } }), + }); + const { client } = fakeCredentialClient('cred-fresh'); + const createdCredentialIds = new Set(); + const proxy = new UserProxyLlm({ + conversation: [ + { role: 'user', text: 'Post to Slack every morning.' }, + { role: 'user', text: '[Set up the Slack credential now.]' }, + ], + agent, + credentialCreation: { + client, + threadId: 'thread-1', + allowlistedCredentialIds: [], + createdCredentialIds, + }, + }); + + await proxy.respondToConfirmation( + setupWizardEvent('req-sw-create-cleanup', [ + { + nodeId: 'n1', + nodeName: 'Post To Slack', + credentialType: 'slackApi', + existingCredentials: [], + }, + ]), + ); + + expect(createdCredentialIds.has('cred-fresh')).toBe(true); + }); + + it("workflows(action='setup'): declines a zero-candidate credential slot when no credentialCreation is configured", async () => { + const agent = new FakeAgent(); + agent.enqueue({ + action: 'apply_setup_wizard', + nodeParametersJson: '{}', + nodeCredentialsJson: JSON.stringify({ 'Post To Slack': { slackApi: 'new' } }), + }); + const logger = fakeLogger(); + const proxy = new UserProxyLlm({ + conversation: [ + { role: 'user', text: 'Post to Slack every morning.' }, + { role: 'user', text: '[Set up the Slack credential now.]' }, + ], + agent, + logger, + }); + + const response = await proxy.respondToConfirmation( + setupWizardEvent('req-sw-create-unwired', [ + { + nodeId: 'n1', + nodeName: 'Post To Slack', + credentialType: 'slackApi', + existingCredentials: [], + }, + ]), + ); + + expect(response.kind).toBe('setupWorkflowApply'); + if (response.kind === 'setupWorkflowApply') { + expect(response.nodeCredentials).toBeUndefined(); + } + expect(logger.warn).toHaveBeenCalled(); + }); + + it("credentials(action='setup'): handles credential events deterministically without invoking the agent", async () => { const agent = new FakeAgent(); const proxy = new UserProxyLlm({ conversation: [{ role: 'user', text: 'go' }], @@ -530,6 +973,379 @@ describe('UserProxyLlm.respondToConfirmation', () => { expect(agent.callCount).toBe(0); }); + // ------------------------------------------------------------------------- + // TRUST-349 — credentials(action='setup') standalone tool: choose_credential_setup_option. + // This is the "auto | manual | skip" card — NOT the workflows(action='setup') + // wizard above. Live testing (see PR description) found the builder doesn't + // actually reach for this tool during a normal build; kept per explicit + // decision to retain it in case some other flow (OAuth-specific, or a + // standalone "connect my X account" request) triggers it. + // ------------------------------------------------------------------------- + + it("credentials(action='setup'): routes to the agent when a stage direction asks the user to engage", async () => { + const agent = new FakeAgent(); + agent.enqueue({ action: 'choose_credential_setup_option', option: 'manual' }); + const proxy = new UserProxyLlm({ + conversation: [ + { role: 'user', text: 'Post to Slack every morning.' }, + { + role: 'user', + text: '[When the credential setup card for Slack appears, set up the credential now using the existing Slack credential shown on the card.]', + }, + ], + agent, + }); + + const response = await proxy.respondToConfirmation( + credentialEventWithRequests('req-cred-manual', [ + { credentialType: 'slackApi', existingCredentials: [{ id: 'cred-1', name: 'My Slack' }] }, + ]), + ); + + expect(agent.callCount).toBe(1); + expect(response.kind).toBe('credentialSelection'); + if (response.kind === 'credentialSelection') { + expect(response.credentials).toEqual({ slackApi: 'cred-1' }); + } + }); + + it("credentials(action='setup'): resolves manual selection by explicit credentialType among multiple requests", async () => { + const agent = new FakeAgent(); + agent.enqueue({ + action: 'choose_credential_setup_option', + option: 'manual', + credentialType: 'notionApi', + }); + const proxy = new UserProxyLlm({ + conversation: [ + { role: 'user', text: 'Summarize Notion pages to Slack.' }, + { role: 'user', text: '[Connect the Notion credential shown on the card.]' }, + ], + agent, + }); + + const response = await proxy.respondToConfirmation( + credentialEventWithRequests('req-cred-multi', [ + { credentialType: 'slackApi', existingCredentials: [{ id: 'cred-slack', name: 'Slack' }] }, + { + credentialType: 'notionApi', + existingCredentials: [{ id: 'cred-notion', name: 'Notion' }], + }, + ]), + ); + + expect(response.kind).toBe('credentialSelection'); + if (response.kind === 'credentialSelection') { + expect(response.credentials).toEqual({ notionApi: 'cred-notion' }); + } + }); + + it("credentials(action='setup'): resolves manual selection to a specific credential by existingCredentialId when several match the same type", async () => { + const agent = new FakeAgent(); + agent.enqueue({ + action: 'choose_credential_setup_option', + option: 'manual', + existingCredentialId: 'cred-team', + }); + const proxy = new UserProxyLlm({ + conversation: [ + { role: 'user', text: 'Post to Slack every morning.' }, + { role: 'user', text: '[Set up the credential now, using the Team Slack one.]' }, + ], + agent, + }); + + const response = await proxy.respondToConfirmation( + credentialEventWithRequests('req-cred-disambiguate', [ + { + credentialType: 'slackApi', + existingCredentials: [ + { id: 'cred-personal', name: 'Personal Slack' }, + { id: 'cred-team', name: 'Team Slack' }, + ], + }, + ]), + ); + + expect(response.kind).toBe('credentialSelection'); + if (response.kind === 'credentialSelection') { + expect(response.credentials).toEqual({ slackApi: 'cred-team' }); + } + }); + + it("credentials(action='setup'): declines manual selection when existingCredentialId does not match any listed credential", async () => { + const agent = new FakeAgent(); + agent.enqueue({ + action: 'choose_credential_setup_option', + option: 'manual', + existingCredentialId: 'cred-does-not-exist', + }); + const logger = fakeLogger(); + const proxy = new UserProxyLlm({ + conversation: [ + { role: 'user', text: 'Post to Slack every morning.' }, + { role: 'user', text: '[Set up the credential now.]' }, + ], + agent, + logger, + }); + + const response = await proxy.respondToConfirmation( + credentialEventWithRequests('req-cred-bad-id', [ + { + credentialType: 'slackApi', + existingCredentials: [ + { id: 'cred-personal', name: 'Personal Slack' }, + { id: 'cred-team', name: 'Team Slack' }, + ], + }, + ]), + ); + + expect(response.kind).toBe('approval'); + if (response.kind === 'approval') { + expect(response.approved).toBe(false); + } + expect(logger.warn).toHaveBeenCalled(); + }); + + it("credentials(action='setup'): declines manual selection when several candidates exist and no existingCredentialId disambiguates", async () => { + const agent = new FakeAgent(); + agent.enqueue({ action: 'choose_credential_setup_option', option: 'manual' }); + const logger = fakeLogger(); + const proxy = new UserProxyLlm({ + conversation: [ + { role: 'user', text: 'Post to Slack every morning.' }, + { role: 'user', text: '[Set up the credential now.]' }, + ], + agent, + logger, + }); + + const response = await proxy.respondToConfirmation( + credentialEventWithRequests('req-cred-ambiguous', [ + { + credentialType: 'slackApi', + existingCredentials: [ + { id: 'cred-personal', name: 'Personal Slack' }, + { id: 'cred-team', name: 'Team Slack' }, + ], + }, + ]), + ); + + expect(response.kind).toBe('approval'); + if (response.kind === 'approval') { + expect(response.approved).toBe(false); + } + expect(logger.warn).toHaveBeenCalled(); + }); + + it("credentials(action='setup'): declines manual selection when the requested type has no existing credential and no credentialCreation is configured", async () => { + const agent = new FakeAgent(); + agent.enqueue({ action: 'choose_credential_setup_option', option: 'manual' }); + const logger = fakeLogger(); + const proxy = new UserProxyLlm({ + conversation: [ + { role: 'user', text: 'Post to Slack every morning.' }, + { role: 'user', text: '[Set up the Slack credential now.]' }, + ], + agent, + logger, + }); + + const response = await proxy.respondToConfirmation( + credentialEventWithRequests('req-cred-none', [{ credentialType: 'slackApi' }]), + ); + + expect(response.kind).toBe('approval'); + if (response.kind === 'approval') { + expect(response.approved).toBe(false); + } + expect(logger.warn).toHaveBeenCalled(); + }); + + it("credentials(action='setup'): manual creates a real credential when the requested type has zero existing candidates", async () => { + const agent = new FakeAgent(); + agent.enqueue({ action: 'choose_credential_setup_option', option: 'manual' }); + const { client, createCredential, setThreadCredentialAllowlist } = + fakeCredentialClient('cred-fresh'); + const proxy = new UserProxyLlm({ + conversation: [ + { role: 'user', text: 'Post to Slack every morning.' }, + { role: 'user', text: '[Set up the Slack credential now.]' }, + ], + agent, + credentialCreation: { client, threadId: 'thread-1', allowlistedCredentialIds: [] }, + }); + + const response = await proxy.respondToConfirmation( + credentialEventWithRequests('req-cred-create', [{ credentialType: 'slackApi' }]), + ); + + expect(createCredential).toHaveBeenCalledWith( + expect.any(String), + 'slackApi', + expect.any(Object), + ); + expect(setThreadCredentialAllowlist).toHaveBeenCalledWith('thread-1', ['cred-fresh']); + expect(response.kind).toBe('credentialSelection'); + if (response.kind === 'credentialSelection') { + expect(response.credentials).toEqual({ slackApi: 'cred-fresh' }); + } + }); + + it("credentials(action='setup'): requests automatic setup when the agent picks auto", async () => { + const agent = new FakeAgent(); + agent.enqueue({ + action: 'choose_credential_setup_option', + option: 'auto', + credentialType: 'slackApi', + }); + const proxy = new UserProxyLlm({ + conversation: [ + { role: 'user', text: 'Post to Slack every morning.' }, + { role: 'user', text: '[Ask for automatic setup of the Slack credential on the card.]' }, + ], + agent, + }); + + const response = await proxy.respondToConfirmation( + credentialEventWithRequests('req-cred-auto', [ + { credentialType: 'slackApi', existingCredentials: [{ id: 'cred-1', name: 'My Slack' }] }, + ]), + ); + + expect(response.kind).toBe('credentialAutoSetup'); + if (response.kind === 'credentialAutoSetup') { + expect(response.credentialType).toBe('slackApi'); + } + }); + + it("credentials(action='setup'): declines auto setup when no credentialType can be resolved from context or the decision", async () => { + const agent = new FakeAgent(); + agent.enqueue({ action: 'choose_credential_setup_option', option: 'auto' }); + const logger = fakeLogger(); + const proxy = new UserProxyLlm({ + conversation: [ + { role: 'user', text: 'Summarize Notion pages to Slack.' }, + { role: 'user', text: '[Ask for automatic setup of the credential on the card.]' }, + ], + agent, + logger, + }); + + const response = await proxy.respondToConfirmation( + credentialEventWithRequests('req-cred-auto-ambiguous', [ + { credentialType: 'slackApi', existingCredentials: [{ id: 'cred-slack', name: 'Slack' }] }, + { + credentialType: 'notionApi', + existingCredentials: [{ id: 'cred-notion', name: 'Notion' }], + }, + ]), + ); + + expect(response.kind).toBe('approval'); + if (response.kind === 'approval') { + expect(response.approved).toBe(false); + } + expect(logger.warn).toHaveBeenCalled(); + }); + + it("credentials(action='setup'): declines when the agent picks skip", async () => { + const agent = new FakeAgent(); + agent.enqueue({ action: 'choose_credential_setup_option', option: 'skip' }); + const proxy = new UserProxyLlm({ + conversation: [ + { role: 'user', text: 'Post to Slack every morning.' }, + { role: 'user', text: '[Explicitly decline the credential setup card for Slack.]' }, + ], + agent, + }); + + const response = await proxy.respondToConfirmation( + credentialEventWithRequests('req-cred-skip', [ + { credentialType: 'slackApi', existingCredentials: [{ id: 'cred-1', name: 'My Slack' }] }, + ]), + ); + + expect(response.kind).toBe('approval'); + if (response.kind === 'approval') { + expect(response.approved).toBe(false); + } + }); + + // ------------------------------------------------------------------------- + // TRUST-349 PR review: the engagement gate was originally a keyword-scoped + // regex (CREDENTIAL_ENGAGEMENT_PATTERN), rejected after a corpus audit found + // it both misfires (matches "API key"/"credential" inside a note that + // explicitly declines engagement) and can't be trusted to infer intent from + // keyword presence alone. Replaced with the same content-agnostic + // "any pending stage direction" check domain access and plan review already + // use (`hasPendingStageDirection`) — the model, not a regex, now decides + // whether a pending note means "engage" or "decline". + // ------------------------------------------------------------------------- + + it("credentials(action='setup'): routes to the agent whenever any stage direction is pending, regardless of content", async () => { + const agent = new FakeAgent(); + agent.enqueue({ action: 'choose_credential_setup_option', option: 'skip' }); + const proxy = new UserProxyLlm({ + conversation: [ + { role: 'user', text: 'Post to Slack every morning.' }, + // No credential/OAuth/connect vocabulary at all — the old + // keyword-scoped gate would have left this deterministic. + { role: 'user', text: '[Reject the plan unless it sorts descending by count.]' }, + ], + agent, + }); + + await proxy.respondToConfirmation(credentialEvent('req-any-pending-direction')); + expect(agent.callCount).toBe(1); + }); + + it("credentials(action='setup'): stays fully deterministic when no stage direction is pending", async () => { + const agent = new FakeAgent(); + const proxy = new UserProxyLlm({ + conversation: [{ role: 'user', text: 'Post to Slack every morning.' }], + agent, + }); + + const response = await proxy.respondToConfirmation(credentialEvent('req-no-pending-direction')); + expect(response.kind).toBe('credentialSelection'); + if (response.kind === 'credentialSelection') { + expect(response.credentials).toEqual({}); + } + expect(agent.callCount).toBe(0); + }); + + it.each([ + "[Stay impatient and hands-off for the whole conversation. If the agent asks you to choose or specify any detail — where to store the orders, the schema, field mappings, which service — don't engage with the specifics.]", + "[If the agent asks for the API key value: don't provide it — say you'll fill it into the credential yourself later. Approve plans/confirmations otherwise.]", + ])( + "credentials(action='setup'): defers correctly on a real, unrelated-or-declining stage direction (%j) — the model is consulted (content-agnostic gate) but still ends up deferred", + async (note) => { + const agent = new FakeAgent(); + agent.enqueue({ action: 'choose_credential_setup_option', option: 'skip' }); + const proxy = new UserProxyLlm({ + conversation: [ + { role: 'user', text: 'Post to Slack every morning.' }, + { role: 'user', text: note }, + ], + agent, + }); + + const response = await proxy.respondToConfirmation(credentialEvent('req-defers-correctly')); + // The gate no longer filters on wording, so the model is consulted... + expect(agent.callCount).toBe(1); + // ...but correctly reads intent and defers, same end result the old + // keyword-scoped gate aimed for without the false-positive risk. + expect(response.kind).toBe('approval'); + if (response.kind === 'approval') { + expect(response.approved).toBe(false); + } + }, + ); + it('handles domain-access events deterministically with allow_all', async () => { const agent = new FakeAgent(); const proxy = new UserProxyLlm({ @@ -613,7 +1429,7 @@ describe('UserProxyLlm.respondToConfirmation', () => { expect(agent.callCount).toBe(0); }); - it('routes setup-wizard events to the agent even when they include credentialRequests', async () => { + it("workflows(action='setup'): routes to the agent even when the payload also includes credentialRequests (setupRequests takes priority)", async () => { const agent = new FakeAgent(); agent.enqueue({ action: 'apply_setup_wizard', diff --git a/packages/@n8n/instance-ai/evaluations/credentials/seeder.ts b/packages/@n8n/instance-ai/evaluations/credentials/seeder.ts index 5df947ffa94..10e1e96c336 100644 --- a/packages/@n8n/instance-ai/evaluations/credentials/seeder.ts +++ b/packages/@n8n/instance-ai/evaluations/credentials/seeder.ts @@ -112,6 +112,47 @@ export interface CreatedCredential { type: string; } +/** + * Create a single credential of the given type. Throws on an unknown type and + * on creation failure — callers decide what a failure means for their flow + * (declared-credential seeding fails the build; a mid-run "create" decision + * falls back to decline, see `user-proxy/tools.ts`). + * + * `usedNames` de-dupes display names across calls that share it (e.g. every + * declared credential in one `createDeclaredCredentials` batch) by appending + * `#2`, `#3`, ... — pass a fresh `Map` for an unrelated, independent batch. + */ +export async function createOneCredential( + client: N8nClient, + credentialType: string, + name: string | undefined, + usedNames: Map, + options?: { logger?: EvalLogger }, +): Promise { + const template = CREDENTIAL_TEMPLATES[credentialType]; + if (!template) { + throw new Error( + `No credential template for type "${credentialType}" — add one to evaluations/credentials/seeder.ts`, + ); + } + + const base = name ?? template.defaultName; + const count = (usedNames.get(base) ?? 0) + 1; + usedNames.set(base, count); + const resolvedName = count > 1 ? `${base} #${count}` : base; + + const envToken = template.envVar ? process.env[template.envVar] : undefined; + const token = envToken ?? PLACEHOLDER_TOKEN; + options?.logger?.verbose(` Creating credential ${resolvedName} (${credentialType})`); + // No retry: a credential POST isn't idempotent, so retrying after a lost response would orphan a duplicate we never capture for cleanup. + const { id } = await client.createCredential( + resolvedName, + credentialType, + template.buildData(token), + ); + return { id, name: resolvedName, type: credentialType }; +} + /** * Create the credentials a test case declares. Throws on unknown types and on * creation failures — declared credentials are load-bearing for the case's @@ -120,36 +161,31 @@ export interface CreatedCredential { * `onCreated` fires per credential as it is created (not only on full * success), so the caller can register every ID for cleanup even when a later * creation throws. + * + * `nameCounts` defaults to a fresh, call-scoped `Map` — pass one in (and reuse + * it for a later `createOneCredential` call, e.g. `UserProxyLlm`'s mid-run + * credential creation) so a credential created mid-run doesn't collide on + * display name with one declared and seeded here (both would otherwise be + * "[eval] Slack" with no `#2` suffix, since the counters wouldn't know about + * each other — see TRUST-349 PR review). */ export async function createDeclaredCredentials( client: N8nClient, declared: TestCaseCredential[], - options?: { onCreated?: (id: string) => void; logger?: EvalLogger }, + options?: { + onCreated?: (id: string) => void; + logger?: EvalLogger; + nameCounts?: Map; + }, ): Promise { const logger = options?.logger; const created: CreatedCredential[] = []; - const nameCounts = new Map(); + const nameCounts = options?.nameCounts ?? new Map(); for (const decl of declared) { - const template = CREDENTIAL_TEMPLATES[decl.type]; - if (!template) { - throw new Error( - `No credential template for type "${decl.type}" — add one to evaluations/credentials/seeder.ts`, - ); - } - - const base = decl.name ?? template.defaultName; - const count = (nameCounts.get(base) ?? 0) + 1; - nameCounts.set(base, count); - const name = count > 1 ? `${base} #${count}` : base; - - const envToken = template.envVar ? process.env[template.envVar] : undefined; - const token = envToken ?? PLACEHOLDER_TOKEN; - logger?.verbose(` Creating credential ${name} (${decl.type})`); - // No retry: a credential POST isn't idempotent, so retrying after a lost response would orphan a duplicate we never capture for cleanup. - const { id } = await client.createCredential(name, decl.type, template.buildData(token)); - options?.onCreated?.(id); - created.push({ id, name, type: decl.type }); + const cred = await createOneCredential(client, decl.type, decl.name, nameCounts, { logger }); + options?.onCreated?.(cred.id); + created.push(cred); } return created; diff --git a/packages/@n8n/instance-ai/evaluations/harness/build-workflow.ts b/packages/@n8n/instance-ai/evaluations/harness/build-workflow.ts index a96c29001d5..67aabd7107e 100644 --- a/packages/@n8n/instance-ai/evaluations/harness/build-workflow.ts +++ b/packages/@n8n/instance-ai/evaluations/harness/build-workflow.ts @@ -91,6 +91,16 @@ interface MultiTurnDriverConfig { /** Appended to the FIRST sent message only (pre-seeded-table hint); the * recorded turn and the proxy's conversation keep the clean prompt. */ openingMessageSuffix?: string; + /** Ids already allowlisted for this thread (from pre-run `createDeclaredCredentials` + * seeding) — wires `UserProxyLlm.credentialCreation` so `manual` can create a + * real credential when a setup card shows zero existing candidates. Omitted + * when the credential view isn't pinned (see `credentialViewPinned`), since + * the allowlist endpoint isn't available in that case either. */ + allowlistedCredentialIds?: string[]; + createdCredentialIds?: Set; + /** Shared with `createDeclaredCredentials`'s pre-run seeding — see + * `CredentialCreationConfig.nameCounts`. */ + credentialNameCounts?: Map; } async function driveMultiTurnConversation( @@ -102,6 +112,17 @@ async function driveMultiTurnConversation( conversation: config.conversation, messageBudget: config.messageBudget, logger: config.logger, + ...(config.allowlistedCredentialIds !== undefined + ? { + credentialCreation: { + client: config.client, + threadId: config.threadId, + allowlistedCredentialIds: config.allowlistedCredentialIds, + createdCredentialIds: config.createdCredentialIds, + nameCounts: config.credentialNameCounts, + }, + } + : {}), }); const confirmationStrategy: ConfirmationStrategy = proxy.respondToConfirmation.bind(proxy); @@ -323,9 +344,14 @@ export async function buildWorkflow(config: BuildWorkflowConfig): Promise(); const createdCredentials = await createDeclaredCredentials(client, declaredCredentials, { onCreated: (id) => config.createdCredentialIds?.add(id), logger, + nameCounts: credentialNameCounts, }); try { await client.setThreadCredentialAllowlist( @@ -431,6 +457,16 @@ export async function buildWorkflow(config: BuildWorkflowConfig): Promise c.id), + createdCredentialIds: config.createdCredentialIds, + credentialNameCounts, + } + : {}), // The pre-seeded-table note goes to the agent, but the recorded turn // (and the graded transcript) keeps the clean user prompt. openingMessageSuffix: scenarioSeedTablesNote, diff --git a/packages/@n8n/instance-ai/evaluations/utils/confirmation-payload.ts b/packages/@n8n/instance-ai/evaluations/utils/confirmation-payload.ts index 28ee9242542..8af19900bf2 100644 --- a/packages/@n8n/instance-ai/evaluations/utils/confirmation-payload.ts +++ b/packages/@n8n/instance-ai/evaluations/utils/confirmation-payload.ts @@ -8,6 +8,21 @@ import type { InstanceAiConfirmRequest } from '@n8n/api-types'; import { getNestedRecord } from './safe-extract'; import type { CapturedEvent } from '../types'; +export interface InfrastructureResponseOptions { + /** + * TRUST-349: when true, a standalone credential-request event is left + * unhandled here (`undefined`) so the caller can route it to the LLM's + * `choose_credential_setup_option` action instead of the default deferral. + * Set only when a stage direction is still pending delivery — see + * `UserProxyLlm.hasPendingStageDirection` in `user-proxy/index.ts`, the + * same content-agnostic check domain access and plan review already use. + * Absent/false reproduces today's behavior exactly, so every existing case + * (and every other caller, e.g. `chat-loop.ts`'s `buildAutoApprovePayload`) + * is unaffected. + */ + allowCredentialEngagement?: boolean; +} + /** * Handle confirmation events that carry no user-intent signal — domain access, * web search, resource decisions, standalone credential requests. The eval @@ -17,6 +32,7 @@ import type { CapturedEvent } from '../types'; */ export function tryInfrastructureResponse( event: CapturedEvent, + options?: InfrastructureResponseOptions, ): InstanceAiConfirmRequest | undefined { const payload = getNestedRecord(event.data, 'payload') ?? {}; @@ -42,6 +58,7 @@ export function tryInfrastructureResponse( // the setup wizard takes priority because it carries node parameters to // fill (handled by the caller). if (Array.isArray(payload.credentialRequests) && !Array.isArray(payload.setupRequests)) { + if (options?.allowCredentialEngagement) return undefined; return { kind: 'credentialSelection', credentials: {} }; } diff --git a/packages/@n8n/instance-ai/evaluations/utils/user-proxy/deterministic.ts b/packages/@n8n/instance-ai/evaluations/utils/user-proxy/deterministic.ts index 91ae59a613a..b91f9cac431 100644 --- a/packages/@n8n/instance-ai/evaluations/utils/user-proxy/deterministic.ts +++ b/packages/@n8n/instance-ai/evaluations/utils/user-proxy/deterministic.ts @@ -3,23 +3,32 @@ import type { InstanceAiConfirmRequest } from '@n8n/api-types'; import type { CapturedEvent } from '../../types'; -import { getEventPayload, tryInfrastructureResponse } from '../confirmation-payload'; +import { + getEventPayload, + tryInfrastructureResponse, + type InfrastructureResponseOptions, +} from '../confirmation-payload'; export function tryDeterministicConfirmationResponse( event: CapturedEvent, + options?: InfrastructureResponseOptions, ): InstanceAiConfirmRequest | undefined { - const infra = tryInfrastructureResponse(event); + const infra = tryInfrastructureResponse(event, options); if (infra) return infra; const payload = getEventPayload(event); - // Setup wizard with credentials-only requests: skip. The eval has no - // credentials and applying an empty payload loops the agent ("partial 0/N"). + // Setup wizard with credentials-only requests: skip by default (the eval + // has no credentials to apply and an empty payload just loops the agent, + // "partial 0/N") — unless a stage direction asks the user to engage with + // this card (TRUST-349's `allowCredentialEngagement`), in which case fall + // through so the LLM's apply_setup_wizard can populate nodeCredentialsJson. // Mixed (credential + parameter issues, or parameter-only) → LLM fills params. if (Array.isArray(payload.setupRequests)) { if ( payload.setupRequests.length > 0 && - payload.setupRequests.every(isCredentialOnlySetupRequest) + payload.setupRequests.every(isCredentialOnlySetupRequest) && + !options?.allowCredentialEngagement ) { return { kind: 'approval', approved: false }; } diff --git a/packages/@n8n/instance-ai/evaluations/utils/user-proxy/index.ts b/packages/@n8n/instance-ai/evaluations/utils/user-proxy/index.ts index 9f2ffc237c1..13dc80c9895 100644 --- a/packages/@n8n/instance-ai/evaluations/utils/user-proxy/index.ts +++ b/packages/@n8n/instance-ai/evaluations/utils/user-proxy/index.ts @@ -6,7 +6,15 @@ import { isRecord } from '@n8n/utils/is-record'; import { createUserProxyAgent, type UserProxyAgent } from './agent'; import { tryDeterministicConfirmationResponse } from './deterministic'; import { buildConfirmationPrompt, buildFollowUpPrompt } from './prompts'; -import { encodeConfirmationDecision, type Decision, type SetupWizardParseContext } from './tools'; +import { + encodeConfirmationDecision, + type Decision, + type SetupWizardParseContext, + type CredentialSetupParseContext, + type CreateCredentialFn, +} from './tools'; +import type { N8nClient } from '../../clients/n8n-client'; +import { createOneCredential } from '../../credentials/seeder'; import { buildAutoApprovePayload } from '../../harness/chat-loop'; import type { NextMessageDecision } from '../../harness/chat-loop'; import type { EvalLogger } from '../../harness/logger'; @@ -14,6 +22,31 @@ import type { CapturedEvent, ConversationTurn } from '../../types'; import { getEventPayload } from '../confirmation-payload'; import { getNestedRecord, getString } from '../safe-extract'; +/** + * Lets `manual` create a real credential (TRUST-349) when a setup card shows + * zero existing candidates for the resolved type — "user fills the New + * Credential modal". Omit for cases that don't exercise credential-setup + * engagement; `manual` then declines with zero candidates instead of crashing. + */ +export interface CredentialCreationConfig { + client: N8nClient; + threadId: string; + /** Ids already allowlisted for this thread (from pre-run seeding via + * `createDeclaredCredentials`) — required because + * `setThreadCredentialAllowlist` REPLACES the whole list, so a mid-run + * creation must include these or it clobbers the case's declared set. */ + allowlistedCredentialIds: string[]; + /** Run-level registry newly-created ids are added to for end-of-run cleanup. */ + createdCredentialIds?: Set; + /** Shared with the same `Map` passed to `createDeclaredCredentials` for this + * build's pre-run seeding, so a mid-run-created credential's display name + * gets the right `#2`/`#3` suffix instead of silently colliding with a + * declared credential of the same default name (e.g. two "[eval] Slack" + * credentials with no way to tell which one an agent picked). Defaults to + * a fresh, unshared `Map` if omitted. */ + nameCounts?: Map; +} + /** * What category of response the proxy sent for a confirmation event. * Mostly mirrors the `kind` of the InstanceAiConfirmRequest, with overlay @@ -54,6 +87,9 @@ export interface UserProxyConfig { logger?: EvalLogger; /** Test seam — inject a fake agent. */ agent?: UserProxyAgent; + /** Wire this in to let `manual` create a real credential when a setup card + * shows zero existing candidates — see `CredentialCreationConfig`. */ + credentialCreation?: CredentialCreationConfig; } // --------------------------------------------------------------------------- @@ -79,12 +115,25 @@ export class UserProxyLlm { private readonly sentScriptUserTurnIndexes = new Set(); private readonly decisionStats: ProxyDecisionStats = {}; + private readonly credentialCreation?: CredentialCreationConfig; + /** Mutable running copy of `credentialCreation.allowlistedCredentialIds` — + * grows as `createCredential` mints new ones, since the allowlist endpoint + * replaces the whole list rather than appending. */ + private allowlistedCredentialIds: string[]; + /** Defaults to a fresh Map when the caller doesn't share one from pre-run + * seeding — see `CredentialCreationConfig.nameCounts`. */ + private readonly createdCredentialNameCounts: Map; + constructor(config: UserProxyConfig) { this.script = config.conversation; this.messageBudget = config.messageBudget ?? DEFAULT_MESSAGE_BUDGET; this.logger = config.logger; this.agent = config.agent ?? createUserProxyAgent({ modelId: config.modelId, logger: config.logger }); + this.credentialCreation = config.credentialCreation; + this.allowlistedCredentialIds = config.credentialCreation?.allowlistedCredentialIds ?? []; + this.createdCredentialNameCounts = + config.credentialCreation?.nameCounts ?? new Map(); // Seed with the opener — the harness has already sent it. const opener = this.script[0]; this.actualTranscript = opener ? [{ role: opener.role, text: opener.text }] : []; @@ -130,7 +179,9 @@ export class UserProxyLlm { return this.responseByRequestId.get(requestId) ?? buildAutoApprovePayload(event); } - const det = tryDeterministicConfirmationResponse(event); + const det = tryDeterministicConfirmationResponse(event, { + allowCredentialEngagement: this.hasPendingStageDirection(), + }); if (det && !this.deferAccessGateToScript(event)) { this.bumpStat('deterministic'); return this.rememberResponse(requestId, det); @@ -150,13 +201,15 @@ export class UserProxyLlm { return this.rememberResponse(requestId, this.fallbackConfirmationResponse(event)); } - const encoded = encodeConfirmationDecision( + const encoded = await encodeConfirmationDecision( decision, (raw, parseError) => this.logger?.warn( - `[user-proxy] nodeParametersJson failed to parse (${String(parseError)}); raw=${raw.slice(0, 200)}`, + `[user-proxy] action=${decision.action} failed to encode (${String(parseError)}); raw=${raw.slice(0, 200)}`, ), extractSetupWizardParseContext(event), + extractCredentialSetupContext(event), + this.credentialCreation ? this.createCredential : undefined, ); if (!encoded) { this.logger?.warn( @@ -174,6 +227,33 @@ export class UserProxyLlm { this.decisionStats[category] = (this.decisionStats[category] ?? 0) + 1; } + /** + * Creates a real credential for `manual`'s "zero existing candidates" + * case, registers it for cleanup, and updates the thread's allowlist so + * both this and any later turn can see it. Arrow field (not a method) so + * it stays correctly bound when passed as a bare `CreateCredentialFn`. + */ + private createCredential: CreateCredentialFn = async (credentialType) => { + if (!this.credentialCreation) { + // encodeConfirmationDecision only receives this function at all when + // `this.credentialCreation` is set (see respondToConfirmation) — a + // throw here means that invariant broke, not a normal runtime failure. + throw new Error('createCredential invoked without a credentialCreation config'); + } + const { client, threadId, createdCredentialIds } = this.credentialCreation; + const created = await createOneCredential( + client, + credentialType, + undefined, + this.createdCredentialNameCounts, + { logger: this.logger }, + ); + createdCredentialIds?.add(created.id); + this.allowlistedCredentialIds = [...this.allowlistedCredentialIds, created.id]; + await client.setThreadCredentialAllowlist(threadId, this.allowlistedCredentialIds); + return created; + }; + /** Counts of proxy decisions by category. Read after the build completes. */ getDecisionStats(): Readonly { return { ...this.decisionStats }; @@ -267,6 +347,25 @@ export class UserProxyLlm { private deferAccessGateToScript(event: CapturedEvent): boolean { const payload = getEventPayload(event); if (!payload.domainAccess && !payload.webSearch) return false; + return this.hasPendingStageDirection(); + } + + /** + * Any stage direction still pending delivery — the one signal the harness + * uses everywhere to decide "consult the model instead of taking the + * deterministic default" (domain access, web search, plan review, and — as + * of TRUST-349 — credential-setup engagement below). Deliberately content- + * agnostic: a keyword-scoped variant was tried and rejected after a corpus + * audit found it both under- and over-fires (a note saying "don't provide + * the API key, fill it in yourself later" matched on "API key"/"credential" + * despite asking for the opposite of engagement — a word match can't tell + * what a note means, but the model reading the actual text can). The + * system prompt already instructs the model to keep deferring unless a + * pending note says otherwise, so routing every pending-direction case + * through it is the same bet already made for domain access and plan + * review, not a new one. + */ + private hasPendingStageDirection(): boolean { return this.remainingUserScriptTurns().some((turn) => hasStageDirection(turn.text)); } @@ -355,33 +454,85 @@ function extractRequestId(event: CapturedEvent): string | undefined { return getString(event.data, 'requestId'); } +/** + * Workflow setup wizard shows one `setupRequests[]` entry per (node, + * credentialType) combo, plus a separate param-only entry — so a node needing + * both a credential and parameter fixes can appear across multiple entries. + * Group by node name/id and merge each field in as encountered. + */ function extractSetupWizardParseContext(event: CapturedEvent): SetupWizardParseContext | undefined { const payload = getEventPayload(event); if (!Array.isArray(payload.setupRequests)) return undefined; - const nodes = payload.setupRequests.flatMap((item) => { - if (!isRecord(item)) return []; + const byNodeName = new Map(); + + for (const item of payload.setupRequests) { + if (!isRecord(item)) continue; const node = isRecord(item.node) ? item.node : undefined; const nodeName = (node ? getString(node, 'name') : undefined) ?? getString(item, 'nodeName'); - if (!nodeName) return []; + if (!nodeName) continue; const nodeId = (node ? getString(node, 'id') : undefined) ?? getString(item, 'nodeId'); + const existing = byNodeName.get(nodeName) ?? { + nodeName, + parameterNames: [], + credentialRequests: [], + }; + // A node can appear across multiple setupRequests[] entries (one per + // credential type, plus a param-only one); backfill nodeId from + // whichever entry actually carries it, not just the first one seen. + if (nodeId && !existing.nodeId) existing.nodeId = nodeId; + const parameterNames = [ + ...existing.parameterNames, ...extractParameterNames(item, 'editableParameters'), ...extractParameterNames(item, 'parameterRequests'), ...extractParameterIssueNames(item), ]; + existing.parameterNames = [...new Set(parameterNames)]; - return [ - { - ...(nodeId ? { nodeId } : {}), - nodeName, - parameterNames: [...new Set(parameterNames)], - }, - ]; + const credentialType = getString(item, 'credentialType'); + if (credentialType) { + existing.credentialRequests = [ + ...existing.credentialRequests, + { credentialType, existingCredentials: extractExistingCredentials(item) }, + ]; + } + + byNodeName.set(nodeName, existing); + } + + const nodes = [...byNodeName.values()]; + return nodes.length > 0 ? { nodes } : undefined; +} + +function extractCredentialSetupContext( + event: CapturedEvent, +): CredentialSetupParseContext | undefined { + const payload = getEventPayload(event); + if (!Array.isArray(payload.credentialRequests)) return undefined; + + const requests = payload.credentialRequests.flatMap((item) => { + if (!isRecord(item)) return []; + const credentialType = getString(item, 'credentialType'); + if (!credentialType) return []; + + return [{ credentialType, existingCredentials: extractExistingCredentials(item) }]; }); - return nodes.length > 0 ? { nodes } : undefined; + return requests.length > 0 ? { requests } : undefined; +} + +function extractExistingCredentials( + item: Record, +): Array<{ id: string; name: string }> { + if (!Array.isArray(item.existingCredentials)) return []; + return item.existingCredentials.flatMap((cred) => { + if (!isRecord(cred)) return []; + const id = getString(cred, 'id'); + const name = getString(cred, 'name'); + return id && name ? [{ id, name }] : []; + }); } function extractParameterNames(item: Record, key: string): string[] { diff --git a/packages/@n8n/instance-ai/evaluations/utils/user-proxy/prompts.ts b/packages/@n8n/instance-ai/evaluations/utils/user-proxy/prompts.ts index 5cb8b649c85..aabe71611f6 100644 --- a/packages/@n8n/instance-ai/evaluations/utils/user-proxy/prompts.ts +++ b/packages/@n8n/instance-ai/evaluations/utils/user-proxy/prompts.ts @@ -52,11 +52,13 @@ One exception: sometimes the dedicated node genuinely cannot do what the script ## One exception: credentials -Never set credentials. They're deferred and the user will configure them via the UI. Credentials are the one and only thing left blank. +Credentials stay deferred by default — never set one up on your own initiative. They're the one thing left blank unless a stage direction says otherwise, on either a standalone credential card OR a setup-wizard card's credential slot (see "Setup cards are not questions" below). + +The one exception: a stage direction governing this exact credential moment tells you to engage — set up now (creating a fresh credential if none exists yet, or picking one if some do), or use automatic setup. On a standalone credential card (payload has \`credentialRequests\`), follow it via \`choose_credential_setup_option\`. On a setup-wizard card's credential slot (payload has \`setupRequests\`, an entry with \`credentialType\`), follow it via \`apply_setup_wizard\`'s \`nodeCredentialsJson\`. Absent such a direction, keep deferring exactly as always. ## Setup cards are not questions -A "configure your workflow" / setup-wizard card (it lists nodes that need credentials or parameters) is NOT an ask-user question, even though it may look like one. Fill its non-credential parameters with \`apply_setup_wizard\`. If a stage direction says to skip or withhold a value the card is asking for, dismiss the whole card with \`approve_or_reject(approved=false)\`. Never answer a setup card with \`answer_questions\`. +A "configure your workflow" / setup-wizard card (it lists nodes that need credentials or parameters) is NOT an ask-user question, even though it may look like one. Fill its non-credential parameters with \`apply_setup_wizard\`'s \`nodeParametersJson\`. Its credential slots stay deferred by default — same rule as any other credential moment (see "One exception: credentials" above) — unless a stage direction governing this exact card asks you to engage, in which case also set \`nodeCredentialsJson\`: reference an id from that slot's \`existingCredentials\` when any exist, or just fill in any value when the list is empty — a fresh credential is created for that slot automatically. If a stage direction says to skip or withhold a parameter value the card is asking for, dismiss the whole card with \`approve_or_reject(approved=false)\`. Never answer a setup card with \`answer_questions\`. ## Pushing back on plans and summaries @@ -82,7 +84,7 @@ You'll be given a SCRIPT (what the user wants overall) and the ACTUAL CONVERSATI - If the agent finished without asking and the plan was already approved or rejected appropriately, pick \`declare_done\`. Don't volunteer late script content as a proactive follow-up — the plan-rejection path is the right channel for steering. (Exception: a stage direction telling you to keep requesting changes overrides this — send the next change as a follow-up even after a successful build.) - When delivering a script user turn, adapt its wording so it reads as a real reply to the agent's last message — but keep every concrete value verbatim. - Don't restate what's already in the transcript. -- Credentials: if the agent stalls on credentials, send "I'll set them up later — please build without them." Do not provide credentials. +- Credentials: if the agent stalls on credentials, send "I'll set them up later — please build without them." Do not provide credentials — unless a stage direction governing this exact moment says to engage instead (see "One exception: credentials" above). ## Format diff --git a/packages/@n8n/instance-ai/evaluations/utils/user-proxy/tools.ts b/packages/@n8n/instance-ai/evaluations/utils/user-proxy/tools.ts index ab1e689a256..5c629a85524 100644 --- a/packages/@n8n/instance-ai/evaluations/utils/user-proxy/tools.ts +++ b/packages/@n8n/instance-ai/evaluations/utils/user-proxy/tools.ts @@ -26,6 +26,14 @@ const applySetupWizardDecisionSchema = z.object({ // JSON-encoded object mapping setup node name -> parameter map. Emitted as a string // because Anthropic structured output rejects nested z.record schemas. nodeParametersJson: z.string(), + /** + * JSON-encoded object mapping setup node name -> credential type -> existing + * credential id to select (`{"": {"": ""}}`), + * e.g. from `setupRequests[].existingCredentials`. Omit/empty by default — + * only populate when a stage direction governing this exact card asks the + * user to engage with a credential slot instead of leaving it deferred. + */ + nodeCredentialsJson: z.string().optional(), }); const approveOrRejectDecisionSchema = z.object({ @@ -44,6 +52,69 @@ const pickResourceDecisionSchema = z.object({ decision: z.string(), }); +/** + * Response to a standalone credential-setup card (`credentials(action='setup')` + * suspending — TRUST-349). This action is always part of `confirmationDecisionSchema` + * and always listed in the tool descriptions, same as `approve_or_reject` — + * it isn't conditionally offered. What's gated is whether the *event* ever + * reaches the model at all: the deterministic default (no pending stage + * direction) short-circuits before the LLM is even called (see + * `confirmation-payload.ts`'s `allowCredentialEngagement`), so in practice the + * model only ever sees this event when a direction is already pending — `skip` + * exists for the case where that pending direction asks the user to decline + * rather than engage. + * + * A normal multi-node workflow BUILD never reaches this tool in practice — live + * testing found the builder routes credential resolution through the workflow + * setup wizard instead (`applySetupWizardDecisionSchema`'s `nodeCredentialsJson` + * below). This tool *is* reached by a standalone credential-connect request with + * no build attached (e.g. "connect my Slack account now, before I build + * anything") — confirmed live against a real instance, all three outcomes: + * + * Live-captured suspend (`credentials(action='setup')` call args): + * ```json + * { "action": "setup", "credentials": [{ "credentialType": "slackApi", "reason": "...", "suggestedName": "Slack account" }] } + * ``` + * + * - `manual` → `{kind:'credentialSelection', credentials:{[type]: id}}` — the + * resume payload itself is how the assistant learns the credential exists + * (tool State 5); no re-check round-trip. Live-captured tool result (one + * existing credential, auto-selected): `{success:true, credentials:{slackApi:"eg_slackapi_key"}}`. + * `manual` covers all three existing-credential counts for the resolved + * type (ticket TRUST-349): + * - zero → the harness creates a real credential for it + * (`UserProxyConfig.credentialCreation`) and selects the new id — + * "user fills the New Credential modal". Falls back to decline (with a + * parse-failure log) if credential-creation support isn't wired in. + * - one → that credential is selected automatically; `existingCredentialId` + * is not needed. + * - many → `existingCredentialId` is required to disambiguate which one + * the direction names; omitting it declines (ambiguous). + * - `auto` → `{kind:'credentialAutoSetup', credentialType}` — triggers an + * agent rebuild server-side (tool State 4). Live-captured tool result: + * `{success:false, needsBrowserSetup:true, credentialType:"slackApi", docsUrl:"...", requiredFields:[...]}`, + * followed by the assistant loading the `credential-setup-with-computer-use` + * skill as designed. Reachable and its shape is real, but not further + * implemented — the harness has no Computer Use tools attached, so a case + * scripting this will stall afterward. Do not push such a case to the + * gated CI suite. + * - `skip` → `{kind:'approval', approved:false}` — tool State 2 (deferred). + * Live-captured tool result: `{success:true, deferred:true, reason:"User skipped credential setup for now...."}`. + */ +const chooseCredentialSetupOptionDecisionSchema = z.object({ + action: z.literal('choose_credential_setup_option'), + option: z.enum(['auto', 'manual', 'skip']), + /** Which `credentialRequests[].credentialType` this applies to. Optional + * when the card requests exactly one credential (the common case). */ + credentialType: z.string().optional(), + /** For `manual` when the card lists more than one existing credential of + * the resolved type — the `id` of the one to select (from + * `credentialRequests[].existingCredentials[].id`). Required to + * disambiguate when there are several; not needed for zero (created) or + * one (auto-selected) candidates. */ + existingCredentialId: z.string().optional(), +}); + const sendFollowUpMessageDecisionSchema = z.object({ action: z.literal('send_follow_up_message'), message: z.string(), @@ -70,6 +141,7 @@ export const confirmationDecisionSchema = z.discriminatedUnion('action', [ approveOrRejectDecisionSchema, respondToDomainAccessDecisionSchema, pickResourceDecisionSchema, + chooseCredentialSetupOptionDecisionSchema, ]); export const userTurnDecisionSchema = z.discriminatedUnion('action', [ @@ -85,6 +157,7 @@ export const decisionSchema = z.discriminatedUnion('action', [ approveOrRejectDecisionSchema, respondToDomainAccessDecisionSchema, pickResourceDecisionSchema, + chooseCredentialSetupOptionDecisionSchema, sendFollowUpMessageDecisionSchema, declareDoneDecisionSchema, ]); @@ -96,6 +169,30 @@ export interface SetupWizardParseContext { nodeId?: string; nodeName: string; parameterNames: string[]; + /** + * Credential types this node still needs (workflow setup wizard shows one + * `setupRequests[]` entry per (node, credentialType) combo), each with its + * existing-credential pick list — mirrors `CredentialSetupParseContext` + * below but scoped per node, since a wizard card can list several nodes. + */ + credentialRequests: Array<{ + credentialType: string; + existingCredentials: Array<{ id: string; name: string }>; + }>; + }>; +} + +/** + * The credential-setup card's `credentialRequests[]`, carried through so + * `manual`/`auto` can resolve a `credentialType` (and, for `manual`, an + * existing credential id already visible under the thread's eval allowlist — + * see `EvalThreadCredentialAllowlistService` — to select) without re-deriving + * it from the model's free-form answer. + */ +export interface CredentialSetupParseContext { + requests: Array<{ + credentialType: string; + existingCredentials: Array<{ id: string; name: string }>; }>; } @@ -107,13 +204,15 @@ export const CONFIRMATION_TOOL_DESCRIPTIONS = `Available actions — confirmatio - answer_questions(answers[]): The agent fired an ask-user confirmation (inputType=questions). Answer every question with a plausible value — stated → implied → invented. Invent rather than skip. Set skipped=true only when the question has no plausible answer of any shape, OR when a [stage direction] in the script tells the user to decline or withhold that value — in that case you MUST set skipped=true with an empty selectedOptions and pick NO option (not even one that looks standard or obvious); picking a value defeats the test. -- apply_setup_wizard(nodeParametersJson): The agent fired a setup-wizard / "configure your workflow" setup card with placeholder parameters. Emit a JSON string that decodes to { "": { "": , ... }, ... }. Fill every non-credential placeholder with a plausible value — stated → implied → invented. Never set credentials. This is the ONLY correct way to fill a setup card — do NOT answer it with answer_questions. To deliberately leave a value unset (e.g. a stage direction says the user skips it), dismiss the whole card with approve_or_reject(approved=false) instead of filling it. +- apply_setup_wizard(nodeParametersJson, nodeCredentialsJson?): The agent fired a setup-wizard / "configure your workflow" setup card with placeholder parameters and/or credential slots (the event's payload has \`setupRequests\`). \`nodeParametersJson\` decodes to { "": { "": , ... }, ... } — fill every non-credential placeholder with a plausible value (stated → implied → invented). Credential slots (a request entry with \`credentialType\`) stay unset by default — omit \`nodeCredentialsJson\` or leave that node/type out of it — UNLESS a stage direction governing this exact card tells the user to engage; then set \`nodeCredentialsJson\` to { "": { "": "" } }. What \`\` should be depends on that request's \`existingCredentials\`: zero entries → put any placeholder string, a real credential will be created for you; exactly one → put its \`id\`; two or more → put the \`id\` of the one the direction names (match by its \`name\`). This is the ONLY correct way to fill a setup card — do NOT answer it with answer_questions. To deliberately leave a value unset (e.g. a stage direction says the user skips it), dismiss the whole card with approve_or_reject(approved=false) instead of filling it. - approve_or_reject(approved, userInput?): A plan-review or free-text confirmation widget is on screen (the event's inputType is plan-review or text). Approve if the plan matches user intent; reject with reason if it diverges. This action only exists as a response to such a widget. - respond_to_domain_access(response): The agent is asking permission to reach the network — either a specific domain (fetch-url) or a web search. Pick allow_once, allow_all, or deny. Default to allow_all; pick deny ONLY when a [stage direction] tells the user to refuse this kind of access. -- pick_resource_decision(decision): The agent is asking the user to pick a gateway resource access option. Pick the option the user would choose.`; +- pick_resource_decision(decision): The agent is asking the user to pick a gateway resource access option. Pick the option the user would choose. + +- choose_credential_setup_option(option, credentialType?, existingCredentialId?): The agent opened a standalone credential setup card (the event's payload has \`credentialRequests\`, not \`setupRequests\`). You are only ever shown this action when a stage direction governs this exact moment — outside that, credentials stay deferred automatically and you never see this event. Follow the direction: \`manual\` fills the card the way a user filling the form would — check \`credentialRequests[].existingCredentials\` for the resolved type: zero entries → a real credential is created for you automatically, no \`existingCredentialId\` needed; exactly one → it's selected automatically, no \`existingCredentialId\` needed; two or more → set \`existingCredentialId\` to the \`id\` of the one the direction names (match by its \`name\`). \`auto\` hands off to automatic browser-based setup (shape-only — the harness cannot actually drive that flow, so only script this in a throwaway local check, never in a case meant for the gated suite). \`skip\` if the direction says to decline. Never pick this action on your own initiative — only in response to a direction that explicitly asks for credential engagement.`; export const USER_TURN_TOOL_DESCRIPTIONS = `Available actions — it is the user's turn. The agent finished its run, no widget is on screen, and the chat input is waiting. The user either types a message or ends the conversation: @@ -125,21 +224,64 @@ export const USER_TURN_TOOL_DESCRIPTIONS = `Available actions — it is the user // Decision → InstanceAiConfirmRequest encoders // --------------------------------------------------------------------------- +/** Creates a real credential of the given type for the "manual, zero existing" + * case — see `UserProxyConfig.credentialCreation` in `user-proxy/index.ts`. */ +export type CreateCredentialFn = (credentialType: string) => Promise<{ id: string; name: string }>; + +/** + * Shared safety net around `createCredential`: missing config and a thrown + * creation error (bad type, network failure, ...) both decline-and-log rather + * than crash the run — a failed credential creation is no different from any + * other unresolvable manual selection from the caller's point of view. + */ +async function tryCreateCredential( + createCredential: CreateCredentialFn | undefined, + credentialType: string, + actionLabel: string, + onFailure?: (raw: string, error: unknown) => void, +): Promise<{ id: string; name: string } | undefined> { + if (!createCredential) { + onFailure?.( + actionLabel, + new Error( + `no existing credential for type "${credentialType}" and no credential-creation support wired in`, + ), + ); + return undefined; + } + try { + return await createCredential(credentialType); + } catch (error) { + onFailure?.(actionLabel, error); + return undefined; + } +} + /** * Encode a confirmation-response action into an InstanceAiConfirmRequest. * Returns null for user-turn actions (send_follow_up_message, declare_done), * which the caller routes separately. */ -export function encodeConfirmationDecision( +export async function encodeConfirmationDecision( decision: Decision, onParseFailure?: (raw: string, error: unknown) => void, setupContext?: SetupWizardParseContext, -): InstanceAiConfirmRequest | null { + credentialSetupContext?: CredentialSetupParseContext, + createCredential?: CreateCredentialFn, +): Promise { switch (decision.action) { case 'answer_questions': return { kind: 'questions', answers: decision.answers }; - case 'apply_setup_wizard': + case 'apply_setup_wizard': { + const nodeCredentials = decision.nodeCredentialsJson + ? await parseNodeCredentialsJson( + decision.nodeCredentialsJson, + onParseFailure, + setupContext, + createCredential, + ) + : undefined; return { kind: 'setupWorkflowApply', nodeParameters: parseNodeParametersJson( @@ -147,7 +289,9 @@ export function encodeConfirmationDecision( onParseFailure, setupContext, ), + ...(nodeCredentials && Object.keys(nodeCredentials).length > 0 ? { nodeCredentials } : {}), }; + } case 'approve_or_reject': return { @@ -173,12 +317,208 @@ export function encodeConfirmationDecision( }; } + case 'choose_credential_setup_option': + return await encodeCredentialSetupDecision( + decision, + onParseFailure, + credentialSetupContext, + createCredential, + ); + case 'send_follow_up_message': case 'declare_done': return null; } } +async function encodeCredentialSetupDecision( + decision: Extract, + onParseFailure?: (raw: string, error: unknown) => void, + credentialSetupContext?: CredentialSetupParseContext, + createCredential?: CreateCredentialFn, +): Promise { + if (decision.option === 'skip') return { kind: 'approval', approved: false }; + + const request = resolveCredentialRequest(decision.credentialType, credentialSetupContext); + + if (decision.option === 'auto') { + const credentialType = request?.credentialType ?? decision.credentialType; + if (!credentialType) { + onParseFailure?.( + decision.action, + new Error('auto setup chosen with no resolvable credentialType'), + ); + return { kind: 'approval', approved: false }; + } + return { kind: 'credentialAutoSetup', credentialType }; + } + + // manual — covers all three existing-credential counts for the resolved type. + return await resolveManualCredentialSelection( + decision, + request, + onParseFailure, + createCredential, + ); +} + +/** + * `manual`'s three-way behavior (TRUST-349): zero existing credentials of the + * resolved type → create one for real; exactly one → auto-select it; + * several → `existingCredentialId` must disambiguate which one. + */ +async function resolveManualCredentialSelection( + decision: Extract, + request: CredentialSetupParseContext['requests'][number] | undefined, + onParseFailure?: (raw: string, error: unknown) => void, + createCredential?: CreateCredentialFn, +): Promise { + const credentialType = request?.credentialType ?? decision.credentialType; + + if (request && request.existingCredentials.length === 0) { + const created = await tryCreateCredential( + createCredential, + request.credentialType, + decision.action, + onParseFailure, + ); + if (!created) return { kind: 'approval', approved: false }; + return { kind: 'credentialSelection', credentials: { [request.credentialType]: created.id } }; + } + + const existingId = request + ? resolveExistingCredentialId(request, decision.existingCredentialId) + : undefined; + if (!request || !existingId) { + onParseFailure?.( + decision.action, + new Error( + `manual credential selection: no existing credential found for type "${credentialType ?? ''}"` + + (decision.existingCredentialId ? ` matching id "${decision.existingCredentialId}"` : ''), + ), + ); + return { kind: 'approval', approved: false }; + } + return { kind: 'credentialSelection', credentials: { [request.credentialType]: existingId } }; +} + +/** + * Which existing credential to select for `manual` when at least one exists. + * When the card lists more than one candidate for the resolved type, + * `existingCredentialId` disambiguates (matched against `existingCredentials[].id`); + * with a single candidate it's optional and that one is used regardless. + */ +function resolveExistingCredentialId( + request: CredentialSetupParseContext['requests'][number], + existingCredentialId: string | undefined, +): string | undefined { + if (existingCredentialId) { + return request.existingCredentials.find((c) => c.id === existingCredentialId)?.id; + } + return request.existingCredentials.length === 1 ? request.existingCredentials[0].id : undefined; +} + +function resolveCredentialRequest( + credentialType: string | undefined, + context: CredentialSetupParseContext | undefined, +): CredentialSetupParseContext['requests'][number] | undefined { + if (!context || context.requests.length === 0) return undefined; + if (credentialType) { + return context.requests.find((r) => r.credentialType === credentialType); + } + // No type specified — fine when the card only asked for one credential. + return context.requests.length === 1 ? context.requests[0] : undefined; +} + +/** + * Parse+validate `nodeCredentialsJson` against the wizard's parse context: + * every node key must be a known setup node, and every credential type must be + * one that node actually requested. Same three-way `manual` behavior as the + * standalone tool (TRUST-349): a (node, type) with existing candidates must + * name a real one from `existingCredentials` (auto-accepted when there's only + * one, regardless of the id string given, since there's nothing else it could + * be); zero candidates creates a real credential instead — the model has + * nothing to reference there, so any value in that slot signals intent to + * engage, not a specific id. Invalid/unresolvable entries are dropped with a + * parse-failure log rather than silently sending a bogus id through. + */ +async function parseNodeCredentialsJson( + json: string, + onFailure?: (raw: string, error: unknown) => void, + setupContext?: SetupWizardParseContext, + createCredential?: CreateCredentialFn, +): Promise>> { + let parsed: unknown; + try { + parsed = JSON.parse(json); + } catch (error) { + onFailure?.(json, error); + return {}; + } + if (!isRecord(parsed)) { + onFailure?.(json, new Error('parsed nodeCredentialsJson is not a plain object')); + return {}; + } + if (!setupContext || setupContext.nodes.length === 0) { + onFailure?.(json, new Error('nodeCredentialsJson supplied with no setup-wizard context')); + return {}; + } + + const nodeByAcceptedKey = new Map(); + for (const node of setupContext.nodes) { + nodeByAcceptedKey.set(node.nodeName, node); + if (node.nodeId) nodeByAcceptedKey.set(node.nodeId, node); + } + + const result: Record> = {}; + for (const [key, credsByType] of Object.entries(parsed)) { + const node = nodeByAcceptedKey.get(key); + if (!node || !isRecord(credsByType)) { + onFailure?.(json, new Error(`nodeCredentialsJson: unknown setup node "${key}"`)); + continue; + } + for (const [credentialType, credentialId] of Object.entries(credsByType)) { + const request = node.credentialRequests.find((r) => r.credentialType === credentialType); + if (!request) { + onFailure?.( + json, + new Error( + `nodeCredentialsJson: node "${key}" did not request credential type "${credentialType}"`, + ), + ); + continue; + } + + if (request.existingCredentials.length === 0) { + const created = await tryCreateCredential( + createCredential, + credentialType, + 'apply_setup_wizard', + onFailure, + ); + if (created) (result[node.nodeName] ??= {})[credentialType] = created.id; + continue; + } + + const match = + request.existingCredentials.length === 1 + ? request.existingCredentials[0] + : request.existingCredentials.find((c) => c.id === credentialId); + if (!match) { + onFailure?.( + json, + new Error( + `nodeCredentialsJson: no existing credential "${String(credentialId)}" of type "${credentialType}" for node "${key}"`, + ), + ); + continue; + } + (result[node.nodeName] ??= {})[credentialType] = match.id; + } + } + return result; +} + function parseNodeParametersJson( json: string, onFailure?: (raw: string, error: unknown) => void,