Skip to content

Commit ce4ee7d

Browse files
icecrasher321icecrasher321
committed
fix(mothership): preserve trace filters and shared app workers
1 parent 65a9a0c commit ce4ee7d

13 files changed

Lines changed: 132 additions & 34 deletions

File tree

‎apps/desktop/src/main/browser-agent/registry.ts‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { Session, WebContents } from 'electron'
2+
import { isAppOrigin } from '@/main/navigation'
23

34
/**
45
* Registry of WebContents that belong to the agent browser (the browser-agent
@@ -10,7 +11,7 @@ import type { Session, WebContents } from 'electron'
1011
* navigation time, so the post-construction registration races nothing.
1112
*/
1213
const agentContents = new WeakSet<WebContents>()
13-
const agentSessions = new WeakSet<Session>()
14+
const agentSessions = new WeakMap<Session, string | undefined>()
1415
const appOrigins = new WeakMap<WebContents, string>()
1516
const navigations = new WeakMap<WebContents, (url: string, method: string) => boolean>()
1617
const permissions = new WeakMap<WebContents, BrowserPermissionHandlers>()
@@ -26,14 +27,19 @@ export function registerAgentWebContents(
2627
handlers?: BrowserPermissionHandlers
2728
): void {
2829
agentContents.add(contents)
29-
agentSessions.add(contents.session)
30+
agentSessions.set(contents.session, appOrigin)
3031
if (appOrigin) appOrigins.set(contents, appOrigin)
3132
if (handlers) permissions.set(contents, handlers)
3233
}
3334

34-
/** Worker requests can outlive their originating tab and omit webContents. */
35-
export function hasAgentSession(session: Session): boolean {
36-
return agentSessions.has(session)
35+
/**
36+
* Unattributed workers retain network guards, but a shared session's configured
37+
* app origin remains reachable for ordinary app workers on self-hosted networks.
38+
*/
39+
export function shouldGuardUnownedAgentRequest(session: Session, url: string): boolean {
40+
if (!agentSessions.has(session)) return false
41+
const appOrigin = agentSessions.get(session)
42+
return !appOrigin || !isAppOrigin(url, appOrigin)
3743
}
3844

3945
export function isAgentWebContents(contents: WebContents): boolean {

‎apps/desktop/src/main/telemetry-policy.test.ts‎

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,12 +63,47 @@ describe('attachTelemetryPolicy', () => {
6363
expect(requestPolicy.handleBrowserRequest).toHaveBeenCalledExactlyOnceWith(request, callback)
6464
expect(callback).not.toHaveBeenCalled()
6565
requestPolicy.handleBrowserRequest.mockClear()
66-
const workerRequest = { ...request, webContents: undefined, resourceType: 'other' as const }
66+
const workerRequest = {
67+
...request,
68+
url: 'http://169.254.169.254/metadata',
69+
webContents: undefined,
70+
resourceType: 'other' as const,
71+
}
6772
listener(workerRequest, callback)
6873
expect(requestPolicy.handleBrowserRequest).toHaveBeenCalledExactlyOnceWith(
6974
workerRequest,
7075
callback
7176
)
7277
expect(callback).not.toHaveBeenCalled()
7378
})
79+
80+
it('preserves ordinary workers on the exact configured LAN app origin', () => {
81+
const contents = new WebContentsView().webContents
82+
const onBeforeRequest = vi.mocked(contents.session.webRequest.onBeforeRequest)
83+
onBeforeRequest.mockClear()
84+
requestPolicy.handleBrowserRequest.mockClear()
85+
registerAgentWebContents(contents, 'http://192.168.1.10:3000')
86+
attachTelemetryPolicy(contents.session, false)
87+
const listener = onBeforeRequest.mock.calls[0][0]
88+
if (typeof listener !== 'function') throw new Error('Missing request policy')
89+
const request: OnBeforeRequestListenerDetails = {
90+
id: 1,
91+
url: 'http://192.168.1.10:3000/editor.worker.js',
92+
method: 'GET',
93+
resourceType: 'script',
94+
referrer: '',
95+
timestamp: 0,
96+
uploadData: [],
97+
}
98+
const callback = vi.fn()
99+
listener(request, callback)
100+
expect(callback).toHaveBeenCalledExactlyOnceWith({ cancel: false })
101+
expect(requestPolicy.handleBrowserRequest).not.toHaveBeenCalled()
102+
103+
for (const url of ['http://192.168.1.11/data', 'http://192.168.1.10:4000/data']) {
104+
const otherRequest = { ...request, url }
105+
listener(otherRequest, callback)
106+
expect(requestPolicy.handleBrowserRequest).toHaveBeenCalledWith(otherRequest, callback)
107+
}
108+
})
74109
})

‎apps/desktop/src/main/telemetry-policy.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { createLogger } from '@sim/logger'
22
import type { Session } from 'electron'
3-
import { hasAgentSession, isAgentWebContents } from '@/main/browser-agent/registry'
3+
import { isAgentWebContents, shouldGuardUnownedAgentRequest } from '@/main/browser-agent/registry'
44
import { handleBrowserRequest } from '@/main/browser-agent/request-policy'
55
import { matchesHostList } from '@/main/navigation'
66

@@ -41,7 +41,9 @@ export function attachTelemetryPolicy(session: Session, enabled: boolean): void
4141
if (enabled && shouldBlockRequest(details.url)) {
4242
callback({ cancel: true })
4343
} else if (
44-
details.webContents ? isAgentWebContents(details.webContents) : hasAgentSession(session)
44+
details.webContents
45+
? isAgentWebContents(details.webContents)
46+
: shouldGuardUnownedAgentRequest(session, details.url)
4547
) {
4648
handleBrowserRequest(details, callback)
4749
} else {

‎apps/sim/app/api/custom-blocks/[id]/usages/route.test.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
*/
44
import { authMockFns, createMockRequest } from '@sim/testing'
55
import { beforeEach, describe, expect, it, vi } from 'vitest'
6+
import { OrchestrationError } from '@/lib/core/orchestration/types'
67

78
const { mockHasWorkspaceAdminAccess, mockOperations } = vi.hoisted(() => ({
89
mockHasWorkspaceAdminAccess: vi.fn(),
@@ -104,4 +105,13 @@ describe('GET /api/custom-blocks/[id]/usages', () => {
104105
'custom_block_abc123'
105106
)
106107
})
108+
109+
it('conceals internal orchestration diagnostics', async () => {
110+
mockOperations.getCustomBlockUsageCounts.mockRejectedValue(
111+
new OrchestrationError('internal', 'Database driver diagnostic')
112+
)
113+
const response = await callRoute()
114+
expect(response.status).toBe(500)
115+
expect(await response.json()).toEqual({ error: 'Internal server error' })
116+
})
107117
})

‎apps/sim/app/api/custom-blocks/errors.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
import {
22
createInternalResourceConcealmentPolicy,
33
internalErrorResponse,
4+
internalOrchestrationErrorPolicy,
45
} from '@/lib/api/server/routes'
56
import { InternalUnauthenticatedError } from '@/lib/api/server/routes/internal-json-route'
67
import {
78
InsufficientWorkspacePermissionsError,
89
NoWorkspaceAccessError,
910
} from '@/lib/core/application/workspace-authorization'
10-
import { OrchestrationError, statusForOrchestrationError } from '@/lib/core/orchestration/types'
11+
import { OrchestrationError } from '@/lib/core/orchestration/types'
1112
import { CustomBlockValidationError } from '@/lib/workflows/custom-blocks/operations'
1213

1314
export function customBlockError(error: unknown, read = false) {
@@ -22,8 +23,7 @@ export function customBlockError(error: unknown, read = false) {
2223
})
2324
if (error instanceof CustomBlockValidationError)
2425
return internalErrorResponse(400, { error: error.message })
25-
if (error instanceof OrchestrationError)
26-
return internalErrorResponse(statusForOrchestrationError(error.code), { error: error.message })
26+
if (error instanceof OrchestrationError) return internalOrchestrationErrorPolicy.project(error)
2727
return null
2828
}
2929

‎apps/sim/app/o/[organizationId]/components/organization-sidebar/organization-sidebar.tsx‎

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
'use client'
22

3-
import { type ComponentProps, memo, useCallback, useRef, useState } from 'react'
3+
import { type ComponentProps, memo, useRef, useState } from 'react'
44
import { Chip, cn, scrollFadeAttributes, scrollFadeClass, useScrollEdges } from '@sim/emcn'
55
import { PanelLeft } from '@sim/emcn/icons'
66
import { createLogger } from '@sim/logger'
@@ -92,11 +92,8 @@ export const OrganizationSidebar = memo(function OrganizationSidebar() {
9292
})
9393

9494
const isMac = isMacPlatform()
95-
const navItems = buildOrganizationNavItems(
96-
organization.id,
97-
searchAccess.memberScoped,
98-
mothershipAvailable && (canBuild || searchAccess.memberScoped)
99-
)
95+
const canUseHome = mothershipAvailable && (canBuild || searchAccess.memberScoped)
96+
const navItems = buildOrganizationNavItems(organization.id, searchAccess.memberScoped, canUseHome)
10097
const settingsPath = organizationRoutes(organization.id).settings
10198
const isSettings = pathname === settingsPath || pathname?.startsWith(`${settingsPath}/`)
10299

@@ -110,13 +107,10 @@ export const OrganizationSidebar = memo(function OrganizationSidebar() {
110107
closeMenu: closeHrefMenu,
111108
} = useContextMenu()
112109

113-
const handleHrefContextMenu = useCallback(
114-
(e: React.MouseEvent, href: string) => {
115-
setMenuHref(href)
116-
openHrefMenu(e)
117-
},
118-
[openHrefMenu]
119-
)
110+
const handleHrefContextMenu = (e: React.MouseEvent, href: string) => {
111+
setMenuHref(href)
112+
openHrefMenu(e)
113+
}
120114

121115
const handleHrefMenuClose = () => {
122116
closeHrefMenu()
@@ -272,7 +266,7 @@ export const OrganizationSidebar = memo(function OrganizationSidebar() {
272266
isCollapsed={isCollapsed}
273267
pathname={pathname}
274268
/>
275-
{(mothershipAvailable || searchAccess.memberScoped) && (
269+
{canUseHome && (
276270
<OrganizationChats
277271
key={organization.id}
278272
organizationId={organization.id}

‎apps/sim/lib/logs/execution/logger.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ import {
4747
pickLatestStartedMarker,
4848
} from '@/lib/logs/execution/progress-markers'
4949
import { snapshotService } from '@/lib/logs/execution/snapshot/service'
50+
import { traceSpansHaveHandledErrors } from '@/lib/logs/execution/trace-spans/handled-errors'
5051
import { traceSpansIndicateFailure } from '@/lib/logs/execution/trace-spans/trace-spans'
5152
import {
5253
copyTraceSpansWithoutCosts,
@@ -498,6 +499,7 @@ export class ExecutionLogger implements IExecutionLoggerService {
498499
: {}),
499500
hasTraceSpans: executionData.hasTraceSpans,
500501
traceSpanCount: executionData.traceSpanCount,
502+
hasHandledErrors: executionData.hasHandledErrors,
501503
traceSpans: summarizeTraceSpansWithoutIo(executionData.traceSpans),
502504
finalOutput: summarizeValueForExecutionData(executionData.finalOutput, MAX_TRACE_IO_BYTES) as
503505
| BlockOutputData
@@ -526,6 +528,7 @@ export class ExecutionLogger implements IExecutionLoggerService {
526528
...(executionData.correlation ? { correlation: executionData.correlation } : {}),
527529
hasTraceSpans: executionData.hasTraceSpans,
528530
traceSpanCount: executionData.traceSpanCount,
531+
hasHandledErrors: executionData.hasHandledErrors,
529532
tokens: executionData.tokens,
530533
models: executionData.models,
531534
executionDataTruncated: true,
@@ -633,6 +636,7 @@ export class ExecutionLogger implements IExecutionLoggerService {
633636
...(finalizationPath ? { finalizationPath } : {}),
634637
hasTraceSpans: traceSpanCount > 0,
635638
traceSpanCount,
639+
hasHandledErrors: traceSpansHaveHandledErrors(traceSpans),
636640
traceSpans,
637641
finalOutput,
638642
tokens: {

‎apps/sim/lib/logs/execution/trace-spans/handled-errors.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
import { isRecordLike, toArray } from '@sim/utils/object'
2+
3+
/** Detects recovered errors in trace structure without inspecting user-provided span IO. */
4+
export function traceSpansHaveHandledErrors(spans: unknown): boolean {
5+
return toArray<unknown>(spans).some(
6+
(span) =>
7+
isRecordLike(span) &&
8+
((span.status === 'error' && span.errorHandled === true) ||
9+
traceSpansHaveHandledErrors(span.children))
10+
)
11+
}

‎apps/sim/lib/logs/execution/trace-store-storage.test.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ describe('trace archive storage round trip', () => {
6363
traceSpans: [{ id: 'span-1', output: 'é'.repeat(MAX_DURABLE_LARGE_VALUE_BYTES) }],
6464
traceSpanCount: 1,
6565
hasTraceSpans: true,
66+
hasHandledErrors: false,
6667
}
6768
const json = JSON.stringify(data)
6869
const size = Buffer.byteLength(json, 'utf8')
@@ -79,6 +80,7 @@ describe('trace archive storage round trip', () => {
7980
[TRACE_STORE_REF_KEY]: expect.objectContaining({ size, key: expect.any(String) }),
8081
traceSpanCount: 1,
8182
hasTraceSpans: true,
83+
hasHandledErrors: false,
8284
})
8385
expect(mockRegisterOwner).toHaveBeenCalledWith(
8486
expect.objectContaining({
@@ -128,7 +130,7 @@ describe('trace archive storage round trip', () => {
128130
},
129131
CONTEXT
130132
)
131-
).resolves.toEqual({ hasTraceSpans: false })
133+
).resolves.toEqual({ hasTraceSpans: false, hasHandledErrors: false })
132134
expect(mockDownloadFile).not.toHaveBeenCalled()
133135
})
134136
})

‎apps/sim/lib/logs/execution/trace-store.test.ts‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,15 +132,45 @@ describe('execution data storage', () => {
132132
correlation,
133133
hasTraceSpans: true,
134134
traceSpanCount: 2,
135+
hasHandledErrors: false,
135136
})
136137

137138
await expect(materializeExecutionData(slim, CONTEXT)).resolves.toEqual({
138139
secretProjectionVersion: SECRET_PROJECTION_VERSION,
139140
correlation,
140141
hasTraceSpans: true,
141142
traceSpanCount: 2,
143+
hasHandledErrors: false,
142144
})
143145
})
146+
147+
it.each([
148+
{ traceSpans: [{ children: [{ status: 'error', errorHandled: true }] }], expected: true },
149+
{ traceSpans: [{ status: 'error', errorHandled: false }], expected: false },
150+
{
151+
traceSpans: [{ status: 'success', output: { status: 'error', errorHandled: true } }],
152+
expected: false,
153+
},
154+
])(
155+
'retains handled-error classification without loading trace IO: $expected',
156+
async ({ traceSpans, expected }) => {
157+
storeExecutionTraceArchiveMock.mockResolvedValue({
158+
__simLargeValueRef: true,
159+
version: 1,
160+
id: 'lv_bbbbbbbbbbbb',
161+
kind: 'object',
162+
size: 128,
163+
key: 'execution/workspace-1/workflow-1/execution-1/large-value-lv_bbbbbbbbbbbb.json',
164+
executionId: 'execution-1',
165+
})
166+
materializeLargeValueRefMock.mockRejectedValue(new Error('object unavailable'))
167+
const slim = await externalizeExecutionData({ traceSpans }, CONTEXT)
168+
expect(slim.hasHandledErrors).toBe(expected)
169+
expect(slim).not.toHaveProperty('traceSpans')
170+
const metadata = await materializeExecutionData(slim, CONTEXT)
171+
expect(metadata.hasHandledErrors).toBe(expected)
172+
}
173+
)
144174
})
145175

146176
describe('projectExecutionDataForDisplay', () => {

0 commit comments

Comments
 (0)