Skip to content

Commit a700b9b

Browse files
committed
fix(mothership): align attachment context with available tools
1 parent c71ae15 commit a700b9b

4 files changed

Lines changed: 75 additions & 34 deletions

File tree

‎apps/sim/lib/mothership/chat/post.test.ts‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -604,6 +604,40 @@ describe('handleUnifiedChatPost', () => {
604604
expect(processContextsServer).not.toHaveBeenCalled()
605605
})
606606

607+
it('passes browser attachment metadata without advertising an unavailable browser agent', async () => {
608+
const response = await handleUnifiedChatPost(
609+
new NextRequest('http://localhost/api/copilot/chat', {
610+
method: 'POST',
611+
body: JSON.stringify({
612+
message: 'Explain the selected page',
613+
workspaceId: 'ws-1',
614+
createNewChat: true,
615+
resourceAttachments: [
616+
{
617+
type: 'browser',
618+
id: 'browser-session',
619+
title: 'Documentation',
620+
url: 'https://docs.example.com/guide',
621+
active: true,
622+
},
623+
],
624+
}),
625+
})
626+
)
627+
expect(response.status).toBe(200)
628+
const payload = buildCopilotRequestPayload.mock.calls[0]?.[0]
629+
expect(payload.contexts).toEqual([
630+
expect.objectContaining({
631+
type: 'active_resource',
632+
tag: '@active_tab',
633+
content: expect.stringContaining('cannot read or drive browser tabs'),
634+
}),
635+
])
636+
expect(payload.contexts[0].content).toContain('https://docs.example.com/guide')
637+
expect(payload.contexts[0].content).toContain('Documentation')
638+
expect(payload.contexts[0].content).not.toContain('browser subagent')
639+
})
640+
607641
it('forwards slash-selected MCP server ids to the request-local tool builder', async () => {
608642
const response = await handleUnifiedChatPost(
609643
new NextRequest('http://localhost/api/copilot/chat', {

‎apps/sim/lib/mothership/chat/post.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -479,9 +479,9 @@ async function resolveAgentContexts(params: {
479479
tag: resource.active ? '@active_tab' : '@open_tab',
480480
content: `The user's ${
481481
resource.active ? 'currently visible browser tab' : 'other open browser tab'
482-
} (driven by the browser subagent) is open on: ${
482+
} is open on: ${
483483
title ? `"${title}" — ` : ''
484-
}${resource.url}`,
484+
}${resource.url}. You cannot read or drive browser tabs here; this attachment supplies only the title and URL.`,
485485
}
486486
}
487487
const ctx = await resolveActiveResourceContext(

‎apps/sim/lib/mothership/chat/process-contents.test.ts‎

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
MAX_TABLE_SELECTION_PREVIEW_LENGTH,
1111
MAX_TABLE_SELECTION_ROWS,
1212
} from '@/lib/mothership/chat/selection-context'
13+
import { buildTaggedMcpToolSchemas } from '@/lib/mothership/mcp-tools'
1314
import { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry'
1415
import type { ChatContext } from '@/stores/panel'
1516

@@ -53,7 +54,10 @@ const {
5354

5455
vi.mock('@/blocks/registry', () => ({ getBlock, getBlockRegistry }))
5556
vi.mock('@/lib/mothership/block-visibility', () => ({ getBlockVisibilityForCopilot }))
56-
vi.mock('@/ee/access-control/utils/permission-check', () => ({ getUserPermissionConfig }))
57+
vi.mock('@/ee/access-control/utils/permission-check', () => ({
58+
getUserPermissionConfig,
59+
validateMcpToolsAllowed: vi.fn(),
60+
}))
5761
vi.mock('@/lib/integrations/availability.server', () => ({
5862
isIntegrationDeploymentAvailableForVisibility: isIntegrationDeploymentAvailable,
5963
}))
@@ -505,7 +509,7 @@ describe('processContextsServer - MCP contexts', () => {
505509
vi.clearAllMocks()
506510
})
507511

508-
it('lists only the tools from the slash-selected MCP server', async () => {
512+
it('references the selected service while the request catalog owns tool discovery', async () => {
509513
discoverServerTools.mockResolvedValue([
510514
{
511515
serverId: 'mcp-server-1',
@@ -523,14 +527,36 @@ describe('processContextsServer - MCP contexts', () => {
523527
'ws-1'
524528
)
525529

526-
expect(discoverServerTools).toHaveBeenCalledWith('user-1', 'mcp-server-1', 'ws-1')
527530
expect(result).toEqual([
528-
expect.objectContaining({
531+
{
529532
type: 'mcp',
530533
tag: '/Docs',
531-
content: expect.stringContaining('mcp-server-1-search'),
532-
}),
534+
content: JSON.stringify({ serverId: 'mcp-server-1', service: 'mcp:mcp-server-1' }),
535+
},
533536
])
537+
expect(discoverServerTools).not.toHaveBeenCalled()
538+
const catalog = await buildTaggedMcpToolSchemas('user-1', 'ws-1', ['mcp-server-1'])
539+
expect(discoverServerTools).toHaveBeenCalledTimes(1)
540+
expect(discoverServerTools).toHaveBeenCalledWith('user-1', 'mcp-server-1', 'ws-1')
541+
expect(catalog).toMatchObject([
542+
{ name: 'mcp-server-1-search', service: JSON.parse(result[0].content).service },
543+
])
544+
})
545+
546+
it('keeps the selected service reference when discovery is unavailable', async () => {
547+
discoverServerTools.mockRejectedValue(new Error('Server temporarily unavailable'))
548+
const result = await processContextsServer(
549+
[{ kind: 'mcp', serverId: 'mcp-server-1', label: 'Docs' }],
550+
'user-1',
551+
'/Docs find auth docs',
552+
'ws-1'
553+
)
554+
expect(result).toHaveLength(1)
555+
expect(JSON.parse(result[0].content)).toEqual({
556+
serverId: 'mcp-server-1',
557+
service: 'mcp:mcp-server-1',
558+
})
559+
expect(discoverServerTools).not.toHaveBeenCalled()
534560
})
535561
})
536562

@@ -557,7 +583,8 @@ describe('processContextsServer - browser and terminal selections', () => {
557583
},
558584
])
559585
expect(result[0].content).toContain('cannot read or drive browser tabs')
560-
expect(result[1].content).toContain('terminal list operation')
586+
expect(result[1].content).toContain('cannot read or drive terminals')
587+
expect(result[1].content).not.toContain('terminal list operation')
561588
})
562589

563590
it('keeps the live browser pointer and appends quoted untrusted page text', async () => {

‎apps/sim/lib/mothership/chat/process-contents.ts‎

Lines changed: 5 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,6 @@ import { isIntegrationDeploymentAvailableForVisibility } from '@/lib/integration
1313
import { readKnowledgeBase } from '@/lib/knowledge/application/knowledge-bases'
1414
import { toOverview } from '@/lib/logs/log-views'
1515
import type { TraceSpan } from '@/lib/logs/types'
16-
import { mcpService } from '@/lib/mcp/service'
17-
import { createMcpToolId } from '@/lib/mcp/utils'
1816
import { createCopilotChatKnowledgePrincipal } from '@/lib/mothership/application/execute-knowledge-use-case'
1917
import { createCopilotChatTablePrincipal } from '@/lib/mothership/application/execute-table-use-case'
2018
import { createCopilotChatFilePrincipal } from '@/lib/mothership/auth/file-delegation'
@@ -73,13 +71,7 @@ interface AgentContext {
7371
type: AgentContextType
7472
tag: string
7573
content: string
76-
/**
77-
* Canonical, URL-encoded VFS path for the tagged resource (e.g.
78-
* `agent/skills/My%20Skill.json`). Tagged resources are sent as path
79-
* pointers so the model reads them on demand via VFS tools instead of the
80-
* full body bloating the request. Skills are the exception: they carry both
81-
* `path` and the full `content` so the skill is autoloaded.
82-
*/
74+
/** A CLI-readable file address; other resources carry canonical references in content. */
8375
path?: string
8476
}
8577

@@ -135,21 +127,11 @@ export async function processContextsServer(
135127
)
136128
}
137129
if (ctx.kind === 'mcp' && ctx.serverId && currentWorkspaceId) {
138-
const tools = await mcpService.discoverServerTools(userId, ctx.serverId, currentWorkspaceId)
139-
if (tools.length === 0) return null
140-
const toolLines = tools.map((tool) => {
141-
const name = createMcpToolId(tool.serverId, tool.name)
142-
return `- ${name}: ${tool.description || tool.name}`
143-
})
130+
/** The authorized request catalog owns discovery; context identifies the selected service. */
144131
return {
145132
type: 'mcp',
146133
tag: ctx.label ? `/${ctx.label}` : '/',
147-
content: [
148-
`The user explicitly enabled the MCP server "${ctx.label || ctx.serverId}". It stays enabled for the rest of this chat, and its tools remain callable on every later turn.`,
149-
'Its tools are listed below and are callable directly by the exact name shown — there is no loading step.',
150-
'Do not narrate discovery, tool-name selection, or retries. Call the tool first, then respond once with the result. Never claim the server works before a successful tool result. Do not automatically retry a timed-out or abandoned MCP call.',
151-
...toolLines,
152-
].join('\n'),
134+
content: JSON.stringify({ serverId: ctx.serverId, service: `mcp:${ctx.serverId}` }),
153135
}
154136
}
155137
if (ctx.kind === 'past_chat' && ctx.chatId) {
@@ -195,9 +177,7 @@ export async function processContextsServer(
195177
currentWorkspaceId
196178
)
197179
}
198-
// Every tab context retains its live pointer. An explicit user selection
199-
// additionally carries the quoted snapshot they chose, while the pointer
200-
// lets the agent inspect or act on the current page/shell when needed.
180+
/** Desktop context carries a reference and optional selection; v1 has no desktop control tools. */
201181
if (ctx.kind === 'browser_tab' && ctx.tabId) {
202182
if (ctx.tabId === BROWSER_SESSION_RESOURCE_ID) {
203183
return {
@@ -222,7 +202,7 @@ export async function processContextsServer(
222202
type: 'terminal_tab',
223203
tag: ctx.label ? `@${ctx.label}` : '@Terminal',
224204
content:
225-
'The user tagged the Terminal resource as a whole, not a specific shell. Inspect the live terminals with the terminal list operation and choose the relevant one from their request. If no terminal is open yet, create one as needed.',
205+
'The user tagged the Terminal resource as a whole, not a specific shell. You cannot read or drive terminals here: work from output the user shares.',
226206
}
227207
}
228208
const pointer = `The user pointed at an open terminal: "${ctx.label}" (terminalId ${ctx.terminalId}). You cannot read or drive terminals here: ask the user to paste the relevant output rather than assuming what is in it.`

0 commit comments

Comments
 (0)