fix(gateway): fail-closed session destination ownership + isolated workers#66703
fix(gateway): fail-closed session destination ownership + isolated workers#66703bogagyn wants to merge 2 commits into
Conversation
…workers One physical unthreaded Telegram DM may have a single foreground owner. Background workers and handoffs must opt into a distinct thread/topic; kanban creator wake is off by default and still refuses root-chat wake. Runtime footer exposes role/thread/title for ownership visibility.
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Comment
+582/-9 gateway fix for fail-closed session destination ownership and isolated workers. Moves kanban watch subscription cleanup into the session lifecycle. Security-sensitive change (process ownership). The delta is substantial but scoped to two files. No obvious issues spotted.
Reviewed by Hermes Agent
teknium1
left a comment
There was a problem hiding this comment.
Thanks for targeting a real session-ownership hazard: current main constructs every Kanban creator wake as chat_type="group" despite documenting that the real type is unavailable (gateway/kanban_watchers.py:517-548).
Problems
gateway/run.py:7824makes every no-thread handoff fail. That regresses the existing, intentional fallback documented ingateway/run.py:7779-7784andgateway/platforms/base.py:2984-3000;/handoffis still documented for WhatsApp, Signal, and Matrix (website/docs/reference/slash-commands.md:63). This is a behavior change even for a single-owner destination.- The footer title lookup added near
gateway/run.py:12350occurs before checking whether the footer is enabled.display.runtime_footer.enableddefaults to false (hermes_cli/config.py:1978-1981), so this adds a SessionDB query to normal gateway replies that will not render a footer.
Suggested changes
- Preserve the no-thread single-owner handoff path, or redesign it around a verified destination-owner check; cover adapters returning
Nonefromcreate_handoff_thread. - Resolve footer configuration before fetching the title, and fetch it only for an enabled footer requesting
title.
Automated hermes-sweeper review.
| # chat: a handoff is a new conversational owner and must be isolated. | ||
| effective_thread_id = new_thread_id or ( | ||
| str(home.thread_id) if home.thread_id else None | ||
| ) |
There was a problem hiding this comment.
Blocking: this turns every adapter without thread support, and every thread-creation failure, into a failed /handoff. Current main deliberately falls back to the home channel in this case (gateway/run.py:7779-7784; BasePlatformAdapter.create_handoff_thread documents it at gateway/platforms/base.py:2984-3000). Preserve a verified single-owner fallback or update the design and supported-platform contract together.
| @@ -12322,13 +12349,32 @@ async def _handle_message_with_agent(self, event, source, _quick_key: str, run_g | |||
| _footer_line = "" | |||
| try: | |||
There was a problem hiding this comment.
The runtime footer is disabled by default, but this title lookup runs before footer enablement and field selection. Resolve the footer config first and query SessionDB only when an enabled footer requests title.
Summary
Two commits hardening Telegram (and general platform) session destination ownership and gateway restart/footer behavior, derived from operational soak on a VPS gateway:
d509ad10e— fix(gateway): harden restart notice and runtime footersend_path_degradedrace where the post-boot "restarted" notice could fire before Telegram was connected).total_tokens,cost,profile,session_id.2b110b438— fix(gateway): fail-closed session destination ownership and isolated workersSessionOwnershipConflictraised on a second unthreaded DM destination owner (prevents split-owner routing)._require_isolated_handoff_thread).kanban.wake_creator_session: falsedefault); even when enabled, it refuses root-chat wake.role/title/thread.No behavior change for correctly-configured single-owner setups; these only add fail-closed guards and fix a startup-notify race. Upstream
origin/mainhas no equivalent of these contracts.Test plan
tests/gateway/test_session_destination_ownership.py,test_handoff_thread_isolation.py,test_kanban_creator_wake_policy.py,test_runtime_footer_ownership.py,tests/hermes_cli/test_kanban_creator_wake_default.py,test_runtime_footer.py,test_restart_notification.pygit statusclean; branch is 2 commits ahead / 0 behindorigin/main.Notes
🤖 Generated with Claude Code