Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
e6d5b28
feat(cache): add in-memory tier, size limits, hex sharding, and obser…
vibegui Mar 18, 2026
8c4e5d1
feat(cache): add comprehensive cache observability metrics
vibegui Mar 18, 2026
9e3c534
fix(cache): preserve response status, validate env vars, fix metric o…
vibegui Mar 18, 2026
7ba9241
fix(cache): address code review findings
vibegui Mar 18, 2026
0010914
fix(cache): address correctness issues in env parsing, shard init, LR…
vibegui Mar 18, 2026
23b5ce1
feat(cache): lazy re-index disk entries into LRU on pod restart
vibegui Mar 18, 2026
b3696ff
perf(cache): eliminate hot-path allocations and fix correctness bugs
vibegui Mar 18, 2026
60d5f50
fix(cache): async mkdir and single-tier cache-tier header
vibegui Mar 18, 2026
ad86a9b
fix(cache): reject oversized entries before writing to disk
vibegui Mar 18, 2026
7b2e290
test(cache): add tests for tiered cache, in-memory cache, and lazy re…
vibegui Mar 18, 2026
9ed8346
fix(cache): evict stale disk entry when oversized write is rejected
vibegui Mar 18, 2026
464e1f0
fix(cache): NaN guard, oversized check in L1, memoize LRU open()
vibegui Mar 18, 2026
093066e
perf(cache): skip body read in L1 put when Content-Length exceeds limit
vibegui Mar 18, 2026
ff09a48
feat(cache): extend default stale window to 1h, add STALE_WINDOW_S en…
vibegui Mar 18, 2026
84560bf
fix(cache): bump STALE_TTL_PERIOD default from 30s to 1h
vibegui Mar 18, 2026
2c062bd
feat(cache): bot write guard, L1 admission filter, eviction logging
vibegui Mar 18, 2026
ac7edcd
fix(cache): update inMemoryCache tests for admission filter
vibegui Mar 18, 2026
7f6bbac
feat(cache): write rate warning and disk fill warning
vibegui Mar 18, 2026
9f84eee
fix(cache): separate singleFlight key for bot requests
vibegui Mar 18, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 63 additions & 2 deletions blocks/loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,21 @@ const stats = {
unit: "ms",
valueType: ValueType.DOUBLE,
}),
cacheEntrySize: meter.createHistogram("loader_cache_entry_size", {
description: "size of cached loader responses in bytes",
unit: "bytes",
valueType: ValueType.DOUBLE,
}),
bgRevalidation: meter.createHistogram("loader_bg_revalidation", {
description: "duration of background stale-while-revalidate calls",
unit: "ms",
valueType: ValueType.DOUBLE,
}),
lruEviction: meter.createCounter("loader_cache_eviction", {
description: "LRU cache evictions",
unit: "1",
valueType: ValueType.DOUBLE,
}),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Wire loader_cache_eviction to the real eviction hook.

This counter is declared here but never incremented. In the provided cache stack, the only actual eviction signal is lru_cache_eviction in runtime/caches/lrucache.ts, so loader_cache_eviction will stay flat as written.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@blocks/loader.ts` around lines 159 - 163, The loader metric lruEviction
(meter counter "loader_cache_eviction") is declared but never incremented; wire
it to the real eviction hook by incrementing that counter when an eviction
happens in the LRU implementation (the lru_cache_eviction signal in
runtime/caches/lrucache.ts). Locate the eviction path in
runtime/caches/lrucache.ts (the function/method that emits or handles
lru_cache_eviction) and call or expose a handler that increments
loader_cache_eviction (lruEviction.increment(...) or equivalent) so every real
eviction increments the loader metric.

};

let maybeCache: Cache | undefined;
Expand All @@ -155,6 +170,9 @@ caches?.open("loader")
.catch(() => maybeCache = undefined);

const MAX_AGE_S = parseInt(Deno.env.get("CACHE_MAX_AGE_S") ?? "60"); // 60 seconds
const CACHE_MAX_ENTRY_SIZE = parseInt(
Deno.env.get("CACHE_MAX_ENTRY_SIZE") ?? "2097152", // 2 MB
) || 2097152;

// Reuse TextEncoder instance to avoid repeated instantiation
const textEncoder = new TextEncoder();
Expand Down Expand Up @@ -248,7 +266,14 @@ const wrapLoader = (
!shouldNotCache && ctx.vary?.push(cacheKeyValue);

status = "bypass";
stats.cache.add(1, { status, loader });
const bypassReason = isCacheNoStore
? "no-store"
: isCacheNoCache
? "no-cache"
: isCacheKeyNull
? "null-key"
: "disabled";
stats.cache.add(1, { status, loader, reason: bypassReason });

RequestContext?.signal?.throwIfAborted();
return await handler(props, req, ctx);
Expand Down Expand Up @@ -297,6 +322,15 @@ const wrapLoader = (
// Serialize and encode once on the main thread.
const jsonStringEncoded = textEncoder.encode(JSON.stringify(json));

// Skip caching oversized entries to protect disk and memory
if (jsonStringEncoded.length > CACHE_MAX_ENTRY_SIZE) {
return json;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
}

if (OTEL_ENABLE_EXTRA_METRICS) {
stats.cacheEntrySize.record(jsonStringEncoded.length, { loader });
}

const expires = new Date(Date.now() + (cacheMaxAge * 1e3))
.toUTCString();
const headerPairs: [string, string][] = [
Expand All @@ -305,7 +339,7 @@ const wrapLoader = (
["Content-Length", "" + jsonStringEncoded.length],
];

// Cache write goes through the full chain (LRU → filesystem)
// Cache write goes through the full chain (LRU → in-memory → filesystem)
// so the LRU registers the key for fast match lookups.
// The filesystem layer offloads the actual I/O to a worker thread
// when DECO_CACHE_WRITE_WORKER=true.
Expand Down Expand Up @@ -336,13 +370,40 @@ const wrapLoader = (
status = "stale";
stats.cache.add(1, { status, loader });

const bgStart = performance.now();
bgFlights.do(request.url, callHandlerAndCache)
.then(() => {
if (OTEL_ENABLE_EXTRA_METRICS) {
stats.bgRevalidation.record(
performance.now() - bgStart,
{ loader },
);
}
})
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
.catch((error) => logger.error(`loader error ${error}`));
} else {
status = "hit";
stats.cache.add(1, { status, loader });
}

if (OTEL_ENABLE_EXTRA_METRICS) {
const cl = parseInt(
matched.headers.get("Content-Length") ?? "0",
);
if (cl > 0) {
stats.cacheEntrySize.record(cl, { loader, status });
}
}

if (OTEL_ENABLE_EXTRA_METRICS) {
const parseStart = performance.now();
const result = await matched.json();
stats.latency.record(performance.now() - parseStart, {
loader,
status: "json_parse",
});
return result;
}
return await matched.json();
};

Expand Down
12 changes: 10 additions & 2 deletions runtime/caches/cacheWriteWorker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,12 @@ function generateCombinedBuffer(
return buf;
}

function shardedPath(cacheDir: string, key: string): string {
const l1 = key.substring(0, 2);
const l2 = key.substring(2, 4);
return `${cacheDir}/${l1}/${l2}/${key}`;
}

// --- Message handler ---

export interface CacheWriteMessage {
Expand Down Expand Up @@ -85,8 +91,10 @@ self.onmessage = async (e: MessageEvent<CacheWriteMessage>) => {
// Combine into single buffer
const buffer = generateCombinedBuffer(body, headersBytes);

// Write to filesystem
const filePath = `${cacheDir}/${cacheKey}`;
// Write to filesystem (with hex sharding for directory distribution)
const filePath = shardedPath(cacheDir, cacheKey);
const dir = filePath.substring(0, filePath.lastIndexOf("/"));
ensureCacheDir(dir);
await Deno.writeFile(filePath, buffer);
} catch (err) {
console.error("[cache-write-worker] error:", err);
Expand Down
6 changes: 6 additions & 0 deletions runtime/caches/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@ export const withInstrumentation = (
const result = getCacheStatus(isMatch);

span.setAttribute("cache_status", result);
if (isMatch) {
const cl = isMatch.headers.get("Content-Length");
if (cl) span.setAttribute("content_length", parseInt(cl));
const tier = isMatch.headers.get("X-Cache-Tier");
if (tier) span.setAttribute("cache_tier", parseInt(tier));
}
cacheHit.add(1, {
result,
engine,
Expand Down
23 changes: 19 additions & 4 deletions runtime/caches/fileSystem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,16 @@ import {
const FILE_SYSTEM_CACHE_DIRECTORY =
Deno.env.get("FILE_SYSTEM_CACHE_DIRECTORY") ?? "/tmp/deco_cache";

const CACHE_MAX_ENTRY_SIZE = parseInt(
Deno.env.get("CACHE_MAX_ENTRY_SIZE") ?? "2097152", // 2 MB
) || 2097152;
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.

@cubic-dev-ai cubic-dev-ai Bot Mar 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Using || 2097152 breaks the explicit CACHE_MAX_ENTRY_SIZE=0 case by restoring the default 2 MB limit.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At runtime/caches/fileSystem.ts, line 16:

<comment>Using `|| 2097152` breaks the explicit `CACHE_MAX_ENTRY_SIZE=0` case by restoring the default 2 MB limit.</comment>

<file context>
@@ -13,7 +13,7 @@ const FILE_SYSTEM_CACHE_DIRECTORY =
 const CACHE_MAX_ENTRY_SIZE = parseInt(
   Deno.env.get("CACHE_MAX_ENTRY_SIZE") ?? "2097152", // 2 MB
-);
+) || 2097152;
 
 const initializedShardDirs = new Set<string>();
</file context>
Fix with Cubic


function shardedPath(cacheDir: string, key: string): string {
const l1 = key.substring(0, 2);
const l2 = key.substring(2, 4);
return `${cacheDir}/${l1}/${l2}/${key}`;
}

// Reuse TextEncoder instance to avoid repeated instantiation
const textEncoder = new TextEncoder();

Expand Down Expand Up @@ -121,8 +131,9 @@ function createFileSystemCache(): CacheStorage {
if (!isCacheInitialized) {
await assertCacheDirectory();
}
const filePath = `${FILE_SYSTEM_CACHE_DIRECTORY}/${key}`;

const filePath = shardedPath(FILE_SYSTEM_CACHE_DIRECTORY, key);
const dir = filePath.substring(0, filePath.lastIndexOf("/"));
await Deno.mkdir(dir, { recursive: true }).catch(() => {});
await Deno.writeFile(filePath, responseArray);
return;
}
Expand All @@ -132,8 +143,12 @@ function createFileSystemCache(): CacheStorage {
await assertCacheDirectory();
}
try {
const filePath = `${FILE_SYSTEM_CACHE_DIRECTORY}/${key}`;
const filePath = shardedPath(FILE_SYSTEM_CACHE_DIRECTORY, key);
const fileContent = await Deno.readFile(filePath);
if (fileContent.length > CACHE_MAX_ENTRY_SIZE) {
Deno.remove(filePath).catch(() => {});
return null;
}
return fileContent;
} catch (_err) {
const err = _err as { code?: string };
Expand All @@ -151,7 +166,7 @@ function createFileSystemCache(): CacheStorage {
await assertCacheDirectory();
}
try {
const filePath = `${FILE_SYSTEM_CACHE_DIRECTORY}/${key}`;
const filePath = shardedPath(FILE_SYSTEM_CACHE_DIRECTORY, key);
await Deno.remove(filePath);
return true;
} catch (err) {
Expand Down
85 changes: 85 additions & 0 deletions runtime/caches/inMemoryCache.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
import { LRUCache } from "npm:lru-cache@10.2.0";
import {
assertCanBeCached,
assertNoOptions,
baseCache,
withCacheNamespace,
} from "./utils.ts";

const MEMORY_CACHE_MAX_SIZE = parseInt(
Deno.env.get("MEMORY_CACHE_MAX_SIZE") ?? "268435456", // 256 MB
);
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
Outdated
const MEMORY_CACHE_MAX_ITEMS = parseInt(
Deno.env.get("MEMORY_CACHE_MAX_ITEMS") ?? "2048",
);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

interface CacheEntry {
body: Uint8Array;
headers: [string, string][];
status: number;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

function createInMemoryCache(): CacheStorage {
const store = new LRUCache<string, CacheEntry>({
max: MEMORY_CACHE_MAX_ITEMS,
maxSize: MEMORY_CACHE_MAX_SIZE,
sizeCalculation: (entry) => entry.body.length,
});

const caches: CacheStorage = {
delete: () => {
throw new Error("Not Implemented");
},
has: () => {
throw new Error("Not Implemented");
},
keys: () => {
throw new Error("Not Implemented");
},
match: () => {
throw new Error("Not Implemented");
},
open: (cacheName: string): Promise<Cache> => {
const requestURLSHA1 = withCacheNamespace(cacheName);
return Promise.resolve({
...baseCache,
delete: async (
request: RequestInfo | URL,
_options?: CacheQueryOptions,
): Promise<boolean> => {
Comment on lines +91 to +94

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

For the Cache API, what is the expected behavior of Cache.delete(request, options)regardingCacheQueryOptionssuch asignoreSearchandignoreMethod?

💡 Result:

The Cache.delete(request, options) method uses the provided CacheQueryOptions (such as ignoreSearch and ignoreMethod) to control how the matching is performed when identifying the cache entry to delete. It finds the Cache entry whose key matches the request according to the matching rules modified by the options, deletes it if found (resolving to true), or resolves to false if no match is found. Specifically: - ignoreSearch: When true, ignores the query string in the URL during matching (defaults to false). - ignoreMethod: When true, does not validate the Request's HTTP method (normally only GET/HEAD allowed; defaults to false). This behavior is identical to how options work in Cache.match and Cache.matchAll.

Citations:


🏁 Script executed:

# First, let's find and examine the inMemoryCache.ts file
find . -name "inMemoryCache.ts" -type f

Repository: deco-cx/deco

Length of output: 89


🏁 Script executed:

# Once found, let's examine the full delete() and match() implementations
cat -n runtime/caches/inMemoryCache.ts | head -100

Repository: deco-cx/deco

Length of output: 3189


🏁 Script executed:

# Search for assertNoOptions usage in the codebase to understand the pattern
rg "assertNoOptions" --type typescript

Repository: deco-cx/deco

Length of output: 90


🏁 Script executed:

# Let's check the utils.ts file to see what assertNoOptions does
cat -n runtime/caches/utils.ts

Repository: deco-cx/deco

Length of output: 4551


🏁 Script executed:

# Search for assertNoOptions usage with context
rg "assertNoOptions" -A 2 -B 2 --type ts

Repository: deco-cx/deco

Length of output: 4070


Add assertion for unsupported CacheQueryOptions in delete() to match match() behavior.

The delete() method accepts CacheQueryOptions but silently ignores them (parameter has underscore prefix), while match() explicitly rejects them via assertNoOptions(). This inconsistency can hide behavior mismatches when callers pass ignoreSearch, ignoreMethod, or ignoreVary. Other cache implementations (e.g., redis.ts) already call assertNoOptions() in their delete() methods—align this implementation with that pattern.

Suggested fix
         delete: async (
           request: RequestInfo | URL,
-          _options?: CacheQueryOptions,
+          options?: CacheQueryOptions,
         ): Promise<boolean> => {
+          assertNoOptions(options);
           const cacheKey = await requestURLSHA1(request);
           return store.delete(cacheKey);
         },
📝 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.

Suggested change
delete: async (
request: RequestInfo | URL,
_options?: CacheQueryOptions,
): Promise<boolean> => {
delete: async (
request: RequestInfo | URL,
options?: CacheQueryOptions,
): Promise<boolean> => {
assertNoOptions(options);
const cacheKey = await requestURLSHA1(request);
return store.delete(cacheKey);
},
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@runtime/caches/inMemoryCache.ts` around lines 46 - 49, The delete() method in
inMemoryCache.ts currently ignores its CacheQueryOptions parameter (named
_options) unlike match() which calls assertNoOptions(); update delete to
assertNoOptions on the options to reject unsupported flags. Inside the async
delete implementation (function named delete), call assertNoOptions(_options,
'delete') at the start (or rename to options and call assertNoOptions(options,
'delete')) so it behaves like redis.ts and match() and consistently rejects
ignoreSearch/ignoreMethod/ignoreVary.

const cacheKey = await requestURLSHA1(request);
return store.delete(cacheKey);
},
match: async (
request: RequestInfo | URL,
options?: CacheQueryOptions,
): Promise<Response | undefined> => {
assertNoOptions(options);
const cacheKey = await requestURLSHA1(request);
const entry = store.get(cacheKey);
if (!entry) return undefined;
return new Response(entry.body as unknown as BodyInit, {
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
headers: new Headers(entry.headers),
status: entry.status,
});
},
put: async (
request: RequestInfo | URL,
response: Response,
): Promise<void> => {
const req = new Request(request);
assertCanBeCached(req, response);
if (!response.body) return;
const cacheKey = await requestURLSHA1(request);
const body = new Uint8Array(await response.arrayBuffer());
const headers: [string, string][] = [...response.headers.entries()];
Comment on lines +118 to +136

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

In the Fetch API, does Response.arrayBuffer() return an empty ArrayBuffer when the response body is null, and is caching such responses valid behavior?

💡 Result:

Yes, Response.arrayBuffer returns an empty ArrayBuffer (with byteLength 0) when the response body is null. Caching such responses is valid behavior. In the Fetch API, Response.body is null for responses constructed with null body (e.g., new Response(null)) or per HTTP semantics for no-body statuses like 204 No Content, 205 Reset Content, and 304 Not Modified. However, browsers do not implement the spec's null body exactly; they provide an empty ReadableStream instead. The arrayBuffer method reads the body stream to completion, resolving to an empty ArrayBuffer for null or empty bodies. This is confirmed by MDN documentation stating arrayBuffer resolves with an ArrayBuffer after reading the stream, and practical observations in issues where null bodies become empty ArrayBuffers (e.g., Safari workaround turning null to empty ArrayBuffer). The WHATWG Fetch spec links to body methods like arrayBuffer, and implementations copy the buffer source, yielding empty for no content. Caching Responses with null/empty bodies is valid and common, e.g., in Service Workers via caches.match returning cached empty-body responses (e.g., 304), as shown in WHATWG discussions and examples.

Citations:


🏁 Script executed:

# Find the file and check its structure
fd -t f "inMemoryCache.ts" runtime/caches/

Repository: deco-cx/deco

Length of output: 87


🏁 Script executed:

# View the specific section around lines 72-75
cat -n runtime/caches/inMemoryCache.ts | sed -n '60,90p'

Repository: deco-cx/deco

Length of output: 1077


🏁 Script executed:

# Find where assertCanBeCached is defined and what it does
rg "assertCanBeCached" -A 5

Repository: deco-cx/deco

Length of output: 3531


🏁 Script executed:

# Check if there are any tests related to empty body caching
rg -i "empty.*body|body.*empty|arrayBuffer.*null" --type ts --type js -A 3

Repository: deco-cx/deco

Length of output: 224


🏁 Script executed:

# Look for any usage of the cache to understand if empty responses are expected
rg "store\.get|store\.set" runtime/caches/ -B 2 -A 2

Repository: deco-cx/deco

Length of output: 868


🏁 Script executed:

# Look for tests related to caching empty bodies or null bodies
find . -type f \( -name "*test*" -o -name "*.test.*" -o -name "*.spec.*" \) | xargs rg -i "cache.*empty|empty.*cache|null.*body|body.*null" 2>/dev/null | head -30

Repository: deco-cx/deco

Length of output: 243


🏁 Script executed:

# Check if there are any comments explaining why the guard exists
rg -B 3 "if.*!response\.body" runtime/caches/

Repository: deco-cx/deco

Length of output: 1267


🏁 Script executed:

# Look for any documentation or comments in the utils file about caching rules
cat -n runtime/caches/utils.ts | head -50

Repository: deco-cx/deco

Length of output: 1591


Remove the early return guard that prevents caching of responses with null/empty bodies.

Line 72 returns early when response.body is null, preventing valid responses from being cached. Per the Fetch API, Response.arrayBuffer() returns an empty ArrayBuffer for null bodies, and such responses are cacheable (e.g., 304 Not Modified, 204 No Content). This pattern exists across all cache implementations (inMemoryCache, redis, fileSystem, headerscache, lrucache) and should be removed from all of them.

Suggested fix
           const req = new Request(request);
           assertCanBeCached(req, response);
-          if (!response.body) return;
           const cacheKey = await requestURLSHA1(request);
           const body = new Uint8Array(await response.arrayBuffer());
           const headers: [string, string][] = [...response.headers.entries()];
           store.set(cacheKey, { body, headers, status: response.status });
📝 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.

Suggested change
if (!response.body) return;
const cacheKey = await requestURLSHA1(request);
const body = new Uint8Array(await response.arrayBuffer());
const headers: [string, string][] = [...response.headers.entries()];
const req = new Request(request);
assertCanBeCached(req, response);
const cacheKey = await requestURLSHA1(request);
const body = new Uint8Array(await response.arrayBuffer());
const headers: [string, string][] = [...response.headers.entries()];
store.set(cacheKey, { body, headers, status: response.status });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@runtime/caches/inMemoryCache.ts` around lines 72 - 75, Remove the early
return that checks response.body (the line "if (!response.body) return;") so
responses with null/empty bodies are still cached; keep using
requestURLSHA1(request) for the cache key and continue to read the body via
response.arrayBuffer() (which yields an empty ArrayBuffer for null bodies) and
collect headers via [...response.headers.entries()] as before; apply the same
removal of the guard across inMemoryCache, redis, fileSystem, headerscache, and
lrucache implementations to ensure 204/304 and other empty-body responses are
cached.

store.set(cacheKey, { body, headers, status: response.status });
},
});
},
};

return caches;
}

export const caches = createInMemoryCache();
38 changes: 37 additions & 1 deletion runtime/caches/lrucache.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,18 @@
import { LRUCache } from "npm:lru-cache@10.2.0";
import { ValueType } from "../../deps.ts";
import { meter } from "../../observability/otel/metrics.ts";
import {
assertCanBeCached,
assertNoOptions,
baseCache,
createBaseCacheStorage,
} from "./utils.ts";

const lruEvictionCounter = meter.createCounter("lru_cache_eviction", {
unit: "1",
valueType: ValueType.DOUBLE,
});

// keep compatible with old variable name
const CACHE_MAX_SIZE = parseInt(
Deno.env.get("CACHE_MAX_SIZE") ?? Deno.env.get("MAX_CACHE_SIZE") ??
Expand All @@ -30,17 +37,46 @@ const cacheOptions = (cache: Cache) => (
maxSize: CACHE_MAX_SIZE,
ttlAutopurge: CACHE_TTL_AUTOPURGE,
ttlResolution: CACHE_TTL_RESOLUTION,
dispose: async (_value: boolean, key: string) => {
dispose: async (_value: boolean, key: string, reason: string) => {
lruEvictionCounter.add(1, { reason });
await cache.delete(key);
},
Comment on lines +42 to 45

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -type f -name "lrucache.ts" | head -5

Repository: deco-cx/deco

Length of output: 84


🏁 Script executed:

cat -n runtime/caches/lrucache.ts

Repository: deco-cx/deco

Length of output: 7197


🏁 Script executed:

npm info lru-cache@10.2.0 description

Repository: deco-cx/deco

Length of output: 114


🏁 Script executed:

# Search for lru-cache documentation or examples in the codebase
rg -i "dispose|noDisposeOnSet" --type ts --type js

Repository: deco-cx/deco

Length of output: 3348


🌐 Web query:

lru-cache npm@10.2.0 dispose callback reason eviction set overwrites

💡 Result:

In lru-cache version 10.2.0, the dispose callback is provided as an option when creating an LRUCache instance: dispose: (value, key, reason) => {}. It is called whenever an item is removed from the cache, including on eviction (when the cache is full and the least recently used item is dropped), overwrites (when cache.set is called on an existing key), explicit deletes, TTL expiration, or fetch failures. The 'reason' parameter is a string from the DisposeReason enum with these values: - 'evict': Item evicted due to cache being full (LRU eviction). - 'set': Item overwritten by a new value via cache.set (this covers "set overwrites"). - 'delete': Explicit deletion via cache.delete, cache.clear, or cache.set(key, undefined). - 'expire': Item removed due to TTL expiration. - 'fetch': Item deleted due to fetchMethod returning undefined or aborting. There is no 'overwrite' reason; overwrites use 'set'. The feature has been stable since v7.2 when reasons were added to dispose (v10.2.0 inherits this). Use noDisposeOnSet: true to suppress dispose calls specifically on overwrites. The callback runs before the item is fully removed from the cache.

Citations:


Filter dispose callback to only delete on true evictions, not overwrites.

In Lines 40-43, the dispose callback unconditionally deletes the backing store for all disposal reasons. The lru-cache library calls dispose on 'set' (overwrites), not only on actual evictions. Since put() calls fileCache.set() at Line 146 before writing to cacheInner at Line 150, a same-key refresh triggers the dispose callback with reason='set', which asynchronously deletes the backing entry while the put is still writing the new value, creating a race condition. Additionally, this counts overwrites as "evictions" in the metric. Check the reason parameter and only delete on true evictions: filter for reason === 'evict' or reason === 'expire', or set noDisposeOnSet: true in the cache options to suppress dispose on overwrites entirely.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@runtime/caches/lrucache.ts` around lines 40 - 43, The dispose callback
currently always deletes the backing store (lruEvictionCounter.add and await
cache.delete) for every disposal reason, causing races on overwrites from put()
(which calls fileCache.set() then writes to cacheInner). Change the behavior so
only true evictions remove the backing entry: either set noDisposeOnSet: true in
the LRU options where fileCache is constructed, or update dispose(_: boolean,
key: string, reason: string) to early-return unless reason === 'evict' || reason
=== 'expire' (and only then call lruEvictionCounter.add and await
cache.delete(key)); reference symbols: dispose, fileCache.set, put(),
cacheInner, lruEvictionCounter, cache.delete.

}
);

const lruSizeGauge = meter.createObservableGauge("lru_cache_keys", {
description: "number of keys in the LRU cache",
unit: "1",
valueType: ValueType.DOUBLE,
});

const lruBytesGauge = meter.createObservableGauge("lru_cache_bytes", {
description: "total bytes tracked by the LRU cache",
unit: "bytes",
valueType: ValueType.DOUBLE,
});

// deno-lint-ignore no-explicit-any
const activeCaches: LRUCache<string, any>[] = [];

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Memory leak: caches accumulate in activeCaches without cleanup.

Caches are pushed to the module-level activeCaches array but never removed. Per mod.ts (context snippet 2), cache instances are created at module load time. In hot-reload or reconfiguration scenarios, new LRUCache instances are added while old ones remain, causing:

  • Memory leaks (old caches not GC'd)
  • Stale/inaccurate gauge metrics (reporting defunct cache sizes)

Consider using a WeakRef or a Set with explicit cleanup, or ensuring caches are singletons.

Suggested approach using WeakRef
-// deno-lint-ignore no-explicit-any
-const activeCaches: LRUCache<string, any>[] = [];
+// deno-lint-ignore no-explicit-any
+const activeCaches: WeakRef<LRUCache<string, any>>[] = [];

 lruSizeGauge.addCallback((observer) => {
-  for (const cache of activeCaches) {
-    observer.observe(cache.size);
+  for (const ref of activeCaches) {
+    const cache = ref.deref();
+    if (cache) observer.observe(cache.size);
   }
 });

 lruBytesGauge.addCallback((observer) => {
-  for (const cache of activeCaches) {
-    observer.observe(cache.calculatedSize);
+  for (const ref of activeCaches) {
+    const cache = ref.deref();
+    if (cache) observer.observe(cache.calculatedSize);
   }
 });

And at line 79:

-      activeCaches.push(fileCache);
+      activeCaches.push(new WeakRef(fileCache));

Also applies to: 79-79

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@runtime/caches/lrucache.ts` around lines 59 - 60, The module-level
activeCaches array currently holds strong references to every LRUCache<string,
any> instance causing leaks; change the registration so activeCaches stores
WeakRef<LRUCache<string, any>> (or use a Set and remove old instances) and
update any code that iterates activeCaches (e.g., the cache registration/push
site and the gauge reporting logic) to dereference the WeakRef, skip collected
caches, and prune dead refs from the collection; alternatively implement a
singleton getter for LRUCache instances to avoid multiple creations and ensure
metrics read only live caches.


lruSizeGauge.addCallback((observer) => {
for (const cache of activeCaches) {
observer.observe(cache.size);
}
});

lruBytesGauge.addCallback((observer) => {
for (const cache of activeCaches) {
observer.observe(cache.calculatedSize);
}
});

function createLruCacheStorage(cacheStorageInner: CacheStorage): CacheStorage {
const caches = createBaseCacheStorage(
cacheStorageInner,
(_cacheName, cacheInner, requestURLSHA1) => {
const fileCache = new LRUCache(cacheOptions(cacheInner));
activeCaches.push(fileCache);
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
Outdated
return Promise.resolve({
...baseCache,
delete: async (
Expand Down
6 changes: 5 additions & 1 deletion runtime/caches/mod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import { caches as lruCache } from "./lrucache.ts";

import { caches as fileSystem } from "./fileSystem.ts";

import { caches as inMemoryCache } from "./inMemoryCache.ts";

export const ENABLE_LOADER_CACHE: boolean =
Deno.env.get("ENABLE_LOADER_CACHE") !== "false";
const DEFAULT_CACHE_ENGINE = "CACHE_API";
Expand All @@ -38,7 +40,9 @@ export const cacheImplByEngine: Record<CacheEngine, CacheStorageOption> = {
isAvailable: typeof globalThis.caches !== "undefined",
},
FILE_SYSTEM: {
implementation: headersCache(lruCache(fileSystem)),
implementation: headersCache(
lruCache(createTieredCache(inMemoryCache, fileSystem)),
),
isAvailable: isFileSystemAvailable,
},
REDIS: {
Expand Down
Loading
Loading