Skip to content

fix(cli): honor --ignore-rules in oneshot mode - #72075

Open
YuYigeng wants to merge 4 commits into
NousResearch:mainfrom
YuYigeng:agent/oneshot-ignore-rules
Open

fix(cli): honor --ignore-rules in oneshot mode#72075
YuYigeng wants to merge 4 commits into
NousResearch:mainfrom
YuYigeng:agent/oneshot-ignore-rules

Conversation

@YuYigeng

@YuYigeng YuYigeng commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Summary

This is a current-main salvage of #26771. The original fix commit is preserved with @honor2030 as its author; a second commit updates it for the current one-shot architecture.

  • thread --ignore-rules through the normal and Termux fast one-shot dispatch paths
  • honor HERMES_IGNORE_RULES=1 and HERMES_SAFE_MODE=1 for direct one-shot callers
  • construct the one-shot agent with both skip_context_files and skip_memory
  • preserve normal context and memory injection when isolation is not requested

Root cause

Top-level -z/--oneshot exits through an early dispatcher before cmd_chat() applies the usual rule-isolation policy. The parsed ignore_rules value was not passed into run_oneshot(), and _run_agent() always constructed AIAgent with context and memory injection enabled.

Current-main salvage work

Compared with #26771, this branch also:

  • updates the Termux light-parser dispatch path identified by the stale-PR sweep
  • preserves the current (response, result) contract, usage reporting, fallback configuration, and agent/session cleanup
  • covers HERMES_SAFE_MODE consistently
  • adds behavior tests for both enabled and disabled isolation

#59402 is broader (--skills and --ignore-user-config as well) and currently overlaps this narrower fix. #34852 established that default SOUL/context inclusion works; this PR does not change that default—it fixes only the explicit isolation path.

Contract note

#86328 separately proposes redefining --ignore-rules so persistent memory remains enabled. This PR intentionally follows the current CLI help and the canonical reports in #26633/#72064, which define explicit isolation as skipping context files, memory, and preloaded skills. If maintainers adopt the new policy, the one-shot wiring can be adjusted to match that decision without changing the dispatcher fix here.

Impact

hermes -z "..." --ignore-rules now matches the currently documented interactive-chat behavior: project context files and persistent memory are not injected. Normal one-shot runs remain unchanged.

Validation

  • rebased onto upstream/main at 5ed4506f4
  • scripts/run_tests.sh tests/hermes_cli/test_tui_resume_flow.py tests/hermes_cli/test_ignore_user_config_flags.py tests/hermes_cli/test_safe_mode.py tests/hermes_cli/test_oneshot_usage_file.py -q23 passed
  • ruff check hermes_cli/main.py hermes_cli/oneshot.py tests/hermes_cli/test_tui_resume_flow.py — passed
  • scripts/check-windows-footguns.py --diff upstream/main — passed
  • git diff --check upstream/main...HEAD — passed

Closes #26633
Related: #72064

@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 area/config Config system, migrations, profiles labels Jul 26, 2026
@YuYigeng
YuYigeng force-pushed the agent/oneshot-ignore-rules branch from 2a2cb8a to d12ec8f Compare July 26, 2026 17:25

Copy link
Copy Markdown
Contributor Author

Updated this Draft as a current-main salvage of #26771 rather than an independent reimplementation. The original honor2030 commit is preserved with its authorship; the follow-up commit addresses the stale-PR sweep feedback by wiring the Termux fast path, retaining current one-shot return/usage/cleanup behavior, and covering safe mode. The focused suite now passes 100 tests, plus Ruff and git diff --check.

@YuYigeng
YuYigeng force-pushed the agent/oneshot-ignore-rules branch from d12ec8f to 35eb464 Compare July 30, 2026 09:43
@YuYigeng
YuYigeng marked this pull request as ready for review July 30, 2026 09:47
@YuYigeng
YuYigeng marked this pull request as draft July 30, 2026 09:51
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for preserving the earlier contributor credit and updating the fix for the current one-shot contract. The premise is verified on current GitHub main: both top-level dispatches omit the isolation flag (hermes_cli/main.py:10761-10767, :12364-12371), and one-shot agent construction does not set either skip flag (hermes_cli/oneshot.py:245-251, :410-435). The patch addresses both dispatches, maps isolation to skip_context_files and skip_memory, and retains the current (response, result) flow.

No blocking implementation defects identified in the reviewed diff.

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 30, 2026
@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown

This was generated by AI during triage.

Summary

Six PRs address or reference this complex: #26669, #26771, and #72075 wire --ignore-rules into one-shot construction, while #40333, #49204, and #57859 address the separate --resume/--continue omission. For the reported isolation cause, #72075 is the current-main salvage covering both normal and Termux dispatchers, environment and safe-mode policy, and both required AIAgent skip flags.

Related pull requests

Duplicates

#26669, #26771, and #72075 implement the same core --ignore-rules one-shot fix, with #72075 as the current-main-complete superset; the explicit chain is #26669#72075 and #26771#72075. Separately, closed #40333 and #57859 substantially overlap on the hydration approach for #49195, while #49204 is the competing reject-rather-than-hydrate approach.

Suggested consolidation

Keep #72075 open with a salvage path: retain its two-dispatcher forwarding, safe-mode/environment resolution, both AIAgent skip flags, current one-shot contracts, focused tests, and preserved #26771 authorship. Close #26771 as duplicate of #72075 despite its keep_open review because #72075 contains the exact Termux and tuple-contract salvage requested there; keep #26669 closed as the earlier superseded reference. Handle #49204 and #57859 under #49195 rather than as isolation fixes: author action on #49204 is to move the guard before startup and add both dispatch-level tests, while author action on #57859 is to add the requested SQLite two-turn integration test and rebase while preserving usage_file; #40333 remains the closed hydration reference superseded by #57859.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I26633(["issue #26633 (open)"])
    I72064(["issue #72064 (open)"])
    subgraph Dup26669 ["PRs duplicating each other"]
        P26669["PR #26669 (closed)"]
        P26771["PR #26771 (open)"]
        P72075["PR #72075 (open)"]
    end
    P72075 -->|best fix| I26633
    P72075 -->|best fix| I72064
    class I26633 open
    class I72064 open
    class P26669 closed
    class P26771 open
    class P72075 open
    class P72075 best
    class P72075 best
    class P72075 target
    click I26633 "https://github.com/NousResearch/hermes-agent/issues/26633"
    click I72064 "https://github.com/NousResearch/hermes-agent/issues/72064"
    click P26669 "https://github.com/NousResearch/hermes-agent/pull/26669"
    click P26771 "https://github.com/NousResearch/hermes-agent/pull/26771"
    click P72075 "https://github.com/NousResearch/hermes-agent/pull/72075"
Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 6 pull requests and 3 issues in this complex. Diffs were read for 5 of 6 PRs (rest unavailable); Assessment working set: 53 kB of PR diffs, 29 kB of issue/PR text, 12 kB of discussion (15 comments), 16 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

@YuYigeng
YuYigeng force-pushed the agent/oneshot-ignore-rules branch from 35eb464 to d7d8e53 Compare August 3, 2026 14:24
@alt-glitch alt-glitch added the needs-decision Awaiting maintainer decision before any implementation label Aug 3, 2026
@YuYigeng
YuYigeng marked this pull request as ready for review August 3, 2026 14:57
@alt-glitch alt-glitch added needs-decision Awaiting maintainer decision before any implementation and removed needs-decision Awaiting maintainer decision before any implementation labels Aug 3, 2026
@YuYigeng
YuYigeng force-pushed the agent/oneshot-ignore-rules branch from d7d8e53 to bb1eb02 Compare August 15, 2026 08:50
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

fix(cli): honor --ignore-rules in oneshot mode

  1. The wiring reads the flag via getattr(args, "ignore_rules", False) — confirm --ignore-rules is actually registered on the argparse parser (and --safe-mode too, since it is also read here). If either is only read via getattr and never defined, the feature silently no-ops; the termux-path test exercises --ignore-rules through argv, which covers that path, but the safe_mode attribute path deserves the same coverage.
  2. effective_ignore_rules is computed twice — in run_oneshot and again inside _run_agent — with identical env checks (HERMES_IGNORE_RULES == "1" / HERMES_SAFE_MODE == "1"). Extracting a single helper keeps the env semantics from drifting between the two sites.
  3. skip_memory=effective_ignore_rules couples "rules" and memory: --ignore-rules also disables memory injection, which is a bigger behavioral surface than the flag name suggests. If that is intended, make sure the flag help text says so explicitly.
  4. Per the repo config policy, behavioral toggles normally live in config.yaml rather than new HERMES_* env vars. HERMES_IGNORE_RULES and HERMES_SAFE_MODE are new non-secret env surfaces — fine if HERMES_SAFE_MODE pre-exists or the env bridge is deliberate, but the doc story for these names should be explicit.

Address automated review feedback on PR NousResearch#72075:
- extract _effective_ignore_rules() so run_oneshot and _run_agent share
  one env/flag contract instead of duplicating the same check
- extract _oneshot_ignore_rules() at the two dispatch sites so
  --ignore-rules/--safe-mode are resolved in one place
- add a parser-registration regression proving both flags are real
  argparse options (not silent getattr fallbacks)
- parametrize the Termux fast-launch test across --ignore-rules,
  --safe-mode, and the bare path
@YuYigeng

Copy link
Copy Markdown
Contributor Author

Addressed the AI review feedback in 0f5689a41:

  1. Parser registration is now covered by a regression test asserting both --ignore-rules and --safe-mode are real argparse options, so the getattr fallback can never silently no-op.
  2. effective_ignore_rules is now resolved by a single _effective_ignore_rules() helper shared by run_oneshot and _run_agent, so the env semantics cannot drift between the two sites.
  3. The safe-mode argv path (Termux fast launch) is now parametrized across --ignore-rules, --safe-mode, and the bare path.
  4. On the coupling/doc points: the existing --ignore-rules help text and the HERMES_IGNORE_RULES / HERMES_SAFE_MODE entries in environment-variables.md already state that rules, memory, and preloaded skills are skipped; both env vars pre-exist on main (used by cli.py and tui_gateway/server.py for HERMES_IGNORE_RULES, and by plugins/MCP/hooks for HERMES_SAFE_MODE), so no new env surface is introduced.

Validation: 21 focused tests pass, ruff check clean, git diff --check clean.

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 needs-decision Awaiting maintainer decision before any implementation 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.

[Bug] hermes -z silently ignores --ignore-rules (oneshot path never reads HERMES_IGNORE_RULES)

6 participants