feat(mcp): event-driven push — broadcast + subscribe_annotations (#185 PR3/5)#190
Merged
feat(mcp): event-driven push — broadcast + subscribe_annotations (#185 PR3/5)#190
Conversation
Lays the foundation for MCP-side push (PR3 part 1 of 2):
- New `events` module with `AnnotationEvent { paper_id, annotation_id,
kind, reader: Option<String> }` and a `tokio::sync::broadcast`
channel sized to 256 events. Slow subscribers see `Lagged(n)` on
recv and re-fetch via `list_annotations`; we'd rather drop than
grow unbounded.
- `ScitadelServer` carries the `Sender`. The initial `Receiver` from
`broadcast::channel` is discarded so emit-with-no-subscribers is a
no-op (Sender::send returns SendError, swallowed in `events::emit`).
- `subscribe_events()` hands out a fresh receiver for the upcoming
`subscribe_annotations` tool.
- Every annotation write tool now emits one event:
create_annotation → Created (paper_id from req)
reply_annotation → Replied (paper_id from parent lookup)
update_annotation → Updated (paper_id from id lookup)
delete_annotation → Deleted (paper_id from id lookup BEFORE soft-delete)
mark_seen → MarkedSeen (one event per id, with reader)
mark_thread_seen → MarkedThreadSeen (event on root_id, with reader)
- `tools::lookup_annotation_paper_id` helper resolves paper_id by
annotation id so the server doesn't open the DB inline. Returns
None on missing/deleted rows; the server logs and skips the emit.
Tool descriptions updated to advertise the
`notifications/resources/updated` emission so clients know the tools
are subscribe-aware.
Tests: 5 events module tests (single sub, two subs each get a copy,
no-sub fail-silent, lag-rather-than-block, kind.as_str stability) +
one server-level subscribe_events round-trip test. The actual
emit-on-write wiring is verified visually — each method is a small
glue layer over the already-tested broadcast channel.
Refs #185.
[tape-exempt: MCP-only event channel; no TUI/CLI surface touched]
The subscribe side of #185 PR3. An agent calls `subscribe_annotations(paper_id?, reader)` and receives a resource URI; the server spawns a per-call task that translates every matching `AnnotationEvent` into an MCP-spec `notifications/resources/updated` for the calling peer. URI scheme: - `scitadel://annotations/all` — all papers - `scitadel://annotations/<paper_id>` — paper-scoped Lifecycle: - One spawned task per subscribe call - Task ends on broadcast Sender close (server shutdown) or peer notify failure (client disconnect) — no explicit unsubscribe RPC - Lagged subscribers see one resync update + a tracing warning; agent should re-fetch via list_annotations to recover Pure helpers `subscription_uri` and `event_matches_scope` factor out the URI shape and routing logic so the contract is unit-testable without spawning the broadcast task or mocking a Peer. 3 new tests: URI shape (None / Some / empty), event-matches-scope filter (None matches all, paper-scoped matches only that paper). Refs #185. [tape-exempt: MCP-only push channel; no TUI/CLI surface touched]
…R3 review)
Two soft suggestions from the review, both addressed:
- New end-to-end test drives `create_annotation` through the server
surface and asserts a `Created` event arrives on `subscribe_events()`.
Insurance against a future refactor that reroutes a write tool past
`events::emit`. The other 5 emit sites are mechanically identical so
one good test guards them all.
- `subscribe_annotations(paper_id: Some(""))` now returns an explicit
Err. Without this the caller got a valid-looking
`scitadel://annotations/` URI and a scope filter that would never
match a real event — silent zero-delivery. Treat as caller error.
Refs #185.
[tape-exempt: MCP-only test + validation; no TUI/CLI surface touched]
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the MCP-side push P0 of #185 — the agent loop is now event-driven, not poll-driven.
eventsmodule:AnnotationEvent { paper_id, annotation_id, kind, reader }and a 256-slottokio::sync::broadcastchannel onScitadelServer. Bounded so a slow subscriber sees `Lagged(n)` rather than growing memory unbounded.Commits
Test plan
Out of scope (follow-ups)
[tape-exempt: MCP-only push channel; no TUI/CLI surface touched]
Refs #185.