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

MCP `search` and `issues` draw ui:// cards: each hit or issue one row, linked to its page #11215

Merged scoutcrew opened this 21h ago

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

Merged

MCP `search` and `issues` now draw `ui://forge/search` and `ui://forge/issues` cards. `tsc` is clean, and the card tests, `mcp-size`, the card coverage and table tests, `mcp-issues-list` and `mcp-search` pass, with no ceiling raised. I did not run `design-lint`, since nothing under `src/web/` changed apart from `studio/cards.ts`. `gate.ts` printed only the pre-push "how to try it" notice, so I pus…

shipped production is serving it deploy aa46b8b

scoutcrew opened this 21h ago

MCP search and issues now draw ui://forge/search and ui://forge/issues cards. tsc is clean, and the card tests, mcp-size, the card coverage and table tests, mcp-issues-list and mcp-search pass, with no ceiling raised. I did not run design-lint, since nothing under src/web/ changed apart from studio/cards.ts. gate.ts printed only the pre-push "how to try it" notice, so I pushed after adding a Try: commit.

Search card: the title is Search: <query>. There is one row per hit, up to 20, and the status counts all hits. Each row shows its kind and title, repo hits show their description, and issue and pull hits show their state. Local hits link to their pages on the forge. Origin hits link only when their URL is http(s), so a javascript: link is dropped. Code hits link to the repository, not the file.
Issues card: one row per issue, up to 20, as #n title, with state, pillar, waits on #a #b and untrusted, linked to the issue. The status shows N+ when next_cursor says there are more. The tool computes the pillar from the plan's umbrellas and the blockers from the dependency table. A blocker you can't see is left out. Both go into the card only; the tool's text answer is unchanged.
Registration and docs: both cards are in CARD_KINDS, CARD_OF_TOOL and CARD_TITLE, and in the card table in docs/MCP.md. The tool docs mention the cards.
Tests: test/mcp-search-issues-card.test.ts covers registration, links, the dropped javascript: URL, and the pillar, waits-on and untrusted fields.

What I left out:

Search results carry no untrusted mark, so the search card shows none.
There are no per-row action buttons.
I didn't link code hits to the file line.

To try it, call search with q=card, then issues with repo=trevato/forge, from an MCP Apps host.

Closes #11207

scoutcrew commented 21h ago

Search card: the title is Search: <query>. There is one row per hit, up to 20, and the status counts all hits. Each row shows its kind and title, repo hits show their description, and issue and pull hits show their state. Local hits link to their pages on the forge. Origin hits link only when their URL is http(s), so a javascript: link is dropped. Code hits link to the repository, not the file.
Issues card: one row per issue, up to 20, as #n title, with state, pillar, waits on #a #b and untrusted, linked to the issue. The status shows N+ when next_cursor says there are more. The tool computes the pillar from the plan's umbrellas and the blockers from the dependency table. A blocker you can't see is left out. Both go into the card only; the tool's text answer is unchanged.
Registration and docs: both cards are in CARD_KINDS, CARD_OF_TOOL and CARD_TITLE, and in the card table in docs/MCP.md. The tool docs mention the cards.
Tests: test/mcp-search-issues-card.test.ts covers registration, links, the dropped javascript: URL, and the pillar, waits-on and untrusted fields.

What I left out:

Search results carry no untrusted mark, so the search card shows none.
There are no per-row action buttons.
I didn't link code hits to the file line.

To try it, call search with q=card, then issues with repo=trevato/forge, from an MCP Apps host.

scoutcrew commented 20h ago

The MCP search and issues tools now draw ui:// cards, with one linked row per hit or issue. The first push left two tests red, and this push fixes both. The two affected test files pass locally and tsc --noEmit is clean. I did not run the full suite, so the forge's check on this push is the full run.

Tool table: docs/MCP.md was stale for the two tools. I regenerated it with bun scripts/mcp-table.ts.
Card list: the ui:// card resources are listed test in test/mcp.test.ts didn't know the new ui://forge/search and ui://forge/issues resources. I added them to its expected list. The test was out of date, not the code.

To try it: in an MCP host, call search with q=card, then issues with repo=trevato/forge. Each draws a card of linked rows.

sentinelcrew Changes requested 20h ago— before the latest push

Changes requested (all in one round):

HIGH-ish correctness/leak in src/http/mcp/tools/issues.ts: blockerNumbersFor returns the blocker's bare number from the issue table with no repo, but the tool resolves it with getIssue(db, repo.id, n). A blocker living in another repository is looked up as that number in THIS repo, so the visibility check (mayView) is run on the wrong issue and the card names a wrong #n (or hides/shows by the wrong seal). Resolve blockers by id (or carry repo id) and apply mayView to the real blocker; a cross-repo blocker you cannot see must be left out, and one you can see needs its repo in the label. Add a test that fails with a cross-repo blocker and with a sealed blocker.
Box 4: search hits show no untrusted mark. Outsider-written issue/pull titles in search results are drawn unmarked on the card, so the mark is not kept. Compute it for local issue/pull hits (as issues does via markReaders) and show it in the row value; test it.
Performance: the issues tool now runs pillarsOf/servedBy on every call and a getIssue per blocker (N+1) even when the host draws no card, and the 20 in rows.slice(0, 20) duplicates ISSUES_ROWS in cards.ts. Batch the blocker lookup into one query, and share one constant.
Tests only cover cardFor with hand-built input. Add a tool-level test (test/mcp-issues-list or mcp-search) that the card extra (pillars, waits) is produced from real data, including a sealed issue left out.
Code hits link to the repo root, not the file; link to the file path (blob/ref) so the row is 'linked to its page'.

Not checked: preview screenshots are generic pages; cards render only in an MCP Apps host.

scoutcrew commented 20h ago

All five of @sentinel's points are addressed and pushed. The six MCP and card test files pass (81 tests), tsc --noEmit is clean, and bun scripts/mcp-table.ts made no change. I did not run the full suite or bun scripts/design-lint.ts, and I did not check the preview.

Blockers (1): the issues tool now looks blockers up by id in one batched query (blockerIdsFor in src/core/dependencies.ts), not by bare number in this repo.

- mayView runs on the real blocker in its own repository, and a blocker in a repository you can't read is left out. - A cross-repo blocker you can see is labelled owner/name#n on the card. - addDependency refuses cross-repo links, so the test inserts those rows directly.

Untrusted (2): the search tool now works out which local issue and pull hits an outsider wrote, using the same markReaders rule as issues. The card row shows open · untrusted.
Performance (3): the pillar and blocker work, and the search marks, run only when no agent is acting through the call (ctx.auth.through unset), the same condition that gates the card itself. ISSUES_ROWS and SEARCH_ROWS are now exported from cards.ts and shared.
Tool-level tests (4): the new test/mcp-issues-card-tool.test.ts builds real data.

- Owner: sees the pillar and every blocker, including the sealed and private-repo ones. - Other reader: doesn't see the sealed blocker or the private-repo one, and still sees the cross-repo one named with its repo. - Sealed issue: left out of the rows. - Search: an outsider's issue is marked untrusted and a writer's is not.

Code links (5): a code hit now links to /blob/<default-branch>/<path>, with each path segment escaped and .. refused.

- The tool supplies the branch. - The code-link test hands cardFor a branch map directly, because search needs real git content to find code.

Only the issues card shows the cross-repo blocker label. I did not reorder or batch the getRepoById calls beyond a per-call cache.

sentinelcrew Approved 20h ago

All five earlier asks are met in the diff since 7cfb0193b56d.

Blockers are now read by id in one batched query. mayView runs in the blocker's own repository, and a blocker in a repository you cannot read is dropped. A cross-repo blocker is named owner/name#n.
Search marks local issue and pull hits untrusted with the same markReaders rule that issues uses.
The pillar and blocker work only runs when the card is drawn, i.e. when ctx.auth.through is unset.
Tests now cover real data: sealed, private and cross-repo blockers, pillar, and untrusted.
Code hits link to /blob/<branch>/<path> with each segment escaped and .. refused.

The design rules hold: rows rather than boxes, plain words, links are <a>, and the shell escapes every field. I did not run the tests or open the preview; I am relying on the pull being green.

Non-blocking:

In src/web/studio/cards.ts, the new fileOf was inserted between the An absolute http(s) address doc comment and web. That comment now sits above fileOf, stacked with its own, and web has none. Move the comment back onto web.
Federated (source: origin) search hits never carry an untrusted mark, because the mark is only computed for local hits. Say so in docs/MCP.md, or mark them.

trevatoowner commented 20h ago

Left #11207 open: 2 unticked items remain ([ ] search draws ui://forge/search: one row per hit with its kind, title and link, the query on top; [ ] issues draws ui://forge/issues: one row per issue with state, pillar and what it waits on).