diff --git a/docs/superpowers/plans/2026-06-09-project-edit-frappe-theme.md b/docs/superpowers/plans/2026-06-09-project-edit-frappe-theme.md deleted file mode 100644 index 9a65f0d..0000000 --- a/docs/superpowers/plans/2026-06-09-project-edit-frappe-theme.md +++ /dev/null @@ -1,808 +0,0 @@ -# Project Editing + Frappé Theme Completion 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:** Let users edit a project's name and path from the TUI, and make the Catppuccin Frappé theme actually look like Frappé (full theme definition, styled dialogs/headers, colored status symbols). - -**Architecture:** `ProjectService` gains an `update` method (unique-path violations surface as `ValueError`). The existing `AddProjectScreen` is generalized into `ProjectFormScreen` with optional pre-filled values, used by both the `a` (add) and new `e` (edit) bindings. The half-specified `FRAPPE_THEME` is replaced with a full definition mirroring Textual's built-in `catppuccin-mocha` structure, plus a `styles.tcss` pass and Rich-markup status colors. - -**Tech Stack:** Python 3.12, Textual 8.x, SQLAlchemy 2.x, pytest + pytest-asyncio, `uv` for everything (`uv run pytest ...`). - -**Spec:** `docs/superpowers/specs/2026-06-09-project-edit-frappe-theme-design.md` - -**Known bug fixed in Task 7:** session labels currently pass `[claude]` to `Label`, which parses it as a Rich markup tag and silently swallows the harness name. Verified by experiment: `Label('x [claude] y')` renders as `'x y'`. The fix escapes the brackets. - ---- - -### Task 1: `ProjectService.update` - -**Files:** -- Modify: `src/hqt/projects/service.py` -- Test: `tests/test_services.py` - -- [ ] **Step 1: Write the failing tests** - -Add to the `TestProjectService` class in `tests/test_services.py`: - -```python - def test_update_name_and_path(self, db): - svc = ProjectService(db) - p = svc.create("old", "/old/path") - updated = svc.update(p.id, "new", "/new/path") - assert updated.name == "new" - assert updated.path == "/new/path" - assert svc.get(p.id).path == "/new/path" - - def test_update_duplicate_path_raises(self, db): - svc = ProjectService(db) - svc.create("a", "/a") - p2 = svc.create("b", "/b") - with pytest.raises(ValueError, match="already uses path"): - svc.update(p2.id, "b", "/a") - # DB session must remain usable after rollback, values unchanged - assert svc.get(p2.id).path == "/b" - - def test_update_unknown_id_raises(self, db): - svc = ProjectService(db) - with pytest.raises(ValueError, match="not found"): - svc.update(9999, "x", "/x") -``` - -- [ ] **Step 2: Run tests to verify they fail** - -Run: `uv run pytest tests/test_services.py -v -k update` -Expected: 3 FAILED with `AttributeError: 'ProjectService' object has no attribute 'update'` - -- [ ] **Step 3: Implement `update`** - -In `src/hqt/projects/service.py`, add the import at the top: - -```python -from sqlalchemy.exc import IntegrityError -``` - -Add this method to `ProjectService` (after `get`, before `archive`): - -```python - def update(self, project_id: int, name: str, path: str) -> Project: - project = self.get(project_id) - if project is None: - raise ValueError(f"Project {project_id} not found") - project.name = name - project.path = path - try: - self.db.commit() - except IntegrityError as err: - self.db.rollback() - raise ValueError(f"Another project already uses path {path}") from err - self.db.refresh(project) - return project -``` - -- [ ] **Step 4: Run tests to verify they pass** - -Run: `uv run pytest tests/test_services.py -v` -Expected: all PASS (including the pre-existing tests) - -- [ ] **Step 5: Commit** - -```bash -git add src/hqt/projects/service.py tests/test_services.py -git commit -m "feat: ProjectService.update with duplicate-path ValueError" -``` - ---- - -### Task 2: `ProjectFormScreen` (rename `AddProjectScreen`, add pre-fill) - -**Files:** -- Rename: `src/hqt/tui/screens/add_project.py` → `src/hqt/tui/screens/project_form.py` -- Modify: `src/hqt/tui/app.py` (import at line 17, `action_add_project` at lines 107–114) -- Test: `tests/test_tui.py` - -- [ ] **Step 1: Write the failing tests** - -Add to `tests/test_tui.py`: - -```python -# --------------------------------------------------------------------------- -# ProjectFormScreen: shared add/edit form -# --------------------------------------------------------------------------- - - -@pytest.mark.asyncio -async def test_project_form_prefills_initial_values(): - app = HqtApp() - async with app.run_test(size=(120, 40)) as pilot: - from hqt.tui.screens.project_form import ProjectFormScreen - from textual.widgets import Input - - app.push_screen( - ProjectFormScreen( - title="Edit Project", - initial_name="myproj", - initial_path="/tmp/myproj", - ) - ) - await pilot.pause() - assert app.screen.query_one("#name-input", Input).value == "myproj" - assert app.screen.query_one("#path-input", Input).value == "/tmp/myproj" - - -@pytest.mark.asyncio -async def test_project_form_add_mode_defaults_name_to_basename(): - app = HqtApp() - async with app.run_test(size=(120, 40)) as pilot: - from hqt.tui.screens.project_form import ProjectFormScreen - from textual.widgets import Button, Input - - results = [] - app.push_screen(ProjectFormScreen(), results.append) - await pilot.pause() - app.screen.query_one("#path-input", Input).value = "/tmp/somerepo" - app.screen.query_one("#ok-btn", Button).press() - await pilot.pause() - assert results == [("somerepo", "/tmp/somerepo")] -``` - -- [ ] **Step 2: Run tests to verify they fail** - -Run: `uv run pytest tests/test_tui.py -v -k project_form` -Expected: 2 FAILED with `ModuleNotFoundError: No module named 'hqt.tui.screens.project_form'` - -- [ ] **Step 3: Rename the file and generalize the class** - -```bash -git mv src/hqt/tui/screens/add_project.py src/hqt/tui/screens/project_form.py -``` - -Replace the entire contents of `src/hqt/tui/screens/project_form.py` with: - -```python -from pathlib import Path - -from textual.app import ComposeResult -from textual.containers import Vertical -from textual.screen import ModalScreen -from textual.widgets import Button, Input, Label - - -class ProjectFormScreen(ModalScreen[tuple[str, str] | None]): - def __init__( - self, - title: str = "Add Project", - initial_name: str = "", - initial_path: str = "", - ) -> None: - self._form_title = title - self._initial_name = initial_name - self._initial_path = initial_path - super().__init__() - - def compose(self) -> ComposeResult: - with Vertical(id="project-form-dialog"): - yield Label(self._form_title) - yield Label("Path:") - yield Input( - value=self._initial_path, - placeholder="/path/to/project", - id="path-input", - ) - yield Label("Name (optional):") - yield Input( - value=self._initial_name, - placeholder="project name", - id="name-input", - ) - yield Button("OK", variant="primary", id="ok-btn") - yield Button("Cancel", id="cancel-btn") - - def on_button_pressed(self, event: Button.Pressed) -> None: - if event.button.id == "ok-btn": - path = self.query_one("#path-input", Input).value.strip() - name = self.query_one("#name-input", Input).value.strip() - if path: - if not name: - name = Path(path).name - self.dismiss((name, path)) - else: - self.dismiss(None) - else: - self.dismiss(None) -``` - -- [ ] **Step 4: Update `app.py` to use the renamed screen** - -In `src/hqt/tui/app.py`, replace the import: - -```python -from hqt.tui.screens.add_project import AddProjectScreen -``` - -with: - -```python -from hqt.tui.screens.project_form import ProjectFormScreen -``` - -and in `action_add_project`, replace: - -```python - self.push_screen(AddProjectScreen(), on_dismiss) -``` - -with: - -```python - self.push_screen(ProjectFormScreen(), on_dismiss) -``` - -- [ ] **Step 5: Run the full TUI test file** - -Run: `uv run pytest tests/test_tui.py -v` -Expected: all PASS - -- [ ] **Step 6: Commit** - -```bash -git add -A src/hqt/tui tests/test_tui.py -git commit -m "refactor: generalize AddProjectScreen into ProjectFormScreen with pre-fill" -``` - ---- - -### Task 3: `ProjectList.get_selected_project_id` - -**Files:** -- Modify: `src/hqt/tui/widgets/project_list.py` -- Test: `tests/test_tui.py` - -- [ ] **Step 1: Write the failing test** - -Add to `tests/test_tui.py`: - -```python -@pytest.mark.asyncio -async def test_project_list_get_selected_project_id(): - app = HqtApp() - async with app.run_test(size=(120, 40)) as pilot: - from hqt.tui.widgets.project_list import ProjectList - - p = app._project_service.create("selproj", "/tmp/selproj") - app._load_projects() - await pilot.pause() - - pl = app.query_one(ProjectList) - lv = pl.query_one("#project-list") - lv.focus() - await pilot.press("down") - await pilot.pause() - - assert pl.get_selected_project_id() == p.id -``` - -- [ ] **Step 2: Run test to verify it fails** - -Run: `uv run pytest tests/test_tui.py::test_project_list_get_selected_project_id -v` -Expected: FAIL with `AttributeError: 'ProjectList' object has no attribute 'get_selected_project_id'` - -- [ ] **Step 3: Implement the method** - -Add to `ProjectList` in `src/hqt/tui/widgets/project_list.py` (same pattern as `SessionList.get_selected_session_id`): - -```python - def get_selected_project_id(self) -> int | None: - lv = self.query_one("#project-list", ListView) - if lv.highlighted_child and hasattr(lv.highlighted_child, "data"): - return lv.highlighted_child.data - return None -``` - -- [ ] **Step 4: Run test to verify it passes** - -Run: `uv run pytest tests/test_tui.py::test_project_list_get_selected_project_id -v` -Expected: PASS - -- [ ] **Step 5: Commit** - -```bash -git add src/hqt/tui/widgets/project_list.py tests/test_tui.py -git commit -m "feat: ProjectList.get_selected_project_id" -``` - ---- - -### Task 4: `e` binding + `action_edit_project` - -**Files:** -- Modify: `src/hqt/tui/app.py` (BINDINGS at lines 42–51, new action after `action_add_project`) -- Test: `tests/test_tui.py` - -- [ ] **Step 1: Write the failing tests** - -Add to `tests/test_tui.py`: - -```python -# --------------------------------------------------------------------------- -# Edit project: binding + action -# --------------------------------------------------------------------------- - - -@pytest.mark.asyncio -async def test_edit_project_binding_exists(): - bindings = {b.key: b for b in HqtApp.BINDINGS} - assert "e" in bindings - assert bindings["e"].action == "edit_project" - - -@pytest.mark.asyncio -async def test_edit_project_no_selection_warns(): - app = HqtApp() - async with app.run_test(size=(120, 40)) as pilot: - from unittest.mock import patch - - notify_calls = [] - with patch.object( - app, "notify", side_effect=lambda msg, **kw: notify_calls.append((msg, kw)) - ): - await app.run_action("edit_project") - await pilot.pause() - - assert any( - kw.get("severity") == "warning" for _, kw in notify_calls - ), f"Expected warning notification, got: {notify_calls}" - - -@pytest.mark.asyncio -async def test_edit_project_opens_prefilled_form_and_updates(): - app = HqtApp() - async with app.run_test(size=(120, 40)) as pilot: - from textual.widgets import Input - from hqt.tui.screens.project_form import ProjectFormScreen - from hqt.tui.widgets.project_list import ProjectList - - p = app._project_service.create("oldname", "/tmp/oldpath") - app._load_projects() - await pilot.pause() - - pl = app.query_one(ProjectList) - pl.query_one("#project-list").focus() - await pilot.press("down") - await pilot.pause() - assert pl.get_selected_project_id() == p.id - - await app.run_action("edit_project") - await pilot.pause() - - assert isinstance(app.screen, ProjectFormScreen) - assert app.screen.query_one("#name-input", Input).value == "oldname" - assert app.screen.query_one("#path-input", Input).value == "/tmp/oldpath" - - app.screen.dismiss(("newname", "/tmp/newpath")) - await pilot.pause() - - refreshed = app._project_service.get(p.id) - assert refreshed.name == "newname" - assert refreshed.path == "/tmp/newpath" - - -@pytest.mark.asyncio -async def test_edit_project_duplicate_path_notifies_error(): - app = HqtApp() - async with app.run_test(size=(120, 40)) as pilot: - from unittest.mock import patch - from hqt.tui.widgets.project_list import ProjectList - - app._project_service.create("first", "/tmp/first") - p2 = app._project_service.create("second", "/tmp/second") - app._load_projects() - await pilot.pause() - - pl = app.query_one(ProjectList) - pl.query_one("#project-list").focus() - # Two items: press down twice to land on the second project - await pilot.press("down") - await pilot.press("down") - await pilot.pause() - assert pl.get_selected_project_id() == p2.id - - notify_calls = [] - with patch.object( - app, "notify", side_effect=lambda msg, **kw: notify_calls.append((msg, kw)) - ): - await app.run_action("edit_project") - await pilot.pause() - app.screen.dismiss(("second", "/tmp/first")) - await pilot.pause() - - assert any( - kw.get("severity") == "error" and "already uses path" in msg - for msg, kw in notify_calls - ), f"Expected duplicate-path error notification, got: {notify_calls}" - # DB unchanged - assert app._project_service.get(p2.id).path == "/tmp/second" -``` - -- [ ] **Step 2: Run tests to verify they fail** - -Run: `uv run pytest tests/test_tui.py -v -k edit_project` -Expected: 4 FAILED (`"e" not in bindings`; the action-based tests fail because the action doesn't exist) - -- [ ] **Step 3: Implement binding and action** - -In `src/hqt/tui/app.py`, add to `BINDINGS` (after the `"a"` binding): - -```python - Binding("e", "edit_project", "Edit Project"), -``` - -Add this method after `action_add_project`: - -```python - def action_edit_project(self) -> None: - project_id = self.query_one(ProjectList).get_selected_project_id() - if project_id is None: - self.notify("Select a project first", severity="warning") - return - project = self._project_service.get(project_id) - if project is None: - self.notify("Project not found", severity="error") - return - - def on_dismiss(result: tuple[str, str] | None) -> None: - if result: - name, path = result - try: - self._project_service.update(project_id, name, path) - except ValueError as err: - self.notify(str(err), severity="error") - return - self._load_projects() - - self.push_screen( - ProjectFormScreen( - title="Edit Project", - initial_name=project.name, - initial_path=project.path, - ), - on_dismiss, - ) -``` - -- [ ] **Step 4: Run tests to verify they pass** - -Run: `uv run pytest tests/test_tui.py -v` -Expected: all PASS - -- [ ] **Step 5: Commit** - -```bash -git add src/hqt/tui/app.py tests/test_tui.py -git commit -m "feat: edit project name/path via 'e' binding" -``` - ---- - -### Task 5: Complete the Frappé theme definition - -**Files:** -- Modify: `src/hqt/tui/app.py:23-36` (`FRAPPE_THEME`) -- Test: `tests/test_tui.py` - -- [ ] **Step 1: Write the failing test** - -Add to `tests/test_tui.py`: - -```python -# --------------------------------------------------------------------------- -# Catppuccin Frappé theme: full definition -# --------------------------------------------------------------------------- - - -@pytest.mark.asyncio -async def test_frappe_theme_fully_applied(): - app = HqtApp() - async with app.run_test() as pilot: - assert app.current_theme.name == "catppuccin-frappe" - variables = app.get_css_variables() - # Spot-check: explicit values, not Textual auto-derivations - assert variables["primary"] == "#8caaee" # Blue - assert variables["background"] == "#292c3c" # Mantle - assert variables["surface"] == "#414559" # Surface0 - assert variables["border"] == "#babbf1" # Lavender - assert variables["footer-background"] == "#51576d" # Surface1 - assert variables["input-cursor-background"] == "#f2d5cf" # Rosewater -``` - -- [ ] **Step 2: Run test to verify it fails** - -Run: `uv run pytest tests/test_tui.py::test_frappe_theme_fully_applied -v` -Expected: FAIL on `variables["background"] == "#292c3c"` (currently `#303446`) - -- [ ] **Step 3: Replace `FRAPPE_THEME`** - -In `src/hqt/tui/app.py`, replace the entire `FRAPPE_THEME = Theme(...)` block with: - -```python -# Catppuccin Frappé, mirroring the structure of Textual's built-in -# catppuccin-mocha theme so no colors are auto-derived off-palette. -FRAPPE_THEME = Theme( - name="catppuccin-frappe", - primary="#8caaee", # Blue - secondary="#ca9ee6", # Mauve - accent="#ef9f76", # Peach - success="#a6d189", # Green - warning="#e5c890", # Yellow - error="#e78284", # Red - foreground="#c6d0f5", # Text - background="#292c3c", # Mantle - surface="#414559", # Surface0 - panel="#51576d", # Surface1 - dark=True, - variables={ - "input-cursor-foreground": "#232634", # Crust - "input-cursor-background": "#f2d5cf", # Rosewater - "input-selection-background": "#949cbb 30%", # Overlay2 30% - "border": "#babbf1", # Lavender - "border-blurred": "#626880", # Surface2 - "footer-background": "#51576d", # Surface1 - "block-cursor-foreground": "#303446", # Base - "block-cursor-text-style": "none", - "button-color-foreground": "#292c3c", # Mantle - }, -) -``` - -- [ ] **Step 4: Run tests to verify they pass** - -Run: `uv run pytest tests/test_tui.py -v` -Expected: all PASS - -- [ ] **Step 5: Commit** - -```bash -git add src/hqt/tui/app.py tests/test_tui.py -git commit -m "feat: fully specify Catppuccin Frappé theme variables" -``` - ---- - -### Task 6: Style dialogs and panel headers in `styles.tcss` - -**Files:** -- Modify: `src/hqt/tui/styles.tcss` -- Test: `tests/test_tui.py` - -- [ ] **Step 1: Write the failing test** - -Add to `tests/test_tui.py`: - -```python -@pytest.mark.asyncio -async def test_project_form_dialog_styled(): - app = HqtApp() - async with app.run_test(size=(120, 40)) as pilot: - from hqt.tui.screens.project_form import ProjectFormScreen - - app.push_screen(ProjectFormScreen()) - await pilot.pause() - dialog = app.screen.query_one("#project-form-dialog") - assert dialog.styles.width.value == 60 - assert dialog.styles.border_top[0] == "solid" -``` - -- [ ] **Step 2: Run test to verify it fails** - -Run: `uv run pytest tests/test_tui.py::test_project_form_dialog_styled -v` -Expected: FAIL (width is not 60; no border set) - -- [ ] **Step 3: Extend the stylesheet** - -Replace the contents of `src/hqt/tui/styles.tcss` with: - -```css -ProjectList { - width: 1fr; - min-width: 20; - max-width: 30; - dock: left; - border-right: solid $primary; -} - -SessionList { - width: 3fr; -} - -#project-header, #session-header { - width: 100%; - text-style: bold; - background: $surface; -} - -ProjectFormScreen, NewSessionScreen { - align: center middle; -} - -#project-form-dialog, #new-session-dialog { - width: 60; - height: auto; - padding: 1 2; - background: $surface; - border: solid $border; -} -``` - -- [ ] **Step 4: Run tests to verify they pass** - -Run: `uv run pytest tests/test_tui.py -v` -Expected: all PASS - -- [ ] **Step 5: Commit** - -```bash -git add src/hqt/tui/styles.tcss tests/test_tui.py -git commit -m "feat: style modal dialogs and panel headers with Frappé palette" -``` - ---- - -### Task 7: Colored status symbols + harness-name markup bug fix - -**Files:** -- Modify: `src/hqt/tui/widgets/session_list.py` -- Test: `tests/test_tui.py` - -Context: labels are Rich markup. The current f-string interpolates `[{s.harness.name}]`, which Rich parses as a markup tag and **silently drops** — the harness name is invisible in the running app today. The new formatting helper escapes user-derived text and adds a Frappé color tag around the status symbol. - -- [ ] **Step 1: Write the failing tests** - -Add to `tests/test_tui.py`: - -```python -# --------------------------------------------------------------------------- -# Status symbol colors + harness-name markup escape -# --------------------------------------------------------------------------- - - -def test_format_session_text_colors_and_escapes(): - from hqt.tui.widgets.session_list import format_session_text - - text = format_session_text("mywork", "claude", "working", "◐") - assert text.startswith("[#a6d189]◐[/]") # Green symbol - assert "\\[claude]" in text # escaped, so brackets render - assert "working" in text - - -def test_format_session_text_status_colors(): - from hqt.tui.widgets.session_list import format_session_text - - assert format_session_text("x", "h", "waiting", "◉").startswith("[#e5c890]") # Yellow - assert format_session_text("x", "h", "idle", "●").startswith("[#81c8be]") # Teal - assert format_session_text("x", "h", "active", "●").startswith("[#81c8be]") # Teal - assert format_session_text("x", "h", "dead", "○").startswith("[#737994]") # Overlay0 - - -@pytest.mark.asyncio -async def test_session_list_renders_harness_name_brackets(): - """Regression: '[claude]' must be visible, not swallowed as a markup tag.""" - app = HqtApp() - async with app.run_test(size=(120, 40)) as pilot: - from textual.widgets import Label - from hqt.tui.widgets.session_list import SessionList - - sl = app.query_one(SessionList) - infos = [_make_session_info(1, "mywork", "claude", "working", True)] - await sl.refresh_sessions(infos) - await pilot.pause() - - lv = sl.query_one("#session-list") - texts = [str(child.query_one(Label).render()) for child in lv.children] - assert any("[claude]" in t for t in texts), f"harness name missing: {texts}" -``` - -- [ ] **Step 2: Run tests to verify they fail** - -Run: `uv run pytest tests/test_tui.py -v -k "format_session_text or renders_harness"` -Expected: 2 FAILED with `ImportError: cannot import name 'format_session_text'`, 1 FAILED on the brackets assertion - -- [ ] **Step 3: Implement the formatting helper and use it** - -In `src/hqt/tui/widgets/session_list.py`, add at the top: - -```python -from rich.markup import escape -``` - -Add at module level (above the `SessionList` class, next to where `_STATUS_SYMBOLS` will remain inside the class): - -```python -_STATUS_COLORS: dict[str, str] = { - "working": "#a6d189", # Green - "waiting": "#e5c890", # Yellow - "active": "#81c8be", # Teal - "idle": "#81c8be", # Teal - "dead": "#737994", # Overlay0 -} - - -def format_session_text( - nickname: str, harness_name: str, status_text: str, symbol: str -) -> str: - color = _STATUS_COLORS.get(status_text, "#c6d0f5") # default: Text - label = escape(f"{nickname} [{harness_name}]") - return f"[{color}]{symbol}[/] {label} {status_text}" -``` - -In `SessionList.refresh_sessions`, replace: - -```python - text = f"{symbol} {s.nickname or s.tmux_session_name} [{s.harness.name}] {status_text}" -``` - -with: - -```python - text = format_session_text( - s.nickname or s.tmux_session_name, s.harness.name, status_text, symbol - ) -``` - -- [ ] **Step 4: Run the full TUI test file** - -Run: `uv run pytest tests/test_tui.py -v` -Expected: all PASS — including the pre-existing `test_session_list_label_includes_status`, which reads plain rendered text and is unaffected by markup - -- [ ] **Step 5: Commit** - -```bash -git add src/hqt/tui/widgets/session_list.py tests/test_tui.py -git commit -m "feat: Frappé-colored status symbols; fix harness name swallowed by markup" -``` - ---- - -### Task 8: Full suite + visual verification - -**Files:** -- None modified (verification only) - -- [ ] **Step 1: Run the entire test suite** - -Run: `uv run pytest` -Expected: all tests PASS, no warnings about unknown CSS - -- [ ] **Step 2: Render a headless screenshot and inspect it** - -```bash -uv run python - <<'EOF' -import asyncio, tempfile -from pathlib import Path -import hqt.config as config - -tmp = Path(tempfile.mkdtemp()) -config._settings = config.Settings(db_path=tmp/'hqt.db', config_dir=tmp, skills_dir=tmp/'skills') - -from hqt.tui.app import HqtApp - -async def main(): - app = HqtApp() - async with app.run_test(size=(100, 30)) as pilot: - app._project_service.create("demo-project", "/tmp/demo") - app._load_projects() - await pilot.pause() - Path('/tmp/hqt_themed.svg').write_text(app.export_screenshot()) - -asyncio.run(main()) -EOF -rsvg-convert -o /tmp/hqt_themed.png /tmp/hqt_themed.svg -``` - -Then view `/tmp/hqt_themed.png` (Read tool or image viewer). Check: Mantle background (`#292c3c`), Surface0 panels, Lavender borders, Blue selection highlight, bold panel headers. Repeat with `app.push_screen(ProjectFormScreen())` before the screenshot to verify the dialog styling. - -- [ ] **Step 3: Final commit if any fixes were needed** - -If the screenshot revealed fixes, commit them: - -```bash -git add -A && git commit -m "fix: theme polish from visual verification" -``` diff --git a/docs/superpowers/plans/2026-06-10-p1-fixes.md b/docs/superpowers/plans/2026-06-10-p1-fixes.md deleted file mode 100644 index efcd905..0000000 --- a/docs/superpowers/plans/2026-06-10-p1-fixes.md +++ /dev/null @@ -1,1248 +0,0 @@ -# P1 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 all three P1 findings in TODO.md: (1) replace the single shared long-lived SQLAlchemy session with short-lived per-operation sessions, (2) turn missing/stale DB-row crashes into user-facing error notifications, (3) add a schema-migration mechanism so existing user databases survive schema changes. - -**Architecture:** Services (`ProjectService`, `McpService`, `SessionService`) stop holding a live `Session` and instead hold a `sessionmaker` factory; every public method opens a scoped session (`with self.factory() as db:`). The factory is created with `expire_on_commit=False` and list queries eager-load the `harness` relationship with `selectinload`, so ORM rows returned to the TUI stay readable after their session closes. A new `ServiceError` exception carries user-facing messages for expected failures (missing project/session rows, uninstalled harnesses, duplicate paths); the TUI catches it and calls `self.notify(...)` instead of crashing the worker. Schema versioning uses SQLite's `PRAGMA user_version` with an ordered in-code migration list — no Alembic dependency. - -**Tech Stack:** Python 3.12+, SQLAlchemy 2.0 ORM, Textual, pytest + pytest-asyncio, SQLite. - -**Quality gates (run before EVERY commit, from repo root):** - -```bash -uv run ruff format src tests && uv run ruff check src tests && uv run ty check -``` - -**IMPORTANT — pre-existing dirty file:** `src/hqt/tmux/runner.py` has uncommitted changes unrelated to this plan. Never `git add -A` or `git add .`; always `git add` the specific files listed in each commit step so that file stays out of these commits. - -**Run tests with:** `uv run pytest -v` - ---- - -## File Structure - -| File | Action | Responsibility | -|---|---|---| -| `src/hqt/db/migrations.py` | Create | `PRAGMA user_version` migration runner + ordered migration list | -| `src/hqt/db/engine.py` | Modify | `ensure_db` delegates to `migrate()`; factory gets `expire_on_commit=False` | -| `src/hqt/errors.py` | Create | `ServiceError` exception | -| `src/hqt/projects/service.py` | Modify | Per-operation sessions; `ServiceError` for duplicates/missing rows | -| `src/hqt/mcp/service.py` | Modify | Per-operation sessions | -| `src/hqt/sessions/service.py` | Modify | Per-operation sessions; `ServiceError` for missing project/session/harness; eager-load `harness` | -| `src/hqt/tui/app.py` | Modify | Hold factory instead of session; catch `ServiceError` → `notify` | -| `src/hqt/cli.py` | Modify | `list_cmd` passes factory to `ProjectService` | -| `tests/test_db.py` | Modify | Migration tests + detached-row test | -| `tests/test_services.py` | Modify | Factory fixture; `ServiceError` assertions | -| `tests/test_sessions.py` | Modify | Factory fixture; new error-path tests | -| `tests/test_integration.py` | Modify | Factory wiring | -| `tests/test_tui.py` | Modify | Seed via `app._db_factory()` instead of `app._db_session` | -| `TODO.md` | Modify | Remove the three resolved P1 lines | - ---- - -### Task 1: Schema migration framework (P1 #3) - -**Files:** -- Create: `src/hqt/db/migrations.py` -- Modify: `src/hqt/db/engine.py` -- Test: `tests/test_db.py` - -Versioning model: `PRAGMA user_version` stores the schema version. Version 1 is the baseline (the schema `Base.metadata.create_all` produces today). Pre-existing user DBs have `user_version == 0` and the baseline schema, so version 0 with tables present is treated as version 1. Future schema changes append `(version, fn)` entries to `MIGRATIONS` and bump nothing else — `LATEST_VERSION` derives from the list. - -- [ ] **Step 1: Write the failing tests** - -Append to `tests/test_db.py` (note: it already imports `Path`, `inspect`, `Settings`, `ensure_db`, `get_engine`, `get_session_factory`, and models at top — add the new imports shown): - -```python -import pytest - -from hqt.db import migrations -from hqt.db.migrations import migrate -from hqt.db.models import Base - - -def _user_version(engine) -> int: - with engine.connect() as conn: - return conn.exec_driver_sql("PRAGMA user_version").scalar() or 0 - - -def test_fresh_db_stamped_latest_version(tmp_path): - settings = _tmp_settings(tmp_path) - ensure_db(settings) - assert _user_version(get_engine(settings)) == migrations.LATEST_VERSION - - -def test_unversioned_existing_db_stamped_baseline(tmp_path): - # Simulates a user DB created before versioning existed: tables present, - # user_version still 0. - settings = _tmp_settings(tmp_path) - settings.db_path.parent.mkdir(parents=True, exist_ok=True) - engine = get_engine(settings) - Base.metadata.create_all(engine) - migrate(engine) - assert _user_version(engine) == migrations.BASELINE_VERSION - - -def test_db_from_newer_hqt_raises(tmp_path): - settings = _tmp_settings(tmp_path) - ensure_db(settings) - engine = get_engine(settings) - with engine.begin() as conn: - conn.exec_driver_sql("PRAGMA user_version = 9999") - with pytest.raises(RuntimeError, match="newer"): - migrate(engine) - - -def test_pending_migrations_applied_in_order_once(tmp_path, monkeypatch): - settings = _tmp_settings(tmp_path) - ensure_db(settings) # stamps BASELINE_VERSION - applied: list[int] = [] - monkeypatch.setattr( - migrations, - "MIGRATIONS", - [(2, lambda conn: applied.append(2)), (3, lambda conn: applied.append(3))], - ) - monkeypatch.setattr(migrations, "LATEST_VERSION", 3) - engine = get_engine(settings) - migrate(engine) - assert applied == [2, 3] - assert _user_version(engine) == 3 - migrate(engine) # re-run is a no-op - assert applied == [2, 3] -``` - -- [ ] **Step 2: Run tests to verify they fail** - -Run: `uv run pytest tests/test_db.py -v` -Expected: the four new tests FAIL with `ModuleNotFoundError: No module named 'hqt.db.migrations'`; pre-existing tests still pass. - -- [ ] **Step 3: Create `src/hqt/db/migrations.py`** - -```python -import logging -from collections.abc import Callable - -from sqlalchemy import Connection, Engine, inspect - -from hqt.db.models import Base - -log = logging.getLogger(__name__) - -# Ordered schema migrations. Each entry upgrades the schema from the previous -# version to `version`. Version 1 (BASELINE_VERSION) is the schema produced by -# Base.metadata.create_all, so entries here start at version 2, e.g.: -# (2, lambda conn: conn.exec_driver_sql( -# "ALTER TABLE sessions ADD COLUMN foo TEXT")), -MIGRATIONS: list[tuple[int, Callable[[Connection], None]]] = [] - -BASELINE_VERSION = 1 -LATEST_VERSION = MIGRATIONS[-1][0] if MIGRATIONS else BASELINE_VERSION - - -def migrate(engine: Engine) -> None: - """Create or upgrade the database schema to LATEST_VERSION. - - Fresh databases are created with create_all and stamped directly. - Databases at user_version 0 that already have tables predate versioning - and are treated as the baseline schema. - """ - with engine.begin() as conn: - version = conn.exec_driver_sql("PRAGMA user_version").scalar() or 0 - if version > LATEST_VERSION: - raise RuntimeError( - f"Database schema version {version} is newer than this hqt " - f"supports ({LATEST_VERSION}); upgrade hqt or use a fresh database." - ) - if not inspect(conn).get_table_names(): - Base.metadata.create_all(conn) - conn.exec_driver_sql(f"PRAGMA user_version = {LATEST_VERSION}") - return - if version == 0: - version = BASELINE_VERSION - for target, fn in MIGRATIONS: - if target > version: - log.info("Migrating database schema to version %d", target) - fn(conn) - version = target - conn.exec_driver_sql(f"PRAGMA user_version = {version}") -``` - -- [ ] **Step 4: Point `ensure_db` at the migration runner** - -Replace `ensure_db` in `src/hqt/db/engine.py` (and drop the now-unused `Base` import): - -```python -from sqlalchemy import Engine, create_engine -from sqlalchemy.orm import sessionmaker - -from hqt.config import Settings -from hqt.db.migrations import migrate - - -def get_engine(settings: Settings) -> Engine: - settings.db_path.parent.mkdir(parents=True, exist_ok=True) - return create_engine(f"sqlite:///{settings.db_path}") - - -def get_session_factory(engine: Engine) -> sessionmaker: - return sessionmaker(bind=engine) - - -def ensure_db(settings: Settings) -> None: - settings.db_path.parent.mkdir(parents=True, exist_ok=True) - migrate(get_engine(settings)) -``` - -- [ ] **Step 5: Run tests to verify they pass** - -Run: `uv run pytest tests/test_db.py -v` -Expected: ALL tests PASS (including pre-existing `test_ensure_db_creates_tables`). - -- [ ] **Step 6: Quality gates + commit** - -```bash -uv run ruff format src tests && uv run ruff check src tests && uv run ty check -git add src/hqt/db/migrations.py src/hqt/db/engine.py tests/test_db.py -git commit -m "feat: add user_version-based schema migrations" -``` - ---- - -### Task 2: `ServiceError` + detached-row-safe session factory - -**Files:** -- Create: `src/hqt/errors.py` -- Modify: `src/hqt/db/engine.py:13-14` -- Test: `tests/test_db.py` - -- [ ] **Step 1: Write the failing test** - -Append to `tests/test_db.py`: - -```python -def test_rows_stay_readable_after_session_closes(tmp_path): - # Services use per-operation sessions; rows they return must remain - # readable after the originating session closes (expire_on_commit=False). - settings = _tmp_settings(tmp_path) - ensure_db(settings) - factory = get_session_factory(get_engine(settings)) - with factory() as session: - p = Project(name="x", path="/x") - session.add(p) - session.commit() - assert p.name == "x" # would raise DetachedInstanceError without the flag -``` - -- [ ] **Step 2: Run test to verify it fails** - -Run: `uv run pytest tests/test_db.py::test_rows_stay_readable_after_session_closes -v` -Expected: FAIL with `DetachedInstanceError` (commit expired the instance, refresh after close is impossible). - -- [ ] **Step 3: Set `expire_on_commit=False` in the factory** - -In `src/hqt/db/engine.py`: - -```python -def get_session_factory(engine: Engine) -> sessionmaker: - # expire_on_commit=False: services open a session per operation and return - # ORM rows to the TUI after the session closes; loaded attributes must - # stay readable on those detached instances. - return sessionmaker(bind=engine, expire_on_commit=False) -``` - -- [ ] **Step 4: Create `src/hqt/errors.py`** - -```python -class ServiceError(Exception): - """Expected, user-facing failure in a service operation. - - Raised for recoverable conditions: missing DB rows (project/session - deleted underneath an action), harnesses no longer installed, duplicate - project paths. The TUI catches this and shows a notification instead of - letting the worker crash. - """ -``` - -- [ ] **Step 5: Run tests to verify they pass** - -Run: `uv run pytest tests/test_db.py -v` -Expected: ALL PASS. - -- [ ] **Step 6: Quality gates + commit** - -```bash -uv run ruff format src tests && uv run ruff check src tests && uv run ty check -git add src/hqt/errors.py src/hqt/db/engine.py tests/test_db.py -git commit -m "feat: add ServiceError and detached-safe session factory" -``` - ---- - -### Task 3: `ProjectService` and `McpService` use per-operation sessions - -**Files:** -- Modify: `src/hqt/projects/service.py` -- Modify: `src/hqt/mcp/service.py` -- Test: `tests/test_services.py` - -- [ ] **Step 1: Rewrite the test fixture and update assertions** - -In `tests/test_services.py`, replace the imports and the `db` fixture at the top of the file with: - -```python -import pytest -from sqlalchemy import create_engine -from sqlalchemy.orm import sessionmaker -from sqlalchemy.pool import StaticPool - -from hqt.db.models import Base -from hqt.errors import ServiceError -from hqt.mcp.service import McpService -from hqt.projects.service import ProjectService - - -@pytest.fixture -def factory(): - # StaticPool + check_same_thread=False: every session created by the - # factory shares the single in-memory connection, so per-operation - # sessions all see the same database. - engine = create_engine( - "sqlite://", - connect_args={"check_same_thread": False}, - poolclass=StaticPool, - ) - Base.metadata.create_all(engine) - return sessionmaker(bind=engine, expire_on_commit=False) -``` - -Then mechanically update the test bodies: -- Replace every `(self, db)` test parameter with `(self, factory)` and every `ProjectService(db)` / `McpService(db)` with `ProjectService(factory)` / `McpService(factory)`. -- In `test_update_duplicate_path_raises` and `test_update_unknown_id_raises`, change `pytest.raises(ValueError, ...)` to `pytest.raises(ServiceError, ...)` (match strings stay the same). -- Add one new test to `TestProjectService`: - -```python - def test_create_duplicate_path_raises_service_error(self, factory): - svc = ProjectService(factory) - svc.create("a", "/dup") - with pytest.raises(ServiceError, match="already uses path"): - svc.create("b", "/dup") -``` - -- [ ] **Step 2: Run tests to verify they fail** - -Run: `uv run pytest tests/test_services.py -v` -Expected: FAIL — `ProjectService.__init__` receives a `sessionmaker`, methods break on `self.db.query` (`sessionmaker` has no `query`). - -- [ ] **Step 3: Rewrite `src/hqt/projects/service.py`** - -```python -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): - self.factory = factory - - def create(self, name: str, path: str) -> Project: - 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: - 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() -``` - -(`db.refresh(project)` calls are gone — `expire_on_commit=False` keeps attributes loaded.) - -- [ ] **Step 4: Rewrite `src/hqt/mcp/service.py`** - -```python -import json - -from sqlalchemy.orm import sessionmaker - -from hqt.db.models import McpServer, ProjectMcpServer - - -class McpService: - def __init__(self, factory: sessionmaker): - self.factory = factory - - def create( - self, - name: str, - transport: str, - command: str | None = None, - args: list[str] | None = None, - url: str | None = None, - ) -> McpServer: - with self.factory() as db: - server = McpServer( - name=name, - transport=transport, - command=command, - args_json=json.dumps(args) if args else None, - url=url, - ) - db.add(server) - db.commit() - return server - - def list_all(self) -> list[McpServer]: - with self.factory() as db: - return list(db.query(McpServer).all()) - - def get(self, name: str) -> McpServer | None: - with self.factory() as db: - return db.query(McpServer).filter_by(name=name).first() - - def delete(self, name: str) -> None: - with self.factory() as db: - server = db.query(McpServer).filter_by(name=name).first() - if server: - db.delete(server) - db.commit() - - def bind_to_project(self, project_id: int, mcp_server_id: int) -> None: - with self.factory() as db: - db.add( - ProjectMcpServer(project_id=project_id, mcp_server_id=mcp_server_id) - ) - db.commit() - - def unbind_from_project(self, project_id: int, mcp_server_id: int) -> None: - with self.factory() as db: - db.query(ProjectMcpServer).filter_by( - project_id=project_id, mcp_server_id=mcp_server_id - ).delete() - db.commit() - - def get_project_mcps(self, project_id: int) -> list[McpServer]: - with self.factory() as db: - ids = [ - r.mcp_server_id - for r in db.query(ProjectMcpServer) - .filter_by(project_id=project_id) - .all() - ] - if not ids: - return [] - return list(db.query(McpServer).filter(McpServer.id.in_(ids)).all()) -``` - -(Note `delete()` re-queries inside its own session rather than calling `self.get()`, because `get()` returns a row detached from a closed session.) - -- [ ] **Step 5: Run tests to verify they pass** - -Run: `uv run pytest tests/test_services.py -v` -Expected: ALL PASS. (`tests/test_integration.py` and `src/hqt/tui/app.py` / `src/hqt/cli.py` are still broken at this point — they're fixed in Tasks 4 and 5; do not run the full suite yet.) - -- [ ] **Step 6: Quality gates + commit** - -`uv run ty check` may flag `cli.py`/`app.py` passing a `Session` where `sessionmaker` is now expected — that is expected breakage fixed in Task 5; if it blocks, note it and commit anyway (gates must be fully green by end of Task 5). - -```bash -uv run ruff format src tests && uv run ruff check src tests -git add src/hqt/projects/service.py src/hqt/mcp/service.py tests/test_services.py -git commit -m "refactor: per-operation DB sessions in Project/Mcp services" -``` - ---- - -### Task 4: `SessionService` — per-operation sessions + user-facing errors (P1 #1, #2) - -**Files:** -- Modify: `src/hqt/sessions/service.py` -- Test: `tests/test_sessions.py`, `tests/test_integration.py` - -- [ ] **Step 1: Update the fixtures in `tests/test_sessions.py`** - -Replace the `db` fixture (lines 13-23) with a `factory` + `db` pair, and update the `service` fixture: - -```python -from sqlalchemy.pool import StaticPool - -from hqt.errors import ServiceError - - -@pytest.fixture -def factory(): - engine = create_engine( - "sqlite://", - connect_args={"check_same_thread": False}, - poolclass=StaticPool, - ) - Base.metadata.create_all(engine) - return sessionmaker(bind=engine, expire_on_commit=False) - - -@pytest.fixture -def db(factory): - # Seeding/assertion handle onto the same in-memory DB the service uses. - session = factory() - session.add(Harness(name="claude-code", display_name="Claude Code")) - session.add(Project(name="myproj", path="/tmp/myproj")) - session.commit() - yield session - session.close() - - -@pytest.fixture -def service(factory, db, tmux, harnesses): - # depends on db so the seed rows exist before the service runs - return SessionService(factory=factory, tmux=tmux, harnesses=harnesses) -``` - -Then mechanically update every inline construction (the `db` fixture must stay in those tests' parameter lists wherever it seeds/asserts): - -```bash -sed -i 's/SessionService(db=db,/SessionService(factory=factory,/' tests/test_sessions.py tests/test_integration.py -``` - -After the sed, every test that constructs `SessionService(factory=factory, ...)` inline must have `factory` in its parameter list (add it where missing — pytest will error loudly on any you miss). - -**Stale-read rule for assertions:** the `db` fixture session caches rows in its identity map and will NOT see service-side mutations on instances it already loaded, and objects returned by the service are detached snapshots. Wherever a test asserts on DB state *after* a service call mutated it (e.g. the capture tests asserting `harness_session_id`, `test_respawn_fallback_rung2_updates_harness_session_id`), insert `db.expire_all()` immediately before the assertion and re-read via `db.get(Session, sess.id)` / `db.query(...)`. Example: - -```python - db.expire_all() - fresh = db.get(Session, result.session.id) - assert fresh.harness_session_id == "captured-id" -``` - -- [ ] **Step 2: Add the new error-path and eager-load tests** - -Append to `tests/test_sessions.py`: - -```python -@pytest.mark.asyncio -async def test_create_session_unknown_project_raises(service): - with pytest.raises(ServiceError, match="no longer exists"): - await service.create_session(9999, "claude-code") - - -@pytest.mark.asyncio -async def test_create_session_uninstalled_harness_raises(factory, db, tmux): - # Harness row exists in the DB but the binary is gone from PATH - # (self.harnesses has no configurator for it). - service = SessionService(factory=factory, tmux=tmux, harnesses={}) - with pytest.raises(ServiceError, match="not installed"): - await service.create_session(1, "claude-code") - - -@pytest.mark.asyncio -async def test_attach_session_missing_row_raises(service): - with pytest.raises(ServiceError, match="not found"): - await service.attach_session(9999) - - -@pytest.mark.asyncio -async def test_list_sessions_rows_usable_after_return(service, db, tmux): - await service.create_session(1, "claude-code") - infos = await service.list_sessions(1) - # The widget reads session.harness.name on detached rows; selectinload - # must have eager-loaded the relationship. - assert infos[0].session.harness.name == "claude-code" - - -@pytest.mark.asyncio -async def test_list_sessions_skips_capture_for_deleted_project( - factory, db, tmux, harnesses_capture -): - # A session whose project row was deleted must not crash the poller. - service = SessionService(factory=factory, tmux=tmux, harnesses=harnesses_capture) - await service.create_session(1, "claude-code") - db.query(Project).delete() - db.commit() - infos = await service.list_sessions(1) - assert len(infos) == 1 # no ServiceError escaped -``` - -(Check the `harnesses_capture` fixture near line 273 for its mock shape; if its `capture_session_id` mock asserts call args, the deleted-project test needs no call to happen at all — which is the behavior under test.) - -- [ ] **Step 3: Run tests to verify they fail** - -Run: `uv run pytest tests/test_sessions.py -v 2>&1 | tail -20` -Expected: widespread FAIL/ERROR — `SessionService` has no `factory` kwarg yet. - -- [ ] **Step 4: Rewrite `src/hqt/sessions/service.py`** - -Full replacement file: - -```python -import asyncio -import logging -import time -from collections.abc import Mapping -from dataclasses import dataclass -from datetime import UTC, datetime -from pathlib import Path - -from sqlalchemy.orm import Session as DBSession -from sqlalchemy.orm import selectinload, sessionmaker - -from hqt.db.models import Harness, Project, Session -from hqt.errors import ServiceError -from hqt.harnesses.base import HarnessConfigurator, SpawnConfig -from hqt.status import window_label -from hqt.tmux.manager import SpawnRequest, TmuxManager -from hqt.tmux.runner import WindowInfo - -log = logging.getLogger(__name__) - -CAPTURE_MAX_ATTEMPTS = 6 -CAPTURE_RETRY_INTERVAL = 0.5 -ACTIVITY_RECENT_SECS = 5 - - -async def _capture_sleep(t: float) -> None: - await asyncio.sleep(t) - - -@dataclass -class SessionInfo: - session: Session - alive: bool - status: str = "dead" - - -@dataclass -class CreateSessionResult: - """Result of create_session, carrying the DB row plus spawn outcome.""" - - session: Session - spawn_ok: bool - spawn_error: str = "" - - -class SessionService: - """Session lifecycle operations. - - Holds a sessionmaker, not a live session: every public method opens its - own short-lived DB session, so concurrent Textual workers and the 3s - poller never share ORM state. Rows returned to callers are detached; - queries eager-load the harness relationship so widgets can read it. - """ - - def __init__( - self, - factory: sessionmaker, - tmux: TmuxManager, - harnesses: Mapping[str, HarnessConfigurator], - ): - self.factory = factory - self.tmux = tmux - self.harnesses = harnesses - - def _configurator(self, harness_name: str) -> HarnessConfigurator: - configurator = self.harnesses.get(harness_name) - if configurator is None: - raise ServiceError(f"Harness '{harness_name}' is not installed") - return configurator - - def _project_path(self, db: DBSession, project_id: int) -> Path: - project = db.get(Project, project_id) - if project is None: - raise ServiceError(f"Project {project_id} no longer exists") - return Path(project.path) - - async def _capture_session_id_with_retry( - self, - configurator: HarnessConfigurator, - project_path: Path, - since: float, - window_name: str, - ) -> str | None: - """Retry capture_session_id up to CAPTURE_MAX_ATTEMPTS times. - - Returns the captured id, or None if all attempts fail (a warning is - logged on exhaustion). - """ - captured_id: str | None = None - for attempt in range(CAPTURE_MAX_ATTEMPTS): - if attempt > 0: - await _capture_sleep(CAPTURE_RETRY_INTERVAL) - captured_id = configurator.capture_session_id(project_path, since) - if captured_id: - break - if not captured_id: - log.warning( - "capture_session_id exhausted %d attempts for window %s; keeping placeholder id", - CAPTURE_MAX_ATTEMPTS, - window_name, - ) - return captured_id - - def _session_created_epoch(self, sess: Session) -> float: - created_at = sess.created_at - if isinstance(created_at, datetime): - if created_at.tzinfo is None: - created_at = created_at.replace(tzinfo=UTC) - return created_at.timestamp() - return 0.0 - - def _maybe_capture_missing_session_id(self, db: DBSession, sess: Session) -> None: - configurator = self.harnesses.get(sess.harness.name) - if configurator is None or not configurator.captures_session_id: - return - - placeholder_id = configurator.generate_session_id(sess.id) - if sess.harness_session_id and sess.harness_session_id != placeholder_id: - return - - # Polling path: a deleted project just means we skip capture, never crash. - project = db.get(Project, sess.project_id) - if project is None: - return - captured_id = configurator.capture_session_id( - Path(project.path), self._session_created_epoch(sess) - ) - if captured_id and captured_id != sess.harness_session_id: - sess.harness_session_id = captured_id - db.commit() - - async def create_session( - self, - project_id: int, - harness_name: str, - nickname: str | None = None, - model: str | None = None, - ) -> "CreateSessionResult": - """Create a new session row, spawn the harness window, and return a - CreateSessionResult with the Session row and spawn outcome. - - The capture-retry loop only runs when spawn succeeded. - """ - log.info("Creating session: project=%s harness=%s", project_id, harness_name) - configurator = self._configurator(harness_name) - with self.factory() as db: - harness_row = db.query(Harness).filter_by(name=harness_name).first() - if not harness_row: - log.error("Harness not in DB: %s", harness_name) - raise ServiceError(f"Unknown harness: {harness_name}") - project_path = self._project_path(db, project_id) - sess = Session( - project_id=project_id, - harness_id=harness_row.id, - nickname=nickname, - model=model, - tmux_session_name="placeholder", - archived=False, - ) - db.add(sess) - db.flush() - sess.tmux_session_name = f"hqt-{sess.id}" - harness_session_id = configurator.generate_session_id(sess.id) - sess.harness_session_id = harness_session_id - 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, - ) - 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() - log.info( - "Session created: id=%s window=%s", sess.id, sess.tmux_session_name - ) - return CreateSessionResult( - session=sess, - spawn_ok=spawn_result.ok, - spawn_error=spawn_result.error or "", - ) - - async def attach_session(self, session_id: int) -> bool: - """Attach to a session. Handles all states: alive, dead pane, gone.""" - with self.factory() as db: - sess = db.get( - Session, session_id, options=[selectinload(Session.harness)] - ) - if sess is None: - raise ServiceError("Session not found") - window_name = sess.tmux_session_name - self._maybe_capture_missing_session_id(db, sess) - - if await self.tmux.is_alive(window_name): - # Window exists, process running — just switch - return await self.tmux.attach(window_name) - - # Window exists but pane is dead, OR window is completely gone - log.info( - "Session %s not alive, respawning with fallback ladder", window_name - ) - ok = await self._respawn_with_fallback(db, sess, window_name) - if not ok: - return False - return await self.tmux.attach(window_name) - - async def _respawn_with_fallback( - self, db: DBSession, sess: Session, window_name: str - ) -> bool: - """Try resume config first; if it dies, fall back to fresh spawn config. - - Returns True if either rung succeeded, False if both failed. - - On rung-2 success, if the harness captures_session_id, a capture-retry - loop updates sess.harness_session_id so later resumes target the fresh - conversation rather than the stale/nonexistent one. - """ - # Rung 1: resume (restore prior conversation) - resume_cfg = self._get_resume_config(db, sess) - result = await self.tmux.respawn_verified( - window_name, resume_cfg.command, str(resume_cfg.cwd), env=resume_cfg.env - ) - if result.ok: - return True - - log.warning( - "Resume failed for %s (%s), falling back to fresh spawn", - window_name, - result.error or "(no output)", - ) - - # 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 - ) - 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 - - async def stop_session(self, session_id: int) -> None: - with self.factory() as db: - sess = db.get(Session, session_id) - if sess is None: - return - window_name = sess.tmux_session_name - if await self.tmux.window_exists(window_name): - await self.tmux.kill(window_name) - - def get_session(self, session_id: int) -> Session | None: - """Return the session row, or None if it does not exist.""" - with self.factory() as db: - return db.get( - Session, session_id, options=[selectinload(Session.harness)] - ) - - def rename_session(self, session_id: int, nickname: str | None) -> None: - """Update a session's display nickname. - - An empty/blank nickname clears it to None, so the label falls back to - the tmux window name. The tmux window label is refreshed by the next - poll (sync_window_labels), so no immediate tmux call is needed here. - """ - with self.factory() as db: - sess = db.get(Session, session_id) - if sess is None: - return - sess.nickname = (nickname or "").strip() or None - db.commit() - - async def delete_session(self, session_id: int) -> None: - with self.factory() as db: - sess = db.get(Session, session_id) - if sess is None: - return - if await self.tmux.window_exists(sess.tmux_session_name): - await self.tmux.kill(sess.tmux_session_name) - db.delete(sess) - db.commit() - - async def _status_for( - self, sess: Session, wi: WindowInfo | None, now: float - ) -> str: - """Compute a session's status string from its window info. - - Dead/missing window → "dead". Alive window → the harness's parsed status - (working/waiting) when recognizable, else an activity-based active/idle. - capture_pane is only called for alive windows. - """ - if wi is None or not wi.alive: - return "dead" - configurator = self.harnesses.get(sess.harness.name) - pane_text = await self.tmux.capture_pane(sess.tmux_session_name) - parsed: str | None = None - if pane_text and configurator is not None: - parsed = configurator.parse_status(pane_text) - if parsed is not None: - return parsed - age = now - wi.last_activity - return "active" if age <= ACTIVITY_RECENT_SECS else "idle" - - async def list_sessions( - self, project_id: int, now: float | None = None - ) -> list[SessionInfo]: - if now is None: - now = time.time() - with self.factory() as db: - sessions = ( - db.query(Session) - .options(selectinload(Session.harness)) - .filter_by(project_id=project_id, archived=False) - .all() - ) - names = [s.tmux_session_name for s in sessions] - info_map = await self.tmux.poll_info(names) - result: list[SessionInfo] = [] - for s in sessions: - self._maybe_capture_missing_session_id(db, s) - wi = info_map.get(s.tmux_session_name) - status = await self._status_for(s, wi, now) - result.append( - SessionInfo(session=s, alive=bool(wi and wi.alive), status=status) - ) - return result - - async def sync_window_labels(self, now: float | None = None) -> list[SessionInfo]: - """Refresh every session's tmux window label to reflect its status. - - Runs session-wide (all projects), so the tmux status bar stays accurate - regardless of which project is selected in the TUI. Returns SessionInfo - for all sessions so the caller can reuse the computed status for the UI - without recomputing. Labels are set by NAME; windows that no longer exist - are harmless no-ops. - """ - if now is None: - now = time.time() - with self.factory() as db: - sessions = ( - db.query(Session) - .options(selectinload(Session.harness)) - .filter_by(archived=False) - .all() - ) - names = [s.tmux_session_name for s in sessions] - info_map = await self.tmux.poll_info(names) - result: list[SessionInfo] = [] - for s in sessions: - self._maybe_capture_missing_session_id(db, s) - wi = info_map.get(s.tmux_session_name) - alive = bool(wi and wi.alive) - status = await self._status_for(s, wi, now) - result.append(SessionInfo(session=s, alive=alive, status=status)) - name = s.nickname or s.tmux_session_name - await self.tmux.set_window_label( - s.tmux_session_name, window_label(status, name, alive=alive) - ) - return result - - def _get_resume_config(self, db: DBSession, sess: Session) -> SpawnConfig: - configurator = self._configurator(sess.harness.name) - harness_session_id = ( - sess.harness_session_id or configurator.generate_session_id(sess.id) - ) - return configurator.build_resume_config( - self._project_path(db, sess.project_id), harness_session_id, sess.model - ) -``` - -- [ ] **Step 5: Update `tests/test_integration.py` wiring** - -In the `setup` fixture: keep the local `factory` variable, change `ProjectService(db)` → `ProjectService(factory)` (the `SessionService(db=db,` line was already handled by the Step-1 sed). The seeding `db = factory()` handle stays as-is. - -- [ ] **Step 6: Run tests, fix stale-read assertions per the rule in Step 1** - -Run: `uv run pytest tests/test_sessions.py tests/test_integration.py tests/test_services.py -v 2>&1 | tail -30` - -Any remaining failures should be exactly the stale-identity-map pattern (assertion sees a pre-mutation value). For each, apply the `db.expire_all()` + re-read pattern from Step 1. Re-run until: ALL PASS. - -- [ ] **Step 7: Quality gates + commit** - -`ty` may still flag `src/hqt/tui/app.py` and `src/hqt/cli.py` (fixed next task) — everything else must be clean. - -```bash -uv run ruff format src tests && uv run ruff check src tests -git add src/hqt/sessions/service.py tests/test_sessions.py tests/test_integration.py -git commit -m "refactor: per-operation DB sessions and ServiceError in SessionService" -``` - ---- - -### Task 5: Wire the TUI and CLI — factory + `ServiceError` → notifications - -**Files:** -- Modify: `src/hqt/tui/app.py` -- Modify: `src/hqt/cli.py:134-138` -- Test: `tests/test_tui.py` - -- [ ] **Step 1: Update `tests/test_tui.py` seeding** - -Tests seed via `app._db_session` (around lines 50-63, 81-95, 205-219, 270+). The app will now expose `_db_factory` instead. In each seeding block, open a session from the factory: - -```python - seed_db = app._db_factory() - proj = Project(name="p", path="/tmp/p") - seed_db.add(proj) - seed_db.flush() - harness = seed_db.query(Harness).first() - ... - seed_db.add(sess) - seed_db.commit() - seed_db.close() -``` - -i.e. mechanically: insert `seed_db = app._db_factory()` at the top of each block, replace `app._db_session` → `seed_db`, and add `seed_db.close()` after the final `commit()` in the block. (Keep any later assertions reading via a fresh `app._db_factory()` session the same way.) - -- [ ] **Step 2: Run TUI tests to verify they fail** - -Run: `uv run pytest tests/test_tui.py -v 2>&1 | tail -10` -Expected: FAIL — `HqtApp` has no `_db_factory` yet. - -- [ ] **Step 3: Update `src/hqt/tui/app.py`** - -Add the import: - -```python -from hqt.errors import ServiceError -``` - -In `__init__`, replace `self._db_session = None` with: - -```python - self._db_factory = None -``` - -In `on_mount` (lines 101-122), replace the DB wiring: - -```python - settings = get_settings() - ensure_db(settings) - engine = get_engine(settings) - self._db_factory = get_session_factory(engine) - with self._db_factory() as db: - ensure_harnesses_in_db(db) -``` - -and the service construction: - -```python - self._project_service = ProjectService(self._db_factory) - self._session_service = SessionService(self._db_factory, tmux, harnesses) -``` - -Add a worker helper after `_sessions()`: - -```python - def _run_service_worker(self, coro) -> None: - """run_worker, but ServiceError surfaces as a notification. - - Services raise ServiceError for expected failures (rows deleted - underneath an action, harness uninstalled); an uncaught exception - would kill the worker and crash the app. - """ - - async def _wrapped() -> None: - try: - await coro - except ServiceError as err: - self.notify(str(err), severity="error") - - self.run_worker(_wrapped()) -``` - -Then route every service-touching worker through it — replace `self.run_worker(_do())` with `self._run_service_worker(_do())` in: `action_new_session.on_dismiss`, `action_attach_session`, `action_delete_session`, `action_stop_session`, and replace `self.run_worker(self._refresh_sessions(restore_selection=True))` in `on_project_selected` and `self.run_worker(self._refresh_sessions())` in `action_rename_session` with `self._run_service_worker(...)` of the same coroutine. Leave `_load_projects`'s widget-refresh worker and the theme worker on plain `run_worker`. - -Wrap the synchronous service calls: - -In `action_add_project.on_dismiss`: - -```python - def on_dismiss(result: tuple[str, str] | None) -> None: - if result: - name, path = result - try: - self._projects().create(name, path) - except ServiceError as err: - self.notify(str(err), severity="error") - return - self._load_projects() -``` - -In `action_edit_project.on_dismiss`, change `except ValueError` to `except ServiceError`. - -In `_poll_sessions`, guard the interval callback (a crash here kills polling for the rest of the app's life): - -```python - async def _poll_sessions(self) -> None: - # Sync every session's tmux window label (session-wide), then update the - # UI for the selected project from the same computed status — no recompute. - try: - infos = await self._sessions().sync_window_labels() - except ServiceError as err: - log.warning("Session poll failed: %s", err) - return - if self._selected_project_id is not None: - subset = [ - i for i in infos if i.session.project_id == self._selected_project_id - ] - await self.query_one(SessionList).refresh_sessions(subset) -``` - -- [ ] **Step 4: Update `src/hqt/cli.py` `list_cmd`** - -```python - settings = get_settings() - ensure_db(settings) - engine = get_engine(settings) - factory = get_session_factory(engine) - svc = ProjectService(factory) - for p in svc.list_all(): - click.echo(f" {p.name} ({p.path})") -``` - -(The `with Session() as db:` block is gone — the service scopes its own sessions.) - -- [ ] **Step 5: Run the FULL test suite** - -Run: `uv run pytest -v 2>&1 | tail -15` -Expected: ALL PASS, no skips introduced. - -- [ ] **Step 6: Quality gates (must be fully green now) + commit** - -```bash -uv run ruff format src tests && uv run ruff check src tests && uv run ty check -git add src/hqt/tui/app.py src/hqt/cli.py tests/test_tui.py -git commit -m "feat: factory-based DB wiring and ServiceError notifications in TUI" -``` - ---- - -### Task 6: Manual smoke test + close out TODO.md - -**Files:** -- Modify: `TODO.md` (remove lines 1-3, the three P1 findings) - -- [ ] **Step 1: Smoke-test against a real pre-existing DB** - -```bash -cp ~/.local/share/hqt/hqt.db /tmp/hqt-backup.db 2>/dev/null || echo "no existing db (fresh install) — skip restore step" -uv run hqt list -``` - -Expected: project list prints without traceback (this exercises `ensure_db` → `migrate()` against a real unversioned DB, stamping it to baseline). Then verify the stamp: - -```bash -sqlite3 ~/.local/share/hqt/hqt.db "PRAGMA user_version" -``` - -Expected output: `1` - -If anything broke, restore: `cp /tmp/hqt-backup.db ~/.local/share/hqt/hqt.db`. - -- [ ] **Step 2: Remove the three P1 lines from `TODO.md`** - -Delete lines 1-3 so the file starts with the first P2 finding. Leave all P2 lines untouched. - -- [ ] **Step 3: Final full verification** - -```bash -uv run pytest -q && uv run ruff format src tests && uv run ruff check src tests && uv run ty check -``` - -Expected: tests all pass, gates green. - -- [ ] **Step 4: Commit** - -```bash -git add TODO.md -git commit -m "chore: close out P1 findings" -``` diff --git a/docs/superpowers/plans/2026-06-10-p2-fixes.md b/docs/superpowers/plans/2026-06-10-p2-fixes.md deleted file mode 100644 index 0a6f49d..0000000 --- a/docs/superpowers/plans/2026-06-10-p2-fixes.md +++ /dev/null @@ -1,822 +0,0 @@ -# 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`. diff --git a/docs/superpowers/plans/2026-06-10-sandboxed-sessions.md b/docs/superpowers/plans/2026-06-10-sandboxed-sessions.md deleted file mode 100644 index 5bf602f..0000000 --- a/docs/superpowers/plans/2026-06-10-sandboxed-sessions.md +++ /dev/null @@ -1,1206 +0,0 @@ -# Sandboxed Sessions (bubblewrap) 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:** Let a user start a harness session inside a bubblewrap (`bwrap`) sandbox, chosen per session, with read-only/read-write project access and an internet on/off toggle; sandboxed sessions automatically get the harness's "skip permissions" flag. - -**Architecture:** A new pure module `hqt/sandbox.py` builds the `bwrap … -- ` argv from a `SandboxPolicy` plus per-harness `Bind`s. The `HarnessConfigurator` ABC gains the per-harness skip-permission flags and the credential/config paths to bind. `SessionService` wraps the spawn/resume command when a session is sandboxed and persists the policy in a new `Session.sandbox_json` column. The New Session dialog exposes the toggles and disables them when `bwrap` is unavailable (the same check `hqt doctor` uses). - -**Tech Stack:** Python 3.14, SQLAlchemy (SQLite, `user_version` migrations), Textual (TUI), Click (CLI), pytest + pytest-asyncio. - ---- - -## File Structure - -- **Create** `src/hqt/sandbox.py` — `Bind`, `SandboxPolicy`, `is_available()`, `wrap()`. Pure (no subprocess); the single source of truth for "can we sandbox?" and "how do we build the bwrap argv". -- **Modify** `src/hqt/harnesses/base.py` — add `sandbox_skip_permission_flags` class attr, `sandbox_binds()` method, and a `sandboxed: bool = False` parameter to the two abstract build methods. -- **Modify** `src/hqt/harnesses/configurators/{claude,codex,kiro,generic}.py` — accept `sandboxed`, append flags, declare binds. -- **Modify** `src/hqt/db/models.py` — add `Session.sandbox_json` column. -- **Modify** `src/hqt/db/migrations.py` — add version-2 migration adding the column. -- **Modify** `src/hqt/sessions/service.py` — `create_session(sandbox=...)`, wrap command, persist policy, re-wrap on resume/respawn. -- **Modify** `src/hqt/tui/screens/new_session.py` — sandbox toggle + sub-controls + availability gating; extend the result tuple. -- **Modify** `src/hqt/tui/app.py` — pass availability to the screen; unpack the sandbox config; pass it to `create_session`. -- **Modify** `src/hqt/cli.py` — `doctor` reports bubblewrap availability. -- **Tests:** `tests/test_sandbox.py` (new), and additions to `tests/test_harnesses.py`, `tests/test_db.py`, `tests/test_sessions.py`, `tests/test_tui.py`, `tests/test_cli.py` (new or existing). - -Run the full suite with `uv run pytest -q` and lint with `uv run ruff check src tests` at checkpoints. - ---- - -## Task 1: Sandbox primitives — `Bind`, `SandboxPolicy`, `is_available()` - -**Files:** -- Create: `src/hqt/sandbox.py` -- Test: `tests/test_sandbox.py` - -- [ ] **Step 1: Write the failing tests** - -```python -# tests/test_sandbox.py -import sys -from pathlib import Path - -from hqt import sandbox -from hqt.sandbox import Bind, SandboxPolicy - - -def test_bind_defaults_readonly(): - b = Bind(Path("/home/u/.claude")) - assert b.src == Path("/home/u/.claude") - assert b.writable is False - - -def test_sandbox_policy_fields(): - p = SandboxPolicy(fs="rw", net=True) - assert p.fs == "rw" - assert p.net is True - - -def test_is_available_true_on_linux_with_bwrap(monkeypatch): - monkeypatch.setattr(sandbox.sys, "platform", "linux") - monkeypatch.setattr(sandbox.shutil, "which", lambda name: "/usr/bin/bwrap") - assert sandbox.is_available() is True - - -def test_is_available_false_without_bwrap(monkeypatch): - monkeypatch.setattr(sandbox.sys, "platform", "linux") - monkeypatch.setattr(sandbox.shutil, "which", lambda name: None) - assert sandbox.is_available() is False - - -def test_is_available_false_off_linux(monkeypatch): - monkeypatch.setattr(sandbox.sys, "platform", "darwin") - monkeypatch.setattr(sandbox.shutil, "which", lambda name: "/usr/bin/bwrap") - assert sandbox.is_available() is False -``` - -- [ ] **Step 2: Run tests to verify they fail** - -Run: `uv run pytest tests/test_sandbox.py -q` -Expected: FAIL — `ModuleNotFoundError: No module named 'hqt.sandbox'`. - -- [ ] **Step 3: Create the module with primitives** - -```python -# src/hqt/sandbox.py -import os -import shutil -import sys -from dataclasses import dataclass -from pathlib import Path - - -@dataclass(frozen=True) -class Bind: - """A host path to expose inside the sandbox (read-only unless writable).""" - - src: Path - writable: bool = False - - -@dataclass(frozen=True) -class SandboxPolicy: - """Per-session sandbox settings. - - fs: "rw" binds the project dir writable; "ro" binds it read-only. - net: True shares the host network namespace; False = no connectivity. - """ - - fs: str - net: bool - - -def is_available() -> bool: - """True when bubblewrap can be used: Linux with `bwrap` on PATH. - - The single source of truth for both `hqt doctor` and the New Session dialog. - """ - return sys.platform.startswith("linux") and shutil.which("bwrap") is not None -``` - -(`os` is imported now; Task 2 uses it for env passthrough.) - -- [ ] **Step 4: Run tests to verify they pass** - -Run: `uv run pytest tests/test_sandbox.py -q` -Expected: PASS (5 passed). - -- [ ] **Step 5: Commit** - -```bash -git add src/hqt/sandbox.py tests/test_sandbox.py -git commit -m "feat: sandbox primitives — Bind, SandboxPolicy, is_available" -``` - ---- - -## Task 2: `wrap()` — build the bwrap argv - -**Files:** -- Modify: `src/hqt/sandbox.py` -- Test: `tests/test_sandbox.py` - -- [ ] **Step 1: Write the failing tests** - -```python -# tests/test_sandbox.py (append) -def _split(args, flag): - """Return list of (src, dst) pairs that follow each occurrence of flag.""" - out = [] - for i, a in enumerate(args): - if a == flag: - out.append((args[i + 1], args[i + 2])) - return out - - -def test_wrap_command_after_double_dash(tmp_path): - cmd = ["claude", "--session-id", "x"] - args = sandbox.wrap(cmd, tmp_path, SandboxPolicy(fs="rw", net=True), []) - assert args[0] == "bwrap" - assert "--" in args - assert args[args.index("--") + 1 :] == cmd - - -def test_wrap_base_flags(tmp_path): - args = sandbox.wrap(["sh"], tmp_path, SandboxPolicy(fs="rw", net=True), []) - assert "--die-with-parent" in args - assert "--unshare-all" in args - assert "--proc" in args and "--dev" in args - - -def test_wrap_net_on_shares_net(tmp_path): - args = sandbox.wrap(["sh"], tmp_path, SandboxPolicy(fs="rw", net=True), []) - assert "--share-net" in args - - -def test_wrap_net_off_does_not_share(tmp_path): - args = sandbox.wrap(["sh"], tmp_path, SandboxPolicy(fs="rw", net=False), []) - assert "--share-net" not in args - - -def test_wrap_cwd_rw_is_bind(tmp_path): - args = sandbox.wrap(["sh"], tmp_path, SandboxPolicy(fs="rw", net=True), []) - assert (str(tmp_path), str(tmp_path)) in _split(args, "--bind") - - -def test_wrap_cwd_ro_is_ro_bind(tmp_path): - args = sandbox.wrap(["sh"], tmp_path, SandboxPolicy(fs="ro", net=True), []) - assert (str(tmp_path), str(tmp_path)) in _split(args, "--ro-bind") - - -def test_wrap_writable_bind_spliced(tmp_path): - cred = tmp_path / "cred" - cred.mkdir() - args = sandbox.wrap( - ["sh"], tmp_path, SandboxPolicy(fs="rw", net=True), [Bind(cred, writable=True)] - ) - assert (str(cred), str(cred)) in _split(args, "--bind") - - -def test_wrap_readonly_bind_spliced(tmp_path): - cred = tmp_path / "cred" - cred.mkdir() - args = sandbox.wrap( - ["sh"], tmp_path, SandboxPolicy(fs="rw", net=True), [Bind(cred)] - ) - assert (str(cred), str(cred)) in _split(args, "--ro-bind") - - -def test_wrap_skips_missing_binds(tmp_path): - missing = tmp_path / "nope" - args = sandbox.wrap( - ["sh"], tmp_path, SandboxPolicy(fs="rw", net=True), [Bind(missing)] - ) - assert str(missing) not in args -``` - -- [ ] **Step 2: Run tests to verify they fail** - -Run: `uv run pytest tests/test_sandbox.py -k wrap -q` -Expected: FAIL — `AttributeError: module 'hqt.sandbox' has no attribute 'wrap'`. - -- [ ] **Step 3: Implement `wrap()`** - -Append to `src/hqt/sandbox.py`: - -```python -# System paths bound read-only so binaries and shared libraries resolve. -_SYSTEM_RO_PATHS = ["/usr", "/bin", "/sbin", "/lib", "/lib64", "/etc", "/opt"] - -# Environment variables passed through into the jail. -_PASSTHROUGH_ENV = ["PATH", "HOME", "TERM", "LANG", "LC_ALL", "USER", "SHELL"] - - -def wrap( - command: list[str], - cwd: Path, - policy: SandboxPolicy, - binds: list[Bind], -) -> list[str]: - """Build the `bwrap … -- ` argv for a sandboxed harness. - - Minimal-allowlist model: $HOME and other user data are NOT exposed. System - dirs are read-only; the project dir (cwd) and each existing harness bind are - bound explicitly. Network is all-or-nothing: shared only when policy.net. - Binds whose source does not exist are skipped (bwrap would otherwise error). - """ - args: list[str] = ["bwrap", "--die-with-parent", "--unshare-all"] - if policy.net: - args.append("--share-net") - for path in _SYSTEM_RO_PATHS: - if Path(path).exists(): - args += ["--ro-bind", path, path] - args += ["--proc", "/proc", "--dev", "/dev", "--tmpfs", "/tmp"] - for bind in binds: - if not bind.src.exists(): - continue - flag = "--bind" if bind.writable else "--ro-bind" - args += [flag, str(bind.src), str(bind.src)] - cwd_flag = "--bind" if policy.fs == "rw" else "--ro-bind" - args += [cwd_flag, str(cwd), str(cwd)] - for name in _PASSTHROUGH_ENV: - value = os.environ.get(name) - if value is not None: - args += ["--setenv", name, value] - args.append("--") - args += command - return args -``` - -- [ ] **Step 4: Run tests to verify they pass** - -Run: `uv run pytest tests/test_sandbox.py -q` -Expected: PASS (all `wrap` tests + Task 1 tests). - -- [ ] **Step 5: Commit** - -```bash -git add src/hqt/sandbox.py tests/test_sandbox.py -git commit -m "feat: sandbox.wrap builds bwrap argv from policy and binds" -``` - ---- - -## Task 3: Configurator ABC — skip-permission flags and binds - -**Files:** -- Modify: `src/hqt/harnesses/base.py` -- Modify: `src/hqt/harnesses/configurators/claude.py:24-38` -- Modify: `src/hqt/harnesses/configurators/codex.py:22-36` -- Modify: `src/hqt/harnesses/configurators/kiro.py:13-21` -- Modify: `src/hqt/harnesses/configurators/generic.py:16-24` -- Test: `tests/test_harnesses.py` - -- [ ] **Step 1: Write the failing tests** - -```python -# tests/test_harnesses.py (append) -from pathlib import Path - -from hqt.harnesses.base import HarnessConfigurator -from hqt.harnesses.configurators.kiro import KiroConfigurator -from hqt.harnesses.configurators.generic import GenericConfigurator - - -def test_base_skip_flags_default_empty(): - assert HarnessConfigurator.sandbox_skip_permission_flags == [] - - -def test_base_sandbox_binds_default_empty(): - assert KiroConfigurator().sandbox_binds() == [] - - -def test_claude_skip_flag_added_when_sandboxed(): - c = ClaudeConfigurator() - sid = c.generate_session_id(1) - cfg = c.build_spawn_config(Path("/tmp"), sid, sandboxed=True) - assert "--dangerously-skip-permissions" in cfg.command - - -def test_claude_skip_flag_absent_when_not_sandboxed(): - c = ClaudeConfigurator() - sid = c.generate_session_id(1) - cfg = c.build_spawn_config(Path("/tmp"), sid, sandboxed=False) - assert "--dangerously-skip-permissions" not in cfg.command - - -def test_claude_resume_skip_flag_when_sandboxed(): - c = ClaudeConfigurator() - sid = c.generate_session_id(1) - cfg = c.build_resume_config(Path("/tmp"), sid, sandboxed=True) - assert "--dangerously-skip-permissions" in cfg.command - - -def test_claude_sandbox_binds_includes_dot_claude(): - c = ClaudeConfigurator() - binds = c.sandbox_binds() - assert any(b.src == Path.home() / ".claude" and b.writable for b in binds) - - -def test_codex_skip_flag_added_when_sandboxed(): - c = CodexConfigurator() - cfg = c.build_spawn_config(Path("/tmp"), "1", sandboxed=True) - assert "--dangerously-bypass-approvals-and-sandbox" in cfg.command - - -def test_generic_sandboxed_is_noop_flagwise(): - g = GenericConfigurator(["mytool"]) - cfg = g.build_spawn_config(Path("/tmp"), "1", sandboxed=True) - assert cfg.command == ["mytool"] -``` - -- [ ] **Step 2: Run tests to verify they fail** - -Run: `uv run pytest tests/test_harnesses.py -k "sandbox or skip or sandboxed or binds" -q` -Expected: FAIL — `TypeError: build_spawn_config() got an unexpected keyword argument 'sandboxed'` / missing attributes. - -- [ ] **Step 3: Update the ABC** - -In `src/hqt/harnesses/base.py`, add the import and the two members, and add `sandboxed` to both abstract signatures: - -```python -from abc import ABC, abstractmethod -from dataclasses import dataclass, field -from pathlib import Path - -from hqt.sandbox import Bind - - -@dataclass -class SpawnConfig: - command: list[str] - env: dict[str, str] = field(default_factory=dict) - cwd: Path = Path(".") - - -class HarnessConfigurator(ABC): - name: str - display_name: str - captures_session_id: bool = False - # Flags appended to the harness command when the session is sandboxed. - sandbox_skip_permission_flags: list[str] = [] - - @abstractmethod - def generate_session_id(self, hqt_session_id: int) -> str: ... - - @abstractmethod - def build_spawn_config( - self, - project_path: Path, - harness_session_id: str, - model: str | None = None, - sandboxed: bool = False, - ) -> SpawnConfig: ... - - @abstractmethod - def build_resume_config( - self, - project_path: Path, - harness_session_id: str, - model: str | None = None, - sandboxed: bool = False, - ) -> SpawnConfig: ... - - def sandbox_binds(self) -> list[Bind]: - """Host paths this harness needs inside the jail (credentials, config).""" - return [] - - def capture_session_id(self, project_path: Path, since: float) -> str | None: - return None - - def parse_status(self, pane_text: str) -> str | None: - """Parse visible pane text to determine harness status. - - Returns "working", "waiting", or None (unknown — caller uses activity-based fallback). - Default implementation returns None for all harnesses that don't override. - """ - return None -``` - -- [ ] **Step 4: Update Claude configurator** - -Replace the body of `src/hqt/harnesses/configurators/claude.py` build methods and add binds. Add `from hqt.sandbox import Bind` to the imports: - -```python -class ClaudeConfigurator(HarnessConfigurator): - name = "claude" - display_name = "Claude Code" - sandbox_skip_permission_flags = ["--dangerously-skip-permissions"] - - # ... _WORKING_MARKERS / _WAITING_MARKERS unchanged ... - - def sandbox_binds(self) -> list[Bind]: - return [Bind(Path.home() / ".claude", writable=True)] - - def build_spawn_config( - self, - project_path: Path, - harness_session_id: str, - model: str | None = None, - sandboxed: bool = False, - ) -> SpawnConfig: - cmd = ["claude", "--session-id", harness_session_id] - if model: - cmd += ["--model", model] - if sandboxed: - cmd += self.sandbox_skip_permission_flags - return SpawnConfig(command=cmd, cwd=project_path) - - def build_resume_config( - self, - project_path: Path, - harness_session_id: str, - model: str | None = None, - sandboxed: bool = False, - ) -> SpawnConfig: - cmd = ["claude", "--resume", harness_session_id] - if model: - cmd += ["--model", model] - if sandboxed: - cmd += self.sandbox_skip_permission_flags - return SpawnConfig(command=cmd, cwd=project_path) -``` - -- [ ] **Step 5: Update Codex configurator** - -In `src/hqt/harnesses/configurators/codex.py` add `from hqt.sandbox import Bind`, set the flag, add binds, and thread `sandboxed`: - -```python -class CodexConfigurator(HarnessConfigurator): - name = "codex" - display_name = "Codex" - captures_session_id = True - sandbox_skip_permission_flags = ["--dangerously-bypass-approvals-and-sandbox"] - - def sandbox_binds(self) -> list[Bind]: - return [Bind(Path.home() / ".codex", writable=True)] - - def build_spawn_config( - self, - project_path: Path, - harness_session_id: str, - model: str | None = None, - sandboxed: bool = False, - ) -> SpawnConfig: - cmd = ["codex"] - if model: - cmd += ["-m", model] - if sandboxed: - cmd += self.sandbox_skip_permission_flags - return SpawnConfig(command=cmd, cwd=project_path) - - def build_resume_config( - self, - project_path: Path, - harness_session_id: str, - model: str | None = None, - sandboxed: bool = False, - ) -> SpawnConfig: - cmd = ["codex", "resume", harness_session_id] - if model: - cmd += ["-m", model] - if sandboxed: - cmd += self.sandbox_skip_permission_flags - return SpawnConfig(command=cmd, cwd=project_path) -``` - -(Leave `capture_session_id`, `parse_status`, and `_meta_timestamp` unchanged.) - -- [ ] **Step 6: Update Kiro and Generic configurators** - -`src/hqt/harnesses/configurators/kiro.py` — add `from hqt.sandbox import Bind`, declare binds, accept `sandboxed` (no flag to append; kiro has none): - -```python -class KiroConfigurator(HarnessConfigurator): - name = "kiro" - display_name = "Kiro" - - def sandbox_binds(self) -> list[Bind]: - return [Bind(Path.home() / ".kiro", writable=True)] - - def generate_session_id(self, hqt_session_id: int) -> str: - return str(hqt_session_id) - - def build_spawn_config( - self, - project_path: Path, - harness_session_id: str, - model: str | None = None, - sandboxed: bool = False, - ) -> SpawnConfig: - return SpawnConfig(command=["kiro-cli", "chat"], cwd=project_path) - - def build_resume_config( - self, - project_path: Path, - harness_session_id: str, - model: str | None = None, - sandboxed: bool = False, - ) -> SpawnConfig: - return SpawnConfig(command=["kiro-cli", "chat"], cwd=project_path) -``` - -`src/hqt/harnesses/configurators/generic.py` — accept `sandboxed` (no flags, no binds): - -```python - def build_spawn_config( - self, - project_path: Path, - harness_session_id: str, - model: str | None = None, - sandboxed: bool = False, - ) -> SpawnConfig: - return SpawnConfig(command=self._command, cwd=project_path) - - def build_resume_config( - self, - project_path: Path, - harness_session_id: str, - model: str | None = None, - sandboxed: bool = False, - ) -> SpawnConfig: - return SpawnConfig(command=self._command, cwd=project_path) -``` - -- [ ] **Step 7: Run tests to verify they pass** - -Run: `uv run pytest tests/test_harnesses.py -q` -Expected: PASS (existing + new). Existing positional calls like `build_spawn_config(Path("/tmp"), sid, model="opus")` still work because `sandboxed` defaults to `False`. - -- [ ] **Step 8: Commit** - -```bash -git add src/hqt/harnesses tests/test_harnesses.py -git commit -m "feat: configurators declare sandbox skip-permission flags and binds" -``` - ---- - -## Task 4: Persist the policy — `Session.sandbox_json` column + migration - -**Files:** -- Modify: `src/hqt/db/models.py:32-45` -- Modify: `src/hqt/db/migrations.py:15` -- Test: `tests/test_db.py` - -- [ ] **Step 1: Write the failing tests** - -```python -# tests/test_db.py (append) -def test_latest_version_is_two(): - assert migrations.LATEST_VERSION == 2 - - -def test_migration_adds_sandbox_json_column(tmp_path): - # Simulate a baseline DB whose sessions table predates the column. - settings = _tmp_settings(tmp_path) - settings.db_path.parent.mkdir(parents=True, exist_ok=True) - engine = get_engine(settings) - with engine.begin() as conn: - conn.exec_driver_sql( - "CREATE TABLE sessions (" - "id INTEGER PRIMARY KEY, project_id INTEGER, harness_id INTEGER, " - "tmux_session_name TEXT)" - ) - conn.exec_driver_sql("PRAGMA user_version = 1") - migrate(engine) - cols = [c["name"] for c in inspect(engine).get_columns("sessions")] - assert "sandbox_json" in cols - assert _user_version(engine) == 2 - - -def test_session_round_trips_sandbox_json(tmp_path): - settings = _tmp_settings(tmp_path) - ensure_db(settings) - factory = get_session_factory(get_engine(settings)) - with factory() as session: - p = Project(name="proj", path="/tmp/proj") - h = Harness(name="claude") - session.add_all([p, h]) - session.commit() - s = Session( - project_id=p.id, - harness_id=h.id, - tmux_session_name="hqt-9", - sandbox_json='{"fs": "rw", "net": true}', - ) - session.add(s) - session.commit() - assert session.get(Session, s.id).sandbox_json == '{"fs": "rw", "net": true}' -``` - -- [ ] **Step 2: Run tests to verify they fail** - -Run: `uv run pytest tests/test_db.py -k "sandbox or version_is_two" -q` -Expected: FAIL — `LATEST_VERSION == 1`, no `sandbox_json` attribute/column. - -- [ ] **Step 3: Add the model column** - -In `src/hqt/db/models.py`, add to `Session` after `model`: - -```python - model: Mapped[str | None] = mapped_column(default=None) - sandbox_json: Mapped[str | None] = mapped_column(default=None) -``` - -- [ ] **Step 4: Add the migration** - -In `src/hqt/db/migrations.py`, replace the empty `MIGRATIONS` list: - -```python -MIGRATIONS: list[tuple[int, Callable[[Connection], None]]] = [ - ( - 2, - lambda conn: conn.exec_driver_sql( - "ALTER TABLE sessions ADD COLUMN sandbox_json TEXT" - ), - ), -] -``` - -- [ ] **Step 5: Run tests to verify they pass** - -Run: `uv run pytest tests/test_db.py -q` -Expected: PASS (existing migration tests still pass; `LATEST_VERSION` is now 2 and fresh DBs are stamped 2). - -- [ ] **Step 6: Commit** - -```bash -git add src/hqt/db/models.py src/hqt/db/migrations.py tests/test_db.py -git commit -m "feat: add Session.sandbox_json column and migration" -``` - ---- - -## Task 5: Service integration — wrap on spawn and resume - -**Files:** -- Modify: `src/hqt/sessions/service.py` -- Test: `tests/test_sessions.py` - -- [ ] **Step 1: Write the failing tests** - -```python -# tests/test_sessions.py (append) -from hqt.sandbox import SandboxPolicy - - -@pytest.fixture -def sandbox_harness(): - h = MagicMock() - h.captures_session_id = False - h.generate_session_id.return_value = "sess-1" - h.build_spawn_config.return_value = MagicMock( - command=["claude", "--dangerously-skip-permissions"], - env={}, - cwd=Path("/tmp/myproj"), - ) - h.sandbox_binds.return_value = [] - h.parse_status = MagicMock(return_value=None) - return {"claude-code": h} - - -@pytest.fixture -def sandbox_service(factory, db, tmux, sandbox_harness): - return SessionService(factory=factory, tmux=tmux, harnesses=sandbox_harness) - - -@pytest.mark.asyncio -async def test_sandboxed_create_wraps_command(sandbox_service, db, tmux, sandbox_harness): - with patch("hqt.sessions.service.sandbox.is_available", return_value=True), patch( - "hqt.sessions.service.sandbox.wrap", return_value=["bwrap", "--", "claude"] - ) as mock_wrap: - result = await sandbox_service.create_session( - project_id=1, - harness_name="claude-code", - sandbox=SandboxPolicy(fs="rw", net=True), - ) - mock_wrap.assert_called_once() - sandbox_harness["claude-code"].build_spawn_config.assert_called_once() - # build_spawn_config was called with sandboxed=True - assert sandbox_harness["claude-code"].build_spawn_config.call_args.kwargs[ - "sandboxed" - ] is True - # tmux.spawn received the wrapped command - assert tmux.spawn.call_args.args[0].command == ["bwrap", "--", "claude"] - # policy persisted on the row - assert result.session.sandbox_json == '{"fs": "rw", "net": true}' - - -@pytest.mark.asyncio -async def test_unsandboxed_create_does_not_wrap(service, db, tmux, harnesses): - with patch("hqt.sessions.service.sandbox.wrap") as mock_wrap: - result = await service.create_session(project_id=1, harness_name="claude-code") - mock_wrap.assert_not_called() - assert result.session.sandbox_json is None - - -@pytest.mark.asyncio -async def test_sandboxed_create_raises_when_bwrap_missing(sandbox_service, db): - with patch("hqt.sessions.service.sandbox.is_available", return_value=False): - with pytest.raises(ServiceError, match="bubblewrap"): - await sandbox_service.create_session( - project_id=1, - harness_name="claude-code", - sandbox=SandboxPolicy(fs="rw", net=True), - ) -``` - -- [ ] **Step 2: Run tests to verify they fail** - -Run: `uv run pytest tests/test_sessions.py -k "sandbox" -q` -Expected: FAIL — `create_session() got an unexpected keyword argument 'sandbox'`. - -- [ ] **Step 3: Add imports and helpers to the service** - -In `src/hqt/sessions/service.py`, add near the top imports: - -```python -import json - -from hqt import sandbox -from hqt.sandbox import SandboxPolicy -``` - -Add these helpers to `SessionService` (e.g. after `_project_path`): - -```python - def _sandbox_policy(self, sess: Session) -> SandboxPolicy | None: - """Decode the persisted sandbox policy, or None if the session is plain.""" - if not sess.sandbox_json: - return None - data = json.loads(sess.sandbox_json) - return SandboxPolicy(fs=data["fs"], net=data["net"]) - - def _wrap_command( - self, - configurator: HarnessConfigurator, - command: list[str], - cwd: Path, - policy: SandboxPolicy | None, - ) -> list[str]: - """Wrap a harness command in bwrap when a policy is set. - - Raises ServiceError if a sandbox is required but bwrap is unavailable — - a backstop; the dialog already disables the toggle in that case. - """ - if policy is None: - return command - if not sandbox.is_available(): - raise ServiceError( - "bubblewrap (bwrap) is not available; cannot start a sandboxed session" - ) - return sandbox.wrap(command, cwd, policy, configurator.sandbox_binds()) -``` - -- [ ] **Step 4: Thread `sandbox` through `create_session`** - -Change the signature and the spawn-config construction in `create_session`: - -```python - async def create_session( - self, - project_id: int, - harness_name: str, - nickname: str | None = None, - model: str | None = None, - sandbox: SandboxPolicy | None = None, - ) -> "CreateSessionResult": -``` - -Inside, after computing `harness_session_id`, persist the policy, build the spawn config with `sandboxed=`, and wrap. The wrapping (and the hard "bwrap missing" failure) is delegated to `_wrap_command`, so this method never references the `sandbox` *module* directly — important because the parameter `sandbox` shadows it within this method body: - -```python - sess.sandbox_json = ( - json.dumps({"fs": sandbox.fs, "net": sandbox.net}) - if sandbox is not None - else None - ) - spawn_cfg = configurator.build_spawn_config( - project_path, harness_session_id, model, sandboxed=sandbox is not None - ) - command = self._wrap_command( - configurator, spawn_cfg.command, spawn_cfg.cwd, sandbox - ) -``` - -Then pass `command` (not `spawn_cfg.command`) into the `SpawnRequest`: - -```python - spawn_result = await self.tmux.spawn( - SpawnRequest( - window_name=sess.tmux_session_name, - command=command, - cwd=str(spawn_cfg.cwd), - env=spawn_cfg.env, - ) - ) -``` - -Why no separate availability pre-check here: `_wrap_command` already raises -`ServiceError` when a policy is set and bwrap is unavailable. Because the row is -added/flushed but **not** committed before `_wrap_command` runs, that exception -propagates out of the `with self.factory() as db:` block without a commit, so no -partial session row is persisted. Keeping the single check inside -`_wrap_command` also means it resolves the `sandbox` module in its own scope -(its only parameter is `policy`), so `patch("hqt.sessions.service.sandbox.is_available", ...)` -in tests takes effect — a module-level alias would not. - -- [ ] **Step 5: Wrap the resume and fallback paths** - -Update `_get_resume_config` to read the policy, pass `sandboxed=`, and wrap: - -```python - def _get_resume_config(self, db: DBSession, sess: Session) -> SpawnConfig: - configurator = self._configurator(sess.harness.name) - harness_session_id = ( - sess.harness_session_id or configurator.generate_session_id(sess.id) - ) - project_path = self._project_path(db, sess.project_id) - policy = self._sandbox_policy(sess) - cfg = configurator.build_resume_config( - project_path, harness_session_id, sess.model, sandboxed=policy is not None - ) - command = self._wrap_command(configurator, cfg.command, cfg.cwd, policy) - return SpawnConfig(command=command, env=cfg.env, cwd=cfg.cwd) -``` - -In `_respawn_with_fallback`, the rung-2 fresh spawn must also wrap. Replace the rung-2 block that builds `spawn_cfg` and calls `respawn_verified`: - -```python - configurator = self._configurator(sess.harness.name) - project_path = self._project_path(db, sess.project_id) - policy = self._sandbox_policy(sess) - 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, sandboxed=policy is not None - ) - command = self._wrap_command( - configurator, spawn_cfg.command, spawn_cfg.cwd, policy - ) - since = time.time() - result = await self.tmux.respawn_verified( - window_name, command, str(spawn_cfg.cwd), env=spawn_cfg.env - ) -``` - -(The rung-1 resume already goes through `_get_resume_config`, which now wraps.) - -- [ ] **Step 6: Run tests to verify they pass** - -Run: `uv run pytest tests/test_sessions.py -q` -Expected: PASS — new sandbox tests pass and the existing `test_create_session` etc. still pass (the `harnesses` fixture's `build_spawn_config` accepts the extra `sandboxed` kwarg via MagicMock, and `sandbox` defaults to `None` so `_wrap_command` returns the command unchanged). - -- [ ] **Step 7: Run the full suite + lint** - -Run: `uv run pytest -q && uv run ruff check src tests` -Expected: PASS / no lint errors. - -- [ ] **Step 8: Commit** - -```bash -git add src/hqt/sessions/service.py tests/test_sessions.py -git commit -m "feat: wrap sandboxed sessions in bwrap on spawn and resume" -``` - ---- - -## Task 6: New Session dialog — toggles, gating, and app wiring - -**Files:** -- Modify: `src/hqt/tui/screens/new_session.py` -- Modify: `src/hqt/tui/app.py:239-274` -- Test: `tests/test_tui.py` - -- [ ] **Step 1: Write the failing tests** - -```python -# tests/test_tui.py (append) -import pytest -from textual.app import App -from textual.widgets import Switch - -from hqt.sandbox import SandboxPolicy -from hqt.tui.screens.new_session import NewSessionScreen - - -def test_policy_from_disabled_returns_none(): - assert NewSessionScreen._policy_from(False, "rw", True) is None - - -def test_policy_from_enabled_builds_policy(): - p = NewSessionScreen._policy_from(True, "ro", False) - assert p == SandboxPolicy(fs="ro", net=False) - - -class _Host(App): - def __init__(self, screen: NewSessionScreen) -> None: - self._scr = screen - super().__init__() - - async def on_mount(self) -> None: - await self.push_screen(self._scr) - - -@pytest.mark.asyncio -async def test_sandbox_switch_disabled_when_unavailable(): - screen = NewSessionScreen(["claude"], sandbox_available=False) - async with _Host(screen).run_test(): - assert screen.query_one("#sandbox-switch", Switch).disabled is True - - -@pytest.mark.asyncio -async def test_sandbox_switch_enabled_when_available(): - screen = NewSessionScreen(["claude"], sandbox_available=True) - async with _Host(screen).run_test(): - assert screen.query_one("#sandbox-switch", Switch).disabled is False -``` - -- [ ] **Step 2: Run tests to verify they fail** - -Run: `uv run pytest tests/test_tui.py -k "policy_from or sandbox_switch" -q` -Expected: FAIL — `NewSessionScreen` has no `_policy_from` and its `__init__` rejects `sandbox_available`. - -- [ ] **Step 3: Rewrite the dialog** - -Replace `src/hqt/tui/screens/new_session.py` with: - -```python -from textual.app import ComposeResult -from textual.containers import Horizontal, Vertical -from textual.screen import ModalScreen -from textual.widgets import Button, Input, Label, Select, Switch - -from hqt.sandbox import SandboxPolicy - -SessionResult = tuple[str, str, str | None, SandboxPolicy | None] - - -class NewSessionScreen(ModalScreen[SessionResult | None]): - def __init__( - self, harness_names: list[str], sandbox_available: bool = False - ) -> None: - self.harness_names = harness_names - self.sandbox_available = sandbox_available - super().__init__() - - def compose(self) -> ComposeResult: - with Vertical(id="new-session-dialog"): - yield Label("New Session") - yield Label("Harness:") - yield Select( - [(n, n) for n in self.harness_names], - id="harness-select", - allow_blank=False, - ) - yield Label("Nickname:") - yield Input(placeholder="session nickname", id="nickname-input") - yield Label("Model (optional):") - yield Input(placeholder="model name", id="model-input") - with Horizontal(classes="sandbox-row"): - yield Label("Sandboxed:") - yield Switch(value=False, id="sandbox-switch", disabled=not self.sandbox_available) - if not self.sandbox_available: - yield Label( - "bubblewrap not found — run `hqt doctor`", - id="sandbox-warning", - ) - yield Label("Filesystem:") - yield Select( - [("Read-write", "rw"), ("Read-only", "ro")], - id="fs-select", - value="rw", - allow_blank=False, - ) - with Horizontal(classes="sandbox-row"): - yield Label("Network:") - yield Switch(value=True, id="net-switch") - with Horizontal(classes="dialog-actions"): - yield Button("OK", variant="primary", id="ok-btn") - yield Button("Cancel", id="cancel-btn") - - @staticmethod - def _policy_from(enabled: bool, fs: str, net: bool) -> SandboxPolicy | None: - """Pure mapping from widget values to a policy (None when disabled).""" - if not enabled: - return None - return SandboxPolicy(fs=fs, net=net) - - def on_button_pressed(self, event: Button.Pressed) -> None: - if event.button.id == "ok-btn": - self._submit() - else: - self.dismiss(None) - - def on_input_submitted(self, event: Input.Submitted) -> None: - self._submit() - - def _submit(self) -> None: - harness = self.query_one("#harness-select", Select).value - nickname = self.query_one("#nickname-input", Input).value - model = self.query_one("#model-input", Input).value or None - enabled = self.query_one("#sandbox-switch", Switch).value - fs = self.query_one("#fs-select", Select).value - net = self.query_one("#net-switch", Switch).value - policy = self._policy_from(bool(enabled), str(fs), bool(net)) - if harness and harness != Select.BLANK: - self.dismiss((str(harness), nickname, model, policy)) - else: - self.dismiss(None) -``` - -- [ ] **Step 4: Wire the app** - -In `src/hqt/tui/app.py`, add `from hqt import sandbox` to the imports, then update `action_new_session`: - -```python - def action_new_session(self) -> None: - if self._selected_project_id is None: - self.notify("Select a project first", severity="warning") - return - project_id = self._selected_project_id - harness_names = list(discover_harnesses().keys()) - if not harness_names: - self.notify("No harnesses found", severity="error") - return - - def on_dismiss(result: tuple[str, str, str | None, object] | None) -> None: - if result: - harness_name, nickname, model, sandbox_policy = result - - async def _do() -> None: - create_result = await self._sessions().create_session( - project_id, harness_name, nickname, model, sandbox=sandbox_policy - ) - if not create_result.spawn_ok: - first_line = next( - ( - ln - for ln in create_result.spawn_error.splitlines() - if ln.strip() - ), - create_result.spawn_error, - )[:120] - self.notify( - f"Session created but harness failed to start: {first_line}", - severity="error", - ) - await self._refresh_sessions() - - self._run_service_worker(_do()) - - self.push_screen( - NewSessionScreen(harness_names, sandbox_available=sandbox.is_available()), - on_dismiss, - ) -``` - -- [ ] **Step 5: Run tests to verify they pass** - -Run: `uv run pytest tests/test_tui.py -q` -Expected: PASS (new dialog tests + existing TUI tests). - -- [ ] **Step 6: Commit** - -```bash -git add src/hqt/tui/screens/new_session.py src/hqt/tui/app.py tests/test_tui.py -git commit -m "feat: sandbox toggles in New Session dialog, gated on bwrap availability" -``` - ---- - -## Task 7: `doctor` reports bubblewrap - -**Files:** -- Modify: `src/hqt/cli.py:105-122` -- Test: `tests/test_cli.py` (create if absent) - -- [ ] **Step 1: Write the failing tests** - -```python -# tests/test_cli.py -from unittest.mock import patch - -from click.testing import CliRunner - -from hqt.cli import main - - -def _which(found: set[str]): - return lambda name: f"/usr/bin/{name}" if name in found else None - - -def test_doctor_reports_bwrap_present(): - with patch("hqt.cli.shutil.which", side_effect=_which({"tmux", "bwrap"})), patch( - "hqt.harnesses.registry.shutil.which", side_effect=_which(set()) - ): - result = CliRunner().invoke(main, ["doctor"]) - assert result.exit_code == 0 - assert "bubblewrap: ✓" in result.output - - -def test_doctor_reports_bwrap_missing(): - with patch("hqt.cli.shutil.which", side_effect=_which({"tmux"})), patch( - "hqt.harnesses.registry.shutil.which", side_effect=_which(set()) - ): - result = CliRunner().invoke(main, ["doctor"]) - assert result.exit_code == 0 - assert "bubblewrap: ✗" in result.output -``` - -- [ ] **Step 2: Run tests to verify they fail** - -Run: `uv run pytest tests/test_cli.py -q` -Expected: FAIL — output contains no "bubblewrap" line. - -- [ ] **Step 3: Add the doctor check** - -In `src/hqt/cli.py`, inside `doctor()`, after the harness loop (before/after is fine), add a non-fatal bubblewrap report: - -```python - bwrap = shutil.which("bwrap") - if bwrap: - click.echo(f"bubblewrap: ✓ {bwrap}") - else: - click.echo("bubblewrap: ✗ not found — sandboxed sessions unavailable") -``` - -- [ ] **Step 4: Run tests to verify they pass** - -Run: `uv run pytest tests/test_cli.py -q` -Expected: PASS (2 passed). - -- [ ] **Step 5: Full suite + lint** - -Run: `uv run pytest -q && uv run ruff check src tests` -Expected: PASS / clean. - -- [ ] **Step 6: Commit** - -```bash -git add src/hqt/cli.py tests/test_cli.py -git commit -m "feat: doctor reports bubblewrap availability" -``` - ---- - -## Self-Review (completed by plan author) - -**Spec coverage:** -- Data model (`sandbox_json` + migration) → Task 4. -- ABC additions (skip-permission flags, `sandbox_binds`, `sandboxed` param) → Task 3. -- Bubblewrap policy module (`is_available`, `wrap`) → Tasks 1–2. -- Spawn/resume integration (create + both respawn rungs + `_get_resume_config`) → Task 5. -- UI toggles + gating → Task 6. -- doctor + availability helper as single source of truth → Tasks 1 & 7 (both use `is_available` / `shutil.which("bwrap")`). -- Failure handling (hard `ServiceError`, no silent downgrade) → Task 5 (`_wrap_command` and create-time check). -- Testing (wrap combos, configurator flags, service wrap + bwrap-missing, availability/UI gating, migration) → Tasks 1–7. - -**Network all-or-nothing & minimal-allowlist** are realized in `wrap()` (Task 2): `--share-net` only when `net`, `$HOME` not bound, system dirs read-only, explicit cwd + harness binds. - -**Placeholder scan:** none — every code step shows complete code; harness flags/paths are concrete (verify against real binaries when implementing, per the spec note). - -**Type/name consistency:** `SandboxPolicy(fs, net)`, `Bind(src, writable)`, `is_available()`, `wrap(command, cwd, policy, binds)`, `sandbox_binds()`, `sandbox_skip_permission_flags`, `_sandbox_policy`, `_wrap_command`, `_policy_from`, result tuple `(harness, nickname, model, policy)` are used identically across tasks. -``` diff --git a/docs/superpowers/plans/2026-06-10-session-attach-rename.md b/docs/superpowers/plans/2026-06-10-session-attach-rename.md deleted file mode 100644 index cac81a1..0000000 --- a/docs/superpowers/plans/2026-06-10-session-attach-rename.md +++ /dev/null @@ -1,550 +0,0 @@ -# Session Attach Simplification + Rename 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:** Remove the redundant `Shift+R` Resume binding (Enter/attach already auto-resumes) and add an `r` Rename action that edits a session's display nickname. - -**Architecture:** Resume is pure deletion — `attach_session()` already respawns dead/gone windows before attaching. Rename is a synchronous DB write to `Session.nickname`; the existing 3-second poll (`sync_window_labels`) propagates the new label to tmux. A new modal `RenameSessionScreen` mirrors the existing `ProjectFormScreen` pattern. - -**Tech Stack:** Python, Textual (TUI), SQLAlchemy, pytest + pytest-asyncio. - ---- - -## File Structure - -- **Modify** `src/hqt/sessions/service.py` — remove `resume_session()`; add `rename_session()` and `get_session()`. -- **Modify** `src/hqt/tui/app.py` — remove the Resume binding + `action_resume_session()`; add the Rename binding + `action_rename_session()`. -- **Create** `src/hqt/tui/screens/rename_session.py` — `RenameSessionScreen` modal. -- **Modify** `tests/test_tui.py` — delete the resume keypress test; add rename-modal + rename-action tests. -- **Modify** `tests/test_sessions.py` — delete four `resume_session` tests; add `rename_session`/`get_session` tests (reusing existing fixtures). - ---- - -## Task 1: Remove Resume from the service - -**Files:** -- Modify: `src/hqt/sessions/service.py:143-150` (the `resume_session` method) -- Modify: `tests/test_sessions.py` (delete four `resume_session` tests) - -- [ ] **Step 1: Inventory every reference** - -Run: `grep -rn "resume_session" src/ tests/` -Expected references: the method in `src/hqt/sessions/service.py`; `action_resume_session` in `src/hqt/tui/app.py`; `test_resume_keypress_triggers_resume` in `tests/test_tui.py`; and four tests in `tests/test_sessions.py` — `test_resume_session_passes_env`, `test_resume_session_resume_ok_attaches`, `test_resume_session_resume_fails_fallback_spawn`, `test_resume_session_both_rungs_fail_returns_false`. No other production callers. - -Coverage note: `_respawn_with_fallback`'s rung-1/rung-2 behavior is independently covered by `test_attach_session_dead_resume_ok_attaches_once`, `test_attach_session_dead_resume_fails_fallback_spawn`, and `test_attach_session_both_rungs_fail_returns_false`, so deleting the resume tests loses no fallback-ladder coverage. - -- [ ] **Step 2: Delete the four `resume_session` tests in `tests/test_sessions.py`** - -Remove these four async test functions in full (including decorators and docstrings): `test_resume_session_passes_env`, `test_resume_session_resume_ok_attaches`, `test_resume_session_resume_fails_fallback_spawn`, `test_resume_session_both_rungs_fail_returns_false`. Leave all `test_attach_session_*` tests intact. - -- [ ] **Step 3: Confirm the resume tests are gone but attach tests remain** - -Run: `grep -n "resume_session\|test_attach_session" tests/test_sessions.py` -Expected: no `resume_session` matches; the three `test_attach_session_*` tests still listed. - -- [ ] **Step 4: Delete the `resume_session` method** - -In `src/hqt/sessions/service.py`, remove this method entirely (currently lines 143-150): - -```python - async def resume_session(self, session_id: int) -> bool: - """Force-restart the harness in this session's window.""" - sess = self.db.get(Session, session_id) - window_name = sess.tmux_session_name - ok = await self._respawn_with_fallback(sess, window_name) - if not ok: - return False - return await self.tmux.attach(window_name) -``` - -Leave `_respawn_with_fallback()` and `attach_session()` untouched — attach depends on the fallback ladder. - -- [ ] **Step 5: Verify nothing in the service references the removed method** - -Run: `grep -n "resume_session" src/hqt/sessions/service.py` -Expected: no output. - -- [ ] **Step 6: Run the service tests** - -Run: `python -m pytest tests/test_sessions.py -q` -Expected: PASS — resume tests removed, attach/fallback tests still green. - -- [ ] **Step 7: Commit** - -```bash -git add src/hqt/sessions/service.py tests/test_sessions.py -git commit -m "refactor: drop redundant resume_session (attach auto-resumes)" -``` - ---- - -## Task 2: Remove Resume from the app + its test - -**Files:** -- Modify: `src/hqt/tui/app.py:62-68` (BINDINGS) and `src/hqt/tui/app.py:235-243` (`action_resume_session`) -- Modify: `tests/test_tui.py:822-871` (delete `test_resume_keypress_triggers_resume`) - -- [ ] **Step 1: Delete the resume test first** - -In `tests/test_tui.py`, remove the section header comment block and the entire `test_resume_keypress_triggers_resume` test (currently lines 822-871), including its `@pytest.mark.parametrize` decorator. - -- [ ] **Step 2: Run the suite to confirm only the deleted test is gone** - -Run: `python -m pytest tests/test_tui.py -q` -Expected: PASS — the resume test no longer collected; the action still exists so nothing else breaks yet. - -- [ ] **Step 3: Remove the Resume binding** - -In `src/hqt/tui/app.py`, delete these lines from `BINDINGS` (currently lines 63-66): - -```python - # A normal terminal sends the character "R" for Shift+R; only the Kitty - # keyboard protocol emits "shift+r". Bind both so Resume fires either way - # (binding "shift+r" alone never matches on a standard terminal). - Binding("R,shift+r", "resume_session", "Resume"), -``` - -- [ ] **Step 4: Remove `action_resume_session`** - -In `src/hqt/tui/app.py`, delete this method (currently lines 235-243): - -```python - def action_resume_session(self) -> None: - sid = self.query_one(SessionList).get_selected_session_id() - if sid: - async def _do() -> None: - ok = await self._session_service.resume_session(sid) - if not ok: - self.notify("Failed to resume session", severity="error") - await self._refresh_sessions() - self.run_worker(_do()) -``` - -- [ ] **Step 5: Confirm no lingering references** - -Run: `grep -rn "resume_session\|Resume" src/hqt/tui/app.py` -Expected: no output. - -- [ ] **Step 6: Run the full suite** - -Run: `python -m pytest tests/test_tui.py -q` -Expected: PASS. - -- [ ] **Step 7: Commit** - -```bash -git add src/hqt/tui/app.py tests/test_tui.py -git commit -m "refactor: remove Shift+R Resume binding and action" -``` - ---- - -## Task 3: Add `rename_session` + `get_session` to the service - -**Files:** -- Modify: `src/hqt/sessions/service.py` (add two methods to `SessionService`) -- Test: `tests/test_sessions.py` - -- [ ] **Step 1: Write the failing tests** - -`tests/test_sessions.py` already exists with shared `db`, `tmux`, `harnesses`, and `service` fixtures (the `db` fixture seeds one `Harness(name="claude-code")` and one `Project` with id 1). Reuse them — do NOT add new setup helpers. Append a small builder and the tests: - -```python -def _seed_session(db, nickname=None): - """Create a session for project 1 / harness 'claude-code' (seeded by the db fixture).""" - from hqt.db.models import Harness, Session - - harness = db.query(Harness).filter_by(name="claude-code").first() - sess = Session( - project_id=1, - harness_id=harness.id, - nickname=nickname, - tmux_session_name="hqt-rename", - archived=False, - ) - db.add(sess) - db.commit() - return sess - - -def test_rename_session_sets_nickname(service, db): - from hqt.db.models import Session - - sess = _seed_session(db, nickname="old") - service.rename_session(sess.id, "new-name") - assert db.get(Session, sess.id).nickname == "new-name" - - -def test_rename_session_empty_clears_to_none(service, db): - from hqt.db.models import Session - - sess = _seed_session(db, nickname="old") - service.rename_session(sess.id, "") - assert db.get(Session, sess.id).nickname is None - - -def test_rename_session_whitespace_clears_to_none(service, db): - from hqt.db.models import Session - - sess = _seed_session(db, nickname="old") - service.rename_session(sess.id, " ") - assert db.get(Session, sess.id).nickname is None - - -def test_get_session_returns_row(service, db): - sess = _seed_session(db, nickname="x") - assert service.get_session(sess.id).id == sess.id - - -def test_get_session_missing_returns_none(service): - assert service.get_session(99999) is None -``` - -- [ ] **Step 2: Run the tests to verify they fail** - -Run: `python -m pytest tests/test_sessions.py -k "rename_session or get_session" -q` -Expected: FAIL with `AttributeError: 'SessionService' object has no attribute 'rename_session'` (and `get_session`). - -- [ ] **Step 3: Implement both methods** - -In `src/hqt/sessions/service.py`, add these methods to `SessionService` (place them next to `delete_session`): - -```python - def get_session(self, session_id: int) -> Session | None: - """Return the session row, or None if it does not exist.""" - return self.db.get(Session, session_id) - - def rename_session(self, session_id: int, nickname: str | None) -> None: - """Update a session's display nickname. - - An empty/blank nickname clears it to None, so the label falls back to - the tmux window name. The tmux window label is refreshed by the next - poll (sync_window_labels), so no immediate tmux call is needed here. - """ - sess = self.db.get(Session, session_id) - sess.nickname = (nickname or "").strip() or None - self.db.commit() -``` - -- [ ] **Step 4: Run the tests to verify they pass** - -Run: `python -m pytest tests/test_sessions.py -k "rename_session or get_session" -q` -Expected: PASS (5 tests). - -- [ ] **Step 5: Commit** - -```bash -git add src/hqt/sessions/service.py tests/test_sessions.py -git commit -m "feat: add rename_session and get_session to SessionService" -``` - ---- - -## Task 4: Create the RenameSessionScreen modal - -**Files:** -- Create: `src/hqt/tui/screens/rename_session.py` -- Test: `tests/test_tui.py` (append) - -- [ ] **Step 1: Write the failing tests** - -Append to `tests/test_tui.py`: - -```python -# --------------------------------------------------------------------------- -# RenameSessionScreen: prefill + submit -# --------------------------------------------------------------------------- - - -@pytest.mark.asyncio -async def test_rename_screen_prefills_current_nickname(): - app = HqtApp() - async with app.run_test(size=(120, 40)) as pilot: - from hqt.tui.screens.rename_session import RenameSessionScreen - from textual.widgets import Input - - app.push_screen(RenameSessionScreen(initial_nickname="mywork")) - await pilot.pause() - assert app.screen.query_one("#nickname-input", Input).value == "mywork" - - -@pytest.mark.asyncio -async def test_rename_screen_ok_returns_stripped_value(): - app = HqtApp() - async with app.run_test(size=(120, 40)) as pilot: - from hqt.tui.screens.rename_session import RenameSessionScreen - from textual.widgets import Button, Input - - results = [] - app.push_screen(RenameSessionScreen(initial_nickname=""), results.append) - await pilot.pause() - app.screen.query_one("#nickname-input", Input).value = " renamed " - app.screen.query_one("#ok-btn", Button).press() - await pilot.pause() - assert results == ["renamed"] - - -@pytest.mark.asyncio -async def test_rename_screen_cancel_returns_none(): - app = HqtApp() - async with app.run_test(size=(120, 40)) as pilot: - from hqt.tui.screens.rename_session import RenameSessionScreen - from textual.widgets import Button - - results = [] - app.push_screen(RenameSessionScreen(initial_nickname="x"), results.append) - await pilot.pause() - app.screen.query_one("#cancel-btn", Button).press() - await pilot.pause() - assert results == [None] -``` - -- [ ] **Step 2: Run the tests to verify they fail** - -Run: `python -m pytest tests/test_tui.py -k rename_screen -q` -Expected: FAIL with `ModuleNotFoundError: No module named 'hqt.tui.screens.rename_session'`. - -- [ ] **Step 3: Implement the modal** - -Create `src/hqt/tui/screens/rename_session.py`: - -```python -from textual.app import ComposeResult -from textual.containers import Horizontal, Vertical -from textual.screen import ModalScreen -from textual.widgets import Button, Input, Label - - -class RenameSessionScreen(ModalScreen[str | None]): - """Rename a session's display nickname. - - Dismisses with the stripped nickname on OK, or None on Cancel. Mirrors - ProjectFormScreen's structure so it picks up the same dialog styling - (#project-form-dialog rules are reused via the shared id). - """ - - def __init__(self, initial_nickname: str = "") -> None: - self._initial_nickname = initial_nickname - super().__init__() - - def compose(self) -> ComposeResult: - with Vertical(id="project-form-dialog"): - yield Label("Rename Session") - yield Label("Name:") - yield Input( - value=self._initial_nickname, - placeholder="session name", - id="nickname-input", - ) - with Horizontal(classes="dialog-actions"): - yield Button("OK", variant="primary", id="ok-btn") - yield Button("Cancel", id="cancel-btn") - - def on_button_pressed(self, event: Button.Pressed) -> None: - if event.button.id == "ok-btn": - self.dismiss(self.query_one("#nickname-input", Input).value.strip()) - else: - self.dismiss(None) - - def on_input_submitted(self, event: Input.Submitted) -> None: - # Enter in the text field confirms, matching the New Session dialog. - self.dismiss(self.query_one("#nickname-input", Input).value.strip()) -``` - -Note: reusing `id="project-form-dialog"` is intentional — it inherits the existing dialog styling in `styles.tcss` (width 60, border, `$surface` fill) verified by `test_project_form_dialog_styled`. No new CSS needed. - -- [ ] **Step 4: Run the tests to verify they pass** - -Run: `python -m pytest tests/test_tui.py -k rename_screen -q` -Expected: PASS (3 tests). - -- [ ] **Step 5: Commit** - -```bash -git add src/hqt/tui/screens/rename_session.py tests/test_tui.py -git commit -m "feat: add RenameSessionScreen modal" -``` - ---- - -## Task 5: Wire the `r` Rename binding + action into the app - -**Files:** -- Modify: `src/hqt/tui/app.py` (BINDINGS, import, new action) -- Test: `tests/test_tui.py` (append) - -- [ ] **Step 1: Write the failing tests** - -Append to `tests/test_tui.py`: - -```python -# --------------------------------------------------------------------------- -# Rename session: binding + action -# --------------------------------------------------------------------------- - - -@pytest.mark.asyncio -async def test_rename_session_binding_exists(): - bindings = {b.key: b for b in HqtApp.BINDINGS} - assert "r" in bindings - assert bindings["r"].action == "rename_session" - - -@pytest.mark.asyncio -async def test_rename_session_no_selection_warns(): - app = HqtApp() - async with app.run_test(size=(120, 40)) as pilot: - from unittest.mock import patch - - notify_calls = [] - with patch.object( - app, "notify", side_effect=lambda msg, **kw: notify_calls.append((msg, kw)) - ): - await app.run_action("rename_session") - await pilot.pause() - - assert any( - kw.get("severity") == "warning" for _, kw in notify_calls - ), f"Expected warning notification, got: {notify_calls}" - - -@pytest.mark.asyncio -async def test_rename_session_opens_prefilled_and_updates(): - app = HqtApp() - async with app.run_test(size=(120, 40)) as pilot: - from hqt.db.models import Project, Harness, Session - from hqt.tui.widgets.session_list import SessionList - from hqt.tui.screens.rename_session import RenameSessionScreen - from textual.widgets import Input - - proj = Project(name="ren-proj", path="/tmp/ren-proj") - app._db_session.add(proj) - app._db_session.flush() - harness = app._db_session.query(Harness).first() - sess = Session( - project_id=proj.id, - harness_id=harness.id, - nickname="before", - tmux_session_name="hqt-ren", - archived=False, - ) - app._db_session.add(sess) - app._db_session.commit() - - app._selected_project_id = proj.id - await app._refresh_sessions() - await pilot.pause() - - sl = app.query_one(SessionList) - sl.query_one("#session-list").focus() - await pilot.press("down") - await pilot.pause() - assert sl.get_selected_session_id() == sess.id - - await app.run_action("rename_session") - await pilot.pause() - - assert isinstance(app.screen, RenameSessionScreen) - assert app.screen.query_one("#nickname-input", Input).value == "before" - - app.screen.dismiss("after") - await pilot.pause() - await app.workers.wait_for_complete() - await pilot.pause() - - assert app._db_session.get(Session, sess.id).nickname == "after" -``` - -- [ ] **Step 2: Run the tests to verify they fail** - -Run: `python -m pytest tests/test_tui.py -k rename_session -q` -Expected: FAIL — `"r"` not in BINDINGS / no `rename_session` action. - -- [ ] **Step 3: Add the import** - -In `src/hqt/tui/app.py`, add next to the other screen imports (near line 19): - -```python -from hqt.tui.screens.rename_session import RenameSessionScreen -``` - -- [ ] **Step 4: Add the binding** - -In `src/hqt/tui/app.py` `BINDINGS`, add this entry (e.g. after the `delete_session` binding): - -```python - Binding("r", "rename_session", "Rename"), -``` - -- [ ] **Step 5: Add the action** - -In `src/hqt/tui/app.py`, add this method (e.g. after `action_attach_session`): - -```python - def action_rename_session(self) -> None: - sid = self.query_one(SessionList).get_selected_session_id() - if sid is None: - self.notify("Select a session first", severity="warning") - return - sess = self._session_service.get_session(sid) - if sess is None: - self.notify("Session not found", severity="error") - return - - def on_dismiss(result: str | None) -> None: - if result is not None: - self._session_service.rename_session(sid, result) - self.run_worker(self._refresh_sessions()) - - self.push_screen( - RenameSessionScreen(initial_nickname=sess.nickname or ""), - on_dismiss, - ) -``` - -- [ ] **Step 6: Run the tests to verify they pass** - -Run: `python -m pytest tests/test_tui.py -k rename_session -q` -Expected: PASS (3 tests). - -- [ ] **Step 7: Commit** - -```bash -git add src/hqt/tui/app.py tests/test_tui.py -git commit -m "feat: add 'r' rename-session binding and action" -``` - ---- - -## Task 6: Full verification + docs - -**Files:** -- Modify: `README.md` (if it documents keybindings — check first) - -- [ ] **Step 1: Run the entire test suite** - -Run: `python -m pytest -q` -Expected: PASS, no failures, no reference to resume. - -- [ ] **Step 2: Update keybinding docs if present** - -Run: `grep -rn "Resume\|Shift+R\|shift+r" README.md` -If matches exist, replace the Resume entry with a Rename (`r`) entry and note that attach (Enter) now auto-resumes. If no matches, skip. - -- [ ] **Step 3: Commit any doc change** - -```bash -git add README.md -git commit -m "docs: replace Resume keybinding with Rename ('r')" -``` - -(Skip this commit if README needed no change.) - ---- - -## Self-Review Notes - -- **Spec coverage:** Change 1 (remove Resume) → Tasks 1-2. Change 2 (rename: service, modal, wiring) → Tasks 3-5. Testing section → tests embedded in every task + Task 6. Out-of-scope items (no `tmux_session_name` change, no migration) are respected — only `nickname` is written. -- **Empty-input → None:** handled in `rename_session` (`(nickname or "").strip() or None`) and covered by `test_rename_session_empty_clears_to_none`. -- **Type consistency:** `get_session(session_id) -> Session | None`, `rename_session(session_id, nickname)`, and `RenameSessionScreen(initial_nickname=...)` are used identically across the service, app action, and tests. -- **Label propagation:** no immediate tmux call; the existing `sync_window_labels` poll (app.py `set_interval(3, ...)`) reads the new nickname — consistent with the approved design. diff --git a/docs/superpowers/plans/2026-06-10-tool-windows.md b/docs/superpowers/plans/2026-06-10-tool-windows.md deleted file mode 100644 index a10dd42..0000000 --- a/docs/superpowers/plans/2026-06-10-tool-windows.md +++ /dev/null @@ -1,961 +0,0 @@ -# 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/plans/2026-06-10-worktree-sessions-plan.md b/docs/superpowers/plans/2026-06-10-worktree-sessions-plan.md deleted file mode 100644 index 6efb158..0000000 --- a/docs/superpowers/plans/2026-06-10-worktree-sessions-plan.md +++ /dev/null @@ -1,216 +0,0 @@ -# Worktree-Isolated Sessions — Implementation Plan - -Spec: docs/superpowers/specs/2026-06-10-worktree-sessions-design.md -Branch: worktree-sessions - -Quality gates after every task (must pass before commit): - -``` -uv run ruff format src tests -uv run ruff check src tests -uv run ty check -uv run pytest -q -``` - -Do NOT stage `src/hqt/tmux/runner.py` — it carries an unrelated uncommitted -user change. Stage only files you created/modified for your task. - -## Task 1 — Git worktree module - -Create `src/hqt/git/__init__.py` and `src/hqt/git/worktree.py`, plus -`tests/test_worktree.py`. - -Module contents (async functions use `asyncio.create_subprocess_exec`, in the -style of `src/hqt/tmux/runner.py`; failures raise `hqt.errors.ServiceError` -with git's stderr included): - -- `slugify(text: str) -> str` — lowercase; keep `[a-z0-9]`; collapse runs of - anything else into single `-`; strip leading/trailing `-`. Pure function. -- `async is_git_repo(path: Path) -> bool` — `git rev-parse - --is-inside-work-tree` in `path`; False on any failure (including missing - directory). -- `WorktreeState` dataclass: `exists: bool`, `dirty: bool`, - `unique_commits: int`. -- `async create_worktree(repo: Path, branch: str) -> Path`: - 1. Validate with `git check-ref-format --branch ` → ServiceError - "Invalid branch name: ..." on failure. - 2. Resolve git common dir (`git rev-parse --git-common-dir`, relative - results resolved against `repo`); append line `.worktrees/` to - `/info/exclude` if not already present (create file if - missing). - 3. `git worktree add -b /.worktrees/` (cwd=repo). - ServiceError with stderr on failure (branch exists, dir exists, etc.). - 4. Return the absolute worktree path. -- `async add_worktree_for_branch(repo: Path, branch: str) -> Path` — - `git worktree add /.worktrees/ ` (no `-b`), same - exclude handling, for recreating a vanished worktree. -- `async worktree_state(repo: Path, path: Path, branch: str) -> - WorktreeState` — `exists` = path exists on disk; `dirty` = non-empty - `git status --porcelain` run in the worktree (False if not exists); - `unique_commits` = line count of `git rev-list --not --all - --count`-style query. Use `git rev-list --count --not - --exclude=refs/heads/ --all`... simplest correct form: - `git for-each-ref --format='%(refname)' refs/heads refs/remotes`, drop the - branch's own ref, then `git rev-list --count --not ` - (0 when no other refs exist is acceptable only if branch tip is reachable - from them; with no other refs, count commits on branch). Implement and test - against a real repo; exact plumbing is the implementer's choice as long as - the tests below pass. -- `async remove_worktree(repo: Path, path: Path, branch: str, force: bool) - -> None` — `git worktree remove [--force] ` (ServiceError on - failure), then best-effort `git branch -d ` (ignore failure), then - `git worktree prune`. - -Tests (real git in `tmp_path` repos; configure user.name/email; create an -initial commit): - -- slugify: mixed case, spaces, punctuation, leading/trailing junk, empty. -- is_git_repo: repo → True; plain dir → False; missing dir → False. -- create_worktree: creates `.worktrees/` with branch checked out; - `.git/info/exclude` contains `.worktrees/`; calling again with same branch - raises ServiceError; invalid branch name (`foo..bar`) raises ServiceError; - exclude not duplicated after two different worktrees. -- worktree_state: fresh worktree → exists, not dirty, 0 unique commits; - touch a file → dirty; commit on the branch → unique_commits == 1; after - removing dir manually → exists False. -- remove_worktree: clean worktree removed, branch (merged/no unique commits) - deleted; dirty worktree without force → ServiceError and dir remains; with - force → removed; unmerged branch survives removal (`git branch -d` fails - silently, branch still listed). - -Commit message: `feat: git worktree module`. - -## Task 2 — Session columns + migration - -Modify `src/hqt/db/models.py` and `src/hqt/db/migrations.py`; extend -`tests/test_db.py`. - -- `Session` gains `worktree_path: Mapped[str | None] = - mapped_column(default=None)` and `worktree_branch: Mapped[str | None] = - mapped_column(default=None)`. -- Add migration entry `(2, ...)` to `MIGRATIONS` executing two - `ALTER TABLE sessions ADD COLUMN` statements (worktree_path TEXT, - worktree_branch TEXT). `LATEST_VERSION` derives automatically. -- Tests: fresh DB has the columns and user_version 2; a DB created at - baseline (simulate: create tables via models without the new columns is - impractical — instead create a v1 DB by issuing the old CREATE TABLE - through exec_driver_sql or by building metadata then dropping the two - columns is messy; acceptable approach: stamp an existing schema to - user_version 1 after removing the columns via SQLite `ALTER TABLE ... DROP - COLUMN`, then run `migrate()` and assert columns exist and version == 2). - Follow the existing migration-test patterns in `tests/test_db.py` if any. - -Commit message: `feat: worktree columns on sessions + migration v2`. - -## Task 3 — SessionService worktree support - -Modify `src/hqt/sessions/service.py`; extend `tests/test_sessions.py` / -`tests/test_services.py` (match where SessionService is currently tested). - -- Import the worktree module as a module (`from hqt.git import worktree`) - so tests can monkeypatch `worktree.create_worktree` etc. -- `create_session(self, project_id, harness_name, nickname=None, model=None, - worktree_branch=None)`: - - When `worktree_branch` is falsy → behavior identical to today. - - When set: after resolving `project_path`, `await - worktree.create_worktree(project_path, worktree_branch)` BEFORE adding - the Session row; on ServiceError let it propagate (no row, no spawn). - Store `worktree_path=str(path)` and `worktree_branch` on the row. Spawn - config must be built with the worktree path, and the capture-retry loop - must also use the worktree path. - - Default nickname: if `worktree_branch` set and nickname blank, nickname - becomes the branch name. -- Add `_session_path(self, db, sess) -> Path`: `Path(sess.worktree_path)` - when set, else `self._project_path(db, sess.project_id)`. Use it in - `_respawn_with_fallback`, `_get_resume_config`, and - `_maybe_capture_missing_session_id` (replace direct project-path lookups - for harness-facing paths; `_maybe_capture_missing_session_id` keeps its - "project deleted → skip" behavior for non-worktree sessions, and for - worktree sessions uses the worktree path without needing the project row). -- `attach_session` rung 0: before the alive check... no — keep order: if - session has `worktree_path` and the path does not exist on disk: - - branch exists (`git rev-parse --verify refs/heads/` helper in - worktree module is NOT needed; use `worktree.add_worktree_for_branch` - and catch ServiceError) → recreate, then continue normal flow. - - recreate fails → raise ServiceError("Worktree and branch are gone; - delete the session"). -- `delete_session(self, session_id, remove_worktree=False, force=False)`: - - Kill tmux window as today. - - If `remove_worktree` and session has worktree: `await - worktree.remove_worktree(project_path, Path(sess.worktree_path), - sess.worktree_branch, force)`. On ServiceError: do NOT delete the DB row; - re-raise (the TUI shows the notification; the tmux window being already - killed is acceptable). - - Then delete row + commit. -- Expose `async worktree_state_for(self, session_id) -> WorktreeState | None` - returning None for non-worktree sessions; the TUI uses it to build the - confirm dialog. - -Tests (fake tmux pattern as existing tests; monkeypatch worktree functions -with AsyncMock/recording fakes): - -- create_session with worktree_branch: create_worktree called with project - path and branch; spawn cwd == worktree path; capture uses worktree path; - row has worktree fields; nickname defaults to branch. -- create_worktree raising → ServiceError propagates, no Session row exists, - no spawn attempted. -- attach with worktree_path missing on disk → add_worktree_for_branch - called; on its failure → ServiceError with the delete-hint message. -- resume config for a worktree session uses worktree path. -- delete_session remove_worktree=True calls worktree.remove_worktree and - deletes row; worktree.remove_worktree raising → row still present. -- worktree_state_for: None for plain sessions; delegates for worktree ones. - -Commit message: `feat: worktree-aware session lifecycle in SessionService`. - -## Task 4 — TUI wiring + docs - -Modify `src/hqt/tui/screens/new_session.py`, add -`src/hqt/tui/screens/confirm_delete.py`, modify `src/hqt/tui/app.py`, -`src/hqt/tui/widgets/session_list.py`, `src/hqt/status.py` callers if -needed, `README.md`; extend `tests/test_tui.py`. - -- `NewSessionScreen(harness_names, is_git_repo)`: - - Result type becomes `tuple[str, str, str | None, str | None] | None` - (harness, nickname, model, worktree_branch). - - Add `Checkbox("Isolate in worktree", id="worktree-checkbox")` (textual - `Checkbox`); when `is_git_repo` is False, `disabled=True` and label - "Isolate in worktree (not a git repo)". - - Branch `Input(id="branch-input")` hidden (`display = False`) until the - checkbox is checked; when first revealed, prefill with - `slugify(nickname)` if the field is empty; user edits freely afterwards. - - On submit with checkbox checked: empty branch → block submit and focus - the branch field (or fall back to slugified nickname if non-empty; - if both empty, keep dialog open). Unchecked → worktree_branch None. -- `ConfirmDeleteScreen(ModalScreen[tuple[bool, bool] | None])` in new file: - constructor takes `session_label: str`, `warning: str | None`. Shows - "Delete session