Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions apps/desktop/e2e/local-files.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ test('native file tools read and import through the installed preload without Si
const png =
'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAusB9Y9Zl1sAAAAASUVORK5CYII='
writeFileSync(join(source, 'image.png'), Buffer.from(png, 'base64'))
let claimed = false
/** Calls the server saw claimed; like the server, only an import refuses a second claim. */
const claimed = new Set<string>()
const calls: Record<string, { toolName: string; args: Record<string, unknown> }> = {
text: { toolName: 'read_local_file', args: { path: join(source, 'report.txt') } },
image: { toolName: 'read_local_file', args: { path: join(source, 'image.png') } },
Expand Down Expand Up @@ -44,11 +45,14 @@ test('native file tools read and import through the installed preload without Si
for await (const chunk of request) body += chunk.toString()
const input = JSON.parse(body)
const call = calls[input.toolCallId]
if (!call || (input.claim && claimed)) {
if (
!call ||
(input.claim && call.toolName === 'import_local_files' && claimed.has(input.toolCallId))
) {
response.writeHead(call ? 409 : 403, { 'Content-Type': 'application/json' }).end('{}')
return
}
if (input.claim) claimed = true
if (input.claim) claimed.add(input.toolCallId)
response
.writeHead(200, { 'Content-Type': 'application/json' })
.end(JSON.stringify({ ...call, chatId: 'org-chat' }))
Expand Down Expand Up @@ -92,6 +96,7 @@ test('native file tools read and import through the installed preload without Si
ok: true,
data: { observations: [{ mediaType: 'image/png', data: png }] },
})
expect([...claimed]).toEqual(['text', 'image'])
const result = await invoke({ operation: 'manifest', toolCallId: 'import' })
if (!result.ok || result.data.kind !== 'manifest') throw new Error(JSON.stringify(result))
expect(result.data.targetWorkspaceId).toBe('target-workspace')
Expand Down
4 changes: 2 additions & 2 deletions apps/desktop/src/main/ipc.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,7 @@ describe('registerIpcHandlers', () => {
expect(mounts).not.toHaveBeenCalled()
expect(fetchAuthorization).toHaveBeenCalledWith(
`${APP}/api/desktop/tool/authorize`,
expect.objectContaining({ body: JSON.stringify({ toolCallId: 'tool-native' }) })
expect.objectContaining({ body: JSON.stringify({ toolCallId: 'tool-native', claim: true }) })
Comment thread
waleedlatif1 marked this conversation as resolved.
)
expect(
await handler?.(evilEvent, { operation: 'read', toolCallId: 'tool-native' })
Expand Down Expand Up @@ -569,7 +569,7 @@ describe('registerIpcHandlers', () => {
expect.objectContaining({
method: 'POST',
credentials: 'include',
body: JSON.stringify({ toolCallId: 'tool-1' }),
body: JSON.stringify({ toolCallId: 'tool-1', claim: true }),
})
)
})
Expand Down
7 changes: 5 additions & 2 deletions apps/desktop/src/main/ipc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -578,10 +578,13 @@ async function authorizeLocalFilesystemTool(
request: unknown
): Promise<boolean> {
if (typeof request !== 'object' || request === null) return false
// Claimed like an import: the server sees the read picked up, and refuses one it already
// failed as never started.
const authorization = await fetchDesktopToolAuthorization(
event,
deps,
(request as { requestId?: unknown }).requestId
(request as { requestId?: unknown }).requestId,
true
)
return authorization
? deps.localFilesystem.isAuthorizedClientToolRequest(request, authorization)
Expand Down Expand Up @@ -2113,7 +2116,7 @@ export function registerIpcHandlers(deps: IpcDeps): void {
event,
deps,
request.toolCallId,
request.operation === 'manifest',
request.operation === 'manifest' || request.operation === 'read',
(status) => {
failureStatus = status
}
Expand Down
15 changes: 3 additions & 12 deletions apps/desktop/src/main/terminal/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,10 @@ import { homedir } from 'node:os'
import type { TerminalShortcutCommand } from '@sim/desktop-bridge'
import { createLogger } from '@sim/logger'
import {
DEFAULT_RUN_WAIT_MS,
isTerminalControlKey,
MAX_INPUT_KEYS,
MAX_RUN_WAIT_MS,
MAX_TOOL_OUTPUT_CHARS,
resolveRunWaitMs,
type TerminalCommandEvent,
type TerminalControlKey,
type TerminalCwdResult,
Expand Down Expand Up @@ -109,14 +108,6 @@ const HANDOFF_MAX_MS = 12 * 60 * 60 * 1000
*/
const HANDOFF_SETTLE_MS = 5_000

/** How long to hold the turn before handing a still-running command back. */
function resolveWaitMs(waitSeconds: number | undefined): number {
const requested = Number(waitSeconds)
return Number.isFinite(requested) && requested > 0
? Math.min(requested * 1000, MAX_RUN_WAIT_MS)
: DEFAULT_RUN_WAIT_MS
}

function elideOutput(value: string): { text: string; truncated: boolean } {
return elide(value, MAX_TOOL_OUTPUT_CHARS)
}
Expand Down Expand Up @@ -1097,7 +1088,7 @@ export class TerminalService {
const handle = await startRun(session, command, terminal.currentCwd, terminal.env)
if ('error' in handle) throw new TerminalError('SPAWN_FAILED', handle.error)

const waitMs = resolveWaitMs(args.waitSeconds)
const waitMs = resolveRunWaitMs(args.waitSeconds)
const outcome = await awaitRun(handle, waitMs)
if (outcome.done) {
await closeRunWindow(handle, terminal.env)
Expand Down Expand Up @@ -1149,7 +1140,7 @@ export class TerminalService {
)
}

return session.runCommand(command, toolCallId, resolveWaitMs(args.waitSeconds))
return session.runCommand(command, toolCallId, resolveRunWaitMs(args.waitSeconds))
}

private spawn(
Expand Down
1 change: 1 addition & 0 deletions apps/desktop/src/preload/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ const api: SimDesktopApi = {
ipcRenderer.invoke('desktop:local-filesystem', request),
localFiles: (request: DesktopLocalFileRequest): Promise<DesktopLocalFileResponse> =>
ipcRenderer.invoke('desktop:local-files', request),
localReadClaims: true,
onCommand: (callback: (command: DesktopCommand) => void): (() => void) => {
const listener = (_event: unknown, command: DesktopCommand) => callback(command)
ipcRenderer.on('desktop:command', listener)
Expand Down
30 changes: 18 additions & 12 deletions apps/sim/app/api/copilot/confirm/route.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import type { Span } from '@opentelemetry/api'
import { isBrowserToolName } from '@sim/browser-protocol'
import { createLogger } from '@sim/logger'
import { isTerminalToolName } from '@sim/terminal-protocol'
import { getErrorMessage, toError } from '@sim/utils/errors'
import { isPlainRecord } from '@sim/utils/object'
import { type NextRequest, NextResponse } from 'next/server'
Expand All @@ -14,6 +12,7 @@ import {
type AsyncCompletionData,
type AsyncConfirmationStatus,
type AsyncTerminalStatus,
getTerminalConfirmationStatus,
isDeliveredAsyncStatus,
isTerminalAsyncStatus,
isWorkflowToolExecutionClaimable,
Expand Down Expand Up @@ -41,12 +40,11 @@ import {
import { withIncomingGoSpan } from '@/lib/mothership/request/otel'
import { sealClientToolSettlement } from '@/lib/mothership/request/tools/client-completion-seal.server'
import { isWorkflowToolName } from '@/lib/mothership/tools/client-executed-tools'
import { getDesktopToolClaimOwner } from '@/lib/mothership/tools/desktop-tools'
import { getDesktopToolClaimOwner, isNativeDesktopTool } from '@/lib/mothership/tools/desktop-tools'
import {
createStructuralWorkflowToolCompletionData,
getWorkflowToolCompletionExecutionId,
getWorkflowToolCompletionMessage,
getWorkflowToolConfirmationStatus,
getWorkflowToolLaunchError,
resolveWorkflowToolTargetId,
WORKFLOW_EXECUTION_BUSY,
Expand Down Expand Up @@ -92,7 +90,7 @@ function acknowledgeSettledToolCall(
toolCallId: string,
storedStatus: AsyncTerminalStatus
): NextResponse {
const settledStatus = getWorkflowToolConfirmationStatus(storedStatus)
const settledStatus = getTerminalConfirmationStatus(storedStatus)
span.setAttributes({
[TraceAttr.ToolConfirmationStatus]: settledStatus,
[TraceAttr.CopilotConfirmOutcome]: CopilotConfirmOutcome.Delivered,
Expand Down Expand Up @@ -264,7 +262,7 @@ export const POST = withRouteHandler((req: NextRequest) => {
return createNotFoundResponse('Completed workflow execution not found')
}

const terminalStatus = getWorkflowToolConfirmationStatus(existing.status)
const terminalStatus = getTerminalConfirmationStatus(existing.status)
span.setAttributes({
[TraceAttr.ToolConfirmationStatus]: terminalStatus,
[TraceAttr.CopilotConfirmOutcome]: CopilotConfirmOutcome.Delivered,
Expand Down Expand Up @@ -308,10 +306,7 @@ export const POST = withRouteHandler((req: NextRequest) => {
const isErrorOrCancelledOutcome =
status === ASYNC_TOOL_CONFIRMATION_STATUS.error ||
status === ASYNC_TOOL_CONFIRMATION_STATUS.cancelled
const isNativeClientTool =
isBrowserToolName(existing.toolName) ||
isTerminalToolName(existing.toolName) ||
existing.toolName === 'import_local_files'
const isNativeClientTool = isNativeDesktopTool(existing.toolName)
const nativeClaimOwner = getDesktopToolClaimOwner(existing.toolName)
const isPreclaimNativeTerminalOutcome =
nativeClaimOwner !== undefined &&
Expand All @@ -330,6 +325,17 @@ export const POST = withRouteHandler((req: NextRequest) => {
span.setAttribute(TraceAttr.CopilotConfirmOutcome, CopilotConfirmOutcome.ToolCallNotFound)
return createNotFoundResponse('Running client tool call not found')
}
// A reporter that says the call never started (a stale replay, a closed view) cannot speak
// for a call the desktop claimed: only the claim's own result may settle it.
if (
isNativeClientTool &&
isPlainRecord(data) &&
data.notStarted === true &&
existing.status !== ASYNC_TOOL_STATUS.pending
) {
span.setAttribute(TraceAttr.CopilotConfirmOutcome, CopilotConfirmOutcome.ToolCallNotFound)
return createNotFoundResponse('Pending client tool call not found')
}

let effectiveStatus = status
let executionId = submittedExecutionId
Expand Down Expand Up @@ -365,7 +371,7 @@ export const POST = withRouteHandler((req: NextRequest) => {
executionId = claimedExecutionId
if (status !== ASYNC_TOOL_CONFIRMATION_STATUS.background) {
if (trustedExecution) {
effectiveStatus = getWorkflowToolConfirmationStatus(trustedExecution.status)
effectiveStatus = getTerminalConfirmationStatus(trustedExecution.status)
} else if (!isErrorOrCancelledOutcome) {
span.setAttribute(
TraceAttr.CopilotConfirmOutcome,
Expand All @@ -378,7 +384,7 @@ export const POST = withRouteHandler((req: NextRequest) => {
executionId = submittedExecutionId
} else if (trustedExecution) {
executionId = trustedExecution.executionId
effectiveStatus = getWorkflowToolConfirmationStatus(trustedExecution.status)
effectiveStatus = getTerminalConfirmationStatus(trustedExecution.status)
} else if (!isErrorOrCancelledOutcome) {
effectiveStatus = ASYNC_TOOL_CONFIRMATION_STATUS.error
executionId = undefined
Expand Down
6 changes: 3 additions & 3 deletions apps/sim/app/api/desktop/tool/authorize/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ vi.mock('@/lib/mothership/async-runs/repository', () => mothershipAsyncRunsMock)

import { POST } from './route'

const claimToolExecution = mothershipAsyncRunsMockFns.mockClaimToolExecution
const claimDesktopToolCall = mothershipAsyncRunsMockFns.mockClaimDesktopToolCall
const getAsyncToolCall = mothershipAsyncRunsMockFns.mockGetAsyncToolCall
const getRunSegment = mothershipAsyncRunsMockFns.mockGetRunSegment
const resolveInvocationWorkspace = mothershipWorkspaceTargetMockFns.mockResolveInvocationWorkspace
Expand Down Expand Up @@ -50,7 +50,7 @@ describe('desktop tool authorization', () => {
userId: 'user-1',
status: 'active',
})
claimToolExecution.mockResolvedValue({ outcome: 'claimed' })
claimDesktopToolCall.mockResolvedValue({ outcome: 'claimed' })
})

it('never returns presentation activity as an executable browser argument', async () => {
Expand Down Expand Up @@ -193,7 +193,7 @@ describe('desktop tool authorization', () => {
new OrchestrationError('not_found', 'Workspace not found')
)
expect((await POST(request('import-1', true))).status).toBe(404)
claimToolExecution.mockResolvedValueOnce({ outcome: 'existing' })
claimDesktopToolCall.mockResolvedValueOnce({ outcome: 'existing' })
expect((await POST(request('import-1', true))).status).toBe(409)
})

Expand Down
56 changes: 34 additions & 22 deletions apps/sim/app/api/desktop/tool/authorize/route.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import { isCurrentBrowserToolName } from '@sim/browser-protocol'
import { isTerminalToolName } from '@sim/terminal-protocol'
import { isRecordLike, omit } from '@sim/utils/object'
import { type NextRequest, NextResponse } from 'next/server'
import { authorizeDesktopToolContract } from '@/lib/api/contracts/desktop-tool-authorization'
Expand All @@ -9,17 +7,21 @@ import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
import { resolveInvocationWorkspace } from '@/lib/mothership/application/workspace-target'
import { DESKTOP_TOOL_CLAIM_OWNER } from '@/lib/mothership/async-runs/lifecycle'
import {
claimToolExecution,
claimDesktopToolCall,
type DesktopToolCallClaim,
getAsyncToolCall,
getRunSegment,
type ToolExecutionClaim,
} from '@/lib/mothership/async-runs/repository'
import {
authenticateCopilotRequestSessionOnly,
createNotFoundResponse,
createUnauthorizedResponse,
} from '@/lib/mothership/request/http'
import { isUserLocalVfsToolCall } from '@/lib/mothership/tools/local-filesystem'
import {
getDesktopToolClaimOwner,
isDesktopToolCall,
isLocalReadToolCall,
} from '@/lib/mothership/tools/desktop-tools'

const admissionClosedResponse = () =>
NextResponse.json(
Expand All @@ -29,7 +31,7 @@ const admissionClosedResponse = () =>

/** A refused claim answers the same way for every tool, except how each reports a lost race. */
function refusedClaimResponse(
claim: Exclude<ToolExecutionClaim['outcome'], 'claimed'>,
claim: Exclude<DesktopToolCallClaim['outcome'], 'claimed'>,
notPending: () => NextResponse
): NextResponse {
if (claim === 'closed') return admissionClosedResponse()
Expand Down Expand Up @@ -74,16 +76,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
}

const args = isRecordLike(toolCall.args) ? (toolCall.args as Record<string, unknown>) : {}
const isBrowserTool = isCurrentBrowserToolName(toolCall.toolName)
const isTerminalTool = isTerminalToolName(toolCall.toolName)
const isLocalFileTool =
toolCall.toolName === 'read_local_file' || toolCall.toolName === 'import_local_files'
const authorized =
isBrowserTool ||
isTerminalTool ||
isLocalFileTool ||
isUserLocalVfsToolCall(toolCall.toolName, args)
if (!authorized) {
if (!isDesktopToolCall(toolCall.toolName, args)) {
return NextResponse.json(
{ error: 'Tool call is not authorized for desktop execution' },
{ status: 403 }
Expand Down Expand Up @@ -115,7 +108,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
{ status: 409 }
)
if (toolCall.status !== 'pending') return alreadyStarted()
const { outcome } = await claimToolExecution({
const { outcome } = await claimDesktopToolCall({
toolCallId: toolCall.toolCallId,
runId: toolCall.runId,
userId,
Expand All @@ -129,20 +122,39 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
return createNotFoundResponse('The import must be started before reading file bytes')
}

// A desktop that claims local reads claims each one before its first read; its later reads of
// the same call ride on that claim. A call persisted running (an older desktop's turn) is read
// as before.
if (parsed.data.body.claim && isLocalReadToolCall(toolCall.toolName, args)) {
const notPending = () => createNotFoundResponse('Pending client tool call not found')
if (toolCall.status === 'pending') {
const { outcome } = await claimDesktopToolCall({
toolCallId: toolCall.toolCallId,
runId: toolCall.runId,
userId,
claimedBy: DESKTOP_TOOL_CLAIM_OWNER.files,
})
if (outcome !== 'claimed') return refusedClaimResponse(outcome, notPending)
} else if (toolCall.claimedBy !== null && toolCall.claimedBy !== DESKTOP_TOOL_CLAIM_OWNER.files)
return notPending()
}

// Browser and terminal actions are one-shot side effects on the user's own
// machine, so the pending call is claimed here, atomically, before crossing
// the Electron boundary — a replayed renderer event must not run a command
// or click a button twice.
if (isBrowserTool || isTerminalTool) {
const actionClaimOwner = getDesktopToolClaimOwner(toolCall.toolName)
if (
actionClaimOwner === DESKTOP_TOOL_CLAIM_OWNER.browser ||
actionClaimOwner === DESKTOP_TOOL_CLAIM_OWNER.terminal
) {
const notPending = () => createNotFoundResponse('Pending client tool call not found')
if (toolCall.status !== 'pending') return notPending()
const { outcome } = await claimToolExecution({
const { outcome } = await claimDesktopToolCall({
toolCallId: toolCall.toolCallId,
runId: toolCall.runId,
userId,
claimedBy: isBrowserTool
? DESKTOP_TOOL_CLAIM_OWNER.browser
: DESKTOP_TOOL_CLAIM_OWNER.terminal,
claimedBy: actionClaimOwner,
})
if (outcome !== 'claimed') return refusedClaimResponse(outcome, notPending)
}
Expand Down
Loading
Loading