feat: Implement cache - #750
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eb48923de5
ℹ️ 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".
| const resolved = await loadStaffPermissions(c, { type, user }); | ||
| await writeStaffPermissions(c, { type, userId: user.id }, resolved); |
There was a problem hiding this comment.
When a cold permission lookup overlaps a role or staff mutation, loadStaffPermissions can resolve the old grants, the mutation can invalidate them, and this later write then repopulates the cache with that stale result. Per-user invalidation rewrites the deleted key, while global invalidation is even more vulnerable because writeStaffPermissions rereads the new epoch and writes the old result into the new generation; subsequent requests can therefore retain revoked privileges for the 60-second TTL. Carry the generation/version used by the initial read through the load and only write if it is still current, including a per-user generation for targeted invalidations.
Useful? React with 👍 / 👎.
| // out its `cacheLife`. `updateTag` rather than `revalidateTag` because this is | ||
| // a Server Action: the admin who pressed the button sees the new feed on the | ||
| // refresh it triggers, not one navigation later. | ||
| updateTag(SEARCH_FEED_TAG); |
There was a problem hiding this comment.
The rebuild endpoint only inserts a database queue row, which is processed later by the process-queue cron job, so this invalidation happens before the index has changed. If the action-triggered refresh or any other feed request runs before the worker finishes, it repopulates the cache from the old or partially rebuilt index, and the worker never invalidates the tag again after completion; /search and /discover then remain stale until cacheLife("minutes") expires. Trigger the invalidation after the queued rebuild task successfully completes.
AGENTS.md reference: AGENTS.md:L28-L30
Useful? React with 👍 / 👎.
| * Next's longest built-in profile (`max`), so an entry that outlives its marker | ||
| * has already been forced to refresh on age alone. | ||
| */ | ||
| const TAG_RETENTION_SECONDS = 60 * 60 * 24 * 30; |
There was a problem hiding this comment.
For a valid custom cacheLife whose revalidate and expire exceed 30 days, a hard tag invalidation marker disappears here while the corresponding "use cache" entry is still retained in Redis. If that entry is not requested and replaced during the marker window, a later request no longer sees evidence of the invalidation and can serve the old value again; this can resurrect deleted or unpublished content. Marker retention needs to cover the maximum lifetime of every tagged entry, or expired entries must be deleted directly.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 384fc09f72
ℹ️ 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".
| const cached = await readStaffPermissions(c, { type, userId: user.id }); | ||
| if (cached) return cached; |
There was a problem hiding this comment.
If Redis is unavailable while a role or staff mutation runs, deleteSystem/setSystem silently drops the invalidation after the database mutation has succeeded. When Redis recovers within the 60-second TTL, this fast path resumes returning the old cached grants, so an administrator whose permissions were revoked can temporarily regain them. Authorization caching needs a durable generation/version or another mechanism that prevents a failed best-effort invalidation from making old grants valid again.
Useful? React with 👍 / 👎.
| export const getAnnouncements = async (locale: string) => { | ||
| 'use cache'; | ||
| cacheLife('hours'); | ||
| cacheTag('announcements'); | ||
| await connection(); |
There was a problem hiding this comment.
Developers copying this documented example will call connection() from inside a "use cache" scope, where Next.js runtime request APIs are not allowed, so the example fails instead of producing cached public data. The implementation in views/search/fetch-feed.ts already demonstrates the required ordering by awaiting connection() in the uncached wrapper before calling the cached function; the documentation should show the same split.
Useful? React with 👍 / 👎.
1 participant
Improving Documentation
pnpm lint:fixto fix formatting issues before opening the PR.Description
What?
Why?