Radish alpha
h
heartwood
Radicle Heartwood Protocol & Stack
Radicle
Git (anonymous pull)
Log in to clone via SSH
Commits ›
REVIEW: Introduce Reference enum for symbolic ref targets
Fintan Halpenny committed 21 days ago
commit c8e494dc77039b29305efa8334d0717b94038a11
parent 25ff38eb11b6ab3d0fcebcff9d639f17f2439e08
7 files changed +504 -148
modified crates/radicle-node/src/worker/fetch.rs
@@ -342,7 +342,7 @@ fn set_canonical_refs(
    let crefs = identity.doc().canonical_refs()?;



    for (name, target) in crefs.symbolic().iter() {
- 
        if let Err(e) = repo.set_symbolic_ref(name, target, LOG_MESSAGE) {
+ 
        if let Err(e) = repo.set_symbolic_ref(name, target.as_refstr(), LOG_MESSAGE) {

            log::warn!(

                target: "worker",

                "Failed to set canonical symbolic reference '{name}' → '{target}': {e}"
modified crates/radicle/src/git/canonical/protect.rs
@@ -57,6 +57,20 @@ impl<T: RefLike> AsRef<T> for Unprotected<T> {
    }

}
+ 
impl<T: RefLike> std::borrow::Borrow<T> for Unprotected<T> {
+ 
    fn borrow(&self) -> &T {
+ 
        &self.0
+ 
    }
+ 
}
+
+ 
/// Enables looking up entries in a map keyed by `Unprotected<RefString>` using
+ 
/// a `&RefStr`.
+ 
impl std::borrow::Borrow<crate::git::fmt::RefStr> for Unprotected<crate::git::fmt::RefString> {
+ 
    fn borrow(&self) -> &crate::git::fmt::RefStr {
+ 
        self.0.as_ref()
+ 
    }
+ 
}
+ 


impl<'de, T: RefLike + serde::Deserialize<'de>> serde::Deserialize<'de> for Unprotected<T> {

    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>

    where
modified crates/radicle/src/git/canonical/symbolic.rs
@@ -6,14 +6,126 @@ use serde::{Deserialize, Serialize};
use thiserror::Error;



use crate::git::fmt::Qualified;
+ 
use crate::git::fmt::RefStr;

use crate::git::fmt::RefString;



use super::protect::Unprotected;
- 
use reachability::reachable;
-
+ 
/// A type alias for a [`RefString`] that has yet to be validated into a
+ 
/// a symbolic reference name.

pub type RawName = RefString;
+ 
/// A type alias for a [`RefString`] that has yet to be validated into a
+ 
/// symbolic reference target.
+ 
pub type RawTarget = RefString;
+
+ 
/// The target of a symbolic reference.
+ 
///
+ 
/// A target is either:
+ 
/// - [`Direct`](Target::Direct): a concrete qualified reference
+ 
///   (e.g. `refs/heads/main`).
+ 
/// - [`Symbolic`](Target::Symbolic): another symbolic reference name
+ 
///   (e.g. `MAIN`) that must itself resolve through the chain.
+ 
#[derive(Clone, Debug, PartialEq, Eq)]
+ 
pub enum Target {
+ 
    /// A concrete qualified reference — the end of a chain.
+ 
    Direct(Direct),
+ 
    /// Another symbolic reference name — an intermediate link.
+ 
    Symbolic(Symbolic),
+ 
}
+
+ 
impl AsRef<RefStr> for Target {
+ 
    fn as_ref(&self) -> &RefStr {
+ 
        match self {
+ 
            Target::Direct(direct) => direct.0.as_ref(),
+ 
            Target::Symbolic(symbolic) => symbolic.0.as_ref(),
+ 
        }
+ 
    }
+ 
}
+
+ 
/// A concrete qualified reference — the end of a chain.
+ 
// `Unprotected` has `super` visibility, so `Direct` is used to allow `Target`
+ 
// to be `pub`.
+ 
#[derive(Clone, Debug, PartialEq, Eq)]
+ 
pub struct Direct(Unprotected<Qualified<'static>>);
+
+ 
impl Direct {
+ 
    fn to_ref_string(&self) -> Unprotected<RefString> {
+ 
        self.0.to_ref_string()
+ 
    }
+ 
}
+
+ 
impl PartialEq<RefString> for Direct {
+ 
    fn eq(&self, other: &RefString) -> bool {
+ 
        **self.0.as_ref() == **other
+ 
    }
+ 
}
+
+ 
impl AsRef<Qualified<'static>> for Direct {
+ 
    fn as_ref(&self) -> &Qualified<'static> {
+ 
        self.0.as_ref()
+ 
    }
+ 
}
+
+ 
/// A concrete qualified reference — the end of a chain.
+ 
// `Unprotected` has `super` visibility, so `Symbolic` is used to allow `Target`
+ 
// to be `pub`.
+ 
#[derive(Clone, Debug, PartialEq, Eq)]
+ 
pub struct Symbolic(Unprotected<RefString>);
+
+ 
impl AsRef<RefString> for Symbolic {
+ 
    fn as_ref(&self) -> &RefString {
+ 
        self.0.as_ref()
+ 
    }
+ 
}
+
+ 
impl Target {
+ 
    /// Returns the underlying reference as a `&RefStr`.
+ 
    pub fn as_refstr(&self) -> &RefStr {
+ 
        match self {
+ 
            Target::Direct(q) => q.as_ref().as_ref(),
+ 
            Target::Symbolic(s) => s.as_ref().as_ref(),
+ 
        }
+ 
    }
+
+ 
    fn direct(d: Unprotected<Qualified<'static>>) -> Self {
+ 
        Self::Direct(Direct(d))
+ 
    }
+
+ 
    fn symbolic(s: Unprotected<RefString>) -> Self {
+ 
        Self::Symbolic(Symbolic(s))
+ 
    }
+
+ 
    /// Classify an [`Unprotected<RefString>`] as either
+ 
    /// [`Direct`](Target::Direct) or [`Symbolic`](Target::Symbolic)
+ 
    /// based on whether it is [`Qualified`].
+ 
    ///
+ 
    /// The [`Unprotected`] proof is preserved in the resulting variant.
+ 
    fn classify(s: Unprotected<RefString>) -> Self {
+ 
        match Qualified::from_refstr(s.as_ref()) {
+ 
            // Safety: the Qualified is derived from an Unprotected string,
+ 
            // so it is also unprotected.
+ 
            Some(q) => Target::direct(
+ 
                Unprotected::new(q.to_owned())
+ 
                    .expect("qualified derived from unprotected is unprotected"),
+ 
            ),
+ 
            None => Target::symbolic(s),
+ 
        }
+ 
    }
+ 
}
+
+ 
impl Serialize for Target {
+ 
    fn serialize<S: serde::Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
+ 
        serializer.serialize_str(self.as_refstr().as_str())
+ 
    }
+ 
}
+
+ 
impl std::fmt::Display for Target {
+ 
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ 
        f.write_str(self.as_refstr().as_str())
+ 
    }
+ 
}
+ 


/// Names of symbolic references are unprotected references.

/// Requiring [`Unprotected`] makes symbolic references that link *from*

/// protected references unrepresentable.
@@ -31,16 +143,14 @@ impl std::cmp::PartialOrd for Name {
    }

}
- 
pub type RawTarget = RefString;
-
- 
/// Targets for symbolic references are unprotected references.
- 
/// Requiring [`Unprotected`] makes symbolic references that link *to*
- 
/// protected references unrepresentable.
- 
pub(super) type Target = Unprotected<RefString>;
- 


/// Maintains a cycle-free set of symbolic references.

/// Note that dangling references are not detected.

///
+ 
/// Internally, targets are stored as [`Target`], which distinguishes
+ 
/// direct (qualified) targets from symbolic (intermediate) ones. This
+ 
/// means resolution and cycle-checking can pattern-match on the variant
+ 
/// rather than re-parsing the string.
+ 
///

/// # Deserialization Order

///

/// Deserialization validates entries in iteration (insertion) order via
@@ -53,39 +163,44 @@ pub(super) type Target = Unprotected<RefString>;
/// While JSON objects are nominally unordered (RFC 8259 §4), `serde_json`

/// with `IndexMap` preserves insertion order. Any serializer producing

/// this JSON must preserve key order for valid round-tripping.
- 
#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)]
- 
#[serde(try_from = "IndexMap<Name, Target>")]
+ 
#[derive(Clone, Debug, Default, PartialEq, Eq, Deserialize, Serialize)]
+ 
#[serde(try_from = "IndexMap<Name, Unprotected<RefString>>")]

pub struct SymbolicRefs(IndexMap<Name, Target>);



/// Read-only access.

impl SymbolicRefs {
- 
    /// Returns an iterator over all contained symbolic references, as pairs of
- 
    /// their name [`RawName`] and [`RawTarget`].
- 
    pub fn iter(&self) -> impl Iterator<Item = (&RawName, &RawTarget)> {
- 
        self.0
- 
            .iter()
- 
            .map(|(name, target)| (name.as_ref(), target.as_ref()))
+ 
    /// Returns an iterator over all contained symbolic references, as pairs
+ 
    /// of their name and [`Target`].
+ 
    pub fn iter(&self) -> impl Iterator<Item = (&RawName, &Target)> {
+ 
        self.0.iter().map(|(name, target)| (name.as_ref(), target))

    }
- 
    /// Returns an iterator over all contained symbolic references, as pairs of
- 
    /// their name [`RawName`] and resolved [`RawTarget`].
- 
    pub fn iter_resolved(&self) -> impl Iterator<Item = (&RawName, &RawTarget)> {
- 
        self.iter_resolved_unprotected()
- 
            .map(|(name, target)| (name.as_ref(), target.as_ref()))
+ 
    /// Returns an iterator over all contained symbolic references that
+ 
    /// resolve to a direct (qualified) target. The yielded target is the
+ 
    /// final [`Qualified`] reference after chasing through any intermediate
+ 
    /// symbolic references.
+ 
    pub fn iter_resolved(&self) -> impl Iterator<Item = (&RawName, &Qualified<'static>)> {
+ 
        self.0.keys().filter_map(|name| {
+ 
            self.resolve(name.as_ref())
+ 
                .map(|target| (name.as_ref(), target))
+ 
        })

    }
- 
    pub(super) fn iter_resolved_unprotected(&self) -> impl Iterator<Item = (&Name, &Target)> {
- 
        self.0
- 
            .keys()
- 
            .filter_map(|name| self.resolve_unprotected(name).map(|target| (name, target)))
- 
    }
-
- 
    fn resolve_unprotected<'a>(&'a self, name: &Name) -> Option<&'a Target> {
- 
        let mut target = self.0.get(name)?;
- 
        while let Some(next) = self.0.get(target) {
- 
            target = next;
+ 
    /// Resolve a name through the chain of symbolic references until a
+ 
    /// [`Target::Direct`] target is reached. Returns `None` if the
+ 
    /// name is not in the map or if the chain dangles (ends at a
+ 
    /// [`Target::Symbolic`] whose name is not a key).
+ 
    fn resolve(&self, name: &RefString) -> Option<&Qualified<'static>> {
+ 
        let mut current = self.0.get(name)?;
+ 
        loop {
+ 
            match current {
+ 
                Target::Direct(q) => return Some(q.as_ref()),
+ 
                Target::Symbolic(s) => match self.0.get(s.as_ref()) {
+ 
                    Some(next) => current = next,
+ 
                    None => return None,
+ 
                },
+ 
            }

        }
- 
        Some(target)

    }



    /// Returns `true` if the set of symbolic references is empty.
@@ -110,67 +225,64 @@ impl SymbolicRefs {
        result

    }
- 
    /// Convenience method to get the target of the `HEAD` reference.
+ 
    /// Convenience method to get the resolved target of the `HEAD` reference.
+ 
    /// Returns the final [`Qualified`] reference after chasing the chain.

    /// See also [`SymbolicRefs::head`].
- 
    pub fn resolve_head(&self) -> Option<&RawTarget> {
- 
        self.resolve_unprotected(&Unprotected::head())
- 
            .map(|target| target.as_ref())
+ 
    pub fn resolve_head(&self) -> Option<&Qualified<'static>> {
+ 
        self.resolve(Unprotected::head().as_ref())

    }

}



#[derive(Debug, Error)]

pub enum InsertionError {

    #[error("inserting symbolic reference '{name} → {target}' would create a cycle")]
- 
    Cyclic { name: RawName, target: RawTarget },
+ 
    Cyclic { name: RawName, target: RawName },



    #[error(

        "inserting symbolic reference '{name} → {target}' would result in a symbolic reference targeting an unqualified reference"

    )]
- 
    TargetNotQualified { name: RawName, target: RawTarget },
+ 
    TargetNotQualified { name: RawName, target: RawName },

}



/// Mutability.

impl SymbolicRefs {
- 
    /// Insert a symbolic reference.
- 
    /// Even though this method will never return [`InsertionError::Protected`]
- 
    /// we opt to return that (slightly more general) error, as it allows
- 
    /// construction of [`InsertionError::Cyclic`] by consuming `name` and
- 
    /// `target`, avoiding an early copy in [`Self::try_insert`].
+ 
    /// Insert a symbolic reference from the given `name` to the given `target`.
+ 
    ///
+ 
    /// Internally, this classifies the `target` into the [`Target`] and
+ 
    /// delegates the insertion.

    pub(super) fn try_insert_unprotected(

        &mut self,

        name: Name,
- 
        target: Target,
+ 
        target: Unprotected<RefString>,

    ) -> Result<(), InsertionError> {
- 
        if reachable(&self.0, &target, &name) {
- 
            return Err(InsertionError::Cyclic {
- 
                name: name.into_inner(),
- 
                target: target.into_inner(),
- 
            });
- 
        }
-
- 
        let target_is_qualified = Qualified::from_refstr(target.as_ref()).is_some();
+ 
        self.insert(name, Target::classify(target))
+ 
    }
- 
        if !target_is_qualified {
- 
            match self.resolve_unprotected(&target) {
- 
                Some(end) => {
- 
                    if Qualified::from_refstr(end.as_ref()).is_none() {
- 
                        return Err(InsertionError::TargetNotQualified {
- 
                            name: name.into_inner(),
- 
                            target: target.into_inner(),
- 
                        });
+ 
    /// Check whether `name` is reachable from `start` by chasing through
+ 
    /// the map. Used to detect cycles before insertion.
+ 
    ///
+ 
    /// Unlike [`resolve`](SymbolicRefs::resolve), this chases through
+ 
    /// *all* entries regardless of whether the target is [`Direct`](Target::Direct)
+ 
    /// or [`Symbolic`](Target::Symbolic), since a qualified ref string can
+ 
    /// also be a key in the map and thus part of a cycle.
+ 
    fn is_reachable_from(&self, start: &RefString, name: &Name) -> bool {
+ 
        let name = name.as_ref();
+ 
        if start == name {
+ 
            return true;
+ 
        }
+ 
        let mut current: &RefStr = start;
+ 
        loop {
+ 
            match self.0.get(current) {
+ 
                None => return false,
+ 
                Some(target) => {
+ 
                    let next = target.as_refstr();
+ 
                    if *next == **name {
+ 
                        return true;

                    }
- 
                }
- 
                None => {
- 
                    return Err(InsertionError::TargetNotQualified {
- 
                        name: name.into_inner(),
- 
                        target: target.into_inner(),
- 
                    });
+ 
                    current = next;

                }

            }

        }
-
- 
        self.0.insert(name, target);
- 
        Ok(())

    }



    /// Try to insert a symbolic reference.
@@ -191,72 +303,87 @@ impl SymbolicRefs {
    /// Consume `other` by iteratively inserting into self.

    pub fn combine(&mut self, other: SymbolicRefs) -> Result<(), InsertionError> {

        for (name, target) in other.0 {
- 
            self.try_insert_unprotected(name, target)?;
+ 
            self.insert(name, target)?;

        }

        Ok(())

    }
- 
}
-
- 
impl TryFrom<IndexMap<Name, Target>> for SymbolicRefs {
- 
    type Error = InsertionError;
-
- 
    fn try_from(map: IndexMap<Name, Target>) -> Result<Self, Self::Error> {
- 
        map.into_iter()
- 
            .try_fold(Self::default(), |mut result, (name, target)| {
- 
                result.try_insert_unprotected(name, target)?;
- 
                Ok(result)
- 
            })
- 
    }
- 
}
-
- 
mod reachability {
- 
    pub(super) trait Get<'a, 'b, K, V> {
- 
        fn get(&'a self, key: &'b K) -> Option<&'a V>;
- 
    }
- 
    impl<'a, 'b, K: Eq + std::hash::Hash, V> Get<'a, 'b, K, V> for indexmap::IndexMap<K, V> {
- 
        fn get(&'a self, key: &'b K) -> Option<&'a V> {
- 
            indexmap::IndexMap::get(self, key)
- 
        }
- 
    }
-
- 
    /// A reachability check linking
- 
    /// from `K` to `V` using [`Get`], and
- 
    /// from `V` to `K` using [`Into`].
- 
    /// Note that the bound is trivial if `K = V`.
+ 
    /// Insert a symbolic reference from the given `name` to the given `target`.

    ///
- 
    /// This can be used to check whether inserting `key → val`
- 
    /// would create a cycle.
+ 
    /// The targets in the map can change their classification from
+ 
    /// [`Target::Direct`] to [`Target::Symbolic`], if the new insertion of the
+ 
    /// `name` or `target` matches an existing key or existing entries.

    ///
- 
    /// # Returns
+ 
    /// # Errors

    ///
- 
    /// Whether `key == val` (under [`Into::into`]) or
- 
    /// `key` is reachable from `val` (under [`Into::into`] and [`Get::get`]).
- 
    pub(super) fn reachable<'a, 'b, K: PartialEq, V: 'a>(
- 
        map: &'a impl Get<'a, 'b, K, V>,
- 
        val: &'b V,
- 
        key: &'b K,
- 
    ) -> bool
- 
    where
- 
        'a: 'b,
- 
        &'b V: Into<&'b K>,
- 
    {
- 
        // Self-Reference
- 
        let src = val.into();
- 
        if *src == *key {
- 
            return true;
+ 
    /// The `target` reference must either:
+ 
    /// - Be a direct [`Qualified`] reference, or
+ 
    /// - Resolve to a direct [`Qualified`] reference, if it is a keyed entry in [`SymbolicRefs`].
+ 
    ///
+ 
    /// If neither of these is satisfied then an
+ 
    /// [`InsertionError::TargetNotQualified`] error is returned.
+ 
    ///
+ 
    /// If the `name` and `target` end up in a cycle, e.g., `a → b → a`, then an
+ 
    /// [`InsertionError::Cyclic`] error is returned.
+ 
    fn insert(&mut self, name: Name, mut target: Target) -> Result<(), InsertionError> {
+ 
        let target_str = match &target {
+ 
            Target::Direct(q) => q.as_ref().to_ref_string(),
+ 
            Target::Symbolic(s) => s.as_ref().clone(),
+ 
        };
+
+ 
        if self.is_reachable_from(&target_str, &name) {
+ 
            return Err(InsertionError::Cyclic {
+ 
                name: name.into_inner(),
+ 
                target: target_str,
+ 
            });

        }
- 
        // Chase
- 
        let mut src = src;
- 
        while let Some(tmp) = map.get(src).map(|value| value.into()) {
- 
            if *tmp == *key {
- 
                return true;
+ 
        // A Direct target whose string is already a key is actually
+ 
        // an intermediate link — downgrade to Symbolic.
+ 
        if let Target::Direct(q) = &target {
+ 
            if self.0.contains_key(&q.as_ref().to_ref_string()) {
+ 
                target = Target::symbolic(q.to_ref_string());

            }
- 
            src = tmp;

        }
- 
        false
+ 
        if let Target::Symbolic(s) = &target {
+ 
            if self.resolve(s.as_ref()).is_none() {
+ 
                return Err(InsertionError::TargetNotQualified {
+ 
                    name: name.into_inner(),
+ 
                    target: target_str,
+ 
                });
+ 
            }
+ 
        }
+
+ 
        self.reclassify_targets(&name);
+ 
        self.0.insert(name, target);
+ 
        Ok(())
+ 
    }
+
+ 
    /// When a new key is inserted, any existing `Direct` target whose
+ 
    /// qualified string matches the new key is no longer terminal — it's
+ 
    /// now an intermediate link and must be reclassified as `Symbolic`.
+ 
    fn reclassify_targets(&mut self, new_key: &Name) {
+ 
        let key = new_key.as_ref();
+ 
        for existing in self.0.values_mut() {
+ 
            if let Target::Direct(q) = existing {
+ 
                if q == key {
+ 
                    *existing = Target::symbolic(q.to_ref_string());
+ 
                }
+ 
            }
+ 
        }
+ 
    }
+ 
}
+
+ 
impl TryFrom<IndexMap<Name, Unprotected<RefString>>> for SymbolicRefs {
+ 
    type Error = InsertionError;
+
+ 
    fn try_from(map: IndexMap<Name, Unprotected<RefString>>) -> Result<Self, Self::Error> {
+ 
        map.into_iter()
+ 
            .try_fold(Self::default(), |mut result, (name, target)| {
+ 
                result.try_insert_unprotected(name, target)?;
+ 
                Ok(result)
+ 
            })

    }

}
@@ -390,4 +517,231 @@ mod test {


        assert_matches!(a.combine(b), Err(InsertionError::Cyclic { .. }));

    }
+
+ 
    /// Verifies that direct targets are stored as [`Target::Direct`].
+ 
    #[test]
+ 
    fn target_classification() {
+ 
        let symrefs = serde_json::from_value::<SymbolicRefs>(serde_json::json!({
+ 
            "HEAD": "refs/heads/main",
+ 
        }))
+ 
        .unwrap();
+
+ 
        let (_, target) = symrefs.iter().next().unwrap();
+ 
        assert_matches!(target, Target::Direct(_));
+ 
    }
+
+ 
    /// Verifies that symbolic targets are stored as [`Target::Symbolic`].
+ 
    #[test]
+ 
    fn target_classification_symbolic() {
+ 
        let symrefs = serde_json::from_value::<SymbolicRefs>(serde_json::json!({
+ 
            "MAIN": "refs/heads/master",
+ 
            "HEAD": "MAIN",
+ 
        }))
+ 
        .unwrap();
+
+ 
        let head_entry = symrefs
+ 
            .iter()
+ 
            .find_map(|(name, target)| (name.as_str() == "HEAD").then_some(target))
+ 
            .unwrap();
+ 
        assert_matches!(head_entry, Target::Symbolic(_));
+
+ 
        let main_entry = symrefs
+ 
            .iter()
+ 
            .find_map(|(name, target)| (name.as_str() == "MAIN").then_some(target))
+ 
            .unwrap();
+ 
        assert_matches!(main_entry, Target::Direct(_));
+ 
    }
+
+ 
    /// Verifies that an existing direct target can become a symbolic target
+ 
    /// during a new insertion.
+ 
    #[test]
+ 
    fn target_reclassification() {
+ 
        let mut symrefs = SymbolicRefs::default();
+ 
        symrefs
+ 
            .try_insert(refname!("HEAD"), refname!("refs/heads/main"))
+ 
            .unwrap();
+ 
        symrefs
+ 
            .try_insert(refname!("refs/heads/main"), refname!("refs/heads/master"))
+ 
            .unwrap();
+ 
        let main = symrefs
+ 
            .iter()
+ 
            .find_map(|(_, target)| {
+ 
                (target.as_refstr().as_str() == "refs/heads/main").then_some(target)
+ 
            })
+ 
            .unwrap();
+ 
        assert_matches!(main, Target::Symbolic(_));
+ 
    }
+
+ 
    /// Verifies that an existing direct target can become a symbolic target
+ 
    /// during a new insertion.
+ 
    #[test]
+ 
    fn target_reclassification_commutative() {
+ 
        let mut symrefs = SymbolicRefs::default();
+ 
        symrefs
+ 
            .try_insert(refname!("refs/heads/main"), refname!("refs/heads/master"))
+ 
            .unwrap();
+ 
        symrefs
+ 
            .try_insert(refname!("HEAD"), refname!("refs/heads/main"))
+ 
            .unwrap();
+ 
        let main = symrefs
+ 
            .iter()
+ 
            .find_map(|(_, target)| {
+ 
                (target.as_refstr().as_str() == "refs/heads/main").then_some(target)
+ 
            })
+ 
            .unwrap();
+ 
        assert_matches!(main, Target::Symbolic(_));
+ 
    }
+
+ 
    #[test]
+ 
    fn reclassification_reverse_chain() {
+ 
        let mut symrefs = SymbolicRefs::default();
+
+ 
        // Build the chain in reverse: terminal first, origin last.
+ 
        for (name, target) in [
+ 
            (refname!("refs/heads/c"), refname!("refs/heads/d")),
+ 
            (refname!("refs/heads/b"), refname!("refs/heads/c")),
+ 
            (refname!("refs/heads/a"), refname!("refs/heads/b")),
+ 
        ] {
+ 
            symrefs.try_insert(name, target).unwrap();
+ 
        }
+
+ 
        // Only refs/heads/d (the terminal) should be Direct.
+ 
        // refs/heads/b and refs/heads/c are both keys AND targets — Symbolic.
+ 
        for (_, target) in symrefs.iter() {
+ 
            match target.as_refstr().as_str() {
+ 
                "refs/heads/d" => assert_matches!(target, Target::Direct(_)),
+ 
                other => {
+ 
                    assert_matches!(target, Target::Symbolic(_), "expected Symbolic for {other}")
+ 
                }
+ 
            }
+ 
        }
+
+ 
        // Resolution should still work through the full chain.
+ 
        assert_eq!(
+ 
            symrefs
+ 
                .resolve(&refname!("refs/heads/a"))
+ 
                .map(|q| q.as_str()),
+ 
            Some("refs/heads/d"),
+ 
        );
+ 
    }
+
+ 
    #[test]
+ 
    fn reclassification_diamond() {
+ 
        let mut symrefs = SymbolicRefs::default();
+ 
        symrefs
+ 
            .try_insert(refname!("HEAD"), refname!("refs/heads/main"))
+ 
            .unwrap();
+ 
        symrefs
+ 
            .try_insert(refname!("DEFAULT"), refname!("refs/heads/main"))
+ 
            .unwrap();
+
+ 
        // Both targets are Direct — refs/heads/main is not a key yet.
+ 
        // Now make it a key:
+ 
        symrefs
+ 
            .try_insert(refname!("refs/heads/main"), refname!("refs/heads/master"))
+ 
            .unwrap();
+
+ 
        // Both HEAD and DEFAULT's targets should now be Symbolic.
+ 
        let targets_for_main: Vec<_> = symrefs
+ 
            .iter()
+ 
            .filter(|(_, t)| t.as_refstr().as_str() == "refs/heads/main")
+ 
            .collect();
+ 
        assert_eq!(targets_for_main.len(), 2);
+ 
        for (name, target) in targets_for_main {
+ 
            assert_matches!(
+ 
                target,
+ 
                Target::Symbolic(_),
+ 
                "expected Symbolic for {name}'s target"
+ 
            );
+ 
        }
+ 
    }
+
+ 
    #[test]
+ 
    fn reclassification_order_invariant() {
+ 
        // Order A: HEAD first, then the chain link.
+ 
        let a = {
+ 
            let mut s = SymbolicRefs::default();
+ 
            s.try_insert(refname!("HEAD"), refname!("refs/heads/main"))
+ 
                .unwrap();
+ 
            s.try_insert(refname!("refs/heads/main"), refname!("refs/heads/master"))
+ 
                .unwrap();
+ 
            s
+ 
        };
+
+ 
        // Order B: chain link first, then HEAD.
+ 
        let b = {
+ 
            let mut s = SymbolicRefs::default();
+ 
            s.try_insert(refname!("refs/heads/main"), refname!("refs/heads/master"))
+ 
                .unwrap();
+ 
            s.try_insert(refname!("HEAD"), refname!("refs/heads/main"))
+ 
                .unwrap();
+ 
            s
+ 
        };
+
+ 
        // Both should resolve HEAD to the same place.
+ 
        assert_eq!(a.resolve_head(), b.resolve_head());
+
+ 
        // Both should have the same classification for the refs/heads/main target.
+ 
        let classify_a = a
+ 
            .iter()
+ 
            .find(|(_, t)| t.as_refstr().as_str() == "refs/heads/main")
+ 
            .unwrap()
+ 
            .1;
+ 
        let classify_b = b
+ 
            .iter()
+ 
            .find(|(_, t)| t.as_refstr().as_str() == "refs/heads/main")
+ 
            .unwrap()
+ 
            .1;
+ 
        assert_matches!(classify_a, Target::Symbolic(_));
+ 
        assert_matches!(classify_b, Target::Symbolic(_));
+ 
    }
+
+ 
    #[test]
+ 
    fn reclassification_combine() {
+ 
        // A has HEAD → refs/heads/main (Direct)
+ 
        let mut a = SymbolicRefs::head(&refname!("main"));
+
+ 
        // B has refs/heads/main → refs/heads/master (Direct)
+ 
        let mut b = SymbolicRefs::default();
+ 
        b.try_insert(refname!("refs/heads/main"), refname!("refs/heads/master"))
+ 
            .unwrap();
+
+ 
        a.combine(b).unwrap();
+
+ 
        // After combine, HEAD's target refs/heads/main should be Symbolic.
+ 
        let main_target = a
+ 
            .iter()
+ 
            .find(|(_, t)| t.as_refstr().as_str() == "refs/heads/main")
+ 
            .unwrap()
+ 
            .1;
+ 
        assert_matches!(main_target, Target::Symbolic(_));
+ 
        assert_eq!(
+ 
            a.resolve_head().map(|q| q.as_str()),
+ 
            Some("refs/heads/master")
+ 
        );
+ 
    }
+
+ 
    #[test]
+ 
    fn reclassification_combine_reverse() {
+ 
        // B has refs/heads/main → refs/heads/master (Direct)
+ 
        let mut b = SymbolicRefs::default();
+ 
        b.try_insert(refname!("refs/heads/main"), refname!("refs/heads/master"))
+ 
            .unwrap();
+
+ 
        // A has HEAD → refs/heads/main (Direct)
+ 
        let a = SymbolicRefs::head(&refname!("main"));
+
+ 
        b.combine(a).unwrap();
+
+ 
        // HEAD's target refs/heads/main IS a key — should be Symbolic.
+ 
        let main_target = b
+ 
            .iter()
+ 
            .find_map(|(_, t)| (t.as_refstr().as_str() == "refs/heads/main").then_some(t))
+ 
            .unwrap();
+ 
        assert_matches!(main_target, Target::Symbolic(_));
+ 
        assert_eq!(
+ 
            b.resolve_head().map(|q| q.as_str()),
+ 
            Some("refs/heads/master")
+ 
        );
+ 
    }

}
modified crates/radicle/src/identity/crefs.rs
@@ -15,7 +15,7 @@ pub enum ValidationError {
    #[error("the target of the symbolic reference '{name} → {target}' is not matched by any rule")]

    Dangling {

        name: symbolic::RawName,
- 
        target: symbolic::RawTarget,
+ 
        target: Qualified<'static>,

    },



    #[error(
@@ -97,10 +97,7 @@ impl CanonicalRefs {
    /// formed set of references when interpreted together.

    pub fn new(rules: Rules, symbolic: SymbolicRefs) -> Result<Self, ValidationError> {

        for (name, target) in symbolic.iter_resolved() {
- 
            if Qualified::from_refstr(target)
- 
                .and_then(|qualified| rules.matches(&qualified).next())
- 
                .is_none()
- 
            {
+ 
            if rules.matches(target).next().is_none() {

                return Err(ValidationError::Dangling {

                    name: name.to_owned(),

                    target: target.to_owned(),
modified crates/radicle/src/identity/doc.rs
@@ -93,9 +93,6 @@ pub enum DefaultBranchError {


    #[error(transparent)]

    CanonicalRefsError(#[from] CanonicalRefsError),
-
- 
    #[error("the target of the symbolic reference `HEAD` is not qualified: {0}")]
- 
    UnqualifiedHead(git::fmt::RefString),

}



/// The version number of the identity document.
@@ -779,13 +776,10 @@ impl Doc {
    /// in this document.

    pub fn default_branch(&self) -> Result<git::fmt::Qualified<'_>, DefaultBranchError> {

        let crefs = self.canonical_refs()?;
- 
        let refname = crefs
+ 
        let qualified = crefs

            .symbolic()

            .resolve_head()

            .ok_or(DefaultBranchError::MissingHead)?;
- 
        let qualified = refname
- 
            .qualified()
- 
            .ok_or(DefaultBranchError::UnqualifiedHead(refname.clone()))?;

        Ok(qualified.to_owned())

    }
@@ -818,15 +812,15 @@ impl Doc {
        // Determine where `HEAD` comes from. The `resolve_head()` result

        // borrows `raw_crefs`, so clone to allow mutation in the synthesis

        // path.
- 
        let head = raw_crefs.symbolic().resolve_head().cloned();
+ 
        let head: Option<Qualified<'static>> = raw_crefs.symbolic().resolve_head().cloned();



        match (head, self.project()) {

            (Some(ref default_branch), Ok(project)) => {
- 
                let project_branch = project.default_branch_qualified().to_ref_string();
+ 
                let project_branch = project.default_branch_qualified();

                if project_branch != *default_branch {

                    return Err(DefaultBranchRuleError::HeadMismatch {
- 
                        cref: default_branch.clone(),
- 
                        project: project_branch,
+ 
                        cref: default_branch.to_ref_string(),
+ 
                        project: project_branch.to_ref_string(),

                    }

                    .into());

                }
@@ -854,12 +848,9 @@ impl Doc {
    fn validate_head_rule(

        &self,

        raw_crefs: &RawCanonicalRefs,
- 
        default_branch: &RefString,
+ 
        default_branch: &Qualified,

    ) -> Result<(), CanonicalRefsError> {
- 
        let Some(qualified) = Qualified::from_refstr(default_branch) else {
- 
            return Ok(());
- 
        };
- 
        let Some((pattern, rule)) = raw_crefs.raw_rules().matches(&qualified).next() else {
+ 
        let Some((pattern, rule)) = raw_crefs.raw_rules().matches(default_branch).next() else {

            return Ok(());

        };
modified crates/radicle/src/identity/doc/update.rs
@@ -234,7 +234,7 @@ pub fn verify(raw: RawDoc) -> Result<Doc, error::DocVerification> {


            if let Some(symbolic) = crefs.symbolic().resolve_head() {

                return Err(error::DocVerification::DisallowDefaultBranchSymbolic {
- 
                    symbolic: symbolic.to_owned(),
+ 
                    symbolic: symbolic.to_ref_string(),

                    default,

                });

            }
modified crates/radicle/src/storage.rs
@@ -676,7 +676,7 @@ pub trait WriteRepository: ReadRepository + SignRepository {
    /// not even exist.

    fn set_canonical_symbolic_refs(&self, message: &str) -> Result<(), RepositoryError> {

        for (name, target) in self.identity_doc()?.canonical_refs()?.symbolic().iter() {
- 
            self.set_symbolic_ref(name, target, message)?;
+ 
            self.set_symbolic_ref(name.as_ref(), target.as_ref(), message)?;

        }

        Ok(())

    }