feat: stop and rename a session from its tmux window
Add "stop" and "rename" to the Alt+p tool palette so a session can be managed from inside its own harness window, mirroring the TUI's s/r keys: - stop kills the harness window (stop_session_for_window). - rename prompts for a nickname, pre-filled with the current one. The name is read from /dev/tty via readline (the fzf pipe leaves our stdin spent) and never passes through tmux's command parser, so there's nothing to quote-escape. Extract require_session_id_for_window as the shared resolve-or-raise guard behind the tool/stop/rename branches, dropping the duplicated "not an hqt session window" check. Also removes the now-shipped feature plans and specs under docs/. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -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"
|
||||
```
|
||||
File diff suppressed because it is too large
Load Diff
@@ -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 <noreply@anthropic.com>
|
||||
```
|
||||
- **NEVER `git add -A` or `git add .`** — `src/hqt/tmux/runner.py` carries unrelated uncommitted WIP that must stay out of every commit. Always `git add` the exact files listed in the task. No task in this plan modifies `runner.py`.
|
||||
- Work on `main`, committing directly to it.
|
||||
|
||||
---
|
||||
|
||||
## File Structure
|
||||
|
||||
| File | Responsibility | Tasks |
|
||||
|------|----------------|-------|
|
||||
| `src/hqt/projects/service.py` | Project CRUD; add path normalization/validation | 1 |
|
||||
| `tests/test_services.py` | ProjectService tests; add path-validation cases | 1 |
|
||||
| `src/hqt/harnesses/configurators/codex.py` | Codex capture; refuse ambiguous matches | 2 |
|
||||
| `tests/test_harnesses.py` | Codex capture tests; replace earliest-wins cases with ambiguity case | 2 |
|
||||
| `src/hqt/sessions/service.py` | Session lifecycle; serialize spawn→capture with a lock | 3 |
|
||||
| `tests/test_sessions.py` | SessionService tests; assert lock behavior | 3 |
|
||||
| `tests/test_tmux_smoke.py` (new) | Real-tmux smoke tests on an isolated server | 4 |
|
||||
| `pyproject.toml` | Register the `tmux` pytest marker | 4 |
|
||||
| `TODO.md` | Remove the three resolved P2 lines | 5 |
|
||||
|
||||
---
|
||||
|
||||
## Task 1: Project path validation
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/hqt/projects/service.py`
|
||||
- Test: `tests/test_services.py`
|
||||
|
||||
- [ ] **Step 1: Write the failing tests**
|
||||
|
||||
Add these tests to the `TestProjectService` class in `tests/test_services.py`. They need a real directory, so they use pytest's `tmp_path` fixture.
|
||||
|
||||
```python
|
||||
def test_create_rejects_nonexistent_path(self, factory, tmp_path):
|
||||
svc = ProjectService(factory)
|
||||
missing = tmp_path / "does-not-exist"
|
||||
with pytest.raises(ServiceError, match="does not exist or is not a directory"):
|
||||
svc.create("p", str(missing))
|
||||
|
||||
def test_create_rejects_file_path(self, factory, tmp_path):
|
||||
svc = ProjectService(factory)
|
||||
a_file = tmp_path / "afile"
|
||||
a_file.write_text("x")
|
||||
with pytest.raises(ServiceError, match="does not exist or is not a directory"):
|
||||
svc.create("p", str(a_file))
|
||||
|
||||
def test_create_stores_resolved_absolute_path(self, factory, tmp_path):
|
||||
svc = ProjectService(factory)
|
||||
p = svc.create("p", str(tmp_path))
|
||||
assert p.path == str(tmp_path.resolve())
|
||||
|
||||
def test_create_expands_user_home(self, factory, tmp_path, monkeypatch):
|
||||
# Point ~ at tmp_path; "~" must expand to the existing tmp_path dir.
|
||||
monkeypatch.setenv("HOME", str(tmp_path))
|
||||
svc = ProjectService(factory)
|
||||
p = svc.create("p", "~")
|
||||
assert p.path == str(tmp_path.resolve())
|
||||
|
||||
def test_update_rejects_nonexistent_path_and_leaves_row(self, factory, tmp_path):
|
||||
svc = ProjectService(factory)
|
||||
p = svc.create("p", str(tmp_path))
|
||||
missing = tmp_path / "nope"
|
||||
with pytest.raises(ServiceError, match="does not exist or is not a directory"):
|
||||
svc.update(p.id, "p", str(missing))
|
||||
assert svc.get(p.id).path == str(tmp_path.resolve())
|
||||
```
|
||||
|
||||
Several existing tests pass made-up paths like `/tmp/test`, `/a`, `/old/path` that are not guaranteed to exist. Update those literals to real directories so they keep passing. Replace the existing `TestProjectService` methods `test_create_and_get`, `test_list_excludes_archived`, `test_update_name_and_path`, `test_update_duplicate_path_raises`, and `test_create_duplicate_path_raises_service_error` to seed paths from `tmp_path`:
|
||||
|
||||
```python
|
||||
def test_create_and_get(self, factory, tmp_path):
|
||||
svc = ProjectService(factory)
|
||||
p = svc.create("test", str(tmp_path))
|
||||
assert p.id is not None
|
||||
assert svc.get(p.id).name == "test"
|
||||
|
||||
def test_list_excludes_archived(self, factory, tmp_path):
|
||||
a = tmp_path / "a"
|
||||
a.mkdir()
|
||||
b = tmp_path / "b"
|
||||
b.mkdir()
|
||||
svc = ProjectService(factory)
|
||||
svc.create("a", str(a))
|
||||
p2 = svc.create("b", str(b))
|
||||
svc.archive(p2.id)
|
||||
assert len(svc.list_all()) == 1
|
||||
assert len(svc.list_all(include_archived=True)) == 2
|
||||
|
||||
def test_update_name_and_path(self, factory, tmp_path):
|
||||
old = tmp_path / "old"
|
||||
old.mkdir()
|
||||
new = tmp_path / "new"
|
||||
new.mkdir()
|
||||
svc = ProjectService(factory)
|
||||
p = svc.create("old", str(old))
|
||||
updated = svc.update(p.id, "new", str(new))
|
||||
assert updated.name == "new"
|
||||
assert updated.path == str(new.resolve())
|
||||
assert svc.get(p.id).path == str(new.resolve())
|
||||
|
||||
def test_update_duplicate_path_raises(self, factory, tmp_path):
|
||||
a = tmp_path / "a"
|
||||
a.mkdir()
|
||||
b = tmp_path / "b"
|
||||
b.mkdir()
|
||||
svc = ProjectService(factory)
|
||||
svc.create("a", str(a))
|
||||
p2 = svc.create("b", str(b))
|
||||
with pytest.raises(ServiceError, match="already uses path"):
|
||||
svc.update(p2.id, "b", str(a))
|
||||
# DB session must remain usable after rollback, values unchanged
|
||||
assert svc.get(p2.id).path == str(b.resolve())
|
||||
assert svc.get(p2.id).name == "b"
|
||||
|
||||
def test_create_duplicate_path_raises_service_error(self, factory, tmp_path):
|
||||
dup = tmp_path / "dup"
|
||||
dup.mkdir()
|
||||
svc = ProjectService(factory)
|
||||
svc.create("a", str(dup))
|
||||
with pytest.raises(ServiceError, match="already uses path"):
|
||||
svc.create("b", str(dup))
|
||||
```
|
||||
|
||||
Note: `TestMcpService.test_bind_unbind` creates a project at `/proj`; change that literal to `str(tmp_path)` and add `tmp_path` to its signature:
|
||||
|
||||
```python
|
||||
def test_bind_unbind(self, factory, tmp_path):
|
||||
psvc = ProjectService(factory)
|
||||
msvc = McpService(factory)
|
||||
project = psvc.create("proj", str(tmp_path))
|
||||
server = msvc.create("s1", "stdio", command="cmd")
|
||||
msvc.bind_to_project(project.id, server.id)
|
||||
assert len(msvc.get_project_mcps(project.id)) == 1
|
||||
msvc.unbind_from_project(project.id, server.id)
|
||||
assert len(msvc.get_project_mcps(project.id)) == 0
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run the new tests to verify they fail**
|
||||
|
||||
Run: `uv run pytest tests/test_services.py -k "rejects or resolved or expands_user" -v`
|
||||
Expected: FAIL — `ServiceError` not raised / path not resolved (validation not implemented yet).
|
||||
|
||||
- [ ] **Step 3: Implement `_normalize_path` and call it from `create`/`update`**
|
||||
|
||||
In `src/hqt/projects/service.py`, add the `Path` import and the helper, and call it at the top of `create` and `update`. Full file:
|
||||
|
||||
```python
|
||||
from pathlib import Path
|
||||
|
||||
from sqlalchemy.exc import IntegrityError
|
||||
from sqlalchemy.orm import sessionmaker
|
||||
|
||||
from hqt.db.models import Project
|
||||
from hqt.errors import ServiceError
|
||||
|
||||
|
||||
class ProjectService:
|
||||
def __init__(self, factory: sessionmaker):
|
||||
# factory must be created with expire_on_commit=False so rows returned
|
||||
# from these methods stay readable after their session closes.
|
||||
self.factory = factory
|
||||
|
||||
def _normalize_path(self, raw: str) -> str:
|
||||
"""Expand ~, require an existing directory, return the resolved absolute path.
|
||||
|
||||
Raises ServiceError if the path does not exist or is not a directory, so
|
||||
the TUI surfaces it as a notification instead of letting the bad path
|
||||
become a dead session later.
|
||||
"""
|
||||
path = Path(raw).expanduser()
|
||||
if not path.is_dir():
|
||||
raise ServiceError(f"Path does not exist or is not a directory: {raw}")
|
||||
return str(path.resolve())
|
||||
|
||||
def create(self, name: str, path: str) -> Project:
|
||||
path = self._normalize_path(path)
|
||||
with self.factory() as db:
|
||||
project = Project(name=name, path=path)
|
||||
db.add(project)
|
||||
try:
|
||||
db.commit()
|
||||
except IntegrityError as err:
|
||||
db.rollback()
|
||||
raise ServiceError(f"Another project already uses path {path}") from err
|
||||
return project
|
||||
|
||||
def list_all(self, include_archived: bool = False) -> list[Project]:
|
||||
with self.factory() as db:
|
||||
q = db.query(Project)
|
||||
if not include_archived:
|
||||
q = q.filter_by(archived=False)
|
||||
return list(q.all())
|
||||
|
||||
def get(self, project_id: int) -> Project | None:
|
||||
with self.factory() as db:
|
||||
return db.get(Project, project_id)
|
||||
|
||||
def update(self, project_id: int, name: str, path: str) -> Project:
|
||||
path = self._normalize_path(path)
|
||||
with self.factory() as db:
|
||||
project = db.get(Project, project_id)
|
||||
if project is None:
|
||||
raise ServiceError(f"Project {project_id} not found")
|
||||
project.name = name
|
||||
project.path = path
|
||||
try:
|
||||
db.commit()
|
||||
except IntegrityError as err:
|
||||
db.rollback()
|
||||
raise ServiceError(f"Another project already uses path {path}") from err
|
||||
return project
|
||||
|
||||
def archive(self, project_id: int) -> None:
|
||||
with self.factory() as db:
|
||||
project = db.get(Project, project_id)
|
||||
if project:
|
||||
project.archived = True
|
||||
db.commit()
|
||||
```
|
||||
|
||||
Note ordering: `_normalize_path` runs **before** opening the DB session, so a bad path is rejected without touching the DB. `update` validates the path before checking the project exists — that's fine; both raise `ServiceError`.
|
||||
|
||||
- [ ] **Step 4: Run the full service test file**
|
||||
|
||||
Run: `uv run pytest tests/test_services.py -v`
|
||||
Expected: PASS (new validation tests + all updated existing tests).
|
||||
|
||||
- [ ] **Step 5: Run quality gates**
|
||||
|
||||
Run: `uv run ruff format src tests && uv run ruff check src tests && uv run ty check`
|
||||
Expected: all green.
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add src/hqt/projects/service.py tests/test_services.py
|
||||
git commit -m "feat: validate project paths at create/update
|
||||
|
||||
Reject non-existent or non-directory paths with ServiceError (already
|
||||
surfaced as a TUI notification), and store the resolved absolute path.
|
||||
Closes the P2 finding where bad paths became dead sessions later.
|
||||
|
||||
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 2: Codex capture refuses ambiguous matches
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/hqt/harnesses/configurators/codex.py`
|
||||
- Test: `tests/test_harnesses.py`
|
||||
|
||||
**Context:** `capture_session_id` scans `~/.codex/sessions/rollout-*.jsonl` for rollouts whose `cwd` matches the project and whose start time is `>= since`. Today it sorts the matches and returns the earliest. When two Codex sessions ran in the same directory in the capture window, that guess can be the wrong conversation. The fix: if more than one rollout matches, refuse to guess (return `None` + warn). The single-match normal case is unchanged.
|
||||
|
||||
- [ ] **Step 1: Replace the earliest-wins tests with an ambiguity test**
|
||||
|
||||
In `tests/test_harnesses.py`:
|
||||
|
||||
1. **Delete** `test_codex_capture_first_post_since_cwd_match_wins` (it asserts the now-removed earliest-wins behavior with two matching rollouts).
|
||||
2. **Delete** `test_codex_capture_prefers_started_nearest_since_over_newest_file` (also asserts earliest-wins among two matching rollouts).
|
||||
3. **Add** this test (place it where the deleted ones were):
|
||||
|
||||
```python
|
||||
def test_codex_capture_ambiguous_returns_none(tmp_path):
|
||||
"""Two rollouts match cwd + since window -> refuse to guess, return None."""
|
||||
sessions_dir = tmp_path / ".codex" / "sessions" / "dir"
|
||||
sessions_dir.mkdir(parents=True)
|
||||
|
||||
rollout_a = sessions_dir / "rollout-a.jsonl"
|
||||
rollout_b = sessions_dir / "rollout-b.jsonl"
|
||||
_write_rollout(rollout_a, "id-a", "/projects/foo")
|
||||
_write_rollout(rollout_b, "id-b", "/projects/foo")
|
||||
|
||||
now = time.time()
|
||||
since = now - 120
|
||||
os.utime(rollout_a, (now - 30, now - 30))
|
||||
os.utime(rollout_b, (now - 10, now - 10))
|
||||
|
||||
c = CodexConfigurator()
|
||||
with patch("hqt.harnesses.configurators.codex.Path.home", return_value=tmp_path):
|
||||
result = c.capture_session_id(Path("/projects/foo"), since)
|
||||
assert result is None
|
||||
```
|
||||
|
||||
The single-match regression cases (`test_codex_capture_session_id`,
|
||||
`test_codex_capture_accepts_file_after_since`,
|
||||
`test_codex_capture_skips_non_matching_cwd_picks_older_matching` — which has only
|
||||
one cwd-matching rollout) stay as-is and must keep passing.
|
||||
|
||||
- [ ] **Step 2: Run the capture tests to verify the new one fails**
|
||||
|
||||
Run: `uv run pytest tests/test_harnesses.py -k "capture" -v`
|
||||
Expected: `test_codex_capture_ambiguous_returns_none` FAILS (current code returns `"id-a"`, not `None`). The deleted tests are gone.
|
||||
|
||||
- [ ] **Step 3: Implement the ambiguity guard**
|
||||
|
||||
In `src/hqt/harnesses/configurators/codex.py`, add logging and change `capture_session_id`. Replace the imports block at the top:
|
||||
|
||||
```python
|
||||
import json
|
||||
import logging
|
||||
from datetime import datetime
|
||||
from pathlib import Path
|
||||
|
||||
from hqt.harnesses.base import HarnessConfigurator, SpawnConfig
|
||||
|
||||
log = logging.getLogger(__name__)
|
||||
```
|
||||
|
||||
Replace the tail of `capture_session_id` (the `candidates.sort(...)` / return lines) with the ambiguity guard. The full method:
|
||||
|
||||
```python
|
||||
def capture_session_id(self, project_path: Path, since: float) -> str | None:
|
||||
sessions_dir = Path.home() / ".codex" / "sessions"
|
||||
if not sessions_dir.exists():
|
||||
return None
|
||||
candidates: list[tuple[float, str]] = []
|
||||
for rollout in sessions_dir.rglob("rollout-*.jsonl"):
|
||||
try:
|
||||
mtime = rollout.stat().st_mtime
|
||||
with rollout.open() as f:
|
||||
meta = json.loads(f.readline())
|
||||
payload = meta["payload"]
|
||||
if meta.get("type") != "session_meta" or str(
|
||||
Path(payload["cwd"])
|
||||
) != str(project_path):
|
||||
continue
|
||||
started_at = self._meta_timestamp(payload) or mtime
|
||||
if started_at >= since:
|
||||
candidates.append((started_at, payload["id"]))
|
||||
except (json.JSONDecodeError, KeyError, OSError, TypeError):
|
||||
continue
|
||||
if len(candidates) > 1:
|
||||
# Ambiguous: more than one Codex rollout matches this cwd + time
|
||||
# window (e.g. a second Codex started in the same project). Guessing
|
||||
# risks storing the wrong conversation id, so keep the placeholder.
|
||||
log.warning(
|
||||
"capture_session_id: %d rollouts match cwd=%s since=%s; refusing to guess",
|
||||
len(candidates),
|
||||
project_path,
|
||||
since,
|
||||
)
|
||||
return None
|
||||
return candidates[0][1] if candidates else None
|
||||
```
|
||||
|
||||
(The `candidates.sort(...)` line is removed — with at most one returned candidate there is nothing to sort.)
|
||||
|
||||
- [ ] **Step 4: Run the capture tests**
|
||||
|
||||
Run: `uv run pytest tests/test_harnesses.py -k "capture" -v`
|
||||
Expected: PASS, including `test_codex_capture_ambiguous_returns_none`.
|
||||
|
||||
- [ ] **Step 5: Run quality gates**
|
||||
|
||||
Run: `uv run ruff format src tests && uv run ruff check src tests && uv run ty check`
|
||||
Expected: all green.
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add src/hqt/harnesses/configurators/codex.py tests/test_harnesses.py
|
||||
git commit -m "fix: refuse ambiguous Codex session-id capture
|
||||
|
||||
When more than one rollout matches the project cwd within the capture
|
||||
window, return None and warn instead of guessing the earliest. The worst
|
||||
case is now a kept placeholder id, never a wrong one. Part of the P2
|
||||
capture-race fix.
|
||||
|
||||
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 3: Serialize the spawn→capture window in SessionService
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/hqt/sessions/service.py`
|
||||
- Test: `tests/test_sessions.py`
|
||||
|
||||
**Context:** The Task 2 guard makes wrong captures impossible, but two hqt-launched Codex sessions starting in the same project would now each see an ambiguous window and capture nothing. An `asyncio.Lock` held across the spawn→capture region (only for harnesses that `captures_session_id`) keeps hqt-launched starts from overlapping, so each capture sees exactly its own rollout.
|
||||
|
||||
- [ ] **Step 1: Write the failing tests**
|
||||
|
||||
Add to `tests/test_sessions.py`. The first asserts the lock is held while a capturing harness spawns+captures; the second asserts it is NOT held for a non-capturing harness (so we only pay the serialization cost when needed).
|
||||
|
||||
```python
|
||||
@pytest.mark.asyncio
|
||||
async def test_create_session_holds_capture_lock_for_capturing_harness(
|
||||
factory, db, tmux
|
||||
):
|
||||
observed = {}
|
||||
|
||||
def _capture(project_path, since):
|
||||
# capture runs inside the spawn->capture critical section
|
||||
observed["locked_during_capture"] = service._capture_lock.locked()
|
||||
return "real-codex-id"
|
||||
|
||||
h = MagicMock()
|
||||
h.captures_session_id = True
|
||||
h.generate_session_id.return_value = "placeholder"
|
||||
h.build_spawn_config.return_value = MagicMock(
|
||||
command=["codex"], env={}, cwd=Path("/tmp/myproj")
|
||||
)
|
||||
h.capture_session_id.side_effect = _capture
|
||||
service = SessionService(factory=factory, tmux=tmux, harnesses={"claude-code": h})
|
||||
|
||||
result = await service.create_session(project_id=1, harness_name="claude-code")
|
||||
|
||||
assert observed["locked_during_capture"] is True
|
||||
assert result.session.harness_session_id == "real-codex-id"
|
||||
# Lock is released after create_session returns.
|
||||
assert service._capture_lock.locked() is False
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_create_session_no_lock_for_non_capturing_harness(service, db, tmux):
|
||||
observed = {}
|
||||
|
||||
async def _spawn(req):
|
||||
observed["locked_during_spawn"] = service._capture_lock.locked()
|
||||
return SpawnResult(ok=True, window_id="@1", error="")
|
||||
|
||||
tmux.spawn.side_effect = _spawn
|
||||
|
||||
await service.create_session(project_id=1, harness_name="claude-code")
|
||||
|
||||
# The `service` fixture's harness has captures_session_id = False, so the
|
||||
# spawn must NOT be serialized under the capture lock.
|
||||
assert observed["locked_during_spawn"] is False
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run the new tests to verify they fail**
|
||||
|
||||
Run: `uv run pytest tests/test_sessions.py -k "capture_lock or no_lock" -v`
|
||||
Expected: FAIL with `AttributeError: 'SessionService' object has no attribute '_capture_lock'`.
|
||||
|
||||
- [ ] **Step 3: Add the lock and the conditional guard helper**
|
||||
|
||||
In `src/hqt/sessions/service.py`:
|
||||
|
||||
Add `import contextlib` near the top imports (alongside `import asyncio`):
|
||||
|
||||
```python
|
||||
import asyncio
|
||||
import contextlib
|
||||
import logging
|
||||
import time
|
||||
```
|
||||
|
||||
Add the lock in `__init__` and a helper context manager. Update `__init__`:
|
||||
|
||||
```python
|
||||
def __init__(
|
||||
self,
|
||||
factory: sessionmaker,
|
||||
tmux: TmuxManager,
|
||||
harnesses: Mapping[str, HarnessConfigurator],
|
||||
):
|
||||
self.factory = factory
|
||||
self.tmux = tmux
|
||||
self.harnesses = harnesses
|
||||
# Serializes the spawn->capture window for harnesses that capture a
|
||||
# session id (codex), so two concurrent hqt starts in the same project
|
||||
# never overlap and confuse capture_session_id.
|
||||
self._capture_lock = asyncio.Lock()
|
||||
|
||||
@contextlib.asynccontextmanager
|
||||
async def _maybe_capture_lock(self, needed: bool):
|
||||
"""Hold the capture lock only when the harness captures a session id."""
|
||||
if needed:
|
||||
async with self._capture_lock:
|
||||
yield
|
||||
else:
|
||||
yield
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Wrap the spawn→capture region in `create_session`**
|
||||
|
||||
In `create_session`, wrap from `since = time.time()` through the capture block in the guard. The relevant region becomes (note the added `async with` and the indentation of the lines it now encloses):
|
||||
|
||||
```python
|
||||
spawn_cfg = configurator.build_spawn_config(
|
||||
project_path, harness_session_id, model
|
||||
)
|
||||
log.info(
|
||||
"Spawning window %s: cmd=%s cwd=%s",
|
||||
sess.tmux_session_name,
|
||||
spawn_cfg.command,
|
||||
spawn_cfg.cwd,
|
||||
)
|
||||
async with self._maybe_capture_lock(configurator.captures_session_id):
|
||||
since = time.time()
|
||||
spawn_result = await self.tmux.spawn(
|
||||
SpawnRequest(
|
||||
window_name=sess.tmux_session_name,
|
||||
command=spawn_cfg.command,
|
||||
cwd=str(spawn_cfg.cwd),
|
||||
env=spawn_cfg.env,
|
||||
)
|
||||
)
|
||||
if not spawn_result.ok:
|
||||
log.error(
|
||||
"Failed to spawn window %s: %s",
|
||||
sess.tmux_session_name,
|
||||
spawn_result.error or "(no output captured)",
|
||||
)
|
||||
# Only attempt capture when the spawn actually succeeded; a failed
|
||||
# spawn could inadvertently capture an unrelated session running in
|
||||
# the same cwd.
|
||||
if spawn_result.ok and configurator.captures_session_id:
|
||||
captured_id = await self._capture_session_id_with_retry(
|
||||
configurator, project_path, since, sess.tmux_session_name
|
||||
)
|
||||
if captured_id:
|
||||
sess.harness_session_id = captured_id
|
||||
db.commit()
|
||||
```
|
||||
|
||||
`spawn_result` is referenced after the `async with` block (in the `return CreateSessionResult(...)` that follows) — it remains in scope because `async with` does not introduce a new scope. Leave the rest of the method (the `log.info` and `return CreateSessionResult(...)`) unchanged.
|
||||
|
||||
- [ ] **Step 5: Wrap the rung-2 fresh-spawn region in `_respawn_with_fallback`**
|
||||
|
||||
In `_respawn_with_fallback`, wrap the rung-2 region (from `since = time.time()` through the capture block) in the same guard. The rung-2 portion becomes:
|
||||
|
||||
```python
|
||||
# Rung 2: fresh spawn (codex ignores the old session id; start fresh)
|
||||
configurator = self._configurator(sess.harness.name)
|
||||
project_path = self._project_path(db, sess.project_id)
|
||||
harness_session_id = (
|
||||
sess.harness_session_id or configurator.generate_session_id(sess.id)
|
||||
)
|
||||
spawn_cfg = configurator.build_spawn_config(
|
||||
project_path, harness_session_id, sess.model
|
||||
)
|
||||
async with self._maybe_capture_lock(configurator.captures_session_id):
|
||||
since = time.time()
|
||||
result = await self.tmux.respawn_verified(
|
||||
window_name, spawn_cfg.command, str(spawn_cfg.cwd), env=spawn_cfg.env
|
||||
)
|
||||
if not result.ok:
|
||||
log.error(
|
||||
"Fallback spawn also failed for %s: %s",
|
||||
window_name,
|
||||
result.error or "(no output)",
|
||||
)
|
||||
return False
|
||||
|
||||
# Rung-2 succeeded — capture the new session id if the harness supports
|
||||
# it so that future resumes target this fresh conversation.
|
||||
if configurator.captures_session_id:
|
||||
new_id = await self._capture_session_id_with_retry(
|
||||
configurator, project_path, since, window_name
|
||||
)
|
||||
if new_id:
|
||||
sess.harness_session_id = new_id
|
||||
db.commit()
|
||||
else:
|
||||
log.warning(
|
||||
"capture_session_id exhausted after rung-2 for %s; keeping old id %s",
|
||||
window_name,
|
||||
sess.harness_session_id,
|
||||
)
|
||||
return True
|
||||
```
|
||||
|
||||
The `return False` inside the `async with` correctly releases the lock on the way out. Leave rung 1 (the resume attempt above this region) unchanged — it does not spawn a new conversation, so it needs no lock.
|
||||
|
||||
- [ ] **Step 6: Run the session tests**
|
||||
|
||||
Run: `uv run pytest tests/test_sessions.py -v`
|
||||
Expected: PASS (new lock tests + all existing session tests).
|
||||
|
||||
- [ ] **Step 7: Run quality gates**
|
||||
|
||||
Run: `uv run ruff format src tests && uv run ruff check src tests && uv run ty check`
|
||||
Expected: all green.
|
||||
|
||||
- [ ] **Step 8: Commit**
|
||||
|
||||
```bash
|
||||
git add src/hqt/sessions/service.py tests/test_sessions.py
|
||||
git commit -m "fix: serialize Codex spawn->capture with a lock
|
||||
|
||||
Hold an asyncio.Lock across the spawn->capture window for harnesses that
|
||||
capture a session id, so two concurrent hqt starts in the same project
|
||||
never produce overlapping capture windows. Completes the P2 capture-race
|
||||
fix alongside the ambiguity guard.
|
||||
|
||||
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 4: Real-tmux smoke tests
|
||||
|
||||
**Files:**
|
||||
- Create: `tests/test_tmux_smoke.py`
|
||||
- Modify: `pyproject.toml`
|
||||
|
||||
**Context:** tmux behavior is otherwise tested with mocked `_exec`. These tests drive the real `TmuxRunner` against a throwaway tmux server isolated via `TMUX_TMPDIR`, so they never touch the user's live sessions, and skip when tmux is absent.
|
||||
|
||||
- [ ] **Step 1: Register the `tmux` pytest marker**
|
||||
|
||||
In `pyproject.toml`, add a `[tool.pytest.ini_options]` block (none exists yet — add it after the `[dependency-groups]` block, before `[tool.hqt.quality]`):
|
||||
|
||||
```toml
|
||||
[tool.pytest.ini_options]
|
||||
markers = [
|
||||
"tmux: real-tmux smoke tests; require a tmux binary and run on an isolated TMUX_TMPDIR server",
|
||||
]
|
||||
```
|
||||
|
||||
Note: the existing async tests already run with explicit `@pytest.mark.asyncio` decorators under pytest-asyncio's default strict mode, so do **not** add an `asyncio_mode` setting — only the `markers` entry is needed here.
|
||||
|
||||
- [ ] **Step 2: Write the smoke tests**
|
||||
|
||||
Create `tests/test_tmux_smoke.py`:
|
||||
|
||||
```python
|
||||
"""Real-tmux smoke tests.
|
||||
|
||||
These drive the actual TmuxRunner against a throwaway tmux server whose socket
|
||||
lives in a temp TMUX_TMPDIR, so they never touch the user's live tmux sessions.
|
||||
They skip when no tmux binary is available.
|
||||
"""
|
||||
|
||||
import asyncio
|
||||
import os
|
||||
import shutil
|
||||
import subprocess
|
||||
|
||||
import pytest
|
||||
|
||||
from hqt.tmux.runner import TmuxRunner
|
||||
|
||||
pytestmark = [
|
||||
pytest.mark.tmux,
|
||||
pytest.mark.skipif(shutil.which("tmux") is None, reason="tmux not installed"),
|
||||
]
|
||||
|
||||
SESSION = "hqt-test"
|
||||
|
||||
|
||||
def _tmux(env, *args):
|
||||
"""Run a raw tmux command against the isolated server, return stdout."""
|
||||
return subprocess.run(
|
||||
["tmux", *args],
|
||||
env=env,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
check=False,
|
||||
)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def tmux_env(tmp_path, monkeypatch):
|
||||
# Point tmux's server socket at the temp dir: a fully isolated server.
|
||||
monkeypatch.setenv("TMUX_TMPDIR", str(tmp_path))
|
||||
env = {**os.environ, "TMUX_TMPDIR": str(tmp_path)}
|
||||
_tmux(env, "new-session", "-d", "-s", SESSION, "-x", "200", "-y", "50")
|
||||
try:
|
||||
yield env
|
||||
finally:
|
||||
_tmux(env, "kill-server")
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def runner(tmux_env):
|
||||
return TmuxRunner(tmux_path="tmux", session_name=SESSION)
|
||||
|
||||
|
||||
async def _wait_pane_dead(runner, window, timeout=2.0):
|
||||
deadline = asyncio.get_running_loop().time() + timeout
|
||||
while asyncio.get_running_loop().time() < deadline:
|
||||
if await runner.is_pane_dead(window):
|
||||
return True
|
||||
await asyncio.sleep(0.05)
|
||||
return await runner.is_pane_dead(window)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_new_window_appears_in_list(runner, tmp_path):
|
||||
window_id = await runner.new_window("hqt-1", str(tmp_path), "sleep 60")
|
||||
assert window_id is not None
|
||||
assert "hqt-1" in await runner.list_windows()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_set_window_label_round_trips(runner, tmp_path, tmux_env):
|
||||
await runner.new_window("hqt-1", str(tmp_path), "sleep 60")
|
||||
await runner.set_window_label("hqt-1", "•hqt-1")
|
||||
shown = _tmux(
|
||||
tmux_env,
|
||||
"show-options",
|
||||
"-w",
|
||||
"-t",
|
||||
f"{SESSION}:=hqt-1",
|
||||
"@hqt_label",
|
||||
).stdout
|
||||
assert "•hqt-1" in shown
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_respawn_revives_dead_pane(runner, tmp_path):
|
||||
# Window whose command exits immediately -> pane dies (remain-on-exit keeps it).
|
||||
await runner.new_window("hqt-1", str(tmp_path), "true")
|
||||
assert await _wait_pane_dead(runner, "hqt-1") is True
|
||||
assert await runner.respawn_pane("hqt-1", "sleep 60", str(tmp_path)) is True
|
||||
alive, _ = await runner.verify_window_alive("hqt-1", timeout=1.0)
|
||||
assert alive is True
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_apply_theme_sets_session_options(runner, tmux_env):
|
||||
await runner.apply_theme()
|
||||
shown = _tmux(tmux_env, "show-options", "-t", SESSION, "status-justify").stdout
|
||||
assert "left" in shown
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Run the smoke tests**
|
||||
|
||||
Run: `uv run pytest tests/test_tmux_smoke.py -v`
|
||||
Expected: PASS where tmux is installed (4 tests). If tmux is absent: all SKIPPED with reason "tmux not installed".
|
||||
|
||||
- [ ] **Step 4: Verify the marker is registered (no warnings)**
|
||||
|
||||
Run: `uv run pytest tests/test_tmux_smoke.py -v -W error::pytest.PytestUnknownMarkWarning`
|
||||
Expected: PASS/SKIP with no `PytestUnknownMarkWarning` (proves the marker is registered).
|
||||
|
||||
- [ ] **Step 5: Run the full suite + quality gates**
|
||||
|
||||
Run: `uv run pytest && uv run ruff format src tests && uv run ruff check src tests && uv run ty check`
|
||||
Expected: full suite green (smoke tests pass or skip), gates green.
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add tests/test_tmux_smoke.py pyproject.toml
|
||||
git commit -m "test: add isolated real-tmux smoke tests
|
||||
|
||||
Drive the real TmuxRunner against a throwaway tmux server isolated via
|
||||
TMUX_TMPDIR (never touches live sessions), covering new windows, label
|
||||
round-trip, respawn, and theming. Auto-skip when tmux is absent; register
|
||||
the 'tmux' marker. Closes the P2 real-tmux testing gap.
|
||||
|
||||
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 5: Remove resolved P2 findings from TODO.md
|
||||
|
||||
**Files:**
|
||||
- Modify: `TODO.md`
|
||||
|
||||
**Context:** All three P2 findings are now resolved. `TODO.md` currently contains exactly those three lines and nothing else.
|
||||
|
||||
- [ ] **Step 1: Empty out the resolved findings**
|
||||
|
||||
`TODO.md` currently reads:
|
||||
|
||||
```
|
||||
P2: Codex session-id capture is heuristic and can store the wrong rollout if another Codex session starts in the same project during the retry window.
|
||||
P2: tmux behavior is mostly tested with mocks; add isolated real-tmux smoke tests for theming, new windows, respawn, and labels.
|
||||
P2: Project paths are accepted without existence validation, so bad paths become dead sessions later instead of being rejected early.
|
||||
```
|
||||
|
||||
Replace the entire file contents with a single placeholder line so the file is not left dangling:
|
||||
|
||||
```
|
||||
No open findings.
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Verify**
|
||||
|
||||
Run: `cat TODO.md`
|
||||
Expected: `No open findings.`
|
||||
|
||||
- [ ] **Step 3: Commit**
|
||||
|
||||
```bash
|
||||
git add TODO.md
|
||||
git commit -m "chore: clear resolved P2 findings from TODO
|
||||
|
||||
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Final verification (after all tasks)
|
||||
|
||||
- [ ] Run the full suite and gates once more:
|
||||
```
|
||||
uv run pytest && uv run ruff format src tests && uv run ruff check src tests && uv run ty check
|
||||
```
|
||||
Expected: all tests pass (or smoke tests skip without tmux), all gates green.
|
||||
- [ ] Confirm `src/hqt/tmux/runner.py` is still only modified in the working tree (its pre-existing WIP) and was never committed:
|
||||
```
|
||||
git log --oneline -6
|
||||
git status --short
|
||||
```
|
||||
Expected: the six new commits are present; `git status --short` shows only ` M src/hqt/tmux/runner.py`.
|
||||
File diff suppressed because it is too large
Load Diff
@@ -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.
|
||||
@@ -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 <window>` (builds the fzf `display-popup`) and `hqt tool <name> <window>` (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 <noreply@anthropic.com>` 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 <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 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 <target> ...` 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 <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 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 <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 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 <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 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-<id> 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 <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 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-<id> 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 <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 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 <window>` 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 <choice> <window>`. `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 {} <window>`
|
||||
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 <choice> <window>`.
|
||||
"""
|
||||
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 <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 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-<id> 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-<id>` 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-<id>` 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.
|
||||
@@ -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 <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
|
||||
`<common-dir>/info/exclude` if not already present (create file if
|
||||
missing).
|
||||
3. `git worktree add -b <branch> <repo>/.worktrees/<branch>` (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 <repo>/.worktrees/<branch> <branch>` (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 <branch> --not --all
|
||||
--count`-style query. Use `git rev-list --count <branch> --not
|
||||
--exclude=refs/heads/<branch> --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 <branch> --not <others...>`
|
||||
(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] <path>` (ServiceError on
|
||||
failure), then best-effort `git branch -d <branch>` (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/<branch>` 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/<branch>` 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 <label>?", optional warning line, `Checkbox("Also remove
|
||||
worktree", value=warning is None)`, Delete/Cancel buttons. Returns
|
||||
`(confirmed, remove_worktree)`; `remove_worktree and warning is not None`
|
||||
means force.
|
||||
- `app.py`:
|
||||
- `action_new_session`: compute `is_git_repo` via
|
||||
`await worktree.is_git_repo(Path(project.path))` (inside the worker or
|
||||
pre-fetched; ProjectService has the path — follow existing access
|
||||
patterns), pass to screen; pass `worktree_branch` to `create_session`.
|
||||
- `action_delete_session`: fetch session; if it has no worktree, keep
|
||||
today's immediate delete. Otherwise call
|
||||
`sessions.worktree_state_for(sid)`, build warning string when dirty or
|
||||
unique_commits > 0 ("N uncommitted file(s), M unmerged commit(s) — work
|
||||
will be lost"), push ConfirmDeleteScreen; on (True, remove) call
|
||||
`delete_session(sid, remove_worktree=remove, force=remove and warned)`.
|
||||
ServiceError already surfaces via `_run_service_worker`.
|
||||
- Session list: rows for worktree sessions append ` ⎇ <branch>`; check
|
||||
`src/hqt/tui/widgets/session_list.py` for the row-label construction.
|
||||
- tmux window label: in `sync_window_labels`, the name passed to
|
||||
`window_label` for worktree sessions is `nickname or branch` (nickname
|
||||
already defaults to branch at creation, so no change may be needed —
|
||||
verify and leave a test).
|
||||
- README: document the worktree checkbox under Key Bindings/feature notes,
|
||||
and the `.worktrees/` location + submodule limitation.
|
||||
- Tests: NewSessionScreen returns worktree_branch when checked, None when
|
||||
unchecked, disabled checkbox when not a repo; ConfirmDeleteScreen returns
|
||||
the right tuples; app delete path for non-worktree sessions unchanged
|
||||
(no modal).
|
||||
|
||||
Commit message: `feat: worktree session UI (dialog checkbox, confirm delete, list marker)`.
|
||||
File diff suppressed because it is too large
Load Diff
@@ -1,353 +0,0 @@
|
||||
# New Session Dialog UX Refinements 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:** Three UX refinements to the New Session modal — hide sandbox sub-options until sandboxing is enabled, unify the focus highlight to the dialog's Peach accent, and add context-aware j/k field navigation.
|
||||
|
||||
**Architecture:** All changes are confined to the `NewSessionScreen` widget (`src/hqt/tui/screens/new_session.py`) and the shared stylesheet (`src/hqt/tui/styles.tcss`). The reveal logic reuses the screen's existing show/hide pattern (the worktree checkbox → branch input), navigation uses Textual's built-in `focus_next`/`focus_previous` screen actions, and the highlight is a scoped CSS rule. No service, DB, or sandbox-policy code is touched.
|
||||
|
||||
**Tech Stack:** Python 3.14, Textual (TUI framework), pytest + pytest-asyncio (`app.run_test()` pilot harness), uv for running commands.
|
||||
|
||||
---
|
||||
|
||||
## Conventions
|
||||
|
||||
- Run tests with: `uv run --active pytest <path> -v`
|
||||
- Run the linter with: `uv run --active ruff check`
|
||||
- Existing screen tests live in `tests/test_tui.py` and use the `app.run_test()` pilot. New tests go there too.
|
||||
- The screen under test is constructed as `NewSessionScreen(harness_names, is_git_repo=..., sandbox_available=...)` and pushed with `app.push_screen(...)`.
|
||||
- Commit messages end with the trailer:
|
||||
`Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>`
|
||||
|
||||
## File Structure
|
||||
|
||||
- `src/hqt/tui/screens/new_session.py` — modal screen. Gains: a `#sandbox-options` container around the fs/net rows, an `on_switch_changed` handler, and `BINDINGS` for j/k.
|
||||
- `src/hqt/tui/styles.tcss` — shared stylesheet. Gains: a scoped `:focus` rule for the dialog's controls.
|
||||
- `tests/test_tui.py` — screen tests. Gains: reveal, focus-highlight, and j/k navigation tests.
|
||||
|
||||
---
|
||||
|
||||
### Task 1: Hide sandbox sub-options until the switch is on
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/hqt/tui/screens/new_session.py:66-74` (the fs/net rows) and add an `on_switch_changed` handler near `on_checkbox_changed` (around line 79)
|
||||
- Test: `tests/test_tui.py`
|
||||
|
||||
- [ ] **Step 1: Write the failing test**
|
||||
|
||||
Append to `tests/test_tui.py`:
|
||||
|
||||
```python
|
||||
@pytest.mark.asyncio
|
||||
async def test_sandbox_options_hidden_until_switch_on():
|
||||
"""The fs/net rows are sub-options of the sandbox switch: hidden until it is
|
||||
turned on, shown when on, hidden again when turned back off."""
|
||||
app = HqtApp()
|
||||
async with app.run_test(size=(120, 40)) as pilot:
|
||||
from hqt.tui.screens.new_session import NewSessionScreen
|
||||
from textual.widgets import Switch
|
||||
|
||||
app.push_screen(
|
||||
NewSessionScreen(["claude"], is_git_repo=True, sandbox_available=True)
|
||||
)
|
||||
await pilot.pause()
|
||||
options = app.screen.query_one("#sandbox-options")
|
||||
assert options.display is False
|
||||
|
||||
switch = app.screen.query_one("#sandbox-switch", Switch)
|
||||
switch.value = True
|
||||
await pilot.pause()
|
||||
assert options.display is True
|
||||
|
||||
switch.value = False
|
||||
await pilot.pause()
|
||||
assert options.display is False
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run test to verify it fails**
|
||||
|
||||
Run: `uv run --active pytest tests/test_tui.py::test_sandbox_options_hidden_until_switch_on -v`
|
||||
Expected: FAIL — `NoMatches` / no widget with id `sandbox-options` (the container does not exist yet).
|
||||
|
||||
- [ ] **Step 3: Wrap the fs/net rows in a hidden container**
|
||||
|
||||
In `src/hqt/tui/screens/new_session.py`, replace the current fs/net block (lines 66-74):
|
||||
|
||||
```python
|
||||
yield Label("Filesystem access:")
|
||||
yield Select(
|
||||
[("Read-write", "rw"), ("Read-only", "ro")],
|
||||
id="fs-select",
|
||||
value="rw",
|
||||
allow_blank=False,
|
||||
)
|
||||
yield Label("Network:")
|
||||
yield Switch(id="net-switch", value=True)
|
||||
```
|
||||
|
||||
with a container that starts hidden (same `display = False` pattern already used
|
||||
for `#branch-input` above):
|
||||
|
||||
```python
|
||||
# Sub-options of the sandbox switch: only meaningful when sandboxing
|
||||
# is on, so hidden until then (mirrors the worktree -> branch reveal).
|
||||
sandbox_options = Vertical(id="sandbox-options")
|
||||
sandbox_options.display = False
|
||||
with sandbox_options:
|
||||
yield Label("Filesystem access:")
|
||||
yield Select(
|
||||
[("Read-write", "rw"), ("Read-only", "ro")],
|
||||
id="fs-select",
|
||||
value="rw",
|
||||
allow_blank=False,
|
||||
)
|
||||
yield Label("Network:")
|
||||
yield Switch(id="net-switch", value=True)
|
||||
```
|
||||
|
||||
`Vertical` is already imported at the top of the file (`from textual.containers import Horizontal, Vertical`).
|
||||
|
||||
- [ ] **Step 4: Add the reveal handler**
|
||||
|
||||
In the same file, add this method directly after `on_checkbox_changed` (it ends
|
||||
around line 90). Guard on the switch id so the `#net-switch` toggle inside the
|
||||
container does not trigger it:
|
||||
|
||||
```python
|
||||
def on_switch_changed(self, event: Switch.Changed) -> None:
|
||||
if event.switch.id != "sandbox-switch":
|
||||
return
|
||||
self.query_one("#sandbox-options").display = event.value
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Run test to verify it passes**
|
||||
|
||||
Run: `uv run --active pytest tests/test_tui.py::test_sandbox_options_hidden_until_switch_on -v`
|
||||
Expected: PASS
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add src/hqt/tui/screens/new_session.py tests/test_tui.py
|
||||
git commit -m "feat: reveal sandbox sub-options only when sandboxing is on
|
||||
|
||||
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 2: Unify the focus highlight to the Peach accent
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/hqt/tui/styles.tcss` (append a scoped `:focus` rule)
|
||||
- Test: `tests/test_tui.py`
|
||||
|
||||
Context: the dialog buttons already get a Peach `$accent` focus highlight, but
|
||||
`Switch`/`Checkbox`/`Select`/`Input` fall back to Textual's default Lavender
|
||||
(`$border`) focus border. This task makes every focusable control in the dialog
|
||||
share the Peach accent.
|
||||
|
||||
- [ ] **Step 1: Write the failing test**
|
||||
|
||||
Append to `tests/test_tui.py`:
|
||||
|
||||
```python
|
||||
def test_dialog_focus_highlight_uses_accent():
|
||||
"""Every focusable control in the New Session dialog shares the Peach accent
|
||||
focus border (consistent with the dialog buttons), not Textual's default
|
||||
Lavender. Guards against the scoped :focus rule being dropped."""
|
||||
from pathlib import Path
|
||||
import hqt.tui
|
||||
|
||||
css = (Path(hqt.tui.__file__).parent / "styles.tcss").read_text()
|
||||
for selector in (
|
||||
"#new-session-dialog Switch:focus",
|
||||
"#new-session-dialog Checkbox:focus",
|
||||
"#new-session-dialog Select:focus",
|
||||
"#new-session-dialog Input:focus",
|
||||
):
|
||||
assert selector in css, f"missing focus rule: {selector}"
|
||||
assert "border: tall $accent;" in css
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run test to verify it fails**
|
||||
|
||||
Run: `uv run --active pytest tests/test_tui.py::test_dialog_focus_highlight_uses_accent -v`
|
||||
Expected: FAIL — selectors not found in `styles.tcss`.
|
||||
|
||||
- [ ] **Step 3: Add the scoped focus rule**
|
||||
|
||||
Append to the end of `src/hqt/tui/styles.tcss`:
|
||||
|
||||
```css
|
||||
/* Unify the focus highlight across the New Session dialog. The buttons get a
|
||||
Peach $accent focus highlight (above), but Switch/Checkbox/Select/Input
|
||||
otherwise fall back to Textual's default Lavender $border focus border, which
|
||||
reads as off-palette next to the buttons. Match `tall` so only the color
|
||||
changes, not the control height. */
|
||||
#new-session-dialog Switch:focus,
|
||||
#new-session-dialog Checkbox:focus,
|
||||
#new-session-dialog Select:focus,
|
||||
#new-session-dialog Input:focus {
|
||||
border: tall $accent;
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run test to verify it passes**
|
||||
|
||||
Run: `uv run --active pytest tests/test_tui.py::test_dialog_focus_highlight_uses_accent -v`
|
||||
Expected: PASS
|
||||
|
||||
- [ ] **Step 5: Sanity-check the dialog still mounts**
|
||||
|
||||
Run: `uv run --active pytest tests/test_tui.py -k new_session -v`
|
||||
Expected: PASS (the CSS parses and the existing New Session tests are unaffected).
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add src/hqt/tui/styles.tcss tests/test_tui.py
|
||||
git commit -m "style: unify New Session dialog focus highlight to accent
|
||||
|
||||
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 3: Soft context-aware j/k field navigation
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/hqt/tui/screens/new_session.py` (add `BINDINGS` and the `Binding` import)
|
||||
- Test: `tests/test_tui.py`
|
||||
|
||||
Context: j/k move focus through the dialog's focus chain. Because `Input`
|
||||
consumes printable keys before screen bindings fire, j/k type literally while
|
||||
editing text and only move focus when the focused control is a toggle/select/
|
||||
button — the "soft" model. `focus_next`/`focus_previous` are built-in Textual
|
||||
screen actions; Tab/Shift+Tab/arrows are unaffected.
|
||||
|
||||
- [ ] **Step 1: Write the failing tests**
|
||||
|
||||
Append to `tests/test_tui.py`:
|
||||
|
||||
```python
|
||||
def test_new_session_has_jk_focus_bindings():
|
||||
"""j -> focus_next, k -> focus_previous, declared on the screen."""
|
||||
from hqt.tui.screens.new_session import NewSessionScreen
|
||||
|
||||
actions = {b.key: b.action for b in NewSessionScreen.BINDINGS}
|
||||
assert actions.get("j") == "focus_next"
|
||||
assert actions.get("k") == "focus_previous"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_jk_moves_focus_between_controls():
|
||||
"""On a non-text control, j moves focus to another control."""
|
||||
app = HqtApp()
|
||||
async with app.run_test(size=(120, 40)) as pilot:
|
||||
from hqt.tui.screens.new_session import NewSessionScreen
|
||||
from textual.widgets import Switch
|
||||
|
||||
app.push_screen(
|
||||
NewSessionScreen(["claude"], is_git_repo=True, sandbox_available=True)
|
||||
)
|
||||
await pilot.pause()
|
||||
switch = app.screen.query_one("#sandbox-switch", Switch)
|
||||
switch.focus()
|
||||
await pilot.pause()
|
||||
assert app.focused is switch
|
||||
|
||||
await pilot.press("j")
|
||||
await pilot.pause()
|
||||
assert app.focused is not switch
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_jk_typed_into_input_is_literal():
|
||||
"""Inside a text input, j/k type literally and do not move focus."""
|
||||
app = HqtApp()
|
||||
async with app.run_test(size=(120, 40)) as pilot:
|
||||
from hqt.tui.screens.new_session import NewSessionScreen
|
||||
from textual.widgets import Input
|
||||
|
||||
app.push_screen(NewSessionScreen(["claude"], is_git_repo=True))
|
||||
await pilot.pause()
|
||||
nickname = app.screen.query_one("#nickname-input", Input)
|
||||
nickname.focus()
|
||||
await pilot.pause()
|
||||
|
||||
await pilot.press("j", "k")
|
||||
await pilot.pause()
|
||||
assert nickname.value == "jk"
|
||||
assert app.focused is nickname
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run tests to verify they fail**
|
||||
|
||||
Run: `uv run --active pytest tests/test_tui.py -k "jk or jk_focus" -v`
|
||||
Expected: FAIL — `NewSessionScreen` has no `BINDINGS` attribute defining j/k (and pressing j on the switch would not move focus).
|
||||
|
||||
- [ ] **Step 3: Add the bindings**
|
||||
|
||||
In `src/hqt/tui/screens/new_session.py`, add the `Binding` import. Change:
|
||||
|
||||
```python
|
||||
from textual.app import ComposeResult
|
||||
```
|
||||
|
||||
to:
|
||||
|
||||
```python
|
||||
from textual.app import ComposeResult
|
||||
from textual.binding import Binding
|
||||
```
|
||||
|
||||
Then add a `BINDINGS` class attribute as the first line inside the
|
||||
`NewSessionScreen` class body (immediately after the `class` line, before
|
||||
`__init__`):
|
||||
|
||||
```python
|
||||
# Soft vim-style field navigation: j/k walk the dialog's focus chain. Inputs
|
||||
# consume printable keys first, so j/k type literally while editing text and
|
||||
# only move focus when a toggle/select/button is focused.
|
||||
BINDINGS = [
|
||||
Binding("j", "focus_next", "Next field", show=False),
|
||||
Binding("k", "focus_previous", "Prev field", show=False),
|
||||
]
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run tests to verify they pass**
|
||||
|
||||
Run: `uv run --active pytest tests/test_tui.py -k "jk or jk_focus" -v`
|
||||
Expected: PASS (all three tests)
|
||||
|
||||
- [ ] **Step 5: Commit**
|
||||
|
||||
```bash
|
||||
git add src/hqt/tui/screens/new_session.py tests/test_tui.py
|
||||
git commit -m "feat: soft j/k field navigation in New Session dialog
|
||||
|
||||
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 4: Full-suite verification
|
||||
|
||||
**Files:** none (verification only)
|
||||
|
||||
- [ ] **Step 1: Run the whole test suite**
|
||||
|
||||
Run: `uv run --active pytest -q`
|
||||
Expected: PASS — no regressions; the new tests included.
|
||||
|
||||
- [ ] **Step 2: Lint**
|
||||
|
||||
Run: `uv run --active ruff check`
|
||||
Expected: `All checks passed!`
|
||||
|
||||
- [ ] **Step 3 (optional manual smoke): launch the TUI and open New Session**
|
||||
|
||||
Run the app, press `n`, and confirm: the fs/net rows appear only after toggling
|
||||
Sandboxed on; focused controls show a Peach (not Lavender) highlight; j/k move
|
||||
between toggles but type literally in the nickname/model/branch fields.
|
||||
Reference in New Issue
Block a user