From b5ed7018677e350a1c11bfa8e3f2ea292f1e53de Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 30 Sep 2026 01:33:21 -0700 Subject: [PATCH 01/14] fix(mothership): keep a tainted mount's verdict across the run_code crossing A run_code / run_function that mounted a workspace file whose sidecar is `unknown` latched its per-call registry through the crossing with only `source-provenance-incomplete` and `inherited-incomplete-source`. Both are in the registry's absence set, so the withheld result named no guard, and an output file written from that registry (outputs.files[].path) was recorded as `unrecorded` absence instead of taint. The crossing now inherits the mounted registry's own reasons when it latched, so the refusal names `mounted-file-provenance-unavailable` and writers keep the taint. The mount refusal also reports the file id. No projection is relaxed: the result is still withheld, exact mounts still redact, clean mounts are unchanged. --- .../function-execute-file-mounts.test.ts | 61 ++++++++++++++++++- .../tools/handlers/function-execute.ts | 14 +++++ 2 files changed, 73 insertions(+), 2 deletions(-) diff --git a/apps/sim/lib/mothership/tools/handlers/function-execute-file-mounts.test.ts b/apps/sim/lib/mothership/tools/handlers/function-execute-file-mounts.test.ts index 4be998d2f62..df8ef9727f3 100644 --- a/apps/sim/lib/mothership/tools/handlers/function-execute-file-mounts.test.ts +++ b/apps/sim/lib/mothership/tools/handlers/function-execute-file-mounts.test.ts @@ -3,7 +3,7 @@ import { dbChainMockFns, queueTableRows, resetDbChainMock } from '@sim/testing' import { createSessionPrincipal } from '@sim/testing/factories/principal.factory' import { encryptionMock, encryptionMockFns } from '@sim/testing/mocks/encryption.mock' import { storageServiceMock, storageServiceMockFns } from '@sim/testing/mocks/storage-service.mock' -import { toolsMock } from '@sim/testing/mocks/tools.mock' +import { toolsMock, toolsMockFns } from '@sim/testing/mocks/tools.mock' import { workspaceAuthzMock, workspaceAuthzMockFns } from '@sim/testing/mocks/workspace-authz.mock' import { workspaceFileManagerMock, @@ -37,7 +37,11 @@ vi.mock('@/tools', () => toolsMock) import type { SandboxFile } from '@/lib/execution/remote-sandbox/types' import { inspectToolResultForCopilot } from '@/lib/mothership/request/tools/resolved-secret-result' import type { ToolExecutionContext } from '@/lib/mothership/tool-executor/types' -import { resolveInputFiles } from '@/lib/mothership/tools/handlers/function-execute' +import { + executeFunctionExecute, + resolveInputFiles, +} from '@/lib/mothership/tools/handlers/function-execute' +import { createWorkspaceFileSecretProvenanceFromRegistry } from '@/lib/uploads/contexts/workspace/workspace-file-secret-provenance' import { readWorkspaceFileMount } from '@/lib/workspace-files/application/read-workspace-file-mount' import { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry' @@ -194,6 +198,59 @@ describe('Mothership file mounts bind content and classification to the same rec } ) + /** + * The run's code can read every mounted byte, so the per-call registry the Copilot projection + * and output-file writers read must carry the mount's own verdict across the crossing: a + * tainted mount stays a taint (never the `unrecorded` absence) and names the guard that tripped, + * while exact mounts keep redacting and clean mounts stay readable. + */ + it.each(['safe', 'secret', 'unknown'] as const)( + 'carries a %s mount verdict across the run_code crossing', + async (kind) => { + queueProvenance(kind === 'unknown' ? 'unknown' : 'exact', revision, kind === 'secret') + toolsMockFns.mockExecuteTool.mockResolvedValue({ + success: true, + output: { result: content, stdout: content }, + }) + const trace = new ResolvedSecretTraceRegistry([], { + userId: 'reader', + workspaceId: 'workspace', + }) + const result = await executeFunctionExecute( + { + code: "print(open('/tmp/source.txt').read())", + language: 'python', + inputs: { files: [{ path: 'file', sandboxPath: '/tmp/source.txt' }] }, + }, + { ...context, resolvedSecretTraceRegistry: trace } + ) + + const observation = inspectToolResultForCopilot(result, trace, 'run_code') + const written = await createWorkspaceFileSecretProvenanceFromRegistry(trace, result.output, { + userId: 'reader', + workspaceId: 'workspace', + }) + expect(JSON.stringify(observation.result).includes(content)).toBe(kind === 'safe') + if (kind === 'unknown') { + expect(observation.safe).toBe(false) + expect.soft(observation.safe ? undefined : observation.cause).toMatchObject({ + kind: 'registry-incomplete', + reasons: expect.arrayContaining(['mounted-file-provenance-unavailable']), + }) + expect.soft(written).toEqual({ safe: false }) + } else { + expect(observation.safe).toBe(true) + expect(written).toMatchObject({ + safe: true, + provenance: { + status: 'exact', + entries: kind === 'secret' ? [expect.objectContaining({})] : [], + }, + }) + } + } + ) + it('denies revoked access before content or signed URL acquisition', async () => { mocks.permission.mockResolvedValue(null) await expect(run()).rejects.toThrow('permissions') diff --git a/apps/sim/lib/mothership/tools/handlers/function-execute.ts b/apps/sim/lib/mothership/tools/handlers/function-execute.ts index aa04f0076a3..1f878ea48d0 100644 --- a/apps/sim/lib/mothership/tools/handlers/function-execute.ts +++ b/apps/sim/lib/mothership/tools/handlers/function-execute.ts @@ -104,6 +104,7 @@ async function pushWorkspaceFileMount( const imported = await importWorkspaceFileSnapshotProvenance({ workspaceId, provenance: result.secretProvenance, + resourceId: record.id, registry, }) if (!imported) registry.markIncomplete('mounted-file-provenance-unavailable') @@ -471,6 +472,19 @@ async function importMountedProvenance( crossingValue: unknown ): Promise { if (!target) return + /** + * The run's code could read every mounted byte, so a mount the source refused is taint in the + * output, not an absence. A serialized envelope drops the reason, and the bare + * `source-provenance-incomplete` it leaves behind is in the absence set: a writer would then + * record the output as `unrecorded`, and a refusal would name no guard. + */ + if (source.isPermanentlyIncomplete()) { + target.markIncomplete('inherited-incomplete-source', { + source, + origin: 'copilotFunctionExecute.crossing', + }) + return + } try { const provenance = source.exportProvenanceForValue(crossingValue) From 68f42a686811336f833cb72b07f12426d4ae7dac Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 30 Sep 2026 01:42:59 -0700 Subject: [PATCH 02/14] improvement(mothership): tell the model why a tool result was withheld A withheld result reached the model as a bare `{ success: true }`, so the agent could not tell a tainted input from an oversized payload and retried or guessed. The withheld result now carries `withheldReason`, chosen from code-defined wording by the guard that tripped: an input file, table, or document with unknown secret provenance; provenance that could not be verified; or content that could not be checked. It rides the existing `resultWithheld` disclosure beside any effect ids, and the key is reserved so an id cannot displace it. No content, reason literal, or origin crosses; an absent registry still carries nothing. --- .../mothership/request/tools/client.test.ts | 4 + .../mothership/request/tools/executor.test.ts | 5 +- .../tools/resolved-secret-result.test.ts | 144 ++++++++++++++++-- .../request/tools/resolved-secret-result.ts | 67 ++++++-- .../function-execute-file-mounts.test.ts | 6 + .../workflow/withheld-run-result.test.ts | 7 +- 6 files changed, 205 insertions(+), 28 deletions(-) diff --git a/apps/sim/lib/mothership/request/tools/client.test.ts b/apps/sim/lib/mothership/request/tools/client.test.ts index 5c2ec94bb88..9f720e61c18 100644 --- a/apps/sim/lib/mothership/request/tools/client.test.ts +++ b/apps/sim/lib/mothership/request/tools/client.test.ts @@ -469,6 +469,8 @@ describe('workflow client tool completion', () => { success: true, workflowId: 'workflow-1', executionId: 'execution-1', + resultWithheld: true, + withheldReason: expect.stringMatching(/could not be checked/), }, }) expect(registry.isComplete()).toBe(true) @@ -571,6 +573,8 @@ describe('workflow client tool completion', () => { success: true, workflowId: 'workflow-1', executionId: 'execution-1', + resultWithheld: true, + withheldReason: expect.stringMatching(/could not be verified/), }, }) expect(registry.isComplete()).toBe(true) diff --git a/apps/sim/lib/mothership/request/tools/executor.test.ts b/apps/sim/lib/mothership/request/tools/executor.test.ts index d0e0f776a57..6810254f8e8 100644 --- a/apps/sim/lib/mothership/request/tools/executor.test.ts +++ b/apps/sim/lib/mothership/request/tools/executor.test.ts @@ -686,7 +686,10 @@ describe('executeToolAndReport provenance isolation', () => { expect(completion).toEqual({ status: MothershipStreamV1ToolOutcome.success, message: 'Tool completed', - data: { success: true }, + data: { + resultWithheld: true, + withheldReason: expect.stringMatching(/could not be verified/), + }, }) expect(registry.isComplete()).toBe(true) expect(registry.getActiveMatches()).toEqual([]) diff --git a/apps/sim/lib/mothership/request/tools/resolved-secret-result.test.ts b/apps/sim/lib/mothership/request/tools/resolved-secret-result.test.ts index 8291620b031..7def95a15ac 100644 --- a/apps/sim/lib/mothership/request/tools/resolved-secret-result.test.ts +++ b/apps/sim/lib/mothership/request/tools/resolved-secret-result.test.ts @@ -132,7 +132,13 @@ describe('projectToolResultForCopilot', () => { }, registry ) - ).toEqual({ success: true }) + ).toEqual({ + success: true, + output: { + resultWithheld: true, + withheldReason: expect.stringMatching(/could not be checked/), + }, + }) }) it('uses an opaque marker when a replacement contains another active literal', () => { @@ -334,7 +340,7 @@ describe('projectToolResultForCopilot', () => { }) it.each([ - ['missing', undefined], + ['missing', undefined, undefined], [ 'incomplete', (() => { @@ -342,20 +348,28 @@ describe('projectToolResultForCopilot', () => { registry.markIncomplete('unspecified') return registry })(), + { resultWithheld: true, withheldReason: expect.stringMatching(/could not be verified/) }, ], - ])('fails closed for %s provenance without changing structural fields', (_label, registry) => { - expect( - projectToolResultForCopilot( - { - success: false, - output: { result: 'possibly-secret' }, - error: 'possibly-secret-error', - resources: [{ type: 'file', id: 'file-1', title: 'report.txt' }], - }, - registry - ) - ).toEqual({ success: false, error: TOOL_RESULT_UNAVAILABLE_ERROR }) - }) + ])( + 'fails closed for %s provenance without changing structural fields', + (_label, registry, output) => { + expect( + projectToolResultForCopilot( + { + success: false, + output: { result: 'possibly-secret' }, + error: 'possibly-secret-error', + resources: [{ type: 'file', id: 'file-1', title: 'report.txt' }], + }, + registry + ) + ).toEqual({ + success: false, + ...(output ? { output } : {}), + error: TOOL_RESULT_UNAVAILABLE_ERROR, + }) + } + ) it('leaves resource metadata outside plaintext result projection', () => { const registry = createRegistry() @@ -558,3 +572,103 @@ describe('effect disclosure on a withheld result', () => { expect(absent.safe === false && absent.cause).toEqual({ kind: 'registry-absent' }) }) }) + +/** + * A withheld result used to reach the model as a bare `{ success: true }`, so the agent retried or + * guessed. The reason it now carries is chosen from code-defined wording by the guard that tripped; + * the payload, its keys, and any caller-supplied text must still never cross. + */ +describe('withholding reason disclosure', () => { + const EXECUTION_ID = '0f4d5a4c-6a1e-4c2f-9b7d-2c8f1a3e5d90' + const CONTENT = 'secret-value inside /files/private-report.txt' + + function latched(reason: 'mounted-file-provenance-unavailable' | 'entry-decrypt-failed') { + const registry = createRegistry() + registry.recordResolved('SECRET', 'secret-value', { propagated: true }) + registry.markIncomplete(reason, { origin: 'files/private-report.txt' }) + return registry + } + + it.each([ + [ + 'mounted-file-provenance-unavailable', + /file, table, or document .* unknown secret provenance/, + ], + ['entry-decrypt-failed', /could not be verified/], + ] as const)('explains a %s latch without any content', (reason, wording) => { + const projected = projectToolResultForCopilot( + { success: true, output: { stdout: CONTENT } }, + latched(reason), + 'run_code' + ) + expect(projected).toEqual({ + success: true, + output: { resultWithheld: true, withheldReason: expect.stringMatching(wording) }, + }) + expect(JSON.stringify(projected)).not.toMatch(/secret-value|private-report/) + }) + + it('explains an unprojectable payload from a complete registry', () => { + const projected = projectToolResultForCopilot( + { success: true, output: { 'secret-value': 'first', '{{SECRET}}': 'second' } }, + (() => { + const registry = createRegistry() + registry.recordResolved('SECRET', 'secret-value', { propagated: true }) + return registry + })(), + 'run_workflow' + ) + expect(projected).toEqual({ + success: true, + output: { + resultWithheld: true, + withheldReason: expect.stringMatching(/could not be checked/), + }, + }) + expect(JSON.stringify(projected)).not.toContain('secret-value') + }) + + it('keeps the reason beside an effect disclosure and the failure wording', () => { + expect( + projectToolResultForCopilot( + { + success: false, + error: CONTENT, + effect: { phase: 'performed', ids: { executionId: EXECUTION_ID } }, + }, + latched('mounted-file-provenance-unavailable'), + 'run_workflow' + ) + ).toEqual({ + success: false, + output: { + resultWithheld: true, + withheldReason: expect.stringMatching(/unknown secret provenance/), + effect: 'performed', + executionId: EXECUTION_ID, + }, + error: expect.stringContaining('Do not retry'), + }) + }) + + it('voids the disclosure when an id would take the reason key', () => { + expect( + projectToolResultForCopilot( + { + success: false, + error: 'why', + effect: { phase: 'performed', ids: { withheldReason: EXECUTION_ID } }, + }, + latched('entry-decrypt-failed'), + 'run_workflow' + ) + ).toEqual({ + success: false, + output: { + resultWithheld: true, + withheldReason: expect.stringMatching(/could not be verified/), + }, + error: TOOL_RESULT_UNAVAILABLE_ERROR, + }) + }) +}) diff --git a/apps/sim/lib/mothership/request/tools/resolved-secret-result.ts b/apps/sim/lib/mothership/request/tools/resolved-secret-result.ts index e8eca2cea89..cb7f7c53155 100644 --- a/apps/sim/lib/mothership/request/tools/resolved-secret-result.ts +++ b/apps/sim/lib/mothership/request/tools/resolved-secret-result.ts @@ -47,7 +47,47 @@ const SERVER_MINTED_ID_PATTERN = /^[0-9a-f]{8}-[0-9a-f]{4}-[1-8][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i /** Field names the disclosure record owns; an id may not take one. */ -const RESERVED_DISCLOSURE_KEYS = new Set(['resultWithheld', 'effect']) +const RESERVED_DISCLOSURE_KEYS = new Set(['resultWithheld', 'withheldReason', 'effect']) + +/** + * Guards that trip because a file, table, or document the call read carries no verified secret + * record. They are the ones a caller can act on by choosing other inputs, so they get their own + * wording; every other latch shares the generic one. + */ +const UNKNOWN_INPUT_PROVENANCE_REASONS = new Set([ + 'mounted-file-provenance-unavailable', + 'workspace-file-provenance-unknown', + 'file-source-unidentified', + 'table-snapshot-unsafe-for-mount', + 'table-result-provenance-unavailable', + 'table-run-state-provenance-unavailable', + 'knowledge-result-provenance-unavailable', + 'knowledge-row-missing', + 'knowledge-row-content-mismatch', +]) + +const WITHHELD_REASON = { + unknownInput: + 'A file, table, or document this call read has unknown secret provenance, so its output could contain a secret value. Use inputs whose provenance is known (for example, a freshly uploaded file) to see the output.', + unverified: + "Secret provenance for this call's inputs could not be verified, so its output could contain a secret value.", + contentRefused: + 'The result could not be checked for secret values, usually because it is too large. Request a narrower result.', +} as const + +/** + * The model-facing explanation for a withheld result, chosen only from the code-defined wording + * above by the guard that tripped. Reasons and origins themselves never cross: an origin is a + * caller-supplied string. An absent registry is a surface defect the model cannot act on, so it + * carries none. + */ +function withheldReason(cause: ToolResultWithholdingCause): string | undefined { + if (cause.kind === 'registry-absent') return undefined + if (cause.kind === 'content-refused') return WITHHELD_REASON.contentRefused + return cause.reasons.some((reason) => UNKNOWN_INPUT_PROVENANCE_REASONS.has(reason)) + ? WITHHELD_REASON.unknownInput + : WITHHELD_REASON.unverified +} /** Chooses the withheld-result message a tool's caller should surface. */ export function toolResultUnavailableError(toolId?: string): string { @@ -90,17 +130,25 @@ function structuralResult(result: ToolExecutionResult): ToolExecutionResult { * on its own — a partially honoured exemption is the one shape a reader would * misread as complete. */ -function omittedResult(result: ToolExecutionResult, toolId?: string): ToolExecutionResult { +function omittedResult( + result: ToolExecutionResult, + cause: ToolResultWithholdingCause, + toolId?: string +): ToolExecutionResult { const effect = vouchableEffect(result.effect) + const reason = withheldReason(cause) + const disclosure = reason ? { resultWithheld: true, withheldReason: reason } : undefined if (!effect) { - return result.success - ? { success: true } - : { success: false, error: toolResultUnavailableError(toolId) } + return { + success: result.success === true, + ...(disclosure ? { output: disclosure } : {}), + ...(result.success ? {} : { error: toolResultUnavailableError(toolId) }), + } } return { success: result.success === true, - output: { resultWithheld: true, effect: effect.phase, ...effect.ids }, + output: { resultWithheld: true, ...disclosure, effect: effect.phase, ...effect.ids }, ...(result.success ? {} : { error: WITHHELD_ERROR_BY_EFFECT_PHASE[effect.phase] }), } } @@ -139,11 +187,8 @@ function withheld( registry: ResolvedSecretTraceRegistry | undefined, toolId: string | undefined ): CopilotToolResultProjection { - return { - safe: false, - result: omittedResult(result, toolId), - cause: withholdingCause(registry), - } + const cause = withholdingCause(registry) + return { safe: false, result: omittedResult(result, cause, toolId), cause } } /** diff --git a/apps/sim/lib/mothership/tools/handlers/function-execute-file-mounts.test.ts b/apps/sim/lib/mothership/tools/handlers/function-execute-file-mounts.test.ts index df8ef9727f3..55543902bf2 100644 --- a/apps/sim/lib/mothership/tools/handlers/function-execute-file-mounts.test.ts +++ b/apps/sim/lib/mothership/tools/handlers/function-execute-file-mounts.test.ts @@ -238,6 +238,12 @@ describe('Mothership file mounts bind content and classification to the same rec reasons: expect.arrayContaining(['mounted-file-provenance-unavailable']), }) expect.soft(written).toEqual({ safe: false }) + expect(observation.result.output).toEqual({ + resultWithheld: true, + withheldReason: expect.stringMatching( + /file, table, or document .* unknown secret provenance/ + ), + }) } else { expect(observation.safe).toBe(true) expect(written).toMatchObject({ diff --git a/apps/sim/lib/mothership/tools/handlers/workflow/withheld-run-result.test.ts b/apps/sim/lib/mothership/tools/handlers/workflow/withheld-run-result.test.ts index a3ed72c3651..aaec93b5b9c 100644 --- a/apps/sim/lib/mothership/tools/handlers/workflow/withheld-run-result.test.ts +++ b/apps/sim/lib/mothership/tools/handlers/workflow/withheld-run-result.test.ts @@ -186,7 +186,11 @@ describe('a withheld run_workflow result', () => { 'run_workflow' ) - expect(result.output).toEqual({ resultWithheld: true, effect: 'not_attempted' }) + expect(result.output).toEqual({ + resultWithheld: true, + withheldReason: expect.stringMatching(/could not be verified/), + effect: 'not_attempted', + }) expect(result.error).toContain('nothing was created') }) @@ -197,6 +201,7 @@ describe('a withheld run_workflow result', () => { expect(result.success).toBe(succeeded) expect(result.output).toEqual({ resultWithheld: true, + withheldReason: expect.stringMatching(/could not be verified/), effect, // An id is present exactly when there is something to resolve. ...(effect === 'not_attempted' ? {} : { executionId: EXECUTION_ID }), From 01ff74be8d3bba876ed6cb88336bce2b9999cf2e Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 30 Sep 2026 02:10:36 -0700 Subject: [PATCH 03/14] improvement(mothership): log the size of a withheld tool result A `content-refused` withholding said only that a complete registry refused the payload. It can be refused by its encoded size or by the number of values the projection walks. Row-shaped payloads reach the 100k-value traversal cap well before the 16 MiB byte cap, and only while the call has an active secret. The withheld log lines now report the result's encoded bytes and value count, so a refusal names which cap it hit. Numbers only; no content is logged. --- .../app/api/copilot/tools/execute/route.ts | 2 ++ .../lib/mothership/request/tools/executor.ts | 2 ++ .../request/tools/resolved-secret-result.ts | 27 +++++++++++++++++++ 3 files changed, 31 insertions(+) diff --git a/apps/sim/app/api/copilot/tools/execute/route.ts b/apps/sim/app/api/copilot/tools/execute/route.ts index 3ced83827e4..4d6beb92521 100644 --- a/apps/sim/app/api/copilot/tools/execute/route.ts +++ b/apps/sim/app/api/copilot/tools/execute/route.ts @@ -14,6 +14,7 @@ import { withIncomingGoSpan } from '@/lib/mothership/request/otel' import { describeWithholdingCause, inspectToolResultForCopilot, + measureWithheldContent, projectToolErrorMessageForCopilot, } from '@/lib/mothership/request/tools/resolved-secret-result' import { handleResourceSideEffects } from '@/lib/mothership/request/tools/resources' @@ -235,6 +236,7 @@ export const POST = withRouteHandler((request: NextRequest) => toolCallId, runtimeSucceeded: result.success, ...describeWithholdingCause(projection.cause), + ...measureWithheldContent(result), }) } if (!projected.success) { diff --git a/apps/sim/lib/mothership/request/tools/executor.ts b/apps/sim/lib/mothership/request/tools/executor.ts index c75d179d423..2b6d075c026 100644 --- a/apps/sim/lib/mothership/request/tools/executor.ts +++ b/apps/sim/lib/mothership/request/tools/executor.ts @@ -67,6 +67,7 @@ import { maybeWriteOutputToFile } from '@/lib/mothership/request/tools/files' import { describeWithholdingCause, inspectToolResultForCopilot, + measureWithheldContent, } from '@/lib/mothership/request/tools/resolved-secret-result' import { handleResourceSideEffects } from '@/lib/mothership/request/tools/resources' import { @@ -915,6 +916,7 @@ async function executeToolAndReportInner( toolName: toolCall.name, runtimeSucceeded: result.success, ...describeWithholdingCause(projection.cause), + ...measureWithheldContent(result), }) } diff --git a/apps/sim/lib/mothership/request/tools/resolved-secret-result.ts b/apps/sim/lib/mothership/request/tools/resolved-secret-result.ts index cb7f7c53155..529f1941dd6 100644 --- a/apps/sim/lib/mothership/request/tools/resolved-secret-result.ts +++ b/apps/sim/lib/mothership/request/tools/resolved-secret-result.ts @@ -255,6 +255,33 @@ export function projectToolErrorMessageForCopilot( return projectToolResultForCopilot({ success: false, error }, registry, toolId).error ?? '' } +/** + * Sizes the content a withheld result would have carried, for the log line only. + * + * A complete registry can still refuse content by its encoded size or by the number of values the + * projection must walk (its node cap is reached well before the byte cap by row-shaped payloads). + * Both measures are reported so a `content-refused` line names which one it hit. Numbers only: + * the content itself never reaches the log. + */ +export function measureWithheldContent(result: ToolExecutionResult): { + resultBytes?: number + resultValues?: number +} { + try { + let values = 0 + const encoded = JSON.stringify( + { output: result.output, error: result.error }, + (_key, value) => { + values += 1 + return value + } + ) + return { resultBytes: Buffer.byteLength(encoded, 'utf8'), resultValues: values } + } catch { + return {} + } +} + /** Flattens a withholding cause into log/span fields, so every surface reports it alike. */ export function describeWithholdingCause( cause: ToolResultWithholdingCause From 805805adba0a284ea48d644514c0d705412ff3c8 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 30 Sep 2026 02:10:36 -0700 Subject: [PATCH 04/14] fix(mothership): bound run_workflow block-log outputs before the secret projection A run_workflow result echoes every block's output in `logs`. A run with many row-shaped block outputs can exceed the projection's 100k-value traversal cap while staying under its byte cap. Whenever the call had an active secret, the whole result was then withheld, including the final output and error, and the model saw a bare success. Block-log outputs are now bounded to a quarter of each projection cap (`MAX_CONTENT_NODES` values and the default byte cap). Past the budget, the bulkiest outputs are replaced, largest first, with a `logs get --trace` pointer, in the same form the oversized-input compaction already uses. The executionId, status, final output, error, and `select` values are untouched, and the other three quarters of each cap stay free for them. --- .../resolved-secret-content-projection.ts | 3 +- .../tools/handlers/workflow/mutations.ts | 71 +++++- .../run-workflow-result-budget.test.ts | 234 ++++++++++++++++++ 3 files changed, 305 insertions(+), 3 deletions(-) create mode 100644 apps/sim/lib/mothership/tools/handlers/workflow/run-workflow-result-budget.test.ts diff --git a/apps/sim/executor/utils/resolved-secret-content-projection.ts b/apps/sim/executor/utils/resolved-secret-content-projection.ts index 9f707c566a1..82d4d594307 100644 --- a/apps/sim/executor/utils/resolved-secret-content-projection.ts +++ b/apps/sim/executor/utils/resolved-secret-content-projection.ts @@ -23,7 +23,8 @@ export { scanResolvedSecretString, } from '@/executor/utils/resolved-secret-matcher' -const MAX_CONTENT_NODES = 100_000 +/** Values one model-content projection will walk before refusing the whole payload. */ +export const MAX_CONTENT_NODES = 100_000 const MAX_CONTENT_DEPTH = 100 const INTERNAL_DIAGNOSTIC_IDENTIFIER_PATTERN = /__var_[A-Za-z0-9_]+|__sim_code_\d+_(?:binding|input|runtime)_\d+[A-Za-z0-9_]*|__sim_placeholder_[a-f0-9]{64}__|__sim_runtime_[A-Za-z0-9_]+_\d+[A-Za-z0-9_]*|__SIM_RUNTIME_PAYLOAD_PATH/g diff --git a/apps/sim/lib/mothership/tools/handlers/workflow/mutations.ts b/apps/sim/lib/mothership/tools/handlers/workflow/mutations.ts index 21266acca3d..fc304a24365 100644 --- a/apps/sim/lib/mothership/tools/handlers/workflow/mutations.ts +++ b/apps/sim/lib/mothership/tools/handlers/workflow/mutations.ts @@ -2,6 +2,7 @@ import { createLogger } from '@sim/logger' import { filterUndefined, isPlainRecord, isRecordLike } from '@sim/utils/object' import { createCopilotWorkspaceApiKey } from '@/lib/api-key/application/create-api-key' import { PlatformEvents } from '@/lib/core/telemetry' +import { MAX_INLINE_MATERIALIZATION_BYTES } from '@/lib/execution/payloads/limits' import { messageForCopilotApplicationError } from '@/lib/mothership/application/error' import { executeCopilotApiKeyUseCase } from '@/lib/mothership/application/execute-api-key-use-case' import { @@ -47,6 +48,7 @@ import { } from '@/lib/workflows/application/update-workflow-content' import { sanitizeForCopilot } from '@/lib/workflows/sanitization/json-sanitizer' import { hasExecutionResult, readAttemptedExecutionId } from '@/executor/utils/errors' +import { MAX_CONTENT_NODES } from '@/executor/utils/resolved-secret-content-projection' import type { WorkflowState } from '@/stores/workflows/workflow/types' const logger = createLogger('WorkflowMutations') @@ -60,7 +62,7 @@ const LOG_INPUT_KEEP_CHARS = 200 /** * Compacts the block inputs echoed back in `logs`. A Function block's `input.code` embeds the * fully serialized upstream rows, so a seven-block run repeated the same rows several times - * across ~14k chars of tool result. Outputs are never touched — they are what the run was for — + * across ~14k chars of tool result. Outputs are bounded separately by {@link compactBlockLogOutputs}, * and the full input stays one `logs get --trace` away. */ function compactBlockLogInputs(logs: unknown, executionId: string | undefined): unknown { @@ -80,6 +82,67 @@ function compactBlockLogInputs(logs: unknown, executionId: string | undefined): }) } +/** + * Budgets for the block outputs echoed back in `logs`, a quarter of each cap the model-facing + * projection enforces on the whole result. Reaching either cap withholds everything, including + * the final output and error the run was for; those stay intact here and share the remaining + * three quarters with the rest of the envelope. The value budget matters first: row-shaped + * outputs reach the projection's traversal cap long before its byte cap. + */ +const LOG_OUTPUT_VALUE_BUDGET = Math.floor(MAX_CONTENT_NODES / 4) +const LOG_OUTPUT_BYTE_BUDGET = Math.floor(MAX_INLINE_MATERIALIZATION_BYTES / 4) + +/** Counts values the way the projection walks them, and the bytes they encode to. */ +function measureLogOutput(value: unknown): { values: number; bytes: number } { + let values = 0 + const encoded = JSON.stringify(value, (_key, item) => { + values += 1 + return item + }) + return { values, bytes: encoded === undefined ? 0 : Buffer.byteLength(encoded, 'utf8') } +} + +/** + * Replaces the bulkiest block outputs in `logs` with a pointer once they exceed the budgets + * above, largest first, so the rest of the run still reaches the model. The full outputs stay in + * the run's archived trace, one `logs get --trace` away, matching how + * {@link compactBlockLogInputs} treats oversized inputs. + */ +function compactBlockLogOutputs(logs: unknown, executionId: string | undefined): unknown { + if (!Array.isArray(logs)) return logs + const reference = executionId ?? '' + const sizes = logs.map((entry) => + isPlainRecord(entry) && entry.output !== undefined ? measureLogOutput(entry.output) : undefined + ) + let values = 0 + let bytes = 0 + for (const size of sizes) { + values += size?.values ?? 0 + bytes += size?.bytes ?? 0 + } + if (values <= LOG_OUTPUT_VALUE_BUDGET && bytes <= LOG_OUTPUT_BYTE_BUDGET) return logs + + const compacted = [...logs] + const bulkiestFirst = sizes + .map((size, index) => ({ size, index })) + .filter((entry): entry is { size: { values: number; bytes: number }; index: number } => + Boolean(entry.size) + ) + .sort( + (left, right) => right.size.values - left.size.values || right.size.bytes - left.size.bytes + ) + for (const { size, index } of bulkiestFirst) { + if (values <= LOG_OUTPUT_VALUE_BUDGET && bytes <= LOG_OUTPUT_BYTE_BUDGET) break + compacted[index] = { + ...(logs[index] as Record), + output: `…[output omitted: ${size.values} values, ${size.bytes} bytes; see logs get ${reference} --trace]`, + } + values -= size.values + bytes -= size.bytes + } + return compacted +} + function stripBinaryFields(value: unknown): unknown { if (value === null || value === undefined) return value if (typeof value !== 'object') return value @@ -182,7 +245,11 @@ function buildExecutionOutput( ...extra, output: lifted ? lifted.output : output, ...(lifted ? { outputFrom: lifted.outputFrom } : {}), - ...presentWorkflowLogs(logs, select), + // `select` reads full values from the run's own logs, so only the echoed logs are bounded. + ...presentWorkflowLogs( + select?.length ? logs : compactBlockLogOutputs(logs, executionId), + select + ), }, error: result.success ? undefined diff --git a/apps/sim/lib/mothership/tools/handlers/workflow/run-workflow-result-budget.test.ts b/apps/sim/lib/mothership/tools/handlers/workflow/run-workflow-result-budget.test.ts new file mode 100644 index 00000000000..1e8b3a6cf1a --- /dev/null +++ b/apps/sim/lib/mothership/tools/handlers/workflow/run-workflow-result-budget.test.ts @@ -0,0 +1,234 @@ +/** + * A run_workflow result sized like the production refusals: thirteen table-query blocks, + * ~11.5k rows, ~9.7 MiB. That is under the 16 MiB byte cap, yet with one active secret the + * projection walked ~138k values against its 100k traversal cap and withheld the whole result, + * leaving the model a bare success. The model-facing result is now bounded before it reaches + * the projection, and the omitted block outputs stay reachable through the run's archived trace. + */ + +import { executeWorkflowMock } from '@sim/testing/mocks/execute-workflow.mock' +import { + largeValueMetadataMock, + largeValueMetadataMockFns, +} from '@sim/testing/mocks/large-value-metadata.mock' +import { storageServiceMockFns } from '@sim/testing/mocks/storage-service.mock' +import { telemetryMock } from '@sim/testing/mocks/telemetry.mock' +import { uploadsMock } from '@sim/testing/mocks/uploads.mock' +import { workflowsOrchestrationMock } from '@sim/testing/mocks/workflows-orchestration.mock' +import { getErrorMessage } from '@sim/utils/errors' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { clearLargeValueCacheForTests } from '@/lib/execution/payloads/cache' +import { + externalizeExecutionData, + materializeExecutionData, +} from '@/lib/logs/execution/trace-store' +import { inspectToolResultForCopilot } from '@/lib/mothership/request/tools/resolved-secret-result' +import type { ExecutionContext } from '@/lib/mothership/request/types' +import { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry' + +const { mocks } = vi.hoisted(() => ({ mocks: { executeWorkflowUseCase: vi.fn() } })) + +vi.mock('@/lib/mothership/application/execute-workflow-use-case', () => ({ + executeCopilotWorkflowUseCase: mocks.executeWorkflowUseCase, + messageForCopilotWorkflowError: (error: unknown, fallback = 'Workflow operation failed') => + getErrorMessage(error, fallback), +})) +vi.mock('@/lib/workflows/sanitization/json-sanitizer', () => ({ + sanitizeForCopilot: vi.fn((state) => state), +})) +vi.mock('@/lib/workflows/executor/execute-workflow', () => executeWorkflowMock) +vi.mock('@/lib/execution/cancel-workflow-execution', () => ({ + cancelWorkflowExecution: vi.fn(), + WorkflowExecutionNotFoundError: class WorkflowExecutionNotFoundError extends Error {}, +})) +vi.mock('@/lib/workflows/orchestration', () => workflowsOrchestrationMock) +vi.mock('@/lib/core/telemetry', () => telemetryMock) +vi.mock('@/lib/uploads', () => uploadsMock) +vi.mock('@/lib/execution/payloads/large-value-metadata', () => largeValueMetadataMock) + +import { executeRunWorkflow } from '@/lib/mothership/tools/handlers/workflow/mutations' + +const EXECUTION_ID = '0f4d5a4c-6a1e-4c2f-9b7d-2c8f1a3e5d90' +const SECRET = 'fake-secret-for-test-only' +const context = { + userId: 'user-1', + workspaceId: 'workspace-1', + toolCallId: 'tool-call-1', +} as ExecutionContext + +/** Row counts and stored bytes of the table queries in one production refusal. */ +const TABLE_QUERIES: ReadonlyArray = [ + [32, 9_513], + [2, 2_590], + [1_848, 2_079_072], + [2_533, 1_698_688], + [4_833, 3_179_855], + [1_294, 1_517_841], + [32, 5_563], + [11, 3_246], + [835, 721_833], + [33, 52_482], + [28, 8_274], + [13, 9_669], + [33, 293_266], +] + +function tableRows(count: number, bytes: number) { + const columns = 8 + const width = Math.max(1, Math.floor(bytes / count / columns) - 12) + return Array.from({ length: count }, (_, index) => ({ + id: `row_${index}`, + data: Object.fromEntries( + Array.from({ length: columns }, (_, column) => [`col_${column}`, 'x'.repeat(width)]) + ), + createdAt: '2026-09-24T00:00:00.000Z', + })) +} + +function traceShapedLogs() { + return TABLE_QUERIES.map(([rows, bytes], index) => { + const result = tableRows(rows, bytes) + return { + blockId: `query-${index}`, + blockName: `Query ${index}`, + success: true, + output: { rows: result, rowCount: result.length }, + } + }) +} + +function secretRegistry() { + const registry = new ResolvedSecretTraceRegistry([ + { name: 'API_KEY', plaintext: SECRET, encryptedValue: 'ciphertext' }, + ]) + registry.recordResolved('API_KEY', SECRET, { propagated: true }) + return registry +} + +describe('run_workflow model-facing result budget', () => { + beforeEach(() => { + mocks.executeWorkflowUseCase.mockReset() + }) + + it('projects a trace-shaped result with an active secret instead of withholding it', async () => { + const logs = traceShapedLogs() + const finalOutput = { summary: `report for ${SECRET}`, rowCount: 11_529 } + mocks.executeWorkflowUseCase.mockResolvedValue({ + success: false, + error: `Report block failed after reading ${SECRET}`, + output: finalOutput, + logs, + metadata: { executionId: EXECUTION_ID }, + }) + + const settled = await executeRunWorkflow({ workflowId: 'wf-1' }, context) + const projection = inspectToolResultForCopilot(settled, secretRegistry(), 'run_workflow') + + expect(projection.safe).toBe(true) + const output = projection.result.output as Record + expect(output.executionId).toBe(EXECUTION_ID) + expect(output.success).toBe(false) + expect(output.output).toEqual({ summary: 'report for {{API_KEY}}', rowCount: 11_529 }) + expect(projection.result.error).toBe('Report block failed after reading {{API_KEY}}') + const presented = output.logs as Array> + expect(presented.map((log) => log.blockName)).toEqual(logs.map((log) => log.blockName)) + const omitted = presented.filter((log) => typeof log.output === 'string') + expect(omitted.length).toBeGreaterThan(0) + for (const log of omitted) { + expect(log.output).toContain(`logs get ${EXECUTION_ID} --trace`) + } + // Small outputs still arrive in full; only the bulky ones are replaced. + expect(presented.find((log) => log.blockName === 'Query 1')?.output).toEqual(logs[1].output) + }) + + /** The final output is what the run was for, so it is never compacted and needs the headroom. */ + it('leaves room for a large final output beside the bounded logs', async () => { + const finalOutput = { rows: tableRows(4_833, 3_179_855) } + mocks.executeWorkflowUseCase.mockResolvedValue({ + success: true, + output: finalOutput, + logs: traceShapedLogs(), + metadata: { executionId: EXECUTION_ID }, + }) + + const settled = await executeRunWorkflow({ workflowId: 'wf-1' }, context) + const projection = inspectToolResultForCopilot(settled, secretRegistry(), 'run_workflow') + + expect(projection.safe).toBe(true) + expect((projection.result.output as Record).output).toEqual(finalOutput) + }) + + /** Narrow rows reach the traversal cap while staying far under every byte budget. */ + it('bounds narrow row outputs by value count alone', async () => { + const logs = Array.from({ length: 6 }, (_, index) => ({ + blockId: `narrow-${index}`, + blockName: `Narrow ${index}`, + success: true, + output: { rows: tableRows(2_500, 2_500 * 8 * 13) }, + })) + const finalOutput = { rows: tableRows(2_000, 2_000 * 8 * 13) } + mocks.executeWorkflowUseCase.mockResolvedValue({ + success: true, + output: finalOutput, + logs, + metadata: { executionId: EXECUTION_ID }, + }) + + const settled = await executeRunWorkflow({ workflowId: 'wf-1' }, context) + expect(Buffer.byteLength(JSON.stringify(settled.output))).toBeLessThan(4 * 1024 * 1024) + const projection = inspectToolResultForCopilot(settled, secretRegistry(), 'run_workflow') + + expect(projection.safe).toBe(true) + expect((projection.result.output as Record).output).toEqual(finalOutput) + }) + + it('keeps every omitted block output reachable through the pointer', async () => { + const logs = traceShapedLogs() + mocks.executeWorkflowUseCase.mockResolvedValue({ + success: true, + output: {}, + logs, + metadata: { executionId: EXECUTION_ID }, + }) + const settled = await executeRunWorkflow({ workflowId: 'wf-1' }, context) + const presented = (settled.output as { logs: Array> }).logs + const omitted = presented.filter((log) => typeof log.output === 'string') + expect(omitted.length).toBeGreaterThan(0) + + // `logs get --trace` reads the run's archived execution data; archive and read it back. + storageServiceMockFns.mockUploadFile.mockImplementation(async ({ customKey, file }) => { + storageServiceMockFns.mockDownloadFile.mockResolvedValue(file) + return { key: customKey } + }) + largeValueMetadataMockFns.mockRegisterLargeValueOwner.mockResolvedValue(true) + clearLargeValueCacheForTests() + const archiveContext = { + workspaceId: 'workspace-1', + workflowId: 'wf-1', + executionId: EXECUTION_ID, + userId: 'user-1', + } + const slim = await externalizeExecutionData( + { + traceSpans: logs.map((log) => ({ + id: log.blockId, + name: log.blockName, + output: log.output, + })), + }, + archiveContext, + { throwOnError: true } + ) + clearLargeValueCacheForTests() + const archived = (await materializeExecutionData(slim, archiveContext)) as { + traceSpans: Array<{ name: string; output: unknown }> + } + + for (const log of omitted) { + const original = logs.find((entry) => entry.blockName === log.blockName) + expect(archived.traceSpans.find((span) => span.name === log.blockName)?.output).toEqual( + original?.output + ) + } + }) +}) From b4fc3c4334fd4c399c75f0ac7f2a3984a3499f36 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 30 Sep 2026 02:10:24 -0700 Subject: [PATCH 05/14] fix(mothership): bound browser-run workflow logs and keep the pointer length-blind run_workflow can complete in the browser, and that restoration projected the raw block logs, so a large browser-run workflow was still withheld. The block-log compaction now lives in the shared workflow-output module and applies to both the server handler and the client restoration when no `select` is given. `select` still reads full values. The omitted-output pointer no longer reports bytes. They were measured before secret projection and so disclosed a secret's length. It reports the value count and says "inspect with logs get" rather than promising the full value. One `measureModelContent` in the projection module now serves both the compaction and the withheld-size logging. It stops counting at the value cap and tolerates values JSON cannot encode. The byte cap is exported beside `MAX_CONTENT_NODES`. --- .../resolved-secret-content-projection.ts | 43 +++++- .../mothership/request/tools/client.test.ts | 45 ++++++ .../lib/mothership/request/tools/client.ts | 7 +- .../request/tools/resolved-secret-result.ts | 28 ++-- .../function-execute-file-mounts.test.ts | 12 +- .../tools/handlers/workflow/mutations.ts | 98 +------------- .../run-workflow-result-budget.test.ts | 128 ++++++++---------- .../lib/mothership/tools/workflow-output.ts | 101 +++++++++++++- 8 files changed, 270 insertions(+), 192 deletions(-) diff --git a/apps/sim/executor/utils/resolved-secret-content-projection.ts b/apps/sim/executor/utils/resolved-secret-content-projection.ts index 82d4d594307..0391fa19650 100644 --- a/apps/sim/executor/utils/resolved-secret-content-projection.ts +++ b/apps/sim/executor/utils/resolved-secret-content-projection.ts @@ -25,6 +25,8 @@ export { /** Values one model-content projection will walk before refusing the whole payload. */ export const MAX_CONTENT_NODES = 100_000 +/** Encoded bytes a model-content projection accepts by default before refusing the payload. */ +export const MAX_MODEL_CONTENT_BYTES = MAX_INLINE_MATERIALIZATION_BYTES const MAX_CONTENT_DEPTH = 100 const INTERNAL_DIAGNOSTIC_IDENTIFIER_PATTERN = /__var_[A-Za-z0-9_]+|__sim_code_\d+_(?:binding|input|runtime)_\d+[A-Za-z0-9_]*|__sim_placeholder_[a-f0-9]{64}__|__sim_runtime_[A-Za-z0-9_]+_\d+[A-Za-z0-9_]*|__SIM_RUNTIME_PAYLOAD_PATH/g @@ -352,12 +354,41 @@ function projectContent( export function projectResolvedSecretContent( value: unknown, matcher: ResolvedSecretMatcher, - maxBytes = MAX_INLINE_MATERIALIZATION_BYTES, + maxBytes = MAX_MODEL_CONTENT_BYTES, options: ResolvedSecretContentProjectionOptions = {} ): ResolvedSecretContentProjection { return projectContent(value, matcher, maxBytes, options) } +/** Size of a value as the model-content projection sees it, for budgeting and diagnostics. */ +export interface ModelContentMeasure { + /** Values walked, stopping at `MAX_CONTENT_NODES + 1`: past the cap the exact count is moot. */ + values: number + /** Encoded bytes; absent when counting stopped at the value cap. */ + bytes?: number +} + +class ModelContentMeasureLimit extends Error {} + +/** + * Measures a value in the units the projection caps, without walking past the value cap: a + * payload that is already over it is reported as over rather than serialized in full. Returns + * undefined for a value JSON cannot encode (a BigInt, a cycle), which the projection refuses too. + */ +export function measureModelContent(value: unknown): ModelContentMeasure | undefined { + let values = 0 + try { + const encoded = JSON.stringify(value, (_key, item: unknown) => { + values += 1 + if (values > MAX_CONTENT_NODES) throw new ModelContentMeasureLimit() + return item + }) + return { values, bytes: encoded === undefined ? 0 : Buffer.byteLength(encoded, 'utf8') } + } catch (error) { + return error instanceof ModelContentMeasureLimit ? { values } : undefined + } +} + /** Returns the registry-revision-cached matcher used for all model-visible projection. */ export function getResolvedSecretModelMatcher( registry: ResolvedSecretTraceRegistry | undefined @@ -397,7 +428,7 @@ export function getResolvedSecretModelMatcher( export function projectResolvedSecretModelContent( value: unknown, registry: ResolvedSecretTraceRegistry | undefined, - maxBytes = MAX_INLINE_MATERIALIZATION_BYTES, + maxBytes = MAX_MODEL_CONTENT_BYTES, options: ResolvedSecretContentProjectionOptions = {} ): ResolvedSecretContentProjection { const snapshot = getResolvedSecretModelMatcher(registry) @@ -420,7 +451,7 @@ export function projectResolvedSecretModelContent( export function projectResolvedSecretModelJsonContent( value: unknown, registry: ResolvedSecretTraceRegistry | undefined, - maxBytes = MAX_INLINE_MATERIALIZATION_BYTES, + maxBytes = MAX_MODEL_CONTENT_BYTES, options: ResolvedSecretContentProjectionOptions = {} ): ResolvedSecretContentProjection { const snapshot = getResolvedSecretModelMatcher(registry) @@ -456,7 +487,7 @@ export function projectResolvedSecretModelJsonContent( export function projectResolvedSecretDiagnosticContent( value: unknown, registry: ResolvedSecretTraceRegistry | undefined, - maxBytes = MAX_INLINE_MATERIALIZATION_BYTES + maxBytes = MAX_MODEL_CONTENT_BYTES ): ResolvedSecretContentProjection { return projectResolvedSecretModelContent(value, registry, maxBytes, { sanitizeInternalIdentifiers: true, @@ -510,7 +541,7 @@ export function isResolvedSecretModelContentUnchanged( if (!snapshot.complete) return false if (!snapshot.matcher) return true - const projection = projectContent(value, snapshot.matcher, MAX_INLINE_MATERIALIZATION_BYTES, { + const projection = projectContent(value, snapshot.matcher, MAX_MODEL_CONTENT_BYTES, { projectPrimitiveLiterals: true, rejectResolvedSecretLiterals: true, }) @@ -524,7 +555,7 @@ export function isResolvedSecretModelContentUnchanged( export function projectResolvedSecretModelJsonStrings( values: readonly (string | undefined)[], registry: ResolvedSecretTraceRegistry | undefined, - maxBytes = MAX_INLINE_MATERIALIZATION_BYTES + maxBytes = MAX_MODEL_CONTENT_BYTES ): ResolvedSecretContentProjection { const snapshot = getResolvedSecretModelMatcher(registry) if (!snapshot.complete) return { safe: false } diff --git a/apps/sim/lib/mothership/request/tools/client.test.ts b/apps/sim/lib/mothership/request/tools/client.test.ts index 9f720e61c18..5898126afd3 100644 --- a/apps/sim/lib/mothership/request/tools/client.test.ts +++ b/apps/sim/lib/mothership/request/tools/client.test.ts @@ -202,6 +202,51 @@ describe('workflow client tool completion', () => { ) }) + /** + * A browser-run workflow reaches the model through this restoration, not the server handler, so + * it needs the same block-log budget: a synthetic run whose block outputs exceed the projection's + * traversal cap would otherwise be withheld whole once a secret is active. + */ + it('bounds bulky block-log outputs so a large browser run still projects', async () => { + const rows = (count: number) => + Array.from({ length: count }, (_, index) => ({ + id: `row_${index}`, + data: { a: 'x', b: 'y', c: 'z', d: 'w' }, + })) + getTrustedWorkflowToolExecution.mockResolvedValue({ + ...trustedExecution('execution-1'), + blockLogs: [ + { blockId: 'small', blockName: 'Small', output: { count: 1 } }, + ...Array.from({ length: 4 }, (_, index) => ({ + blockId: `query-${index}`, + blockName: `Query ${index}`, + output: { rows: rows(5_000) }, + })), + ], + }) + waitForToolConfirmation.mockResolvedValue({ + status: 'success', + data: { workflowId: 'workflow-1', executionId: 'execution-1' }, + }) + + const completion = await waitForWorkflowToolCompletion({ + toolCallId: 'tool-1', + workflowId: 'workflow-1', + timeoutMs: 1_000, + registry: createParentRegistry(), + }) + + const data = completion?.data as Record + expect(data.output).toEqual({ value: 'child read {{PARENT_SECRET}} from execution-1' }) + const logs = data.logs as Array> + expect(logs[0]?.output).toEqual({ count: 1 }) + expect(logs.some((log) => typeof log.output === 'string')).toBe(true) + for (const log of logs.filter((entry) => typeof entry.output === 'string')) { + expect(log.output).toContain('logs get execution-1 --trace') + } + expect(JSON.stringify(completion)).not.toContain('parent-secret-value') + }) + it('preserves the server-confirmed status while omitting unavailable execution content', async () => { const registry = createParentRegistry() waitForToolConfirmation.mockResolvedValue({ diff --git a/apps/sim/lib/mothership/request/tools/client.ts b/apps/sim/lib/mothership/request/tools/client.ts index 6545d0187e5..a312ee82229 100644 --- a/apps/sim/lib/mothership/request/tools/client.ts +++ b/apps/sim/lib/mothership/request/tools/client.ts @@ -15,7 +15,7 @@ import { unsealClientToolContext, } from '@/lib/mothership/request/tools/client-completion-seal.server' import { inspectToolResultForCopilot } from '@/lib/mothership/request/tools/resolved-secret-result' -import { presentWorkflowLogs } from '@/lib/mothership/tools/workflow-output' +import { compactBlockLogOutputs, presentWorkflowLogs } from '@/lib/mothership/tools/workflow-output' import { createStructuralWorkflowToolCompletionData, getWorkflowToolCompletionExecutionId, @@ -376,7 +376,10 @@ export async function waitForWorkflowToolCompletion({ ...(Object.hasOwn(trustedExecution, 'finalOutput') ? { output: trustedExecution.finalOutput } : {}), - logs: trustedExecution.blockLogs, + // `select` reads full values from these logs; only logs echoed whole are bounded. + logs: select?.length + ? trustedExecution.blockLogs + : compactBlockLogOutputs(trustedExecution.blockLogs, executionId), ...(trustedExecution.error !== undefined ? { error: trustedExecution.error } : {}), ...(status === MothershipStreamV1ToolOutcome.cancelled ? { reason: 'user_cancelled', cancelledByUser: true } diff --git a/apps/sim/lib/mothership/request/tools/resolved-secret-result.ts b/apps/sim/lib/mothership/request/tools/resolved-secret-result.ts index 529f1941dd6..07d4e9d4d7f 100644 --- a/apps/sim/lib/mothership/request/tools/resolved-secret-result.ts +++ b/apps/sim/lib/mothership/request/tools/resolved-secret-result.ts @@ -1,6 +1,9 @@ import type { ToolCallEffect, ToolExecutionResult } from '@/lib/mothership/tool-executor/types' import { TOOL_EFFECT_PHASE } from '@/lib/mothership/tool-executor/types' -import { projectResolvedSecretModelJsonContent } from '@/executor/utils/resolved-secret-content-projection' +import { + measureModelContent, + projectResolvedSecretModelJsonContent, +} from '@/executor/utils/resolved-secret-content-projection' import type { ResolvedSecretIncompletenessReason, ResolvedSecretTraceRegistry, @@ -259,26 +262,19 @@ export function projectToolErrorMessageForCopilot( * Sizes the content a withheld result would have carried, for the log line only. * * A complete registry can still refuse content by its encoded size or by the number of values the - * projection must walk (its node cap is reached well before the byte cap by row-shaped payloads). - * Both measures are reported so a `content-refused` line names which one it hit. Numbers only: - * the content itself never reaches the log. + * projection must walk (its value cap is reached well before the byte cap by row-shaped payloads). + * Both measures are reported so a `content-refused` line names which one it hit; counting stops + * at the value cap, so a huge payload is not serialized again just to be logged. Numbers only. */ export function measureWithheldContent(result: ToolExecutionResult): { resultBytes?: number resultValues?: number } { - try { - let values = 0 - const encoded = JSON.stringify( - { output: result.output, error: result.error }, - (_key, value) => { - values += 1 - return value - } - ) - return { resultBytes: Buffer.byteLength(encoded, 'utf8'), resultValues: values } - } catch { - return {} + const measure = measureModelContent({ output: result.output, error: result.error }) + if (!measure) return {} + return { + resultValues: measure.values, + ...(measure.bytes !== undefined ? { resultBytes: measure.bytes } : {}), } } diff --git a/apps/sim/lib/mothership/tools/handlers/function-execute-file-mounts.test.ts b/apps/sim/lib/mothership/tools/handlers/function-execute-file-mounts.test.ts index 55543902bf2..5d8d3e2f210 100644 --- a/apps/sim/lib/mothership/tools/handlers/function-execute-file-mounts.test.ts +++ b/apps/sim/lib/mothership/tools/handlers/function-execute-file-mounts.test.ts @@ -250,7 +250,17 @@ describe('Mothership file mounts bind content and classification to the same rec safe: true, provenance: { status: 'exact', - entries: kind === 'secret' ? [expect.objectContaining({})] : [], + // A mounted file's secret crosses anonymously: its ciphertext binds it, not a name. + entries: + kind === 'secret' + ? [ + { + encryptedValue: 'fixture-ciphertext', + sourceUserId: 'reader', + sourceWorkspaceId: 'workspace', + }, + ] + : [], }, }) } diff --git a/apps/sim/lib/mothership/tools/handlers/workflow/mutations.ts b/apps/sim/lib/mothership/tools/handlers/workflow/mutations.ts index fc304a24365..c9709406a92 100644 --- a/apps/sim/lib/mothership/tools/handlers/workflow/mutations.ts +++ b/apps/sim/lib/mothership/tools/handlers/workflow/mutations.ts @@ -2,7 +2,6 @@ import { createLogger } from '@sim/logger' import { filterUndefined, isPlainRecord, isRecordLike } from '@sim/utils/object' import { createCopilotWorkspaceApiKey } from '@/lib/api-key/application/create-api-key' import { PlatformEvents } from '@/lib/core/telemetry' -import { MAX_INLINE_MATERIALIZATION_BYTES } from '@/lib/execution/payloads/limits' import { messageForCopilotApplicationError } from '@/lib/mothership/application/error' import { executeCopilotApiKeyUseCase } from '@/lib/mothership/application/execute-api-key-use-case' import { @@ -30,7 +29,11 @@ import type { VariableOperation, } from '@/lib/mothership/tools/handlers/param-types' import { requireCopilotWorkspace } from '@/lib/mothership/tools/server/workspace-scope' -import { presentWorkflowLogs } from '@/lib/mothership/tools/workflow-output' +import { + compactBlockLogInputs, + compactBlockLogOutputs, + presentWorkflowLogs, +} from '@/lib/mothership/tools/workflow-output' import { decodeVfsPathSegments, encodeVfsPathSegments } from '@/lib/mothership/vfs/path-utils' import { cancelWorkflowRun } from '@/lib/workflows/application/cancel-run' import { createWorkflow } from '@/lib/workflows/application/create-workflow' @@ -48,101 +51,10 @@ import { } from '@/lib/workflows/application/update-workflow-content' import { sanitizeForCopilot } from '@/lib/workflows/sanitization/json-sanitizer' import { hasExecutionResult, readAttemptedExecutionId } from '@/executor/utils/errors' -import { MAX_CONTENT_NODES } from '@/executor/utils/resolved-secret-content-projection' import type { WorkflowState } from '@/stores/workflows/workflow/types' const logger = createLogger('WorkflowMutations') -/** Above this a Function block's `input.code` is echoed upstream JSON, not code worth reading. */ -const LOG_CODE_INPUT_MAX_CHARS = 240 -/** Any other echoed input string over this is data the caller already has, or can fetch. */ -const LOG_INPUT_STRING_MAX_CHARS = 2_000 -const LOG_INPUT_KEEP_CHARS = 200 - -/** - * Compacts the block inputs echoed back in `logs`. A Function block's `input.code` embeds the - * fully serialized upstream rows, so a seven-block run repeated the same rows several times - * across ~14k chars of tool result. Outputs are bounded separately by {@link compactBlockLogOutputs}, - * and the full input stays one `logs get --trace` away. - */ -function compactBlockLogInputs(logs: unknown, executionId: string | undefined): unknown { - if (!Array.isArray(logs)) return logs - const reference = executionId ?? '' - return logs.map((entry) => { - if (!isPlainRecord(entry) || !isPlainRecord(entry.input)) return entry - const input: Record = {} - for (const [key, value] of Object.entries(entry.input)) { - const limit = key === 'code' ? LOG_CODE_INPUT_MAX_CHARS : LOG_INPUT_STRING_MAX_CHARS - input[key] = - typeof value === 'string' && value.length > limit - ? `${value.slice(0, LOG_INPUT_KEEP_CHARS)} …[${value.length} chars, see logs get ${reference} --trace]` - : value - } - return { ...entry, input } - }) -} - -/** - * Budgets for the block outputs echoed back in `logs`, a quarter of each cap the model-facing - * projection enforces on the whole result. Reaching either cap withholds everything, including - * the final output and error the run was for; those stay intact here and share the remaining - * three quarters with the rest of the envelope. The value budget matters first: row-shaped - * outputs reach the projection's traversal cap long before its byte cap. - */ -const LOG_OUTPUT_VALUE_BUDGET = Math.floor(MAX_CONTENT_NODES / 4) -const LOG_OUTPUT_BYTE_BUDGET = Math.floor(MAX_INLINE_MATERIALIZATION_BYTES / 4) - -/** Counts values the way the projection walks them, and the bytes they encode to. */ -function measureLogOutput(value: unknown): { values: number; bytes: number } { - let values = 0 - const encoded = JSON.stringify(value, (_key, item) => { - values += 1 - return item - }) - return { values, bytes: encoded === undefined ? 0 : Buffer.byteLength(encoded, 'utf8') } -} - -/** - * Replaces the bulkiest block outputs in `logs` with a pointer once they exceed the budgets - * above, largest first, so the rest of the run still reaches the model. The full outputs stay in - * the run's archived trace, one `logs get --trace` away, matching how - * {@link compactBlockLogInputs} treats oversized inputs. - */ -function compactBlockLogOutputs(logs: unknown, executionId: string | undefined): unknown { - if (!Array.isArray(logs)) return logs - const reference = executionId ?? '' - const sizes = logs.map((entry) => - isPlainRecord(entry) && entry.output !== undefined ? measureLogOutput(entry.output) : undefined - ) - let values = 0 - let bytes = 0 - for (const size of sizes) { - values += size?.values ?? 0 - bytes += size?.bytes ?? 0 - } - if (values <= LOG_OUTPUT_VALUE_BUDGET && bytes <= LOG_OUTPUT_BYTE_BUDGET) return logs - - const compacted = [...logs] - const bulkiestFirst = sizes - .map((size, index) => ({ size, index })) - .filter((entry): entry is { size: { values: number; bytes: number }; index: number } => - Boolean(entry.size) - ) - .sort( - (left, right) => right.size.values - left.size.values || right.size.bytes - left.size.bytes - ) - for (const { size, index } of bulkiestFirst) { - if (values <= LOG_OUTPUT_VALUE_BUDGET && bytes <= LOG_OUTPUT_BYTE_BUDGET) break - compacted[index] = { - ...(logs[index] as Record), - output: `…[output omitted: ${size.values} values, ${size.bytes} bytes; see logs get ${reference} --trace]`, - } - values -= size.values - bytes -= size.bytes - } - return compacted -} - function stripBinaryFields(value: unknown): unknown { if (value === null || value === undefined) return value if (typeof value !== 'object') return value diff --git a/apps/sim/lib/mothership/tools/handlers/workflow/run-workflow-result-budget.test.ts b/apps/sim/lib/mothership/tools/handlers/workflow/run-workflow-result-budget.test.ts index 1e8b3a6cf1a..83d02fb3466 100644 --- a/apps/sim/lib/mothership/tools/handlers/workflow/run-workflow-result-budget.test.ts +++ b/apps/sim/lib/mothership/tools/handlers/workflow/run-workflow-result-budget.test.ts @@ -1,27 +1,14 @@ /** - * A run_workflow result sized like the production refusals: thirteen table-query blocks, - * ~11.5k rows, ~9.7 MiB. That is under the 16 MiB byte cap, yet with one active secret the - * projection walked ~138k values against its 100k traversal cap and withheld the whole result, - * leaving the model a bare success. The model-facing result is now bounded before it reaches - * the projection, and the omitted block outputs stay reachable through the run's archived trace. + * A synthetic trace-shaped run_workflow result: about a dozen table-query blocks whose outputs + * exceed the projection's 100k-value traversal cap while staying under its byte cap. With one + * active secret the projection refused the whole result, leaving the model a bare success. The + * model-facing result is now bounded before it reaches the projection. */ - import { executeWorkflowMock } from '@sim/testing/mocks/execute-workflow.mock' -import { - largeValueMetadataMock, - largeValueMetadataMockFns, -} from '@sim/testing/mocks/large-value-metadata.mock' -import { storageServiceMockFns } from '@sim/testing/mocks/storage-service.mock' import { telemetryMock } from '@sim/testing/mocks/telemetry.mock' -import { uploadsMock } from '@sim/testing/mocks/uploads.mock' import { workflowsOrchestrationMock } from '@sim/testing/mocks/workflows-orchestration.mock' import { getErrorMessage } from '@sim/utils/errors' import { beforeEach, describe, expect, it, vi } from 'vitest' -import { clearLargeValueCacheForTests } from '@/lib/execution/payloads/cache' -import { - externalizeExecutionData, - materializeExecutionData, -} from '@/lib/logs/execution/trace-store' import { inspectToolResultForCopilot } from '@/lib/mothership/request/tools/resolved-secret-result' import type { ExecutionContext } from '@/lib/mothership/request/types' import { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry' @@ -43,8 +30,6 @@ vi.mock('@/lib/execution/cancel-workflow-execution', () => ({ })) vi.mock('@/lib/workflows/orchestration', () => workflowsOrchestrationMock) vi.mock('@/lib/core/telemetry', () => telemetryMock) -vi.mock('@/lib/uploads', () => uploadsMock) -vi.mock('@/lib/execution/payloads/large-value-metadata', () => largeValueMetadataMock) import { executeRunWorkflow } from '@/lib/mothership/tools/handlers/workflow/mutations' @@ -56,21 +41,21 @@ const context = { toolCallId: 'tool-call-1', } as ExecutionContext -/** Row counts and stored bytes of the table queries in one production refusal. */ +/** Rows and approximate encoded bytes per block of a synthetic trace-shaped run. */ const TABLE_QUERIES: ReadonlyArray = [ - [32, 9_513], - [2, 2_590], - [1_848, 2_079_072], - [2_533, 1_698_688], - [4_833, 3_179_855], - [1_294, 1_517_841], - [32, 5_563], - [11, 3_246], - [835, 721_833], - [33, 52_482], - [28, 8_274], - [13, 9_669], - [33, 293_266], + [30, 10_000], + [2, 3_000], + [1_850, 2_100_000], + [2_500, 1_700_000], + [4_800, 3_200_000], + [1_300, 1_500_000], + [30, 6_000], + [10, 3_000], + [850, 700_000], + [30, 50_000], + [30, 8_000], + [10, 10_000], + [30, 300_000], ] function tableRows(count: number, bytes: number) { @@ -112,7 +97,7 @@ describe('run_workflow model-facing result budget', () => { it('projects a trace-shaped result with an active secret instead of withholding it', async () => { const logs = traceShapedLogs() - const finalOutput = { summary: `report for ${SECRET}`, rowCount: 11_529 } + const finalOutput = { summary: `report for ${SECRET}`, rowCount: 11_500 } mocks.executeWorkflowUseCase.mockResolvedValue({ success: false, error: `Report block failed after reading ${SECRET}`, @@ -128,7 +113,7 @@ describe('run_workflow model-facing result budget', () => { const output = projection.result.output as Record expect(output.executionId).toBe(EXECUTION_ID) expect(output.success).toBe(false) - expect(output.output).toEqual({ summary: 'report for {{API_KEY}}', rowCount: 11_529 }) + expect(output.output).toEqual({ summary: 'report for {{API_KEY}}', rowCount: 11_500 }) expect(projection.result.error).toBe('Report block failed after reading {{API_KEY}}') const presented = output.logs as Array> expect(presented.map((log) => log.blockName)).toEqual(logs.map((log) => log.blockName)) @@ -143,7 +128,7 @@ describe('run_workflow model-facing result budget', () => { /** The final output is what the run was for, so it is never compacted and needs the headroom. */ it('leaves room for a large final output beside the bounded logs', async () => { - const finalOutput = { rows: tableRows(4_833, 3_179_855) } + const finalOutput = { rows: tableRows(4_800, 3_200_000) } mocks.executeWorkflowUseCase.mockResolvedValue({ success: true, output: finalOutput, @@ -182,7 +167,7 @@ describe('run_workflow model-facing result budget', () => { expect((projection.result.output as Record).output).toEqual(finalOutput) }) - it('keeps every omitted block output reachable through the pointer', async () => { + it('returns selected values in full, bypassing the log budget', async () => { const logs = traceShapedLogs() mocks.executeWorkflowUseCase.mockResolvedValue({ success: true, @@ -190,45 +175,42 @@ describe('run_workflow model-facing result budget', () => { logs, metadata: { executionId: EXECUTION_ID }, }) - const settled = await executeRunWorkflow({ workflowId: 'wf-1' }, context) - const presented = (settled.output as { logs: Array> }).logs - const omitted = presented.filter((log) => typeof log.output === 'string') - expect(omitted.length).toBeGreaterThan(0) - // `logs get --trace` reads the run's archived execution data; archive and read it back. - storageServiceMockFns.mockUploadFile.mockImplementation(async ({ customKey, file }) => { - storageServiceMockFns.mockDownloadFile.mockResolvedValue(file) - return { key: customKey } - }) - largeValueMetadataMockFns.mockRegisterLargeValueOwner.mockResolvedValue(true) - clearLargeValueCacheForTests() - const archiveContext = { - workspaceId: 'workspace-1', - workflowId: 'wf-1', - executionId: EXECUTION_ID, - userId: 'user-1', - } - const slim = await externalizeExecutionData( - { - traceSpans: logs.map((log) => ({ - id: log.blockId, - name: log.blockName, - output: log.output, - })), - }, - archiveContext, - { throwOnError: true } + const settled = await executeRunWorkflow( + { workflowId: 'wf-1', select: ['Query 4.rows'] }, + context ) - clearLargeValueCacheForTests() - const archived = (await materializeExecutionData(slim, archiveContext)) as { - traceSpans: Array<{ name: string; output: unknown }> - } + const projection = inspectToolResultForCopilot(settled, secretRegistry(), 'run_workflow') - for (const log of omitted) { - const original = logs.find((entry) => entry.blockName === log.blockName) - expect(archived.traceSpans.find((span) => span.name === log.blockName)?.output).toEqual( - original?.output - ) + expect(projection.safe).toBe(true) + const output = projection.result.output as Record + expect(output.logsOmitted).toBe(true) + expect(output.selected).toEqual({ 'Query 4.rows': logs[4].output.rows }) + }) + + /** The pointer is written before secret projection, so it must not vary with a secret's length. */ + it('reports nothing about an omitted output that depends on secret length', async () => { + async function pointerFor(secret: string) { + mocks.executeWorkflowUseCase.mockResolvedValue({ + success: true, + output: {}, + logs: [ + { + blockId: 'query', + blockName: 'Query', + success: true, + output: { rows: tableRows(4_800, 3_200_000), token: secret }, + }, + ], + metadata: { executionId: EXECUTION_ID }, + }) + const settled = await executeRunWorkflow({ workflowId: 'wf-1' }, context) + return (settled.output as { logs: Array<{ output: unknown }> }).logs[0]?.output } + + const short = await pointerFor('short-secret-1') + const long = await pointerFor('a-considerably-longer-secret-value-for-the-same-slot') + expect(short).toEqual(expect.stringContaining(`logs get ${EXECUTION_ID} --trace`)) + expect(long).toBe(short) }) }) diff --git a/apps/sim/lib/mothership/tools/workflow-output.ts b/apps/sim/lib/mothership/tools/workflow-output.ts index a51877c12e4..bd198d617da 100644 --- a/apps/sim/lib/mothership/tools/workflow-output.ts +++ b/apps/sim/lib/mothership/tools/workflow-output.ts @@ -1,4 +1,9 @@ -import { isRecordLike } from '@sim/utils/object' +import { isPlainRecord, isRecordLike } from '@sim/utils/object' +import { + MAX_CONTENT_NODES, + MAX_MODEL_CONTENT_BYTES, + measureModelContent, +} from '@/executor/utils/resolved-secret-content-projection' /** Shared presentation for server execution and trusted client execution restoration. */ export function presentWorkflowLogs(logs: unknown, select?: string[]): Record { @@ -44,3 +49,97 @@ function selectFromLogs(selectors: string[], logs: unknown[]): Record --trace`. + */ +export function compactBlockLogInputs(logs: unknown, executionId: string | undefined): unknown { + if (!Array.isArray(logs)) return logs + const reference = executionId ?? '' + return logs.map((entry) => { + if (!isPlainRecord(entry) || !isPlainRecord(entry.input)) return entry + const input: Record = {} + for (const [key, value] of Object.entries(entry.input)) { + const limit = key === 'code' ? LOG_CODE_INPUT_MAX_CHARS : LOG_INPUT_STRING_MAX_CHARS + input[key] = + typeof value === 'string' && value.length > limit + ? `${value.slice(0, LOG_INPUT_KEEP_CHARS)} …[${value.length} chars, see logs get ${reference} --trace]` + : value + } + return { ...entry, input } + }) +} + +/** + * Budgets for the block outputs echoed back in `logs`, a quarter of each cap the model-facing + * projection enforces on the whole result. Reaching either cap withholds everything, including + * the final output and error the run was for; those are never compacted and share the remaining + * three quarters with the rest of the envelope. The value budget usually binds first: row-shaped + * outputs reach the projection's traversal cap long before its byte cap. + */ +const LOG_OUTPUT_VALUE_BUDGET = Math.floor(MAX_CONTENT_NODES / 4) +const LOG_OUTPUT_BYTE_BUDGET = Math.floor(MAX_MODEL_CONTENT_BYTES / 4) + +interface MeasuredLogOutput { + index: number + entry: Record + values: number + bytes: number + /** What the pointer reports: exact, past the projection's cap, or unencodable. */ + label: string +} + +/** + * Replaces the bulkiest block outputs in `logs` with a pointer once they exceed the budgets + * above, largest first, so the rest of the run still reaches the model. + * + * The pointer names only the value count: bytes are measured before secret projection, so they + * would disclose a secret's length. It says "inspect" rather than promising the full value, since + * the stored trace can itself be summarized. + */ +export function compactBlockLogOutputs(logs: unknown, executionId: string | undefined): unknown { + if (!Array.isArray(logs)) return logs + const reference = executionId ?? '' + const measured: MeasuredLogOutput[] = [] + let values = 0 + let bytes = 0 + for (const [index, entry] of logs.entries()) { + if (!isPlainRecord(entry) || entry.output === undefined) continue + const measure = measureModelContent(entry.output) + const exact = measure?.bytes !== undefined + const size = { + values: exact ? measure.values : MAX_CONTENT_NODES + 1, + bytes: measure?.bytes ?? MAX_MODEL_CONTENT_BYTES + 1, + } + const label = !measure + ? 'output omitted' + : `output omitted: ${exact ? measure.values : `over ${MAX_CONTENT_NODES}`} values` + measured.push({ index, entry, ...size, label }) + values += size.values + bytes += size.bytes + } + if (values <= LOG_OUTPUT_VALUE_BUDGET && bytes <= LOG_OUTPUT_BYTE_BUDGET) return logs + + measured.sort((left, right) => right.values - left.values || right.bytes - left.bytes) + const compacted = [...logs] + for (const item of measured) { + if (values <= LOG_OUTPUT_VALUE_BUDGET && bytes <= LOG_OUTPUT_BYTE_BUDGET) break + compacted[item.index] = { + ...item.entry, + output: `…[${item.label}; inspect with logs get ${reference} --trace]`, + } + values -= item.values + bytes -= item.bytes + } + return compacted +} From b44e3c18016624c90c33c993b9e749b05ffc74b9 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 30 Sep 2026 02:14:44 -0700 Subject: [PATCH 06/14] fix(mothership): resolve a browser run's select before the secret projection The browser-run restoration projected the full raw block logs and only then applied `select`, so a large run was still withheld whenever a selector was given. The server handler selects first. Both paths now build their log fields through one shared `presentWorkflowLogsForModel`, from raw logs and before projection: a `select` resolves against the full logs and replaces them; otherwise the echoed logs are bounded. Selected values are still projected, so a selected secret is redacted. --- .../mothership/request/tools/client.test.ts | 40 +++++++++++++++++++ .../lib/mothership/request/tools/client.ts | 12 ++---- .../tools/handlers/workflow/mutations.ts | 9 +---- .../lib/mothership/tools/workflow-output.ts | 21 +++++++++- 4 files changed, 65 insertions(+), 17 deletions(-) diff --git a/apps/sim/lib/mothership/request/tools/client.test.ts b/apps/sim/lib/mothership/request/tools/client.test.ts index 5898126afd3..8e6b674e44d 100644 --- a/apps/sim/lib/mothership/request/tools/client.test.ts +++ b/apps/sim/lib/mothership/request/tools/client.test.ts @@ -247,6 +247,46 @@ describe('workflow client tool completion', () => { expect(JSON.stringify(completion)).not.toContain('parent-secret-value') }) + /** Parity with the server path: a `select` is resolved from raw logs before projection. */ + it('projects selected values from a large browser run instead of withholding it', async () => { + const rows = (count: number) => + Array.from({ length: count }, (_, index) => ({ + id: `row_${index}`, + data: { a: 'x', b: 'y', c: 'z', d: 'w' }, + })) + getTrustedWorkflowToolExecution.mockResolvedValue({ + ...trustedExecution('execution-1'), + blockLogs: [ + { blockId: 'reader', blockName: 'Reader', output: { token: 'parent-secret-value', n: 2 } }, + ...Array.from({ length: 4 }, (_, index) => ({ + blockId: `query-${index}`, + blockName: `Query ${index}`, + output: { rows: rows(5_000) }, + })), + ], + }) + waitForToolConfirmation.mockResolvedValue({ + status: 'success', + data: { workflowId: 'workflow-1', executionId: 'execution-1' }, + }) + + const completion = await waitForWorkflowToolCompletion({ + toolCallId: 'tool-1', + workflowId: 'workflow-1', + timeoutMs: 1_000, + registry: createParentRegistry(), + select: ['Reader.token', 'Reader.n'], + }) + + expect(completion?.data).toMatchObject({ + output: { value: 'child read {{PARENT_SECRET}} from execution-1' }, + selected: { 'Reader.token': '{{PARENT_SECRET}}', 'Reader.n': 2 }, + logsOmitted: true, + }) + expect(completion?.data).not.toHaveProperty('logs') + expect(JSON.stringify(completion)).not.toContain('parent-secret-value') + }) + it('preserves the server-confirmed status while omitting unavailable execution content', async () => { const registry = createParentRegistry() waitForToolConfirmation.mockResolvedValue({ diff --git a/apps/sim/lib/mothership/request/tools/client.ts b/apps/sim/lib/mothership/request/tools/client.ts index a312ee82229..fc3ebb53081 100644 --- a/apps/sim/lib/mothership/request/tools/client.ts +++ b/apps/sim/lib/mothership/request/tools/client.ts @@ -15,7 +15,7 @@ import { unsealClientToolContext, } from '@/lib/mothership/request/tools/client-completion-seal.server' import { inspectToolResultForCopilot } from '@/lib/mothership/request/tools/resolved-secret-result' -import { compactBlockLogOutputs, presentWorkflowLogs } from '@/lib/mothership/tools/workflow-output' +import { presentWorkflowLogsForModel } from '@/lib/mothership/tools/workflow-output' import { createStructuralWorkflowToolCompletionData, getWorkflowToolCompletionExecutionId, @@ -376,10 +376,8 @@ export async function waitForWorkflowToolCompletion({ ...(Object.hasOwn(trustedExecution, 'finalOutput') ? { output: trustedExecution.finalOutput } : {}), - // `select` reads full values from these logs; only logs echoed whole are bounded. - logs: select?.length - ? trustedExecution.blockLogs - : compactBlockLogOutputs(trustedExecution.blockLogs, executionId), + // Built from raw logs before projection, matching the server handler's presentation. + ...presentWorkflowLogsForModel(trustedExecution.blockLogs, executionId, select), ...(trustedExecution.error !== undefined ? { error: trustedExecution.error } : {}), ...(status === MothershipStreamV1ToolOutcome.cancelled ? { reason: 'user_cancelled', cancelledByUser: true } @@ -397,10 +395,8 @@ export async function waitForWorkflowToolCompletion({ ) const projected = projection.result const projectedData = isPlainRecord(projected.output) ? projected.output : {} - const { logs, ...projectedFields } = projectedData const data = { - ...projectedFields, - ...(Object.hasOwn(projectedData, 'logs') ? presentWorkflowLogs(logs, select) : {}), + ...projectedData, ...createStructuralWorkflowToolCompletionData(status, workflowId, executionId), } const message = diff --git a/apps/sim/lib/mothership/tools/handlers/workflow/mutations.ts b/apps/sim/lib/mothership/tools/handlers/workflow/mutations.ts index c9709406a92..2e21fae9a78 100644 --- a/apps/sim/lib/mothership/tools/handlers/workflow/mutations.ts +++ b/apps/sim/lib/mothership/tools/handlers/workflow/mutations.ts @@ -31,8 +31,7 @@ import type { import { requireCopilotWorkspace } from '@/lib/mothership/tools/server/workspace-scope' import { compactBlockLogInputs, - compactBlockLogOutputs, - presentWorkflowLogs, + presentWorkflowLogsForModel, } from '@/lib/mothership/tools/workflow-output' import { decodeVfsPathSegments, encodeVfsPathSegments } from '@/lib/mothership/vfs/path-utils' import { cancelWorkflowRun } from '@/lib/workflows/application/cancel-run' @@ -157,11 +156,7 @@ function buildExecutionOutput( ...extra, output: lifted ? lifted.output : output, ...(lifted ? { outputFrom: lifted.outputFrom } : {}), - // `select` reads full values from the run's own logs, so only the echoed logs are bounded. - ...presentWorkflowLogs( - select?.length ? logs : compactBlockLogOutputs(logs, executionId), - select - ), + ...presentWorkflowLogsForModel(logs, executionId, select), }, error: result.success ? undefined diff --git a/apps/sim/lib/mothership/tools/workflow-output.ts b/apps/sim/lib/mothership/tools/workflow-output.ts index bd198d617da..a1527ac7346 100644 --- a/apps/sim/lib/mothership/tools/workflow-output.ts +++ b/apps/sim/lib/mothership/tools/workflow-output.ts @@ -6,12 +6,29 @@ import { } from '@/executor/utils/resolved-secret-content-projection' /** Shared presentation for server execution and trusted client execution restoration. */ -export function presentWorkflowLogs(logs: unknown, select?: string[]): Record { +function presentWorkflowLogs(logs: unknown, select?: string[]): Record { return select?.length ? { selected: selectFromLogs(select, Array.isArray(logs) ? logs : []), logsOmitted: true } : { logs } } +/** + * The model-facing log fields for one run, built from raw logs before secret projection so both the + * server handler and browser-run restoration present the same bounded shape: a `select` resolves + * against the full logs and replaces them, otherwise the echoed logs are bounded by + * {@link compactBlockLogOutputs}. + */ +export function presentWorkflowLogsForModel( + logs: unknown, + executionId: string | undefined, + select?: string[] +): Record { + return presentWorkflowLogs( + select?.length ? logs : compactBlockLogOutputs(logs, executionId), + select + ) +} + /** The executor's block-name rule: lowercase, whitespace and dots removed. */ function normalizeSelectorHead(value: string): string { return value.toLowerCase().replace(/[\s.]+/g, '') @@ -107,7 +124,7 @@ interface MeasuredLogOutput { * would disclose a secret's length. It says "inspect" rather than promising the full value, since * the stored trace can itself be summarized. */ -export function compactBlockLogOutputs(logs: unknown, executionId: string | undefined): unknown { +function compactBlockLogOutputs(logs: unknown, executionId: string | undefined): unknown { if (!Array.isArray(logs)) return logs const reference = executionId ?? '' const measured: MeasuredLogOutput[] = [] From ec785db7675c28b26d95490c917ef32c275120fd Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 30 Sep 2026 02:34:13 -0700 Subject: [PATCH 07/14] test(providers): expect the withheld reason on provider tool results The provider tool path projects through the same withheld-result shape, so an omitted model result now carries `resultWithheld` and its fixed-wording `withheldReason`. The expectations still asserted the old empty output. --- apps/sim/providers/runtime-context.test.ts | 26 ++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/apps/sim/providers/runtime-context.test.ts b/apps/sim/providers/runtime-context.test.ts index f8782cf3469..6c05cefbe89 100644 --- a/apps/sim/providers/runtime-context.test.ts +++ b/apps/sim/providers/runtime-context.test.ts @@ -815,7 +815,13 @@ describe('provider runtime context', () => { () => executeProviderTool('custom-tool', {}) ) - expect(result).toEqual({ success: true, output: {} }) + expect(result).toEqual({ + success: true, + output: { + resultWithheld: true, + withheldReason: expect.stringMatching(/could not be verified/), + }, + }) expect(registry.isComplete()).toBe(false) expect(registry.getActiveMatches()).toEqual([]) }) @@ -840,7 +846,10 @@ describe('provider runtime context', () => { expect(result).toEqual({ success: false, - output: {}, + output: { + resultWithheld: true, + withheldReason: expect.stringMatching(/could not be verified/), + }, error: 'Tool execution settled, but its result could not be returned safely. Do not retry a mutation automatically.', }) @@ -867,7 +876,13 @@ describe('provider runtime context', () => { ) expect(execution.rawResponse.output).toHaveProperty('value', 'secret-value') - expect(execution.modelResponse).toEqual({ success: true, output: {} }) + expect(execution.modelResponse).toEqual({ + success: true, + output: { + resultWithheld: true, + withheldReason: expect.stringMatching(/could not be checked/), + }, + }) expect(registry.isComplete()).toBe(true) expect(registry.getActiveMatches()).toEqual([ { plaintext: 'secret-value', replacement: '{{TOKEN}}' }, @@ -895,7 +910,10 @@ describe('provider runtime context', () => { }) expect(execution.modelResponse).toEqual({ success: false, - output: {}, + output: { + resultWithheld: true, + withheldReason: expect.stringMatching(/could not be verified/), + }, error: 'Tool execution settled, but its result could not be returned safely. Do not retry a mutation automatically.', }) From 42e012e8cc6167c9b91b6bd4f7d0c7435bda5afb Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 30 Sep 2026 02:51:56 -0700 Subject: [PATCH 08/14] fix(mothership): bound lifted outputs, length-blind input markers, and a walk-based measure - run_block and run_workflow_until_block lift the stopping block's output into `output`. That copy is now bounded with the same budget and pointer as a block-log output, so one huge block no longer gets the whole response withheld. - The truncated-input marker no longer carries the input's length. It is written before secret projection, so the length disclosed a secret's length. - `measureModelContent` now walks the value by JSON's rules instead of serializing it. It stops at the first value, byte, or depth limit it passes and reports `exceeded`, measuring a string only while it fits the remaining byte budget. An output past a limit, including one nested past the projection's depth limit, becomes a pointer instead of voiding the run. - The browser-run restoration now truncates echoed block inputs as the server handler does: both paths build their log fields through `presentWorkflowLogsForModel`. --- ...resolved-secret-content-projection.test.ts | 48 ++++++++ .../resolved-secret-content-projection.ts | 103 ++++++++++++++--- .../mothership/request/tools/client.test.ts | 30 +++++ .../request/tools/resolved-secret-result.ts | 9 +- .../tools/handlers/workflow/mutations.test.ts | 4 +- .../tools/handlers/workflow/mutations.ts | 6 +- .../run-workflow-result-budget.test.ts | 82 ++++++++++++- .../lib/mothership/tools/workflow-output.ts | 109 +++++++++++------- 8 files changed, 325 insertions(+), 66 deletions(-) diff --git a/apps/sim/executor/utils/resolved-secret-content-projection.test.ts b/apps/sim/executor/utils/resolved-secret-content-projection.test.ts index ce677365dbb..d3b8ca4dc86 100644 --- a/apps/sim/executor/utils/resolved-secret-content-projection.test.ts +++ b/apps/sim/executor/utils/resolved-secret-content-projection.test.ts @@ -1,6 +1,9 @@ import { describe, expect, it, vi } from 'vitest' import { createResolvedSecretMatcher, + MAX_CONTENT_NODES, + MAX_MODEL_CONTENT_BYTES, + measureModelContent, projectResolvedSecretContent, projectResolvedSecretDiagnosticError, projectResolvedSecretModelContent, @@ -307,3 +310,48 @@ describe('literals too small to identify anything', () => { }) }) }) + +/** + * The measure feeds budgets and log lines for payloads that may be far over the caps, so it must + * report "over" without materializing them, and must agree with the projection on what is over. + */ +describe('measureModelContent', () => { + it('reports a string past the byte cap as over it', () => { + const measure = measureModelContent({ text: 'x'.repeat(MAX_MODEL_CONTENT_BYTES + 1) }) + expect(measure).toMatchObject({ exceeded: true }) + }) + + it('reports content past the value cap as over it', () => { + const measure = measureModelContent(Array.from({ length: MAX_CONTENT_NODES + 10 }, () => 1)) + expect(measure).toMatchObject({ exceeded: true }) + }) + + it('reports content nested past the projection depth limit as over it', () => { + let deep: Record = { leaf: 1 } + for (let level = 0; level < 150; level += 1) deep = { next: deep } + expect(measureModelContent(deep)).toMatchObject({ exceeded: true }) + }) + + it('measures ordinary content exactly, as JSON encodes it', () => { + const value = { a: 'é', list: [1, null, true], at: new Date(0) } + expect(measureModelContent(value)).toEqual({ + exceeded: false, + values: 7, + bytes: Buffer.byteLength(JSON.stringify(value), 'utf8'), + }) + }) + + it.each([ + ['a BigInt', { n: BigInt(1) }], + [ + 'a cycle', + (() => { + const cyclic: Record = {} + cyclic.self = cyclic + return cyclic + })(), + ], + ])('returns nothing for %s, which JSON cannot encode', (_label, value) => { + expect(measureModelContent(value)).toBeUndefined() + }) +}) diff --git a/apps/sim/executor/utils/resolved-secret-content-projection.ts b/apps/sim/executor/utils/resolved-secret-content-projection.ts index 0391fa19650..bc7eb1f256f 100644 --- a/apps/sim/executor/utils/resolved-secret-content-projection.ts +++ b/apps/sim/executor/utils/resolved-secret-content-projection.ts @@ -362,30 +362,105 @@ export function projectResolvedSecretContent( /** Size of a value as the model-content projection sees it, for budgeting and diagnostics. */ export interface ModelContentMeasure { - /** Values walked, stopping at `MAX_CONTENT_NODES + 1`: past the cap the exact count is moot. */ + /** Values walked: the root and every array item and object property value JSON encodes. */ values: number - /** Encoded bytes; absent when counting stopped at the value cap. */ - bytes?: number + /** Bytes of the value's JSON encoding. */ + bytes: number + /** + * True once the walk passed the projection's value, byte, or depth limit and stopped; `values` + * and `bytes` are then only what was counted before stopping. + */ + exceeded: boolean } -class ModelContentMeasureLimit extends Error {} +class ModelContentMeasureExceeded extends Error {} +class ModelContentUnencodable extends Error {} /** - * Measures a value in the units the projection caps, without walking past the value cap: a - * payload that is already over it is reported as over rather than serialized in full. Returns - * undefined for a value JSON cannot encode (a BigInt, a cycle), which the projection refuses too. + * Measures a value in the units the projection caps, following JSON's encoding rules, without + * serializing it: strings are measured one at a time and only while they fit the remaining byte + * budget, and the walk stops at the first limit it passes. Returns undefined for a value JSON + * cannot encode (a BigInt, a cycle), which the projection refuses too. */ export function measureModelContent(value: unknown): ModelContentMeasure | undefined { let values = 0 + let bytes = 0 + const ancestors = new Set() + + const addBytes = (count: number): void => { + bytes += count + if (bytes > MAX_MODEL_CONTENT_BYTES) throw new ModelContentMeasureExceeded() + } + const addString = (text: string): void => { + // A JSON string is never shorter than its UTF-16 length plus its quotes. + if (text.length + 2 > MAX_MODEL_CONTENT_BYTES - bytes) throw new ModelContentMeasureExceeded() + addBytes(Buffer.byteLength(JSON.stringify(text), 'utf8')) + } + const walk = (raw: unknown, key: string, depth: number): void => { + const item = + raw !== null && + typeof raw === 'object' && + typeof (raw as { toJSON?: unknown }).toJSON === 'function' + ? (raw as { toJSON: (key: string) => unknown }).toJSON(key) + : raw + values += 1 + if (values > MAX_CONTENT_NODES || depth > MAX_CONTENT_DEPTH) { + throw new ModelContentMeasureExceeded() + } + if (typeof item === 'string') { + addString(item) + return + } + if (typeof item === 'number') { + addBytes(Number.isFinite(item) ? String(item).length : 4) + return + } + if (typeof item === 'boolean') { + addBytes(item ? 4 : 5) + return + } + if (typeof item === 'bigint') throw new ModelContentUnencodable() + if (item === null || typeof item !== 'object') { + addBytes(4) + return + } + if (ancestors.has(item)) throw new ModelContentUnencodable() + ancestors.add(item) + if (Array.isArray(item)) { + addBytes(2 + Math.max(0, item.length - 1)) + for (const [index, child] of item.entries()) { + if (child === undefined || typeof child === 'function' || typeof child === 'symbol') { + values += 1 + addBytes(4) + } else { + walk(child, String(index), depth + 1) + } + } + } else { + addBytes(2) + let first = true + for (const [childKey, child] of Object.entries(item)) { + if (child === undefined || typeof child === 'function' || typeof child === 'symbol') + continue + if (!first) addBytes(1) + first = false + addString(childKey) + addBytes(1) + walk(child, childKey, depth + 1) + } + } + ancestors.delete(item) + } + + if (value === undefined || typeof value === 'function' || typeof value === 'symbol') { + return { values: 0, bytes: 0, exceeded: false } + } try { - const encoded = JSON.stringify(value, (_key, item: unknown) => { - values += 1 - if (values > MAX_CONTENT_NODES) throw new ModelContentMeasureLimit() - return item - }) - return { values, bytes: encoded === undefined ? 0 : Buffer.byteLength(encoded, 'utf8') } + walk(value, '', 0) + return { values, bytes, exceeded: false } } catch (error) { - return error instanceof ModelContentMeasureLimit ? { values } : undefined + if (error instanceof ModelContentMeasureExceeded) return { values, bytes, exceeded: true } + return undefined } } diff --git a/apps/sim/lib/mothership/request/tools/client.test.ts b/apps/sim/lib/mothership/request/tools/client.test.ts index 8e6b674e44d..444aa64713d 100644 --- a/apps/sim/lib/mothership/request/tools/client.test.ts +++ b/apps/sim/lib/mothership/request/tools/client.test.ts @@ -287,6 +287,36 @@ describe('workflow client tool completion', () => { expect(JSON.stringify(completion)).not.toContain('parent-secret-value') }) + /** Parity with the server path: echoed block inputs are truncated before projection. */ + it('truncates long echoed block inputs on a browser run', async () => { + getTrustedWorkflowToolExecution.mockResolvedValue({ + ...trustedExecution('execution-1'), + blockLogs: [ + { + blockId: 'fn', + blockName: 'Function', + input: { code: 'c'.repeat(5_000) }, + output: { ok: true }, + }, + ], + }) + waitForToolConfirmation.mockResolvedValue({ + status: 'success', + data: { workflowId: 'workflow-1', executionId: 'execution-1' }, + }) + + const completion = await waitForWorkflowToolCompletion({ + toolCallId: 'tool-1', + workflowId: 'workflow-1', + timeoutMs: 1_000, + registry: createParentRegistry(), + }) + + const logs = (completion?.data as { logs: Array<{ input: { code: string } }> }).logs + expect(logs[0]?.input.code).toContain('logs get execution-1 --trace') + expect(logs[0]?.input.code.length).toBeLessThan(400) + }) + it('preserves the server-confirmed status while omitting unavailable execution content', async () => { const registry = createParentRegistry() waitForToolConfirmation.mockResolvedValue({ diff --git a/apps/sim/lib/mothership/request/tools/resolved-secret-result.ts b/apps/sim/lib/mothership/request/tools/resolved-secret-result.ts index 07d4e9d4d7f..f28fd786357 100644 --- a/apps/sim/lib/mothership/request/tools/resolved-secret-result.ts +++ b/apps/sim/lib/mothership/request/tools/resolved-secret-result.ts @@ -263,18 +263,21 @@ export function projectToolErrorMessageForCopilot( * * A complete registry can still refuse content by its encoded size or by the number of values the * projection must walk (its value cap is reached well before the byte cap by row-shaped payloads). - * Both measures are reported so a `content-refused` line names which one it hit; counting stops - * at the value cap, so a huge payload is not serialized again just to be logged. Numbers only. + * Both measures are reported so a `content-refused` line names which one it hit. Counting stops at + * the first limit passed (`resultOverLimit`), so a huge payload is never serialized just to be + * logged. Numbers only. */ export function measureWithheldContent(result: ToolExecutionResult): { resultBytes?: number resultValues?: number + resultOverLimit?: true } { const measure = measureModelContent({ output: result.output, error: result.error }) if (!measure) return {} return { resultValues: measure.values, - ...(measure.bytes !== undefined ? { resultBytes: measure.bytes } : {}), + resultBytes: measure.bytes, + ...(measure.exceeded ? { resultOverLimit: true } : {}), } } diff --git a/apps/sim/lib/mothership/tools/handlers/workflow/mutations.test.ts b/apps/sim/lib/mothership/tools/handlers/workflow/mutations.test.ts index d73cb186d2b..a682df2259d 100644 --- a/apps/sim/lib/mothership/tools/handlers/workflow/mutations.test.ts +++ b/apps/sim/lib/mothership/tools/handlers/workflow/mutations.test.ts @@ -181,10 +181,10 @@ describe('workflow mutation Copilot adapters', () => { } expect(code.length).toBeGreaterThan(240) expect(output.logs[0].input.code).toBe( - `${code.slice(0, 200)} …[${code.length} chars, see logs get execution-1 --trace]` + `${code.slice(0, 200)} …[truncated; inspect with logs get execution-1 --trace]` ) expect(output.logs[0].input.note).toBe( - `${'n'.repeat(200)} …[2001 chars, see logs get execution-1 --trace]` + `${'n'.repeat(200)} …[truncated; inspect with logs get execution-1 --trace]` ) expect(output.logs[0].input.language).toBe('javascript') expect(output.logs[0].output.result).toBe(code) diff --git a/apps/sim/lib/mothership/tools/handlers/workflow/mutations.ts b/apps/sim/lib/mothership/tools/handlers/workflow/mutations.ts index 2e21fae9a78..e8371304d67 100644 --- a/apps/sim/lib/mothership/tools/handlers/workflow/mutations.ts +++ b/apps/sim/lib/mothership/tools/handlers/workflow/mutations.ts @@ -30,7 +30,7 @@ import type { } from '@/lib/mothership/tools/handlers/param-types' import { requireCopilotWorkspace } from '@/lib/mothership/tools/server/workspace-scope' import { - compactBlockLogInputs, + compactLiftedBlockOutput, presentWorkflowLogsForModel, } from '@/lib/mothership/tools/workflow-output' import { decodeVfsPathSegments, encodeVfsPathSegments } from '@/lib/mothership/vfs/path-utils' @@ -144,7 +144,7 @@ function buildExecutionOutput( ): ToolCallResult { const executionId = result.metadata?.executionId const output = stripBinaryFields(result.output) - const logs = compactBlockLogInputs(stripBinaryFields(result.logs), executionId) + const logs = stripBinaryFields(result.logs) const lifted = isEmptyOutput(output) ? lastBlockOutput(logs) : undefined // A caller that names the outputs it wants gets those and nothing else: a seven-block // run otherwise costs ~14K chars of logs to learn one headline. @@ -154,7 +154,7 @@ function buildExecutionOutput( executionId, success: result.success, ...extra, - output: lifted ? lifted.output : output, + output: lifted ? compactLiftedBlockOutput(lifted.output, executionId) : output, ...(lifted ? { outputFrom: lifted.outputFrom } : {}), ...presentWorkflowLogsForModel(logs, executionId, select), }, diff --git a/apps/sim/lib/mothership/tools/handlers/workflow/run-workflow-result-budget.test.ts b/apps/sim/lib/mothership/tools/handlers/workflow/run-workflow-result-budget.test.ts index 83d02fb3466..795fbb1aa7f 100644 --- a/apps/sim/lib/mothership/tools/handlers/workflow/run-workflow-result-budget.test.ts +++ b/apps/sim/lib/mothership/tools/handlers/workflow/run-workflow-result-budget.test.ts @@ -31,7 +31,10 @@ vi.mock('@/lib/execution/cancel-workflow-execution', () => ({ vi.mock('@/lib/workflows/orchestration', () => workflowsOrchestrationMock) vi.mock('@/lib/core/telemetry', () => telemetryMock) -import { executeRunWorkflow } from '@/lib/mothership/tools/handlers/workflow/mutations' +import { + executeRunWorkflow, + executeRunWorkflowUntilBlock, +} from '@/lib/mothership/tools/handlers/workflow/mutations' const EXECUTION_ID = '0f4d5a4c-6a1e-4c2f-9b7d-2c8f1a3e5d90' const SECRET = 'fake-secret-for-test-only' @@ -213,4 +216,81 @@ describe('run_workflow model-facing result budget', () => { expect(short).toEqual(expect.stringContaining(`logs get ${EXECUTION_ID} --trace`)) expect(long).toBe(short) }) + + /** run_workflow_until_block lifts the stopping block's output into `output`; that copy is bounded too. */ + it('bounds a lifted terminal block output instead of withholding the run', async () => { + const narrow = (count: number) => + Array.from({ length: count }, (_, index) => ({ id: `r${index}`, data: { a: 'x', b: 'y' } })) + mocks.executeWorkflowUseCase.mockResolvedValue({ + success: true, + output: {}, + logs: [ + { blockId: 'start', blockName: 'Start', success: true, output: { ok: true } }, + { blockId: 'query', blockName: 'Query', success: true, output: { rows: narrow(25_000) } }, + ], + metadata: { executionId: EXECUTION_ID }, + }) + + const settled = await executeRunWorkflowUntilBlock( + { workflowId: 'wf-1', stopAfterBlockId: 'query' }, + context + ) + const projection = inspectToolResultForCopilot(settled, secretRegistry(), 'run_workflow') + + expect(projection.safe).toBe(true) + const output = projection.result.output as Record + expect(output.output).toEqual(expect.stringContaining(`logs get ${EXECUTION_ID} --trace`)) + expect(output.outputFrom).toEqual({ blockId: 'query', blockName: 'Query' }) + expect(output.stoppedAfterBlockId).toBe('query') + }) + + /** The truncation marker is written before secret projection, so it must not carry a length. */ + it('marks a truncated block input without disclosing its length', async () => { + async function inputFor(secret: string) { + mocks.executeWorkflowUseCase.mockResolvedValue({ + success: true, + output: { done: true }, + logs: [ + { + blockId: 'fn', + blockName: 'Function', + success: true, + input: { code: `${'a'.repeat(300)}${secret}${'b'.repeat(3_000)}` }, + output: { ok: true }, + }, + ], + metadata: { executionId: EXECUTION_ID }, + }) + const settled = await executeRunWorkflow({ workflowId: 'wf-1' }, context) + return (settled.output as { logs: Array<{ input: { code: string } }> }).logs[0]?.input.code + } + + const short = await inputFor('short-secret-1') + const long = await inputFor('a-considerably-longer-secret-value-for-the-same-slot') + expect(short).toEqual(expect.stringContaining(`logs get ${EXECUTION_ID} --trace`)) + expect(long).toBe(short) + }) + + /** The projection refuses content past its depth limit however few values it holds. */ + it('replaces a block output nested past the projection depth limit', async () => { + let deep: Record = { leaf: true } + for (let level = 0; level < 150; level += 1) deep = { next: deep } + mocks.executeWorkflowUseCase.mockResolvedValue({ + success: true, + output: { done: true }, + logs: [ + { blockId: 'small', blockName: 'Small', success: true, output: { ok: true } }, + { blockId: 'deep', blockName: 'Deep', success: true, output: deep }, + ], + metadata: { executionId: EXECUTION_ID }, + }) + + const settled = await executeRunWorkflow({ workflowId: 'wf-1' }, context) + const projection = inspectToolResultForCopilot(settled, secretRegistry(), 'run_workflow') + + expect(projection.safe).toBe(true) + const logs = (projection.result.output as { logs: Array> }).logs + expect(logs[0]?.output).toEqual({ ok: true }) + expect(logs[1]?.output).toEqual(expect.stringContaining(`logs get ${EXECUTION_ID} --trace`)) + }) }) diff --git a/apps/sim/lib/mothership/tools/workflow-output.ts b/apps/sim/lib/mothership/tools/workflow-output.ts index a1527ac7346..06537b553fc 100644 --- a/apps/sim/lib/mothership/tools/workflow-output.ts +++ b/apps/sim/lib/mothership/tools/workflow-output.ts @@ -15,17 +15,17 @@ function presentWorkflowLogs(logs: unknown, select?: string[]): Record { + if (select?.length) return presentWorkflowLogs(logs, select) return presentWorkflowLogs( - select?.length ? logs : compactBlockLogOutputs(logs, executionId), - select + compactBlockLogOutputs(compactBlockLogInputs(logs, executionId), executionId) ) } @@ -76,11 +76,10 @@ const LOG_INPUT_KEEP_CHARS = 200 /** * Compacts the block inputs echoed back in `logs`. A Function block's `input.code` embeds the * fully serialized upstream rows, so a seven-block run repeated the same rows several times - * across ~14k chars of tool result. Outputs are bounded separately by - * {@link compactBlockLogOutputs}, and the input stays inspectable with - * `logs get --trace`. + * across ~14k chars of tool result. The marker carries no length: it is written before secret + * projection, so a length would disclose the length of any secret in the input. */ -export function compactBlockLogInputs(logs: unknown, executionId: string | undefined): unknown { +function compactBlockLogInputs(logs: unknown, executionId: string | undefined): unknown { if (!Array.isArray(logs)) return logs const reference = executionId ?? '' return logs.map((entry) => { @@ -90,7 +89,7 @@ export function compactBlockLogInputs(logs: unknown, executionId: string | undef const limit = key === 'code' ? LOG_CODE_INPUT_MAX_CHARS : LOG_INPUT_STRING_MAX_CHARS input[key] = typeof value === 'string' && value.length > limit - ? `${value.slice(0, LOG_INPUT_KEEP_CHARS)} …[${value.length} chars, see logs get ${reference} --trace]` + ? `${value.slice(0, LOG_INPUT_KEEP_CHARS)} …[truncated; inspect with logs get ${reference} --trace]` : value } return { ...entry, input } @@ -98,65 +97,89 @@ export function compactBlockLogInputs(logs: unknown, executionId: string | undef } /** - * Budgets for the block outputs echoed back in `logs`, a quarter of each cap the model-facing - * projection enforces on the whole result. Reaching either cap withholds everything, including - * the final output and error the run was for; those are never compacted and share the remaining - * three quarters with the rest of the envelope. The value budget usually binds first: row-shaped - * outputs reach the projection's traversal cap long before its byte cap. + * Budgets for block outputs echoed to the model, a quarter of each cap the model-facing projection + * enforces on the whole result. Reaching either cap withholds everything, including the final + * output and error the run was for, so those keep the remaining three quarters. The value budget + * usually binds first: row-shaped outputs reach the projection's traversal cap long before its + * byte cap. */ -const LOG_OUTPUT_VALUE_BUDGET = Math.floor(MAX_CONTENT_NODES / 4) -const LOG_OUTPUT_BYTE_BUDGET = Math.floor(MAX_MODEL_CONTENT_BYTES / 4) +const BLOCK_OUTPUT_VALUE_BUDGET = Math.floor(MAX_CONTENT_NODES / 4) +const BLOCK_OUTPUT_BYTE_BUDGET = Math.floor(MAX_MODEL_CONTENT_BYTES / 4) -interface MeasuredLogOutput { - index: number - entry: Record +interface BlockOutputSize { values: number bytes: number - /** What the pointer reports: exact, past the projection's cap, or unencodable. */ + /** What the pointer reports. Never a byte count: bytes are measured before secret projection. */ label: string } +/** + * Sizes one block output for the budgets. An output past a projection limit (values, bytes, or + * depth) or one JSON cannot encode would be refused whatever else the result holds, so it is + * sized past every budget and is always the first to be replaced. + */ +function sizeBlockOutput(output: unknown): BlockOutputSize { + const measure = measureModelContent(output) + if (!measure || measure.exceeded) { + return { + values: MAX_CONTENT_NODES + 1, + bytes: MAX_MODEL_CONTENT_BYTES + 1, + label: 'output omitted: too large to return', + } + } + return { + values: measure.values, + bytes: measure.bytes, + label: `output omitted: ${measure.values} values`, + } +} + +function blockOutputPointer(label: string, executionId: string | undefined): string { + return `…[${label}; inspect with logs get ${executionId ?? ''} --trace]` +} + /** * Replaces the bulkiest block outputs in `logs` with a pointer once they exceed the budgets - * above, largest first, so the rest of the run still reaches the model. - * - * The pointer names only the value count: bytes are measured before secret projection, so they - * would disclose a secret's length. It says "inspect" rather than promising the full value, since - * the stored trace can itself be summarized. + * above, largest first, so the rest of the run still reaches the model. The pointer says + * "inspect" rather than promising the full value, since the stored trace can itself be summarized. */ function compactBlockLogOutputs(logs: unknown, executionId: string | undefined): unknown { if (!Array.isArray(logs)) return logs - const reference = executionId ?? '' - const measured: MeasuredLogOutput[] = [] + const measured: Array }> = [] let values = 0 let bytes = 0 for (const [index, entry] of logs.entries()) { if (!isPlainRecord(entry) || entry.output === undefined) continue - const measure = measureModelContent(entry.output) - const exact = measure?.bytes !== undefined - const size = { - values: exact ? measure.values : MAX_CONTENT_NODES + 1, - bytes: measure?.bytes ?? MAX_MODEL_CONTENT_BYTES + 1, - } - const label = !measure - ? 'output omitted' - : `output omitted: ${exact ? measure.values : `over ${MAX_CONTENT_NODES}`} values` - measured.push({ index, entry, ...size, label }) + const size = sizeBlockOutput(entry.output) + measured.push({ index, entry, ...size }) values += size.values bytes += size.bytes } - if (values <= LOG_OUTPUT_VALUE_BUDGET && bytes <= LOG_OUTPUT_BYTE_BUDGET) return logs + if (values <= BLOCK_OUTPUT_VALUE_BUDGET && bytes <= BLOCK_OUTPUT_BYTE_BUDGET) return logs measured.sort((left, right) => right.values - left.values || right.bytes - left.bytes) const compacted = [...logs] for (const item of measured) { - if (values <= LOG_OUTPUT_VALUE_BUDGET && bytes <= LOG_OUTPUT_BYTE_BUDGET) break - compacted[item.index] = { - ...item.entry, - output: `…[${item.label}; inspect with logs get ${reference} --trace]`, - } + if (values <= BLOCK_OUTPUT_VALUE_BUDGET && bytes <= BLOCK_OUTPUT_BYTE_BUDGET) break + compacted[item.index] = { ...item.entry, output: blockOutputPointer(item.label, executionId) } values -= item.values bytes -= item.bytes } return compacted } + +/** + * Bounds a block output lifted into a run's `output` (run_block and run_workflow_until_block stop + * before any Response block, so the stopping block's output stands in for the run's). It is a copy + * of a log output, so it gets the same budget and pointer rather than the headroom a real final + * output keeps. + */ +export function compactLiftedBlockOutput( + output: unknown, + executionId: string | undefined +): unknown { + const size = sizeBlockOutput(output) + return size.values <= BLOCK_OUTPUT_VALUE_BUDGET && size.bytes <= BLOCK_OUTPUT_BYTE_BUDGET + ? output + : blockOutputPointer(size.label, executionId) +} From 4b143ceea264071b52f92f921bc364cd7fc44ca4 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 30 Sep 2026 03:03:49 -0700 Subject: [PATCH 09/14] fix(mothership): keep no raw prefix in a truncated block input MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A truncated block input kept its first 200 raw characters, written before secret projection. A secret straddling that cut left a fragment that whole-literal redaction cannot match, so part of the secret reached the model. The marker now keeps nothing of the raw input: `…[input omitted; inspect with logs get --trace]`. Inputs over the limit are echoed upstream data the caller already has or can fetch, so the preview carried little. --- .../tools/handlers/workflow/mutations.test.ts | 4 +-- .../run-workflow-result-budget.test.ts | 29 +++++++++++++++++++ .../lib/mothership/tools/workflow-output.ts | 8 ++--- 3 files changed, 35 insertions(+), 6 deletions(-) diff --git a/apps/sim/lib/mothership/tools/handlers/workflow/mutations.test.ts b/apps/sim/lib/mothership/tools/handlers/workflow/mutations.test.ts index a682df2259d..76a66aaccd9 100644 --- a/apps/sim/lib/mothership/tools/handlers/workflow/mutations.test.ts +++ b/apps/sim/lib/mothership/tools/handlers/workflow/mutations.test.ts @@ -181,10 +181,10 @@ describe('workflow mutation Copilot adapters', () => { } expect(code.length).toBeGreaterThan(240) expect(output.logs[0].input.code).toBe( - `${code.slice(0, 200)} …[truncated; inspect with logs get execution-1 --trace]` + '…[input omitted; inspect with logs get execution-1 --trace]' ) expect(output.logs[0].input.note).toBe( - `${'n'.repeat(200)} …[truncated; inspect with logs get execution-1 --trace]` + '…[input omitted; inspect with logs get execution-1 --trace]' ) expect(output.logs[0].input.language).toBe('javascript') expect(output.logs[0].output.result).toBe(code) diff --git a/apps/sim/lib/mothership/tools/handlers/workflow/run-workflow-result-budget.test.ts b/apps/sim/lib/mothership/tools/handlers/workflow/run-workflow-result-budget.test.ts index 795fbb1aa7f..f778707a0b4 100644 --- a/apps/sim/lib/mothership/tools/handlers/workflow/run-workflow-result-budget.test.ts +++ b/apps/sim/lib/mothership/tools/handlers/workflow/run-workflow-result-budget.test.ts @@ -293,4 +293,33 @@ describe('run_workflow model-facing result budget', () => { expect(logs[0]?.output).toEqual({ ok: true }) expect(logs[1]?.output).toEqual(expect.stringContaining(`logs get ${EXECUTION_ID} --trace`)) }) + + /** A cut through a secret leaves a fragment no whole-literal redaction can match. */ + it('never exposes part of a secret that straddles an input truncation point', async () => { + const straddling = `${'a'.repeat(190)}${SECRET}${'b'.repeat(3_000)}` + mocks.executeWorkflowUseCase.mockResolvedValue({ + success: true, + output: { done: true }, + logs: [ + { + blockId: 'fn', + blockName: 'Function', + success: true, + input: { code: straddling, note: straddling }, + output: { ok: true }, + }, + ], + metadata: { executionId: EXECUTION_ID }, + }) + + const settled = await executeRunWorkflow({ workflowId: 'wf-1' }, context) + const projection = inspectToolResultForCopilot(settled, secretRegistry(), 'run_workflow') + + expect(projection.safe).toBe(true) + const serialized = JSON.stringify(projection.result) + expect(serialized).toContain(`logs get ${EXECUTION_ID} --trace`) + for (let length = 4; length <= SECRET.length; length += 1) { + expect(serialized).not.toContain(SECRET.slice(0, length)) + } + }) }) diff --git a/apps/sim/lib/mothership/tools/workflow-output.ts b/apps/sim/lib/mothership/tools/workflow-output.ts index 06537b553fc..2122c9501e1 100644 --- a/apps/sim/lib/mothership/tools/workflow-output.ts +++ b/apps/sim/lib/mothership/tools/workflow-output.ts @@ -71,13 +71,13 @@ function selectFromLogs(selectors: string[], logs: unknown[]): Record limit - ? `${value.slice(0, LOG_INPUT_KEEP_CHARS)} …[truncated; inspect with logs get ${reference} --trace]` + ? `…[input omitted; inspect with logs get ${reference} --trace]` : value } return { ...entry, input } From 0cb15590f1bf3f13bb5f00dd8c808c78c7763237 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 30 Sep 2026 10:19:28 -0700 Subject: [PATCH 10/14] fix(mothership): bound run outputs only when the projection walks them against a secret Without an active secret the model-facing projection passes JSON through under its byte cap alone, so the block-output budget turned a large lifted run_block output into a pointer for no reason. Output compaction now applies only when the call's registry makes the projection walk the result, and a lifted output keeps the final output's share beside the bounded logs. --- .../lib/mothership/request/tools/client.ts | 2 +- .../request/tools/resolved-secret-result.ts | 18 ++++ .../tools/handlers/workflow/mutations.ts | 31 ++++-- .../run-workflow-result-budget.test.ts | 101 +++++++++++++++--- .../lib/mothership/tools/workflow-output.ts | 29 +++-- 5 files changed, 149 insertions(+), 32 deletions(-) diff --git a/apps/sim/lib/mothership/request/tools/client.ts b/apps/sim/lib/mothership/request/tools/client.ts index fc3ebb53081..b2244318256 100644 --- a/apps/sim/lib/mothership/request/tools/client.ts +++ b/apps/sim/lib/mothership/request/tools/client.ts @@ -377,7 +377,7 @@ export async function waitForWorkflowToolCompletion({ ? { output: trustedExecution.finalOutput } : {}), // Built from raw logs before projection, matching the server handler's presentation. - ...presentWorkflowLogsForModel(trustedExecution.blockLogs, executionId, select), + ...presentWorkflowLogsForModel(trustedExecution.blockLogs, executionId, toolRegistry, select), ...(trustedExecution.error !== undefined ? { error: trustedExecution.error } : {}), ...(status === MothershipStreamV1ToolOutcome.cancelled ? { reason: 'user_cancelled', cancelledByUser: true } diff --git a/apps/sim/lib/mothership/request/tools/resolved-secret-result.ts b/apps/sim/lib/mothership/request/tools/resolved-secret-result.ts index f28fd786357..22ebd083a8b 100644 --- a/apps/sim/lib/mothership/request/tools/resolved-secret-result.ts +++ b/apps/sim/lib/mothership/request/tools/resolved-secret-result.ts @@ -1,6 +1,7 @@ import type { ToolCallEffect, ToolExecutionResult } from '@/lib/mothership/tool-executor/types' import { TOOL_EFFECT_PHASE } from '@/lib/mothership/tool-executor/types' import { + getResolvedSecretModelMatcher, measureModelContent, projectResolvedSecretModelJsonContent, } from '@/executor/utils/resolved-secret-content-projection' @@ -237,6 +238,23 @@ export function inspectToolResultForCopilot( } } +/** + * Whether {@link inspectToolResultForCopilot} will walk a result under `registry` against active + * secrets, and so refuse it past the projection's value and depth caps as well as its byte cap. + * With no active secret the projection passes JSON through under the byte cap alone. A registry + * the projection cannot use withholds the result anyway, so it counts as walked. + */ +export function copilotProjectionWalksContent( + registry: ResolvedSecretTraceRegistry | undefined +): boolean { + try { + const snapshot = getResolvedSecretModelMatcher(registry?.forkForPropagatedEntries()) + return !snapshot.complete || snapshot.matcher !== undefined + } catch { + return true + } +} + /** * Projects terminal tool content before it can cross back into Copilot. * Runtime output remains unchanged for raw post-processing and context updates. diff --git a/apps/sim/lib/mothership/tools/handlers/workflow/mutations.ts b/apps/sim/lib/mothership/tools/handlers/workflow/mutations.ts index e8371304d67..dcf5373c089 100644 --- a/apps/sim/lib/mothership/tools/handlers/workflow/mutations.ts +++ b/apps/sim/lib/mothership/tools/handlers/workflow/mutations.ts @@ -50,6 +50,7 @@ import { } from '@/lib/workflows/application/update-workflow-content' import { sanitizeForCopilot } from '@/lib/workflows/sanitization/json-sanitizer' import { hasExecutionResult, readAttemptedExecutionId } from '@/executor/utils/errors' +import type { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry' import type { WorkflowState } from '@/stores/workflows/workflow/types' const logger = createLogger('WorkflowMutations') @@ -138,6 +139,7 @@ function buildExecutionOutput( error?: string status?: ExecutionResultStatus }, + registry: ResolvedSecretTraceRegistry | undefined, phase: ToolEffectPhase, extra?: Record, select?: string[] @@ -154,9 +156,9 @@ function buildExecutionOutput( executionId, success: result.success, ...extra, - output: lifted ? compactLiftedBlockOutput(lifted.output, executionId) : output, + output: lifted ? compactLiftedBlockOutput(lifted.output, executionId, registry) : output, ...(lifted ? { outputFrom: lifted.outputFrom } : {}), - ...presentWorkflowLogsForModel(logs, executionId, select), + ...presentWorkflowLogsForModel(logs, executionId, registry, select), }, error: result.success ? undefined @@ -189,7 +191,10 @@ function failedBlockError(logs: unknown): string | undefined { return undefined } -function buildExecutionError(error: unknown): ToolCallResult { +function buildExecutionError( + error: unknown, + registry: ResolvedSecretTraceRegistry | undefined +): ToolCallResult { if (hasExecutionResult(error)) { return buildExecutionOutput( { @@ -197,6 +202,7 @@ function buildExecutionError(error: unknown): ToolCallResult { success: false, error: error.executionResult.error || 'Workflow execution failed', }, + registry, settledPhase(error.executionResult.status) ) } @@ -344,9 +350,15 @@ export async function executeRunWorkflow( lifecycle: copilotRunLifecycle(context), }) - return buildExecutionOutput(result, settledPhase(result.status), undefined, params.select) + return buildExecutionOutput( + result, + context.resolvedSecretTraceRegistry, + settledPhase(result.status), + undefined, + params.select + ) } catch (error) { - return buildExecutionError(error) + return buildExecutionError(error, context.resolvedSecretTraceRegistry) } } @@ -507,12 +519,13 @@ export async function executeRunWorkflowUntilBlock( return buildExecutionOutput( result, + context.resolvedSecretTraceRegistry, settledPhase(result.status), { stoppedAfterBlockId: params.stopAfterBlockId }, params.select ) } catch (error) { - return buildExecutionError(error) + return buildExecutionError(error, context.resolvedSecretTraceRegistry) } } @@ -588,12 +601,13 @@ export async function executeRunFromBlock( return buildExecutionOutput( result, + context.resolvedSecretTraceRegistry, settledPhase(result.status), { startBlockId: params.startBlockId }, params.select ) } catch (error) { - return buildExecutionError(error) + return buildExecutionError(error, context.resolvedSecretTraceRegistry) } } @@ -680,11 +694,12 @@ export async function executeRunBlock( return buildExecutionOutput( result, + context.resolvedSecretTraceRegistry, settledPhase(result.status), { blockId: params.blockId }, params.select ) } catch (error) { - return buildExecutionError(error) + return buildExecutionError(error, context.resolvedSecretTraceRegistry) } } diff --git a/apps/sim/lib/mothership/tools/handlers/workflow/run-workflow-result-budget.test.ts b/apps/sim/lib/mothership/tools/handlers/workflow/run-workflow-result-budget.test.ts index f778707a0b4..57d21e683d8 100644 --- a/apps/sim/lib/mothership/tools/handlers/workflow/run-workflow-result-budget.test.ts +++ b/apps/sim/lib/mothership/tools/handlers/workflow/run-workflow-result-budget.test.ts @@ -38,11 +38,16 @@ import { const EXECUTION_ID = '0f4d5a4c-6a1e-4c2f-9b7d-2c8f1a3e5d90' const SECRET = 'fake-secret-for-test-only' -const context = { - userId: 'user-1', - workspaceId: 'workspace-1', - toolCallId: 'tool-call-1', -} as ExecutionContext + +/** The handler and the projection share the call's registry, as the tool executor wires them. */ +function callContext(registry: ResolvedSecretTraceRegistry) { + return { + userId: 'user-1', + workspaceId: 'workspace-1', + toolCallId: 'tool-call-1', + resolvedSecretTraceRegistry: registry, + } as ExecutionContext +} /** Rows and approximate encoded bytes per block of a synthetic trace-shaped run. */ const TABLE_QUERIES: ReadonlyArray = [ @@ -85,17 +90,30 @@ function traceShapedLogs() { }) } -function secretRegistry() { +/** A configured secret; `active` records that the run resolved it into its result. */ +function secretRegistry({ active = true } = {}) { const registry = new ResolvedSecretTraceRegistry([ { name: 'API_KEY', plaintext: SECRET, encryptedValue: 'ciphertext' }, ]) - registry.recordResolved('API_KEY', SECRET, { propagated: true }) + if (active) registry.recordResolved('API_KEY', SECRET, { propagated: true }) return registry } +/** Row-shaped output of about 27.5k values: past the log budget, under the projection's cap. */ +function wideRows() { + return Array.from({ length: 2_500 }, (_, index) => + Object.fromEntries(Array.from({ length: 10 }, (_, column) => [`c${column}`, `r${index}`])) + ) +} + describe('run_workflow model-facing result budget', () => { + let registry: ResolvedSecretTraceRegistry + let context: ExecutionContext + beforeEach(() => { mocks.executeWorkflowUseCase.mockReset() + registry = secretRegistry() + context = callContext(registry) }) it('projects a trace-shaped result with an active secret instead of withholding it', async () => { @@ -110,7 +128,7 @@ describe('run_workflow model-facing result budget', () => { }) const settled = await executeRunWorkflow({ workflowId: 'wf-1' }, context) - const projection = inspectToolResultForCopilot(settled, secretRegistry(), 'run_workflow') + const projection = inspectToolResultForCopilot(settled, registry, 'run_workflow') expect(projection.safe).toBe(true) const output = projection.result.output as Record @@ -140,7 +158,7 @@ describe('run_workflow model-facing result budget', () => { }) const settled = await executeRunWorkflow({ workflowId: 'wf-1' }, context) - const projection = inspectToolResultForCopilot(settled, secretRegistry(), 'run_workflow') + const projection = inspectToolResultForCopilot(settled, registry, 'run_workflow') expect(projection.safe).toBe(true) expect((projection.result.output as Record).output).toEqual(finalOutput) @@ -164,7 +182,7 @@ describe('run_workflow model-facing result budget', () => { const settled = await executeRunWorkflow({ workflowId: 'wf-1' }, context) expect(Buffer.byteLength(JSON.stringify(settled.output))).toBeLessThan(4 * 1024 * 1024) - const projection = inspectToolResultForCopilot(settled, secretRegistry(), 'run_workflow') + const projection = inspectToolResultForCopilot(settled, registry, 'run_workflow') expect(projection.safe).toBe(true) expect((projection.result.output as Record).output).toEqual(finalOutput) @@ -183,7 +201,7 @@ describe('run_workflow model-facing result budget', () => { { workflowId: 'wf-1', select: ['Query 4.rows'] }, context ) - const projection = inspectToolResultForCopilot(settled, secretRegistry(), 'run_workflow') + const projection = inspectToolResultForCopilot(settled, registry, 'run_workflow') expect(projection.safe).toBe(true) const output = projection.result.output as Record @@ -235,7 +253,7 @@ describe('run_workflow model-facing result budget', () => { { workflowId: 'wf-1', stopAfterBlockId: 'query' }, context ) - const projection = inspectToolResultForCopilot(settled, secretRegistry(), 'run_workflow') + const projection = inspectToolResultForCopilot(settled, registry, 'run_workflow') expect(projection.safe).toBe(true) const output = projection.result.output as Record @@ -286,7 +304,7 @@ describe('run_workflow model-facing result budget', () => { }) const settled = await executeRunWorkflow({ workflowId: 'wf-1' }, context) - const projection = inspectToolResultForCopilot(settled, secretRegistry(), 'run_workflow') + const projection = inspectToolResultForCopilot(settled, registry, 'run_workflow') expect(projection.safe).toBe(true) const logs = (projection.result.output as { logs: Array> }).logs @@ -313,7 +331,7 @@ describe('run_workflow model-facing result budget', () => { }) const settled = await executeRunWorkflow({ workflowId: 'wf-1' }, context) - const projection = inspectToolResultForCopilot(settled, secretRegistry(), 'run_workflow') + const projection = inspectToolResultForCopilot(settled, registry, 'run_workflow') expect(projection.safe).toBe(true) const serialized = JSON.stringify(projection.result) @@ -322,4 +340,59 @@ describe('run_workflow model-facing result budget', () => { expect(serialized).not.toContain(SECRET.slice(0, length)) } }) + + /** + * Without an active secret the projection passes JSON through under its byte cap alone, so + * nothing is bounded: a lifted output is the whole point of run_block and reaches the worker in + * full, which spills an oversized one to storage for the model to read. + */ + it('returns a large lifted output and its logs in full when no secret is active', async () => { + const rows = wideRows() + mocks.executeWorkflowUseCase.mockResolvedValue({ + success: true, + output: {}, + logs: [ + { blockId: 'start', blockName: 'Start', success: true, output: { ok: true } }, + { blockId: 'query', blockName: 'Query', success: true, output: { rows } }, + ], + metadata: { executionId: EXECUTION_ID }, + }) + const inactive = secretRegistry({ active: false }) + + const settled = await executeRunWorkflowUntilBlock( + { workflowId: 'wf-1', stopAfterBlockId: 'query' }, + callContext(inactive) + ) + const projection = inspectToolResultForCopilot(settled, inactive, 'run_workflow') + + expect(projection.safe).toBe(true) + const output = projection.result.output as Record + expect(output.output).toEqual({ rows }) + expect((output.logs as Array>)[1]?.output).toEqual({ rows }) + }) + + /** A lifted output is the run's final output, so it keeps the final output's larger share. */ + it('returns a lifted output within the final share in full while a secret is active', async () => { + const rows = wideRows() + mocks.executeWorkflowUseCase.mockResolvedValue({ + success: true, + output: {}, + logs: [{ blockId: 'query', blockName: 'Query', success: true, output: { rows } }], + metadata: { executionId: EXECUTION_ID }, + }) + + const settled = await executeRunWorkflowUntilBlock( + { workflowId: 'wf-1', stopAfterBlockId: 'query' }, + context + ) + const projection = inspectToolResultForCopilot(settled, registry, 'run_workflow') + + expect(projection.safe).toBe(true) + const output = projection.result.output as Record + expect(output.output).toEqual({ rows }) + // Its log copy is still bounded, so the two together stay under the projection's caps. + expect((output.logs as Array>)[0]?.output).toEqual( + expect.stringContaining(`logs get ${EXECUTION_ID} --trace`) + ) + }) }) diff --git a/apps/sim/lib/mothership/tools/workflow-output.ts b/apps/sim/lib/mothership/tools/workflow-output.ts index 2122c9501e1..328812feac0 100644 --- a/apps/sim/lib/mothership/tools/workflow-output.ts +++ b/apps/sim/lib/mothership/tools/workflow-output.ts @@ -1,9 +1,11 @@ import { isPlainRecord, isRecordLike } from '@sim/utils/object' +import { copilotProjectionWalksContent } from '@/lib/mothership/request/tools/resolved-secret-result' import { MAX_CONTENT_NODES, MAX_MODEL_CONTENT_BYTES, measureModelContent, } from '@/executor/utils/resolved-secret-content-projection' +import type { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry' /** Shared presentation for server execution and trusted client execution restoration. */ function presentWorkflowLogs(logs: unknown, select?: string[]): Record { @@ -14,18 +16,23 @@ function presentWorkflowLogs(logs: unknown, select?: string[]): Record { if (select?.length) return presentWorkflowLogs(logs, select) + const compacted = compactBlockLogInputs(logs, executionId) return presentWorkflowLogs( - compactBlockLogOutputs(compactBlockLogInputs(logs, executionId), executionId) + copilotProjectionWalksContent(registry) + ? compactBlockLogOutputs(compacted, executionId) + : compacted ) } @@ -105,6 +112,8 @@ function compactBlockLogInputs(logs: unknown, executionId: string | undefined): */ const BLOCK_OUTPUT_VALUE_BUDGET = Math.floor(MAX_CONTENT_NODES / 4) const BLOCK_OUTPUT_BYTE_BUDGET = Math.floor(MAX_MODEL_CONTENT_BYTES / 4) +const FINAL_OUTPUT_VALUE_BUDGET = MAX_CONTENT_NODES - BLOCK_OUTPUT_VALUE_BUDGET +const FINAL_OUTPUT_BYTE_BUDGET = MAX_MODEL_CONTENT_BYTES - BLOCK_OUTPUT_BYTE_BUDGET interface BlockOutputSize { values: number @@ -170,16 +179,18 @@ function compactBlockLogOutputs(logs: unknown, executionId: string | undefined): /** * Bounds a block output lifted into a run's `output` (run_block and run_workflow_until_block stop - * before any Response block, so the stopping block's output stands in for the run's). It is a copy - * of a log output, so it gets the same budget and pointer rather than the headroom a real final - * output keeps. + * before any Response block, so the stopping block's output stands in for the run's). It is the + * run's final output, so it keeps the final output's share beside the bounded logs, and like them it + * is bounded only when `registry` makes the projection walk the result. */ export function compactLiftedBlockOutput( output: unknown, - executionId: string | undefined + executionId: string | undefined, + registry: ResolvedSecretTraceRegistry | undefined ): unknown { + if (!copilotProjectionWalksContent(registry)) return output const size = sizeBlockOutput(output) - return size.values <= BLOCK_OUTPUT_VALUE_BUDGET && size.bytes <= BLOCK_OUTPUT_BYTE_BUDGET + return size.values <= FINAL_OUTPUT_VALUE_BUDGET && size.bytes <= FINAL_OUTPUT_BYTE_BUDGET ? output : blockOutputPointer(size.label, executionId) } From 118a58b52485235774fe6714fc0267367da55f5b Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 30 Sep 2026 10:41:54 -0700 Subject: [PATCH 11/14] fix(mothership): measure a run's whole model-facing result and bound its final output A fixed share for the lifted output left no room for the log entries, envelope and error the projection also counts, and a Response block's final output was never bounded, so a run near the caps was still withheld whole. The assembled result is now measured as the projection will walk it, and a final output that would push it past a cap is replaced with a pointer, on the server and browser-run paths alike. An unencodable result is still refused as before. --- .../mothership/request/tools/client.test.ts | 28 ++++++++++ .../lib/mothership/request/tools/client.ts | 44 +++++++++------ .../tools/handlers/workflow/mutations.ts | 28 ++++++---- .../run-workflow-result-budget.test.ts | 55 +++++++++++++++++++ .../lib/mothership/tools/workflow-output.ts | 33 ++++++----- 5 files changed, 145 insertions(+), 43 deletions(-) diff --git a/apps/sim/lib/mothership/request/tools/client.test.ts b/apps/sim/lib/mothership/request/tools/client.test.ts index 444aa64713d..600e378d84c 100644 --- a/apps/sim/lib/mothership/request/tools/client.test.ts +++ b/apps/sim/lib/mothership/request/tools/client.test.ts @@ -247,6 +247,34 @@ describe('workflow client tool completion', () => { expect(JSON.stringify(completion)).not.toContain('parent-secret-value') }) + /** Parity with the server path: a final output that would push the result past a cap is replaced. */ + it('replaces an oversized final output so a browser run still projects', async () => { + const rows = Array.from({ length: 20_000 }, (_, index) => ({ + id: `row_${index}`, + data: { a: 'x', b: 'y', c: 'z', d: 'w' }, + })) + getTrustedWorkflowToolExecution.mockResolvedValue({ + ...trustedExecution('execution-1'), + finalOutput: { rows }, + blockLogs: [{ blockId: 'small', blockName: 'Small', output: { count: 1 } }], + }) + waitForToolConfirmation.mockResolvedValue({ + status: 'success', + data: { workflowId: 'workflow-1', executionId: 'execution-1' }, + }) + + const completion = await waitForWorkflowToolCompletion({ + toolCallId: 'tool-1', + workflowId: 'workflow-1', + timeoutMs: 1_000, + registry: createParentRegistry(), + }) + + const data = completion?.data as Record + expect(data.output).toEqual(expect.stringContaining('logs get execution-1 --trace')) + expect((data.logs as Array>)[0]?.output).toEqual({ count: 1 }) + }) + /** Parity with the server path: a `select` is resolved from raw logs before projection. */ it('projects selected values from a large browser run instead of withholding it', async () => { const rows = (count: number) => diff --git a/apps/sim/lib/mothership/request/tools/client.ts b/apps/sim/lib/mothership/request/tools/client.ts index b2244318256..4d8c293392a 100644 --- a/apps/sim/lib/mothership/request/tools/client.ts +++ b/apps/sim/lib/mothership/request/tools/client.ts @@ -15,7 +15,10 @@ import { unsealClientToolContext, } from '@/lib/mothership/request/tools/client-completion-seal.server' import { inspectToolResultForCopilot } from '@/lib/mothership/request/tools/resolved-secret-result' -import { presentWorkflowLogsForModel } from '@/lib/mothership/tools/workflow-output' +import { + boundRunResultForModel, + presentWorkflowLogsForModel, +} from '@/lib/mothership/tools/workflow-output' import { createStructuralWorkflowToolCompletionData, getWorkflowToolCompletionExecutionId, @@ -369,27 +372,34 @@ export async function waitForWorkflowToolCompletion({ const executionId = trustedExecution.executionId const status = getWorkflowToolConfirmationStatus(trustedExecution.status) const genericMessage = getWorkflowToolCompletionMessage(status) - const rawData: Record = { - success: status === MothershipStreamV1ToolOutcome.success, - workflowId, + const error = + status !== MothershipStreamV1ToolOutcome.success + ? (trustedExecution.error ?? genericMessage) + : undefined + const rawData = boundRunResultForModel( + { + success: status === MothershipStreamV1ToolOutcome.success, + workflowId, + executionId, + ...(Object.hasOwn(trustedExecution, 'finalOutput') + ? { output: trustedExecution.finalOutput } + : {}), + // Built from raw logs before projection, matching the server handler's presentation. + ...presentWorkflowLogsForModel(trustedExecution.blockLogs, executionId, toolRegistry, select), + ...(trustedExecution.error !== undefined ? { error: trustedExecution.error } : {}), + ...(status === MothershipStreamV1ToolOutcome.cancelled + ? { reason: 'user_cancelled', cancelledByUser: true } + : {}), + }, + error, executionId, - ...(Object.hasOwn(trustedExecution, 'finalOutput') - ? { output: trustedExecution.finalOutput } - : {}), - // Built from raw logs before projection, matching the server handler's presentation. - ...presentWorkflowLogsForModel(trustedExecution.blockLogs, executionId, toolRegistry, select), - ...(trustedExecution.error !== undefined ? { error: trustedExecution.error } : {}), - ...(status === MothershipStreamV1ToolOutcome.cancelled - ? { reason: 'user_cancelled', cancelledByUser: true } - : {}), - } + toolRegistry + ) const projection = inspectToolResultForCopilot( { success: status === MothershipStreamV1ToolOutcome.success, output: rawData, - ...(status !== MothershipStreamV1ToolOutcome.success - ? { error: trustedExecution.error ?? genericMessage } - : {}), + ...(error !== undefined ? { error } : {}), }, toolRegistry ) diff --git a/apps/sim/lib/mothership/tools/handlers/workflow/mutations.ts b/apps/sim/lib/mothership/tools/handlers/workflow/mutations.ts index dcf5373c089..0367575252a 100644 --- a/apps/sim/lib/mothership/tools/handlers/workflow/mutations.ts +++ b/apps/sim/lib/mothership/tools/handlers/workflow/mutations.ts @@ -30,7 +30,7 @@ import type { } from '@/lib/mothership/tools/handlers/param-types' import { requireCopilotWorkspace } from '@/lib/mothership/tools/server/workspace-scope' import { - compactLiftedBlockOutput, + boundRunResultForModel, presentWorkflowLogsForModel, } from '@/lib/mothership/tools/workflow-output' import { decodeVfsPathSegments, encodeVfsPathSegments } from '@/lib/mothership/vfs/path-utils' @@ -148,21 +148,27 @@ function buildExecutionOutput( const output = stripBinaryFields(result.output) const logs = stripBinaryFields(result.logs) const lifted = isEmptyOutput(output) ? lastBlockOutput(logs) : undefined + const error = result.success + ? undefined + : result.error || failedBlockError(logs) || 'Workflow execution failed' // A caller that names the outputs it wants gets those and nothing else: a seven-block // run otherwise costs ~14K chars of logs to learn one headline. return { success: result.success, - output: { + output: boundRunResultForModel( + { + executionId, + success: result.success, + ...extra, + output: lifted ? lifted.output : output, + ...(lifted ? { outputFrom: lifted.outputFrom } : {}), + ...presentWorkflowLogsForModel(logs, executionId, registry, select), + }, + error, executionId, - success: result.success, - ...extra, - output: lifted ? compactLiftedBlockOutput(lifted.output, executionId, registry) : output, - ...(lifted ? { outputFrom: lifted.outputFrom } : {}), - ...presentWorkflowLogsForModel(logs, executionId, registry, select), - }, - error: result.success - ? undefined - : result.error || failedBlockError(logs) || 'Workflow execution failed', + registry + ), + error, effect: executionEffect(phase, executionId), } } diff --git a/apps/sim/lib/mothership/tools/handlers/workflow/run-workflow-result-budget.test.ts b/apps/sim/lib/mothership/tools/handlers/workflow/run-workflow-result-budget.test.ts index 57d21e683d8..f1bdcd9a063 100644 --- a/apps/sim/lib/mothership/tools/handlers/workflow/run-workflow-result-budget.test.ts +++ b/apps/sim/lib/mothership/tools/handlers/workflow/run-workflow-result-budget.test.ts @@ -395,4 +395,59 @@ describe('run_workflow model-facing result budget', () => { expect.stringContaining(`logs get ${EXECUTION_ID} --trace`) ) }) + + /** A Response block's output is the final output too, so it is replaced rather than voiding the run. */ + it('replaces a final output past the projection cap instead of withholding the run', async () => { + const narrow = Array.from({ length: 25_000 }, (_, index) => ({ + id: `r${index}`, + data: { a: 'x' }, + })) + mocks.executeWorkflowUseCase.mockResolvedValue({ + success: true, + output: { rows: narrow }, + logs: [{ blockId: 'small', blockName: 'Small', success: true, output: { ok: true } }], + metadata: { executionId: EXECUTION_ID }, + }) + + const settled = await executeRunWorkflow({ workflowId: 'wf-1' }, context) + const projection = inspectToolResultForCopilot(settled, registry, 'run_workflow') + + expect(projection.safe).toBe(true) + const output = projection.result.output as Record + expect(output.output).toEqual(expect.stringContaining(`logs get ${EXECUTION_ID} --trace`)) + expect((output.logs as Array>)[0]?.output).toEqual({ ok: true }) + }) + + /** + * Each part can sit just inside its own share while the whole result, envelope included, passes + * the projection's value cap, so the result is measured whole. + */ + it('replaces the final output when the whole result passes the cap at the share limits', async () => { + // Five values per row, two for the wrapping object and array. + const rows = (values: number) => + Array.from({ length: Math.floor((values - 2) / 5) }, (_, index) => ({ + id: `r${index}`, + data: { a: 'x', b: 'y' }, + })) + mocks.executeWorkflowUseCase.mockResolvedValue({ + success: true, + output: {}, + logs: [ + { blockId: 'other', blockName: 'Other', success: true, output: { rows: rows(25_000) } }, + { blockId: 'query', blockName: 'Query', success: true, output: { rows: rows(75_000) } }, + ], + metadata: { executionId: EXECUTION_ID }, + }) + + const settled = await executeRunWorkflowUntilBlock( + { workflowId: 'wf-1', stopAfterBlockId: 'query' }, + context + ) + const projection = inspectToolResultForCopilot(settled, registry, 'run_workflow') + + expect(projection.safe).toBe(true) + const output = projection.result.output as Record + expect(output.output).toEqual(expect.stringContaining(`logs get ${EXECUTION_ID} --trace`)) + expect(output.outputFrom).toEqual({ blockId: 'query', blockName: 'Query' }) + }) }) diff --git a/apps/sim/lib/mothership/tools/workflow-output.ts b/apps/sim/lib/mothership/tools/workflow-output.ts index 328812feac0..98f81870965 100644 --- a/apps/sim/lib/mothership/tools/workflow-output.ts +++ b/apps/sim/lib/mothership/tools/workflow-output.ts @@ -106,14 +106,12 @@ function compactBlockLogInputs(logs: unknown, executionId: string | undefined): /** * Budgets for block outputs echoed to the model, a quarter of each cap the model-facing projection * enforces on the whole result. Reaching either cap withholds everything, including the final - * output and error the run was for, so those keep the remaining three quarters. The value budget + * output and error the run was for, so those keep the rest (see {@link boundRunResultForModel}). The value budget * usually binds first: row-shaped outputs reach the projection's traversal cap long before its * byte cap. */ const BLOCK_OUTPUT_VALUE_BUDGET = Math.floor(MAX_CONTENT_NODES / 4) const BLOCK_OUTPUT_BYTE_BUDGET = Math.floor(MAX_MODEL_CONTENT_BYTES / 4) -const FINAL_OUTPUT_VALUE_BUDGET = MAX_CONTENT_NODES - BLOCK_OUTPUT_VALUE_BUDGET -const FINAL_OUTPUT_BYTE_BUDGET = MAX_MODEL_CONTENT_BYTES - BLOCK_OUTPUT_BYTE_BUDGET interface BlockOutputSize { values: number @@ -178,19 +176,24 @@ function compactBlockLogOutputs(logs: unknown, executionId: string | undefined): } /** - * Bounds a block output lifted into a run's `output` (run_block and run_workflow_until_block stop - * before any Response block, so the stopping block's output stands in for the run's). It is the - * run's final output, so it keeps the final output's share beside the bounded logs, and like them it - * is bounded only when `registry` makes the projection walk the result. + * Bounds a run's whole model-facing result before secret projection. When `registry` makes the + * projection walk the result, one past any of its caps is withheld entirely, so the final output + * (the only part not already bounded) is replaced with a pointer instead. The result is measured + * whole, envelope and error included, so the final output gets exactly the room the bounded logs + * leave. A block output that run_block or run_workflow_until_block lifted into `output` is the + * run's final output here too. */ -export function compactLiftedBlockOutput( - output: unknown, +export function boundRunResultForModel( + data: Record, + error: string | undefined, executionId: string | undefined, registry: ResolvedSecretTraceRegistry | undefined -): unknown { - if (!copilotProjectionWalksContent(registry)) return output - const size = sizeBlockOutput(output) - return size.values <= FINAL_OUTPUT_VALUE_BUDGET && size.bytes <= FINAL_OUTPUT_BYTE_BUDGET - ? output - : blockOutputPointer(size.label, executionId) +): Record { + if (!Object.hasOwn(data, 'output') || !copilotProjectionWalksContent(registry)) return data + const whole = measureModelContent( + error === undefined ? { output: data } : { output: data, error } + ) + // A result JSON cannot encode is refused whatever its size, so only one past a cap is bounded. + if (!whole?.exceeded) return data + return { ...data, output: blockOutputPointer(sizeBlockOutput(data.output).label, executionId) } } From e4ba8a3cae001a946e69ad380df9cc1d986290cd Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 30 Sep 2026 11:00:07 -0700 Subject: [PATCH 12/14] fix(mothership): leave an unencodable block output for the projection to refuse The output bound handles size only. A block output JSON cannot encode cannot be checked, so it is no longer sized past the budget and replaced with a pointer; the projection refuses it as it did before this PR. --- .../run-workflow-result-budget.test.ts | 30 +++++++++++++++++++ .../lib/mothership/tools/workflow-output.ts | 14 +++++---- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/apps/sim/lib/mothership/tools/handlers/workflow/run-workflow-result-budget.test.ts b/apps/sim/lib/mothership/tools/handlers/workflow/run-workflow-result-budget.test.ts index f1bdcd9a063..036c9c4ac5a 100644 --- a/apps/sim/lib/mothership/tools/handlers/workflow/run-workflow-result-budget.test.ts +++ b/apps/sim/lib/mothership/tools/handlers/workflow/run-workflow-result-budget.test.ts @@ -450,4 +450,34 @@ describe('run_workflow model-facing result budget', () => { expect(output.output).toEqual(expect.stringContaining(`logs get ${EXECUTION_ID} --trace`)) expect(output.outputFrom).toEqual({ blockId: 'query', blockName: 'Query' }) }) + + /** + * The bound only handles size. An output JSON cannot encode cannot be checked, so the run is still + * refused rather than the value being hidden behind a pointer. + */ + it('still refuses a run with an unencodable block output while bounding bulky ones', async () => { + mocks.executeWorkflowUseCase.mockResolvedValue({ + success: true, + output: {}, + logs: [ + { + blockId: 'big', + blockName: 'Big', + success: true, + output: { rows: tableRows(4_800, 3_200_000) }, + }, + { blockId: 'odd', blockName: 'Odd', success: true, output: { count: 1n } }, + ], + metadata: { executionId: EXECUTION_ID }, + }) + + const settled = await executeRunWorkflowUntilBlock( + { workflowId: 'wf-1', stopAfterBlockId: 'odd' }, + context + ) + const presented = settled.output as { output: unknown; logs: Array<{ output: unknown }> } + expect(presented.output).toEqual({ count: 1n }) + expect(presented.logs[1]?.output).toEqual({ count: 1n }) + expect(inspectToolResultForCopilot(settled, registry, 'run_workflow').safe).toBe(false) + }) }) diff --git a/apps/sim/lib/mothership/tools/workflow-output.ts b/apps/sim/lib/mothership/tools/workflow-output.ts index 98f81870965..b5356ad3d26 100644 --- a/apps/sim/lib/mothership/tools/workflow-output.ts +++ b/apps/sim/lib/mothership/tools/workflow-output.ts @@ -122,12 +122,14 @@ interface BlockOutputSize { /** * Sizes one block output for the budgets. An output past a projection limit (values, bytes, or - * depth) or one JSON cannot encode would be refused whatever else the result holds, so it is - * sized past every budget and is always the first to be replaced. + * depth) would be refused whatever else the result holds, so it is sized past every budget and is + * always the first to be replaced. One JSON cannot encode is not a size problem: it returns + * undefined and is left for the projection to refuse, as it always has been. */ -function sizeBlockOutput(output: unknown): BlockOutputSize { +function sizeBlockOutput(output: unknown): BlockOutputSize | undefined { const measure = measureModelContent(output) - if (!measure || measure.exceeded) { + if (!measure) return undefined + if (measure.exceeded) { return { values: MAX_CONTENT_NODES + 1, bytes: MAX_MODEL_CONTENT_BYTES + 1, @@ -158,6 +160,7 @@ function compactBlockLogOutputs(logs: unknown, executionId: string | undefined): for (const [index, entry] of logs.entries()) { if (!isPlainRecord(entry) || entry.output === undefined) continue const size = sizeBlockOutput(entry.output) + if (!size) continue measured.push({ index, entry, ...size }) values += size.values bytes += size.bytes @@ -195,5 +198,6 @@ export function boundRunResultForModel( ) // A result JSON cannot encode is refused whatever its size, so only one past a cap is bounded. if (!whole?.exceeded) return data - return { ...data, output: blockOutputPointer(sizeBlockOutput(data.output).label, executionId) } + const size = sizeBlockOutput(data.output) + return size ? { ...data, output: blockOutputPointer(size.label, executionId) } : data } From 2c0b369d1299ff354f9c524f7a7b6a6d8ecd187c Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 30 Sep 2026 11:19:12 -0700 Subject: [PATCH 13/14] fix(mothership): leave a result that fits the caps, and every result without a secret, as staging returns it Block-log outputs were bounded whenever a secret was active, so a 30k-value or 4.5 MB result that crossed in full before became pointers. Outputs are now bounded only when the whole result would pass a projection cap. Without an active secret the server handler keeps its preview input marker and the browser-run path leaves its logs untouched, in their original position, so a result with no secret is byte-identical to before; only a walked call gets the marker that keeps nothing. --- .../mothership/request/tools/client.test.ts | 38 ++++++++ .../lib/mothership/request/tools/client.ts | 7 +- .../tools/handlers/workflow/mutations.ts | 4 +- .../run-workflow-result-budget.test.ts | 74 ++++++++++++--- .../lib/mothership/tools/workflow-output.ts | 91 ++++++++++++------- 5 files changed, 165 insertions(+), 49 deletions(-) diff --git a/apps/sim/lib/mothership/request/tools/client.test.ts b/apps/sim/lib/mothership/request/tools/client.test.ts index 600e378d84c..aec3216eea5 100644 --- a/apps/sim/lib/mothership/request/tools/client.test.ts +++ b/apps/sim/lib/mothership/request/tools/client.test.ts @@ -247,6 +247,44 @@ describe('workflow client tool completion', () => { expect(JSON.stringify(completion)).not.toContain('parent-secret-value') }) + /** Without an active secret a browser run's logs cross untouched, as they always have. */ + it('leaves a browser run without an active secret untouched', async () => { + const blockLogs = [ + { + blockId: 'fn', + blockName: 'Function', + input: { code: 'x'.repeat(3_000) }, + output: { ok: 1 }, + }, + ] + getTrustedWorkflowToolExecution.mockResolvedValue({ + ...trustedExecution('execution-1'), + finalOutput: { value: 'plain' }, + blockLogs, + provenance: { version: 1 as const, complete: true, entries: [], scope: TRACE_SCOPE }, + }) + waitForToolConfirmation.mockResolvedValue({ + status: 'success', + data: { workflowId: 'workflow-1', executionId: 'execution-1' }, + }) + + const completion = await waitForWorkflowToolCompletion({ + toolCallId: 'tool-1', + workflowId: 'workflow-1', + timeoutMs: 1_000, + registry: new ResolvedSecretTraceRegistry([], TRACE_SCOPE), + }) + + expect(Object.keys(completion?.data as object)).toEqual([ + 'success', + 'workflowId', + 'executionId', + 'output', + 'logs', + ]) + expect((completion?.data as Record).logs).toEqual(blockLogs) + }) + /** Parity with the server path: a final output that would push the result past a cap is replaced. */ it('replaces an oversized final output so a browser run still projects', async () => { const rows = Array.from({ length: 20_000 }, (_, index) => ({ diff --git a/apps/sim/lib/mothership/request/tools/client.ts b/apps/sim/lib/mothership/request/tools/client.ts index 4d8c293392a..3ae54119d1e 100644 --- a/apps/sim/lib/mothership/request/tools/client.ts +++ b/apps/sim/lib/mothership/request/tools/client.ts @@ -1,6 +1,6 @@ import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' -import { isPlainRecord } from '@sim/utils/object' +import { filterUndefined, isPlainRecord } from '@sim/utils/object' import { ASYNC_TOOL_CONFIRMATION_STATUS, type AsyncTerminalCompletionSnapshot, @@ -405,8 +405,11 @@ export async function waitForWorkflowToolCompletion({ ) const projected = projection.result const projectedData = isPlainRecord(projected.output) ? projected.output : {} + // Log fields go last, where they have always been, ahead of the structural fields. + const { logs, selected, logsOmitted, ...projectedFields } = projectedData const data = { - ...projectedData, + ...projectedFields, + ...filterUndefined({ logs, selected, logsOmitted }), ...createStructuralWorkflowToolCompletionData(status, workflowId, executionId), } const message = diff --git a/apps/sim/lib/mothership/tools/handlers/workflow/mutations.ts b/apps/sim/lib/mothership/tools/handlers/workflow/mutations.ts index 0367575252a..bc8bcc88bb9 100644 --- a/apps/sim/lib/mothership/tools/handlers/workflow/mutations.ts +++ b/apps/sim/lib/mothership/tools/handlers/workflow/mutations.ts @@ -162,7 +162,9 @@ function buildExecutionOutput( ...extra, output: lifted ? lifted.output : output, ...(lifted ? { outputFrom: lifted.outputFrom } : {}), - ...presentWorkflowLogsForModel(logs, executionId, registry, select), + ...presentWorkflowLogsForModel(logs, executionId, registry, select, { + previewLongInputs: true, + }), }, error, executionId, diff --git a/apps/sim/lib/mothership/tools/handlers/workflow/run-workflow-result-budget.test.ts b/apps/sim/lib/mothership/tools/handlers/workflow/run-workflow-result-budget.test.ts index 036c9c4ac5a..4454194a78e 100644 --- a/apps/sim/lib/mothership/tools/handlers/workflow/run-workflow-result-budget.test.ts +++ b/apps/sim/lib/mothership/tools/handlers/workflow/run-workflow-result-budget.test.ts @@ -371,8 +371,12 @@ describe('run_workflow model-facing result budget', () => { expect((output.logs as Array>)[1]?.output).toEqual({ rows }) }) - /** A lifted output is the run's final output, so it keeps the final output's larger share. */ - it('returns a lifted output within the final share in full while a secret is active', async () => { + /** + * Nothing is bounded while the whole result fits the projection's caps, so a result that crossed + * in full before still does: a lifted output and its log copy of about 27.5k values each, or one + * 4.5 MB block output. + */ + it('returns a result that fits the caps in full while a secret is active', async () => { const rows = wideRows() mocks.executeWorkflowUseCase.mockResolvedValue({ success: true, @@ -380,20 +384,66 @@ describe('run_workflow model-facing result budget', () => { logs: [{ blockId: 'query', blockName: 'Query', success: true, output: { rows } }], metadata: { executionId: EXECUTION_ID }, }) + const lifted = inspectToolResultForCopilot( + await executeRunWorkflowUntilBlock( + { workflowId: 'wf-1', stopAfterBlockId: 'query' }, + context + ), + registry, + 'run_workflow' + ) + expect(lifted.safe).toBe(true) + const liftedOutput = lifted.result.output as Record + expect(liftedOutput.output).toEqual({ rows }) + expect((liftedOutput.logs as Array>)[0]?.output).toEqual({ rows }) - const settled = await executeRunWorkflowUntilBlock( - { workflowId: 'wf-1', stopAfterBlockId: 'query' }, - context + const text = { text: 'y'.repeat(4_500_000) } + mocks.executeWorkflowUseCase.mockResolvedValue({ + success: true, + output: { done: true }, + logs: [{ blockId: 'big', blockName: 'Big', success: true, output: text }], + metadata: { executionId: EXECUTION_ID }, + }) + const wide = inspectToolResultForCopilot( + await executeRunWorkflow({ workflowId: 'wf-1' }, context), + registry, + 'run_workflow' ) - const projection = inspectToolResultForCopilot(settled, registry, 'run_workflow') + expect(wide.safe).toBe(true) + expect((wide.result.output as { logs: Array<{ output: unknown }> }).logs[0]?.output).toEqual( + text + ) + }) - expect(projection.safe).toBe(true) - const output = projection.result.output as Record - expect(output.output).toEqual({ rows }) - // Its log copy is still bounded, so the two together stay under the projection's caps. - expect((output.logs as Array>)[0]?.output).toEqual( - expect.stringContaining(`logs get ${EXECUTION_ID} --trace`) + /** + * Without an active secret echoed inputs keep the server's preview marker and the browser path's + * untouched logs; only a walked call gets the marker that keeps nothing of the input. + */ + it('previews a long input on the server path when no secret is active', async () => { + const code = `${'a'.repeat(300)}${'b'.repeat(3_000)}` + mocks.executeWorkflowUseCase.mockResolvedValue({ + success: true, + output: { done: true }, + logs: [ + { + blockId: 'fn', + blockName: 'Function', + success: true, + input: { code }, + output: { ok: true }, + }, + ], + metadata: { executionId: EXECUTION_ID }, + }) + + const settled = await executeRunWorkflow( + { workflowId: 'wf-1' }, + callContext(secretRegistry({ active: false })) ) + + expect( + (settled.output as { logs: Array<{ input: { code: string } }> }).logs[0]?.input.code + ).toBe(`${'a'.repeat(200)} …[${code.length} chars, see logs get ${EXECUTION_ID} --trace]`) }) /** A Response block's output is the final output too, so it is replaced rather than voiding the run. */ diff --git a/apps/sim/lib/mothership/tools/workflow-output.ts b/apps/sim/lib/mothership/tools/workflow-output.ts index b5356ad3d26..136a4a9f50f 100644 --- a/apps/sim/lib/mothership/tools/workflow-output.ts +++ b/apps/sim/lib/mothership/tools/workflow-output.ts @@ -15,24 +15,26 @@ function presentWorkflowLogs(logs: unknown, select?: string[]): Record { if (select?.length) return presentWorkflowLogs(logs, select) - const compacted = compactBlockLogInputs(logs, executionId) + if (copilotProjectionWalksContent(registry)) { + return presentWorkflowLogs(compactBlockLogInputs(logs, executionId, omittedInputMarker)) + } return presentWorkflowLogs( - copilotProjectionWalksContent(registry) - ? compactBlockLogOutputs(compacted, executionId) - : compacted + previewLongInputs ? compactBlockLogInputs(logs, executionId, previewedInputMarker) : logs ) } @@ -78,15 +80,32 @@ function selectFromLogs(selectors: string[], logs: unknown[]): Record string + +/** + * The marker for a call walked against active secrets. It is written before secret projection, so + * it keeps nothing of the raw input: a kept prefix could cut through a secret and leave a fragment + * no whole-literal redaction matches, and a length would disclose the length of any secret in it. + */ +const omittedInputMarker: InputMarker = (_value, reference) => + `…[input omitted; inspect with logs get ${reference} --trace]` + +/** The server handler's marker when no secret is active: a short preview and the full length. */ +const previewedInputMarker: InputMarker = (value, reference) => + `${value.slice(0, LOG_INPUT_KEEP_CHARS)} …[${value.length} chars, see logs get ${reference} --trace]` /** * Compacts the block inputs echoed back in `logs`. A Function block's `input.code` embeds the * fully serialized upstream rows, so a seven-block run repeated the same rows several times - * across ~14k chars of tool result. The marker is written before secret projection, so it keeps - * nothing of the raw input: a kept prefix could cut through a secret and leave a fragment no - * whole-literal redaction matches, and a length would disclose the length of any secret in it. + * across ~14k chars of tool result. The full input stays one `logs get --trace` away. */ -function compactBlockLogInputs(logs: unknown, executionId: string | undefined): unknown { +function compactBlockLogInputs( + logs: unknown, + executionId: string | undefined, + marker: InputMarker +): unknown { if (!Array.isArray(logs)) return logs const reference = executionId ?? '' return logs.map((entry) => { @@ -95,20 +114,17 @@ function compactBlockLogInputs(logs: unknown, executionId: string | undefined): for (const [key, value] of Object.entries(entry.input)) { const limit = key === 'code' ? LOG_CODE_INPUT_MAX_CHARS : LOG_INPUT_STRING_MAX_CHARS input[key] = - typeof value === 'string' && value.length > limit - ? `…[input omitted; inspect with logs get ${reference} --trace]` - : value + typeof value === 'string' && value.length > limit ? marker(value, reference) : value } return { ...entry, input } }) } /** - * Budgets for block outputs echoed to the model, a quarter of each cap the model-facing projection - * enforces on the whole result. Reaching either cap withholds everything, including the final - * output and error the run was for, so those keep the rest (see {@link boundRunResultForModel}). The value budget - * usually binds first: row-shaped outputs reach the projection's traversal cap long before its - * byte cap. + * Budgets for block outputs echoed to the model once a whole result would pass a projection cap: + * a quarter of each cap, so the final output and error the run was for keep the rest. The value + * budget usually binds first: row-shaped outputs reach the projection's traversal cap long before + * its byte cap. */ const BLOCK_OUTPUT_VALUE_BUDGET = Math.floor(MAX_CONTENT_NODES / 4) const BLOCK_OUTPUT_BYTE_BUDGET = Math.floor(MAX_MODEL_CONTENT_BYTES / 4) @@ -180,11 +196,11 @@ function compactBlockLogOutputs(logs: unknown, executionId: string | undefined): /** * Bounds a run's whole model-facing result before secret projection. When `registry` makes the - * projection walk the result, one past any of its caps is withheld entirely, so the final output - * (the only part not already bounded) is replaced with a pointer instead. The result is measured - * whole, envelope and error included, so the final output gets exactly the room the bounded logs - * leave. A block output that run_block or run_workflow_until_block lifted into `output` is the - * run's final output here too. + * projection walk the result, one past any of its caps is withheld entirely. So a result that + * would pass a cap first has its bulkiest block-log outputs replaced with pointers, down to their + * budget, and if it still would, its final output too. The result is measured whole, envelope and + * error included, and a result that fits is returned untouched. A block output that run_block or + * run_workflow_until_block lifted into `output` is the run's final output here too. */ export function boundRunResultForModel( data: Record, @@ -192,12 +208,19 @@ export function boundRunResultForModel( executionId: string | undefined, registry: ResolvedSecretTraceRegistry | undefined ): Record { - if (!Object.hasOwn(data, 'output') || !copilotProjectionWalksContent(registry)) return data - const whole = measureModelContent( - error === undefined ? { output: data } : { output: data, error } - ) + if (!copilotProjectionWalksContent(registry)) return data // A result JSON cannot encode is refused whatever its size, so only one past a cap is bounded. - if (!whole?.exceeded) return data - const size = sizeBlockOutput(data.output) - return size ? { ...data, output: blockOutputPointer(size.label, executionId) } : data + const passesCap = (candidate: Record): boolean => + measureModelContent(error === undefined ? { output: candidate } : { output: candidate, error }) + ?.exceeded === true + if (!passesCap(data)) return data + + const logsBounded = Array.isArray(data.logs) + ? { ...data, logs: compactBlockLogOutputs(data.logs, executionId) } + : data + if (!passesCap(logsBounded) || !Object.hasOwn(logsBounded, 'output')) return logsBounded + const size = sizeBlockOutput(logsBounded.output) + return size + ? { ...logsBounded, output: blockOutputPointer(size.label, executionId) } + : logsBounded } From e25849b6bd5db2f1b7940b425519cb6c3cdfa1cf Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 30 Sep 2026 11:48:40 -0700 Subject: [PATCH 14/14] test(mothership): pin the unencodable-output guard and the error in the whole-result measure The unencodable-output test placed the BigInt in the lifted output, which the whole-result walk reaches first, so it passed without the guard. It now puts a bulky log ahead of the BigInt log. A new test fails if the error is left out of the measure the result is bounded against. --- .../run-workflow-result-budget.test.ts | 49 ++++++++++++++----- 1 file changed, 36 insertions(+), 13 deletions(-) diff --git a/apps/sim/lib/mothership/tools/handlers/workflow/run-workflow-result-budget.test.ts b/apps/sim/lib/mothership/tools/handlers/workflow/run-workflow-result-budget.test.ts index 4454194a78e..ab92e9d0dd0 100644 --- a/apps/sim/lib/mothership/tools/handlers/workflow/run-workflow-result-budget.test.ts +++ b/apps/sim/lib/mothership/tools/handlers/workflow/run-workflow-result-budget.test.ts @@ -9,6 +9,7 @@ import { telemetryMock } from '@sim/testing/mocks/telemetry.mock' import { workflowsOrchestrationMock } from '@sim/testing/mocks/workflows-orchestration.mock' import { getErrorMessage } from '@sim/utils/errors' import { beforeEach, describe, expect, it, vi } from 'vitest' +import { MAX_INLINE_MATERIALIZATION_BYTES } from '@/lib/execution/payloads/limits' import { inspectToolResultForCopilot } from '@/lib/mothership/request/tools/resolved-secret-result' import type { ExecutionContext } from '@/lib/mothership/request/types' import { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry' @@ -503,31 +504,53 @@ describe('run_workflow model-facing result budget', () => { /** * The bound only handles size. An output JSON cannot encode cannot be checked, so the run is still - * refused rather than the value being hidden behind a pointer. + * refused rather than the value being hidden behind a pointer, even while a bulky log beside it + * is replaced. The bulky log comes first, so the whole-result walk passes the cap before it ever + * reaches the unencodable one. */ it('still refuses a run with an unencodable block output while bounding bulky ones', async () => { + const narrow = Array.from({ length: 25_000 }, (_, index) => ({ + id: `r${index}`, + data: { a: 'x' }, + })) mocks.executeWorkflowUseCase.mockResolvedValue({ success: true, - output: {}, + output: { done: true }, logs: [ - { - blockId: 'big', - blockName: 'Big', - success: true, - output: { rows: tableRows(4_800, 3_200_000) }, - }, + { blockId: 'big', blockName: 'Big', success: true, output: { rows: narrow } }, { blockId: 'odd', blockName: 'Odd', success: true, output: { count: 1n } }, ], metadata: { executionId: EXECUTION_ID }, }) - const settled = await executeRunWorkflowUntilBlock( - { workflowId: 'wf-1', stopAfterBlockId: 'odd' }, - context + const settled = await executeRunWorkflow({ workflowId: 'wf-1' }, context) + const presented = settled.output as { logs: Array<{ output: unknown }> } + expect(presented.logs[0]?.output).toEqual( + expect.stringContaining(`logs get ${EXECUTION_ID} --trace`) ) - const presented = settled.output as { output: unknown; logs: Array<{ output: unknown }> } - expect(presented.output).toEqual({ count: 1n }) expect(presented.logs[1]?.output).toEqual({ count: 1n }) expect(inspectToolResultForCopilot(settled, registry, 'run_workflow').safe).toBe(false) }) + + /** The projection checks the error with the output, so the whole-result measure includes it. */ + it('counts the error toward the caps a run result is bounded against', async () => { + const text = { text: 'y'.repeat(Math.floor(MAX_INLINE_MATERIALIZATION_BYTES * 0.6)) } + const error = `Report failed: ${'e'.repeat(Math.floor(MAX_INLINE_MATERIALIZATION_BYTES * 0.5))}` + mocks.executeWorkflowUseCase.mockResolvedValue({ + success: false, + error, + output: { done: false }, + logs: [{ blockId: 'big', blockName: 'Big', success: true, output: text }], + metadata: { executionId: EXECUTION_ID }, + }) + + const settled = await executeRunWorkflow({ workflowId: 'wf-1' }, context) + const projection = inspectToolResultForCopilot(settled, registry, 'run_workflow') + + expect(projection.safe).toBe(true) + expect(projection.result.error).toBe(error) + expect( + (projection.result.output as { logs: Array<{ output: unknown }> }).logs[0]?.output + ).toEqual(expect.stringContaining(`logs get ${EXECUTION_ID} --trace`)) + }) })