Merge branch 'main' into worktree-sessions

# Conflicts: # src/hqt/sessions/service.py # tests/test_sessions.py
2026-06-10 21:07:20 -04:00
parent ace537e3d6 d78d8ee149
commit dff53144c9
14 changed files with 1826 additions and 133 deletions
@@ -0,0 +1,204 @@
+# Sandboxed sessions via bubblewrap — design
+
+**Date:** 2026-06-10
+**Status:** Approved (pending implementation plan)
+
+## Problem
+
+Users want to start harness sessions inside a [bubblewrap](https://github.com/containers/bubblewrap)
+(`bwrap`) sandbox. Inside the jail the agent can run freely — so the harness is
+launched with its "skip permissions" flag (`--dangerously-skip-permissions` for
+Claude Code, the equivalent for other harnesses) — while bubblewrap provides the
+real safety boundary: controlled filesystem access (read-only or read-write to
+the project directory) and optional network access. The configuration must be
+quick to set per session.
+
+## Decisions (from brainstorming)
+
+- **Config scope:** per session, chosen in the New Session dialog and stored on
+  the session row so resume/respawn reuse it.
+- **Controls:** a "Sandboxed" toggle plus two sub-toggles — filesystem mode
+  (read-only / read-write) and network (on / off).
+- **Skip-permissions:** automatic. Sandbox on ⇒ the harness's skip-permissions
+  flag is added. The bwrap container is the safety boundary, so there is no
+  separate widget for it.
+- **Filesystem model:** minimal allowlist. User data (`$HOME` and other
+  directories) is hidden. System directories are bound read-only so binaries and
+  libraries work; the project directory and a per-harness set of
+  credential/config paths are bound explicitly.
+- **Defaults when sandbox is enabled:** read-write project dir, network on (the
+  common "let it work, but contained" case; the harness needs network to reach a
+  hosted model API).
+- **bwrap missing:** hard failure, never a silent downgrade to unsandboxed.
+
+## Key constraint: network is all-or-nothing
+
+bubblewrap toggles networking at the namespace level — it cannot selectively
+allow the model API while blocking everything else. Therefore:
+
+- **Network on** = the sandbox shares the host network namespace; the agent (and
+  its tools: `curl`, `git push`, etc.) can reach anything the host can.
+- **Network off** = no connectivity at all. The harness process itself cannot
+  reach a hosted model API, so this mode is only useful with local/offline
+  models. The toggle's value is blocking the agent's network for offline or
+  untrusted-code review.
+
+This constraint is surfaced in the UI defaults (network defaults on).
+
+## Architecture
+
+### 1. Data model
+
+Add one nullable column to `Session`:
+
+```python
+sandbox_json: Mapped[str | None] = mapped_column(default=None)
+```
+
+`NULL` = unsandboxed. When set it holds:
+
+```json
+{ "fs": "rw" | "ro", "net": true }
+```
+
+Presence of the value means sandboxed; `fs` and `net` capture the two
+sub-toggles. Skip-permissions is *derived* (sandboxed ⇒ on) and is not stored.
+A single `user_version`-based migration (see `db/migrations.py`) adds the column.
+
+Because the config lives on the row, `attach_session` → `_respawn_with_fallback`
+and `_get_resume_config` rebuild the same sandboxed command after a stop.
+
+### 2. The configuration ABC — `HarnessConfigurator`
+
+Per-harness knowledge lives here. Two additions to `harnesses/base.py`:
+
+```python
+class Bind:
+    """A path to expose inside the sandbox."""
+    src: Path
+    writable: bool = False
+
+class HarnessConfigurator(ABC):
+    ...
+    sandbox_skip_permission_flags: list[str] = []
+
+    def sandbox_binds(self) -> list[Bind]:
+        return []
+```
+
+Per-harness values:
+
+| Harness   | `sandbox_skip_permission_flags`              | `sandbox_binds()`        |
+|-----------|----------------------------------------------|--------------------------|
+| `claude`  | `["--dangerously-skip-permissions"]`         | `~/.claude` (rw)         |
+| `codex`   | `["--dangerously-bypass-approvals-and-sandbox"]` | `~/.codex` (rw)      |
+| `kiro`    | `[]`                                          | `~/.kiro` (rw)           |
+| `generic` | `[]`                                          | `[]`                     |
+
+The exact flag spelling and credential paths must be verified against each
+installed binary during implementation (e.g. `claude --help`); the values above
+are the expected defaults.
+
+`build_spawn_config` / `build_resume_config` gain a `sandboxed: bool = False`
+parameter. When true, the configurator appends
+`self.sandbox_skip_permission_flags` to the command list. The configurator does
+**not** construct the bwrap invocation — it only declares flags and binds.
+
+### 3. Bubblewrap policy module — `hqt/sandbox.py`
+
+A pure, argv-only function:
+
+```python
+@dataclass(frozen=True)
+class SandboxPolicy:
+    fs: str          # "rw" | "ro"
+    net: bool
+
+def wrap(
+    command: list[str],
+    cwd: Path,
+    policy: SandboxPolicy,
+    binds: list[Bind],
+) -> list[str]:
+    """Return the `bwrap … -- <command>` argv."""
+```
+
+Assembled from three layers:
+
+- **Base (always):** `--unshare-all`, `--die-with-parent`; RO binds of system
+  dirs (`/usr`, `/bin`, `/lib`, `/lib64`, and curated `/etc` essentials such as
+  `resolv.conf`, `ssl`, `passwd`); `--proc /proc`; `--dev /dev`; `--tmpfs /tmp`;
+  pass-through of `PATH`, `HOME`, `TERM`, `LANG`. `$HOME` itself is not bound, so
+  user data is hidden. `~/.gitconfig` bound RO when present.
+- **Harness binds:** each `Bind` from the configurator, `--bind` (writable) or
+  `--ro-bind`, creating parent dirs as needed.
+- **cwd:** `--bind cwd cwd` when `fs == "rw"`, else `--ro-bind cwd cwd`.
+- **net:** when `policy.net` is true, the network namespace is shared (omit the
+  net-unshare); otherwise it stays unshared and there is no connectivity.
+
+Being pure and producing only argv (no subprocess), `wrap` is unit-testable
+without bwrap installed.
+
+### 4. Spawn / resume integration — `SessionService`
+
+In `create_session`: call `build_spawn_config(..., sandboxed=True)` when the
+request is sandboxed, then pass `SpawnConfig.command` through `sandbox.wrap(...)`
+(using the configurator's `sandbox_binds()` and the session policy) before
+constructing the `SpawnRequest`. `cwd` remains the project path — bwrap binds it.
+
+The same wrapping applies in `_respawn_with_fallback` (both the resume rung and
+the fresh-spawn rung) and in `_get_resume_config`, reading the policy back from
+`sandbox_json`.
+
+`create_session` gains a `sandbox` parameter (the parsed policy or `None`).
+
+### 5. UI — New Session dialog
+
+`tui/screens/new_session.py` adds:
+
+- A `Switch` labelled "Sandboxed" (default off).
+- Revealed when on: a filesystem `Select` ("Read-write" / "Read-only", default
+  Read-write) and a "Network" `Switch` (default on).
+
+The dialog's result tuple is extended to carry the sandbox config (or `None`),
+threaded into `create_session`.
+
+**bwrap availability gates the toggle.** The dialog checks bubblewrap
+availability (the same check `doctor` uses — see §6) on mount. When bwrap is
+unavailable, the "Sandboxed" switch is disabled (cannot be turned on) and an
+inline warning explains why (e.g. "bubblewrap not found — run `hqt doctor`").
+This makes the unavailable state visible up front rather than only at spawn.
+
+### 6. Availability check, doctor & failure handling
+
+- A single shared helper (e.g. `sandbox.is_available()`) reports whether `bwrap`
+  is on `PATH` and the platform is Linux. Both `doctor` and the New Session
+  dialog use it, so there is one source of truth.
+- `hqt doctor` reports bubblewrap availability as an optional capability
+  (present / missing), alongside the existing checks.
+- The dialog uses the helper to disable the Sandboxed toggle (§5), so an
+  unsandboxable environment is caught before the user picks anything.
+- As a backstop (defense in depth), if a sandboxed session somehow reaches spawn
+  while bwrap is unavailable, spawn fails loudly with a `ServiceError` — never a
+  silent downgrade to an unsandboxed session.
+
+## Testing
+
+- **`sandbox.wrap` unit tests:** assert the argv for each toggle combination
+  (rw/ro × net/no-net), that harness binds are spliced in with the right
+  `--bind`/`--ro-bind`, and that the command appears after `--`.
+- **Configurator tests:** skip-permission flags are appended only when
+  `sandboxed=True`; absent otherwise.
+- **Service test:** a sandboxed `create_session` wraps the command in `bwrap`;
+  a bwrap-missing environment raises `ServiceError`.
+- **Availability/UI test:** `sandbox.is_available()` is false when `bwrap` is
+  absent or the platform is non-Linux, and the dialog disables the Sandboxed
+  toggle in that case.
+- **Migration test:** the new column is added and round-trips a policy.
+
+## Out of scope (YAGNI for v1)
+
+- Per-project or named global sandbox profiles (per-session only for now).
+- A freeform "extra bind paths" field in the dialog.
+- Selective network filtering (proxy/firewall) — bwrap is namespace-level only.
+- Remembering the last-used sandbox config as a default.
@@ -0,0 +1,269 @@
+# Tool palette: nvim / lazygit / shell / clone per session
+
+**Date:** 2026-06-10
+**Status:** Approved (revised — adds the Alt+p palette and the clone action)
+
+## Goal
+
+From anywhere inside a harness pane, press **Alt+p** to get a small fuzzy
+launcher for the current session's project:
+
+- **nvim / lazygit / shell** — open the tool in a **new tmux window appended at
+  the next free index**, styled exactly like hqt's own windows, in the session's
+  project directory. The window closes when you quit the tool.
+- **clone** — open a **fresh harness session** (a real `hqt-<id>` window) with the
+  **same project, harness, and model** as the current session, but a brand-new
+  conversation.
+
+One key (Alt+p), one fuzzy list, so there are no per-tool shortcuts to memorize.
+The palette is a tmux binding, so it works over the TUI and inside any harness.
+
+## Background
+
+A session is a tmux window named `tmux_session_name` (`hqt-{id}`), created in its
+project's directory by `TmuxRunner.new_window()`, which appends at
+`_next_window_index()` (= `max(window indices) + 1`) and applies the Frappé
+per-window theme. A session row stores `project_id`, `harness_id`, and `model`
+(`db/models.py`), so cloning is just `create_session` with those three values.
+
+Three facts make this design cheap and safe:
+
+- hqt tracks windows **by name, keyed to DB sessions**, and **never prunes
+  unknown windows**. `sync_window_labels()` (the 3s poll) only labels rows it
+  knows. A window that is not a DB session is never killed and never relabeled.
+- The status-bar cell renders `#I:#{?@hqt_label,#{@hqt_label},#W}#F`. A window
+  with no `@hqt_label` falls back to its raw name; setting `@hqt_label` gives a
+  tool window a clean label and makes it show nicely in the `Alt+o` switcher too.
+- `new_window()` hardcodes `remain-on-exit on` (so a harness that dies instantly
+  leaves a visible pane). Tool windows want the **opposite** — close when you
+  quit — which is tmux's default. Hence a separate spawn path
+  (`new_aux_window`) rather than a flag on `new_window()`.
+
+So a **tool window** is purely a tmux window: hqt spawns and styles it, then
+forgets it. A **clone** is the opposite — a fully tracked session created through
+the existing `create_session` path, so it appears in the session list on the next
+poll with no special handling.
+
+## Architecture: how Alt+p reaches hqt with the right window
+
+Two tmux facts (verified empirically on tmux 3.6b) shape the bridge:
+
+- **`run-shell` format-expands its command.** `run-shell "echo #{window_name}"`
+  runs `echo hqt-5`.
+- **`display-popup` does NOT expand its command (or its `-e` value).** The fzf
+  popup needs `display-popup` for an interactive terminal, but it receives the
+  literal string `#{window_name}` — useless. And `display-message -p
+  '#{window_name}'` *inside* the popup resolves against the "current" client,
+  which is ambiguous when more than one client is attached (it returned the wrong
+  window in testing).
+
+So the binding bridges through `run-shell` (which expands the name) into a small
+CLI subcommand that bakes the resolved name into the popup as a literal:
+
+```tmux
+bind -n M-p run-shell -b "hqt palette '#{window_name}'"
+```
+
+`run-shell` expands `#{window_name}` → `hqt palette 'hqt-5'`. `hqt palette` then
+builds and runs `display-popup -E "<fzf> | xargs -I{} hqt tool {} hqt-5"`, where
+`hqt-5` is a concrete literal — no further tmux expansion required, and no
+multi-client ambiguity. The selected entry runs `hqt tool <choice> hqt-5`, which
+funnels into the same service code the rest of hqt uses. No IPC, no daemon.
+
+`-b` keeps the tmux server responsive during hqt's ~0.3–0.6s startup.
+
+**TUI note.** From the TUI home window, `#{window_name}` is the TUI window, not a
+session — the palette can't know which session is *highlighted* there. So the
+palette is, by design, for harness/session windows; from the TUI you attach
+(Enter) first, then Alt+p. From a non-session window the palette shows a brief
+hint instead of an unusable menu (see `hqt palette` below). This is the accepted
+trade-off of a single tmux-level key over a TUI-specific palette.
+
+## Components
+
+### 1. Tool registry — `src/hqt/tools.py` (new)
+
+Maps a tool name to its spawn spec:
+
+```python
+@dataclass(frozen=True)
+class Tool:
+    label: str            # base label for @hqt_label, e.g. "nvim"
+    command: list[str]    # [] => tmux default shell (the plain shell)
+
+TOOLS: dict[str, Tool] = {
+    "nvim":    Tool(label="nvim",    command=["nvim"]),
+    "lazygit": Tool(label="lazygit", command=["lazygit"]),
+    "shell":   Tool(label="shell",   command=[]),
+}
+```
+
+`clone` is **not** in this registry — it creates a session, not an aux window —
+and is handled as a distinct path. The per-spawn status label is
+`f"{tool.label} · {project_name}"` so the always-fresh windows stay
+distinguishable.
+
+### 2. `TmuxRunner.new_aux_window(name, cwd, command, label) -> str | None` (new)
+
+The aux-window workhorse. Everything after creation is targeted by **`window_id`**
+(e.g. `@7`), not name — so duplicate tool-window names are never ambiguous.
+
+1. `idx = await self._next_window_index()` (appends at the right).
+2. `new-window -t <session>:<idx> -n <name> -c <cwd> -P -F '#{window_id}'`,
+   appending the joined `command` when non-empty (empty → default shell). Capture
+   the returned `window_id`. **No `remain-on-exit`.**
+3. One atomic `set-option` call (`;`-separated argv) on the `window_id`:
+   `automatic-rename off`, `@hqt_label <label>`, then `*_window_theme_args(wid)`.
+4. `select-window -t <window_id>`.
+5. Return the `window_id`; on `new-window` failure return `None` (logged); on
+   `set-option` failure kill the half-built window and return `None`.
+
+### 3. `TmuxManager.open_aux_window(name, cwd, command, label) -> str | None` (new)
+
+Thin delegate to `runner.new_aux_window`, matching the manager/runner layering.
+
+### 4. `SessionService` methods (new)
+
+```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."""
+    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
+```
+
+```python
+async def open_tool_window(self, session_id: int, tool: str) -> str | None:
+    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, label = project.path, f"{spec.label} · {project.name}"
+    return await self.tmux.open_aux_window(spec.label, cwd, spec.command, label)
+```
+
+`open_tool_window_for_window(window_name, tool)` resolves via
+`session_id_for_window` and raises `ServiceError` when the name is not a session.
+The `shutil.which` pre-check turns a missing `lazygit` into a clean message rather
+than a window that flashes and vanishes.
+
+```python
+async def clone_session_for_window(self, window_name: str) -> CreateSessionResult:
+    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, harness_name, model = sess.project_id, sess.harness.name, sess.model
+    return await self.create_session(project_id, harness_name, nickname=None, model=model)
+```
+
+clone reuses `create_session` wholesale (spawn + capture-retry + DB row). Because
+a tool window or the TUI home window does not resolve to a session, invoking clone
+from there raises `ServiceError` and nothing happens — exactly the "do nothing
+from a vim/shell window" requirement, for free.
+
+### 5. CLI — `src/hqt/cli.py`
+
+Two subcommands, sharing one `_build_session_service()` helper that mirrors
+`HqtApp.on_mount`'s wiring (`TmuxRunner` → `TmuxManager` → `SessionService(factory,
+tmux, discover_harnesses())`).
+
+- **`hqt tool <name> <window>`** — dispatch: `name == "clone"` →
+  `clone_session_for_window(window)`; otherwise →
+  `open_tool_window_for_window(window, name)`. On `ServiceError`, print to stderr
+  and exit 1 (the popup surfaces it briefly).
+- **`hqt palette <window>`** — if `session_id_for_window(window)` is `None`, run
+  `tmux display-message "hqt: open the tool palette from a harness window"` and
+  return (graceful no-op from a tool/TUI window). Otherwise run
+  `tmux display-popup -E ...` whose command is:
+
+  ```
+  printf 'nvim\nlazygit\nshell\nclone\n' \
+    | fzf --reverse --no-info --prompt='tool  ' --pointer='▌' --color='<frappé>' \
+    | xargs -r -I{} hqt tool {} '<window>'
+  ```
+
+  The window name is baked in as a `shlex.quote`d literal. fzf colors match the
+  `Alt+o` switcher (Catppuccin Frappé). Cancelling fzf (`xargs -r`) runs nothing.
+
+### 6. `~/.tmux.conf` (user-owned keybindings file)
+
+One global binding, in the existing comment style:
+
+```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.
+bind -n M-p run-shell -b "hqt palette '#{window_name}'"
+```
+
+## Data flow
+
+`Alt+p` → `run-shell` expands `#{window_name}` → `hqt palette hqt-5` →
+[not a session? → tmux message, done] → `display-popup` fzf → selection →
+`hqt tool <choice> hqt-5` →
+
+- tool → resolve project → `new_aux_window(cwd=project, command=tool, label)` →
+  styled window appended at the next index → `select-window`. Never written to the
+  DB, so the 3s poll ignores it and hqt never prunes it.
+- clone → resolve project/harness/model → `create_session(...)` → fresh `hqt-<id>`
+  window; the running TUI's next poll lists it.
+
+## Error handling
+
+- Missing binary → `ServiceError` → CLI stderr + exit 1 (popup shows it briefly).
+- Not a session window (tool window, TUI home, unrelated window) → `hqt palette`
+  shows a one-line tmux hint and never opens the menu; a direct `hqt tool` raises
+  `ServiceError`.
+- `new-window` / `set-option` failure → logged; aux path returns `None` and cleans
+  up a half-built window.
+
+## Testing
+
+Mirror existing patterns:
+
+- **Service** (`MagicMock(spec=TmuxManager)`): `open_tool_window` raises on
+  unknown tool / missing binary (monkeypatched `shutil.which`) / missing session;
+  passes the right `cwd`/command/label on success. `session_id_for_window`
+  resolves a known name and returns `None` for an unknown one.
+  `clone_session_for_window` calls `create_session` with the source session's
+  project/harness/model and raises for an unknown window.
+- **Runner** (`AsyncMock` `_exec` side-effect queue): `new_aux_window` issues
+  next-index → `new-window -P -F` → `set-option`(by `window_id`) → `select-window`
+  in order, **never** sets `remain-on-exit`; covers a command tool and the empty
+  shell.
+- **CLI** (`CliRunner`, monkeypatched service + `subprocess.run`): `hqt tool`
+  dispatches clone vs. tool and maps `ServiceError` → exit 1; `hqt palette` runs
+  `display-message` for a non-session window and `display-popup` (command
+  containing the entries and the quoted window) for a session window.
+
+## Out of scope
+
+- No TUI-side palette or per-tool TUI bindings — Alt+p (tmux) is the only trigger
+  (decision: one consistent key, no Ctrl+P clobbering nvim/shell). The cost is
+  that launching from the TUI home window is a no-op hint; attach first.
+- No reuse/dedupe of tool windows (always spawn new).
+- No DB rows or status-glyph logic for tool windows.
+- No configurability beyond the `TOOLS` registry and the palette entry list.
+- clone copies project/harness/model only — not nickname, MCP, or skill overrides
+  (a fresh sibling, not a deep copy).