Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 38 additions & 5 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ on:
branches: [main]
tags: ["v*"]
pull_request:
branches: [main]

jobs:
lint:
Expand All @@ -16,8 +15,15 @@ jobs:
steps:
- uses: actions/checkout@v6
- uses: astral-sh/setup-uv@v7
- run: uvx --python ${{ matrix.python-version }} ruff check src/
- run: uvx --python ${{ matrix.python-version }} ruff format --check src/
- run: uvx --python ${{ matrix.python-version }} ruff check src/ tests/ scripts/
- run: uvx --python ${{ matrix.python-version }} ruff format --check src/ tests/ scripts/

guards:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v6
- name: Run CI guards
run: bash scripts/ci_guards.sh

type-check:
runs-on: ubuntu-latest
Expand All @@ -28,7 +34,34 @@ jobs:
run: |
sudo apt-get update
sudo apt-get install -y libcairo2-dev libgirepository-2.0-dev libdbus-1-dev pkg-config
- run: uv run ty check src/
- run: uv run ty check src/ tests/ scripts/

test:
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
# Skip 3.15 (alpha): rpds-py has no wheel and Rust source build fails on cpython-3.15.0a8.
python-version: ["3.12", "3.13", "3.14"]
steps:
- uses: actions/checkout@v6
- uses: astral-sh/setup-uv@v7
- name: Install system dependencies
run: |
sudo apt-get update
sudo apt-get install -y libcairo2-dev libgirepository-2.0-dev libdbus-1-dev pkg-config
- name: Run non-kwin pytest (tolerate exit 5 = no tests collected)
# Ignore tests/integration: those tests require a real KWin/Wayland session
# and import kwin_mcp.input which dlopens libei.so.1 (not available on GHA).
run: |
set +e
uv run --python ${{ matrix.python-version }} pytest -m "not kwin" --ignore=tests/integration
rc=$?
set -e
if [ "$rc" -eq 0 ] || [ "$rc" -eq 5 ]; then
exit 0
fi
exit "$rc"

build:
runs-on: ubuntu-latest
Expand All @@ -48,7 +81,7 @@ jobs:

publish:
if: startsWith(github.ref, 'refs/tags/v')
needs: [lint, type-check, build]
needs: [lint, type-check, test, build, guards]
runs-on: ubuntu-latest
environment: pypi
permissions:
Expand Down
1 change: 0 additions & 1 deletion .github/workflows/docs-seo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ name: Docs & SEO Review
# when the detected changes do not warrant a documentation review.
on:
pull_request:
branches: [main]
paths:
- "src/kwin_mcp/**"
- "pyproject.toml"
Expand Down
15 changes: 15 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Added

- `dbus_call` MCP tool now accepts typed-JSON dicts (`{"type": "int32", "value": 42}`) alongside legacy dbus-send strings (`"int32:42"`); both shapes can mix freely in one `args` list. Container shapes supported: `array` (basic element types), `dict` (basic key + value types), `variant` (basic value type).
- New module `kwin_mcp.dbus_args` exposing `parse_dbus_send_arg`, `parse_typed_arg`, `parse_arg`, and `to_dbus_send_string` (80 unit tests).

### Changed

- `dbus_call` reply formatting: single-primitive returns are now the bare value (e.g. `org.freedesktop.DBus.GetId` returns just the UUID string instead of the multi-line `method return … string "…"` block); container and multi-value returns are JSON. Error replies use the format `D-Bus error: <error-name>: <message>` instead of raw `dbus-send` stderr text. Callers that parse the structured value see equivalent data; callers grepping for the literal `method return` prefix must update.

### Internal

- Replaced `subprocess.run(["dbus-send", …])` inside `dbus_call` with in-process `dbus.bus.BusConnection` + `dbus.Interface`, removing one fork+exec per call and surfacing structured `dbus.DBusException` instead of CLI exit codes.
- Replaced per-call `subprocess.run([sys.executable, "-m", "kwin_mcp.accessibility", …])` for AT-SPI queries with a long-lived spawn-context `multiprocessing.Pool(processes=1)` worker. New module `kwin_mcp.accessibility_worker` is never imported by the parent process (CI guard 3 enforced); worker init validates `Atspi.get_desktop(0).get_child_count() >= 0` against the bus address. Cold start ≤ 1.5s, warm calls < 200ms (vs ~700ms per subprocess). Pool teardown protocol: `close → join 5s → terminate → join 2s → SIGKILL → join 1s` survives hung workers within 7s. External `SIGKILL` of the worker is recovered by lazy pool recreation on the next call.
- Screenshot backend selection now runs an eager capability probe at session_start. `_probe_screenshot_capability(dbus_address, wayland_socket)` tries a 1×1 `CaptureArea` via in-process ScreenShot2 D-Bus first; on any failure it falls back to a real `spectacle` capture. The result (`screenshot2_dbus` / `spectacle_cli` / `unavailable`) is stored on `SessionInfo.screenshot_backend` and consumed by `capture_screenshot_to_file` and `capture_frame_burst` via an explicit keyword argument — no global state. `unavailable` raises `RuntimeError("No screenshot backend available; install spectacle or fix KWin EglBackend").`

### Fixed

- Segfault on Python 3.14 caused by missing `argtypes` on variadic `ei_seat_bind_capabilities` ctypes call
Expand Down
4 changes: 4 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,11 @@ See ROADMAP.md. Key modules:
| `src/kwin_mcp/server.py` | tool-registration, code-general | Check tool count, README tool tables, CONTRIBUTING structure, **integrations/*/skills/*/SKILL.md tool list freshness** |
| `src/kwin_mcp/session.py` | session-api | Check README arch diagram, CONTRIBUTING session docs |
| `src/kwin_mcp/core.py` | engine-api, code-general | Check README arch description, CONTRIBUTING structure |
| `src/kwin_mcp/dbus_args.py` | code-general | Check concrete numbers, CONTRIBUTING file listing, **integrations/*/skills/*/SKILL.md "30 capabilities" reference** |
| `src/kwin_mcp/window.py` | engine-api, code-general | Check README arch description, CONTRIBUTING structure |
| `src/kwin_mcp/accessibility_worker.py` | engine-api, code-general | Check README arch description, CONTRIBUTING structure |
| `src/kwin_mcp/*.py` (any) | code-general | Check concrete numbers, CONTRIBUTING file listing, **integrations/*/skills/*/SKILL.md "30 capabilities" reference** |
| `docs/design/*.md` | code-general | Check docs/design architecture notes and related docs-seo guidance |
| `pyproject.toml` | package-metadata | Sync keywords with `.claude/positioning.yml`; check CLAUDE.md keyword tiers; **run `python3 scripts/sync_plugin_version.py` to propagate version to integrations manifests** |
| `CHANGELOG.md` | changelog-update | Sync docs-seo.md positioning; add new search intents |
| `README.md` | readme-update | Sync docs-seo.md positioning; update CLAUDE.md keyword tiers |
Expand Down
70 changes: 70 additions & 0 deletions docs/design/dbus-call-call-sites.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
# dbus_call Call Site Map

## Summary

- **Total internal code call sites**: 1 (`server.py` → `core.py`)
- **Hard-coded arg patterns in source**: 0 (all args are pass-through from external callers)
- **External API callers**: unlimited (LLM agents / MCP clients pass arbitrary dbus-send syntax)

## Definition Sites

| Location | Role | Current Args Type |
|---|---|---|
| `src/kwin_mcp/core.py:717` | `dbus_call()` method — implementation | `args: list[str] \| None` — passed directly to `dbus-send` CLI |
| `src/kwin_mcp/server.py:751` | `dbus_call` MCP tool — schema + wrapper | Annotated `list[str] \| None`, description mentions dbus-send format |

## Internal Call Sites

| Location | Caller | Service | Path | Interface | Method | Args | Type Complexity |
|---|---|---|---|---|---|---|---|
| `src/kwin_mcp/server.py:769` | MCP tool handler | pass-through | pass-through | pass-through | pass-through | pass-through from MCP user | varies |

**There are no internal call sites with hard-coded arg patterns.** All args originate from external MCP clients (LLM agents).

## External Arg Patterns (from MCP tool docstring)

The `dbus_call` MCP tool description (`server.py:759-768`) documents the accepted format:
```
Method arguments in dbus-send format (e.g. "string:value", "int32:42", "boolean:true")
```

Examples that MCP clients are known to supply (from plan QA scenarios and typical agent behavior):
- `[]` — no args (e.g., `org.freedesktop.DBus.GetId`, `org.kde.KWin.showDesktop`)
- `["string:hello"]` — single basic string arg
- `["int32:42"]` — integer arg
- `["boolean:true"]` — boolean arg
- `["string:hello", "int32:42"]` — mixed basic args

## Required Parser Features

Based on the zero internal hard-coded call sites and the MCP tool's documented usage, the parser in `dbus_args.py` **MUST support** all standard dbus-send basic types because external LLM agents may use any of them:

| Type prefix | Example | Priority |
|---|---|---|
| `string:` | `string:hello` | **Required** — most common |
| `int32:` | `int32:42` | **Required** — most common integer |
| `boolean:` | `boolean:true` | **Required** — frequently used |
| `uint32:` | `uint32:1234` | **Required** — KWin D-Bus methods use uint32 |
| `uint64:` | `uint64:999` | **Required** — timestamps, PIDs |
| `int64:` | `int64:-1` | **Required** — standard type |
| `byte:` | `byte:255` | **Required** — protocol completeness |
| `int16:` | `int16:10` | **Required** — protocol completeness |
| `uint16:` | `uint16:10` | **Required** — protocol completeness |
| `double:` | `double:3.14` | **Required** — used by KWin geometry |
| `objpath:` | `objpath:/org/kde/KWin` | **Required** — object references |
| `signature:` | `signature:s` | **Required** — introspection |
| `array:` | `array:string:a,b,c` | **Required** — lists |
| `dict:` | `dict:string:int32:k:1` | **Required** — D-Bus dicts |
| `variant:` | `variant:string:hello` | **Required** — polymorphic values |

**No types from the dbus-send man page are deferred** — the MCP tool is a public API and agents may use any valid dbus-send syntax.

## Scope Impact on Task 7 (Parser)

Task 7 must implement a full recursive-descent parser for the dbus-send argument syntax. The scope is NOT reducible based on current callers because:

1. The tool is an external API with no internal call sites to scope-cap from
2. LLM agents generate diverse arg patterns based on D-Bus introspection output
3. Supporting only 2-3 types would break real-world agent workflows

**Minimum viable parser scope**: all basic types + `array:`, `dict:`, `variant:` containers + nested containers.
22 changes: 20 additions & 2 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,21 @@ requires = ["uv_build>=0.10.3,<0.12.0"]
build-backend = "uv_build"

[dependency-groups]
dev = ["ruff>=0.11.0", "ty>=0.0.1"]
dev = [
"ruff>=0.11.0",
"ty>=0.0.1",
"pytest>=8.0",
"pytest-asyncio>=0.23",
"pytest-timeout>=2.3.0",
]

[tool.pytest.ini_options]
testpaths = ["tests"]
addopts = "-ra -q"
asyncio_mode = "auto"
timeout = 30
timeout_method = "thread"
markers = ["kwin: marks tests requiring a real KWin virtual session (deselect with -m 'not kwin')"]

[tool.ruff]
target-version = "py312"
Expand Down Expand Up @@ -108,7 +122,11 @@ python-version = "3.12"

# gi.repository and dbus.bus are dynamic modules that cannot be resolved statically
[[tool.ty.overrides]]
include = ["src/kwin_mcp/accessibility.py", "src/kwin_mcp/server.py"]
include = [
"src/kwin_mcp/accessibility.py",
"src/kwin_mcp/accessibility_worker.py",
"src/kwin_mcp/server.py",
]
[tool.ty.overrides.rules]
unresolved-import = "ignore"

Expand Down
162 changes: 162 additions & 0 deletions scripts/ci_guards.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
#!/usr/bin/env bash
# CI Guards - enforce backend overhaul invariants.
#
# Each guard prints a clear failure message identifying the rule and
# the offending location(s), then exits non-zero. On a clean tree,
# the script exits 0 silently (per-guard "ok" lines are emitted to
# stderr for visibility in CI logs).
#
# Guards:
# 1. No reachable `print()` in src/kwin_mcp/ (excluding __main__ blocks)
# 2. Every `kwin_wayland` invocation in tests/, scripts/, docs/ has --virtual
# 3. core.py does not import accessibility_worker at module top
# 4. No os.fork or raw multiprocessing.Process( in src/kwin_mcp/

set -euo pipefail

REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
cd "$REPO_ROOT"

fail=0

log_ok() {
echo "[ci_guards] OK: $1" >&2
}

log_fail() {
echo "[ci_guards] FAIL: $1" >&2
fail=1
}

# ---------------------------------------------------------------------------
# Guard 1: No reachable `print()` in MCP server code paths under src/kwin_mcp/.
# Excludes:
# - __main__.py (module entrypoint guard blocks)
# - cli.py (interactive REPL whose contract IS to write to stdout)
# Rationale: any print() reachable from server.py would corrupt the MCP
# stdio JSON-RPC stream.
# ---------------------------------------------------------------------------
guard_1_no_print() {
local matches
matches="$(grep -rn -E '^[[:space:]]*print\(' src/kwin_mcp/ \
--include='*.py' 2>/dev/null \
| grep -v '__main__' \
| grep -v 'src/kwin_mcp/cli.py:' || true)"
if [[ -n "$matches" ]]; then
log_fail "Guard 1 - reachable print() found in MCP server code paths (forbidden, use logging instead):"
echo "$matches" >&2
return
fi
log_ok "Guard 1 - no reachable print() in MCP server code paths"
}

# ---------------------------------------------------------------------------
# Guard 2: Every kwin_wayland reference in tests/, scripts/, docs/ uses
# --virtual on the same line.
# ---------------------------------------------------------------------------
guard_2_kwin_virtual() {
local matches=""
for d in tests scripts docs; do
if [[ -d "$d" ]]; then
local found
# Exclusions (non-launch references):
# - this guards script itself (documents kwin_wayland)
# - pkill cleanup lines (process kill, not a launch)
# - --version queries (introspection, not a launch)
# - lines whose first non-whitespace token after `path:lineno:` is `echo`
# (documentation strings inside shell scripts)
found="$(grep -rn 'kwin_wayland' "$d" 2>/dev/null \
| grep -v -- '--virtual' \
| grep -v 'scripts/ci_guards.sh' \
| grep -v 'pkill' \
| grep -v -- '--version' \
| grep -Pv '^\S+:\s*echo\s' || true)"
if [[ -n "$found" ]]; then
matches+="$found"$'\n'
fi
fi
done
if [[ -n "$matches" ]]; then
log_fail "Guard 2 - kwin_wayland invocation without --virtual in tests/scripts/docs:"
printf '%s' "$matches" >&2
return
fi
log_ok "Guard 2 - all kwin_wayland references in tests/scripts/docs use --virtual"
}

# ---------------------------------------------------------------------------
# Guard 3: src/kwin_mcp/core.py must not import accessibility_worker at module
# top level. Uses Python AST so conditional/inline imports are tolerated.
# ---------------------------------------------------------------------------
guard_3_no_module_top_worker_import() {
local target="src/kwin_mcp/core.py"
if [[ ! -f "$target" ]]; then
log_ok "Guard 3 - $target absent, skipping"
return
fi
local result
result="$(python3 - "$target" <<'PYEOF'
import ast
import sys

path = sys.argv[1]
with open(path, encoding="utf-8") as f:
tree = ast.parse(f.read(), filename=path)

violations = []
for node in tree.body:
if isinstance(node, ast.Import):
for alias in node.names:
if "accessibility_worker" in alias.name:
violations.append(f"line {node.lineno}: import {alias.name}")
elif isinstance(node, ast.ImportFrom):
mod = node.module or ""
if "accessibility_worker" in mod:
names = ", ".join(a.name for a in node.names)
violations.append(f"line {node.lineno}: from {mod} import {names}")
else:
for alias in node.names:
if "accessibility_worker" in alias.name:
violations.append(
f"line {node.lineno}: from {mod} import {alias.name}"
)

if violations:
print("\n".join(violations))
sys.exit(1)
PYEOF
)" || {
log_fail "Guard 3 - core.py has module-top accessibility_worker import (must be lazy/local):"
echo "$result" >&2
return
}
log_ok "Guard 3 - core.py has no module-top accessibility_worker import"
}

# ---------------------------------------------------------------------------
# Guard 4: No os.fork or raw multiprocessing.Process( in src/kwin_mcp/.
# ---------------------------------------------------------------------------
guard_4_no_fork_or_process() {
local matches
matches="$(grep -rn -E 'os\.fork\b|multiprocessing\.Process\(' src/kwin_mcp/ \
--include='*.py' 2>/dev/null || true)"
if [[ -n "$matches" ]]; then
log_fail "Guard 4 - os.fork or multiprocessing.Process( found in src/kwin_mcp/ (forbidden):"
echo "$matches" >&2
return
fi
log_ok "Guard 4 - no os.fork or multiprocessing.Process( in src/kwin_mcp/"
}

guard_1_no_print
guard_2_kwin_virtual
guard_3_no_module_top_worker_import
guard_4_no_fork_or_process

if [[ "$fail" -ne 0 ]]; then
echo "[ci_guards] One or more guards failed." >&2
exit 1
fi

echo "[ci_guards] All guards passed." >&2
exit 0
Loading