blockers: the report's cooldown was deciding whether to block

Both clients made the same mistake, in the same shape: the throttle that keeps
the daily report from filling with one row per poll was placed in front of the
enforcement call. So blocking worked exactly once per target, and the second
launch of a banned app — or the second visit to a banned site — simply went
through.

Android, `BlockerEngine`: `claim()` now gates only `report()`/`announce()`.
Enforcement runs on every decision behind a 1.2s debounce whose only job is to
stop the accessibility event and the usage-stats poll both pressing home for one
visit. An app still in front on the next pass is pressed again — one
GLOBAL_ACTION_HOME is a request, not a result — and a visit is no longer
consumed when no accessibility service is attached to consume it with, which
turned a missing grant into a silently unblocked app. `lastForeground` and
`lastBlockedApp` are @Volatile like every other field here; they are written
from the service's main thread and the poll coroutine both. The usage-stats
cadence now follows the server's `app_poll_seconds`, and the first look happens
at once rather than four seconds in.

Linux, `BlockerManager._enforce_apps`: the pid went into the dedupe set *before*
the kill was attempted, so a process whose signal was refused was exempt for the
rest of the interval, and a pid is reused — a relaunch landing on a recycled
number inherited the exemption. Processes are now identified by (pid, start
time) from /proc/<pid>/stat, the kill is attempted on every pass, and only the
report is throttled. The books are pruned of dead processes so an all-day
interval cannot leak.

`processes.terminate()` no longer busy-waits two seconds for the process to die
before escalating — on the desktop that wait ran on the thread drawing the
countdown. SIGTERM now, SIGKILL on a later pass once the grace has gone by,
which is also gentler than the old fixed two seconds. With nothing blocking,
the scan gets its own timer at the cadence the server asks for instead of
riding the five-second policy poll.

Neither enforcement path had any test: the desktop's blocker suite hardcoded an
empty app list and nothing anywhere touched BlockerEngine. 16 new tests, on
fakes and the committed fixtures.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

desktop/kadence_desktop/blocking/manager.py

@@ -25,6 +25,8 @@ machine spends a moment asking a dead port.
 from __future__ import annotations
 
 import logging
+import time
+from types import ModuleType
 from typing import Callable
 
 from kadence_desktop.blocking import processes
@@ -43,17 +45,35 @@ class BlockerManager:
         notify: Callable[[str, str], None] | None = None,
         stub: StubResolver | None = None,
         route: ResolvedRoute | None = None,
+        procs: ModuleType | None = None,
+        report_cooldown: float = 20.0,
+        kill_grace: float = 5.0,
     ) -> None:
         self._hosts = HostsBlocker(helper_path)
         # Injectable so tests can drive the ordering without binding ports or
         # asking a real systemd-resolved for anything.
         self._stub = stub if stub is not None else StubResolver()
         self._route = route if route is not None else ResolvedRoute(helper_path)
+        # Injectable too, so the app-blocking tests do not have to own a victim
+        # process — and because `/proc` is not something a test should need.
+        self._procs = procs if procs is not None else processes
         self._report = report
         self._notify = notify
         self._active = False
         self._applied: list[str] = []
-        self._announced: set[int] = set()
+        #: The app list and kill/warn flag from the last policy, so the scan
+        #: timer has something to work from between polls.
+        self._apps: list[str] = []
+        self._kill_apps = True
+        #: What we have already told you about, keyed by (pid, start time) and
+        #: stamped with when. It throttles the *report*, never the block — see
+        #: [_enforce_apps].
+        self._reported: dict[tuple[int, int], float] = {}
+        #: When each process was asked politely to quit, so the next pass can
+        #: insist. Keyed the same way.
+        self._asked: dict[tuple[int, int], float] = {}
+        self.report_cooldown = report_cooldown
+        self.kill_grace = kill_grace
         self._redirect_ip = "127.0.0.1"
         self.enforce_hosts = True
         self.enforce_apps = True
@@ -115,9 +135,10 @@ class BlockerManager:
                     log.info("blocking %s domain(s)", len(domains))
                 else:
                     self.last_error = self._hosts.last_error
-            if self.enforce_apps and apps:
-                self._enforce_apps(apps, kill=bool(policy.get("kill_apps", True)))
+            self._apps = apps
+            self._kill_apps = bool(policy.get("kill_apps", True))
             self._active = True
+            self.enforce_apps_now()
         else:
             self.release()
 
@@ -165,7 +186,8 @@ class BlockerManager:
             else:
                 self.last_error = self._hosts.last_error
         self._active = False
-        self._announced.clear()
+        self._reported.clear()
+        self._asked.clear()
         # The stub keeps running and keeps its route: the zones are still pointed
         # at it, so it has to answer — it just forwards everything again.
         self._stub.set_rules([], [], self._redirect_ip, active=False)
@@ -186,18 +208,85 @@ class BlockerManager:
 
     # ---- apps --------------------------------------------------------------
 
-    def _enforce_apps(self, patterns: list[str], *, kill: bool) -> None:
-        for app in processes.scan(patterns):
-            if app.pid in self._announced:
-                continue
-            self._announced.add(app.pid)
+    def enforce_apps_now(self) -> None:
+        """One app-blocking pass, from the timer that owns the cadence.
 
-            if kill and processes.terminate(app):
-                self.killed_today += 1
-                self._report("app", app.matched, "killed", f"{app.name} (pid {app.pid})")
-                if self._notify:
-                    self._notify("Closed a distraction", f"{app.name} is blocked during focus.")
-            else:
-                self._report("app", app.matched, "warned", f"{app.name} (pid {app.pid})")
-                if self._notify:
-                    self._notify("That app is blocked", f"{app.name} is on your block list.")
+        Separate from [sync] because the two halves want different clocks: the
+        hosts file only changes when the policy does, while a process can be
+        launched at any moment and the server says how often to look
+        (`blocker.app_poll_seconds`, arriving as `poll_seconds`).
+        """
+        if not (self._active and self.enforce_apps and self._apps):
+            return
+        self._enforce_apps(self._apps, kill=self._kill_apps)
+
+    def _enforce_apps(self, patterns: list[str], *, kill: bool) -> None:
+        """Close (or report) every blocked process running right now.
+
+        The important word is *every*, on *every* pass. An earlier version kept a
+        set of pids it had dealt with and skipped them forever, which conflated
+        two different jobs: not spamming the daily report with one row per poll,
+        and actually blocking the app. The second launch of a blocked app — or
+        the first one whose SIGTERM was refused — then went unchallenged for the
+        rest of the interval, which is the whole bug this docstring exists for.
+
+        So the enforcement runs again each pass and only the *report* is
+        throttled, per process per [report_cooldown]. "Per process" is
+        (pid, start time), not pid: a pid alone is reused, and a relaunch
+        landing on a recycled number must not inherit the old entry.
+
+        The kill escalates across passes rather than inside one: SIGTERM now,
+        SIGKILL once [kill_grace] has gone by and the process is still there.
+        That is gentler than the old two-second busy-wait and it never blocks
+        the thread drawing the countdown.
+        """
+        self._forget_dead()
+        now = time.monotonic()
+        for app in self._procs.scan(patterns):
+            identity = app.identity
+            if not kill:
+                self._announce(app, "warned")
+                continue
+
+            asked_at = self._asked.get(identity)
+            if asked_at is None:
+                delivered = self._procs.terminate(app)
+                self._asked[identity] = now
+                if delivered:
+                    self.killed_today += 1
+                self._announce(app, "killed" if delivered else "warned")
+            elif now - asked_at >= self.kill_grace:
+                # It sat through the polite request. Insist, and keep insisting
+                # one grace period at a time if something refuses the signal.
+                self._procs.terminate(app, force=True)
+                self._asked[identity] = now
+                self._announce(app, "killed")
+
+    def _announce(self, app: "processes.RunningApp", action: str) -> None:
+        """Report and notify, at most once per process per cooldown."""
+        if not self._claim_report(app.identity):
+            return
+        details = f"{app.name} (pid {app.pid})"
+        self._report("app", app.matched, action, details)
+        if not self._notify:
+            return
+        if action == "killed":
+            self._notify("Closed a distraction", f"{app.name} is blocked during focus.")
+        else:
+            self._notify("That app is blocked", f"{app.name} is on your block list.")
+
+    def _claim_report(self, identity: tuple[int, int]) -> bool:
+        """Whether this process is due a report, and stamp it if so."""
+        now = time.monotonic()
+        last = self._reported.get(identity)
+        if last is not None and now - last < self.report_cooldown:
+            return False
+        self._reported[identity] = now
+        return True
+
+    def _forget_dead(self) -> None:
+        """Drop processes that are gone, so an all-day interval cannot leak."""
+        for book in (self._reported, self._asked):
+            for pid, started in list(book):
+                if not self._procs.alive(pid, started):
+                    del book[(pid, started)]

desktop/kadence_desktop/blocking/processes.py

@@ -10,6 +10,12 @@ Matching is deliberately forgiving — `steam` catches `steamwebhelper`, and
 a glob (`steam*`, `*discord*`, `telegram-?`), matched against both the process
 name and its command line. It never matches ourselves, PID 1, or anything
 outside the current user's processes.
+
+Every match carries its **start time** as well as its pid. A pid on its own is
+not an identity: Linux reuses them, and the blocker remembers which processes it
+has already told you about. Keyed by pid alone, a relaunch that happens to land
+on a recycled number would inherit the old entry and be let through — which is
+exactly the "it started the second time" this file is here to prevent.
 """
 
 from __future__ import annotations
@@ -17,7 +23,6 @@ from __future__ import annotations
 import logging
 import os
 import signal
-import time
 from fnmatch import fnmatch
 from dataclasses import dataclass
 from pathlib import Path
@@ -48,6 +53,13 @@ class RunningApp:
     name: str
     cmdline: str
     matched: str
+    #: Field 22 of `/proc/<pid>/stat` — clock ticks since boot. With the pid it
+    #: makes an identity that survives pid reuse. 0 when it could not be read.
+    start_time: int = 0
+
+    @property
+    def identity(self) -> tuple[int, int]:
+        return (self.pid, self.start_time)
 
 
 def _read(path: Path) -> str:
@@ -57,6 +69,34 @@ def _read(path: Path) -> str:
         return ""
 
 
+def start_time(pid: int) -> int:
+    """Clock ticks since boot at which this pid started, 0 if unreadable.
+
+    `comm` sits in `stat` wrapped in parentheses and may itself contain spaces
+    and parentheses, so the fields are counted from the *last* `)` — the one
+    place this file cannot naively split on whitespace.
+    """
+    raw = _read(Path(f"/proc/{pid}/stat"))
+    cut = raw.rfind(")")
+    if cut < 0:
+        return 0
+    fields = raw[cut + 2 :].split()
+    # stat's field 3 is `state`, so field 22 is index 19 of what follows comm.
+    if len(fields) < 20:
+        return 0
+    try:
+        return int(fields[19])
+    except ValueError:
+        return 0
+
+
+def alive(pid: int, started: int = 0) -> bool:
+    """Whether this exact process is still running — not merely this pid."""
+    if not Path(f"/proc/{pid}").exists():
+        return False
+    return started == 0 or start_time(pid) == started
+
+
 def scan(patterns: list[str]) -> list[RunningApp]:
     """Every live process of this user matching one of `patterns`."""
     wanted = [pattern.strip().lower() for pattern in patterns if pattern.strip()]
@@ -89,32 +129,46 @@ def scan(patterns: list[str]) -> list[RunningApp]:
 
         match = next((pattern for pattern in wanted if matches(pattern, name, command)), None)
         if match:
-            found.append(RunningApp(pid=pid, name=comm, cmdline=cmdline[:200], matched=match))
+            found.append(
+                RunningApp(
+                    pid=pid,
+                    name=comm,
+                    cmdline=cmdline[:200],
+                    matched=match,
+                    start_time=start_time(pid),
+                )
+            )
     return found
 
 
-def terminate(app: RunningApp, *, grace_seconds: float = 2.0) -> bool:
-    """Ask the process to quit, then insist. True when it is gone."""
+def terminate(app: RunningApp, *, force: bool = False) -> bool:
+    """Signal the process once. True when the signal landed (or it is gone).
+
+    Deliberately **not** blocking. The previous version sent SIGTERM and then
+    busy-waited two seconds for the process to die before escalating, on
+    whichever thread happened to call it — which on the desktop is the one
+    drawing the countdown. Since the blocker now rescans every few seconds
+    anyway, the escalation is simply the *next* pass's job (see
+    `BlockerManager._enforce_apps`), and nothing has to wait for anybody.
+
+    A `PermissionError` on SIGTERM escalates immediately rather than next pass:
+    a process that refused the polite signal on permission grounds is not about
+    to quit on its own.
+    """
     try:
-        os.kill(app.pid, signal.SIGTERM)
+        os.kill(app.pid, signal.SIGKILL if force else signal.SIGTERM)
     except ProcessLookupError:
         return True
     except PermissionError:
-        log.warning("not allowed to stop %s (pid %s)", app.name, app.pid)
-        return False
-
-    deadline = time.monotonic() + grace_seconds
-    while time.monotonic() < deadline:
+        if force:
+            log.warning("not allowed to stop %s (pid %s)", app.name, app.pid)
+            return False
+        log.warning("not allowed to ask %s (pid %s) to quit", app.name, app.pid)
         try:
-            os.kill(app.pid, 0)
+            os.kill(app.pid, signal.SIGKILL)
         except ProcessLookupError:
             return True
-        time.sleep(0.1)
-
-    try:
-        os.kill(app.pid, signal.SIGKILL)
-    except ProcessLookupError:
-        return True
-    except PermissionError:
-        return False
+        except PermissionError:
+            log.warning("not allowed to stop %s (pid %s)", app.name, app.pid)
+            return False
     return True

desktop/kadence_desktop/controller.py

@@ -138,6 +138,15 @@ class Controller(QObject):
         self._idle_check.timeout.connect(self._check_idle)
         self._idle_check.start()
 
+        # A blocked app can be launched at any second, so the scan has its own
+        # clock rather than riding the policy poll — and the server is the one
+        # that says how fast (`blocker.app_poll_seconds`). It is cheap: a
+        # `/proc` walk and, at most, one signal per blocked process.
+        self._app_scan = QTimer(self)
+        self._app_scan.setInterval(self._app_scan_interval())
+        self._app_scan.timeout.connect(self._blocker.enforce_apps_now)
+        self._app_scan.start()
+
     # ======================================================================
     # Properties for QML
     # ======================================================================
@@ -647,8 +656,24 @@ class Controller(QObject):
         self._policy = policy
         if self._config.enforce_blocking:
             self._blocker.sync(policy)
+        wanted = self._app_scan_interval()
+        if self._app_scan.interval() != wanted:
+            self._app_scan.setInterval(wanted)
         self.blockerChanged.emit()
 
+    def _app_scan_interval(self) -> int:
+        """Milliseconds between `/proc` scans, as the server asked.
+
+        Clamped because this comes from a settings document a human edits: a
+        zero would spin, and a minute would let a game run for a minute.
+        """
+        seconds = self._policy.get("poll_seconds") or self._config.poll_seconds
+        try:
+            seconds = int(seconds)
+        except (TypeError, ValueError):
+            seconds = self._config.poll_seconds
+        return max(1, min(30, seconds)) * 1000
+
     def _on_tasks(self, payload: dict[str, Any]) -> None:
         self._tasks = list(payload.get("tasks", []))
         self._task_counts = dict(payload.get("counts", {}))
@@ -818,6 +843,7 @@ class Controller(QObject):
         """Hand everything back: inhibits, DND, the hosts file and the DNS route."""
         self._tick.stop()
         self._idle_check.stop()
+        self._app_scan.stop()
         self._screensaver.release()
         self._dnd.release()
         self._launcher.clear()