java: MeshJob Stream subscription — subscribeEvents (Java) - #1051
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughThe PR adds cursor-based job event subscription across C/Rust/Java layers, refactors JobProxy to use ReentrantReadWriteLock for concurrent FFI safety, and provides EventSubscription as a long-polling iterator with optional type filtering and internal event buffering. ChangesJob Event Subscription Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/runtime/java/mcp-mesh-sdk/src/main/java/io/mcpmesh/EventSubscription.java`:
- Around line 132-137: In EventSubscription (check around the code using
seqRaw/seqNum and next_after), reject fractional numeric values before advancing
the cursor: explicitly verify that seqRaw and next_after are integer-valued
(e.g., compare seqRaw.doubleValue() == seqRaw.longValue() or test for integral
subclasses) and throw a MeshException with a clear message if they are
fractional or not numeric; only then set long seq = seqNum.longValue() and
update cursor. Ensure the same check is applied to the next_after handling block
as pointed out (the second occurrence around lines 150-153).
In
`@src/runtime/java/mcp-mesh-sdk/src/main/java/io/mcpmesh/SubscribeOptions.java`:
- Around line 109-111: In SubscribeOptions.Builder.longPoll, validate the
argument by rejecting null and non-positive durations immediately: in the
Builder.longPoll(Duration longPoll) method (class SubscribeOptions.Builder) call
Objects.requireNonNull(longPoll, "...") and then check that longPoll.toMillis()
> 0 (or longPoll.isZero()/isNegative()) and throw an IllegalArgumentException
with a clear message if invalid; return this unchanged on success. This causes
fast-fail behavior for invalid longPoll values instead of deferring the error to
runtime.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ca5a9d44-f94e-465d-b6eb-81553a8a7aba
📒 Files selected for processing (11)
src/runtime/core/include/mcp_mesh_core.hsrc/runtime/core/src/jobs_ffi.rssrc/runtime/java/mcp-mesh-core/src/main/java/io/mcpmesh/core/MeshCore.javasrc/runtime/java/mcp-mesh-sdk/src/main/java/io/mcpmesh/EventSubscription.javasrc/runtime/java/mcp-mesh-sdk/src/main/java/io/mcpmesh/JobProxy.javasrc/runtime/java/mcp-mesh-sdk/src/main/java/io/mcpmesh/MeshJobs.javasrc/runtime/java/mcp-mesh-sdk/src/main/java/io/mcpmesh/SubscribeOptions.javasrc/runtime/java/mcp-mesh-sdk/src/test/java/io/mcpmesh/MeshJobsSubscribeEventsTest.javatests/integration/suites/uc23_meshjob_java/fixtures/long-task-consumer-java/src/main/java/com/example/longtaskconsumer/LongTaskConsumerApplication.javatests/integration/suites/uc23_meshjob_java/fixtures/long-task-provider-java/src/main/java/com/example/longtaskprovider/LongTaskProviderApplication.javatests/integration/suites/uc23_meshjob_java/tc27_subscribe_events_streams_observer_pattern_java/test.yaml
| public Builder longPoll(Duration longPoll) { | ||
| this.longPoll = longPoll; | ||
| return this; |
There was a problem hiding this comment.
Validate longPoll in the builder (non-null and > 0).
longPoll(null) defers failure to a later runtime path, and non-positive durations can cause aggressive empty-page polling in a long-lived subscription loop. Fail fast here.
Proposed fix
+import java.util.Objects;
@@
public Builder longPoll(Duration longPoll) {
- this.longPoll = longPoll;
+ Objects.requireNonNull(longPoll, "SubscribeOptions.longPoll is required");
+ if (longPoll.isZero() || longPoll.isNegative()) {
+ throw new IllegalArgumentException(
+ "SubscribeOptions.longPoll must be > 0 for streaming subscriptions");
+ }
+ this.longPoll = longPoll;
return this;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public Builder longPoll(Duration longPoll) { | |
| this.longPoll = longPoll; | |
| return this; | |
| public Builder longPoll(Duration longPoll) { | |
| Objects.requireNonNull(longPoll, "SubscribeOptions.longPoll is required"); | |
| if (longPoll.isZero() || longPoll.isNegative()) { | |
| throw new IllegalArgumentException( | |
| "SubscribeOptions.longPoll must be > 0 for streaming subscriptions"); | |
| } | |
| this.longPoll = longPoll; | |
| return this; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/runtime/java/mcp-mesh-sdk/src/main/java/io/mcpmesh/SubscribeOptions.java`
around lines 109 - 111, In SubscribeOptions.Builder.longPoll, validate the
argument by rejecting null and non-positive durations immediately: in the
Builder.longPoll(Duration longPoll) method (class SubscribeOptions.Builder) call
Objects.requireNonNull(longPoll, "...") and then check that longPoll.toMillis()
> 0 (or longPoll.isZero()/isNegative()) and throw an IllegalArgumentException
with a clear message if invalid; return this unchanged on success. This causes
fast-fail behavior for invalid longPoll values instead of deferring the error to
runtime.
2747cae to
4e6ab36
Compare
…1053) ## Summary Documentation-only PR covering the six commits shipped since v2.1.0 (MeshJob event-injection trilogy: #1041 / #1043 / #1045 for `recv_event` / `send_event` / `post_event`; #1047 / #1049 / #1051 for `subscribe_events`). - `docs/concepts/jobs.md` — adds two new sections after the existing v2.0 baseline material: **Event injection** (producer/consumer model + recv loop + post pattern + type filtering + synthetic cancel event with grace window + typed errors) and **Stream subscription** (per-call cursor + `next_after` watermark + no-automatic-terminal-detection + multiple-concurrent-subscribers semantics). Both sections carry cross-runtime Python/TS/Java tabbed examples respecting the three distinct iterator shapes (Python async generator, TS async generator, Java blocking `Closeable` iterator). - `docs/concepts/stateful-agents.md` — replaces the stale "Coming soon: mesh-managed event channel" block (which referenced #1032 as roadmap) with an accurate cross-link to the new sections. - `docs/concepts/index.md` — grid card teaser mentions event injection. - `docs/environment-variables.md` — documents `MCP_MESH_JOBPROXY_CACHE_MAX` (default 256) and `MCP_MESH_CANCEL_EVENT_GRACE_MS` (default 200, capped 10000). - `src/core/cli/man/content/` — same env-var content mirrored to `environment.md`; new **Event injection** + **Stream subscription** sections added to `jobs.md`, `jobs_typescript.md`, `jobs_java.md` (each variant carries idiomatic native-runtime code). - `RELEASE_NOTES.md` — `## v2.2.0 (UNRELEASED)` stub at the top, grouping the six commits by feature rather than PR number, mirroring the v2.1.0 entry's voice and structure. `meshctl man` content is **hand-maintained markdown**, not generated from `docs/` — both surfaces were updated to keep them in sync. ## Review Notes Independent review caught 3 substantive WARNINGs (all addressed) and surfaced one pre-existing adjacent inaccuracy (also folded in): 1. **Java `Map.of` foot-gun** — the cancel-event handling example used `Map.of("reason", payloadMap.get("reason"))`, which NPEs if the synthetic cancel event has no `reason` field (the API allows null reasons). Replaced with a null-tolerant pattern using `Objects.toString(payload.get("reason"), "")`. 2. **Misstated Java cancel mechanism** — `jobs_java.md` claimed handlers observe cancel via `InterruptedException`. Per `JobController.java:239-249`, Java's `Thread#sleep` cannot be interrupted by the Tokio cancel token firing — handlers must poll `isCancelled()` between work units OR park on `recvEvent(["cancelled", ...])`. Rewrote the section accurately. 3. **TS double-bang + `as any` cast** — the canonical TS doc example used `job!.recvEvent!(...)` and `(event.payload as any)?.reason`. Replaced with idiomatic early-return narrowing + `Record<string, unknown>` payload typing so the example doesn't teach non-idiomatic TS. 4. **Adjacent pre-existing inaccuracy** — `jobs_java.md:308-310` (outside the diff) carried the same wrong `Thread.sleep` interrupt claim in the Cancellation section. Folded a one-paragraph fix in since the same surface area was already being edited. All API signatures, env-var defaults (256, 200ms, 10000ms cap), tab indentation, sequence-diagram syntax, anchor links, and framing verified accurate against source files. Closes #1052 A follow-up PR (tracked separately) will add a runnable example agent under `examples/` demonstrating the same APIs. ## Test plan - [x] `mkdocs build` clean (no new errors/warnings introduced; pre-existing orphan-page warnings unrelated) - [x] `mkdocs serve` renders new sections cleanly (grid cards, tabbed code, mermaid `sequenceDiagram` blocks) - [x] `meshctl man environment --raw | grep -E 'MCP_MESH_(JOBPROXY|CANCEL_EVENT)'` matches both env vars - [x] `meshctl man jobs [--typescript|--java] --raw` matches new Event injection + Stream subscription sections per runtime - [x] `meshctl man jobs --java --raw | grep InterruptedException` returns only an unrelated `throws` clause in a code sample — the wrong cancel-mechanism claim is gone - [x] `meshctl man jobs --typescript --raw | grep -E '!\.|\bas any\b'` zero matches — non-idiomatic TS scrubbed - [x] `go build ./...` clean (man content compiles via `//go:embed`) - [x] No-deprecated-in-docs rule observed; public-artifact framing observed (no naming of downstream consumers, no behavioral framing) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * MeshJob event injection: post and receive job events across all supported runtimes with typed error handling and configurable cancel-event grace window. * MeshJob stream subscription: non-destructive event observation with independent per-call cursors and watermark filtering. * **Documentation** * Added comprehensive event injection and stream subscription guidance across Python, TypeScript, and Java documentation. * Documented new environment variables for event channel tuning. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/dhyansraj/mcp-mesh/pull/1053?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary Release v2.2.0 — the MeshJob event-injection wave. Rolls up six feature PRs (cross-runtime point-to-point event injection + stream subscription) plus three docs/examples PRs into a stable release: - **MeshJob event injection** — `recv_event` / `send_event` / `post_event` across Python (#1041), TypeScript (#1043), Java (#1045). - **MeshJob stream subscription** — `subscribe_events` async generator (Python #1047, TS #1049) and `EventSubscription` Closeable iterator (Java #1051). - **Docs + examples** — concepts + env vars + meshctl man + release notes (#1053), signature-drift follow-up (#1056), runnable event-injection example agents across all three runtimes (#1058). See `RELEASE_NOTES.md` for the per-feature narrative grouping all six feature PRs. ## Mechanical changes in this PR - `scripts/bump_version.py 2.1.0 2.2.0` — 417 files updated across 36 categories (Cargo manifests, pyproject.toml, package.json, helm charts, scaffold templates, man content, test config, release workflow versions). - `helm dependency update helm/mcp-mesh-core` — Chart.lock regenerated. - `cargo generate-lockfile` (src/runtime/core) — Cargo.lock refreshed. - `RELEASE_NOTES.md` — `## v2.2.0 (UNRELEASED)` → `## v2.2.0 (2026-05-19)`, top-of-file Full Changelog link bumped to `v2.2.0...HEAD`, new `v2.1.0...v2.2.0` link added above the v2.2.0 heading. Net diff: 398 files changed, +613 / -611. Mostly one-line version bumps. Closes #1059 ## Test plan - [x] Dry-run `bump_version.py` matched expected scope (537 files / 36 categories) - [x] No stray `2.1.0` mcp-mesh-internal references left after bump (sanity grep — only transitive npm-dep matches remain) - [x] `helm dependency update` + `cargo generate-lockfile` reminders followed - [x] All upstream feature PRs (#1041 → #1058) merged and validated end-to-end before this release was cut - [ ] **After merge**: tag `v2.2.0` pushed to origin (HOLD for user inspection of merged main) - [ ] **After tag**: publish workflow fired (HOLD for explicit user go-ahead) Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Summary
MeshJobs.subscribeEvents(jobId, options) -> EventSubscriptionreturning a blockingCloseableiterator overMap<String, Object>events.subscribe_events) and ts: MeshJob Stream subscription — subscribeEvents (TypeScript) #1049 (TSsubscribeEvents). Multiple subscribers can observe the same job's events independently via per-call cursors.mesh_job_proxy_list_eventsreturning a{events, next_after}JSON envelope; JNR binding inMeshCore.java; Java SDKEventSubscription+SubscribeOptions+MeshJobs.subscribeEventsoverloads reusing the existing LinkedHashMap-based LRUproxyCachefrom java: MeshJob event injection parity — recvEvent/sendEvent/postEvent #1045; integration testtc27_javaextending the long-task fixtures withrun_until_done(provider) andcommission_subscribe_observer(consumer) capabilities.EventSubscription implements Closeable. Try-with-resources gives Java something Python'sasyncio.CancelledErrorprovides naturally and TS can't replicate withoutAbortControllerplumbing — clean iterator shutdown that stops issuing new FFI long-polls.Review Notes
Independent review caught 1 BLOCKER and 5 substantive WARNINGs (all addressed in the amended commit):
JobProxylock serialized concurrent subscribers (BLOCKER) — everyJobProxymethod was wrapped in a singlesynchronizedblock. TwoEventSubscriptioninstances sharing one cachedJobProxywould serialize their 30-second long-polls. Replaced withReentrantReadWriteLock:close()takes the write lock;listEvents/sendEvent/status/await/canceltake the read lock and can run concurrently. The Rust core'sJobProxyisSync(Arc + String, no internal mutex) so concurrent FFI calls are safe — empirically validated by the fact that Python/TS verticals have always worked without any client-side lock.closedfield neededvolatile— without it, cross-threadclose()calls from arbitrary threads may not be observed by the iterator'swhile (!closed)loop. Textbook JMM hole. Nowvolatile.SubscribeOptions.Builderlackedafter >= 0validation — fail-fast belongs at the builder. Now throwsIllegalArgumentExceptionfor negative values.EventSubscriptionon poster exception —Thread.sleep/postEventcalls could throw without closing the subscription. Now wrapped in try-with-resources.RuntimeException—JobNotFoundException/ malformed payload errors produced emptyobserved_eventswith no diagnostic. Now logged via slf4j.close()doesn't interrupt an in-flight FFI long-poll — the original JavaDoc overstated the close() contract as "the Java analog of Python'sasyncio.CancelledErrorpropagation". Rewrote honestly: close() flips a flag that stops future long-polls but does NOT interrupt the in-flight one (true interruption would require plumbing cancellation through FFI — out of scope). Documented the practical guidance: shortlongPollfor rapid shutdown.Other findings (4 INFOs + 2 cosmetic WARNINGs) skipped as not load-bearing.
Closes #1050
Test plan
SubscribeOptionsbuilder validation, 2 forReentrantReadWriteLocksemantics — including a behavioral test that asserts two threads can enter the read lock simultaneously viagetReadLockCount() == 2)src-tests12/12 pass (image rebuilt with the new FFI symbol)parse_ffi_timeout_secsreused at FFI boundary (NaN/Inf/negative-sentinel)JobProxycache reused from java: MeshJob event injection parity — recvEvent/sendEvent/postEvent #1045 (no new cache)typesfilter forwarded end-to-end (no client-side post-filter)next_afterwatermark advances cursor on empty pages (parity with MeshJob: Stream subscription — subscribe_events (Python) #1047 Python and ts: MeshJob Stream subscription — subscribeEvents (TypeScript) #1049 TS)Boolean-seqand missing-seqrejection (parity with sibling verticals)Summary by CodeRabbit
New Features
subscribeEvents()method with configurable options for cursor positioning and timeout behavior.Bug Fixes & Improvements