Skip to content

@tus/file-store: preserve completed uploads during expiration cleanup - #898

Merged
Murderlon merged 1 commit into
tus:mainfrom
ferhatelmas:file-expiration-cleanup
Oct 28, 2026
Merged

Murderlon merged 1 commit into
tus:mainfrom
ferhatelmas:file-expiration-cleanup

Conversation

@ferhatelmas

@ferhatelmas ferhatelmas commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

related to #320 (introduced)
related to #894 (extended)

Currently, metadata offset is checked but it's never persisted. Cleanup could remove completed files. Compare the actual file size instead.

There is an inherent issue for uploads in progress because cleanup doesn't take the lock but it should be handled separately since requires a refactoring.


Devin Review

@changeset-bot

changeset-bot Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: eb50704

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@tus/file-store Patch

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

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Devin Review found 2 potential issues.

Devin Review

Comment on lines 195 to +197
info.size !== info.offset &&
info.creation_date
info.creation_date &&
now > Date.parse(info.creation_date) + expiration

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔍 Completed uploads remain expired to protocol clients

Cleanup preserves completed files, but HeadHandler and PatchHandler still return 410 after their creation-based deadline. Clients cannot query those uploads through HEAD.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this is existing but head is addressed in #900 after this, patch is related to #888 and I will follow up accordingly

Comment thread packages/file-store/src/index.ts Resolved
Signed-off-by: Ferhat Elmas <elmas.ferhat@gmail.com>
@Murderlon
Murderlon enabled auto-merge October 28, 2026 12:39
@Murderlon
Murderlon added this pull request to the merge queue Oct 28, 2026
Merged via the queue into tus:main with commit 670e47b Oct 28, 2026
4 checks passed

This branch was successfully deployed

1 active deployment
external-testing — eb507049 Deployed Oct 26, 2026 by ferhatelmas via Tests #117
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.

2 participants