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

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. `files` is `user-or-llm`
and deduped nowhere, so a model repeating an id — or naming a file the
code also references with `<block.file.path>` — produced a duplicate
that 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. Collapse by storage
key, first occurrence wins. The contract already requires a non-empty
key, so there is no keyless case to carry.

Exports: MAX_SANDBOX_OUTPUT_FILES is documented as what one execution
may export "whether declared by path or discovered by harvesting", and
collectExportedFiles already runs the byte ceiling that way. The count
ceiling did not, so a request declaring paths and harvesting a directory
could export 20 of each. Count declared and discovered together, with a
declared path inside the directory dropped from the discovered set so it
is not billed on both sides. With no declared paths — every call
execute-request makes, since it sets outputSandboxDir only when nothing
declares a sandboxPath — the check and its message are unchanged.

The resolver's marker reuse is no longer what keeps a twice-referenced
file to one mount; its comment said otherwise.

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/executor/variables/resolver.ts

@@ -694,9 +694,10 @@ export class VariableResolver {
       return null
     }
 
-    // Reuse an existing marker for the same file so referencing one path twice
-    // mounts it once, rather than transferring a second copy under a
-    // collision-suffixed name and spending the mount budget twice.
+    // Reuse the marker already standing for this file so a path referenced twice
+    // costs one context variable rather than two. What keeps it to one mount is
+    // `planUserFileMounts`, which collapses by storage key across every source —
+    // this only keeps the duplicate out of the request body.
     const existing = Object.entries(contextVarAccumulator).find(
       ([, value]) => isSandboxFileMountRef(value) && value.file.key === file.key
     )

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(/produced 21 files .* over the 20-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 count
+ * toward the ceiling: the limit is what one execution exports, not what one
+ * directory holds, so declaring and harvesting 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 exported = declaredPaths.size + files.length
+  if (exported > MAX_SANDBOX_OUTPUT_FILES) {
+    throw new SandboxOutputFileCountError(exported, outputSandboxDir)
   }
   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,24 @@ 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',
+    ])
+  })
 })
 
 describe('resolveUserFileMounts', () => {
@@ -193,14 +211,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,32 @@ 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.
+ *
+ * A storage key mounts once. The same object arrives 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 costs a presign, a
+ * duplicate transfer, and a second charge against both the byte budget and the
+ * per-request file ceiling. 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) {
+    if (mountedKeys.has(userFile.key)) continue
+    mountedKeys.add(userFile.key)
+    planned.push({
+      userFile,
+      mountPath: `${mountDir}/${uniqueMountFileName(userFile.name, used)}`,
+    })
+  }
+
+  return planned
 }
 
 /**