From 751fa568808489801e5f274fb8907280f33c227c Mon Sep 17 00:00:00 2001 From: Luke Melia Date: Tue, 27 Oct 2026 19:36:35 -0400 Subject: [PATCH 1/2] Read a revision's tree and files back from an explicit-commit repo `/tree` lists every file in a revision with its git blob ID, and `/read` returns the bytes at given paths, paging past a 32 MiB ceiling through `remaining`. realm-server's recovery uses them to compare a realm's disk with HEAD and to bring disk up to HEAD. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AnysKbB8Y5FgNf4nQuAd6M --- packages/deckd/README.md | 59 ++++- packages/deckd/src/api.rs | 68 +++++- packages/deckd/src/history/commit.rs | 345 +++++++++++++++++++++++++-- 3 files changed, 450 insertions(+), 22 deletions(-) diff --git a/packages/deckd/README.md b/packages/deckd/README.md index adf8e61..e50799d 100644 --- a/packages/deckd/README.md +++ b/packages/deckd/README.md @@ -72,11 +72,13 @@ When Hub uses `DECKD_URL`, Node watchers publish + HMR only — they do not | ensure / seal / restore | per-workspace **write** lock | | list / file-at / file-list-at | per-workspace **read** lock (concurrent) | | commit / outcome | per-repo **file** lock (cross-process) | +| tree / read / tree-hash | none; the file lock only on a miss | ## Explicit commits `/commit`, `/outcome`, `/head` and `/tree-hash` record a revision from content -the caller staged, never from a working copy. A writer such as realm-server +the caller staged, never from a working copy. `/tree` and `/read` read a +revision back. A writer such as realm-server can then record each write exactly once, even when deckd's response is lost. Each call names `repo`, the absolute path of one history directory (for @@ -230,6 +232,59 @@ any repo, whoever committed it and whatever the message. The revision must be in the repo's history; a commit object that was written but never published is `revision-not-found`. +### `POST /tree` + +```json +{ + "repo": "/realms/acme/.history", + "revision": "6f1c2d8e0a9b4c3d2e1f0a9b8c7d6e5f4a3b2c1d" +} +``` + +```json +{ + "entries": [ + { + "path": "Author/jane.json", + "blob": "3b18e512dba79e4c8300dd08aeb37f8e728b8dad", + "executable": false + } + ] +} +``` + +Every file in the revision's tree, in path order. `blob` is the git blob ID +of the file's bytes (`git hash-object`), so a caller can compare files on disk +with a revision without reading them out of history. The revision must be in +the repo's history, as for `/tree-hash`. + +### `POST /read` + +```json +{ + "repo": "/realms/acme/.history", + "revision": "6f1c2d8e0a9b4c3d2e1f0a9b8c7d6e5f4a3b2c1d", + "paths": ["Author/jane.json", "Author/old.json"] +} +``` + +```json +{ + "files": [ + { "path": "Author/jane.json", "contentBase64": "eyJuYW1lIjoiSmFuZSJ9" }, + { "path": "Author/old.json", "absent": true } + ], + "remaining": [] +} +``` + +- Each path's bytes at the revision, in the order asked. A path the revision + has no file at, including a directory, is `absent`. +- One answer carries at most 32 MiB of content. Paths past that point are not + read and are listed in `remaining`, in order, for the caller to ask for + again. The first path is always read, so every file can be read. +- Paths follow `/commit`'s rules. + ### Errors Every error body is `{ "code", "error" }`, plus fields named below, on every @@ -238,7 +293,7 @@ route, including a request deckd can't parse. | Status | `code` | When | | ----------- | -------------------- | ----------------------------------------------------------------------- | | 400/415/422 | `invalid-request` | Unparseable JSON, a missing field, the wrong content type, or bad input | -| 404 | `revision-not-found` | `/tree-hash` for a revision the repo doesn't have | +| 404 | `revision-not-found` | `/tree-hash`, `/tree` or `/read` for a revision the repo doesn't have | | 409 | `parent-mismatch` | `parent` isn't HEAD; body has `head` | | 409 | `operation-fenced` | The operation ID was reported `absent` by `/outcome` | | 409 | `operation-mismatch` | The operation ID committed a different request; body has `revision` | diff --git a/packages/deckd/src/api.rs b/packages/deckd/src/api.rs index d38d581..ac2d714 100644 --- a/packages/deckd/src/api.rs +++ b/packages/deckd/src/api.rs @@ -13,7 +13,7 @@ use tower_http::trace::TraceLayer; use crate::history::commit::{ Author, Change, CommitError, CommitOutcome, CommitRequest, CommitService, CommitTrailer, - OperationOutcome, RevisionTree, + OperationOutcome, RevisionTree, TreeEntry, }; use crate::history::{Actor, HistoryBackend, HistoryBackendError, HistoryEntry, RestorePlan}; use crate::watch::WatchPlane; @@ -54,6 +54,8 @@ pub fn router(state: AppState) -> Router { .route("/outcome", post(outcome)) .route("/head", post(head)) .route("/tree-hash", post(tree_hash)) + .route("/tree", post(tree)) + .route("/read", post(read)) .route("/ensure-build", post(ensure_build)) .route("/artifact-at", post(artifact_at)) .layer(TraceLayer::new_for_http()) @@ -77,6 +79,8 @@ pub fn router(state: AppState) -> Router { .route("/outcome", post(outcome)) .route("/head", post(head)) .route("/tree-hash", post(tree_hash)) + .route("/tree", post(tree)) + .route("/read", post(read)) .layer(TraceLayer::new_for_http()) .with_state(state); @@ -312,6 +316,34 @@ struct TreeHashResponse { tree: String, } +#[derive(Debug, Serialize)] +struct TreeResponse { + entries: Vec, +} + +#[derive(Debug, Deserialize)] +struct ReadBody { + repo: String, + revision: String, + paths: Vec, +} + +#[derive(Debug, Serialize)] +#[serde(rename_all = "camelCase")] +struct ReadFile { + path: String, + #[serde(skip_serializing_if = "Option::is_none")] + content_base64: Option, + #[serde(skip_serializing_if = "std::ops::Not::not")] + absent: bool, +} + +#[derive(Debug, Serialize)] +struct ReadResponse { + files: Vec, + remaining: Vec, +} + async fn commit( State(st): State, Body(body): Body, @@ -383,6 +415,40 @@ async fn tree_hash( Ok(Json(TreeHashResponse { tree: found.tree })) } +async fn tree( + State(st): State, + Body(body): Body, +) -> Result, ApiError> { + let repo = std::path::PathBuf::from(body.repo); + let revision = body.revision; + let entries = run_commits(&st, move |c| c.tree(&repo, &revision)).await?; + Ok(Json(TreeResponse { entries })) +} + +async fn read( + State(st): State, + Body(body): Body, +) -> Result, ApiError> { + let repo = std::path::PathBuf::from(body.repo); + let revision = body.revision; + let paths = body.paths; + let found = run_commits(&st, move |c| c.read(&repo, &revision, &paths)).await?; + Ok(Json(ReadResponse { + files: found + .files + .into_iter() + .map(|file| ReadFile { + path: file.path, + absent: file.content.is_none(), + content_base64: file + .content + .map(|bytes| base64::engine::general_purpose::STANDARD.encode(bytes)), + }) + .collect(), + remaining: found.remaining, + })) +} + /// Commit work waits on repo locks and drives jj-lib's `!Send` futures, so it /// runs on a blocking thread. async fn run_commits(st: &AppState, f: F) -> Result diff --git a/packages/deckd/src/history/commit.rs b/packages/deckd/src/history/commit.rs index 26c2375..8aa22e0 100644 --- a/packages/deckd/src/history/commit.rs +++ b/packages/deckd/src/history/commit.rs @@ -52,6 +52,7 @@ use std::path::{Path, PathBuf}; use std::sync::{Arc, Mutex, MutexGuard, TryLockError as MutexTryLockError, Weak}; use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; +use futures::AsyncReadExt as _; use jj_lib::backend::{BackendError, CommitId, CopyId, Signature, Timestamp, TreeValue}; use jj_lib::commit::Commit; use jj_lib::default_backend_factories::default_backend_factories; @@ -168,6 +169,34 @@ pub struct RevisionTree { pub tree: String, } +/// One file in a revision's tree. +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct TreeEntry { + pub path: String, + /// The git blob ID of the file's bytes. + pub blob: String, + pub executable: bool, +} + +/// A path's bytes at a revision, or `None` where the revision has no file. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct FileContent { + pub path: String, + pub content: Option>, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ReadResult { + pub files: Vec, + /// The paths asked for and not read, because reading them would have + /// taken the answer past `READ_MAX_BYTES`. + pub remaining: Vec, +} + +/// The most file content one `read` returns. The answer is base64 in one JSON +/// body, a third larger again, and is held in memory on both sides. +pub const READ_MAX_BYTES: usize = if cfg!(test) { 16 } else { 32 * 1024 * 1024 }; + /// Explicit-content commits, operation outcomes and tree hashes. Holds no /// state that correctness depends on; a fresh instance behaves exactly like /// one that has been running. @@ -240,6 +269,106 @@ impl CommitService { /// The tree of `revision`, which must be in the repo's history: a commit /// object that was written but never published is not found. pub fn tree_hash(&self, repo_dir: &Path, revision: &str) -> CommitResult { + let (_repo, commit) = self.published(repo_dir, revision)?; + Ok(RevisionTree { + revision: commit.id().hex(), + tree: tree_hash(&commit), + }) + } + + /// Every file in `revision`'s tree with its git blob ID, in path order. + /// The blob ID is the git object ID of the file's bytes, so a caller can + /// tell whether a file on disk matches without reading it out of history. + pub fn tree(&self, repo_dir: &Path, revision: &str) -> CommitResult> { + let (_repo, commit) = self.published(repo_dir, revision)?; + let mut entries = Vec::new(); + for (path, value) in commit.tree().entries() { + let path = path.as_internal_file_string().to_owned(); + match value.map_err(internal)?.into_resolved() { + Ok(Some(TreeValue::File { id, executable, .. })) => entries.push(TreeEntry { + path, + blob: id.hex(), + executable, + }), + // Explicit commits write only regular files, so anything else + // is a repo this service did not write. + _ => { + return Err(CommitError::Internal(format!( + "{path:?} is not a regular file in revision {revision}" + ))) + } + } + } + Ok(entries) + } + + /// The bytes `revision` holds at each of `paths`, in the order asked, or + /// `None` for a path that is absent or a directory there. Stops before the + /// file that would take the content past `READ_MAX_BYTES` (unless it is + /// the first), returning the paths it did not read as `remaining`, so a + /// caller reads a large set in several calls. + pub fn read( + &self, + repo_dir: &Path, + revision: &str, + paths: &[String], + ) -> CommitResult { + for path in paths { + if !is_valid_commit_path(path) { + return Err(CommitError::Invalid(format!("invalid path {path:?}"))); + } + } + let (repo, commit) = self.published(repo_dir, revision)?; + pollster::block_on(async { + let tree = commit.tree(); + let mut files = Vec::new(); + let mut total = 0usize; + for (index, path) in paths.iter().enumerate() { + let repo_path = RepoPathBuf::from_internal_string(path.clone()) + .map_err(|e| CommitError::Invalid(format!("invalid path {path:?}: {e}")))?; + let value = tree.path_value(&repo_path).await.map_err(internal)?; + let content = match value.as_resolved() { + Some(Some(TreeValue::File { id, .. })) => { + let mut bytes = Vec::new(); + repo.store() + .read_file(&repo_path, id) + .await + .map_err(internal)? + .read_to_end(&mut bytes) + .await + .map_err(internal)?; + Some(bytes) + } + _ => None, + }; + let size = content.as_ref().map_or(0, Vec::len); + if index > 0 && total + size > READ_MAX_BYTES { + return Ok(ReadResult { + files, + remaining: paths[index..].to_vec(), + }); + } + total += size; + files.push(FileContent { + path: path.clone(), + content, + }); + } + Ok(ReadResult { + files, + remaining: Vec::new(), + }) + }) + } + + /// The repo and `revision` as a commit in its history. Reads without the + /// repo locks, and takes them only when this host's view doesn't show the + /// revision, which settles HEAD first. + fn published( + &self, + repo_dir: &Path, + revision: &str, + ) -> CommitResult<(Arc, Commit)> { let repo_dir = self.resolve_repo_dir(repo_dir)?; let not_found = || CommitError::RevisionNotFound(revision.to_owned()); let id = CommitId::try_from_hex(revision).ok_or_else(not_found)?; @@ -251,23 +380,19 @@ impl CommitService { Err(error) => return Err(error), }; let heads: Vec = repo.view().heads().iter().cloned().collect(); - let commit = match pollster::block_on(published_commit(&repo, &heads, &id))? { - Some(commit) => commit, - // Unlocked, this host's listing may not show a peer's latest - // commit yet. Only a miss pays for the lock, which settles HEAD. - None => self - .locked(&repo_dir, |repo_dir, head_file| { - pollster::block_on(async { - let (repo, head) = settled_head(repo_dir, head_file).await?; - published_commit(&repo, &[head.id().clone()], &id).await - }) - })? - .ok_or_else(not_found)?, - }; - Ok(RevisionTree { - revision: commit.id().hex(), - tree: tree_hash(&commit), - }) + if let Some(commit) = pollster::block_on(published_commit(&repo, &heads, &id))? { + return Ok((repo, commit)); + } + // Unlocked, this host's listing may not show a peer's latest commit + // yet. Only a miss pays for the lock, which settles HEAD. + self.locked(&repo_dir, |repo_dir, head_file| { + pollster::block_on(async { + let (repo, head) = settled_head(repo_dir, head_file).await?; + let commit = published_commit(&repo, &[head.id().clone()], &id).await?; + Ok(commit.map(|commit| (repo, commit))) + }) + })? + .ok_or_else(not_found) } /// Run `body` holding both repo locks, with the resolved repo directory @@ -1209,8 +1334,6 @@ fn nonce() -> String { #[cfg(test)] mod tests { - use futures::AsyncReadExt as _; - use super::*; /// A realm directory whose history repo is `/.history`. The repo @@ -2379,4 +2502,188 @@ mod tests { }); assert!(executable); } + + fn git_blob_id(content: &str) -> String { + let mut child = std::process::Command::new("git") + .args(["hash-object", "--stdin"]) + .stdin(std::process::Stdio::piped()) + .stdout(std::process::Stdio::piped()) + .spawn() + .unwrap(); + child + .stdin + .take() + .unwrap() + .write_all(content.as_bytes()) + .unwrap(); + let output = child.wait_with_output().unwrap(); + String::from_utf8(output.stdout).unwrap().trim().to_owned() + } + + #[test] + fn tree_lists_every_file_with_its_git_blob_id() { + let realm = Realm::new("tree-lists"); + let service = CommitService::new(); + let root = head(&service, &realm); + let revision = commit( + &service, + &realm, + request( + &root, + "op-tree", + vec![ + write("b.json", "{\"b\":1}"), + write("dir/a.gts", "export {}"), + ], + ), + ) + .unwrap() + .revision; + let entries = service.tree(&realm.repo(), &revision).unwrap(); + assert_eq!( + entries, + vec![ + TreeEntry { + path: "b.json".into(), + blob: git_blob_id("{\"b\":1}"), + executable: false, + }, + TreeEntry { + path: "dir/a.gts".into(), + blob: git_blob_id("export {}"), + executable: false, + }, + ] + ); + assert_eq!(service.tree(&realm.repo(), &root).unwrap(), vec![]); + } + + #[test] + fn tree_of_an_unknown_revision_is_not_found() { + let realm = Realm::new("tree-unknown"); + let service = CommitService::new(); + head(&service, &realm); + let result = service.tree(&realm.repo(), &"ab".repeat(20)); + assert!(matches!(result, Err(CommitError::RevisionNotFound(_)))); + } + + #[test] + fn read_returns_each_path_at_the_revision() { + let realm = Realm::new("read-at"); + let service = CommitService::new(); + let root = head(&service, &realm); + let first = commit( + &service, + &realm, + request( + &root, + "op-1", + vec![write("a.txt", "one"), write("d/b.txt", "two")], + ), + ) + .unwrap() + .revision; + let second = commit( + &service, + &realm, + request( + &first, + "op-2", + vec![write("a.txt", "uno"), delete("d/b.txt")], + ), + ) + .unwrap() + .revision; + let paths: Vec = ["a.txt", "d/b.txt", "d", "missing.txt"] + .iter() + .map(|p| p.to_string()) + .collect(); + let at = |revision: &str| { + let found = service.read(&realm.repo(), revision, &paths).unwrap(); + assert!(found.remaining.is_empty()); + found + .files + .into_iter() + .map(|f| (f.path, f.content.map(|c| String::from_utf8(c).unwrap()))) + .collect::>() + }; + assert_eq!( + at(&first), + vec![ + ("a.txt".into(), Some("one".into())), + ("d/b.txt".into(), Some("two".into())), + ("d".into(), None), + ("missing.txt".into(), None), + ] + ); + assert_eq!( + at(&second), + vec![ + ("a.txt".into(), Some("uno".into())), + ("d/b.txt".into(), None), + ("d".into(), None), + ("missing.txt".into(), None), + ] + ); + } + + #[test] + fn read_stops_before_the_byte_ceiling_and_names_the_rest() { + let realm = Realm::new("read-ceiling"); + let service = CommitService::new(); + let root = head(&service, &realm); + let revision = commit( + &service, + &realm, + request( + &root, + "op-big", + vec![ + write("a.txt", "0123456789"), + write("b.txt", "0123456789"), + write("c.txt", "0123456789abcdefghij"), + ], + ), + ) + .unwrap() + .revision; + let paths: Vec = ["a.txt", "b.txt", "c.txt"] + .iter() + .map(|p| p.to_string()) + .collect(); + let found = service.read(&realm.repo(), &revision, &paths).unwrap(); + assert_eq!( + found + .files + .iter() + .map(|f| f.path.as_str()) + .collect::>(), + vec!["a.txt"] + ); + assert_eq!( + found.remaining, + vec!["b.txt".to_string(), "c.txt".to_string()] + ); + // A file over the ceiling on its own is still returned when it is + // the first asked for, so every path can be read. + let found = service + .read(&realm.repo(), &revision, &found.remaining[1..]) + .unwrap(); + assert_eq!(found.files[0].content.as_ref().map(Vec::len), Some(20)); + assert!(found.remaining.is_empty()); + } + + #[test] + fn read_refuses_invalid_paths() { + let realm = Realm::new("read-invalid"); + let service = CommitService::new(); + let root = head(&service, &realm); + for path in ["../x", "/abs", "a//b", ".git/config"] { + let result = service.read(&realm.repo(), &root, &[path.to_string()]); + assert!( + matches!(result, Err(CommitError::Invalid(_))), + "{path} was not refused" + ); + } + } } From eb116367aa9b29d3be8c781b20fd05aabb12afcb Mon Sep 17 00:00:00 2001 From: Luke Melia Date: Wed, 28 Oct 2026 12:48:38 -0400 Subject: [PATCH 2/2] Report a revision's own parent from /commit and /outcome A caller that journals revisions needs each one's real parent. A replay answered with only the revision left the caller to assume its parent was the HEAD it asked against, which is wrong once other commits have landed since. /commit and /outcome now answer with the revision's parent, and with whether the operation wrote that revision or found the tree unchanged, which a replay could not tell apart before. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AnysKbB8Y5FgNf4nQuAd6M --- packages/deckd/README.md | 17 +++- packages/deckd/src/history/commit.rs | 110 ++++++++++++++++++++----- packages/deckd/tests/commit_restart.rs | 8 +- 3 files changed, 111 insertions(+), 24 deletions(-) diff --git a/packages/deckd/README.md b/packages/deckd/README.md index e50799d..9d68440 100644 --- a/packages/deckd/README.md +++ b/packages/deckd/README.md @@ -171,12 +171,19 @@ Each commit's message ends with the caller's trailers, then ```json { "revision": "6f1c2d8e0a9b4c3d2e1f0a9b8c7d6e5f4a3b2c1d", + "parent": "0000000000000000000000000000000000000000", "tree": "9a3e1b7c5d2f4e6a8b0c1d3e5f7a9b1c3d5e7f9a", "created": true, "replayed": false } ``` +- `parent` is `revision`'s own parent, which on a replay may be older than + the request's `parent`. It is `null` only for the root commit. +- `created` is false when the changes left HEAD's tree as it was, and + `revision` is that HEAD. On a replay (`replayed: true`) it says whether the + operation's first commit wrote `revision` or found the tree unchanged. + - `operationId`: 1–128 characters from `[A-Za-z0-9._-]`, not starting with `.`. - `changes`: each entry has `path` plus exactly one of `contentBase64` or `delete: true`. A path appears at most once. Paths are relative and may not @@ -195,10 +202,16 @@ Each commit's message ends with the caller's trailers, then ``` ```json -{ "state": "committed", "revision": "6f1c2d8e0a9b4c3d2e1f0a9b8c7d6e5f4a3b2c1d" } +{ + "state": "committed", + "revision": "6f1c2d8e0a9b4c3d2e1f0a9b8c7d6e5f4a3b2c1d", + "parent": "0000000000000000000000000000000000000000", + "created": true +} ``` -or `{ "state": "absent" }`, after which the ID is fenced. It is a POST because +`parent` and `created` mean what they do in `/commit`'s answer. Or the answer +is `{ "state": "absent" }`, after which the ID is fenced. It is a POST because it can change state. ### `POST /head` diff --git a/packages/deckd/src/history/commit.rs b/packages/deckd/src/history/commit.rs index 8aa22e0..c636342 100644 --- a/packages/deckd/src/history/commit.rs +++ b/packages/deckd/src/history/commit.rs @@ -147,9 +147,12 @@ pub struct CommitRequest { #[derive(Debug, Clone, PartialEq, Eq, Serialize)] pub struct CommitOutcome { pub revision: String, + /// `revision`'s parent; `None` only for the root commit. + pub parent: Option, pub tree: String, /// False when the changes left the tree as it was and no revision was - /// written; `revision` is then the unchanged HEAD. + /// written; `revision` is then the unchanged HEAD. On a replay, whether + /// the operation's first commit wrote `revision`. pub created: bool, /// True when this operation ID had already committed and nothing new /// was written. @@ -159,7 +162,13 @@ pub struct CommitOutcome { #[derive(Debug, Clone, PartialEq, Eq, Serialize)] #[serde(tag = "state", rename_all = "camelCase")] pub enum OperationOutcome { - Committed { revision: String }, + /// `created` is false when the operation's changes left the tree as it + /// was, so `revision` is the HEAD it found rather than one it wrote. + Committed { + revision: String, + parent: Option, + created: bool, + }, Absent, } @@ -500,8 +509,9 @@ async fn commit_locked( let commit = get_commit(&repo, &revision)?; return Ok(CommitOutcome { revision, + parent: parent_of(&commit), tree: tree_hash(&commit), - created: false, + created: written_by(&commit, &request.operation_id), replayed: true, }); } @@ -545,6 +555,7 @@ async fn commit_locked( )?; return Ok(CommitOutcome { revision: head_hex, + parent: parent_of(&head), tree: tree_hash(&head), created: false, replayed: false, @@ -587,6 +598,7 @@ async fn commit_locked( )?; Ok(CommitOutcome { revision, + parent: Some(head_hex), tree: tree_hash(&commit), created: true, replayed: false, @@ -599,7 +611,7 @@ async fn outcome_locked( head_file: &mut File, operation_id: &str, ) -> CommitResult { - let (_repo, head) = settled_head(repo_dir, head_file).await?; + let (repo, head) = settled_head(repo_dir, head_file).await?; repair_head_record(repo_dir, &head)?; let record_path = operation_record_path(repo_dir, operation_id); @@ -607,7 +619,14 @@ async fn outcome_locked( return Ok(OperationOutcome::Absent); } match read_record(&record_path)? { - Record::Committed { revision, .. } => Ok(OperationOutcome::Committed { revision }), + Record::Committed { revision, .. } => { + let commit = get_commit(&repo, &revision)?; + Ok(OperationOutcome::Committed { + parent: parent_of(&commit), + created: written_by(&commit, operation_id), + revision, + }) + } Record::Fenced => Ok(OperationOutcome::Absent), Record::Pending(_) => { write_record(&record_path, &Record::Fenced)?; @@ -977,6 +996,19 @@ fn get_commit(repo: &Arc, revision: &str) -> CommitResult } } +/// An explicit commit's one parent, or `None` for the root commit. +fn parent_of(commit: &Commit) -> Option { + commit.parent_ids().first().map(|id| id.hex()) +} + +/// Whether `commit` is the one `operation_id` wrote, rather than the HEAD an +/// operation whose changes left the tree as it was found. +fn written_by(commit: &Commit, operation_id: &str) -> bool { + parse_description_trailers(commit.description()) + .into_iter() + .any(|t| t.key == OPERATION_ID_TRAILER && t.value == operation_id) +} + fn tree_hash(commit: &Commit) -> String { match commit.tree_ids().as_resolved() { Some(tree_id) => tree_id.hex(), @@ -1577,28 +1609,56 @@ mod tests { let root = head(&service, &realm); let req = request(&root, "op-1", vec![write("a.txt", "1")]); let first = commit(&service, &realm, req.clone()).unwrap(); + let later = commit( + &service, + &realm, + request(&first.revision, "op-2", vec![write("b.txt", "2")]), + ) + .unwrap(); - // The retry's parent is now stale; the replay still wins. + // The retry's parent is now stale; the replay still wins, and reports + // the revision's own parent rather than the current HEAD. let again = commit(&service, &realm, req).unwrap(); assert_eq!(again.revision, first.revision); assert_eq!(again.tree, first.tree); + assert_eq!(again.parent, Some(root.clone())); assert!(again.replayed); - assert!(!again.created); - assert_eq!(head(&service, &realm), first.revision); - let parent = pollster::block_on(async { - let repo = load_repo(&realm.repo()).await.unwrap(); - get_commit(&repo, &first.revision) - .unwrap() - .parent_ids() - .to_vec() - }); + assert!(again.created, "op-1 wrote the revision it replays"); + assert_eq!(head(&service, &realm), later.revision); assert_eq!( - parent.iter().map(|id| id.hex()).collect::>(), - vec![root] + outcome(&service, &realm, "op-1"), + OperationOutcome::Committed { + revision: first.revision, + parent: Some(root), + created: true, + } ); } + #[test] + fn replaying_an_operation_that_changed_nothing_says_it_wrote_nothing() { + let realm = Realm::new("idempotent-unchanged"); + let service = CommitService::new(); + let root = head(&service, &realm); + let first = commit( + &service, + &realm, + request(&root, "op-1", vec![write("a.txt", "1")]), + ) + .unwrap(); + let req = request(&first.revision, "op-2", vec![write("a.txt", "1")]); + let same = commit(&service, &realm, req.clone()).unwrap(); + assert!(!same.created); + + let again = commit(&service, &realm, req).unwrap(); + + assert!(again.replayed); + assert!(!again.created); + assert_eq!(again.revision, first.revision); + assert_eq!(again.parent, Some(root)); + } + #[test] fn absent_outcome_fences_the_operation_across_restarts() { let realm = Realm::new("fence"); @@ -1645,7 +1705,9 @@ mod tests { assert_eq!( outcome(&restarted, &realm, "op-1"), OperationOutcome::Committed { - revision: done.revision.clone() + revision: done.revision.clone(), + parent: Some(root.clone()), + created: true, } ); // Asking doesn't fence a committed operation; a retry still replays. @@ -1676,7 +1738,9 @@ mod tests { assert_eq!( outcome(&service, &realm, "op-1"), OperationOutcome::Committed { - revision: done.revision.clone() + revision: done.revision.clone(), + parent: Some(root.clone()), + created: true, } ); std::fs::remove_file(&record).unwrap(); @@ -2048,7 +2112,9 @@ mod tests { assert_eq!( outcome(&restarted, &realm, "op-crashed"), OperationOutcome::Committed { - revision: revision.clone() + revision: revision.clone(), + parent: Some(root.clone()), + created: true, } ); assert_eq!(head(&restarted, &realm), revision); @@ -2214,7 +2280,9 @@ mod tests { assert_eq!( outcome(&service, &realm, "op-2"), OperationOutcome::Committed { - revision: first.revision + revision: first.revision, + parent: first.parent, + created: false, } ); } diff --git a/packages/deckd/tests/commit_restart.rs b/packages/deckd/tests/commit_restart.rs index a63a744..14a18e3 100644 --- a/packages/deckd/tests/commit_restart.rs +++ b/packages/deckd/tests/commit_restart.rs @@ -42,11 +42,17 @@ fn committed_outcome_survives_a_restart() { assert_eq!(status, 200); assert_eq!( outcome, - json!({ "state": "committed", "revision": committed["revision"] }) + json!({ + "state": "committed", + "revision": committed["revision"], + "parent": root, + "created": true, + }) ); let (_, retried) = deckd.post("/commit", commit_body(&repo, &root, "op-1")); assert_eq!(retried["revision"], committed["revision"]); assert_eq!(retried["replayed"], json!(true)); + assert_eq!(retried["parent"], json!(root)); let (_, tree) = deckd.post( "/tree-hash", json!({ "repo": repo, "revision": committed["revision"] }),