refactor(delegate): remove dead delegation.default_toolsets config key#13681
Merged
Conversation
delegation.default_toolsets was declared in cli.py's CLI_CONFIG default dict and documented in cli-config.yaml.example, but never read: none of tools/delegate_tool.py, _load_config(), or any call site ever looked it up. The live fallback is the DEFAULT_TOOLSETS module constant at tools/delegate_tool.py:101, which stays as-is. hermes_cli/config.py's DEFAULT_CONFIG["delegation"] already omits the key — this commit aligns cli.py with that. Adds a regression test in tests/hermes_cli/test_config_drift.py so a future refactor that re-adds the key without wiring it up to _load_config() fails loudly. Part of Initiative 2 / M0.5.
Matches the default-config removal in the preceding commit. default_toolsets was documented for users to set but was never actually read at runtime, so showing it in the example config and the delegation user guide was misleading. No deprecation note is added: the key was always a no-op, so users who copied it from the example continue to see no behavior change. Their config.yaml still parses; the key is just silently unused, same as before. Part of Initiative 2 / M0.5.
…config
The prior form of this test asserted on CLI_CONFIG["delegation"] after
importing cli, which only passed by accident of pytest-xdist worker
scheduling. cli._hermes_home is frozen at module import time (cli.py:76),
before the tests/conftest.py autouse HERMES_HOME-isolation fixture can
fire, so CLI_CONFIG ends up populated by deep-merging the contributor's
actual ~/.hermes/config.yaml over the defaults (cli.py:359-366). Any
contributor (like me) who still has the legacy key set in their own
config causes a false failure the moment another test file in the same
xdist worker imports cli at module level.
Asserting on the source of load_cli_config() instead sidesteps all of
that: the test now checks the defaults literal directly and is
independent of user config, HERMES_HOME, import order, and worker
scheduling.
Demonstrated failure mode before this fix:
pytest tests/hermes_cli/test_config_drift.py \
tests/hermes_cli/test_skills_hub.py -o addopts=""
-> FAILED (CLI_CONFIG["delegation"] contained "default_toolsets"
from the user's ~/.hermes/config.yaml)
Part of Initiative 2 / M0.5.
9 tasks
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
3 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
Removes
delegation.default_toolsets— documented in the example config and delegation docs, declared incli.py'sCLI_CONFIG, but never read by_load_config()or any other site. The live fallback is theDEFAULT_TOOLSETSmodule constant intools/delegate_tool.py, which stays.Cherry-picked from @pefontana's #11215; the concurrency bump, orchestrator role, and nested-delegation pieces are left for separate review on the original PR.
Changes
cli.py: drop the dead key fromCLI_CONFIGdefaults.cli-config.yaml.example,website/docs/user-guide/features/delegation.md: stop advertising it.tests/hermes_cli/test_config_drift.py: new regression test that asserts onload_cli_config's source (robust to user config + xdist worker scheduling), so a future refactor that re-adds the key without wiring it up fails loudly.Validation
delegation.default_toolsetsadvertiseddelegation.default_toolsetsactually readtools/delegate_tool.pyDEFAULT_TOOLSETSfallbackDELEGATE_BLOCKED_TOOLS, parent intersection)scripts/run_tests.sh tests/hermes_cli/test_config_drift.py— 1 passed.Closes part of #11215.