diff --git a/.github/workflows/rivet-delta.yml b/.github/workflows/rivet-delta.yml index 894de18..8d548db 100644 --- a/.github/workflows/rivet-delta.yml +++ b/.github/workflows/rivet-delta.yml @@ -7,6 +7,13 @@ name: Rivet Delta # for deep inspection. The comment is updated in place on subsequent # pushes via a hidden marker (). # +# v0.4.3: the mermaid diagram is also pre-rendered to SVG and pushed to +# an orphan branch (`rivet-delta-renders`) so the graph shows up in email +# notifications and the GitHub mobile app — both of which display +# ```mermaid fenced blocks as raw source rather than rendering them. +# The interactive mermaid block is still preserved in a collapsed +#
below the image for the GitHub web UI. +# # This is informational — the workflow never blocks a merge. If the diff # can't be computed (parse errors, missing base, etc.) it posts a # warning comment instead of failing. @@ -23,7 +30,10 @@ on: - "scripts/diff-to-markdown.mjs" permissions: - contents: read + # `contents: write` needed for the SVG-to-orphan-branch push step. + # Everything else stays read-only; pull-requests:write is for the + # comment bot only. + contents: write pull-requests: write concurrency: @@ -43,6 +53,7 @@ jobs: BASE_SHA: ${{ github.event.pull_request.base.sha }} RUN_ID: ${{ github.run_id }} REPO: ${{ github.repository }} + RENDERS_BRANCH: rivet-delta-renders steps: - name: Checkout head uses: actions/checkout@v6 @@ -99,9 +110,110 @@ jobs: --version-label "pr-$PR_NUMBER" \ --offline || echo "export failed" > delta-out/export.err - - name: Generate markdown summary + # Pass 1 — generate markdown AND extract mermaid source to a file. + # The --svg-url is intentionally omitted; the second pass fills it in + # once we have the raw URL. + - name: Generate markdown (pass 1 — emit mermaid source) + working-directory: head + run: | + set -euo pipefail + node scripts/diff-to-markdown.mjs \ + --diff delta-out/diff.json \ + --impact delta-out/impact.json \ + --pr "$PR_NUMBER" \ + --run "$RUN_ID" \ + --repo "$REPO" \ + --mmd-out delta-out/diagram.mmd \ + > delta-out/comment.md + + # Render the mermaid source to SVG only if the first pass emitted a + # diagram (empty diffs don't write diagram.mmd). + - name: Render mermaid → SVG + id: render + working-directory: head + continue-on-error: true + run: | + set -euo pipefail + if [ ! -s delta-out/diagram.mmd ]; then + echo "no diagram to render (empty diff)" + echo "rendered=false" >> "$GITHUB_OUTPUT" + exit 0 + fi + npx -y -p @mermaid-js/mermaid-cli@11.4.2 mmdc \ + -i delta-out/diagram.mmd \ + -o delta-out/diagram.svg \ + -b transparent + echo "rendered=true" >> "$GITHUB_OUTPUT" + + # Push the SVG to a dedicated orphan branch so we get permanent, + # auth-free raw.githubusercontent.com URLs the email client and + # mobile app can fetch. Each PR-run gets its own path; history of + # prior renders stays viewable but can be GC'd by a separate + # workflow if the branch grows unwieldy. + - name: Push SVG to renders branch + if: steps.render.outputs.rendered == 'true' + id: push_svg + continue-on-error: true + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + set -euo pipefail + RENDERS_DIR="$RUNNER_TEMP/renders" + TARGET_DIR="pr-${PR_NUMBER}/run-${RUN_ID}" + SVG_PATH="${TARGET_DIR}/diagram.svg" + + # Fetch the orphan branch if it exists; otherwise create it. + git clone --depth 1 --branch "$RENDERS_BRANCH" \ + "https://x-access-token:${GITHUB_TOKEN}@github.com/${REPO}.git" \ + "$RENDERS_DIR" 2>/dev/null || { + git clone --depth 1 \ + "https://x-access-token:${GITHUB_TOKEN}@github.com/${REPO}.git" \ + "$RENDERS_DIR" + cd "$RENDERS_DIR" + git checkout --orphan "$RENDERS_BRANCH" + git rm -rf . 2>/dev/null || true + cat > README.md <<'EOF' + # rivet-delta-renders + + Pre-rendered SVG diagrams for `rivet-delta` PR comments. + Paths: `pr-/run-/diagram.svg`. + + This branch is written by `.github/workflows/rivet-delta.yml` + so the graph embedded in PR comments renders in email + notifications and the GitHub mobile app (both of which show + `\`\`\`mermaid` fenced blocks as raw source). + + Safe to prune old directories if size becomes a problem — + the only consumers are the comment `` references, + which become broken on deletion but don't block anything. + EOF + git add README.md + git commit -m "chore: initialize rivet-delta-renders branch" + git push origin "$RENDERS_BRANCH" + } + + cd "$RENDERS_DIR" + mkdir -p "$TARGET_DIR" + cp "$GITHUB_WORKSPACE/head/delta-out/diagram.svg" "$SVG_PATH" + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + git add "$SVG_PATH" + git commit -m "render: PR #${PR_NUMBER} run ${RUN_ID}" + git push origin "$RENDERS_BRANCH" + + SVG_URL="https://raw.githubusercontent.com/${REPO}/${RENDERS_BRANCH}/${SVG_PATH}" + echo "svg_url=${SVG_URL}" >> "$GITHUB_OUTPUT" + + # Pass 2 — re-generate the markdown with --svg-url so the comment + # body includes an tag pointing at the raw URL. If the SVG + # render/push failed we skip this pass and the original + # mermaid-only comment stays. + - name: Generate markdown (pass 2 — inject SVG) id: summary + if: steps.push_svg.outputs.svg_url != '' working-directory: head + env: + SVG_URL: ${{ steps.push_svg.outputs.svg_url }} run: | set -euo pipefail node scripts/diff-to-markdown.mjs \ @@ -110,9 +222,19 @@ jobs: --pr "$PR_NUMBER" \ --run "$RUN_ID" \ --repo "$REPO" \ + --svg-url "$SVG_URL" \ > delta-out/comment.md echo "comment_file=head/delta-out/comment.md" >> "$GITHUB_OUTPUT" + # Fallback path — if pass 2 was skipped (no SVG), use the pass-1 + # comment we already generated. + - name: Expose pass-1 comment as fallback + if: steps.push_svg.outputs.svg_url == '' + id: summary_fallback + working-directory: head + run: | + echo "comment_file=head/delta-out/comment.md" >> "$GITHUB_OUTPUT" + - name: Upload delta artifacts uses: actions/upload-artifact@v4 with: @@ -134,4 +256,4 @@ jobs: issue-number: ${{ github.event.pull_request.number }} comment-id: ${{ steps.find_comment.outputs.comment-id }} edit-mode: replace - body-path: ${{ steps.summary.outputs.comment_file }} + body-path: ${{ steps.summary.outputs.comment_file || steps.summary_fallback.outputs.comment_file }} diff --git a/rivet-cli/src/serve/mod.rs b/rivet-cli/src/serve/mod.rs index fea2157..0ab8cce 100644 --- a/rivet-cli/src/serve/mod.rs +++ b/rivet-cli/src/serve/mod.rs @@ -1242,10 +1242,15 @@ async fn reload_handler( match result { Ok(()) => { - // Redirect back to wherever the user was (HTMX sends HX-Current-URL). - // Extract the path portion from the full URL (e.g. "http://localhost:3001/documents/DOC-001" → "/documents/DOC-001"). - // Navigate back to wherever the user was (HTMX sends HX-Current-URL). - // HX-Location does a client-side HTMX navigation (fetch + swap + push-url). + // Use HX-Redirect (full browser navigation) instead of + // HX-Location targeting #content. The sidebar badges + // (artifact count, document count, variant count, STPA + // count, diagnostic count) live OUTSIDE #content, so a + // partial swap left them stale after every reload. A full + // page navigation re-renders the whole shell — cheap + // because HTMX does the redirect in the same browser + // session and the prior page state was fetched just moments + // earlier. let redirect_url = headers .get("HX-Current-URL") .and_then(|v| v.to_str().ok()) @@ -1260,14 +1265,9 @@ async fn reload_handler( }) .unwrap_or_else(|| "/".to_owned()); - let location_json = format!( - "{{\"path\":\"{}\",\"target\":\"#content\"}}", - redirect_url.replace('"', "\\\"") - ); - ( axum::http::StatusCode::OK, - [("HX-Location", location_json)], + [("HX-Redirect", redirect_url)], "reloaded".to_owned(), ) } @@ -1275,10 +1275,7 @@ async fn reload_handler( eprintln!("reload error: {e:#}"); ( axum::http::StatusCode::INTERNAL_SERVER_ERROR, - [( - "HX-Location", - "{\"path\":\"/\",\"target\":\"#content\"}".to_owned(), - )], + [("HX-Redirect", "/".to_owned())], format!("reload failed: {e}"), ) } diff --git a/scripts/diff-to-markdown.mjs b/scripts/diff-to-markdown.mjs index d1f52f0..62a2156 100644 --- a/scripts/diff-to-markdown.mjs +++ b/scripts/diff-to-markdown.mjs @@ -6,12 +6,24 @@ // node scripts/diff-to-markdown.mjs \ // --diff path/to/diff.json \ // --impact path/to/impact.json \ -// --pr 123 --run 456 --repo owner/name +// --pr 123 --run 456 --repo owner/name \ +// [--mmd-out path/to/diagram.mmd] \ +// [--svg-url https://raw.githubusercontent.com/.../diagram.svg] // // Emits markdown on stdout. The first line is a hidden HTML comment // marker () so the workflow can find-and-replace // the same comment on subsequent pushes. // +// Two-pass invocation pattern in the workflow: +// 1. First pass with --mmd-out to extract the mermaid source for the +// CLI renderer. Script emits the mermaid fenced block as usual. +// 2. After SVG is rendered and pushed to the orphan branch, a second +// pass with --svg-url inserts an reference above the mermaid +// block so the image shows up in email + mobile app (where +// mermaid fenced blocks render as raw source). The mermaid block +// stays wrapped in
so GitHub web users still get the +// interactive version. +// // Guarantees: // * Never throws on malformed input — emits a warning comment instead. // * Caps the mermaid graph at MERMAID_NODE_CAP nodes; overflow goes @@ -19,7 +31,7 @@ // * All inputs sanitised with `escapeMd` before rendering so artifact // IDs or titles containing markdown metacharacters cannot break out. -import { readFileSync } from "node:fs"; +import { readFileSync, writeFileSync } from "node:fs"; import { argv, stdout, stderr } from "node:process"; const MARKER = ""; @@ -35,6 +47,8 @@ function parseArgs(argv) { else if (arg === "--pr") out.pr = argv[++i]; else if (arg === "--run") out.run = argv[++i]; else if (arg === "--repo") out.repo = argv[++i]; + else if (arg === "--mmd-out") out.mmdOut = argv[++i]; + else if (arg === "--svg-url") out.svgUrl = argv[++i]; } return out; } @@ -99,12 +113,18 @@ function renderCountsTable({ added, removed, modified, impacted }) { } function renderMermaid({ added, removed, modified }) { + // Classification priority: added/removed > modified. If an ID shows up + // in more than one set (shouldn't happen from rivet diff, but defensive + // against malformed inputs), the terminal classification (it existed + // only on one side) wins over "modified in both". Earlier versions of + // this script had the opposite order and miscoloured new-file + // artifacts as modified. const nodes = new Map(); // id → class - for (const id of added) nodes.set(String(id), "added"); - for (const id of removed) nodes.set(String(id), "removed"); for (const m of modified) { if (m && m.id) nodes.set(String(m.id), "modified"); } + for (const id of added) nodes.set(String(id), "added"); + for (const id of removed) nodes.set(String(id), "removed"); const total = nodes.size; if (total === 0) { @@ -266,7 +286,39 @@ function main() { const { md: graph, truncated, total: nodeCount } = renderMermaid(n); if (graph) { md += "### Graph\n\n"; - md += graph; + + // If the workflow has already rendered the diagram to SVG and pushed + // it to the orphan branch, surface the image FIRST (renders in email + // notifications and the GitHub mobile app). The interactive mermaid + // block stays available below in a collapsed
for readers + // on the GitHub web UI. + if (args.svgUrl) { + md += `![Rivet artifact delta graph](${args.svgUrl})\n\n`; + md += "
Interactive graph (mermaid source)\n\n"; + md += graph; + md += "\n
\n\n"; + } else { + md += graph; + } + + // Write the mermaid source to disk for the workflow's SVG renderer. + // Only meaningful on the first pass (when --mmd-out is supplied); + // the second pass with --svg-url already has the SVG, so it will + // still write the file but the workflow ignores it. + if (args.mmdOut) { + try { + // Extract raw mermaid source from the fenced block (strip the + // ```mermaid and ``` fences so the CLI renderer sees pure + // graph syntax). + const m = graph.match(/```mermaid\n([\s\S]*?)```/); + if (m) { + writeFileSync(args.mmdOut, m[1]); + } + } catch (e) { + stderr.write(`diff-to-markdown: failed to write ${args.mmdOut}: ${e.message}\n`); + } + } + if (truncated) { md += `\n_Showing first ${MERMAID_NODE_CAP} of ${nodeCount} changed artifacts; full list below._\n\n`; } diff --git a/tests/playwright/navigation.spec.ts b/tests/playwright/navigation.spec.ts index dbbd244..d47e5ef 100644 --- a/tests/playwright/navigation.spec.ts +++ b/tests/playwright/navigation.spec.ts @@ -39,4 +39,29 @@ test.describe("Navigation", () => { const btn = page.locator('button:has-text("Reload")'); await expect(btn).toBeVisible(); }); + + // Regression: clicking Reload used to target #content only via + // HX-Location, so the sidebar badges (artifact count, doc count, + // variant count, STPA count) stayed stale after reload. Now uses + // HX-Redirect to drive a full browser navigation that re-renders + // the whole shell. We can't make the file-system change the backend + // reads in this test, so we pin the contract instead: the reload + // response must arrive as an HX-Redirect (full navigation), not an + // HX-Location (partial swap). That's what keeps the sidebar fresh. + test("reload triggers full-page navigation (HX-Redirect, not HX-Location)", async ({ + page, + }) => { + await page.goto("/artifacts"); + const resp = page.waitForResponse( + (r) => r.url().endsWith("/reload") && r.request().method() === "POST", + ); + await page.locator('button:has-text("Reload")').click(); + const response = await resp; + expect(response.status()).toBe(200); + const headers = response.headers(); + // Either-or: the old bad shape (HX-Location targeting #content) + // would leave the sidebar stale. The fix is a full navigation. + expect(headers["hx-redirect"]).toBeDefined(); + expect(headers["hx-location"]).toBeUndefined(); + }); }); diff --git a/tests/playwright/rivet-delta.spec.ts b/tests/playwright/rivet-delta.spec.ts index 4997865..ad3b212 100644 --- a/tests/playwright/rivet-delta.spec.ts +++ b/tests/playwright/rivet-delta.spec.ts @@ -101,7 +101,13 @@ function extractMermaid(md: string): string { function runDiffToMarkdown( diff: unknown, impact: unknown, - opts: { pr?: string; run?: string; repo?: string } = {}, + opts: { + pr?: string; + run?: string; + repo?: string; + mmdOut?: string; + svgUrl?: string; + } = {}, ): string { const dir = mkdtempSync(join(tmpdir(), "rivet-delta-test-")); const diffPath = join(dir, "diff.json"); @@ -121,6 +127,8 @@ function runDiffToMarkdown( "--repo", opts.repo ?? "pulseengine/rivet", ]; + if (opts.mmdOut) args.push("--mmd-out", opts.mmdOut); + if (opts.svgUrl) args.push("--svg-url", opts.svgUrl); return execFileSync("node", args, { cwd: REPO_ROOT, encoding: "utf8" }); } @@ -350,4 +358,87 @@ test.describe("rivet-delta PR-comment output", () => { // structurally sound. expect(md).toContain("REQ-\\|pipe\\|"); }); + + // v0.4.3: the second-pass --svg-url invocation must emit an + // reference above the mermaid block so the diagram renders in email + // notifications and the GitHub mobile app (both show ```mermaid + // fenced blocks as raw source otherwise). The interactive mermaid + // block stays available in a collapsed
for GitHub web. + test("svg-url flag injects image above the mermaid block", async ({ + page, + }) => { + const diff = { + added: ["REQ-X"], + removed: [], + modified: [], + summary: "1 added", + }; + const svgUrl = + "https://raw.githubusercontent.com/pulseengine/rivet/rivet-delta-renders/pr-42/run-101/diagram.svg"; + const md = runDiffToMarkdown(diff, { impacted: [] }, { svgUrl }); + + // Image must appear BEFORE the mermaid block — email clients that + // strip the mermaid source as plain text still show the image. + const imgIndex = md.indexOf(`![Rivet artifact delta graph](${svgUrl})`); + const mermaidIndex = md.indexOf("```mermaid"); + expect(imgIndex).toBeGreaterThan(-1); + expect(mermaidIndex).toBeGreaterThan(imgIndex); + + // Mermaid must now be inside a
so it collapses on the web + // UI and doesn't duplicate the image visually. + expect(md).toContain("
Interactive graph (mermaid source)"); + + // Render and verify the image tag is present in the DOM. + await page.setContent(`${mdToHtml(md)}`); + const img = page.locator("img[alt='Rivet artifact delta graph']"); + await expect(img).toHaveAttribute("src", svgUrl); + }); + + test("classification priority: added wins over modified when duplicated", async () => { + // Regression: v0.4.2 PR #192 delta showed a newly-added artifact + // (REQ-060) as "modified" yellow because the mermaid node map was + // built with modified last, overwriting the added entry. Fix: build + // modified first, then added/removed last so terminal classes win. + const diff = { + added: ["NEW-1"], + removed: ["OLD-1"], + // Deliberately duplicate NEW-1 in modified to simulate any upstream + // pipeline oddity that puts the same ID in two lists. + modified: [ + { + id: "NEW-1", + status_changed: null, + title_changed: null, + description_changed: true, + tags_added: [], + tags_removed: [], + links_added: [], + links_removed: [], + }, + ], + summary: "duplicate", + }; + const md = runDiffToMarkdown(diff, { impacted: [] }); + const mermaid = extractMermaid(md); + // NEW-1 must be coloured as "added" (terminal), not "modified". + expect(mermaid).toMatch(/NEW_1\["NEW-1"\]:::added/); + }); + + test("mmd-out flag writes the mermaid source to a file", async () => { + const diff = { + added: ["REQ-Y"], + removed: [], + modified: [], + summary: "1 added", + }; + const dir = mkdtempSync(join(tmpdir(), "rivet-delta-mmdout-")); + const mmdOut = join(dir, "diagram.mmd"); + runDiffToMarkdown(diff, { impacted: [] }, { mmdOut }); + + // The file must exist and contain raw mermaid source (no fences). + const body = readFileSync(mmdOut, "utf8"); + expect(body.trim().startsWith("graph LR")).toBe(true); + expect(body).not.toContain("```"); + expect(body).toContain("REQ_Y"); + }); });