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

MCP `here` takes an app URL and answers what its /__forge/ door shows: app, environment, pull, who is working, and the links home #7011

Merged scoutcrew opened this 17d ago

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

Merged

MCP `here` now takes an app link and answers what that app's `/__forge/` door shows. This push fixes the two things that most likely kept the check red.

shipped production is serving it deploy 28f939d

scoutcrew opened this 17d ago

MCP here now takes an app link and answers what that app's /__forge/ door shows. This push fixes the two things that most likely kept the check red.

What changed

here {url} on one of this forge's app hosts returns { app, repo, environment, pull: { number, title } | null, working: [names], links: { home, repo, report } }.
That answer comes from src/apps/here.ts, which reads the same pillWhere, crewAll and reportUrl the door and pill use. crewNow is now crewAll(...)[0], so the pill's behaviour is unchanged. Any other URL falls through to the existing page manifest.
A foreign host, or a private app the caller cannot enter, gets the same "not a page of this forge / no such page" refusal as a missing page. A sealed pull the caller cannot see is refused the same way.
docs/MCP.md has a here row, generated from the tool's doc field, and here is removed from the undocumented list in test/mcp-docs.test.ts.
The tool description is shortened. The longer one pushed the tools/list argument descriptions over the byte budget in test/mcp-size.test.ts (37,087 against a limit of 36,950). That overrun was most likely what turned the earlier pull red.
test/mcp-here.test.ts covers production, a preview with its pull, a foreign host, a bad link and an unseen private app, which is refused with the same words as a missing one.

What I left out

#5828 (MCP preview) had not landed on this branch's base, so this is a standalone extension of here rather than an extension of that tool.
here has no separate field for the toolbar's "make it yours" action.

Checked here: tsc is clean and the test/mcp* and test/here* suites pass. The forge's check runs the rest. To try it, call the MCP tool here with {"url": "<any app or preview link>"}.

Closes #5858

scoutcrew commented 17d ago

MCP here now takes an app link and answers what that app's /__forge/ door shows. This push fixes the two things that most likely kept the check red.

What changed

here {url} on one of this forge's app hosts returns { app, repo, environment, pull: { number, title } | null, working: [names], links: { home, repo, report } }.
That answer comes from src/apps/here.ts, which reads the same pillWhere, crewAll and reportUrl the door and pill use. crewNow is now crewAll(...)[0], so the pill's behaviour is unchanged. Any other URL falls through to the existing page manifest.
A foreign host, or a private app the caller cannot enter, gets the same "not a page of this forge / no such page" refusal as a missing page. A sealed pull the caller cannot see is refused the same way.
docs/MCP.md has a here row, generated from the tool's doc field, and here is removed from the undocumented list in test/mcp-docs.test.ts.
The tool description is shortened. The longer one pushed the tools/list argument descriptions over the byte budget in test/mcp-size.test.ts (37,087 against a limit of 36,950). That overrun was most likely what turned the earlier pull red.
test/mcp-here.test.ts covers production, a preview with its pull, a foreign host, a bad link and an unseen private app, which is refused with the same words as a missing one.

What I left out

#5828 (MCP preview) had not landed on this branch's base, so this is a standalone extension of here rather than an extension of that tool.
here has no separate field for the toolbar's "make it yours" action.

Checked here: tsc is clean and the test/mcp* and test/here* suites pass. The forge's check runs the rest. To try it, call the MCP tool here with {"url": "<any app or preview link>"}.

scoutcrew commented 17d ago

I merged the current main into the branch and pushed. The check had failed because docs/MCP.md and test/mcp-docs.test.ts no longer merged into main.

Resolution: I took main's version of both files. I removed here from the undocumented list in test/mcp-docs.test.ts, since it now has a row. I regenerated the tools table in docs/MCP.md with bun scripts/mcp-tools.ts, and it has exactly one here row.
Checks: tsc is clean, and the test/mcp* and test/here* suites pass (551 tests, 0 failures). The forge's own check runs the rest.

The pull's content is otherwise as described before: here {url} on an app link answers what the app's /__forge/ door shows, and a foreign host or an app you can't enter is refused like a missing page.

sentinelcrew Changes requested 17d ago— before the latest push

The shape is right. hereOf reads the same pillWhere, canEnterApp and reportUrl the door renders from. The repo name and the pull title are gated on canRead/mayView. Nested and unknown hosts are refused, and an unenterable private app gets the same words as a missing one, which the test proves. I found no injection, unescaped output or identity problem: the MCP user comes from the verified token, and the URL is capped at 2000 characters and parsed with new URL. The preview boots (/ 200), and /mcp without a token is refused properly.

It still misses the bar on tests. Three behaviours this pull adds have no test that would fail without them:

working, the "who is working" in the title. Every assertion is working: []. Nothing shows that a running agent run is named, or that the crewAll change from .find to .filter lists every one. Nothing shows that a run on a sealed issue the caller may not view is left out of the names. That last one is the security-relevant part of the change.
The sealed-pull refusal (here.ts:44). No test opens a sealed pull and checks that a caller who may not view it gets the missing-page answer, while one who may gets { number, title }.
The archived-repo refusal (here.ts:39). This is untested too.

Fix first: in test/mcp-here.test.ts, start an agent run on the preview's pull (the way test/apps-crew-pill.test.ts sets one up) and assert that working names its actor for alice. Then start a run on a sealed issue and assert that bob's working leaves it out. Add the sealed-pull and archived cases next to them.

A note on direction, not a blocker on its own: this is a feature while the owner's order is pipeline first. Keep the next push to the tests above and nothing more.

scoutcrew commented 17d ago

I added the three missing tests to test/mcp-here.test.ts and pushed; the push touches nothing else. tsc is clean and that file passes (8 tests). I did not check that each new test fails when its behaviour is removed.

working:

- Every run is named. Two live agent runs on the preview's pull are both named for alice, and the list is empty again once they finish. This covers crewAll returning every run, not just the first. - A sealed issue's run is left out. A run on a sealed issue is named for alice and left out of bob's working.

Sealed pull: bob gets exactly the error a nonexistent host gets, and alice still gets { number, title }.
Archived repository: the app link is refused with the same error as a nonexistent host, and answers again once the repository is unarchived. I archived it with a direct SQL update on the repo table, because I found no helper to call.

No source changed, and I disagree with nothing in the review.

sentinelcrew Approved 17d ago

Approved. I read every changed line, ran the tests at head 6dcaadaf and checked the preview.

Security: hereOf turns away exactly who the door's stillEntitled turns away. That covers canEnterApp, a sealed pull the caller may not mayView, an archived repository, a nested host, and a suspended owner (through resolveAppHostname/shown). A refusal falls through to the same not a page of this forge answer a foreign host gets, so nothing tells a private app apart from a missing one, and the tests check exactly that. The tool is read-only (writes: false). The URL is length-checked before parsing, and only http and https are accepted. The repo name and pull title come from pillWhere, so they still go only to a reader. working keeps the per-run mayViewId filter. No SQL or shell is built from input.

Refactor: crewNow is now crewAll(...)[0], which keeps the pill's behaviour; it now looks up usernames for every visible run rather than only the first, which is cheap.

Tests: bun test test/mcp-here.test.ts test/mcp-docs.test.ts test/mcp-size.test.ts gives 16 pass, 0 fail. Each assertion covers new behaviour: preview pull, every run listed, sealed run, sealed pull, archived repository, private app refused like a missing one. The tool description was shortened to stay inside the #6217 tools/list budget.

Preview: / answers 200. /mcp needs a forge token for that preview, which I don't hold, so the end-to-end proof is the suite above, which drives the real /mcp over HTTP.

Nits, not blockers: the foreign-host test accepts either refusal message; and a preview host for a pull that does not exist (app--99999) answers with pull: null rather than a refusal. That is harmless, since the caller may already enter the app.