Skip to content

MCP resources: core's machinery assumes install-wide resources, so a per-user fork resource lists twice and cannot be broadcast #823

Description

@JohnD-EE

Every MCP resource Sunrise has shipped is the same for everybody on the install: the agent list, the workflow list, knowledge search, pattern detail. Two pieces of the machinery around resources were built on that assumption, and a fork whose resources belong to one person meets both.

Found building two per-user resources in a fork (resparkable://today, resparkable://project/{slug}) on the registerMcpResourceHandler seam from #563. Neither half is a trust boundary: the data behind both is correctly scoped, and both reads refuse cleanly. The cost is a confusing list entry and a permanently stale subscriber.

(a) A template row also lists as a concrete resource

listMcpResourceTemplates() filters rows on the placeholder:

return rows.filter((r) => /\{[^}]+\}/.test(r.uri) || r.uri.includes('?'))

listMcpResources() eight lines away selects every enabled row with no filter at all, so a row whose URI is …/project/{slug} appears in both resources/list and resources/templates/list.

A client that renders the first offers an entry that can only be read literally. Doing so sends resources/read with the literal {slug}, readMcpResource exact-matches the row, the handler cannot parse a slug out of it, and the conversation gets a "use the slug from its URL" sentence attached as though it were the resource.

Sunrise has never hit this because none of its own four seeded rows is a template. A fork cannot fix it: both functions are core's.

Ask: give listMcpResources() the inverse of the filter listMcpResourceTemplates() already has.

(b) A subscription is accepted that nothing can ever fire

isRegisteredMcpResourceUri() accepts a concrete URI by exact match and a template by prefix, so resources/subscribe succeeds for both of the fork's resources. Nothing then fires, because broadcastMcpResourceUpdated(uri) takes a URI and notifies every subscriber of it:

const recipients = manager.getSubscribers(uri);

There is no notion of whose data changed. That is correct for an install-wide resource and unusable for a per-user one: firing …://today when one person captures a thought would signal it to every other subscriber of that URI. So the fork deliberately does not wire it, and a subscriber holds its first snapshot forever — which is the misleading subscription the accept-check's own doc comment says it exists to prevent.

Ask: either let a broadcast name a user, or let a resource type declare that it cannot be broadcast for so resources/subscribe refuses it honestly.

Effort

(a) is one .filter() mirroring the function next to it. (b) is a design question rather than a patch: per-user broadcast needs the session manager to know which subscriber is which person, so splitting it out is reasonable if (a) should land on its own.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions