Fix Telegram doctor migration for multi-account configs#62263
Conversation
Greptile SummaryAdds a guard in Confidence Score: 5/5Safe to merge; the fix is logically correct and the test adequately covers the regression. The only finding is a P2 architecture style note about a hardcoded channel ID in core. No P0 or P1 issues exist — the migration logic is correct and the test validates the key invariant. No files require special attention. Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/commands/doctor/shared/legacy-config-core-normalizers.ts
Line: 126-131
Comment:
**Hardcoded channel ID in core migration**
The `channelId === "telegram"` guard is Telegram-specific behavior embedded in a generic core normalizer. Per the project's architecture rules, core should not contain hardcoded channel-id special cases when a plugin-owned contract or manifest capability can express the same thing. A cleaner path would be for the Telegram plugin to register a capability flag (e.g. `topLevelConfigInheritedByNamedAccounts`) and have this normalizer check that generic flag instead of naming `"telegram"` directly — following the "refactor trigger" pattern described in the architecture guidelines.
**Context Used:** CLAUDE.md ([source](https://app.greptile.com/review/custom-context?memory=fd949e91-5c3a-4ab5-90a1-cbe184fd6ce8))
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix: skip telegram default-account docto..." | Re-trigger Greptile |
9eee38a to
e3ea85f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 24ee0f6f4b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -1 +1,2 @@ | |||
| export const singleAccountKeysToMove = ["streaming"]; | |||
| export const namedAccountPromotionKeys: string[] = []; | |||
There was a problem hiding this comment.
Keep Telegram auth keys promotable for named-account migrations
Defining namedAccountPromotionKeys as an empty list makes resolveSingleAccountKeysToMove reject all promotable top-level keys whenever accounts already exists, so doctor no longer seeds channels.telegram.accounts.default from legacy top-level credentials. In configs that still carry a top-level botToken/tokenFile plus named accounts, this leaves the legacy default bot unlisted (Telegram account enumeration is keyed off channels.telegram.accounts), which can silently stop that bot from being monitored/used after normalization. Restricting only policy keys (while still promoting auth keys) avoids dropping the legacy default account.
Useful? React with 👍 / 👎.
* test: cover telegram doctor multi-account fallback * fix: skip telegram default-account doctor seeding * fix: preserve telegram doctor allowlist fallback (openclaw#62263) * fix: keep doctor promotion keys plugin-owned (openclaw#62263)
* test: cover telegram doctor multi-account fallback * fix: skip telegram default-account doctor seeding * fix: preserve telegram doctor allowlist fallback (openclaw#62263) * fix: keep doctor promotion keys plugin-owned (openclaw#62263)
* test: cover telegram doctor multi-account fallback * fix: skip telegram default-account doctor seeding * fix: preserve telegram doctor allowlist fallback (openclaw#62263) * fix: keep doctor promotion keys plugin-owned (openclaw#62263)
Fixes #62200.
Summary:
Testing:
bun --eval 'import { normalizeCompatibilityConfigValues } from "./src/commands/doctor-legacy-config.ts"; import { mergeTelegramAccountConfig } from "./extensions/telegram/src/accounts.ts"; const res = normalizeCompatibilityConfigValues({ channels: { telegram: { enabled: true, dmPolicy: "allowlist", allowFrom: ["123"], groupPolicy: "allowlist", accounts: { bot1: { enabled: true, botToken: "bot-1-token" }, bot2: { enabled: true, botToken: "bot-2-token" } } } } }); console.log(JSON.stringify({ changes: res.changes, bot1: mergeTelegramAccountConfig(res.config, "bot1"), bot2: mergeTelegramAccountConfig(res.config, "bot2") }, null, 2));'pnpm test src/commands/doctor-legacy-config.migrations.test.ts -t 'preserves top-level Telegram allowlist fallback for existing named accounts'(hangs locally after starting Vitest; no result flushed)pnpm checkblocked by unrelated existingtsgofailures on this branch base