Repository navigation
Conversation
fileUrl() runs when files are added, and pinned the bucket of the latest listing, so a file listed in one bucket could be imported from another after a later listing reported a different one. The bucket is now remembered per listed item.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
🦋 Changeset detectedLatest commit: 0385b47 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Files are identified by key only, so when a session sees the same key in two buckets, the selection can't say which one the user picked. Refuse it instead of importing from whichever bucket listed it last.
There was a problem hiding this comment.
🟡 Changes recommended
The picker throws a plain Error that is only logged, so clicking Done adds no files and hides the logout instruction users need to recover.
1 open issue
What changed in this PR
This PR fixes S3 imports using the wrong bucket after a later listing changes the session’s bucket.
Changes:
- Tracks the bucket for each listed file and clears that tracking on logout.
- Rejects keys listed in multiple buckets rather than choosing an ambiguous source.
- Adds regression coverage and a patch changeset.
| File | Description |
|---|---|
packages/@uppy/s3/test/S3.browser.test.tsx |
Tests per-file bucket retention and ambiguous-key rejection. |
packages/@uppy/s3/src/S3.tsx |
Replaces the shared bucket field with per-path tracking. |
.changeset/s3-bucket-per-file.md |
Documents the fix for an @uppy/s3 patch release. |
Review effort: Balanced
| if (!this.#bucket) | ||
| const bucket = this.#bucketByPath.get(id) | ||
| if (bucket === null) | ||
| throw new Error( |
@uppy/s3: a selected file could be imported from the wrong bucket.fileUrl()runs when the selected files are added (on Done), and pinned the bucket that the most recent listing returned. If a later listing returned a different bucket before Done, files listed in the first bucket were imported with the second bucket's name. The plugin now remembers the bucket of each listed item when the listing arrives, andfileUrl()uses that; logout clears it. The old single#bucketfield is gone.Files are identified by their key, so if one session lists the same key in two different buckets, the selection can't tell which one was picked. Such a file is now refused ("log out and select it again") instead of being imported from whichever bucket listed it last.
Tests
@uppy/s3and@uppy/transloadit-storagesuites pass; types and Biome clean.🤖 Generated with Claude Code