diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 9ed77ec..0be0bd0 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -27,8 +27,10 @@ CI must stay green (`test`, `secrets-hygiene`). Also say what **you** ran: | `uv run python scripts/test_dialog_ergonomics.py` | Pass / Fail / Skip | | | `uv run python scripts/test_windows_backend.py` | Pass / Fail / Skip | | | `uv run python scripts/test_contracts.py` | Pass / Fail / Skip | | +| `uv run python scripts/test_mcq_images.py` | Pass / Fail / Skip | | | `check_setup` in a real host | Pass / Fail / Skip | host: | | `ask_multiple_choice` dialog click | Pass / Fail / Skip | | +| Image preview in Gtk dialog (`image`/`images`) | Pass / Fail / Skip | | | Keyboard 1–8 / Enter / Esc | Pass / Fail / Skip | | | Freeform / Something else | Pass / Fail / Skip | | | Speak / duck (if claiming voice) | Pass / Fail / Skip | | diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 65f549e..813301e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -36,6 +36,7 @@ jobs: uv run python scripts/test_dialog_ergonomics.py uv run python scripts/test_windows_backend.py uv run python scripts/test_contracts.py + uv run python scripts/test_mcq_images.py - name: Import package run: uv run python -c "import ask_question_mcp; print('ok')" diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index dad86e5..911970f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -15,6 +15,7 @@ uv run python scripts/test_install.py uv run python scripts/test_dialog_ergonomics.py uv run python scripts/test_windows_backend.py uv run python scripts/test_contracts.py +uv run python scripts/test_mcq_images.py ``` Optional voice: set `ASK_QUESTION_TTS_URL` and `ASK_QUESTION_STT_URL` to your diff --git a/docs/AGENTS.md b/docs/AGENTS.md index 719e3e7..13b0695 100644 --- a/docs/AGENTS.md +++ b/docs/AGENTS.md @@ -76,6 +76,13 @@ skill via `ask-question-install --skill` (`~/.cursor/skills/ask-multiple-choice` | `agent` | string \| null | **strongly yes** | Window title prefix `[agent]` | | `timeout_sec` | int | no | default `300`; `0` = no timeout | | `entry_seed` | string \| null | no | Prefill Something else / entry | +| `image` | string \| null | no | Local PNG/JPEG (etc.) path or `file://` URI — Gtk preview above the question (Linux). Missing/unsupported files are skipped. | +| `images` | string[] \| null | no | Same as `image`, up to 4 paths (combined with `image`, deduped). Prefer one clear still when possible. | + +**Images in the dialog (Linux Gtk):** pass an absolute path or `file://` URI so +Alex sees the still *inside* the MCQ (not only in chat). Chat `Read` of a PNG +does not put pixels in the dialog — use `image` / `images`. Windows Phase 1 +ignores these args (text-only). Pattern: `mcq-with-image`. ### Example (single choice) @@ -108,6 +115,22 @@ Routine forks stay short (no referent dump): } ``` +### Example (with image preview) + +```json +{ + "question": "Does this rear I/O still look clear enough?", + "title": "Visual check", + "agent": "enclosure-review", + "recommended_id": "ok", + "image": "/abs/path/to/eth-rear-io.png", + "options": [ + { "id": "ok", "label": "Looks good (recommended)" }, + { "id": "redo", "label": "Re-capture" } + ] +} +``` + ### Example (dangerous) ```json diff --git a/scripts/test_mcq_images.py b/scripts/test_mcq_images.py new file mode 100644 index 0000000..ebfd0be --- /dev/null +++ b/scripts/test_mcq_images.py @@ -0,0 +1,47 @@ +#!/usr/bin/env python3 +"""Unit tests for MCQ image path normalization (no DISPLAY).""" + +from __future__ import annotations + +import sys +import tempfile +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] +sys.path.insert(0, str(ROOT / "src")) + +from ask_question_mcp.mcq_images import normalize_mcq_images # noqa: E402 + + +def test_normalize_path_and_file_uri() -> None: + with tempfile.TemporaryDirectory(prefix="askq-img-") as td: + real = Path(td) / "preview.png" + real.write_bytes(b"\x89PNG\r\n\x1a\n" + b"\x00" * 16) + + got = normalize_mcq_images(image=str(real)) + assert got == [str(real.resolve())], got + + uri = real.resolve().as_uri() + got_uri = normalize_mcq_images(image=uri) + assert got_uri == [str(real.resolve())], got_uri + + missing = normalize_mcq_images(image="/no/such/mcq-image-xyz.png") + assert missing == [] + + dup = normalize_mcq_images(image=str(real), images=[str(real), uri]) + assert dup == [str(real.resolve())] + + # Non-image suffix skipped + txt = Path(td) / "notes.txt" + txt.write_text("x", encoding="utf-8") + assert normalize_mcq_images(image=str(txt)) == [] + + +def main() -> int: + test_normalize_path_and_file_uri() + print("test_mcq_images: ok") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/skills/ask-multiple-choice/SKILL.md b/skills/ask-multiple-choice/SKILL.md index 1085167..f529fda 100644 --- a/skills/ask-multiple-choice/SKILL.md +++ b/skills/ask-multiple-choice/SKILL.md @@ -25,7 +25,12 @@ choose among options the human must decide. forks. No meta about dialogs/voice. 4. Mark preferred only as **`Label (recommended)`** + **`recommended_id`**. 5. **`dangerous=true`** for irreversible / high-risk forks. -6. Wait for the JSON result. On cancel → stop. On freeform → use **`freeform_text`**. +6. **Images the human must see in the dialog** (not only in chat): pass + **`image=`** (one path / `file://` URI) or **`images=`** (list, max 4). + Linux Gtk shows a scaled preview above the question (~320px max height). + Chat `Read` of a PNG does **not** put pixels in the MCQ. Pattern: + `mcq-with-image`. +7. Wait for the JSON result. On cancel → stop. On freeform → use **`freeform_text`**. Humans use the dialog keyboard (**1–8**, Enter, Esc); do not put hotkey instructions in `question`. Detail: repo `docs/AGENTS.md` (Dialog UX). @@ -34,6 +39,8 @@ instructions in `question`. Detail: repo `docs/AGENTS.md` (Dialog UX). - Markdown A/B/C, numbered lists, or host AskQuestion when this MCP is available - “Send now?” / “Ship it?” with no body/path when the human has not seen the draft +- Asking Alex to judge a still that exists only in chat when the dialog can take + **`image=`** / **`images=`** - Stuffing PATTERN/PROPOSAL/OWNS walls into every MCQ - `check_setup` before routine MCQs (only first enable, dialog failure, or before voice) - Invent a choice after `cancelled: true` diff --git a/src/ask_question_mcp/gtk4_list_ask.py b/src/ask_question_mcp/gtk4_list_ask.py index f8a7fba..0cc68b2 100644 --- a/src/ask_question_mcp/gtk4_list_ask.py +++ b/src/ask_question_mcp/gtk4_list_ask.py @@ -234,7 +234,8 @@ def _main() -> int: gi.require_version("Gtk", "4.0") gi.require_version("Adw", "1") - from gi.repository import Adw, Gdk, GLib, Gtk, Pango + gi.require_version("GdkPixbuf", "2.0") + from gi.repository import Adw, Gdk, GdkPixbuf, GLib, Gtk, Pango question = str(payload.get("question") or "").strip() title = str(payload.get("title") or "Decide") @@ -247,6 +248,11 @@ def _main() -> int: dangerous = bool(payload.get("dangerous")) allow_multiple = bool(payload.get("allow_multiple")) timeout_sec = int(payload.get("timeout_sec") or 0) + image_paths = [ + str(x).strip() + for x in (payload.get("images") or []) + if str(x).strip() + ] speak_pgid_file = payload.get("speak_pgid_file") speak_pgid_file_s = str(speak_pgid_file) if speak_pgid_file else None speak_enabled = bool(payload.get("speak_enabled")) @@ -308,8 +314,12 @@ def _build_and_run(application: Adw.Application) -> None: # Confirm dialog — that left a huge empty band under Cancel/OK. geom_w = min(max(geom_w, 420), 900) geom_h = min(max(geom_h, 360), 560) - if len(question) < 200 and len(ids) <= 6: + if len(question) < 200 and len(ids) <= 6 and not image_paths: geom_h = min(geom_h, 480) + if image_paths: + # Room for ~320px preview + question + options without crushing OK. + geom_w = min(max(geom_w, 640), 1100) + geom_h = min(max(geom_h, 640), 900) win.set_default_size(geom_w, geom_h) win.set_modal(True) @@ -545,6 +555,53 @@ def on_replay(*_args: object) -> None: header.pack_end(header_replay) root.append(header) + def _append_image_previews(parent: Gtk.Box) -> None: + """Scaled PNG/JPEG preview above the question (max height ~320px).""" + if not image_paths: + return + max_h, max_w = 320, 720 + box = Gtk.Box(orientation=Gtk.Orientation.VERTICAL, spacing=8) + box.set_margin_top(8) + box.set_margin_start(16) + box.set_margin_end(16) + box.set_margin_bottom(4) + box.set_hexpand(True) + shown = 0 + for path in image_paths: + try: + pixbuf = GdkPixbuf.Pixbuf.new_from_file_at_scale( + path, max_w, max_h, True + ) + except Exception as exc: # noqa: BLE001 + print( + f"ask-question: skip image {path}: {exc}", + file=sys.stderr, + ) + continue + if pixbuf is None: + continue + try: + texture = Gdk.Texture.new_for_pixbuf(pixbuf) + picture = Gtk.Picture.new_for_paintable(texture) + except Exception: # noqa: BLE001 + picture = Gtk.Picture.new_for_filename(path) + picture.set_can_shrink(True) + try: + picture.set_content_fit(Gtk.ContentFit.CONTAIN) + except AttributeError: + pass + picture.set_halign(Gtk.Align.CENTER) + picture.set_size_request(-1, min(max_h, int(pixbuf.get_height()))) + picture.set_tooltip_text(path) + frame = Gtk.Frame() + frame.set_child(picture) + box.append(frame) + shown += 1 + if shown: + parent.append(box) + + _append_image_previews(root) + # Confirm / question body: title (danger) stays fixed; body scrolls inside # a height cap so tall self-contained referents cannot crush Cancel/OK. body_text = question diff --git a/src/ask_question_mcp/mcq_images.py b/src/ask_question_mcp/mcq_images.py new file mode 100644 index 0000000..12eadb6 --- /dev/null +++ b/src/ask_question_mcp/mcq_images.py @@ -0,0 +1,68 @@ +"""Resolve optional MCQ image paths for dialog previews. + +Agents pass filesystem paths or ``file://`` URIs via ``image`` / ``images``. +Missing or unsupported files are skipped so the MCQ still opens. +""" + +from __future__ import annotations + +from pathlib import Path +from urllib.parse import unquote, urlparse + +# GdkPixbuf / Gtk.Picture common formats on Linux. +_IMAGE_SUFFIXES = frozenset({".png", ".jpg", ".jpeg", ".webp", ".gif", ".bmp"}) +_MAX_IMAGES = 4 + + +def normalize_mcq_images( + image: str | None = None, + images: list[str] | None = None, +) -> list[str]: + """Return existing absolute image paths (order preserved, deduped, capped).""" + raw: list[str] = [] + if image is not None and str(image).strip(): + raw.append(str(image).strip()) + if images: + for item in images: + if item is not None and str(item).strip(): + raw.append(str(item).strip()) + + out: list[str] = [] + seen: set[str] = set() + for item in raw: + path = _resolve_one(item) + if path is None: + continue + key = str(path) + if key in seen: + continue + seen.add(key) + out.append(key) + if len(out) >= _MAX_IMAGES: + break + return out + + +def _resolve_one(item: str) -> Path | None: + s = item.strip() + if not s: + return None + if s.startswith("file:"): + parsed = urlparse(s) + if parsed.scheme != "file": + return None + # Reject remote-ish file://host/… (keep local file:///path). + if parsed.netloc and parsed.netloc.lower() not in {"", "localhost"}: + return None + path = Path(unquote(parsed.path or "")) + else: + path = Path(s).expanduser() + try: + path = path.resolve(strict=False) + except OSError: + return None + if not path.is_file(): + return None + if path.suffix.lower() not in _IMAGE_SUFFIXES: + return None + return path diff --git a/src/ask_question_mcp/server.py b/src/ask_question_mcp/server.py index c9c1eb2..c267bd3 100644 --- a/src/ask_question_mcp/server.py +++ b/src/ask_question_mcp/server.py @@ -62,8 +62,10 @@ def ask_multiple_choice( agent: str | None = None, entry_seed: str | None = None, timeout_sec: int = 300, + image: str | None = None, + images: list[str] | None = None, ) -> str: - """Desktop MCQ for every decision fork — never markdown A/B/C when available. agent=LANE.id; recommended in label + recommended_id; Something else always; dangerous arms OK ~4s. On cancel/errors → check_setup once.""" + """Desktop MCQ for every decision fork — never markdown A/B/C when available. agent=LANE.id; recommended in label + recommended_id; Something else always; optional image/images (local path or file://) for Gtk preview; dangerous arms OK ~4s. On cancel/errors → check_setup once.""" try: result = ask_zenity( question, @@ -78,6 +80,8 @@ def ask_multiple_choice( agent=agent, entry_seed=entry_seed, timeout_sec=timeout_sec, + image=image, + images=images, ) return json.dumps(result, ensure_ascii=False) except AskCancelled as exc: diff --git a/src/ask_question_mcp/zenity_ask.py b/src/ask_question_mcp/zenity_ask.py index af03bac..87a78af 100644 --- a/src/ask_question_mcp/zenity_ask.py +++ b/src/ask_question_mcp/zenity_ask.py @@ -24,6 +24,7 @@ from pathlib import Path from typing import Any +from ask_question_mcp.mcq_images import normalize_mcq_images from ask_question_mcp.voice_acks import ( read_ack_allowed, resolve_agent, @@ -447,6 +448,7 @@ def _ask_list( voice_answer: bool = False, audio_mode: str = "text_only", capability_notes: list[str] | None = None, + images: list[str] | None = None, ) -> tuple[list[str], dict[str, Any], str | None]: """Radiolist/checklist via Gtk (Linux) or tkinter (Windows). @@ -454,6 +456,7 @@ def _ask_list( dialog already confirmed a spoken/typed freeform answer, ``freeform_text`` is set and the entry step is skipped. """ + image_paths = [str(p) for p in (images or []) if str(p).strip()] if _is_windows(): if not _WIN_LIST_ASK.is_file(): raise RuntimeError(f"missing Windows list dialog: {_WIN_LIST_ASK}") @@ -475,6 +478,8 @@ def _ask_list( "voice_answer": False, "audio_mode": audio_mode or "text_only", "capability_notes": list(capability_notes or []), + # Preview is Linux Gtk-only for now; paths ignored on Windows. + "images": image_paths, } try: proc = subprocess.run( @@ -547,6 +552,7 @@ def _ask_list( "voice_answer": listen_on, "audio_mode": audio_mode, "capability_notes": list(capability_notes or []), + "images": image_paths, } env = {**os.environ, "DISPLAY": display} try: @@ -617,6 +623,8 @@ def ask_zenity( agent: str | None = None, timeout_sec: int = 300, entry_seed: str | None = None, + image: str | None = None, + images: list[str] | None = None, ) -> dict[str, Any]: """Block until the user picks. Mark recommended only in option labels. @@ -631,6 +639,8 @@ def ask_zenity( ``ASK_QUESTION_SPEAK=0`` to mute. ``agent`` (or LANE.id / ``ASK_QUESTION_AGENT``) is prefixed in the window title so multi-agent sessions stay distinguishable. + ``image`` / ``images``: optional local PNG/JPEG (etc.) path or ``file://`` + URI for a Gtk preview above the question (Linux; skipped if missing). ``allow_other`` is accepted for API compatibility but ignored — Something else is always appended when missing (unless already present / no room). @@ -661,6 +671,7 @@ def ask_zenity( do_listen = caps.listen_active who = resolve_agent(agent) + image_paths = normalize_mcq_images(image=image, images=images) ids: list[str] = [] labels: dict[str, str] = {} @@ -803,6 +814,7 @@ def _release_session_duck() -> None: voice_answer=do_listen, audio_mode=caps.audio_mode, capability_notes=caps.notes, + images=image_paths, ) except AskCancelled: # Cancel / timeout / close — cut question audio immediately.