Skip to content

fix: classify landed file mutations with diagnostics - #24929

Closed
GodsBoy wants to merge 1 commit into
NousResearch:mainfrom
GodsBoy:fix/file-mutation-verifier-lint-false-positive
Closed

fix: classify landed file mutations with diagnostics#24929
GodsBoy wants to merge 1 commit into
NousResearch:mainfrom
GodsBoy:fix/file-mutation-verifier-lint-false-positive

Conversation

@GodsBoy

@GodsBoy GodsBoy commented May 13, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes a false positive in file mutation result handling where successful write_file and patch calls could be treated as failed when their JSON result contained nested diagnostic text such as lint errors or LSP diagnostics. The change centralizes landed mutation classification so verifier footers, CLI tool status, and tool guardrails agree on the same success contract.

Related Issue

Fixes #24927

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Security fix
  • Documentation update
  • Tests (adding or improving test coverage)
  • Refactor (no behavior change)
  • New skill (bundled or hub)

Changes Made

  • Added shared file mutation result classification in agent/tool_result_classification.py.
  • Updated the turn-end file mutation verifier to clear stale failures when a diagnostic-bearing mutation result proves the write landed.
  • Updated CLI display and tool guardrail failure classification to avoid marking landed file mutations as failed because of nested diagnostics.
  • Added regression coverage for verifier recovery, display status, guardrail classification, and the shared classifier helper.

How to Test

  1. uv run --extra dev python -m pytest -q tests/run_agent/test_file_mutation_verifier.py tests/agent/test_display.py::TestCuteToolMessagePreviewLength tests/agent/test_tool_guardrails.py tests/agent/test_tool_result_classification.py
  2. uv run --extra dev python -m ruff check agent/tool_result_classification.py agent/display.py agent/tool_guardrails.py run_agent.py tests/agent/test_tool_result_classification.py tests/agent/test_display.py tests/agent/test_tool_guardrails.py tests/run_agent/test_file_mutation_verifier.py
  3. git diff --check

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes
  • I've tested on my platform: Windows 11

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior or N/A

For New Skills

N/A

Screenshots / Logs

Targeted verifier, display, guardrail, and classifier tests passed: 55 passed.
Ruff passed on changed Python files.
Whitespace check passed with git diff --check.

@GodsBoy

GodsBoy commented May 13, 2026

Copy link
Copy Markdown
Contributor Author

CI triage update:

All checks except the full test job are green on this PR:

  • Supply chain scan passed
  • Attribution check passed
  • Windows footguns passed
  • Ruff enforcement passed
  • ruff + ty diff passed
  • e2e passed
  • amd64 and arm64 builds passed
  • nix checks passed on Ubuntu and macOS

The remaining test job failed in areas unrelated to this PR. The failures are in Bedrock optional dependency and packaging checks, Dingtalk card lifecycle tests, Feishu bot identity hydration, startup plugin gating for lsp, switch-model context length preservation, local transcription optional dependencies, and KittenTTS importing numpy.

This PR only changes file mutation result classification plus focused tests for verifier, display, guardrail, and classifier behavior. None of the failed CI tests are in the changed files or related code paths.

Local verification for this PR:

  • uv run --extra dev python -m pytest -q tests/run_agent/test_file_mutation_verifier.py tests/agent/test_display.py::TestCuteToolMessagePreviewLength tests/agent/test_tool_guardrails.py tests/agent/test_tool_result_classification.py
  • uv run --extra dev python -m ruff check agent/tool_result_classification.py agent/display.py agent/tool_guardrails.py run_agent.py tests/agent/test_tool_result_classification.py tests/agent/test_display.py tests/agent/test_tool_guardrails.py tests/run_agent/test_file_mutation_verifier.py
  • git diff --check

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint tool/file File tools (read, write, patch, search) labels May 13, 2026
@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Merged via #25011. Your commit is preserved as-is in git history (da0ddbf88 fix: classify landed file mutations with diagnostics) with original authorship. We added a follow-up refactor on top to deduplicate _FILE_MUTATING_TOOLS (importing the canonical FILE_MUTATING_TOOL_NAMES you exported from agent/tool_result_classification.py into run_agent.py instead of redefining it). Thanks for the thorough work on this — the shared-helper design was the key win.

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 P2 Medium — degraded but workaround exists tool/file File tools (read, write, patch, search) type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix file mutation verifier false positive for diagnostic-bearing writes

3 participants