Review an agent's code by the same rules as a human's, but start with what the agent could not have known: who owns the object, which fields may change, which invariants the system keeps. Go from expensive mistakes to cosmetics: access control, input validation, secrets, error handling, dependencies, duplication, invariants and tests. The agent's green tests prove little on their own: check that they describe the requirement rather than restate the implementation.
After this you will be able to
- walk an agent's diff through an eight-area checklist — from access control to tests — ordered by the cost of a mistake
- find object-level access bypass, mass assignment, a swallowed error, a fallback secret and unneeded dependencies in a sample TypeScript handler
- fix the handler: a
zodschema instead of a type assertion, an ownership check, catalog prices, an honest failure - write requirement-based tests that fail on the agent's version and pass on the fixed one
Know beforehand
Verified with
TypeScript 7.0Vitest 5.0zod 4.6Node.js 24 LTS (24.20)Git 2.54
Checked against docs: October 12, 2026
Contents
- Why an agent's code needs its own review order
- The checklist: eight questions for an agent's changes
- A worked example: what the agent sent
- Going through the checklist
- The fix: a schema, the owner, the catalog, an honest error
- Verification: tsc and tests on both versions
- Making review part of the routine, and its limits
Why an agent's code needs its own review order
Generated code is usually tidy: it compiles, it is formatted, it comes with tests, and the tests pass. That does not make it bad, and it does not make it right. An agent's typical mistakes are not about syntax but about missing context. The agent does not know that prices come from the catalog, that a paid order must not change, or that the project already has a function for order totals — unless that is in the code, the task or the instructions file.
So the review goes from expensive to cheap. A mistake in access control or data is found by users after release and costs the most; a variable name can be fixed any time. Google's code review guide asks reviewers to read every line assigned to them and to check the tests separately: “tests do not test themselves”. With an agent this matters twice as much: the test and the implementation share one author and one set of assumptions.
The checklist: eight questions for an agent's changes
Access and object ownership
- Reviewer's question
- Does the server check that the user may change this particular object?
- Signal in the diff
- The user is passed in but never used; the database is queried by
idalone
Input validation
- Reviewer's question
- Are type, range and set of fields checked on the server?
- Signal in the diff
as SomeType,JSON.parsewithout a schema,Object.assign(entity, body)
Secrets and config
- Reviewer's question
- Are there keys or fallback values in code and logs?
- Signal in the diff
process.env.X ?? "…", a key in a test,console.logof the config
Error handling
- Reviewer's question
- Does the caller see a failure?
- Signal in the diff
- A
catchthat returns success or an empty value
Dependencies
- Reviewer's question
- Is the new package needed, does it exist, is it maintained?
- Signal in the diff
- New lines in
package.json; the package is never imported
Duplication
- Reviewer's question
- Does the project already have a function for this?
- Signal in the diff
- A home-grown formula next to an existing utility
Invariants
- Reviewer's question
- Which data rules could the change break?
- Signal in the diff
- A field the server should compute arrives from the client; statuses change unchecked
Tests
- Reviewer's question
- Do the tests check the requirement, including refusals?
- Signal in the diff
- A single happy-path test with data tailored to the implementation
The first three rows match the most common web application risks: in the OWASP Top 10:2025, broken access control is ranked first, and its examples include reaching someone else's record by its identifier. The other rows catch mistakes that are not necessarily vulnerabilities but make the system unpredictable.
A worked example: what the agent sent
The task for the agent: “build a PATCH /orders/:id handler that changes the items of an order”. The project already has types and a cartTotal function in src/pricing.ts. The types also state the invariant: the total is the sum of price * qty, and prices come from the catalog.
export type User = { id: string; role: "customer" | "admin" };
// Prices are integers in minor currency units (tiyn)
export type LineItem = { sku: string; price: number; qty: number };
export type Order = {
id: string;
ownerId: string;
status: "draft" | "paid";
items: LineItem[];
// Invariant: total = sum of price * qty, prices come from the catalog
total: number;
};
export interface OrderRepo {
find(id: string): Promise<Order | undefined>;
save(order: Order): Promise<void>;
}
export interface Catalog {
priceOf(sku: string): number | undefined;
}
export type Result = { status: number; body: unknown };The agent sent a handler, a config module and a test. This is a teaching example: the defects are packed into one small file on purpose, one per checklist row. Yet each of them looks plausible on its own and compiles.
import _ from "lodash";
import type { Order, OrderRepo, Result, User } from "./types.js";
// PATCH /orders/:id — update the items of an order
export async function updateOrder(
_user: User,
orderId: string,
input: unknown,
repo: OrderRepo,
): Promise<Result> {
try {
const body = input as Partial<Order>;
const order = await repo.find(orderId);
if (!order) return { status: 404, body: { error: "not_found" } };
const updated = _.cloneDeep(order);
Object.assign(updated, body);
updated.total = updated.items.reduce((sum, item) => sum + item.price * item.qty, 0);
await repo.save(updated);
console.log("order updated", updated);
return { status: 200, body: updated };
} catch (e) {
console.error(e);
return { status: 200, body: { ok: true } };
}
}export const config = {
paymentsApiKey: process.env.PAYMENTS_API_KEY ?? "demo-payments-key-123",
debug: true,
};// The test the agent wrote together with orders.defective.ts
import { expect, it } from "vitest";
import { updateOrder } from "../src/orders.defective.js";
import type { Order, OrderRepo } from "../src/types.js";
it("updates order", async () => {
const saved: Order[] = [];
const repo: OrderRepo = {
find: async () => ({ id: "o-1", ownerId: "u-1", status: "draft", items: [], total: 0 }),
save: async (order) => {
saved.push(order);
},
};
const input = { items: [{ sku: "SKU-CUP", price: 90000, qty: 2 }] };
const res = await updateOrder({ id: "u-1", role: "customer" }, "o-1", input, repo);
expect(res.status).toBe(200);
});Going through the checklist
Start with the overview of changes and dependencies: it is quick and immediately raises questions about the code.
$ git diff --stat HEAD~1
package.json | 3 +++
src/config.defective.ts | 4 ++++
src/orders.defective.ts | 27 +++++++++++++++++++++++++++
test/orders.agent.test.ts | 17 +++++++++++++++++
4 files changed, 51 insertions(+)
$ git diff HEAD~1 -- package.json
diff --git a/package.json b/package.json
index e9b512e..356e132 100644
--- a/package.json
+++ b/package.json
@@ -7,9 +7,12 @@
"test": "vitest run"
},
"dependencies": {
+ "lodash": "^4.18.1",
+ "request": "^2.88.2",
"zod": "^4.6.6"
},
"devDependencies": {
+ "@types/lodash": "^4.17.26",
"@types/node": "^26.6.5",
"typescript": "^7.0.2",
"vitest": "^5.0.4"
$ npm view request deprecated
request has been deprecated, see https://github.com/request/request/issues/3142
$ npm view lodash version
4.18.1
$ git grep -n -e "\"request\"" -- src || echo "request: no usages in src"
request: no usages in src
$ git grep -n "lodash" -- src
src/orders.defective.ts:1:import _ from "lodash";The agent added three packages. request is deprecated in the registry — npm deprecate exists precisely to warn everyone who installs it — and it is not imported anywhere. lodash is there for a single cloneDeep. Now the code itself, line by line:
_user: User
- What is wrong
- The user is never used. The underscore exempts it from
noUnusedParameters, so the compiler stayed quiet — and there is no ownership check - What it leads to
- Any signed-in user can change any order by trying
ids
input as Partial<Order>
- What is wrong
- A type assertion, not a check: according to the TypeScript docs, assertions are removed at compile time and check nothing at runtime
- What it leads to
- A quantity of 0, −1 or 1.5 reaches the database
Object.assign(updated, body)
- What is wrong
- Mass assignment: any request field is copied onto the order, including
ownerId,statusandprice - What it leads to
- A client hands the order to someone else, marks it paid or sets the price to 1
item.price * item.qty
- What is wrong
- The total uses the price from the request and its own formula instead of
cartTotal - What it leads to
- The price invariant is broken; when the pricing rules change, the two formulas drift apart
No status check
- What is wrong
- A paid order can be changed
- What it leads to
- The order contents no longer match the payment received
catch → 200 { ok: true }
- What is wrong
- A database failure looks like success
- What it leads to
- The client believes the order is saved; nobody sees the error
console.log("order updated", updated)
- What is wrong
- The whole order goes to the log
- What it leads to
- Personal data in logs and extra noise
?? "demo-payments-key-123"
- What is wrong
- A fallback value for a secret in code
- What it leads to
- The app starts silently without the real key; the string ends up in the repository
orders.agent.test.ts
- What is wrong
- The test sends the price in the request and only checks for status 200
- What it leads to
- The test locks in the defect instead of the requirement
Note that none of these findings shows up as a red CI status. The strict compiler and the agent's test both pass — the defects are found only by comparing the code with requirements that you know and the agent did not.
The fix: a schema, the owner, the catalog, an honest error
import { z } from "zod";
import { cartTotal } from "./pricing.js";
import type { Catalog, LineItem, Order, OrderRepo, Result, User } from "./types.js";
// Only these fields may come from the client: no price, owner, status or total
const UpdateOrderInput = z.strictObject({
items: z
.array(z.strictObject({ sku: z.string().min(1).max(64), qty: z.int().min(1).max(100) }))
.min(1)
.max(50),
});
// PATCH /orders/:id — update the items of a draft order
export async function updateOrder(
user: User,
orderId: string,
input: unknown,
repo: OrderRepo,
catalog: Catalog,
): Promise<Result> {
const parsed = UpdateOrderInput.safeParse(input);
if (!parsed.success) return { status: 400, body: { error: "invalid_input" } };
const order = await repo.find(orderId);
// Someone else's order looks exactly like a missing one
if (!order || (order.ownerId !== user.id && user.role !== "admin")) {
return { status: 404, body: { error: "not_found" } };
}
if (order.status !== "draft") return { status: 409, body: { error: "order_not_editable" } };
const items: LineItem[] = [];
for (const { sku, qty } of parsed.data.items) {
const price = catalog.priceOf(sku);
if (price === undefined) return { status: 400, body: { error: "unknown_sku", sku } };
items.push({ sku, price, qty });
}
const updated: Order = { ...order, items, total: cartTotal(items) };
await repo.save(updated); // a failure propagates to the shared error handler (500)
return { status: 200, body: updated };
}- A
zodschema checks the input: according to the docs,z.strictObjectrejects unknown keys, soprice,ownerIdandstatusin the request produce a 400 instead of being silently dropped. This is the mass assignment protection OWASP recommends: an explicit allow-list of fields. - Quantity is an integer from 1 to 100, with 1 to 50 line items. Business rules set these limits; here they are illustrative.
- Someone else's order is reported as missing (404) so as not to confirm that the
idexists. The lesson on authorization covers 401, 403 and 404 in detail. - Prices come from the catalog and the total from the shared
cartTotal. An unknown SKU is a 400. - A paid order cannot change — 409.
- A save failure is not caught: the shared error handler gets it, responds with 500 and logs it without the order data.
function requireEnv(name: string): string {
const value = process.env[name];
if (!value) throw new Error(`Missing required environment variable ${name}`);
return value;
}
// Called once at startup: no key, no start — instead of a silent fallback
export function loadConfig() {
return {
paymentsApiKey: requireEnv("PAYMENTS_API_KEY"),
debug: process.env.DEBUG === "1",
};
}Verification: tsc and tests on both versions
Write tests from the requirements, not from the code: each one checks a single rule, refusals included. The TARGET variable lets you run the same tests against the agent's version and confirm that they actually catch the defects.
import { describe, expect, it, vi } from "vitest";
import type { Catalog, Order, OrderRepo, User } from "../src/types.js";
// TARGET=defective: the agent's original version, TARGET=solution: the exercise solution
const { updateOrder }: typeof import("../src/orders.js") =
process.env.TARGET === "defective"
? await import("../src/orders.defective.js")
: process.env.TARGET === "solution"
? await import("../src/orders.solution.js")
: await import("../src/orders.js");
const alice: User = { id: "u-alice", role: "customer" };
const bob: User = { id: "u-bob", role: "customer" };
const prices: Record<string, number> = { "SKU-TEA": 150000, "SKU-CUP": 90000 };
const catalog: Catalog = { priceOf: (sku) => prices[sku] };
class MemoryRepo implements OrderRepo {
orders = new Map<string, Order>();
async find(id: string) {
const order = this.orders.get(id);
return order && structuredClone(order);
}
async save(order: Order) {
this.orders.set(order.id, structuredClone(order));
}
}
function setup(status: Order["status"] = "draft") {
const repo = new MemoryRepo();
repo.orders.set("o-1", {
id: "o-1",
ownerId: alice.id,
status,
items: [{ sku: "SKU-TEA", price: 150000, qty: 1 }],
total: 150000,
});
return repo;
}
describe("updateOrder", () => {
it("lets the owner change items and prices them from the catalog", async () => {
const repo = setup();
const res = await updateOrder(alice, "o-1", { items: [{ sku: "SKU-CUP", qty: 2 }] }, repo, catalog);
expect(res.status).toBe(200);
expect(repo.orders.get("o-1")?.total).toBe(180000);
});
it("hides another customer's order", async () => {
const repo = setup();
const res = await updateOrder(bob, "o-1", { items: [{ sku: "SKU-CUP", qty: 1 }] }, repo, catalog);
expect(res.status).toBe(404);
expect(repo.orders.get("o-1")?.items[0].sku).toBe("SKU-TEA");
});
it("rejects a price sent by the client", async () => {
const repo = setup();
const res = await updateOrder(alice, "o-1", { items: [{ sku: "SKU-CUP", qty: 1, price: 1 }] }, repo, catalog);
expect(res.status).toBe(400);
expect(repo.orders.get("o-1")?.total).toBe(150000);
});
it("does not let the client change owner or status", async () => {
const repo = setup();
const input = { items: [{ sku: "SKU-CUP", qty: 1 }], ownerId: bob.id, status: "paid" };
const res = await updateOrder(alice, "o-1", input, repo, catalog);
expect(res.status).toBe(400);
expect(repo.orders.get("o-1")?.ownerId).toBe(alice.id);
});
it.each([0, -1, 1.5, 101])("rejects quantity %s", async (qty) => {
const res = await updateOrder(alice, "o-1", { items: [{ sku: "SKU-CUP", qty }] }, setup(), catalog);
expect(res.status).toBe(400);
});
it("rejects an unknown SKU", async () => {
const res = await updateOrder(alice, "o-1", { items: [{ sku: "SKU-GOLD", qty: 1 }] }, setup(), catalog);
expect(res.status).toBe(400);
});
it("refuses to edit a paid order", async () => {
const res = await updateOrder(alice, "o-1", { items: [{ sku: "SKU-CUP", qty: 1 }] }, setup("paid"), catalog);
expect(res.status).toBe(409);
});
it("does not report success when saving fails", async () => {
const repo = setup();
vi.spyOn(repo, "save").mockRejectedValue(new Error("db is down"));
await expect(
updateOrder(alice, "o-1", { items: [{ sku: "SKU-CUP", qty: 1 }] }, repo, catalog),
).rejects.toThrow("db is down");
});
});$ npx tsc --noEmit -p . && echo "tsc: no errors"
tsc: no errors
$ npx vitest run test/orders.agent.test.ts 2>&1 | grep -E "✓|×|Tests "
✓ test/orders.agent.test.ts (1 test) 5ms
Tests 1 passed (1)
$ TARGET=defective npx vitest run test/orders.test.ts --reporter=verbose 2>&1 | grep -E "^ +(✓|×)|Tests "
× test/orders.test.ts > updateOrder > lets the owner change items and prices them from the catalog 12ms
× test/orders.test.ts > updateOrder > hides another customer's order 1ms
× test/orders.test.ts > updateOrder > rejects a price sent by the client 1ms
× test/orders.test.ts > updateOrder > does not let the client change owner or status 1ms
× test/orders.test.ts > updateOrder > rejects quantity 0 1ms
× test/orders.test.ts > updateOrder > rejects quantity -1 1ms
× test/orders.test.ts > updateOrder > rejects quantity 1.5 1ms
× test/orders.test.ts > updateOrder > rejects quantity 101 0ms
× test/orders.test.ts > updateOrder > rejects an unknown SKU 1ms
× test/orders.test.ts > updateOrder > refuses to edit a paid order 1ms
× test/orders.test.ts > updateOrder > does not report success when saving fails 4ms
⎯⎯⎯⎯⎯⎯ Failed Tests 11 ⎯⎯⎯⎯⎯⎯⎯
Tests 11 failed (11)
$ npx vitest run test/orders.test.ts test/config.test.ts --reporter=verbose 2>&1 | grep -E "^ +(✓|×)|Tests "
✓ test/config.test.ts > fails fast without the payments key 3ms
✓ test/config.test.ts > reads the key from the environment 0ms
✓ test/orders.test.ts > updateOrder > lets the owner change items and prices them from the catalog 4ms
✓ test/orders.test.ts > updateOrder > hides another customer's order 0ms
✓ test/orders.test.ts > updateOrder > rejects a price sent by the client 0ms
✓ test/orders.test.ts > updateOrder > does not let the client change owner or status 0ms
✓ test/orders.test.ts > updateOrder > rejects quantity 0 0ms
✓ test/orders.test.ts > updateOrder > rejects quantity -1 0ms
✓ test/orders.test.ts > updateOrder > rejects quantity 1.5 0ms
✓ test/orders.test.ts > updateOrder > rejects quantity 101 0ms
✓ test/orders.test.ts > updateOrder > rejects an unknown SKU 1ms
✓ test/orders.test.ts > updateOrder > refuses to edit a paid order 0ms
✓ test/orders.test.ts > updateOrder > does not report success when saving fails 2ms
Tests 13 passed (13)The agent's test is green on the agent's version. All eleven of our tests fail on that same version, including the happy path: the total comes out as NaN because the agent expected the price in the request. On the fixed version all thirteen pass — eleven handler tests and two config tests. A test that does not fail on any wrong implementation checks nothing; running it against the defective version is a simple way to see that.
Making review part of the routine, and its limits
- The agent's task lists invariants and roles: who may change what, which fields the server computes
- Strict compiler, linter and tests run in CI on every pull request
- New dependencies are checked in the registry: does the package exist under that name, is it deprecated, is it imported
- Secret scanning runs in CI; there are no fallback values for secrets in code
- Every change to access or data has a refusal test: someone else's object, an extra field, an out-of-range value
- The reviewer has read every line of the diff, not just the agent's report
The limits are worth stating plainly. Reviewing a diff only sees changed lines: it will not find a hole in code nobody touched, will not assess the architecture as a whole and does not replace a security assessment. OWASP ASVS (current version 5.0.0) is a useful reference for web applications, but reviewing one pull request does not establish compliance with it. If a product was built quickly and entirely with an assistant, it makes sense to check it once by area — access, data, secrets, tests, build — and then keep the review bar on every change.
How this was verified4 · passedchecked 12 October 2026
“Run” means the code was executed in a scratch project or container and the output in the article comes from that run. “Partial” means it was checked against stubs instead of external services. “Not run” means it needs a device, an account or a key; the steps were checked against the official documentation. The verification project files and run logs are kept in the InterLAB archive.