Skip to content

Commit 98b6782

Browse files
committed
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.
1 parent 0157aa9 commit 98b6782

5 files changed

Lines changed: 149 additions & 32 deletions

File tree

‎apps/sim/lib/mothership/request/tools/client.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,7 @@ export async function waitForWorkflowToolCompletion({
377377
? { output: trustedExecution.finalOutput }
378378
: {}),
379379
// Built from raw logs before projection, matching the server handler's presentation.
380-
...presentWorkflowLogsForModel(trustedExecution.blockLogs, executionId, select),
380+
...presentWorkflowLogsForModel(trustedExecution.blockLogs, executionId, toolRegistry, select),
381381
...(trustedExecution.error !== undefined ? { error: trustedExecution.error } : {}),
382382
...(status === MothershipStreamV1ToolOutcome.cancelled
383383
? { reason: 'user_cancelled', cancelledByUser: true }

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import type { ToolCallEffect, ToolExecutionResult } from '@/lib/mothership/tool-executor/types'
22
import { TOOL_EFFECT_PHASE } from '@/lib/mothership/tool-executor/types'
33
import {
4+
getResolvedSecretModelMatcher,
45
measureModelContent,
56
projectResolvedSecretModelJsonContent,
67
} from '@/executor/utils/resolved-secret-content-projection'
@@ -237,6 +238,23 @@ export function inspectToolResultForCopilot(
237238
}
238239
}
239240

241+
/**
242+
* Whether {@link inspectToolResultForCopilot} will walk a result under `registry` against active
243+
* secrets, and so refuse it past the projection's value and depth caps as well as its byte cap.
244+
* With no active secret the projection passes JSON through under the byte cap alone. A registry
245+
* the projection cannot use withholds the result anyway, so it counts as walked.
246+
*/
247+
export function copilotProjectionWalksContent(
248+
registry: ResolvedSecretTraceRegistry | undefined
249+
): boolean {
250+
try {
251+
const snapshot = getResolvedSecretModelMatcher(registry?.forkForPropagatedEntries())
252+
return !snapshot.complete || snapshot.matcher !== undefined
253+
} catch {
254+
return true
255+
}
256+
}
257+
240258
/**
241259
* Projects terminal tool content before it can cross back into Copilot.
242260
* Runtime output remains unchanged for raw post-processing and context updates.

‎apps/sim/lib/mothership/tools/handlers/workflow/mutations.ts‎

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ import {
5050
} from '@/lib/workflows/application/update-workflow-content'
5151
import { sanitizeForCopilot } from '@/lib/workflows/sanitization/json-sanitizer'
5252
import { hasExecutionResult, readAttemptedExecutionId } from '@/executor/utils/errors'
53+
import type { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry'
5354
import type { WorkflowState } from '@/stores/workflows/workflow/types'
5455

5556
const logger = createLogger('WorkflowMutations')
@@ -138,6 +139,7 @@ function buildExecutionOutput(
138139
error?: string
139140
status?: ExecutionResultStatus
140141
},
142+
registry: ResolvedSecretTraceRegistry | undefined,
141143
phase: ToolEffectPhase,
142144
extra?: Record<string, unknown>,
143145
select?: string[]
@@ -154,9 +156,9 @@ function buildExecutionOutput(
154156
executionId,
155157
success: result.success,
156158
...extra,
157-
output: lifted ? compactLiftedBlockOutput(lifted.output, executionId) : output,
159+
output: lifted ? compactLiftedBlockOutput(lifted.output, executionId, registry) : output,
158160
...(lifted ? { outputFrom: lifted.outputFrom } : {}),
159-
...presentWorkflowLogsForModel(logs, executionId, select),
161+
...presentWorkflowLogsForModel(logs, executionId, registry, select),
160162
},
161163
error: result.success
162164
? undefined
@@ -189,14 +191,18 @@ function failedBlockError(logs: unknown): string | undefined {
189191
return undefined
190192
}
191193

192-
function buildExecutionError(error: unknown): ToolCallResult {
194+
function buildExecutionError(
195+
error: unknown,
196+
registry: ResolvedSecretTraceRegistry | undefined
197+
): ToolCallResult {
193198
if (hasExecutionResult(error)) {
194199
return buildExecutionOutput(
195200
{
196201
...error.executionResult,
197202
success: false,
198203
error: error.executionResult.error || 'Workflow execution failed',
199204
},
205+
registry,
200206
settledPhase(error.executionResult.status)
201207
)
202208
}
@@ -344,9 +350,15 @@ export async function executeRunWorkflow(
344350
lifecycle: copilotRunLifecycle(context),
345351
})
346352

347-
return buildExecutionOutput(result, settledPhase(result.status), undefined, params.select)
353+
return buildExecutionOutput(
354+
result,
355+
context.resolvedSecretTraceRegistry,
356+
settledPhase(result.status),
357+
undefined,
358+
params.select
359+
)
348360
} catch (error) {
349-
return buildExecutionError(error)
361+
return buildExecutionError(error, context.resolvedSecretTraceRegistry)
350362
}
351363
}
352364

@@ -507,12 +519,13 @@ export async function executeRunWorkflowUntilBlock(
507519

508520
return buildExecutionOutput(
509521
result,
522+
context.resolvedSecretTraceRegistry,
510523
settledPhase(result.status),
511524
{ stoppedAfterBlockId: params.stopAfterBlockId },
512525
params.select
513526
)
514527
} catch (error) {
515-
return buildExecutionError(error)
528+
return buildExecutionError(error, context.resolvedSecretTraceRegistry)
516529
}
517530
}
518531

@@ -588,12 +601,13 @@ export async function executeRunFromBlock(
588601

589602
return buildExecutionOutput(
590603
result,
604+
context.resolvedSecretTraceRegistry,
591605
settledPhase(result.status),
592606
{ startBlockId: params.startBlockId },
593607
params.select
594608
)
595609
} catch (error) {
596-
return buildExecutionError(error)
610+
return buildExecutionError(error, context.resolvedSecretTraceRegistry)
597611
}
598612
}
599613

@@ -680,11 +694,12 @@ export async function executeRunBlock(
680694

681695
return buildExecutionOutput(
682696
result,
697+
context.resolvedSecretTraceRegistry,
683698
settledPhase(result.status),
684699
{ blockId: params.blockId },
685700
params.select
686701
)
687702
} catch (error) {
688-
return buildExecutionError(error)
703+
return buildExecutionError(error, context.resolvedSecretTraceRegistry)
689704
}
690705
}

‎apps/sim/lib/mothership/tools/handlers/workflow/run-workflow-result-budget.test.ts‎

Lines changed: 87 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,16 @@ import {
3838

3939
const EXECUTION_ID = '0f4d5a4c-6a1e-4c2f-9b7d-2c8f1a3e5d90'
4040
const SECRET = 'fake-secret-for-test-only'
41-
const context = {
42-
userId: 'user-1',
43-
workspaceId: 'workspace-1',
44-
toolCallId: 'tool-call-1',
45-
} as ExecutionContext
41+
42+
/** The handler and the projection share the call's registry, as the tool executor wires them. */
43+
function callContext(registry: ResolvedSecretTraceRegistry) {
44+
return {
45+
userId: 'user-1',
46+
workspaceId: 'workspace-1',
47+
toolCallId: 'tool-call-1',
48+
resolvedSecretTraceRegistry: registry,
49+
} as ExecutionContext
50+
}
4651

4752
/** Rows and approximate encoded bytes per block of a synthetic trace-shaped run. */
4853
const TABLE_QUERIES: ReadonlyArray<readonly [rows: number, bytes: number]> = [
@@ -85,17 +90,30 @@ function traceShapedLogs() {
8590
})
8691
}
8792

88-
function secretRegistry() {
93+
/** A configured secret; `active` records that the run resolved it into its result. */
94+
function secretRegistry({ active = true } = {}) {
8995
const registry = new ResolvedSecretTraceRegistry([
9096
{ name: 'API_KEY', plaintext: SECRET, encryptedValue: 'ciphertext' },
9197
])
92-
registry.recordResolved('API_KEY', SECRET, { propagated: true })
98+
if (active) registry.recordResolved('API_KEY', SECRET, { propagated: true })
9399
return registry
94100
}
95101

102+
/** Row-shaped output of about 27.5k values: past the log budget, under the projection's cap. */
103+
function wideRows() {
104+
return Array.from({ length: 2_500 }, (_, index) =>
105+
Object.fromEntries(Array.from({ length: 10 }, (_, column) => [`c${column}`, `r${index}`]))
106+
)
107+
}
108+
96109
describe('run_workflow model-facing result budget', () => {
110+
let registry: ResolvedSecretTraceRegistry
111+
let context: ExecutionContext
112+
97113
beforeEach(() => {
98114
mocks.executeWorkflowUseCase.mockReset()
115+
registry = secretRegistry()
116+
context = callContext(registry)
99117
})
100118

101119
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', () => {
110128
})
111129

112130
const settled = await executeRunWorkflow({ workflowId: 'wf-1' }, context)
113-
const projection = inspectToolResultForCopilot(settled, secretRegistry(), 'run_workflow')
131+
const projection = inspectToolResultForCopilot(settled, registry, 'run_workflow')
114132

115133
expect(projection.safe).toBe(true)
116134
const output = projection.result.output as Record<string, unknown>
@@ -140,7 +158,7 @@ describe('run_workflow model-facing result budget', () => {
140158
})
141159

142160
const settled = await executeRunWorkflow({ workflowId: 'wf-1' }, context)
143-
const projection = inspectToolResultForCopilot(settled, secretRegistry(), 'run_workflow')
161+
const projection = inspectToolResultForCopilot(settled, registry, 'run_workflow')
144162

145163
expect(projection.safe).toBe(true)
146164
expect((projection.result.output as Record<string, unknown>).output).toEqual(finalOutput)
@@ -164,7 +182,7 @@ describe('run_workflow model-facing result budget', () => {
164182

165183
const settled = await executeRunWorkflow({ workflowId: 'wf-1' }, context)
166184
expect(Buffer.byteLength(JSON.stringify(settled.output))).toBeLessThan(4 * 1024 * 1024)
167-
const projection = inspectToolResultForCopilot(settled, secretRegistry(), 'run_workflow')
185+
const projection = inspectToolResultForCopilot(settled, registry, 'run_workflow')
168186

169187
expect(projection.safe).toBe(true)
170188
expect((projection.result.output as Record<string, unknown>).output).toEqual(finalOutput)
@@ -183,7 +201,7 @@ describe('run_workflow model-facing result budget', () => {
183201
{ workflowId: 'wf-1', select: ['Query 4.rows'] },
184202
context
185203
)
186-
const projection = inspectToolResultForCopilot(settled, secretRegistry(), 'run_workflow')
204+
const projection = inspectToolResultForCopilot(settled, registry, 'run_workflow')
187205

188206
expect(projection.safe).toBe(true)
189207
const output = projection.result.output as Record<string, unknown>
@@ -235,7 +253,7 @@ describe('run_workflow model-facing result budget', () => {
235253
{ workflowId: 'wf-1', stopAfterBlockId: 'query' },
236254
context
237255
)
238-
const projection = inspectToolResultForCopilot(settled, secretRegistry(), 'run_workflow')
256+
const projection = inspectToolResultForCopilot(settled, registry, 'run_workflow')
239257

240258
expect(projection.safe).toBe(true)
241259
const output = projection.result.output as Record<string, unknown>
@@ -286,7 +304,7 @@ describe('run_workflow model-facing result budget', () => {
286304
})
287305

288306
const settled = await executeRunWorkflow({ workflowId: 'wf-1' }, context)
289-
const projection = inspectToolResultForCopilot(settled, secretRegistry(), 'run_workflow')
307+
const projection = inspectToolResultForCopilot(settled, registry, 'run_workflow')
290308

291309
expect(projection.safe).toBe(true)
292310
const logs = (projection.result.output as { logs: Array<Record<string, unknown>> }).logs
@@ -313,7 +331,7 @@ describe('run_workflow model-facing result budget', () => {
313331
})
314332

315333
const settled = await executeRunWorkflow({ workflowId: 'wf-1' }, context)
316-
const projection = inspectToolResultForCopilot(settled, secretRegistry(), 'run_workflow')
334+
const projection = inspectToolResultForCopilot(settled, registry, 'run_workflow')
317335

318336
expect(projection.safe).toBe(true)
319337
const serialized = JSON.stringify(projection.result)
@@ -322,4 +340,59 @@ describe('run_workflow model-facing result budget', () => {
322340
expect(serialized).not.toContain(SECRET.slice(0, length))
323341
}
324342
})
343+
344+
/**
345+
* Without an active secret the projection passes JSON through under its byte cap alone, so
346+
* nothing is bounded: a lifted output is the whole point of run_block and reaches the worker in
347+
* full, which spills an oversized one to storage for the model to read.
348+
*/
349+
it('returns a large lifted output and its logs in full when no secret is active', async () => {
350+
const rows = wideRows()
351+
mocks.executeWorkflowUseCase.mockResolvedValue({
352+
success: true,
353+
output: {},
354+
logs: [
355+
{ blockId: 'start', blockName: 'Start', success: true, output: { ok: true } },
356+
{ blockId: 'query', blockName: 'Query', success: true, output: { rows } },
357+
],
358+
metadata: { executionId: EXECUTION_ID },
359+
})
360+
const inactive = secretRegistry({ active: false })
361+
362+
const settled = await executeRunWorkflowUntilBlock(
363+
{ workflowId: 'wf-1', stopAfterBlockId: 'query' },
364+
callContext(inactive)
365+
)
366+
const projection = inspectToolResultForCopilot(settled, inactive, 'run_workflow')
367+
368+
expect(projection.safe).toBe(true)
369+
const output = projection.result.output as Record<string, unknown>
370+
expect(output.output).toEqual({ rows })
371+
expect((output.logs as Array<Record<string, unknown>>)[1]?.output).toEqual({ rows })
372+
})
373+
374+
/** A lifted output is the run's final output, so it keeps the final output's larger share. */
375+
it('returns a lifted output within the final share in full while a secret is active', async () => {
376+
const rows = wideRows()
377+
mocks.executeWorkflowUseCase.mockResolvedValue({
378+
success: true,
379+
output: {},
380+
logs: [{ blockId: 'query', blockName: 'Query', success: true, output: { rows } }],
381+
metadata: { executionId: EXECUTION_ID },
382+
})
383+
384+
const settled = await executeRunWorkflowUntilBlock(
385+
{ workflowId: 'wf-1', stopAfterBlockId: 'query' },
386+
context
387+
)
388+
const projection = inspectToolResultForCopilot(settled, registry, 'run_workflow')
389+
390+
expect(projection.safe).toBe(true)
391+
const output = projection.result.output as Record<string, unknown>
392+
expect(output.output).toEqual({ rows })
393+
// Its log copy is still bounded, so the two together stay under the projection's caps.
394+
expect((output.logs as Array<Record<string, unknown>>)[0]?.output).toEqual(
395+
expect.stringContaining(`logs get ${EXECUTION_ID} --trace`)
396+
)
397+
})
325398
})

0 commit comments

Comments
 (0)