Skip to content

fix: Telegram callback persistence queue discards per-session ordering and spawns untracked goroutines when full#829

Closed
sam-saffron-jarvis wants to merge 1 commit into
SamSaffron:mainfrom
sam-saffron-jarvis:feat/codereview-998721dd
Closed

fix: Telegram callback persistence queue discards per-session ordering and spawns untracked goroutines when full#829
sam-saffron-jarvis wants to merge 1 commit into
SamSaffron:mainfrom
sam-saffron-jarvis:feat/codereview-998721dd

Conversation

@sam-saffron-jarvis

Copy link
Copy Markdown
Contributor

What changed

  • Removed the Telegram callback persistence queue overflow bypass that spawned runStoreOpWithoutCancel in an untracked goroutine when the 128-slot queue filled.
  • Made telegramStoreOpQueue.enqueue() always hand work to the single serialized worker so per-session persistence stays ordered even under backpressure.
  • Added TestTelegramStoreOpQueueFullStillSerializesOps to cover the full-queue case and assert that a trailing callback op does not escape the queue and that closeAndWait() does not return before it finishes.

Why this is high-value

The existing overflow path broke the core guarantee that this queue is a single serialized persistence lane for a Telegram session. Once the buffer filled, later writes could run concurrently with earlier queued writes, so transcript rows, metrics updates, and context-estimate updates could be persisted out of order. It also spawned unbounded 5-second store goroutines that closeAndWait() never tracked, which directly risks durability and responsiveness during slow-store or tool-heavy runs.

This fix applies backpressure instead of bypassing serialization, which preserves message ordering and ensures shutdown waits on the same persistence path that handled the callbacks.

Validation

  • gofmt -w internal/serve/telegram.go internal/serve/telegram_queue_test.go
  • go build ./...
  • go test ./...
  • Added go test ./internal/serve -run TestTelegramStoreOpQueueFullStillSerializesOps -count=1

@SamSaffron

Copy link
Copy Markdown
Owner

Closing as requested: the high-confidence fixes have been applied or superseded in the current working tree.

@SamSaffron SamSaffron closed this Jun 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants