Friday afternoon. A pull request lands from an AI agent. It adds a rate limiter to a small API.
The diff looks clean. The tests pass. Something still feels off.
This article walks through that review. It shows what to trust, what to revert, and what to test.
The PR came from an agent running on MonkeyCode's free server tier. The agent used the free model access with a 10 million token budget. Disclosure: This article was prepared as part of MonkeyCode's product outreach.
MonkeyCode is an open source project. The free tier includes model access and a disposable server. A previous field test on this account measured the token budget.
The example PR below is illustrative. It is based on a typical agent output. Run the checks against your own PR.
The Pull Request
The agent produced one middleware file. It also touched the API response shape. It added one dependency.
Here is the core change:
// pr/rate-limit.js
const rateLimit = new Map();
function rateLimitMiddleware(req, res, next) {
const key = req.ip;
const now = Date.now();
const windowMs = 60 * 1000;
const max = 100;
if (!rateLimit.has(key)) {
rateLimit.set(key, []);
}
const timestamps = rateLimit.get(key).filter(t => now - t < windowMs);
if (timestamps.length >= max) {
return res.status(429).json({ error: "Too many requests" });
}
timestamps.push(now);
rateLimit.set(key, timestamps);
next();
}
The diff also changed every error response. Old shape: { message: "..." }. New shape: { error: "..." }.
It added express-rate-limit to package.json. The middleware never imports that package.
What to Trust
Three things look solid. The middleware is small and readable. It returns a proper HTTP 429.
The happy path tests pass. The core idea is sound. A sliding window per IP is a reasonable default.
The code is easy to trace. An agent produced this in one pass.
What to Revert
The first problem is unbounded state. The Map stores an array per IP. Stale keys are never deleted.
A million IPs means a million entries. Memory grows forever.
The second problem is a breaking change. The response shape changed without a migration. Any client parsing message breaks silently.
This change must be reverted.
The third problem is the unused dependency. express-rate-limit was added but never imported. It bloats the install.
It widens the supply chain surface. Revert that too.
The fourth problem is trust in req.ip. Behind a proxy, this value can be spoofed. The middleware does not validate it.
Attackers can forge headers and bypass the limit.
What to Test
The agent's tests covered the happy path only. They did not cover concurrency. They did not cover spoofed headers.
They did not measure memory. Here is a concurrency check. It fires 150 requests at once.
It counts how many return 429.
# test-concurrency.sh
for i in $(seq 1 150); do
curl -s -o /dev/null -w "%{http_code}\n" http://localhost:3000/api &
done
wait
sort | uniq -c
A correct limiter returns about 100 successes and 50 rate-limited responses. A broken one returns 150 successes.
Here is a memory check. It simulates many distinct IPs. It measures heap growth after ten seconds.
// memory-check.js
const before = process.memoryUsage().heapUsed;
// send requests with 1000 distinct X-Forwarded-For values
// wait 10 seconds
const after = process.memoryUsage().heapUsed;
const growth = (after - before) / 1024 / 1024;
console.log(`Heap growth: ${growth.toFixed(2)} MB`);
The unbounded Map fails this check. The fix is a periodic cleanup. Or a fixed-size LRU cache.
The Fix
A minimal fix keeps the sliding window. It adds a cleanup pass. It also validates the client key.
// fixed/rate-limit.js
const rateLimit = new Map();
const WINDOW_MS = 60 * 1000;
const MAX = 100;
function cleanup(now) {
for (const [key, timestamps] of rateLimit) {
const fresh = timestamps.filter(t => now - t < WINDOW_MS);
if (fresh.length === 0) {
rateLimit.delete(key);
} else {
rateLimit.set(key, fresh);
}
}
}
function rateLimitMiddleware(req, res, next) {
const key = req.headers["x-forwarded-for"] || req.ip;
const now = Date.now();
if (rateLimit.size > 10000) {
cleanup(now);
}
const timestamps = (rateLimit.get(key) || []).filter(t => now - t < WINDOW_MS);
if (timestamps.length >= MAX) {
return res.status(429).json({ message: "Too many requests" });
}
timestamps.push(now);
rateLimit.set(key, timestamps);
next();
}
The fix keeps the original response shape. It caps the map size. It adds a cleanup trigger.
It still needs a real cache for production.
The Review Workflow
Use this order on the next agent PR.
- Read the diff before reading the agent's description.
- List every file outside the stated scope.
- Look for unbounded data structures.
- Check every error path, not just the happy path.
- Verify dependency changes line by line.
- Run a concurrency or race test.
- Write one failing test for the suspected bug.
- Decide: merge, request changes, or revert.
This order catches expensive mistakes early. It keeps the review fast.
The Decision Table
| Signal | Action |
|---|---|
| Unbounded state or cache | Revert or fix |
| Breaking API change without migration | Revert |
| Unused dependency | Revert |
| Missing error handling | Test and fix |
| Passing tests with no edge cases | Add tests |
| Small, focused, readable diff | Trust |
Use the table as a quick filter. It maps common signals to actions.
Limitations
This workflow covers one PR type. It does not replace a security audit. It does not cover large refactors.
The free server is ephemeral. Do not store real user data there. The free model can produce different code on each run.
Every output needs a human review. Teams without a human reviewer should not use this approach.
The reviewer is the safety net. Removing the reviewer removes the safety.
Closing
Agent PRs are now part of daily work. The reviewer role is the new bottleneck. A short checklist makes that role manageable.
Run this workflow on the next agent-generated PR. The whole review fits in one free server session.
Top comments (0)