Skip to content

Commit de7be1e

Browse files
only21milclaude
only21mil
and
claude
committed
fix(server): keep device support off when a start fails after it is turned off
The device list and host retry published a start's failure without checking settings, so a start that ended after device support was turned off marked the local host "failed", and the setup switch read that as enabled. They now publish under the lifecycle lock and only while device support is on. An agent start that ends after agent access was turned off returns no readiness, so a retry starts the device hub alone, as before the lock change. Co-Authored-By: Claude Opus 5.5 <[email protected]>
1 parent 771ef5c commit de7be1e

2 files changed

Lines changed: 123 additions & 24 deletions

File tree

‎apps/server/src/device/DeviceMultiHost.test.ts‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -306,3 +306,99 @@ it.effect("stops a start that fails after device support was turned off before i
306306
),
307307
),
308308
);
309+
310+
// A local host whose `pausing` start waits until the test lets it finish.
311+
const pausedLocalHost = (pausing: "ensureReady" | "ensureAgentReady") =>
312+
Effect.gen(function* () {
313+
const started = yield* Deferred.make<void>();
314+
const finish = yield* Deferred.make<void>();
315+
const calls: string[] = [];
316+
const run = () => Effect.succeed({ stdout: "", stderr: "", code: 0 });
317+
const ready: DeviceHost.DeviceHostAgentReady = {
318+
nodePath: process.execPath,
319+
hub: { origin: "http://local" },
320+
devices: makeDeviceHubOperations({
321+
hostId: LOCAL_DEVICE_HOST_ID,
322+
origin: "http://local",
323+
http: HttpClient.make((request) =>
324+
Effect.succeed(
325+
HttpClientResponse.fromWeb(request, Response.json({ simulators: [], emulators: [] })),
326+
),
327+
),
328+
run,
329+
androidAvailable: Effect.succeed(false),
330+
}),
331+
agentDevice: null,
332+
run,
333+
helpers: { serveSimAxSettings: null, serveSimCli: null },
334+
};
335+
const start = (step: typeof pausing) =>
336+
step === pausing
337+
? Deferred.succeed(started, undefined).pipe(
338+
Effect.andThen(Deferred.await(finish)),
339+
Effect.as(ready),
340+
)
341+
: Effect.succeed(ready);
342+
const host: DeviceHost.DeviceHost["Service"] = {
343+
id: LOCAL_DEVICE_HOST_ID,
344+
label: "Local",
345+
summary: Effect.succeed({
346+
id: LOCAL_DEVICE_HOST_ID,
347+
label: "Local",
348+
kind: "local",
349+
hubInstalled: true,
350+
agentDeviceInstalled: true,
351+
platforms: [{ platform: "android", available: true }],
352+
}),
353+
platformAvailability: (platform) => Effect.succeed({ platform, available: true }),
354+
ensureReady: () => start("ensureReady"),
355+
ensureAgentReady: () => start("ensureAgentReady"),
356+
current: Effect.succeed(null),
357+
stopAgent: Effect.sync(() => {
358+
calls.push("stopAgent");
359+
}),
360+
stop: Effect.sync(() => {
361+
calls.push("stop");
362+
}),
363+
};
364+
return { host, started, finish, calls };
365+
});
366+
367+
it.effect("stays disabled when device support is turned off while listing starts a host", () =>
368+
Effect.gen(function* () {
369+
const { host, started, finish, calls } = yield* pausedLocalHost("ensureReady");
370+
const service = yield* DeviceService.makeWithHosts(new Map([[host.id, host]])).pipe(
371+
Effect.provide(NodeCrypto.layer),
372+
);
373+
const listing = yield* service.list.pipe(Effect.forkChild);
374+
yield* Deferred.await(started);
375+
yield* service.configure({ enabled: false });
376+
yield* Deferred.succeed(finish, undefined);
377+
yield* Fiber.join(listing);
378+
expect(calls).toEqual(["stop", "stop"]);
379+
const state = yield* service.state;
380+
expect(state.hostStatus).toBe("disabled");
381+
expect(state.hostStatuses).toEqual({});
382+
}).pipe(Effect.provide(ServerSettings.layerTest({ enableDeviceSupport: true }))),
383+
);
384+
385+
it.effect("retries without agent tools when agent access is turned off while they start", () =>
386+
Effect.gen(function* () {
387+
const { host, started, finish, calls } = yield* pausedLocalHost("ensureAgentReady");
388+
const service = yield* DeviceService.makeWithHosts(new Map([[host.id, host]])).pipe(
389+
Effect.provide(NodeCrypto.layer),
390+
);
391+
const retrying = yield* service.retryHost(LOCAL_DEVICE_HOST_ID).pipe(Effect.forkChild);
392+
yield* Deferred.await(started);
393+
yield* service.configure({ agentAccessEnabled: false });
394+
yield* Deferred.succeed(finish, undefined);
395+
const state = yield* Fiber.join(retrying);
396+
expect(calls).toEqual(["stopAgent", "stopAgent"]);
397+
expect(state.agentAccessEnabled).toBe(false);
398+
expect(state.hostStatus).toBe("ready");
399+
}).pipe(
400+
Effect.provide(
401+
ServerSettings.layerTest({ enableDeviceSupport: true, enableAgentDeviceAccess: true }),
402+
),
403+
),
404+
);

‎apps/server/src/device/DeviceService.ts‎

Lines changed: 27 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -297,23 +297,20 @@ export const makeWithHosts = Effect.fn("DeviceService.makeWithHosts")(function*
297297
),
298298
);
299299

300+
// Whether a start that just ended is still wanted; a replaced host fails it.
300301
const confirmStarted = Effect.fn("DeviceService.confirmStarted")(function* (
301302
host: DeviceHost.DeviceHost["Service"],
302303
agent: boolean,
303304
) {
304305
const current = yield* readDeviceSettings;
305-
if (!current.enabled || (agent && !current.agentAccessEnabled))
306-
return yield* new DeviceHostUnavailableError({
307-
hostId: host.id,
308-
hostLabel: host.label,
309-
reason: `${current.enabled ? "Agent device access" : "Device support"} was turned off while the host was starting.`,
310-
});
306+
if (!current.enabled || (agent && !current.agentAccessEnabled)) return false;
311307
if (hosts.get(host.id) !== host)
312308
return yield* new DeviceHostUnavailableError({
313309
hostId: host.id,
314310
hostLabel: host.label,
315311
reason: "Host configuration changed. Retry the operation.",
316312
});
313+
return true;
317314
});
318315

319316
const readiness: DeviceService["Service"]["readiness"] = Effect.fn("DeviceService.readiness")(
@@ -359,7 +356,12 @@ export const makeWithHosts = Effect.fn("DeviceService.makeWithHosts")(function*
359356
);
360357
return yield* lifecycleLock.withPermit(
361358
Effect.gen(function* () {
362-
yield* confirmStarted(host, false);
359+
if (!(yield* confirmStarted(host, false)))
360+
return yield* new DeviceHostUnavailableError({
361+
hostId: host.id,
362+
hostLabel: host.label,
363+
reason: "Device support was turned off while the host was starting.",
364+
});
363365
const { state } = yield* SynchronizedRef.get(stateRef);
364366
if (state.hostStatuses[host.id]?.status !== "ready") {
365367
yield* setHostStatus(host.id, { status: "ready" });
@@ -425,7 +427,7 @@ export const makeWithHosts = Effect.fn("DeviceService.makeWithHosts")(function*
425427
);
426428
return yield* lifecycleLock.withPermit(
427429
Effect.gen(function* () {
428-
yield* confirmStarted(host, true);
430+
if (!(yield* confirmStarted(host, true))) return null;
429431
const hostSummaries = yield* Effect.forEach(
430432
hosts.values(),
431433
(candidate) => candidate.summary,
@@ -499,6 +501,21 @@ export const makeWithHosts = Effect.fn("DeviceService.makeWithHosts")(function*
499501
);
500502
});
501503

504+
// Checked under the lock configure publishes under, so a failure that ends
505+
// after device support was turned off cannot mark the setting back on.
506+
const publishStartFailure = (hostId: DeviceHostId, error: DeviceError) =>
507+
lifecycleLock
508+
.withPermit(
509+
Effect.gen(function* () {
510+
if (!(yield* readDeviceSettings).enabled) return;
511+
yield* setHostStatus(hostId, {
512+
status: "failed",
513+
detail: error._tag === "DeviceHostUnavailableError" ? error.reason : error.message,
514+
});
515+
}),
516+
)
517+
.pipe(Effect.ignore);
518+
502519
const list: DeviceService["Service"]["list"] = Effect.gen(function* () {
503520
if (!(yield* readDeviceSettings).enabled) return (yield* SynchronizedRef.get(stateRef)).state;
504521
yield* Effect.forEach(
@@ -507,14 +524,7 @@ export const makeWithHosts = Effect.fn("DeviceService.makeWithHosts")(function*
507524
Effect.gen(function* () {
508525
const ready = yield* readinessIfSupported(host.id);
509526
if (ready) yield* refresh(ready);
510-
}).pipe(
511-
Effect.catch((error) =>
512-
setHostStatus(host.id, {
513-
status: "failed",
514-
detail: error._tag === "DeviceHostUnavailableError" ? error.reason : error.message,
515-
}),
516-
),
517-
),
527+
}).pipe(Effect.catch((error) => publishStartFailure(host.id, error))),
518528
{ concurrency: 4 },
519529
);
520530
return (yield* SynchronizedRef.get(stateRef)).state;
@@ -560,14 +570,7 @@ export const makeWithHosts = Effect.fn("DeviceService.makeWithHosts")(function*
560570
const ready =
561571
(yield* agentReadinessIfSupported(hostId)) ?? (yield* readinessIfSupported(hostId));
562572
if (ready) yield* refresh(ready);
563-
}).pipe(
564-
Effect.catch((error) =>
565-
setHostStatus(hostId, {
566-
status: "failed",
567-
detail: error._tag === "DeviceHostUnavailableError" ? error.reason : error.message,
568-
}),
569-
),
570-
);
573+
}).pipe(Effect.catch((error) => publishStartFailure(hostId, error)));
571574
return (yield* SynchronizedRef.get(stateRef)).state;
572575
},
573576
);

0 commit comments

Comments
 (0)