diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..9bc7443 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,172 @@ +name: CI + +# The gate on pull requests. Until this existed, chuzz ran nothing on a PR: +# `release.yml` fires on push to master and is the only other workflow, so the +# first build of any change was the macOS release job, after the merge. +# +# What this can and cannot check, and why it is split that way: +# +# `chuzz-control` is platform-neutral (serde, tokio, a unix socket) and builds +# and tests on Linux. `chuzz-gui` pulls the Blitz engine, which is a macOS +# preview runtime; nothing in either manifest is target-gated, so a Linux runner +# would try to build the whole windowing stack and fail for reasons no PR +# introduced. Making it build on Linux is separate portability work. So the +# Linux jobs cover what genuinely is portable, and the macOS job covers the app. +# +# Runners are `ubicloud-standard-*` throughout, except the macOS one: Ubicloud +# offers Linux only, so the app build uses the same Namespace profile +# `release.yml` does. + +on: + pull_request: + workflow_dispatch: + +# A force-push supersedes the run it interrupted. Unlike the release job, which +# must never be cancelled halfway, a cancelled PR check costs nothing. +concurrency: + group: ci-${{ github.ref }} + cancel-in-progress: true + +permissions: + contents: read + +jobs: + # Formatting needs no dependency graph, so it answers in seconds on the + # smallest runner and fails fast before anything expensive starts. + fmt: + runs-on: ubicloud-standard-2 + steps: + - uses: actions/checkout@v4 + + - name: Install Rust stable + uses: dtolnay/rust-toolchain@stable + with: + components: rustfmt + + - name: Check formatting + run: cargo fmt --all -- --check + + # Reads Cargo.lock only, so it needs neither a toolchain nor a network. This + # is the check that would have caught the engine cascade being repinned + # halfway; see the script's own header for why that failure reads as a broken + # engine rather than as two of them. + one-rev: + runs-on: ubicloud-standard-2 + steps: + - uses: actions/checkout@v4 + + - name: One rev per git source + run: scripts/check-one-rev-per-git-source.sh + + # The portable half of the workspace, linted and tested for real. + control: + runs-on: ubicloud-standard-4 + steps: + - uses: actions/checkout@v4 + + - name: Install Rust stable + uses: dtolnay/rust-toolchain@stable + with: + components: clippy + + - name: Cache cargo build + uses: Swatinem/rust-cache@v2 + with: + shared-key: chuzz-control-ci + + # `--all-targets` so the tests and the `chuzz-inspect` binary are linted + # too, not just the library. Warnings are denied here rather than in a + # config file so a local `cargo clippy` stays quiet and advisory. + - name: Clippy + run: cargo clippy -p chuzz-control --all-targets --all-features -- -D warnings + + - name: Test + run: cargo test -p chuzz-control --all-features + + # The frontend, which is where most changes actually land. Independent of the + # Rust jobs, so a TypeScript error does not wait on a cargo build. + # + # `bun install` must precede any build: the `prebuild` script runs + # `solid-layouts-library` from `node_modules/.bin`, and that binary arrives + # only because `solid-layouts-oxc` is a direct devDependency. Bun links the + # bins of direct dependencies into the root `.bin` and leaves a transitive one + # under its own dependent, which is how the release job failed for fourteen + # releases while `bun install` reported success. `--frozen-lockfile` so a PR + # cannot quietly resolve a different tree from the committed bun.lock. + frontend: + runs-on: ubicloud-standard-4 + defaults: + run: + working-directory: apps/chuzz/frontend + steps: + - uses: actions/checkout@v4 + + - name: Setup Bun + uses: oven-sh/setup-bun@v2 + with: + bun-version: latest + + - name: Install dependencies + run: bun install --frozen-lockfile + + # Proves the binary the release job needs is actually on PATH, and names + # it plainly if it is not. Cheap, and it turns the one failure that has + # historically reached master into a PR-time error with an obvious cause. + - name: Confirm the layouts CLI is linked + run: | + if [ ! -x node_modules/.bin/solid-layouts-library ]; then + echo "::error::solid-layouts-library is missing from node_modules/.bin." + echo "solid-layouts-oxc must stay a *direct* devDependency: bun links" + echo "the bins of direct dependencies only. See release.yml." + ls -la node_modules/.bin || true + exit 1 + fi + echo "solid-layouts-library is linked" + + - name: Lint + run: bun run lint + + - name: Typecheck + run: bun run typecheck + + - name: Test + run: bun run test:run + + # The app itself, on the platform it targets. This is the job that proves a + # change compiles against the pinned engine, which no Linux runner can do. + # + # `build-app.sh` is deliberately not used: bundling, signing and stamping are + # the release job's business. A PR only needs to know the code compiles and + # its tests pass. + app: + runs-on: namespace-profile-agency-tahoe + steps: + - uses: actions/checkout@v4 + + - name: Setup Bun + uses: oven-sh/setup-bun@v2 + with: + bun-version: latest + + # The Rust build script shells out to `bun run build`, so the frontend + # dependencies have to be present before cargo starts. + - name: Install frontend dependencies + working-directory: apps/chuzz/frontend + run: bun install --frozen-lockfile + + - name: Install Rust stable + uses: dtolnay/rust-toolchain@stable + with: + targets: aarch64-apple-darwin + components: clippy + + - name: Cache cargo build + uses: Swatinem/rust-cache@v2 + with: + shared-key: chuzz-macos-ci + + - name: Clippy + run: cargo clippy -p chuzz-gui --all-targets -- -D warnings + + - name: Test + run: cargo test --workspace diff --git a/Cargo.lock b/Cargo.lock index c7ccfba..510ef80 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -997,7 +997,7 @@ dependencies = [ [[package]] name = "chuzz-control" -version = "0.1.32" +version = "0.1.33" dependencies = [ "endpoint-libs 2.1.5 (git+https://github.com/pathscale/endpoint-libs.git?rev=7eff4d850f1c84c1104c0729c68f50711940b7fb)", "serde", @@ -1007,7 +1007,7 @@ dependencies = [ [[package]] name = "chuzz-gui" -version = "0.1.32" +version = "0.1.33" dependencies = [ "blitz-html", "blitz-net", diff --git a/apps/chuzz/frontend/src/api/client.ts b/apps/chuzz/frontend/src/api/client.ts index ed21b06..cf3fdaf 100644 --- a/apps/chuzz/frontend/src/api/client.ts +++ b/apps/chuzz/frontend/src/api/client.ts @@ -85,6 +85,14 @@ export interface BrowserEvents { "status-changed": StatusReadout; "panel-changed": PanelState; "debug-entry": DebugEntry; + /** + * The macOS View > View Source item was chosen. + * + * Carries no payload on purpose. What view source means depends on the active + * tab, which the chrome already knows and already decides for Cmd-U; sending + * a URL from the menu handler would be a second answer to the same question. + */ + "menu-view-source": null; } export type BrowserEvent = keyof BrowserEvents; diff --git a/apps/chuzz/frontend/src/api/mock.ts b/apps/chuzz/frontend/src/api/mock.ts index e1a9747..ab3660a 100644 --- a/apps/chuzz/frontend/src/api/mock.ts +++ b/apps/chuzz/frontend/src/api/mock.ts @@ -43,6 +43,10 @@ export function createMockApi(): BrowserApi { "status-changed": new Set(), "debug-entry": new Set(), "panel-changed": new Set(), + // Never emitted by the mock: there is no menu bar in a browser dev build. + // Present because the shell subscribes to it unconditionally, and an + // absent Set would throw on registration rather than simply stay quiet. + "menu-view-source": new Set(), }; function emit(event: K, payload: BrowserEvents[K]): void { diff --git a/apps/chuzz/frontend/src/stores/browser.tsx b/apps/chuzz/frontend/src/stores/browser.tsx index 1fb2d43..621c32e 100644 --- a/apps/chuzz/frontend/src/stores/browser.tsx +++ b/apps/chuzz/frontend/src/stores/browser.tsx @@ -181,6 +181,12 @@ function createBrowserStore() { }; window.addEventListener("keydown", onKeyDown, true); + // The macOS menu item runs the keystroke's action rather than its own, so + // the two cannot drift. Tracked like every other listener: registration is + // async, and an unmount mid-flight would otherwise leave it holding this + // owner. + track(api.on("menu-view-source", () => runShortcut("view-source"))); + onCleanup(() => { disposed = true; window.removeEventListener("keydown", onKeyDown, true); diff --git a/apps/chuzz/src/browser.rs b/apps/chuzz/src/browser.rs index 8b1b060..01200a1 100644 --- a/apps/chuzz/src/browser.rs +++ b/apps/chuzz/src/browser.rs @@ -853,7 +853,7 @@ async fn fetch_page_module( /// pretty-print or re-serialise it. A document that showed a parsed and /// re-emitted tree would be answering a different question, and for a page /// whose claim is "there is no script here" it would be the wrong answer. -fn source_html(text: &str) -> String { +pub(crate) fn source_html(text: &str) -> String { let escaped = text .replace('&', "&") .replace('<', "<") diff --git a/apps/chuzz/src/capture.rs b/apps/chuzz/src/capture.rs index f00245b..c68f09c 100644 --- a/apps/chuzz/src/capture.rs +++ b/apps/chuzz/src/capture.rs @@ -424,4 +424,79 @@ mod tests { "the rows do not stack in order: {rows:?}" ); } + + /// `view-source:` renders through the capture path, escaped rather than + /// parsed. + /// + /// Hermetic on purpose. The loader's `view-source:` branch fetches the + /// inner URL, so driving it end to end would put a network round trip in + /// the suite and make this test fail for reasons that have nothing to do + /// with rendering. What the branch does *after* the fetch is wrap the bytes + /// with `browser::source_html` and parse that, and it is the wrapping that + /// can regress: escaping that stopped escaping would render the source as a + /// page, which is the one thing view source must never do. + /// + /// So this paints exactly what the loader would hand to the renderer, and + /// reads the pixels back the way the wasm test does. + #[test] + fn a_source_document_paints_its_markup_rather_than_rendering_it() { + use blitz_dom::{BaseDocument, DocumentConfig}; + + // A page whose rendered form is unmistakably different from its source: + // an

would paint large and bold, and the tags would vanish. + const PAGE: &str = "

Example Domain

a & b

"; + let html = crate::browser::source_html(PAGE); + + // The escaping is the contract the picture depends on, so assert it + // before painting: a failure here explains a failure below. + assert!( + html.contains("<h1>") && !html.contains("

"), + "the markup must be escaped, not embedded: {html}" + ); + assert!( + html.contains("&amp;"), + "an entity in the source must itself be escaped, or the picture \ + shows `&` where the server sent `&`: {html}" + ); + + let document = blitz_html::HtmlDocument::from_html( + &html, + DocumentConfig { + html_parser_provider: Some(std::sync::Arc::new(blitz_html::HtmlProvider)), + ..Default::default() + }, + ); + let mut document: BaseDocument = document.into_inner(); + + let png = scratch("view-source.png"); + let _ = std::fs::remove_file(&png); + let tree = scratch("view-source-tree.txt"); + let _ = std::fs::remove_file(&tree); + let buffer = super::paint(&mut document, 1.0, 1440, 960, Some(&tree)); + super::write_png(&buffer, 1440, 960, &png).expect("the png should be written"); + + // The same reasoning as the wasm test: a blank image would satisfy any + // weaker assertion, and blank is this path's characteristic failure. + let painted = non_background_pixels(&png); + assert!( + painted.differing > 250, + "the source document painted almost nothing ({} of {}), which is \ + what a missing font looks like", + painted.differing, + painted.total + ); + + // And the tree proves it is *source*, not a rendered page: the text + // sits in a
, and no 

was ever created from the escaped input. + let dump = std::fs::read_to_string(&tree).expect("the tree dump should be written"); + assert!( + boxes(&dump).iter().any(|box_| box_.name == "pre"), + "the source should be laid out in a
: {dump}"
+        );
+        assert!(
+            !boxes(&dump).iter().any(|box_| box_.name == "h1"),
+            "an 

in the tree means the markup was rendered rather than \ + shown: {dump}" + ); + } } diff --git a/apps/chuzz/src/document_loader.rs b/apps/chuzz/src/document_loader.rs index 698c346..73f99b9 100644 --- a/apps/chuzz/src/document_loader.rs +++ b/apps/chuzz/src/document_loader.rs @@ -247,6 +247,40 @@ pub async fn load_for_capture( ) -> Result> { use blitz_dom::Document as _; + // `view-source:` is the browser's, and a capture that could not take it was + // the one address a tab could show and a PNG could not. The scheme is not a + // fetchable one, so this has to come before the net provider sees it: the + // inner URL is what gets fetched, and the bytes are escaped rather than + // parsed. + // + // `browser::source_html` rather than a second copy of the escaping. What + // the capture writes has to be byte-for-byte what the tab shows, or the PNG + // stops being evidence about the browser and becomes evidence about this + // function. + // + // Nothing below applies to a source document: it has no scripts to run and + // no images to wait for, so it returns here rather than falling through to + // the script pump. + if request.url.scheme() == "view-source" { + let inner = request.url.path().to_owned(); + let url = Url::parse(&inner).map_err(|error| format!("{inner} is not a URL: {error}"))?; + let (_, bytes) = net_provider + .fetch_async(Request::get(url)) + .await + .map_err(|error| format!("could not fetch {inner}: {error:?}"))?; + let html = crate::browser::source_html(&decode_body(&bytes)); + return Ok(CapturedDocument::Html(Box::new( + blitz_html::HtmlDocument::from_html( + &html, + DocumentConfig { + html_parser_provider: Some(Arc::new(HtmlProvider)), + ..Default::default() + }, + ) + .into_inner(), + ))); + } + let (resolved_url, bytes) = net_provider .fetch_async(request) .await diff --git a/apps/chuzz/src/tauri_main.rs b/apps/chuzz/src/tauri_main.rs index aa9eb9c..148c97d 100644 --- a/apps/chuzz/src/tauri_main.rs +++ b/apps/chuzz/src/tauri_main.rs @@ -1,8 +1,97 @@ // Do not open a console window alongside the browser on Windows. #![cfg_attr(all(not(test), target_os = "windows"), windows_subsystem = "windows")] +#[cfg(target_os = "macos")] +use tauri::Emitter as _; use tauri::Manager; +/// The macOS menu bar, and the one item in it chuzz adds. +/// +/// Cmd-U already worked: the binding lives in the Solid chrome, in +/// `resolveBrowserShortcut`, and reaches `runShortcut`. What was missing is the +/// menu entry, which is the only place a shortcut is *discoverable* on macOS. +/// Someone who does not already know the key has no way to find it. +/// +/// The whole bar has to be built, not just the one submenu. Setting a menu +/// replaces Tauri's default wholesale, so leaving out the app submenu would +/// take Quit, Hide and About with it, and leaving out Edit would break Copy and +/// Paste in the address bar. Everything except View is predefined, so this adds +/// an entry rather than reimplementing a menu bar. +/// +/// The item emits `menu-view-source` and the chrome runs it through the same +/// `runShortcut("view-source")` the keystroke does. A second implementation +/// here could drift from the keystroke's, and the interesting part of that +/// action, refusing to open `view-source:view-source:`, is a fact about the +/// active tab that the frontend knows and this does not. +#[cfg(target_os = "macos")] +fn build_menu( + handle: &tauri::AppHandle, +) -> tauri::Result> { + use tauri::menu::{AboutMetadata, Menu, MenuItem, PredefinedMenuItem, Submenu}; + + let app = Submenu::with_items( + handle, + "Chuzz", + true, + &[ + &PredefinedMenuItem::about(handle, None, Some(AboutMetadata::default()))?, + &PredefinedMenuItem::separator(handle)?, + &PredefinedMenuItem::services(handle, None)?, + &PredefinedMenuItem::separator(handle)?, + &PredefinedMenuItem::hide(handle, None)?, + &PredefinedMenuItem::hide_others(handle, None)?, + &PredefinedMenuItem::show_all(handle, None)?, + &PredefinedMenuItem::separator(handle)?, + &PredefinedMenuItem::quit(handle, None)?, + ], + )?; + + // Without this the address bar cannot copy or paste: on macOS those are + // menu-driven, and the webview never sees the keystroke if no item claims + // it. + let edit = Submenu::with_items( + handle, + "Edit", + true, + &[ + &PredefinedMenuItem::undo(handle, None)?, + &PredefinedMenuItem::redo(handle, None)?, + &PredefinedMenuItem::separator(handle)?, + &PredefinedMenuItem::cut(handle, None)?, + &PredefinedMenuItem::copy(handle, None)?, + &PredefinedMenuItem::paste(handle, None)?, + &PredefinedMenuItem::select_all(handle, None)?, + ], + )?; + + // `CmdOrCtrl+U` rather than `Cmd+U`, to match what the chrome accepts. + let view_source = MenuItem::with_id( + handle, + MENU_VIEW_SOURCE, + "View Source", + true, + Some("CmdOrCtrl+U"), + )?; + let view = Submenu::with_items(handle, "View", true, &[&view_source])?; + + let window = Submenu::with_items( + handle, + "Window", + true, + &[ + &PredefinedMenuItem::minimize(handle, None)?, + &PredefinedMenuItem::close_window(handle, None)?, + ], + )?; + + Menu::with_items(handle, &[&app, &edit, &view, &window]) +} + +/// The menu item's id, and the event the chrome listens for. One constant so +/// the two cannot drift apart. +#[cfg(target_os = "macos")] +const MENU_VIEW_SOURCE: &str = "menu-view-source"; + mod browser; #[cfg(feature = "capture")] mod capture; @@ -182,6 +271,21 @@ fn main() { ]) .setup(move |app| { setup_browser.attach_app(app.handle().clone()); + // macOS only: it is the only platform here with a menu bar, and the + // item exists to make Cmd-U discoverable rather than to add a + // second way of doing it. + #[cfg(target_os = "macos")] + { + let menu = build_menu(app.handle())?; + app.set_menu(menu)?; + app.on_menu_event(|app, event| { + if event.id() == MENU_VIEW_SOURCE { + // Emitted rather than handled here: the chrome owns + // what view-source means for the active tab. + let _ = app.emit(MENU_VIEW_SOURCE, ()); + } + }); + } // The Settings switches, as they were left, with `CHUZZ_CONTROL` // able to force inspection on but never off. That asymmetry is the // way back in: a window whose stored choice left inspection off can @@ -203,3 +307,42 @@ fn main() { .expect("failed to build Chuzz") .run(|_, _| {}); } + +#[cfg(all(test, target_os = "macos"))] +mod tests { + /// The menu item's id is also the event name the chrome listens for, and + /// the two live in different languages: `MENU_VIEW_SOURCE` here, and the + /// `"menu-view-source"` key of `BrowserEvents` in `api/client.ts`. + /// + /// Nothing else connects them. Renaming one and not the other leaves a menu + /// item that emits into the void, the item stays enabled, clicking it does + /// nothing, and no error is reported anywhere. So the string is pinned + /// against the TypeScript that consumes it, read from disk rather than + /// copied here, because a copy would agree with itself forever. + #[test] + fn the_menu_id_matches_the_event_the_chrome_listens_for() { + let client = include_str!("../frontend/src/api/client.ts"); + assert!( + client.contains(&format!("\"{}\":", super::MENU_VIEW_SOURCE)), + "`{}` is not declared in BrowserEvents; the menu item would emit an \ + event nothing is listening for", + super::MENU_VIEW_SOURCE + ); + + let store = include_str!("../frontend/src/stores/browser.tsx"); + assert!( + store.contains(&format!("api.on(\"{}\"", super::MENU_VIEW_SOURCE)), + "the chrome does not subscribe to `{}`", + super::MENU_VIEW_SOURCE + ); + + // And the mock has to carry it too, or a dev build throws on + // registration instead of quietly having no menu. + let mock = include_str!("../frontend/src/api/mock.ts"); + assert!( + mock.contains(&format!("\"{}\":", super::MENU_VIEW_SOURCE)), + "the mock api has no handler set for `{}`", + super::MENU_VIEW_SOURCE + ); + } +} diff --git a/apps/chuzz/src/wasm_page.rs b/apps/chuzz/src/wasm_page.rs index 9066abd..1fbb024 100644 --- a/apps/chuzz/src/wasm_page.rs +++ b/apps/chuzz/src/wasm_page.rs @@ -248,4 +248,50 @@ mod tests { assert!(validate_module(b"\0as").is_err()); assert!(validate_module(b"").is_err()); } + + /// A guest that instantiates, exports `run` and returns `OK` without + /// touching the document is the one failure that looks like a success. + /// + /// It is not an error, so `run_guest_bytes` must return the document rather + /// than refusing, and the three cases in + /// `browser::tests::a_failed_module_leaves_the_fallback_standing` do not + /// reach it: each of those fails before or during the call, and this one + /// gets all the way to `Ok`. The warning at the top of this file is the + /// only thing standing between a blank page and no explanation for it, so + /// what is pinned here is that the inert guest is distinguishable from a + /// guest that built something. + #[test] + fn a_guest_returning_ok_without_mutating_is_not_an_error() { + let inert = wat::parse_str( + r#"(module + (memory (export "memory") 1) + (func (export "run") (result i32) (i32.const 0)))"#, + ) + .expect("the inert guest should assemble"); + + let (document, mount) = empty_document(DocumentConfig::default()); + let built = run_guest_bytes(&inert, document, mount) + .expect("a guest that returns OK is not a failure, however little it did"); + + // The mount is still childless, which is exactly the state the warning + // describes and the reason the page would come out blank. + assert_eq!( + built.get_node(mount).map(|node| node.children.len()), + Some(0), + "the inert guest must not have built anything" + ); + + // And the fixture guest, through the same door, does mutate. Without + // this half the assertion above would also pass if `run_guest_bytes` + // had quietly stopped running guests at all. + let fixture = std::fs::read(fixture_module()).expect("fixture module"); + let (document, mount) = empty_document(DocumentConfig::default()); + let built = run_guest_bytes(&fixture, document, mount).expect("the fixture guest runs"); + assert!( + built + .get_node(mount) + .is_some_and(|node| !node.children.is_empty()), + "the fixture guest should have built a tree, so the check above means something" + ); + } } diff --git a/scripts/check-one-rev-per-git-source.sh b/scripts/check-one-rev-per-git-source.sh new file mode 100755 index 0000000..913e919 --- /dev/null +++ b/scripts/check-one-rev-per-git-source.sh @@ -0,0 +1,49 @@ +#!/usr/bin/env bash +# +# Fail if Cargo.lock resolves any git dependency at more than one revision. +# +# Cargo treats two revs of one repository as two unrelated crates. Both get +# built, both export the same type names, and the types do not unify. Here that +# surfaces as +# +# error[E0277]: the trait bound `VelloCpuScenePainter: PaintScene` is not satisfied +# note: there are multiple different versions of crate `ps_anyrender_vello_cpu` +# +# which reads as a broken engine rather than as two of them. +# +# The engine is a cascade: ps-anyrender -> ps-blitz -> tauri-runtime-blitz -> +# chuzz. Repinning fewer than all of them does not leave the tree one release +# behind, it puts two copies of a crate in the graph. Every manifest in this +# workspace moves together, and `tauri-runtime-blitz` has to have been +# republished at the ps-blitz rev this repository wants. +# +# Worth its own check because chuzz's only build was, for a long time, the macOS +# release job: a mismatched rev reached master unchallenged and failed after the +# merge. Adapted from agencyzero's script of the same name, where the same class +# of mismatch cut 0.6.0 with a bundle that could not build. +# +# Reading the lockfile rather than running cargo keeps it honest on any runner +# and costs nothing, and the lockfile is the resolution the release job uses. +set -euo pipefail + +lock="${1:-Cargo.lock}" + +# `source = "git+URL?rev=SHA#SHORTSHA"` - strip the fragment, split on `?rev=`. +duplicates=$( + grep -o 'source = "git+[^"]*"' "$lock" | + sed 's/source = "git+//; s/"$//; s/#.*//' | + sort -u | + awk -F'\\?rev=' 'NF == 2 { count[$1]++; revs[$1] = revs[$1] "\n " $2 } + END { for (url in count) if (count[url] > 1) print url revs[url] }' +) + +if [[ -n $duplicates ]]; then + echo "Cargo.lock resolves a git dependency at more than one revision:" >&2 + echo "$duplicates" >&2 + echo >&2 + echo "Point every manifest at the same rev. A dependency that pins one of" >&2 + echo "these itself has to be republished at the rev this repository wants." >&2 + exit 1 +fi + +echo "one rev per git source"