Skip to content

fix(agent): suppress stale mutation verifier warnings - #65584

Open
aftership-bot-vlt-platform wants to merge 1 commit into
NousResearch:mainfrom
aftership-bot-vlt-platform:agent/fix-file-mutation-verifier-external-change
Open

aftership-bot-vlt-platform wants to merge 1 commit into
NousResearch:mainfrom
aftership-bot-vlt-platform:agent/fix-file-mutation-verifier-external-change

Conversation

@aftership-bot-vlt-platform

Copy link
Copy Markdown

Summary

  • track a local stat fingerprint when a file mutation fails
  • suppress the verifier footer when the target is later changed by terminal/CLI or another external path
  • retain warnings for unchanged or non-local/unverifiable targets

Reproduction

A protected patch to ~/.hermes/config.yaml can fail, followed by a successful hermes config set command. The config is modified, but the verifier previously still claimed the file was not changed because it only observed file-tool success.

Verification

  • uv run --extra dev pytest tests/run_agent/test_file_mutation_verifier.py -q -o "addopts=" — 44 passed
  • turn-finalizer regression set — 28 passed
  • combined post-commit regression set — 72 passed
  • uv run --extra dev ruff check run_agent.py agent/turn_finalizer.py tests/run_agent/test_file_mutation_verifier.py — passed
  • exact external-mutation reproduction — passed
  • git diff --check — passed

Risk

Stat-based comparison intentionally avoids hashing large files. Remote backends fail closed and retain the warning.

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint labels Jul 16, 2026
@alt-glitch

Copy link
Copy Markdown

This was generated by AI during triage.

Related: #53165 addresses the same stale mutation-verifier warning with backend-aware content fingerprints. This PR uses a smaller local stat-fingerprint approach; maintainers should choose the desired verifier semantics.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for isolating the stale-verifier case and adding local/remote regression coverage. The current-main premise is valid: run_agent.py:2910-2916 tracks only write_file/patch, while agent/turn_finalizer.py:304-310 renders any remaining failure, so a later terminal recovery cannot currently clear the warning.

Problems

  • run_agent.py:2926 fingerprints st_mode and st_ctime_ns. A metadata-only operation such as chmod changes those fields without changing content, yet run_agent.py:2994-2998 would clear the warning. The footer's assertion that the requested mutation did not land can therefore be hidden by an unrelated metadata change.
  • The linked #53165 records the stronger recovery contract: backend-aware content identity plus a successful foreground fallback. This PR instead treats any observable local change as recovery; the member comment on this PR explicitly asks maintainers to choose the semantics.

Suggested changes

  • Add a metadata-only regression (chmod/chown) and retain the footer unless contents changed.
  • Align the chosen implementation with an explicit recovery contract; if fallback causality is required, capture the resolved backend target and compare it around the successful foreground fallback.

Automated hermes-sweeper review.

Comment thread run_agent.py
# message shouldn't silently overwrite the original. Still
# refresh the filesystem fingerprint so a later unrelated
# external edit cannot hide the most recent failed attempt.
fingerprint = self._get_local_file_mutation_fingerprint(path)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This fingerprint includes st_mode and st_ctime_ns, so a metadata-only chmod/chown after a failed patch changes the fingerprint and clears the footer even though file contents were not recovered. Please require a content change (and add a metadata-only regression) before suppressing the warning.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 18, 2026

This branch has not been deployed

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

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have 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 sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants