Problem
Independent review of PR #15 (head 9e58558) confirmed the direction but found lifecycle guarantees that are stated stronger than the code ensures. Evidence: full-diff review of src/kwin_mcp/input.py at 9e58558 + libei 1.6.0 API docs (ei_event_unref refcount contract, PAUSED neutral-state reset, DISCONNECT is the last event, sequence +1 rule) + precedent Python clients using try/finally around ei_event_unref.
Findings (all in src/kwin_mcp/input.py unless noted):
- F1.
_drain_events() leaks the event reference if _handle_event() raises — ei_event_unref() is skipped. Central dispatcher for handshake/flush/readiness-wait, so one unexpected handler error leaks an FFI reference.
- F2.
_remove_device() / _handle_seat_removed() unref the same device handle once per slot in _DEVICE_ATTRS. _register_device() can place one multi-capability handle into several slots → double/triple unref of a live handle.
- F3.
_flush() with a released context (_ei == 0) marks dead and returns silently — a success report for an injection that never reached libei. Inconsistent with the #234 honest-delivery contract (dispatch failure raises ToolError).
- F4.
_teardown_connection() docstring promises cleanup "can never abort halfway", but ei_device_stop_emulating / ei_device_unref / ei_unref are unguarded — one failure aborts the remaining slots/context cleanup.
- F5.
_drain_events() keeps processing queued events after a DISCONNECT (which only sets _connection_dead) — a RESUMED later in the same queue can start_emulating + replay held state on a dead connection.
- F6.
_invalidate_touches() sends ei_touch_up() on teardown/dead connections. Releasing (unref + drop) is required; sending up into a dead context is at best meaningless.
- F7. Reconnect budget is count-only (
_RECONNECT_ATTEMPTS = 3, handshake timeout=5.0, re-check 0.5): worst case is far above the "3–4s" the comment claims, with no wall-clock bound per MCP call.
- F8.
_wait_emulating() does select(0.05) before draining, so already-queued events pay up to 50ms before the readiness probe sees them, despite the drain-before-probe docstring.
Desired result
- F1: event unref'd exactly once on every path through
_drain_events (regression test: raising handler → unref still called).
- F2: a handle occupying several slots is unref'd exactly once on remove/seat-removed (regression test: multi-cap device remove → single unref per unique handle).
- F3:
_flush() on a released context surfaces a clean ToolError instead of silent success.
- F4: teardown completes all slots + context reset even if one step fails (or the docstring is weakened to match — code fix preferred); regression test.
- F5: drain stops after
DISCONNECT (no start_emulating/replay on the dead connection); regression test.
- F6: teardown/invalidate path never sends
touch_up on a dead connection (split send vs invalidate, or guard on liveness); regression test.
- F7: an overall recovery wall-clock deadline bounds one call alongside the attempt count; comment states the honest worst case.
- F8: already-queued events are drained+probed before the first
select wait.
- Full suite green:
uv run pytest tests/ -q, uv run ruff check ., uv run ruff format --check ., uv run ty check, uv build. Commits pushed to the PR branch fix/01sw-port (same-branch fix loop, no new branch/PR).
Constraints and out of scope
- Scope:
src/kwin_mcp/input.py, lifecycle/reconnect tests under tests/, CHANGELOG.md entries for behavior changes (F3/F5/F7). Nothing else.
- Out of scope:
core.py (screen auto-detect, incl. any rename), server.py, EISConnection extraction, held-state replay redesign, new live-KWin integration tests, existing-test weakening (a test change is allowed only if it encoded superseded behavior, with justification).
- All comments/docstrings in English; ruff line-length 100, double quotes, type hints on signatures.
Problem
Independent review of PR #15 (
head9e58558) confirmed the direction but found lifecycle guarantees that are stated stronger than the code ensures. Evidence: full-diff review ofsrc/kwin_mcp/input.pyat9e58558+ libei 1.6.0 API docs (ei_event_unrefrefcount contract,PAUSEDneutral-state reset,DISCONNECTis the last event, sequence+1rule) + precedent Python clients usingtry/finallyaroundei_event_unref.Findings (all in
src/kwin_mcp/input.pyunless noted):_drain_events()leaks the event reference if_handle_event()raises —ei_event_unref()is skipped. Central dispatcher for handshake/flush/readiness-wait, so one unexpected handler error leaks an FFI reference._remove_device()/_handle_seat_removed()unref the same device handle once per slot in_DEVICE_ATTRS._register_device()can place one multi-capability handle into several slots → double/tripleunrefof a live handle._flush()with a released context (_ei == 0) marks dead and returns silently — a success report for an injection that never reached libei. Inconsistent with the #234 honest-delivery contract (dispatch failure raisesToolError)._teardown_connection()docstring promises cleanup "can never abort halfway", butei_device_stop_emulating/ei_device_unref/ei_unrefare unguarded — one failure aborts the remaining slots/context cleanup._drain_events()keeps processing queued events after aDISCONNECT(which only sets_connection_dead) — aRESUMEDlater in the same queue canstart_emulating+ replay held state on a dead connection._invalidate_touches()sendsei_touch_up()on teardown/dead connections. Releasing (unref + drop) is required; sendingupinto a dead context is at best meaningless._RECONNECT_ATTEMPTS = 3, handshaketimeout=5.0, re-check0.5): worst case is far above the "3–4s" the comment claims, with no wall-clock bound per MCP call._wait_emulating()doesselect(0.05)before draining, so already-queued events pay up to 50ms before the readiness probe sees them, despite the drain-before-probe docstring.Desired result
_drain_events(regression test: raising handler → unref still called)._flush()on a released context surfaces a cleanToolErrorinstead of silent success.DISCONNECT(nostart_emulating/replay on the dead connection); regression test.touch_upon a dead connection (split send vs invalidate, or guard on liveness); regression test.selectwait.uv run pytest tests/ -q,uv run ruff check .,uv run ruff format --check .,uv run ty check,uv build. Commits pushed to the PR branchfix/01sw-port(same-branch fix loop, no new branch/PR).Constraints and out of scope
src/kwin_mcp/input.py, lifecycle/reconnect tests undertests/,CHANGELOG.mdentries for behavior changes (F3/F5/F7). Nothing else.core.py(screen auto-detect, incl. any rename),server.py, EISConnection extraction, held-state replay redesign, new live-KWin integration tests, existing-test weakening (a test change is allowed only if it encoded superseded behavior, with justification).