[java] Add linux-x64 implementation of in process Copilot CLI - #2301
[java] Add linux-x64 implementation of in process Copilot CLI#2301edburns wants to merge 12 commits into
Conversation
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
@edburns here are some comments from my review agent, hope these make sense. Happy to take another human look afterwards!
Requesting changes. The overall transport architecture broadly aligns with the other SDKs, and the large file count is mostly explainable: 1,522 of 1,596 files are byte-identical moves into java/sdk/. The module split is reasonable, but the published dependency graph, native ABI/lifecycle, and release validation still have blocking issues.
GitHub cannot attach inline review comments to unchanged files, so these relocation omissions are called out here:
.github/workflows/java-publish-maven.yml:204,207still referencesjava/jbang-example.java, so release preparation will fail after the move tojava/sdk/jbang-example.java.scripts/docs-validation/validate.ts:388-394searches the parent POM for artifactcopilot-sdk-java; it now falls back to1.0.0-SNAPSHOTinstead of validating the reactor's1.0.11-preview.0-SNAPSHOTartifact..github/actions/java-test-report/action.yml:7,11,15still searchesjava/target/**; current CI logs report that no test reports were found even though results are underjava/sdk/target/**..github/workflows/java-smoke-test.yml:66,139still points to the pre-move prompt path.
Please address the inline findings and these unchanged-file omissions before merging.
roji
left a comment
There was a problem hiding this comment.
Here are a few more comments.
Another thing I noticed is that while all other language SDKs automatically download the correct platform package with the native binary, the current approach in this PR requires users to manually take a dependency on e.g. the linux-x64 package, in addition to the platform-agnostic SDK package.
I don't know anything about how this kind of thing works with Java/Maven; is it impossible/not "the right way" to offer something that does this automatically (as all the other SDKs do)? Or maybe you're planning to look at that separately in a later PR (obviously completely fine too). Just raising the question.
Squashed from PR #2295 (branch edburns/…-review-02). Includes Java multi-module Maven restructure, copilot-native submodule for bundling the Rust CLI runtime, codegen updates, and related workflow changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 90cbda40-cda3-4ecd-b381-9f9ba0573d0a
Child modules (copilot-sdk-java, copilot-sdk-java-runtime) inherit from copilot-sdk-java-parent, which is not published to Maven Central. Without flattening, consumers resolving a child artifact would fail to resolve the parent POM. The flatten-maven-plugin (ossrh mode) inlines all inherited metadata so the deployed POM is fully self-contained. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3a69f33e-eea6-4b8c-9ef0-8b4d56d53a9c
Previously the tracked callback was removed in a finally block, releasing its GC root even when native connection_close failed or threw. Native code could still retain and invoke the stale function pointer, crashing the JVM after JNA collected the callback. Now the callback reference is only removed from trackedCallbacks when the native call succeeds, ensuring the function pointer stays rooted while native code may still hold it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3a69f33e-eea6-4b8c-9ef0-8b4d56d53a9c
Remove CallbackTestLib interface and all tests that depended on the spike libcallback_test.so from the removed 1917-java-embed-rust-cli-runtime-remove-before-merge directory. Rewrite the 3 duplicate-load guard tests to use NativeRuntimeLoader.resolve() to locate the real runtime.node binary instead of the spike library. Remove the startWithSpikeLibrarySupportsLifecycleAndDataFlow integration test from FfiRuntimeHostTest — this functionality is covered by E2E tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3a69f33e-eea6-4b8c-9ef0-8b4d56d53a9c
JNA is declared optional in the SDK POM so subprocess-mode users don't pull it transitively. The in-process mode section was missing this required third dependency, which would cause NoClassDefFoundError at runtime. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3a69f33e-eea6-4b8c-9ef0-8b4d56d53a9c
Maven Central requires javadoc and sources classifier JARs for every non-POM artifact. Since copilot-native has no Java sources, the parent's maven-javadoc-plugin produced nothing. Add explicit empty-archive executions to maven-jar-plugin so the module passes Central validation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3a69f33e-eea6-4b8c-9ef0-8b4d56d53a9c
The C ABI declares all buffer-length parameters as size_t (8 bytes on 64-bit), but the Java FFI layer was using int (always 4 bytes). While harmless for current JSON-RPC payloads, this is incorrect on 64-bit platforms and would be wrong on Windows x64 where NativeLong (C long) is also only 4 bytes. Introduce SizeT, a minimal IntegerType subclass sized via Native.SIZE_T_SIZE, and use it in CopilotRuntimeLibrary and OutboundCallback. The NativeBinding Java abstraction layer keeps int parameters; JnaNativeBinding converts at the boundary. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3a69f33e-eea6-4b8c-9ef0-8b4d56d53a9c
The process-wide Native.setCallbackExceptionHandler was redundant: the local catch (Throwable) in createOutboundCallback() already prevents any exception from escaping across the FFI boundary. The global mutation affected unrelated JNA callbacks in the same process and was never restored. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3a69f33e-eea6-4b8c-9ef0-8b4d56d53a9c
Remove continue-on-error so FFI regressions in the in-process transport block PRs. The in-process transport is now production code and must not silently regress. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3a69f33e-eea6-4b8c-9ef0-8b4d56d53a9c
The JBang example moved from java/jbang-example.java to java/sdk/jbang-example.java but two references were not updated: the runnable JBang URL in README.md and the release workflow's update-documentation-versions script invocation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3a69f33e-eea6-4b8c-9ef0-8b4d56d53a9c
The ADRs are at java/docs/adr/, not java/sdk/docs/adr/. Fix the relative links for ADR-006 and ADR-004. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3a69f33e-eea6-4b8c-9ef0-8b4d56d53a9c
c125e9d to
c0779fa
Compare
Auto-committed by java-codegen-check workflow.
Cross-SDK Consistency Review ✅This PR adds the in-process FFI transport to the Java SDK, bringing it into full parity with all other SDK implementations. The feature already exists across every other language:
Naming conventions correctly follow each language's idioms (camelCase for Java/Node.js, PascalCase for .NET, snake_case for Python/Rust, PascalCase type for Go). No consistency gaps found. Java was the last SDK without in-process FFI support; this PR closes that gap.
|
Java Codegen Fix — Automated AnalysisThe Root cause of
|
Supercedes #2295 .
This PR is the roll up of the agentic work done in the subtasks of #2166 . At each step of those subtasks, the CI was clean and all reviews were applied as appropriate.
PR 2295 — Reviewer's guide: In-process FFI runtime for the Java SDK
TL;DR
This PR does for the Java SDK what #1901 did for .NET and #1915 did for Rust: it adds an in-process connection mode that loads the Copilot runtime (
runtime.nodecdylib) as a native library via JNA, eliminating the need for a separate CLI child process. Currently scoped to linux-x64 only; the entire in-process API surface is marked@CopilotExperimental.The PR also restructures the Java Maven project from a single module into a multi-module reactor to support publishing the native runtime binaries as separate classifier JARs alongside the existing SDK JAR.
What's in the native binary, where does it come from, and how is it loaded?
The binary:
runtime.nodeDespite the
.nodeextension (a napi-rs naming convention),runtime.nodeis an ordinary platform-specific shared library (.soon Linux). It is a Rustcdylibproduced by thesrc/runtimecrate ingithub/copilot-agent-runtime. It exposes two front doors:extern "C"lifecycle/transport entry points callable by any language via FFI without Node.js.The 5 C ABI entry points are:
copilot_runtime_host_startcopilot_runtime_host_shutdowncopilot_runtime_connection_openon_outboundcallback for runtime→SDK data delivery.copilot_runtime_connection_writecopilot_runtime_connection_closeAll JSON-RPC methods travel as data through this fixed 5-function transport; the export surface never changes as the method set grows.
Where it comes from (build-time)
The
copilot-nativeMaven module's build fetches the binary from npm duringgenerate-resources:fetch-native.mjsreads the pinned version and SHA-512 integrity hash for@github/copilot-linux-x64fromnodejs/package-lock.json.npm packto download the exact tarball, verifies it against the integrity hash.runtime.nodeand thecopilotCLI executable into a staging directory.maven-jar-pluginpackages them into a classifier JAR (copilot-sdk-java-runtime-<version>-linux-x64.jar) with the layoutnative/linux-x64/runtime.node.How it's loaded (runtime)
PlatformDetector(303 lines) determines the classifier usingos.name,os.arch, and on Linux, ELF PT_INTERP parsing to distinguish glibc vs musl — no subprocesses, no heuristics.NativeRuntimeLoader(466 lines) resolves the binary in this order:COPILOT_CLI_PATHenv var → checks forruntime.nodealongside the CLI.native/<classifier>/runtime.node→ extracts atomically to~/.copilot/runtime-cache/<version>/<classifier>/runtime.node.runtime.nodealongside the bundledcopilotexecutable.JnaNativeBinding(253 lines) loads the library by absolute path via JNA and maps each C ABI export. Enforces a one-library-per-process invariant (library handle isstatic, never unloaded). Duplicate loads from the same path are silently accepted; different paths are rejected.FfiRuntimeHost(349 lines) orchestrates the lifecycle: starts the host, opens a connection, bridges the bidirectional JSON-RPC transport. Theon_outboundcallback (invoked by native threads) feeds received data into aQueueInputStreamthat the SDK's existingJsonRpcClientreads from.Structural changes
Multi-module Maven reactor
The single-module
java/pom.xmlis now a parent POM (pompackaging) with two submodules:java/pom.xmlcopilot-sdk-java-parentmaven.deploy.skip=true). Holds thereleaseprofile (GPG signing) inherited by all submodules.java/sdk/copilot-sdk-javajava/src/→java/sdk/src/.java/copilot-native/copilot-sdk-java-runtimelinux-x64only, ~20-26 MB).Consumer dependency declaration
Consumer usage
New public API surface (all
@CopilotExperimental)RuntimeConnection(sealed class)forStdio(),forTcp(),forUri(String),forInProcess().StdioRuntimeConnectionTcpRuntimeConnectionUriRuntimeConnectionInProcessRuntimeConnectionCopilotClientOptions.setConnection()/getConnection()The
RuntimeConnectionAPI replaces the previous pattern of settingcliUrl,cliPath,useStdio,port, andtcpConnectionTokenindividually. When aRuntimeConnectionis set, it takes precedence; conflicting legacy options causeIllegalArgumentException.New internal packages
com.github.copilot.ffi(9 classes, ~1,752 lines)FfiRuntimeHostJnaNativeBindingNativeBindingNativeRuntimeLoaderruntime.node: env var → classpath → cache. Atomic extraction with file locking.PlatformDetectorQueueInputStreamFfiOutputStreamconnection_write.OutboundCallbackon_outbound.ReaderThreadFactoryTests for FFI (6 files, ~2,054 lines)
FfiRuntimeHostTestJnaNativeBindingTestNativeRuntimeLoaderTestPlatformDetectorTestQueueInputStreamTestInProcessTransportITCI/workflow changes
java-sdk-inprocessinjava-sdk-tests.yml: runsmvn clean verify -Pinprocesson ubuntu-latest (linux-x64). Usescontinue-on-error: truewhile experimental.java/target/→java/sdk/target/for surefire/failsafe reports and coverage data.-pl sdkto restrict to the SDK module (the native module requires JDK 25 build tools).java/sdk/module layout.✅ Note that the existing java publishing jobs will continue to work as currently written.
Key design decisions (from ADR-007)
JNA over Panama FFM: JNA supports the Java 17 baseline with zero consumer configuration. Panama FFM requires Java 22+ and
--enable-native-accessflags. Performance difference is irrelevant (JSON-RPC I/O dominates).Per-platform classifier JARs over monolithic JAR: A monolithic JAR with all 6 common platforms would be ~132 MB. Classifier JARs let consumers pull only their target platform (~20-26 MB each). An uber-JAR can be assembled via
maven-assembly-pluginif needed.Library-never-unloads pattern: The loaded native library is held in a
staticfield and never released. Native worker threads outlive any individualFfiRuntimeHostinstance; unloading would crash.One library per process: Enforced by a process-wide guard, consistent with Rust, .NET, Go, and Python SDK implementations.
Diff statistics
java/src/→java/sdk/src/)copilot-native/pom.xml(214 lines),fetch-native.mjs(114 lines)Recommended review order
java/docs/adr/adr-007-native-bundling-strategy.md— context, options considered, decision rationale.rpc/RuntimeConnection.java,rpc/InProcessRuntimeConnection.java, andrpc/CopilotClientOptions.java(thesetConnection/getConnectionmethods).NativeBinding.java→JnaNativeBinding.java→FfiRuntimeHost.java→NativeRuntimeLoader.java→PlatformDetector.java.copilot-native/pom.xmlandcopilot-native/scripts/fetch-native.mjs.java/pom.xml(parent) andjava/sdk/pom.xml(child)..github/workflows/java-sdk-tests.yml(new inprocess job, path updates).ffi/test package ande2e/InProcessTransportIT.java.Implementation details.
Implemented agentically using https://aka.ms/coreai/shepherd-task/slides .