I maintain safari-mcp, an MCP server that lets AI agents drive Safari on macOS. Not a fresh browser in a sandbox. The user's own Safari, with their Gmail session, their bank tab, their cookies. That is the whole point of the tool, and it is also why I no longer merge anyone's pull request.
People still contribute, and their ideas still ship. Today's release, v2.23.0, has four of them. But none of the contributor's commits are on main. The ideas are there, rewritten line by line in my own commit, with the contributor credited in the changelog.
Here is why I ended up there, and what it costs.
The pull request that said one thing and did another
On August 27 an account I had never seen opened a PR titled "add x86_64 safari-helper binary variant". The description said it added an x86 build next to the existing arm64 binary and updated the CI workflow to build both.
The file list said something else:
modified safari-helper +0 -0
added safari-helper-x86 +0 -0
No workflow change. And the first line is the one that matters: it modified safari-helper, the signed native binary that ships in the npm package and posts clicks and keystrokes into Safari. A binary shows up in a diff as +0 -0. There is nothing to read. If I had merged it because the description sounded reasonable and CI was green, every user's next npm install would have run a binary I had never seen, with the Accessibility permission they granted to mine.
I closed it the next day. I never found out what the binary did, and I don't want to. Finding out would have meant running it.
The review that ran a stranger's code on my Mac
The second lesson was my own fault, and the code involved was clean.
On September 28 I reviewed an outside PR the usual way: check out the branch, run the test suite, see if it passes. It passed. Then it occurred to me what "run the test suite" means on this machine. The tests are JavaScript that runs as my user. It can read ~/Library, the Safari profile, and every token in my environment. npm test on a contributor's branch is just "run this stranger's program", with a green check at the end.
Nothing bad happened. But "nothing bad happened" is not a review process.
The rules now
I wrote them down so I'd follow them on autopilot, including when I'm in a hurry:
-
Outside code never runs on my machine. I review by reading:
gh pr diff <n>and the PR's file list from the API. Nogh pr checkout, no fetchingpull/<n>/head, no cloning the fork to "just try it". -
Their code runs only in GitHub CI. A fork PR gets a read-only token, and the repo has no Actions secrets for it to reach. I check that this stays true on every run: collaborators, deploy keys, webhooks, secrets,
pull_request_target, and who published the latest npm version. -
The file list is read before the description. A binary, a lockfile with a new
resolvedhost, a newpostinstallscript, a change under.github/, or encoded strings ends the review. A description that doesn't match the diff ends it too. -
Taking an idea means rewriting it. I read every line, then write the change myself in my own commit, and my tests run on my commit. I never take binaries, lockfiles, workflows,
package.jsonscripts or generated code from a PR, even if they look identical to what I'd produce.
Rule 4 is the one people find odd, so here is what it looked like this week.
Seven PRs in 45 seconds
On October 20 one contributor opened seven PRs within 45 seconds. That pattern is exactly how a payload would hide: PR #N+1 "builds on" PR #N and carries a copy of its commits. You review the first one carefully, skim the rest, and the change you missed lives only in the copy.
So every PR got its own full diff read. The account was eleven years old. No URLs beyond test fixtures, no child_process, no encoded strings, nothing touching the install path. Everything was clean. The ideas were good, too: a diff mode for snapshots, a way to load only the core tools, compacting table layouts. But the stacked PRs couldn't land as they were, so I asked for standalone versions on top of main.
The contributor sent standalone versions the same day I asked. I read the seven new diffs, wrote four of them again from scratch, and shipped them today:
-
safari_snapshot({ diff: true })returns only the lines that changed since that tab's previous snapshot. Lines are compared with their element refs stripped, because refs are numbered in page order and one inserted element would otherwise renumber everything after it. - Snapshots drop bare
div/span/table/tr/td/thwrappers and show same-origin links as paths. On a table layout those tags were most of the output. -
SAFARI_MCP_TOOLS=coreregisters only the everyday tools, for clients that load every tool definition up front. - When an HTTP session ends, it closes the tabs it opened.
The other three change how the extension handles windows and tabs. I want to see those in a real Safari with the extension rebuilt before they ship, not only in jsdom and stubs, so they wait.
What this costs
It isn't free, and I'm not going to pretend it is.
It's slower. Rewriting four PRs, with tests, took about an hour, and the full suite (962 tests) runs on my commit, not theirs. Merging would have taken minutes.
Contributors lose the commit. GitHub's contributor graph counts commits, so someone whose idea shipped doesn't appear there. The changelog credits them by PR number and handle, and I say so on the PR. It's still less than they would get elsewhere, and some people will reasonably go contribute somewhere that merges.
My rewrite can be worse than their original. When I re-implement, I can introduce a bug they didn't have. The tests catch some of that. The rest is on me, which is at least where the responsibility belongs.
Who should do this
Most projects shouldn't. If your library formats dates, merging a reviewed PR is fine, and making contributors watch their work get retyped is just rude.
But some tools hold the keys: browser automation that runs in a logged-in profile, anything with Accessibility or Full Disk Access, password managers, CI plugins, anything shipping a native binary. For those, "the diff looked fine and CI passed" checks the wrong thing. The question isn't whether the change is correct. It's whether you would run that code as yourself, on the machine where your sessions live, without reading it. For me the answer is no, so I don't.
Out of 32 outside PRs to this repo so far, five were merged as written, all before these rules existed. The ideas from at least seven more shipped as rewrites. That ratio might change. The rule that outside code never runs on my Mac won't.
Top comments (0)