Repository navigation
Feat/groups and channels - #2
Merged
Merged
Conversation
build_client() called TelegramClient with no identity parameters, so every connection announced itself as Telethon-on-Python with lang_code='en' regardless of the account. Worse, flood_sleep_threshold was left at Telethon's default of 60, which makes the library silently sleep through - that is, retry - every flood wait of 60 seconds or less. Scraping-induced waits are typically 5-30 seconds, so in practice every one of them was invisible: the FloodWaitError handling in sync_db.py could not fire, and AGENTS.md's rule against retrying a flood wait was being violated inside the library on every run. Pin all of it explicitly at the single construction site: - Identity comes from Config: device model, system version, app version, lang_code and system_lang_code. Honest and generic, never impersonating an official client - Telegram knows from the api_id that this is not one, and a lie it can check is worse than the truth. - flood_sleep_threshold=0, so every flood and slow-mode wait surfaces. - receive_updates=False and catch_up=False pinned. - request_retries=1, connection_retries=2, retry_delay=5, entity_cache_limit=500. TG_SYSTEM_VERSION defaults to the real platform at major.minor precision only, so a kernel patch bump cannot mutate the fingerprint on the next reconnect. TG_TIMEZONE is detected from the machine rather than hard-coded, and TG_QUIET_HOURS is parsed here so the pure predicate stays in safety.py. TG_LANG_CODE defaults to 'en' because this is a public repository and no language is right for everyone. That default is unsafe for most users, so .env.example and README.md both carry a prominent warning: it must match the interface language of the official Telegram app on the account owner's phone, or every connection contradicts what Telegram already knows. Tests assert every pinned value by name through a recording stand-in rather than a real client, and that the source tree registers zero Telethon event handlers - with receive_updates=False a handler silently never fires, and "fixing" that by re-enabling updates would subscribe the account to every message in every group it belongs to. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ADR-0004 has sync_db.py work on a copy of the session file so two processes never write one SQLite database. That reasoning holds, but it left a worse hole open and nothing guarded it: the clone carries the SAME authorization key. ADR-0004 said "Telegram sees one account with two connections, which it permits" - that sentence was the defect. Telegram permits a limit and punishes the excess with AUTH_KEY_DUPLICATED, and its own documentation says that when the error arrives "the session is already invalidated". There is no warning early enough to back off; the login is simply gone until a new SMS code arrives. Nothing stopped `just tg-sync` running while the MCP server held a connection, and --in-place being documented as "the unsafe mode" actively implied the clone was the safe one. Group support makes runs longer, which makes the overlap likelier. Add an exclusive fcntl.flock on <session_name>.lock, taken by whichever process is about to connect and released on disconnect, covering auth.py, server.py and sync_db.py. The lock is keyed to the PRIMARY session name so the server and the sync contend for the same file. The loser does not wait and does not retry: sync_db.py exits 1, server.py returns a ToolError naming the holder. sync_db.py takes the lock before cloning. Cloning first would also copy a SQLite file the server may be mid-write on, which can produce a torn clone. Verified end to end: with the lock held, a sync now refuses, exits 1, and creates no clone at all. AuthKeyDuplicatedError is translated to say plainly that the session is already dead, because an untranslated error invites exactly the retry that cannot help. Docs: ADR-0009 records the whole anti-ban envelope this feature is being built inside, including the decisions already shipped in the previous commit; ADR-0010 records the lock and explicitly does not supersede ADR-0004. SPEC-SEC-006..010 added. RISK-08 added; RISK-05 revised in place rather than duplicated, since it already covered datacentre addresses. The revoked-session runbook gains the AUTH_KEY_DUPLICATED path, and its Prevention section is corrected - it claimed the clone "makes this safe", which is the belief that caused the hole. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every rate-limit control in this project lived in a Python variable, inside an MCP stdio server that the user's editor respawns on every launch and after every crash. A 300-second cooldown was therefore cleared by reopening an editor. In-memory rate limiting in an MCP server is not rate limiting. Move the state to PostgreSQL - api_call_log, api_flood_log, api_kill_switch, client_identity - and put one limiter at the Telethon client level rather than at each call site, so a new call site cannot bypass it. Both entrypoints share the tables, so the budget is one account-wide budget. Two Telethon facts shaped the implementation, both verified against the pinned 1.36.0 rather than assumed: - _call issues NESTED requests. Every request's resolve() may call get_input_entity, which calls self(GetUsersRequest(...)), and the migrate path calls is_user_authorized() -> self(GetStateRequest()). A plain asyncio.Lock held across the delegate therefore deadlocks in the same task, permanently and with no traceback. The guard tracks an owner *task* and lets a nested call through the lock while still counting and pacing it. A ContextVar would be wrong here: it is copied into child tasks, so it would also exempt Telethon's genuinely concurrent auto-reconnect and update work, which must queue. Both nested tests are timeout-bounded, and removing the owner check makes them fail in ~2s rather than hang the suite. - connect() costs RPCs. An earlier draft exempted them from denial; that was dropped before it was written, because identifying "bootstrap" requests means matching class names and users.GetUsers is also ordinary entity resolution. Everything is counted and deniable instead, and tg_whoami now reads the budget, the kill switch and the archive straight from PostgreSQL so it still explains itself when Telegram is being refused. ADR-0009 is amended to record that, before any of its code existed. Controls: 1.5s + additive jitter between any two RPCs; 60/hour and 500/day rolling budgets that refuse rather than queue; three flood events in a rolling hour trip a 24-hour project-wide kill switch; PeerFloodError trips it indefinitely and only `just tg-killswitch-clear` turns it off; quiet hours block Telegram while leaving archive-only tools working; 200 Telegram tool calls per server process as the runaway-loop backstop. An unreachable database fails closed - a budget that evaporates when Postgres stops would make `docker stop` the bypass. jittered() is additive only. The symmetric form drops below the reviewed constant half the time, which is exactly the lowering of CHUNK_DELAY_SECONDS that AGENTS.md forbids; a 20k-sample property test pins it. FloodPremiumWaitError does not exist in telethon 1.36.0 - there is no FLOOD_PREMIUM string in the package at all - so it is resolved via getattr and also matched by its wire string. An uncounted flood would be a blind spot in the one control that must not have one. Verified against the live account: `just tg-status` reports DC 2, the identity strings and 57/60 requests left, and the ledger shows the three connect RPCs spaced 1.75s and 1.59s apart. A sync during a synthetic quiet window refuses without even cloning the session, and a tripped kill switch now exits 1 with a message instead of a traceback. Docs: SPEC-LIM-001..007 and a new LIM area, RISK-09 for silent flood accumulation, the flood runbook rewritten around the kill switch and the @SpamBot check, and `just tg-killswitch{,-clear}`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Widen the peer model so a Peer is a User, a Group or a Channel, and add the column, index and guard that go with it. resolve_peer keeps its hard rejection of non-users, narrowed rather than deleted. Before this, `--dialog @some_public_channel` performed a cold contacts.ResolveUsername on a channel the account had never joined and only then rejected the result - the request having already been made. The guard is now structural: cold resolution is permitted ONLY for a caller that will accept nothing but a User, so a caller admitting Groups or Channels has no cold path at all. It lives inside resolve_peer rather than at its call sites, so --dialog and every present and future tool inherit it. The tests assert the client mock was never touched, and removing the guard makes them fail on exactly that. Peer classification is by what you can do in a place, not by flag names: a megagroup is a Group, a gigagroup is a Channel because members cannot write in one. can_post() reads channel.admin_rights.post_messages and a group's default_banned_rights from the cached entity, so the check costs no API call. is_archivable() gains include_groups, defaulting to False, so a full sync is exactly what it was and widening it needs an explicit argument. PeerIndex now indexes groups and channels, bounds its GetDialogs walk at 200 dialogs, and keeps a shared snapshot so the unread tool can stop walking the list a second time. CACHE_TTL_SECONDS 300 -> 600. Two bugs in the contacts.getContacts hash, both found by running it against the real account rather than by reading the spec: - The field is a SIGNED 64-bit long. Returning the unsigned accumulator makes Telethon raise struct.error for about half of all inputs. It did. - The documented algorithm folds in the previous response's saved_count, not the number of users returned. On this account those are 372 and 502. Using the wrong one produces a hash that never matches, so the caching silently does nothing and there is no error to notice. Both now have regression tests. Verified live: the corrected hash earns contactsContactsNotModified, and a fresh PeerIndex over the persisted store resolves 502 contacts without Telegram re-sending them - which is the point, since an MCP server that restarts constantly would otherwise send hash=0 every launch. Schema: dialogs.peer_type as an idempotent ALTER (a column added inside CREATE TABLE IF NOT EXISTS silently never appears on an existing archive), peer_snapshot for membership evidence, contacts_cache, and a COMMENT on messages.sender_id saying it is attribution only - a min-constructor id that must never be resolved or used as a send target. Verified against the live account: 198 dialogs indexed as 125 users, 59 channels and 14 groups; subscriber channels report can_post=False while the two the account administers report True; a read-only group correctly reports False. Docs: glossary widened first per the maintenance protocol, with Group, Channel, Peer Type, Min Peer, Client Identity, Connection Lock, RPC Budget, Kill Switch and Quiet Window added. SPEC-SYNC-001 and SPEC-SND-006 revised in place, keeping their ids. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The feature itself, on top of the envelope built in the previous commits. Sync. Groups and channels are reachable only through --targets; a run with no Target Filter is exactly what it always was. Each one is read with EXACTLY one messages.getHistory of 100 messages - a separate code path, because sync_dialog's iter_messages paginates until exhausted and pagination is the scraper signature this avoids. A run naming more than 5 group targets aborts naming all of them, a run that would exceed 20 reads in a rolling 24 hours aborts, and consecutive group reads are 15s apart plus jitter. The honest consequence is stated rather than hidden: group history is a rolling window, not a backfill. A group that produced more than 100 messages since the last run leaves a permanent gap, and the sync says so instead of reporting success. Tools. tg_get_recent_messages reads a group or channel behind a 300-second per-target cooldown and the 20/day cap, both in PostgreSQL. tg_send_message requires current membership for a group and admin_rights.post_messages for a channel, both read from the cached entity so a refusal costs no API call, and refuses any group message that would need more than one chunk - in a DM that is a long reply, in a group it is flooding. A peer whose only provenance is a group message is refused before anything connects. tg_get_unread_dialogs now reads the shared dialog snapshot instead of walking the list a second time, and stops after a bounded number of dialogs examined rather than unread ones found. Personas are refused for groups and channels and filtered out of the overview. Three bugs found by running this against the real account rather than by reasoning about it: - peer_match_keys read .phone and .first_name directly, so naming any group in --targets raised AttributeError. The three peer types do not share a shape. - PeerIndex.lookup matched only usernames and ids, so a group named by its title was reported as "not one you are a member of" - the sync path worked because it used matches_target. It now falls back to a title scan over the bounded snapshot, and refuses rather than guesses when several match. - The group-read budget referenced db.GROUP_READ_SCOPE, which had been moved to safety. @guarded_tool caught it and returned text, as designed. Verified live: reading one group costs exactly one GetHistoryRequest and no read receipt of any kind - provable now that every RPC is logged; an immediate second read is refused with the remaining cooldown; and a BRAND NEW PROCESS still refuses it, which is the whole argument for putting this state in PostgreSQL. A send to a subscriber-only channel is refused with no API call, a 9000-character group message is refused, and @durov - a channel this account has not joined - is refused without contacting Telegram at all. Every error in the group/channel table is now translated, including SlowModeWaitError, which is a sibling of FloodWaitError rather than a subclass and so fell through untranslated. FloodPremiumWaitError is looked up defensively and its test skips rather than pretending to cover it. The blacklist test scans the shipped source for 50 forbidden identifiers - member scraping, joining, forwarding, presence, typing, media download, read receipts, the peer-harvesting family, and the stats and phone namespaces - matching exact identifiers so prose about them does not fail the build. ImportContacts stays for the single-contact path and is guarded by its own test that the list can never hold more than one. Docs: SPEC-SND-001 and SPEC-RCV-003 revised in place; SPEC-SND-007, SPEC-SND-008, SPEC-RCV-005, SPEC-SYNC-007 and SPEC-PSN-009 added; RISK-10 for the archive quietly becoming a member database. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The code changes in the previous five commits carried their own ADRs, SPEC clauses and RISK entries. This closes the rest of the union the documentation map produces for them. Corrections, not just additions - three documents asserted things the code had made false: - deployment-plan.md said "just tg-sync is safe to run while an agent session is open - it works on a session clone". That is precisely the belief ADR-0010 disproves: the clone shares the authorization key, and the sync now refuses rather than duplicating it. - The PRD listed groups and channels as a non-goal, and README, USE-CASES, llms.txt and both Russian mirrors said the project reads private chats only. - sad.md repeated the "Telegram sees one account with two connections - which it permits" line from ADR-0004, which is the sentence that hid the hole. Additions: the data model gains peer_type, the API safety tables and the explanation of why peer_type ships as an ALTER; sad.md gains the layering that lets the RPC limiter span db and tg_client without either importing the other; mcp-tools.md gains a groups-and-channels section stating both rate limits; qa-and-testing.md gains eight test files and eight manual checks, plus a note that "coverage" here means the clause table rather than a measured percentage, since adding a coverage tool would need its own ADR. status.md is rewritten rather than appended to, per the protocol. It now records what was actually verified against the live account today, including the three tests that were checked by deliberately breaking the code. The anti-ban language warning is mirrored in Russian, which is the case it was written for: an account whose app is in Russian while every connection reports "en" is the exact mismatch it describes. TASK-014 (set TG_LANG_CODE), TASK-015 (watch the flood log over several days) and Q-003 (whether group history should ever paginate) are queued rather than left implicit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes the gaps left in the required test list. These were verified against the live account but had no offline test, which means nothing would have caught a regression. - The Group and Channel send guard end to end: a two-chunk message is refused and NOT EVEN THE FIRST CHUNK is sent; a channel subscriber cannot post; a group the account has left is refused; a short group message goes through. - A peer seen only inside a group is refused before anything connects. That test deliberately leaves telegram() unstubbed, so reaching it fails. - Persona isolation, including that neither persona tool can reach around the guard by calling archived_dialog directly, and that a NULL peer_type (a row archived before the column existed) is not treated as a group. - The volume caps pinned as values - 100 messages, 5 targets, 20 reads, 300 seconds - and the channel delay proven longer than the private one. - The min-peer query shape and the sender_id schema comment. Checked by sabotage: disabling the chunk guard makes its test fail, as with the re-entrancy, cold-resolution and blacklist tests earlier. The SRS Test lines that said "manual" for these now name the tests instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
Why
Checklist
just checkpassesSPEC-clause indocs/10-product/srs.md, naming its test.env,*.session) appears in the diffDOCS SYNC