[WIP] Achieve 100% InProcess and out-of-process test parity (#2272)

Author: Ed Burns <edburns@microsoft.com>
Date:   Fri Aug 7 17:29:06 2026 +0000

    Fix Java in-process test lifecycle and parity
    
    Prevent the Java in-process test profile from corrupting Surefire control
    streams or poisoning later tests through the runtime's process-global LLM
    provider registration. Preserve explicit subprocess and TCP transport choices,
    sanitize process-only options before constructing in-process clients, and run
    request-handler tests over their required isolated stdio runtime.
    
    Fix the remaining test-contract issues by making fake socket RPC handler
    registration atomic with reader startup, honoring the configured CLI
    entrypoint when runtime.node is in a prebuilds directory, and isolating the
    streaming model-cache scenario. Remove in-process skip annotations from tests
    that already exercise an explicit subprocess transport.
    
    The complete `mvn clean verify -Pinprocess` run now finishes successfully
    without hangs, transport timeouts, provider-ownership failures, or Surefire
    stream corruption.
    
    File-by-file manifest:
    
    - `java/sdk/pom.xml`: use Surefire's TCP fork channel for unit and integration
      tests so native runtime output cannot corrupt Maven's process-pipe protocol.
    - `java/sdk/src/main/java/com/github/copilot/CopilotClient.java`: preserve
      explicitly selected TCP options when the default connection environment
      requests in-process transport.
    - `java/sdk/src/main/java/com/github/copilot/JsonRpcClient.java`: add a socket
      construction hook that registers handlers before the reader thread starts.
    - `java/sdk/src/main/java/com/github/copilot/ffi/NativeRuntimeLoader.java`:
      resolve the configured Copilot executable separately from runtime.node when
      the native library uses the package's prebuilds layout.
    - `java/sdk/src/main/java/com/github/copilot/rpc/CopilotClientOptions.java`:
      allow `setCwd(null)` to clear a previously configured working directory.
    - `java/sdk/src/test/java/com/github/copilot/ClientOptionsE2ETest.java`: run
      explicit fake-stdio option forwarding tests under the in-process profile.
    - `java/sdk/src/test/java/com/github/copilot/ConfigCloneTest.java`: cover
      clearing a configured working directory.
    - `java/sdk/src/test/java/com/github/copilot/CopilotClientTest.java`: remove
      obsolete in-process skips from explicit subprocess and TCP lifecycle tests.
    - `java/sdk/src/test/java/com/github/copilot/CopilotClientTransportTest.java`:
      test explicit transport precedence, in-process option sanitization, and TCP
      token selection under the profile default.
    - `java/sdk/src/test/java/com/github/copilot/CopilotRequestTestSupport.java`:
      explicitly select stdio for request-handler tests that register the
      process-global LLM inference provider.
    - `java/sdk/src/test/java/com/github/copilot/E2ETestContext.java`: honor
      explicit transports, route request-handler clients to subprocess isolation,
      and clear environment, cwd, and CLI arguments before in-process client
      construction.
    - `java/sdk/src/test/java/com/github/copilot/GitHubTelemetryTest.java`: register
      fake runtime RPC handlers before socket message processing begins.
    - `java/sdk/src/test/java/com/github/copilot/MetadataApiTest.java`: run explicit
      stdio metadata tests instead of skipping them under the profile.
    - `java/sdk/src/test/java/com/github/copilot/PerSessionAuthTest.java`: run the
      explicit subprocess unauthenticated case under the profile.
    - `java/sdk/src/test/java/com/github/copilot/RpcServerMiscE2ETest.java`: run the
      explicit subprocess account lifecycle case under the profile.
    - `java/sdk/src/test/java/com/github/copilot/StreamingFidelityTest.java`: give
      the gpt-5.4 reasoning/streaming scenario an isolated proxy and runtime model
      cache.
    - `java/sdk/src/test/java/com/github/copilot/ffi/FfiRuntimeHostTest.java`: cover
      failed connection-open cleanup followed by successful sequential startup.
    - `java/sdk/src/test/java/com/github/copilot/ffi/NativeRuntimeLoaderTest.java`:
      cover resolving a configured CLI beside a prebuilds runtime.
    
    Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
    Copilot-Session: e03c4e94-97b0-41ad-9f4e-c01633dc0bf7

Author: Copilot

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

@@ -135,7 +135,9 @@ The .NET PR uses MSBuild targets to copy `runtime.node` from `runtimes/<rid>/nat
 
 The `package.json`-as-dependency-manifest approach was ruled out by experiment: `npm install` returns `EBADPLATFORM` for cross-platform packages, and `npm install --force` disables all npm safety checks. `npm pack` downloads the tarball without any platform check and does not require `--force`.
 
-Long-term target shape: the `copilot-native` module's `generate-resources` phase runs `npm pack @github/copilot-<platform>@${project.version}` for each supported platform. This produces `.tgz` tarballs, which are then extracted with `tar` to stage the `runtime.node` binary at `target/native-staging/<classifier>/native/<classifier>/runtime.node`. The version comes from `${project.version}` — the SDK and npm package versions are identical, so no separate version property is needed.
+Long-term target shape: the `copilot-native` module's `generate-resources` phase runs `npm pack @github/copilot-<platform>@${project.version}` for each supported platform. This produces `.tgz` tarballs, which are then extracted with `tar` to stage **both** the `runtime.node` shared library and the `copilot` CLI executable at `target/native-staging/<classifier>/native/<classifier>/`. The version comes from `${project.version}` — the SDK and npm package versions are identical, so no separate version property is needed.
+
+**Necessary-and-sufficient runtime artifact invariant:** The classifier JAR must contain both `native/<classifier>/runtime.node` (the cdylib loaded via JNA) **and** `native/<classifier>/copilot` (the CLI executable passed as `argv[0]` to `copilot_runtime_host_start`). The Rust `embedded_host.rs` spawns the CLI as a child process to service TypeScript method bodies not yet ported to Rust. Without the CLI executable, `host_start` fails — the classifier JAR is not self-sufficient. Both artifacts ship together in the same `@github/copilot-<platform>` npm package; both must be extracted and bundled. This matches the .NET SDK, which bundles the CLI binary and cdylib together under `runtimes/<rid>/native/`. When the TypeScript migration completes and `embedded_host.rs` no longer spawns a child process, the CLI executable can be removed from the classifier JAR.
 
 Temporary invariant (`linux-x64` only for now): perform this only for `linux-x64` on Ubuntu 24.04 in this phase; all other platform packaging is deferred to a later phase.
 

1917-java-embed-rust-cli-runtime-remove-before-merge/post-agentic-01-test-parity-fix-remaining-tests-01.md

@@ -0,0 +1,172 @@
+# Prompt: make the Java InProcess test run clean
+
+You are working in `/home/edburns/workareas/copilot-sdk-01`, branch
+`edburns/review-copilot-pr-2272`. Read these files first:
+
+- `1917-java-embed-rust-cli-runtime-remove-before-merge/post-agentic-01-test-parity-fix-remaining-tests.md`
+- `java/20260807-0145-job-logs.txt`
+- the current git diff and the Java test/runtime/harness sources
+
+The target command is:
+
+```bash
+cd java
+mvn clean verify -Pinprocess
+```
+
+Make the implementation and test changes necessary for a genuinely clean,
+non-hanging run. Do not solve this by broadly skipping tests, increasing
+timeouts, weakening assertions, or hiding errors. Preserve the negative-test
+assertions; expected negative cases may be logged, but they must not be
+reported as test errors.
+
+## What the interrupted log establishes
+
+The run was interrupted after more than an hour; it has no `BUILD SUCCESS`.
+There are 88 errors in 20 suites. The failures are highly clustered:
+
+- `std/in stream corrupted` appears during `AskUserTest`.
+- `ByokBearerTokenProviderE2ETest` has the expected fake 404 in one negative
+  case, but the other two tests fail because
+  `llmInference.setProvider` says “Another client is already the LLM inference
+  provider.”
+- The same provider-ownership error breaks
+  `CopilotRequestCancelErrorE2ETest`, `CopilotRequestHandlerE2ETest`,
+  `SessionConfigE2ETest`, and other provider/handler tests.
+- `CompactionTest`, `CopilotSessionTest`, `ErrorHandlingTest`,
+  `EventFidelityTest`, `ExecutorWiringTest`, `HooksTest`, `McpAndAgentsTest`,
+  `ModeHandlersTest`, `MultiProviderRegistryE2ETest`, `PermissionsTest`,
+  `PreMcpToolCallHookTest`, `RpcSessionStateExtrasE2ETest`,
+  `SessionConfigE2ETest`, and `SessionEventsE2ETest` contain repeated
+  approximately 60-second `sendAndWait`/future timeouts.
+- `GitHubTelemetryTest` fails immediately because an InProcess connection
+  receives `Method not found: connect` and `Method not found: ping`; determine
+  whether this test must explicitly use the subprocess/socket transport or
+  whether the InProcess endpoint is missing required handlers.
+- `RpcServerE2ETest` has a 30-second RPC timeout and
+  `RpcSessionStateExtrasE2ETest` has a 60-second timeout.
+- `PerSessionAuthTest` has one skipped test and a negative 401 “Bad
+  credentials” trace. The test itself is not an error.
+- `ClientOptionsE2ETest` skips all three tests. Other suites also report
+  intentional-looking skips: `CopilotClientTest` (14),
+  `CopilotClientTransportTest` (4), `MetadataApiTest` (3),
+  `RpcServerMiscE2ETest` (1), and `CompactionTest` (1).
+- Many stack traces in `CreateSessionReKeyEntryTest`, `JsonRpcClientTest`,
+  `LifecycleEventManagerTest`, `RpcHandlerDispatcherTest`, and
+  `SessionHandlerTest` are deliberately generated negative-test traces and
+  are followed by passing summaries. Do not misclassify them as failures.
+
+## Priority 1: stop stream corruption and fix InProcess ownership/lifecycle
+
+Investigate `std/in stream corrupted` first. Trace every process and stream
+created by the InProcess FFI path, `host_start`, the bundled `copilot`
+entrypoint, `NativeRuntimeLoader`, `InProcessRuntimeConnection`, `CapiProxy`,
+and Surefire. Identify which native/child process is writing bytes to the
+Surefire-controlled stdout/stdin protocol. Ensure child stdout/stderr are
+consumed or redirected in the same way as the supported transport and that
+the FFI receive/send streams are not closed or reused by another client.
+Do not merely suppress Surefire output.
+
+Then fix the “Another client is already the LLM inference provider” root
+cause. Determine whether clients, native hosts, provider registrations, or
+`InProcessEnvGuard` instances survive test teardown. Verify the close path on
+both successful and failed `start()`, failed `createSession()`, and failed
+requests. Ensure a failed startup cannot leave a provider registered and that
+each test context closes its client/proxy/runtime deterministically. If the
+InProcess runtime is process-global, serialize or otherwise coordinate provider
+ownership rather than allowing overlapping providers. Add focused regression
+coverage for failed-start cleanup and sequential client startup.
+
+The earlier context notes that `E2ETestContext.applyContextOptions()` must
+clear InProcess-incompatible `cwd` and `cliArgs` in addition to `environment`.
+Implement that carefully, and verify the actual setter semantics:
+`setEnvironment(null)` clears to an empty map, while `setCwd(null)` and
+`setCliArgs(null)` must be checked rather than assumed. Add or update tests so
+the options are truly absent according to constructor validation.
+
+## Priority 2: isolate and repair the common timeout
+
+After Priority 1, run small, serial selectors, not the full suite:
+
+```bash
+cd java
+COPILOT_SDK_DEFAULT_CONNECTION=inprocess mvn test -pl sdk \
+  -Dtest="AskUserTest,ByokBearerTokenProviderE2ETest,CopilotSessionTest" \
+  -DfailIfNoTests=false
+```
+
+Use a bounded shell timeout while debugging so a regression cannot consume an
+hour. For any remaining timeout, capture a thread dump and inspect the
+corresponding Surefire report plus replay-proxy output. Follow one request
+from Java JSON-RPC send, through the FFI callback/`QueueInputStream`, into the
+replay proxy, and back to the Java reader. Confirm that:
+
+1. `host_start` returns a valid handle and the child `copilot` entrypoint is
+   reachable.
+2. The request reaches the proxy with the expected snapshot.
+3. Every response/event is framed correctly and enqueued to the receive
+   stream.
+4. stream completion/EOF and client close wake blocked readers.
+5. callbacks do not depend on a thread or executor that has already shut down.
+
+Use `StreamingFidelityTest.testShouldEmitStreamingDeltasWithReasoningEffortConfigured`
+as the minimal streaming reproducer, but also test one ordinary
+`CopilotSessionTest` request. Do not patch each timed-out suite individually;
+the repeated 60-second failures indicate a shared transport or lifecycle
+defect. Once the common path works, rerun representative handler, hook,
+permission, event, session-config, MCP, and RPC-server selectors and only
+then the complete profile.
+
+`GitHubTelemetryTest` is a separate transport-contract issue: inspect its
+test setup and the supported connection mode. If it intentionally uses a
+minimal fake RPC peer that only supports telemetry, make it explicitly select
+that transport so the global InProcess profile cannot route it to a runtime
+without `connect`/`ping`. If InProcess is intended, implement the missing
+protocol surface and add focused coverage.
+
+## Priority 3: remove unjustified skips
+
+Audit every skipped test in the log and the associated assumptions. For each:
+
+- make it run under InProcess when the behavior is transport-independent;
+- explicitly force subprocess/socket transport when the test is specifically
+  validating subprocess-only options or protocol behavior; or
+- change the test setup so the same public behavior is exercised through
+  InProcess.
+
+Do not add a profile-wide exclusion and do not convert skipped tests to
+passing assertions. In particular, investigate all three
+`ClientOptionsE2ETest` skips, the `PerSessionAuthTest` skip, and the skips in
+`CopilotClientTest`, `CopilotClientTransportTest`, `MetadataApiTest`,
+`RpcServerMiscE2ETest`, and `CompactionTest`. The final profile run should
+have zero skips unless a test is demonstrably impossible on the platform and
+the repository’s existing policy explicitly permits it; document any
+remaining exception in the test source.
+
+## Priority 4: make expected negative output intentional
+
+Do not alter assertions for negative tests. After all tests pass, reduce noisy
+expected stack-trace logging only where the repository’s logging conventions
+support it: distinguish expected test-triggered failures from unexpected
+transport failures, and avoid logging full stack traces at warning/error for
+the expected path if that can be done without hiding real failures. The
+`fake byok endpoint`, `401 Bad credentials`, `session.resume` not-found,
+handler exceptions, malformed JSON, socket-close, and re-key traces must
+remain asserted and diagnosable.
+
+## Validation and completion criteria
+
+Use the repository’s normal Java bootstrap and Maven logging conventions.
+Format Java changes with `mvn spotless:apply` from `java`. Run focused tests
+after each root-cause fix, then:
+
+```bash
+cd java
+mvn clean verify -Pinprocess
+```
+
+The task is complete only when this command terminates normally with
+`BUILD SUCCESS`, all test suites report zero failures and zero errors, no
+test hangs or 60-second transport timeouts occur, no Surefire stream
+corruption occurs, and the skip count is zero or each explicitly justified
+platform exception is documented and approved by the existing test policy.

1917-java-embed-rust-cli-runtime-remove-before-merge/post-agentic-01-test-parity-fix-remaining-tests.md

@@ -0,0 +1,125 @@
+# Fix remaining InProcess test parity failures
+
+## Context
+
+Branch: `edburns/review-copilot-pr-2272` (local worktree at `copilot-sdk-01`)
+Push target: `git push upstream HEAD:copilot/edburns1917-java-embed-rust-cli-runtime-post-agent`
+
+The `-Pinprocess` Maven profile sets `COPILOT_SDK_DEFAULT_CONNECTION=inprocess`, which forces all E2E tests to use the InProcess FFI transport instead of subprocess. Most tests now pass. 24 tests still fail in two categories.
+
+## Category 1: Tests that set `cwd` or `cliArgs` on options
+
+These tests go through `ctx.createClient(options)` → `E2ETestContext.applyContextOptions()`. The InProcess branch absorbs `environment` into `InProcessEnvGuard` and nulls it, but does NOT do the same for `cwd` or `cliArgs`. The `CopilotClient` constructor then calls `validateEnvironmentOptions()` which rejects non-null `cwd`/`cliArgs` for InProcess.
+
+**Fix:** In `E2ETestContext.applyContextOptions()`, when InProcess mode is detected, also null out `cwd` and `cliArgs` before constructing the client. For `cwd`, it's meaningless in InProcess (host process cwd is already set). For `cliArgs`, they're subprocess-specific flags.
+
+Location: `java/sdk/src/test/java/com/github/copilot/E2ETestContext.java` lines 354-376
+
+Current InProcess branch in `applyContextOptions`:
+```java
+if (isInProcessMode(options)) {
+    InProcessEnvGuard guard = new InProcessEnvGuard(buildInProcessEnvironment(options));
+    inProcessEnvGuards.add(guard);
+    try {
+        options.setEnvironment(null);
+        return new CopilotClient(options, guard::close);
+    } catch (RuntimeException e) {
+        guard.close();
+        throw e;
+    }
+}
+```
+
+Needs to also null `cwd` and `cliArgs`:
+```java
+options.setEnvironment(null);
+options.setCwd(null);
+options.setCliArgs(null);
+```
+
+Affected tests: `PerSessionAuthTest` (sets cwd+environment), possibly others.
+
+## Category 2: StreamingFidelityTest hang
+
+`StreamingFidelityTest.testShouldEmitStreamingDeltasWithReasoningEffortConfigured` hangs indefinitely in InProcess mode. The main thread is blocked on `CompletableFuture.get()` at line 258. The JSON-RPC reader thread is reading from `QueueInputStream` (the InProcess FFI receive stream) but never receives the expected response.
+
+This is a functional issue, not a validation issue. The replay proxy is running (CapiProxy thread is active), but the InProcess transport isn't completing the streaming interaction.
+
+Diagnosis approach:
+1. Check if the test's replay snapshot exists and is correct for streaming
+2. Check if `host_start` succeeds for this test (serverHandle != 0)
+3. jstack showed the reader thread blocked in `QueueInputStream.read()` — no data arriving via the FFI callback
+4. Possible causes: the replay proxy response format doesn't match what the InProcess runtime expects for streaming, or the connection isn't routing correctly through the replay proxy
+
+## Key architectural facts
+
+- `runtime.node` is loaded via JNA. `copilot` CLI binary is spawned as child by `host_start` via `argv[0]`.
+- Both are now bundled in the classifier JAR at `native/<classifier>/runtime.node` and `native/<classifier>/copilot`.
+- `NativeRuntimeLoader.resolve()` extracts both to `~/.copilot/runtime-cache/<version>/<classifier>/`.
+- `NativeRuntimeLoader.resolveEntrypoint()` finds `copilot` alongside `runtime.node`.
+- `CopilotClient.resolveInProcessEntrypoint()` simply calls `NativeRuntimeLoader.resolveEntrypoint().toString()`.
+- `InProcessEnvGuard` uses JNA `libc.setenv()` to mutate the native process env (not visible to `System.getenv()`).
+- The replay proxy (CapiProxy) runs as a Node.js subprocess serving YAML snapshot responses.
+
+## CopilotClientOptions.setEnvironment(null) quirk
+
+`setEnvironment(null)` does NOT set the field to null — it calls `this.environment.clear()`, leaving an empty HashMap. `getEnvironment()` then returns a non-null empty map. The validation now checks `!isEmpty()` too (already fixed).
+
+Similarly, check if `setCwd(null)` / `setCliArgs(null)` have similar behavior. If `setCwd(null)` doesn't actually null the field, the validation might still fire.
+
+## Validation in CopilotClient constructor
+
+```java
+private static void validateEnvironmentOptions(CopilotClientOptions options, RuntimeConnection connection) {
+    if (!(connection instanceof InProcessRuntimeConnection)) return;
+    rejectInProcessOption("Environment", options.getEnvironment() != null && !options.getEnvironment().isEmpty(), ...);
+    rejectInProcessOption("Telemetry", options.getTelemetry() != null, ...);
+    rejectInProcessOption("Cwd", options.getCwd() != null, ...);
+    rejectInProcessOption("CliArgs", options.getCliArgs() != null && options.getCliArgs().length > 0, ...);
+}
+```
+
+## resolveDefaultConnection precedence (already fixed)
+
+When `COPILOT_SDK_DEFAULT_CONNECTION=inprocess` but `cliUrl`/`cliPath`/`port` are explicitly set, the explicit options win and subprocess transport is used. Tests like `McpAuthInterestRegistrationTest` that create `new CopilotClient(options.setCliUrl(...))` directly now correctly bypass InProcess.
+
+## Full list of 24 failing test methods
+
+```
+ByokBearerTokenProviderE2ETest (3 methods)
+CopilotRequestCancelErrorE2ETest (2)
+CopilotRequestHandlerE2ETest (2)
+CopilotRequestSessionIdE2ETest (1)
+GitHubTelemetryTest (2)
+McpAuthInterestRegistrationTest (3)
+ModeHandlersTest (2)
+PerSessionAuthTest (3)
+ProviderEndpointE2ETest (2)
+RpcServerE2ETest (1 - testShouldAddSecretFilterValues — NOW PASSES)
+SessionConfigE2ETest (2)
+StreamingFidelityTest (1 - hangs)
+SubagentHooksE2ETest (1)
+```
+
+## Commands
+
+```bash
+# Run all tests with InProcess
+cd java && mvn clean verify -Pinprocess
+
+# Run specific failing tests
+COPILOT_SDK_DEFAULT_CONNECTION=inprocess mvn test -pl sdk -Dtest="PerSessionAuthTest,StreamingFidelityTest" -DfailIfNoTests=false
+
+# Format before commit
+mvn spotless:apply
+
+# Push
+git push upstream HEAD:copilot/edburns1917-java-embed-rust-cli-runtime-post-agent
+```
+
+## Java env bootstrap (required before any mvn/java command)
+```bash
+export JAVA_HOME="/usr/lib/jvm/msopenjdk-25-amd64"
+export M2_HOME="${HOME}/Downloads/apache-maven-3.9.8"
+export PATH="${M2_HOME}/bin:${JAVA_HOME}/bin:${PATH}"
+```