fix(pipeline,collector): wire client_pool in CLI; safe JSON for Telethon action payloads

[axisrow]

Summary

• pipeline run и pipeline generate в CLI теперь инициализируют ClientPool через runtime.init_pool() и передают его в ContentGenerationService — фиксит молчаливый пропуск ReactHandler/ForwardHandler с no client_pool при запуске вне веб-воркера
• Collector._get_service_action_payload: добавлен default= обработчик для datetime (.isoformat()), bytes (.hex()) и любых других неизвестных типов (repr()) — фиксит краш channel collect на сервисных сообщениях типа MessageActionChatJoinedByRequest

Closes

• bug: json.dumps crashes on non-serializable types from Telethon objects (datetime, bytes, TLObject) #469 — json.dumps crash on non-serializable Telethon types

Test plan

• python -m src.main pipeline run 1 — реакции ставятся, нет no client_pool warning
• python -m src.main channel collect <channel_with_join_requests> — завершается без TypeError

🤖 Generated with Claude Code

[axisrow]

@claude review. Focus on critical issues: bugs, security vulnerabilities, logical errors, data loss risks, performance problems. Do NOT nitpick style, naming conventions, minor formatting, or subjective preferences — only flag issues that could break functionality or cause real harm in production.

[claude]

Claude finished @axisrow's task in 4m 8s —— View job

---

PR Review

• Read changed files and gather context
• Analyze critical issues: bugs, security, logical errors, data loss, performance
• Post review findings

---

Findings

1. Connection leak in sign_in_fresh when 2FA is needed — src/telegram/auth.py:208-223

When SessionPasswordNeededError fires and no password is provided, needs_2fa is set to True and a ValueError is raised. The finally block then skips the disconnect:

finally:
    if not needs_2fa:   # False → client is never disconnected
        await client.disconnect()

The live TelegramClient (already connected at client.connect() line 207) is abandoned without cleanup. For the CLI this may be benign (process exits), but sign_in_fresh is a general helper that could be called from longer-lived contexts. The fix is straightforward: always disconnect in finally, regardless of needs_2fa.

Fix this →

---

2. 2FA CLI retry path likely broken — src/cli/commands/account.py:79-92 + src/telegram/auth.py:210-211

The two-step CLI flow for 2FA accounts is:

1. account verify-code --code CODE → raises ValueError("2FA password required")
2. account verify-code --code CODE --password SECRET → should complete auth

In step 2, sign_in_fresh creates a new client from the original pre-auth session_str (saved at send_code time) and unconditionally calls client.sign_in(phone, code, phone_code_hash=...) again. But Telegram consumed the code during step 1 when SessionPasswordNeededError was returned — auth.signIn had already been accepted server-side.

Re-submitting the same code to a fresh client will return PHONE_CODE_INVALID or PHONE_CODE_EXPIRED, causing sign_in(password=...) never to be reached. The only path that reliably works is when password_2fa is supplied on the first verify-code call (i.e. single-step, no retry needed).

Fix needed: After SessionPasswordNeededError, save the current session (post-code-verification state) back to the pending key before raising ValueError, so the retry can reconstruct a client that is already past the signIn step and only needs checkPassword.

Fix this →

---

3. client_pool not disconnected in pipeline run/generate — src/cli/commands/pipeline.py:406,461

The outer pool variable (line 147) is cleaned up in finally at line 858. However, the client_pool variables created at lines 406 and 461 are separate local variables in their elif branches and are never assigned to pool, so pool.disconnect_all() in the finally block does not cover them.

pool = None          # line 147
...
_, client_pool = await runtime.init_pool(config, db)  # lines 406, 461 — NOT assigned to `pool`
...
finally:
    if pool is not None:   # pool is still None for run/generate
        await pool.disconnect_all()

All Telegram connections opened by client_pool remain open until process exit. For a short-lived CLI invocation this is harmless, but it's worth assigning to pool (or disconnecting explicitly) for consistency.

Fix this →

---

4. Web resend_code and verify_code routes enqueue on empty inputs — src/web/routes/auth.py:149-187

Form(...) was replaced with Form("") for required fields. Unlike the send_code and save_credentials routes which now have explicit early-return guards, resend_code and verify_code accept empty strings and immediately enqueue a command with blank phone/phone_code_hash/code fields. The worker will error on these, producing an unhelpful failure in the command log. A guard similar to the other routes should be added.

---

5. Pre-auth session string stored unencrypted — src/cli/commands/account.py:54

db.set_setting(_pending_key(phone), json.dumps(info)) stores the raw MTProto session string (from send_code) in the settings table as plain JSON. The system has SESSION_ENCRYPTION_KEY support that encrypts session strings in the accounts table, but set_setting is not part of that encryption path. The window is short (until verify-code clears it), and the session is pre-auth (no user is signed in yet), but it is still a DH-keyed MTProto session that could be replayed if someone has DB read access during that window.

---

Summary

#	Severity	Location	Issue
1	Medium	auth.py:218	Connection leak — client not disconnected when needs_2fa=True
2	High	auth.py:210, account.py:79	2FA CLI retry likely fails — re-submits consumed phone code
3	Low	pipeline.py:406,461	client_pool not in finally cleanup scope
4	Low	auth.py:149,163	resend_code/verify_code enqueue on empty required fields
5	Low	account.py:54	Pre-auth session_str bypasses encryption in transit

The JSON serialization fix and the Form("") / redirect approach are both correct. The main risk to production correctness is issue #2 — 2FA accounts will fail to authenticate via CLI unless the password is passed in the same verify-code invocation.

[axisrow]

Re: Issues 1, 2, 4, 5 — already fixed in merged PR #467

This PR was based on a pre-merge commit of #467. After rebasing onto main (which now includes all #467 fixes), these issues no longer apply:

1. Connection leak in sign_in_fresh — Fixed in fix(auth): two-step CLI auth + fix 422 on web forms + persist StringSession #467: finally now disconnects unconditionally.
2. 2FA CLI retry broken — Fixed in fix(auth): two-step CLI auth + fix 422 on web forms + persist StringSession #467: code_consumed flag implemented.
3. resend_code/verify_code empty guards — Fixed in fix(auth): two-step CLI auth + fix 422 on web forms + persist StringSession #467: early-return guards added.
4. session_str unencrypted — Accepted trade-off, acknowledged in fix(auth): two-step CLI auth + fix 422 on web forms + persist StringSession #467.

Only Issue 3 (client_pool not in finally cleanup) was unique to this PR — now fixed.

[axisrow]

@claude review. Focus on critical issues: bugs, security vulnerabilities, logical errors, data loss risks, performance problems. Do NOT nitpick style, naming conventions, minor formatting, or subjective preferences — only flag issues that could break functionality or cause real harm in production.

[claude]

Claude finished @axisrow's task in 1m 42s —— View job

---

PR Review

• Read changed files and gather context
• Analyze critical issues: bugs, security, logical errors, data loss, performance
• Post review findings

---

Findings

No critical issues found in the two changed files. Here's the breakdown:

src/cli/commands/pipeline.py — client_pool wiring

The fix correctly assigns to the outer pool variable (lines 406, 462), which is initialized to None at line 147 and cleaned up in the finally block at line 860:

pool = None                                 # line 147 — outer var
...
_, pool = await runtime.init_pool(config, db)  # lines 406, 462 — overwrites outer var
client_pool = pool                             # alias for ContentGenerationService kwarg
...
finally:
    if pool is not None:
        await pool.disconnect_all()            # line 860 — correctly covers run/generate

This resolves Issue #3 from the previous review. The early return paths (provider check) exit before init_pool, so pool stays None and the finally guard if pool is not None correctly skips cleanup in those cases. ✓

src/telegram/collector.py — _get_service_action_payload JSON serializer

The default handler is correct and complete:

• datetime/date → .isoformat() — proper, reversible serialization ✓
• bytes → .hex() — correct for Telethon binary fields ✓
• fallback → repr() — safe; prevents crash at the cost of a non-reversible string, acceptable for a logging/debug payload ✓

The isinstance(obj, (datetime, date)) check works correctly — both types serialize the same way so datetime being a subclass of date causes no ambiguity. ✓

One non-critical observation: the inline from datetime import datetime, date import inside the static method re-runs on every call. Python's import system caches it so there's no performance cost, but if this method is called at high frequency (e.g. bulk collection), moving the import to the module top level would be cleaner. Not a blocker.

---

Summary

Both fixes are correct and production-safe. The previous Issue #3 (connection leak) is properly resolved. No new bugs, security issues, or data loss risks introduced.