Skip to content

fix: prefer execution-local message routing metadata#1449

Merged
ilblackdragon merged 4 commits intostagingfrom
codex/fix-routine-telegram-routing
Mar 20, 2026
Merged

fix: prefer execution-local message routing metadata#1449
ilblackdragon merged 4 commits intostagingfrom
codex/fix-routine-telegram-routing

Conversation

@henrypark133
Copy link
Copy Markdown
Collaborator

Summary

This fixes a Telegram delivery bug where autonomous routine or resume-driven message calls could reuse stale shared chat routing context instead of the execution-local notify_* metadata.

Problem

When the last interactive turn came from the web gateway, the shared message tool context could stay pinned to gateway. A later autonomous send with notify_channel=telegram could then route to gateway instead of Telegram, and the message would be dropped because there was no gateway thread id.

Fix

  • make message prefer execution-local notify_channel and notify_user metadata over shared defaults
  • keep shared chat defaults only as a legacy fallback when no execution-local routing metadata exists
  • add a shared chat metadata helper so normal chat turns and approval resumes both pass the real routing target downstream
  • propagate notify_thread_id into gateway deliveries so intentional gateway sends stay visible in the originating thread

Validation

  • cargo fmt --all
  • cargo clippy --all-targets --all-features -- -D warnings
  • cargo test message_tool_ --lib
  • cargo test chat_tool_execution_metadata --lib
  • cargo test --test e2e_telegram_message_routing --features libsql
  • cargo test routine_event_trigger_telegram_channel_fires --test e2e_advanced_traces --features libsql

Copilot AI review requested due to automatic review settings March 20, 2026 03:10
@github-actions github-actions Bot added size: L 200-499 changed lines scope: agent Agent core (agent loop, router, scheduler) scope: tool/builtin Built-in tools risk: medium Business logic, config, or moderate-risk modules contributor: core 20+ merged PRs and removed size: L 200-499 changed lines labels Mar 20, 2026
@gemini-code-assist
Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a critical bug where autonomous message deliveries could incorrectly reuse outdated routing information, leading to messages being dropped or misdirected. The changes ensure that message routing consistently uses the most relevant, execution-local metadata, improving the reliability of notifications across different channels and scenarios.

Highlights

  • Execution-Local Metadata Preference: The message tool now prioritizes execution-local notify_channel and notify_user metadata over shared default routing contexts, preventing stale context reuse.
  • Shared Chat Metadata Helper: A new helper function, chat_tool_execution_metadata, was introduced to consistently generate routing metadata from IncomingMessage objects for various contexts, including normal chat turns and approval resumes.
  • Gateway Thread ID Propagation: The notify_thread_id from execution metadata is now propagated into gateway deliveries, ensuring that messages sent intentionally to the gateway remain visible in their originating threads.
  • Refactored Message Routing Logic: The internal logic for resolving message channels and targets within the MessageTool has been significantly refactored to implement the new metadata prioritization rules.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request correctly addresses a message routing bug by prioritizing execution-local metadata over shared conversation defaults. The changes are well-structured, centralizing metadata creation and applying the fix to both normal chat turns and approval-resumed operations. The new tests are comprehensive and validate the fix effectively. However, I've identified a critical logic bug in the new routing implementation within the message tool that could prevent fallback targets from being used correctly. My review includes a suggested fix for this issue.

Comment thread src/tools/builtin/message.rs Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a routing bug in the message built-in tool by making it prefer execution-local routing metadata (notify_channel / notify_user / notify_thread_id) over shared per-conversation defaults, preventing stale context (e.g., “gateway”) from hijacking autonomous Telegram deliveries.

Changes:

  • Update message tool routing resolution to prioritize execution-local metadata and only fall back to shared defaults when no routing metadata exists.
  • Add a shared helper to build consistent chat execution metadata for both normal turns and approval resumes.
  • Propagate notify_thread_id into gateway OutgoingResponse thread routing so gateway broadcasts aren’t silently dropped.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
src/tools/builtin/message.rs Adjusts channel/target resolution precedence and injects notify_thread_id into gateway responses; adds focused regression tests using recording channels.
src/agent/dispatcher.rs Switches chat JobContext.metadata construction to a shared helper for consistent routing.
src/agent/thread_ops.rs Ensures approval-resume tool executions also get correct routing metadata via the shared helper.
src/agent/agent_loop.rs Introduces chat_tool_execution_metadata() and adds unit tests for its routing-target selection behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/agent/agent_loop.rs
@github-actions github-actions Bot added the size: L 200-499 changed lines label Mar 20, 2026
@henrypark133 henrypark133 requested a review from Copilot March 20, 2026 03:50
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/agent/agent_loop.rs
Comment thread src/tools/builtin/message.rs Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/tools/builtin/message.rs Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ilblackdragon ilblackdragon merged commit b952d22 into staging Mar 20, 2026
18 checks passed
@ilblackdragon ilblackdragon deleted the codex/fix-routine-telegram-routing branch March 20, 2026 06:07
zmanian pushed a commit that referenced this pull request Mar 21, 2026
* fix: prefer execution-local message routing metadata

* test: cover message routing fallback metadata

* refactor: simplify message target resolution

* fix: ignore stale channel defaults for notify user metadata
zmanian pushed a commit that referenced this pull request Mar 21, 2026
* fix: prefer execution-local message routing metadata

* test: cover message routing fallback metadata

* refactor: simplify message target resolution

* fix: ignore stale channel defaults for notify user metadata
bkutasi pushed a commit to bkutasi/ironclaw that referenced this pull request Mar 28, 2026
* fix: prefer execution-local message routing metadata

* test: cover message routing fallback metadata

* refactor: simplify message target resolution

* fix: ignore stale channel defaults for notify user metadata
drchirag1991 pushed a commit to drchirag1991/ironclaw that referenced this pull request Apr 8, 2026
* fix: prefer execution-local message routing metadata

* test: cover message routing fallback metadata

* refactor: simplify message target resolution

* fix: ignore stale channel defaults for notify user metadata
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor: core 20+ merged PRs risk: medium Business logic, config, or moderate-risk modules scope: agent Agent core (agent loop, router, scheduler) scope: tool/builtin Built-in tools size: L 200-499 changed lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants