Builder interface for History objects (#933)

* [feature] store env variables in History records

WIP: remove `HistoryWithoutDelete`, add some docstrings, tests

* Create History objects through builders.

Assure in compile-time that all required fields
are set for the given construction scenario

* (from #882) split Cmd::run into subfns

* Update `History` doc

* remove rmp-serde from history

* update warning

---------

Co-authored-by: Conrad Ludgate <conrad.ludgate@truelayer.com>

Cargo.lock

@@ -154,6 +154,7 @@ dependencies = [
  "rand",
  "regex",
  "reqwest",
+ "rmp",
  "rmp-serde",
  "semver",
  "serde",
@@ -164,6 +165,7 @@ dependencies = [
  "sql-builder",
  "sqlx",
  "tokio",
+ "typed-builder",
  "urlencoding",
  "uuid",
  "whoami",

atuin-client/Cargo.toml

@@ -50,7 +50,9 @@ fs-err = { workspace = true }
 sql-builder = "3"
 lazy_static = "1"
 memchr = "2.5"
+rmp = { version = "0.8.11" }
 rmp-serde = { version = "1.1.1" }
+typed-builder = "0.14.0"
 
 # sync
 urlencoding = { version = "2.1.0", optional = true }

atuin-client/src/database.rs

@@ -168,17 +168,18 @@ impl Sqlite {
     fn query_history(row: SqliteRow) -> History {
         let deleted_at: Option<i64> = row.get("deleted_at");
 
-        History {
+        History::from_db()
-            id: row.get("id"),
+            .id(row.get("id"))
-            timestamp: Utc.timestamp_nanos(row.get("timestamp")),
+            .timestamp(Utc.timestamp_nanos(row.get("timestamp")))
-            duration: row.get("duration"),
+            .duration(row.get("duration"))
-            exit: row.get("exit"),
+            .exit(row.get("exit"))
-            command: row.get("command"),
+            .command(row.get("command"))
-            cwd: row.get("cwd"),
+            .cwd(row.get("cwd"))
-            session: row.get("session"),
+            .session(row.get("session"))
-            hostname: row.get("hostname"),
+            .hostname(row.get("hostname"))
-            deleted_at: deleted_at.map(|t| Utc.timestamp_nanos(t)),
+            .deleted_at(deleted_at.map(|t| Utc.timestamp_nanos(t)))
-        }
+            .build()
+            .into()
     }
 }
 
@@ -594,17 +595,19 @@ mod test {
     }
 
     async fn new_history_item(db: &mut impl Database, cmd: &str) -> Result<()> {
-        let history = History::new(
+        let mut captured: History = History::capture()
-            chrono::Utc::now(),
+            .timestamp(chrono::Utc::now())
-            cmd.to_string(),
+            .command(cmd)
-            "/home/ellie".to_string(),
+            .cwd("/home/ellie")
-            0,
+            .build()
-            1,
+            .into();
-            Some("beep boop".to_string()),
+
-            Some("booop".to_string()),
+        captured.exit = 0;
-            None,
+        captured.duration = 1;
-        );
+        captured.session = "beep boop".to_string();
-        db.save(&history).await
+        captured.hostname = "booop".to_string();
+
+        db.save(&captured).await
     }
 
     #[tokio::test(flavor = "multi_thread")]

atuin-client/src/encryption.rs

@@ -11,8 +11,10 @@
 use std::{io::prelude::*, path::PathBuf};
 
 use base64::prelude::{Engine, BASE64_STANDARD};
-use eyre::{eyre, Context, Result};
+use chrono::{DateTime, Utc};
+use eyre::{bail, eyre, Context, Result};
 use fs_err as fs;
+use rmp::{decode::Bytes, Marker};
 use serde::{Deserialize, Serialize};
 pub use xsalsa20poly1305::Key;
 use xsalsa20poly1305::{
@@ -20,10 +22,7 @@ use xsalsa20poly1305::{
     AeadInPlace, KeyInit, XSalsa20Poly1305,
 };
 
-use crate::{
+use crate::{history::History, settings::Settings};
-    history::{History, HistoryWithoutDelete},
-    settings::Settings,
-};
 
 #[derive(Debug, Serialize, Deserialize)]
 pub struct EncryptedHistory {
@@ -75,7 +74,9 @@ pub fn load_encoded_key(settings: &Settings) -> Result<String> {
 }
 
 pub fn encode_key(key: &Key) -> Result<String> {
-    let buf = rmp_serde::to_vec(key.as_slice()).wrap_err("could not encode key to message pack")?;
+    let mut buf = vec![];
+    rmp::encode::write_bin(&mut buf, key.as_slice())
+        .wrap_err("could not encode key to message pack")?;
     let buf = BASE64_STANDARD.encode(buf);
 
     Ok(buf)
@@ -86,23 +87,23 @@ pub fn decode_key(key: String) -> Result<Key> {
         .decode(key.trim_end())
         .wrap_err("encryption key is not a valid base64 encoding")?;
 
-    let mbuf: Result<[u8; 32]> =
+    // old code wrote the key as a fixed length array of 32 bytes
-        rmp_serde::from_slice(&buf).wrap_err("encryption key is not a valid message pack encoding");
+    // new code writes the key with a length prefix
-
+    if buf.len() == 32 {
-    match mbuf {
+        Ok(*Key::from_slice(&buf))
-        Ok(b) => Ok(*Key::from_slice(&b)),
+    } else {
-        Err(_) => {
+        let mut bytes = Bytes::new(&buf);
-            let buf: &[u8] = rmp_serde::from_slice(&buf)
+        let key_len = rmp::decode::read_bin_len(&mut bytes).map_err(error_report)?;
-                .wrap_err("encryption key is not a valid message pack encoding")?;
+        if key_len != 32 || bytes.remaining_slice().len() != key_len as usize {
-
+            bail!("encryption key is not the correct size")
-            Ok(*Key::from_slice(buf))
         }
+        Ok(*Key::from_slice(bytes.remaining_slice()))
     }
 }
 
 pub fn encrypt(history: &History, key: &Key) -> Result<EncryptedHistory> {
     // serialize with msgpack
-    let mut buf = rmp_serde::to_vec(history)?;
+    let mut buf = encode(history)?;
 
     let nonce = XSalsa20Poly1305::generate_nonce(&mut OsRng);
     XSalsa20Poly1305::new(key)
@@ -125,50 +126,138 @@ pub fn decrypt(mut encrypted_history: EncryptedHistory, key: &Key) -> Result<His
         .map_err(|_| eyre!("could not encrypt"))?;
     let plaintext = encrypted_history.ciphertext;
 
-    let history = rmp_serde::from_slice(&plaintext);
+    let history = decode(&plaintext)?;
-
-    let Ok(history) = history else {
-        let history: HistoryWithoutDelete = rmp_serde::from_slice(&plaintext)?;
-
-        return Ok(History {
-            id: history.id,
-            cwd: history.cwd,
-            exit: history.exit,
-            command: history.command,
-            session: history.session,
-            duration: history.duration,
-            hostname: history.hostname,
-            timestamp: history.timestamp,
-            deleted_at: None,
-        });
-    };
 
     Ok(history)
 }
 
+fn encode(h: &History) -> Result<Vec<u8>> {
+    use rmp::encode;
+
+    let mut output = vec![];
+    // INFO: ensure this is updated when adding new fields
+    encode::write_array_len(&mut output, 9)?;
+
+    encode::write_str(&mut output, &h.id)?;
+    encode::write_str(
+        &mut output,
+        &dbg!(h
+            .timestamp
+            .to_rfc3339_opts(chrono::SecondsFormat::AutoSi, true)),
+    )?;
+    encode::write_sint(&mut output, h.duration)?;
+    encode::write_sint(&mut output, h.exit)?;
+    encode::write_str(&mut output, &h.command)?;
+    encode::write_str(&mut output, &h.cwd)?;
+    encode::write_str(&mut output, &h.session)?;
+    encode::write_str(&mut output, &h.hostname)?;
+    match h.deleted_at {
+        Some(d) => encode::write_str(
+            &mut output,
+            &d.to_rfc3339_opts(chrono::SecondsFormat::AutoSi, true),
+        )?,
+        None => encode::write_nil(&mut output)?,
+    }
+
+    Ok(output)
+}
+
+fn decode(bytes: &[u8]) -> Result<History> {
+    use rmp::decode::{self, DecodeStringError};
+
+    let mut bytes = Bytes::new(bytes);
+
+    let nfields = decode::read_array_len(&mut bytes).map_err(error_report)?;
+    if nfields < 8 {
+        bail!("malformed decrypted history")
+    }
+    if nfields > 9 {
+        bail!("cannot decrypt history from a newer version of atuin");
+    }
+
+    let bytes = bytes.remaining_slice();
+    let (id, bytes) = decode::read_str_from_slice(bytes).map_err(error_report)?;
+    let (timestamp, bytes) = decode::read_str_from_slice(bytes).map_err(error_report)?;
+
+    let mut bytes = Bytes::new(bytes);
+    let duration = decode::read_int(&mut bytes).map_err(error_report)?;
+    let exit = decode::read_int(&mut bytes).map_err(error_report)?;
+
+    let bytes = bytes.remaining_slice();
+    let (command, bytes) = decode::read_str_from_slice(bytes).map_err(error_report)?;
+    let (cwd, bytes) = decode::read_str_from_slice(bytes).map_err(error_report)?;
+    let (session, bytes) = decode::read_str_from_slice(bytes).map_err(error_report)?;
+    let (hostname, bytes) = decode::read_str_from_slice(bytes).map_err(error_report)?;
+
+    // if we have more fields, try and get the deleted_at
+    let mut deleted_at = None;
+    let mut bytes = bytes;
+    if nfields > 8 {
+        bytes = match decode::read_str_from_slice(bytes) {
+            Ok((d, b)) => {
+                deleted_at = Some(d);
+                b
+            }
+            // we accept null here
+            Err(DecodeStringError::TypeMismatch(Marker::Null)) => {
+                // consume the null marker
+                let mut c = Bytes::new(bytes);
+                decode::read_nil(&mut c).map_err(error_report)?;
+                c.remaining_slice()
+            }
+            Err(err) => return Err(error_report(err)),
+        };
+    }
+
+    if !bytes.is_empty() {
+        bail!("trailing bytes in encoded history. malformed")
+    }
+
+    Ok(History {
+        id: id.to_owned(),
+        timestamp: DateTime::parse_from_rfc3339(timestamp)?.with_timezone(&Utc),
+        duration,
+        exit,
+        command: command.to_owned(),
+        cwd: cwd.to_owned(),
+        session: session.to_owned(),
+        hostname: hostname.to_owned(),
+        deleted_at: deleted_at
+            .map(DateTime::parse_from_rfc3339)
+            .transpose()?
+            .map(|dt| dt.with_timezone(&Utc)),
+    })
+}
+
+fn error_report<E: std::fmt::Debug>(err: E) -> eyre::Report {
+    eyre!("{err:?}")
+}
+
 #[cfg(test)]
 mod test {
     use xsalsa20poly1305::{aead::OsRng, KeyInit, XSalsa20Poly1305};
 
     use crate::history::History;
 
-    use super::{decrypt, encrypt};
+    use super::{decode, decrypt, encode, encrypt};
 
     #[test]
     fn test_encrypt_decrypt() {
         let key1 = XSalsa20Poly1305::generate_key(&mut OsRng);
         let key2 = XSalsa20Poly1305::generate_key(&mut OsRng);
 
-        let history = History::new(
+        let history = History::from_db()
-            chrono::Utc::now(),
+            .id("1".into())
-            "ls".to_string(),
+            .timestamp(chrono::Utc::now())
-            "/home/ellie".to_string(),
+            .command("ls".into())
-            0,
+            .cwd("/home/ellie".into())
-            1,
+            .exit(0)
-            Some("beep boop".to_string()),
+            .duration(1)
-            Some("booop".to_string()),
+            .session("beep boop".into())
-            None,
+            .hostname("booop".into())
-        );
+            .deleted_at(None)
+            .build()
+            .into();
 
         let e1 = encrypt(&history, &key1).unwrap();
         let e2 = encrypt(&history, &key2).unwrap();
@@ -186,4 +275,86 @@ mod test {
         // this should err
         let _ = decrypt(e2, &key1).expect_err("expected an error decrypting with invalid key");
     }
+
+    #[test]
+    fn test_decode() {
+        let bytes = [
+            0x99, 0xD9, 32, 54, 54, 100, 49, 54, 99, 98, 101, 101, 55, 99, 100, 52, 55, 53, 51, 56,
+            101, 53, 99, 53, 98, 56, 98, 52, 52, 101, 57, 48, 48, 54, 101, 187, 50, 48, 50, 51, 45,
+            48, 53, 45, 50, 56, 84, 49, 56, 58, 51, 53, 58, 52, 48, 46, 54, 51, 51, 56, 55, 50, 90,
+            206, 2, 238, 210, 240, 0, 170, 103, 105, 116, 32, 115, 116, 97, 116, 117, 115, 217, 42,
+            47, 85, 115, 101, 114, 115, 47, 99, 111, 110, 114, 97, 100, 46, 108, 117, 100, 103, 97,
+            116, 101, 47, 68, 111, 99, 117, 109, 101, 110, 116, 115, 47, 99, 111, 100, 101, 47, 97,
+            116, 117, 105, 110, 217, 32, 98, 57, 55, 100, 57, 97, 51, 48, 54, 102, 50, 55, 52, 52,
+            55, 51, 97, 50, 48, 51, 100, 50, 101, 98, 97, 52, 49, 102, 57, 52, 53, 55, 187, 102,
+            118, 102, 103, 57, 51, 54, 99, 48, 107, 112, 102, 58, 99, 111, 110, 114, 97, 100, 46,
+            108, 117, 100, 103, 97, 116, 101, 192,
+        ];
+        let history = History {
+            id: "66d16cbee7cd47538e5c5b8b44e9006e".to_owned(),
+            timestamp: "2023-05-28T18:35:40.633872Z".parse().unwrap(),
+            duration: 49206000,
+            exit: 0,
+            command: "git status".to_owned(),
+            cwd: "/Users/conrad.ludgate/Documents/code/atuin".to_owned(),
+            session: "b97d9a306f274473a203d2eba41f9457".to_owned(),
+            hostname: "fvfg936c0kpf:conrad.ludgate".to_owned(),
+            deleted_at: None,
+        };
+
+        let h = decode(&bytes).unwrap();
+        assert_eq!(history, h);
+
+        let b = encode(&h).unwrap();
+        assert_eq!(&bytes, &*b);
+    }
+
+    #[test]
+    fn test_decode_deleted() {
+        let history = History {
+            id: "66d16cbee7cd47538e5c5b8b44e9006e".to_owned(),
+            timestamp: "2023-05-28T18:35:40.633872Z".parse().unwrap(),
+            duration: 49206000,
+            exit: 0,
+            command: "git status".to_owned(),
+            cwd: "/Users/conrad.ludgate/Documents/code/atuin".to_owned(),
+            session: "b97d9a306f274473a203d2eba41f9457".to_owned(),
+            hostname: "fvfg936c0kpf:conrad.ludgate".to_owned(),
+            deleted_at: Some("2023-05-28T18:35:40.633872Z".parse().unwrap()),
+        };
+
+        let b = encode(&history).unwrap();
+        let h = decode(&b).unwrap();
+        assert_eq!(history, h);
+    }
+
+    #[test]
+    fn test_decode_old() {
+        let bytes = [
+            0x98, 0xD9, 32, 54, 54, 100, 49, 54, 99, 98, 101, 101, 55, 99, 100, 52, 55, 53, 51, 56,
+            101, 53, 99, 53, 98, 56, 98, 52, 52, 101, 57, 48, 48, 54, 101, 187, 50, 48, 50, 51, 45,
+            48, 53, 45, 50, 56, 84, 49, 56, 58, 51, 53, 58, 52, 48, 46, 54, 51, 51, 56, 55, 50, 90,
+            206, 2, 238, 210, 240, 0, 170, 103, 105, 116, 32, 115, 116, 97, 116, 117, 115, 217, 42,
+            47, 85, 115, 101, 114, 115, 47, 99, 111, 110, 114, 97, 100, 46, 108, 117, 100, 103, 97,
+            116, 101, 47, 68, 111, 99, 117, 109, 101, 110, 116, 115, 47, 99, 111, 100, 101, 47, 97,
+            116, 117, 105, 110, 217, 32, 98, 57, 55, 100, 57, 97, 51, 48, 54, 102, 50, 55, 52, 52,
+            55, 51, 97, 50, 48, 51, 100, 50, 101, 98, 97, 52, 49, 102, 57, 52, 53, 55, 187, 102,
+            118, 102, 103, 57, 51, 54, 99, 48, 107, 112, 102, 58, 99, 111, 110, 114, 97, 100, 46,
+            108, 117, 100, 103, 97, 116, 101,
+        ];
+        let history = History {
+            id: "66d16cbee7cd47538e5c5b8b44e9006e".to_owned(),
+            timestamp: "2023-05-28T18:35:40.633872Z".parse().unwrap(),
+            duration: 49206000,
+            exit: 0,
+            command: "git status".to_owned(),
+            cwd: "/Users/conrad.ludgate/Documents/code/atuin".to_owned(),
+            session: "b97d9a306f274473a203d2eba41f9457".to_owned(),
+            hostname: "fvfg936c0kpf:conrad.ludgate".to_owned(),
+            deleted_at: None,
+        };
+
+        let h = decode(&bytes).unwrap();
+        assert_eq!(history, h);
+    }
 }

atuin-client/src/history.rs

@@ -1,42 +1,51 @@
 use std::env;
 
 use chrono::Utc;
-use serde::{Deserialize, Serialize};
 
 use atuin_common::utils::uuid_v7;
 
-// Any new fields MUST be Optional<>!
+mod builder;
-#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, sqlx::FromRow)]
-pub struct History {
-    pub id: String,
-    pub timestamp: chrono::DateTime<Utc>,
-    pub duration: i64,
-    pub exit: i64,
-    pub command: String,
-    pub cwd: String,
-    pub session: String,
-    pub hostname: String,
-    pub deleted_at: Option<chrono::DateTime<Utc>>,
-}
 
-// Forgive me, for I have sinned
+/// Client-side history entry.
-// I need to replace rmp with something that is more backwards-compatible.
+///
-// Protobuf, or maybe just json
+/// Client stores data unencrypted, and only encrypts it before sending to the server.
-#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, sqlx::FromRow)]
+///
-pub struct HistoryWithoutDelete {
+/// To create a new history entry, use one of the builders:
+/// - [`History::import()`] to import an entry from the shell history file
+/// - [`History::capture()`] to capture an entry via hook
+/// - [`History::from_db()`] to create an instance from the database entry
+//
+// ## Implementation Notes
+//
+// New fields must should be added to `encryption::{encode, decode}` in a backwards
+// compatible way. (eg sensible defaults and updating the nfields parameter)
+#[derive(Debug, Clone, PartialEq, Eq, sqlx::FromRow)]
+pub struct History {
+    /// A client-generated ID, used to identify the entry when syncing.
+    ///
+    /// Stored as `client_id` in the database.
     pub id: String,
+    /// When the command was run.
     pub timestamp: chrono::DateTime<Utc>,
+    /// How long the command took to run.
     pub duration: i64,
+    /// The exit code of the command.
     pub exit: i64,
+    /// The command that was run.
     pub command: String,
+    /// The current working directory when the command was run.
     pub cwd: String,
+    /// The session ID, associated with a terminal session.
     pub session: String,
+    /// The hostname of the machine the command was run on.
     pub hostname: String,
+    /// Timestamp, which is set when the entry is deleted, allowing a soft delete.
+    pub deleted_at: Option<chrono::DateTime<Utc>>,
 }
 
 impl History {
     #[allow(clippy::too_many_arguments)]
-    pub fn new(
+    fn new(
         timestamp: chrono::DateTime<Utc>,
         command: String,
         cwd: String,
@@ -70,6 +79,109 @@ impl History {
         }
     }
 
+    /// Builder for a history entry that is imported from shell history.
+    ///
+    /// The only two required fields are `timestamp` and `command`.
+    ///
+    /// ## Examples
+    /// ```
+    /// use atuin_client::history::History;
+    ///
+    /// let history: History = History::import()
+    ///     .timestamp(chrono::Utc::now())
+    ///     .command("ls -la")
+    ///     .build()
+    ///     .into();
+    /// ```
+    ///
+    /// If shell history contains more information, it can be added to the builder:
+    /// ```
+    /// use atuin_client::history::History;
+    ///
+    /// let history: History = History::import()
+    ///     .timestamp(chrono::Utc::now())
+    ///     .command("ls -la")
+    ///     .cwd("/home/user")
+    ///     .exit(0)
+    ///     .duration(100)
+    ///     .build()
+    ///     .into();
+    /// ```
+    ///
+    /// Unknown command or command without timestamp cannot be imported, which
+    /// is forced at compile time:
+    ///
+    /// ```compile_fail
+    /// use atuin_client::history::History;
+    ///
+    /// // this will not compile because timestamp is missing
+    /// let history: History = History::import()
+    ///     .command("ls -la")
+    ///     .build()
+    ///     .into();
+    /// ```
+    pub fn import() -> builder::HistoryImportedBuilder {
+        builder::HistoryImported::builder()
+    }
+
+    /// Builder for a history entry that is captured via hook.
+    ///
+    /// This builder is used only at the `start` step of the hook,
+    /// so it doesn't have any fields which are known only after
+    /// the command is finished, such as `exit` or `duration`.
+    ///
+    /// ## Examples
+    /// ```rust
+    /// use atuin_client::history::History;
+    ///
+    /// let history: History = History::capture()
+    ///     .timestamp(chrono::Utc::now())
+    ///     .command("ls -la")
+    ///     .cwd("/home/user")
+    ///     .build()
+    ///     .into();
+    /// ```
+    ///
+    /// Command without any required info cannot be captured, which is forced at compile time:
+    ///
+    /// ```compile_fail
+    /// use atuin_client::history::History;
+    ///
+    /// // this will not compile because `cwd` is missing
+    /// let history: History = History::capture()
+    ///     .timestamp(chrono::Utc::now())
+    ///     .command("ls -la")
+    ///     .build()
+    ///     .into();
+    /// ```
+    pub fn capture() -> builder::HistoryCapturedBuilder {
+        builder::HistoryCaptured::builder()
+    }
+
+    /// Builder for a history entry that is imported from the database.
+    ///
+    /// All fields are required, as they are all present in the database.
+    ///
+    /// ```compile_fail
+    /// use atuin_client::history::History;
+    ///
+    /// // this will not compile because `id` field is missing
+    /// let history: History = History::from_db()
+    ///     .timestamp(chrono::Utc::now())
+    ///     .command("ls -la".to_string())
+    ///     .cwd("/home/user".to_string())
+    ///     .exit(0)
+    ///     .duration(100)
+    ///     .session("somesession".to_string())
+    ///     .hostname("localhost".to_string())
+    ///     .deleted_at(None)
+    ///     .build()
+    ///     .into();
+    /// ```
+    pub fn from_db() -> builder::HistoryFromDbBuilder {
+        builder::HistoryFromDb::builder()
+    }
+
     pub fn success(&self) -> bool {
         self.exit == 0 || self.duration == -1
     }

atuin-client/src/history/builder.rs

@@ -0,0 +1,100 @@
+use chrono::Utc;
+use typed_builder::TypedBuilder;
+
+use super::History;
+
+/// Builder for a history entry that is imported from shell history.
+///
+/// The only two required fields are `timestamp` and `command`.
+#[derive(Debug, Clone, TypedBuilder)]
+pub struct HistoryImported {
+    timestamp: chrono::DateTime<Utc>,
+    #[builder(setter(into))]
+    command: String,
+    #[builder(default = "unknown".into(), setter(into))]
+    cwd: String,
+    #[builder(default = -1)]
+    exit: i64,
+    #[builder(default = -1)]
+    duration: i64,
+    #[builder(default, setter(strip_option, into))]
+    session: Option<String>,
+    #[builder(default, setter(strip_option, into))]
+    hostname: Option<String>,
+}
+
+impl From<HistoryImported> for History {
+    fn from(imported: HistoryImported) -> Self {
+        History::new(
+            imported.timestamp,
+            imported.command,
+            imported.cwd,
+            imported.exit,
+            imported.duration,
+            imported.session,
+            imported.hostname,
+            None,
+        )
+    }
+}
+
+/// Builder for a history entry that is captured via hook.
+///
+/// This builder is used only at the `start` step of the hook,
+/// so it doesn't have any fields which are known only after
+/// the command is finished, such as `exit` or `duration`.
+#[derive(Debug, Clone, TypedBuilder)]
+pub struct HistoryCaptured {
+    timestamp: chrono::DateTime<Utc>,
+    #[builder(setter(into))]
+    command: String,
+    #[builder(setter(into))]
+    cwd: String,
+}
+
+impl From<HistoryCaptured> for History {
+    fn from(captured: HistoryCaptured) -> Self {
+        History::new(
+            captured.timestamp,
+            captured.command,
+            captured.cwd,
+            -1,
+            -1,
+            None,
+            None,
+            None,
+        )
+    }
+}
+
+/// Builder for a history entry that is loaded from the database.
+///
+/// All fields are required, as they are all present in the database.
+#[derive(Debug, Clone, TypedBuilder)]
+pub struct HistoryFromDb {
+    id: String,
+    timestamp: chrono::DateTime<Utc>,
+    command: String,
+    cwd: String,
+    exit: i64,
+    duration: i64,
+    session: String,
+    hostname: String,
+    deleted_at: Option<chrono::DateTime<Utc>>,
+}
+
+impl From<HistoryFromDb> for History {
+    fn from(from_db: HistoryFromDb) -> Self {
+        History {
+            id: from_db.id,
+            timestamp: from_db.timestamp,
+            exit: from_db.exit,
+            command: from_db.command,
+            cwd: from_db.cwd,
+            duration: from_db.duration,
+            session: from_db.session,
+            hostname: from_db.hostname,
+            deleted_at: from_db.deleted_at,
+        }
+    }
+}

atuin-client/src/import/bash.rs

@@ -80,17 +80,9 @@ impl Importer for Bash {
                     next_timestamp = t;
                 }
                 LineType::Command(c) => {
-                    let entry = History::new(
+                    let imported = History::import().timestamp(next_timestamp).command(c);
-                        next_timestamp,
+
-                        c.into(),
+                    h.push(imported.build().into()).await?;
-                        "unknown".into(),
-                        -1,
-                        -1,
-                        None,
-                        None,
-                        None,
-                    );
-                    h.push(entry).await?;
                     next_timestamp += timestamp_increment;
                 }
             }

atuin-client/src/import/fish.rs

@@ -73,19 +73,9 @@ impl Importer for Fish {
                 // first, we must deal with the prev cmd
                 if let Some(cmd) = cmd.take() {
                     let time = time.unwrap_or(now);
+                    let entry = History::import().timestamp(time).command(cmd);
 
-                    loader
+                    loader.push(entry.build().into()).await?;
-                        .push(History::new(
-                            time,
-                            cmd,
-                            "unknown".into(),
-                            -1,
-                            -1,
-                            None,
-                            None,
-                            None,
-                        ))
-                        .await?;
                 }
 
                 // using raw strings to avoid needing escaping.
@@ -109,19 +99,9 @@ impl Importer for Fish {
         // we might have a trailing cmd
         if let Some(cmd) = cmd.take() {
             let time = time.unwrap_or(now);
+            let entry = History::import().timestamp(time).command(cmd);
 
-            loader
+            loader.push(entry.build().into()).await?;
-                .push(History::new(
-                    time,
-                    cmd,
-                    "unknown".into(),
-                    -1,
-                    -1,
-                    None,
-                    None,
-                    None,
-                ))
-                .await?;
         }
 
         Ok(())

atuin-client/src/import/nu.rs

@@ -58,17 +58,9 @@ impl Importer for Nu {
             let offset = chrono::Duration::nanoseconds(counter);
             counter += 1;
 
-            h.push(History::new(
+            let entry = History::import().timestamp(now - offset).command(cmd);
-                now - offset, // preserve ordering
+
-                cmd,
+            h.push(entry.build().into()).await?;
-                String::from("unknown"),
-                -1,
-                -1,
-                None,
-                None,
-                None,
-            ))
-            .await?;
         }
 
         Ok(())

atuin-client/src/import/nu_histdb.rs

@@ -30,16 +30,19 @@ impl From<HistDbEntry> for History {
     fn from(histdb_item: HistDbEntry) -> Self {
         let ts_secs = histdb_item.start_timestamp / 1000;
         let ts_ns = (histdb_item.start_timestamp % 1000) * 1_000_000;
-        History::new(
+        let imported = History::import()
-            DateTime::from_utc(NaiveDateTime::from_timestamp(ts_secs, ts_ns as u32), Utc),
+            .timestamp(DateTime::from_utc(
-            String::from_utf8(histdb_item.command_line).unwrap(),
+                NaiveDateTime::from_timestamp(ts_secs, ts_ns as u32),
-            String::from_utf8(histdb_item.cwd).unwrap(),
+                Utc,
-            histdb_item.exit_status,
+            ))
-            histdb_item.duration_ms,
+            .command(String::from_utf8(histdb_item.command_line).unwrap())
-            Some(format!("{:x}", histdb_item.session_id)),
+            .cwd(String::from_utf8(histdb_item.cwd).unwrap())
-            Some(String::from_utf8(histdb_item.hostname).unwrap()),
+            .exit(histdb_item.exit_status)
-            None,
+            .duration(histdb_item.duration_ms)
-        )
+            .session(format!("{:x}", histdb_item.session_id))
+            .hostname(String::from_utf8(histdb_item.hostname).unwrap());
+
+        imported.build().into()
     }
 }
 

atuin-client/src/import/resh.rs

@@ -122,18 +122,17 @@ impl Importer for Resh {
                 difference.num_nanoseconds().unwrap_or(0)
             };
 
-            h.push(History {
+            let imported = History::import()
-                id: uuid_v7().as_simple().to_string(),
+                .command(entry.cmd_line)
-                timestamp,
+                .timestamp(timestamp)
-                duration,
+                .duration(duration)
-                exit: entry.exit_code,
+                .exit(entry.exit_code)
-                command: entry.cmd_line,
+                .cwd(entry.pwd)
-                cwd: entry.pwd,
+                .hostname(entry.host)
-                session: uuid_v7().as_simple().to_string(),
+                // CHECK: should we add uuid here? It's not set in the other importers
-                hostname: entry.host,
+                .session(uuid_v7().as_simple().to_string());
-                deleted_at: None,
+
-            })
+            h.push(imported.build().into()).await?;
-            .await?;
         }
 
         Ok(())

atuin-client/src/import/zsh.rs

@@ -82,17 +82,12 @@ impl Importer for Zsh {
                     let offset = chrono::Duration::seconds(counter);
                     counter += 1;
 
-                    h.push(History::new(
+                    let imported = History::import()
-                        now - offset, // preserve ordering
+                        // preserve ordering
-                        command.trim_end().to_string(),
+                        .timestamp(now - offset)
-                        String::from("unknown"),
+                        .command(command.trim_end().to_string());
-                        -1,
+
-                        -1,
+                    h.push(imported.build().into()).await?;
-                        None,
-                        None,
-                        None,
-                    ))
-                    .await?;
                 }
             }
         }
@@ -113,19 +108,15 @@ fn parse_extended(line: &str, counter: i64) -> History {
     let time = Utc.timestamp(time, 0);
     let time = time + offset;
 
+    // use nanos, because why the hell not? we won't display them.
     let duration = duration.parse::<i64>().map_or(-1, |t| t * 1_000_000_000);
 
-    // use nanos, because why the hell not? we won't display them.
+    let imported = History::import()
-    History::new(
+        .timestamp(time)
-        time,
+        .command(command.trim_end().to_string())
-        command.trim_end().to_string(),
+        .duration(duration);
-        String::from("unknown"),
+
-        0, // assume 0, we have no way of knowing :(
+    imported.build().into()
-        duration,
-        None,
-        None,
-        None,
-    )
 }
 
 #[cfg(test)]

atuin-client/src/import/zsh_histdb.rs

@@ -61,27 +61,29 @@ pub struct HistDbEntry {
 
 impl From<HistDbEntry> for History {
     fn from(histdb_item: HistDbEntry) -> Self {
-        History::new(
+        let imported = History::import()
-            DateTime::from_utc(histdb_item.start_time, Utc), // must assume UTC?
+            .timestamp(DateTime::from_utc(histdb_item.start_time, Utc))
-            String::from_utf8(histdb_item.argv)
+            .command(
-                .unwrap_or_else(|_e| String::from(""))
+                String::from_utf8(histdb_item.argv)
-                .trim_end()
+                    .unwrap_or_else(|_e| String::from(""))
-                .to_string(),
+                    .trim_end()
-            String::from_utf8(histdb_item.dir)
+                    .to_string(),
-                .unwrap_or_else(|_e| String::from(""))
+            )
-                .trim_end()
+            .cwd(
-                .to_string(),
+                String::from_utf8(histdb_item.dir)
-            0, // assume 0, we have no way of knowing :(
+                    .unwrap_or_else(|_e| String::from(""))
-            histdb_item.duration,
+                    .trim_end()
-            None,
+                    .to_string(),
-            Some(
+            )
+            .duration(histdb_item.duration)
+            .hostname(
                 String::from_utf8(histdb_item.host)
                     .unwrap_or_else(|_e| String::from(""))
                     .trim_end()
                     .to_string(),
-            ),
+            );
-            None,
+
-        )
+        imported.build().into()
     }
 }
 

atuin-client/src/message.rs

@@ -1,5 +0,0 @@
-
-pub struct Message {
-    pub id: Uuid,
-    pub type: String,
-}

atuin-server-database/src/models.rs

@@ -7,6 +7,9 @@ pub struct History {
     pub hostname: String,
     pub timestamp: NaiveDateTime,
 
+    /// All the data we have about this command, encrypted.
+    ///
+    /// Currently this is an encrypted msgpack object, but this may change in the future.
     pub data: String,
 
     pub created_at: NaiveDateTime,
@@ -18,6 +21,9 @@ pub struct NewHistory {
     pub hostname: String,
     pub timestamp: chrono::NaiveDateTime,
 
+    /// All the data we have about this command, encrypted.
+    ///
+    /// Currently this is an encrypted msgpack object, but this may change in the future.
     pub data: String,
 }
 

atuin/src/command/client/history.rs

@@ -184,67 +184,126 @@ fn parse_fmt(format: &str) -> ParsedFmt {
 }
 
 impl Cmd {
-    pub async fn run(&self, settings: &Settings, db: &mut impl Database) -> Result<()> {
+    async fn handle_start(
+        db: &mut impl Database,
+        settings: &Settings,
+        command: &[String],
+    ) -> Result<()> {
+        let command = command.join(" ");
+
+        if command.starts_with(' ') || settings.history_filter.is_match(&command) {
+            return Ok(());
+        }
+
+        // It's better for atuin to silently fail here and attempt to
+        // store whatever is ran, than to throw an error to the terminal
+        let cwd = utils::get_current_dir();
+        if !cwd.is_empty() && settings.cwd_filter.is_match(&cwd) {
+            return Ok(());
+        }
+
+        let h: History = History::capture()
+            .timestamp(chrono::Utc::now())
+            .command(command)
+            .cwd(cwd)
+            .build()
+            .into();
+
+        // print the ID
+        // we use this as the key for calling end
+        println!("{}", h.id);
+        db.save(&h).await?;
+        Ok(())
+    }
+
+    async fn handle_end(
+        db: &mut impl Database,
+        settings: &Settings,
+        id: &str,
+        exit: i64,
+    ) -> Result<()> {
+        if id.trim() == "" {
+            return Ok(());
+        }
+
+        let mut h = db.load(id).await?;
+
+        if h.duration > 0 {
+            debug!("cannot end history - already has duration");
+
+            // returning OK as this can occur if someone Ctrl-c a prompt
+            return Ok(());
+        }
+
+        h.exit = exit;
+        h.duration = chrono::Utc::now().timestamp_nanos() - h.timestamp.timestamp_nanos();
+
+        db.update(&h).await?;
+
+        if settings.should_sync()? {
+            #[cfg(feature = "sync")]
+            {
+                debug!("running periodic background sync");
+                sync::sync(settings, false, db).await?;
+            }
+            #[cfg(not(feature = "sync"))]
+            debug!("not compiled with sync support");
+        } else {
+            debug!("sync disabled! not syncing");
+        }
+
+        Ok(())
+    }
+
+    async fn handle_list(
+        db: &mut impl Database,
+        settings: &Settings,
+        context: atuin_client::database::Context,
+        session: bool,
+        cwd: bool,
+        mode: ListMode,
+        format: Option<String>,
+    ) -> Result<()> {
+        let session = if session {
+            Some(env::var("ATUIN_SESSION")?)
+        } else {
+            None
+        };
+        let cwd = if cwd {
+            Some(utils::get_current_dir())
+        } else {
+            None
+        };
+
+        let history = match (session, cwd) {
+            (None, None) => db.list(settings.filter_mode, &context, None, false).await?,
+            (None, Some(cwd)) => {
+                let query = format!("select * from history where cwd = '{cwd}';");
+                db.query_history(&query).await?
+            }
+            (Some(session), None) => {
+                let query = format!("select * from history where session = '{session}';");
+                db.query_history(&query).await?
+            }
+            (Some(session), Some(cwd)) => {
+                let query = format!(
+                    "select * from history where cwd = '{cwd}' and session = '{session}';",
+                );
+                db.query_history(&query).await?
+            }
+        };
+
+        print_list(&history, mode, format.as_deref());
+
+        Ok(())
+    }
+
+    pub async fn run(self, settings: &Settings, db: &mut impl Database) -> Result<()> {
         let context = current_context();
 
         match self {
-            Self::Start { command: words } => {
+            Self::Start { command } => Self::handle_start(db, settings, &command).await,
-                let command = words.join(" ");
+            Self::End { id, exit } => Self::handle_end(db, settings, &id, exit).await,
-
-                if command.starts_with(' ') || settings.history_filter.is_match(&command) {
-                    return Ok(());
-                }
-
-                // It's better for atuin to silently fail here and attempt to
-                // store whatever is ran, than to throw an error to the terminal
-                let cwd = utils::get_current_dir();
-                if !cwd.is_empty() && settings.cwd_filter.is_match(&cwd) {
-                    return Ok(());
-                }
-
-                let h = History::new(chrono::Utc::now(), command, cwd, -1, -1, None, None, None);
-
-                // print the ID
-                // we use this as the key for calling end
-                println!("{}", h.id);
-                db.save(&h).await?;
-                Ok(())
-            }
-
-            Self::End { id, exit } => {
-                if id.trim() == "" {
-                    return Ok(());
-                }
-
-                let mut h = db.load(id).await?;
-
-                if h.duration > 0 {
-                    debug!("cannot end history - already has duration");
-
-                    // returning OK as this can occur if someone Ctrl-c a prompt
-                    return Ok(());
-                }
-
-                h.exit = *exit;
-                h.duration = chrono::Utc::now().timestamp_nanos() - h.timestamp.timestamp_nanos();
-
-                db.update(&h).await?;
-
-                if settings.should_sync()? {
-                    #[cfg(feature = "sync")]
-                    {
-                        debug!("running periodic background sync");
-                        sync::sync(settings, false, db).await?;
-                    }
-                    #[cfg(not(feature = "sync"))]
-                    debug!("not compiled with sync support");
-                } else {
-                    debug!("sync disabled! not syncing");
-                }
-
-                Ok(())
-            }
-
             Self::List {
                 session,
                 cwd,
@@ -252,42 +311,8 @@ impl Cmd {
                 cmd_only,
                 format,
             } => {
-                let session = if *session {
+                let mode = ListMode::from_flags(human, cmd_only);
-                    Some(env::var("ATUIN_SESSION")?)
+                Self::handle_list(db, settings, context, session, cwd, mode, format).await
-                } else {
-                    None
-                };
-                let cwd = if *cwd {
-                    Some(utils::get_current_dir())
-                } else {
-                    None
-                };
-
-                let history = match (session, cwd) {
-                    (None, None) => db.list(settings.filter_mode, &context, None, false).await?,
-                    (None, Some(cwd)) => {
-                        let query = format!("select * from history where cwd = '{cwd}';");
-                        db.query_history(&query).await?
-                    }
-                    (Some(session), None) => {
-                        let query = format!("select * from history where session = '{session}';");
-                        db.query_history(&query).await?
-                    }
-                    (Some(session), Some(cwd)) => {
-                        let query = format!(
-                            "select * from history where cwd = '{cwd}' and session = '{session}';",
-                        );
-                        db.query_history(&query).await?
-                    }
-                };
-
-                print_list(
-                    &history,
-                    ListMode::from_flags(*human, *cmd_only),
-                    format.as_deref(),
-                );
-
-                Ok(())
             }
 
             Self::Last {
@@ -298,7 +323,7 @@ impl Cmd {
                 let last = db.last().await?;
                 print_list(
                     &[last],
-                    ListMode::from_flags(*human, *cmd_only),
+                    ListMode::from_flags(human, cmd_only),
                     format.as_deref(),
                 );