feat: DevFlow Web Inspector with AJAX refresh and modal support#295
feat: DevFlow Web Inspector with AJAX refresh and modal support#295mattleibow wants to merge 13 commits into
Conversation
Adds 'maui devflow inspector' command that serves the running MAUI app as an interactive HTML page at localhost:5223. An external tool can connect to this URL and see the native app rendered with: - Screenshot background with positioned divs for each UI element - data-* attributes exposing all DevFlow element properties - Click → tap, wheel → scroll, pointer drag → gesture proxying - Toolbar with back/refresh buttons - Auto-refresh via WebSocket when UI tree changes Architecture: CLI hosts the inspector HTTP server (not in-app), proxies all interactions to the DevFlow agent via its existing REST API. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ewport sizing
- Remove toolbar and hover highlighting (host inspector provides its own)
- Extract HTML to inspector.html template with {{placeholders}}
- Separate CSS into devflow.css (editable with proper syntax highlighting)
- Use actual screenshot PNG dimensions for viewport (not tree bounds)
- Read PNG IHDR chunk for width/height
- Add Playwright tests validating HTML structure, no chrome, correct sizing
- Add ValidateXcodeVersion=false to Directory.Build.props
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…wright tests - Viewport dimensions now come from agent status API (device.windowWidth/Height) which works cross-platform (Android, iOS, Mac, Windows) - Fallback to PNG IHDR dimensions if status unavailable - CSS zoom-to-fit: viewport scales down to fit browser window (never upscales) - JS coordinate conversion accounts for zoom scale factor - Replace Node.js Playwright tests with proper C# xUnit + Microsoft.Playwright tests in src/DevFlow/Microsoft.Maui.DevFlow.Inspector.Tests/ - 12 tests covering: viewport sizing, zoom, no chrome, element positioning, nested tree, data attributes, click→tap interaction, screenshot endpoint Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The web inspector is now served directly by the broker at
http://localhost:19223/inspector/ (or /inspector/{agentId}/ for
multi-agent). Since the broker is already running whenever a MAUI
app connects, this removes the need for a separate 'maui devflow
inspector' command.
Changes:
- BrokerServer: add /inspector routes that proxy to InspectorServer
- InspectorServer: add HandleBrokerRequestAsync + WebSocket relay
- Remove standalone 'inspector' CLI subcommand
- Make all asset paths relative (works under any URL prefix)
- devflow.js: derive basePath from location.pathname
- Clean up inspector on agent disconnect
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Replace full page reload with AJAX state polling (/api/state endpoint)
to avoid page flash during 3-second refresh interval
- Fix FindRootPageId to use last child of Window (topmost/modal page)
instead of first child, fixing blank screenshot when modal is showing
- Change tap handler to return 200 {ok:false} instead of 404 when no
tappable element found at coordinates
- Extract RenderElements method for generating element HTML independently
- Fix wheel scroll closure bug (capture coords before timeout fires)
- Update Playwright tests for flat div structure and AJAX behavior
- Add StateEndpoint and AjaxRefresh tests
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace naive remove-all/insert-all element refresh with a keyed diff based on data-id. Elements that haven't changed stay in the DOM, preserving browser hover state, inspector selection, and DevTools highlighting. Only style/attributes are updated in-place when they differ. New elements are inserted, removed ones are deleted. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds a broker-hosted DevFlow Web Inspector (/inspector/) that renders a connected app as HTML (screenshot + positioned element overlays) and proxies browser interactions (tap/scroll/gesture) back to the DevFlow agent. The PR also introduces inspector web assets, a Playwright-based test project scaffold, documentation, and a small extension to the Driver element model.
Changes:
- Add broker integration and inspector server implementation (HTML rendering, AJAX polling, interaction proxying, WebSocket proxy).
- Embed inspector web assets (HTML/CSS/JS) into the CLI and add documentation/spec.
- Extend
ElementInfowithWindowBoundsand introduce a Playwright test project + Playwright package version.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/DevFlow/Microsoft.Maui.DevFlow.Inspector.Tests/Microsoft.Maui.DevFlow.Inspector.Tests.csproj | New Playwright-based test project (references CLI + Driver). |
| src/DevFlow/Microsoft.Maui.DevFlow.Inspector.Tests/InspectorPageTests.cs | Playwright integration tests exercising inspector rendering and interactions. |
| src/DevFlow/Microsoft.Maui.DevFlow.Driver/ElementInfo.cs | Adds WindowBounds to support window-absolute positioning in the inspector. |
| src/Cli/Microsoft.Maui.Cli/Microsoft.Maui.Cli.csproj | Embeds inspector web assets as resources for serving from the broker/CLI. |
| src/Cli/Microsoft.Maui.Cli/DevFlow/Inspector/Web/inspector.html | HTML template for the rendered inspector page. |
| src/Cli/Microsoft.Maui.Cli/DevFlow/Inspector/Web/devflow.js | Client-side polling + event interception (click/wheel/pointer) → proxy endpoints. |
| src/Cli/Microsoft.Maui.Cli/DevFlow/Inspector/Web/devflow.css | Minimal styles for viewport + overlay elements. |
| src/Cli/Microsoft.Maui.Cli/DevFlow/Inspector/InspectorServer.cs | Core inspector HTTP routing, screenshot caching, proxy endpoints, and WS proxy logic. |
| src/Cli/Microsoft.Maui.Cli/DevFlow/Inspector/HtmlRenderer.cs | Visual tree → flat, positioned <div> overlay renderer with data-* attributes. |
| src/Cli/Microsoft.Maui.Cli/DevFlow/Broker/BrokerServer.cs | Routes /inspector/... through per-agent InspectorServer instances and serves an agent list page. |
| docs/DevFlow/inspector.md | Inspector design/spec documentation (currently diverges from implementation). |
| Directory.Packages.props | Adds central version for Microsoft.Playwright. |
| Directory.Build.props | Disables Xcode validation globally via ValidateXcodeVersion=false. |
|
That's a great idea and can also host in a VS Code extension easily as a side project. |
Expert Code Review — PR #295Methodology: 3 independent reviewers with adversarial consensus + targeted follow-up validation for disputed findings. Summary11 findings posted as inline comments — 3 critical, 5 moderate, 3 minor.
Disputed Findings (resolved via follow-up)Three findings initially flagged by only 1 reviewer were validated via follow-up agents — all 3 achieved consensus (2/2 follow-up agreement) and were included:
Discarded Findings (no consensus)
CI Status✅ 11/12 checks passed (CLA, build on macOS/Windows/Ubuntu across all products). 1 check ( Test CoverageThe PR includes a new Playwright-based test project (
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Expert Code Review: 11 findings posted inline (3 critical, 5 moderate, 3 minor). See the summary comment for methodology and discarded findings.
Generated by Expert Code Review (auto) for issue #295 · ● 21M
|
@lextm I m poc adding to the vscode maui extension |
|
Sure. I will add this concept to the wpf-labs repo and vscode-wpf extension as foundation of Live Preview. |
…sts) Security hardening: - BrokerServer: replace wildcard CORS with localhost-only Origin mirroring; validate Origin BEFORE dispatching POST handlers (the previous check ran after HandleShutdown(), so a cross-origin POST still tore down the broker even though we returned 403). Verified end-to-end with curl. - BrokerServer: reject non-loopback Origin on /ws/agent WebSocket upgrade. - InspectorServer: drop "Access-Control-Allow-Origin: *"; same-origin only. - InspectorServer: enforce Origin on POST endpoints and /ws/v1/ui/events WebSocket proxy (CSRF / cross-origin event subscription). - Add shared LocalOriginValidator helper with unit tests. Correctness: - InspectorServer: cap request bodies at 1 MiB, return 413 on oversize (both broker-hosted and standalone TCP paths), with per-read 10 s timeout on body reads to prevent slow-drip resource exhaustion. - InspectorServer: share a single AgentClient (and dispose it) instead of constructing one per request; gate the screenshot cache with a lock. - InspectorServer: fix WS proxy URL (/ws/events -> /ws/v1/ui/events) and wire lifetime cancellation through _lifetimeCts so the proxy actually tears down on dispose. - InspectorServer: read request bodies as raw bytes once and decode as UTF-8 (the prior ASCII path mangled non-ASCII fill text). - InspectorServer: validate PNG signature + reject negative dims before trusting embedded dimensions. - HtmlRenderer: format all numeric attributes with InvariantCulture and cache the template via Lazy<string> (ExecutionAndPublication). - BrokerServer: HtmlEncode AppName/Platform/Tfm and UrlEncode agent ids in the agent list page (stored XSS via agent metadata). - BrokerServer: dispose hosted inspectors on shutdown. Tests: - New InspectorHtmlRendererTests cover XSS escaping and invariant-culture number formatting (forced via pt-PT comma decimal). - New LocalOriginValidatorTests cover loopback / hostile / malformed Origin. - InspectorPageTests now compose URLs via Uri and parse doubles with InvariantCulture; default URL points at the broker single-agent fallback. Build: - Directory.Build.props: scope <ValidateXcodeVersion>false</> to Apple TFMs (ios/maccatalyst/macos/tvos) instead of repo-wide. 558 unit tests pass; broker security boundaries verified live via curl. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Clicking an Entry, Editor, SearchBar, or other text-input element in the web inspector now opens a teal-bordered overlay editor (input or textarea) on top of the field, pre-filled with the field's current text. Pressing Enter or clicking away commits the change via POST /api/fill; Escape cancels. The native control still receives a tap so it focuses on the app side. Detection uses data-type (Entry, Editor, SearchBar, TextField, TextBox, TextArea, TextView, UITextField, UITextView, EditText, NSTextField) and a heuristic on data-traits for 'textinput' / 'editable'. Uses document.elementFromPoint(clientX, clientY) to find the underlying element because the existing gesture handler calls viewport.setPointerCapture(), which causes real mouse clicks to report the viewport itself as e.target. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The inspector spec previously documented a standalone `maui devflow inspector` command on port 5223 with a toolbar and nested element tree. The implemented behavior is broker-hosted at port 19223 with a flat element list and no toolbar. Update the architecture diagram and Usage section to reflect the current broker-hosted setup at http://localhost:19223/inspector/, and re-label the standalone command as future work. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/review |
|
✅ Expert Code Review completed successfully! |
- LocalOriginValidator: reject literal Origin "null" (file://, sandboxed iframes, data:, opaque origins). Empty/missing Origin still allowed for non-browser clients. Tests updated. - InspectorServer.HandleClientAsync: apply LocalOriginValidator to POSTs and /ws/events upgrades in standalone-listener mode, mirroring broker. - InspectorPageTests.ViewportUsesWindowDimensionsFromAgent: correlate DOM data-width/height with /api/state instead of asserting != 390/844 (which would false-positive on real iPhone-sized devices). - InspectorPageTests.DataAttributesUseCamelCase: assert against raw HTML text (case-sensitive) instead of CSS attribute selectors (case-insensitive in HTML), so lowercase serialization regressions actually fail. - Directory.Build.props: scope ValidateXcodeVersion=false to non-CI builds so CI/official builds keep the Xcode-version guardrail; local dev still unblocked. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Expert Code Review: 6 findings posted inline (3 critical, 2 moderate, 1 minor). See the summary comment for full methodology and discarded findings.
Generated by Expert Code Review for issue #295 · ● 22.4M
- BrokerServer.HandleInspectorRoute: fix ConcurrentDictionary.GetOrAdd race where the factory may be invoked concurrently for the same agent id. Use TryGetValue + GetOrAdd with dispose-on-loss so the discarded InspectorServer doesn't leak its AgentClient/HttpClient, CTS, and TCP listener. - BrokerServer.HandleRequestAsync: enforce loopback on RemoteEndPoint as a defense-in-depth check. The broker is designed for loopback only, but HttpListener falls back to binding on all interfaces (http://+:port/) on Windows/non-localhost platforms. Origin-based CSRF protection alone doesn't help against non-browser LAN callers (curl, scripts, attacker) that don't send Origin. All legitimate uses are loopback. - InspectorServer.HandleWebSocketProxy: bidirectional relay via Task.WhenAny so a closed browser tab unblocks the loop immediately instead of leaking a task until the agent next sends data (or _lifetimeCts is cancelled). Extracted a shared RelayLoopAsync helper. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/review |
|
✅ Expert Code Review completed successfully! |
Expert Code Review — PR #295Methodology: 3 independent reviewers with adversarial consensus Result: 6 findings posted as inline comments (3 🟡 moderate, 3 🟢 minor) All findings are posted inline — no overflow table needed. Discarded Findings (did not reach consensus)
CI Status✅ All 13 check runs passed (macOS + Windows builds, CLA) Test CoverageThe PR includes both unit tests (
|
There was a problem hiding this comment.
Expert Code Review: 6 findings posted inline (3 moderate, 3 minor). See the summary comment for methodology and discarded findings.
Generated by Expert Code Review for issue #295 · ● 22.1M
|
|
||
| var host = uri.Host; | ||
| return host.Equals("localhost", StringComparison.OrdinalIgnoreCase) |
There was a problem hiding this comment.
🟡 MODERATE · Consensus: 2/3 reviewers
CSRF via any-port localhost origin: IsAllowed validates scheme and host but not port. A malicious page at (localhost/redacted) (e.g., compromised dev server, VS Code webview) can POSTto(localhost/redacted) or inspector action endpoints. Its Origin: (localhost/redacted) passes this check because the host is localhost` regardless of port.
Per RFC 6454, an origin is scheme+host+port — a page on port 3000 is a distinct security principal.
Suggestion: Also validate the port matches the broker/inspector port, or add a CSRF nonce/session token mechanism:
return (host.Equals("localhost", ...) || host == "127.0.0.1" || ...)
&& (uri.Port == expectedPort || uri.Port == -1);| { | ||
| // Try partial match | ||
| connection = _agents.Values.FirstOrDefault(a => |
There was a problem hiding this comment.
🟢 MINOR · Consensus: 2/3 reviewers
Partial agent-ID prefix match can route to wrong agent: With multiple agents connected (e.g., com.example.FooApp-android and com.example.FooBarApp-ios), a request to /inspector/com.example.Foo/ matches the first one in ConcurrentDictionary iteration order (non-deterministic). Combined with the single-agent fallback on line 525, any invalid ID routes to the only agent — convenient but potentially surprising.
Suggestion: Require the prefix to be unambiguous (error if multiple agents match the prefix), or remove prefix matching entirely and rely only on exact match + single-agent fallback:
if (!_agents.TryGetValue(agentId, out var connection))
{
if (_agents.Count == 1)
connection = _agents.Values.First();
// else: 404 (no ambiguous prefix matching)
}| timeoutCts.CancelAfter(TimeSpan.FromSeconds(5)); | ||
|
|
||
| try | ||
| { | ||
| read = await stream.ReadAsync(buffer, timeoutCts.Token); | ||
| if (read == 0) return (null, false); |
There was a problem hiding this comment.
🟡 MODERATE · Consensus: 3/3 reviewers
Single-read HTTP parsing drops fragmented headers: ReadRequestAsync performs one stream.ReadAsync of up to 8192 bytes and expects the full HTTP headers within that single read. TCP can fragment data arbitrarily — if \r\n\r\n isn't found in the first read, the request is silently dropped (return (null, false)).
Failing scenario: A browser with many cookies on localhost, or a slow network/proxy, splits headers across multiple TCP segments. The request is silently dropped with no response and no log entry.
Suggestion: Accumulate reads in a loop until \r\n\r\n is found or a max header size (e.g., 64KB) is exceeded:
var ms = new MemoryStream();
while (headerEnd < 0 && ms.Length < MaxHeaderBytes)
{
read = await stream.ReadAsync(buffer, timeoutCts.Token);
if (read == 0) break;
ms.Write(buffer, 0, read);
// Re-scan for \r\n\r\n
}| if (!_inspectors.TryGetValue(connection.Registration.Id, out var inspector)) | ||
| { | ||
| var created = new InspectorServer(0, "localhost", connection.Registration.Port); | ||
| inspector = _inspectors.GetOrAdd(connection.Registration.Id, created); |
There was a problem hiding this comment.
🟡 MODERATE · Consensus: 3/3 reviewers
Inspector lifecycle gaps — stale/orphaned instances: Three related issues identified by all reviewers:
-
Race condition: If an agent disconnects between the
_agents.TryGetValue(line 517) and thisGetOrAdd, the newInspectorServeris permanently orphaned in_inspectors—MonitorAgentConnectionalready ran itsTryRemovefor this ID so the inspector is never cleaned up. -
Stale port on reconnect: If an agent restarts with a new port, the cached inspector still holds the old
AgentClient(host, oldPort). All requests route to the dead port. -
Shutdown leak: The
Shutdown()method (outside this diff) clears_agentsbut doesn't dispose_inspectors. If the process exits throughShutdown()withoutDispose(), inspectors leak.
Suggestion: After GetOrAdd, verify the agent is still registered. On port mismatch (reconnect), remove and recreate. Ensure Shutdown() also disposes inspectors:
if (!_agents.ContainsKey(connection.Registration.Id))
{
_inspectors.TryRemove(connection.Registration.Id, out _);
inspector.Dispose();
// Return 503
}| function patchElements(newHtml) { | ||
| // Parse new elements into a temp container | ||
| const temp = document.createElement('div'); | ||
| temp.innerHTML = newHtml; |
There was a problem hiding this comment.
🟢 MINOR · Consensus: validated via follow-up (3/3 agree — defense-in-depth)
innerHTML with server-rendered HTML — fragile XSS boundary: patchElements() sets temp.innerHTML = newHtml where content comes from the server's HtmlRenderer. Server-side HttpUtility.HtmlAttributeEncode escapes ", &, < which prevents breakout from double-quoted attributes today. However, innerHTML is inherently XSS-risky — if escaping or output format ever changes, this becomes exploitable.
Not an active exploit with current code, but a fragile trust boundary.
Suggestion: Consider using DOM APIs (createElement + setAttribute) instead of innerHTML parsing, or add a comment documenting the security invariant that the server must always produce safe HTML for this path.
| await agentWs.SendAsync(subscribe, System.Net.WebSockets.WebSocketMessageType.Text, true, ct); | ||
|
|
||
| // Relay agent messages to browser | ||
| var buffer = new byte[8192]; | ||
| while (!ct.IsCancellationRequested && agentWs.State == System.Net.WebSockets.WebSocketState.Open) |
There was a problem hiding this comment.
🟢 MINOR · Consensus: validated via follow-up (3/3 agree)
Standalone WebSocket proxy is unidirectional — closed tabs hang: HandleWebSocketProxy only reads from the agent WebSocket (agent→browser direction). When a browser tab closes and sends a Close frame, nothing reads it from the TCP stream. The relay loop only exits when the agent sends its next message or _cts is cancelled.
Each abandoned browser tab holds a TcpClient + NetworkStream + ClientWebSocket open until the next agent event or global shutdown.
Contrast: The broker-mode HandleBrokerWebSocketProxy correctly uses bidirectional Task.WhenAny(agentToClient, clientToAgent) to detect browser-side closes.
Suggestion: Mirror the broker-mode pattern — add a concurrent task draining the browser→agent direction and use Task.WhenAny to exit when either side closes.
|
Did an independent multi-model review pass against current HEAD (
One thing I don't think was raised and is still present on HEAD:
Worth noting this may be moot today: the bundled Minor (low impact, optional): |
Summary
Adds a web-based visual inspector for .NET MAUI DevFlow that renders a running app as an interactive HTML page in the browser. The inspector integrates into the DevFlow broker daemon and is accessible at
http://localhost:19223/inspector/.Features
Architecture
/inspector/(agent list),/inspector/{agentId}/(app view)GET /api/statereturns JSON with screenshot URL + element HTML for AJAX updatesFiles
src/Cli/.../Inspector/InspectorServer.cs— HTTP server + broker integrationsrc/Cli/.../Inspector/HtmlRenderer.cs— Tree → flat HTMLsrc/Cli/.../Inspector/Web/— HTML template, CSS, JSsrc/DevFlow/.../Inspector.Tests/— Playwright testssrc/DevFlow/.../ElementInfo.cs— Added WindowBounds property