fix(cli,gateway): guard quick_commands non-dict entries before .get()#20654
Open
haosenwang1018 wants to merge 1 commit into
Open
fix(cli,gateway): guard quick_commands non-dict entries before .get()#20654haosenwang1018 wants to merge 1 commit into
haosenwang1018 wants to merge 1 commit into
Conversation
Closes NousResearch#18816 A quick_commands entry that is not a dict (e.g. ``foo: ''`` in config.yaml) crashed slash-command dispatch with ``AttributeError: 'str' object has no attribute 'get'``. Worse, since quick_commands dispatch runs before plugin/skill commands, a single malformed entry poisoned every command sharing that key — a valid ``/mygif`` skill could not load if ``quick_commands.mygif`` was a string. Three places had the same crash pattern: - ``cli.py``: the main slash-command dispatcher - ``gateway/run.py``: the gateway's main quick_commands dispatch - ``gateway/run.py``: the early alias-resolution block that runs before built-in dispatch All three now guard ``isinstance(qcmd, dict)`` and log a warning before falling through to plugin/skill dispatch. Behavior for valid dict entries is unchanged. Tests cover: - a string entry no longer crashes - a string entry with the same name as a skill command does not block skill dispatch - the gateway path also no longer crashes on string entries Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collaborator
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.
Issue
Closes #18816
Root cause
A
quick_commandsentry that is not a dict (e.g.foo: ''inconfig.yaml) crashed slash-command dispatch withAttributeError: 'str' object has no attribute 'get'. Worse, since quick_commands dispatch runs before plugin/skill commands, a single malformed entry poisoned every command sharing that key — a valid/mygifskill could not load ifquick_commands.mygifwas a string.The same crash pattern existed in three places — the bug report flagged one, but I found two more in
gateway/run.pywhile writing the regression test:cli.py:6580— the interactive CLI's main slash-command dispatchergateway/run.py:5247— the gateway's early alias resolution block (runs before built-in dispatch so aliases like/model openai/...reach the/modelhandler)gateway/run.py:5446— the gateway's main quick_commands dispatchFix
All three sites now guard
isinstance(qcmd, dict)before calling.get(). When a malformed entry is encountered, we log a warning and fall through to the next dispatch tier (plugin commands → skill commands → built-in commands). Behavior for valid dict entries is unchanged.The cli.py change also defensively re-validates that
quick_commandsitself is a dict (matching the pattern already used ingateway/run.py:5214).Tests
Added three regression tests in
tests/cli/test_quick_commands.py:test_string_value_does_not_crash—quick_commands={'foo': ''},/foono longer raisesAttributeError; warning is logged.test_string_value_does_not_block_skill_command_with_same_name— confirms the skill-poisoning scenario from the bug report: a stringquick_commands.mygifno longer blocks a same-named skill command from loading.test_string_value_does_not_crash_gateway— gateway equivalent; would have failed twice without both gateway fixes.All 19 tests in
tests/cli/test_quick_commands.pypass: