fix(oneshot): forward --skills/-s through -z/--oneshot - #75930
Closed
pos-ei-don wants to merge 2 commits into
Closed
fix(oneshot): forward --skills/-s through -z/--oneshot#75930pos-ei-don wants to merge 2 commits into
pos-ei-don wants to merge 2 commits into
Conversation
-z/--oneshot ("bypasses cli.py entirely") never forwarded the --skills
argument past _run_and_exit_oneshot()/run_oneshot() -- it was parsed at
the top level but silently dropped at both call boundaries. hermes -z
"..." --skills <name> therefore ran with no skill content injected at
all, despite exiting 0 and looking like it worked.
Wire skills through to _run_agent() and inject the preloaded skill
content via the existing ephemeral_system_prompt mechanism (kept out of
the cached/stored system prompt, injected only at actual API-call time --
matching how the interactive/chat path already treats this kind of data).
Unknown-skill validation happens before the stdout/stderr redirect so the
error message actually reaches the caller instead of vanishing into
devnull.
Verified against a real skill + real backend: without the fix the model
had zero knowledge of the skill body and asked a generic clarifying
question; with the fix it correctly used the skill's guidance and its
answer matched live source-of-truth data exactly.
alt-glitch
added
type/bug
teknium1
reviewed
Aug 1, 2026
teknium1
left a comment
teknium1
left a comment
Contributor
There was a problem hiding this comment.
Thanks for tracing the full one-shot call chain. The underlying omission is present on current main: hermes_cli/_parser.py:198 parses --skills, while both one-shot dispatches (hermes_cli/main.py:10836 and hermes_cli/main.py:12441) do not forward it. The proposed ephemeral_system_prompt route fits the cache-preserving API assembly at agent/conversation_loop.py:1585.
Problems
- The change has no regression test for the new forwarding chain. The existing construction test at
tests/hermes_cli/test_tui_resume_flow.py:151calls_run_agentdirectly and cannot catch--skillsbeing dropped inrun_oneshotor either dispatcher.
Suggested changes
- Add a hermetic
run_oneshot(..., skills=...)test that stubs the skill loader and_run_agent, then asserts the generated prompt is passed asephemeral_system_prompt; include the all-missing-skill stderr/exit-2 case. - Add dispatch coverage for both
hermes_cli/main.py:10836andhermes_cli/main.py:12441, or consolidate their argument forwarding behind a testable helper.
Automated hermes-sweeper review.
| provider=provider, | ||
| toolsets=explicit_toolsets, | ||
| use_config_toolsets=use_config_toolsets, | ||
| ephemeral_system_prompt=ephemeral_system_prompt, |
Contributor
There was a problem hiding this comment.
Please add a hermetic regression test around this handoff: stub the preload builder and _run_agent, invoke run_oneshot(..., skills=...), and assert the generated content reaches _run_agent as ephemeral_system_prompt. Existing tests call _run_agent directly, so they cannot catch a dropped --skills value.
teknium1
added
the
sweeper:blast-contained
…ispatch helper Addresses the hermes-sweeper review on NousResearch#75930: no test previously covered the forwarding chain, and the construction test at tests/hermes_cli/test_tui_resume_flow.py:151 calls _run_agent directly so it cannot catch --skills being dropped anywhere upstream. - Consolidates the two identical inline kwargs blocks at hermes_cli/main.py (formerly lines ~10836 and ~12441) behind _oneshot_kwargs_from_args(), so a field forwarded on one dispatch path cannot silently go missing on the other -- exactly how --skills got dropped in the first place. - tests/hermes_cli/test_oneshot_skills.py: hermetic coverage of _oneshot_kwargs_from_args, run_oneshot(skills=...) -> ephemeral_system_prompt (including all-missing/partial-missing/ empty-body/whitespace-only-skills-string branches), and _run_and_exit_oneshot -> run_oneshot forwarding -- closing the gap the reviewer flagged (tests calling run_oneshot directly could not have caught a regression introduced between _run_and_exit_oneshot and run_oneshot). - Verified these tests fail-to-collect against the pre-fix source (checked out upstream/main for the two touched files) before restoring the fix, confirming they actually depend on it. Uses a sys.modules stand-in for cli._parse_skills_argument rather than unittest.mock.patch, since run_oneshot does a local `from cli import _parse_skills_argument` and patch() would still import the real ~17k-line cli.py (and its prompt_toolkit dependency) to resolve the target.
Contributor
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.
-z/--oneshot("bypasses cli.py entirely" per the module's own docstring) never forwarded the--skillsargument past_run_and_exit_oneshot()/run_oneshot()— it was parsed at the top level but silently dropped at both call boundaries.hermes -z "..." --skills <name>therefore ran with zero skill content injected, despite exiting 0 and looking like it worked.This wires
skillsthrough to_run_agent()and injects the preloaded content via the existingephemeral_system_promptmechanism (kept out of the cached/stored system prompt, injected only at actual API-call time — same treatment the interactive/chat path already gives this kind of data). Unknown-skill validation happens before the stdout/stderr redirect so the error message actually reaches the caller instead of vanishing into devnull.Verified against a real skill + real backend, not just unit-level: without the fix, the model had zero knowledge of the skill body and asked a generic clarifying question. With the fix, it correctly used the skill's guidance and its answer matched live source-of-truth data exactly (two independent runs, cross-checked against the underlying system's real state).
No existing tests cover this path (
tests/hermes_cli/test_oneshot_usage_file.py/tests/agent/test_oneshot.pydon't exercise--skills) — happy to add a regression test if useful, wanted to get the fix itself in front of you first.