Merge the Logger, Revert the Payload: Reviewing Agent PRs That Record Prompts

Merge the logger. Revert the payload. Prove redaction with an offline test before any model call.

Agent pull requests often add tracing in the same diff as a new tool. The logger name looks harmless. The attributes do not. A span that stores messages, tool_args, or Authorization is an export, not a debug aid. Treat that export as the change under review.

Public DEV discussion in early October 2026 clusters around agent experiments and model behavior. Those posts are topic signals, not evidence for this review. The rule below does not depend on any of them. It depends on what the diff writes to disk, to a collector, or to a third-party endpoint.

This note uses a synthetic pull request. Nothing here is a measured incident, a benchmark, or a claim about a named production system.

The diff under review

The proposed change does four things:

1. It wraps tool execution in an OpenTelemetry-style span helper.
2. It attaches the full chat messages array and raw tool arguments as span attributes.
3. It adds GET /debug/last-trace so a reviewer can “see what the model saw.”
4. It points a fixture at a shared model server so the new path “stays realistic.”

The first item can be a real improvement. Items 2 through 4 change the trust boundary. Review them separately, even when they arrive in one commit.

A shortened form of the risky helper, labeled as a proposal and not as shipped code:

# Synthetic bad pattern. Do not copy into a service.
def run_tool(span, tool_name, tool_args, messages, headers):
    span.set_attribute("tool.name", tool_name)
    span.set_attribute("tool.args", tool_args)          # may include tokens
    span.set_attribute("llm.messages", messages)        # user content export
    span.set_attribute("http.authorization", headers.get("Authorization"))
    return tool_registry[tool_name](**tool_args)

Short attributes such as tool name, status, latency, and token counts are reviewable. Unbounded payloads are not.

What to trust

Trust only what you can check on the branch without a network call.

• The span helper is pure enough to unit test: given a dict, it returns attributes, and it does not perform I/O itself.
• Attribute keys are a fixed allowlist, not **payload spread into the span.
• String values have an explicit max length, and over-limit values are dropped or hashed, not truncated into a still-readable secret.
• The debug route is absent, or it is gated by an admin auth check that already exists in this codebase and is covered by a test.
• Tests use a fake transport. They do not require DNS, a vendor key, or a shared server.

A green CI run is not evidence here. If the new test calls a live endpoint, the green result only proves that the endpoint answered. It does not prove that logs are safe.

What to revert

Revert these even if the rest of the PR is useful:

• Default-on capture of messages, prompt, tool_args, tool_result, request bodies, or response bodies.
• Header attributes, especially Authorization, Cookie, Set-Cookie, and x-api-key.
• A debug route that returns the last prompt, the last tool payload, or a trace ID mapped to raw content.
• Fixture or CI configuration that calls a free or shared model server to assert behavior.
• A “temporary” sample rate of 1.0 on a path that still records content. Sampling does not fix an export. It only reduces how often the export happens.

Keep the logger skeleton if the allowlist is sound. Revert the payload attachment in the same review. Do not leave a TODO that re-enables full capture behind an environment flag defaulting to true.

What to prove

Prove three claims before merge: sensitive keys never become attributes, nested encodings do not sneak through a shallow check, and the test fails closed when the helper changes.

Offline redaction check

The following is an unexecuted review artifact. It is a proposal for the PR branch, not a result from a run on 8 October 2026.

import json

DENY_KEYS = {
    "messages", "prompt", "tool_args", "tool_result",
    "authorization", "api_key", "cookie", "set-cookie",
}

def flatten_keys(value, prefix=""):
    keys = set()
    if isinstance(value, dict):
        for key, child in value.items():
            path = f"{prefix}.{key}" if prefix else str(key)
            keys.add(str(key).lower())
            keys |= flatten_keys(child, path)
    elif isinstance(value, list):
        for child in value:
            keys |= flatten_keys(child, prefix)
    elif isinstance(value, str):
        stripped = value.strip()
        if stripped.startswith("{") or stripped.startswith("["):
            try:
                keys |= flatten_keys(json.loads(stripped), prefix)
            except json.JSONDecodeError:
                pass
    return keys

def assert_span_safe(attributes: dict) -> None:
    found = flatten_keys(attributes) & DENY_KEYS
    if found:
        raise AssertionError(f"span exports sensitive keys: {sorted(found)}")

Add one case per deny key, plus one case where the payload is a JSON string inside a “safe” field such as note. A shallow key in attributes check misses that case. This helper still misses base64 and homoglyph keys. Say that in the test module docstring so the next reviewer does not treat the denylist as complete.

Commands for the review

Run these from the PR branch. They are inspection commands, not a deploy.

git diff --unified=3 origin/main...HEAD -- '*.py' '*.ts' '*.yml'
rg -n -i "messages|prompt|tool_args|authorization|api_key|last-trace" \
  --glob '!**/node_modules/**' --glob '!**/.venv/**'
rg -n "set_attribute\(|span\.setAttribute\(" -g '!**/vendor/**'

Then classify each hit. A key in a test fixture that expects rejection is fine. The same key in a production helper is a revert.

Decision table

Diff signal	Trust	Revert	Prove
Span around tool call, name + status + latency only	Yes, if helper is pure	No	Unit test the attribute dict
Full messages or tool args on the span	No	Yes	Denylist test must fail on current patch, pass after revert
Token-count integer, no text	Yes, after type check	No	Assert value is int and bounded
/debug/last-trace without auth	No	Yes	Route test expects 404 or 401, never 200 with a body
Fixture URL set to a shared model host	No	Yes	Test suite passes with network disabled
Env flag LOG_PROMPTS=true by default	No	Yes	Default config test expects false or absent

If a row is “Revert,” do not spend the review on naming, formatting, or model choice in that hunk. Fix the boundary first.

Where a free model endpoint fits

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

MonkeyCode is an open-source project. The outreach brief for this article states free model access, a free server option, and a free allowance on the order of 10 million tokens. This draft does not independently verify model names, hardware, duration, rate limits, or whether that allowance is still current on 8 October 2026. Confirm the live terms before you plan any review budget around them. Do not treat the number as a permanent quota.

Use that access only after the offline test is red. A disposable server is an optional manual repro for a scrubbed, synthetic prompt. It is not a test dependency, not a fixture host, and not a place to replay production traces. The earlier failure mode, calling a free server from a fixture and calling the result a contract, still stands. This workflow inverts it: the contract is the local assertion. The server, if used at all, comes later and stays out of CI.

A practical order:

1. Land the denylist test and watch it fail on the unsafe helper.
2. Remove payload attributes and the debug route. Watch the test pass with the network off.
3. If you still need to see a live tool round-trip, send one synthetic prompt from a local script, outside CI, to a free server whose current terms you just checked.
4. Record latency or error class in the PR comment if you want. Do not paste the prompt, the tool args, or the response body into the comment.

If the free server is down, slow, or quota-exhausted, the review does not block. The merge gate is the offline test.

Before you spend a live call on an agent diff, read the current MonkeyCode free-access terms and keep that server out of the test process.

Who should not use this workflow

Skip the live-repro step, and skip any third-party model endpoint, when any of these are true:

• Policy forbids sending even synthetic prompts to an external service, or the data class is unknown.
• The payload under review may contain customer content, credentials, or regulated data. Do not “sanitize and retry” on a hunch. Revert the export and review locally.
• You need a quota, model list, or uptime guarantee. None of those are established in this article.
• The PR’s real risk is authorization logic, schema compatibility, or retry amplification. Those need their own reviews. A redaction test will not catch them.

The denylist is also the wrong tool for a team that has not agreed which keys are sensitive. Agree the key set in the review, commit it next to the helper, and fail the build when the set and the helper diverge.

Review outcome

Approve the tracing shape when attributes are small, typed, and allowlisted. Request changes when the diff records prompts, tool payloads, or headers, or when a test needs a shared server to pass. The logger can ship the same day. The payload should not ship at all.

Limitations are part of the outcome. This check is key-oriented. It will not catch every encoding, every side channel, or every future attribute name. Re-run the rg pass on the next agent PR that touches telemetry. Agent diffs repeat this pattern under new file names.