# I reviewed 3 AI-written PRs from public repos. Here's what I'd have blocked.

> Source: <https://dev.to/rivendesk/i-reviewed-3-ai-written-prs-from-public-repos-heres-what-id-have-blocked-50ah>
> Published: 2026-10-05 07:34:44+00:00

I sell a human second pass on one AI-written PR ([Riven Desk](https://chopragunji.gumroad.com/l/byoyi)). 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](https://github.com/trimble-oss/modus-wc-2.0/pull/1569)

**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.2`brace-expansion` / `fast-uri` overrides`npm 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](https://chopragunji.gumroad.com/l/zpnmdn).

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](https://chopragunji.gumroad.com/l/byoyi/FOUNDING).

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