hqt/docs/superpowers/specs/2026-06-10-p2-fixes-design.md

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 <id> 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:

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.

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:

import shutil
import pytest

pytestmark = [
    pytest.mark.tmux,
    pytest.mark.skipif(shutil.which("tmux") is None, reason="tmux not installed"),
]

Isolation fixture.

@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:

[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:

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.