fix(delegation): prefer disk config over stale runtime defaults in long-lived processes#8004
fix(delegation): prefer disk config over stale runtime defaults in long-lived processes#8004fancydirty wants to merge 2 commits into
Conversation
…ng-lived processes _load_config() previously returned CLI_CONFIG delegation values immediately if they were non-empty. In long-running processes (gateway, cron) this caused config.yaml edits to be ignored because the runtime snapshot was frozen at process startup. Now disk config (config.yaml) is authoritative, and runtime config is only used to fill keys missing from disk. This prevents stale runtime defaults (e.g. max_iterations) from masking newer edits. Adds unit tests for disk-only, runtime-fill, runtime-no-override, and empty-runtime scenarios.
There was a problem hiding this comment.
Pull request overview
This PR changes how delegation configuration is resolved so that updates to config.yaml are picked up in long-lived processes (gateway/cron) instead of being masked by a stale runtime CLI_CONFIG snapshot.
Changes:
- Reworked
tools/delegate_tool.py::_load_config()to prefer disk config and only use runtime config to fill missing keys. - Added unit tests validating disk-first and runtime-fill behavior for delegation config.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tools/delegate_tool.py | Changes config resolution order/merge behavior for delegation settings. |
| tests/tools/test_delegate.py | Adds tests for the new disk-first + runtime-fill config semantics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| from hermes_cli.config import load_config | ||
| disk_cfg = load_config().get("delegation", {}) or {} | ||
| except Exception: | ||
| pass | ||
|
|
||
| runtime_cfg = {} | ||
| try: | ||
| from hermes_cli.config import load_config | ||
| full = load_config() | ||
| return full.get("delegation", {}) | ||
| from cli import CLI_CONFIG | ||
| runtime_cfg = CLI_CONFIG.get("delegation", {}) or {} | ||
| except Exception: | ||
| return {} | ||
| pass | ||
|
|
||
| # Prefer disk config; runtime only fills missing keys so that | ||
| # CLI_CONFIG defaults (e.g. max_iterations) do not overwrite | ||
| # newer config.yaml edits in long-lived processes. | ||
| merged = dict(disk_cfg) | ||
| for k, v in runtime_cfg.items(): | ||
| if k not in merged and v not in (None, ""): | ||
| merged[k] = v | ||
| return merged |
There was a problem hiding this comment.
_load_config() uses hermes_cli.config.load_config(), which deep-merges DEFAULT_CONFIG before returning. Because DEFAULT_CONFIG['delegation'] already contains keys like max_iterations, those keys will almost always be present in disk_cfg, so the runtime-fill loop will never apply for them—even when they are absent from config.yaml. This contradicts the intended behavior (“runtime only fills keys missing from disk”) and can cause runtime overrides (e.g. gateway/CLI-provided delegation.max_iterations) to be ignored.
Consider basing the “missing from disk” decision on read_raw_config() (or tracking the raw disk keys) and then layering defaults separately (e.g. start from DEFAULT_CONFIG['delegation'], overlay raw disk values, then fill keys not present in the raw disk dict from CLI_CONFIG).
| with patch("hermes_cli.config.load_config") as mock_load_config: | ||
| mock_load_config.return_value = {"delegation": {"model": "glm-5.1", "provider": "zai-customize"}} | ||
| from tools.delegate_tool import _load_config | ||
| cfg = _load_config() | ||
| self.assertEqual(cfg["model"], "glm-5.1") | ||
| self.assertEqual(cfg["provider"], "zai-customize") | ||
|
|
||
| def test_runtime_fills_missing_keys(self): | ||
| mock_cli = MagicMock() | ||
| mock_cli.CLI_CONFIG = {"delegation": {"max_iterations": 50, "reasoning_effort": "low"}} | ||
| with patch.dict(sys.modules, {"cli": mock_cli}): | ||
| with patch("hermes_cli.config.load_config") as mock_load_config: | ||
| mock_load_config.return_value = {"delegation": {"model": "glm-5.1", "provider": "zai-customize"}} | ||
| from tools.delegate_tool import _load_config |
There was a problem hiding this comment.
These tests patch hermes_cli.config.load_config to return only the user-provided delegation keys, but the real load_config() deep-merges DEFAULT_CONFIG (including delegation.max_iterations=50, etc.). Since the new _load_config() implementation currently relies on load_config(), this mocking can hide the exact behavior difference that breaks runtime-fill.
Consider adjusting the tests to match production behavior (e.g. patch read_raw_config() instead of load_config(), or include the default delegation keys in the mocked load_config() return and assert runtime-fill still works based on raw disk keys).
- _load_config() now tracks raw disk keys via read_raw_config() instead of load_config() so that runtime defaults can still fill keys missing from config.yaml. load_config() deep-merges DEFAULT_CONFIG, which made every default key appear present and prevented runtime-fill. - Tests updated to patch read_raw_config and DEFAULT_CONFIG so they match real production behavior.
|
@copilot-pull-request-reviewer thanks for the review — both points addressed in
Please take another look. |
|
Hi @copilot-pull-request-reviewer, thanks for the review. These comments appear to be based on an earlier revision of the PR. The latest commit ( Could you re-review the latest diff? |
|
Likely duplicate of #15540 — same root cause: _load_config() in delegate_tool.py returns stale CLI_CONFIG instead of re-reading disk config. |
Problem
In long-running processes (gateway, cron), `_load_config()` in `tools/delegate_tool.py` checked runtime `CLI_CONFIG` first and returned it immediately when non-empty. This meant any edits to `config.yaml` (e.g. changing `delegation.model`) were ignored after process startup, because the runtime snapshot was frozen.
Fix
Make disk config (`config.yaml`) authoritative. Runtime config (`CLI_CONFIG`) is now only used to fill keys that are missing from disk, preventing stale runtime defaults from masking newer edits.
Changes
Verification
cd /Users/mac/.hermes/hermes-agent && source venv/bin/activate && python -m pytest tests/tools/test_delegate.py -q
71 passed