Stop the recovery tick stealing our own grab, and fix pre-commit for the monorepo

`_holds_grab` asked `grab_current() is self._root`, but our own transient
children take the grab for themselves: a posted Tk menu grabs the menu
window, so the test read "the grab was lost" and `grab_set_global()` took it
back a second later, off our own widget mid-interaction. That is what made
screen-locker's sport selector unusable while locked on 2026-07-26. The
predicate is now `grab_current() is not None` -- any window we own means the
lock still has pointer and keyboard -- and it is public, `holds_grab`, so an
embedder can ask the same question instead of re-deriving it. Anything that
does not resolve against our widget map (a Tcl-created `ttk.Combobox`
popdown, a stale name) raises `KeyError` and counts as lost: fail closed,
because losing a popdown beats believing we hold a grab we do not. The
blind spot for a second `tk.Tk()` reporting its root as "." is unchanged from
the old test, and now written down.

`_verify` and `_drain` scheduled the next tick *after* running it, so any
exception ended all re-assertion of coverage, grab and VT for the rest of the
lock -- the exact fail-open this module exists to prevent, in a file whose
docstring is a monotonicity proof. The reschedule moves into `finally`, and
the warnings only fire when the reason changes: these states can last the
whole lock, and a 1Hz journal stream is not the alternative to silence.

The pre-commit side: both subproject configs were written as if cwd were the
subproject, but pre-commit runs hooks from the git root -- which is ~/utils,
not gatelock. Every pushed run since at least 2026-07-11 has been red, and
for compounding reasons: pylint and bandit could not find their config,
bandit's tests exclusion never matched, the pytest hook ran with no config at
all, and with no `files:` filter each config linted the *whole* monorepo, so
gatelock's mypy was type-checking crdt-sync. All paths are now root-relative,
each config is scoped to its own subproject, and the pytest hook goes through
a shared `scripts/run_subproject_tests.sh` that supplies the missing `cd` and
fails loudly if the subproject is gone.

Scoping via `files:` has one sharp edge -- rename the directory and the regex
matches nothing, so every hook passes on zero files, green and useless. An
`always_run` scope-guard hook turns that silent pass into a failure.

bandit 1.7.10 threw inside its AST visitor on Python 3.14 and *skipped* every
file while still reporting zero issues; 1.9.4 actually scans. Codespell was
also flagging a real typo the noise had buried.

Housekeeping the above turned up: `gatelock/.github/` and `crdt-sync/.github/`
were dead -- GitHub only reads the root `.github/workflows`, which already has
all four jobs -- and the two subproject `.gitignore`s were byte-identical to
each other and subsumed by the new root one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Krzysztof Rudnicki 2026-07-26 15:08:41 +02:00
parent 7327c287f4
commit 39e52d07c2
14 changed files with 307 additions and 145 deletions

17
.gitignore vendored Normal file
View File

@ -0,0 +1,17 @@
# Python build/test artifacts, across every subproject in this monorepo.
__pycache__/
*.py[cod]
*.egg-info/
build/
dist/
.venv/
.mypy_cache/
.ruff_cache/
.pytest_cache/
# Coverage output. Each subproject writes these next to its own pyproject.toml,
# and a run from the repo root drops a pair here too.
.coverage
.coverage.*
coverage.lcov
htmlcov/

View File

@ -1,19 +0,0 @@
name: pre-commit
on:
push:
branches: [main]
pull_request:
branches: [main]
jobs:
pre-commit:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: actions/setup-python@v5
with:
python-version: "3.12"
- name: Install dependencies
run: pip install -r requirements.txt
- uses: pre-commit/action@v3.0.1

View File

@ -1,28 +0,0 @@
name: Tests
on:
push:
branches: [main]
pull_request:
branches: [main]
jobs:
test:
runs-on: ubuntu-latest
strategy:
matrix:
python-version: ["3.10", "3.11", "3.12"]
steps:
- uses: actions/checkout@v4
- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v5
with:
python-version: ${{ matrix.python-version }}
- name: Install dependencies
run: pip install -r requirements.txt
- name: Run tests with coverage
run: python -m pytest -v

12
crdt-sync/.gitignore vendored
View File

@ -1,12 +0,0 @@
__pycache__/
*.pyc
.venv/
.mypy_cache/
.ruff_cache/
.pytest_cache/
.coverage
coverage.lcov
htmlcov/
dist/
build/
*.egg-info/

View File

@ -3,6 +3,14 @@
# Run: pre-commit run --all-files # Run: pre-commit run --all-files
# Update: pre-commit autoupdate # Update: pre-commit autoupdate
# NOTE: this subproject lives inside the ~/utils monorepo, whose git root is
# one level up. pre-commit always runs hooks from the git root, so every path
# below is written relative to THAT, not to this directory, and the top-level
# `files` key keeps these hooks off the sibling subprojects. CI invokes this
# config exactly the same way:
# pre-commit run --config crdt-sync/.pre-commit-config.yaml --all-files
files: ^crdt-sync/
default_language_version: default_language_version:
python: python3 python: python3
default_stages: [pre-commit] default_stages: [pre-commit]
@ -32,6 +40,14 @@ repos:
- repo: local - repo: local
hooks: hooks:
- id: subproject-scope
name: subproject scope regex still matches
entry: scripts/check_subproject_scope.sh crdt-sync
language: script
# always_run so the top-level `files` filter cannot skip the one hook
# whose job is to notice that filter has stopped matching anything.
always_run: true
pass_filenames: false
- id: no-noqa - id: no-noqa
name: Block noqa comments name: Block noqa comments
entry: '(?i)#\s*(noqa|type:\s*ignore)' entry: '(?i)#\s*(noqa|type:\s*ignore)'
@ -86,7 +102,7 @@ repos:
hooks: hooks:
- id: pylint - id: pylint
args: args:
- --rcfile=pyproject.toml - --rcfile=crdt-sync/pyproject.toml
- --fail-under=8.0 - --fail-under=8.0
- --jobs=4 - --jobs=4
additional_dependencies: additional_dependencies:
@ -94,24 +110,24 @@ repos:
- requests - requests
- repo: https://github.com/PyCQA/bandit - repo: https://github.com/PyCQA/bandit
rev: 1.7.10 rev: 1.9.4
hooks: hooks:
- id: bandit - id: bandit
args: args:
- -c - -c
- pyproject.toml - crdt-sync/pyproject.toml
- --severity-level=high - --severity-level=high
- --confidence-level=medium - --confidence-level=medium
- --skip=B113 - --skip=B113
additional_dependencies: ["bandit[toml]"] additional_dependencies: ["bandit[toml]"]
exclude: ^(tests/|.*test.*\.py$) exclude: ^(crdt-sync/crdt_sync/tests/|.*test.*\.py$)
- repo: local - repo: local
hooks: hooks:
- id: pytest-coverage - id: pytest-coverage
name: pytest with coverage enforcement name: pytest with coverage enforcement
entry: python -m pytest entry: scripts/run_subproject_tests.sh crdt-sync
language: system language: script
types: [python] types: [python]
pass_filenames: false pass_filenames: false
require_serial: true require_serial: true

View File

@ -1,19 +0,0 @@
name: pre-commit
on:
push:
branches: [main]
pull_request:
branches: [main]
jobs:
pre-commit:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: actions/setup-python@v5
with:
python-version: "3.12"
- name: Install dependencies
run: pip install -r requirements.txt
- uses: pre-commit/action@v3.0.1

View File

@ -1,28 +0,0 @@
name: Tests
on:
push:
branches: [main]
pull_request:
branches: [main]
jobs:
test:
runs-on: ubuntu-latest
strategy:
matrix:
python-version: ["3.10", "3.11", "3.12"]
steps:
- uses: actions/checkout@v4
- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v5
with:
python-version: ${{ matrix.python-version }}
- name: Install dependencies
run: pip install -r requirements.txt
- name: Run tests with coverage
run: python -m pytest -v

12
gatelock/.gitignore vendored
View File

@ -1,12 +0,0 @@
__pycache__/
*.pyc
.venv/
.mypy_cache/
.ruff_cache/
.pytest_cache/
.coverage
coverage.lcov
htmlcov/
dist/
build/
*.egg-info/

View File

@ -3,6 +3,14 @@
# Run: pre-commit run --all-files # Run: pre-commit run --all-files
# Update: pre-commit autoupdate # Update: pre-commit autoupdate
# NOTE: this subproject lives inside the ~/utils monorepo, whose git root is
# one level up. pre-commit always runs hooks from the git root, so every path
# below is written relative to THAT, not to this directory, and the top-level
# `files` key keeps these hooks off the sibling subprojects. CI invokes this
# config exactly the same way:
# pre-commit run --config gatelock/.pre-commit-config.yaml --all-files
files: ^gatelock/
default_language_version: default_language_version:
python: python3 python: python3
default_stages: [pre-commit] default_stages: [pre-commit]
@ -32,6 +40,14 @@ repos:
- repo: local - repo: local
hooks: hooks:
- id: subproject-scope
name: subproject scope regex still matches
entry: scripts/check_subproject_scope.sh gatelock
language: script
# always_run so the top-level `files` filter cannot skip the one hook
# whose job is to notice that filter has stopped matching anything.
always_run: true
pass_filenames: false
- id: no-noqa - id: no-noqa
name: Block noqa comments name: Block noqa comments
entry: '(?i)#\s*(noqa|type:\s*ignore)' entry: '(?i)#\s*(noqa|type:\s*ignore)'
@ -84,31 +100,31 @@ repos:
hooks: hooks:
- id: pylint - id: pylint
args: args:
- --rcfile=pyproject.toml - --rcfile=gatelock/pyproject.toml
- --fail-under=8.0 - --fail-under=8.0
- --jobs=4 - --jobs=4
additional_dependencies: additional_dependencies:
- pytest - pytest
- repo: https://github.com/PyCQA/bandit - repo: https://github.com/PyCQA/bandit
rev: 1.7.10 rev: 1.9.4
hooks: hooks:
- id: bandit - id: bandit
args: args:
- -c - -c
- pyproject.toml - gatelock/pyproject.toml
- --severity-level=high - --severity-level=high
- --confidence-level=medium - --confidence-level=medium
- --skip=B113 - --skip=B113
additional_dependencies: ["bandit[toml]"] additional_dependencies: ["bandit[toml]"]
exclude: ^(tests/|.*test.*\.py$) exclude: ^(gatelock/gatelock/tests/|.*test.*\.py$)
- repo: local - repo: local
hooks: hooks:
- id: pytest-coverage - id: pytest-coverage
name: pytest with coverage enforcement name: pytest with coverage enforcement
entry: python -m pytest entry: scripts/run_subproject_tests.sh gatelock
language: system language: script
types: [python] types: [python]
pass_filenames: false pass_filenames: false
require_serial: true require_serial: true

View File

@ -90,6 +90,7 @@ class RecoveryLoop:
self._ticks = 0 self._ticks = 0
self._drain_job: str | None = None self._drain_job: str | None = None
self._verify_job: str | None = None self._verify_job: str | None = None
self._last_grab_warning: str | None = None
@property @property
def ticks(self) -> int: def ticks(self) -> int:
@ -126,15 +127,32 @@ class RecoveryLoop:
def _drain(self) -> None: def _drain(self) -> None:
"""Run a full tick only if a push signal arrived. Cheap otherwise.""" """Run a full tick only if a push signal arrived. Cheap otherwise."""
if self._detector.take_pending(): try:
_logger.debug("output-change signal received; re-asserting the lock") if self._detector.take_pending():
self.tick() _logger.debug("output-change signal received; re-asserting the lock")
self._schedule_drain() self.tick()
# Same fail-open reasoning as _verify.
except Exception:
_logger.exception("drain tick raised; the lock loop keeps running")
finally:
self._schedule_drain()
def _verify(self) -> None: def _verify(self) -> None:
"""Run a full tick unconditionally.""" """Run a full tick unconditionally.
self.tick()
self._schedule_verify() The reschedule is in ``finally`` because it is the only thing keeping
the loop alive: a Tk ``after`` callback that raises just logs and
returns, so scheduling *after* ``tick()`` meant one unexpected
exception silently ended all re-assertion of coverage, grab and VT for
the rest of the lock. Fail-open is the one outcome this module exists
to prevent, so a broken tick must still leave the next one queued.
"""
try:
self.tick()
except Exception:
_logger.exception("verify tick raised; the lock loop keeps running")
finally:
self._schedule_verify()
def tick(self) -> RecoveryReport: def tick(self) -> RecoveryReport:
"""Re-assert coverage, placement, grab and VT. Never weakens any of them. """Re-assert coverage, placement, grab and VT. Never weakens any of them.
@ -198,7 +216,7 @@ class RecoveryLoop:
""" """
if self._config.resolved_grab() != "global": if self._config.resolved_grab() != "global":
return False return False
if self._holds_grab(): if self.holds_grab():
return False return False
with contextlib.suppress(tk.TclError): with contextlib.suppress(tk.TclError):
self._root.grab_set_global() self._root.grab_set_global()
@ -206,13 +224,59 @@ class RecoveryLoop:
return True return True
return False return False
def _holds_grab(self) -> bool: def holds_grab(self) -> bool:
"""Whether our root still owns the grab.""" """Whether the grab is held by a window this application owns.
Deliberately *not* ``grab_current() is self._root``. Our own transient
children take the grab for themselves: a posted Tk menu grabs the menu
window, and the old identity test read that as "the grab was lost" and
called ``grab_set_global()`` a second later, stealing it back off our
own widget mid-interaction. That is what made the screen-locker sport
selector unusable while locked on 2026-07-26. Any window we own still
means the lock has the pointer and keyboard.
``grab_current`` resolves the name Tcl gives it against our own widget
map, so anything that does not resolve -- a Tcl-created popdown such as
a ``ttk.Combobox``'s, or a stale name -- raises ``KeyError`` and is NOT
provably ours. That counts as lost and the grab is re-taken: fail
closed, because this module exists to keep the lock, and losing a
popdown beats believing we hold a grab we do not.
Known limitation: a second ``tk.Tk()`` in the same process reports its
root as ``"."`` too, which is indistinguishable from ours. The old
identity test had exactly the same blind spot. ``_heat_skip``'s
throwaway root is the only other one in screen-locker, and it is
destroyed before the lock window exists.
Public so an embedder (or a verification harness) can ask the same
question the loop asks, rather than re-implementing it and drifting.
"""
try: try:
current = self._root.grab_current() held = self._root.grab_current() is not None
except tk.TclError: except KeyError as exc:
self._warn_once(
f"the grab is held by {exc.args[0]!r}, not one of our windows"
)
return False return False
return current is self._root except tk.TclError:
self._warn_once("could not read the current grab")
return False
if held:
self._last_grab_warning = None
return held
def _warn_once(self, reason: str) -> None:
"""Warn about a lost grab, but only when the reason changes.
These states can persist for the whole lock (a dark display, a popdown
that keeps re-grabbing), and this runs once a second -- warning every
tick would push a line per second into the journal for as long as it
lasts. Silence is not the alternative: the first occurrence and every
change of cause are still logged at WARNING.
"""
if reason != self._last_grab_warning:
_logger.warning("%s; treating it as lost and re-taking it", reason)
self._last_grab_warning = reason
def _reassert_vt(self) -> bool: def _reassert_vt(self) -> bool:
"""Periodically re-disable VT switching. Never re-enables it. """Periodically re-disable VT switching. Never re-enables it.

View File

@ -133,7 +133,7 @@ class TestParseXrandrQuery:
assert output.rect is None assert output.rect is None
def test_garbage_and_empty(self) -> None: def test_garbage_and_empty(self) -> None:
"""Unparseable input yields no outputs rather than raising.""" """Unparsable input yields no outputs rather than raising."""
assert parse_xrandr_query(XRANDR_GARBAGE) == () assert parse_xrandr_query(XRANDR_GARBAGE) == ()
assert parse_xrandr_query("") == () assert parse_xrandr_query("") == ()

View File

@ -8,6 +8,7 @@ lock never weakens, and per-branch coverage.
from __future__ import annotations from __future__ import annotations
import ast import ast
import logging
from pathlib import Path from pathlib import Path
import tkinter as tk import tkinter as tk
from unittest.mock import MagicMock, patch from unittest.mock import MagicMock, patch
@ -190,6 +191,57 @@ class TestGrabReassertion:
mock_root.grab_current.return_value = mock_root mock_root.grab_current.return_value = mock_root
assert recovery.tick().grab_reasserted is False assert recovery.tick().grab_reasserted is False
def test_noop_when_our_own_child_holds_the_grab(
self, loop: tuple[RecoveryLoop, MagicMock, SurfaceSet], mock_root: MagicMock
) -> None:
"""A posted menu owns the grab; we must not steal it back.
Regression: the old ``grab_current() is root`` test read a Tk menu's
own grab as "lost" and re-grabbed a second later, which made the
screen-locker sport selector unusable while locked.
"""
recovery, enumerator, _surfaces = loop
enumerator.scan.return_value = BOTH
mock_root.grab_current.return_value = MagicMock(name="posted menu")
assert recovery.tick().grab_reasserted is False
mock_root.grab_set_global.assert_not_called()
def test_reasserts_when_the_holder_is_not_one_of_our_windows(
self,
loop: tuple[RecoveryLoop, MagicMock, SurfaceSet],
mock_root: MagicMock,
caplog: pytest.LogCaptureFixture,
) -> None:
"""Fail closed: a name we cannot resolve is not provably ours.
A ``ttk.Combobox`` popdown is a Tcl-created toplevel absent from
Tkinter's widget map, so ``nametowidget`` raises ``KeyError``. Keeping
the lock beats keeping a banned widget's popdown.
"""
recovery, enumerator, _surfaces = loop
enumerator.scan.return_value = BOTH
mock_root.grab_current.side_effect = KeyError("!popdown")
with caplog.at_level(logging.WARNING):
assert recovery.tick().grab_reasserted is True
mock_root.grab_set_global.assert_called()
assert "not one of our windows" in caplog.text
def test_a_persistent_lost_grab_warns_once_not_once_per_tick(
self,
loop: tuple[RecoveryLoop, MagicMock, SurfaceSet],
mock_root: MagicMock,
caplog: pytest.LogCaptureFixture,
) -> None:
"""A state that lasts the whole lock must not stream to the journal."""
recovery, enumerator, _surfaces = loop
enumerator.scan.return_value = BOTH
mock_root.grab_current.side_effect = tk.TclError("gone")
with caplog.at_level(logging.WARNING):
for _ in range(5):
recovery.tick()
lost = [r for r in caplog.records if "current grab" in r.message]
assert len(lost) == 1, f"warned {len(lost)} times over 5 ticks"
def test_tclerror_while_regrabbing_is_swallowed( def test_tclerror_while_regrabbing_is_swallowed(
self, loop: tuple[RecoveryLoop, MagicMock, SurfaceSet], mock_root: MagicMock self, loop: tuple[RecoveryLoop, MagicMock, SurfaceSet], mock_root: MagicMock
) -> None: ) -> None:
@ -200,7 +252,7 @@ class TestGrabReassertion:
mock_root.grab_set_global.side_effect = tk.TclError("held") mock_root.grab_set_global.side_effect = tk.TclError("held")
assert recovery.tick().grab_reasserted is False assert recovery.tick().grab_reasserted is False
def test_grab_current_tclerror( def test_grab_query_tclerror(
self, loop: tuple[RecoveryLoop, MagicMock, SurfaceSet], mock_root: MagicMock self, loop: tuple[RecoveryLoop, MagicMock, SurfaceSet], mock_root: MagicMock
) -> None: ) -> None:
"""A TclError querying the grab counts as not holding it.""" """A TclError querying the grab counts as not holding it."""
@ -286,6 +338,37 @@ class TestScheduling:
recovery._verify() recovery._verify()
assert recovery.ticks == 1 assert recovery.ticks == 1
def test_verify_reschedules_even_when_the_tick_raises(
self,
loop: tuple[RecoveryLoop, MagicMock, SurfaceSet],
mock_root: MagicMock,
caplog: pytest.LogCaptureFixture,
) -> None:
"""A broken tick must not end the loop -- that would fail open."""
recovery, enumerator, _surfaces = loop
enumerator.scan.side_effect = RuntimeError("boom")
recovery._running = True
with caplog.at_level(logging.ERROR):
recovery._verify()
mock_root.after.assert_called_once()
assert "verify tick raised" in caplog.text
def test_drain_reschedules_even_when_the_tick_raises(
self,
loop: tuple[RecoveryLoop, MagicMock, SurfaceSet],
mock_root: MagicMock,
caplog: pytest.LogCaptureFixture,
) -> None:
"""Same for the cheap cadence."""
recovery, enumerator, _surfaces = loop
enumerator.scan.side_effect = RuntimeError("boom")
recovery._detector.take_pending.return_value = True
recovery._running = True
with caplog.at_level(logging.ERROR):
recovery._drain()
mock_root.after.assert_called_once()
assert "drain tick raised" in caplog.text
def test_stop_cancels_and_halts( def test_stop_cancels_and_halts(
self, loop: tuple[RecoveryLoop, MagicMock, SurfaceSet], mock_root: MagicMock self, loop: tuple[RecoveryLoop, MagicMock, SurfaceSet], mock_root: MagicMock
) -> None: ) -> None:

View File

@ -0,0 +1,40 @@
#!/bin/bash
# ============================================================================
# Fail if a subproject's pre-commit config is scoped to a directory that has
# moved or vanished.
#
# Each subproject config carries `files: ^<subproject>/`. If the directory is
# renamed, that regex matches nothing and EVERY hook in the config reports
# "no files to check" -- green CI with no linting at all. This hook runs with
# always_run, so the top-level filter cannot skip it, and turns that silent
# pass into a loud failure.
#
# Usage: scripts/check_subproject_scope.sh <subproject>
# ============================================================================
set -euo pipefail
readonly REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
main() {
if [[ $# -ne 1 ]]; then
echo "Usage: $(basename "$0") <subproject>" >&2
exit 1
fi
local subproject="$1"
local config="$REPO_ROOT/$subproject/.pre-commit-config.yaml"
if [[ ! -f "$config" ]]; then
echo "Error: $subproject/.pre-commit-config.yaml is missing." >&2
echo " Every hook scoped to '^$subproject/' would silently pass." >&2
exit 1
fi
if ! grep -qx "files: \^$subproject/" "$config"; then
echo "Error: $config does not scope itself to '^$subproject/'." >&2
echo " Hooks would run over the whole monorepo, or over nothing." >&2
exit 1
fi
}
main "$@"

44
scripts/run_subproject_tests.sh Executable file
View File

@ -0,0 +1,44 @@
#!/bin/bash
# ============================================================================
# Run one subproject's test suite from that subproject's directory.
#
# Every subproject here keeps its own pyproject.toml, and its pytest config
# (testpaths, --cov=..., coverage source) is relative to that directory --
# but pre-commit always runs hooks from the monorepo's git root. This wrapper
# supplies the missing `cd`, and fails loudly if the subproject is gone,
# rather than silently testing nothing.
#
# Usage: scripts/run_subproject_tests.sh <subproject> [pytest args...]
# ============================================================================
set -euo pipefail
readonly REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
readonly SCRIPT_NAME="$(basename "$0")"
usage() {
echo "Usage: $SCRIPT_NAME <subproject> [pytest args...]"
echo " <subproject> directory under $REPO_ROOT holding a pyproject.toml"
exit 1
}
main() {
if [[ $# -lt 1 ]]; then
usage
fi
local subproject="$1"
shift
local project_dir="$REPO_ROOT/$subproject"
if [[ ! -f "$project_dir/pyproject.toml" ]]; then
echo "Error: no $subproject/pyproject.toml under $REPO_ROOT" >&2
echo " (was the subproject renamed or removed?)" >&2
exit 1
fi
cd "$project_dir"
exec python -m pytest "$@"
}
main "$@"