Commit 8bbcfd98 by PLN (Algolia)

fix(tray): say what gear is ALREADY UP — and stop opening the wrong Ardour session (#69)

PLN: "i rightclick the perf indicator and see no gear status?"

He was right, and the state had been there all along. launchers.is_running()
existed; _build_menu() just ran ONCE at startup and read only `available`. So a
running Ardour looked identical to a stopped one, and — worse — the safe action
("do nothing, it's already up") looked identical to the dangerous one. Two
SuperColliders is a zombie port 6010 and a silent rig.

Now: a `Gear ▸ 3/6 up` submenu, each row labelled with what it IS —
  ● Pulsar — running     (greyed: launching a second one is never what you meant)
  ○ MIDI Monitor         (clickable)
  ✗ Something — not installed
Glyphs rather than colour, because a tray menu inherits the desktop palette and
this gets read in a dark room seconds before playing. Web tools stay clickable
when running, since clicking them opens a URL rather than spawning a duplicate.
Refreshed on menu-OPEN only, never on the 2 s icon timer: reading gear state walks
/proc, and doing that 30x a minute for a label nobody is looking at is exactly the
per-tick cost this rig keeps getting bitten by.

THREE REAL BUGS FOUND WHILE WIRING IT UP, each worse than the missing label

1. THE TRAY OPENED THE WRONG ARDOUR SESSION. ARDOUR_SESSION pointed at
   "Tidal Multi" — the older ARCHIVE of per-orbit recordings — not "Tidal Live",
   the session that performs and records the stems. Any faders touched there would
   have been the wrong ones. Auditing that same archive as if it were live already
   produced a confidently wrong fader report on 2026-07-28; this was the same
   mixup one layer down, waiting to happen again 6 days before OPAL.

2. IT COULD NEVER HAVE LAUNCHED ARDOUR ANYWAY. Candidates were
   ardour8/7/6/ardour; the installed binary is ardour9 (real exe `ardour-9.2.0`).
   So the entry reported "unavailable" and greyed itself out while Ardour was
   running on the same machine.

3. THE RUNNING CHECK MATCHED THE WHOLE WORLD. `pgrep -f ardour` matched the
   `tidal-ardour-autoroute.sh` helper script AND any shell whose command line
   merely mentioned the word — including the shell I was testing from. So it could
   report Ardour UP while Ardour was DOWN, which is worse than reporting nothing,
   because it is the state you act on. is_running now matches the EXECUTABLE
   basename (`exe`), with full-line matching kept only for interpreted tools where
   argv[0] is `python3` and the identity is the script path.

AND IT NO LONGER FORKS
is_running was one `pgrep` subprocess PER launcher, and snapshot() is called by
the web Bridge's poll as well as the tray. Replaced with a single forkless /proc
scan shared across the whole snapshot: 6 items in 10.7 ms, zero forks, down from
6 forks per refresh. Same lesson as the LED daemon's per-event fork and gig-log's
per-sample sampling — on an audio rig, do not pay a process for a boolean.

Verified live: all six entries now report correctly (Pulsar/Ardour/QjackCtl up,
MIDI Monitor + Foundry + Armada down); perf-tray restarted and active.

TESTS: +11 in test_launchers.py, suite 424 -> 435, all green. Pinned: the
autoroute-script and bare-shell false positives; a version-suffixed binary
(ardour-9.2.0, and a hypothetical ardour-10.0.1) matching; a filename ending in
".ardour" NOT counting as a running Ardour; snapshot() scanning /proc exactly once;
is_running spawning no subprocess at all (subprocess.run/Popen monkeypatched to
raise); and the session path being the LIVE one with "Tidal Multi" absent.
parent ab368180
...@@ -161,13 +161,24 @@ class PerfTray: ...@@ -161,13 +161,24 @@ class PerfTray:
bridge_act.triggered.connect(lambda: self._open_url(BRIDGE_URL)) bridge_act.triggered.connect(lambda: self._open_url(BRIDGE_URL))
self.menu.addAction(bridge_act) self.menu.addAction(bridge_act)
launch_menu = self.menu.addMenu("Launch ▸") # #69 — the menu offered to LAUNCH things without ever saying what was
# already up. The state was there all along (launchers.is_running), but
# _build_menu ran once at startup and read only `available`, so a running
# Ardour looked identical to a stopped one — and the safe action ("do
# nothing, it's up") looked identical to the dangerous one.
self.gear_menu = self.menu.addMenu("Gear ▸")
self.gear_actions = {}
snap = LA.snapshot() snap = LA.snapshot()
for item in (*snap["web"], *snap["apps"]): for item in (*snap["web"], *snap["apps"]):
act = QAction(item["name"], launch_menu) act = QAction(item["name"], self.gear_menu)
act.setEnabled(item["available"])
act.triggered.connect(lambda _c, k=item["key"]: self._launch(k)) act.triggered.connect(lambda _c, k=item["key"]: self._launch(k))
launch_menu.addAction(act) self.gear_menu.addAction(act)
self.gear_actions[item["key"]] = act
self._apply_gear(snap)
# Gear state is refreshed on menu-open ONLY, never on the 2s timer: reading
# it walks /proc, and doing that 30x a minute for a label nobody is looking
# at is the per-tick cost this rig keeps getting bitten by.
self.menu.aboutToShow.connect(self.refresh_gear)
self.menu.addSeparator() self.menu.addSeparator()
self.autostart_action = QAction("Start at login", self.menu, checkable=True) self.autostart_action = QAction("Start at login", self.menu, checkable=True)
...@@ -221,6 +232,42 @@ class PerfTray: ...@@ -221,6 +232,42 @@ class PerfTray:
proc.finished.connect(done) proc.finished.connect(done)
proc.start() proc.start()
# Glyphs, not colour: a tray menu inherits the desktop palette, so colour is not
# reliably legible, and PLN reads this in a dark room seconds before playing.
GEAR_GLYPH = {"running": "●", "stopped": "○", "missing": "✗"}
def _apply_gear(self, snap):
"""Label every gear entry with what it IS, not just what it could become."""
up = 0
total = 0
for item in (*snap["web"], *snap["apps"]):
act = self.gear_actions.get(item["key"])
if act is None:
continue
total += 1
if not item["available"]:
state = "missing"
elif item["running"]:
state = "running"
up += 1
else:
state = "stopped"
suffix = {"running": " — running", "stopped": "", "missing": " — not installed"}
act.setText(f"{self.GEAR_GLYPH[state]} {item['name']}{suffix[state]}")
# A running app stays clickable ONLY if clicking it opens something (a web
# tool's URL). For a native app, launch() would refuse anyway, and a
# greyed row says "already up" faster than a notification does.
act.setEnabled(item["available"] and
(item["kind"] == "web" or not item["running"]))
self.gear_menu.setTitle(f"Gear ▸ {up}/{total} up")
def refresh_gear(self):
try:
self._apply_gear(LA.snapshot())
except Exception:
# The tray is non-essential: a status label must never take the menu down.
pass
def _open_url(self, url): def _open_url(self, url):
QProcess.startDetached("xdg-open", [url]) QProcess.startDetached("xdg-open", [url])
......
...@@ -10,7 +10,9 @@ rather than spawning a duplicate. ...@@ -10,7 +10,9 @@ rather than spawning a duplicate.
""" """
from __future__ import annotations from __future__ import annotations
import glob
import os import os
import re
import shutil import shutil
import socket import socket
import subprocess import subprocess
...@@ -18,7 +20,13 @@ from pathlib import Path ...@@ -18,7 +20,13 @@ from pathlib import Path
HERE = Path(__file__).resolve().parent HERE = Path(__file__).resolve().parent
TIDAL = Path.home() / "Work" / "Sound" / "Tidal" TIDAL = Path.home() / "Work" / "Sound" / "Tidal"
ARDOUR_SESSION = Path.home() / "Work" / "Sound" / "Ardour" / "Tidal Multi" / "Tidal Multi.ardour" # "Tidal Live" is THE performing session (records every orbit as a stem). "Tidal
# Multi" is the older ARCHIVE session holding the historical per-orbit recordings —
# it was wired in here, so the tray's Ardour button opened the archive instead of the
# rig, and any faders touched there would have been the wrong ones. Auditing that
# same archive as if it were live already produced a confidently wrong fader report
# on 2026-07-28; this is the same mixup, one layer down.
ARDOUR_SESSION = Path.home() / "Work" / "Sound" / "Ardour" / "Tidal Live" / "Tidal Live.ardour"
MIDIMON = HERE / "midimon.py" MIDIMON = HERE / "midimon.py"
# Terminal emulators we know how to wrap, best first. # Terminal emulators we know how to wrap, best first.
...@@ -50,12 +58,17 @@ def _term_argv(inner): ...@@ -50,12 +58,17 @@ def _term_argv(inner):
# `pgrep` = running-check pattern; `terminal` = run inside a terminal emulator. # `pgrep` = running-check pattern; `terminal` = run inside a terminal emulator.
LAUNCHERS = [ LAUNCHERS = [
{"key": "pulsar", "name": "Pulsar", "blurb": "editor — open the Tidal folder", {"key": "pulsar", "name": "Pulsar", "blurb": "editor — open the Tidal folder",
"candidates": ["pulsar"], "args": [str(TIDAL)], "pgrep": "pulsar"}, "candidates": ["pulsar"], "args": [str(TIDAL)], "exe": r"pulsar$"},
{"key": "ardour", "name": "Ardour", "blurb": "DAW — the Tidal Multi session", # `ardour9` was missing from the candidates, so this entry could never actually
"candidates": ["ardour8", "ardour7", "ardour6", "ardour"], # start the installed Ardour (9.2.0) — it reported "unavailable" while Ardour was
"args": ([str(ARDOUR_SESSION)] if ARDOUR_SESSION.exists() else []), "pgrep": "ardour"}, # running. Newest first, and the version-suffixed real binary (`ardour-9.2.0`) is
# what the exe check has to match.
{"key": "ardour", "name": "Ardour", "blurb": "DAW — the Tidal Live session (records the stems)",
"candidates": ["ardour9", "ardour8", "ardour7", "ardour6", "ardour"],
"args": ([str(ARDOUR_SESSION)] if ARDOUR_SESSION.exists() else []),
"exe": r"[Aa]rdour[-\d.]*$"},
{"key": "qjackctl", "name": "QjackCtl", "blurb": "JACK / audio graph control", {"key": "qjackctl", "name": "QjackCtl", "blurb": "JACK / audio graph control",
"candidates": ["qjackctl"], "args": [], "pgrep": "qjackctl"}, "candidates": ["qjackctl"], "args": [], "exe": r"qjackctl$"},
{"key": "midimon", "name": "MIDI Monitor", "blurb": "live, parsed MIDI seq (aseqdump done right)", {"key": "midimon", "name": "MIDI Monitor", "blurb": "live, parsed MIDI seq (aseqdump done right)",
"candidates": ["python3"], "args": [str(MIDIMON)], "pgrep": "midimon.py", "terminal": True}, "candidates": ["python3"], "args": [str(MIDIMON)], "pgrep": "midimon.py", "terminal": True},
] ]
...@@ -95,12 +108,59 @@ def available(spec): ...@@ -95,12 +108,59 @@ def available(spec):
return True return True
def is_running(spec): def _cmdlines():
"""[(argv0 basename, full command line)] for every process, with NO forks.
This used to be one `pgrep -f` per launcher, so a single snapshot() forked N
processes — and snapshot() is called by the web Bridge's poll AND (now) by the
tray menu. A fork per item per refresh is the shape of bug that made the LED
daemon lag; on an audio rig it is not worth paying for a boolean.
"""
out = []
for p in glob.glob("/proc/[0-9]*/cmdline"):
if p.split("/")[2] == str(os.getpid()):
continue
try: try:
return subprocess.run(["pgrep", "-f", spec["pgrep"]], with open(p, "rb") as f:
capture_output=True).returncode == 0 raw = f.read()
except OSError: except OSError:
continue # the pid exited between glob and open; ordinary
if not raw:
continue
line = raw.replace(b"\x00", b" ").decode("utf-8", "replace").strip()
argv0 = line.split(" ", 1)[0].rsplit("/", 1)[-1] if line else ""
out.append((argv0, line))
return out
def is_running(spec, lines=None):
"""True if a matching process is running.
Matches the EXECUTABLE NAME (`exe`) rather than the whole command line wherever
possible, because the whole line is full of false positives: `pgrep -f ardour`
matched the `tidal-ardour-autoroute.sh` helper AND any shell whose command line
merely mentioned the word — so the tray could report Ardour up when it was down,
which is worse than reporting nothing. Full-line matching (`pgrep`) is kept only
for interpreted tools, where argv[0] is `python3` and the real identity is the
script path.
`lines` lets a caller scan /proc ONCE for a whole snapshot instead of per item.
"""
exe = spec.get("exe")
if exe:
try:
rx = re.compile(exe)
except re.error:
return False
return any(rx.match(a) for a, _ in (lines if lines is not None else _cmdlines()))
pat = spec.get("pgrep")
if not pat:
return False
try:
rx = re.compile(pat)
except re.error:
return False return False
return any(rx.search(ln) for _, ln in (lines if lines is not None else _cmdlines()))
def _argv(spec): def _argv(spec):
...@@ -146,8 +206,9 @@ def launch(key): ...@@ -146,8 +206,9 @@ def launch(key):
def snapshot(): def snapshot():
"""Apps + web tools with live state for the UI / tray.""" """Apps + web tools with live state for the UI / tray."""
lines = _cmdlines() # scan /proc ONCE for the whole snapshot
apps = [{"key": s["key"], "name": s["name"], "blurb": s["blurb"], "kind": "app", apps = [{"key": s["key"], "name": s["name"], "blurb": s["blurb"], "kind": "app",
"available": available(s), "running": is_running(s), "available": available(s), "running": is_running(s, lines),
"terminal": bool(s.get("terminal"))} for s in LAUNCHERS] "terminal": bool(s.get("terminal"))} for s in LAUNCHERS]
web = [{"key": s["key"], "name": s["name"], "blurb": s["blurb"], "kind": "web", web = [{"key": s["key"], "name": s["name"], "blurb": s["blurb"], "kind": "web",
"url": s["url"], "available": True, "running": _port_open(s["port"])} "url": s["url"], "available": True, "running": _port_open(s["port"])}
......
...@@ -42,3 +42,99 @@ def test_web_launch_already_running_opens(monkeypatch): ...@@ -42,3 +42,99 @@ def test_web_launch_already_running_opens(monkeypatch):
r = L.launch("foundry") r = L.launch("foundry")
assert r["ok"] is True and r["status"] == "already-running" assert r["ok"] is True and r["status"] == "already-running"
assert r["url"].endswith(":8765/") assert r["url"].endswith(":8765/")
# --------------------------------------------------------------------------- #
# is_running: forkless, and no longer matching the whole world (#69)
# --------------------------------------------------------------------------- #
def test_cmdlines_returns_argv0_and_the_full_line():
rows = L._cmdlines()
assert rows, "no processes seen — the /proc scan is broken"
for argv0, line in rows[:50]:
assert "/" not in argv0 # basename only
assert isinstance(line, str)
def test_an_exe_pattern_matches_the_executable_not_a_mention_of_it():
"""The bug: `pgrep -f ardour` matched the `tidal-ardour-autoroute.sh` helper AND
any SHELL whose command line merely mentioned the word, so the tray could report
Ardour up while it was down. Reporting gear up when it is down is worse than
reporting nothing, because it is the state you act on."""
spec = L.BY_KEY["ardour"]
lines = [("bash", "bash /home/x/.local/bin/tidal-ardour-autoroute.sh"),
("zsh", "zsh -c 'grep ardour something'")]
assert L.is_running(spec, lines) is False
lines.append(("ardour-9.2.0", "/usr/bin/ardour-9.2.0 /home/x/Tidal Live.ardour"))
assert L.is_running(spec, lines) is True
def test_the_ardour_exe_pattern_matches_a_version_suffixed_binary():
"""The installed binary is `ardour-9.2.0`, not `ardour` — a bare `ardour$` match
would have reported it as down forever."""
spec = L.BY_KEY["ardour"]
for exe in ("ardour", "ardour9", "ardour-9.2.0", "Ardour", "ardour-10.0.1"):
assert L.is_running(spec, [(exe, f"/usr/bin/{exe}")]) is True, exe
def test_a_session_filename_ending_in_dot_ardour_is_not_a_running_ardour():
spec = L.BY_KEY["ardour"]
assert L.is_running(spec, [("cp", "cp 'Tidal Live.ardour' /tmp")]) is False
def test_an_interpreted_tool_still_matches_on_the_full_line():
"""midimon runs as `python3 .../midimon.py`, so argv[0] is python3 and the real
identity is the script path. That is the one case full-line matching is for."""
spec = L.BY_KEY["midimon"]
assert spec.get("exe") is None and spec.get("pgrep")
assert L.is_running(spec, [("python3", "python3 /x/tools/bridge/midimon.py")]) is True
assert L.is_running(spec, [("python3", "python3 /x/other.py")]) is False
def test_is_running_is_false_for_a_spec_with_no_pattern():
assert L.is_running({"key": "x"}, [("anything", "anything")]) is False
def test_is_running_survives_a_broken_pattern():
assert L.is_running({"exe": "([unclosed"}, [("x", "x")]) is False
def test_snapshot_scans_proc_once_for_all_apps(monkeypatch):
"""It used to fork one `pgrep` PER launcher, and snapshot() is called by both the
web Bridge's poll and the tray menu. A fork per item per refresh is the shape of
bug that made the LED daemon lag."""
calls = []
real = L._cmdlines
monkeypatch.setattr(L, "_cmdlines", lambda: (calls.append(1), real())[1])
L.snapshot()
assert len(calls) == 1
def test_is_running_spawns_no_subprocess(monkeypatch):
def boom(*a, **k):
raise AssertionError("is_running must not fork")
monkeypatch.setattr(L.subprocess, "run", boom)
monkeypatch.setattr(L.subprocess, "Popen", boom)
L.snapshot()
# --------------------------------------------------------------------------- #
# the session it opens
# --------------------------------------------------------------------------- #
def test_ardour_opens_the_LIVE_session_not_the_archive():
""""Tidal Live" is the performing session; "Tidal Multi" is the older archive of
per-orbit recordings. This entry pointed at the ARCHIVE, so the tray's Ardour
button opened the wrong session and any faders touched there were the wrong ones.
Auditing that same archive as if it were live already produced a confidently
wrong fader report on 2026-07-28."""
assert L.ARDOUR_SESSION.name == "Tidal Live.ardour"
assert "Tidal Multi" not in str(L.ARDOUR_SESSION)
def test_ardour_candidates_include_the_installed_major_version():
"""`ardour9` was missing, so this entry could never start the installed Ardour —
it reported "unavailable" while Ardour was running."""
cands = L.BY_KEY["ardour"]["candidates"]
assert "ardour9" in cands
assert cands.index("ardour9") < cands.index("ardour8") # newest first
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment