fix(copilot): put the dispatch boundary at the logging session, not the core

Review was right that entering the execution core is too early. The core
loads custom blocks, workflow state, and the environment before
`safeStart` writes a row, so a setup failure — which ran nothing and is
safely retryable — still reported a dispatched run and sent the caller
looking for it.

Move the marker to `loggingStarted`, read before the catch's own recovery
`safeStart` writes a row for the failure itself. That is the first point
blocks may have executed, so it is the honest line, and it lets
executeWorkflow go back to a plain rethrow.

The thrown value stays exactly as received, including a non-Error one:
the core's finalization guard identifies it, and three existing tests pin
that. A thrown primitive therefore carries no id, which costs nothing
today because every throw site past `safeStart` raises an Error — noted in
the code rather than papered over.

Also read the marker with `Object.hasOwn` rather than `in`, so an id
reached through a prototype chain can never disclose an unrelated run, and
cover the reserved-key branch of the disclosure guard.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: icecrasher321

apps/sim/executor/utils/errors.ts

@@ -52,13 +52,17 @@ const ATTEMPTED_EXECUTION_ID = 'attemptedExecutionId'
  */
 export function attachAttemptedExecutionId(error: unknown, executionId: string): void {
   if (!isAttachableThrown(error) || !executionId) return
-  if (ATTEMPTED_EXECUTION_ID in error) return
+  if (Object.hasOwn(error, ATTEMPTED_EXECUTION_ID)) return
   Object.assign(error, { [ATTEMPTED_EXECUTION_ID]: executionId })
 }
 
 /** Reads the dispatched-run id a thrown value carries, if dispatch was reached at all. */
 export function readAttemptedExecutionId(error: unknown): string | undefined {
-  if (!isAttachableThrown(error) || !(ATTEMPTED_EXECUTION_ID in error)) return undefined
+  /**
+   * Own property only. `in` would accept one reached through the prototype chain, which
+   * would let an unrelated run's id be disclosed for a refusal that started nothing.
+   */
+  if (!isAttachableThrown(error) || !Object.hasOwn(error, ATTEMPTED_EXECUTION_ID)) return undefined
   const value = (error as Record<string, unknown>)[ATTEMPTED_EXECUTION_ID]
   return typeof value === 'string' && value.length > 0 ? value : undefined
 }

apps/sim/lib/copilot/request/tools/resolved-secret-result.test.ts

@@ -492,6 +492,23 @@ describe('effect disclosure on a withheld result', () => {
     ).toEqual({ success: false, error: TOOL_RESULT_UNAVAILABLE_ERROR })
   })
 
+  it.each(['effect', 'resultWithheld'])(
+    'voids the disclosure when an id would take the reserved key %s',
+    (reserved) => {
+      expect(
+        projectToolResultForCopilot(
+          {
+            success: false,
+            error: 'why',
+            effect: { phase: 'performed', ids: { [reserved]: EXECUTION_ID } },
+          },
+          undefined,
+          'run_workflow'
+        )
+      ).toEqual({ success: false, error: TOOL_RESULT_UNAVAILABLE_ERROR })
+    }
+  )
+
   it('reports the phase and ids when every id is vouchable', () => {
     expect(
       projectToolResultForCopilot(

apps/sim/lib/workflows/application/run-workflow-from-copilot.test.ts

@@ -326,7 +326,7 @@ describe('Copilot workflow run application commands', () => {
 
     it('passes through the id executeWorkflow attached at its dispatch boundary', async () => {
       mocks.executeWorkflow.mockImplementationOnce(() => {
-        // Exactly what executeWorkflow does once it enters the core.
+        // Exactly what the execution core does once its logging session has started.
         const dispatchFailure = new Error('database unavailable')
         Object.assign(dispatchFailure, { attemptedExecutionId: 'child-execution-1' })
         throw dispatchFailure

apps/sim/lib/workflows/executor/execute-workflow.ts

@@ -1,6 +1,5 @@
 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,
@@ -14,7 +13,6 @@ 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 { attachAttemptedExecutionId } from '@/executor/utils/errors'
 import type { ResolvedSecretTraceProvenanceV1 } from '@/executor/utils/resolved-secret-trace-registry'
 import type { CoreTriggerType } from '@/stores/logs/filters/types'
 
@@ -130,7 +128,6 @@ export async function executeWorkflow(
     loggingSession.setTrustedExecutionCorrelation(streamConfig.trustedExecutionCorrelation)
   }
   let postExecutionOwnershipTransferred = false
-  let dispatched = false
 
   try {
     const metadata: ExecutionMetadata = {
@@ -172,13 +169,6 @@ export async function executeWorkflow(
 
     const executionStartMs = Date.now()
 
-    /**
-     * Entering the core is the point past which an execution row may exist. Everything above
-     * it — workspace and billing preflight, snapshot construction — fails without creating
-     * anything, so a caller can read the absence of this id as "nothing was started" rather
-     * than as an admission of not knowing.
-     */
-    dispatched = true
     const result = await executeWorkflowCore({
       snapshot,
       callbacks: {
@@ -250,14 +240,7 @@ export async function executeWorkflow(
     }
 
     return result
-  } catch (thrown: unknown) {
-    /**
-     * A thrown primitive has nowhere to carry the dispatched-run id, and losing it would make
-     * a real run read as never started. Normalizing only past the dispatch boundary keeps the
-     * rethrown value identical on every other path.
-     */
-    const error = dispatched && typeof thrown !== 'object' ? toError(thrown) : thrown
-    if (dispatched) attachAttemptedExecutionId(error, executionId)
+  } catch (error: unknown) {
     const errorDiagnostic = loggingSession.projectDiagnosticError(error)
     logger.error(`[${requestId}] Workflow execution failed`, errorDiagnostic)
 

apps/sim/lib/workflows/executor/execution-core.ts

@@ -49,7 +49,7 @@ import type {
   SerializableExecutionState,
 } from '@/executor/execution/types'
 import type { ExecutionResult, StartBlockRunMetadata } from '@/executor/types'
-import { hasExecutionResult } from '@/executor/utils/errors'
+import { attachAttemptedExecutionId, hasExecutionResult } from '@/executor/utils/errors'
 import { projectResolvedSecretDiagnosticError } from '@/executor/utils/resolved-secret-content-projection'
 import {
   createResolvedSecretTraceRegistry,
@@ -1097,6 +1097,18 @@ async function executeWorkflowCoreImpl(
 
     return result
   } catch (error: unknown) {
+    /**
+     * Whether the run had started when it failed, read before the recovery `safeStart` below
+     * writes a row for the failure itself. This — not entry into this function — is what says
+     * blocks may have executed: everything above it (custom-block loading, state loading,
+     * environment and secret setup) fails having run nothing, and a caller told otherwise
+     * would refuse a retry that was safe.
+     *
+     * The thrown value is rethrown exactly as received, including a non-Error one, because
+     * the finalization guard below identifies it. A primitive therefore carries no id, which
+     * costs nothing today: every throw site past `safeStart` raises an Error.
+     */
+    if (loggingStarted) attachAttemptedExecutionId(error, executionId)
     const errorCause = describeErrorCause(error)
     logger.error(
       `[${requestId}] Execution failed:`,