Review an Agent PR That Fetches Model-Named Docs URLs

A maintainer found pull request 218 on a quiet Tuesday. An agent had added a docs fetch to a small coding tool. The summary promised richer answers from live documentation pages.

This write-up is a constructed review case, not a named outage. The file names are concrete so a team can replay the pass. This case claims no production metric and no customer incident.

The change on the branch

The diff touched three files in the agent runtime. One file pulled a URL out of the model text. Another file downloaded the page and built the next prompt.

A third file wrote that prompt beside the source tree. The agent described the feature as a context helper. The helper read a URL from the model reply.

It then inserted the page body into the following prompt. A narrow docs workflow can justify a bounded helper. This patch left the network path wide open.

What the reviewer can trust

Trust belongs to pure functions and to pinned fixtures. A delimiter wrapper can stay when it never opens a socket. A size constant can stay when a test locks the number.

A dry-run flag can stay when it only prints the plan. That flag must default to closed network access. A green summary written by the agent is not evidence.

function canTrust(change) {
  return change.pure === true
    && change.network === false
    && change.testsPinLimits === true;
}

The sketch above is a review aid, not a measured result. It does not approve any changed file by itself. A human reviewer still has to read the full diff.

What to revert before merge

Revert the open fetch of any model-named URL. A name inside a model reply is not an allowlist. A confident summary is not a host policy either.

Revert redirect following on that HTTP client immediately. A redirect can move the request after the first host check. The revert stands without a catalog of bypass tricks.

Revert every packed-prompt write inside the repository tree. Packed text may contain tool output or local secrets. A debug file in the tree can enter the next commit.

Revert paths that map a failed fetch to empty success. Empty success hides a broken context pack from the log. The agent then continues as if the page had been read.

Numbered review steps

1. Freeze the network first

Check out the pull request on a disposable branch first. Run the suite with external network access disabled. A helper that fails closed is easier to judge fairly.

git fetch origin pull/218/head:pr-218
git switch --detach pr-218
npm test

Those three commands above are only a review sketch. They are not a transcript from a finished execution. Replace the remote name if the fork uses another label.

2. Separate pure packing from I/O

Keep one function that only wraps text and enforces a cap. Move fetch calls and disk writes behind a narrow interface. Leave that interface unimplemented inside the unit tests.

export function packUntrusted(label, text, limit) {
  const body = String(text).slice(0, limit);
  return [
    `<untrusted source="${label}">`,
    body,
    "</untrusted>",
  ].join("\n");
}

The wrapper above is a proposal for the review harness. It does not prove the model will obey the fence. It only keeps untrusted text bounded and easy to spot.

3. Replace the open client with an allowlist

Accept exact hosts from a list a human already reviewed. Require HTTPS on every URL the helper approves. Refuse userinfo, blank hosts, and every non-HTTPS scheme.

const ALLOWED = new Set(["docs.example.com"]);

export function approveUrl(raw) {
  const url = new URL(raw);
  if (url.protocol !== "https:") {
    return { ok: false, reason: "scheme" };
  }
  if (url.username || url.password) {
    return { ok: false, reason: "userinfo" };
  }
  if (!ALLOWED.has(url.hostname)) {
    return { ok: false, reason: "host" };
  }
  return { ok: true, href: url.href };
}

Do not add private addresses just for local convenience. Do not match hosts with a loose substring check. Exact set membership is the rule this review enforces.

4. Pin limits with fixture tests

Cover one allowed host and one rejected host in tests. Cover a response body longer than the stated cap. Assert the packed body line stays within that cap.

import test from "node:test";
import assert from "node:assert/strict";
import { approveUrl } from "./approve-url.js";
import { packUntrusted } from "./pack.js";

test("rejects a host outside the set", () => {
  const verdict = approveUrl("https://evil.example/page");
  assert.equal(verdict.ok, false);
  assert.equal(verdict.reason, "host");
});

test("rejects a non-HTTPS scheme", () => {
  const verdict = approveUrl("http://docs.example.com/guide");
  assert.equal(verdict.ok, false);
});

test("caps packed untrusted text", () => {
  const packed = packUntrusted("docs", "x".repeat(80), 16);
  const lines = packed.split("\n");
  assert.equal(lines[1], "x".repeat(16));
  assert.equal(lines.length, 3);
});

The tests above are unexecuted examples for the reviewer. They do not replace a run against the actual branch. Apply the revert list before treating a green run as final.

5. Keep debug bundles outside the worktree

Send debug output to a temp directory the reviewer chooses. Refuse any path that resolves inside the project root. Delete that temp directory when the review session ends.

import path from "node:path";

export function assertOutsideRepo(repoRoot, requested) {
  if (!path.isAbsolute(requested)) {
    throw new Error("debug path must be absolute");
  }
  const root = path.resolve(repoRoot);
  const full = path.resolve(requested);
  const rel = path.relative(root, full);
  const inside = rel === ""
    || (!rel.startsWith("..") && !path.isAbsolute(rel));
  if (inside) {
    throw new Error("debug bundle must stay outside the repo");
  }
  return full;
}

The path helper above is proposed review code only. This draft does not include an execution log for it. Run it on a temp path and a repo path before trust.

6. Write the decision before any model rerun

Put a short decision table in the review note itself. Rerun a model only after the revert list is applied. A second model opinion must not reopen a reverted fetch.

Decision table

Change	Trust	Revert	Test next
Delimiter wrapper with no I/O	Yes, if pure	No	Fixture round trip
Size cap constant	Yes, if pinned	No	Over-cap fixture
Fetch of model-named URLs	No	Yes	Allowlist rejection
Redirect following	No	Yes	Client flag stays false
Debug file inside the repo	No	Yes	Path refusal
Failed fetch as empty success	No	Yes	Error stays visible

Failure modes worth a fixture

A missing allowlist fails open under a friendly model prompt. The model can name a host the project never reviewed. The fixture should reject that host before any socket opens.

A size cap that lives only in a comment will drift. The next edit can raise the slice without a failing test. Pin the number in an assertion, not in pull request prose.

A debug write beside the source tree looks harmless in a demo. The next commit can pick the file up with a broad add. The path test should fail when the target sits inside the root.

Empty success on a fetch failure looks like a small convenience. Callers then build the next prompt around a silent gap. The test should expect a thrown error or a visible failure flag.

Where a scratch server fits

After the local revert, move the branch to a disposable server. MonkeyCode is relevant as one place to rerun the patched agent. Disclosure: This article was prepared as part of MonkeyCode's product outreach.

Operator materials cite free model access, a free server, and open-source status. They also cite a free token allowance near ten million tokens. This article does not re-measure that allowance or name any model.

Confirm the live terms before a review depends on them. Treat the free server as a scratch runner, not a trust boundary. Point that runner at the patched branch and the fixture file.

Keep production credentials off that disposable machine entirely. A model reached through free access may draft the review note. It still cannot approve the host list or the merge.

git clone --branch pr-218-reverted /path/to/local/repo review-218
cd review-218
node --test test/approve-url.test.js

The scratch commands above are unverified in this draft. Stop if the suite asks for a secret or a production host. Do not widen the allowlist just to turn a red test green.

Record the red case in the review note instead of hiding it. A free server share is not a hardware spec or a duration promise. Remove the scratch workspace when the review session ends.

Limits of this pass

The allowlist rots when a documentation host changes its name. Exact checks miss the new domain until a human edits the set. That host edit belongs in the same pull request as the domain.

The wrapper does not prove a model will respect delimiters. It only makes untrusted text visible and length-capped for review. Broader injection study sits outside this particular review pass.

Quotas and model lists can change without notice in public docs. Do not copy an older token figure into a runbook as a guarantee. Read the current project page on the day of the review.

Who should skip this approach

Skip this approach for repositories that store customer data. Skip it when security review already locked the network policy. Skip it if no human will actually read the revert list.

Do not use the free server as a production runtime. Do not treat a free token pool as a load-test budget. Do not merge because a model agreed with the agent author.

Teams without fixture discipline should not adopt the helper yet. A verbal allowlist written in chat will drift by the next sprint. Wait until the rejection tests live in the repository itself.

Closing note

The useful fragment was the packing idea, not the open client. Keep the pure wrapper and the pinned size cap. Revert the fetch, the redirects, and the in-repo debug write.

Run the fixture tests before any model sees the branch again. Readers who keep a scratch server can replay this same review. Check current free-access terms, then run the rejection tests.