From 790be6d755026d61bd13a970dd77176c12ca0425 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sun, 20 Sep 2026 22:08:51 -0700 Subject: [PATCH] fix(devices): safely reclaim obsolete managed tool versions (#12819) --- apps/server/src/device/DeviceService.ts | 2 +- apps/server/src/device/LocalDeviceHost.ts | 11 ++ .../src/device/deviceToolMaintenance.test.ts | 159 ++++++++++++++++++ .../src/device/deviceToolMaintenance.ts | 148 ++++++++++++++++ .../server/src/device/sshDeviceScript.test.ts | 3 + apps/server/src/device/sshDeviceScript.ts | 3 + 6 files changed, 325 insertions(+), 1 deletion(-) create mode 100644 apps/server/src/device/deviceToolMaintenance.test.ts create mode 100644 apps/server/src/device/deviceToolMaintenance.ts diff --git a/apps/server/src/device/DeviceService.ts b/apps/server/src/device/DeviceService.ts index ddbe07ad9de430dc52a523f8cd0b49046445a1ae..f99522b2bd7b54e3e101bcc740fb36d58bbc4c47 100644 --- a/apps/server/src/device/DeviceService.ts +++ b/apps/server/src/device/DeviceService.ts @@ -1111,7 +1111,7 @@ export const make = Effect.gen(function* () { return { ...service, agentCli: resolveNodeExecutable("Device automation").pipe( - Effect.flatMap(() => ensureAgentDevice(config.baseDir)), + Effect.andThen(ensureAgentDevice(config.baseDir)), Effect.provideService(FileSystem.FileSystem, fs), Effect.provideService(Path.Path, path), Effect.provideService(ProcessRunner.ProcessRunner, runner), diff --git a/apps/server/src/device/LocalDeviceHost.ts b/apps/server/src/device/LocalDeviceHost.ts index 0f7992c613796dd936eb438191742642c0302ea2..e704d28bbc34edafa9504dc4a5d0e60530bcfa79 100644 --- a/apps/server/src/device/LocalDeviceHost.ts +++ b/apps/server/src/device/LocalDeviceHost.ts @@ -1,3 +1,4 @@ +import { pruneLocalDeviceTools } from "./deviceToolMaintenance.ts"; import { deviceToolInstallMessage } from "@t3tools/contracts"; /** * The device host that is this machine. @@ -593,6 +594,11 @@ export const make = Effect.fn("LocalDeviceHost.make")(function* () { ); yield* onPhase("starting"); const hub = yield* spawnHub(hubTool, nodePath); + yield* pruneLocalDeviceTools(config.baseDir, nodePath, "hub").pipe( + Effect.provideService(Path.Path, path), + Effect.provideService(ProcessRunner.ProcessRunner, runner), + Effect.ignore, + ); const candidate = helperPaths(hubTool); const [axExists, cliExists] = yield* Effect.all([ fs.exists(candidate.serveSimAxSettings).pipe(Effect.orElseSucceed(() => false)), @@ -655,6 +661,11 @@ export const make = Effect.fn("LocalDeviceHost.make")(function* () { agentToolRef = { entryPath: agentTool.entryPath, nodePath: running.hub.nodePath }; yield* onPhase("starting"); const agentDevice = yield* startAgentDeviceDaemon(agentTool, running.hub.nodePath); + yield* pruneLocalDeviceTools(config.baseDir, running.hub.nodePath, "agent").pipe( + Effect.provideService(Path.Path, path), + Effect.provideService(ProcessRunner.ProcessRunner, runner), + Effect.ignore, + ); const next = { ...running, agentDevice }; yield* Ref.set(runningRef, next); return { ...toReady(next), agentDevice }; diff --git a/apps/server/src/device/deviceToolMaintenance.test.ts b/apps/server/src/device/deviceToolMaintenance.test.ts new file mode 100644 index 0000000000000000000000000000000000000000..4c46be11bba4f478267ad484fbff70a20a06581c --- /dev/null +++ b/apps/server/src/device/deviceToolMaintenance.test.ts @@ -0,0 +1,159 @@ +import * as Effect from "effect/Effect"; +import * as NodePathLayer from "@effect/platform-node/NodePath"; +import * as ProcessRunner from "../processRunner.ts"; +import * as ChildProcessSpawner from "effect/unstable/process/ChildProcessSpawner"; +// @effect-diagnostics nodeBuiltinImport:off - tests the same standalone script used by local and SSH hosts. +import { describe, expect, it } from "@effect/vitest"; +import * as NodeFSP from "node:fs/promises"; +import * as NodeOS from "node:os"; +import * as NodePath from "node:path"; +import * as NodeChildProcess from "node:child_process"; +import * as NodeUtil from "node:util"; +import { pruneLocalDeviceTools, deviceToolMaintenanceScript } from "./deviceToolMaintenance.ts"; + +const exec = NodeUtil.promisify(NodeChildProcess.execFile); + +describe.each([false, true])("device tool cleanup, flat=%s", (flat) => { + it("keeps current, previous, active and incomplete installs, pruning unused completed versions", async () => { + const root = await NodeFSP.mkdtemp(NodePath.join(NodeOS.tmpdir(), "t3-tool-cleanup-")); + const name = "expo-device-hub"; + const directory = (version: string) => + flat ? NodePath.join(root, `${name}@${version}`) : NodePath.join(root, name, version); + try { + for (const version of ["0.1.0", "0.2.0", "0.3.0", "0.4.0", "0.5.0", "0.6.0"]) { + await NodeFSP.mkdir(directory(version), { recursive: true }); + if (version === "0.5.0") continue; + const sentinel = NodePath.join(directory(version), ".install-complete"); + await NodeFSP.writeFile(sentinel, version); + await NodeFSP.utimes( + sentinel, + Number(version.split(".")[1]), + Number(version.split(".")[1]), + ); + } + const script = + deviceToolMaintenanceScript + + ` +(async () => { + const root = ${JSON.stringify(root)}; + await pruneTools(root, [['${name}', '0.6.0']], ${flat}); +})().catch(error => { console.error(error); process.exitCode = 1; });`; + await exec(process.execPath, [ + "-e", + script, + NodePath.join(directory("0.2.0"), "active-helper.cjs"), + ]); + await expect(NodeFSP.stat(directory("0.1.0"))).rejects.toThrow(); + await expect(NodeFSP.stat(directory("0.3.0"))).rejects.toThrow(); + for (const version of ["0.2.0", "0.4.0", "0.5.0", "0.6.0"]) + expect((await NodeFSP.stat(directory(version))).isDirectory()).toBe(true); + } finally { + await NodeFSP.rm(root, { recursive: true, force: true }); + } + }); + + it("keeps every install when the process scan fails", async () => { + const root = await NodeFSP.mkdtemp(NodePath.join(NodeOS.tmpdir(), "t3-tool-scan-")); + try { + for (const version of ["0.1.0", "0.2.0", "0.3.0"]) { + const dir = flat + ? NodePath.join(root, `expo-device-hub@${version}`) + : NodePath.join(root, "expo-device-hub", version); + await NodeFSP.mkdir(dir, { recursive: true }); + await NodeFSP.writeFile(NodePath.join(dir, ".install-complete"), version); + } + await exec(process.execPath, [ + "-e", + deviceToolMaintenanceScript + + ` + require('node:child_process').spawnSync = () => ({ status: 1, stdout: '' }); + pruneTools(${JSON.stringify(root)}, [['expo-device-hub', '0.3.0']], ${flat}).catch(() => process.exitCode = 1); + `, + ]); + const parent = flat ? root : NodePath.join(root, "expo-device-hub"); + expect((await NodeFSP.readdir(parent)).length).toBe(3); + } finally { + await NodeFSP.rm(root, { recursive: true, force: true }); + } + }); + + it("does not prune before the required version has completed installation", async () => { + const root = await NodeFSP.mkdtemp(NodePath.join(NodeOS.tmpdir(), "t3-tool-cleanup-")); + try { + const dir = flat + ? NodePath.join(root, "expo-device-hub@0.1.0") + : NodePath.join(root, "expo-device-hub/0.1.0"); + await NodeFSP.mkdir(dir, { recursive: true }); + await NodeFSP.writeFile(NodePath.join(dir, ".install-complete"), "0.1.0"); + await exec(process.execPath, [ + "-e", + deviceToolMaintenanceScript + + `pruneTools(${JSON.stringify(root)}, [['expo-device-hub','0.6.0']], ${flat}).catch(() => process.exitCode = 1);`, + ]); + expect((await NodeFSP.stat(dir)).isDirectory()).toBe(true); + } finally { + await NodeFSP.rm(root, { recursive: true, force: true }); + } + }); +}); + +it.effect("maintenance failures retain safe context and the original process result", () => + Effect.gen(function* () { + const output = { + code: ChildProcessSpawner.ExitCode(1), + stdout: "", + stderr: "private child diagnostics", + timedOut: false, + stdoutTruncated: false, + stderrTruncated: false, + stdoutInvalidUtf8: false, + stderrInvalidUtf8: false, + }; + for (const [operation, run] of [["prune", pruneLocalDeviceTools]] as const) { + const error = yield* run("/tools", process.execPath, "hub").pipe( + Effect.provideService(ProcessRunner.ProcessRunner, { run: () => Effect.succeed(output) }), + Effect.flip, + ); + expect(error).toMatchObject({ + _tag: "DeviceToolMaintenanceError", + operation, + tool: "hub", + exitCode: 1, + cause: output, + }); + expect(error.message).toBe(`Device tool ${operation} failed for hub (exit code 1).`); + expect(error.message).not.toContain(output.stderr); + } + }).pipe(Effect.provide(NodePathLayer.layer)), +); + +it("serializes competing maintenance processes after reclaiming a stale lock", async () => { + const root = await NodeFSP.mkdtemp(NodePath.join(NodeOS.tmpdir(), "t3-tool-contention-")); + try { + const lock = NodePath.join(root, ".maintenance-lock"); + await NodeFSP.mkdir(lock); + await NodeFSP.writeFile( + NodePath.join(lock, "stale-owner.json"), + JSON.stringify({ pid: 2147483647, identity: "dead" }), + ); + const script = + deviceToolMaintenanceScript + + ` +(async () => { + const root = ${JSON.stringify(root)}; + const marker = maintenancePath.join(root, 'critical-section'); + for (let attempt = 0; attempt < 8; attempt++) await withToolMaintenance(root, () => { + maintenanceFs.writeFileSync(marker, String(process.pid), { flag: 'wx' }); + for (let check = 0; check < 100; check++) { + if (maintenanceFs.readFileSync(marker, 'utf8') !== String(process.pid)) throw Error('Overlapping maintenance'); + } + maintenanceFs.unlinkSync(marker); + }); +})().catch(error => { console.error(error); process.exitCode = 1; });`; + await Promise.all(Array.from({ length: 6 }, () => exec(process.execPath, ["-e", script]))); + await expect(NodeFSP.stat(lock)).rejects.toThrow(); + expect(await NodeFSP.readdir(root)).toEqual([]); + } finally { + await NodeFSP.rm(root, { recursive: true, force: true }); + } +}); diff --git a/apps/server/src/device/deviceToolMaintenance.ts b/apps/server/src/device/deviceToolMaintenance.ts new file mode 100644 index 0000000000000000000000000000000000000000..43cc64e8facac6d353e5989f378de08bab41b82d --- /dev/null +++ b/apps/server/src/device/deviceToolMaintenance.ts @@ -0,0 +1,148 @@ +// @effect-diagnostics preferSchemaOverJson:off - JSON string literals safely embed paths and arguments in generated JavaScript. +import * as Schema from "effect/Schema"; +import * as Effect from "effect/Effect"; +import * as Path from "effect/Path"; +import * as ProcessRunner from "../processRunner.ts"; +import { AGENT_DEVICE_VERSION, DEVICE_HUB_VERSION } from "./DeviceToolchain.ts"; + +/** Shared with the SSH bootstrap. Cleanup runs only after successful startup. */ +export const deviceToolMaintenanceScript = String.raw` +const maintenanceFs = require('node:fs'); +const maintenancePath = require('node:path'); +const maintenanceAlive = pid => { + try { process.kill(pid, 0); return true; } + catch (error) { return error.code !== 'ESRCH'; } +}; +async function withToolMaintenance(root, operation) { + maintenanceFs.mkdirSync(root, { recursive: true }); + const lock = maintenancePath.join(root, '.maintenance-lock'); + const nonce = require('node:crypto').randomUUID(); + const ownerFile = process.pid + '.' + nonce + '.json'; + const candidate = lock + '.' + nonce; + const holder = { pid: process.pid }; + const deadline = Date.now() + 30000; + const removeEmptyLock = () => { + try { maintenanceFs.rmdirSync(lock); } + catch (error) { if (!['ENOENT', 'ENOTEMPTY', 'EEXIST', 'EPERM'].includes(error.code)) throw error; } + }; + maintenanceFs.mkdirSync(candidate); + try { + maintenanceFs.writeFileSync(maintenancePath.join(candidate, ownerFile), JSON.stringify(holder)); + while (true) { + try { + // Publish a populated directory atomically; rename cannot replace another populated lock. + maintenanceFs.renameSync(candidate, lock); + break; + } + catch (error) { + if (!['EEXIST', 'ENOTEMPTY', 'EPERM', 'EACCES'].includes(error.code)) throw error; + let files = []; + try { files = maintenanceFs.readdirSync(lock); } catch (error) { if (error.code !== 'ENOENT') throw error; } + if (files.length === 1) { + const previousFile = maintenancePath.join(lock, files[0]); + let previous; + try { previous = JSON.parse(maintenanceFs.readFileSync(previousFile, 'utf8')); } catch {} + if (Number.isSafeInteger(previous?.pid) && previous.pid > 0 && !maintenanceAlive(previous.pid)) { + // The unique filename belongs only to that owner. Never unlink a replacement owner's file. + try { maintenanceFs.unlinkSync(previousFile); } catch (error) { if (error.code !== 'ENOENT') throw error; } + } + } + // A concurrent acquirer publishes its owner file with the directory, so this cannot remove it. + removeEmptyLock(); + if (Date.now() >= deadline) throw Error('Device tool maintenance is locked. Retry when the other operation finishes.'); + await new Promise(resolve => setTimeout(resolve, 50)); + } + } + try { return operation(); } + finally { + maintenanceFs.unlinkSync(maintenancePath.join(lock, ownerFile)); + removeEmptyLock(); + } + } finally { + maintenanceFs.rmSync(candidate, { recursive: true, force: true }); + } +} +function pruneTools(root, specs, flat) { + return withToolMaintenance(root, () => { + // Keep installs used by any running helper, including older T3 releases. + const scan = process.platform === 'win32' + ? require('node:child_process').spawnSync('powershell.exe', ['-NoProfile', '-NonInteractive', '-Command', 'Get-CimInstance Win32_Process | Select-Object -ExpandProperty CommandLine'], { encoding: 'utf8', timeout: 10000 }) + : require('node:child_process').spawnSync('ps', ['-ax', '-o', 'command='], { encoding: 'utf8', timeout: 10000 }); + if (scan.status !== 0 || !scan.stdout) return; + for (const [name, required] of specs) { + const parent = flat ? root : maintenancePath.join(root, name); + let names; + try { names = maintenanceFs.readdirSync(parent); } catch { continue; } + const completed = []; + for (const item of names) { + const version = flat ? (item.startsWith(name + '@') ? item.slice(name.length + 1) : '') : item; + if (!/^[0-9]+\.[0-9]+\.[0-9]+(?:-[a-zA-Z0-9.-]+)?$/.test(version)) continue; + const directory = maintenancePath.join(parent, item); + try { + if (!maintenanceFs.lstatSync(directory).isDirectory()) continue; + if (maintenanceFs.readFileSync(maintenancePath.join(directory, '.install-complete'), 'utf8').trim() !== version) continue; + completed.push({ version, directory, modified: maintenanceFs.statSync(maintenancePath.join(directory, '.install-complete')).mtimeMs }); + } catch {} + } + // Never prune until the required install has completed. Retain the last other successful install. + if (!completed.some(value => value.version === required)) continue; + const previous = completed.filter(value => value.version !== required).sort((a, b) => b.modified - a.modified || b.version.localeCompare(a.version, 'en', { numeric: true }))[0]?.version; + for (const { version, directory } of completed) { + if (version === required || version === previous || scan.stdout.includes(directory + maintenancePath.sep)) continue; + maintenanceFs.rmSync(directory, { recursive: true, force: true }); + } + } + }); +} +`; + +class DeviceToolMaintenanceError extends Schema.TaggedError()( + "DeviceToolMaintenanceError", + { + operation: Schema.Literal("prune"), + tool: Schema.Literals(["hub", "agent"]), + exitCode: Schema.NullOr(Schema.Int), + cause: Schema.Defect(), + }, +) { + override get message() { + return `Device tool ${this.operation} failed for ${this.tool} (exit code ${this.exitCode ?? "unknown"}).`; + } +} + +const runMaintenance = Effect.fn("DeviceToolchain.maintenance")(function* ( + nodePath: string, + script: string, + operation: "prune", + tool: "hub" | "agent", +) { + const runner = yield* ProcessRunner.ProcessRunner; + const result = yield* runner.run({ + command: nodePath, + args: [ + "-e", + deviceToolMaintenanceScript + + "\n" + + script + + ".catch(error => { console.error(error.message); process.exitCode = 1; });", + ], + }); + if (result.code !== 0) + return yield* Effect.fail( + new DeviceToolMaintenanceError({ operation, tool, exitCode: result.code, cause: result }), + ); +}); + +export const pruneLocalDeviceTools = Effect.fn("DeviceToolchain.prune")(function* ( + baseDir: string, + nodePath: string, + tool: "hub" | "agent", +) { + const path = yield* Path.Path; + yield* runMaintenance( + nodePath, + `pruneTools(${JSON.stringify(path.join(baseDir, "tools"))}, ${JSON.stringify(tool === "hub" ? [["expo-device-hub", DEVICE_HUB_VERSION]] : [["agent-device", AGENT_DEVICE_VERSION]])}, false)`, + "prune", + tool, + ); +}); diff --git a/apps/server/src/device/sshDeviceScript.test.ts b/apps/server/src/device/sshDeviceScript.test.ts index 89021d8c0474fc5ff97e68905443b386e1742c23..8d9bc77d891cccd352f40566ce21e062649b2db0 100644 --- a/apps/server/src/device/sshDeviceScript.test.ts +++ b/apps/server/src/device/sshDeviceScript.test.ts @@ -125,6 +125,9 @@ else { const child=spawn(process.execPath,[path.join(path.dirname(process.argv[1 ); await NodeFSP.mkdir(NodePath.join(root, "hosts/one"), { recursive: true }); await NodeFSP.writeFile(NodePath.join(root, "hosts/one/fail-start-once"), ""); + // Unavailable advisory bookkeeping must not prevent either helper from starting. + await NodeFSP.writeFile(NodePath.join(root, "tools/.maintenance-lock"), "blocked"); + await NodeFSP.writeFile(NodePath.join(root, "tools/.users"), "unwritable lease directory"); try { const [manual, concurrent] = await Promise.all([ invoke("one", "start"), diff --git a/apps/server/src/device/sshDeviceScript.ts b/apps/server/src/device/sshDeviceScript.ts index 4c156870a4f0fb5e3efe7833400a6b8f56a1ea6e..de412b5873d5fcf8da92311a7c384a918d176674 100644 --- a/apps/server/src/device/sshDeviceScript.ts +++ b/apps/server/src/device/sshDeviceScript.ts @@ -1,3 +1,4 @@ +import { deviceToolMaintenanceScript } from "./deviceToolMaintenance.ts"; import { AGENT_DEVICE_VERSION, DEVICE_HUB_VERSION } from "./DeviceToolchain.ts"; export const quoteRemoteArg = (value: string) => `'${value.replaceAll("'", "'\"'\"'")}'`; @@ -28,6 +29,7 @@ const mode = ${JSON.stringify(mode)}; const hubVersion = ${JSON.stringify(DEVICE_HUB_VERSION)}; const agentVersion = ${JSON.stringify(AGENT_DEVICE_VERSION)}; ` + + deviceToolMaintenanceScript + String.raw` const fs = require('node:fs'); const path = require('node:path'); @@ -214,6 +216,7 @@ async function install(name, version, entry) { } const vendor = path.resolve(path.dirname(hubEntry), '../../vendor/serve-sim/dist'); const optional = file => fs.existsSync(file) ? file : null; + await pruneTools(path.join(root, 'tools'), [['expo-device-hub', hubVersion], ...(mode === 'agent-start' ? [['agent-device', agentVersion]] : [])], true).catch(() => {}); console.log(JSON.stringify({ nodePath: process.execPath, platforms, tools: versions(), hubPort: hub.port, ...agentResult, helpers: { serveSimAxSettings: optional(path.join(vendor, 'simax/serve-sim-ax-settings')), serveSimCli: optional(path.join(vendor, 'serve-sim.js')) } })); } finally { releaseHost(); } -- 2.54.0