fix(copilot): name the run from the executor, not the logging session

Review found the logging session wrong in both directions, and it is: the
result of `safeStart` is never checked, so blocks execute even when it
fails — reporting that nothing started for a run that did, which is the
direction that duplicates work — and it flips before trigger resolution
and serialization, reporting a run for failures that never reached a
block. A resume whose conditional update matches no row returns true and
named a run that does not exist.

Entering the executor is the only honest answer to "could a side effect
have occurred", because side effects come from blocks rather than from log
rows. Moving the marker there settles all three at once.

Also from review:
- Stop returning the thrown environment or database error to the model
  when the egress catalog is unavailable. Nothing there can project it —
  the catalog it would need is the very thing that is missing — so the
  reason stays in the log and the response carries fixed text plus the
  workspace id the caller itself supplied.
- Guard the attach against a frozen failure, which would otherwise throw
  and replace the original error partway through cleanup, making a
  diagnostic aid the thing that loses the diagnosis.

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

Author: icecrasher321

apps/sim/app/api/copilot/tools/execute/route.test.ts

@@ -109,8 +109,12 @@ describe('POST /api/copilot/tools/execute (in-band)', () => {
 
     expect(mockHandler).not.toHaveBeenCalled()
     expect(body.success).toBe(false)
-    expect(body.error).toContain('Workspace ws-gone does not exist')
     expect(body.output).toEqual({ resultWithheld: true, effect: 'not_attempted' })
+    // The thrown reason is an unprojectable environment failure — the catalog that would
+    // vouch for it is the very thing missing — so it stays in the log.
+    expect(body.error).not.toContain('does not exist')
+    expect(body.error).toContain(BASE_BODY.workspaceId)
+    expect(body.error).toContain('could not be resolved')
   })
 
   it('reuses one turn registry across calls that share a messageId', async () => {

apps/sim/app/api/copilot/tools/execute/route.ts

@@ -134,18 +134,25 @@ export const POST = withRouteHandler((request: NextRequest) =>
          * The cause is almost always the workspace itself — a deleted or inaccessible id
          * reaching this lane — which is actionable, so it is reported rather than swallowed.
          */
-        const reason = getErrorMessage(err)
         logger.error('In-band egress registry unavailable; refusing the call', {
           toolName,
           toolCallId,
           userId,
           workspaceId,
-          error: reason,
+          error: getErrorMessage(err),
         })
         rootSpan.setAttributes({ [TraceAttr.ToolOutcome]: MothershipStreamV1ToolOutcome.error })
+        /**
+         * The thrown reason stays in the log. It is an environment or database failure that
+         * nothing here can project — the catalog it needed is the very thing that is missing —
+         * so this is the one message on this route that must be fixed text. The workspace id
+         * is echoed because the caller supplied it, and it is what makes this actionable.
+         */
         return NextResponse.json({
           success: false,
-          error: `${toolName} was not run: ${reason}`,
+          error: workspaceId
+            ? `${toolName} was not run: its workspace (${workspaceId}) could not be resolved. Check that the workspace exists and is accessible before retrying.`
+            : `${toolName} was not run: its execution environment could not be resolved.`,
           output: { resultWithheld: true, effect: TOOL_EFFECT_PHASE.notAttempted },
         })
       }

apps/sim/executor/utils/errors.ts

@@ -53,7 +53,14 @@ const ATTEMPTED_EXECUTION_ID = 'attemptedExecutionId'
 export function attachAttemptedExecutionId(error: unknown, executionId: string): void {
   if (!isAttachableThrown(error) || !executionId) return
   if (Object.hasOwn(error, ATTEMPTED_EXECUTION_ID)) return
-  Object.assign(error, { [ATTEMPTED_EXECUTION_ID]: executionId })
+  /**
+   * A frozen or sealed failure would otherwise throw here and replace the original error
+   * partway through cleanup, turning a diagnostic aid into the thing that loses the
+   * diagnosis. Losing the id is the lesser failure, and the log still records the run.
+   */
+  try {
+    Object.assign(error, { [ATTEMPTED_EXECUTION_ID]: executionId })
+  } catch {}
 }
 
 /** Reads the dispatched-run id a thrown value carries, if dispatch was reached at all. */

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

@@ -424,6 +424,7 @@ async function executeWorkflowCoreImpl(
   let processedInput = input || {}
   let deploymentVersionId: string | undefined
   let loggingStarted = false
+  let executorStarted = false
   let resolvedSecretTraceRegistry: ResolvedSecretTraceRegistry | undefined
   const pendingLifecycleCallbacks = new Set<Promise<void>>()
 
@@ -1051,6 +1052,14 @@ async function executeWorkflowCoreImpl(
       contextExtensions,
     })
 
+    /**
+     * The last statement before a block can run, and therefore the only honest answer to
+     * "could a side effect have occurred". The logging session is the wrong proxy in both
+     * directions: `safeStart`'s result is never checked, so blocks execute even when it
+     * fails — reporting nothing started for a run that did — and it flips before trigger
+     * resolution and serialization, reporting a run for failures that never reached a block.
+     */
+    executorStarted = true
     const result = runFromBlock
       ? ((await executorInstance.executeFromBlock(
           workflowId,
@@ -1098,17 +1107,12 @@ 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.
+     * Named only once a block could have run. 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 because every throw
+     * site past this point raises an Error.
      */
-    if (loggingStarted) attachAttemptedExecutionId(error, executionId)
+    if (executorStarted) attachAttemptedExecutionId(error, executionId)
     const errorCause = describeErrorCause(error)
     logger.error(
       `[${requestId}] Execution failed:`,