fix(cli): interrupt sync before draining the save task on quit

zingo-cli/src/commands.rs

@@ -30,7 +30,7 @@ use zingolib::lightclient::migrate::{
     ImmediateMigrationPhase, ImmediateMigrationStatus, PartSendResult, SplitOutcome, SplitPhase,
     SplitStatus, SplitStep,
 };
-use zingolib::lightclient::{LightClient, SaveShutdown, TransmitProgressHandle};
+use zingolib::lightclient::{LightClient, SaveShutdown, SyncShutdown, TransmitProgressHandle};
 use zingolib::utils::conversion::txid_from_hex_encoded_str;
 use zingolib::wallet::keys::WalletAddressRef;
 use zingolib::wallet::keys::unified::{ReceiverSelection, UnifiedKeyStore};
@@ -545,7 +545,25 @@ async fn quickshield(lightclient: &mut LightClient) -> Result<String, CommandErr
     transmit_txids(lightclient.quick_shield(zip32::AccountId::ZERO)).await
 }
 
+/// The ceiling on quit's graceful sync stop, generous beside the engine's one remaining batch yet far below the scan a wedged engine would never finish.
+const QUIT_SYNC_STOP_BOUND: std::time::Duration = std::time::Duration::from_secs(30);
+
 async fn quit(lightclient: &mut LightClient) -> Result<String, CommandError> {
+    if lightclient.sync_mode() != SyncMode::NotRunning {
+        eprintln!("Stopping sync task...");
+    }
+    match lightclient.shutdown_sync(QUIT_SYNC_STOP_BOUND).await {
+        SyncShutdown::NotRunning => {}
+        SyncShutdown::Stopped(sync_result) => eprintln!("Sync task stopped. {sync_result}"),
+        SyncShutdown::Failed(e) => eprintln!(
+            "Error: the sync task ended with an error. {}",
+            render_error_chain(&e)
+        ),
+        SyncShutdown::Aborted => eprintln!(
+            "Sync task did not stop within {}s; aborted.",
+            QUIT_SYNC_STOP_BOUND.as_secs()
+        ),
+    }
     match lightclient.shutdown_save_task().await {
         Ok(SaveShutdown::ShutDown) => eprintln!("Save task shutdown successfully."),
         Ok(SaveShutdown::NotRunning) => eprintln!("No save task was running."),

zingo-cli/src/lib.rs

@@ -522,6 +522,14 @@ fn start_noninteractive(command: &commands::CliCommand, ch: CommandChannel) -> E
         ExitCode::FAILURE
     }
 }
+/// Dispatches the quit teardown to the command loop, narrating its trailer the way the one-shot path does.
+fn close_session(send_request: &impl Fn(Request) -> Result<String, String>) {
+    match send_request(Request::Command(commands::CliCommand::Quit)) {
+        Ok(trailer) => eprintln!("{trailer}"),
+        Err(rendered) => eprintln!("{rendered}"),
+    }
+}
+
 /// Runs the interactive prompt until it closes, returning the exit code the
 /// session earned: success when the user ended it, failure when the terminal
 /// did (ADR 0031).
@@ -620,18 +628,21 @@ fn start_interactive(cli_config: &CliConfigTemplate, ch: CommandChannel) -> Exit
             Err(rustyline::error::ReadlineError::Interrupted) => {
                 println!("CTRL-C");
                 info!("CTRL-C");
+                close_session(&send_request);
                 break ExitCode::SUCCESS;
             }
             Err(rustyline::error::ReadlineError::Eof) => {
                 println!("CTRL-D");
                 info!("CTRL-D");
+                close_session(&send_request);
                 break ExitCode::SUCCESS;
             }
             Err(err) => {
                 // The terminal ended the session, not the user, so the shell
                 // hears failure rather than a clean close.
                 eprintln!("Error: the interactive prompt failed: {err}");
                 error!("the interactive prompt failed: {err}");
+                close_session(&send_request);
                 break ExitCode::FAILURE;
             }
         }

zingo-cli/tests/interactive_exit.rs

@@ -0,0 +1,79 @@
+//! An offline acceptance test for the interactive session's exit arms.
+//!
+//! ZIN-70's fix taught the typed `quit` command to stop sync and drain the
+//! save task, but a session also ends through rustyline's Ctrl-C, Ctrl-D,
+//! and terminal-error arms. This test ends an offline session by closing
+//! stdin — the Ctrl-D arm — and asserts the same teardown ran, using the
+//! quit command's trailer as the evidence. All three arms share the one
+//! dispatch, so the reachable arm stands in for the pair a piped harness
+//! cannot trigger.
+
+#![forbid(unsafe_code)]
+
+use std::process::{Command, Stdio};
+use std::time::{Duration, Instant};
+
+/// The ceiling on the whole offline session, from spawn to exit.
+const SESSION_BOUND: Duration = Duration::from_secs(60);
+
+/// The cadence at which the harness polls the child for exit.
+const CHILD_POLL: Duration = Duration::from_millis(100);
+
+/// The trailer the quit command prints, proving the teardown ran.
+const QUIT_TRAILER: &str = "Zingo CLI quit successfully.";
+
+/// The acceptance test for an EOF-ended session: the exit arm must dispatch the quit teardown before the process exits.
+#[test]
+fn an_eof_ended_session_still_runs_the_quit_teardown() {
+    let cli = env!("CARGO_BIN_EXE_zingo-cli");
+    let data_dir = tempfile::tempdir().expect("a wallet tempdir opens");
+
+    // A fresh wallet under --offline is the deliberate no-network launch,
+    // so the session boots without any indexer or seed restore.
+    let mut child = Command::new(cli)
+        .arg("--data-dir")
+        .arg(data_dir.path())
+        .arg("--offline")
+        .stdin(Stdio::piped())
+        .stdout(Stdio::piped())
+        .stderr(Stdio::piped())
+        .spawn()
+        .expect("zingo-cli spawns");
+
+    // Closing stdin before any command is the piped spelling of Ctrl-D:
+    // rustyline reports Eof on the first read.
+    drop(child.stdin.take());
+
+    let deadline = Instant::now() + SESSION_BOUND;
+    loop {
+        if child.try_wait().expect("the child polls").is_some() {
+            break;
+        }
+        if Instant::now() >= deadline {
+            child.kill().expect("the child dies");
+            child.wait().expect("the killed child reaps");
+            panic!("the session did not exit within {SESSION_BOUND:?} after EOF");
+        }
+        std::thread::sleep(CHILD_POLL);
+    }
+
+    let output = child.wait_with_output().expect("the exited child reaps");
+    let transcript = format!(
+        "{}{}",
+        String::from_utf8_lossy(&output.stdout),
+        String::from_utf8_lossy(&output.stderr),
+    );
+    assert!(
+        output.status.success(),
+        "the EOF-ended session exited {}: {transcript}",
+        output.status
+    );
+    assert!(
+        transcript.contains("CTRL-D"),
+        "the session did not end through the Eof arm: {transcript}"
+    );
+    assert!(
+        transcript.contains(QUIT_TRAILER),
+        "the Eof arm exited without running the quit teardown: {transcript}"
+    );
+}

zingo-cli/tests/quit_mid_sync.rs

@@ -0,0 +1,120 @@
+//! The acceptance test for ZIN-70: a `quit` typed while the scan is running
+//! must end the session within a bound and leave a loadable wallet, instead
+//! of waiting out the sync behind the wallet lock.
+
+#![forbid(unsafe_code)]
+
+#[path = "support/cli_session.rs"]
+mod cli_session;
+
+use std::io::Write as _;
+use std::process::{Command, Stdio};
+use std::time::{Duration, Instant};
+
+/// The log filter the session runs under, opening pepper-sync's debug stream so the scan-progress marker reaches the log.
+const LOG_FILTER: &str = "info,pepper_sync=debug";
+
+/// The pepper-sync debug line following a scanned batch's processing under the wallet lock, which proves the scan holds the lock quit must contend with.
+const SCAN_PROGRESS_MARKER: &str = "Scan results processed.";
+
+/// The ceiling on session startup plus the first scanned batch reaching the log.
+const SCAN_EVIDENCE_DEADLINE: Duration = Duration::from_secs(300);
+
+/// The ceiling between typing `quit` and process exit, far below the minutes the remaining sync would take, so a shutdown that waits out the scan fails the test.
+const QUIT_EXIT_BOUND: Duration = Duration::from_secs(120);
+
+/// The acceptance test for a quit issued mid-sync: the session must exit within `QUIT_EXIT_BOUND` and leave a wallet file a fresh session loads.
+#[test]
+#[ignore = "network-bound acceptance test; run explicitly"]
+fn quit_mid_sync_exits_within_bound_and_leaves_a_loadable_wallet() {
+    let indexer = std::env::var("QUIT_MID_SYNC_INDEXER")
+        .unwrap_or_else(|_| cli_session::DEFAULT_INDEXER.to_string());
+    let cli = env!("CARGO_BIN_EXE_zingo-cli");
+    let proxy = cli_session::nym_proxy_beside(cli);
+    let data_dir = tempfile::tempdir().expect("a wallet tempdir opens");
+    let log_path = data_dir.path().join("cli.log");
+
+    let mut child = Command::new(cli)
+        .env("RUST_LOG", LOG_FILTER)
+        .env("ZINGO_NYM_PROXY", &proxy)
+        .arg("--data-dir")
+        .arg(data_dir.path())
+        .arg("--log-file")
+        .arg(&log_path)
+        .arg("--server")
+        .arg(&indexer)
+        .arg("--seed")
+        .arg(cli_session::MNEMONIC)
+        .arg("--birthday")
+        .arg(cli_session::BIRTHDAY.to_string())
+        .stdin(Stdio::piped())
+        .stdout(Stdio::inherit())
+        .stderr(Stdio::inherit())
+        .spawn()
+        .expect("zingo-cli spawns");
+
+    // Hold quit until a scanned batch's results have been processed: the
+    // span-open marker precedes any scan work, so a quit gated on it can
+    // land while the wallet lock is still free and prove nothing.
+    let scan_evidence_deadline = Instant::now() + SCAN_EVIDENCE_DEADLINE;
+    loop {
+        if std::fs::read_to_string(&log_path)
+            .unwrap_or_default()
+            .contains(SCAN_PROGRESS_MARKER)
+        {
+            break;
+        }
+        if let Some(status) = child.try_wait().expect("the child polls") {
+            panic!("zingo-cli exited {status} before the scan processed a batch");
+        }
+        assert!(
+            Instant::now() < scan_evidence_deadline,
+            "no {SCAN_PROGRESS_MARKER:?} line reached the log file within \
+             {SCAN_EVIDENCE_DEADLINE:?}"
+        );
+        std::thread::sleep(cli_session::CHILD_POLL);
+    }
+
+    child
+        .stdin
+        .as_mut()
+        .expect("stdin was piped")
+        .write_all(b"quit\n")
+        .expect("quit reaches the session");
+    let quit_sent = Instant::now();
+
+    loop {
+        if let Some(status) = child.try_wait().expect("the child polls") {
+            assert!(status.success(), "zingo-cli exited {status} after quit");
+            break;
+        }
+        if quit_sent.elapsed() > QUIT_EXIT_BOUND {
+            child.kill().expect("the child dies");
+            child.wait().expect("the killed child reaps");
+            panic!(
+                "quit did not end the session within {QUIT_EXIT_BOUND:?}; \
+                 the save-task shutdown is again waiting behind the scan"
+            );
+        }
+        std::thread::sleep(cli_session::CHILD_POLL);
+    }
+
+    // A second session over the same data dir proves the interrupted
+    // shutdown persisted a loadable wallet: with no consent act the
+    // launch is offline, and the one-shot command must answer from the
+    // wallet file alone.
+    let reload = Command::new(cli)
+        .arg("--data-dir")
+        .arg(data_dir.path())
+        .arg("--nosync")
+        .arg("addresses")
+        .output()
+        .expect("the reload session runs");
+    assert!(
+        reload.status.success(),
+        "the wallet saved by the interrupted quit did not load: {}\nstdout:\n{}\nstderr:\n{}",
+        reload.status,
+        String::from_utf8_lossy(&reload.stdout),
+        String::from_utf8_lossy(&reload.stderr),
+    );
+}

zingo-cli/tests/support/cli_session.rs

@@ -0,0 +1,31 @@
+//! Constants and helpers shared by the acceptance tests that drive a real
+//! `zingo-cli` session over the fixed mainnet window, so the window, the
+//! wallet, and the proxy resolution cannot drift apart between tests.
+
+use std::path::{Path, PathBuf};
+use std::time::Duration;
+
+/// The number of mainnet blocks below the authoring-day tip the fixed birthday sits.
+pub const SYNC_WINDOW: u32 = 20_000;
+
+/// The mainnet chain height on the day the fixed window was authored.
+pub const TIP_AT_AUTHORING: u32 = 3_445_000;
+
+/// The fixed wallet birthday, one sync window below the authoring-day tip.
+pub const BIRTHDAY: u32 = TIP_AT_AUTHORING - SYNC_WINDOW;
+
+/// The default indexer URI the sessions sync against.
+pub const DEFAULT_INDEXER: &str = "https://zec.rocks:443";
+
+/// The cadence at which a harness polls the child session for exit.
+pub const CHILD_POLL: Duration = Duration::from_millis(500);
+
+/// A fundless BIP-39 mnemonic, so an interrupted or measured sync scans pure chain data.
+pub const MNEMONIC: &str = "abandon abandon abandon abandon abandon abandon abandon abandon \
+     abandon abandon abandon abandon abandon abandon abandon abandon \
+     abandon abandon abandon abandon abandon abandon abandon art";
+
+/// Returns the nym-proxy path beside the CLI binary, where a mixnet-provisioning startup expects a protocol-matched proxy.
+pub fn nym_proxy_beside(cli: &str) -> PathBuf {
+    Path::new(cli).with_file_name("nym-proxy")
+}