Repository navigation
fix: end the agent-device connection and host session dirs of a hosted macOS app - #2527
Conversation
janicduplessis
left a comment
There was a problem hiding this comment.
Review of the diff against origin/main (correctness first). No blocking bugs. Three small confirmed issues and some notes.
Confirmed
-
Driver crash-restart wipes the session directories of sessions that are still live (
packages/server/src/agent-device-driver.ts,stop()).AgentDriverRegistry.lost()inagent-driver.tscallsdriver.stop()after the daemon exits, thenstart()andissue()again for every live app.stop()now sweepsstim.<session>_*for every lease inthis.leases, and those leases are for apps that keep running and get re-issued a moment later. The agent's session loses its request logs (requests/*.ndjson) and repair tombstone across a daemon restart. The same sweep runs on the failed-restartstop()calls inside the retry loop. The issue only asks for removal when the hosted session ends. Sweeping only inrevoke()and in the final stop (when no live app remains) would match that. If you keep it, the restart case should have a test. -
A failed lease DELETE leaves the directories behind for good (
revoke()).this.leases.delete(session)runs first, so ifadminRequest(..., 'DELETE', ...)throws, the linesthis.released.add(session)andthis.removeSessionDirectories(session)are never reached.AgentDriverRegistry.drop()catches and logs the error. After that the session is in neitherleasesnorreleased, so evenstop()will not sweep it. Moving thereleased.addand the removal into afinallyfixes it. -
The
releasedset has no test. It exists so thatstop()re-sweeps a revoked session after the daemon is gone, in case the daemon recreated the directory in between. Nothing recreates a directory afterrevoke()and then callsstop(), so deletingreleasedwould leave every test green. The newstoptest also does not prove the "after daemon teardown" ordering in its title. Its directories are removed whether the sweep runs before or after teardown. A test that recreatesstim.<session>_defaultafterrevoke()and beforestop()covers both.
Notes (low, no action required)
agent-deviceleaves.adreplay scripts as files directly insessions/(<sanitized session>-<timestamp>.ad, from the installed 0.21.12 dist,resolveScriptPath).removeSessionDirectoriesonly removes directories (entry.isDirectory()), so a recorded script for a hosted session would stay. I did not confirm that a hostedmacos-applease ever records one, so this may not apply.- Docs (
guide/macos.ts,website/docs/macos.md) say the connection that "agent-device reports as connected to that remote config" is closed. In practiceconnection statuswith no--sessionreports one connection only (the default or active session). A connection under another--sessionname that was made with this config is left alone. The cleanup is also skipped silently whenagent-deviceis not on PATH. One clause on each would make the docs exact. "When the hosted session ends" is really "when the app stops or the driver stops". - In
hosted-macos-client.test.tstheexpect(...)calls inside the mockedrunFilethrow intocloseAgentConnection's own catch. A failing ordering assertion is swallowed and shows up only indirectly, throughcallsor stderr. This works, but those checks are weaker than they look.
Dismissed
- Touching another connection or another session's directory. The sweep matches
readdirSyncentry names againststim.${session}_and joins them to the sessions dir, so no path from the caller is used and traversal is impossible. Session ids come fromrandomUUID()(device-host.ts) and a UUID has no_, sostim.<uuid>_cannot prefix another session. On the client, the realpath comparison of the workspace's config with the statusremoteConfiglimits action to that config. A missing config file, a symlink and a different config are covered by tests. - Failure escaping
stop. All three agent-device calls, the executable lookup and the JSON parsing sit inside one try/catch incloseAgentConnection, and each ofcloseanddisconnecthas its own catch, sodisconnectstill runs whenclosefails. I ranagent-device0.21.12 against a throwaway--state-dir.connection status --jsonanddisconnect --jsonreturnsuccess: true.close --jsonfor an absent session exits 1 withsuccess: false, which the code handles. - Order of close then host stop. The remote config still exists when
closeruns, and if the host stop fails afterwards a retry finds the connection already gone and does nothing. - Races in
revoke/issue.issue()revokes the same session first, which is correct for a new pid.revokeawaits the in-flight renewal before the DELETE, as before. - Blocking
runFile. At most 30 s of sync wait instop; acceptable since it is best effort and only runs when a connection matches. - Comment and ASCII policy. The diff adds no comments and no non-ASCII characters in sources or tests. The
child_processrule is respected because the new code goes throughgetExecutor().runFile.
Description
After
stim stopof a hosted macOS app, the client's agent-device connection stays bound to that app's remote config, so the next hosted workspace's--remote-configfails with "A different remote connection is already active" untilagent-device disconnectruns by hand. On the hosting Mac, each hosted session also leaves~/.stim/server/agent-device/sessions/stim.<session>_*directories behind.Solution
Client:
stopHostedMacos(reached bystop,worktree removeand gc throughstopMacosApp) now ends the agent-device connection before it stops the host session, only when the placement's agent driver isagent-device. It runsagent-device connection status --jsonand acts only when agent-device reportsconnectedwith aremoteConfigwhose realpath equals this workspace's config. It then runsclose --remote-config <file> --session <reported session>anddisconnect --session <reported session>, so a connection to anything else (another hosted app, an EAS simulator) is never touched. The calls go throughexec.ts, use the sameagent-devicelookup as the device-teardown cleanup, are bounded to 30 s, and are best effort: failures print one stderr line and never block the stop. This runs before the host session stops becausecloseneeds the host's proxy and the config file.Server:
AgentDeviceDriver.revokeremoves the session's agent-device directories (<state>/agent-device/sessions/stim.<session>_*) once the lease is released (also when the lease DELETE fails), andstopremoves the revoked sessions' directories again once the daemon is gone. Live sessions' directories survive a daemon restart, and nothing else undersessions/is touched.stim guide macosandwebsite/docs/macos.mddocument both behaviours.Test plan
pnpm run format:check,lint,build,typecheck,knippass.pnpm testonhosted-macos-client,guide,macos,agent-device-driverandagent-driver: 143 tests pass. New cases cover the matching connection (status, close, disconnect in order, config file and host session still present), a different or disconnected connection (nothing run), a symlinked config path, no agent-device on PATH, a failingclose(disconnect and stop still run), and the server directory removal (targetstim.<S>_*dirs gone;sessions/defaultand another session's dir kept; a revoked session's directory recreated beforestopis removed after the daemon exits while a live session's is kept).agent-device0.21.20macos-applease build (feat(remote): add a host-allocated macos-app lease backend callstack/agent-device#3236) and the Mac mini host. A fixture app was hosted twice in a row from two workspaces;stopprinted "closed agent-device connection for default",agent-device connection status --jsonthen reportedconnected: false, and the second workspace'sopen --remote-configworked without a manual disconnect. The server part is verified after deploying this merged commit to the mini (result in a follow-up comment).Fixes #2522
The code was written by Codex gpt-6.1-sol.