Skip to content

Commit c941abe

Browse files
committed
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.
1 parent dd8a373 commit c941abe

4 files changed

Lines changed: 65 additions & 17 deletions

File tree

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,46 @@ describe('workflow client tool completion', () => {
247247
expect(JSON.stringify(completion)).not.toContain('parent-secret-value')
248248
})
249249

250+
/** Parity with the server path: a `select` is resolved from raw logs before projection. */
251+
it('projects selected values from a large browser run instead of withholding it', async () => {
252+
const rows = (count: number) =>
253+
Array.from({ length: count }, (_, index) => ({
254+
id: `row_${index}`,
255+
data: { a: 'x', b: 'y', c: 'z', d: 'w' },
256+
}))
257+
getTrustedWorkflowToolExecution.mockResolvedValue({
258+
...trustedExecution('execution-1'),
259+
blockLogs: [
260+
{ blockId: 'reader', blockName: 'Reader', output: { token: 'parent-secret-value', n: 2 } },
261+
...Array.from({ length: 4 }, (_, index) => ({
262+
blockId: `query-${index}`,
263+
blockName: `Query ${index}`,
264+
output: { rows: rows(5_000) },
265+
})),
266+
],
267+
})
268+
waitForToolConfirmation.mockResolvedValue({
269+
status: 'success',
270+
data: { workflowId: 'workflow-1', executionId: 'execution-1' },
271+
})
272+
273+
const completion = await waitForWorkflowToolCompletion({
274+
toolCallId: 'tool-1',
275+
workflowId: 'workflow-1',
276+
timeoutMs: 1_000,
277+
registry: createParentRegistry(),
278+
select: ['Reader.token', 'Reader.n'],
279+
})
280+
281+
expect(completion?.data).toMatchObject({
282+
output: { value: 'child read {{PARENT_SECRET}} from execution-1' },
283+
selected: { 'Reader.token': '{{PARENT_SECRET}}', 'Reader.n': 2 },
284+
logsOmitted: true,
285+
})
286+
expect(completion?.data).not.toHaveProperty('logs')
287+
expect(JSON.stringify(completion)).not.toContain('parent-secret-value')
288+
})
289+
250290
it('preserves the server-confirmed status while omitting unavailable execution content', async () => {
251291
const registry = createParentRegistry()
252292
waitForToolConfirmation.mockResolvedValue({

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

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import {
1515
unsealClientToolContext,
1616
} from '@/lib/mothership/request/tools/client-completion-seal.server'
1717
import { inspectToolResultForCopilot } from '@/lib/mothership/request/tools/resolved-secret-result'
18-
import { compactBlockLogOutputs, presentWorkflowLogs } from '@/lib/mothership/tools/workflow-output'
18+
import { presentWorkflowLogsForModel } from '@/lib/mothership/tools/workflow-output'
1919
import {
2020
createStructuralWorkflowToolCompletionData,
2121
getWorkflowToolCompletionExecutionId,
@@ -376,10 +376,8 @@ export async function waitForWorkflowToolCompletion({
376376
...(Object.hasOwn(trustedExecution, 'finalOutput')
377377
? { output: trustedExecution.finalOutput }
378378
: {}),
379-
// `select` reads full values from these logs; only logs echoed whole are bounded.
380-
logs: select?.length
381-
? trustedExecution.blockLogs
382-
: compactBlockLogOutputs(trustedExecution.blockLogs, executionId),
379+
// Built from raw logs before projection, matching the server handler's presentation.
380+
...presentWorkflowLogsForModel(trustedExecution.blockLogs, executionId, select),
383381
...(trustedExecution.error !== undefined ? { error: trustedExecution.error } : {}),
384382
...(status === MothershipStreamV1ToolOutcome.cancelled
385383
? { reason: 'user_cancelled', cancelledByUser: true }
@@ -397,10 +395,8 @@ export async function waitForWorkflowToolCompletion({
397395
)
398396
const projected = projection.result
399397
const projectedData = isPlainRecord(projected.output) ? projected.output : {}
400-
const { logs, ...projectedFields } = projectedData
401398
const data = {
402-
...projectedFields,
403-
...(Object.hasOwn(projectedData, 'logs') ? presentWorkflowLogs(logs, select) : {}),
399+
...projectedData,
404400
...createStructuralWorkflowToolCompletionData(status, workflowId, executionId),
405401
}
406402
const message =

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

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,7 @@ import type {
3131
import { requireCopilotWorkspace } from '@/lib/mothership/tools/server/workspace-scope'
3232
import {
3333
compactBlockLogInputs,
34-
compactBlockLogOutputs,
35-
presentWorkflowLogs,
34+
presentWorkflowLogsForModel,
3635
} from '@/lib/mothership/tools/workflow-output'
3736
import { decodeVfsPathSegments, encodeVfsPathSegments } from '@/lib/mothership/vfs/path-utils'
3837
import { cancelWorkflowRun } from '@/lib/workflows/application/cancel-run'
@@ -157,11 +156,7 @@ function buildExecutionOutput(
157156
...extra,
158157
output: lifted ? lifted.output : output,
159158
...(lifted ? { outputFrom: lifted.outputFrom } : {}),
160-
// `select` reads full values from the run's own logs, so only the echoed logs are bounded.
161-
...presentWorkflowLogs(
162-
select?.length ? logs : compactBlockLogOutputs(logs, executionId),
163-
select
164-
),
159+
...presentWorkflowLogsForModel(logs, executionId, select),
165160
},
166161
error: result.success
167162
? undefined

‎apps/sim/lib/mothership/tools/workflow-output.ts‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,29 @@ import {
66
} from '@/executor/utils/resolved-secret-content-projection'
77

88
/** Shared presentation for server execution and trusted client execution restoration. */
9-
export function presentWorkflowLogs(logs: unknown, select?: string[]): Record<string, unknown> {
9+
function presentWorkflowLogs(logs: unknown, select?: string[]): Record<string, unknown> {
1010
return select?.length
1111
? { selected: selectFromLogs(select, Array.isArray(logs) ? logs : []), logsOmitted: true }
1212
: { logs }
1313
}
1414

15+
/**
16+
* The model-facing log fields for one run, built from raw logs before secret projection so both the
17+
* server handler and browser-run restoration present the same bounded shape: a `select` resolves
18+
* against the full logs and replaces them, otherwise the echoed logs are bounded by
19+
* {@link compactBlockLogOutputs}.
20+
*/
21+
export function presentWorkflowLogsForModel(
22+
logs: unknown,
23+
executionId: string | undefined,
24+
select?: string[]
25+
): Record<string, unknown> {
26+
return presentWorkflowLogs(
27+
select?.length ? logs : compactBlockLogOutputs(logs, executionId),
28+
select
29+
)
30+
}
31+
1532
/** The executor's block-name rule: lowercase, whitespace and dots removed. */
1633
function normalizeSelectorHead(value: string): string {
1734
return value.toLowerCase().replace(/[\s.]+/g, '')
@@ -107,7 +124,7 @@ interface MeasuredLogOutput {
107124
* would disclose a secret's length. It says "inspect" rather than promising the full value, since
108125
* the stored trace can itself be summarized.
109126
*/
110-
export function compactBlockLogOutputs(logs: unknown, executionId: string | undefined): unknown {
127+
function compactBlockLogOutputs(logs: unknown, executionId: string | undefined): unknown {
111128
if (!Array.isArray(logs)) return logs
112129
const reference = executionId ?? '<executionId>'
113130
const measured: MeasuredLogOutput[] = []

0 commit comments

Comments
 (0)