fix(agent): clarify verifier after approved config set - #53093
Open
MarcvsTvllivs wants to merge 1 commit into
Open
MarcvsTvllivs wants to merge 1 commit into
MarcvsTvllivs wants to merge 1 commit into
Conversation
MarcvsTvllivs
force-pushed
the
fix/file-mutation-verifier-config-set
branch
from
June 26, 2026 14:48
4e98d13 to
61ebbf7
Compare
teknium1
reviewed
Jul 15, 2026
teknium1
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for identifying a real stale-verifier path: current run_agent.py:2829-2855 records only file-tool outcomes, while agent/turn_finalizer.py:295-301 still renders a remaining failure.
Problems
- The added matcher at
run_agent.py:2603accepts;/|chained shell commands and trusts any output line matchingSet … in …config.yaml. A command can print that line and then successfully runhermes config set OPENROUTER_API_KEY value; currenthermes_cli/config.py:8134-8136routes that real command to.env, but the PR would suppress the protectedconfig.yamlfailure.
Suggested changes
- Restrict recovery to a parsed, sole sanctioned
hermes config setinvocation, rather than a substring in a shell command, and add a regression for forged/prefixed output plus an.env-routed key. - Add a temp-
HERMES_HOMEintegration test for the actual config command and end-of-turn footer path.
Automated hermes-sweeper review.
| This intentionally recognizes only the narrow, approved path that | ||
| resolves protected config.yaml patch failures: `hermes config set ...` | ||
| with a zero exit code and the CLI's success output naming config.yaml. | ||
| It does not try to infer arbitrary file changes from terminal output. |
Collaborator
There was a problem hiding this comment.
This accepts ;/| command chains and trusts any matching output line. For example, a command can print Set x in ~/.hermes/config.yaml and then run hermes config set OPENROUTER_API_KEY value; that real command writes .env (hermes_cli/config.py:8134-8136), but this matcher would mark the blocked config.yaml edit as recovered. Require a sole parsed hermes config set invocation and add this regression case.
MarcvsTvllivs
force-pushed
the
fix/file-mutation-verifier-config-set
branch
from
August 2, 2026 20:14
61ebbf7 to
6b2f9b1
Compare
This branch has not been deployed
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.
What does this PR do?
Clarifies the per-turn file-mutation verifier when a protected
config.yamldirect edit is blocked but the same turn successfully applies the config change through the approvedhermes config set ...path.Today, if
patch/write_fileis refused for a protected Hermes config file and the agent then recovers by runninghermes config set ..., the final verifier footer can still imply the config file was not modified. This PR preserves both facts instead:The recovery is intentionally narrow. It only recognizes successful terminal calls whose command is
hermes config set ..., whose JSON result hasexit_code == 0, and whose CLI output names the sameconfig.yamlpath. It does not infer arbitrary terminal-side file edits.Known limitation: recovery is matched at the
config.yamlpath level, not by proving content/key-level equivalence. That keeps the patch small and targeted to the common flow: blocked direct config edit → retry the same change viahermes config set. The footer still includes verification guidance where unresolved failures remain.Related Issue
No single issue filed for this exact
hermes config setrecovery case.Related prior art / adjacent PRs found during duplicate search:
config.yamlverifier footer security issue and path neutralization.This PR is intentionally narrower than the broader recovery PRs: it handles only the sanctioned
hermes config setpath and leaves arbitrary terminal recovery to separate work.Type of Change
Changes Made
agent/turn_context.py_turn_superseded_file_mutationsper turn alongside existing verifier state.agent/turn_finalizer.pyrun_agent.pyterminalcalls that runhermes config set ...and report aconfig.yamlpath.config.yamlmutation from failed state to a superseded/recovered bucket.~vs absolute path matching viaos.path.expanduser()and slash normalization.tests/run_agent/test_file_mutation_verifier.py~/.hermes/config.yamlvs absolute path matching.How to Test
Reproduce the old behavior conceptually:
~/.hermes/config.yamlis denied.hermes config set ....Verify the new behavior with focused tests:
Result:
Verify syntax and whitespace:
Check Windows/path footguns on the changed files:
Result:
Full-suite status:
Result: full suite was attempted. The default per-file timeout hit
tests/gateway/test_email.py; that file passes when rerun directly with a longer wall clock (87 passed, 2 warnings in 243.94s). With--file-timeout 300, the suite reached two unrelated failing files:Both failures reproduce on a fresh detached
origin/mainworktree at8ab7246c4:These full-suite blockers appear to be current baseline failures, not introduced by this branch.
Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — N/Acli-config.yaml.exampleif I added/changed config keys — N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — N/AScreenshots / Logs
Focused verification:
Windows-footgun check:
Full-suite attempt: