felixm/hqt
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
Files
5b4e800883b5fea9e7a0c395efdf49fdb10ae978
hqt/docs/superpowers/plans/2026-06-10-p2-fixes.md
T
felixm 682c6a3f74 docs: implementation plan for P2 fixes 
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-06-10 20:01:06 -04:00

823 lines
32 KiB
Markdown
Raw Blame History

# 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 <noreply@anthropic.com>
```
- **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 <noreply@anthropic.com>"
```
---
## 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 <noreply@anthropic.com>"
```
---
## 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 <noreply@anthropic.com>"
```
---
## 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 <noreply@anthropic.com>"
```
---
## 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 <noreply@anthropic.com>"
```
---
## 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`.
Reference in New Issue View Git Blame Copy Permalink