fix(copilot): say whether a withheld tool call changed anything

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

@@ -95,12 +95,26 @@ describe('POST /api/copilot/tools/execute (in-band)', () => {
     expect(body.error).toBe('File not found: files/a.md')
   })
 
-  it('withholds results when no egress registry can be built', async () => {
-    mockPrepareEnvironmentContext.mockRejectedValue(new Error('env unavailable'))
+  /**
+   * Running the tool without a catalog used to produce the worst pair of outcomes available:
+   * the side effect happened and the caller got a bare `{success: true}` naming neither the
+   * cause nor whether anything had changed.
+   */
+  it('refuses the call, without running the tool, when no egress registry can be built', async () => {
+    mockPrepareEnvironmentContext.mockRejectedValue(new Error('Workspace ws-gone does not exist'))
     mockHandler.mockResolvedValue({ success: true, output: { content: 'sensitive' } })
+
     const res = await POST(makeRequest({ ...BASE_BODY, messageId: 'msg-no-registry' }) as never)
     const body = await res.json()
-    expect(body).toEqual({ success: true })
+
+    expect(mockHandler).not.toHaveBeenCalled()
+    expect(body.success).toBe(false)
+    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

@@ -4,18 +4,21 @@ import { type NextRequest, NextResponse } from 'next/server'
 import { copilotToolExecuteInternalBodySchema } from '@/lib/api/contracts/copilot'
 import { validationErrorResponse } from '@/lib/api/server'
 import { prepareCopilotEnvironmentContext } from '@/lib/copilot/environment-context'
+import { MothershipStreamV1ToolOutcome } from '@/lib/copilot/generated/mothership-stream-v1'
 import { TraceAttr } from '@/lib/copilot/generated/trace-attributes-v1'
 import { TraceSpan } from '@/lib/copilot/generated/trace-spans-v1'
 import { checkInternalApiKey } from '@/lib/copilot/request/http'
 import { withIncomingGoSpan } from '@/lib/copilot/request/otel'
 import {
+  describeWithholdingCause,
   inspectToolResultForCopilot,
   projectToolErrorMessageForCopilot,
 } from '@/lib/copilot/request/tools/resolved-secret-result'
 import { handleResourceSideEffects } from '@/lib/copilot/request/tools/resources'
 import type { ToolCallResult } from '@/lib/copilot/request/types'
 import { ensureHandlersRegistered } from '@/lib/copilot/tool-executor'
 import { executeTool } from '@/lib/copilot/tool-executor/executor'
+import { TOOL_EFFECT_PHASE } from '@/lib/copilot/tool-executor/types'
 import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
 import type { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry'
 
@@ -115,17 +118,43 @@ export const POST = withRouteHandler((request: NextRequest) =>
         [TraceAttr.UserId]: userId,
       })
 
-      let toolRegistry: ResolvedSecretTraceRegistry | undefined
-      let turnRegistry: ResolvedSecretTraceRegistry | undefined
+      let toolRegistry: ResolvedSecretTraceRegistry
+      let turnRegistry: ResolvedSecretTraceRegistry
       try {
         turnRegistry = await getTurnEgressRegistry(userId, workspaceId, messageId)
         toolRegistry = turnRegistry.forkForInputPaths([])
       } catch (err) {
-        logger.error('In-band egress registry unavailable; results will be withheld', {
+        /**
+         * Without a catalog the projection can vouch for nothing, so every result this call
+         * could produce would be withheld. Running the tool anyway was the worst of both
+         * outcomes: the side effect happened and the caller got an opaque sentinel that named
+         * neither the cause nor whether anything had changed. Refusing before dispatch is
+         * both truthful and the only answer that leaves nothing behind.
+         *
+         * 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.
+         */
+        logger.error('In-band egress registry unavailable; refusing the call', {
           toolName,
           toolCallId,
+          userId,
+          workspaceId,
           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: 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 },
+        })
       }
 
       try {
@@ -148,9 +177,23 @@ export const POST = withRouteHandler((request: NextRequest) =>
         })
         const projection = inspectToolResultForCopilot(result, toolRegistry, toolName)
         const projected = projection.result
-        if (projection.safe && toolRegistry?.isComplete() && turnRegistry) {
+        if (projection.safe && toolRegistry.isComplete()) {
           turnRegistry.mergeToolCallRegistry(toolRegistry)
         }
+        if (!projection.safe) {
+          /**
+           * Reported on its own rather than folded into the failure branch below: a withheld
+           * SUCCESS keeps `projected.success` true, so gating on failure meant the one case
+           * that leaves no other trace — the model reads a bare success — was also the one
+           * case whose cause was never written down.
+           */
+          logger.warn('In-band tool result withheld by egress projection', {
+            toolName,
+            toolCallId,
+            runtimeSucceeded: result.success,
+            ...describeWithholdingCause(projection.cause),
+          })
+        }
         if (!projected.success) {
           logger.warn('In-band tool execution failed', {
             toolName,

apps/sim/executor/utils/errors.ts

@@ -36,6 +36,55 @@ export function attachExecutionResult(error: Error, executionResult: ExecutionRe
   Object.assign(error, { executionResult })
 }
 
+/**
+ * Dispatched-run ids, keyed by the thrown value itself.
+ *
+ * A side table rather than a property on the error, for the same reason
+ * {@link markExecutionFinalizedByCore} keeps one: a thrown value is not reliably writable.
+ * `Object.assign` throws on a frozen or sealed failure, and guarding that throw would drop
+ * the marker instead — silently converting "this run exists" into "nothing started", which
+ * is the one direction that duplicates work. Identity keying also means no id can arrive
+ * through a prototype chain, and nothing is added to the error's own surface, so a
+ * serialized error carries no stray field.
+ */
+const attemptedExecutionIds = new WeakMap<object, string>()
+
+/**
+ * Names the run a failure belongs to once dispatch has been attempted.
+ *
+ * A caller that only sees the thrown error cannot tell an authorization refusal — which
+ * created nothing — from a crash after the run was already dispatched, and those need
+ * opposite retry decisions. Recording the id at the point of no return makes its absence
+ * mean "nothing was started" rather than "we do not know", and its presence a key that
+ * resolves to zero or one executions.
+ *
+ * Distinct from {@link attachExecutionResult}: that says the workflow ran and produced a
+ * result, this says only that it was dispatched.
+ */
+export function attachAttemptedExecutionId(error: unknown, executionId: string): void {
+  if (!isRecordedThrown(error) || !executionId) return
+  if (attemptedExecutionIds.has(error)) return
+  attemptedExecutionIds.set(error, executionId)
+}
+
+/** Reads the dispatched-run id a thrown value carries, if dispatch was reached at all. */
+export function readAttemptedExecutionId(error: unknown): string | undefined {
+  return isRecordedThrown(error) ? attemptedExecutionIds.get(error) : undefined
+}
+
+/**
+ * Any non-null object, not only an `Error`.
+ *
+ * Restricting this to `Error` would silently invert the invariant for a thrown plain object:
+ * no id would be recorded, its absence would read as "nothing was started", and the caller
+ * would retry a run that already exists. A thrown primitive cannot be keyed at all, which
+ * costs nothing today because every throw site past the dispatch boundary raises an `Error`.
+ */
+function isRecordedThrown(value: unknown): value is object {
+  /** Functions key a WeakMap as well as objects do, so excluding them would drop the record. */
+  return (typeof value === 'object' || typeof value === 'function') && value !== null
+}
+
 export interface BlockExecutionErrorDetails {
   block: SerializedBlock
   error: Error | string

apps/sim/lib/copilot/request/tools/executor.ts

@@ -61,7 +61,10 @@ import {
   setTerminalToolCallState,
 } from '@/lib/copilot/request/tool-call-state'
 import { maybeWriteOutputToFile } from '@/lib/copilot/request/tools/files'
-import { inspectToolResultForCopilot } from '@/lib/copilot/request/tools/resolved-secret-result'
+import {
+  describeWithholdingCause,
+  inspectToolResultForCopilot,
+} from '@/lib/copilot/request/tools/resolved-secret-result'
 import { handleResourceSideEffects } from '@/lib/copilot/request/tools/resources'
 import {
   maybeWriteOutputToTable,
@@ -737,15 +740,20 @@ async function executeToolAndReportInner(
     toolSpan.attributes = {
       ...toolSpan.attributes,
       ...summarizeToolResultForSpan(copilotResult),
-      ...(projection.safe ? {} : { resultWithheld: true }),
+      ...(projection.safe
+        ? {}
+        : { resultWithheld: true, ...describeWithholdingCause(projection.cause) }),
     }
     if (!projection.safe) {
       // A withheld SUCCESS otherwise leaves no trace anywhere: the span reads
       // ok and the model just sees a bare `{success: true}` with no output.
+      // The cause is what says whether a guard latched, no catalog was built,
+      // or the payload itself was unprojectable — three different fixes.
       logger.warn('Tool result withheld by egress projection', {
         toolCallId: toolCall.id,
         toolName: toolCall.name,
         runtimeSucceeded: result.success,
+        ...describeWithholdingCause(projection.cause),
       })
     }
 

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

@@ -4,6 +4,8 @@
 import { describe, expect, it } from 'vitest'
 import { RunCode, RunFunction } from '@/lib/copilot/generated/tool-catalog-v1'
 import {
+  describeWithholdingCause,
+  inspectToolResultForCopilot,
   projectToolResultForCopilot,
   READ_TOOL_RESULT_UNAVAILABLE_ERROR,
   TOOL_RESULT_UNAVAILABLE_ERROR,
@@ -457,3 +459,105 @@ describe('projectToolResultForCopilot', () => {
     expect(toolResultUnavailableError(undefined)).toBe(TOOL_RESULT_UNAVAILABLE_ERROR)
   })
 })
+
+describe('effect disclosure on a withheld result', () => {
+  const EXECUTION_ID = '0f4d5a4c-6a1e-4c2f-9b7d-2c8f1a3e5d90'
+
+  it('carries nothing extra for a tool that declared no effect', () => {
+    expect(projectToolResultForCopilot({ success: true, output: { a: 1 } }, undefined)).toEqual({
+      success: true,
+    })
+    expect(projectToolResultForCopilot({ success: false, error: 'why' }, undefined)).toEqual({
+      success: false,
+      error: TOOL_RESULT_UNAVAILABLE_ERROR,
+    })
+  })
+
+  /**
+   * The exemption is what makes the disclosure trustworthy, so it has to be all or
+   * nothing: a disclosure that silently dropped the id it could not vouch for would
+   * read exactly like one that never had a run to name.
+   */
+  it('voids the whole disclosure when an id is not a shape this system mints', () => {
+    expect(
+      projectToolResultForCopilot(
+        {
+          success: false,
+          error: 'why',
+          effect: { phase: 'performed', ids: { executionId: 'not-a-server-minted-id' } },
+        },
+        undefined,
+        'run_workflow'
+      )
+    ).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(
+        {
+          success: false,
+          error: 'why',
+          effect: { phase: 'attempted', ids: { executionId: EXECUTION_ID } },
+        },
+        undefined,
+        'run_workflow'
+      )
+    ).toEqual({
+      success: false,
+      output: { resultWithheld: true, effect: 'attempted', executionId: EXECUTION_ID },
+      error: expect.stringContaining('At most one run exists'),
+    })
+  })
+
+  it('never leaks the disclosure into a result that projected cleanly', () => {
+    const registry = new ResolvedSecretTraceRegistry()
+
+    expect(
+      projectToolResultForCopilot(
+        {
+          success: true,
+          output: { executionId: EXECUTION_ID },
+          effect: { phase: 'performed', ids: { executionId: EXECUTION_ID } },
+        },
+        registry,
+        'run_workflow'
+      )
+    ).toEqual({ success: true, output: { executionId: EXECUTION_ID } })
+  })
+
+  it('names why the content was withheld, for the surface about to log it', () => {
+    const latched = createRegistry()
+    latched.markIncomplete('source-provenance-incomplete', { origin: 'test.origin' })
+
+    const projection = inspectToolResultForCopilot({ success: false }, latched, 'run_workflow')
+    expect(projection.safe).toBe(false)
+    // The per-call fork adds its own propagation reason; the guard that originally
+    // tripped has to survive alongside it, or a refusal names only the messenger.
+    expect(projection.safe === false && describeWithholdingCause(projection.cause)).toEqual({
+      withheldCause: 'registry-incomplete',
+      withheldReasons: expect.arrayContaining(['source-provenance-incomplete']),
+      withheldOrigins: ['test.origin'],
+    })
+
+    const absent = inspectToolResultForCopilot({ success: false }, undefined)
+    expect(absent.safe === false && absent.cause).toEqual({ kind: 'registry-absent' })
+  })
+})