From 50f0f582dc198503dfc9606c59b2aa00b277efa0 Mon Sep 17 00:00:00 2001 From: Bastien Gallay Date: Sat, 25 Jul 2026 22:35:02 +0200 Subject: [PATCH 1/5] fix(app): run_in_session pointed agents at the poll-race the wait rung replaced MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The tool description is what an LLM reads when it chooses a tool, and this one told it to read a command's result "with `snapshot`'s scoped terminal text" — polling, which races the very transition the caller is watching for. That was accurate when orchestration shipped and `wait_for_status` did not exist; it has been wrong since. It now routes callers to `wait_for_status` then `read_terminal`, and says plainly not to poll. Two doc-comments still called the wait "a later rung" for the same reason. --- crates/app/src/mcp/handler.rs | 14 +++++++++----- crates/app/src/shell/bridge.rs | 5 +++-- crates/app/src/shell/orchestrate.rs | 3 ++- 3 files changed, 14 insertions(+), 8 deletions(-) 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) { From 2b96ab92e8cecddf5630bda20ebf8481024e15ab Mon Sep 17 00:00:00 2001 From: Bastien Gallay Date: Sat, 25 Jul 2026 22:35:18 +0200 Subject: [PATCH 2/5] =?UTF-8?q?docs(mcp):=20the=20control=20surface=20ship?= =?UTF-8?q?ped=20=E2=80=94=20reconcile=20README,=20AGENTS=20and=20ARCHITEC?= =?UTF-8?q?TURE?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six slices of #90 landed, and three documents still described the surface as unbuilt. README called workspace orchestration "a planned follow-up"; AGENTS listed `mcp` among the adapters yet to land (and still called `crates/app` a tracing stub); ARCHITECTURE said nothing in the v1 path imports it. README now documents both surfaces and the act -> wait -> observe loop they exist to serve, and AGENTS gains an entry point naming what is settled, what is open, and where each piece lives. The ARCHITECTURE claim is retracted in place rather than edited into correctness: a reader arriving with the old belief needs to recognise it. It also earns a disambiguation, because the collision is real — §15's deferred `mcp` crate is `F-mcp-ide-bridge`, termherd as a *client* of Claude's IDE bridge, which is genuinely unbuilt. The shipped surface runs the other way round and lives in `app`, since it is a bridge into the shell rather than a port `core` calls out through. --- AGENTS.md | 49 ++++++++++++++++++++++++++++++++++++++++---- README.md | 43 +++++++++++++++++++++++++++++++------- docs/ARCHITECTURE.md | 15 ++++++++++++-- 3 files changed, 94 insertions(+), 13 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index a7d5c39..6696c45 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 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/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: From f5b5765055101dc76165eabe7e02084f55a11a16 Mon Sep 17 00:00:00 2001 From: Bastien Gallay Date: Sat, 25 Jul 2026 22:35:31 +0200 Subject: [PATCH 3/5] =?UTF-8?q?docs(agents):=20an=20invariant=20expressed?= =?UTF-8?q?=20twice=20will=20drift=20=E2=80=94=20extract=20the=20predicate?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The one lesson from the MCP milestone that generalises past its own feature. The wait rung stated 'has this settled?' at two call sites with hand-written conditions; one counted a session exit as settling, the other only compared against the requested statuses. A wait placed after a crash therefore parked until the caller's timeout — on a session core had already marked terminal. Recorded with its instance rather than as an abstraction, and with the part that makes it a trap rather than an oversight: the doc-comment stating the correct rule sat directly above the code that broke it. A comment is not enforcement. --- AGENTS.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 6696c45..1d0cd20 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -220,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 From 636e86fce7241ddef8d8ddebcd5d93043b6ca406 Mon Sep 17 00:00:00 2001 From: Bastien Gallay Date: Sat, 25 Jul 2026 22:35:42 +0200 Subject: [PATCH 4/5] chore: ignore .impeccable caches and stop linting gitignored notes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two recurring dirt sources, tied off at the root rather than cleaned again next time. The impeccable skill drops a hook.cache.json beside every source directory it reads; four had accumulated under crates/ and none were ignored, so each one surfaced as untracked noise in git status. markdownlint was linting .personal/ and .local/, which are gitignored working notes CI never sees. It reported four errors on a fresh clone's 24 files becoming 37 locally — errors nobody could act on, which teaches you to skim past the summary line. Local and CI now lint the same 24 files. --- .gitignore | 3 +++ .markdownlint-cli2.jsonc | 13 +++++++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) 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/**" + ] } From cf2e8acc1b6e3b57358218328e807c2da9dc1ed0 Mon Sep 17 00:00:00 2001 From: Bastien Gallay Date: Sat, 25 Jul 2026 22:35:42 +0200 Subject: [PATCH 5/5] chore(wrap): record the wrap configuration inferred on the first pass The pass ran unconfigured and re-derived the canonical locations, the gate list and the commit conventions from the tree. Written down so the next one does not, and so a moved location is corrected in one place. Two entries encode things this session learned the hard way: worktree removal must check the PR state rather than `git log main..branch` (PRs are squash-merged, so a merged branch's commits never appear on main), and .impeccable caches are ignored rather than deleted because they regenerate. --- .wrap.md | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 .wrap.md 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.