feat(telegram): interactive resume session picker with inline keyboard - #49038
feat(telegram): interactive resume session picker with inline keyboard#49038eltecnicowd wants to merge 3 commits into
Conversation
Native notifications (approval / sudo / secret / clarify) are tagged with the gateway *runtime* session id — the key under which the session lives in the gateway's in-memory `_sessions` map and the id every event carries (`tui_gateway/server.py` `_emit(event, sid, ...)`). The chat route, however, is keyed by the *stored* session id (`stored_session_id`), which is a different value: a new chat gets its runtime id immediately but its stored id only once the first turn persists. `onFocusSession` navigated straight to `sessionRoute(<runtime id>)`, so clicking a notification (e.g. an approval prompt) sent the route-resume path a runtime id where it expects a stored id. `useRouteResume` then resumed it as a stored session -> REST `/api/sessions/<runtime id>` 404 "session not found", and the running session was navigated away, which the user experiences as the session being destroyed. Translate runtime -> stored before navigating via the existing `runtimeIdByStoredSessionId` map (new `storedSessionIdForNotification` helper), falling back to the id as-is when no mapping is known. The Approve/Reject notification button path is untouched: `approval.respond` is routed by the runtime id (`_sess()` -> `_sessions[session_id]`), so it must keep carrying the runtime id.
Unit-test `storedSessionIdForNotification`: runtime ids resolve to their stored id, unknown ids and empty maps pass through unchanged, the right stored id is picked among several sessions, and stored ids (map keys) are never rewritten.
Adds an interactive inline-keyboard picker for /resume on Telegram, replacing the numbered text list with tap-to-select buttons. - TelegramAdapter: send_resume_picker, _handle_resume_picker_callback, _build_resume_keyboard with pagination (8 per page, Prev/Next nav) - GatewaySlashCommandsMixin: _resume_session_by_id callback wired in - SlashCommandsMixin._handle_resume_command checks for send_resume_picker capability on the adapter and uses the picker when available - Session list query expanded from 10 to 50 for better coverage Backward compatible: platforms without send_resume_picker keep the existing numbered text response.
f928ad2 to
67c6a76
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for addressing a real mobile UX gap: current main still renders bare gateway /resume as a numbered text list (gateway/slash_commands.py:3556-3584).
Problems
- The new callback dispatch (
plugins/platforms/telegram/adapter.py:4236-4240) reaches a session-switch callback without authenticating the tapping user or rechecking target visibility. Current/resumehas an explicit IDOR guard atgateway/slash_commands.py:3633-3640; the picker needs the same protection. - The proposed helper uses synchronous
session_store.switch_session(gateway/slash_commands.py:2949in this PR). Current main requires the async boundary atgateway/slash_commands.py:3651and also clears session-scoped model state at3656-3675. - Picker state is keyed only by chat (
plugins/platforms/telegram/adapter.py:3661), so another picker in the same group can overwrite an existing keyboard's state. - The PR has no tests; current picker tests provide a useful pattern in
tests/gateway/test_telegram_model_picker.py.
Suggested changes
- Rebase the behavior on a shared current-main resume helper that accepts the callback
SessionSource, preserves authorization/continuation handling, and usesasync_session_store. - Authorize callbacks, bind state to message ID plus source, and add authorization/concurrency/pagination regression tests.
Automated hermes-sweeper review.
| query_user_name = getattr(query.from_user, "first_name", None) | ||
|
|
||
| # --- Resume picker callbacks --- | ||
| if data.startswith(("rs:", "rg:", "rx")): |
There was a problem hiding this comment.
Please authorize this callback and route selection through the same target-visibility guard as text /resume. As written, any participant who can tap a picker in a shared chat reaches the selection callback; current main protects this boundary in gateway/slash_commands.py:3633-3640.
| if not self._session_db: | ||
| return "Session database not available." | ||
| self._release_running_agent_state(session_key) | ||
| new_entry = self.session_store.switch_session(session_key, session_id) |
There was a problem hiding this comment.
This needs to use current main's async SessionStore boundary (await self.async_session_store.switch_session(...) at gateway/slash_commands.py:3651) and preserve the model/reasoning cache cleanup immediately following it. The synchronous store call is stale after commit 9d38a2309ece.
| message_thread_id=int(thread_id) if thread_id else None, | ||
| **self._link_preview_kwargs(), | ||
| ) | ||
| self._resume_picker_state[chat_key] = { |
There was a problem hiding this comment.
A chat-wide key permits only one active picker: a later /resume in another topic or by another user overwrites this state while the old keyboard remains tappable. Key state by the picker message ID and bind it to the originating source.
GottZ
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Summary
Three PRs address or reference the Telegram /resume UX gap. #33702 implements a basic 12-session inline list but mixes in unrelated cron retry changes, #33725 has no cached code changes and therefore cannot substantiate its claimed implementation, and #49038 implements a paginated picker but does not preserve the existing authorization and async session-switch safeguards.
Related pull requests
- #33702 [closed]
related— (+666/-1) — do not revive: The closed PR implements Telegram resume buttons, callback routing, session filtering, and tests, but its diff also includes unrelated cron retry and auto-pause logic; it remains relevant as an earlier implementation of the same UX direction, not as a clean merge candidate. - #33725 [closed]
related— (+0/-0) — no implementation to merge: The closed PR has an empty complete cached diff, so its resume-picker scope and status as a scoped resubmission of #33702 are not confirmed; the contributor discussion only says that relationship and scope appear to be the case. - #49038
related— (+186/-2) — keep open pending fixes: The paginated Telegram picker directly addresses the mobile/resumefriction, but the visible keep_open review on #49038 identifies diff-backed blockers: callbacks lack user authorization and target-visibility checks, session switching bypasses the current async boundary and model-state cleanup, picker state is keyed only by chat, and no tests are included.
Duplicates
#33702 and #49038 substantially overlap in implementing a Telegram inline-button /resume picker, although their designs and scope differ. #33725 shares the title and claimed approach, but its empty diff is insufficient to classify it as a substantive code duplicate.
Suggested consolidation
Consolidate on #49038, but do not merge it yet: retain the contributor's keep_open verdict until the authorization/IDOR guard, async session-switch path with complete state cleanup, collision-safe picker state, and automated tests are added. Keep #33702 closed because its implementation contains unrelated cron scope, and keep #33725 closed because it provides no code to consolidate or merge.
Cross-PR triage: Reviewed 3 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 41 kB of PR diffs, 4 kB of issue/PR text, 2 kB of discussion (2 comments), 1 verify verdict. verdicts reflect diff content, not PR titles. Part of an automated triage batch.
Replace the numbered text reply for /sessions on Telegram with an inline-keyboard picker. Each session is a button; tapping it resumes that session directly. The currently-active session is filtered out so the list only shows resumable targets. Pickers paginate when there are more than 8 sessions (Prev / N/M / Next), and a Cancel button dismisses the picker. Buttons render the title on its own line and the first-message preview on a second line (truncated to 40 chars, 64-char Telegram cap). Built on the same picker state pattern as the existing model picker / choice picker / approval picker — but with the security-aware fixes that the blocker sweep on NousResearch#43695 and NousResearch#49038 flagged: 1. IDOR guard at the runner boundary. The picker's on_session_selected callback re-runs _resume_target_allowed with the captured SessionSource before delegating to the shared _resume_session_by_id helper. A co-member in a shared group cannot tap a button to bind to another user's persisted session — same gate the text /resume <id> path uses. The adapter also re-checks via _is_callback_user_authorized for a cheap fail-closed layer. 2. Collision-safe state key. State is keyed by (chat_id, msg_id, thread_id) instead of chat_id alone. A second /sessions opened in the same chat (forum threads, /sessions called twice in a row) cannot overwrite the first picker's state, and a stale click on the old keyboard after a new /sessions has replaced it is rejected at the adapter before the runner is invoked. 3. Session-switch via the funnel. _resume_session_by_id uses async_session_store.switch_session and calls _release_running_agent_state + _clear_conversation_scope + _evict_cached_agent — the same funnel that fixed the bug-class regressions NousResearch#10702, NousResearch#58403, NousResearch#6672. The text /resume path still uses the inline switch logic (kept distinct to preserve the Matrix --cross-room branch which needs source-object-aware title substitution). 4. Origin-scoped listing. The picker receives only the rows the runner already filtered through _resume_row_visible + _resume_target_allowed — same scope the text list uses, so the picker cannot bypass the IDOR guard that the listing already enforces. The picker branch lifts the legacy 10-cap (text fallback still caps at 10) so the picker can paginate through the full origin-scoped list (up to 50). 5. Authorization gate at the adapter. Mirrors the approval / choice picker pattern: a co-member tap is rejected at the Telegram adapter before the runner callback runs. Tests: tests/gateway/test_telegram_sessions_picker.py — 16 tests tests/gateway/test_sessions_command_picker_integration.py — 6 tests Regression-safe: existing 27 tests in test_resume_command.py all still pass. 49 passed. Why not just merge NousResearch#43695 / NousResearch#49038: both were kept open by the hermes-sweeper with the same four blockers. Salvage credit: the pagination shape, cancel button, and adapter scaffold follow the pattern eltecnicowd opened in NousResearch#49038. Rebased on top of the current plugins/platforms/telegram/adapter.py (the path NousResearch#43695 conflicted on).
What
Replaces the static numbered text list on Telegram
/resumewith an interactive inline-keyboard picker. Tapping a session name resumes it immediately -- no typing numbers.Why
The current text list is fine for CLI but clunky on mobile. Three taps (type number, send, wait) vs one tap with the picker. This matters when you have 20+ named sessions.
How
TelegramAdaptergainssend_resume_picker,_handle_resume_picker_callback,_build_resume_keyboardwith pagination (8 per page, Prev/Next nav)GatewaySlashCommandsMixingets_resume_session_by_id-- does the actual session switch and returns the confirmation text_handle_resume_commandchecks forsend_resume_pickeron the adapter before falling back to the text listBackward compatibility
Platforms without
send_resume_picker(Discord, Slack, Matrix, CLI) keep the existing numbered text response. Zero impact.Testing
Manual on running gateway with 30+ named sessions. Picker appears on bare
/resume, navigating pages works, tapping a session switches correctly, cancel dismisses the keyboard, expired state shows a hint to run/resumeagain.