perf(ci): build one feature set, drop the OpenSSL C build

All three cargo steps now run at `serve,e2e-tests`. They previously used three different sets, so the lib and the ~144 crates `serve` adds compiled once per step. clippy gains `--all-targets` so `cargo test` reuses the test targets instead of rebuilding them. The test run is split by thread budget rather than feature set, so neither half recompiles: the lib suite spawns no tmux and gets 8 threads, the integration and e2e binaries keep the cap at 3. git2 drops `vendored-openssl`, its only non-default feature. git2's `default` is empty, so `https`/`ssh` were never on and openssl-sys was in the tree solely to be vendored — building OpenSSL from C source on every cold cache. Nothing reaches a remote through libgit2: src/git/remote.rs reads origin's URL, and `Repository::clone` appears only in tests cloning a tempdir over the built-in local transport. flake.nix loses `perl` with it. Linting `serve` for the first time surfaced 16 pre-existing violations, fixed here. The `result_large_err` family is allowed at the `server::api` module root: those handlers return `Response` in the `Err` arm, and boxing it would put an allocation on every error path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-08-22 04:44:20 +01:00
parent 016576666d
commit c93812b05b
11 changed files with 94 additions and 90 deletions
@@ -57,28 +57,39 @@ jobs:
      - uses: actions/setup-node@v4
        with:
          node-version: 22
-      - run: cargo clippy -- -D warnings
+      # Every cargo invocation below uses the same `serve,e2e-tests` feature set
+      # on purpose. `e2e-tests` is empty, but any difference in the enabled set
+      # changes the cargo fingerprint, so a step at a narrower set rebuilds the
+      # lib and the ~144 crates `serve` adds. `--all-targets` builds the test
+      # targets here so `cargo test` below reuses them rather than compiling
+      # them again.
+      - run: cargo clippy --features serve,e2e-tests --all-targets -- -D warnings
      # tests/integration/acp_smoke.rs spawns the Node ACP shim, whose deps
      # must be installed before the integration crate runs.
      - name: Install ACP shim deps
        working-directory: acp-worker/test-shim
        run: npm ci
-      # One invocation at one feature set so the crate compiles once:
-      # `e2e-tests` is an empty feature, but adding it changes the cargo
-      # fingerprint, so a separate `--features serve` run would recompile the
-      # lib. RUST_TEST_THREADS=3 reaches every test binary; the e2e suite
-      # trades wall for flake risk above 3 (see tests/e2e/harness.rs).
-      - name: Cargo test (serve + e2e)
+      # Split by thread budget, not by feature set, so neither step recompiles.
+      # Only the tmux-backed binaries need the low cap: the e2e suite trades
+      # wall for flake risk above 3 (see tests/e2e/harness.rs). The lib suite
+      # spawns no tmux and is the bulk of the tests, so it gets the full runner.
+      - name: Cargo test (lib)
+        env:
+          RUST_TEST_THREADS: "8"
+        run: cargo test --features serve,e2e-tests --lib --bins
+      # `--test '*'` selects every integration target and nothing else, so the
+      # lib suite is not re-run here.
+      - name: Cargo test (integration + e2e)
        env:
          RUST_TEST_THREADS: "3"
-        run: cargo test --features serve,e2e-tests
+        run: cargo test --features serve,e2e-tests --test '*'
      - name: Check rustdoc is warning-clean
        env:
          RUSTDOCFLAGS: "-D warnings"
        run: |
          mkdir -p target/ci-web-dist
          printf '<!doctype html><title>aoe docs placeholder</title>\n' > target/ci-web-dist/index.html
-          AOE_WEB_DIST="$PWD/target/ci-web-dist" cargo doc --no-deps --features serve
+          AOE_WEB_DIST="$PWD/target/ci-web-dist" cargo doc --no-deps --features serve,e2e-tests

  web:
    name: Web
@@ -1661,7 +1661,6 @@ dependencies = [
 "libc",
 "libgit2-sys",
 "log",
- "openssl-sys",
 ]

 [[package]]
@@ -2306,7 +2305,6 @@ dependencies = [
 "cc",
 "libc",
 "libz-sys",
- "openssl-sys",
 "pkg-config",
 ]

@@ -2754,28 +2752,6 @@ version = "0.2.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "7c87def4c32ab89d880effc9e097653c8da5d6ef28e6b539d313baaacfbafcbe"

-[[package]]
-name = "openssl-src"
-version = "300.6.0+3.6.2"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "a8e8cbfd3a4a8c8f089147fd7aaa33cf8c7450c4d09f8f80698a0cf093abeff4"
-dependencies = [
- "cc",
-]
-
-[[package]]
-name = "openssl-sys"
-version = "0.9.114"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "13ce1245cd07fcc4cfdb438f7507b0c7e4f3849a69fd84d52374c66d83741bb6"
-dependencies = [
- "cc",
- "libc",
- "openssl-src",
- "pkg-config",
- "vcpkg",
-]
-
 [[package]]
 name = "option-ext"
 version = "0.2.0"
@@ -112,8 +112,12 @@ rattles = "0.3"
 # Regex
 regex = "1.10"

-# Git operations
-git2 = { version = "0.21", features = ["vendored-openssl"] }
+# Git operations. git2's `default` is empty, so this drops `https`/`ssh` and
+# with them openssl-sys, which built OpenSSL from C source on every cold cache.
+# Safe because nothing reaches a remote through libgit2: src/git/remote.rs only
+# reads origin's configured URL, and `Repository::clone` appears only in tests
+# cloning a tempdir over libgit2's built-in local transport.
+git2 = "0.21"

 # Diff computation
 similar = "3.1"
@@ -15,11 +15,11 @@
        let
          craneLib = inputs.crane.mkLib pkgs;

-          # git2 uses vendored-openssl (needs perl to build OpenSSL)
-          # and libgit2-sys vendors libgit2 (needs cmake to build it)
+          # libgit2-sys falls back to vendoring libgit2 (needs cmake to build
+          # it) when pkg-config finds no system copy. git2 is built without
+          # `https`/`ssh`, so there is no OpenSSL to compile and no perl.
          nativeBuildInputs = with pkgs; [
            pkg-config
-            perl
            cmake
            installShellFiles
          ];
@@ -14240,7 +14240,6 @@ done
                tool_call_id,
                is_error,
                content,
-                completed_at: _,
                ..
            } => {
                assert_eq!(tool_call_id, "tc-1");
@@ -624,7 +624,7 @@ mod tests {
        let line =
            r#"{"type":"assistant","message":{"content":[{"type":"tool_use","name":"Bash"}]}}"#;
        // First write ends mid-line (no trailing newline): nothing folds yet.
-        tokio::fs::write(&path, format!("{line}")).await.unwrap();
+        tokio::fs::write(&path, line).await.unwrap();
        let mut offset = 0u64;
        let mut buf = String::new();
        let mut snap = Snapshot::default();
@@ -735,14 +735,22 @@ mod tests {
        assert!(p.chars().count() <= TEXT_PREVIEW_CHARS + 1);
    }

+    /// (lines, expected status, expected result, warning substring, case)
+    type IdleOutcomeCase<'a> = (
+        Vec<&'a str>,
+        BackgroundAgentStatus,
+        Option<&'a str>,
+        &'a str,
+        &'a str,
+    );
+
    /// #3232: Claude Code's async-Task transcripts don't always tag the
    /// final assistant record `stop_reason: "end_turn"`. `infer_idle_outcome`
    /// is what an `ABORT_AFTER` idle-timeout falls back on to tell a
    /// sub-agent that actually finished from one genuinely hung.
    #[test]
    fn infer_idle_outcome_distinguishes_finished_from_hung() {
-        // (lines, expected status, expected result, warning substring, case)
-        let cases: Vec<(Vec<&str>, BackgroundAgentStatus, Option<&str>, &str, &str)> = vec![
+        let cases: Vec<IdleOutcomeCase> = vec![
            (
                vec![
                    r#"{"type":"assistant","message":{"content":[{"type":"tool_use","id":"t1","name":"Bash"}]}}"#,
@@ -8,6 +8,12 @@
 //!   - this file: shared validation helpers + module declarations and
 //!     re-exports so external callers keep `api::*` paths.

+// Handlers here return `axum::response::Response` in the `Err` arm so a failure
+// carries its own status and body. That type is 128 bytes, over
+// `result_large_err`'s threshold; boxing it would put an allocation on every
+// handler's error path.
+#![allow(clippy::result_large_err)]
+
 pub(super) use super::AppState;

 #[cfg(feature = "serve")]
@@ -2350,7 +2350,7 @@ mod tests {
        let mut new_plain = Instance::new("new_plain", "/tmp/np");
        new_plain.created_at = chrono::Utc::now();

-        let instances = vec![old_fav, new_plain];
+        let instances = [old_fav, new_plain];

        // Feature off: plain newest-first, unchanged behavior.
        let mut refs: Vec<&Instance> = instances.iter().collect();
@@ -2375,7 +2375,7 @@ mod tests {
        fav_new.favorite();
        let plain = Instance::new("plain", "/tmp/p");

-        let instances = vec![fav_old, fav_new, plain];
+        let instances = [fav_old, fav_new, plain];
        let mut refs: Vec<&Instance> = instances.iter().collect();
        sort_sessions_inner(&mut refs, SortOrder::Newest, true);

@@ -2391,7 +2391,7 @@ mod tests {
        z_fav.favorite();
        let a_plain = Instance::new("apple", "/tmp/a");

-        let instances = vec![z_fav, a_plain];
+        let instances = [z_fav, a_plain];

        let mut refs: Vec<&Instance> = instances.iter().collect();
        sort_sessions_inner(&mut refs, SortOrder::AZ, true);
@@ -2412,7 +2412,7 @@ mod tests {
        let mut new_plain = Instance::new("new_plain", "/tmp/np");
        new_plain.created_at = chrono::Utc::now();

-        let instances = vec![snoozed_fav, new_plain];
+        let instances = [snoozed_fav, new_plain];
        let mut refs: Vec<&Instance> = instances.iter().collect();
        sort_sessions_inner(&mut refs, SortOrder::Newest, true);

@@ -2566,7 +2566,7 @@ mod tests {
        let mut plain_waiting = Instance::new("plain_waiting", "/tmp/pw");
        plain_waiting.status = crate::session::Status::Waiting;

-        let instances = vec![fav_idle, plain_waiting];
+        let instances = [fav_idle, plain_waiting];

        let mut on: Vec<&Instance> = instances.iter().collect();
        sort_sessions_inner(&mut on, SortOrder::Attention, true);
@@ -1558,7 +1558,7 @@ mod serve {
            // differs only by a trailing slash.
            let mut existing = crate::session::Instance::new("Already owned", "/tmp/shared");
            existing.source_profile = "default".to_string();
-            let instances = vec![existing];
+            let instances = [existing];
            let cases = [
                ("Already owned", "/tmp/shared", true),
                // "/tmp/shared/" and "/tmp/shared" are equal after trim_end_matches('/').
@@ -653,7 +653,7 @@ mod tests {
            if let Some(home_str) = home.to_str() {
                let path = format!("{}extra/not/home", home_str);
                let shortened = shorten_path(&path);
-                assert_eq!(shortened, format!("~extra/not/home"));
+                assert_eq!(shortened, "~extra/not/home");
            }
        }
    }
@@ -5882,47 +5882,6 @@ fn permission_response_tokens(
    }
 }

-#[cfg(test)]
-mod permission_response_tokens_tests {
-    use super::*;
-    use crate::agents::{KeyToken, PermissionResponse};
-    use crate::tui::dialogs::PermissionResponseChoice;
-
-    #[test]
-    fn maps_each_choice_to_its_own_field() {
-        let response = PermissionResponse {
-            allow: &[KeyToken::Literal("1")],
-            allow_always: Some(&[KeyToken::Literal("2")]),
-            deny: &[KeyToken::Literal("3")],
-        };
-        assert_eq!(
-            permission_response_tokens(&response, PermissionResponseChoice::Allow),
-            Some(response.allow)
-        );
-        assert_eq!(
-            permission_response_tokens(&response, PermissionResponseChoice::AllowAlways),
-            response.allow_always
-        );
-        assert_eq!(
-            permission_response_tokens(&response, PermissionResponseChoice::Deny),
-            Some(response.deny)
-        );
-    }
-
-    #[test]
-    fn allow_always_none_maps_to_none() {
-        let response = PermissionResponse {
-            allow: &[KeyToken::Named("Enter")],
-            allow_always: None,
-            deny: &[KeyToken::Named("Down"), KeyToken::Named("Enter")],
-        };
-        assert_eq!(
-            permission_response_tokens(&response, PermissionResponseChoice::AllowAlways),
-            None
-        );
-    }
-}
-
 impl HomeView {
    /// Whether the agent row is in a live status with its tmux pane up, so a
    /// revive cascade (`ensure_pane_ready` / `prepare_live_send`) is expected
@@ -7581,3 +7540,44 @@ impl HomeView {
            .map(|_| ())
    }
 }
+
+#[cfg(test)]
+mod permission_response_tokens_tests {
+    use super::*;
+    use crate::agents::{KeyToken, PermissionResponse};
+    use crate::tui::dialogs::PermissionResponseChoice;
+
+    #[test]
+    fn maps_each_choice_to_its_own_field() {
+        let response = PermissionResponse {
+            allow: &[KeyToken::Literal("1")],
+            allow_always: Some(&[KeyToken::Literal("2")]),
+            deny: &[KeyToken::Literal("3")],
+        };
+        assert_eq!(
+            permission_response_tokens(&response, PermissionResponseChoice::Allow),
+            Some(response.allow)
+        );
+        assert_eq!(
+            permission_response_tokens(&response, PermissionResponseChoice::AllowAlways),
+            response.allow_always
+        );
+        assert_eq!(
+            permission_response_tokens(&response, PermissionResponseChoice::Deny),
+            Some(response.deny)
+        );
+    }
+
+    #[test]
+    fn allow_always_none_maps_to_none() {
+        let response = PermissionResponse {
+            allow: &[KeyToken::Named("Enter")],
+            allow_always: None,
+            deny: &[KeyToken::Named("Down"), KeyToken::Named("Enter")],
+        };
+        assert_eq!(
+            permission_response_tokens(&response, PermissionResponseChoice::AllowAlways),
+            None
+        );
+    }
+}