Skip to content

Commit b1168ac

Browse files
authored
fix(mothership): keep display activity out of integration and mcp tool arguments (#9284)
* fix(mothership): keep display activity out of integration and mcp tool arguments * fix(mothership): strip only the display label, and keep a tool's own activity argument * refactor(mothership): one display-activity check, and omit for stripping it * fix(mothership): hold an inherited activity beside a call's arguments instead of stripping it at execution * fix(mothership): persist an inherited grouping for an operation without arguments
1 parent 3544f9f commit b1168ac

5 files changed

Lines changed: 166 additions & 9 deletions

File tree

‎apps/sim/lib/mothership/chat/persisted-message.test.ts‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,34 @@ describe('persisted-message', () => {
101101
}
102102
)
103103

104+
it.each([
105+
['with its arguments', { category_id: null }],
106+
['without arguments', undefined],
107+
])("persists a resolved operation's inherited grouping %s", (_case, params) => {
108+
const persisted = buildPersistedAssistantMessage({
109+
success: true,
110+
content: '',
111+
toolCalls: [],
112+
contentBlocks: [
113+
{
114+
type: 'tool_call',
115+
timestamp: 1,
116+
toolCall: {
117+
id: 'gateway-call',
118+
name: 'mcp-server-get_overview',
119+
status: 'success',
120+
params,
121+
activity: { id: 'gauge-read', title: 'Reading Gauge', completedTitle: 'Read Gauge' },
122+
},
123+
},
124+
],
125+
})
126+
expect(persisted.contentBlocks?.[0].toolCall?.params).toEqual({
127+
...params,
128+
activity: { id: 'gauge-read', title: 'Reading Gauge', completedTitle: 'Read Gauge' },
129+
})
130+
})
131+
104132
it('preserves activity metadata on legacy tool blocks without inferring it from old titles', () => {
105133
const message = normalizeMessage({
106134
id: 'message-1',

‎apps/sim/lib/mothership/chat/persisted-message.ts‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import {
2424
MothershipStreamV1ToolOutcome,
2525
MothershipStreamV1ToolPhase,
2626
} from '@/lib/mothership/generated/mothership-stream-v1'
27+
import type { ToolActivity } from '@/lib/mothership/generated/protocol'
2728
import type {
2829
ContentBlock,
2930
LocalToolCallStatus,
@@ -273,6 +274,14 @@ export function displayContentOf(content: PersistedMessage): PersistedMessage |
273274

274275
// Write: OrchestratorResult → PersistedMessage
275276

277+
/** A call's arguments as history reads them, with an inherited display grouping restored. */
278+
function persistedParams(
279+
params: Record<string, unknown> | undefined,
280+
activity: ToolActivity | undefined
281+
): Record<string, unknown> | undefined {
282+
return activity && params?.activity === undefined ? { ...params, activity } : params
283+
}
284+
276285
function resolveToolState(block: ContentBlock): PersistedToolState {
277286
const tc = block.toolCall
278287
if (!tc) return 'pending'
@@ -382,17 +391,14 @@ function mapContentBlockBody(block: ContentBlock): PersistedContentBlock {
382391
const activityDescription = normalizeToolActivityDescription(
383392
block.toolCall.activityDescription
384393
)
394+
const params = persistedParams(block.toolCall.params, block.toolCall.activity)
385395
const toolCall: PersistedToolCall = {
386396
id: block.toolCall.id,
387397
name: block.toolCall.name,
388398
state,
389399
...(activityDescription ? { activityDescription } : {}),
390400
...(isSubagentTool && isNonTerminal ? {} : { result: redactedResult }),
391-
...(isSubagentTool && isNonTerminal
392-
? {}
393-
: block.toolCall.params
394-
? { params: block.toolCall.params }
395-
: {}),
401+
...(isSubagentTool && isNonTerminal ? {} : params ? { params } : {}),
396402
...(block.calledBy ? { calledBy: block.calledBy } : {}),
397403
...(block.toolCall.displayTitle
398404
? {

‎apps/sim/lib/mothership/request/handlers/handlers.test.ts‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { flushMacrotask } from '@sim/testing/helpers/async'
12
import { createDeferred } from '@sim/testing/helpers/deferred'
23
import {
34
mothershipAsyncRunsMock,
@@ -2252,6 +2253,112 @@ describe('sse-handlers tool lifecycle', () => {
22522253
)
22532254
})
22542255

2256+
it("executes a resolved MCP operation with its own arguments under the gateway's grouping", async () => {
2257+
isSimExecuted.mockReturnValue(false)
2258+
executeTool.mockResolvedValueOnce({ success: true, output: {} })
2259+
const grouping = { id: 'gauge-read', title: 'Reading Gauge', completedTitle: 'Read Gauge' }
2260+
2261+
await sseHandlers.tool(
2262+
{
2263+
type: MothershipStreamV1EventType.tool,
2264+
payload: {
2265+
toolCallId: 'gateway-mcp',
2266+
toolName: 'call_integration_tool',
2267+
arguments: {
2268+
toolId: 'mcp-server-get_overview',
2269+
description: 'Pulling the overview',
2270+
activity: grouping,
2271+
arguments: { category_id: null },
2272+
},
2273+
executor: MothershipStreamV1ToolExecutor.go,
2274+
mode: MothershipStreamV1ToolMode.sync,
2275+
phase: MothershipStreamV1ToolPhase.call,
2276+
},
2277+
} satisfies StreamEvent,
2278+
context,
2279+
execContext,
2280+
{ interactive: false, timeout: 1000 }
2281+
)
2282+
2283+
await sseHandlers.tool(
2284+
{
2285+
type: MothershipStreamV1EventType.tool,
2286+
payload: {
2287+
toolCallId: 'gateway-mcp',
2288+
toolName: 'mcp-server-get_overview',
2289+
arguments: { category_id: null },
2290+
executor: MothershipStreamV1ToolExecutor.sim,
2291+
mode: MothershipStreamV1ToolMode.async,
2292+
phase: MothershipStreamV1ToolPhase.call,
2293+
},
2294+
} satisfies StreamEvent,
2295+
context,
2296+
execContext,
2297+
{ interactive: false, timeout: 1000 }
2298+
)
2299+
2300+
await flushMacrotask()
2301+
2302+
expect(executeTool).toHaveBeenCalledWith(
2303+
'mcp-server-get_overview',
2304+
{ category_id: null },
2305+
expect.any(Object)
2306+
)
2307+
expect(context.toolCalls.get('gateway-mcp')?.activity).toEqual(grouping)
2308+
})
2309+
2310+
it("keeps an integration operation's own activity argument under the gateway's grouping", async () => {
2311+
isSimExecuted.mockReturnValue(false)
2312+
executeTool.mockResolvedValueOnce({ success: true, output: {} })
2313+
2314+
await sseHandlers.tool(
2315+
{
2316+
type: MothershipStreamV1EventType.tool,
2317+
payload: {
2318+
toolCallId: 'gateway-work-item',
2319+
toolName: 'call_integration_tool',
2320+
arguments: {
2321+
toolId: 'azure_devops_create_work_item',
2322+
description: 'Filing the task',
2323+
activity: { id: 'filing', title: 'Filing tasks', completedTitle: 'Filed tasks' },
2324+
arguments: { title: 'Ship it', activity: 'Development' },
2325+
},
2326+
executor: MothershipStreamV1ToolExecutor.go,
2327+
mode: MothershipStreamV1ToolMode.sync,
2328+
phase: MothershipStreamV1ToolPhase.call,
2329+
},
2330+
} satisfies StreamEvent,
2331+
context,
2332+
execContext,
2333+
{ interactive: false, timeout: 1000 }
2334+
)
2335+
2336+
await sseHandlers.tool(
2337+
{
2338+
type: MothershipStreamV1EventType.tool,
2339+
payload: {
2340+
toolCallId: 'gateway-work-item',
2341+
toolName: 'azure_devops_create_work_item',
2342+
arguments: { title: 'Ship it', activity: 'Development' },
2343+
executor: MothershipStreamV1ToolExecutor.sim,
2344+
mode: MothershipStreamV1ToolMode.async,
2345+
phase: MothershipStreamV1ToolPhase.call,
2346+
},
2347+
} satisfies StreamEvent,
2348+
context,
2349+
execContext,
2350+
{ interactive: false, timeout: 1000 }
2351+
)
2352+
2353+
await flushMacrotask()
2354+
2355+
expect(executeTool).toHaveBeenCalledWith(
2356+
'azure_devops_create_work_item',
2357+
{ title: 'Ship it', activity: 'Development' },
2358+
expect.any(Object)
2359+
)
2360+
})
2361+
22552362
it('clears pending continuation state when a run resumes', async () => {
22562363
context.awaitingAsyncContinuation = {
22572364
checkpointId: 'cp-1',

‎apps/sim/lib/mothership/request/handlers/tool.ts‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
MothershipStreamV1ToolOutcome,
2121
type MothershipStreamV1ToolResultPayload,
2222
} from '@/lib/mothership/generated/mothership-stream-v1'
23+
import type { ToolActivity } from '@/lib/mothership/generated/protocol'
2324
import { TraceAttr } from '@/lib/mothership/generated/trace-attributes-v1'
2425
import { TraceSpan } from '@/lib/mothership/generated/trace-spans-v1'
2526
import { withCopilotSpan } from '@/lib/mothership/request/otel'
@@ -145,6 +146,11 @@ function applyToolDisplay(toolCall: ToolCallState | undefined): void {
145146

146147
const INTEGRATION_GATEWAY_TOOL = 'call_integration_tool'
147148

149+
/** A call's display grouping: carried in its own arguments, or inherited and held beside them. */
150+
function toolCallActivity(toolCall: ToolCallState | undefined): ToolActivity | undefined {
151+
return toolCall?.activity ?? readToolActivity(toolCall?.params)
152+
}
153+
148154
function handleToolArgsDelta(
149155
data: { argumentsDelta: string; toolCallId: string; toolName: string },
150156
context: StreamingContext
@@ -579,15 +585,19 @@ async function handleCallPhase(
579585
existing.activityDescription ??= activityDescription
580586
}
581587
/** Persist visible labels even when the introducing discovery call is omitted from the transcript. */
582-
const suppliedActivity = readToolActivity(args) ?? readToolActivity(existing?.params)
588+
const argsActivity = readToolActivity(args)
589+
const suppliedActivity = argsActivity ?? toolCallActivity(existing)
583590
const activity = suppliedActivity?.completedTitle
584591
? suppliedActivity
585592
: suppliedActivity &&
586593
Array.from(context.toolCalls.values())
587594
.filter((tool) => tool.agentId === agentId && tool.parentToolCallId === parentToolCallId)
588-
.map((tool) => readToolActivity(tool.params))
595+
.map(toolCallActivity)
589596
.find((candidate) => candidate?.id === suppliedActivity.id && candidate.completedTitle)
590-
const displayArgs = activity ? { ...args, activity } : args
597+
// Only a label the frame's own arguments carry is completed in place. An inherited one stays
598+
// off the arguments, which execute as given (an MCP schema refuses keys it never declared).
599+
const displayArgs = activity && argsActivity ? { ...args, activity } : args
600+
if (existing && activity && !argsActivity) existing.activity = activity
591601
const isSubagent = scope === 'subagent'
592602
const ui = getToolCallUI(data)
593603

‎apps/sim/lib/mothership/request/types.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import {
33
type MothershipStreamV1CompletionStatus,
44
MothershipStreamV1ToolOutcome,
55
} from '@/lib/mothership/generated/mothership-stream-v1'
6-
import type { StreamActivityReceipt } from '@/lib/mothership/generated/protocol'
6+
import type { StreamActivityReceipt, ToolActivity } from '@/lib/mothership/generated/protocol'
77
import type { RequestTraceV1Span } from '@/lib/mothership/generated/request-trace-v1'
88
import type { ProviderToolCallIdentity } from '@/lib/mothership/request/go/tool-call-identity'
99
import type { StreamEvent } from '@/lib/mothership/request/session'
@@ -42,6 +42,12 @@ export interface ToolCallState {
4242
activityDescription?: string
4343
/** Model-authored activity text for a gateway-resolved integration call. */
4444
integrationDescription?: string
45+
/**
46+
* Display grouping the call inherited rather than carried in its own arguments (a gateway's
47+
* grouping on its resolved operation). Kept out of `params`, which execute as given; it is
48+
* merged back only into the persisted arguments, where history reads it.
49+
*/
50+
activity?: ToolActivity
4551
/** Accumulated partial JSON of the arguments while the model streams them. */
4652
streamingArgs?: string
4753
params?: Record<string, unknown>

0 commit comments

Comments
 (0)