fix(gateway): restore built-in platforms dropped from the setup picker - #89
Merged
Merged
Conversation
An incomplete plugin-migration refactor removed the telegram, slack, matrix, whatsapp, email, and sms entries from `_PLATFORMS` in hermes_cli/gateway.py, with comments claiming they had "moved to plugins/platforms/<name>/". Those plugin directories were never created, and the adapters for all six still live as built-in modules under gateway/platforms/. The result: these platforms silently vanished from `hermes setup gateway` and `_all_platforms()`, and tests/hermes_cli/test_gateway_platform_gating.py went red on main (matrix/telegram missing from the picker). Restore the six entries alongside the other still-built-in adapters (mattermost, signal, weixin, bluebubbles, qqbot, yuanbao), recovered from the pre-refactor history, and drop the now-stale "moved to plugins" comments. Windows host-gating for matrix (no python-olm wheel) is preserved by the existing filter in `_all_platforms()`. Fixes the 3 failing TestMatrixHiddenOnWindows cases; no other test behavior changes (verified: identical pass/fail set on the surrounding gateway/setup suites with and without this change). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bym5HhKPQ3CWb5r9bCvBq4
🔎 Lint report:
|
![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: bde5a60d25
ℹ️ 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".
Comment on lines
+4702
to
+4705
| "key": "whatsapp", | ||
| "label": "WhatsApp", | ||
| "emoji": "📲", | ||
| "token_var": "WHATSAPP_ENABLED", |
There was a problem hiding this comment.
When a user selects this restored WhatsApp option, it has no vars, no registry entry, and no handler in _builtin_setup_fn(), so _configure_platform() only prints the fallback instruction to edit gateway.platforms.whatsapp and returns. That cannot configure the built-in adapter: WHATSAPP_ENABLED is deliberately written only after QR pairing in hermes_cli.main.cmd_whatsapp(). Route this picker entry to that pairing flow instead of exposing a nonfunctional setup option.
Useful? React with 👍 / 👎.
Comment on lines
+4690
to
+4691
| {"name": "MATRIX_ACCESS_TOKEN", "prompt": "Access token (leave empty to use password login instead)", "password": True, | ||
| "help": "Paste your access token, or leave empty and provide user ID + password below."}, |
There was a problem hiding this comment.
When a user follows this prompt and leaves the token empty for password login, _setup_standard_platform() treats MATRIX_ACCESS_TOKEN as the required token_var, warns that Matrix will not work, and returns before asking for the user ID; the restored schema also has no MATRIX_PASSWORD field. Thus the password-login path advertised immediately above cannot be configured from the restored picker. Dispatching to the existing hermes_cli.setup._setup_matrix() flow would preserve this supported authentication mode.
Useful? React with 👍 / 👎.
Comment on lines
+4742
to
+4744
| "4. Set up your webhook URL for inbound SMS:", | ||
| " Twilio Console → Phone Numbers → Active Numbers → your number", | ||
| " → Messaging → A MESSAGE COMES IN → Webhook → https://your-server:8080/webhooks/twilio", |
There was a problem hiding this comment.
Even when a user completes every prompt in this restored SMS flow, the schema never asks for or saves SMS_WEBHOOK_URL; it only tells the user to configure the URL in Twilio. SmsAdapter.connect() requires the same public URL locally for signature validation and refuses to start when it is absent unless the unsafe SMS_INSECURE_NO_SIGNATURE=true override is used, so a fresh setup performed through this picker cannot produce a working SMS gateway. Add a prompt that persists the exact public webhook URL.
Useful? React with 👍 / 👎.
Comment on lines
+4647
to
+4649
| "1. Go to https://api.slack.com/apps → Create New App → From Scratch", | ||
| "2. Enable Socket Mode: Settings → Socket Mode → Enable", | ||
| " Create an App-Level Token with scope: connections:write → copy xapp-... token", |
There was a problem hiding this comment.
When a user creates a new Slack app from scratch by following this restored flow, none of the listed steps enables the App Home Messages Tab. Slack then blocks users from sending DMs to the bot even though message.im is subscribed; the repository's generated manifest explicitly sets messages_tab_enabled, and the Slack setup guide identifies this as required. Add the App Home step or direct users through the existing manifest-based _setup_slack() flow.
Useful? React with 👍 / 👎.
7 tasks
dizhaky
pushed a commit
that referenced
this pull request
Jul 17, 2026
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
This was referenced Jul 17, 2026
3 tasks
dizhaky
added a commit
that referenced
this pull request
Jul 17, 2026
Covers the _enabled_explicit flow end-to-end: explicit enabled:false in config.yaml survives env-token auto-enable and plugin env-enablement; implicit disabled platforms still auto-enable. The Slack setup-menu restore originally in this commit was superseded by the broader #89 (all built-in platforms restored to the picker) already on main. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
dizhaky
added a commit
that referenced
this pull request
Jul 17, 2026
* fix(gateway): honor explicit platform disable in env auto-enable Generalize the _enabled_explicit marker from Slack-only to every platform: any 'enabled' key set in config.yaml is now recorded as an explicit user choice, and _apply_env_overrides skips its auto-enable pass (env token detection / plugin env-enablement) when the user deliberately set enabled: false. Fixes the Discord adapter retry-looping at boot despite discord.enabled: false (DAN-2140). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(gateway): regression tests for explicit platform disable Covers the _enabled_explicit flow end-to-end: explicit enabled:false in config.yaml survives env-token auto-enable and plugin env-enablement; implicit disabled platforms still auto-enable. The Slack setup-menu restore originally in this commit was superseded by the broader #89 (all built-in platforms restored to the picker) already on main. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(gateway): explicit disable survives direct env token-detection passes Codex review P1 on #97: _apply_env_overrides has ~23 per-platform blocks that set enabled = True unconditionally when a token env var is present, running before the guarded plugin env-enablement pass — so an explicit enabled: false was still overridden whenever a token existed. Snapshot explicitly-disabled platforms at function entry and re-assert the flag at exit; tokens/extras seeded by env remain (harmless while disabled). Regression tests: DISCORD_BOT_TOKEN + explicit disable stays disabled (verified failing before this fix); token without explicit choice still auto-enables. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
dizhaky
added a commit
that referenced
this pull request
Jul 17, 2026
…restoration Main independently landed most of this branch's repairs while it was in flight (platform picker restore #89, bespoke setup dispatch #94, stale test repairs #84, agent.json bump). Resolution takes main's side everywhere; the only remaining delta is restoring _setup_feishu (deleted by the incomplete plugin-migration rewrite e39b468 — the plugins/platforms/feishu/ dir it supposedly moved to never existed) and its _builtin_setup_fn mapping. tests/gateway/test_setup_feishu.py is the last red file on main's CI (run 29596109244). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
github-actions Bot
pushed a commit
that referenced
this pull request
Jul 17, 2026
) * fix(gateway): restore remaining platforms dropped from setup picker 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. * fix(gateway): resolve ty type-checker warnings in restored _setup_feishu 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. * docs: record CodeQL false-positive decision on restored Feishu secret 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 * fix(gateway): address Codex review findings on restored dingtalk/wecom 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 * fix(gateway): address second round of Codex findings on setup wizard 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 --------- Co-authored-by: Claude <noreply@anthropic.com>
3 tasks
github-actions Bot
pushed a commit
that referenced
this pull request
Jul 21, 2026
…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>
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
2 participants
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 does this PR do?
Fixes a pre-existing breakage on
main: the Tests workflow has been red for days becausetests/hermes_cli/test_gateway_platform_gating.py::TestMatrixHiddenOnWindowsfails —matrixandtelegramare missing from_all_platforms().Root cause: An incomplete plugin-migration refactor deleted the
telegram,slack,matrix,whatsapp,email, andsmsentries from the static_PLATFORMSlist inhermes_cli/gateway.py, leaving comments that each "moved toplugins/platforms/<name>/". But those plugin directories were never created (onlydiscord,google_chat,irc,line,ntfy,simplex,teamsexist as platform plugins), and the adapters for all six still live as built-in modules undergateway/platforms/(telegram.py,matrix.py,slack.py,whatsapp.py,email.py,sms.py). Net effect: those platforms register nowhere during setup enumeration, so they silently vanished fromhermes setup gatewayand_all_platforms().Fix: Restore the six entries (recovered verbatim from pre-refactor history) alongside the other still-built-in adapters that kept their entries (
mattermost,signal,weixin,bluebubbles,qqbot,yuanbao), and drop the now-stale "moved to plugins" comments. This is the minimal, consistent fix — the adapters are built-in, so they belong in the built-in list. Windows host-gating for Matrix (nopython-olmwheel) is preserved by the existing filter in_all_platforms().Related Issue
Fixes #
Type of Change
Changes Made
hermes_cli/gateway.py: restoretelegram/slack/matrix/whatsapp/email/smsentries in_PLATFORMS; replace the inaccurate "moved to plugins/platforms/…" comments with a note explaining these remain built-in adapters.How to Test
pytest tests/hermes_cli/test_gateway_platform_gating.py -q→ 4 passed (was 3 failed / 1 passed).matrix/telegram/slackappear in_all_platforms(); on Windows,matrixis hidden whiletelegramstays.tests/hermes_cli/test_setup.py,test_gateway_service.py, andtest_kanban_notify.pysuites produce an identical pass/fail set with and without this change (the remaining failures there are pre-existing environment/dependency issues onmain, untouched by this PR).Checklist
Code
fix(gateway): …)ruff check hermes_cli/gateway.pypassesDocumentation & Housekeeping
cli-config.yaml.example— N/A (no config keys changed)CONTRIBUTING.md/AGENTS.md— N/ANotes / follow-ups
dingtalk,feishu, andwecomwere removed by the same refactor and are likely also affected, but they aren't covered by the failing test and their setup metadata is more involved — left out to keep this fix focused. Happy to follow up separately.plugins/platforms/<name>/(asdiscordwas) to honor the original refactor's intent; this PR just unbreaksmainwith the low-risk restore.🤖 Generated with Claude Code
https://claude.ai/code/session_01Bym5HhKPQ3CWb5r9bCvBq4
Generated by Claude Code