Conversation
|
Caution Review failedPull request was closed or merged during review 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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (7)
📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (4)
📝 WalkthroughWalkthroughRemoves two static pages and related polyfills/deps; adds Next.js middleware cache policy, entry-age post caching and refinement, event-loop watchdog and sanitization, request timeouts, SDK/search tweaks, tests, and a Cloudflare purge script. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as "Client"
participant Middleware as "Next Middleware"
participant CachePolicy as "Cache Policy\nResolver"
participant PostAgeCache as "Post-Age\nCache"
participant HiveRPC as "Hive RPC"
participant Response as "Response"
Client->>Middleware: GET /category/@author/permlink
Middleware->>CachePolicy: getCachePolicyForPath(path)
CachePolicy-->>Middleware: { tier: "entry", ... }
alt Unauthenticated & entry tier
Middleware->>Middleware: parseEntryUrl(path)
Middleware->>PostAgeCache: getCachedPostCreatedMs(author, permlink)
alt Cached timestamp
PostAgeCache-->>Middleware: timestamp(ms)
Middleware->>CachePolicy: getEntryTierForAge(ageMs)
CachePolicy-->>Middleware: refined tier
else Cache miss (undefined)
PostAgeCache-->>Middleware: undefined
Middleware->>HiveRPC: refreshPostCreatedMs(...) (via event.waitUntil)
HiveRPC-->>PostAgeCache: created timestamp -> cache
else Negative cache (null)
PostAgeCache-->>Middleware: null
end
end
Middleware->>Middleware: buildCacheControlHeader(policy, isLoggedIn)
Middleware->>Response: set Cache-Control + x-cache-tier
Response-->>Client: Response with cache directives
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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.
♻️ Duplicate comments (2)
apps/web/src/middleware.ts (2)
35-42:⚠️ Potential issue | 🟠 MajorHeader-based Server Action bypass is too broad.
next-actionis just a client-supplied header, so anyPOST/PATCHto a page route can use it to skip the 405 fast-path and re-enter the expensive Next.js pipeline this guard is meant to avoid. If Server Actions need an exception later, it should be scoped to actual action endpoints, not raw header presence.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/middleware.ts` around lines 35 - 42, The middleware currently treats any request with the next-action header as a legitimate Server Action (isServerAction), allowing client-supplied headers to bypass the 405 fast-path; change this to only accept Server Actions for known action endpoints by replacing the header-only check with a route-scoped validation (e.g., add a helper isServerActionRoute(pathname) or match a specific action route pattern) and only treat the request as a server action if both the pathname matches that action route and, optionally, the header/value validation passes; update the isServerAction usage in the write-method guard (where isWriteMethod, isApiRoute, isPlausibleProxy, isServerAction are checked) to use this stricter isServerActionRoute check instead of request.headers.has("next-action").
91-92:⚠️ Potential issue | 🟠 MajorPublic cache headers are still set before the real status exists.
NextResponse.next()is only a passthrough 200 here, soapplyCacheHeaders()derives a publicCache-Controlpurely from the pathname. Dynamic entry/profile routes that later resolve to 404/500/redirects will keep that header, which can make misses and transient failures sticky in shared cache.Also applies to: 128-129
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/middleware.ts` around lines 91 - 92, The middleware is applying public cache headers immediately after calling NextResponse.next(), which is a passthrough 200 and can cause incorrect public caching for routes that later resolve to 404/500/redirect; stop setting cache headers for the NextResponse.next() passthrough. Change the code so applyCacheHeaders(request, response, path, event) is only invoked for responses the middleware actively constructs (e.g., branches that return NextResponse.rewrite(), NextResponse.redirect(), or any response with a final status), and remove or guard the applyCacheHeaders call for the NextResponse.next() path (use the response type/branch checks around NextResponse.rewrite/redirect or only call applyCacheHeaders after you have a non-passthrough/final response).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/web/src/middleware.ts`:
- Around line 35-42: The middleware currently treats any request with the
next-action header as a legitimate Server Action (isServerAction), allowing
client-supplied headers to bypass the 405 fast-path; change this to only accept
Server Actions for known action endpoints by replacing the header-only check
with a route-scoped validation (e.g., add a helper isServerActionRoute(pathname)
or match a specific action route pattern) and only treat the request as a server
action if both the pathname matches that action route and, optionally, the
header/value validation passes; update the isServerAction usage in the
write-method guard (where isWriteMethod, isApiRoute, isPlausibleProxy,
isServerAction are checked) to use this stricter isServerActionRoute check
instead of request.headers.has("next-action").
- Around line 91-92: The middleware is applying public cache headers immediately
after calling NextResponse.next(), which is a passthrough 200 and can cause
incorrect public caching for routes that later resolve to 404/500/redirect; stop
setting cache headers for the NextResponse.next() passthrough. Change the code
so applyCacheHeaders(request, response, path, event) is only invoked for
responses the middleware actively constructs (e.g., branches that return
NextResponse.rewrite(), NextResponse.redirect(), or any response with a final
status), and remove or guard the applyCacheHeaders call for the
NextResponse.next() path (use the response type/branch checks around
NextResponse.rewrite/redirect or only call applyCacheHeaders after you have a
non-passthrough/final response).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 71dbdfbc-c71e-4822-a864-f7c0007cefa7
📒 Files selected for processing (6)
apps/web/src/middleware.tspackages/sdk/src/modules/core/config.tspackages/sdk/src/modules/search/queries/get-search-api-infinite-query-options.tspackages/sdk/src/modules/search/queries/get-search-query-options.tspackages/sdk/src/modules/search/queries/get-similar-entries-query-options.tspackages/sdk/src/modules/search/requests.ts
✅ Files skipped from review due to trivial changes (1)
- packages/sdk/src/modules/core/config.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/sdk/src/modules/search/queries/get-search-api-infinite-query-options.ts
- packages/sdk/src/modules/search/queries/get-similar-entries-query-options.ts
- packages/sdk/src/modules/search/requests.ts
- packages/sdk/src/modules/search/queries/get-search-query-options.ts
Summary by CodeRabbit
New Features
Bug Fixes
Chores