Skip to content

improve filename generation and conflict handling#3380

Merged
ComputelessComputer merged 4 commits intomainfrom
fix/improve-file-upload-mechanism
Jan 26, 2026
Merged

improve filename generation and conflict handling#3380
ComputelessComputer merged 4 commits intomainfrom
fix/improve-file-upload-mechanism

Conversation

@ComputelessComputer
Copy link
Collaborator

@ComputelessComputer ComputelessComputer commented Jan 26, 2026

Summary

Refactor file upload mechanism to generate more readable filenames, prevent special characters, and handle potential filename conflicts by appending a numeric suffix when needed.

Review & Testing Checklist for Human

  • Filename sanitization coverage: Test with edge-case filenames — unicode characters, OS-reserved names (CON, NUL, AUX on Windows), zero-width characters, names consisting entirely of dots or spaces — and verify each produces a valid, non-empty filename
  • Conflict resolution under concurrency: Upload two files with the same name simultaneously and verify the numeric suffix logic doesn't race (e.g., both getting -1 instead of -1 and -2)
  • Round-trip readability: Upload a file with a complex original name, then download or reference the stored file — confirm the generated name is human-readable and the file extension is preserved correctly

Suggested test plan: Upload files with a variety of names: normal (report.pdf), special chars (my file (final) [v2].pdf), unicode (レポート.pdf), empty-ish (. .pdf), and duplicates. Verify each stored filename is valid, readable, and unique.

Notes

Link to Devin run: https://app.devin.ai/sessions/1b9ae9acc87349e393883ca54ed961c6
Requested by: @ComputelessComputer

@netlify
Copy link

netlify bot commented Jan 26, 2026

Deploy Preview for hyprnote failed.

Name Link
🔨 Latest commit 538c2ac
🔍 Latest deploy log https://app.netlify.com/projects/hyprnote/deploys/6976df004f3d6e0008452faf

@netlify
Copy link

netlify bot commented Jan 26, 2026

Deploy Preview for howto-fix-macos-audio-selection canceled.

Name Link
🔨 Latest commit 538c2ac
🔍 Latest deploy log https://app.netlify.com/projects/howto-fix-macos-audio-selection/deploys/6976df00d1b4670008e6f20f

@netlify
Copy link

netlify bot commented Jan 26, 2026

Deploy Preview for hyprnote-storybook canceled.

Name Link
🔨 Latest commit 538c2ac
🔍 Latest deploy log https://app.netlify.com/projects/hyprnote-storybook/deploys/6976df00cf7f430008040af0

@ComputelessComputer ComputelessComputer force-pushed the fix/improve-file-upload-mechanism branch from 2fdf623 to e82b92c Compare January 26, 2026 03:17
Refactor file upload mechanism to generate more readable filenames,
prevent special characters, and handle potential filename conflicts
by appending a numeric suffix when needed.
@ComputelessComputer ComputelessComputer force-pushed the fix/improve-file-upload-mechanism branch from 4c67a67 to 538c2ac Compare January 26, 2026 03:26
Comment on lines +334 to +348
for (const filePath of allFiles) {
const relativePath = filePath.substring(fromPath.length);
const newFilePath = toPath + relativePath;

const { error } = await supabase.storage
.from(BUCKET_NAME)
.move(filePath, newFilePath);

if (error) {
return {
success: false,
error: `Failed to move ${filePath}: ${error.message}`,
};
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Partial failure leaves folder in inconsistent state

When moving a folder with multiple files, if any file move operation fails, some files will have been moved while others remain in the original location. There's no transaction or rollback mechanism, leaving the folder structure corrupted.

for (const filePath of allFiles) {
  const relativePath = filePath.substring(fromPath.length);
  const newFilePath = toPath + relativePath;

  const { error } = await supabase.storage
    .from(BUCKET_NAME)
    .move(filePath, newFilePath);

  if (error) {
    return {
      success: false,
      error: `Failed to move ${filePath}: ${error.message}`,
    };
  }
}

Implement a rollback mechanism or use a transaction-like approach to move all files atomically, or at least document this limitation to users.

Suggested change
for (const filePath of allFiles) {
const relativePath = filePath.substring(fromPath.length);
const newFilePath = toPath + relativePath;
const { error } = await supabase.storage
.from(BUCKET_NAME)
.move(filePath, newFilePath);
if (error) {
return {
success: false,
error: `Failed to move ${filePath}: ${error.message}`,
};
}
}
// First phase: collect all move operations to perform
const moveOperations = allFiles.map(filePath => {
const relativePath = filePath.substring(fromPath.length);
const newFilePath = toPath + relativePath;
return { source: filePath, destination: newFilePath };
});
// Second phase: execute all moves
const movedFiles = [];
for (const { source, destination } of moveOperations) {
const { error } = await supabase.storage
.from(BUCKET_NAME)
.move(source, destination);
if (error) {
// Rollback: move all previously moved files back to their original locations
for (const { source: origSrc, destination: origDest } of movedFiles) {
await supabase.storage
.from(BUCKET_NAME)
.move(origDest, origSrc); // Move back (ignoring errors during rollback)
}
return {
success: false,
error: `Failed to move ${source}: ${error.message}. All operations have been rolled back.`,
};
}
movedFiles.push({ source, destination });
}

Spotted by Graphite Agent

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

This comment came from an experimental review—please leave feedback if it was helpful/unhelpful. Learn more about experimental comments here.

@ComputelessComputer ComputelessComputer merged commit afa7c70 into main Jan 26, 2026
15 of 20 checks passed
@ComputelessComputer ComputelessComputer deleted the fix/improve-file-upload-mechanism branch January 26, 2026 03:32
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

Comments