Conversation
zccyman
force-pushed
the
fix/25823-guardrails-hard-stop-default
branch
from
May 18, 2026 00:31
f09d8e2 to
879ef18
Compare
teknium1
reviewed
Jul 13, 2026
teknium1
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for the focused guardrail coverage. Current main still has the warning-only default, so the underlying retry-cost report is real.
Problems
- The global flip conflicts with the current documented design for interactive sessions:
website/docs/user-guide/configuration.md:1344-1362says warning-only is intentional for CLI/TUI and hard stops are an explicit unattended-deployment setting.agent/tool_guardrails.py:65-73andhermes_cli/config.py:1394-1399implement that same contract. - The diff changes that contract without updating
website/docs/user-guide/configuration.md:1344-1362orwebsite/docs/user-guide/docker.md:73-82. - The updated tests cover the dataclass default, but not the production
load_config()→agent/agent_init.py:1307-1318path or an explicitfalseopt-out.
Suggested changes
- Resolve the global-versus-platform-specific behavior decision before salvage; related PR #49189 describes the platform-aware alternative.
- If the global default is chosen, update both docs and add configuration-to-runtime coverage.
This is an automated hermes-sweeper review.
|
|
||
| warnings_enabled: bool = True | ||
| hard_stop_enabled: bool = False | ||
| hard_stop_enabled: bool = True |
Collaborator
There was a problem hiding this comment.
This changes the documented interactive-session contract globally. Current main explicitly preserves warning-only CLI/TUI behavior and requires an opt-in for unattended deployments (website/docs/user-guide/configuration.md:1344-1362); please resolve that product decision and update the corresponding docs before changing this default.
1 task
18 tasks
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
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.
Problem
tool_loop_guardrails.hard_stop_enableddefaults tofalse. Guardrails detect retry loops (same failing command, same tool failure, idempotent no-progress) and inject warning text, but models frequently ignore text guidance and continue retrying untilmax_iterations(90). A single terminal failure loop can consume 90+ API calls doing nothing useful.Fix
Change default from
false→truein two locations:ToolCallGuardrailConfig.hard_stop_enableddataclass defaulthermes_cli/config.pydefault config dictUsers who prefer unlimited retries can opt out via:
Validation
Updated 3 existing tests that assumed
hard_stop_enabled=Falseto explicitly passhard_stop_enabled=False. Updated 1 test that asserted the default value. All 13 guardrail tests pass.Closes #25823