Skip to content

Commit 2a03a84

Browse files
JacksonGLmeta-codesync[bot]
authored andcommitted
feat(cmp-server): Improve MCP server cache/growth/duplicate-string detectors
Summary: Three detector improvements to the MemLab MCP server, based on recurring gaps hit while analyzing real Node.js server heap snapshots. Source of truth is edited in `www/scripts/webspeed/memory_lab/packages/mcp-server/src/` and synced to the internal Claude plugin copy under `fbcode/claude-templates/components/plugins/memlab/memlab-server/` via `sync-mcp-to-plugin.sh`. **1. Recognize the `{<array>, timestamp/loadedAt, map}` TTL-cache shape (`cache-analysis`).** The object-cache detector already matched `hasDataProp && hasTimestampProp`, but its `DATA_PROPS` set lacked `entries` and its `TIMESTAMP_PROPS` set lacked `loadedAt`, so the two most common in-process Nest cache shapes — `{entries, timestamp, map}` and `{items, loadedAt}` — were classified as "plain collection" instead of "cache-like." Added `entries`/`records`/`nodes`/`list`/`cache` to the data-property set and `loadedAt`/`loaded_at`/`cached_at`/`fetchedAt`/`fetched_at`/`lastFetched`/`lastLoaded`/`lastUpdated` to the timestamp set. **2. Stop flagging load-once TTL caches as unbounded growth (`growth-signals`).** A large `Array` held by a `{…, loadedAt/timestamp}` wrapper is a point-in-time snapshot refreshed wholesale, not append-only growth, yet it was reported as an "append-only candidate." Added a shared `hasFreshnessTimestampSibling()` helper and skip such arrays, reporting a count of how many were excluded so the heuristic stays transparent. **3. Detect cached failure payloads (`duplicated-strings`).** Added `looksLikeFailurePayload()` and a callout when heavily-duplicated strings look like cached error/failure responses (JSON-ish payloads carrying markers such as `UNKNOWN_FAILURE`, `"status":"…FAILURE"`, or a non-null `"error"`). These are both wasted memory and a strong signal that an upstream dependency is failing and its failures are being cached — linking a memory finding to a likely correctness bug. New shared helpers (`FRESHNESS_TIMESTAMP_PROPS`, `hasFreshnessTimestampSibling`, `looksLikeFailurePayload`) live in `utils.ts`. All three changes are additive and backward-compatible; package/plugin versions bumped accordingly (mcp-server 2.1.2→2.2.0, internal server 2.0.6→2.1.0, plugin 1.7.0→1.8.0). Reviewed By: boujeepossum Differential Revision: D107701087 fbshipit-source-id: f6270b7cd9f0892758d0f53e59bdb1a75e519329
1 parent 52bf3f0 commit 2a03a84

5 files changed

Lines changed: 135 additions & 3 deletions

File tree

packages/mcp-server/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@memlab/mcp-server",
3-
"version": "2.1.2",
3+
"version": "2.2.0",
44
"license": "MIT",
55
"description": "MCP server for MemLab heap snapshot analysis — gives AI coding assistants tools to explore JavaScript heap snapshots, find memory leaks, and identify optimization opportunities",
66
"author": "Liang Gong <lgong@meta.com>",

packages/mcp-server/src/tools/cache-analysis.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -397,6 +397,14 @@ export function registerCacheAnalysis(server: McpServer): void {
397397
'value',
398398
'response',
399399
'payload',
400+
// Common collection-property names on Nest/in-process TTL caches
401+
// that were missed before (e.g. `{entries, timestamp, map}`,
402+
// `{records, loadedAt}`). Feedback round 4 §B.
403+
'entries',
404+
'records',
405+
'nodes',
406+
'list',
407+
'cache',
400408
]);
401409
const TIMESTAMP_PROPS = new Set([
402410
'timestamp',
@@ -408,6 +416,17 @@ export function registerCacheAnalysis(server: McpServer): void {
408416
'ttlMs',
409417
'expiry',
410418
'createdAt',
419+
// Freshness markers used by load-once warm caches. `loadedAt` in
420+
// particular is a very common Nest pattern that was missed before.
421+
// Feedback round 4 §B.
422+
'loadedAt',
423+
'loaded_at',
424+
'cached_at',
425+
'fetchedAt',
426+
'fetched_at',
427+
'lastFetched',
428+
'lastLoaded',
429+
'lastUpdated',
411430
]);
412431
const CONFIG_PROPS = new Set([
413432
'ttlMs',

packages/mcp-server/src/tools/duplicated-strings.ts

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,12 @@
1111
import type {McpServer} from '@modelcontextprotocol/sdk/server/mcp.js';
1212
import {z} from 'zod';
1313
import {getSnapshot} from '../heap-state.js';
14-
import {formatBytes, errorResult, toolResult} from '../utils.js';
14+
import {
15+
formatBytes,
16+
errorResult,
17+
toolResult,
18+
looksLikeFailurePayload,
19+
} from '../utils.js';
1520

1621
export function registerDuplicatedStrings(server: McpServer): void {
1722
server.tool(
@@ -181,8 +186,32 @@ export function registerDuplicatedStrings(server: McpServer): void {
181186
: '';
182187
return `${i + 1}. "${val}" x ${d.count} copies, ${d.total_size_formatted} total${savingsLabel}${actionLabel}${nodeIdsPart}${context}`;
183188
});
189+
// Cached failure payloads: duplicated JSON-ish strings carrying an
190+
// explicit failure/error marker. These are both wasted memory and a
191+
// signal that an upstream dependency is failing (and that failures are
192+
// being cached). Feedback round 4 §D.
193+
const failurePayloads = duplicated.filter(d =>
194+
looksLikeFailurePayload(d.value),
195+
);
196+
184197
const hasHeavyDups = duplicated.some(d => d.count >= 1000);
185198
const suggestions: string[] = [];
199+
if (failurePayloads.length > 0) {
200+
const totalFailureCopies = failurePayloads.reduce(
201+
(sum, d) => sum + d.count,
202+
0,
203+
);
204+
const failureBytes = failurePayloads.reduce(
205+
(sum, d) => sum + d.total_size,
206+
0,
207+
);
208+
suggestions.push(
209+
`⚠️ **Cached failure payloads detected:** ${failurePayloads.length} of the duplicated strings look like cached error/failure responses ` +
210+
`(${totalFailureCopies.toLocaleString('en-US')} copies, ${formatBytes(failureBytes)}). ` +
211+
'This usually means an upstream dependency is failing AND those failures are being cached. ' +
212+
'Check the upstream call (permissions, timeouts, bad input) and avoid caching error responses (or cache them with a short TTL).',
213+
);
214+
}
186215
if (hasHeavyDups) {
187216
suggestions.push(
188217
'**Suggested action:** Heavily duplicated strings often come from `JSON.parse()` or API responses. ' +

packages/mcp-server/src/tools/growth-signals.ts

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
markdownTable,
1919
errorResult,
2020
toolResult,
21+
hasFreshnessTimestampSibling,
2122
} from '../utils.js';
2223

2324
type KeyPattern = 'sequential-int' | 'timestamp' | 'monotonic-string' | 'mixed';
@@ -118,6 +119,10 @@ export function registerGrowthSignals(server: McpServer): void {
118119
const meta = getSnapshotMetadata();
119120
const totalSize = meta?.totalSize ?? 0;
120121
const candidates: GrowthCandidate[] = [];
122+
// Count of large arrays skipped because they are the data side of a
123+
// load-once `{entries, loadedAt}`-style TTL cache (a point-in-time
124+
// snapshot, not append-only growth). Feedback round 4 §C.
125+
let excludedLoadOnce = 0;
121126

122127
const insert = (c: GrowthCandidate) => {
123128
let i = 0;
@@ -164,6 +169,13 @@ export function registerGrowthSignals(server: McpServer): void {
164169
} else if (node.name === 'Array') {
165170
const count = node.edge_count;
166171
if (count < min_entries) return;
172+
// A large array held by a `{..., loadedAt/timestamp}` cache wrapper
173+
// is a load-once snapshot refreshed wholesale, not append-only
174+
// growth — don't flag it as a growth signal. Feedback round 4 §C.
175+
if (hasFreshnessTimestampSibling(node)) {
176+
excludedLoadOnce++;
177+
return;
178+
}
167179
// Heuristic: large dense arrays are append-only growth candidates.
168180
insert({
169181
nodeId: node.id,
@@ -176,9 +188,14 @@ export function registerGrowthSignals(server: McpServer): void {
176188
}
177189
});
178190

191+
const loadOnceNote =
192+
excludedLoadOnce > 0
193+
? ` (${formatNumber(excludedLoadOnce)} large array(s) held by a \`{…, loadedAt/timestamp}\` cache wrapper were excluded as load-once snapshots, not append-only growth)`
194+
: '';
195+
179196
if (candidates.length === 0) {
180197
return toolResult(
181-
`No growth signals found (no timestamp/sequentially-keyed collections or large arrays >= ${formatNumber(min_entries)} entries and >= ${formatBytes(min_retained_size)}). This is a single-snapshot heuristic — capture a second snapshot later and use memlab_diff_snapshots to confirm actual growth.`,
198+
`No growth signals found (no timestamp/sequentially-keyed collections or large arrays >= ${formatNumber(min_entries)} entries and >= ${formatBytes(min_retained_size)})${loadOnceNote}. This is a single-snapshot heuristic — capture a second snapshot later and use memlab_diff_snapshots to confirm actual growth.`,
182199
);
183200
}
184201

@@ -209,6 +226,7 @@ export function registerGrowthSignals(server: McpServer): void {
209226
'',
210227
markdownTable(headers, rows, rightCols),
211228
'',
229+
...(loadOnceNote ? [`_Note:${loadOnceNote}_`, ''] : []),
212230
'_Heuristic only — timestamp/sequential keys and large dense arrays *suggest* append-only growth but do not prove it. Confirm by capturing a later snapshot and running `memlab_diff_snapshots`, or trace one with `memlab_retainer_trace` to see what keeps it alive._',
213231
];
214232

packages/mcp-server/src/utils.ts

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,72 @@ export function isNodeWorthInspecting(node: IHeapNode): boolean {
222222
return true;
223223
}
224224

225+
/**
226+
* Property names that mark a "freshness" timestamp on a TTL/warm cache wrapper
227+
* object (e.g. `{entries, loadedAt}`, `{items, timestamp}`). Their presence next
228+
* to a large array/Map means the collection was loaded as a point-in-time
229+
* snapshot, not appended to over time. Shared by cache-analysis (to recognize
230+
* the cache shape) and growth-signals (to suppress the append-only
231+
* false-positive on load-once caches). Feedback round 4 §B/§C.
232+
*/
233+
export const FRESHNESS_TIMESTAMP_PROPS: ReadonlySet<string> = new Set([
234+
'timestamp',
235+
'loadedAt',
236+
'loaded_at',
237+
'ts',
238+
'cachedAt',
239+
'cached_at',
240+
'fetchedAt',
241+
'fetched_at',
242+
'lastFetched',
243+
'lastLoaded',
244+
'lastUpdated',
245+
'updatedAt',
246+
'expiresAt',
247+
'expiry',
248+
]);
249+
250+
/**
251+
* True when `node` (typically a large Array/Map) is held by a wrapper object
252+
* that also carries a freshness-timestamp sibling property — i.e. it is the data
253+
* side of a `{entries, loadedAt}`-style TTL/warm cache, loaded once rather than
254+
* grown unboundedly.
255+
*/
256+
export function hasFreshnessTimestampSibling(node: IHeapNode): boolean {
257+
for (const ref of node.referrers) {
258+
if (ref.type !== 'property') continue;
259+
const parent = ref.fromNode;
260+
if (!parent) continue;
261+
for (const edge of parent.references) {
262+
if (
263+
edge.type === 'property' &&
264+
FRESHNESS_TIMESTAMP_PROPS.has(String(edge.name_or_index))
265+
) {
266+
return true;
267+
}
268+
}
269+
}
270+
return false;
271+
}
272+
273+
/**
274+
* Heuristic: does a (JSON-ish) string look like a cached *failure* response?
275+
* Cached error payloads are both wasted memory and a signal that an upstream
276+
* dependency is failing — surfacing them links a memory finding to a likely
277+
* correctness bug. Conservative: requires a JSON-object/array head plus an
278+
* explicit failure marker. Feedback round 4 §D.
279+
*/
280+
export function looksLikeFailurePayload(value: string): boolean {
281+
const head = value.length > 4096 ? value.slice(0, 4096) : value;
282+
const trimmed = head.trimStart();
283+
if (!(trimmed.startsWith('{') || trimmed.startsWith('['))) return false;
284+
return (
285+
/\bUNKNOWN_FAILURE\b/.test(head) ||
286+
/"status"\s*:\s*"[^"]*(?:FAILURE|FAILED|ERROR)/i.test(head) ||
287+
/"error"\s*:\s*"(?!null)[^"]+/i.test(head)
288+
);
289+
}
290+
225291
export function filterLargestObjects(
226292
snapshot: IHeapSnapshot,
227293
filter: (node: IHeapNode) => boolean,

0 commit comments

Comments
 (0)