From 008d5be0b2bba3110ec775d9192b38086a7e9517 Mon Sep 17 00:00:00 2001 From: Ralf Anton Beier Date: Sun, 26 Apr 2026 13:05:58 +0200 Subject: [PATCH 1/3] fix(playwright): close out remaining 8 dashboard test failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI on main has had Playwright failing for ~25 runs straight. After PR #214 bumped the per-test timeout to 60s the cascading timeout failures (graph viewer, variant nav) cleared, leaving eight real failures in the latest run on 58801b6: 1. artifacts.spec.ts:70 — mermaid-in-description not in svg-viewer 2. diagram-viewer.spec.ts:27,46 (schema-linkage) — wrong URL 3. filter-sort.spec.ts:225 — fill() doesn't fire keyup 4. filter-sort.spec.ts:253 — duplicate #cmd-k-input after reload 5. rivet-delta.spec.ts:142 — REQ-NEW-1 inside collapsed
6. serve-variant.spec.ts:25 — selectOption + inline onchange flaky 7. api.spec.ts:291 — /embed/artifacts/REQ-001 returned 404 This commit fixes all eight, split between tests, render, and serve: * `rivet-cli/src/render/artifacts.rs`: wrap any `
`
  emitted by `render_markdown(description)` in the same `.svg-viewer`
  toolbar shell used by the dedicated `diagram:` field. Adds
  `wrap_markdown_mermaid_in_svg_viewer` helper with three unit tests
  (single-block, multi-block, no-op). Pins the diagram-viewer parity
  contract for ARCH-CORE-001 and any other artifact whose description
  embeds a fenced mermaid block.

* `rivet-cli/src/serve/layout.rs`: replace the inline `onchange`
  attribute on `#variant-selector` with a delegated `change` event
  listener in the variant-sync script. Inline event handlers are
  silently dropped by some headless Chromium configurations (the test
  case observed `[selected]` on the option but the URL never changed),
  while addEventListener fires reliably under Playwright's
  selectOption().

* `rivet-cli/src/serve/mod.rs`: two middleware fixes.
  - Build the rewritten /embed URI directly from the path+query string
    instead of round-tripping through `Uri::into_parts` /
    `Uri::from_parts`. The round-trip preserved an empty
    scheme/authority pair that confused the axum 0.8 router so the
    handler-side path was empty, returning a wrapped 404. The
    relative-form URI is what hyper actually hands axum for incoming
    requests, and it routes correctly.
  - Add `/search` to the wrap_full_page bypass list. The Cmd+K JS
    fetches `/search?q=...` via `fetch()` (no HX-Request header) and
    dumps the body into `#cmd-k-results`; without this bypass, the
    fragment was being wrapped in the full layout, so a second
    `` ended up inside `#cmd-k-results`,
    tripping Playwright's strict-mode locator on reload.

* `tests/playwright/diagram-viewer.spec.ts`: schema-linkage URL is
  `/help` (where the diagram lives), not `/help/schema` (the type-list
  table).

* `tests/playwright/filter-sort.spec.ts`: switch from `fill("OSLC")` to
  `pressSequentially("OSLC")`. The HTMX trigger is `keyup changed
  delay:300ms`; `fill()` only fires `input`, so HTMX never saw the
  trigger and the URL never picked up `?q=OSLC`.

* `tests/playwright/rivet-delta.spec.ts`: open the `
Added` block before asserting REQ-NEW-1 is visible — the artifact ID lives inside that collapsed disclosure (rendered by scripts/diff-to-markdown.mjs:181-188). The serve middleware tweak is the only behaviour change visible to end-users; everything else is test-code or a layout-equivalence fix. Implements: REQ-007 Fixes: REQ-007 Verifies: REQ-007 Refs: FEAT-001 --- rivet-cli/src/render/artifacts.rs | 105 +++++++++++++++++++++++- rivet-cli/src/serve/layout.rs | 28 ++++++- rivet-cli/src/serve/mod.rs | 29 ++++++- tests/playwright/diagram-viewer.spec.ts | 7 +- tests/playwright/filter-sort.spec.ts | 10 ++- tests/playwright/rivet-delta.spec.ts | 5 ++ 6 files changed, 171 insertions(+), 13 deletions(-) diff --git a/rivet-cli/src/render/artifacts.rs b/rivet-cli/src/render/artifacts.rs index 1fd5eef..dbeed07 100644 --- a/rivet-cli/src/render/artifacts.rs +++ b/rivet-cli/src/render/artifacts.rs @@ -44,6 +44,101 @@ use super::RenderResult; use super::helpers::badge_for_type; use crate::serve::components::ViewParams; +/// Wrap any `
` 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 +/// `
`.
+///
+/// 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 `
` (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 = "
";
+    if !html.contains(NEEDLE) {
+        return html.to_string();
+    }
+    const TOOLBAR: &str = "
\ +
\ + \ + \ + \ +
"; + 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 `
` close. No nested `
` is
+        // possible inside a fenced code block, so the first `
` + // after the open is the right one. + if let Some(close_rel) = html[abs_start..].find("
") { + let close_end = abs_start + close_rel + "
".len(); + out.push_str(TOOLBAR); + out.push_str(&html[abs_start..close_end]); + out.push_str(""); // .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 +} + +#[cfg(test)] +mod mermaid_wrap_tests { + use super::*; + + // rivet: verifies REQ-007 (artifact diagram viewer parity) + #[test] + fn wraps_single_mermaid_block_in_svg_viewer() { + let html = "

intro

graph LR\nA-->B

outro

"; + let wrapped = wrap_markdown_mermaid_in_svg_viewer(html); + assert!(wrapped.contains("
")); + assert!(wrapped.contains("svg-viewer-toolbar")); + assert!(wrapped.contains("title=\"Fullscreen\"")); + // Pre block still in place. + assert!(wrapped.contains("
graph LR"));
+        // Surrounding paragraphs preserved.
+        assert!(wrapped.contains("

intro

")); + assert!(wrapped.contains("

outro

")); + } + + // rivet: verifies REQ-007 + #[test] + fn no_mermaid_means_no_change() { + let html = "

plain description with foo

"; + assert_eq!(wrap_markdown_mermaid_in_svg_viewer(html), html); + } + + // rivet: verifies REQ-007 + #[test] + fn wraps_multiple_mermaid_blocks() { + let html = + "
A
mid
B
"; + let wrapped = wrap_markdown_mermaid_in_svg_viewer(html); + // Two viewer wrappers present. + assert_eq!(wrapped.matches("
").count(), 2); + assert_eq!(wrapped.matches("svg-viewer-toolbar").count(), 2); + } +} + // ── Artifacts list ──────────────────────────────────────────────────────── pub(crate) fn render_artifacts_list(ctx: &RenderContext, params: &ViewParams) -> String { @@ -451,9 +546,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 `
` 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!(
-            "
Description
{}
", - render_markdown(desc) + "
Description
{rendered}
" )); } if let Some(status) = &artifact.status { diff --git a/rivet-cli/src/serve/layout.rs b/rivet-cli/src/serve/layout.rs index 10c987d..14f1a9d 100644 --- a/rivet-cli/src/serve/layout.rs +++ b/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( "/\ ` 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/") diff --git a/tests/playwright/diagram-viewer.spec.ts b/tests/playwright/diagram-viewer.spec.ts index 1467eb6..b6422bf 100644 --- a/tests/playwright/diagram-viewer.spec.ts +++ b/tests/playwright/diagram-viewer.spec.ts @@ -19,8 +19,11 @@ const VIEWER_PAGES = [ { name: "graph", url: "/graph?limit=2000" }, // Doc linkage view. { name: "doc-linkage", url: "/doc-linkage" }, - // Help / schema page renders the schema-linkage mermaid diagram. - { name: "schema-linkage", url: "/help/schema" }, + // /help renders the schema-linkage mermaid diagram (a Schema Linkage + // card showing artifact-type relationships in the dashboard's design + // language). The diagram lives on the help index, not /help/schema — + // /help/schema is the type-list table. + { name: "schema-linkage", url: "/help" }, ]; for (const page of VIEWER_PAGES) { diff --git a/tests/playwright/filter-sort.spec.ts b/tests/playwright/filter-sort.spec.ts index e8cee50..6eddd70 100644 --- a/tests/playwright/filter-sort.spec.ts +++ b/tests/playwright/filter-sort.spec.ts @@ -236,8 +236,14 @@ test.describe("Artifacts Filter/Sort/Pagination", () => { // Verify the input is wired for URL push-on-type. await expect(searchInput).toHaveAttribute("hx-push-url", "true"); - // Type a query — HTMX fires a debounced hx-get and pushes the URL. - await searchInput.fill("OSLC"); + // Type a query — HTMX fires a debounced hx-get on `keyup changed` + // (see rivet-cli/src/serve/components.rs ~line 216), so we need to + // generate real keypress events. Playwright's `fill()` sets the + // value via JS and only fires `input`, not `keyup`, so HTMX would + // never see the trigger. `pressSequentially` types one char at a + // time, dispatching keydown/keypress/keyup for each. + await searchInput.click(); + await searchInput.pressSequentially("OSLC"); await waitForHtmx(page); // URL must now carry ?q=OSLC. diff --git a/tests/playwright/rivet-delta.spec.ts b/tests/playwright/rivet-delta.spec.ts index f301e21..e04678f 100644 --- a/tests/playwright/rivet-delta.spec.ts +++ b/tests/playwright/rivet-delta.spec.ts @@ -190,6 +190,11 @@ test.describe("rivet-delta PR-comment output", () => { await expect( page.locator("h2", { hasText: "Rivet artifact delta" }), ).toBeVisible(); + // REQ-NEW-1 lives inside the collapsed
Added + // block (rendered by scripts/diff-to-markdown.mjs:181-188), so it is + // attached to the DOM but not "visible" until that
is + // expanded. Open it before checking visibility. + await page.locator("details summary", { hasText: "Added" }).click(); await expect( page .locator("code:not(.language-mermaid)", { hasText: "REQ-NEW-1" }) From 2d6d20fb50c92e63327d85ed9e5f82edc6718857 Mon Sep 17 00:00:00 2001 From: Ralf Anton Beier Date: Sun, 26 Apr 2026 13:13:01 +0200 Subject: [PATCH 2/3] style(render/artifacts): move tests module to end of file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clippy's `items_after_test_module` lint (escalated to deny in PR #195's restriction-family bump) was flagging the new `mermaid_wrap_tests` module added in the previous commit because many production functions (`render_artifact_detail`, `linkify_source_refs`, `find_source_ref`, etc.) follow it in source order. Move the `#[cfg(test)] mod tests` to the bottom of the file — same behaviour, satisfies the lint. No production code change. Trace: skip --- rivet-cli/src/render/artifacts.rs | 77 ++++++++++++++++--------------- 1 file changed, 39 insertions(+), 38 deletions(-) diff --git a/rivet-cli/src/render/artifacts.rs b/rivet-cli/src/render/artifacts.rs index dbeed07..40ba287 100644 --- a/rivet-cli/src/render/artifacts.rs +++ b/rivet-cli/src/render/artifacts.rs @@ -101,44 +101,6 @@ fn wrap_markdown_mermaid_in_svg_viewer(html: &str) -> String { out } -#[cfg(test)] -mod mermaid_wrap_tests { - use super::*; - - // rivet: verifies REQ-007 (artifact diagram viewer parity) - #[test] - fn wraps_single_mermaid_block_in_svg_viewer() { - let html = "

intro

graph LR\nA-->B

outro

"; - let wrapped = wrap_markdown_mermaid_in_svg_viewer(html); - assert!(wrapped.contains("
")); - assert!(wrapped.contains("svg-viewer-toolbar")); - assert!(wrapped.contains("title=\"Fullscreen\"")); - // Pre block still in place. - assert!(wrapped.contains("
graph LR"));
-        // Surrounding paragraphs preserved.
-        assert!(wrapped.contains("

intro

")); - assert!(wrapped.contains("

outro

")); - } - - // rivet: verifies REQ-007 - #[test] - fn no_mermaid_means_no_change() { - let html = "

plain description with foo

"; - assert_eq!(wrap_markdown_mermaid_in_svg_viewer(html), html); - } - - // rivet: verifies REQ-007 - #[test] - fn wraps_multiple_mermaid_blocks() { - let html = - "
A
mid
B
"; - let wrapped = wrap_markdown_mermaid_in_svg_viewer(html); - // Two viewer wrappers present. - assert_eq!(wrapped.matches("
").count(), 2); - assert_eq!(wrapped.matches("svg-viewer-toolbar").count(), 2); - } -} - // ── Artifacts list ──────────────────────────────────────────────────────── pub(crate) fn render_artifacts_list(ctx: &RenderContext, params: &ViewParams) -> String { @@ -905,3 +867,42 @@ fn find_source_ref(s: &str) -> Option { } 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 = "

intro

graph LR\nA-->B

outro

"; + let wrapped = wrap_markdown_mermaid_in_svg_viewer(html); + assert!(wrapped.contains("
")); + assert!(wrapped.contains("svg-viewer-toolbar")); + assert!(wrapped.contains("title=\"Fullscreen\"")); + // Pre block still in place. + assert!(wrapped.contains("
graph LR"));
+        // Surrounding paragraphs preserved.
+        assert!(wrapped.contains("

intro

")); + assert!(wrapped.contains("

outro

")); + } + + // rivet: verifies REQ-007 + #[test] + fn no_mermaid_means_no_change() { + let html = "

plain description with foo

"; + assert_eq!(wrap_markdown_mermaid_in_svg_viewer(html), html); + } + + // rivet: verifies REQ-007 + #[test] + fn wraps_multiple_mermaid_blocks() { + let html = "
A
mid
B
"; + let wrapped = wrap_markdown_mermaid_in_svg_viewer(html); + // Two viewer wrappers present. + assert_eq!(wrapped.matches("
").count(), 2); + assert_eq!(wrapped.matches("svg-viewer-toolbar").count(), 2); + } +} From ef3d00ba223700be39b44486ead0baafe207e6a7 Mon Sep 17 00:00:00 2001 From: Ralf Anton Beier Date: Sun, 26 Apr 2026 13:48:17 +0200 Subject: [PATCH 3/3] fix(serve): mount dashboard routes under /embed via Router::nest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #217 was meant to fix `tests/playwright/api.spec.ts:291` (oEmbed iframe page returning 404), but the in-place URI rewrite in `wrap_full_page` middleware turned out to be insufficient under axum 0.8: the router uses internal path state set up before `from_fn_with_state` middleware runs, so `head.uri = rewritten` is silently ignored. Added integration test `embed_artifact_returns_200_with_embed_layout` which reproduces the original 404 and pins the desired behaviour. Replace the URI-mutation approach with the axum-supported `Router::nest("/embed", view_routes())` pattern: extract the dashboard view routes into a small `view_routes` builder closure, mount it at both `/` and `/embed`. axum's `nest` strips the prefix when delegating to the inner router, so `/embed/artifacts/REQ-001` reaches the same `artifact_detail` handler as `/artifacts/REQ-001`. The `wrap_full_page` middleware still detects `is_embed = path.starts_with("/embed/")` to pick the embed layout (no nav, no sidebar), but no longer touches the request URI. The route inventory is preserved bit-for-bit: * All previously-routed paths still resolve at `/`. * Asset/API/hook routes (`/source-raw/{*path}`, `/api/links/{id}`, `/oembed`, `/api/v1/*`, `/wasm/{*path}`, `/docs-asset/{*path}`, `/assets/htmx.js`, `/assets/mermaid.js`, `/reload`, `/mcp`) stay at the root only — they were never useful under `/embed`. Tests: * `embed_artifact_returns_200_with_embed_layout` — regression guard (previously failing, now passing). * `embed_unknown_artifact_returns_200_with_not_found_body` — embed wrapping still applies for missing-artifact case. * All existing `oembed_*` and `embed_*` tests still pass (10/10). Implements: REQ-007 Fixes: REQ-007 Verifies: REQ-007 Refs: FEAT-001 --- rivet-cli/src/serve/mod.rs | 132 ++++++++++++++------------- rivet-cli/tests/serve_integration.rs | 61 +++++++++++++ 2 files changed, 128 insertions(+), 65 deletions(-) diff --git a/rivet-cli/src/serve/mod.rs b/rivet-cli/src/serve/mod.rs index e4d4019..e439063 100644 --- a/rivet-cli/src/serve/mod.rs +++ b/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 { + 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,37 +1000,18 @@ async fn wrap_full_page( || original_path == "/embed"; let method = req.method().clone(); - // Strip /embed prefix so existing route handlers match. - // - // axum 0.8 routing uses `req.uri().path()` at request-time to - // dispatch, so rewriting the URI in a top-level middleware is - // sufficient to redirect /embed/foo → /foo without registering - // duplicate routes. Two subtleties: + // 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. // - // 1. `Uri::from_parts(parts).unwrap()` can panic if the URI loses - // its scheme/authority and we keep them set; build the new URI - // directly from path+query so the result is a relative-form URI - // (which is what hyper hands axum for incoming requests anyway). - // 2. The matcher in axum 0.8 also caches `OriginalUri` in - // extensions; we don't strip that, so callers that inspect the - // original /embed/... path can still do so. - 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 new_pq = if query.is_empty() { - new_path.to_string() - } else { - format!("{new_path}?{query}") - }; - let new_uri: axum::http::Uri = new_pq - .parse() - .expect("rewritten /embed path is a valid relative URI"); - let (mut head, body) = req.into_parts(); - head.uri = new_uri; - axum::extract::Request::from_parts(head, body) - } else { - req - }; + // 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") diff --git a/rivet-cli/tests/serve_integration.rs b/rivet-cli/tests/serve_integration.rs index 7e438c8..95c9368 100644 --- a/rivet-cli/tests/serve_integration.rs +++ b/rivet-cli/tests/serve_integration.rs @@ -1106,3 +1106,64 @@ fn stats_page_shows_variant_banner_when_scoped() { child.kill().ok(); child.wait().ok(); } + +// ── /embed/* path rewriting (REQ-007 + tests/playwright/api.spec.ts:291) ── + +#[test] +fn embed_artifact_returns_200_with_embed_layout() { + // Regression: the wrap_full_page middleware strips /embed and routes + // to /artifacts/{id} so the dashboard can iframe-embed an artifact + // without registering duplicate routes. A previous URI-rewriting + // bug (round-tripping `Uri::into_parts` / `from_parts`) left the + // inner router with an empty matched path, returning a wrapped 404 + // — exactly the symptom Playwright's api.spec.ts:291 catches. + let (mut child, port) = start_server(); + let (status, body, _) = fetch(port, "/embed/artifacts/REQ-001", false); + assert_eq!( + status, 200, + "/embed/artifacts/REQ-001 must route through to artifact_detail" + ); + // embed_layout (no nav, no .shell) — distinct from page_layout. + assert!( + !body.contains("class=\"shell\""), + "/embed/* must not render the sidebar shell" + ); + assert!( + !body.contains("Main navigation"), + "/embed/* must not render the main nav" + ); + // Artifact body still renders (REQ-001 always exists in the test fixture). + assert!( + body.contains("REQ-001"), + "embed body must contain the artifact ID, got body of length {}", + body.len() + ); + // htmx is loaded so the embedded view stays interactive. + assert!( + body.contains("htmx"), + "embed body must include htmx (script tag)" + ); + child.kill().ok(); + child.wait().ok(); +} + +#[test] +fn embed_unknown_artifact_returns_200_with_not_found_body() { + // Unknown artifact under /embed should still go through the embed + // layout — render_artifact_detail returns a 200 with a "Not Found" + // body, which the embed wrap preserves. Exercises the same + // middleware-strip path as the happy case. + let (mut child, port) = start_server(); + let (status, body, _) = fetch(port, "/embed/artifacts/DOES-NOT-EXIST", false); + assert_eq!(status, 200); + assert!( + body.contains("Not Found"), + "embed body for unknown artifact should include 'Not Found'" + ); + assert!( + !body.contains("Main navigation"), + "/embed/* must not render the main nav even for not-found" + ); + child.kill().ok(); + child.wait().ok(); +}