Skip to content

fix: branch resolution#25

Open
SionoiS wants to merge 2 commits into
cryptidtech:mainfrom
SionoiS:fix--branch-resolution
Open

fix: branch resolution#25
SionoiS wants to merge 2 commits into
cryptidtech:mainfrom
SionoiS:fix--branch-resolution

Conversation

@SionoiS

@SionoiS SionoiS commented Jul 10, 2026

Copy link
Copy Markdown

My AI found this. Let me know what ya think.

Root cause: Log::verify never set Comrade’s domain, so branch("pubkey") always resolved under "/".

Change in crates/provenance-log/src/log.rs: before unlock/lock, set domain from the entry ops context (longest common branch):

let domain = entry.context().to_string();
let mut unlocked = match Comrade::new(&kvp_lock, &entry)
    .with_domain(&domain)
    .try_unlock(unlock)

That matches the README’s Mike/Walker model: lock at /delegated/, ops under /delegated/mike/ → branch("pubkey") → /delegated/mike/pubkey. Unlock scripts still use absolute paths, so setting domain before unlock is safe.

@SionoiS SionoiS changed the title fix branch resolution fix: branch resolution Jul 10, 2026
Comment thread crates/bs/src/ops/update.rs
@DougAnderson444

Copy link
Copy Markdown
Collaborator

Hey SP, thanks for the note.

It's been a while since I touched this repo, but I recall I didn't get as far as the delegation paths, so this might be totally unimplemented from what I recall.


let mut unlocked = match Comrade::new(&kvp_lock, &entry).try_unlock(unlock) {
let mut unlocked = match Comrade::new(&kvp_lock, &entry)
.with_domain(&domain)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I recall, this looks about right. I am surprised I missed this, this line was the whole point of with_domain()

@SionoiS

SionoiS commented Jul 13, 2026

Copy link
Copy Markdown
Author

Hey SP, thanks for the note.

It's been a while since I touched this repo, but I recall I didn't get as far as the delegation paths, so this might be totally unimplemented from what I recall.

Thanks for the review! I also plan to update the dependencies in the future but I have to switch back to my previous project for now. Hopefully this can be merged soon. 🙏

If everything goes according to plan this will be used as the foundational tech for the Logos Identity stack. :O I'm pretty excited.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants