fix(mothership): surface actionable workbench file read failures (#9048)

* fix(mothership): surface actionable workbench file read failures

* chore(lint): drop VfsPathError from unused-exports baseline

* fix(mothership): classify workbench staging failures apart from source reads

* fix(mothership): report a workbench socket path as not a regular file

Author: waleedlatif1

apps/sim/lib/execution/remote-sandbox/session-file-snapshot.test.ts

@@ -1,14 +1,23 @@
-import { execFile } from 'node:child_process'
-import { mkdtemp, readFile, realpath, rm, stat, symlink, writeFile } from 'node:fs/promises'
+import { spawnSync } from 'node:child_process'
+import {
+  chmod,
+  mkdir,
+  mkdtemp,
+  readFile,
+  realpath,
+  rm,
+  stat,
+  symlink,
+  writeFile,
+} from 'node:fs/promises'
+import { createServer } from 'node:net'
 import { tmpdir } from 'node:os'
 import { join } from 'node:path'
-import { promisify } from 'node:util'
 import {
   remoteSandboxProviderMock,
   remoteSandboxProviderMockFns,
 } from '@sim/testing/mocks/remote-sandbox-provider.mock'
-import { getErrorMessage } from '@sim/utils/errors'
-import { afterEach, beforeEach, expect, it, vi } from 'vitest'
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
 
 const mocks = vi.hoisted(() => ({
   find: vi.fn(),
@@ -27,13 +36,13 @@ vi.mock('@/lib/execution/remote-sandbox/session-lock', () => ({
 }))
 
 import { openSessionFileSnapshot } from '@/lib/execution/remote-sandbox/session-file-snapshot'
+import { fileReadFailure } from '@/lib/mothership/agent-cli/engines/file-view'
 
 remoteSandboxProviderMockFns.mockResolveProvider.mockImplementation(() => ({
   id: 'e2b',
   findSessionSandbox: mocks.find,
 }))
 
-const execute = promisify(execFile)
 let directory = ''
 let staged = ''
 beforeEach(async () => {
@@ -47,14 +56,11 @@ beforeEach(async () => {
     getFileSize: mocks.size,
   })
   mocks.command.mockImplementation(async (command, options) => {
-    try {
-      const result = await execute('/bin/sh', ['-c', command], {
-        env: { ...process.env, ...options.envs, SIM_UPLOAD_SNAPSHOT: staged },
-      })
-      return { ...result, exitCode: 0 }
-    } catch (error) {
-      return { exitCode: 1, stderr: getErrorMessage(error, 'failed') }
-    }
+    const result = spawnSync('/bin/sh', ['-c', command], {
+      encoding: 'utf8',
+      env: { ...process.env, ...options.envs, SIM_UPLOAD_SNAPSHOT: staged },
+    })
+    return { exitCode: result.status ?? 1, stdout: result.stdout, stderr: result.stderr }
   })
   mocks.size.mockImplementation(async () => (await stat(staged)).size)
   mocks.read.mockImplementation(async () => new Blob([await readFile(staged)]).stream())
@@ -83,7 +89,7 @@ it('rejects symlink escape before any bytes are streamed', async () => {
       allowedRoots: [directory],
       maxBytes: 100,
     })
-  ).rejects.toThrow('outside the permitted')
+  ).rejects.toThrow('outside the permitted workbench directories')
   expect(mocks.read).not.toHaveBeenCalled()
 })
 it('bounds the remote copy itself, not just the eventual response stream', async () => {
@@ -127,3 +133,107 @@ it('never creates a replacement machine for a missing read', async () => {
   )
   expect(mocks.command).not.toHaveBeenCalled()
 })
+
+describe('copy failures reach the model as their actionable cause', () => {
+  const snapshot = (path: string, maxBytes = 100) =>
+    openSessionFileSnapshot('chat', path, undefined, undefined, {
+      allowedRoots: [directory],
+      maxBytes,
+    })
+  const read = (path: string, maxBytes?: number) =>
+    snapshot(path, maxBytes).then(
+      () => {
+        throw new Error('expected the snapshot to fail')
+      },
+      (error: unknown) => fileReadFailure(error).stderr
+    )
+
+  it('names a missing file and the race with the step that writes it', async () => {
+    const path = join(directory, 'missing.csv')
+    expect(await read(path)).toBe(
+      `Error: No file exists at ${path} on this chat's workbench. Check the path, or wait for the step that writes it to finish.`
+    )
+  })
+  it('names a directory as not a regular file', async () => {
+    const path = join(directory, 'folder')
+    await mkdir(path)
+    expect(await read(path)).toContain(`${path} is not a regular file`)
+  })
+  /** Linux, the workbench OS, refuses to open a socket with ENXIO before any stat. */
+  it.skipIf(process.platform !== 'linux')('names a socket as not a regular file', async () => {
+    const path = join(directory, 'agent.sock')
+    const server = createServer()
+    await new Promise<void>((resolve) => server.listen(path, resolve))
+    try {
+      expect(await read(path)).toContain(`${path} is not a regular file`)
+    } finally {
+      await new Promise((resolve) => server.close(resolve))
+    }
+  })
+  it.skipIf(process.getuid?.() === 0)('names a permission failure', async () => {
+    const path = join(directory, 'locked.txt')
+    await writeFile(path, 'secret')
+    await chmod(path, 0o000)
+    expect(await read(path)).toBe(
+      `Error: The workbench user does not have permission to read ${path}.`
+    )
+  })
+  it('names the read size limit', async () => {
+    const path = join(directory, 'large.txt')
+    await writeFile(path, '123456')
+    expect(await read(path, 3)).toBe(`Error: ${path} exceeds the 3-byte size limit.`)
+  })
+  it('names a path outside the permitted roots', async () => {
+    const path = join(directory, 'escape')
+    await symlink('/etc/passwd', path)
+    expect(await read(path)).toBe(
+      `Error: ${path} resolves outside the permitted workbench directories.`
+    )
+  })
+  it.skipIf(process.getuid?.() === 0)(
+    'does not blame the source for a staging file it cannot create',
+    async () => {
+      const path = join(directory, 'readable.txt')
+      await writeFile(path, 'content')
+      const locked = join(directory, 'locked')
+      await mkdir(locked, { mode: 0o500 })
+      staged = join(locked, 'snapshot')
+      expect(await read(path)).toBe(
+        'Error: The file could not be read. Retry this read before making content claims.'
+      )
+      await chmod(locked, 0o700)
+    }
+  )
+  it('names a full workbench disk', async () => {
+    mocks.command.mockResolvedValueOnce({
+      exitCode: 3,
+      stdout: 'SIM_SNAPSHOT_FAILURE no_space OSError\n',
+      stderr: '',
+    })
+    expect(await read(join(directory, 'any.txt'))).toBe(
+      "Error: This chat's workbench is out of disk space. Delete files it no longer needs, then retry."
+    )
+  })
+  it('names a chat with no workbench', async () => {
+    mocks.find.mockResolvedValueOnce(null)
+    expect(await read('/tmp/out.csv')).toBe(
+      'Error: No workbench exists for this chat; write "/tmp/out.csv" first.'
+    )
+  })
+  it('keeps raw stderr out of an unclassified failure, reporting only exit code and class', async () => {
+    mocks.command.mockResolvedValueOnce({
+      exitCode: 3,
+      stdout: 'SIM_SNAPSHOT_FAILURE unknown MemoryError\n',
+      stderr: 'Traceback: /home/user/private-name.csv',
+    })
+    await expect(snapshot(join(directory, 'any.txt'))).rejects.toThrow(
+      /^Could not prepare the workbench upload file \(exit 3, MemoryError\)$/
+    )
+  })
+  it('gives the model the generic retry message for a copy killed without a marker', async () => {
+    mocks.command.mockResolvedValueOnce({ exitCode: 137, stdout: '', stderr: 'Killed' })
+    expect(await read(join(directory, 'any.txt'))).toBe(
+      'Error: The file could not be read. Retry this read before making content claims.'
+    )
+  })
+})

apps/sim/lib/execution/remote-sandbox/session-file-snapshot.ts

@@ -1,6 +1,7 @@
 import { createLogger } from '@sim/logger'
 import { generateShortId } from '@sim/utils/id'
 import type { EmbeddedFileSnapshot } from 'sim/embed'
+import { OrchestrationError } from '@/lib/core/orchestration/types'
 import { prepareSandboxSessionAccess } from '@/lib/execution/remote-sandbox/observer'
 import { resolveProvider } from '@/lib/execution/remote-sandbox/provider'
 import { SESSION_SANDBOX_IDLE_MS } from '@/lib/execution/remote-sandbox/session'
@@ -12,34 +13,102 @@ import { MAX_WORKSPACE_FILE_SIZE } from '@/lib/uploads/shared/types'
 const logger = createLogger('SessionFileSnapshot')
 const SNAPSHOT_TIMEOUT_MS = 10 * 60_000
 
-/** Copy once on the workbench; metadata and every upload part then refer to these exact bytes. */
+const FAILURE_MARKER = 'SIM_SNAPSHOT_FAILURE'
+
+/**
+ * Copy once on the workbench; metadata and every upload part then refer to these exact bytes.
+ * A failure prints one marker line naming a reason from a closed set (plus the exception class
+ * when unclassified) and never raw stderr, which can echo workbench paths and contents. Once
+ * staging begins, only a full disk is classified: an unwritable staging file says nothing about
+ * the requested source.
+ */
 const COPY_FILE = `python3 - <<'SIM_UPLOAD_SNAPSHOT'
-import os, stat, json
-source = os.path.realpath(os.environ['SIM_UPLOAD_SOURCE'])
-roots = json.loads(os.environ.get('SIM_UPLOAD_ALLOWED_ROOTS', '[]'))
-if roots and not any(source.startswith(root + '/') for root in roots):
-    raise ValueError('Scratch file resolves outside the permitted sandbox directories')
-target = os.environ['SIM_UPLOAD_SNAPSHOT']
-limit = int(os.environ['SIM_UPLOAD_MAX_BYTES'])
-fd = os.open(source, os.O_RDONLY | os.O_NONBLOCK)
-with os.fdopen(fd, 'rb') as reader:
-    info = os.fstat(reader.fileno())
+import errno, json, os, stat, sys
+def fail(reason, detail=''):
+    print('${FAILURE_MARKER} ' + reason + (' ' + detail if detail else ''))
+    sys.exit(3)
+SOURCE_REASONS = {errno.ENOENT: 'not_found', errno.ENOTDIR: 'not_found',
+                  errno.EISDIR: 'not_regular', errno.ENXIO: 'not_regular',
+                  errno.EACCES: 'permission', errno.EPERM: 'permission'}
+STAGING_REASONS = {errno.ENOSPC: 'no_space', errno.EDQUOT: 'no_space'}
+reasons = SOURCE_REASONS
+try:
+    source = os.path.realpath(os.environ['SIM_UPLOAD_SOURCE'])
+    roots = json.loads(os.environ.get('SIM_UPLOAD_ALLOWED_ROOTS', '[]'))
+    if roots and not any(source.startswith(root + '/') for root in roots):
+        fail('outside_roots')
+    target = os.environ['SIM_UPLOAD_SNAPSHOT']
+    limit = int(os.environ['SIM_UPLOAD_MAX_BYTES'])
+    fd = os.open(source, os.O_RDONLY | os.O_NONBLOCK)
+    info = os.fstat(fd)
     if not stat.S_ISREG(info.st_mode):
-        raise ValueError('Upload source must be a regular file')
+        fail('not_regular')
     if info.st_size > limit:
-        raise ValueError('Upload source exceeds the workspace file size limit')
-    with os.fdopen(os.open(target, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o600), 'wb') as writer:
+        fail('too_large')
+    reasons = STAGING_REASONS
+    with os.fdopen(fd, 'rb') as reader, os.fdopen(
+        os.open(target, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o600), 'wb'
+    ) as writer:
         size = 0
         while True:
             chunk = reader.read(min(1024 * 1024, limit - size + 1))
             if not chunk:
                 break
             size += len(chunk)
             if size > limit:
-                raise ValueError('Upload source exceeds the workspace file size limit')
+                fail('too_large')
             writer.write(chunk)
+except Exception as error:
+    fail(reasons.get(getattr(error, 'errno', None), 'unknown'), type(error).__name__)
 SIM_UPLOAD_SNAPSHOT`
 
+const FAILURE_LINE = new RegExp(`^${FAILURE_MARKER} ([a-z_]+)(?: ([A-Za-z][A-Za-z0-9_]*))?$`, 'm')
+
+/** Maps the copy script's closed failure set to caller-fixable errors the model can act on. */
+function snapshotFailure(
+  copied: { exitCode: number; stdout: string },
+  path: string,
+  maxBytes: number
+): Error {
+  const [, reason, exceptionClass] = FAILURE_LINE.exec(copied.stdout) ?? []
+  switch (reason) {
+    case 'not_found':
+      return new OrchestrationError(
+        'not_found',
+        `No file exists at ${path} on this chat's workbench. Check the path, or wait for the step that writes it to finish.`
+      )
+    case 'not_regular':
+      return new OrchestrationError(
+        'validation',
+        `${path} is not a regular file on this chat's workbench (it may be a directory). Use a file inside it instead.`
+      )
+    case 'permission':
+      return new OrchestrationError(
+        'forbidden',
+        `The workbench user does not have permission to read ${path}.`
+      )
+    case 'outside_roots':
+      return new OrchestrationError(
+        'forbidden',
+        `${path} resolves outside the permitted workbench directories.`
+      )
+    case 'too_large':
+      return new OrchestrationError(
+        'payload_too_large',
+        `${path} exceeds the ${maxBytes}-byte size limit.`
+      )
+    case 'no_space':
+      return new OrchestrationError(
+        'conflict',
+        "This chat's workbench is out of disk space. Delete files it no longer needs, then retry."
+      )
+    default:
+      return new Error(
+        `Could not prepare the workbench upload file (exit ${copied.exitCode}${exceptionClass ? `, ${exceptionClass}` : ''})`
+      )
+  }
+}
+
 /** Only the existing chat machine may supply an upload; a missing read never allocates one. */
 export async function openSessionFileSnapshot(
   sessionKey: string,
@@ -66,7 +135,11 @@ export async function openSessionFileSnapshot(
         lifetimeMs: SESSION_SANDBOX_IDLE_MS,
       })
       accessSignal.throwIfAborted()
-      if (!sandbox) throw new Error(`No workbench exists for this chat; write "${path}" first.`)
+      if (!sandbox)
+        throw new OrchestrationError(
+          'not_found',
+          `No workbench exists for this chat; write "${path}" first.`
+        )
       const readStream = sandbox.readFileStream?.bind(sandbox)
       if (!readStream) throw new Error('This workbench does not support file streaming')
       const staged = `/tmp/.sim-upload-${generateShortId(16)}`
@@ -103,14 +176,7 @@ export async function openSessionFileSnapshot(
         })
         accessSignal.throwIfAborted()
         if (copied.timedOut) throw new Error('Workbench upload snapshot timed out')
-        if (copied.exitCode !== 0) {
-          const reason = [
-            'Scratch file resolves outside the permitted sandbox directories',
-            'Upload source must be a regular file',
-            'Upload source exceeds the workspace file size limit',
-          ].find((message) => copied.stderr.trim().split('\n').at(-1)?.includes(message))
-          throw new Error(reason ?? 'Could not prepare the workbench upload file')
-        }
+        if (copied.exitCode !== 0) throw snapshotFailure(copied, path, maxBytes)
         const size = await sandbox.getFileSize(staged)
         accessSignal.throwIfAborted()
         if (!Number.isSafeInteger(size) || size < 0 || size > maxBytes) {

apps/sim/lib/mothership/chat/application/read-sandbox-file.test.ts

@@ -7,6 +7,8 @@ import {
 import { workspaceAuthzMock, workspaceAuthzMockFns } from '@sim/testing/mocks/workspace-authz.mock'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 import { ContentRefusedError, observeContentDelivery } from '@/lib/core/application'
+import { OrchestrationError } from '@/lib/core/orchestration/types'
+import { fileReadFailure } from '@/lib/mothership/agent-cli/engines/file-view'
 import { createCopilotChatFilePrincipal } from '@/lib/mothership/auth/file-delegation'
 import { createRegistryContentGate } from '@/lib/mothership/content-delivery'
 
@@ -131,6 +133,13 @@ describe('owned scratch snapshot reads', () => {
       hasRecordedSecrets: true,
     })
   })
+  it('passes a classified workbench failure through to the model unchanged', async () => {
+    const missing = new OrchestrationError('not_found', 'No file exists at /tmp/image.png')
+    mocks.snapshot.mockRejectedValue(missing)
+    const failure = await readAsChat().catch((error: unknown) => error)
+    expect(failure).toBe(missing)
+    expect(fileReadFailure(failure).stderr).toBe('Error: No file exists at /tmp/image.png')
+  })
   it('enforces size before streaming and still disposes the snapshot', async () => {
     await expect(
       readChatSandboxFile.execute({ principal, input: { ...input, maxBytes: 2 } })

apps/sim/lib/workspace-files/application/resolve-workspace-file-reference.test.ts

@@ -11,6 +11,8 @@ vi.mock('@sim/platform-authz/workspace', () => workspaceAuthzMock)
 vi.mock('@/lib/uploads/contexts/workspace/file-manager', () => workspaceFileManagerMock)
 
 import { defineWorkspaceOperation } from '@/lib/core/application'
+import { fileReadFailure } from '@/lib/mothership/agent-cli/engines/file-view'
+import { decodeVfsPathSegments } from '@/lib/vfs/path'
 import { fileOperations } from '@/lib/workspace-files/application/operations'
 import { resolveWorkspaceFileReference } from '@/lib/workspace-files/application/resolve-workspace-file-reference'
 
@@ -47,6 +49,25 @@ describe('workspace file reference application service', () => {
     mocks.fetchBuffer.mockResolvedValue(Buffer.from('source'))
   })
 
+  it('rejects a reference that is not a VFS path, such as a URL, as a validation error the model sees', async () => {
+    const reference = 'https://example.com/report.pdf'
+    mocks.resolveStoredReference.mockImplementation(async (_workspaceId, value: string) => {
+      decodeVfsPathSegments(value)
+      return file
+    })
+    const failure = await resolveWorkspaceFileReference({
+      principal,
+      operation: fileOperations.readContent,
+      workspaceId: file.workspaceId,
+      reference,
+    }).catch((error: unknown) => error)
+    expect(failure).toMatchObject({ name: 'OrchestrationError', code: 'validation' })
+    expect(fileReadFailure(failure).stderr).toBe(
+      `Error: "${reference}" is not a valid workspace file path or ID: VFS path segment cannot be empty or a dot segment`
+    )
+    expect(mocks.loadContext).not.toHaveBeenCalled()
+  })
+
   it('carries trusted chat scope into an authorized upload lookup', async () => {
     await resolveWorkspaceFileReference({
       principal,