Skip to content

fix(cli): clear Feishu home channel when setup prompt left blank (#12423) - #12451

Open
0x003600x wants to merge 1 commit into
NousResearch:mainfrom
0x003600x:fix/feishu-home-channel-12423
Open

fix(cli): clear Feishu home channel when setup prompt left blank (#12423)#12451
0x003600x wants to merge 1 commit into
NousResearch:mainfrom
0x003600x:fix/feishu-home-channel-12423

Conversation

@0x003600x

Copy link
Copy Markdown

Problem

Interactive Feishu / Lark gateway setup (_setup_feishu() in hermes_cli/gateway.py) only called save_env_value("FEISHU_HOME_CHANNEL", ...) when the user entered a non-empty home chat ID. Leaving the prompt blank kept any previously saved value in ~/.hermes/.env, so users could not clear an optional home channel during reconfiguration (reported in #12423).

Changes

  • After the optional home-chat prompt, trim input; if non-empty, persist with save_env_value as before.
  • If blank, call remove_env_value("FEISHU_HOME_CHANNEL") so the key is dropped from persisted env.
  • Emit a short print_info line stating that an empty answer clears a previously saved home channel.
  • On successful removal, print_success(" Home channel cleared.").

Tests

  • Extend _run_setup_feishu mocks to record remove_env_value calls.
  • Add TestSetupFeishuHomeChannel: blank removes existing key, blank without prior key still invokes remove once, non-empty saves and does not remove.
  • Adapter integration tests: coerce setup result to a plain dict; inject HERMES_HOME under patch.dict(..., clear=True) so adapter init does not rely on Path.home() with an emptied environment.

Verification

python -m pytest tests/gateway/test_setup_feishu.py -q -o addopts=

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard platform/feishu Feishu / Lark adapter labels Apr 23, 2026
@DavidMetcalfe

Copy link
Copy Markdown
Contributor

Thanks for putting this together — the fix approach is correct. However, this PR targets hermes_cli/gateway.py, which no longer contains the setup function. It was moved to plugins/platforms/feishu/adapter.py::interactive_setup during the bundled-plugins refactor.

#58421 supersedes this PR — it targets the current file location and also fixes the same pattern across Discord, Slack, Mattermost, and WhatsApp (which all had the identical bug). It includes the whitespace-stripping, UX hint, and test coverage that were flagged in review here.

Recommend closing this one in favor of #58421.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused regression coverage. The underlying behavior is still present on current main, but this PR now targets a removed setup surface.

Problems

  • hermes_cli/gateway.py:5691-5692 states that _setup_feishu moved to plugins/platforms/feishu/adapter.py::interactive_setup; the active prompt still preserves a prior value on blank input at plugins/platforms/feishu/adapter.py:5606-5609.
  • The submitted test changes patch/import the legacy hermes_cli.gateway setup function, while current tests invoke the plugin setup function and patch its lazy imports at tests/gateway/test_setup_feishu.py:42-55.

Suggested changes

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists platform/feishu Feishu / Lark adapter sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants