fix(gateway): restore bespoke telegram/slack/matrix setup dispatch#94
Conversation
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 <[email protected]> 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: fc69726486
ℹ️ 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".
| # documented "leave the access token empty for password login" | ||
| # path outright, and loses Telegram's token-format validation and | ||
| # Slack's manifest-regeneration prompt. | ||
| "telegram": _s._setup_telegram, |
There was a problem hiding this comment.
When a first-time Telegram user follows _setup_telegram() and leaves the allowlist empty for the advertised open-access mode, this dispatch bypasses _setup_standard_platform(), the only flow that asks for an access policy and writes GATEWAY_ALLOW_ALL_USERS=true. The bespoke helper merely claims anyone can use the bot without saving that flag or a wildcard, while Telegram authorization fails closed without either, so the newly configured bot does not provide the access mode the user selected.
Useful? React with 👍 / 👎.
| # Slack's manifest-regeneration prompt. | ||
| "telegram": _s._setup_telegram, | ||
| "slack": _s._setup_slack, | ||
| "matrix": _s._setup_matrix, |
There was a problem hiding this comment.
When MATRIX_ACCESS_TOKEN already exists and a user chooses Reconfigure, leaves the token prompt empty, and supplies a password, _setup_matrix() saves MATRIX_PASSWORD but never removes the old access token. MatrixAdapter prioritizes the access token whenever it is present, so it continues using the old token—and may keep failing if that token was revoked—instead of switching to the newly entered password.
Useful? React with 👍 / 👎.
| # Slack's manifest-regeneration prompt. | ||
| "telegram": _s._setup_telegram, | ||
| "slack": _s._setup_slack, | ||
| "matrix": _s._setup_matrix, |
There was a problem hiding this comment.
When an existing Matrix setup has MATRIX_ENCRYPTION=true, accepting the bespoke flow's default “No” answer to “Enable end-to-end encryption?” neither removes the variable nor writes false; the adapter therefore still enables E2EE even though this flow proceeds as if it were disabled and only selects the base mautrix package. This prevents users from disabling encryption through the restored wizard and can leave startup failing when the E2EE dependencies are unavailable.
Useful? React with 👍 / 👎.
| # Slack's manifest-regeneration prompt. | ||
| "telegram": _s._setup_telegram, | ||
| "slack": _s._setup_slack, | ||
| "matrix": _s._setup_matrix, |
There was a problem hiding this comment.
When a Matrix user leaves MATRIX_ALLOWED_USERS empty as the bespoke prompt explicitly permits for open access, _setup_matrix() only prints that anyone can message the bot and saves no MATRIX_ALLOW_ALL_USERS or GATEWAY_ALLOW_ALL_USERS flag. Gateway authorization defaults to deny when no allowlist or allow-all flag exists, so this newly restored path produces a bot whose actual access policy contradicts the wizard and prevents unpaired users from using it.
Useful? React with 👍 / 👎.
| # Slack's manifest-regeneration prompt. | ||
| "telegram": _s._setup_telegram, | ||
| "slack": _s._setup_slack, | ||
| "matrix": _s._setup_matrix, |
There was a problem hiding this comment.
For a fresh password-login setup, _setup_matrix() accepts an empty user ID, then saves the password and reports that Matrix credentials were saved. The platform is enabled from the presence of MATRIX_PASSWORD, but MatrixAdapter.connect() requires both a user ID and password and immediately fails, so the restored password path can finish successfully with credentials that cannot authenticate; reject or reprompt for an empty user ID before saving this mode.
Useful? React with 👍 / 👎.
| # path outright, and loses Telegram's token-format validation and | ||
| # Slack's manifest-regeneration prompt. | ||
| "telegram": _s._setup_telegram, | ||
| "slack": _s._setup_slack, |
There was a problem hiding this comment.
When reconfiguring Slack with an existing SLACK_ALLOWED_USERS value, leaving the bespoke allowlist prompt empty is described as denying everyone except paired users, but _setup_slack() never removes the existing value. The gateway therefore continues authorizing every previously listed user despite the access policy selected in the restored wizard, which can unintentionally retain access that the operator intended to revoke.
Useful? React with 👍 / 👎.
| # Slack's manifest-regeneration prompt. | ||
| "telegram": _s._setup_telegram, | ||
| "slack": _s._setup_slack, | ||
| "matrix": _s._setup_matrix, |
There was a problem hiding this comment.
When base mautrix is already installed without its encryption dependencies and the user enables E2EE, _setup_matrix() tests only __import__("mautrix"), so it skips installing mautrix[encryption] and still reports E2EE enabled. The gateway's E2EE requirements check then refuses to connect because dependencies such as python-olm are absent; the restored flow needs to check the encryption imports or install the selected extra even when the base package imports successfully.
Useful? React with 👍 / 👎.
…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 <[email protected]>
…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 <[email protected]>
2 participants
What does this PR do?
Follow-up to #89, found by a fan-out audit of that fix. Deliberately scoped narrowly to avoid overlapping with #93, which I found already open and in-flight while working on this — see "Relationship to #93" below.
e39b468(the same commit that dropped the_PLATFORMSentries #89 restored) also unwired the bespoke_setup_telegram/_setup_slack/_setup_matrixinteractive-setup functions from_builtin_setup_fn(), even though those functions still exist intact inhermes_cli/setup.py. Without a_builtin_setup_fn()entry,_configure_platform()falls through to the generic_setup_standard_platform(), which treats the firstvarsentry (token_var) as mandatory and aborts the entire wizard if it's left empty.For Matrix that's a real, independently-verified functional break: the restored 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. The bespoke
_setup_matrix()function (still present, untouched, insetup.py) correctly implements that path. Re-wiring it back in also restores Telegram's token-format validation and Slack's manifest-regeneration prompt, both lost the same way.Relationship to #93
While preparing this, I found #93 already open, independently restoring the
dingtalk/feishu/wecom/wecom_callback_PLATFORMSentries dropped by the samee39b468refactor — and, more thoroughly than an earlier version of this branch attempted, the bespoke_setup_feishu()QR-registration flow thattests/gateway/test_setup_feishu.pyrequires. I'd initially restored those four platforms as generic vars-schema entries too, but confirmed that leavestest_setup_feishu.py's 14 tests still failing (it needs the actual bespoke function, not just picker metadata) — #93 gets that right and I don't want to duplicate/conflict with it. This PR now touches only the telegram/slack/matrix dispatch, which is untouched by #93's diff, so the two should merge cleanly in either order.Related Issue
Fixes #
Type of Change
Changes Made
hermes_cli/gateway.py: re-wire"telegram": _s._setup_telegram,"slack": _s._setup_slack,"matrix": _s._setup_matrixinto_builtin_setup_fn(); correct the stale "moved into the plugin" comments for those three (and mattermost, which was never actually plugin-migrated either — only discord genuinely is).tests/hermes_cli/test_gateway_platform_gating.py: two new tests — a regression guard tying every currently-restored built-in adapter to an_all_platforms()picker entry, and an assertion that telegram/slack/matrix resolve to their bespoke setup functions instead of silently falling back to the generic flow.How to Test
pytest tests/hermes_cli/test_gateway_platform_gating.py -v→ 6 passed (was 4).python3 -c "from hermes_cli import setup as _s; import hermes_cli.gateway as g; print(g._builtin_setup_fn('matrix') is _s._setup_matrix)"→True.pytest tests/hermes_cli/ -q -k "platform or gateway or setup"→ same 3 pre-existing failures as onmain(verified viagit stashon a clean checkout), no new ones.tests/gateway/test_setup_feishu.pystill shows its 14 pre-existing failures, unchanged — correctly out of scope here, left for fix(gateway): restore remaining platforms dropped from setup picker #93.Checklist
Code
fix(gateway): …)ruff check hermes_cli/gateway.py tests/hermes_cli/test_gateway_platform_gating.pypassesDocumentation & Housekeeping
cli-config.yaml.example— N/A (no config keys changed)🤖 Generated with Claude Code
https://claude.ai/code/session_01Bym5HhKPQ3CWb5r9bCvBq4
Generated by Claude Code