From 33fbfc0778f86524dda9122dd615fa9b1b13b777 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 18:25:05 +0000 Subject: [PATCH 1/7] Add CI: build and test on Linux, macOS, and Windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The repository had no CI. A passing test suite that nothing runs is a suite that silently stops passing. .github/workflows/ci.yml builds and tests on all three platforms, with fail-fast disabled so one platform's failure doesn't mask the others. Notes on the design: - Uses the runners' preinstalled Rust and only first-party actions (checkout, cache), so no third-party actions enter the supply chain. - The Linux lane installs libgtk-3-dev and libwebkit2gtk-4.1-dev. Cargo cannot declare these, and without them the build fails inside gdk-sys with an error that never names the fix. Keeping the step in the workflow means it stays correct in a way documentation does not. - Builds before testing. tests/mcp_tests.rs spawns `cargo run -- --mcp` and waits a fixed two seconds for startup, which would be tight if the binary still had to compile. - No xvfb or display setup: no surviving test creates a window. Verified by running the full suite with no DISPLAY and no X server. - No fmt or clippy gate. The tree is not rustfmt-clean and the build emits warnings; gating either today would make the workflow red on arrival. Both are recorded in the roadmap as deliberate follow-ups. The Linux lane is verified end-to-end locally: cargo build --all-targets succeeds and cargo test reports 167 passed, 0 failed. The macOS and Windows lanes have never been exercised, so the first run may surface genuine cross-platform breakage — which is the matrix doing its job. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Pg53xf5xcUV7oc5CkaiCpz --- .github/workflows/ci.yml | 73 ++++++++++++++++++++++++++++++++++++++++ ROADMAP.md | 50 +++++++++++++++++++++++---- 2 files changed, 116 insertions(+), 7 deletions(-) create mode 100644 .github/workflows/ci.yml 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..355f948 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -110,14 +110,46 @@ 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 @@ -131,7 +163,11 @@ That makes this milestone load-bearing rather than housekeeping. delete. - [ ] **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. --- From b17c859144e9c306793dd9b0521625c0c2365556 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 18:59:16 +0000 Subject: [PATCH 2/7] Expose console and performance monitoring over MCP Console capture and the performance suite were both fully implemented and reachable over REST, but absent from the MCP tool list. An agent driving the browser could act on a page and never learn whether the page complained -- arguably the most useful question it could ask after an interaction. Adds nine tools, taking the advertised surface from 16 to 25: start_console_monitoring start_performance_monitoring stop_console_monitoring stop_performance_monitoring get_console_logs get_core_web_vitals clear_console_logs get_memory_metrics get_performance_summary Each maps to a BrowserCommand the engine already handles, so this is a binding rather than new capability. get_console_logs takes an optional level filter; omitting it means all levels, and is not an error. Tests assert the dispatched BrowserCommand rather than only that the call succeeded, and check that every new tool is advertised in tools/list -- a tool that dispatches but isn't listed is invisible to an agent, so both halves need covering. Verified over the real protocol: spawning the binary and calling tools/list returns all 25 tools. cargo test reports 172 passed, 0 failed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Pg53xf5xcUV7oc5CkaiCpz --- ROADMAP.md | 8 +- docs/mcp-server.md | 32 ++++++++ readme.md | 4 + src/mcp/mod.rs | 180 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 220 insertions(+), 4 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 355f948..aadd5d0 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -180,10 +180,10 @@ 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. +- [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. An agent can now + ask "did that click throw a console error?", which it previously could not. All nine were + already reachable over REST; only the MCP binding was missing. - [ ] **Expose recording/replay over MCP.** Let an agent record its own session and replay it. - [ ] **Structured errors for agents.** Failures should return machine-readable causes, not prose. - [ ] **MCP resources and prompts.** `handle_resources_list` and `handle_prompts_list` return empty. diff --git a/docs/mcp-server.md b/docs/mcp-server.md index 28531ca..c5c5341 100644 --- a/docs/mcp-server.md +++ b/docs/mcp-server.md @@ -207,6 +207,38 @@ Execute JavaScript code in the page context. **Arguments:** - `script` (string, required): JavaScript code to execute +### 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..b593161 100644 --- a/readme.md +++ b/readme.md @@ -173,6 +173,8 @@ 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 See [MCP Server Documentation](docs/mcp-server.md) for complete details. @@ -220,6 +222,8 @@ breakdown citing implementing files and test counts. - **Keyboard input isn't exposed.** `browser/keyboard.rs` handles shortcuts internally but isn't reachable over the API or MCP, so keyboard-driven testing (tab order, accessibility) isn't scriptable yet. +- **Recording/replay isn't exposed over MCP.** Reachable over REST only, so an + agent can't record or replay its own session. - **Assertions are minimal.** Recordings can store expected state, but there's no authoring UX and no pass/fail surfacing. diff --git a/src/mcp/mod.rs b/src/mcp/mod.rs index 7990c4c..11570b7 100644 --- a/src/mcp/mod.rs +++ b/src/mcp/mod.rs @@ -387,6 +387,65 @@ 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": {} }), + ), + self.tool_definition( + "get_performance_summary", + "Get an aggregate performance summary for the current page", + json!({ "type": "object", "properties": {} }), + ), ]; Ok(json!({ @@ -538,6 +597,22 @@ impl McpServer { script: script.to_string(), } } + "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 +884,111 @@ mod tests { assert!(result.is_ok()); } + #[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", + ] { + assert!(names.contains(&expected), "{} missing from tools/list", expected); + } + } + #[test] fn test_network_monitoring_tools() { let (mut server, _rx) = setup_test_server(); From d82a0fb340689a35064cc2b89e4f2815ade75f89 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 20:03:19 +0000 Subject: [PATCH 3/7] Expose recording and replay over MCP, and fix roadmap contradictions Recording and replay were reachable over REST only, so an agent could drive the browser but not capture what it did or replay it. Adds eight tools, taking the advertised MCP surface from 25 to 33: start_recording start_playback stop_recording stop_playback save_recording get_playback_state load_recording step_playback step_playback takes a direction rather than exposing StepForward and StepBackward as separate tools -- an agent narrowing down which event causes a failure thinks in terms of stepping. An unrecognised direction is an error rather than a silent default, because stepping the wrong way would mislead exactly the bisect the tool exists to serve. Also fixes two problems in ROADMAP.md: - It listed "CI of any kind. No .github/workflows." under Not started while also reporting the matrix green under M2. Self-contradictory, and precisely the drift the document exists to prevent. - Its test count was stale, and it described the keyboard gap as "expose browser/keyboard.rs". That module maps chrome shortcuts (Ctrl+T, Alt+Left) to commands the API already exposes directly, so binding it would add no capability. Testing tab order and keyboard accessibility needs events dispatched into the page, and synthetic KeyboardEvents from JavaScript cannot do it -- browsers refuse default actions like focus movement for untrusted events. Recorded as needing a design decision, not a binding. Verified over the real protocol: tools/list returns all 33 tools. cargo test reports 177 passed, 0 failed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Pg53xf5xcUV7oc5CkaiCpz --- ROADMAP.md | 36 +++++--- docs/mcp-server.md | 22 +++++ readme.md | 3 +- src/mcp/mod.rs | 225 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 271 insertions(+), 15 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index aadd5d0..fdb288c 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 **177 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 | `mcp/mod.rs` | 44 | | 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 @@ -51,10 +51,15 @@ including tests in files that were never compiled — see the note on dead modul ### 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). @@ -184,13 +189,18 @@ tests, DOM find/click/type, JavaScript execution, and network monitoring. five for performance — taking the advertised surface from 16 tools to 25. An agent can now ask "did that click throw a console error?", which it previously could not. All nine were already reachable over REST; only the MCP binding was missing. -- [ ] **Expose recording/replay over MCP.** Let an agent record its own session and replay it. +- [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. - [ ] **Structured errors for agents.** Failures should return machine-readable causes, not prose. - [ ] **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. +- [ ] **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. --- diff --git a/docs/mcp-server.md b/docs/mcp-server.md index c5c5341..9012000 100644 --- a/docs/mcp-server.md +++ b/docs/mcp-server.md @@ -207,6 +207,28 @@ 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 diff --git a/readme.md b/readme.md index b593161..f171b77 100644 --- a/readme.md +++ b/readme.md @@ -175,6 +175,7 @@ Then ask Claude to control the browser: - **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. @@ -222,8 +223,6 @@ breakdown citing implementing files and test counts. - **Keyboard input isn't exposed.** `browser/keyboard.rs` handles shortcuts internally but isn't reachable over the API or MCP, so keyboard-driven testing (tab order, accessibility) isn't scriptable yet. -- **Recording/replay isn't exposed over MCP.** Reachable over REST only, so an - agent can't record or replay its own session. - **Assertions are minimal.** Recordings can store expected state, but there's no authoring UX and no pass/fail surfacing. diff --git a/src/mcp/mod.rs b/src/mcp/mod.rs index 11570b7..f214c07 100644 --- a/src/mcp/mod.rs +++ b/src/mcp/mod.rs @@ -441,6 +441,78 @@ impl McpServer { "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", @@ -597,6 +669,60 @@ 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 { @@ -884,6 +1010,97 @@ mod tests { assert!(result.is_ok()); } + #[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(); @@ -984,6 +1201,14 @@ mod tests { "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); } From bf63243d1aa5f9c6cfea33f4079c53ffe3888814 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 20:05:42 +0000 Subject: [PATCH 4/7] Record the js-engine branch review in the roadmap The "Explicitly not doing" section recommended against embedding multiple JS engines, but was written without knowing that feat/js-engine-integration (30 commits, January 2025) had already attempted exactly that. Reviewed it. The conclusion stands, on stronger grounds than the original reasoning. That branch's SpiderMonkeyEngine builds a bare mozjs Runtime with SIMPLE_GLOBAL_CLASS, and JavaScriptCoreEngine a bare javascriptcore_rs Context: standalone interpreters with no DOM, no window, no document. Its Cargo.toml makes them independent of the webview rather than replacements -- webview = ["dep:wry", "dep:tao"] sits alongside v8 = ["dep:v8"] -- so the embedded engines never render the page. Cross-browser bugs live in DOM behavior, layout, CSS, and browser APIs. A bare ECMAScript interpreter cannot observe any of them, so even a working version would have answered a question nobody asked. The branch also never compiled; its own commit message records the build as failing. The CI matrix added in this PR already meets the underlying goal, running real WebKit and real Chromium against real pages. Also notes what is worth salvaging from that branch independently: its Cargo feature reorganization, which is sound practice regardless of the engine work. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Pg53xf5xcUV7oc5CkaiCpz --- ROADMAP.md | 36 ++++++++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index fdb288c..3ee5019 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -264,14 +264,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. From 79421aa30b98ff05fc846d7137d39448fa8ac528 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 20:12:24 +0000 Subject: [PATCH 5/7] Document that MCP tools do not return their results While scoping MCP resources, found that every MCP tool is 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 holds for get_core_web_vitals, get_page_info, find_element, execute_javascript, take_screenshot, and every other read. This predates the console and performance tools added earlier in this branch, but those commit and PR descriptions overstated what they delivered by saying an agent could now ask whether a click threw a console error. It can trigger the question and never receives the answer. Corrected here. The pieces exist but are not joined: the engine publishes results as BrowserEvents, and McpServer holds an event_rx receiver that nothing ever reads. The open design problem is correlation -- events carry no request id and some are emitted spontaneously by the page, so a time-window collect is racy. Records two candidate approaches and notes that whichever is chosen needs a timeout, since a blocking read without a deadline is exactly what hung CI on all three platforms earlier in this milestone. No behaviour change. Documents the limitation in the readme and the MCP guide so users reach for the REST API when they need a value back, and files the fix as the top item in Track A, blocking MCP resources and structured errors. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Pg53xf5xcUV7oc5CkaiCpz --- ROADMAP.md | 37 ++++++++++++++++++++++++++++++++++--- docs/mcp-server.md | 11 +++++++++++ readme.md | 6 ++++++ 3 files changed, 51 insertions(+), 3 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 3ee5019..75dbc4c 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -186,17 +186,48 @@ tests, DOM find/click/type, JavaScript execution, and network monitoring. ### M3 — Close the agent feedback loop - [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. An agent can now - ask "did that click throw a console error?", which it previously could not. All nine were + 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. + 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 diff --git a/docs/mcp-server.md b/docs/mcp-server.md index 9012000..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 diff --git a/readme.md b/readme.md index f171b77..d6365ad 100644 --- a/readme.md +++ b/readme.md @@ -225,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 From e241601c3737cba9339520f03b2aa3ebbc769350 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 20:28:29 +0000 Subject: [PATCH 6/7] Return real data from MCP reads, and record the --mcp display bug Implements the direct-accessor approach for the MCP result path. Reads that the engine can answer from its own state now return the value instead of the string "Command '' sent successfully". BrowserState carries clones of the engine's Arc> handles for the console monitor, performance monitor, network monitor, and event player. The MCP server runs on its own thread in the same process, so it reads them directly. try_local_read answers these seven tools: get_console_logs get_network_stats get_core_web_vitals export_network_har get_memory_metrics get_playback_state get_performance_summary Everything else, including all actions, falls through to the existing broadcast path unchanged. Without attached state the server behaves exactly as before, so the change is additive. get_playback_state returns position, duration, index and event count rather than the bare PlaybackState enum, since the enum alone does not tell an agent what to do next. Not covered: get_page_info, execute_javascript, find_element and take_screenshot need the WebView itself, which is owned by the thread running the event loop. Those remain fire-and-forget and are still documented as such. Tests populate a monitor and assert the read returns the data, that a level filter applies to it, that actions still acknowledge, and that a server without state falls back cleanly. One of them caught a real defect in its own harness: dropping the command receiver makes broadcast::send fail, so action tools error. Separately, records a pre-existing bug found while chasing an intermittent test failure. --mcp starts the browser engine, which initialises GTK and aborts when no display exists, racing the MCP thread's reply. Sequential runs fail 0/20; three concurrent fail 11/24; --headless narrows but does not close the window at 3/12. This makes tests/mcp_tests.rs a latent CI flake that has passed on luck, and it affects the headless Claude Desktop configuration in the readme. Deferred by request, documented so a future red run is recognised. cargo test: 182 passed, 0 failed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Pg53xf5xcUV7oc5CkaiCpz --- ROADMAP.md | 34 ++++++ src/browser/mod.rs | 4 +- src/main.rs | 12 ++- src/mcp/mod.rs | 263 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 311 insertions(+), 2 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 75dbc4c..f3cdcf8 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -49,6 +49,40 @@ including tests in files that were never compiled — see the dead-modules entry 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 - Test generation from recordings. 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/main.rs b/src/main.rs index 2f0a9bd..80a6eeb 100644 --- a/src/main.rs +++ b/src/main.rs @@ -233,12 +233,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 f214c07..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,9 +86,111 @@ 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) pub fn run(&mut self) -> Result<(), Box> { info!("🚀 Starting MCP server on stdio"); @@ -543,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 { @@ -1010,6 +1146,133 @@ 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(); From c4f743cb49331ac22b3a483d8e4a6e1a8dc5b8d3 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 20:42:52 +0000 Subject: [PATCH 7/7] Deduplicate the module tree so the crate compiles once main.rs declared api, browser, event, and templates, all of which lib.rs already exported. The binary therefore compiled the whole crate a second time and ran every shared test once per target. mcp moves into the library, since it was declared only in main.rs, and the binary now links against the library rather than re-declaring modules. platform stays library-only, as before. Before After Incremental rebuild after touching src/browser/mod.rs 21.5s 4.0s Warnings from the binary target 91 2 Test executions 182 140 Unique test names 140 140 The fall in executions is duplication disappearing, not lost coverage: `cargo test -- --list` yields identical sets of 140 names before and after, with an empty diff. This also corrects a number that earlier commits in this branch reported incorrectly. Figures like "182 passed" counted duplicate executions and were quoted as though they were test counts. The project has 140 tests; it had 140 before this change too. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Pg53xf5xcUV7oc5CkaiCpz --- ROADMAP.md | 27 +++++++++++++++++++-------- readme.md | 2 +- src/lib.rs | 1 + src/main.rs | 14 ++++++-------- 4 files changed, 27 insertions(+), 17 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index f3cdcf8..becc842 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -16,7 +16,7 @@ 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 **177 passed, 0 failed, 0 ignored**, verified on Linux +`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 @@ -31,7 +31,7 @@ including tests in files that were never compiled — see the dead-modules entry | 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), 33 tools | `mcp/mod.rs` | 44 | +| 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 | @@ -194,12 +194,23 @@ a matter of comparing their results rather than acquiring the coverage. 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. Deliberately not gated in CI yet: turning warnings into errors diff --git a/readme.md b/readme.md index d6365ad..be376b2 100644 --- a/readme.md +++ b/readme.md @@ -192,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 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 80a6eeb..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, };