Skip to content

Commit b8e9880

Browse files
committed
fix(desktop): an earlier backend run no longer removes a newer update marker
The wait that keeps the update marker until the new backend publishes its runtime ran in the pool scope and removed whatever marker existed when it ended. It outlived its backend run, so after a restart, or when a later update wrote a fresh marker, it could remove that marker when its own timeout ran out and reopen the window for a competing SSH server. The wait now runs in a fiber handle: a backend shutdown interrupts it and a later run's readiness replaces it. It also remembers the marker it started with and removes the marker only if the contents still match.
1 parent ca81d85 commit b8e9880

2 files changed

Lines changed: 87 additions & 14 deletions

File tree

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

Lines changed: 72 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import * as Path from "effect/Path";
1111
import * as DesktopEnvironment from "../app/DesktopEnvironment.ts";
1212
import * as DesktopConfig from "../app/DesktopConfig.ts";
1313
import * as Option from "effect/Option";
14+
import * as Queue from "effect/Queue";
1415
import * as Ref from "effect/Ref";
1516
import * as Stream from "effect/Stream";
1617
import { HttpClient, HttpClientResponse } from "effect/http";
@@ -135,7 +136,8 @@ const makeUpdateMarkerFixture = (backendMode: "native" | "WSL") =>
135136
const ready = yield* Deferred.make<void>();
136137
const runtimeChecked = yield* Deferred.make<void>();
137138
const markerRemoved = yield* Deferred.make<void>();
138-
const exited = yield* Deferred.make<ChildProcessSpawner.ExitCode>();
139+
const readyRuns = yield* Queue.unbounded<void>();
140+
let nextPid = 123;
139141
const files = new Map([[markerPath, JSON.stringify({ startedAt: 0, expiresAt: 60_000 })]]);
140142
const removed: string[] = [];
141143
const config: DesktopBackendStartConfig = {
@@ -180,30 +182,38 @@ const makeUpdateMarkerFixture = (backendMode: "native" | "WSL") =>
180182
),
181183
},
182184
spawner: ChildProcessSpawner.make(() =>
183-
Effect.succeed(
184-
ChildProcessSpawner.makeHandle({
185-
pid: ChildProcessSpawner.ProcessId(123),
185+
Effect.gen(function* () {
186+
const exited = yield* Deferred.make<ChildProcessSpawner.ExitCode>();
187+
const exit = Deferred.succeed(exited, ChildProcessSpawner.ExitCode(0)).pipe(
188+
Effect.asVoid,
189+
);
190+
// Like the real spawner, closing the run's scope ends the process.
191+
yield* Effect.addFinalizer(() => exit);
192+
return ChildProcessSpawner.makeHandle({
193+
pid: ChildProcessSpawner.ProcessId(nextPid++),
186194
stdout: Stream.empty,
187195
stderr: Stream.empty,
188196
all: Stream.empty,
189197
exitCode: Deferred.await(exited),
190198
isRunning: Effect.succeed(true),
191-
kill: () =>
192-
Deferred.succeed(exited, ChildProcessSpawner.ExitCode(0)).pipe(Effect.asVoid),
199+
kill: () => exit,
193200
stdin: Sink.drain,
194201
getInputFd: () => Sink.drain,
195202
getOutputFd: () => Stream.empty,
196203
unref: Effect.succeed(Effect.void),
197-
}),
198-
),
204+
});
205+
}),
199206
),
200207
httpClient: HttpClient.make((request) =>
201208
Deferred.succeed(probeStarted, undefined).pipe(
202209
Effect.andThen(Deferred.await(allowReady)),
203210
Effect.as(HttpClientResponse.fromWeb(request, new Response(null, { status: 200 }))),
204211
),
205212
),
206-
onReady: Deferred.succeed(ready, undefined).pipe(Effect.asVoid),
213+
onReady: Deferred.succeed(ready, undefined).pipe(
214+
Effect.andThen(Queue.offer(readyRuns, undefined)),
215+
Effect.asVoid,
216+
),
207217
});
208218
const publishRuntime = (pid: number) =>
209219
files.set(
@@ -223,6 +233,7 @@ const makeUpdateMarkerFixture = (backendMode: "native" | "WSL") =>
223233
probeStarted,
224234
allowReady,
225235
ready,
236+
readyRuns,
226237
runtimeChecked,
227238
markerRemoved,
228239
publishRuntime,
@@ -315,6 +326,58 @@ describe("DesktopBackendPool", () => {
315326
),
316327
);
317328

329+
it.effect("leaves a replacement update marker written while an earlier wait is pending", () =>
330+
Effect.scoped(
331+
Effect.gen(function* () {
332+
const fixture = yield* makeUpdateMarkerFixture("native");
333+
yield* Effect.gen(function* () {
334+
const pool = yield* DesktopBackendPool.DesktopBackendPool;
335+
const primary = yield* pool.primary;
336+
yield* primary.start;
337+
yield* Deferred.succeed(fixture.allowReady, undefined);
338+
yield* Deferred.await(fixture.ready);
339+
yield* Deferred.await(fixture.runtimeChecked);
340+
yield* TestClock.adjust(Duration.millis(DESKTOP_UPDATE_IN_PROGRESS_TTL_MS - 1_000));
341+
const replacement = JSON.stringify({ startedAt: 59_000, expiresAt: 119_000 });
342+
fixture.files.set(markerPath, replacement);
343+
yield* TestClock.adjust(Duration.millis(DESKTOP_UPDATE_IN_PROGRESS_TTL_MS));
344+
assert.equal(fixture.files.get(markerPath), replacement);
345+
assert.deepEqual(fixture.removed, []);
346+
}).pipe(Effect.provide(fixture.layer));
347+
}),
348+
),
349+
);
350+
351+
it.effect("a later backend run's readiness supersedes the earlier run's marker wait", () =>
352+
Effect.scoped(
353+
Effect.gen(function* () {
354+
const fixture = yield* makeUpdateMarkerFixture("native");
355+
yield* Effect.gen(function* () {
356+
const pool = yield* DesktopBackendPool.DesktopBackendPool;
357+
const primary = yield* pool.primary;
358+
yield* primary.start;
359+
yield* Deferred.succeed(fixture.allowReady, undefined);
360+
yield* Queue.take(fixture.readyRuns);
361+
yield* Deferred.await(fixture.runtimeChecked);
362+
yield* TestClock.adjust(Duration.seconds(30));
363+
yield* primary.stop();
364+
yield* primary.start;
365+
yield* Queue.take(fixture.readyRuns);
366+
yield* TestClock.adjust(Duration.seconds(31));
367+
assert.isTrue(
368+
fixture.files.has(markerPath),
369+
"the earlier run's wait must not end the later run's handoff",
370+
);
371+
assert.deepEqual(fixture.removed, []);
372+
fixture.publishRuntime(124);
373+
yield* TestClock.adjust(Duration.millis(100));
374+
yield* Deferred.await(fixture.markerRemoved);
375+
assert.deepEqual(fixture.removed, [markerPath]);
376+
}).pipe(Effect.provide(fixture.layer));
377+
}),
378+
),
379+
);
380+
318381
it.effect("unregister stops the registered instance", () =>
319382
Effect.scoped(
320383
Effect.gen(function* () {

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

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import * as Deferred from "effect/Deferred";
77
import * as Duration from "effect/Duration";
88
import * as Effect from "effect/Effect";
99
import * as Exit from "effect/Exit";
10+
import * as FiberHandle from "effect/FiberHandle";
1011
import * as Layer from "effect/Layer";
1112
import * as Option from "effect/Option";
1213
import * as Schedule from "effect/Schedule";
@@ -138,7 +139,8 @@ export const layer = Layer.effect(
138139
httpBaseUrl: URL,
139140
pid: number,
140141
) {
141-
if (!(yield* fileSystem.exists(updateMarkerPath).pipe(Effect.orElseSucceed(() => false)))) {
142+
const marker = yield* fileSystem.readFileString(updateMarkerPath).pipe(Effect.option);
143+
if (Option.isNone(marker)) {
142144
return;
143145
}
144146
yield* fileSystem.readFileString(serverRuntimePath).pipe(
@@ -151,8 +153,14 @@ export const layer = Layer.effect(
151153
}),
152154
Effect.timeoutOption(Duration.millis(DESKTOP_UPDATE_IN_PROGRESS_TTL_MS)),
153155
);
154-
yield* fileSystem.remove(updateMarkerPath, { force: true }).pipe(Effect.ignore);
156+
// A later update may have replaced the marker; that handoff is not ours to end.
157+
const current = yield* fileSystem.readFileString(updateMarkerPath).pipe(Effect.option);
158+
if (Option.isSome(current) && current.value === marker.value) {
159+
yield* fileSystem.remove(updateMarkerPath, { force: true }).pipe(Effect.ignore);
160+
}
155161
});
162+
// One wait at a time: each run's shutdown or readiness ends the previous run's wait.
163+
const updateMarkerWait = yield* FiberHandle.make<void>();
156164

157165
// Persist fatal Windows fallbacks; transient transport failures must preserve the user's WSL preference.
158166
const handlePrimaryPreflightFailure = Effect.fn("desktop.backendPool.primaryPreflightFailed")(
@@ -201,8 +209,7 @@ export const layer = Layer.effect(
201209
configResolve: configuration.resolvePrimary,
202210
// Absorb window errors so readiness can reset restartAttempt.
203211
onReady: (httpBaseUrl, pid) =>
204-
releaseUpdateMarker(httpBaseUrl, pid).pipe(
205-
Effect.forkIn(layerScope),
212+
FiberHandle.run(updateMarkerWait, releaseUpdateMarker(httpBaseUrl, pid)).pipe(
206213
Effect.andThen(desktopWindow.handleBackendReady(httpBaseUrl)),
207214
// Only once the main window exists, so the recovery window never closes into nothing.
208215
Effect.andThen(SubscriptionRef.set(primaryBackendStartup, { _tag: "Ready" })),
@@ -214,7 +221,10 @@ export const layer = Layer.effect(
214221
),
215222
onStartupFailure: (failure) =>
216223
SubscriptionRef.set(primaryBackendStartup, { _tag: "Failing", ...failure }),
217-
onShutdown: () => desktopWindow.handleBackendNotReady,
224+
onShutdown: () =>
225+
FiberHandle.clear(updateMarkerWait).pipe(
226+
Effect.andThen(desktopWindow.handleBackendNotReady),
227+
),
218228
onPreflightFailed: handlePrimaryPreflightFailure,
219229
onStateDirOwned: () =>
220230
electronDialog

0 commit comments

Comments
 (0)