I sell a human second pass on one AI-written PR (Riven Desk). Before pitching that, I wanted to do the work in public: pick three recent agent PRs from real repos, read the diffs (not just the summaries), and apply the same STOP checklist I give away for free.
Method, briefly:
copilot-swe-agent and bodies/trailers mentioning Claude Code (Co-Authored-By: Claude / claude.com/claude-code).
These are outsider reviews. I don't maintain these projects. Maintainers may have context I don't. I'm grading the diff as written, not the people.
**PR:** [Deduplicate sample binlog argument construction](https://github.com/microsoft/testfx/pull/11740)
**Author signal:** GitHub Copilot coding agent (`copilot-swe-agent`)
Size: ~27 changed lines across eng/build-samples.ps1, eng/samples-tools.ps1, eng/test-samples.ps1
State when reviewed: merged
Extracts repeated “build a -bl: / /bl: path under $BinaryLogDirectory” into Get-SampleBinlogArgument in eng/samples-tools.ps1, then calls it from the sample build/test scripts. The call sites already dot-source samples-tools.ps1, so the helper is in scope.
| STOP | Fires? | Notes |
|---|---|---|
| 1 Secrets | No | No credentials or env files |
| 2 Blast radius / no boundary | No | One clear intent: dedupe binlog arg construction |
| 3 Mixed concerns | No | Script-only, no lockfile/infra hitchhikers |
| 4 “No behavior change” while surface moved | Borderline | Behavior should match; see nit below |
| 5 Rollback story | Fine | One revert undoes it |
| 6 Security-sensitive paths | No | Build helper only |
| 7 Prompt/tool surface | No | |
| 8 CI / tests | N/A from diff alone | Trivial pure helper; no new failing assertion added |
-bl:`` Get-SampleBinlogArgument). Call sites that need MSBuild-style /bl: pass -ArgumentPrefix "/bl:" explicitly. That looks correct in the diff — just something a human should eyeball once so a future caller doesn’t assume the wrong flag..binlog to $LogName. Call sites that previously built "$name.binlog" now pass $name (or "$name.restore"). Consistent in this PR; don’t re-add Would merge. Nothing on the STOP list fires hard. This is the kind of agent PR that should land with a short human glance, not a drama review.
What I’d fix before merge (optional): one sentence in the PR body: “Default prefix -bl:; MSBuild restore/build paths pass /bl:.” Saves the next reviewer two minutes.
PR: Update vulnerable dependency pins Author signal: GitHub Copilot coding agent
Size: package.json + package-lock.json (~280 line churn, mostly lockfile)
State when reviewed: open
Updates npm overrides / pins for brace-expansion@1|2|5 and fast-uri, and bumps @stencil/react-output-target from 1.2.0 → 1.6.2 (lockfile follows, including @lit/react, ts-morph, nested minimatch, etc.).
| STOP | Fires? | Notes |
|---|---|---|
| 1 Secrets | No | |
| 2 Blast radius / no boundary | Yes — ask/split | Title says vulnerable pins; diff also jumps a codegen package several minors |
| 3 Mixed concerns | Yes — split | Security pin refresh + Stencil React output-target upgrade in one PR |
| 4 Surface moved | Ask | React wrapper generation can change across 1.2→1.6 with no app source in the diff |
| 5 Rollback | Partial | Revert works; “why these versions” isn’t written |
| 6 Security paths | Skimmed | Dependency pins are security-adjacent — need the CVE/advisory names in the PR |
| 7 Prompt/tool | No | |
| 8 Tests that catch the regression | Ask | Lockfile-only PRs often go green without proving consumers still build |
@stencil/react-output-target 1.2.0 → 1.6.2brace-expansion / fast-uri overridesnpm run of those packages.
Would not merge as written — request changes / split.
Smallest clear path:
This is a classic agent shape: honest security cleanup, then a larger upgrade rides along because the agent “fixed versions” broadly.
**PR:** [feat(lenses): built-in MCP Calls lens](https://github.com/Asymptote-Labs/agent-beacon/pull/723)
**Author signal:** human opener + `Co-Authored-By: Claude` / `claude.com/claude-code` markers
Size: ~387 changed lines — new mcp.lens.html, Playwright e2e, fixture lines, docs
State when reviewed: open
Adds a built-in dashboard “MCP Calls” lens: group MCP tool calls by server/tool, show args/results, mark failures, document it in docs/concepts/lenses.mdx, and cover it with Playwright ( builtin-mcp.spec.ts) including an XSS-shaped payload in fixture args.
| STOP | Fires? | Notes |
|---|---|---|
| 1 Secrets | No | |
| 2 Blast radius | No | Matches “add MCP lens” intent |
| 3 Mixed concerns | No | Feature + tests + docs for the same lens |
| 4 Surface claim | No | Docs say seven built-in lenses now |
| 5 Rollback | Fine | Revert removes the lens file + docs line |
| 6 Security-sensitive | Reviewed | Renders untrusted trace payloads in the browser |
| 7 Prompt/tool / rendered AI output | Watched closely — clears | Uses textContent /el() helpers; e2e asserts markup in args does not execute |
| 8 Tests | Strong | Playwright checks grouping, failure flag, XSS non-execution, empty state |
<img src=x onerror=...>; the test opens Arguments and expects window.pwned to stay undefined. That’s the right bar for a lens that prints agent/MCP payloads.arguments / result under gen_ai.tool.call, while the lens JS reads event.tool.arguments / event.tool.result and event.tool_call_id. If window.beacon.getTrace() normalizes those fields before the lens runs, fine — and the e2e implies it does. If someone later feeds raw JSONL into the lens, args/results would silently go missing. Worth one maintainer sentence: “getTrace maps gen_ai.call → tool.*.”mcp__server__tool, MCP:tool, event.type === "mcp") are documented enough in empty-state copy. Edge cases (weird tool names) are acceptable for v1.
Would merge after confirming the e2e job that runs builtin-mcp.spec.ts is green on the PR. I would not block on style. I would leave the field-mapping note as a non-blocking comment.
This is closer to what you want from an agent: feature-sized, security-aware rendering, and a test that would fail if someone “helpfully” switched to innerHTML.
Across these three:
| PR | Block? | Why |
|---|---|---|
| testfx#11740 | No | Small, matched intent, easy revert |
| modus-wc#1569 | Yes (as packaged) | Security pins mixed with a multi-minor Stencil React target bump; missing advisory + verification notes |
| agent-beacon#723 | No (pending green e2e) | Untrusted output handled; tests watch the scary path |
The interesting failure mode wasn’t “AI can’t code.” It was scope creep inside a true-sounding title (vuln pins) and whether security-sensitive UI proves it doesn’t execute untrusted text.
CI green is not a STOP clear. Title confidence isn’t either.
I used the free STOP one-pager while writing this: STOP conditions before you merge an AI agent PR.
I’m running a founding price on a single human review of one AI-written PR: $49 for the first 5 (normally $99) → Agent PR Audit — founding offer.
You get concrete findings with file names, a merge stance, and what to fix — same shape as the sections above, on your PR. No fake “bugs down X%” claims. The point is fewer merges you can’t explain.
— Gunjit / Riven Desk