fix(telegram): preserve auto-titled sessions and bind chat on switch (#121)#123
Merged
adolfousier merged 1 commit intoMay 26, 2026
Conversation
Fixes adolfousier#121. Stop label drift from overwriting auto-titled DM sessions; resolve sessions via chat_sessions before suffix fallback; apply idle timeout on the bound path; remove dead extra_sessions map. Adds session_resolve helpers and unit tests.
Owner
|
Checking those in a few. Last few days couldn't be more active as I am out and atm traveling back home. These updates should be up on today's release later on. |
Contributor
Author
|
Sure. BTW, im in real estate as well - been meaning to have a chat with you on that if you are interested. |
Contributor
Author
|
Meanwhile, can you approve CI tests on this PR? |
adolfousier
reviewed
May 26, 2026
Owner
There was a problem hiding this comment.
Finally back to the office and the audit is done. Approving with three non-blocking follow-up notes.
Root cause analysis is correct and finds the bug my own 1aeaca2a missed. My e2e test for #121 only sent one message, so the clobber on the second message slipped through. The full equality check session.title != session_title was reverting the auto-titled name to the default template on every subsequent message. should_refresh_label is the right fix.
Strengths:
- Clean separation in
session_resolve.rsso the logic is testable without teloxide. - Explicit policy: default to default different = refresh, group rename = refresh, auto-titled or custom = never clobber.
- Chat-bound resolution path fixes the secondary bug where
find_session_by_title_suffixwould return the wrong row after a /sessions switch (most recent updated_at winner). - Removes the
extra_sessionsHashMap, which was written but never read on ingest. - 13 new tests covering policy precedence, label drift matrix, idle expiration boundary, suffix lookup after switch.
Follow-up tasks I am working on (non-blocking):
-
No second-message end-to-end test. All new tests are pure helpers or DB direct writes. They prove
should_refresh_labelworks in isolation but do not drivehandle_messageacross two messages. Myauto_title_e2e_testalready exists atsrc/tests/auto_title_e2e_test.rsand would be a natural place to add a phase 3 that sends a second message and asserts the auto-titled name survives. I will add that after merge. -
Discord, Slack, and WhatsApp handlers likely have the same clobber pattern. The module name
session_resolveis good but it lives underchannels/telegram/. Worth lifting tochannels/session_resolve.rsso other handlers can share the helper. Out of scope here. -
The in-memory
chat_sessionsmap is process scoped, so the first message after an opencrabs restart misses the chat-bound path and falls through to suffix lookup. Not a bug (suffix still works), but a comment inchoose_resolve_sourceexplaining the cold start behavior would help future readers.
Merging locally now to ship in v0.3.29 along with my followup improvements.
Owner
That sounds great bro, let's have a chat. Please send me a DM on X (x.com/adolfousier) so we can exchange contacts. |
adolfousier
added a commit
that referenced
this pull request
May 26, 2026
… (PR #123 follow-up) Two improvements on top of the merged PR #123: 1. New `auto_titled_name_survives_second_message_round_trip` test in `auto_title_e2e_test.rs`. Sends two messages through the agent and asserts the auto-titled name from turn 1 survives turn 2. This is the test that should have existed before #121 was ever opened — it directly exercises the label-drift clobber scenario the PR fixed at the handler level. Locks in regression coverage so the next time someone "fixes" auto-title without thinking past the first message, the suite fails loudly. 2. Cold-start clarifying comment on `choose_resolve_source` in `session_resolve.rs`. The `chat_sessions` map is in-memory and process-scoped — after an opencrabs restart the first message in any chat hits the Suffix branch, not Create. The docs now spell that out so a future reader reading the policy in isolation doesn't misread an empty map as "create a new session". Verified: full suite 2939 passing (1 new), 22 ignored, 0 failed. Clippy clean. Fmt clean.
adolfousier
added a commit
that referenced
this pull request
May 26, 2026
… slack, whatsapp Discord/Slack/WhatsApp handlers looked up sessions via exact-title `find_session_by_title(&template)`. After the agent auto-renamed a row, the next message's lookup missed and a brand-new duplicate session was created on every turn — same root cause as the Telegram clobber bug PR #123 fixed, different symptom (orphan + duplicate instead of revert). Adds a channel-agnostic resolver in `channels::session_resolve`: - `chat_id_suffix(id)` — stable `[chat:<id>]` suffix shared with telegram - `resolve_or_create_channel_session(...)` — suffix-first lookup, legacy exact-title fallback with one-shot forward-migration to suffix format, idle archive + recreate, otherwise create. Each handler now builds its title with a per-channel stable id: - Discord DM: `[chat:discord-dm-<user_id>]` - Discord channel:`[chat:discord-<channel_id>]` - Slack DM: `[chat:slack-dm-<user_id>]` - Slack channel: `[chat:slack-<channel_id>]` - WhatsApp: `[chat:wa-<phone>]` Slack `/new` also runs the suffix-first lookup so auto-titled rows still get archived when the user starts a fresh session. Tests: 7 in src/tests/channel_session_resolve_test.rs covering suffix resolve, auto-rename survival, legacy forward-migration, fresh create, and idle archive + recreate.
adolfousier
added a commit
that referenced
this pull request
May 26, 2026
The file-top comment explicitly states all scenarios must run inside a
single `#[tokio::test]` because adding a second one causes the first to
fail with `Database("Failed to create message")` — shared/global state
in the AgentService stack breaks between-test isolation for this entry
point even though each test creates a UUID-keyed in-memory DB.
The PR #123 follow-up commit added `auto_titled_name_survives_second_
message_round_trip` as a separate `#[tokio::test]` without honoring
that constraint, regressing the suite. Folding the round-trip
assertions back into the original test as Phase 3 restores the
constraint and the full suite goes green again (2945 passing).
Contributor
Author
|
Nice follow ups! Shame I missed it. Will ping you on X. |
adolfousier
added a commit
that referenced
this pull request
May 27, 2026
…drift fix, in-memory test pool hardening 10 commits since v0.3.28. Patch release closing issue #121 (the reporter's session stuck on the default channel-generated title) on two fronts: reasoning models that return ONLY a Thinking block for the title call now fall back to extracting a candidate from that block (extract_title_candidate with pluck_title_from_thinking heuristic — last quoted phrase, falling back to last short sentence) instead of dropping the title silently, and the Telegram handler stops clobbering non-default session titles on every subsequent message via a new should_refresh_label policy plus explicit chat→session binding on /sessions switch (PR #123 by @leshchenko1979). Audit of Discord, Slack, and WhatsApp handlers found the same root-cause bug with a different symptom — exact-title find_session_by_title lookup orphaned auto-renamed sessions and created a duplicate row on every subsequent message — fixed by lifting Telegram's stable [chat:<id>] suffix pattern into a shared channels::session_resolve module that all three channels now use, with suffix-first lookup, legacy exact-title fallback, and one-shot forward-migration of pre-suffix rows. Test infra got two real bug fixes that were masking CI failures: the in-memory test pool switched from WAL journal mode (a no-op for :memory: but a source of spurious SQLITE_LOCKED 262 on shared-cache under concurrent writers in release mode) to MEMORY journal with a single serialized connection, eliminating cross-connection lock contention; a release-mode rfc3339 nanosecond collision flake in the profile registry test got a 1 ms guard so back-to-back Utc::now() calls produce strictly-ordered timestamps. Closes #121. 18 files changed, +1564/-292. Contributions from @leshchenko1979 (PR #123).
adolfousier
added a commit
that referenced
this pull request
May 28, 2026
adolfousier
added a commit
that referenced
this pull request
May 28, 2026
… (PR #123 follow-up) Two improvements on top of the merged PR #123: 1. New `auto_titled_name_survives_second_message_round_trip` test in `auto_title_e2e_test.rs`. Sends two messages through the agent and asserts the auto-titled name from turn 1 survives turn 2. This is the test that should have existed before #121 was ever opened — it directly exercises the label-drift clobber scenario the PR fixed at the handler level. Locks in regression coverage so the next time someone "fixes" auto-title without thinking past the first message, the suite fails loudly. 2. Cold-start clarifying comment on `choose_resolve_source` in `session_resolve.rs`. The `chat_sessions` map is in-memory and process-scoped — after an opencrabs restart the first message in any chat hits the Suffix branch, not Create. The docs now spell that out so a future reader reading the policy in isolation doesn't misread an empty map as "create a new session". Verified: full suite 2939 passing (1 new), 22 ignored, 0 failed. Clippy clean. Fmt clean.
adolfousier
added a commit
that referenced
this pull request
May 28, 2026
… slack, whatsapp Discord/Slack/WhatsApp handlers looked up sessions via exact-title `find_session_by_title(&template)`. After the agent auto-renamed a row, the next message's lookup missed and a brand-new duplicate session was created on every turn — same root cause as the Telegram clobber bug PR #123 fixed, different symptom (orphan + duplicate instead of revert). Adds a channel-agnostic resolver in `channels::session_resolve`: - `chat_id_suffix(id)` — stable `[chat:<id>]` suffix shared with telegram - `resolve_or_create_channel_session(...)` — suffix-first lookup, legacy exact-title fallback with one-shot forward-migration to suffix format, idle archive + recreate, otherwise create. Each handler now builds its title with a per-channel stable id: - Discord DM: `[chat:discord-dm-<user_id>]` - Discord channel:`[chat:discord-<channel_id>]` - Slack DM: `[chat:slack-dm-<user_id>]` - Slack channel: `[chat:slack-<channel_id>]` - WhatsApp: `[chat:wa-<phone>]` Slack `/new` also runs the suffix-first lookup so auto-titled rows still get archived when the user starts a fresh session. Tests: 7 in src/tests/channel_session_resolve_test.rs covering suffix resolve, auto-rename survival, legacy forward-migration, fresh create, and idle archive + recreate.
adolfousier
added a commit
that referenced
this pull request
May 28, 2026
The file-top comment explicitly states all scenarios must run inside a
single `#[tokio::test]` because adding a second one causes the first to
fail with `Database("Failed to create message")` — shared/global state
in the AgentService stack breaks between-test isolation for this entry
point even though each test creates a UUID-keyed in-memory DB.
The PR #123 follow-up commit added `auto_titled_name_survives_second_
message_round_trip` as a separate `#[tokio::test]` without honoring
that constraint, regressing the suite. Folding the round-trip
assertions back into the original test as Phase 3 restores the
constraint and the full suite goes green again (2945 passing).
adolfousier
added a commit
that referenced
this pull request
May 28, 2026
…drift fix, in-memory test pool hardening 10 commits since v0.3.28. Patch release closing issue #121 (the reporter's session stuck on the default channel-generated title) on two fronts: reasoning models that return ONLY a Thinking block for the title call now fall back to extracting a candidate from that block (extract_title_candidate with pluck_title_from_thinking heuristic — last quoted phrase, falling back to last short sentence) instead of dropping the title silently, and the Telegram handler stops clobbering non-default session titles on every subsequent message via a new should_refresh_label policy plus explicit chat→session binding on /sessions switch (PR #123 by @leshchenko1979). Audit of Discord, Slack, and WhatsApp handlers found the same root-cause bug with a different symptom — exact-title find_session_by_title lookup orphaned auto-renamed sessions and created a duplicate row on every subsequent message — fixed by lifting Telegram's stable [chat:<id>] suffix pattern into a shared channels::session_resolve module that all three channels now use, with suffix-first lookup, legacy exact-title fallback, and one-shot forward-migration of pre-suffix rows. Test infra got two real bug fixes that were masking CI failures: the in-memory test pool switched from WAL journal mode (a no-op for :memory: but a source of spurious SQLITE_LOCKED 262 on shared-cache under concurrent writers in release mode) to MEMORY journal with a single serialized connection, eliminating cross-connection lock contention; a release-mode rfc3339 nanosecond collision flake in the profile registry test got a 1 ms guard so back-to-back Utc::now() calls produce strictly-ordered timestamps. Closes #121. 18 files changed, +1564/-292. Contributions from @leshchenko1979 (PR #123).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #121: Telegram DM sessions kept the default
Telegram: DM … [chat:ID]title after auto-title ran,/sessionslooked like duplicate rows, and switching felt broken.Root cause: Label drift compared the full stored title to the default DM template on every message reuse. Auto-titled names were overwritten on message 2+;
auto_title_attemptedstayed true, so rename never retried.Fix:
session_resolve::should_refresh_label— never clobber auto-titled / custom DM titleshandle_message—chat_sessionsmap first, then suffix fallback; idle timeout on bound path too/newusessession_resolve::build_session_titleextra_sessionsremoved — guest switching usesregister_session_chatonly (same as owner)Test plan
cargo test telegram_session_resolve session_resolve::testsManual:
/new→ message → wait for auto-title → second message (title must stay non-default) →/sessionsswitch → confirm context follows the chosen session.Supersedes #122.