feat(server): add streamable HTTP stream resumability via pluggable EventStore - #930

server/streamable_http_resume.go (2)

374-400: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Heartbeats are recorded into the EventStore forever, even though replaying stale pings on resume is pointless.

Every periodic ping goes through st.deliver, which persists it via StoreEvent just like real application messages. For long-lived idle connections this permanently grows storage with data nobody needs to replay. Consider a "live-only" delivery path (skip StoreEvent) for heartbeat pings.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/streamable_http_resume.go` around lines 374 - 400, Update
startConnHeartbeat so periodic ping messages use a live-only delivery path that
sends them to the active stream without calling StoreEvent. Preserve the
existing heartbeat interval, request construction, and cancellation behavior
while ensuring heartbeat pings are never persisted for resume replay.

---

421-433: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

eventID is written into the SSE id: line without sanitization.

The EventStore interface documents that "any non-empty value is acceptable" for event IDs, but a custom implementation returning an ID containing \n/\r would corrupt the SSE framing (or inject extra fields) here, since eventID is interpolated raw into "id: %s\n".

🛡️ Proposed guard

 func writeSSEEventRaw(w io.Writer, eventID string, data json.RawMessage) error {
 	if eventID != "" {
+		if strings.ContainsAny(eventID, "\r\n") {
+			return fmt.Errorf("invalid SSE event id: contains control characters")
+		}
 		if _, err := fmt.Fprintf(w, "id: %s\n", eventID); err != nil {

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/streamable_http_resume.go` around lines 421 - 433, Update
writeSSEEventRaw to validate eventID before writing the id field, rejecting or
safely handling values containing carriage-return or newline characters so they
cannot alter SSE framing. Preserve the existing behavior for empty and otherwise
valid event IDs, including the current error propagation.

server/event_store.go (2)

19-30: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Distinguish "unknown ID" from other store failures with a sentinel error.

ReplayEventsAfter's single error return conflates "lastEventID not found" with any underlying store failure. handleResumeGet (server/streamable_http_resume.go) treats every error here as a 400 client error, which will misreport genuine backend failures (e.g. a Redis-backed store timing out) as "Unknown Last-Event-ID" instead of a 5xx.

As per coding guidelines, "Return sentinel errors (e.g., ErrMethodNotFound), wrap with fmt.Errorf("context: %w", err), and use errors.Is/As for checking."

♻️ Proposed sentinel error

+// ErrUnknownEventID indicates lastEventID is not known for the given session.
+var ErrUnknownEventID = errors.New("unknown event ID")

 type EventStore interface {
   ...
-	// ... It returns an error if lastEventID is not known
-	// for the given session, or if send returns an error.
+	// ... It returns an error wrapping ErrUnknownEventID if lastEventID is
+	// not known for the given session, or the error from send if it returns one.
 	ReplayEventsAfter(...) (string, error)
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/event_store.go` around lines 19 - 30, Introduce a package-level
sentinel error for an unknown last event ID and make ReplayEventsAfter return
it, wrapped with context where appropriate, only when the requested ID is
absent. Update handleResumeGet to use errors.Is against that sentinel so only
unknown IDs produce a 400 response; propagate other ReplayEventsAfter failures
as server errors.

Source: Coding guidelines

---

65-116: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add godoc comments to the exported StoreEvent/ReplayEventsAfter methods.

Neither method has its own doc comment (only the interface documents them).

As per coding guidelines, "All exported types and functions MUST have godoc comments starting with the name."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/event_store.go` around lines 65 - 116, Add GoDoc comments immediately
before the exported InMemoryEventStore methods StoreEvent and ReplayEventsAfter,
with each comment starting with the corresponding method name and briefly
describing its behavior.

Source: Coding guidelines

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes