Skip to content

WIP: Base branch for epic 1917-java-embed-rust#2049

Draft
edburns wants to merge 38 commits into
mainfrom
edburns/1917-java-embed-rust-cli-runtime
Draft

WIP: Base branch for epic 1917-java-embed-rust#2049
edburns wants to merge 38 commits into
mainfrom
edburns/1917-java-embed-rust-cli-runtime

Conversation

@edburns

@edburns edburns commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Base branch for work in #1917 .

Copilot AI review requested due to automatic review settings July 22, 2026 00:02
@edburns edburns changed the title Edburns/1917 java embed rust cli runtime WIP: Base branch for epic 1917-java-embed-rust Jul 22, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Documents research and an implementation plan for embedding the Rust-based Copilot runtime in the Java SDK.

Changes:

  • Records research discussions about native packaging, JNA, and Panama.
  • Proposes phased implementation, packaging, testing, and CI work.
  • Captures a workflow-skill creation transcript.
Show a summary per file
File Description
20260721-prompts.md Records skill-development prompts.
20260710-prompts.md Records JNA and packaging research.
20260706-prompts.md Records native-runtime architecture research.
1917-embed-cli-runtime-ignorance-reduction-plan.md Defines the proposed Java implementation plan.

Review details

  • Files reviewed: 3/4 changed files
  • Comments generated: 7
  • Review effort level: Medium

Comment on lines +23 to +31
### C ABI entry points to bind (from .NET PR #1901 and Rust PR #1915)

| Entry point | Signature (C) | Purpose |
|-------------|---------------|---------|
| `copilot_runtime_host_start` | `(const uint8_t* entrypoint, size_t len, const uint8_t* args, size_t args_len) → uint32_t` | Start the runtime host; returns server handle |
| `copilot_runtime_host_shutdown` | `(uint32_t server) → bool` | Shut down the host |
| `copilot_runtime_connection_open` | `(uint32_t server, callback, user_data) → uint32_t` | Open a connection; registers outbound callback |
| `copilot_runtime_connection_write` | `(uint32_t conn, const uint8_t* data, size_t len) → bool` | Write JSON-RPC frame to the runtime |
| `copilot_runtime_connection_close` | `(uint32_t conn) → bool` | Close a connection |

**Spike needed:** Write a minimal JNA program that loads a test `.so`, registers a callback, and verifies callback invocation from a native thread. Confirm JNA's thread attachment behavior.

**Recommendation:** Use `PipedInputStream`/`PipedOutputStream` to bridge the callback into the existing `JsonRpcClient` input stream model. Hold the `Callback` instance as a field in the transport class (prevents GC). Track active callbacks with `AtomicInteger` and drain on close, mirroring the Rust pattern.
3. **Scope:** Should JNA be a required dependency or optional? If the SDK works without native binaries (subprocess transport), JNA is only needed for InProcess transport. Making it `<optional>true</optional>` means consumers using only CLI transport don't pull it in.
4. **GraalVM native-image:** JNA has established `native-image.properties` in its JAR. Verify this works for the callback pattern we need.

**Recommendation:** Add JNA as an `<optional>true</optional>` dependency. Only required when using InProcess transport. Use `jna` (not `jna-platform`). Version 5.16.0 or later.

**Spike needed:** Determine whether the replay proxy can be adapted to work with InProcess transport, or if InProcess tests must use the real runtime binary.

**Recommendation:** InProcess E2E tests use the real `runtime.node` binary (not the replay proxy). They run only in CI environments where the binary is available, gated by a Maven profile or system property. Existing YAML snapshots are orthogonal (they're for the replay proxy). Unit tests for the binding layer use a test `.so`/`.dylib` with a minimal C ABI surface.

Options for resolution order:

1. `COPILOT_CLI_PATH` environment variable → explicit path to the runtime binary

### 4.6 — Maven module for per-platform classifier JARs

**What:** New `copilot-sdk-java-runtime` Maven module that packages `runtime.node` binaries into classifier JARs.
**Files to modify:**
- `.github/workflows/java-sdk-tests.yml`

**Gating criteria:** CI runs InProcess E2E tests on linux-x64 and darwin-arm64. Tests are skipped gracefully when `runtime.node` is not available.
Copilot AI review requested due to automatic review settings July 22, 2026 00:06
@edburns
edburns force-pushed the edburns/1917-java-embed-rust-cli-runtime branch from 7b65681 to 1fe2d25 Compare July 22, 2026 00:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Comments suppressed due to low confidence (5)

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:29

  • These signatures do not match the merged FFI implementations. host_start receives argv JSON and environment JSON, while connection_open also has extension-source, extension-name, and connection-token pointer/length pairs (rust/src/ffi.rs:27-42; dotnet/src/FfiRuntimeHost.cs:369-377). Treating the abbreviated table as the C contract would generate incompatible JNA bindings.
| `copilot_runtime_host_start` | `(const uint8_t* entrypoint, size_t len, const uint8_t* args, size_t args_len) → uint32_t` | Start the runtime host; returns server handle |
| `copilot_runtime_host_shutdown` | `(uint32_t server) → bool` | Shut down the host |
| `copilot_runtime_connection_open` | `(uint32_t server, callback, user_data) → uint32_t` | Open a connection; registers outbound callback |

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:162

  • A PipedOutputStream write can block the native callback thread once its small bounded pipe fills (or before the JSON-RPC reader starts). Both reference transports intentionally copy each callback buffer into a nonblocking unbounded channel (rust/src/ffi.rs:47-65; dotnet/src/FfiRuntimeHost.cs:576-586). The Java design should preserve that property rather than introducing callback-thread backpressure.
**Recommendation:** Use `PipedInputStream`/`PipedOutputStream` to bridge the callback into the existing `JsonRpcClient` input stream model. Hold the `Callback` instance as a field in the transport class (prevents GC). Track active callbacks with `AtomicInteger` and drain on close, mirroring the Rust pattern.

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:249

  • An optional Maven dependency is not propagated to consumers. With this recommendation, applications can select IN_PROCESS and include the native classifier yet still fail with missing JNA classes unless they independently discover and declare JNA. Either make JNA a normal implementation dependency or move the JNA transport into an explicitly selected artifact that declares JNA normally.
**Recommendation:** Add JNA as an `<optional>true</optional>` dependency. Only required when using InProcess transport. Use `jna` (not `jna-platform`). Version 5.16.0 or later.

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:298

  • In-process only replaces the SDK-to-runtime transport; the runtime still makes CAPI requests, so discarding the replay proxy would make this test hit live services or fail authentication. The Rust and .NET in-process harnesses retain the proxy and mirror its proxy/auth environment into the host process (rust/tests/e2e/support.rs:603-650,793-826; dotnet/test/Harness/E2ETestContext.cs:273-319).
**Recommendation:** InProcess E2E tests use the real `runtime.node` binary (not the replay proxy). They run only in CI environments where the binary is available, gated by a Maven profile or system property. Existing YAML snapshots are orthogonal (they're for the replay proxy). Unit tests for the binding layer use a test `.so`/`.dylib` with a minimal C ABI surface.

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:447

  • A dedicated in-process CI job must not skip when runtime.node is absent; that would let artifact provisioning or classifier-packaging regressions pass with zero in-process coverage. Match the .NET smoke test's fail-hard behavior and reserve graceful skipping for ordinary local/non-in-process test runs.
**Gating criteria:** CI runs InProcess E2E tests on linux-x64 and darwin-arm64. Tests are skipped gracefully when `runtime.node` is not available.
  • Files reviewed: 3/4 changed files
  • Comments generated: 3
  • Review effort level: Medium


@FunctionalInterface
interface OutboundCallback extends Callback {
void invoke(Pointer userData, Pointer data, int length);

The Java SDK currently has no JNA dependency. Adding it introduces:

1. **Version selection:** JNA 5.x is current. The latest is 5.16.0 (as of 2025). It supports Java 8+. The SDK targets Java 17.
|---------|-------|
| **Java 17 baseline** | JNA works on Java 17. No Panama FFM. No `--enable-native-access` needed. |
| **GraalVM native-image** | Verify JNA callback pattern works under native-image. Add reachability metadata if needed. |
| **Windows path handling** | `runtime.node` on Windows is `copilot_runtime.dll`. Path separators, temp directory behavior differ. |
Copilot AI review requested due to automatic review settings July 22, 2026 00:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Comments suppressed due to low confidence (9)

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:27

  • The host_start contract is misidentified. The reference implementations serialize the complete argv array and environment map as JSON byte buffers; passing an entrypoint string plus an args string as this table suggests would give the native host malformed input (dotnet/src/FfiRuntimeHost.cs:127-130,358-362).
| `copilot_runtime_host_start` | `(const uint8_t* entrypoint, size_t len, const uint8_t* args, size_t args_len) → uint32_t` | Start the runtime host; returns server handle |

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:29

  • This omits six required parameters from the current C ABI. connection_open also accepts nullable source, name, and connection-token byte buffers with a size_t length for each; a JNA binding generated from this table would use the wrong calling convention/stack layout (rust/src/ffi.rs:31-41).
| `copilot_runtime_connection_open` | `(uint32_t server, callback, user_data) → uint32_t` | Open a connection; registers outbound callback |

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:162

  • Writing callback bytes to a PipedOutputStream can block the native callback thread once its small pipe buffer fills, potentially stalling the runtime while the JSON-RPC reader is delayed. The .NET and Rust implementations instead copy callback data into unbounded channels; use the analogous queue-backed InputStream so the callback returns promptly.
**Recommendation:** Use `PipedInputStream`/`PipedOutputStream` to bridge the callback into the existing `JsonRpcClient` input stream model. Hold the `Callback` instance as a field in the transport class (prevents GC). Track active callbacks with `AtomicInteger` and drain on close, mirroring the Rust pattern.

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:291

  • In-process only replaces the SDK↔runtime JSON-RPC transport; the loaded runtime still makes HTTP requests to CAPI/GitHub. The Rust in-process harness explicitly sets COPILOT_API_URL and COPILOT_DEBUG_GITHUB_API_URL to its replay proxy (rust/tests/e2e/support.rs:301-308), so treating the proxy as incompatible would send tests toward live services.
1. Can E2E tests use the InProcess transport against the replay proxy? The replay proxy is a network endpoint — InProcess transport bypasses network entirely. These are different transport paths.

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:298

  • A real runtime.node and the replay proxy are complementary, not alternatives. The integration test needs the real cdylib to exercise FFI while retaining the proxy and snapshots to keep CAPI traffic deterministic and hermetic; otherwise this plan requires live credentials/network and cannot safely replay existing scenarios.
**Recommendation:** InProcess E2E tests use the real `runtime.node` binary (not the replay proxy). They run only in CI environments where the binary is available, gated by a Maven profile or system property. Existing YAML snapshots are orthogonal (they're for the replay proxy). Unit tests for the binding layer use a test `.so`/`.dylib` with a minimal C ABI surface.

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:325

  • COPILOT_CLI_PATH names the CLI entrypoint passed through host_start argv, not the cdylib. The .NET reference resolves that entrypoint first and separately locates the sibling/prebuilds native library (dotnet/src/Client.cs:2270-2284); interpreting this variable as a library path would attempt to load or invoke the wrong artifact.
1. `COPILOT_CLI_PATH` environment variable → explicit path to the runtime binary

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:331

  • This resolution order conflates two independent artifacts. Resolve the CLI entrypoint used by host_start separately from the native library loaded by JNA; otherwise the explicit override path cannot work with the current host ABI.
**Recommendation:** Resolution order: `COPILOT_CLI_PATH` (explicit) → classpath resource (classifier JAR) → alongside bundled CLI. This matches the .NET pattern and gives operators an override.

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:447

  • The dedicated InProcess CI jobs must fail when runtime.node is missing; skipping would make a broken download/package/classifier pipeline appear green. The Rust reference test explicitly “fails hard if the in-process runtime library cannot be loaded” (rust/tests/e2e/inprocess.rs:3-9).
**Gating criteria:** CI runs InProcess E2E tests on linux-x64 and darwin-arm64. Tests are skipped gracefully when `runtime.node` is not available.

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:123

  • The native callback length is size_t, so mapping it to Java int truncates the ABI on every supported 64-bit target. Define a pointer-width size_t JNA type (including Windows LLP64) and use that consistently for callback and downcall lengths before treating this sketch as the binding contract.
    void invoke(Pointer userData, Pointer data, int length);
  • Files reviewed: 3/4 changed files
  • Comments generated: 1
  • Review effort level: Medium


**Question:** Should the InProcess transport API be annotated with `@CopilotExperimental`?

The existing SDK marks experimental features with `@CopilotExperimental` (compile-time check via `CopilotExperimentalProcessor`). The .NET PR's InProcess transport appears to be non-experimental (it's opt-in via connection type). The Rust PR's `Transport::InProcess` is additive.
Copilot AI review requested due to automatic review settings July 22, 2026 16:28
@edburns
edburns force-pushed the edburns/1917-java-embed-rust-cli-runtime branch from 1fe2d25 to 69d5490 Compare July 22, 2026 16:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Comments suppressed due to low confidence (6)

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:30

  • These two declarations do not match the ABI used by the referenced implementations. host_start receives JSON-encoded argv and environment buffers, while connection_open also requires the extension-source, extension-name, and connection-token buffer/length pairs (go/internal/ffihost/ffihost.go:19-28, dotnet/src/FfiRuntimeHost.cs:358-377). Implementing this table would omit required native arguments and can cause failed startup or ABI-level memory corruption; update the later NativeBinding sketch consistently.
| `copilot_runtime_host_start` | `(const uint8_t* entrypoint, size_t len, const uint8_t* args, size_t args_len) → uint32_t` | Start the runtime host; returns server handle |
| `copilot_runtime_host_shutdown` | `(uint32_t server) → bool` | Shut down the host |
| `copilot_runtime_connection_open` | `(uint32_t server, callback, user_data) → uint32_t` | Open a connection; registers outbound callback |
| `copilot_runtime_connection_write` | `(uint32_t conn, const uint8_t* data, size_t len) → bool` | Write JSON-RPC frame to the runtime |

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:123

  • The native callback's length parameter is size_t, but this sketch narrows it to Java int. All supported runtime targets are 64-bit, so the JNA callback declaration must use a pointer-width-compatible mapping; otherwise the Java declaration does not faithfully match the C ABI and large lengths are truncated.
    void invoke(Pointer userData, Pointer data, int length);

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:162

  • A PipedOutputStream write blocks when its small pipe buffer fills. Because this write would run inside the native callback, slow JSON-RPC consumption or shutdown can block a runtime-owned thread and deadlock while close waits for active callbacks. Keep the callback limited to copying and enqueueing bytes, as the referenced Rust implementation does with an unbounded channel (rust/src/ffi.rs:45-65,143-146), and expose a queue-backed InputStream to JsonRpcClient.
**Recommendation:** Use `PipedInputStream`/`PipedOutputStream` to bridge the callback into the existing `JsonRpcClient` input stream model. Hold the `Callback` instance as a field in the transport class (prevents GC). Track active callbacks with `AtomicInteger` and drain on close, mirroring the Rust pattern.

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:291

  • In-process only replaces the SDK-to-runtime stdio/TCP hop; the runtime still makes HTTP requests to CAPI. The Rust in-process suite explicitly passes COPILOT_API_URL and COPILOT_DEBUG_GITHUB_API_URL to the same replay proxy (rust/tests/e2e/support.rs:298-308,603-644), and Java's harness already configures those values. Avoiding the proxy would make this test non-hermetic and risk live API access instead of replaying snapshots.
1. Can E2E tests use the InProcess transport against the replay proxy? The replay proxy is a network endpoint — InProcess transport bypasses network entirely. These are different transport paths.

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:325

  • This redefines the established meaning of COPILOT_CLI_PATH. In the current Rust and .NET in-process implementations it points to the CLI entrypoint, and the host resolves the sibling cdylib (rust/src/resolve.rs:39-43, dotnet/src/Client.cs:2270-2289). Treating it as a runtime.node path would make host_start receive a shared library as argv[0] and break the residual-worker startup path. Resolve the CLI entrypoint and native library separately; use a new option only if a direct native-library override is needed.
1. `COPILOT_CLI_PATH` environment variable → explicit path to the runtime binary

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:420

  • Packaging only runtime.node is not sufficient for the currently merged C ABI host. host_start still launches the residual worker using a packaged CLI entrypoint (dotnet/src/FfiRuntimeHost.cs:20-23,73-101; rust/tests/e2e/support.rs:190-200). Add an explicit implementation gate: either provision a version-matched CLI entrypoint alongside each native artifact during the transition, or defer this phase until the residual-worker requirement is removed. Otherwise the classifier JARs can pass packaging checks but cannot start.
**What:** New `copilot-sdk-java-runtime` Maven module that packages `runtime.node` binaries into classifier JARs.
  • Files reviewed: 3/4 changed files
  • Comments generated: 0 new
  • Review effort level: Medium

Copilot AI review requested due to automatic review settings July 22, 2026 18:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Comments suppressed due to low confidence (9)

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:124

  • The callback maps C size_t to Java int, but all supported x64/arm64 targets use a 64-bit size_t. This gives JNA the wrong native callback signature and can truncate the payload length. Use a 64-bit mapping (or a custom size_t IntegerType) and bounds-check before copying into a Java array.
    void invoke(Pointer userData, Pointer data, int length);

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:503

  • This contradicts the plan's packaging convention: the Windows classifier JAR contains native/win32-*/runtime.node. copilot_runtime.dll is a natural name used only if the extracted file is deliberately renamed (as the .NET build does), so the plan must distinguish the resource name from the extracted filename.
### 4.8 — CI workflow updates

1917-java-embed-rust-cli-runtime-remove-before-merge/supply-chain-security-to-stephentoub.md:11

  • The blanket “at build time” statement is inaccurate: Python downloads the native runtime lazily when InProcess is selected (python/copilot/_cli_download.py:376-432), while Node resolves packages during installation. Rephrase this as a set of provisioning paths so the supply-chain analysis does not misstate when network access occurs.
Every non-Java SDK in the monorepo downloads `@github/copilot-{platform}` tarballs directly from `registry.npmjs.org` at build time:

1917-java-embed-rust-cli-runtime-remove-before-merge/supply-chain-security-to-stephentoub.md:16

  • Python's registry is not hardcoded-only: both its packument and tarball URL builders honor COPILOT_NPM_REGISTRY_URL (python/copilot/_cli_version.py:145-162). Omitting that override makes the comparison and mitigation assessment incorrect.
- **Python** — `python/copilot/_cli_version.py` (line 44): Hardcoded `_NPM_REGISTRY_BASE_URL = "https://registry.npmjs.org"`

1917-java-embed-rust-cli-runtime-remove-before-merge/supply-chain-security-to-stephentoub.md:23

  • This mitigation summary is outdated for Python. It supports a registry override and verifies npm Subresource Integrity, failing closed when integrity is unavailable (python/copilot/_cli_version.py:145-162, _cli_download.py:325-360,421-432).
- .NET is the only SDK that makes the registry URL configurable (`CopilotNpmRegistryUrl` property), so enterprise consumers can point at a private/mirrored registry.
- Rust verifies SHA-512 integrity after download.
- Go and Python have no registry override and no integrity verification visible in the code I examined.

1917-java-embed-rust-cli-runtime-remove-before-merge/supply-chain-security-to-stephentoub.md:7

  • Use the contraction “it's” (“it is”), not the possessive “its.”
I realize we're at GitHub, and its different here, but might we want to be SFI proactive here?

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:297

  • In-process hosting only replaces the SDK-to-runtime transport; the runtime still makes HTTP requests to CAPI/GitHub. The Rust in-process suite proves this by propagating COPILOT_API_URL and COPILOT_DEBUG_GITHUB_API_URL from its replay context (rust/tests/e2e/support.rs:39-44,301-307). Following this recommendation would bypass the repository's deterministic snapshots and risk hitting real services.
1. Can E2E tests use the InProcess transport against the replay proxy? The replay proxy is a network endpoint — InProcess transport bypasses network entirely. These are different transport paths.

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:332

  • COPILOT_CLI_PATH already means the CLI executable/entrypoint, not the native library (see dotnet/src/Client.cs:2270-2289 and the Java test harness). Reinterpreting it as runtime.node would break the established cross-SDK contract; the .NET implementation uses it to find the entrypoint and then resolves the sibling cdylib.
1. `COPILOT_CLI_PATH` environment variable → explicit path to the runtime binary

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:119

  • This proposed binding abstraction drops required C ABI inputs: host_start needs both argv JSON and environment JSON, and connection_open accepts extension source/name and connection-token buffers. In particular, omitting env_json prevents the host from receiving auth/proxy configuration. Keep the binding interface aligned with the five signatures documented above; higher-level entrypoint/argument conversion belongs in FfiRuntimeHost.
    int hostStart(String entrypoint, String args);
    boolean hostShutdown(int serverHandle);
    int connectionOpen(int serverHandle, OutboundCallback callback, Pointer userData);
    boolean connectionWrite(int connectionHandle, byte[] data);
    boolean connectionClose(int connectionHandle);
  • Files reviewed: 6/7 changed files
  • Comments generated: 3
  • Review effort level: Medium


**Question:** What is the exact implementation of platform detection, particularly the ELF PT_INTERP parsing for musl vs. glibc on Linux?

ADR-007 specifies reading the first 2 KB of `/proc/self/exe` and parsing the ELF PT_INTERP segment. This is the same approach as the `detect-libc` npm package.

Hi Stephen,

As I was working on #1917, I was reminded of an SFI concern for which I forced the mitigation on my previous team (Copilot modernization, under Dmirty (now retired)). Specifically: the team was pulling artifacts directly from npmjs.org, rather from an internal AzDO pipeline that lists npmjs.org as an upstream.

- `.github/workflows/java-sdk-tests.yml`

**Gating criteria:** CI runs InProcess E2E tests on linux-x64 and darwin-arm64. Tests are skipped gracefully when `runtime.node` is not available.
Copilot AI review requested due to automatic review settings July 22, 2026 18:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Comments suppressed due to low confidence (10)

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:22

  • The current runtime is not usable from runtime.node alone: both reference hosts put a packaged CLI entrypoint in argv[0], and host_start uses it to launch the residual worker. The plan must either package that entrypoint alongside each runtime (and update the size/distribution analysis) or explicitly gate delivery on a runtime version that no longer needs it; otherwise the planned classifier JARs cannot start today's runtime.
1. Ship per-platform classifier JARs containing the `runtime.node` binary for each of the 8 platform targets (Option 2).
2. Support uber-jar assembly via `maven-assembly-plugin` that merges all (or a subset of) platform JARs into a single distributable artifact (Option 1 compatibility).
3. Detect the current platform at runtime, extract the matching native binary, and load it via JNA to call the 5 `extern "C"` entry points of the runtime's C ABI front door.
4. Bridge bidirectional JSON-RPC transport over the FFI boundary (Java → native downcalls, native → Java upcall callbacks).

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:119

  • This sketch still uses the superseded ABI shape. host_start receives argv/environment JSON buffers, and connection_open also requires the nullable extension source, extension name, and connection-token buffers. Leaving the old shape here will guide the binding implementation to omit required arguments.
    int hostStart(String entrypoint, String args);
    boolean hostShutdown(int serverHandle);
    int connectionOpen(int serverHandle, OutboundCallback callback, Pointer userData);
    boolean connectionWrite(int connectionHandle, byte[] data);
    boolean connectionClose(int connectionHandle);

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:124

  • The native callback’s len is size_t, but Java int is only 32 bits. All targeted platforms are 64-bit, so the callback mapping must use a 64-bit value (or a dedicated JNA size_t mapping) to match the ABI.
    void invoke(Pointer userData, Pointer data, int length);

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:166

  • PipedOutputStream.write can block the native callback thread once its small buffer fills, which can stall runtime progress and deadlock shutdown while waiting for active callbacks to drain. Copy callback bytes into a nonblocking queue and expose a queue-backed InputStream, as the Rust implementation does with an unbounded channel.
**Recommendation:** Use `PipedInputStream`/`PipedOutputStream` to bridge the callback into the existing `JsonRpcClient` input stream model. Hold the `Callback` instance as a field in the transport class (prevents GC). Track active callbacks with `AtomicInteger` and drain on close, mirroring the Rust pattern.

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:214

  • A fixed 2 KB prefix is not sufficient to parse ELF reliably: ELF permits the program-header table and the PT_INTERP payload at offsets beyond that prefix. The implementation should read the ELF header, then the declared program-header table, then the interpreter bytes at the segment’s declared offset.
ADR-007 specifies reading the first 2 KB of `/proc/self/exe` and parsing the ELF PT_INTERP segment. This is the same approach as the `detect-libc` npm package.

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:338

  • COPILOT_CLI_PATH points to a CLI entrypoint, not directly to runtime.node. The reference hosts use that entrypoint in argv and locate the native library beside it; treating the variable as a library path would make Java attempt to load an executable through JNA.
1. `COPILOT_CLI_PATH` environment variable → explicit path to the runtime binary
2. Classpath resource (`native/<classifier>/runtime.node`) → from classifier JAR
3. Bundled CLI location (existing `CliServerManager` path) → the current subprocess path, but load the `.so`/`.dylib`/`.dll` sibling

The .NET PR resolves the entrypoint from `COPILOT_CLI_PATH` and falls back to the bundled CLI location. The Rust PR discovers or extracts the platform library alongside the embedded CLI.

**Recommendation:** Resolution order: `COPILOT_CLI_PATH` (explicit) → classpath resource (classifier JAR) → alongside bundled CLI. This matches the .NET pattern and gives operators an override.

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:267

  • This section still describes the old “entrypoint + args” ABI even though the table above was corrected. The ABI accepts serialized argv and optional environment JSON; in the reference implementations argv contains the CLI entrypoint and --embedded-host arguments. Keeping the obsolete questions leaves the core packaging requirement unresolved.
The .NET PR passes an `entrypoint` path and `args`. The Rust PR similarly passes entrypoint and args as byte buffers. We need to understand:

1. What is the `entrypoint` parameter? Is it the path to the `runtime.node` binary itself, or a path to a Node.js entry script?
2. What are the `args`? JSON-formatted startup options? CLI-style flags?
3. Does the host need the `runtime.node` file path passed as entrypoint, or does it use the loaded library's own location?

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:301

  • This section incorrectly treats the native runtime and replay proxy as alternatives. In-process changes only the SDK↔runtime transport; the runtime still makes CAPI requests. The Rust in-process suite launches the same replay proxy, sets COPILOT_API_URL/proxy environment, and reuses snapshots, so Java should do likewise to avoid live CAPI calls.
1. Can E2E tests use the InProcess transport against the replay proxy? The replay proxy is a network endpoint — InProcess transport bypasses network entirely. These are different transport paths.
2. Should InProcess E2E tests use a **real** `runtime.node` binary? This would require the binary to be available in CI.
3. How do we mock/stub the native library for unit testing the JNA binding layer without a real `runtime.node`?
4. Should InProcess E2E tests reuse existing YAML snapshots, or do they need separate snapshots?
5. **Should the entire existing E2E test suite be run with each valid transport (subprocess and InProcess)?** The Rust PR does this — the same E2E tests run in a separate CI job with `COPILOT_SDK_DEFAULT_CONNECTION=inprocess`, providing confidence that both transport paths produce identical behavior. The researcher should determine whether the Java E2E suite can be structured the same way (e.g., a separate Maven profile or CI matrix entry that sets the transport to InProcess and re-runs the full suite).

1917-java-embed-rust-cli-runtime-remove-before-merge/supply-chain-security-to-stephentoub.md:5

  • The comparison requires “rather than,” not “rather.”
As I was working on #1917, I was reminded of an SFI concern for which I forced the mitigation on my previous team (Copilot modernization, under Dmirty (now retired)). Specifically: the team was pulling artifacts directly from npmjs.org, rather from an internal AzDO pipeline that lists npmjs.org as an upstream.

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:365

  • Correct the typo in this retained plan text.
   Where is this `runtimes` direcory? Is it committed to `git`? I doubt that. Is it in `~/.copilot`?
  • Files reviewed: 6/7 changed files
  • Comments generated: 7
  • Review effort level: Medium


- **[napi](#references) front door** — loaded by a Node.js process as a native addon (current CLI path).
- **[C ABI](#references) front door** — a fixed set of approximately 12 `extern "C"` lifecycle and transport entry points (`copilot_runtime_server_create`, `copilot_runtime_connection_open`, etc.) that any language can call in-process via [FFI](#references) ([JNA](#references) for Java, Python/cffi, C#/`DllImport`, Go/purego) **without a Node.js process**. All API methods travel as JSON-RPC data through this fixed transport; the export list never changes as the method set grows.
- **[C ABI](#references) front door** — a fixed set of 5 `extern "C"` lifecycle and transport entry points that any language can call in-process via [FFI](#references) ([JNA](#references) for Java, Python/cffi, C#/`DllImport`, Go/purego) **without a Node.js process**. All API methods travel as JSON-RPC data through this fixed transport; the export list never changes as the method set grows. The 5 entry points are:
Comment on lines +199 to +200
/** Use IN_PROCESS if native binary available, else fall back to CLI. */
DEFAULT

## What I found

Every non-Java SDK in the monorepo downloads `@github/copilot-{platform}` tarballs directly from `registry.npmjs.org` at build time:
- **.NET** — `dotnet/src/build/GitHub.Copilot.SDK.targets` (line 58): `DownloadFile` from `registry.npmjs.org` (configurable via `CopilotNpmRegistryUrl` property)
- **Rust** — `rust/build/in_process.rs` (line 86): Hardcoded `registry.npmjs.org` download in `build.rs`
- **Go** — `go/cmd/bundler/main.go` (line 38): Hardcoded `registry.npmjs.org` download URL
- **Python** — `python/copilot/_cli_version.py` (line 44): Hardcoded `_NPM_REGISTRY_BASE_URL = "https://registry.npmjs.org"`
Comment on lines +21 to +23
- .NET is the only SDK that makes the registry URL configurable (`CopilotNpmRegistryUrl` property), so enterprise consumers can point at a private/mirrored registry.
- Rust verifies SHA-512 integrity after download.
- Go and Python have no registry override and no integrity verification visible in the code I examined.

As I was working on #1917, I was reminded of an SFI concern for which I forced the mitigation on my previous team (Copilot modernization, under Dmirty (now retired)). Specifically: the team was pulling artifacts directly from npmjs.org, rather from an internal AzDO pipeline that lists npmjs.org as an upstream.

I realize we're at GitHub, and its different here, but might we want to be SFI proactive here?

### 3.15 Additional human generated questions while reviewing the first draft of this plan, committed in 292a9036aa

1. Is the set of C ABI entry points listed in the table at "C ABI entry points to bind" sufficient? I thought ypou said there were "12 `extern "C"` entry points? That table only has 5.
Copilot AI review requested due to automatic review settings July 22, 2026 18:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Comments suppressed due to low confidence (12)

1917-java-embed-rust-cli-runtime-remove-before-merge/supply-chain-security-to-stephentoub.md:11

  • This summary is inaccurate for Python: its tarball is fetched when the in-process runtime is provisioned, not during the package build. Rephrase the claim so it does not characterize every SDK as a build-time downloader.
Every non-Java SDK in the monorepo downloads `@github/copilot-{platform}` tarballs directly from `registry.npmjs.org` at build time:

1917-java-embed-rust-cli-runtime-remove-before-merge/supply-chain-security-to-stephentoub.md:16

  • This bullet treats the fallback constant as a hardcoded-only endpoint, but the current Python implementation uses COPILOT_NPM_REGISTRY_URL in both URL builders (python/copilot/_cli_version.py:149,161). The note should record that override rather than reporting Python as fixed to npmjs.org.
- **Python** — `python/copilot/_cli_version.py` (line 44): Hardcoded `_NPM_REGISTRY_BASE_URL = "https://registry.npmjs.org"`

1917-java-embed-rust-cli-runtime-remove-before-merge/supply-chain-security-to-stephentoub.md:23

  • The Python mitigation inventory is outdated. Python now supports a registry override and verifies npm Subresource Integrity, failing closed when integrity is absent or mismatched (python/copilot/_cli_version.py:149-161, python/copilot/_cli_download.py:421-432).
- .NET is the only SDK that makes the registry URL configurable (`CopilotNpmRegistryUrl` property), so enterprise consumers can point at a private/mirrored registry.
- Rust verifies SHA-512 integrity after download.
- Go and Python have no registry override and no integrity verification visible in the code I examined.

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:153

  • This proposed interface still models the obsolete entrypoint/args form and omits the three nullable metadata buffers from connection_open, contradicting the corrected ABI table above. If implementation follows this sample, it cannot forward external source/name or connection token and will bind the wrong host-start semantics.
    int hostStart(String entrypoint, String args);
    boolean hostShutdown(int serverHandle);
    int connectionOpen(int serverHandle, OutboundCallback callback, Pointer userData);
    boolean connectionWrite(int connectionHandle, byte[] data);
    boolean connectionClose(int connectionHandle);

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:158

  • The callback's C size_t len is 64-bit on every supported target, but this JNA sketch maps it to a 32-bit Java int. That does not faithfully describe the ABI and can truncate the callback length; use a 64-bit mapping (or a dedicated JNA size_t type).
    void invoke(Pointer userData, Pointer data, int length);

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:200

  • Writing callback payloads directly into a PipedOutputStream can block the native callback thread whenever the pipe fills (the default pipe is small), potentially stalling the runtime during startup or while JSON-RPC consumption is delayed. The callback should synchronously copy the native buffer into a nonblocking queue/channel and return; an InputStream adapter can consume that queue separately.
**Recommendation:** Use `PipedInputStream`/`PipedOutputStream` to bridge the callback into the existing `JsonRpcClient` input stream model. Hold the `Callback` instance as a field in the transport class (prevents GC). Track active callbacks with `AtomicInteger` and drain on close, mirroring the Rust pattern.

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:248

  • A fixed 2 KB read cannot parse every valid ELF: the ELF header gives the program-header table offset/count, and the selected PT_INTERP segment's bytes may lie beyond that prefix. The implementation should read the header and program headers first, then seek to the segment's declared offset and length.
ADR-007 specifies reading the first 2 KB of `/proc/self/exe` and parsing the ELF PT_INTERP segment. This is the same approach as the `detect-libc` npm package.

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:301

  • This section was not updated with the corrected ABI. host_start receives one argv_json buffer and one optional env_json buffer; the CLI entrypoint is an element of the argv array, while the loaded runtime.node path is not a host-start argument. Leaving these obsolete questions in the implementation plan reintroduces the ambiguity this PR intended to resolve.
The .NET PR passes an `entrypoint` path and `args`. The Rust PR similarly passes entrypoint and args as byte buffers. We need to understand:

1. What is the `entrypoint` parameter? Is it the path to the `runtime.node` binary itself, or a path to a Node.js entry script?
2. What are the `args`? JSON-formatted startup options? CLI-style flags?
3. Does the host need the `runtime.node` file path passed as entrypoint, or does it use the loaded library's own location?

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:331

  • In-process hosting bypasses only the SDK-to-runtime JSON-RPC process transport; the runtime still performs HTTP requests to CAPI/GitHub endpoints. Existing Rust in-process E2Es explicitly set COPILOT_API_URL and COPILOT_DEBUG_GITHUB_API_URL to the replay proxy (rust/tests/e2e/support.rs:301-307), so these are not mutually exclusive paths.
1. Can E2E tests use the InProcess transport against the replay proxy? The replay proxy is a network endpoint — InProcess transport bypasses network entirely. These are different transport paths.

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:339

  • Running the real runtime without the replay proxy would either contact live services or fail, and it would discard the deterministic snapshot coverage that enables the full dual-transport suite. Keep the real runtime.node for the FFI boundary while routing its outbound model/API traffic through the existing replay proxy and snapshots, as the Rust suite does.
**Spike needed:** Determine whether the replay proxy can be adapted to work with InProcess transport, or if InProcess tests must use the real runtime binary. Determine whether the full E2E suite can run under both transports, or if certain tests are inherently transport-specific.

**Recommendation:** InProcess E2E tests use the real `runtime.node` binary (not the replay proxy). They run only in CI environments where the binary is available, gated by a Maven profile or system property. Existing YAML snapshots are orthogonal (they're for the replay proxy). Unit tests for the binding layer use a test `.so`/`.dylib` with a minimal C ABI surface. The full E2E suite should be run under both subprocess and InProcess transports in CI, mirroring the Rust PR's approach.

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:366

  • COPILOT_CLI_PATH is an override for the CLI entrypoint included in argv_json, not a path to runtime.node. The .NET implementation resolves the native library separately beside that entrypoint. Repurposing this variable as the library path would make host_start try to launch the cdylib as its embedded-host program.
1. `COPILOT_CLI_PATH` environment variable → explicit path to the runtime binary

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:545

  • A dedicated InProcess CI job that silently skips when its required native artifact is missing can remain green while exercising no in-process behavior. Since the preceding step provisions the artifact and the plan forbids skipped tests, these jobs should fail fast when runtime.node is unavailable.
**Gating criteria:** CI runs InProcess E2E tests on linux-x64 and darwin-arm64. Tests are skipped gracefully when `runtime.node` is not available.
  • Files reviewed: 6/7 changed files
  • Comments generated: 1
  • Review effort level: Medium

<archive>
<manifestEntries>
<Copilot-Runtime-Platform>darwin-arm64</Copilot-Runtime-Platform>
<Copilot-Runtime-Version>${project.version}</Copilot-Runtime-Version>
Copilot AI review requested due to automatic review settings July 22, 2026 19:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Comments suppressed due to low confidence (7)

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:120

  • This proposed interface still models the obsolete ABI: hostStart drops env_json, connectionOpen drops ext_source, ext_name, and conn_token, and the callback maps native size_t to Java int. Following this sketch would prevent environment/auth metadata from reaching the host and can misdeclare the callback on 64-bit platforms. Update it to match the five signatures documented at lines 26–34.
    int hostStart(String entrypoint, String args);
    boolean hostShutdown(int serverHandle);
    int connectionOpen(int serverHandle, OutboundCallback callback, Pointer userData);
    boolean connectionWrite(int connectionHandle, byte[] data);
    boolean connectionClose(int connectionHandle);

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:306

  • In-process FFI bypasses only the SDK↔runtime process transport; the runtime still makes outbound CAPI network requests. The Rust in-process suite starts the replay proxy, passes COPILOT_API_URL/proxy environment into the host, and reuses snapshots (rust/tests/e2e/support.rs:289-307,786-903). Running the real cdylib “not the replay proxy” would instead hit live services and make these tests nondeterministic.
**Recommendation:** InProcess E2E tests use the real `runtime.node` binary (not the replay proxy). They run only in CI environments where the binary is available, gated by a Maven profile or system property. Existing YAML snapshots are orthogonal (they're for the replay proxy). Unit tests for the binding layer use a test `.so`/`.dylib` with a minimal C ABI surface. The full E2E suite should be run under both subprocess and InProcess transports in CI, mirroring the Rust PR's approach.

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:482

  • Packaging only runtime.node is insufficient while the active runtime still launches the residual TypeScript worker: host_start receives a CLI entrypoint, and both existing hosts bundle/resolve that executable alongside the cdylib (rust/src/ffi.rs:230-306, dotnet/src/FfiRuntimeHost.cs:73-101). Without also provisioning that platform CLI binary, Java still requires an external COPILOT_CLI_PATH and does not meet the plan's stated goal. Decide whether classifier JARs include both artifacts or implementation waits until the residual worker is gone.
**What:** New `copilot-sdk-java-runtime` Maven module that packages `runtime.node` binaries into classifier JARs.

1917-java-embed-rust-cli-runtime-remove-before-merge/supply-chain-security-to-stephentoub.md:11

  • Python is no longer part of this “build time” statement. Its runtime library is downloaded lazily only when in-process transport is selected (or by the explicit download command), as documented in python/copilot/_cli_download.py:376-387. Distinguish build-time acquisition from Python's runtime provisioning so the note accurately scopes the exposure.
Every non-Java SDK in the monorepo downloads `@github/copilot-{platform}` tarballs directly from `registry.npmjs.org` at build time:

1917-java-embed-rust-cli-runtime-remove-before-merge/supply-chain-security-to-stephentoub.md:23

  • This mitigation inventory is stale. Python honors COPILOT_NPM_REGISTRY_URL for both packument and tarball URLs (_cli_version.py:145-162) and refuses to load unless npm Subresource Integrity is available and verifies (_cli_download.py:325-360,418-432). Sending the current text would incorrectly report two missing Python mitigations.
- .NET is the only SDK that makes the registry URL configurable (`CopilotNpmRegistryUrl` property), so enterprise consumers can point at a private/mirrored registry.
- Rust verifies SHA-512 integrity after download.
- Go and Python have no registry override and no integrity verification visible in the code I examined.

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:512

  • A dedicated in-process CI job should fail when its required runtime.node is missing. Skipping would turn a broken package/download/profile configuration—the exact integration this job is meant to validate—into green CI and conflicts with the no-skipping test discipline above.
**Gating criteria:** CI runs InProcess E2E tests on linux-x64 and darwin-arm64. Tests are skipped gracefully when `runtime.node` is not available.

1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md:268

  • This section reverted to the old entrypoint + args ABI description. Both reference implementations build argv_json (including the residual CLI entrypoint and arguments) and pass a separate optional env_json; runtime.node is loaded independently. Keeping these questions sends the spike toward a contract that the corrected table has already ruled out.
The .NET PR passes an `entrypoint` path and `args`. The Rust PR similarly passes entrypoint and args as byte buffers. We need to understand:

1. What is the `entrypoint` parameter? Is it the path to the `runtime.node` binary itself, or a path to a Node.js entry script?
2. What are the `args`? JSON-formatted startup options? CLI-style flags?
3. Does the host need the `runtime.node` file path passed as entrypoint, or does it use the loaded library's own location?
  • Files reviewed: 6/7 changed files
  • Comments generated: 3
  • Review effort level: Medium


- **[napi](#references) front door** — loaded by a Node.js process as a native addon (current CLI path).
- **[C ABI](#references) front door** — a fixed set of approximately 12 `extern "C"` lifecycle and transport entry points (`copilot_runtime_server_create`, `copilot_runtime_connection_open`, etc.) that any language can call in-process via [FFI](#references) ([JNA](#references) for Java, Python/cffi, C#/`DllImport`, Go/purego) **without a Node.js process**. All API methods travel as JSON-RPC data through this fixed transport; the export list never changes as the method set grows.
- **[C ABI](#references) front door** — a fixed set of 5 `extern "C"` lifecycle and transport entry points that any language can call in-process via [FFI](#references) ([JNA](#references) for Java, Python/cffi, C#/`DllImport`, Go/purego) **without a Node.js process**. All API methods travel as JSON-RPC data through this fixed transport; the export list never changes as the method set grows. The 5 entry points are:
3. Which CI runner platforms need InProcess test coverage? (linux-x64 and darwin-arm64 minimum?)
4. Should InProcess tests be gated behind a `runtime.node` availability check to avoid failing when the binary isn't present?

**Recommendation:** Modify the existing `java-sdk-tests.yml` to add InProcess test jobs on linux-x64 and darwin-arm64 runners. Native binaries are downloaded from the `copilot-agent-runtime` release artifacts. InProcess tests run as a separate Maven profile.
Comment on lines +449 to +454
**What:** `FfiRuntimeHost` class that manages the full lifecycle: `host_start` → `connection_open` → duplex stream bridging → `connection_close` → `host_shutdown`. Provides `InputStream`/`OutputStream` compatible with `JsonRpcClient`.

**Files to create:**

- `java/src/main/java/com/github/copilot/ffi/FfiRuntimeHost.java`

Copilot AI review requested due to automatic review settings July 22, 2026 20:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Comments suppressed due to low confidence (16)

1917-java-embed-rust-cli-runtime-remove-before-merge/supply-chain-security-to-stephentoub.md:11

  • This generalization is inaccurate for Python: ensure_runtime_library is explicitly lazy and only runs when in-process transport is selected (python/copilot/_cli_download.py:376-387), not during the SDK build. Distinguish build-time acquisition from Python's first-use provisioning so the note describes the actual exposure.
    1917-java-embed-rust-cli-runtime-remove-before-merge/supply-chain-security-to-stephentoub.md:23
  • The Python conclusion is now false. Python honors COPILOT_NPM_REGISTRY_URL (_cli_version.py:145-162) and fails closed unless the downloaded runtime passes npm SRI verification (_cli_download.py:421-432). As written, this note could drive a supply-chain decision from stale evidence.
    1917-java-embed-rust-cli-runtime-remove-before-merge/20260722-prompts-3_1-and-3_2-gaps.md:41
  • These versions are demonstrably independent: java/pom.xml:10 is 1.0.9-SNAPSHOT, while nodejs/package-lock.json:702-727 pins the runtime/platform packages to 1.0.73. Using ${project.version} in npm pack will therefore request the wrong runtime version and break the proposed native build. Keep a separate pinned runtime-version property and update the same assumption in the earlier resolution on line 15 and the implementation plan.
The npm package version and the SDK version are the **same version**. `@github/copilot-linux-x64@1.0.9-preview.0` and `com.github:copilot-sdk-java:1.0.9-preview.0` — same string.

So there's no version mapping at all. The `copilot-native` module downloads `@github/copilot-<platform>@${project.version}` from npm. One version, everywhere. No extra property needed.

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-6-platform-detection-win32-x64/src/main/java/com/github/copilot/spike/platform/PlatformDetector.java:108

  • UNKNOWN currently falls through to the glibc classifier. On a musl host where /proc is unavailable or ELF parsing fails, this deterministically selects the incompatible linux-* library instead of reporting that detection failed. Fail closed unless libc was positively identified.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-6-platform-detection-linux-x64/src/main/java/com/github/copilot/spike/platform/PlatformDetector.java:102
  • UNKNOWN currently falls through to the glibc classifier. On a musl host where /proc is unavailable or ELF parsing fails, this selects an incompatible linux-* library. Fail closed unless libc was positively identified.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-6-platform-detection-darwin-arm64/src/main/java/com/github/copilot/spike/platform/PlatformDetector.java:99
  • UNKNOWN currently falls through to the glibc classifier. If this shared implementation is extrapolated to Linux and /proc cannot be read, a musl host will select the incompatible linux-* library. Fail closed unless libc was positively identified.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/CallbackTestLibrary.java:42
  • This binding maps Rust's usize/C size_t lengths to Java int. On all supported 64-bit targets size_t is 64-bit, so the callback and connection_write declarations validate a different ABI and truncate large lengths. Introduce a JNA integer type sized with Native.SIZE_T_SIZE and use it for every length parameter.
    int connection_open(int serverHandle, OutboundCallback callback, Pointer userData, int burstCount);

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/src/main/java/com/github/copilot/spike/mrjar/NativeBindingProvider.java:87

  • The real ABI already delivers complete LSP Content-Length: frames unchanged (rust/src/ffi.rs:7-10), and JsonRpcClient parses that framing (java/src/main/java/com/github/copilot/JsonRpcClient.java:197-251). Adding a private four-byte prefix and consuming it with a custom reader means this spike never validates the claimed callback-to-JsonRpcClient bridge. Have the mock emit real LSP frames and enqueue the callback bytes unchanged.
                byte[] frame = new byte[4 + len];
                frame[0] = (byte) (len >> 24);
                frame[1] = (byte) (len >> 16);
                frame[2] = (byte) (len >> 8);
                frame[3] = (byte) len;

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/CallbackSpikeMain.java:174

  • A timeout does not prevent the subsequent connection/host shutdown, even though the detached Rust producer may still be invoking this callback. Because the mock close functions neither cancel nor join that thread, this can release lifecycle state while a native callback remains possible. The spike should make close stop/join the producer and drain activeCallbacks before shutdown.
        boolean completed = allCallbacksDone.await(10, TimeUnit.SECONDS);
        if (completed) {
            LOG.info("All callbacks completed. activeCallbacks=" + activeCallbacks.get());
        } else {
            LOG.warning("Timed out waiting for callbacks! activeCallbacks=" + activeCallbacks.get());

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/README.md:1

  • This README describes an FFM implementation that is not present. The JDK 25 overlay only replaces ReaderThreadFactory; NativeBindingProvider remains the baseline JNA implementation, as the program itself reports on lines 149-150. Consequently the FFM command, expected logs, and comparison table are all incorrect and --enable-native-access is unnecessary for this spike.
# Multi-Release JAR Spike: JNA (JDK 17) vs FFM (JDK 25)

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/README.md:25

  • This path does not exist; the sibling project created by this PR is named java-program-that-invokes-rust-dll-jdk17. Following the documented commands from rust-dll therefore fails before Maven runs.
cd ../java-program-that-invokes-rust-dll

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/src/main/java/com/github/copilot/spike/mrjar/QueueInputStream.java:64

  • This override omits the InputStream bounds contract. With an invalid off/len, it can block in queue.take() before eventually throwing (and at EOF it may incorrectly return -1), instead of immediately throwing IndexOutOfBoundsException. Validate the range before checking EOF.
    public int read(byte[] b, int off, int len) throws IOException {

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/QueueInputStream.java:77

  • This override does not validate off and len before blocking. Invalid arguments can wait indefinitely in queue.take() (or return -1 at EOF) rather than immediately satisfying the InputStream contract with IndexOutOfBoundsException. Check the range first.
    public int read(byte[] b, int off, int len) throws IOException {

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/src/main/java/com/github/copilot/spike/mrjar/NativeBindingProvider.java:29

  • These int len declarations do not mirror Rust's usize/C size_t ABI (rust-dll/src/lib.rs:6,106), which is pointer-width and therefore 64-bit on every target in this epic. The callback on line 35 has the same mismatch. Use a portable JNA size_t type based on Native.SIZE_T_SIZE; otherwise this spike validates a different ABI and truncates lengths above 32 bits.
        int connection_open(int serverHandle, OutboundCallback callback, Pointer userData, int burstCount);
        boolean connection_write(int connectionHandle, byte[] data, int len);

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/src/main/java/com/github/copilot/spike/mrjar/CallbackMrJarMain.java:133

  • After this timeout the code continues into connection_close/host_shutdown even though callbacks may still be running. The mock's close functions do not cancel or join its detached Rust thread, so native code can invoke the callback after shutdown; this is exactly the lifetime race the spike is meant to evaluate. Make close stop/join the producer and wait for active callbacks to drain before releasing the callback.
        boolean completed = allCallbacksDone.await(10, TimeUnit.SECONDS);
        if (completed) {
            LOG.info("All callbacks completed. activeCallbacks=" + activeCallbacks.get());
        } else {
            LOG.warning("Timed out! activeCallbacks=" + activeCallbacks.get());

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/pom.xml:16

  • The artifact metadata still claims the JDK 25 overlay uses FFM, but this module has only one NativeBindingProvider, under the Java 17 source root, and it uses JNA on every JDK. Update the name/description to say that only the reader thread changes (platform thread on 17, virtual thread on 25).
    <name>Spike 3.4 — Multi-Release JAR: JNA (17) + FFM (25)</name>
    <description>
        Demonstrates the multi-release JAR pattern for native callback bridging:
        - JDK 17 baseline: JNA callback + QueueInputStream
        - JDK 25 overlay: FFM (Panama) upcall stub + QueueInputStream
  • Files reviewed: 43/46 changed files
  • Comments generated: 1
  • Review effort level: Medium

Comment on lines +106 to +110
// Write a length-prefixed frame: 4-byte big-endian length + data
byte[] frame = new byte[4 + len];
frame[0] = (byte) (len >> 24);
frame[1] = (byte) (len >> 16);
frame[2] = (byte) (len >> 8);
Copilot AI review requested due to automatic review settings July 24, 2026 20:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Comments suppressed due to low confidence (20)

1917-java-embed-rust-cli-runtime-remove-before-merge/20260722-prompts-3_1-and-3_2-gaps.md:41

  • The SDK and npm runtime are independently versioned: the current Java POM is 1.0.9-SNAPSHOT, while @github/copilot and its platform packages are 1.0.73. Downloading @github/copilot-<platform>@${project.version} would therefore request the wrong (often nonexistent) package. Keep a separate pinned runtime version while publishing the classifier artifact with the SDK version.
The npm package version and the SDK version are the **same version**. `@github/copilot-linux-x64@1.0.9-preview.0` and `com.github:copilot-sdk-java:1.0.9-preview.0` — same string.

So there's no version mapping at all. The `copilot-native` module downloads `@github/copilot-<platform>@${project.version}` from npm. One version, everywhere. No extra property needed.

1917-java-embed-rust-cli-runtime-remove-before-merge/supply-chain-security-to-stephentoub.md:23

  • Python is no longer missing these mitigations. get_runtime_lib_packument_url and get_runtime_lib_url honor COPILOT_NPM_REGISTRY_URL (python/copilot/_cli_version.py:145-162), and _cli_download.py:421-432 fails closed unless npm Subresource Integrity verifies. Leaving this note as written gives an incorrect supply-chain assessment.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-6-platform-detection-darwin-arm64/src/main/java/com/github/copilot/spike/platform/PlatformDetector.java:99
  • detectLinuxLibc() returns UNKNOWN when /proc/self/exe cannot be read or parsed, but this fallback classifies every non-musl result as glibc. On a musl host with restricted /proc, this selects an incompatible linux-* binary. Fail closed unless libc is explicitly GLIBC or MUSL.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-6-platform-detection-linux-x64/src/main/java/com/github/copilot/spike/platform/PlatformDetector.java:102
  • detectLinuxLibc() returns UNKNOWN when /proc/self/exe cannot be read or parsed, but this fallback classifies every non-musl result as glibc. On a musl host with restricted /proc, this selects an incompatible linux-* binary. Fail closed unless libc is explicitly GLIBC or MUSL.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-6-platform-detection-win32-x64/src/main/java/com/github/copilot/spike/platform/PlatformDetector.java:109
  • detectLinuxLibc() returns UNKNOWN when /proc/self/exe cannot be read or parsed, but this else classifies every non-musl result as glibc. On a musl host with restricted /proc, this selects an incompatible linux-* binary. Fail closed unless libc is explicitly GLIBC or MUSL.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/src/main/java/com/github/copilot/spike/mrjar/QueueInputStream.java:69
  • The EOF check precedes argument validation and the zero-length case. Consequently read(buffer, off, 0) returns -1 after EOF instead of the InputStream-required 0, and invalid/null arguments can be silently accepted at EOF. Validate the range and handle len == 0 first.
        if (eof) {
            return -1;
        }
        if (len == 0) {
            return 0;

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/QueueInputStream.java:82

  • The EOF check precedes argument validation and the zero-length case. Consequently read(buffer, off, 0) returns -1 after EOF instead of the InputStream-required 0, and invalid/null arguments can be silently accepted at EOF. Validate the range and handle len == 0 first.
        if (eof) {
            return -1;
        }
        if (len == 0) {
            return 0;

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/pom.xml:17

  • The project metadata claims a Java 25 FFM implementation, but the Java 25 source tree only overlays ReaderThreadFactory; NativeBindingProvider remains the baseline JNA implementation on all JDKs. This description should reflect the actual JNA + platform/virtual-reader comparison.
    <name>Spike 3.4 — Multi-Release JAR: JNA (17) + FFM (25)</name>
    <description>
        Demonstrates the multi-release JAR pattern for native callback bridging:
        - JDK 17 baseline: JNA callback + QueueInputStream
        - JDK 25 overlay: FFM (Panama) upcall stub + QueueInputStream
        Same NativeBindingProvider surface, different implementation selected at runtime.

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/src/main/java/com/github/copilot/spike/mrjar/CallbackMrJarMain.java:134

  • The timeout path continues into connection_write, connection_close, and host shutdown while the Rust worker may still be invoking the callback. That can race native teardown with an active JNA callback, and the spike still exits successfully. Treat timeout as failure and drain/cancel callbacks before closing the connection.
        boolean completed = allCallbacksDone.await(10, TimeUnit.SECONDS);
        if (completed) {
            LOG.info("All callbacks completed. activeCallbacks=" + activeCallbacks.get());
        } else {
            LOG.warning("Timed out! activeCallbacks=" + activeCallbacks.get());
        }

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/CallbackSpikeMain.java:91

  • A local variable is not guaranteed to keep a JNA callback reachable after its last Java use; the JIT may consider callback dead while the detached Rust thread still holds its function pointer. This can leave a dangling callback and crash the JVM. Retain it in a field or call Reference.reachabilityFence(callback) only after native callback production and connection teardown are complete.
        // --- Create the JNA callback ---
        // CRITICAL: hold this as a strong reference to prevent GC!
        // If this gets GC'd, the native function pointer becomes dangling → JVM crash.
        CallbackTestLibrary.OutboundCallback callback = (Pointer userData, Pointer data, int len) -> {

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/README.md:25

  • This build command names a sibling directory that does not exist; the project directory is java-program-that-invokes-rust-dll-jdk17. Following the README from rust-dll therefore fails before Maven runs.
cd ../java-program-that-invokes-rust-dll

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/dependency-reduced-pom.xml:2

  • This is Maven Shade’s generated build output, not a maintained project input. Committing it duplicates the POM, is already stale (it still describes the nonexistent FFM overlay), and will be rewritten on every package build. Remove it and disable/ignore dependency-reduced-pom.xml generation.
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">

1917-java-embed-rust-cli-runtime-remove-before-merge/djl_temp2.txt:1

  • This opaque base64 dump is an unexplained temporary artifact and provides no reviewable or maintainable project input. The parent directory is explicitly marked remove-before-merge; remove this file rather than shipping encoded scratch data.
cGx1Z2lucyB7CiAgICBhaS5kamwuamF2YVByb2plY3QKICAgIGFpLmRqbC5w

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/README.md:32

  • There is no FFM path in this project. NativeBindingProvider explicitly uses JNA on every JDK, and the only Java 25 overlay is ReaderThreadFactory, which changes the reader to a virtual thread. Rewrite the JDK 25 section and comparison table as “JNA + virtual reader” rather than claiming an FFM upcall implementation.
## Run on JDK 25 (FFM path)

```sh
java --enable-native-access=ALL-UNNAMED ^
     -Djava.library.path=../rust-dll/target/debug ^

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/src/main/java/com/github/copilot/spike/mrjar/NativeBindingProvider.java:30

  • The Rust ABI uses bool (one byte; compare the explicit UnmanagedType.U1 mappings in dotnet/src/FfiRuntimeHost.cs:364-387) and size_t (64 bits on every supported target), while these JNA declarations use Java boolean and int, both 32-bit native mappings. This does not faithfully exercise the real ABI and can truncate callback/frame lengths or misread return values. Introduce explicit one-byte boolean and native-size size_t mappings.
        boolean host_shutdown(int serverHandle);
        int connection_open(int serverHandle, OutboundCallback callback, Pointer userData, int burstCount);
        boolean connection_write(int connectionHandle, byte[] data, int len);
        boolean connection_close(int connectionHandle);

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/CallbackTestLibrary.java:52

  • The native function takes size_t and returns Rust/C bool, but this declaration maps them as 32-bit Java int and JNA’s 32-bit boolean. All supported targets have 64-bit size_t, while the bool ABI is one byte (the .NET binding uses UnmanagedType.U1). Use explicit native-size and one-byte mappings throughout this interface.
     * @param len              length of data
     * @return true on success
     */
    boolean connection_write(int connectionHandle, byte[] data, int len);

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/CallbackSpikeMain.java:175

  • The timeout path still writes to and closes the connection, signals EOF, and shuts down the host while the detached Rust thread may be inside or about to enter the callback. This races native teardown with an active JNA callback and still exits successfully. Treat timeout as failure and drain/cancel callbacks before teardown.
        boolean completed = allCallbacksDone.await(10, TimeUnit.SECONDS);
        if (completed) {
            LOG.info("All callbacks completed. activeCallbacks=" + activeCallbacks.get());
        } else {
            LOG.warning("Timed out waiting for callbacks! activeCallbacks=" + activeCallbacks.get());
        }

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/dependency-reduced-pom.xml:2

  • This is Maven Shade’s generated build output, not a maintained project input. Committing it duplicates the source POM and will be rewritten by package builds. Remove it and disable or ignore dependency-reduced-pom.xml generation.
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">

1917-java-embed-rust-cli-runtime-remove-before-merge/djl_temp3.txt:1

  • This opaque base64 dump is an unexplained temporary artifact and provides no reviewable or maintainable project input. The parent directory is explicitly marked remove-before-merge; remove this file rather than shipping encoded scratch data.
cGx1Z2lucyB7CiAgICBhaS5kamwuamF2YVByb2plY3QKICAgIGFpLmRqbC5w

1917-java-embed-rust-cli-runtime-remove-before-merge/djl_temp4.txt:1

  • This opaque base64 dump is an unexplained temporary artifact and provides no reviewable or maintainable project input. The parent directory is explicitly marked remove-before-merge; remove this file rather than shipping encoded scratch data.
cm9vdFByb2plY3QubmFtZSA9ICJkamwiCgpwbHVnaW5zIHsKICAgIGlkKCJv
  • Files reviewed: 44/47 changed files
  • Comments generated: 2
  • Review effort level: Medium


int connectionOpen(int serverHandle, QueueInputStream queueIn,
AtomicInteger activeCallbacks, int burstCount) {
LOG.info("[JNA] Creating JNA Callback (new Java thread per invocation)...");
Comment on lines +40 to +43
1. **JNA thread attachment** — each callback invocation gets a new JNA-attached
Java thread (automatic, no manual `AttachCurrentThread` needed).
2. **PipedStream failure** — `PipedInputStream`/`PipedOutputStream` does NOT work
because JNA's short-lived callback threads cause "Write end dead" errors.
Add the isolated Spike 3.8 experiment for the callback-dependent runtime ABI
using JNA 5.19.1 and a Rust native fixture.

- verify one synchronous callback and five Rust-thread callbacks on the JVM
- build and run a GraalVM Native Image executable with application-specific
  proxy, reflection, and JNI reachability metadata
- demonstrate that JNA callback upcalls fail before entering Java callback code
  in the tested Windows x64 GraalVM 25.0.4 configuration
- document the exact toolchain and platform constraints so the result is not
  extrapolated to untested systems
- resolve Question 3.8 with actionable JNA version, artifact, optionality,
  consumer setup, and Native Image support decisions

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 24, 2026 21:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Comments suppressed due to low confidence (19)

1917-java-embed-rust-cli-runtime-remove-before-merge/20260722-prompts-3_1-and-3_2-gaps.md:15

  • ${project.version} is the Java SDK version, not the npm runtime version. The current checkout has Java 1.0.9-SNAPSHOT but @github/copilot-linux-x64 1.0.73, so this command would request a nonexistent npm package. Pin the npm runtime version separately and use that property for npm pack.
**Resolution:** Skip `package.json` entirely for the native module. Use `exec-maven-plugin` to run `npm pack @github/copilot-<platform>@<version>` for each platform, then extract with `tar`. Version comes from `${project.version}` (SDK and npm versions are identical). Integrity is verified by a build step that reads the `integrity` field from the monorepo's `nodejs/package-lock.json` for each platform package and checks the SHA-512 of the downloaded `.tgz`, mirroring Rust's `resolve_version_and_integrity` → `cached_download` → verify pattern in `build/in_process.rs`.

1917-java-embed-rust-cli-runtime-remove-before-merge/20260722-prompts-3_1-and-3_2-gaps.md:41

  • This resolution contradicts both the preceding correct statement and the repository: the Maven SDK and npm runtime have independent versions. Using ${project.version} for the npm coordinate will fail whenever those streams differ, as they do now. Keep the Maven artifact on ${project.version} and introduce a separately pinned runtime version.
The npm package version and the SDK version are the **same version**. `@github/copilot-linux-x64@1.0.9-preview.0` and `com.github:copilot-sdk-java:1.0.9-preview.0` — same string.

So there's no version mapping at all. The `copilot-native` module downloads `@github/copilot-<platform>@${project.version}` from npm. One version, everywhere. No extra property needed.

1917-java-embed-rust-cli-runtime-remove-before-merge/supply-chain-security-to-stephentoub.md:16

  • Python does not only use a hardcoded registry. _cli_version.py:149-162 honors COPILOT_NPM_REGISTRY_URL, and _cli_download.py:376-432 downloads lazily only when the in-process runtime is selected. The current wording overstates both the hardcoding and build-time behavior.
    1917-java-embed-rust-cli-runtime-remove-before-merge/supply-chain-security-to-stephentoub.md:23
  • These mitigation conclusions are outdated. Python supports a registry override (_cli_version.py:149-162) and fails closed unless npm Subresource Integrity is present and matches (_cli_download.py:421-432). Leaving this as written incorrectly singles out .NET and reports Python as unverified.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-6-platform-detection-linux-x64/src/main/java/com/github/copilot/spike/platform/PlatformDetector.java:102
  • When ELF inspection fails or returns an unrecognized interpreter, detectLinuxLibc() returns UNKNOWN, but this line silently selects the glibc artifact. On a musl host with /proc unavailable, that deterministically chooses the wrong library. Fail closed on UNKNOWN rather than treating it as glibc.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-6-platform-detection-darwin-arm64/src/main/java/com/github/copilot/spike/platform/PlatformDetector.java:99
  • When ELF inspection fails or returns an unrecognized interpreter, detectLinuxLibc() returns UNKNOWN, but this line silently selects the glibc artifact. On a musl host with /proc unavailable, that deterministically chooses the wrong library. Fail closed on UNKNOWN rather than treating it as glibc.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-6-platform-detection-win32-x64/src/main/java/com/github/copilot/spike/platform/PlatformDetector.java:108
  • UNKNOWN currently falls through to the glibc classifier. If /proc/self/exe cannot be read on a musl host, this selects an incompatible binary instead of reporting that detection failed. Handle GLIBC explicitly and reject UNKNOWN.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/QueueInputStream.java:78
  • This override violates the InputStream argument-validation contract: after EOF it returns -1 for invalid offsets, and with len == 0 it can return 0 for a null buffer. Validate the range before checking stream state.
    public int read(byte[] b, int off, int len) throws IOException {
        if (eof) {

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/src/main/java/com/github/copilot/spike/mrjar/QueueInputStream.java:65

  • This override violates the InputStream argument-validation contract: after EOF it returns -1 for invalid offsets, and with len == 0 it can return 0 for a null buffer. Validate the range before checking stream state.
    public int read(byte[] b, int off, int len) throws IOException {
        if (eof) {

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/README.md:28

  • This documents an FFM path that the project does not contain. NativeBindingProvider explicitly uses JNA on every JDK, and the JDK 25 overlay only changes the reader to a virtual thread. The native-access flag, expected log text, and FFM row below are therefore misleading; update this README and the POM metadata to describe JNA + virtual threads.
## Run on JDK 25 (FFM path)

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/CallbackSpikeMain.java:206

  • completed only means every callback reached enqueue; it does not mean the reader consumed or decoded every frame. Reader exceptions are swallowed, and join(2000) is not checked, so this can print SUCCESS while the reader failed or remains blocked. Track reader completion/errors separately and require the reader to finish before reporting bridge success.
        LOG.info("  - QueueInputStream bridging: " + (completed ? "SUCCESS" : "TIMEOUT"));

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/CallbackSpikeMain.java:227

  • completed only means every callback reached enqueue; it does not mean the reader consumed or decoded every frame. Reader exceptions are swallowed, and join(2000) is not checked, so this can print SUCCESS while the reader failed or remains blocked. Track reader completion/errors separately and require the reader to finish before reporting bridge success.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/src/main/java/com/github/copilot/spike/mrjar/CallbackMrJarMain.java:155
  • completed is decremented by the callback wrapper immediately after enqueueing, not by the reader. Consequently this reports SUCCESS even if the reader throws, parses a truncated frame, or is still alive after the unchecked timed join. Record and propagate reader completion before claiming the bridge succeeded.
        LOG.info("  - QueueInputStream bridging: " + (completed ? "SUCCESS" : "TIMEOUT"));

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/rust-dll/README.md:33

  • The Java side uses QueueInputStream, not a piped stream, and the associated spike explicitly rejects PipedInputStream because of its writer-thread checks. This sentence states the opposite bridging strategy.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/README.md:25
  • This cd target does not exist; the sibling directory added by this PR is named java-program-that-invokes-rust-dll-jdk17. Following the documented build steps therefore fails before Maven runs.
cd ../java-program-that-invokes-rust-dll

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/pom.xml:15

  • The implementation no longer pipes through PipedInputStream/PipedOutputStream; it uses QueueInputStream and documents piped streams as rejected. Update the artifact metadata so it describes the experiment actually being built.
        Minimal program to test JNA callback invocation from a native (Rust) thread,
        piping data through PipedInputStream/PipedOutputStream.

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/dependency-reduced-pom.xml:1

  • This is generated output from maven-shade-plugin, not a source POM. Committing it duplicates pom.xml and will become stale whenever the real build changes. Remove it from source control and configure the shade execution to place or suppress the dependency-reduced POM as appropriate.
<?xml version="1.0" encoding="UTF-8"?>

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/dependency-reduced-pom.xml:1

  • This is generated output from maven-shade-plugin, not a source POM. Committing it duplicates pom.xml and will become stale whenever the real build changes. Remove it from source control and configure the shade execution to place or suppress the dependency-reduced POM as appropriate.
<?xml version="1.0" encoding="UTF-8"?>

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/CallbackSpikeMain.java:91

  • A local variable does not guarantee callback reachability until the end of its lexical scope; the JIT may treat it as dead after the last Java use (the connection_open call) while the detached Rust thread still owns the function pointer. Keep it in lifecycle state or call Reference.reachabilityFence(callback) after connection shutdown so GC cannot invalidate an in-flight native callback.
        CallbackTestLibrary.OutboundCallback callback = (Pointer userData, Pointer data, int len) -> {
  • Files reviewed: 62/66 changed files
  • Comments generated: 10
  • Review effort level: Medium

Comment on lines +77 to +78
public int read(byte[] b, int off, int len) throws IOException {
if (eof) {
Comment on lines +64 to +65
public int read(byte[] b, int off, int len) throws IOException {
if (eof) {
On JDK 25+, the `java25-multi-release` profile activates automatically and
compiles `src/main/java25/` into `META-INF/versions/25/`.

## Run on JDK 25 (FFM path)
LOG.info(" - Server handle: " + serverHandle);
LOG.info(" - Connection handle: " + connHandle);
LOG.info(" - Messages received: " + burstCount);
LOG.info(" - QueueInputStream bridging: " + (completed ? "SUCCESS" : "TIMEOUT"));

## What it does

Exports 5 `extern "C"` functions mirroring the real `runtime.node` C ABI:
Comment on lines +31 to +33
The key behavior is `connection_open`: the callback is invoked on a **different
native thread** than the caller, exactly like the real runtime. This lets us
verify JNA's automatic thread attachment and the PipedStream bridging strategy.
@@ -0,0 +1,71 @@
<?xml version="1.0" encoding="UTF-8"?>
AtomicInteger activeCallbacks, int burstCount) {
LOG.info("[JNA] Creating JNA Callback (new Java thread per invocation)...");

callbackRef = (Pointer userData, Pointer data, int len) -> {
AtomicInteger activeCallbacks, int burstCount) {
LOG.info("[JNA] Creating JNA Callback (new Java thread per invocation)...");

callbackRef = (Pointer userData, Pointer data, int len) -> {
Comment on lines +136 to +137
CallbackTestLibrary.OutboundCallback callback =
new OutboundCallbackImpl(queueIn, activeCallbacks, allCallbacksDone);
Copilot AI review requested due to automatic review settings July 24, 2026 21:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Comments suppressed due to low confidence (21)

1917-java-embed-rust-cli-runtime-remove-before-merge/supply-chain-security-to-stephentoub.md:23

  • This security assessment is stale: Python now honors COPILOT_NPM_REGISTRY_URL in _cli_version.py:145-162 and requires npm Subresource Integrity before loading the runtime in _cli_download.py:418-432. As written, the note incorrectly identifies Python as an unmitigated supply-chain gap.
    1917-java-embed-rust-cli-runtime-remove-before-merge/20260722-prompts-3_1-and-3_2-gaps.md:43
  • This resolution contradicts both the preceding section and the repository: the Java SDK is 1.0.9-SNAPSHOT (java/pom.xml:10) while the runtime packages are 1.0.73 (nodejs/package-lock.json:702-721). Using ${project.version} would therefore request a different, potentially nonexistent npm artifact; keep an explicit independently pinned runtime version.
The npm package version and the SDK version are the **same version**. `@github/copilot-linux-x64@1.0.9-preview.0` and `com.github:copilot-sdk-java:1.0.9-preview.0` — same string.

So there's no version mapping at all. The `copilot-native` module downloads `@github/copilot-<platform>@${project.version}` from npm. One version, everywhere. No extra property needed.

Gap #3 is not a gap.

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-6-platform-detection-linux-x64/src/main/java/com/github/copilot/spike/platform/PlatformDetector.java:102

  • UNKNOWN currently falls through to the glibc classifier. If /proc/self/exe is unavailable or ELF parsing fails on musl, this deterministically selects the incompatible linux-* library instead of reporting that detection failed. Fail closed when libc is unknown.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-6-platform-detection-darwin-arm64/src/main/java/com/github/copilot/spike/platform/PlatformDetector.java:99
  • UNKNOWN currently falls through to the glibc classifier. If /proc/self/exe is unavailable or ELF parsing fails on musl, this selects an incompatible linux-* library instead of reporting failed detection. Fail closed when libc is unknown.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-6-platform-detection-win32-x64/src/main/java/com/github/copilot/spike/platform/PlatformDetector.java:108
  • Any UNKNOWN libc result is treated as glibc here. On a musl host where /proc/self/exe cannot be parsed, this silently chooses the incompatible linux-* runtime. Preserve deterministic platform selection by rejecting UNKNOWN.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/QueueInputStream.java:83
  • The bulk-read contract is checked in the wrong order and lacks upfront bounds validation. After EOF, read(b, off, 0) returns -1 although InputStream requires 0; invalid ranges can also block in queue.take() before eventually failing. Validate arguments and zero length before checking EOF.
        if (eof) {
            return -1;
        }
        if (len == 0) {
            return 0;
        }

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/src/main/java/com/github/copilot/spike/mrjar/QueueInputStream.java:70

  • The bulk-read contract is checked in the wrong order and lacks upfront bounds validation. After EOF, read(b, off, 0) returns -1 although InputStream requires 0; invalid ranges can also block in queue.take() before eventually failing. Validate arguments and zero length before checking EOF.
        if (eof) {
            return -1;
        }
        if (len == 0) {
            return 0;
        }

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/QueueInputStream.java:83

  • The bulk-read contract is checked in the wrong order and lacks upfront bounds validation. After EOF, read(b, off, 0) returns -1 although InputStream requires 0; invalid ranges can also block in queue.take() before eventually failing. Validate arguments and zero length before checking EOF.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/java-program-that-invokes-rust-dll-mr-jar-17-25/src/main/java/com/github/copilot/spike/mrjar/QueueInputStream.java:70
  • The bulk-read contract is checked in the wrong order and lacks upfront bounds validation. After EOF, read(b, off, 0) returns -1 although InputStream requires 0; invalid ranges can also block in queue.take() before eventually failing. Validate arguments and zero length before checking EOF.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/CallbackSpikeMain.java:175
  • A callback timeout is only logged, so this verification executable continues through cleanup and exits successfully even though the behavior under test failed. Make the timeout fail the spike so CI or a developer cannot record a false-positive result.
        } else {
            LOG.warning("Timed out waiting for callbacks! activeCallbacks=" + activeCallbacks.get());
        }

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/src/main/java/com/github/copilot/spike/mrjar/CallbackMrJarMain.java:134

  • A callback timeout is only logged, so this verification executable continues and exits successfully even though the callback/threading behavior failed. Throw on timeout to make the spike result reliable in automation.
        } else {
            LOG.warning("Timed out! activeCallbacks=" + activeCallbacks.get());
        }

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/java-program-that-invokes-rust-dll-mr-jar-17-25/src/main/java/com/github/copilot/spike/mrjar/CallbackMrJarMain.java:134

  • A callback timeout is only logged, so this verification executable continues and exits successfully even though the callback/threading behavior failed. Throw on timeout to make the spike result reliable in automation.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/README.md:4
  • This README describes an FFM implementation that is not present. NativeBindingProvider explicitly uses JNA on every JDK and says FFM is deferred; the JDK 25 overlay only changes the reader to a virtual thread. Update the title, JDK 25 run instructions, expected logs, and comparison table to describe JNA plus platform/virtual reader threads.
# Multi-Release JAR Spike: JNA (JDK 17) vs FFM (JDK 25)

Demonstrates the multi-release JAR pattern for native callback bridging,
matching the Copilot SDK's existing `InternalExecutorProvider` approach.

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/java-program-that-invokes-rust-dll-mr-jar-17-25/README.md:4

  • This README describes an FFM implementation that is not present. NativeBindingProvider explicitly uses JNA on every JDK and says FFM is deferred; the JDK 25 overlay only changes the reader to a virtual thread. Update the title, JDK 25 run instructions, expected logs, and comparison table to describe JNA plus platform/virtual reader threads.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/pom.xml:17
  • The artifact metadata claims that JDK 25 swaps in an FFM binding, but the only overlay class is ReaderThreadFactory; NativeBindingProvider remains the baseline JNA implementation on all JDKs. Correct the POM metadata so published/build output describes the actual experiment.
    <name>Spike 3.4 — Multi-Release JAR: JNA (17) + FFM (25)</name>
    <description>
        Demonstrates the multi-release JAR pattern for native callback bridging:
        - JDK 17 baseline: JNA callback + QueueInputStream
        - JDK 25 overlay: FFM (Panama) upcall stub + QueueInputStream
        Same NativeBindingProvider surface, different implementation selected at runtime.

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/src/main/java/com/github/copilot/spike/mrjar/NativeBindingProvider.java:30

  • These JNA signatures do not match the Rust C ABI being exercised. Rust uses pointer-sized usize/C size_t for both lengths, but Java int is always 32-bit; JNA boolean also uses its default C-boolean mapping rather than Rust's one-byte bool (compare the explicit nuint/U1 mappings in dotnet/src/FfiRuntimeHost.cs:346-382). Define a pointer-sized JNA IntegerType for lengths and an explicit one-byte boolean mapping, including the callback signature.
        int host_start();
        boolean host_shutdown(int serverHandle);
        int connection_open(int serverHandle, OutboundCallback callback, Pointer userData, int burstCount);
        boolean connection_write(int connectionHandle, byte[] data, int len);
        boolean connection_close(int connectionHandle);

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/CallbackTestLibrary.java:42

  • The JNA mapping does not match the fixture's Rust C ABI: callback/write lengths are Rust usize (C size_t) but mapped as 32-bit Java int, and Rust's one-byte bool is mapped as JNA's default boolean. This can truncate lengths and misread returns on the 64-bit targets the epic supports. Use explicit pointer-sized length and one-byte boolean mappings throughout this interface.
    int connection_open(int serverHandle, OutboundCallback callback, Pointer userData, int burstCount);

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/rust-dll/README.md:21

  • The fixture now exports six functions because callback_once was added, but this table still says five and omits that control path. It also calls the bridge PipedStream even though both Java spikes use QueueInputStream. Keep the fixture documentation aligned with the experiment.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/rust-dll/README.md:33
  • The Java side uses QueueInputStream, and its own findings explicitly reject PipedStream because of JNA callback thread lifetimes. Calling this a PipedStream verification misstates what this fixture demonstrates.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/README.md:26
  • This build command targets a directory that does not exist; the added sibling project is named java-program-that-invokes-rust-dll-jdk17. Following the README from rust-dll therefore fails before Maven runs.
cd ../java-program-that-invokes-rust-dll
mvn package -q

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/pom.xml:15

  • The project metadata says data is piped through PipedInputStream/PipedOutputStream, but the implementation uses QueueInputStream and explicitly rejects PipedStream. Correct this description so generated artifact metadata does not report the opposite design.
        Minimal program to test JNA callback invocation from a native (Rust) thread,
        piping data through PipedInputStream/PipedOutputStream.
  • Files reviewed: 62/67 changed files
  • Comments generated: 3
  • Review effort level: Medium

Comment on lines +12 to +18
<name>Spike 3.4 — Multi-Release JAR: JNA (17) + FFM (25)</name>
<description>
Demonstrates the multi-release JAR pattern for native callback bridging:
- JDK 17 baseline: JNA callback + QueueInputStream
- JDK 25 overlay: FFM (Panama) upcall stub + QueueInputStream
Same NativeBindingProvider surface, different implementation selected at runtime.
</description>
Comment on lines +26 to +30
int host_start();
boolean host_shutdown(int serverHandle);
int connection_open(int serverHandle, OutboundCallback callback, Pointer userData, int burstCount);
boolean connection_write(int connectionHandle, byte[] data, int len);
boolean connection_close(int connectionHandle);
* @param burstCount number of messages the native thread will send
* @return connection handle (0 = failure)
*/
int connection_open(int serverHandle, OutboundCallback callback, Pointer userData, int burstCount);
The original four questions in 3.9 targeted a parameter ("entrypoint") that
does not exist in the actual ABI signature, and covered only host_start while
leaving connection_open's three nullable metadata buffers (ext_source,
ext_name, conn_token) completely unaddressed.

Rewrite 3.9 as "C ABI parameter semantics" with eleven numbered questions
spanning all five functions:

- argv_json: full valid-argument set and --embedded-host semantics
- env_json: complete key inventory (auth token, proxy URLs, log level, others)
- argv_json / env_json nullability and zero-length handling
- host_start failure and error-retrieval companion function
- ext_source and ext_name semantics, encoding, and null-safety
- conn_token: per-connection vs. global auth, format
- user_data: confirm null is safe; Java should use closure capture not the
  void-pointer cookie mechanism
- multiple concurrent connections per server handle
- wire frame format for connection_write and on_outbound
- buffer lifetime for connection_write

Name the spike spike-3-9-deep-entrypoint-questions and require a complete
call-by-call reference as the deliverable.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 24, 2026 21:46

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Comments suppressed due to low confidence (32)

1917-java-embed-rust-cli-runtime-remove-before-merge/supply-chain-security-to-stephentoub.md:23

  • This assessment is stale: Python now honors COPILOT_NPM_REGISTRY_URL and fails closed unless the downloaded native runtime passes npm Subresource Integrity verification (python/copilot/_cli_version.py:145-162, _cli_download.py:325-360,418-432). As written, the note incorrectly identifies .NET as the only configurable SDK and Python as having neither mitigation.
    1917-java-embed-rust-cli-runtime-remove-before-merge/20260722-prompts-3_1-and-3_2-gaps.md:41
  • The resolution reverses the verified version relationship. The current Java artifact is 1.0.9-SNAPSHOT (java/pom.xml:10) while @github/copilot is 1.0.73 (nodejs/package-lock.json:702-705), so ${project.version} would request a different—and potentially nonexistent—npm runtime. Keep a separate pinned runtime version and record both versions in the packaged metadata.
The npm package version and the SDK version are the **same version**. `@github/copilot-linux-x64@1.0.9-preview.0` and `com.github:copilot-sdk-java:1.0.9-preview.0` — same string.

So there's no version mapping at all. The `copilot-native` module downloads `@github/copilot-<platform>@${project.version}` from npm. One version, everywhere. No extra property needed.

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-6-platform-detection-win32-x64/src/main/java/com/github/copilot/spike/platform/PlatformDetector.java:108

  • The fallback branch treats UNKNOWN libc detection as glibc. On a musl host where /proc/self/exe cannot be parsed, the detector therefore selects an incompatible binary rather than failing. Distinguish explicit GLIBC from UNKNOWN.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/java-program-that-invokes-rust-dll-mr-jar-17-25/src/main/java/com/github/copilot/spike/mrjar/NativeBindingProvider.java:30
  • These JNA declarations do not match the Rust ABI: JNA maps Java boolean as a 32-bit C integer while Rust bool/C _Bool is one byte, and Java int is 32-bit while usize/size_t is 64-bit on every supported x64/arm64 target. The fixture's always-true, small values can mask both mismatches. Map boolean results as a byte/custom type and sizes as long/a size_t type, then convert in Java wrappers.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/CallbackTestLibrary.java:60
  • This binding uses Java boolean for Rust bool and Java int for Rust usize. JNA's boolean is a 32-bit C integer, whereas the ABI returns a one-byte _Bool; size_t is 64-bit on the supported targets. The current fixture values hide the mismatch. Use a byte/custom boolean mapping and a 64-bit size_t mapping throughout the interface and callers.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/src/main/java/com/github/copilot/spike/mrjar/NativeBindingProvider.java:30
  • These JNA declarations mismatch the Rust ABI: Java boolean maps to a 32-bit C integer rather than Rust's one-byte bool, and Java int cannot represent the 64-bit usize length used on supported targets. Always-true returns and small frames can let the spike pass despite the invalid binding. Use byte/custom boolean and 64-bit size_t mappings.
        boolean host_shutdown(int serverHandle);
        int connection_open(int serverHandle, OutboundCallback callback, Pointer userData, int burstCount);
        boolean connection_write(int connectionHandle, byte[] data, int len);
        boolean connection_close(int connectionHandle);

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/CallbackTestLibrary.java:52

  • This declaration combines two ABI mismatches: JNA's Java boolean is a 32-bit C integer while Rust returns a one-byte bool, and Java int is 32-bit while Rust's usize length is 64-bit on x64/arm64. The fixture's small payloads and always-true result mask this. Map the result and size_t explicitly and update the callers.
    boolean connection_write(int connectionHandle, byte[] data, int len);

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-6-platform-detection-linux-x64/src/main/java/com/github/copilot/spike/platform/PlatformDetector.java:100

  • UNKNOWN currently falls through to the glibc classifier. If /proc/self/exe is unavailable or PT_INTERP is unrecognized on a musl host, this selects the incompatible glibc library instead of reporting that detection failed. Only map an explicit GLIBC result to linux-*; fail for UNKNOWN.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-6-platform-detection-darwin-arm64/src/main/java/com/github/copilot/spike/platform/PlatformDetector.java:97
  • UNKNOWN currently falls through to the glibc classifier. If PT_INTERP detection fails on a musl Linux host, this selects the incompatible glibc library. Only return linux-* for an explicit GLIBC result and fail closed for UNKNOWN.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/java-program-that-invokes-rust-dll-mr-jar-17-25/src/main/java/com/github/copilot/spike/mrjar/QueueInputStream.java:68
  • This violates the InputStream contract in two observable cases: after EOF, a zero-length read must return 0 rather than -1, and invalid (off,len) ranges must be rejected before waiting for queue data. Currently an invalid request can block indefinitely, while len == 0 can bypass bounds validation. Validate the range first, then handle zero length before EOF.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/QueueInputStream.java:81
  • The method checks EOF before the required zero-length and range semantics. Thus read(b, off, 0) returns -1 after EOF, and invalid ranges may block on queue.take() instead of immediately throwing. Validate (off,len) first and return 0 for zero length before checking EOF.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/src/main/java/com/github/copilot/spike/mrjar/QueueInputStream.java:68
  • This ordering breaks InputStream.read(byte[],off,len): a zero-length read must return 0 even at EOF, and invalid bounds must fail before any blocking read. Here EOF wins and malformed ranges can wait on the queue. Validate first, then handle zero length, then EOF.
    public int read(byte[] b, int off, int len) throws IOException {
        if (eof) {
            return -1;
        }
        if (len == 0) {

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/QueueInputStream.java:81

  • The range and zero-length requirements of InputStream are evaluated too late. len == 0 incorrectly returns -1 after EOF, and invalid offsets can block while waiting for data. Perform bounds validation first and process zero length before EOF.
    public int read(byte[] b, int off, int len) throws IOException {
        if (eof) {
            return -1;
        }
        if (len == 0) {

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/CallbackSpikeMain.java:174

  • A callback timeout only emits a warning, after which the program continues cleanup, prints Spike complete, and exits successfully. That makes a failed threading experiment indistinguishable from a passing run to automation. Fail the process when the latch does not complete.
        boolean completed = allCallbacksDone.await(10, TimeUnit.SECONDS);
        if (completed) {
            LOG.info("All callbacks completed. activeCallbacks=" + activeCallbacks.get());
        } else {
            LOG.warning("Timed out waiting for callbacks! activeCallbacks=" + activeCallbacks.get());

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/src/main/java/com/github/copilot/spike/mrjar/CallbackMrJarMain.java:133

  • The timeout path still exits with status 0 and prints a completion summary, so this spike can report success when native callbacks never arrive. Treat the timeout as a failed experiment instead of only logging it.
        boolean completed = allCallbacksDone.await(10, TimeUnit.SECONDS);
        if (completed) {
            LOG.info("All callbacks completed. activeCallbacks=" + activeCallbacks.get());
        } else {
            LOG.warning("Timed out! activeCallbacks=" + activeCallbacks.get());

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/java-program-that-invokes-rust-dll-mr-jar-17-25/src/main/java/com/github/copilot/spike/mrjar/CallbackMrJarMain.java:133

  • On timeout this program still proceeds to print Spike complete and returns success, allowing a broken JNA callback path to pass the spike. Make timeout terminate with a nonzero result.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/java-program-that-invokes-rust-dll-mr-jar-17-25/README.md:32
  • This project has no FFM path. NativeBindingProvider explicitly uses JNA on every JDK and the only JDK 25 overlay is ReaderThreadFactory (virtual versus platform thread). The native-access flag, claimed [JDK-25/FFM] output, and FFM table row therefore document behavior the JAR cannot produce. Rewrite the JDK 25 instructions and table around JNA plus a virtual reader thread.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/java-program-that-invokes-rust-dll-mr-jar-17-25/pom.xml:16
  • The described JDK 25 FFM implementation does not exist: both JDKs load NativeBindingProvider via JNA, while only ReaderThreadFactory is overlaid. Correct the build metadata so it does not claim a second binding implementation.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/README.md:32
  • The documented JDK 25 FFM path is absent. The source says JNA is used on all JDK versions and only the reader-thread factory changes in the multi-release overlay, so this command/output and the later FFM table row are unattainable. Document JNA plus the virtual reader thread instead.
## Run on JDK 25 (FFM path)

```sh
java --enable-native-access=ALL-UNNAMED ^
     -Djava.library.path=../rust-dll/target/debug ^

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/pom.xml:16

  • This metadata claims a JDK 25 FFM implementation, but NativeBindingProvider is the same JNA class on all JDKs; only ReaderThreadFactory has an overlay. Update the artifact description to match the actual MR-JAR swap point.
    <name>Spike 3.4 — Multi-Release JAR: JNA (17) + FFM (25)</name>
    <description>
        Demonstrates the multi-release JAR pattern for native callback bridging:
        - JDK 17 baseline: JNA callback + QueueInputStream
        - JDK 25 overlay: FFM (Panama) upcall stub + QueueInputStream

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/rust-dll/README.md:21

  • The fixture now exports six functions: callback_once was added at src/lib.rs:30-40, but this count and table omit it. That omission hides the synchronous control central to the GraalVM experiment.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/rust-dll/README.md:33
  • The Java side uses QueueInputStream, and the accompanying spike specifically rejects PipedStream because of JNA callback thread affinity. Calling this a PipedStream strategy states the opposite result.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/rust-dll/README.md:33
  • The implemented bridge is QueueInputStream; PipedStream is explicitly documented as rejected by the Java spike. This sentence currently describes the failed alternative as the strategy being verified.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/pom.xml:15
  • The program does not pipe data through PipedInputStream/PipedOutputStream; it uses QueueInputStream and records PipedStream as the rejected alternative. The artifact description should identify the mechanism actually tested.
        Minimal program to test JNA callback invocation from a native (Rust) thread,
        piping data through PipedInputStream/PipedOutputStream.

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/java-program-that-invokes-rust-dll-jdk17/dependency-reduced-pom.xml:1

  • This is a generated Maven Shade output, not source configuration; the accompanying work log explicitly identifies dependency-reduced-pom.xml as a build artifact to exclude. Committing it creates a second POM that Maven will overwrite and that can drift from pom.xml. Remove it and either ignore it or set the Shade output under target.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/dependency-reduced-pom.xml:1
  • dependency-reduced-pom.xml is generated by the Shade plugin and is overwritten on packaging. Keeping it in source duplicates pom.xml and permits stale build metadata; remove it and configure/ignore the generated output outside the source tree.
<?xml version="1.0" encoding="UTF-8"?>

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/dependency-reduced-pom.xml:1

  • This reduced POM is Maven Shade build output rather than a maintained input. It will be regenerated from pom.xml, so committing it adds stale duplicate metadata. Remove it and prevent Shade from writing generated artifacts into the tracked project root.
<?xml version="1.0" encoding="UTF-8"?>

1917-java-embed-rust-cli-runtime-remove-before-merge/djl_temp2.txt:1

  • This is unreferenced base64 scratch output produced by redirecting a GitHub API response (as recorded in 20260722-prompts.md), not a consumable project fixture or source file. Remove the temporary download artifact before merging.
cGx1Z2lucyB7CiAgICBhaS5kamwuamF2YVByb2plY3QKICAgIGFpLmRqbC5w

1917-java-embed-rust-cli-runtime-remove-before-merge/djl_temp3.txt:1

  • This opaque base64 file is a shell scratch artifact from fetching DJL sources and has no references in the repository. It should not be retained as project content; remove it before merge.
cGx1Z2lucyB7CiAgICBhaS5kamwuamF2YVByb2plY3QKICAgIGFpLmRqbC5w

1917-java-embed-rust-cli-runtime-remove-before-merge/djl_temp4.txt:1

  • This file is temporary base64 output from a gh api command, not documented or referenced source. Keeping it adds opaque vendored data with no build or research use. Remove it before merge.
cm9vdFByb2plY3QubmFtZSA9ICJkamwiCgpwbHVnaW5zIHsKICAgIGlkKCJv

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/java-program-that-invokes-rust-dll-mr-jar-17-25/src/main/java/com/github/copilot/spike/mrjar/CallbackMrJarMain.java:78

  • This latch is named and awaited as “all callbacks done,” but it is decremented inside enqueue, before NativeBindingProvider reaches its callback finally block and decrements activeCallbacks. The main thread can therefore close the connection/host while the final native callback is still executing. Move completion signaling into the provider callback's finally after the active count is decremented, or explicitly wait for activeCallbacks == 0 before shutdown.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/src/main/java/com/github/copilot/spike/mrjar/CallbackMrJarMain.java:78
  • The latch counts message enqueue, not callback completion: it reaches zero before the provider's callback finally decrements activeCallbacks. Consequently the main thread may close the native connection and host while a callback is still active. Signal completion after callback teardown in NativeBindingProvider, or drain the active count before shutdown.
                allCallbacksDone.countDown();
  • Files reviewed: 62/67 changed files
  • Comments generated: 0 new
  • Review effort level: Medium

Copilot AI review requested due to automatic review settings July 24, 2026 22:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Comments suppressed due to low confidence (20)

1917-java-embed-rust-cli-runtime-remove-before-merge/supply-chain-security-to-stephentoub.md:23

  • This mitigation summary is stale: Python now honors COPILOT_NPM_REGISTRY_URL for both packument and tarball requests (python/copilot/_cli_version.py:145-162) and refuses to load the runtime unless npm SRI is present and verifies (python/copilot/_cli_download.py:421-432). As written, the security note materially overstates Python's exposure.
    1917-java-embed-rust-cli-runtime-remove-before-merge/20260722-prompts-3_1-and-3_2-gaps.md:41
  • This resolution contradicts the repository's independent version streams: java/pom.xml:10 is currently 1.0.9-SNAPSHOT, while nodejs/package-lock.json:702-721 pins the runtime packages at 1.0.73. Downloading @github/copilot-<platform>@${project.version} can therefore request a nonexistent package; keep a separate pinned runtime version.
The npm package version and the SDK version are the **same version**. `@github/copilot-linux-x64@1.0.9-preview.0` and `com.github:copilot-sdk-java:1.0.9-preview.0` — same string.

So there's no version mapping at all. The `copilot-native` module downloads `@github/copilot-<platform>@${project.version}` from npm. One version, everywhere. No extra property needed.

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-6-platform-detection-linux-x64/src/main/java/com/github/copilot/spike/platform/PlatformDetector.java:102

  • UNKNOWN currently falls through to the glibc classifier. If /proc/self/exe is unavailable or ELF parsing fails on a musl host, this deterministically selects an incompatible glibc library. Fail with a diagnostic unless libc was positively identified.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-6-platform-detection-darwin-arm64/src/main/java/com/github/copilot/spike/platform/PlatformDetector.java:99
  • UNKNOWN currently falls through to the glibc classifier. If /proc/self/exe is unavailable or ELF parsing fails on a musl host, this selects an incompatible runtime. Require a positive GLIBC result instead of guessing.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-6-platform-detection-win32-x64/src/main/java/com/github/copilot/spike/platform/PlatformDetector.java:108
  • Any UNKNOWN libc result is treated as glibc here. On a musl system where /proc is unavailable or parsing fails, the detector will select a binary that cannot load. Preserve the distinction and fail unless GLIBC or MUSL was positively detected.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/CallbackSpikeMain.java:204
  • A local variable is not guaranteed to remain reachable after its last use; the JVM may collect this callback after connection_open returns while Rust still owns its function pointer, causing a native crash. Add a reachability fence after native shutdown (or retain it in a field) rather than merely reporting the local as GC protection.
        LOG.info("  - Callback GC protection: strong reference held as local variable");

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/CallbackSpikeMain.java:225

  • Keeping the callback only in a local does not guarantee reachability after its last Java use; it can be collected while the Rust thread still holds the function pointer. Fence it after native shutdown (or store it in a field) so this JVM control cannot intermittently crash.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/README.md:37
  • The JDK 25 overlay does not implement FFM: only ReaderThreadFactory is overlaid, while NativeBindingProvider explicitly uses JNA on every JDK. The FFM heading, command flag, expected logs, and result table therefore describe behavior this artifact cannot produce.
Logs will show `[JDK-25/FFM]` — upcall stub executes directly on the native
thread, no new Java thread per callback.

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/java-program-that-invokes-rust-dll-mr-jar-17-25/README.md:37

  • The JDK 25 overlay does not implement FFM: only ReaderThreadFactory is overlaid, while NativeBindingProvider uses JNA on every JDK. The FFM heading, native-access flag, expected logs, and result table are therefore inaccurate.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/pom.xml:16
  • The artifact metadata claims a JDK 25 FFM implementation, but this project overlays only ReaderThreadFactory; NativeBindingProvider remains the shared JNA implementation. Correct the name and description so build metadata matches the spike.
    <name>Spike 3.4 — Multi-Release JAR: JNA (17) + FFM (25)</name>
    <description>
        Demonstrates the multi-release JAR pattern for native callback bridging:
        - JDK 17 baseline: JNA callback + QueueInputStream
        - JDK 25 overlay: FFM (Panama) upcall stub + QueueInputStream

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/java-program-that-invokes-rust-dll-mr-jar-17-25/pom.xml:16

  • The artifact metadata claims a JDK 25 FFM implementation, but this project overlays only ReaderThreadFactory; NativeBindingProvider remains the shared JNA implementation. Correct the name and description to match the spike.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/rust-dll/README.md:21
  • This fixture exports six functions because callback_once was added in src/lib.rs:31-40, but the README still says five and omits that key synchronous control path.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/src/main/java/com/github/copilot/spike/mrjar/NativeBindingProvider.java:35
  • The Rust callback declares len as usize (rust-dll/src/lib.rs:6), which is 64-bit on every supported target, but this JNA mapping declares a 32-bit int. The callback closure therefore does not match the native ABI and can truncate lengths; map the size parameter as a 64-bit value and bounds-check before converting for Java arrays.
        void invoke(Pointer userData, Pointer data, int len);

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/java-program-that-invokes-rust-dll-mr-jar-17-25/src/main/java/com/github/copilot/spike/mrjar/NativeBindingProvider.java:35

  • The Rust callback's len is usize (rust-dll/src/lib.rs:6), but this JNA callback maps it to a 32-bit int. On the supported 64-bit targets that is not the declared native ABI and can truncate lengths; use a 64-bit size mapping and validate before converting to Java array sizes.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/CallbackTestLibrary.java:76
  • This maps the Rust callback's usize len to a 32-bit JNA int. Since all target architectures are 64-bit, the callback signature does not match the fixture's C ABI and can truncate lengths. Use a 64-bit size mapping and bounds-check before Java array operations.
        void invoke(Pointer userData, Pointer data, int len);

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/CallbackTestLibrary.java:84

  • This maps the Rust callback's usize len to a 32-bit JNA int. On all supported 64-bit targets that is an ABI mismatch and can truncate lengths, undermining what the GraalVM fixture is intended to test. Use a 64-bit size mapping and validate before converting for Java arrays.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/src/main/java/com/github/copilot/spike/mrjar/NativeBindingProvider.java:29
  • connection_write also maps Rust's 64-bit usize len to a 32-bit JNA int (rust-dll/src/lib.rs:106). This is not the exported ABI on the supported targets; use the same 64-bit size mapping as the corrected callback signature.
        boolean connection_write(int connectionHandle, byte[] data, int len);

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/java-program-that-invokes-rust-dll-mr-jar-17-25/src/main/java/com/github/copilot/spike/mrjar/NativeBindingProvider.java:29

  • connection_write maps Rust's 64-bit usize len to a 32-bit JNA int (rust-dll/src/lib.rs:118). This does not match the native ABI on the supported targets; use a 64-bit size mapping here as well.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/CallbackTestLibrary.java:52
  • The Rust export takes usize len, but this JNA declaration passes a 32-bit int. On the supported 64-bit targets the downcall signature is wrong; map the native size as 64-bit rather than relying on register behavior for small test payloads.
    boolean connection_write(int connectionHandle, byte[] data, int len);

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/CallbackTestLibrary.java:60

  • The Rust connection_write export takes usize len, but this JNA declaration uses a 32-bit int. That is an ABI mismatch on every supported target and can truncate larger payloads; map this parameter as 64-bit.
  • Files reviewed: 62/67 changed files
  • Comments generated: 0 new
  • Review effort level: Medium

- Add spike-3-9-c-abi-parameter-semantics/ with complete call-by-call
  parameter reference derived from all 5 SDK implementations
- Add inline resolution to 3.9 in the ignorance reduction plan
- Answers all 11 open questions: argv/env format, nullability, wire
  framing (LSP Content-Length), buffer lifetime, ext_source/ext_name/
  conn_token (all null), user_data (null safe), one connection per server
Copilot AI review requested due to automatic review settings July 24, 2026 22:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Comments suppressed due to low confidence (18)

1917-java-embed-rust-cli-runtime-remove-before-merge/supply-chain-security-to-stephentoub.md:23

  • This mitigation summary is stale: Python now honors COPILOT_NPM_REGISTRY_URL and refuses to load the native runtime unless npm Subresource Integrity verifies (python/copilot/_cli_version.py:145-162, _cli_download.py:418-432). As written, this note reports a Python supply-chain gap that no longer exists.
    1917-java-embed-rust-cli-runtime-remove-before-merge/20260722-prompts-3_1-and-3_2-gaps.md:41
  • This resolution reverses the correct finding immediately above it. The Java SDK and npm runtime are independently versioned: this branch's Maven project is 1.0.9-SNAPSHOT, while nodejs/package-lock.json:702-721 pins runtime 1.0.73. Using ${project.version} would request a runtime package version that is unrelated and may not exist; keep a separate pinned runtime version.
The npm package version and the SDK version are the **same version**. `@github/copilot-linux-x64@1.0.9-preview.0` and `com.github:copilot-sdk-java:1.0.9-preview.0` — same string.

So there's no version mapping at all. The `copilot-native` module downloads `@github/copilot-<platform>@${project.version}` from npm. One version, everywhere. No extra property needed.

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/README.md:1

  • This README describes an FFM overlay that does not exist. The only JDK 25 overlay is ReaderThreadFactory; NativeBindingProvider remains the same JNA implementation on both JDKs, as CallbackMrJarMain also states. The JDK 25 command, expected logs, and comparison table are therefore misleading and should describe JNA plus a virtual reader thread instead.
# Multi-Release JAR Spike: JNA (JDK 17) vs FFM (JDK 25)

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/java-program-that-invokes-rust-dll-mr-jar-17-25/README.md:1

  • This README describes an FFM overlay that does not exist. The only JDK 25 overlay is ReaderThreadFactory; NativeBindingProvider remains the same JNA implementation on both JDKs, as CallbackMrJarMain also states. The JDK 25 command, expected logs, and comparison table are therefore misleading and should describe JNA plus a virtual reader thread instead.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/pom.xml:16
  • The project metadata still claims the JDK 25 overlay uses FFM, but this project uses JNA on every JDK and overlays only the reader thread factory. This metadata will propagate the obsolete experiment description into the built artifact.
    <name>Spike 3.4 — Multi-Release JAR: JNA (17) + FFM (25)</name>
    <description>
        Demonstrates the multi-release JAR pattern for native callback bridging:
        - JDK 17 baseline: JNA callback + QueueInputStream
        - JDK 25 overlay: FFM (Panama) upcall stub + QueueInputStream

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/java-program-that-invokes-rust-dll-mr-jar-17-25/pom.xml:16

  • The project metadata still claims the JDK 25 overlay uses FFM, but this project uses JNA on every JDK and overlays only the reader thread factory. This metadata will propagate the obsolete experiment description into the built artifact.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-6-platform-detection-linux-x64/src/main/java/com/github/copilot/spike/platform/PlatformDetector.java:101
  • UNKNOWN is silently treated as glibc here. If /proc/self/exe is unavailable or PT_INTERP parsing fails on a musl host, this selects linux-* and loads the incompatible glibc library. Because libc is part of the runtime classifier, fail closed when detection is unknown rather than guessing.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-6-platform-detection-win32-x64/src/main/java/com/github/copilot/spike/platform/PlatformDetector.java:108
  • UNKNOWN is silently treated as glibc here. If /proc/self/exe is unavailable or PT_INTERP parsing fails on a musl host, this selects linux-* and loads the incompatible glibc library. Because libc is part of the runtime classifier, fail closed when detection is unknown rather than guessing.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-6-platform-detection-darwin-arm64/src/main/java/com/github/copilot/spike/platform/PlatformDetector.java:99
  • UNKNOWN is silently treated as glibc here. If /proc/self/exe is unavailable or PT_INTERP parsing fails on a musl host, this selects linux-* and loads the incompatible glibc library. Because libc is part of the runtime classifier, fail closed when detection is unknown rather than guessing.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/src/main/java/com/github/copilot/spike/mrjar/NativeBindingProvider.java:87
  • The real callback already supplies an opaque LSP Content-Length:-framed byte stream; adding a four-byte binary length prefix here produces input that JsonRpcClient cannot parse (JsonRpcClient.java:205-239). This fixture therefore does not validate the production bridge. Have the Rust fixture emit an LSP-framed message and enqueue the callback bytes unchanged.
                byte[] frame = new byte[4 + len];
                frame[0] = (byte) (len >> 24);
                frame[1] = (byte) (len >> 16);
                frame[2] = (byte) (len >> 8);
                frame[3] = (byte) len;

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/CallbackSpikeMain.java:111

  • The real callback already supplies an opaque LSP Content-Length:-framed byte stream; adding a four-byte binary length prefix here produces input that JsonRpcClient cannot parse (JsonRpcClient.java:205-239). This fixture therefore does not validate the production bridge. Have the Rust fixture emit an LSP-framed message and enqueue the callback bytes unchanged.
                byte[] frame = new byte[4 + len];
                frame[0] = (byte) (len >> 24);
                frame[1] = (byte) (len >> 16);
                frame[2] = (byte) (len >> 8);
                frame[3] = (byte) len;

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/src/main/java/com/github/copilot/spike/mrjar/CallbackMrJarMain.java:134

  • A callback timeout is only logged, after which the program continues, shuts down, prints “Spike complete,” and exits successfully. That turns a failed callback/threading experiment into a passing run. Fail the process on timeout, as the Graal JDK17 spike already does.
        } else {
            LOG.warning("Timed out! activeCallbacks=" + activeCallbacks.get());
        }

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/CallbackTestLibrary.java:76

  • This callback signature does not match the C ABI: Rust's usize/C size_t is 64-bit on every supported x64/arm64 target, while Java int is 32-bit. Use a 64-bit size type and checked conversion before getByteArray; update the connection-write length declaration for the same reason.
        void invoke(Pointer userData, Pointer data, int len);

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/CallbackTestLibrary.java:84

  • This callback signature does not match the C ABI: Rust's usize/C size_t is 64-bit on every supported x64/arm64 target, while Java int is 32-bit. Use a 64-bit size type and checked conversion before getByteArray; update the connection-write length declaration for the same reason.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/rust-dll/README.md:21
  • The fixture exports six functions, not five: callback_once is also exported and is central to the synchronous-control experiment described by the adjacent Java README. Document it in this table so the fixture's tested ABI is complete.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/README.md:25
  • This cd target does not exist in the proposed tree; the sibling directory is named java-program-that-invokes-rust-dll-jdk17. Following the documented build steps therefore fails before Maven runs.
cd ../java-program-that-invokes-rust-dll

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/src/main/java/com/github/copilot/spike/mrjar/NativeBindingProvider.java:35

  • This callback does not match the C ABI: Rust's usize/C size_t is 64-bit on every supported x64/arm64 target, while Java int is 32-bit. Use a 64-bit size type with checked conversion before Java array operations, and correct the connection_write length declaration for the same reason.
        void invoke(Pointer userData, Pointer data, int len);

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/java-program-that-invokes-rust-dll-mr-jar-17-25/src/main/java/com/github/copilot/spike/mrjar/NativeBindingProvider.java:35

  • This callback does not match the C ABI: Rust's usize/C size_t is 64-bit on every supported x64/arm64 target, while Java int is 32-bit. Use a 64-bit size type with checked conversion before Java array operations, and correct the connection_write length declaration for the same reason.
  • Files reviewed: 63/68 changed files
  • Comments generated: 3
  • Review effort level: Medium

Comment on lines +83 to +87
byte[] frame = new byte[4 + len];
frame[0] = (byte) (len >> 24);
frame[1] = (byte) (len >> 16);
frame[2] = (byte) (len >> 8);
frame[3] = (byte) len;
Comment on lines +79 to +83
byte[] frame = new byte[4 + len];
frame[0] = (byte) (len >> 24);
frame[1] = (byte) (len >> 16);
frame[2] = (byte) (len >> 8);
frame[3] = (byte) len;
Comment on lines +132 to +134
} else {
LOG.warning("Timed out! activeCallbacks=" + activeCallbacks.get());
}
- No error message channel in C ABI; construct diagnostic strings
- Use IllegalStateException (matching .NET InvalidOperationException)
- Native crash = process termination; do not enable JNA protected mode
- java.util.logging at FINE for startup/teardown, WARNING for callbacks
- Callback try-catch wrapping (Go/Python pattern, better than .NET)
- JNA UncaughtExceptionHandler as secondary defense
- AutoCloseable.close() never throws; best-effort teardown
Copilot AI review requested due to automatic review settings July 24, 2026 22:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Comments suppressed due to low confidence (23)

1917-java-embed-rust-cli-runtime-remove-before-merge/20260722-prompts-3_1-and-3_2-gaps.md:41

  • This resolution is contradicted by the repository's current versions: java/pom.xml:10 is 1.0.9-SNAPSHOT, while nodejs/package-lock.json:703 pins the runtime to 1.0.73. Downloading the npm package at ${project.version} will therefore request a different, often nonexistent version. Keep an explicit npm runtime-version mapping instead of declaring this gap resolved.
The npm package version and the SDK version are the **same version**. `@github/copilot-linux-x64@1.0.9-preview.0` and `com.github:copilot-sdk-java:1.0.9-preview.0` — same string.

So there's no version mapping at all. The `copilot-native` module downloads `@github/copilot-<platform>@${project.version}` from npm. One version, everywhere. No extra property needed.

1917-java-embed-rust-cli-runtime-remove-before-merge/supply-chain-security-to-stephentoub.md:23

  • The Python assessment is outdated. python/copilot/_cli_version.py:149-161 honors COPILOT_NPM_REGISTRY_URL, and _cli_download.py:421-432 fails closed unless npm SRI verification succeeds. As written, this note would incorrectly report both the exposure and the available mitigation.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-6-platform-detection-linux-x64/src/main/java/com/github/copilot/spike/platform/PlatformDetector.java:102
  • UNKNOWN currently falls through to the glibc classifier. If /proc is unavailable or ELF parsing fails on a musl host, this deterministically selects the incompatible linux-* library. Fail closed unless libc was positively identified; the epic explicitly requires deterministic native selection.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-6-platform-detection-darwin-arm64/src/main/java/com/github/copilot/spike/platform/PlatformDetector.java:99
  • UNKNOWN currently falls through to the glibc classifier. If /proc is unavailable or ELF parsing fails on a musl host, this selects an incompatible runtime instead of reporting that classification failed. Require a positive GLIBC result before returning linux-*.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-6-platform-detection-win32-x64/src/main/java/com/github/copilot/spike/platform/PlatformDetector.java:108
  • The else branch also includes LinuxLibc.UNKNOWN, so an ELF/procfs failure on musl is silently classified as glibc. This can load the wrong native binary. Handle GLIBC explicitly and fail for UNKNOWN.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/src/main/java/com/github/copilot/spike/mrjar/NativeBindingProvider.java:30
  • These JNA declarations do not match the Rust C ABI: JNA maps Java boolean to a 32-bit C int, while Rust bool is one byte, and Java int len does not match 64-bit usize on every supported target. This can mis-marshal return values and buffer lengths, invalidating the ABI spike. Bind booleans as a one-byte native value and lengths as a pointer-sized value, converting only in Java wrappers.
        boolean host_shutdown(int serverHandle);
        int connection_open(int serverHandle, OutboundCallback callback, Pointer userData, int burstCount);
        boolean connection_write(int connectionHandle, byte[] data, int len);
        boolean connection_close(int connectionHandle);

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/java-program-that-invokes-rust-dll-mr-jar-17-25/src/main/java/com/github/copilot/spike/mrjar/NativeBindingProvider.java:30

  • These JNA declarations do not match the Rust C ABI: JNA maps Java boolean to a 32-bit C int, while Rust bool is one byte, and Java int len does not match 64-bit usize on every supported target. Bind booleans as a one-byte native value and lengths as a pointer-sized value before relying on this spike's result.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/CallbackTestLibrary.java:52
  • This binding uses Java boolean for Rust's one-byte bool and Java int for Rust's 64-bit usize length. JNA's default boolean mapping is a 32-bit C int, so these signatures do not mirror the fixture ABI and can yield platform-dependent results. Use explicit one-byte boolean and pointer-sized length mappings throughout the interface and callers.
    boolean connection_write(int connectionHandle, byte[] data, int len);

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/CallbackTestLibrary.java:60

  • This binding uses Java boolean for Rust's one-byte bool and Java int for Rust's 64-bit usize length. JNA's default boolean mapping is a 32-bit C int, so these signatures do not mirror the fixture ABI and can yield platform-dependent results. Use explicit one-byte boolean and pointer-sized length mappings throughout the interface and callers.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/CallbackSpikeMain.java:91
  • A local variable is not guaranteed to remain GC-reachable until lexical scope exit; after the connection_open call the JIT may consider callback dead while Rust still owns its function pointer. That makes the stated GC protection unreliable and can crash the JVM during asynchronous callbacks. Keep it in a field or call Reference.reachabilityFence(callback) only after connection_close.
        // CRITICAL: hold this as a strong reference to prevent GC!
        // If this gets GC'd, the native function pointer becomes dangling → JVM crash.
        CallbackTestLibrary.OutboundCallback callback = (Pointer userData, Pointer data, int len) -> {

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/CallbackSpikeMain.java:137

  • Holding the callback only in a local does not guarantee reachability until method exit; the JIT may collect it after its last Java use while the Rust thread still holds the function pointer. Keep it in a field or place Reference.reachabilityFence(callback) after connection_close so this test cannot intermittently crash.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/README.md:37
  • The JDK 25 overlay only replaces ReaderThreadFactory; NativeBindingProvider explicitly uses JNA on all JDKs and says FFM is deferred. These run instructions and the result table therefore describe code that does not exist, including an unnecessary native-access flag. Rewrite this as JNA + virtual-thread behavior (and align the POM metadata).
## Run on JDK 25 (FFM path)

```sh
java --enable-native-access=ALL-UNNAMED ^
     -Djava.library.path=../rust-dll/target/debug ^
     -jar target/jna-callback-mrjar-spike-0.1.0.jar 5

Logs will show [JDK-25/FFM] — upcall stub executes directly on the native
thread, no new Java thread per callback.

**1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/java-program-that-invokes-rust-dll-mr-jar-17-25/README.md:37**
* The JDK 25 overlay only replaces `ReaderThreadFactory`; `NativeBindingProvider` explicitly uses JNA on all JDKs and says FFM is deferred. These run instructions and the result table therefore describe code that does not exist, including an unnecessary native-access flag. Rewrite this as JNA + virtual-thread behavior (and align the POM metadata).
**1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/README.md:25**
* This directory does not exist; the sibling is named `java-program-that-invokes-rust-dll-jdk17`. Following the documented build sequence therefore fails before Maven runs.

cd ../java-program-that-invokes-rust-dll

**1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/dependency-reduced-pom.xml:1**
* This is Maven Shade's generated build output, not source. The accompanying research transcript explicitly says `dependency-reduced-pom.xml` should remain untracked and be excluded from the commit. Remove it to avoid maintaining stale duplicate build configuration.
**1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/dependency-reduced-pom.xml:1**
* This is Maven Shade's generated build output, not source. The research transcript explicitly says generated `dependency-reduced-pom.xml` files were to be excluded. Remove it rather than committing a stale duplicate of the module POM.
**1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/src/main/java/com/github/copilot/spike/mrjar/CallbackMrJarMain.java:134**
* A callback timeout is logged but the program continues, performs native calls, and exits successfully while reporting only `TIMEOUT` in the summary. That allows this verification spike to pass even when its central callback bridge failed. Treat the timeout as a test failure before continuing.
    if (completed) {
        LOG.info("All callbacks completed. activeCallbacks=" + activeCallbacks.get());
    } else {
        LOG.warning("Timed out! activeCallbacks=" + activeCallbacks.get());
    }
**1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/java-program-that-invokes-rust-dll-mr-jar-17-25/src/main/java/com/github/copilot/spike/mrjar/CallbackMrJarMain.java:134**
* A callback timeout is logged but the program continues and exits successfully, so this verification spike can pass when its central callback bridge failed. Throw on timeout before making further native calls.
**1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/rust-dll/README.md:29**
* This fixture now exports six functions: the five simplified runtime ABI functions plus the test-only `callback_once` control used by the GraalVM test. The current count/table omit that control and incorrectly characterize the complete export set. Document the extra test-only function explicitly.
**1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/CallbackSpikeMain.java:175**
* A callback timeout only emits a warning; the program continues through native writes/cleanup and exits successfully, so the spike can report success when callback delivery failed. Treat this as a failed verification rather than continuing.
    boolean completed = allCallbacksDone.await(10, TimeUnit.SECONDS);
    if (completed) {
        LOG.info("All callbacks completed. activeCallbacks=" + activeCallbacks.get());
    } else {
        LOG.warning("Timed out waiting for callbacks! activeCallbacks=" + activeCallbacks.get());
    }
**1917-java-embed-rust-cli-runtime-remove-before-merge/djl_temp2.txt:1**
* This is an opaque base64-encoded temporary research artifact, and no source/build code consumes it. Committing it makes the epic branch harder to audit and gives future contributors no provenance or purpose. Remove it (or add the decoded, named source only if it is genuinely required).

cGx1Z2lucyB7CiAgICBhaS5kamwuamF2YVByb2plY3QKICAgIGFpLmRqbC5w

**1917-java-embed-rust-cli-runtime-remove-before-merge/djl_temp3.txt:1**
* This is an opaque base64-encoded temporary research artifact, and no source/build code consumes it. Remove it rather than committing unauditable temporary data.

cGx1Z2lucyB7CiAgICBhaS5kamwuamF2YVByb2plY3QKICAgIGFpLmRqbC5w

**1917-java-embed-rust-cli-runtime-remove-before-merge/djl_temp4.txt:1**
* This is an opaque base64-encoded temporary research artifact, and no source/build code consumes it. Remove it rather than committing unauditable temporary data.

cm9vdFByb2plY3QubmFtZSA9ICJkamwiCgpwbHVnaW5zIHsKICAgIGlkKCJv

</details>


- **Files reviewed:** 63/68 changed files
- **Comments generated:** 0 new
- **Review effort level:** Medium

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Comments suppressed due to low confidence (17)

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-6-platform-detection-win32-x64/src/main/java/com/github/copilot/spike/platform/PlatformDetector.java:108

  • UNKNOWN currently falls through to the glibc classifier. If /proc is unavailable or ELF parsing fails on a musl host, this deterministically selects the incompatible linux-* library. Fail closed when libc cannot be identified instead of silently treating unknown as glibc.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/CallbackSpikeMain.java:137
  • A local variable is not a guaranteed strong root until method exit; HotSpot may collect it after its last Java use, which is immediately after connection_open, while Rust still owns the callback pointer asynchronously. This can leave a dangling native callback and crash the JVM. Retain it in an owner field through connection_close, or place Reference.reachabilityFence(callback) after close.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/CallbackSpikeMain.java:91
  • A local variable is not a guaranteed strong root until method exit; HotSpot may collect it after its last Java use, which is immediately after connection_open, while Rust still owns the callback pointer asynchronously. This can leave a dangling native callback and crash the JVM. Retain it in an owner field through connection_close, or place Reference.reachabilityFence(callback) after close.
        // CRITICAL: hold this as a strong reference to prevent GC!
        // If this gets GC'd, the native function pointer becomes dangling → JVM crash.
        CallbackTestLibrary.OutboundCallback callback = (Pointer userData, Pointer data, int len) -> {

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/java-program-that-invokes-rust-dll-mr-jar-17-25/README.md:28

  • The JDK 25 path does not use FFM. The only Java 25 overlay is ReaderThreadFactory, while NativeBindingProvider explicitly uses JNA on every JDK. This command also adds an unnecessary native-access flag and the claimed log messages/table are impossible, so update the title, run section, and table to describe “JNA + virtual thread.”
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/README.md:28
  • The JDK 25 path does not use FFM. The only Java 25 overlay is ReaderThreadFactory, while NativeBindingProvider explicitly uses JNA on every JDK. This command also adds an unnecessary native-access flag and the claimed log messages/table are impossible, so update the title, run section, and table to describe “JNA + virtual thread.”
## Run on JDK 25 (FFM path)

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-6-platform-detection-linux-x64/src/main/java/com/github/copilot/spike/platform/PlatformDetector.java:102

  • UNKNOWN currently falls through to the glibc classifier. If /proc is unavailable or ELF parsing fails on a musl host, this deterministically selects the incompatible linux-* library. Fail closed when libc cannot be identified instead of silently treating unknown as glibc.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-6-platform-detection-darwin-arm64/src/main/java/com/github/copilot/spike/platform/PlatformDetector.java:99
  • UNKNOWN currently falls through to the glibc classifier. If /proc is unavailable or ELF parsing fails on a musl host, this deterministically selects the incompatible linux-* library. Fail closed when libc cannot be identified instead of silently treating unknown as glibc.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/CallbackSpikeMain.java:174
  • A callback timeout is only logged, after which the spike continues cleanup and exits successfully while native callbacks may still be running. This both reports a failed experiment as success and races callback execution with shutdown. Abort on timeout before proceeding.
            LOG.warning("Timed out waiting for callbacks! activeCallbacks=" + activeCallbacks.get());

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/src/main/java/com/github/copilot/spike/mrjar/CallbackMrJarMain.java:133

  • A callback timeout is only logged, after which the spike continues cleanup and exits successfully while native callbacks may still be running. This both reports a failed experiment as success and races callback execution with shutdown. Abort on timeout before proceeding.
            LOG.warning("Timed out! activeCallbacks=" + activeCallbacks.get());

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/java-program-that-invokes-rust-dll-mr-jar-17-25/src/main/java/com/github/copilot/spike/mrjar/CallbackMrJarMain.java:133

  • A callback timeout is only logged, after which the spike continues cleanup and exits successfully while native callbacks may still be running. This both reports a failed experiment as success and races callback execution with shutdown. Abort on timeout before proceeding.
    1917-java-embed-rust-cli-runtime-remove-before-merge/supply-chain-security-to-stephentoub.md:15
  • This inventory is outdated: Python now honors COPILOT_NPM_REGISTRY_URL and refuses to load the runtime unless npm Subresource Integrity verifies (python/copilot/_cli_version.py:145-162, _cli_download.py:418-432). It is also fetched lazily at runtime rather than at build time, so the note currently gives an incorrect supply-chain assessment.
    1917-java-embed-rust-cli-runtime-remove-before-merge/20260722-prompts-3_1-and-3_2-gaps.md:41
  • This resolution reverses the correct finding immediately above it. The Java SDK is currently 1.0.9-SNAPSHOT while @github/copilot is 1.0.73 (java/pom.xml:8-10, nodejs/package-lock.json:702-721), so ${project.version} cannot select the runtime package. Keep an explicit runtime-version pin and mapping instead.
The npm package version and the SDK version are the **same version**. `@github/copilot-linux-x64@1.0.9-preview.0` and `com.github:copilot-sdk-java:1.0.9-preview.0` — same string.

So there's no version mapping at all. The `copilot-native` module downloads `@github/copilot-<platform>@${project.version}` from npm. One version, everywhere. No extra property needed.

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/rust-dll/README.md:21

  • The fixture now exports six functions because callback_once was added at src/lib.rs:32, but this inventory still says five and omits it. That makes the fixture documentation incomplete.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/java-program-that-invokes-rust-dll-mr-jar-17-25/src/main/java/com/github/copilot/spike/mrjar/NativeBindingProvider.java:30
  • These return types do not match the Rust ABI: JNA maps Java boolean as a 4-byte C int, while Rust bool is the 1-byte C _Bool (the production .NET binding explicitly uses UnmanagedType.U1). This can make the fixture read undefined upper return bytes. Declare the native returns as byte and convert != 0 in the Java wrapper.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/CallbackTestLibrary.java:28
  • This return type does not match the Rust ABI: JNA maps Java boolean as a 4-byte C int, while Rust bool is the 1-byte C _Bool (the production .NET binding explicitly uses UnmanagedType.U1). The same mismatch affects the other boolean exports below. Declare native returns as byte and convert != 0.
    1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/src/main/java/com/github/copilot/spike/mrjar/NativeBindingProvider.java:30
  • These return types do not match the Rust ABI: JNA maps Java boolean as a 4-byte C int, while Rust bool is the 1-byte C _Bool (the production .NET binding explicitly uses UnmanagedType.U1). This can make the fixture read undefined upper return bytes. Declare the native returns as byte and convert != 0 in the Java wrapper.
        boolean host_shutdown(int serverHandle);
        int connection_open(int serverHandle, OutboundCallback callback, Pointer userData, int burstCount);
        boolean connection_write(int connectionHandle, byte[] data, int len);
        boolean connection_close(int connectionHandle);

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/CallbackTestLibrary.java:28

  • This return type does not match the Rust ABI: JNA maps Java boolean as a 4-byte C int, while Rust bool is the 1-byte C _Bool (the production .NET binding explicitly uses UnmanagedType.U1). The same mismatch affects the other boolean exports below. Declare native returns as byte and convert != 0.
    boolean host_shutdown(int serverHandle);
  • Files reviewed: 63/68 changed files
  • Comments generated: 0 new
  • Review effort level: Medium

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants