diff --git a/.gitignore b/.gitignore index a39a29a..e770a69 100644 --- a/.gitignore +++ b/.gitignore @@ -30,3 +30,6 @@ .claude/worktrees/ # Local PRDs, planning notes, and extracted artifacts; locally controlled. .local/ +# Per-directory tool caches the impeccable skill drops beside the sources it +# reads; they reappear on every run, so ignore rather than clean repeatedly. +.impeccable/ diff --git a/.markdownlint-cli2.jsonc b/.markdownlint-cli2.jsonc index f3cca05..4a53178 100644 --- a/.markdownlint-cli2.jsonc +++ b/.markdownlint-cli2.jsonc @@ -9,6 +9,15 @@ // Lint every doc by default, so a bare `markdownlint-cli2` mirrors CI // (whose action passes the same glob explicitly). "globs": ["**/*.md"], - // CLAUDE.md is only an `@AGENTS.md` import pointer, not prose. - "ignores": ["target/**", "node_modules/**", "CLAUDE.md"] + // CLAUDE.md is only an `@AGENTS.md` import pointer, not prose. `.personal/` + // and `.local/` are gitignored working notes: CI never sees them (fresh + // clone), so linting them locally only reports errors no one can act on and + // teaches you to skim past the summary line. + "ignores": [ + "target/**", + "node_modules/**", + "CLAUDE.md", + ".personal/**", + ".local/**" + ] } diff --git a/.wrap.md b/.wrap.md new file mode 100644 index 0000000..9f84681 --- /dev/null +++ b/.wrap.md @@ -0,0 +1,60 @@ +# wrap configuration + +Inferred during the first unconfigured `/wrap` pass (the MCP control-surface +milestone) and written down so the next one does not re-derive it. Correct it +in place when a location moves — a stale entry here silently sends the pass to +the wrong file. + +## Canonical locations + +- index: `ROADMAP.md` — feature status (MoSCoW from `docs/PRD.md` §5). The + table that must match reality; a shipped feature is ticked here with an + ADR-style note explaining *why*, not just *what*. +- lessons: `AGENTS.md` — the file every contributor and agent reads first. + Durable, repo-wide rules go in its **Quality bar** section. +- skills: `.claude/skills/` — operational procedures (how to run something), + as opposed to rules about the code. +- decisions: this repo has no `adr/` directory. A cross-cutting decision is + recorded as prose in `docs/ARCHITECTURE.md` (structure) or `docs/PRD.md` + (product), and a per-issue investigation as a note in `docs/notes/`. +- feature verdicts: `.personal/feature-torture/reports/.md` (gitignored, + local) — cited from the `ROADMAP.md` entry. +- journal: none. Shipped history lives in `ROADMAP.md` entries and git. + +## Verification commands + +Mirror of the CI gates (see `docs/CI.md`). CI runs each only when its file +category changed, but a wrap pass touching both docs and code should run all +of them. + +- blocking: `cargo fmt --all --check` +- blocking: `cargo clippy --workspace --all-targets -- -D warnings` +- blocking: `cargo test --workspace` +- blocking: `markdownlint-cli2` +- blocking: `just check-deps` — the hexagonal crate dependency rule +- blocking: `just check-arch` — intra-crate module seams + OS-cfg containment +- advisory: `cargo deny check` — only if cargo-deny is installed +- advisory: `cargo machete` — unused deps; only if installed + +## Disposable zones + +- `target/` +- `.impeccable/` — per-directory tool caches, gitignored; they regenerate on + every run, so ignore them rather than deleting them each pass +- `.claude/worktrees/` — per-machine working copies. Remove a worktree only + after confirming its PR merged **and** the directory is clean; the branch's + commits will not appear on `main` because PRs are squash-merged, so check + the PR state, not `git log main..branch`. +- `.DS_Store` + +## Commit conventions + +- format: `(): ` +- type: `feat` / `fix` / `refactor` / `docs` / `chore` / `ci` / `test` +- scope: the crate or module the change lands in — `app`, `core`, `pty`, + `scan`, `mcp`, `agents`, `roadmap` +- Issue refs (`#NN`) belong in the commit **body** and in PR descriptions, + never in code comments, doc-comments or test names — see `CONTRIBUTING.md`. +- No AI/Claude signature or "Generated with" trailer, anywhere. +- Bookkeeping (a `ROADMAP.md` tick, a doc reconciliation) rides **with** the + feature PR it describes; it never becomes a standalone follow-up PR. diff --git a/AGENTS.md b/AGENTS.md index a7d5c39..1d0cd20 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -108,6 +108,39 @@ idle→recording state machine (frames are the time proxy — no clock), and the `gif` encoder runs on a dedicated thread in `app` (`crates/app/src/record.rs`) so the UI — and the recording — stay smooth. +### Driving termherd over MCP (#90) + +A Claude session **launched from termherd** gets an in-process MCP server wired +into its `mcpServers` at spawn (loopback, per-session token) — so it can read +and drive the workspace it runs in. This is the richer sibling of the capture +dump above: same `WorkspaceSnapshot` model, live instead of a file. + +**Settled.** Six slices shipped: `list_sessions` + `snapshot` (perception), +`open_session` / `split_pane` / `focus_pane` / `rename_tab` / `close_pane` / +`run_in_session` (action), `wait_for_status` + `read_terminal` +(synchronisation). The loop they exist to serve is **act → wait → observe**: +`run_in_session` returns immediately, so synchronise with `wait_for_status` +and then `read_terminal`. Do **not** poll `snapshot` in a loop — it races the +transition you are watching for, which is why the wait rung exists. + +Sessions are addressed by a stable `handle` (the runtime `SessionId`), never +the Claude `resume_id`, which re-keys on a fork / plan-accept (Q6). Every call +is `tokio::timeout`-bounded in `BridgeHandle::call` (Q7). + +Where it lives: tools in `crates/app/src/mcp/handler.rs`, transport in +`shell::bridge`, and the shell's answers in `shell::serve` — the one place an +external caller meets `core::App`. `core` has no MCP awareness at all; every +mutation goes through an existing `Event`. + +**Still open.** `F-mcp-screenshot` (#215) and `F-mcp-agent-loop` (#196, scope +question open — may be cut). Both tracked on the #90 epic. + +**Looks like a contradiction, is not.** `docs/ARCHITECTURE.md` §15 lists an +`mcp` crate as *deferred (Unsure)*. That is a **different feature** — +`F-mcp-ide-bridge`, termherd as an MCP *client* of Claude's IDE bridge — and it +really is unbuilt. The surface described here runs the other way round +(termherd is the server) and lives in `app`, not in a crate of its own. + ## Architecture — the dependency rule Hexagonal workspace. The single most important invariant: @@ -125,12 +158,20 @@ app ──► core ◄── adapters (adapters depend on core, nev `crates/core/Cargo.toml`). - `crates/claude` — pure Claude CLI format codec (path encode/derive, JSONL digest, OSC decode). Same strict lint profile as `core`. -- `crates/app` — iced GUI shell. Currently a tracing + single-instance stub; - M1+ will construct adapters in `main()` and inject them into `core::App`. +- `crates/app` — iced GUI shell. Constructs the adapters in `main()` and + injects them into `core::App`; owns the one effect executor + (`shell::effects`) and the MCP control surface (`app::mcp` + `shell::bridge` + / `shell::serve`). - `crates/scan` — filesystem discovery adapter (walks `~/.claude/projects` via the `claude` codec; implements `core::ports::ProjectScanner`). -- Remaining adapters land per `docs/ARCHITECTURE.md` §15: `pty` (M2), - `store` (Should, PRD rev. 4), optional `mcp` (Unsure). +- `crates/pty` — terminal adapter (`portable-pty` + `alacritty_terminal`); + implements `core::ports::PtyHost`. +- `store` (Should, PRD rev. 4) is the one adapter still unbuilt. The **MCP + control surface shipped** as a module inside `app`, not as its own crate — + it is a bridge into the shell, not a port `core` calls out through. The + separate `mcp` *crate* sketched in `docs/ARCHITECTURE.md` §15 is a different + feature (`F-mcp-ide-bridge`: termherd as an MCP **client** of Claude's IDE + bridge), still unbuilt. When adding code, ask: *which crate does this belong in?* If the answer is "`core` should call this adapter directly," the answer is wrong — add a port @@ -179,6 +220,15 @@ exists). Do not relax them locally. complexity. A function that exceeds it on purpose (a flat dispatcher / layout builder) carries a local `#[allow(clippy::too_many_lines)]` with a rationale, never a relaxed global threshold. +- **An invariant expressed twice will drift — extract the predicate.** Two + call sites deciding "has this settled?" with hand-written conditions is a + bug waiting on the first edit that touches one of them. It bit the + `wait_for_status` rung: one site treated a session exit as settling a wait, + the other only compared against the requested statuses, so a wait placed + after a crash parked until the caller's timeout. Both now go through one + `settles()` predicate. A doc-comment asserting the rule is *not* enforcement + — the comment describing the correct behaviour sat directly above the code + that broke it. ## Conventions diff --git a/README.md b/README.md index d09313e..fe3060c 100644 --- a/README.md +++ b/README.md @@ -141,13 +141,40 @@ description (the same card the sidebar shows). ## MCP control surface (experimental) -`termherd-mcp` is a small [MCP](https://modelcontextprotocol.io) server that -exposes termherd's own configuration to a Claude session, so you can ask "what -can I configure here?" — or "switch me to a light theme" — from inside the -conversation termherd already hosts. It exposes two tools, `list_options` -(read) and `set_option` (write), plus the option **schema** as a resource, all -reflecting `~/.termherd/settings.json`. Workspace orchestration (open session, -split, focus, …) is a planned follow-up (`F-mcp-control-surface`, [#90]). +termherd exposes itself to the Claude sessions it hosts over +[MCP](https://modelcontextprotocol.io), so a session can read and drive the +workspace it is running in (`F-mcp-control-surface`, [#90]). There are **two +surfaces**, and which one you get depends on how the session was started. + +### The live bridge (automatic) + +A Claude session **launched from termherd** is wired to an in-process server on +loopback, with a per-session token, injected into its `mcpServers` at spawn — +nothing to configure. It exposes the running workspace: + +| Tool | What it does | +| --- | --- | +| `list_sessions` | every live session with its stable `handle` | +| `snapshot` | the whole state — config, sidebar, tabs and panes; filterable, no terminal text by default | +| `open_session` · `split_pane` · `focus_pane` · `rename_tab` · `close_pane` | workspace actions, each reporting the resulting focus | +| `run_in_session` | type into a terminal (returns immediately) | +| `wait_for_status` | block until a session goes idle / wants attention | +| `read_terminal` | one pane's visible text | + +The loop that makes it useful is **act → wait → observe**: `run_in_session`, +then `wait_for_status`, then `read_terminal`. Sessions are addressed by a +stable `handle` that survives a Claude-side session re-key. + +Screenshots ([#215]) and an opt-in agent-drives-agent loop ([#196]) are the +remaining follow-ups. + +### The stdio server (manual) + +`termherd-mcp` is a separate small binary that exposes termherd's own +**configuration** — so you can ask "what can I configure here?", or "switch me +to a light theme", from any Claude session. Two tools, `list_options` (read) +and `set_option` (write), plus the option **schema** as a resource, all +reflecting `~/.termherd/settings.json`. It speaks JSON-RPC over stdio. Register it with Claude Code by adding it to your `mcpServers` config (point `command` at the built binary): @@ -163,6 +190,8 @@ It speaks JSON-RPC over stdio. Register it with Claude Code by adding it to your Build the binary with `cargo build -p termherd-mcp` (it lands in `target/`). [#90]: https://github.com/Termherd/termherd/issues/90 +[#196]: https://github.com/Termherd/termherd/issues/196 +[#215]: https://github.com/Termherd/termherd/issues/215 ## Test diff --git a/crates/app/src/mcp/handler.rs b/crates/app/src/mcp/handler.rs index e48b168..d7a2a44 100644 --- a/crates/app/src/mcp/handler.rs +++ b/crates/app/src/mcp/handler.rs @@ -229,14 +229,18 @@ impl TermherdMcp { self.act(Action::Close { pane }).await } - /// Type text into a session's terminal without waiting. → `run_in_session`. + /// Type text into a session's terminal; the wait is a separate call. + /// → `run_in_session`. #[tool( name = "run_in_session", description = "Type text into a session's terminal, as if typed at the \ - keyboard — does not wait for it to finish (read the result \ - with `snapshot`'s scoped terminal text). Include a trailing \ - newline in `text` to submit a command. Args: `session` (its \ - stable handle), `text`. Returns the `focused_handle`." + keyboard. Returns as soon as the text is sent — it does \ + not wait for the command to finish. To act on the result, \ + follow with `wait_for_status` then `read_terminal`; do \ + not poll `snapshot`, which races the transition you are \ + watching for. Include a trailing newline in `text` to \ + submit a command. Args: `session` (its stable handle), \ + `text`. Returns the `focused_handle`." )] async fn run_in_session( &self, diff --git a/crates/app/src/shell/bridge.rs b/crates/app/src/shell/bridge.rs index a2e3fcb..5f593f5 100644 --- a/crates/app/src/shell/bridge.rs +++ b/crates/app/src/shell/bridge.rs @@ -125,8 +125,9 @@ pub enum Action { /// lone pane closes its whole tab (core collapses to `close_tab`, killing the /// PTY). → `[RevealPane +] CloseFocusedPane`. Close { pane: Option }, - /// Type `bytes` into a session's PTY without waiting (waiting is a later - /// rung). → `Event::TerminalInput`. + /// Type `bytes` into a session's PTY without waiting; a caller that needs + /// to synchronise follows with [`Request::WaitForStatus`]. + /// → `Event::TerminalInput`. Run { session: u64, bytes: Vec }, } diff --git a/crates/app/src/shell/orchestrate.rs b/crates/app/src/shell/orchestrate.rs index e7911e2..304b5a9 100644 --- a/crates/app/src/shell/orchestrate.rs +++ b/crates/app/src/shell/orchestrate.rs @@ -107,7 +107,8 @@ impl Shell { (self.applied(), task) } - /// Type bytes into a session's PTY without waiting (waiting is a later rung). + /// Type bytes into a session's PTY without waiting; synchronising is a + /// separate request (`WaitForStatus`), served in `shell::serve`. /// Rejects an unknown handle, so a stale target can't misfire into a live /// terminal. fn act_run(&mut self, session: u64, bytes: Vec) -> (ActionOutcome, Task) { diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 2564e42..5435c1f 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -94,8 +94,11 @@ structurally prevents a new god-object. - `core` defines port traits (`SessionStore`, `PtyHost`, `ProjectScanner`, `Clock`, `FileSystem`) and never names a concrete adapter. -- `mcp` exists in the workspace only if/when the Unsure bet is taken; nothing in - the v1 path imports it. +- ~~`mcp` exists in the workspace only if/when the Unsure bet is taken; nothing + in the v1 path imports it.~~ **Superseded.** This still holds for the `mcp` + *crate* below (§15, `IdeBridge` — termherd as a **client**). It does not hold + for the MCP **control surface** (`F-mcp-control-surface`, #90), which shipped + in the v1 path as a module inside `app` — see §8. - The rule is CI-enforceable, so "main.js → 1,461 LOC" cannot recur. ## 5. The headless core (the quality keystone) @@ -191,6 +194,14 @@ Each adapter implements a `core` port and fixes a specific v0.0.30 defect. server with a `~/.claude/ide` lock and JSON-RPC, `tokio::timeout` on every round-trip (Q7). Not in the v1 path. + **Not to be confused with the shipped MCP control surface** (#90). That one + runs the other way round — termherd is the *server* and the Claude sessions + it hosts are the clients — and it lives in `app` (`app::mcp` over + `shell::bridge` / `shell::serve`), not in a crate of its own, because it is a + bridge *into* the shell rather than a port `core` calls *out* through. The + `tokio::timeout`-on-every-round-trip rule (Q7) is shared: it is enforced in + `BridgeHandle::call`. + ## 8. GUI shell (iced) `crates/app` is intentionally thin: