Skip to content

test(files): prove files/ VFS path downloads over real HTTP instead of a mocked route call - #9662

Merged
waleedlatif1 merged 2 commits into
stagingfrom
fix/vfs-download-http-proof
Nov 4, 2026
Merged

waleedlatif1 merged 2 commits into
stagingfrom
fix/vfs-download-http-proof

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

Greptile flagged on the v0.9.43 release PR (#9660) that the v2 file download case added in #9637 mocked the download use case, passed the already-decoded route parameter straight to the handler, and asserted toHaveBeenCalledWith on the mock. CLAUDE.md forbids that pattern, and it could not catch the one thing the integration test does not reach: Next.js routing a %2F-encoded files/... path into the single [fileId] segment, which is exactly how sim files get files/... calls the route (encodeURIComponent on path params).

  • Removes the mocked route case. Every mock and import it used is still used by the other cases.
  • Adds one check to apps/sim/scripts/test-file-delivery-e2e.ts (CI: project-files http e2e group, via test-project-files-e2e.ts): it creates a VfsDocs folder, a root vfs-report.csv and a same-named copy in the folder, then downloads files/VfsDocs/vfs-report.csv and files/vfs-report.csv through /api/v2/files/<encoded path> with an API key over real HTTP. Each must return its own bytes, and a missing path returns 404. A routing bug that truncated the segment at / would land on the root copy or 404.
  • Service-level resolution, scoping and refusals stay covered by lib/workspace-files/delivery.integration.ts (unchanged).

No product code changes.

Testing

  • Red proof: tmp/vfs-download-red is this test plus the route with acceptVfsPath: true removed; the checks workflow dispatched on it must fail the project-files e2e group on this check (run linked in a comment below once it finishes).
  • Green: this PR's CI e2e (project-files).
  • biome check on both files.

…f a mocked route call

The v2 file download case added in #9637 passed the decoded path straight to the
handler and asserted on the mocked use case, so it could not catch an encoded
files/ path failing to reach the route or resolving to the wrong file. The file
delivery e2e now uploads same-named files at the root and in a folder and
downloads each by its %2F-encoded VFS path through the running app.
@vercel

vercel Bot commented Nov 4, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
docs Skipped Skipped Nov 4, 2026 9:28am UTC

Request Review

@greptile-apps

greptile-apps Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Low impact] This test-only PR appears safe to merge.

Summary

Replaces the mocked VFS download test with real HTTP coverage.

  • Workspace file tests check encoded paths through real HTTP.

Reviews (1) · Last reviewed commit: "test(files): keep the HTTP check to the ..." · Reviewed by Greptile

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 2 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Turn on auto-fix | Re-trigger cubic

@waleedlatif1
waleedlatif1 merged commit f0a3432 into staging Nov 4, 2026
49 checks passed
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

Red proof: the checks workflow dispatched on tmp/vfs-download-red (this test plus the route with acceptVfsPath: true removed), run https://github.com/simstudioai/sim/actions/runs/41807505693, failed e2e (project-files) on exactly this check: files/VfsDocs/vfs-report.csv returned 404. Every other delivery check passed. This PR's own run passed it.

This branch was previously deployed

1 inactive deployment
Preview — 267e8e80 Deployed Nov 4, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant