From 9737df17be09f4bea8300230e5a26ede1a74b80b Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Mon, 23 Mar 2026 23:49:12 +0200 Subject: [PATCH 01/14] feat: support locally provided CLI binaries via binaryDestination Reimplemented `binPath` to support three modes: - default data directory - absolute path to a pre-existing local CLI (when downloads are disabled) - and base directory with host-specific subdirectory (when downloads are enabled) This modes are controlled by the Binary directory setting now renamed to Binary destination and the enable downloads setting. - resolves #285 --- CHANGELOG.md | 4 + .../toolbox/settings/ReadOnlyCoderSettings.kt | 13 +- .../coder/toolbox/store/CoderSettingsStore.kt | 32 +++-- .../com/coder/toolbox/store/StoreKeys.kt | 5 +- .../coder/toolbox/views/CoderSettingsPage.kt | 4 +- .../resources/localization/defaultMessages.po | 2 +- .../toolbox/store/CoderSettingsStoreTest.kt | 135 +++++++++++++++++- 7 files changed, 169 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9639dc35..82912ce8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Added + +- support for using a locally provided CLI binary when downloads are disabled + ## 0.8.6 - 2026-03-05 ### Changed diff --git a/src/main/kotlin/com/coder/toolbox/settings/ReadOnlyCoderSettings.kt b/src/main/kotlin/com/coder/toolbox/settings/ReadOnlyCoderSettings.kt index 2c19e93b..95be6363 100644 --- a/src/main/kotlin/com/coder/toolbox/settings/ReadOnlyCoderSettings.kt +++ b/src/main/kotlin/com/coder/toolbox/settings/ReadOnlyCoderSettings.kt @@ -35,10 +35,10 @@ interface ReadOnlyCoderSettings { val binarySource: String? /** - * Directories are created here that store the CLI for each domain to which - * the plugin connects. Defaults to the data directory. + * An absolute path to a local directly where the CLI will be downloaded. If [enableDownloads] is true + * then this setting can point to the CLI file locally. Defaults to the data directory. */ - val binaryDirectory: String? + val binaryDestination: String? /** * Controls whether we verify the cli signature @@ -60,11 +60,6 @@ interface ReadOnlyCoderSettings { */ val defaultCliBinaryNameByOsAndArch: String - /** - * Configurable CLI binary name with extension, dependent on OS and arch - */ - val binaryName: String - /** * Default CLI signature name based on OS and architecture */ @@ -72,7 +67,7 @@ interface ReadOnlyCoderSettings { /** * Where to save plugin data like the Coder binary (if not configured with - * binaryDirectory) and the deployment URL and session token. + * binaryDestination) and the deployment URL and session token. */ val dataDirectory: String? diff --git a/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt b/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt index b365d680..5baeb8b9 100644 --- a/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt +++ b/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt @@ -41,7 +41,7 @@ class CoderSettingsStore( override val defaultURL: String get() = store[DEFAULT_URL] ?: "https://dev.coder.com" override val useAppNameAsTitle: Boolean get() = store[APP_NAME_AS_TITLE]?.toBooleanStrictOrNull() ?: false override val binarySource: String? get() = store[BINARY_SOURCE] - override val binaryDirectory: String? get() = store[BINARY_DIRECTORY] + override val binaryDestination: String? get() = store[BINARY_DESTINATION] ?: store[BINARY_DIRECTORY] override val disableSignatureVerification: Boolean get() = store[DISABLE_SIGNATURE_VALIDATION]?.toBooleanStrictOrNull() ?: false override val fallbackOnCoderForSignatures: SignatureFallbackStrategy @@ -49,7 +49,6 @@ class CoderSettingsStore( override val httpClientLogLevel: HttpLoggingVerbosity get() = HttpLoggingVerbosity.fromValue(store[HTTP_CLIENT_LOG_LEVEL]) override val defaultCliBinaryNameByOsAndArch: String get() = getCoderCLIForOS(getOS(), getArch()) - override val binaryName: String get() = store[BINARY_NAME] ?: getCoderCLIForOS(getOS(), getArch()) override val defaultSignatureNameByOsAndArch: String get() = getCoderSignatureForOS(getOS(), getArch()) override val dataDirectory: String? get() = store[DATA_DIRECTORY] override val globalDataDirectory: String get() = getDefaultGlobalDataDir().normalize().toString() @@ -125,20 +124,33 @@ class CoderSettingsStore( /** * To where the specified deployment should download the binary. + * + * Resolution logic: + * 1. If [binaryDestination] is null/empty or [forceDownloadToData] is true, + * the binary is placed in the deployment's data directory with the + * default CLI binary name for the current OS and architecture. + * 2. If [binaryDestination] is set and [enableDownloads] is false, the value + * is treated as an absolute path to a pre-existing CLI binary (~ and $HOME + * are expanded). + * 3. Otherwise [binaryDestination] is treated as a base directory; the binary + * is placed under a host-specific subdirectory with the default CLI binary + * name. */ override fun binPath( url: URL, forceDownloadToData: Boolean, ): Path { - binaryDirectory.let { - val dir = - if (forceDownloadToData || it.isNullOrBlank()) { - dataDir(url) - } else { - withHost(Path.of(expand(it)), url) - } - return dir.resolve(binaryName).toAbsolutePath() + val dest = binaryDestination + if (dest.isNullOrBlank() || forceDownloadToData) { + return dataDir(url).resolve(defaultCliBinaryNameByOsAndArch).toAbsolutePath() } + + val expanded = Path.of(expand(dest)) + if (!enableDownloads) { + return expanded.toAbsolutePath() + } + + return withHost(expanded, url).resolve(defaultCliBinaryNameByOsAndArch).toAbsolutePath() } /** diff --git a/src/main/kotlin/com/coder/toolbox/store/StoreKeys.kt b/src/main/kotlin/com/coder/toolbox/store/StoreKeys.kt index 7cd9ec5c..278f7f9e 100644 --- a/src/main/kotlin/com/coder/toolbox/store/StoreKeys.kt +++ b/src/main/kotlin/com/coder/toolbox/store/StoreKeys.kt @@ -10,7 +10,10 @@ internal const val APP_NAME_AS_TITLE = "useAppNameAsTitle" internal const val BINARY_SOURCE = "binarySource" -internal const val BINARY_DIRECTORY = "binaryDirectory" +@Deprecated("Use binaryDestination instead", replaceWith = ReplaceWith("StoreKeys.BINARY_DESTINATION")) +internal const val BINARY_DIRECTORY = "binaryDestination" + +internal const val BINARY_DESTINATION = "binaryDestination" internal const val DISABLE_SIGNATURE_VALIDATION = "disableSignatureValidation" diff --git a/src/main/kotlin/com/coder/toolbox/views/CoderSettingsPage.kt b/src/main/kotlin/com/coder/toolbox/views/CoderSettingsPage.kt index d268baa9..367ae3bb 100644 --- a/src/main/kotlin/com/coder/toolbox/views/CoderSettingsPage.kt +++ b/src/main/kotlin/com/coder/toolbox/views/CoderSettingsPage.kt @@ -39,7 +39,7 @@ class CoderSettingsPage( private val binarySourceField = TextField(context.i18n.ptrl("Binary source"), settings.binarySource ?: "", TextType.General) private val binaryDirectoryField = - TextField(context.i18n.ptrl("Binary directory"), settings.binaryDirectory ?: "", TextType.General) + TextField(context.i18n.ptrl("Binary destination"), settings.binaryDestination ?: "", TextType.General) private val dataDirectoryField = TextField(context.i18n.ptrl("Data directory"), settings.dataDirectory ?: "", TextType.General) private val enableDownloadsField = @@ -201,7 +201,7 @@ class CoderSettingsPage( settings.binarySource ?: "" } binaryDirectoryField.contentState.update { - settings.binaryDirectory ?: "" + settings.binaryDestination ?: "" } dataDirectoryField.contentState.update { settings.dataDirectory ?: "" diff --git a/src/main/resources/localization/defaultMessages.po b/src/main/resources/localization/defaultMessages.po index abff66c3..ad6a692f 100644 --- a/src/main/resources/localization/defaultMessages.po +++ b/src/main/resources/localization/defaultMessages.po @@ -70,7 +70,7 @@ msgstr "" msgid "Binary source" msgstr "" -msgid "Binary directory" +msgid "Binary destination" msgstr "" msgid "Data directory" diff --git a/src/test/kotlin/com/coder/toolbox/store/CoderSettingsStoreTest.kt b/src/test/kotlin/com/coder/toolbox/store/CoderSettingsStoreTest.kt index 636ef611..8dbe2ecd 100644 --- a/src/test/kotlin/com/coder/toolbox/store/CoderSettingsStoreTest.kt +++ b/src/test/kotlin/com/coder/toolbox/store/CoderSettingsStoreTest.kt @@ -5,6 +5,8 @@ import com.coder.toolbox.util.pluginTestSettingsStore import com.jetbrains.toolbox.api.core.diagnostics.Logger import io.mockk.mockk import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertTrue +import java.net.URL import kotlin.test.AfterTest import kotlin.test.BeforeTest import kotlin.test.Test @@ -69,17 +71,144 @@ class CoderSettingsStoreTest { fun `Default CLI and signature for unknown Arch fallback on Linux`() = assertBinaryAndSignature("Linux", "mips64", "coder-linux-amd64", "coder-linux-amd64.asc") + // --- binPath tests --- + @Test + fun `binPath uses dataDir when binaryDestination is null`() { + setOsAndArch("Linux", "x86_64") + val settings = storeWith() + val url = URL("https://test.coder.com") + val result = settings.binPath(url) + + assertTrue(result.isAbsolute) + assertTrue(result.toString().endsWith("/test.coder.com/coder-linux-amd64")) + } + + @Test + fun `binPath uses dataDir when binaryDestination is empty`() { + setOsAndArch("Linux", "x86_64") + val settings = storeWith(BINARY_DESTINATION to "") + val url = URL("https://test.coder.com") + val result = settings.binPath(url) + + assertTrue(result.isAbsolute) + assertTrue(result.toString().endsWith("/test.coder.com/coder-linux-amd64")) + } + + @Test + fun `binPath uses dataDir when forceDownloadToData is true even with binaryDestination set`() { + setOsAndArch("Linux", "x86_64") + val settings = storeWith(BINARY_DESTINATION to "/custom/path") + val url = URL("https://test.coder.com") + val result = settings.binPath(url, forceDownloadToData = true) + + assertTrue(result.isAbsolute) + assertTrue(result.toString().endsWith("/test.coder.com/coder-linux-amd64")) + assertTrue(!result.toString().contains("/custom/path")) + } + + @Test + fun `binPath returns absolute binaryDestination path when downloads are disabled`() { + setOsAndArch("Linux", "x86_64") + val settings = storeWith( + BINARY_DESTINATION to "/usr/local/bin/coder", + ENABLE_DOWNLOADS to "false" + ) + val url = URL("https://test.coder.com") + val result = settings.binPath(url) + + assertEquals("/usr/local/bin/coder", result.toString()) + } + + @Test + fun `binPath expands tilde in binaryDestination when downloads are disabled`() { + setOsAndArch("Linux", "x86_64") + val settings = storeWith( + BINARY_DESTINATION to "~/bin/coder", + ENABLE_DOWNLOADS to "false" + ) + val url = URL("https://test.coder.com") + val result = settings.binPath(url) + val home = System.getProperty("user.home") + + assertTrue(result.isAbsolute) + assertEquals("$home/bin/coder", result.toString()) + } + + @Test + fun `binPath expands HOME in binaryDestination when downloads are disabled`() { + setOsAndArch("Linux", "x86_64") + val settings = storeWith( + BINARY_DESTINATION to "\$HOME/bin/coder", + ENABLE_DOWNLOADS to "false" + ) + val url = URL("https://test.coder.com") + val result = settings.binPath(url) + val home = System.getProperty("user.home") + + assertTrue(result.isAbsolute) + assertEquals("$home/bin/coder", result.toString()) + } + + @Test + fun `binPath uses binaryDestination as base dir with host subdirectory when downloads are enabled`() { + setOsAndArch("Linux", "x86_64") + val settings = storeWith(BINARY_DESTINATION to "/opt/coder-cli") + val url = URL("https://test.coder.com") + val result = settings.binPath(url) + + assertEquals("/opt/coder-cli/test.coder.com/coder-linux-amd64", result.toString()) + } + + @Test + fun `binPath includes port in host subdirectory when URL has non-default port`() { + setOsAndArch("Linux", "x86_64") + val settings = storeWith(BINARY_DESTINATION to "/opt/coder-cli") + val url = URL("https://test.coder.com:8443") + val result = settings.binPath(url) + + assertEquals("/opt/coder-cli/test.coder.com-8443/coder-linux-amd64", result.toString()) + } + + @Test + fun `binPath uses correct binary name for Windows`() { + setOsAndArch("Windows 10", "amd64") + val settings = storeWith(BINARY_DESTINATION to "/opt/coder-cli") + val url = URL("https://test.coder.com") + val result = settings.binPath(url) + + assertTrue(result.toString().endsWith("coder-windows-amd64.exe")) + } + + @Test + fun `binPath uses correct binary name for Mac ARM64`() { + setOsAndArch("Mac OS X", "aarch64") + val settings = storeWith(BINARY_DESTINATION to "/opt/coder-cli") + val url = URL("https://test.coder.com") + val result = settings.binPath(url) + + assertTrue(result.toString().endsWith("coder-darwin-arm64")) + } + private fun assertBinaryAndSignature( osName: String?, arch: String?, expectedBinary: String, expectedSignature: String ) { - if (osName == null) System.clearProperty("os.name") else System.setProperty("os.name", osName) - if (arch == null) System.clearProperty("os.arch") else System.setProperty("os.arch", arch) - + setOsAndArch(osName, arch) assertEquals(expectedBinary, store.defaultCliBinaryNameByOsAndArch) assertEquals(expectedSignature, store.defaultSignatureNameByOsAndArch) } + private fun setOsAndArch(osName: String?, arch: String?) { + if (osName == null) System.clearProperty("os.name") else System.setProperty("os.name", osName) + if (arch == null) System.clearProperty("os.arch") else System.setProperty("os.arch", arch) + } + + private fun storeWith(vararg pairs: Pair): CoderSettingsStore = + CoderSettingsStore( + pluginTestSettingsStore(*pairs), + Environment(), + mockk(relaxed = true) + ) } \ No newline at end of file From cd2d923f5f672ca773cabbe766542732af3c34c2 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Mon, 23 Mar 2026 23:55:52 +0200 Subject: [PATCH 02/14] chore: next version is 0.8.7 --- gradle.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle.properties b/gradle.properties index eb319e2e..9ea18a21 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,3 +1,3 @@ -version=0.8.6 +version=0.8.7 group=com.coder.toolbox name=coder-toolbox \ No newline at end of file From cab30480291bb00b7988fd38f56586d82a2bc613 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Tue, 24 Mar 2026 22:25:53 +0200 Subject: [PATCH 03/14] chore: update tests with new scenarios That tests the fallback configuration from binaryDestination to the deprecated binaryDirectory. In addition, the existing tests were updated as they now have to take into account that enableDownloads option affects the output of binPath API. --- .../com/coder/toolbox/store/StoreKeys.kt | 6 +- .../coder/toolbox/cli/CoderCLIManagerTest.kt | 111 ++++++++++++++++-- .../toolbox/settings/CoderSettingsTest.kt | 13 +- 3 files changed, 110 insertions(+), 20 deletions(-) diff --git a/src/main/kotlin/com/coder/toolbox/store/StoreKeys.kt b/src/main/kotlin/com/coder/toolbox/store/StoreKeys.kt index 278f7f9e..3325faef 100644 --- a/src/main/kotlin/com/coder/toolbox/store/StoreKeys.kt +++ b/src/main/kotlin/com/coder/toolbox/store/StoreKeys.kt @@ -10,8 +10,8 @@ internal const val APP_NAME_AS_TITLE = "useAppNameAsTitle" internal const val BINARY_SOURCE = "binarySource" -@Deprecated("Use binaryDestination instead", replaceWith = ReplaceWith("StoreKeys.BINARY_DESTINATION")) -internal const val BINARY_DIRECTORY = "binaryDestination" +@Deprecated("Use BINARY_DESTINATION instead", replaceWith = ReplaceWith("BINARY_DESTINATION")) +internal const val BINARY_DIRECTORY = "binaryDirectory" internal const val BINARY_DESTINATION = "binaryDestination" @@ -21,8 +21,6 @@ internal const val FALLBACK_ON_CODER_FOR_SIGNATURES = "signatureFallbackStrategy internal const val HTTP_CLIENT_LOG_LEVEL = "httpClientLogLevel" -internal const val BINARY_NAME = "binaryName" - internal const val DATA_DIRECTORY = "dataDirectory" internal const val ENABLE_DOWNLOADS = "enableDownloads" diff --git a/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt b/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt index 15ebfcdd..478782b5 100644 --- a/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt +++ b/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt @@ -7,8 +7,8 @@ import com.coder.toolbox.cli.ex.SSHConfigFormatException import com.coder.toolbox.sdk.DataGen.Companion.workspace import com.coder.toolbox.sdk.v2.models.Workspace import com.coder.toolbox.settings.Environment +import com.coder.toolbox.store.BINARY_DESTINATION import com.coder.toolbox.store.BINARY_DIRECTORY -import com.coder.toolbox.store.BINARY_NAME import com.coder.toolbox.store.BINARY_SOURCE import com.coder.toolbox.store.CODER_SSH_CONFIG_OPTIONS import com.coder.toolbox.store.CoderSecretsStore @@ -184,7 +184,7 @@ internal class CoderCLIManagerTest { val settings = CoderSettingsStore( pluginTestSettingsStore( DATA_DIRECTORY to tmpdir.resolve("cli-data-dir").toString(), - BINARY_DIRECTORY to tmpdir.resolve("cli-bin-dir").toString(), + BINARY_DESTINATION to tmpdir.resolve("cli-bin-dir").toString(), ), Environment(), context.logger @@ -203,6 +203,91 @@ internal class CoderCLIManagerTest { assertEquals(settings.binPath(url, true), ccm2.localBinaryPath) } + @Test + fun `testBinaryDestination with downloads enabled places binary under host subdirectory`() { + val (srv, url) = mockServer() + val binDir = tmpdir.resolve("bin-dest-downloads-enabled") + val settings = CoderSettingsStore( + pluginTestSettingsStore( + BINARY_DESTINATION to binDir.toString(), + FALLBACK_ON_CODER_FOR_SIGNATURES to "allow", + ), + Environment(), + context.logger + ) + + val ccm = CoderCLIManager(context.copy(settingsStore = settings), url) + + // With downloads enabled (default), binaryDestination is a base directory + // and the binary is placed under // + val expectedPath = binDir.resolve("localhost-${url.port}").resolve(settings.defaultCliBinaryNameByOsAndArch) + assertEquals(expectedPath.toAbsolutePath(), ccm.localBinaryPath) + + // Verify it actually downloads successfully to that location. + assertTrue(runBlocking { ccm.download(VERSION_FOR_PROGRESS_REPORTING, noOpTextProgress) }) + assertTrue(ccm.localBinaryPath.toFile().exists()) + + srv.stop(0) + } + + @Test + fun `testBinaryDestination with downloads disabled points directly to binary`() { + val binaryFile = tmpdir.resolve("local-cli").resolve("my-coder-binary") + binaryFile.parent.toFile().mkdirs() + binaryFile.toFile().writeText(mkbinVersion("1.0.0")) + if (getOS() != OS.WINDOWS) { + binaryFile.toFile().setExecutable(true) + } + + val settings = CoderSettingsStore( + pluginTestSettingsStore( + BINARY_DESTINATION to binaryFile.toString(), + ENABLE_DOWNLOADS to "false", + ), + Environment(), + context.logger + ) + + val ccm = CoderCLIManager(context.copy(settingsStore = settings), URL("https://test.coder.com")) + + // With downloads disabled, binaryDestination is the absolute path to the binary itself. + assertEquals(binaryFile.toAbsolutePath(), ccm.localBinaryPath) + assertEquals(SemVer(1, 0, 0), ccm.version()) + } + + @Test + @Suppress("DEPRECATION") + fun `testBinaryDirectory fallback to BINARY_DESTINATION`() { + val binDir = tmpdir.resolve("bin-dir-fallback") + val url = URL("http://localhost") + + // Using the deprecated BINARY_DIRECTORY key should still work + // because binaryDestination falls back to it. + val settingsWithOldKey = CoderSettingsStore( + pluginTestSettingsStore( + BINARY_DIRECTORY to binDir.toString(), + ), + Environment(), + context.logger + ) + val expectedOldKey = binDir.resolve("localhost").resolve(settingsWithOldKey.defaultCliBinaryNameByOsAndArch) + assertEquals(expectedOldKey.toAbsolutePath(), settingsWithOldKey.binPath(url)) + + // BINARY_DESTINATION takes priority over BINARY_DIRECTORY. + val overrideDir = tmpdir.resolve("bin-dest-override") + val settingsWithBothKeys = CoderSettingsStore( + pluginTestSettingsStore( + BINARY_DESTINATION to overrideDir.toString(), + BINARY_DIRECTORY to binDir.toString(), + ), + Environment(), + context.logger + ) + val expectedNewKey = + overrideDir.resolve("localhost").resolve(settingsWithBothKeys.defaultCliBinaryNameByOsAndArch) + assertEquals(expectedNewKey.toAbsolutePath(), settingsWithBothKeys.binPath(url)) + } + @Test fun testFailsToWrite() { if (getOS() == OS.WINDOWS) { @@ -279,7 +364,6 @@ internal class CoderCLIManagerTest { context.copy( settingsStore = CoderSettingsStore( pluginTestSettingsStore( - BINARY_NAME to "coder.bat", DATA_DIRECTORY to tmpdir.resolve("mock-cli").toString(), FALLBACK_ON_CODER_FOR_SIGNATURES to "allow", ), @@ -736,8 +820,7 @@ internal class CoderCLIManagerTest { context.copy( settingsStore = CoderSettingsStore( pluginTestSettingsStore( - BINARY_NAME to "coder.bat", - BINARY_DIRECTORY to tmpdir.resolve("bad-version").toString(), + BINARY_DESTINATION to tmpdir.resolve("bad-version").toString(), ), Environment(), context.logger, @@ -790,8 +873,7 @@ internal class CoderCLIManagerTest { context.copy( settingsStore = CoderSettingsStore( pluginTestSettingsStore( - BINARY_NAME to "coder.bat", - BINARY_DIRECTORY to tmpdir.resolve("matches-version").toString(), + BINARY_DESTINATION to tmpdir.resolve("matches-version").toString(), ), Environment(), context.logger, @@ -884,14 +966,26 @@ internal class CoderCLIManagerTest { ) val (srv, url) = mockServer() + val binaryName = context.settingsStore.defaultCliBinaryNameByOsAndArch + val host = "localhost-${url.port}" tests.forEach { + // When downloads are disabled, binPath treats binaryDestination + // as a direct file path. We must provide the full path including + // host subdirectory and binary name so that binPath(url).parent + // does not resolve to the shared cli-manager tmpdir. + val binDestination = if (it.enableDownloads) { + tmpdir.resolve("ensure-bin-dir").toString() + } else { + tmpdir.resolve("ensure-bin-dir").resolve(host).resolve(binaryName).toString() + } + val settingsStore = CoderSettingsStore( pluginTestSettingsStore( ENABLE_DOWNLOADS to it.enableDownloads.toString(), ENABLE_BINARY_DIR_FALLBACK to it.enableFallback.toString(), DATA_DIRECTORY to tmpdir.resolve("ensure-data-dir").toString(), - BINARY_DIRECTORY to tmpdir.resolve("ensure-bin-dir").toString(), + BINARY_DESTINATION to binDestination, FALLBACK_ON_CODER_FOR_SIGNATURES to "allow" ), Environment(), @@ -1007,7 +1101,6 @@ internal class CoderCLIManagerTest { context.copy( settingsStore = CoderSettingsStore( pluginTestSettingsStore( - BINARY_NAME to "coder.bat", DATA_DIRECTORY to tmpdir.resolve("features").toString(), FALLBACK_ON_CODER_FOR_SIGNATURES to "allow" ), diff --git a/src/test/kotlin/com/coder/toolbox/settings/CoderSettingsTest.kt b/src/test/kotlin/com/coder/toolbox/settings/CoderSettingsTest.kt index 9d38c4fe..b7bd44a1 100644 --- a/src/test/kotlin/com/coder/toolbox/settings/CoderSettingsTest.kt +++ b/src/test/kotlin/com/coder/toolbox/settings/CoderSettingsTest.kt @@ -1,6 +1,5 @@ package com.coder.toolbox.settings -import com.coder.toolbox.store.BINARY_NAME import com.coder.toolbox.store.CODER_SSH_CONFIG_OPTIONS import com.coder.toolbox.store.CoderSettingsStore import com.coder.toolbox.store.DISABLE_AUTOSTART @@ -114,11 +113,7 @@ internal class CoderSettingsTest { @Test fun testBinPath() { - val settings = CoderSettingsStore( - pluginTestSettingsStore( - BINARY_NAME to "foo-bar.baz" - ), Environment(), logger - ) + val settings = CoderSettingsStore(pluginTestSettingsStore(), Environment(), logger) // The binary path should fall back to the data directory but that is // already tested in the data directory tests. val url = URL("http://localhost") @@ -133,7 +128,11 @@ internal class CoderSettingsTest { expected = "/tmp/coder-toolbox-test/data-dir/localhost" assertEquals(Path.of(expected).toAbsolutePath(), settings.readOnly().binPath(url, true).parent) - assertEquals("foo-bar.baz", settings.readOnly().binPath(url).fileName.toString()) + // Binary name is always determined by OS and architecture. + assertEquals( + settings.readOnly().defaultCliBinaryNameByOsAndArch, + settings.readOnly().binPath(url).fileName.toString() + ) } @Test From efc54cd65b69fc13ee29a4ff22211a3d04701ac2 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Tue, 24 Mar 2026 23:30:52 +0200 Subject: [PATCH 04/14] chore: update tests with new scenarios on windows Windows needs special handling because we can't mock the exe binary as a bash script like we do on Linux/Mac. In addition, there were a couple of unix paths hardcoded in some of the tests. --- .../coder/toolbox/cli/CoderCLIManagerTest.kt | 38 ++++++++++++++---- .../toolbox/store/CoderSettingsStoreTest.kt | 40 +++++++++++-------- 2 files changed, 54 insertions(+), 24 deletions(-) diff --git a/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt b/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt index 478782b5..f77fd7eb 100644 --- a/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt +++ b/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt @@ -375,7 +375,11 @@ internal class CoderCLIManagerTest { ) assertEquals(true, runBlocking { ccm.download(VERSION_FOR_PROGRESS_REPORTING, noOpTextProgress) }) - assertEquals(SemVer(url.port.toLong(), 0, 0), ccm.version()) + // On Windows the downloaded script is saved as .exe and cannot be executed + // as a batch script, so skip the version check. + if (getOS() != OS.WINDOWS) { + assertEquals(SemVer(url.port.toLong(), 0, 0), ccm.version()) + } // It should skip the second attempt. assertEquals(false, runBlocking { ccm.download(VERSION_FOR_PROGRESS_REPORTING, noOpTextProgress) }) @@ -819,9 +823,16 @@ internal class CoderCLIManagerTest { val ccm = CoderCLIManager( context.copy( settingsStore = CoderSettingsStore( - pluginTestSettingsStore( - BINARY_DESTINATION to tmpdir.resolve("bad-version").toString(), - ), + if (getOS() == OS.WINDOWS) { + pluginTestSettingsStore( + BINARY_DESTINATION to tmpdir.resolve("bad-version").resolve("coder.bat").toString(), + ENABLE_DOWNLOADS to "false", + ) + } else { + pluginTestSettingsStore( + BINARY_DESTINATION to tmpdir.resolve("bad-version").toString(), + ) + }, Environment(), context.logger, ) @@ -872,9 +883,16 @@ internal class CoderCLIManagerTest { val ccm = CoderCLIManager( context.copy( settingsStore = CoderSettingsStore( - pluginTestSettingsStore( - BINARY_DESTINATION to tmpdir.resolve("matches-version").toString(), - ), + if (getOS() == OS.WINDOWS) { + pluginTestSettingsStore( + BINARY_DESTINATION to tmpdir.resolve("matches-version").resolve("coder.bat").toString(), + ENABLE_DOWNLOADS to "false", + ) + } else { + pluginTestSettingsStore( + BINARY_DESTINATION to tmpdir.resolve("matches-version").toString(), + ) + }, Environment(), context.logger, ) @@ -1111,7 +1129,11 @@ internal class CoderCLIManagerTest { url, ) assertEquals(true, runBlocking { ccm.download(VERSION_FOR_PROGRESS_REPORTING, noOpTextProgress) }) - assertEquals(it.second, ccm.features, "version: ${it.first}") + // On Windows the downloaded script is saved as .exe and cannot be executed + // as a batch script, so skip the features check. + if (getOS() != OS.WINDOWS) { + assertEquals(it.second, ccm.features, "version: ${it.first}") + } srv.stop(0) } diff --git a/src/test/kotlin/com/coder/toolbox/store/CoderSettingsStoreTest.kt b/src/test/kotlin/com/coder/toolbox/store/CoderSettingsStoreTest.kt index 8dbe2ecd..d46c48e9 100644 --- a/src/test/kotlin/com/coder/toolbox/store/CoderSettingsStoreTest.kt +++ b/src/test/kotlin/com/coder/toolbox/store/CoderSettingsStoreTest.kt @@ -5,8 +5,10 @@ import com.coder.toolbox.util.pluginTestSettingsStore import com.jetbrains.toolbox.api.core.diagnostics.Logger import io.mockk.mockk import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertFalse import org.junit.jupiter.api.Assertions.assertTrue import java.net.URL +import java.nio.file.Path import kotlin.test.AfterTest import kotlin.test.BeforeTest import kotlin.test.Test @@ -80,7 +82,7 @@ class CoderSettingsStoreTest { val result = settings.binPath(url) assertTrue(result.isAbsolute) - assertTrue(result.toString().endsWith("/test.coder.com/coder-linux-amd64")) + assertTrue(result.endsWith(Path.of("test.coder.com", "coder-linux-amd64"))) } @Test @@ -91,7 +93,7 @@ class CoderSettingsStoreTest { val result = settings.binPath(url) assertTrue(result.isAbsolute) - assertTrue(result.toString().endsWith("/test.coder.com/coder-linux-amd64")) + assertTrue(result.endsWith(Path.of("test.coder.com", "coder-linux-amd64"))) } @Test @@ -102,13 +104,13 @@ class CoderSettingsStoreTest { val result = settings.binPath(url, forceDownloadToData = true) assertTrue(result.isAbsolute) - assertTrue(result.toString().endsWith("/test.coder.com/coder-linux-amd64")) - assertTrue(!result.toString().contains("/custom/path")) + assertTrue(result.endsWith(Path.of("test.coder.com", "coder-linux-amd64"))) + // The custom path should NOT be part of the result when forceDownloadToData is true. + assertFalse(result.startsWith(Path.of("/custom/path").toAbsolutePath())) } @Test fun `binPath returns absolute binaryDestination path when downloads are disabled`() { - setOsAndArch("Linux", "x86_64") val settings = storeWith( BINARY_DESTINATION to "/usr/local/bin/coder", ENABLE_DOWNLOADS to "false" @@ -116,37 +118,37 @@ class CoderSettingsStoreTest { val url = URL("https://test.coder.com") val result = settings.binPath(url) - assertEquals("/usr/local/bin/coder", result.toString()) + assertEquals(Path.of("/usr/local/bin/coder").toAbsolutePath(), result) } @Test fun `binPath expands tilde in binaryDestination when downloads are disabled`() { - setOsAndArch("Linux", "x86_64") + // Don't override OS — tilde expansion depends on the real File.separator. val settings = storeWith( BINARY_DESTINATION to "~/bin/coder", ENABLE_DOWNLOADS to "false" ) val url = URL("https://test.coder.com") val result = settings.binPath(url) - val home = System.getProperty("user.home") + val home = Path.of(System.getProperty("user.home")) assertTrue(result.isAbsolute) - assertEquals("$home/bin/coder", result.toString()) + assertEquals(home.resolve("bin").resolve("coder").toAbsolutePath(), result) } @Test fun `binPath expands HOME in binaryDestination when downloads are disabled`() { - setOsAndArch("Linux", "x86_64") + // Don't override OS — $HOME expansion depends on the real File.separator. val settings = storeWith( BINARY_DESTINATION to "\$HOME/bin/coder", ENABLE_DOWNLOADS to "false" ) val url = URL("https://test.coder.com") val result = settings.binPath(url) - val home = System.getProperty("user.home") + val home = Path.of(System.getProperty("user.home")) assertTrue(result.isAbsolute) - assertEquals("$home/bin/coder", result.toString()) + assertEquals(home.resolve("bin").resolve("coder").toAbsolutePath(), result) } @Test @@ -156,7 +158,10 @@ class CoderSettingsStoreTest { val url = URL("https://test.coder.com") val result = settings.binPath(url) - assertEquals("/opt/coder-cli/test.coder.com/coder-linux-amd64", result.toString()) + assertEquals( + Path.of("/opt/coder-cli", "test.coder.com", "coder-linux-amd64").toAbsolutePath(), + result + ) } @Test @@ -166,7 +171,10 @@ class CoderSettingsStoreTest { val url = URL("https://test.coder.com:8443") val result = settings.binPath(url) - assertEquals("/opt/coder-cli/test.coder.com-8443/coder-linux-amd64", result.toString()) + assertEquals( + Path.of("/opt/coder-cli", "test.coder.com-8443", "coder-linux-amd64").toAbsolutePath(), + result + ) } @Test @@ -176,7 +184,7 @@ class CoderSettingsStoreTest { val url = URL("https://test.coder.com") val result = settings.binPath(url) - assertTrue(result.toString().endsWith("coder-windows-amd64.exe")) + assertEquals("coder-windows-amd64.exe", result.fileName.toString()) } @Test @@ -186,7 +194,7 @@ class CoderSettingsStoreTest { val url = URL("https://test.coder.com") val result = settings.binPath(url) - assertTrue(result.toString().endsWith("coder-darwin-arm64")) + assertEquals("coder-darwin-arm64", result.fileName.toString()) } private fun assertBinaryAndSignature( From 98379f116b34cd6d81674b7cec31a2223ce17cd7 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Wed, 25 Mar 2026 22:04:30 +0200 Subject: [PATCH 05/14] chore: update tests with new scenarios on windows (2) Windows needs special handling because we can't mock the exe binary as a bash script like we do on Linux/Mac. In addition, there were a couple of unix paths hardcoded in some of the tests. --- .../com/coder/toolbox/cli/CoderCLIManagerTest.kt | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt b/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt index f77fd7eb..be8313ff 100644 --- a/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt +++ b/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt @@ -204,7 +204,7 @@ internal class CoderCLIManagerTest { } @Test - fun `testBinaryDestination with downloads enabled places binary under host subdirectory`() { + fun `test binaryDestination with downloads enabled places binary under host subdirectory`() { val (srv, url) = mockServer() val binDir = tmpdir.resolve("bin-dest-downloads-enabled") val settings = CoderSettingsStore( @@ -231,7 +231,7 @@ internal class CoderCLIManagerTest { } @Test - fun `testBinaryDestination with downloads disabled points directly to binary`() { + fun `test binaryDestination with downloads disabled points directly to binary`() { val binaryFile = tmpdir.resolve("local-cli").resolve("my-coder-binary") binaryFile.parent.toFile().mkdirs() binaryFile.toFile().writeText(mkbinVersion("1.0.0")) @@ -252,12 +252,16 @@ internal class CoderCLIManagerTest { // With downloads disabled, binaryDestination is the absolute path to the binary itself. assertEquals(binaryFile.toAbsolutePath(), ccm.localBinaryPath) - assertEquals(SemVer(1, 0, 0), ccm.version()) + // On Windows the downloaded script is saved as .exe and cannot be executed + // as a batch script, so skip the version check. + if (getOS() != OS.WINDOWS) { + assertEquals(SemVer(1, 0, 0), ccm.version()) + } } @Test @Suppress("DEPRECATION") - fun `testBinaryDirectory fallback to BINARY_DESTINATION`() { + fun `test binaryDirectory fallback to BINARY_DESTINATION`() { val binDir = tmpdir.resolve("bin-dir-fallback") val url = URL("http://localhost") From bba9449d5b55e4a6bc54499067272b02a287f300 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Wed, 25 Mar 2026 22:12:38 +0200 Subject: [PATCH 06/14] chore: fix typo --- .../kotlin/com/coder/toolbox/settings/ReadOnlyCoderSettings.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/kotlin/com/coder/toolbox/settings/ReadOnlyCoderSettings.kt b/src/main/kotlin/com/coder/toolbox/settings/ReadOnlyCoderSettings.kt index 95be6363..3e796e58 100644 --- a/src/main/kotlin/com/coder/toolbox/settings/ReadOnlyCoderSettings.kt +++ b/src/main/kotlin/com/coder/toolbox/settings/ReadOnlyCoderSettings.kt @@ -35,7 +35,7 @@ interface ReadOnlyCoderSettings { val binarySource: String? /** - * An absolute path to a local directly where the CLI will be downloaded. If [enableDownloads] is true + * An absolute path to a local directory where the CLI will be downloaded. If [enableDownloads] is true * then this setting can point to the CLI file locally. Defaults to the data directory. */ val binaryDestination: String? From fcb3970e00087b42a655a082212d51fccf1d1d94 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Fri, 27 Mar 2026 00:37:26 +0200 Subject: [PATCH 07/14] impl: fix cli resolution binaryDestination can now take a path to an executable or a path to a download directory. --- .../com/coder/toolbox/cli/CoderCLIManager.kt | 44 +++++++++++++------ .../toolbox/settings/ReadOnlyCoderSettings.kt | 7 ++- .../coder/toolbox/store/CoderSettingsStore.kt | 35 ++++++++------- 3 files changed, 55 insertions(+), 31 deletions(-) diff --git a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt index 9963e753..ce368d30 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt @@ -50,14 +50,22 @@ internal data class Version( /** * Do as much as possible to get a valid, up-to-date CLI. * - * 1. Read the binary directory for the provided URL. - * 2. Abort if we already have an up-to-date version. - * 3. Download the binary using an ETag. - * 4. Abort if we get a 304 (covers cases where the binary is older and does not - * have a version command). - * 5. Download on top of the existing binary. - * 6. Since the binary directory can be read-only, if downloading fails, start - * from step 2 with the data directory. + * 1. Create a CLI manager for the deployment URL. + * 2. If the CLI version matches the build version, return it immediately. + * 3. If downloads are enabled, attempt to download the CLI. + * a. On success, return the CLI. + * b. On [java.nio.file.AccessDeniedException]: if the binary path parent + * differs from the data directory and binary directory fallback is + * enabled, try downloading to the data directory instead. If the + * fallback data directory already has a matching version, return it + * without downloading. Otherwise, rethrow. + * c. Any other exception propagates to the caller. + * 4. If downloads are disabled, check the data directory for a matching CLI. + * a. If the data directory CLI version matches, return it. + * b. If neither the configured binary path nor the data directory has a + * usable CLI and downloads are disabled, throw [IllegalStateException]. + * c. Prefer the configured binary path unless only the data directory has + * a working binary. */ suspend fun ensureCLI( context: CoderToolboxContext, @@ -97,6 +105,17 @@ suspend fun ensureCLI( if (binPath.parent == dataDir || !settings.enableBinaryDirectoryFallback) { throw e } + // fall back to the data directory. + val fallbackCLI = CoderCLIManager(context, deploymentURL, true) + val fallbackMatches = fallbackCLI.matchesVersion(buildVersion) + if (fallbackMatches == true) { + reportProgress("Local CLI version from data directory matches server version: $buildVersion") + return fallbackCLI + } + + reportProgress("Downloading Coder CLI to the data directory...") + fallbackCLI.download(buildVersion, showTextProgress) + return fallbackCLI } } @@ -108,14 +127,11 @@ suspend fun ensureCLI( return dataCLI } - if (settings.enableDownloads) { - reportProgress("Downloading Coder CLI to the data directory...") - dataCLI.download(buildVersion, showTextProgress) - return dataCLI - } - // Prefer the binary directory unless the data directory has a // working binary and the binary directory does not. + if (cliMatches == null && dataCLIMatches == null && !settings.enableDownloads) { + throw IllegalStateException("Can't resolve Coder CLI and downloads are disabled") + } return if (cliMatches == null && dataCLIMatches != null) dataCLI else cli } diff --git a/src/main/kotlin/com/coder/toolbox/settings/ReadOnlyCoderSettings.kt b/src/main/kotlin/com/coder/toolbox/settings/ReadOnlyCoderSettings.kt index 3e796e58..a925ba1e 100644 --- a/src/main/kotlin/com/coder/toolbox/settings/ReadOnlyCoderSettings.kt +++ b/src/main/kotlin/com/coder/toolbox/settings/ReadOnlyCoderSettings.kt @@ -35,8 +35,11 @@ interface ReadOnlyCoderSettings { val binarySource: String? /** - * An absolute path to a local directory where the CLI will be downloaded. If [enableDownloads] is true - * then this setting can point to the CLI file locally. Defaults to the data directory. + * An absolute path to either a directory or an existing executable CLI binary. + * When the path points to an existing executable file, it is used as the CLI + * binary path directly. Otherwise, it is treated as a base directory under + * which the CLI is placed in a host-specific subdirectory. Defaults to the + * data directory when not set. */ val binaryDestination: String? diff --git a/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt b/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt index 5baeb8b9..62ea48c8 100644 --- a/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt +++ b/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt @@ -17,6 +17,7 @@ import com.jetbrains.toolbox.api.core.PluginSettingsStore import com.jetbrains.toolbox.api.core.diagnostics.Logger import java.net.URL import java.nio.file.Files +import java.nio.file.LinkOption import java.nio.file.Path import java.nio.file.Paths @@ -123,16 +124,17 @@ class CoderSettingsStore( } /** - * To where the specified deployment should download the binary. + * To where the specified deployment should place the CLI binary. * * Resolution logic: - * 1. If [binaryDestination] is null/empty or [forceDownloadToData] is true, - * the binary is placed in the deployment's data directory with the - * default CLI binary name for the current OS and architecture. - * 2. If [binaryDestination] is set and [enableDownloads] is false, the value - * is treated as an absolute path to a pre-existing CLI binary (~ and $HOME - * are expanded). - * 3. Otherwise [binaryDestination] is treated as a base directory; the binary + * 1. If [binaryDestination] is null/empty, the binary is placed in the + * deployment's data directory with the default CLI binary name for the + * current OS and architecture. + * 2. If [forceDownloadToData] is true, the binary is placed in a + * host-specific subdirectory under the data directory. + * 3. If [binaryDestination] points to an existing executable file, the path + * is used as-is (~ and $HOME are expanded). + * 4. Otherwise [binaryDestination] is treated as a base directory; the binary * is placed under a host-specific subdirectory with the default CLI binary * name. */ @@ -140,17 +142,20 @@ class CoderSettingsStore( url: URL, forceDownloadToData: Boolean, ): Path { - val dest = binaryDestination - if (dest.isNullOrBlank() || forceDownloadToData) { + if (binaryDestination.isNullOrBlank()) { return dataDir(url).resolve(defaultCliBinaryNameByOsAndArch).toAbsolutePath() } - val expanded = Path.of(expand(dest)) - if (!enableDownloads) { - return expanded.toAbsolutePath() - } + val dest = Path.of(expand(binaryDestination!!)) + val isExecutable = Files.isRegularFile(dest, LinkOption.NOFOLLOW_LINKS) && Files.isExecutable(dest) - return withHost(expanded, url).resolve(defaultCliBinaryNameByOsAndArch).toAbsolutePath() + if (forceDownloadToData) { + return withHost(dataDir(url), url).resolve(defaultCliBinaryNameByOsAndArch).toAbsolutePath() + } + if (isExecutable) { + return dest.toAbsolutePath() + } + return withHost(dest, url).resolve(defaultCliBinaryNameByOsAndArch).toAbsolutePath() } /** From 9a93f53f9f00d6a056ff42719e1647c307487b9d Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Fri, 27 Mar 2026 22:55:48 +0200 Subject: [PATCH 08/14] chore: update method docs In order to reflect the new behavior regarding CLI resolution in general and the binary destination in particular. --- .../com/coder/toolbox/cli/CoderCLIManager.kt | 20 +++++++++---------- .../coder/toolbox/store/CoderSettingsStore.kt | 19 +++++++++--------- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt index ce368d30..4bbe68af 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt @@ -54,18 +54,18 @@ internal data class Version( * 2. If the CLI version matches the build version, return it immediately. * 3. If downloads are enabled, attempt to download the CLI. * a. On success, return the CLI. - * b. On [java.nio.file.AccessDeniedException]: if the binary path parent - * differs from the data directory and binary directory fallback is - * enabled, try downloading to the data directory instead. If the - * fallback data directory already has a matching version, return it - * without downloading. Otherwise, rethrow. + * b. On [java.nio.file.AccessDeniedException]: rethrow if the binary + * path parent equals the data directory or if binary directory + * fallback is disabled. Otherwise, if the fallback data directory + * CLI already matches the build version return it; if not, download + * to the data directory and return the fallback CLI. * c. Any other exception propagates to the caller. - * 4. If downloads are disabled, check the data directory for a matching CLI. + * 4. If downloads are disabled: * a. If the data directory CLI version matches, return it. - * b. If neither the configured binary path nor the data directory has a - * usable CLI and downloads are disabled, throw [IllegalStateException]. - * c. Prefer the configured binary path unless only the data directory has - * a working binary. + * b. If neither the configured binary nor the data directory CLI can + * report a version, throw [IllegalStateException]. + * c. Prefer the configured binary; fall back to the data directory CLI + * only when the configured binary is missing or unexecutable. */ suspend fun ensureCLI( context: CoderToolboxContext, diff --git a/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt b/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt index 62ea48c8..334daae4 100644 --- a/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt +++ b/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt @@ -127,16 +127,15 @@ class CoderSettingsStore( * To where the specified deployment should place the CLI binary. * * Resolution logic: - * 1. If [binaryDestination] is null/empty, the binary is placed in the - * deployment's data directory with the default CLI binary name for the - * current OS and architecture. - * 2. If [forceDownloadToData] is true, the binary is placed in a - * host-specific subdirectory under the data directory. - * 3. If [binaryDestination] points to an existing executable file, the path - * is used as-is (~ and $HOME are expanded). - * 4. Otherwise [binaryDestination] is treated as a base directory; the binary - * is placed under a host-specific subdirectory with the default CLI binary - * name. + * 1. If [binaryDestination] is null/blank, return the deployment's data + * directory with the default CLI binary name. [forceDownloadToData] + * is ignored because both paths resolve to the same location. + * 2. If [forceDownloadToData] is true, return a host-specific subdirectory + * under the deployment's data directory with the default CLI binary name. + * 3. If the expanded (~ and $HOME) [binaryDestination] is an existing executable file, + * return it as-is. + * 4. Otherwise, treat [binaryDestination] as a base directory and return a + * host-specific subdirectory with the default CLI binary name. */ override fun binPath( url: URL, From b79e4529ca655b79923710be346c720bc291be47 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Fri, 27 Mar 2026 23:32:30 +0200 Subject: [PATCH 09/14] chore: update README Adds a chapter describing the behavior of binaryDestination, dataDir, enableDownloads, and enableBinaryDirFallback, and explains how these settings interact and work together. --- README.md | 33 +++++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 733efa77..53c3359a 100644 --- a/README.md +++ b/README.md @@ -448,15 +448,18 @@ storage paths. The options can be configured from the plugin's main Workspaces p If a relative path is provided, it is resolved against the deployment domain. - `Enable downloads` allows automatic downloading of the CLI if the current version is missing or outdated. + Defaults to enabled. -- `Binary directory` specifies the directory where CLI binaries are stored. If omitted, it defaults to the data - directory. +- `Binary destination` specifies where the CLI binary is placed. This can be a path to an existing + executable (used as-is) or a base directory (the CLI is placed under a host-specific subdirectory). + If blank, the data directory is used. Supports `~` and `$HOME` expansion. -- `Enable binary directory fallback` if enabled, falls back to the data directory when the specified binary - directory is not writable. +- `Enable binary directory fallback` when enabled, if the binary destination is not writable the + plugin falls back to the data directory instead of failing. Only takes effect when downloads are + enabled and the binary destination differs from the data directory. Defaults to disabled. -- `Data directory` directory where plugin-specific data such as session tokens and binaries are stored if not - overridden by the binary directory setting. +- `Data directory` directory where deployment-specific data such as session tokens and CLI binaries + are stored. Each deployment gets a host-specific subdirectory (e.g. `coder.example.com`). - `Header command` command that outputs additional HTTP headers. Each line of output must be in the format key=value. The environment variable CODER_URL will be available to the command process. @@ -471,6 +474,24 @@ storage paths. The options can be configured from the plugin's main Workspaces p Helpful for customers that have their own in-house dashboards. Defaults to the Coder deployment templates page. This setting supports `$workspaceOwner` as placeholder with the replacing value being the username that logged in. +#### How CLI resolution works + +When connecting to a deployment the plugin ensures a compatible CLI binary is available. +The settings above interact as follows: + +1. If a CLI already exists at the binary destination and its version matches the deployment, it is + used immediately. +2. If **downloads are enabled**, the plugin downloads the matching version to the binary destination. + - If the download fails with a permission error and **binary directory fallback** is enabled (and + the binary destination is not already in the data directory), the plugin checks whether the data + directory already has a matching CLI. If so it is used; otherwise the plugin downloads to the + data directory instead. + - Any other download error is reported to the user. +3. If **downloads are disabled**, the plugin checks the data directory for a CLI whose version + matches the deployment. If no exact match is found anywhere, whichever CLI is available is + returned — preferring the binary destination unless it is missing, in which case the data + directory CLI is used regardless of its version. If no CLI exists at all, an error is raised. + ### TLS settings The following options control the secure communication behavior of the plugin with Coder deployment and its available From a5bb35711da96a638942986f902ff5585bc45774 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Sat, 28 Mar 2026 00:40:32 +0200 Subject: [PATCH 10/14] test: extensive UTs covering CLI resolution Refactored some of the existing tests into a new battery with a bunch additional tests to cover the complex and intertwined way of working for binaryDestination, enableDownloads, dataDir and enableFallback settings in the way CLI is resolved. --- .../coder/toolbox/cli/CoderCLIManagerTest.kt | 172 --- .../com/coder/toolbox/cli/EnsureCLITest.kt | 1023 +++++++++++++++++ .../com/coder/toolbox/util/IgnoreOnWindows.kt | 9 + 3 files changed, 1032 insertions(+), 172 deletions(-) create mode 100644 src/test/kotlin/com/coder/toolbox/cli/EnsureCLITest.kt create mode 100644 src/test/kotlin/com/coder/toolbox/util/IgnoreOnWindows.kt diff --git a/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt b/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt index be8313ff..6cb50f35 100644 --- a/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt +++ b/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt @@ -15,7 +15,6 @@ import com.coder.toolbox.store.CoderSecretsStore import com.coder.toolbox.store.CoderSettingsStore import com.coder.toolbox.store.DATA_DIRECTORY import com.coder.toolbox.store.DISABLE_AUTOSTART -import com.coder.toolbox.store.ENABLE_BINARY_DIR_FALLBACK import com.coder.toolbox.store.ENABLE_DOWNLOADS import com.coder.toolbox.store.FALLBACK_ON_CODER_FOR_SIGNATURES import com.coder.toolbox.store.HEADER_COMMAND @@ -919,177 +918,6 @@ internal class CoderCLIManagerTest { } } - enum class Result { - ERROR, // Tried to download but got an error. - NONE, // Skipped download; binary does not exist. - DL_BIN, // Downloaded the binary to bin. - DL_DATA, // Downloaded the binary to data. - USE_BIN, // Used existing binary in bin. - USE_DATA, // Used existing binary in data. - } - - data class EnsureCLITest( - val version: String?, - val fallbackVersion: String?, - val buildVersion: String, - val writable: Boolean, - val enableDownloads: Boolean, - val enableFallback: Boolean, - val expect: Result, - ) - - @Test - fun testEnsureCLI() { - if (getOS() == OS.WINDOWS) { - // TODO: setWritable() does not work the same way on Windows but we - // should test what we can. - return - } - - @Suppress("BooleanLiteralArgument") - val tests = - listOf( - // CLI is writable. - EnsureCLITest(null, null, "1.0.0", true, true, true, Result.DL_BIN), // Download. - EnsureCLITest(null, null, "1.0.0", true, false, true, Result.NONE), // No download, error when used. - EnsureCLITest("1.0.1", null, "1.0.0", true, true, true, Result.DL_BIN), // Update. - EnsureCLITest("1.0.1", null, "1.0.0", true, false, true, Result.USE_BIN), // No update, use outdated. - EnsureCLITest("1.0.0", null, "1.0.0", true, false, true, Result.USE_BIN), // Use existing. - // CLI is *not* writable and fallback disabled. - EnsureCLITest(null, null, "1.0.0", false, true, false, Result.ERROR), // Fail to download. - EnsureCLITest(null, null, "1.0.0", false, false, false, Result.NONE), // No download, error when used. - EnsureCLITest("1.0.1", null, "1.0.0", false, true, false, Result.ERROR), // Fail to update. - EnsureCLITest("1.0.1", null, "1.0.0", false, false, false, Result.USE_BIN), // No update, use outdated. - EnsureCLITest("1.0.0", null, "1.0.0", false, false, false, Result.USE_BIN), // Use existing. - // CLI is *not* writable and fallback enabled. - EnsureCLITest(null, null, "1.0.0", false, true, true, Result.DL_DATA), // Download to fallback. - EnsureCLITest(null, null, "1.0.0", false, false, true, Result.NONE), // No download, error when used. - EnsureCLITest("1.0.1", "1.0.1", "1.0.0", false, true, true, Result.DL_DATA), // Update fallback. - EnsureCLITest( - "1.0.1", - "1.0.2", - "1.0.0", - false, - false, - true, - Result.USE_BIN - ), // No update, use outdated. - EnsureCLITest( - null, - "1.0.2", - "1.0.0", - false, - false, - true, - Result.USE_DATA - ), // No update, use outdated fallback. - EnsureCLITest("1.0.0", null, "1.0.0", false, false, true, Result.USE_BIN), // Use existing. - EnsureCLITest("1.0.1", "1.0.0", "1.0.0", false, false, true, Result.USE_DATA), // Use existing fallback. - ) - - val (srv, url) = mockServer() - val binaryName = context.settingsStore.defaultCliBinaryNameByOsAndArch - val host = "localhost-${url.port}" - - tests.forEach { - // When downloads are disabled, binPath treats binaryDestination - // as a direct file path. We must provide the full path including - // host subdirectory and binary name so that binPath(url).parent - // does not resolve to the shared cli-manager tmpdir. - val binDestination = if (it.enableDownloads) { - tmpdir.resolve("ensure-bin-dir").toString() - } else { - tmpdir.resolve("ensure-bin-dir").resolve(host).resolve(binaryName).toString() - } - - val settingsStore = CoderSettingsStore( - pluginTestSettingsStore( - ENABLE_DOWNLOADS to it.enableDownloads.toString(), - ENABLE_BINARY_DIR_FALLBACK to it.enableFallback.toString(), - DATA_DIRECTORY to tmpdir.resolve("ensure-data-dir").toString(), - BINARY_DESTINATION to binDestination, - FALLBACK_ON_CODER_FOR_SIGNATURES to "allow" - ), - Environment(), - context.logger - ) - val settings = settingsStore.readOnly() - val localContext = context.copy(settingsStore = settingsStore) - // Clean up from previous test. - tmpdir.resolve("ensure-data-dir").toFile().deleteRecursively() - tmpdir.resolve("ensure-bin-dir").toFile().deleteRecursively() - - // Create a binary in the regular location. - if (it.version != null) { - settings.binPath(url).parent.toFile().mkdirs() - settings.binPath(url).toFile().writeText(mkbinVersion(it.version)) - settings.binPath(url).toFile().setExecutable(true) - } - - // This not being writable will make it fall back, if enabled. - if (!it.writable) { - settings.binPath(url).parent.toFile().mkdirs() - settings.binPath(url).parent.toFile().setWritable(false) - } - - // Create a binary in the fallback location. - if (it.fallbackVersion != null) { - settings.binPath(url, true).parent.toFile().mkdirs() - settings.binPath(url, true).toFile().writeText(mkbinVersion(it.fallbackVersion)) - settings.binPath(url, true).toFile().setExecutable(true) - } - - when (it.expect) { - Result.ERROR -> { - assertFailsWith( - exceptionClass = AccessDeniedException::class, - block = { runBlocking { ensureCLI(localContext, url, it.buildVersion, noOpTextProgress) } } - ) - } - - Result.NONE -> { - val ccm = runBlocking { ensureCLI(localContext, url, it.buildVersion, noOpTextProgress) } - assertEquals(settings.binPath(url), ccm.localBinaryPath) - assertFailsWith( - exceptionClass = ProcessInitException::class, - block = { ccm.version() }, - ) - } - - Result.DL_BIN -> { - val ccm = runBlocking { ensureCLI(localContext, url, it.buildVersion, noOpTextProgress) } - assertEquals(settings.binPath(url), ccm.localBinaryPath) - assertEquals(SemVer(url.port.toLong(), 0, 0), ccm.version()) - } - - Result.DL_DATA -> { - val ccm = runBlocking { ensureCLI(localContext, url, it.buildVersion, noOpTextProgress) } - assertEquals(settings.binPath(url, true), ccm.localBinaryPath) - assertEquals(SemVer(url.port.toLong(), 0, 0), ccm.version()) - } - - Result.USE_BIN -> { - val ccm = runBlocking { ensureCLI(localContext, url, it.buildVersion, noOpTextProgress) } - assertEquals(settings.binPath(url), ccm.localBinaryPath) - assertEquals(SemVer.parse(it.version ?: ""), ccm.version()) - } - - Result.USE_DATA -> { - val ccm = runBlocking { ensureCLI(localContext, url, it.buildVersion, noOpTextProgress) } - assertEquals(settings.binPath(url, true), ccm.localBinaryPath) - assertEquals(SemVer.parse(it.fallbackVersion ?: ""), ccm.version()) - } - } - - // Make writable again so it can get cleaned up. - if (!it.writable) { - settings.binPath(url).parent.toFile().setWritable(true) - } - } - - srv.stop(0) - } - @Test fun testFeatures() { val tests = diff --git a/src/test/kotlin/com/coder/toolbox/cli/EnsureCLITest.kt b/src/test/kotlin/com/coder/toolbox/cli/EnsureCLITest.kt new file mode 100644 index 00000000..a922fb58 --- /dev/null +++ b/src/test/kotlin/com/coder/toolbox/cli/EnsureCLITest.kt @@ -0,0 +1,1023 @@ +package com.coder.toolbox.cli + +import com.coder.toolbox.CoderToolboxContext +import com.coder.toolbox.cli.ex.ResponseException +import com.coder.toolbox.settings.Environment +import com.coder.toolbox.store.BINARY_DESTINATION +import com.coder.toolbox.store.CoderSecretsStore +import com.coder.toolbox.store.CoderSettingsStore +import com.coder.toolbox.store.DATA_DIRECTORY +import com.coder.toolbox.store.DISABLE_SIGNATURE_VALIDATION +import com.coder.toolbox.store.ENABLE_BINARY_DIR_FALLBACK +import com.coder.toolbox.store.ENABLE_DOWNLOADS +import com.coder.toolbox.util.ConnectionMonitoringService +import com.coder.toolbox.util.IgnoreOnWindows +import com.coder.toolbox.util.OS +import com.coder.toolbox.util.SemVer +import com.coder.toolbox.util.getOS +import com.coder.toolbox.util.pluginTestSettingsStore +import com.coder.toolbox.util.sha1 +import com.jetbrains.toolbox.api.core.diagnostics.Logger +import com.jetbrains.toolbox.api.core.os.LocalDesktopManager +import com.jetbrains.toolbox.api.localization.LocalizableStringFactory +import com.jetbrains.toolbox.api.remoteDev.connection.ClientHelper +import com.jetbrains.toolbox.api.remoteDev.connection.ProxyAuth +import com.jetbrains.toolbox.api.remoteDev.connection.RemoteToolsHelper +import com.jetbrains.toolbox.api.remoteDev.connection.ToolboxProxySettings +import com.jetbrains.toolbox.api.remoteDev.states.EnvironmentStateColorPalette +import com.jetbrains.toolbox.api.remoteDev.ui.EnvironmentUiPageManager +import com.jetbrains.toolbox.api.ui.ToolboxUi +import com.sun.net.httpserver.HttpServer +import io.mockk.coEvery +import io.mockk.mockk +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.runBlocking +import org.junit.jupiter.api.BeforeAll +import java.net.HttpURLConnection +import java.net.InetSocketAddress +import java.net.Proxy +import java.net.ProxySelector +import java.net.URL +import java.nio.file.AccessDeniedException +import java.nio.file.Path +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertTrue + +private val noOpTextProgress: (String) -> Unit = { _ -> } + +/** + * Comprehensive tests for [ensureCLI] covering all combinations of + * binaryDestination (directory / executable / unset) and enableDownloads + * (true / false). + * + * Tests that require executing mock CLI scripts or setWritable(false) are + * skipped on Windows because: + * - Mock CLIs are shell/batch scripts saved with the default binary name + * (e.g. coder-windows-amd64.exe) and cannot be executed as .exe files. + * - File.setWritable(false) does not reliably prevent writes on Windows. + */ +internal class EnsureCLITest { + private val ui = mockk(relaxed = true) + private val baseContext = CoderToolboxContext( + ui, + mockk(), + mockk(), + mockk(), + mockk(), + mockk(), + mockk(), + mockk(relaxed = true), + mockk(relaxed = true), + CoderSettingsStore(pluginTestSettingsStore(), Environment(), mockk(relaxed = true)), + mockk(), + object : ToolboxProxySettings { + override fun getProxy(): Proxy? = null + override fun getProxySelector(): ProxySelector? = null + override fun getProxyAuth(): ProxyAuth? = null + override fun addProxyChangeListener(listener: Runnable) {} + override fun removeProxyChangeListener(listener: Runnable) {} + }, + mockk(), + ) + + @BeforeTest + fun setup() { + coEvery { ui.showYesNoPopup(any(), any(), any(), any()) } returns true + } + + + @Test + @IgnoreOnWindows + fun `given binaryDestination is a directory and downloads are enabled, returns the CLI when version already matches`() { + val url = URL("http://test.coder.invalid") + val s = settings( + BINARY_DESTINATION to testDir("dir-dest-dl-on-match/bin").toString(), + DATA_DIRECTORY to testDir("dir-dest-dl-on-match/data").toString(), + DISABLE_SIGNATURE_VALIDATION to "true", + ) + // Compute expected path before creating files so isExecutable on the + // directory does not interfere with the resolution. + val expected = s.binPath(url) + createBinary(expected, "1.0.0") + + val ccm = runBlocking { ensureCLI(ctx(s), url, "1.0.0", noOpTextProgress) } + assertEquals(expected, ccm.localBinaryPath) + assertEquals(SemVer(1, 0, 0), ccm.version()) + } + + @Test + @IgnoreOnWindows + fun `given binaryDestination is a directory and downloads are enabled, downloads and returns a fresh CLI when version mismatches`() { + val (srv, url) = mockServer(version = "2.0.0") + try { + val s = settings( + BINARY_DESTINATION to testDir("dir-dest-dl-on-mismatch/bin").toString(), + DATA_DIRECTORY to testDir("dir-dest-dl-on-mismatch/data").toString(), + DISABLE_SIGNATURE_VALIDATION to "true", + ) + val expected = s.binPath(url) + createBinary(expected, "1.0.0") + + val ccm = runBlocking { ensureCLI(ctx(s), url, "2.0.0", noOpTextProgress) } + assertEquals(expected, ccm.localBinaryPath) + assertTrue(ccm.localBinaryPath.toFile().exists()) + assertEquals(SemVer(2, 0, 0), ccm.version()) + } finally { + srv.stop(0) + } + } + + @Test + fun `given binaryDestination is a directory and downloads are enabled, propagates non-AccessDenied errors during download to the caller`() { + val (srv, url) = mockServer(errorCode = HttpURLConnection.HTTP_INTERNAL_ERROR) + try { + val s = settings( + BINARY_DESTINATION to testDir("dir-dest-dl-on-error/bin").toString(), + DATA_DIRECTORY to testDir("dir-dest-dl-on-error/data").toString(), + DISABLE_SIGNATURE_VALIDATION to "true", + ) + assertFailsWith(ResponseException::class) { + runBlocking { ensureCLI(ctx(s), url, "1.0.0", noOpTextProgress) } + } + } finally { + srv.stop(0) + } + } + + @Test + @IgnoreOnWindows + fun `given binaryDestination is a directory and downloads are enabled, falls back to the data directory CLI when access is denied and data directory version matches`() { + val (srv, url) = mockServer(version = "1.9.0") + val s = settings( + BINARY_DESTINATION to testDir("dir-dest-dl-on-fallback-match/bin").toString(), + DATA_DIRECTORY to testDir("dir-dest-dl-on-fallback-match/data").toString(), + ENABLE_BINARY_DIR_FALLBACK to "true", + DISABLE_SIGNATURE_VALIDATION to "true", + ) + val binParent = s.binPath(url).parent + val fallbackPath = s.binPath(url, true) + createBinary(fallbackPath, "2.0.0") + try { + binParent.toFile().mkdirs() + binParent.toFile().setWritable(false) + val ccm = runBlocking { ensureCLI(ctx(s), url, "2.0.0", noOpTextProgress) } + assertEquals(fallbackPath, ccm.localBinaryPath) + assertEquals(SemVer(2, 0, 0), ccm.version()) + } finally { + binParent.toFile().setWritable(true) + srv.stop(0) + } + } + + @Test + @IgnoreOnWindows + fun `given binaryDestination is a directory and downloads are enabled, downloads CLI to the data directory when access is denied and data directory CLI is missing`() { + val (srv, url) = mockServer(version = "1.0.0") + val s = settings( + BINARY_DESTINATION to testDir("dir-dest-dl-on-fallback-missing/bin").toString(), + DATA_DIRECTORY to testDir("dir-dest-dl-on-fallback-missing/data").toString(), + ENABLE_BINARY_DIR_FALLBACK to "true", + DISABLE_SIGNATURE_VALIDATION to "true", + ) + val binParent = s.binPath(url).parent + val fallbackPath = s.binPath(url, true) + try { + binParent.toFile().mkdirs() + binParent.toFile().setWritable(false) + val ccm = runBlocking { ensureCLI(ctx(s), url, "1.0.0", noOpTextProgress) } + assertEquals(fallbackPath, ccm.localBinaryPath) + assertTrue(ccm.localBinaryPath.toFile().exists()) + assertEquals(SemVer(1, 0, 0), ccm.version()) + } finally { + binParent.toFile().setWritable(true) + srv.stop(0) + } + } + + @Test + @IgnoreOnWindows + fun `given binaryDestination is a directory and downloads are enabled, rethrows AccessDeniedException when fallback is enabled but binary path is already inside the data directory`() { + val (srv, url) = mockServer() + // Set BINARY_DESTINATION to the same base as DATA_DIRECTORY so that + // binPath(url).parent == dataDir(url). + val sharedBase = testDir("dir-dest-dl-on-fallback-same-dir/shared") + val s = settings( + BINARY_DESTINATION to sharedBase.toString(), + DATA_DIRECTORY to sharedBase.toString(), + ENABLE_BINARY_DIR_FALLBACK to "true", + DISABLE_SIGNATURE_VALIDATION to "true", + ) + val binParent = s.binPath(url).parent + try { + binParent.toFile().mkdirs() + binParent.toFile().setWritable(false) + assertFailsWith(AccessDeniedException::class) { + runBlocking { ensureCLI(ctx(s), url, "1.0.0", noOpTextProgress) } + } + } finally { + binParent.toFile().setWritable(true) + srv.stop(0) + } + } + + @Test + @IgnoreOnWindows + fun `given binaryDestination is a directory and downloads are enabled, rethrows AccessDeniedException when binary directory fallback is disabled`() { + val (srv, url) = mockServer() + val s = settings( + BINARY_DESTINATION to testDir("dir-dest-dl-on-no-fallback/bin").toString(), + DATA_DIRECTORY to testDir("dir-dest-dl-on-no-fallback/data").toString(), + ENABLE_BINARY_DIR_FALLBACK to "false", + DISABLE_SIGNATURE_VALIDATION to "true", + ) + val binParent = s.binPath(url).parent + try { + binParent.toFile().mkdirs() + binParent.toFile().setWritable(false) + assertFailsWith(AccessDeniedException::class) { + runBlocking { ensureCLI(ctx(s), url, "1.0.0", noOpTextProgress) } + } + } finally { + binParent.toFile().setWritable(true) + srv.stop(0) + } + } + + @Test + @IgnoreOnWindows + fun `given binaryDestination is a directory and downloads are enabled, rethrows AccessDeniedException when fallback is disabled and binary path is inside the data directory`() { + val (srv, url) = mockServer() + val sharedBase = testDir("dir-dest-dl-on-no-fallback-same-dir/shared") + val s = settings( + BINARY_DESTINATION to sharedBase.toString(), + DATA_DIRECTORY to sharedBase.toString(), + ENABLE_BINARY_DIR_FALLBACK to "false", + DISABLE_SIGNATURE_VALIDATION to "true", + ) + val binParent = s.binPath(url).parent + try { + binParent.toFile().mkdirs() + binParent.toFile().setWritable(false) + assertFailsWith(AccessDeniedException::class) { + runBlocking { ensureCLI(ctx(s), url, "1.0.0", noOpTextProgress) } + } + } finally { + binParent.toFile().setWritable(true) + srv.stop(0) + } + } + + @Test + @IgnoreOnWindows + fun `given binaryDestination is an existing executable and downloads are enabled, returns the CLI when version already matches`() { + val url = URL("http://test.coder.invalid") + val binaryFile = testDir("exec-dest-dl-on-match/bin/my-coder") + createBinary(binaryFile, "1.0.0") + val s = settings( + BINARY_DESTINATION to binaryFile.toString(), + DATA_DIRECTORY to testDir("exec-dest-dl-on-match/data").toString(), + DISABLE_SIGNATURE_VALIDATION to "true", + ) + val expected = s.binPath(url) + assertEquals(binaryFile.toAbsolutePath(), expected) + + val ccm = runBlocking { ensureCLI(ctx(s), url, "1.0.0", noOpTextProgress) } + assertEquals(expected, ccm.localBinaryPath) + assertEquals(SemVer(1, 0, 0), ccm.version()) + } + + @Test + @IgnoreOnWindows + fun `given binaryDestination is an existing executable and downloads are enabled, downloads and returns a fresh CLI when version mismatches`() { + val (srv, url) = mockServer(version = "2.0.0") + try { + val binaryFile = testDir("exec-dest-dl-on-mismatch/bin/my-coder") + createBinary(binaryFile, "1.0.0") + val s = settings( + BINARY_DESTINATION to binaryFile.toString(), + DATA_DIRECTORY to testDir("exec-dest-dl-on-mismatch/data").toString(), + DISABLE_SIGNATURE_VALIDATION to "true", + ) + val expected = s.binPath(url) + val ccm = runBlocking { ensureCLI(ctx(s), url, "2.0.0", noOpTextProgress) } + assertEquals(expected, ccm.localBinaryPath) + assertTrue(ccm.localBinaryPath.toFile().exists()) + assertEquals(SemVer(2, 0, 0), ccm.version()) + } finally { + srv.stop(0) + } + } + + @Test + @IgnoreOnWindows + fun `given binaryDestination is an existing executable and downloads are enabled, propagates non-AccessDenied errors during download to the caller`() { + val (srv, url) = mockServer(errorCode = HttpURLConnection.HTTP_INTERNAL_ERROR) + try { + val binaryFile = testDir("exec-dest-dl-on-error/bin/my-coder") + createBinary(binaryFile, "1.0.0") + val s = settings( + BINARY_DESTINATION to binaryFile.toString(), + DATA_DIRECTORY to testDir("exec-dest-dl-on-error/data").toString(), + DISABLE_SIGNATURE_VALIDATION to "true", + ) + assertFailsWith(ResponseException::class) { + runBlocking { ensureCLI(ctx(s), url, "2.0.0", noOpTextProgress) } + } + } finally { + srv.stop(0) + } + } + + @Test + @IgnoreOnWindows + fun `given binaryDestination is an existing executable and downloads are enabled, falls back to the data directory CLI when access is denied and data directory version matches`() { + val (srv, url) = mockServer(version = "1.9.9") + val binaryFile = testDir("exec-dest-dl-on-fallback-match/bin/my-coder") + createBinary(binaryFile, "1.0.0") + val s = settings( + BINARY_DESTINATION to binaryFile.toString(), + DATA_DIRECTORY to testDir("exec-dest-dl-on-fallback-match/data").toString(), + ENABLE_BINARY_DIR_FALLBACK to "true", + DISABLE_SIGNATURE_VALIDATION to "true", + ) + val fallbackPath = s.binPath(url, true) + createBinary(fallbackPath, "2.0.0") + try { + binaryFile.parent.toFile().setWritable(false) + val ccm = runBlocking { ensureCLI(ctx(s), url, "2.0.0", noOpTextProgress) } + assertEquals(fallbackPath, ccm.localBinaryPath) + assertEquals(SemVer(2, 0, 0), ccm.version()) + } finally { + binaryFile.parent.toFile().setWritable(true) + srv.stop(0) + } + } + + @Test + @IgnoreOnWindows + fun `given binaryDestination is an existing executable and downloads are enabled, downloads CLI to the data directory when access is denied and data directory CLI is missing`() { + val (srv, url) = mockServer(version = "2.0.0") + val binaryFile = testDir("exec-dest-dl-on-fallback-missing/bin/my-coder") + createBinary(binaryFile, "1.0.0") + val s = settings( + BINARY_DESTINATION to binaryFile.toString(), + DATA_DIRECTORY to testDir("exec-dest-dl-on-fallback-missing/data").toString(), + ENABLE_BINARY_DIR_FALLBACK to "true", + DISABLE_SIGNATURE_VALIDATION to "true", + ) + val fallbackPath = s.binPath(url, true) + try { + binaryFile.parent.toFile().setWritable(false) + val ccm = runBlocking { ensureCLI(ctx(s), url, "2.0.0", noOpTextProgress) } + assertEquals(fallbackPath, ccm.localBinaryPath) + assertTrue(ccm.localBinaryPath.toFile().exists()) + assertEquals(SemVer(2, 0, 0), ccm.version()) + } finally { + binaryFile.parent.toFile().setWritable(true) + srv.stop(0) + } + } + + @Test + @IgnoreOnWindows + fun `given binaryDestination is an existing executable and downloads are enabled, rethrows AccessDeniedException when fallback is enabled but binary path is already inside the data directory`() { + val (srv, url) = mockServer() + // Place the executable inside the data dir so binPath.parent == dataDir. + val dataBase = testDir("exec-dest-dl-on-fallback-same-dir/data") + val dataDirForUrl = dataBase.resolve(hostDir(url)) + val binaryFile = dataDirForUrl.resolve("my-coder") + createBinary(binaryFile, "1.0.0") + val s = settings( + BINARY_DESTINATION to binaryFile.toString(), + DATA_DIRECTORY to dataBase.toString(), + ENABLE_BINARY_DIR_FALLBACK to "true", + DISABLE_SIGNATURE_VALIDATION to "true", + ) + // Verify the precondition: binPath.parent == dataDir + assertEquals(s.dataDir(url), s.binPath(url).parent) + try { + binaryFile.parent.toFile().setWritable(false) + assertFailsWith(AccessDeniedException::class) { + runBlocking { ensureCLI(ctx(s), url, "2.0.0", noOpTextProgress) } + } + } finally { + binaryFile.parent.toFile().setWritable(true) + srv.stop(0) + } + } + + @Test + @IgnoreOnWindows + fun `given binaryDestination is an existing executable and downloads are enabled, rethrows AccessDeniedException when binary directory fallback is disabled`() { + val (srv, url) = mockServer() + val binaryFile = testDir("exec-dest-dl-on-no-fallback/bin/my-coder") + createBinary(binaryFile, "1.0.0") + val s = settings( + BINARY_DESTINATION to binaryFile.toString(), + DATA_DIRECTORY to testDir("exec-dest-dl-on-no-fallback/data").toString(), + ENABLE_BINARY_DIR_FALLBACK to "false", + DISABLE_SIGNATURE_VALIDATION to "true", + ) + try { + binaryFile.parent.toFile().setWritable(false) + assertFailsWith(AccessDeniedException::class) { + runBlocking { ensureCLI(ctx(s), url, "2.0.0", noOpTextProgress) } + } + } finally { + binaryFile.parent.toFile().setWritable(true) + srv.stop(0) + } + } + + @Test + @IgnoreOnWindows + fun `given binaryDestination is an existing executable and downloads are enabled, rethrows AccessDeniedException when fallback is disabled and binary path is inside the data directory`() { + val (srv, url) = mockServer() + val dataBase = testDir("exec-dest-dl-on-no-fallback-same-dir/data") + val dataDirForUrl = dataBase.resolve(hostDir(url)) + val binaryFile = dataDirForUrl.resolve("my-coder") + createBinary(binaryFile, "1.0.0") + val s = settings( + BINARY_DESTINATION to binaryFile.toString(), + DATA_DIRECTORY to dataBase.toString(), + ENABLE_BINARY_DIR_FALLBACK to "false", + DISABLE_SIGNATURE_VALIDATION to "true", + ) + assertEquals(s.dataDir(url), s.binPath(url).parent) + try { + binaryFile.parent.toFile().setWritable(false) + assertFailsWith(AccessDeniedException::class) { + runBlocking { ensureCLI(ctx(s), url, "2.0.0", noOpTextProgress) } + } + } finally { + binaryFile.parent.toFile().setWritable(true) + srv.stop(0) + } + } + + @Test + @IgnoreOnWindows + fun `given binaryDestination is an existing executable and downloads are disabled, returns the CLI at destination when version matches`() { + val url = URL("http://test.coder.invalid") + val binaryFile = testDir("exec-dest-dl-off-match/bin/my-coder") + createBinary(binaryFile, "1.0.0") + val s = settings( + BINARY_DESTINATION to binaryFile.toString(), + DATA_DIRECTORY to testDir("exec-dest-dl-off-match/data").toString(), + ENABLE_DOWNLOADS to "false", + ) + val ccm = runBlocking { ensureCLI(ctx(s), url, "1.0.0", noOpTextProgress) } + assertEquals(binaryFile.toAbsolutePath(), ccm.localBinaryPath) + assertEquals(SemVer(1, 0, 0), ccm.version()) + } + + @Test + @IgnoreOnWindows + fun `given binaryDestination is an existing executable and downloads are disabled, returns the data directory CLI when destination version mismatches but data directory version matches`() { + val url = URL("http://test.coder.invalid") + val binaryFile = testDir("exec-dest-dl-off-mismatch-datacli-match/bin/my-coder") + createBinary(binaryFile, "1.0.0") + val s = settings( + BINARY_DESTINATION to binaryFile.toString(), + DATA_DIRECTORY to testDir("exec-dest-dl-off-mismatch-datacli-match/data").toString(), + ENABLE_DOWNLOADS to "false", + ) + val fallbackPath = s.binPath(url, true) + createBinary(fallbackPath, "2.0.0") + + val ccm = runBlocking { ensureCLI(ctx(s), url, "2.0.0", noOpTextProgress) } + assertEquals(fallbackPath, ccm.localBinaryPath) + assertEquals(SemVer(2, 0, 0), ccm.version()) + } + + @Test + @IgnoreOnWindows + fun `given binaryDestination is an existing executable and downloads are disabled, returns the stale destination CLI when both destination and data directory versions mismatch`() { + val url = URL("http://test.coder.invalid") + val binaryFile = testDir("exec-dest-dl-off-mismatch-datacli-wrong/bin/my-coder") + createBinary(binaryFile, "1.0.0") + val s = settings( + BINARY_DESTINATION to binaryFile.toString(), + DATA_DIRECTORY to testDir("exec-dest-dl-off-mismatch-datacli-wrong/data").toString(), + ENABLE_DOWNLOADS to "false", + ) + val fallbackPath = s.binPath(url, true) + createBinary(fallbackPath, "1.5.0") + + val ccm = runBlocking { ensureCLI(ctx(s), url, "2.0.0", noOpTextProgress) } + assertEquals(binaryFile.toAbsolutePath(), ccm.localBinaryPath) + assertEquals(SemVer(1, 0, 0), ccm.version()) + } + + @Test + @IgnoreOnWindows + fun `given binaryDestination is an existing executable and downloads are disabled, returns the stale destination CLI when version mismatches and data directory CLI is missing`() { + val url = URL("http://test.coder.invalid") + val binaryFile = testDir("exec-dest-dl-off-mismatch-datacli-missing/bin/my-coder") + createBinary(binaryFile, "1.0.0") + val s = settings( + BINARY_DESTINATION to binaryFile.toString(), + DATA_DIRECTORY to testDir("exec-dest-dl-off-mismatch-datacli-missing/data").toString(), + ENABLE_DOWNLOADS to "false", + ) + val ccm = runBlocking { ensureCLI(ctx(s), url, "2.0.0", noOpTextProgress) } + assertEquals(binaryFile.toAbsolutePath(), ccm.localBinaryPath) + assertEquals(SemVer(1, 0, 0), ccm.version()) + } + + @Test + @IgnoreOnWindows + fun `given binaryDestination is an existing executable and downloads are disabled, returns the data directory CLI when destination is missing and data directory version matches`() { + val url = URL("http://test.coder.invalid") + val binaryFile = testDir("exec-dest-dl-off-no-cli-datacli-match/bin/my-coder") + val s = settings( + BINARY_DESTINATION to binaryFile.toString(), + DATA_DIRECTORY to testDir("exec-dest-dl-off-no-cli-datacli-match/data").toString(), + ENABLE_DOWNLOADS to "false", + ) + val fallbackPath = s.binPath(url, true) + createBinary(fallbackPath, "1.0.0") + + val ccm = runBlocking { ensureCLI(ctx(s), url, "1.0.0", noOpTextProgress) } + assertEquals(fallbackPath, ccm.localBinaryPath) + assertEquals(SemVer(1, 0, 0), ccm.version()) + } + + @Test + @IgnoreOnWindows + fun `given binaryDestination is an existing executable and downloads are disabled, returns the stale data directory CLI when destination is missing and data directory version mismatches`() { + val url = URL("http://test.coder.invalid") + val binaryFile = testDir("exec-dest-dl-off-no-cli-datacli-wrong/bin/my-coder") + val s = settings( + BINARY_DESTINATION to binaryFile.toString(), + DATA_DIRECTORY to testDir("exec-dest-dl-off-no-cli-datacli-wrong/data").toString(), + ENABLE_DOWNLOADS to "false", + ) + val fallbackPath = s.binPath(url, true) + createBinary(fallbackPath, "1.0.0") + + val ccm = runBlocking { ensureCLI(ctx(s), url, "2.0.0", noOpTextProgress) } + assertEquals(fallbackPath, ccm.localBinaryPath) + assertEquals(SemVer(1, 0, 0), ccm.version()) + } + + @Test + fun `given binaryDestination is an existing executable and downloads are disabled, throws when both destination and data directory CLIs are missing`() { + val url = URL("http://test.coder.invalid") + val binaryFile = testDir("exec-dest-dl-off-both-missing/bin/my-coder") + val s = settings( + BINARY_DESTINATION to binaryFile.toString(), + DATA_DIRECTORY to testDir("exec-dest-dl-off-both-missing/data").toString(), + ENABLE_DOWNLOADS to "false", + ) + assertFailsWith(IllegalStateException::class) { + runBlocking { ensureCLI(ctx(s), url, "1.0.0", noOpTextProgress) } + } + } + + @Test + @IgnoreOnWindows + fun `given binaryDestination is a directory and downloads are disabled, returns the CLI at destination when version matches`() { + val url = URL("http://test.coder.invalid") + val s = settings( + BINARY_DESTINATION to testDir("dir-dest-dl-off-match/bin").toString(), + DATA_DIRECTORY to testDir("dir-dest-dl-off-match/data").toString(), + ENABLE_DOWNLOADS to "false", + ) + val expected = s.binPath(url) + createBinary(expected, "1.0.0") + + val ccm = runBlocking { ensureCLI(ctx(s), url, "1.0.0", noOpTextProgress) } + assertEquals(expected, ccm.localBinaryPath) + assertEquals(SemVer(1, 0, 0), ccm.version()) + } + + @Test + @IgnoreOnWindows + fun `given binaryDestination is a directory and downloads are disabled, returns the data directory CLI when destination version mismatches but data directory version matches`() { + val url = URL("http://test.coder.invalid") + val s = settings( + BINARY_DESTINATION to testDir("dir-dest-dl-off-mismatch-datacli-match/bin").toString(), + DATA_DIRECTORY to testDir("dir-dest-dl-off-mismatch-datacli-match/data").toString(), + ENABLE_DOWNLOADS to "false", + ) + val expected = s.binPath(url) + createBinary(expected, "1.0.0") + val fallbackPath = s.binPath(url, true) + createBinary(fallbackPath, "2.0.0") + + val ccm = runBlocking { ensureCLI(ctx(s), url, "2.0.0", noOpTextProgress) } + assertEquals(fallbackPath, ccm.localBinaryPath) + assertEquals(SemVer(2, 0, 0), ccm.version()) + } + + @Test + @IgnoreOnWindows + fun `given binaryDestination is a directory and downloads are disabled, returns the stale destination CLI when both destination and data directory versions mismatch`() { + val url = URL("http://test.coder.invalid") + val s = settings( + BINARY_DESTINATION to testDir("dir-dest-dl-off-mismatch-datacli-wrong/bin").toString(), + DATA_DIRECTORY to testDir("dir-dest-dl-off-mismatch-datacli-wrong/data").toString(), + ENABLE_DOWNLOADS to "false", + ) + val expected = s.binPath(url) + createBinary(expected, "1.0.0") + val fallbackPath = s.binPath(url, true) + createBinary(fallbackPath, "1.5.0") + + val ccm = runBlocking { ensureCLI(ctx(s), url, "2.0.0", noOpTextProgress) } + assertEquals(expected, ccm.localBinaryPath) + assertEquals(SemVer(1, 0, 0), ccm.version()) + } + + @Test + @IgnoreOnWindows + fun `given binaryDestination is a directory and downloads are disabled, returns the stale destination CLI when version mismatches and data directory CLI is missing`() { + val url = URL("http://test.coder.invalid") + val s = settings( + BINARY_DESTINATION to testDir("dir-dest-dl-off-mismatch-datacli-missing/bin").toString(), + DATA_DIRECTORY to testDir("dir-dest-dl-off-mismatch-datacli-missing/data").toString(), + ENABLE_DOWNLOADS to "false", + ) + val expected = s.binPath(url) + createBinary(expected, "1.0.0") + + val ccm = runBlocking { ensureCLI(ctx(s), url, "2.0.0", noOpTextProgress) } + assertEquals(expected, ccm.localBinaryPath) + assertEquals(SemVer(1, 0, 0), ccm.version()) + } + + @Test + @IgnoreOnWindows + fun `given binaryDestination is a directory and downloads are disabled, returns the data directory CLI when destination CLI is missing and data directory version matches`() { + val url = URL("http://test.coder.invalid") + val s = settings( + BINARY_DESTINATION to testDir("dir-dest-dl-off-no-cli-datacli-match/bin").toString(), + DATA_DIRECTORY to testDir("dir-dest-dl-off-no-cli-datacli-match/data").toString(), + ENABLE_DOWNLOADS to "false", + ) + val fallbackPath = s.binPath(url, true) + createBinary(fallbackPath, "1.0.0") + + val ccm = runBlocking { ensureCLI(ctx(s), url, "1.0.0", noOpTextProgress) } + assertEquals(fallbackPath, ccm.localBinaryPath) + assertEquals(SemVer(1, 0, 0), ccm.version()) + } + + @Test + @IgnoreOnWindows + fun `given binaryDestination is a directory and downloads are disabled, returns the stale data directory CLI when destination CLI is missing and data directory version mismatches`() { + val url = URL("http://test.coder.invalid") + val s = settings( + BINARY_DESTINATION to testDir("dir-dest-dl-off-no-cli-datacli-wrong/bin").toString(), + DATA_DIRECTORY to testDir("dir-dest-dl-off-no-cli-datacli-wrong/data").toString(), + ENABLE_DOWNLOADS to "false", + ) + val fallbackPath = s.binPath(url, true) + createBinary(fallbackPath, "1.0.0") + + val ccm = runBlocking { ensureCLI(ctx(s), url, "2.0.0", noOpTextProgress) } + assertEquals(fallbackPath, ccm.localBinaryPath) + assertEquals(SemVer(1, 0, 0), ccm.version()) + } + + @Test + @IgnoreOnWindows + fun `given binaryDestination is a directory and downloads are disabled, throws when both destination and data directory CLIs are missing`() { + val url = URL("http://test.coder.invalid") + val s = settings( + BINARY_DESTINATION to testDir("dir-dest-dl-off-both-missing/bin").toString(), + DATA_DIRECTORY to testDir("dir-dest-dl-off-both-missing/data").toString(), + ENABLE_DOWNLOADS to "false", + ) + assertFailsWith(IllegalStateException::class) { + runBlocking { ensureCLI(ctx(s), url, "1.0.0", noOpTextProgress) } + } + } + + @Test + @IgnoreOnWindows + fun `given no binaryDestination configured and downloads are disabled, returns the CLI when version matches`() { + val url = URL("http://test.coder.invalid") + val s = settings( + DATA_DIRECTORY to testDir("no-dest-dl-off-match/data").toString(), + ENABLE_DOWNLOADS to "false", + ) + val expected = s.binPath(url) + createBinary(expected, "1.0.0") + + val ccm = runBlocking { ensureCLI(ctx(s), url, "1.0.0", noOpTextProgress) } + assertEquals(expected, ccm.localBinaryPath) + assertEquals(SemVer(1, 0, 0), ccm.version()) + } + + @Test + @IgnoreOnWindows + fun `given no binaryDestination configured and downloads are disabled, returns the CLI when the shared path is overwritten with a matching version`() { + val url = URL("http://test.coder.invalid") + val s = settings( + DATA_DIRECTORY to testDir("no-dest-dl-off-mismatch-datacli-match/data").toString(), + ENABLE_DOWNLOADS to "false", + ) + val expected = s.binPath(url) + createBinary(expected, "1.0.0") + val fallbackPath = s.binPath(url, true) + createBinary(fallbackPath, "2.0.0") + + val ccm = runBlocking { ensureCLI(ctx(s), url, "2.0.0", noOpTextProgress) } + assertEquals(fallbackPath, ccm.localBinaryPath) + assertEquals(SemVer(2, 0, 0), ccm.version()) + } + + @Test + @IgnoreOnWindows + fun `given no binaryDestination configured and downloads are disabled, returns the stale CLI when version mismatches confirming CLI and data directory paths are identical`() { + val url = URL("http://test.coder.invalid") + val s = settings( + DATA_DIRECTORY to testDir("no-dest-dl-off-mismatch/data").toString(), + ENABLE_DOWNLOADS to "false", + ) + val expected = s.binPath(url) + // When binaryDestination is not configured, binPath(url) and + // binPath(url, true) resolve to the same path because the + // isNullOrBlank() early return in binPath fires before the + // forceDownloadToData check. So cli and dataCLI share a binary. + assertEquals(expected, s.binPath(url, true)) + createBinary(expected, "1.0.0") + + val ccm = runBlocking { ensureCLI(ctx(s), url, "2.0.0", noOpTextProgress) } + assertEquals(expected, ccm.localBinaryPath) + assertEquals(SemVer(1, 0, 0), ccm.version()) + } + + @Test + @IgnoreOnWindows + fun `given no binaryDestination configured and downloads are disabled, returns the stale CLI when version mismatches and no separate data directory CLI path exists`() { + val url = URL("http://test.coder.invalid") + val s = settings( + DATA_DIRECTORY to testDir("no-dest-dl-off-mismatch-datacli-missing/data").toString(), + ENABLE_DOWNLOADS to "false", + ) + val expected = s.binPath(url) + createBinary(expected, "1.0.0") + + val ccm = runBlocking { ensureCLI(ctx(s), url, "2.0.0", noOpTextProgress) } + assertEquals(expected, ccm.localBinaryPath) + assertEquals(SemVer(1, 0, 0), ccm.version()) + } + + @Test + @IgnoreOnWindows + fun `given no binaryDestination configured and downloads are disabled, returns the CLI when version matches at the shared data directory path`() { + val url = URL("http://test.coder.invalid") + val s = settings( + DATA_DIRECTORY to testDir("no-dest-dl-off-no-cli-datacli-match/data").toString(), + ENABLE_DOWNLOADS to "false", + ) + val fallbackPath = s.binPath(url, true) + createBinary(fallbackPath, "1.0.0") + + val ccm = runBlocking { ensureCLI(ctx(s), url, "1.0.0", noOpTextProgress) } + assertEquals(fallbackPath, ccm.localBinaryPath) + assertEquals(SemVer(1, 0, 0), ccm.version()) + } + + @Test + @IgnoreOnWindows + fun `given no binaryDestination configured and downloads are disabled, returns the stale CLI when version mismatches at the shared data directory path`() { + val url = URL("http://test.coder.invalid") + val s = settings( + DATA_DIRECTORY to testDir("no-dest-dl-off-no-cli-datacli-wrong/data").toString(), + ENABLE_DOWNLOADS to "false", + ) + val fallbackPath = s.binPath(url, true) + createBinary(fallbackPath, "1.0.0") + + val ccm = runBlocking { ensureCLI(ctx(s), url, "2.0.0", noOpTextProgress) } + assertEquals(fallbackPath, ccm.localBinaryPath) + assertEquals(SemVer(1, 0, 0), ccm.version()) + } + + @Test + fun `given no binaryDestination configured and downloads are disabled, throws when no CLI binary exists`() { + val url = URL("http://test.coder.invalid") + val s = settings( + DATA_DIRECTORY to testDir("no-dest-dl-off-both-missing/data").toString(), + ENABLE_DOWNLOADS to "false", + ) + assertFailsWith(IllegalStateException::class) { + runBlocking { ensureCLI(ctx(s), url, "1.0.0", noOpTextProgress) } + } + } + + @Test + @IgnoreOnWindows + fun `given no binaryDestination configured and downloads are enabled, returns the CLI when version already matches`() { + val url = URL("http://test.coder.invalid") + val s = settings( + DATA_DIRECTORY to testDir("no-dest-dl-on-match/data").toString(), + DISABLE_SIGNATURE_VALIDATION to "true", + ) + val expected = s.binPath(url) + createBinary(expected, "1.0.0") + + val ccm = runBlocking { ensureCLI(ctx(s), url, "1.0.0", noOpTextProgress) } + assertEquals(expected, ccm.localBinaryPath) + assertEquals(SemVer(1, 0, 0), ccm.version()) + } + + @Test + @IgnoreOnWindows + fun `given no binaryDestination configured and downloads are enabled, downloads and returns a fresh CLI when version mismatches`() { + val (srv, url) = mockServer(version = "2.0.0") + try { + val s = settings( + DATA_DIRECTORY to testDir("no-dest-dl-on-mismatch/data").toString(), + DISABLE_SIGNATURE_VALIDATION to "true", + ) + val expected = s.binPath(url) + createBinary(expected, "1.0.0") + + val ccm = runBlocking { ensureCLI(ctx(s), url, "2.0.0", noOpTextProgress) } + assertEquals(expected, ccm.localBinaryPath) + assertTrue(ccm.localBinaryPath.toFile().exists()) + assertEquals(SemVer(2, 0, 0), ccm.version()) + } finally { + srv.stop(0) + } + } + + @Test + fun `given no binaryDestination configured and downloads are enabled, propagates non-AccessDenied errors during download to the caller`() { + val (srv, url) = mockServer(errorCode = HttpURLConnection.HTTP_INTERNAL_ERROR) + try { + val s = settings( + DATA_DIRECTORY to testDir("no-dest-dl-on-error/data").toString(), + DISABLE_SIGNATURE_VALIDATION to "true", + ) + assertFailsWith(ResponseException::class) { + runBlocking { ensureCLI(ctx(s), url, "1.0.0", noOpTextProgress) } + } + } finally { + srv.stop(0) + } + } + + @Test + @IgnoreOnWindows + fun `given no binaryDestination configured and downloads are enabled, rethrows AccessDeniedException since the binary path is always inside the data directory`() { + val (srv, url) = mockServer() + val s = settings( + DATA_DIRECTORY to testDir("no-dest-dl-on-access-denied/data").toString(), + DISABLE_SIGNATURE_VALIDATION to "true", + ) + val binParent = s.binPath(url).parent + try { + binParent.toFile().mkdirs() + binParent.toFile().setWritable(false) + assertFailsWith(AccessDeniedException::class) { + runBlocking { ensureCLI(ctx(s), url, "1.0.0", noOpTextProgress) } + } + } finally { + binParent.toFile().setWritable(true) + srv.stop(0) + } + } + + @Test + fun `given no binaryDestination configured and downloads are enabled, downloads and returns a fresh CLI when none exists yet`() { + val (srv, url) = mockServer(version = "1.0.0") + try { + val s = settings( + DATA_DIRECTORY to testDir("no-dest-dl-on-no-cli/data").toString(), + DISABLE_SIGNATURE_VALIDATION to "true", + ) + val expected = s.binPath(url) + + val ccm = runBlocking { ensureCLI(ctx(s), url, "1.0.0", noOpTextProgress) } + assertEquals(expected, ccm.localBinaryPath) + assertTrue(ccm.localBinaryPath.toFile().exists()) + if (getOS() != OS.WINDOWS) { + assertEquals(SemVer(1, 0, 0), ccm.version()) + } + } finally { + srv.stop(0) + } + } + + @Test + fun `given no binaryDestination configured and downloads are enabled, propagates non-AccessDenied errors during download when CLI is missing`() { + val (srv, url) = mockServer(errorCode = HttpURLConnection.HTTP_INTERNAL_ERROR) + try { + val s = settings( + DATA_DIRECTORY to testDir("no-dest-dl-on-no-cli-error/data").toString(), + DISABLE_SIGNATURE_VALIDATION to "true", + ) + assertFailsWith(ResponseException::class) { + runBlocking { ensureCLI(ctx(s), url, "1.0.0", noOpTextProgress) } + } + } finally { + srv.stop(0) + } + } + + @Test + @IgnoreOnWindows + fun `given no binaryDestination configured and downloads are enabled, rethrows AccessDeniedException when CLI is missing and access to the data directory is denied`() { + val (srv, url) = mockServer() + val s = settings( + DATA_DIRECTORY to testDir("no-dest-dl-on-no-cli-access-denied/data").toString(), + DISABLE_SIGNATURE_VALIDATION to "true", + ) + val binParent = s.binPath(url).parent + try { + binParent.toFile().mkdirs() + binParent.toFile().setWritable(false) + assertFailsWith(AccessDeniedException::class) { + runBlocking { ensureCLI(ctx(s), url, "1.0.0", noOpTextProgress) } + } + } finally { + binParent.toFile().setWritable(true) + srv.stop(0) + } + } + + // Utilities + + private fun testDir(id: String): Path = tmpdir.resolve(id) + + private fun mkbin(str: String): String = if (getOS() == OS.WINDOWS) { + listOf("@echo off", str) + } else { + listOf("#!/bin/sh", str) + }.joinToString(System.lineSeparator()) + + private fun echo(str: String): String = if (getOS() == OS.WINDOWS) "echo $str" else "echo '$str'" + + private fun mkbinVersion(version: String): String = mkbin(echo("""{"version": "$version"}""")) + + private fun settings(vararg pairs: Pair): CoderSettingsStore = + CoderSettingsStore(pluginTestSettingsStore(*pairs), Environment(), baseContext.logger) + + private fun ctx(s: CoderSettingsStore): CoderToolboxContext = baseContext.copy(settingsStore = s) + + private fun createBinary(path: Path, version: String) { + path.parent.toFile().mkdirs() + path.toFile().writeText(mkbinVersion(version)) + if (getOS() != OS.WINDOWS) { + path.toFile().setExecutable(true) + } + } + + private fun mockServer( + errorCode: Int = 0, + version: String? = null, + ): Pair { + val srv = HttpServer.create(InetSocketAddress(0), 0) + srv.createContext("/") { exchange -> + var code = HttpURLConnection.HTTP_OK + var response = mkbinVersion(version ?: "${srv.address.port}.0.0") + if (exchange.requestURI.path.contains(".asc")) { + code = HttpURLConnection.HTTP_NOT_FOUND + response = "not found" + } else if (!exchange.requestURI.path.startsWith("/bin/coder-")) { + code = HttpURLConnection.HTTP_NOT_FOUND + response = "not found" + } else if (errorCode != 0) { + code = errorCode + response = "error code $code" + } else { + val eTags = exchange.requestHeaders["If-None-Match"] + if (eTags != null && eTags.contains("\"${sha1(response.byteInputStream())}\"")) { + code = HttpURLConnection.HTTP_NOT_MODIFIED + response = "not modified" + } + } + val body = response.toByteArray() + exchange.responseHeaders["Content-Type"] = "application/octet-stream" + exchange.sendResponseHeaders(code, if (code == HttpURLConnection.HTTP_OK) body.size.toLong() else -1) + exchange.responseBody.write(body) + exchange.close() + } + srv.start() + return Pair(srv, URL("http://localhost:" + srv.address.port)) + } + + /** Build the host directory component used by [CoderSettingsStore.withHost]. */ + private fun hostDir(url: URL): String = + if (url.port > 0) "${url.host}-${url.port}" else url.host + + companion object { + private val tmpdir: Path = Path.of(System.getProperty("java.io.tmpdir")) + .resolve("coder-toolbox-test").resolve("ensure-cli") + + @JvmStatic + @BeforeAll + fun cleanup() { + tmpdir.toFile().deleteRecursively() + } + } +} diff --git a/src/test/kotlin/com/coder/toolbox/util/IgnoreOnWindows.kt b/src/test/kotlin/com/coder/toolbox/util/IgnoreOnWindows.kt new file mode 100644 index 00000000..766af4dc --- /dev/null +++ b/src/test/kotlin/com/coder/toolbox/util/IgnoreOnWindows.kt @@ -0,0 +1,9 @@ +package com.coder.toolbox.util + +import org.junit.jupiter.api.condition.DisabledOnOs +import org.junit.jupiter.api.condition.OS as JUnitOS + +@Target(AnnotationTarget.FUNCTION, AnnotationTarget.CLASS) +@Retention(AnnotationRetention.RUNTIME) +@DisabledOnOs(JUnitOS.WINDOWS) +annotation class IgnoreOnWindows From 3caa5baa0caa67eb45ad0af04bcb4a7b4f9c615e Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Sat, 28 Mar 2026 01:09:42 +0200 Subject: [PATCH 11/14] fix: data dir already includes the hostname --- .../coder/toolbox/store/CoderSettingsStore.kt | 2 +- .../toolbox/settings/CoderSettingsTest.kt | 6 +- .../toolbox/store/CoderSettingsStoreTest.kt | 66 +++++++++++-------- 3 files changed, 42 insertions(+), 32 deletions(-) diff --git a/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt b/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt index 334daae4..44522528 100644 --- a/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt +++ b/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt @@ -149,7 +149,7 @@ class CoderSettingsStore( val isExecutable = Files.isRegularFile(dest, LinkOption.NOFOLLOW_LINKS) && Files.isExecutable(dest) if (forceDownloadToData) { - return withHost(dataDir(url), url).resolve(defaultCliBinaryNameByOsAndArch).toAbsolutePath() + return dataDir(url).resolve(defaultCliBinaryNameByOsAndArch).toAbsolutePath() } if (isExecutable) { return dest.toAbsolutePath() diff --git a/src/test/kotlin/com/coder/toolbox/settings/CoderSettingsTest.kt b/src/test/kotlin/com/coder/toolbox/settings/CoderSettingsTest.kt index b7bd44a1..911b12fd 100644 --- a/src/test/kotlin/com/coder/toolbox/settings/CoderSettingsTest.kt +++ b/src/test/kotlin/com/coder/toolbox/settings/CoderSettingsTest.kt @@ -116,16 +116,16 @@ internal class CoderSettingsTest { val settings = CoderSettingsStore(pluginTestSettingsStore(), Environment(), logger) // The binary path should fall back to the data directory but that is // already tested in the data directory tests. - val url = URL("http://localhost") + val url = URL("http://test.coder.com") // Override with settings. settings.updateBinaryDirectory("/tmp/coder-toolbox-test/bin-dir") - var expected = "/tmp/coder-toolbox-test/bin-dir/localhost" + var expected = "/tmp/coder-toolbox-test/bin-dir/test.coder.com" assertEquals(Path.of(expected).toAbsolutePath(), settings.readOnly().binPath(url).parent) // Second argument bypasses override. settings.updateDataDirectory("/tmp/coder-toolbox-test/data-dir") - expected = "/tmp/coder-toolbox-test/data-dir/localhost" + expected = "/tmp/coder-toolbox-test/data-dir/test.coder.com" assertEquals(Path.of(expected).toAbsolutePath(), settings.readOnly().binPath(url, true).parent) // Binary name is always determined by OS and architecture. diff --git a/src/test/kotlin/com/coder/toolbox/store/CoderSettingsStoreTest.kt b/src/test/kotlin/com/coder/toolbox/store/CoderSettingsStoreTest.kt index d46c48e9..0158c83e 100644 --- a/src/test/kotlin/com/coder/toolbox/store/CoderSettingsStoreTest.kt +++ b/src/test/kotlin/com/coder/toolbox/store/CoderSettingsStoreTest.kt @@ -8,6 +8,7 @@ import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Assertions.assertFalse import org.junit.jupiter.api.Assertions.assertTrue import java.net.URL +import java.nio.file.Files import java.nio.file.Path import kotlin.test.AfterTest import kotlin.test.BeforeTest @@ -110,45 +111,54 @@ class CoderSettingsStoreTest { } @Test - fun `binPath returns absolute binaryDestination path when downloads are disabled`() { - val settings = storeWith( - BINARY_DESTINATION to "/usr/local/bin/coder", - ENABLE_DOWNLOADS to "false" - ) - val url = URL("https://test.coder.com") - val result = settings.binPath(url) + fun `binPath returns absolute binaryDestination path when it points to an existing executable`() { + val tmpBin = Files.createTempFile("coder-test", null) + tmpBin.toFile().setExecutable(true) + try { + val settings = storeWith(BINARY_DESTINATION to tmpBin.toString()) + val url = URL("https://test.coder.com") + val result = settings.binPath(url) - assertEquals(Path.of("/usr/local/bin/coder").toAbsolutePath(), result) + assertEquals(tmpBin.toAbsolutePath(), result) + } finally { + Files.deleteIfExists(tmpBin) + } } @Test - fun `binPath expands tilde in binaryDestination when downloads are disabled`() { + fun `binPath expands tilde in binaryDestination when it points to an existing executable`() { // Don't override OS — tilde expansion depends on the real File.separator. - val settings = storeWith( - BINARY_DESTINATION to "~/bin/coder", - ENABLE_DOWNLOADS to "false" - ) - val url = URL("https://test.coder.com") - val result = settings.binPath(url) val home = Path.of(System.getProperty("user.home")) - - assertTrue(result.isAbsolute) - assertEquals(home.resolve("bin").resolve("coder").toAbsolutePath(), result) + val tmpBin = Files.createTempFile(home, "coder-test", null) + tmpBin.toFile().setExecutable(true) + try { + val settings = storeWith(BINARY_DESTINATION to "~/${tmpBin.fileName}") + val url = URL("https://test.coder.com") + val result = settings.binPath(url) + + assertTrue(result.isAbsolute) + assertEquals(tmpBin.toAbsolutePath(), result) + } finally { + Files.deleteIfExists(tmpBin) + } } @Test - fun `binPath expands HOME in binaryDestination when downloads are disabled`() { + fun `binPath expands HOME in binaryDestination when it points to an existing executable`() { // Don't override OS — $HOME expansion depends on the real File.separator. - val settings = storeWith( - BINARY_DESTINATION to "\$HOME/bin/coder", - ENABLE_DOWNLOADS to "false" - ) - val url = URL("https://test.coder.com") - val result = settings.binPath(url) val home = Path.of(System.getProperty("user.home")) - - assertTrue(result.isAbsolute) - assertEquals(home.resolve("bin").resolve("coder").toAbsolutePath(), result) + val tmpBin = Files.createTempFile(home, "coder-test", null) + tmpBin.toFile().setExecutable(true) + try { + val settings = storeWith(BINARY_DESTINATION to "\$HOME/${tmpBin.fileName}") + val url = URL("https://test.coder.com") + val result = settings.binPath(url) + + assertTrue(result.isAbsolute) + assertEquals(tmpBin.toAbsolutePath(), result) + } finally { + Files.deleteIfExists(tmpBin) + } } @Test From 7e6e8d0f0b4723ba9695865be3578b90f5b591e3 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Mon, 30 Mar 2026 21:41:11 +0300 Subject: [PATCH 12/14] test: some tests can't run on Windows (1) Without GIT bash utilities installed. To remove the dependency I duplicated some of the tests and used Windows commands only. --- .../coder/toolbox/cli/CoderCLIManagerTest.kt | 20 ++++-- .../com/coder/toolbox/util/HeadersTest.kt | 63 ++++++++++++++++++- .../com/coder/toolbox/util/IgnoreOnUnix.kt | 9 +++ 3 files changed, 87 insertions(+), 5 deletions(-) create mode 100644 src/test/kotlin/com/coder/toolbox/util/IgnoreOnUnix.kt diff --git a/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt b/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt index 6cb50f35..d0caf714 100644 --- a/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt +++ b/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt @@ -827,9 +827,15 @@ internal class CoderCLIManagerTest { context.copy( settingsStore = CoderSettingsStore( if (getOS() == OS.WINDOWS) { + // Pre-create the .bat file so binPath detects it as an existing + // executable and returns it directly rather than treating it as + // a base directory (which would produce a .exe path that Windows + // cannot run as a batch script). + val batPath = tmpdir.resolve("bad-version").resolve("coder.bat") + batPath.parent.toFile().mkdirs() + batPath.toFile().createNewFile() pluginTestSettingsStore( - BINARY_DESTINATION to tmpdir.resolve("bad-version").resolve("coder.bat").toString(), - ENABLE_DOWNLOADS to "false", + BINARY_DESTINATION to batPath.toString(), ) } else { pluginTestSettingsStore( @@ -887,9 +893,15 @@ internal class CoderCLIManagerTest { context.copy( settingsStore = CoderSettingsStore( if (getOS() == OS.WINDOWS) { + // Pre-create the .bat file so binPath detects it as an existing + // executable and returns it directly rather than treating it as + // a base directory (which would produce a .exe path that Windows + // cannot run as a batch script). + val batPath = tmpdir.resolve("matches-version").resolve("coder.bat") + batPath.parent.toFile().mkdirs() + batPath.toFile().createNewFile() pluginTestSettingsStore( - BINARY_DESTINATION to tmpdir.resolve("matches-version").resolve("coder.bat").toString(), - ENABLE_DOWNLOADS to "false", + BINARY_DESTINATION to batPath.toString(), ) } else { pluginTestSettingsStore( diff --git a/src/test/kotlin/com/coder/toolbox/util/HeadersTest.kt b/src/test/kotlin/com/coder/toolbox/util/HeadersTest.kt index 9df68bc6..c3b75d2b 100644 --- a/src/test/kotlin/com/coder/toolbox/util/HeadersTest.kt +++ b/src/test/kotlin/com/coder/toolbox/util/HeadersTest.kt @@ -8,6 +8,7 @@ import kotlin.test.assertFailsWith internal class HeadersTest { @Test + @IgnoreOnWindows fun testGetHeadersOK() { val tests = mapOf( @@ -35,6 +36,7 @@ internal class HeadersTest { } @Test + @IgnoreOnWindows fun testGetHeadersFail() { val tests = mapOf( @@ -61,11 +63,70 @@ internal class HeadersTest { } } + @Test + @IgnoreOnUnix + fun testGetHeadersOKOnWindows() { + val tests = + mapOf( + null to emptyMap(), + "" to emptyMap(), + // No spaces around && to avoid trailing-space artifacts from cmd echo. + "echo foo=bar&&echo baz=qux" to mapOf("foo" to "bar", "baz" to "qux"), + "echo foo=bar" to mapOf("foo" to "bar"), + "echo foo=bar=" to mapOf("foo" to "bar="), + "echo foo=bar=baz" to mapOf("foo" to "bar=baz"), + "echo foo=" to mapOf("foo" to ""), + // type nul outputs 0 bytes → treated as empty output → empty map. + "type nul" to mapOf(), + "exit /b 0" to mapOf(), + // >&2 redirects stdout to stderr; stdout stays empty. + "echo ignore me>&2" to mapOf(), + "echo foo=bar&&echo ignore me>&2" to mapOf("foo" to "bar"), + ) + tests.forEach { + assertEquals( + it.value, + getHeaders(URL("http://localhost"), it.key), + ) + } + } + + @Test + @IgnoreOnUnix + fun testGetHeadersFailOnWindows() { + val tests = + mapOf( + "echo =foo" to "Header name is missing in \"=foo\"", + "echo foo" to "Header \"foo\" does not have two parts", + // cmd.exe strips one space after the command name, so three spaces → two spaces in output. + "echo =foo" to "Header name is missing in \" =foo\"", + "echo foo =bar" to "Header name cannot contain spaces, got \"foo \"", + "echo foo foo=bar" to "Header name cannot contain spaces, got \"foo foo\"", + "echo foo=bar " to "Header name cannot contain spaces, got \" foo\"", + "exit /b 1" to "Unexpected exit value: 1", + // "foobar" appears in the InvalidExitValueException message as part of the command string. + "echo foobar>&2&&exit /b 1" to "foobar", + // echo. outputs a bare CRLF; a blank line anywhere in the output is an error. + "echo foo=bar&&echo." to "Blank lines are not allowed", + "echo.&&echo foo=bar" to "Blank lines are not allowed", + "echo." to "Blank lines are not allowed", + "echo f=b&&echo.&&echo b=q" to "Blank lines are not allowed", + ) + tests.forEach { + val ex = + assertFailsWith( + exceptionClass = Exception::class, + block = { getHeaders(URL("http://localhost"), it.key) }, + ) + assertContains(ex.message.toString(), it.value) + } + } + @Test fun testSetsEnvironment() { val headers = if (getOS() == OS.WINDOWS) { - getHeaders(URL("http://localhost12345"), "printf url=%CODER_URL%") + getHeaders(URL("http://localhost12345"), "echo url=%CODER_URL%") } else { getHeaders(URL("http://localhost12345"), "printf url=\$CODER_URL") } diff --git a/src/test/kotlin/com/coder/toolbox/util/IgnoreOnUnix.kt b/src/test/kotlin/com/coder/toolbox/util/IgnoreOnUnix.kt new file mode 100644 index 00000000..d6ead63d --- /dev/null +++ b/src/test/kotlin/com/coder/toolbox/util/IgnoreOnUnix.kt @@ -0,0 +1,9 @@ +package com.coder.toolbox.util + +import org.junit.jupiter.api.condition.DisabledOnOs +import org.junit.jupiter.api.condition.OS as JUnitOS + +@Target(AnnotationTarget.FUNCTION, AnnotationTarget.CLASS) +@Retention(AnnotationRetention.RUNTIME) +@DisabledOnOs(JUnitOS.LINUX, JUnitOS.MAC) +annotation class IgnoreOnUnix From e463e86c0f4785de2604e9819eb611b8f9228261 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Tue, 31 Mar 2026 20:48:52 +0300 Subject: [PATCH 13/14] fix: follow symlinks in CLI resolution --- .../coder/toolbox/store/CoderSettingsStore.kt | 3 +-- .../com/coder/toolbox/cli/EnsureCLITest.kt | 26 +++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt b/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt index 44522528..35a2c51f 100644 --- a/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt +++ b/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt @@ -17,7 +17,6 @@ import com.jetbrains.toolbox.api.core.PluginSettingsStore import com.jetbrains.toolbox.api.core.diagnostics.Logger import java.net.URL import java.nio.file.Files -import java.nio.file.LinkOption import java.nio.file.Path import java.nio.file.Paths @@ -146,7 +145,7 @@ class CoderSettingsStore( } val dest = Path.of(expand(binaryDestination!!)) - val isExecutable = Files.isRegularFile(dest, LinkOption.NOFOLLOW_LINKS) && Files.isExecutable(dest) + val isExecutable = Files.isRegularFile(dest) && Files.isExecutable(dest) if (forceDownloadToData) { return dataDir(url).resolve(defaultCliBinaryNameByOsAndArch).toAbsolutePath() diff --git a/src/test/kotlin/com/coder/toolbox/cli/EnsureCLITest.kt b/src/test/kotlin/com/coder/toolbox/cli/EnsureCLITest.kt index a922fb58..a35ab4df 100644 --- a/src/test/kotlin/com/coder/toolbox/cli/EnsureCLITest.kt +++ b/src/test/kotlin/com/coder/toolbox/cli/EnsureCLITest.kt @@ -39,6 +39,7 @@ import java.net.Proxy import java.net.ProxySelector import java.net.URL import java.nio.file.AccessDeniedException +import java.nio.file.Files import java.nio.file.Path import kotlin.test.BeforeTest import kotlin.test.Test @@ -578,6 +579,31 @@ internal class EnsureCLITest { } } + @Test + @IgnoreOnWindows + fun `given binaryDestination is a symlink to an existing executable, uses the symlink target directly`() { + val url = URL("http://test.coder.invalid") + + // Simulate e.g. /usr/local/bin/coder being a symlink created by a package manager. + val realBinary = testDir("symlink-dest/real/my-coder") + createBinary(realBinary, "1.0.0") + val symlinkDest = testDir("symlink-dest/link/my-coder") + symlinkDest.parent.toFile().mkdirs() + Files.createSymbolicLink(symlinkDest, realBinary.toAbsolutePath()) + + val s = settings( + BINARY_DESTINATION to symlinkDest.toString(), + DATA_DIRECTORY to testDir("symlink-dest/data").toString(), + ENABLE_DOWNLOADS to "false", + ) + + assertEquals(symlinkDest.toAbsolutePath(), s.binPath(url)) + + val ccm = runBlocking { ensureCLI(ctx(s), url, "1.0.0", noOpTextProgress) } + assertEquals(symlinkDest.toAbsolutePath(), ccm.localBinaryPath) + assertEquals(SemVer(1, 0, 0), ccm.version()) + } + @Test @IgnoreOnWindows fun `given binaryDestination is a directory and downloads are disabled, returns the CLI at destination when version matches`() { From 1e0d31cd57face8d2763c03ad02d7aeac7b04712 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Wed, 1 Apr 2026 22:43:14 +0300 Subject: [PATCH 14/14] fix: rename property setter And use the correct key, instead of the deprecated one. --- .../kotlin/com/coder/toolbox/store/CoderSettingsStore.kt | 4 ++-- .../kotlin/com/coder/toolbox/views/CoderSettingsPage.kt | 8 ++++---- .../com/coder/toolbox/settings/CoderSettingsTest.kt | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt b/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt index 35a2c51f..1dcc8959 100644 --- a/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt +++ b/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt @@ -194,8 +194,8 @@ class CoderSettingsStore( store[BINARY_SOURCE] = source } - fun updateBinaryDirectory(dir: String) { - store[BINARY_DIRECTORY] = dir + fun updateBinaryDestination(dest: String) { + store[BINARY_DESTINATION] = dest } fun updateDataDirectory(dir: String) { diff --git a/src/main/kotlin/com/coder/toolbox/views/CoderSettingsPage.kt b/src/main/kotlin/com/coder/toolbox/views/CoderSettingsPage.kt index 367ae3bb..28a40f39 100644 --- a/src/main/kotlin/com/coder/toolbox/views/CoderSettingsPage.kt +++ b/src/main/kotlin/com/coder/toolbox/views/CoderSettingsPage.kt @@ -38,7 +38,7 @@ class CoderSettingsPage( // TODO: Copy over the descriptions, holding until I can test this page. private val binarySourceField = TextField(context.i18n.ptrl("Binary source"), settings.binarySource ?: "", TextType.General) - private val binaryDirectoryField = + private val binaryDestinationField = TextField(context.i18n.ptrl("Binary destination"), settings.binaryDestination ?: "", TextType.General) private val dataDirectoryField = TextField(context.i18n.ptrl("Data directory"), settings.dataDirectory ?: "", TextType.General) @@ -131,7 +131,7 @@ class CoderSettingsPage( binarySourceField, enableDownloadsField, useAppNameField, - binaryDirectoryField, + binaryDestinationField, enableBinaryDirectoryFallbackField, disableSignatureVerificationField, signatureFallbackStrategyField, @@ -156,7 +156,7 @@ class CoderSettingsPage( Action(context, "Save", closesPage = true) { with(context.settingsStore) { updateBinarySource(binarySourceField.contentState.value) - updateBinaryDirectory(binaryDirectoryField.contentState.value) + updateBinaryDestination(binaryDestinationField.contentState.value) updateDataDirectory(dataDirectoryField.contentState.value) updateEnableDownloads(enableDownloadsField.checkedState.value) updateUseAppNameAsTitle(useAppNameField.checkedState.value) @@ -200,7 +200,7 @@ class CoderSettingsPage( binarySourceField.contentState.update { settings.binarySource ?: "" } - binaryDirectoryField.contentState.update { + binaryDestinationField.contentState.update { settings.binaryDestination ?: "" } dataDirectoryField.contentState.update { diff --git a/src/test/kotlin/com/coder/toolbox/settings/CoderSettingsTest.kt b/src/test/kotlin/com/coder/toolbox/settings/CoderSettingsTest.kt index 911b12fd..b57354c0 100644 --- a/src/test/kotlin/com/coder/toolbox/settings/CoderSettingsTest.kt +++ b/src/test/kotlin/com/coder/toolbox/settings/CoderSettingsTest.kt @@ -33,7 +33,7 @@ internal class CoderSettingsTest { val url = URL("http://localhost") val home = Path.of(System.getProperty("user.home")) - settings.updateBinaryDirectory(Path.of("~/coder-toolbox-test/expand-bin-dir").toString()) + settings.updateBinaryDestination(Path.of("~/coder-toolbox-test/expand-bin-dir").toString()) var expected = home.resolve("coder-toolbox-test/expand-bin-dir/localhost") assertEquals(expected.toAbsolutePath(), settings.readOnly().binPath(url).parent) @@ -119,7 +119,7 @@ internal class CoderSettingsTest { val url = URL("http://test.coder.com") // Override with settings. - settings.updateBinaryDirectory("/tmp/coder-toolbox-test/bin-dir") + settings.updateBinaryDestination("/tmp/coder-toolbox-test/bin-dir") var expected = "/tmp/coder-toolbox-test/bin-dir/test.coder.com" assertEquals(Path.of(expected).toAbsolutePath(), settings.readOnly().binPath(url).parent)