Forty review rounds on code that had already been reviewed

Last time I wrote about skillmem — a local memory for coding agents — it was about a hole we found in it: a stranger's README could come back a day later looking like the user's own rule. Two independent reviews later, 0.10 closed it, and I believed the hole was one and it was shut.

0.11.0 shipped yesterday. Between the two releases: forty review rounds by two models on opposite sides (one Claude-side, one GPT-side), each reading the code as an adversary and required to reproduce every finding with a command, not an argument. Here is what came out, and why I no longer trust a clean review that isn't followed by another one.

The stop rule

"Review until there are no bugs" does not terminate: fresh eyes on any module always find something. We agreed on this instead: stop when two consecutive rounds produce no reproducible P1/P2 from either reviewer — data loss, wrong answer, a crossed trust boundary, a crash on realistic input. P3s go to the next release's issue.

It fired on rounds 39–40. The path there was not straight.

Three waves

Wave 1 (rounds 1–18): the audit. Six P1s, none in the Stop hook everyone had been staring at. HTTP /write could take over another agent's record; /learn wrote public skills without the permission; body files were shared between databases; trust could be granted by any process; kind was a path traversal in the exporter. Fixed; three rounds spent fixing what the fixes broke.

Wave 2 (rounds 19–23): rehearsing the release. I wrote the release procedure as a spec and handed it to the same reviewers. Rehearsing init on a copy of the config found that moving the venv doubled every hook — the recap would have run twice per session. Then, in a chain: config backups named by the second overwrote each other; they were created 0644 next to the file holding the OAuth account; the Codex backup was not byte-exact. Eleven P2s in installer code that "worked".

Wave 3 (rounds 24–40): fresh eyes on the core. The interesting part. Every time a reviewer got a module nobody had re-read, they found one to three old P2s inherited from main:

• The multi-agent HTTP server: a 409 conflict quoted the titles of another agent's private records. Backlinks on a public record named a private one's slug. /search, /list and /recall cut the page before the visibility filter — 110 of someone else's records and your own came back as an empty 200 while /get found it.
• import-vault followed a symlinked *.md out of the vault and stored the target — your ~/.zshrc, say. Attachments and packs already refused this; notes did not.
• skills rm <pack> removed every record under the pack's project, the owner's own note included.
• The Windows scheduler carried none of the environment launchd, cron and systemd did.
• My favourite: scrub, the function that redacts secrets before a write, was not idempotent. A value already rendered as [secret redacted] matched again on every re-write and grew into [secret redacted] redacted]. The content hash changed; the owner's approval was dropped. Restoring a dump over the same database silently stripped approvals.

Every second fix bred a regression

This is the lesson. The search-crowding fix took four iterations, and the next round caught each one:

1. Candidate window of 5, filtered after the limit: five hidden rows crowd yours out.
2. Window of 100: a hundred hidden rows do the same.
3. No limit, walk until N visible: without LIMIT, SQLite sorts every match before the first row comes out, and the wide SELECT dragged every body through the sort — 20 seconds per request on a 9k-row database.
4. Narrow walk, bodies only for the kept rows: 52 ms unfiltered, 73–87 filtered. Done? No: the predicate went to the master identity too, so master over HTTP ranked on an unbounded pool — a different top-5 than the CLI for five queries out of six.

Iteration five: master takes the unfiltered path, plus a test "HTTP master == S.search" that fails on the previous commit. Only then was the round clean.

Without a review after the first fix we would have shipped a 20-second /search, confident the leak was closed.

What is worth copying

• Two reviewers from different sides, not one. The models fail differently; over forty rounds the intersection of their findings was visibly smaller than the union.
• A rigid report format: P1|P2|P3 · file:line · what · evidence (command + output). "Looks racy" is a P3, not a gate.
• Reviewers don't write code. Twice in the cycle a reviewer broke read-only (an edit to pyproject.toml, a stray file beside the working clone) — check git status after every round.
• Every fix is a new round, one-liners especially.
• Name the stop rule up front, or the loop either never ends or ends where you got tired.

Cost: a day of machine time and ~350 tests instead of 221. Result, 0.11.0: the trust boundary closed on HTTP, MCP, the CLI and the hooks alike, and every line of the release notes tried with a command before it was written. What stayed P3 is in issue 0.11.1.

pip install skillmem · GitHub · Release notes