-
Notifications
You must be signed in to change notification settings - Fork 0
fix(deploy): use REST API for uploads to control MIME types #27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Supabase Storage rejects MIME types with charset suffix (e.g., 'text/javascript; charset=utf-8'). The supabase CLI's storage cp command auto-detects MIME types and adds charset, causing 415 errors. This fix replaces CLI uploads with direct REST API calls using curl, with a custom get_mime_type function that returns clean MIME types without charset suffix. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The upload test was using multipart form upload (-F flag) which auto- detects MIME type as text/plain. The bucket only allows specific MIME types, so the test was failing. Changed test to: - Use JSON file instead of plain text - Use --data-binary with explicit Content-Type header - Match exactly how deploy.yml uploads files 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
athola
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Review Summary
Scope Analysis
Problem: Supabase Storage rejects MIME types with ; charset=utf-8 suffix. The supabase storage cp CLI auto-adds this, causing 415 errors for .js and .html files.
Solution: Replace CLI with direct REST API calls using curl with explicit Content-Type headers.
Blocking Issues (0)
✅ None found. This is a focused, targeted fix.
In-Scope Issues (0)
✅ All requirements addressed.
Suggestions (2)
| ID | Description | Priority |
|---|---|---|
| G1 | MIME type coverage is complete for bucket's allowed types | Info |
| G2 | Consider adding upload success logging (low priority) | Low |
Quality Assessment
| Aspect | Status |
|---|---|
| Solves root cause | ✅ Uses REST API to control exact headers |
| Matches allowed types | ✅ All bucket MIME types covered |
| Test alignment | ✅ dry-run now uses same method as deploy |
| Error handling | ✅ Retry logic preserved, HTTP status shown |
| Security | ✅ No secrets exposed, uses existing patterns |
Recommendation
APPROVE - Clean, focused fix that addresses the root cause. Ready to merge.
Review generated by Claude Code PR Review
athola
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review Summary
Issues Found & Fixed
| Priority | Issue | Status |
|---|---|---|
| Important | Missing x-upsert: true header in dry-run test |
✅ Fixed |
| Important | Fallback MIME type application/octet-stream not in allowed list |
✅ Fixed |
Fixes Applied (pending push)
- deploy-dry-run.yml: Added
x-upsert: trueheader to match production upload behavior - deploy.yml: Added
text/plainandapplication/octet-streamto bucket'sallowed_mime_types
Positive Observations
- ✅ Proper error handling with
set +e/set -earound curl calls - ✅ Good retry logic with exponential backoff (
sleep $((attempt * 2))) - ✅ Correct
--data-binaryusage preserves file contents exactly - ✅ Secrets properly passed via headers (not CLI args)
- ✅ HTTP status extraction pattern is reliable
- ✅
x-upsert: truein production prevents 409 conflicts
Version Bump
Also includes version bump to 0.1.7 with changelog entries for both 0.1.6 and 0.1.7 releases.
Recommendation
APPROVE after pushing the review fixes. All identified issues have been addressed.
Review generated by Claude Code PR Review
Version bump: - pyproject.toml: 0.1.5 → 0.1.7 - frontend/package.json: 0.1.1 → 0.1.7 - CHANGELOG.md: Added 0.1.6 and 0.1.7 entries Review fixes: - deploy-dry-run.yml: Add x-upsert header to match production - deploy.yml: Add text/plain and application/octet-stream to allowed MIME types 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Review Feedback Resolution SummaryAll review feedback has been addressed: Issues Fixed (in commit 66fc4b4)
Out-of-Scope Items → GitHub Issues
Informational Items
Generated by Enhanced PR Fix workflow |
Summary
.jsand.htmlfiles with415 invalid_mime_typeerrorssupabase storage cpCLI with direct REST API calls using curlget_mime_type()function that returns clean MIME types without charset suffixProblem
The Supabase CLI's
storage cpcommand auto-detects MIME types and adds; charset=utf-8suffix (e.g.,text/javascript; charset=utf-8). Supabase Storage API rejects these extended MIME types with a 415 error.Solution
Use the Storage REST API directly with curl, where we control the exact
Content-Typeheader. The customget_mime_typefunction maps file extensions to clean MIME types.Test plan
🤖 Generated with Claude Code