Skip to content

Commit e23c432

Browse files
authored
impl: remove support for data dir fallback setting (#287)
This PR is a result of discussion that happened in the #286. Basically it as an attempt of simplifying the CLI resolution and trying to be more aligned with the VS Code extension. The existing implementation was too cumbersome to understand, brittle and a lot of tedious work needed to solve all of it's usecases. This PR removes the enable fallback to data dir setting, which was used only for access denied exceptions. The CLI resolution uses the binary destination if it was configured by the user, or it automatically falls back to data dir if binary destination was not configured. The implementation respects the user's choice and no longer tries to make smart choices on behalf of the user. For example if the user configured the binary destination but disabled downloads we just prompt him with an error instead of trying to fall on data dir. There were a couple of other small improvements left from the previous PR that are now addressed. - resolves #285
1 parent 015ac13 commit e23c432

15 files changed

Lines changed: 94 additions & 719 deletions

File tree

CHANGELOG.md

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,13 @@
44

55
### Added
66

7-
- support for using a locally provided CLI binary when downloads are disabled
7+
- `Binary destination` can now point directly to an executable, used as-is; otherwise it is treated as a base directory
8+
as before
9+
10+
### Removed
11+
12+
- support for enabling or disabling the fallback to data dir. CLI resolution will always fall back on data dir if binary
13+
destination is not configured.
814

915
## 0.8.6 - 2026-03-05
1016

@@ -17,7 +23,8 @@
1723
### Added
1824

1925
- support for configuring the SSH connection timeout, defaults to 10 seconds
20-
- enhanced IDE resolution by supporting latest EAP, latest release, latest installed labels with clear fallback behavior in URI handlers
26+
- enhanced IDE resolution by supporting latest EAP, latest release, latest installed labels with clear fallback behavior
27+
in URI handlers
2128

2229
### Fixed
2330

README.md

Lines changed: 19 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -447,19 +447,16 @@ storage paths. The options can be configured from the plugin's main Workspaces p
447447
- `Binary source` specifies the source URL or relative path from which the Coder CLI should be downloaded.
448448
If a relative path is provided, it is resolved against the deployment domain.
449449

450-
- `Enable downloads` allows automatic downloading of the CLI if the current version is missing or outdated.
451-
Defaults to enabled.
450+
- `Enable downloads` allows automatic downloading of the CLI if the current version is missing or outdated. Enabled by
451+
default.
452452

453453
- `Binary destination` specifies where the CLI binary is placed. This can be a path to an existing
454454
executable (used as-is) or a base directory (the CLI is placed under a host-specific subdirectory).
455455
If blank, the data directory is used. Supports `~` and `$HOME` expansion.
456456

457-
- `Enable binary directory fallback` when enabled, if the binary destination is not writable the
458-
plugin falls back to the data directory instead of failing. Only takes effect when downloads are
459-
enabled and the binary destination differs from the data directory. Defaults to disabled.
460-
461457
- `Data directory` directory where deployment-specific data such as session tokens and CLI binaries
462-
are stored. Each deployment gets a host-specific subdirectory (e.g. `coder.example.com`).
458+
are stored. Each deployment gets a host-specific subdirectory (e.g. `coder.example.com`). Supports `~` and `$HOME`
459+
expansion.
463460

464461
- `Header command` command that outputs additional HTTP headers. Each line of output must be in the format key=value.
465462
The environment variable CODER_URL will be available to the command process.
@@ -477,20 +474,21 @@ storage paths. The options can be configured from the plugin's main Workspaces p
477474
#### How CLI resolution works
478475

479476
When connecting to a deployment the plugin ensures a compatible CLI binary is available.
480-
The settings above interact as follows:
481-
482-
1. If a CLI already exists at the binary destination and its version matches the deployment, it is
483-
used immediately.
484-
2. If **downloads are enabled**, the plugin downloads the matching version to the binary destination.
485-
- If the download fails with a permission error and **binary directory fallback** is enabled (and
486-
the binary destination is not already in the data directory), the plugin checks whether the data
487-
directory already has a matching CLI. If so it is used; otherwise the plugin downloads to the
488-
data directory instead.
489-
- Any other download error is reported to the user.
490-
3. If **downloads are disabled**, the plugin checks the data directory for a CLI whose version
491-
matches the deployment. If no exact match is found anywhere, whichever CLI is available is
492-
returned — preferring the binary destination unless it is missing, in which case the data
493-
directory CLI is used regardless of its version. If no CLI exists at all, an error is raised.
477+
The binary location is resolved as follows:
478+
479+
- If **binary destination** points to an existing executable file, it is used as-is.
480+
- If **binary destination** is set but is not an executable file, it is treated as a base
481+
directory and the CLI is placed under a host-specific subdirectory (e.g.
482+
`<binary destination>/coder.example.com/<default-cli-name>`).
483+
- If **binary destination** is not set, the data directory is used instead.
484+
485+
Once the binary location is resolved:
486+
487+
1. If a CLI already exists there and its version matches the deployment, it is used immediately.
488+
2. Otherwise, if **downloads are enabled**, the plugin downloads the matching version to that location.
489+
Any download error is reported to the user.
490+
3. If **downloads are disabled** and the CLI exists but its version does not match, the stale
491+
CLI is used with a warning. If no CLI exists at all, an error is raised.
494492

495493
### TLS settings
496494

src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt

Lines changed: 14 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -48,24 +48,17 @@ internal data class Version(
4848

4949

5050
/**
51-
* Do as much as possible to get a valid, up-to-date CLI.
51+
* Best effort to get an up-to-date CLI.
5252
*
5353
* 1. Create a CLI manager for the deployment URL.
5454
* 2. If the CLI version matches the build version, return it immediately.
55-
* 3. If downloads are enabled, attempt to download the CLI.
55+
* 3. Otherwise, if downloads are enabled, attempt to download the CLI.
5656
* a. On success, return the CLI.
57-
* b. On [java.nio.file.AccessDeniedException]: rethrow if the binary
58-
* path parent equals the data directory or if binary directory
59-
* fallback is disabled. Otherwise, if the fallback data directory
60-
* CLI already matches the build version return it; if not, download
61-
* to the data directory and return the fallback CLI.
62-
* c. Any other exception propagates to the caller.
57+
* b. Any exception propagates to the user.
6358
* 4. If downloads are disabled:
64-
* a. If the data directory CLI version matches, return it.
65-
* b. If neither the configured binary nor the data directory CLI can
66-
* report a version, throw [IllegalStateException].
67-
* c. Prefer the configured binary; fall back to the data directory CLI
68-
* only when the configured binary is missing or unexecutable.
59+
* a. [IllegalStateException] is raised if the CLI does not exist (look into binary destination if it was configured,
60+
* fallback to data dir otherwise)
61+
* b. Otherwise, warn the user and return the mismatched version.
6962
*/
7063
suspend fun ensureCLI(
7164
context: CoderToolboxContext,
@@ -95,44 +88,15 @@ suspend fun ensureCLI(
9588
// If downloads are enabled download the new version.
9689
if (settings.enableDownloads) {
9790
reportProgress("Downloading Coder CLI...")
98-
try {
99-
cli.download(buildVersion, showTextProgress)
100-
return cli
101-
} catch (e: java.nio.file.AccessDeniedException) {
102-
// Might be able to fall back to the data directory.
103-
val binPath = settings.binPath(deploymentURL)
104-
val dataDir = settings.dataDir(deploymentURL)
105-
if (binPath.parent == dataDir || !settings.enableBinaryDirectoryFallback) {
106-
throw e
107-
}
108-
// fall back to the data directory.
109-
val fallbackCLI = CoderCLIManager(context, deploymentURL, true)
110-
val fallbackMatches = fallbackCLI.matchesVersion(buildVersion)
111-
if (fallbackMatches == true) {
112-
reportProgress("Local CLI version from data directory matches server version: $buildVersion")
113-
return fallbackCLI
114-
}
115-
116-
reportProgress("Downloading Coder CLI to the data directory...")
117-
fallbackCLI.download(buildVersion, showTextProgress)
118-
return fallbackCLI
119-
}
120-
}
121-
122-
// Try falling back to the data directory.
123-
val dataCLI = CoderCLIManager(context, deploymentURL, true)
124-
val dataCLIMatches = dataCLI.matchesVersion(buildVersion)
125-
if (dataCLIMatches == true) {
126-
reportProgress("Local CLI version from data directory matches server version: $buildVersion")
127-
return dataCLI
91+
cli.download(buildVersion, showTextProgress)
92+
return cli
12893
}
12994

130-
// Prefer the binary directory unless the data directory has a
131-
// working binary and the binary directory does not.
132-
if (cliMatches == null && dataCLIMatches == null && !settings.enableDownloads) {
95+
if (cliMatches == null) {
13396
throw IllegalStateException("Can't resolve Coder CLI and downloads are disabled")
13497
}
135-
return if (cliMatches == null && dataCLIMatches != null) dataCLI else cli
98+
reportProgress("Downloads are disabled, and a cached CLI is used which does not match the server version $buildVersion and could cause compatibility issues")
99+
return cli
136100
}
137101

138102
/**
@@ -151,16 +115,13 @@ data class Features(
151115
class CoderCLIManager(
152116
private val context: CoderToolboxContext,
153117
// The URL of the deployment this CLI is for.
154-
private val deploymentURL: URL,
155-
// If the binary directory is not writable, this can be used to force the
156-
// manager to download to the data directory instead.
157-
private val forceDownloadToData: Boolean = false,
118+
private val deploymentURL: URL
158119
) {
159120
private val downloader = createDownloadService()
160121
private val gpgVerifier = GPGVerifier(context)
161122

162123
val remoteBinaryURL: URL = context.settingsStore.binSource(deploymentURL)
163-
val localBinaryPath: Path = context.settingsStore.binPath(deploymentURL, forceDownloadToData)
124+
val localBinaryPath: Path = context.settingsStore.binPath(deploymentURL)
164125
val coderConfigPath: Path = context.settingsStore.dataDir(deploymentURL).resolve("config")
165126

166127
private fun createDownloadService(): CoderDownloadService {
@@ -180,7 +141,7 @@ class CoderCLIManager(
180141
.build()
181142

182143
val service = retrofit.create(CoderDownloadApi::class.java)
183-
return CoderDownloadService(context, service, deploymentURL, forceDownloadToData)
144+
return CoderDownloadService(context, service, deploymentURL)
184145
}
185146

186147
/**

src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadService.kt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,17 +38,17 @@ private val SUPPORTED_BIN_MIME_TYPES = listOf(
3838
"application/x-ms-dos-executable",
3939
"application/vnd.microsoft.portable-executable"
4040
)
41+
4142
/**
4243
* Handles the download steps of Coder CLI
4344
*/
4445
class CoderDownloadService(
4546
private val context: CoderToolboxContext,
4647
private val downloadApi: CoderDownloadApi,
47-
private val deploymentUrl: URL,
48-
forceDownloadToData: Boolean,
48+
private val deploymentUrl: URL
4949
) {
5050
private val remoteBinaryURL: URL = context.settingsStore.binSource(deploymentUrl)
51-
private val cliFinalDst: Path = context.settingsStore.binPath(deploymentUrl, forceDownloadToData)
51+
private val cliFinalDst: Path = context.settingsStore.binPath(deploymentUrl)
5252
private val cliTempDst: Path = cliFinalDst.resolveSibling("${cliFinalDst.name}.tmp")
5353

5454
suspend fun downloadCli(buildVersion: String, showTextProgress: (String) -> Unit): DownloadResult {

src/main/kotlin/com/coder/toolbox/settings/ReadOnlyCoderSettings.kt

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -90,12 +90,6 @@ interface ReadOnlyCoderSettings {
9090
*/
9191
val enableDownloads: Boolean
9292

93-
/**
94-
* Whether to allow the plugin to fall back to the data directory when the
95-
* CLI directory is not writable.
96-
*/
97-
val enableBinaryDirectoryFallback: Boolean
98-
9993
/**
10094
* An external command that outputs additional HTTP headers added to all
10195
* requests. The command must output each header as `key=value` on its own
@@ -179,9 +173,9 @@ interface ReadOnlyCoderSettings {
179173
fun binSource(url: URL): URL
180174

181175
/**
182-
* To where the specified deployment should download the binary.
176+
* Returns a path to where the specified deployment should place the CLI binary.
183177
*/
184-
fun binPath(url: URL, forceDownloadToData: Boolean = false): Path
178+
fun binPath(url: URL): Path
185179

186180
/**
187181
* Return the URL and token from the config, if they exist.

src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt

Lines changed: 9 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,6 @@ class CoderSettingsStore(
5454
override val globalDataDirectory: String get() = getDefaultGlobalDataDir().normalize().toString()
5555
override val globalConfigDir: String get() = getDefaultGlobalConfigDir().normalize().toString()
5656
override val enableDownloads: Boolean get() = store[ENABLE_DOWNLOADS]?.toBooleanStrictOrNull() ?: true
57-
override val enableBinaryDirectoryFallback: Boolean
58-
get() = store[ENABLE_BINARY_DIR_FALLBACK]?.toBooleanStrictOrNull() ?: false
5957
override val headerCommand: String? get() = store[HEADER_COMMAND]
6058
override val tls: ReadOnlyTLSSettings
6159
get() = TLSSettings(
@@ -127,30 +125,19 @@ class CoderSettingsStore(
127125
*
128126
* Resolution logic:
129127
* 1. If [binaryDestination] is null/blank, return the deployment's data
130-
* directory with the default CLI binary name. [forceDownloadToData]
131-
* is ignored because both paths resolve to the same location.
132-
* 2. If [forceDownloadToData] is true, return a host-specific subdirectory
133-
* under the deployment's data directory with the default CLI binary name.
134-
* 3. If the expanded (~ and $HOME) [binaryDestination] is an existing executable file,
128+
* directory with the default CLI binary name.
129+
* 2. If the expanded (~ and $HOME) [binaryDestination] is an existing executable file,
135130
* return it as-is.
136-
* 4. Otherwise, treat [binaryDestination] as a base directory and return a
131+
* 3. Otherwise, treat [binaryDestination] as a base directory and return a
137132
* host-specific subdirectory with the default CLI binary name.
138133
*/
139-
override fun binPath(
140-
url: URL,
141-
forceDownloadToData: Boolean,
142-
): Path {
143-
if (binaryDestination.isNullOrBlank()) {
144-
return dataDir(url).resolve(defaultCliBinaryNameByOsAndArch).toAbsolutePath()
145-
}
146-
147-
val dest = Path.of(expand(binaryDestination!!))
148-
val isExecutable = Files.isRegularFile(dest) && Files.isExecutable(dest)
134+
override fun binPath(url: URL): Path {
135+
val raw = binaryDestination?.takeIf { it.isNotBlank() } ?: return dataDir(url).resolve(
136+
defaultCliBinaryNameByOsAndArch
137+
).toAbsolutePath()
149138

150-
if (forceDownloadToData) {
151-
return dataDir(url).resolve(defaultCliBinaryNameByOsAndArch).toAbsolutePath()
152-
}
153-
if (isExecutable) {
139+
val dest = Path.of(expand(raw))
140+
if (Files.isRegularFile(dest) && Files.isExecutable(dest)) {
154141
return dest.toAbsolutePath()
155142
}
156143
return withHost(dest, url).resolve(defaultCliBinaryNameByOsAndArch).toAbsolutePath()
@@ -222,10 +209,6 @@ class CoderSettingsStore(
222209
store[HTTP_CLIENT_LOG_LEVEL] = level.toString()
223210
}
224211

225-
fun updateBinaryDirectoryFallback(shouldEnableBinDirFallback: Boolean) {
226-
store[ENABLE_BINARY_DIR_FALLBACK] = shouldEnableBinDirFallback.toString()
227-
}
228-
229212
fun updateHeaderCommand(cmd: String) {
230213
store[HEADER_COMMAND] = cmd
231214
}

src/main/kotlin/com/coder/toolbox/store/StoreKeys.kt

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,6 @@ internal const val DATA_DIRECTORY = "dataDirectory"
2525

2626
internal const val ENABLE_DOWNLOADS = "enableDownloads"
2727

28-
internal const val ENABLE_BINARY_DIR_FALLBACK = "enableBinaryDirectoryFallback"
29-
3028
internal const val HEADER_COMMAND = "headerCommand"
3129

3230
internal const val TLS_CERT_PATH = "tlsCertPath"

src/main/kotlin/com/coder/toolbox/util/Error.kt

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4,31 +4,29 @@ import com.coder.toolbox.cli.ex.ResponseException
44
import com.coder.toolbox.sdk.ex.APIResponseException
55
import org.zeroturnaround.exec.InvalidExitValueException
66
import java.net.ConnectException
7-
import java.net.SocketTimeoutException
8-
import java.net.URL
97
import java.net.UnknownHostException
10-
import javax.net.ssl.SSLHandshakeException
118

12-
fun humanizeConnectionError(deploymentURL: URL, requireTokenAuth: Boolean, e: Exception): String {
13-
val reason = e.message ?: "No reason was provided."
14-
return when (e) {
15-
is java.nio.file.AccessDeniedException -> "Access denied to ${e.file}."
16-
is UnknownHostException -> "Unknown host ${e.message ?: deploymentURL.host}."
17-
is InvalidExitValueException -> "CLI exited unexpectedly with ${e.exitValue}."
9+
private fun accessDenied(file: String) = "Access denied to $file"
10+
private fun fileSystemFailed(file: String) = "A file system operation failed when trying to access $file"
11+
12+
fun Throwable.prettify(): String {
13+
val reason = this.message ?: "No reason was provided"
14+
return when (this) {
15+
is AccessDeniedException -> accessDenied(this.file.toString())
16+
is java.nio.file.AccessDeniedException -> accessDenied(this.file)
17+
is FileSystemException -> fileSystemFailed(this.file.toString())
18+
is java.nio.file.FileSystemException -> fileSystemFailed(this.file)
19+
is UnknownHostException -> "Unknown host $reason"
20+
is InvalidExitValueException -> "CLI exited unexpectedly with ${this.exitValue}."
1821
is APIResponseException -> {
19-
if (e.isUnauthorized) {
20-
if (requireTokenAuth) {
21-
"Token was rejected by $deploymentURL; has your token expired?"
22-
} else {
23-
"Authorization failed to $deploymentURL."
24-
}
22+
if (this.isUnauthorized) {
23+
"Authorization failed"
2524
} else {
2625
reason
2726
}
2827
}
29-
is SocketTimeoutException -> "Unable to connect to $deploymentURL; is it up?"
28+
3029
is ResponseException, is ConnectException -> "Failed to download Coder CLI: $reason"
31-
is SSLHandshakeException -> "Connection to $deploymentURL failed: $reason. See the <a href='https://coder.com/docs/v2/latest/ides/gateway#configuring-the-gateway-plugin-to-use-internal-certificates'>documentation for TLS certificates</a> for information on how to make your system trust certificates coming from your deployment."
3230
else -> reason
3331
}
3432
}

src/main/kotlin/com/coder/toolbox/views/CoderSettingsPage.kt

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -68,10 +68,6 @@ class CoderSettingsPage(
6868
)
6969
)
7070

71-
private val enableBinaryDirectoryFallbackField = CheckboxField(
72-
settings.enableBinaryDirectoryFallback,
73-
context.i18n.ptrl("Enable binary directory fallback")
74-
)
7571
private val headerCommandField = TextField(
7672
context.i18n.ptrl("Header command"),
7773
settings.headerCommand ?: "",
@@ -132,7 +128,6 @@ class CoderSettingsPage(
132128
enableDownloadsField,
133129
useAppNameField,
134130
binaryDestinationField,
135-
enableBinaryDirectoryFallbackField,
136131
disableSignatureVerificationField,
137132
signatureFallbackStrategyField,
138133
httpLoggingField,
@@ -163,7 +158,6 @@ class CoderSettingsPage(
163158
updateDisableSignatureVerification(disableSignatureVerificationField.checkedState.value)
164159
updateSignatureFallbackStrategy(signatureFallbackStrategyField.checkedState.value)
165160
updateHttpClientLogLevel(httpLoggingField.selectedValueState.value)
166-
updateBinaryDirectoryFallback(enableBinaryDirectoryFallbackField.checkedState.value)
167161
updateHeaderCommand(headerCommandField.contentState.value)
168162
updateCertPath(tlsCertPathField.contentState.value)
169163
updateKeyPath(tlsKeyPathField.contentState.value)
@@ -216,10 +210,6 @@ class CoderSettingsPage(
216210
settings.fallbackOnCoderForSignatures.isAllowed()
217211
}
218212

219-
enableBinaryDirectoryFallbackField.checkedState.update {
220-
settings.enableBinaryDirectoryFallback
221-
}
222-
223213
headerCommandField.contentState.update {
224214
settings.headerCommand ?: ""
225215
}

0 commit comments

Comments
 (0)