ReadyCIO
Menu

Playbook

Reviewing code you did not write

The review pass I run on everything an AI coding tool produces: what to read first, the mistakes that show up most, tests as the contract, and how to keep context so the next session does not repeat the last.

For AI-coders Updated September 5, 2026 reviewing ai codesecurity

Generated code fails in specific, predictable ways, so review it in a fixed order rather than wandering. This is the checklist I run on code from an AI coding tool before it goes anywhere near a client’s business. It is written for a solo engineer or a small team shipping real systems, and it assumes the tool is good, because it is.

Before anything is generated

The cheapest review happens before the code exists.

Interrogate the plan. Make the agent interview you before it builds anything touching a schema, an architecture or money.

The prompt is three sentences: ask about every aspect of the plan until we share an understanding; one question at a time; give your recommended answer with each; if the codebase can answer the question, read the codebase instead. Sessions run ten to fifty questions. What surfaces is the ambiguity that would otherwise be resolved by a silent wrong assumption: how is “complete” defined? Is that total per board or per project? What happens when a task moves backwards?

Write the answers down. A spec file in the repo or a decision note.

The agent will not remember the interrogation next session, and neither will you.

First read: shape

Read the diff once, fast, for structure. Where did the new code go? Does it follow the patterns already in the repo, or did it invent a parallel one?

The single most common problem with generated code is a second way of doing something the codebase already does: a new query helper beside the existing one, a new validation approach, a new place for configuration. Send it back to reuse the existing pattern before reading further. The second read is expensive and should be spent on code that will stay.

Second read: the places business rules live

Now read slowly, in trust-flow order. Click any item for what to look for.

  1. Authentication. Can this be reached without a valid identity?

    Look for optional secrets used in comparisons. A secret typed as optional, unset in one environment, compared with a template string becomes the literal “Bearer undefined” and authenticates anyone who sends it. Secrets fail closed when absent, and compare in constant time.

  2. Authorization. Given a valid identity, can it reach another user's objects?

    Middleware that sets the current user proves authentication, not authorization. For each handler: is the identity actually used in the query’s scope, or is the object addressed purely by an id from the URL? An id from the URL and no ownership check is the most common serious finding in generated code. Prefer 404 over 403 for objects the caller should not know exist.

  3. The documented invariant. Find the one path that skips the rule every other path applies.

    Find the sentence in the codebase that states a rule (“this join is the visibility gate”) and grep for every path that touches the same data. The bug is the one path that skips it. Generated code is very good at adding that path.

  4. Injection. Confirm parameterization rather than assuming it.

    Including dynamic fragments: ordering clauses, optional filters, hand-serialized arrays. Hand-rolled serializers are where escaping bugs hide.

  5. Outbound calls. Follow every server-side fetch back to the source of its URL.

    Any user-influenced part with no scheme and host validation is a server-side request forgery. If the response status is reflected to the caller, it is an exploitable one.

  6. Secrets and configuration. Nothing secret to the client or the logs.

    Anything optional is treated as fail-open until proven otherwise.

  7. What the database actually enforces. Check which role the connection string uses.

    If the app connects with a privileged role that bypasses row-level security, the policies are not a backstop for logic flaws. Check the role before believing “the policies protect us”.

Two cheap questions close with a grep: search for the dangerous HTML sinks and confirm none are used, and search for the risky query helper’s name and confirm every caller passes the scoping argument.

Tests as the contract

Generated tests tend to test what the generated code does, which proves nothing. Write, or make the tool write, tests from the plan: the behaviour you asked for, including the cases you named in the interrogation. If the code cannot pass tests written from the specification, the code is wrong, however clean it looks.

Where the rule lives in the database, the tests live there too. A policy test suite that proves each isolation rule is what lets you trust that a new feature inherits the isolation.

The check that runs forever

Tests cover the cases you thought of. Generated logic tends to be right on those and quietly wrong on the ones nobody considered. Quietly is the problem: nothing crashes, the number is just off.

So every system gets invariants: properties that must hold for every row, checked against real data. The header total equals the sum of the lines. A draft cannot become paid without passing approved. No row references a parent in another tenant. Counts agree with the external system. Evaluate them inline on the write path and again in a nightly sweep that catches drift from imports, migrations and manual edits.

Three rules keep this alive:

  • Log and continue, never throw. A wrong invariant blocking a user is worse than the bug. Hard-fail only tenant isolation and money.
  • Fix or delete a breached invariant within the week, or the alerts become wallpaper.
  • Tag every error with the version and commit that produced it, with a fingerprint so occurrences group. “This started in build 47” turns an afternoon of bisecting into a diff.

When to throw it away

If the second read finds a structural problem, do not patch the generated code. Re-prompt with the specific constraint the code violated and let it regenerate. Patching generated code by hand produces a hybrid nobody understands, and the next session will happily generate against the wrong half of it.

Keeping context between sessions

The reason a session repeats last week’s mistake is that nothing told it about last week. Keep, in the repo:

  • an instructions file under two hundred lines
  • a short architecture document that is actually re-read at session start
  • a decision log with reasoning
  • a one-line-per-entry memory of what worked and what did not

Update them before closing a session, and mid-session when something important is settled. A long session’s context gets summarized, and the thing decided an hour ago is what gets lost.

Cross-project knowledge, the kind that repeats across codebases, goes in a vault the agent reads at session start. That is a separate section.

The report shape

For a review with findings, one fixed shape per finding: where, exactly; the bug, as a mechanism not a feeling; the exploit, as concrete steps; the fix, minimal and reusing the repo’s own patterns. Then what was checked and cleared, so the coverage is visible, and what was not covered. One logical fix per commit, with a message that states the vulnerability, the exploit and the fix.

Changelog

  • 2026-09-05: first version.
  • 2026-09-08: trust-flow items collapsed to one-liners with the detail a click away; the three invariant rules and the repo files made into lists; short version added.