revert: withdraw multi-account Telegram until a bot is governable (#2203)

[chenmingwei23]

What is the problem?

Multi-account Telegram support (#2203) shipped an inbound-trust surface without a governance layer. A named account under telegram.accounts carries its own bot token and sender allow-list, and nothing else:

• No per-account enable switch. A token present under telegram.accounts.<name> means that bot is live. The only kill switch is the global telegram.enabled, which takes down every account at once.
• Posture is scoped to the channel type, not the bot. The governance scope member is telegram, so every account shares one ceiling. An operator cannot run one bot read-only and another permissive.
• The sender allow-list lives in config, not in the policy layer. allowed_user_ids sits in config.json, which the agent can rewrite — so the thing that decides who may talk to a bot is not agent-immutable.
• Named accounts are misattributed in the audit log. The dispatcher builds session keys as telegram.<account>:… (telegram/gateway.py, channel_name), while sel._infer_source matches only the telegram: and telegram_ prefixes. Every named-account turn therefore falls through to the trailing return "slack" and is recorded against the wrong surface.
• Named accounts are additionally excluded from dashboard mirroring and /link, and forum topics are force-disabled for them, so the feature is partial even on its own terms.

Why this issue matters to the user

Adding a second bot opens a second globally-reachable inbound door — a Telegram bot is addressable by @username from anywhere — and the operator gets no way to close that one door, no way to give it a narrower posture than the first, and audit records that name the wrong surface when they go looking. For the enterprise case this is the inverse of the feature's purpose: it widens the reachable attack surface while making the widening harder to observe. Reviewer feedback on #2203 asked for exactly this governance control before the capability ships.

How our fix solves it

Withdraw the multi-account runtime, and keep the config surface it shipped as a deprecated, inert passthrough.

Chaining from symptom to root cause: the symptom is that a named bot cannot be disabled, cannot carry its own posture, and reports as slack. The cause is that the account map was added as transport-level plumbing — telegram.accounts.<name> → TelegramAccountConfig, consumed directly by the startup loop — so an account never becomes a first-class entity anywhere above the transport: it has no identity in the governance ScopedMap, no enrollment step distinct from "a token exists", and no entry in _infer_source's namespace list. The root cause is that multi-account landed as a transport loop rather than as a new governed unit, so every layer above the transport still assumes exactly one Telegram connection exists.

That assumption cannot be patched away incrementally without first deciding what the governed unit is. Withdrawing the runtime restores the single-account invariant the rest of the stack already agrees on (governance scope, SEL attribution, dashboard mirroring, /link) and leaves the capability free to return on top of a connection-level governance model rather than underneath one.

The config keys, however, are not unshipped — they went out in v0.2.0-rc.6–rc.8, so an operator's config.json may already hold tokens and allow-lists under them. A plain revert would delete the dataclass, and since to_dict() rewrites the whole telegram section from asdict(self.telegram), the next cfg.save() would erase those tokens and the agents.<a>.telegram_account bindings with no way to recover them. So the withdrawal is split:

Runtime — removed:

• the per-account startup loop and _resolve_agent_for_account (telegram/gateway.py)
• channel_name threading through TelegramDispatcher, restoring the single "telegram" session-key channel (telegram/transport_dispatch.py)
• TelegramConfig.resolved_accounts(), the shim that made an account map stand in for the top-level fields

Config — retained, marked deprecated=True, read by nothing:

• telegram.accounts and TelegramAccountConfig, still parsed by _parse_telegram_accounts and still serialized by to_dict(), so an existing config round-trips intact
• agents.<a>.telegram_account, same reasoning

A shadowed token must stay shadowed. resolved_accounts() returned the account map directly when it was non-empty, so a config with named accounts served only those accounts — the top-level bot_token and allowed_user_ids were shadowed, and maybe_start_telegram extended that to the credential path by gating the TELEGRAM_BOT_TOKEN env override on not has_explicit_accounts. Removing the runtime without accounting for that would let the top-level token take over on upgrade: a bot the operator stopped when they migrated starts polling again, under an allow-list that may be wider than the per-account list which replaced it. So the enabled predicate keeps Telegram off while an account map is present:

self._telegram_enabled = bool(
    cfg.telegram.enabled and self._telegram_bot_token and not cfg.telegram.accounts
)

This is strictly narrowing and takes no working bot away — for every existing config the set of served bots either shrinks (named accounts stop, which is the point) or is unchanged (a shadowed top-level token stays shadowed, which is the status quo). Re-enabling is an explicit edit rather than an upgrade side effect.

Withdrawal is announced, not silent. The orchestrator logs a WARNING naming every configured account, stating that the channel stays OFF while telegram.accounts is set and why (the entries already shadowed the top-level token, so falling back to it would start a bot you had stopped), and naming the remediation: remove the accounts block and put the one token you want served in telegram.bot_token.

CHANGELOG.md is reconciled in the same commit: the 0.2.0 line no longer advertises multiple bot accounts, and says what happens to an accounts entry written by a release candidate.

What tests we did

New test/test_telegram_accounts_deprecated.py (11 tests) locks in the three properties the withdrawal has to preserve:

• Data survives a rewrite — accounts round-trips through to_dict() with every field intact (token, allow-list, forum ids, threshold), save() writes them back to disk, and agents.<a>.telegram_account round-trips. This is the regression guard for the erase-on-save defect.
• No bot comes back to life — an account map alone never enables the channel; a stale top-level token stays shadowed; a stale env credential stays shadowed (its own case, since that path bypasses cfg.bot_token); and the channel does serve once the accounts block is removed.
• Withdrawal is observable — exactly one WARNING, naming every account, the "stays OFF" wording and the remediation; no warning at all when accounts is empty.

Verification:

• 4 mutations, all killed: dropping the not cfg.telegram.accounts gate → 2 tests red; warning suppressed → 2 red; accounts filtered out of serialization → 2 red; telegram_account filtered out → 1 red. Suite restored green after each.
• Targeted suite (-k "telegram or config_loader or slack_gateway or config_valid"): 839 passed, 13 skipped.
• isort --check-only, flake8, mypy src/kiro_crew/ (860 files) all clean. Frontend untouched, so tsc/vitest are outside this diff's blast radius.
• Repo-wide search confirms no remaining runtime reader of accounts — the only references left are the config dataclass, its parser, the enabled-predicate gate, and the deprecation warning.

Manual verification: N/A — the behavior is a config round-trip, a boolean predicate, and a log line, all covered by unit tests at the exact chokepoints (to_dict/save and the orchestrator constructor).

One file outside the withdrawal: dashboard/token_auth.py

Backend Lint & Type Check was failing on this branch for a reason that has nothing to do with the withdrawal, and the same 7-line reorder is carried here so this PR can go green on its own.

main is red: in token_auth.py the kiro_crew.dashboard.tailnet import block sits above kiro_crew.dashboard.revocation_gen, which is out of alphabetical order. Neither contributing PR could have caught it — #2424 merged at 04:49:37Z adding the tailnet block, and #2388 merged six minutes later inserting revocation_gen at a position that was correct against its base, where tailnet did not yet exist. Each branch was isort-clean alone; only the merged result is unsorted, and because the two insertions do not overlap, git reported no conflict. Every open PR in the repository inherits the failure through its merge ref.

Reproduced on a clean main checkout at 584bbb05f before concluding it was inherited. The hunk here is the exact output of the pinned isort==6.0.0 — 7 lines moved, no import added, removed, or renamed, # noqa: F401 # re-exports and its explanatory comment untouched.

The same fix is also open standalone as #2478, since it unblocks every other PR and can land independently. Whichever merges first makes the other's hunk a no-op — this is a pure formatter reorder, so the two cannot diverge.

Any other suggestions on the work

The capability is worth having — it needs a governed unit first. The smaller re-land shape I would propose keeps the (now deprecated) account map as the connection identity and adds the four things that were missing:

1. a per-account enabled field, so each bot has its own kill switch;
2. a governance ScopedMap member keyed by connection id, so posture is per-bot rather than per-transport;
3. the telegram.<name>: form in _infer_source's namespace list, so audit attribution is honest;
4. the sender allow-list read from posture rather than config, so an agent cannot widen its own inbound door.

None of that requires changing session-key syntax or introducing a new abstraction layer, and the retained config keys mean a re-land can reactivate an operator's existing accounts rather than asking them to re-enter tokens. I will file it as a follow-up issue so the re-land has a spec to land against.

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed f7e5e65a8787871bd4fe0d3897c16f6bbe430349 — this comment is updated in place on each push.

Review details

All references to removed symbols are clean, orchestrator sets all attributes the rewritten gateway reads, _meta accepts **kwargs so deprecated=True is safe, the enabled predicate is all-AND (fail-closed, narrowing), and the dispatcher no longer references channel_name. The revert is internally consistent.

No findings.

[OPUS-REVIEWED] f7e5e65

Verdict parsed from the review's SHA-scoped output markers for commit f7e5e65a8787871bd4fe0d3897c16f6bbe430349.

False positive or not applicable? A repository writer can comment:
/ai-review override fable f7e5e65a8787871bd4fe0d3897c16f6bbe430349: <one-sentence reason>

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of f7e5e65a8787871bd4fe0d3897c16f6bbe430349 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] f7e5e65

False positive or not applicable? A repository writer can comment:
/ai-review override gpt f7e5e65a8787871bd4fe0d3897c16f6bbe430349: <one-sentence reason>

[github-actions]

Design Review (Fable 5) — ✅ PASS

Advisory design-level review of f7e5e65a8787871bd4fe0d3897c16f6bbe430349 — updated in place on each push; does not block merge.

Design-Verdict: PASS

A deliberate, reversible withdrawal: runtime removed, shipped config preserved round-trip, shadowing semantics kept so no stopped bot restarts on upgrade.

Suggestions

• The withdrawal announcement lives only in a gateway log line; with enabled=true + accounts set, the dashboard shows Telegram not-connected with an empty telegram_connect_error. Set that error string (e.g. in maybe_start_telegram's early return or the orchestrator) so the operator sees the reason and remediation where they'll actually look.
• The config docstrings say the accounts map is "read by nothing," but the enabled predicate in slack/gateway.py does read it to hold the channel off — state that one remaining reader in the field's metadata so a future cleanup doesn't delete the gate along with the "dead" keys.

[DESIGN-REVIEWED] f7e5e65

[chenmingwei23]

Dispositions for review of e7dbea95833081243931b6534ded79eddfaeaddb, now at af4ae8851.

GPT 5.6 — BLOCKING — config/loader.py:4104 — "Config saves erase reverted Telegram account settings": FIXED.

The finding is correct and the mechanism is exactly as described. to_dict() rebuilds the whole telegram section from asdict(self.telegram), so deleting the dataclass field does not merely stop reading accounts — it makes the next cfg.save() write a telegram section without it. Any token or allow-list an operator had under telegram.accounts, plus every agents.<a>.telegram_account binding, would be gone with no recovery path. Confirmed shipped surface, not pre-release: git tag --contains 53a2ec1c8 returns v0.2.0-rc.6, rc.7, rc.8.

The fix follows the suggested shape — preserve parsing and serialization, disable the runtime:

• TelegramAccountConfig, _parse_telegram_accounts, telegram.accounts and agents.<a>.telegram_account are all retained and now carry deprecated=True metadata (the flag config/validation.py::_is_deprecated_path already reads).
• Nothing reads them at runtime. The per-account startup loop, _resolve_agent_for_account, the channel_name threading through TelegramDispatcher, and the or cfg.telegram.accounts clause in the enabled predicate all stay removed.

Guarded by test/test_telegram_accounts_deprecated.py::TestAccountsSurvivesSave — round-trip through to_dict(), a real save() to disk, and the agent binding. Mutation-verified: filtering accounts out of serialization turns 2 tests red, filtering telegram_account out turns 1 red.

Opus 4.8 — no findings: noted, nothing to disposition.

[chenmingwei23]

Disposition for Design Review (Fable 5) — 🟡 CONCERNS on e7dbea95833081243931b6534ded79eddfaeaddb, now at af4ae8851. Both suggestions accepted and implemented — the "treats a shipped config contract as unshipped" reading was right, and it turned out to be the same defect GPT flagged from the data-loss side.

Watch — "the reverted surface is not pre-release; RC users lose their channel silently": FIXED.

Confirmed independently: git tag --contains 53a2ec1c8 returns v0.2.0-rc.6, rc.7, rc.8. The PR's original compat check was indeed one deployment, not the installed base, and the conclusion I drew from it was too broad. Two changes:

1. The config keys are no longer deleted at all. telegram.accounts, TelegramAccountConfig and agents.<a>.telegram_account are retained as deprecated=True passthroughs that are parsed and serialized but read by no runtime path — so an RC user's tokens and allow-lists survive the next cfg.save() instead of being erased by the section rewrite in to_dict().
2. Suggestion 1 (log a warning) — implemented. GatewayOrchestrator.__init__ now logs a WARNING when raw config carries a non-empty telegram.accounts, naming every configured account, stating the remediation (move the token you want served to telegram.bot_token), and — going slightly past the suggestion — appending No telegram.bot_token is set, so the Telegram channel is OFF. only when that is actually true, so the line distinguishes "your second bot stopped" from "your only bot stopped". Guarded by four tests including the presence and absence of the OFF clause; mutation-verified (suppressing the warning turns 2 tests red).

Suggestion 2 (reconcile CHANGELOG.md:97) — implemented in this commit, per docs/build/release.md. The 0.2.0 channels line no longer advertises "multiple bot accounts per gateway"; it states that serving multiple accounts was withdrawn before the release, gives the one-sentence reason (a second inbound door is only worth having once a bot can be turned off, given its own posture, and named honestly in the audit log), and tells an RC user what happens to an existing accounts entry.

One-way-door check: withdrawal is deliberately not irreversible. Retaining the config keys is what keeps it reversible — a future re-land can reactivate an operator's existing accounts rather than asking them to re-enter tokens.

No UX Review concern was raised (that check reported PASS); nothing to disposition there.

[chenmingwei23]

Disposition for review of af4ae885190435d6b3717191813bc6c4079558ec, now at facd23450.

GPT 5.6 — BLOCKING — slack/gateway.py:884 — "Deprecated accounts can reactivate a stale bot": FIXED.

Verified against the pre-withdrawal code before changing anything, and the finding holds. TelegramConfig.resolved_accounts() returned self.accounts directly when it was non-empty, so a config carrying an account map served only those accounts — the top-level bot_token and allowed_user_ids were shadowed. maybe_start_telegram extended the same shadowing to the credential path: the env_token override was gated on not has_explicit_accounts, so even a TELEGRAM_BOT_TOKEN in .env did not start a bot while accounts were configured.

So the reachable consequence is worse than "an old token is still present": on upgrade, a bot the operator had deliberately stopped when they migrated starts polling again, under whatever allow-list the top-level fields still carry — which may be wider than the per-account list that replaced it. That is precisely the class of silently-widened inbound surface this PR exists to reduce, so it would have been self-defeating to ship.

Fixed as suggested — Telegram stays disabled while cfg.telegram.accounts is non-empty:

self._telegram_enabled = bool(
    cfg.telegram.enabled and self._telegram_bot_token and not cfg.telegram.accounts
)

This is strictly narrowing and takes no working bot away: a config with a non-empty account map was already not serving its top-level token, so the set of served bots for every existing config is unchanged (named accounts stop, which is the point of the PR; the top-level token stays shadowed, which is the status quo). Re-enabling is an explicit edit, not an upgrade side effect.

The warning was rewritten to match, since the old wording implied the top-level token would take over. It now states that the channel stays OFF while telegram.accounts is set, gives the reason (the entries already shadowed the top-level token, so falling back to it would start a bot you had stopped), and names the remediation: remove the accounts block and put the one token you want served in telegram.bot_token.

Tests: TestAccountsDoNotStartABot grew three cases — a stale top-level token stays shadowed, a stale env credential stays shadowed (the override path needed its own case, since it bypasses cfg.bot_token), and the channel does serve once the accounts block is removed. TestWithdrawalIsAnnounced now asserts the "stays OFF" wording.

Now 11 tests, 4 mutations all killed: dropping the not cfg.telegram.accounts gate → 2 red; suppressing the warning → 2 red; filtering accounts out of serialization → 2 red; filtering telegram_account out → 1 red. Targeted suite 839 passed / 13 skipped; isort, flake8, mypy clean.

Opus 4.8 — no findings and Design Review — PASS at af4ae8851: noted, nothing to disposition.

[chenmingwei23]

Follow-up to the facd23450 disposition, now at e37cb5797 — one correction to that comment.

It said TelegramConfig.resolved_accounts() was removed. It was not: restoring the config surface brought the method back too, and it survived with zero callers. Now genuinely deleted.

Leaving it would have been worse than ordinary dead code. resolved_accounts() is the shim that made an account map stand in for the top-level fields (if self.accounts: return self.accounts) — the exact semantics the new not cfg.telegram.accounts gate exists to preserve deliberately rather than incidentally. A future reader wiring that method back in would silently restore the shadowing behaviour from a different layer, with no test objecting.

No behaviour change from facd23450: the method had no callers at that SHA either. Gates re-run after the deletion — targeted suite 839 passed / 13 skipped, isort + flake8 + mypy clean.

[chenmingwei23]

Carrying the dashboard/token_auth.py import reorder from #2478 here as well, at 06647d26b, so this PR can reach green without waiting on that one to merge.

It is main's breakage, not this PR's: #2424 (04:49:37Z) added the tailnet import block and #2388 (04:55:14Z) inserted revocation_gen at a position correct against its own base, where tailnet did not yet exist. Both branches were isort-clean alone; only the merge is unsorted, and the insertions do not overlap so git reported no conflict. Reproduced on a clean main checkout at 584bbb05f before concluding it was inherited.

#2478 stays open because it unblocks every other PR in the repo and can land independently. Whichever merges first makes the other's hunk a no-op — it is pure formatter output, so the two cannot diverge.

[chenmingwei23]

Backend Tests (3.10, 1) went red at 06647d26b on a test unrelated to this change. Fixed at f7e5e65a8, and the underlying leak is filed separately as #2485.

test_agent_spec_preflight.py::test_healthy_install_is_silent failed on an ERROR record from the stdlib asyncio logger — a task abandoned by a different test (AcpClient._drain_stderr() over an AsyncMock stream, dying with 'coroutine' object has no attribute 'strip') whose exception surfaced inside this test's capture window. 9910 other tests in that shard passed.

The victim's assertion was over-broad: it scopes caplog.at_level to kiro_crew.slack.gateway and then asserts on every record at ERROR regardless of logger, so any stdlib noise in the worker fails it. Narrowed to r.name.startswith("kiro_crew"), which is what the test claims to prove — a healthy install logs no error from KiroCrew.

Scoped, not weakened: mutation-verified by injecting a real logger.error into GatewayOrchestrator.__init__, which still fails the test (KILLED), with the suite green again after restoring.

The leak itself is untouched here and is the real bug — a leaked drain task can inject that ERROR into any later test in the same xdist worker, so which test gets blamed depends on how the duration split happens to shard. #2485 has the diagnosis and the fix direction (await after cancel() in the owning test, MagicMock for the sync decode).

Gates after the change: 1085 passed / 5 skipped on the affected areas, isort + flake8 + mypy clean.