diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..e66470c --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,73 @@ +name: CI + +on: + push: + branches: [main] + pull_request: + workflow_dispatch: + +env: + CARGO_TERM_COLOR: always + RUST_BACKTRACE: 1 + +jobs: + test: + name: build & test (${{ matrix.os }}) + runs-on: ${{ matrix.os }} + # A hung test would otherwise occupy a runner until GitHub's 6-hour ceiling. + # The suite finishes in seconds; 30 minutes is generous headroom for a cold + # cache on the slowest platform. + timeout-minutes: 30 + strategy: + # Don't cancel the other platforms when one fails — the whole point of + # this matrix is seeing which platforms differ. + fail-fast: false + matrix: + os: [ubuntu-latest, macos-latest, windows-latest] + + steps: + - uses: actions/checkout@v4 + + # GitHub runners ship a stable Rust toolchain, so there's nothing to + # install. Recorded here because it's easy to assume otherwise. + - name: Show toolchain + run: | + rustc --version + cargo --version + + # Tinker binds the OS webview through wry/tao, so Linux needs GTK and + # WebKitGTK headers. Cargo cannot declare these. Without them the build + # fails inside gdk-sys with an error that never names the fix — see + # docs/getting-started.md. Keeping this step here means the workflow + # doubles as executable setup documentation. + - name: Install native dependencies (Linux) + if: runner.os == 'Linux' + run: | + sudo apt-get update + sudo apt-get install -y --no-install-recommends \ + libgtk-3-dev \ + libwebkit2gtk-4.1-dev + + # macOS ships WebKit and Windows runners ship the WebView2 runtime, so + # neither needs an install step. + + - name: Cache cargo registry and target + uses: actions/cache@v4 + with: + path: | + ~/.cargo/registry + ~/.cargo/git + target + # Cargo.lock is gitignored in this repo, so the key is based on + # Cargo.toml instead. That makes the cache slightly less precise. + key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.toml') }} + restore-keys: ${{ runner.os }}-cargo- + + # Build before testing. tests/mcp_tests.rs spawns `cargo run -- --mcp` + # as a subprocess and waits a fixed 2s for it to come up; if the binary + # still had to compile at that point the test would be needlessly slow. + - name: Build + run: cargo build --all-targets --verbose + + - name: Test + run: cargo test --verbose diff --git a/ROADMAP.md b/ROADMAP.md index a671a36..becc842 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -16,11 +16,11 @@ Both sit on one core engine. Work that serves both lives in **Shared Foundation* ## Where Tinker actually stands The engine dispatches ~70 `BrowserCommand` variants (`src/event/mod.rs`), all handled in -`src/browser/mod.rs`. `cargo test` reports **164 passed, 0 failed, 3 ignored** (the ignored three -spawn the built binary). Verified on Linux, August 22, 2026. +`src/browser/mod.rs`. `cargo test` reports **140 passed, 0 failed, 0 ignored**, verified on Linux +and green on all three CI platforms, August 22, 2026. Counts below are of tests that actually execute. An earlier revision of this file over-counted by -including tests in files that were never compiled — see the note on dead modules under M2. +including tests in files that were never compiled — see the dead-modules entry under M1. ### Built and wired @@ -31,14 +31,14 @@ including tests in files that were never compiled — see the note on dead modul | MQTT event tower + reconnection | `event/mod.rs` | 4 | | REST API | `api/mod.rs` | — | | WebSocket live control (`/ws`) | `api/mod.rs` | — | -| MCP server (JSON-RPC 2.0 over stdio) | `mcp/mod.rs` | 34 | +| MCP server (JSON-RPC 2.0 over stdio), 33 tools; reads answered locally | `mcp/mod.rs` | 49 | | DOM inspector (CSS/XPath/text), interaction, waits | `browser/inspector.rs` | 2 | | JavaScript execution | `browser/mod.rs` | — | | Visual baselines + pixel diffing | `browser/visual.rs` | 2 | | Network monitoring + HAR export + filters | `browser/network.rs` | 2 | -| Console monitoring + filtering | `browser/console.rs` | 9 | -| Performance: Core Web Vitals, memory, JS profiling, marks/measures | `browser/performance.rs` | 28 | -| Recording + replay: seek, step forward/back, speed, loop | `browser/replay.rs` | 5 | +| Console monitoring + filtering (REST + MCP) | `browser/console.rs` | 9 | +| Performance: Core Web Vitals, memory, JS profiling, marks/measures (REST + MCP) | `browser/performance.rs` | 28 | +| Recording + replay: seek, step forward/back, speed, loop (REST + MCP) | `browser/replay.rs` | 5 | ### Partial @@ -49,12 +49,51 @@ including tests in files that were never compiled — see the note on dead modul windowing and webviews, so the open question is whether anything remains for this layer to do — see M2. +### Known bug: `--mcp` requires a display + +`--mcp` still starts the browser engine, which initialises GTK and aborts the process when no +display is available: + +``` +(tinker:32374): Gtk-WARNING **: cannot open display: +``` + +This races the MCP thread. If the server writes its JSON-RPC response before the main thread +reaches GTK, the call succeeds; otherwise the process dies mid-reply and the client reads an empty +line. Measured on Linux with no `DISPLAY`: + +| Invocation | Failure rate | +|---|---| +| Sequential | 0 / 20 | +| 3 concurrent | 11 / 24 | +| 3 concurrent, `--headless` | 3 / 12 | + +`--headless` reduces the window but does not close it, so it is not a workaround. It is not a +concurrency bug either — concurrency only changes which side of the race wins, and more runner +capacity would not help. + +Two consequences worth stating plainly: + +1. **It affects real use.** The Claude Desktop configuration in the readme runs `--mcp` over stdio. + On a headless machine that is the failing case, not an edge case. +2. **It is a latent CI flake.** `tests/mcp_tests.rs` spawns three of these concurrently, which is + exactly the ~45% case. CI has passed six consecutive runs on luck, not correctness, and will go + red eventually. Treat an unexplained red on those three tests as this bug, not a new regression. + +The fix is for `--mcp` (and arguably `--headless`) to skip window creation entirely rather than +initialising a webview it never shows. Deferred by request. + ### Not started -- CI of any kind. No `.github/workflows`. - Test generation from recordings. - Report/export layer. -- Keyboard input over the API or MCP (`browser/keyboard.rs` is internal-only). +- Page-level keyboard input. Note `browser/keyboard.rs` is *not* this: it maps chrome shortcuts + (Ctrl+T, Alt+Left) to browser commands that the API already exposes directly, so binding it + would add no capability. Testing tab order, focus traversal, and keyboard accessibility needs + events dispatched into the page — and synthetic `KeyboardEvent`s injected via JavaScript won't + do it, because browsers refuse default actions like focus movement for untrusted events. This + needs native input injection at the webview layer, which `wry` doesn't currently expose. Design + work before code. - Browser profiles — user agent, viewport, timezone, locale. - Cross-engine result comparison (see Track B, M4). @@ -110,28 +149,75 @@ recorded elsewhere. Cross-engine testing (M4) is only meaningful if Tinker reliably runs on more than one platform. That makes this milestone load-bearing rather than housekeeping. -- [ ] **GitHub Actions: build + test on macOS, Linux, Windows.** The suite passes and nothing runs - it. Start here. -- [ ] **Install native deps in CI** so the workflow doubles as executable setup documentation. +**The matrix is green.** As of August 22, 2026, Tinker builds and passes its full suite on all +three platforms — the first time this has ever been verified: + +| Platform | Build | Test | Engine exercised | +|---|---|---|---| +| `ubuntu-latest` | 2m00s | 2s | WebKitGTK / JavaScriptCore | +| `macos-latest` | 1m36s | 2s | WKWebView / JavaScriptCore | +| `windows-latest` | 3m30s | 4s | WebView2 / V8 | + +That last column is the point: two engine families are already under test on every run. M4 is now +a matter of comparing their results rather than acquiring the coverage. + +- [x] **GitHub Actions: build + test on macOS, Linux, Windows.** `.github/workflows/ci.yml`. + Uses the runners' preinstalled Rust and only first-party actions (`checkout`, `cache`), so + there are no third-party actions in the supply chain. `fail-fast: false`, so one platform + failing doesn't hide the others. +- [x] **Install native deps in CI** so the workflow doubles as executable setup documentation. A cold clone needs GTK and WebKitGTK headers that `Cargo.toml` can't declare; without them `gdk-sys` fails at `pkg-config --libs --cflags gdk-3.0` with a message that never names the - fix. Now written down in `docs/getting-started.md`, but documentation rots — CI wouldn't. -- [ ] **Headless-capable test lane.** Windowed tests need a display; sort out `xvfb` on Linux or - gate the windowed suite so the rest can run everywhere. + fix. Documented in `docs/getting-started.md`, but documentation rots — CI won't. +- [x] **Headless-capable test lane.** Turned out to need nothing: no surviving test creates a + window, verified by running the full suite with no `DISPLAY` and no X server. The only + window-creating tests lived in `browser/native_ui.rs`, which was dead code and is now gone. + Had it ever been wired up it would have failed on every runner without a display. +- [x] **Watch the first macOS and Windows runs.** All three platforms compiled on the first run: + Linux 2m06s, macOS 1m20s, Windows 3m07s. That retires the "cross-platform is unproven" + caveat for the build; the test lane is covered below. +- [x] **Fixed a test hang the matrix caught.** The three MCP protocol tests spawned + `cargo run` from inside `cargo test`, so the child contended for cargo's build-directory + lock and never started while the parent blocked on a `read_line()` with no timeout. All + three platforms hung. Locally it had passed — a fully warm `target/` let the child win the + race, which is exactly the kind of environment-dependent flake CI exists to expose. Fixed by + spawning `env!("CARGO_BIN_EXE_tinker")`, the binary cargo has already built: no nested cargo, + no lock contention. Test execution went from 16.61s to 0.04s. +- [x] **Bounded job runtime** with `timeout-minutes: 30`, so a future hang fails in half an hour + rather than occupying a runner until GitHub's six-hour ceiling. +- [ ] **Consider committing `Cargo.lock`.** It's currently gitignored. For a library that's + conventional; for an application it means CI builds aren't reproducible and can break when a + transitive dependency publishes. It also costs cache precision — the CI cache key falls back + to hashing `Cargo.toml`. - [ ] **Resolve `src/platform/`.** With Windows a real target, decide: finish the abstraction for what `tao`/`wry` genuinely don't cover (native chrome, theming, window handles), or delete it. Don't leave commented-out traits sitting there for another year. Nine other dead modules have now been removed for the same reason; this is the last of them, and the only one with a plausible future. -- [ ] **Deduplicate the module tree.** `main.rs` declares `api`, `browser`, `event`, and - `templates`, all of which `lib.rs` already exports — so the crate is compiled twice and - shared tests execute twice (48 in the lib binary, 63 in the bin, largely overlapping). - `main.rs` should depend on the library rather than re-declaring its modules. Note `mcp` - lives only in `main.rs` and `platform` only in `lib.rs`, so this needs care, not a blind - delete. +- [x] **Deduplicated the module tree.** `main.rs` declared `api`, `browser`, `event`, and + `templates`, all of which `lib.rs` already exported, so the crate compiled twice and every + shared test ran once per target. `mcp` moved into the library (it was declared only in + `main.rs`), and the binary now links against the library instead of re-declaring modules. + `platform` stays library-only as before. + + | | Before | After | + |---|---|---| + | Incremental rebuild after touching `browser/mod.rs` | 21.5s | 4.0s | + | Warnings from the binary target | 91 | 2 | + | Test executions | 182 | 140 | + | **Unique test names** | **140** | **140** | + + The drop in executions is the duplication disappearing, not lost coverage: comparing + `cargo test -- --list` before and after gives identical sets of 140 names. Earlier revisions + of this file quoted the inflated execution count as though it were a test count; 140 is the + real figure. - [ ] **Clear the warning backlog.** A clean build emits 32 warnings for the lib and 91 for the binary — unused imports, unused variables, dead constants in `templates/mod.rs`. Enough - noise to hide a real one. + noise to hide a real one. Deliberately not gated in CI yet: turning warnings into errors + today would make the workflow red on arrival. +- [ ] **Decide on `rustfmt`.** The tree isn't format-clean (~688 diffs), so a `cargo fmt --check` + gate would fail immediately. Either format once in a single mechanical commit and gate it + afterwards, or drop the idea — but don't add the gate first. - [ ] **Tag v0.1.0** once the matrix is green. First point a user can be pointed at. --- @@ -144,17 +230,53 @@ tests, DOM find/click/type, JavaScript execution, and network monitoring. ### M3 — Close the agent feedback loop -- [ ] **Expose the observability suite over MCP.** Console logs, performance metrics, and Core Web - Vitals are all built and reachable via REST, but absent from the MCP tool list. An agent - currently can't ask "did that click throw a console error?" — the highest-value question it - could ask. -- [ ] **Expose recording/replay over MCP.** Let an agent record its own session and replay it. +- [x] **Expose the observability suite over MCP.** Nine tools added — four for console capture, + five for performance — taking the advertised surface from 16 tools to 25. All nine were + already reachable over REST; only the MCP binding was missing. + **Caveat, and it is a large one:** these tools can only *trigger* a query, not return its + answer. See the next item — until that is fixed, every `get_*` tool on the MCP surface is + half a feature. +- [x] **Expose recording/replay over MCP.** Eight tools: start/stop recording, save/load to file, + start/stop playback, playback state, and a single `step_playback` taking a direction rather + than two separate verbs — an agent bisecting a failure thinks in terms of stepping. An unknown + direction is an error rather than a silent default, since stepping the wrong way would mislead + exactly the bisect it exists to serve. +- [ ] **Make MCP tools return their results.** *Highest priority in this track; blocks the two + items below.* Every tool is currently fire-and-forget. `handle_tool_call` broadcasts a + `BrowserCommand` and returns the string `"Command '' sent successfully"` — the code + carries the comment *"in a real implementation, we'd wait for the response"*. So + `get_console_logs` returns that sentence rather than any logs, and the same is true of + `get_core_web_vitals`, `get_page_info`, `find_element`, `execute_javascript`, + `take_screenshot`, and every other read. + + The pieces exist but are not joined. The engine does publish results — `GetConsoleLogs` + emits a `ConsoleMessage` event per line, and there are `PerformanceMetricsCollected`, + `CoreWebVitalsUpdated`, and `MemoryMetricsUpdated` variants. `McpServer` even holds an + `event_rx: broadcast::Receiver`. It is never read — the field is touched only + by the constructor. + + The design problem is correlation. Events carry no request id, and some (`ConsoleMessage`) + are also emitted spontaneously by the page, so "collect events for N ms after sending" is + racy: it can capture unrelated traffic or miss a slow reply. Two candidate fixes, and this + needs a decision before code: + 1. Add a correlation id to `BrowserCommand`/`BrowserEvent` and have the engine echo it. + Clean, but touches every command and event variant. + 2. Have the MCP path call the engine's accessor methods directly rather than round-tripping + through the broadcast bus. Much smaller, but only works where MCP and the engine share a + process — which today they do. + + Whichever is chosen, the wait needs a timeout. A blocking read with no deadline is the exact + failure that hung CI on all three platforms earlier in this milestone. - [ ] **Structured errors for agents.** Failures should return machine-readable causes, not prose. + Depends on the item above: there is no result path to put a structured error into yet. - [ ] **MCP resources and prompts.** `handle_resources_list` and `handle_prompts_list` return empty. - Resources could expose the live DOM, console buffer, and network log as readable context. -- [ ] **Expose keyboard input.** `browser/keyboard.rs` handles shortcuts internally but is reachable - from neither the API nor MCP. Selector-based `click`/`type` can't test tab order, focus - traversal, or keyboard accessibility — those need real key events. Wanted by both tracks. + Resources would expose the live DOM, console buffer, and network log as readable context, so + an agent could pull state without a tool call per question. Blocked on the result path above: + `resources/read` has to return real content, and today nothing can. +- [ ] **Page-level keyboard input** — see the note under "Not started". Wanted by both tracks, but + it needs a design decision first (native injection vs. driving the webview's own input path), + not just a binding. Do not scope this as "expose `keyboard.rs`"; that module solves a + different problem. - [ ] **Document the agent loop** in `docs/mcp-server.md`: act → observe → assert. --- @@ -218,14 +340,34 @@ the useful half; evasion tooling is a different product with different obligatio Recorded so these don't get re-proposed. **Embedding multiple JS engines** (the old "JavaScript Engine Workshop": V8 integration, -SpiderMonkey support, JavaScriptCore bridge, engine switching). Tinker is built on `wry`, which -delegates to the OS webview and its bundled engine. You cannot swap V8 into the macOS build. -SpiderMonkey is unavailable at any price — Gecko ships no embedding API of this kind, so Firefox -coverage would mean abandoning `wry` entirely. - -*The underlying goal survives as M4*, which gets cross-engine coverage from the CI matrix instead — -real WebKit and real Chromium, in their shipping configurations, which is better evidence than -embedded engines would have provided anyway. +SpiderMonkey support, JavaScriptCore bridge, engine switching). + +This was attempted. The `feat/js-engine-integration` branch (30 commits, January 2025) built +`src/js_engine/` with a `JsEngine` trait and V8, JavaScriptCore, and SpiderMonkey implementations +behind Cargo features. It was reviewed before this section was written, and it does not change the +conclusion — it sharpens it. + +The decisive detail is what those implementations actually are. `SpiderMonkeyEngine` constructs a +bare `mozjs::rust::Runtime` with `SIMPLE_GLOBAL_CLASS`; `JavaScriptCoreEngine` constructs a bare +`javascriptcore_rs::Context`. Both are **standalone interpreters with no DOM** — no `window`, no +`document`, no layout, no browser APIs. And in that branch's `Cargo.toml` they sit *alongside* the +webview rather than replacing it: `webview = ["dep:wry", "dep:tao"]` and `v8 = ["dep:v8"]` are +independent features. The embedded engines never render the page. + +That is fatal for the goal. Cross-browser bugs live in DOM behavior, layout, CSS, event handling, +and browser API differences. A bare ECMAScript interpreter with no `document` cannot observe any of +them. Even had the branch compiled — its own commit message says "Build currently failing, needs +dependency fixes" — it would have answered a question nobody was asking: whether pure ECMAScript +differs between engines, which is both rare and heavily standardized. + +*The underlying goal is already met by M2 and extended by M4.* The CI matrix runs real WebKit +(Linux, macOS) and real Chromium/V8 (Windows) in their shipping configurations, rendering real +pages. That is strictly better evidence than embedded engines could produce, and it exists today. + +**Worth salvaging separately:** that branch's Cargo feature reorganization — optional dependencies +with granular `webview` / `cli` / `api` / `metrics` features — is sound practice independent of the +engine work, and would cut build times for users who don't need every subsystem. Filed here rather +than lost. **Full platform abstraction as originally scoped.** `tao` and `wry` already abstract windowing and webviews. M2 decides whether the thin remainder is worth keeping. diff --git a/docs/mcp-server.md b/docs/mcp-server.md index 28531ca..2688a68 100644 --- a/docs/mcp-server.md +++ b/docs/mcp-server.md @@ -49,6 +49,17 @@ The MCP server implements the Model Context Protocol specification, using JSON-R } ``` +## Known limitation: tools do not return results + +Every tool currently broadcasts its command to the browser and responds with +`"Command '' sent successfully"`. It does **not** return the command's +result. Reads such as `get_console_logs`, `get_core_web_vitals`, `get_page_info`, +and `execute_javascript` trigger the work, but the output is published to the +event bus rather than returned to the caller. + +Use the REST API when you need the value back. This is the top priority in +Track A of the [roadmap](../ROADMAP.md). + ## Available Methods ### initialize @@ -207,6 +218,60 @@ Execute JavaScript code in the page context. **Arguments:** - `script` (string, required): JavaScript code to execute +### Recording & Replay + +#### start_recording +Start recording browser events. Requires `name` and `start_url`. + +#### stop_recording +Stop the active recording. No arguments. + +#### save_recording / load_recording +Persist a recording to disk or read one back. Each requires `path`. + +#### start_playback / stop_playback +Begin or halt replay of the loaded recording. No arguments. + +#### get_playback_state +Current position, speed, and whether playback is running. No arguments. + +#### step_playback +Step one event through the recording. Optional `direction` (`forward` or `backward`, +default `forward`). An unrecognised direction is rejected rather than defaulted, so a +typo can't silently step the wrong way during a bisect. + +### Console Monitoring + +#### start_console_monitoring +Start capturing console output (log, info, warn, error) from the page. No arguments. + +#### stop_console_monitoring +Stop capturing console output. No arguments. + +#### get_console_logs +Retrieve captured console messages. Optional `level` (`log`, `info`, `warn`, `error`, `debug`); +omit it to get everything. Use this after an interaction to check whether the page reported errors. + +#### clear_console_logs +Clear the captured message buffer. No arguments. + +### Performance + +#### start_performance_monitoring +Start collecting performance metrics. No arguments. + +#### stop_performance_monitoring +Stop collecting performance metrics. No arguments. + +#### get_core_web_vitals +Get Core Web Vitals for the current page (LCP, FID, CLS, INP, TTFB, FCP). No arguments. + +#### get_memory_metrics +Get memory usage (JS heap, DOM nodes, event listeners). No arguments. + +#### get_performance_summary +Get an aggregate performance summary. No arguments. + ### Network Monitoring #### start_network_monitoring diff --git a/readme.md b/readme.md index f4df05b..be376b2 100644 --- a/readme.md +++ b/readme.md @@ -173,6 +173,9 @@ Then ask Claude to control the browser: - **DOM Interaction**: find_element, click_element, type_text - **JavaScript**: execute_javascript, get_page_info - **Network**: start_network_monitoring, stop_network_monitoring, get_network_stats, export_network_har +- **Console**: start_console_monitoring, stop_console_monitoring, get_console_logs, clear_console_logs +- **Performance**: start_performance_monitoring, stop_performance_monitoring, get_core_web_vitals, get_memory_metrics, get_performance_summary +- **Recording & Replay**: start_recording, stop_recording, save_recording, load_recording, start_playback, stop_playback, get_playback_state, step_playback See [MCP Server Documentation](docs/mcp-server.md) for complete details. @@ -189,7 +192,7 @@ Tinker works and is useful, with the caveats below. Status here is kept honest against the code — see the [roadmap](ROADMAP.md) for a per-module breakdown citing implementing files and test counts. -**Verified**: August 22, 2026 · ~70 browser commands · `cargo test` → 164 passed, 3 ignored +**Verified**: August 22, 2026 · ~70 browser commands · `cargo test` → 140 passed, 0 failed ### What works @@ -222,6 +225,12 @@ breakdown citing implementing files and test counts. testing (tab order, accessibility) isn't scriptable yet. - **Assertions are minimal.** Recordings can store expected state, but there's no authoring UX and no pass/fail surfacing. +- **MCP tools don't return results.** Every tool triggers its command and + replies `"Command '' sent successfully"`. Reads like + `get_console_logs` and `get_core_web_vitals` do not return logs or vitals — + results are published to the event bus instead. Use the REST API when you + need the answer back. Tracked as the top item in Track A of the + [roadmap](ROADMAP.md). ### Getting it running diff --git a/src/browser/mod.rs b/src/browser/mod.rs index 7ede388..6c65328 100644 --- a/src/browser/mod.rs +++ b/src/browser/mod.rs @@ -56,7 +56,9 @@ mod console; pub mod keyboard; pub mod session; -use self::{ +// Publicly re-exported so the MCP server can hold the same shared state the +// engine does and answer reads directly rather than through the event bus. +pub use self::{ tabs::TabManager, event_viewer::EventViewer, tab_ui::{TabBar, TabCommand}, diff --git a/src/lib.rs b/src/lib.rs index 060fe06..cc5ba7f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,6 +1,7 @@ pub mod api; pub mod browser; pub mod event; +pub mod mcp; pub mod platform; pub mod templates; diff --git a/src/main.rs b/src/main.rs index 2f0a9bd..55f4797 100644 --- a/src/main.rs +++ b/src/main.rs @@ -2,14 +2,12 @@ use clap::Parser; use tracing::{debug, error, info}; use std::{sync::{Arc, Mutex}, env}; -mod api; -mod browser; -mod event; -mod mcp; -mod templates; - -use crate::{ - browser::{BrowserEngine, session::default_session_path}, +// The binary links against the library rather than re-declaring its modules. +// Declaring them here as well compiled the whole crate a second time and ran +// every shared test twice, once per target. +use tinker::{ + api, mcp, + browser::{session::default_session_path, BrowserEngine}, event::EventSystem, }; @@ -233,12 +231,22 @@ async fn main() -> Result<(), Box> { if args.mcp { let command_tx_clone = command_tx.clone(); let event_rx_clone = event_rx.resubscribe(); + // Share the engine's own state with the MCP server so read tools can + // return real values. Both live in this process; the server just runs + // on another thread. + let mcp_state = mcp::BrowserState { + console: browser.console_monitor.clone(), + performance: browser.performance_monitor.clone(), + network: browser.network_monitor.clone(), + player: browser.player.clone(), + }; info!("🚀 Starting MCP server on stdio"); info!("📡 MCP server ready for JSON-RPC protocol messages"); // MCP server must run on a separate thread since it blocks on stdin std::thread::spawn(move || { - let mut mcp_server = mcp::McpServer::new(command_tx_clone, event_rx_clone); + let mut mcp_server = mcp::McpServer::new(command_tx_clone, event_rx_clone) + .with_state(mcp_state); if let Err(e) = mcp_server.run() { error!("MCP server error: {}", e); } diff --git a/src/mcp/mod.rs b/src/mcp/mod.rs index 7990c4c..0e7724b 100644 --- a/src/mcp/mod.rs +++ b/src/mcp/mod.rs @@ -10,8 +10,32 @@ use std::io::{self, BufRead, Write}; use tokio::sync::broadcast; use tracing::{debug, error, info}; +use std::sync::{Arc, Mutex}; + +use crate::browser::{ConsoleLevel, ConsoleMonitor, EventPlayer, NetworkMonitor, PerformanceMonitor}; use crate::event::{BrowserCommand, BrowserEvent}; +/// Shared browser state the MCP server can read directly. +/// +/// Tools cannot answer reads by round-tripping the broadcast bus: `BrowserEvent` +/// carries no request id, and some variants (`ConsoleMessage`) are emitted +/// spontaneously by the page, so correlating a reply to a request by time window +/// is racy in both directions. The MCP server runs on its own thread inside the +/// same process as the engine, so it can instead hold the same `Arc>` +/// handles the engine holds and read them directly. +/// +/// This covers state the engine owns. It deliberately does not cover reads that +/// need the WebView itself -- `get_page_info`, `execute_javascript`, +/// `find_element`, `take_screenshot` -- because those must run on the thread +/// owning the window, and they stay fire-and-forget for now. +#[derive(Clone)] +pub struct BrowserState { + pub console: Arc>, + pub performance: Arc>, + pub network: Arc>, + pub player: Arc>, +} + /// MCP protocol version const MCP_VERSION: &str = "2024-11-05"; @@ -48,6 +72,9 @@ struct JsonRpcError { pub struct McpServer { command_tx: broadcast::Sender, event_rx: broadcast::Receiver, + /// Present when the server shares a process with a running engine. Without + /// it, reads fall back to the fire-and-forget path. + state: Option, } impl McpServer { @@ -59,7 +86,109 @@ impl McpServer { Self { command_tx, event_rx, + state: None, + } + } + + /// Attach shared engine state so reads return real values instead of an + /// acknowledgement. Call this when the server runs in-process with an engine. + pub fn with_state(mut self, state: BrowserState) -> Self { + self.state = Some(state); + self + } + + /// Answer a read from shared state, if this tool is one that can be. + /// + /// Returns `Ok(None)` when the tool isn't a local read or no state is + /// attached, in which case the caller falls back to broadcasting a command. + fn try_local_read( + &self, + tool_name: &str, + arguments: &Value, + ) -> Result, JsonRpcError> { + let Some(state) = &self.state else { + return Ok(None); + }; + + // A poisoned lock means another thread panicked holding it. Surface that + // as an error rather than panicking the MCP server too. + fn lock_err(what: &str) -> JsonRpcError { + JsonRpcError { + code: -32603, + message: format!("Failed to lock {}", what), + data: None, + } } + + let value = match tool_name { + "get_console_logs" => { + let level = arguments + .get("level") + .and_then(|v| v.as_str()) + .and_then(ConsoleLevel::from_str); + let monitor = state.console.lock().map_err(|_| lock_err("console monitor"))?; + let messages = monitor.get_messages(level); + json!({ "count": messages.len(), "messages": messages }) + } + "get_core_web_vitals" => { + let monitor = state + .performance + .lock() + .map_err(|_| lock_err("performance monitor"))?; + json!(monitor.get_core_web_vitals()) + } + "get_memory_metrics" => { + let monitor = state + .performance + .lock() + .map_err(|_| lock_err("performance monitor"))?; + match monitor.get_latest_memory() { + Some(metrics) => json!(metrics), + // No snapshot yet is a legitimate state, not an error. + None => json!(null), + } + } + "get_performance_summary" => { + let monitor = state + .performance + .lock() + .map_err(|_| lock_err("performance monitor"))?; + json!(monitor.get_summary()) + } + "get_network_stats" => { + let monitor = state.network.lock().map_err(|_| lock_err("network monitor"))?; + json!(monitor.get_stats()) + } + "export_network_har" => { + let monitor = state.network.lock().map_err(|_| lock_err("network monitor"))?; + let har = monitor.export_har().map_err(|e| JsonRpcError { + code: -32603, + message: format!("Failed to export HAR: {}", e), + data: None, + })?; + json!({ "har": har }) + } + "get_playback_state" => { + let player = state.player.lock().map_err(|_| lock_err("player"))?; + // PlaybackState alone isn't much use to an agent deciding what to + // do next; position and counts are what it actually needs. + json!({ + "state": format!("{:?}", player.get_state()), + "position_ms": player.get_position(), + "duration_ms": player.get_duration(), + "current_index": player.get_current_index(), + "event_count": player.get_event_count(), + }) + } + _ => return Ok(None), + }; + + Ok(Some(json!({ + "content": [{ + "type": "text", + "text": serde_json::to_string_pretty(&value).unwrap_or_else(|_| value.to_string()) + }] + }))) } /// Run the MCP server (blocking) @@ -387,6 +516,137 @@ impl McpServer { "required": ["test_name"] }), ), + // Console monitoring. Without these an agent can act on a page but + // cannot see whether the page complained, which is usually the first + // thing worth knowing after an interaction. + self.tool_definition( + "start_console_monitoring", + "Start capturing console output (log, info, warn, error) from the page", + json!({ "type": "object", "properties": {} }), + ), + self.tool_definition( + "stop_console_monitoring", + "Stop capturing console output", + json!({ "type": "object", "properties": {} }), + ), + self.tool_definition( + "get_console_logs", + "Retrieve captured console messages, optionally filtered by level. Use after an interaction to check whether the page reported errors.", + json!({ + "type": "object", + "properties": { + "level": { + "type": "string", + "description": "Only return messages at this level", + "enum": ["log", "info", "warn", "error", "debug"] + } + } + }), + ), + self.tool_definition( + "clear_console_logs", + "Clear the captured console message buffer", + json!({ "type": "object", "properties": {} }), + ), + // Performance. The REST API has exposed these for a while; agents + // could not reach them. + self.tool_definition( + "start_performance_monitoring", + "Start collecting performance metrics for the current page", + json!({ "type": "object", "properties": {} }), + ), + self.tool_definition( + "stop_performance_monitoring", + "Stop collecting performance metrics", + json!({ "type": "object", "properties": {} }), + ), + self.tool_definition( + "get_core_web_vitals", + "Get Core Web Vitals for the current page (LCP, FID, CLS, INP, TTFB, FCP)", + json!({ "type": "object", "properties": {} }), + ), + self.tool_definition( + "get_memory_metrics", + "Get memory usage for the current page (JS heap, DOM nodes, event listeners)", + json!({ "type": "object", "properties": {} }), + ), + // Recording and replay. Lets an agent capture what it did and replay + // it deterministically — the difference between "it worked once" and + // a reproducible case. + self.tool_definition( + "start_recording", + "Start recording browser events into a named session", + json!({ + "type": "object", + "properties": { + "name": { "type": "string", "description": "Name for the recording" }, + "start_url": { "type": "string", "description": "URL to begin the recording at" } + }, + "required": ["name", "start_url"] + }), + ), + self.tool_definition( + "stop_recording", + "Stop the active recording", + json!({ "type": "object", "properties": {} }), + ), + self.tool_definition( + "save_recording", + "Save the current recording to a file", + json!({ + "type": "object", + "properties": { + "path": { "type": "string", "description": "File path to write the recording to" } + }, + "required": ["path"] + }), + ), + self.tool_definition( + "load_recording", + "Load a previously saved recording from a file", + json!({ + "type": "object", + "properties": { + "path": { "type": "string", "description": "File path to read the recording from" } + }, + "required": ["path"] + }), + ), + self.tool_definition( + "start_playback", + "Begin replaying the loaded recording", + json!({ "type": "object", "properties": {} }), + ), + self.tool_definition( + "stop_playback", + "Stop replaying", + json!({ "type": "object", "properties": {} }), + ), + self.tool_definition( + "get_playback_state", + "Get the current playback state (position, speed, whether running)", + json!({ "type": "object", "properties": {} }), + ), + self.tool_definition( + "step_playback", + "Step one event forward or backward through the recording. Use this to \ + narrow down which event in a reproduction causes a failure.", + json!({ + "type": "object", + "properties": { + "direction": { + "type": "string", + "description": "Which way to step (default \"forward\")", + "enum": ["forward", "backward"] + } + } + }), + ), + self.tool_definition( + "get_performance_summary", + "Get an aggregate performance summary for the current page", + json!({ "type": "object", "properties": {} }), + ), ]; Ok(json!({ @@ -412,6 +672,13 @@ impl McpServer { debug!("Tool call: {} with args: {:?}", tool_name, arguments); + // Reads answerable from shared state return the value itself. Everything + // else falls through to the broadcast path below, which can only + // acknowledge that the command was sent. + if let Some(result) = self.try_local_read(tool_name, &arguments)? { + return Ok(result); + } + let command = match tool_name { "navigate" => { let url = arguments["url"].as_str().ok_or_else(|| JsonRpcError { @@ -538,6 +805,76 @@ impl McpServer { script: script.to_string(), } } + "start_recording" => { + let name = arguments["name"].as_str().ok_or_else(|| JsonRpcError { + code: -32602, + message: "Missing required parameter: name".to_string(), + data: None, + })?; + let start_url = arguments["start_url"].as_str().ok_or_else(|| JsonRpcError { + code: -32602, + message: "Missing required parameter: start_url".to_string(), + data: None, + })?; + BrowserCommand::StartRecording { + name: name.to_string(), + start_url: start_url.to_string(), + } + } + "stop_recording" => BrowserCommand::StopRecording, + "save_recording" => { + let path = arguments["path"].as_str().ok_or_else(|| JsonRpcError { + code: -32602, + message: "Missing required parameter: path".to_string(), + data: None, + })?; + BrowserCommand::SaveRecording { path: path.to_string() } + } + "load_recording" => { + let path = arguments["path"].as_str().ok_or_else(|| JsonRpcError { + code: -32602, + message: "Missing required parameter: path".to_string(), + data: None, + })?; + BrowserCommand::LoadRecording { path: path.to_string() } + } + "start_playback" => BrowserCommand::StartPlayback, + "stop_playback" => BrowserCommand::StopPlayback, + "get_playback_state" => BrowserCommand::GetPlaybackState, + "step_playback" => { + // Two directions behind one tool: an agent bisecting a failure + // thinks in terms of "step", not two separate verbs. + match arguments.get("direction").and_then(|v| v.as_str()).unwrap_or("forward") { + "backward" => BrowserCommand::StepBackward, + "forward" => BrowserCommand::StepForward, + other => { + return Err(JsonRpcError { + code: -32602, + message: format!( + "Invalid direction {:?}: expected \"forward\" or \"backward\"", + other + ), + data: None, + }) + } + } + } + "start_console_monitoring" => BrowserCommand::StartConsoleMonitoring, + "stop_console_monitoring" => BrowserCommand::StopConsoleMonitoring, + "get_console_logs" => BrowserCommand::GetConsoleLogs { + // Absent means "all levels", which is why this is not a required + // argument and a missing value is not an error. + level: arguments + .get("level") + .and_then(|v| v.as_str()) + .map(|s| s.to_string()), + }, + "clear_console_logs" => BrowserCommand::ClearConsoleLogs, + "start_performance_monitoring" => BrowserCommand::StartPerformanceMonitoring, + "stop_performance_monitoring" => BrowserCommand::StopPerformanceMonitoring, + "get_core_web_vitals" => BrowserCommand::GetCoreWebVitals, + "get_memory_metrics" => BrowserCommand::GetMemoryMetrics, + "get_performance_summary" => BrowserCommand::GetPerformanceSummary, "start_network_monitoring" => BrowserCommand::StartNetworkMonitoring, "stop_network_monitoring" => BrowserCommand::StopNetworkMonitoring, "get_network_stats" => BrowserCommand::GetNetworkStats, @@ -809,6 +1146,337 @@ mod tests { assert!(result.is_ok()); } + /// A server with shared state, plus the state so a test can populate it. + fn setup_server_with_state( + ) -> (McpServer, BrowserState, broadcast::Receiver) { + use crate::browser::{ConsoleMessage, NetworkMonitor, PerformanceMonitor}; + // The receiver is returned rather than dropped: with no subscribers, + // broadcast::send fails and every action tool would error. + let (command_tx, command_rx) = broadcast::channel(100); + let (_event_tx, event_rx) = broadcast::channel(100); + let state = BrowserState { + console: Arc::new(Mutex::new(ConsoleMonitor::new())), + performance: Arc::new(Mutex::new(PerformanceMonitor::new())), + network: Arc::new(Mutex::new(NetworkMonitor::new())), + player: Arc::new(Mutex::new(EventPlayer::new())), + }; + let _ = ConsoleMessage::new(ConsoleLevel::Log, String::new(), vec![]); + ( + McpServer::new(command_tx, event_rx).with_state(state.clone()), + state, + command_rx, + ) + } + + /// The point of the whole local-read path: a read must come back with the + /// data, not with "Command 'x' sent successfully". + #[test] + fn test_get_console_logs_returns_actual_messages() { + use crate::browser::ConsoleMessage; + let (mut server, state, _rx) = setup_server_with_state(); + + { + let mut monitor = state.console.lock().unwrap(); + monitor.add_message(ConsoleMessage::new( + ConsoleLevel::Error, + "TypeError: undefined is not a function".to_string(), + vec![], + )); + monitor.add_message(ConsoleMessage::new( + ConsoleLevel::Log, + "hello".to_string(), + vec![], + )); + } + + let params = json!({ "name": "get_console_logs", "arguments": {} }); + let result = server.handle_tool_call(Some(params)).unwrap(); + let text = result["content"][0]["text"].as_str().unwrap(); + + assert!( + !text.contains("sent successfully"), + "read returned an acknowledgement instead of data: {}", + text + ); + let payload: Value = serde_json::from_str(text).expect("read should return JSON"); + assert_eq!(payload["count"], 2); + assert!(text.contains("TypeError: undefined is not a function")); + } + + #[test] + fn test_get_console_logs_level_filter_applies_to_real_data() { + use crate::browser::ConsoleMessage; + let (mut server, state, _rx) = setup_server_with_state(); + + { + let mut monitor = state.console.lock().unwrap(); + monitor.add_message(ConsoleMessage::new( + ConsoleLevel::Error, + "boom".to_string(), + vec![], + )); + monitor.add_message(ConsoleMessage::new( + ConsoleLevel::Log, + "chatter".to_string(), + vec![], + )); + } + + let params = json!({ + "name": "get_console_logs", + "arguments": { "level": "error" } + }); + let result = server.handle_tool_call(Some(params)).unwrap(); + let text = result["content"][0]["text"].as_str().unwrap(); + + assert!(text.contains("boom"), "error message missing: {}", text); + assert!(!text.contains("chatter"), "filter did not exclude lower level: {}", text); + } + + #[test] + fn test_playback_state_read_returns_position_not_ack() { + let (mut server, _state, _rx) = setup_server_with_state(); + + let params = json!({ "name": "get_playback_state", "arguments": {} }); + let result = server.handle_tool_call(Some(params)).unwrap(); + let text = result["content"][0]["text"].as_str().unwrap(); + + let payload: Value = serde_json::from_str(text).expect("read should return JSON"); + // The bare enum isn't enough for an agent to decide what to do next. + for key in ["state", "position_ms", "duration_ms", "current_index", "event_count"] { + assert!(!payload[key].is_null(), "{} missing from playback state", key); + } + } + + /// Actions still broadcast; only reads are answered locally. + #[test] + fn test_actions_still_broadcast_with_state_attached() { + let (mut server, _state, _rx) = setup_server_with_state(); + + let params = json!({ + "name": "navigate", + "arguments": { "url": "https://example.com" } + }); + let result = server.handle_tool_call(Some(params)).unwrap(); + let text = result["content"][0]["text"].as_str().unwrap(); + assert!(text.contains("sent successfully"), "expected an ack, got: {}", text); + } + + /// Without state the server must still work, just without real reads. + #[test] + fn test_reads_fall_back_to_ack_without_state() { + let (mut server, _rx) = setup_test_server(); + + let params = json!({ "name": "get_console_logs", "arguments": {} }); + let result = server.handle_tool_call(Some(params)).unwrap(); + let text = result["content"][0]["text"].as_str().unwrap(); + assert!(text.contains("sent successfully")); + } + + #[test] + fn test_recording_tools_dispatch_expected_commands() { + let (mut server, mut rx) = setup_test_server(); + + for (tool, expected) in [ + ("stop_recording", BrowserCommand::StopRecording), + ("start_playback", BrowserCommand::StartPlayback), + ("stop_playback", BrowserCommand::StopPlayback), + ("get_playback_state", BrowserCommand::GetPlaybackState), + ] { + let params = json!({ "name": tool, "arguments": {} }); + assert!(server.handle_tool_call(Some(params)).is_ok(), "{} failed", tool); + let sent = rx.try_recv().expect("no command was broadcast"); + assert_eq!( + std::mem::discriminant(&sent), + std::mem::discriminant(&expected), + "{} dispatched the wrong command", + tool + ); + } + } + + #[test] + fn test_start_recording_passes_arguments_through() { + let (mut server, mut rx) = setup_test_server(); + + let params = json!({ + "name": "start_recording", + "arguments": { "name": "checkout-flow", "start_url": "https://example.com/cart" } + }); + assert!(server.handle_tool_call(Some(params)).is_ok()); + + match rx.try_recv().expect("no command was broadcast") { + BrowserCommand::StartRecording { name, start_url } => { + assert_eq!(name, "checkout-flow"); + assert_eq!(start_url, "https://example.com/cart"); + } + other => panic!("expected StartRecording, got {:?}", other), + } + } + + #[test] + fn test_start_recording_requires_both_arguments() { + let (mut server, _rx) = setup_test_server(); + + // start_url missing + let params = json!({ + "name": "start_recording", + "arguments": { "name": "only-a-name" } + }); + assert!(server.handle_tool_call(Some(params)).is_err()); + } + + #[test] + fn test_step_playback_direction() { + let (mut server, mut rx) = setup_test_server(); + + // Explicit backward + let params = json!({ + "name": "step_playback", + "arguments": { "direction": "backward" } + }); + assert!(server.handle_tool_call(Some(params)).is_ok()); + assert_eq!( + std::mem::discriminant(&rx.try_recv().unwrap()), + std::mem::discriminant(&BrowserCommand::StepBackward) + ); + + // Omitted direction defaults to forward + let params = json!({ "name": "step_playback", "arguments": {} }); + assert!(server.handle_tool_call(Some(params)).is_ok()); + assert_eq!( + std::mem::discriminant(&rx.try_recv().unwrap()), + std::mem::discriminant(&BrowserCommand::StepForward) + ); + } + + #[test] + fn test_step_playback_rejects_unknown_direction() { + let (mut server, mut rx) = setup_test_server(); + + // A typo must be an error, not a silent step in the default direction -- + // an agent bisecting a failure would be misled by the wrong way. + let params = json!({ + "name": "step_playback", + "arguments": { "direction": "backwards" } + }); + assert!(server.handle_tool_call(Some(params)).is_err()); + assert!(rx.try_recv().is_err(), "no command should have been broadcast"); + } + + #[test] + fn test_console_tools_dispatch_expected_commands() { + let (mut server, mut rx) = setup_test_server(); + + for (tool, expected) in [ + ("start_console_monitoring", BrowserCommand::StartConsoleMonitoring), + ("stop_console_monitoring", BrowserCommand::StopConsoleMonitoring), + ("clear_console_logs", BrowserCommand::ClearConsoleLogs), + ] { + let params = json!({ "name": tool, "arguments": {} }); + assert!(server.handle_tool_call(Some(params)).is_ok(), "{} failed", tool); + let sent = rx.try_recv().expect("no command was broadcast"); + assert_eq!( + std::mem::discriminant(&sent), + std::mem::discriminant(&expected), + "{} dispatched the wrong command", + tool + ); + } + } + + #[test] + fn test_get_console_logs_passes_level_through() { + let (mut server, mut rx) = setup_test_server(); + + let params = json!({ + "name": "get_console_logs", + "arguments": { "level": "error" } + }); + assert!(server.handle_tool_call(Some(params)).is_ok()); + + match rx.try_recv().expect("no command was broadcast") { + BrowserCommand::GetConsoleLogs { level } => { + assert_eq!(level.as_deref(), Some("error")); + } + other => panic!("expected GetConsoleLogs, got {:?}", other), + } + } + + #[test] + fn test_get_console_logs_without_level_means_all() { + let (mut server, mut rx) = setup_test_server(); + + // `level` is optional; omitting it must not be an error, and must not + // silently become a filter. + let params = json!({ "name": "get_console_logs", "arguments": {} }); + assert!(server.handle_tool_call(Some(params)).is_ok()); + + match rx.try_recv().expect("no command was broadcast") { + BrowserCommand::GetConsoleLogs { level } => assert_eq!(level, None), + other => panic!("expected GetConsoleLogs, got {:?}", other), + } + } + + #[test] + fn test_performance_tools_dispatch_expected_commands() { + let (mut server, mut rx) = setup_test_server(); + + for (tool, expected) in [ + ("start_performance_monitoring", BrowserCommand::StartPerformanceMonitoring), + ("stop_performance_monitoring", BrowserCommand::StopPerformanceMonitoring), + ("get_core_web_vitals", BrowserCommand::GetCoreWebVitals), + ("get_memory_metrics", BrowserCommand::GetMemoryMetrics), + ("get_performance_summary", BrowserCommand::GetPerformanceSummary), + ] { + let params = json!({ "name": tool, "arguments": {} }); + assert!(server.handle_tool_call(Some(params)).is_ok(), "{} failed", tool); + let sent = rx.try_recv().expect("no command was broadcast"); + assert_eq!( + std::mem::discriminant(&sent), + std::mem::discriminant(&expected), + "{} dispatched the wrong command", + tool + ); + } + } + + #[test] + fn test_observability_tools_are_advertised() { + let (server, _rx) = setup_test_server(); + let result = server.handle_tools_list().unwrap(); + let names: Vec<&str> = result["tools"] + .as_array() + .unwrap() + .iter() + .map(|t| t["name"].as_str().unwrap()) + .collect(); + + // A tool that dispatches but isn't advertised is invisible to an agent, + // so listing and dispatch have to be checked together. + for expected in [ + "start_console_monitoring", + "stop_console_monitoring", + "get_console_logs", + "clear_console_logs", + "start_performance_monitoring", + "stop_performance_monitoring", + "get_core_web_vitals", + "get_memory_metrics", + "get_performance_summary", + "start_recording", + "stop_recording", + "save_recording", + "load_recording", + "start_playback", + "stop_playback", + "get_playback_state", + "step_playback", + ] { + assert!(names.contains(&expected), "{} missing from tools/list", expected); + } + } + #[test] fn test_network_monitoring_tools() { let (mut server, _rx) = setup_test_server();