fix(agent): prevent mutation verifier false positives from lint diagnostics

[zccyman]

Summary

Fixes #24927

Successful file mutations (write_file, patch) were incorrectly reported as failures when their tool results included nested lint or LSP diagnostics containing "error" or "failed" keys.

Problem

The generic heuristic in _detect_tool_failure() and classify_tool_failure() searches the entire result string for '"error"' or '"failed"' substrings. This causes false positives when:

• write_file returns {"bytes_written": 100, "lint": {"errors": [{"message": "unused variable", "severity": "error"}]}} — the nested "error" in lint diagnostics triggers the heuristic
• patch returns {"success": true, "diagnostics": {"error": "type mismatch"}} — the nested "error" in LSP diagnostics triggers the heuristic

Root Cause

Both _detect_tool_failure() in agent/display.py and its mirror classify_tool_failure() in agent/tool_guardrails.py apply a generic substring heuristic that doesn't account for file-mutation tools returning structured diagnostic payloads alongside success markers.

Fix

Added an early-exit check for write_file and patch tools before the generic heuristic:

• write_file: if bytes_written is present and no top-level error → return success
• patch: if success is True → return success regardless of diagnostics

Genuine failures (top-level error key, or no success markers) still fall through to the heuristic correctly.

Testing

Added 12 regression tests across both files:

• TestDetectToolFailure in test_display.py (8 tests):

  • write_file with bytes_written → success
  • write_file with lint diagnostics → success
  • write_file with top-level error → failure
  • patch with success=true → success
  • patch without success → failure
  • None result → success
  • terminal non-zero exit → failure
  • generic tools with error → failure

• TestClassifyToolFailureMutationFalsePositive in test_tool_guardrails.py (4 tests):

  • Mirror tests for the guardrail classifier

All 53 tests pass (41 existing + 12 new), confirming zero regression.

Files Changed

• agent/display.py — early-exit for mutation tools in _detect_tool_failure()
• agent/tool_guardrails.py — matching early-exit in classify_tool_failure()
• tests/agent/test_display.py — 8 new tests
• tests/agent/test_tool_guardrails.py — 4 new tests

[alt-glitch]

Duplicate of #24929 — both fix #24927 (mutation verifier false positives from lint diagnostics). #24929 has broader scope: extracts shared tool_result_classification.py module and also patches run_agent.py's _record_file_mutation_result(), covering three call sites vs two.

[kshitijk4poor]

Closing as duplicate of #24929 (also flagged earlier in this thread by @alt-glitch).

#24929 was selected for the salvage in #25011 because:

1. Broader coverage — extracts a shared agent/tool_result_classification.py helper and patches three callsites: agent/display.py::_detect_tool_failure(), agent/tool_guardrails.py::classify_tool_failure(), and run_agent.py::_record_file_mutation_result(). This PR patches two (display + guardrails). The verifier in run_agent.py upstream of those classifiers wasn't covered.

2. Correctness on the empty-write edge case — this PR uses data.get("bytes_written") (truthiness check), which misclassifies a write_file(path, "") with lint diagnostics as a failure. bytes_written=0 is falsy → falls through to the substring heuristic → "\"error\"" in the lint payload triggers [error]. Verified empirically on this branch:

   from agent.display import _detect_tool_failure
   import json
   result = json.dumps({"bytes_written": 0, "lint": {"errors": [{"severity": "error", "message": "x"}]}})
   _detect_tool_failure("write_file", result)
   # → (True, ' [error]')   ← bug

   fix: classify landed file mutations with diagnostics #24929 uses "bytes_written" in data (presence check) which handles the 0-byte case correctly.

Thanks for the contribution and the fast turnaround on #24927 — the underlying bug is now being addressed via #25011.