fix(updater): support encoded manifest asset downloads#2247
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughCentralizes CLI partial-upload path encoding, adds safe multi-candidate R2 read selection for scoped attachments, supplies manifest encoding utilities and normalization, adds unit and Bun tests for encoding/lookup behavior, and provides a manifest-audit/repair script. ChangesPath Encoding Fix and Safe R2 Read Handling
Sequence DiagramsequenceDiagram
participant Client
participant getHandler
participant getSafeAttachmentReadCandidateKeys
participant R2Bucket
Client->>getHandler: GET /file/{fileId}
getHandler->>getHandler: extract fileId (decoded) and rawFileId from route
getHandler->>getSafeAttachmentReadCandidateKeys: (fileId, rawFileId)
getSafeAttachmentReadCandidateKeys-->>getHandler: candidateKeys: string[]
loop for each candidateKey
getHandler->>R2Bucket: head/get(candidateKey, headers, range)
alt object found
R2Bucket-->>getHandler: object data
getHandler->>Client: return object
else not found
R2Bucket-->>getHandler: null
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Merging this PR will not alter performance
Comparing 
Footnotes
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@supabase/functions/_backend/files/files.ts`:
- Around line 474-483: The restore logic currently only checks head(fileId)
before writing back, which can cause duplicate restores when a fallback read
served a legacy key; update the restore existence check to reuse the same
candidateKeys produced by getSafeAttachmentReadCandidateKeys(rawFileId) (the
same array iterated with RetryBucket.get) and check for existence across those
keys (e.g., call head or equivalent existence check for each candidateKey)
before calling put(fileId, ...); in other words, when preparing to restore into
fileId, iterate candidateKeys and skip the put if any candidateKey already
exists to avoid unnecessary put(fileId, ...) and alert noise (reference
functions/objects: getRawAttachmentRouteId, getSafeAttachmentReadCandidateKeys,
RetryBucket, DEFAULT_RETRY_PARAMS, head, put).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bd24327c-c8ab-4a61-a20f-171b36f34985
📒 Files selected for processing (8)
cli/package.jsoncli/src/bundle/partial.tscli/test/test-manifest-path-encoding.mjsscripts/audit_manifest_filename_encoding.tssupabase/functions/_backend/files/files.tssupabase/functions/_backend/files/util.tstests/manifest-path-encoding.unit.test.tstests/upload-path-encoding.unit.test.ts
|
The range preflight still only checks the decoded key with That means out-of-range legacy range requests will not return the intended |
|
Fixed in cbbd60e: range preflight now heads the same decoded/raw candidate key set used by downloads, and the unit test covers the legacy |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/manifest-path-encoding.unit.test.ts`:
- Around line 3-4: Import order violates perfectionist/sort-imports; move the
import of getManifestUrl so it appears before
normalizeLegacyEncodedManifestFileName to satisfy the linter. Update the two
import statements so getManifestUrl is imported first (from
../supabase/functions/_backend/utils/downloadUrl.ts) followed by
normalizeLegacyEncodedManifestFileName (from
../supabase/functions/_backend/utils/manifest_encoding.ts).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a96b6904-3a27-4c84-8b84-690d4fe33dda
📒 Files selected for processing (4)
supabase/functions/_backend/triggers/on_version_update.tssupabase/functions/_backend/utils/downloadUrl.tssupabase/functions/_backend/utils/manifest_encoding.tstests/manifest-path-encoding.unit.test.ts
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub. |
|
3 participants
Summary (AI generated)
file_namevalues at/updatesresponse time while keeping the response shape unchanged.on_version_updateso future uploads from old CLI versions save raw installer paths server-side.file_namevalues and URL-safes3_pathobject keys.Motivation (AI generated)
Customers on
@capgo/capacitor-updater8.45.11 can fail manifest downloads for assets containing characters like@. The visible encoded filename in logs is only a symptom: old uploads can have encoded local manifest names and encoded object keys, while the file route may decode before the R2 lookup. This fixes the actual download and install path without changing the endpoint contract or requiring a response version bump.Business Impact (AI generated)
After the backend deploy, old affected uploads should start returning updater-safe manifest paths without customers re-uploading, reinstalling, or shipping a native release. Old CLI users get corrected manifest rows on future uploads because the backend normalizes at insert time, and new CLI uploads remain correct by preserving raw local names.
/updatesonly gets cheap string normalization per manifest row; no R2, DB, or cache lookup is added to the hot update-check path.Test Plan (AI generated)
bunx vitest run tests/manifest-path-encoding.unit.test.ts tests/upload-path-encoding.unit.test.tsbun run --cwd cli test:manifest-path-encodingbun scripts/audit_manifest_filename_encoding.ts --helpbun lint:backendbun lint --quietbun typecheckbun run cli:typecheckbun run cli:buildgit diff --checkGenerated with AI
Summary by CodeRabbit
Bug Fixes
@symbol in file paths.Tests