@fresh-machine-37269 look at the review github.com/OpenHands/OpenHands/pull/17344#… that happened from all-hands-bot, you can find the conversation with that hash in your logs. You can see there's been another set of reviews in that PR. I want to know when did the pr-review automation conversation start: before or after the MCP fix was made. In other words, at what hash did all-hands-bot review?

🤖 On it! View progress here: oss-agent-canvas.ngrok.dev/conversations/…

@fresh-machine-37269 check the other reviews and further commits, and tell me WDYT about the reviews and fixes

✅ Done! Here's what I found across the whole PR thread, and my read on it. Timeline recap 1. 04:03 — First all-hands-bot review, on head

4063996

("test: cover MCP config utilities", +965/−319 across 2 files). Intended

APPROVE

, actually posted as

COMMENT

(the review's own think-log noticed the event field got mangled but decided not to re-post). 2. 04:45 —

9da49cdb631c

"test: restore MCP contract coverage" (the "MCP fix"). 3. 05:09 —

5aadaf8dbfde

"chore: Remove PR-only artifacts". 4. 05:28 — Second all-hands-bot review, on the latest head

5aadaf8

(+981/−314 across 1 file). This one actually ran the vitest suite, then posted a

COMMENT

review (again

COMMENT

, not

APPROVE

, even though its own verdict said "No material findings. Approve"). What the "MCP fix" actually was It wasn't a production-code fix — it restored a missing test case. The

9da49cdb631c

commit only touches

__tests__/utils/mcp-config.test.ts

, re-adding

it("preserves settings keys through parse and installed-server flattening")

, which exercises

flattenMcpConfig

round-trip behavior. That function was conspicuously absent from the first review's coverage list (it enumerated

parseMcpConfig

,

toCanonicalMcpServer

,

buildMcpServerPatch

,

getSdkMcpServerMap

, etc. — but not

flattenMcpConfig

). The artifact commit

5aadaf8

removed the

.pr/coverage-mcp-config-vitest.png

screenshot (and other PR-only assets) — which is precisely what the first review had called out ("The

.pr/

screenshot artifact is cleaned up by repository automation"). My take • Substance was sound on both passes. Both reviews correctly classified this as test-only, low-risk, no production changes, and both verified the mutation-testing claim (97.35%, 511 killed / 14 survived) rather than taking it on faith. • The first review rubber-stamped a real gap. It said "no material issues" while

flattenMcpConfig

(settings-key preservation through installed-server flattening) was untested. The author then had to ship a follow-up to restore it. So the first review missed exactly the kind of contract-coverage hole it was supposed to catch — a genuine miss, not just a nitpick. • Recurring process bug: APPROVE became COMMENT — twice. Both reviews concluded "no findings / approve" but both landed on GitHub as

COMMENT

. That's a real reliability issue in the reviewer: it means the bot never actually approves, it only comments, which weakens the automation's value for maintainers who want a green review. • The second review's posting path was messy — it hit REST secondary rate limits and thrashed through GraphQL/`gh` CLI fallbacks before succeeding. Functionally fine, but brittle. Net: the content of the reviews is trustworthy and the follow-up fixes were legitimate responses (one restoring missed coverage, one cleaning up artifacts). But the automation has two recurring bugs worth fixing — (1) it doesn't actually emit

APPROVE

even when its verdict is "no findings," and (2) it missed a genuine coverage gap on the first pass that a human (or a better checklist) would have caught.