Record the two conventions the live v2 bugs produced
Build image / build-and-push (push) Successful in 9s
Build image / build-and-push (push) Successful in 9s
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) <[email protected]> Claude-Session: https://claude.ai/code/session_01H3zbym8Doka2d7D48maSgZ
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
Reference in New Issue
Block a user