Skip to content

Commit 353cc82

Browse files
icecrasher321claude
andcommitted
fix(copilot): disclose how far a withheld run got instead of one opaque sentinel
A tool result the egress projection cannot vouch for is reduced to a bare success or to `TOOL_RESULT_UNAVAILABLE_ERROR`. Both drop the execution id with the payload, and the sentinel also overwrites the real error text, so a call rejected on its own arguments and a run that already executed come back byte-identical. Those need opposite retry decisions. Reproduced against real code by latching a registry the way production latches one — a child run that returned no provenance envelope — and driving the real handler and the real projection: a pre-dispatch rejection and a post-dispatch failure were identical, and a completed run arrived as `{"success":true}` with nothing to look it up by. Two distinct shapes for three outcomes. The registry is right to fail closed; the boundary was discarding facts it never needed to redact. A tool may now declare a `ToolCallEffect` — a phase and server-minted ids — which the projection preserves when it withholds content, because neither is derived from that content. The exemption is enforced rather than asserted: ids must match the identifier shape this system mints, and one that does not voids the whole disclosure. The phase is attached in the application layer from dispatch onward and nowhere earlier, which is what makes the id's absence the positive statement that nothing was created rather than an admission of not knowing. Withholding also now reports its cause — a latched registry names the guard that tripped, an absent one means no catalog was built, and a content refusal means the registry was fine — so the next occurrence is diagnosable from the logs it already writes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 474c105 commit 353cc82

11 files changed

Lines changed: 604 additions & 22 deletions

File tree

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { TraceSpan } from '@/lib/copilot/generated/trace-spans-v1'
99
import { checkInternalApiKey } from '@/lib/copilot/request/http'
1010
import { withIncomingGoSpan } from '@/lib/copilot/request/otel'
1111
import {
12+
describeWithholdingCause,
1213
inspectToolResultForCopilot,
1314
projectToolErrorMessageForCopilot,
1415
} from '@/lib/copilot/request/tools/resolved-secret-result'
@@ -158,6 +159,7 @@ export const POST = withRouteHandler((request: NextRequest) =>
158159
error: projected.error,
159160
runtimeSucceeded: result.success,
160161
projectionSafe: projection.safe,
162+
...(projection.safe ? {} : describeWithholdingCause(projection.cause)),
161163
})
162164
}
163165
if (result.success && chatId) {

apps/sim/executor/utils/errors.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,33 @@ export function attachExecutionResult(error: Error, executionResult: ExecutionRe
3636
Object.assign(error, { executionResult })
3737
}
3838

39+
const ATTEMPTED_EXECUTION_ID = 'attemptedExecutionId'
40+
41+
/**
42+
* Names the run a failure belongs to once dispatch has been attempted.
43+
*
44+
* A caller that only sees the thrown error cannot tell an authorization refusal — which
45+
* created nothing — from a crash after the run was already dispatched, and those need
46+
* opposite retry decisions. Attaching the id at the point of no return makes its absence
47+
* mean "nothing was started" rather than "we do not know", and its presence a key that
48+
* resolves to zero or one executions.
49+
*
50+
* Distinct from {@link attachExecutionResult}: that says the workflow ran and produced a
51+
* result, this says only that it was dispatched.
52+
*/
53+
export function attachAttemptedExecutionId(error: unknown, executionId: string): void {
54+
if (!(error instanceof Error) || !executionId) return
55+
if (ATTEMPTED_EXECUTION_ID in error) return
56+
Object.assign(error, { [ATTEMPTED_EXECUTION_ID]: executionId })
57+
}
58+
59+
/** Reads the dispatched-run id an error carries, if dispatch was reached at all. */
60+
export function readAttemptedExecutionId(error: unknown): string | undefined {
61+
if (!(error instanceof Error) || !(ATTEMPTED_EXECUTION_ID in error)) return undefined
62+
const value = (error as Error & Record<string, unknown>)[ATTEMPTED_EXECUTION_ID]
63+
return typeof value === 'string' && value.length > 0 ? value : undefined
64+
}
65+
3966
export interface BlockExecutionErrorDetails {
4067
block: SerializedBlock
4168
error: Error | string

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

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,10 @@ import {
6161
setTerminalToolCallState,
6262
} from '@/lib/copilot/request/tool-call-state'
6363
import { maybeWriteOutputToFile } from '@/lib/copilot/request/tools/files'
64-
import { inspectToolResultForCopilot } from '@/lib/copilot/request/tools/resolved-secret-result'
64+
import {
65+
describeWithholdingCause,
66+
inspectToolResultForCopilot,
67+
} from '@/lib/copilot/request/tools/resolved-secret-result'
6568
import { handleResourceSideEffects } from '@/lib/copilot/request/tools/resources'
6669
import {
6770
maybeWriteOutputToTable,
@@ -737,15 +740,20 @@ async function executeToolAndReportInner(
737740
toolSpan.attributes = {
738741
...toolSpan.attributes,
739742
...summarizeToolResultForSpan(copilotResult),
740-
...(projection.safe ? {} : { resultWithheld: true }),
743+
...(projection.safe
744+
? {}
745+
: { resultWithheld: true, ...describeWithholdingCause(projection.cause) }),
741746
}
742747
if (!projection.safe) {
743748
// A withheld SUCCESS otherwise leaves no trace anywhere: the span reads
744749
// ok and the model just sees a bare `{success: true}` with no output.
750+
// The cause is what says whether a guard latched, no catalog was built,
751+
// or the payload itself was unprojectable — three different fixes.
745752
logger.warn('Tool result withheld by egress projection', {
746753
toolCallId: toolCall.id,
747754
toolName: toolCall.name,
748755
runtimeSucceeded: result.success,
756+
...describeWithholdingCause(projection.cause),
749757
})
750758
}
751759

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

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
import { describe, expect, it } from 'vitest'
55
import { RunCode, RunFunction } from '@/lib/copilot/generated/tool-catalog-v1'
66
import {
7+
describeWithholdingCause,
8+
inspectToolResultForCopilot,
79
projectToolResultForCopilot,
810
READ_TOOL_RESULT_UNAVAILABLE_ERROR,
911
TOOL_RESULT_UNAVAILABLE_ERROR,
@@ -457,3 +459,88 @@ describe('projectToolResultForCopilot', () => {
457459
expect(toolResultUnavailableError(undefined)).toBe(TOOL_RESULT_UNAVAILABLE_ERROR)
458460
})
459461
})
462+
463+
describe('effect disclosure on a withheld result', () => {
464+
const EXECUTION_ID = '0f4d5a4c-6a1e-4c2f-9b7d-2c8f1a3e5d90'
465+
466+
it('carries nothing extra for a tool that declared no effect', () => {
467+
expect(projectToolResultForCopilot({ success: true, output: { a: 1 } }, undefined)).toEqual({
468+
success: true,
469+
})
470+
expect(projectToolResultForCopilot({ success: false, error: 'why' }, undefined)).toEqual({
471+
success: false,
472+
error: TOOL_RESULT_UNAVAILABLE_ERROR,
473+
})
474+
})
475+
476+
/**
477+
* The exemption is what makes the disclosure trustworthy, so it has to be all or
478+
* nothing: a disclosure that silently dropped the id it could not vouch for would
479+
* read exactly like one that never had a run to name.
480+
*/
481+
it('voids the whole disclosure when an id is not a shape this system mints', () => {
482+
expect(
483+
projectToolResultForCopilot(
484+
{
485+
success: false,
486+
error: 'why',
487+
effect: { phase: 'performed', ids: { executionId: 'sk-live-9Qv2XbTn4LmZa8Rd' } },
488+
},
489+
undefined,
490+
'run_workflow'
491+
)
492+
).toEqual({ success: false, error: TOOL_RESULT_UNAVAILABLE_ERROR })
493+
})
494+
495+
it('reports the phase and ids when every id is vouchable', () => {
496+
expect(
497+
projectToolResultForCopilot(
498+
{
499+
success: false,
500+
error: 'why',
501+
effect: { phase: 'attempted', ids: { executionId: EXECUTION_ID } },
502+
},
503+
undefined,
504+
'run_workflow'
505+
)
506+
).toEqual({
507+
success: false,
508+
output: { resultWithheld: true, effect: 'attempted', executionId: EXECUTION_ID },
509+
error: expect.stringContaining('At most one run exists'),
510+
})
511+
})
512+
513+
it('never leaks the disclosure into a result that projected cleanly', () => {
514+
const registry = new ResolvedSecretTraceRegistry()
515+
516+
expect(
517+
projectToolResultForCopilot(
518+
{
519+
success: true,
520+
output: { executionId: EXECUTION_ID },
521+
effect: { phase: 'performed', ids: { executionId: EXECUTION_ID } },
522+
},
523+
registry,
524+
'run_workflow'
525+
)
526+
).toEqual({ success: true, output: { executionId: EXECUTION_ID } })
527+
})
528+
529+
it('names why the content was withheld, for the surface about to log it', () => {
530+
const latched = createRegistry()
531+
latched.markIncomplete('source-provenance-incomplete', { origin: 'test.origin' })
532+
533+
const projection = inspectToolResultForCopilot({ success: false }, latched, 'run_workflow')
534+
expect(projection.safe).toBe(false)
535+
// The per-call fork adds its own propagation reason; the guard that originally
536+
// tripped has to survive alongside it, or a refusal names only the messenger.
537+
expect(projection.safe === false && describeWithholdingCause(projection.cause)).toEqual({
538+
withheldCause: 'registry-incomplete',
539+
withheldReasons: expect.arrayContaining(['source-provenance-incomplete']),
540+
withheldOrigins: ['test.origin'],
541+
})
542+
543+
const absent = inspectToolResultForCopilot({ success: false }, undefined)
544+
expect(absent.safe === false && absent.cause).toEqual({ kind: 'registry-absent' })
545+
})
546+
})

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

Lines changed: 126 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
1-
import type { ToolExecutionResult } from '@/lib/copilot/tool-executor/types'
1+
import type { ToolCallEffect, ToolExecutionResult } from '@/lib/copilot/tool-executor/types'
2+
import { TOOL_EFFECT_PHASE } from '@/lib/copilot/tool-executor/types'
23
import { projectResolvedSecretModelJsonContent } from '@/executor/utils/resolved-secret-content-projection'
3-
import type { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry'
4+
import type {
5+
ResolvedSecretIncompletenessReason,
6+
ResolvedSecretTraceRegistry,
7+
} from '@/executor/utils/resolved-secret-trace-registry'
48

59
export const TOOL_RESULT_UNAVAILABLE_ERROR =
610
'Tool execution settled, but its result could not be returned safely. Do not retry a mutation automatically.'
@@ -13,33 +17,131 @@ export const TOOL_RESULT_UNAVAILABLE_ERROR =
1317
export const READ_TOOL_RESULT_UNAVAILABLE_ERROR =
1418
'Tool executed, but its result could not be returned safely. The call was read-only, so you may retry it or continue without the result.'
1519

20+
/**
21+
* Withheld-result wording for a call that disclosed how far its side effect got.
22+
*
23+
* The generic message above has to cover both "nothing happened" and "it happened,
24+
* you just cannot see it", which is why a caller could not build a retry policy from
25+
* it: a rejected call and a completed mutation read identically. A tool that declares
26+
* its {@link ToolCallEffect} gets the phrasing its phase actually warrants.
27+
*/
28+
const WITHHELD_ERROR_BY_EFFECT_PHASE: Record<ToolCallEffect['phase'], string> = {
29+
[TOOL_EFFECT_PHASE.notAttempted]:
30+
'Tool call was rejected before it ran, so nothing was created or changed. The reason could not be returned safely — correct the call and try again.',
31+
[TOOL_EFFECT_PHASE.attempted]:
32+
'Tool execution was dispatched but its outcome could not be returned safely. At most one run exists for the ids in this result — resolve it before retrying a mutation.',
33+
[TOOL_EFFECT_PHASE.performed]:
34+
'Tool execution completed but its result could not be returned safely. Do not retry — read the outcome using the ids in this result.',
35+
}
36+
1637
const READ_ONLY_RESULT_TOOLS = new Set(['read', 'glob', 'grep'])
1738

39+
/**
40+
* The shape of an identifier this system mints — `generateId`'s UUID, and the
41+
* database ids that share it. Effect ids bypass secret projection, so the set of
42+
* values that may occupy one is pinned to a syntax no credential we issue or store
43+
* takes. A caller with a differently shaped id has to widen this deliberately,
44+
* where the exemption is reviewed, rather than by passing it.
45+
*/
46+
const SERVER_MINTED_ID_PATTERN =
47+
/^[0-9a-f]{8}-[0-9a-f]{4}-[1-8][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i
48+
1849
/** Chooses the withheld-result message a tool's caller should surface. */
1950
export function toolResultUnavailableError(toolId?: string): string {
2051
return toolId && READ_ONLY_RESULT_TOOLS.has(toolId)
2152
? READ_TOOL_RESULT_UNAVAILABLE_ERROR
2253
: TOOL_RESULT_UNAVAILABLE_ERROR
2354
}
2455

56+
/**
57+
* Why complete content could not cross, for the caller that is about to log a refusal.
58+
*
59+
* The three causes need different fixes — a latched registry names the guard that tripped,
60+
* an absent one means the surface never built a catalog, and a content refusal means the
61+
* registry was fine and the payload itself was unprojectable — so they are not collapsed.
62+
*/
63+
export type ToolResultWithholdingCause =
64+
| {
65+
kind: 'registry-incomplete'
66+
reasons: readonly ResolvedSecretIncompletenessReason[]
67+
origins: readonly string[]
68+
}
69+
| { kind: 'registry-absent' }
70+
| { kind: 'content-refused' }
71+
72+
export type CopilotToolResultProjection =
73+
| { safe: true; result: ToolExecutionResult }
74+
| { safe: false; result: ToolExecutionResult; cause: ToolResultWithholdingCause }
75+
2576
function structuralResult(result: ToolExecutionResult): ToolExecutionResult {
2677
return { success: result.success === true }
2778
}
2879

80+
/**
81+
* Reduces a withheld result to the facts the tool asserted about the call itself.
82+
*
83+
* Content is dropped because nothing here can prove it secret-free. The effect
84+
* disclosure survives because it is not derived from content: the phase is a
85+
* code-defined literal and every id is checked against {@link SERVER_MINTED_ID_PATTERN}.
86+
* An id that fails that check voids the whole disclosure rather than being dropped
87+
* on its own — a partially honoured exemption is the one shape a reader would
88+
* misread as complete.
89+
*/
2990
function omittedResult(result: ToolExecutionResult, toolId?: string): ToolExecutionResult {
30-
if (result.success) return { success: true }
31-
return { success: false, error: toolResultUnavailableError(toolId) }
91+
const effect = vouchableEffect(result.effect)
92+
if (!effect) {
93+
return result.success
94+
? { success: true }
95+
: { success: false, error: toolResultUnavailableError(toolId) }
96+
}
97+
98+
return {
99+
success: result.success === true,
100+
output: { resultWithheld: true, effect: effect.phase, ...effect.ids },
101+
...(result.success ? {} : { error: WITHHELD_ERROR_BY_EFFECT_PHASE[effect.phase] }),
102+
}
32103
}
33104

34-
export type CopilotToolResultProjection =
35-
| { safe: true; result: ToolExecutionResult }
36-
| { safe: false; result: ToolExecutionResult }
105+
/** Returns the disclosure only when every id it carries is a shape this system mints. */
106+
function vouchableEffect(effect: ToolCallEffect | undefined): ToolCallEffect | undefined {
107+
if (!effect) return undefined
108+
for (const value of Object.values(effect.ids ?? {})) {
109+
if (typeof value !== 'string' || !SERVER_MINTED_ID_PATTERN.test(value)) return undefined
110+
}
111+
return effect
112+
}
113+
114+
function withholdingCause(
115+
registry: ResolvedSecretTraceRegistry | undefined
116+
): ToolResultWithholdingCause {
117+
if (!registry) return { kind: 'registry-absent' }
118+
const diagnostics = registry.getIncompletenessDiagnostics()
119+
return diagnostics
120+
? {
121+
kind: 'registry-incomplete',
122+
reasons: diagnostics.reasons,
123+
origins: diagnostics.origins,
124+
}
125+
: { kind: 'content-refused' }
126+
}
127+
128+
function withheld(
129+
result: ToolExecutionResult,
130+
registry: ResolvedSecretTraceRegistry | undefined,
131+
toolId: string | undefined
132+
): CopilotToolResultProjection {
133+
return {
134+
safe: false,
135+
result: omittedResult(result, toolId),
136+
cause: withholdingCause(registry),
137+
}
138+
}
37139

38140
/**
39141
* Projects terminal tool content and reports whether the complete content was safe to cross.
40142
* Callers that isolate provenance per tool call may merge that child registry only when `safe`
41143
* is true and the child is complete. The returned result is always safe to expose: an unsafe
42-
* projection is reduced to a structural success or failure.
144+
* projection is reduced to a structural success or failure, plus any effect the tool disclosed.
43145
*/
44146
export function inspectToolResultForCopilot(
45147
result: ToolExecutionResult,
@@ -54,15 +156,15 @@ export function inspectToolResultForCopilot(
54156
if (Object.hasOwn(result, 'error')) content.error = result.error
55157
const projection = projectResolvedSecretModelJsonContent(content, resultRegistry)
56158
if (!projection.safe || !projection.value || typeof projection.value !== 'object') {
57-
return { safe: false, result: omittedResult(result, toolId) }
159+
return withheld(result, resultRegistry, toolId)
58160
}
59161

60162
const projectedContent = projection.value as Record<string, unknown>
61163
const projected = structuralResult(result)
62164
if (Object.hasOwn(projectedContent, 'output')) projected.output = projectedContent.output
63165
if (Object.hasOwn(projectedContent, 'error')) {
64166
if (typeof projectedContent.error !== 'string') {
65-
return { safe: false, result: omittedResult(result, toolId) }
167+
return withheld(result, resultRegistry, toolId)
66168
}
67169
projected.error = projectedContent.error
68170
}
@@ -74,7 +176,7 @@ export function inspectToolResultForCopilot(
74176
}
75177
return { safe: true, result: projected }
76178
} catch {
77-
return { safe: false, result: omittedResult(result, toolId) }
179+
return withheld(result, registry, toolId)
78180
}
79181
}
80182

@@ -98,3 +200,16 @@ export function projectToolErrorMessageForCopilot(
98200
): string {
99201
return projectToolResultForCopilot({ success: false, error }, registry, toolId).error ?? ''
100202
}
203+
204+
/** Flattens a withholding cause into log/span fields, so every surface reports it alike. */
205+
export function describeWithholdingCause(
206+
cause: ToolResultWithholdingCause
207+
): Record<string, unknown> {
208+
return cause.kind === 'registry-incomplete'
209+
? {
210+
withheldCause: cause.kind,
211+
withheldReasons: [...cause.reasons],
212+
...(cause.origins.length > 0 ? { withheldOrigins: [...cause.origins] } : {}),
213+
}
214+
: { withheldCause: cause.kind }
215+
}

0 commit comments

Comments
 (0)