refactor: reliable process tracking - #3
Merged
Merged
Conversation
Centralises all OS-process concerns — spawning, identity, signalling, and liveness — in a single new module so that registry.py (persistence) and server.py (orchestration) don't each carry their own copies. Public surface: spawn() asyncio subprocess as its own process group leader is_alive(pid) signal-0 liveness check start_time(pid) ps-reported start time (PID-reuse guard) is_same(pid, t) liveness + start-time identity check kill_group(pid) send signal to full process group terminate(proc) SIGTERM → wait 5s → SIGKILL wait_until_dead() poll until pid is gone or timeout
Wire server.py to the new process module instead of carrying its own OS-process helpers: - process.spawn() replaces asyncio.create_subprocess_exec directly; start_new_session=True means kill_group() reaches every child the opencode binary spawns, not just the pid we hold - process.terminate() replaces the local _terminate_process() helper - process.is_alive() replaces registry.is_alive() and _is_process_alive() at every call site - process.kill_group() + process.wait_until_dead() replace the inline os.kill + poll loop in stop(); stop() now kills before deleting the registry entry, so the entry stays discoverable if the process can't be confirmed dead SQLite registry, ServerState, claimed_at, and last_used_at are unchanged — storage format is a separate concern from process handling.
Storage: - One JSON file per server at <key>.json; atomic write via temp-then-rename so readers never see a partial write - Per-key lock file (O_CREAT|O_EXCL) used only for the two operations that read-then-conditionally-write: claim_starting (abandoned-claim reclaim) and delete_if_instance (generation- scoped delete); plain reads and writes are lock-free - schema.py and test_schema.py deleted — no schema, no migrations; missing fields default to None on read (_FIELD_NAMES filter) Generation fencing: - RegistryEntry gains instance_id and pid_start_time fields - delete_if_instance(key, instance_id) replaces bare delete() in stop() and the get_or_start() failure path, so a slow stop() can't delete a replacement server that started in the gap - pid_start_time recorded at spawn; process.is_same() uses it to guard against PID reuse (stop/wait_until_dead now pass it through) Exceptions: - RegistryBusyError added (lock wait timeout) - RegistrySchemaError removed (belonged to the SQLite schema system) ServerState / claimed_at / last_used_at are still present in RegistryEntry — they are removed in the following commits.
touch() / last_used_at exist to support idle-based cleanup, but that cleanup logic doesn't exist yet — the field was written and read but never acted on. Removing it now keeps the data model honest and shrinks the write surface. - RegistryEntry.last_used_at field removed - ServerManager.touch() removed - runtime.py no longer calls touch() after get_or_start() - cli.py cmd_inspect() no longer shows a "Last used" row
Previously server state was a persisted field (ServerState: STARTING / RUNNING / STOPPING / FAILED) that drove a DisplayStatus (6 values) computed from state + liveness + health. Now status is fully derived at read time from two OS observations: - process.is_same(pid, pid_start_time) — is the pid alive and the same process generation we recorded? - /global/health HTTP check — is opencode responding? RuntimeStatus (running / unhealthy / stale) replaces ServerState + DisplayStatus. A pid-less entry (still starting) has status=None. Concrete changes: - RegistryEntry drops the state field entirely - ServerState and DisplayStatus enums removed from server.py - _compute_display_status() removed - ServerStatus.display → ServerStatus.status (RuntimeStatus | None) - cli.py status icons/colors keyed on RuntimeStatus | None - cmd_health() branches on RuntimeStatus instead of DisplayStatus - ServerManager.find() now checks pid is not None instead of state == RUNNING; get_or_start() / _wait_for_ready() likewise
BLE001: add noqa suppression with reason on intentional broad-except sites (health probes, uptime formatting, timestamp parsing — caller cannot know which specific exception network/parse code raises). PLW1510: add check=False to subprocess.run() in process.start_time(). PYI036: tighten __aexit__ signature to type[BaseException] | None, BaseException | None, types.TracebackType | None. ASYNC230 / SIM115: suppress open() in _start() — the file handle is intentionally opened before spawn() and kept for the subprocess lifetime; wrapping it in a context manager would close it immediately.
The manual os.kill(pid,0) + /proc zombie check and the ps-based start_time() were both fragile: - os.kill(pid,0) returns True for zombie processes on Linux, causing stop() to never confirm death and leave registry entries behind - ps -o lstart= is macOS/BSD-only; Linux ps ignores the = suppressor and may produce empty output or behave differently across distros Replace both with psutil: - is_alive() → psutil.Process.is_running() (zombie-aware, all platforms) - start_time() → psutil.Process.create_time() (float, all platforms) subprocess import removed. psutil added to runtime dependencies. types-psutil added to dev dependencies for mypy. pid_start_time field type updated str→float in RegistryEntry.
wait_until_dead() previously polled is_same() every 100ms in an asyncio sleep loop. psutil.wait_procs() blocks at the OS level until the process exits or the timeout elapses — no busy-polling, and it correctly handles the generation check (create_time mismatch) before waiting.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The registry is now a dumb ledger of facts. Status is derived from the OS at read time — never persisted. Servers spawn in their own process group so stops reach their children. Kills are confirmed before entries are removed. A generation ID and recorded process start time close the PID-reuse and slow-stop races. SQLite and its migration system are gone; each server is one JSON file.