fix(copilot): say whether a withheld tool call changed anything
What a caller saw
A Copilot tool whose result the secret-egress boundary could not vouch for came back as either a bare {"success": true} with no payload, or one fixed sentence:
Tool execution settled, but its result could not be returned safely. Do not retry a mutation automatically.
Both drop the execution id along with the payload, and the sentence also overwrites the tool's real error. So a call rejected on its own arguments — nothing created — and a run that had already executed came back byte-identical. No caller could build a retry policy from that: retrying risks duplicate side effects, not retrying silently drops the work.
Reproduced before designing anything
A test latches a real ResolvedSecretTraceRegistry the way production latches one (a child run that returned no provenance envelope) and drives the real handler through the real projection:
| outcome | before | after |
|---|---|---|
| rejected pre-dispatch | {"success":false,"error":"…could not be returned safely…"} | effect: "not_attempted" |
| failed after dispatch | ← byte-identical to the row above | effect: "attempted" + executionId |
| completed | {"success":true}, no id | effect: "performed" + executionId |
| distinguishable | 2 / 3 | 3 / 3 |
No run content crosses the boundary in either column.
Two changes
1. A withheld result now states how far the call got.
A tool may declare a ToolCallEffect — a phase (not_attempted / attempted / performed) plus 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, may not take one of the disclosure record's own field names, and a single violation voids the whole disclosure.
The phase is recorded at the point a block could first have run — immediately before executorInstance.execute — so its absence is the positive statement that nothing started rather than an admission of not knowing. It lives in a WeakMap keyed by the thrown value rather than written onto it, matching what markExecutionFinalizedByCore already keeps nearby, and for the same reason: a thrown value is not reliably writable.
2. The in-band route no longer runs tools it cannot report on.
This is where the reported symptom actually originated. Over fourteen days in production, every occurrence of an unavailable egress catalog came from /api/copilot/tools/execute, and every one was a workspace id that no longer exists reaching that lane — concentrated in a small number of workspaces. The route logged a warning and then ran the tool anyway, which is the worst pair of outcomes available: the side effect happened, and because the projection can vouch for nothing without a catalog, the caller got a bare success or the sentinel.
It now refuses before dispatch. Nothing runs, so there is nothing to be uncertain about, and the response names the workspace. The thrown reason stays in the log — it is an environment failure that nothing here can project, since the catalog that would vouch for it is the very thing missing.
A missing workspace also reported itself as an access denial, sending every deleted-workspace call down a permissions path nobody could reproduce. checkWorkspaceAccess already distinguishes the two, so it now says which.
Telemetry
Withholding reports its cause on the warning and span it already wrote: registry-incomplete (carrying the closed reason and origin enums that name the guard which tripped), registry-absent, or content-refused — three causes with three different fixes. A withheld success is now reported as well; it previously logged nothing, which made the one case that leaves no other trace also the only one with no record of why.
For the reviewer
apps/sim/lib/workflows/executor/execution-core.ts is the file to read closely. The change is a single flag set immediately before the executor runs, but it sits on the path of every workflow execution, not only Copilot ones.
Verification
- 293 files / 4,838 tests across the affected suites
tscclean; 37 repo audits pass- New coverage: the three-outcome pin against the real handler and a real latched registry; the id-shape and reserved-key guards; a frozen error still naming its run; and dispatch-boundary tests for executor failure, admission refusal and authorization refusal
GitGuardian flags a test fixture in an early commit's diff — a synthetic string, never a real credential, already replaced and absent from the final tree.
🤖 Generated with Claude Code