Skip to content

fix(cli): Fix oneshot skills preload - #31591

Closed
GarlicGo wants to merge 1 commit into
NousResearch:mainfrom
GarlicGo:codex/fix-oneshot-skills
Closed

fix(cli): Fix oneshot skills preload#31591
GarlicGo wants to merge 1 commit into
NousResearch:mainfrom
GarlicGo:codex/fix-oneshot-skills

Conversation

@GarlicGo

Copy link
Copy Markdown
Contributor

Summary

  • pass top-level --skills through both oneshot launch paths
  • preload requested skills into the oneshot agent system prompt
  • add regression coverage for CLI forwarding and oneshot skill prompt injection

Root cause

hermes -z/--oneshot accepted --skills at the parser layer, but the oneshot launch path did not pass that value into run_oneshot(), and the oneshot agent construction did not build the preloaded skill prompt.

Fixes #31548.

Tests

  • uv run --with pytest --with pytest-timeout python -m pytest tests/hermes_cli/test_tui_resume_flow.py -q
  • git diff --check

@GarlicGo
GarlicGo marked this pull request as ready for review May 24, 2026 17:28
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard labels May 24, 2026
@GarlicGo GarlicGo changed the title [codex] Fix oneshot skills preload fix: Fix oneshot skills preload May 25, 2026
@GarlicGo GarlicGo changed the title fix: Fix oneshot skills preload fix(cli): Fix oneshot skills preload May 25, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for targeting a real current-main gap: both -z/--oneshot dispatches still drop args.skills (hermes_cli/main.py:12580-12586, hermes_cli/main.py:14737-14743), and hermes_cli/oneshot.py:169-426 has no skill-prompt path.

Problems

  • The proposed helper fails whenever any skill is missing. That differs from the existing CLI contract: cli.py:15931-15947 continues when at least one requested skill loaded, and fails only when all are missing.
  • This branch conflicts with current main, whose oneshot path now includes usage_file and uses _run_agent()run_conversation() returning (response, result) (hermes_cli/oneshot.py:425-426). The added fake agent's chat() method no longer matches that current contract.

Suggested changes

  • Preserve current usage_file forwarding, add the two skills forwards, and inject the built payload through ephemeral_system_prompt.
  • Mirror the mixed-valid/missing behavior in cli.py:15931-15947; add regression coverage for that case and for the current run_conversation() return shape.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 13, 2026
@GarlicGo
GarlicGo force-pushed the codex/fix-oneshot-skills branch from 6d5d023 to 2c714c4 Compare July 21, 2026 08:08
Willhong added a commit to Willhong/hermes-agent that referenced this pull request Jul 31, 2026
hermes -z accepted -s/--skills at the parser level but both dispatch
sites dropped it before run_oneshot(), so skills were silently
discarded — despite oneshot.py's own docstring promising 'preloaded
skills = same as a normal chat turn'.

- forward args.skills at both -z dispatch sites (main.py)
- build the skills prompt via agent.skill_commands
  .build_preloaded_skills_prompt and inject it through AIAgent's
  existing ephemeral_system_prompt seam
- mirror the interactive CLI contract (cli.py main()): partial-missing
  skills warn on stderr and continue; only all-missing hard-fails
  (exit 2, before the agent runs and before the stderr redirect)
- reuse cli._parse_skills_argument for flag normalization parity

Fixes NousResearch#31548. Builds on the approach from PR NousResearch#31591 (GarlicGo),
updated per review: mixed valid/missing degradation and the current
_run_agent (response, result) contract.

Tests: tests/hermes_cli/test_oneshot_skills.py (12), updated 2
run_oneshot forwarding expectations in test_tui_resume_flow.py;
66 passed across oneshot/tui-flow suites.

(cherry picked from commit 0b6bad769794855915a77d3d40a1a986a602edd9)
@teknium1

Copy link
Copy Markdown
Contributor

Merged via PR #90389 — your commits were cherry-picked onto current main with your authorship preserved in git log, plus a widening fix for a fast-path call site added after your PR. Thanks for the fix, and for being the earliest of five independent submissions!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

hermes -z (oneshot) discards --skills silently

3 participants