fix(gateway): restore mattermost/whatsapp/dingtalk/wecom interactive setup - #103
fix(gateway): restore mattermost/whatsapp/dingtalk/wecom interactive setup#103dizhaky wants to merge 2 commits into
Conversation
…setup The plugin-migration refactor (e39b468) removed these from the bespoke setup dispatch claiming plugins/platforms/<name>/ took over, but those plugins were never created: - mattermost: dispatch mapping restored (_setup_mattermost still lives in hermes_cli/setup.py; the generic vars fallback lost its guided flow) - whatsapp: _setup_whatsapp restored (delegates to cmd_whatsapp); the platform had fallen through to a bare config.yaml hint - dingtalk, wecom: setup functions AND _PLATFORMS entries restored verbatim from e39b468^ — both platforms had vanished from the setup menu entirely (they only appear via plugin registration, which does not exist). Their adapters (gateway/platforms/{dingtalk,wecom}.py) and QR-auth helpers were never removed and still work. feishu is intentionally left out — it is being restored on dan/DAN-2143-fix-main-tests. wecom_callback remains missing (follow-up). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Same incomplete plugin migration (e39b468): the wecom_callback _PLATFORMS dict was deleted claiming a plugin took over, but no plugin exists, so the platform was invisible in 'hermes gateway setup'. The adapter (gateway/platforms/wecom_callback.py) and env-based config wiring were never removed. Entry restored verbatim from e39b468^; it uses the generic vars-based flow, so no dispatch mapping is needed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![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: 2839617a9e
ℹ️ 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".
| client_id, client_secret = result | ||
| save_env_value("DINGTALK_CLIENT_ID", client_id) | ||
| save_env_value("DINGTALK_CLIENT_SECRET", client_secret) | ||
| save_env_value("DINGTALK_ALLOW_ALL_USERS", "true") |
There was a problem hiding this comment.
When the DingTalk wizard succeeds, this writes DINGTALK_ALLOW_ALL_USERS=true without an explicit user choice; the manual path repeats the same write below. gateway/run.py::_is_user_authorized treats that as a per-platform allow-all, while gateway/platforms/dingtalk.py defaults to no sender allowlist and DINGTALK_REQUIRE_MENTION=false, so a newly configured bot added to a group will process messages from any member immediately after setup. Please keep the deny-by-default pairing/allowlist flow unless the operator explicitly opts into open access.
Useful? React with 👍 / 👎.
| "key": "dingtalk", | ||
| "label": "DingTalk", | ||
| "emoji": "💬", | ||
| "token_var": "DINGTALK_CLIENT_ID", |
There was a problem hiding this comment.
With token_var set only to DINGTALK_CLIENT_ID, the generic fallback used by the manual path treats DINGTALK_CLIENT_SECRET as optional (_setup_standard_platform only aborts when the token var is blank), and _platform_status also reports configured as soon as the client ID exists. gateway/config.py only enables DingTalk when both ID and secret are present, so a user who enters an AppKey but skips AppSecret can finish setup/restart with a platform that never loads. Please validate both credentials before the setup is considered successful.
Useful? React with 👍 / 👎.
| "key": "wecom_callback", | ||
| "label": "WeCom Callback (Self-Built App)", | ||
| "emoji": "💬", | ||
| "token_var": "WECOM_CALLBACK_CORP_ID", |
There was a problem hiding this comment.
This new generic setup entry makes only WECOM_CALLBACK_CORP_ID mandatory, so the wizard can save a Corp ID, skip WECOM_CALLBACK_CORP_SECRET, print the platform as configured, and offer to start/restart the gateway. However gateway/config.py only creates Platform.WECOM_CALLBACK when both Corp ID and Corp Secret are set, so that completed setup leaves WeCom Callback absent at runtime. Please require the secret (or provide a bespoke validator/status check) before reporting success.
Useful? React with 👍 / 👎.
| # mattermost moved into the plugin: setup_fn is registered by | ||
| # plugins/platforms/mattermost/adapter.py::register() and dispatched | ||
| # via the plugin path in _configure_platform(). | ||
| "mattermost": _s._setup_mattermost, |
There was a problem hiding this comment.
Routing Mattermost to _setup_mattermost makes the "leave empty for open access" path ineffective: that helper only prints the warning and never writes MATTERMOST_ALLOW_ALL_USERS or GATEWAY_ALLOW_ALL_USERS. The gateway then falls through _is_user_authorized with no allowlists and denies every Mattermost sender, so a user who follows the setup prompt gets a bot that appears configured but cannot be used until they manually edit auth settings.
Useful? React with 👍 / 👎.
| access_idx = prompt_choice(" How should unauthorized users be handled?", access_choices, 1) | ||
| if access_idx == 0: | ||
| save_env_value("WECOM_DM_POLICY", "open") | ||
| save_env_value("GATEWAY_ALLOW_ALL_USERS", "true") |
There was a problem hiding this comment.
When the WeCom wizard's open-access option is selected, this writes the global GATEWAY_ALLOW_ALL_USERS=true instead of the platform-specific WECOM_ALLOW_ALL_USERS=true. In installs with other platforms that rely on the default deny behavior and have no allowlist, opting WeCom into open access unexpectedly authorizes users on those platforms too.
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, |
There was a problem hiding this comment.
These callback Token and EncodingAESKey prompts are optional under _setup_standard_platform, so a user can skip them and still be told WeCom Callback is configured. The callback adapter constructs WXBizMsgCrypt for every incoming verification/message, and gateway/platforms/wecom_crypto.py raises when either value is missing or the AES key is not 43 chars, so WeCom's callbacks will be rejected with 400s even though setup completed.
Useful? React with 👍 / 👎.
| "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."}, | ||
| {"name": "WECOM_CALLBACK_PORT", "prompt": "Callback server port (default: 8645)", "password": False, |
There was a problem hiding this comment.
The generic prompt saves any non-empty WECOM_CALLBACK_PORT string, but gateway/config.py later does int(os.getenv("WECOM_CALLBACK_PORT", "8645")) without catching ValueError. If the operator types a non-numeric port during setup, the next gateway config load crashes before startup instead of falling back or showing a setup error.
Useful? React with 👍 / 👎.
| "help": "Your WeCom enterprise Corp ID."}, | ||
| {"name": "WECOM_CALLBACK_CORP_SECRET", "prompt": "Corp Secret", "password": True, | ||
| "help": "The secret for your self-built application."}, | ||
| {"name": "WECOM_CALLBACK_AGENT_ID", "prompt": "Agent ID", "password": False, |
There was a problem hiding this comment.
This Agent ID prompt is optional in the generic setup flow, but the callback adapter uses it for every reply as agentid in the WeCom message/send payload and falls back to 0 when it is blank. With Corp ID/secret plus callback crypto configured but no Agent ID, the gateway can receive callbacks yet all agent responses are rejected by WeCom, so setup should require a non-empty valid Agent ID before reporting the platform configured.
Useful? React with 👍 / 👎.
| "help": "The EncodingAESKey from your WeCom callback configuration."}, | ||
| {"name": "WECOM_CALLBACK_PORT", "prompt": "Callback server port (default: 8645)", "password": False, | ||
| "help": "Port for the HTTP callback server."}, | ||
| {"name": "WECOM_CALLBACK_ALLOWED_USERS", "prompt": "Allowed user IDs (comma-separated, or empty)", "password": False, |
There was a problem hiding this comment.
Adding this allowlist field sends WeCom Callback through _setup_standard_platform; when the operator chooses its "Enable open access" option, that helper writes the global GATEWAY_ALLOW_ALL_USERS=true rather than WECOM_CALLBACK_ALLOW_ALL_USERS=true. On a multi-platform gateway, opting only WeCom Callback into open access therefore authorizes users on every other platform that has no allowlist.
Useful? React with 👍 / 👎.
| _setup_standard_platform(dingtalk_platform) | ||
| return | ||
|
|
||
| result = dingtalk_qr_auth() |
There was a problem hiding this comment.
Selecting the new default QR setup path calls dingtalk_qr_auth(), whose missing-qrcode fallback runs uv pip install qrcode / pip install qrcode without a pinned version. On hosts without the [dingtalk] extra this now performs an unpinned runtime install from the setup wizard, bypassing the repo's pinned qrcode==7.4.2 dependency and lockfile policy; use the pinned lazy-deps path or just show the manual link when the package is absent.
Useful? React with 👍 / 👎.
|
Closing as superseded. The interactive setup for these platforms already landed in main via #93. Main dispatches setup through This PR's approach — adding bespoke |
1 participant
The plugin-migration refactor (e39b468) removed these from the bespoke
setup dispatch claiming plugins/platforms// took over, but those
plugins were never created:
in hermes_cli/setup.py; the generic vars fallback lost its guided flow)
platform had fallen through to a bare config.yaml hint
verbatim from e39b468^ — both platforms had vanished from the setup
menu entirely (they only appear via plugin registration, which does
not exist). Their adapters (gateway/platforms/{dingtalk,wecom}.py) and
QR-auth helpers were never removed and still work.
feishu is intentionally left out — it is being restored on
dan/DAN-2143-fix-main-tests. wecom_callback remains missing (follow-up).
Co-Authored-By: Claude Fable 5 noreply@anthropic.com