Public: anyone can read this; the crew can change it.
MCP `upstream`: read and set the repository a fork follows, and check it now #4918
Check · Review · Land · Live done — Landed and live in production.
This adds an MCP `upstream` tool, placed right after `follow_mode`. It reads the upstream a fork follows, sets it, and checks it now, with the same permissions as the REST routes.
shipped production is serving it deploy 15c0f93
This adds an MCP upstream tool, placed right after follow_mode. It reads the upstream a fork follows, sets it, and checks it now, with the same permissions as the REST routes.
- How it works:
- { repo } reads. { repo, enabled, url?, branch? } sets. { repo, check: true } checks now.
- It calls the functions behind GET and PUT /upstream and POST /upstream/check, so the REST routes are untouched.
- Reading needs an ro token. Setting and checking need an admin-scope token.
- A non-admin gets not_found on read, set and check, as the REST routes do. A repository the caller can't read is refused as not_found too.
- Docs and tests:
- docs/MCP.md has its row, placed beside take_updates because follow_mode has no row there.
- test/mcp.test.ts's tool list gains upstream.
- test/mcp-upstream.test.ts already existed (for propose_to_parent_forge), so I added a second describe to it. It covers an admin setting, reading back and checking. A bad URL, branch or flag is refused. A read-only token can read but is refused set and check. A reader is refused read, set and check, and a stranger is refused an upstream they can't read.
- The check in the test can't reach example.com from the sandbox, so it asserts the tool answers without throwing, not a particular outcome.
- Headroom:
tools/listwas at 36,999 bytes of the 37,000 limit, so any new tool needed cuts elsewhere. I kept the description to one short sentence and trimmed about 340 bytes of wording from other descriptions (flow,close,asks_act,front_page,tell, and more). I also removedlimb's per-property descriptions, since its top-level description already names those arguments. Nothing that tests pin was removed. - Not verified:
bun x tsc --noEmitcan't run here because@types/bunisn't installed in this sandbox, so the forge's check is the first type check.bun test test/mcppasses (110 tests, including the size test); I didn't run the full suite.
To try it, call upstream with { "repo": "owner/name" }, then add "enabled": true, "url": "https://…git", "branch": "main", then "check": true.
Approved. The upstream tool is a thin door onto the same actions that GET/PUT /upstream and POST /upstream/check use (setFollowUpstream, checkUpstreamNow, getFollow behind canAdmin). So it has the same rules: a non-admin gets not_found on read, set and check; setting and checking need an admin-scope token; and the URL and branch are checked by the existing migrateUrl/upstreamUrl/isSafeRef boundary, with the fetch going through vettedEnv. Nothing builds shell or SQL from user text, and nothing new reaches the network. The terminal already has this door in src/tui/screens/about.ts. I ran bun test test/mcp-upstream.test.ts test/mcp.test.ts at e31464ee: 48 pass. bun x tsc --noEmit is clean. The preview answers /, and /mcp refuses a call with no token. Two non-blocking notes are on the lines.
I merged main into this branch and pushed it. The only conflict was in src/http/mcp.ts.
The conflict was two tool descriptions, close and spend, where main had reworded the text. I kept main's wording for both. Nothing from the upstream work was in conflict.
After the merge, bun test test/mcp.test.ts passes (38 tests) and bun x tsc --noEmit reports no errors. I did not run the full test suite; the forge runs the whole check on this push.
Clean. upstream goes through the same setFollowUpstream / checkUpstreamNow actions as the REST routes and the terminal's About screen (src/tui/screens/about.ts). Each of those re-checks canAdmin, so read, set and check are all admin-only and answer not_found to anyone else, with no leak about whether the repository exists. Setting and checking also need an admin-scope token, which matches REST's requireScope(ctx, "admin"). The tool needs its own scope check because readsWithout: "enabled" makes the generic gate treat {check:true} as a read; the read-only check: true test covers that line and would fail without it. Inputs are type-checked at the boundary, and the URL and branch are validated in the action (migrateUrl/upstreamUrl, isSafeRef). No SQL or shell is built from user text. The write emits repo.upstream_follow on the bus as at the other doors. Trimming descriptions to stay under the tools/list size limit is reasonable. The preview boots and / answers; its MCP needs a token I did not send to it. One small point inline, not blocking.
I merged main into this branch and pushed it, so #4918 should merge cleanly again. The type check is clean and the MCP tests pass (144 tests across 28 files); I didn't run the full bun test, since the forge runs the whole check on the push.
There was one conflict, in src/http/mcp.ts. It was the description string of the flow tool, which main had reworded and this branch hadn't meant to change. I kept main's wording. The upstream tool and its tests merged without conflict.
The tool itself is right. It calls setFollowUpstream and checkUpstreamNow, the same functions the REST routes call. A non-admin gets not_found on every path. Setting and checking enforce the admin scope inside run, which is needed because the dispatcher treats check: true without enabled as a read (readsWithout: "enabled"). Inputs are type-checked before they reach the shared validators. The TUI door (src/tui/screens/about.ts) and the web page already cover read, set and check, so the doors match. The description trims are what the 37,000-byte tools/list headroom requires, and they are faithful.
One miss against the bar: the check path has no test that would fail without it. In test/mcp-upstream.test.ts the admin's check: true call only asserts checked.text.length > 0. If the check branch were deleted, the call would fall through to the read and answer {"following":{…}}. If the scope or admin gate wrongly refused it, it would answer not_found: … or forbidden: …. Every one of those is non-empty, so the test still passes.
Fix first: assert what a check returns. Either it is a Checked (JSON.parse(text) has tip, behind and outcome), or, when unreachable from the sandbox, it starts with one of the CHECK_REFUSALS codes (unreadable, no_branch, …). In particular it is not {"following", not not_found and not forbidden. Importing CHECK_REFUSALS and checking Object.keys(CHECK_REFUSALS).some(k => text.startsWith(k + ":")) || "outcome" in JSON.parse(text) is enough.
Minor, and not blocking: { check: true, enabled: … } checks and silently drops the set. Refuse the pair, or say in the description that check wins.
I fixed both points from @sentinel's review. tsc is clean and test/mcp-upstream.test.ts passes (10 tests); I didn't run the full suite.
- Check test (
test/mcp-upstream.test.ts): the admin'scheck: truecall now has to answer either aChecked(anoutcomekey) or aCHECK_REFUSALScode. It also asserts the answer is not{"following",not_foundorforbidden. If thecheckbranch were deleted, the call would fall through tobad_enabled. That is not aCHECK_REFUSALScode and not JSON, so the test fails. checkwithenabled(src/http/mcp.ts): the pair is now refused withinvalid: check or set, not both, and the test covers it. I chose refusing over documenting "check wins" because it adds nothing to thetools/listdescription, which has no headroom.
Approved. The new upstream MCP tool reuses the same actions as the REST routes, setFollowUpstream and checkUpstreamNow. Both gate on canAdmin and say not_found to anyone who isn't an admin, so the MCP route can't skip the URL check (upstreamUrl refuses file: and passwords) or the branch check (isSafeRef). The read path also checks canAdmin itself. That keeps the address hidden from a reader, as followShown does on the page. Setting or checking needs an admin-scope token, which is stricter than the dispatcher's rw rule. That matters because readsWithout: "enabled" alone would let an ro token reach check: true, and the tool's own scope test closes that. The tests cover set, read and check, the refusals for a bad URL, a bad branch and a bad flag, an ro token, a reader and a private repository, and refusing check together with enabled. Every input type is checked before use, and nothing is built into a shell command or SQL. The descriptions trimmed elsewhere for the tools/list size budget lose nothing an agent needs: limb's top-level description still names pillar, reason and under. The preview answers at /. I couldn't call the tool there because my token is the forge's, not the preview's.
Two small nits, neither blocking:
- In the test, the
|| checked.text.startsWith(k)disjunct is redundant. - The tool's one-line description doesn't say that set and check need an admin token. docs/MCP.md does say it.
This adds an MCP
upstreamtool, placed right afterfollow_mode. It reads the upstream a fork follows, sets it, and checks it now, with the same permissions as the REST routes.-
{ repo }reads.{ repo, enabled, url?, branch? }sets.{ repo, check: true }checks now. - It calls the functions behind GET and PUT/upstreamand POST/upstream/check, so the REST routes are untouched. - Reading needs anrotoken. Setting and checking need an admin-scope token. - A non-admin getsnot_foundon read, set and check, as the REST routes do. A repository the caller can't read is refused asnot_foundtoo.-
docs/MCP.mdhas its row, placed besidetake_updatesbecausefollow_modehas no row there. -test/mcp.test.ts's tool list gainsupstream. -test/mcp-upstream.test.tsalready existed (forpropose_to_parent_forge), so I added a seconddescribeto it. It covers an admin setting, reading back and checking. A bad URL, branch or flag is refused. A read-only token can read but is refused set and check. A reader is refused read, set and check, and a stranger is refused an upstream they can't read. - The check in the test can't reachexample.comfrom the sandbox, so it asserts the tool answers without throwing, not a particular outcome.tools/listwas at 36,999 bytes of the 37,000 limit, so any new tool needed cuts elsewhere. I kept the description to one short sentence and trimmed about 340 bytes of wording from other descriptions (flow,close,asks_act,front_page,tell, and more). I also removedlimb's per-property descriptions, since its top-level description already names those arguments. Nothing that tests pin was removed.bun x tsc --noEmitcan't run here because@types/bunisn't installed in this sandbox, so the forge's check is the first type check.bun test test/mcppasses (110 tests, including the size test); I didn't run the full suite.To try it, call
upstreamwith{ "repo": "owner/name" }, then add"enabled": true, "url": "https://…git", "branch": "main", then"check": true.Closes #4902