From ea83de120e0a9c762f33ce05f467b41590bd07ce Mon Sep 17 00:00:00 2001 From: Janic Duplessis Date: Mon, 5 Oct 2026 15:10:48 -0400 Subject: [PATCH 1/2] fix: end the agent-device connection and host session dirs of a hosted macOS app --- .../__tests__/agent-device-driver.test.ts | 41 ++++++++++ packages/server/src/agent-device-driver.ts | 29 ++++++- .../src/__tests__/hosted-macos-client.test.ts | 81 +++++++++++++++++++ .../src/device-host/agent-connection.ts | 58 +++++++++++++ .../stim-cli/src/device-host/hosted-macos.ts | 2 + packages/stim-cli/src/guide/macos.ts | 8 +- website/docs/macos.md | 8 +- 7 files changed, 223 insertions(+), 4 deletions(-) create mode 100644 packages/stim-cli/src/device-host/agent-connection.ts diff --git a/packages/server/__tests__/agent-device-driver.test.ts b/packages/server/__tests__/agent-device-driver.test.ts index 2827beeb8..6d05f3bd4 100644 --- a/packages/server/__tests__/agent-device-driver.test.ts +++ b/packages/server/__tests__/agent-device-driver.test.ts @@ -1,5 +1,6 @@ import { chmodSync, + existsSync, mkdirSync, mkdtempSync, readFileSync, @@ -188,6 +189,13 @@ describe.skipIf(process.platform === 'win32')('agent-device driver', () => { const driver = driverIn(root, { leaseRenewMs: 50 }); await driver.start(); try { + const sessionsDir = join(root, 'state', 'sessions'); + const removed = [`stim.${SESSION}_default`, `stim.${SESSION}_other`]; + const kept = ['default', 'stim.other_default']; + for (const name of [...removed, ...kept]) { + mkdirSync(join(sessionsDir, name, 'requests'), { recursive: true }); + writeFileSync(join(sessionsDir, name, 'session.json'), '{}'); + } const grant = await driver.issue({ client: 'c', session: SESSION, @@ -214,6 +222,8 @@ describe.skipIf(process.platform === 'win32')('agent-device driver', () => { }); await vi.waitFor(() => expect(calls().filter((call) => call.method === 'PUT').length).toBeGreaterThan(2)); await driver.revoke(SESSION); + for (const name of removed) expect(existsSync(join(sessionsDir, name))).toBe(false); + for (const name of kept) expect(existsSync(join(sessionsDir, name, 'session.json'))).toBe(true); expect(calls().at(-1)).toMatchObject({ method: 'DELETE', url: `/admin/leases/${grant.scope}` }); const after = calls().length; await new Promise((resolve) => setTimeout(resolve, 200)); @@ -224,6 +234,37 @@ describe.skipIf(process.platform === 'win32')('agent-device driver', () => { } }); + test.each([false, true])('stop removes leased sessions after daemon teardown (crashed: %s)', async (crashed) => { + install(root); + const driver = driverIn(root); + await driver.start(); + const daemonPid = (JSON.parse(readFileSync(join(root, 'state', 'daemon.json'), 'utf8')) as { pid: number }).pid; + const identity = captureProcessIdentity(daemonPid); + const sessionsDir = join(root, 'state', 'sessions'); + const sessions = [SESSION, '22222222-2222-4222-8222-222222222222']; + try { + for (const session of sessions) { + await driver.issue({ client: 'c', session, bundleId: 'dev.example.app.hosted1', pid: 4242 }); + mkdirSync(join(sessionsDir, `stim.${session}_default`, 'requests'), { recursive: true }); + writeFileSync(join(sessionsDir, `stim.${session}_default`, 'session.json'), '{}'); + } + mkdirSync(join(sessionsDir, 'default'), { recursive: true }); + mkdirSync(join(sessionsDir, 'stim.other_default')); + if (crashed) { + const exited = new Promise((resolve) => driver.onExit(resolve)); + process.kill(daemonPid, 'SIGKILL'); + await exited; + } + await driver.stop(); + expect(inspectProcessIdentity({ pid: daemonPid, processToken: identity.ok ? identity.token : '' })).toBe('gone'); + for (const session of sessions) expect(existsSync(join(sessionsDir, `stim.${session}_default`))).toBe(false); + expect(existsSync(join(sessionsDir, 'default'))).toBe(true); + expect(existsSync(join(sessionsDir, 'stim.other_default'))).toBe(true); + } finally { + await driver.stop(); + } + }); + test('revoke waits for a renewal in flight so the DELETE is the last admin call', async () => { install(root); const driver = driverIn(root, { leaseRenewMs: 300 }, { FAKE_ADMIN_PUT_MS: '200' }); diff --git a/packages/server/src/agent-device-driver.ts b/packages/server/src/agent-device-driver.ts index e346feeb2..4923a546d 100644 --- a/packages/server/src/agent-device-driver.ts +++ b/packages/server/src/agent-device-driver.ts @@ -6,6 +6,7 @@ import { constants, mkdirSync, readFileSync, + readdirSync, realpathSync, rmSync, writeFileSync, @@ -177,6 +178,7 @@ export class AgentDeviceDriver implements HostedAgentDriver { private daemonRecord: ProcessRecord | null = null; private listener: (() => void) | null = null; private readonly leases = new Map(); + private readonly released = new Set(); constructor(options: AgentDeviceDriverOptions) { this.options = { @@ -342,13 +344,15 @@ export class AgentDeviceDriver implements HostedAgentDriver { async stop(): Promise { this.stopping = true; await this.starting?.catch(() => undefined); + const sessions = [...this.leases.keys(), ...this.released]; + this.released.clear(); for (const lease of this.leases.values()) clearInterval(lease.renew); this.leases.clear(); const running = this.running; - if (!running && !this.claim) return; this.running = null; if (running) clearInterval(running.watch); - await this.teardown(running?.proxy, running ?? undefined); + if (running || this.claim) await this.teardown(running?.proxy, running ?? undefined); + for (const session of sessions) this.removeSessionDirectories(session); } private async teardown(proxy?: ChildProcess, running?: Running): Promise { @@ -438,6 +442,27 @@ export class AgentDeviceDriver implements HostedAgentDriver { clearInterval(lease.renew); await lease.renewing; if (this.running) await adminRequest(this.running.admin, 'DELETE', lease.id); + this.released.add(session); + this.removeSessionDirectories(session); + } + + private removeSessionDirectories(session: string): void { + const sessionsDir = join(this.options.stateDir, 'sessions'); + try { + for (const entry of readdirSync(sessionsDir, { withFileTypes: true })) { + if (!entry.isDirectory() || !entry.name.startsWith(`stim.${session}_`)) continue; + try { + rmSync(join(sessionsDir, entry.name), { recursive: true, force: true }); + } catch (error) { + process.stderr.write( + `Hosted agent session cleanup failed: ${(error as Error).message.replace(/\s+/g, ' ')}\n`, + ); + } + } + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') + process.stderr.write(`Hosted agent session cleanup failed: ${(error as Error).message.replace(/\s+/g, ' ')}\n`); + } } forward(session: string, request: IncomingMessage, response: ServerResponse): void { diff --git a/packages/stim-cli/src/__tests__/hosted-macos-client.test.ts b/packages/stim-cli/src/__tests__/hosted-macos-client.test.ts index ce890dfda..c10326c95 100644 --- a/packages/stim-cli/src/__tests__/hosted-macos-client.test.ts +++ b/packages/stim-cli/src/__tests__/hosted-macos-client.test.ts @@ -184,6 +184,7 @@ beforeEach(async () => { const real = getExecutor(); setExecutor({ ...real, + findExecutable: (name: string) => (name === 'agent-device' ? null : real.findExecutable(name)), spawn: (cmd: string, args: string[], opts: object) => { spawned.push(cmd); if (cmd !== 'swift') throw new Error(`unexpected spawn ${cmd}`); @@ -262,13 +263,93 @@ describe.skipIf(process.platform !== 'darwin')('stim macos --host (SwiftPM and c expect(host.methods.filter((method) => method === 'device-host.reserve')).toHaveLength(1); expect(host.offers.at(-1)!.attempt).not.toBe(host.offers[0]!.attempt); + const calls: string[][] = []; + setExecutor({ + ...getExecutor(), + findExecutable: () => '/fake/agent-device', + runFile: (file: string, commandArgs: string[]) => { + expect(file).toBe('agent-device'); + expect(existsSync(config)).toBe(true); + expect(host.methods).not.toContain('device-host.stop'); + calls.push(commandArgs); + return JSON.stringify({ success: true, data: { connected: true, session: 'default', remoteConfig: config } }); + }, + }); await stopMacosApp(root); + expect(calls).toEqual([ + ['connection', 'status', '--json'], + ['close', '--remote-config', config, '--session', 'default', '--json'], + ['disconnect', '--session', 'default', '--json'], + ]); expect(host.methods).toContain('device-host.stop'); expect(existsSync(config)).toBe(false); expect(readMacosRecord(root)?.host).toBeUndefined(); expect(macosAppState(readMacosRecord(root))?.state).toBe('stopped'); }); + test.each(['different config', 'disconnected', 'unavailable', 'close fails', 'canonical config', 'missing config'])( + 'stop cleans up only its own agent connection, best effort: %s', + async (scenario) => { + host.grant = GRANT; + await runMacos(root, () => {}, 'mini'); + const config = agentRemoteConfig(root); + const otherConfig = join(dir, 'other-config.json'); + if (scenario === 'canonical config') symlinkSync(config, otherConfig); + else writeFileSync(otherConfig, '{}'); + if (scenario === 'missing config') rmSync(config); + const calls: string[][] = []; + setExecutor({ + ...getExecutor(), + findExecutable: () => (scenario === 'unavailable' ? null : '/fake/agent-device'), + runFile: (file: string, args: string[]) => { + expect(file).toBe('agent-device'); + expect(host.methods).not.toContain('device-host.stop'); + expect(existsSync(config)).toBe(scenario !== 'missing config'); + calls.push(args); + if (scenario === 'close fails' && args[0] === 'close') throw new Error('app already exited\nclose failed'); + return JSON.stringify({ + success: true, + data: { + connected: scenario !== 'disconnected', + session: 'named-session', + remoteConfig: ['different config', 'canonical config'].includes(scenario) ? otherConfig : config, + }, + }); + }, + }); + const stderr = vi.spyOn(process.stderr, 'write').mockImplementation(() => true); + try { + await stopMacosApp(root); + expect(calls).toEqual( + scenario === 'unavailable' + ? [] + : [ + ['connection', 'status', '--json'], + ...(['close fails', 'canonical config'].includes(scenario) + ? [ + ['close', '--remote-config', config, '--session', 'named-session', '--json'], + ['disconnect', '--session', 'named-session', '--json'], + ] + : []), + ], + ); + expect(stderr).toHaveBeenCalledTimes(['close fails', 'canonical config'].includes(scenario) ? 1 : 0); + expect(stderr.mock.calls.map(([line]) => line).join('')).toMatch( + scenario === 'close fails' + ? /app already exited close failed[^\n]*\n$/ + : scenario === 'canonical config' + ? /closed agent-device connection for named-session/ + : /^$/, + ); + expect(host.methods).toContain('device-host.stop'); + expect(existsSync(config)).toBe(false); + expect(readMacosRecord(root)?.host).toBeUndefined(); + } finally { + stderr.mockRestore(); + } + }, + ); + test('over-limit macos.arguments refuse before the host reserves a session', async () => { writeFileSync( join(root, '.stim.json'), diff --git a/packages/stim-cli/src/device-host/agent-connection.ts b/packages/stim-cli/src/device-host/agent-connection.ts new file mode 100644 index 000000000..6ec69aa38 --- /dev/null +++ b/packages/stim-cli/src/device-host/agent-connection.ts @@ -0,0 +1,58 @@ +import { realpathSync } from 'node:fs'; +import { phaseLine } from '../command-output.ts'; +import { getExecutor } from '../exec.ts'; + +function canonical(path: string): string | null { + try { + return realpathSync(path); + } catch { + return null; + } +} + +function report(message: string): void { + process.stderr.write(`${phaseLine('device', message.replace(/\s+/g, ' '))}\n`); +} + +export function closeAgentConnection(remoteConfig: string): void { + const exec = getExecutor(); + const deadline = Date.now() + 30_000; + const run = (args: string[]) => { + const remaining = deadline - Date.now(); + if (remaining <= 0) throw new Error('agent-device cleanup timed out'); + const result = JSON.parse( + exec.runFile('agent-device', [...args, '--json'], { + timeoutMs: Math.min(10_000, remaining), + killSignal: 'SIGKILL', + }), + ); + if (result?.success !== true) throw new Error('agent-device did not confirm the command'); + return result.data; + }; + try { + if (!exec.findExecutable('agent-device')) return; + const status = run(['connection', 'status']); + if (status?.connected !== true || typeof status.session !== 'string' || typeof status.remoteConfig !== 'string') + return; + const config = canonical(remoteConfig); + if (config === null || canonical(status.remoteConfig) !== config) return; + const failures: string[] = []; + for (const args of [ + ['close', '--remote-config', remoteConfig, '--session', status.session], + ['disconnect', '--session', status.session], + ]) { + try { + run(args); + } catch (error) { + failures.push((error as Error).message); + } + } + report( + failures.length + ? `could not clean up agent-device connection for ${status.session}: ${failures.join('; ')}` + : `closed agent-device connection for ${status.session}`, + ); + } catch (error) { + report(`could not clean up agent-device connection: ${(error as Error).message}`); + } +} diff --git a/packages/stim-cli/src/device-host/hosted-macos.ts b/packages/stim-cli/src/device-host/hosted-macos.ts index f46ec2b5e..8a667f0ad 100644 --- a/packages/stim-cli/src/device-host/hosted-macos.ts +++ b/packages/stim-cli/src/device-host/hosted-macos.ts @@ -26,6 +26,7 @@ import { import { macosDir } from '../macos/state.ts'; import { BuildConnection } from '../offload/client.ts'; import { parseMachine, pinnedEndpoint } from '../offload/tailnet.ts'; +import { closeAgentConnection } from './agent-connection.ts'; import { configuredMachines } from './machines.ts'; const CONNECT_TIMEOUT_MS = 10_000; @@ -352,6 +353,7 @@ export async function placeHostedMacos( /** Stops a recorded hosted session and waits until the host confirms it stopped; anything else is a failure. */ export async function stopHostedMacos(root: string, placement: HostedMacosPlacement): Promise { + if (placement.agent.driver === 'agent-device') closeAgentConnection(placement.agent.remoteConfig); const host = await connectHost(placement.machine); try { let session = hostedSession(host, await call(host, 'device-host.stop', { session: placement.session })); diff --git a/packages/stim-cli/src/guide/macos.ts b/packages/stim-cli/src/guide/macos.ts index 5d376ca6f..8e55060ed 100644 --- a/packages/stim-cli/src/guide/macos.ts +++ b/packages/stim-cli/src/guide/macos.ts @@ -191,7 +191,13 @@ how a coding agent drives the app: { driver: "none", setting: "hosting.agentDriver" } until the hosting Mac's owner sets that setting there, or { driver: "agent-device", remoteConfig, command }. remoteConfig is a mode 0600 file in the workspace directory that holds the credential; run the command -it names and never print the file. stop deletes it. Start with agent-device open +it names and never print the file. stop, worktree remove and gc first run +agent-device close and disconnect for the connection that agent-device reports +as connected to that remote config, then delete the file, so the next hosted +workspace needs no manual disconnect. Any other connection stays untouched, and +a failure there is reported without blocking the stop. When the hosted session +ends, stim-server removes its agent-device session directories under its own +state directory. Start with agent-device open --remote-config , using macos.host.bundleId; the lease allows open, close, snapshot, wait, find, get, is, click, fill, press, type, focus, scroll, screenshot and batch on that app only. The agent-device on this Mac must diff --git a/website/docs/macos.md b/website/docs/macos.md index 2c3ba8d38..342dcc297 100644 --- a/website/docs/macos.md +++ b/website/docs/macos.md @@ -205,7 +205,13 @@ confirm a live app) and does not contact the host. `agent` is owner turns on a driver with that setting. With `agent-device`, it names a `remoteConfig` file (mode 0600, in the workspace directory) and the `command` to run, such as `agent-device screenshot --remote-config `. The credential stays -in that file and never appears in command output. Start with +in that file and never appears in command output. `stop`, `worktree remove` and +`gc` first run `agent-device close` and `disconnect` for the connection that +agent-device reports as connected to that remote config, then delete the file, so +the next hosted workspace needs no manual disconnect. Any other connection stays +untouched, and a failure there is reported without blocking the stop. When the +hosted session ends, stim-server removes its agent-device session directories +under its own state directory. Start with `agent-device open --remote-config `; the lease allows only commands that drive that one app (`snapshot`, `click`, `fill`, `type`, `press`, `scroll`, `screenshot` and similar), and the agent-device on the client needs the From 3acce25a339ceeff5b8a38e7166ed5799037b3f4 Mon Sep 17 00:00:00 2001 From: Janic Duplessis Date: Mon, 5 Oct 2026 15:16:37 -0400 Subject: [PATCH 2/2] fix: keep live session dirs on driver restart and sweep after a failed lease delete --- .../__tests__/agent-device-driver.test.ts | 24 +++++++------------ packages/server/src/agent-device-driver.ts | 13 ++++++---- packages/stim-cli/src/guide/macos.ts | 11 +++++---- website/docs/macos.md | 11 +++++---- 4 files changed, 29 insertions(+), 30 deletions(-) diff --git a/packages/server/__tests__/agent-device-driver.test.ts b/packages/server/__tests__/agent-device-driver.test.ts index 6d05f3bd4..439444297 100644 --- a/packages/server/__tests__/agent-device-driver.test.ts +++ b/packages/server/__tests__/agent-device-driver.test.ts @@ -234,32 +234,26 @@ describe.skipIf(process.platform === 'win32')('agent-device driver', () => { } }); - test.each([false, true])('stop removes leased sessions after daemon teardown (crashed: %s)', async (crashed) => { + test('stop removes the directories of revoked sessions after the daemon is gone and keeps live ones', async () => { install(root); const driver = driverIn(root); await driver.start(); const daemonPid = (JSON.parse(readFileSync(join(root, 'state', 'daemon.json'), 'utf8')) as { pid: number }).pid; const identity = captureProcessIdentity(daemonPid); const sessionsDir = join(root, 'state', 'sessions'); - const sessions = [SESSION, '22222222-2222-4222-8222-222222222222']; + const live = '22222222-2222-4222-8222-222222222222'; + const directory = (session: string) => join(sessionsDir, `stim.${session}_default`); try { - for (const session of sessions) { + for (const session of [SESSION, live]) { await driver.issue({ client: 'c', session, bundleId: 'dev.example.app.hosted1', pid: 4242 }); - mkdirSync(join(sessionsDir, `stim.${session}_default`, 'requests'), { recursive: true }); - writeFileSync(join(sessionsDir, `stim.${session}_default`, 'session.json'), '{}'); - } - mkdirSync(join(sessionsDir, 'default'), { recursive: true }); - mkdirSync(join(sessionsDir, 'stim.other_default')); - if (crashed) { - const exited = new Promise((resolve) => driver.onExit(resolve)); - process.kill(daemonPid, 'SIGKILL'); - await exited; + mkdirSync(directory(session), { recursive: true }); } + await driver.revoke(SESSION); + mkdirSync(directory(SESSION), { recursive: true }); await driver.stop(); expect(inspectProcessIdentity({ pid: daemonPid, processToken: identity.ok ? identity.token : '' })).toBe('gone'); - for (const session of sessions) expect(existsSync(join(sessionsDir, `stim.${session}_default`))).toBe(false); - expect(existsSync(join(sessionsDir, 'default'))).toBe(true); - expect(existsSync(join(sessionsDir, 'stim.other_default'))).toBe(true); + expect(existsSync(directory(SESSION))).toBe(false); + expect(existsSync(directory(live))).toBe(true); } finally { await driver.stop(); } diff --git a/packages/server/src/agent-device-driver.ts b/packages/server/src/agent-device-driver.ts index 4923a546d..f5c7a3600 100644 --- a/packages/server/src/agent-device-driver.ts +++ b/packages/server/src/agent-device-driver.ts @@ -344,7 +344,7 @@ export class AgentDeviceDriver implements HostedAgentDriver { async stop(): Promise { this.stopping = true; await this.starting?.catch(() => undefined); - const sessions = [...this.leases.keys(), ...this.released]; + const sessions = [...this.released]; this.released.clear(); for (const lease of this.leases.values()) clearInterval(lease.renew); this.leases.clear(); @@ -439,11 +439,14 @@ export class AgentDeviceDriver implements HostedAgentDriver { const lease = this.leases.get(session); if (!lease) return; this.leases.delete(session); - clearInterval(lease.renew); - await lease.renewing; - if (this.running) await adminRequest(this.running.admin, 'DELETE', lease.id); this.released.add(session); - this.removeSessionDirectories(session); + clearInterval(lease.renew); + try { + await lease.renewing; + if (this.running) await adminRequest(this.running.admin, 'DELETE', lease.id); + } finally { + this.removeSessionDirectories(session); + } } private removeSessionDirectories(session: string): void { diff --git a/packages/stim-cli/src/guide/macos.ts b/packages/stim-cli/src/guide/macos.ts index 8e55060ed..fa0a7405f 100644 --- a/packages/stim-cli/src/guide/macos.ts +++ b/packages/stim-cli/src/guide/macos.ts @@ -193,11 +193,12 @@ or { driver: "agent-device", remoteConfig, command }. remoteConfig is a mode 0600 file in the workspace directory that holds the credential; run the command it names and never print the file. stop, worktree remove and gc first run agent-device close and disconnect for the connection that agent-device reports -as connected to that remote config, then delete the file, so the next hosted -workspace needs no manual disconnect. Any other connection stays untouched, and -a failure there is reported without blocking the stop. When the hosted session -ends, stim-server removes its agent-device session directories under its own -state directory. Start with agent-device open +as connected to that remote config (the default or active session), then delete +the file, so the next hosted workspace needs no manual disconnect. Any other +connection, including one under another session name, stays untouched, and a +failure or a missing agent-device is reported without blocking the stop. When +the hosted session ends, stim-server removes its agent-device session +directories under its own state directory. Start with agent-device open --remote-config , using macos.host.bundleId; the lease allows open, close, snapshot, wait, find, get, is, click, fill, press, type, focus, scroll, screenshot and batch on that app only. The agent-device on this Mac must diff --git a/website/docs/macos.md b/website/docs/macos.md index 342dcc297..2c185794c 100644 --- a/website/docs/macos.md +++ b/website/docs/macos.md @@ -207,11 +207,12 @@ owner turns on a driver with that setting. With `agent-device`, it names a run, such as `agent-device screenshot --remote-config `. The credential stays in that file and never appears in command output. `stop`, `worktree remove` and `gc` first run `agent-device close` and `disconnect` for the connection that -agent-device reports as connected to that remote config, then delete the file, so -the next hosted workspace needs no manual disconnect. Any other connection stays -untouched, and a failure there is reported without blocking the stop. When the -hosted session ends, stim-server removes its agent-device session directories -under its own state directory. Start with +agent-device reports as connected to that remote config (the default or active +session), then delete the file, so the next hosted workspace needs no manual +disconnect. Any other connection, including one under another session name, stays +untouched, and a failure or a missing agent-device is reported without blocking +the stop. When the hosted session ends, stim-server removes its agent-device +session directories under its own state directory. Start with `agent-device open --remote-config `; the lease allows only commands that drive that one app (`snapshot`, `click`, `fill`, `type`, `press`, `scroll`, `screenshot` and similar), and the agent-device on the client needs the