From f7f3f96804be86dcc5a24525349eeec02875673a Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Sat, 29 Aug 2026 12:38:48 -0700 Subject: [PATCH 1/4] fix(provenance): let a run that never started report why it failed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A copilot-run workflow that fails before reaching the engine crossed back with no provenance, which latched the tool's registry and reduced the result to "result unavailable". The caller was told its run failed but not that the workflow was undeployed, or the input invalid, or the slot unavailable — the reasons this layer produces before any block runs, naming no secret because none had been resolved yet. The executor attaches its execution result to every throw, so the absence of one is proof that no block ran: output, logs and error are all undefined and the only content is a message this layer wrote. That is an absence, not an inability to vouch, so the crossing now carries an exact-empty envelope. The message still passes the tool boundary's egress projection against the same registry, so anything that registry knows is still redacted. A run that did execute and could not vouch hands back its incomplete envelope exactly as before, and that still latches. Make the attach total rather than conditional to keep that inference sound. A block failure is already normalized on the way in, so the old `instanceof Error` guard held in practice; what it did not give was a guarantee covering a non-Error raised by the engine's own synchronous work. toError is identity-preserving, so ordinary failures keep their type. The empty envelope moves to the registry module, which owns the vocabulary, replacing a private copy in the logging session so one definition states what "vouched for, naming nothing" is. --- apps/sim/executor/execution/engine.test.ts | 34 ++++++++- apps/sim/executor/execution/engine.ts | 15 ++-- .../utils/resolved-secret-trace-registry.ts | 12 ++++ .../sim/lib/logs/execution/logging-session.ts | 5 +- .../run-workflow-from-copilot.test.ts | 71 +++++++++++++++++++ .../application/run-workflow-from-copilot.ts | 31 +++++--- 6 files changed, 149 insertions(+), 19 deletions(-) diff --git a/apps/sim/executor/execution/engine.test.ts b/apps/sim/executor/execution/engine.test.ts index 2677a9f89a6..d6cfa285bee 100644 --- a/apps/sim/executor/execution/engine.test.ts +++ b/apps/sim/executor/execution/engine.test.ts @@ -28,7 +28,7 @@ import { EDGE } from '@/executor/constants' import type { DAG, DAGNode } from '@/executor/dag/builder' import type { EdgeManager } from '@/executor/execution/edge-manager' import type { NodeExecutionOrchestrator } from '@/executor/orchestrators/node' -import type { ExecutionContext } from '@/executor/types' +import type { ExecutionContext, ExecutionResult } from '@/executor/types' import { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry' import type { SerializedBlock } from '@/serializer/types' import { ExecutionEngine } from './engine' @@ -275,6 +275,38 @@ describe('ExecutionEngine', () => { expect(provenance?.entries).toEqual([{ name: 'TOKEN', encryptedValue: 'ciphertext' }]) }) + /** + * The crossing at the copilot boundary reads the absence of an attached result as "no block + * ran". That inference is only total if every throw out of `run` carries one. + */ + it('attaches the execution result to every throw out of a failed run', async () => { + const node = createMockNode('function-1', 'function') + const registry = new ResolvedSecretTraceRegistry([ + { name: 'TOKEN', plaintext: 'secret-value-1234', encryptedValue: 'ciphertext' }, + ]) + registry.recordResolved('TOKEN', 'secret-value-1234') + const context = createMockContext({ + decisions: { router: new Map(), condition: new Map() }, + resolvedSecretTraceRegistry: registry, + }) + const nodeOrchestrator = createMockNodeOrchestrator() + vi.mocked(nodeOrchestrator.executeNode).mockRejectedValue(new Error('block exploded')) + + const engine = new ExecutionEngine( + context, + createMockDAG([node]), + createMockEdgeManager(), + nodeOrchestrator + ) + + const thrown = await engine.run(node.id).catch((error: unknown) => error) + + expect(thrown).toBeInstanceOf(Error) + const attached = (thrown as Error & { executionResult?: ExecutionResult }).executionResult + expect(attached).toBeDefined() + expect(attached?.executionState?.resolvedSecretTraceProvenance).toBeDefined() + }) + /** Deriving must not weaken the guarantee: a latched registry still exports incomplete. */ it('keeps the final output envelope incomplete when the registry latched', async () => { const node = createMockNode('loop-1', 'loop') diff --git a/apps/sim/executor/execution/engine.ts b/apps/sim/executor/execution/engine.ts index acfa7cfa42b..b6091f395e7 100644 --- a/apps/sim/executor/execution/engine.ts +++ b/apps/sim/executor/execution/engine.ts @@ -185,10 +185,17 @@ export class ExecutionEngine { metadata: this.context.metadata, } - if (error instanceof Error) { - attachExecutionResult(error, executionResult) - } - throw error + /** + * Normalized first so the attach is total rather than conditional on the throw already + * being an `Error`. A block failure is normalized on the way in, so the old guard held in + * practice; what it did not give was a guarantee. The copilot crossing reads a missing + * result as proof that no block ran, and that inference has to hold for every throw out of + * here, including a non-`Error` raised by this file's own synchronous work. `toError` + * returns an `Error` unchanged, so ordinary failures keep their identity and their type. + */ + const thrown = toError(error) + attachExecutionResult(thrown, executionResult) + throw thrown } finally { this.cleanup() } diff --git a/apps/sim/executor/utils/resolved-secret-trace-registry.ts b/apps/sim/executor/utils/resolved-secret-trace-registry.ts index 05e8d51cc71..432e0178a85 100644 --- a/apps/sim/executor/utils/resolved-secret-trace-registry.ts +++ b/apps/sim/executor/utils/resolved-secret-trace-registry.ts @@ -226,6 +226,18 @@ export interface ResolvedSecretIncompletenessDiagnostics { export const ANONYMOUS_SECRET_TRACE_REPLACEMENT = OPAQUE_RESOLVED_SECRET_REPLACEMENT export const RESOLVED_SECRET_TRACE_CHECKPOINT_VERSION = 1 +/** + * The envelope for content no secret ever reached: vouched for, naming nothing. + * + * Distinct from an incomplete envelope, which says the opposite — that something may be carried + * and cannot be named. A boundary that knows nothing was resolved should say so with this rather + * than latch, since latching is the claim that redaction is impossible. Returned fresh so no + * caller shares a value it may serialize or extend. + */ +export function emptyResolvedSecretTraceProvenance(): ResolvedSecretTraceProvenanceV1 { + return { version: 1, complete: true, entries: [] } +} + const MAX_PROVENANCE_ENTRIES = PROVENANCE_MAX_ENTRIES const MAX_SERIALIZED_PROVENANCE_BYTES = PROVENANCE_MAX_SERIALIZED_BYTES const MAX_TRACE_CATALOG_ENTRIES = PROVENANCE_MAX_ENTRIES diff --git a/apps/sim/lib/logs/execution/logging-session.ts b/apps/sim/lib/logs/execution/logging-session.ts index c2fe239248b..ece15ee9196 100644 --- a/apps/sim/lib/logs/execution/logging-session.ts +++ b/apps/sim/lib/logs/execution/logging-session.ts @@ -46,6 +46,7 @@ import type { SerializableExecutionState } from '@/executor/execution/types' import type { BlockLog } from '@/executor/types' import { projectResolvedSecretDiagnosticError } from '@/executor/utils/resolved-secret-content-projection' import { + emptyResolvedSecretTraceProvenance, isResolvedSecretTraceProvenanceV1, RESOLVED_SECRET_TRACE_CHECKPOINT_VERSION, type ResolvedSecretTraceProvenanceV1, @@ -124,10 +125,6 @@ function getActiveBlockDisplayProvenance( const logger = createLogger('LoggingSession') -function emptyResolvedSecretTraceProvenance(): ResolvedSecretTraceProvenanceV1 { - return { version: 1, complete: true, entries: [] } -} - type CompletionAttempt = 'complete' | 'error' | 'cancelled' | 'paused' export interface SecretSafeDisplayContent { diff --git a/apps/sim/lib/workflows/application/run-workflow-from-copilot.test.ts b/apps/sim/lib/workflows/application/run-workflow-from-copilot.test.ts index 593516912b4..8d3bf7a0fce 100644 --- a/apps/sim/lib/workflows/application/run-workflow-from-copilot.test.ts +++ b/apps/sim/lib/workflows/application/run-workflow-from-copilot.test.ts @@ -391,4 +391,75 @@ describe('Copilot workflow run application commands', () => { expect(readAttemptedExecutionId(error)).toBeUndefined() }) }) + + describe('failed-run provenance crossing', () => { + function trackingLifecycle() { + const importCrossingProvenance = vi.fn().mockResolvedValue(true) + return { + importCrossingProvenance, + lifecycle: { + resolvedSecretTraceRegistry: { + exportProvenanceForValue: vi.fn(() => undefined), + beginPendingActivation: vi.fn(() => vi.fn()), + importCrossingProvenance, + }, + }, + } + } + + async function runExpectingFailure(input: { lifecycle: unknown }) { + await expect( + runWorkflowFromCopilot.execute({ + principal, + input: { + workflowId: 'workflow-1', + useDraftState: true, + lifecycle: input.lifecycle, + hasWorkflowInput: false, + useMockPayload: true, + }, + }) + ).rejects.toThrow() + } + + /** + * The executor attaches its result to every throw, so a failure without one never reached a + * block. Nothing crossed, and saying so keeps the caller's tool result — and the reason its + * run could not start — instead of reducing it to "result unavailable". + */ + it('vouches for a failure that never reached the engine', async () => { + const { importCrossingProvenance, lifecycle: tracked } = trackingLifecycle() + mocks.executeWorkflow.mockRejectedValueOnce(new Error('workflow is not deployed')) + + await runExpectingFailure({ lifecycle: tracked }) + + expect(importCrossingProvenance).toHaveBeenCalledWith( + { version: 1, complete: true, entries: [] }, + expect.objectContaining({ thrownMessage: 'workflow is not deployed' }), + expect.objectContaining({ origin: 'copilotWorkflowMutation.failedRunCrossing' }) + ) + }) + + /** A run that did execute and could not vouch still hands back its incomplete envelope. */ + it('passes through an incomplete envelope from a run that did execute', async () => { + const { importCrossingProvenance, lifecycle: tracked } = trackingLifecycle() + const incomplete = { version: 1 as const, complete: false, entries: [] } + const failure = Object.assign(new Error('block failed'), { + executionResult: { + success: false, + output: { partial: true }, + executionState: { resolvedSecretTraceProvenance: incomplete }, + }, + }) + mocks.executeWorkflow.mockRejectedValueOnce(failure) + + await runExpectingFailure({ lifecycle: tracked }) + + expect(importCrossingProvenance).toHaveBeenCalledWith( + incomplete, + expect.objectContaining({ output: { partial: true } }), + expect.objectContaining({ origin: 'copilotWorkflowMutation.failedRunCrossing' }) + ) + }) + }) }) diff --git a/apps/sim/lib/workflows/application/run-workflow-from-copilot.ts b/apps/sim/lib/workflows/application/run-workflow-from-copilot.ts index 6467156f445..4aed63cf31c 100644 --- a/apps/sim/lib/workflows/application/run-workflow-from-copilot.ts +++ b/apps/sim/lib/workflows/application/run-workflow-from-copilot.ts @@ -29,11 +29,14 @@ import { } from '@/lib/workflows/triggers/run-options' import type { SerializableExecutionState } from '@/executor/execution/types' import type { ExecutionResult } from '@/executor/types' -import { attachAttemptedExecutionId } from '@/executor/utils/errors' +import { attachAttemptedExecutionId, hasExecutionResult } from '@/executor/utils/errors' const logger = createLogger('CopilotWorkflowRun') -import type { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry' +import { + emptyResolvedSecretTraceProvenance, + type ResolvedSecretTraceRegistry, +} from '@/executor/utils/resolved-secret-trace-registry' export interface CopilotWorkflowRunLifecycle { billingAttribution?: BillingAttributionSnapshot @@ -325,16 +328,24 @@ async function executeCopilotRun(params: { * as never started and invite the duplicate this id exists to prevent. */ if (registry) { - const executionResult = - typeof error === 'object' && - error !== null && - 'executionResult' in error && - typeof error.executionResult === 'object' - ? (error.executionResult as ExecutionResult) - : undefined + const executionResult = hasExecutionResult(error) ? error.executionResult : undefined try { + /** + * A run that never reached the engine carried nothing back: the executor attaches its + * result to every throw, so its absence means no block ran, and the three content fields + * below are all undefined. The only content is `thrownMessage`, which this layer produced + * — an admission, validation, or setup failure — and which the tool boundary still + * projects against this registry before any of it reaches a model. + * + * Reporting that as unvouchable cost the caller the reason its run failed: the crossing + * latched, and the tool result was reduced to "result unavailable" for a message that + * named no secret because none had been resolved yet. An engine that did run and could + * not vouch still hands back an incomplete envelope, and that still latches. + */ await registry.importCrossingProvenance( - executionResult?.executionState?.resolvedSecretTraceProvenance, + executionResult + ? executionResult.executionState?.resolvedSecretTraceProvenance + : emptyResolvedSecretTraceProvenance(), { output: executionResult?.output, logs: executionResult?.logs, From 60a77ccb264cf93d3a6d514acdad55248bb9567e Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Sat, 29 Aug 2026 12:57:04 -0700 Subject: [PATCH 2/4] fix(provenance): keep the post-run crossing window out of the never-started claim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review round 1, both findings accepted. The post-run crossing runs inside the same try as the executor call, so when that import is what throws, the catch sees an error carrying no execution result — the same evidence a run that never started leaves. The previous condition read that as "nothing crossed" and vouched for it, when in fact an execution exists and its provenance was never imported, which is exactly the content that cannot be vouched for. Record whether the executor returned and require both facts before claiming the absence: not past the executor, and no result attached. Everything else hands back whatever envelope it has, and an incomplete one still latches. The executor test also could not fail against the old gated attach: a block failure is normalized on the way in, so its rejection already arrived as an Error. Drive it through the cancellation subscribe run() awaits before the queue instead, which is its own synchronous work and reaches the catch untouched — the case the total attach exists for. --- apps/sim/executor/execution/engine.test.ts | 11 ++++--- .../run-workflow-from-copilot.test.ts | 31 +++++++++++++++++++ .../application/run-workflow-from-copilot.ts | 31 ++++++++++++------- 3 files changed, 56 insertions(+), 17 deletions(-) diff --git a/apps/sim/executor/execution/engine.test.ts b/apps/sim/executor/execution/engine.test.ts index d6cfa285bee..49163f6b3ad 100644 --- a/apps/sim/executor/execution/engine.test.ts +++ b/apps/sim/executor/execution/engine.test.ts @@ -277,9 +277,11 @@ describe('ExecutionEngine', () => { /** * The crossing at the copilot boundary reads the absence of an attached result as "no block - * ran". That inference is only total if every throw out of `run` carries one. + * ran", so the attach has to be total. A block failure is normalized on the way in, so only + * a non-Error raised by `run`'s own work — here the cancellation subscribe it awaits before + * the queue — reaches the catch untouched and exercises the guarantee. */ - it('attaches the execution result to every throw out of a failed run', async () => { + it('attaches the execution result to a non-Error thrown by its own work', async () => { const node = createMockNode('function-1', 'function') const registry = new ResolvedSecretTraceRegistry([ { name: 'TOKEN', plaintext: 'secret-value-1234', encryptedValue: 'ciphertext' }, @@ -289,14 +291,13 @@ describe('ExecutionEngine', () => { decisions: { router: new Map(), condition: new Map() }, resolvedSecretTraceRegistry: registry, }) - const nodeOrchestrator = createMockNodeOrchestrator() - vi.mocked(nodeOrchestrator.executeNode).mockRejectedValue(new Error('block exploded')) + mockIsExecutionCancelled.mockRejectedValueOnce('cancellation lookup exploded') const engine = new ExecutionEngine( context, createMockDAG([node]), createMockEdgeManager(), - nodeOrchestrator + createMockNodeOrchestrator() ) const thrown = await engine.run(node.id).catch((error: unknown) => error) diff --git a/apps/sim/lib/workflows/application/run-workflow-from-copilot.test.ts b/apps/sim/lib/workflows/application/run-workflow-from-copilot.test.ts index 8d3bf7a0fce..adcb61ced11 100644 --- a/apps/sim/lib/workflows/application/run-workflow-from-copilot.test.ts +++ b/apps/sim/lib/workflows/application/run-workflow-from-copilot.test.ts @@ -440,6 +440,37 @@ describe('Copilot workflow run application commands', () => { ) }) + /** + * The post-run crossing is inside the same try, so its failure reaches the catch with no + * execution result — the same evidence a never-started run leaves. An execution exists and + * its provenance was never imported, so this must not be vouched for. + */ + it('does not vouch when the crossing threw after the run returned', async () => { + const importCrossingProvenance = vi + .fn() + .mockImplementationOnce(() => { + throw new Error('crossing import failed') + }) + .mockResolvedValue(true) + + await runExpectingFailure({ + lifecycle: { + resolvedSecretTraceRegistry: { + exportProvenanceForValue: vi.fn(() => undefined), + beginPendingActivation: vi.fn(() => vi.fn()), + importCrossingProvenance, + }, + }, + }) + + expect(importCrossingProvenance).toHaveBeenNthCalledWith( + 2, + undefined, + expect.objectContaining({ thrownMessage: 'crossing import failed' }), + expect.objectContaining({ origin: 'copilotWorkflowMutation.failedRunCrossing' }) + ) + }) + /** A run that did execute and could not vouch still hands back its incomplete envelope. */ it('passes through an incomplete envelope from a run that did execute', async () => { const { importCrossingProvenance, lifecycle: tracked } = trackingLifecycle() diff --git a/apps/sim/lib/workflows/application/run-workflow-from-copilot.ts b/apps/sim/lib/workflows/application/run-workflow-from-copilot.ts index 4aed63cf31c..68f9884b0e2 100644 --- a/apps/sim/lib/workflows/application/run-workflow-from-copilot.ts +++ b/apps/sim/lib/workflows/application/run-workflow-from-copilot.ts @@ -253,6 +253,12 @@ async function executeCopilotRun(params: { params.executionInput ) const completePendingActivation = registry?.beginPendingActivation() + /** + * Whether the executor returned. The post-run crossing is inside the same `try`, so its own + * failure lands in the catch carrying no execution result — indistinguishable, on that + * evidence alone, from a run that never started. This records the difference the error cannot. + */ + let executed = false /** * The executor call is the first statement of this `try`, so everything caught below is * post-dispatch by construction, while authorization, admission and provenance export all @@ -305,6 +311,7 @@ async function executeCopilotRun(params: { }, childExecutionId ) + executed = true if (registry) { await registry.importCrossingProvenance( result.executionState?.resolvedSecretTraceProvenance, @@ -331,21 +338,21 @@ async function executeCopilotRun(params: { const executionResult = hasExecutionResult(error) ? error.executionResult : undefined try { /** - * A run that never reached the engine carried nothing back: the executor attaches its - * result to every throw, so its absence means no block ran, and the three content fields - * below are all undefined. The only content is `thrownMessage`, which this layer produced - * — an admission, validation, or setup failure — and which the tool boundary still - * projects against this registry before any of it reaches a model. + * A run that never started carried nothing back, and saying so keeps the caller's + * failure reason instead of reducing the tool result to "result unavailable" for a + * message that named no secret because none had been resolved yet. * - * Reporting that as unvouchable cost the caller the reason its run failed: the crossing - * latched, and the tool result was reduced to "result unavailable" for a message that - * named no secret because none had been resolved yet. An engine that did run and could - * not vouch still hands back an incomplete envelope, and that still latches. + * Both conditions are required to claim it. `executed` rules out the post-run window, + * where the crossing import is what threw: an execution exists and its provenance was + * never imported, so its content is exactly what cannot be vouched for. The absent + * execution result then rules out the engine having run at all — it attaches one to + * every throw. Anything else hands back whatever envelope it has, and an incomplete one + * still latches. */ await registry.importCrossingProvenance( - executionResult - ? executionResult.executionState?.resolvedSecretTraceProvenance - : emptyResolvedSecretTraceProvenance(), + !executed && !executionResult + ? emptyResolvedSecretTraceProvenance() + : executionResult?.executionState?.resolvedSecretTraceProvenance, { output: executionResult?.output, logs: executionResult?.logs, From 4260004d3a72e1d8ab2b6e53ff5c2c3c4a311534 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Sat, 29 Aug 2026 13:08:04 -0700 Subject: [PATCH 3/4] fix(provenance): carry the run's result through post-execution failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 2, cubic's finding accepted — and it was a distinct window, not a restatement of round 1. The executor's post-execution work runs after the run has produced a result but before `executeWorkflow` returns, so a failure there reached callers with no result attached: the run threw nothing itself, and the flag added last round could not be set yet. Every consumer that reads a missing result as "no block ran" was wrong in that window, this crossing included. Fix it where the result lives rather than at each reader. The executor attaches its own on the throws it raises; `executeWorkflow` now does the same for failures raised after it holds one, skipping the case the executor already recorded. Logging and trace spans get the same benefit for free — they read the identical signal. That makes an absent result total again, so the boolean flag goes and the crossing reads one thing: the result from the error, or the one already returned when the failure came later still, from the crossing itself. Only a failure with neither can claim nothing ran. The post-return case now describes content with the run's real envelope rather than latching blind, which is strictly more accurate than either prior behaviour. --- .../run-workflow-from-copilot.test.ts | 27 +++++++++++++ .../application/run-workflow-from-copilot.ts | 38 +++++++++---------- .../workflows/executor/execute-workflow.ts | 20 +++++++++- 3 files changed, 64 insertions(+), 21 deletions(-) diff --git a/apps/sim/lib/workflows/application/run-workflow-from-copilot.test.ts b/apps/sim/lib/workflows/application/run-workflow-from-copilot.test.ts index adcb61ced11..d8976c7264e 100644 --- a/apps/sim/lib/workflows/application/run-workflow-from-copilot.test.ts +++ b/apps/sim/lib/workflows/application/run-workflow-from-copilot.test.ts @@ -471,6 +471,33 @@ describe('Copilot workflow run application commands', () => { ) }) + /** + * The executor's post-execution work can throw after a run has already produced a result. + * `executeWorkflow` carries it on that throw, so this reaches the catch with a result and + * must not be claimed as never-started. + */ + it('does not vouch when post-execution work threw after the engine ran', async () => { + const { importCrossingProvenance, lifecycle: tracked } = trackingLifecycle() + const incomplete = { version: 1 as const, complete: false, entries: [] } + mocks.executeWorkflow.mockRejectedValueOnce( + Object.assign(new Error('post-execution persistence failed'), { + executionResult: { + success: true, + output: { ran: true }, + executionState: { resolvedSecretTraceProvenance: incomplete }, + }, + }) + ) + + await runExpectingFailure({ lifecycle: tracked }) + + expect(importCrossingProvenance).toHaveBeenCalledWith( + incomplete, + expect.objectContaining({ output: { ran: true } }), + expect.objectContaining({ origin: 'copilotWorkflowMutation.failedRunCrossing' }) + ) + }) + /** A run that did execute and could not vouch still hands back its incomplete envelope. */ it('passes through an incomplete envelope from a run that did execute', async () => { const { importCrossingProvenance, lifecycle: tracked } = trackingLifecycle() diff --git a/apps/sim/lib/workflows/application/run-workflow-from-copilot.ts b/apps/sim/lib/workflows/application/run-workflow-from-copilot.ts index 68f9884b0e2..31ed03811fc 100644 --- a/apps/sim/lib/workflows/application/run-workflow-from-copilot.ts +++ b/apps/sim/lib/workflows/application/run-workflow-from-copilot.ts @@ -254,11 +254,12 @@ async function executeCopilotRun(params: { ) const completePendingActivation = registry?.beginPendingActivation() /** - * Whether the executor returned. The post-run crossing is inside the same `try`, so its own - * failure lands in the catch carrying no execution result — indistinguishable, on that - * evidence alone, from a run that never started. This records the difference the error cannot. + * The run's own result, once the executor returns it. The post-run crossing below is inside the + * same `try`, so its failure reaches the catch carrying nothing — and on that evidence alone it + * is indistinguishable from a run that never started. Holding the result here keeps the real + * envelope available to describe content that certainly exists. */ - let executed = false + let runResult: ExecutionResult | undefined /** * The executor call is the first statement of this `try`, so everything caught below is * post-dispatch by construction, while authorization, admission and provenance export all @@ -311,7 +312,7 @@ async function executeCopilotRun(params: { }, childExecutionId ) - executed = true + runResult = result if (registry) { await registry.importCrossingProvenance( result.executionState?.resolvedSecretTraceProvenance, @@ -335,24 +336,23 @@ async function executeCopilotRun(params: { * as never started and invite the duplicate this id exists to prevent. */ if (registry) { - const executionResult = hasExecutionResult(error) ? error.executionResult : undefined + /** + * Either source counts as proof a run exists: the error carries the result when the run or + * its post-execution work threw, and `runResult` holds it when the failure came later still + * — from the crossing below, after the executor had already returned. + */ + const executionResult = hasExecutionResult(error) ? error.executionResult : runResult try { /** - * A run that never started carried nothing back, and saying so keeps the caller's - * failure reason instead of reducing the tool result to "result unavailable" for a - * message that named no secret because none had been resolved yet. - * - * Both conditions are required to claim it. `executed` rules out the post-run window, - * where the crossing import is what threw: an execution exists and its provenance was - * never imported, so its content is exactly what cannot be vouched for. The absent - * execution result then rules out the engine having run at all — it attaches one to - * every throw. Anything else hands back whatever envelope it has, and an incomplete one - * still latches. + * Only a failure with no result from either source can claim nothing ran, and saying so + * keeps the caller's failure reason instead of reducing the tool result to "result + * unavailable" for a message that named no secret because none had been resolved yet. + * Every other failure hands back the envelope it has, and an incomplete one still latches. */ await registry.importCrossingProvenance( - !executed && !executionResult - ? emptyResolvedSecretTraceProvenance() - : executionResult?.executionState?.resolvedSecretTraceProvenance, + executionResult + ? executionResult.executionState?.resolvedSecretTraceProvenance + : emptyResolvedSecretTraceProvenance(), { output: executionResult?.output, logs: executionResult?.logs, diff --git a/apps/sim/lib/workflows/executor/execute-workflow.ts b/apps/sim/lib/workflows/executor/execute-workflow.ts index 8336ea332d2..c62b872e510 100644 --- a/apps/sim/lib/workflows/executor/execute-workflow.ts +++ b/apps/sim/lib/workflows/executor/execute-workflow.ts @@ -13,6 +13,7 @@ import { handlePostExecutionPauseState } from '@/lib/workflows/executor/pause-pe import { ExecutionSnapshot } from '@/executor/execution/snapshot' import type { ExecutionMetadata, SerializableExecutionState } from '@/executor/execution/types' import type { ExecutionResult, StreamingExecution } from '@/executor/types' +import { attachExecutionResult, hasExecutionResult } from '@/executor/utils/errors' import type { ResolvedSecretTraceProvenanceV1 } from '@/executor/utils/resolved-secret-trace-registry' import type { CoreTriggerType } from '@/stores/logs/filters/types' @@ -128,6 +129,12 @@ export async function executeWorkflow( loggingSession.setTrustedExecutionCorrelation(streamConfig.trustedExecutionCorrelation) } let postExecutionOwnershipTransferred = false + /** + * Held outside the `try` so the catch can carry it. The executor attaches its result when the + * run itself throws, but the post-execution work below can throw after a run has already + * produced one — and callers read a missing result as proof that no block ran. + */ + let executionResult: ExecutionResult | undefined try { const metadata: ExecutionMetadata = { @@ -169,7 +176,7 @@ export async function executeWorkflow( const executionStartMs = Date.now() - const result = await executeWorkflowCore({ + const result = (executionResult = await executeWorkflowCore({ snapshot, callbacks: { onStream: streamConfig?.onStream, @@ -197,7 +204,7 @@ export async function executeWorkflow( trustedInitialResolvedSecretTraceProvenance: streamConfig?.trustedInitialResolvedSecretTraceProvenance, runFromBlock: streamConfig?.runFromBlock, - }) + })) const blockTypes = [ ...new Set( @@ -241,6 +248,15 @@ export async function executeWorkflow( return result } catch (error: unknown) { + /** + * Carries the run's result on a failure raised after it produced one — the post-execution + * work below the executor call can throw, and callers read a missing result as proof that no + * block ran. Skipped when the executor already attached its own, which is the more specific + * record, and when the throw is not an object to carry it. + */ + if (executionResult && error instanceof Error && !hasExecutionResult(error)) { + attachExecutionResult(error, executionResult) + } const errorDiagnostic = loggingSession.projectDiagnosticError(error) logger.error(`[${requestId}] Workflow execution failed`, errorDiagnostic) From 33673691b5499bf954ffd59b92be1a7f07549ac5 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Sat, 29 Aug 2026 13:16:27 -0700 Subject: [PATCH 4/4] fix(provenance): normalize a post-execution failure so it can carry the result MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 3, cubic's finding accepted. The guard added last round required the caught value to already be an `Error`, so a non-Error raised by post-execution work skipped the attach and was rethrown bare — the same hole this branch closed in the executor, left open one layer up by my own change. A Copilot run would have reported an executed workflow as never started and vouched for content it cannot describe. Normalize once at the top of the catch and use that value throughout, including the rethrow, matching what the executor does. `toError` returns an `Error` unchanged, so a custom error class keeps its identity and every ordinary failure is untouched — the existing identity assertion on the rejection path still holds. Two tests: the result reaches an ordinary post-execution failure, and a non-Error one is normalized so it can carry the result too. The second fails against the previous guard. --- .../executor/execute-workflow.test.ts | 39 +++++++++++++++++++ .../workflows/executor/execute-workflow.ts | 17 +++++--- 2 files changed, 51 insertions(+), 5 deletions(-) diff --git a/apps/sim/lib/workflows/executor/execute-workflow.test.ts b/apps/sim/lib/workflows/executor/execute-workflow.test.ts index 4240058f7d3..f68fadb8a19 100644 --- a/apps/sim/lib/workflows/executor/execute-workflow.test.ts +++ b/apps/sim/lib/workflows/executor/execute-workflow.test.ts @@ -56,6 +56,7 @@ vi.mock('@/lib/workflows/executor/pause-persistence', () => ({ })) import { executeWorkflow } from '@/lib/workflows/executor/execute-workflow' +import { hasExecutionResult } from '@/executor/utils/errors' const workflowExecutionLoggerCallIndex = loggerMock.createLogger.mock.calls.findIndex( ([name]) => name === 'WorkflowExecution' @@ -296,6 +297,44 @@ describe('executeWorkflow', () => { expect(executionSettled).toBe(true) }) + /** + * Post-execution work runs after the core has produced a result and the executor never sees + * its failure, so this layer is the only one that can carry the result onto it. Callers read a + * missing result as proof that no block ran — a Copilot run would report an executed workflow + * as never started and vouch for content it cannot describe. + */ + it('carries the execution result onto a post-execution failure', async () => { + const result = { success: true, output: { ran: true }, logs: [] } + executeWorkflowCoreMock.mockResolvedValueOnce(result) + handlePostExecutionPauseStateMock.mockRejectedValueOnce(new Error('pause persistence failed')) + + const thrown = await executeWorkflow(workflow, 'request-1', undefined, 'actor-1', { + enabled: true, + principal, + billingAttribution, + }).catch((error: unknown) => error) + + expect(hasExecutionResult(thrown)).toBe(true) + expect((thrown as { executionResult?: unknown }).executionResult).toBe(result) + }) + + /** A non-Error cannot carry the result, so it is normalized before anything reads it. */ + it('normalizes a non-Error post-execution failure so it can carry the result', async () => { + const result = { success: true, output: { ran: true }, logs: [] } + executeWorkflowCoreMock.mockResolvedValueOnce(result) + handlePostExecutionPauseStateMock.mockRejectedValueOnce('pause persistence exploded') + + const thrown = await executeWorkflow(workflow, 'request-1', undefined, 'actor-1', { + enabled: true, + principal, + billingAttribution, + }).catch((error: unknown) => error) + + expect(thrown).toBeInstanceOf(Error) + expect(hasExecutionResult(thrown)).toBe(true) + expect((thrown as { executionResult?: unknown }).executionResult).toBe(result) + }) + it('transfers post-execution ownership with successful streaming metadata', async () => { const result = await executeWorkflow(workflow, 'request-1', undefined, 'actor-1', { enabled: true, diff --git a/apps/sim/lib/workflows/executor/execute-workflow.ts b/apps/sim/lib/workflows/executor/execute-workflow.ts index c62b872e510..b6779b5c673 100644 --- a/apps/sim/lib/workflows/executor/execute-workflow.ts +++ b/apps/sim/lib/workflows/executor/execute-workflow.ts @@ -1,5 +1,6 @@ import type { WorkflowExecutionPrincipal } from '@sim/auth/principal' import { createLogger } from '@sim/logger' +import { toError } from '@sim/utils/errors' import { generateId } from '@sim/utils/id' import { assertBillingAttributionSnapshot, @@ -247,14 +248,20 @@ export async function executeWorkflow( } return result - } catch (error: unknown) { + } catch (caught: unknown) { + /** + * Normalized before anything reads it, for the reason the executor normalizes its own throw: + * a value that cannot carry the result would otherwise reach callers bare, and they read a + * missing result as proof that no block ran. `toError` returns an `Error` unchanged, so a + * custom error class keeps its identity and every ordinary failure is untouched. + */ + const error = toError(caught) /** * Carries the run's result on a failure raised after it produced one — the post-execution - * work below the executor call can throw, and callers read a missing result as proof that no - * block ran. Skipped when the executor already attached its own, which is the more specific - * record, and when the throw is not an object to carry it. + * work below the executor call can throw, and the executor never saw it. Skipped when the + * executor already attached its own, which is the more specific record. */ - if (executionResult && error instanceof Error && !hasExecutionResult(error)) { + if (executionResult && !hasExecutionResult(error)) { attachExecutionResult(error, executionResult) } const errorDiagnostic = loggingSession.projectDiagnosticError(error)