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

[edburns]

Base branch for work in #1917 .

[Copilot]

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

[Copilot]

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

[Copilot]

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

[Copilot]

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]

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

[Copilot]

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

[Copilot]

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

[Copilot]

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

[Copilot]

Review details

Comments suppressed due to low confidence (16)

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

• This states the opposite of the current Python implementation. Python honors COPILOT_NPM_REGISTRY_URL (python/copilot/_cli_version.py:145-162) and fails closed unless the downloaded runtime passes npm Subresource Integrity verification (python/copilot/_cli_download.py:418-432). As written, this security note would give its recipient materially incorrect information.
  1917-java-embed-rust-cli-runtime-remove-before-merge/20260722-prompts-3_1-and-3_2-gaps.md:41
• This resolution contradicts both the evidence immediately above it and the repository: Java is 1.0.9-SNAPSHOT, while @github/copilot and its platform packages are 1.0.73. Keeping this conclusion would make the native packaging design download a nonexistent runtime version; retain the independent-version mapping described in lines 29-35.

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-11-replay-proxy-and-in-process/java-inprocess-e2e-win32-x64/src/main/java/com/github/copilot/spike/inprocess/InProcessEnvGuard.java:216

• Both Unix failure paths continue after setenv/unsetenv fails. In particular, a failed proxy-URL override can silently allow authenticated traffic to reach the real endpoint. Treat either nonzero result as fatal and roll back all environment changes already made by this guard.

            if (rc != 0) {
                LOG.warning("[InProcessEnvGuard] setenv() failed for key=" + name + " rc=" + rc);

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 the ELF probe cannot identify its interpreter, this deterministically selects the wrong native binary on musl instead of reporting that detection failed. Only map an explicit GLIBC result to linux-*; reject 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
• This treats every Linux result other than explicit MUSL—including UNKNOWN after an I/O/parsing failure—as glibc. That can select and load an incompatible binary on musl. Require an explicit GLIBC result and fail when libc cannot be determined.
  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 path maps LinuxLibc.UNKNOWN to linux-*, so an unreadable or unrecognized interpreter silently becomes glibc. This violates the deterministic selection requirement and can load an incompatible binary on musl; fail unless detection explicitly returns 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-jdk17/src/main/java/com/github/copilot/spike/CallbackTestLibrary.java:28
• These Rust exports return a one-byte bool, but JNA's default Java boolean mapping is a 32-bit C integer. The declarations can misread false results; bind the ABI return as byte and convert it in Java wrappers. The same mismatch affects all three boolean-returning methods in this interface.

    boolean host_shutdown(int serverHandle);

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 fixture exports Rust bool (one byte), while JNA maps Java boolean as a 32-bit native integer by default. This makes all three boolean-returning declarations ABI-incompatible and can hide native failures; use one-byte return values and 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:28
• These methods bind one-byte Rust bool returns as JNA boolean, whose default native representation is a 32-bit integer. False returns may be misread, undermining the spike's result checks. Bind as byte and convert in wrapper code.
  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
• JNA's Java boolean uses a 32-bit native integer, but the Rust fixture exports one-byte bool values. All three declarations therefore have the wrong ABI and can report failures as successes; expose byte returns and convert them in the provider methods.
  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
• There is no JDK 25 FFM path in this project: NativeBindingProvider is only present under the baseline source tree and explicitly uses JNA on every JDK; the sole Java 25 overlay is ReaderThreadFactory. The run instructions, expected logs, and result table therefore describe behavior this JAR cannot produce. Either add the missing FFM overlay or rewrite all FFM claims to describe JNA plus a virtual reader thread.

## Run on JDK 25 (FFM path)

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

• This project has no FFM binding or Java 25 NativeBindingProvider overlay; JDK 25 only swaps in a virtual-thread ReaderThreadFactory, while callbacks still use JNA. Consequently these instructions and the result table promise an FFM path and log output that cannot occur. Implement the missing FFM path or update every FFM reference to the actual JNA behavior.
  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 count and table still list only five. Include the synchronous control export so the fixture documentation matches the Graal experiment.
  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, so this verification program proceeds through cleanup and exits successfully even though its core callback/queue experiment failed. Make the timeout fail the run, as the Graal spike already does.

            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 returns success, allowing this spike to claim completion when native callbacks never arrived. Throw on timeout so the result can be used as a reliable verification.
  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
• This timeout only emits a warning and the process exits 0, so the MR-JAR experiment can be reported as successful without receiving its callbacks. Fail the run on timeout.

• Files reviewed: 70/76 changed files
• Comments generated: 3
• Review effort level: Medium

[Copilot]

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

• This resolution contradicts the repository's actual versioning: the Java SDK is 1.0.9-SNAPSHOT (java/pom.xml:8-10) while @github/copilot is 1.0.73 (nodejs/package-lock.json:702-703). Deriving the npm package version from ${project.version} would request a different or nonexistent runtime package; retain an explicit runtime-version pin separate from the Maven artifact 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

• The Python findings are stale: both its packument and tarball URLs honor COPILOT_NPM_REGISTRY_URL (python/copilot/_cli_version.py:145-162), and runtime provisioning fails closed when npm SRI is unavailable or mismatched (python/copilot/_cli_download.py:421-432). As written, this note would send an incorrect supply-chain assessment.
  1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-11-replay-proxy-and-in-process/java-inprocess-e2e-win32-x64/src/main/java/com/github/copilot/spike/inprocess/CopilotRuntimeLibrary.java:28
• JNA's default Java boolean mapping is a 32-bit C int, not the one-byte C/Rust bool claimed here. Binding Rust extern "C" fn -> bool this way is an ABI mismatch and can misread return values on supported platforms. Use byte-sized return declarations with explicit != 0 conversion (or a custom type mapper) for all three boolean exports.

 * <p><b>bool return:</b> JNA maps Java {@code boolean} to C {@code _Bool}/
 * {@code bool} (1 byte).  The Rust {@code #[repr(C)]} bool ABI is stable.

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, ELF parsing fails, or the interpreter is unrecognized, this selects a potentially incompatible glibc library rather than detecting the platform. 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:100
• UNKNOWN is treated as glibc here. Any failure to read or parse /proc/self/exe therefore selects linux-* even though libc was not determined, which can load an incompatible native binary. Return a classifier only for explicit GLIBC or MUSL; otherwise fail closed.
  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
• This copy also defaults an UNKNOWN libc result to glibc. That converts a detection failure into selection of a potentially incompatible library. Require an explicit GLIBC result before returning linux-*, and throw for UNKNOWN.
  1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-11-replay-proxy-and-in-process/java-inprocess-e2e-win32-x64/src/main/java/com/github/copilot/spike/inprocess/InProcessEnvGuard.java:132
• nativeSetEnv only logs OS failures, so the constructor still reports the guard as active and the runtime can start without replay-proxy redirection or credential suppression; a failed COPILOT_API_URL update may send E2E traffic to the real service. Make mutations fail closed and roll back any entries already changed if construction fails.

            nativeSetEnv(name, value);
            LOG.info("[Env] " + name + "=" + (name.contains("TOKEN") ? "<redacted>" : value)
                    + " (saved previous: " + (previous == null ? "null" : "<present>") + ")");

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 Java boolean declarations use JNA's 32-bit C int mapping, but the Rust fixture exports one-byte Rust/C booleans. That is an ABI mismatch for every boolean method in this interface and can make the spike report incorrect results. Bind byte-sized returns and convert explicitly.
  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
• The JNA interface maps Java boolean as a 32-bit C int, while the Rust DLL exports Rust/C bool. This ABI mismatch affects all boolean exports in the interface; use byte-sized return values and explicit conversion.
  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 Java boolean declarations are ABI-incompatible with the Rust fixture's one-byte bool returns because JNA maps Java boolean to a 32-bit C int. Change every boolean export to a byte-sized binding and convert in the wrappers.
  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
• JNA maps Java boolean to a 32-bit C int, not the Rust fixture's one-byte bool. All boolean methods in this interface therefore use the wrong ABI and can return unreliable values. Use byte-sized declarations and explicit conversion.

    boolean host_shutdown(int serverHandle);

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:56

• The table claims a JDK 25 FFM overlay, but the only overlay is ReaderThreadFactory; NativeBindingProvider remains the baseline JNA implementation on every JDK. Record JNA plus virtual-reader behavior here.
  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:56
• The JDK 25 row documents an FFM NativeBindingProvider overlay that does not exist. Both JDKs use JNA; the MR-JAR changes only the reader thread from platform to virtual.

| JVM    | Class loaded                       | Callback mechanism | Thread behavior                |
| ------ | ---------------------------------- | ------------------ | ------------------------------ |
| JDK 17 | `NativeBindingProvider` (baseline) | JNA `Callback`     | New Java thread per invocation |
| JDK 25 | `NativeBindingProvider` (overlay)  | FFM upcall stub    | Executes on native thread      |

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

• The fixture now exports six functions because callback_once was added, but the README still says five and omits that control path from the table.
  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
• The JDK 25 implementation still uses NativeBindingProvider's JNA callback; only ReaderThreadFactory is overlaid to create a virtual reader thread. This run section describes an FFM implementation that is not present and even adds an unnecessary FFM native-access flag.
  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
• This copy also has no FFM binding: JDK 25 overlays only the reader-thread factory while callbacks remain JNA-based. The heading, command flag, and expected logs describe code that is not in the JAR.

## 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-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, after which the spike proceeds to close the connection and exits successfully. This turns the callback behavior being tested into a false-positive pass; fail the run on timeout.
  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
• This timeout path still allows the spike to continue cleanup and exit 0, so a failed callback experiment is reported only as a warning. Throw on timeout to make the result reliable.
  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 core JNA callback spike exits successfully after this timeout and prints a completion summary, producing a false positive when native callbacks fail. Treat timeout as a failed experiment.

        } else {
            LOG.warning("Timed out waiting for callbacks! activeCallbacks=" + activeCallbacks.get());
        }

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

• This resolution repeats the invalid version coupling: ${project.version} is the Java SDK version, not the independently pinned npm runtime version (java/pom.xml:8-10 is 1.0.9-SNAPSHOT, while nodejs/package-lock.json:702-703 is 1.0.73). The npm pack command needs a dedicated runtime-version value.

**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`.

• Files reviewed: 70/76 changed files
• Comments generated: 0 new
• Review effort level: Medium

[Copilot]

Review details

Comments suppressed due to low confidence (31)

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-11-replay-proxy-and-in-process/java-inprocess-e2e-win32-x64/src/main/java/com/github/copilot/spike/inprocess/CopilotRuntimeLibrary.java:28

• JNA's default boolean mapping is a 32-bit C int, but the runtime ABI returns Rust bool/C _Bool (one byte). This can misread false/error returns. Bind all three boolean-returning entry points as byte (or use a 1-byte TypeMapper) and convert with result != 0 in Java.

 * <p><b>bool return:</b> JNA maps Java {@code boolean} to C {@code _Bool}/
 * {@code bool} (1 byte).  The Rust {@code #[repr(C)]} bool ABI is stable.

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

• This interface maps Rust bool returns to JNA boolean, which JNA marshals as a 32-bit C int; Rust's C ABI uses a one-byte _Bool. The mismatch makes false/error returns unreliable. Use a one-byte return mapping for every boolean-returning function and convert it in the wrappers.
  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
• This interface maps Rust bool returns to JNA boolean, which JNA marshals as a 32-bit C int; Rust's C ABI uses a one-byte _Bool. The mismatch makes false/error returns unreliable. Use a one-byte return mapping for every boolean-returning function and convert it in the wrappers.
  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
• The Rust fixture returns a one-byte C-ABI bool, whereas JNA marshals Java boolean as a 32-bit C int. This return signature is therefore ABI-incompatible and can turn a false result into an arbitrary value. Apply a one-byte mapping to all boolean returns in this interface.
  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
• The Rust fixture returns a one-byte C-ABI bool, whereas JNA marshals Java boolean as a 32-bit C int. This return signature is therefore ABI-incompatible and can turn a false result into an arbitrary value. Apply a one-byte mapping to all boolean returns in this interface.
  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 treated as glibc here. On a musl host where /proc/self/exe cannot be read or parsed, that deterministically selects the wrong native library. Only produce linux-* after detecting GLIBC; otherwise report that libc detection failed.
  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 system, Java will select and load the incompatible glibc library. Fail closed unless libc was positively identified as GLIBC or MUSL.
  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 system, Java will select and load the incompatible glibc library. Fail closed unless libc was positively identified as GLIBC or MUSL.
  1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-11-replay-proxy-and-in-process/java-inprocess-e2e-win32-x64/src/main/java/com/github/copilot/spike/inprocess/InProcessEnvGuard.java:208
• A failed SetEnvironmentVariableW only emits a warning, so execution can continue with the original environment. In particular, failure to set the replay-proxy URL can send requests and credentials to the real service. Fail closed and roll back any variables already changed by this constructor.

        if (!ok) {
            LOG.warning("[InProcessEnvGuard] SetEnvironmentVariableW failed for key=" + name);
        }

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-11-replay-proxy-and-in-process/java-inprocess-e2e-win32-x64/src/main/java/com/github/copilot/spike/inprocess/InProcessEnvGuard.java:217

• Ignoring a setenv failure lets the runtime continue without the requested proxy/auth isolation and may direct a replay test to the live service. Treat this as construction failure and roll back earlier environment changes.

            if (rc != 0) {
                LOG.warning("[InProcessEnvGuard] setenv() failed for key=" + name + " rc=" + rc);
            }

1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-11-replay-proxy-and-in-process/java-inprocess-e2e-win32-x64/src/main/java/com/github/copilot/spike/inprocess/InProcessEnvGuard.java:222

• Ignoring an unsetenv failure can leave HMAC credentials active even though this guard promises to suppress them for replay-proxy traffic. Fail construction/restoration and roll back consistently when deletion fails.

            if (rc != 0) {
                LOG.warning("[InProcessEnvGuard] unsetenv() failed for key=" + name + " rc=" + rc);
            }

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

• These mitigations are outdated: Python now honors COPILOT_NPM_REGISTRY_URL (python/copilot/_cli_version.py:145-162) and fails closed unless npm Subresource Integrity verifies (python/copilot/_cli_download.py:421-432). As written, this note gives the recipient 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 contradicts the repository's actual versioning: java/pom.xml is version 1.0.9-SNAPSHOT, while the runtime packages in nodejs/package-lock.json are 1.0.73. Using ${project.version} as the npm runtime version therefore requests a package that need not exist. Keep a separate pinned runtime version and update the earlier resolution that also relies on ${project.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:4

• This README describes an FFM implementation on JDK 25, but the only JDK 25 overlay is ReaderThreadFactory; NativeBindingProvider explicitly uses JNA on every JDK and says FFM is deferred. The title, run instructions, expected logs, and behavior table should describe JNA with platform versus virtual reader threads.
  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 on JDK 25, but the only JDK 25 overlay is ReaderThreadFactory; NativeBindingProvider explicitly uses JNA on every JDK and says FFM is deferred. The title, run instructions, expected logs, and behavior table should describe JNA with platform versus 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:16
• The artifact metadata claims that JDK 25 supplies an FFM NativeBindingProvider, but this project only overlays ReaderThreadFactory and uses the baseline JNA binding on all JDKs. Correct the name and description so published/generated metadata does not report an implementation that is absent.
  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 that JDK 25 supplies an FFM NativeBindingProvider, but this project only overlays ReaderThreadFactory and uses the baseline JNA binding on all JDKs. Correct the name and description so published/generated metadata does not report an implementation that is absent.
  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, but the README still says five and omits it from the table. This makes the documented ABI disagree with both lib.rs and the Java binding.
  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 spike exits successfully and even prints a completion summary. That allows the experiment to be treated as passing when native callbacks never arrived. Throw or exit nonzero on timeout.
  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, after which the spike exits successfully and even prints a completion summary. That allows the experiment to be treated as passing when native callbacks never arrived. Throw or exit nonzero on timeout.
  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
• The build command changes into a sibling directory that does not exist; this 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

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 produces a warning, so this verification program still exits zero and reports the spike complete despite missing callbacks. Make timeout a failing condition as the Graal spike already does.

        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/QueueInputStream.java:69

• The EOF/zero-length returns happen before argument validation, so this does not honor the InputStream contract for null buffers or invalid ranges. Validate first, then handle zero-length and 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:69
• The EOF/zero-length returns happen before argument validation, so this does not honor the InputStream contract for null buffers or invalid ranges. Validate first, then handle zero-length and 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/QueueInputStream.java:82
• The EOF/zero-length returns happen before argument validation, so this does not honor the InputStream contract for null buffers or invalid ranges. Validate first, then handle zero-length and 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:82
• The EOF/zero-length returns happen before argument validation, so this does not honor the InputStream contract for null buffers or invalid ranges. Validate first, then handle zero-length and 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/NativeBindingProvider.java:29
• The Rust fixture declares this length as usize, which is 64-bit on every target in #1917, but this JNA signature marshals a 32-bit int. The callback length has the same mismatch. Use long consistently for both size_t parameters so the spike exercises the actual ABI.
  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
• The Rust fixture declares this length as usize, which is 64-bit on every target in #1917, but this JNA signature marshals a 32-bit int. The callback length has the same mismatch. Use long consistently for both size_t parameters so the spike exercises the actual ABI.
  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
• Rust's connection_write takes usize len, but Java int is only 32 bits on the x64/arm64 targets under test; OutboundCallback.invoke has the same mismatch. Map both size_t values as Java long and update their callers.
  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
• Rust's connection_write takes usize len, but Java int is only 32 bits on the x64/arm64 targets under test; OutboundCallback.invoke has the same mismatch. Map both size_t values as Java long and update their callers.
  1917-java-embed-rust-cli-runtime-remove-before-merge/supply-chain-security-to-stephentoub.md:11
• “At build time” is not true for every SDK: Python's in-process runtime is downloaded on demand by _cli_download.py, whereas Rust/Go/.NET perform build or bundling downloads. Describe these as direct registry downloads without assigning a single lifecycle phase.

• Files reviewed: 75/81 changed files
• Comments generated: 2
• Review effort level: Medium

[Copilot]

Review details

Comments suppressed due to low confidence (14)

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

• These mitigation claims are stale: Python now honors COPILOT_NPM_REGISTRY_URL (python/copilot/_cli_version.py:145-162) and refuses to load a runtime unless npm Subresource Integrity is available and verifies (python/copilot/_cli_download.py:421-432). As written, this note incorrectly tells its recipient that only .NET is configurable and that Python has no integrity verification.
  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: the Java SDK is 1.0.9-SNAPSHOT (java/pom.xml:8-10) while the runtime packages are 1.0.73 (nodejs/package-lock.json:702-721). Using ${project.version} for npm downloads will therefore request runtime versions that do not exist; retain 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-8-graal-research/rust-dll/README.md:21

• This fixture exports six functions, not five: callback_once is also exported by rust-dll/src/lib.rs:30-40 and is used as the synchronous control in the GraalVM test. Omitting it 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/README.md:28
• The implementation does not have an FFM path: NativeBindingProvider explicitly uses JNA on every JDK and says FFM is deferred, while the JDK 25 overlay only replaces ReaderThreadFactory. The heading, expected logs, and behavior table therefore describe code that this spike cannot execute; document JNA + virtual threads for JDK 25 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/README.md:28
• The implementation does not have an FFM path: NativeBindingProvider explicitly uses JNA on every JDK and says FFM is deferred, while the JDK 25 overlay only replaces ReaderThreadFactory. The heading, expected logs, and behavior table therefore describe code that this spike cannot execute; document JNA + virtual threads for JDK 25 instead.
  1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-11-replay-proxy-and-in-process/java-inprocess-e2e-win32-x64/src/main/java/com/github/copilot/spike/inprocess/CopilotRuntimeLibrary.java:28
• JNA maps Java boolean to a 4-byte C int, not to the runtime's 1-byte Rust/C bool; the .NET binding explicitly uses UnmanagedType.U1 for the same ABI (dotnet/src/FfiRuntimeHost.cs:364-387). Consequently the three boolean-returning declarations below can read undefined upper return-register bytes and misreport native failures as success. Bind these returns as byte (or add an explicit 1-byte type mapper) and convert to boolean in Java.

 * <p><b>bool return:</b> JNA maps Java {@code boolean} to C {@code _Bool}/
 * {@code bool} (1 byte).  The Rust {@code #[repr(C)]} bool ABI is stable.

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 override violates InputStream.read(byte[], off, len): a zero-length read must return 0, including after EOF, and invalid buffer ranges must be rejected before the EOF result. Validate the range first, then handle len == 0 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
• This override violates InputStream.read(byte[], off, len): a zero-length read must return 0, including after EOF, and invalid buffer ranges must be rejected before the EOF result. Validate the range first, then handle len == 0 before 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 override violates InputStream.read(byte[], off, len): a zero-length read must return 0, including after EOF, and invalid buffer ranges must be rejected before the EOF result. Validate the range first, then handle len == 0 before 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/QueueInputStream.java:81
• This override violates InputStream.read(byte[], off, len): a zero-length read must return 0, including after EOF, and invalid buffer ranges must be rejected before the EOF result. Validate the range first, then handle len == 0 before eof.
  1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-11-replay-proxy-and-in-process/java-inprocess-e2e-win32-x64/src/main/java/com/github/copilot/spike/inprocess/QueueInputStream.java:103
• This override violates InputStream.read(byte[], off, len): a zero-length read must return 0, including after EOF, and invalid buffer ranges must be rejected before the EOF result. Validate the range first, then handle len == 0 before eof.

    public int read(byte[] b, int off, int len) throws IOException {
        if (eof) return -1;
        if (len == 0) return 0;

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, the 2 KiB probe is insufficient, or the interpreter path is unrecognized on a musl JVM, this deterministically selects the incompatible glibc library rather than reporting that detection failed. Only map explicit GLIBC/MUSL results and reject 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 is unavailable, the 2 KiB probe is insufficient, or the interpreter path is unrecognized on a musl JVM, this deterministically selects the incompatible glibc library rather than reporting that detection failed. Only map explicit GLIBC/MUSL results and reject 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
• UNKNOWN currently falls through to the glibc classifier. If /proc is unavailable, the 2 KiB probe is insufficient, or the interpreter path is unrecognized on a musl JVM, this selects the incompatible glibc library rather than reporting that detection failed. Only map explicit GLIBC/MUSL results and reject UNKNOWN.

• Files reviewed: 75/81 changed files
• Comments generated: 2
• Review effort level: Medium

[Copilot]

Review details

Comments suppressed due to low confidence (34)

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

• This resolution reverses the correct observation above it. The Java SDK is currently 1.0.9-SNAPSHOT (java/pom.xml:8-10), while the runtime packages are 1.0.73 (nodejs/package-lock.json:702-721), so using ${project.version} would request a nonexistent or unrelated npm runtime release. Keep a separate runtime-version pin and package that runtime under the SDK artifact 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's registry is no longer hardcoded: both its packument and tarball URL builders honor COPILOT_NPM_REGISTRY_URL (python/copilot/_cli_version.py:145-162). This entry should describe the default plus that override rather than reporting the old behavior.
  1917-java-embed-rust-cli-runtime-remove-before-merge/supply-chain-security-to-stephentoub.md:23
• These mitigation conclusions are outdated. Python now supports a registry override and refuses to load the runtime unless npm Subresource Integrity is available and verifies (python/copilot/_cli_download.py:325-360, 421-432); leaving this claim in the security note materially misstates the current exposure.
  1917-java-embed-rust-cli-runtime-remove-before-merge/supply-chain-security-to-stephentoub.md:7
• Use the possessive contraction here.
  1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-11-replay-proxy-and-in-process/java-inprocess-e2e-win32-x64/src/main/java/com/github/copilot/spike/inprocess/CopilotRuntimeLibrary.java:28
• The bool mapping documented here is incorrect: JNA's default Java boolean mapping is a 32-bit C int, while these Rust/C ABI functions return a 1-byte bool (the .NET binding explicitly uses UnmanagedType.U1 at dotnet/src/FfiRuntimeHost.cs:364-387). The three JNA return declarations therefore do not match the ABI; bind them as byte (or use a 1-byte type mapper) and test nonzero at call sites.

 * <p><b>bool return:</b> JNA maps Java {@code boolean} to C {@code _Bool}/
 * {@code bool} (1 byte).  The Rust {@code #[repr(C)]} bool ABI is stable.

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 wrong native binary instead of reporting that detection failed. Only select linux-* for an explicit GLIBC; fail fast for UNKNOWN.
  1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-11-replay-proxy-and-in-process/java-inprocess-e2e-win32-x64/src/main/java/com/github/copilot/spike/inprocess/InProcessEnvGuard.java:208
• Native environment mutation failures are only logged, so the test continues after failing to set the token or replay-proxy URL. That can turn an intended replay run into an unauthenticated failure or a request to the live API. Fail the guard construction and restore any variables already changed when SetEnvironmentVariableW fails.

        if (!ok) {
            LOG.warning("[InProcessEnvGuard] SetEnvironmentVariableW failed for key=" + name);
        }

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

• This section describes an FFM implementation that is not present. The JDK 25 overlay only replaces ReaderThreadFactory with a virtual-thread implementation; NativeBindingProvider remains the baseline JNA class on both JDKs. Update the JDK 25 run instructions and comparison table to say “JNA + virtual thread” and remove --enable-native-access.
  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 says JDK 25 swaps in FFM and a different NativeBindingProvider, but the only versioned class is ReaderThreadFactory; native binding remains JNA. This metadata will misdescribe the produced artifact.
  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 copied README still describes a JDK 25 FFM path, but this spike explicitly defers FFM and only overlays ReaderThreadFactory. Update the JDK 25 section and table to document JNA with a virtual reader thread, and remove the unnecessary native-access flag.
  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 the JDK 25 variant uses FFM and replaces NativeBindingProvider, but no such overlay exists; both variants use JNA and differ only in reader-thread type.
  1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/rust-dll/README.md:21
• This fixture now exports six functions because callback_once was added, but the README still says five and omits it from the table. Document the synchronous callback control so the fixture contract matches src/lib.rs.
  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 Shade output, not source. The research log itself says dependency-reduced-pom.xml should remain untracked, and committing it creates a stale second project definition that will diverge from pom.xml. Delete it and configure/extend .gitignore so packaging cannot re-add it.

<?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 generated Shade POM should not be committed. It duplicates the authoritative pom.xml, is regenerated by builds, and the accompanying research notes explicitly intended to exclude it. Delete the file and ignore dependency-reduced-pom.xml.
  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 generated Maven Shade output and should remain untracked; otherwise it is a stale duplicate of the real project configuration. Delete it and add dependency-reduced-pom.xml to this spike's ignore rules.
  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 JNA interface maps Rust's 1-byte bool returns as Java boolean, which JNA marshals as a 32-bit C int. That is an ABI mismatch for host_shutdown, connection_write, and connection_close; use byte return values and treat nonzero as success.
  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
• The fixture returns Rust/C ABI 1-byte bools, but JNA's default Java boolean mapping is a 32-bit native integer. Correct all three bool-returning declarations to byte and compare their result with zero at the 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 Java boolean returns do not match the Rust fixture's 1-byte bool ABI because JNA maps Java boolean to a 32-bit C integer. Bind each bool result as byte, then convert with result != 0 in the provider methods.
  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
• JNA marshals Java boolean as a 32-bit C integer, not the fixture's 1-byte Rust/C bool. The three bool-returning methods therefore have mismatched signatures; bind them as byte and convert nonzero results in the wrappers.
  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 the failure condition this spike is meant to detect, but this branch only logs a warning and then exits successfully with a “complete” summary. Throw here (after cleanup via finally) so automation cannot record a failed callback experiment as passing.

        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:134

• The experiment still exits zero after callback timeout, so it can report completion even when the behavior under test failed. Make timeout fatal while preserving native cleanup in a finally block.
  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
• This copied spike treats callback timeout as a warning and continues to a successful process exit. Since callback delivery is the assertion under test, timeout must produce a nonzero failure after running cleanup.
  1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-11-replay-proxy-and-in-process/java-inprocess-e2e-win32-x64/src/main/java/com/github/copilot/spike/inprocess/QueueInputStream.java:103
• InputStream.read must validate b/off/len, and a zero-length request returns 0 even at EOF. This implementation does neither because it checks EOF first and returns before touching invalid zero-length arguments.

    public int read(byte[] b, int off, int len) throws IOException {
        if (eof) return -1;
        if (len == 0) return 0;

1917-java-embed-rust-cli-runtime-remove-before-merge/dd-3038503-prepare-devbox/02-create-storage-upload-blob.ps1:100

• This leaves the storage account key in the invoking PowerShell process after uploads complete or fail, allowing unrelated later commands to inherit it. Scope the key usage with try/finally and remove Env:AZURE_STORAGE_KEY in the finally block.

$env:AZURE_STORAGE_ACCOUNT = $StorageAccount
$env:AZURE_STORAGE_KEY = (Invoke-Az -SensitiveOutput -Arguments @(

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. A read/parse failure on a musl system would therefore load linux-*, exactly the wrong cdylib. Return the glibc classifier only for explicit GLIBC, and fail fast 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:99
• UNKNOWN currently falls through to the glibc classifier. On a musl system where /proc/self/exe cannot be read, this silently chooses an incompatible runtime. Require explicit GLIBC before returning linux-*, and fail fast for 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:82
• This violates InputStream.read(byte[], off, len): a zero-length read must return 0 even after EOF, and invalid/null buffer arguments must be rejected before returning. Validate the range, then handle len == 0, then test 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:82
• The bulk-read contract requires argument validation and requires len == 0 to return 0, including at EOF. The current order returns -1 at EOF and accepts invalid zero-length ranges.
  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
• This bulk-read implementation checks EOF before the mandatory zero-length case and never validates b/off/len. That breaks the InputStream contract for zero-length reads at EOF and invalid ranges.
  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:69
• Honor the InputStream bulk-read contract by validating the buffer/range and returning 0 for len == 0 before checking EOF. As written, EOF returns -1 for an empty read and invalid empty ranges are silently accepted.
  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:17
• The class-level description says this spike uses Piped streams, but the implementation uses QueueInputStream throughout and specifically rejects Piped streams. Rename this description to avoid documenting the opposite of what is tested.

 * Spike 3.4 — JNA callback threading with PipedInputStream/PipedOutputStream bridging.

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:16

• The POM says the program pipes through PipedInputStream/PipedOutputStream, but the code uses QueueInputStream because Piped streams are the rejected alternative. Correct the artifact metadata.

    <description>
        Minimal program to test JNA callback invocation from a native (Rust) thread,
        piping data through PipedInputStream/PipedOutputStream.
    </description>

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

• This opaque base64 payload is an unexplained temporary artifact and has no relationship to the Java runtime embedding work. Keeping it in the base branch adds unreviewable content with no build or documentation use; remove it or replace it with a named, readable research fixture and provenance.

cGx1Z2lucyB7CiAgICBhaS5kamwuamF2YVByb2plY3QKICAgIGFpLmRqbC5w

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

• This second opaque base64 djl_temp artifact is not referenced by the spike and cannot be meaningfully reviewed in this form. Remove it from the PR, or commit the decoded source under a descriptive name with an explanation if it is genuinely required.

cm9vdFByb2plY3QubmFtZSA9ICJkamwiCgpwbHVnaW5zIHsKICAgIGlkKCJv

• Files reviewed: 75/81 changed files
• Comments generated: 3
• Review effort level: Medium