test(cli): extract shared Fixture into tests/common

The single bare-metal integration test now reuses a `LazyLock<Fixture>`
that spawns picloud once on a private port and shares it across every
test in the binary. Sets the stage for per-surface journey modules
(auth, apps, scripts, invoke, logs, roles, output) without each one
paying for its own server spawn — same trick the dashboard Playwright
suite uses with global-setup.

Notes:
- `tests/cli.rs` becomes a tiny module list; the seed flow moved to
  `tests/integration.rs`. The seed slug now goes through
  `common::unique_slug` so parallel/serial reruns can't collide.
- `autotests = false` + an explicit `[[test]] name = "cli"` keeps Cargo
  from auto-promoting future `tests/*.rs` files into their own binaries
  (which would each respawn picloud).
- Subprocess cleanup uses `libc::atexit` to SIGTERM picloud when the
  test binary exits. PR_SET_PDEATHSIG was tried and rejected: it fires
  when the *thread* that forked dies, and cargo's per-test worker
  threads exit between tests, which killed the fixture mid-suite.
- New helpers: AppGuard/UserGuard (RAII teardown), member_user /
  grant_membership / update_membership (direct API for role tests),
  unique_slug / unique_username, pic_as / pic_no_env.
- Two `fixture_url_is_shared_*` tests prove the LazyLock is actually
  shared, not respawned per test.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

crates/picloud-cli/tests/common/cleanup.rs

@@ -0,0 +1,61 @@
+//! RAII guards that delete server-side resources on `Drop`.
+//!
+//! Each guard owns the minimum it needs to issue a single DELETE: the
+//! base URL, an admin bearer token, and the resource identifier.
+//! Failures are swallowed because Drop runs during teardown — a panic
+//! here would just mask the real failure that the test was reporting.
+
+pub struct AppGuard {
+    url: String,
+    token: String,
+    slug: String,
+}
+
+impl AppGuard {
+    pub fn new(url: &str, token: &str, slug: &str) -> Self {
+        Self {
+            url: url.to_string(),
+            token: token.to_string(),
+            slug: slug.to_string(),
+        }
+    }
+}
+
+impl Drop for AppGuard {
+    fn drop(&mut self) {
+        let client = reqwest::blocking::Client::new();
+        let _ = client
+            .delete(format!(
+                "{}/api/v1/admin/apps/{}?force=true",
+                self.url, self.slug
+            ))
+            .bearer_auth(&self.token)
+            .send();
+    }
+}
+
+pub struct UserGuard {
+    url: String,
+    token: String,
+    user_id: String,
+}
+
+impl UserGuard {
+    pub fn new(url: &str, token: &str, user_id: &str) -> Self {
+        Self {
+            url: url.to_string(),
+            token: token.to_string(),
+            user_id: user_id.to_string(),
+        }
+    }
+}
+
+impl Drop for UserGuard {
+    fn drop(&mut self) {
+        let client = reqwest::blocking::Client::new();
+        let _ = client
+            .delete(format!("{}/api/v1/admin/admins/{}", self.url, self.user_id))
+            .bearer_auth(&self.token)
+            .send();
+    }
+}

crates/picloud-cli/tests/common/member.rs

@@ -0,0 +1,97 @@
+//! Helpers for non-admin (`instance_role: Member`) user lifecycle plus
+//! direct API calls for granting / updating app memberships.
+//!
+//! These talk to the manager HTTP surface directly instead of going
+//! through the CLI, so role-gated tests can stage state without
+//! requiring `pic` to grow new commands.
+
+use serde_json::{json, Value};
+
+use super::cleanup::UserGuard;
+use super::Fixture;
+
+pub const MEMBER_PASSWORD: &str = "pic-cli-test-pw-12345678";
+
+pub struct MemberUser {
+    pub id: String,
+    pub username: String,
+    pub token: String,
+    pub _guard: UserGuard,
+}
+
+/// Mint a fresh `instance_role: Member` user, log them in for a bearer
+/// token, and register a `UserGuard` for teardown.
+pub fn member_user(fx: &Fixture, username: &str) -> MemberUser {
+    let client = reqwest::blocking::Client::new();
+
+    let create = client
+        .post(format!("{}/api/v1/admin/admins", fx.url))
+        .bearer_auth(&fx.admin_token)
+        .json(&json!({
+            "username": username,
+            "password": MEMBER_PASSWORD,
+            "instance_role": "Member",
+        }))
+        .send()
+        .expect("create member user");
+    assert!(
+        create.status().is_success(),
+        "create member user failed: {} {}",
+        create.status(),
+        create.text().unwrap_or_default(),
+    );
+    let body: Value = create.json().expect("admin create json");
+    let id = body["id"]
+        .as_str()
+        .expect("admin create returns id")
+        .to_string();
+
+    // Register cleanup before we attempt anything else that could fail.
+    let guard = UserGuard::new(&fx.url, &fx.admin_token, &id);
+
+    let token = super::server::login_for_bearer_token(&fx.url, username, MEMBER_PASSWORD);
+
+    MemberUser {
+        id,
+        username: username.to_string(),
+        token,
+        _guard: guard,
+    }
+}
+
+/// `POST /api/v1/admin/apps/{slug}/members` — grant `role` to `user_id`.
+pub fn grant_membership(fx: &Fixture, app_slug: &str, user_id: &str, role: &str) {
+    let client = reqwest::blocking::Client::new();
+    let resp = client
+        .post(format!("{}/api/v1/admin/apps/{}/members", fx.url, app_slug))
+        .bearer_auth(&fx.admin_token)
+        .json(&json!({ "user_id": user_id, "role": role }))
+        .send()
+        .expect("grant membership");
+    assert!(
+        resp.status().is_success(),
+        "grant membership failed: {} {}",
+        resp.status(),
+        resp.text().unwrap_or_default(),
+    );
+}
+
+/// `PATCH /api/v1/admin/apps/{slug}/members/{user_id}` — promote/demote.
+pub fn update_membership(fx: &Fixture, app_slug: &str, user_id: &str, role: &str) {
+    let client = reqwest::blocking::Client::new();
+    let resp = client
+        .patch(format!(
+            "{}/api/v1/admin/apps/{}/members/{}",
+            fx.url, app_slug, user_id
+        ))
+        .bearer_auth(&fx.admin_token)
+        .json(&json!({ "role": role }))
+        .send()
+        .expect("update membership");
+    assert!(
+        resp.status().is_success(),
+        "update membership failed: {} {}",
+        resp.status(),
+        resp.text().unwrap_or_default(),
+    );
+}

crates/picloud-cli/tests/common/mod.rs

@@ -0,0 +1,247 @@
+//! Shared fixture and helpers for the CLI integration test binary.
+//!
+//! All tests in `tests/cli.rs` route through `fixture()`, a `LazyLock`
+//! that spawns picloud on a private port the first time it's touched
+//! and reuses that subprocess for every subsequent test. The dashboard
+//! Playwright suite pays the same cost once for 63 tests; we do the
+//! same here.
+
+#![allow(dead_code)] // shared helpers — not every module uses every fn.
+
+pub mod cleanup;
+pub mod member;
+pub mod server;
+
+use std::path::PathBuf;
+use std::process::Child;
+use std::sync::atomic::{AtomicU64, Ordering};
+use std::sync::{LazyLock, Mutex, OnceLock};
+use std::time::{Duration, SystemTime, UNIX_EPOCH};
+
+use assert_cmd::Command as AssertCommand;
+use tempfile::TempDir;
+
+// --------------------------------------------------------------------
+// Fixture
+// --------------------------------------------------------------------
+
+pub struct Fixture {
+    pub url: String,
+    pub admin_token: String,
+    pub admin_username: String,
+    // Held in a Mutex so Drop can kill it without UB; we never re-enter.
+    child: Mutex<Option<Child>>,
+}
+
+impl Drop for Fixture {
+    fn drop(&mut self) {
+        if let Ok(mut guard) = self.child.lock() {
+            if let Some(mut child) = guard.take() {
+                server::kill_subprocess(&mut child);
+            }
+        }
+    }
+}
+
+static FIXTURE: LazyLock<Fixture> = LazyLock::new(init_fixture);
+
+fn init_fixture() -> Fixture {
+    let database_url =
+        std::env::var("DATABASE_URL").expect("DATABASE_URL is required to spawn picloud");
+    let username = admin_username();
+    let password = admin_password();
+    let port = server::pick_free_port();
+    let url = format!("http://127.0.0.1:{port}");
+    let mut child = server::spawn_picloud(&database_url, port, &username, &password);
+    if let Err(e) = server::wait_for_health(&url, Duration::from_secs(60)) {
+        server::kill_subprocess(&mut child);
+        panic!("picloud failed to become healthy: {e}");
+    }
+    let token = server::login_for_bearer_token(&url, &username, &password);
+    Fixture {
+        url,
+        admin_token: token,
+        admin_username: username,
+        child: Mutex::new(Some(child)),
+    }
+}
+
+/// Returns the shared fixture, spawning the picloud subprocess on first
+/// call. Returns `None` (and prints a skip message) when `DATABASE_URL`
+/// is absent — matching the existing convention so the suite is a
+/// no-op outside the integration environment.
+pub fn fixture_or_skip() -> Option<&'static Fixture> {
+    if std::env::var("DATABASE_URL").is_err() {
+        eprintln!("skipping: DATABASE_URL not set");
+        return None;
+    }
+    Some(&FIXTURE)
+}
+
+// --------------------------------------------------------------------
+// Per-test env
+// --------------------------------------------------------------------
+
+pub struct TestEnv {
+    pub url: String,
+    pub token: String,
+    pub config_dir: TempDir,
+    pub home: TempDir,
+}
+
+/// Per-test env pre-loaded with the admin token. Mirrors what the seed
+/// test built inline.
+pub fn admin_env(fx: &Fixture) -> TestEnv {
+    TestEnv {
+        url: fx.url.clone(),
+        token: fx.admin_token.clone(),
+        config_dir: TempDir::new().expect("config tempdir"),
+        home: TempDir::new().expect("home tempdir"),
+    }
+}
+
+/// Per-test env pre-loaded with a specific (URL, token) pair. Used by
+/// tests that want a non-admin token, a bogus token, or an unreachable
+/// URL.
+pub fn custom_env(url: &str, token: &str) -> TestEnv {
+    TestEnv {
+        url: url.to_string(),
+        token: token.to_string(),
+        config_dir: TempDir::new().expect("config tempdir"),
+        home: TempDir::new().expect("home tempdir"),
+    }
+}
+
+/// `pic` invocation with the env wired up — credentials dir, HOME, and
+/// the `PICLOUD_URL`/`PICLOUD_TOKEN` shortcut env vars.
+pub fn pic_as(env: &TestEnv) -> AssertCommand {
+    let mut cmd = AssertCommand::cargo_bin("pic").expect("pic binary");
+    cmd.env("PICLOUD_URL", &env.url)
+        .env("PICLOUD_TOKEN", &env.token)
+        .env("PICLOUD_CONFIG_DIR", env.config_dir.path())
+        .env("HOME", env.home.path());
+    cmd
+}
+
+/// `pic` invocation with `PICLOUD_URL`/`PICLOUD_TOKEN` *cleared*, so the
+/// command sees only the on-disk credentials file (or lack thereof).
+pub fn pic_no_env(env: &TestEnv) -> AssertCommand {
+    let mut cmd = AssertCommand::cargo_bin("pic").expect("pic binary");
+    cmd.env_remove("PICLOUD_URL")
+        .env_remove("PICLOUD_TOKEN")
+        .env("PICLOUD_CONFIG_DIR", env.config_dir.path())
+        .env("HOME", env.home.path());
+    cmd
+}
+
+// --------------------------------------------------------------------
+// Unique slugs / usernames
+// --------------------------------------------------------------------
+
+static UNIQUE_COUNTER: AtomicU64 = AtomicU64::new(0);
+
+pub fn unique_slug(prefix: &str) -> String {
+    let ms = SystemTime::now()
+        .duration_since(UNIX_EPOCH)
+        .unwrap()
+        .as_millis();
+    let n = UNIQUE_COUNTER.fetch_add(1, Ordering::Relaxed);
+    format!("pic-cli-{prefix}-{ms}-{n:x}")
+}
+
+pub fn unique_username(prefix: &str) -> String {
+    // Server regex: [a-z0-9._-]{2,32}. Build out of lowercase
+    // alphanumerics only; "piccli" prefix keeps collisions with other
+    // test suites obvious. Caller's `prefix` must be ≤8 chars and
+    // already match the regex.
+    let ms = SystemTime::now()
+        .duration_since(UNIX_EPOCH)
+        .unwrap()
+        .as_millis();
+    let n = UNIQUE_COUNTER.fetch_add(1, Ordering::Relaxed);
+    let name = format!("piccli{prefix}{ms:x}{n:x}");
+    assert!(name.len() <= 32, "username overflow: {name}");
+    name
+}
+
+// --------------------------------------------------------------------
+// Misc helpers
+// --------------------------------------------------------------------
+
+pub fn admin_username() -> String {
+    std::env::var("PICLOUD_CLI_E2E_USERNAME").unwrap_or_else(|_| "admin".to_string())
+}
+
+pub fn admin_password() -> String {
+    std::env::var("PICLOUD_CLI_E2E_PASSWORD").unwrap_or_else(|_| "admin".to_string())
+}
+
+pub fn fixture_path(name: &str) -> PathBuf {
+    PathBuf::from(env!("CARGO_MANIFEST_DIR"))
+        .join("tests")
+        .join("fixtures")
+        .join(name)
+}
+
+/// First data row's first tab-delimited cell, used to extract IDs from
+/// `pic scripts ls` output. The header is expected to start with "id".
+pub fn parse_first_id(table: &str) -> Option<String> {
+    let mut lines = table.lines().filter(|l| !l.trim().is_empty());
+    let header = lines.next()?;
+    if !header.starts_with("id") {
+        return None;
+    }
+    let row = lines.next()?;
+    let first = row.split('\t').next()?.trim();
+    if first.is_empty() {
+        None
+    } else {
+        Some(first.to_string())
+    }
+}
+
+// --------------------------------------------------------------------
+// Fixture-sharing sanity check
+// --------------------------------------------------------------------
+//
+// Two tests record `fixture().url` into the same `OnceLock` — if the
+// fixture isn't actually shared, the second test sees a different URL
+// and panics. Belt-and-suspenders: pointer identity on `&Fixture`.
+
+static OBSERVED_URL: OnceLock<String> = OnceLock::new();
+
+fn observe_fixture_url(label: &str) {
+    let Some(fx) = fixture_or_skip() else {
+        return;
+    };
+    let url = fx.url.clone();
+    match OBSERVED_URL.get() {
+        Some(prev) => assert_eq!(
+            prev, &url,
+            "{label} observed a different fixture URL: prior={prev} now={url}"
+        ),
+        None => {
+            let _ = OBSERVED_URL.set(url);
+        }
+    }
+    // Same `&'static Fixture` from every call — proves the LazyLock is
+    // sharing, not respawning.
+    let a = fixture_or_skip().unwrap();
+    let b = fixture_or_skip().unwrap();
+    assert!(
+        std::ptr::eq(a, b),
+        "fixture_or_skip should return the same &'static Fixture"
+    );
+}
+
+#[ignore = "needs DATABASE_URL pointing at a running Postgres"]
+#[test]
+fn fixture_url_is_shared_a() {
+    observe_fixture_url("fixture_url_is_shared_a");
+}
+
+#[ignore = "needs DATABASE_URL pointing at a running Postgres"]
+#[test]
+fn fixture_url_is_shared_b() {
+    observe_fixture_url("fixture_url_is_shared_b");
+}

crates/picloud-cli/tests/common/server.rs

@@ -0,0 +1,166 @@
+//! Picloud subprocess lifecycle for the CLI integration test binary.
+//!
+//! Mirrors what the original seed test did inline, lifted out so it can
+//! be shared across modules via `common::fixture()`. The Fixture lives
+//! in a `LazyLock` — and `LazyLock<T>` never drops, so we register an
+//! `atexit` handler that SIGTERMs the child when the test binary exits
+//! normally (which is the common case under `cargo test`).
+//!
+//! `PR_SET_PDEATHSIG` is intentionally *not* used: it fires when the
+//! creating thread dies, and cargo runs each `#[test]` on its own
+//! worker thread that exits as soon as the test returns — which would
+//! kill picloud after the first test that triggered the LazyLock,
+//! breaking every test after it.
+//!
+//! Abnormal exit (SIGKILL of the test binary) leaks the child; the
+//! dashboard Playwright suite accepts the same tradeoff.
+
+use std::io::{BufRead, BufReader};
+use std::path::PathBuf;
+use std::process::{Child, Command as StdCommand, Stdio};
+use std::sync::atomic::{AtomicI32, Ordering};
+use std::sync::mpsc;
+use std::thread;
+use std::time::{Duration, Instant};
+
+use serde_json::Value;
+
+pub fn pick_free_port() -> u16 {
+    let listener =
+        std::net::TcpListener::bind("127.0.0.1:0").expect("bind 127.0.0.1:0 to pick port");
+    listener.local_addr().expect("local addr").port()
+}
+
+pub fn picloud_binary_path() -> PathBuf {
+    // The integration test binary lives at
+    // `<target>/debug/deps/cli-<hash>`. Walk up two levels to reach
+    // `<target>/debug` and look for `picloud` next to ourselves.
+    let exe = std::env::current_exe().expect("current_exe");
+    let debug_dir = exe
+        .parent()
+        .and_then(|p| p.parent())
+        .expect("test binary should live under target/debug/deps");
+    debug_dir.join(if cfg!(windows) {
+        "picloud.exe"
+    } else {
+        "picloud"
+    })
+}
+
+pub fn spawn_picloud(database_url: &str, port: u16, admin_user: &str, admin_pass: &str) -> Child {
+    let binary = picloud_binary_path();
+    assert!(
+        binary.exists(),
+        "expected picloud binary at {}. Run `cargo build -p picloud` first \
+         (or use `cargo test --workspace -- --include-ignored` which builds it)",
+        binary.display()
+    );
+    let mut child = StdCommand::new(&binary)
+        .env("PICLOUD_BIND", format!("127.0.0.1:{port}"))
+        .env("DATABASE_URL", database_url)
+        .env("PICLOUD_ADMIN_USERNAME", admin_user)
+        .env("PICLOUD_ADMIN_PASSWORD", admin_pass)
+        .env("RUST_LOG", "warn")
+        .stdout(Stdio::null())
+        .stderr(Stdio::piped())
+        .spawn()
+        .expect("spawn picloud");
+
+    // Drain stderr in a side thread so the pipe buffer doesn't fill and
+    // block the server.
+    if let Some(err) = child.stderr.take().map(BufReader::new) {
+        let (tx, _rx) = mpsc::channel::<String>();
+        thread::spawn(move || {
+            for line in err.lines().map_while(Result::ok) {
+                let _ = tx.send(line);
+            }
+        });
+    }
+
+    register_atexit_killer(child.id());
+    child
+}
+
+// --------------------------------------------------------------------
+// atexit-based child cleanup
+// --------------------------------------------------------------------
+
+static PICLOUD_PID: AtomicI32 = AtomicI32::new(0);
+
+fn register_atexit_killer(pid: u32) {
+    // First spawn wins; subsequent spawns (none expected today) would
+    // overwrite, but the previous child would already be tracked via
+    // its Drop path on the Fixture.
+    PICLOUD_PID.store(pid as i32, Ordering::SeqCst);
+
+    #[cfg(unix)]
+    {
+        use std::sync::Once;
+        static REGISTERED: Once = Once::new();
+        REGISTERED.call_once(|| {
+            // SAFETY: atexit's contract is a `extern "C" fn()` callback;
+            // ours signals a child PID we own.
+            unsafe {
+                libc::atexit(kill_picloud_at_exit);
+            }
+        });
+    }
+}
+
+#[cfg(unix)]
+extern "C" fn kill_picloud_at_exit() {
+    let pid = PICLOUD_PID.swap(0, Ordering::SeqCst);
+    if pid > 0 {
+        // SAFETY: SIGTERM to a PID we recorded; if PID has been reused
+        // we're killing the wrong process — accepted risk for a test
+        // helper.
+        unsafe {
+            libc::kill(pid, libc::SIGTERM);
+        }
+    }
+}
+
+pub fn wait_for_health(url: &str, timeout: Duration) -> Result<(), String> {
+    let deadline = Instant::now() + timeout;
+    let client = reqwest::blocking::Client::builder()
+        .timeout(Duration::from_secs(2))
+        .build()
+        .map_err(|e| e.to_string())?;
+    while Instant::now() < deadline {
+        if let Ok(resp) = client.get(format!("{url}/healthz")).send() {
+            if resp.status().is_success() {
+                return Ok(());
+            }
+        }
+        thread::sleep(Duration::from_millis(250));
+    }
+    Err(format!("/healthz never returned 200 within {timeout:?}"))
+}
+
+pub fn login_for_bearer_token(url: &str, username: &str, password: &str) -> String {
+    let client = reqwest::blocking::Client::new();
+    let resp = client
+        .post(format!("{url}/api/v1/admin/auth/login"))
+        .json(&serde_json::json!({
+            "username": username,
+            "password": password,
+        }))
+        .send()
+        .expect("login request");
+    assert!(
+        resp.status().is_success(),
+        "login should succeed, got {}: {}",
+        resp.status(),
+        resp.text().unwrap_or_default()
+    );
+    let v: Value = resp.json().expect("login json");
+    v["token"]
+        .as_str()
+        .expect("login returns token")
+        .to_string()
+}
+
+pub fn kill_subprocess(child: &mut Child) {
+    let _ = child.kill();
+    let _ = child.wait();
+}