From 8f04f77cf5f136f1c8b910ff4ad30a9777a78ff7 Mon Sep 17 00:00:00 2001 From: Ragnor Comerford Date: Fri, 16 Oct 2026 11:27:38 -0700 Subject: [PATCH 1/4] feat(server,cli): report each branch's head, fork and creator in the branch listing `GET /branches?detail=true` and `omnigraph branch list --detail` add a `details` array beside the names: for each branch the caller may read, its head commit, the branch (`from`) and commit (`fork`) its current lifetime was created from, the actor that created it, and when it was created and last moved. Every field derives from the branch refs and each branch's own `__manifest` (the latest version, and the creation version once the branch has written a commit); no lineage is read, so the cost per branch is flat in history. Branch creation now records its actor on the new native ref (`omnigraph.created_by_actor_id`) right after the native create. A failure there leaves the branch without an actor and reports uncertain completion. --- changelog.d/branch-list-details.added.md | 11 + crates/omnigraph-api-types/src/lib.rs | 74 +++++++ crates/omnigraph-catalog/src/branch_detail.rs | 178 ++++++++++++++++ crates/omnigraph-catalog/src/lib.rs | 34 ++- crates/omnigraph-cli/src/cli.rs | 4 + crates/omnigraph-cli/src/client.rs | 40 +++- crates/omnigraph-cli/src/main.rs | 18 +- crates/omnigraph-cli/src/output.rs | 31 +++ .../tests/integration/cli_data.rs | 32 +++ .../tests/integration/parity_matrix.rs | 7 + crates/omnigraph-core/src/branch_control.rs | 46 +++++ crates/omnigraph-server/src/handlers.rs | 47 ++++- crates/omnigraph-server/src/mcp.rs | 16 +- .../tests/integration/auth_policy.rs | 24 ++- .../tests/integration/data_routes.rs | 23 +++ crates/omnigraph/src/db/graph_coordinator.rs | 4 +- crates/omnigraph/src/db/mod.rs | 22 +- crates/omnigraph/src/db/omnigraph.rs | 108 +++++++++- crates/omnigraph/src/seams/catalog.rs | 1 + crates/omnigraph/tests/failpoints.rs | 50 +++++ crates/omnigraph/tests/forbidden_apis.rs | 13 +- .../tests/integration/branch_control_cost.rs | 114 ++++++++++- .../omnigraph/tests/integration/branching.rs | 193 +++++++++++++++++- docs/dev/lance.md | 3 + docs/dev/writes.md | 14 ++ docs/user/branching/index.md | 30 +++ docs/user/cli/reference.md | 2 +- docs/user/operations/server.md | 2 +- openapi.json | 82 +++++++- skills/omnigraph/references/commands.md | 5 +- 30 files changed, 1172 insertions(+), 56 deletions(-) create mode 100644 changelog.d/branch-list-details.added.md create mode 100644 crates/omnigraph-catalog/src/branch_detail.rs diff --git a/changelog.d/branch-list-details.added.md b/changelog.d/branch-list-details.added.md new file mode 100644 index 000000000..57022073c --- /dev/null +++ b/changelog.d/branch-list-details.added.md @@ -0,0 +1,11 @@ +- `GET /graphs/{graph_id}/branches?detail=true` and `omnigraph branch list + --detail` report each branch's head commit, the branch and commit it was + created from, the actor that created it and when, and when it last moved, + beside the names. A client no longer reads each branch's whole commit list + for these: the listing reads each branch's manifest once, or twice once the + branch has written a commit, whatever its history. Branch creation now + records its actor on the branch; a branch created earlier lists + `actor_id: null`. A branch deleted and created again reports its new + creation and actor. See [Branches][branch-list-details]. + +[branch-list-details]: ../docs/user/branching/index.md#branch-workflow diff --git a/crates/omnigraph-api-types/src/lib.rs b/crates/omnigraph-api-types/src/lib.rs index f012845af..556983522 100644 --- a/crates/omnigraph-api-types/src/lib.rs +++ b/crates/omnigraph-api-types/src/lib.rs @@ -335,9 +335,58 @@ pub struct BranchCreateOutput { pub actor_id: Option, } +/// Query parameters for listing branches. +#[derive(Debug, Clone, Default, Deserialize, IntoParams)] +#[into_params(parameter_in = Query)] +pub struct BranchListQuery { + /// `true` adds `details`: each branch's head, the branch and commit it + /// was created from, who created it and when, and when it last moved. + /// Reads each listed branch's latest version, and the version it was + /// created from once it has written a commit, never its commit history; + /// the names alone read neither. + #[serde(default)] + pub detail: bool, +} + #[derive(Debug, Clone, Serialize, Deserialize, ToSchema)] pub struct BranchListOutput { + /// Every live branch, sorted by name in byte order. pub branches: Vec, + /// Present only with `detail=true`: one entry per branch the caller may + /// `read`, in the order of `branches`. A branch the caller may not read is + /// listed in `branches` alone. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub details: Option>, +} + +/// One live branch as `GET /branches?detail=true` reports it. `from`, +/// `actor_id` and `created_at` describe the branch's creation as its +/// `POST /branches` receipt did. A branch deleted and created again under the +/// same name is a new lifetime: its creation and fork are the later creation's. +#[derive(Debug, Clone, Serialize, Deserialize, ToSchema)] +pub struct BranchDetailOutput { + pub name: String, + /// The commit a read of the branch sees: its newest commit, or `fork` + /// while it has written none. + pub head: CommitOutput, + /// The branch this one was created from, as that branch was named then; + /// it may since have been deleted or recreated. `null` on `main`. + pub from: Option, + /// The head of `from` when this branch was created. A later merge does + /// not move it; `GET /branches/merge-base` answers where two branches + /// last met. `null` on `main`. + pub fork: Option, + /// The actor that created the branch; `null` on `main` and when the + /// creation recorded no actor. + pub actor_id: Option, + /// When the branch was created, as Unix epoch microseconds recorded to + /// the second; `null` on `main`. + #[schema(example = 1714000000000000i64)] + pub created_at: Option, + /// When the head last moved, as Unix epoch microseconds: the time of the + /// branch's newest commit, or `created_at` while it has written none. + #[schema(example = 1714000000000000i64)] + pub updated_at: i64, } #[derive(Debug, Clone, Serialize, Deserialize, ToSchema)] @@ -2332,6 +2381,31 @@ pub fn diff_output(page: &omnigraph::changes::GraphDiffPage) -> DiffOutput { } } +/// A detailed branch listing (`detail=true`): names and details, each in +/// byte order. +pub fn branch_list_output(listing: &omnigraph::db::BranchListing) -> BranchListOutput { + let mut branches = listing.branches.clone(); + branches.sort(); + let mut details: Vec<_> = listing.details.iter().map(branch_detail_output).collect(); + details.sort_by(|left, right| left.name.cmp(&right.name)); + BranchListOutput { + branches, + details: Some(details), + } +} + +pub fn branch_detail_output(detail: &omnigraph::db::BranchDetail) -> BranchDetailOutput { + BranchDetailOutput { + name: detail.name.clone(), + head: commit_output(&detail.head), + from: detail.from.clone(), + fork: detail.fork.as_ref().map(commit_output), + actor_id: detail.actor_id.clone(), + created_at: detail.created_at, + updated_at: detail.updated_at, + } +} + pub fn merge_base_output( source: &str, target: &str, diff --git a/crates/omnigraph-catalog/src/branch_detail.rs b/crates/omnigraph-catalog/src/branch_detail.rs new file mode 100644 index 000000000..9e8cf67f9 --- /dev/null +++ b/crates/omnigraph-catalog/src/branch_detail.rs @@ -0,0 +1,178 @@ +//! What a detailed branch listing reports about each live graph branch: its +//! head, the branch and commit its current lifetime was created from, and when +//! and by whom that lifetime was created. Every fact is read from the branch +//! refs and each branch's own `__manifest`; nothing is stored for the listing. + +use std::sync::Arc; + +use futures::StreamExt; +use lance::Dataset; +use lance::dataset::refs::BranchContents; + +use crate::branch_control::list_live_manifest_branch_contents; +use crate::error::{OmniError, Result}; +use crate::history::ExtentCache; +use crate::layout::open_manifest_dataset_with_session; +use crate::state::{GraphLineageRow, read_manifest_rows_projected}; + +/// How many branches a listing reads at once. +const BRANCH_DETAIL_READ_CONCURRENCY: usize = 8; + +/// One listing of the live graph branches. +#[derive(Debug, Clone)] +pub struct GraphBranchListing { + /// Every live branch, `main` first and the rest by name. + pub branches: Vec, + /// One entry per branch the caller admitted, in the order of `branches`. + pub details: Vec, +} + +/// One live graph branch as a detailed listing reports it. +#[derive(Debug, Clone)] +pub struct GraphBranchDetail { + pub name: String, + /// The head record of the branch's latest `__manifest` version: its newest + /// commit, or the commit it was created at while it has written none. + pub head: GraphLineageRow, + /// Whether the branch's current lifetime wrote `head`. + pub head_is_own: bool, + /// The logical name of the branch the current lifetime was created from, + /// as that branch was named then; it may since have been deleted or + /// recreated. `None` on main. + pub from: Option, + /// The head of `from` at the `__manifest` version the lifetime was + /// created from; `None` on main. + pub fork: Option, + /// The actor recorded on the lifetime's native ref as its creator; `None` + /// on main and when no actor was recorded. + pub actor_id: Option, + /// `BranchContents::create_at` of the lifetime's native ref, in Unix + /// seconds; `None` on main. + pub created_at_seconds: Option, +} + +/// Every live graph branch, with details of those `can_read` admits. One open +/// of main's `__manifest` and one listing of its refs; then, for each admitted +/// branch, a checkout of its latest version and one scan of its rows, and when +/// the branch has written a commit, the checkout and scan of the version it was +/// created from. A branch whose ref is gone before its own reads keeps its name +/// and has no details: it was deleted after the listing. +pub async fn list_graph_branches_detailed( + root_uri: &str, + control_session: &Arc, + can_read: &mut (dyn FnMut(&str) -> bool + Send + '_), +) -> Result { + let root = root_uri.trim_end_matches('/'); + let main = open_manifest_dataset_with_session(root, None, control_session).await?; + let mut named = Vec::new(); + for (native, contents) in list_live_manifest_branch_contents(&main).await? { + if native == "main" { + continue; + } + let logical = crate::branch_names::logical_branch_name(&native).to_string(); + named.push((logical, native, contents)); + } + named.sort_by(|left, right| left.0.cmp(&right.0)); + if let Some(pair) = named.windows(2).find(|pair| pair[0].0 == pair[1].0) { + return Err(OmniError::manifest_conflict(format!( + "branch '{}' has more than one live native incarnation; run cleanup before using it", + pair[0].0 + ))); + } + + let branches = std::iter::once("main".to_string()) + .chain(named.iter().map(|(logical, _, _)| logical.clone())) + .collect(); + let mut details = Vec::with_capacity(named.len() + 1); + if can_read("main") { + details.push(GraphBranchDetail { + name: "main".to_string(), + head: read_manifest_rows_projected(&main).await?.head, + head_is_own: true, + from: None, + fork: None, + actor_id: None, + created_at_seconds: None, + }); + } + let admitted: Vec<_> = named + .into_iter() + .filter(|(logical, _, _)| can_read(logical)) + .collect(); + let legacy = ExtentCache::default(); + let mut reads = futures::stream::iter(admitted) + .map(|(logical, native, contents)| { + let (main, legacy) = (&main, &legacy); + async move { branch_detail(root, main, legacy, logical, native, contents).await } + }) + .buffered(BRANCH_DETAIL_READ_CONCURRENCY); + while let Some(detail) = reads.next().await { + details.extend(detail?); + } + Ok(GraphBranchListing { branches, details }) +} + +async fn branch_detail( + root: &str, + main: &Dataset, + legacy: &ExtentCache, + logical: String, + native: String, + contents: BranchContents, +) -> Result> { + match read_branch(root, main, legacy, &native, &contents).await { + Ok((head, fork_commit)) => { + let head_is_own = head.native_branch.as_deref() == Some(native.as_str()); + let from = contents + .parent_branch + .as_deref() + .map(crate::branch_names::logical_branch_name) + .unwrap_or("main") + .to_string(); + Ok(Some(GraphBranchDetail { + name: logical, + from: Some(from), + fork: Some(fork_commit.unwrap_or_else(|| head.clone())), + head, + head_is_own, + actor_id: crate::branch_control::branch_creator(&contents).map(str::to_string), + created_at_seconds: Some(contents.create_at), + })) + } + // The listing saw this lifetime live, and a native name is never + // reused. A read that fails once its ref is gone reports a branch + // deleted since; any other failure is the read's own. + Err(error) => match crate::branch_control::get_branch_identifier(main, &native).await { + Err(lance::Error::RefNotFound { .. }) => Ok(None), + _ => Err(error), + }, + } +} + +/// The latest head of the lifetime `native` and, when the lifetime wrote it, +/// the head at the version the lifetime was created from. +async fn read_branch( + root: &str, + main: &Dataset, + legacy: &ExtentCache, + native: &str, + contents: &BranchContents, +) -> Result<(GraphLineageRow, Option)> { + let branch = main + .checkout_branch(native) + .await + .map_err(OmniError::storage)?; + let head = read_manifest_rows_projected(&branch).await?.head; + if head.native_branch.as_deref() != Some(native) { + return Ok((head, None)); + } + let created = branch + .checkout_version(contents.parent_version) + .await + .map_err(OmniError::storage)?; + let fork = match crate::legacy::at_version(root, &created.session(), legacy, &created).await? { + None => read_manifest_rows_projected(&created).await?.head, + Some(at) => at.served(Some(native), contents.parent_version)?.commit, + }; + Ok((head, Some(fork))) +} diff --git a/crates/omnigraph-catalog/src/lib.rs b/crates/omnigraph-catalog/src/lib.rs index 76446b60a..674efb5b5 100644 --- a/crates/omnigraph-catalog/src/lib.rs +++ b/crates/omnigraph-catalog/src/lib.rs @@ -14,6 +14,7 @@ pub(crate) use omnigraph_core::{ branch_control, branch_names, dataset_index, error, handle_cache, instrumentation, lance_access, lance_clone, metadata, seams, staging, storage, }; +pub mod branch_detail; pub mod commit_graph; mod commit; @@ -63,6 +64,7 @@ pub mod retention; pub mod state; pub mod unattached; pub mod unpublished; +pub use branch_detail::{GraphBranchDetail, GraphBranchListing, list_graph_branches_detailed}; pub use branch_names::is_merge_input_tag; pub use migrations::{ UNATTACHED_BRANCHES_STAMP, UNPUBLISHED_COMMITS_STAMP, require_unattached_branches, @@ -2335,6 +2337,15 @@ impl ManifestCoordinator { /// legacy ref named exactly `name` also counts as existing. #[doc(hidden)] pub async fn create_branch(&mut self, name: &str) -> Result<()> { + self.create_branch_as(name, None).await + } + + /// [`Self::create_branch`], recording `actor` on the new ref as the actor + /// that created it. The branch exists once the native create succeeds; a + /// failure to record the actor afterwards reports uncertain completion, + /// and the branch lists with no actor. + #[doc(hidden)] + pub async fn create_branch_as(&mut self, name: &str, actor: Option<&str>) -> Result<()> { crate::branch_names::ensure_logical_branch_name(name)?; let mut ds = self.dataset.clone(); crate::branch_control::ensure_manifest_branch_create_namespace(&ds, name) @@ -2345,11 +2356,26 @@ impl ManifestCoordinator { match crate::branch_control::create_branch_recoverably(&mut ds, &native, self.version()) .await? { - crate::branch_control::BranchCreateOutcome::Created => Ok(()), - crate::branch_control::BranchCreateOutcome::RefAlreadyExists => Err( - OmniError::manifest_conflict(format!("branch '{}' already exists", name)), - ), + crate::branch_control::BranchCreateOutcome::Created => {} + crate::branch_control::BranchCreateOutcome::RefAlreadyExists => { + return Err(OmniError::manifest_conflict(format!( + "branch '{}' already exists", + name + ))); + } } + let Some(actor) = actor else { + return Ok(()); + }; + crate::branch_control::record_branch_creator(&ds, &native, actor) + .await + .map_err(|error| { + OmniError::manifest_internal(format!( + "branch '{name}' was created, but recording the actor that created it \ + failed ({error}); it lists with no actor" + )) + .with_completion_evidence(crate::error::CompletionEvidence::Uncertain) + }) } async fn open_branch_control_dataset(&self) -> Result { diff --git a/crates/omnigraph-cli/src/cli.rs b/crates/omnigraph-cli/src/cli.rs index 0db253c8f..5a30be912 100644 --- a/crates/omnigraph-cli/src/cli.rs +++ b/crates/omnigraph-cli/src/cli.rs @@ -943,6 +943,10 @@ pub(crate) enum BranchCommand { /// Graph URI #[arg(long)] uri: Option, + /// Also report each branch's head, the branch and commit it was + /// created from, who created it and when, and when it last moved + #[arg(long)] + detail: bool, #[arg(long)] json: bool, }, diff --git a/crates/omnigraph-cli/src/client.rs b/crates/omnigraph-cli/src/client.rs index 869852795..31175a7cb 100644 --- a/crates/omnigraph-cli/src/client.rs +++ b/crates/omnigraph-cli/src/client.rs @@ -37,10 +37,10 @@ use omnigraph_api_types::{ ChangeRequest, CommitChangesOutput, CommitListOutput, CommitOutput, DiffOutput, EntityKindOutput, ExportRequest, GraphBatchLoadOutput, GraphDiscoveryResponse, GraphListResponse, InvokeStoredQueryRequest, MergeBaseOutput, QueryRequest, ReadOutput, - SchemaApplyOutput, SchemaOutput, SettingsRequest, SnapshotOutput, branch_list_read_output, - branch_read_output, change_baseline_output, change_feed_output, change_scope, - commit_changes_output, commit_output, diff_output, merge_base_output, read_output, - schema_apply_output, show_read_output, snapshot_payload, + SchemaApplyOutput, SchemaOutput, SettingsRequest, SnapshotOutput, branch_list_output, + branch_list_read_output, branch_read_output, change_baseline_output, change_feed_output, + change_scope, commit_changes_output, commit_output, diff_output, merge_base_output, + read_output, schema_apply_output, show_read_output, snapshot_payload, }; use omnigraph_compiler::catalog::Catalog; use omnigraph_compiler::query::ast::{BranchRead, BranchWrite, CommitWrite}; @@ -628,7 +628,37 @@ impl GraphClient { let session = Self::open_read_session(uri, &[]).await?; let mut branches = session.branch_list().await?; branches.sort(); - Ok(BranchListOutput { branches }) + Ok(BranchListOutput { + branches, + details: None, + }) + } + } + } + + /// [`Self::branch_list`] with each readable branch's details + /// (`GET /branches?detail=true`). + pub(crate) async fn branch_list_detailed(&self) -> Result { + match self { + GraphClient::Remote { + http, + base_url, + token, + .. + } => { + remote_json( + http, + Method::GET, + remote_url(base_url, &["branches"], &[("detail", "true")])?, + None, + token.as_deref(), + ) + .await + } + GraphClient::Embedded { uri, .. } => { + let session = Self::open_read_session(uri, &[]).await?; + let listing = session.branch_list_detailed(&mut |_| true).await?; + Ok(branch_list_output(&listing)) } } } diff --git a/crates/omnigraph-cli/src/main.rs b/crates/omnigraph-cli/src/main.rs index 11a4d3fc5..33858503a 100644 --- a/crates/omnigraph-cli/src/main.rs +++ b/crates/omnigraph-cli/src/main.rs @@ -5,9 +5,9 @@ use color_eyre::eyre::{Result, bail}; use omnigraph::db::{Omnigraph, ReadTarget, SnapshotId}; use omnigraph::loader::LoadMode; use omnigraph_api_types::{ - BlobContentKindOutput, BlobStatOutput, BranchOutcomeOutput, ChangeOutput, CommitKindOutput, - CommitOutput, ErrorOutput, GraphBatchDeclarationOutput, GraphBatchLoadOutput, ReadOutput, - SchemaApplyOutput, SnapshotDatasetOutput, query_file_refusals, + BlobContentKindOutput, BlobStatOutput, BranchListOutput, BranchOutcomeOutput, ChangeOutput, + CommitKindOutput, CommitOutput, ErrorOutput, GraphBatchDeclarationOutput, GraphBatchLoadOutput, + ReadOutput, SchemaApplyOutput, SnapshotDatasetOutput, query_file_refusals, }; use omnigraph_cluster::{ DiagnosticSeverity, ForceUnlockOutput, PlanOutput, StateSyncOutput, StatusOutput, @@ -719,7 +719,7 @@ async fn run(cli: Cli) -> Result<()> { println!("created branch {} from {}", payload.name, payload.from); } } - BranchCommand::List { uri, json } => { + BranchCommand::List { uri, detail, json } => { let client = client::GraphClient::resolve( capability, cli.server.as_deref(), @@ -729,13 +729,15 @@ async fn run(cli: Cli) -> Result<()> { cli.store.as_deref(), ) .await?; - let payload = client.branch_list().await?; + let payload = if detail { + client.branch_list_detailed().await? + } else { + client.branch_list().await? + }; if json { print_json(&payload)?; } else { - for branch in payload.branches { - println!("{}", branch); - } + print_branch_list_human(&payload); } } BranchCommand::Delete { uri, name, json } => { diff --git a/crates/omnigraph-cli/src/output.rs b/crates/omnigraph-cli/src/output.rs index 74a9aafd4..b35e073c8 100644 --- a/crates/omnigraph-cli/src/output.rs +++ b/crates/omnigraph-cli/src/output.rs @@ -821,6 +821,37 @@ pub(crate) fn print_change_human(output: &ChangeOutput) { } } +/// One line per branch: its name, then with details its head, the branch and +/// commit it was created from, the actor that created it, and its creation and +/// last-move times. A branch +/// without details (one the caller may not read) prints its name alone. +pub(crate) fn print_branch_list_human(payload: &BranchListOutput) { + let details = payload.details.as_deref().unwrap_or_default(); + for branch in &payload.branches { + let Some(detail) = details.iter().find(|detail| &detail.name == branch) else { + println!("{branch}"); + continue; + }; + let fork = match (&detail.from, &detail.fork) { + (Some(from), Some(fork)) => format!(" from={from} fork={}", fork.graph_commit_id), + _ => String::new(), + }; + let actor = detail + .actor_id + .as_deref() + .map(|actor| format!(" actor={actor}")) + .unwrap_or_default(); + let created_at = detail + .created_at + .map(|created_at| format!(" created_at={created_at}")) + .unwrap_or_default(); + println!( + "{branch} head={}{fork}{actor}{created_at} updated_at={}", + detail.head.graph_commit_id, detail.updated_at + ); + } +} + pub(crate) fn print_commit_list_human(commits: &[CommitOutput]) { for commit in commits { let branch = commit.graph_branch.as_deref().unwrap_or("main"); diff --git a/crates/omnigraph-cli/tests/integration/cli_data.rs b/crates/omnigraph-cli/tests/integration/cli_data.rs index 583aa5722..f9a173332 100644 --- a/crates/omnigraph-cli/tests/integration/cli_data.rs +++ b/crates/omnigraph-cli/tests/integration/cli_data.rs @@ -3142,6 +3142,38 @@ fn branch_list_outputs_sorted_branches() { .collect::>(); assert_eq!(lines, vec!["alpha", "main", "zeta"]); + + // `--detail` keeps one line per branch, in the same order, and adds the + // head, the fork and the times. + let detailed = output_success( + cli() + .arg("branch") + .arg("list") + .arg("--detail") + .arg("--uri") + .arg(&graph), + ); + let stdout = stdout_string(&detailed); + let lines = stdout + .lines() + .map(str::trim) + .filter(|line| !line.is_empty()) + .collect::>(); + assert_eq!(lines.len(), 3, "{stdout}"); + let head = lines[1] + .strip_prefix("main head=") + .and_then(|rest| rest.split(' ').next()) + .unwrap_or_else(|| panic!("{stdout}")); + for (line, name) in [(lines[0], "alpha"), (lines[2], "zeta")] { + assert!( + line.starts_with(&format!("{name} head={head} from=main fork={head} ")), + "{stdout}" + ); + assert!( + line.contains(" created_at=") && line.contains(" updated_at="), + "{stdout}" + ); + } } #[test] diff --git a/crates/omnigraph-cli/tests/integration/parity_matrix.rs b/crates/omnigraph-cli/tests/integration/parity_matrix.rs index f04ab0d6f..b1db9cb90 100644 --- a/crates/omnigraph-cli/tests/integration/parity_matrix.rs +++ b/crates/omnigraph-cli/tests/integration/parity_matrix.rs @@ -265,6 +265,13 @@ fn parity_branch_list() { let p = parity(); let (l, r) = p.run(&["branch", "list", "--json"]); assert_parity("branch list", &l, &r); + let (l, r) = p.run(&["branch", "list", "--detail", "--json"]); + assert_parity("branch list --detail", &l, &r); + assert!( + parse_stdout_json(&l)["details"][0]["head"]["graph_commit_id"].is_string(), + "{}", + String::from_utf8_lossy(&l.stdout) + ); } #[test] diff --git a/crates/omnigraph-core/src/branch_control.rs b/crates/omnigraph-core/src/branch_control.rs index 877c7b7ff..323b3b999 100644 --- a/crates/omnigraph-core/src/branch_control.rs +++ b/crates/omnigraph-core/src/branch_control.rs @@ -788,6 +788,52 @@ decide_seam! { pub static BRANCH_CREATE_POST_NATIVE = ("branch_create.post_native", BranchCreate, [Fail]); } +decide_seam! { + /// After the native create, before the creating actor is recorded on the + /// new ref. The branch exists and lists with no actor; the create reports + /// uncertain completion. + pub static BRANCH_CREATE_PRE_CREATOR = ("branch_create.pre_creator", BranchCreate, [Fail]); +} + +/// The ref metadata key holding the actor that created a native manifest +/// branch. Absent on a ref created without a known actor or before the key +/// existed; retirement preserves it. +const BRANCH_CREATOR_KEY: &str = "omnigraph.created_by_actor_id"; + +/// The actor recorded as the creator of a native manifest branch. +pub fn branch_creator(contents: &BranchContents) -> Option<&str> { + contents + .metadata + .get(BRANCH_CREATOR_KEY) + .map(String::as_str) +} + +/// Record `actor` as the creator of `branch`, a ref this caller has just +/// created under its branch gate and so carries no other metadata yet. Lance +/// writes the whole ref again; a concurrent reader's torn read of it is +/// reread at the bounded ref-read boundary. A failed write is read back, so a +/// lost acknowledgement counts as recorded. +pub async fn record_branch_creator(dataset: &Dataset, branch: &str, actor: &str) -> Result<()> { + let written = match fail(&BRANCH_CREATE_PRE_CREATOR) { + Ok(()) => dataset + .branches() + .replace_metadata( + branch, + HashMap::from([(BRANCH_CREATOR_KEY.to_string(), actor.to_string())]), + ) + .await + .map_err(OmniError::storage), + Err(error) => Err(error), + }; + let Err(error) = written else { + return Ok(()); + }; + match get_branch_contents(dataset, branch).await { + Ok(contents) if branch_creator(&contents) == Some(actor) => Ok(()), + _ => Err(error), + } +} + decide_seam! { /// After the namespace inventory (refs listed, path collision checked, /// a ref-less tree reclaimed), before the native create. No CAS covers diff --git a/crates/omnigraph-server/src/handlers.rs b/crates/omnigraph-server/src/handlers.rs index c481d8764..e4a477dd3 100644 --- a/crates/omnigraph-server/src/handlers.rs +++ b/crates/omnigraph-server/src/handlers.rs @@ -2687,6 +2687,7 @@ pub(crate) async fn server_load_ndjson( path = "/branches", tag = "branches", operation_id = "listBranches", + params(api::BranchListQuery), responses( (status = 200, description = "List of branches", body = BranchListOutput), (status = 401, description = "Unauthorized", body = ErrorOutput), @@ -2698,12 +2699,54 @@ pub(crate) async fn server_load_ndjson( /// /// Returns branch names sorted by name in byte order. Read-only. The GQ statement /// `branch list` on `POST /query` runs the same body. +/// +/// `detail=true` adds `details`: for each branch the caller may `read`, its +/// head commit, the branch and commit it was created from, who created it and +/// when, and when it last moved. Each listed branch costs a read of its latest +/// version, and of the version it was created from once it has written a +/// commit; no commit history is read. pub(crate) async fn server_branch_list( Extension(handle): Extension, actor: Option>, + Query(query): Query, ) -> std::result::Result, ApiError> { - let branches = branch_list_body(&handle, actor.as_ref().map(|Extension(actor)| actor)).await?; - Ok(Json(BranchListOutput { branches })) + let actor = actor.as_ref().map(|Extension(actor)| actor); + if !query.detail { + let branches = branch_list_body(&handle, actor).await?; + return Ok(Json(BranchListOutput { + branches, + details: None, + })); + } + authorize_scope_free_read(&handle, actor)?; + let policy = handle.policy.as_deref(); + // A policy failure (a missing bearer, an evaluation error) is not a + // denial: it stops the listing, and its own error answers. + let mut failure = None; + let mut can_read = |branch: &str| { + let request = PolicyRequest { + action: PolicyAction::Read, + branch: Some(branch.to_string()), + target_branch: None, + }; + match authorize(actor, policy, request) { + Ok(Authz::Allowed) => true, + Ok(Authz::Denied(_)) => false, + Err(error) => { + failure.get_or_insert(error); + false + } + } + }; + let listing = handle + .engine + .branch_list_detailed(&mut can_read) + .await + .map_err(ApiError::from_omni)?; + if let Some(error) = failure { + return Err(error); + } + Ok(Json(api::branch_list_output(&listing))) } /// The scope-free `read` decision shared by `GET /branches`, `branch list`, diff --git a/crates/omnigraph-server/src/mcp.rs b/crates/omnigraph-server/src/mcp.rs index 6877b88a3..dca782d82 100644 --- a/crates/omnigraph-server/src/mcp.rs +++ b/crates/omnigraph-server/src/mcp.rs @@ -876,8 +876,12 @@ impl GraphTools { "branches_list" => { let args: GraphArguments = arguments_as(arguments)?; let (handle, actor) = self.graph(&args.graph, actor, &graph)?; - let output = - handlers::server_branch_list(Extension(handle), Some(Extension(actor))).await?; + let output = handlers::server_branch_list( + Extension(handle), + Some(Extension(actor)), + Query(Default::default()), + ) + .await?; bounded_result(&output.0) } "commits_list" => { @@ -1078,8 +1082,12 @@ impl GraphTools { } GraphResource::Branches(name) => { let (handle, actor) = self.graph(&name, actor, &graph)?; - let output = - handlers::server_branch_list(Extension(handle), Some(Extension(actor))).await?; + let output = handlers::server_branch_list( + Extension(handle), + Some(Extension(actor)), + Query(Default::default()), + ) + .await?; Ok(("application/json", pretty(&output.0)?)) } } diff --git a/crates/omnigraph-server/tests/integration/auth_policy.rs b/crates/omnigraph-server/tests/integration/auth_policy.rs index 3c13c682b..d33869927 100644 --- a/crates/omnigraph-server/tests/integration/auth_policy.rs +++ b/crates/omnigraph-server/tests/integration/auth_policy.rs @@ -2276,6 +2276,16 @@ async fn branch_statements_take_their_routes_policy_decision() { assert_forbidden(status, body, "branch list by an actor with no read rule"); let (status, body) = json_response(&app, get_request(&g("/branches"), "reader-token")).await; assert_eq!(status, StatusCode::OK, "{body}"); + // The detailed listing names the actor the create receipt named. + let (status, body) = json_response( + &app, + get_request(&g("/branches?detail=true"), "reader-token"), + ) + .await; + assert_eq!(status, StatusCode::OK, "{body}"); + assert_eq!(body["details"][0]["name"], "feature", "{body}"); + assert_eq!(body["details"][0]["actor_id"], "act-ragnor", "{body}"); + assert_eq!(body["details"][1]["actor_id"], Value::Null, "{body}"); let (status, body) = json_response(&app, get_request(&g("/branches"), "admin-token")).await; assert_forbidden(status, body, "GET /branches by an actor with no read rule"); @@ -2331,12 +2341,14 @@ async fn branch_list_is_denied_by_a_branch_scoped_read_rule_as_the_route_is() { &format!("{statement} under a protected-only read rule"), ); } - let (status, body) = json_response(&app, get_request(&g("/branches"), "team-token")).await; - assert_forbidden( - status, - body, - "GET /branches under a protected-only read rule", - ); + for route in ["/branches", "/branches?detail=true"] { + let (status, body) = json_response(&app, get_request(&g(route), "team-token")).await; + assert_forbidden( + status, + body, + &format!("GET {route} under a protected-only read rule"), + ); + } } const SHOW_SOURCES: [&str; 1] = ["show all;"]; diff --git a/crates/omnigraph-server/tests/integration/data_routes.rs b/crates/omnigraph-server/tests/integration/data_routes.rs index 93ebc8112..73f0b035d 100644 --- a/crates/omnigraph-server/tests/integration/data_routes.rs +++ b/crates/omnigraph-server/tests/integration/data_routes.rs @@ -5349,6 +5349,29 @@ async fn remote_branch_list_create_merge_flow_works() { assert_eq!(change_body["branch"], "feature"); assert_eq!(change_body["affected_nodes"], 1); + // `detail=true` adds what `GET /commits?branch=` would otherwise be read + // for, per branch; the plain listing stays names only. + assert!(list_body.get("details").is_none(), "{list_body}"); + let (status, detailed) = get_json(&app, g("/branches?detail=true")).await; + assert_eq!(status, StatusCode::OK, "{detailed}"); + assert_eq!(detailed["branches"], json!(["feature", "main"])); + let (_, main_commits) = get_json(&app, g("/commits?branch=main")).await; + let main_head = &main_commits["commits"][0]; + let main = &detailed["details"][1]; + assert_eq!(main["name"], "main"); + assert_eq!(&main["head"], main_head); + assert_eq!((&main["from"], &main["fork"]), (&Value::Null, &Value::Null)); + assert_eq!(main["created_at"], Value::Null); + assert_eq!(main["updated_at"], main_head["created_at"]); + let feature = &detailed["details"][0]; + assert_eq!(feature["name"], "feature"); + assert_eq!(feature["head"], change_body["commit"]); + assert_eq!(feature["updated_at"], change_body["commit"]["created_at"]); + assert_eq!(feature["from"], create_body["from"]); + assert_eq!(&feature["fork"], main_head); + assert!(feature["created_at"].as_i64().unwrap() > 0, "{feature}"); + assert_eq!(feature["actor_id"], create_body["actor_id"]); + let read_main_before = QueryRequest { query: fs::read_to_string(fixture("test.gq")).unwrap(), name: Some("get_person".to_string()), diff --git a/crates/omnigraph/src/db/graph_coordinator.rs b/crates/omnigraph/src/db/graph_coordinator.rs index a50377dec..67480f1cd 100644 --- a/crates/omnigraph/src/db/graph_coordinator.rs +++ b/crates/omnigraph/src/db/graph_coordinator.rs @@ -438,7 +438,7 @@ impl GraphCoordinator { self.manifest.native_branch() } - pub(crate) async fn branch_create(&mut self, name: &str) -> Result<()> { + pub(crate) async fn branch_create(&mut self, name: &str, actor: Option<&str>) -> Result<()> { let branch = normalize_branch_name(name)? .ok_or_else(|| OmniError::manifest("cannot create branch 'main'".to_string()))?; @@ -446,7 +446,7 @@ impl GraphCoordinator { // it in two physical phases (shallow clone, then BranchContents); the // manifest coordinator classifies/reclaims a clone-only zombie before // a bounded retry. No graph-lineage branch is created or rolled back. - self.manifest.create_branch(&branch).await + self.manifest.create_branch_as(&branch, actor).await } /// Delete the branch represented by an operation-local post-gate capture. diff --git a/crates/omnigraph/src/db/mod.rs b/crates/omnigraph/src/db/mod.rs index 4764178c9..72aca0647 100644 --- a/crates/omnigraph/src/db/mod.rs +++ b/crates/omnigraph/src/db/mod.rs @@ -16,17 +16,17 @@ pub use commit_graph::{GraphCommit, GraphCommitKind}; pub use graph_coordinator::{ReadTarget, ResolvedTarget, SnapshotId}; pub use manifest::{DatasetEntry, DatasetUpdate}; pub use omnigraph::{ - CleanupPolicyOptions, CollectorCost, CollectorPathSnapshot, CollectorReport, - CollectorRowSummary, DatasetCleanupStats, DatasetOptimizeStats, DatasetRepairStats, - EXPORT_CHUNK_MAX_BYTES, ExportCut, FullTextIndexRebuildResult, GraphCreateReconciliation, - InitOptions, MergeOutcome, MergeResult, Omnigraph, OpenMode, PendingIndex, PreparedGraphCreate, - PreparedSchemaApply, PreparedSchemaSettlement, ReadCheck, RebuiltFullTextIndex, RepairAction, - RepairClassification, RepairOptions, RepairStats, RetainedManifestVersions, - SYSTEM_COLUMNS_PREFLIGHT, SchemaApplyReconciliation, SchemaApplyResult, SchemaApplySettlement, - SchemaContractDigest, SchemaNonPublicationProof, SkipReason, StagingVerdict, - SystemColumnUpgradeFinding, SystemColumnUpgradeOptions, SystemColumnUpgradeOutcome, - SystemColumnUpgradeReport, TableCollectionPlan, UnattachedTagRecord, UnpublishedManifest, - unattached_release_order, + BranchDetail, BranchListing, CleanupPolicyOptions, CollectorCost, CollectorPathSnapshot, + CollectorReport, CollectorRowSummary, DatasetCleanupStats, DatasetOptimizeStats, + DatasetRepairStats, EXPORT_CHUNK_MAX_BYTES, ExportCut, FullTextIndexRebuildResult, + GraphCreateReconciliation, InitOptions, MergeOutcome, MergeResult, Omnigraph, OpenMode, + PendingIndex, PreparedGraphCreate, PreparedSchemaApply, PreparedSchemaSettlement, ReadCheck, + RebuiltFullTextIndex, RepairAction, RepairClassification, RepairOptions, RepairStats, + RetainedManifestVersions, SYSTEM_COLUMNS_PREFLIGHT, SchemaApplyReconciliation, + SchemaApplyResult, SchemaApplySettlement, SchemaContractDigest, SchemaNonPublicationProof, + SkipReason, StagingVerdict, SystemColumnUpgradeFinding, SystemColumnUpgradeOptions, + SystemColumnUpgradeOutcome, SystemColumnUpgradeReport, TableCollectionPlan, + UnattachedTagRecord, UnpublishedManifest, unattached_release_order, }; pub(crate) use omnigraph::{ LogicalBlobValue, RangedExternalBlobs, logical_row_image, row_blob_values, diff --git a/crates/omnigraph/src/db/omnigraph.rs b/crates/omnigraph/src/db/omnigraph.rs index 65c5b8e93..cc0611f70 100644 --- a/crates/omnigraph/src/db/omnigraph.rs +++ b/crates/omnigraph/src/db/omnigraph.rs @@ -3523,7 +3523,7 @@ impl Omnigraph { ) .await .map_err(OmniError::before_effect)?; - source_coord.branch_create(&target).await?; + source_coord.branch_create(&target, actor).await?; self.invalidate_read_caches().await; Ok(()) } @@ -3562,10 +3562,15 @@ impl Omnigraph { }, actor, )?; - self.branch_create_from_impl(target, name).await + self.branch_create_from_impl(target, name, actor).await } - async fn branch_create_from_impl(&self, from: impl Into, name: &str) -> Result<()> { + async fn branch_create_from_impl( + &self, + from: impl Into, + name: &str, + actor: Option<&str>, + ) -> Result<()> { let target = from.into(); let ReadTarget::Branch(branch_name) = target else { return Err(OmniError::manifest( @@ -3614,7 +3619,7 @@ impl Omnigraph { // A locally owned source coordinator cannot be swapped by a concurrent // `branch_create_from`; the ref write is durable whichever handle // issued it. - source_coord.branch_create(&target_branch).await?; + source_coord.branch_create(&target_branch, actor).await?; self.invalidate_read_caches().await; Ok(()) } @@ -3624,6 +3629,35 @@ impl Omnigraph { self.coordinator.read().await.branch_list().await } + /// [`Self::branch_list`] with the details of each branch `can_read` + /// admits: its head, the branch and commit its current lifetime was + /// created from, and when and by whom that lifetime was created. One + /// listing of the branch refs; `can_read` is asked once per branch, before + /// any read of that branch. Each admitted branch costs a read of its + /// latest `__manifest` version and, when it has written a commit, of the + /// version it was created from; never a read of its lineage. A branch + /// deleted after the listing keeps its name and has no details. + pub async fn branch_list_detailed( + &self, + can_read: &mut ReadCheck<'_>, + ) -> Result { + self.ensure_schema_state_valid().await?; + let listing = crate::db::manifest::list_graph_branches_detailed( + self.uri(), + &self.control_session(), + can_read, + ) + .await?; + Ok(BranchListing { + branches: listing.branches, + details: listing + .details + .into_iter() + .map(BranchDetail::from_catalog) + .collect(), + }) + } + pub async fn branch_delete(&self, name: &str) -> Result<()> { self.branch_delete_as(name, None).await } @@ -4589,9 +4623,73 @@ pub(crate) fn schema_for_table_key(catalog: &Catalog, table_key: &str) -> Result } /// A caller's answer to "may it read this branch?", asked once per two-point -/// graph diff side; see [`Omnigraph::diff_page_readable`]. +/// graph diff side ([`Omnigraph::diff_page_readable`]) and once per listed +/// branch ([`Omnigraph::branch_list_detailed`]). pub type ReadCheck<'a> = dyn FnMut(&str) -> bool + Send + 'a; +/// The live graph branches, as [`Omnigraph::branch_list_detailed`] lists them. +#[derive(Debug, Clone)] +pub struct BranchListing { + /// Every live branch, `main` first and the rest by name. + pub branches: Vec, + /// One entry per branch the caller may read, in the order of `branches`. + pub details: Vec, +} + +/// One live graph branch, as [`Omnigraph::branch_list_detailed`] reports it. +/// `from`, `actor_id` and `created_at` describe the branch's creation as its +/// create receipt did. A branch deleted and created again under the same name +/// is a new lifetime: its creation and fork are the new lifetime's. +#[derive(Debug, Clone)] +pub struct BranchDetail { + pub name: String, + /// The commit a read of the branch sees: its newest commit, or `fork` + /// while it has written none. + pub head: GraphCommit, + /// The branch this one was created from, as that branch was named then; + /// it may since have been deleted or recreated. `None` on `main`. + pub from: Option, + /// The head of `from` when this branch was created. Fixed for the + /// branch's lifetime: unlike the merge base, a later merge does not move + /// it. `None` on `main`. + pub fork: Option, + /// The actor that created the branch; `None` on `main` and when the + /// creation recorded no actor. + pub actor_id: Option, + /// When the branch was created, in Unix microseconds recorded to the + /// second; `None` on `main`. + pub created_at: Option, + /// When the head last moved, in Unix microseconds: the time of the + /// branch's newest commit, or `created_at` while it has written none. + pub updated_at: i64, +} + +impl BranchDetail { + fn from_catalog(detail: crate::db::manifest::GraphBranchDetail) -> Self { + let head = crate::db::commit_graph::graph_commit_from_manifest_row(detail.head); + let created_at = detail.created_at_seconds.map(|seconds| { + i64::try_from(seconds) + .unwrap_or(i64::MAX) + .saturating_mul(1_000_000) + }); + let updated_at = match created_at { + Some(created_at) if !detail.head_is_own => created_at, + _ => head.created_at, + }; + Self { + name: detail.name, + head, + from: detail.from, + fork: detail + .fork + .map(crate::db::commit_graph::graph_commit_from_manifest_row), + actor_id: detail.actor_id, + created_at, + updated_at, + } + } +} + /// What one graph diff endpoint names. enum DiffReadTarget { Branch(Box), diff --git a/crates/omnigraph/src/seams/catalog.rs b/crates/omnigraph/src/seams/catalog.rs index 5d3234bcc..c46fdcca0 100644 --- a/crates/omnigraph/src/seams/catalog.rs +++ b/crates/omnigraph/src/seams/catalog.rs @@ -9,6 +9,7 @@ omnigraph_seams::catalog! { crate::blob::BLOB_READ_POST_CAPTURE, crate::branch_control::BRANCH_CREATE_POST_INVENTORY_PRE_NATIVE, crate::branch_control::BRANCH_CREATE_POST_NATIVE, + crate::branch_control::BRANCH_CREATE_PRE_CREATOR, crate::branch_control::BRANCH_DELETE_POST_NATIVE, crate::branch_control::BRANCH_DELETE_POST_ARCHIVE, crate::changes::enumerate::CHANGE_FEED_POST_HEAD_WITNESS, diff --git a/crates/omnigraph/tests/failpoints.rs b/crates/omnigraph/tests/failpoints.rs index a5fe45157..46db45b5a 100644 --- a/crates/omnigraph/tests/failpoints.rs +++ b/crates/omnigraph/tests/failpoints.rs @@ -434,6 +434,56 @@ async fn native_branch_controls_reclassify_lost_acknowledgements() { ); } +// Creation records its actor with a second write of the new ref. A failure +// before that write leaves a usable branch that lists with no actor, and the +// create reports uncertain completion rather than success or a clean refusal. +#[tokio::test] +#[serial] +async fn branch_create_reports_an_unrecorded_creator_as_uncertain() { + let _scenario = FailScenario::setup(); + let dir = tempfile::tempdir().unwrap(); + let db = helpers::init_and_load(&dir).await; + + { + let _fp = catalog::BRANCH_CREATE_PRE_CREATOR.fire_always(); + let error = db + .branch_create_as("feature", Some("act-alice")) + .await + .expect_err("a create whose actor was not recorded must not report success"); + assert_eq!( + error.completion_evidence(), + Some(CompletionEvidence::Uncertain), + "{error}" + ); + } + db.branch_create_as("recorded", Some("act-alice")) + .await + .unwrap(); + helpers::mutate_branch( + &db, + "feature", + MUTATION_QUERIES, + "insert_person", + &mixed_params(&[("$name", "Eve")], &[("$age", 22)]), + ) + .await + .expect("the branch exists and takes writes"); + let listing = db.branch_list_detailed(&mut |_| true).await.unwrap(); + let actors: Vec<_> = listing + .details + .iter() + .map(|detail| (detail.name.as_str(), detail.actor_id.as_deref())) + .collect(); + assert_eq!( + actors, + [ + ("main", None), + ("feature", None), + ("recorded", Some("act-alice")) + ] + ); +} + /// Branch delete leaves the deleted branch's detached staging for `cleanup`: /// a failed collection keeps it and the retry reclaims it. #[tokio::test] diff --git a/crates/omnigraph/tests/forbidden_apis.rs b/crates/omnigraph/tests/forbidden_apis.rs index f2ef70357..c85ad88fc 100644 --- a/crates/omnigraph/tests/forbidden_apis.rs +++ b/crates/omnigraph/tests/forbidden_apis.rs @@ -328,6 +328,7 @@ const READ_ONLY_SURFACES: &[(&str, &str)] = &[ ("db/omnigraph.rs", "graph_index"), ("blob.rs", "read_blob_at"), ("db/omnigraph.rs", "branch_list"), + ("db/omnigraph.rs", "branch_list_detailed"), ("db/omnigraph.rs", "get_commit"), ("db/omnigraph.rs", "commit_target_branch"), ("db/omnigraph.rs", "list_commits"), @@ -661,6 +662,12 @@ const LOW_LEVEL_WRITE_SURFACES: &[(&str, &str, &str, WriteProtocol)] = &[ "create_branch", WriteProtocol::NativeRefControl, ), + ( + "omnigraph-catalog/lib.rs", + "ManifestCoordinator", + "create_branch_as", + WriteProtocol::NativeRefControl, + ), ( "omnigraph-catalog/lib.rs", "ManifestCoordinator", @@ -1070,13 +1077,14 @@ durable_calls! { ("db/omnigraph/optimize.rs", ".update_config(", 1, WriteProtocol::PhysicalOnly), ("db/omnigraph.rs", ".branch_create(", 2, WriteProtocol::NativeRefControl), ("db/omnigraph.rs", ".branch_delete_captured(", 1, WriteProtocol::NativeRefControl), - ("db/graph_coordinator.rs", ".create_branch(", 1, WriteProtocol::NativeRefControl), + ("db/graph_coordinator.rs", ".create_branch_as(", 1, WriteProtocol::NativeRefControl), + ("omnigraph-catalog/lib.rs", ".create_branch_as(", 1, WriteProtocol::NativeRefControl), ("db/graph_coordinator.rs", ".delete_branch_with_expected(", 1, WriteProtocol::NativeRefControl), ("omnigraph-core/branch_control.rs", ".create_branch(", 1, WriteProtocol::Composed("graph/data native refs")), ("omnigraph-core/lance_clone.rs", ".create_branch(", 1, WriteProtocol::Composed("scoped native clone index-origin forwarding")), ("omnigraph-core/lance_clone.rs", ".commit(", 1, WriteProtocol::Composed("Lance commit-handler publication forwarding")), ("omnigraph-core/lance_clone.rs", ".delete(", 1, WriteProtocol::Composed("Lance commit-handler deletion forwarding")), - ("omnigraph-core/branch_control.rs", ".replace_metadata(", 1, WriteProtocol::NativeRefControl), + ("omnigraph-core/branch_control.rs", ".replace_metadata(", 2, WriteProtocol::Composed("retirement marker and the creating actor, each on its exact native ref under the branch gate")), ("omnigraph-core/branch_control.rs", ".put_opts(", 1, WriteProtocol::Composed("create-only exact retirement archive before native ref unlink")), ("omnigraph-core/branch_control.rs", ".delete(", 1, WriteProtocol::Composed("unlink only the freshly validated retired ref after its exact archive is durable")), ("omnigraph-core/branch_control.rs", ".tags(", 1, WriteProtocol::ReadOnlyAccess), @@ -1168,6 +1176,7 @@ const DURABLE_PRIMITIVES: &[&str] = &[ ".branch_delete(", ".branch_delete_captured(", ".create_branch(", + ".create_branch_as(", ".delete_branch(", ".delete_branch_with_expected(", ".replace_metadata(", diff --git a/crates/omnigraph/tests/integration/branch_control_cost.rs b/crates/omnigraph/tests/integration/branch_control_cost.rs index 5aa1f1485..51909fc70 100644 --- a/crates/omnigraph/tests/integration/branch_control_cost.rs +++ b/crates/omnigraph/tests/integration/branch_control_cost.rs @@ -21,7 +21,11 @@ use std::future::Future; -use crate::helpers::cost::{IoCounts, cost_harness, local_graph, measure}; +use crate::helpers::cost::{ + DEEP_HISTORY_DEPTH, IoCounts, assert_flat, cost_harness, local_graph, measure, s3_graph, +}; +use crate::helpers::{MUTATION_QUERIES, mixed_params}; +use omnigraph::db::BranchListing; /// Run an async test body on a thread with a large stack (see module docs). fn on_big_stack(body: impl FnOnce() -> F + Send + 'static) @@ -109,3 +113,111 @@ fn branch_delete_manifest_reads_bounded_per_surviving_branch() { }) }); } + +/// `__manifest` reads `branch_list_detailed` may spend per listed branch that has +/// written a commit: a checkout of its latest version and of the version it was +/// created from, each with one scan of the rows. Measured at exactly 8 requests +/// on an S3 emulator (5 for a branch that has written none); a local store +/// counts 3, because Lance reads local files without the counting wrapper. +const DETAILS_PER_BRANCH_BUDGET: u64 = 8; + +async fn insert_on(db: &omnigraph::Session, branch: &str, name: &str) { + db.mutate( + branch, + MUTATION_QUERIES, + "insert_person", + &mixed_params(&[("$name", name)], &[("$age", 30)]), + ) + .await + .unwrap(); +} + +async fn measured_details(db: &omnigraph::Session) -> (BranchListing, IoCounts) { + let (listing, io) = measure(db.branch_list_detailed(&mut |_| true)).await; + (listing.unwrap(), io) +} + +/// A detailed branch listing reads each listed branch's `__manifest` a bounded +/// number of times, never its lineage: the reads per added branch stay within +/// `DETAILS_PER_BRANCH_BUDGET`, and a branch and main that wrote +/// `DEEP_HISTORY_DEPTH` commits cost what they cost after one. +#[test] +fn branch_list_detailed_reads_bounded_per_branch_and_flat_in_history() { + on_big_stack(|| { + cost_harness(async { + let dir = tempfile::tempdir().unwrap(); + assert_branch_list_detailed_cost(local_graph(&dir).await).await; + }) + }); +} + +/// The same gate on an object store, where every `__manifest` read is a +/// request. Skips without `OMNIGRAPH_S3_TEST_BUCKET`. +#[test] +fn branch_list_detailed_reads_bounded_per_branch_and_flat_in_history_s3() { + on_big_stack(|| { + cost_harness(async { + let Some(db) = s3_graph("branch-details").await else { + eprintln!("skipping: OMNIGRAPH_S3_TEST_BUCKET is not set"); + return; + }; + assert_branch_list_detailed_cost(db).await; + }) + }); +} + +async fn assert_branch_list_detailed_cost(db: omnigraph::Session) { + db.branch_create("unwritten").await.unwrap(); + let (_, before) = measured_details(&db).await; + db.branch_create("unwritten_2").await.unwrap(); + let (_, after) = measured_details(&db).await; + eprintln!( + "one added unwritten branch: LIST DETAILED manifest_reads +{}", + after.manifest_reads - before.manifest_reads + ); + db.branch_delete("unwritten").await.unwrap(); + db.branch_delete("unwritten_2").await.unwrap(); + + let mut curve: Vec<(u64, IoCounts)> = Vec::new(); + let mut created = 0u64; + for n in [2u64, 6] { + while created < n { + let name = format!("b{created}"); + db.branch_create(&name).await.unwrap(); + insert_on(&db, &name, &format!("{name}_0")).await; + created += 1; + } + let (listing, io) = measured_details(&db).await; + assert_eq!(listing.details.len() as u64, n + 1); + eprintln!( + "written branches={n} (+main): LIST DETAILED manifest_reads={} manifest_scans={} \ + internal_opens={}", + io.manifest_reads, io.manifest_scan_count, io.internal_open_count + ); + curve.push((n, io)); + } + let added = curve[1].0 - curve[0].0; + let per_branch = (curve[1].1.manifest_reads - curve[0].1.manifest_reads).div_ceil(added); + assert!( + per_branch <= DETAILS_PER_BRANCH_BUDGET, + "branch_list_detailed read {per_branch} __manifest objects per written branch \ + (budget {DETAILS_PER_BRANCH_BUDGET}): it must read each branch's latest and \ + creation versions, not its lineage", + ); + + for i in 1..DEEP_HISTORY_DEPTH { + insert_on(&db, "b0", &format!("b0_{i}")).await; + insert_on(&db, "main", &format!("main_{i}")).await; + } + let (_, deep) = measured_details(&db).await; + eprintln!( + "after {DEEP_HISTORY_DEPTH} commits on b0 and main: LIST DETAILED manifest_reads={}", + deep.manifest_reads + ); + assert_flat( + &[(1, curve[1].1), (DEEP_HISTORY_DEPTH, deep)], + |io| io.manifest_reads, + SLOPE_SLACK, + "branch_list_detailed __manifest reads against branch history depth", + ); +} diff --git a/crates/omnigraph/tests/integration/branching.rs b/crates/omnigraph/tests/integration/branching.rs index 2653cc708..b978f4d1d 100644 --- a/crates/omnigraph/tests/integration/branching.rs +++ b/crates/omnigraph/tests/integration/branching.rs @@ -1,5 +1,6 @@ use crate::helpers; +use std::collections::BTreeMap; use std::fmt::Write as _; use std::fs; @@ -9,7 +10,7 @@ use lance::Dataset; use lance_index::is_system_index; use omnigraph::db::commit_graph::CommitGraph; -use omnigraph::db::{MergeOutcome, Omnigraph, ReadTarget}; +use omnigraph::db::{BranchDetail, GraphCommit, MergeOutcome, Omnigraph, ReadTarget}; use omnigraph::error::{CompletionEvidence, ManifestErrorKind, MergeConflictKind, OmniError}; use omnigraph::instrumentation::{MergeWriteProbes, with_merge_write_probes}; use omnigraph::loader::LoadMode; @@ -280,6 +281,196 @@ async fn branch_create_open_list_and_lazy_branching_work() { assert_eq!(count_rows(&main, "node:Person").await, 4); } +/// The newest commit `list_commits` reports for `branch`. +async fn newest_commit(db: &Omnigraph, branch: &str) -> GraphCommit { + db.list_commits(Some(branch)).await.unwrap().remove(0) +} + +/// Every branch's detail, keyed by name, from a listing that may read all. +async fn detailed_branches(db: &Omnigraph) -> BTreeMap { + let listing = db.branch_list_detailed(&mut |_| true).await.unwrap(); + let names: Vec<_> = listing.details.iter().map(|d| d.name.clone()).collect(); + assert_eq!(names, listing.branches, "every readable branch has details"); + listing + .details + .into_iter() + .map(|detail| (detail.name.clone(), detail)) + .collect() +} + +fn unix_micros_now() -> i64 { + let elapsed = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap(); + i64::try_from(elapsed.as_micros()).unwrap() +} + +/// A branch's details answer what a client would otherwise derive from its +/// whole commit list: its head, the branch and commit its current lifetime was +/// created from, who created it and when, and when it last moved. The fork is +/// fixed for the lifetime, so a merge moves the merge base but not the fork; +/// deleting and recreating the name starts a new lifetime with its own fork, +/// creator and creation time; cleanup leaves every fork readable. +#[tokio::test] +async fn branch_list_detailed_reports_head_fork_creator_and_lifetime() { + let dir = tempfile::tempdir().unwrap(); + let db = init_and_load(&dir).await; + let insert = |name: &str| mixed_params(&[("$name", name)], &[("$age", 30)]); + let loaded = newest_commit(&db, "main").await; + + let before_create = unix_micros_now() / 1_000_000 * 1_000_000; + db.branch_create_as("feature", Some("act-alice")) + .await + .unwrap(); + let after_create = unix_micros_now(); + let details = detailed_branches(&db).await; + let main = &details["main"]; + assert_eq!(main.head, loaded); + assert_eq!((main.from.as_deref(), main.fork.as_ref()), (None, None)); + assert_eq!((main.actor_id.as_deref(), main.created_at), (None, None)); + assert_eq!(main.updated_at, loaded.created_at); + let unwritten = &details["feature"]; + assert_eq!(unwritten.head, loaded, "an unwritten branch reads its fork"); + assert_eq!( + (unwritten.from.as_deref(), unwritten.fork.as_ref()), + (Some("main"), Some(&loaded)) + ); + assert_eq!(unwritten.actor_id.as_deref(), Some("act-alice")); + let created_at = unwritten.created_at.unwrap(); + assert!( + (before_create..=after_create).contains(&created_at), + "{before_create} <= {created_at} <= {after_create}" + ); + assert_eq!(unwritten.updated_at, created_at); + + mutate_main(&db, MUTATION_QUERIES, "insert_person", &insert("Mia")) + .await + .unwrap(); + mutate_branch( + &db, + "feature", + MUTATION_QUERIES, + "insert_person", + &insert("Eve"), + ) + .await + .unwrap(); + let written = newest_commit(&db, "feature").await; + assert_eq!( + written.parent_commit_id.as_deref(), + Some(loaded.graph_commit_id.as_str()) + ); + db.branch_create_from_as(ReadTarget::branch("feature"), "child", Some("act-bob")) + .await + .unwrap(); + mutate_branch( + &db, + "child", + MUTATION_QUERIES, + "insert_person", + &insert("Kid"), + ) + .await + .unwrap(); + let details = detailed_branches(&db).await; + let feature = &details["feature"]; + assert_eq!(feature.head, written); + assert_eq!(feature.updated_at, written.created_at); + assert_eq!( + feature.fork.as_ref(), + Some(&loaded), + "a written branch keeps the commit it was created at" + ); + let child = &details["child"]; + assert_eq!(child.head, newest_commit(&db, "child").await); + assert_eq!(child.actor_id.as_deref(), Some("act-bob")); + assert_eq!( + (child.from.as_deref(), child.fork.as_ref()), + (Some("feature"), Some(&written)) + ); + + let main_head = newest_commit(&db, "main").await; + db.branch_merge("main", "feature").await.unwrap(); + let merged = newest_commit(&db, "feature").await; + assert_eq!( + merged.merged_parent_commit_id.as_deref(), + Some(main_head.graph_commit_id.as_str()) + ); + let base = db.merge_base("feature", "main").await.unwrap().merge_base; + assert_eq!(base, main_head, "the merge moved the merge base"); + let details = detailed_branches(&db).await; + assert_eq!(details["feature"].head, merged); + assert_eq!( + details["feature"].fork.as_ref(), + Some(&loaded), + "a merge does not move the fork" + ); + + db.branch_delete("feature").await.unwrap(); + db.branch_create("feature").await.unwrap(); + let details = detailed_branches(&db).await; + assert_eq!( + details.keys().collect::>(), + ["child", "feature", "main"] + ); + let recreated = &details["feature"]; + assert_eq!( + (recreated.from.as_deref(), recreated.fork.as_ref()), + (Some("main"), Some(&main_head)) + ); + assert_eq!(recreated.head, main_head); + assert!(recreated.created_at.unwrap() >= created_at); + assert_eq!( + recreated.actor_id, None, + "a new lifetime does not inherit its predecessor's creator" + ); + let child = &details["child"]; + assert_eq!( + (child.from.as_deref(), child.fork.as_ref()), + (Some("feature"), Some(&written)), + "`from` names the source as it was named then" + ); + + mutate_branch( + &db, + "feature", + MUTATION_QUERIES, + "insert_person", + &insert("Ann"), + ) + .await + .unwrap(); + db.cleanup(omnigraph::db::CleanupPolicyOptions { + keep_versions: Some(1), + older_than: None, + }) + .await + .unwrap(); + let details = detailed_branches(&db).await; + assert_eq!(details["feature"].fork.as_ref(), Some(&main_head)); + assert_eq!(details["child"].fork.as_ref(), Some(&written)); + + let mut asked = Vec::new(); + let listing = db + .branch_list_detailed(&mut |name: &str| { + asked.push(name.to_string()); + name != "child" + }) + .await + .unwrap(); + assert_eq!(asked, ["main", "child", "feature"], "asked once per branch"); + assert_eq!(listing.branches, ["main", "child", "feature"]); + assert_eq!( + listing + .details + .iter() + .map(|d| d.name.as_str()) + .collect::>(), + ["main", "feature"], + "a branch the caller may not read lists by name alone" + ); +} + #[tokio::test] async fn explicit_target_query_reads_multiple_branches_from_one_handle() { let dir = tempfile::tempdir().unwrap(); diff --git a/docs/dev/lance.md b/docs/dev/lance.md index ad335a33d..3edbaa0d4 100644 --- a/docs/dev/lance.md +++ b/docs/dev/lance.md @@ -243,6 +243,9 @@ Stock `Branches::get` and `list` include every physical ref. OmniGraph's logical manifest helpers validate and filter `omnigraph.retired_manifest_branch`, a version-1 JSON value binding the exact native name and identifier. An absent marker means live; malformed, unsupported, or mismatched metadata fails closed. +Creation records the creating actor under `omnigraph.created_by_actor_id` with +`replace_metadata` after the native create; it is provenance a detailed branch +listing reports, never authority, and retirement preserves it. The metadata update publishes retirement and preserves unrelated metadata. Before unlinking the active ref, retirement archives the exact `BranchContents` inside its native tree; retries validate the same identity in the ref or archive. diff --git a/docs/dev/writes.md b/docs/dev/writes.md index f2011f116..1be2db8f5 100644 --- a/docs/dev/writes.md +++ b/docs/dev/writes.md @@ -471,6 +471,20 @@ Cached write admission checks the ref's identifier and retirement metadata through the existing lookup, with no additional storage request. Cold branch enumeration reads active refs; creation does not scan retired histories. +A detailed listing (`list_graph_branches_detailed` in `omnigraph-catalog`) +derives every field from the same authority: each live ref's `BranchContents` +(`parent_branch`, `parent_version`, `create_at`, and the creating actor under +the metadata key `omnigraph.created_by_actor_id`), the head row of the +branch's latest `__manifest` version and, once the branch has written a +commit, the head row of its clone at `parent_version`. Creation records the +actor with a second write of the new ref after the native create +(`record_branch_creator`); retirement preserves the key. When that write +fails, read back, the branch exists without an actor and the create reports +uncertain completion. It reads no lineage, so its cost per +branch is flat in history (`branch_control_cost.rs`). No path prunes a live +branch's `__manifest` versions; a reclamation that did would have to keep each +live branch's `parent_version`. + Stable table/incarnation identity, not `table_key`, determines whether a registration, rename, tombstone, or pointer belongs to the same lifetime. diff --git a/docs/user/branching/index.md b/docs/user/branching/index.md index 26c826df8..eed0091a5 100644 --- a/docs/user/branching/index.md +++ b/docs/user/branching/index.md @@ -28,6 +28,36 @@ omnigraph branch list --store graph.omni omnigraph branch delete review/abandoned --store graph.omni ``` +`branch list --detail` (served: `GET /branches?detail=true`, which adds a +`details` array beside `branches`) also reports, for each branch: + +| Field | Meaning | +|---|---| +| `head` | The commit a read of the branch sees: its newest commit, or the commit it was created at while it has written none. | +| `from` | The branch this one was created from, as the create receipt named it; that branch may since have been deleted or recreated. | +| `fork` | The head of `from` when this branch was created. It is fixed for the branch's lifetime, so a later merge moves the merge base (`branch merge-base`) but not the fork. | +| `actor_id` | The actor that created this branch, as the create receipt named it; `null` when the create named no actor, and for branches created before OmniGraph recorded creators. | +| `created_at` | When this branch was created, as Unix microseconds recorded to the second. | +| `updated_at` | When the head last moved: the newest commit's time, or `created_at` while the branch has written none. | + +`main` has no `from`, `fork`, `actor_id` or `created_at`. A branch deleted and created +again under the same name is a new lifetime, so the listing reports the second +creation, its actor and its fork. A create that cannot record its actor +after creating the branch fails with an uncertain outcome; the branch exists +and lists with `actor_id: null`. The details are read from each branch's own manifest, +never from its commit history: a listing reads the latest version of each +branch and, once the branch has written a commit, the version it was created +from, whatever the history depth. The text output prints one line per branch: + +```text +feature head= from=main fork= actor= created_at= updated_at= +main head= updated_at= +``` + +The served listing needs the same `read` as `GET /branches`, and reports +details only for the branches the caller may `read`; another branch appears in +`branches` alone. + Creating a branch defaults to `main` when `--from` is omitted. A load can create a missing target branch by combining `--branch ` with `--from `. diff --git a/docs/user/cli/reference.md b/docs/user/cli/reference.md index eb3c9da5c..74eb5ecf6 100644 --- a/docs/user/cli/reference.md +++ b/docs/user/cli/reference.md @@ -44,7 +44,7 @@ server resolves the actor from the bearer token. Drop it, or use `--store ` | `mutate` | Run an insert/update/delete query (`as commit` leaves it unpublished; `as unattached branch [on ""]` records a snapshot that never publishes), or a `branch create`, `branch delete`, `branch merge`, `commit publish`, `commit drop`, or [`load csv`](../mutations/index.md#csv-loading) statement | direct or served | | `load` | Load graph JSONL in `overwrite`, `append`, or `merge` mode | direct or served | | `blob get/stat/put/clear` | Read, inspect, replace or clear one Blob cell | direct or served | -| `branch create/list/delete/merge/diff/merge-base` | Manage graph branches; `merge --as-commit` leaves the merge unpublished, `merge --dry-run` reports the outcome or conflicts and writes nothing, `diff` reviews what a branch changed since its merge base, `merge-base` prints that base | direct or served | +| `branch create/list/delete/merge/diff/merge-base` | Manage graph branches; `list --detail` adds each branch's head, fork and times, `merge --as-commit` leaves the merge unpublished, `merge --dry-run` reports the outcome or conflicts and writes nothing, `diff` reviews what a branch changed since its merge base, `merge-base` prints that base | direct or served | | `snapshot` | Show a branch snapshot: `internal_schema_version`, `graph_manifest_version`, and the datasets of one captured graph version | direct or served | | `commit list/show/changes/publish/drop` | Inspect history (`list --unpublished` for a branch's unpublished commits, `list --unattached` for its live unattached snapshots; the two flags exclude each other) or one commit's entity changes; publish an unpublished commit onto its recorded target branch (an unattached snapshot refuses with `unattached_cannot_publish`), or drop it (an unattached snapshot takes every descendant with it) | direct or served | | `diff` | Net entity changes between two points: branches or commit ids (`--from`, `--to`) | direct or served | diff --git a/docs/user/operations/server.md b/docs/user/operations/server.md index 376faef13..3ddbaf67c 100644 --- a/docs/user/operations/server.md +++ b/docs/user/operations/server.md @@ -214,7 +214,7 @@ endpoint, its tools, settings and limits. | `/graphs/{id}/queries` | List and invoke stored queries, including conditional mutations | | `/graphs/{id}/load`, `/load/ndjson` | Bounded batch loading | | `/graphs/{id}/blob` | GET/HEAD one Blob cell; PUT/DELETE to replace or clear it | -| `/graphs/{id}/branches` | Branch management and merge, a merge dry run (`dry_run: true`), a branch's diff against its merge base (`GET /branches/diff`) and that merge base (`GET /branches/merge-base`) | +| `/graphs/{id}/branches` | Branch management and merge, a detailed listing (`GET /branches?detail=true`), a merge dry run (`dry_run: true`), a branch's diff against its merge base (`GET /branches/diff`) and that merge base (`GET /branches/merge-base`) | | `/graphs/{id}/snapshot`, `/commits` | Snapshot (`internal_schema_version`, `graph_manifest_version`, and the datasets of one captured graph version), history, per-commit changes, and the unpublished commits of a branch (`GET /commits?unpublished=true`, `POST /commits/{commit_id}/publish`, `DELETE /commits/{commit_id}`) | | `/graphs/{id}/changes`, `/diff` | Poll a branch feed, establish a baseline, or diff two points (branches or commit ids) | | `/graphs/{id}/schema` | Show the accepted schema | diff --git a/openapi.json b/openapi.json index 28ad5fc20..a39d1d0c9 100644 --- a/openapi.json +++ b/openapi.json @@ -2713,7 +2713,7 @@ "branches" ], "summary": "List all branches.", - "description": "Returns branch names sorted by name in byte order. Read-only. The GQ statement\n`branch list` on `POST /query` runs the same body.", + "description": "Returns branch names sorted by name in byte order. Read-only. The GQ statement\n`branch list` on `POST /query` runs the same body.\n\n`detail=true` adds `details`: for each branch the caller may `read`, its\nhead commit, the branch and commit it was created from, who created it and\nwhen, and when it last moved. Each listed branch costs a read of its latest\nversion, and of the version it was created from once it has written a\ncommit; no commit history is read.", "operationId": "cluster_listBranches", "parameters": [ { @@ -2725,6 +2725,15 @@ "type": "string" } }, + { + "name": "detail", + "in": "query", + "description": "`true` adds `details`: each branch's head, the branch and commit it\nwas created from, who created it and when, and when it last moved.\nReads each listed branch's latest version, and the version it was\ncreated from once it has written a commit, never its commit history;\nthe names alone read neither.", + "required": false, + "schema": { + "type": "boolean" + } + }, { "name": "omnigraph-http-api", "in": "header", @@ -9548,6 +9557,64 @@ } } }, + "BranchDetailOutput": { + "type": "object", + "description": "One live branch as `GET /branches?detail=true` reports it. `from`,\n`actor_id` and `created_at` describe the branch's creation as its\n`POST /branches` receipt did. A branch deleted and created again under the\nsame name is a new lifetime: its creation and fork are the later creation's.", + "required": [ + "name", + "head", + "updated_at" + ], + "properties": { + "actor_id": { + "type": [ + "string", + "null" + ], + "description": "The actor that created the branch; `null` on `main` and when the\ncreation recorded no actor." + }, + "created_at": { + "type": [ + "integer", + "null" + ], + "format": "int64", + "description": "When the branch was created, as Unix epoch microseconds recorded to\nthe second; `null` on `main`.", + "example": 1714000000000000 + }, + "fork": { + "oneOf": [ + { + "type": "null" + }, + { + "$ref": "#/components/schemas/CommitOutput", + "description": "The head of `from` when this branch was created. A later merge does\nnot move it; `GET /branches/merge-base` answers where two branches\nlast met. `null` on `main`." + } + ] + }, + "from": { + "type": [ + "string", + "null" + ], + "description": "The branch this one was created from, as that branch was named then;\nit may since have been deleted or recreated. `null` on `main`." + }, + "head": { + "$ref": "#/components/schemas/CommitOutput", + "description": "The commit a read of the branch sees: its newest commit, or `fork`\nwhile it has written none." + }, + "name": { + "type": "string" + }, + "updated_at": { + "type": "integer", + "format": "int64", + "description": "When the head last moved, as Unix epoch microseconds: the time of the\nbranch's newest commit, or `created_at` while it has written none.", + "example": 1714000000000000 + } + } + }, "BranchListOutput": { "type": "object", "required": [ @@ -9558,7 +9625,18 @@ "type": "array", "items": { "type": "string" - } + }, + "description": "Every live branch, sorted by name in byte order." + }, + "details": { + "type": [ + "array", + "null" + ], + "items": { + "$ref": "#/components/schemas/BranchDetailOutput" + }, + "description": "Present only with `detail=true`: one entry per branch the caller may\n`read`, in the order of `branches`. A branch the caller may not read is\nlisted in `branches` alone." } } }, diff --git a/skills/omnigraph/references/commands.md b/skills/omnigraph/references/commands.md index 9b622a2cd..191032605 100644 --- a/skills/omnigraph/references/commands.md +++ b/skills/omnigraph/references/commands.md @@ -69,7 +69,10 @@ omnigraph branch delete --store $REPO All support `--json`. `--from` and `--into` default to `main`. `--delete-branch` removes the source only after a successful merge publication. -A served `branch delete --json` needs `--yes`. +A served `branch delete --json` needs `--yes`. `branch list --detail` adds each +branch's head, the branch (`from`) and commit (`fork`) it was created from, +its creator (`actor_id`), and `created_at`/`updated_at`, without reading +commit history. The same operations are GQ branch statements, useful when a client already sends `.gq` source: From a82600d7a07d4353d78740f2a1efeb49c45ab20a Mon Sep 17 00:00:00 2001 From: Ragnor Comerford Date: Fri, 16 Oct 2026 22:13:25 -0700 Subject: [PATCH 2/4] refactor(branches): address review of the detailed branch listing - One live-branch enumeration (`live_named_branches`) serves both `list_graph_branches` and `list_graph_branches_detailed`, so names and details agree on what is live; `head_is_own` uses `state::exact_head`. - The cost gate pins scans exactly on every backend (one per branch that has written nothing, two per branch that has, flat in history) and a measured read budget per backend (local 1/3, S3 5/8). - The storage-upgrade owner asserts forks served from the legacy history; the branching test pins a fast-forward into an unwritten branch. - The branching guide says the listing is not one cut across branches, that a racing create can list without an actor, and how to confirm an uncertain create. --- crates/omnigraph-catalog/src/branch_detail.rs | 78 ++++++++++------ crates/omnigraph-catalog/src/lib.rs | 21 +---- crates/omnigraph/src/db/upgrade/tests.rs | 35 ++++++++ .../tests/integration/branch_control_cost.rs | 88 ++++++++++++++----- .../omnigraph/tests/integration/branching.rs | 17 ++++ docs/user/branching/index.md | 7 ++ 6 files changed, 177 insertions(+), 69 deletions(-) diff --git a/crates/omnigraph-catalog/src/branch_detail.rs b/crates/omnigraph-catalog/src/branch_detail.rs index 9e8cf67f9..632c43099 100644 --- a/crates/omnigraph-catalog/src/branch_detail.rs +++ b/crates/omnigraph-catalog/src/branch_detail.rs @@ -13,7 +13,40 @@ use crate::branch_control::list_live_manifest_branch_contents; use crate::error::{OmniError, Result}; use crate::history::ExtentCache; use crate::layout::open_manifest_dataset_with_session; -use crate::state::{GraphLineageRow, read_manifest_rows_projected}; +use crate::state::{GraphLineageRow, exact_head, read_manifest_rows_projected}; + +/// One live named graph branch: its logical name and the native ref, with +/// that ref's contents, of its current lifetime. +pub(crate) struct LiveBranch { + pub(crate) name: String, + pub(crate) native: String, + pub(crate) contents: BranchContents, +} + +/// The live named graph branches of a `__manifest`, sorted by name: each live +/// native ref mapped to its one logical name, `main` excluded. A name with +/// more than one live incarnation fails loudly. The one enumeration both +/// branch listings use, so names and details agree on what is live. +pub(crate) async fn live_named_branches(dataset: &Dataset) -> Result> { + let mut named: Vec<_> = list_live_manifest_branch_contents(dataset) + .await? + .into_iter() + .filter(|(native, _)| native != "main") + .map(|(native, contents)| LiveBranch { + name: crate::branch_names::logical_branch_name(&native).to_string(), + native, + contents, + }) + .collect(); + named.sort_by(|left, right| left.name.cmp(&right.name)); + if let Some(pair) = named.windows(2).find(|pair| pair[0].name == pair[1].name) { + return Err(OmniError::manifest_conflict(format!( + "branch '{}' has more than one live native incarnation; run cleanup before using it", + pair[0].name + ))); + } + Ok(named) +} /// How many branches a listing reads at once. const BRANCH_DETAIL_READ_CONCURRENCY: usize = 8; @@ -64,31 +97,17 @@ pub async fn list_graph_branches_detailed( ) -> Result { let root = root_uri.trim_end_matches('/'); let main = open_manifest_dataset_with_session(root, None, control_session).await?; - let mut named = Vec::new(); - for (native, contents) in list_live_manifest_branch_contents(&main).await? { - if native == "main" { - continue; - } - let logical = crate::branch_names::logical_branch_name(&native).to_string(); - named.push((logical, native, contents)); - } - named.sort_by(|left, right| left.0.cmp(&right.0)); - if let Some(pair) = named.windows(2).find(|pair| pair[0].0 == pair[1].0) { - return Err(OmniError::manifest_conflict(format!( - "branch '{}' has more than one live native incarnation; run cleanup before using it", - pair[0].0 - ))); - } - + let named = live_named_branches(&main).await?; let branches = std::iter::once("main".to_string()) - .chain(named.iter().map(|(logical, _, _)| logical.clone())) + .chain(named.iter().map(|branch| branch.name.clone())) .collect(); let mut details = Vec::with_capacity(named.len() + 1); if can_read("main") { + let head = read_manifest_rows_projected(&main).await?.head; details.push(GraphBranchDetail { name: "main".to_string(), - head: read_manifest_rows_projected(&main).await?.head, - head_is_own: true, + head_is_own: exact_head(&head, None).is_some(), + head, from: None, fork: None, actor_id: None, @@ -97,13 +116,13 @@ pub async fn list_graph_branches_detailed( } let admitted: Vec<_> = named .into_iter() - .filter(|(logical, _, _)| can_read(logical)) + .filter(|branch| can_read(&branch.name)) .collect(); let legacy = ExtentCache::default(); let mut reads = futures::stream::iter(admitted) - .map(|(logical, native, contents)| { + .map(|branch| { let (main, legacy) = (&main, &legacy); - async move { branch_detail(root, main, legacy, logical, native, contents).await } + async move { branch_detail(root, main, legacy, branch).await } }) .buffered(BRANCH_DETAIL_READ_CONCURRENCY); while let Some(detail) = reads.next().await { @@ -116,13 +135,16 @@ async fn branch_detail( root: &str, main: &Dataset, legacy: &ExtentCache, - logical: String, - native: String, - contents: BranchContents, + branch: LiveBranch, ) -> Result> { + let LiveBranch { + name: logical, + native, + contents, + } = branch; match read_branch(root, main, legacy, &native, &contents).await { Ok((head, fork_commit)) => { - let head_is_own = head.native_branch.as_deref() == Some(native.as_str()); + let head_is_own = exact_head(&head, Some(&native)).is_some(); let from = contents .parent_branch .as_deref() @@ -163,7 +185,7 @@ async fn read_branch( .await .map_err(OmniError::storage)?; let head = read_manifest_rows_projected(&branch).await?.head; - if head.native_branch.as_deref() != Some(native) { + if exact_head(&head, Some(native)).is_none() { return Ok((head, None)); } let created = branch diff --git a/crates/omnigraph-catalog/src/lib.rs b/crates/omnigraph-catalog/src/lib.rs index 674efb5b5..889750ad8 100644 --- a/crates/omnigraph-catalog/src/lib.rs +++ b/crates/omnigraph-catalog/src/lib.rs @@ -2470,23 +2470,10 @@ impl ManifestCoordinator { /// Logical graph branches, `main` first. Each live native ref maps to /// exactly one logical name; a duplicate incarnation fails loudly. pub async fn list_graph_branches(&self) -> Result> { - let branches = list_live_manifest_branch_contents(&self.dataset).await?; - let mut names = Vec::with_capacity(branches.len()); - let mut seen = HashSet::with_capacity(branches.len()); - for native in branches.keys().filter(|name| *name != "main") { - let logical = crate::branch_names::logical_branch_name(native).to_string(); - if !seen.insert(logical.clone()) { - return Err(OmniError::manifest_conflict(format!( - "branch '{logical}' has more than one live native incarnation; run cleanup \ - before using it" - ))); - } - names.push(logical); - } - names.sort(); - let mut all = vec!["main".to_string()]; - all.extend(names); - Ok(all) + let named = branch_detail::live_named_branches(&self.dataset).await?; + Ok(std::iter::once("main".to_string()) + .chain(named.into_iter().map(|branch| branch.name)) + .collect()) } } diff --git a/crates/omnigraph/src/db/upgrade/tests.rs b/crates/omnigraph/src/db/upgrade/tests.rs index 1a3ac86ea..6693fd2a3 100644 --- a/crates/omnigraph/src/db/upgrade/tests.rs +++ b/crates/omnigraph/src/db/upgrade/tests.rs @@ -521,6 +521,41 @@ async fn assert_converted(source: &Source) { let dataset = open(root, Some(branch)).await.unwrap(); assert_eq!(dataset.version().version, version, "{branch}"); } + // A detailed listing serves each converted branch's fork from the legacy + // history: `feature` wrote its own head, so its fork is read at the + // pre-upgrade version it was created from; `child` was forked from the + // retired `gone`. + let listing = crate::db::manifest::list_graph_branches_detailed( + root, + &crate::lance_access::control_session(), + &mut |_| true, + ) + .await + .unwrap(); + let details: Vec<_> = listing + .details + .iter() + .map(|detail| { + ( + detail.name.as_str(), + detail.from.as_deref(), + detail + .fork + .as_ref() + .map(|fork| fork.graph_commit_id.clone()), + detail.head.graph_commit_id.clone(), + ) + }) + .collect(); + assert_eq!( + details, + [ + ("main", None, None, id(4)), + ("child", Some("gone"), Some(id(3)), id(3)), + ("feature", Some("main"), Some(id(2)), id(6)), + ("fresh", Some("feature"), Some(id(5)), id(5)), + ] + ); assert_eq!( root_objects(source.dir.path()), root_objects_of(source), diff --git a/crates/omnigraph/tests/integration/branch_control_cost.rs b/crates/omnigraph/tests/integration/branch_control_cost.rs index 51909fc70..a2019c0a9 100644 --- a/crates/omnigraph/tests/integration/branch_control_cost.rs +++ b/crates/omnigraph/tests/integration/branch_control_cost.rs @@ -114,12 +114,30 @@ fn branch_delete_manifest_reads_bounded_per_surviving_branch() { }); } -/// `__manifest` reads `branch_list_detailed` may spend per listed branch that has -/// written a commit: a checkout of its latest version and of the version it was -/// created from, each with one scan of the rows. Measured at exactly 8 requests -/// on an S3 emulator (5 for a branch that has written none); a local store -/// counts 3, because Lance reads local files without the counting wrapper. -const DETAILS_PER_BRANCH_BUDGET: u64 = 8; +/// What `branch_list_detailed` may spend per listed branch, from a measured +/// curve. Scans are the same on every backend: one of the branch's latest +/// `__manifest` version, and one of the version it was created from once it has +/// written a commit. Object reads differ by backend, so each pins its own. +struct DetailedListingCost { + /// `__manifest` object reads per branch that has written none. + unwritten_reads: u64, + /// `__manifest` object reads per branch that has written a commit. + written_reads: u64, +} + +/// A local store counts the listing, the ref read and the version resolution of +/// each checkout; Lance reads the local files themselves without the counting +/// wrapper. +const LOCAL_DETAILED_LISTING: DetailedListingCost = DetailedListingCost { + unwritten_reads: 1, + written_reads: 3, +}; + +/// Every read is a request on an object store; measured on an S3 emulator. +const S3_DETAILED_LISTING: DetailedListingCost = DetailedListingCost { + unwritten_reads: 5, + written_reads: 8, +}; async fn insert_on(db: &omnigraph::Session, branch: &str, name: &str) { db.mutate( @@ -137,16 +155,17 @@ async fn measured_details(db: &omnigraph::Session) -> (BranchListing, IoCounts) (listing.unwrap(), io) } -/// A detailed branch listing reads each listed branch's `__manifest` a bounded -/// number of times, never its lineage: the reads per added branch stay within -/// `DETAILS_PER_BRANCH_BUDGET`, and a branch and main that wrote -/// `DEEP_HISTORY_DEPTH` commits cost what they cost after one. +/// A detailed branch listing reads each listed branch's `__manifest` a fixed +/// number of times, never its lineage: one scan per branch that has written +/// none, two per branch that has, within the backend's read budget, and a +/// branch and main that wrote `DEEP_HISTORY_DEPTH` commits cost what they cost +/// after one. #[test] fn branch_list_detailed_reads_bounded_per_branch_and_flat_in_history() { on_big_stack(|| { cost_harness(async { let dir = tempfile::tempdir().unwrap(); - assert_branch_list_detailed_cost(local_graph(&dir).await).await; + assert_branch_list_detailed_cost(local_graph(&dir).await, LOCAL_DETAILED_LISTING).await; }) }); } @@ -161,19 +180,26 @@ fn branch_list_detailed_reads_bounded_per_branch_and_flat_in_history_s3() { eprintln!("skipping: OMNIGRAPH_S3_TEST_BUCKET is not set"); return; }; - assert_branch_list_detailed_cost(db).await; + assert_branch_list_detailed_cost(db, S3_DETAILED_LISTING).await; }) }); } -async fn assert_branch_list_detailed_cost(db: omnigraph::Session) { +async fn assert_branch_list_detailed_cost(db: omnigraph::Session, budget: DetailedListingCost) { db.branch_create("unwritten").await.unwrap(); let (_, before) = measured_details(&db).await; db.branch_create("unwritten_2").await.unwrap(); let (_, after) = measured_details(&db).await; - eprintln!( - "one added unwritten branch: LIST DETAILED manifest_reads +{}", - after.manifest_reads - before.manifest_reads + let (reads, scans) = ( + after.manifest_reads - before.manifest_reads, + after.manifest_scan_count - before.manifest_scan_count, + ); + eprintln!("one added unwritten branch: LIST DETAILED manifest_reads +{reads} scans +{scans}"); + assert_eq!(scans, 1, "a branch that has written none is scanned once"); + assert!( + reads <= budget.unwritten_reads, + "{reads} __manifest reads for a branch that has written none (budget {})", + budget.unwritten_reads ); db.branch_delete("unwritten").await.unwrap(); db.branch_delete("unwritten_2").await.unwrap(); @@ -197,12 +223,18 @@ async fn assert_branch_list_detailed_cost(db: omnigraph::Session) { curve.push((n, io)); } let added = curve[1].0 - curve[0].0; - let per_branch = (curve[1].1.manifest_reads - curve[0].1.manifest_reads).div_ceil(added); + let scans = curve[1].1.manifest_scan_count - curve[0].1.manifest_scan_count; + assert_eq!( + scans, + 2 * added, + "a branch that has written a commit is scanned twice: its latest and its creation \ + version, not its lineage", + ); + let reads = (curve[1].1.manifest_reads - curve[0].1.manifest_reads).div_ceil(added); assert!( - per_branch <= DETAILS_PER_BRANCH_BUDGET, - "branch_list_detailed read {per_branch} __manifest objects per written branch \ - (budget {DETAILS_PER_BRANCH_BUDGET}): it must read each branch's latest and \ - creation versions, not its lineage", + reads <= budget.written_reads, + "branch_list_detailed read {reads} __manifest objects per written branch (budget {})", + budget.written_reads ); for i in 1..DEEP_HISTORY_DEPTH { @@ -211,13 +243,21 @@ async fn assert_branch_list_detailed_cost(db: omnigraph::Session) { } let (_, deep) = measured_details(&db).await; eprintln!( - "after {DEEP_HISTORY_DEPTH} commits on b0 and main: LIST DETAILED manifest_reads={}", - deep.manifest_reads + "after {DEEP_HISTORY_DEPTH} commits on b0 and main: LIST DETAILED manifest_reads={} \ + manifest_scans={}", + deep.manifest_reads, deep.manifest_scan_count ); + let depth_curve = [(1, curve[1].1), (DEEP_HISTORY_DEPTH, deep)]; assert_flat( - &[(1, curve[1].1), (DEEP_HISTORY_DEPTH, deep)], + &depth_curve, |io| io.manifest_reads, SLOPE_SLACK, "branch_list_detailed __manifest reads against branch history depth", ); + assert_flat( + &depth_curve, + |io| io.manifest_scan_count, + 0, + "branch_list_detailed __manifest scans against branch history depth", + ); } diff --git a/crates/omnigraph/tests/integration/branching.rs b/crates/omnigraph/tests/integration/branching.rs index b978f4d1d..882a69294 100644 --- a/crates/omnigraph/tests/integration/branching.rs +++ b/crates/omnigraph/tests/integration/branching.rs @@ -469,6 +469,23 @@ async fn branch_list_detailed_reports_head_fork_creator_and_lifetime() { ["main", "feature"], "a branch the caller may not read lists by name alone" ); + + // A fast-forward into a branch that has written nothing publishes a new + // commit on it: the head and `updated_at` follow that commit, and the fork + // stays where the branch was created. + let created_from = newest_commit(&db, "main").await; + db.branch_create("ff").await.unwrap(); + mutate_main(&db, MUTATION_QUERIES, "insert_person", &insert("Fay")) + .await + .unwrap(); + let outcome = db.branch_merge("main", "ff").await.unwrap(); + assert_eq!(outcome.outcome, MergeOutcome::FastForward); + let fast_forwarded = newest_commit(&db, "ff").await; + let details = detailed_branches(&db).await; + let ff = &details["ff"]; + assert_eq!(ff.head, fast_forwarded); + assert_eq!(ff.fork.as_ref(), Some(&created_from)); + assert_eq!(ff.updated_at, fast_forwarded.created_at); } #[tokio::test] diff --git a/docs/user/branching/index.md b/docs/user/branching/index.md index eed0091a5..8e934b762 100644 --- a/docs/user/branching/index.md +++ b/docs/user/branching/index.md @@ -58,6 +58,13 @@ The served listing needs the same `read` as `GET /branches`, and reports details only for the branches the caller may `read`; another branch appears in `branches` alone. +Each branch is read on its own, so the details are not one cut across +branches: a branch written or created during the listing may show its state +from just before or just after that write. A branch whose create is still +recording its actor lists with `actor_id: null` for that moment. After a create +that reports an uncertain outcome, a retry answers `409` (`already exists`); +list the branches to confirm the branch exists. + Creating a branch defaults to `main` when `--from` is omitted. A load can create a missing target branch by combining `--branch ` with `--from `. From 16b832f54c1bef1e013083466df80aee265279f2 Mon Sep 17 00:00:00 2001 From: Ragnor Comerford Date: Tue, 20 Oct 2026 19:09:16 -0700 Subject: [PATCH 3/4] test(cli): expect the target-kind refusal for branch list under og_catalog --- crates/omnigraph-cli/tests/integration/cli_data.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/omnigraph-cli/tests/integration/cli_data.rs b/crates/omnigraph-cli/tests/integration/cli_data.rs index 4652358df..eeff91ae7 100644 --- a/crates/omnigraph-cli/tests/integration/cli_data.rs +++ b/crates/omnigraph-cli/tests/integration/cli_data.rs @@ -5527,7 +5527,8 @@ fn catalogue_scope_reads_the_binary_registries_without_a_store() { ); let stderr = String::from_utf8_lossy(&listed.stderr); assert!( - stderr.contains("`branch list` is not available under `og_catalog`"), + stderr.contains("target_incompatible") + && stderr.contains("branch list is not available for this target kind and scope"), "{stderr}" ); } From d549bcaab124e41fa43fa3a62911703e77ea939b Mon Sep 17 00:00:00 2001 From: Ragnor Comerford Date: Mon, 26 Oct 2026 11:52:46 +0800 Subject: [PATCH 4/4] feat(mcp): take `detailed` on branches_list as the route and CLI do The graph access RFC gives each operation one JSON contract on the CLI, HTTP and MCP, and #1139 closed the MCP rows. `branch list` now has a detailed form on the route and the CLI, so the tool takes the same `detailed` argument, calls the route's handler with it, and declares `details` in its output schema. The branches resource stays names-only. --- changelog.d/branch-list-details.added.md | 16 +++++------ crates/omnigraph-mcp/src/arguments.rs | 9 ++++++- crates/omnigraph-mcp/src/catalog.rs | 27 ++++++++++++++++--- crates/omnigraph-mcp/src/lib.rs | 9 ++++--- crates/omnigraph-server/src/mcp.rs | 4 ++- .../omnigraph-server/tests/integration/mcp.rs | 5 ++++ docs/user/branching/index.md | 24 ++++++++--------- docs/user/operations/mcp.md | 2 +- 8 files changed, 66 insertions(+), 30 deletions(-) diff --git a/changelog.d/branch-list-details.added.md b/changelog.d/branch-list-details.added.md index 48d02d58e..3801a8b75 100644 --- a/changelog.d/branch-list-details.added.md +++ b/changelog.d/branch-list-details.added.md @@ -1,10 +1,10 @@ -- `GET /graphs/{graph_id}/branches?detailed=true` and `omnigraph branch list - --detailed` report each branch's head commit, the branch and commit it - started from (the `from` and `from_commit_id` its create receipt named), - and when it was created and last moved, beside the names. A client no - longer reads each branch's whole commit list for these: the listing reads - each branch's manifest once, or twice once the branch has written a - commit, whatever its history. A branch deleted and created again reports - its new creation. See [Branches][branch-list-details]. +- `GET /graphs/{graph_id}/branches?detailed=true`, the MCP `branches_list` + tool with `detailed` and `omnigraph branch list --detailed` report each + branch's head commit, the branch and commit it started from (the `from` and + `from_commit_id` its create receipt named), and when it was created and last + moved, beside the names. A client no longer reads each branch's whole commit + list for these: the listing reads each branch's manifest once, or twice once + the branch has written a commit, whatever its history. A branch deleted and + created again reports its new creation. See [Branches][branch-list-details]. [branch-list-details]: ../docs/user/branching/index.md#branch-workflow diff --git a/crates/omnigraph-mcp/src/arguments.rs b/crates/omnigraph-mcp/src/arguments.rs index 1657cede8..3249dab89 100644 --- a/crates/omnigraph-mcp/src/arguments.rs +++ b/crates/omnigraph-mcp/src/arguments.rs @@ -13,6 +13,13 @@ pub struct GraphArguments { } #[derive(Debug, Deserialize)] #[serde(deny_unknown_fields)] +pub struct BranchesListArguments { + pub graph: String, + #[serde(default)] + pub detailed: bool, +} +#[derive(Debug, Deserialize)] +#[serde(deny_unknown_fields)] pub struct BranchArguments { pub graph: String, #[serde(default)] @@ -310,7 +317,7 @@ pub enum ToolCall { Read(ReadArguments), Snapshot(BranchArguments), Access(AccessArguments), - BranchesList(GraphArguments), + BranchesList(BranchesListArguments), CommitsList(CommitsListArguments), CommitsGet(CommitArguments), CommitsChanges(CommitChangesArguments), diff --git a/crates/omnigraph-mcp/src/catalog.rs b/crates/omnigraph-mcp/src/catalog.rs index 62daafaf3..3cf9da32f 100644 --- a/crates/omnigraph-mcp/src/catalog.rs +++ b/crates/omnigraph-mcp/src/catalog.rs @@ -125,8 +125,8 @@ const TOOLS: &[ToolSpec] = &[ ToolSpec { name: "branches_list", title: "List branches", - description: "List a graph's branch names.", - input: graph_schema, + description: "List a graph's branch names. With `detailed`, also each readable branch's head commit, the branch and commit it started from, and when it was created and last moved.", + input: branches_list_schema, output: Some(branches_output_schema), effect: Effect::Read, gate: Gate::Always, @@ -377,6 +377,13 @@ pub(super) fn graph_schema() -> Value { arguments(json!({}), &[]) } +pub(super) fn branches_list_schema() -> Value { + arguments( + json!({"detailed": {"type": "boolean", "description": "`true` adds `details`: for each branch the caller may read, its `head` commit, the branch (`from`) and commit (`from_commit_id`) it started from, and `created_at` and `updated_at`. Defaults to `false`."}}), + &[], + ) +} + pub(super) fn branch_schema() -> Value { arguments( json!({"branch": text_schema("Branch. Defaults to `main`.")}), @@ -674,7 +681,21 @@ pub(super) fn access_output_schema() -> Value { pub(super) fn branches_output_schema() -> Value { json!({ "type": "object", - "properties": {"branches": {"type": "array", "items": {"type": "string"}}}, + "properties": { + "branches": {"type": "array", "items": {"type": "string"}}, + "details": {"type": "array", "items": { + "type": "object", + "properties": { + "name": {"type": "string"}, + "head": {"type": "object"}, + "from": {"type": "string"}, + "from_commit_id": {"type": "string"}, + "created_at": {"type": "integer"}, + "updated_at": {"type": "integer"}, + }, + "required": ["name", "head", "updated_at"], + }}, + }, "required": ["branches"], }) } diff --git a/crates/omnigraph-mcp/src/lib.rs b/crates/omnigraph-mcp/src/lib.rs index baf5af5a5..de3175975 100644 --- a/crates/omnigraph-mcp/src/lib.rs +++ b/crates/omnigraph-mcp/src/lib.rs @@ -13,10 +13,11 @@ mod settings; pub use arguments::{ AccessArguments, BlobClearArguments, BlobStatArguments, BranchArguments, BranchCreateArguments, - BranchDeleteArguments, BranchDiffArguments, BranchMergeArguments, ChangeFilters, - ChangesPollArguments, CommitArguments, CommitChangesArguments, CommitsListArguments, - DiffArguments, GraphArguments, InvalidArguments, LoadArguments, MergeBaseArguments, - MutateArguments, QueryArguments, ReadArguments, ToolCall, ToolRequest, change_query, + BranchDeleteArguments, BranchDiffArguments, BranchMergeArguments, BranchesListArguments, + ChangeFilters, ChangesPollArguments, CommitArguments, CommitChangesArguments, + CommitsListArguments, DiffArguments, GraphArguments, InvalidArguments, LoadArguments, + MergeBaseArguments, MutateArguments, QueryArguments, ReadArguments, ToolCall, ToolRequest, + change_query, }; pub use resources::{ResourceOutput, ResourceRequest}; pub use result::{HostError, bounded_result}; diff --git a/crates/omnigraph-server/src/mcp.rs b/crates/omnigraph-server/src/mcp.rs index cba72a846..a17878472 100644 --- a/crates/omnigraph-server/src/mcp.rs +++ b/crates/omnigraph-server/src/mcp.rs @@ -463,7 +463,9 @@ impl ServerHost { let output = handlers::server_branch_list( Extension(handle), Some(Extension(actor)), - ApiQuery(Default::default()), + ApiQuery(crate::api::BranchListQuery { + detailed: args.detailed, + }), ) .await?; bounded_result(&output.0) diff --git a/crates/omnigraph-server/tests/integration/mcp.rs b/crates/omnigraph-server/tests/integration/mcp.rs index 13e97489e..cd3c3b8b6 100644 --- a/crates/omnigraph-server/tests/integration/mcp.rs +++ b/crates/omnigraph-server/tests/integration/mcp.rs @@ -1086,6 +1086,11 @@ async fn mcp_read_parity_tools_equal_their_routes() { json!({"graph":"default"}), "/graphs/default/branches".to_string(), ), + ( + "branches_list", + json!({"graph":"default","detailed":true}), + "/graphs/default/branches?detailed=true".to_string(), + ), ( "access", json!({"graph":"default"}), diff --git a/docs/user/branching/index.md b/docs/user/branching/index.md index ea2202c20..419b9e707 100644 --- a/docs/user/branching/index.md +++ b/docs/user/branching/index.md @@ -28,18 +28,18 @@ omnigraph branch list --target ./graph.omni omnigraph branch delete review/abandoned --target ./graph.omni ``` -`branch list --detailed` (served: `GET /branches?detailed=true`) adds a -`details` array. Per branch it reports `head`, the commit a read sees; `from` -and `from_commit_id`, the branch and commit it started from, as its create -receipt named them (a later merge moves the merge base, not these); -`created_at`, recorded to the second; and `updated_at`, the newest commit's -time, or `created_at` while the branch has written none. Times are Unix -microseconds. `main` omits `from`, `from_commit_id` and `created_at`. A branch -deleted and created again reports its new creation. Each branch costs a read -of its own manifest, two once it has written a commit, whatever its history, -and the branches are not read as one cut. The served listing needs the `read` -that `GET /branches` needs, and has details only for branches the caller may -`read`. +`branch list --detailed` (served: `GET /branches?detailed=true`; MCP: +`branches_list` with `detailed`) adds a `details` array. Per branch it reports +`head`, the commit a read sees; `from` and `from_commit_id`, the branch and +commit it started from, as its create receipt named them (a later merge moves +the merge base, not these); `created_at`, recorded to the second; and +`updated_at`, the newest commit's time, or `created_at` while the branch has +written none. Times are Unix microseconds. `main` omits `from`, +`from_commit_id` and `created_at`. A branch deleted and created again reports +its new creation. Each branch costs a read of its own manifest, two once it has +written a commit, whatever its history, and the branches are not read as one +cut. The served listing needs the `read` that `GET /branches` needs, and has +details only for branches the caller may `read`. Creating a branch defaults to `main` when `--from` is omitted. To start a branch at a past commit instead of a branch head, use `--from-commit`; see diff --git a/docs/user/operations/mcp.md b/docs/user/operations/mcp.md index 88cf9da8b..fdc7654ad 100644 --- a/docs/user/operations/mcp.md +++ b/docs/user/operations/mcp.md @@ -170,7 +170,7 @@ for a deployment whose policy was written with an LLM caller in mind. | `read` (opt-in) | `graph`, `query`, optional `name`, `params`, `branch`, `snapshot` | `POST /graphs/{graph_id}/query` | `read`; `change` for `explain load csv` | | `snapshot` | `graph`, optional `branch` | `GET /graphs/{graph_id}/snapshot` | `read` | | `access` | `graph`, optional `branch`, `target_branch` | `GET /graphs/{graph_id}/access` | none | -| `branches_list` | `graph` | `GET /graphs/{graph_id}/branches` | `read` | +| `branches_list` | `graph`, optional `detailed` | `GET /graphs/{graph_id}/branches` | `read`; with `detailed`, details only for branches the caller may `read` | | `branches_diff` | `graph`, `source`, optional `target`, `kind`, `type`, `op`, `page_token`, `limit` | `GET /graphs/{graph_id}/branches/diff` | `read` on both branches | | `branches_merge_base` | `graph`, `source`, optional `target` | `GET /graphs/{graph_id}/branches/merge-base` | `read` on both branches | | `commits_list` | `graph`, optional `branch`, `listing`, `limit`, `page_token` | `GET /graphs/{graph_id}/commits` | `read` |