Conversation
…dentials TUI gateway _save_cfg() used raw yaml.safe_dump() which stripped comments, env-var templates, and didn't use atomic writes. Every TUI preference change silently destroyed those parts of config.yaml. Delegate to the existing save_config() from hermes_cli/config.py, which handles comment preservation, env-var round-tripping, and atomic writes via the same code path the CLI uses. Also fixes NousResearch#14218: hermes auth stored credentials for custom providers in auth.json, but removing the provider from config.yaml left orphaned entries with no way to clean them up. The interactive remove menu now merges auth.json custom:* pool keys with config.yaml providers so orphans appear and can be removed.
|
Initially thought this was the same issue as #14218 while investigating the TUI config destruction. Turns out the orphan problem has a separate root cause (provider removed from config.yaml, credentials stay in auth.json), but the config wipe cascade that #14276 prevents would actually mass-orphan credentials — a broken config.yaml plus any TUI interaction writes defaults to disk, stripping all custom_providers and leaving their auth.json entries stranded. The guard in #14276 blocks that write path so the broken config (custom_providers intact) stays on disk until repaired. |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for identifying the orphaned custom-pool cleanup gap. That auth issue still exists on current main, but the TUI config portion needs re-scoping before salvage.
Problems
- Current
_save_cfg()already usesatomic_config_write()attui_gateway/server.py:1954-1957(commit123c6f3a2). The proposedsave_config()replacement would not preserve arbitrary comments:save_config()performs a whole-file YAML serialization athermes_cli/config.py:7202-7206; the comment-preserving helper isutils.py:296-305. - This PR adds no regression tests for the auth fix. The still-live path is
_get_custom_provider_names()athermes_cli/auth_commands.py:40-58, consumed by_pick_provider()at:633-647. Related #14269 has focused orphan-pool discovery and picker coverage worth retaining when salvaging.
Suggested changes
- Keep and test the auth-only fix: seed an auth.json-only
custom:*pool, assert it is offered by the picker, and verify it can be selected for removal. - Drop the stale TUI save hunk; handle comment preservation separately if it remains a required behavior.
This is an automated hermes-sweeper review.
| @@ -340,15 +340,25 @@ def _load_cfg() -> dict: | |||
|
|
|||
| def _save_cfg(cfg: dict): | |||
| global _cfg_cache, _cfg_mtime | |||
There was a problem hiding this comment.
Current main has already replaced this path with atomic_config_write() at tui_gateway/server.py:1954-1957 (123c6f3a2). Please drop this stale substitution: save_config() still performs a whole-file YAML dump, so it does not preserve arbitrary existing comments.
Body
The TUI gateway writes config back to disk with raw
yaml.safe_dump(), which strips comments and env-var templates like${GLM_API_KEY}. Every time the TUI sets a display preference it silently destroys those parts of the user's config. This switches_save_cfg()to delegate tosave_config()— the same atomic write path the CLI uses — so env-var templates, comments, and structure survive round-trips.The second fix addresses orphaned credentials. When a custom provider is removed from config.yaml, its entries stay in auth.json.
hermes auth listshows them (reads auth.json) but the interactive remove menu doesn't (reads config.yaml only)._get_custom_provider_names()now also scans auth.json for orphanedcustom:*pool keys so they appear in the remove menu. Closes #14218.Changes
tui_gateway/server.py:_save_cfg()delegates tosave_config()with HERMES_HOME env var synchermes_cli/auth_commands.py:_get_custom_provider_names()merges orphaned auth.json entriesTesting
89 tests pass (TUI gateway server + auth commands). Config round-trip verified byte-identical on a 331-line config with custom_providers, mcp_servers, matrix config, and env-var templates.