diff --git a/.lastmerge b/.lastmerge index c5649a512..ad028f197 100644 --- a/.lastmerge +++ b/.lastmerge @@ -1 +1 @@ -062b61c8aa63b9b5d45fa1d7b01723e6660ffa83 +485ea5ed1ce43125075bab2f3d2681f1816a4f9a \ No newline at end of file diff --git a/src/main/java/com/github/copilot/sdk/CliServerManager.java b/src/main/java/com/github/copilot/sdk/CliServerManager.java index b2a798ada..217699986 100644 --- a/src/main/java/com/github/copilot/sdk/CliServerManager.java +++ b/src/main/java/com/github/copilot/sdk/CliServerManager.java @@ -110,6 +110,28 @@ ProcessInfo startCliServer() throws IOException, InterruptedException { pb.environment().put("COPILOT_SDK_AUTH_TOKEN", options.getGitHubToken()); } + // Set telemetry environment variables if configured + if (options.getTelemetry() != null) { + var telemetry = options.getTelemetry(); + pb.environment().put("COPILOT_OTEL_ENABLED", "true"); + if (telemetry.getOtlpEndpoint() != null) { + pb.environment().put("OTEL_EXPORTER_OTLP_ENDPOINT", telemetry.getOtlpEndpoint()); + } + if (telemetry.getFilePath() != null) { + pb.environment().put("COPILOT_OTEL_FILE_EXPORTER_PATH", telemetry.getFilePath()); + } + if (telemetry.getExporterType() != null) { + pb.environment().put("COPILOT_OTEL_EXPORTER_TYPE", telemetry.getExporterType()); + } + if (telemetry.getSourceName() != null) { + pb.environment().put("COPILOT_OTEL_SOURCE_NAME", telemetry.getSourceName()); + } + if (telemetry.getCaptureContent() != null) { + pb.environment().put("OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT", + telemetry.getCaptureContent() ? "true" : "false"); + } + } + Process process = pb.start(); // Forward stderr to logger in background diff --git a/src/main/java/com/github/copilot/sdk/CopilotSession.java b/src/main/java/com/github/copilot/sdk/CopilotSession.java index 452e82671..820f201bc 100644 --- a/src/main/java/com/github/copilot/sdk/CopilotSession.java +++ b/src/main/java/com/github/copilot/sdk/CopilotSession.java @@ -10,9 +10,10 @@ import java.util.Collections; import java.util.List; import java.util.Map; -import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; @@ -113,7 +114,7 @@ public final class CopilotSession implements AutoCloseable { private volatile String sessionId; private volatile String workspacePath; private final JsonRpcClient rpc; - private final Set> eventHandlers = ConcurrentHashMap.newKeySet(); + private final CopyOnWriteArrayList> eventHandlers = new CopyOnWriteArrayList<>(); private final Map toolHandlers = new ConcurrentHashMap<>(); private final AtomicReference permissionHandler = new AtomicReference<>(); private final AtomicReference userInputHandler = new AtomicReference<>(); @@ -121,6 +122,13 @@ public final class CopilotSession implements AutoCloseable { private volatile EventErrorHandler eventErrorHandler; private volatile EventErrorPolicy eventErrorPolicy = EventErrorPolicy.PROPAGATE_AND_LOG_ERRORS; + /** + * Single-threaded executor that serializes event dispatch. Events are enqueued + * by {@link #dispatchEvent} and processed one at a time, preserving arrival + * order and ensuring handlers are never called concurrently. + */ + private final ExecutorService eventDispatcher; + /** Tracks whether this session instance has been terminated via close(). */ private volatile boolean isTerminated = false; @@ -156,6 +164,11 @@ public final class CopilotSession implements AutoCloseable { this.sessionId = sessionId; this.rpc = rpc; this.workspacePath = workspacePath; + this.eventDispatcher = Executors.newSingleThreadExecutor(r -> { + var t = new Thread(r, "copilot-session-events"); + t.setDaemon(true); + return t; + }); } /** @@ -563,20 +576,16 @@ public Closeable on(Class eventType, Consume /** * Dispatches an event to all registered handlers. *

- * This is called internally when events are received from the server. Each - * handler is invoked in its own try/catch block. Errors are always logged at - * {@link Level#WARNING}. Whether dispatch continues after a handler error - * depends on the configured {@link EventErrorPolicy}: - *

    - *
  • {@link EventErrorPolicy#PROPAGATE_AND_LOG_ERRORS} (default) — dispatch - * stops after the first error
    • - *
  • {@link EventErrorPolicy#SUPPRESS_AND_LOG_ERRORS} — remaining handlers - * still execute
    • - *
+ * This is called internally when events are received from the server. Broadcast + * request events (protocol v3) are handled concurrently and immediately. + * User-registered handlers are invoked serially on a single background thread, + * preserving event arrival order and ensuring handlers are never called + * concurrently with each other on the same session. *

- * The configured {@link EventErrorHandler} is always invoked (if set), - * regardless of the policy. If the error handler itself throws, dispatch stops - * regardless of policy and the error is logged at {@link Level#SEVERE}. + * Handler exceptions are caught, logged, and do not halt delivery of subsequent + * events. The configured {@link EventErrorHandler} is invoked when set. Whether + * remaining handlers for the same event execute depends on the configured + * {@link EventErrorPolicy}. * * @param event * the event to dispatch @@ -584,28 +593,35 @@ public Closeable on(Class eventType, Consume * @see #setEventErrorPolicy(EventErrorPolicy) */ void dispatchEvent(AbstractSessionEvent event) { - // Handle broadcast request events (protocol v3) before dispatching to user - // handlers. These are fire-and-forget: the response is sent asynchronously. + // Handle broadcast request events (protocol v3) concurrently (fire-and-forget). + // Done outside the serial queue so a stalled broadcast handler doesn't block + // user event delivery. handleBroadcastEventAsync(event); - for (Consumer handler : eventHandlers) { - try { - handler.accept(event); - } catch (Exception e) { - LOG.log(Level.WARNING, "Error in event handler", e); - EventErrorHandler errorHandler = this.eventErrorHandler; - if (errorHandler != null) { + // Enqueue for serial processing by user handlers on the background thread. + // If the executor has been shut down (session closed), silently drop. + if (!eventDispatcher.isShutdown()) { + eventDispatcher.execute(() -> { + for (Consumer handler : eventHandlers) { try { - errorHandler.handleError(event, e); - } catch (Exception errorHandlerException) { - LOG.log(Level.SEVERE, "Error in event error handler", errorHandlerException); - break; // error handler itself failed — stop regardless of policy + handler.accept(event); + } catch (Exception e) { + LOG.log(Level.WARNING, "Error in event handler", e); + EventErrorHandler errorHandler = this.eventErrorHandler; + if (errorHandler != null) { + try { + errorHandler.handleError(event, e); + } catch (Exception errorHandlerException) { + LOG.log(Level.SEVERE, "Error in event error handler", errorHandlerException); + break; // error handler itself failed — stop regardless of policy + } + } + if (eventErrorPolicy == EventErrorPolicy.PROPAGATE_AND_LOG_ERRORS) { + break; + } } } - if (eventErrorPolicy == EventErrorPolicy.PROPAGATE_AND_LOG_ERRORS) { - break; - } - } + }); } } @@ -708,6 +724,12 @@ private void executePermissionAndRespondAsync(String requestId, PermissionReques var invocation = new PermissionInvocation(); invocation.setSessionId(sessionId); handler.handle(permissionRequest, invocation).thenAccept(result -> { + // If the handler returns no-result, leave the request unanswered + // (another client in a multi-client scenario may handle it). + if (PermissionRequestResultKind.NO_RESULT + .equals(new PermissionRequestResultKind(result.getKind()))) { + return; + } try { rpc.invoke("session.permissions.handlePendingPermissionRequest", Map.of("sessionId", sessionId, "requestId", requestId, "result", result), Object.class); @@ -982,6 +1004,39 @@ public CompletableFuture abort() { return rpc.invoke("session.abort", Map.of("sessionId", sessionId), Void.class); } + /** + * Changes the model for this session. + *

+ * The new model takes effect for the next message. Conversation history is + * preserved. + * + * 

{@code
+     * session.setModel("gpt-4.1").get();
+     * session.setModel("claude-sonnet-4.6", "high").get();
+     * }
+ * + * @param model + * the model ID to switch to (e.g., {@code "gpt-4.1"}) + * @param reasoningEffort + * the reasoning effort level (e.g., {@code "low"}, {@code "medium"}, + * {@code "high"}, {@code "xhigh"}), or {@code null} to use the + * model's default + * @return a future that completes when the model switch is acknowledged + * @throws IllegalStateException + * if this session has been terminated + * @since 1.0.12 + */ + public CompletableFuture setModel(String model, String reasoningEffort) { + ensureNotTerminated(); + var params = new java.util.HashMap(); + params.put("sessionId", sessionId); + params.put("modelId", model); + if (reasoningEffort != null) { + params.put("reasoningEffort", reasoningEffort); + } + return rpc.invoke("session.model.switchTo", params, Void.class); + } + /** * Changes the model for this session. *

@@ -1000,8 +1055,7 @@ public CompletableFuture abort() { * @since 1.0.11 */ public CompletableFuture setModel(String model) { - ensureNotTerminated(); - return rpc.invoke("session.model.switchTo", Map.of("sessionId", sessionId, "modelId", model), Void.class); + return setModel(model, null); } /** @@ -1165,6 +1219,10 @@ public void close() { isTerminated = true; } + // Shut down the event dispatcher: no new events will be accepted, but + // already-queued events will still be delivered to handlers. + eventDispatcher.shutdown(); + try { rpc.invoke("session.destroy", Map.of("sessionId", sessionId), Void.class).get(5, TimeUnit.SECONDS); } catch (Exception e) { @@ -1192,4 +1250,18 @@ private record AgentGetCurrentResponse(@JsonProperty("agent") AgentInfo agent) { private record AgentSelectResponse(@JsonProperty("agent") AgentInfo agent) { } + /** + * Package-private test helper: submits a barrier task to the event dispatcher + * and blocks until all previously queued events have been processed. + *

+ * This ensures that any events dispatched before this call have been fully + * delivered to all registered handlers. + */ + void awaitEventDispatch() throws Exception { + if (!eventDispatcher.isShutdown()) { + eventDispatcher.submit(() -> { + }).get(5, TimeUnit.SECONDS); + } + } + } diff --git a/src/main/java/com/github/copilot/sdk/events/ExternalToolRequestedEvent.java b/src/main/java/com/github/copilot/sdk/events/ExternalToolRequestedEvent.java index 8eb11f5b8..fdf8db197 100644 --- a/src/main/java/com/github/copilot/sdk/events/ExternalToolRequestedEvent.java +++ b/src/main/java/com/github/copilot/sdk/events/ExternalToolRequestedEvent.java @@ -38,6 +38,7 @@ public void setData(ExternalToolRequestedData data) { @JsonIgnoreProperties(ignoreUnknown = true) public record ExternalToolRequestedData(@JsonProperty("requestId") String requestId, @JsonProperty("sessionId") String sessionId, @JsonProperty("toolCallId") String toolCallId, - @JsonProperty("toolName") String toolName, @JsonProperty("arguments") Object arguments) { + @JsonProperty("toolName") String toolName, @JsonProperty("arguments") Object arguments, + @JsonProperty("traceparent") String traceparent, @JsonProperty("tracestate") String tracestate) { } } diff --git a/src/main/java/com/github/copilot/sdk/events/SessionModelChangeEvent.java b/src/main/java/com/github/copilot/sdk/events/SessionModelChangeEvent.java index 57d0b5499..8b992114f 100644 --- a/src/main/java/com/github/copilot/sdk/events/SessionModelChangeEvent.java +++ b/src/main/java/com/github/copilot/sdk/events/SessionModelChangeEvent.java @@ -33,6 +33,8 @@ public void setData(SessionModelChangeData data) { @JsonIgnoreProperties(ignoreUnknown = true) public record SessionModelChangeData(@JsonProperty("previousModel") String previousModel, - @JsonProperty("newModel") String newModel) { + @JsonProperty("newModel") String newModel, + @JsonProperty("previousReasoningEffort") String previousReasoningEffort, + @JsonProperty("reasoningEffort") String reasoningEffort) { } } diff --git a/src/main/java/com/github/copilot/sdk/json/CopilotClientOptions.java b/src/main/java/com/github/copilot/sdk/json/CopilotClientOptions.java index 4fd55d3ba..d8725a3d6 100644 --- a/src/main/java/com/github/copilot/sdk/json/CopilotClientOptions.java +++ b/src/main/java/com/github/copilot/sdk/json/CopilotClientOptions.java @@ -42,11 +42,13 @@ public class CopilotClientOptions { private String cliUrl; private String logLevel = "info"; private boolean autoStart = true; - private boolean autoRestart = true; + @Deprecated + private boolean autoRestart; private Map environment; private String gitHubToken; private Boolean useLoggedInUser; private Supplier>> onListModels; + private TelemetryConfig telemetry; /** * Gets the path to the Copilot CLI executable. @@ -236,8 +238,11 @@ public CopilotClientOptions setAutoStart(boolean autoStart) { /** * Returns whether the client should automatically restart the server on crash. * - * @return {@code true} to auto-restart (default), {@code false} otherwise + * @return {@code false} always; this option has no effect + * @deprecated {@code autoRestart} has no effect and will be removed in a future + * release. */ + @Deprecated public boolean isAutoRestart() { return autoRestart; } @@ -247,9 +252,12 @@ public boolean isAutoRestart() { * crashes unexpectedly. * * @param autoRestart - * {@code true} to auto-restart, {@code false} otherwise + * ignored; this option has no effect * @return this options instance for method chaining + * @deprecated {@code autoRestart} has no effect and will be removed in a future + * release. */ + @Deprecated public CopilotClientOptions setAutoRestart(boolean autoRestart) { this.autoRestart = autoRestart; return this; @@ -378,6 +386,34 @@ public CopilotClientOptions setOnListModels(Supplier + * When set to a non-{@code null} instance, the CLI server is started with + * OpenTelemetry instrumentation enabled. The individual properties of + * {@link TelemetryConfig} map to specific environment variables consumed by the + * CLI. + * + * @param telemetry + * the telemetry configuration, or {@code null} to disable telemetry + * @return this options instance for method chaining + * @since 1.0.12 + */ + public CopilotClientOptions setTelemetry(TelemetryConfig telemetry) { + this.telemetry = telemetry; + return this; + } + /** * Creates a shallow clone of this {@code CopilotClientOptions} instance. *

@@ -404,6 +440,7 @@ public CopilotClientOptions clone() { copy.gitHubToken = this.gitHubToken; copy.useLoggedInUser = this.useLoggedInUser; copy.onListModels = this.onListModels; + copy.telemetry = this.telemetry; return copy; } } diff --git a/src/main/java/com/github/copilot/sdk/json/PermissionRequestResult.java b/src/main/java/com/github/copilot/sdk/json/PermissionRequestResult.java index 3d7390f03..b9f6731ea 100644 --- a/src/main/java/com/github/copilot/sdk/json/PermissionRequestResult.java +++ b/src/main/java/com/github/copilot/sdk/json/PermissionRequestResult.java @@ -24,6 +24,8 @@ * no handler and couldn't ask user *

  • {@link PermissionRequestResultKind#DENIED_INTERACTIVELY_BY_USER} — denied * by the user interactively
    • + *
  • {@link PermissionRequestResultKind#NO_RESULT} — leave the request + * unanswered (another client may handle it)
    • * * * @see PermissionHandler diff --git a/src/main/java/com/github/copilot/sdk/json/PermissionRequestResultKind.java b/src/main/java/com/github/copilot/sdk/json/PermissionRequestResultKind.java index f85a0df1c..64c4e226c 100644 --- a/src/main/java/com/github/copilot/sdk/json/PermissionRequestResultKind.java +++ b/src/main/java/com/github/copilot/sdk/json/PermissionRequestResultKind.java @@ -51,6 +51,18 @@ public final class PermissionRequestResultKind { public static final PermissionRequestResultKind DENIED_INTERACTIVELY_BY_USER = new PermissionRequestResultKind( "denied-interactively-by-user"); + /** + * Indicates that the permission request should be left unanswered (no result). + *

    + * When a handler returns this kind, the pending permission request is not + * responded to, allowing another client (in a multi-client scenario) to handle + * it. + *

    + * + * @since 1.0.12 + */ + public static final PermissionRequestResultKind NO_RESULT = new PermissionRequestResultKind("no-result"); + private final String value; /** diff --git a/src/main/java/com/github/copilot/sdk/json/TelemetryConfig.java b/src/main/java/com/github/copilot/sdk/json/TelemetryConfig.java new file mode 100644 index 000000000..dfbeb31c8 --- /dev/null +++ b/src/main/java/com/github/copilot/sdk/json/TelemetryConfig.java @@ -0,0 +1,160 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + *--------------------------------------------------------------------------------------------*/ + +package com.github.copilot.sdk.json; + +/** + * OpenTelemetry configuration for the Copilot CLI server. + *

    + * When set on {@link CopilotClientOptions#setTelemetry(TelemetryConfig)}, the + * CLI server is started with OpenTelemetry instrumentation enabled. Each + * property maps directly to an environment variable consumed by the CLI. + * + *

    Example Usage

    + * + * 
    {@code
+ * var telemetry = new TelemetryConfig().setOtlpEndpoint("http://localhost:4317").setExporterType("otlp-http");
+ *
+ * var options = new CopilotClientOptions().setTelemetry(telemetry);
+ * }
    + * + * @see CopilotClientOptions#setTelemetry(TelemetryConfig) + * @since 1.0.12 + */ +public final class TelemetryConfig { + + private String otlpEndpoint; + private String filePath; + private String exporterType; + private String sourceName; + private Boolean captureContent; + + /** + * Gets the OTLP exporter endpoint URL. + *

    + * Maps to the {@code OTEL_EXPORTER_OTLP_ENDPOINT} environment variable. + * + * @return the OTLP endpoint URL, or {@code null} if not set + */ + public String getOtlpEndpoint() { + return otlpEndpoint; + } + + /** + * Sets the OTLP exporter endpoint URL. + *

    + * Maps to the {@code OTEL_EXPORTER_OTLP_ENDPOINT} environment variable. + * + * @param otlpEndpoint + * the OTLP endpoint URL + * @return this config for method chaining + */ + public TelemetryConfig setOtlpEndpoint(String otlpEndpoint) { + this.otlpEndpoint = otlpEndpoint; + return this; + } + + /** + * Gets the file path for the file exporter. + *

    + * Maps to the {@code COPILOT_OTEL_FILE_EXPORTER_PATH} environment variable. + * + * @return the file exporter path, or {@code null} if not set + */ + public String getFilePath() { + return filePath; + } + + /** + * Sets the file path for the file exporter. + *

    + * Maps to the {@code COPILOT_OTEL_FILE_EXPORTER_PATH} environment variable. + * + * @param filePath + * the file exporter path + * @return this config for method chaining + */ + public TelemetryConfig setFilePath(String filePath) { + this.filePath = filePath; + return this; + } + + /** + * Gets the exporter type ({@code "otlp-http"} or {@code "file"}). + *

    + * Maps to the {@code COPILOT_OTEL_EXPORTER_TYPE} environment variable. + * + * @return the exporter type, or {@code null} if not set + */ + public String getExporterType() { + return exporterType; + } + + /** + * Sets the exporter type ({@code "otlp-http"} or {@code "file"}). + *

    + * Maps to the {@code COPILOT_OTEL_EXPORTER_TYPE} environment variable. + * + * @param exporterType + * the exporter type + * @return this config for method chaining + */ + public TelemetryConfig setExporterType(String exporterType) { + this.exporterType = exporterType; + return this; + } + + /** + * Gets the source name for telemetry spans. + *

    + * Maps to the {@code COPILOT_OTEL_SOURCE_NAME} environment variable. + * + * @return the source name, or {@code null} if not set + */ + public String getSourceName() { + return sourceName; + } + + /** + * Sets the source name for telemetry spans. + *

    + * Maps to the {@code COPILOT_OTEL_SOURCE_NAME} environment variable. + * + * @param sourceName + * the source name + * @return this config for method chaining + */ + public TelemetryConfig setSourceName(String sourceName) { + this.sourceName = sourceName; + return this; + } + + /** + * Gets whether to capture message content as part of telemetry. + *

    + * Maps to the {@code OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT} + * environment variable. + * + * @return {@code true} to capture content, {@code false} to suppress it, or + * {@code null} to use the default + */ + public Boolean getCaptureContent() { + return captureContent; + } + + /** + * Sets whether to capture message content as part of telemetry. + *

    + * Maps to the {@code OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT} + * environment variable. + * + * @param captureContent + * {@code true} to capture, {@code false} to suppress + * @return this config for method chaining + */ + public TelemetryConfig setCaptureContent(Boolean captureContent) { + this.captureContent = captureContent; + return this; + } +} diff --git a/src/main/java/com/github/copilot/sdk/json/ToolDefinition.java b/src/main/java/com/github/copilot/sdk/json/ToolDefinition.java index 9b3087a42..37ec085ec 100644 --- a/src/main/java/com/github/copilot/sdk/json/ToolDefinition.java +++ b/src/main/java/com/github/copilot/sdk/json/ToolDefinition.java @@ -52,6 +52,10 @@ * when {@code true}, indicates that this tool intentionally * overrides a built-in CLI tool with the same name; {@code null} or * {@code false} means the tool is purely custom + * @param skipPermission + * when {@code true}, the tool is executed without triggering a + * permission prompt; {@code null} or {@code false} means the default + * permission behaviour applies * @see SessionConfig#setTools(java.util.List) * @see ToolHandler * @since 1.0.0 @@ -59,7 +63,8 @@ @JsonInclude(JsonInclude.Include.NON_NULL) public record ToolDefinition(@JsonProperty("name") String name, @JsonProperty("description") String description, @JsonProperty("parameters") Object parameters, @JsonIgnore ToolHandler handler, - @JsonProperty("overridesBuiltInTool") Boolean overridesBuiltInTool) { + @JsonProperty("overridesBuiltInTool") Boolean overridesBuiltInTool, + @JsonProperty("skipPermission") Boolean skipPermission) { /** * Creates a tool definition with a JSON schema for parameters. @@ -79,7 +84,7 @@ public record ToolDefinition(@JsonProperty("name") String name, @JsonProperty("d */ public static ToolDefinition create(String name, String description, Map schema, ToolHandler handler) { - return new ToolDefinition(name, description, schema, handler, null); + return new ToolDefinition(name, description, schema, handler, null, null); } /** @@ -103,6 +108,30 @@ public static ToolDefinition create(String name, String description, Map schema, ToolHandler handler) { - return new ToolDefinition(name, description, schema, handler, true); + return new ToolDefinition(name, description, schema, handler, true, null); + } + + /** + * Creates a tool definition that skips the permission prompt. + *

    + * Use this factory method when your tool is safe to run without a permission + * confirmation from the user. Setting {@code skipPermission} to {@code true} + * signals to the CLI that no permission prompt should be shown before executing + * this tool. + * + * @param name + * the unique name of the tool + * @param description + * a description of what the tool does + * @param schema + * the JSON Schema as a {@code Map} + * @param handler + * the handler function to execute when invoked + * @return a new tool definition with the skip-permission flag set + * @since 1.0.12 + */ + public static ToolDefinition createSkipPermission(String name, String description, Map schema, + ToolHandler handler) { + return new ToolDefinition(name, description, schema, handler, null, true); } } diff --git a/src/test/java/com/github/copilot/sdk/PermissionRequestResultKindTest.java b/src/test/java/com/github/copilot/sdk/PermissionRequestResultKindTest.java index b21f96e83..e49b532ec 100644 --- a/src/test/java/com/github/copilot/sdk/PermissionRequestResultKindTest.java +++ b/src/test/java/com/github/copilot/sdk/PermissionRequestResultKindTest.java @@ -28,6 +28,7 @@ void wellKnownKinds_haveExpectedValues() { PermissionRequestResultKind.DENIED_COULD_NOT_REQUEST_FROM_USER.getValue()); assertEquals("denied-interactively-by-user", PermissionRequestResultKind.DENIED_INTERACTIVELY_BY_USER.getValue()); + assertEquals("no-result", PermissionRequestResultKind.NO_RESULT.getValue()); } @Test diff --git a/src/test/java/com/github/copilot/sdk/SessionEventHandlingTest.java b/src/test/java/com/github/copilot/sdk/SessionEventHandlingTest.java index 2e9da4fd2..bee3c109f 100644 --- a/src/test/java/com/github/copilot/sdk/SessionEventHandlingTest.java +++ b/src/test/java/com/github/copilot/sdk/SessionEventHandlingTest.java @@ -310,14 +310,16 @@ void testHandlersRunOnDispatchThread() throws Exception { latch.countDown(); }); - // Dispatch from a named thread to simulate the jsonrpc-reader + // Dispatch from a named thread var t = new Thread(() -> dispatchEvent(createAssistantMessageEvent("async")), "jsonrpc-reader-mock"); t.start(); assertTrue(latch.await(5, TimeUnit.SECONDS), "Handler should be invoked within timeout"); t.join(5000); - assertEquals("jsonrpc-reader-mock", handlerThreadName.get(), - "Handler should run on the dispatch thread, not a different one"); + // With serial dispatch, handlers run on the dedicated event-dispatch thread, + // not on the calling thread. + assertEquals("copilot-session-events", handlerThreadName.get(), + "Handler should run on the dedicated event-dispatch thread"); } @Test @@ -336,20 +338,22 @@ void testHandlersRunOffMainThread() throws Exception { assertTrue(latch.await(5, TimeUnit.SECONDS), "Handler should be invoked within timeout"); assertNotEquals(mainThreadName, handlerThreadName.get(), "Handler should NOT run on the main/test thread"); - assertEquals("background-dispatcher", handlerThreadName.get(), - "Handler should run on the background dispatch thread"); + // With serial dispatch, handlers always run on the dedicated event-dispatch + // thread + assertEquals("copilot-session-events", handlerThreadName.get(), + "Handler should run on the dedicated event-dispatch thread"); } @Test void testConcurrentDispatchFromMultipleThreads() throws Exception { var totalEvents = 100; var receivedCount = new AtomicInteger(); - var threadNames = ConcurrentHashMap.newKeySet(); + var handlerThreadNames = ConcurrentHashMap.newKeySet(); var latch = new CountDownLatch(totalEvents); session.on(AssistantMessageEvent.class, msg -> { receivedCount.incrementAndGet(); - threadNames.add(Thread.currentThread().getName()); + handlerThreadNames.add(Thread.currentThread().getName()); latch.countDown(); }); @@ -359,7 +363,14 @@ void testConcurrentDispatchFromMultipleThreads() throws Exception { var threadIdx = i; var t = new Thread(() -> { for (int j = 0; j < 10; j++) { - dispatchEvent(createAssistantMessageEvent("msg-" + threadIdx + "-" + j)); + try { + Method dispatchMethod = CopilotSession.class.getDeclaredMethod("dispatchEvent", + AbstractSessionEvent.class); + dispatchMethod.setAccessible(true); + dispatchMethod.invoke(session, createAssistantMessageEvent("msg-" + threadIdx + "-" + j)); + } catch (Exception e) { + throw new RuntimeException(e); + } } }, "dispatcher-" + i); threads.add(t); @@ -375,7 +386,10 @@ void testConcurrentDispatchFromMultipleThreads() throws Exception { } assertEquals(totalEvents, receivedCount.get(), "All " + totalEvents + " events should be delivered"); - assertTrue(threadNames.size() > 1, "Events should have been dispatched from multiple threads"); + // With serial dispatch, all handlers run on the single event-dispatch thread + assertEquals(1, handlerThreadNames.size(), "All handlers should run on the same serial event-dispatch thread"); + assertEquals("copilot-session-events", handlerThreadNames.iterator().next(), + "Handlers should run on the dedicated event-dispatch thread"); } // Helper methods to dispatch events using reflection @@ -843,11 +857,19 @@ private void dispatchEvent(AbstractSessionEvent event) { Method dispatchMethod = CopilotSession.class.getDeclaredMethod("dispatchEvent", AbstractSessionEvent.class); dispatchMethod.setAccessible(true); dispatchMethod.invoke(session, event); + // Drain the event dispatcher so handlers have run before we check state. + awaitEventDispatch(); } catch (Exception e) { throw new RuntimeException("Failed to dispatch event", e); } } + private void awaitEventDispatch() throws Exception { + Method method = CopilotSession.class.getDeclaredMethod("awaitEventDispatch"); + method.setAccessible(true); + method.invoke(session); + } + // Factory methods for creating test events private SessionStartEvent createSessionStartEvent() { return createSessionStartEvent("test-session"); diff --git a/src/test/java/com/github/copilot/sdk/SessionEventsE2ETest.java b/src/test/java/com/github/copilot/sdk/SessionEventsE2ETest.java index dad3f5907..82b605ebf 100644 --- a/src/test/java/com/github/copilot/sdk/SessionEventsE2ETest.java +++ b/src/test/java/com/github/copilot/sdk/SessionEventsE2ETest.java @@ -10,7 +10,9 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; @@ -28,7 +30,6 @@ import com.github.copilot.sdk.json.MessageOptions; import com.github.copilot.sdk.json.PermissionHandler; import com.github.copilot.sdk.json.SessionConfig; - /** * E2E tests for session events to verify event lifecycle. *

    @@ -277,4 +278,77 @@ void testInvokesBuiltInTools_eventOrderDuringToolExecution() throws Exception { assertTrue(toolCompleteIdx < turnEndIdx, "tool.execution_complete should be before turn_end"); } } + + /** + * Verifies that a handler exception does not halt event delivery to subsequent + * events. + * + * @see Snapshot: session/handler_exception_does_not_halt_event_delivery + */ + @Test + void testHandlerExceptionDoesNotHaltEventDelivery() throws Exception { + ctx.configureForTest("session", "handler_exception_does_not_halt_event_delivery"); + + try (CopilotClient client = ctx.createClient()) { + CopilotSession session = client + .createSession(new SessionConfig().setOnPermissionRequest(PermissionHandler.APPROVE_ALL)).get(); + + var eventCount = new AtomicInteger(0); + var gotIdle = new CompletableFuture(); + + session.setEventErrorPolicy(EventErrorPolicy.SUPPRESS_AND_LOG_ERRORS); + session.on(event -> { + int count = eventCount.incrementAndGet(); + // Throw on the first event to verify the loop keeps going. + if (count == 1) { + throw new RuntimeException("boom"); + } + if (event instanceof SessionIdleEvent) { + gotIdle.complete(null); + } + }); + + session.send("What is 1+1?"); + + gotIdle.get(30, TimeUnit.SECONDS); + + // Handler saw more than just the first (throwing) event. + assertTrue(eventCount.get() > 1, "Handler should be called for events after the throwing one"); + + session.close(); + } + } + + /** + * Verifies that calling {@code close()} from within an event handler does not + * deadlock. + * + * @see Snapshot: session/disposeasync_from_handler_does_not_deadlock + */ + @Test + void testCloseFromHandlerDoesNotDeadlock() throws Exception { + ctx.configureForTest("session", "disposeasync_from_handler_does_not_deadlock"); + + try (CopilotClient client = ctx.createClient()) { + CopilotSession session = client + .createSession(new SessionConfig().setOnPermissionRequest(PermissionHandler.APPROVE_ALL)).get(); + + var closed = new CompletableFuture(); + + session.on(event -> { + if (event instanceof UserMessageEvent) { + // Call close() from within a handler — must not deadlock. + CompletableFuture.runAsync(() -> { + session.close(); + closed.complete(null); + }); + } + }); + + session.send("What is 1+1?"); + + // If this times out, we deadlocked. + closed.get(10, TimeUnit.SECONDS); + } + } } diff --git a/src/test/java/com/github/copilot/sdk/TelemetryConfigTest.java b/src/test/java/com/github/copilot/sdk/TelemetryConfigTest.java new file mode 100644 index 000000000..962379046 --- /dev/null +++ b/src/test/java/com/github/copilot/sdk/TelemetryConfigTest.java @@ -0,0 +1,59 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + *--------------------------------------------------------------------------------------------*/ + +package com.github.copilot.sdk; + +import static org.junit.jupiter.api.Assertions.*; + +import org.junit.jupiter.api.Test; + +import com.github.copilot.sdk.json.CopilotClientOptions; +import com.github.copilot.sdk.json.TelemetryConfig; + +/** + * Unit tests for {@link TelemetryConfig} and its integration with + * {@link CopilotClientOptions}. + */ +class TelemetryConfigTest { + + @Test + void telemetryConfig_defaultValues_areNull() { + var config = new TelemetryConfig(); + + assertNull(config.getOtlpEndpoint()); + assertNull(config.getFilePath()); + assertNull(config.getExporterType()); + assertNull(config.getSourceName()); + assertNull(config.getCaptureContent()); + } + + @Test + void telemetryConfig_canSetAllProperties() { + var config = new TelemetryConfig().setOtlpEndpoint("http://localhost:4318").setFilePath("/tmp/traces.json") + .setExporterType("otlp-http").setSourceName("my-app").setCaptureContent(true); + + assertEquals("http://localhost:4318", config.getOtlpEndpoint()); + assertEquals("/tmp/traces.json", config.getFilePath()); + assertEquals("otlp-http", config.getExporterType()); + assertEquals("my-app", config.getSourceName()); + assertTrue(config.getCaptureContent()); + } + + @Test + void copilotClientOptions_telemetry_defaultsToNull() { + var options = new CopilotClientOptions(); + + assertNull(options.getTelemetry()); + } + + @Test + void copilotClientOptions_clone_copiesTelemetry() { + var telemetry = new TelemetryConfig().setOtlpEndpoint("http://localhost:4318").setExporterType("otlp-http"); + + var options = new CopilotClientOptions().setTelemetry(telemetry); + var clone = options.clone(); + + assertSame(telemetry, clone.getTelemetry()); + } +} diff --git a/src/test/java/com/github/copilot/sdk/ToolsTest.java b/src/test/java/com/github/copilot/sdk/ToolsTest.java index 538da74d2..c2109fe6c 100644 --- a/src/test/java/com/github/copilot/sdk/ToolsTest.java +++ b/src/test/java/com/github/copilot/sdk/ToolsTest.java @@ -25,6 +25,7 @@ import com.github.copilot.sdk.json.PermissionHandler; import com.github.copilot.sdk.json.PermissionRequest; import com.github.copilot.sdk.json.PermissionRequestResult; +import com.github.copilot.sdk.json.PermissionRequestResultKind; import com.github.copilot.sdk.json.SessionConfig; import com.github.copilot.sdk.json.ToolDefinition; @@ -360,4 +361,46 @@ void testOverridesBuiltInToolWithCustomTool() throws Exception { session.close(); } } + + /** + * Verifies that a tool with skipPermission=true is executed without triggering + * a permission prompt. + * + * @see Snapshot: tools/skippermission_sent_in_tool_definition + */ + @Test + void testSkipPermissionSentInToolDefinition() throws Exception { + ctx.configureForTest("tools", "skippermission_sent_in_tool_definition"); + + var parameters = Map.of("type", "object", "properties", + Map.of("id", Map.of("type", "string", "description", "Lookup ID")), "required", List.of("id")); + + var didRunPermissionRequest = new boolean[]{false}; + + ToolDefinition safeLookup = ToolDefinition.createSkipPermission("safe_lookup", "A tool that skips permission", + parameters, invocation -> { + String id = (String) invocation.getArguments().get("id"); + return CompletableFuture.completedFuture("RESULT: " + id); + }); + + try (CopilotClient client = ctx.createClient()) { + CopilotSession session = client.createSession( + new SessionConfig().setTools(List.of(safeLookup)).setOnPermissionRequest((request, invocation) -> { + didRunPermissionRequest[0] = true; + return CompletableFuture.completedFuture( + new PermissionRequestResult().setKind(PermissionRequestResultKind.NO_RESULT)); + })).get(); + + AssistantMessageEvent response = session + .sendAndWait(new MessageOptions().setPrompt("Use safe_lookup to look up 'test123'")) + .get(60, TimeUnit.SECONDS); + + assertNotNull(response); + assertTrue(response.getData().content().contains("RESULT"), + "Response should contain RESULT: " + response.getData().content()); + assertFalse(didRunPermissionRequest[0], "Permission handler should not be called for skipPermission tools"); + + session.close(); + } + } }