Fix Telegram auto-verify flow and routing#1273
Merged
henrypark133 merged 5 commits intostagingfrom Mar 17, 2026
Merged
Conversation
github-actions
bot
added
scope: agent
Contributor
Summary of ChangesHello, 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 significantly enhances the Telegram integration by streamlining the owner verification process, making it more intuitive and less prone to user interface disruptions. It also refines how messages are routed within Telegram channels, ensuring that notifications and routine-triggered messages are delivered accurately. The changes aim to provide a smoother and more reliable experience for users interacting with Telegram through the system, backed by improved internal logic and comprehensive test coverage. Highlights
Changelog
Using Gemini Code AssistThe 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
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 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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request significantly improves the Telegram channel setup flow by introducing an auto-verification process and making the UI more robust. It also fixes an important message routing issue where owner-bound targets were not being used correctly, and adds comprehensive tests for this. Additionally, it corrects the behavior of event-triggered routines to prevent duplicate agent responses. The changes are well-implemented across both the frontend and backend. I have one minor suggestion for code simplification in the JavaScript part.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refines Telegram channel setup and routing across the Rust backend and the web UI, aiming to make Telegram owner verification automatic and to ensure outbound notifications/messages route to the bound Telegram owner target.
Changes:
- Updates the web configure modal + gateway SSE behavior so Telegram setup stays inline, auto-starts verification, and doesn’t reopen/overwrite an existing modal on
auth_required. - Extends the message tool + agent notification routing to resolve a channel-bound notification target (e.g., Telegram owner chat id) via the
ExtensionManager. - Adds regression/E2E coverage for Telegram routing and event-triggered routine consumption (preventing duplicate “main agent + routine” turns).
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/support/test_rig.rs | Updates test rig to register message tool with optional extension manager support. |
| tests/fixtures/llm_traces/advanced/routine_event_telegram.json | Adjusts trace fixture to match new event-trigger consumption behavior (no extra assistant turn). |
| tests/fixtures/llm_traces/advanced/routine_event_any_channel.json | Same as above for the “any channel” variant. |
| tests/e2e_telegram_message_routing.rs | Adds E2E coverage asserting Telegram broadcasts use the bound owner target when target is omitted. |
| tests/e2e_advanced_traces.rs | Tightens assertions to ensure only the routine LLM call happens and only one notification is emitted. |
| tests/e2e/scenarios/test_telegram_hot_activation.py | Updates Playwright scenario to validate inline Telegram /start CODE + waiting state and auto-verify behavior. |
| tests/e2e/scenarios/test_extensions.py | Adds regression test ensuring auth_required SSE doesn’t clobber an already-open Telegram configure modal. |
| src/tools/registry.rs | Changes register_message_tools to accept an optional ExtensionManager and wires it into MessageTool. |
| src/tools/builtin/message.rs | Updates target/channel resolution to prefer bound channel owner target when appropriate. |
| src/main.rs | Passes extension_manager into message tool registration. |
| src/extensions/manager.rs | Implements Telegram sendMessage helper + exposes notification_target_for_channel; adjusts Telegram verification flow and messaging. |
| src/channels/web/static/style.css | Adds inline error/status styles for the configure modal. |
| src/channels/web/static/i18n/en.js | Updates Telegram copy and adds strings for command label + start-over flow. |
| src/channels/web/static/app.js | Keeps Telegram setup modal open, renders /start CODE inline, auto-verifies, and avoids reopening modal on auth_required. |
| src/channels/web/server.rs | Stops broadcasting auth_required SSE for Telegram verification responses; adds regression test. |
| src/agent/agent_loop.rs | Consumes event-triggered routine messages before main pipeline; resolves notification targets via ExtensionManager. |
| FEATURE_PARITY.md | Updates Telegram parity note to “owner auto-verification”. |
Comments suppressed due to low confidence (1)
src/agent/agent_loop.rs:610
- Routine notification fallback uses
broadcast_all(&user, ...), butusermay now resolve to a channel-specific target vianotification_target_for_channelwhennotify_channelis set. Becausebroadcast_allpasses the sameuser_idto every channel, fallback routing to channels that expect the owner scope may break. Consider using the owner scope (owner_idfrom metadata) for fallback broadcast, or resolving a per-channel target list for the fallback path.
let Some(user) = resolve_routine_notification_target(
extension_manager.as_ref(),
&response.metadata,
)
.await
else {
tracing::warn!(
notify_channel = ?notify_channel,
"Skipping routine notification with no explicit target or owner scope"
);
continue;
};
// Try the configured channel first, fall back to
// broadcasting on all channels.
let targeted_ok = if let Some(ref channel) = notify_channel {
match channels.broadcast(channel, &user, response.clone()).await {
Ok(()) => true,
Err(e) => {
let should_fallback =
should_fallback_routine_notification(&e);
tracing::warn!(
channel = %channel,
user = %user,
error = %e,
should_fallback,
"Failed to send routine notification to configured channel"
);
if !should_fallback {
continue;
}
false
}
}
} else {
false
};
if !targeted_ok {
let results = channels.broadcast_all(&user, response).await;
for (ch, result) in results {
if let Err(e) = result {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes the Telegram setup/verification UX in the web gateway and corrects Telegram notification/message routing by preferring the channel’s bound owner target when appropriate. It also updates event-triggered routine handling to avoid duplicate LLM turns and adds regression coverage across Rust and Playwright tests.
Changes:
- Update the web configure modal to keep Telegram verification inline, auto-start verification, and avoid
auth_requiredSSE re-opening/clobbering the modal. - Improve routing for Telegram notifications/messages by resolving the channel owner binding via
ExtensionManager(used by the message tool and routine/heartbeat notification forwarding). - Add/adjust E2E and trace-based tests to cover Telegram routing and ensure event-triggered routines don’t produce duplicate responses.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/support/test_rig.rs | Update message tool registration to pass extension_manager. |
| tests/fixtures/llm_traces/advanced/routine_event_telegram.json | Adjust fixture to match new event-trigger consumption behavior (fewer responses). |
| tests/fixtures/llm_traces/advanced/routine_event_any_channel.json | Same as above for “any channel” fixture. |
| tests/e2e_telegram_message_routing.rs | New E2E coverage for Telegram message tool target resolution (bound owner vs explicit target). |
| tests/e2e_advanced_traces.rs | Tighten assertions around event-triggered routines (LLM calls + expected responses). |
| tests/e2e/scenarios/test_telegram_hot_activation.py | Playwright regression coverage for Telegram modal staying open + auto-verify request flow. |
| tests/e2e/scenarios/test_extensions.py | Regression test to ensure auth_required SSE doesn’t reopen an existing configure modal. |
| src/tools/registry.rs | Extend register_message_tools to optionally wire ExtensionManager into MessageTool. |
| src/tools/builtin/message.rs | Resolve channel-scoped notification target via owner binding when target omitted. |
| src/main.rs | Pass extension_manager into message tool registration. |
| src/extensions/manager.rs | Add Telegram sendMessage helper; expose notification_target_for_channel; update Telegram verification text + behavior. |
| src/channels/web/static/style.css | Add inline status/error styles for the configure modal. |
| src/channels/web/static/i18n/en.js | Update Telegram setup strings and add new labels (“Send this in Telegram”, “Start over”, etc.). |
| src/channels/web/static/app.js | Prevent modal clobbering by auth_required; keep Telegram verification inline; auto-verify flow + composer unlock changes. |
| src/channels/web/server.rs | Stop broadcasting auth_required SSE on Telegram verification response; add regression test. |
| src/agent/agent_loop.rs | Route notifications using channel-bound targets; consume event-triggered inputs before main chat loop to prevent duplicate turns. |
| FEATURE_PARITY.md | Update Telegram feature note to reflect auto-verification. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
nickpismenkov
previously approved these changes
Mar 17, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes Telegram’s setup/auto-verify UX in the web gateway, prevents auth_required SSE events from reopening/clobbering an existing configure modal, and corrects Telegram notification routing so outbound messages use the bound owner target when switching channels.
Changes:
- Update the web configure modal to render the
/start CODEcommand inline, keep the modal open during Telegram verification, and auto-start owner verification without a second click. - Fix notification/message routing by resolving per-channel notification targets (e.g., Telegram chat_id) via
ExtensionManagerwhen the target is omitted. - Add/adjust regression tests covering Telegram routing, SSE event behavior, and event-trigger routine traces.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/support/test_rig.rs | Updates message-tool registration to pass the extension manager so tests match runtime routing behavior. |
| tests/fixtures/llm_traces/advanced/routine_event_telegram.json | Updates fixture trace to reflect new “consume event-trigger message” behavior (avoids duplicate turns). |
| tests/fixtures/llm_traces/advanced/routine_event_any_channel.json | Same as above for any-channel event-trigger trace fixture. |
| tests/e2e_telegram_message_routing.rs | Adds E2E regression coverage ensuring Telegram outbound routing prefers bound owner target when target is omitted. |
| tests/e2e_advanced_traces.rs | Tightens assertions to ensure only the routine LLM call/notification occurs for matching event triggers. |
| tests/e2e/scenarios/test_telegram_hot_activation.py | Updates Playwright scenario for the new Telegram auto-verify setup flow and waiting state UX. |
| tests/e2e/scenarios/test_extensions.py | Adds regression test to ensure auth_required SSE does not reopen an already-open configure modal. |
| src/tools/registry.rs | Extends register_message_tools to optionally wire in ExtensionManager for target resolution. |
| src/tools/builtin/message.rs | Adds per-channel target resolution using ExtensionManager and fixes default/metadata target application rules. |
| src/main.rs | Passes the extension manager into message-tool registration in the main runtime. |
| src/extensions/manager.rs | Implements Telegram auto-verification messaging, improves timeout handling, and exposes channel notification target resolution. |
| src/channels/web/static/style.css | Adds inline status/error styling for the configure modal verification flow. |
| src/channels/web/static/i18n/en.js | Updates/extends Telegram configure strings for the new auto-verify UI. |
| src/channels/web/static/i18n/zh-CN.js | Adds matching Telegram configure strings for parity with English. |
| src/channels/web/static/app.js | Prevents auth_required from clobbering an open modal; implements inline Telegram verification command/status + auto-verify UX. |
| src/channels/web/server.rs | Stops broadcasting auth_required SSE for Telegram verification responses; adds a regression test. |
| src/agent/agent_loop.rs | Resolves routine/heartbeat notification targets with channel bindings and consumes matching event-trigger messages pre-pipeline. |
| FEATURE_PARITY.md | Updates Telegram note to reflect auto-verification behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+3047
to
+3058
| let deadline = tokio::time::Instant::now() + Duration::from_millis(100); | ||
| loop { | ||
| let remaining = deadline.saturating_duration_since(tokio::time::Instant::now()); | ||
| if remaining.is_zero() { | ||
| break; | ||
| } | ||
| match timeout(remaining, receiver.recv()).await { | ||
| Ok(Ok(crate::channels::web::types::SseEvent::AuthRequired { .. })) => { | ||
| panic!("verification responses should not emit auth_required SSE events") | ||
| } | ||
| Ok(Ok(_)) => continue, | ||
| Ok(Err(_)) | Err(_) => break, |
nickpismenkov
approved these changes
Mar 17, 2026
This was referenced Mar 17, 2026
Merged
bkutasi
pushed a commit
to bkutasi/ironclaw
that referenced
this pull request
Mar 28, 2026
* Fix Telegram auto-verify flow and routing * Fix CI formatting and clippy follow-ups * Simplify Telegram waiting state update * Fix notification fallback scopes * Fix message metadata routing and zh-CN copy
drchirag1991
pushed a commit
to drchirag1991/ironclaw
that referenced
this pull request
Apr 8, 2026
* Fix Telegram auto-verify flow and routing * Fix CI formatting and clippy follow-ups * Simplify Telegram waiting state update * Fix notification fallback scopes * Fix message metadata routing and zh-CN copy
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
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
/start CODEcommand inline, and auto-start owner verification without a second clickauth_requiredSSE events from clobbering the modal, clear the chat composer lock, and remove the non-actionable waiting buttonTesting
Manual testing