Commit 8e3ef375 by PLN (Algolia)

feat(tray): a Close that closes, and stays closed

PLN: "ensure i can close midi mon atm it forces on when i want the gui/tray to
allow run and or close? its too sticky atm ahah" -- the second report of the
same complaint, because the first fix was committed and never made live: the
shared checkout still runs 5468507b, so tonight's midiviz has neither the X nor
the latch. The code was right and the deploy was missing, which from where he
sits is indistinguishable from not being fixed.

Adds the other half he asked for: launchers.stop(key), and a "Close ▸" entry
under Rig.

Closability is OPT-IN per launcher, keyed on declaring a systemd unit. A
generic close-anything row is one mis-click from stopping Ardour mid-set, and
this menu's whole value is being safe to touch while playing. Only the MIDI
lens opts in today; a test asserts ardour/pulsar/supercollider never quietly
acquire it.

Close ▸ is a separate submenu rather than a Raise/Close pair on each row. The
rows have one job mid-set -- one click raises the tool you need -- and putting
a Close next to that is the mis-click itself. So closing lives in one place,
holds only what is running AND closable, and is empty and disabled otherwise.

The latch is written BEFORE anything is stopped. ensure() runs from gig-up and
the Bridge watcher, a converge can land in the same second, and a latch
written afterwards races it -- the window returns and the close looks broken,
which is the exact complaint. A test pins the ORDER, not just the outcome.

stop() prefers `systemctl --user stop` where a unit owns the process: killing
it directly leaves systemd's view wrong, and for a unit with Restart= systemd
is what decides next. Where there is no unit it SIGTERMs, using a /proc walk
rather than pgrep -f -- the Bash wrapper embeds our own command line, so
pgrep -f matching would find this session and the kill would take down the
tool doing the killing. Same identity rule as is_running(), so "it says
running" and "this is what I would kill" cannot disagree.

And it confirms instead of assuming: it polls for up to 2s and returns
"still-running" if the process outlives the request. Reporting a clean close
while the window is still on screen is the same class of lie as this morning's
delete that reported 0/14 for fourteen successful deletions.
parent a8c2af8c
......@@ -378,6 +378,16 @@ class PerfTray:
act.triggered.connect(lambda _c, k=item["key"]: self._launch(k))
self.rig_menu.addAction(act)
self.rig_actions[item["key"]] = act
# "Close ▸" is a SEPARATE submenu rather than a per-row Raise/Close pair,
# because the rows above have one job mid-set: one click raises the tool
# you need. Turning each into a submenu would cost that click, and a
# "Close" sitting next to a "Raise" is a mis-click that stops a running
# instrument. So closing lives in one place, holds only what is
# currently running AND opts in to being closable, and is empty (and
# disabled) the rest of the time. PLN, 2026-09-05: "i want the gui/tray
# to allow run and or close".
self.close_menu = self.rig_menu.addMenu("Close ▸")
self._rebuild_close_menu(snap)
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
......@@ -468,6 +478,41 @@ class PerfTray:
# 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.
def _rebuild_close_menu(self, snap):
"""Only what is running and closable. Rebuilt per menu-open."""
self.close_menu.clear()
items = [i for i in (*snap["web"], *snap["apps"])
if i.get("running") and i.get("closable")]
if not items:
a = QAction("nothing running to close", self.close_menu)
a.setEnabled(False)
self.close_menu.addAction(a)
self.close_menu.setEnabled(False)
return
self.close_menu.setEnabled(True)
for item in items:
a = QAction(f"⏹ {item['name']}", self.close_menu)
a.triggered.connect(lambda _c, k=item["key"], n=item["name"]:
self._close(k, n))
self.close_menu.addAction(a)
def _close(self, key, name):
"""Ask launchers to close it, and report what actually happened."""
try:
res = LA.stop(key)
except Exception as e: # noqa: BLE001
self.tray.showMessage("perf-tray", f"{name}: {e}",
QSystemTrayIcon.Warning, 5000)
return
# stop() confirms rather than assumes, so its message is shown verbatim
# -- including "asked to stop but still running", which is the case a
# silent success would hide. Only surface a failure as a popup, matching
# _launch: a toast for every successful close would be noise mid-set.
if not res.get("ok"):
self.tray.showMessage("perf-tray", res.get("msg") or name,
QSystemTrayIcon.Warning, 5000)
self.refresh_gear()
GEAR_GLYPH = {"running": "●", "stopped": "○", "missing": "✗"}
def _apply_gear(self, snap):
......@@ -644,7 +689,11 @@ class PerfTray:
def refresh_gear(self):
try:
self._apply_gear(LA.snapshot())
snap = LA.snapshot()
self._apply_gear(snap)
# Rebuilt from the SAME snapshot the labels came from. Two reads
# would let the rows say "running" while Close ▸ disagreed.
self._rebuild_close_menu(snap)
except Exception:
# The tray is non-essential: a status label must never take the menu down.
pass
......
......@@ -12,6 +12,7 @@ from __future__ import annotations
import glob
import os
import signal
import re
import shutil
import socket
......@@ -90,7 +91,14 @@ LAUNCHERS = [
# un-says a previous close (midiviz writes a latch that
# rig_units.ensure() honours until logout). Cleared only when we
# actually SPAWN -- focusing a running window is not a new decision.
"latch": "midiviz"},
"latch": "midiviz",
# Declaring the unit is what makes this row CLOSABLE from the tray. PLN,
# 2026-09-05: "ensure i can close midi mon atm it forces on when i want
# the gui/tray to allow run and or close". Closability is opt-in per
# launcher on purpose -- a generic "close anything" row is one mis-click
# from killing Ardour mid-set, and the whole point of this menu is to be
# safe to touch while playing.
"unit": "midiviz"},
]
# Web tools = start-or-open. `port` open ⇒ running; else `cmd` is spawned, then
......@@ -236,6 +244,122 @@ def _cmdlines():
return out
def matching_pids(spec) -> list[int]:
"""PIDs of the processes is_running() would have matched.
Deliberately re-implements the /proc walk rather than calling pgrep -f:
the Bash wrapper embeds our own command text, so a `pgrep -f midiviz.py`
from inside this session matches the session itself and a kill built on it
takes down the tool doing the killing. Skipping os.getpid() is not enough
either -- the match must be the same exe/pgrep rule is_running() uses, so
that "it says running" and "this is what I would kill" can never disagree.
"""
exe, pat = spec.get("exe"), spec.get("pgrep")
try:
rx = re.compile(exe or pat) if (exe or pat) else None
except re.error:
return []
if rx is None:
return []
me, out = os.getpid(), []
for path in glob.glob("/proc/[0-9]*/cmdline"):
try:
pid = int(path.split("/")[2])
except ValueError:
continue
if pid == me:
continue
try:
with open(path, "rb") as f:
raw = f.read()
except OSError:
continue
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 ""
if rx.match(argv0) if exe else rx.search(line):
out.append(pid)
return out
def closable(spec) -> bool:
"""Only launchers that opt in. See the note on midiviz's "unit" key."""
return bool(spec.get("unit") or spec.get("closable"))
def _write_latch(spec):
"""Record that a human closed this, so ensure() will not undo it."""
unit = spec.get("latch") or spec.get("unit")
if not unit:
return
rt = os.environ.get("XDG_RUNTIME_DIR") or f"/run/user/{os.getuid()}"
try:
d = os.path.join(rt, "parvagues")
os.makedirs(d, exist_ok=True)
with open(os.path.join(d, f"{unit}.closed"), "w") as fh:
fh.write(time.strftime("%Y-%m-%dT%H:%M:%S%z") + "\n")
except OSError:
pass
def stop(key):
"""Close a launcher the human asked to close. Returns {ok, status, msg}.
Order matters: the latch is written BEFORE anything is killed. rig_units
.ensure() runs from gig-up and the Bridge watcher on a converge that can
land in the same second, and a latch written afterwards races it -- the
window would come back and look like the close had failed, which is
exactly the complaint this is fixing.
Prefers `systemctl --user stop` where a unit owns the process. Killing the
process directly instead would leave systemd's view of it wrong, and for
a unit with Restart= it is systemd that decides what happens next.
"""
spec = BY_KEY.get(key)
if not spec:
return {"ok": False, "status": "unknown", "msg": f"unknown launcher {key!r}"}
if not closable(spec):
return {"ok": False, "status": "not-closable",
"msg": f"{spec['name']} does not support closing from here"}
if not is_running(spec):
return {"ok": True, "status": "already-stopped",
"msg": f"{spec['name']} is not running"}
_write_latch(spec)
unit, how = spec.get("unit"), ""
if unit:
r = subprocess.run(["systemctl", "--user", "stop", f"{unit}.service"],
capture_output=True, text=True)
how = f"systemctl --user stop {unit}"
if r.returncode != 0:
return {"ok": False, "status": "error",
"msg": f"{how} failed: {(r.stderr or '').strip()[:200]}"}
else:
pids = matching_pids(spec)
if not pids:
return {"ok": True, "status": "already-stopped",
"msg": f"{spec['name']} left no process to signal"}
how = f"SIGTERM {pids}"
for pid in pids:
try:
os.kill(pid, signal.SIGTERM)
except (ProcessLookupError, PermissionError) as e:
return {"ok": False, "status": "error", "msg": f"kill {pid}: {e}"}
# Confirmed, not assumed. A GUI process takes a moment to tear down its
# window, and reporting "closed" while it is still on screen is the same
# class of lie as a delete that reports success before it propagates.
for _ in range(20):
if not is_running(spec):
return {"ok": True, "status": "stopped",
"msg": f"{spec['name']} closed", "how": how}
time.sleep(0.1)
return {"ok": False, "status": "still-running",
"msg": f"{spec['name']} was asked to stop ({how}) but is still "
f"running after 2s", "how": how}
def is_running(spec, lines=None):
"""True if a matching process is running.
......@@ -477,8 +601,10 @@ def snapshot():
lines = _cmdlines() # scan /proc ONCE for the whole snapshot
apps = [{"key": s["key"], "name": s["name"], "blurb": s["blurb"], "kind": "app",
"available": available(s), "running": is_running(s, lines),
"closable": closable(s),
"terminal": bool(s.get("terminal"))} for s in LAUNCHERS]
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"]),
"closable": closable(s)}
for s in WEB]
return {"web": web, "apps": apps}
......@@ -152,3 +152,118 @@ def test_ardour_candidates_include_the_installed_major_version():
cands = L.BY_KEY["ardour"]["candidates"]
assert "ardour9" in cands
assert cands.index("ardour9") < cands.index("ardour8") # newest first
# ── closing, and staying closed ────────────────────────────────────────────
#
# PLN: "ensure i can close midi mon atm it forces on when i want the gui/tray
# to allow run and or close". Two things have to be true: the close must
# happen, and it must SURVIVE the next converge. The second is the one that
# was broken, and it is the one a unit test can actually pin.
def _launchers(tmp_path, monkeypatch):
import importlib.util
import sys as _sys
from pathlib import Path as _Path
monkeypatch.setenv("XDG_RUNTIME_DIR", str(tmp_path))
root = _Path(__file__).resolve().parents[3]
out = []
for name, rel in (("_la_stop", "tools/bridge/launchers.py"),
("_ru_stop", "tools/rig_units.py")):
spec = importlib.util.spec_from_file_location(name, root / rel)
mod = importlib.util.module_from_spec(spec)
_sys.modules[name] = mod
spec.loader.exec_module(mod)
out.append(mod)
return out
def test_closable_is_opt_in():
"""A generic close-anything row is one mis-click from killing Ardour."""
import importlib.util
import sys as _sys
from pathlib import Path as _Path
root = _Path(__file__).resolve().parents[3]
spec = importlib.util.spec_from_file_location("_la_optin",
root / "tools/bridge/launchers.py")
la = importlib.util.module_from_spec(spec)
_sys.modules["_la_optin"] = la
spec.loader.exec_module(la)
by_key = la.BY_KEY
assert la.closable(by_key["midimon"]), \
"the MIDI lens is the one PLN asked to be closable"
closables = [k for k, s in by_key.items() if la.closable(s)]
for risky in ("ardour", "pulsar", "supercollider", "sclang"):
assert risky not in closables, (
f"{risky!r} became closable from the tray — that is a mis-click "
f"away from stopping the set")
def test_stop_writes_the_latch_before_it_kills(tmp_path, monkeypatch):
"""Order matters: a converge landing in the same second must lose."""
la, rig = _launchers(tmp_path, monkeypatch)
order = []
spec = dict(la.BY_KEY["midimon"])
monkeypatch.setattr(la, "BY_KEY", dict(la.BY_KEY, midimon=spec))
monkeypatch.setattr(la, "is_running", lambda s, lines=None: not order)
real_latch = la._write_latch
def spy_latch(s):
order.append("latch")
real_latch(s)
def fake_run(argv, **kw):
order.append("stop")
import subprocess as _sp
return _sp.CompletedProcess(argv, 0, "", "")
monkeypatch.setattr(la, "_write_latch", spy_latch)
monkeypatch.setattr(la.subprocess, "run", fake_run)
res = la.stop("midimon")
assert res["ok"], res
assert order[0] == "latch", (
f"the latch must be written before the process is stopped, got "
f"{order} — otherwise ensure() can restart it in the gap and the "
f"close looks like it failed")
assert rig.closed_by_user("midiviz"), \
"rig_units.ensure() cannot see the close the tray just performed"
def test_an_unclosable_key_is_refused_without_touching_anything(tmp_path, monkeypatch):
la, _rig = _launchers(tmp_path, monkeypatch)
calls = []
monkeypatch.setattr(la.subprocess, "run",
lambda *a, **k: calls.append(a) or None)
res = la.stop("ardour") if "ardour" in la.BY_KEY else {"ok": False,
"status": "unknown"}
assert not res["ok"]
assert res["status"] in ("not-closable", "unknown")
assert calls == [], "a refused close still ran a command"
def test_stop_reports_still_running_rather_than_lying(tmp_path, monkeypatch):
"""If it will not die, say so — do not report a clean close."""
la, _rig = _launchers(tmp_path, monkeypatch)
monkeypatch.setattr(la, "is_running", lambda s, lines=None: True)
monkeypatch.setattr(la, "_write_latch", lambda s: None)
import subprocess as _sp
monkeypatch.setattr(la.subprocess, "run",
lambda argv, **kw: _sp.CompletedProcess(argv, 0, "", ""))
monkeypatch.setattr(la.time, "sleep", lambda _s: None)
res = la.stop("midimon")
assert not res["ok"]
assert res["status"] == "still-running"
assert "still running" in res["msg"]
def test_matching_pids_never_returns_our_own_pid(tmp_path, monkeypatch):
"""pgrep -f style matching finds the session running the search."""
la, _rig = _launchers(tmp_path, monkeypatch)
import os as _os
assert _os.getpid() not in la.matching_pids(la.BY_KEY["midimon"]), \
"matching_pids would have SIGTERMed the process doing the killing"
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