From 602f86c19cf3e68f5d300326671a1fed37b97563 Mon Sep 17 00:00:00 2001 From: Jose Date: Sat, 25 Jul 2026 16:37:15 +0100 Subject: [PATCH] fix(ai-builder): Stop the bypass-pin guard from replacing empty pins with a phantom item MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first smoke run proved the prompt+parse fix produced correct [] pins, which the Phase-1.5 guard then converted back into [{json:{}}] — its length===0 branch conflated "missing entry" (real-execution risk) with "intentionally empty" (the engine honors [] as pinned-zero-items via a presence check). Backfill only missing entries, and with [] instead of a phantom item. Co-Authored-By: Claude Fable 5 --- .../eval/__tests__/execution.service.test.ts | 23 +++++++++++++++++++ .../instance-ai/eval/execution.service.ts | 19 ++++++++------- 2 files changed, 32 insertions(+), 10 deletions(-) diff --git a/packages/cli/src/modules/instance-ai/eval/__tests__/execution.service.test.ts b/packages/cli/src/modules/instance-ai/eval/__tests__/execution.service.test.ts index 23dce50273a..0ba7a3f2213 100644 --- a/packages/cli/src/modules/instance-ai/eval/__tests__/execution.service.test.ts +++ b/packages/cli/src/modules/instance-ai/eval/__tests__/execution.service.test.ts @@ -462,6 +462,29 @@ describe('EvalExecutionService', () => { expect(workflowStaticDataService.saveStaticDataById).toHaveBeenCalledWith('wf-1', {}); }); + it('preserves an intentional zero-item bypass pin instead of injecting a phantom item', async () => { + const bypassNode = { + id: 'node-3', + name: 'Only New Jobs', + type: 'n8n-nodes-base.dataTable', + typeVersion: 1, + position: [400, 0], + parameters: {}, + } as INode; + workflowFinderService.findWorkflowForUser.mockResolvedValue( + makeWorkflowEntity({ nodes: [makeStartNode(), bypassNode] }) as never, + ); + identifyNodesForPinDataMock.mockReturnValue([bypassNode]); + generatePinDataMock.mockResolvedValue({ 'Only New Jobs': [] }); + + await service.executeWithLlmMock('wf-1', makeUser()); + + const runArg = workflowRunner.run.mock.calls[0][0] as unknown as { + pinData?: Record; + }; + expect(runArg.pinData?.['Only New Jobs']).toEqual([]); + }); + it('returns a framework failure when bypass pin data generation fails', async () => { const bypassNode = { id: 'node-3', diff --git a/packages/cli/src/modules/instance-ai/eval/execution.service.ts b/packages/cli/src/modules/instance-ai/eval/execution.service.ts index a5dbf78f4ce..054b0c244f5 100644 --- a/packages/cli/src/modules/instance-ai/eval/execution.service.ts +++ b/packages/cli/src/modules/instance-ai/eval/execution.service.ts @@ -303,19 +303,18 @@ export class EvalExecutionService { const normalized = normalizePinData(result as unknown as IPinData); - // generatePinData swallows internal failures (LLM timeout, parse error) - // and returns {} or a partial map instead of throwing, so the catch - // fallback below never fires for those. An unpinned bypass node - // EXECUTES for real — for AI roots the vendor SDK then makes real - // network calls (observed in CI: un-mocked Anthropic request → - // "Authorization failed"). Guarantee every bypass node is pinned, - // even if only with an empty item. + // A MISSING bypass entry would let the node execute for real — for AI + // roots the vendor SDK then makes real network calls (observed in CI: + // un-mocked Anthropic request → "Authorization failed"). An EMPTY array + // is different: the execution engine honors it as "pinned, zero items" + // (presence check, not length), and zero-item scenario premises depend + // on it — never replace [] with a phantom item (TRUST-343). for (const nodeName of bypassNodeNames) { - if (!normalized[nodeName] || normalized[nodeName].length === 0) { + if (!normalized[nodeName]) { this.logger.warn( - `[EvalMock] Phase 1.5 produced no pin data for bypass node "${nodeName}" — pinning an empty item to prevent real execution`, + `[EvalMock] Phase 1.5 produced no pin data for bypass node "${nodeName}" — pinning empty to prevent real execution`, ); - normalized[nodeName] = [{ json: {} }]; + normalized[nodeName] = []; } }