Skip to content

Commit abef8d0

Browse files
juliusmarmingejuliusmarminge
committed
fix(desktop): preserve server hydration after failed Windows probes
1 parent 456ef3b commit abef8d0

5 files changed

Lines changed: 106 additions & 43 deletions

File tree

‎apps/desktop/src/backend/DesktopBackendConfiguration.test.ts‎

Lines changed: 44 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import { ChildProcessSpawner } from "effect/process";
1515

1616
import * as DesktopEnvironment from "../app/DesktopEnvironment.ts";
1717
import * as DesktopBackendConfiguration from "./DesktopBackendConfiguration.ts";
18+
import * as DesktopShellEnvironment from "../shell/DesktopShellEnvironment.ts";
1819
import * as DesktopConfig from "../app/DesktopConfig.ts";
1920
import * as DesktopServerExposure from "./DesktopServerExposure.ts";
2021
import * as DesktopAppSettings from "../settings/DesktopAppSettings.ts";
@@ -37,6 +38,14 @@ const isDesktopBackendObservabilitySettingsReadError = Schema.is(
3738
DesktopBackendConfiguration.DesktopBackendObservabilitySettingsReadError,
3839
);
3940

41+
const layerShellEnvironment = (prepared = true) =>
42+
Layer.succeed(DesktopShellEnvironment.DesktopShellEnvironment, {
43+
installIntoProcess: Effect.succeed(prepared),
44+
});
45+
46+
const layerBackendConfiguration = (prepared = true) =>
47+
DesktopBackendConfiguration.layer.pipe(Layer.provide(layerShellEnvironment(prepared)));
48+
4049
const layerServerExposure = Layer.succeed(DesktopServerExposure.DesktopServerExposure, {
4150
getState: Effect.die("unexpected getState"),
4251
backendConfig: Effect.succeed({
@@ -114,6 +123,7 @@ const withHarness = <A, E, R>(
114123
| FileSystem.FileSystem
115124
| DesktopBackendConfiguration.DesktopBackendConfiguration
116125
>,
126+
prepared = true,
117127
) =>
118128
Effect.gen(function* () {
119129
const fileSystem = yield* FileSystem.FileSystem;
@@ -123,7 +133,7 @@ const withHarness = <A, E, R>(
123133

124134
return yield* effect.pipe(
125135
Effect.provide(
126-
DesktopBackendConfiguration.layer.pipe(
136+
layerBackendConfiguration(prepared).pipe(
127137
Layer.provideMerge(layerServerExposure),
128138
Layer.provideMerge(DesktopAppSettings.layerTest()),
129139
Layer.provideMerge(DesktopWslEnvironment.layerTest()),
@@ -201,7 +211,7 @@ const withPackagedWslHarness = <A, E, R>(
201211

202212
return yield* effect(context).pipe(
203213
Effect.provide(
204-
DesktopBackendConfiguration.layer.pipe(
214+
layerBackendConfiguration().pipe(
205215
Layer.provideMerge(layerServerExposure),
206216
Layer.provideMerge(DesktopAppSettings.layerTest()),
207217
Layer.provideMerge(layerServerTree),
@@ -293,7 +303,7 @@ describe("DesktopBackendConfiguration", () => {
293303
return yield* configuration.resolvePrimary;
294304
}).pipe(
295305
Effect.provide(
296-
DesktopBackendConfiguration.layer.pipe(
306+
layerBackendConfiguration().pipe(
297307
Layer.provideMerge(layerServerExposure),
298308
Layer.provideMerge(DesktopAppSettings.layerTest()),
299309
Layer.provideMerge(DesktopWslEnvironment.layerTest()),
@@ -340,6 +350,17 @@ describe("DesktopBackendConfiguration", () => {
340350
),
341351
);
342352

353+
it.effect("keeps server hydration enabled when desktop preparation failed", () =>
354+
withHarness(
355+
Effect.gen(function* () {
356+
const configuration = yield* DesktopBackendConfiguration.DesktopBackendConfiguration;
357+
const primary = yield* configuration.resolvePrimary;
358+
assert.equal(primary.bootstrap.shellEnvironmentPrepared, false);
359+
}),
360+
false,
361+
),
362+
);
363+
343364
it.effect("resolveWsl reuses the primary's bootstrap token", () =>
344365
withHarness(
345366
Effect.gen(function* () {
@@ -370,7 +391,7 @@ describe("DesktopBackendConfiguration", () => {
370391
return yield* configuration.resolveWsl({ port: 5000, distro: null });
371392
}).pipe(
372393
Effect.provide(
373-
DesktopBackendConfiguration.layer.pipe(
394+
layerBackendConfiguration().pipe(
374395
Layer.provideMerge(layerServerExposure),
375396
Layer.provideMerge(DesktopAppSettings.layerTest()),
376397
Layer.provideMerge(DesktopWslServerTree.layerTest()),
@@ -692,7 +713,7 @@ describe("DesktopBackendConfiguration", () => {
692713
return yield* configuration.resolveWsl({ port: 5000, distro: "Ubuntu" });
693714
}).pipe(
694715
Effect.provide(
695-
DesktopBackendConfiguration.layer.pipe(
716+
layerBackendConfiguration().pipe(
696717
Layer.provideMerge(layerServerExposure),
697718
Layer.provideMerge(DesktopAppSettings.layerTest()),
698719
Layer.provideMerge(DesktopWslServerTree.layerTest()),
@@ -820,7 +841,7 @@ describe("DesktopBackendConfiguration", () => {
820841
assert.equal(config.bootstrap.otlpLogsUrl, "http://127.0.0.1:4318/v1/logs");
821842
}).pipe(
822843
Effect.provide(
823-
DesktopBackendConfiguration.layer.pipe(
844+
layerBackendConfiguration().pipe(
824845
Layer.provideMerge(layerServerExposure),
825846
Layer.provideMerge(DesktopAppSettings.layerTest()),
826847
Layer.provideMerge(DesktopWslServerTree.layerTest()),
@@ -878,7 +899,7 @@ describe("DesktopBackendConfiguration", () => {
878899
assert.equal(config.bootstrap.otlpMetricsUrl, "http://persisted:4318/v1/metrics");
879900
}).pipe(
880901
Effect.provide(
881-
DesktopBackendConfiguration.layer.pipe(
902+
layerBackendConfiguration().pipe(
882903
Layer.provideMerge(layerServerExposure),
883904
Layer.provideMerge(DesktopAppSettings.layerTest()),
884905
Layer.provideMerge(DesktopWslServerTree.layerTest()),
@@ -923,7 +944,7 @@ describe("DesktopBackendConfiguration", () => {
923944
}).pipe(
924945
Effect.provide(
925946
Layer.mergeAll(
926-
DesktopBackendConfiguration.layer.pipe(
947+
layerBackendConfiguration().pipe(
927948
Layer.provideMerge(layerServerExposure),
928949
Layer.provideMerge(DesktopAppSettings.layerTest()),
929950
Layer.provideMerge(DesktopWslServerTree.layerTest()),
@@ -968,7 +989,7 @@ describe("DesktopBackendConfiguration", () => {
968989
assert.notInclude(config.args, "--require");
969990
}).pipe(
970991
Effect.provide(
971-
DesktopBackendConfiguration.layer.pipe(
992+
layerBackendConfiguration().pipe(
972993
Layer.provideMerge(layerServerExposure),
973994
Layer.provideMerge(DesktopAppSettings.layerTest()),
974995
Layer.provideMerge(DesktopWslServerTree.layerTest()),
@@ -1010,7 +1031,7 @@ describe("DesktopBackendConfiguration", () => {
10101031
assert.include((config.env.WSLENV ?? "").split(":"), "T3CODE_TELEMETRY_ENABLED");
10111032
}).pipe(
10121033
Effect.provide(
1013-
DesktopBackendConfiguration.layer.pipe(
1034+
layerBackendConfiguration().pipe(
10141035
Layer.provideMerge(layerServerExposure),
10151036
Layer.provideMerge(DesktopAppSettings.layerTest()),
10161037
Layer.provideMerge(DesktopWslServerTree.layerTest()),
@@ -1077,7 +1098,7 @@ describe("DesktopBackendConfiguration", () => {
10771098
assert.include(wslEnv, "T3CODE_OTLP_TRACES_URL");
10781099
}).pipe(
10791100
Effect.provide(
1080-
DesktopBackendConfiguration.layer.pipe(
1101+
layerBackendConfiguration().pipe(
10811102
Layer.provideMerge(layerServerExposure),
10821103
Layer.provideMerge(DesktopAppSettings.layerTest()),
10831104
Layer.provideMerge(DesktopWslServerTree.layerTest()),
@@ -1151,7 +1172,7 @@ describe("DesktopBackendConfiguration", () => {
11511172
);
11521173
}).pipe(
11531174
Effect.provide(
1154-
DesktopBackendConfiguration.layer.pipe(
1175+
layerBackendConfiguration().pipe(
11551176
Layer.provideMerge(layerServerExposure),
11561177
Layer.provideMerge(DesktopAppSettings.layerTest()),
11571178
Layer.provideMerge(DesktopWslServerTree.layerTest()),
@@ -1199,7 +1220,7 @@ describe("DesktopBackendConfiguration", () => {
11991220
assert.isTrue(Option.isNone(config.preflightFailure));
12001221
}).pipe(
12011222
Effect.provide(
1202-
DesktopBackendConfiguration.layer.pipe(
1223+
layerBackendConfiguration().pipe(
12031224
Layer.provideMerge(layerServerExposure),
12041225
Layer.provideMerge(
12051226
DesktopAppSettings.layerTest({
@@ -1236,7 +1257,7 @@ describe("DesktopBackendConfiguration", () => {
12361257
assert.include(failure.reason, "Removed-Distro");
12371258
}).pipe(
12381259
Effect.provide(
1239-
DesktopBackendConfiguration.layer.pipe(
1260+
layerBackendConfiguration().pipe(
12401261
Layer.provideMerge(layerServerExposure),
12411262
Layer.provideMerge(
12421263
DesktopAppSettings.layerTest({
@@ -1277,7 +1298,7 @@ describe("DesktopBackendConfiguration", () => {
12771298
assert.include(failure.reason, "timed out");
12781299
}).pipe(
12791300
Effect.provide(
1280-
DesktopBackendConfiguration.layer.pipe(
1301+
layerBackendConfiguration().pipe(
12811302
Layer.provideMerge(layerServerExposure),
12821303
Layer.provideMerge(DesktopAppSettings.layerTest()),
12831304
Layer.provideMerge(DesktopWslServerTree.layerTest()),
@@ -1312,7 +1333,7 @@ describe("DesktopBackendConfiguration", () => {
13121333
assert.include(failure.reason, "missing server entry");
13131334
}).pipe(
13141335
Effect.provide(
1315-
DesktopBackendConfiguration.layer.pipe(
1336+
layerBackendConfiguration().pipe(
13161337
Layer.provideMerge(layerServerExposure),
13171338
Layer.provideMerge(DesktopAppSettings.layerTest()),
13181339
Layer.provideMerge(DesktopWslServerTree.layerTest()),
@@ -1346,7 +1367,7 @@ describe("DesktopBackendConfiguration", () => {
13461367
assert.include(failure.reason, "could not be extracted");
13471368
}).pipe(
13481369
Effect.provide(
1349-
DesktopBackendConfiguration.layer.pipe(
1370+
layerBackendConfiguration().pipe(
13501371
Layer.provideMerge(layerServerExposure),
13511372
Layer.provideMerge(DesktopAppSettings.layerTest()),
13521373
Layer.provideMerge(
@@ -1387,7 +1408,7 @@ describe("DesktopBackendConfiguration", () => {
13871408
assert.include(failure.reason, "Removed-Distro");
13881409
}).pipe(
13891410
Effect.provide(
1390-
DesktopBackendConfiguration.layer.pipe(
1411+
layerBackendConfiguration().pipe(
13911412
Layer.provideMerge(layerServerExposure),
13921413
Layer.provideMerge(DesktopAppSettings.layerTest()),
13931414
Layer.provideMerge(DesktopWslServerTree.layerTest()),
@@ -1417,7 +1438,7 @@ describe("DesktopBackendConfiguration", () => {
14171438
assert.equal(label, "WSL (Ubuntu)");
14181439
}).pipe(
14191440
Effect.provide(
1420-
DesktopBackendConfiguration.layer.pipe(
1441+
layerBackendConfiguration().pipe(
14211442
Layer.provideMerge(layerServerExposure),
14221443
Layer.provideMerge(
14231444
DesktopAppSettings.layerTest({
@@ -1466,7 +1487,7 @@ describe("DesktopBackendConfiguration", () => {
14661487
assert.equal(config.bootstrap.desktopTelemetryControlFd, 5);
14671488
}).pipe(
14681489
Effect.provide(
1469-
DesktopBackendConfiguration.layer.pipe(
1490+
layerBackendConfiguration().pipe(
14701491
Layer.provideMerge(layerServerExposure),
14711492
Layer.provideMerge(DesktopAppSettings.layerTest()),
14721493
Layer.provideMerge(DesktopWslServerTree.layerTest()),
@@ -1512,7 +1533,7 @@ describe("DesktopBackendConfiguration", () => {
15121533
assert.equal(config.bootstrap.resourceMonitorPath, releaseMonitorPath);
15131534
}).pipe(
15141535
Effect.provide(
1515-
DesktopBackendConfiguration.layer.pipe(
1536+
layerBackendConfiguration().pipe(
15161537
Layer.provideMerge(layerServerExposure),
15171538
Layer.provideMerge(DesktopAppSettings.layerTest()),
15181539
Layer.provideMerge(DesktopWslServerTree.layerTest()),
@@ -1556,7 +1577,7 @@ describe("DesktopBackendConfiguration", () => {
15561577
assert.equal(label, "Windows");
15571578
}).pipe(
15581579
Effect.provide(
1559-
DesktopBackendConfiguration.layer.pipe(
1580+
layerBackendConfiguration().pipe(
15601581
Layer.provideMerge(layerServerExposure),
15611582
Layer.provideMerge(
15621583
DesktopAppSettings.layerTest({
@@ -1585,7 +1606,7 @@ describe("DesktopBackendConfiguration", () => {
15851606
// top-level runSync, exactly as the handler does.
15861607
// oxlint-disable-next-line t3code/no-manual-effect-runtime-in-tests -- This test intentionally replicates the sync IPC handler's runSync path to catch a regression to async-only resolution; it.effect would mask it.
15871608
const runtime = ManagedRuntime.make(
1588-
DesktopBackendConfiguration.layer.pipe(
1609+
layerBackendConfiguration().pipe(
15891610
Layer.provideMerge(layerServerExposure),
15901611
Layer.provideMerge(DesktopAppSettings.layerTest()),
15911612
Layer.provideMerge(DesktopWslServerTree.layerTest()),

‎apps/desktop/src/backend/DesktopBackendConfiguration.ts‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import * as DesktopBackendManager from "./DesktopBackendManager.ts";
2020
import * as DesktopCliShim from "../app/DesktopCliShim.ts";
2121
import * as DesktopEnvironment from "../app/DesktopEnvironment.ts";
2222
import * as DesktopServerExposure from "./DesktopServerExposure.ts";
23+
import * as DesktopShellEnvironment from "../shell/DesktopShellEnvironment.ts";
2324
import * as DesktopAppSettings from "../settings/DesktopAppSettings.ts";
2425
import * as DesktopWslEnvironment from "../wsl/DesktopWslEnvironment.ts";
2526
import * as DesktopWslServerTree from "../wsl/DesktopWslServerTree.ts";
@@ -549,6 +550,7 @@ const resolvePrimaryStartConfig = Effect.fn("desktop.backendConfiguration.resolv
549550
input: SharedBootstrapInput & {
550551
readonly resourceMonitorPath: Option.Option<string>;
551552
readonly cliPath: Option.Option<string>;
553+
readonly shellEnvironmentPrepared: boolean;
552554
},
553555
): Effect.fn.Return<
554556
DesktopBackendManager.DesktopBackendStartConfig,
@@ -567,10 +569,8 @@ const resolvePrimaryStartConfig = Effect.fn("desktop.backendConfiguration.resolv
567569
host: backendExposure.bindHost,
568570
desktopBootstrapToken: input.bootstrapToken,
569571
desktopBootstrapSecret: input.bootstrapSecret,
570-
// DesktopApp installs the shell environment into process.env before any
571-
// backend starts, and this child extends it (extendEnv below). WSL gets
572-
// a different environment, so its bootstrap never sets this.
573-
shellEnvironmentPrepared: true,
572+
// Failed Windows probes leave server hydration enabled for another attempt.
573+
shellEnvironmentPrepared: input.shellEnvironmentPrepared,
574574
tailscaleServeEnabled: backendExposure.tailscaleServeEnabled,
575575
tailscaleServePort: backendExposure.tailscaleServePort,
576576
desktopTelemetryFd: 4,
@@ -842,6 +842,7 @@ const resolveWslStartConfig = Effect.fn("desktop.backendConfiguration.resolveWsl
842842
/** @public Service construction is part of the canonical Effect module API. */
843843
export const make = Effect.gen(function* () {
844844
const environment = yield* DesktopEnvironment.DesktopEnvironment;
845+
const shellEnvironment = yield* DesktopShellEnvironment.DesktopShellEnvironment;
845846
const fileSystem = yield* FileSystem.FileSystem;
846847
const serverExposure = yield* DesktopServerExposure.DesktopServerExposure;
847848
const wslEnvironment = yield* DesktopWslEnvironment.DesktopWslEnvironment;
@@ -924,7 +925,13 @@ export const make = Effect.gen(function* () {
924925
Effect.provideService(FileSystem.FileSystem, fileSystem),
925926
Effect.provideService(DesktopEnvironment.DesktopEnvironment, environment),
926927
);
927-
return yield* resolvePrimaryStartConfig({ ...shared, resourceMonitorPath, cliPath }).pipe(
928+
const shellEnvironmentPrepared = yield* shellEnvironment.installIntoProcess;
929+
return yield* resolvePrimaryStartConfig({
930+
...shared,
931+
resourceMonitorPath,
932+
cliPath,
933+
shellEnvironmentPrepared,
934+
}).pipe(
928935
Effect.provideService(DesktopEnvironment.DesktopEnvironment, environment),
929936
Effect.provideService(DesktopServerExposure.DesktopServerExposure, serverExposure),
930937
);

‎apps/desktop/src/main.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,7 @@ const layerDesktopAppActivation = DesktopAppActivation.layer.pipe(
195195
const layerDesktopBackend = DesktopBackendPool.layer.pipe(
196196
Layer.provideMerge(DesktopAppIdentity.layer),
197197
Layer.provideMerge(DesktopBackendConfiguration.layer),
198+
Layer.provideMerge(DesktopShellEnvironment.layer),
198199
Layer.provideMerge(DesktopWslEnvironment.layer),
199200
Layer.provideMerge(DesktopWslServerTree.layer),
200201
Layer.provideMerge(DesktopTelemetryPublisher.layer),
@@ -219,7 +220,6 @@ const layerDesktopApplication = Layer.mergeAll(
219220
DesktopApplicationMenu.layer,
220221
DesktopLinuxUrlHandler.layer,
221222
DesktopCliCommand.layer,
222-
DesktopShellEnvironment.layer,
223223
layerDesktopSsh,
224224
).pipe(
225225
Layer.provideMerge(layerDesktopSnapShot),

‎apps/desktop/src/shell/DesktopShellEnvironment.test.ts‎

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,9 @@ function runShellEnvironment(input: {
9898

9999
const program = Effect.gen(function* () {
100100
const shellEnvironment = yield* DesktopShellEnvironment.DesktopShellEnvironment;
101-
yield* shellEnvironment.installIntoProcess;
101+
const prepared = yield* shellEnvironment.installIntoProcess;
102+
assert.equal(yield* shellEnvironment.installIntoProcess, prepared);
103+
return prepared;
102104
}).pipe(
103105
Effect.provide(
104106
DesktopShellEnvironment.layer.pipe(
@@ -446,7 +448,7 @@ describe("DesktopShellEnvironment", () => {
446448
USERPROFILE: "C:\\Users\\testuser",
447449
};
448450

449-
yield* runShellEnvironment({
451+
const prepared = yield* runShellEnvironment({
450452
env,
451453
platform: "win32",
452454
handler: (command) => {
@@ -462,6 +464,7 @@ describe("DesktopShellEnvironment", () => {
462464
},
463465
});
464466

467+
assert.isTrue(prepared);
465468
assert.equal(
466469
env.PATH,
467470
[
@@ -486,6 +489,30 @@ describe("DesktopShellEnvironment", () => {
486489
}),
487490
);
488491

492+
it.effect.each([false, true])(
493+
"leaves Windows preparation incomplete when loadProfile=%s fails",
494+
(loadProfile) =>
495+
Effect.gen(function* () {
496+
const env: NodeJS.ProcessEnv = { PATH: "C:\\Windows\\System32" };
497+
let calls = 0;
498+
const prepared = yield* runShellEnvironment({
499+
env,
500+
platform: "win32",
501+
handler: (command) => {
502+
calls++;
503+
if (command._tag !== "StandardCommand") return "";
504+
return !command.args.includes("-NoProfile") === loadProfile
505+
? { output: "", exitCode: 1 }
506+
: envOutput({ PATH: "C:\\Working" });
507+
},
508+
}).pipe(Effect.provide(Logger.layer([])));
509+
assert.isFalse(prepared);
510+
assert.equal(calls, 3);
511+
assert.include(env.PATH, "C:\\Working");
512+
assert.include(env.PATH, "C:\\Windows\\System32");
513+
}),
514+
);
515+
489516
it.effect("replaces a Path-spelled Windows PATH instead of adding a second key", () =>
490517
Effect.gen(function* () {
491518
const env: NodeJS.ProcessEnv = { Path: "C:\\Windows\\System32" };

0 commit comments

Comments
 (0)