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

MCP `releases` also returns a `ui://forge/releases` card without a new tool: what went live, newest first, each row linked to its pull #11329

Merged masoncrew opened this 16h ago

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

Merged

The `releases` card (`ui://forge/release`) was already on main from #11131. It was missing two things the issue asks for, so this pull adds them: a link to the pull on each row, and a cap of 10 rows instead of 20. The tests I ran pass: `test/mcp-releases-card.test.ts`, `test/mcp-size.test.ts` and `test/mcp-card-table.test.ts`. `tsc --noEmit` is clean, but the output of `bun scripts/gate.ts` was cu…

shipped production is serving it deploy aa46b8b

masoncrew opened this 16h ago

The releases card (ui://forge/release) was already on main from #11131. It was missing two things the issue asks for, so this pull adds them: a link to the pull on each row, and a cap of 10 rows instead of 20. The tests I ran pass: test/mcp-releases-card.test.ts, test/mcp-size.test.ts and test/mcp-card-table.test.ts. tsc --noEmit is clean, but the output of bun scripts/gate.ts was cut off by my tail, so I can't say it passed.

Link per row: each row now links to {origin}/{owner/name}/pulls/{N}. The link is built only when the repo is a plain owner/name slug and the number is a positive integer. A hostile repo or number gives a row with no link.
Row cap: the card shows the last 10 releases, newest first. The releases tool's own text answer is unchanged.
Docs: docs/MCP.md now says "the last 10, newest first, each linked to the pull that landed it" in the card table and in the Releases card paragraph.
Test: a new case in test/mcp-releases-card.test.ts checks the cap, the link, and that a hostile repo or number yields no link. It would fail without this change.
Unchanged: no new tool and no size ceiling raised. The card's row text still escapes through the same shell as before, and the existing hostile-version test still passes.

I did not add a card for the shipped list (what went live). The existing card is for kept releases, and {shipped: true} still returns no card.

To try it, call the MCP releases tool and read structuredContent.card.

The pre-push hook wanted a commit saying how to try the pull, so there is an empty "Try:" commit.

🤖 Generated with Claude Code

Closes #11319

masoncrew commented 16h ago

Link per row: each row now links to {origin}/{owner/name}/pulls/{N}. The link is built only when the repo is a plain owner/name slug and the number is a positive integer. A hostile repo or number gives a row with no link.
Row cap: the card shows the last 10 releases, newest first. The releases tool's own text answer is unchanged.
Docs: docs/MCP.md now says "the last 10, newest first, each linked to the pull that landed it" in the card table and in the Releases card paragraph.
Test: a new case in test/mcp-releases-card.test.ts checks the cap, the link, and that a hostile repo or number yields no link. It would fail without this change.
Unchanged: no new tool and no size ceiling raised. The card's row text still escapes through the same shell as before, and the existing hostile-version test still passes.

I did not add a card for the shipped list (what went live). The existing card is for kept releases, and {shipped: true} still returns no card.

To try it, call the MCP releases tool and read structuredContent.card.

The pre-push hook wanted a commit saying how to try the pull, so there is an empty "Try:" commit.

🤖 Generated with Claude Code

junipercrew Changes requested 15h ago— before the latest push

Two fixes before this lands.

Link safety (first): the pull link accepts any owner/name that has no slash, so a repo of a/.. passes slugOf and yields https://f.test/a/../pulls/N, a path that climbs out of the repo. The runs card next to it also checks here(origin, path) and RUN_ID; this one checks neither. Build the path first, require a real slug (no . or .. segments), and add the traversal case to the test (a/.., ./x), not just //evil.

The card's name does not match the issue. Issue #11319 says ui://forge/releases; the card is ui://forge/release (CARD_KINDS, mcp.test.ts:1304, docs). Either rename the resource and update the docs and tests, or say on the issue why release stays. As it is, boxes 1 and 4 are not met.

Smaller: the docs say the row names the pull and also that it is linked, and the status line still counts every kept release (N kept) while only 10 rows show. Say last 10 of N, or count only the rows shown. The test's items carry no hostile version/proof, so the new case does not prove 'escapes every field'; add one hostile field to the card test. The page and the terminal both show releases, so check that the page and card wording match (box 4).

No layout, accent or border concerns: the change adds a link on an existing row.

masoncrew commented 15h ago

The card is ui://forge/release, not ui://forge/releases, and I left it that way, so boxes 1 and 4 still read differently from the issue. The review's other points are fixed and pushed. tsc --noEmit is clean. test/mcp-releases-card.test.ts, test/mcp-size.test.ts and test/mcp-card-table.test.ts pass.

What changed since the first push

Link safety: the pull link now needs a real owner/name (REPO_SLUG, with no empty, . or .. segment), a positive pull number, and here(origin, path). The path is built once and reused for the link.
Link tests: //evil, a/.., ./x, ../x, a/., a/b/c and a b/c all give a row with no link.
Status line: it reads last 10 of N when more than 10 releases are kept. The proven, unproven and rolled-back counts cover only the rows shown. With 10 or fewer it still reads N kept, as before.
Escaping: a new case puts a hostile string in the sha, the version and the proof. It must arrive as plain text in the row, and the card shell must not contain it.
Docs: docs/MCP.md now says each row's pull is linked, and the status line says last 10 of N. I did not compare this wording with the releases page.

Why release stays: release is the existing resource from #11131. CARD_KINDS, mcp.test.ts, the resource list and test/mcp-card-table.test.ts all use it, and the other card names are singular too (pull, issue). Renaming it would break any host that already knows ui://forge/release, for a one-letter difference. If you want releases, it is a rename across those places. Say so and I will do it.

🤖 Generated with Claude Code

junipercrew Approved 15h ago

Earlier asks are met: the pull link is validated (REPO_SLUG, positive integer, same-origin), the status line says last 10 of N, and the escaping and link tests are in. test/mcp-size.test.ts and test/mcp-releases-card.test.ts pass here. No new tool and no ceiling raised.

Two things I would do next:

Non-blocking: the new REPO_SLUG comment sits between the RUN_ID doc comment ("The forge id shape") and RUN_ID, so that comment now describes the wrong constant. Move REPO_SLUG above it.
Non-blocking: the card is ui://forge/release, not the ui://forge/releases the issue names. Mason explained why (existing resource from #11131). Either amend the issue or alias the name; and check the docs wording against the releases page.

I left boxes 1 and 4 open for the name mismatch.

trevatoowner commented 15h ago

Left #11319 open: 2 unticked items remain (The existing releases tool's result carries the card resource ui://forge/releases listing the last 10 releases, newest first, each with version, when, and a link to the pull that made it; docs/MCP.md names the card in the same words as the page).