Summary
Session.mu is documented to protect Messages, but several hot paths read/iterate sess.Messages without the lock, racing with HTTP AddMessage/compaction that mutate a live session. Risks: data races (race-detector visible), wrong SessionPosition in emitted events, and index-out-of-range panics if compaction truncates concurrently.
Locations
pkg/runtime/loop.go:62, 375, 963 — len(sess.Messages)-1 for UserMessage event positions
pkg/runtime/runtime.go:1573-1574 — slices.Backward(sess.Messages) on session restore
pkg/runtime/compactor/compactor.go:346 — len(sess.Messages) in firstKeptSessionIndex
pkg/server/session_manager.go:468 — ForkSession's ordinal map iterates s.Messages on a live shared pointer (InMemorySessionStore.GetSession returns the live *Session)
pkg/session/branch.go:39-52 — branch/fork reads parent.Messages unlocked (contrast Clone at :72-73 which takes RLock)
Fix
- Add/use locked accessors (e.g.
MessageCount(), snapshot-based iteration à la snapshotItems).
- Make
branch.go fork path take the RLock like Clone does; have ForkSession operate on a locked snapshot.
- 409-busy guard (folded in per audit decision): additionally reject HTTP
AddMessage/UpdateMessage with 409 Conflict while the session has an active stream, as a design-level safeguard on top of the locking.
Acceptance
- No data race under
go test -race on pkg/session, pkg/runtime, pkg/server, pkg/app.
- Race regression tests pinning the fix (pattern:
pkg/session/session_race_test.go).
- 409 returned for mutation attempts during an active stream, with a handler test.
Summary
Session.muis documented to protectMessages, but several hot paths read/iteratesess.Messageswithout the lock, racing with HTTPAddMessage/compaction that mutate a live session. Risks: data races (race-detector visible), wrongSessionPositionin emitted events, and index-out-of-range panics if compaction truncates concurrently.Locations
pkg/runtime/loop.go:62, 375, 963—len(sess.Messages)-1for UserMessage event positionspkg/runtime/runtime.go:1573-1574—slices.Backward(sess.Messages)on session restorepkg/runtime/compactor/compactor.go:346—len(sess.Messages)in firstKeptSessionIndexpkg/server/session_manager.go:468— ForkSession's ordinal map iteratess.Messageson a live shared pointer (InMemorySessionStore.GetSessionreturns the live*Session)pkg/session/branch.go:39-52— branch/fork readsparent.Messagesunlocked (contrastCloneat :72-73 which takes RLock)Fix
MessageCount(), snapshot-based iteration à lasnapshotItems).branch.gofork path take the RLock likeClonedoes; haveForkSessionoperate on a locked snapshot.AddMessage/UpdateMessagewith409 Conflictwhile the session has an active stream, as a design-level safeguard on top of the locking.Acceptance
go test -raceon pkg/session, pkg/runtime, pkg/server, pkg/app.pkg/session/session_race_test.go).