Default on Exception Is Not a Fix: Auditing Failure Paths in Agent PRs

Agent-generated pull requests usually look finished on the success path. The failure path is where they quietly change product behavior. Before merge, classify every new except, catch, retry, timeout, and default return as trust, revert, or test. If the handler returns a sentinel, logs and continues, or retries without a budget, treat it as a behavior change, not a cleanup.

This is a review protocol, not a vibe check. It runs on a unified diff plus a short classifier. No production metrics are claimed below. The examples are labeled as a proposed workflow you can run locally.

Why failure paths concentrate review risk

Coding agents are rewarded for green tests and a compact diff. Swallowing an error is a cheap way to get both. A catch-all that returns {} will often satisfy a unit test that only asserts isinstance(result, dict).

The public contract still moved. Callers that used to see ConnectionError now see empty JSON. On-call playbooks that grepped a specific exception name go dark. That is a product change disguised as robustness.

Three patterns show up repeatedly in agent diffs:

• Sentinel conversion — except Exception: return None / return [] / return {}
• Log-and-continue — the exception is formatted, then execution proceeds as success
• Unbounded rescue — retry loops, time.sleep, or fallback HTTP calls with no deadline

Happy-path additions can be trusted more often. Failure-path additions need a named contract.

Trust, revert, or test

Use the table as a first pass. It is a decision aid, not a merge policy.

Diff signal	Default action	Why
Narrow except FileNotFoundError around a documented optional file	Trust after reading the call site	Failure mode already existed
except Exception, bare except, or catch (Exception e)	Revert	Hides programmer errors and contract bugs
Return of None, [], {}, 0, or HTTP 200 on failure	Revert unless the public docs already promised that sentinel	Changes caller branching
Log line without re-raise, and no new metric/counter	Test the negative path, then decide	Observability may have been the whole point
Retry / sleep / fallback HTTP added in the same hunk as the feature	Revert the retry; keep the feature if isolated	Retries change latency and idempotency
Exception type renamed (ValueError → RuntimeError)	Test every caller and the API schema	Tests that assert raises(Exception) will not catch this
timeout= added to an existing client call	Test with a fake clock or a stubbed delay	Timeout values are behavior
pytest.raises removed, or assertion replaced with or True	Revert	Failure contract was deleted

If two rows fire on the same hunk, take the stricter action. Revert beats test. Test beats trust.

Artifact: classify failure hunks from a diff

Proposed workflow. Run it against the agent branch before human review, not instead of it.

git fetch origin
git diff origin/main...HEAD > /tmp/agent.pr.diff
python3 failure_path_review.py /tmp/agent.pr.diff

#!/usr/bin/env python3
"""failure_path_review.py — classify failure-path hunks in a unified diff.

Proposed local review aid. It does not execute the patch and does not
prove correctness. Exit code 2 means at least one REVERT row fired.
"""
from __future__ import annotations

import re
import sys
from dataclasses import dataclass
from pathlib import Path

REVERT_PATTERNS = [
    (r"except\s+Exception\b", "broad-except"),
    (r"except\s*:", "bare-except"),
    (r"catch\s*\(\s*Exception\b", "broad-catch"),
    (r"except\b[^:]*:\s*(return|pass)\b", "swallow-return"),
    (r"return\s+(\[\]|\{\}|None|0)\s*$", "sentinel-return"),
    (r"time\.sleep\s*\(", "sleep-retry"),
    (r"for\s+_\s+in\s+range\s*\(\s*\d+", "counted-retry"),
]

TEST_PATTERNS = [
    (r"timeout\s*=", "timeout-kw"),
    (r"logger\.(debug|info|warning|error|exception)", "log-on-failure"),
    (r"raise\s+\w+", "reraise-or-wrap"),
    (r"pytest\.raises|assertRaises", "negative-test"),
]

TRUST_PATTERNS = [
    (r"except\s+(FileNotFoundError|json\.JSONDecodeError|KeyError)\b", "narrow-except"),
]

ADDED = re.compile(r"^\+(?!\+)")
HUNK_FILE = re.compile(r"^\+\+\+\s+b/(.+)$")


@dataclass
class Finding:
    path: str
    line: str
    action: str
    rule: str


def classify_added_line(path: str, line: str) -> Finding | None:
    body = line[1:].strip()
    if not body or body.startswith("#"):
        return None
    for pat, rule in REVERT_PATTERNS:
        if re.search(pat, body):
            return Finding(path, body, "REVERT", rule)
    for pat, rule in TEST_PATTERNS:
        if re.search(pat, body):
            return Finding(path, body, "TEST", rule)
    for pat, rule in TRUST_PATTERNS:
        if re.search(pat, body):
            return Finding(path, body, "TRUST", rule)
    return None


def scan(diff_text: str) -> list[Finding]:
    current = "<unknown>"
    out: list[Finding] = []
    for raw in diff_text.splitlines():
        m = HUNK_FILE.match(raw)
        if m:
            current = m.group(1)
            continue
        if not ADDED.match(raw):
            continue
        found = classify_added_line(current, raw)
        if found:
            out.append(found)
    return out


def main(argv: list[str]) -> int:
    if len(argv) != 2:
        print("usage: failure_path_review.py <unified.diff>", file=sys.stderr)
        return 2
    text = Path(argv[1]).read_text(encoding="utf-8", errors="replace")
    findings = scan(text)
    if not findings:
        print("No failure-path signals in added lines.")
        return 0
    revert = 0
    for f in findings:
        print(f"{f.action:6}  {f.rule:16}  {f.path}: {f.line}")
        if f.action == "REVERT":
            revert += 1
    print(f"-- {len(findings)} signal(s), {revert} revert")
    return 2 if revert else 0


if __name__ == "__main__":
    raise SystemExit(main(sys.argv))

The classifier is lexical on added lines. That is intentional. Reviewers need a noisy, cheap net before they read the patch. A later pass can ignore comments and string literals; this pass should not.

Worked example (synthetic patch)

Proposed example. This is not a report from a private repo.

--- a/billing/fetch.py
+++ b/billing/fetch.py
@@ -18,7 +18,16 @@ def load_invoice(client, invoice_id):
-    return client.get(f"/invoices/{invoice_id}").json()
+    try:
+        return client.get(f"/invoices/{invoice_id}", timeout=2).json()
+    except Exception:
+        logger.warning("invoice miss %s", invoice_id)
+        return {}

Run the classifier:

REVERT  broad-except      billing/fetch.py: except Exception:
REVERT  sentinel-return   billing/fetch.py: return {}
TEST    timeout-kw        billing/fetch.py: return client.get(f"/invoices/{invoice_id}", timeout=2).json()
TEST    log-on-failure    billing/fetch.py: logger.warning("invoice miss %s", invoice_id)
-- 4 signal(s), 2 revert

Split the hunk before any other discussion. Keep timeout=2 only if the HTTP client previously had no deadline and the service SLO allows a 2-second cut. Revert except Exception and return {}. Those two lines convert every outage, 4xx, 5xx, and JSON parse error into an empty invoice object.

A replacement that preserves the contract:

def load_invoice(client, invoice_id):
    response = client.get(f"/invoices/{invoice_id}", timeout=2)
    response.raise_for_status()
    return response.json()

If a missing invoice is a documented case, name that case:

class InvoiceNotFound(LookupError):
    pass

def load_invoice(client, invoice_id):
    response = client.get(f"/invoices/{invoice_id}", timeout=2)
    if response.status_code == 404:
        raise InvoiceNotFound(invoice_id)
    response.raise_for_status()
    return response.json()

The second version is longer. It is also reviewable. Agents optimize for the first version unless the review comment forbids sentinel conversion.

Tests the agent will not add unless asked

Failure-path review is incomplete without a negative test that the agent did not write. Minimum set for the invoice example:

1. Transport failure — stub client.get to raise ConnectionError; expect the same exception type (or the documented wrapper), not {}.
2. Not found — stub HTTP 404; expect InvoiceNotFound, not HTTP 200 semantics.
3. Timeout — stub a delay past 2 seconds; expect the client timeout error.
4. Parse failure — stub HTTP 200 with a body of "nope"; expect JSONDecodeError or a named parse error.

Sketch with a fake client:

class FakeResp:
    def __init__(self, status_code, payload, json_error=False):
        self.status_code = status_code
        self._payload = payload
        self._json_error = json_error

    def raise_for_status(self):
        if self.status_code >= 400:
            raise RuntimeError(f"http {self.status_code}")

    def json(self):
        if self._json_error:
            raise ValueError("invalid json")
        return self._payload


def test_404_does_not_become_empty_dict():
    client = type("C", (), {"get": staticmethod(lambda *a, **k: FakeResp(404, {}))})()
    try:
        load_invoice(client, "inv_1")
    except (InvoiceNotFound, RuntimeError):
        return
    raise AssertionError("404 was converted into a successful empty invoice")

If the agent PR already contains a test file, read the assertions before the implementation. A test that only checks assert result == {} after a patched exception is evidence that the failure contract was rewritten to match the handler.

Review comment template

Paste this under the hunk, then fill the blanks. Short comments get acted on; essays do not.

Failure-path audit:
- Signal: [broad-except | sentinel-return | retry | timeout | wrap]
- Action: [REVERT | TEST | TRUST]
- Contract before this PR: [exception type / status code / empty]
- Contract after this PR:  [exception type / status code / empty]
- Required test: [transport | 4xx | timeout | parse]
- Retry/sleep allowed?: no, unless idempotency key is cited in this thread

Require the agent (or the author) to answer the contract-before / contract-after lines in the same thread. If those two lines are identical, the handler may stay. If they differ, the PR description must say so in one sentence.

Where a disposable agent environment fits

The audit is local and tool-agnostic. It only needs git diff and Python 3.

If the PR itself was produced by an agent loop, keep that loop off the laptop that holds production credentials. Disclosure: This article was prepared as part of MonkeyCode's product outreach. MonkeyCode offers free model access and a free server option, which is enough to generate a candidate patch in an isolated workspace and then export the diff into failure_path_review.py. The classifier does not call the product and does not depend on it.

Do not paste secrets, customer payloads, or production URLs into that workspace. The review still happens on the exported diff.

Limitations

• Lexical matching misses except (OSError, TimeoutError) as exc: that is actually correct, and it flags return {} in non-error helpers.
• It does not understand Go if err != nil, Rust ?, or Java checked exceptions unless you add patterns.
• It cannot see deleted raise statements that sit on removed lines. Pair it with git diff -U5 and a scan of lines starting with -.
• Timeout values, retry counts, and sleep durations need domain SLOs. The script will not know whether timeout=2 is safe.
• A clean classifier run is not a security review. It will not catch SSRF, path traversal, or prompt injection in the same PR.

Who should not use this as the only gate

Skip a merge decision based solely on this script when:

• the change is a public API version bump where error codes are the release
• the codebase already uses a Result/Either type and almost never raises
• the agent was asked to add retries for a known flaky vendor, and idempotency is documented
• reviewers cannot run the four negative tests above because the dependency has no fake

In those cases, write the failure contract in the PR body first, then generate code. Auditing after the fact is for the default agent PR, where nobody wrote that contract.

Merge rule

Keep the try. Revert an except that invents success. Test every timeout and every renamed error. If the classifier prints REVERT, the human review starts there, not at the README the agent also rewrote.