diff --git a/cmd_chat/agent/bridge.py b/cmd_chat/agent/bridge.py index 2b116e6..775a9e2 100644 --- a/cmd_chat/agent/bridge.py +++ b/cmd_chat/agent/bridge.py @@ -662,14 +662,66 @@ class AgentBridge(Client): await worker return "".join(parts) + # Reconnect policy. A session that stays up at least _RECONNECT_STABLE_SECS + # is treated as healthy: the next drop resets backoff so a one-off blip + # reconnects fast, while only rapid repeated failures back off (capped). + _RECONNECT_STABLE_SECS = 30.0 + _RECONNECT_MAX_BACKOFF = 30.0 + # Keepalive: ping every 20s but tolerate a slow pong, since a heavy CPU-only + # Ollama generation can briefly starve the event loop. A real drop is caught + # by the reconnect loop, so a forgiving timeout just avoids needless churn. + _PING_INTERVAL = 20.0 + _PING_TIMEOUT = 60.0 + async def run_async(self) -> None: - self.srp_authenticate() + """Join the room and serve forever, reconnecting on any unexpected drop. + + The agent used to hold a single websocket with no retry: any close — + server restart, idle/ping reap, laptop sleep, a transient network blip — + ended the serve loop, so ``run_async`` returned and the process exited + silently. The agent then vanished from the roster with no ``/ai stop`` and + no goodbye. We now wrap the connection in a backoff-reconnect loop. The + server frees our session + name when a socket drops, so each attempt + re-runs SRP to mint a fresh token before reopening. Only Ctrl-C / process + kill (KeyboardInterrupt / CancelledError — the latter is how ``/ai stop`` + terminates us) ends the loop.""" + backoff = 1.0 + first = True + while True: + loop = asyncio.get_running_loop() + started = loop.time() + try: + await self._connect_and_serve(reconnect=not first) + except (KeyboardInterrupt, asyncio.CancelledError): + raise # intentional shutdown — never reconnect + except (websockets.ConnectionClosed, OSError) as e: + self.info(f"connection lost ({type(e).__name__}); reconnecting…") + except Exception as e: # noqa: BLE001 — auth/transient error: retry, don't die + self.info(f"agent loop error ({type(e).__name__}: {e}); reconnecting…") + else: + self.info("disconnected; reconnecting…") # clean close under us + if loop.time() - started >= self._RECONNECT_STABLE_SECS: + backoff = 1.0 # the session was healthy → reconnect promptly + first = False + await asyncio.sleep(backoff) + backoff = min(backoff * 2, self._RECONNECT_MAX_BACKOFF) + + async def _connect_and_serve(self, reconnect: bool) -> None: + """One connection lifecycle: (re)authenticate, open the socket, announce, + then serve frames until the socket closes. Raises on any drop so the outer + ``run_async`` loop can decide whether to reconnect.""" + self.srp_authenticate() # fresh session each attempt; the old token dies on a drop url = f"{self.ws_url}/ws/chat?user_id={self.user_id}&ws_token={self.ws_token}" - self.info(f"agent '{self.name}' connecting via {self.provider.name}/{self.provider.model}…") - async with websockets.connect(url, ssl=self._ws_ssl_context()) as ws: + verb = "reconnecting" if reconnect else "connecting" + self.info(f"agent '{self.name}' {verb} via {self.provider.name}/{self.provider.model}…") + async with websockets.connect( + url, ssl=self._ws_ssl_context(), + ping_interval=self._PING_INTERVAL, ping_timeout=self._PING_TIMEOUT, + ) as ws: self.running = True announce = ( - f"{self.name} (ai) online — {self.provider.name}/{self.provider.model}. " + f"{self.name} (ai) {'back online' if reconnect else 'online'} — " + f"{self.provider.name}/{self.provider.model}. " f"Ask me with /ai ; /ai {self.name} ! to act in the sandbox." ) await ws.send(self.room_fernet.encrypt(announce.encode()).decode()) @@ -685,44 +737,57 @@ class AgentBridge(Client): embed_task.cancel() async def _serve(self, ws) -> None: + """Drain frames until the socket closes. Each frame is handled under a + guard so a single malformed/poisoned frame — or a provider/handler error — + can never unwind the serve loop and drop the agent. A closed socket or a + cancellation propagates up to the reconnect loop / shutdown.""" async for raw in ws: if not self.running: break try: - data = json.loads(raw) - except json.JSONDecodeError: - continue - mtype = data.get("type") - if mtype == "init": - self.users = data.get("users", []) - self._seed_transcript(data.get("messages", [])) - continue - if mtype == "roster": - self.users = data.get("users", []) - continue - if mtype != "message": - continue - msg = self.decrypt_message(data.get("data", {})) - text = msg.get("text", "") - sender = msg.get("username", "?") - if sender == self.name: - continue # never react to our own messages - if text.startswith('{"_'): - self._handle_control(text) # track ACL grants; ignore other ctrl frames - continue - question = self._addressed_question(text) - if question is None: - # keep a short rolling transcript for context on future asks, - # and feed the line to long-term semantic memory - captured = Msg("user", f"{sender}: {text}") - self.transcript.append(captured) - self.transcript = self.transcript[-(self.context_window * 2):] - self._remember(captured) - elif question.startswith("!"): - self.info(f"{sender} → /ai !sbx: {question[1:].strip()}") - await self._run_in_sandbox(ws, question[1:].strip(), sender) - elif question.strip().lower() == "confirm": - await self._confirm_pending(ws, sender) - else: - self.info(f"{sender} → /ai: {question}") - await self._answer(ws, question, sender) + await self._handle_frame(ws, raw) + except (websockets.ConnectionClosed, asyncio.CancelledError): + raise # socket gone / shutting down → let run_async decide + except Exception as e: # noqa: BLE001 — one bad frame must not kill the agent + self.info(f"skipped frame after error ({type(e).__name__}: {e})") + + async def _handle_frame(self, ws, raw) -> None: + """Decode and act on one server frame (init/roster/message).""" + try: + data = json.loads(raw) + except json.JSONDecodeError: + return + mtype = data.get("type") + if mtype == "init": + self.users = data.get("users", []) + self._seed_transcript(data.get("messages", [])) + return + if mtype == "roster": + self.users = data.get("users", []) + return + if mtype != "message": + return + msg = self.decrypt_message(data.get("data", {})) + text = msg.get("text", "") + sender = msg.get("username", "?") + if sender == self.name: + return # never react to our own messages + if text.startswith('{"_'): + self._handle_control(text) # track ACL grants; ignore other ctrl frames + return + question = self._addressed_question(text) + if question is None: + # keep a short rolling transcript for context on future asks, + # and feed the line to long-term semantic memory + captured = Msg("user", f"{sender}: {text}") + self.transcript.append(captured) + self.transcript = self.transcript[-(self.context_window * 2):] + self._remember(captured) + elif question.startswith("!"): + self.info(f"{sender} → /ai !sbx: {question[1:].strip()}") + await self._run_in_sandbox(ws, question[1:].strip(), sender) + elif question.strip().lower() == "confirm": + await self._confirm_pending(ws, sender) + else: + self.info(f"{sender} → /ai: {question}") + await self._answer(ws, question, sender) diff --git a/docs/spec-native-harness.md b/docs/spec-native-harness.md new file mode 100644 index 0000000..39dcc6f --- /dev/null +++ b/docs/spec-native-harness.md @@ -0,0 +1,182 @@ +# hack-house → Native lightweight harness (strip Goose) — Spec + +> **Status:** Draft v1 · **Date:** 2026-06-08 +> **Scope:** Replace the **Goose** agentic harness on the `/ai !` +> path with a **lightweight, host-side, Ollama-native harness**, and **strip all +> Goose integration** from the codebase + bootstrap scripts. +> **Baseline reviewed:** `cmd_chat/agent/bridge.py`, `cmd_chat/agent/__main__.py`, +> `cmd_chat/agent/providers.py`, `hh/src/sbx.rs`, `hh/src/app.rs`, +> `hh/scripts/bootstrap.sh`, `hh/scripts/bootstrap-ai.sh`, +> `hh/scripts/sandbox-bootstrap.sh` @ `main` (f93c8c5). +> **Supersedes:** `docs/spec-goose-harness.md` (Goose harness portion only — the +> Podman backend it also introduced **stays**). +> **Related (do NOT touch):** `headroom/` references Goose for an unrelated CLI +> wrapper experiment; out of scope here. + +--- + +## 0. Why + +| Problem with Goose | Consequence on this project | +|---|---| +| Agentic loop = **N sequential model calls** (`--max-turns 15`) | On a CPU-only box (i5-8350U, no GPU) each call is already slow; Goose multiplies per-call latency by N → "takes forever". | +| Heavyweight external **Rust binary** baked into every sandbox | Extra install in `sandbox-bootstrap.sh` + host install in `bootstrap-ai.sh`; a moving release dependency. | +| In-container Goose must reach **host Ollama** | The documented **slirp4netns loopback bug**: rootless Podman's `host.containers.internal` resolves to the host LAN IP and can't reach a `127.0.0.1`-bound Ollama. Forced the `--network=slirp4netns:allow_host_loopback=true` + `OLLAMA_HOST=10.0.2.2` workaround in `sbx.rs`. | +| Opaque: we relay raw stdout, no control of the tool schema | Hard to bound, hard to render, hard to reason about. | + +**What we already have that works decently** (commit `47019dd`, still alive today +as `_run_simple`): a **one-shot keystroke injector** — *one* model call → a +```` ```sh ```` block → typed into the shared sandbox PTY via the existing +`_sbx:input` frames, guarded by a destructive-command regex + blast-radius caps, +echoed to the room first as an audit trail. It is fast (single call), needs **no +external binary**, and reuses transport that already exists. Its only real +limitation: it types **blind** — it never reads command output back. + +**Decision:** keep the proven injector as the floor, optionally add a **bounded, +host-side, Ollama-native tool-calling loop** as the ceiling, and remove Goose +entirely. + +--- + +## 1. Native harness design + +### 1.1 Key architectural shift: host-side model, container-side execution + +Goose ran the *whole* loop **inside** the sandbox (model calls + command +execution), which is why the container needed to reach host Ollama. The native +harness splits these: + +- **Model call → host.** The bridge (co-located with the broker, already on the + host) calls Ollama on the host directly via `providers.py`. No container→host + Ollama hop. +- **Command execution → sandbox.** Commands run in the sandbox via the engine the + broker advertises (`_sbx:status` → `engine` + `name`), using the same + ` exec …` plumbing Goose used for its probe. + +**Consequence:** the entire in-container Ollama gateway surface +(`--add-host=host.docker.internal:host-gateway`, `--network=slirp4netns:allow_host_loopback=true`, +in-container `OLLAMA_HOST`) becomes **dead** and is removed — **the slirp4netns +loopback bug ceases to exist.** + +### 1.2 Two harness levels (both host-side model, grant-gated) + +The grant tiers from the Goose spec are unchanged (`/grant` is the switch): +**not granted → advisory chat-only**; **granted → act in the sandbox**. What +changes is *how* the granted path acts: + +| Level | What it does | Reads output? | Model calls | Default | +|---|---|---|---|---| +| **`simple`** | One model call → ```` ```sh ```` block → type into the shared PTY (`_sbx:input`). The proven `47019dd` injector. | No (blind) | 1 | fallback | +| **`native`** | Bounded tool-calling loop: Ollama `/api/chat` with a small `tools` schema; bridge execs each tool call in the sandbox, captures stdout, feeds it back; cap at **3–5 turns**. | Yes | ≤ turns | **default** | + +`native` degrades to `simple` automatically when the model doesn't advertise tool +support (so it is default but never load-bearing — same safety property Goose +had, minus the binary). + +### 1.3 The `native` loop (Ollama-native function calling) + +`qwen2.5` (our default) supports function calling through Ollama's `/api/chat` +`tools` field — we just aren't using it yet (`providers.py:104/119` send no +`tools`). Add an opt-in tool-calling completion to `OllamaProvider`: + +``` +POST /api/chat +{ "model": …, "messages": [...], "tools": [ ], "stream": false } +→ message.tool_calls: [ {function:{name, arguments}}, … ] +``` + +**Minimal tool schema** (the whole surface — deliberately tiny): + +| Tool | Args | Maps to (sandbox exec) | +|---|---|---| +| `run_shell` | `command: string` | ` exec sh -c ""`, capture stdout+stderr+rc | +| `write_file` | `path: string, content: string` | heredoc write via exec (no host fs) | +| `read_file` | `path: string` | `cat` via exec | + +Loop (per `!task`, granted): +1. Seed messages with `SANDBOX_SYSTEM` + the task + recent transcript window. +2. Call Ollama with `tools`. If `tool_calls` present → run each through the + **same guards** (`DESTRUCTIVE` regex + `/confirm`, `MAX_COMMANDS`/`MAX_BYTES`), + exec in the sandbox, append a `tool` role message with captured output. +3. Repeat until the model returns a plain answer or the **turn cap** (3–5) is hit. +4. Stream progress to chat (reuse `_send_stream`); post the final line; append a + capped summary to the transcript. + +**Containment unchanged from the Goose spec §2.1:** execution reach == the +sandbox namespace. Container/VM backends isolate it; `local` is the host by +explicit, warned choice. Output is byte-capped + throttled. Room text stays +untrusted; the sandbox is the blast radius. + +### 1.4 Why this fixes "takes forever" + +- **We own the turn budget** (3–5, not 15) → bounded worst case. +- **Model runs host-side** with the CPU-tuned `OllamaProvider` knobs already in + place (`num_ctx`/`num_thread`/`num_predict`, plus the `qwen2.5-coder` code path). +- A trivial task that the model nails in one tool call costs ~1 round-trip — same + ballpark as the old one-shot injector. + +--- + +## 2. Nomenclature + +Drop the Goose-implicit grammar (where `plain` *disabled* Goose and there was no +positive keyword). New, symmetric harness selector: + +``` +/ai start [profile|model] [native|simple] [allow] +``` + +- **harness word** (optional): `native` (default) or `simple`. `plain` kept as a + back-compat alias for `simple`. +- **`allow`** (unchanged): pre-grant sandbox drive on spawn (owner only). +- CLI: `--harness {native,simple}` on `python -m cmd_chat.agent` + (replaces `{goose,simple}`); drop `--goose-max-turns`, add + `--max-turns` (default 5) for the `native` loop. +- `models.toml`: `harness = "native"|"simple"` (was `"goose"|"simple"`). + +--- + +## 3. Strip plan — file by file + +> `headroom/**` is a different project; **excluded**. + +| File | Change | +|---|---| +| `cmd_chat/agent/bridge.py` | Remove `_run_goose`, `_goose_argv`, `_goose_present`, `_goose_present_cache`, and the `GOOSE_*` consts. Rewrite `_run_in_sandbox`: granted → `native` loop (new `_run_native`) with `simple` fallback; not granted → `_advise` (unchanged). Default `self.harness = "native"`. Keep `_run_simple`, `_advise`, `_inject`, `_extract_commands`, `_confirm_pending`, the destructive gate, and blast caps. Keep `_handle_control` reading `_sbx:status` `engine`+`name` (the `native` exec path uses them). | +| `cmd_chat/agent/providers.py` | Add tool-calling completion to `OllamaProvider` (`complete_with_tools(system, messages, tools) -> (text, tool_calls)`); add a capability probe (does the model accept `tools`). Chat/`stream` paths unchanged. | +| `cmd_chat/agent/__main__.py` | `--harness {native,simple}` (was `{goose,simple}`); remove `--goose-max-turns` + `GOOSE_MAX_TURNS` import; add `--max-turns`. Default harness `native`. | +| `hh/src/app.rs` | Update the `/ai start` parser (≈2628–2651): accept `native|simple` (+ `plain` alias) instead of just `plain`; pass `--harness native|simple`. Refresh the comment at 3248. | +| `hh/src/sbx.rs` | Remove the in-container Ollama gateway: the `--add-host=host.docker.internal:host-gateway` / `--network=slirp4netns:allow_host_loopback=true` block (≈780–792) and the in-container `OLLAMA_HOST` set in `dk_bootstrap` (≈1249). Drop the Goose-reaches-host comments (688, 780, 1249). **Verify** nothing else depends on the gateway before deleting. | +| `hh/scripts/bootstrap.sh` | Remove `goose` from the prereq probe loop (line 52). | +| `hh/scripts/bootstrap-ai.sh` | Remove the entire Goose install block + `--no-goose` flag + `GOOSE_INSTALLER_URL` + host `~/.config/goose/config.yaml` writer + the `goose_bin()` helper and its status lines. Update header/usage. | +| `hh/scripts/sandbox-bootstrap.sh` | Remove the Goose section (≈53–80): binary install + container-side `~/.config/goose/config.yaml`. (Mirrors the noVNC strip already done.) | +| `models.toml` | `harness` values doc: `goose` → `native`. | +| `docs/spec-goose-harness.md` | Add a banner: **Harness section superseded by `spec-native-harness.md`; the Podman backend portion still stands.** | + +**Keep (not Goose-specific):** Podman backend, `_sbx:status` `engine`+`name` +fields (the `native` exec path needs them), the grant/ACL gate, `_advise`, +`_run_simple`, destructive gate + blast caps, CPU tuning + `qwen2.5-coder` path. + +--- + +## 4. Phasing + +| Phase | Scope | +|---|---| +| **0 (done)** | Fix the agent disconnect bug (reconnect loop + per-frame shield in `bridge.py`). | +| **1** | Strip Goose: `bridge.py`/`__main__.py` harness plumbing, `app.rs`/`sbx.rs`, bootstrap scripts, `models.toml`, supersede banner. Default falls back to `simple` until Phase 2 lands. `cargo check` + `py_compile` clean. | +| **2** | Implement the `native` tool-calling loop: `OllamaProvider.complete_with_tools` + tool probe; `_run_native` with the 3-tool schema, turn cap, guards, exec-capture, stream→chat, `simple` fallback. | +| **3** | Bench `native` vs the old `simple`/Goose on this box (latency, success on a few canonical `!task`s); tune turn cap + tool schema; update help/usage + memory. | + +--- + +## 5. Open questions + +1. **Tool schema breadth:** start with `run_shell` only (closest to the proven + injector, simplest), or ship `write_file`/`read_file` from the start? +2. **Turn cap default:** 3 (snappy) vs 5 (more self-correction) — decide after the + Phase 3 bench. +3. **`native` for non-Ollama providers** (Anthropic/OpenAI also do tool calls): + in scope now, or Ollama-only first and treat cloud as a later generalization? +4. **`simple` fate:** keep indefinitely as the zero-dependency fallback (recommended), + or retire once `native` is proven?