Skip to content

Commit e866c3c

Browse files
committed
refactor: linearize fetchBinary decision tree for readability
The version check, lock-wait, and rename logic in fetchBinary used nested if/else-if blocks and a needsDownload flag that made the control flow hard to follow. This refactors it into sequential guard clauses (version match, downloads disabled, version mismatch) and replaces the flag with an early return. The duplicated rename-to- final-path logic is extracted into a small helper used in both the lock-wait and download paths. No behavior changes.
1 parent 7cda5ed commit e866c3c

File tree

2 files changed

+74
-51
lines changed

2 files changed

+74
-51
lines changed

CHANGELOG.md

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,21 @@
55
from published versions since it shows up in the VS Code extension changelog
66
tab and is confusing to users. Add it back between releases if needed. -->
77

8+
## Unreleased
9+
10+
### Added
11+
12+
- `coder.binaryDestination` now accepts a full file path (e.g. `/usr/bin/coder`) in addition
13+
to a directory. The extension checks the binary's version against the server and downloads a
14+
replacement when needed. When set to a directory, the simple name (`coder` / `coder.exe`) is
15+
tried as a fallback after the platform-specific name, so package-manager-installed CLIs work
16+
without symlinking.
17+
18+
### Fixed
19+
20+
- Cleanup of old/temp files in shared directories like `/usr/bin` is now scoped to the binary's
21+
own basename, preventing accidental removal of unrelated files.
22+
823
## [v1.14.3](https://github.com/coder/vscode-coder/releases/tag/v1.14.3) 2026-03-30
924

1025
### Added

src/core/cliManager.ts

Lines changed: 59 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -139,27 +139,15 @@ export class CliManager {
139139
`Resolved binary: ${resolved.binPath} (${resolved.source})`,
140140
);
141141

142-
// Check existing binary version when one was found.
142+
let existingVersion: string | null = null;
143143
if (resolved.source !== "not-found") {
144144
this.output.debug(
145145
"Existing binary size is",
146146
prettyBytes(resolved.stat.size),
147147
);
148148
try {
149-
const version = await cliVersion(resolved.binPath);
150-
this.output.debug("Existing binary version is", version);
151-
if (version === buildInfo.version) {
152-
this.output.debug("Existing binary matches server version");
153-
return resolved.binPath;
154-
} else if (!enableDownloads) {
155-
this.output.info(
156-
"Using existing binary despite version mismatch because downloads are disabled",
157-
);
158-
return resolved.binPath;
159-
}
160-
this.output.info(
161-
"Downloading since existing binary does not match the server version",
162-
);
149+
existingVersion = await cliVersion(resolved.binPath);
150+
this.output.debug("Existing binary version is", existingVersion);
163151
} catch (error) {
164152
this.output.warn(
165153
"Unable to get version of existing binary, downloading instead",
@@ -170,11 +158,28 @@ export class CliManager {
170158
this.output.info("No existing binary found, starting download");
171159
}
172160

161+
if (existingVersion === buildInfo.version) {
162+
this.output.debug("Existing binary matches server version");
163+
return resolved.binPath;
164+
}
165+
173166
if (!enableDownloads) {
167+
if (existingVersion) {
168+
this.output.info(
169+
"Using existing binary despite version mismatch because downloads are disabled",
170+
);
171+
return resolved.binPath;
172+
}
174173
this.output.warn("Unable to download CLI because downloads are disabled");
175174
throw new Error("Unable to download CLI because downloads are disabled");
176175
}
177176

177+
if (existingVersion) {
178+
this.output.info(
179+
"Downloading since existing binary does not match the server version",
180+
);
181+
}
182+
178183
// Always download using the platform-specific name.
179184
const downloadBinPath = path.join(
180185
path.dirname(resolved.binPath),
@@ -196,8 +201,7 @@ export class CliManager {
196201
);
197202
this.output.debug("Acquired download lock");
198203

199-
// If we waited for another process, re-check if binary is now ready
200-
let needsDownload = true;
204+
// Another process may have finished the download while we waited.
201205
if (lockResult.waited) {
202206
const latestBuildInfo = await restClient.getBuildInfo();
203207
this.output.debug("Got latest server version", latestBuildInfo.version);
@@ -207,43 +211,26 @@ export class CliManager {
207211
latestBuildInfo.version,
208212
);
209213
if (recheckAfterWait.matches) {
210-
this.output.debug(
211-
"Using existing binary since it matches the latest server version",
212-
);
213-
needsDownload = false;
214-
} else {
215-
const latestParsedVersion = semver.parse(latestBuildInfo.version);
216-
if (!latestParsedVersion) {
217-
throw new Error(
218-
`Got invalid version from deployment: ${latestBuildInfo.version}`,
219-
);
220-
}
221-
latestVersion = latestParsedVersion;
214+
this.output.debug("Binary already matches server version after wait");
215+
return await this.renameToFinalPath(resolved, downloadBinPath);
222216
}
223-
}
224217

225-
if (needsDownload) {
226-
await this.performBinaryDownload(
227-
restClient,
228-
latestVersion,
229-
downloadBinPath,
230-
progressLogPath,
231-
);
218+
const latestParsedVersion = semver.parse(latestBuildInfo.version);
219+
if (!latestParsedVersion) {
220+
throw new Error(
221+
`Got invalid version from deployment: ${latestBuildInfo.version}`,
222+
);
223+
}
224+
latestVersion = latestParsedVersion;
232225
}
233226

234-
// Rename to user-configured file path while we hold the lock.
235-
if (
236-
resolved.source === "file-path" &&
237-
downloadBinPath !== resolved.binPath
238-
) {
239-
this.output.info(
240-
"Renaming downloaded binary to",
241-
path.basename(resolved.binPath),
242-
);
243-
await fs.rename(downloadBinPath, resolved.binPath);
244-
return resolved.binPath;
245-
}
246-
return downloadBinPath;
227+
await this.performBinaryDownload(
228+
restClient,
229+
latestVersion,
230+
downloadBinPath,
231+
progressLogPath,
232+
);
233+
return await this.renameToFinalPath(resolved, downloadBinPath);
247234
} catch (error) {
248235
const fallback = await this.handleAnyBinaryFailure(
249236
error,
@@ -288,6 +275,27 @@ export class CliManager {
288275
}
289276
}
290277

278+
/**
279+
* Rename the downloaded binary to the user-configured file path if needed.
280+
*/
281+
private async renameToFinalPath(
282+
resolved: ResolvedBinary,
283+
downloadBinPath: string,
284+
): Promise<string> {
285+
if (
286+
resolved.source === "file-path" &&
287+
downloadBinPath !== resolved.binPath
288+
) {
289+
this.output.info(
290+
"Renaming downloaded binary to",
291+
path.basename(resolved.binPath),
292+
);
293+
await fs.rename(downloadBinPath, resolved.binPath);
294+
return resolved.binPath;
295+
}
296+
return downloadBinPath;
297+
}
298+
291299
/**
292300
* Prompt the user to use an existing binary version.
293301
*/
@@ -388,7 +396,7 @@ export class CliManager {
388396
}
389397
}
390398

391-
// Last resort: try the most recent .old-* backup.
399+
// Last resort: most recent .old-* backup (deferred to avoid IO when unnecessary).
392400
const oldBinaries = await cliUtils.findOldBinaries(binPath);
393401
if (oldBinaries.length > 0) {
394402
const old = await tryCandidate(oldBinaries[0]);

0 commit comments

Comments
 (0)