Skip to content

fix(cli): preserve one-shot isolation flags - #51797

Open
hzhaoy wants to merge 4 commits into
NousResearch:mainfrom
hzhaoy:codex/fix-oneshot-ignore-rules
Open

fix(cli): preserve one-shot isolation flags#51797
hzhaoy wants to merge 4 commits into
NousResearch:mainfrom
hzhaoy:codex/fix-oneshot-ignore-rules

Conversation

@hzhaoy

@hzhaoy hzhaoy commented Jun 24, 2026

Copy link
Copy Markdown

Summary

  • Apply isolation env setup before chat and top-level one-shot startup discovery, while keeping _apply_safe_mode(args) as the safe-mode helper.
  • Make hermes_cli.config.load_config() honor HERMES_IGNORE_USER_CONFIG=1, skipping the user config.yaml while preserving built-in defaults and managed-scope config.
  • Thread ignore_rules into the one-shot runner so AIAgent skips context files and persistent memory for --ignore-rules / --safe-mode.
  • Preserve current -z --usage-file support while adding the new isolation plumbing.
  • Add regression coverage for top-level --safe-mode -z, direct one-shot ignore_rules, config-loader user-config isolation, and startup env ordering.

Root Cause

Top-level -z bypassed the cmd_chat path where isolation environment variables were set. The one-shot runner also constructed AIAgent without skip_context_files / skip_memory, so AGENTS.md could still be loaded even when the user requested --ignore-rules or --safe-mode.

After #45300, another related gap remained: HERMES_IGNORE_USER_CONFIG was honored by cli.load_cli_config(), the interactive cli.py loader path, but not by the shared hermes_cli.config.load_config() loader used by startup discovery, MCP, shell hooks, and other config consumers. This PR moves that boundary into load_config() and uses a cache signature sentinel so normal and isolated loads do not reuse each other's cached config.

Rebase Notes

  • Merged current origin/main (240afd0b70a016ba17568d597e0f2c32f94f4cfd) and resolved conflicts.
  • Preserved main's current one-shot hard-exit cleanup wrapper, stateless-channel declaration, explicit agent/session cleanup, and requested_provider wiring.
  • Kept _apply_safe_mode(args) intact.
  • Collapsed the separate ignore-user-config / ignore-rules env helpers into _apply_isolation_env(args) so startup call sites have a single ordering-sensitive helper.
  • Kept usage_file in the run_oneshot() signature and dispatch calls.

Test Plan

  • scripts/run_tests.sh tests/hermes_cli/test_tui_resume_flow.py tests/hermes_cli/test_safe_mode.py tests/hermes_cli/test_ignore_user_config_flags.py tests/hermes_cli/test_config_env_expansion.py tests/hermes_cli/test_managed_scope_config.py tests/hermes_cli/test_managed_scope_regression.py tests/hermes_cli/test_mcp_startup.py (37 passed)
  • venv/bin/python -m ruff check hermes_cli/main.py hermes_cli/oneshot.py hermes_cli/config.py tests/hermes_cli/test_tui_resume_flow.py tests/hermes_cli/test_safe_mode.py tests/hermes_cli/test_ignore_user_config_flags.py
  • git diff --cached --check -- hermes_cli/main.py hermes_cli/oneshot.py hermes_cli/config.py tests/hermes_cli/test_tui_resume_flow.py tests/hermes_cli/test_safe_mode.py tests/hermes_cli/test_ignore_user_config_flags.py

@hzhaoy
hzhaoy force-pushed the codex/fix-oneshot-ignore-rules branch from f7ecd5c to 15cd7f5 Compare June 24, 2026 09:15
@hzhaoy
hzhaoy marked this pull request as ready for review June 24, 2026 09:17
@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard area/config Config system, migrations, profiles P2 Medium — degraded but workaround exists labels Jun 24, 2026
@hzhaoy
hzhaoy force-pushed the codex/fix-oneshot-ignore-rules branch 3 times, most recently from ec1ffb3 to 95e674a Compare July 8, 2026 06:52
Top-level one-shot mode bypasses the chat command path, so safe-mode, ignore-user-config, and ignore-rules must be applied before plugin, MCP, shell-hook, and config startup side effects. Honor HERMES_IGNORE_USER_CONFIG in load_config() itself so one-shot and chat share the same configuration boundary, and keep ignore-rules threaded into AIAgent so context files and memory stay disabled.

Constraint: Top-level -z intentionally bypasses cli.py for clean stdout output

Constraint: --safe-mode and --ignore-user-config must affect startup discovery before load_config() consumers run

Rejected: Handle user-config isolation only inside oneshot | load_config() is the shared boundary used by MCP, shell hooks, plugins, and startup config reads

Rejected: Split ignore-user-config and ignore-rules env helpers | one startup helper better preserves ordering while keeping _apply_safe_mode(args) intact

Confidence: high

Scope-risk: moderate

Directive: Keep top-level oneshot, Termux fast-path, and chat startup isolation env setup in sync

Tested: scripts/run_tests.sh tests/hermes_cli/test_ignore_user_config_flags.py tests/hermes_cli/test_safe_mode.py tests/hermes_cli/test_tui_resume_flow.py tests/hermes_cli/test_config_env_expansion.py tests/hermes_cli/test_managed_scope_config.py tests/hermes_cli/test_managed_scope_regression.py tests/hermes_cli/test_mcp_startup.py

Tested: venv/bin/python -m ruff check hermes_cli/main.py hermes_cli/config.py hermes_cli/oneshot.py tests/hermes_cli/test_ignore_user_config_flags.py tests/hermes_cli/test_safe_mode.py tests/hermes_cli/test_tui_resume_flow.py

Tested: git diff --check

Not-tested: Full scripts/run_tests.sh pass after the final load_config() adjustment
@hzhaoy
hzhaoy force-pushed the codex/fix-oneshot-ignore-rules branch from 95e674a to 0cdd6c7 Compare July 8, 2026 07:36
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused isolation fix. Static review confirms the premise remains present on current main: top-level one-shot dispatch does not forward isolation state (hermes_cli/main.py:14737), and one-shot agent construction omits skip_context_files / skip_memory (hermes_cli/oneshot.py:393). The shared loader also still merges an existing user config unconditionally (hermes_cli/config.py:6953).

The PR's changes target those exact gaps, including separate isolated config-cache state, without adding model-tool surface or altering conversation context mid-session.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 15, 2026

@GottZ GottZ left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This was generated by AI during triage.

Summary

Two open PRs address the same one-shot isolation gap: user config and rules/memory isolation flags not reaching the embedded agent when invoked via top-level -z/--safe-mode. #51797 (+308/-22) fixes the gap at all three levels named in the root cause — top-level dispatch (main.py), config loader (config.py), and oneshot agent construction (oneshot.py). #73005 (+216/-5) only touches the config-loader and oneshot layers, leaving the top-level dispatch chokepoint untouched.

Related pull requests

  • #51797 related — (+308/-22) — keep_open (contributor teknium1, 7327 commits): diff adds _apply_isolation_env() as a chokepoint called from both cmd_chat and _prepare_agent_startup, so top-level -z/--safe-mode sets isolation env vars before plugin/tool discovery; also adds a sentinel cache signature in load_config() for HERMES_IGNORE_USER_CONFIG and threads an explicit ignore_rules parameter through run_oneshot/_run_agent into AIAgent(skip_context_files=, skip_memory=). Reviewer's static check against current main confirmed all three cited gaps (main.py:14737, oneshot.py:393, config.py:6953) are real and are exactly what this diff closes.
  • #73005 duplicate — (+216/-5) — no review verdict recorded: diff introduces a shared should_ignore_user_config() helper used by cli.py's load_cli_config() and by _load_config_impl (setting user_sig=None instead of a sentinel signature), and has oneshot._run_agent read HERMES_IGNORE_RULES directly from the env rather than via a passed parameter. It does not modify main.py, so it does not fix the top-level dispatch gap that #51797's reviewer confirmed is still present on current main — meaning -z invocations that never set the env vars in the first place would still leak user config/rules under this PR alone.

Duplicates

#51797 and #73005 both fix the same root cause (isolation flags not reaching load_config()/AIAgent in the one-shot -z path) with overlapping changes to hermes_cli/config.py and hermes_cli/oneshot.py, but #73005 lacks the main.py top-level dispatch fix that #51797 includes.

Suggested consolidation

merge #51797 — the keep_open review from teknium1 (contributor, 7327 commits) verified against current main that its diff closes all three root-cause gaps, including the top-level dispatch chokepoint in main.py that #73005 does not touch. #73005 can be closed as superseded by #51797: it duplicates the config.py/oneshot.py portion of the fix but is incomplete without the main.py change, so merging it alone would not fully resolve the reported issue.

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
    subgraph Dup51797 ["PRs duplicating each other"]
        P51797["PR #51797 (open)"]
        P73005["PR #73005 (open)"]
    end
    class P51797 open
    class P73005 open
    class P51797 target
    click P51797 "https://github.com/NousResearch/hermes-agent/pull/51797"
    click P73005 "https://github.com/NousResearch/hermes-agent/pull/73005"
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 or no verify verdict yet (state tag in the node label).

Cross-PR triage: Reviewed 2 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 30 kB of PR diffs, 4 kB of issue/PR text, <1 kB of discussion (1 comments), 1 verify verdict. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

hzhaoy added 2 commits July 30, 2026 08:57
Resolve conflicts by preserving current one-shot cleanup/startup wrappers and reapplying ignore-rules/user-config propagation plus regression coverage.
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-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) 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.

4 participants