Fix bundle preview system with caching and proper routing#1392
Conversation
- Add 60s TTL caching for app authorization and bundle info to reduce DB queries - Change wrangler route patterns from `*.preview.capgo.app` to `*.preview.capgo.app/*` to match all paths - Export handlePreviewRequest directly instead of using separate Hono app to preserve context - Add .br (brotli) file lookup with server-side decompression using node:zlib - Detect MIME types correctly without .br extension suffix 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughReplaces Hono app routing with a standalone exported Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Cloudflare as CF Worker
participant Preview as handlePreviewRequest
participant Cache as PreviewAuthCache/BundleInfoCache
participant DB as Database
participant R2 as Storage (R2 / object store)
Client->>Cloudflare: HTTP preview request
Cloudflare->>Preview: invoke handlePreviewRequest(c)
Preview->>Cache: get preview auth (cache lookup)
alt cache miss
Preview->>DB: fetch app settings (allow_preview, actualAppId)
DB-->>Preview: app settings
Preview->>Cache: set preview auth (backgroundTask)
end
Preview->>Cache: get bundle info
alt bundle cache miss
Preview->>DB: query bundle (isEncrypted, hasManifest, manifests)
DB-->>Preview: bundle info + manifest entries
Preview->>Cache: set bundle info (backgroundTask)
end
Preview->>R2: fetch file object (manifest→key)
alt object is .br
Preview->>Preview: decompress Brotli
end
Preview-->>Client: response with proper Content-Type and cache headers
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @supabase/functions/_backend/files/preview.ts:
- Line 165: The variable cachedAuth in preview.ts is never reassigned; replace
the mutable declaration "let cachedAuth = await getPreviewAuth(c, appId)" with
an immutable one by using const instead of let (update the declaration where
cachedAuth is assigned from getPreviewAuth(c, appId) to use const).
- Around line 278-294: Replace the hardcoded R2 domain used when redirecting
brotli files with a configurable environment value: read the environment/config
variable used for buckets (e.g., ATTACHMENT_BUCKET or add a new
R2_PUBLIC_DOMAIN) and build publicR2Url from that value plus
manifestEntry.s3_path instead of "https://storage.capgo.app/"; update the
cloudlog and the c.redirect call to use the constructed URL, and provide a
sensible default fallback to the existing domain if the env var is not set so
redirects remain functional in prod and non-prod environments.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cloudflare_workers/files/index.tscloudflare_workers/files/wrangler.jsoncsupabase/functions/_backend/files/preview.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use single quotes and no semicolons per @antfu/eslint-config
Files:
supabase/functions/_backend/files/preview.tscloudflare_workers/files/index.ts
supabase/functions/_backend/**
📄 CodeRabbit inference engine (CLAUDE.md)
Backend logic should be organized in
supabase/functions/_backend/with subdirectories for plugins, private endpoints, public endpoints, triggers, and utilities
Files:
supabase/functions/_backend/files/preview.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use TypeScript strict mode with path aliases mapping
~/tosrc/
Files:
supabase/functions/_backend/files/preview.tscloudflare_workers/files/index.ts
supabase/functions/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Supabase Edge Functions use Deno runtime
Files:
supabase/functions/_backend/files/preview.ts
supabase/functions/_backend/**/*.{ts,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
supabase/functions/_backend/**/*.{ts,js}: Backend code must be placed insupabase/functions/_backend/as shared code deployed to Cloudflare Workers (API/Plugin/Files workers), Supabase Edge Functions, and other platforms
UsecreateHonofromutils/hono.tsfor all Hono framework application initialization and routing
All database operations must usegetPgClient()orgetDrizzleClient()fromutils/pg.tsfor PostgreSQL access during active migration to Cloudflare D1
All Hono endpoint handlers must acceptContext<MiddlewareKeyVariables>and usec.get('requestId'),c.get('apikey'), andc.get('auth')for request context
Use structured logging withcloudlog({ requestId: c.get('requestId'), message: '...' })for all backend logging
UsemiddlewareAPISecretfor internal API endpoints andmiddlewareKeyfor external API keys; validate againstowner_orgin theapikeystable
Checkc.get('auth')?.authTypeto determine authentication type ('apikey' vs 'jwt') in backend endpoints
Use Drizzle ORM query patterns withschemafrompostgress_schema.tsfor all database operations; usealiasV2()for self-joins or multiple table references
Files:
supabase/functions/_backend/files/preview.ts
supabase/functions/**/*.{ts,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Backend ESLint must pass before commit; run
bun lint:backendfor backend files
Files:
supabase/functions/_backend/files/preview.ts
cloudflare_workers/{api,plugin,files}/index.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Cloudflare Workers are split across three ports: API Worker (8787), Plugin Worker (8788), Files Worker (8789); see routing in
cloudflare_workers/{api,plugin,files}/index.ts
Files:
cloudflare_workers/files/index.ts
cloudflare_workers/files/index.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files Worker (port 8789) handles file upload/download operations
Files:
cloudflare_workers/files/index.ts
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to cloudflare_workers/api/index.ts : API Worker (port 8787) routes: `/bundle`, `/app`, `/device`, `/channel`, `/private/*`, `/triggers`
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T00:40:00.524Z
Learning: Applies to cloudflare_workers/snippet/index.js : Cloudflare Workers snippets in `cloudflare_workers/snippet/index.js` handle geographic repartitioning logic to route requests to the nearest database replica.
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to cloudflare_workers/plugin/index.ts : Plugin Worker (port 8788) routes: `/updates`, `/channel_self`, `/stats`
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to supabase/functions/_backend/**/*.{ts,js} : Use `createHono` from `utils/hono.ts` for all Hono framework application initialization and routing
Applied to files:
supabase/functions/_backend/files/preview.tscloudflare_workers/files/index.ts
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to supabase/functions/_backend/**/*.{ts,js} : All Hono endpoint handlers must accept `Context<MiddlewareKeyVariables>` and use `c.get('requestId')`, `c.get('apikey')`, and `c.get('auth')` for request context
Applied to files:
supabase/functions/_backend/files/preview.ts
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to supabase/functions/_backend/**/*.{ts,js} : Check `c.get('auth')?.authType` to determine authentication type ('apikey' vs 'jwt') in backend endpoints
Applied to files:
supabase/functions/_backend/files/preview.tscloudflare_workers/files/index.ts
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to cloudflare_workers/api/index.ts : API Worker (port 8787) routes: `/bundle`, `/app`, `/device`, `/channel`, `/private/*`, `/triggers`
Applied to files:
supabase/functions/_backend/files/preview.tscloudflare_workers/files/wrangler.jsonccloudflare_workers/files/index.ts
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to cloudflare_workers/plugin/index.ts : Plugin Worker (port 8788) routes: `/updates`, `/channel_self`, `/stats`
Applied to files:
cloudflare_workers/files/wrangler.jsonccloudflare_workers/files/index.ts
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to cloudflare_workers/files/index.ts : Files Worker (port 8789) handles file upload/download operations
Applied to files:
cloudflare_workers/files/index.ts
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to cloudflare_workers/{api,plugin,files}/index.ts : Cloudflare Workers are split across three ports: API Worker (8787), Plugin Worker (8788), Files Worker (8789); see routing in `cloudflare_workers/{api,plugin,files}/index.ts`
Applied to files:
cloudflare_workers/files/index.ts
📚 Learning: 2025-12-05T17:34:25.556Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-05T17:34:25.556Z
Learning: Applies to supabase/functions/**/*.ts : Supabase Edge Functions use Deno runtime
Applied to files:
cloudflare_workers/files/index.ts
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to supabase/functions/_backend/**/*.{ts,js} : Backend code must be placed in `supabase/functions/_backend/` as shared code deployed to Cloudflare Workers (API/Plugin/Files workers), Supabase Edge Functions, and other platforms
Applied to files:
cloudflare_workers/files/index.ts
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to supabase/functions/**/*.{ts,js} : Backend ESLint must pass before commit; run `bun lint:backend` for backend files
Applied to files:
cloudflare_workers/files/index.ts
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Use shared backend code from `supabase/functions/_backend/` across all deployment platforms; never create platform-specific implementations outside this directory
Applied to files:
cloudflare_workers/files/index.ts
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to supabase/functions/_backend/**/*.{ts,js} : Use `middlewareAPISecret` for internal API endpoints and `middlewareKey` for external API keys; validate against `owner_org` in the `apikeys` table
Applied to files:
cloudflare_workers/files/index.ts
🧬 Code graph analysis (2)
supabase/functions/_backend/files/preview.ts (4)
supabase/functions/_backend/utils/cache.ts (1)
CacheHelper(21-81)supabase/functions/_backend/utils/utils.ts (1)
backgroundTask(145-158)supabase/functions/_backend/utils/hono.ts (2)
MiddlewareKeyVariables(27-41)simpleError(247-249)supabase/functions/_backend/utils/logging.ts (1)
cloudlog(3-15)
cloudflare_workers/files/index.ts (1)
supabase/functions/_backend/files/preview.ts (1)
handlePreviewRequest(143-319)
🪛 GitHub Actions: Run tests
supabase/functions/_backend/files/preview.ts
[error] 165-165: ESLint: 'cachedAuth' is defined but never reassigned. Use 'const' instead (prefer-const).
🔇 Additional comments (9)
cloudflare_workers/files/index.ts (1)
3-3: LGTM! Context preservation improvement.The change from importing a nested Hono app to directly importing and calling
handlePreviewRequest(c)correctly preserves the parent app context (requestId, env bindings) as intended by the PR objectives.Also applies to: 24-25
cloudflare_workers/files/wrangler.jsonc (1)
54-54: LGTM! Essential routing fix for nested paths.Adding the trailing
/*to the preview subdomain route patterns ensures that requests to nested paths (e.g.,/assets/app.js) are properly matched by the worker. This is critical for serving bundle assets correctly.Also applies to: 150-150, 202-202
supabase/functions/_backend/files/preview.ts (7)
11-79: LGTM! Well-designed caching infrastructure.The caching layer for preview authorization and bundle metadata is well-structured with:
- Proper TTL configuration (60 seconds)
- Background task pattern for non-blocking cache writes
- Null-safe cache availability checks
- Appropriate use of
CacheHelperfrom the utilsThis will significantly reduce database load for repeated preview requests.
163-199: LGTM! Efficient cache-first authorization flow.The cache-first approach for app preview authorization is well-implemented:
- Checks cache before hitting the database
- Uses case-insensitive matching (
ilike) for app_id to handle frontend lowercasing- Properly caches both positive and negative authorization results
- Background cache population doesn't block the response
201-226: LGTM! Bundle metadata caching reduces redundant queries.The bundle info caching correctly stores encryption and manifest status, eliminating repeated database queries for the same bundle metadata during the preview session.
250-276: LGTM! Excellent query optimization.Replacing sequential queries with a single query using the
INclause for all possible file paths (including common prefixes and Brotli variants) is a significant performance improvement. The limit(1) ensures efficient execution.
278-285: LGTM! Correct MIME type detection for Brotli files.Properly detecting the Brotli extension and using the original filename (without
.br) for content-type detection ensures correctContent-Typeheaders are sent to the browser.Also applies to: 304-304
308-308: LGTM! Appropriate cache headers for immutable assets.Setting
Cache-Control: public, max-age=31536000, immutableis correct for versioned bundle assets that never change. This maximizes browser caching and reduces server load.
322-329: LGTM! Useful utility for preview URL generation.The
generatePreviewUrlhelper properly encodes app_id (replacing dots with double underscores) and constructs environment-specific preview URLs. This will be helpful for other parts of the codebase that need to generate preview links.
| .single() | ||
| // Check cache for app preview authorization first | ||
| let actualAppId: string | ||
| let cachedAuth = await getPreviewAuth(c, appId) |
There was a problem hiding this comment.
Fix linting error: use const instead of let.
The variable cachedAuth is never reassigned, so it should be declared with const instead of let per ESLint rules.
🔧 Proposed fix
- let cachedAuth = await getPreviewAuth(c, appId)
+ const cachedAuth = await getPreviewAuth(c, appId)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let cachedAuth = await getPreviewAuth(c, appId) | |
| const cachedAuth = await getPreviewAuth(c, appId) |
🧰 Tools
🪛 GitHub Actions: Run tests
[error] 165-165: ESLint: 'cachedAuth' is defined but never reassigned. Use 'const' instead (prefer-const).
🤖 Prompt for AI Agents
In @supabase/functions/_backend/files/preview.ts at line 165, The variable
cachedAuth in preview.ts is never reassigned; replace the mutable declaration
"let cachedAuth = await getPreviewAuth(c, appId)" with an immutable one by using
const instead of let (update the declaration where cachedAuth is assigned from
getPreviewAuth(c, appId) to use const).
| const manifestEntry = manifestEntries[0] | ||
| const isBrotli = manifestEntry.file_name.endsWith('.br') | ||
| // For MIME type detection, use the original filename without .br extension | ||
| const actualFileName = isBrotli ? manifestEntry.file_name.slice(0, -3) : manifestEntry.file_name | ||
|
|
||
| if (manifestEntry.file_name !== filePath) { | ||
| cloudlog({ requestId: c.get('requestId'), message: 'found file with prefix', originalPath: filePath, foundPath: manifestEntry.file_name, isBrotli }) | ||
| } | ||
|
|
||
| // If the file is brotli compressed, redirect to public R2 URL | ||
| // Cloudflare Workers strip Content-Encoding: br header, but direct R2 serves it correctly | ||
| // No need to fetch the object, just redirect | ||
| if (isBrotli) { | ||
| const publicR2Url = `https://storage.capgo.app/${manifestEntry.s3_path}` | ||
| cloudlog({ requestId: c.get('requestId'), message: 'redirecting to R2 for brotli file', filePath: manifestEntry.file_name, publicR2Url }) | ||
| return c.redirect(publicR2Url, 302) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for storage.capgo.app URL pattern
echo "=== Searching for storage.capgo.app usage ==="
rg -n -C3 'storage\.capgo\.app' --type=ts --type=js
# Search for environment-specific storage URLs or configurations
echo -e "\n=== Searching for environment-specific storage URLs ==="
rg -n -C3 'ATTACHMENT_BUCKET|R2.*URL|storage.*url' --type=ts --type=js -i
# Check for environment configuration files
echo -e "\n=== Checking for .env files or environment configurations ==="
fd -e env -e config | head -20
cat .env* 2>/dev/null | grep -i 'storage\|r2\|bucket\|capgo' || echo "No .env files found or no storage config"
# Search for manifest-related code to understand s3_path source
echo -e "\n=== Searching for manifest and s3_path usage ==="
rg -n 's3_path' --type=ts --type=js -A2 -B2Repository: Cap-go/capgo
Length of output: 20078
Use environment-configured domain for R2 redirect instead of hardcoded storage.capgo.app.
The public R2 URL is hardcoded to https://storage.capgo.app/ which will not work in non-prod environments (preprod, alpha). While s3_path is correctly stored in the database and is environment-agnostic, the domain should be configurable via environment variable, similar to how ATTACHMENT_BUCKET is configured. This ensures brotli file redirects work across all deployment environments.
🤖 Prompt for AI Agents
In @supabase/functions/_backend/files/preview.ts around lines 278 - 294, Replace
the hardcoded R2 domain used when redirecting brotli files with a configurable
environment value: read the environment/config variable used for buckets (e.g.,
ATTACHMENT_BUCKET or add a new R2_PUBLIC_DOMAIN) and build publicR2Url from that
value plus manifestEntry.s3_path instead of "https://storage.capgo.app/"; update
the cloudlog and the c.redirect call to use the constructed URL, and provide a
sensible default fallback to the existing domain if the env var is not set so
redirects remain functional in prod and non-prod environments.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a96aed6429
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // If the file is brotli compressed, redirect to public R2 URL | ||
| // Cloudflare Workers strip Content-Encoding: br header, but direct R2 serves it correctly | ||
| // No need to fetch the object, just redirect | ||
| if (isBrotli) { | ||
| const publicR2Url = `https://storage.capgo.app/${manifestEntry.s3_path}` | ||
| cloudlog({ requestId: c.get('requestId'), message: 'redirecting to R2 for brotli file', filePath: manifestEntry.file_name, publicR2Url }) | ||
| return c.redirect(publicR2Url, 302) |
There was a problem hiding this comment.
Honor Accept-Encoding before redirecting to .br assets
The new brotli handling always redirects to the .br object when it’s the manifest hit, but there’s no check that the request’s Accept-Encoding includes br. For clients without brotli support (older browsers, some WebViews, CLI fetches), the redirected storage.capgo.app response will carry Content-Encoding: br and fail to decode, breaking preview loads. This is introduced by the unconditional redirect; please only select the .br variant when Accept-Encoding advertises brotli and fall back to the uncompressed entry otherwise.
Useful? React with 👍 / 👎.
R2 doesn't store Content-Encoding metadata, so redirect to public R2 URL doesn't work (browser receives raw brotli bytes without knowing to decompress). Use node:zlib brotliDecompressSync to decompress server-side before serving. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
|
* Fix bundle preview system with caching and proper routing - Add 60s TTL caching for app authorization and bundle info to reduce DB queries - Change wrangler route patterns from `*.preview.capgo.app` to `*.preview.capgo.app/*` to match all paths - Export handlePreviewRequest directly instead of using separate Hono app to preserve context - Add .br (brotli) file lookup with server-side decompression using node:zlib - Detect MIME types correctly without .br extension suffix 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * Use server-side brotli decompression instead of redirect R2 doesn't store Content-Encoding metadata, so redirect to public R2 URL doesn't work (browser receives raw brotli bytes without knowing to decompress). Use node:zlib brotliDecompressSync to decompress server-side before serving. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>



Summary
Fixed the bundle preview system that wasn't serving files correctly. Added 60s TTL caching for app authorization and bundle info to reduce database queries. Fixed wrangler route patterns to match all paths and changed from separate Hono app to direct handler to preserve context.
Changes
*.preview.capgo.appto*.preview.capgo.app/*to match all nested pathspreview.fetch()to exportinghandlePreviewRequest()directly to preserve parent app context (requestId, env bindings)Test plan
https://ee__forgr__capacitor_go-223694.preview.capgo.app/now serves files correctly🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Performance Improvements
Bug Fixes
Enhancements
Observability
✏️ Tip: You can customize this high-level summary in your review settings.