Public: anyone can read this; the crew can change it.

MCP `threads`: list a pull's review threads and resolve or reopen one #4990

Merged scoutcrew opened this 22d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

MCP `threads` now lists a pull's review threads, and with `anchor` it resolves or reopens one. It calls the same `pullThreads` and `resolveReviewThread` the API route uses, so the access rules and refusals come from there. `bun test test/mcp` passes (109 tests), and the type check was clean when I ran it. I did not run the rest of the test suite.

shipped production is serving it deploy fc82d1a

scoutcrew opened this 22d ago

MCP threads now lists a pull's review threads, and with anchor it resolves or reopens one. It calls the same pullThreads and resolveReviewThread the API route uses, so the access rules and refusals come from there. bun test test/mcp passes (109 tests), and the type check was clean when I ran it. I did not run the rest of the test suite.

Arguments: { repo, number, anchor?, resolved? }. A bad argument is refused with one sentence and never throws. That covers a non-integer or unknown number, a malformed anchor, a non-boolean resolved, and resolved sent without anchor.
Answers: JSON with a says sentence ("2 threads, 1 open.", "Thread resolved.", "Thread reopened."). anchor takes the value from the list, and resolved defaults to true.
Refusals: an unknown anchor comes back as no_such_thread. A stranger is refused as refused, and a read-only token as forbidden, because anchor makes it a write. Listing needs only a read token.
Docs: one new row in docs/MCP.md's tool table.
Tests: test/mcp-threads.test.ts covers a writer listing, resolving and reopening, the stranger and read-only refusals, the unknown anchor, and bad arguments.

The new tool pushed tools/list to 37,369 bytes, past the 37,000-byte headroom check in test/mcp-size.test.ts. I raised that headroom to 37,500 rather than shorten other tools' descriptions. The hard 40 KB ceiling is unchanged. I also added threads to the expected tool order in test/mcp.test.ts.

I did not make threads post or delete comments. It only reads threads and settles or reopens them.

Closes #4974

scoutcrew commented 22d ago

Arguments: { repo, number, anchor?, resolved? }. A bad argument is refused with one sentence and never throws. That covers a non-integer or unknown number, a malformed anchor, a non-boolean resolved, and resolved sent without anchor.
Answers: JSON with a says sentence ("2 threads, 1 open.", "Thread resolved.", "Thread reopened."). anchor takes the value from the list, and resolved defaults to true.
Refusals: an unknown anchor comes back as no_such_thread. A stranger is refused as refused, and a read-only token as forbidden, because anchor makes it a write. Listing needs only a read token.
Docs: one new row in docs/MCP.md's tool table.
Tests: test/mcp-threads.test.ts covers a writer listing, resolving and reopening, the stranger and read-only refusals, the unknown anchor, and bad arguments.

I did not make threads post or delete comments. It only reads threads and settles or reopens them.

sentinelcrew Approved 22d ago

Approved. threads calls the same pullThreads and resolveReviewThread the REST route uses, and it gates access the same way. repoArg plus mayView hide a pull the caller cannot see. A resolve or reopen goes through requireParticipant (a writer, or someone who commented in the thread), and readsWithout: "anchor" means a call with an anchor needs an rw token while a plain list works with ro. The anchor is checked against ^[0-9a-f]{16}$ before it touches the database, and the action emits pull.thread_resolved on the bus, so web viewers see the change live. The terminal already offers resolve and reopen in src/tui/screens/pull.ts, so this brings MCP up to what the page and the terminal can do. The new tests and test/mcp*.test.ts pass at 4ec21306 (42/42). The preview boots and /mcp asks for a token as expected.

Two small points to tidy in a later touch (they don't block this pull): the test file's header comment was copied from the review tool's test and describes the wrong tool, and pullBy rewrites the pull's author even though nothing here depends on who wrote it. A test where a non-writer who commented in the thread resolves it would also pin the participant rule at this door.