fix(reliability): harden diagnostics and test infrastructure

.github/actions/use-ci-release/action.yml

@@ -13,6 +13,21 @@ runs:
   using: composite
   steps:
     - name: Download release artifact
+      id: download
+      continue-on-error: true
+      uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4
+      with:
+        name: moli-release-${{ inputs.side }}
+        path: target/ci-artifacts/${{ inputs.side }}
+    - name: Check downloaded archive completeness
+      id: archive
+      continue-on-error: true
+      shell: bash
+      env:
+        RELEASE_SIDE: ${{ inputs.side }}
+      run: gzip -t "target/ci-artifacts/$RELEASE_SIDE/moli-release-$RELEASE_SIDE.tar.gz"
+    - name: Retry incomplete release download
+      if: steps.download.outcome != 'success' || steps.archive.outcome != 'success'
       uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4
       with:
         name: moli-release-${{ inputs.side }}

.github/scripts/unpack-ci-release.sh

@@ -16,6 +16,9 @@ esac
 release_name="moli-release-$side"
 archive="target/ci-artifacts/$side/$release_name.tar.gz"
 test -f "$archive"
+# A download action can terminate with a partial extracted archive. Reject it
+# before touching the executable directory, including after the bounded retry.
+gzip -t "$archive"
 mkdir -p target/ci-bin
 tar -xzf "$archive" -C target/ci-bin
 (cd "target/ci-bin/$release_name" && sha256sum --check SHA256SUMS)

.github/scripts/unpack-ci-release.test.cjs

@@ -0,0 +1,60 @@
+'use strict';
+
+const assert = require('node:assert/strict');
+const { spawnSync } = require('node:child_process');
+const { createHash } = require('node:crypto');
+const fs = require('node:fs');
+const os = require('node:os');
+const path = require('node:path');
+const test = require('node:test');
+
+const script = path.join(__dirname, 'unpack-ci-release.sh');
+
+function fixture(t) {
+  const root = fs.mkdtempSync(path.join(os.tmpdir(), 'moli-ci-release-test-'));
+  t.after(() => fs.rmSync(root, { recursive: true, force: true }));
+  const packageDir = path.join(root, 'package', 'moli-release-head');
+  const archive = path.join(root, 'target', 'ci-artifacts', 'head', 'moli-release-head.tar.gz');
+  fs.mkdirSync(packageDir, { recursive: true });
+  fs.mkdirSync(path.dirname(archive), { recursive: true });
+  const binary = Buffer.from('test release binary');
+  fs.writeFileSync(path.join(packageDir, 'moli'), binary);
+  fs.writeFileSync(path.join(packageDir, 'revision.txt'), 'expected-revision\n');
+  fs.writeFileSync(path.join(packageDir, 'SHA256SUMS'),
+    `${createHash('sha256').update(binary).digest('hex')}  moli\n`);
+  function pack() {
+    const result = spawnSync('tar', ['-czf', archive, '-C', path.join(root, 'package'), 'moli-release-head']);
+    assert.equal(result.status, 0, String(result.stderr));
+  }
+  pack();
+  return { root, archive, packageDir, pack };
+}
+
+function unpack(root, revision = 'expected-revision') {
+  return spawnSync('bash', [script, 'head', revision], { cwd: root, encoding: 'utf8' });
+}
+
+test('valid archive passes payload and exact revision verification', (t) => {
+  const { root } = fixture(t);
+  const result = unpack(root);
+  assert.equal(result.status, 0, result.stderr);
+  assert.equal(fs.readFileSync(path.join(root, 'target/ci-bin/moli-release-head/moli'), 'utf8'), 'test release binary');
+});
+
+test('truncated download cannot write a partial executable; redownload recovers', (t) => {
+  const { root, archive } = fixture(t);
+  const complete = fs.readFileSync(archive);
+  fs.writeFileSync(archive, complete.subarray(0, complete.length - 16));
+  assert.notEqual(unpack(root).status, 0);
+  assert.equal(fs.existsSync(path.join(root, 'target/ci-bin')), false);
+  fs.writeFileSync(archive, complete);
+  assert.equal(unpack(root).status, 0);
+});
+
+test('complete downloads still reject wrong revisions and altered payloads', (t) => {
+  const { root, packageDir, pack } = fixture(t);
+  assert.notEqual(unpack(root, 'wrong-revision').status, 0);
+  fs.writeFileSync(path.join(packageDir, 'moli'), 'tampered payload');
+  pack();
+  assert.notEqual(unpack(root).status, 0);
+});

moli-fetch/src/tests/support.rs

@@ -1003,14 +1003,85 @@ impl ScriptedHttpServer {
     }
 }
 
+#[test]
+fn scripted_server_waits_for_request_on_nonblocking_accepted_socket() {
+    let listener = TcpListener::bind("127.0.0.1:0").unwrap();
+    let mut client = TcpStream::connect(listener.local_addr().unwrap()).unwrap();
+    client
+        .set_read_timeout(Some(Duration::from_secs(2)))
+        .unwrap();
+    let (stream, _) = listener.accept().unwrap();
+    // macOS may inherit this mode from the listening socket.
+    stream.set_nonblocking(true).unwrap();
+    let hits = Arc::new(AtomicUsize::new(0));
+    let requests = Arc::new(Mutex::new(Vec::new()));
+    let responses = Arc::new(Mutex::new(VecDeque::from([ScriptedResponse::ok("ready")])));
+    let (done_tx, done_rx) = std_mpsc::channel();
+    let handler = {
+        let hits = Arc::clone(&hits);
+        let requests = Arc::clone(&requests);
+        thread::spawn(move || {
+            handle_scripted_connection(stream, hits, requests, responses);
+            done_tx.send(()).unwrap();
+        })
+    };
+    assert!(
+        matches!(
+            done_rx.recv_timeout(Duration::from_millis(100)),
+            Err(std_mpsc::RecvTimeoutError::Timeout)
+        ),
+        "a connection without HTTP bytes must not receive a scripted response"
+    );
+    client
+        .write_all(b"GET /delayed HTTP/1.1\r\nHost: localhost\r\n\r\n")
+        .unwrap();
+    let mut response = String::new();
+    client.read_to_string(&mut response).unwrap();
+    handler.join().unwrap();
+    assert!(response.ends_with("ready"));
+    assert_eq!(hits.load(Ordering::SeqCst), 1);
+    assert!(requests.lock()[0].starts_with("GET /delayed HTTP/1.1\r\n"));
+}
+
+#[test]
+fn scripted_server_does_not_consume_response_for_closed_unused_connection() {
+    let listener = TcpListener::bind("127.0.0.1:0").unwrap();
+    let client = TcpStream::connect(listener.local_addr().unwrap()).unwrap();
+    let (stream, _) = listener.accept().unwrap();
+    drop(client);
+    let hits = Arc::new(AtomicUsize::new(0));
+    let requests = Arc::new(Mutex::new(Vec::new()));
+    let responses = Arc::new(Mutex::new(VecDeque::from([ScriptedResponse::ok("ready")])));
+    handle_scripted_connection(
+        stream,
+        Arc::clone(&hits),
+        Arc::clone(&requests),
+        Arc::clone(&responses),
+    );
+    assert_eq!(hits.load(Ordering::SeqCst), 0);
+    assert!(requests.lock().is_empty());
+    assert_eq!(responses.lock().len(), 1);
+}
+
 fn handle_scripted_connection(
     mut stream: std::net::TcpStream,
     hits: Arc<AtomicUsize>,
     requests: Arc<Mutex<Vec<String>>>,
     responses: Arc<Mutex<VecDeque<ScriptedResponse>>>,
 ) {
+    stream
+        .set_nonblocking(false)
+        .expect("scripted HTTP connection should use blocking reads");
+    stream
+        .set_read_timeout(Some(Duration::from_secs(5)))
+        .expect("scripted HTTP connection should have a bounded read");
     let mut request = [0; 1024];
-    let bytes_read = stream.read(&mut request).unwrap_or(0);
+    let bytes_read = stream
+        .read(&mut request)
+        .expect("read scripted HTTP request");
+    if bytes_read == 0 {
+        return;
+    }
     let request_text = String::from_utf8_lossy(&request[..bytes_read]).into_owned();
     requests.lock().push(request_text);
     let _ = hits.fetch_add(1, Ordering::SeqCst) + 1;

moli-layout/tests/phase4_layout_contract.rs

@@ -1586,6 +1586,19 @@ fn inline_blocks_use_their_internal_last_line_baseline_and_overflow_fallback() {
     );
     assert_close(baselines[0], more.y + more.height);
     assert_close(baselines[1], more.y + more.height);
+
+    // Extra block-end padding moves the text-backed peers by the same amount
+    // while leaving the clipped inline-block's top edge fixed.
+    let fallback_news = rect(&fallback, RED);
+    let fallback_hao = rect(&fallback, GREEN);
+    styles
+        .primary
+        .insert(5, nav_item_style(BLUE, 29.0, Overflow::Hidden));
+    let deeper_fallback = render(&source, &mut styles, 300, 100);
+    assert_close(rect(&deeper_fallback, RED).y, fallback_news.y + 10.0);
+    assert_close(rect(&deeper_fallback, GREEN).y, fallback_hao.y + 10.0);
+    assert_close(rect(&deeper_fallback, BLUE).y, more.y);
+    assert_close(rect(&deeper_fallback, BLUE).height, more.height + 10.0);
 }
 
 #[test]

moli-layout/tests/phase5_output_contract.rs

@@ -766,12 +766,48 @@ fn auto_scrollbar_feedback_reveals_the_perpendicular_axis() {
     assert!(extent.vertical_scrollbar.is_some());
 }
 
+fn fixed_inline_font() -> (ResolvedLayoutStyle, DocumentLayoutServices) {
+    use style::values::computed::font::{
+        FamilyName, FontFamily, FontFamilyList, FontFamilyNameSyntax, SingleFontFamily,
+    };
+    let mut font = style::properties::style_structs::Font::initial_values();
+    font.set_font_family(FontFamily {
+        families: FontFamilyList {
+            list: style::ArcSlice::from_iter(std::iter::once(SingleFontFamily::FamilyName(
+                FamilyName {
+                    name: Atom::from("Moli Ahem"),
+                    syntax: FontFamilyNameSyntax::Quoted,
+                },
+            ))),
+        },
+        is_system_font: false,
+        is_initial: false,
+    });
+    let style = ResolvedLayoutStyle::from_stylo(
+        style::properties::ComputedValues::initial_values_with_font_override(font),
+    );
+    let mut services =
+        DocumentLayoutServices::with_system_font_policy(moli_layout::SystemFontPolicy::Disabled);
+    services
+        .register_web_font(moli_layout::WebFontRegistration::new(
+            "fixed",
+            moli_layout::WebFontFace::new("Moli Ahem"),
+            include_bytes!("fixtures/moli-ahem.ttf").to_vec(),
+        ))
+        .unwrap();
+    (style, services)
+}
+
 #[test]
 fn scrollbar_feedback_rebreaks_the_reused_inline_layout_at_its_final_width() {
-    const TEXT: &str = "alpha beta gamma delta epsilon zeta eta theta iota kappa";
+    const TEXT: &str = concat!(
+        "alpha beta gamma delta epsilon zeta eta theta iota kappa ",
+        "supercalifragilisticexpialidocious",
+    );
     let source = Source(vec![
         Node::element("root", vec![1]),
         Node::element("scroller", vec![2]),
+        Node::element("fixed-font", vec![3]),
         Node::text("text", TEXT),
     ]);
     let mut styles = Styles::default();
@@ -788,8 +824,9 @@ fn scrollbar_feedback_rebreaks_the_reused_inline_layout_at_its_final_width() {
                     height: length(40.0),
                 },
                 overflow: Point {
-                    // Only vertical feedback is needed to narrow the text.
-                    // Horizontal overflow depends on platform font advances.
+                    // Only the vertical scrollbar narrows the text. Keep the
+                    // long word's horizontal overflow without showing a
+                    // horizontal scrollbar.
                     x: Overflow::Hidden,
                     y: Overflow::Scroll,
                 },
@@ -798,13 +835,32 @@ fn scrollbar_feedback_rebreaks_the_reused_inline_layout_at_its_final_width() {
         ),
     );
 
-    let feedback = build(&source, &mut styles);
+    // The old system-font fixture happened to fit on macOS but exposed
+    // hanging-space overflow on Linux. Bind shaping to the same test face.
+    let (font_style, mut fixed_services) = fixed_inline_font();
+    styles.0.insert(2, font_style);
+    let feedback = build_layout_pass(
+        &source,
+        &mut styles,
+        &mut fixed_services,
+        LayoutPassRequest::new(LayoutViewport::new(320, 240, 1.0), LayoutFlushReason::Test),
+    )
+    .unwrap();
     assert_eq!(feedback.metrics.numeric_layout_pass_count, 2);
     assert_eq!(
         feedback.element_metrics_for_source(1).unwrap().client_size,
         moli_layout::LayoutSize::new(85.0, 40.0),
     );
-    let feedback_text = feedback.client_rects_for_source(2);
+    let scroller_box = feedback.source_output(1).unwrap().principal_box.unwrap();
+    let extent = feedback.scroll_extent(scroller_box).unwrap();
+    assert!(extent.vertical_scrollbar.is_some());
+    assert!(extent.horizontal_scrollbar.is_none());
+    assert!(!extent.allows_user_scroll_x);
+    let feedback_text = feedback.text_range_rects(3, 0..TEXT.encode_utf16().count());
+    assert!(
+        !feedback_text.is_empty(),
+        "compare real text fragments, not element-only client rects"
+    );
 
     // Lay out the same paragraph directly at the converged 85px content
     // width. Its line fragments must match the scrollbar-corrected result;
@@ -822,8 +878,23 @@ fn scrollbar_feedback_rebreaks_the_reused_inline_layout_at_its_final_width() {
             },
         ),
     );
-    let direct = build(&source, &mut styles);
-    assert_eq!(feedback_text, direct.client_rects_for_source(2));
+    let direct = build_layout_pass(
+        &source,
+        &mut styles,
+        &mut fixed_services,
+        LayoutPassRequest::new(LayoutViewport::new(320, 240, 1.0), LayoutFlushReason::Test),
+    )
+    .unwrap();
+    assert_eq!(
+        feedback_text,
+        direct.text_range_rects(3, 0..TEXT.encode_utf16().count())
+    );
+    assert!(
+        feedback_text
+            .iter()
+            .any(|quad| quad.points.iter().any(|point| point.x > 85.0)),
+        "the unbreakable final word should retain horizontal overflow"
+    );
 }
 
 #[test]

moli/src/telemetry.rs

@@ -19,6 +19,7 @@ pub fn init(log_filter: &str) {
         .with_env_filter(filter)
         .with_target(false)
         .with_writer(std::io::stderr)
+        .log_internal_errors(false)
         .try_init();
 }
 
@@ -107,4 +108,31 @@ mod tests {
         assert!(output.contains("fallback-info"));
         assert!(!output.contains("dependency-warning"));
     }
+
+    #[test]
+    fn broken_diagnostic_writer_does_not_panic() {
+        const CHILD: &str = "MOLI_TEST_BROKEN_DIAGNOSTIC_SINK";
+        if std::env::var_os(CHILD).is_some() {
+            super::init("error");
+            tracing::error!("the diagnostic receiver has already exited");
+            return;
+        }
+        let (reader, writer) = std::io::pipe().unwrap();
+        drop(reader);
+        let status = std::process::Command::new(std::env::current_exe().unwrap())
+            .args([
+                "--exact",
+                "telemetry::tests::broken_diagnostic_writer_does_not_panic",
+                "--nocapture",
+            ])
+            .env(CHILD, "1")
+            .stdout(std::process::Stdio::null())
+            .stderr(writer)
+            .status()
+            .unwrap();
+        assert!(
+            status.success(),
+            "logging to a closed diagnostic pipe terminated the process: {status}"
+        );
+    }
 }

moli/tests/fetch_cli.rs

@@ -1315,6 +1315,73 @@ fn cli_dump_html_keeps_uncaught_script_errors_out_of_stdout() -> Result<()> {
     Ok(())
 }
 
+#[test]
+fn cli_uncaught_script_error_is_logged_to_writable_stderr() -> Result<()> {
+    let output = Command::new(env!("CARGO_BIN_EXE_moli"))
+        .args([
+            "fetch",
+            "--log-level",
+            "error",
+            "--wait-until",
+            "load",
+            "--timeout",
+            "5000",
+            "--dump",
+            "html",
+            "data:text/html,<script>throw%20new%20Error(%22stderr-control%22)</script><main%20id=%22after%22>after</main>",
+        ])
+        .output()?;
+
+    let stdout = clean_output(&output.stdout);
+    let stderr = clean_output(&output.stderr);
+    assert!(output.status.success(), "stdout={stdout}\nstderr={stderr}");
+    assert!(stdout.contains("<main id=\"after\">after</main>"));
+    assert!(stderr.contains("Uncaught Error: stderr-control"));
+    Ok(())
+}
+
+#[cfg(unix)]
+#[test]
+fn cli_uncaught_script_error_survives_broken_stderr_pipe() -> Result<()> {
+    use std::{fs::File, os::fd::FromRawFd, process::Stdio};
+
+    let mut pipe_fds = [0; 2];
+    // SAFETY: `pipe_fds` points to two writable integers. Each returned file
+    // descriptor is transferred to exactly one owner or closed below.
+    if unsafe { libc::pipe(pipe_fds.as_mut_ptr()) } != 0 {
+        return Err(std::io::Error::last_os_error().into());
+    }
+    // Closing the read endpoint makes the child's first diagnostic write
+    // return BrokenPipe, matching a launcher whose stderr reader disappeared.
+    unsafe { libc::close(pipe_fds[0]) };
+    // SAFETY: the write descriptor is valid and ownership moves into `File`.
+    let broken_stderr = unsafe { File::from_raw_fd(pipe_fds[1]) };
+    let output = Command::new(env!("CARGO_BIN_EXE_moli"))
+        .args([
+            "fetch",
+            "--log-level",
+            "error",
+            "--wait-until",
+            "load",
+            "--timeout",
+            "5000",
+            "--dump",
+            "html",
+            "data:text/html,<script>throw%20new%20Error(%22broken-stderr%22)</script><main%20id=%22after%22>after</main>",
+        ])
+        .stderr(Stdio::from(broken_stderr))
+        .output()?;
+
+    let stdout = clean_output(&output.stdout);
+    assert_eq!(
+        output.status.code(),
+        Some(0),
+        "broken diagnostic pipe must not abort the browser: stdout={stdout}"
+    );
+    assert!(stdout.contains("<main id=\"after\">after</main>"));
+    Ok(())
+}
+
 #[test]
 fn cli_http_cache_dir_reuses_cached_response_across_processes() -> Result<()> {
     let runtime = tokio::runtime::Runtime::new()?;