fix(docs): add auth to root cache key #4655
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
| const getRootCached = (cacheConfig: Required<CacheConfig>) => | ||
| cache(async (domainKey: string, authState: AuthState, authConfig: AuthEdgeConfig | undefined) => { | ||
| const authCacheKey = authState.authed | ||
| ? `authed:${authState.user?.roles?.sort().join(",") || "noroles"}` |
There was a problem hiding this comment.
| ? `authed:${authState.user?.roles?.sort().join(",") || "noroles"}` | |
| ? `authed:${authState.user?.roles?.slice().sort().join(",") || "noroles"}` |
The code mutates the original authState.user.roles array in-place by calling .sort() directly on it. This should create a copy before sorting to avoid mutating shared state.
View Details
Analysis
Array mutation bug in cache key generation mutates authState.user.roles
What fails: getRootCached() in packages/commons/docs-loader/src/readonly-docs-loader.ts:567 mutates authState.user.roles array when generating cache keys by calling .sort() directly on the original array
How to reproduce:
const authState = { authed: true, user: { roles: ["admin", "user", "editor"] } };
// Line 567: authState.user?.roles?.sort().join(",")
console.log(authState.user.roles); // ["admin", "editor", "user"] - mutated!Result: The same mutated authState object is passed to pruneWithAuthState() for authorization decisions, causing authorization logic to see sorted roles instead of original JWT order
Expected: Authorization should use original role order from JWT; cache key generation should not mutate shared state
Fix: Create array copy before sorting: authState.user?.roles?.slice().sort().join(",")
| ) | ||
| ), | ||
| getRoot: async () => getRootCached(config)(domainKey, await getAuthState(), await authConfig), | ||
| getRoot: async () => getRoot(domainKey, await getAuthState(), await authConfig, config), |
There was a problem hiding this comment.
| getRoot: async () => getRoot(domainKey, await getAuthState(), await authConfig, config), | |
| getRoot: async () => getRootCached(config)(domainKey, await getAuthState(), await authConfig), |
The change from calling getRootCached() to calling getRoot() directly removes critical caching layers from the RBAC pruning operation, causing a significant performance regression where the expensive tree pruning will be re-executed on every request instead of being cached.
View Details
Analysis
Performance regression: RBAC pruning bypasses caching in getRoot() call
What fails: The change from getRootCached() to direct getRoot() call in readonly-docs-loader.ts line 1178 removes caching layers, causing expensive RBAC tree pruning to execute on every request instead of being cached.
How to reproduce:
// Multiple calls to the same domain with identical auth state
const loader = createCachedDocsLoader(config);
await loader.getRoot(); // First call - executes pruneWithAuthState()
await loader.getRoot(); // Second call - executes pruneWithAuthState() AGAIN (should be cached)Result: pruneWithAuthState() runs fresh on every request, traversing entire navigation tree and applying RBAC rules repeatedly
Expected: RBAC pruning should be cached per (domain + auth state) combination using both React's cache() (request-level deduplication) and Next.js unstable_cache() (persistent caching) as provided by getRootCached()
Technical details:
getRootCached()wraps the entiregetRoot()operation (base tree + pruning) with dual caching layers- Direct
getRoot()call only caches base tree viaunsafe_getRootCached(), but pruning runs uncached - Performance impact scales with documentation tree size (O(n) nodes per request vs O(1) cached)
| // Write to local file for debugging | ||
| try { | ||
| const timestamp = new Date().toISOString().replace(/[:.]/g, '-'); | ||
| const filename = `/tmp/docs-definition-${cleanDomain.replace(/[^a-zA-Z0-9]/g, '-')}-${timestamp}.json`; | ||
| await writeFile(filename, JSON.stringify(json, null, 2)); | ||
| console.log(`💾 [DEBUG] Saved docs definition to: ${filename}`); | ||
| } catch (writeError) { | ||
| console.error("Failed to write docs definition to file:", writeError); |
There was a problem hiding this comment.
| // Write to local file for debugging | |
| try { | |
| const timestamp = new Date().toISOString().replace(/[:.]/g, '-'); | |
| const filename = `/tmp/docs-definition-${cleanDomain.replace(/[^a-zA-Z0-9]/g, '-')}-${timestamp}.json`; | |
| await writeFile(filename, JSON.stringify(json, null, 2)); | |
| console.log(`💾 [DEBUG] Saved docs definition to: ${filename}`); | |
| } catch (writeError) { | |
| console.error("Failed to write docs definition to file:", writeError); | |
| // Write to local file for debugging (development only) | |
| if (process.env.NODE_ENV === "development") { | |
| try { | |
| const timestamp = new Date().toISOString().replace(/[:.]/g, '-'); | |
| const filename = `/tmp/docs-definition-${cleanDomain.replace(/[^a-zA-Z0-9]/g, '-')}-${timestamp}.json`; | |
| await writeFile(filename, JSON.stringify(json, null, 2)); | |
| console.log(`💾 [DEBUG] Saved docs definition to: ${filename}`); | |
| } catch (writeError) { | |
| console.error("Failed to write docs definition to file:", writeError); | |
| } |
Debug code writes the full docs definition JSON to /tmp/ on every S3 load, which can cause disk space issues and I/O performance problems in production.
View Details
Analysis
Debug code writes docs definition to /tmp/ in production without environment check
What fails: loadDocsDefinitionFromS3() in packages/commons/docs-server/src/loadDocsDefinitionFromS3.ts writes JSON files to /tmp/ on every successful S3 load without checking NODE_ENV, causing disk space consumption in production
How to reproduce:
# Call loadDocsDefinitionFromS3 in production environment
NODE_ENV=production node -e "
const fs = require('fs').promises;
// Simulate the exact debug code from lines 62-70
(async () => {
const json = {test: 'large docs data'};
const cleanDomain = 'example.com';
const timestamp = new Date().toISOString().replace(/[:.]/g, '-');
const filename = \`/tmp/docs-definition-\ Result: Creates files like /tmp/docs-definition-example-com-2025-11-03T22-43-57-118Z.json (0.4+ MB each) that accumulate indefinitely
Expected: Debug file writing should only occur in development environment, similar to other conditional debug code in the codebase (e.g. packages/commons/docs-server/src/analytics/posthog.ts:42)
Cache getRoot by auth