Files
agentic-mobile-control/openspec/changes/host-agent-single-instance-lock/design.md
T
q792602257andClaude Opus 4.6 82567fd248
Tests / Test passed: 759
chore(openspec): add host-agent-single-instance-lock change artifacts
Proposal, design, spec, and tasks for the per-installation exclusive
instance lock. 15/16 tasks complete; only manual real-environment
verification (5.4) remains, with semantics covered by unit tests in
test_app.py and test_instance_lock.py.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-07-14 15:49:45 +08:00

12 KiB

Context

  • identity_path (host_agent/config.py:22, default Path("tasks/host_identity.json"), overridable via HOST_AGENT_IDENTITY_PATH) is persisted through HostIdentityStore (host_agent/identity.py). Its parent directory is already the de facto per-installation state directory: create_application() derives host_console_history.sqlite3 (app.py:178) and host_governance_policy.json (app.py:229) from resolved_config.identity_path.parent.
  • create_application() (app.py:145-172) is fully synchronous and front-loads every side-effecting startup call: resolve_host_identity() (may enroll a new host with the control plane) then _configured_device_manager() (enrolls every locally configured device). Both are network calls against the same control plane the claim/heartbeat loops later use.
  • A repo-wide search confirms no pidfile, lockfile, or process-uniqueness mechanism exists anywhere in apps/device-host-agent today.
  • This repo has no CI configuration (no .github/workflows); tests are run locally by whichever machine the developer/operator uses. The primary development machine observed in this session is Windows; the documented production target is macOS (docs/MACOS_IPHONE_SETUP.md). Any locking mechanism must be correct on both without a CI matrix to catch a platform-specific regression.
  • filelock 3.29.7 is already present in uv.lock as a transitive dependency (pulled in by unrelated ML-related packages), but is not a direct dependency of any workspace member today.
  • The recently archived host-agent-dependency-supervisor (apps/device-host-agent/host_agent/dependency_supervisor.py) spawns Appium/Runtime child processes via subprocess.Popen and is architecturally adjacent — also concerned with Host Agent process lifecycle. Its design Decision 5 explicitly chose stdlib-only subprocess.Popen over adding a dependency for spawning. That precedent is weighed below (Decision 2) but not followed as-is.
  • apps/device-host-agent/tests/test_app.py was audited for existing create_application() call patterns: all 9 call sites are one-per-test-function (no test constructs two applications against the same identity_path in one process today), and each uses a pytest tmp_path-scoped identity path. A new duplicate-instance test therefore needs to keep the first HostAgentApplication's lock handle alive across the assertion — see Decision 5.

Goals / Non-Goals

Goals:

  • Guarantee at most one Host Agent process is ever actively participating in the claim/heartbeat protocol for a given identity_path state directory at a time.
  • Fail fast and loudly when a duplicate instance is detected, before any control-plane side effect (enrollment, heartbeat, claim) occurs.
  • Require zero manual cleanup after an unclean shutdown (crash, kill -9, power loss).
  • Work correctly on both macOS (production target) and Windows (a real development machine for this repo) with no CI matrix to lean on.

Non-Goals:

  • No distributed/cross-machine locking. This only prevents duplicate local processes for one installation; it does not address two different physical hosts being misconfigured with the same host_id (a separate concern, partially already handled by device-identity-ownership-conflict behavior in device-pool).
  • No change to the claim/heartbeat/lease protocol itself (host-agent-protocol is unmodified).
  • No general-purpose process supervision or auto-restart. This is an exclusivity guard on the Host Agent's own process, not a supervisor of other processes (contrast with host-agent-dependency-supervisor).
  • No reliance on the console_port bind as the exclusivity mechanism — that conflict is real but surfaces too late (deep inside run_async(), after enrollment/heartbeat side effects have already started) and is silently bypassed if two instances ever end up configured with different ports.

Decisions

1. Lock file location: identity_path.parent / "host_agent.lock", no new config field. Matches the existing convention of colocating per-installation auxiliary files next to identity_path (app.py:178,229). A lock scoped to a directory independent of the identity it protects would be a foot-gun if the two were ever configured to diverge, and a dedicated env var would just duplicate information HOST_AGENT_IDENTITY_PATH already carries.

2. Use filelock (new direct dependency) rather than hand-rolled fcntl/msvcrt branching. Alternative considered: a small in-repo module branching on sys.platform, calling fcntl.flock(fd, LOCK_EX | LOCK_NB) on POSIX and msvcrt.locking(fd, LK_NBLCK, 1) on Windows — matching this repo's general preference for avoiding new dependencies (host-agent-dependency-supervisor Decision 5). Rejected in favor of filelock because:

  • This repo has no CI matrix, so a platform-specific bug in whichever branch isn't the implementer's local OS could ship unverified. Concretely, the production target (macOS/fcntl) differs from a development machine already in use for this repo (Windows/msvcrt).
  • filelock is small, pure-Python, has zero dependencies of its own, is already resolved in this repo's uv.lock transitively (so promoting it to a direct dependency of device-host-agent adds no new package to the lock, only a new dependency edge), and is widely used (virtualenv, tox, huggingface_hub all depend on it).
  • Getting advisory locking subtly wrong (a missed non-blocking flag, a probe/acquire race) would reintroduce exactly the class of bug this change exists to eliminate. This is a case where "don't reinvent it" outweighs the general new-dependency-avoidance default — unlike subprocess.Popen, which stdlib already handles with no cross-platform pitfalls.

3. Acquire the lock as the first action inside create_application(), before resolve_host_identity(). startup_config.identity_path is already resolved from config/env at function entry, so this is the earliest point with the information needed, and it precedes every side-effecting call create_application() makes. A duplicate instance is rejected before it can enroll, heartbeat, or claim — not merely before it can serve console traffic.

4. Non-blocking acquisition; fail immediately rather than wait or retry. Alternative considered: block/retry with backoff until the lock frees up, treating a duplicate launch as "wait your turn." Rejected because a Host Agent is normally started by an operator or a process supervisor expecting an immediate up/down signal; silently blocking would look identical to a hang and would mask the actual problem (something didn't clean up the previous instance) instead of surfacing it. filelock.FileLock(path, timeout=0) raises Timeout immediately if already held; create_application() translates this into a clear, named "duplicate instance" error.

5. The lock handle is owned by HostAgentApplication for its full lifetime, released in run_async()'s existing finally block. The acquired filelock.FileLock is stored as a field on the returned HostAgentApplication (not a bare local variable inside create_application()), so it stays held for as long as the object is alive, and is explicitly released during the existing graceful-shutdown finally block in run_async() (app.py:88-111) rather than left to eventual garbage collection. Two consequences:

  • A clean restart doesn't need to wait on GC/OS cleanup timing — the next process can acquire the lock immediately after graceful shutdown completes.
  • A test can hold a first instance's lock open (by keeping its HostAgentApplication/lock reference alive) and assert that a second create_application() call against the same identity_path raises — this is the concrete new test called out in tasks.md.

6. Release also relies on OS-level advisory-lock semantics, not an explicit-unlock-only or pidfile-liveness-check design. Even without the explicit release in Decision 5, the lock is released automatically when the holding process's file descriptor closes for any reason — clean exit, unhandled exception, SIGKILL, crash — because that's a property flock/LockFileEx the kernel enforces, not application code. This sidesteps the entire "stale pidfile after a crash" problem class that a hand-rolled os.path.exists()-based lock or a PID-liveness-check design would otherwise need to solve (including PID-reuse races and cross-platform "is this PID still alive" checks).

  • Subprocess-inheritance note: dependency_supervisor.py spawns Appium/Runtime via subprocess.Popen. Per PEP 446, file descriptors opened by Python (including filelock's underlying os.open) are non-inheritable by default unless explicitly passed via pass_fds, which this change does not do — so a spawned child cannot accidentally keep the lock held after the parent Host Agent process exits. This is confirmed as a non-issue rather than left open, since it follows directly from a documented Python default already in effect for every other file descriptor this codebase opens.

Risks / Trade-offs

  • [Risk] A hung-but-not-dead process holds the lock and blocks a legitimate restart. → Mitigation: intentional, matches Goal 2 ("fail fast and loudly"). A hung process should be diagnosed and killed by the operator/supervisor, not silently bypassed — the point of this change is to stop pretending a second instance is safe to run. The rejection error names the lock file path so an operator can inspect what's holding it (e.g. lsof, Get-Process).
  • [Risk] New direct dependency (filelock). → Mitigation: already resolved in uv.lock transitively today (no new package added to the lock, only a new direct edge), the package itself is small and dependency-free, and is widely used in the Python ecosystem. See Decision 2 for the full rationale on why this outweighs the stdlib-only precedent from host-agent-dependency-supervisor.
  • [Risk] This addresses the symptom (duplicate processes racing) rather than a confirmed single trigger. → Accepted: the incident diagnosis could not confirm, without access to the affected Mac mini, whether the trigger was a deploy-triggered restart race, an orphaned process, or operator error. This change makes all three safe by construction rather than requiring the exact trigger to be identified first. If host process history later points to a specific deploy-tooling bug (e.g. a supervisor not waiting for the old process to exit), that would be a separate, complementary fix outside this change's scope.
  • [Trade-off] Lock scope is per-identity_path, not machine-global. → Accepted per Non-Goals: a machine could legitimately run multiple independent installations pointed at different identity paths; this is also simpler to test, since parallel test runs never share lock state unless they explicitly share a path.

Migration Plan

  • Fully additive: existing single-instance deployments are unaffected beyond a new lock file appearing next to host_identity.json (inside the already-untracked tasks/-style state directory).
  • No configuration changes required to adopt — the lock is unconditional, not opt-in/opt-out, since there is no legitimate scenario where running two instances against the same identity is desirable.
  • Rollback: revert the code change; there is no persistent state or schema to unwind (the lock file is inert once the code no longer checks it).

Open Questions

  • Should a rejected duplicate-launch attempt also be surfaced somewhere operator-visible beyond process logs (e.g. recorded by the console-owning first instance via ConsoleHistoryStore)? Left for a follow-up if log-only proves insufficient in practice — the rejected instance itself never starts a console to display it from, so this would require the surviving instance to detect and record the contention, which is more than this change's scope requires.