fix(e2e): unconditionally skip Telegram reachability probe for fake-token tests#4555
Conversation
…oken tests Commit b13e4f4 introduced `checkTelegramReachability()` which sends an HTTP request to api.telegram.org to validate the bot token. On HTTP 401/404 it marks the channel as "skipped", silently dropping Telegram from onboard. The three E2E scripts that use fake Telegram tokens (test-messaging-providers, test-channels-stop-start, test-token-rotation) only set NEMOCLAW_SKIP_TELEGRAM_REACHABILITY when api.telegram.org is *unreachable* from the host. On GitHub Actions runners the host IS reachable, so the env var is never set, the probe fires with the fake token, gets 401, and onboard drops Telegram — breaking every assertion that expects Telegram to be configured. Fix: export NEMOCLAW_SKIP_TELEGRAM_REACHABILITY=1 unconditionally in all three scripts so the probe is always bypassed when running with fake tokens. Signed-off-by: Hung Le <hple@nvidia.com>
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThree E2E test scripts add a fake-token detector and set NEMOCLAW_SKIP_TELEGRAM_REACHABILITY=1 when tokens match the fake pattern, removing prior curl-based api.telegram.org reachability checks and restricting the skip behavior to fake-token test runs. ChangesE2E Telegram fake-token gating
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
…ram-probe-fake-token-a5e7e63 # Conflicts: # test/e2e/test-messaging-providers.sh
E2E Advisor RecommendationRequired E2E: None Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
E2E Scenario Advisor RecommendationRequired scenario E2E: None Full scenario advisor summaryE2E Scenario AdvisorBase: Required scenario E2E
Optional scenario E2E
Relevant changed files
|
PR Review AdvisorFindings: 0 needs attention, 2 worth checking, 0 nice ideas Review findings🛠️ Needs attention
🔎 Worth checking
🌱 Nice ideas
Since last review detailsCurrent findings:
This is an automated advisory review. A human maintainer must make the final merge decision. |
Selective E2E Results — ✅ All requested jobs passedRun: 26675728346
|
Keep live Telegram runs on the onboard validation path while preserving the fake-token E2E workaround for messaging provider, channel lifecycle, and token rotation tests. Signed-off-by: Carlos Villela <cvillela@nvidia.com>
|
Addressed the advisor findings in b88c011: the Telegram reachability skip now applies only in fake-token mode, preserves live-token onboard validation, and documents the source boundary/removal condition in the affected E2E scripts. Verified with shfmt, shellcheck, and bash -n for the three changed scripts. |
Summary
Fixes #4556
Three nightly E2E jobs (
messaging-providers-e2e,channels-stop-start-e2e,token-rotation-e2e) failed in run 26668952264because commit b13e4f4 introduced
checkTelegramReachability()which validatesthe Telegram bot token via HTTP before onboarding.
The E2E scripts use fake tokens. On GitHub Actions runners
api.telegram.orgis reachable, so the existing conditional guard(
if ! curl … api.telegram.org) never fires. The new probe sends the faketoken, receives HTTP 401, and silently drops Telegram from onboard —
breaking every assertion that expects Telegram to be configured.
Root cause
src/lib/onboard/telegram-reachability.ts(line 65-69){skipped: true}removes Telegram fromfoundchannelssrc/lib/onboard.ts(setupMessagingChannels)foundwhenreachability.skippedis truetest/e2e/test-messaging-providers.sh(line 567-572)test/e2e/test-channels-stop-start.sh(line 422-427)test/e2e/test-token-rotation.shFix
Export
NEMOCLAW_SKIP_TELEGRAM_REACHABILITY=1unconditionally in all threetest scripts so the probe is always bypassed when running with fake tokens.
This is the correct layer: the workflow YAML already passes fake tokens as
env vars, and the scripts know they are running with fake credentials.
Changes
test/e2e/test-messaging-providers.sh— replace conditionalcurl-reachability check with unconditional export
test/e2e/test-channels-stop-start.sh— sametest/e2e/test-token-rotation.sh— add unconditional export beforefirst token assignment
Validation
Test plan
nightly-e2e.yamlon this branch with the three affected jobsmessaging-providers-e2epasses (Telegram channel configured)channels-stop-start-e2epasses (Telegram stop/start works)token-rotation-e2epasses (Telegram token rotation detected)Signed-off-by: Hung Le hple@nvidia.com
Summary by CodeRabbit