From ca0d028a8702f372b0e17306c11400ad1215d317 Mon Sep 17 00:00:00 2001 From: Felix Martin Date: Wed, 10 Jun 2026 19:56:26 -0400 Subject: [PATCH 1/9] docs: design spec for P2 fixes Co-Authored-By: Claude Fable 5 --- .../specs/2026-06-10-p2-fixes-design.md | 259 ++++++++++++++++++ 1 file changed, 259 insertions(+) create mode 100644 docs/superpowers/specs/2026-06-10-p2-fixes-design.md diff --git a/docs/superpowers/specs/2026-06-10-p2-fixes-design.md b/docs/superpowers/specs/2026-06-10-p2-fixes-design.md new file mode 100644 index 0000000..c43194c --- /dev/null +++ b/docs/superpowers/specs/2026-06-10-p2-fixes-design.md @@ -0,0 +1,259 @@ +# P2 Fixes Design + +**Date:** 2026-06-10 + +**Goal:** Resolve the three P2 findings in `TODO.md` — the Codex session-id +capture race, the absence of real-tmux smoke tests, and unvalidated project +paths — without touching unrelated behavior. + +**Scope:** These three findings only. No other P2 hardening, no refactors beyond +what each fix strictly requires. + +--- + +## Finding 1 — Codex session-id capture race + +### Problem + +`SessionService` captures a Codex rollout id after spawning by scanning +`~/.codex/sessions/rollout-*.jsonl` for the earliest rollout whose `cwd` matches +the project path and whose `started_at >= since` (the spawn timestamp). If a +second Codex session starts in the same project during the retry window +(`CAPTURE_MAX_ATTEMPTS=6` × `CAPTURE_RETRY_INTERVAL=0.5s` ≈ 3s), two rollouts +match and the code blindly picks the earliest — which may be the wrong +conversation. A later `codex resume ` then restores the wrong session. + +### Approach: serialize hqt starts + refuse ambiguous matches + +Two complementary changes. The lock removes the race between hqt-launched +sessions; the ambiguity guard makes a wrong capture impossible even when the +lock cannot help (a Codex started manually by the user in the same directory). + +**1. Serialize the spawn→capture critical section.** + +Add an `asyncio.Lock` to `SessionService`: + +```python +def __init__(self, factory, tmux, harnesses): + self.factory = factory + self.tmux = tmux + self.harnesses = harnesses + # Serializes the spawn->capture window for harnesses that capture a + # session id (codex), so two concurrent hqt starts in the same project + # never overlap and confuse capture_session_id. + self._capture_lock = asyncio.Lock() +``` + +Hold the lock around the `since = time.time()` → `tmux.spawn`/`respawn` → +`_capture_session_id_with_retry` region, but **only when** +`configurator.captures_session_id` is true (non-capturing harnesses need no +serialization). This applies in two places: + +- `create_session` — the initial spawn + capture. +- `_respawn_with_fallback` rung 2 — the fresh-spawn + capture. + +Holding the lock across the retry loop means it can be held for up to ~3s, so +rapid session creation against capturing harnesses queues. Session creation is +user-initiated and infrequent, so this is acceptable. + +**2. Refuse ambiguous captures in `CodexConfigurator.capture_session_id`.** + +Today the method sorts candidates and returns the earliest. Change it so that: + +- **0 candidates** → return `None` (unchanged; retry loop tries again). +- **exactly 1 candidate** → return its id (the lock-protected normal case). +- **more than 1 candidate** → ambiguous; log a warning and return `None` + rather than guess. + +```python +candidates.sort(key=lambda t: t[0]) +if len(candidates) > 1: + log.warning( + "capture_session_id: %d rollouts match cwd=%s since=%s; " + "refusing to guess", + len(candidates), project_path, since, + ) + return None +return candidates[0][1] if candidates else None +``` + +`codex.py` gains a module-level `log = logging.getLogger(__name__)`. + +This guard also hardens the poller path +(`SessionService._maybe_capture_missing_session_id`), which calls +`capture_session_id` without the lock: an ambiguous match there now keeps the +placeholder id instead of risking a wrong one. + +### Outcome + +The worst case becomes "keep the placeholder id" (a missing-but-safe capture), +never "store the wrong id." The placeholder path already exists and is logged. + +### Tests (`tests/test_harnesses.py`, `tests/test_sessions.py`) + +- `capture_session_id` with two matching rollouts in the same cwd/time window + returns `None` and logs a warning (use `tmp_path` as a fake `~/.codex`, + monkeypatching `Path.home`). +- `capture_session_id` with exactly one matching rollout returns its id + (regression guard for the normal case). +- `SessionService` exposes `_capture_lock` as an `asyncio.Lock`; a unit test + asserts `create_session` acquires it for a capturing harness. (Drive via the + existing fake/mocked tmux + a stub configurator with + `captures_session_id=True`; assert the lock is held during the spawn call, + e.g. by checking `service._capture_lock.locked()` from inside the stub's + `capture_session_id`.) + +--- + +## Finding 2 — Real-tmux smoke tests + +### Problem + +tmux behavior is exercised almost entirely through mocked `_exec`. Theming, new +windows, respawn, and labels are never validated against a real tmux binary, so +a wrong flag or format string passes the suite. + +### Approach: isolated real-tmux server via `TMUX_TMPDIR`, auto-skip + +Add `tests/test_tmux_smoke.py`. No source change to `runner.py` is required: +tmux places its server socket in `$TMUX_TMPDIR`, so pointing that at a fresh +temp dir gives a throwaway server fully isolated from the user's real sessions. + +**Gating.** Module-level skip when tmux is unavailable: + +```python +import shutil +import pytest + +pytestmark = [ + pytest.mark.tmux, + pytest.mark.skipif(shutil.which("tmux") is None, reason="tmux not installed"), +] +``` + +**Isolation fixture.** + +```python +@pytest.fixture +def tmux_runner(tmp_path, monkeypatch): + monkeypatch.setenv("TMUX_TMPDIR", str(tmp_path)) + session = "hqt-test" + subprocess.run( + ["tmux", "new-session", "-d", "-s", session, "-x", "200", "-y", "50"], + check=True, + env={**os.environ, "TMUX_TMPDIR": str(tmp_path)}, + ) + runner = TmuxRunner(tmux_path="tmux", session_name=session) + try: + yield runner + finally: + subprocess.run( + ["tmux", "kill-server"], + env={**os.environ, "TMUX_TMPDIR": str(tmp_path)}, + check=False, + ) +``` + +`TmuxRunner._exec` uses `asyncio.create_subprocess_exec` without an explicit +`env`, so it inherits the monkeypatched `TMUX_TMPDIR` and targets the isolated +server automatically. + +**Coverage (one test each, all `async`/`pytest.mark.asyncio`):** + +- **new window:** `await runner.new_window("hqt-1", str(tmp_path), "sh")` returns + a pane/window id; the window appears in `await runner.list_windows()`. +- **label round-trip:** `await runner.set_window_label("hqt-1", "•hqt-1")`, then + read the `@hqt_label` user option back via a raw + `tmux show-options -w -t hqt-test:=hqt-1` (or `runner` accessor if present) + and assert it equals what was set. +- **respawn after death:** create a window running a command that exits, confirm + the pane is dead (`await runner.is_pane_dead(...)` is true), then + `await runner.respawn_pane(...)` with a long-lived command and assert the pane + is alive again via `await runner.verify_window_alive(...)`. (Test the runner + directly; `TmuxManager.respawn_verified` is out of scope here.) +- **theme:** `await runner.apply_theme()`, then assert a representative session + option (e.g. `status-justify left` or `status` on) is set via raw + `tmux show-options -t hqt-test`. + +**Marker registration (`pyproject.toml`).** Register the `tmux` marker so pytest +emits no unknown-marker warning: + +```toml +[tool.pytest.ini_options] +markers = [ + "tmux: real-tmux smoke tests; require a tmux binary and run on an isolated TMUX_TMPDIR server", +] +``` + +(If a `[tool.pytest.ini_options]` block does not yet exist, add it; otherwise +extend it.) + +### Outcome + +Real-tmux tests run by default wherever tmux exists (developer machines, CI with +tmux installed) and skip cleanly elsewhere. They never touch the user's live +tmux server because every invocation is scoped to the temp `TMUX_TMPDIR`. + +--- + +## Finding 3 — Project path validation + +### Problem + +`ProjectService.create`/`update` store whatever path string they are given. A +nonexistent or non-directory path is accepted and only fails later when a +session spawned there dies, surfacing as a confusing dead session rather than an +early, clear rejection. + +### Approach: validate and normalize at create/update + +Add a private helper to `ProjectService`: + +```python +from pathlib import Path + +def _normalize_path(self, raw: str) -> str: + """Expand ~, require an existing directory, return the resolved absolute path. + + Raises ServiceError if the path does not exist or is not a directory, so the + TUI surfaces it as a notification instead of letting the bad path become a + dead session later. + """ + path = Path(raw).expanduser() + if not path.is_dir(): + raise ServiceError(f"Path does not exist or is not a directory: {raw}") + return str(path.resolve()) +``` + +`create` and `update` call `path = self._normalize_path(path)` before +constructing/assigning the `Project`, so the stored path is always an absolute, +existing directory. The existing duplicate-path `IntegrityError` handling is +unchanged and runs after normalization (so duplicate detection compares resolved +paths consistently). + +**No TUI change.** `action_add_project` (`app.py:201-205`) and +`action_edit_project` (`app.py:223-227`) already catch `ServiceError` and call +`self.notify(str(err), severity="error")`, so a rejected path shows as an error +notification with no further wiring. + +### Tests (`tests/test_services.py`) + +- `create` with a nonexistent path raises `ServiceError`. +- `create` with a path that is a file (not a directory) raises `ServiceError`. +- `create` with a valid directory stores the resolved absolute path + (assert `project.path == str(tmp_path.resolve())`). +- `create` with a `~`-prefixed path expands it (monkeypatch `Path.home` or + `HOME` to `tmp_path` and assert expansion). +- `update` with a nonexistent path raises `ServiceError` and leaves the row + unchanged (re-read after `db.expire_all()`). + +--- + +## Out of scope + +- Any change to `src/hqt/tmux/runner.py` (it carries unrelated uncommitted WIP; + the chosen designs deliberately avoid touching it). +- Broader Codex correctness work (e.g. matching on an injected marker, or + isolating `CODEX_HOME` — rejected because a per-session `CODEX_HOME` would also + isolate Codex's `config.toml`/auth and break the harness). +- Any new P3-level hardening or refactors. From ad7f9f87aaaf1e8fc28a3cd175e5229b8432793a Mon Sep 17 00:00:00 2001 From: Felix Martin Date: Wed, 10 Jun 2026 20:01:06 -0400 Subject: [PATCH 2/9] docs: implementation plan for P2 fixes Co-Authored-By: Claude Fable 5 --- docs/superpowers/plans/2026-06-10-p2-fixes.md | 822 ++++++++++++++++++ 1 file changed, 822 insertions(+) create mode 100644 docs/superpowers/plans/2026-06-10-p2-fixes.md diff --git a/docs/superpowers/plans/2026-06-10-p2-fixes.md b/docs/superpowers/plans/2026-06-10-p2-fixes.md new file mode 100644 index 0000000..0a6f49d --- /dev/null +++ b/docs/superpowers/plans/2026-06-10-p2-fixes.md @@ -0,0 +1,822 @@ +# P2 Fixes Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Resolve the three P2 findings in `TODO.md` — validate project paths at create/update, harden Codex session-id capture against the multi-session race, and add isolated real-tmux smoke tests. + +**Architecture:** Three independent fixes. (1) `ProjectService` gains a `_normalize_path` helper that rejects non-directories. (2) `CodexConfigurator.capture_session_id` refuses to guess when more than one rollout matches, and `SessionService` serializes the spawn→capture window with an `asyncio.Lock` for capturing harnesses. (3) A new `tests/test_tmux_smoke.py` drives the real `TmuxRunner` against a throwaway tmux server isolated via `TMUX_TMPDIR`, auto-skipping when tmux is absent. + +**Tech Stack:** Python 3.12, SQLAlchemy 2.0, Textual, pytest + pytest-asyncio, `uv`, tmux. + +**Design spec:** `docs/superpowers/specs/2026-06-10-p2-fixes-design.md` + +**Conventions for every task:** +- Run quality gates after code/test changes: `uv run ruff format src tests && uv run ruff check src tests && uv run ty check`. +- Run tests with `uv run pytest`. +- Commit messages end with the trailer: + ``` + Co-Authored-By: Claude Fable 5 + ``` +- **NEVER `git add -A` or `git add .`** — `src/hqt/tmux/runner.py` carries unrelated uncommitted WIP that must stay out of every commit. Always `git add` the exact files listed in the task. No task in this plan modifies `runner.py`. +- Work on `main`, committing directly to it. + +--- + +## File Structure + +| File | Responsibility | Tasks | +|------|----------------|-------| +| `src/hqt/projects/service.py` | Project CRUD; add path normalization/validation | 1 | +| `tests/test_services.py` | ProjectService tests; add path-validation cases | 1 | +| `src/hqt/harnesses/configurators/codex.py` | Codex capture; refuse ambiguous matches | 2 | +| `tests/test_harnesses.py` | Codex capture tests; replace earliest-wins cases with ambiguity case | 2 | +| `src/hqt/sessions/service.py` | Session lifecycle; serialize spawn→capture with a lock | 3 | +| `tests/test_sessions.py` | SessionService tests; assert lock behavior | 3 | +| `tests/test_tmux_smoke.py` (new) | Real-tmux smoke tests on an isolated server | 4 | +| `pyproject.toml` | Register the `tmux` pytest marker | 4 | +| `TODO.md` | Remove the three resolved P2 lines | 5 | + +--- + +## Task 1: Project path validation + +**Files:** +- Modify: `src/hqt/projects/service.py` +- Test: `tests/test_services.py` + +- [ ] **Step 1: Write the failing tests** + +Add these tests to the `TestProjectService` class in `tests/test_services.py`. They need a real directory, so they use pytest's `tmp_path` fixture. + +```python + def test_create_rejects_nonexistent_path(self, factory, tmp_path): + svc = ProjectService(factory) + missing = tmp_path / "does-not-exist" + with pytest.raises(ServiceError, match="does not exist or is not a directory"): + svc.create("p", str(missing)) + + def test_create_rejects_file_path(self, factory, tmp_path): + svc = ProjectService(factory) + a_file = tmp_path / "afile" + a_file.write_text("x") + with pytest.raises(ServiceError, match="does not exist or is not a directory"): + svc.create("p", str(a_file)) + + def test_create_stores_resolved_absolute_path(self, factory, tmp_path): + svc = ProjectService(factory) + p = svc.create("p", str(tmp_path)) + assert p.path == str(tmp_path.resolve()) + + def test_create_expands_user_home(self, factory, tmp_path, monkeypatch): + # Point ~ at tmp_path; "~" must expand to the existing tmp_path dir. + monkeypatch.setenv("HOME", str(tmp_path)) + svc = ProjectService(factory) + p = svc.create("p", "~") + assert p.path == str(tmp_path.resolve()) + + def test_update_rejects_nonexistent_path_and_leaves_row(self, factory, tmp_path): + svc = ProjectService(factory) + p = svc.create("p", str(tmp_path)) + missing = tmp_path / "nope" + with pytest.raises(ServiceError, match="does not exist or is not a directory"): + svc.update(p.id, "p", str(missing)) + assert svc.get(p.id).path == str(tmp_path.resolve()) +``` + +Several existing tests pass made-up paths like `/tmp/test`, `/a`, `/old/path` that are not guaranteed to exist. Update those literals to real directories so they keep passing. Replace the existing `TestProjectService` methods `test_create_and_get`, `test_list_excludes_archived`, `test_update_name_and_path`, `test_update_duplicate_path_raises`, and `test_create_duplicate_path_raises_service_error` to seed paths from `tmp_path`: + +```python + def test_create_and_get(self, factory, tmp_path): + svc = ProjectService(factory) + p = svc.create("test", str(tmp_path)) + assert p.id is not None + assert svc.get(p.id).name == "test" + + def test_list_excludes_archived(self, factory, tmp_path): + a = tmp_path / "a" + a.mkdir() + b = tmp_path / "b" + b.mkdir() + svc = ProjectService(factory) + svc.create("a", str(a)) + p2 = svc.create("b", str(b)) + svc.archive(p2.id) + assert len(svc.list_all()) == 1 + assert len(svc.list_all(include_archived=True)) == 2 + + def test_update_name_and_path(self, factory, tmp_path): + old = tmp_path / "old" + old.mkdir() + new = tmp_path / "new" + new.mkdir() + svc = ProjectService(factory) + p = svc.create("old", str(old)) + updated = svc.update(p.id, "new", str(new)) + assert updated.name == "new" + assert updated.path == str(new.resolve()) + assert svc.get(p.id).path == str(new.resolve()) + + def test_update_duplicate_path_raises(self, factory, tmp_path): + a = tmp_path / "a" + a.mkdir() + b = tmp_path / "b" + b.mkdir() + svc = ProjectService(factory) + svc.create("a", str(a)) + p2 = svc.create("b", str(b)) + with pytest.raises(ServiceError, match="already uses path"): + svc.update(p2.id, "b", str(a)) + # DB session must remain usable after rollback, values unchanged + assert svc.get(p2.id).path == str(b.resolve()) + assert svc.get(p2.id).name == "b" + + def test_create_duplicate_path_raises_service_error(self, factory, tmp_path): + dup = tmp_path / "dup" + dup.mkdir() + svc = ProjectService(factory) + svc.create("a", str(dup)) + with pytest.raises(ServiceError, match="already uses path"): + svc.create("b", str(dup)) +``` + +Note: `TestMcpService.test_bind_unbind` creates a project at `/proj`; change that literal to `str(tmp_path)` and add `tmp_path` to its signature: + +```python + def test_bind_unbind(self, factory, tmp_path): + psvc = ProjectService(factory) + msvc = McpService(factory) + project = psvc.create("proj", str(tmp_path)) + server = msvc.create("s1", "stdio", command="cmd") + msvc.bind_to_project(project.id, server.id) + assert len(msvc.get_project_mcps(project.id)) == 1 + msvc.unbind_from_project(project.id, server.id) + assert len(msvc.get_project_mcps(project.id)) == 0 +``` + +- [ ] **Step 2: Run the new tests to verify they fail** + +Run: `uv run pytest tests/test_services.py -k "rejects or resolved or expands_user" -v` +Expected: FAIL — `ServiceError` not raised / path not resolved (validation not implemented yet). + +- [ ] **Step 3: Implement `_normalize_path` and call it from `create`/`update`** + +In `src/hqt/projects/service.py`, add the `Path` import and the helper, and call it at the top of `create` and `update`. Full file: + +```python +from pathlib import Path + +from sqlalchemy.exc import IntegrityError +from sqlalchemy.orm import sessionmaker + +from hqt.db.models import Project +from hqt.errors import ServiceError + + +class ProjectService: + def __init__(self, factory: sessionmaker): + # factory must be created with expire_on_commit=False so rows returned + # from these methods stay readable after their session closes. + self.factory = factory + + def _normalize_path(self, raw: str) -> str: + """Expand ~, require an existing directory, return the resolved absolute path. + + Raises ServiceError if the path does not exist or is not a directory, so + the TUI surfaces it as a notification instead of letting the bad path + become a dead session later. + """ + path = Path(raw).expanduser() + if not path.is_dir(): + raise ServiceError(f"Path does not exist or is not a directory: {raw}") + return str(path.resolve()) + + def create(self, name: str, path: str) -> Project: + path = self._normalize_path(path) + with self.factory() as db: + project = Project(name=name, path=path) + db.add(project) + try: + db.commit() + except IntegrityError as err: + db.rollback() + raise ServiceError(f"Another project already uses path {path}") from err + return project + + def list_all(self, include_archived: bool = False) -> list[Project]: + with self.factory() as db: + q = db.query(Project) + if not include_archived: + q = q.filter_by(archived=False) + return list(q.all()) + + def get(self, project_id: int) -> Project | None: + with self.factory() as db: + return db.get(Project, project_id) + + def update(self, project_id: int, name: str, path: str) -> Project: + path = self._normalize_path(path) + with self.factory() as db: + project = db.get(Project, project_id) + if project is None: + raise ServiceError(f"Project {project_id} not found") + project.name = name + project.path = path + try: + db.commit() + except IntegrityError as err: + db.rollback() + raise ServiceError(f"Another project already uses path {path}") from err + return project + + def archive(self, project_id: int) -> None: + with self.factory() as db: + project = db.get(Project, project_id) + if project: + project.archived = True + db.commit() +``` + +Note ordering: `_normalize_path` runs **before** opening the DB session, so a bad path is rejected without touching the DB. `update` validates the path before checking the project exists — that's fine; both raise `ServiceError`. + +- [ ] **Step 4: Run the full service test file** + +Run: `uv run pytest tests/test_services.py -v` +Expected: PASS (new validation tests + all updated existing tests). + +- [ ] **Step 5: Run quality gates** + +Run: `uv run ruff format src tests && uv run ruff check src tests && uv run ty check` +Expected: all green. + +- [ ] **Step 6: Commit** + +```bash +git add src/hqt/projects/service.py tests/test_services.py +git commit -m "feat: validate project paths at create/update + +Reject non-existent or non-directory paths with ServiceError (already +surfaced as a TUI notification), and store the resolved absolute path. +Closes the P2 finding where bad paths became dead sessions later. + +Co-Authored-By: Claude Fable 5 " +``` + +--- + +## Task 2: Codex capture refuses ambiguous matches + +**Files:** +- Modify: `src/hqt/harnesses/configurators/codex.py` +- Test: `tests/test_harnesses.py` + +**Context:** `capture_session_id` scans `~/.codex/sessions/rollout-*.jsonl` for rollouts whose `cwd` matches the project and whose start time is `>= since`. Today it sorts the matches and returns the earliest. When two Codex sessions ran in the same directory in the capture window, that guess can be the wrong conversation. The fix: if more than one rollout matches, refuse to guess (return `None` + warn). The single-match normal case is unchanged. + +- [ ] **Step 1: Replace the earliest-wins tests with an ambiguity test** + +In `tests/test_harnesses.py`: + +1. **Delete** `test_codex_capture_first_post_since_cwd_match_wins` (it asserts the now-removed earliest-wins behavior with two matching rollouts). +2. **Delete** `test_codex_capture_prefers_started_nearest_since_over_newest_file` (also asserts earliest-wins among two matching rollouts). +3. **Add** this test (place it where the deleted ones were): + +```python +def test_codex_capture_ambiguous_returns_none(tmp_path): + """Two rollouts match cwd + since window -> refuse to guess, return None.""" + sessions_dir = tmp_path / ".codex" / "sessions" / "dir" + sessions_dir.mkdir(parents=True) + + rollout_a = sessions_dir / "rollout-a.jsonl" + rollout_b = sessions_dir / "rollout-b.jsonl" + _write_rollout(rollout_a, "id-a", "/projects/foo") + _write_rollout(rollout_b, "id-b", "/projects/foo") + + now = time.time() + since = now - 120 + os.utime(rollout_a, (now - 30, now - 30)) + os.utime(rollout_b, (now - 10, now - 10)) + + c = CodexConfigurator() + with patch("hqt.harnesses.configurators.codex.Path.home", return_value=tmp_path): + result = c.capture_session_id(Path("/projects/foo"), since) + assert result is None +``` + +The single-match regression cases (`test_codex_capture_session_id`, +`test_codex_capture_accepts_file_after_since`, +`test_codex_capture_skips_non_matching_cwd_picks_older_matching` — which has only +one cwd-matching rollout) stay as-is and must keep passing. + +- [ ] **Step 2: Run the capture tests to verify the new one fails** + +Run: `uv run pytest tests/test_harnesses.py -k "capture" -v` +Expected: `test_codex_capture_ambiguous_returns_none` FAILS (current code returns `"id-a"`, not `None`). The deleted tests are gone. + +- [ ] **Step 3: Implement the ambiguity guard** + +In `src/hqt/harnesses/configurators/codex.py`, add logging and change `capture_session_id`. Replace the imports block at the top: + +```python +import json +import logging +from datetime import datetime +from pathlib import Path + +from hqt.harnesses.base import HarnessConfigurator, SpawnConfig + +log = logging.getLogger(__name__) +``` + +Replace the tail of `capture_session_id` (the `candidates.sort(...)` / return lines) with the ambiguity guard. The full method: + +```python + def capture_session_id(self, project_path: Path, since: float) -> str | None: + sessions_dir = Path.home() / ".codex" / "sessions" + if not sessions_dir.exists(): + return None + candidates: list[tuple[float, str]] = [] + for rollout in sessions_dir.rglob("rollout-*.jsonl"): + try: + mtime = rollout.stat().st_mtime + with rollout.open() as f: + meta = json.loads(f.readline()) + payload = meta["payload"] + if meta.get("type") != "session_meta" or str( + Path(payload["cwd"]) + ) != str(project_path): + continue + started_at = self._meta_timestamp(payload) or mtime + if started_at >= since: + candidates.append((started_at, payload["id"])) + except (json.JSONDecodeError, KeyError, OSError, TypeError): + continue + if len(candidates) > 1: + # Ambiguous: more than one Codex rollout matches this cwd + time + # window (e.g. a second Codex started in the same project). Guessing + # risks storing the wrong conversation id, so keep the placeholder. + log.warning( + "capture_session_id: %d rollouts match cwd=%s since=%s; refusing to guess", + len(candidates), + project_path, + since, + ) + return None + return candidates[0][1] if candidates else None +``` + +(The `candidates.sort(...)` line is removed — with at most one returned candidate there is nothing to sort.) + +- [ ] **Step 4: Run the capture tests** + +Run: `uv run pytest tests/test_harnesses.py -k "capture" -v` +Expected: PASS, including `test_codex_capture_ambiguous_returns_none`. + +- [ ] **Step 5: Run quality gates** + +Run: `uv run ruff format src tests && uv run ruff check src tests && uv run ty check` +Expected: all green. + +- [ ] **Step 6: Commit** + +```bash +git add src/hqt/harnesses/configurators/codex.py tests/test_harnesses.py +git commit -m "fix: refuse ambiguous Codex session-id capture + +When more than one rollout matches the project cwd within the capture +window, return None and warn instead of guessing the earliest. The worst +case is now a kept placeholder id, never a wrong one. Part of the P2 +capture-race fix. + +Co-Authored-By: Claude Fable 5 " +``` + +--- + +## Task 3: Serialize the spawn→capture window in SessionService + +**Files:** +- Modify: `src/hqt/sessions/service.py` +- Test: `tests/test_sessions.py` + +**Context:** The Task 2 guard makes wrong captures impossible, but two hqt-launched Codex sessions starting in the same project would now each see an ambiguous window and capture nothing. An `asyncio.Lock` held across the spawn→capture region (only for harnesses that `captures_session_id`) keeps hqt-launched starts from overlapping, so each capture sees exactly its own rollout. + +- [ ] **Step 1: Write the failing tests** + +Add to `tests/test_sessions.py`. The first asserts the lock is held while a capturing harness spawns+captures; the second asserts it is NOT held for a non-capturing harness (so we only pay the serialization cost when needed). + +```python +@pytest.mark.asyncio +async def test_create_session_holds_capture_lock_for_capturing_harness( + factory, db, tmux +): + observed = {} + + def _capture(project_path, since): + # capture runs inside the spawn->capture critical section + observed["locked_during_capture"] = service._capture_lock.locked() + return "real-codex-id" + + h = MagicMock() + h.captures_session_id = True + h.generate_session_id.return_value = "placeholder" + h.build_spawn_config.return_value = MagicMock( + command=["codex"], env={}, cwd=Path("/tmp/myproj") + ) + h.capture_session_id.side_effect = _capture + service = SessionService(factory=factory, tmux=tmux, harnesses={"claude-code": h}) + + result = await service.create_session(project_id=1, harness_name="claude-code") + + assert observed["locked_during_capture"] is True + assert result.session.harness_session_id == "real-codex-id" + # Lock is released after create_session returns. + assert service._capture_lock.locked() is False + + +@pytest.mark.asyncio +async def test_create_session_no_lock_for_non_capturing_harness(service, db, tmux): + observed = {} + + async def _spawn(req): + observed["locked_during_spawn"] = service._capture_lock.locked() + return SpawnResult(ok=True, window_id="@1", error="") + + tmux.spawn.side_effect = _spawn + + await service.create_session(project_id=1, harness_name="claude-code") + + # The `service` fixture's harness has captures_session_id = False, so the + # spawn must NOT be serialized under the capture lock. + assert observed["locked_during_spawn"] is False +``` + +- [ ] **Step 2: Run the new tests to verify they fail** + +Run: `uv run pytest tests/test_sessions.py -k "capture_lock or no_lock" -v` +Expected: FAIL with `AttributeError: 'SessionService' object has no attribute '_capture_lock'`. + +- [ ] **Step 3: Add the lock and the conditional guard helper** + +In `src/hqt/sessions/service.py`: + +Add `import contextlib` near the top imports (alongside `import asyncio`): + +```python +import asyncio +import contextlib +import logging +import time +``` + +Add the lock in `__init__` and a helper context manager. Update `__init__`: + +```python + def __init__( + self, + factory: sessionmaker, + tmux: TmuxManager, + harnesses: Mapping[str, HarnessConfigurator], + ): + self.factory = factory + self.tmux = tmux + self.harnesses = harnesses + # Serializes the spawn->capture window for harnesses that capture a + # session id (codex), so two concurrent hqt starts in the same project + # never overlap and confuse capture_session_id. + self._capture_lock = asyncio.Lock() + + @contextlib.asynccontextmanager + async def _maybe_capture_lock(self, needed: bool): + """Hold the capture lock only when the harness captures a session id.""" + if needed: + async with self._capture_lock: + yield + else: + yield +``` + +- [ ] **Step 4: Wrap the spawn→capture region in `create_session`** + +In `create_session`, wrap from `since = time.time()` through the capture block in the guard. The relevant region becomes (note the added `async with` and the indentation of the lines it now encloses): + +```python + spawn_cfg = configurator.build_spawn_config( + project_path, harness_session_id, model + ) + log.info( + "Spawning window %s: cmd=%s cwd=%s", + sess.tmux_session_name, + spawn_cfg.command, + spawn_cfg.cwd, + ) + async with self._maybe_capture_lock(configurator.captures_session_id): + since = time.time() + spawn_result = await self.tmux.spawn( + SpawnRequest( + window_name=sess.tmux_session_name, + command=spawn_cfg.command, + cwd=str(spawn_cfg.cwd), + env=spawn_cfg.env, + ) + ) + if not spawn_result.ok: + log.error( + "Failed to spawn window %s: %s", + sess.tmux_session_name, + spawn_result.error or "(no output captured)", + ) + # Only attempt capture when the spawn actually succeeded; a failed + # spawn could inadvertently capture an unrelated session running in + # the same cwd. + if spawn_result.ok and configurator.captures_session_id: + captured_id = await self._capture_session_id_with_retry( + configurator, project_path, since, sess.tmux_session_name + ) + if captured_id: + sess.harness_session_id = captured_id + db.commit() +``` + +`spawn_result` is referenced after the `async with` block (in the `return CreateSessionResult(...)` that follows) — it remains in scope because `async with` does not introduce a new scope. Leave the rest of the method (the `log.info` and `return CreateSessionResult(...)`) unchanged. + +- [ ] **Step 5: Wrap the rung-2 fresh-spawn region in `_respawn_with_fallback`** + +In `_respawn_with_fallback`, wrap the rung-2 region (from `since = time.time()` through the capture block) in the same guard. The rung-2 portion becomes: + +```python + # Rung 2: fresh spawn (codex ignores the old session id; start fresh) + configurator = self._configurator(sess.harness.name) + project_path = self._project_path(db, sess.project_id) + harness_session_id = ( + sess.harness_session_id or configurator.generate_session_id(sess.id) + ) + spawn_cfg = configurator.build_spawn_config( + project_path, harness_session_id, sess.model + ) + async with self._maybe_capture_lock(configurator.captures_session_id): + since = time.time() + result = await self.tmux.respawn_verified( + window_name, spawn_cfg.command, str(spawn_cfg.cwd), env=spawn_cfg.env + ) + if not result.ok: + log.error( + "Fallback spawn also failed for %s: %s", + window_name, + result.error or "(no output)", + ) + return False + + # Rung-2 succeeded — capture the new session id if the harness supports + # it so that future resumes target this fresh conversation. + if configurator.captures_session_id: + new_id = await self._capture_session_id_with_retry( + configurator, project_path, since, window_name + ) + if new_id: + sess.harness_session_id = new_id + db.commit() + else: + log.warning( + "capture_session_id exhausted after rung-2 for %s; keeping old id %s", + window_name, + sess.harness_session_id, + ) + return True +``` + +The `return False` inside the `async with` correctly releases the lock on the way out. Leave rung 1 (the resume attempt above this region) unchanged — it does not spawn a new conversation, so it needs no lock. + +- [ ] **Step 6: Run the session tests** + +Run: `uv run pytest tests/test_sessions.py -v` +Expected: PASS (new lock tests + all existing session tests). + +- [ ] **Step 7: Run quality gates** + +Run: `uv run ruff format src tests && uv run ruff check src tests && uv run ty check` +Expected: all green. + +- [ ] **Step 8: Commit** + +```bash +git add src/hqt/sessions/service.py tests/test_sessions.py +git commit -m "fix: serialize Codex spawn->capture with a lock + +Hold an asyncio.Lock across the spawn->capture window for harnesses that +capture a session id, so two concurrent hqt starts in the same project +never produce overlapping capture windows. Completes the P2 capture-race +fix alongside the ambiguity guard. + +Co-Authored-By: Claude Fable 5 " +``` + +--- + +## Task 4: Real-tmux smoke tests + +**Files:** +- Create: `tests/test_tmux_smoke.py` +- Modify: `pyproject.toml` + +**Context:** tmux behavior is otherwise tested with mocked `_exec`. These tests drive the real `TmuxRunner` against a throwaway tmux server isolated via `TMUX_TMPDIR`, so they never touch the user's live sessions, and skip when tmux is absent. + +- [ ] **Step 1: Register the `tmux` pytest marker** + +In `pyproject.toml`, add a `[tool.pytest.ini_options]` block (none exists yet — add it after the `[dependency-groups]` block, before `[tool.hqt.quality]`): + +```toml +[tool.pytest.ini_options] +markers = [ + "tmux: real-tmux smoke tests; require a tmux binary and run on an isolated TMUX_TMPDIR server", +] +``` + +Note: the existing async tests already run with explicit `@pytest.mark.asyncio` decorators under pytest-asyncio's default strict mode, so do **not** add an `asyncio_mode` setting — only the `markers` entry is needed here. + +- [ ] **Step 2: Write the smoke tests** + +Create `tests/test_tmux_smoke.py`: + +```python +"""Real-tmux smoke tests. + +These drive the actual TmuxRunner against a throwaway tmux server whose socket +lives in a temp TMUX_TMPDIR, so they never touch the user's live tmux sessions. +They skip when no tmux binary is available. +""" + +import asyncio +import os +import shutil +import subprocess + +import pytest + +from hqt.tmux.runner import TmuxRunner + +pytestmark = [ + pytest.mark.tmux, + pytest.mark.skipif(shutil.which("tmux") is None, reason="tmux not installed"), +] + +SESSION = "hqt-test" + + +def _tmux(env, *args): + """Run a raw tmux command against the isolated server, return stdout.""" + return subprocess.run( + ["tmux", *args], + env=env, + capture_output=True, + text=True, + check=False, + ) + + +@pytest.fixture +def tmux_env(tmp_path, monkeypatch): + # Point tmux's server socket at the temp dir: a fully isolated server. + monkeypatch.setenv("TMUX_TMPDIR", str(tmp_path)) + env = {**os.environ, "TMUX_TMPDIR": str(tmp_path)} + _tmux(env, "new-session", "-d", "-s", SESSION, "-x", "200", "-y", "50") + try: + yield env + finally: + _tmux(env, "kill-server") + + +@pytest.fixture +def runner(tmux_env): + return TmuxRunner(tmux_path="tmux", session_name=SESSION) + + +async def _wait_pane_dead(runner, window, timeout=2.0): + deadline = asyncio.get_running_loop().time() + timeout + while asyncio.get_running_loop().time() < deadline: + if await runner.is_pane_dead(window): + return True + await asyncio.sleep(0.05) + return await runner.is_pane_dead(window) + + +@pytest.mark.asyncio +async def test_new_window_appears_in_list(runner, tmp_path): + window_id = await runner.new_window("hqt-1", str(tmp_path), "sleep 60") + assert window_id is not None + assert "hqt-1" in await runner.list_windows() + + +@pytest.mark.asyncio +async def test_set_window_label_round_trips(runner, tmp_path, tmux_env): + await runner.new_window("hqt-1", str(tmp_path), "sleep 60") + await runner.set_window_label("hqt-1", "•hqt-1") + shown = _tmux( + tmux_env, + "show-options", + "-w", + "-t", + f"{SESSION}:=hqt-1", + "@hqt_label", + ).stdout + assert "•hqt-1" in shown + + +@pytest.mark.asyncio +async def test_respawn_revives_dead_pane(runner, tmp_path): + # Window whose command exits immediately -> pane dies (remain-on-exit keeps it). + await runner.new_window("hqt-1", str(tmp_path), "true") + assert await _wait_pane_dead(runner, "hqt-1") is True + assert await runner.respawn_pane("hqt-1", "sleep 60", str(tmp_path)) is True + alive, _ = await runner.verify_window_alive("hqt-1", timeout=1.0) + assert alive is True + + +@pytest.mark.asyncio +async def test_apply_theme_sets_session_options(runner, tmux_env): + await runner.apply_theme() + shown = _tmux(tmux_env, "show-options", "-t", SESSION, "status-justify").stdout + assert "left" in shown +``` + +- [ ] **Step 3: Run the smoke tests** + +Run: `uv run pytest tests/test_tmux_smoke.py -v` +Expected: PASS where tmux is installed (4 tests). If tmux is absent: all SKIPPED with reason "tmux not installed". + +- [ ] **Step 4: Verify the marker is registered (no warnings)** + +Run: `uv run pytest tests/test_tmux_smoke.py -v -W error::pytest.PytestUnknownMarkWarning` +Expected: PASS/SKIP with no `PytestUnknownMarkWarning` (proves the marker is registered). + +- [ ] **Step 5: Run the full suite + quality gates** + +Run: `uv run pytest && uv run ruff format src tests && uv run ruff check src tests && uv run ty check` +Expected: full suite green (smoke tests pass or skip), gates green. + +- [ ] **Step 6: Commit** + +```bash +git add tests/test_tmux_smoke.py pyproject.toml +git commit -m "test: add isolated real-tmux smoke tests + +Drive the real TmuxRunner against a throwaway tmux server isolated via +TMUX_TMPDIR (never touches live sessions), covering new windows, label +round-trip, respawn, and theming. Auto-skip when tmux is absent; register +the 'tmux' marker. Closes the P2 real-tmux testing gap. + +Co-Authored-By: Claude Fable 5 " +``` + +--- + +## Task 5: Remove resolved P2 findings from TODO.md + +**Files:** +- Modify: `TODO.md` + +**Context:** All three P2 findings are now resolved. `TODO.md` currently contains exactly those three lines and nothing else. + +- [ ] **Step 1: Empty out the resolved findings** + +`TODO.md` currently reads: + +``` +P2: Codex session-id capture is heuristic and can store the wrong rollout if another Codex session starts in the same project during the retry window. +P2: tmux behavior is mostly tested with mocks; add isolated real-tmux smoke tests for theming, new windows, respawn, and labels. +P2: Project paths are accepted without existence validation, so bad paths become dead sessions later instead of being rejected early. +``` + +Replace the entire file contents with a single placeholder line so the file is not left dangling: + +``` +No open findings. +``` + +- [ ] **Step 2: Verify** + +Run: `cat TODO.md` +Expected: `No open findings.` + +- [ ] **Step 3: Commit** + +```bash +git add TODO.md +git commit -m "chore: clear resolved P2 findings from TODO + +Co-Authored-By: Claude Fable 5 " +``` + +--- + +## Final verification (after all tasks) + +- [ ] Run the full suite and gates once more: + ``` + uv run pytest && uv run ruff format src tests && uv run ruff check src tests && uv run ty check + ``` + Expected: all tests pass (or smoke tests skip without tmux), all gates green. +- [ ] Confirm `src/hqt/tmux/runner.py` is still only modified in the working tree (its pre-existing WIP) and was never committed: + ``` + git log --oneline -6 + git status --short + ``` + Expected: the six new commits are present; `git status --short` shows only ` M src/hqt/tmux/runner.py`. From 1b79530214c1f6a31a8bce40410974c1e45a9387 Mon Sep 17 00:00:00 2001 From: Felix Martin Date: Wed, 10 Jun 2026 20:40:13 -0400 Subject: [PATCH 3/9] feat: validate project paths at create/update Reject non-existent or non-directory paths with ServiceError (already surfaced as a TUI notification), and store the resolved absolute path. Closes the P2 finding where bad paths became dead sessions later. Co-Authored-By: Claude Fable 5 --- src/hqt/projects/service.py | 16 +++++++ tests/test_services.py | 91 ++++++++++++++++++++++++++++--------- 2 files changed, 85 insertions(+), 22 deletions(-) diff --git a/src/hqt/projects/service.py b/src/hqt/projects/service.py index cfd2286..40496ee 100644 --- a/src/hqt/projects/service.py +++ b/src/hqt/projects/service.py @@ -1,3 +1,5 @@ +from pathlib import Path + from sqlalchemy.exc import IntegrityError from sqlalchemy.orm import sessionmaker @@ -11,7 +13,20 @@ class ProjectService: # from these methods stay readable after their session closes. self.factory = factory + def _normalize_path(self, raw: str) -> str: + """Expand ~, require an existing directory, return the resolved absolute path. + + Raises ServiceError if the path does not exist or is not a directory, so + the TUI surfaces it as a notification instead of letting the bad path + become a dead session later. + """ + path = Path(raw).expanduser() + if not path.is_dir(): + raise ServiceError(f"Path does not exist or is not a directory: {raw}") + return str(path.resolve()) + def create(self, name: str, path: str) -> Project: + path = self._normalize_path(path) with self.factory() as db: project = Project(name=name, path=path) db.add(project) @@ -34,6 +49,7 @@ class ProjectService: return db.get(Project, project_id) def update(self, project_id: int, name: str, path: str) -> Project: + path = self._normalize_path(path) with self.factory() as db: project = db.get(Project, project_id) if project is None: diff --git a/tests/test_services.py b/tests/test_services.py index 989c7da..0710a0a 100644 --- a/tests/test_services.py +++ b/tests/test_services.py @@ -24,48 +24,95 @@ def factory(): class TestProjectService: - def test_create_and_get(self, factory): + def test_create_and_get(self, factory, tmp_path): svc = ProjectService(factory) - p = svc.create("test", "/tmp/test") + p = svc.create("test", str(tmp_path)) assert p.id is not None assert svc.get(p.id).name == "test" - def test_list_excludes_archived(self, factory): + def test_list_excludes_archived(self, factory, tmp_path): + a = tmp_path / "a" + a.mkdir() + b = tmp_path / "b" + b.mkdir() svc = ProjectService(factory) - svc.create("a", "/a") - p2 = svc.create("b", "/b") + svc.create("a", str(a)) + p2 = svc.create("b", str(b)) svc.archive(p2.id) assert len(svc.list_all()) == 1 assert len(svc.list_all(include_archived=True)) == 2 - def test_update_name_and_path(self, factory): + def test_update_name_and_path(self, factory, tmp_path): + old = tmp_path / "old" + old.mkdir() + new = tmp_path / "new" + new.mkdir() svc = ProjectService(factory) - p = svc.create("old", "/old/path") - updated = svc.update(p.id, "new", "/new/path") + p = svc.create("old", str(old)) + updated = svc.update(p.id, "new", str(new)) assert updated.name == "new" - assert updated.path == "/new/path" - assert svc.get(p.id).path == "/new/path" + assert updated.path == str(new.resolve()) + assert svc.get(p.id).path == str(new.resolve()) - def test_update_duplicate_path_raises(self, factory): + def test_update_duplicate_path_raises(self, factory, tmp_path): + a = tmp_path / "a" + a.mkdir() + b = tmp_path / "b" + b.mkdir() svc = ProjectService(factory) - svc.create("a", "/a") - p2 = svc.create("b", "/b") + svc.create("a", str(a)) + p2 = svc.create("b", str(b)) with pytest.raises(ServiceError, match="already uses path"): - svc.update(p2.id, "b", "/a") + svc.update(p2.id, "b", str(a)) # DB session must remain usable after rollback, values unchanged - assert svc.get(p2.id).path == "/b" + assert svc.get(p2.id).path == str(b.resolve()) assert svc.get(p2.id).name == "b" - def test_update_unknown_id_raises(self, factory): + def test_update_unknown_id_raises(self, factory, tmp_path): svc = ProjectService(factory) with pytest.raises(ServiceError, match="not found"): - svc.update(9999, "x", "/x") + svc.update(9999, "x", str(tmp_path)) - def test_create_duplicate_path_raises_service_error(self, factory): + def test_create_duplicate_path_raises_service_error(self, factory, tmp_path): + dup = tmp_path / "dup" + dup.mkdir() svc = ProjectService(factory) - svc.create("a", "/dup") + svc.create("a", str(dup)) with pytest.raises(ServiceError, match="already uses path"): - svc.create("b", "/dup") + svc.create("b", str(dup)) + + def test_create_rejects_nonexistent_path(self, factory, tmp_path): + svc = ProjectService(factory) + missing = tmp_path / "does-not-exist" + with pytest.raises(ServiceError, match="does not exist or is not a directory"): + svc.create("p", str(missing)) + + def test_create_rejects_file_path(self, factory, tmp_path): + svc = ProjectService(factory) + a_file = tmp_path / "afile" + a_file.write_text("x") + with pytest.raises(ServiceError, match="does not exist or is not a directory"): + svc.create("p", str(a_file)) + + def test_create_stores_resolved_absolute_path(self, factory, tmp_path): + svc = ProjectService(factory) + p = svc.create("p", str(tmp_path)) + assert p.path == str(tmp_path.resolve()) + + def test_create_expands_user_home(self, factory, tmp_path, monkeypatch): + # Point ~ at tmp_path; "~" must expand to the existing tmp_path dir. + monkeypatch.setenv("HOME", str(tmp_path)) + svc = ProjectService(factory) + p = svc.create("p", "~") + assert p.path == str(tmp_path.resolve()) + + def test_update_rejects_nonexistent_path_and_leaves_row(self, factory, tmp_path): + svc = ProjectService(factory) + p = svc.create("p", str(tmp_path)) + missing = tmp_path / "nope" + with pytest.raises(ServiceError, match="does not exist or is not a directory"): + svc.update(p.id, "p", str(missing)) + assert svc.get(p.id).path == str(tmp_path.resolve()) class TestMcpService: @@ -78,10 +125,10 @@ class TestMcpService: svc.delete("test-mcp") assert svc.get("test-mcp") is None - def test_bind_unbind(self, factory): + def test_bind_unbind(self, factory, tmp_path): psvc = ProjectService(factory) msvc = McpService(factory) - project = psvc.create("proj", "/proj") + project = psvc.create("proj", str(tmp_path)) server = msvc.create("s1", "stdio", command="cmd") msvc.bind_to_project(project.id, server.id) assert len(msvc.get_project_mcps(project.id)) == 1 From f6a90e8f4d262868f9fc3e0c75cfbb1644116d8a Mon Sep 17 00:00:00 2001 From: Felix Martin Date: Wed, 10 Jun 2026 20:43:52 -0400 Subject: [PATCH 4/9] fix: refuse ambiguous Codex session-id capture When more than one rollout matches the project cwd within the capture window, return None and warn instead of guessing the earliest. The worst case is now a kept placeholder id, never a wrong one. Part of the P2 capture-race fix. Co-Authored-By: Claude Fable 5 --- src/hqt/harnesses/configurators/codex.py | 15 ++++++- tests/test_harnesses.py | 51 +++++------------------- 2 files changed, 23 insertions(+), 43 deletions(-) diff --git a/src/hqt/harnesses/configurators/codex.py b/src/hqt/harnesses/configurators/codex.py index dead135..10107f4 100644 --- a/src/hqt/harnesses/configurators/codex.py +++ b/src/hqt/harnesses/configurators/codex.py @@ -1,9 +1,12 @@ import json +import logging from datetime import datetime from pathlib import Path from hqt.harnesses.base import HarnessConfigurator, SpawnConfig +log = logging.getLogger(__name__) + class CodexConfigurator(HarnessConfigurator): name = "codex" @@ -73,5 +76,15 @@ class CodexConfigurator(HarnessConfigurator): candidates.append((started_at, payload["id"])) except (json.JSONDecodeError, KeyError, OSError, TypeError): continue - candidates.sort(key=lambda t: t[0]) + if len(candidates) > 1: + # Ambiguous: more than one Codex rollout matches this cwd + time + # window (e.g. a second Codex started in the same project). Guessing + # risks storing the wrong conversation id, so keep the placeholder. + log.warning( + "capture_session_id: %d rollouts match cwd=%s since=%s; refusing to guess", + len(candidates), + project_path, + since, + ) + return None return candidates[0][1] if candidates else None diff --git a/tests/test_harnesses.py b/tests/test_harnesses.py index 74dddb8..2e164a6 100644 --- a/tests/test_harnesses.py +++ b/tests/test_harnesses.py @@ -1,7 +1,6 @@ import json import os import time -from datetime import UTC, datetime from pathlib import Path from unittest.mock import patch @@ -184,57 +183,25 @@ def test_codex_capture_accepts_file_after_since(tmp_path): assert result == "new-id" -def test_codex_capture_first_post_since_cwd_match_wins(tmp_path): - """Among multiple post-since rollouts, the one started nearest since wins.""" +def test_codex_capture_ambiguous_returns_none(tmp_path): + """Two rollouts match cwd + since window -> refuse to guess, return None.""" sessions_dir = tmp_path / ".codex" / "sessions" / "dir" sessions_dir.mkdir(parents=True) - rollout_older = sessions_dir / "rollout-older.jsonl" - rollout_newer = sessions_dir / "rollout-newer.jsonl" - _write_rollout(rollout_older, "older-id", "/projects/foo") - _write_rollout(rollout_newer, "newer-id", "/projects/foo") + rollout_a = sessions_dir / "rollout-a.jsonl" + rollout_b = sessions_dir / "rollout-b.jsonl" + _write_rollout(rollout_a, "id-a", "/projects/foo") + _write_rollout(rollout_b, "id-b", "/projects/foo") now = time.time() since = now - 120 - os.utime(rollout_older, (now - 30, now - 30)) - os.utime(rollout_newer, (now - 10, now - 10)) + os.utime(rollout_a, (now - 30, now - 30)) + os.utime(rollout_b, (now - 10, now - 10)) c = CodexConfigurator() with patch("hqt.harnesses.configurators.codex.Path.home", return_value=tmp_path): result = c.capture_session_id(Path("/projects/foo"), since) - assert result == "older-id" - - -def test_codex_capture_prefers_started_nearest_since_over_newest_file(tmp_path): - """Late capture should match the Codex session spawned at since, not a later same-cwd session.""" - sessions_dir = tmp_path / ".codex" / "sessions" / "dir" - sessions_dir.mkdir(parents=True) - - target = sessions_dir / "rollout-target.jsonl" - unrelated_later = sessions_dir / "rollout-later.jsonl" - - since = datetime(2026, 6, 10, 21, 9, 15, tzinfo=UTC).timestamp() - _write_rollout( - target, - "target-id", - "/projects/foo", - timestamp="2026-06-10T21:09:16.000Z", - ) - _write_rollout( - unrelated_later, - "later-id", - "/projects/foo", - timestamp="2026-06-10T21:45:45.000Z", - ) - - now = time.time() - os.utime(target, (now - 60, now - 60)) - os.utime(unrelated_later, (now, now)) - - c = CodexConfigurator() - with patch("hqt.harnesses.configurators.codex.Path.home", return_value=tmp_path): - result = c.capture_session_id(Path("/projects/foo"), since) - assert result == "target-id" + assert result is None def test_codex_capture_skips_non_matching_cwd_picks_older_matching(tmp_path): From 6ed598b4433614019f31aec51857bbc872cd8b33 Mon Sep 17 00:00:00 2001 From: Felix Martin Date: Wed, 10 Jun 2026 20:47:35 -0400 Subject: [PATCH 5/9] fix: serialize Codex spawn->capture with a lock Hold an asyncio.Lock across the spawn->capture window for harnesses that capture a session id, so two concurrent hqt starts in the same project never produce overlapping capture windows. Completes the P2 capture-race fix alongside the ambiguity guard. Co-Authored-By: Claude Fable 5 --- src/hqt/sessions/service.py | 110 +++++++++++++++++++++--------------- tests/test_sessions.py | 50 ++++++++++++++++ 2 files changed, 113 insertions(+), 47 deletions(-) diff --git a/src/hqt/sessions/service.py b/src/hqt/sessions/service.py index 4289905..c54a6f0 100644 --- a/src/hqt/sessions/service.py +++ b/src/hqt/sessions/service.py @@ -1,4 +1,5 @@ import asyncio +import contextlib import logging import time from collections.abc import Mapping @@ -61,6 +62,19 @@ class SessionService: self.factory = factory self.tmux = tmux self.harnesses = harnesses + # Serializes the spawn->capture window for harnesses that capture a + # session id (codex), so two concurrent hqt starts in the same project + # never overlap and confuse capture_session_id. + self._capture_lock = asyncio.Lock() + + @contextlib.asynccontextmanager + async def _maybe_capture_lock(self, needed: bool): + """Hold the capture lock only when the harness captures a session id.""" + if needed: + async with self._capture_lock: + yield + else: + yield def _configurator(self, harness_name: str) -> HarnessConfigurator: configurator = self.harnesses.get(harness_name) @@ -171,30 +185,31 @@ class SessionService: spawn_cfg.command, spawn_cfg.cwd, ) - since = time.time() - spawn_result = await self.tmux.spawn( - SpawnRequest( - window_name=sess.tmux_session_name, - command=spawn_cfg.command, - cwd=str(spawn_cfg.cwd), - env=spawn_cfg.env, + async with self._maybe_capture_lock(configurator.captures_session_id): + since = time.time() + spawn_result = await self.tmux.spawn( + SpawnRequest( + window_name=sess.tmux_session_name, + command=spawn_cfg.command, + cwd=str(spawn_cfg.cwd), + env=spawn_cfg.env, + ) ) - ) - if not spawn_result.ok: - log.error( - "Failed to spawn window %s: %s", - sess.tmux_session_name, - spawn_result.error or "(no output captured)", - ) - # Only attempt capture when the spawn actually succeeded; a failed - # spawn could inadvertently capture an unrelated session running in - # the same cwd. - if spawn_result.ok and configurator.captures_session_id: - captured_id = await self._capture_session_id_with_retry( - configurator, project_path, since, sess.tmux_session_name - ) - if captured_id: - sess.harness_session_id = captured_id + if not spawn_result.ok: + log.error( + "Failed to spawn window %s: %s", + sess.tmux_session_name, + spawn_result.error or "(no output captured)", + ) + # Only attempt capture when the spawn actually succeeded; a failed + # spawn could inadvertently capture an unrelated session running in + # the same cwd. + if spawn_result.ok and configurator.captures_session_id: + captured_id = await self._capture_session_id_with_retry( + configurator, project_path, since, sess.tmux_session_name + ) + if captured_id: + sess.harness_session_id = captured_id db.commit() log.info( "Session created: id=%s window=%s", sess.id, sess.tmux_session_name @@ -261,33 +276,34 @@ class SessionService: spawn_cfg = configurator.build_spawn_config( project_path, harness_session_id, sess.model ) - since = time.time() - result = await self.tmux.respawn_verified( - window_name, spawn_cfg.command, str(spawn_cfg.cwd), env=spawn_cfg.env - ) - if not result.ok: - log.error( - "Fallback spawn also failed for %s: %s", - window_name, - result.error or "(no output)", + async with self._maybe_capture_lock(configurator.captures_session_id): + since = time.time() + result = await self.tmux.respawn_verified( + window_name, spawn_cfg.command, str(spawn_cfg.cwd), env=spawn_cfg.env ) - return False - - # Rung-2 succeeded — capture the new session id if the harness supports - # it so that future resumes target this fresh conversation. - if configurator.captures_session_id: - new_id = await self._capture_session_id_with_retry( - configurator, project_path, since, window_name - ) - if new_id: - sess.harness_session_id = new_id - db.commit() - else: - log.warning( - "capture_session_id exhausted after rung-2 for %s; keeping old id %s", + if not result.ok: + log.error( + "Fallback spawn also failed for %s: %s", window_name, - sess.harness_session_id, + result.error or "(no output)", ) + return False + + # Rung-2 succeeded — capture the new session id if the harness + # supports it so that future resumes target this fresh conversation. + if configurator.captures_session_id: + new_id = await self._capture_session_id_with_retry( + configurator, project_path, since, window_name + ) + if new_id: + sess.harness_session_id = new_id + db.commit() + else: + log.warning( + "capture_session_id exhausted after rung-2 for %s; keeping old id %s", + window_name, + sess.harness_session_id, + ) return True async def stop_session(self, session_id: int) -> None: diff --git a/tests/test_sessions.py b/tests/test_sessions.py index 60eabc2..d5895e1 100644 --- a/tests/test_sessions.py +++ b/tests/test_sessions.py @@ -1106,3 +1106,53 @@ async def test_list_sessions_skips_capture_for_deleted_project( harnesses_capture["claude-code"].capture_session_id.return_value = "would-capture" infos = await service.list_sessions(1) assert len(infos) == 1 # no ServiceError escaped + + +# --------------------------------------------------------------------------- +# Task 3 (P2): serialize the spawn->capture window with a lock +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_create_session_holds_capture_lock_for_capturing_harness( + factory, db, tmux +): + observed = {} + + def _capture(project_path, since): + # capture runs inside the spawn->capture critical section + observed["locked_during_capture"] = service._capture_lock.locked() + return "real-codex-id" + + h = MagicMock() + h.captures_session_id = True + h.generate_session_id.return_value = "placeholder" + h.build_spawn_config.return_value = MagicMock( + command=["codex"], env={}, cwd=Path("/tmp/myproj") + ) + h.capture_session_id.side_effect = _capture + service = SessionService(factory=factory, tmux=tmux, harnesses={"claude-code": h}) + + result = await service.create_session(project_id=1, harness_name="claude-code") + + assert observed["locked_during_capture"] is True + assert result.session.harness_session_id == "real-codex-id" + # Lock is released after create_session returns. + assert service._capture_lock.locked() is False + + +@pytest.mark.asyncio +async def test_create_session_no_lock_for_non_capturing_harness(service, db, tmux): + observed = {} + + async def _spawn(req): + observed["locked_during_spawn"] = service._capture_lock.locked() + return SpawnResult(ok=True, window_id="@1", error="") + + tmux.spawn.side_effect = _spawn + + await service.create_session(project_id=1, harness_name="claude-code") + + # The `service` fixture's harness has captures_session_id = False, so the + # spawn must NOT be serialized under the capture lock. + assert observed["locked_during_spawn"] is False From cabac16fd4925a705f0f3d4462cb1f80be23e3d4 Mon Sep 17 00:00:00 2001 From: Felix Martin Date: Wed, 10 Jun 2026 20:54:09 -0400 Subject: [PATCH 6/9] docs: design spec for bubblewrap-sandboxed sessions Co-Authored-By: Claude Opus 4.8 --- .../2026-06-10-sandboxed-sessions-design.md | 204 ++++++++++++++++++ 1 file changed, 204 insertions(+) create mode 100644 docs/superpowers/specs/2026-06-10-sandboxed-sessions-design.md diff --git a/docs/superpowers/specs/2026-06-10-sandboxed-sessions-design.md b/docs/superpowers/specs/2026-06-10-sandboxed-sessions-design.md new file mode 100644 index 0000000..27ac26a --- /dev/null +++ b/docs/superpowers/specs/2026-06-10-sandboxed-sessions-design.md @@ -0,0 +1,204 @@ +# Sandboxed sessions via bubblewrap — design + +**Date:** 2026-06-10 +**Status:** Approved (pending implementation plan) + +## Problem + +Users want to start harness sessions inside a [bubblewrap](https://github.com/containers/bubblewrap) +(`bwrap`) sandbox. Inside the jail the agent can run freely — so the harness is +launched with its "skip permissions" flag (`--dangerously-skip-permissions` for +Claude Code, the equivalent for other harnesses) — while bubblewrap provides the +real safety boundary: controlled filesystem access (read-only or read-write to +the project directory) and optional network access. The configuration must be +quick to set per session. + +## Decisions (from brainstorming) + +- **Config scope:** per session, chosen in the New Session dialog and stored on + the session row so resume/respawn reuse it. +- **Controls:** a "Sandboxed" toggle plus two sub-toggles — filesystem mode + (read-only / read-write) and network (on / off). +- **Skip-permissions:** automatic. Sandbox on ⇒ the harness's skip-permissions + flag is added. The bwrap container is the safety boundary, so there is no + separate widget for it. +- **Filesystem model:** minimal allowlist. User data (`$HOME` and other + directories) is hidden. System directories are bound read-only so binaries and + libraries work; the project directory and a per-harness set of + credential/config paths are bound explicitly. +- **Defaults when sandbox is enabled:** read-write project dir, network on (the + common "let it work, but contained" case; the harness needs network to reach a + hosted model API). +- **bwrap missing:** hard failure, never a silent downgrade to unsandboxed. + +## Key constraint: network is all-or-nothing + +bubblewrap toggles networking at the namespace level — it cannot selectively +allow the model API while blocking everything else. Therefore: + +- **Network on** = the sandbox shares the host network namespace; the agent (and + its tools: `curl`, `git push`, etc.) can reach anything the host can. +- **Network off** = no connectivity at all. The harness process itself cannot + reach a hosted model API, so this mode is only useful with local/offline + models. The toggle's value is blocking the agent's network for offline or + untrusted-code review. + +This constraint is surfaced in the UI defaults (network defaults on). + +## Architecture + +### 1. Data model + +Add one nullable column to `Session`: + +```python +sandbox_json: Mapped[str | None] = mapped_column(default=None) +``` + +`NULL` = unsandboxed. When set it holds: + +```json +{ "fs": "rw" | "ro", "net": true } +``` + +Presence of the value means sandboxed; `fs` and `net` capture the two +sub-toggles. Skip-permissions is *derived* (sandboxed ⇒ on) and is not stored. +A single `user_version`-based migration (see `db/migrations.py`) adds the column. + +Because the config lives on the row, `attach_session` → `_respawn_with_fallback` +and `_get_resume_config` rebuild the same sandboxed command after a stop. + +### 2. The configuration ABC — `HarnessConfigurator` + +Per-harness knowledge lives here. Two additions to `harnesses/base.py`: + +```python +class Bind: + """A path to expose inside the sandbox.""" + src: Path + writable: bool = False + +class HarnessConfigurator(ABC): + ... + sandbox_skip_permission_flags: list[str] = [] + + def sandbox_binds(self) -> list[Bind]: + return [] +``` + +Per-harness values: + +| Harness | `sandbox_skip_permission_flags` | `sandbox_binds()` | +|-----------|----------------------------------------------|--------------------------| +| `claude` | `["--dangerously-skip-permissions"]` | `~/.claude` (rw) | +| `codex` | `["--dangerously-bypass-approvals-and-sandbox"]` | `~/.codex` (rw) | +| `kiro` | `[]` | `~/.kiro` (rw) | +| `generic` | `[]` | `[]` | + +The exact flag spelling and credential paths must be verified against each +installed binary during implementation (e.g. `claude --help`); the values above +are the expected defaults. + +`build_spawn_config` / `build_resume_config` gain a `sandboxed: bool = False` +parameter. When true, the configurator appends +`self.sandbox_skip_permission_flags` to the command list. The configurator does +**not** construct the bwrap invocation — it only declares flags and binds. + +### 3. Bubblewrap policy module — `hqt/sandbox.py` + +A pure, argv-only function: + +```python +@dataclass(frozen=True) +class SandboxPolicy: + fs: str # "rw" | "ro" + net: bool + +def wrap( + command: list[str], + cwd: Path, + policy: SandboxPolicy, + binds: list[Bind], +) -> list[str]: + """Return the `bwrap … -- ` argv.""" +``` + +Assembled from three layers: + +- **Base (always):** `--unshare-all`, `--die-with-parent`; RO binds of system + dirs (`/usr`, `/bin`, `/lib`, `/lib64`, and curated `/etc` essentials such as + `resolv.conf`, `ssl`, `passwd`); `--proc /proc`; `--dev /dev`; `--tmpfs /tmp`; + pass-through of `PATH`, `HOME`, `TERM`, `LANG`. `$HOME` itself is not bound, so + user data is hidden. `~/.gitconfig` bound RO when present. +- **Harness binds:** each `Bind` from the configurator, `--bind` (writable) or + `--ro-bind`, creating parent dirs as needed. +- **cwd:** `--bind cwd cwd` when `fs == "rw"`, else `--ro-bind cwd cwd`. +- **net:** when `policy.net` is true, the network namespace is shared (omit the + net-unshare); otherwise it stays unshared and there is no connectivity. + +Being pure and producing only argv (no subprocess), `wrap` is unit-testable +without bwrap installed. + +### 4. Spawn / resume integration — `SessionService` + +In `create_session`: call `build_spawn_config(..., sandboxed=True)` when the +request is sandboxed, then pass `SpawnConfig.command` through `sandbox.wrap(...)` +(using the configurator's `sandbox_binds()` and the session policy) before +constructing the `SpawnRequest`. `cwd` remains the project path — bwrap binds it. + +The same wrapping applies in `_respawn_with_fallback` (both the resume rung and +the fresh-spawn rung) and in `_get_resume_config`, reading the policy back from +`sandbox_json`. + +`create_session` gains a `sandbox` parameter (the parsed policy or `None`). + +### 5. UI — New Session dialog + +`tui/screens/new_session.py` adds: + +- A `Switch` labelled "Sandboxed" (default off). +- Revealed when on: a filesystem `Select` ("Read-write" / "Read-only", default + Read-write) and a "Network" `Switch` (default on). + +The dialog's result tuple is extended to carry the sandbox config (or `None`), +threaded into `create_session`. + +**bwrap availability gates the toggle.** The dialog checks bubblewrap +availability (the same check `doctor` uses — see §6) on mount. When bwrap is +unavailable, the "Sandboxed" switch is disabled (cannot be turned on) and an +inline warning explains why (e.g. "bubblewrap not found — run `hqt doctor`"). +This makes the unavailable state visible up front rather than only at spawn. + +### 6. Availability check, doctor & failure handling + +- A single shared helper (e.g. `sandbox.is_available()`) reports whether `bwrap` + is on `PATH` and the platform is Linux. Both `doctor` and the New Session + dialog use it, so there is one source of truth. +- `hqt doctor` reports bubblewrap availability as an optional capability + (present / missing), alongside the existing checks. +- The dialog uses the helper to disable the Sandboxed toggle (§5), so an + unsandboxable environment is caught before the user picks anything. +- As a backstop (defense in depth), if a sandboxed session somehow reaches spawn + while bwrap is unavailable, spawn fails loudly with a `ServiceError` — never a + silent downgrade to an unsandboxed session. + +## Testing + +- **`sandbox.wrap` unit tests:** assert the argv for each toggle combination + (rw/ro × net/no-net), that harness binds are spliced in with the right + `--bind`/`--ro-bind`, and that the command appears after `--`. +- **Configurator tests:** skip-permission flags are appended only when + `sandboxed=True`; absent otherwise. +- **Service test:** a sandboxed `create_session` wraps the command in `bwrap`; + a bwrap-missing environment raises `ServiceError`. +- **Availability/UI test:** `sandbox.is_available()` is false when `bwrap` is + absent or the platform is non-Linux, and the dialog disables the Sandboxed + toggle in that case. +- **Migration test:** the new column is added and round-trips a policy. + +## Out of scope (YAGNI for v1) + +- Per-project or named global sandbox profiles (per-session only for now). +- A freeform "extra bind paths" field in the dialog. +- Selective network filtering (proxy/firewall) — bwrap is namespace-level only. +- Remembering the last-used sandbox config as a default. From 5c63d0fbecb6f2664030f0808e3eac7b52efabba Mon Sep 17 00:00:00 2001 From: Felix Martin Date: Wed, 10 Jun 2026 20:56:33 -0400 Subject: [PATCH 7/9] docs: spec + plan for Alt+p tool palette (nvim/lazygit/shell/clone) Adds a single tmux Alt+p fzf palette that opens nvim/lazygit/shell as styled tool windows for the current session's project, or clones a fresh harness with the same project+model. Bridges via run-shell (expands #{window_name}) into a new 'hqt palette' CLI subcommand, since display-popup does not format-expand its command (verified on tmux 3.6b). Co-Authored-By: Claude Opus 4.8 --- .../plans/2026-06-10-tool-windows.md | 961 ++++++++++++++++++ .../specs/2026-06-10-tool-windows-design.md | 269 +++++ 2 files changed, 1230 insertions(+) create mode 100644 docs/superpowers/plans/2026-06-10-tool-windows.md create mode 100644 docs/superpowers/specs/2026-06-10-tool-windows-design.md diff --git a/docs/superpowers/plans/2026-06-10-tool-windows.md b/docs/superpowers/plans/2026-06-10-tool-windows.md new file mode 100644 index 0000000..a10dd42 --- /dev/null +++ b/docs/superpowers/plans/2026-06-10-tool-windows.md @@ -0,0 +1,961 @@ +# Tool Palette (nvim / lazygit / shell / clone) Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** An `Alt+p` fuzzy palette (one tmux binding) that, for the current hqt session's project, opens nvim/lazygit/shell in a new hqt-styled tmux window, or `clone`s a fresh harness session with the same project + harness + model. + +**Architecture:** A tool registry feeds a low-level `TmuxRunner.new_aux_window` (spawn + style + switch a non-session window, targeting by `window_id`, no `remain-on-exit`). `SessionService` resolves a tmux window name to a session and either opens a tool window or, for `clone`, reuses `create_session`. Two CLI subcommands bridge tmux to the service: `hqt palette ` (builds the fzf `display-popup`) and `hqt tool ` (dispatches tool vs. clone). The binding is `bind -n M-p run-shell -b "hqt palette '#{window_name}'"` — `run-shell` format-expands `#{window_name}` (verified), `display-popup` does not (verified), so the resolved name is baked into the popup as a literal. + +**Tech Stack:** Python 3, async/await, tmux CLI (3.6b), Textual, click, pytest + unittest.mock, fzf. + +**Conventions:** TDD per task (test → see it fail → implement → see it pass → commit). All commit commands include the `Co-Authored-By: Claude Opus 4.8 ` trailer. **Work happens on `main`** (the user asked to commit there directly). The design spec lives at `docs/superpowers/specs/2026-06-10-tool-windows-design.md`. + +--- + +## File Structure + +- **Create** `src/hqt/tools.py` — `Tool` dataclass + `TOOLS` registry. One job: name → aux spawn spec. +- **Modify** `src/hqt/tmux/runner.py` — add `new_aux_window` (+ `import shlex`). +- **Modify** `src/hqt/tmux/manager.py` — add `open_aux_window` delegate. +- **Modify** `src/hqt/sessions/service.py` — add `session_id_for_window`, `open_tool_window`, `open_tool_window_for_window`, `clone_session_for_window` (+ `import shutil`, `from hqt.tools import TOOLS`). +- **Modify** `src/hqt/cli.py` — `_build_session_service` helper, `hqt tool` and `hqt palette` subcommands. +- **Modify** `~/.tmux.conf` — one `M-p` palette binding (user-owned file; manual verify via `source-file`). +- **Tests:** `tests/test_tools.py` (new), `tests/test_tmux.py`, `tests/test_sessions.py`, `tests/test_cli.py` (new if absent). + +No TUI changes: per the design decision, Alt+p (tmux) is the only trigger. + +--- + +## Task 1: Tool registry + +**Files:** +- Create: `src/hqt/tools.py` +- Test: `tests/test_tools.py` + +- [ ] **Step 1: Write the failing test** + +Create `tests/test_tools.py`: + +```python +from hqt.tools import TOOLS, Tool + + +def test_registry_has_the_three_tools(): + assert set(TOOLS) == {"nvim", "lazygit", "shell"} + + +def test_each_entry_is_a_tool(): + assert all(isinstance(t, Tool) for t in TOOLS.values()) + + +def test_nvim_and_lazygit_have_commands(): + assert TOOLS["nvim"].command == ["nvim"] + assert TOOLS["lazygit"].command == ["lazygit"] + + +def test_shell_has_empty_command_meaning_default_shell(): + assert TOOLS["shell"].command == [] + + +def test_labels_are_the_bare_tool_names(): + assert TOOLS["nvim"].label == "nvim" + assert TOOLS["lazygit"].label == "lazygit" + assert TOOLS["shell"].label == "shell" +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `uv run pytest tests/test_tools.py -v` +Expected: FAIL with `ModuleNotFoundError: No module named 'hqt.tools'`. + +- [ ] **Step 3: Write minimal implementation** + +Create `src/hqt/tools.py`: + +```python +"""Tools that can be opened in their own tmux window for a session's project. + +A tool window is NOT an hqt session: hqt spawns and styles it, then forgets it. +It lives purely as a tmux window until the tool exits. (``clone`` is handled +separately in the service — it creates a real session, not an aux window.) +""" + +from dataclasses import dataclass + + +@dataclass(frozen=True) +class Tool: + """How to open a tool in its own window. + + ``label`` is the base ``@hqt_label`` text (the project name is appended per + spawn). ``command`` is the argv to run; an empty list means "use tmux's + default shell" (a plain interactive shell). + """ + + label: str + command: list[str] + + +TOOLS: dict[str, Tool] = { + "nvim": Tool(label="nvim", command=["nvim"]), + "lazygit": Tool(label="lazygit", command=["lazygit"]), + "shell": Tool(label="shell", command=[]), +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `uv run pytest tests/test_tools.py -v` +Expected: PASS (5 passed). + +- [ ] **Step 5: Commit** + +```bash +git add src/hqt/tools.py tests/test_tools.py +git commit -m "Add tool registry for tool windows" \ + -m "Co-Authored-By: Claude Opus 4.8 " +``` + +--- + +## Task 2: `TmuxRunner.new_aux_window` + +**Files:** +- Modify: `src/hqt/tmux/runner.py` (add `import shlex` near the top; add the method after `new_window`) +- Test: `tests/test_tmux.py` + +Background: `_next_window_index()` runs `list-windows -F '#{window_index}'` and returns `max(indices) + 1`. The `runner` fixture in `tests/test_tmux.py` uses `session_name="hqt-main"` and stubs `_exec` with an `AsyncMock`, so a side-effect queue drives each tmux call. `_window_theme_args(target)` builds the Frappé per-window `set-option -w -t ...` argv (no leading/trailing `;`). + +- [ ] **Step 1: Write the failing tests** + +Append to `tests/test_tmux.py`: + +```python +@pytest.mark.asyncio +async def test_new_aux_window_spawns_styles_and_selects(runner): + runner._exec.side_effect = [ + (0, "0\n", ""), # _next_window_index: indices [0] -> next index 1 + (0, "@7\n", ""), # new-window -P -F '#{window_id}' + (0, "", ""), # set-option (automatic-rename + @hqt_label + theme) + (0, "", ""), # select-window + ] + wid = await runner.new_aux_window("lazygit", "/proj", ["lazygit"], "lazygit · proj") + assert wid == "@7" + + calls = runner._exec.call_args_list + # new-window: next free index, name, cwd, print window_id, then the command. + assert calls[1].args == ( + "new-window", "-t", "hqt-main:1", "-n", "lazygit", + "-c", "/proj", "-P", "-F", "#{window_id}", "lazygit", + ) + # post-creation options target the window_id and NEVER set remain-on-exit. + assert calls[2].args[:6] == ( + "set-option", "-w", "-t", "@7", "automatic-rename", "off", + ) + assert "@hqt_label" in calls[2].args + assert "lazygit · proj" in calls[2].args + assert "remain-on-exit" not in calls[2].args + # switch to it. + assert calls[3].args == ("select-window", "-t", "@7") + + +@pytest.mark.asyncio +async def test_new_aux_window_shell_omits_command_arg(runner): + runner._exec.side_effect = [ + (0, "2\n", ""), # indices [2] -> next index 3 + (0, "@9\n", ""), + (0, "", ""), + (0, "", ""), + ] + wid = await runner.new_aux_window("shell", "/proj", [], "shell · proj") + assert wid == "@9" + calls = runner._exec.call_args_list + assert calls[1].args == ( + "new-window", "-t", "hqt-main:3", "-n", "shell", + "-c", "/proj", "-P", "-F", "#{window_id}", + ) + + +@pytest.mark.asyncio +async def test_new_aux_window_returns_none_when_new_window_fails(runner): + runner._exec.side_effect = [ + (0, "0\n", ""), # next index + (1, "", "boom"), # new-window fails + ] + wid = await runner.new_aux_window("nvim", "/p", ["nvim"], "nvim · p") + assert wid is None +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `uv run pytest tests/test_tmux.py -k new_aux_window -v` +Expected: FAIL with `AttributeError: 'TmuxRunner' object has no attribute 'new_aux_window'`. + +- [ ] **Step 3: Write the implementation** + +In `src/hqt/tmux/runner.py`, add `import shlex` to the top import block (it currently reads `import asyncio` / `import logging` / `from dataclasses import dataclass` — insert `import shlex` alphabetically after `import logging`). + +Then add this method immediately after `new_window` (after its `return window_id`): + +```python + async def new_aux_window( + self, name: str, cwd: str, command: list[str], label: str + ) -> str | None: + """Create an auxiliary (non-session) tool window and switch to it. + + Appends at the next free index, then styles by window_id — not name — so + duplicate names (tool windows are spawned fresh every time) stay + unambiguous. Deliberately does NOT set remain-on-exit: the window closes + when the tool exits (a quit shell closes its window too). The window is + never tracked by hqt; it lives purely as a tmux window. + + Returns the window_id, or None on failure (the half-created window is + cleaned up). + """ + idx = await self._next_window_index() + args = [ + "new-window", + "-t", + f"{self.session_name}:{idx}", + "-n", + name, + "-c", + cwd, + "-P", + "-F", + "#{window_id}", + ] + if command: + args.append(shlex.join(command)) + rc, stdout, err = await self._exec(*args) + if rc != 0: + log.error("new-window (aux) failed: %s", err) + return None + window_id = stdout.strip() + + # Style by window_id: automatic-rename off, the @hqt_label, then the + # Frappé per-window theme — one atomic invocation (";" argv separators). + rc, _, err = await self._exec( + "set-option", + "-w", + "-t", + window_id, + "automatic-rename", + "off", + ";", + "set-option", + "-w", + "-t", + window_id, + "@hqt_label", + label, + ";", + *_window_theme_args(window_id), + ) + if rc != 0: + log.error("set-option (aux window) failed for %s: %s", name, err) + await self._exec("kill-window", "-t", window_id) + return None + + await self._exec("select-window", "-t", window_id) + return window_id +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `uv run pytest tests/test_tmux.py -k new_aux_window -v` +Expected: PASS (3 passed). + +- [ ] **Step 5: Commit** + +```bash +git add src/hqt/tmux/runner.py tests/test_tmux.py +git commit -m "Add TmuxRunner.new_aux_window for styled tool windows" \ + -m "Co-Authored-By: Claude Opus 4.8 " +``` + +--- + +## Task 3: `TmuxManager.open_aux_window` + +**Files:** +- Modify: `src/hqt/tmux/manager.py` (add method to `TmuxManager`) +- Test: `tests/test_tmux.py` + +- [ ] **Step 1: Write the failing test** + +Append to `tests/test_tmux.py`: + +```python +@pytest.mark.asyncio +async def test_manager_open_aux_window_delegates(runner): + from unittest.mock import AsyncMock + + runner.new_aux_window = AsyncMock(return_value="@4") + mgr = TmuxManager(runner) + wid = await mgr.open_aux_window("nvim", "/p", ["nvim"], "nvim · p") + assert wid == "@4" + runner.new_aux_window.assert_awaited_once_with("nvim", "/p", ["nvim"], "nvim · p") +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `uv run pytest tests/test_tmux.py -k manager_open_aux -v` +Expected: FAIL with `AttributeError: 'TmuxManager' object has no attribute 'open_aux_window'`. + +- [ ] **Step 3: Write the implementation** + +In `src/hqt/tmux/manager.py`, add this method to `TmuxManager` (e.g. after `set_window_label`): + +```python + async def open_aux_window( + self, name: str, cwd: str, command: list[str], label: str + ) -> str | None: + """Open a styled tool window (non-session) and switch to it. + + Returns the new window_id, or None on failure. + """ + return await self.runner.new_aux_window(name, cwd, command, label) +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `uv run pytest tests/test_tmux.py -k manager_open_aux -v` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add src/hqt/tmux/manager.py tests/test_tmux.py +git commit -m "Add TmuxManager.open_aux_window delegate" \ + -m "Co-Authored-By: Claude Opus 4.8 " +``` + +--- + +## Task 4: `SessionService` tool-window methods + +**Files:** +- Modify: `src/hqt/sessions/service.py` (add `import shutil`, `from hqt.tools import TOOLS`, three methods) +- Test: `tests/test_sessions.py` + +Background: `ServiceError` comes from `hqt.errors`. The `tmux` fixture is `MagicMock(spec=TmuxManager)`, so `open_aux_window` is allowed once Task 3 added it; tests set it to an `AsyncMock`. The seeded project is `Project(name="myproj", path="/tmp/myproj")` at `project_id=1`; `create_session` makes window `hqt-1`. `Project`/`Session` and `selectinload`/`sessionmaker` are already imported in the service. + +- [ ] **Step 1: Write the failing tests** + +Append to `tests/test_sessions.py`: + +```python +@pytest.mark.asyncio +async def test_open_tool_window_spawns_styled_window(service, db, tmux, monkeypatch): + await service.create_session(project_id=1, harness_name="claude-code") + monkeypatch.setattr("hqt.sessions.service.shutil.which", lambda b: "/usr/bin/" + b) + tmux.open_aux_window = AsyncMock(return_value="@5") + + wid = await service.open_tool_window(1, "lazygit") + + assert wid == "@5" + tmux.open_aux_window.assert_awaited_once_with( + "lazygit", "/tmp/myproj", ["lazygit"], "lazygit · myproj" + ) + + +@pytest.mark.asyncio +async def test_open_tool_window_shell_skips_which_check(service, db, tmux, monkeypatch): + await service.create_session(project_id=1, harness_name="claude-code") + monkeypatch.setattr("hqt.sessions.service.shutil.which", lambda b: None) + tmux.open_aux_window = AsyncMock(return_value="@3") + + wid = await service.open_tool_window(1, "shell") + + assert wid == "@3" + tmux.open_aux_window.assert_awaited_once_with( + "shell", "/tmp/myproj", [], "shell · myproj" + ) + + +@pytest.mark.asyncio +async def test_open_tool_window_unknown_tool_raises(service): + with pytest.raises(ServiceError): + await service.open_tool_window(1, "emacs") + + +@pytest.mark.asyncio +async def test_open_tool_window_missing_binary_raises(service, db, monkeypatch): + await service.create_session(project_id=1, harness_name="claude-code") + monkeypatch.setattr("hqt.sessions.service.shutil.which", lambda b: None) + with pytest.raises(ServiceError): + await service.open_tool_window(1, "lazygit") + + +@pytest.mark.asyncio +async def test_open_tool_window_unknown_session_raises(service, monkeypatch): + monkeypatch.setattr("hqt.sessions.service.shutil.which", lambda b: "/usr/bin/" + b) + with pytest.raises(ServiceError): + await service.open_tool_window(999, "nvim") + + +def test_session_id_for_window_resolves_and_misses(service, db): + import asyncio + + asyncio.run(service.create_session(project_id=1, harness_name="claude-code")) + assert service.session_id_for_window("hqt-1") == 1 + assert service.session_id_for_window("not-an-hqt-window") is None + + +@pytest.mark.asyncio +async def test_open_tool_window_for_window_resolves_by_name( + service, db, tmux, monkeypatch +): + await service.create_session(project_id=1, harness_name="claude-code") + monkeypatch.setattr("hqt.sessions.service.shutil.which", lambda b: "/usr/bin/" + b) + tmux.open_aux_window = AsyncMock(return_value="@2") + + wid = await service.open_tool_window_for_window("hqt-1", "nvim") + + assert wid == "@2" + tmux.open_aux_window.assert_awaited_once_with( + "nvim", "/tmp/myproj", ["nvim"], "nvim · myproj" + ) + + +@pytest.mark.asyncio +async def test_open_tool_window_for_window_unknown_window_raises(service): + with pytest.raises(ServiceError): + await service.open_tool_window_for_window("not-an-hqt-window", "nvim") +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `uv run pytest tests/test_sessions.py -k "tool_window or session_id_for_window" -v` +Expected: FAIL with `AttributeError: 'SessionService' object has no attribute 'open_tool_window'`. + +- [ ] **Step 3: Write the implementation** + +In `src/hqt/sessions/service.py`: add `import shutil` with the other stdlib imports, and `from hqt.tools import TOOLS` with the other `hqt` imports. + +Add these three methods to `SessionService` (e.g. after `attach_session`, before `_status_for`): + +```python + def session_id_for_window(self, window_name: str) -> int | None: + """Resolve a tmux window name to its active hqt session id, or None. + + None means the window is not an hqt session window (a tool window, the + TUI home window, or an unrelated tmux window). + """ + with self.factory() as db: + sess = ( + db.query(Session) + .filter_by(tmux_session_name=window_name, archived=False) + .first() + ) + return sess.id if sess else None + + async def open_tool_window(self, session_id: int, tool: str) -> str | None: + """Open a tool (nvim/lazygit/shell) in a new styled window for a session. + + Spawns at the next free index in the session's project directory and + switches to it. The window is NOT a tracked session. Raises ServiceError + for an unknown tool, a missing binary, or a missing session/project. + Returns the new window_id, or None if the tmux spawn fails. + """ + spec = TOOLS.get(tool) + if spec is None: + raise ServiceError(f"Unknown tool '{tool}'") + if spec.command and shutil.which(spec.command[0]) is None: + raise ServiceError(f"{spec.command[0]} not found on PATH") + with self.factory() as db: + sess = db.get(Session, session_id) + if sess is None: + raise ServiceError("Session not found") + project = db.get(Project, sess.project_id) + if project is None: + raise ServiceError("Project no longer exists") + cwd = project.path + label = f"{spec.label} · {project.name}" + return await self.tmux.open_aux_window(spec.label, cwd, spec.command, label) + + async def open_tool_window_for_window( + self, window_name: str, tool: str + ) -> str | None: + """Open a tool window for the session identified by its tmux window name. + + Used by the `hqt tool` CLI (the tmux binding passes #{window_name}). + Raises ServiceError if the name is not an active hqt session window. + """ + session_id = self.session_id_for_window(window_name) + if session_id is None: + raise ServiceError(f"{window_name!r} is not an hqt session window") + return await self.open_tool_window(session_id, tool) +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `uv run pytest tests/test_sessions.py -k "tool_window or session_id_for_window" -v` +Expected: PASS (8 passed). + +- [ ] **Step 5: Commit** + +```bash +git add src/hqt/sessions/service.py tests/test_sessions.py +git commit -m "Add SessionService tool-window + window-resolution methods" \ + -m "Co-Authored-By: Claude Opus 4.8 " +``` + +--- + +## Task 5: `SessionService.clone_session_for_window` + +**Files:** +- Modify: `src/hqt/sessions/service.py` (add one method) +- Test: `tests/test_sessions.py` + +Background: `create_session(project_id, harness_name, nickname, model)` returns a `CreateSessionResult` and spawns the harness window. clone reads the source session's project/harness/model and delegates. The seeded harness is `"claude-code"`; `selectinload` and `CreateSessionResult` are already in the module. + +- [ ] **Step 1: Write the failing tests** + +Append to `tests/test_sessions.py`: + +```python +@pytest.mark.asyncio +async def test_clone_session_for_window_reuses_project_harness_model( + service, db, monkeypatch +): + await service.create_session( + project_id=1, harness_name="claude-code", nickname="orig", model="opus" + ) + captured = {} + + async def fake_create(project_id, harness_name, nickname=None, model=None): + captured["args"] = (project_id, harness_name, nickname, model) + return "SENTINEL" + + monkeypatch.setattr(service, "create_session", fake_create) + + result = await service.clone_session_for_window("hqt-1") + + assert result == "SENTINEL" + # Same project + harness + model; a fresh sibling, so no nickname. + assert captured["args"] == (1, "claude-code", None, "opus") + + +@pytest.mark.asyncio +async def test_clone_session_for_window_unknown_window_raises(service): + with pytest.raises(ServiceError): + await service.clone_session_for_window("not-an-hqt-window") +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `uv run pytest tests/test_sessions.py -k clone_session_for_window -v` +Expected: FAIL with `AttributeError: 'SessionService' object has no attribute 'clone_session_for_window'`. + +- [ ] **Step 3: Write the implementation** + +In `src/hqt/sessions/service.py`, add this method to `SessionService` (next to the other tool methods from Task 4): + +```python + async def clone_session_for_window( + self, window_name: str + ) -> "CreateSessionResult": + """Open a fresh harness session cloning the one in `window_name`. + + Same project, harness, and model as the source session, but a brand-new + conversation (a new hqt- window at the next index). Raises + ServiceError if `window_name` is not an active hqt session window — so + invoking clone from a tool window (nvim/shell) or the TUI is a clean + no-op. + """ + with self.factory() as db: + sess = ( + db.query(Session) + .options(selectinload(Session.harness)) + .filter_by(tmux_session_name=window_name, archived=False) + .first() + ) + if sess is None: + raise ServiceError(f"{window_name!r} is not an hqt session window") + project_id = sess.project_id + harness_name = sess.harness.name + model = sess.model + return await self.create_session( + project_id, harness_name, nickname=None, model=model + ) +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `uv run pytest tests/test_sessions.py -k clone_session_for_window -v` +Expected: PASS (2 passed). + +- [ ] **Step 5: Commit** + +```bash +git add src/hqt/sessions/service.py tests/test_sessions.py +git commit -m "Add SessionService.clone_session_for_window" \ + -m "Co-Authored-By: Claude Opus 4.8 " +``` + +--- + +## Task 6: CLI `hqt tool` subcommand (tool + clone dispatch) + +**Files:** +- Modify: `src/hqt/cli.py` (add `_build_session_service` helper + `tool` subcommand) +- Test: `tests/test_cli.py` (create if absent) + +Background: existing subcommands (`doctor`, `list`) import their deps inside the function. `Settings` accepts `db_path=`. `ensure_db` creates the sqlite file. The helper mirrors `HqtApp.on_mount`'s wiring. + +- [ ] **Step 1: Write the failing tests** + +Create (or append to) `tests/test_cli.py`: + +```python +import pytest +from click.testing import CliRunner + +from hqt import cli +from hqt.config import Settings + + +def test_tool_cmd_opens_tool_window(monkeypatch, tmp_path): + monkeypatch.setattr( + "hqt.config.get_settings", lambda: Settings(db_path=tmp_path / "t.db") + ) + calls = {} + + async def fake_for_window(self, window, tool): + calls["tool"] = (window, tool) + return "@9" + + monkeypatch.setattr( + "hqt.sessions.service.SessionService.open_tool_window_for_window", + fake_for_window, + ) + + result = CliRunner().invoke(cli.main, ["tool", "lazygit", "hqt-5"]) + + assert result.exit_code == 0, result.output + assert calls["tool"] == ("hqt-5", "lazygit") + + +def test_tool_cmd_clone_dispatches_to_clone(monkeypatch, tmp_path): + monkeypatch.setattr( + "hqt.config.get_settings", lambda: Settings(db_path=tmp_path / "t.db") + ) + calls = {} + + async def fake_clone(self, window): + calls["clone"] = window + return "RESULT" + + monkeypatch.setattr( + "hqt.sessions.service.SessionService.clone_session_for_window", fake_clone + ) + + result = CliRunner().invoke(cli.main, ["tool", "clone", "hqt-5"]) + + assert result.exit_code == 0, result.output + assert calls["clone"] == "hqt-5" + + +def test_tool_cmd_reports_service_error(monkeypatch, tmp_path): + from hqt.errors import ServiceError + + monkeypatch.setattr( + "hqt.config.get_settings", lambda: Settings(db_path=tmp_path / "t.db") + ) + + async def boom(self, window, tool): + raise ServiceError("'hqt-5' is not an hqt session window") + + monkeypatch.setattr( + "hqt.sessions.service.SessionService.open_tool_window_for_window", boom + ) + + result = CliRunner().invoke(cli.main, ["tool", "nvim", "hqt-5"]) + + assert result.exit_code == 1 + assert "not an hqt session window" in result.output +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `uv run pytest tests/test_cli.py -k tool_cmd -v` +Expected: FAIL — `tool` is not a command (`Error: No such command 'tool'`), so exit_code != 0. + +- [ ] **Step 3: Write the implementation** + +In `src/hqt/cli.py`, add a module-level helper and the `tool` command after `list_cmd`: + +```python +def _build_session_service(): + """Wire a SessionService the same way HqtApp.on_mount does (for CLI use).""" + from hqt.config import get_settings + from hqt.db.engine import ensure_db, get_engine, get_session_factory + from hqt.harnesses.registry import discover_harnesses + from hqt.sessions.service import SessionService + from hqt.tmux.manager import TmuxManager + from hqt.tmux.runner import TmuxRunner + + settings = get_settings() + ensure_db(settings) + factory = get_session_factory(get_engine(settings)) + runner = TmuxRunner(settings.tmux_path, settings.tui_session_name) + return SessionService(factory, TmuxManager(runner), discover_harnesses()) + + +@main.command(name="tool") +@click.argument("tool") +@click.argument("window") +def tool_cmd(tool, window): + """Run TOOL for the session in tmux WINDOW. + + TOOL is nvim/lazygit/shell (opens a styled tool window) or "clone" (a fresh + harness with the same project + model). WINDOW is the tmux window name (the + hqt- key, e.g. from #{window_name}). + """ + import asyncio + + from hqt.errors import ServiceError + + svc = _build_session_service() + try: + if tool == "clone": + asyncio.run(svc.clone_session_for_window(window)) + else: + asyncio.run(svc.open_tool_window_for_window(window, tool)) + except ServiceError as err: + click.echo(str(err), err=True) + raise SystemExit(1) +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `uv run pytest tests/test_cli.py -k tool_cmd -v` +Expected: PASS (3 passed). + +Note: `CliRunner` mixes stderr into `result.output`, so the `click.echo(..., err=True)` message is asserted via `result.output`. + +- [ ] **Step 5: Commit** + +```bash +git add src/hqt/cli.py tests/test_cli.py +git commit -m "Add hqt tool CLI subcommand (tool + clone dispatch)" \ + -m "Co-Authored-By: Claude Opus 4.8 " +``` + +--- + +## Task 7: CLI `hqt palette` subcommand + +**Files:** +- Modify: `src/hqt/cli.py` (add palette helpers + `palette` subcommand) +- Test: `tests/test_cli.py` + +Background: `hqt palette ` is what the `M-p` binding invokes (via `run-shell`, which has already expanded `#{window_name}` to a concrete name). It pre-checks the window: a non-session window gets a one-line tmux message; a session window gets the fzf `display-popup`, whose selection runs `hqt tool `. `display-popup` does NOT expand formats, so the window name is baked in as a `shlex.quote`d literal. Both branches call `tmux` via `subprocess.run`, which the test monkeypatches. + +- [ ] **Step 1: Write the failing tests** + +Append to `tests/test_cli.py`: + +```python +def test_palette_cmd_shows_popup_for_session_window(monkeypatch, tmp_path): + monkeypatch.setattr( + "hqt.config.get_settings", lambda: Settings(db_path=tmp_path / "t.db") + ) + monkeypatch.setattr( + "hqt.sessions.service.SessionService.session_id_for_window", + lambda self, window: 5, + ) + runs = [] + monkeypatch.setattr(cli.subprocess, "run", lambda argv, **kw: runs.append(argv)) + + result = CliRunner().invoke(cli.main, ["palette", "hqt-5"]) + + assert result.exit_code == 0, result.output + assert len(runs) == 1 + argv = runs[0] + assert "display-popup" in argv + popup_cmd = argv[-1] + assert "fzf" in popup_cmd + assert "nvim" in popup_cmd and "clone" in popup_cmd + # the window is baked into the command for `hqt tool {} ` + assert "hqt-5" in popup_cmd + + +def test_palette_cmd_hints_for_non_session_window(monkeypatch, tmp_path): + monkeypatch.setattr( + "hqt.config.get_settings", lambda: Settings(db_path=tmp_path / "t.db") + ) + monkeypatch.setattr( + "hqt.sessions.service.SessionService.session_id_for_window", + lambda self, window: None, + ) + runs = [] + monkeypatch.setattr(cli.subprocess, "run", lambda argv, **kw: runs.append(argv)) + + result = CliRunner().invoke(cli.main, ["palette", "nvim"]) + + assert result.exit_code == 0, result.output + assert len(runs) == 1 + argv = runs[0] + assert "display-message" in argv + assert "display-popup" not in argv +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `uv run pytest tests/test_cli.py -k palette_cmd -v` +Expected: FAIL — `palette` is not a command, OR `AttributeError: module 'hqt.cli' has no attribute 'subprocess'` if `subprocess` is not imported at module level yet (it is — `cli.py` already `import subprocess`). + +- [ ] **Step 3: Write the implementation** + +In `src/hqt/cli.py`, add the palette helpers and command after `tool_cmd` (`subprocess` is already imported at the top of the module): + +```python +PALETTE_ENTRIES = ["nvim", "lazygit", "shell", "clone"] + +# fzf colors matching the Catppuccin Frappé status bar / the Alt+o switcher. +_PALETTE_FZF_COLORS = ( + "bg:#292c3c,bg+:#414559,fg:#c6d0f5,fg+:#c6d0f5,hl:#ef9f76,hl+:#ef9f76," + "pointer:#ef9f76,prompt:#8caaee,info:#838ba7,border:#838ba7" +) + + +def _palette_popup_command(window: str) -> str: + """Shell pipeline for the fzf popup; `window` is baked in as a literal. + + display-popup does NOT format-expand its command, so the window name must be + concrete here (run-shell already expanded #{window_name} before `hqt palette` + ran). The selected entry runs `hqt tool `. + """ + import shlex + + entries = "\\n".join(PALETTE_ENTRIES) + "\\n" + return ( + f"printf '{entries}' | " + f"fzf --reverse --no-info --prompt='tool ' --pointer='▌' " + f"--color='{_PALETTE_FZF_COLORS}' | " + f"xargs -r -I{{}} hqt tool {{}} {shlex.quote(window)}" + ) + + +@main.command(name="palette") +@click.argument("window") +def palette_cmd(window): + """Pop an fzf tool palette for the session in tmux WINDOW (bound to M-p).""" + from hqt.config import get_settings + + settings = get_settings() + svc = _build_session_service() + if svc.session_id_for_window(window) is None: + subprocess.run( + [ + settings.tmux_path, + "display-message", + "hqt: open the tool palette from a harness window", + ] + ) + return + subprocess.run( + [ + settings.tmux_path, + "display-popup", + "-E", + "-w", + "40%", + "-h", + "30%", + "-T", + " open tool ", + "-S", + "fg=#838ba7", + _palette_popup_command(window), + ] + ) +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `uv run pytest tests/test_cli.py -k palette_cmd -v` +Expected: PASS (2 passed). + +- [ ] **Step 5: Commit** + +```bash +git add src/hqt/cli.py tests/test_cli.py +git commit -m "Add hqt palette CLI subcommand (fzf tool launcher)" \ + -m "Co-Authored-By: Claude Opus 4.8 " +``` + +--- + +## Task 8: tmux keybinding (`~/.tmux.conf`) + +**Files:** +- Modify: `~/.tmux.conf` (user-owned; append one binding) + +This file is the user's own keybindings file (global tmux bindings are inherently server-wide and live here deliberately). No automated test — verify by re-sourcing. + +- [ ] **Step 1: Append the binding** + +Add to the end of `~/.tmux.conf` (near the existing `Alt+o` switcher): + +```tmux +# Tool palette: Alt+p pops an fzf launcher for the CURRENT hqt session's project +# (works inside a harness). Pick nvim / lazygit / shell to open a styled window at +# the next index, or "clone" for a fresh harness with the same project+model. +# run-shell expands #{window_name} (the hqt- key) and hands it to `hqt palette`, +# which builds the popup — display-popup does NOT expand formats, so the name is +# resolved here. From a non-session window it shows a brief hint. -b keeps the tmux +# server responsive during hqt's ~0.3-0.6s startup. +bind -n M-p run-shell -b "hqt palette '#{window_name}'" +``` + +- [ ] **Step 2: Re-source and verify the binding registered** + +Run: +```bash +tmux source-file ~/.tmux.conf && echo "sourced OK" +tmux list-keys -T root | grep -E "M-p\b" +``` +Expected: `sourced OK`, then a line showing `run-shell -b "hqt palette ..."` bound to `M-p`. + +- [ ] **Step 3: Manual smoke test** + +Confirm `hqt` is on `PATH` (`command -v hqt`) and `fzf` is installed +(`command -v fzf`). From inside a harness pane (an `hqt-` window) press +`M-p`: +- the fzf popup lists nvim / lazygit / shell / clone; +- pick `lazygit` → a styled lazygit window appears at the next index and closes on + quit; repeat for `nvim` and `shell`; +- pick `clone` → a fresh `hqt-` harness window appears (same project + model) + and shows up in the TUI session list within ~3s. + +Then press `M-p` from the TUI home window (or a tool window) → a brief +"open the tool palette from a harness window" message, no menu. (No commit — +`~/.tmux.conf` is outside the repo.) + +--- + +## Final verification + +- [ ] **Run the whole suite** + +Run: `uv run pytest -q` +Expected: all green (existing tests unaffected; new tests pass). + +- [ ] **Lint/type check (match the project's quality gates)** + +Run: `uv run ruff check . && uv run ty check` (or the project's configured gates). +Expected: clean. Fix any issues, then amend/commit. + +--- + +## Self-Review (completed during authoring) + +- **Spec coverage:** registry (Task 1); `new_aux_window` styling / no-remain-on-exit / window_id targeting / append-right (Task 2); manager delegate (Task 3); `session_id_for_window` + `open_tool_window` + `_for_window` resolution + which-check (Task 4); `clone_session_for_window` reuse of project/harness/model + no-op guard (Task 5); `hqt tool` clone-vs-tool dispatch + error mapping (Task 6); `hqt palette` popup-vs-hint (Task 7); `M-p` binding via run-shell bridge (Task 8). Every spec section maps to a task. +- **Type consistency:** `new_aux_window(name, cwd, command, label)` / `open_aux_window(name, cwd, command, label)` signatures match across runner/manager/service/tests; the service calls them with `name=spec.label`. `session_id_for_window` is sync and reused by `open_tool_window_for_window`. `clone_session_for_window` returns `CreateSessionResult` (the same type `create_session` returns). `ServiceError` imported from `hqt.errors` everywhere. The CLI `tool`/`palette` commands and `_build_session_service` use the verified wiring. +- **Verified tmux facts (tmux 3.6b):** `run-shell` expands `#{window_name}`; `display-popup` and its `-e` value do NOT; `display-message -p` inside a popup is client-ambiguous — hence the `run-shell → hqt palette → display-popup (literal window)` bridge. +- **No placeholders:** every code/test step contains complete code; every run step has an exact command and expected result. diff --git a/docs/superpowers/specs/2026-06-10-tool-windows-design.md b/docs/superpowers/specs/2026-06-10-tool-windows-design.md new file mode 100644 index 0000000..d92f90f --- /dev/null +++ b/docs/superpowers/specs/2026-06-10-tool-windows-design.md @@ -0,0 +1,269 @@ +# Tool palette: nvim / lazygit / shell / clone per session + +**Date:** 2026-06-10 +**Status:** Approved (revised — adds the Alt+p palette and the clone action) + +## Goal + +From anywhere inside a harness pane, press **Alt+p** to get a small fuzzy +launcher for the current session's project: + +- **nvim / lazygit / shell** — open the tool in a **new tmux window appended at + the next free index**, styled exactly like hqt's own windows, in the session's + project directory. The window closes when you quit the tool. +- **clone** — open a **fresh harness session** (a real `hqt-` window) with the + **same project, harness, and model** as the current session, but a brand-new + conversation. + +One key (Alt+p), one fuzzy list, so there are no per-tool shortcuts to memorize. +The palette is a tmux binding, so it works over the TUI and inside any harness. + +## Background + +A session is a tmux window named `tmux_session_name` (`hqt-{id}`), created in its +project's directory by `TmuxRunner.new_window()`, which appends at +`_next_window_index()` (= `max(window indices) + 1`) and applies the Frappé +per-window theme. A session row stores `project_id`, `harness_id`, and `model` +(`db/models.py`), so cloning is just `create_session` with those three values. + +Three facts make this design cheap and safe: + +- hqt tracks windows **by name, keyed to DB sessions**, and **never prunes + unknown windows**. `sync_window_labels()` (the 3s poll) only labels rows it + knows. A window that is not a DB session is never killed and never relabeled. +- The status-bar cell renders `#I:#{?@hqt_label,#{@hqt_label},#W}#F`. A window + with no `@hqt_label` falls back to its raw name; setting `@hqt_label` gives a + tool window a clean label and makes it show nicely in the `Alt+o` switcher too. +- `new_window()` hardcodes `remain-on-exit on` (so a harness that dies instantly + leaves a visible pane). Tool windows want the **opposite** — close when you + quit — which is tmux's default. Hence a separate spawn path + (`new_aux_window`) rather than a flag on `new_window()`. + +So a **tool window** is purely a tmux window: hqt spawns and styles it, then +forgets it. A **clone** is the opposite — a fully tracked session created through +the existing `create_session` path, so it appears in the session list on the next +poll with no special handling. + +## Architecture: how Alt+p reaches hqt with the right window + +Two tmux facts (verified empirically on tmux 3.6b) shape the bridge: + +- **`run-shell` format-expands its command.** `run-shell "echo #{window_name}"` + runs `echo hqt-5`. +- **`display-popup` does NOT expand its command (or its `-e` value).** The fzf + popup needs `display-popup` for an interactive terminal, but it receives the + literal string `#{window_name}` — useless. And `display-message -p + '#{window_name}'` *inside* the popup resolves against the "current" client, + which is ambiguous when more than one client is attached (it returned the wrong + window in testing). + +So the binding bridges through `run-shell` (which expands the name) into a small +CLI subcommand that bakes the resolved name into the popup as a literal: + +```tmux +bind -n M-p run-shell -b "hqt palette '#{window_name}'" +``` + +`run-shell` expands `#{window_name}` → `hqt palette 'hqt-5'`. `hqt palette` then +builds and runs `display-popup -E " | xargs -I{} hqt tool {} hqt-5"`, where +`hqt-5` is a concrete literal — no further tmux expansion required, and no +multi-client ambiguity. The selected entry runs `hqt tool hqt-5`, which +funnels into the same service code the rest of hqt uses. No IPC, no daemon. + +`-b` keeps the tmux server responsive during hqt's ~0.3–0.6s startup. + +**TUI note.** From the TUI home window, `#{window_name}` is the TUI window, not a +session — the palette can't know which session is *highlighted* there. So the +palette is, by design, for harness/session windows; from the TUI you attach +(Enter) first, then Alt+p. From a non-session window the palette shows a brief +hint instead of an unusable menu (see `hqt palette` below). This is the accepted +trade-off of a single tmux-level key over a TUI-specific palette. + +## Components + +### 1. Tool registry — `src/hqt/tools.py` (new) + +Maps a tool name to its spawn spec: + +```python +@dataclass(frozen=True) +class Tool: + label: str # base label for @hqt_label, e.g. "nvim" + command: list[str] # [] => tmux default shell (the plain shell) + +TOOLS: dict[str, Tool] = { + "nvim": Tool(label="nvim", command=["nvim"]), + "lazygit": Tool(label="lazygit", command=["lazygit"]), + "shell": Tool(label="shell", command=[]), +} +``` + +`clone` is **not** in this registry — it creates a session, not an aux window — +and is handled as a distinct path. The per-spawn status label is +`f"{tool.label} · {project_name}"` so the always-fresh windows stay +distinguishable. + +### 2. `TmuxRunner.new_aux_window(name, cwd, command, label) -> str | None` (new) + +The aux-window workhorse. Everything after creation is targeted by **`window_id`** +(e.g. `@7`), not name — so duplicate tool-window names are never ambiguous. + +1. `idx = await self._next_window_index()` (appends at the right). +2. `new-window -t : -n -c -P -F '#{window_id}'`, + appending the joined `command` when non-empty (empty → default shell). Capture + the returned `window_id`. **No `remain-on-exit`.** +3. One atomic `set-option` call (`;`-separated argv) on the `window_id`: + `automatic-rename off`, `@hqt_label