From c1c873d3f0dbacf71ac9da58b5392d7640963012 Mon Sep 17 00:00:00 2001 From: Steve Dudenhoeffer Date: Tue, 21 Jul 2026 09:20:00 -0400 Subject: [PATCH] Record the two conventions the live v2 bugs produced MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The history write's context.WithoutCancel reads like a mistake if you don't know why it's there, and "tidying" it is exactly how the orphan-history bug came back a second time (#73). Written down so the next person — or the next session — doesn't remove it on the way past. Also a short testing section. Two of the three bugs only real use found were invisible to the tests I had: a route that was never registered (service tests can't see that) and a fixture that populated a field the real response leaves empty (a test asserting my mental model rather than the API). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01H3zbym8Doka2d7D48maSgZ --- CLAUDE.md | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index f522e8b..58cab74 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -92,6 +92,29 @@ Frontend: React 19 + Vite + Tailwind 4 + TanStack Router/Query, built into - **Every service mutation lands in history** (#48). If you add one, record it — see `internal/service/revisions.go`. Multi-row operations pass all their changes to a single `record` call so they undo as one unit. +- **The history write is detached from cancellation on purpose.** `commitScope` + calls `context.WithoutCancel` — that is not a mistake to tidy up. By the time + a commit runs, the rows it describes are already written, so cancelling it + cannot undo anything; it can only leave real changes with no way to undo them. + This was a live bug twice (#73): a client disconnect mid-request orphaned 18 + plantings. Fixing it per-call-site is how it came back, which is why the rule + lives in `commitScope` where no caller can forget it. + +## Testing + +Match the test to the failure it would catch: + +- **Anything addressed by its own id needs an API-level test through the router.** + Service tests can't see a route that was never registered — PATCH/DELETE + `/journal/:id` once shipped fully implemented, fully unit-tested, and + completely unreachable. +- **Watch for fixtures that assert your assumptions instead of the API.** A test + for the undo message passed because the fixture I wrote populated a field the + real response leaves empty. If a test builds the thing it's testing against, + it is checking your mental model, not the system. +- Some things only real use finds. The agent's whole loop is covered by + majordomo's scriptable fake provider (`provider/fake`), which is worth using — + but the three worst v2 bugs all turned up in one live session afterwards. ## Workflow