Hidden I/O in Agent Pull Requests: What to Trust, Revert, and Test

Agent-generated pull requests usually keep the local algorithm looking right while they change how the process talks to the world. A new HTTP client, a time.sleep, a module-level lock, or an unexpected os.environ read is not a style nit. It is a change to the failure domain.

Review that surface first. Green tests do not prove the process still has the same I/O contract.

Why this pass is not a style review

Large language models are trained to make the snippet work. Working, in that training distribution, often means reaching for the network, the clock, or a global. The resulting diff compiles. The unit tests, which the agent also wrote or extended, mock the new client. Production then inherits a timeout, a retry storm, or a hidden credential read that no reviewer was asked to approve.

This is adjacent to, not the same as, contract drift in tests or drive-by refactors. The question here is narrower. Did the agent add a side effect that moves a CPU-bound function onto the network, the filesystem, or shared mutable state?

If the answer is yes, the rest of the review is downstream of that fact.

Decision table: trust, revert, test

Classify every hunk before arguing about names.

Signal in the diff	Default action	Why
Pure function body, same signature, no new imports	Trust, then type-check	Failure domain unchanged
New requests / httpx / urllib / socket	Revert unless the ticket named an endpoint	New network failure mode
subprocess, os.system, Popen	Revert	New process boundary and injection surface
open(..., "w"), pathlib writes, shutil	Test at the filesystem boundary or revert	Durability and permission failures
time.sleep, retry loops with backoff	Revert	Latency budgets and thundering herds
Module-level Lock, singleton cache	Revert	Cross-request coupling
New os.environ / config key	Revert by default	Invented control plane
Logging of request bodies, tokens, emails	Revert	Data-handling expansion
datetime.now() / time.time() used as identity	Test or revert	Clock and uniqueness bugs
Comment-only or docstring-only	Trust if it matches the code	No runtime change
Signature change on a public function	Test callers; do not trust tests the agent added	Contract change

The table is a default, not a court. A ticket that says "call the billing API" makes the HTTP import expected. A ticket that says "fix the off-by-one in pagination" does not.

A reproducible audit over the unified diff

Do not start in the GitHub UI. Start with the patch. The script below reads git diff / git show output and prints every added line that expands I/O, clock, or process surface. It is a filter. It is not a linter for correctness.

#!/usr/bin/env python3
"""Flag added lines in a unified diff that expand I/O or shared state.

Usage:
  git diff origin/main...HEAD | python3 agent_pr_io_audit.py
"""
from __future__ import annotations

import re
import sys
from collections import defaultdict

RULES = [
    ("http_client", re.compile(r"\b(requests|httpx|urllib|aiohttp|httplib|socket)\b")),
    ("subprocess", re.compile(r"\b(subprocess|os\.system|os\.popen|Popen)\b")),
    ("fs_write", re.compile(r"\b(open\(|Path\(.*\)\.(write_text|write_bytes|touch)|shutil\.)")),
    ("sleep_retry", re.compile(r"\b(time\.sleep|asyncio\.sleep|backoff|retry)\b")),
    ("env_config", re.compile(r"\b(os\.environ|os\.getenv|getenv)\b")),
    ("shared_state", re.compile(r"\b(threading\.Lock|multiprocessing\.|lru_cache)\b")),
    ("clock", re.compile(r"\b(datetime\.now\(|time\.time\(|time\.monotonic\()")),
    ("secrets_log", re.compile(r"\b(password|secret|token|authorization|api_key)\b", re.I)),
]

FILE_RE = re.compile(r"^\+\+\+ b/(.+)$")
HUNK_RE = re.compile(r"^@@ .* @@")


def audit(lines: list[str]) -> dict[str, list[tuple[str, int, str, str]]]:
    hits: dict[str, list[tuple[str, int, str, str]]] = defaultdict(list)
    path = "<stdin>"
    line_no = 0
    for raw in lines:
        if raw.startswith("+++ b/"):
            m = FILE_RE.match(raw.rstrip("\n"))
            path = m.group(1) if m else path
            line_no = 0
            continue
        if raw.startswith("@@"):
            # Approximate added-line numbers from the + side of the hunk header.
            plus = re.search(r"\+(\\d+)", raw)
            line_no = int(plus.group(1)) - 1 if plus else line_no
            continue
        if raw.startswith("+") and not raw.startswith("+++"):
            line_no += 1
            text = raw[1:]
            for label, rx in RULES:
                if rx.search(text):
                    hits[label].append((path, line_no, label, text.rstrip()))
        elif not raw.startswith("-") and not raw.startswith("\\"):
            if not raw.startswith("diff ") and not raw.startswith("index "):
                line_no += 1
    return hits


def main() -> int:
    hits = audit(sys.stdin.readlines())
    total = 0
    for label, rows in sorted(hits.items()):
        print(f"## {label} ({len(rows)})")
        for path, n, _, text in rows:
            print(f"{path}:{n}: {text}")
            total += 1
        print()
    print(f"total_flags={total}")
    return 1 if total else 0


if __name__ == "__main__":
    raise SystemExit(main())

Run it against the agent branch:

git fetch origin
git diff origin/main...HEAD > /tmp/agent.patch
python3 agent_pr_io_audit.py < /tmp/agent.patch
echo exit:$?

Treat a non-zero class count as "stop and classify," not as "reject the PR." False positives happen. import json is not I/O. pathlib.Path used only for joins is not a write. The script over-flags on purpose so a human still decides.

If you need to drop a single invented knob without discarding the logic hunk, isolate first:

git log --oneline origin/main..HEAD
git diff -U0 origin/main...HEAD -- '*.py'
git checkout -p origin/main -- path/to/file.py   # keep only reviewed hunks

git checkout -p is slower than a blanket revert. It is also the only way to keep a pure-logic fix when the agent bundled a socket next to it.

Worked example (synthetic; not from a production repo)

The following hunk is labeled as an example. An agent was asked to "make fetch_page resilient."

# EXAMPLE — agent hunk to classify, not production code
import time
import requests

def fetch_page(page_id: str) -> dict:
    for attempt in range(5):
        try:
            resp = requests.get(
                f"https://api.internal.example/pages/{page_id}",
                timeout=None,
            )
            return resp.json()
        except Exception:
            time.sleep(2 ** attempt)
    return {}

Trust nothing in that hunk except the intent. Revert the sleep. Revert timeout=None. Revert the hardcoded URL unless the ticket named it. Revert the empty-dict fallback: it turns every transport failure into a successful miss.

If the HTTP call is actually in scope, keep I/O at the edge and make the caller inject a transport:

# EXAMPLE — proposed shape after revert
from typing import Protocol

class Transport(Protocol):
    def get_json(self, page_id: str) -> dict: ...

def fetch_page(page_id: str, transport: Transport) -> dict:
    payload = transport.get_json(page_id)
    if "id" not in payload:
        raise ValueError("page payload missing id")
    return payload

The reviewer still needs a contract test against a fake transport. The point of the pass is to force that test to exist, not to bless the algorithm.

Clocks and globals are I/O too

Agents often "fix" flaky tests by freezing nothing and calling datetime.now(). That is a hidden input. It will pass in UTC CI and fail for a developer whose laptop is in a positive offset. It will also collide if used as a unique key.

Same rule for module-level dicts used as caches. They look local. They couple requests. Under a threaded server they become races; under a forked server they become stale reads. Revert them unless the ticket named a cache and named its invalidation.

# EXAMPLE — revert this class of hunk
_CACHE: dict[str, dict] = {}

def get_user(user_id: str) -> dict:
    if user_id not in _CACHE:
        _CACHE[user_id] = db.fetch_user(user_id)
    return _CACHE[user_id]

A reviewer who only reads the function body will call this an optimization. A reviewer who starts at I/O will call it a new consistency model.

What to test once I/O is admitted

If the team keeps a new side effect, the tests must hit the real failure domain. A framework-agnostic checklist:

1. Timeout: the callee never returns.
2. Transport error: connection refused, DNS failure, TLS mismatch.
3. Non-2xx with a body, and non-2xx with an empty body.
4. Retry behavior: at-most-once versus at-least-once, and whether retries duplicate writes.
5. Auth: missing token, expired token, token present in logs.
6. Cancellation: caller drops the request; does the child I/O stop?
7. Size: empty list, one item, payload over the documented limit.

If those seven are missing, the PR is not tested. It is demonstrated on the happy path.

Proposal (unexecuted) for a transport fake:

# PROPOSAL — pytest sketch, not run in this article
import pytest

class FakeTransport:
    def __init__(self, behavior):
        self.behavior = behavior
        self.calls = 0

    def get_json(self, page_id: str) -> dict:
        self.calls += 1
        if self.behavior == "timeout":
            raise TimeoutError("synthetic")
        if self.behavior == "empty":
            return {}
        return {"id": page_id, "title": "ok"}

@pytest.mark.parametrize("behavior", ["timeout", "empty", "ok"])
def test_fetch_page_boundary(behavior):
    transport = FakeTransport(behavior)
    if behavior == "ok":
        assert fetch_page("p1", transport)["id"] == "p1"
        return
    with pytest.raises((TimeoutError, ValueError)):
        fetch_page("p1", transport)

Do not accept a unit test that patches requests.get to always return 200. That test documents the mock, not the boundary.

Second pass: classify leftover hunks

After the script, leftover hunks are logic, types, and comments. Those can be reviewed at normal speed. For a remaining diff larger than working memory, a constrained second model pass is useful. It is optional.

Disclosure: This article was prepared as part of MonkeyCode's product outreach.

MonkeyCode's free model access and free server option can host that pass: feed it the audit output plus the remaining hunks, and ask for a three-column classification (trust / revert / test) with a one-line reason each. Do not ask the model to approve the PR. Do not let it rewrite the patch in the same turn as the classification. The value is a checklist, not another generator. The audit script does not depend on that service.

Prompt shape that stays a classifier:

You are classifying a code review, not generating code.
For each hunk, output exactly: PATH:LINE | trust|revert|test | reason<=20 words.
Refuse to rewrite the patch. If I/O was already flagged by the audit, do not
mark it trust.

If the classifier and the script disagree, keep the script's flag. Models under-report sleeps wrapped in helpers.

Limitations

The script does not parse typed ASTs across languages. It will miss I/O hidden behind a project-specific client factory. It will flag comments that mention subprocess. It has no notion of whether a new HTTP call is inside an existing security boundary.

Line numbers from unified diffs are approximate when hunks mix context lines. Re-open the file before you comment on GitHub. Binary files, generated lockfiles, and vendored trees should be excluded before the pipe; otherwise token-like strings in package-lock.json will drown the signal.

git diff origin/main...HEAD -- . \
  ':!package-lock.json' ':!yarn.lock' ':!poetry.lock' ':!vendor/**' \
  | python3 agent_pr_io_audit.py

Do not use this approach as a merge gate for memory-unsafe code, for cryptography, or for anything that needs a threat model. Do not treat a clean audit as proof that the agent did not change behavior. Pure-logic regressions still exist.

Who should skip it

Skip this protocol if the PR is generated inside a sandbox that cannot perform I/O, and that sandbox is the merge environment. Skip it if the change is a data-only migration with reviewed DDL. Skip it if you cannot name a human who owns the new endpoint, file path, or environment variable.

Teams that already run a mature SAST pipeline should keep it. This pass is a human protocol for agent diffs, not a replacement for that pipeline.

Otherwise, run the filter, classify, and only then argue about names. The algorithm is the easy part of an agent PR. The new socket is the part that ships.