fix(gateway): restore remaining platforms dropped from setup picker - #93
Conversation
PR #89 restored telegram/slack/matrix/whatsapp/email/sms after an incomplete plugin-migration refactor (e39b468) dropped them from _PLATFORMS with comments claiming they'd "moved to plugins/platforms/ <name>/" -- directories that were never created. Re-auditing _PLATFORMS against its pre-refactor history found four more casualties of the same refactor that PR #89 didn't cover: dingtalk, feishu, wecom, and wecom_callback. - Restored _PLATFORMS entries for all four, recovered verbatim from history (e39b468~1), following PR #89's precedent of using the generic vars-schema-driven _setup_standard_platform() flow. - Feishu needed more: its bespoke _setup_feishu() interactive function (QR-code bot registration via gateway/platforms/feishu.qr_register()) was deleted outright, not just its picker metadata. Restored it verbatim and wired it back into _builtin_setup_fn() -- this is what tests/gateway/test_setup_feishu.py (pre-existing, unrelated to either fix) actually exercises; the generic vars flow can't reproduce its QR-registration UX or save_env_value() call sequence. - Cleaned up the now-stale "moved to plugins" comments for dingtalk/ wecom/wecom_callback and the telegram/slack/matrix/mattermost block in _builtin_setup_fn() (mattermost was never plugin-registered either -- only discord genuinely is). - tests/hermes_cli/test_setup.py: two tests needed gateway_mod's own prompt_yes_no mocked too -- Matrix is now pre-selected as "already configured" in the picker, so _configure_platform() reaches _setup_standard_platform()'s "Reconfigure Matrix?" prompt, which gateway.py answers via its own imported prompt_yes_no reference (separate from hermes_cli.setup's). Updates .plans/missing-platform-plugin-shims.md to Closed.
🔎 Lint report:
|
| Rule | Count |
|---|---|
unresolved-import |
1 |
First entries
tests/gateway/test_setup_feishu.py:54: [unresolved-import] unresolved-import: Module `hermes_cli.gateway` has no member `_setup_feishu`
Unchanged: 4605 pre-existing issues carried over.
Diagnostics are surfaced as warnings — this check never fails the build.
Two issues the ty diagnostic bot flagged on the recovered function:
- qr_register was bound only inside the try block, so the except
branch's `qr_register = None` fallback widened the type after ty had
already inferred it from the successful import. Pre-declare it as
None before the try so the union type is established up front.
- credentials["app_id"]/["app_secret"]/.get("domain", ...) all read as
`Unknown | str | None` from the plain (untyped) dict merge across the
QR-registration and manual-entry code paths, even though app_id/
app_secret are guaranteed non-empty strings by construction (both
paths early-return before reaching this point if either is falsy).
Explicit str() coercion satisfies the checker without changing
behavior.
Follow-up to #89, found by a fan-out audit of that fix. Scoped narrowly to avoid overlapping with #93 (which independently restores the dingtalk/feishu/wecom/wecom_callback _PLATFORMS entries dropped by the same e39b468 refactor, and — more thoroughly than an earlier draft of this change attempted — the bespoke _setup_feishu() QR flow needed by tests/gateway/test_setup_feishu.py). e39b468 also unwired the bespoke _setup_telegram/_setup_slack/ _setup_matrix flows from _builtin_setup_fn(), even though those functions still exist intact in hermes_cli/setup.py. Without them, _configure_platform() falls through to the generic _setup_standard_platform, which treats the first `vars` entry (token_var) as mandatory and aborts the whole wizard if left empty. For Matrix that's a real functional break, independent of anything #93 touches: the entry's own help text says "leave empty to use password login instead", but the generic flow aborts right there before ever asking for a user ID or password. Re-wired the three bespoke functions back into _builtin_setup_fn() to fix that, and to restore Telegram's token-format validation and Slack's manifest-regeneration prompt. Adds two regression tests: one ties every _PLATFORMS-restored built-in adapter (excluding dingtalk/feishu/wecom/wecom_callback, tracked separately per .plans/missing-platform-plugin-shims.md so this test doesn't depend on merge order with #93) to a picker entry; the other asserts telegram/slack/matrix resolve to their bespoke setup functions rather than silently falling back to the generic flow. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bym5HhKPQ3CWb5r9bCvBq4
Pre-existing, unrelated to this PR's crm changes — found while triaging CI red across the fan-out audit follow-up PRs. _setup_standard_platform() (hermes_cli/gateway.py) calls its own imported prompt_yes_no, not hermes_cli.setup's. Since Matrix now shows as "already configured" in these tests' env fixture, _configure_platform() reaches the "Reconfigure Matrix?" prompt, which was only mocked on the setup_mod side — the real gateway_mod reference fell through to a live input() call and crashed under pytest's captured stdout with "reading from stdin while output is captured". Mock gateway_mod.prompt_yes_no too, matching the pattern PR #93 independently arrived at for the same root cause. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bym5HhKPQ3CWb5r9bCvBq4
Pre-existing, unrelated to this PR's exit-code changes — found while triaging CI red across the fan-out audit follow-up PRs. _setup_standard_platform() (hermes_cli/gateway.py) calls its own imported prompt_yes_no, not hermes_cli.setup's. Since Matrix now shows as "already configured" in these tests' env fixture, _configure_platform() reaches the "Reconfigure Matrix?" prompt, which was only mocked on the setup_mod side — the real gateway_mod reference fell through to a live input() call and crashed under pytest's captured stdout with "reading from stdin while output is captured". Mock gateway_mod.prompt_yes_no too, matching the pattern PR #93 independently arrived at for the same root cause. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bym5HhKPQ3CWb5r9bCvBq4
… storage
The clear-text-storage alert on save_env_value("FEISHU_APP_SECRET", ...) in
the restored _setup_feishu() matches the identical, unflagged pattern used
for every other credential in gateway.py -- dismissed on GitHub's Security
tab rather than changed.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PTSqxb1h9MRkvGvVJqSuQh
Re-wires the bespoke _setup_telegram/_setup_slack/_setup_matrix interactive setup functions back into _builtin_setup_fn(), fixing a real functional break where Matrix's documented "leave the token empty for password login" path aborted the wizard instead (the generic _setup_standard_platform fallback treats the first vars entry as mandatory). Also restores Telegram's token-format validation and Slack's manifest-regeneration prompt. Adds two regression tests. Scoped to avoid overlap with #93.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a4540c6ef3
ℹ️ 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".
…xh2x9x # Conflicts: # hermes_cli/gateway.py
…m setup Fixes five issues Codex flagged on PR #93's restored platform-picker entries: - gateway/config.py: WECOM_CALLBACK_PORT crashed the entire gateway on startup (not just WeCom Callback) if set to a non-integer value, since int(os.getenv(...)) was unguarded. Now falls back to 8645 with a warning. - hermes_cli/gateway.py _setup_standard_platform(): - Password-type vars (WECOM_CALLBACK_CORP_SECRET, WECOM_CALLBACK_TOKEN, WECOM_CALLBACK_ENCODING_AES_KEY, DINGTALK_CLIENT_SECRET, WECOM_SECRET) were echoed in cleartext via "Current: {existing}" during reconfiguration. - DingTalk/WeCom/WeCom Callback's secret fields were treated as optional (only token_var was enforced), so the wizard could report a platform "configured!" while gateway/config.py's enablement check (which requires both the ID and secret) left it silently disabled. Added a "required" var flag, enforced alongside token_var. - The "Enable open access" choice wrote the global GATEWAY_ALLOW_ALL_USERS instead of the platform-specific {KEY}_ALLOW_ALL_USERS that gateway/run.py's authorization check already reads per-platform for every one of these platforms -- opening one bot to all users silently opened every other unconfigured-allowlist platform too. - Added a "numeric" var flag, used by WECOM_CALLBACK_PORT, so a non-numeric port is rejected at prompt time instead of persisted. - DingTalk's _PLATFORMS entry was missing an allowed-users field entirely (unlike Feishu/WeCom/WeCom Callback), so group messages were silently dropped with no pairing code after setup reported success. Added DINGTALK_ALLOWED_USERS with is_allowlist, matching the other three. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PTSqxb1h9MRkvGvVJqSuQh
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 085856f417
ℹ️ 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".
Four more issues from Codex's re-review of the earlier fixes:
- _setup_standard_platform(): the "Enable open access" choice wrote
{PLATFORM}_ALLOW_ALL_USERS=true but the DM-pairing and skip branches never
cleared it back to false. Reconfiguring a previously-open platform to a
restricted policy left it silently open, since gateway/run.py checks that
flag before pairing/allowlist logic. Now every branch writes an explicit
true/false, matching the bespoke Feishu setup's existing convention.
- Required secret fields (DingTalk/WeCom/WeCom Callback) aborted the whole
wizard on blank input even when a value was already stored -- contradicting
the "leave blank to keep it" message shown one line above. Now only aborts
when both the prompt and the existing stored value are empty.
- The WECOM_CALLBACK_PORT numeric check accepted any digit string (e.g.
70000), which passes int() but fails to bind a real TCP listener,
silently leaving the adapter disconnected. Now validates the 1-65535
range, not just numeric syntax.
- _setup_feishu(): webhook mode could be enabled with neither
FEISHU_ENCRYPT_KEY nor FEISHU_VERIFICATION_TOKEN set, which
gateway/platforms/feishu.py's webhook handler treats as "skip both auth
checks" -- letting anyone who can reach the public endpoint forge events
as any user, including allowlisted ones. Now collects a verification
credential during setup and falls back to WebSocket mode (no public
listener) if neither is provided.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PTSqxb1h9MRkvGvVJqSuQh
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f06fb3462e
ℹ️ 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".
| access_choices, | ||
| default_access_idx, | ||
| ) | ||
| allow_all_var = "EMAIL_ALLOW_ALL_USERS" if is_email else f"{platform['key'].upper()}_ALLOW_ALL_USERS" |
There was a problem hiding this comment.
When reconfiguring a platform that previously had {PLATFORM}_ALLOW_ALL_USERS=true, entering a new allowlist takes the if value branch and never clears that flag. The fresh follow-up computes and resets allow_all_var only inside the no-allowlist else branch; gateway/run.py:6292-6295 checks allow-all before consulting the saved allowlist, so every user remains authorized despite the operator selecting restricted access.
Useful? React with 👍 / 👎.
| elif access_idx == 1: | ||
| save_env_value(allow_all_var, "false") |
There was a problem hiding this comment.
When a previously allowlisted platform is reconfigured, leaving the allowlist prompt blank and choosing DM pairing leaves the old {PLATFORM}_ALLOWED_USERS value intact. _get_unauthorized_dm_behavior() at gateway/run.py:6465-6466 treats any remaining allowlist as an instruction to ignore unknown DMs, so those users never receive the pairing code promised by this branch; clear the platform allowlist when pairing is selected.
Useful? React with 👍 / 👎.
| {"name": "WECOM_CALLBACK_TOKEN", "prompt": "Callback Token", "password": True, | ||
| "help": "The Token from your WeCom callback configuration."}, | ||
| {"name": "WECOM_CALLBACK_ENCODING_AES_KEY", "prompt": "Encoding AES Key", "password": True, | ||
| "help": "The EncodingAESKey from your WeCom callback configuration."}, |
There was a problem hiding this comment.
On a fresh WeCom Callback setup, both the callback token and AES key can still be left blank while the wizard reports success and gateway/config.py enables the adapter from only the Corp ID/Secret. The follow-up added required only to Corp Secret; gateway/platforms/wecom_callback.py:354-358 passes these blank values to WXBizMsgCrypt, whose constructor rejects either omission, so URL verification and every incoming callback fail. Mark both fields required and validate the 43-character AES key before completing setup.
Useful? React with 👍 / 👎.
| if not verification_token and not encrypt_key: | ||
| print_warning(" No verification credential provided — falling back to WebSocket mode.") | ||
| connection_mode = "websocket" |
There was a problem hiding this comment.
When reconfiguring an existing authenticated Feishu webhook and selecting webhook mode again, leaving both verification prompts blank forces connection_mode to websocket even if FEISHU_VERIFICATION_TOKEN or FEISHU_ENCRYPT_KEY is already stored. The saved authentication material remains valid, but the wizard silently disregards the selected mode; consult the existing values before falling back so users do not have to re-enter secrets merely to retain webhook mode.
Useful? React with 👍 / 👎.
| {"name": "WECOM_ALLOWED_USERS", "prompt": "Allowed user IDs (comma-separated, or empty)", "password": False, | ||
| "is_allowlist": True, | ||
| "help": "Restrict which WeCom users can interact with the bot."}, |
There was a problem hiding this comment.
For users whose previous WeCom setup saved WECOM_DM_POLICY=disabled, reconfiguring through this restored generic schema cannot re-enable DMs: its access choices update WECOM_ALLOWED_USERS and WECOM_ALLOW_ALL_USERS but never update the adapter policy. gateway/platforms/wecom.py:839-840 rejects the message before gateway pairing or allow-all checks run, so even selecting open access or DM pairing leaves the bot silent. Clear or update WECOM_DM_POLICY when the wizard selects a new access mode.
Useful? React with 👍 / 👎.
| "key": "wecom", | ||
| "label": "WeCom (Enterprise WeChat)", | ||
| "emoji": "💬", | ||
| "token_var": "WECOM_BOT_ID", |
There was a problem hiding this comment.
If a user enters WECOM_BOT_ID and then omits the required secret, the wizard has already persisted the ID before aborting. Because the restored entry supplies only that ID as token_var, _platform_status() at lines 5183-5184 reports the platform as configured even though gateway/config.py:1617-1619 will not enable it; the post-setup flow can consequently offer to install or restart the gateway for a nonfunctional adapter. Add multi-credential status handling, as already done for Email and Matrix; the same issue affects the restored DingTalk, Feishu, and WeCom Callback entries.
Useful? React with 👍 / 👎.
PR #93's design note says whatsapp uses the generic vars-schema flow, but its _PLATFORMS entry has no vars schema — WhatsApp auth is QR pairing that writes session creds, not env-var credentials — so setup fell through to the unusable 'configure in config.yaml' hint. Restore the pre-refactor (e39b468^) bespoke delegate to cmd_whatsapp and map it in _builtin_setup_fn; amend the design note accordingly. Fixes DAN-2181. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ion (#104) PR #93's design note says whatsapp uses the generic vars-schema flow, but its _PLATFORMS entry has no vars schema — WhatsApp auth is QR pairing that writes session creds, not env-var credentials — so setup fell through to the unusable 'configure in config.yaml' hint. Restore the pre-refactor (e39b468^) bespoke delegate to cmd_whatsapp and map it in _builtin_setup_fn; amend the design note accordingly. Fixes DAN-2181. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…2141) (#108) The Tests workflow's status badge on main is stuck red on run 29596109244 (commit b763acc, 2026-07-17). That commit predates the restoration of _setup_feishu (PR #93) and the other test-suite repairs (#89/#94/#97/#104/#107), so its slice 5 fails on tests/gateway/ test_setup_feishu.py — ImportError: cannot import name '_setup_feishu'. The fixes are all on current main (verified locally: agent.json pins 0.15.0 matching pyproject; systemd unit renders WorkingDirectory; the issue's named tests — test_registry_manifest, test_gateway_service TestGatewayStopCleanup/TestSystemUnitPathRemapping, test_setup_feishu — all pass). PR #107's CI run (29763092727) was fully green across all six slices on Linux, proving current main is green. The badge never refreshed because the fix-bearing PRs were squash- merged by GitHub's auto-merge bot; those pushes are performed with the repository GITHUB_TOKEN, which GitHub will not use to spawn new push-triggered workflow runs. The CI Auto-Healer can only List, view, and watch recent workflow runs from GitHub Actions. USAGE gh run <command> [flags] AVAILABLE COMMANDS cancel: Cancel a workflow run delete: Delete a workflow run download: Download artifacts generated by a workflow run list: List recent workflow runs rerun: Rerun a run view: View a summary of a workflow run watch: Watch a run until it completes, showing its progress FLAGS -R, --repo [HOST/]OWNER/REPO Select another repository using the [HOST/]OWNER/REPO format INHERITED FLAGS --help Show help for command LEARN MORE Use `gh <command> <subcommand> --help` for more information about a command. Read the manual at https://cli.github.com/manual Learn about exit codes using `gh help exit-codes` Learn about accessibility experiences using `gh help accessibility` the frozen red commit (now at run_attempt 3/3), so it cannot repair a stale badge on a newer HEAD. Add so a fresh Tests run can be triggered on main's current HEAD (Actions tab or ), refreshing the badge once the code is already green. No test or source change needed — the underlying failures are already fixed on main. Fixes DAN-2141 Co-authored-by: Claude <noreply@anthropic.com>
2 participants
What does this PR do?
Finishes the platform-picker restoration that PR #89 started. An incomplete plugin-migration refactor (
e39b468) removed several messaging platforms fromhermes_cli/gateway.py's_PLATFORMSlist, claiming each had "moved toplugins/platforms/<name>/" — directories that were never created. PR #89 restored telegram/slack/matrix/whatsapp/email/sms. Re-auditing_PLATFORMSagainst its pre-refactor git history turned up four more casualties of the same refactor: dingtalk, feishu, wecom, and wecom_callback (the last three weren't in the original bug inventory — found while doing this pass).Related Issue
Fixes #
Closes the tracking doc
.plans/missing-platform-plugin-shims.md.Type of Change
Changes Made
hermes_cli/gateway.py:_PLATFORMSentries fordingtalk,feishu,wecom,wecom_callback— recovered verbatim frome39b468~1, using the generic vars-schema-driven_setup_standard_platform()flow (matching PR fix(gateway): restore built-in platforms dropped from the setup picker #89's precedent for the other six)._setup_feishu()interactive function (QR-code bot registration viagateway/platforms/feishu.qr_register()), which had been deleted outright — not just its picker metadata. The generic vars flow can't reproduce its QR UX orsave_env_value()call sequence, andtests/gateway/test_setup_feishu.py(pre-existing, unrelated to either fix) exercises exactly that. Wired it back into_builtin_setup_fn()._builtin_setup_fn()(mattermost was never actually plugin-registered either — only discord genuinely is).tests/hermes_cli/test_setup.py: two tests neededgateway_mod's ownprompt_yes_nomocked too. Matrix is now pre-selected as "already configured" in the picker, so_configure_platform()reaches_setup_standard_platform()'s "Reconfigure Matrix?" prompt — answered viagateway.py's own importedprompt_yes_noreference, separate fromhermes_cli.setup's (which was already mocked)..plans/missing-platform-plugin-shims.md: updated from Open to Closed with the full resolution history.How to Test
TZ=UTC LANG=C.UTF-8 PYTHONHASHSEED=0 python3 -m pytest tests/hermes_cli/test_gateway_platform_gating.py tests/hermes_cli/test_setup.py tests/hermes_cli/test_setup_openclaw_migration.py tests/gateway/test_setup_feishu.py tests/hermes_cli/test_gateway.py tests/hermes_cli/test_gateway_service.py tests/hermes_cli/test_gateway_linger.py -q— 240 passed (previously 16 failing across the first four files)python3 -c "import hermes_cli.gateway as gw; print([p['key'] for p in gw._all_platforms()])"— confirms all 10 restored/newly-added platforms plus the 7 genuine plugin-registry entries are presentruff check hermes_cli/gateway.py tests/hermes_cli/test_setup.py— cleantests/hermes_cli/suite (scripts/run_tests_parallel.py) — the only failures are pre-existing and unrelated (verified identical onmainviagit stash):test_arcee_provider.py,test_auth_nous_provider.py,test_kanban_core_functionality.py,test_migrate_xai.py,test_web_server.pyPTY/websocket tests.Checklist
Code
fix(scope):)Documentation & Housekeeping
.plans/missing-platform-plugin-shims.mdGenerated by Claude Code