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 (21)

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

• This Python entry is outdated: the default is npmjs.org, but _cli_version.py:149-162 honors COPILOT_NPM_REGISTRY_URL for both packument and tarball URLs. Calling it hardcoded incorrectly understates the existing enterprise-registry mitigation.
  1917-java-embed-rust-cli-runtime-remove-before-merge/supply-chain-security-to-stephentoub.md:23
• Python does have both mitigations stated as absent here: registry override support and fail-closed npm Subresource Integrity verification (python/copilot/_cli_download.py:421-432). Please separate Go's current behavior from Python's so this security assessment is accurate.
  1917-java-embed-rust-cli-runtime-remove-before-merge/20260722-prompts-3_1-and-3_2-gaps.md:41
• This resolution contradicts both the preceding finding and the repository: the Java SDK is 1.0.9-SNAPSHOT, while the npm runtime in nodejs/package-lock.json:702-721 is 1.0.73. Deriving the npm package version from ${project.version} will request a package version that generally does not exist; retain an independently pinned runtime version.

The npm package version and the SDK version are the **same version**. `@github/copilot-linux-x64@1.0.9-preview.0` and `com.github:copilot-sdk-java:1.0.9-preview.0` — same string.

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

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 maps to a 32-bit C int, not the runtime ABI's one-byte Rust/C bool. Consequently all three boolean return declarations below use the wrong ABI and may read undefined upper return-register bits. Bind these returns as byte (or add an explicit one-byte type mapper) and convert 0/1 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/rust-dll/README.md:21

• This fixture now also exports callback_once, so it has six exported C functions rather than five. The missing row makes the Graal synchronous-control path undocumented.
  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 treated only as a warning, after which the spike closes the connection and exits successfully. That makes the experiment report success even when its core callback/threading assertion failed; fail the process 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 treated only as a warning, after which the spike closes the connection and exits successfully. That makes the experiment report success even when its core callback/threading assertion failed; fail the process 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/src/main/java/com/github/copilot/spike/CallbackSpikeMain.java:91
• A local variable does not guarantee callback reachability until native work finishes—the JVM may collect it after its last Java use (connection_open), leaving Rust with a dangling trampoline and causing a JVM crash. Keep the callback in a field or call Reference.reachabilityFence(callback) after connection close/shutdown.

        // CRITICAL: hold this as a strong reference to prevent GC!
        // If this gets GC'd, the native function pointer becomes dangling → JVM crash.
        CallbackTestLibrary.OutboundCallback callback = (Pointer userData, Pointer data, int len) -> {

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

• The JDK 25 overlay only replaces ReaderThreadFactory; NativeBindingProvider remains the baseline JNA implementation on every JDK. This FFM command and expected behavior therefore cannot occur. Update this section and the matching POM metadata to describe JNA plus a virtual reader thread.
  1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/java-program-that-invokes-rust-dll-mr-jar-17-25/README.md:28
• The JDK 25 overlay only replaces ReaderThreadFactory; NativeBindingProvider remains the baseline JNA implementation on every JDK. This FFM command and expected behavior therefore cannot occur. Update this section and the matching POM metadata to describe JNA plus a virtual reader thread.
  1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/CallbackSpikeMain.java:137
• A local variable does not guarantee callback reachability until native work finishes—the JVM may collect it after its last Java use (connection_open), leaving Rust with a dangling trampoline and causing a JVM crash. Keep the callback in a field or call Reference.reachabilityFence(callback) after connection close/shutdown.
  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, PT_INTERP parsing fails, or the loader name is unfamiliar, this deterministically loads the wrong native binary instead of reporting that libc could not be identified. Return linux-* only for GLIBC and throw 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/dependency-reduced-pom.xml:1
• This is generated output from Maven Shade, not source. The accompanying research log explicitly says dependency-reduced-pom.xml is not part of the spike design and should remain untracked; committing it creates a second, stale POM that will be regenerated on every build. Delete it and ignore this generated filename.

<?xml version="1.0" encoding="UTF-8"?>

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

• This is generated output from Maven Shade, not source. The accompanying research log explicitly says dependency-reduced-pom.xml is not part of the spike design and should remain untracked; committing it creates a second, stale POM that will be regenerated on every build. Delete it and ignore this generated filename.
  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 output from Maven Shade, not source. The accompanying research log explicitly says copied spikes should exclude dependency-reduced-pom.xml; committing it creates a stale second POM that is regenerated on every build. Delete it and ignore this generated filename.
  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
• From the sibling rust-dll directory, this path does not exist—the added Java directory is named java-program-that-invokes-rust-dll-jdk17. As written, the documented build steps fail before Maven runs.

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

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

• If SetEnvironmentVariableW fails, the guard continues and later claims the replay environment is active. In particular, failure to set COPILOT_API_URL can make this E2E spike contact the real endpoint instead of the replay proxy. Treat mutation failure as fatal rather than running with the wrong process environment.

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

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

• This is raw base64 output from a one-off gh api research command and has no consumer in the repository. Keeping opaque temporary command output makes the base branch harder to audit; remove it and retain the relevant conclusion in the research document instead.

cGx1Z2lucyB7CiAgICBhaS5kamwuamF2YVByb2plY3QKICAgIGFpLmRqbC5w

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

• This is raw base64 output from a one-off gh api research command and has no consumer in the repository. Keeping opaque temporary command output makes the base branch harder to audit; remove it and retain the relevant conclusion in the research document instead.

cm9vdFByb2plY3QubmFtZSA9ICJkamwiCgpwbHVnaW5zIHsKICAgIGlkKCJv

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 treated only as a warning, after which this verification program performs cleanup and exits successfully. That allows a failed callback/threading experiment to be reported as a successful run; fail immediately as the Graal variant already does.

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

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

• The Unix path also continues after setenv/unsetenv fails, so the guard can falsely report that replay isolation is active and then run against an unintended endpoint. Throw on a nonzero return code instead of merely logging it.

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

• Files reviewed: 75/81 changed files
• Comments generated: 0 new
• 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:11

• This inventory is stale and would give the recipient an incorrect risk assessment. Python now honors COPILOT_NPM_REGISTRY_URL (python/copilot/_cli_version.py:145-162) and fails closed unless the downloaded runtime passes npm SRI verification (python/copilot/_cli_download.py:418-432); its download is also on demand rather than a build-time step. Please update both the Python entry and mitigation summary.
  1917-java-embed-rust-cli-runtime-remove-before-merge/20260722-prompts-3_1-and-3_2-gaps.md:41
• This resolution reverses the correct finding immediately above it. The versions are currently independent (java/pom.xml:10 is 1.0.9-SNAPSHOT, while nodejs/package-lock.json:702-704 pins runtime 1.0.73), so using ${project.version} as the npm version will request packages that do not exist. Keep a separate runtime-version pin and record the mapping.

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/CopilotRuntimeLibrary.java:28

• The C ABI mapping here is incorrect: JNA maps Java boolean to a 4-byte native integer by default, while Rust bool/C _Bool is 1 byte. Consequently the three boolean-returning functions can misread return values (especially false, whose upper return-register bits are not part of the Rust value). Map these returns as byte and interpret nonzero as true, or install an explicit 1-byte type mapper.

 * <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-11-replay-proxy-and-in-process/java-inprocess-e2e-win32-x64/pom.xml:14

• The module claims Java 17 compatibility, but InProcessSpikeMain.java:179 calls Thread.threadId(), which was added after Java 17. A JDK 17 build fails, and a class compiled on a newer JDK with target 17 fails with NoSuchMethodError on JDK 17. Use the Java 17-compatible getId() in the source (or raise the declared baseline).

        <maven.compiler.source>17</maven.compiler.source>
        <maven.compiler.target>17</maven.compiler.target>

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

• An UNKNOWN libc result is silently classified as glibc here. If ELF inspection fails on a musl host, this selects the incompatible linux-* library instead of reporting that selection was impossible. Handle GLIBC explicitly and fail closed for UNKNOWN.
  1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/README.md:28
• This documents an FFM path that does not exist. The JDK 25 overlay contains only ReaderThreadFactory; NativeBindingProvider remains the baseline JNA implementation on every JDK and its Javadoc says FFM is deferred. Update the title, JDK 25 run section, expected logs, and comparison table to describe JNA plus a virtual reader thread.
  1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/java-program-that-invokes-rust-dll-mr-jar-17-25/README.md:28
• This documents an FFM path that does not exist. The JDK 25 overlay contains only ReaderThreadFactory; NativeBindingProvider remains the baseline JNA implementation on every JDK and its Javadoc says FFM is deferred. Update the title, JDK 25 run section, expected logs, and comparison table to describe JNA plus a virtual reader thread.
  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 in src/lib.rs:32, but this inventory still says five and omits it. That makes the documented ABI incomplete.
  1917-java-embed-rust-cli-runtime-remove-before-merge/dd-3038503-prepare-devbox/03-download-from-blob.ps1:40
• The storage-account key is assigned to the process-wide environment and never removed, so it remains exposed to subsequent child processes after this script completes or throws. Wrap the key-dependent downloads in try/finally and clear AZURE_STORAGE_KEY in finally (or use identity-based --auth-mode login).

$env:AZURE_STORAGE_KEY = az storage account keys list `
    --resource-group $ResourceGroup `
    --account-name $StorageAccount `
    --query "[0].value" `
    --output tsv

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

• detectLinuxLibc() explicitly returns UNKNOWN when /proc/self/exe cannot be read or parsed, but this branch silently treats every non-musl result as glibc. That can select and load an incompatible glibc library on musl. Fail closed when libc is unknown so native selection remains deterministic.
  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
• An UNKNOWN libc result is silently classified as glibc here. If ELF inspection fails on a musl host, this selects the incompatible linux-* library instead of reporting that selection was impossible. Handle GLIBC explicitly and fail closed for UNKNOWN.
  1917-java-embed-rust-cli-runtime-remove-before-merge/dd-3038503-prepare-devbox/02-create-storage-upload-blob.ps1:104
• The storage-account key is placed in the process-wide environment and never removed. It therefore remains available to every later child process in this PowerShell session, including when an upload command throws. Limit the key's lifetime with try/finally and clear AZURE_STORAGE_KEY in the finally block (or use identity-based --auth-mode login).

$env:AZURE_STORAGE_KEY = (Invoke-Az -SensitiveOutput -Arguments @(
    "storage", "account", "keys", "list",
    "--resource-group", $ResourceGroup,
    "--account-name", $StorageAccount,
    "--query", "[0].value",

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 timeout is treated as a successful spike run: execution continues through cleanup, prints “Spike complete,” and exits 0. Since this program is evidence for callback viability, missing callbacks must fail the process rather than merely emit a warning.
  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 timeout is treated as a successful spike run: execution continues through cleanup, prints “Spike complete,” and exits 0. Since this program is evidence for callback viability, missing callbacks must fail the process rather than merely emit a warning.
  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 logs a warning, then the program prints a success summary and exits 0. That can record a failed threading experiment as a passing result. Throw on timeout so the spike's exit status reflects the failure.
  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:227
• Darwin.toLowerCase() contains "win", so macOS is routed to kernel32 instead of the Unix setenv implementation. This contradicts the class's documented macOS support and fails as soon as the guard is constructed on macOS.

        return System.getProperty("os.name", "").toLowerCase().contains("win");

• Files reviewed: 76/82 changed files
• Comments generated: 0 new
• 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:16

• This is no longer hardcoded in the effective Python download path: both the packument and tarball URL builders honor COPILOT_NPM_REGISTRY_URL (python/copilot/_cli_version.py:145-162). As written, this note would give the recipient stale information.
  1917-java-embed-rust-cli-runtime-remove-before-merge/supply-chain-security-to-stephentoub.md:23
• The Python conclusions are stale. Python supports a registry override and fails closed unless the downloaded native runtime passes npm Subresource Integrity verification (python/copilot/_cli_download.py:421-432), so .NET is not the only configurable SDK and Python should not be grouped with Go 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
• JNA's default boolean mapping is a 4-byte C int, not a 1-byte C _Bool/Rust bool. Consequently all three boolean return declarations below use the wrong ABI and can misread the runtime's result. Use a one-byte type mapper (the currently allocated options map is unused) or declare byte returns and convert them in Java; callers must be updated with the chosen mapping.

 * <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/dd-3038503-prepare-devbox/02-create-storage-upload-blob.ps1:35

• For -SensitiveOutput, this failure path writes the captured output verbatim. The only sensitive invocation retrieves the storage-account key, so partial/error output can expose that credential in logs—the opposite of the switch's purpose. Keep sensitive output redacted even when the command fails.

        if ($SensitiveOutput -and $Output) {
            $Output | ForEach-Object { Write-Diagnostic "az error: $_" }
        }

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 environment update is only logged, after which the spike continues and may load the runtime without the intended token or replay-proxy redirect. In the proxy case that can send traffic to the real endpoint. Fail closed so the runtime is never started with a partially applied environment.

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

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

• UNKNOWN currently falls through to the glibc classifier. If /proc/self/exe cannot be read or its interpreter is unrecognized on a musl host, this deterministically selects an incompatible glibc binary. Treat only explicit GLIBC as glibc and reject UNKNOWN.
  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 ELF inspection fails on a musl host, the detector therefore chooses an incompatible binary instead of reporting that deterministic selection was impossible. Return glibc only for explicit GLIBC 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 ELF inspection fails on a musl host, the detector therefore chooses an incompatible binary instead of reporting that deterministic selection was impossible. Return glibc only for explicit GLIBC and reject UNKNOWN.
  1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/java-program-that-invokes-rust-dll-mr-jar-17-25/README.md:1
• This README describes an FFM JDK-25 implementation, but the only multi-release overlay is ReaderThreadFactory; NativeBindingProvider uses JNA on every JDK, as its own Javadoc states. The JDK-25 command, expected log text, and behavior table are therefore misleading and should describe JNA plus a virtual reader thread instead.
  1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/README.md:1
• This README describes an FFM JDK-25 implementation, but the only multi-release overlay is ReaderThreadFactory; NativeBindingProvider uses JNA on every JDK, as its own Javadoc states. The JDK-25 command, expected log text, and behavior table are therefore misleading and should describe JNA plus a virtual reader thread instead.
  1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/rust-dll/README.md:21
• This fixture now exports six functions because callback_once was added, but the count and table still describe five. Include the synchronous control callback so the documented Graal experiment matches the library being built.
  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 only produces a warning, after which the validation continues, performs cleanup, prints a completion summary, and exits successfully. This lets the spike report success even when its central callback/threading check failed. Make the timeout fail the run.
  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 only produces a warning, after which the validation continues, performs cleanup, prints a completion summary, and exits successfully. This lets the spike report success even when its central callback/threading check failed. Make the timeout fail the run.
  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, then the spike continues and exits successfully. Since callback delivery is the behavior this program validates, a timeout must fail the run rather than merely changing the summary text.
  1917-java-embed-rust-cli-runtime-remove-before-merge/20260722-prompts-3_1-and-3_2-gaps.md:15
• This resolution would request the SDK version from npm, but the SDK and runtime are independently versioned: java/pom.xml:10 is 1.0.9-SNAPSHOT, while nodejs/package-lock.json:703-717 pins the runtime packages at 1.0.73. The native module needs a dedicated pinned runtime-version property; ${project.version} must remain the Maven artifact version only.

**Resolution:** Skip `package.json` entirely for the native module. Use `exec-maven-plugin` to run `npm pack @github/copilot-<platform>@<version>` for each platform, then extract with `tar`. Version comes from `${project.version}` (SDK and npm versions are identical). Integrity is verified by a build step that reads the `integrity` field from the monorepo's `nodejs/package-lock.json` for each platform package and checks the SHA-512 of the downloaded `.tgz`, mirroring Rust's `resolve_version_and_integrity` → `cached_download` → verify pattern in `build/in_process.rs`.

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

• This conclusion is contradicted by the repository: the Java SDK is 1.0.9-SNAPSHOT, while @github/copilot and its platform packages are 1.0.73. Treating them as one version would make the Java build request an unrelated or nonexistent npm artifact. Record and update 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.

• Files reviewed: 77/83 changed files
• Comments generated: 0 new
• Review effort level: Medium

[Copilot]

Review details

Comments suppressed due to low confidence (30)

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

• This assessment is outdated: Python now honors COPILOT_NPM_REGISTRY_URL for both packument and tarball URLs and refuses to load a runtime unless npm SRI verification succeeds (python/copilot/_cli_version.py:145-162, python/copilot/_cli_download.py:418-432). As written, the note incorrectly singles out .NET and understates the existing Python mitigation.
  1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/rust-dll/README.md:21
• The fixture exports six functions because callback_once is also exported at src/lib.rs:32, but this section claims five and omits it from the table. Add callback_once and describe its synchronous calling-thread behavior so the documented ABI matches the fixture used by the Graal test.
  1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/README.md:28
• This README describes an FFM JDK 25 path, but the only JDK 25 overlay is ReaderThreadFactory; NativeBindingProvider remains the JNA implementation on every JDK, as its Javadoc and CallbackMrJarMain state. The command, expected logs, title, and result table therefore document behavior this artifact cannot produce.
  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 README describes an FFM JDK 25 path, but the only JDK 25 overlay is ReaderThreadFactory; NativeBindingProvider remains the JNA implementation on every JDK. The command, expected logs, title, and result table therefore document behavior this artifact cannot produce.
  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 POM says the JDK 25 overlay uses FFM, but the overlay contains only ReaderThreadFactory; both versions bind through the baseline JNA NativeBindingProvider. This metadata will misidentify what the packaged spike validates.
  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 POM says the JDK 25 overlay uses FFM, but the overlay contains only ReaderThreadFactory; both versions bind through the baseline JNA NativeBindingProvider. This metadata will misidentify what the packaged spike validates.
  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 parser cannot recognize the interpreter on a musl host, this deterministically selects the wrong native binary instead of distinguishing libc as required by #1917. Fail with a descriptive unsupported-platform error when libc is unknown.
  1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-6-platform-detection-darwin-arm64/src/main/java/com/github/copilot/spike/platform/PlatformDetector.java:99
• UNKNOWN currently falls through to the glibc classifier. If /proc/self/exe is unavailable or the ELF parser cannot recognize the interpreter on a musl host, this selects the wrong native binary instead of distinguishing libc as required by #1917. Fail when libc cannot be identified.
  1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-6-platform-detection-win32-x64/src/main/java/com/github/copilot/spike/platform/PlatformDetector.java:108
• Any libc result other than MUSL, including UNKNOWN, is silently classified as glibc. A read/parser failure on a musl host will therefore select and load the incompatible binary, contrary to #1917's deterministic libc-selection requirement. Handle GLIBC explicitly and reject UNKNOWN.
  1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-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 continues, shuts down the host while callbacks may still be outstanding, and exits successfully. This can report a failed callback/reader experiment as a passing run; make the timeout fail the process as the Graal spike already does.
  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 continues, shuts down the host while callbacks may still be outstanding, and exits successfully. This can report a failed callback/reader experiment as a passing run; make the timeout fail the process as the Graal spike already does.
  1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/CallbackSpikeMain.java:175
• A callback timeout is only logged, so this validation still performs cleanup and exits successfully even though the native callback path failed. Throw on timeout so automation cannot treat the spike as passing.
  1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-11-replay-proxy-and-in-process/java-inprocess-e2e-win32-x64/pom.xml:14
• Using source/target does not restrict compilation to the Java 17 API. The existing InProcessSpikeMain calls Thread.threadId() at line 179 (introduced after Java 17), so a JDK 25 build can produce a Java-17-classfile JAR that fails on JDK 17, while a JDK 17 build fails to compile. Use --release 17 and replace that call with a Java 17 API.

        <maven.compiler.source>17</maven.compiler.source>
        <maven.compiler.target>17</maven.compiler.target>

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 call is treated as success. In particular, failure to set the replay-proxy URL leaves the runtime free to contact the real API with the supplied token while the test continues. Fail closed and roll back any overrides already applied if construction cannot complete.

        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

• A failed setenv() call is only logged, allowing the runtime to start with a partially applied environment and potentially bypass the replay proxy. Fail closed and roll back earlier mutations when applying overrides fails.

            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

• A failed unsetenv() call is only logged. If HMAC keys cannot be removed, the replay run proceeds under the wrong authentication mode rather than stopping; restoration failures can likewise leave process-global state corrupted. Surface failures while still attempting all required rollback/restoration operations.

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

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

• This latch is decremented when a frame is enqueued, before NativeBindingProvider decrements activeCallbacks in the callback's finally. The main thread can therefore observe completion and close the connection/host while the final native callback is still running, so this does not validate safe callback draining. Signal completion from the callback's finally or explicitly wait for activeCallbacks == 0 before cleanup.
  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:79
• This latch is decremented when a frame is enqueued, before NativeBindingProvider decrements activeCallbacks in the callback's finally. The main thread can therefore observe completion and close the connection/host while the final native callback is still running. Signal completion from the callback's finally or explicitly wait for activeCallbacks == 0 before cleanup.
  1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/dependency-reduced-pom.xml:1
• This is Maven Shade's generated reduced POM, so every package build can rewrite a tracked source file and leave the worktree dirty. Remove the generated artifact from the PR and add dependency-reduced-pom.xml to this spike's .gitignore (or configure dependencyReducedPomLocation under target/).

<?xml version="1.0" encoding="UTF-8"?>

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

• This is Maven Shade's generated reduced POM, so every package build can rewrite a tracked source file and leave the worktree dirty. Remove the generated artifact from the PR and add dependency-reduced-pom.xml to this spike's .gitignore (or configure dependencyReducedPomLocation under target/).
  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 Maven Shade's generated reduced POM, so every package build can rewrite a tracked source file and leave the worktree dirty. Remove the generated artifact from the PR and add dependency-reduced-pom.xml to this spike's .gitignore (or configure dependencyReducedPomLocation under target/).
  1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/README.md:26
• The documented directory does not exist; this spike's Java directory is named java-program-that-invokes-rust-dll-jdk17. Following the build steps from rust-dll therefore fails at this cd.

cd ../java-program-that-invokes-rust-dll
mvn package -q

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

• The project description says data is bridged through PipedInputStream/PipedOutputStream, but the implementation uses QueueInputStream and explicitly rejects piped streams. Correct the artifact metadata so it reflects what the spike actually validates.
  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 title says this spike bridges through piped streams, but the implemented bridge is QueueInputStream; piped streams are only documented below as the rejected alternative. Rename the title to avoid reporting the opposite design.
  1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/src/main/java/com/github/copilot/spike/mrjar/QueueInputStream.java:65
• This override does not validate b, off, and len before checking EOF or blocking on the queue. Invalid ranges can block indefinitely waiting for data, and at EOF they return -1 instead of throwing the exceptions required by InputStream. Validate the range first.
  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:65
• This override does not validate b, off, and len before checking EOF or blocking on the queue. Invalid ranges can block indefinitely waiting for data, and at EOF they return -1 instead of throwing the exceptions required by InputStream. Validate the range first.
  1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/QueueInputStream.java:78
• This override does not validate b, off, and len before checking EOF or blocking on the queue. Invalid ranges can block indefinitely waiting for data, and at EOF they return -1 instead of throwing the exceptions required by InputStream. Validate the range first.
  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:78
• This override does not validate b, off, and len before checking EOF or blocking on the queue. Invalid ranges can block indefinitely waiting for data, and at EOF they return -1 instead of throwing the exceptions required by InputStream. Validate the range first.
  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:102
• This override does not validate b, off, and len before checking EOF or blocking on the queue. Invalid ranges can block indefinitely waiting for callback data, and at EOF they return -1 instead of throwing the exceptions required by InputStream. Validate the range first.

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

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

• This logs every environment value unless its name happens to contain uppercase TOKEN. The guard accepts arbitrary keys, so credentials named *_SECRET, *_PASSWORD, lowercase token names, or similar will be written to logs. Log only the key and whether a prior value existed.

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

• Files reviewed: 77/83 changed files
• Comments generated: 0 new
• Review effort level: Medium

[Copilot]

Review details

Comments suppressed due to low confidence (21)

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

• This resolution contradicts both the preceding text and the current repository: the Java SDK is 1.0.9-SNAPSHOT, while the pinned @github/copilot runtime is 1.0.73. Deriving the npm package from ${project.version} would therefore request a nonexistent or unrelated package. Keep a separate runtime-version pin and update its integrity metadata independently.

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. Python now honors COPILOT_NPM_REGISTRY_URL for both packument and tarball URLs and refuses to load the runtime unless npm Subresource Integrity verifies (python/copilot/_cli_version.py:145-162, _cli_download.py:325-360,418-432). As written, this note gives the recipient an incorrect supply-chain assessment.
  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. If /proc/self/exe is unavailable, has no PT_INTERP, or uses an unrecognized loader, this silently selects linux-* and can load an ABI-incompatible native library. Since classifier selection is intended to be deterministic, fail explicitly unless libc is known.
  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
• This ABI statement is incorrect: JNA's default Java boolean mapping is a 32-bit C int, while these Rust exports return a one-byte bool (the .NET binding explicitly uses UnmanagedType.U1). Consequently the three boolean methods below have mismatched return ABIs and may read incorrect values. Bind the native returns as byte and convert to Java booleans in wrappers.

 * <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/NativeBindingProvider.java:30

• The fixture declarations do not match the Rust C ABI: JNA maps Java boolean as a 32-bit C int, but Rust returns a one-byte bool; the usize length used by connection_write and the callback is also declared as 32-bit int on these 64-bit targets. These mismatches make the spike's conclusions unreliable. Bind bool returns as byte and size values as long, then convert in Java wrappers.
  1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-8-graal-research/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/CallbackTestLibrary.java:28
• This and the other boolean declarations use JNA's 32-bit boolean mapping for Rust's one-byte bool; the usize callback/write lengths are also represented as 32-bit int on a 64-bit target. Use byte native returns plus Java wrappers, and long for size_t/usize, so the Graal/JNA experiment tests 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-mr-jar-17-25/src/main/java/com/github/copilot/spike/mrjar/NativeBindingProvider.java:30
• The fixture declarations do not match the Rust C ABI: JNA maps Java boolean as a 32-bit C int, but Rust returns a one-byte bool; the usize length used by connection_write and the callback is also declared as 32-bit int on these 64-bit targets. Bind bool returns as byte and size values as long, then convert in Java 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
• This and the other boolean declarations use JNA's 32-bit boolean mapping for Rust's one-byte bool; the usize callback/write lengths are also represented as 32-bit int on a 64-bit target. Use byte native returns plus Java wrappers, and long for size_t/usize, so the callback experiment tests 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/README.md:1
• This README describes an FFM implementation that is not present. Only ReaderThreadFactory has a JDK 25 overlay; NativeBindingProvider remains the baseline JNA implementation on both JDKs, as the source and the canonical plan explicitly state. Update the title, JDK 25 run instructions, expected logs, and comparison table to describe JNA plus a virtual reader thread.
  1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/README.md:1
• This README describes an FFM implementation that is not present. Only ReaderThreadFactory has a JDK 25 overlay; NativeBindingProvider remains the baseline JNA implementation on both JDKs, as the source and the canonical plan explicitly state. Update the title, JDK 25 run instructions, expected logs, and comparison table to describe JNA plus a virtual reader thread.
  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 falls through to the glibc classifier. A failed or unrecognized ELF probe would therefore select a potentially ABI-incompatible library instead of reporting that detection failed. Handle GLIBC explicitly 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 falls through to the glibc classifier. A failed or unrecognized ELF probe would therefore select a potentially ABI-incompatible library instead of reporting that detection failed. Handle GLIBC explicitly and reject 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
• Failure to set a Windows override is only logged, yet construction continues and later declares the guard active. If the replay-proxy URL fails to apply, this can send the E2E request to the real Copilot API. Fail closed instead of continuing with a partially applied environment.

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

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

• The POM claims the JDK 25 overlay replaces JNA with FFM, but only ReaderThreadFactory has an overlay; native binding remains JNA on both versions. Describe the actual platform-thread versus virtual-thread swap.
  1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/pom.xml:17
• The POM claims the JDK 25 overlay replaces JNA with FFM, but only ReaderThreadFactory has an overlay; native binding remains JNA on both versions. Describe the actual platform-thread versus virtual-thread swap.
  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 only emits a warning, after which the program continues cleanup and reports the spike as complete. A broken callback path can therefore exit successfully and be mistaken for a passing experiment. Make the timeout fail the run.
  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 only emits a warning, after which the program continues cleanup and reports the spike as complete. A broken callback path can therefore exit successfully and be mistaken for a passing experiment. Make the timeout fail the run.
  1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-jdk17/src/main/java/com/github/copilot/spike/CallbackSpikeMain.java:175
• A callback timeout only emits a warning, so this validation program proceeds and reports completion even though the behavior under test failed. Throw on timeout so automation and readers cannot treat a failed spike as successful.
  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
• The Unix path also continues after setenv or unsetenv fails. That leaves the guard partially applied while claiming success; in replay mode, a failed URL override can leak traffic to the real service. Propagate either failure.

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

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

• This fixture now also exports callback_once, so the stated count and table omit one exported function. Document it as the synchronous same-thread control used by 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/README.md:25
• This directory is java-program-that-invokes-rust-dll-jdk17; the documented sibling path does not exist, so following the build steps from rust-dll fails before Maven runs.

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

• Files reviewed: 77/83 changed files
• Comments generated: 0 new
• Review effort level: Medium

[Copilot]

Review details

Comments suppressed due to low confidence (16)

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

• This resolution couples two independently versioned artifacts. The current Java SDK is 1.0.9-SNAPSHOT (java/pom.xml:10) while the runtime packages are 1.0.73 (nodejs/package-lock.json:702-721), so using ${project.version} will request an npm version that does not exist. Keep a separate pinned runtime version (and integrity metadata) from 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:23

• The mitigation summary is stale: Python now honors COPILOT_NPM_REGISTRY_URL and refuses to load the runtime unless npm Subresource Integrity verifies (python/copilot/_cli_version.py:145-162, python/copilot/_cli_download.py:421-432). As written, this note materially overstates the Python supply-chain gap.
  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 environment mutation is only logged, after which the guard claims to be active and the spike may contact the real Copilot API instead of the replay proxy. Since these overrides control endpoint routing and credentials, fail immediately when SetEnvironmentVariableW fails.

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

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

• SensitiveOutput suppresses the storage key on success but prints the captured output on failure. Because this helper is used for storage account keys list, partial or diagnostic output could expose the account key in logs. Never emit captured output for sensitive invocations.

    if ($ExitCode -ne 0) {
        if ($SensitiveOutput -and $Output) {
            $Output | ForEach-Object { Write-Diagnostic "az error: $_" }
        }
        throw "Azure CLI command failed: az $($Arguments -join ' ')"

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

• This documents an FFM path that the project does not contain. The JDK 25 overlay only replaces ReaderThreadFactory with a virtual-thread implementation; NativeBindingProvider remains JNA on every JDK and the code explicitly says FFM is deferred. Update the title, run instructions, log expectations, and result table to describe JNA + platform thread versus JNA + virtual thread.
  1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/java-program-that-invokes-rust-dll-mr-jar-17-25/README.md:37
• This documents an FFM path that is not implemented. The JDK 25 overlay only swaps in a virtual ReaderThreadFactory; native callbacks still use the baseline JNA NativeBindingProvider. Update the title, run section, and comparison table to describe the actual platform-thread versus virtual-thread experiment.
  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 in src/lib.rs:30-40, but this list still claims five and omits that control path.
  1917-java-embed-rust-cli-runtime-remove-before-merge/spike-3-4-jna-callback-and-threading/rust-dll/README.md:33
• This says the fixture verifies a PipedStream strategy, but the Java spike uses QueueInputStream and explicitly rejects PipedStream because JNA's short-lived callback threads trigger “Write end dead.” Describe the queue-backed bridge instead.
  1917-java-embed-rust-cli-runtime-remove-before-merge/djl_temp3.txt:1
• These are raw base64 files created as temporary output by the gh api commands recorded in 20260722-prompts.md:999-1015; no build or documentation consumes them. Checking opaque scratch artifacts into the branch adds unrelated, unauditable content. Remove this file (and djl_temp4.txt) or replace it with a cited, human-readable research note.

cGx1Z2lucyB7CiAgICBhaS5kamwuamF2YVByb2plY3QKICAgIGFpLmRqbC5w

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

• This is another raw base64 scratch file produced by the temporary DJL research command and is not consumed anywhere. Remove it with djl_temp3.txt or preserve the relevant findings as readable documentation instead.

cm9vdFByb2plY3QubmFtZSA9ICJkamwiCgpwbHVnaW5zIHsKICAgIGlkKCJv

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 into the glibc branch, so any /proc/self/exe read/parsing failure silently selects linux-*. On a musl host this loads an ABI-incompatible library. Preserve deterministic platform selection by failing when libc cannot be identified rather than treating unknown as glibc.
  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, so a missing /proc, an unrecognized interpreter, or an ELF parsing failure can select an ABI-incompatible linux-* runtime on musl. Fail closed unless detection explicitly returns 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, so a Linux detection failure can select an ABI-incompatible linux-* runtime on musl. Fail closed unless detection explicitly returns 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:217
• The Unix path has the same fail-open behavior: after setenv fails, callers continue as though endpoint and credential overrides were applied. Throw on a nonzero return so an E2E run cannot silently use the inherited/live environment.

            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

• If unsetenv fails, HMAC credentials may remain active even though the guard reports that they were suppressed. Treat this as a hard failure rather than continuing with unintended authentication behavior.

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

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

• This still says the fixture verifies a PipedStream strategy, but the corresponding Java code uses QueueInputStream; the Graal research also relies on that queue-backed bridge. Update the stated purpose so the fixture documentation matches what is exercised.

• Files reviewed: 77/83 changed files
• Comments generated: 0 new
• Review effort level: Medium