Skip to content

fix(oneshot): honor --skills / --ignore-rules / --ignore-user-config in -z mode - #59402

Open
beimingju-dafu wants to merge 1 commit into
NousResearch:mainfrom
beimingju-dafu:fix/oneshot-honor-skills-ignore-flags
Open

fix(oneshot): honor --skills / --ignore-rules / --ignore-user-config in -z mode#59402
beimingju-dafu wants to merge 1 commit into
NousResearch:mainfrom
beimingju-dafu:fix/oneshot-honor-skills-ignore-flags

Conversation

@beimingju-dafu

Copy link
Copy Markdown

Summary

hermes -z accepts --skills, --ignore-rules, and --ignore-user-config on the CLI but silently drops all three before run_oneshot() — despite the module docstring in hermes_cli/oneshot.py:8 explicitly promising:

Rules / memory / AGENTS.md / preloaded skills = same as a normal chat turn.

This PR connects the three flags to the actual agent construction so -z mode behavior matches the docstring and mirrors what hermes chat already does with -s/--ignore-rules/--ignore-user-config.

Motivation

Downstream evaluation harness (libero) hit this bug hard:

  • Ran 20 smoke tests with hermes -z "..." --skills libero-decompose --ignore-user-config
  • Every system_prompt_len came out identical (31311) with all user MEMORY, USER PROFILE, and AGENTS.md content — the flags had no effect
  • Root-caused via source read: run_oneshot(prompt, model, provider, toolsets) signature never consumed skills/ignore-* args; main.py dispatch sites (lines 12458, 13891) didn't forward them either
  • After the fix, the model can quote SKILL body content verbatim in -z mode, which was previously impossible

Changes

  • hermes_cli/oneshot.py (+78 lines)
    • Extend run_oneshot() signature with skills: Optional[str], ignore_rules: bool, ignore_user_config: bool
    • Set HERMES_IGNORE_USER_CONFIG / HERMES_IGNORE_RULES env vars early (mirrors main.py:cmd_chat lines 2328-2343)
    • Forward skip_context_files / skip_memory to AIAgent when --ignore-rules is set
    • Inject preloaded skills into agent.ephemeral_system_prompt (mirrors cli.py:15190-15202); use ephemeral_system_prompt — not _cached_system_prompt — because the cached prompt is rebuilt during the conversation loop
    • New helper _parse_skills_list() normalizes "a,b,c" / list / None
  • hermes_cli/main.py (+6 lines)
    • Two run_oneshot() dispatch sites (lines 12458, 13891) now forward the three params
  • tests/cli/test_oneshot_preloaded_skills.py (+154 lines, new file)
    • 10 tests covering _parse_skills_list (5), env var setting (3), skill injection (2)
    • Verified against existing tests/cli/test_cli_preloaded_skills.py — no regression (3/3 chat mode tests still green)

Verification

# Model can now quote SKILL body content verbatim in -z mode:
hermes -z "quote the first sentence of principle #9 in libero-decompose SKILL" \
  --skills libero-decompose --ignore-rules --ignore-user-config
# → outputs the actual SKILL body text (previously impossible)

# --ignore-rules now removes MEMORY from state.db system_prompt:
# Before this fix: system_prompt_len = 31311 (with MEMORY/USER PROFILE)
# After:           system_prompt_len = 23478

# All tests:
pytest tests/cli/test_oneshot_preloaded_skills.py tests/cli/test_cli_preloaded_skills.py -v
# → 13 passed

Design notes

  • Why ephemeral_system_prompt and not _cached_system_prompt?
    _cached_system_prompt is rebuilt by _restore_or_build_system_prompt() inside the conversation loop, which would overwrite any direct mutation. ephemeral_system_prompt is injected at API-call time (see agent/conversation_loop.py:490 and :815) after the cached prompt is resolved — the same mechanism chat mode uses (via HermesCLI.system_promptephemeral_system_prompt in cli_agent_setup_mixin.py:359).

  • Why not just set env vars in main.py?
    Env vars for --ignore-rules alone are insufficient — AIAgent also needs skip_context_files=True and skip_memory=True passed to its constructor (see agent/agent_init.py:1166-1170). Setting only env vars was tested and did not fully suppress MEMORY injection.

  • Behavior for unknown skills: prints stderr warning, does not abort (mirrors chat mode's tolerance in cli.py:15195-15197, though chat currently raises — this PR intentionally chooses tolerance for -z because scripted callers can't respond to interactive errors).

Backward compatibility

  • Adds only new optional parameters to run_oneshot() (default: None / False); existing callers unaffected
  • No changes to argparse — the three flags were already registered upstream
  • Chat mode untouched (verified by re-running test_cli_preloaded_skills.py)

Downstream impact

libero — a local-first AI time-management coach — depends on hermes -z for its evaluation harness. This bug caused all their smoke tests to run with polluted system_prompt (contaminated with user MEMORY that included decompose-specific preferences), producing a completely misleading 15% drift rate. After this fix + closing pollution, drift dropped to 5%. Full write-up: [link to your smoke report if you want to share]

…in -z mode

The three parameters were accepted by argparse but silently dropped by
run_oneshot(), contradicting the module docstring promise that
'Rules / memory / AGENTS.md / preloaded skills = same as a normal chat turn'.

- run_oneshot() signature: add skills / ignore_rules / ignore_user_config
- env vars HERMES_IGNORE_RULES / HERMES_IGNORE_USER_CONFIG set early
- skip_context_files / skip_memory forwarded to AIAgent
- SKILL body injected via ephemeral_system_prompt (matches chat mode)
- main.py two dispatch sites forward the new params

Verified: model can quote SKILL body verbatim when --skills is passed,
which was previously impossible in -z mode.

Carry patch — upstream PR pending.
@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 tool/skills Skills system (list, view, manage) area/config Config system, migrations, profiles labels Jul 6, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: #26633 / #31548 (oneshot drops --ignore-rules / --skills), open PR #26771 (--ignore-rules only) and open PR #51797 (--safe-mode + --ignore-rules isolation). This PR is the most comprehensive of the cluster (all three: --skills, --ignore-rules, --ignore-user-config) but #51797 additionally covers --safe-mode. Competing/overlapping fixes — flagging for a maintainer to pick or merge together.

@beimingju-dafu

Copy link
Copy Markdown
Author

This was generated by AI during triage.

Related: #26633 / #31548 (oneshot drops --ignore-rules / --skills), open PR #26771 (--ignore-rules only) and open PR #51797 (--safe-mode + --ignore-rules isolation). This PR is the most comprehensive of the cluster (all three: --skills, --ignore-rules, --ignore-user-config) but #51797 additionally covers --safe-mode. Competing/overlapping fixes — flagging for a maintainer to pick or merge together.

Thanks for the triage @alt-glitch — helpful map.

To align expectations: our downstream (libero) eval harness is currently blocked by the silent system_prompt pollution, so we are looking for the cleanest, fastest path to land this 3-flag fix.

One quick clarification on the overlap with #51797 to help triage:

  • Where they'll conflict: Both PRs fix the --ignore-rules path in oneshot.py (forwarding skip_context_files / skip_memory to AIAgent). Whichever lands first, the other rebases on those lines.
  • Unique to fix(cli): preserve one-shot isolation flags #51797: --safe-mode gates plugin/MCP discovery before startup — a completely different lifecycle stage.
  • Unique to this PR: --skills (via ephemeral_system_prompt) and --ignore-user-config. These are the missing pieces promised by the oneshot.py docstring.

We heavily lean toward keeping this scoped to the 3 flags (Option 1). The discovery-gating in #51797 is a separable architectural stage. Letting #51797 land independently keeps both reviews narrow, and I am happy to rebase on top of it immediately.

Cc @hzhaoy for awareness on the potential rebase coordination. If the maintainers strongly prefer folding the safe-mode axis here instead, let me know.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tracing the oneshot path. The core premise is verified on current main: hermes_cli/_parser.py:200-232 accepts these flags, while the two run_oneshot() dispatches (hermes_cli/main.py:12675-12681, :14832-14838) still omit them and hermes_cli/oneshot.py:393-417 does not pass the normal CLI skip flags.

Problems

  • The new test asserts mock_agent.chat.called at tests/cli/test_oneshot_preloaded_skills.py:105, but the implementation calls agent.run_conversation() at PR hermes_cli/oneshot.py:450; the test will fail. Its scalar _run_agent stubs at test lines 57 and 68 also no longer match the (response, result) contract.
  • The warning at PR hermes_cli/oneshot.py:441 is emitted inside the stderr-to-devnull redirect established at line 226, so unknown skills remain silent in actual -z runs.
  • Current main added --usage-file support in 7dfd5077ceef4d5a6f7953c050bad1a75e86e215; the PR is conflicting and salvage must preserve that newer argument and reporting behavior.

Suggested changes

  • Rebase the implementation mechanically onto the current signature and preserve usage_file.
  • Update tests for run_conversation() and tuple returns, and assert the public forwarding plus the two AIAgent skip kwargs.
  • Surface all-missing skill failures or route the warning outside the redirect.

Automated hermes-sweeper review.


monkeypatch.delenv("HERMES_IGNORE_RULES", raising=False)

with patch("hermes_cli.oneshot._run_agent", return_value="mock response"):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_run_agent() returns (final_response, result) (also declared by this PR at hermes_cli/oneshot.py:300), but this scalar stub makes run_oneshot() hit its unpacking-error path. Return a tuple and assert _run_agent received the expected flag-derived behavior.

_run_agent("test prompt", skills="libero-decompose")

assert "SKILL_BODY libero-decompose" in mock_agent.ephemeral_system_prompt
assert mock_agent.chat.called

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_run_agent() calls agent.run_conversation() at hermes_cli/oneshot.py:450, not agent.chat(). Set mock_agent.run_conversation.return_value to a result dict and assert that method instead; this assertion otherwise fails.

Comment thread hermes_cli/oneshot.py
task_id=getattr(agent, "session_id", None),
)
if missing_skills:
sys.stderr.write(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This write occurs under redirect_stderr(devnull) from run_oneshot() line 226, so the advertised warning is never visible in a real -z invocation. Surface it through the saved stderr outside the redirect, or fail when every requested skill is missing as normal CLI does.

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

Labels

area/config Config system, migrations, profiles comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades tool/skills Skills system (list, view, manage) type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants