From e46a8a0a75782ee120a8052ee4315cd4ee0a1995 Mon Sep 17 00:00:00 2001 From: binjamshid <143622128+binjamshid@users.noreply.github.com> Date: Wed, 11 Nov 2026 08:53:33 +0400 Subject: [PATCH] fix(server): archiving or deleting a thread releases its devices Archiving or deleting a thread, or deleting its project, left the thread's device sessions claimed, so another thread opening the same simulator or emulator got DEVICE_IN_USE. The per-thread teardown effect that archive and delete already queue now also releases each of the thread's device sessions the way device_close does, leaving the devices booted. Fixes #34165. --- .../src/guest/GuestShareService.test.ts | 3 +- .../src/orchestration-v2/EffectWorker.ts | 13 ++- .../ResourceCleanupService.ts | 22 +++- .../src/orchestration-v2/runtimeLayer.test.ts | 108 ++++++++++++++++++ .../src/provider/userInputAttachments.test.ts | 2 + 5 files changed, 145 insertions(+), 3 deletions(-) diff --git a/apps/server/src/guest/GuestShareService.test.ts b/apps/server/src/guest/GuestShareService.test.ts index 7140222478720..cf2f9e3f70782 100644 --- a/apps/server/src/guest/GuestShareService.test.ts +++ b/apps/server/src/guest/GuestShareService.test.ts @@ -90,7 +90,7 @@ const layerProviders = Layer.mock(ProviderRegistry.ProviderRegistry)({ getProviders: Effect.succeed([]), }); const layerConfig = ServerConfig.layerTest(process.cwd(), { prefix: "t3-guest-share-" }); -// The real cleanup also closes terminals and previews; these tests only need the files gone. +// The real cleanup also closes terminals, previews and devices; these tests only need the files gone. const layerCleanup = Layer.effect( ResourceCleanupService.ResourceCleanupService, Effect.gen(function* () { @@ -100,6 +100,7 @@ const layerCleanup = Layer.effect( cleanupScheduledTasks: () => Effect.void, cleanupTerminals: () => Effect.void, cleanupPreviews: () => Effect.void, + cleanupDevices: () => Effect.void, cleanupAttachments: (attachmentIds) => Effect.forEach( attachmentIds, diff --git a/apps/server/src/orchestration-v2/EffectWorker.ts b/apps/server/src/orchestration-v2/EffectWorker.ts index c5b5ea9b383a9..f9b8d14953546 100644 --- a/apps/server/src/orchestration-v2/EffectWorker.ts +++ b/apps/server/src/orchestration-v2/EffectWorker.ts @@ -505,8 +505,19 @@ export const layerExecutor: Layer.Layer< }), ), ); + // Archive and delete queue this one per-thread teardown, so it also releases the + // thread's device claims; the devices stay booted for other threads. case "terminal.cleanup": - return resourceCleanup.cleanupTerminals(effect.threadId).pipe( + return Effect.partition( + [ + resourceCleanup.cleanupTerminals(effect.threadId), + resourceCleanup.cleanupDevices(effect.threadId), + ], + (cleanup) => cleanup, + ).pipe( + Effect.flatMap(([, [failure]]) => + failure === undefined ? Effect.void : Effect.fail(failure), + ), Effect.mapError( (cause) => new OrchestrationEffectExecutionError({ diff --git a/apps/server/src/orchestration-v2/ResourceCleanupService.ts b/apps/server/src/orchestration-v2/ResourceCleanupService.ts index ff56b42c492c1..fd45ecac667ef 100644 --- a/apps/server/src/orchestration-v2/ResourceCleanupService.ts +++ b/apps/server/src/orchestration-v2/ResourceCleanupService.ts @@ -8,6 +8,7 @@ import * as Schema from "effect/Schema"; import { resolveAttachmentRelativePath } from "../attachmentPaths.ts"; import { parseAttachmentFileExtension, resolveAttachmentPathById } from "../attachmentStore.ts"; import * as ServerConfig from "../config.ts"; +import * as DeviceService from "../device/DeviceService.ts"; import * as PreviewManager from "../preview/Manager.ts"; import * as ScheduledTaskService from "../scheduledTasks/ScheduledTaskService.ts"; import * as TerminalManager from "../terminal/Manager.ts"; @@ -15,7 +16,7 @@ import * as TerminalManager from "../terminal/Manager.ts"; export class ResourceCleanupError extends Schema.TaggedError()( "ResourceCleanupError", { - operation: Schema.Literals(["terminal", "preview", "attachment", "scheduled-tasks"]), + operation: Schema.Literals(["terminal", "preview", "device", "attachment", "scheduled-tasks"]), threadId: Schema.optional(Schema.String), attachmentId: Schema.optional(Schema.String), cause: Schema.Defect(), @@ -26,6 +27,8 @@ export class ResourceCleanupService extends Context.Reference<{ readonly cleanupScheduledTasks: (threadId: ThreadId) => Effect.Effect; readonly cleanupTerminals: (threadId: string) => Effect.Effect; readonly cleanupPreviews: (threadId: string) => Effect.Effect; + /** Releases the thread's device claims, as `device_close` does; the devices stay booted. */ + readonly cleanupDevices: (threadId: ThreadId) => Effect.Effect; readonly cleanupAttachments: ( attachmentIds: ReadonlyArray, ) => Effect.Effect; @@ -34,6 +37,7 @@ export class ResourceCleanupService extends Context.Reference<{ cleanupScheduledTasks: () => Effect.void, cleanupTerminals: () => Effect.void, cleanupPreviews: () => Effect.void, + cleanupDevices: () => Effect.void, cleanupAttachments: () => Effect.void, }), }) {} @@ -43,6 +47,7 @@ export const layer = Layer.effect( Effect.gen(function* () { const terminals = yield* TerminalManager.TerminalManager; const previews = yield* PreviewManager.PreviewManager; + const devices = yield* DeviceService.DeviceService; const fileSystem = yield* FileSystem.FileSystem; const config = yield* ServerConfig.ServerConfig; const scheduledTasks = yield* ScheduledTaskService.ScheduledTaskService; @@ -81,6 +86,21 @@ export const layer = Layer.effect( ), ); }), + cleanupDevices: Effect.fn("ResourceCleanupService.cleanupDevices")(function* ( + threadId: ThreadId, + ) { + // One close per session, so a reconnecting host does not keep the other hosts' claims. + const sessions = yield* devices.sessionsForThread(threadId); + const [, failures] = yield* Effect.partition( + sessions, + (session) => + devices.close({ threadId, hostId: session.hostId, deviceId: session.deviceId }), + { concurrency: 1 }, + ); + const [failure] = failures; + if (failure !== undefined) + return yield* new ResourceCleanupError({ operation: "device", threadId, cause: failure }); + }), cleanupAttachments: Effect.fn("ResourceCleanupService.cleanupAttachments")(function ( attachmentIds: ReadonlyArray, ) { diff --git a/apps/server/src/orchestration-v2/runtimeLayer.test.ts b/apps/server/src/orchestration-v2/runtimeLayer.test.ts index a49e323770cc1..bccb3db73c0ff 100644 --- a/apps/server/src/orchestration-v2/runtimeLayer.test.ts +++ b/apps/server/src/orchestration-v2/runtimeLayer.test.ts @@ -87,6 +87,8 @@ import * as ScheduledTaskService from "../scheduledTasks/ScheduledTaskService.ts import * as ManagedPolicy from "../policy/ManagedPolicy.ts"; import * as Scheduler from "../scheduling/Scheduler.ts"; import * as SecretRequests from "../secrets/SecretRequests.ts"; +import * as DeviceHost from "../device/DeviceHost.ts"; +import * as DeviceService from "../device/DeviceService.ts"; import * as PreviewManager from "../preview/Manager.ts"; import * as TerminalManager from "../terminal/Manager.ts"; import * as ResourceCleanupService from "./ResourceCleanupService.ts"; @@ -367,8 +369,59 @@ const layerScheduledTasksTest = ScheduledTaskService.layer.pipe( ), ), ); +const bootedSimulatorReady: DeviceHost.DeviceHostAgentReady = { + nodePath: process.execPath, + hub: { origin: "http://device.test" }, + agentDevice: null, + helpers: { serveSimAxSettings: null, serveSimCli: null }, + run: () => Effect.die("Thread cleanup must not run host commands"), + releaseAgentSession: () => Effect.void, + devices: { + list: Effect.succeed({ + devices: [ + { + id: "phone", + name: "iPhone", + platform: "ios", + version: "26", + booted: true, + physical: false, + }, + ], + detail: undefined, + }), + boot: () => Effect.die("The simulator is already booted"), + attach: () => Effect.void, + shutdown: () => Effect.die("Thread cleanup must keep the simulator booted"), + screenshot: () => Effect.die("unused screenshot"), + }, +}; +const bootedSimulatorHost = DeviceHost.DeviceHost.of({ + id: "local", + label: "Local", + summary: Effect.succeed({ + id: "local", + label: "Local", + kind: "local", + hubInstalled: true, + agentDeviceInstalled: true, + platforms: [{ platform: "ios", available: true }], + }), + platformAvailability: (platform) => Effect.succeed({ platform, available: true }), + ensureReady: () => Effect.succeed(bootedSimulatorReady), + ensureAgentReady: () => Effect.succeed(bootedSimulatorReady), + current: Effect.succeed(bootedSimulatorReady), + stopAgent: Effect.void, + stop: Effect.void, +}); const layerResourceCleanupTest = ResourceCleanupService.layer.pipe( Layer.provideMerge(layerScheduledTasksTest), + Layer.provideMerge( + Layer.effect( + DeviceService.DeviceService, + DeviceService.makeWithHosts(new Map([[bootedSimulatorHost.id, bootedSimulatorHost]])), + ), + ), Layer.provide( Layer.mergeAll( Layer.mock(PreviewManager.PreviewManager)({ close: () => Effect.void }), @@ -544,6 +597,61 @@ it.layer(layerProjectDeletionTest)("project deletion during thread commands", (i }), ); + it.effect.each(["archive", "thread delete", "project delete"] as const)( + "%s releases the thread's devices and keeps them booted", + (lifecycle) => + Effect.gen(function* () { + const projects = yield* ProjectService.ProjectService; + const orchestrator = yield* Orchestrator.OrchestratorV2; + const worker = yield* EffectWorker.OrchestrationEffectWorkerV2; + const devices = yield* DeviceService.DeviceService; + yield* devices.configure({ enabled: true, agentAccessEnabled: true }); + const projectId = ProjectId.make(`device-cleanup-project:${lifecycle}`); + const threadId = ThreadId.make(`device-cleanup-thread:${lifecycle}`); + const otherThreadId = ThreadId.make(`device-surviving-thread:${lifecycle}`); + yield* projects.create({ + commandId: CommandId.make(`device-cleanup-create-project:${lifecycle}`), + projectId, + title: "Device cleanup", + workspaceRoot: `/work/device-cleanup/${lifecycle}`, + }); + for (const id of [threadId, otherThreadId]) { + yield* orchestrator.dispatch({ + type: "thread.create", + commandId: CommandId.make(`device-cleanup-create:${id}`), + createdBy: "user", + creationSource: "web", + threadId: id, + projectId: id === threadId ? projectId : ProjectId.make("device-cleanup-other-project"), + title: "Device user", + modelSelection, + runtimeMode: "full-access", + interactionMode: "default", + branch: null, + worktreePath: null, + }); + yield* devices.open({ threadId: id, deviceId: "phone", platform: "ios" }); + } + const otherSessions = yield* devices.sessionsForThread(otherThreadId); + + const commandId = CommandId.make(`device-cleanup:${lifecycle}`); + if (lifecycle === "project delete") { + yield* projects.delete({ commandId, projectId, force: true }); + } else { + yield* orchestrator.dispatch({ + type: lifecycle === "archive" ? "thread.archive" : "thread.delete", + commandId, + threadId, + }); + } + yield* worker.drain(); + + assert.deepEqual(yield* devices.sessionsForThread(threadId), []); + assert.deepEqual(yield* devices.sessionsForThread(otherThreadId), otherSessions); + assert.isTrue((yield* devices.state).devices[0]?.booted); + }), + ); + it.effect("waits for an in-flight thread update before planning deletion", () => Effect.gen(function* () { const projects = yield* ProjectService.ProjectService; diff --git a/apps/server/src/provider/userInputAttachments.test.ts b/apps/server/src/provider/userInputAttachments.test.ts index f9b41babaea2f..aacc345b3cfef 100644 --- a/apps/server/src/provider/userInputAttachments.test.ts +++ b/apps/server/src/provider/userInputAttachments.test.ts @@ -16,6 +16,7 @@ import * as ServerConfig from "../config.ts"; import * as DocumentText from "./DocumentText.ts"; import { extractDocumentText } from "./documentTextWorker.ts"; import * as Cleanup from "../orchestration-v2/ResourceCleanupService.ts"; +import * as DeviceService from "../device/DeviceService.ts"; import * as TerminalManager from "../terminal/Manager.ts"; import * as PreviewManager from "../preview/Manager.ts"; import * as ScheduledTaskService from "../scheduledTasks/ScheduledTaskService.ts"; @@ -328,6 +329,7 @@ describe("question answer paths", () => { Layer.mergeAll( Layer.mock(TerminalManager.TerminalManager)({}), Layer.mock(PreviewManager.PreviewManager)({}), + Layer.mock(DeviceService.DeviceService)({}), Layer.mock(ScheduledTaskService.ScheduledTaskService)({}), ), ),