Skip to content

Commit 5848bd3

Browse files
authored
feat: Push based cancellation for diagnostic services (#236)
Signed-off-by: David Kornel <kornys@outlook.com>
1 parent ac4f051 commit 5848bd3

17 files changed

Lines changed: 436 additions & 61 deletions

AGENTS.md

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -408,7 +408,7 @@ public class DiagnosticTools {
408408
- **Graceful degradation**: Works without Sampling/Elicitation support (gathers everything, returns raw data)
409409
- **Step failure resilience**: Individual step failures are recorded in `stepsFailed`, workflow continues
410410
- **Progress tracking**: Reports progress via `DiagnosticHelper.sendProgress()`
411-
- **Cancellation**: Checks `DiagnosticHelper.checkCancellation()` between steps
411+
- **Cancellation**: Supports both poll-based (`checkCancellation()` between steps) and push-based (`registerCancellationCallback()` for async operations)
412412
- **No duplication**: Calls existing domain services (KafkaService, StrimziOperatorService, etc.)
413413
- **Configurable token limits**: `mcp.sampling.triage-max-tokens` and `mcp.sampling.analysis-max-tokens`
414414
- **OpenTelemetry tracing**: Gather/triage/analysis methods are annotated with `@WithSpan` for
@@ -420,12 +420,50 @@ public class DiagnosticTools {
420420

421421
Shared MCP framework utilities in `common/src/.../service/DiagnosticHelper.java`:
422422
- `sendProgress(Progress, step, totalSteps, message)` — progress update with descriptive message to MCP client
423-
- `checkCancellation(Cancellation)` — abort if client cancelled
423+
- `checkCancellation(Cancellation)` — poll-based cancellation check; throws if client cancelled (use for synchronous code paths)
424+
- `registerCancellationCallback(Cancellation, AtomicBoolean)` — push-based cancellation callback; immediately sets flag when cancelled (use for async operations like Sampling/Elicitation)
425+
- `checkAsyncCancellation(AtomicBoolean)` — check flag set by callback and throw if cancelled (use after async operations)
424426
- `putIfNotNull(Map, String, Object)` — conditional map insertion
425427
- `elicitSelection(Elicitation, message, propertyName, description, options)` — generic single-select Elicitation
426428
- `extractSamplingText(SamplingResponse)` — safe text extraction from Sampling response
427429
- `MAP_TYPE_REF` — reusable `TypeReference<Map<String, Object>>` for JSON parsing
428430

431+
#### Cancellation Patterns
432+
433+
Diagnostic services support two cancellation mechanisms:
434+
435+
1. **Poll-based** (`checkCancellation()`) — For synchronous operations
436+
- Checks if cancellation occurred since the last check
437+
- Throws immediately if cancelled
438+
- Use between discrete workflow steps (after gathering data, before next phase)
439+
440+
2. **Push-based** (`registerCancellationCallback()`) — For async operations
441+
- Registers a callback that fires immediately when cancellation occurs
442+
- Sets an AtomicBoolean flag that can be checked at any point
443+
- Suitable for Sampling/Elicitation calls that may take seconds to minutes
444+
- Catches cancellation during the async call, not just at poll points
445+
446+
**When to use each:**
447+
- Use **poll-based** for synchronous code paths (gathering data, parsing responses, building reports)
448+
- Use **push-based** for async operations (Sampling, Elicitation, long-running external API calls)
449+
- Can mix both — poll-based for sync phases, push-based for async phases
450+
451+
**Example usage:**
452+
```java
453+
// Register callback once at start of diagnose()
454+
AtomicBoolean cancelled = new AtomicBoolean(false);
455+
DiagnosticHelper.registerCancellationCallback(cancellation, cancelled);
456+
457+
// Check after async operations (sampling/elicitation)
458+
InvestigationAreas areas = performTriage(sampling, ...);
459+
DiagnosticHelper.checkAsyncCancellation(cancelled); // throws if cancelled
460+
461+
// Continue with poll-based checks for sync phases
462+
DiagnosticHelper.checkCancellation(cancellation);
463+
```
464+
465+
See `KafkaClusterDiagnosticService` for complete reference implementation.
466+
429467
### BaseDiagnosticService (common module)
430468

431469
Abstract base class in `common/src/.../service/BaseDiagnosticService.java` that all
@@ -435,8 +473,14 @@ diagnostic services must extend:
435473
- Binds `mcp.sampling.analysis-max-tokens``analysisMaxTokens`
436474
- Binds `mcp.log.tail-lines``defaultTailLines`
437475
- `performSampling(sampling, systemPrompt, data, maxTokens)` — sends a Sampling request and returns extracted text
476+
- `performSampling(sampling, systemPrompt, data, maxTokens, cancelled)` — overload with push-based cancellation support
438477
- `performAnalysis(sampling, systemPrompt, fullData)` — convenience wrapper using `analysisMaxTokens`
478+
- `performAnalysis(sampling, systemPrompt, fullData, cancelled)` — overload with push-based cancellation support
439479
- `performTriage(sampling, systemPrompt, phase1Summary)` — sends triage Sampling, parses JSON response to Map
480+
- `performTriage(sampling, systemPrompt, phase1Summary, cancelled)` — overload with push-based cancellation support
481+
482+
All cancellation-aware overloads accept an optional `AtomicBoolean cancelled` parameter. If non-null,
483+
the flag is checked before and after the async `sendAndAwait()` call, returning null if cancelled.
440484

441485
### NamespaceElicitationHelper (common module)
442486

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99

1010
### Added
1111

12+
- **Push-based cancellation for diagnostics** — Added `DiagnosticHelper.registerCancellationCallback()` for async diagnostic operations (sampling, elicitation). The callback approach catches cancellation immediately via `Cancellation#onCancelled()` instead of polling with `skipProcessingIfCancelled()`, preventing wasted work during long-running LLM calls. Existing `checkCancellation()` remains for synchronous code paths.
1213
- **Elasticsearch/OpenSearch log provider** (`mcp.log.provider=streamshub-elasticsearch`) for querying logs from Elasticsearch or OpenSearch
1314
- **Dev environment manifests and scripts** for deploying Elasticsearch with Fluent Bit on Kind clusters and ECK Operator on OpenShift with automated API key provisioning, ClusterLogForwarder integration, and passthrough TLS routes
1415
- **System tests** for Elasticsearch log provider covering log collection, field mapping, time window queries, and error handling

common/src/main/java/io/streamshub/mcp/common/service/BaseDiagnosticService.java

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import org.jboss.logging.Logger;
1414

1515
import java.util.Map;
16+
import java.util.concurrent.atomic.AtomicBoolean;
1617

1718
/**
1819
* Base class for diagnostic service implementations.
@@ -53,9 +54,32 @@ protected BaseDiagnosticService() {
5354
*/
5455
protected String performSampling(Sampling sampling, String systemPrompt,
5556
Map<String, Object> data, int maxTokens) {
57+
return performSampling(sampling, systemPrompt, data, maxTokens, null);
58+
}
59+
60+
/**
61+
* Send a Sampling request with cancellation support.
62+
*
63+
* <p>Checks the cancellation flag before and after the async sampling call.
64+
* Returns {@code null} if Sampling is unavailable, unsupported, or fails,
65+
* or if cancelled before the call completes.</p>
66+
*
67+
* @param sampling MCP Sampling interface (may be null or unsupported)
68+
* @param systemPrompt the system prompt for the LLM
69+
* @param data the data map to serialize as JSON input
70+
* @param maxTokens maximum tokens for the response
71+
* @param cancelled optional flag set by push-based cancellation callback (may be null)
72+
* @return the extracted text, or null on failure or cancellation
73+
*/
74+
protected String performSampling(Sampling sampling, String systemPrompt,
75+
Map<String, Object> data, int maxTokens,
76+
AtomicBoolean cancelled) {
5677
if (sampling == null || !sampling.isSupported()) {
5778
return null;
5879
}
80+
if (cancelled != null && cancelled.get()) {
81+
return null;
82+
}
5983
try {
6084
String dataJson = objectMapper.writeValueAsString(data);
6185
SamplingResponse response = sampling.requestBuilder()
@@ -64,6 +88,9 @@ protected String performSampling(Sampling sampling, String systemPrompt,
6488
.setMaxTokens(maxTokens)
6589
.build()
6690
.sendAndAwait();
91+
if (cancelled != null && cancelled.get()) {
92+
return null;
93+
}
6794
return DiagnosticHelper.extractSamplingText(response);
6895
} catch (Exception e) {
6996
getLogger().warnf("Sampling failed: %s: %s",
@@ -87,6 +114,24 @@ protected String performAnalysis(Sampling sampling, String systemPrompt,
87114
return performSampling(sampling, systemPrompt, fullData, analysisMaxTokens);
88115
}
89116

117+
/**
118+
* Perform analysis Sampling with cancellation support.
119+
*
120+
* <p>Convenience wrapper that uses {@link #analysisMaxTokens} and checks
121+
* the cancellation flag before and after the sampling call.</p>
122+
*
123+
* @param sampling MCP Sampling interface
124+
* @param systemPrompt the analysis system prompt
125+
* @param fullData the full gathered data map
126+
* @param cancelled optional flag set by push-based cancellation callback (may be null)
127+
* @return the analysis text, or null on failure or cancellation
128+
*/
129+
protected String performAnalysis(Sampling sampling, String systemPrompt,
130+
Map<String, Object> fullData,
131+
AtomicBoolean cancelled) {
132+
return performSampling(sampling, systemPrompt, fullData, analysisMaxTokens, cancelled);
133+
}
134+
90135
/**
91136
* Perform triage Sampling and return the parsed JSON response as a map.
92137
*
@@ -101,7 +146,27 @@ protected String performAnalysis(Sampling sampling, String systemPrompt,
101146
*/
102147
protected Map<String, Object> performTriage(Sampling sampling, String systemPrompt,
103148
Map<String, Object> phase1Summary) {
104-
String text = performSampling(sampling, systemPrompt, phase1Summary, triageMaxTokens);
149+
return performTriage(sampling, systemPrompt, phase1Summary, null);
150+
}
151+
152+
/**
153+
* Perform triage Sampling with cancellation support.
154+
*
155+
* <p>Checks the cancellation flag before and after the sampling call.
156+
* Returns {@code null} if Sampling is unavailable, the response cannot
157+
* be parsed as JSON, or the operation was cancelled. Callers should fall
158+
* back to investigating all areas when null is returned.</p>
159+
*
160+
* @param sampling MCP Sampling interface
161+
* @param systemPrompt the triage system prompt
162+
* @param phase1Summary the Phase 1 summary data map
163+
* @param cancelled optional flag set by push-based cancellation callback (may be null)
164+
* @return the parsed triage response, or null on failure or cancellation
165+
*/
166+
protected Map<String, Object> performTriage(Sampling sampling, String systemPrompt,
167+
Map<String, Object> phase1Summary,
168+
AtomicBoolean cancelled) {
169+
String text = performSampling(sampling, systemPrompt, phase1Summary, triageMaxTokens, cancelled);
105170
if (text == null) {
106171
return null;
107172
}

common/src/main/java/io/streamshub/mcp/common/service/DiagnosticHelper.java

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,12 @@
1111
import io.quarkiverse.mcp.server.ElicitationResponse;
1212
import io.quarkiverse.mcp.server.Progress;
1313
import io.quarkiverse.mcp.server.SamplingResponse;
14+
import io.quarkiverse.mcp.server.ToolCallException;
1415
import org.jboss.logging.Logger;
1516

1617
import java.util.List;
1718
import java.util.Map;
19+
import java.util.concurrent.atomic.AtomicBoolean;
1820

1921
/**
2022
* Shared utilities for diagnostic service implementations.
@@ -66,6 +68,51 @@ public static void checkCancellation(final Cancellation cancellation) {
6668
}
6769
}
6870

71+
/**
72+
* Register a push-based cancellation callback for async operations.
73+
*
74+
* <p>This method sets up a callback that immediately fires when the MCP client
75+
* cancels the operation, setting the provided AtomicBoolean to true. This is
76+
* more responsive than polling with {@link #checkCancellation(Cancellation)}
77+
* and is suitable for async operations like Sampling and Elicitation that may
78+
* take seconds to minutes.</p>
79+
*
80+
* <p>The callback does not throw — it only sets the flag. Callers should check
81+
* the flag before and after expensive operations using {@link #checkAsyncCancellation(AtomicBoolean)}.</p>
82+
*
83+
* <p>Use {@link #checkCancellation(Cancellation)} for synchronous code paths
84+
* where polling is sufficient. Use this method for async operations where
85+
* immediate notification is important to avoid wasted work.</p>
86+
*
87+
* @param cancellation the MCP cancellation (may be null)
88+
* @param cancelledFlag the flag to set when cancellation occurs (must not be null if cancellation is non-null)
89+
*/
90+
public static void registerCancellationCallback(final Cancellation cancellation,
91+
final AtomicBoolean cancelledFlag) {
92+
if (cancellation != null) {
93+
cancellation.onCancelled(reason -> {
94+
LOG.infof("Cancellation received: %s", reason.orElse("no reason provided"));
95+
cancelledFlag.set(true);
96+
});
97+
}
98+
}
99+
100+
/**
101+
* Check if the operation was cancelled during an async operation (sampling/elicitation).
102+
*
103+
* <p>This method checks the flag set by {@link #registerCancellationCallback(Cancellation, AtomicBoolean)}
104+
* and throws if cancellation has occurred. Use this after async operations like Sampling
105+
* to detect cancellation that occurred during the operation.</p>
106+
*
107+
* @param cancelledFlag the flag set by the cancellation callback
108+
* @throws ToolCallException if the operation was cancelled
109+
*/
110+
public static void checkAsyncCancellation(final AtomicBoolean cancelledFlag) {
111+
if (cancelledFlag != null && cancelledFlag.get()) {
112+
throw new ToolCallException("Operation cancelled");
113+
}
114+
}
115+
69116
/**
70117
* Put a value into a map only if it is not null.
71118
*
@@ -98,6 +145,32 @@ public static String elicitSelection(final Elicitation elicitation,
98145
final String propertyName,
99146
final String description,
100147
final List<String> options) {
148+
return elicitSelection(elicitation, message, propertyName, description, options, null);
149+
}
150+
151+
/**
152+
* Ask the user to select a single value from a list via MCP Elicitation with cancellation support.
153+
*
154+
* <p>This is a generic Elicitation wrapper usable for any disambiguation
155+
* (namespace selection, cluster selection, etc.).</p>
156+
*
157+
* @param elicitation the MCP Elicitation interface
158+
* @param message the prompt message shown to the user
159+
* @param propertyName the schema property name (e.g., "namespace")
160+
* @param description the property description
161+
* @param options the list of options to choose from
162+
* @param cancelled optional flag set by push-based cancellation callback (may be null)
163+
* @return the selected value, or null if the user declined, elicitation failed, or operation was cancelled
164+
*/
165+
public static String elicitSelection(final Elicitation elicitation,
166+
final String message,
167+
final String propertyName,
168+
final String description,
169+
final List<String> options,
170+
final AtomicBoolean cancelled) {
171+
if (cancelled != null && cancelled.get()) {
172+
return null;
173+
}
101174
try {
102175
ElicitationResponse response = elicitation.requestBuilder()
103176
.setMessage(message)
@@ -109,6 +182,9 @@ public static String elicitSelection(final Elicitation elicitation,
109182
.build()
110183
.sendAndAwait();
111184

185+
if (cancelled != null && cancelled.get()) {
186+
return null;
187+
}
112188
if (response.actionAccepted()) {
113189
return response.content().getString(propertyName);
114190
}
Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
/*
2+
* Copyright StreamsHub authors.
3+
* License: Apache License 2.0 (see the file LICENSE or http://apache.org/licenses/LICENSE-2.0.html).
4+
*/
5+
package io.streamshub.mcp.common.service;
6+
7+
import io.quarkiverse.mcp.server.Cancellation;
8+
import io.quarkiverse.mcp.server.ToolCallException;
9+
import org.junit.jupiter.api.Test;
10+
import org.mockito.ArgumentCaptor;
11+
12+
import java.util.Optional;
13+
import java.util.concurrent.atomic.AtomicBoolean;
14+
import java.util.function.Consumer;
15+
16+
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
17+
import static org.junit.jupiter.api.Assertions.assertEquals;
18+
import static org.junit.jupiter.api.Assertions.assertFalse;
19+
import static org.junit.jupiter.api.Assertions.assertThrows;
20+
import static org.junit.jupiter.api.Assertions.assertTrue;
21+
import static org.mockito.Mockito.mock;
22+
import static org.mockito.Mockito.verify;
23+
24+
/**
25+
* Unit tests for {@link DiagnosticHelper} cancellation callback registration.
26+
*/
27+
class DiagnosticHelperTest {
28+
29+
DiagnosticHelperTest() {
30+
}
31+
32+
@Test
33+
void registerCancellationCallback_withNullCancellation_doesNotThrow() {
34+
AtomicBoolean cancelled = new AtomicBoolean(false);
35+
assertDoesNotThrow(() -> DiagnosticHelper.registerCancellationCallback(null, cancelled));
36+
assertFalse(cancelled.get(), "Flag should remain false when cancellation is null");
37+
}
38+
39+
@Test
40+
@SuppressWarnings("unchecked")
41+
void registerCancellationCallback_withValidCancellation_setsFlag() {
42+
AtomicBoolean cancelled = new AtomicBoolean(false);
43+
Cancellation cancellation = mock(Cancellation.class);
44+
45+
ArgumentCaptor<Consumer<Optional<String>>> captor =
46+
ArgumentCaptor.forClass(Consumer.class);
47+
48+
DiagnosticHelper.registerCancellationCallback(cancellation, cancelled);
49+
50+
verify(cancellation).onCancelled(captor.capture());
51+
assertFalse(cancelled.get(), "Flag should be false before cancellation fires");
52+
53+
captor.getValue().accept(Optional.empty());
54+
55+
assertTrue(cancelled.get(), "Flag should be true after cancellation fires");
56+
}
57+
58+
@Test
59+
@SuppressWarnings("unchecked")
60+
void registerCancellationCallback_withCancellationReason_setsFlag() {
61+
AtomicBoolean cancelled = new AtomicBoolean(false);
62+
Cancellation cancellation = mock(Cancellation.class);
63+
64+
ArgumentCaptor<Consumer<Optional<String>>> captor =
65+
ArgumentCaptor.forClass(Consumer.class);
66+
67+
DiagnosticHelper.registerCancellationCallback(cancellation, cancelled);
68+
69+
verify(cancellation).onCancelled(captor.capture());
70+
71+
captor.getValue().accept(Optional.of("User cancelled operation"));
72+
73+
assertTrue(cancelled.get(), "Flag should be true after cancellation with reason");
74+
}
75+
76+
@Test
77+
void checkAsyncCancellation_withNullFlag_doesNotThrow() {
78+
assertDoesNotThrow(() -> DiagnosticHelper.checkAsyncCancellation(null));
79+
}
80+
81+
@Test
82+
void checkAsyncCancellation_withFalseFlag_doesNotThrow() {
83+
AtomicBoolean cancelled = new AtomicBoolean(false);
84+
assertDoesNotThrow(() -> DiagnosticHelper.checkAsyncCancellation(cancelled));
85+
}
86+
87+
@Test
88+
void checkAsyncCancellation_withTrueFlag_throws() {
89+
AtomicBoolean cancelled = new AtomicBoolean(true);
90+
Exception exception = assertThrows(
91+
ToolCallException.class,
92+
() -> DiagnosticHelper.checkAsyncCancellation(cancelled)
93+
);
94+
assertEquals("Operation cancelled", exception.getMessage());
95+
}
96+
}

0 commit comments

Comments
 (0)