Agent常在PR中添加module级Map缓存但不处理多租户隔离、失效机制和容量限制,导致跨租户数据污染。给出了审查fixture和常见缺陷模式。
一个队列 Worker 的 PR 在周一落地了。一个 Agent 用模块级别的 Map 包装了 loadAccount。愉快的路径演示减少了两轮数据库往返。然后第二个租户复用了同一个进程。那个 Map 还保留着第一个租户的 plan 标志。计费系统在十一分钟内用了错误的配额上限。
Agent 的缓存片段隐藏在性能提交里。忙碌的审查者把它们当作本地小改动。它们是进程级别的行为变更,不是本地小改动。
把缓存当作共享状态来对待
内存缓存是共享的可变状态。它比单个请求处理器存活得更久。它无视租户边界,除非 key 里编码了它们。它也会无视写操作,除非存在一个 bust 路径。
Agent 的补丁往往先加 Map。然后很少接着加失效机制。几乎从不加大小上限。有时 key 只用了 userId。那个 key 在同一个进程内会发生跨租户碰撞。
这些片段长什么样
下面的代码片段是一个审查 fixture。不是生产代码。它展示了一种常见的 agent 模式。
// agent-generated "optimization"
const accountCache = new Map();
async function loadAccount(accountId) {
if (accountCache.has(accountId)) {
return accountCache.get(accountId);
}
const row = await db.accounts.findById(accountId);
accountCache.set(accountId, row);
return row;
}
写路径完全没动。updatePlan 还是只写 SQL。读者持续提供 stale 的行。Map 上没有 TTL 或 maxSize。Worker 进程变成了一个沉默的事实来源。
一个稍微安全一点的 agent 变体仍然过不了审查。
import { LRUCache } from "lru-cache";
const cache = new LRUCache({ max: 500, ttl: 60_000 });
export async function loadAccount(ctx, accountId) {
const hit = cache.get(accountId);
if (hit) return hit;
const row = await db.accounts.findById(accountId);
cache.set(accountId, row);
return row;
}
LRU 边界对内存压力有帮助。TTL 对意外的 freshness 有帮助。Key 仍然漏掉了 ctx.tenantId。跨租户数据渗漏在部署后仍然可能发生。审查必须把那个遗漏当作一个 defect。
请求作用域 versus 模块作用域
模块作用域是通常的失败点。请求作用域是另一个类别。下一个 fixture 把数据保持在 req 上。
function accountCacheFor(req) {
if (!req.accountCache) {
req.accountCache = new Map();
}
return req.accountCache;
}
export async function loadAccount(req, accountId) {
const cache = accountCacheFor(req);
if (cache.has(accountId)) return cache.get(accountId);
const row = await db.accounts.findById(accountId);
cache.set(accountId, row);
return row;
}
Map 随请求一起消亡。跨请求的租户渗漏不可能在这里发生。写耦合在单个请求内部仍然重要。审查者仍然应该测试那条路径。
每个缓存片段的五个信号
审查者可以用五个信号给片段打分。每个信号是一个合并门。缺少任何信号意味着额外的测试。
Key 组合。Key 必须包含 tenant、locale 和 auth scope。
写耦合。每个变更路径必须 bust 或更新条目。
有界和 TTL。无界的 map 会持续增长直到 worker OOM。
Null 缓存。缓存 undefined 可能隐藏新创建的行。
进程形态。Cluster workers 不共享 Map,所以 sticky state 会出现。
不要在绿色 CI 上做 vibe-based approval。在审查备注里给五个信号打分。把分数贴在 merge 按钮上方。
编号化审查工作流
在原始 diff 上使用这个序列。不要从渲染后的 UI 开始。
隔离分配 Map、WeakMap、LRUCache 或 memoize 包装器的片段。
列出每个现在返回缓存数据的读函数。
列出每个变更相同记录的写函数。
检查这些写操作是否在同一个 pull request 里。
要求一个纯且全的 key builder。
如果写操作在另一个服务里就 revert 缓存。
只有在测试覆盖了渗漏、stale reads 和驱逐时才保留缓存。
这个工作流是机械性的,目的是如此。Agent 重复同样的捷径。人类在日历压力下会错过它。
信任、revert 或测试
授权缓存值得硬 revert。延迟的 deny 变成 allow。延迟的 allow 变成 lockout。都不属于 agent 路过打补丁的场景。
Artifact:扫描 unified diff
下面的脚本是一个审查辅助工具。不是生产软件。审查者应该把 git diff 管道过去。它打印类缓存的添加行,带文件名和行号。
#!/usr/bin/env node
"use strict";
const fs = require("fs");
const PATTERNS = [
{ name: "map_alloc", re: /\bnew\s+(Map|WeakMap|LRUCache)\b/ },
{ name: "memoize", re: /\b(memoize|lru_cache|cacheable)\s*\(/i },
{ name: "cache_ident", re: /\b\w*[Cc]ache\w*\.(get|set|has|delete|clear)\s*\(/ },
{ name: "ttl", re: /\b(ttl|maxAge|expiresIn)\b/ },
{ name: "unbounded_set", re: /\.\s*set\s*\([^,]+,\s*[^)]+\)/ },
];
function scan(diff) {
const findings = [];
let file = "";
let newLine = 0;
for (const raw of diff.split(/\r?\n/)) {
if (raw.startsWith("+++ b/")) {
file = raw.slice(6);
continue;
}
if (raw.startsWith("@@")) {
const m = raw.match(/\+(\d+)/);
newLine = m ? Number(m[1]) : 0;
continue;
}
if (raw.startsWith("+") && !raw.startsWith("+++")) {
const line = raw.slice(1);
for (const p of PATTERNS) {
if (p.re.test(line)) {
findings.push({
file,
line: newLine,
signal: p.name,
text: line.trim(),
});
}
}
newLine += 1;
continue;
}
if (raw.startsWith(" ")) newLine += 1;
}
return findings;
}
const diff = fs.readFileSync(0, "utf8");
const findings = scan(diff);
if (!findings.length) {
console.log("No cache-like added lines.");
process.exit(0);
}
for (const f of findings) {
console.log(`${f.file}:${f.line} [${f.signal}] ${f.text}`);
}
process.exit(2);
从一个干净的 checkout 运行它。下面的命令保持在本地且可重复。
git fetch origin pull/812/head:pr-812
git checkout pr-812
git diff main...HEAD -- '*.js' '*.ts' ':!dist' ':!vendor' \
| node scan-cache-hunks.js
Exit code 2 意味着必须有人给五个信号打分。Exit code 0 意味着扫描器没看到任何东西。没东西不等于安全。动态缓存可以藏在名为 remember 的辅助函数后面。
不要仅仅因为绿色单元测试就合并。缓存的读需要对抗性用例。
两个租户,同一个 accountId 空间,顺序读。期待隔离。
更新一个缓存的字段,然后立即读。期待新值。
缓存一个缺失的行,然后插入它。期待命中新的行。
填充超过 max 然后读最老的 key。期待驱逐,不是 OOM。
TTL 过期,然后读。期待一次 store 往返。
运行两个 Node cluster workers。期待没有跨 worker 的假设。
信号 1 的一个最小测试 double 像这样。把它标记为必需示例,不是测量运行。
test("cache key includes tenant", async () => {
const t1 = { tenantId: "acme", accountId: "001" };
const t2 = { tenantId: "globex", accountId: "001" };
await loadAccount(t1);
await db.accounts.updatePlan("acme", "001", "pro");
const a = await loadAccount(t1);
const b = await loadAccount(t2);
assert.equal(a.plan, "pro");
assert.equal(b.tenantId, "globex");
assert.notEqual(a.plan, b.plan);
});
Agent 通常只测试命中路径。命中路径是最没意思的路径。Stale writes 和租户碰撞才是合并门。
不在笔记本上跑扫描器
大 diff 会拖垮便宜的审查笔记本。远程 shell 保持审查机器凉爽。
披露:本文是 MonkeyCode 产品推广的一部分。
MonkeyCode 是一个开源项目。它提供免费的托管模型访问。也提供免费的服务端选项。审查者可以把扫描器和 diff 放在那个服务器上。托管模型可以起草一个租户感知的 key builder。服务器可以运行上面的 git diff 管道。审查者仍然拥有合并决定权。扫描器不是。
这不是基准测试。这里没有声称延迟或配额数字。
正则扫描器会漏掉工厂。名为 remember 的辅助函数会溜过去。提到 cache 的注释会误报。压缩过的 vendor 文件会让报告有噪音。在 diff pathspec 里排除 vendor/ 和 dist/。
这本 playbook 也忽略分布式缓存。Redis 失效是另一套审查。本文只覆盖进程内存。TTL 值是产品策略。扫描器选不了它们。Owner 必须。
谁不应该用这个方法
对于模块测试内部的纯函数 memoization 跳过这本 playbook。当平台缓存已经坐在审查过的库后面时跳过它。当团队没有测试数据库时跳过它。没有隔离测试的缓存是等待流量发生的生产事故。
不要把扫描器当作 CI 绿灯用。把它当作分类网用。人类仍然读写路径。人类仍然拒绝授权缓存。
在周一的 PR 上闭环
周一的 Worker 需要一个额外的 key 字段。tenantId 应该在 Map key 里。updatePlan 需要 accountCache.delete(key)。需要一个有文档说明 lag 的 60 秒 TTL。在那些落地之前,正确的审查动作是 revert。
缓存片段感觉像是来自 agent 的礼物。它们是 stale state 的所有权。把它们当作状态机来审查,不是当作加速。把信任只给请求作用域的情况。看到 policy 缓存就 revert。通过 revert 关卡的所有东西都要测试。
MonkeyCode 提供可以运行这个工作流的免费模型。