fix(sandbox): spend each file ceiling once across every source

Two ceilings on a Function run's sandbox files were charged per source
rather than per execution.

Mounts: planUserFileMounts assigned a path per element, so one storage
key named by two sources became two mounts. The resolver already reuses
a marker for a file the code references twice, but nothing collapsed a
caller's `files` list against itself or against those markers — and
`files` is `user-or-llm`, so a model repeating an id is the ordinary
case. The duplicate cost a presign, a second transfer of identical
bytes, and a second charge against both the byte budget and the 20-file
mount ceiling, either of which then refuses a request that fits. Key on
the storage key, first occurrence wins; keyless files stay separate
since an empty key identifies nothing.

Exports: MAX_SANDBOX_OUTPUT_FILES is documented as what one execution
may export "whether declared by path or discovered by harvesting", and
the byte ceiling is already shared and dedup-aware across the two. The
count ceiling was not — a request declaring paths and harvesting a
directory could export 20 of each. Check the harvest against what the
declared paths leave, and drop a declared path inside the directory from
the discovered set so it is not billed on both sides. Unreachable from
execute-request today, which sets outputSandboxDir only when nothing
declares a sandboxPath, but the layer is caller-agnostic and already
commits to the combined rule for bytes.

Fixture keys in sandbox-mounts.test.ts were identical across files the
tests meant to be distinct; they now differ, which is what those tests
always claimed to set up.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: icecrasher321

apps/sim/lib/execution/remote-sandbox/conformance.test.ts

@@ -868,6 +868,59 @@ describe.each(PROVIDERS)('sandbox conformance [%s]', (provider) => {
     ).rejects.toThrow(/over the 20-file export limit/)
   })
 
+  it('spends one file ceiling across declared and harvested outputs', async () => {
+    // The limit is what an execution exports, not what one directory holds, so a
+    // request that both declares and harvests cannot take 20 of each.
+    stubCodeRun(provider, `__SIM_RESULT__=${JSON.stringify('done')}`)
+    stubOutputFileSizes(provider, 1, 1)
+    stubOutputDirListing(
+      Array.from({ length: MAX_SANDBOX_OUTPUT_FILES - 1 }, (_, index) => ({
+        path: `/tmp/sim/outputs/file-${index}.txt`,
+        size: 1,
+      }))
+    )
+
+    await expect(
+      executeInSandbox({
+        code: 'x',
+        language: CodeLanguage.Python,
+        timeoutMs: 1000,
+        outputSandboxPaths: ['/out/first.txt', '/out/second.txt'],
+        outputSandboxDir: '/tmp/sim/outputs',
+      })
+    ).rejects.toThrow(/over the 18-file export limit/)
+  })
+
+  it('does not charge a declared path inside the harvest directory to the ceiling twice', async () => {
+    // The directory holds exactly the limit and the request names one of those
+    // files. Charging it on both sides would refuse a run exporting 20 files.
+    stubCodeRun(provider, `__SIM_RESULT__=${JSON.stringify('done')}`)
+    // One inspection for the declared path, then one per file actually read.
+    stubOutputFileSizes(provider, ...Array.from({ length: MAX_SANDBOX_OUTPUT_FILES + 1 }, () => 1))
+    stubOutputDirListing(
+      Array.from({ length: MAX_SANDBOX_OUTPUT_FILES }, (_, index) => ({
+        path: `/tmp/sim/outputs/file-${index}.txt`,
+        size: 1,
+      }))
+    )
+    for (let index = 0; index < MAX_SANDBOX_OUTPUT_FILES; index += 1) {
+      stubOutputFileRead(provider, 'x')
+    }
+
+    const result = await executeInSandbox({
+      code: 'x',
+      language: CodeLanguage.Python,
+      timeoutMs: 1000,
+      outputSandboxPath: '/tmp/sim/outputs/file-0.txt',
+      outputSandboxDir: '/tmp/sim/outputs',
+    })
+
+    // Exported once as a declared path, rather than a second time as a harvest.
+    expect(Object.keys(result.exportedFiles ?? {})).toEqual(['/tmp/sim/outputs/file-0.txt'])
+    expect(result.collectedFiles).toHaveLength(MAX_SANDBOX_OUTPUT_FILES - 1)
+    expect(result.collectedFiles?.map((file) => file.relativePath)).not.toContain('file-0.txt')
+  })
+
   it('does not list the output directory when no harvest was requested', async () => {
     stubCodeRun(provider, `__SIM_RESULT__=${JSON.stringify('done')}`)
 

apps/sim/lib/execution/remote-sandbox/index.ts

@@ -554,10 +554,16 @@ function requestedOutputSandboxPaths(req: {
  * too many files, or nesting past what the listing reaches — before a single
  * byte is read. Sorted so a multi-file result is stable run to run rather than
  * inheriting whatever order the provider happened to return.
+ *
+ * `declaredPaths` are the files the request already named. One sitting inside the
+ * directory is dropped rather than harvested a second time, and the rest still
+ * consume the ceiling: the limit is what one execution exports, not what one
+ * directory holds, so declaring and harvesting together cannot spend it twice.
  */
 async function listOutputDirectoryFiles(
   sandbox: SandboxHandle,
   outputSandboxDir: string,
+  declaredPaths: ReadonlySet<string>,
   signal: AbortSignal
 ): Promise<SandboxDirectoryEntry[]> {
   let listed: SandboxDirectoryEntry[]
@@ -593,9 +599,10 @@ async function listOutputDirectoryFiles(
     )
   }
 
-  const files = entries.filter((entry) => entry.kind === 'file')
-  if (files.length > MAX_SANDBOX_OUTPUT_FILES) {
-    throw new SandboxOutputFileCountError(files.length, outputSandboxDir)
+  const files = entries.filter((entry) => entry.kind === 'file' && !declaredPaths.has(entry.path))
+  const remaining = Math.max(0, MAX_SANDBOX_OUTPUT_FILES - declaredPaths.size)
+  if (files.length > remaining) {
+    throw new SandboxOutputFileCountError(files.length, outputSandboxDir, remaining)
   }
   return files.sort((a, b) => a.path.localeCompare(b.path))
 }
@@ -640,16 +647,14 @@ async function collectExportedFiles(
   }
 
   // Sized into the same running total as the declared paths, so an execution
-  // cannot spend the ceiling twice by both declaring and harvesting. A declared
-  // path that happens to sit inside the harvest directory is dropped from the
-  // discovered set rather than counted again — double-billing it would reject a
-  // single output larger than half the ceiling as oversized.
+  // cannot spend the byte ceiling twice by both declaring and harvesting. The
+  // listing applies the same rule to the file-count ceiling and drops a declared
+  // path that happens to sit inside the harvest directory — double-billing it
+  // would reject a single output larger than half the ceiling as oversized.
   const declaredPaths = new Set(readablePaths)
-  const discovered = (
-    req.outputSandboxDir
-      ? await listOutputDirectoryFiles(sandbox, req.outputSandboxDir, options.signal)
-      : []
-  ).filter((entry) => !declaredPaths.has(entry.path))
+  const discovered = req.outputSandboxDir
+    ? await listOutputDirectoryFiles(sandbox, req.outputSandboxDir, declaredPaths, options.signal)
+    : []
   for (const entry of discovered) {
     totalOutputBytes += entry.size
     if (totalOutputBytes > MAX_SANDBOX_OUTPUT_BYTES) {

apps/sim/lib/function-execution/sandbox-mounts.test.ts

@@ -88,7 +88,7 @@ describe('planUserFileMounts', () => {
   it('cannot be escaped by a traversal in the file name', () => {
     const planned = planUserFileMounts([
       executionFile({ name: '../../etc/passwd' }),
-      executionFile({ id: 'file_2', name: '..' }),
+      executionFile({ id: 'file_2', key: 'execution/other', name: '..' }),
     ])
 
     for (const { mountPath } of planned) {
@@ -100,9 +100,9 @@ describe('planUserFileMounts', () => {
 
   it('suffixes colliding names so neither file is silently overwritten', () => {
     const planned = planUserFileMounts([
-      executionFile({ id: 'file_1', name: 'report.csv' }),
-      executionFile({ id: 'file_2', name: 'report.csv' }),
-      executionFile({ id: 'file_3', name: 'report.csv' }),
+      executionFile({ id: 'file_1', key: 'execution/a/report.csv', name: 'report.csv' }),
+      executionFile({ id: 'file_2', key: 'execution/b/report.csv', name: 'report.csv' }),
+      executionFile({ id: 'file_3', key: 'execution/c/report.csv', name: 'report.csv' }),
     ])
 
     expect(planned.map((entry) => entry.mountPath)).toEqual([
@@ -111,6 +111,38 @@ describe('planUserFileMounts', () => {
       '/tmp/sim/inputs/report-3.csv',
     ])
   })
+
+  it('mounts one storage key once however many sources named it', () => {
+    // A caller listing the same file twice, and a `<block.file.path>` marker for
+    // a file the caller also passed explicitly, both land in one list here. A
+    // second copy of identical bytes costs a presign and a duplicate transfer,
+    // and charges the byte budget and the 20-file ceiling twice over.
+    const planned = planUserFileMounts([
+      executionFile({ id: 'file_1', name: 'report.csv' }),
+      executionFile({ id: 'file_1_again', name: 'report.csv' }),
+      executionFile({ id: 'file_2', name: 'renamed.csv' }),
+      workspaceFile(),
+    ])
+
+    expect(planned.map((entry) => entry.mountPath)).toEqual([
+      '/tmp/sim/inputs/report.csv',
+      '/tmp/sim/inputs/brief.pdf',
+    ])
+  })
+
+  it('keeps keyless files apart rather than collapsing them onto each other', () => {
+    // An empty key identifies no stored object, so two of them are not evidence
+    // of the same file.
+    const planned = planUserFileMounts([
+      executionFile({ id: 'file_1', key: '', name: 'a.csv' }),
+      executionFile({ id: 'file_2', key: '', name: 'b.csv' }),
+    ])
+
+    expect(planned.map((entry) => entry.mountPath)).toEqual([
+      '/tmp/sim/inputs/a.csv',
+      '/tmp/sim/inputs/b.csv',
+    ])
+  })
 })
 
 describe('resolveUserFileMounts', () => {
@@ -193,14 +225,16 @@ describe('resolveUserFileMounts', () => {
 
     await expect(
       resolveUserFileMounts({
-        planned: planUserFileMounts([
-          executionFile({ id: 'a', name: 'a.bin', size: 9 * 1024 * 1024 }),
-          executionFile({ id: 'b', name: 'b.bin', size: 9 * 1024 * 1024 }),
-          executionFile({ id: 'c', name: 'c.bin', size: 9 * 1024 * 1024 }),
-          executionFile({ id: 'd', name: 'd.bin', size: 9 * 1024 * 1024 }),
-          executionFile({ id: 'e', name: 'e.bin', size: 9 * 1024 * 1024 }),
-          executionFile({ id: 'f', name: 'f.bin', size: 9 * 1024 * 1024 }),
-        ]),
+        planned: planUserFileMounts(
+          ['a', 'b', 'c', 'd', 'e', 'f'].map((id) =>
+            executionFile({
+              id,
+              key: `execution/${WORKSPACE_ID}/${WORKFLOW_ID}/${EXECUTION_ID}/${id}/${id}.bin`,
+              name: `${id}.bin`,
+              size: 9 * 1024 * 1024,
+            })
+          )
+        ),
         context: executionContext,
       })
     ).rejects.toThrow(/total mount limit/)

apps/sim/lib/function-execution/sandbox-mounts.ts

@@ -227,16 +227,39 @@ function uniqueMountFileName(name: string, used: Set<string>): string {
  * Assigns each file a deterministic mount path. Pure and I/O-free, so a caller
  * can decide whether an execution needs a sandbox filesystem before spending a
  * presign or a byte of transfer on a request that may still be refused.
+ *
+ * One storage key mounts once. The same object reaches here from independent
+ * sources — a caller listing it twice, or listing one the code also asked for
+ * with `<block.file.path>` — and a second copy of identical bytes buys nothing
+ * while costing a presign, a duplicate transfer, and a second charge against
+ * both the byte budget and the per-request file ceiling, either of which can
+ * refuse a request that fits. First occurrence wins, so the name listed first is
+ * the one the code sees.
  */
 export function planUserFileMounts(
   files: readonly UserFile[],
   mountDir: string = SANDBOX_INPUT_DIR
 ): PlannedUserFileMount[] {
   const used = new Set<string>()
-  return files.map((userFile) => ({
-    userFile,
-    mountPath: `${mountDir}/${uniqueMountFileName(userFile.name, used)}`,
-  }))
+  const mountedKeys = new Set<string>()
+  const planned: PlannedUserFileMount[] = []
+
+  for (const userFile of files) {
+    // Keyed on the storage key because that is the stored object's identity, and
+    // what the caller later looks mount paths up by. An empty key identifies
+    // nothing, so those files stay separate rather than collapsing onto whichever
+    // one came first.
+    if (userFile.key) {
+      if (mountedKeys.has(userFile.key)) continue
+      mountedKeys.add(userFile.key)
+    }
+    planned.push({
+      userFile,
+      mountPath: `${mountDir}/${uniqueMountFileName(userFile.name, used)}`,
+    })
+  }
+
+  return planned
 }
 
 /**