fix: Telegram bot token validation fails intermittently (HTTP 404) by nickpismenkov · Pull Request #1166 · nearai/ironclaw

nickpismenkov · 2026-03-14T07:50:54Z

Summary

Change Type

Linked Issue

Validation

cargo fmt
cargo clippy --all --benches --tests --examples --all-features
Relevant tests pass:
Manual testing:

Security Impact

Database Impact

Blast Radius

Rollback Plan

Review track:

gemini-code-assist · 2026-03-14T07:51:09Z

Summary of Changes

Hello, I'm Gemini Code Assist¹! 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 and fixes an intermittent HTTP 404 error encountered during Telegram bot token validation. The issue stemmed from the system incorrectly URL-encoding the colon character present in Telegram tokens, which the Telegram API does not expect. The solution involves adjusting the token substitution mechanism to ensure that these tokens are passed in their original, unencoded format. This change is thoroughly validated with new unit tests specifically targeting token format preservation and comprehensive end-to-end tests that verify the user interface's behavior and error handling during the token configuration process.

Highlights

Bug Fix: Telegram Token Validation: Resolved an intermittent HTTP 404 error that occurred during Telegram bot token validation due to incorrect URL encoding of the colon character within the token.
URL Encoding Removal: Modified the secret substitution logic in src/extensions/manager.rs and src/setup/channels.rs to prevent URL encoding of Telegram bot tokens, ensuring the colon separator is preserved.
New Unit Test: Added a dedicated unit test (test_telegram_token_colon_preserved_in_validation_url) to explicitly verify that the colon in Telegram tokens is not URL-encoded during validation URL construction.
Comprehensive E2E Tests: Introduced a new end-to-end test suite (test_telegram_token_validation.py) to cover the UI flow, error handling, and acceptance of various Telegram token formats during configuration.

Changelog

src/extensions/manager.rs
- Removed URL encoding for secret values when constructing validation URLs.
- Added a new unit test to verify Telegram token colon preservation.
src/setup/channels.rs
- Removed URL encoding for secret values during placeholder substitution in validation URLs.
- Modified an existing test to specifically check Telegram token handling.
tests/e2e/scenarios/test_telegram_token_validation.py
- Added a new file containing comprehensive end-to-end tests for Telegram bot token validation, covering UI rendering, input acceptance, error handling, and token format preservation.

Activity

No specific activity (comments, reviews, progress updates) was provided in the pull request description.

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.

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.

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. ↩

gemini-code-assist

Code Review

This pull request addresses a Telegram bot token validation issue by removing URL encoding during secret substitution. However, this fix is applied too broadly, introducing a high-severity regression for other extensions that rely on URL encoding for secrets containing special characters in query parameters. The comments highlight this issue, suggesting more targeted fixes in line with the principle of precisely scoping changes to avoid unintended side effects and regressions.

src/extensions/manager.rs

src/setup/channels.rs

zmanian

Review: Fix Telegram bot token colon encoding in validation URL

The core fix is correct -- Telegram bot tokens contain colons in the URL path, and URL-encoding them to %3A breaks the Telegram API endpoint. Good unit test coverage for the URL building logic.

Blocking: `pycache` files committed

Binary .pyc files are included in the diff:

tests/e2e/__pycache__/conftest.cpython-313-pytest-8.4.0.pyc
tests/e2e/__pycache__/helpers.cpython-313.pyc
tests/e2e/scenarios/__pycache__/__init__.cpython-313.pyc
tests/e2e/scenarios/__pycache__/test_telegram_token_validation.cpython-313-pytest-8.4.0.pyc

These should not be committed. Remove them and add __pycache__/ to .gitignore if not already present.

Non-blocking

The name == "telegram" hardcode is pragmatic but not extensible. If other extensions need raw tokens in URL paths, consider a capability flag like "url_encode_secrets": false. Fine for now.
E2E tests verify the configure modal UI accepts tokens with colons, which is useful but the real validation happens server-side (covered by the unit test).

Resolved merge conflict in e2e.yml by including test_telegram_token_validation.py from fix/bot-token-validation along with oauth credential tests from staging.

zmanian

Review: APPROVE

Previous blocking feedback addressed -- __pycache__ files removed, .gitignore updated, URL encoding bypass properly scoped to Telegram only (no longer affects other extensions).

Root cause

Telegram bot tokens contain : (e.g., 123456789:AABBccDDeeFFgg). The code applied form_urlencoded::byte_serialize, encoding : to %3A. Since tokens go in the URL path (/bot{token}/getMe), not query params, Telegram returns 404.

Fix assessment

Correct and properly scoped -- only the telegram extension skips URL encoding, all others retain it. The conditional is clean and well-commented.

Minor suggestions (non-blocking)

Unit test doesn't exercise production code -- it manually replicates the fix logic rather than calling through the actual URL-building path. If someone modifies the conditional later, this test would still pass. Consider extracting the URL-building logic into a testable helper.
E2E test duplication -- the three test functions repeat ~50 lines of identical route-mocking boilerplate. A @pytest.fixture would help.
Commit history -- consider squashing the "fix", "fix", "fix" commits before merge for a clean history.

CI all green. Previous review items addressed.

…03-16 05:35 UTC) (#1236) * refactor(setup): extract init logic from wizard into owning modules (#1210) * refactor(setup): extract init logic from wizard into owning modules Move database, LLM model discovery, and secrets initialization logic out of the setup wizard and into their owning modules, following the CLAUDE.md principle that module-specific initialization must live in the owning module as a public factory function. Database (src/db/mod.rs, src/config/database.rs): - Add DatabaseConfig::from_postgres_url() and from_libsql_path() - Add connect_without_migrations() for connectivity testing - Add validate_postgres() returning structured PgDiagnostic results LLM (src/llm/models.rs — new file): - Extract 8 model-fetching functions from wizard.rs (~380 lines) - fetch_anthropic_models, fetch_openai_models, fetch_ollama_models, fetch_openai_compatible_models, build_nearai_model_fetch_config, and OpenAI sorting/filtering helpers Secrets (src/secrets/mod.rs): - Add resolve_master_key() unifying env var + keychain resolution - Add crypto_from_hex() convenience wrapper Wizard restructuring (src/setup/wizard.rs): - Replace cfg-gated db_pool/db_backend fields with generic db: Option<Arc<dyn Database>> + db_handles: Option<DatabaseHandles> - Delete 6 backend-specific methods (reconnect_postgres/libsql, test_database_connection_postgres/libsql, run_migrations_postgres/ libsql, create_postgres/libsql_secrets_store) - Simplify persist_settings, try_load_existing_settings, persist_session_to_db, init_secrets_context to backend-agnostic implementations using the new module factories - Eliminate all references to deadpool_postgres, PoolConfig, LibSqlBackend, Store::from_pool, refinery::embed_migrations Net: -878 lines from wizard, +395 lines in owning modules, +378 new. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test(settings): add wizard re-run regression tests Add 10 tests covering settings preservation during wizard re-runs: - provider_only rerun preserves channels/embeddings/heartbeat - channels_only rerun preserves provider/model/embeddings - quick mode rerun preserves prior channels and heartbeat - full rerun same provider preserves model through merge - full rerun different provider clears model through merge - incremental persist doesn't clobber prior steps - switching DB backend allows fresh connection settings - merge preserves true booleans when overlay has default false - embeddings survive rerun that skips step 5 These cover the scenarios where re-running the wizard would previously risk resetting models, providers, or channel settings. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor(setup): eliminate cfg(feature) gates from wizard methods Replace compile-time #[cfg(feature)] dispatch in the wizard with runtime dispatch via DatabaseBackend enum and cfg!() macro constants. - Merge step_database_postgres + step_database_libsql into step_database using runtime backend selection - Rewrite auto_setup_database without feature gates - Remove cfg(feature = "postgres") from mask_password_in_url (pure fn) - Remove cfg(feature = "postgres") from test_mask_password_in_url Only one internal #[cfg(feature = "postgres")] remains: guarding the call to db::validate_postgres() which is itself feature-gated. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor(db): fold PG validation into connect_without_migrations Move PostgreSQL prerequisite validation (version >= 15, pgvector) from the wizard into connect_without_migrations() in the db module. The validation now returns DatabaseError directly with user-facing messages, eliminating the PgDiagnostic enum and the last #[cfg(feature)] gate from the wizard. The wizard's test_database_connection() is now a 5-line method that calls the db module factory and stores the result. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address PR review comments [skip-regression-check] - Use .as_ref().map() to avoid partial move of db_config.libsql_path (gemini-code-assist) - Default to available backend when DATABASE_BACKEND is invalid, not unconditionally to Postgres which may not be compiled (Copilot) - Match DatabaseBackend::Postgres explicitly instead of _ => wildcard in connect_with_handles, connect_without_migrations, and create_secrets_store to avoid silently routing LibSql configs through the Postgres path when libsql feature is disabled (Copilot) - Upgrade Ollama connection failure log from info to warn with the base URL for better visibility in wizard UX (Copilot) - Clarify crypto_from_hex doc: SecretsCrypto validates key length, not hex encoding (Copilot) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address zmanian's PR review feedback [skip-regression-check] - Update src/setup/README.md to reflect Arc<dyn Database> flow - Remove stale "Test PostgreSQL connection" doc comment - Replace unwrap_or(0) in validate_postgres with descriptive error - Add NearAiConfig::for_model_discovery() constructor - Narrow pub to pub(crate) for internal model helpers Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address Copilot review comments (quick-mode postgres gate, empty env vars) [skip-regression-check] - Gate DATABASE_URL auto-detection on POSTGRES_AVAILABLE in quick mode so libsql-only builds don't attempt a postgres connection - Match empty-env-var filtering in key source detection to align with resolve_master_key() behavior - Filter empty strings to None in DatabaseConfig::from_libsql_path() for turso_url/turso_token Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: Telegram bot token validation fails intermittently (HTTP 404) (#1166) * fix: Telegram bot token validation fails intermittently (HTTP 404) * fix: code style * fix * fix * fix * review fix --------- Co-authored-by: Illia Polosukhin <ilblackdragon@gmail.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Nick Pismenkov <50764773+nickpismenkov@users.noreply.github.com>

…earai#1166) * fix: Telegram bot token validation fails intermittently (HTTP 404) * fix: code style * fix * fix * fix * review fix

…03-16 05:35 UTC) (nearai#1236) * refactor(setup): extract init logic from wizard into owning modules (nearai#1210) * refactor(setup): extract init logic from wizard into owning modules Move database, LLM model discovery, and secrets initialization logic out of the setup wizard and into their owning modules, following the CLAUDE.md principle that module-specific initialization must live in the owning module as a public factory function. Database (src/db/mod.rs, src/config/database.rs): - Add DatabaseConfig::from_postgres_url() and from_libsql_path() - Add connect_without_migrations() for connectivity testing - Add validate_postgres() returning structured PgDiagnostic results LLM (src/llm/models.rs — new file): - Extract 8 model-fetching functions from wizard.rs (~380 lines) - fetch_anthropic_models, fetch_openai_models, fetch_ollama_models, fetch_openai_compatible_models, build_nearai_model_fetch_config, and OpenAI sorting/filtering helpers Secrets (src/secrets/mod.rs): - Add resolve_master_key() unifying env var + keychain resolution - Add crypto_from_hex() convenience wrapper Wizard restructuring (src/setup/wizard.rs): - Replace cfg-gated db_pool/db_backend fields with generic db: Option<Arc<dyn Database>> + db_handles: Option<DatabaseHandles> - Delete 6 backend-specific methods (reconnect_postgres/libsql, test_database_connection_postgres/libsql, run_migrations_postgres/ libsql, create_postgres/libsql_secrets_store) - Simplify persist_settings, try_load_existing_settings, persist_session_to_db, init_secrets_context to backend-agnostic implementations using the new module factories - Eliminate all references to deadpool_postgres, PoolConfig, LibSqlBackend, Store::from_pool, refinery::embed_migrations Net: -878 lines from wizard, +395 lines in owning modules, +378 new. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test(settings): add wizard re-run regression tests Add 10 tests covering settings preservation during wizard re-runs: - provider_only rerun preserves channels/embeddings/heartbeat - channels_only rerun preserves provider/model/embeddings - quick mode rerun preserves prior channels and heartbeat - full rerun same provider preserves model through merge - full rerun different provider clears model through merge - incremental persist doesn't clobber prior steps - switching DB backend allows fresh connection settings - merge preserves true booleans when overlay has default false - embeddings survive rerun that skips step 5 These cover the scenarios where re-running the wizard would previously risk resetting models, providers, or channel settings. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor(setup): eliminate cfg(feature) gates from wizard methods Replace compile-time #[cfg(feature)] dispatch in the wizard with runtime dispatch via DatabaseBackend enum and cfg!() macro constants. - Merge step_database_postgres + step_database_libsql into step_database using runtime backend selection - Rewrite auto_setup_database without feature gates - Remove cfg(feature = "postgres") from mask_password_in_url (pure fn) - Remove cfg(feature = "postgres") from test_mask_password_in_url Only one internal #[cfg(feature = "postgres")] remains: guarding the call to db::validate_postgres() which is itself feature-gated. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor(db): fold PG validation into connect_without_migrations Move PostgreSQL prerequisite validation (version >= 15, pgvector) from the wizard into connect_without_migrations() in the db module. The validation now returns DatabaseError directly with user-facing messages, eliminating the PgDiagnostic enum and the last #[cfg(feature)] gate from the wizard. The wizard's test_database_connection() is now a 5-line method that calls the db module factory and stores the result. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address PR review comments [skip-regression-check] - Use .as_ref().map() to avoid partial move of db_config.libsql_path (gemini-code-assist) - Default to available backend when DATABASE_BACKEND is invalid, not unconditionally to Postgres which may not be compiled (Copilot) - Match DatabaseBackend::Postgres explicitly instead of _ => wildcard in connect_with_handles, connect_without_migrations, and create_secrets_store to avoid silently routing LibSql configs through the Postgres path when libsql feature is disabled (Copilot) - Upgrade Ollama connection failure log from info to warn with the base URL for better visibility in wizard UX (Copilot) - Clarify crypto_from_hex doc: SecretsCrypto validates key length, not hex encoding (Copilot) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address zmanian's PR review feedback [skip-regression-check] - Update src/setup/README.md to reflect Arc<dyn Database> flow - Remove stale "Test PostgreSQL connection" doc comment - Replace unwrap_or(0) in validate_postgres with descriptive error - Add NearAiConfig::for_model_discovery() constructor - Narrow pub to pub(crate) for internal model helpers Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address Copilot review comments (quick-mode postgres gate, empty env vars) [skip-regression-check] - Gate DATABASE_URL auto-detection on POSTGRES_AVAILABLE in quick mode so libsql-only builds don't attempt a postgres connection - Match empty-env-var filtering in key source detection to align with resolve_master_key() behavior - Filter empty strings to None in DatabaseConfig::from_libsql_path() for turso_url/turso_token Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: Telegram bot token validation fails intermittently (HTTP 404) (nearai#1166) * fix: Telegram bot token validation fails intermittently (HTTP 404) * fix: code style * fix * fix * fix * review fix --------- Co-authored-by: Illia Polosukhin <ilblackdragon@gmail.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Nick Pismenkov <50764773+nickpismenkov@users.noreply.github.com>

fix: Telegram bot token validation fails intermittently (HTTP 404)

dd4fc58

github-actions bot added scope: extensions Extension management scope: setup Onboarding / setup labels Mar 14, 2026

nickpismenkov linked an issue Mar 14, 2026 that may be closed by this pull request

Telegram bot token validation fails intermittently (HTTP 404) #991

Closed

github-actions bot added size: L 200-499 changed lines risk: high Safety, secrets, auth, or critical infrastructure labels Mar 14, 2026

github-actions bot added the contributor: experienced 6-19 merged PRs label Mar 14, 2026

gemini-code-assist bot reviewed Mar 14, 2026

View reviewed changes

src/extensions/manager.rs Outdated Show resolved Hide resolved

src/setup/channels.rs Outdated Show resolved Hide resolved

fix: code style

e4466cf

github-actions bot added risk: medium Business logic, config, or moderate-risk modules and removed risk: high Safety, secrets, auth, or critical infrastructure labels Mar 14, 2026

fix

577d02e

github-actions bot added the scope: ci CI/CD workflows label Mar 14, 2026

nickpismenkov added 2 commits March 14, 2026 01:20

fix

be94327

fix

87d6a39

zmanian requested changes Mar 14, 2026

View reviewed changes

zmanian mentioned this pull request Mar 14, 2026

chore: add __pycache__ and *.pyc to .gitignore #1175

Closed

1 task

nickpismenkov added 2 commits March 15, 2026 21:12

Merge branch 'staging' into fix/bot-token-validation

9224235

Resolved merge conflict in e2e.yml by including test_telegram_token_validation.py from fix/bot-token-validation along with oauth credential tests from staging.

review fix

dafde5c

nickpismenkov requested a review from zmanian March 16, 2026 04:13

github-actions bot added contributor: core 20+ merged PRs and removed contributor: experienced 6-19 merged PRs labels Mar 16, 2026

zmanian approved these changes Mar 16, 2026

View reviewed changes

nickpismenkov merged commit 81724ca into staging Mar 16, 2026
19 checks passed

nickpismenkov deleted the fix/bot-token-validation branch March 16, 2026 05:06

ironclaw-ci bot mentioned this pull request Mar 16, 2026

chore: promote staging to staging-promote/57c397bd-23120362128 (2026-03-16 05:35 UTC) #1236

Merged

This was referenced Mar 16, 2026

chore: promote staging to staging-promote/57c397bd-23120362128 (2026-03-16 08:20 UTC) #1239

Merged

chore: promote staging to staging-promote/3f874e73-23119318963 (2026-03-15 22:04 UTC) #1231

Merged

This was referenced Mar 16, 2026

chore: promote staging to staging-promote/e0f393bf-23105705354 (2026-03-15 21:06 UTC) #1212

Merged

chore: promote staging to staging-promote/e74214dc-23104855330 (2026-03-15 07:18 UTC) #1197

Merged

ironclaw-ci bot mentioned this pull request Mar 17, 2026

chore: release v0.19.0 #973

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Telegram bot token validation fails intermittently (HTTP 404)#1166

fix: Telegram bot token validation fails intermittently (HTTP 404)#1166
nickpismenkov merged 7 commits intostagingfrom
fix/bot-token-validation

nickpismenkov commented Mar 14, 2026

Uh oh!

gemini-code-assist bot commented Mar 14, 2026

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

Uh oh!

Uh oh!

zmanian left a comment

Uh oh!

zmanian left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

nickpismenkov commented Mar 14, 2026

Summary

Change Type

Linked Issue

Validation

Security Impact

Database Impact

Blast Radius

Rollback Plan

Uh oh!

gemini-code-assist bot commented Mar 14, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

zmanian left a comment

Choose a reason for hiding this comment

Review: Fix Telegram bot token colon encoding in validation URL

Blocking: __pycache__ files committed

Non-blocking

Uh oh!

zmanian left a comment

Choose a reason for hiding this comment

Review: APPROVE

Root cause

Fix assessment

Minor suggestions (non-blocking)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Blocking: `pycache` files committed