feat: give issues a content-hash genesis key and a promotion-assigned number
commit
58c9ae9feat: give issues a content-hash genesis key and a promotion-assigned number
An issue is a git object, so it always has a content hash; there is no "no origin" case. new_id derives the stable genesis key from that hash (or from an originating object’s id, when the issue derives from one) instead of a counter, so filing an issue never contends anything and a stranger can never collide with another filer. The genesis key is the ref’s last segment and is never renamed, so cross-references stay valid across promotion.
The friendly sequential number is a new id: Option<String> field,
None until promote_with assigns one by CAS-incrementing the shared
refs/meta/issue-number counter with amend (not store): two
promotions racing for the same number must fail closed rather than
have the structural merge treat two identical successor values as
harmlessly equal and let both callers believe they claimed it.
feat: add Store::content_hash for computing an id without touching a repo feat: add issues::new_id deriving the genesis key from an origin or content feat: add Issue::id and issues::promote_with/promote refactor: add issues::load_with/store_with/list_with to thread a Store docs: add the issues.id requirement Assisted-by: Claude:claude-sonnet-5
Reviews
No reviews of this commit yet — record a verdict below.
Start a review
docs/specification.adoc
@@ -339,11 +339,27 @@
--
Each issue MUST be stored at `refs/meta/issues/<id>` as an `Issue` document
with fields: `title`, `body`, `state` (`open` or `closed`), `labels` (plain
-strings, no separate registry), and `author`.
+strings, no separate registry), `author`, and `id` (the friendly number
+described below).
One ref per issue keeps issues independently loadable and separately historied;
the ref's commit chain is the issue's edit history.
--
+[role="requirement", id="issues.id"]
+.Issue Identity
+--
+An issue's stable identifier MUST be a content hash — the object id of the
+originating object, or of the issue's own initial content when it has none
+upstream — and MUST be the issue ref's last path segment, never renamed.
+This identifier is conflict-free and requires no counter.
+An issue MAY additionally carry a friendly sequential number, assigned only
+when a maintainer promotes it; before promotion this number is absent.
+Only promotion advances the number counter, so filing an issue never
+contends it.
+Cross-references (comments, reviews) MUST key off the stable content-hash
+identifier, not the friendly number.
+--
+
=== Web UI
[role="requirement", id="web.server-rendered"]
crates/git-ents/src/issues.rs
@@ -6,6 +6,24 @@
//! chain is the issue's edit history — and labels are plain strings so the index
//! can derive its filter set from whatever labels exist, with no separate label
//! registry to keep in sync.
+//!
+//! # Identity
+//!
+//! An issue carries two identifiers with two different jobs:
+//!
+//! * The **genesis key** — the ref's last segment, computed by [`new_id`] and
+//! never renamed — is the object id of the object the issue derives from (a
+//! review or proposal), or, when it derives from nothing, the hash of the
+//! issue's own initial content. Every issue is a git object, so there is no
+//! "no origin" case. Content-addressed and conflict-free: filing an issue
+//! never contends a counter, and one origin can never file the same issue
+//! twice. Cross-references (comments, reviews) key off this identifier, so
+//! it must never change.
+//! * The **friendly number** — the `id` field, `None` until [`promote_with`]
+//! assigns one — is lifecycle state, not a key. `Option` is the one field
+//! kind `facet-git-tree` auto-defaults on an absent entry, so adding this
+//! field is backward compatible with every issue ref already on disk:
+//! nothing but promotion ever touches the shared counter that assigns it.
use std::path::Path;
@@ -15,6 +33,10 @@
/// `refs/meta/issues/<id>`, per issue.
pub const ISSUES_NS: &str = "refs/meta/issues";
+/// The ref holding the shared friendly-number counter. Only [`promote_with`]
+/// advances it, so filing an issue never contends it.
+pub const ISSUE_NUMBER_REF: &str = "refs/meta/issue-number";
+
/// One issue stored at `refs/meta/issues/<id>`.
#[derive(Debug, Clone, PartialEq, Eq, Facet)]
pub struct Issue {
@@ -28,6 +50,10 @@
pub labels: Vec<String>,
/// The identity that opened the issue.
pub author: String,
+ /// The friendly sequential number [`promote_with`] assigned, or `None`
+ /// before a maintainer promotes the issue. Lifecycle state, not the
+ /// issue's key — the ref's genesis hash is that.
+ pub id: Option<String>,
}
impl Issue {
@@ -38,33 +64,60 @@
}
}
+/// The `refs/meta/issue-number` document: the next friendly number a
+/// promotion will assign.
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Facet)]
+struct IssueNumber {
+ next: u64,
+}
+
+/// Derive an issue's stable genesis key: `origin`'s object id (hex) when the
+/// issue derives from one — one origin, one issue, deduplicated on
+/// provenance — otherwise the hash of the issue's own initial content, since
+/// every issue is a git object and so always has one.
+pub fn new_id(origin: Option<&str>, content: &Issue) -> Result<String, git_store::Error> {
+ match origin {
+ Some(origin) => Ok(origin.to_owned()),
+ None => git_store::content_hash(content),
+ }
+}
+
+/// Load the issue named `id` from an already-open `store`.
+pub fn load_with(store: &git_store::Store, id: &str) -> Result<Option<Issue>, git_store::Error> {
+ store.load_item(ISSUES_NS, id)
+}
+
/// Load the issue recorded at `refs/meta/issues/<id>` in `repo`, or `None` when
/// no such issue exists.
pub fn load(repo: &Path, id: &str) -> Result<Option<Issue>, git_store::Error> {
- git_store::Store::open(repo)?.load::<Issue>(&format!("{ISSUES_NS}/{id}"))
+ load_with(&git_store::Store::open(repo)?, id)
}
-/// Write `issue` to `refs/meta/issues/<id>`, replacing any existing value, as a
-/// new commit so the ref's commit chain is the issue's edit history.
+/// Write `issue` to `refs/meta/issues/<id>` through an already-open `store`,
+/// replacing any existing value as a new commit so the ref's commit chain is
+/// the issue's edit history.
+pub fn store_with(
+ store: &git_store::Store,
+ id: &str,
+ issue: &Issue,
+) -> Result<(), git_store::Error> {
+ store.store_item(ISSUES_NS, id, issue, "Update issue")
+}
+
+/// Write `issue` to `refs/meta/issues/<id>`. See [`store_with`].
pub fn store(repo: &Path, id: &str, issue: &Issue) -> Result<(), git_store::Error> {
- git_store::Store::open(repo)?.store(&format!("{ISSUES_NS}/{id}"), issue, "Update issue")?;
- Ok(())
+ store_with(&git_store::Store::open(repo)?, id, issue)
}
-/// List every issue as `(id, issue)` pairs, newest issue ref first.
+/// List every issue from an already-open `store` as `(id, issue)` pairs,
+/// newest issue ref first.
+pub fn list_with(store: &git_store::Store) -> Result<Vec<(String, Issue)>, git_store::Error> {
+ store.list_items(ISSUES_NS)
+}
+
+/// List every issue in `repo` as `(id, issue)` pairs, newest issue ref first.
pub fn list(repo: &Path) -> Result<Vec<(String, Issue)>, git_store::Error> {
- let store = git_store::Store::open(repo)?;
- let prefix = format!("{ISSUES_NS}/");
- let mut issues = Vec::new();
- for refname in store.list(&prefix)? {
- let Some(id) = refname.strip_prefix(&prefix) else {
- continue;
- };
- if let Some(issue) = store.load::<Issue>(&refname)? {
- issues.push((id.to_owned(), issue));
- }
- }
- Ok(issues)
+ list_with(&git_store::Store::open(repo)?)
}
/// The number of open issues in `repo`.
@@ -75,6 +128,65 @@
.count())
}
+/// Why [`promote_with`] could not promote an issue.
+#[derive(Debug, thiserror::Error)]
+pub enum PromoteError {
+ /// The underlying store failed to read or write a ref.
+ #[error(transparent)]
+ Store(#[from] git_store::Error),
+ /// No issue is recorded at `id`.
+ #[error("no issue at {0:?}")]
+ NotFound(String),
+}
+
+/// How many times [`promote_with`] retries the counter CAS before giving up.
+/// Bounds retry under sustained contention; ordinary races resolve in one or
+/// two rounds.
+const MAX_PROMOTE_RETRIES: usize = 5;
+
+/// Promote the issue at the stable genesis key `id`: allocate the next
+/// friendly number by CAS-incrementing [`ISSUE_NUMBER_REF`], then write it
+/// into the issue's `id` field as a new commit on the *same* ref — the ref is
+/// never renamed, so every cross-reference keyed off it still resolves.
+///
+/// The counter is advanced with [`Store::amend`](git_store::Store::amend),
+/// not [`Store::store`](git_store::Store::store): two promotions racing for
+/// the same number must never both succeed by merging, since a structural
+/// merge would consider two identical successor values equal and let both
+/// callers believe they claimed it. A CAS conflict here is retried by
+/// re-reading the counter, so the number handed back is always the one
+/// actually reserved for this call.
+pub fn promote_with(store: &git_store::Store, id: &str) -> Result<String, PromoteError> {
+ let mut number = None;
+ for _ in 0..=MAX_PROMOTE_RETRIES {
+ let current = store
+ .load::<IssueNumber>(ISSUE_NUMBER_REF)?
+ .unwrap_or(IssueNumber { next: 1 });
+ let next = IssueNumber {
+ next: current.next.saturating_add(1),
+ };
+ match store.amend(ISSUE_NUMBER_REF, &next, "Allocate issue number") {
+ Ok(()) => {
+ number = Some(current.next);
+ break;
+ }
+ Err(git_store::Error::Conflict) => continue,
+ Err(error) => return Err(error.into()),
+ }
+ }
+ let number = number.ok_or(git_store::Error::Conflict)?.to_string();
+
+ let mut issue = load_with(store, id)?.ok_or_else(|| PromoteError::NotFound(id.to_owned()))?;
+ issue.id = Some(number.clone());
+ store_with(store, id, &issue)?;
+ Ok(number)
+}
+
+/// Promote the issue at `id` in `repo`. See [`promote_with`].
+pub fn promote(repo: &Path, id: &str) -> Result<String, PromoteError> {
+ promote_with(&git_store::Store::open(repo)?, id)
+}
+
#[cfg(test)]
mod tests {
#![allow(
@@ -97,6 +209,7 @@
state: state.to_owned(),
labels: labels.iter().map(|l| (*l).to_owned()).collect(),
author: "alice".to_owned(),
+ id: None,
}
}
@@ -150,4 +263,64 @@
);
let _ = std::fs::remove_dir_all(&repo);
}
+
+ #[test]
+ fn new_id_uses_the_origin_when_one_is_given() {
+ let content = issue("A bug", "open", &[]);
+ assert_eq!(new_id(Some("deadbeef"), &content).unwrap(), "deadbeef");
+ }
+
+ #[test]
+ fn new_id_hashes_its_own_content_with_no_origin() {
+ let a = issue("A bug", "open", &[]);
+ let b = issue("A different bug", "open", &[]);
+ let a_id = new_id(None, &a).unwrap();
+ let b_id = new_id(None, &b).unwrap();
+ // Content-addressed: same content yields the same id, different
+ // content yields a different one, with no counter involved.
+ assert_eq!(a_id, new_id(None, &a).unwrap());
+ assert_ne!(a_id, b_id);
+ }
+
+ #[test]
+ fn filing_an_issue_leaves_its_friendly_number_unset() {
+ let repo = unique_repo();
+ let content = issue("A bug", "open", &[]);
+ let id = new_id(None, &content).unwrap();
+ store(&repo, &id, &content).unwrap();
+ assert_eq!(load(&repo, &id).unwrap().unwrap().id, None);
+ let _ = std::fs::remove_dir_all(&repo);
+ }
+
+ #[test]
+ fn promotion_assigns_a_number_and_advances_the_counter_without_renaming_the_ref() {
+ let repo = unique_repo();
+ let content = issue("A bug", "open", &[]);
+ let id = new_id(None, &content).unwrap();
+ store(&repo, &id, &content).unwrap();
+
+ let first = promote(&repo, &id).unwrap();
+ assert_eq!(first, "1");
+ let promoted = load(&repo, &id).unwrap().unwrap();
+ assert_eq!(promoted.id, Some("1".to_owned()));
+
+ // A second issue promotes to the next number; the first issue's ref
+ // — keyed by its stable genesis hash — still resolves.
+ let other = issue("Another bug", "open", &[]);
+ let other_id = new_id(None, &other).unwrap();
+ store(&repo, &other_id, &other).unwrap();
+ assert_eq!(promote(&repo, &other_id).unwrap(), "2");
+ assert_eq!(load(&repo, &id).unwrap().unwrap().id, Some("1".to_owned()));
+ let _ = std::fs::remove_dir_all(&repo);
+ }
+
+ #[test]
+ fn promoting_an_absent_issue_fails() {
+ let repo = unique_repo();
+ assert!(matches!(
+ promote(&repo, "missing"),
+ Err(PromoteError::NotFound(id)) if id == "missing"
+ ));
+ let _ = std::fs::remove_dir_all(&repo);
+ }
}
crates/git-store/src/lib.rs
@@ -99,6 +99,17 @@
fn id(&self) -> &str;
}
+/// The content-addressed object id `value` would serialize to, as a hex
+/// string — computed against a throwaway in-memory object store, so it is
+/// available before deciding whether (or where) a repository should hold it.
+/// A genesis key (an issue's or a comment's stable id) is exactly this: the
+/// hash of the content that originates it, needing no counter and no ref to
+/// already exist.
+pub fn content_hash<T: for<'a> Facet<'a>>(value: &T) -> Result<String, Error> {
+ let (oid, _store) = facet_git_tree::serialize(value)?;
+ Ok(oid.to_string())
+}
+
/// A repository's typed `refs/meta/*` store.
///
/// Refs are read and updated through the high-level [`gix`] API, while all