diff --git a/lore-proto/proto/lore/thin_client/v1/model.proto b/lore-proto/proto/lore/thin_client/v1/model.proto index bb7a9045..8d35fb89 100644 --- a/lore-proto/proto/lore/thin_client/v1/model.proto +++ b/lore-proto/proto/lore/thin_client/v1/model.proto @@ -117,6 +117,23 @@ message DiffPartition { bytes link_partition = 2; } +// The revision that last modified a tree entry. Directories report the +// revision of the most recent change anywhere beneath them. +message TreeCommit { + // Content signature of the revision. + bytes signature = 1; + // Free-form commit message. + string commit_message = 2; + // Commit timestamp (Unix epoch milliseconds). + uint64 timestamp = 3; + // Resolved (branch, number) of the revision. Carried in full rather than as + // a bare number because the number is per-branch and this is not enough to + // describe its' provenance + lore.model.v1.RevisionIdentifier identifier = 4; + // Identity that committed the revision. + string committed_by = 5; +} + // A single entry in a revision tree listing. message TreeNode { // Repository-relative path of this entry. @@ -132,6 +149,10 @@ message TreeNode { // True when a link entry tracks its parent's branch; false for pinned links // and non-link entries. bool tracking = 6; + // Revision that last modified this entry, set only when the request asks + // for it. Absent wherever the server cannot attribute an entry - most + // commonly the repository root. + optional TreeCommit last_commit = 7; } // Self-describing revision record. Carries the resolved RevisionIdentifier diff --git a/lore-proto/proto/lore/thin_client/v1/thin_client.proto b/lore-proto/proto/lore/thin_client/v1/thin_client.proto index 417b6521..bd0a9a3f 100644 --- a/lore-proto/proto/lore/thin_client/v1/thin_client.proto +++ b/lore-proto/proto/lore/thin_client/v1/thin_client.proto @@ -124,6 +124,10 @@ message RevisionTreeRequest { // emits only direct children of the prefix root; 2 emits direct // children plus grandchildren; etc. 0 or unset means unbounded. optional uint32 max_depth = 4; + // If true, populate `TreeNode.last_commit`. Off by default. + // Note: Attribution costs one delta-block read per state plus one + // file-metadata-block read per entry. + bool include_last_commit = 5; } // Header for a RevisionTree stream. Echoes the resolved revision so diff --git a/lore-proto/src/grpc/lore.thin_client.v1.rs b/lore-proto/src/grpc/lore.thin_client.v1.rs index 9cf39ecc..644f94d1 100644 --- a/lore-proto/src/grpc/lore.thin_client.v1.rs +++ b/lore-proto/src/grpc/lore.thin_client.v1.rs @@ -104,6 +104,38 @@ impl ::prost::Name for DiffPartition { "/lore.thin_client.v1.DiffPartition".into() } } +/// The revision that last modified a tree entry. Directories report the +/// revision of the most recent change anywhere beneath them. +#[derive(Clone, PartialEq, Eq, Hash, ::prost::Message)] +pub struct TreeCommit { + /// Content signature of the revision. + #[prost(bytes = "bytes", tag = "1")] + pub signature: ::prost::bytes::Bytes, + /// Free-form commit message. + #[prost(string, tag = "2")] + pub commit_message: ::prost::alloc::string::String, + /// Commit timestamp (Unix epoch milliseconds). + #[prost(uint64, tag = "3")] + pub timestamp: u64, + /// Resolved (branch, number) of the revision. Carried in full rather than as + /// a bare number because the number is per-branch and this is not enough to + /// describe its' provenance + #[prost(message, optional, tag = "4")] + pub identifier: ::core::option::Option, + /// Identity that committed the revision. + #[prost(string, tag = "5")] + pub committed_by: ::prost::alloc::string::String, +} +impl ::prost::Name for TreeCommit { + const NAME: &'static str = "TreeCommit"; + const PACKAGE: &'static str = "lore.thin_client.v1"; + fn full_name() -> ::prost::alloc::string::String { + "lore.thin_client.v1.TreeCommit".into() + } + fn type_url() -> ::prost::alloc::string::String { + "/lore.thin_client.v1.TreeCommit".into() + } +} /// A single entry in a revision tree listing. #[derive(Clone, PartialEq, Eq, Hash, ::prost::Message)] pub struct TreeNode { @@ -126,6 +158,11 @@ pub struct TreeNode { /// and non-link entries. #[prost(bool, tag = "6")] pub tracking: bool, + /// Revision that last modified this entry, set only when the request asks + /// for it. Absent wherever the server cannot attribute an entry - most + /// commonly the repository root. + #[prost(message, optional, tag = "7")] + pub last_commit: ::core::option::Option, } impl ::prost::Name for TreeNode { const NAME: &'static str = "TreeNode"; @@ -714,6 +751,11 @@ pub struct RevisionTreeRequest { /// children plus grandchildren; etc. 0 or unset means unbounded. #[prost(uint32, optional, tag = "4")] pub max_depth: ::core::option::Option, + /// If true, populate `TreeNode.last_commit`. Off by default. + /// Note: Attribution costs one delta-block read per state plus one + /// file-metadata-block read per entry. + #[prost(bool, tag = "5")] + pub include_last_commit: bool, /// Revision specifier. #[prost(oneof = "revision_tree_request::Query", tags = "1, 2")] pub query: ::core::option::Option, diff --git a/lore-proto/tests/v1_thin_client.rs b/lore-proto/tests/v1_thin_client.rs index 8628a62d..e03f420b 100644 --- a/lore-proto/tests/v1_thin_client.rs +++ b/lore-proto/tests/v1_thin_client.rs @@ -23,6 +23,7 @@ use lore_proto::lore::thin_client::v1::RevisionInfoResponse; use lore_proto::lore::thin_client::v1::RevisionTreeHeader; use lore_proto::lore::thin_client::v1::RevisionTreeRequest; use lore_proto::lore::thin_client::v1::RevisionTreeResponse; +use lore_proto::lore::thin_client::v1::TreeCommit; use lore_proto::lore::thin_client::v1::TreeNode; use lore_proto::lore::thin_client::v1::content_diff_response::Payload as ContentDiffPayload; use lore_proto::lore::thin_client::v1::revision::Parent as RevisionParent; @@ -119,7 +120,15 @@ fn v1_thin_client_field_shapes() { size: _, mode: _, tracking: _, + last_commit: _, } = TreeNode::default(); + let TreeCommit { + signature: _, + commit_message: _, + timestamp: _, + identifier: _, + committed_by: _, + } = TreeCommit::default(); // Revision + nested Parent + Metadata let Revision { @@ -179,6 +188,7 @@ fn v1_thin_client_field_shapes() { query: _, path_prefix: _, max_depth: _, + include_last_commit: _, } = RevisionTreeRequest::default(); let _ = RevisionTreeQuery::Identifier(Default::default()); let _ = RevisionTreeQuery::Signature(Default::default()); diff --git a/lore-revision/src/revision.rs b/lore-revision/src/revision.rs index c3340c18..ee2a5c93 100644 --- a/lore-revision/src/revision.rs +++ b/lore-revision/src/revision.rs @@ -1125,12 +1125,18 @@ pub struct TreeResult { pub paths: Vec, } +/// Walk the tree at `revision`, optionally attributing each entry with the +/// revision that last modified it. +/// +/// `include_last_commit` populates [`TreePath::last_revision`] and +/// [`TreePath::last_revision_repository`]. Off by default. pub async fn tree( repository: Arc, revision: Hash, path: RelativePath, max_depth: usize, can_read: crate::state::CanReadRepository, + include_last_commit: bool, ) -> Result { lore_debug!( "Gathering tree in repository {} revision: {} path: {}", @@ -1139,7 +1145,15 @@ pub async fn tree( path.as_str() ); let state = State::deserialize(repository.clone(), revision).await?; - let paths = gather_tree_paths(state, repository, path, max_depth, can_read).await?; + let paths = gather_tree_paths( + state, + repository, + path, + max_depth, + can_read, + include_last_commit, + ) + .await?; Ok(TreeResult { paths }) } diff --git a/lore-revision/src/state.rs b/lore-revision/src/state.rs index 2b738f67..61ab00a7 100644 --- a/lore-revision/src/state.rs +++ b/lore-revision/src/state.rs @@ -5,6 +5,7 @@ pub mod dump; mod sink; use core::str; +use std::collections::HashSet; use std::future::Future; use std::io::Write; use std::mem::size_of; @@ -4665,6 +4666,91 @@ pub struct TreePath { /// True when a link node tracks its parent's branch; false for pinned /// links and all non-link nodes. pub tracking: bool, + /// Revision that last modified this entry, zero when not attributed. + /// Set only when the walk is asked for it, and zero even then wherever + /// an entry cannot be attributed - most commonly the repository root. + pub last_revision: Hash, + /// Repository the `last_revision` lives in. Equals the walked repository + /// for top-level entries; for entries inside a linked subtree it is the + /// linked repository, since the walker crosses link boundaries and each + /// side's revisions belong to their own state. Zero when `last_revision` + /// is zero. + pub last_revision_repository: RepositoryId, +} + +/// Per-state context for resolving [`TreePath::last_revision`]. If the walked +/// revision changed the entry, that is the answer. Otherwise it is +/// `revision[0]` from the entry's file-metadata record, or zero when the +/// entry has no metadata record. +/// +/// Bound to one `(state, repository)` pair. The walker crosses link +/// boundaries into linked repositories with their own state; each side of +/// the boundary needs its own `TreeAttribution`, built from the state that +/// side's entries came from. Mixing them would silently attribute against +/// the wrong state - `NodeID` is a plain `u32` index, so a foreign one +/// happily reads a valid-looking record and returns plausible garbage. +struct TreeAttribution { + /// Nodes the walked revision itself changed. + changed: HashSet, + /// The walked revision. + revision: Hash, + /// Repository the walked revision belongs to. + repository_id: RepositoryId, +} + +impl TreeAttribution { + async fn new(state: &State, repository: Arc) -> Result { + let repository_id = repository.id; + let delta_block = state + .delta_block(repository) + .await? + .to_aligned::(); + let changed = delta_block + .as_type_slice::() + .iter() + .map(|delta| delta.node) + .collect(); + Ok(Self { + changed, + revision: state.revision(), + repository_id, + }) + } + + fn repository_id(&self) -> RepositoryId { + self.repository_id + } + + /// Revision that last modified `node`. + /// + /// Reads slot 0 only. Slot 1 holds the other side of a merge, which + /// matters when walking back through both parents' histories but not + /// for naming the most recent change. + async fn last_revision( + &self, + state: &State, + repository: Arc, + node: NodeID, + ) -> Result { + if self.changed.contains(&node) { + return Ok(self.revision); + } + let metadata_node = node_to_file_metadata(node); + // Not `block_file_metadata`: it materialises a throwaway 64KB block + // when none exists. No block means no attribution, and a zero + // revision already expresses that. + let Some(block) = state + .try_block_file_metadata_existing( + repository, + NodeFileMetadataBlock::index(metadata_node), + ) + .await? + else { + return Ok(Hash::default()); + }; + let reader = block.read(); + Ok(reader.node(NodeFileMetadata::index(metadata_node)).revision[0]) + } } pub type CanReadRepository = Arc bool + Send + Sync>; @@ -4683,6 +4769,7 @@ pub async fn gather_tree_paths( path: RelativePath, max_depth: usize, can_read: CanReadRepository, + include_last_commit: bool, ) -> Result, StateError> { let (walk_state, walk_repository, parent_node_id) = if path.is_empty() { (state, repository, ROOT_NODE) @@ -4720,6 +4807,7 @@ pub async fn gather_tree_paths( 0, can_read, &mut paths, + include_last_commit, ) .await?; Ok(paths) @@ -4736,6 +4824,7 @@ async fn enumerate_children( link_depth: usize, can_read: CanReadRepository, result: &mut Vec, + include_last_commit: bool, ) -> Result<(), StateError> { let block_index = NodeBlock::index(parent_node_id); let node_index = Node::index(parent_node_id); @@ -4747,6 +4836,17 @@ async fn enumerate_children( } .into()); } + // Attribution is per-`enumerate_children` invocation so that each side of + // a link boundary gets its own context built from its own state. The + // walker calls `enumerate_children` at the top level and again on every + // link descent (see `gather_tree_paths_node` below). + let attribution = if include_last_commit { + Some(Arc::new( + TreeAttribution::new(&state, repository.clone()).await?, + )) + } else { + None + }; let mut cycle = SiblingCycleGuard::new(parent_node_id); gather_tree_paths_node_recurse( state, @@ -4760,6 +4860,8 @@ async fn enumerate_children( can_read, result, &mut cycle, + attribution, + include_last_commit, ) .await } @@ -4784,6 +4886,7 @@ fn log_linked_subtree_failure( } } +#[allow(clippy::too_many_arguments)] #[allow(clippy::too_many_arguments)] async fn gather_tree_paths_node( state: Arc, @@ -4797,6 +4900,8 @@ async fn gather_tree_paths_node( can_read: CanReadRepository, result: &mut Vec, cycle: &mut SiblingCycleGuard, + attribution: Option>, + include_last_commit: bool, ) -> Result, StateError> { let block_index = NodeBlock::index(node_id); let node_index = Node::index(node_id); @@ -4834,6 +4939,23 @@ async fn gather_tree_paths_node( } else { false }; + let (last_revision, last_revision_repository) = match attribution.as_ref() { + Some(attribution) => ( + attribution + .last_revision(&state, repository.clone(), node_id) + .await?, + attribution.repository_id(), + ), + None => (Hash::default(), RepositoryId::default()), + }; + // A zero last_revision has no meaningful repository. Keep the repository + // field zero in that case so downstream consumers do not have to remember + // the pairing rule. + let last_revision_repository = if last_revision.is_zero() { + RepositoryId::default() + } else { + last_revision_repository + }; result.push(TreePath { path: node_path.clone(), address, @@ -4841,6 +4963,8 @@ async fn gather_tree_paths_node( size: node.size, mode: node.mode as u64, tracking, + last_revision, + last_revision_repository, }); let depth_remaining = max_depth == 0 || depth + 1 < max_depth; @@ -4858,6 +4982,8 @@ async fn gather_tree_paths_node( can_read, result, &mut child_cycle, + attribution, + include_last_commit, ) .await?; } else if node.is_link() && depth_remaining && link_depth < MAX_LINK_DEPTH { @@ -4871,6 +4997,11 @@ async fn gather_tree_paths_node( let linked_repo = Arc::new(repository.to_link_context(link.repository).await); match State::deserialize(linked_repo.clone(), link.revision).await { Ok(linked_state) => { + // Attribution follows the walker across the link + // boundary. `enumerate_children` builds a fresh + // `TreeAttribution` from the linked state, so entries + // inside the subtree are attributed against their own + // repository's revisions (see `TreeAttribution` doc). if let Err(err) = enumerate_children( linked_state, linked_repo, @@ -4881,6 +5012,7 @@ async fn gather_tree_paths_node( link_depth + 1, can_read, result, + include_last_commit, ) .await { @@ -4920,6 +5052,8 @@ fn gather_tree_paths_node_recurse<'a>( can_read: CanReadRepository, result: &'a mut Vec, cycle: &'a mut SiblingCycleGuard, + attribution: Option>, + include_last_commit: bool, ) -> Pin> + Send + 'a>> { Box::pin(async move { let mut next = first_child; @@ -4936,6 +5070,8 @@ fn gather_tree_paths_node_recurse<'a>( can_read.clone(), result, cycle, + attribution.clone(), + include_last_commit, ) .await?; } diff --git a/lore-revision/tests/file_commit_history.rs b/lore-revision/tests/file_commit_history.rs new file mode 100644 index 00000000..0a49257b --- /dev/null +++ b/lore-revision/tests/file_commit_history.rs @@ -0,0 +1,740 @@ +// SPDX-FileCopyrightText: 2026 LoreLab.io +// SPDX-License-Identifier: MIT + +//! Pins down how per-entry change attribution is recorded, which is what +//! `TreeNode` last-commit attribution builds on. +//! +//! Findings, proved by this test: +//! +//! 1. `revision[0]` is a per-entry back-pointer, not a parent-revision stamp +//! 2. Directories propagate descendant changes +//! 3. The root node carries no attribution +//! 4. Last-touched is the tip when the entry appears in the tip's delta block, +//! `revision[0]` otherwise. No walking; the pairing `file::history` uses +//! +//! Four revisions are needed. With only r1 and r2 both files point at r1, hiding +//! the difference between a back-pointer and a parent stamp; r4 then covers an +//! entry that did not change at the tip. +//! +//! The observation dumps are commented out. To see the raw records, uncomment +//! them and run: +//! cargo test -p lore-revision --test file_commit_history -- --nocapture + +#[cfg(test)] +mod tests { + #![allow(clippy::disallowed_methods)] // Test fixture writes; not subject to repository write-token discipline. + + use std::sync::Arc; + + use lore_base::runtime::LORE_CONTEXT; + use lore_base::runtime::runtime; + use lore_base::types::Address; + use lore_base::types::BranchId; + use lore_base::types::Context; + use lore_base::types::Hash; + use lore_revision::branch; + use lore_revision::commit::commit_in_memory_revision; + use lore_revision::metadata::Metadata; + use lore_revision::node::*; + use lore_revision::repository::InMemoryContext; + use lore_revision::repository::RepositoryContext; + use lore_revision::repository::RepositoryWriteToken; + use lore_revision::revision::tree; + use lore_revision::state::State; + use lore_revision::state::allow_all_repositories; + use lore_revision::util::path::RelativePath; + use lore_storage::hash::hash_string; + use lore_storage::local::immutable_store::LocalImmutableStore; + + include!("helper.rs"); + + struct InMemoryMarker; + impl InMemoryContext for InMemoryMarker {} + const IN_MEMORY_MARKER: InMemoryMarker = InMemoryMarker; + + /// Path-less context, matching the shape the in-memory revision tree builds on. + async fn test_repository( + mutable_store: Arc, + ) -> Arc { + let immutable_store = LocalImmutableStore::new( + None, + lore_storage::local::immutable_store::ImmutableStoreSettings::default(), + ) + .await + .expect("Failed to create store"); + Arc::new( + RepositoryContext::new(default_repository_creation_args( + immutable_store, + mutable_store, + )) + .with_write_token(RepositoryWriteToken::in_memory(&IN_MEMORY_MARKER)), + ) + } + + fn file(name: &str, content: u64) -> Node { + Node { + flags: NodeFlags::File.bits(), + mode: 0o644, + size: 10, + address: Address { + hash: Hash::from_u64(content), + context: Context::from(uuid::Uuid::now_v7()), + }, + name_hash: hash_string(name), + ..Default::default() + } + } + + fn directory(name: &str) -> Node { + Node { + flags: NodeFlags::NoFlags.bits(), + mode: 0o755, + name_hash: hash_string(name), + ..Default::default() + } + } + + async fn add( + state: &State, + repository: Arc, + parent: NodeID, + node: Node, + name: &str, + ) -> NodeID { + let node_id = state + .node_add(repository.clone(), parent, node, name) + .await + .expect("adding the node must succeed"); + state + .node_mark_staged( + repository, + node_id, + NodeFlags::StagedAdd, + NodeFlags::DirtyAdd, + ) + .await + .expect("marking the addition must succeed"); + node_id + } + + fn metadata_on(branch: BranchId) -> Metadata { + let mut metadata = Metadata::new(); + metadata + .set_branch(branch) + .expect("setting the branch must succeed"); + metadata + } + + fn token() -> RepositoryWriteToken { + RepositoryWriteToken::in_memory(&IN_MEMORY_MARKER) + } + + fn branch_id() -> BranchId { + Context::from(uuid::Uuid::now_v7()) + } + + /// Slot 0 of a node's file-metadata record. Slot 1 carries the other side of + /// a merge, which this test does not exercise + /// + /// Note: `node` and `action` are only read if the println block at the end + /// of the test is enabled + #[allow(dead_code)] + struct FileMetadataRecord { + revision: Hash, + node: u32, + action: u16, + } + + /// The file-metadata record for a node, read as `file::history` does + async fn file_metadata_of( + state: &State, + repository: Arc, + node_id: NodeID, + ) -> FileMetadataRecord { + let metadata_node_id = node_to_file_metadata(node_id); + let block_index = NodeFileMetadataBlock::index(metadata_node_id); + let node_index = NodeFileMetadata::index(metadata_node_id); + let block = state + .block_file_metadata(repository, block_index) + .await + .expect("the file-metadata block must read back"); + let reader = block.read(); + let record = reader.node(node_index); + FileMetadataRecord { + revision: record.revision[0], + node: record.node[0], + action: record.action[0], + } + } + + async fn node_id_for(state: &State, repository: Arc, path: &str) -> NodeID { + state + .find_node_link(repository, path) + .await + .unwrap_or_else(|err| panic!("path {path} must resolve at this revision: {err:?}")) + .node + } + + /// r1 adds both files, r2 and r3 modify `touched.bin`, r4 modifies + /// `untouched.bin`. Reading the records at each tip separates a per-entry + /// back-pointer from a parent-revision stamp + #[tokio::test] + async fn file_metadata_revision_attribution_at_the_tip() { + let (_immutable, mutable, execution) = + test_store_create().await.expect("Failed to create stores"); + runtime() + .spawn(LORE_CONTEXT.scope(execution, async move { + let repository = test_repository(mutable).await; + let branch = branch_id(); + + // r1: add a/touched.bin and a/untouched.bin + let staged = Arc::new(State::new()); + let dir = add(&staged, repository.clone(), ROOT_NODE, directory("a"), "a").await; + add( + &staged, + repository.clone(), + dir, + file("touched.bin", 0x11), + "touched.bin", + ) + .await; + add( + &staged, + repository.clone(), + dir, + file("untouched.bin", 0x22), + "untouched.bin", + ) + .await; + + let r1 = commit_in_memory_revision( + repository.clone(), + &token(), + staged, + metadata_on(branch), + Hash::default(), + branch, + ) + .await + .expect("committing r1 must succeed"); + + // r2: modify a/touched.bin only + let staged2 = State::deserialize(repository.clone(), r1) + .await + .expect("r1 must deserialize"); + let touched_id = node_id_for(&staged2, repository.clone(), "a/touched.bin").await; + staged2 + .node_modify( + repository.clone(), + touched_id, + 0o644, + 4096, + Address { + hash: Hash::from_u64(0x33), + context: Context::default(), + }, + ) + .await + .expect("modifying the file must succeed"); + staged2 + .node_mark_staged( + repository.clone(), + touched_id, + NodeFlags::StagedModify, + NodeFlags::DirtyModify, + ) + .await + .expect("marking the modification must succeed"); + + let r2 = commit_in_memory_revision( + repository.clone(), + &token(), + staged2, + metadata_on(branch), + r1, + branch, + ) + .await + .expect("committing r2 must succeed"); + + assert_ne!(r1, r2, "the two commits must be distinct revisions"); + + // r3: Modify a/touched.bin again so we diverge + let staged3 = State::deserialize(repository.clone(), r2) + .await + .expect("r2 must deserialize"); + let touched_id3 = node_id_for(&staged3, repository.clone(), "a/touched.bin").await; + staged3 + .node_modify( + repository.clone(), + touched_id3, + 0o644, + 8192, + Address { + hash: Hash::from_u64(0x44), + context: Context::default(), + }, + ) + .await + .expect("modifying the file again must succeed"); + staged3 + .node_mark_staged( + repository.clone(), + touched_id3, + NodeFlags::StagedModify, + NodeFlags::DirtyModify, + ) + .await + .expect("marking the second modification must succeed"); + + let r3 = commit_in_memory_revision( + repository.clone(), + &token(), + staged3, + metadata_on(branch), + r2, + branch, + ) + .await + .expect("committing r3 must succeed"); + + assert_ne!(r2, r3, "r3 must be a distinct revision"); + assert_ne!(r1, r3, "r3 must also differ from r1"); + assert_eq!( + branch::load_latest(repository.clone(), branch) + .await + .expect("the branch tip must read back"), + r3, + "the branch tip must be r3" + ); + + // Observe the metadata records at the tip + let tip = State::deserialize(repository.clone(), r3) + .await + .expect("r3 must deserialize"); + + let touched = node_id_for(&tip, repository.clone(), "a/touched.bin").await; + let untouched = node_id_for(&tip, repository.clone(), "a/untouched.bin").await; + let dir_at_tip = node_id_for(&tip, repository.clone(), "a").await; + + let touched_meta = file_metadata_of(&tip, repository.clone(), touched).await; + let untouched_meta = file_metadata_of(&tip, repository.clone(), untouched).await; + let dir_meta = file_metadata_of(&tip, repository.clone(), dir_at_tip).await; + let root_meta = file_metadata_of(&tip, repository.clone(), ROOT_NODE).await; + + // Without the modification, every metadata reading is pointless + let touched_node = tip + .node(repository.clone(), touched) + .await + .expect("the touched node must read back"); + let untouched_node = tip + .node(repository.clone(), untouched) + .await + .expect("the untouched node must read back"); + + assert_eq!( + touched_node.size, 8192, + "r3's modification must be in the committed tree" + ); + assert_eq!(untouched_node.size, 10, "untouched.bin must be untouched"); + + /* + println!("--- file_commit_history observations ---"); + println!("r1 (add both) = {r1}"); + println!("r2 (modify touched) = {r2}"); + println!("r3 (modify touched) = {r3}"); + println!( + "a/touched.bin node size={} hash={} (expect size=8192 hash=..44 if r3 applied)", + touched_node.size, touched_node.address.hash + ); + println!( + "a/untouched.bin node size={} hash={}", + untouched_node.size, untouched_node.address.hash + ); + println!( + "a/touched.bin revision[0]={} node[0]={} action[0]={}", + touched_meta.revision, touched_meta.node, touched_meta.action + ); + println!( + "a/untouched.bin revision[0]={} node[0]={} action[0]={}", + untouched_meta.revision, untouched_meta.node, untouched_meta.action + ); + println!( + "a (directory) revision[0]={} node[0]={} action[0]={}", + dir_meta.revision, dir_meta.node, dir_meta.action + ); + println!( + "root revision[0]={} node[0]={} action[0]={}", + root_meta.revision, root_meta.node, root_meta.action + ); + println!("--- end observations ---"); + */ + + // A failure below means Lore's attribution semantics have changed + assert_eq!( + touched_meta.revision, r2, + "touched.bin changed in r3, so it must back-point at r2" + ); + assert_eq!( + untouched_meta.revision, r1, + "untouched.bin is unchanged since r1, so it must still point at r1" + ); + // Directories propagate, so folder rows can be attributed directly + assert_eq!( + dir_meta.revision, r2, + "directory 'a' must move with its changed child, not stay at r1" + ); + assert!( + root_meta.revision.is_zero(), + "the root node carries no attribution" + ); + + // r4: Modify untouched.bin only, so touched.bin is not changed at the tip + let staged4 = State::deserialize(repository.clone(), r3) + .await + .expect("r3 must deserialize"); + let untouched_id4 = + node_id_for(&staged4, repository.clone(), "a/untouched.bin").await; + staged4 + .node_modify( + repository.clone(), + untouched_id4, + 0o644, + 2048, + Address { + hash: Hash::from_u64(0x55), + context: Context::default(), + }, + ) + .await + .expect("modifying untouched.bin must succeed"); + staged4 + .node_mark_staged( + repository.clone(), + untouched_id4, + NodeFlags::StagedModify, + NodeFlags::DirtyModify, + ) + .await + .expect("marking the r4 modification must succeed"); + + let r4 = commit_in_memory_revision( + repository.clone(), + &token(), + staged4, + metadata_on(branch), + r3, + branch, + ) + .await + .expect("committing r4 must succeed"); + + let tip4 = State::deserialize(repository.clone(), r4) + .await + .expect("r4 must deserialize"); + let touched4 = node_id_for(&tip4, repository.clone(), "a/touched.bin").await; + let touched_meta4 = file_metadata_of(&tip4, repository.clone(), touched4).await; + + /* + println!("--- r4 probe ---"); + println!("r3 = {r3}"); + println!("r4 = {r4}"); + println!( + "a/touched.bin at r4 revision[0]={} action[0]={} (r3 => reachable, r2 => stale)", + touched_meta4.revision, touched_meta4.action + ); + println!("--- end r4 probe ---"); + */ + + assert_eq!( + touched_meta4.revision, r3, + "at r4, touched.bin must still resolve to its last change (r3)" + ); + })) + .await + .expect("Task failed"); + } + + /// Attribution follows the walker across link boundaries. `TreeAttribution` is + /// built per `enumerate_children`, so each side of the boundary attributes + /// against its own state. The linked-subtree entries report the linked + /// repository's revisions - not the walked repository's. + #[tokio::test] + async fn tree_attributes_across_a_link() { + let (_immutable, mutable, execution) = + test_store_create().await.expect("Failed to create stores"); + runtime() + .spawn(LORE_CONTEXT.scope(execution, async move { + let repository = test_repository(mutable).await; + + // The link target: its own repository, sharing this one's stores + // and write token, holding a single file at its root. + let target_id = Context::from(uuid::Uuid::now_v7()).into(); + let target = Arc::new(repository.to_link_context(target_id).await); + let target_branch = branch_id(); + let target_staged = Arc::new(State::new()); + add( + &target_staged, + target.clone(), + ROOT_NODE, + file("inner.bin", 0x77), + "inner.bin", + ) + .await; + let target_revision = commit_in_memory_revision( + target.clone(), + &token(), + target_staged, + metadata_on(target_branch), + Hash::default(), + target_branch, + ) + .await + .expect("committing the link target must succeed"); + + // The walked repository: one ordinary file, plus a link at the + // root pointing into the target's root. + let branch = branch_id(); + let staged = Arc::new(State::new()); + add( + &staged, + repository.clone(), + ROOT_NODE, + file("top.bin", 0x88), + "top.bin", + ) + .await; + let link = Node { + flags: NodeFlags::Link.bits(), + mode: 0o755, + name_hash: hash_string("vendor"), + child: ROOT_NODE, + address: Address { + hash: target_revision, + context: target_id.into(), + }, + ..Default::default() + }; + add(&staged, repository.clone(), ROOT_NODE, link, "vendor").await; + let revision = commit_in_memory_revision( + repository.clone(), + &token(), + staged, + metadata_on(branch), + Hash::default(), + branch, + ) + .await + .expect("committing the linking revision must succeed"); + + let paths = tree( + repository.clone(), + revision, + RelativePath::default(), + 0, + allow_all_repositories(), + true, + ) + .await + .expect("the tree walk must succeed") + .paths; + + let entry_for = |name: &str| { + paths + .iter() + .find(|entry| entry.path.as_str() == name) + .unwrap_or_else(|| { + panic!( + "{name} must appear in the walk, got {:?}", + paths.iter().map(|e| e.path.as_str()).collect::>() + ) + }) + }; + + let top = entry_for("top.bin"); + let vendor = entry_for("vendor"); + let inner = entry_for("vendor/inner.bin"); + + assert_eq!( + top.last_revision, revision, + "an ordinary entry is attributed against the walked revision" + ); + assert_eq!( + top.last_revision_repository, repository.id, + "top-level attribution names the walked repository" + ); + + assert_eq!( + vendor.last_revision, revision, + "the link node itself lives in the walked state, so it is attributed \ + against the walked revision" + ); + assert_eq!( + vendor.last_revision_repository, repository.id, + "the link node's attribution names the walked repository" + ); + + // The linked subtree's entries live in the linked state. The + // walker now crosses the boundary, so we get real attribution + // - the linked repository's revision, in the linked repository. + assert_eq!( + inner.last_revision, target_revision, + "content behind a link is attributed against the linked state's revision" + ); + assert_eq!( + inner.last_revision_repository, target_id, + "linked-subtree attribution names the linked repository, not the walked one" + ); + })) + .await + .expect("Task failed"); + } + + /// `revision::tree` applies the rules above. Entries changed at the walked + /// revision report it, the rest report their own last change (off by default). + #[tokio::test] + async fn tree_attributes_with_last_commit() { + let (_immutable, mutable, execution) = + test_store_create().await.expect("Failed to create stores"); + runtime() + .spawn(LORE_CONTEXT.scope(execution, async move { + let repository = test_repository(mutable).await; + let branch = branch_id(); + + // r1 adds both files, r2 modifies only touched.bin + let staged = Arc::new(State::new()); + let dir = add(&staged, repository.clone(), ROOT_NODE, directory("a"), "a").await; + add( + &staged, + repository.clone(), + dir, + file("touched.bin", 0x11), + "touched.bin", + ) + .await; + add( + &staged, + repository.clone(), + dir, + file("untouched.bin", 0x22), + "untouched.bin", + ) + .await; + let r1 = commit_in_memory_revision( + repository.clone(), + &token(), + staged, + metadata_on(branch), + Hash::default(), + branch, + ) + .await + .expect("committing r1 must succeed"); + + let staged2 = State::deserialize(repository.clone(), r1) + .await + .expect("r1 must deserialize"); + let touched_id = node_id_for(&staged2, repository.clone(), "a/touched.bin").await; + staged2 + .node_modify( + repository.clone(), + touched_id, + 0o644, + 4096, + Address { + hash: Hash::from_u64(0x33), + context: Context::default(), + }, + ) + .await + .expect("modifying the file must succeed"); + staged2 + .node_mark_staged( + repository.clone(), + touched_id, + NodeFlags::StagedModify, + NodeFlags::DirtyModify, + ) + .await + .expect("marking the modification must succeed"); + let r2 = commit_in_memory_revision( + repository.clone(), + &token(), + staged2, + metadata_on(branch), + r1, + branch, + ) + .await + .expect("committing r2 must succeed"); + + let walk_depth = async |max_depth, include_last_commit| { + tree( + repository.clone(), + r2, + RelativePath::default(), + max_depth, + allow_all_repositories(), + include_last_commit, + ) + .await + .expect("the tree walk must succeed") + .paths + }; + // 0 = unbounded + let walk = async |include_last_commit| walk_depth(0, include_last_commit).await; + + // Off: nothing attributed. `last_revision_repository` also stays + // zero - a zero revision has no meaningful repository. + for entry in walk(false).await { + assert!( + entry.last_revision.is_zero(), + "{} must be unattributed when the walk is not asked", + entry.path + ); + assert!( + entry.last_revision_repository.is_zero(), + "{} must carry a zero repository when unattributed", + entry.path + ); + } + + let attributed = walk(true).await; + let entry_for = |name: &str| { + attributed + .iter() + .find(|entry| entry.path.as_str() == name) + .unwrap_or_else(|| panic!("{name} must appear in the walk")) + }; + let of = |name: &str| entry_for(name).last_revision; + let repo_of = |name: &str| entry_for(name).last_revision_repository; + + assert_eq!(of("a/touched.bin"), r2, "changed at the walked revision"); + assert_eq!(of("a/untouched.bin"), r1, "unchanged since r1"); + assert_eq!(of("a"), r2, "a directory follows its changed descendant"); + + // Attribution stays within the walked repository - no links here, + // so every entry names the same repository. + assert_eq!(repo_of("a/touched.bin"), repository.id); + assert_eq!(repo_of("a/untouched.bin"), repository.id); + assert_eq!(repo_of("a"), repository.id); + + // Depth 1 is what a single-level directory listing uses, so it is + // worth asserting directly rather than inferring from the + // unbounded walk: only `a` is reached, and it is still attributed. + let shallow = walk_depth(1, true).await; + let names: Vec<&str> = shallow.iter().map(|e| e.path.as_str()).collect(); + assert_eq!(names, ["a"], "depth 1 from root reaches only the directory"); + assert_eq!( + shallow[0].last_revision, r2, + "attribution applies at depth 1, not only on an unbounded walk" + ); + assert_eq!( + shallow[0].last_revision_repository, repository.id, + "attribution carries the repository at depth 1 too" + ); + })) + .await + .expect("Task failed"); + } +} diff --git a/lore-server/src/grpc/handlers/revision_tree.rs b/lore-server/src/grpc/handlers/revision_tree.rs index 9f125488..616a56af 100644 --- a/lore-server/src/grpc/handlers/revision_tree.rs +++ b/lore-server/src/grpc/handlers/revision_tree.rs @@ -60,7 +60,8 @@ pub async fn handler( LORE_CONTEXT .scope(execution, async move { - tree(repository.clone(), revision, path, max_depth, can_read) + // No attribution: this legacy message has no field to carry it. + tree(repository.clone(), revision, path, max_depth, can_read, false) .await .map(|result| { debug!("Got tree"); diff --git a/lore-server/src/grpc/thinclient/v1/helpers.rs b/lore-server/src/grpc/thinclient/v1/helpers.rs index 58b482da..c589cf3c 100644 --- a/lore-server/src/grpc/thinclient/v1/helpers.rs +++ b/lore-server/src/grpc/thinclient/v1/helpers.rs @@ -318,6 +318,84 @@ pub(super) async fn diff_conflict_from_pair( } } +/// Load the `TreeCommit` describing `signature` from `repository`. +/// +/// Deliberately not `load_revision`: that also resolves both parents, two +/// loads per revision a tree listing has no use for. +/// +/// Attribution is decoration on a listing, so a revision that fails to load +/// is warned about and reported as `None` rather than failing the whole walk. +/// +/// `repository` must be the context the revision belongs to. When the tree +/// walk crossed a link boundary the linked repository's revisions live in +/// the linked context, not the walked one - the caller resolves that pairing +/// via `TreePath::last_revision_repository` before calling in here. +pub(super) async fn load_tree_commit( + repository: &Arc, + signature: Hash, +) -> Option { + let state = State::deserialize(repository.clone(), signature) + .await + .inspect_err(|err| { + warn!( + {REPOSITORY_ID} = %repository.id, {REVISION} = %signature, ?err, + "Skipping tree attribution: revision state did not load", + ); + }) + .ok()?; + let metadata_hash = state.metadata_hash(); + let metadata = Metadata::deserialize(repository.clone(), metadata_hash) + .await + .inspect_err(|err| { + warn!( + {REPOSITORY_ID} = %repository.id, {REVISION} = %signature, + {METADATA} = %metadata_hash, ?err, + "Skipping tree attribution: revision metadata did not load", + ); + }) + .ok()?; + + // A revision whose metadata names no branch still has a usable message + // and timestamp, which are what a listing displays. Fall back to a zero + // branch rather than dropping the whole record for a missing provenance + // detail. + let branch_id = metadata.get_branch().unwrap_or_else(|err| { + warn!( + {REPOSITORY_ID} = %repository.id, {REVISION} = %signature, ?err, + "Tree attribution: revision metadata names no branch", + ); + BranchId::default() + }); + + let mut commit = thin_client_v1::TreeCommit { + signature: signature.into(), + identifier: Some(model_v1::RevisionIdentifier { + branch_id: branch_id.into(), + number: state.revision_number(), + }), + ..Default::default() + }; + metadata.walk(|key, value, _value_type| { + let Ok(key) = std::str::from_utf8(key) else { + return; + }; + match key { + lore_revision::metadata::MESSAGE => { + commit.commit_message = String::from_utf8_lossy(value).into_owned(); + } + lore_revision::metadata::TIMESTAMP if value.len() == std::mem::size_of::() => { + commit.timestamp = u64::from_le_bytes(value.try_into().unwrap_or_default()); + } + lore_revision::metadata::COMMITTED_BY => { + commit.committed_by = String::from_utf8_lossy(value).into_owned(); + } + _ => {} + } + }); + + Some(commit) +} + #[cfg(test)] mod tests { use std::str::FromStr; diff --git a/lore-server/src/grpc/thinclient/v1/revision_tree.rs b/lore-server/src/grpc/thinclient/v1/revision_tree.rs index 0b456103..cebd6cc4 100644 --- a/lore-server/src/grpc/thinclient/v1/revision_tree.rs +++ b/lore-server/src/grpc/thinclient/v1/revision_tree.rs @@ -1,5 +1,7 @@ // SPDX-FileCopyrightText: 2026 Epic Games, Inc. // SPDX-License-Identifier: MIT +use std::collections::HashMap; +use std::collections::hash_map; use std::pin::Pin; use std::sync::Arc; @@ -11,6 +13,7 @@ use lore_proto::lore::thin_client::v1 as thin_client_v1; use lore_proto::lore::thin_client::v1::RevisionTreeRequest; use lore_proto::lore::thin_client::v1::RevisionTreeResponse; use lore_proto::lore::thin_client::v1::revision_tree_response::Payload; +use lore_revision::lore::RepositoryId; use lore_revision::repository::RepositoryContext; use lore_revision::revision::tree; use lore_revision::util::path::RelativePath; @@ -26,6 +29,7 @@ use tracing::Instrument; use tracing::debug; use tracing::warn; +use super::helpers::load_tree_commit; use super::helpers::node_flags_to_node_type; use super::helpers::resolve_to_identifier; use crate::grpc::extract_correlation_id; @@ -71,6 +75,7 @@ pub async fn handler( _ => RelativePath::new(), }; let max_depth = req.max_depth.map_or(usize::MAX, |d| d as usize); + let include_last_commit = req.include_last_commit; let execution = setup_execution(module_path!(), correlation_id, user_id); let repository = Arc::new(RepositoryContext::new_server_context( @@ -100,7 +105,17 @@ pub async fn handler( lore_spawn!( async move { - stream_tree(repository, signature, path, max_depth, can_read, header, tx).await; + stream_tree( + repository, + signature, + path, + max_depth, + can_read, + header, + tx, + include_last_commit, + ) + .await; } .in_current_span() ); @@ -120,6 +135,7 @@ async fn stream_tree( can_read: lore_revision::state::CanReadRepository, header: thin_client_v1::RevisionTreeHeader, tx: mpsc::Sender>, + include_last_commit: bool, ) { // Emit header first. If the client has already dropped, just bail. if tx @@ -133,7 +149,16 @@ async fn stream_tree( return; } - let result = match tree(repository.clone(), signature, path, max_depth, can_read).await { + let result = match tree( + repository.clone(), + signature, + path, + max_depth, + can_read, + include_last_commit, + ) + .await + { Ok(result) => result, Err(err) => { let status = if err.is_invalid_path() { @@ -152,19 +177,43 @@ async fn stream_tree( } }; + // Entries commonly share a last-touching revision, so resolve each + // distinct one once rather than per entry. The key is + // (repository, signature) rather than just signature because the walker + // crosses link boundaries. An entry inside a linked subtree + // reports the linked repository, and its revisions live in that + // repository's context, not the walked one. + // + // Empty when attribution was not asked for, since every `last_revision` + // is then zero. + let mut commits: HashMap<(RepositoryId, Hash), thin_client_v1::TreeCommit> = HashMap::new(); + for key in result + .paths + .iter() + .map(|tree_path| (tree_path.last_revision_repository, tree_path.last_revision)) + .filter(|(_, last_revision)| !last_revision.is_zero()) + { + let hash_map::Entry::Vacant(entry) = commits.entry(key) else { + continue; + }; + // Reuse the walked context when the revision belongs to the walked + // repository; build a linked context only when it does not. Building + // one for the common case would pay `to_link_context` every time. + let (repository_id, last_revision) = key; + let resolved = if repository_id == repository.id { + load_tree_commit(&repository, last_revision).await + } else { + let linked = Arc::new(repository.to_link_context(repository_id).await); + load_tree_commit(&linked, last_revision).await + }; + if let Some(commit) = resolved { + entry.insert(commit); + } + } + let mut emitted: u64 = 0; for tree_path in result.paths { - let node = thin_client_v1::TreeNode { - path: tree_path.path.to_string(), - node_type: node_flags_to_node_type(tree_path.flags) as i32, - address: tree_path.address.map(|address| model_v1::Address { - hash: address.hash.into(), - context: address.context.into(), - }), - size: tree_path.size, - mode: tree_path.mode, - tracking: tree_path.tracking, - }; + let node = tree_node(&tree_path, &commits); if tx .send(Ok(RevisionTreeResponse { payload: Some(Payload::Node(node)), @@ -181,6 +230,32 @@ async fn stream_tree( debug!(emitted, "RevisionTree complete"); } +/// Project one walked entry onto the wire, attaching its attribution. +/// +/// `commits` is keyed by (repository, signature), so entries sharing a +/// last-touching revision share one record. A zero `last_revision` +/// (attribution not requested, or an entry the walk declined to attribute) +/// finds nothing and stays `None`. +fn tree_node( + tree_path: &lore_revision::state::TreePath, + commits: &HashMap<(RepositoryId, Hash), thin_client_v1::TreeCommit>, +) -> thin_client_v1::TreeNode { + thin_client_v1::TreeNode { + path: tree_path.path.to_string(), + node_type: node_flags_to_node_type(tree_path.flags) as i32, + address: tree_path.address.map(|address| model_v1::Address { + hash: address.hash.into(), + context: address.context.into(), + }), + size: tree_path.size, + mode: tree_path.mode, + tracking: tree_path.tracking, + last_commit: commits + .get(&(tree_path.last_revision_repository, tree_path.last_revision)) + .cloned(), + } +} + #[cfg(test)] mod test { use lore_base::runtime::LORE_CONTEXT; @@ -216,6 +291,7 @@ mod test { query: Some(query), path_prefix, max_depth, + include_last_commit: false, }); request.metadata_mut().insert_bin( REPOSITORY_ID_KEY, @@ -224,6 +300,17 @@ mod test { request } + /// `make_request`, with attribution asked for. + fn make_attributed_request( + repository: RepositoryId, + query: Query, + max_depth: Option, + ) -> Request { + let mut request = make_request(repository, query, None, max_depth); + request.get_mut().include_last_commit = true; + request + } + /// Pushes a fresh branch and one revision for each of `revisions`; each /// revision's state contains `revisions[i]` as File nodes at the root. /// Returns the branch id and the revision signatures in push order. @@ -501,6 +588,7 @@ mod test { query: None, path_prefix: None, max_depth: None, + include_last_commit: false, }); request.metadata_mut().insert_bin( REPOSITORY_ID_KEY, @@ -637,8 +725,8 @@ mod test { let response = handler( make_request(repository, Query::Signature(signature.into()), None, None), - immutable_store, - mutable_store, + immutable_store.clone(), + mutable_store.clone(), ) .await .expect("handler ok"); @@ -650,6 +738,27 @@ mod test { .collect(); assert_eq!(items.len(), 1); assert!(matches!(items[0].payload, Some(Payload::Header(_)))); + + // Same revision, this time asking for attribution. A freshly + // created repository is empty, so this is the first thing a UI + // requesting `include_last_commit` will hit: building the + // attribution context reads the delta block, and an empty + // revision may not have one. + let attributed = handler( + make_attributed_request(repository, Query::Signature(signature.into()), None), + immutable_store, + mutable_store, + ) + .await + .expect("attribution must not fail on an empty revision"); + + let items: Vec<_> = collect(attributed) + .await + .into_iter() + .map(|r| r.expect("attribution must not error mid-stream")) + .collect(); + assert_eq!(items.len(), 1, "still header-only"); + assert!(matches!(items[0].payload, Some(Payload::Header(_)))); })) .await; } @@ -1546,4 +1655,218 @@ mod test { })) .await; } + + /// `include_last_commit` gates the field: absent unless asked for. + /// Does not change which nodes the walk emits. + #[tokio::test] + async fn last_commit_is_gated_by_the_request_flag() { + let repository = random::(); + let (immutable_store, mutable_store, execution) = + test_store_create().await.expect("test stores"); + + Box::pin(LORE_CONTEXT.scope(execution, async move { + let repository_context = Arc::new(RepositoryContext::new_server_context( + immutable_store.clone(), + mutable_store.clone(), + repository, + )); + let (_branch, signatures) = + push_branch_with_revisions(&repository_context, &[&["top.txt"]]).await; + let tip = *signatures.last().expect("one revision"); + + let nodes = async |request| { + collect( + handler(request, immutable_store.clone(), mutable_store.clone()) + .await + .expect("handler ok"), + ) + .await + .into_iter() + .map(|r| r.expect("stream item")) + .filter_map(|item| match item.payload { + Some(Payload::Node(node)) => Some(node), + _ => None, + }) + .collect::>() + }; + + let unattributed = nodes(make_request( + repository, + Query::Signature(tip.into()), + None, + None, + )) + .await; + assert!(!unattributed.is_empty(), "the walk must emit nodes"); + assert!( + unattributed.iter().all(|node| node.last_commit.is_none()), + "last_commit must be absent when not requested", + ); + + // Asking for attribution must not break the walk. These fixtures + // serialize states directly rather than committing, so no delta + // block or file-metadata records exist to attribute against and + // every entry stays unattributed. Attribution correctness is + // proved in `lore-revision`'s `tree_attributes_with_last_commit` + // and `tree_attributes_across_a_link`; what matters here is that + // the request flag is honoured and the stream still emits the + // same nodes. + let attributed = nodes(make_attributed_request( + repository, + Query::Signature(tip.into()), + None, + )) + .await; + assert_eq!( + attributed.len(), + unattributed.len(), + "attribution must not change which nodes are emitted", + ); + })) + .await; + } + + /// Each entry must receive its **own** attribution, not a neighbour's. + /// This is the seam between attribution (proved in `lore-revision`) and + /// resolution (proved by `load_tree_commit_reads_the_revision_record`), + /// and it needs no store: hand-built entries and a hand-built map are + /// enough. + #[tokio::test] + async fn tree_node_attaches_each_entry_to_its_own_commit() { + fn entry( + path: &str, + last_revision_repository: RepositoryId, + last_revision: Hash, + ) -> lore_revision::state::TreePath { + lore_revision::state::TreePath { + path: lore_revision::util::path::RelativePath::new_from_initial_path(path) + .expect("valid path"), + address: None, + flags: NodeFlags::File, + size: 0, + mode: 0, + tracking: false, + last_revision, + last_revision_repository, + } + } + fn commit(number: u64, revision: Hash) -> thin_client_v1::TreeCommit { + thin_client_v1::TreeCommit { + signature: revision.into(), + identifier: Some(model_v1::RevisionIdentifier { + branch_id: BranchId::default().into(), + number, + }), + ..Default::default() + } + } + fn number_of(node: &thin_client_v1::TreeNode) -> u64 { + node.last_commit + .as_ref() + .expect("attributed") + .identifier + .as_ref() + .expect("an attributed commit always carries its identifier") + .number + } + + let repo = random::(); + let older = Hash::from_u64(0x11); + let newer = Hash::from_u64(0x22); + let commits = + HashMap::from([((repo, older), commit(1, older)), ((repo, newer), commit(2, newer))]); + + let from_older = tree_node(&entry("old.txt", repo, older), &commits); + let from_newer = tree_node(&entry("new.txt", repo, newer), &commits); + let unattributed = tree_node( + &entry("none.txt", RepositoryId::default(), Hash::default()), + &commits, + ); + + assert_eq!( + number_of(&from_older), + 1, + "an entry must take the commit for its own revision", + ); + assert_eq!( + number_of(&from_newer), + 2, + "a sibling on a different revision must not inherit the first's commit", + ); + assert!( + unattributed.last_commit.is_none(), + "a zero last_revision must stay unattributed", + ); + + // An entry naming a revision the resolver skipped stays unattributed + // rather than borrowing whatever else is in the map. + let missing = tree_node(&entry("gone.txt", repo, Hash::from_u64(0x99)), &commits); + assert!( + missing.last_commit.is_none(), + "an unresolved revision must not fall back to another entry's commit", + ); + + // The same signature in a different repository is a different key - + // linked-subtree entries carry the linked repo id, so their lookup + // must not collide with the walked repo's map entries. + let other_repo = random::(); + let cross_repo = tree_node(&entry("linked/inner.txt", other_repo, older), &commits); + assert!( + cross_repo.last_commit.is_none(), + "a signature under a different repository must not borrow the walked repo's commit", + ); + } + + /// The signature-to-`TreeCommit` resolution the handler applies to every + /// attributed entry. + #[tokio::test] + async fn load_tree_commit_reads_the_revision_record() { + let repository = random::(); + let (immutable_store, mutable_store, execution) = + test_store_create().await.expect("test stores"); + + Box::pin(LORE_CONTEXT.scope(execution, async move { + let repository_context = Arc::new(RepositoryContext::new_server_context( + immutable_store, + mutable_store, + repository, + )); + let (branch, signatures) = + push_branch_with_revisions(&repository_context, &[&["a.txt"], &["b.txt"]]).await; + let second = signatures[1]; + + let commit = load_tree_commit(&repository_context, second) + .await + .expect("a pushed revision must resolve"); + assert_eq!( + commit.signature, + Into::::into(second), + "the record must describe the revision it was asked for", + ); + + // The identifier is what makes attribution usable as provenance: + // the number alone is per-branch and cannot say which branch it + // counts on. + let identifier = commit + .identifier + .as_ref() + .expect("a resolved commit must carry its identifier"); + assert_eq!(identifier.number, 2, "second revision on the branch"); + assert_eq!( + identifier.branch_id, + Into::::into(branch), + "the identifier must name the branch the revision was committed to", + ); + + // A signature that was never pushed is reported as unresolvable + // rather than failing the walk. + assert!( + load_tree_commit(&repository_context, Hash::from_u64(0xdead)) + .await + .is_none(), + "an unknown revision must be skipped, not fatal", + ); + })) + .await; + } } diff --git a/scripts/test/protobuf_wire.py b/scripts/test/protobuf_wire.py index 5b4936dc..edd6b24e 100644 --- a/scripts/test/protobuf_wire.py +++ b/scripts/test/protobuf_wire.py @@ -122,3 +122,18 @@ def field_strings(fields: Fields, field_number: int) -> list[str]: raise TypeError(f"Field {field_number} is a varint, not a string") decoded.append(value.decode("utf-8")) return decoded + + +def field_message(fields: Fields, field_number: int) -> Fields | None: + """Decoded fields of a nested message field; `None` when the field is + absent, distinct from empty which parses to `{}`. Nested messages are + length-delimited, so this is a `parse_fields` of the payload bytes.""" + values = fields.get(field_number) + if not values: + return None + value = values[-1] + if not isinstance(value, bytes): + raise TypeError( + f"Field {field_number} is a varint, not a length-delimited message" + ) + return parse_fields(value) diff --git a/scripts/test/test_revision_tree_last_commit.py b/scripts/test/test_revision_tree_last_commit.py new file mode 100644 index 00000000..317a88ad --- /dev/null +++ b/scripts/test/test_revision_tree_last_commit.py @@ -0,0 +1,265 @@ +# SPDX-FileCopyrightText: 2026 LoreLab.io +# SPDX-License-Identifier: MIT + +"""End-to-end coverage for `TreeNode.last_commit` on `ThinClientService.RevisionTree`. + +The fully joined attribution path is proved in pieces below the wire (attribution in +`lore-revision`, resolution and projection in `lore-server`), but the lore-server +fixtures serialize states directly rather than committing, so nothing attributes there. + +This runs against a real server, so it is the only regression net for the +joined path. + +Uses the wire-level `thin_client.py` harness rather than a generated client - the test +environment has no protobuf runtime and no generated stubs. +""" + +import logging + +import pytest +from thin_client import ( + NODE_TYPE_DIRECTORY, + NODE_TYPE_FILE, + NODE_TYPE_LINK, + revision_tree, +) + +from lore import Lore + +logger = logging.getLogger(__name__) + + +def _wire_identity(repo: Lore) -> tuple[bytes, bytes]: + """The repository id and latest revision signature as the raw bytes the + thin-client wire expects.""" + latest = repo.branch_info().local_latest + assert len(latest) == 64, f"Expected a full revision signature, got {latest!r}" + return bytes.fromhex(repo.get_id()), bytes.fromhex(latest) + + +def _by_path(nodes): + """Index a tree walk by path; asserts uniqueness so a missing entry + surfaces at lookup time rather than as an arbitrary duplicate.""" + indexed = {} + for node in nodes: + assert node.path not in indexed, f"{node.path} appears twice in the walk" + indexed[node.path] = node + return indexed + + +@pytest.mark.smoke +def test_thin_client_tree_last_commit_gated_by_request_flag( + new_lore_repo, lore_grpc_target +): + """Same revision, walked twice: `include_last_commit=False` returns no + `last_commit` on any node; `True` returns one on every attributed node. + Asserts the wire respects the flag, distinct from asserting attribution + correctness.""" + repo: Lore = new_lore_repo() + + with repo.open_file("a.txt", "w+") as f: + f.write("first\n") + repo.stage(scan=True) + repo.commit(message="add a.txt") + repo.push() + + repository_id, signature = _wire_identity(repo) + + unattributed = revision_tree( + lore_grpc_target, repository_id, signature, include_last_commit=False + ) + assert unattributed, "The walk must emit at least one node" + assert all(node.last_commit is None for node in unattributed), ( + "last_commit must be absent when the request does not ask for it, " + f"got {[node for node in unattributed if node.last_commit is not None]}" + ) + + attributed = revision_tree( + lore_grpc_target, repository_id, signature, include_last_commit=True + ) + assert len(attributed) == len(unattributed), ( + "The flag must not change which nodes are emitted" + ) + file_entries = [node for node in attributed if node.node_type == NODE_TYPE_FILE] + assert file_entries, "Sanity: fixture must produce at least one file entry" + assert all(node.last_commit is not None for node in file_entries), ( + "Every file entry in a committed revision must be attributed, " + f"got {[node.path for node in file_entries if node.last_commit is None]}" + ) + + +@pytest.mark.smoke +def test_thin_client_tree_last_commit_matches_the_touching_revision( + new_lore_repo, lore_grpc_target +): + """A two-revision fixture separates a per-entry back-pointer from a + parent-revision stamp. `a.txt` (touched at r2) and `b.txt` (added at r2) + both attribute to r2; the point of the test is that both commit messages + round-trip through the wire, so the projection is doing what the + lore-revision unit tests can only prove locally.""" + repo: Lore = new_lore_repo() + + # r1: add a.txt with the "first" content. + with repo.open_file("a.txt", "w+") as f: + f.write("first\n") + repo.stage(scan=True) + repo.commit(message="r1 add a.txt") + repo.push() + + # r2: modify a.txt and add b.txt. + with repo.open_file("a.txt", "w+") as f: + f.write("second\n") + with repo.open_file("b.txt", "w+") as f: + f.write("brand new\n") + repo.stage(scan=True) + repo.commit(message="r2 modify a.txt and add b.txt") + repo.push() + + repository_id, signature = _wire_identity(repo) + nodes = _by_path( + revision_tree( + lore_grpc_target, repository_id, signature, include_last_commit=True + ) + ) + + for path in ("a.txt", "b.txt"): + assert path in nodes, f"{path} missing from tree: {sorted(nodes)}" + node = nodes[path] + assert node.last_commit is not None, f"{path} must be attributed" + assert node.last_commit.commit_message == "r2 modify a.txt and add b.txt", ( + f"{path} must attribute to r2's commit, got " + f"{node.last_commit.commit_message!r}" + ) + # A r2 walk carries r2's identifier, which is number 2 on the branch. + assert node.last_commit.number == 2, ( + f"{path} must carry the r2 branch-relative number, got " + f"{node.last_commit.number}" + ) + # `branch_id` comes back as a big-endian UUID (lore ids are network + # byte order; see .agents/Discoveries.md). The wire test only checks + # length; comparing to `branch_info` involves the byte-order rule. + assert len(node.last_commit.branch_id) == 16, ( + f"branch_id must be a 16-byte UUID, got {node.last_commit.branch_id!r}" + ) + assert node.last_commit.signature == signature, ( + f"{path} must be attributed against the walked revision's signature" + ) + + +@pytest.mark.smoke +def test_thin_client_tree_last_commit_attributes_across_a_link( + new_lore_repo, lore_grpc_target +): + """A link's contents live in the linked repository, and their attribution + lives with them: `linked/inner.txt` reports the *linked* repository's + revision on the wire, not the parent's. The link entry itself is + attributed against the walked repository because it lives in the walked + state.""" + linked: Lore = new_lore_repo() + with linked.open_file("inner.txt", "w+") as f: + f.write("inner content\n") + linked.stage(scan=True) + linked.commit(message="inner commit in linked repo") + linked.push() + _, linked_signature = _wire_identity(linked) + + parent: Lore = new_lore_repo() + with parent.open_file("top.txt", "w+") as f: + f.write("parent content\n") + parent.stage(scan=True) + parent.commit(message="parent commit before linking") + parent.push() + + parent.link_add("vendor", linked.get_id(), "/") + parent.commit(message="parent commit adding link") + parent.push() + + parent_id, parent_signature = _wire_identity(parent) + nodes = _by_path( + revision_tree( + lore_grpc_target, parent_id, parent_signature, include_last_commit=True + ) + ) + + # Files in the parent tree attribute against the parent's most recent + # commit that touched them. + assert "top.txt" in nodes, f"top.txt missing from tree: {sorted(nodes)}" + assert nodes["top.txt"].last_commit is not None + assert ( + nodes["top.txt"].last_commit.commit_message == "parent commit before linking" + ), ( + "top.txt must attribute to its own add revision, not the one that " + f"added the link; got {nodes['top.txt'].last_commit.commit_message!r}" + ) + + # The link entry itself lives in the parent state and attributes to the + # revision that added it. + assert "vendor" in nodes, f"vendor link missing from tree: {sorted(nodes)}" + assert nodes["vendor"].node_type == NODE_TYPE_LINK + assert nodes["vendor"].last_commit is not None + assert ( + nodes["vendor"].last_commit.commit_message == "parent commit adding link" + ), ( + "The link node must attribute to the parent revision that added it, " + f"got {nodes['vendor'].last_commit.commit_message!r}" + ) + + # Content inside the link lives in the linked repo's state and attributes + # against that repo's revision - not the parent's. The commit message + # carried on the wire proves the resolution reached the right repo. + assert "vendor/inner.txt" in nodes, ( + f"linked-subtree content missing: {sorted(nodes)}" + ) + inner = nodes["vendor/inner.txt"] + assert inner.last_commit is not None, ( + "Content behind a link must be attributed now the walker crosses links" + ) + assert inner.last_commit.commit_message == "inner commit in linked repo", ( + "Content inside a link must attribute to the linked repository's " + f"revision, got {inner.last_commit.commit_message!r}" + ) + assert inner.last_commit.signature == linked_signature, ( + "The linked-subtree entry must carry the linked repository's revision " + "signature, not the walked repository's" + ) + + +@pytest.mark.smoke +def test_thin_client_tree_last_commit_directory_follows_descendant( + new_lore_repo, lore_grpc_target +): + """Directories propagate descendant changes: a subdirectory whose child + just changed reports the tip's commit, not an earlier one. This is what + lets a UI attribute directory rows without a max-over-descendants pass.""" + repo: Lore = new_lore_repo() + + repo.make_dirs("sub") + with repo.open_file("sub/child.txt", "w+") as f: + f.write("first\n") + repo.stage(scan=True) + repo.commit(message="r1 add sub/child.txt") + repo.push() + + with repo.open_file("sub/child.txt", "w+") as f: + f.write("second\n") + repo.stage(scan=True) + repo.commit(message="r2 modify sub/child.txt") + repo.push() + + repository_id, signature = _wire_identity(repo) + nodes = _by_path( + revision_tree( + lore_grpc_target, repository_id, signature, include_last_commit=True + ) + ) + + assert "sub" in nodes, f"sub directory missing: {sorted(nodes)}" + sub = nodes["sub"] + assert sub.node_type == NODE_TYPE_DIRECTORY, ( + f"sub must be a directory, got node_type={sub.node_type}" + ) + assert sub.last_commit is not None, "Directories propagate and must attribute" + assert sub.last_commit.commit_message == "r2 modify sub/child.txt", ( + f"sub must move with its descendant to r2, got " + f"{sub.last_commit.commit_message!r}" + ) diff --git a/scripts/test/thin_client.py b/scripts/test/thin_client.py index 36bdd5e6..12f0256c 100644 --- a/scripts/test/thin_client.py +++ b/scripts/test/thin_client.py @@ -17,10 +17,18 @@ field_bool, field_bytes, field_int, + field_message, field_string, parse_fields, ) + +def _encode_bool_field(field_number: int, value: bool) -> bytes: + """Encode one `bool` field. Proto3 default is False, which is not + serialised - callers should only emit this when the value is True.""" + tag = field_number << 3 | 0 # wire type 0 = VARINT + return bytes([tag, 1 if value else 0]) + logger = logging.getLogger(__name__) _REVISION_TREE_METHOD = "/lore.thin_client.v1.ThinClientService/RevisionTree" @@ -38,6 +46,7 @@ ACTION_DELETE = 2 _TREE_REQUEST_SIGNATURE = 2 +_TREE_REQUEST_INCLUDE_LAST_COMMIT = 5 _DIFF_REQUEST_SIGNATURE_FROM = 2 _DIFF_REQUEST_SIGNATURE_TO = 4 @@ -48,6 +57,17 @@ _TREE_NODE_PATH = 1 _TREE_NODE_NODE_TYPE = 2 _TREE_NODE_TRACKING = 6 +_TREE_NODE_LAST_COMMIT = 7 + +_TREE_COMMIT_SIGNATURE = 1 +_TREE_COMMIT_MESSAGE = 2 +_TREE_COMMIT_TIMESTAMP = 3 +_TREE_COMMIT_IDENTIFIER = 4 +_TREE_COMMIT_COMMITTED_BY = 5 + +# lore.model.v1.RevisionIdentifier +_REVISION_IDENTIFIER_BRANCH_ID = 1 +_REVISION_IDENTIFIER_NUMBER = 2 _DIFF_CHANGE_PATH = 1 _DIFF_CHANGE_ACTION = 3 @@ -59,6 +79,21 @@ _DIFF_PARTITION_LINK_PARTITION = 2 +@dataclass(frozen=True) +class TreeCommit: + """One `lore.thin_client.v1.TreeCommit` attached to a `TreeNode`. + + `branch_id` is the raw bytes of the resolved `RevisionIdentifier.branch_id` + - hex-format at the callsite to compare against `Lore.get_id()`.""" + + signature: bytes + commit_message: str + timestamp: int + branch_id: bytes + number: int + committed_by: str + + @dataclass(frozen=True) class TreeNode: """One `lore.thin_client.v1.TreeNode` off a `RevisionTree` stream.""" @@ -66,6 +101,7 @@ class TreeNode: path: str node_type: int tracking: bool + last_commit: TreeCommit | None @dataclass(frozen=True) @@ -108,12 +144,32 @@ def _payloads(response: bytes, payload_field: int) -> list[dict]: ] +def _tree_commit(node: dict) -> TreeCommit | None: + """`TreeCommit` off a `TreeNode`, or `None` when the server did not + populate one - either the walk was not asked to attribute (proto3 + default: field absent), or the walk could not attribute this entry + (root, or an entry the resolver skipped).""" + commit = field_message(node, _TREE_NODE_LAST_COMMIT) + if commit is None: + return None + identifier = field_message(commit, _TREE_COMMIT_IDENTIFIER) or {} + return TreeCommit( + signature=field_bytes(commit, _TREE_COMMIT_SIGNATURE), + commit_message=field_string(commit, _TREE_COMMIT_MESSAGE), + timestamp=field_int(commit, _TREE_COMMIT_TIMESTAMP), + branch_id=field_bytes(identifier, _REVISION_IDENTIFIER_BRANCH_ID), + number=field_int(identifier, _REVISION_IDENTIFIER_NUMBER), + committed_by=field_string(commit, _TREE_COMMIT_COMMITTED_BY), + ) + + def _tree_nodes(response: bytes) -> list[TreeNode]: return [ TreeNode( path=field_string(node, _TREE_NODE_PATH), node_type=field_int(node, _TREE_NODE_NODE_TYPE), tracking=field_bool(node, _TREE_NODE_TRACKING), + last_commit=_tree_commit(node), ) for node in _payloads(response, _TREE_RESPONSE_NODE) ] @@ -168,12 +224,22 @@ def revision_tree( repository_id: bytes, signature: bytes, timeout: float = 30.0, + include_last_commit: bool = False, ) -> list[TreeNode]: - """Every `TreeNode` the server streams for `signature`, in stream order.""" + """Every `TreeNode` the server streams for `signature`, in stream order. + + Setting `include_last_commit` populates `TreeNode.last_commit` on entries + the walker can attribute - files, links, and directories that live in the + walked repository, plus everything under a link that lives in the linked + repository.""" + request = encode_bytes_field(_TREE_REQUEST_SIGNATURE, signature) + # Proto3 default for bool is False, so only serialise when True. + if include_last_commit: + request += _encode_bool_field(_TREE_REQUEST_INCLUDE_LAST_COMMIT, True) nodes = _collect_stream( grpc_target, _REVISION_TREE_METHOD, - encode_bytes_field(_TREE_REQUEST_SIGNATURE, signature), + request, repository_id, _tree_nodes, timeout,