From 4894db0ddd5b95b4d4e8c1fdb12b419411538e58 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 30 Jul 2026 21:41:32 +0000 Subject: [PATCH 1/5] Initial plan From a2b9815ea5f02bb737ef5b7f69bc1263d72afa85 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 30 Jul 2026 21:57:49 +0000 Subject: [PATCH 2/5] feat(java): implement NativeRuntimeLoader for runtime.node extraction and caching (task 4.3) Co-authored-by: edburns <75821+edburns@users.noreply.github.com> --- java/sdk/pom.xml | 6 + .../copilot/ffi/NativeRuntimeLoader.java | 248 ++++++++++++++++ .../main/resources/copilot-runtime.properties | 3 + .../copilot/ffi/NativeRuntimeLoaderTest.java | 276 ++++++++++++++++++ 4 files changed, 533 insertions(+) create mode 100644 java/sdk/src/main/java/com/github/copilot/ffi/NativeRuntimeLoader.java create mode 100644 java/sdk/src/main/resources/copilot-runtime.properties create mode 100644 java/sdk/src/test/java/com/github/copilot/ffi/NativeRuntimeLoaderTest.java diff --git a/java/sdk/pom.xml b/java/sdk/pom.xml index 6aa000cd9d..e24c2ebf2e 100644 --- a/java/sdk/pom.xml +++ b/java/sdk/pom.xml @@ -106,6 +106,12 @@ + + + src/main/resources + true + + diff --git a/java/sdk/src/main/java/com/github/copilot/ffi/NativeRuntimeLoader.java b/java/sdk/src/main/java/com/github/copilot/ffi/NativeRuntimeLoader.java new file mode 100644 index 0000000000..a476e0a204 --- /dev/null +++ b/java/sdk/src/main/java/com/github/copilot/ffi/NativeRuntimeLoader.java @@ -0,0 +1,248 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + *--------------------------------------------------------------------------------------------*/ + +package com.github.copilot.ffi; + +import java.io.FileNotFoundException; +import java.io.IOException; +import java.io.InputStream; +import java.net.URL; +import java.nio.channels.FileChannel; +import java.nio.file.AtomicMoveNotSupportedException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.nio.file.StandardOpenOption; +import java.util.Properties; + +/** + * Locates the {@code runtime.node} native binary, extracts it to a versioned + * cache directory, and returns the filesystem path for JNA to load. + * + *

+ * Resolution order: + *

    + *
  1. {@code COPILOT_CLI_PATH} environment variable — if set, checks for + * {@code runtime.node} in the same directory as the specified CLI binary.
    2. + *
  2. Classpath resource {@code native//runtime.node} — extracted + * atomically to + * {@code ~/.copilot/runtime-cache///runtime.node}.
    3. + *
+ */ +public final class NativeRuntimeLoader { + + static final String RUNTIME_FILENAME = "runtime.node"; + static final String COPILOT_CLI_PATH_ENV = "COPILOT_CLI_PATH"; + static final String VERSION_RESOURCE = "copilot-runtime.properties"; + + private NativeRuntimeLoader() { + } + + /** + * Resolves the filesystem path to the {@code runtime.node} binary. + * + *

+ * Follows the resolution order documented on this class. The returned path is + * guaranteed to refer to a regular, non-empty file at the time of return. + * + * @return absolute path to the {@code runtime.node} binary + * @throws IOException + * if the binary cannot be located or extracted + * @throws IllegalStateException + * if required resources are missing or extraction fails + */ + public static Path resolve() throws IOException { + ClassLoader loader = NativeRuntimeLoader.class.getClassLoader(); + String classifier = PlatformDetector.detectClassifier(); + String version = readVersion(loader); + Path cacheBase = defaultCacheBase(); + return resolve(System.getenv(COPILOT_CLI_PATH_ENV), cacheBase, loader, classifier, version); + } + + /** + * Reads the SDK version from the filtered {@code copilot-runtime.properties} + * resource. + * + * @return the version string + * @throws IOException + * if the resource cannot be read + * @throws IllegalStateException + * if the resource is missing or the version property is blank + */ + static String readVersion(ClassLoader loader) throws IOException { + URL resource = loader.getResource(VERSION_RESOURCE); + if (resource == null) { + throw new IllegalStateException("Missing version resource: " + VERSION_RESOURCE + + " — ensure Maven resource filtering has run (mvn process-resources)"); + } + Properties props = new Properties(); + try (InputStream in = resource.openStream()) { + props.load(in); + } + String version = props.getProperty("version"); + if (version == null || version.isBlank()) { + throw new IllegalStateException("Blank or missing 'version' property in " + VERSION_RESOURCE + + " — check Maven resource filtering configuration"); + } + return version; + } + + /** + * Resolves the runtime binary path using the given parameters. Package-private + * to allow injection of test doubles in unit tests. + * + * @param cliPathEnv + * value of the {@code COPILOT_CLI_PATH} environment variable, or + * {@code null} + * @param cacheBase + * base directory for the extraction cache + * @param loader + * class loader used to locate classpath resources + * @param classifier + * platform classifier (e.g. {@code linux-x64}) + * @param version + * SDK version used as the cache key + * @return path to the resolved {@code runtime.node} binary + * @throws IOException + * if extraction or file I/O fails + * @throws IllegalStateException + * if required resources are missing or extraction fails + */ + static Path resolve(String cliPathEnv, Path cacheBase, ClassLoader loader, String classifier, String version) + throws IOException { + Path cliOverride = resolveFromCliPath(cliPathEnv); + if (cliOverride != null) { + return cliOverride; + } + return extractToCache(cacheBase, loader, classifier, version); + } + + /** + * Checks whether a {@code runtime.node} file exists alongside the binary + * referred to by {@code cliPathStr}. + * + * @param cliPathStr + * value of the {@code COPILOT_CLI_PATH} environment variable + * @return path to the sibling {@code runtime.node} if it is a regular non-empty + * file, or {@code null} if the override does not apply + * @throws IOException + * if file-size probing fails + */ + static Path resolveFromCliPath(String cliPathStr) throws IOException { + if (cliPathStr == null || cliPathStr.isBlank()) { + return null; + } + Path cliPath = Path.of(cliPathStr); + Path parent = cliPath.getParent(); + Path candidate = parent != null ? parent.resolve(RUNTIME_FILENAME) : Path.of(RUNTIME_FILENAME); + if (Files.isRegularFile(candidate) && Files.size(candidate) > 0) { + return candidate; + } + return null; + } + + /** + * Extracts the classpath resource {@code native//runtime.node} to + * the versioned cache directory, using an atomic publish sequence to prevent + * readers from observing a partially-written file. + * + * @param cacheBase + * root cache directory (e.g. {@code ~/.copilot/runtime-cache}) + * @param loader + * class loader used to open the classpath resource + * @param classifier + * platform classifier (e.g. {@code linux-x64}) + * @param version + * SDK version used as the cache key + * @return path to the extracted {@code runtime.node} binary + * @throws IOException + * if I/O or the atomic rename fails + * @throws IllegalStateException + * if the classpath resource is missing or empty, or if the + * filesystem does not support atomic moves + */ + static Path extractToCache(Path cacheBase, ClassLoader loader, String classifier, String version) + throws IOException { + String resourcePath = "native/" + classifier + "/" + RUNTIME_FILENAME; + Path cacheDir = cacheBase.resolve(version).resolve(classifier); + Path cached = cacheDir.resolve(RUNTIME_FILENAME); + + // Step 1 — fast path: return an existing valid cache entry. + if (isValidCachedFile(cached)) { + return cached; + } + + // Step 2 — locate the classpath resource before creating any files. + URL resource = loader.getResource(resourcePath); + if (resource == null) { + throw new FileNotFoundException("Native runtime not found on classpath: " + resourcePath + + " — add the matching classifier JAR to the classpath"); + } + + // Step 3 — ensure the cache directory exists. + Files.createDirectories(cacheDir); + + // Step 4 — write to a unique sibling temp file, then publish atomically. + Path temp = Files.createTempFile(cacheDir, "runtime-tmp-", ".node"); + try { + copyResourceToTemp(resource, resourcePath, temp); + publishAtomically(temp, cached); + temp = null; // transfer ownership; do not delete in finally + } finally { + if (temp != null) { + tryDelete(temp); + } + } + + return cached; + } + + private static boolean isValidCachedFile(Path path) throws IOException { + if (!Files.isRegularFile(path)) { + return false; + } + return Files.size(path) > 0; + } + + private static void copyResourceToTemp(URL resource, String resourcePath, Path temp) throws IOException { + try (InputStream in = resource.openStream()) { + long bytesWritten = Files.copy(in, temp, StandardCopyOption.REPLACE_EXISTING); + if (bytesWritten == 0) { + throw new IllegalStateException("Classpath resource is empty: " + resourcePath); + } + } + // Flush OS buffers to durable storage before the atomic rename. + try (FileChannel channel = FileChannel.open(temp, StandardOpenOption.WRITE)) { + channel.force(true); + } + } + + private static void publishAtomically(Path temp, Path cached) throws IOException { + try { + Files.move(temp, cached, StandardCopyOption.ATOMIC_MOVE); + } catch (AtomicMoveNotSupportedException ex) { + throw new IllegalStateException( + "Filesystem does not support atomic moves; cannot safely publish runtime.node to " + cached, ex); + } catch (java.nio.file.FileAlreadyExistsException ex) { + // Another process won the race — accept the winner if it is a valid file. + if (isValidCachedFile(cached)) { + return; + } + throw new IllegalStateException( + "Concurrent extraction race: target already exists but is not a valid file: " + cached, ex); + } + } + + private static void tryDelete(Path path) { + try { + Files.deleteIfExists(path); + } catch (IOException ignored) { + // Best-effort cleanup; an orphaned temp file in the cache directory is benign. + } + } + + private static Path defaultCacheBase() { + return Path.of(System.getProperty("user.home"), ".copilot", "runtime-cache"); + } +} diff --git a/java/sdk/src/main/resources/copilot-runtime.properties b/java/sdk/src/main/resources/copilot-runtime.properties new file mode 100644 index 0000000000..2900464443 --- /dev/null +++ b/java/sdk/src/main/resources/copilot-runtime.properties @@ -0,0 +1,3 @@ +# This file is processed by Maven resource filtering. +# The ${project.version} placeholder is replaced at build time. +version=${project.version} diff --git a/java/sdk/src/test/java/com/github/copilot/ffi/NativeRuntimeLoaderTest.java b/java/sdk/src/test/java/com/github/copilot/ffi/NativeRuntimeLoaderTest.java new file mode 100644 index 0000000000..0dcdc559ef --- /dev/null +++ b/java/sdk/src/test/java/com/github/copilot/ffi/NativeRuntimeLoaderTest.java @@ -0,0 +1,276 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + *--------------------------------------------------------------------------------------------*/ + +package com.github.copilot.ffi; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.IOException; +import java.net.URL; +import java.net.URLClassLoader; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +class NativeRuntimeLoaderTest { + + private static final String TEST_CLASSIFIER = "linux-x64"; + private static final String TEST_VERSION = "1.2.3-test"; + private static final byte[] FAKE_BINARY_CONTENT = "fake runtime.node binary content".getBytes(); + + // ------------------------------------------------------------------------- + // Version properties resource reading + // ------------------------------------------------------------------------- + + @Test + void readVersionReturnsVersionFromPropertiesResource(@TempDir Path tempDir) throws Exception { + ClassLoader loader = classLoaderWithVersionResource(tempDir, "1.0.5-preview"); + assertEquals("1.0.5-preview", NativeRuntimeLoader.readVersion(loader)); + } + + @Test + void readVersionThrowsWhenResourceMissing() { + ClassLoader emptyLoader = new URLClassLoader(new URL[0], null); + IllegalStateException ex = assertThrows(IllegalStateException.class, + () -> NativeRuntimeLoader.readVersion(emptyLoader)); + assertTrue(ex.getMessage().contains(NativeRuntimeLoader.VERSION_RESOURCE)); + } + + @Test + void readVersionThrowsWhenVersionPropertyIsBlank(@TempDir Path tempDir) throws Exception { + ClassLoader loader = classLoaderWithVersionResource(tempDir, " "); + IllegalStateException ex = assertThrows(IllegalStateException.class, + () -> NativeRuntimeLoader.readVersion(loader)); + assertTrue(ex.getMessage().contains("version")); + } + + // ------------------------------------------------------------------------- + // COPILOT_CLI_PATH override + // ------------------------------------------------------------------------- + + @Test + void resolveFromCliPathReturnsSiblingWhenRuntimeNodeExists(@TempDir Path tempDir) throws Exception { + Path fakeCliPath = tempDir.resolve("copilot"); + Files.createFile(fakeCliPath); + Path runtimeNode = tempDir.resolve(NativeRuntimeLoader.RUNTIME_FILENAME); + Files.write(runtimeNode, FAKE_BINARY_CONTENT); + + Path result = NativeRuntimeLoader.resolveFromCliPath(fakeCliPath.toString()); + + assertEquals(runtimeNode, result); + } + + @Test + void resolveFromCliPathReturnsNullWhenRuntimeNodeMissing(@TempDir Path tempDir) throws Exception { + Path fakeCliPath = tempDir.resolve("copilot"); + Files.createFile(fakeCliPath); + + assertNull(NativeRuntimeLoader.resolveFromCliPath(fakeCliPath.toString())); + } + + @Test + void resolveFromCliPathReturnsNullWhenEnvIsNull() throws Exception { + assertNull(NativeRuntimeLoader.resolveFromCliPath(null)); + } + + @Test + void resolveFromCliPathReturnsNullWhenEnvIsBlank() throws Exception { + assertNull(NativeRuntimeLoader.resolveFromCliPath(" ")); + } + + @Test + void resolveFromCliPathReturnsNullWhenRuntimeNodeIsEmpty(@TempDir Path tempDir) throws Exception { + Path fakeCliPath = tempDir.resolve("copilot"); + Files.createFile(fakeCliPath); + Path runtimeNode = tempDir.resolve(NativeRuntimeLoader.RUNTIME_FILENAME); + Files.createFile(runtimeNode); // empty file + + assertNull(NativeRuntimeLoader.resolveFromCliPath(fakeCliPath.toString())); + } + + @Test + void cliPathOverrideTakesPriorityOverClasspathExtraction(@TempDir Path tempDir) throws Exception { + // Create a valid runtime.node alongside the fake CLI path + Path fakeCliDir = tempDir.resolve("cli-dir"); + Files.createDirectories(fakeCliDir); + Path fakeCliPath = fakeCliDir.resolve("copilot"); + Files.createFile(fakeCliPath); + Path runtimeNode = fakeCliDir.resolve(NativeRuntimeLoader.RUNTIME_FILENAME); + Files.write(runtimeNode, FAKE_BINARY_CONTENT); + + // Provide a classpath loader that also has the resource (should be ignored) + Path cacheBase = tempDir.resolve("cache"); + ClassLoader loader = classLoaderWithRuntimeResource(tempDir, TEST_CLASSIFIER); + + Path result = NativeRuntimeLoader.resolve(fakeCliPath.toString(), cacheBase, loader, TEST_CLASSIFIER, + TEST_VERSION); + + assertEquals(runtimeNode, result); + } + + // ------------------------------------------------------------------------- + // Classpath extraction to cache + // ------------------------------------------------------------------------- + + @Test + void extractToCacheCopiesResourceToVersionedCacheDirectory(@TempDir Path tempDir) throws Exception { + Path cacheBase = tempDir.resolve("cache"); + ClassLoader loader = classLoaderWithRuntimeResource(tempDir, TEST_CLASSIFIER); + + Path result = NativeRuntimeLoader.extractToCache(cacheBase, loader, TEST_CLASSIFIER, TEST_VERSION); + + Path expected = cacheBase.resolve(TEST_VERSION).resolve(TEST_CLASSIFIER) + .resolve(NativeRuntimeLoader.RUNTIME_FILENAME); + assertEquals(expected, result); + assertTrue(Files.isRegularFile(result)); + assertTrue(Files.size(result) > 0); + } + + @Test + void extractToCacheReturnsCachedFileOnSecondCall(@TempDir Path tempDir) throws Exception { + Path cacheBase = tempDir.resolve("cache"); + ClassLoader loader = classLoaderWithRuntimeResource(tempDir, TEST_CLASSIFIER); + + Path first = NativeRuntimeLoader.extractToCache(cacheBase, loader, TEST_CLASSIFIER, TEST_VERSION); + long modifiedAfterFirstExtraction = Files.getLastModifiedTime(first).toMillis(); + + // Small delay so modification time would differ if the file were rewritten + Thread.sleep(50); + + Path second = NativeRuntimeLoader.extractToCache(cacheBase, loader, TEST_CLASSIFIER, TEST_VERSION); + long modifiedAfterSecondCall = Files.getLastModifiedTime(second).toMillis(); + + assertEquals(first, second); + assertEquals(modifiedAfterFirstExtraction, modifiedAfterSecondCall, + "Cached file must not be overwritten on cache hit"); + } + + @Test + void extractToCacheThrowsWhenClasspathResourceMissing(@TempDir Path tempDir) { + Path cacheBase = tempDir.resolve("cache"); + ClassLoader emptyLoader = new URLClassLoader(new URL[0], null); + + assertThrows(IOException.class, + () -> NativeRuntimeLoader.extractToCache(cacheBase, emptyLoader, TEST_CLASSIFIER, TEST_VERSION)); + } + + @Test + void extractedBinaryContentsMatchClasspathResource(@TempDir Path tempDir) throws Exception { + Path cacheBase = tempDir.resolve("cache"); + ClassLoader loader = classLoaderWithRuntimeResource(tempDir, TEST_CLASSIFIER); + + Path result = NativeRuntimeLoader.extractToCache(cacheBase, loader, TEST_CLASSIFIER, TEST_VERSION); + + byte[] extracted = Files.readAllBytes(result); + assertBytesEqual(FAKE_BINARY_CONTENT, extracted); + } + + @Test + void extractToCacheFiltersClasspathByClassifier(@TempDir Path tempDir) throws Exception { + // Put resources for two classifiers; extraction must target only the requested + // one + Path cacheBase = tempDir.resolve("cache"); + ClassLoader loader = classLoaderWithRuntimeResource(tempDir, TEST_CLASSIFIER); + + Path result = NativeRuntimeLoader.extractToCache(cacheBase, loader, TEST_CLASSIFIER, TEST_VERSION); + + assertTrue(result.toString().contains(TEST_CLASSIFIER), "Cache path must include the classifier: " + result); + } + + // ------------------------------------------------------------------------- + // Concurrent extraction safety + // ------------------------------------------------------------------------- + + @Test + void concurrentExtractionByMultipleThreadsBothSucceed(@TempDir Path tempDir) throws Exception { + Path cacheBase = tempDir.resolve("cache"); + ClassLoader loader = classLoaderWithRuntimeResource(tempDir, TEST_CLASSIFIER); + int threadCount = 8; + CountDownLatch startGate = new CountDownLatch(1); + ExecutorService pool = Executors.newFixedThreadPool(threadCount); + List> futures = new ArrayList<>(); + + for (int i = 0; i < threadCount; i++) { + futures.add(pool.submit(() -> { + startGate.await(); + return NativeRuntimeLoader.extractToCache(cacheBase, loader, TEST_CLASSIFIER, TEST_VERSION); + })); + } + + startGate.countDown(); + pool.shutdown(); + assertTrue(pool.awaitTermination(10, TimeUnit.SECONDS)); + + Path expected = cacheBase.resolve(TEST_VERSION).resolve(TEST_CLASSIFIER) + .resolve(NativeRuntimeLoader.RUNTIME_FILENAME); + for (Future future : futures) { + Path result = future.get(); + assertEquals(expected, result); + assertTrue(Files.isRegularFile(result)); + assertTrue(Files.size(result) > 0); + } + } + + // ------------------------------------------------------------------------- + // resolve() -- full resolution chain + // ------------------------------------------------------------------------- + + @Test + void resolveWithNullCliEnvExtractsFromClasspath(@TempDir Path tempDir) throws Exception { + Path cacheBase = tempDir.resolve("cache"); + ClassLoader loader = classLoaderWithRuntimeResource(tempDir, TEST_CLASSIFIER); + + Path result = NativeRuntimeLoader.resolve(null, cacheBase, loader, TEST_CLASSIFIER, TEST_VERSION); + + assertNotNull(result); + assertTrue(Files.isRegularFile(result)); + assertTrue(Files.size(result) > 0); + } + + @Test + void resolveThrowsWhenNoClasspathResourceAndNoCliOverride(@TempDir Path tempDir) { + Path cacheBase = tempDir.resolve("cache"); + ClassLoader emptyLoader = new URLClassLoader(new URL[0], null); + + assertThrows(IOException.class, + () -> NativeRuntimeLoader.resolve(null, cacheBase, emptyLoader, TEST_CLASSIFIER, TEST_VERSION)); + } + + // ------------------------------------------------------------------------- + // Helpers + // ------------------------------------------------------------------------- + + private static ClassLoader classLoaderWithVersionResource(Path tempDir, String version) throws IOException { + Path propsFile = tempDir.resolve(NativeRuntimeLoader.VERSION_RESOURCE); + Files.writeString(propsFile, "version=" + version + "\n"); + return new URLClassLoader(new URL[]{tempDir.toUri().toURL()}, null); + } + + private static ClassLoader classLoaderWithRuntimeResource(Path tempDir, String classifier) throws IOException { + Path resourceDir = tempDir.resolve("native").resolve(classifier); + Files.createDirectories(resourceDir); + Files.write(resourceDir.resolve(NativeRuntimeLoader.RUNTIME_FILENAME), FAKE_BINARY_CONTENT); + return new URLClassLoader(new URL[]{tempDir.toUri().toURL()}, null); + } + + private static void assertBytesEqual(byte[] expected, byte[] actual) { + assertEquals(expected.length, actual.length, "Array lengths differ"); + for (int i = 0; i < expected.length; i++) { + assertEquals(expected[i], actual[i], "Byte differs at index " + i); + } + } +} From 48ce2eb57482323120cfba03fe9ff14cb20106c0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 30 Jul 2026 22:26:32 +0000 Subject: [PATCH 3/5] fix(java): fix NativeRuntimeLoader to implement 3-source resolution order and add AtomicPublisher test seam Co-authored-by: edburns <75821+edburns@users.noreply.github.com> --- .../copilot/ffi/NativeRuntimeLoader.java | 273 +++++++++++++++--- .../copilot/ffi/NativeRuntimeLoaderTest.java | 196 ++++++++++--- 2 files changed, 382 insertions(+), 87 deletions(-) diff --git a/java/sdk/src/main/java/com/github/copilot/ffi/NativeRuntimeLoader.java b/java/sdk/src/main/java/com/github/copilot/ffi/NativeRuntimeLoader.java index a476e0a204..8b5ce3d90b 100644 --- a/java/sdk/src/main/java/com/github/copilot/ffi/NativeRuntimeLoader.java +++ b/java/sdk/src/main/java/com/github/copilot/ffi/NativeRuntimeLoader.java @@ -23,11 +23,17 @@ *

* Resolution order: *

    - *
  1. {@code COPILOT_CLI_PATH} environment variable — if set, checks for - * {@code runtime.node} in the same directory as the specified CLI binary.
    2. - *
  2. Classpath resource {@code native//runtime.node} — extracted - * atomically to + *
  3. {@code COPILOT_CLI_PATH} — explicit path to the + * {@code runtime.node} binary. Checked before any classpath or platform work so + * the override is usable even when those resources are unavailable. A non-blank + * value that does not refer to a regular, non-empty file is a configuration + * error; no silent fallback occurs.
    4. + *
  4. Classpath resource + * {@code native//runtime.node} — extracted atomically to * {@code ~/.copilot/runtime-cache///runtime.node}.
    5. + *
  5. Bundled-CLI sibling — {@code runtime.node} in the + * directory where the in-process runtime was installed alongside the bundled + * CLI binary. Used only when the classpath resource is absent.
    6. *
*/ public final class NativeRuntimeLoader { @@ -36,6 +42,51 @@ public final class NativeRuntimeLoader { static final String COPILOT_CLI_PATH_ENV = "COPILOT_CLI_PATH"; static final String VERSION_RESOURCE = "copilot-runtime.properties"; + /** + * Abstraction for the atomic publish step, enabling deterministic failure + * injection in tests while preserving {@link StandardCopyOption#ATOMIC_MOVE} in + * production. + */ + @FunctionalInterface + interface AtomicPublisher { + /** + * Atomically publishes {@code temp} to {@code cached}. + * + * @param temp + * fully-written temporary file in the same directory as + * {@code cached} + * @param cached + * intended final location + * @throws IOException + * if the move fails + */ + void publish(Path temp, Path cached) throws IOException; + } + + /** + * Production publisher: {@link Files#move} with + * {@link StandardCopyOption#ATOMIC_MOVE}. + */ + static final AtomicPublisher DEFAULT_PUBLISHER = (temp, cached) -> { + try { + Files.move(temp, cached, StandardCopyOption.ATOMIC_MOVE); + } catch (AtomicMoveNotSupportedException ex) { + throw new IllegalStateException("Filesystem does not support atomic moves; cannot safely publish " + + RUNTIME_FILENAME + " to " + cached, ex); + } catch (java.nio.file.FileAlreadyExistsException ex) { + // Another process won the race — accept the winner if it is a valid file. + try { + if (isValidCachedFile(cached)) { + return; + } + } catch (IOException ignored) { + // fall through to the error below + } + throw new IllegalStateException( + "Concurrent extraction race: target already exists but is not a valid file: " + cached, ex); + } + }; + private NativeRuntimeLoader() { } @@ -43,8 +94,9 @@ private NativeRuntimeLoader() { * Resolves the filesystem path to the {@code runtime.node} binary. * *

- * Follows the resolution order documented on this class. The returned path is - * guaranteed to refer to a regular, non-empty file at the time of return. + * Follows the three-step resolution order documented on this class. The + * returned path is guaranteed to refer to a regular, non-empty file at the time + * of return. * * @return absolute path to the {@code runtime.node} binary * @throws IOException @@ -53,11 +105,23 @@ private NativeRuntimeLoader() { * if required resources are missing or extraction fails */ public static Path resolve() throws IOException { + String cliPathEnv = System.getenv(COPILOT_CLI_PATH_ENV); + + // Source 1: COPILOT_CLI_PATH as explicit runtime override. + // Checked before any classpath or platform work so the override is usable + // even when those resources are unavailable. + if (cliPathEnv != null && !cliPathEnv.isBlank()) { + return resolveFromExplicitPath(cliPathEnv); + } + ClassLoader loader = NativeRuntimeLoader.class.getClassLoader(); String classifier = PlatformDetector.detectClassifier(); String version = readVersion(loader); Path cacheBase = defaultCacheBase(); - return resolve(System.getenv(COPILOT_CLI_PATH_ENV), cacheBase, loader, classifier, version); + Path bundledCliDir = findBundledCliDirectory(); + + return resolveFromClasspathOrBundledCli(cacheBase, loader, classifier, version, bundledCliDir, + DEFAULT_PUBLISHER); } /** @@ -90,7 +154,8 @@ static String readVersion(ClassLoader loader) throws IOException { /** * Resolves the runtime binary path using the given parameters. Package-private - * to allow injection of test doubles in unit tests. + * to allow injection of test doubles in unit tests. Uses + * {@link #DEFAULT_PUBLISHER} and no bundled-CLI directory. * * @param cliPathEnv * value of the {@code COPILOT_CLI_PATH} environment variable, or @@ -111,41 +176,105 @@ static String readVersion(ClassLoader loader) throws IOException { */ static Path resolve(String cliPathEnv, Path cacheBase, ClassLoader loader, String classifier, String version) throws IOException { - Path cliOverride = resolveFromCliPath(cliPathEnv); - if (cliOverride != null) { - return cliOverride; - } - return extractToCache(cacheBase, loader, classifier, version); + return resolve(cliPathEnv, cacheBase, loader, classifier, version, null, DEFAULT_PUBLISHER); } /** - * Checks whether a {@code runtime.node} file exists alongside the binary - * referred to by {@code cliPathStr}. + * Resolves the runtime binary path with an optional bundled-CLI directory. + * Package-private to allow injection of test doubles in unit tests. * - * @param cliPathStr - * value of the {@code COPILOT_CLI_PATH} environment variable - * @return path to the sibling {@code runtime.node} if it is a regular non-empty - * file, or {@code null} if the override does not apply + * @param cliPathEnv + * value of the {@code COPILOT_CLI_PATH} environment variable, or + * {@code null} + * @param cacheBase + * base directory for the extraction cache + * @param loader + * class loader used to locate classpath resources + * @param classifier + * platform classifier (e.g. {@code linux-x64}) + * @param version + * SDK version used as the cache key + * @param bundledCliDir + * directory where the bundled CLI binary and its sibling + * {@code runtime.node} reside (source 3), or {@code null} to skip + * @return path to the resolved {@code runtime.node} binary * @throws IOException - * if file-size probing fails + * if extraction or file I/O fails + * @throws IllegalStateException + * if required resources are missing or extraction fails */ - static Path resolveFromCliPath(String cliPathStr) throws IOException { - if (cliPathStr == null || cliPathStr.isBlank()) { - return null; + static Path resolve(String cliPathEnv, Path cacheBase, ClassLoader loader, String classifier, String version, + Path bundledCliDir) throws IOException { + return resolve(cliPathEnv, cacheBase, loader, classifier, version, bundledCliDir, DEFAULT_PUBLISHER); + } + + /** + * Full-control overload: injects all external dependencies. Package-private for + * unit tests. + * + * @param cliPathEnv + * value of the {@code COPILOT_CLI_PATH} environment variable, or + * {@code null} + * @param cacheBase + * base directory for the extraction cache + * @param loader + * class loader used to locate classpath resources + * @param classifier + * platform classifier (e.g. {@code linux-x64}) + * @param version + * SDK version used as the cache key + * @param bundledCliDir + * directory where the bundled CLI binary and its sibling + * {@code runtime.node} reside (source 3), or {@code null} to skip + * @param publisher + * atomic publish implementation + * @return path to the resolved {@code runtime.node} binary + * @throws IOException + * if extraction or file I/O fails + * @throws IllegalStateException + * if required resources are missing or extraction fails + */ + static Path resolve(String cliPathEnv, Path cacheBase, ClassLoader loader, String classifier, String version, + Path bundledCliDir, AtomicPublisher publisher) throws IOException { + // Source 1: COPILOT_CLI_PATH as explicit runtime override. + if (cliPathEnv != null && !cliPathEnv.isBlank()) { + return resolveFromExplicitPath(cliPathEnv); } - Path cliPath = Path.of(cliPathStr); - Path parent = cliPath.getParent(); - Path candidate = parent != null ? parent.resolve(RUNTIME_FILENAME) : Path.of(RUNTIME_FILENAME); - if (Files.isRegularFile(candidate) && Files.size(candidate) > 0) { - return candidate; + return resolveFromClasspathOrBundledCli(cacheBase, loader, classifier, version, bundledCliDir, publisher); + } + + /** + * Resolves an explicit {@code runtime.node} override from the path given by + * {@code COPILOT_CLI_PATH}. The path is treated as the direct location of the + * {@code runtime.node} binary itself — a non-blank value that does not refer to + * a regular, non-empty file is a configuration error; no silent fallback + * occurs. + * + * @param pathStr + * non-blank value of the {@code COPILOT_CLI_PATH} environment + * variable + * @return the validated path + * @throws IOException + * if file-attribute probing fails + * @throws IllegalStateException + * if the path does not refer to a regular, non-empty file + */ + static Path resolveFromExplicitPath(String pathStr) throws IOException { + Path path = Path.of(pathStr); + if (!isValidCachedFile(path)) { + throw new IllegalStateException(COPILOT_CLI_PATH_ENV + " is set to '" + pathStr + + "' but does not refer to a regular, non-empty file. " + "Set " + COPILOT_CLI_PATH_ENV + + " to the absolute path of the " + RUNTIME_FILENAME + + " binary, or unset it to use the classpath resource."); } - return null; + return path; } /** * Extracts the classpath resource {@code native//runtime.node} to * the versioned cache directory, using an atomic publish sequence to prevent - * readers from observing a partially-written file. + * readers from observing a partially-written file. Uses + * {@link #DEFAULT_PUBLISHER}. * * @param cacheBase * root cache directory (e.g. {@code ~/.copilot/runtime-cache}) @@ -164,6 +293,31 @@ static Path resolveFromCliPath(String cliPathStr) throws IOException { */ static Path extractToCache(Path cacheBase, ClassLoader loader, String classifier, String version) throws IOException { + return extractToCache(cacheBase, loader, classifier, version, DEFAULT_PUBLISHER); + } + + /** + * Extracts the classpath resource to the versioned cache directory with an + * injectable publisher. Package-private for unit tests. + * + * @param cacheBase + * root cache directory + * @param loader + * class loader used to open the classpath resource + * @param classifier + * platform classifier + * @param version + * SDK version used as the cache key + * @param publisher + * atomic publish implementation + * @return path to the extracted {@code runtime.node} binary + * @throws IOException + * if I/O or the atomic rename fails + * @throws IllegalStateException + * if the classpath resource is missing or empty + */ + static Path extractToCache(Path cacheBase, ClassLoader loader, String classifier, String version, + AtomicPublisher publisher) throws IOException { String resourcePath = "native/" + classifier + "/" + RUNTIME_FILENAME; Path cacheDir = cacheBase.resolve(version).resolve(classifier); Path cached = cacheDir.resolve(RUNTIME_FILENAME); @@ -187,7 +341,7 @@ static Path extractToCache(Path cacheBase, ClassLoader loader, String classifier Path temp = Files.createTempFile(cacheDir, "runtime-tmp-", ".node"); try { copyResourceToTemp(resource, resourcePath, temp); - publishAtomically(temp, cached); + publisher.publish(temp, cached); temp = null; // transfer ownership; do not delete in finally } finally { if (temp != null) { @@ -198,6 +352,31 @@ static Path extractToCache(Path cacheBase, ClassLoader loader, String classifier return cached; } + /** + * Tries source 2 (classpath extraction) first and falls back to source 3 + * (bundled-CLI sibling) only when the classpath resource is absent. + */ + private static Path resolveFromClasspathOrBundledCli(Path cacheBase, ClassLoader loader, String classifier, + String version, Path bundledCliDir, AtomicPublisher publisher) throws IOException { + // Source 2: classpath resource. + try { + return extractToCache(cacheBase, loader, classifier, version, publisher); + } catch (FileNotFoundException ex) { + // Source 3: runtime.node alongside the bundled CLI binary. + if (bundledCliDir != null) { + Path candidate = bundledCliDir.resolve(RUNTIME_FILENAME); + try { + if (isValidCachedFile(candidate)) { + return candidate; + } + } catch (IOException ignored) { + // fall through and rethrow the original classpath error + } + } + throw ex; + } + } + private static boolean isValidCachedFile(Path path) throws IOException { if (!Files.isRegularFile(path)) { return false; @@ -218,22 +397,6 @@ private static void copyResourceToTemp(URL resource, String resourcePath, Path t } } - private static void publishAtomically(Path temp, Path cached) throws IOException { - try { - Files.move(temp, cached, StandardCopyOption.ATOMIC_MOVE); - } catch (AtomicMoveNotSupportedException ex) { - throw new IllegalStateException( - "Filesystem does not support atomic moves; cannot safely publish runtime.node to " + cached, ex); - } catch (java.nio.file.FileAlreadyExistsException ex) { - // Another process won the race — accept the winner if it is a valid file. - if (isValidCachedFile(cached)) { - return; - } - throw new IllegalStateException( - "Concurrent extraction race: target already exists but is not a valid file: " + cached, ex); - } - } - private static void tryDelete(Path path) { try { Files.deleteIfExists(path); @@ -245,4 +408,20 @@ private static void tryDelete(Path path) { private static Path defaultCacheBase() { return Path.of(System.getProperty("user.home"), ".copilot", "runtime-cache"); } + + /** + * Returns the directory where the bundled CLI binary and its sibling + * {@code runtime.node} reside, or {@code null} if the bundled CLI has not been + * installed. + * + *

+ * This is a placeholder for a future embedded-CLI installation step. When the + * bundled CLI is implemented, this method will return the installation + * directory. + * + * @return installation directory, or {@code null} + */ + private static Path findBundledCliDirectory() { + return null; + } } diff --git a/java/sdk/src/test/java/com/github/copilot/ffi/NativeRuntimeLoaderTest.java b/java/sdk/src/test/java/com/github/copilot/ffi/NativeRuntimeLoaderTest.java index 0dcdc559ef..8d63525ead 100644 --- a/java/sdk/src/test/java/com/github/copilot/ffi/NativeRuntimeLoaderTest.java +++ b/java/sdk/src/test/java/com/github/copilot/ffi/NativeRuntimeLoaderTest.java @@ -5,14 +5,16 @@ package com.github.copilot.ffi; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.IOException; import java.net.URL; import java.net.URLClassLoader; +import java.nio.file.AtomicMoveNotSupportedException; import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; @@ -59,71 +61,70 @@ void readVersionThrowsWhenVersionPropertyIsBlank(@TempDir Path tempDir) throws E } // ------------------------------------------------------------------------- - // COPILOT_CLI_PATH override + // Source 1: COPILOT_CLI_PATH as explicit runtime override // ------------------------------------------------------------------------- @Test - void resolveFromCliPathReturnsSiblingWhenRuntimeNodeExists(@TempDir Path tempDir) throws Exception { - Path fakeCliPath = tempDir.resolve("copilot"); - Files.createFile(fakeCliPath); + void resolveFromExplicitPathReturnsPathWhenFileIsValid(@TempDir Path tempDir) throws Exception { Path runtimeNode = tempDir.resolve(NativeRuntimeLoader.RUNTIME_FILENAME); Files.write(runtimeNode, FAKE_BINARY_CONTENT); - Path result = NativeRuntimeLoader.resolveFromCliPath(fakeCliPath.toString()); + Path result = NativeRuntimeLoader.resolveFromExplicitPath(runtimeNode.toString()); assertEquals(runtimeNode, result); } @Test - void resolveFromCliPathReturnsNullWhenRuntimeNodeMissing(@TempDir Path tempDir) throws Exception { - Path fakeCliPath = tempDir.resolve("copilot"); - Files.createFile(fakeCliPath); + void resolveFromExplicitPathThrowsWhenFileDoesNotExist(@TempDir Path tempDir) { + Path missing = tempDir.resolve("nonexistent.node"); - assertNull(NativeRuntimeLoader.resolveFromCliPath(fakeCliPath.toString())); + IllegalStateException ex = assertThrows(IllegalStateException.class, + () -> NativeRuntimeLoader.resolveFromExplicitPath(missing.toString())); + assertTrue(ex.getMessage().contains(NativeRuntimeLoader.COPILOT_CLI_PATH_ENV), + "Error must mention the env variable name: " + ex.getMessage()); } @Test - void resolveFromCliPathReturnsNullWhenEnvIsNull() throws Exception { - assertNull(NativeRuntimeLoader.resolveFromCliPath(null)); - } + void resolveFromExplicitPathThrowsWhenFileIsEmpty(@TempDir Path tempDir) throws Exception { + Path empty = tempDir.resolve(NativeRuntimeLoader.RUNTIME_FILENAME); + Files.createFile(empty); // zero bytes - @Test - void resolveFromCliPathReturnsNullWhenEnvIsBlank() throws Exception { - assertNull(NativeRuntimeLoader.resolveFromCliPath(" ")); + IllegalStateException ex = assertThrows(IllegalStateException.class, + () -> NativeRuntimeLoader.resolveFromExplicitPath(empty.toString())); + assertTrue(ex.getMessage().contains(NativeRuntimeLoader.COPILOT_CLI_PATH_ENV), + "Error must mention the env variable name: " + ex.getMessage()); } @Test - void resolveFromCliPathReturnsNullWhenRuntimeNodeIsEmpty(@TempDir Path tempDir) throws Exception { - Path fakeCliPath = tempDir.resolve("copilot"); - Files.createFile(fakeCliPath); + void explicitOverrideTakesPriorityOverClasspathExtraction(@TempDir Path tempDir) throws Exception { + // Source 1: runtime.node directly specified via COPILOT_CLI_PATH Path runtimeNode = tempDir.resolve(NativeRuntimeLoader.RUNTIME_FILENAME); - Files.createFile(runtimeNode); // empty file - - assertNull(NativeRuntimeLoader.resolveFromCliPath(fakeCliPath.toString())); - } - - @Test - void cliPathOverrideTakesPriorityOverClasspathExtraction(@TempDir Path tempDir) throws Exception { - // Create a valid runtime.node alongside the fake CLI path - Path fakeCliDir = tempDir.resolve("cli-dir"); - Files.createDirectories(fakeCliDir); - Path fakeCliPath = fakeCliDir.resolve("copilot"); - Files.createFile(fakeCliPath); - Path runtimeNode = fakeCliDir.resolve(NativeRuntimeLoader.RUNTIME_FILENAME); Files.write(runtimeNode, FAKE_BINARY_CONTENT); - // Provide a classpath loader that also has the resource (should be ignored) + // Source 2 is also available (should be ignored) Path cacheBase = tempDir.resolve("cache"); ClassLoader loader = classLoaderWithRuntimeResource(tempDir, TEST_CLASSIFIER); - Path result = NativeRuntimeLoader.resolve(fakeCliPath.toString(), cacheBase, loader, TEST_CLASSIFIER, + Path result = NativeRuntimeLoader.resolve(runtimeNode.toString(), cacheBase, loader, TEST_CLASSIFIER, TEST_VERSION); - assertEquals(runtimeNode, result); + assertEquals(runtimeNode, result, "Source 1 (COPILOT_CLI_PATH) must take priority over classpath extraction"); + } + + @Test + void explicitOverrideThrowsImmediatelyWhenPathIsInvalid(@TempDir Path tempDir) throws Exception { + // Source 2 is available, but source 1 is invalid — must throw, not silently + // fall through + Path missing = tempDir.resolve("not-a-runtime.node"); + Path cacheBase = tempDir.resolve("cache"); + ClassLoader loader = classLoaderWithRuntimeResource(tempDir, TEST_CLASSIFIER); + + assertThrows(IllegalStateException.class, () -> NativeRuntimeLoader.resolve(missing.toString(), cacheBase, + loader, TEST_CLASSIFIER, TEST_VERSION)); } // ------------------------------------------------------------------------- - // Classpath extraction to cache + // Source 2: classpath extraction to cache // ------------------------------------------------------------------------- @Test @@ -181,8 +182,6 @@ void extractedBinaryContentsMatchClasspathResource(@TempDir Path tempDir) throws @Test void extractToCacheFiltersClasspathByClassifier(@TempDir Path tempDir) throws Exception { - // Put resources for two classifiers; extraction must target only the requested - // one Path cacheBase = tempDir.resolve("cache"); ClassLoader loader = classLoaderWithRuntimeResource(tempDir, TEST_CLASSIFIER); @@ -191,6 +190,122 @@ void extractToCacheFiltersClasspathByClassifier(@TempDir Path tempDir) throws Ex assertTrue(result.toString().contains(TEST_CLASSIFIER), "Cache path must include the classifier: " + result); } + // ------------------------------------------------------------------------- + // Source 3: bundled-CLI sibling + // ------------------------------------------------------------------------- + + @Test + void bundledCliSiblingIsUsedWhenClasspathResourceAbsent(@TempDir Path tempDir) throws Exception { + Path bundledCliDir = tempDir.resolve("bundled-cli"); + Files.createDirectories(bundledCliDir); + Path runtimeNode = bundledCliDir.resolve(NativeRuntimeLoader.RUNTIME_FILENAME); + Files.write(runtimeNode, FAKE_BINARY_CONTENT); + + Path cacheBase = tempDir.resolve("cache"); + ClassLoader emptyLoader = new URLClassLoader(new URL[0], null); // no classpath resource + + Path result = NativeRuntimeLoader.resolve(null, cacheBase, emptyLoader, TEST_CLASSIFIER, TEST_VERSION, + bundledCliDir); + + assertEquals(runtimeNode, result, + "Source 3 (bundled-CLI sibling) must be used when classpath resource is absent"); + } + + @Test + void classpathResourceWinsOverBundledCliSibling(@TempDir Path tempDir) throws Exception { + // Source 3: bundled CLI dir with runtime.node (should NOT win) + Path bundledCliDir = tempDir.resolve("bundled-cli"); + Files.createDirectories(bundledCliDir); + Files.write(bundledCliDir.resolve(NativeRuntimeLoader.RUNTIME_FILENAME), "bundled".getBytes()); + + // Source 2: classpath resource (should win) + Path cacheBase = tempDir.resolve("cache"); + ClassLoader loader = classLoaderWithRuntimeResource(tempDir, TEST_CLASSIFIER); + + Path result = NativeRuntimeLoader.resolve(null, cacheBase, loader, TEST_CLASSIFIER, TEST_VERSION, + bundledCliDir); + + Path expectedFromClasspath = cacheBase.resolve(TEST_VERSION).resolve(TEST_CLASSIFIER) + .resolve(NativeRuntimeLoader.RUNTIME_FILENAME); + assertEquals(expectedFromClasspath, result, + "Source 2 (classpath) must win over source 3 (bundled-CLI sibling)"); + assertNotEquals(bundledCliDir.resolve(NativeRuntimeLoader.RUNTIME_FILENAME), result); + } + + @Test + void bundledCliSiblingIsIgnoredWhenRuntimeNodeMissing(@TempDir Path tempDir) { + Path bundledCliDir = tempDir.resolve("bundled-cli-no-runtime"); + // bundledCliDir doesn't even exist — no runtime.node present + + Path cacheBase = tempDir.resolve("cache"); + ClassLoader emptyLoader = new URLClassLoader(new URL[0], null); + + // Both source 2 and source 3 absent: must throw (the classpath error) + IOException ex = assertThrows(IOException.class, () -> NativeRuntimeLoader.resolve(null, cacheBase, emptyLoader, + TEST_CLASSIFIER, TEST_VERSION, bundledCliDir)); + assertTrue(ex.getMessage().contains("classpath"), "Error should mention classpath: " + ex.getMessage()); + } + + // ------------------------------------------------------------------------- + // Atomic publication test seam + // ------------------------------------------------------------------------- + + @Test + void defaultPublisherMovesSourceToTarget(@TempDir Path tempDir) throws Exception { + Path temp = Files.createTempFile(tempDir, "runtime-tmp-", ".node"); + Files.write(temp, FAKE_BINARY_CONTENT); + Path target = tempDir.resolve(NativeRuntimeLoader.RUNTIME_FILENAME); + + NativeRuntimeLoader.DEFAULT_PUBLISHER.publish(temp, target); + + assertTrue(Files.isRegularFile(target), "Target must exist after publication"); + assertTrue(Files.size(target) > 0, "Target must be non-empty"); + assertFalse(Files.exists(temp), "Source temp file must be absent after atomic move"); + } + + @Test + void extractionCleansUpTempFileWhenPublicationFails(@TempDir Path tempDir) throws Exception { + Path cacheBase = tempDir.resolve("cache"); + ClassLoader loader = classLoaderWithRuntimeResource(tempDir, TEST_CLASSIFIER); + + // Capture the temp path so we can verify it was deleted + Path[] capturedTemp = {null}; + NativeRuntimeLoader.AtomicPublisher failingPublisher = (temp, cached) -> { + capturedTemp[0] = temp; + throw new AtomicMoveNotSupportedException(temp.toString(), cached.toString(), + "filesystem does not support atomic moves — test"); + }; + + assertThrows(AtomicMoveNotSupportedException.class, () -> NativeRuntimeLoader.extractToCache(cacheBase, loader, + TEST_CLASSIFIER, TEST_VERSION, failingPublisher)); + + assertNotNull(capturedTemp[0], "Publisher must have been invoked"); + assertFalse(Files.exists(capturedTemp[0]), "Temp file must be deleted after failed publication"); + } + + @Test + void extractionCleansUpTempFileWhenPublisherThrowsIllegalStateException(@TempDir Path tempDir) throws Exception { + Path cacheBase = tempDir.resolve("cache"); + ClassLoader loader = classLoaderWithRuntimeResource(tempDir, TEST_CLASSIFIER); + + Path[] capturedTemp = {null}; + NativeRuntimeLoader.AtomicPublisher unsupportedPublisher = (temp, cached) -> { + capturedTemp[0] = temp; + // Simulate the wrapping that DEFAULT_PUBLISHER performs for + // AtomicMoveNotSupportedException + throw new IllegalStateException("Filesystem does not support atomic moves; cannot safely publish " + + NativeRuntimeLoader.RUNTIME_FILENAME + " to " + cached); + }; + + IllegalStateException ex = assertThrows(IllegalStateException.class, () -> NativeRuntimeLoader + .extractToCache(cacheBase, loader, TEST_CLASSIFIER, TEST_VERSION, unsupportedPublisher)); + + assertTrue(ex.getMessage().contains("atomic moves"), + "Error message should describe the atomic-move failure: " + ex.getMessage()); + assertNotNull(capturedTemp[0], "Publisher must have been invoked"); + assertFalse(Files.exists(capturedTemp[0]), "Temp file must be deleted after failed atomic publication"); + } + // ------------------------------------------------------------------------- // Concurrent extraction safety // ------------------------------------------------------------------------- @@ -226,7 +341,7 @@ void concurrentExtractionByMultipleThreadsBothSucceed(@TempDir Path tempDir) thr } // ------------------------------------------------------------------------- - // resolve() -- full resolution chain + // resolve() -- full three-source resolution chain // ------------------------------------------------------------------------- @Test @@ -242,10 +357,11 @@ void resolveWithNullCliEnvExtractsFromClasspath(@TempDir Path tempDir) throws Ex } @Test - void resolveThrowsWhenNoClasspathResourceAndNoCliOverride(@TempDir Path tempDir) { + void resolveThrowsWhenNoSourceIsAvailable(@TempDir Path tempDir) { Path cacheBase = tempDir.resolve("cache"); ClassLoader emptyLoader = new URLClassLoader(new URL[0], null); + // No CLI env, no classpath resource, no bundled-CLI dir → throw assertThrows(IOException.class, () -> NativeRuntimeLoader.resolve(null, cacheBase, emptyLoader, TEST_CLASSIFIER, TEST_VERSION)); } From d262f116d80a27e6d8e72f24c329fb3214090f11 Mon Sep 17 00:00:00 2001 From: Ed Burns Date: Thu, 30 Jul 2026 22:29:05 +0000 Subject: [PATCH 4/5] fix(java): harden native runtime resolution Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../copilot/ffi/NativeRuntimeLoader.java | 125 +++++++++++------- .../copilot/ffi/NativeRuntimeLoaderTest.java | 76 ++++++++++- 2 files changed, 153 insertions(+), 48 deletions(-) diff --git a/java/sdk/src/main/java/com/github/copilot/ffi/NativeRuntimeLoader.java b/java/sdk/src/main/java/com/github/copilot/ffi/NativeRuntimeLoader.java index 8b5ce3d90b..021fa7b3a5 100644 --- a/java/sdk/src/main/java/com/github/copilot/ffi/NativeRuntimeLoader.java +++ b/java/sdk/src/main/java/com/github/copilot/ffi/NativeRuntimeLoader.java @@ -10,6 +10,7 @@ import java.net.URL; import java.nio.channels.FileChannel; import java.nio.file.AtomicMoveNotSupportedException; +import java.nio.file.FileAlreadyExistsException; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardCopyOption; @@ -31,9 +32,8 @@ *

  • Classpath resource * {@code native//runtime.node} — extracted atomically to * {@code ~/.copilot/runtime-cache///runtime.node}.
    • - *
  • Bundled-CLI sibling — {@code runtime.node} in the - * directory where the in-process runtime was installed alongside the bundled - * CLI binary. Used only when the classpath resource is absent.
    • + *
  • {@code runtime.node} alongside the bundled {@code copilot} + * executable.
    • * */ public final class NativeRuntimeLoader { @@ -106,22 +106,16 @@ private NativeRuntimeLoader() { */ public static Path resolve() throws IOException { String cliPathEnv = System.getenv(COPILOT_CLI_PATH_ENV); - - // Source 1: COPILOT_CLI_PATH as explicit runtime override. - // Checked before any classpath or platform work so the override is usable - // even when those resources are unavailable. - if (cliPathEnv != null && !cliPathEnv.isBlank()) { - return resolveFromExplicitPath(cliPathEnv); + Path cliOverride = resolveFromCliPath(cliPathEnv); + if (cliOverride != null) { + return cliOverride; } ClassLoader loader = NativeRuntimeLoader.class.getClassLoader(); String classifier = PlatformDetector.detectClassifier(); String version = readVersion(loader); Path cacheBase = defaultCacheBase(); - Path bundledCliDir = findBundledCliDirectory(); - - return resolveFromClasspathOrBundledCli(cacheBase, loader, classifier, version, bundledCliDir, - DEFAULT_PUBLISHER); + return resolve(null, findCliOnPath(), cacheBase, loader, classifier, version); } /** @@ -176,7 +170,25 @@ static String readVersion(ClassLoader loader) throws IOException { */ static Path resolve(String cliPathEnv, Path cacheBase, ClassLoader loader, String classifier, String version) throws IOException { - return resolve(cliPathEnv, cacheBase, loader, classifier, version, null, DEFAULT_PUBLISHER); + return resolve(cliPathEnv, null, cacheBase, loader, classifier, version); + } + + static Path resolve(String cliPathEnv, String bundledCliPath, Path cacheBase, ClassLoader loader, String classifier, + String version) throws IOException { + Path cliOverride = resolveFromCliPath(cliPathEnv); + if (cliOverride != null) { + return cliOverride; + } + + try { + return extractToCache(cacheBase, loader, classifier, version); + } catch (FileNotFoundException ex) { + Path bundledRuntime = resolveFromCliPath(bundledCliPath); + if (bundledRuntime != null) { + return bundledRuntime; + } + throw ex; + } } /** @@ -240,32 +252,11 @@ static Path resolve(String cliPathEnv, Path cacheBase, ClassLoader loader, Strin if (cliPathEnv != null && !cliPathEnv.isBlank()) { return resolveFromExplicitPath(cliPathEnv); } - return resolveFromClasspathOrBundledCli(cacheBase, loader, classifier, version, bundledCliDir, publisher); - } - - /** - * Resolves an explicit {@code runtime.node} override from the path given by - * {@code COPILOT_CLI_PATH}. The path is treated as the direct location of the - * {@code runtime.node} binary itself — a non-blank value that does not refer to - * a regular, non-empty file is a configuration error; no silent fallback - * occurs. - * - * @param pathStr - * non-blank value of the {@code COPILOT_CLI_PATH} environment - * variable - * @return the validated path - * @throws IOException - * if file-attribute probing fails - * @throws IllegalStateException - * if the path does not refer to a regular, non-empty file - */ - static Path resolveFromExplicitPath(String pathStr) throws IOException { - Path path = Path.of(pathStr); - if (!isValidCachedFile(path)) { - throw new IllegalStateException(COPILOT_CLI_PATH_ENV + " is set to '" + pathStr - + "' but does not refer to a regular, non-empty file. " + "Set " + COPILOT_CLI_PATH_ENV - + " to the absolute path of the " + RUNTIME_FILENAME - + " binary, or unset it to use the classpath resource."); + Path cliPath = Path.of(cliPathStr).toAbsolutePath().normalize(); + Path parent = cliPath.getParent(); + Path candidate = parent.resolve(RUNTIME_FILENAME); + if (Files.isRegularFile(candidate) && Files.size(candidate) > 0) { + return candidate; } return path; } @@ -341,12 +332,9 @@ static Path extractToCache(Path cacheBase, ClassLoader loader, String classifier Path temp = Files.createTempFile(cacheDir, "runtime-tmp-", ".node"); try { copyResourceToTemp(resource, resourcePath, temp); - publisher.publish(temp, cached); - temp = null; // transfer ownership; do not delete in finally + publishAtomically(temp, cached); } finally { - if (temp != null) { - tryDelete(temp); - } + tryDelete(temp); } return cached; @@ -397,6 +385,53 @@ private static void copyResourceToTemp(URL resource, String resourcePath, Path t } } + private static void publishAtomically(Path temp, Path cached) throws IOException { + try { + Files.move(temp, cached, StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING); + } catch (AtomicMoveNotSupportedException ex) { + throw new IllegalStateException( + "Filesystem does not support atomic moves; cannot safely publish runtime.node to " + cached, ex); + } catch (FileAlreadyExistsException ex) { + // Another process won the race — accept the winner if it is a valid file. + if (isValidCachedFile(cached)) { + return; + } + throw new IllegalStateException( + "Concurrent extraction race: target already exists but is not a valid file: " + cached, ex); + } + } + + private static String findCliOnPath() { + String pathValue = System.getenv("PATH"); + if (pathValue == null || pathValue.isBlank()) { + return null; + } + + String[] executableNames = isWindows() + ? new String[]{"copilot.exe", "copilot.cmd", "copilot.bat", "copilot"} + : new String[]{"copilot"}; + for (String directory : pathValue.split(java.io.File.pathSeparator)) { + if (directory.isBlank()) { + continue; + } + for (String executableName : executableNames) { + Path candidate = Path.of(directory, executableName); + if (Files.isRegularFile(candidate)) { + try { + return candidate.toRealPath().toString(); + } catch (IOException ignored) { + return candidate.toAbsolutePath().normalize().toString(); + } + } + } + } + return null; + } + + private static boolean isWindows() { + return System.getProperty("os.name", "").toLowerCase(java.util.Locale.ROOT).contains("win"); + } + private static void tryDelete(Path path) { try { Files.deleteIfExists(path); diff --git a/java/sdk/src/test/java/com/github/copilot/ffi/NativeRuntimeLoaderTest.java b/java/sdk/src/test/java/com/github/copilot/ffi/NativeRuntimeLoaderTest.java index 8d63525ead..ca0f1213c3 100644 --- a/java/sdk/src/test/java/com/github/copilot/ffi/NativeRuntimeLoaderTest.java +++ b/java/sdk/src/test/java/com/github/copilot/ffi/NativeRuntimeLoaderTest.java @@ -31,8 +31,10 @@ class NativeRuntimeLoaderTest { private static final String TEST_CLASSIFIER = "linux-x64"; + private static final String OTHER_CLASSIFIER = "darwin-arm64"; private static final String TEST_VERSION = "1.2.3-test"; private static final byte[] FAKE_BINARY_CONTENT = "fake runtime.node binary content".getBytes(); + private static final byte[] OTHER_BINARY_CONTENT = "other runtime.node binary content".getBytes(); // ------------------------------------------------------------------------- // Version properties resource reading @@ -99,6 +101,34 @@ void resolveFromExplicitPathThrowsWhenFileIsEmpty(@TempDir Path tempDir) throws void explicitOverrideTakesPriorityOverClasspathExtraction(@TempDir Path tempDir) throws Exception { // Source 1: runtime.node directly specified via COPILOT_CLI_PATH Path runtimeNode = tempDir.resolve(NativeRuntimeLoader.RUNTIME_FILENAME); + Files.createFile(runtimeNode); // empty file + + assertNull(NativeRuntimeLoader.resolveFromCliPath(fakeCliPath.toString())); + } + + @Test + void resolveFromCliPathReturnsAbsolutePathForRelativeCliPath(@TempDir Path tempDir) throws Exception { + Path workingDirectory = Path.of("").toAbsolutePath(); + Path fakeCliDir = tempDir.resolve("cli-dir"); + Files.createDirectories(fakeCliDir); + Path fakeCliPath = fakeCliDir.resolve("copilot"); + Files.createFile(fakeCliPath); + Path runtimeNode = fakeCliDir.resolve(NativeRuntimeLoader.RUNTIME_FILENAME); + Files.write(runtimeNode, FAKE_BINARY_CONTENT); + + Path relativeCliPath = workingDirectory.relativize(fakeCliPath); + + assertEquals(runtimeNode, NativeRuntimeLoader.resolveFromCliPath(relativeCliPath.toString())); + } + + @Test + void cliPathOverrideTakesPriorityOverClasspathExtraction(@TempDir Path tempDir) throws Exception { + // Create a valid runtime.node alongside the fake CLI path + Path fakeCliDir = tempDir.resolve("cli-dir"); + Files.createDirectories(fakeCliDir); + Path fakeCliPath = fakeCliDir.resolve("copilot"); + Files.createFile(fakeCliPath); + Path runtimeNode = fakeCliDir.resolve(NativeRuntimeLoader.RUNTIME_FILENAME); Files.write(runtimeNode, FAKE_BINARY_CONTENT); // Source 2 is also available (should be ignored) @@ -183,11 +213,29 @@ void extractedBinaryContentsMatchClasspathResource(@TempDir Path tempDir) throws @Test void extractToCacheFiltersClasspathByClassifier(@TempDir Path tempDir) throws Exception { Path cacheBase = tempDir.resolve("cache"); - ClassLoader loader = classLoaderWithRuntimeResource(tempDir, TEST_CLASSIFIER); + writeRuntimeResource(tempDir, TEST_CLASSIFIER, FAKE_BINARY_CONTENT); + writeRuntimeResource(tempDir, OTHER_CLASSIFIER, OTHER_BINARY_CONTENT); + ClassLoader loader = new URLClassLoader(new URL[]{tempDir.toUri().toURL()}, null); Path result = NativeRuntimeLoader.extractToCache(cacheBase, loader, TEST_CLASSIFIER, TEST_VERSION); assertTrue(result.toString().contains(TEST_CLASSIFIER), "Cache path must include the classifier: " + result); + assertBytesEqual(FAKE_BINARY_CONTENT, Files.readAllBytes(result)); + } + + @Test + void extractToCacheRepairsInvalidCacheEntry(@TempDir Path tempDir) throws Exception { + Path cacheBase = tempDir.resolve("cache"); + Path cached = cacheBase.resolve(TEST_VERSION).resolve(TEST_CLASSIFIER) + .resolve(NativeRuntimeLoader.RUNTIME_FILENAME); + Files.createDirectories(cached.getParent()); + Files.createFile(cached); + ClassLoader loader = classLoaderWithRuntimeResource(tempDir, TEST_CLASSIFIER); + + Path result = NativeRuntimeLoader.extractToCache(cacheBase, loader, TEST_CLASSIFIER, TEST_VERSION); + + assertEquals(cached, result); + assertBytesEqual(FAKE_BINARY_CONTENT, Files.readAllBytes(result)); } // ------------------------------------------------------------------------- @@ -338,6 +386,9 @@ void concurrentExtractionByMultipleThreadsBothSucceed(@TempDir Path tempDir) thr assertTrue(Files.isRegularFile(result)); assertTrue(Files.size(result) > 0); } + try (var files = Files.list(expected.getParent())) { + assertEquals(List.of(expected), files.toList(), "Concurrent extraction must clean up temporary files"); + } } // ------------------------------------------------------------------------- @@ -366,6 +417,21 @@ void resolveThrowsWhenNoSourceIsAvailable(@TempDir Path tempDir) { () -> NativeRuntimeLoader.resolve(null, cacheBase, emptyLoader, TEST_CLASSIFIER, TEST_VERSION)); } + @Test + void resolveFallsBackToRuntimeAlongsideBundledCli(@TempDir Path tempDir) throws Exception { + Path cacheBase = tempDir.resolve("cache"); + ClassLoader emptyLoader = new URLClassLoader(new URL[0], null); + Path bundledCli = tempDir.resolve("copilot"); + Files.createFile(bundledCli); + Path runtimeNode = tempDir.resolve(NativeRuntimeLoader.RUNTIME_FILENAME); + Files.write(runtimeNode, FAKE_BINARY_CONTENT); + + Path result = NativeRuntimeLoader.resolve(null, bundledCli.toString(), cacheBase, emptyLoader, TEST_CLASSIFIER, + TEST_VERSION); + + assertEquals(runtimeNode, result); + } + // ------------------------------------------------------------------------- // Helpers // ------------------------------------------------------------------------- @@ -377,10 +443,14 @@ private static ClassLoader classLoaderWithVersionResource(Path tempDir, String v } private static ClassLoader classLoaderWithRuntimeResource(Path tempDir, String classifier) throws IOException { + writeRuntimeResource(tempDir, classifier, FAKE_BINARY_CONTENT); + return new URLClassLoader(new URL[]{tempDir.toUri().toURL()}, null); + } + + private static void writeRuntimeResource(Path tempDir, String classifier, byte[] content) throws IOException { Path resourceDir = tempDir.resolve("native").resolve(classifier); Files.createDirectories(resourceDir); - Files.write(resourceDir.resolve(NativeRuntimeLoader.RUNTIME_FILENAME), FAKE_BINARY_CONTENT); - return new URLClassLoader(new URL[]{tempDir.toUri().toURL()}, null); + Files.write(resourceDir.resolve(NativeRuntimeLoader.RUNTIME_FILENAME), content); } private static void assertBytesEqual(byte[] expected, byte[] actual) { From d779d146b268f8efdf8bfcd65327b5f3fdca942c Mon Sep 17 00:00:00 2001 From: Ed Burns Date: Thu, 30 Jul 2026 22:31:39 +0000 Subject: [PATCH 5/5] fix(java): reconcile native loader review fixes Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../copilot/ffi/NativeRuntimeLoader.java | 152 +++--------------- .../copilot/ffi/NativeRuntimeLoaderTest.java | 51 +++--- 2 files changed, 45 insertions(+), 158 deletions(-) diff --git a/java/sdk/src/main/java/com/github/copilot/ffi/NativeRuntimeLoader.java b/java/sdk/src/main/java/com/github/copilot/ffi/NativeRuntimeLoader.java index 021fa7b3a5..ebf43e7c32 100644 --- a/java/sdk/src/main/java/com/github/copilot/ffi/NativeRuntimeLoader.java +++ b/java/sdk/src/main/java/com/github/copilot/ffi/NativeRuntimeLoader.java @@ -24,11 +24,9 @@ *

    * Resolution order: *

      - *
    1. {@code COPILOT_CLI_PATH} — explicit path to the - * {@code runtime.node} binary. Checked before any classpath or platform work so - * the override is usable even when those resources are unavailable. A non-blank - * value that does not refer to a regular, non-empty file is a configuration - * error; no silent fallback occurs.
      2. + *
    2. {@code COPILOT_CLI_PATH} — checks for + * {@code runtime.node} alongside the configured CLI before any classpath or + * platform work.
      3. *
    3. Classpath resource * {@code native//runtime.node} — extracted atomically to * {@code ~/.copilot/runtime-cache///runtime.node}.
      4. @@ -69,11 +67,11 @@ interface AtomicPublisher { */ static final AtomicPublisher DEFAULT_PUBLISHER = (temp, cached) -> { try { - Files.move(temp, cached, StandardCopyOption.ATOMIC_MOVE); + Files.move(temp, cached, StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING); } catch (AtomicMoveNotSupportedException ex) { throw new IllegalStateException("Filesystem does not support atomic moves; cannot safely publish " + RUNTIME_FILENAME + " to " + cached, ex); - } catch (java.nio.file.FileAlreadyExistsException ex) { + } catch (FileAlreadyExistsException ex) { // Another process won the race — accept the winner if it is a valid file. try { if (isValidCachedFile(cached)) { @@ -148,109 +146,40 @@ static String readVersion(ClassLoader loader) throws IOException { /** * Resolves the runtime binary path using the given parameters. Package-private - * to allow injection of test doubles in unit tests. Uses - * {@link #DEFAULT_PUBLISHER} and no bundled-CLI directory. - * - * @param cliPathEnv - * value of the {@code COPILOT_CLI_PATH} environment variable, or - * {@code null} - * @param cacheBase - * base directory for the extraction cache - * @param loader - * class loader used to locate classpath resources - * @param classifier - * platform classifier (e.g. {@code linux-x64}) - * @param version - * SDK version used as the cache key - * @return path to the resolved {@code runtime.node} binary - * @throws IOException - * if extraction or file I/O fails - * @throws IllegalStateException - * if required resources are missing or extraction fails + * to allow injection of test doubles in unit tests. */ static Path resolve(String cliPathEnv, Path cacheBase, ClassLoader loader, String classifier, String version) throws IOException { - return resolve(cliPathEnv, null, cacheBase, loader, classifier, version); + return resolve(cliPathEnv, cacheBase, loader, classifier, version, null, DEFAULT_PUBLISHER); } static Path resolve(String cliPathEnv, String bundledCliPath, Path cacheBase, ClassLoader loader, String classifier, String version) throws IOException { - Path cliOverride = resolveFromCliPath(cliPathEnv); - if (cliOverride != null) { - return cliOverride; - } - - try { - return extractToCache(cacheBase, loader, classifier, version); - } catch (FileNotFoundException ex) { - Path bundledRuntime = resolveFromCliPath(bundledCliPath); - if (bundledRuntime != null) { - return bundledRuntime; - } - throw ex; - } + Path bundledCliDir = bundledCliPath == null ? null : Path.of(bundledCliPath).toAbsolutePath().getParent(); + return resolve(cliPathEnv, cacheBase, loader, classifier, version, bundledCliDir, DEFAULT_PUBLISHER); } - /** - * Resolves the runtime binary path with an optional bundled-CLI directory. - * Package-private to allow injection of test doubles in unit tests. - * - * @param cliPathEnv - * value of the {@code COPILOT_CLI_PATH} environment variable, or - * {@code null} - * @param cacheBase - * base directory for the extraction cache - * @param loader - * class loader used to locate classpath resources - * @param classifier - * platform classifier (e.g. {@code linux-x64}) - * @param version - * SDK version used as the cache key - * @param bundledCliDir - * directory where the bundled CLI binary and its sibling - * {@code runtime.node} reside (source 3), or {@code null} to skip - * @return path to the resolved {@code runtime.node} binary - * @throws IOException - * if extraction or file I/O fails - * @throws IllegalStateException - * if required resources are missing or extraction fails - */ static Path resolve(String cliPathEnv, Path cacheBase, ClassLoader loader, String classifier, String version, Path bundledCliDir) throws IOException { return resolve(cliPathEnv, cacheBase, loader, classifier, version, bundledCliDir, DEFAULT_PUBLISHER); } - /** - * Full-control overload: injects all external dependencies. Package-private for - * unit tests. - * - * @param cliPathEnv - * value of the {@code COPILOT_CLI_PATH} environment variable, or - * {@code null} - * @param cacheBase - * base directory for the extraction cache - * @param loader - * class loader used to locate classpath resources - * @param classifier - * platform classifier (e.g. {@code linux-x64}) - * @param version - * SDK version used as the cache key - * @param bundledCliDir - * directory where the bundled CLI binary and its sibling - * {@code runtime.node} reside (source 3), or {@code null} to skip - * @param publisher - * atomic publish implementation - * @return path to the resolved {@code runtime.node} binary - * @throws IOException - * if extraction or file I/O fails - * @throws IllegalStateException - * if required resources are missing or extraction fails - */ static Path resolve(String cliPathEnv, Path cacheBase, ClassLoader loader, String classifier, String version, Path bundledCliDir, AtomicPublisher publisher) throws IOException { - // Source 1: COPILOT_CLI_PATH as explicit runtime override. - if (cliPathEnv != null && !cliPathEnv.isBlank()) { - return resolveFromExplicitPath(cliPathEnv); + Path cliOverride = resolveFromCliPath(cliPathEnv); + if (cliOverride != null) { + return cliOverride; + } + + return resolveFromClasspathOrBundledCli(cacheBase, loader, classifier, version, bundledCliDir, publisher); + } + + /** + * Checks for {@code runtime.node} alongside the configured CLI. + */ + static Path resolveFromCliPath(String cliPathStr) throws IOException { + if (cliPathStr == null || cliPathStr.isBlank()) { + return null; } Path cliPath = Path.of(cliPathStr).toAbsolutePath().normalize(); Path parent = cliPath.getParent(); @@ -258,7 +187,7 @@ static Path resolve(String cliPathEnv, Path cacheBase, ClassLoader loader, Strin if (Files.isRegularFile(candidate) && Files.size(candidate) > 0) { return candidate; } - return path; + return null; } /** @@ -332,7 +261,7 @@ static Path extractToCache(Path cacheBase, ClassLoader loader, String classifier Path temp = Files.createTempFile(cacheDir, "runtime-tmp-", ".node"); try { copyResourceToTemp(resource, resourcePath, temp); - publishAtomically(temp, cached); + publisher.publish(temp, cached); } finally { tryDelete(temp); } @@ -385,22 +314,6 @@ private static void copyResourceToTemp(URL resource, String resourcePath, Path t } } - private static void publishAtomically(Path temp, Path cached) throws IOException { - try { - Files.move(temp, cached, StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING); - } catch (AtomicMoveNotSupportedException ex) { - throw new IllegalStateException( - "Filesystem does not support atomic moves; cannot safely publish runtime.node to " + cached, ex); - } catch (FileAlreadyExistsException ex) { - // Another process won the race — accept the winner if it is a valid file. - if (isValidCachedFile(cached)) { - return; - } - throw new IllegalStateException( - "Concurrent extraction race: target already exists but is not a valid file: " + cached, ex); - } - } - private static String findCliOnPath() { String pathValue = System.getenv("PATH"); if (pathValue == null || pathValue.isBlank()) { @@ -444,19 +357,4 @@ private static Path defaultCacheBase() { return Path.of(System.getProperty("user.home"), ".copilot", "runtime-cache"); } - /** - * Returns the directory where the bundled CLI binary and its sibling - * {@code runtime.node} reside, or {@code null} if the bundled CLI has not been - * installed. - * - *

      - * This is a placeholder for a future embedded-CLI installation step. When the - * bundled CLI is implemented, this method will return the installation - * directory. - * - * @return installation directory, or {@code null} - */ - private static Path findBundledCliDirectory() { - return null; - } } diff --git a/java/sdk/src/test/java/com/github/copilot/ffi/NativeRuntimeLoaderTest.java b/java/sdk/src/test/java/com/github/copilot/ffi/NativeRuntimeLoaderTest.java index ca0f1213c3..e02009c94b 100644 --- a/java/sdk/src/test/java/com/github/copilot/ffi/NativeRuntimeLoaderTest.java +++ b/java/sdk/src/test/java/com/github/copilot/ffi/NativeRuntimeLoaderTest.java @@ -8,6 +8,7 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -63,43 +64,43 @@ void readVersionThrowsWhenVersionPropertyIsBlank(@TempDir Path tempDir) throws E } // ------------------------------------------------------------------------- - // Source 1: COPILOT_CLI_PATH as explicit runtime override + // COPILOT_CLI_PATH override // ------------------------------------------------------------------------- @Test - void resolveFromExplicitPathReturnsPathWhenFileIsValid(@TempDir Path tempDir) throws Exception { + void resolveFromCliPathReturnsSiblingWhenRuntimeNodeExists(@TempDir Path tempDir) throws Exception { + Path fakeCliPath = tempDir.resolve("copilot"); + Files.createFile(fakeCliPath); Path runtimeNode = tempDir.resolve(NativeRuntimeLoader.RUNTIME_FILENAME); Files.write(runtimeNode, FAKE_BINARY_CONTENT); - Path result = NativeRuntimeLoader.resolveFromExplicitPath(runtimeNode.toString()); + Path result = NativeRuntimeLoader.resolveFromCliPath(fakeCliPath.toString()); assertEquals(runtimeNode, result); } @Test - void resolveFromExplicitPathThrowsWhenFileDoesNotExist(@TempDir Path tempDir) { - Path missing = tempDir.resolve("nonexistent.node"); + void resolveFromCliPathReturnsNullWhenRuntimeNodeMissing(@TempDir Path tempDir) throws Exception { + Path fakeCliPath = tempDir.resolve("copilot"); + Files.createFile(fakeCliPath); - IllegalStateException ex = assertThrows(IllegalStateException.class, - () -> NativeRuntimeLoader.resolveFromExplicitPath(missing.toString())); - assertTrue(ex.getMessage().contains(NativeRuntimeLoader.COPILOT_CLI_PATH_ENV), - "Error must mention the env variable name: " + ex.getMessage()); + assertNull(NativeRuntimeLoader.resolveFromCliPath(fakeCliPath.toString())); } @Test - void resolveFromExplicitPathThrowsWhenFileIsEmpty(@TempDir Path tempDir) throws Exception { - Path empty = tempDir.resolve(NativeRuntimeLoader.RUNTIME_FILENAME); - Files.createFile(empty); // zero bytes + void resolveFromCliPathReturnsNullWhenEnvIsNull() throws Exception { + assertNull(NativeRuntimeLoader.resolveFromCliPath(null)); + } - IllegalStateException ex = assertThrows(IllegalStateException.class, - () -> NativeRuntimeLoader.resolveFromExplicitPath(empty.toString())); - assertTrue(ex.getMessage().contains(NativeRuntimeLoader.COPILOT_CLI_PATH_ENV), - "Error must mention the env variable name: " + ex.getMessage()); + @Test + void resolveFromCliPathReturnsNullWhenEnvIsBlank() throws Exception { + assertNull(NativeRuntimeLoader.resolveFromCliPath(" ")); } @Test - void explicitOverrideTakesPriorityOverClasspathExtraction(@TempDir Path tempDir) throws Exception { - // Source 1: runtime.node directly specified via COPILOT_CLI_PATH + void resolveFromCliPathReturnsNullWhenRuntimeNodeIsEmpty(@TempDir Path tempDir) throws Exception { + Path fakeCliPath = tempDir.resolve("copilot"); + Files.createFile(fakeCliPath); Path runtimeNode = tempDir.resolve(NativeRuntimeLoader.RUNTIME_FILENAME); Files.createFile(runtimeNode); // empty file @@ -135,24 +136,12 @@ void cliPathOverrideTakesPriorityOverClasspathExtraction(@TempDir Path tempDir) Path cacheBase = tempDir.resolve("cache"); ClassLoader loader = classLoaderWithRuntimeResource(tempDir, TEST_CLASSIFIER); - Path result = NativeRuntimeLoader.resolve(runtimeNode.toString(), cacheBase, loader, TEST_CLASSIFIER, + Path result = NativeRuntimeLoader.resolve(fakeCliPath.toString(), cacheBase, loader, TEST_CLASSIFIER, TEST_VERSION); assertEquals(runtimeNode, result, "Source 1 (COPILOT_CLI_PATH) must take priority over classpath extraction"); } - @Test - void explicitOverrideThrowsImmediatelyWhenPathIsInvalid(@TempDir Path tempDir) throws Exception { - // Source 2 is available, but source 1 is invalid — must throw, not silently - // fall through - Path missing = tempDir.resolve("not-a-runtime.node"); - Path cacheBase = tempDir.resolve("cache"); - ClassLoader loader = classLoaderWithRuntimeResource(tempDir, TEST_CLASSIFIER); - - assertThrows(IllegalStateException.class, () -> NativeRuntimeLoader.resolve(missing.toString(), cacheBase, - loader, TEST_CLASSIFIER, TEST_VERSION)); - } - // ------------------------------------------------------------------------- // Source 2: classpath extraction to cache // -------------------------------------------------------------------------