A notebook of mistakes
Most of the code in Cmssy is written by Claude Code and reviewed by me. That works because of one habit: every time something goes wrong, the agent writes a note. What happened, why, and how to apply the lesson next time.
The notes live in my Obsidian vault, and the agent reads the index for an area before it touches that area. There are over 500 of them now.
A large share is about tests. They keep arriving at the same sentence from different directions: a green test proves nothing until you have watched it fail. Below are five cases from that notebook, with dates. Two of them were security holes in my own product. Both are fixed.
What a note looks like
Every note has the same shape: a one-line description, the concrete incident, why it happened and how to apply the rule. This one is shortened and translated; the originals are in Polish.
---
name: guard-must-be-proven-to-refuse
description: A test where I set the header myself and get 200 proves the guard lets in, never that it refuses
type: feedback
---
**Why:** an admit test is green even when there is no guard at all.
The only proof that a guard exists is a request that should be refused, and is.
**How to apply:**
- Every guard = a pair of tests: the authorised request passes, the unauthorised one gets 401/403.
- Name the forged request in the test title.
- Before writing "this is not exposed" in a PR, send that request to a live deploy.1. The guard that looked for a string
July 29. The agent wrote two source guards around a field mapper. Both were green. Both were worthless.
The first one checked that a file contained the name toPropertyField. Simplified:
it("uses the shared field mapper", () => {
const source = readFileSync(mapperPath, "utf8");
expect(source).toContain("toPropertyField");
});It passed with the old, broken mapper fully restored, because toPropertyFields contains that name as a substring. The second guard detected a flat-to-nested translation, while the actual bug was dropped keys. A reviewing agent proved the point by writing a sixth mapper that had the bug and still passed.
The note written afterwards says that a test which cannot turn red is decoration, and that it raises risk because it looks like coverage. Assert the property ("nothing the caller can set is lost"), not the presence of a symbol. And when the bug is something missing, a source scan cannot see it by definition. That needs an invariant, such as a round trip.
2. The test that defended the vulnerability
October 4. The admin app's auth tests contained this:
it("never calls the backend - the token is already proof", async () => {
await authorize("oauth-token", { accessToken: accessToken() });
expect(authFetch).not.toHaveBeenCalled();
});That sentence was the vulnerability. The sign-in callback is a public endpoint, and authorize only decoded the middle segment of the JWT. No signature check, no issuer check. Anyone could assemble a token by hand and receive a valid admin session.
The suite was green for one reason: the test required that the backend was not asked. A token is proof only for someone who knows who minted it. A public endpoint takes the caller's word for it.
The fix shipped on October 7. What stayed is a way of reading tests in auth and tenancy code: the question is "what does this sentence claim about the world", not "does it pass". A test asserting that a verifying call did not happen is a warning sign. And a test like this gets deleted and named in the pull request, not softened. A softened version leaves the claim in the repo.
3. The guard that was only proven to let people in
September 10. SDK 16.7.0 shipped a block-data route, and the pull request said it "answers only a verified editor request". It did not. The guard read an x-cmssy-edit header. The proxy overwrites that header only on routes its matcher covers, and the standard matcher excludes /api/*. On an API route it was simply a header sent by the caller.
It got through because the local test set the header by hand, received 200 and recorded success. That measures that the mechanism lets a request in. Nobody asked whether it refuses one.
| Check | What it proves |
|---|---|
| Request with the header returns 200 | The route lets requests in. Also true with no guard at all |
| GET returns 405 | The route is mounted |
| POST with a forged header returns 403 | The guard refuses |
Every guard now comes as a pair: the authorised request passes, the unauthorised one gets 401 or 403. The first test alone is worth zero. The note adds a design rule too: mint the authorisation signal on the verified side and carry it in the request. A header set by middleware is trustworthy only on the routes that middleware really handles, so check the matcher, not the assumption.
4. The review advice that weakened the test
A reviewer suggested a sensible-sounding change: don't assert the graphql-js error message, because a minor release will reword it. Check that data is empty and that there is one error.
After applying it, the mutant t.int → t.float survived. With Float the fraction reaches the service-level guard, which also returns one error and empty data. The assertion stopped telling the two layers apart.
Review advice is usually about brittleness, not strength. A brittle message can be the only witness that separates two layers failing the same way. So every change that comes out of a review meets the same bar as the original assertion: run the mutant again after the change. If it survives, the advice was wrong.
5. The agent that crashed between the mutation and the restore
October 6, cmssy-sdk pull request #324. Two review agents died on HTTP 529 Overloaded. One of them had just started its mutation experiments. It crashed after changing the code and before restoring it, so a mutated type (formId: string turned into formId?: string | null) stayed in the worktree. There was no report, because the agent never came back.
Without a check, the next agent would have reviewed mutated code. Three rules came out of it. Run git status --short after every agent, including the ones that failed or timed out. Run mutating agents one at a time. Commit before letting an agent in, so a restore is exact.
The rule: mutate it red
All five cases end in the same procedure. Before a test counts as protection:
- Break exactly the property the test is supposed to defend.
- Run the test and see red.
- Restore the code.
- Report the mutation result, not just "green".
Two traps sit inside the procedure itself.
The mutation may never have landed. On September 26 a regex mutation removed "boolean" from a file where that token appears three times. It hit a different array than the intended one, the tests stayed green, and it read as a surviving mutant: a gap in the tests. It was a mutation that never touched its target. After a mutation, read the value, not the file.
A subagent's report is a hypothesis. In one review round twelve findings came back. Five were real bugs and three were wrong, and the wrong ones were specific and plausible. "Fixing" them would have broken working code. In another case ten agents agreed unanimously and were wrong. Every finding gets checked in the code before it reaches a plan or a pull request.
What this changed in the repo
The notes turned into mechanisms. A hook rejects assertions that cannot fail. Guard tests are documented with one house rule: a new guard is proven by mutation, and green on the first run proves nothing. A blind review by a separate agent runs before the pull request is opened, not after.
An AI agent does not make fewer mistakes than I do. It makes them faster. The notebook is how the same mistake does not get made twice, and a test is worth trusting only once I have watched it fail.