Skip to content

fix(conversation): avoid creating a conversation on chat reconnect#42

Closed
xinkuleee wants to merge 1 commit into
microsoft:mainfrom
xinkuleee:fix/conversation-reconnect-ensure-conversation
Closed

fix(conversation): avoid creating a conversation on chat reconnect#42
xinkuleee wants to merge 1 commit into
microsoft:mainfrom
xinkuleee:fix/conversation-reconnect-ensure-conversation

Conversation

@xinkuleee

Copy link
Copy Markdown

Summary

Reconnect previously called ensureConversation (get-or-create). Reconnect
only resumes an in-progress turn, so when a client reconnects to an agent
instance that has no existing conversation, creating one is both unnecessary and
a side effect — it leaves a stray empty conversation and then immediately ends
the stream because there is no ongoing turn to resume.

This changes Reconnect to look the conversation up read-only
(conversationRepo.Get). If none exists, it sends the terminal done event and
returns without creating a conversation or touching the ongoing-turn cache.
Chat still uses ensureConversation (first-turn creation is expected there).

Validation

  • make precommit-run
  • cd backend && go test ./...
  • cd core && uv run pytest
  • Frontend build/lint, if working from a frontend source checkout
  • Not run; explain below

Backend-only change. make precommit-run and full go test ./... pass. Core
(uv run pytest) and frontend suites were not run because neither is touched by
this change.

Checklist

  • Linked relevant issues or explained why there is no issue.
  • Updated docs and examples where behavior changed.
  • Updated CHANGELOG.md under ## [Unreleased] for user-facing changes.
  • Regenerated and committed generated files after proto, Wire, or OpenAPI changes.
  • Confirmed no secrets, tokens, credentials, or sensitive logs are included.

Notes for reviewers

  • New test TestReconnect covers the "no conversation -> don't create, send
    done" path. The cache-backed resume paths (lookupOngoingTurnID /
    replayCachedResponses) still require Redis and aren't unit-tested here;
    s.cache is a concrete *redis.Client, so covering them would need miniredis
    or a cache interface — flagged as a possible follow-up.
  • Files changed: chat.go (the fix) and chat_test.go (the test), 2 files total.

@xinkuleee xinkuleee closed this Jul 7, 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.

1 participant