From c36d75996ec445e49a7139ca5ccb07770065cfd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 17 Jul 2026 12:54:40 +0200 Subject: [PATCH 1/6] feat: add daemon stop lifecycle --- src/cli-schema/command-overrides.ts | 10 ++ src/cli.ts | 6 +- src/cli/commands/daemon.ts | 41 ++++++ src/cli/commands/router.ts | 2 + .../flag-definitions-connection.ts | 7 + src/contracts/cli-flags.ts | 1 + src/core/command-descriptor/registry.ts | 9 ++ .../__tests__/provider-lease-expiry.test.ts | 28 ++++ src/daemon/daemon-shutdown-report.ts | 74 ++++++++++ src/daemon/daemon-stop.ts | 131 ++++++++++++++++++ src/daemon/provider-lease-expiry.ts | 14 ++ src/daemon/server/daemon-runtime.ts | 65 ++++++++- website/docs/docs/commands.md | 1 + 13 files changed, 385 insertions(+), 4 deletions(-) create mode 100644 src/cli/commands/daemon.ts create mode 100644 src/daemon/daemon-shutdown-report.ts create mode 100644 src/daemon/daemon-stop.ts diff --git a/src/cli-schema/command-overrides.ts b/src/cli-schema/command-overrides.ts index 8b4dc6b47..ec4e3e3b7 100644 --- a/src/cli-schema/command-overrides.ts +++ b/src/cli-schema/command-overrides.ts @@ -29,6 +29,16 @@ const SCHEMA_ONLY_CLI_COMMAND_SCHEMAS = { positionalArgs: ['status|login|logout'], supportedFlags: ['remoteConfig', 'stateDir'], }, + daemon: { + usageOverride: 'daemon stop [--state-dir ] [--clean]', + listUsageOverride: 'daemon stop', + helpDescription: + 'Stop a local daemon after verifying its PID/start-time identity. Use --clean to remove retained Apple runner processes and leases owned by that daemon.', + summary: 'Safely stop a local daemon and optionally clean retained runner resources', + positionalArgs: ['stop'], + allowedFlags: ['clean'], + supportedFlags: ['stateDir'], + }, connect: { usageOverride: 'connect [cloud|proxy|limrun|browserstack|aws-device-farm] [--remote-config ] [--daemon-base-url ] [--tenant ] [--run-id ] [--lease-id ] [--lease-backend ] [--force] [--no-login]', diff --git a/src/cli.ts b/src/cli.ts index c9acd341b..82cb2dee9 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -75,6 +75,7 @@ const REMOTE_MATERIALIZATION_DEFERRED_COMMANDS = new Set([ 'connect', 'connection', 'close', + 'daemon', 'disconnect', 'metro', 'proxy', @@ -554,6 +555,7 @@ function resolveActiveConnectionDefaults(options: { if ( options.command === 'connect' || options.command === 'connection' || + options.command === 'daemon' || options.command === 'proxy' ) { return null; @@ -577,7 +579,9 @@ function shouldMaterializeRemoteConnection(command: string): boolean { } function shouldResolveRemoteAuth(command: string): boolean { - return command !== 'auth' && command !== 'connection' && command !== 'proxy'; + return ( + command !== 'auth' && command !== 'connection' && command !== 'daemon' && command !== 'proxy' + ); } function shouldWarnOpenMayMissRemoteRuntime(options: { diff --git a/src/cli/commands/daemon.ts b/src/cli/commands/daemon.ts new file mode 100644 index 000000000..04aec7636 --- /dev/null +++ b/src/cli/commands/daemon.ts @@ -0,0 +1,41 @@ +import { resolveDaemonPaths } from '../../daemon/config.ts'; +import { readDaemonStopIdentity, stopDaemon } from '../../daemon/daemon-stop.ts'; +import { readDaemonShutdownReport } from '../../daemon/daemon-shutdown-report.ts'; +import { AppError } from '../../kernel/errors.ts'; +import { cleanupRunnerLeasesForOwner } from '../../platforms/apple/core/runner/runner-lease.ts'; +import { runnerLeaseCleanupAdapter } from '../../platforms/apple/core/runner/runner-disposal.ts'; +import { writeCommandOutput } from './shared.ts'; +import type { ClientCommandHandler } from './router-types.ts'; + +export const daemonCommand: ClientCommandHandler = async ({ positionals, flags }) => { + const subcommand = positionals[0]; + if (subcommand !== 'stop' || positionals.length !== 1) { + throw new AppError('INVALID_ARGS', 'daemon accepts only: stop'); + } + const paths = resolveDaemonPaths(flags.stateDir); + const identity = readDaemonStopIdentity(paths.infoPath); + const stopped = await stopDaemon({ paths }); + const report = stopped.mode === 'graceful' ? readDaemonShutdownReport(paths.baseDir) : null; + const result = report + ? { ...stopped, providerReleases: { status: 'completed' as const, ...report.providerReleases } } + : stopped; + if (flags.clean === true && identity !== null && result.stopped) { + await cleanupRunnerLeasesForOwner(identity, runnerLeaseCleanupAdapter); + } + const cleaned = flags.clean === true && identity !== null && result.stopped; + const data = { ...result, clean: cleaned }; + writeCommandOutput(flags, data, () => renderDaemonStop(data)); + return true; +}; + +function renderDaemonStop(result: { + stopped: boolean; + mode: string; + clean: boolean; + warnings: readonly string[]; +}): string { + const headline = result.stopped ? `Daemon stopped (${result.mode}).` : 'No running daemon found.'; + return [headline, result.clean ? 'Retained runner cleanup completed.' : null, ...result.warnings] + .filter((line): line is string => Boolean(line)) + .join('\n'); +} diff --git a/src/cli/commands/router.ts b/src/cli/commands/router.ts index f47cf15fd..cefbd4aa3 100644 --- a/src/cli/commands/router.ts +++ b/src/cli/commands/router.ts @@ -3,6 +3,7 @@ import type { AgentDeviceClient } from '../../agent-device-client.ts'; import { isClientBackedCliCommandName, type ClientBackedCliCommandName } from './client-backed.ts'; import { connectCommand, connectionCommand, disconnectCommand } from './connection.ts'; import { authCommand } from './auth.ts'; +import { daemonCommand } from './daemon.ts'; import { proxyCommand } from './proxy.ts'; import { replayCommand } from './replay.ts'; import { screenshotCommand, diffCommand } from './screenshot.ts'; @@ -19,6 +20,7 @@ const dedicatedCliCommandHandlers = { disconnect: disconnectCommand, connection: connectionCommand, auth: authCommand, + daemon: daemonCommand, proxy: proxyCommand, replay: replayCommand, screenshot: screenshotCommand, diff --git a/src/commands/cli-grammar/flag-definitions-connection.ts b/src/commands/cli-grammar/flag-definitions-connection.ts index 612a93d19..e2976da40 100644 --- a/src/commands/cli-grammar/flag-definitions-connection.ts +++ b/src/commands/cli-grammar/flag-definitions-connection.ts @@ -201,6 +201,13 @@ export const CONNECTION_FLAG_DEFINITIONS: readonly FlagDefinition[] = [ usageDescription: 'Force replacement of existing state (connect: replace an active connection; --save-script: overwrite an existing target instead of refusing)', }, + { + key: 'clean', + names: ['--clean'], + type: 'boolean', + usageLabel: '--clean', + usageDescription: 'Daemon stop: remove retained runner processes and leases after stopping', + }, { key: 'noLogin', names: ['--no-login'], diff --git a/src/contracts/cli-flags.ts b/src/contracts/cli-flags.ts index 445b9033c..330a8494b 100644 --- a/src/contracts/cli-flags.ts +++ b/src/contracts/cli-flags.ts @@ -42,6 +42,7 @@ export type CliFlags = CloudProviderProfileFields & provider?: string; providerSessionId?: string; force?: boolean; + clean?: boolean; noLogin?: boolean; kind?: string; perfTemplate?: string; diff --git a/src/core/command-descriptor/registry.ts b/src/core/command-descriptor/registry.ts index a079f32cd..e45b44168 100644 --- a/src/core/command-descriptor/registry.ts +++ b/src/core/command-descriptor/registry.ts @@ -1075,6 +1075,15 @@ export const RAW_COMMAND_DESCRIPTORS = [ timeoutPolicy: DEFAULT_TIMEOUT_POLICY, batchable: false, }, + { + name: 'daemon', + ...(ownerFilesEnabled ? { ownerFiles: ['src/cli/commands/daemon.ts'] as const } : {}), + catalog: { group: 'local-cli' }, + recordsSessionAction: false, + timeoutPolicy: DEFAULT_TIMEOUT_POLICY, + batchable: false, + mcpExposed: false, + }, { name: 'metro', ...(ownerFilesEnabled ? { ownerFiles: ['src/commands/metro/index.ts'] as const } : {}), diff --git a/src/daemon/__tests__/provider-lease-expiry.test.ts b/src/daemon/__tests__/provider-lease-expiry.test.ts index 87b823594..892c20384 100644 --- a/src/daemon/__tests__/provider-lease-expiry.test.ts +++ b/src/daemon/__tests__/provider-lease-expiry.test.ts @@ -120,3 +120,31 @@ test('does not release until the expired lease record is durable', async () => { fs.rmSync(stateDir, { recursive: true, force: true }); } }); + +test('final drain reports releases completed during daemon shutdown', async () => { + const lease = new LeaseRegistry().allocateLease({ + tenantId: 'tenant-a', + runId: 'run-1', + leaseProvider: 'browserstack', + }); + const release = vi + .fn() + .mockRejectedValueOnce(new Error('temporary outage')) + .mockResolvedValue({}); + const releaser = createExpiredProviderLeaseReleaser({ + leaseLifecycleProvider: { release }, + providerRuntimeIds: ['browserstack'], + }); + + try { + await releaser.release(lease); + const drained = await releaser.drain(10); + + expect(release).toHaveBeenCalledTimes(2); + expect(release).toHaveBeenLastCalledWith(lease); + expect(drained.released).toEqual([lease]); + expect(drained.pending).toEqual([]); + } finally { + releaser.shutdown(); + } +}); diff --git a/src/daemon/daemon-shutdown-report.ts b/src/daemon/daemon-shutdown-report.ts new file mode 100644 index 000000000..4234501bd --- /dev/null +++ b/src/daemon/daemon-shutdown-report.ts @@ -0,0 +1,74 @@ +import fs from 'node:fs'; +import path from 'node:path'; +import type { DeviceLease } from './lease-registry.ts'; + +const SHUTDOWN_REPORT_FILE = 'daemon-shutdown.json'; + +export type ProviderReleaseRecord = { + leaseId: string; + provider?: string; +}; + +export type DaemonShutdownReport = { + providerReleases: { + released: ProviderReleaseRecord[]; + pending: ProviderReleaseRecord[]; + }; +}; + +export function writeDaemonShutdownReport( + stateDir: string, + providerReleases: { released: readonly DeviceLease[]; pending: readonly DeviceLease[] }, +): void { + const report: DaemonShutdownReport = { + providerReleases: { + released: providerReleases.released.map(toProviderReleaseRecord), + pending: providerReleases.pending.map(toProviderReleaseRecord), + }, + }; + const filePath = shutdownReportPath(stateDir); + const temporaryPath = `${filePath}.${process.pid}.${Date.now()}.tmp`; + try { + fs.writeFileSync(temporaryPath, `${JSON.stringify(report)}\n`, { mode: 0o600 }); + fs.renameSync(temporaryPath, filePath); + fs.chmodSync(filePath, 0o600); + } catch { + try { + fs.rmSync(temporaryPath, { force: true }); + } catch {} + } +} + +export function readDaemonShutdownReport(stateDir: string): DaemonShutdownReport | null { + try { + const parsed = JSON.parse(fs.readFileSync(shutdownReportPath(stateDir), 'utf8')) as unknown; + return isDaemonShutdownReport(parsed) ? parsed : null; + } catch { + return null; + } +} + +export function clearDaemonShutdownReport(stateDir: string): void { + try { + fs.rmSync(shutdownReportPath(stateDir), { force: true }); + } catch {} +} + +function shutdownReportPath(stateDir: string): string { + return path.join(stateDir, SHUTDOWN_REPORT_FILE); +} + +function toProviderReleaseRecord(lease: DeviceLease): ProviderReleaseRecord { + return { + leaseId: lease.leaseId, + ...(lease.leaseProvider ? { provider: lease.leaseProvider } : {}), + }; +} + +function isDaemonShutdownReport(value: unknown): value is DaemonShutdownReport { + if (!value || typeof value !== 'object') return false; + const releases = (value as { providerReleases?: unknown }).providerReleases; + if (!releases || typeof releases !== 'object') return false; + const records = releases as { released?: unknown; pending?: unknown }; + return Array.isArray(records.released) && Array.isArray(records.pending); +} diff --git a/src/daemon/daemon-stop.ts b/src/daemon/daemon-stop.ts new file mode 100644 index 000000000..0f71541c3 --- /dev/null +++ b/src/daemon/daemon-stop.ts @@ -0,0 +1,131 @@ +import fs from 'node:fs'; +import { AppError } from '../kernel/errors.ts'; +import { isAgentDeviceDaemonProcess } from './daemon-process.ts'; +import { isProcessAlive, waitForProcessExit } from '../utils/host-process.ts'; +import { sleep } from '../utils/timeouts.ts'; +import type { DaemonPaths } from './config.ts'; +import type { ProviderReleaseRecord } from './daemon-shutdown-report.ts'; + +const DAEMON_STOP_GRACE_TIMEOUT_MS = 10_000; +const DAEMON_STOP_KILL_TIMEOUT_MS = 2_000; +const DAEMON_STOP_METADATA_WAIT_MS = 1_000; + +type DaemonInfo = { + pid?: unknown; + processStartTime?: unknown; +}; + +export type DaemonStopResult = { + stopped: boolean; + mode: 'graceful' | 'forced' | 'not-running'; + cleanupConfidence: 'known' | 'unknown'; + claimsReleased: []; + claimsOrphaned: []; + providerReleases: { + status: 'completed' | 'unknown'; + released: ProviderReleaseRecord[]; + pending: ProviderReleaseRecord[] | null; + }; + warnings: string[]; +}; + +export async function stopDaemon(params: { + paths: DaemonPaths; + graceTimeoutMs?: number; + killTimeoutMs?: number; +}): Promise { + const info = readDaemonInfo(params.paths.infoPath); + if (!info) return notRunningResult(); + if (!isAgentDeviceDaemonProcess(info.pid, info.processStartTime)) { + if (!isProcessAlive(info.pid)) return notRunningResult(); + throw new AppError( + 'COMMAND_FAILED', + 'Refusing to stop a daemon whose PID or start-time identity could not be verified.', + { pid: info.pid, processStartTime: info.processStartTime }, + ); + } + + process.kill(info.pid, 'SIGTERM'); + const graceful = await waitForProcessExit( + info.pid, + params.graceTimeoutMs ?? DAEMON_STOP_GRACE_TIMEOUT_MS, + ); + if (graceful) { + await waitForDaemonMetadataRemoval(params.paths, DAEMON_STOP_METADATA_WAIT_MS); + return { + stopped: true, + mode: 'graceful', + cleanupConfidence: 'known', + claimsReleased: [], + claimsOrphaned: [], + providerReleases: { status: 'completed', released: [], pending: [] }, + warnings: [], + }; + } + + // Re-verify immediately before escalation so a PID cannot be reused between + // the graceful wait and SIGKILL. + if (isAgentDeviceDaemonProcess(info.pid, info.processStartTime)) { + process.kill(info.pid, 'SIGKILL'); + } + const stopped = await waitForProcessExit( + info.pid, + params.killTimeoutMs ?? DAEMON_STOP_KILL_TIMEOUT_MS, + ); + if (!stopped) { + throw new AppError('COMMAND_FAILED', 'Daemon did not exit after SIGKILL.', { pid: info.pid }); + } + return { + stopped: true, + mode: 'forced', + cleanupConfidence: 'unknown', + claimsReleased: [], + claimsOrphaned: [], + providerReleases: { status: 'unknown', released: [], pending: null }, + warnings: [ + 'The daemon was force-killed before provider lease state could be finalized. Provider allocations may remain active.', + ], + }; +} + +export function readDaemonStopIdentity( + infoPath: string, +): { pid: number; processStartTime?: string } | null { + return readDaemonInfo(infoPath); +} + +function readDaemonInfo(infoPath: string): { pid: number; processStartTime?: string } | null { + try { + const parsed = JSON.parse(fs.readFileSync(infoPath, 'utf8')) as DaemonInfo; + const pid = parsed.pid; + if (typeof pid !== 'number' || !Number.isInteger(pid) || pid <= 0) return null; + return { + pid, + ...(typeof parsed.processStartTime === 'string' + ? { processStartTime: parsed.processStartTime } + : {}), + }; + } catch { + return null; + } +} + +async function waitForDaemonMetadataRemoval(paths: DaemonPaths, timeoutMs: number): Promise { + const startedAt = Date.now(); + while (Date.now() - startedAt < timeoutMs) { + if (!fs.existsSync(paths.infoPath) && !fs.existsSync(paths.lockPath)) return; + await sleep(25); + } +} + +function notRunningResult(): DaemonStopResult { + return { + stopped: false, + mode: 'not-running', + cleanupConfidence: 'known', + claimsReleased: [], + claimsOrphaned: [], + providerReleases: { status: 'completed', released: [], pending: [] }, + warnings: [], + }; +} diff --git a/src/daemon/provider-lease-expiry.ts b/src/daemon/provider-lease-expiry.ts index e271ac950..64a4b7142 100644 --- a/src/daemon/provider-lease-expiry.ts +++ b/src/daemon/provider-lease-expiry.ts @@ -5,6 +5,7 @@ import type { LeaseLifecycleProvider } from './handlers/lease.ts'; import { releaseExpiredProviderLease } from './lease-lifecycle.ts'; import type { DeviceLease } from './lease-registry.ts'; import { emitDiagnostic } from '../utils/diagnostics.ts'; +import { sleep } from '../utils/timeouts.ts'; const DEFAULT_RETRY_DELAY_MS = 1_000; const PENDING_RELEASES_FILE = 'expired-provider-leases.json'; @@ -28,6 +29,7 @@ type PersistedExpiredProviderLeases = { export type ExpiredProviderLeaseReleaser = { release: (lease: DeviceLease) => Promise; retryPending: () => Promise; + drain: (timeoutMs: number) => Promise<{ pending: DeviceLease[]; released: DeviceLease[] }>; shutdown: () => void; }; @@ -45,6 +47,7 @@ export function createExpiredProviderLeaseReleaser(options: { const retryDelayMs = options.retryDelayMs ?? DEFAULT_RETRY_DELAY_MS; let retryTimer: Timer | undefined; let retrying = false; + const releasedLeases = new Map(); const persistPendingLeases = (): boolean => { if (!options.stateDir) { @@ -109,6 +112,7 @@ export function createExpiredProviderLeaseReleaser(options: { if (!isProviderRuntimeAvailable(lease, options.providerRuntimeIds)) continue; if (await releaseLiveProviderLease(options.leaseLifecycleProvider, lease)) { pendingLiveLeases.delete(lease.leaseId); + releasedLeases.set(lease.leaseId, lease); } } }; @@ -119,6 +123,7 @@ export function createExpiredProviderLeaseReleaser(options: { if (!persistedRecoveryLeaseIds.has(lease.leaseId) && !persistPendingLeases()) continue; if (await releaseExpiredProviderLease(options.recoverExpiredLease, lease)) { pendingRecoveryLeases.delete(lease.leaseId); + releasedLeases.set(lease.leaseId, lease); persistPendingLeases(); } } @@ -162,6 +167,15 @@ export function createExpiredProviderLeaseReleaser(options: { } }, retryPending, + drain: async (timeoutMs) => { + // Shutdown gets one final, bounded attempt at every pending release. A + // provider call that hangs must not keep the daemon alive indefinitely. + await Promise.race([retryPending(), sleep(Math.max(0, timeoutMs))]); + return { + pending: [...pendingLiveLeases.values(), ...pendingRecoveryLeases.values()], + released: [...releasedLeases.values()], + }; + }, shutdown: () => { if (retryTimer) clearTimeout(retryTimer); retryTimer = undefined; diff --git a/src/daemon/server/daemon-runtime.ts b/src/daemon/server/daemon-runtime.ts index 601747c1f..e7ed0477f 100644 --- a/src/daemon/server/daemon-runtime.ts +++ b/src/daemon/server/daemon-runtime.ts @@ -16,6 +16,8 @@ import { } from '../../provider-device-runtimes.ts'; import { LeaseRegistry } from '../lease-registry.ts'; import { createExpiredProviderLeaseReleaser } from '../provider-lease-expiry.ts'; +import { leaseScopeToReleaseRequest } from '../../core/lease-scope.ts'; +import { clearDaemonShutdownReport, writeDaemonShutdownReport } from '../daemon-shutdown-report.ts'; import { createRequestHandler } from '../request-router.ts'; import { teardownSessionResources } from '../session-teardown.ts'; import { IOS_SIMULATOR_RECORDING_STOP_ESCALATION_BUDGET_MS } from '../handlers/record-trace-ios-simulator.ts'; @@ -57,6 +59,7 @@ import { const DAEMON_SESSION_TEARDOWN_TIMEOUT_MS = 5_000; const DAEMON_PNG_WORKER_TERMINATE_TIMEOUT_MS = 1_000; +const DAEMON_PROVIDER_RELEASE_DRAIN_TIMEOUT_MS = 2_000; type WritableOutput = { write: (chunk: string) => unknown; @@ -90,8 +93,9 @@ export async function teardownDaemonSessionForShutdown(params: { sessionStore: SessionStore; stateDir?: string; stderr: WritableOutput; + beforeDelete?: (session: SessionState) => Promise; }): Promise { - const { session, sessionStore, stateDir, stderr } = params; + const { session, sessionStore, stateDir, stderr, beforeDelete } = params; const timeoutMs = resolveDaemonSessionTeardownTimeoutMs(session); const teardown = teardownSessionResources(session, session.name, stateDir).catch((error) => { stderr.write( @@ -110,6 +114,7 @@ export async function teardownDaemonSessionForShutdown(params: { // `.ad` iff the repair transaction completed, else leave a bounded // `REPAIR_SESSION_EXPIRED` tombstone for the reaped-before-finalize case. sessionStore.finalizeRepairTeardown(session); + await beforeDelete?.(session); sessionStore.delete(session.name); } @@ -206,8 +211,49 @@ export async function startDaemonRuntime( ); }; + const finalizeDaemonSessionLease = async (session: SessionState): Promise => { + if (!session.lease) return; + const releaseRequest = leaseScopeToReleaseRequest({ + leaseId: session.lease.leaseId, + tenantId: session.lease.tenantId, + runId: session.lease.runId, + leaseBackend: session.lease.leaseBackend, + leaseProvider: session.lease.leaseProvider, + deviceKey: session.lease.deviceKey, + clientId: session.lease.clientId, + }); + const activeLease = leaseRegistry.getLease(releaseRequest); + if (!activeLease) return; + try { + await expiredProviderLeaseReleaser.release(activeLease); + leaseRegistry.releaseLease(releaseRequest); + } catch (error) { + // The session is about to be removed. Queue the provider allocation while + // this daemon can still persist retry state, then release only the local + // registry entry. This mirrors expiry recovery without pretending the + // external provider release succeeded. + if (activeLease) await expiredProviderLeaseReleaser.release(activeLease); + leaseRegistry.releaseLease(releaseRequest); + emitDiagnostic({ + level: 'warn', + phase: 'daemon_shutdown_session_lease_release_failed', + data: { + session: session.name, + leaseId: session.lease.leaseId, + error: error instanceof Error ? error.message : String(error), + }, + }); + } + }; + const teardownDaemonSession = async (session: SessionState): Promise => - await teardownDaemonSessionForShutdown({ session, sessionStore, stateDir: baseDir, stderr }); + await teardownDaemonSessionForShutdown({ + session, + sessionStore, + stateDir: baseDir, + stderr, + beforeDelete: finalizeDaemonSessionLease, + }); const teardownDaemonSessions = async (): Promise => { const sessionsToStop = sessionStore.toArray(); @@ -300,6 +346,7 @@ export async function startDaemonRuntime( exit(0); return null; } + clearDaemonShutdownReport(baseDir); let servers: DaemonServer[] = []; let socketPort: number | undefined; @@ -341,7 +388,6 @@ export async function startDaemonRuntime( idleReap.cancel(); if (shuttingDown) return; shuttingDown = true; - expiredProviderLeaseReleaser.shutdown(); if (shutdownOptions.cause) { await emitFatalDiagnostic(shutdownOptions.cause); } @@ -355,6 +401,19 @@ export async function startDaemonRuntime( await detachIosSimulatorRunnerSessionsForShutdown(); } catch {} await teardownDaemonSessions(); + const providerReleaseDrain = await expiredProviderLeaseReleaser.drain( + DAEMON_PROVIDER_RELEASE_DRAIN_TIMEOUT_MS, + ); + writeDaemonShutdownReport(baseDir, providerReleaseDrain); + emitDiagnostic({ + level: providerReleaseDrain.pending.length === 0 ? 'info' : 'warn', + phase: 'daemon_shutdown_provider_release_drain', + data: { + releasedLeaseIds: providerReleaseDrain.released.map((lease) => lease.leaseId), + pendingLeaseIds: providerReleaseDrain.pending.map((lease) => lease.leaseId), + }, + }); + expiredProviderLeaseReleaser.shutdown(); await Promise.allSettled( providerDeviceRuntimes.map(async (runtime) => await runtime.shutdown()), ); diff --git a/website/docs/docs/commands.md b/website/docs/docs/commands.md index d0b9b62dd..e05fe5223 100644 --- a/website/docs/docs/commands.md +++ b/website/docs/docs/commands.md @@ -58,6 +58,7 @@ agent-device app-switcher - `boot` requires either an active session or an explicit device selector. - `shutdown` turns off the selected Apple simulator or Android emulator. - `shutdown` must not target an active session device; use `close --shutdown` to end the session and turn it off. +- `daemon stop --state-dir ` verifies the daemon PID/start-time identity, requests graceful shutdown, and reports whether provider-release state is known. Use `daemon stop --clean` to also remove retained Apple runner processes and leases owned by that daemon. - `--platform apple` is an alias for the Apple automation backend (`ios`, `tvOS`, `macOS` selection). - Use `--target mobile|tv|desktop` with `--platform` (required) to select phone/tablet vs TV-class vs desktop-class targets. - `boot` is mainly needed when starting a new session and `open` fails because no booted simulator/emulator is available. From 39379acc9be66bca75bb332536f13e271f81a07a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 17 Jul 2026 13:10:39 +0200 Subject: [PATCH 2/6] fix: harden daemon stop cleanup --- src/cli/commands/__tests__/daemon.test.ts | 105 +++++++++++++ src/cli/commands/daemon.ts | 34 +++-- .../__tests__/daemon-shutdown-report.test.ts | 47 ++++++ src/daemon/__tests__/daemon-stop.test.ts | 144 ++++++++++++++++++ src/daemon/daemon-process.ts | 2 +- src/daemon/daemon-stop.ts | 15 +- src/daemon/server/daemon-runtime.ts | 28 ++-- 7 files changed, 341 insertions(+), 34 deletions(-) create mode 100644 src/cli/commands/__tests__/daemon.test.ts create mode 100644 src/daemon/__tests__/daemon-shutdown-report.test.ts create mode 100644 src/daemon/__tests__/daemon-stop.test.ts diff --git a/src/cli/commands/__tests__/daemon.test.ts b/src/cli/commands/__tests__/daemon.test.ts new file mode 100644 index 000000000..1d673ce48 --- /dev/null +++ b/src/cli/commands/__tests__/daemon.test.ts @@ -0,0 +1,105 @@ +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { afterEach, expect, test, vi } from 'vitest'; +import type { DaemonStopResult } from '../../../daemon/daemon-stop.ts'; + +const mocks = vi.hoisted(() => ({ + cleanupRunnerLeasesForOwner: vi.fn(async () => undefined), + readDaemonShutdownReport: vi.fn(), + readDaemonStopIdentity: vi.fn(), + stopDaemon: vi.fn(), + writeCommandOutput: vi.fn(), +})); + +vi.mock('../../../daemon/daemon-stop.ts', () => ({ + readDaemonStopIdentity: mocks.readDaemonStopIdentity, + stopDaemon: mocks.stopDaemon, +})); +vi.mock('../../../daemon/daemon-shutdown-report.ts', () => ({ + readDaemonShutdownReport: mocks.readDaemonShutdownReport, +})); +vi.mock('../../../platforms/apple/core/runner/runner-lease.ts', () => ({ + cleanupRunnerLeasesForOwner: mocks.cleanupRunnerLeasesForOwner, +})); +vi.mock('../../../platforms/apple/core/runner/runner-disposal.ts', () => ({ + runnerLeaseCleanupAdapter: {}, +})); +vi.mock('../shared.ts', () => ({ writeCommandOutput: mocks.writeCommandOutput })); + +import { daemonCommand } from '../daemon.ts'; + +const GRACEFUL_RESULT: DaemonStopResult = { + stopped: true, + mode: 'graceful', + cleanupConfidence: 'known', + claimsReleased: [], + claimsOrphaned: [], + providerReleases: { status: 'completed', released: [], pending: [] }, + warnings: [], +}; + +afterEach(() => { + vi.clearAllMocks(); +}); + +test('accepts only daemon stop', async () => { + await assert.rejects( + async () => + await daemonCommand({ + positionals: [], + flags: { help: false, json: false, version: false }, + client: {} as never, + }), + (error: { code?: string }) => error.code === 'INVALID_ARGS', + ); + await assert.rejects( + async () => + await daemonCommand({ + positionals: ['stop', 'extra'], + flags: { help: false, json: false, version: false }, + client: {} as never, + }), + (error: { code?: string }) => error.code === 'INVALID_ARGS', + ); +}); + +test('merges a graceful shutdown report and cleans runner leases with the start-time identity', async () => { + const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-daemon-command-')); + mocks.readDaemonStopIdentity.mockReturnValue({ pid: 123, processStartTime: 'start-time' }); + mocks.stopDaemon.mockResolvedValue(GRACEFUL_RESULT); + mocks.readDaemonShutdownReport.mockReturnValue({ + providerReleases: { + released: [{ leaseId: 'lease-1', provider: 'limrun' }], + pending: [], + }, + }); + + try { + await daemonCommand({ + positionals: ['stop'], + flags: { clean: true, help: false, json: false, stateDir, version: false }, + client: {} as never, + }); + + expect(mocks.cleanupRunnerLeasesForOwner).toHaveBeenCalledWith( + { pid: 123, startTime: 'start-time' }, + {}, + ); + expect(mocks.writeCommandOutput).toHaveBeenCalledWith( + expect.objectContaining({ clean: true, json: false }), + expect.objectContaining({ + clean: true, + providerReleases: { + status: 'completed', + released: [{ leaseId: 'lease-1', provider: 'limrun' }], + pending: [], + }, + }), + expect.any(Function), + ); + } finally { + fs.rmSync(stateDir, { recursive: true, force: true }); + } +}); diff --git a/src/cli/commands/daemon.ts b/src/cli/commands/daemon.ts index 04aec7636..9908d271f 100644 --- a/src/cli/commands/daemon.ts +++ b/src/cli/commands/daemon.ts @@ -1,9 +1,11 @@ import { resolveDaemonPaths } from '../../daemon/config.ts'; -import { readDaemonStopIdentity, stopDaemon } from '../../daemon/daemon-stop.ts'; +import { + readDaemonStopIdentity, + stopDaemon, + type DaemonStopResult, +} from '../../daemon/daemon-stop.ts'; import { readDaemonShutdownReport } from '../../daemon/daemon-shutdown-report.ts'; import { AppError } from '../../kernel/errors.ts'; -import { cleanupRunnerLeasesForOwner } from '../../platforms/apple/core/runner/runner-lease.ts'; -import { runnerLeaseCleanupAdapter } from '../../platforms/apple/core/runner/runner-disposal.ts'; import { writeCommandOutput } from './shared.ts'; import type { ClientCommandHandler } from './router-types.ts'; @@ -19,21 +21,27 @@ export const daemonCommand: ClientCommandHandler = async ({ positionals, flags } const result = report ? { ...stopped, providerReleases: { status: 'completed' as const, ...report.providerReleases } } : stopped; - if (flags.clean === true && identity !== null && result.stopped) { - await cleanupRunnerLeasesForOwner(identity, runnerLeaseCleanupAdapter); + const shouldClean = flags.clean === true && identity !== null && result.stopped; + if (shouldClean) { + const [{ cleanupRunnerLeasesForOwner }, { runnerLeaseCleanupAdapter }] = await Promise.all([ + import('../../platforms/apple/core/runner/runner-lease.ts'), + import('../../platforms/apple/core/runner/runner-disposal.ts'), + ]); + await cleanupRunnerLeasesForOwner( + { pid: identity.pid, startTime: identity.processStartTime }, + runnerLeaseCleanupAdapter, + ); } - const cleaned = flags.clean === true && identity !== null && result.stopped; - const data = { ...result, clean: cleaned }; + const data = { ...result, clean: shouldClean }; writeCommandOutput(flags, data, () => renderDaemonStop(data)); return true; }; -function renderDaemonStop(result: { - stopped: boolean; - mode: string; - clean: boolean; - warnings: readonly string[]; -}): string { +function renderDaemonStop( + result: Pick & { + clean: boolean; + }, +): string { const headline = result.stopped ? `Daemon stopped (${result.mode}).` : 'No running daemon found.'; return [headline, result.clean ? 'Retained runner cleanup completed.' : null, ...result.warnings] .filter((line): line is string => Boolean(line)) diff --git a/src/daemon/__tests__/daemon-shutdown-report.test.ts b/src/daemon/__tests__/daemon-shutdown-report.test.ts new file mode 100644 index 000000000..3d206af0e --- /dev/null +++ b/src/daemon/__tests__/daemon-shutdown-report.test.ts @@ -0,0 +1,47 @@ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { expect, test } from 'vitest'; +import { + clearDaemonShutdownReport, + readDaemonShutdownReport, + writeDaemonShutdownReport, +} from '../daemon-shutdown-report.ts'; +import { LeaseRegistry } from '../lease-registry.ts'; + +test('round-trips provider release records without persisting lease credentials', () => { + const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-shutdown-report-')); + const lease = new LeaseRegistry().allocateLease({ + tenantId: 'tenant-a', + runId: 'run-1', + leaseProvider: 'limrun', + }); + + try { + writeDaemonShutdownReport(stateDir, { released: [lease], pending: [lease] }); + + expect(readDaemonShutdownReport(stateDir)).toEqual({ + providerReleases: { + released: [{ leaseId: lease.leaseId, provider: 'limrun' }], + pending: [{ leaseId: lease.leaseId, provider: 'limrun' }], + }, + }); + } finally { + fs.rmSync(stateDir, { recursive: true, force: true }); + } +}); + +test('ignores malformed shutdown reports and clear removes a prior report', () => { + const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-shutdown-report-')); + const reportPath = path.join(stateDir, 'daemon-shutdown.json'); + + try { + fs.writeFileSync(reportPath, JSON.stringify({ providerReleases: { released: [] } })); + expect(readDaemonShutdownReport(stateDir)).toBeNull(); + + clearDaemonShutdownReport(stateDir); + expect(fs.existsSync(reportPath)).toBe(false); + } finally { + fs.rmSync(stateDir, { recursive: true, force: true }); + } +}); diff --git a/src/daemon/__tests__/daemon-stop.test.ts b/src/daemon/__tests__/daemon-stop.test.ts new file mode 100644 index 000000000..e1631963c --- /dev/null +++ b/src/daemon/__tests__/daemon-stop.test.ts @@ -0,0 +1,144 @@ +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { afterEach, expect, test, vi } from 'vitest'; + +const mocks = vi.hoisted(() => ({ + isAgentDeviceDaemonProcess: vi.fn(), + isProcessAlive: vi.fn(), + sleep: vi.fn(async () => undefined), + trySignalProcess: vi.fn(), + waitForProcessExit: vi.fn(), +})); + +vi.mock('../daemon-process.ts', () => ({ + isAgentDeviceDaemonProcess: mocks.isAgentDeviceDaemonProcess, + trySignalProcess: mocks.trySignalProcess, +})); +vi.mock('../../utils/host-process.ts', () => ({ + isProcessAlive: mocks.isProcessAlive, + waitForProcessExit: mocks.waitForProcessExit, +})); +vi.mock('../../utils/timeouts.ts', () => ({ sleep: mocks.sleep })); + +import { resolveDaemonPaths } from '../config.ts'; +import { stopDaemon } from '../daemon-stop.ts'; + +afterEach(() => { + vi.clearAllMocks(); +}); + +function createDaemonPaths(): ReturnType { + const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-daemon-stop-')); + const paths = resolveDaemonPaths(stateDir); + fs.mkdirSync(paths.baseDir, { recursive: true }); + fs.writeFileSync(paths.infoPath, JSON.stringify({ pid: 123, processStartTime: 'start-time' })); + return paths; +} + +function removeDaemonPaths(paths: ReturnType): void { + fs.rmSync(paths.baseDir, { recursive: true, force: true }); +} + +test('reports not-running when daemon metadata is absent', async () => { + const paths = resolveDaemonPaths( + fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-daemon-stop-')), + ); + + try { + const result = await stopDaemon({ paths }); + expect(result).toMatchObject({ stopped: false, mode: 'not-running' }); + } finally { + removeDaemonPaths(paths); + } +}); + +test('refuses to signal a live process whose daemon identity cannot be verified', async () => { + const paths = createDaemonPaths(); + mocks.isAgentDeviceDaemonProcess.mockReturnValue(false); + mocks.isProcessAlive.mockReturnValue(true); + + try { + await assert.rejects( + async () => await stopDaemon({ paths }), + (error: { code?: string }) => error.code === 'COMMAND_FAILED', + ); + expect(mocks.trySignalProcess).not.toHaveBeenCalled(); + } finally { + removeDaemonPaths(paths); + } +}); + +test('treats an exited daemon between identity verification and SIGTERM as not-running', async () => { + const paths = createDaemonPaths(); + mocks.isAgentDeviceDaemonProcess.mockReturnValue(true); + mocks.trySignalProcess.mockReturnValue(false); + mocks.isProcessAlive.mockReturnValue(false); + + try { + const result = await stopDaemon({ paths }); + expect(result).toMatchObject({ stopped: false, mode: 'not-running' }); + } finally { + removeDaemonPaths(paths); + } +}); + +test('reports graceful cleanup after SIGTERM exits the verified daemon', async () => { + const paths = createDaemonPaths(); + mocks.isAgentDeviceDaemonProcess.mockReturnValue(true); + mocks.trySignalProcess.mockReturnValue(true); + mocks.waitForProcessExit.mockImplementation(async () => { + fs.rmSync(paths.infoPath, { force: true }); + return true; + }); + + try { + const result = await stopDaemon({ paths }); + expect(result).toMatchObject({ + stopped: true, + mode: 'graceful', + cleanupConfidence: 'known', + providerReleases: { pending: [] }, + }); + expect(mocks.trySignalProcess).toHaveBeenCalledWith(123, 'SIGTERM'); + } finally { + removeDaemonPaths(paths); + } +}); + +test('re-verifies identity before SIGKILL and reports forced cleanup as unknown', async () => { + const paths = createDaemonPaths(); + mocks.isAgentDeviceDaemonProcess.mockReturnValue(true); + mocks.trySignalProcess.mockReturnValue(true); + mocks.waitForProcessExit.mockResolvedValueOnce(false).mockResolvedValueOnce(true); + + try { + const result = await stopDaemon({ paths }); + expect(result).toMatchObject({ + stopped: true, + mode: 'forced', + cleanupConfidence: 'unknown', + providerReleases: { pending: null }, + }); + expect(mocks.trySignalProcess).toHaveBeenNthCalledWith(1, 123, 'SIGTERM'); + expect(mocks.trySignalProcess).toHaveBeenNthCalledWith(2, 123, 'SIGKILL'); + } finally { + removeDaemonPaths(paths); + } +}); + +test('does not send SIGKILL if the daemon identity changes during the graceful wait', async () => { + const paths = createDaemonPaths(); + mocks.isAgentDeviceDaemonProcess.mockReturnValueOnce(true).mockReturnValueOnce(false); + mocks.trySignalProcess.mockReturnValue(true); + mocks.waitForProcessExit.mockResolvedValueOnce(false).mockResolvedValueOnce(true); + + try { + await stopDaemon({ paths }); + expect(mocks.trySignalProcess).toHaveBeenCalledTimes(1); + expect(mocks.trySignalProcess).toHaveBeenCalledWith(123, 'SIGTERM'); + } finally { + removeDaemonPaths(paths); + } +}); diff --git a/src/daemon/daemon-process.ts b/src/daemon/daemon-process.ts index 5e130ce29..b81670875 100644 --- a/src/daemon/daemon-process.ts +++ b/src/daemon/daemon-process.ts @@ -28,7 +28,7 @@ export function isAgentDeviceDaemonProcess(pid: number, expectedStartTime?: stri return isAgentDeviceDaemonCommand(command); } -function trySignalProcess(pid: number, signal: NodeJS.Signals): boolean { +export function trySignalProcess(pid: number, signal: NodeJS.Signals): boolean { try { process.kill(pid, signal); return true; diff --git a/src/daemon/daemon-stop.ts b/src/daemon/daemon-stop.ts index 0f71541c3..5864ea0bd 100644 --- a/src/daemon/daemon-stop.ts +++ b/src/daemon/daemon-stop.ts @@ -1,6 +1,6 @@ import fs from 'node:fs'; import { AppError } from '../kernel/errors.ts'; -import { isAgentDeviceDaemonProcess } from './daemon-process.ts'; +import { isAgentDeviceDaemonProcess, trySignalProcess } from './daemon-process.ts'; import { isProcessAlive, waitForProcessExit } from '../utils/host-process.ts'; import { sleep } from '../utils/timeouts.ts'; import type { DaemonPaths } from './config.ts'; @@ -45,7 +45,7 @@ export async function stopDaemon(params: { ); } - process.kill(info.pid, 'SIGTERM'); + if (!signalDaemonProcess(info.pid, 'SIGTERM')) return notRunningResult(); const graceful = await waitForProcessExit( info.pid, params.graceTimeoutMs ?? DAEMON_STOP_GRACE_TIMEOUT_MS, @@ -66,7 +66,7 @@ export async function stopDaemon(params: { // Re-verify immediately before escalation so a PID cannot be reused between // the graceful wait and SIGKILL. if (isAgentDeviceDaemonProcess(info.pid, info.processStartTime)) { - process.kill(info.pid, 'SIGKILL'); + signalDaemonProcess(info.pid, 'SIGKILL'); } const stopped = await waitForProcessExit( info.pid, @@ -88,6 +88,15 @@ export async function stopDaemon(params: { }; } +function signalDaemonProcess(pid: number, signal: NodeJS.Signals): boolean { + if (trySignalProcess(pid, signal)) return true; + if (!isProcessAlive(pid)) return false; + throw new AppError('COMMAND_FAILED', `Daemon could not be signaled with ${signal}.`, { + pid, + signal, + }); +} + export function readDaemonStopIdentity( infoPath: string, ): { pid: number; processStartTime?: string } | null { diff --git a/src/daemon/server/daemon-runtime.ts b/src/daemon/server/daemon-runtime.ts index e7ed0477f..db1179eaa 100644 --- a/src/daemon/server/daemon-runtime.ts +++ b/src/daemon/server/daemon-runtime.ts @@ -213,27 +213,21 @@ export async function startDaemonRuntime( const finalizeDaemonSessionLease = async (session: SessionState): Promise => { if (!session.lease) return; - const releaseRequest = leaseScopeToReleaseRequest({ - leaseId: session.lease.leaseId, - tenantId: session.lease.tenantId, - runId: session.lease.runId, - leaseBackend: session.lease.leaseBackend, - leaseProvider: session.lease.leaseProvider, - deviceKey: session.lease.deviceKey, - clientId: session.lease.clientId, - }); - const activeLease = leaseRegistry.getLease(releaseRequest); - if (!activeLease) return; try { + const releaseRequest = leaseScopeToReleaseRequest({ + leaseId: session.lease.leaseId, + tenantId: session.lease.tenantId, + runId: session.lease.runId, + leaseBackend: session.lease.leaseBackend, + leaseProvider: session.lease.leaseProvider, + deviceKey: session.lease.deviceKey, + clientId: session.lease.clientId, + }); + const activeLease = leaseRegistry.getLease(releaseRequest); + if (!activeLease) return; await expiredProviderLeaseReleaser.release(activeLease); leaseRegistry.releaseLease(releaseRequest); } catch (error) { - // The session is about to be removed. Queue the provider allocation while - // this daemon can still persist retry state, then release only the local - // registry entry. This mirrors expiry recovery without pretending the - // external provider release succeeded. - if (activeLease) await expiredProviderLeaseReleaser.release(activeLease); - leaseRegistry.releaseLease(releaseRequest); emitDiagnostic({ level: 'warn', phase: 'daemon_shutdown_session_lease_release_failed', From b4b3effa15b3060d0d4685a028be3087016e72d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 17 Jul 2026 13:20:06 +0200 Subject: [PATCH 3/6] fix: fail closed daemon stop cleanup --- scripts/integration-progress-model.ts | 5 ++++ src/cli/commands/__tests__/daemon.test.ts | 28 +++++++++++++++++++ src/cli/commands/daemon.ts | 24 ++++++++++++++-- .../__tests__/daemon-shutdown-report.test.ts | 7 +++++ src/daemon/__tests__/daemon-stop.test.ts | 17 +++++++++++ .../__tests__/provider-lease-expiry.test.ts | 12 ++++++-- src/daemon/daemon-shutdown-report.ts | 17 ++++++++++- src/daemon/daemon-stop.ts | 20 +++++++++++-- src/daemon/provider-lease-expiry.ts | 24 +++++++++------- 9 files changed, 135 insertions(+), 19 deletions(-) diff --git a/scripts/integration-progress-model.ts b/scripts/integration-progress-model.ts index 841cdf5e7..3a2e95bfc 100644 --- a/scripts/integration-progress-model.ts +++ b/scripts/integration-progress-model.ts @@ -319,6 +319,11 @@ function summarizeProviderScenarioFlagExclusions() { 'proxyPort', ], }, + { + name: 'daemon lifecycle control', + owner: 'daemon CLI lifecycle tests', + keys: ['clean'], + }, { name: 'platform boot fallback without provider seam', owner: 'handler and Android platform unit tests', diff --git a/src/cli/commands/__tests__/daemon.test.ts b/src/cli/commands/__tests__/daemon.test.ts index 1d673ce48..ad0aeda54 100644 --- a/src/cli/commands/__tests__/daemon.test.ts +++ b/src/cli/commands/__tests__/daemon.test.ts @@ -103,3 +103,31 @@ test('merges a graceful shutdown report and cleans runner leases with the start- fs.rmSync(stateDir, { recursive: true, force: true }); } }); + +test('reports graceful provider cleanup as unknown when the shutdown report is unavailable', async () => { + const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-daemon-command-')); + mocks.readDaemonStopIdentity.mockReturnValue(null); + mocks.stopDaemon.mockResolvedValue(GRACEFUL_RESULT); + mocks.readDaemonShutdownReport.mockReturnValue(null); + + try { + await daemonCommand({ + positionals: ['stop'], + flags: { help: false, json: false, stateDir, version: false }, + client: {} as never, + }); + + expect(mocks.writeCommandOutput).toHaveBeenCalledWith( + expect.objectContaining({ json: false }), + expect.objectContaining({ + clean: false, + cleanupConfidence: 'unknown', + providerReleases: { status: 'unknown', released: [], pending: null }, + warnings: [expect.stringContaining('provider cleanup state is unknown')], + }), + expect.any(Function), + ); + } finally { + fs.rmSync(stateDir, { recursive: true, force: true }); + } +}); diff --git a/src/cli/commands/daemon.ts b/src/cli/commands/daemon.ts index 9908d271f..d5ffd3a46 100644 --- a/src/cli/commands/daemon.ts +++ b/src/cli/commands/daemon.ts @@ -18,9 +18,7 @@ export const daemonCommand: ClientCommandHandler = async ({ positionals, flags } const identity = readDaemonStopIdentity(paths.infoPath); const stopped = await stopDaemon({ paths }); const report = stopped.mode === 'graceful' ? readDaemonShutdownReport(paths.baseDir) : null; - const result = report - ? { ...stopped, providerReleases: { status: 'completed' as const, ...report.providerReleases } } - : stopped; + const result = mergeShutdownReport(stopped, report); const shouldClean = flags.clean === true && identity !== null && result.stopped; if (shouldClean) { const [{ cleanupRunnerLeasesForOwner }, { runnerLeaseCleanupAdapter }] = await Promise.all([ @@ -37,6 +35,26 @@ export const daemonCommand: ClientCommandHandler = async ({ positionals, flags } return true; }; +function mergeShutdownReport( + stopped: DaemonStopResult, + report: ReturnType, +): DaemonStopResult { + if (stopped.mode !== 'graceful' || report) { + return report + ? { ...stopped, providerReleases: { status: 'completed', ...report.providerReleases } } + : stopped; + } + return { + ...stopped, + cleanupConfidence: 'unknown', + providerReleases: { status: 'unknown', released: [], pending: null }, + warnings: [ + ...stopped.warnings, + 'The graceful shutdown report is unavailable, so provider cleanup state is unknown. Provider allocations may remain active.', + ], + }; +} + function renderDaemonStop( result: Pick & { clean: boolean; diff --git a/src/daemon/__tests__/daemon-shutdown-report.test.ts b/src/daemon/__tests__/daemon-shutdown-report.test.ts index 3d206af0e..559b453bb 100644 --- a/src/daemon/__tests__/daemon-shutdown-report.test.ts +++ b/src/daemon/__tests__/daemon-shutdown-report.test.ts @@ -36,9 +36,16 @@ test('ignores malformed shutdown reports and clear removes a prior report', () = const reportPath = path.join(stateDir, 'daemon-shutdown.json'); try { + expect(readDaemonShutdownReport(stateDir)).toBeNull(); fs.writeFileSync(reportPath, JSON.stringify({ providerReleases: { released: [] } })); expect(readDaemonShutdownReport(stateDir)).toBeNull(); + fs.writeFileSync( + reportPath, + JSON.stringify({ providerReleases: { released: [{}], pending: [] } }), + ); + expect(readDaemonShutdownReport(stateDir)).toBeNull(); + clearDaemonShutdownReport(stateDir); expect(fs.existsSync(reportPath)).toBe(false); } finally { diff --git a/src/daemon/__tests__/daemon-stop.test.ts b/src/daemon/__tests__/daemon-stop.test.ts index e1631963c..4309d304c 100644 --- a/src/daemon/__tests__/daemon-stop.test.ts +++ b/src/daemon/__tests__/daemon-stop.test.ts @@ -70,6 +70,23 @@ test('refuses to signal a live process whose daemon identity cannot be verified' } }); +test('refuses to signal a live process when daemon metadata lacks a non-empty start-time identity', async () => { + const paths = createDaemonPaths(); + fs.writeFileSync(paths.infoPath, JSON.stringify({ pid: 123, processStartTime: ' ' })); + mocks.isProcessAlive.mockReturnValue(true); + + try { + await assert.rejects( + async () => await stopDaemon({ paths }), + (error: { code?: string }) => error.code === 'COMMAND_FAILED', + ); + expect(mocks.isAgentDeviceDaemonProcess).not.toHaveBeenCalled(); + expect(mocks.trySignalProcess).not.toHaveBeenCalled(); + } finally { + removeDaemonPaths(paths); + } +}); + test('treats an exited daemon between identity verification and SIGTERM as not-running', async () => { const paths = createDaemonPaths(); mocks.isAgentDeviceDaemonProcess.mockReturnValue(true); diff --git a/src/daemon/__tests__/provider-lease-expiry.test.ts b/src/daemon/__tests__/provider-lease-expiry.test.ts index 892c20384..c668af352 100644 --- a/src/daemon/__tests__/provider-lease-expiry.test.ts +++ b/src/daemon/__tests__/provider-lease-expiry.test.ts @@ -122,13 +122,20 @@ test('does not release until the expired lease record is durable', async () => { }); test('final drain reports releases completed during daemon shutdown', async () => { - const lease = new LeaseRegistry().allocateLease({ + const leaseRegistry = new LeaseRegistry(); + const releasedBeforeShutdown = leaseRegistry.allocateLease({ + tenantId: 'tenant-a', + runId: 'run-0', + leaseProvider: 'browserstack', + }); + const lease = leaseRegistry.allocateLease({ tenantId: 'tenant-a', runId: 'run-1', leaseProvider: 'browserstack', }); const release = vi .fn() + .mockResolvedValueOnce({}) .mockRejectedValueOnce(new Error('temporary outage')) .mockResolvedValue({}); const releaser = createExpiredProviderLeaseReleaser({ @@ -137,10 +144,11 @@ test('final drain reports releases completed during daemon shutdown', async () = }); try { + await releaser.release(releasedBeforeShutdown); await releaser.release(lease); const drained = await releaser.drain(10); - expect(release).toHaveBeenCalledTimes(2); + expect(release).toHaveBeenCalledTimes(3); expect(release).toHaveBeenLastCalledWith(lease); expect(drained.released).toEqual([lease]); expect(drained.pending).toEqual([]); diff --git a/src/daemon/daemon-shutdown-report.ts b/src/daemon/daemon-shutdown-report.ts index 4234501bd..8982fb8aa 100644 --- a/src/daemon/daemon-shutdown-report.ts +++ b/src/daemon/daemon-shutdown-report.ts @@ -70,5 +70,20 @@ function isDaemonShutdownReport(value: unknown): value is DaemonShutdownReport { const releases = (value as { providerReleases?: unknown }).providerReleases; if (!releases || typeof releases !== 'object') return false; const records = releases as { released?: unknown; pending?: unknown }; - return Array.isArray(records.released) && Array.isArray(records.pending); + return ( + Array.isArray(records.released) && + Array.isArray(records.pending) && + records.released.every(isProviderReleaseRecord) && + records.pending.every(isProviderReleaseRecord) + ); +} + +function isProviderReleaseRecord(value: unknown): value is ProviderReleaseRecord { + if (!value || typeof value !== 'object') return false; + const record = value as { leaseId?: unknown; provider?: unknown }; + return ( + typeof record.leaseId === 'string' && + record.leaseId.trim().length > 0 && + (record.provider === undefined || typeof record.provider === 'string') + ); } diff --git a/src/daemon/daemon-stop.ts b/src/daemon/daemon-stop.ts index 5864ea0bd..815405916 100644 --- a/src/daemon/daemon-stop.ts +++ b/src/daemon/daemon-stop.ts @@ -36,6 +36,14 @@ export async function stopDaemon(params: { }): Promise { const info = readDaemonInfo(params.paths.infoPath); if (!info) return notRunningResult(); + if (!info.processStartTime) { + if (!isProcessAlive(info.pid)) return notRunningResult(); + throw new AppError( + 'COMMAND_FAILED', + 'Refusing to stop a daemon without a verified process start-time identity.', + { pid: info.pid }, + ); + } if (!isAgentDeviceDaemonProcess(info.pid, info.processStartTime)) { if (!isProcessAlive(info.pid)) return notRunningResult(); throw new AppError( @@ -99,8 +107,10 @@ function signalDaemonProcess(pid: number, signal: NodeJS.Signals): boolean { export function readDaemonStopIdentity( infoPath: string, -): { pid: number; processStartTime?: string } | null { - return readDaemonInfo(infoPath); +): { pid: number; processStartTime: string } | null { + const info = readDaemonInfo(infoPath); + if (!info?.processStartTime) return null; + return { pid: info.pid, processStartTime: info.processStartTime }; } function readDaemonInfo(infoPath: string): { pid: number; processStartTime?: string } | null { @@ -110,7 +120,7 @@ function readDaemonInfo(infoPath: string): { pid: number; processStartTime?: str if (typeof pid !== 'number' || !Number.isInteger(pid) || pid <= 0) return null; return { pid, - ...(typeof parsed.processStartTime === 'string' + ...(hasProcessStartTime(parsed.processStartTime) ? { processStartTime: parsed.processStartTime } : {}), }; @@ -119,6 +129,10 @@ function readDaemonInfo(infoPath: string): { pid: number; processStartTime?: str } } +function hasProcessStartTime(value: unknown): value is string { + return typeof value === 'string' && value.trim().length > 0; +} + async function waitForDaemonMetadataRemoval(paths: DaemonPaths, timeoutMs: number): Promise { const startedAt = Date.now(); while (Date.now() - startedAt < timeoutMs) { diff --git a/src/daemon/provider-lease-expiry.ts b/src/daemon/provider-lease-expiry.ts index 64a4b7142..53ef10cb3 100644 --- a/src/daemon/provider-lease-expiry.ts +++ b/src/daemon/provider-lease-expiry.ts @@ -47,7 +47,6 @@ export function createExpiredProviderLeaseReleaser(options: { const retryDelayMs = options.retryDelayMs ?? DEFAULT_RETRY_DELAY_MS; let retryTimer: Timer | undefined; let retrying = false; - const releasedLeases = new Map(); const persistPendingLeases = (): boolean => { if (!options.stateDir) { @@ -107,34 +106,38 @@ export function createExpiredProviderLeaseReleaser(options: { }); }; - const retryPendingLiveLeases = async (): Promise => { + const retryPendingLiveLeases = async ( + released: Map | undefined, + ): Promise => { for (const lease of pendingLiveLeases.values()) { if (!isProviderRuntimeAvailable(lease, options.providerRuntimeIds)) continue; if (await releaseLiveProviderLease(options.leaseLifecycleProvider, lease)) { pendingLiveLeases.delete(lease.leaseId); - releasedLeases.set(lease.leaseId, lease); + released?.set(lease.leaseId, lease); } } }; - const retryPendingRecoveryLeases = async (): Promise => { + const retryPendingRecoveryLeases = async ( + released: Map | undefined, + ): Promise => { for (const lease of pendingRecoveryLeases.values()) { if (!isRecoverableProviderAvailable(lease, options.recoverableProviderIds)) continue; if (!persistedRecoveryLeaseIds.has(lease.leaseId) && !persistPendingLeases()) continue; if (await releaseExpiredProviderLease(options.recoverExpiredLease, lease)) { pendingRecoveryLeases.delete(lease.leaseId); - releasedLeases.set(lease.leaseId, lease); + released?.set(lease.leaseId, lease); persistPendingLeases(); } } }; - const retryPending = async (): Promise => { + const retryPending = async (released?: Map): Promise => { if (retrying) return; retrying = true; try { - await retryPendingLiveLeases(); - await retryPendingRecoveryLeases(); + await retryPendingLiveLeases(released); + await retryPendingRecoveryLeases(released); } finally { retrying = false; if (pendingLiveLeases.size === 0 && pendingRecoveryLeases.size === 0 && retryTimer) { @@ -170,10 +173,11 @@ export function createExpiredProviderLeaseReleaser(options: { drain: async (timeoutMs) => { // Shutdown gets one final, bounded attempt at every pending release. A // provider call that hangs must not keep the daemon alive indefinitely. - await Promise.race([retryPending(), sleep(Math.max(0, timeoutMs))]); + const released = new Map(); + await Promise.race([retryPending(released), sleep(Math.max(0, timeoutMs))]); return { pending: [...pendingLiveLeases.values(), ...pendingRecoveryLeases.values()], - released: [...releasedLeases.values()], + released: [...released.values()], }; }, shutdown: () => { From be97b44147b8053453f6dcf38b266816f031a612 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 17 Jul 2026 13:34:49 +0200 Subject: [PATCH 4/6] fix: bound daemon shutdown lease releases --- .../__tests__/provider-lease-expiry.test.ts | 15 ++-- src/daemon/provider-lease-expiry.ts | 39 +++++++---- src/daemon/server/daemon-runtime.ts | 41 +++-------- .../daemon-session-lease-finalizer.test.ts | 60 ++++++++++++++++ .../server/daemon-session-lease-finalizer.ts | 70 +++++++++++++++++++ 5 files changed, 175 insertions(+), 50 deletions(-) create mode 100644 src/daemon/server/daemon-session-lease-finalizer.test.ts create mode 100644 src/daemon/server/daemon-session-lease-finalizer.ts diff --git a/src/daemon/__tests__/provider-lease-expiry.test.ts b/src/daemon/__tests__/provider-lease-expiry.test.ts index c668af352..bd55b397b 100644 --- a/src/daemon/__tests__/provider-lease-expiry.test.ts +++ b/src/daemon/__tests__/provider-lease-expiry.test.ts @@ -121,18 +121,23 @@ test('does not release until the expired lease record is durable', async () => { } }); -test('final drain reports releases completed during daemon shutdown', async () => { +test('shutdown drain reports releases completed before and during its final retry', async () => { const leaseRegistry = new LeaseRegistry(); const releasedBeforeShutdown = leaseRegistry.allocateLease({ tenantId: 'tenant-a', runId: 'run-0', leaseProvider: 'browserstack', }); - const lease = leaseRegistry.allocateLease({ + const releasedDuringShutdown = leaseRegistry.allocateLease({ tenantId: 'tenant-a', runId: 'run-1', leaseProvider: 'browserstack', }); + const lease = leaseRegistry.allocateLease({ + tenantId: 'tenant-a', + runId: 'run-2', + leaseProvider: 'browserstack', + }); const release = vi .fn() .mockResolvedValueOnce({}) @@ -145,12 +150,14 @@ test('final drain reports releases completed during daemon shutdown', async () = try { await releaser.release(releasedBeforeShutdown); + releaser.beginShutdown(); + await releaser.release(releasedDuringShutdown); await releaser.release(lease); const drained = await releaser.drain(10); - expect(release).toHaveBeenCalledTimes(3); + expect(release).toHaveBeenCalledTimes(4); expect(release).toHaveBeenLastCalledWith(lease); - expect(drained.released).toEqual([lease]); + expect(drained.released).toEqual([releasedDuringShutdown, lease]); expect(drained.pending).toEqual([]); } finally { releaser.shutdown(); diff --git a/src/daemon/provider-lease-expiry.ts b/src/daemon/provider-lease-expiry.ts index 53ef10cb3..20a055a7d 100644 --- a/src/daemon/provider-lease-expiry.ts +++ b/src/daemon/provider-lease-expiry.ts @@ -27,6 +27,7 @@ type PersistedExpiredProviderLeases = { }; export type ExpiredProviderLeaseReleaser = { + beginShutdown: () => void; release: (lease: DeviceLease) => Promise; retryPending: () => Promise; drain: (timeoutMs: number) => Promise<{ pending: DeviceLease[]; released: DeviceLease[] }>; @@ -47,6 +48,12 @@ export function createExpiredProviderLeaseReleaser(options: { const retryDelayMs = options.retryDelayMs ?? DEFAULT_RETRY_DELAY_MS; let retryTimer: Timer | undefined; let retrying = false; + let trackingShutdownReleases = false; + const shutdownReleasedLeases = new Map(); + + const recordReleasedLease = (lease: DeviceLease): void => { + if (trackingShutdownReleases) shutdownReleasedLeases.set(lease.leaseId, lease); + }; const persistPendingLeases = (): boolean => { if (!options.stateDir) { @@ -106,38 +113,34 @@ export function createExpiredProviderLeaseReleaser(options: { }); }; - const retryPendingLiveLeases = async ( - released: Map | undefined, - ): Promise => { + const retryPendingLiveLeases = async (): Promise => { for (const lease of pendingLiveLeases.values()) { if (!isProviderRuntimeAvailable(lease, options.providerRuntimeIds)) continue; if (await releaseLiveProviderLease(options.leaseLifecycleProvider, lease)) { pendingLiveLeases.delete(lease.leaseId); - released?.set(lease.leaseId, lease); + recordReleasedLease(lease); } } }; - const retryPendingRecoveryLeases = async ( - released: Map | undefined, - ): Promise => { + const retryPendingRecoveryLeases = async (): Promise => { for (const lease of pendingRecoveryLeases.values()) { if (!isRecoverableProviderAvailable(lease, options.recoverableProviderIds)) continue; if (!persistedRecoveryLeaseIds.has(lease.leaseId) && !persistPendingLeases()) continue; if (await releaseExpiredProviderLease(options.recoverExpiredLease, lease)) { pendingRecoveryLeases.delete(lease.leaseId); - released?.set(lease.leaseId, lease); + recordReleasedLease(lease); persistPendingLeases(); } } }; - const retryPending = async (released?: Map): Promise => { + const retryPending = async (): Promise => { if (retrying) return; retrying = true; try { - await retryPendingLiveLeases(released); - await retryPendingRecoveryLeases(released); + await retryPendingLiveLeases(); + await retryPendingRecoveryLeases(); } finally { retrying = false; if (pendingLiveLeases.size === 0 && pendingRecoveryLeases.size === 0 && retryTimer) { @@ -150,6 +153,10 @@ export function createExpiredProviderLeaseReleaser(options: { }; return { + beginShutdown: () => { + shutdownReleasedLeases.clear(); + trackingShutdownReleases = true; + }, release: async (lease) => { if (!lease.leaseProvider) return; if (isRecoverableProviderAvailable(lease, options.recoverableProviderIds)) { @@ -173,12 +180,14 @@ export function createExpiredProviderLeaseReleaser(options: { drain: async (timeoutMs) => { // Shutdown gets one final, bounded attempt at every pending release. A // provider call that hangs must not keep the daemon alive indefinitely. - const released = new Map(); - await Promise.race([retryPending(released), sleep(Math.max(0, timeoutMs))]); - return { + await Promise.race([retryPending(), sleep(Math.max(0, timeoutMs))]); + const result = { pending: [...pendingLiveLeases.values(), ...pendingRecoveryLeases.values()], - released: [...released.values()], + released: [...shutdownReleasedLeases.values()], }; + trackingShutdownReleases = false; + shutdownReleasedLeases.clear(); + return result; }, shutdown: () => { if (retryTimer) clearTimeout(retryTimer); diff --git a/src/daemon/server/daemon-runtime.ts b/src/daemon/server/daemon-runtime.ts index db1179eaa..e421d6784 100644 --- a/src/daemon/server/daemon-runtime.ts +++ b/src/daemon/server/daemon-runtime.ts @@ -16,7 +16,6 @@ import { } from '../../provider-device-runtimes.ts'; import { LeaseRegistry } from '../lease-registry.ts'; import { createExpiredProviderLeaseReleaser } from '../provider-lease-expiry.ts'; -import { leaseScopeToReleaseRequest } from '../../core/lease-scope.ts'; import { clearDaemonShutdownReport, writeDaemonShutdownReport } from '../daemon-shutdown-report.ts'; import { createRequestHandler } from '../request-router.ts'; import { teardownSessionResources } from '../session-teardown.ts'; @@ -24,6 +23,7 @@ import { IOS_SIMULATOR_RECORDING_STOP_ESCALATION_BUDGET_MS } from '../handlers/r import { closeDaemonServers } from './server-shutdown.ts'; import type { DaemonInvokeFn, SessionState } from '../types.ts'; import { createDaemonIdleReap } from './daemon-idle-reap.ts'; +import { finalizeDaemonSessionLease } from './daemon-session-lease-finalizer.ts'; import { emitDiagnostic, flushDiagnosticsToSessionFile, @@ -58,6 +58,7 @@ import { } from '../../platforms/android/ime-lifecycle.ts'; const DAEMON_SESSION_TEARDOWN_TIMEOUT_MS = 5_000; +const DAEMON_SESSION_LEASE_RELEASE_TIMEOUT_MS = 1_000; const DAEMON_PNG_WORKER_TERMINATE_TIMEOUT_MS = 1_000; const DAEMON_PROVIDER_RELEASE_DRAIN_TIMEOUT_MS = 2_000; @@ -211,42 +212,19 @@ export async function startDaemonRuntime( ); }; - const finalizeDaemonSessionLease = async (session: SessionState): Promise => { - if (!session.lease) return; - try { - const releaseRequest = leaseScopeToReleaseRequest({ - leaseId: session.lease.leaseId, - tenantId: session.lease.tenantId, - runId: session.lease.runId, - leaseBackend: session.lease.leaseBackend, - leaseProvider: session.lease.leaseProvider, - deviceKey: session.lease.deviceKey, - clientId: session.lease.clientId, - }); - const activeLease = leaseRegistry.getLease(releaseRequest); - if (!activeLease) return; - await expiredProviderLeaseReleaser.release(activeLease); - leaseRegistry.releaseLease(releaseRequest); - } catch (error) { - emitDiagnostic({ - level: 'warn', - phase: 'daemon_shutdown_session_lease_release_failed', - data: { - session: session.name, - leaseId: session.lease.leaseId, - error: error instanceof Error ? error.message : String(error), - }, - }); - } - }; - const teardownDaemonSession = async (session: SessionState): Promise => await teardownDaemonSessionForShutdown({ session, sessionStore, stateDir: baseDir, stderr, - beforeDelete: finalizeDaemonSessionLease, + beforeDelete: async (sessionToFinalize) => + await finalizeDaemonSessionLease({ + session: sessionToFinalize, + leaseRegistry, + expiredProviderLeaseReleaser, + timeoutMs: DAEMON_SESSION_LEASE_RELEASE_TIMEOUT_MS, + }), }); const teardownDaemonSessions = async (): Promise => { @@ -394,6 +372,7 @@ export async function startDaemonRuntime( try { await detachIosSimulatorRunnerSessionsForShutdown(); } catch {} + expiredProviderLeaseReleaser.beginShutdown(); await teardownDaemonSessions(); const providerReleaseDrain = await expiredProviderLeaseReleaser.drain( DAEMON_PROVIDER_RELEASE_DRAIN_TIMEOUT_MS, diff --git a/src/daemon/server/daemon-session-lease-finalizer.test.ts b/src/daemon/server/daemon-session-lease-finalizer.test.ts new file mode 100644 index 000000000..63ed1e31c --- /dev/null +++ b/src/daemon/server/daemon-session-lease-finalizer.test.ts @@ -0,0 +1,60 @@ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { expect, test, vi } from 'vitest'; +import { makeIosSession } from '../../__tests__/test-utils/session-factories.ts'; +import { LeaseRegistry } from '../lease-registry.ts'; +import { createExpiredProviderLeaseReleaser } from '../provider-lease-expiry.ts'; +import { finalizeDaemonSessionLease } from './daemon-session-lease-finalizer.ts'; + +test('journals and bounds a hung recoverable session lease release before the final drain', async () => { + vi.useFakeTimers(); + const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-daemon-lease-finalizer-')); + const leaseRegistry = new LeaseRegistry(); + const lease = leaseRegistry.allocateLease({ + tenantId: 'tenant-a', + runId: 'run-1', + leaseProvider: 'limrun', + }); + const recoverExpiredLease = vi.fn(() => new Promise(() => {})); + const expiredProviderLeaseReleaser = createExpiredProviderLeaseReleaser({ + recoverExpiredLease, + recoverableProviderIds: ['limrun'], + stateDir, + }); + const session = makeIosSession('default', { + lease: { + leaseId: lease.leaseId, + tenantId: lease.tenantId, + runId: lease.runId, + leaseBackend: lease.backend, + leaseProvider: lease.leaseProvider, + expiresAt: lease.expiresAt, + }, + }); + + try { + expiredProviderLeaseReleaser.beginShutdown(); + const finalization = finalizeDaemonSessionLease({ + session, + leaseRegistry, + expiredProviderLeaseReleaser, + timeoutMs: 10, + }); + + await vi.advanceTimersByTimeAsync(10); + await finalization; + + expect(recoverExpiredLease).toHaveBeenCalledWith(lease); + expect(leaseRegistry.listActiveLeases()).toEqual([]); + expect(fs.existsSync(path.join(stateDir, 'expired-provider-leases.json'))).toBe(true); + await expect(expiredProviderLeaseReleaser.drain(10)).resolves.toEqual({ + pending: [lease], + released: [], + }); + } finally { + expiredProviderLeaseReleaser.shutdown(); + vi.useRealTimers(); + fs.rmSync(stateDir, { recursive: true, force: true }); + } +}); diff --git a/src/daemon/server/daemon-session-lease-finalizer.ts b/src/daemon/server/daemon-session-lease-finalizer.ts new file mode 100644 index 000000000..4e48b1029 --- /dev/null +++ b/src/daemon/server/daemon-session-lease-finalizer.ts @@ -0,0 +1,70 @@ +import { leaseScopeToReleaseRequest } from '../../core/lease-scope.ts'; +import { emitDiagnostic } from '../../utils/diagnostics.ts'; +import type { ExpiredProviderLeaseReleaser } from '../provider-lease-expiry.ts'; +import type { LeaseRegistry } from '../lease-registry.ts'; +import type { SessionState } from '../types.ts'; + +export async function finalizeDaemonSessionLease(params: { + session: SessionState; + leaseRegistry: LeaseRegistry; + expiredProviderLeaseReleaser: ExpiredProviderLeaseReleaser; + timeoutMs: number; +}): Promise { + const { session, leaseRegistry, expiredProviderLeaseReleaser, timeoutMs } = params; + if (!session.lease) return; + try { + const releaseRequest = leaseScopeToReleaseRequest({ + leaseId: session.lease.leaseId, + tenantId: session.lease.tenantId, + runId: session.lease.runId, + leaseBackend: session.lease.leaseBackend, + leaseProvider: session.lease.leaseProvider, + deviceKey: session.lease.deviceKey, + clientId: session.lease.clientId, + }); + const activeLease = leaseRegistry.getLease(releaseRequest); + if (!activeLease) return; + const completed = await releaseWithinTimeout( + expiredProviderLeaseReleaser.release(activeLease), + timeoutMs, + ); + leaseRegistry.releaseLease(releaseRequest); + if (!completed) { + emitDiagnostic({ + level: 'warn', + phase: 'daemon_shutdown_session_lease_release_timed_out', + data: { + session: session.name, + leaseId: session.lease.leaseId, + timeoutMs, + }, + }); + } + } catch (error) { + emitDiagnostic({ + level: 'warn', + phase: 'daemon_shutdown_session_lease_release_failed', + data: { + session: session.name, + leaseId: session.lease.leaseId, + error: error instanceof Error ? error.message : String(error), + }, + }); + } +} + +async function releaseWithinTimeout(release: Promise, timeoutMs: number): Promise { + return await new Promise((resolve, reject) => { + const timeout = setTimeout(() => resolve(false), timeoutMs); + void release.then( + () => { + clearTimeout(timeout); + resolve(true); + }, + (error: unknown) => { + clearTimeout(timeout); + reject(error); + }, + ); + }); +} From 497b644e555289c45ca9cde39ba03f9b2b380a57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 17 Jul 2026 14:00:12 +0200 Subject: [PATCH 5/6] fix: await active shutdown lease release --- src/daemon/provider-lease-expiry.ts | 40 ++++++++------ .../daemon-session-lease-finalizer.test.ts | 54 ++++++++++++++++++- 2 files changed, 78 insertions(+), 16 deletions(-) diff --git a/src/daemon/provider-lease-expiry.ts b/src/daemon/provider-lease-expiry.ts index 20a055a7d..7cdabaff9 100644 --- a/src/daemon/provider-lease-expiry.ts +++ b/src/daemon/provider-lease-expiry.ts @@ -47,7 +47,7 @@ export function createExpiredProviderLeaseReleaser(options: { const persistedRecoveryLeaseIds = new Set(pendingRecoveryLeases.keys()); const retryDelayMs = options.retryDelayMs ?? DEFAULT_RETRY_DELAY_MS; let retryTimer: Timer | undefined; - let retrying = false; + let activeRetry: Promise | undefined; let trackingShutdownReleases = false; const shutdownReleasedLeases = new Map(); @@ -135,21 +135,31 @@ export function createExpiredProviderLeaseReleaser(options: { } }; - const retryPending = async (): Promise => { - if (retrying) return; - retrying = true; - try { - await retryPendingLiveLeases(); - await retryPendingRecoveryLeases(); - } finally { - retrying = false; - if (pendingLiveLeases.size === 0 && pendingRecoveryLeases.size === 0 && retryTimer) { - clearTimeout(retryTimer); - retryTimer = undefined; - } else { - scheduleRetry(); + const retryPending = (): Promise => { + if (activeRetry) return activeRetry; + const retry = (async (): Promise => { + try { + await retryPendingLiveLeases(); + await retryPendingRecoveryLeases(); + } finally { + if (pendingLiveLeases.size === 0 && pendingRecoveryLeases.size === 0 && retryTimer) { + clearTimeout(retryTimer); + retryTimer = undefined; + } else { + scheduleRetry(); + } } - } + })(); + activeRetry = retry; + void retry.then( + () => { + if (activeRetry === retry) activeRetry = undefined; + }, + () => { + if (activeRetry === retry) activeRetry = undefined; + }, + ); + return retry; }; return { diff --git a/src/daemon/server/daemon-session-lease-finalizer.test.ts b/src/daemon/server/daemon-session-lease-finalizer.test.ts index 63ed1e31c..473062b0a 100644 --- a/src/daemon/server/daemon-session-lease-finalizer.test.ts +++ b/src/daemon/server/daemon-session-lease-finalizer.test.ts @@ -48,7 +48,9 @@ test('journals and bounds a hung recoverable session lease release before the fi expect(recoverExpiredLease).toHaveBeenCalledWith(lease); expect(leaseRegistry.listActiveLeases()).toEqual([]); expect(fs.existsSync(path.join(stateDir, 'expired-provider-leases.json'))).toBe(true); - await expect(expiredProviderLeaseReleaser.drain(10)).resolves.toEqual({ + const drain = expiredProviderLeaseReleaser.drain(10); + await vi.advanceTimersByTimeAsync(10); + await expect(drain).resolves.toEqual({ pending: [lease], released: [], }); @@ -58,3 +60,53 @@ test('journals and bounds a hung recoverable session lease release before the fi fs.rmSync(stateDir, { recursive: true, force: true }); } }); + +test('final drain joins a release that completes after the session timeout', async () => { + vi.useFakeTimers(); + const leaseRegistry = new LeaseRegistry(); + const lease = leaseRegistry.allocateLease({ + tenantId: 'tenant-a', + runId: 'run-1', + leaseProvider: 'browserstack', + }); + const release = vi.fn( + () => + new Promise>((resolve) => { + setTimeout(() => resolve({}), 1_500); + }), + ); + const expiredProviderLeaseReleaser = createExpiredProviderLeaseReleaser({ + leaseLifecycleProvider: { release }, + providerRuntimeIds: ['browserstack'], + }); + const session = makeIosSession('default', { + lease: { + leaseId: lease.leaseId, + tenantId: lease.tenantId, + runId: lease.runId, + leaseBackend: lease.backend, + leaseProvider: lease.leaseProvider, + expiresAt: lease.expiresAt, + }, + }); + + try { + expiredProviderLeaseReleaser.beginShutdown(); + const finalization = finalizeDaemonSessionLease({ + session, + leaseRegistry, + expiredProviderLeaseReleaser, + timeoutMs: 1_000, + }); + + await vi.advanceTimersByTimeAsync(1_000); + await finalization; + const drain = expiredProviderLeaseReleaser.drain(2_000); + + await vi.advanceTimersByTimeAsync(500); + await expect(drain).resolves.toEqual({ pending: [], released: [lease] }); + } finally { + expiredProviderLeaseReleaser.shutdown(); + vi.useRealTimers(); + } +}); From 630e0bb6592e3c0eba57444d06083a79a945830b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 17 Jul 2026 17:41:50 +0200 Subject: [PATCH 6/6] fix: release provider leases independently on shutdown --- .../__tests__/provider-lease-expiry.test.ts | 5 +- src/daemon/provider-lease-expiry.ts | 82 ++++++++++--------- .../daemon-session-lease-finalizer.test.ts | 69 +++++++++++++++- 3 files changed, 116 insertions(+), 40 deletions(-) diff --git a/src/daemon/__tests__/provider-lease-expiry.test.ts b/src/daemon/__tests__/provider-lease-expiry.test.ts index bd55b397b..e07332783 100644 --- a/src/daemon/__tests__/provider-lease-expiry.test.ts +++ b/src/daemon/__tests__/provider-lease-expiry.test.ts @@ -156,8 +156,9 @@ test('shutdown drain reports releases completed before and during its final retr const drained = await releaser.drain(10); expect(release).toHaveBeenCalledTimes(4); - expect(release).toHaveBeenLastCalledWith(lease); - expect(drained.released).toEqual([releasedDuringShutdown, lease]); + expect(release).toHaveBeenCalledWith(lease); + expect(drained.released).toHaveLength(2); + expect(drained.released).toEqual(expect.arrayContaining([releasedDuringShutdown, lease])); expect(drained.pending).toEqual([]); } finally { releaser.shutdown(); diff --git a/src/daemon/provider-lease-expiry.ts b/src/daemon/provider-lease-expiry.ts index 7cdabaff9..bf9b06a2b 100644 --- a/src/daemon/provider-lease-expiry.ts +++ b/src/daemon/provider-lease-expiry.ts @@ -47,7 +47,7 @@ export function createExpiredProviderLeaseReleaser(options: { const persistedRecoveryLeaseIds = new Set(pendingRecoveryLeases.keys()); const retryDelayMs = options.retryDelayMs ?? DEFAULT_RETRY_DELAY_MS; let retryTimer: Timer | undefined; - let activeRetry: Promise | undefined; + const activeReleaseAttempts = new Map>(); let trackingShutdownReleases = false; const shutdownReleasedLeases = new Map(); @@ -113,53 +113,61 @@ export function createExpiredProviderLeaseReleaser(options: { }); }; - const retryPendingLiveLeases = async (): Promise => { - for (const lease of pendingLiveLeases.values()) { - if (!isProviderRuntimeAvailable(lease, options.providerRuntimeIds)) continue; + const trackReleaseAttempt = (key: string, attempt: Promise): Promise => { + activeReleaseAttempts.set(key, attempt); + const finish = (): void => { + if (activeReleaseAttempts.get(key) === attempt) activeReleaseAttempts.delete(key); + if (pendingLiveLeases.size === 0 && pendingRecoveryLeases.size === 0 && retryTimer) { + clearTimeout(retryTimer); + retryTimer = undefined; + } else { + scheduleRetry(); + } + }; + void attempt.then(finish, finish); + return attempt; + }; + + const ensureLiveReleaseAttempt = (lease: DeviceLease): Promise | undefined => { + if (!isProviderRuntimeAvailable(lease, options.providerRuntimeIds)) return undefined; + const key = `live:${lease.leaseId}`; + const activeAttempt = activeReleaseAttempts.get(key); + if (activeAttempt) return activeAttempt; + const attempt = (async (): Promise => { if (await releaseLiveProviderLease(options.leaseLifecycleProvider, lease)) { pendingLiveLeases.delete(lease.leaseId); recordReleasedLease(lease); } - } + })(); + return trackReleaseAttempt(key, attempt); }; - const retryPendingRecoveryLeases = async (): Promise => { - for (const lease of pendingRecoveryLeases.values()) { - if (!isRecoverableProviderAvailable(lease, options.recoverableProviderIds)) continue; - if (!persistedRecoveryLeaseIds.has(lease.leaseId) && !persistPendingLeases()) continue; + const ensureRecoveryReleaseAttempt = (lease: DeviceLease): Promise | undefined => { + if (!isRecoverableProviderAvailable(lease, options.recoverableProviderIds)) return undefined; + if (!persistedRecoveryLeaseIds.has(lease.leaseId) && !persistPendingLeases()) return undefined; + const key = `recovery:${lease.leaseId}`; + const activeAttempt = activeReleaseAttempts.get(key); + if (activeAttempt) return activeAttempt; + const attempt = (async (): Promise => { if (await releaseExpiredProviderLease(options.recoverExpiredLease, lease)) { pendingRecoveryLeases.delete(lease.leaseId); recordReleasedLease(lease); persistPendingLeases(); } - } + })(); + return trackReleaseAttempt(key, attempt); }; - const retryPending = (): Promise => { - if (activeRetry) return activeRetry; - const retry = (async (): Promise => { - try { - await retryPendingLiveLeases(); - await retryPendingRecoveryLeases(); - } finally { - if (pendingLiveLeases.size === 0 && pendingRecoveryLeases.size === 0 && retryTimer) { - clearTimeout(retryTimer); - retryTimer = undefined; - } else { - scheduleRetry(); - } - } - })(); - activeRetry = retry; - void retry.then( - () => { - if (activeRetry === retry) activeRetry = undefined; - }, - () => { - if (activeRetry === retry) activeRetry = undefined; - }, - ); - return retry; + const retryPending = async (): Promise => { + const attempts = [ + ...[...pendingLiveLeases.values()].map(ensureLiveReleaseAttempt), + ...[...pendingRecoveryLeases.values()].map(ensureRecoveryReleaseAttempt), + ].filter((attempt): attempt is Promise => attempt !== undefined); + if (attempts.length === 0) { + scheduleRetry(); + return; + } + await Promise.all(attempts); }; return { @@ -172,7 +180,7 @@ export function createExpiredProviderLeaseReleaser(options: { if (isRecoverableProviderAvailable(lease, options.recoverableProviderIds)) { pendingRecoveryLeases.set(lease.leaseId, lease); if (persistPendingLeases()) { - await retryPending(); + await ensureRecoveryReleaseAttempt(lease); } else { scheduleRetry(); } @@ -181,7 +189,7 @@ export function createExpiredProviderLeaseReleaser(options: { if (!isProviderRuntimeAvailable(lease, options.providerRuntimeIds)) return; if (!options.leaseLifecycleProvider?.release) return; pendingLiveLeases.set(lease.leaseId, lease); - await retryPending(); + await ensureLiveReleaseAttempt(lease); if (pendingLiveLeases.has(lease.leaseId)) { scheduleRetry(); } diff --git a/src/daemon/server/daemon-session-lease-finalizer.test.ts b/src/daemon/server/daemon-session-lease-finalizer.test.ts index 473062b0a..319a840da 100644 --- a/src/daemon/server/daemon-session-lease-finalizer.test.ts +++ b/src/daemon/server/daemon-session-lease-finalizer.test.ts @@ -3,7 +3,7 @@ import os from 'node:os'; import path from 'node:path'; import { expect, test, vi } from 'vitest'; import { makeIosSession } from '../../__tests__/test-utils/session-factories.ts'; -import { LeaseRegistry } from '../lease-registry.ts'; +import { LeaseRegistry, type DeviceLease } from '../lease-registry.ts'; import { createExpiredProviderLeaseReleaser } from '../provider-lease-expiry.ts'; import { finalizeDaemonSessionLease } from './daemon-session-lease-finalizer.ts'; @@ -110,3 +110,70 @@ test('final drain joins a release that completes after the session timeout', asy vi.useRealTimers(); } }); + +test('a hung provider release does not starve a later session during shutdown', async () => { + vi.useFakeTimers(); + const leaseRegistry = new LeaseRegistry(); + const hungLease = leaseRegistry.allocateLease({ + tenantId: 'tenant-a', + runId: 'run-1', + leaseProvider: 'browserstack', + }); + const releasedLease = leaseRegistry.allocateLease({ + tenantId: 'tenant-a', + runId: 'run-2', + leaseProvider: 'browserstack', + }); + const release = vi.fn((lease: DeviceLease) => + lease.leaseId === hungLease.leaseId + ? new Promise>(() => {}) + : Promise.resolve({}), + ); + const expiredProviderLeaseReleaser = createExpiredProviderLeaseReleaser({ + leaseLifecycleProvider: { release }, + providerRuntimeIds: ['browserstack'], + }); + const sessions = [ + makeIosSession('hung', { lease: sessionLease(hungLease) }), + makeIosSession('released', { lease: sessionLease(releasedLease) }), + ]; + + try { + expiredProviderLeaseReleaser.beginShutdown(); + const finalization = Promise.all( + sessions.map( + async (session) => + await finalizeDaemonSessionLease({ + session, + leaseRegistry, + expiredProviderLeaseReleaser, + timeoutMs: 1_000, + }), + ), + ); + + await vi.advanceTimersByTimeAsync(1_000); + await finalization; + + expect(release).toHaveBeenCalledWith(hungLease); + expect(release).toHaveBeenCalledWith(releasedLease); + expect(leaseRegistry.listActiveLeases()).toEqual([]); + const drain = expiredProviderLeaseReleaser.drain(2_000); + await vi.advanceTimersByTimeAsync(2_000); + await expect(drain).resolves.toEqual({ pending: [hungLease], released: [releasedLease] }); + } finally { + expiredProviderLeaseReleaser.shutdown(); + vi.useRealTimers(); + } +}); + +function sessionLease(lease: DeviceLease) { + return { + leaseId: lease.leaseId, + tenantId: lease.tenantId, + runId: lease.runId, + leaseBackend: lease.backend, + leaseProvider: lease.leaseProvider, + expiresAt: lease.expiresAt, + }; +}