Skip to content

Commit f8d389d

Browse files
committed
fix(mothership): keep reads out of resource panels
1 parent 554714a commit f8d389d

10 files changed

Lines changed: 79 additions & 40 deletions

File tree

‎apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-registry/resource-invalidation.test.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,22 @@ describe('resource cache reconciliation', () => {
5252
expect(client.getQueryState(file)?.isInvalidated).toBe(true)
5353
})
5454

55+
it.each(['file', undefined])(
56+
'refreshes addressed file metadata and versioned content after an edit (id: %s)',
57+
(id) => {
58+
const client = new QueryClient()
59+
const affected = [
60+
workspaceFilesKeys.record('w', 'file'),
61+
workspaceFilesKeys.content('w', 'file', 'text', 'old-storage-key'),
62+
]
63+
const other = workspaceFilesKeys.record('other-workspace', 'file')
64+
for (const key of [...affected, other]) client.setQueryData(key, {})
65+
invalidateResourceQueries(client, 'w', 'file', id)
66+
for (const key of affected) expect(client.getQueryState(key)?.isInvalidated).toBe(true)
67+
expect(client.getQueryState(other)?.isInvalidated).toBe(id === undefined)
68+
}
69+
)
70+
5571
it('updates workflow publication metadata shown beside the draft', () => {
5672
const client = new QueryClient()
5773
const affected = [

‎apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-registry/resource-invalidation.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ const RESOURCE_INVALIDATORS: Record<
2828
},
2929
file: (qc, wId, id) => {
3030
invalidate(qc, workspaceFilesKeys.lists())
31+
invalidate(qc, id ? workspaceFilesKeys.record(wId, id) : workspaceFilesKeys.records())
3132
invalidate(qc, id ? workspaceFilesKeys.contentFile(wId, id) : workspaceFilesKeys.contents())
3233
invalidate(qc, workspaceFilesKeys.storageInfo())
3334
},

‎apps/sim/app/workspace/[workspaceId]/home/hooks/stream/handle-resource-event.test.ts‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ describe('handleResourceEvent removal', () => {
110110
})
111111

112112
it.each(['workflow', 'table', 'file', 'knowledgebase', 'log'] as const)(
113-
'opens an authorized %s read without invalidating or reconciling editable content',
113+
'ignores an authorized %s read without opening, saving or changing focus',
114114
(type) => {
115115
const onResourceEvent = vi.fn()
116116
const deps = makeStreamLoopDeps({ onResourceEventRef: { current: onResourceEvent } })
@@ -124,12 +124,13 @@ describe('handleResourceEvent removal', () => {
124124
resource: { type, id: 'addressed', title: 'Addressed resource' },
125125
},
126126
})
127-
expect(onResourceEvent).toHaveBeenCalledWith('addressed')
128-
expect(deps.setResources).toHaveBeenCalled()
127+
expect(onResourceEvent).not.toHaveBeenCalled()
128+
expect(deps.setResources).not.toHaveBeenCalled()
129129
expect(deps.addResource).not.toHaveBeenCalled()
130130
expect(mocks.invalidateResourceQueries).not.toHaveBeenCalled()
131131
expect(mocks.notifyWorkflowExternalUpdate).not.toHaveBeenCalled()
132132
expect(deps.ensureWorkflowInRegistry).not.toHaveBeenCalled()
133+
expect(deps.queryClient.invalidateQueries).not.toHaveBeenCalled()
133134
}
134135
)
135136

@@ -152,13 +153,12 @@ describe('handleResourceEvent removal', () => {
152153
expect(deps.setResources).not.toHaveBeenCalled()
153154
expect(mocks.invalidateResourceQueries).not.toHaveBeenCalled()
154155
expect(mocks.notifyWorkflowExternalUpdate).not.toHaveBeenCalled()
155-
expect(deps.queryClient.invalidateQueries).toHaveBeenCalledWith({
156-
queryKey: ['mothership-chats', 'detail', 'chat'],
157-
})
156+
expect(deps.queryClient.invalidateQueries).not.toHaveBeenCalled()
158157
})
159158

160-
it('still reconciles a committed workflow edit after opening it for a read', () => {
161-
const deps = makeStreamLoopDeps()
159+
it('still opens and reconciles a committed workflow edit after a read', () => {
160+
const onResourceEvent = vi.fn()
161+
const deps = makeStreamLoopDeps({ onResourceEventRef: { current: onResourceEvent } })
162162
const event = removeEvent('workflow', 'wf')
163163
handleResourceEvent(
164164
{ deps } as StreamLoopContext,
@@ -173,6 +173,8 @@ describe('handleResourceEvent removal', () => {
173173
)
174174
expect(mocks.invalidateResourceQueries).toHaveBeenCalledTimes(1)
175175
expect(mocks.notifyWorkflowExternalUpdate).toHaveBeenCalledExactlyOnceWith('wf')
176+
expect(deps.addResource).toHaveBeenCalledTimes(1)
177+
expect(onResourceEvent).toHaveBeenCalledExactlyOnceWith('wf')
176178
})
177179

178180
it.each([

‎apps/sim/app/workspace/[workspaceId]/home/hooks/stream/handle-resource-event.ts‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ export function handleResourceEvent(ctx: StreamLoopContext, parsed: ResourceEven
5757
} = ctx.deps
5858
const onResourceEvent = onResourceEventRef.current
5959
const payload = parsed.payload
60+
if (payload.op === 'upsert' && payload.readOnly) return
6061
if (payload.resource.type === 'settings') {
6162
const settings = payload.resource
6263
if (
@@ -142,10 +143,8 @@ export function handleResourceEvent(ctx: StreamLoopContext, parsed: ResourceEven
142143
// lists, never from the stream; older servers announced them as resources.
143144
if (payload.resource.type === 'browser' || payload.resource.type === 'terminal') return
144145
const resourceType = payload.resource.type
145-
const readOnly = payload.op === 'upsert' && payload.readOnly === true
146-
if (!readOnly)
147-
invalidateResourceQueries(queryClient, workspaceId, resourceType, payload.resource.id)
148-
if (!readOnly && resourceType === 'workflow' && payload.op !== 'remove' && payload.resource.id) {
146+
invalidateResourceQueries(queryClient, workspaceId, resourceType, payload.resource.id)
147+
if (resourceType === 'workflow' && payload.op !== 'remove' && payload.resource.id) {
149148
notifyWorkflowExternalUpdate(payload.resource.id)
150149
}
151150
if (payload.op === 'refresh') return
@@ -303,7 +302,7 @@ export function handleResourceEvent(ctx: StreamLoopContext, parsed: ResourceEven
303302
} else onResourceEvent?.(getChatResourceSelectionId(resource))
304303
}
305304

306-
if (resource.type === 'workflow' && !readOnly) {
305+
if (resource.type === 'workflow') {
307306
ensureWorkflowInRegistry(resource.id, resource.title, workspaceId)
308307
}
309308
}

‎apps/sim/hooks/queries/workspace-files.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,9 @@ export const workspaceFilesKeys = {
4444
workspaceLists: (workspaceId: string) => [...workspaceFilesKeys.lists(), workspaceId] as const,
4545
list: (workspaceId: string, scope: WorkspaceFileQueryScope = 'active') =>
4646
[...workspaceFilesKeys.workspaceLists(workspaceId), scope] as const,
47+
records: () => [...workspaceFilesKeys.all, 'record'] as const,
4748
record: (workspaceId: string, fileId: string) =>
48-
[...workspaceFilesKeys.all, 'record', workspaceId, fileId] as const,
49+
[...workspaceFilesKeys.records(), workspaceId, fileId] as const,
4950
contents: () => [...workspaceFilesKeys.all, 'content'] as const,
5051
contentFile: (workspaceId: string, fileId: string) =>
5152
[...workspaceFilesKeys.contents(), workspaceId, fileId] as const,

‎apps/sim/lib/mothership/request/go/stream.test.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -379,7 +379,7 @@ describe('copilot go stream helpers', () => {
379379
{ fail: false, readOnly: true },
380380
{ fail: true, readOnly: true },
381381
])(
382-
'commits worker resource effects before forwarding (failure: $fail, read only: $readOnly)',
382+
'persists mutations but never read-only panels (failure: $fail, read only: $readOnly)',
383383
async ({ fail, readOnly }) => {
384384
const resource = { type: 'workflow', id: 'wf', title: 'A workflow' }
385385
const order: string[] = []
@@ -428,8 +428,13 @@ describe('copilot go stream helpers', () => {
428428
},
429429
}
430430
)
431-
if (fail) await expect(promise).rejects.toThrow('Resource storage unavailable')
431+
if (fail && !readOnly) await expect(promise).rejects.toThrow('Resource storage unavailable')
432432
else await promise
433+
if (readOnly) {
434+
expect(order).toEqual(['publish'])
435+
expect(changeStoredChatResourcesMock).not.toHaveBeenCalled()
436+
return
437+
}
433438
expect(order).toEqual(fail ? ['persist'] : ['persist', 'publish'])
434439
expect(changeStoredChatResourcesMock).toHaveBeenCalledWith(
435440
'chat',

‎apps/sim/lib/mothership/resources/extraction.test.ts‎

Lines changed: 26 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -5,31 +5,34 @@ import { describe, expect, it } from 'vitest'
55
import { extractDeletedResourcesFromToolResult, extractResourcesFromToolResult } from './extraction'
66

77
describe('extractResourcesFromToolResult', () => {
8-
it('extracts file resources from create_empty_file results', () => {
9-
const resources = extractResourcesFromToolResult(
10-
'create_empty_file',
11-
{
12-
fileName: 'notes.md',
13-
},
14-
{
15-
success: true,
16-
message: 'File "notes.md" created successfully',
17-
data: {
18-
id: 'file_123',
19-
name: 'notes.md',
20-
contentType: 'text/markdown',
8+
it.each(['create_empty_file', 'prepare_file_edit', 'apply_file_edit'])(
9+
'extracts committed file identity from %s results',
10+
(toolName) => {
11+
const resources = extractResourcesFromToolResult(
12+
toolName,
13+
{
14+
fileName: 'notes.md',
2115
},
22-
}
23-
)
16+
{
17+
success: true,
18+
message: 'File "notes.md" created successfully',
19+
data: {
20+
id: 'file_123',
21+
name: 'notes.md',
22+
contentType: 'text/markdown',
23+
},
24+
}
25+
)
2426

25-
expect(resources).toEqual([
26-
{
27-
type: 'file',
28-
id: 'file_123',
29-
title: 'notes.md',
30-
},
31-
])
32-
})
27+
expect(resources).toEqual([
28+
{
29+
type: 'file',
30+
id: 'file_123',
31+
title: 'notes.md',
32+
},
33+
])
34+
}
35+
)
3336

3437
it('uses the knowledge base id for manage_knowledge_base tag mutations', () => {
3538
const resources = extractResourcesFromToolResult(

‎apps/sim/lib/mothership/resources/extraction.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { toRecord } from '@sim/utils/object'
22
import {
3+
ApplyFileEdit,
34
CreateEmptyFile,
45
CreateWorkflow,
56
DownloadFile,
@@ -26,6 +27,7 @@ const RESOURCE_TOOL_NAMES: Set<string> = new Set([
2627
TableViews.id,
2728
CreateEmptyFile.id,
2829
PrepareFileEdit.id,
30+
ApplyFileEdit.id,
2931
DownloadFile.id,
3032
CreateWorkflow.id,
3133
EditWorkflow.id,
@@ -111,6 +113,7 @@ export function extractResourcesFromToolResult(
111113
}
112114

113115
case CreateEmptyFile.id:
116+
case ApplyFileEdit.id:
114117
case PrepareFileEdit.id: {
115118
const file = toRecord(data.file)
116119
if (file.id) {

‎apps/sim/lib/mothership/resources/persist-effect.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,12 @@ export async function persistResourceEffect(
77
chatId: string,
88
payload: ResourcePayload
99
): Promise<void> {
10-
if (!payload.effectId || payload.op === 'refresh') return
10+
if (
11+
!payload.effectId ||
12+
payload.op === 'refresh' ||
13+
(payload.op === 'upsert' && payload.readOnly)
14+
)
15+
return
1116
if (payload.op === 'clear_view') {
1217
await changeStoredChatResources(
1318
chatId,

‎apps/sim/lib/mothership/tools/server/files/file-edit-roundtrip.test.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ vi.mock('@/lib/workspace-files/application/update-workspace-file-content', () =>
5151
updateWorkspaceFileContent: { execute: vi.fn() },
5252
}))
5353

54+
import { extractResourcesFromToolResult } from '@/lib/mothership/resources/extraction'
5455
import { editContentServerTool } from '@/lib/mothership/tools/server/files/edit-content'
5556
import { consumeLatestFileIntent } from '@/lib/mothership/tools/server/files/file-intent-store'
5657
import { workspaceFileServerTool } from '@/lib/mothership/tools/server/files/workspace-file'
@@ -102,6 +103,9 @@ describe('prepared file write across tool invocations', () => {
102103
const applyContext = { ...prepareContext, toolCallId: 'apply' }
103104
const applied = await editContentServerTool.execute({ content }, applyContext)
104105
expect(applied.success, applied.message).toBe(true)
106+
expect(extractResourcesFromToolResult('apply_file_edit', { content }, applied)).toEqual([
107+
{ type: 'file', id: created.id, title: created.name },
108+
])
105109
expect(executeFileUseCase).toHaveBeenNthCalledWith(
106110
1,
107111
prepareContext,

0 commit comments

Comments
 (0)