[T0ha]

Summary

• Extract shared Telegram bot logic into Bodhi.TgUpdateHandler, replacing Bodhi.TgWebhookHandler
• Add Bodhi.TgPollingHandler (thin wrapper for dev/test) and Bodhi.TgHookHandler (thin wrapper for production webhook mode)
• Dynamic handler selection in Bodhi.Application based on :tg_mode config (:polling, :webhook, or :disabled)
• Production webhook configured via TG_WEBHOOK_URL, TG_WEBHOOK_SECRET, TG_WEBHOOK_PORT env vars in runtime.exs

Test plan

• mix compile — no warnings
• mix test — 250 tests, 0 failures
• mix format — clean
• mix dialyzer — 0 errors
• Verify polling starts in dev with iex -S mix
• Verify webhook mode activates in production with correct env vars

🤖 Generated with Claude Code

[claude]

Code Review

Overview

Clean refactor that separates transport concerns (webhook vs. polling) from shared bot logic. The three-way :tg_mode split (:webhook / :polling / :disabled) is well-designed and the Enum.reject(children, &is_nil/1) trick for conditional supervisor children is idiomatic.

---

Issues

Bug / Crash Risk — TgHookHandler.on_boot/0

{:ok, true} = Telegex.delete_webhook()
{:ok, true} = Telegex.set_webhook(env_config[:webhook_url], ...)

Bare pattern-matching with = will raise MatchError if either Telegram API call returns an error (network issue, bad token, rate-limit). That crashes the supervisor child on boot and, depending on restart strategy, could take down the whole app.

Suggestion: Use with and log or re-raise with a descriptive message:

def on_boot do
  env_config = Application.get_env(:bodhi, __MODULE__, [])

  with {:ok, true} <- Telegex.delete_webhook(),
       {:ok, true} <- Telegex.set_webhook(env_config[:webhook_url],
                        secret_token: env_config[:secret_token]) do
    %Telegex.Hook.Config{server_port: env_config[:server_port]}
  else
    error ->
      raise "Failed to configure Telegram webhook: #{inspect(error)}"
  end
end

---

Security — TG_WEBHOOK_SECRET is optional

In runtime.exs, TG_WEBHOOK_URL is required but TG_WEBHOOK_SECRET silently defaults to nil:

secret_token: System.get_env("TG_WEBHOOK_SECRET"),  # can be nil

Without a secret token, any actor who discovers the webhook URL can forge updates. For production this should either be required (with || raise) or at least emit a loud warning at startup:

secret_token =
  System.get_env("TG_WEBHOOK_SECRET") ||
    raise "environment variable TG_WEBHOOK_SECRET is missing"

---

Stale File Path in AGENTS.md

The module was renamed but the file path was not updated:

- **`Bodhi.TgUpdateHandler`** (`lib/bodhi/tg_webhook_handler.ex`) - Updated at line ~130

Should be lib/bodhi/tg_update_handler.ex.

---

Unintended Dev Port Change

config/dev.exs changes the Phoenix endpoint port from 4000 → 4003. This looks like a local development leftover that shouldn't be in this PR.

---

Missing server_port Fallback in TgHookHandler

If TG_WEBHOOK_PORT is not set, env_config[:server_port] is nil (the String.to_integer default only applies when the env var is absent — wait, actually it does default to "4001" in runtime.exs, so this is only a concern if TgHookHandler is somehow started without going through runtime.exs). Low risk, but worth a note or a module-level @default_port 4001.

---

Minor Notes

• TgPollingHandler and TgHookHandler have no tests. Since they are thin wrappers the risk is low, but a smoke test asserting on_boot/0 returns the right struct type would give future regressions a clear signal.
• The 80-character line-wrap refactoring across tg_update_handler.ex is good cleanup.
• The new @spec for send_message/3 is a welcome addition.
• The :disabled mode for test is elegant — no need to mock the handler at all.

---

Summary

Architecture	✅ Clean separation of transport vs. business logic
Correctness	⚠️ on_boot crash risk; optional webhook secret
Style	⚠️ Stale file path in AGENTS.md; dev port change leaking in
Tests	✅ Existing tests correctly updated; thin wrappers untested (low risk)
Security	⚠️ Webhook secret should be required in production

The two blocking items are the crash risk in on_boot and the optional webhook secret. Everything else is minor polish.

🤖 Generated with Claude Code

[T0ha]

Could we integrate webhook into phoenix routes without separate port?