Commit history detached from cancellation on every path, not just failure #73

Merged

steve merged 2 commits from fix/commit-survives-disconnect into main 2026-07-21 12:49:18 +00:00

Conversation 4 Commits 2 Files Changed 3

+181 -22

2 Commits

Author	SHA1	Message	Date
steveandClaude Opus 4.8	d59303238f	Route auto-scoped mutations through the detached commit too ... Gadfly caught me making the same mistake one level up. This PR claimed the history write is "detached from cancellation, always" — and record()'s auto-scope path still called store.WriteChangeSet directly with the caller's context, so every plain REST mutation kept the orphan-history window the PR was written to close. That is the path virtually every change takes; the agent is the exception. It goes through commitScope now, which is where the rule lives, so no caller can be the one that forgets. Tested the same way as the agent path: cancel right after the mutation returns, assert the change set exists and reverts. Also: stopBeat is deferred so a panic in the run can't leak the ticker goroutine (and is idempotent, since the handler stops it explicitly on the normal path); the keep-alive interval is a named constant rather than a literal duplicated between comment and call site; and commitScope's doc states the rule and why it lives there, instead of narrating the incident that produced it — that belongs in a commit message, which is where it now is. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01H3zbym8Doka2d7D48maSgZ	2026-07-21 08:48:43 -04:00
steveandClaude Opus 4.8	9ef4593fa8	Commit history detached from cancellation on EVERY path, not just failure ... Found by using it: the agent planted 18 beans, the client's stream dropped, and those 18 plantings ended up in the garden with no change set behind them — real changes with no way to undo them, which is the one guarantee this whole design rests on. The earlier fix detached the FAILURE path and reasoned that a cancelled context is why fn failed. That missed the commoner case. An agent turn whose client disconnects can still COMPLETE — the model finishes, the tools have already written — and then the success path committed with a dead context, the write failed, WithChangeSet returned an error, and the work was orphaned. commitScope now detaches from cancellation itself, so every caller gets it. That is the right home for the rule: by the time a commit runs, the data it describes has already been written, so cancelling it cannot undo anything — it can only lose the record of what happened. There is no path on which that is the behaviour anyone wants. Also added an SSE keep-alive, since the dropped connection is worth not having in the first place: a model thinking between tool calls sends nothing for a while, and an idle proxy will cut a quiet stream. A comment frame every 20s keeps it open, and SSE ignores comment frames so the client is unaffected. That introduced a second goroutine writing the response, so the event stream now serializes writes behind a mutex — step events come from the agent's run goroutine while the ticker writes from its own, and two goroutines writing a ResponseWriter concurrently corrupts frames long before it crashes anything. Verified under -race. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01H3zbym8Doka2d7D48maSgZ	2026-07-21 08:38:24 -04:00