feat(messaging): add Telegram adapter scaffold (Phase 0 of #889)#951
feat(messaging): add Telegram adapter scaffold (Phase 0 of #889)#951theonlychant wants to merge 10 commits intoamd:mainfrom
Conversation
Signed-off-by: theonlychant <sacehenry@gmail.com>
Signed-off-by: theonlychant <sacehenry@gmail.com>
Signed-off-by: theonlychant <sacehenry@gmail.com>
There was a problem hiding this comment.
@theonlychant — thanks for picking up #889 and getting a Phase 0 scaffold up. A bit of guidance on the shape before code-level review, because the PR as-shaped is hard to land:
The PR description and the in-PR doc disagree about what was built
The body says "Implements a Telegram messaging adapter for GAIA (Phase 0)" and the test plan checks off four passing test files. But docs/guides/telegram.mdx (added in the same PR) says:
Status: scaffolded — implementation work in progress.
Next steps:
- Implement runtime using
python-telegram-botApplication and handlers.- Wire uploads to VLM/RAG ingestion.
- Add tests for allowed-users gate and restricted tools enforcement.
Those "next steps" overlap directly with what the description and #889's acceptance criteria say is done. Pick one story: either the body softens to "scaffold for #889, runtime in follow-up," or the doc and code catch up to the description.
The test plan also says gaia chat --telegram, but the CLI you actually added is gaia telegram start --token X. Description should match what shipped.
Title doesn't fit the change
docs(spec): add Telegram adapter notes reads as a docs-only PR. This is a 600-line feat(messaging): change with a new dependency, a new CLI surface, and a new package. Per CLAUDE.md the title should be conventional-commits and describe the actual change — something like feat(messaging): add Telegram adapter scaffold (Phase 0 of #889). Reviewers (and downstream changelog tooling) read the title first.
Drop the unrelated electron-builder URL changes
Three files have nothing to do with Telegram and look like a separate cleanup pass:
docs/deployment/code-signing.mdx— electron-builder URL swapdocs/plans/desktop-installer.mdx— sameinstaller/nsis/installer.nsh— same
These should be a separate one-line PR. Bundling them here makes the diff harder to review and risks the messaging work blocking on unrelated discussion (and vice versa).
Repo convention violations that block as written
CLAUDE.md has two explicit rules this PR breaks:
-
"No Silent Fallbacks — Fail Loudly." The Telegram code is full of bare
except Exception: pass/log.exception(...)then continue:telegram.pyPID-file write — swallowedtelegram.pylog-file open — swallowedtelegram.pyreply.edit_textin the streaming loop — swallowedtelegram.pysignal.signal(...)registration — swallowedtelegram.pyrun_generationthread —Exceptioncaught and converted to a string posted into chattelegram.pyimport oftelegram.ext— whenbackground=True, returns silently withapplication = None, which is exactly the "silent degradation path" the rule prohibitscli.pytelegramstatus— when the health-check URL fails, falls through to PID-file existence check; "PID exists, may be unhealthy" is the silent-degradation pattern the rule namescli.pyallowed_usersparse —except Exceptioninstead ofexcept ValueError
Per CLAUDE.md these need specific exception types, re-raise with context, or — at system boundaries (the message handler) — translate to a structured user-visible error. "Tech debt, not precedent" is the line in the rule.
-
"Every new feature must be documented … Update
docs/docs.jsonto add new pages to the appropriate navigation section."docs/guides/telegram.mdxis added butdocs/docs.jsonis not touched — the page won't appear in the rendered site nav.
Concrete next steps
- Split out the electron-builder URL changes into their own PR. Keep this one focused on Telegram.
- Retitle with
feat(messaging): ...form. - Reconcile description vs. doc vs. CLI. Either lower the body to "scaffolding + plumbing" matching the doc, or finish the runtime so the body's claims hold. The acceptance criteria in #889 are concrete (
/startreply within 5s, image→VLM, file→RAG, restricted tools) — neither the doc nor the tests cover those today, so the honest framing is "scaffolding." - Update
docs/docs.jsonto register the new guide. - Replace the silent-fallback sites with specific exception types, re-raise with context, or boundary translations. The PID-write, dependency-missing, and message-handler error paths are the load-bearing ones.
- Either use
_USER_SESSIONSin_handle_messageor delete it. Right nowget_or_create_sessionis defined and tested, but the actual handler creates a freshAgentSDK(config)per message — so per-user session isolation (an explicit acceptance criterion of #889) is not implemented despite tests asserting the helper exists.
Two technical notes for the follow-up runtime PR (not blocking this one)
tmp_path = os.path.join("/tmp", ...)in the photo/document handlers won't work on Windows, which is GAIA's primary platform. Usetempfile.mkstemp/tempfile.gettempdir().- The CLI offers
--health-hostand--health-port, but the in-process health server is hardcoded to127.0.0.1:8765. Either thread the values through, or drop the CLI args.
For exemplar PR shape, see #935 (concise summary, clear test plan, single concern) and #931 (docs-only with a tight scope).
Happy to re-review once this is split, retitled, and the description matches what's actually implemented. Thanks for sticking with it — the messaging work in #889 / #635 is high-value, just needs to land in reviewable chunks.
Signed-off-by: theonlychant <sacehenry@gmail.com>
Alright I will be changing those now |
…ndle leak Signed-off-by: theonlychant <sacehenry@gmail.com>
Signed-off-by: theonlychant <sacehenry@gmail.com>
dislovelhl
left a comment
There was a problem hiding this comment.
Good progress overall — the nine fixes from the prior round have all landed cleanly (scoped exceptions on PID/log/import/signal paths, tempfile.gettempdir(), session wiring, PR title + description, and docs.json registration). Three items still need attention before this can merge.
Blocking
1. Merge conflict in docs/runbooks/google-oauth-client.md
The file was flagged in the prior review as an unrelated change that should be split out. It is still in the PR, and it now also contains raw conflict markers:
<<<<<<< docs/telegram-adapter
---
title: Google OAuth Client Setup
...
=======
# Google OAuth Client — Runbook
...
>>>>>>> main
This branch cannot be cleanly merged as-is. Please either revert this file entirely (the right call — it has nothing to do with the Telegram adapter) or resolve the conflict and split it into its own PR.
2. docs/guides/telegram.mdx is orphaned and contradicts reality
Two Telegram guide files were added:
| File | In docs.json? |
Accurate? |
|---|---|---|
docs/guides/telegram-adapter.mdx |
✅ yes | ✅ yes |
docs/guides/telegram.mdx |
❌ no (orphaned) | ❌ no |
telegram.mdx says "Status: scaffolded — implementation work in progress" and lists "Implement runtime using python-telegram-bot Application and handlers" as a next step — but that runtime is implemented in this very PR. The prior review asked to reconcile the description, doc, and CLI; this file wasn't updated to match.
Simplest fix: delete docs/guides/telegram.mdx. The registered telegram-adapter.mdx already covers the feature correctly.
3. except Exception still present in cli.py — allowed-users parse
# src/gaia/cli.py
try:
allowed = set(int(x.strip()) for x in args.allowed_users.split(",") if x.strip())
except Exception: # ← still broad
print("Invalid --allowed-users format; expected comma-separated integers", ...)
sys.exit(2)The only exception this generator can raise is ValueError (from int()). Per CLAUDE.md's "No Silent Fallbacks" rule, please narrow this to except ValueError:.
Non-blocking (worth a follow-up)
The stop-action final fallback (except Exception as e around the SIGTERM path) and the streaming edit fallback (except Exception: in _handle_message) still use broad catches. Not a merge blocker here, but worth a quick pass — OSError for the stop path, and the telegram-specific exception types (TimedOut, NetworkError, etc.) that are already imported for the edit path cover the real failure modes there.
Signed-off-by: theonlychant <sacehenry@gmail.com>
Signed-off-by: theonlychant <sacehenry@gmail.com>
Signed-off-by: theonlychant <sacehenry@gmail.com>
alright check the changes I made just now |
Summary
Adds a Phase 0 scaffold for the Telegram messaging adapter — bot wiring,
CLI subcommand, session helpers, and a setup guide. Runtime implementation
(message handling, VLM/RAG ingestion, allowed-users gate) follows in a
Phase 1 PR.
Why
GAIA has no way to reach users on mobile messaging platforms. Telegram
offers a clean OSS bot path with no TOS risk and a free API. This scaffold
establishes the adapter pattern and plumbing so Phase 1 can focus purely
on runtime behaviour. See #889 for full acceptance criteria.
Linked issue
Refs #889
Changes
src/gaia/messaging/telegram.py- bot scaffold and session helperssrc/gaia/messaging/ingest.py- shared message ingestion plumbinggaia telegram start|stop|statusCLI subcommand insrc/gaia/cli.pytelegramextras insetup.py(python-telegram-bot>=20.3)docs/guides/telegram.mdx- setup guide (status: scaffolded)docs/docs.jsonentry to register the guide in site navexcept Exceptionfallbacks with specific exceptiontypes and re-raise with context throughout
telegram.pyandcli.py_USER_SESSIONS- wired into_handle_messagefor per-usersession isolation
Test plan
pytest tests/unit/test_telegram_adapter.py- passingpytest tests/unit/test_telegram_background.py- passingpytest tests/unit/test_telegram_sessions.py- passingpytest tests/unit/test_ingest_helpers.py- passingpython util/lint.py --all- no failuresgaia telegram start --token Xstarts bot,/startreplieswithin 5s (Phase 1 acceptance criterion - not yet implemented)
Checklist
Refs #889).docs/guides/telegram.mdxand registered indocs/docs.json.