fix(mcp-store): group mcp tools by read-only vs write/delete

[edgeorgie]

Problem

A user approving an MCP server's tools sees every tool in one flat list, with no way to tell which ones only read data versus which ones can write or delete it. On a server with a dozen-plus tools, delete_issue renders identically to list_issues, so approval becomes a guess rather than an informed decision.

The information needed to fix this already existed and was already being thrown away. MCP servers declare a readOnlyHint annotation on each tool per the MCP spec, the backend already persists it unmodified on MCPServerInstallationTool.annotations, but no serializer field ever surfaced it, so neither frontend could read it.

A prior attempt, #76411, fixed this for the web app only (products/mcp_store) and was closed for inactivity before merging, not on its merits. The issue's own title and screenshots are specifically about PostHog Desktop, which #76411 never touched. Desktop has its own independent tool-list UI (ServerDetailView.tsx) backed by the same backend API, so closing #76236 properly means extending #76411's approach to both surfaces, not just reviving it for web.

Closes #76236

Changes

Shared contract, two independent consumers. The backend exposes one new boolean field; both frontends read it and build their own grouped UI from it. Nothing is shared between the two UI implementations beyond the field itself, which is why both needed a change:

flowchart LR
    A{{"MCP server"}} -->|"declares readOnlyHint"| B["MCPServerInstallationTool.annotations<br/>(stored unmodified, pre-existing)"]
    B --> C["MCPServerInstallationToolSerializer<br/>get_is_read_only()"]
    C -->|"is_read_only: bool"| D[/"Tool list API response"/]
    D --> E["ServerDetailPanel.tsx<br/>web: products/mcp_store"]
    D --> F["ServerDetailView.tsx<br/>desktop: products/desktop"]

    classDef phBlue fill:#1d4aff,stroke:#1d4aff,color:#fff;
    classDef phRed fill:#f54e00,stroke:#f54e00,color:#fff;
    classDef phYellow fill:#f9bd2b,stroke:#f9bd2b,color:#000;
    classDef phGray fill:#e5e7eb,stroke:#c7ccd1,color:#000;

    class A phYellow;
    class B phGray;
    class C phBlue;
    class D phRed;
    class E,F phBlue;

Loading

• Backend: MCPServerInstallationToolSerializer adds is_read_only, a SerializerMethodField derived from annotations.readOnlyHint.
• A missing or falsy hint maps to is_read_only: false (write/delete-capable), never to true. The annotation is server-reported and unverified, so an absent or ambiguous hint has to assume the more dangerous capability. This mirrors the existing policy layer's stance elsewhere in this serializer: an untrusted signal can only tighten restriction, never loosen it.
• An alternative considered and rejected: adding a differently-named top-level field (for example read_only_hint) instead of deriving a normalized is_read_only boolean. Exposing the raw annotation key would leak MCP-spec vocabulary into the API contract and push the trust decision (what counts as "read-only enough to group separately") into every consumer. Deriving one boolean in the serializer keeps that decision in one place and gives both frontends an API contract that's already a plain boolean, not a hint they'd each have to interpret the same way independently.
• Web (products/mcp_store/frontend/scene/ServerDetailPanel.tsx): the flat tool list becomes two LemonCollapse panels, "Read-only tools" and "Write or delete tools", each with a count badge. Both default open.
• Desktop (products/desktop): added groupToolsByReadOnly next to the existing tool-derivation helpers in toolDerivation.ts; ServerDetailView.tsx renders the same two labeled groups through a new ToolGroup component instead of one flat list.
• An empty group renders nothing on both surfaces, so a server reporting no read-only tools looks exactly like it did before this change.

How did you test this code?

Test rationale: This is a display/grouping change with no new state machine, so coverage sits at the three points where behavior actually branches: the serializer's is_read_only derivation, and the new groupToolsByReadOnly helper on desktop. Web already has component-level logic coverage in mcpStoreLogic.test.ts; its fixture was updated for the new field rather than adding a parallel test, matching the existing pattern there.

• Added test_list_tools_reports_is_read_only_from_annotations to products/mcp_store/backend/test/test_api.py, covering readOnlyHint: true, readOnlyHint: false, and missing annotations.
• Added two cases to products/desktop/packages/core/src/mcp-servers/toolDerivation.test.ts for groupToolsByReadOnly (splits correctly, treats missing is_read_only as write/delete).
• Ran pnpm --filter @posthog/core vitest suite for toolDerivation — 6/6 passing.
• Ran pnpm --filter=@posthog/frontend exec jest --testPathPattern=products/mcp_store/frontend — 16 suites / 133 tests passing.
• Ran oxlint on the changed web files — clean.
• python3 -m py_compile on the changed backend files — clean.
• Not run: the Django test suite itself, because this environment has no local Postgres/ClickHouse. The new backend test was written and compiled but not executed against a live database. Also not run: the full-repo tsgo typecheck (OOMs on this machine regardless of this change) and Desktop's @posthog/ui vitest suite (pre-existing failures from an unbuilt @posthog/shared package, unrelated to this change) — confirmed zero new TypeScript errors via a before/after error diff instead.

Release status

• No feature flag controls this change

Docs update

None — internal UI grouping only, no user-facing docs reference tool list layout.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: Claude Code, Claude Sonnet 4.5

• Skills invoked: /improving-drf-endpoints (serializer field gets help_text and @extend_schema_field, matching the checklist for a SerializerMethodField), /writing-tests (new coverage extends the nearest existing test file/block rather than adding a parallel suite), and /writing-pr-descriptions (shaped this body and its Mermaid diagram).
• /adopting-generated-api-types was checked but doesn't apply: this PR adds a serializer field and regenerates api.schemas.ts/generated.ts through the normal pipeline, it does not touch any manual api.get/ApiRequest call sites.
• No duplicate: searched gh pr list --search "mcp tool group" and gh api search/issues?q=repo:PostHog/posthog+is:pr+76236 before starting. Found feat(mcp-store): group installed tools by read and write/delete #76411 (web-only, closed for inactivity, not on merits, discussed above) and no other open attempt.
• Patch coverage: new and changed lines are covered by the tests listed above, except the Desktop ServerDetailView.tsx UI composition itself, which has no existing test file for the component it was changed in (confirmed by searching). No test was added here to avoid introducing a first-of-its-kind UI test inconsistent with the file's current state.

[trunk-io]

Merging to master in this repository is managed by Trunk.

• To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

[parameterai]

Risk: No findings

This increment only relabels the MCP tool grouping headers and adds hover disclaimers on both the web and desktop approval UIs, to address the earlier finding that an unverified server self-attestation was presented as a trust label. No new inputs, data flows, or logic were introduced, and the fix correctly reframes the readOnlyHint group as a server-reported, unverifiable claim on both surfaces.

_{Sentinel reviewed 57e8e7b · Review settings}