fix(cli): list 'cron' platform in hermes tools curses UI (#51771) - #51847
Closed
mkslzk wants to merge 2 commits into
Closed
fix(cli): list 'cron' platform in hermes tools curses UI (#51771)#51847mkslzk wants to merge 2 commits into
mkslzk wants to merge 2 commits into
Conversation
…ousResearch#51166) `_load_config()` in the Hindsight memory plugin short-circuited with `return json.loads(...)` whenever a `~/.hermes/hindsight/config.json` existed, so the HINDSIGHT_BANK_ID (and HINDSIGHT_MODE, HINDSIGHT_API_KEY, HINDSIGHT_BUDGET, ...) env vars in a profile's .env were silently ignored. The user set `HINDSIGHT_BANK_ID=nihai-tcm` but the plugin loaded bank `hermes` (or whatever the JSON had) and routed every auto retain/recall to the wrong bank. After the fix, HINDSIGHT_* env vars are treated as a per-process overlay on top of the loaded config. JSON is still the source of truth when no env var is set; unsetting the env var is the operator-facing way to let the JSON value win. HINDSIGHT_BANK_ID is surfaced at both `config.bank_id` (the read site in HindsightMemoryProvider.__init__ at line 1287) and the legacy `config.banks.hermes.bankId` location so both lookup shapes work regardless of which path seeded the config. Covered by 5 regression tests in `tests/plugins/memory/test_hindsight_load_config.py`: - env var wins when config.json exists (the headline NousResearch#51166 case) - all HINDSIGHT_* overlay env vars (mode, apiKey, budget) work - JSON value still wins when env var is unset - env vars seed the config when no config.json exists - env-var bank_id is reachable through the read site's lookup chain All 128 existing Hindsight tests remain green.
…h#51771) `hermes tools` (curses UI) did not show the `cron` platform even though `hermes tools list --platform cron` worked and the docs explicitly tell users to configure per-cron-job toolsets via the wizard. Root cause: `_get_enabled_platforms()` in `hermes_cli/tools_config.py` had branches for cli/telegram/discord/slack/whatsapp/qqbot but no branch for `cron`, so the curses UI never offered it as a configurable platform. `cron` is a non-messaging, local-only platform that needs no token, and `hermes_cli.platforms.PLATFORMS` already registers it with label "⏰ Cron" and default_toolset "hermes-cron" — the omission was a writer-side bug, not a missing registration. The fix adds `cron` to the always-included list alongside `cli`, which matches the docs (`website/docs/user-guide/features/cron.md`: "the toolset you configured for the `cron` platform in `hermes tools`"). Covered by 6 regression tests in `tests/hermes_cli/test_get_enabled_platforms.py`: - `cron` is always in the enabled list (the headline NousResearch#51771 case) - `cron` is present alongside messaging platforms - minimal install shows exactly `{cli, cron}` (no surprise platforms) - each messaging platform is still correctly gated by its env-var - `cron` is independent of all messaging tokens - the function is symmetric regardless of list ordering All 97 existing `tools_config` tests remain green — no regressions.
alt-glitch
added
type/bug
Collaborator
Duplicate of #51789 — same file ( |
Contributor
Author
|
Thanks for catching this and the detailed write-up, @alt-glitch! You're right — #51847 duplicates #51789 (and #51780 is the maintainer-preferred approach via the shared platform registry). Closing this one in favor of #51780 as suggested. Also noted on the scope-creep feedback regarding the unrelated Appreciate the maintainer time on this 🙇 |
1 task
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.
Summary
hermes tools(curses UI) did not show thecronplatform even thoughhermes tools list --platform cronworked and the docs explicitly tell users to configure per-cron-job toolsets via the wizard. Root cause:_get_enabled_platforms()inhermes_cli/tools_config.pyhad branches for cli/telegram/discord/slack/whatsapp/qqbot but no branch forcron, so the curses UI never offered it as a configurable platform.Why
cronbelongs in the always-included listcronis a non-messaging, local-only platform that needs no token, andhermes_cli.platforms.PLATFORMSalready registers it with label "⏰ Cron" and default_toolsethermes-cron— the omission was a writer-side bug, not a missing registration. The fix addscronto the always-included list alongsidecli, which matches the docs:Repro (before the fix)
Behaviour (after the fix)
hermes tools # curses UI now shows: CLI, ⏰ Cron, Discord (if configured), global — cron is configurableTests
tests/hermes_cli/test_get_enabled_platforms.py:cronis always in the enabled list (the headline [Bug]: hermes tools does not show cron platform #51771 case)cronis present alongside messaging platforms{cli, cron}(no surprise platforms)cronis independent of all messaging tokenstools_configtests remain green — no regressions (103/103 total)Related
hermes_cli/platforms.py:43registerscronin the platform registry, so no registry update is needed — only the writer-side gate in_get_enabled_platforms()website/docs/user-guide/features/cron.mdalready documents the workflow this PR enables