Skip to content
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions apps/desktop/src/app/DesktopStartupRecovery.test.ts

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This extends the existing retained-download failure case above. Could you fold this assertion into that test and rename it to cover both failure contexts?

Posted via Macroscope — Test Signal

Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,24 @@ describe("resolveStartupRecoveryOffer", () => {
});
});

it("explains why the offered update did not install", () => {
expect(
resolveStartupRecoveryOffer(
updateState({
status: "downloaded",
downloadedVersion: "0.1.4",
errorContext: "install",
message: "Not enough disk space to install the update.",
}),
"0.1.2",
),
).toEqual({
_tag: "Update",
version: "0.1.4",
installError: "Not enough disk space to install the update.",
});
});

it("does not offer an update that already failed to apply here", () => {
expect(
resolveStartupRecoveryOffer(
Expand Down
9 changes: 6 additions & 3 deletions apps/desktop/src/app/DesktopStartupRecovery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ const { logInfo, logWarning } = DesktopObservability.makeComponentLogger(

export type StartupRecoveryOffer =
| { readonly _tag: "Checking" }
| { readonly _tag: "Update"; readonly version: string }
| { readonly _tag: "Update"; readonly version: string; readonly installError?: string }
| { readonly _tag: "Downloading"; readonly version: string; readonly percent: number | null }
| { readonly _tag: "Reinstall"; readonly version: string }
| { readonly _tag: "Releases" };
Expand All @@ -53,7 +53,10 @@ export function resolveStartupRecoveryOffer(
update.downloadedVersion ?? (update.status === "available" ? update.availableVersion : null);
// A version that already failed to apply here would loop through install and relaunch.
if (updateVersion !== null && updateVersion !== update.notAppliedVersion) {
return { _tag: "Update", version: updateVersion };
const installError = update.errorContext === "install" ? update.message?.trim() : undefined;
return installError
? { _tag: "Update", version: updateVersion, installError }
: { _tag: "Update", version: updateVersion };
}
if (update.enabled && (update.status === "idle" || update.status === "checking")) {
return { _tag: "Checking" };
Expand All @@ -80,7 +83,7 @@ function describeOffer(offer: StartupRecoveryOffer): { text: string; action: str
return { text: "Checking for a newer version…", action: "" };
case "Update":
return {
text: `Version ${offer.version} is available. It can contain a fix.`,
text: offer.installError ?? `Version ${offer.version} is available. It can contain a fix.`,
action: actionLink("update", `Update to ${offer.version}`, true),
};
case "Downloading":
Expand Down
3 changes: 2 additions & 1 deletion apps/desktop/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ import * as DesktopState from "./app/DesktopState.ts";
import * as DesktopLegacyLocalStorage from "./app/DesktopLegacyLocalStorage.ts";
import * as DesktopTelemetryPublisher from "./telemetry/DesktopTelemetryPublisher.ts";
import * as DesktopRendererHistory from "./telemetry/DesktopRendererHistory.ts";
import * as DesktopDiskSpace from "./updates/DesktopDiskSpace.ts";
import * as DesktopUpdates from "./updates/DesktopUpdates.ts";
import * as BrowserImport from "./preview/BrowserImport/BrowserImport.ts";
import * as LinuxBrowserSecret from "./preview/BrowserImport/LinuxBrowserSecret.ts";
Expand Down Expand Up @@ -211,7 +212,7 @@ const layerDesktopApplication = Layer.mergeAll(
layerDesktopSsh,
).pipe(
Layer.provideMerge(layerDesktopSnapShot),
Layer.provideMerge(DesktopUpdates.layer),
Layer.provideMerge(DesktopUpdates.layer.pipe(Layer.provide(DesktopDiskSpace.layer))),
Layer.provideMerge(layerDesktopWslBackend),
Layer.provideMerge(layerDesktopLocalEnvironmentAuth),
);
Expand Down
35 changes: 35 additions & 0 deletions apps/desktop/src/updates/DesktopDiskSpace.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import * as NodeServices from "@effect/platform-node/NodeServices";
import { assert, describe, it } from "@effect/vitest";
import * as Effect from "effect/Effect";
import * as Exit from "effect/Exit";
import * as FileSystem from "effect/FileSystem";
import * as Layer from "effect/Layer";
import * as Path from "effect/Path";

import * as DesktopDiskSpace from "./DesktopDiskSpace.ts";

describe("DesktopDiskSpace", () => {
it.effect("sums nested files without following symlinks", () =>
Effect.gen(function* () {
const fileSystem = yield* FileSystem.FileSystem;
const path = yield* Path.Path;
const diskSpace = yield* DesktopDiskSpace.DesktopDiskSpace;
const root = yield* fileSystem.makeTempDirectoryScoped();
const outside = yield* fileSystem.makeTempDirectoryScoped();
yield* fileSystem.makeDirectory(path.join(root, "resources", "app"), { recursive: true });
yield* fileSystem.writeFileString(path.join(root, "app.exe"), "x".repeat(100));
yield* fileSystem.writeFileString(
path.join(root, "resources", "app", "a.js"),
"x".repeat(20),
);
yield* fileSystem.writeFileString(path.join(outside, "big.bin"), "x".repeat(5000));
yield* fileSystem.symlink(path.join(outside, "big.bin"), path.join(root, "big-link"));
yield* fileSystem.symlink(outside, path.join(root, "outside-link"));

assert.equal(yield* diskSpace.directoryBytes(root), 120);
assert.isTrue(
Exit.isFailure(yield* Effect.exit(diskSpace.directoryBytes(path.join(root, "missing")))),
);
}).pipe(Effect.scoped, Effect.provide(Layer.merge(DesktopDiskSpace.layer, NodeServices.layer))),
);
});
85 changes: 85 additions & 0 deletions apps/desktop/src/updates/DesktopDiskSpace.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
// @effect-diagnostics nodeBuiltinImport:off -- FileSystem has no statfs, and its stat follows symlinks.
import * as NodeFS from "node:fs";
import * as NodeOS from "node:os";
import * as NodePath from "node:path";

import * as Context from "effect/Context";
import * as Effect from "effect/Effect";
import * as Layer from "effect/Layer";
import * as Schema from "effect/Schema";

const MAX_DIRECTORY_ENTRIES = 100_000;

// Electron's fs reports .asar archives as empty directories; original-fs sees the real files.
const NodeFSP = ((process.getBuiltinModule("original-fs") as typeof NodeFS | undefined) ?? NodeFS)
.promises;

export class DesktopDiskSpaceError extends Schema.TaggedError<DesktopDiskSpaceError>()(
"DesktopDiskSpaceError",
{
operation: Schema.Literals(["freeBytes", "directoryBytes"]),
cause: Schema.Defect(),
},
) {
override get message(): string {
return `Could not measure disk space (${this.operation}).`;
}
}

export class DesktopDiskSpace extends Context.Service<
DesktopDiskSpace,
{
/** The temp directory a spawned installer inherits. */
readonly tempDirectory: string;
/** Bytes the current user can still write on the volume holding `path`. */
readonly freeBytes: (path: string) => Effect.Effect<number, DesktopDiskSpaceError>;
/** Total size of the regular files below `path`, without following symlinks. */
readonly directoryBytes: (path: string) => Effect.Effect<number, DesktopDiskSpaceError>;
}
>()("@t3tools/desktop/updates/DesktopDiskSpace") {}

async function sumDirectoryBytes(root: string): Promise<number> {
let total = 0;
let entries = 0;
const pending = [root];
for (let directory = pending.pop(); directory !== undefined; directory = pending.pop()) {
const children = await NodeFSP.readdir(directory, { withFileTypes: true }).catch(
(cause: unknown) => {
if (directory === root) throw cause;
return [];
},
);
entries += children.length;
if (entries > MAX_DIRECTORY_ENTRIES) {
throw new Error(`More than ${MAX_DIRECTORY_ENTRIES} entries below the directory.`);
}
for (const child of children) {
const childPath = NodePath.join(directory, child.name);
if (child.isDirectory()) pending.push(childPath);
else if (child.isFile()) {
total += await NodeFSP.lstat(childPath).then(
(stats) => stats.size,
() => 0,
);
}
}
}
return total;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This Layer.succeed captures imperative filesystem setup from the module-global NodeFSP and evaluates NodeOS.tmpdir() at import time. The service conventions prohibit hiding runtime-backed acquisition this way. Please move filesystem selection and temporary-directory acquisition into a lazy service constructor (for example, Layer.sync), keeping filesystem operations inside their existing typed Effects. This requires moving the helper's filesystem dependency too, so a single-hunk suggestion is not sufficient.

Posted via Macroscope — Effect Service Conventions

export const layer = Layer.succeed(DesktopDiskSpace, {
tempDirectory: NodeOS.tmpdir(),
freeBytes: Effect.fn("DesktopDiskSpace.freeBytes")(function* (path: string) {
const stats = yield* Effect.tryPromise({
try: () => NodeFSP.statfs(path),
catch: (cause) => new DesktopDiskSpaceError({ operation: "freeBytes", cause }),
});
return stats.bavail * stats.bsize;
}),
directoryBytes: Effect.fn("DesktopDiskSpace.directoryBytes")(function* (path: string) {
return yield* Effect.tryPromise({
try: () => sumDirectoryBytes(path),
catch: (cause) => new DesktopDiskSpaceError({ operation: "directoryBytes", cause }),
});
}),
});
126 changes: 126 additions & 0 deletions apps/desktop/src/updates/DesktopUpdates.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,132 @@ describe("DesktopUpdates", () => {
).pipe(Effect.provide(Layer.merge(TestClock.layer(), harness.layer)));
});

// regression: #27486 a silent NSIS update on a full volume left a half-written install that no longer launched
it.effect("keeps a Windows update retryable when the temp volume lacks space", () => {
const megabyte = 1024 * 1024;
const diskSpace = {
tempFreeBytes: 200 * megabyte,
installerBytes: 150 * megabyte,
installDirectoryBytes: 400 * megabyte,
};
const harness = makeHarness({ platform: "win32", diskSpace });

return Effect.scoped(
Effect.gen(function* () {
const desktopState = yield* DesktopState.DesktopState;
const pool = yield* DesktopBackendPool.DesktopBackendPool;
const updates = yield* DesktopUpdates.DesktopUpdates;
yield* updates.configure;
harness.emit("update-downloaded", {
version: "1.2.4",
downloadedFile: "C:\\Users\\me\\AppData\\Local\\t3code-updater\\pending\\installer.exe",
});
yield* flushCallbacks;

const result = yield* updates.install;
assert.equal(harness.quitAndInstalls(), 0, "the installer ran without enough disk space");
assert.isFalse(yield* Ref.get(desktopState.quitting));
assert.isTrue((yield* (yield* pool.primary).snapshot).desiredRunning);
assert.isFalse(result.completed);
assert.equal(result.state.status, "downloaded");
assert.equal(result.state.downloadedVersion, "1.2.4");
assert.equal(result.state.errorContext, "install");
assert.isTrue(result.state.canRetry);
assert.match(result.state.message ?? "", /space/i);

diskSpace.tempFreeBytes = 2000 * megabyte;
const retry = yield* updates.install;
assert.equal(harness.quitAndInstalls(), 1);
assert.isNull(retry.state.message, "a successful retry still reports the earlier refusal");
}),
).pipe(Effect.provide(Layer.merge(TestClock.layer(), harness.layer)));
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This repeats the temp-volume refusal and successful retry in the preceding test. Could you use requiredBytes - 1 and requiredBytes there and remove this separate test?

Posted via Macroscope — Test Signal

it.effect(
"installs a Windows update only with room for the installer, install folder and margin",
() => {
const megabyte = 1024 * 1024;
const requiredBytes = (150 + 400 + 128) * megabyte;
const installWithTempFreeBytes = (tempFreeBytes: number) => {
const harness = makeHarness({
platform: "win32",
diskSpace: {
tempFreeBytes,
installerBytes: 150 * megabyte,
installDirectoryBytes: 400 * megabyte,
},
});
return Effect.scoped(
Effect.gen(function* () {
const updates = yield* DesktopUpdates.DesktopUpdates;
yield* updates.configure;
harness.emit("update-downloaded", {
version: "1.2.4",
downloadedFile:
"C:\\Users\\me\\AppData\\Local\\t3code-updater\\pending\\installer.exe",
});
yield* flushCallbacks;
yield* updates.install;
return harness.quitAndInstalls();
}),
).pipe(Effect.provide(Layer.merge(TestClock.layer(), harness.layer)));
};

return Effect.gen(function* () {
assert.equal(yield* installWithTempFreeBytes(requiredBytes - 1), 0);
assert.equal(yield* installWithTempFreeBytes(requiredBytes), 1);
});
},
);

it.effect("keeps a Windows update retryable when a separate install volume is full", () => {
const megabyte = 1024 * 1024;
const harness = makeHarness({
platform: "win32",
diskSpace: {
tempFreeBytes: 2000 * megabyte,
installFreeBytes: 5 * megabyte,
installerBytes: 150 * megabyte,
installDirectoryBytes: 400 * megabyte,
},
});

return Effect.scoped(
Effect.gen(function* () {
const updates = yield* DesktopUpdates.DesktopUpdates;
yield* updates.configure;
harness.emit("update-downloaded", {
version: "1.2.4",
downloadedFile: "C:\\Users\\me\\AppData\\Local\\t3code-updater\\pending\\installer.exe",
});
yield* flushCallbacks;

const result = yield* updates.install;
assert.equal(harness.quitAndInstalls(), 0);
assert.match(result.state.message ?? "", /space/i);
}),
).pipe(Effect.provide(Layer.merge(TestClock.layer(), harness.layer)));
});

it.effect("installs a Windows update when disk space cannot be measured", () => {
const harness = makeHarness({ platform: "win32" });

return Effect.scoped(
Effect.gen(function* () {
const updates = yield* DesktopUpdates.DesktopUpdates;
yield* updates.configure;
harness.emit("update-downloaded", {
version: "1.2.4",
downloadedFile: "C:\\Users\\me\\AppData\\Local\\t3code-updater\\pending\\installer.exe",
});
yield* flushCallbacks;

yield* updates.install;
assert.equal(harness.quitAndInstalls(), 1, "an unmeasured volume blocked the update");
}),
).pipe(Effect.provide(Layer.merge(TestClock.layer(), harness.layer)));
});

it.effect("restores only desired-running instances after an installer error event", () => {
const starts: string[] = [];
const stops: string[] = [];
Expand Down
Loading
Loading