agentic-mobile-control/openspec/changes/host-agent-single-instance-lock/design.md

## 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.