Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
ee7850c
docs(dev-plan): add reviewed plan for stoppable orphan sessions
vr000m Jun 19, 2026
ad5d7cc
feat(cli): add identity-checked `onoats stop` (SIGTERM) subcommand
vr000m Jun 20, 2026
b10c079
refactor(dual): extract shutdown tail to `_finalize_shutdown_status`
vr000m Jun 20, 2026
f245f71
docs(dev-plan): record orphan-recovery flows + stop→start race constr…
vr000m Jun 20, 2026
5fb4e0f
fix(runtime): close stop→immediate-start pid-file race
vr000m Jun 20, 2026
ff12681
feat(menubar): Phase 2 — Stop works for orphaned/external sessions
vr000m Jun 20, 2026
e6fbbe4
docs: record Phase 2 + pid-race fix (Codex review follow-up)
vr000m Jun 20, 2026
5227f7d
fix(runtime): refuse start over an indeterminate-but-live recorder
vr000m Jun 20, 2026
ab5e763
fix(menubar): re-enable Stop when external `onoats stop` exits non-zero
vr000m Jun 20, 2026
2a66d4a
docs: record Codex re-review round-2 fixes
vr000m Jun 20, 2026
85b9556
docs: deep-review polish (cross-refs, README clarity, plan split)
vr000m Jun 20, 2026
b272ba4
docs(dev-plan): record on-device orphan-then-stop smoke PASS
vr000m Jun 22, 2026
5572e75
fix(runtime): close residual pid-file deletion window (atomic write +…
vr000m Jun 22, 2026
7713cae
fix(runtime): atomic flock single-instance lock (close concurrent-sta…
vr000m Jun 22, 2026
9968bbf
fix(runtime): hoist single-instance lock before capturer spawn / devi…
vr000m Jun 22, 2026
8d30016
fix(runtime): bot-single lock parity + compare-and-unlink stale cleanup
vr000m Jun 22, 2026
7a7d028
fix: legacy-recorder identity preflight in lock + Stop-button wedge
vr000m Jun 22, 2026
fcb30b3
fix(menubar): clear Stop latch on same-pid recycle (start-epoch discr…
vr000m Jun 24, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,4 @@ dist/
*.db
secrets.env
.deep-review/
.review-plan/
77 changes: 77 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,83 @@ registry dep, not a git pin).
`dual.main` and the supervisor route through it, so interactive/category
handling can't drift.

## Identity-checked signalling (`onoats flush` / `onoats stop`)

Both CLI signal subcommands share **one** identity gate — `resolve_flush_target`
(`_vendor/pid.py`) — and differ **only** in the signal sent:

- `onoats flush` → **SIGUSR1** (continuation flush: rotate buffer, keep recording).
- `onoats stop` → **SIGTERM** (graceful shutdown: drain + final flush, then exit;
same trigger as a single Ctrl-C and the menu bar's owned `Process.terminate()`).

Load-bearing invariants (pinned by `tests/test_cli.py`):

- **Never signal an unverified or recycled pid.** The resolver validates the
marker, requires a cmdline fingerprint, probes liveness (`kill(0)`), and
compares the live `ps` cmdline against the stored one. Only a fully-verified
pid is signalled; unlink **only** when `stale=True`, and even then
**compare-and-unlink** (`_compare_and_unlink_stale_pid` → `_remove_pid_file`
ownership check) — never blindly, or a fresh recorder that won the lock and wrote
its pid in the resolve→cleanup window would be deleted; treat `ProcessLookupError`
at signal time (TOCTOU) as stale. This matters **more** for `stop` than `flush`
— SIGTERM kills by default, so a blind signal to a recycled foreign pid would
terminate an unrelated process. A differential test asserts `stop` sends
SIGTERM-not-SIGUSR1 and `flush` sends SIGUSR1-not-SIGTERM, so a copy-paste
signal swap fails.
- **`stop` is a near-clone of `flush`, not a refactor.** Drift is pinned by tests
rather than a shared helper, keeping the shipped `flush` path untouched.
- Both return on **signal delivery**, not confirmed exit — a consumer must derive
"stopped" from the process actually exiting, never from the CLI exit code.
- **`onoats stop --help` resolves without booting a service** (local argparse +
lazy resolver import), like `flush`.

Single-instance + pid-file ownership (`runtime.py`, pinned by
`tests/test_status_file.py` + `tests/test_socket_supervisor.py`):

- **Start is gated by an atomic `flock` single-instance lock, acquired before any
capture side effect.** `_acquire_instance_lock` takes an exclusive
`flock(LOCK_EX|LOCK_NB)` on `.active/onoats.lock`. It is hoisted to the EARLIEST
point in each entrypoint — the socket supervisor takes it *before spawning the
capturer* (`_supervise_socket_session`), `run_onoats_dual` *before opening
PortAudio*, and `run_onoats` (`bot-single` / `python -m onoats`) *before even
importing the native deps* — so a losing concurrent start raises
`RecorderAlreadyRunningError` (clean rc=1 at all CLI boundaries) **before** it
touches CoreAudio/TCC/a device, never after. Acquisition is **idempotent**
(already-held → no-op), so the later `_write_pid_file` call (a backstop) is a
no-op in the hoisted paths. This is the primary gate and the only *atomic* one: of N
racing starts exactly one wins. The lock is held for the **whole process
lifetime**; the kernel releases it on exit (graceful OR crash/SIGKILL) — there is
no stale lock to reclaim and **no teardown release call** (releasing during
shutdown would free the slot while the supervisor is still tearing down its
capturer). POSIX-only (no-op on Windows; macOS product).
- **Identity preflight is the secondary guard, and it runs INSIDE the lock.**
`_acquire_instance_lock` calls `_refuse_if_live_recorder` (`resolve_flush_target`
+ the indeterminate-but-live refusal) immediately after taking the `flock`, so
both guards fire at the same hoisted, before-capture point. This catches a live
legacy/cross-version recorder that holds no `flock` (an older build) — without
it, such a start would acquire the `flock` and spawn the capturer before
refusing late in `_write_pid_file`. A stale/recycled/foreign pid does NOT block a
legitimate start. The `flock` catches concurrent same-version starts the
read-then-act identity check cannot; the identity preflight catches the legacy
recorder the `flock` cannot. On Windows (no `flock`) the preflight is the only
guard.
- **Pid writes are atomic.** `_write_pid_file` writes to a temp file and
`os.replace`s it into place (same dir → atomic rename), never truncating in
place — mirrors `onoats.status.write_status`. A concurrent reader (a draining
recorder's owner-checked removal) sees either the complete old record or the
complete new one, never an empty/partial file mid-write.
- **Pid removal is ownership-checked and fails closed.**
`_remove_pid_file(pid_path, owner_pid=…)` unlinks **only** when the file still
records exactly that pid. If it reads back as `None` (unreadable/foreign/already
gone) it is left in place — it must never be assumed to be our own benign
mid-write. Because `stop` returns on signal delivery (not exit), a
`stop`-then-immediate-`bot` could otherwise let a draining recorder delete a
NEWER recorder's pid file. Recorder teardown passes `owner_pid=os.getpid()`; the
GUI's menu gating (Start only in `.stopped`) already prevents this from the app,
so the guard protects the CLI/scripted path. A leftover invalid pid file is
self-healing: `status` reports no valid recorder and the next start atomically
replaces it.

## Reviewing a subprocess / process-boundary change

When a change spawns a child process (`create_subprocess_*` / `Popen` / `exec`)
Expand Down
46 changes: 46 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,52 @@ no backdated tags exist). PR numbers `#1`–`#7` refer to this repository;
older history predates the extraction and is cited by merge-commit SHA.
Annotated tags exist from `v0.9.0` forward.

## [Unreleased]

### Added
- `onoats stop` subcommand: signal the running recorder to stop gracefully
(SIGTERM → drain + final flush, then exit). It is a behavioural twin of
`onoats flush` and reuses the same identity gate (`resolve_flush_target`:
marker + cmdline-fingerprint + liveness), so it only ever signals the verified
recorder and never a recycled pid — which matters more than for flush because
SIGTERM kills by default. Returns on signal delivery, not confirmed exit; like
flush, `onoats stop --help` resolves without booting a service.
- Menu-bar **Stop now works for orphaned/external sessions**: a GUI-started
recorder orphaned by an app crash (seen as `running(ours: false)` on relaunch),
or any terminal-started session, can be stopped from the menu. Owned sessions
keep the in-handle `Process.terminate()`; verified external sessions route
through `onoats stop`. The menu shows "stopping (draining)…" for the whole
drain and flips to Stopped only when the supervisor actually exits (polled),
never faking a terminal state. (Completes the stoppable-orphan-session fix.)

### Fixed
- **Stop→immediate-start pid-file race.** `onoats stop` returns on signal
delivery, not exit, so a new `onoats bot` launched during the old recorder's
drain could overwrite the draining recorder's pid file — and the drainer would
then unlink the *new* recorder's file, leaving it invisible to
`status`/`stop`/`flush`. Guards close this: (1) an **atomic `flock`
single-instance lock** acquired before any capture side effect — the socket
supervisor takes an exclusive `flock(LOCK_EX|LOCK_NB)` on `.active/onoats.lock`
*before spawning the capturer*, `run_onoats_dual` *before opening PortAudio*, and
`run_onoats` (`bot-single` / `python -m onoats`) *before importing the native
deps*, so of N racing starts exactly one wins and the rest raise
`RecorderAlreadyRunningError` **before touching CoreAudio/TCC/a device**; held
for the whole process lifetime and released by the kernel on exit (graceful or
crash), so there is no stale lock to reclaim, and a chained `onoats stop &&
onoats bot` cleanly refuses until the drainer's process exits; (2) the identity
check (`resolve_flush_target`) remains as a secondary guard refusing a verified
or indeterminate-but-live recorder (legacy/cross-version), never blocking on a
stale/recycled/foreign pid; (3) pid-file writes are atomic (temp + `os.replace`,
never a truncating in-place write) so a concurrent reader never sees an
empty/partial file; (4) pid-file removal is ownership-checked and fails closed —
a recorder unlinks only a file that still records *its own* pid, and leaves an
unreadable/foreign record in place rather than deleting a newer recorder's
(possibly in-progress) file; (5) `stop`/`flush` stale cleanup is
**compare-and-unlink** (not a blind `unlink`) so a fresh recorder that won the
lock and wrote its pid in the resolve→cleanup window is never deleted. The menu's external Stop also re-enables itself if
the `onoats stop` subprocess fails or exits non-zero (e.g. a stale installed
CLI), rather than wedging the only Stop control until app restart.

## [1.1.0] - 2026-06-12

First PyPI release (`pip install onoats` / `uv tool install onoats`).
Expand Down
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,10 @@ Other subcommands:
```bash
onoats bot-single # legacy mic-only recorder
onoats flush # tell the running recorder to rotate its buffer now
onoats stop # stop the running recorder gracefully: SIGTERM → drain +
# final flush, then EXIT (unlike flush, which keeps
# recording). Identity-checked like flush, so it only ever
# signals the verified recorder — never a recycled pid
onoats devices # list audio input/output devices (PortAudio's view; under
# the socket path it adds a note — the native capturer binds
# the system default input / default-output tap instead)
Expand Down
Loading