fix(playwright): close out remaining 8 dashboard test failures

rivet-cli/src/render/artifacts.rs

@@ -44,6 +44,63 @@ use super::RenderResult;
 use super::helpers::badge_for_type;
 use crate::serve::components::ViewParams;
 
+/// Wrap any `<pre class="mermaid">…</pre>` block inside an HTML fragment
+/// (typically the output of `render_markdown` on an artifact description)
+/// in the shared `.svg-viewer` shell with the standard zoom-fit /
+/// fullscreen / popout toolbar.
+///
+/// The dashboard renders mermaid diagrams from two surfaces:
+///
+/// * The structured `diagram:` field — already wrapped in `.svg-viewer`
+///   by `render_artifact_detail` below.
+/// * Fenced ```mermaid blocks inside the artifact's `description` —
+///   emitted by `rivet_core::markdown::render_markdown` as bare
+///   `<pre class="mermaid">`.
+///
+/// Without this wrapping the description-based diagrams missed the
+/// toolbar, breaking the cross-view diagram-viewer parity contract
+/// pinned by `tests/playwright/artifacts.spec.ts:70`.
+///
+/// The transform is a textual scan rather than a DOM rewrite to keep
+/// the render path dependency-free; pulldown-cmark always emits the
+/// open tag verbatim as `<pre class="mermaid">` (see the
+/// `MERMAID_OPEN` sentinel in `rivet_core::markdown`), so a literal
+/// substring match is reliable.
+fn wrap_markdown_mermaid_in_svg_viewer(html: &str) -> String {
+    const NEEDLE: &str = "<pre class=\"mermaid\">";
+    if !html.contains(NEEDLE) {
+        return html.to_string();
+    }
+    const TOOLBAR: &str = "<div class=\"svg-viewer\">\
+         <div class=\"svg-viewer-toolbar\">\
+           <button onclick=\"svgZoomFit(this)\" title=\"Zoom to fit\">\u{229e}</button>\
+           <button onclick=\"svgFullscreen(this)\" title=\"Fullscreen\">\u{26f6}</button>\
+           <button onclick=\"svgPopout(this)\" title=\"Open in new window\">\u{2197}</button>\
+         </div>";
+    let mut out = String::with_capacity(html.len() + 256);
+    let mut cursor = 0;
+    while let Some(start) = html[cursor..].find(NEEDLE) {
+        let abs_start = cursor + start;
+        out.push_str(&html[cursor..abs_start]);
+        // Find the matching `</pre>` close.  No nested `<pre>` is
+        // possible inside a fenced code block, so the first `</pre>`
+        // after the open is the right one.
+        if let Some(close_rel) = html[abs_start..].find("</pre>") {
+            let close_end = abs_start + close_rel + "</pre>".len();
+            out.push_str(TOOLBAR);
+            out.push_str(&html[abs_start..close_end]);
+            out.push_str("</div>"); // .svg-viewer
+            cursor = close_end;
+        } else {
+            // Unterminated — emit the rest as-is and stop.
+            out.push_str(&html[abs_start..]);
+            return out;
+        }
+    }
+    out.push_str(&html[cursor..]);
+    out
+}
+
 // ── Artifacts list ────────────────────────────────────────────────────────
 
 pub(crate) fn render_artifacts_list(ctx: &RenderContext, params: &ViewParams) -> String {
@@ -451,9 +508,15 @@ pub(crate) fn render_artifact_detail(ctx: &RenderContext, id: &str) -> RenderRes
         html_escape(&artifact.title)
     ));
     if let Some(desc) = &artifact.description {
+        // Render markdown, then wrap any `<pre class="mermaid">` blocks in
+        // the same `.svg-viewer` toolbar shell used by the dedicated
+        // `diagram:` field below — so a mermaid diagram embedded in a
+        // description gets the same zoom-fit / fullscreen / popout
+        // controls as one supplied via the structured field. Pins the
+        // diagram-viewer parity contract (tests/playwright/artifacts.spec.ts:70).
+        let rendered = wrap_markdown_mermaid_in_svg_viewer(&render_markdown(desc));
         html.push_str(&format!(
-            "<dt>Description</dt><dd class=\"artifact-desc\">{}</dd>",
-            render_markdown(desc)
+            "<dt>Description</dt><dd class=\"artifact-desc artifact-diagram\">{rendered}</dd>"
         ));
     }
     if let Some(status) = &artifact.status {
@@ -804,3 +867,42 @@ fn find_source_ref(s: &str) -> Option<SourceRefMatch> {
     }
     None
 }
+
+// ── Tests ────────────────────────────────────────────────────────────────
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    // rivet: verifies REQ-007 (artifact diagram viewer parity)
+    #[test]
+    fn wraps_single_mermaid_block_in_svg_viewer() {
+        let html = "<p>intro</p><pre class=\"mermaid\">graph LR\nA-->B</pre><p>outro</p>";
+        let wrapped = wrap_markdown_mermaid_in_svg_viewer(html);
+        assert!(wrapped.contains("<div class=\"svg-viewer\">"));
+        assert!(wrapped.contains("svg-viewer-toolbar"));
+        assert!(wrapped.contains("title=\"Fullscreen\""));
+        // Pre block still in place.
+        assert!(wrapped.contains("<pre class=\"mermaid\">graph LR"));
+        // Surrounding paragraphs preserved.
+        assert!(wrapped.contains("<p>intro</p>"));
+        assert!(wrapped.contains("<p>outro</p>"));
+    }
+
+    // rivet: verifies REQ-007
+    #[test]
+    fn no_mermaid_means_no_change() {
+        let html = "<p>plain description with <code>foo</code></p>";
+        assert_eq!(wrap_markdown_mermaid_in_svg_viewer(html), html);
+    }
+
+    // rivet: verifies REQ-007
+    #[test]
+    fn wraps_multiple_mermaid_blocks() {
+        let html = "<pre class=\"mermaid\">A</pre> mid <pre class=\"mermaid\">B</pre>";
+        let wrapped = wrap_markdown_mermaid_in_svg_viewer(html);
+        // Two viewer wrappers present.
+        assert_eq!(wrapped.matches("<div class=\"svg-viewer\">").count(), 2);
+        assert_eq!(wrapped.matches("svg-viewer-toolbar").count(), 2);
+    }
+}

rivet-cli/src/serve/layout.rs

@@ -200,13 +200,18 @@ pub(crate) fn page_layout_with_variant(
         s
     };
     // Variant selector: only rendered when the project has a feature model.
+    //
+    // The change handler is wired up in the variant-sync script (below)
+    // via `addEventListener('change', …)` instead of an inline `onchange`
+    // attribute. Inline handlers are blocked by some headless Chromium
+    // configs (e.g. Playwright's default page CSP) and can be silently
+    // skipped when the selectOption action mutates the DOM via the
+    // accessibility tree. The addEventListener route runs in every
+    // env we care about.
     let variant_selector_html = if state.variants.has_model() {
         let mut s = String::from(
             "<span class=\"ctx-sep\">/</span>\
              <select id=\"variant-selector\" name=\"variant\" \
-             onchange=\"(function(sel){var u=new URL(window.location.href);\
-             if(sel.value){u.searchParams.set('variant',sel.value)}else{u.searchParams.delete('variant')}\
-             window.location.href=u.toString()})(this)\" \
              style=\"padding:.2rem .5rem;font-size:.72rem;font-family:var(--mono);\
              background:var(--surface);color:var(--text);border:1px solid var(--border);\
              border-radius:4px;max-width:220px\" \
@@ -323,6 +328,23 @@ document.addEventListener('DOMContentLoaded',renderMermaid);
   }}
   document.addEventListener('htmx:afterSettle',sync);
   document.addEventListener('htmx:pushedIntoHistory',sync);
+
+  // Variant dropdown navigation: setting `?variant=...` (or clearing it)
+  // and reloading. Implemented via `change` event listener instead of an
+  // inline onchange so that:
+  //   1. CSP profiles that block inline event handlers still work.
+  //   2. Playwright's selectOption() reliably triggers the navigation
+  //      (Chromium fires `change` after selectOption; an inline onchange
+  //      attribute was occasionally skipped, leaving `tests/playwright/
+  //      serve-variant.spec.ts:25` stuck on the original URL).
+  document.addEventListener('change', function(e){{
+    var sel = e.target;
+    if(!sel || sel.id !== 'variant-selector') return;
+    var u = new URL(window.location.href);
+    if(sel.value){{ u.searchParams.set('variant', sel.value); }}
+    else {{ u.searchParams.delete('variant'); }}
+    window.location.href = u.toString();
+  }});
 }})();
 </script>
 </head>

rivet-cli/src/serve/mod.rs

@@ -864,33 +864,64 @@ pub async fn run(app_state: AppState, bind: String, watch: bool) -> Result<()> {
             mcp_config,
         );
 
+    // Build the dashboard view routes once, then mount them at both
+    // `/` (the regular dashboard) and `/embed` (sidebar-free for
+    // iframe / VS Code WebView embedding).
+    //
+    // Mounting via `Router::nest` is the supported axum 0.8 way to
+    // handle prefix-stripping — the inner router sees `/artifacts/{id}`
+    // for both `/artifacts/REQ-001` and `/embed/artifacts/REQ-001`. An
+    // earlier version of `wrap_full_page` tried to strip `/embed` by
+    // mutating the request URI in middleware, but axum 0.8's router
+    // ignores URI mutation done in `from_fn_with_state` middleware
+    // (the matcher uses internal path state set up beforehand). The
+    // result was a wrapped 404 — see `tests/serve_integration.rs ::
+    // embed_artifact_returns_200_with_embed_layout` for the regression
+    // guard.
+    let view_routes = || -> Router<SharedState> {
+        Router::new()
+            .route("/", get(views::index))
+            .route("/artifacts", get(views::artifacts_list))
+            .route("/artifacts/{id}", get(views::artifact_detail))
+            .route("/artifacts/{id}/preview", get(views::artifact_preview))
+            .route("/artifacts/{id}/graph", get(views::artifact_graph))
+            .route("/validate", get(views::validate_view))
+            .route("/matrix", get(views::matrix_view))
+            .route("/matrix/cell", get(views::matrix_cell_detail))
+            .route("/graph", get(views::graph_view))
+            .route("/stats", get(views::stats_view))
+            .route("/coverage", get(views::coverage_view))
+            .route("/documents", get(views::documents_list))
+            .route("/documents/{id}", get(views::document_detail))
+            .route("/search", get(views::search_view))
+            .route("/verification", get(views::verification_view))
+            .route("/stpa", get(views::stpa_view))
+            .route("/eu-ai-act", get(views::eu_ai_act_view))
+            .route("/results", get(views::results_view))
+            .route("/results/{run_id}", get(views::result_detail))
+            .route("/source", get(views::source_tree_view))
+            .route("/source/{*path}", get(views::source_file_view))
+            .route("/diff", get(views::diff_view))
+            .route("/doc-linkage", get(views::doc_linkage_view))
+            .route("/traceability", get(views::traceability_view))
+            .route("/traceability/history", get(views::traceability_history))
+            .route("/help", get(views::help_view))
+            .route("/help/docs", get(views::help_docs_list))
+            .route("/help/docs/{*slug}", get(views::help_docs_topic))
+            .route("/help/schema", get(views::help_schema_list))
+            .route("/help/schema/{name}", get(views::help_schema_show))
+            .route("/help/links", get(views::help_links_view))
+            .route("/help/rules", get(views::help_rules_view))
+            .route("/externals", get(views::externals_list))
+            .route("/externals/{prefix}", get(views::external_detail))
+            .route("/variants", get(views::variants_list))
+    };
+
     let app = Router::new()
-        .route("/", get(views::index))
-        .route("/artifacts", get(views::artifacts_list))
-        .route("/artifacts/{id}", get(views::artifact_detail))
-        .route("/artifacts/{id}/preview", get(views::artifact_preview))
-        .route("/artifacts/{id}/graph", get(views::artifact_graph))
-        .route("/validate", get(views::validate_view))
-        .route("/matrix", get(views::matrix_view))
-        .route("/matrix/cell", get(views::matrix_cell_detail))
-        .route("/graph", get(views::graph_view))
-        .route("/stats", get(views::stats_view))
-        .route("/coverage", get(views::coverage_view))
-        .route("/documents", get(views::documents_list))
-        .route("/documents/{id}", get(views::document_detail))
-        .route("/search", get(views::search_view))
-        .route("/verification", get(views::verification_view))
-        .route("/stpa", get(views::stpa_view))
-        .route("/eu-ai-act", get(views::eu_ai_act_view))
-        .route("/results", get(views::results_view))
-        .route("/results/{run_id}", get(views::result_detail))
-        .route("/source", get(views::source_tree_view))
-        .route("/source/{*path}", get(views::source_file_view))
+        .merge(view_routes())
+        .nest("/embed", view_routes())
+        // Routes that exist only at the root (assets, APIs, hooks).
         .route("/source-raw/{*path}", get(source_raw))
-        .route("/diff", get(views::diff_view))
-        .route("/doc-linkage", get(views::doc_linkage_view))
-        .route("/traceability", get(views::traceability_view))
-        .route("/traceability/history", get(views::traceability_history))
         .route("/api/links/{id}", get(api_artifact_links))
         .route("/oembed", get(api::oembed))
         .nest(
@@ -905,16 +936,6 @@ pub async fn run(app_state: AppState, bind: String, watch: bool) -> Result<()> {
                 .with_state(state.clone()),
         )
         .route("/wasm/{*path}", get(wasm_asset))
-        .route("/help", get(views::help_view))
-        .route("/help/docs", get(views::help_docs_list))
-        .route("/help/docs/{*slug}", get(views::help_docs_topic))
-        .route("/help/schema", get(views::help_schema_list))
-        .route("/help/schema/{name}", get(views::help_schema_show))
-        .route("/help/links", get(views::help_links_view))
-        .route("/help/rules", get(views::help_rules_view))
-        .route("/externals", get(views::externals_list))
-        .route("/externals/{prefix}", get(views::external_detail))
-        .route("/variants", get(views::variants_list))
         .route("/docs-asset/{*path}", get(docs_asset))
         .route("/assets/htmx.js", get(htmx_asset))
         .route("/assets/mermaid.js", get(mermaid_asset))
@@ -979,24 +1000,18 @@ async fn wrap_full_page(
         || original_path == "/embed";
     let method = req.method().clone();
 
-    // Strip /embed prefix so existing route handlers match
-    let req = if is_embed && original_path.starts_with("/embed") {
-        let new_path = original_path.strip_prefix("/embed").unwrap_or("/");
-        let new_path = if new_path.is_empty() { "/" } else { new_path };
-        let mut parts = req.uri().clone().into_parts();
-        let new_pq = if query.is_empty() {
-            new_path.to_string()
-        } else {
-            format!("{new_path}?{query}")
-        };
-        parts.path_and_query = Some(new_pq.parse().unwrap());
-        let new_uri = axum::http::Uri::from_parts(parts).unwrap();
-        let (mut head, body) = req.into_parts();
-        head.uri = new_uri;
-        axum::extract::Request::from_parts(head, body)
-    } else {
-        req
-    };
+    // The /embed/* routes are registered separately via
+    // `Router::nest("/embed", …)` (see `run` above) so we do NOT mutate
+    // the request URI here.  An earlier version of this middleware
+    // tried to strip /embed in-place via `head.uri = rewritten`, but
+    // axum 0.8's router consults internal path state set up before
+    // top-level `from_fn_with_state` middleware runs and ignores the
+    // URI mutation — confirmed by `tests/serve_integration.rs ::
+    // embed_artifact_returns_200_with_embed_layout`, which kept seeing
+    // a wrapped 404 with the in-place rewrite.
+    //
+    // What we still need from `is_embed` here: pick the right layout
+    // when wrapping the inner handler's HTML body below.
     let path = if is_embed && original_path.starts_with("/embed") {
         original_path
             .strip_prefix("/embed")
@@ -1010,12 +1025,20 @@ async fn wrap_full_page(
 
     // Only wrap GET requests to view routes (not assets or APIs)
     // For "/" without print/embed, the index handler already renders the full page.
+    //
+    // /search is a fragment-only endpoint — the Cmd+K JS fetches it via
+    // `fetch()` (no `HX-Request` header) and dumps the body into
+    // `#cmd-k-results`. If we wrap it in the full layout, a second
+    // `<input id="cmd-k-input">` ends up nested inside `#cmd-k-results`,
+    // tripping Playwright's strict-mode locator and confusing keyboard
+    // navigation. Treat /search like the other API endpoints.
     if method == axum::http::Method::GET
         && !is_htmx
         && (path != "/" || is_print || is_embed)
         && !path.starts_with("/api/")
         && !path.starts_with("/mcp")
         && !path.starts_with("/oembed")
+        && !path.starts_with("/search")
         && !path.starts_with("/assets/")
         && !path.starts_with("/wasm/")
         && !path.starts_with("/source-raw/")